diff --git a/scripts/hermes-dashboard-proxy.js b/scripts/hermes-dashboard-proxy.js index fda959f3b..fe046a1b6 100644 --- a/scripts/hermes-dashboard-proxy.js +++ b/scripts/hermes-dashboard-proxy.js @@ -99,6 +99,26 @@ const WS_HANDSHAKE_TIMEOUT_MS = envMs("HERMES_DASH_WS_TIMEOUT_MS", 15_000); // the upgrade handshake complete and stand its timeout down. const HEADER_TERMINATOR = "\r\n\r\n"; +// Prepare a string that came off the wire for a log line. Two rules, both about +// the shape of the record rather than its content: one value stays one line, and +// the record's size does not follow its input's. The same rules as +// src/lib/log-safe.ts, restated here because this script is CommonJS and runs as +// its own process, so it cannot import the TypeScript module. +// +// \p{Cc} is the Unicode "control" category: the C0 range, DEL, and C1. Replaced +// rather than stripped, so two values differing only in control characters do +// not collapse into the same line. U+FFFD is the conventional stand-in. +const LOG_CONTROL_CHARACTERS = /\p{Cc}/gu; +const LOG_FIELD_MAX_LENGTH = 200; +function logSafe(value, maxLength = LOG_FIELD_MAX_LENGTH) { + const s = String(value); + if (s.length <= maxLength) return s.replace(LOG_CONTROL_CHARACTERS, "�"); + // Cut first, then sanitise the head only: every character the pattern matches + // is one UTF-16 code unit replaced by one, so no match can straddle the cut. + const head = s.slice(0, maxLength).replace(LOG_CONTROL_CHARACTERS, "�"); + return `${head}...[+${s.length - maxLength} chars]`; +} + // Rewrite the origin part of a Referer to the upstream authority, keeping path. function rewriteReferer(value) { return typeof value === "string" ? value.replace(/^https?:\/\/[^/]+/i, UPSTREAM_ORIGIN) : value; @@ -389,7 +409,8 @@ function hermesLogin() { up.on("end", () => { const setCookies = up.headers["set-cookie"]; if (up.statusCode !== 200 || !Array.isArray(setCookies) || setCookies.length === 0) { - console.error(`[hermes-dashboard-proxy] login failed: HTTP ${up.statusCode} ${Buffer.concat(chunks).toString().slice(0, 120)}`); + // The body is the upstream's, so bound it before logging it. + console.error(`[hermes-dashboard-proxy] login failed: HTTP ${up.statusCode} ${logSafe(Buffer.concat(chunks).toString(), 120)}`); settle(null); return; } diff --git a/src/app/setup-api/ai-models/configure/route.ts b/src/app/setup-api/ai-models/configure/route.ts index 9ca89f4d5..11976067d 100644 --- a/src/app/setup-api/ai-models/configure/route.ts +++ b/src/app/setup-api/ai-models/configure/route.ts @@ -46,6 +46,12 @@ import { OPENROUTER_CURATED_MODELS, OPENROUTER_DEFAULT_MODEL_ID } from "@/lib/op import { resolveEntitledCodexModel } from "@/lib/codex-model-probe"; import { isValidModelId, isCatalogProvider, GOOGLE_MODELS, ANTHROPIC_MODELS, extractProviderModelId } from "@/lib/provider-models"; import { refreshInBackground as refreshCatalogInBackground } from "@/app/setup-api/ai-models/catalog/route"; +// The model name on this route arrives in the request body. For a local +// provider it is the whole of `apiKey`, which nothing further constrains, and +// it reaches the lines below both directly and inside a subprocess error that +// quotes the command it ran. Bound every such field before logging it — see +// src/lib/log-safe.ts. +import { logSafe } from "@/lib/log-safe"; const OPENCLAW_BIN = findOpenclawBin(); const OPENCLAW_HOME_DIR = @@ -378,7 +384,7 @@ async function ensureFallbackModel( if (fallbackCandidates.length > 0) { await setFallbackModels([fallbackCandidates[0]]); - console.log(`[AI Config] Configured local fallback model: ${fallbackCandidates[0]}`); + console.log(`[AI Config] Configured local fallback model: ${logSafe(fallbackCandidates[0])}`); return; } @@ -937,7 +943,7 @@ export async function POST(request: Request) { config.defaultModel, ]); if (shouldPromoteLocalToPrimary) { - console.log(`[AI Config] Promoted local model to active primary: ${config.defaultModel}`); + console.log(`[AI Config] Promoted local model to active primary: ${logSafe(config.defaultModel)}`); } } // Reserve sized to the active model's context window. Local models run on @@ -1099,7 +1105,7 @@ export async function POST(request: Request) { // Non-fatal: Ollama will still work, just use more memory console.warn("[AI Config] Failed to optimize Ollama service:", err instanceof Error ? err.message : err); } - console.log(`[AI Config] Set ollama provider in openclaw.json: ${modelName} (context=${OLLAMA_CONTEXT_WINDOW}, mode=replace)`); + console.log(`[AI Config] Set ollama provider in openclaw.json: ${logSafe(modelName)} (context=${OLLAMA_CONTEXT_WINDOW}, mode=replace)`); } else if (isLlamaCpp) { const modelName = config.defaultModel.replace(/^llamacpp\//, ""); const providerDef = JSON.stringify({ @@ -1123,7 +1129,7 @@ export async function POST(request: Request) { "config", "set", "models.mode", isLocalScope ? "merge" : "replace", ]); await ensureFallbackModel(shouldPromoteLocalToPrimary ? config.defaultModel : (isLocalScope ? null : config.defaultModel), config.defaultModel); - console.log(`[AI Config] Set llama.cpp provider in openclaw.json: ${modelName} (context=${llamaCppContextWindow}, mode=replace)`); + console.log(`[AI Config] Set llama.cpp provider in openclaw.json: ${logSafe(modelName)} (context=${llamaCppContextWindow}, mode=replace)`); } else if (isOpenRouter) { // OpenRouter has no native OpenClaw adapter, so without this explicit // provider entry the chat turn silently returns usage 0/0/0. @@ -1134,7 +1140,7 @@ export async function POST(request: Request) { defaultModel: config.defaultModel, curatedModels: OPENROUTER_CURATED_MODELS, }); - console.log(`[AI Config] Set openrouter provider (openai-compat): ${config.defaultModel}`); + console.log(`[AI Config] Set openrouter provider (openai-compat): ${logSafe(config.defaultModel)}`); } else if (isGoogle) { // Native google plugin registers Gemini models but its 2026.6.8 auth // fails at call time (runs fall back with reason=auth). Route through @@ -1146,7 +1152,7 @@ export async function POST(request: Request) { defaultModel: config.defaultModel, curatedModels: GOOGLE_MODELS, }); - console.log(`[AI Config] Set google provider (openai-compat): ${config.defaultModel}`); + console.log(`[AI Config] Set google provider (openai-compat): ${logSafe(config.defaultModel)}`); } else if (isAnthropic) { // Native anthropic plugin reads a per-agent sqlite auth store that // ClawBox's file auth profile doesn't populate, so it fails with @@ -1159,7 +1165,7 @@ export async function POST(request: Request) { defaultModel: config.defaultModel, curatedModels: ANTHROPIC_MODELS, }); - console.log(`[AI Config] Set anthropic provider (openai-compat): ${config.defaultModel}`); + console.log(`[AI Config] Set anthropic provider (openai-compat): ${logSafe(config.defaultModel)}`); } else { // Switching away from Ollama/ClawBox AI — reset models.mode so cloud providers // auto-detect their model catalog normally. @@ -1267,7 +1273,10 @@ export async function POST(request: Request) { // Never surface the raw error: it can carry CLI internals and filesystem // paths. Log it server-side for diagnosis and return a generic, actionable // message (mirrors the sanitized gateway-restart branch above). - console.error("[configure] Failed to configure AI model:", err instanceof Error ? err.message : err); + console.error( + "[configure] Failed to configure AI model:", + err instanceof Error ? logSafe(err.message) : err, + ); // Classify so the message matches the cause. A local on-device model has no // credentials, and an edition without the openclaw binary is not something // the user can fix by re-checking a key — "check your credentials" is wrong diff --git a/src/app/setup-api/apps/install/route.ts b/src/app/setup-api/apps/install/route.ts index 3b97d5bcc..a550a9a7a 100644 --- a/src/app/setup-api/apps/install/route.ts +++ b/src/app/setup-api/apps/install/route.ts @@ -7,6 +7,7 @@ import path from "path"; import { DATA_DIR, getAll as configGetAll, setMany as configSetMany } from "@/lib/config-store"; import { getSkillsDir, findOpenclawBin } from "@/lib/openclaw-config"; import { CATEGORY_COLORS, DEFAULT_CATEGORY_COLOR, type InstalledMeta } from "@/lib/store-categories"; +import { boundPreferenceText, sanitizePreferenceWrites } from "@/lib/preference-schema"; const STORE_SEARCH_API = "https://openclawhardware.dev/api/store/apps"; const STORE_ICONS_BASE = "https://openclawhardware.dev/store/icons"; @@ -39,8 +40,9 @@ async function lookupStoreMeta(appId: string): Promise { // in the POST handler failed. Matches what AppStore.tsx's apiToStoreApp // stores for UI-initiated installs, so both paths produce identical meta. const remoteIconUrl = `${STORE_ICONS_BASE}/${appId}.png`; + // The name ends up in a stored preference, so bound it to what one may hold. const fallback: InstalledMeta = { - name: titleCaseFromSlug(appId), + name: boundPreferenceText(titleCaseFromSlug(appId), appId), color: DEFAULT_CATEGORY_COLOR, iconUrl: remoteIconUrl, }; @@ -61,7 +63,7 @@ async function lookupStoreMeta(appId: string): Promise { ? CATEGORY_COLORS[category] : DEFAULT_CATEGORY_COLOR; return { - name: match.name ?? fallback.name, + name: boundPreferenceText(match.name, fallback.name), color, iconUrl: remoteIconUrl, }; @@ -169,8 +171,13 @@ async function syncInstalledPreferences(appId: string): Promise 0) { - await configSetMany(nextUpdates); + // This writes to the config store directly rather than through + // POST /setup-api/preferences, so the preference rules are applied here. + // The check covers the entries carried over from the read above as well as + // the one being added. See src/lib/preference-schema.ts. + const checkedUpdates = sanitizePreferenceWrites(nextUpdates); + if (Object.keys(checkedUpdates).length > 0) { + await configSetMany(checkedUpdates); } return undefined; } catch (err) { diff --git a/src/app/setup-api/code/route.ts b/src/app/setup-api/code/route.ts index fe15f3fbe..c0e066f1d 100644 --- a/src/app/setup-api/code/route.ts +++ b/src/app/setup-api/code/route.ts @@ -52,6 +52,9 @@ export async function POST(request: NextRequest) { const { projectId, name, color, description, template } = body; if (!projectId || !validateProjectId(projectId)) return err("Invalid project ID"); if (!name) return err("Project name required"); + // Shape and length are checked inside initProject too, before it creates + // anything, so the MCP door gets the same rules; a ValidationError from + // there is answered as a 400 below. const meta = await initProject(projectId, name, { color, description, template }); return ok({ success: true, project: meta }); } diff --git a/src/app/setup-api/preferences/route.ts b/src/app/setup-api/preferences/route.ts index 58c061cc6..9317ba0a7 100644 --- a/src/app/setup-api/preferences/route.ts +++ b/src/app/setup-api/preferences/route.ts @@ -14,6 +14,12 @@ function isAllowed(key: string) { return ALLOWED_PREFIXES.some((p) => key.startsWith(p)); } +// Most keys one read may name. Every caller in the app asks for a single key +// (see SettingsApp, i18n, mascot-client); the whole set is fetched with `all=1` +// instead. Bound it so the size of a response follows the store rather than the +// request. +const MAX_KEYS_PER_READ = 32; + // GET /setup-api/preferences?keys=wp_opacity,wp_bg_color // GET /setup-api/preferences?all=1 (returns all pref:* keys) // @@ -46,9 +52,19 @@ export async function GET(req: Request) { return NextResponse.json({ error: "keys or all param required" }, { status: 400 }); } const keys = keysParam.split(",").filter(isAllowed); + if (keys.length > MAX_KEYS_PER_READ) { + return NextResponse.json( + { error: `at most ${MAX_KEYS_PER_READ} keys per request` }, + { status: 400 }, + ); + } + // One read of the store rather than one per key: config.get() re-reads and + // re-parses the whole file synchronously on every call, so the work of a + // request would otherwise follow the length of its `keys` parameter. + const allConfig = await config.getAll(); const result: Record = Object.create(null); for (const key of keys) { - result[key] = await config.get(`pref:${key}`); + result[key] = allConfig[`pref:${key}`]; } return NextResponse.json(sanitizePreferences(result)); } diff --git a/src/lib/code-projects.ts b/src/lib/code-projects.ts index 529d317ce..f911dfee5 100644 --- a/src/lib/code-projects.ts +++ b/src/lib/code-projects.ts @@ -76,6 +76,29 @@ export function validateProjectId(id: string): boolean { return APP_ID_RE.test(id); } +/** Longest project name the desktop label and the starter templates carry. */ +export const MAX_PROJECT_NAME_LENGTH = 60; + +/** + * The name a project may be created with, or a ValidationError. + * + * Checked here rather than at each caller because initProject writes the + * directory and project.json before the name reaches the templates: a name the + * templates cannot render has to be refused while nothing has been created yet, + * so a rejected request leaves no project behind for the next attempt to + * collide with. The MCP door declares the same limit (`zText(60)` in + * mcp/tools/desktop.ts); this is where it is enforced. + */ +export function validateProjectName(name: unknown): string { + if (typeof name !== "string") throw new ValidationError("Project name must be a string"); + const trimmed = name.trim(); + if (!trimmed) throw new ValidationError("Project name required"); + if (trimmed.length > MAX_PROJECT_NAME_LENGTH) { + throw new ValidationError(`Project name must be at most ${MAX_PROJECT_NAME_LENGTH} characters`); + } + return trimmed; +} + /** Resolve a file path inside a project directory, preventing traversal. */ function safePath(projectId: string, filePath: string): string { if (!validateProjectId(projectId)) throw new ValidationError("Invalid project ID"); @@ -119,6 +142,8 @@ export async function initProject( opts?: { color?: string; description?: string; template?: "blank" | "app" } ): Promise { if (!validateProjectId(projectId)) throw new ValidationError("Invalid project ID"); + // Before anything is created on disk — see validateProjectName. + const projectName = validateProjectName(name); const dir = projectDir(projectId); const exists = await fs.stat(dir).catch(() => null); @@ -129,7 +154,7 @@ export async function initProject( const now = new Date().toISOString(); const meta: ProjectMeta = { projectId, - name, + name: projectName, color: opts?.color || "#f97316", description: opts?.description || "", created: now, @@ -147,14 +172,14 @@ export async function initProject( - ${escapeHtml(name)} + ${escapeHtml(projectName)} -

${escapeHtml(name)}

+

${escapeHtml(projectName)}

`, "utf-8" @@ -168,7 +193,7 @@ export async function initProject( - ${escapeHtml(name)} + ${escapeHtml(projectName)} @@ -207,8 +232,8 @@ h1 { // template-literal metacharacters (` $ \) and newlines untouched, so a name // like "`;fetch('/setup-api/...')`" would break out of the literal and run // as code when the built app loads on the ClawBox origin (stored XSS). - const commentName = name.replace(/[\r\n]+/g, " "); - const innerName = jsTemplateEscape(escapeHtml(name)); + const commentName = projectName.replace(/[\r\n]+/g, " "); + const innerName = jsTemplateEscape(escapeHtml(projectName)); await fs.writeFile( path.join(dir, "app.js"), `// ${commentName} — ClawBox Web App diff --git a/src/lib/hermes-dashboard-auth.ts b/src/lib/hermes-dashboard-auth.ts index eb6c6e3a8..836c1e4ff 100644 --- a/src/lib/hermes-dashboard-auth.ts +++ b/src/lib/hermes-dashboard-auth.ts @@ -30,6 +30,15 @@ const LOGIN_RETRY_COOLDOWN_MS = 10_000; // promise to every later caller until the server is restarted. Every request // this module issues is therefore bounded. const REQUEST_TIMEOUT_MS = 8_000; +// Every request below sets this. Node's fetch defaults to "follow", which makes +// a redirect invisible to the caller — the response that comes back is the one +// from wherever Location pointed, not from the path we asked for, and a +// redirected request can carry its body and headers there. Resolving redirects +// manually keeps each call's answer the answer to the call it made: a 3xx from +// the dashboard means the request did not reach the API, which is what the +// callers below already treat as "not signed in". Same rule and same reason as +// mcp/lib/api.ts. +const REDIRECT_POLICY = "manual" as const; async function readPassword(): Promise { try { @@ -46,10 +55,13 @@ async function login(): Promise { method: "POST", headers: { "content-type": "application/json" }, body: JSON.stringify({ provider: "basic", username: USERNAME, password: pw, next: "/" }), + redirect: REDIRECT_POLICY, // The caller's `init.signal` only covers `attempt()` below — login runs // before it and would otherwise be unbounded. signal: AbortSignal.timeout(REQUEST_TIMEOUT_MS), }); + // A redirect here is not a login: the cookies are read off THIS response, so + // treat anything that is not a 2xx as "no session". if (!res.ok) return null; const setCookies = typeof res.headers.getSetCookie === "function" ? res.headers.getSetCookie() : []; const cookie = setCookies.map((c) => c.split(";", 1)[0]).filter(Boolean).join("; "); @@ -69,6 +81,7 @@ export async function dashboardFetch(apiPath: string, init?: RequestInit): Promi const attempt = () => fetch(`${DASH_ORIGIN}${apiPath}`, { ...init, + redirect: REDIRECT_POLICY, // Callers that don't bring their own deadline still get one — no request // from this module may be able to hang indefinitely. signal: init?.signal ?? AbortSignal.timeout(REQUEST_TIMEOUT_MS), diff --git a/src/lib/preference-schema.ts b/src/lib/preference-schema.ts index fc6dd255d..eb3857c4b 100644 --- a/src/lib/preference-schema.ts +++ b/src/lib/preference-schema.ts @@ -31,6 +31,14 @@ // // The same rules run on read, so a value stored before these checks existed // stops being served rather than lingering until something overwrites it. +// +// On read the rules are applied PER ENTRY. Several preferences hold a +// collection — installed-app metadata, an icon grid, a list of open windows — +// where each member is independent of the others. A member that does not pass +// costs only itself; the rest of the collection is still served. Anything that +// writes straight to the config store rather than through +// POST /setup-api/preferences applies the same rules with +// `sanitizePreferenceWrites`, so every door agrees on what may be stored. export const PREFERENCE_LANGUAGES = [ "en", @@ -61,11 +69,31 @@ const MAX_PREFERENCE_DEPTH = 12; // eslint-disable-next-line no-control-regex const CONTROL_CHARACTERS = /[\u0000-\u001F\u007F]/; +// The same class, global, so it can be used with replace() rather than test(). +// Built from the pattern above so the two cannot drift. String.replace resets a +// global pattern's lastIndex, so it is stateless there; .test on this object +// would not be — use CONTROL_CHARACTERS for that. +const CONTROL_CHARACTERS_GLOBAL = new RegExp(CONTROL_CHARACTERS.source, "g"); + +/** How a preference is spelled in the config store. */ +export const PREFERENCE_KEY_PREFIX = "pref:"; + const CLOSED_DOMAINS: Record = { ui_language: PREFERENCE_LANGUAGES, wp_fit: WALLPAPER_FITS, }; +/** + * The domain for a key, if it has one. Own properties only: the table is a + * plain object literal, so a name such as `constructor` would otherwise resolve + * to an inherited value that is not a list of legal strings. + */ +function closedDomainFor(key: string): readonly string[] | undefined { + return Object.prototype.hasOwnProperty.call(CLOSED_DOMAINS, key) + ? CLOSED_DOMAINS[key] + : undefined; +} + export function isPreferenceLanguage(value: unknown): value is PreferenceLanguage { return typeof value === "string" && (PREFERENCE_LANGUAGES as readonly string[]).includes(value); } @@ -116,7 +144,7 @@ function checkShape(value: unknown, depth: number): string | null { * cannot be stored) and on read (so junk stored earlier is not served). */ export function validatePreference(key: string, value: unknown): PreferenceCheck { - const domain = CLOSED_DOMAINS[key]; + const domain = closedDomainFor(key); if (domain) { if (typeof value === "string" && domain.includes(value)) return { ok: true }; return { ok: false, reason: `${key} must be one of: ${domain.join(", ")}` }; @@ -138,13 +166,61 @@ export function validatePreference(key: string, value: unknown): PreferenceCheck } /** - * Drop every entry that would not be accepted on write. Used on the read path - * so a value written before validation existed cannot reach a caller — most - * importantly the agent, via the `preferences_get` tool. + * Rebuild a collection from the members that pass on their own, or return + * undefined for a value that has no members to sort through. * - * Dropping (rather than substituting a default) keeps the response honest: the - * key reads as absent, which every consumer already handles, instead of - * claiming a value the store does not hold. + * Members sit one level below the value itself, which is the depth `checkShape` + * reaches them at when it walks the value whole — so they are checked at that + * same depth here and the two agree on what passes. + */ +function keepPassingMembers(value: unknown): unknown | undefined { + if (Array.isArray(value)) { + return value.filter((item) => checkShape(item, 1) === null); + } + if (value !== null && typeof value === "object") { + // Null-prototype accumulator: the names come from stored data, so an + // assignment here must define an own property and never reach an inherited + // one such as `__proto__`. + const out: Record = Object.create(null); + for (const [k, v] of Object.entries(value as Record)) { + if (checkShape(k, 1) === null && checkShape(v, 1) === null) out[k] = v; + } + return out; + } + return undefined; +} + +/** The storable form of one preference value, or nothing. */ +export type PreferenceOutcome = { ok: true; value: unknown } | { ok: false }; + +/** + * Reduce one preference to what may be stored and served. + * + * A value that passes whole is kept whole. A collection that does not is + * rebuilt from the members that pass on their own, so one unusable member + * costs only itself and the rest of the collection survives. The rebuilt + * collection is then checked again as a whole, so the caps that apply to the + * key still hold for what comes back. + * + * A scalar has no members to keep part of, and neither does a key with a closed + * domain: those are kept whole or not at all. Returning nothing (rather than + * substituting a default) keeps the answer honest — the key reads as absent, + * which every consumer already handles, instead of claiming a value the store + * does not hold. + */ +export function sanitizePreferenceValue(key: string, value: unknown): PreferenceOutcome { + if (validatePreference(key, value).ok) return { ok: true, value }; + if (closedDomainFor(key)) return { ok: false }; + + const pruned = keepPassingMembers(value); + if (pruned === undefined) return { ok: false }; + return validatePreference(key, pruned).ok ? { ok: true, value: pruned } : { ok: false }; +} + +/** + * Apply the rules to a whole set of entries. Used on the read path so a value + * written before validation existed cannot reach a caller — most importantly + * the agent, via the `preferences_get` tool. */ export function sanitizePreferences(entries: Record): Record { // Null-prototype accumulator: `key` comes from the caller's entries, so the @@ -153,7 +229,44 @@ export function sanitizePreferences(entries: Record): Record = Object.create(null); for (const [key, value] of Object.entries(entries)) { if (value === undefined) continue; - if (validatePreference(key, value).ok) out[key] = value; + const kept = sanitizePreferenceValue(key, value); + if (kept.ok) out[key] = kept.value; } return out; } + +/** + * Reduce a set of `pref:*` config-store updates to what may be stored. + * + * For the writers that reach the config store directly instead of going through + * POST /setup-api/preferences. Those writes have to meet the same rules, and + * they usually carry entries they just read back — so anything already stored + * that no longer passes is dropped here rather than written out again. + */ +export function sanitizePreferenceWrites( + updates: Record, +): Record { + const out: Record = {}; + for (const [storeKey, value] of Object.entries(updates)) { + const key = storeKey.startsWith(PREFERENCE_KEY_PREFIX) + ? storeKey.slice(PREFERENCE_KEY_PREFIX.length) + : storeKey; + const kept = sanitizePreferenceValue(key, value); + if (kept.ok) out[storeKey] = kept.value; + } + return out; +} + +/** + * Reduce a caller-supplied label to what a preference may hold: one line, no + * longer than a stored string is allowed to be. Anything that is not a string, + * or that this leaves empty, becomes `fallback`. + */ +export function boundPreferenceText(value: unknown, fallback: string): string { + if (typeof value !== "string") return fallback; + const bounded = value + .replace(CONTROL_CHARACTERS_GLOBAL, " ") + .trim() + .slice(0, MAX_PREFERENCE_STRING_LENGTH); + return bounded || fallback; +} diff --git a/src/lib/webapp-registry.ts b/src/lib/webapp-registry.ts index 59f794f08..f1d01e8ea 100644 --- a/src/lib/webapp-registry.ts +++ b/src/lib/webapp-registry.ts @@ -1,4 +1,5 @@ import { getAll, setMany } from "@/lib/config-store"; +import { boundPreferenceText, sanitizePreferenceWrites } from "@/lib/preference-schema"; interface InstalledMeta { name: string; @@ -33,12 +34,17 @@ export async function registerWebappInPreferences( const installedMeta = (prefs["pref:installed_meta"] as Record | undefined) ?? {}; const hiddenInstalled = (prefs["pref:hidden_installed"] as string[] | undefined) ?? []; - await setMany({ + // This writes to the config store directly rather than through + // POST /setup-api/preferences, so the preference rules are applied here: the + // label is bounded before it goes in, and the whole update — including the + // entries carried over from the read above — goes through the same check the + // route applies. See src/lib/preference-schema.ts. + const updates = sanitizePreferenceWrites({ "pref:installed_apps": installedApps.includes(appId) ? installedApps : [...installedApps, appId], "pref:installed_meta": { ...installedMeta, [appId]: { - name, + name: boundPreferenceText(name, appId), color: opts.color || "#f97316", iconUrl: opts.iconUrl || "", webappUrl: opts.webappUrl || `/setup-api/webapps?app=${appId}`, @@ -47,4 +53,5 @@ export async function registerWebappInPreferences( // A freshly (re)created app shouldn't stay hidden. "pref:hidden_installed": hiddenInstalled.filter((id) => id !== appId), }); + if (Object.keys(updates).length > 0) await setMany(updates); } diff --git a/src/tests/routes/preferences-language.test.ts b/src/tests/routes/preferences-language.test.ts index e047eaec6..bbb8551cf 100644 --- a/src/tests/routes/preferences-language.test.ts +++ b/src/tests/routes/preferences-language.test.ts @@ -137,7 +137,7 @@ describe("/setup-api/preferences — language", () => { describe("an already-stored invalid locale is not served", () => { it("omits it from a keyed read", async () => { - mockGet.mockResolvedValue(STORED_JUNK_LOCALE as never); + mockGetAll.mockResolvedValue({ "pref:ui_language": STORED_JUNK_LOCALE }); const res = await GET(new Request("http://localhost/setup-api/preferences?keys=ui_language")); const body = await res.json(); expect(body).not.toHaveProperty("ui_language"); @@ -156,7 +156,7 @@ describe("/setup-api/preferences — language", () => { }); it("serves a valid stored locale unchanged", async () => { - mockGet.mockResolvedValue("bg" as never); + mockGetAll.mockResolvedValue({ "pref:ui_language": "bg" }); const res = await GET(new Request("http://localhost/setup-api/preferences?keys=ui_language")); expect(await res.json()).toEqual({ ui_language: "bg" }); }); diff --git a/src/tests/routes/preferences.test.ts b/src/tests/routes/preferences.test.ts index 5c7b62e09..8f95c890f 100644 --- a/src/tests/routes/preferences.test.ts +++ b/src/tests/routes/preferences.test.ts @@ -50,16 +50,30 @@ describe("/setup-api/preferences", () => { }); it("returns specific keys", async () => { - mockGet.mockResolvedValue(80 as never); + mockGetAll.mockResolvedValue({ "pref:wp_opacity": 80, "pref:ui_theme": "dark" }); const req = new Request("http://localhost/setup-api/preferences?keys=wp_opacity"); const res = await GET(req); const body = await res.json(); expect(body).toEqual({ wp_opacity: 80 }); - expect(mockGet).toHaveBeenCalledWith("pref:wp_opacity"); + }); + + it("reads the store once however many keys are named", async () => { + mockGetAll.mockResolvedValue({ "pref:wp_opacity": 80, "pref:ui_theme": "dark" }); + const req = new Request("http://localhost/setup-api/preferences?keys=wp_opacity,ui_theme"); + const res = await GET(req); + expect(await res.json()).toEqual({ wp_opacity: 80, ui_theme: "dark" }); + expect(mockGetAll).toHaveBeenCalledTimes(1); + }); + + it("answers 400 when more keys are named than a read may carry", async () => { + const many = Array.from({ length: 40 }, (_, i) => `ui_key${i}`).join(","); + const res = await GET(new Request(`http://localhost/setup-api/preferences?keys=${many}`)); + expect(res.status).toBe(400); + expect(mockGetAll).not.toHaveBeenCalled(); }); it("filters out non-allowed keys", async () => { - mockGet.mockResolvedValue(80 as never); + mockGetAll.mockResolvedValue({ "pref:wp_opacity": 80, "pref:bad_key": "x" }); const req = new Request("http://localhost/setup-api/preferences?keys=wp_opacity,bad_key"); const res = await GET(req); const body = await res.json(); diff --git a/src/tests/unit/code-projects.test.ts b/src/tests/unit/code-projects.test.ts index f618d5104..fd8680f3e 100644 --- a/src/tests/unit/code-projects.test.ts +++ b/src/tests/unit/code-projects.test.ts @@ -57,6 +57,7 @@ import { searchFiles, buildProject, APP_ID_RE, + MAX_PROJECT_NAME_LENGTH, WEBAPPS_DIR, ValidationError, NotFoundError, @@ -140,6 +141,41 @@ describe("code-projects", () => { await expect(initProject("../hack", "Bad")).rejects.toThrow(ValidationError); }); + describe("the name it will accept", () => { + // Checked before anything is created, so a refused name leaves no + // directory behind for the next attempt to collide with. + const badNames: Array<[string, unknown]> = [ + ["a name of the wrong type", 42], + ["a missing name", undefined], + ["an empty name", ""], + ["a name that is only spaces", " "], + ["a name past the length limit", "x".repeat(MAX_PROJECT_NAME_LENGTH + 1)], + ]; + + for (const [label, name] of badNames) { + it(`refuses ${label} without creating anything`, async () => { + mockStat.mockRejectedValue(new Error("ENOENT")); + await expect( + initProject("test-app", name as string), + ).rejects.toThrow(ValidationError); + expect(mockMkdir).not.toHaveBeenCalled(); + expect(mockWriteFile).not.toHaveBeenCalled(); + }); + } + + it("stores a name with surrounding whitespace trimmed", async () => { + mockStat.mockRejectedValue(new Error("ENOENT")); + const meta = await initProject("test-app", " Test App "); + expect(meta.name).toBe("Test App"); + }); + + it("accepts a name exactly at the length limit", async () => { + mockStat.mockRejectedValue(new Error("ENOENT")); + const name = "x".repeat(MAX_PROJECT_NAME_LENGTH); + await expect(initProject("test-app", name)).resolves.toMatchObject({ name }); + }); + }); + it("rejects duplicate project", async () => { mockStat.mockResolvedValue({ isDirectory: () => true, size: 0 } as never); await expect(initProject("exists", "Exists")).rejects.toThrow(ValidationError); diff --git a/src/tests/unit/edition-license.test.ts b/src/tests/unit/edition-license.test.ts new file mode 100644 index 000000000..a81c580c4 --- /dev/null +++ b/src/tests/unit/edition-license.test.ts @@ -0,0 +1,104 @@ +import { afterEach, describe, expect, it } from "vitest"; +import crypto from "node:crypto"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +import { verifyDualLicense } from "@/lib/edition-license"; + +/** + * The key the verifier trusts is a module constant, and it stays one. + * + * The rule these pin: the answer comes from the embedded key and from nothing + * the running device can set. The verifier reads the licence itself from the + * environment and from disk — both writable by the account the service runs as + * — so the key it checks that licence against is the one thing that has to be + * fixed. It used to be `(process.env.CLAWBOX_LICENSE_PUBKEY || EMBEDDED)`, and + * these hold that shape from coming back. + * + * Each case signs a licence with a key of its own and offers the matching + * public key through every environment name the module has ever read, plus the + * shapes that made the old expression fall through to a blank key. + */ + +const ENV_KEYS = [ + "CLAWBOX_LICENSE_PUBKEY", + "CLAWBOX_DUAL_LICENSE_PUBKEY", + "CLAWBOX_LICENSE_PUBLIC_KEY", +]; +const TOUCHED = [...ENV_KEYS, "CLAWBOX_DUAL_LICENSE", "CLAWBOX_ROOT", "CLAWBOX_DEVICE_ID"]; + +const saved = new Map(); +for (const name of TOUCHED) saved.set(name, process.env[name]); + +afterEach(() => { + for (const [name, value] of saved) { + if (value === undefined) delete process.env[name]; + else process.env[name] = value; + } +}); + +/** A licence in the module's format, signed by a freshly minted keypair. */ +function mintLicence(payload: Record) { + const { publicKey, privateKey } = crypto.generateKeyPairSync("ed25519"); + const payloadBuf = Buffer.from(JSON.stringify(payload), "utf8"); + const signature = crypto.sign(null, payloadBuf, privateKey); + return { + licence: `${payloadBuf.toString("base64url")}.${signature.toString("base64url")}`, + publicKeyPem: publicKey.export({ type: "spki", format: "pem" }) as string, + }; +} + +function perpetualDualLicence() { + return mintLicence({ feature: "dual", iat: Math.floor(Date.now() / 1000) }); +} + +describe("verifyDualLicense", () => { + it("does not accept a licence signed by a key named in the environment", () => { + const { licence, publicKeyPem } = perpetualDualLicence(); + process.env.CLAWBOX_DUAL_LICENSE = licence; + for (const name of ENV_KEYS) process.env[name] = publicKeyPem; + + expect(verifyDualLicense()).toBe(false); + }); + + it("does not accept a licence when the environment offers a blank key", () => { + // The shapes that used to win a `||` and then reduce to an empty string, + // which read as "nothing to verify against". + const { licence } = perpetualDualLicence(); + process.env.CLAWBOX_DUAL_LICENSE = licence; + + for (const blank of [" ", "\t", "\n", " \r\n ", ""]) { + for (const name of ENV_KEYS) process.env[name] = blank; + expect(verifyDualLicense()).toBe(false); + } + }); + + it("does not accept a licence read from disk and signed by an environment key", () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), "clawbox-licence-")); + fs.mkdirSync(path.join(root, "data"), { recursive: true }); + const { licence, publicKeyPem } = perpetualDualLicence(); + fs.writeFileSync(path.join(root, "data", "dual-license.txt"), licence); + + delete process.env.CLAWBOX_DUAL_LICENSE; + process.env.CLAWBOX_ROOT = root; + for (const name of ENV_KEYS) process.env[name] = publicKeyPem; + + expect(verifyDualLicense()).toBe(false); + }); + + it("answers false when there is no licence at all", () => { + delete process.env.CLAWBOX_DUAL_LICENSE; + process.env.CLAWBOX_ROOT = fs.mkdtempSync(path.join(os.tmpdir(), "clawbox-licence-")); + + expect(verifyDualLicense()).toBe(false); + }); + + it("answers false for a licence that is not in the expected format", () => { + process.env.CLAWBOX_ROOT = fs.mkdtempSync(path.join(os.tmpdir(), "clawbox-licence-")); + for (const bad of ["", ".", "no-dot", ".leading", "trailing.", "a.b"]) { + process.env.CLAWBOX_DUAL_LICENSE = bad; + expect(verifyDualLicense()).toBe(false); + } + }); +}); diff --git a/src/tests/unit/preference-schema.test.ts b/src/tests/unit/preference-schema.test.ts index f2f1fb6a1..2b7a03e24 100644 --- a/src/tests/unit/preference-schema.test.ts +++ b/src/tests/unit/preference-schema.test.ts @@ -2,11 +2,19 @@ import { describe, expect, it } from "vitest"; import { MAX_PREFERENCE_STRING_LENGTH, PREFERENCE_LANGUAGES, + boundPreferenceText, isPreferenceLanguage, sanitizePreferences, + sanitizePreferenceValue, + sanitizePreferenceWrites, validatePreference, } from "@/lib/preference-schema"; +// Written by code point rather than as a literal: an escape is easy to lose +// in an edit, and a raw control character in a source file is invisible to +// whoever reads it next. +const CONTROL = String.fromCharCode(7); + describe("preference-schema", () => { describe("ui_language closed domain", () => { it("accepts every locale the device ships", () => { @@ -118,4 +126,93 @@ describe("preference-schema", () => { expect(sanitizePreferences({ ui_language: undefined })).toEqual({}); }); }); + + describe("collections degrade one member at a time", () => { + // installed_meta is one entry per installed app under a single key. The + // desktop reads it on mount and writes back what it read, so what this + // returns for one app decides what is kept for all of them. + const installedMeta = { + notes: { name: "Notes", color: "#f97316", iconUrl: "" }, + timer: { name: `Ti${CONTROL}mer`, color: "#f97316", iconUrl: "" }, + radio: { name: "Radio", color: "#22d3ee", iconUrl: "" }, + }; + + it("keeps other entries when one is malformed", () => { + const kept = sanitizePreferenceValue("installed_meta", installedMeta); + expect(kept.ok).toBe(true); + expect(kept.ok && kept.value).toEqual({ + notes: { name: "Notes", color: "#f97316", iconUrl: "" }, + radio: { name: "Radio", color: "#22d3ee", iconUrl: "" }, + }); + }); + + it("serves the surviving entries through the read path", () => { + const out = sanitizePreferences({ installed_meta: installedMeta, wp_opacity: 80 }); + expect(Object.keys(out.installed_meta as object)).toEqual(["notes", "radio"]); + expect(out.wp_opacity).toBe(80); + }); + + it("keeps the good members of a list", () => { + const kept = sanitizePreferenceValue("installed_apps", ["notes", `ti${CONTROL}mer`, "radio"]); + expect(kept.ok && kept.value).toEqual(["notes", "radio"]); + }); + + it("keeps a whole value that already passes", () => { + const value = { notes: { name: "Notes" } }; + const kept = sanitizePreferenceValue("installed_meta", value); + expect(kept.ok && kept.value).toBe(value); + }); + + it("keeps nothing for a scalar or a closed domain, which have no members", () => { + expect(sanitizePreferenceValue("ui_user_name", `Ali${CONTROL}ce`).ok).toBe(false); + expect(sanitizePreferenceValue("ui_language", "de\n## Heading").ok).toBe(false); + }); + }); + + describe("sanitizePreferenceWrites", () => { + it("checks pref:-prefixed store keys and passes the rest through", () => { + const out = sanitizePreferenceWrites({ + "pref:installed_apps": ["notes", `ti${CONTROL}mer`], + "pref:wp_opacity": 80, + }); + expect(out).toEqual({ + "pref:installed_apps": ["notes"], + "pref:wp_opacity": 80, + }); + }); + + it("keeps a store key whose value passes unchanged", () => { + expect(sanitizePreferenceWrites({ "pref:ui_language": "bg" })).toEqual({ + "pref:ui_language": "bg", + }); + }); + }); + + describe("boundPreferenceText", () => { + it("returns a single line", () => { + expect(boundPreferenceText("Notes\napp", "fallback")).toBe("Notes app"); + expect(boundPreferenceText(`Ti${CONTROL}mer`, "fallback")).toBe("Ti mer"); + }); + + it("clamps to the longest string a preference may hold", () => { + const bounded = boundPreferenceText("x".repeat(MAX_PREFERENCE_STRING_LENGTH + 50), "fallback"); + expect(bounded).toHaveLength(MAX_PREFERENCE_STRING_LENGTH); + }); + + it("falls back for a non-string, and for anything it leaves empty", () => { + expect(boundPreferenceText(42, "fallback")).toBe("fallback"); + expect(boundPreferenceText(undefined, "fallback")).toBe("fallback"); + expect(boundPreferenceText(" ", "fallback")).toBe("fallback"); + expect(boundPreferenceText(`${CONTROL}`, "fallback")).toBe("fallback"); + }); + + it("leaves a name that is already fine alone", () => { + expect(boundPreferenceText("Notes", "fallback")).toBe("Notes"); + }); + + it("produces a value the write rules accept", () => { + const bounded = boundPreferenceText(`Ti${CONTROL}mer\nApp`, "fallback"); + expect(validatePreference("ui_user_name", bounded).ok).toBe(true); + }); + }); });