diff --git a/mcp/lib/guard.ts b/mcp/lib/guard.ts index c06b546f5..942edad32 100644 --- a/mcp/lib/guard.ts +++ b/mcp/lib/guard.ts @@ -93,7 +93,7 @@ export function isAllowedPath(abs: string): boolean { * Used by the `bash` pre-flight, which sees a shell string rather than a path. */ export const SECRET_NAME_RE = - /(^|[^\w.-])\.(ssh|hermes|openclaw|codex|gnupg|aws|kube|env|envrc|netrc|npmrc|pypirc|pgpass|git-credentials|session-secret|mcp-token|local-ai-token|hermes-dashboard-pw)(?![\w-])|(^|[^\w-])id_(rsa|ecdsa|ed25519)(?![\w-])/i; + /(^|[^\w.-])\.(ssh|hermes|openclaw|clawkeep|codex|gnupg|aws|kube|env|envrc|netrc|npmrc|pypirc|pgpass|git-credentials|session-secret|mcp-token|local-ai-token|hermes-dashboard-pw)(?![\w-])|(^|[^\w-])id_(rsa|ecdsa|ed25519)(?![\w-])/i; /** * Throw a BLOCKED_PATH the agent can act on. The message deliberately names no diff --git a/src/app/setup-api/ai-models/configure/route.ts b/src/app/setup-api/ai-models/configure/route.ts index 777de6069..19e80ac77 100644 --- a/src/app/setup-api/ai-models/configure/route.ts +++ b/src/app/setup-api/ai-models/configure/route.ts @@ -4,7 +4,8 @@ import { NextResponse } from "next/server"; import { spawn } from "child_process"; import fs from "fs/promises"; import path from "path"; -import { DATA_DIR, getAll, setMany } from "@/lib/config-store"; +import { getAll, setMany } from "@/lib/config-store"; +import { HANDOFF_TOKENS_PATH, HANDOFF_TTL_MS } from "@/lib/oauth-handoff"; import { restartGateway, findOpenclawBin, @@ -483,7 +484,6 @@ export async function POST(request: Request) { // 15-minute TTL rather than forcing a full re-auth. let pendingHandoffTokensPath: string | null = null; if (body.authMode === "subscription" && body.oauthHandoff) { - const tokensPath = path.join(DATA_DIR, "oauth-device-tokens.json"); let handoff: { provider?: string; access_token?: string; @@ -493,19 +493,22 @@ export async function POST(request: Request) { createdAt?: number; }; try { - handoff = JSON.parse(await fs.readFile(tokensPath, "utf-8")); + handoff = JSON.parse(await fs.readFile(HANDOFF_TOKENS_PATH, "utf-8")); } catch { return NextResponse.json( { error: "No pending OAuth tokens. Restart the sign-in flow." }, { status: 400 }, ); } + // A file with no `createdAt` has no age we can check, so it cannot be + // shown to be inside the TTL — treat it the same as one that is past it. if ( !handoff.access_token || - (handoff.createdAt && Date.now() - handoff.createdAt > 15 * 60 * 1000) + !handoff.createdAt || + Date.now() - handoff.createdAt > HANDOFF_TTL_MS ) { // Stale/invalid credential material — consume it so it can't linger. - await fs.unlink(tokensPath).catch(() => {}); + await fs.unlink(HANDOFF_TOKENS_PATH).catch(() => {}); return NextResponse.json( { error: "OAuth tokens missing or expired. Restart the sign-in flow." }, { status: 400 }, @@ -519,7 +522,7 @@ export async function POST(request: Request) { body.idToken = handoff.id_token; body.refreshToken = handoff.refresh_token; body.expiresIn = handoff.expires_in; - pendingHandoffTokensPath = tokensPath; + pendingHandoffTokensPath = HANDOFF_TOKENS_PATH; } const { provider, apiKey, authMode = "token", idToken, refreshToken, expiresIn, projectId, scope = "primary", model: bodyModel } = body; diff --git a/src/app/setup-api/ai-models/oauth/device-poll/route.ts b/src/app/setup-api/ai-models/oauth/device-poll/route.ts index 6a85a1940..f9c5aa054 100644 --- a/src/app/setup-api/ai-models/oauth/device-poll/route.ts +++ b/src/app/setup-api/ai-models/oauth/device-poll/route.ts @@ -3,6 +3,12 @@ import crypto from "crypto"; import fs from "fs/promises"; import path from "path"; import { DATA_DIR } from "@/lib/config-store"; +import { + HANDOFF_TOKENS_PATH, + HANDOFF_TTL_MS, + clearHandoffTokens, + sweepStaleHandoffTokens, +} from "@/lib/oauth-handoff"; import { OPENAI_CLIENT_ID, OPENAI_DEVICE_TOKEN_URL, @@ -13,7 +19,22 @@ import { export const dynamic = "force-dynamic"; const STATE_PATH = path.join(DATA_DIR, "oauth-device-state.json"); -const TOKENS_PATH = path.join(DATA_DIR, "oauth-device-tokens.json"); + +/** + * Drop both halves of a sign-in the provider ended without completing: the + * in-flight state, and any handoff tokens an earlier attempt left behind. + * + * Used on the provider-failure branches only. The expiry branch in POST drops + * the state alone, because a device-code state can be expired while the handoff + * file holds fresh tokens from the *other* (authorization-code) flow; those are + * left for the age sweep to judge on their own timestamp. + */ +async function discardFlow(): Promise { + await Promise.all([ + fs.unlink(STATE_PATH).catch(() => {}), + clearHandoffTokens(), + ]); +} interface DeviceTokens { access_token?: string; @@ -32,13 +53,13 @@ async function persistTokensAndAck( tokens: DeviceTokens, ): Promise { await fs.mkdir(DATA_DIR, { recursive: true }); - const tmpPath = `${TOKENS_PATH}.tmp.${crypto.randomBytes(8).toString("hex")}`; + const tmpPath = `${HANDOFF_TOKENS_PATH}.tmp.${crypto.randomBytes(8).toString("hex")}`; await fs.writeFile( tmpPath, JSON.stringify({ provider, ...tokens, createdAt: Date.now() }), { mode: 0o600 }, ); - await fs.rename(tmpPath, TOKENS_PATH); + await fs.rename(tmpPath, HANDOFF_TOKENS_PATH); return NextResponse.json({ status: "complete" }); } @@ -108,7 +129,7 @@ async function pollOpenAI(stored: StoredState): Promise { const verifier = pollData.code_verifier; if (!verifier) { console.error("[device-poll/openai] No code_verifier in poll response:", pollData); - await fs.unlink(STATE_PATH).catch(() => {}); + await discardFlow(); return NextResponse.json( { error: "OpenAI did not return code_verifier" }, { status: 502 } @@ -141,7 +162,7 @@ async function pollOpenAI(stored: StoredState): Promise { exchangeRes.status, errText ); - await fs.unlink(STATE_PATH).catch(() => {}); + await discardFlow(); return NextResponse.json( { error: `Token exchange failed (${exchangeRes.status})` }, { status: 502 } @@ -173,6 +194,8 @@ async function pollOpenAI(stored: StoredState): Promise { export async function POST() { try { + await sweepStaleHandoffTokens(); + let stored: StoredState; try { const raw = await fs.readFile(STATE_PATH, "utf-8"); @@ -189,7 +212,7 @@ export async function POST() { if (!stored.device_id && stored.device_auth_id) stored.device_id = stored.device_auth_id; // 15-minute expiry - if (Date.now() - stored.createdAt > 15 * 60 * 1000) { + if (Date.now() - stored.createdAt > HANDOFF_TTL_MS) { await fs.unlink(STATE_PATH).catch(() => {}); return NextResponse.json( { error: "Device auth session expired. Please start again." }, diff --git a/src/app/setup-api/ai-models/oauth/device-start/route.ts b/src/app/setup-api/ai-models/oauth/device-start/route.ts index ee0a8f6ba..2b1dac8d7 100644 --- a/src/app/setup-api/ai-models/oauth/device-start/route.ts +++ b/src/app/setup-api/ai-models/oauth/device-start/route.ts @@ -3,19 +3,19 @@ import crypto from "crypto"; import fs from "fs/promises"; import path from "path"; import { DATA_DIR } from "@/lib/config-store"; +import { clearHandoffTokens } from "@/lib/oauth-handoff"; import { DEVICE_AUTH_PROVIDERS } from "@/lib/oauth-config"; export const dynamic = "force-dynamic"; const STATE_PATH = path.join(DATA_DIR, "oauth-device-state.json"); -const TOKENS_PATH = path.join(DATA_DIR, "oauth-device-tokens.json"); export async function POST(request: Request) { try { // A new sign-in flow supersedes any prior one. Best-effort clear a stale // token-handoff file left behind by an abandoned earlier flow so it can // never be consumed by a later configure call. - await fs.unlink(TOKENS_PATH).catch(() => {}); + await clearHandoffTokens(); let body: { provider?: string } = {}; try { diff --git a/src/app/setup-api/ai-models/oauth/exchange/route.ts b/src/app/setup-api/ai-models/oauth/exchange/route.ts index 5d7cb576d..481bad1e2 100644 --- a/src/app/setup-api/ai-models/oauth/exchange/route.ts +++ b/src/app/setup-api/ai-models/oauth/exchange/route.ts @@ -5,13 +5,14 @@ import crypto from "crypto"; import fs from "fs/promises"; import path from "path"; import { DATA_DIR } from "@/lib/config-store"; +// Server-only handoff file the configure route reads on the `oauthHandoff` +// path — the SAME file the device-code flow (device-poll) uses, so both take +// its path from the one module that owns it. +import { HANDOFF_TOKENS_PATH } from "@/lib/oauth-handoff"; import { OAUTH_PROVIDERS, isGoogleConfigured } from "@/lib/oauth-config"; import { discoverGoogleProject } from "@/lib/google-project"; const STATE_PATH = path.join(DATA_DIR, "oauth-state.json"); -// Server-only handoff file the configure route reads on the `oauthHandoff` -// path — the SAME file the device-code flow (device-poll) uses. -const TOKENS_PATH = path.join(DATA_DIR, "oauth-device-tokens.json"); // Persist the freshly-issued provider tokens to a 0600 server file and return // just a status, so the access/refresh/id tokens never travel back through the @@ -24,13 +25,13 @@ async function persistTokensAndAck( extra?: { projectId?: string }, ): Promise { await fs.mkdir(DATA_DIR, { recursive: true }); - const tmpPath = `${TOKENS_PATH}.tmp.${crypto.randomBytes(8).toString("hex")}`; + const tmpPath = `${HANDOFF_TOKENS_PATH}.tmp.${crypto.randomBytes(8).toString("hex")}`; await fs.writeFile( tmpPath, JSON.stringify({ provider, ...tokens, createdAt: Date.now() }), { mode: 0o600 }, ); - await fs.rename(tmpPath, TOKENS_PATH); + await fs.rename(tmpPath, HANDOFF_TOKENS_PATH); return NextResponse.json({ status: "complete", ...(extra?.projectId ? { projectId: extra.projectId } : {}) }); } diff --git a/src/app/setup-api/ai-models/oauth/start/route.ts b/src/app/setup-api/ai-models/oauth/start/route.ts index 715c07b95..8276d0adc 100644 --- a/src/app/setup-api/ai-models/oauth/start/route.ts +++ b/src/app/setup-api/ai-models/oauth/start/route.ts @@ -5,6 +5,7 @@ import crypto from "crypto"; import fs from "fs/promises"; import path from "path"; import { DATA_DIR } from "@/lib/config-store"; +import { clearHandoffTokens } from "@/lib/oauth-handoff"; import { OAUTH_PROVIDERS, isGoogleConfigured } from "@/lib/oauth-config"; const STATE_PATH = path.join(DATA_DIR, "oauth-state.json"); @@ -15,6 +16,11 @@ function base64url(buf: Buffer): string { export async function POST(request: Request) { try { + // A new sign-in supersedes any prior one — same rule the device-code entry + // point applies, so the handoff file always belongs to the flow in progress + // rather than to whichever one was abandoned last. + await clearHandoffTokens(); + let body: { provider?: string } = {}; try { body = await request.json(); diff --git a/src/app/setup-api/preferences/route.ts b/src/app/setup-api/preferences/route.ts index 2b0a37398..58c061cc6 100644 --- a/src/app/setup-api/preferences/route.ts +++ b/src/app/setup-api/preferences/route.ts @@ -3,6 +3,7 @@ import * as config from "@/lib/config-store"; import { getActiveHarness } from "@/lib/harness"; import { sanitizePreferences, validatePreference } from "@/lib/preference-schema"; import { personaFilesFor, writeLanguagePersona } from "@/lib/language-persona"; +import { logSafe } from "@/lib/log-safe"; export const dynamic = "force-dynamic"; @@ -27,7 +28,11 @@ export async function GET(req: Request) { if (allParam) { // Return all preferences const allConfig = await config.getAll(); - const result: Record = {}; + // Null-prototype accumulator: the names come from outside this function, so + // an assignment here should always define an own property and never reach + // an inherited one such as `__proto__`. Same below, and in + // sanitizePreferences, which is where these objects end up. + const result: Record = Object.create(null); for (const [key, value] of Object.entries(allConfig)) { if (key.startsWith("pref:")) { result[key.slice(5)] = value; @@ -41,7 +46,7 @@ export async function GET(req: Request) { return NextResponse.json({ error: "keys or all param required" }, { status: 400 }); } const keys = keysParam.split(",").filter(isAllowed); - const result: Record = {}; + const result: Record = Object.create(null); for (const key of keys) { result[key] = await config.get(`pref:${key}`); } @@ -61,7 +66,10 @@ export async function POST(req: Request) { if (!isAllowed(key)) continue; const check = validatePreference(key, value); if (!check.ok) { - console.error(`[preferences] Rejected write: ${check.reason}`); + // The reason is built from the rejected key, which is caller-supplied + // and only prefix-checked — bound and sanitise it like any other + // request-derived log field. + console.error(`[preferences] Rejected write: ${logSafe(check.reason ?? "")}`); return NextResponse.json({ error: check.reason ?? "Invalid preference value" }, { status: 400 }); } entries[`pref:${key}`] = value; diff --git a/src/app/setup-api/wifi/update/route.ts b/src/app/setup-api/wifi/update/route.ts index 01e9f4473..ab293e6ad 100644 --- a/src/app/setup-api/wifi/update/route.ts +++ b/src/app/setup-api/wifi/update/route.ts @@ -1,6 +1,7 @@ import { NextResponse } from "next/server"; import { execFile } from "child_process"; import { promisify } from "util"; +import { logSafe } from "@/lib/log-safe"; const execFileAsync = promisify(execFile); @@ -8,6 +9,10 @@ export const dynamic = "force-dynamic"; const AP_PROFILE = "ClawBox-Setup"; +// 802.11 defines the SSID element as at most 32 octets, so a longer value +// cannot name a network nmcli could act on. +const SSID_MAX_OCTETS = 32; + export async function POST(request: Request) { let body: { ssid?: string; password?: string; action?: "update" | "forget" }; try { body = await request.json(); } catch { return NextResponse.json({ error: "Invalid JSON" }, { status: 400 }); } @@ -18,6 +23,9 @@ export async function POST(request: Request) { } const normalizedSsid = (body.ssid ?? "").trim(); if (!normalizedSsid) return NextResponse.json({ error: "Network name is required" }, { status: 400 }); + if (Buffer.byteLength(normalizedSsid, "utf8") > SSID_MAX_OCTETS) { + return NextResponse.json({ error: `Network name must be at most ${SSID_MAX_OCTETS} bytes` }, { status: 400 }); + } if (normalizedSsid === AP_PROFILE) return NextResponse.json({ error: "Cannot modify the hotspot profile here" }, { status: 400 }); try { @@ -42,16 +50,19 @@ export async function POST(request: Request) { } catch (err) { connected = false; reactivateError = err instanceof Error ? err.message : "Failed to reconnect"; - console.warn(`[wifi/update] reactivate ${normalizedSsid} failed:`, err); + console.warn(`[wifi/update] reactivate ${logSafe(normalizedSsid)} failed: ${logSafe(reactivateError)}`); } return NextResponse.json({ success: true, action: "update", connected, reactivateError }); } catch (err) { // The `connection modify` argv includes `wifi-sec.psk `, which // execFile embeds into its error message — scrub the PSK before logging so - // it doesn't land in the journal in cleartext. + // it doesn't land in the journal in cleartext. The scrubbed text still + // carries the SSID nmcli echoed back, so it goes through logSafe too. const raw = err instanceof Error ? err.message : String(err); - const safe = body.password ? raw.split(String(body.password)).join("***") : raw; - console.warn(`[wifi/update] ${action} ${normalizedSsid} failed: ${safe}`); + // replaceAll rather than split/join: the message is bounded by execFile's + // 1 MB maxBuffer, and split would allocate an array of every fragment of it. + const safe = body.password ? raw.replaceAll(String(body.password), "***") : raw; + console.warn(`[wifi/update] ${action} ${logSafe(normalizedSsid)} failed: ${logSafe(safe)}`); return NextResponse.json({ error: "Failed to update WiFi network" }, { status: 500 }); } } diff --git a/src/lib/file-guard.ts b/src/lib/file-guard.ts index 4be8bf871..b5f94b506 100644 --- a/src/lib/file-guard.ts +++ b/src/lib/file-guard.ts @@ -6,11 +6,14 @@ import { DATA_DIR } from "./config-store"; // // The Files API browses the home directory, so its every secret store lives // *inside* the sandbox root — `..` containment alone doesn't protect them. This -// denylist keeps credential/key material off the read, write, list, rename and +// module keeps credential/key material off the read, write, list, rename and // download paths. Matched against the realpath'd path so an in-base symlink // can't dodge the check (CWE-59). (realpath resolves symlinks, not hard links — // a hard link to a secret already needs read access to create, a separate // fuller-privilege surface.) +// +// Two shapes of rule: named credential stores elsewhere in the home directory +// are listed below, and the ClawBox data directory is covered by containment. const PROTECTED_DIR_RES: RegExp[] = [ /(^|\/)\.ssh(\/|$)/, @@ -20,6 +23,10 @@ const PROTECTED_DIR_RES: RegExp[] = [ // keys) and auth.json (OAuth tokens) — the Hermes equivalent of ~/.openclaw. /(^|\/)\.hermes(\/|$)/, /(^|\/)\.codex(\/|$)/, + // ClawKeep keeps its portal token and the device's backup-encryption + // passphrase in ~/.clawkeep. Its API route is already classed as sensitive + // in middleware.ts; this is the same rule applied to the store behind it. + /(^|\/)\.clawkeep(\/|$)/, /(^|\/)\.gnupg(\/|$)/, /(^|\/)\.aws(\/|$)/, /(^|\/)\.kube(\/|$)/, @@ -39,17 +46,68 @@ const PROTECTED_FILE_RES: RegExp[] = [ /(^|\/)\.config\/git\/credentials$/, ]; -// Exact secret files in the ClawBox data dir: the session-secret (forge cookies), -// the service bearer tokens, and the config/kv stores that carry provider keys. -// `.hermes-dashboard-pw` is the server-side password the dashboard proxy logs in -// with — reading it is a full sign-in to the Hermes dashboard. -const PROTECTED_FILES = new Set( - [".session-secret", ".mcp-token", ".local-ai-token", ".hermes-dashboard-pw", "config.json", "kv.json"] - .map((n) => path.join(DATA_DIR, n)), -); +// ── The ClawBox data directory ────────────────────────────────────────────── +// +// DATA_DIR is server state rather than user content: the config and kv stores, +// the service bearer tokens, the session secret, the OAuth flow files, tunnel +// and network state, the local-model runtime. The rule for it is containment — +// everything under it is protected except the subtrees below, which hold +// material the desktop is meant to show. +// +// Containment rather than a list of filenames, because a list cannot describe +// this directory even in principle: an atomic write stages `.tmp.` +// beside its target, so some of what lands here is named at runtime. A +// hand-maintained list also only describes the code as it was when the list was +// last edited. +// +// DATA_DIR *itself* is deliberately not protected. The Files API filters a +// directory listing entry by entry, so keeping the directory openable is what +// lets the public subtrees below appear at all. +// +// The names are spelled out here rather than imported from the modules that +// own them (code-projects, llamacpp-server, the app-store routes) because those +// import graphs use the "@/" alias, and mcp/lib/guard.ts — which consumes this +// file — may only import modules whose whole graph is relative paths and node +// builtins. Read the import rule at the top of mcp/lib/guard.ts before changing +// this: an import here breaks the MCP server at startup, not at build time. +const DATA_DIR_PUBLIC_SUBTREES = new Set([ + "webapps", // built desktop webapps, also served by the webapps route + "icons", // installed-app icons, also served by the icon route + "catalog-cache", // cached copies of the providers' public model catalogues + "code-projects", // the code assistant's project sources + "llamacpp", // local-model runtime: downloaded weights, pid file, log +]); + +// DATA_DIR is already absolute and normalised (config-store builds it with +// path.join off an absolute root), so a prefix test is all this needs. +const DATA_DIR_PREFIX = DATA_DIR + path.sep; + +/** + * Takes an already-normalised absolute path — every caller resolves before + * calling, and isProtectedFilePath's realpath pass re-checks anything that + * exists on disk, so a `..` segment cannot survive into a real lookup. + * + * Deliberately a prefix test rather than path.relative: this runs once per + * entry in a directory listing (up to 20k on a search), where path.relative's + * two normalisation passes and its segment array cost about six times the rest + * of the guard put together. + */ +function isProtectedDataDirPath(abs: string): boolean { + // Both the data dir itself and a sibling such as `data-backup` fail this + // test — the first for want of a trailing separator, the second on the name. + if (!abs.startsWith(DATA_DIR_PREFIX)) return false; + const rest = abs.slice(DATA_DIR_PREFIX.length); + // Only the first segment matters, so find one separator instead of splitting + // the whole path. Splitting on a character class of both separators would + // also be wrong on POSIX, where a backslash is a legal filename character. + const cut = rest.indexOf(path.sep); + const top = cut === -1 ? rest : rest.slice(0, cut); + if (top === "" || top === "..") return false; + return !DATA_DIR_PUBLIC_SUBTREES.has(top); +} function isProtected(abs: string): boolean { - if (PROTECTED_FILES.has(abs)) return true; + if (isProtectedDataDirPath(abs)) return true; if (PROTECTED_FILE_RES.some((re) => re.test(abs))) return true; return PROTECTED_DIR_RES.some((re) => re.test(abs)); } diff --git a/src/lib/log-safe.ts b/src/lib/log-safe.ts new file mode 100644 index 000000000..113cc8361 --- /dev/null +++ b/src/lib/log-safe.ts @@ -0,0 +1,43 @@ +/** + * Prepare an untrusted string for a log line. + * + * Two rules, both about the shape of the record rather than its content: + * + * - one value stays one line. Control characters are replaced, so a value + * carrying CR/LF cannot become extra log records, and one carrying ESC is + * read as text by a terminal rather than acted on as an escape sequence. + * - the record's size does not follow its input's. A long value is cut to + * `maxLength` with a count of what was dropped, so the caller of an API does + * not decide how much gets written per call. This half is not a nicety: + * capping the field is what keeps a stream of these lines bounded. + * + * Takes a string, not `unknown` — an `Error` or a plain object formats itself + * in ways the caller should choose. + */ + +// \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 log line. +// +// Use it with .replace only. String.replace resets a global pattern's +// lastIndex, so it is stateless here; .test on the same object would not be. +const CONTROL_CHARACTERS = /\p{Cc}/gu; + +// U+FFFD REPLACEMENT CHARACTER — the conventional stand-in for a character that +// cannot be shown. Written by code point rather than as a literal so the glyph +// does not read as mojibake in an editor. +const REPLACEMENT = String.fromCharCode(0xfffd); + +/** Default cap for a single logged field. */ +export const LOG_FIELD_MAX_LENGTH = 200; + +export function logSafe(value: string, maxLength: number = LOG_FIELD_MAX_LENGTH): string { + if (value.length <= maxLength) return value.replace(CONTROL_CHARACTERS, REPLACEMENT); + // Cut first, then sanitise the head only. Every character the pattern matches + // is one UTF-16 code unit replaced by one, so sanitising cannot change any + // index and no match can straddle the cut — this gives the same string as + // sanitising the whole value would, without walking a caller-sized input to + // produce a bounded line. An execFile error message can be a megabyte. + const head = value.slice(0, maxLength).replace(CONTROL_CHARACTERS, REPLACEMENT); + return `${head}...[+${value.length - maxLength} chars]`; +} diff --git a/src/lib/oauth-handoff.ts b/src/lib/oauth-handoff.ts new file mode 100644 index 000000000..2a68cf01e --- /dev/null +++ b/src/lib/oauth-handoff.ts @@ -0,0 +1,52 @@ +/** + * The server-side OAuth token handoff file. + * + * One file with five call sites: the device-code flow (`oauth/device-poll`) and + * the authorization-code flow (`oauth/exchange`) both write it, the configure + * route consumes it, and both flow entry points (`oauth/device-start`, + * `oauth/start`) clear it before starting a new sign-in. + * + * Its path and its lifetime live here so those call sites cannot describe the + * same file differently — a second spelling of either is a divergence nobody + * notices until the two disagree. + */ + +import fs from "fs/promises"; +import path from "path"; +import { DATA_DIR } from "./config-store"; + +export const HANDOFF_TOKENS_PATH = path.join(DATA_DIR, "oauth-device-tokens.json"); + +/** + * How long a sign-in may stay in flight. The configure route refuses handoff + * material older than this, and the sweep below removes it. + */ +export const HANDOFF_TTL_MS = 15 * 60 * 1000; + +/** + * Best-effort removal of the handoff file. Called when a new flow starts and + * when a flow ends without completing, so a sign-in leaves nothing behind. + */ +export async function clearHandoffTokens(): Promise { + await fs.unlink(HANDOFF_TOKENS_PATH).catch(() => {}); +} + +/** + * Remove a handoff file older than the TTL. Past that age configure refuses it, + * so the file has no reader left and its age alone is reason enough to drop it. + * + * Ages by mtime rather than by the `createdAt` the writers record inside the + * file: this runs on a polled endpoint, and mtime costs one stat instead of a + * read plus a parse. The two agree, because the file is written once by an + * atomic rename and never updated in place. + */ +export async function sweepStaleHandoffTokens(): Promise { + try { + const stat = await fs.stat(HANDOFF_TOKENS_PATH); + if (Date.now() - stat.mtimeMs > HANDOFF_TTL_MS) { + await clearHandoffTokens(); + } + } catch { + // No handoff file — nothing to sweep. + } +} diff --git a/src/lib/preference-schema.ts b/src/lib/preference-schema.ts index 755a87e91..fc6dd255d 100644 --- a/src/lib/preference-schema.ts +++ b/src/lib/preference-schema.ts @@ -147,7 +147,10 @@ export function validatePreference(key: string, value: unknown): PreferenceCheck * claiming a value the store does not hold. */ export function sanitizePreferences(entries: Record): Record { - const out: Record = {}; + // Null-prototype accumulator: `key` comes from the caller's entries, so the + // assignment below should always define an own property. This is the object + // that actually reaches the response, so the rule has to hold here. + const out: Record = Object.create(null); for (const [key, value] of Object.entries(entries)) { if (value === undefined) continue; if (validatePreference(key, value).ok) out[key] = value; diff --git a/src/tests/routes/ai-models/configure.test.ts b/src/tests/routes/ai-models/configure.test.ts index de2115ccf..7faddf62a 100644 --- a/src/tests/routes/ai-models/configure.test.ts +++ b/src/tests/routes/ai-models/configure.test.ts @@ -1111,4 +1111,32 @@ describe("POST /setup-api/ai-models/configure", () => { expect(res.status).toBe(400); expect(body.error).toContain("expired"); }); + + it("rejects and removes a handoff file that carries no createdAt", async () => { + // Without a timestamp the file's age is unknown, so it cannot be shown to + // be inside the TTL — it is refused like an expired one, and removed rather + // than left on disk for the next request to find. + mockFs.readFile.mockImplementation(async (file) => + String(file).endsWith("oauth-device-tokens.json") + ? JSON.stringify({ + provider: "openai", + access_token: "access.token.jwt", + id_token: "id.token.jwt", + }) + : JSON.stringify({ version: 1, profiles: {} }), + ); + + const res = await configurePost(jsonRequest({ + provider: "openai", + authMode: "subscription", + oauthHandoff: true, + })); + const body = await res.json(); + + expect(res.status).toBe(400); + expect(body.error).toContain("expired"); + expect(mockFs.unlink).toHaveBeenCalledWith( + expect.stringContaining("oauth-device-tokens.json"), + ); + }); }); diff --git a/src/tests/routes/ai-models/device-poll.test.ts b/src/tests/routes/ai-models/device-poll.test.ts index 756467baa..734dd6eb8 100644 --- a/src/tests/routes/ai-models/device-poll.test.ts +++ b/src/tests/routes/ai-models/device-poll.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it, vi, beforeEach, afterEach } from "vitest"; import fsp from "fs/promises"; +import path from "path"; vi.mock("fs/promises", () => ({ default: { @@ -8,9 +9,15 @@ vi.mock("fs/promises", () => ({ writeFile: vi.fn(), rename: vi.fn(), mkdir: vi.fn(), + stat: vi.fn(), }, })); +// Built the same way the route builds them, so the expectations hold on a +// developer's machine as well as on the device. +const STATE_PATH = path.join("/test/data", "oauth-device-state.json"); +const TOKENS_PATH = path.join("/test/data", "oauth-device-tokens.json"); + vi.mock("@/lib/config-store", () => ({ DATA_DIR: "/test/data", })); @@ -44,6 +51,8 @@ describe("POST /setup-api/ai-models/oauth/device-poll", () => { mockFs.writeFile.mockResolvedValue(); mockFs.rename.mockResolvedValue(); mockFs.mkdir.mockResolvedValue(undefined); + // Default: no handoff file on disk, so the age sweep is a no-op. + mockFs.stat.mockRejectedValue(new Error("ENOENT")); vi.stubGlobal("fetch", vi.fn()); @@ -307,6 +316,66 @@ describe("POST /setup-api/ai-models/oauth/device-poll", () => { expect(body.status).toBe("pending"); }); + // A flow that ends without completing should leave nothing behind: the state + // file and the token handoff file are two halves of the same flow, so both go. + describe("interrupted flow cleanup", () => { + it("removes the handoff tokens when the token exchange fails", async () => { + vi.stubGlobal("fetch", vi.fn() + .mockResolvedValueOnce({ + ok: true, + json: () => Promise.resolve({ + authorization_code: "test-auth-code", + code_verifier: "test-verifier", + }), + }) + .mockResolvedValueOnce({ + ok: false, + status: 400, + text: () => Promise.resolve("Invalid code"), + })); + + const res = await devicePollPost(); + + expect(res.status).toBe(502); + expect(mockFs.unlink).toHaveBeenCalledWith(TOKENS_PATH); + expect(mockFs.unlink).toHaveBeenCalledWith(STATE_PATH); + }); + + it("removes the handoff tokens when the provider returns no code_verifier", async () => { + vi.stubGlobal("fetch", vi.fn().mockResolvedValue({ + ok: true, + json: () => Promise.resolve({ authorization_code: "test-auth-code" }), + })); + + const res = await devicePollPost(); + + expect(res.status).toBe(502); + expect(mockFs.unlink).toHaveBeenCalledWith(TOKENS_PATH); + expect(mockFs.unlink).toHaveBeenCalledWith(STATE_PATH); + }); + + it("sweeps a handoff file that is past the TTL", async () => { + mockFs.stat.mockResolvedValue({ mtimeMs: Date.now() - 20 * 60 * 1000 } as never); + mockFs.readFile.mockRejectedValue(new Error("ENOENT")); + + const res = await devicePollPost(); + + // No state file, so the poll itself is a 400 — the sweep runs regardless. + expect(res.status).toBe(400); + expect(mockFs.unlink).toHaveBeenCalledWith(TOKENS_PATH); + }); + + it("leaves a handoff file that is still inside the TTL", async () => { + mockFs.stat.mockResolvedValue({ mtimeMs: Date.now() - 60 * 1000 } as never); + vi.stubGlobal("fetch", vi.fn().mockResolvedValue({ ok: false, status: 403 })); + + const res = await devicePollPost(); + + expect((await res.json()).status).toBe("pending"); + expect(mockFs.unlink).not.toHaveBeenCalledWith(TOKENS_PATH); + }); + }); + it("returns pending for unknown response format", async () => { const mockFetch = vi.fn().mockResolvedValue({ ok: true, diff --git a/src/tests/routes/files/path.test.ts b/src/tests/routes/files/path.test.ts index 511e33640..373247541 100644 --- a/src/tests/routes/files/path.test.ts +++ b/src/tests/routes/files/path.test.ts @@ -24,8 +24,14 @@ function createParams(pathSegments: string[]): { params: Promise<{ path: string[ return { params: Promise.resolve({ path: pathSegments }) }; } +// Point CLAWBOX_ROOT at the same temp tree as FILES_ROOT so the ClawBox data +// dir lands *inside* the browse root — which is the real arrangement on the +// device, where the browse root is $HOME and the data dir sits under it. +const DATA_DIR = path.join(TEST_ROOT, "data"); + beforeAll(async () => { process.env.FILES_ROOT = TEST_ROOT; + process.env.CLAWBOX_ROOT = TEST_ROOT; await fsp.mkdir(TEST_ROOT, { recursive: true }); vi.resetModules(); ({ GET: filesPathGet, PUT: filesPathPut, DELETE: filesPathDelete } = await import("@/app/setup-api/files/[...path]/route")); @@ -38,6 +44,7 @@ beforeEach(async () => { afterAll(async () => { delete process.env.FILES_ROOT; + delete process.env.CLAWBOX_ROOT; await fsp.rm(TEST_ROOT, { recursive: true, force: true }); }); @@ -190,6 +197,82 @@ describe("PUT /setup-api/files/[...path]", () => { }); }); +describe("the ClawBox data directory through the files route", () => { + // file-guard's own suite owns the inventory; these rows exist to pin that the + // route is wired to it — a long-standing store, a name an atomic write + // generates at runtime, a name that does not exist yet, and a nested one. + const serverState = [ + ["a long-standing store", ["data", "config.json"]], + ["a runtime-named sidecar", ["data", "oauth-device-tokens.json.tmp.deadbeef"]], + ["a name added after this test was written", ["data", "some-future-store.json"]], + ["a nested file", ["data", "cloudflared", "cert.pem"]], + ] as const; + + beforeEach(async () => { + for (const [, segments] of serverState) { + const abs = path.join(TEST_ROOT, ...segments); + await fsp.mkdir(path.dirname(abs), { recursive: true }); + await fsp.writeFile(abs, "server state"); + } + }); + + it.each(serverState)("does not download %s", async (_label, segments) => { + const res = await filesPathGet( + createRequest(`/setup-api/files/${segments.join("/")}`), + createParams([...segments]), + ); + expect(res.status).toBe(400); + expect((await res.json()).error).toBe("Invalid path"); + }); + + it.each(serverState)("does not delete %s", async (_label, segments) => { + const res = await filesPathDelete( + createRequest(`/setup-api/files/${segments.join("/")}`, { method: "DELETE" }), + createParams([...segments]), + ); + expect(res.status).toBe(400); + expect(fs.existsSync(path.join(TEST_ROOT, ...segments))).toBe(true); + }); + + it("does not rename a data-dir file out of the way", async () => { + const res = await filesPathPut( + createRequest("/setup-api/files/data/config.json", { + method: "PUT", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ newName: "config.txt" }), + }), + createParams(["data", "config.json"]), + ); + expect(res.status).toBe(400); + expect(fs.existsSync(path.join(DATA_DIR, "config.json"))).toBe(true); + }); + + it.each([ + ["webapps", ["data", "webapps", "demo", "index.html"]], + ["code-projects", ["data", "code-projects", "my-app", "app.js"]], + ] as const)("still downloads from the public subtree %s", async (_label, segments) => { + const abs = path.join(TEST_ROOT, ...segments); + await fsp.mkdir(path.dirname(abs), { recursive: true }); + await fsp.writeFile(abs, "public content"); + + const res = await filesPathGet( + createRequest(`/setup-api/files/${segments.join("/")}`), + createParams([...segments]), + ); + expect(res.status).toBe(200); + expect(new TextDecoder().decode(await res.arrayBuffer())).toBe("public content"); + }); + + it("still reaches an ordinary file elsewhere under the browse root", async () => { + fs.writeFileSync(path.join(TEST_ROOT, "notes.txt"), "mine"); + const res = await filesPathGet( + createRequest("/setup-api/files/notes.txt"), + createParams(["notes.txt"]), + ); + expect(res.status).toBe(200); + }); +}); + describe("DELETE /setup-api/files/[...path]", () => { it("deletes a file", async () => { fs.writeFileSync(path.join(TEST_ROOT, "todelete.txt"), "content"); diff --git a/src/tests/routes/wifi-saved-update.test.ts b/src/tests/routes/wifi-saved-update.test.ts index 928e1daa5..8457431f8 100644 --- a/src/tests/routes/wifi-saved-update.test.ts +++ b/src/tests/routes/wifi-saved-update.test.ts @@ -87,6 +87,28 @@ describe("/setup-api/wifi/update", () => { expect(res.status).toBe(400); }); + it("rejects an SSID longer than the 32 octets 802.11 allows", async () => { + const mod = await import("@/app/setup-api/wifi/update/route"); + const res = await mod.POST(makeRequest({ action: "forget", ssid: "a".repeat(33) })); + expect(res.status).toBe(400); + expect(execFileMock).not.toHaveBeenCalled(); + }); + + it("counts SSID length in octets, not code points", async () => { + const mod = await import("@/app/setup-api/wifi/update/route"); + // 17 Cyrillic characters = 34 octets in UTF-8: under the limit counted by + // code point, over it counted the way 802.11 counts. + const res = await mod.POST(makeRequest({ action: "forget", ssid: "мрежа".repeat(3) + "аб" })); + expect(res.status).toBe(400); + }); + + it("accepts an SSID of exactly 32 octets", async () => { + execFileMock.mockReturnValue({ stdout: "" }); + const mod = await import("@/app/setup-api/wifi/update/route"); + const res = await mod.POST(makeRequest({ action: "forget", ssid: "a".repeat(32) })); + expect(res.status).toBe(200); + }); + it("refuses to modify the hotspot profile", async () => { const mod = await import("@/app/setup-api/wifi/update/route"); const res = await mod.POST(makeRequest({ action: "forget", ssid: "ClawBox-Setup" })); diff --git a/src/tests/unit/file-guard.test.ts b/src/tests/unit/file-guard.test.ts index 89c45df31..2546c0dc7 100644 --- a/src/tests/unit/file-guard.test.ts +++ b/src/tests/unit/file-guard.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it, beforeAll, afterAll, beforeEach } from "vitest"; +import { describe, expect, it, beforeAll, afterAll } from "vitest"; import fs from "fs"; import os from "os"; import path from "path"; @@ -44,10 +44,106 @@ describe("isProtectedFilePath", () => { expect(guard.isProtectedFilePath(p)).toBe(true); }); - it("flags the ClawBox data-dir secrets", () => { - for (const n of [".session-secret", ".mcp-token", ".local-ai-token", ".hermes-dashboard-pw", "config.json", "kv.json"]) { - expect(guard.isProtectedFilePath(path.join(DATA_DIR, n))).toBe(true); - } + // The inventory the product actually writes under DATA_DIR, as of this + // commit. It is here to show the rule covers the real directory — the rule + // itself is pinned by the runtime-name and unknown-name cases below, which do + // not depend on this list staying current. + it.each([ + // Tokens and stores. + ".session-secret", + ".mcp-token", + ".local-ai-token", + ".local-ai-token-migrated", + ".hermes-dashboard-pw", + "config.json", + "kv.json", + "clawbox.db", + "dual-license.txt", + // OAuth flow files. + "oauth-device-tokens.json", + "oauth-device-state.json", + "oauth-state.json", + "oauth-org.json", + "clawai-connect-state.json", + // Credentials-change and login state. + ".chpasswd-input", + ".login-attempts.json", + // Tunnel and network state. + "tunnel-state.json", + "tunnel.pid", + "tunnel-url.txt", + "control-ui-origins.json", + "network.env", + "hotspot.env", + "ap-runtime.env", + "hostname.env", + "wifi-scan-cache.json", + ])("flags the data-dir server-state file %s", (n) => { + expect(guard.isProtectedFilePath(path.join(DATA_DIR, n))).toBe(true); + }); + + it("flags a nested file under a protected data-dir subtree", () => { + expect(guard.isProtectedFilePath(path.join(DATA_DIR, "cloudflared"))).toBe(true); + expect(guard.isProtectedFilePath(path.join(DATA_DIR, "cloudflared", "cert.pem"))).toBe(true); + }); + + // The rule is containment, so it has to hold for names that no list could + // carry: one the code generates at runtime, and one that does not exist yet. + it("flags a runtime-named atomic-write sidecar", () => { + expect( + guard.isProtectedFilePath(path.join(DATA_DIR, "oauth-device-tokens.json.tmp.deadbeef")), + ).toBe(true); + expect(guard.isProtectedFilePath(path.join(DATA_DIR, "config.json.tmp"))).toBe(true); + }); + + it("flags a data-dir file this test has never heard of", () => { + expect(guard.isProtectedFilePath(path.join(DATA_DIR, "some-future-store.json"))).toBe(true); + expect(guard.isProtectedFilePath(path.join(DATA_DIR, "future-dir", "nested", "x.bin"))).toBe(true); + }); + + // POSIX only: on Windows a backslash really is a separator, so the question + // does not arise. On the device it is a legal filename character, and a name + // containing one is a single entry in the data dir — not a path into the + // public subtree its first half happens to spell. + it.skipIf(path.sep !== "/")("reads a backslash in a name as part of the name", () => { + expect(guard.isProtectedFilePath(`${DATA_DIR}/webapps\\evil`)).toBe(true); + expect(guard.isProtectedFilePath(`${DATA_DIR}/icons\\..\\config.json`)).toBe(true); + }); + + it("keeps the data dir itself listable so its public subtrees can be reached", () => { + // The Files API filters a listing entry by entry; a protected DATA_DIR + // would make the whole directory unopenable and hide the subtrees below. + expect(guard.isProtectedFilePath(DATA_DIR)).toBe(false); + }); + + it.each([ + "webapps", + "icons", + "catalog-cache", + "code-projects", + "llamacpp", + ])("does not over-block the public data-dir subtree %s", (sub) => { + expect(guard.isProtectedFilePath(path.join(DATA_DIR, sub))).toBe(false); + expect(guard.isProtectedFilePath(path.join(DATA_DIR, sub, "demo", "index.html"))).toBe(false); + }); + + it("does not over-block a sibling of the data dir", () => { + expect(guard.isProtectedFilePath(path.join(TEST_ROOT, "data-backup", "notes.txt"))).toBe(false); + expect(guard.isProtectedFilePath(path.join(TEST_ROOT, "src", "index.ts"))).toBe(false); + }); + + it.each([ + `${home}/.clawkeep`, + `${home}/.clawkeep/token`, + `${home}/.clawkeep/passphrase`, + `${home}/.clawkeep/config.toml`, + ])("flags the backup tool's store %s", (p) => { + expect(guard.isProtectedFilePath(p)).toBe(true); + }); + + it("does not over-block a name that merely starts with .clawkeep", () => { + expect(guard.isProtectedFilePath(`${home}/.clawkeep-notes.txt`)).toBe(false); + expect(guard.isProtectedFilePath(`${home}/clawkeep/readme.md`)).toBe(false); }); it.each([ diff --git a/src/tests/unit/log-safe.test.ts b/src/tests/unit/log-safe.test.ts new file mode 100644 index 000000000..c2ab7a4ad --- /dev/null +++ b/src/tests/unit/log-safe.test.ts @@ -0,0 +1,63 @@ +import { describe, expect, it } from "vitest"; +import { logSafe, LOG_FIELD_MAX_LENGTH } from "@/lib/log-safe"; + +// Control characters are built by code point so this file stays plain ASCII and +// the expectations are readable. +const NUL = String.fromCharCode(0x00); +const LF = String.fromCharCode(0x0a); +const CR = String.fromCharCode(0x0d); +const ESC = String.fromCharCode(0x1b); +const DEL = String.fromCharCode(0x7f); +const C1 = String.fromCharCode(0x9b); +const REPLACEMENT = String.fromCharCode(0xfffd); + +describe("logSafe", () => { + it("leaves an ordinary value alone", () => { + expect(logSafe("TestNet-Home")).toBe("TestNet-Home"); + expect(logSafe("")).toBe(""); + }); + + it.each([ + ["NUL", NUL], + ["LF", LF], + ["CR", CR], + ["ESC", ESC], + ["DEL", DEL], + ["C1", C1], + ])("replaces %s so the value stays one line of text", (_label, ch) => { + expect(logSafe(`a${ch}b`)).toBe(`a${REPLACEMENT}b`); + }); + + it("keeps a value on a single line", () => { + expect(logSafe(`first${CR}${LF}second`)).toBe(`first${REPLACEMENT}${REPLACEMENT}second`); + }); + + it("replaces rather than strips, so distinct values stay distinct", () => { + expect(logSafe(`a${LF}b`)).not.toBe(logSafe("ab")); + }); + + it("keeps non-ASCII text that is not a control character", () => { + expect(logSafe("мрежа-Дом")).toBe("мрежа-Дом"); + }); + + it("caps a long value and says how much was dropped", () => { + const out = logSafe("x".repeat(500)); + expect(out.startsWith("x".repeat(LOG_FIELD_MAX_LENGTH))).toBe(true); + expect(out).toContain(`[+${500 - LOG_FIELD_MAX_LENGTH} chars]`); + expect(out.length).toBeLessThan(LOG_FIELD_MAX_LENGTH + 40); + }); + + it("does not cap a value at exactly the limit", () => { + const exact = "y".repeat(LOG_FIELD_MAX_LENGTH); + expect(logSafe(exact)).toBe(exact); + }); + + it("honours an explicit cap", () => { + expect(logSafe("abcdef", 3)).toBe("abc...[+3 chars]"); + }); + + it("bounds the output whatever the input size", () => { + const out = logSafe(`${LF.repeat(10_000)}tail`); + expect(out.length).toBeLessThan(LOG_FIELD_MAX_LENGTH + 40); + }); +}); diff --git a/src/tests/unit/mcp-path-guard.test.ts b/src/tests/unit/mcp-path-guard.test.ts index 818b1bb96..73c2deb9e 100644 --- a/src/tests/unit/mcp-path-guard.test.ts +++ b/src/tests/unit/mcp-path-guard.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect } from "vitest"; -import { isAllowedPath, filterAllowedPaths, assertPathAllowed } from "../../../mcp/lib/guard"; +import path from "path"; +import { isAllowedPath, filterAllowedPaths, assertPathAllowed, SECRET_NAME_RE } from "../../../mcp/lib/guard"; // The MCP's file tools (read_file, write_file, edit_file, list_directory, glob, // grep, notebook_edit) all funnel through isAllowedPath. These tests pin the @@ -12,11 +13,19 @@ import { isAllowedPath, filterAllowedPaths, assertPathAllowed } from "../../../m // clawbox-setup.service loads last, so it is both a read and a write // concern. // -// Paths are written as literal POSIX strings rather than built with path.join, -// so the expectations are identical on a developer's machine and on the device. +// Credential-store paths are written as literal POSIX strings rather than built +// with path.join, so the expectations are identical on a developer's machine and +// on the device — those rules are regexes over the string. The data-dir paths +// further down are the exception and say why. const HERMES = "/home/clawbox/.hermes"; const OPENCLAW = "/home/clawbox/.openclaw"; +const CLAWKEEP = "/home/clawbox/.clawkeep"; +// Built the way config-store builds DATA_DIR (path.join off the default root, +// which is what applies when CLAWBOX_ROOT is unset) so these expectations hold +// on a developer's machine as well as on the device. The credential-store paths +// above stay literal — those are matched by regex, not by path arithmetic. +const DATA = path.join("/home/clawbox/clawbox", "data"); describe("mcp path guard — credential directories", () => { it("blocks the ~/.hermes directory itself", () => { @@ -49,9 +58,19 @@ describe("mcp path guard — credential directories", () => { expect(isAllowedPath(p)).toBe(false); }); + it.each([ + CLAWKEEP, + `${CLAWKEEP}/token`, + `${CLAWKEEP}/passphrase`, + `${CLAWKEEP}/config.toml`, + ])("blocks the backup tool's store %s", (p) => { + expect(isAllowedPath(p)).toBe(false); + }); + it("does not block a directory that merely starts with the same letters", () => { expect(isAllowedPath("/home/clawbox/.hermesx/notes.md")).toBe(true); expect(isAllowedPath("/home/clawbox/hermes/notes.md")).toBe(true); + expect(isAllowedPath("/home/clawbox/.clawkeep-notes.txt")).toBe(true); }); it("blocks a path that reaches a credential directory through a parent segment", () => { @@ -78,6 +97,52 @@ describe("mcp path guard — dotenv files", () => { }); }); +// isAllowedPath delegates the data-dir rule to file-guard, whose own suite owns +// the full inventory. These cases pin that the delegation is in place and that +// the tools inherit both halves of the rule — not the inventory again. +describe("mcp path guard — the ClawBox data directory", () => { + it.each([ + path.join(DATA, "config.json"), + path.join(DATA, ".session-secret"), + path.join(DATA, "cloudflared", "cert.pem"), + // Named at runtime by an atomic write, and a name that does not exist yet: + // the rule is containment, so neither needs to be listed anywhere. + path.join(DATA, "oauth-device-tokens.json.tmp.deadbeef"), + path.join(DATA, "some-future-store.json"), + ])("blocks the server-state file %s", (p) => { + expect(isAllowedPath(p)).toBe(false); + }); + + it("leaves the data directory itself reachable", () => { + expect(isAllowedPath(DATA)).toBe(true); + }); + + it.each([ + path.join(DATA, "webapps", "demo", "index.html"), + path.join(DATA, "code-projects", "my-app", "app.js"), + ])("keeps the public subtree entry %s usable", (p) => { + expect(isAllowedPath(p)).toBe(true); + }); +}); + +describe("mcp path guard — credential names in a shell string", () => { + it.each([ + "cat ~/.clawkeep/token", + "tar czf /tmp/x.tgz $HOME/.clawkeep", + "cat ~/.ssh/id_rsa", + ])("recognises %s", (cmd) => { + expect(SECRET_NAME_RE.test(cmd)).toBe(true); + }); + + it.each([ + "echo clawkeep is a backup tool", + "cat ~/.clawkeep-notes.txt", + "ls ~/clawkeep", + ])("does not fire on %s", (cmd) => { + expect(SECRET_NAME_RE.test(cmd)).toBe(false); + }); +}); + describe("mcp path guard — device and kernel paths", () => { it.each([ "/proc/self/environ", @@ -94,7 +159,6 @@ describe("mcp path guard — ordinary project paths stay usable", () => { it.each([ "/home/clawbox/clawbox/package.json", "/home/clawbox/clawbox/src/app/page.tsx", - "/home/clawbox/clawbox/data/webapps/demo/index.html", "/home/clawbox/Documents/notes.md", "/var/log/syslog", "/tmp/scratch.txt",