diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 86df5b6b4..f43e23846 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -411,7 +411,7 @@ tool call - **Path Escape** (`path-escape-plugin.ts`) — Canonicalizes path-like arguments against `cwd` and blocks `..` escapes, except into a root the permission layer's worktree-roots provider allowlists (e.g. a sibling git worktree of the same repo). `tool-output://` and `archive:///` refs pass through unresolved. Runs first so later plugins see resolved paths. - **Evidence archive** (`evidence-archive-search-plugin.ts`, `evidence-archive-path-guard.ts`) — Primary-session compaction evidence is a first-class search/read surface on `search_files` / `read_file` / `grep` via `archive:///` refs. Dump paths (`evidence-archive/`, `tool-output/archive-*`) stay blocked so the on-disk sidecar is not the retrieval API. Blob keys reject `/` so they cannot nest under `tool-output`. - **Tool-output URI** (`tool-output-uri-plugin.ts`) — Normalizes mistaken `read_file` blob URIs to `tool-output:///id` (corbits-only; interchange stays unpatched). -- **Secret Guard** (`secret-guard-plugin.ts`) — Hard-denies path-keyed tool calls (`read_file`, `write_file`, …) that would put a sensitive file into (or write it from) the model context. Runs before the permission plugin, so the path-arg deny holds even under `--dangerously-skip-permissions`. An operator-chosen `--config` path cannot be covered by static patterns, so entry points pass the resolved active settings path as `extraDeniedPaths` (exact match, lexical plus realpath). Shell commands that _reference_ a sensitive path (tokenized so `cat .env`, `bun --env-file=.env run …`, and quote/env-assignment forms are detected) are not hard-denied here. Normal mode requires operator approval via the permission gate, and auto mode forces the same ask through the auto-shell policy (`sensitive-path` rule). Yolo/skip-permissions bypasses that prompt after catastrophic authorization checks, but path-keyed access to statically protected secret paths remains hard-denied. Shell detection is best-effort: token matching defeats quoting and env-assignment/redirection forms but not dynamic path construction (variable indirection, `printf` assembly). Tool-result secret scrub still redacts credential-shaped output. +- **Secret Guard** (`secret-guard-plugin.ts`) — Hard-denies path-keyed tool calls (`read_file`, `write_file`, …) that would put a sensitive file into (or write it from) the model context. Runs before the permission plugin, so the path-arg deny holds even under `--dangerously-skip-permissions`. An operator-chosen `--config` path cannot be covered by static patterns, so entry points pass the resolved active settings path as `extraDeniedPaths` (exact match, lexical plus realpath). Shell commands that _reference_ a sensitive path (tokenized so `cat .env`, `bun --env-file=.env run …`, and quote/env-assignment forms are detected) are not hard-denied here. Normal mode requires operator approval via the permission gate, and auto mode forces the same ask through the auto-shell policy (`sensitive-path` rule). Yolo/skip-permissions bypasses that prompt after catastrophic authorization checks, but path-keyed access to statically protected secret paths remains hard-denied. Shell detection is best-effort: token matching defeats quoting and env-assignment/redirection forms but not dynamic path construction (bare-variable indirection, `printf` assembly). A leading `$HOME`/`${HOME}` or `$USER`/`${USER}` expands from the environment before matching, and any other path-shaped `$-token` fails closed to ask. Tool-result secret scrub still redacts credential-shaped output. - **Authorization** (`run-shell-authz.ts`, enforced by the permission gate) — Denies catastrophic shell command patterns by regex, and hard-blocks shell `find`, head-position `rg`, and recursive `grep -r` (they can walk huge trees and OOM the host). Bounded `grep`/`search_files` tools remain practical alternatives (timeout + output caps); the patterns match those three command shapes only — an `ls -R`, `fd`, or scripted `os.walk` is just as unbounded and is not caught, so the block message tells the model not to substitute one. The gate hard-denies these at the top of its verdict path — before auto-allow, prompting, grants, and skipPermissions — so no mode or stored grant can admit them. - **Permission** (`permission-plugin.ts`) — Delegates consequential calls to the permission gate. - **Shell Guard** (`shell-guard-plugin.ts`) — Corbits Code-only replacement for stock `run_shell` (interchange stays unpatched): 120s foreground default (`settings.shell.timeoutMs` overrides that default only; a positive per-call `timeout` is the bound with no ceiling — `maxTimeoutMs` does not clamp it), background `run_shell` has no default (only a per-call timeout arms a timer), 512KB display cap with head+tail retention (the process keeps running when the cap is hit), process-group kill on timeout, abort, and plugin dispose (live children tracked in the plugin and reaped by `posixTools.dispose`), and `background: true` — the call returns a `shell_id` at once (registry in `src/shell/background-shell.ts`), the process group keeps running past the turn, completion is process-exit (stdio-close is not required), delivered on a later turn via `buildShellBackgroundMessage`, and `shell_collect` retrieves or cancels with a 300s wait cap (schema advertised by `advertiseShellGuardTimeout` only when `shell_collect` is mounted; evaluated by the permission chain at start time like any shell call). Foreground expiry is exit 124 + `timedOut:true` plus a nudge to retry with `background:true`. Also applies a 10s wall-clock budget to `grep`/`search_files` (budget expiry and budget-torn-down aborts surface as explicit timeout errors, never as empty results; an unbounded workspace-root `search_files` walk is refused up front with scope guidance instead of running to the budget). Ripgrep detached spawns are not tracked. diff --git a/src/permission/classify-security.test.ts b/src/permission/classify-security.test.ts index 4b48021d4..4a0730dd2 100644 --- a/src/permission/classify-security.test.ts +++ b/src/permission/classify-security.test.ts @@ -43,6 +43,9 @@ describe("isAutoAllowedShellCall — sensitive-path arguments", () => { ["cat .ssh/known_hosts", false], ["cat .netrc", false], ["cat .git-credentials", false], + ["cat $HOME/.ssh/id_rsa", false], + ["cat $HOME/.env", false], + ["cat ${HOME}/.aws/credentials", false], ["env FILE=.envrc sh -c 'cat \"$FILE\"'", false], ["sed -Enf.flaskenv input", false], ["sed --fil=.envrc input.txt", false], @@ -290,6 +293,28 @@ describe("sensitive-path shell commands require approval, not a hard deny", () = expect(rule?.effect).toBe("ask"); }); + test("auto mode forces ask for dollar-prefixed home secret paths", () => { + for (const command of [ + "cat $HOME/.ssh/id_rsa", + "cat $HOME/.env", + "cat ${HOME}/.aws/credentials", + ]) { + expect(autoShellRuleForCall(shellCall(command))).toMatchObject({ + name: "sensitive-path", + effect: "ask", + }); + expect(isAutoAllowedShellCall(shellCall(command))).toBe(false); + } + }); + + test("auto mode fails closed on path-shaped unknown-variable tokens", () => { + expect(autoShellRuleForCall(shellCall("cat $BASE/mylink"))).toMatchObject({ + name: "sensitive-path", + effect: "ask", + }); + expect(isAutoAllowedShellCall(shellCall("cat $BASE/mylink"))).toBe(false); + }); + test("auto mode asks for clustered bash secret reads", () => { for (const command of [ "bash -lc \"cat \\$'.envrc'\"", diff --git a/src/plugins/secret-guard-plugin.test.ts b/src/plugins/secret-guard-plugin.test.ts index 206d126cb..9bb283875 100644 --- a/src/plugins/secret-guard-plugin.test.ts +++ b/src/plugins/secret-guard-plugin.test.ts @@ -1,5 +1,5 @@ import { describe, test, expect } from "bun:test"; -import { mkdir, mkdtemp, rm } from "node:fs/promises"; +import { mkdir, mkdtemp, rm, symlink, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { createPosixTools } from "@intx/tools-posix"; @@ -10,6 +10,7 @@ import { loadProjectApprovals } from "../permission/store.js"; import { secretGuardPlugin, isSensitivePath, + isSensitiveShellToken, commandReferencesSensitivePath, } from "./secret-guard-plugin.js"; @@ -287,6 +288,14 @@ describe("commandReferencesSensitivePath", () => { "cat release.keystore", "cat server.ppk", "cat service-account.json", + // Dollar-prefixed home tokens resolve before matching (CL-8999): the + // shell expands these to the operator's home at runtime, so they must + // ask exactly like their literal forms. + "cat $HOME/.ssh/id_rsa", + "cat $HOME/.env", + "cat ${HOME}/.aws/credentials", + // Literal tilde keeps asking (unchanged behavior, pinned here). + "cat ~/.ssh/id_rsa", ]; test("flags every command touching a credential path", () => { expect( @@ -308,6 +317,9 @@ describe("commandReferencesSensitivePath", () => { "sed --f=.envrc input.txt", "grep --fil=.envrc needle", "bun test", + // A bare variable with no path shape never names a file (CL-8999). + "echo $HOME", + "cat $HOMEPATH", ]; test("allows every benign command", () => { expect( @@ -316,6 +328,40 @@ describe("commandReferencesSensitivePath", () => { }); }); +describe("dollar-prefixed secret paths", () => { + test("$HOME-prefixed symlink into a secret file asks like the literal form", async () => { + const dir = await mkdtemp(join(tmpdir(), "cl8999-dollar-")); + try { + await writeFile(join(dir, ".env"), "SECRET=x\n"); + await symlink(join(dir, ".env"), join(dir, "mylink")); + const savedHome = process.env.HOME; + process.env.HOME = dir; + try { + expect(isSensitiveShellToken("mylink", dir)).toBe(true); + expect(isSensitiveShellToken("$HOME/mylink", dir)).toBe(true); + expect(isSensitiveShellToken("${HOME}/mylink", dir)).toBe(true); + expect( + commandReferencesSensitivePath("cat $HOME/mylink", dir), + ).not.toBeUndefined(); + } finally { + if (savedHome === undefined) delete process.env.HOME; + else process.env.HOME = savedHome; + } + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); + + test("path-shaped tokens under unknown variables fail closed to ask", () => { + expect(isSensitiveShellToken("$BASE/mylink")).toBe(true); + expect( + commandReferencesSensitivePath("cat $BASE/mylink"), + ).not.toBeUndefined(); + expect(isSensitiveShellToken("$HOME")).toBe(false); + expect(commandReferencesSensitivePath("echo $HOME")).toBeUndefined(); + }); +}); + describe("secretGuardPlugin run_shell", () => { // Shell commands that mention a secret path are no longer hard-denied here — // they require operator approval at the permission gate. The plugin only diff --git a/src/plugins/secret-guard-plugin.ts b/src/plugins/secret-guard-plugin.ts index 5ce24be87..c9b8385ea 100644 --- a/src/plugins/secret-guard-plugin.ts +++ b/src/plugins/secret-guard-plugin.ts @@ -211,6 +211,12 @@ export function createExtraDeniedPathMatcher( // symlink the matcher only ever saw as `*`. Perfect shell sandboxing is out // of scope; the goal is to force a prompt for the trivial, single-token // references that make exfiltration easy. Tool-result secret scrub still redacts credential-shaped output. +// A leading $HOME/${HOME} or $USER/${USER} expands from the environment +// before matching, so `$HOME/.ssh/id_rsa` meets the same legs as the literal +// path; any other path-shaped $-token (`$BASE/mylink`) fails closed to ask +// because the matcher cannot know what it expands to. A bare `$VAR` with no +// path shape stays invisible — when the value is a secret path the matcher +// never sees one token that names it. // Programs that only print directory names / metadata — listing a name never // dumps file contents. Single owner for this set: the resolve-leg skip below // and classify.ts's pure-listing exemption both read it, so a new names-only @@ -225,6 +231,30 @@ export const PURE_DIRECTORY_LISTING_PROGRAMS = new Set(["ls", "tree"]); // pattern, so `cat *.txt` cannot resolve without running the shell — but a // glob CAN expand into a symlink at runtime, which stays a stated residual // (see the threat model below), not something this filter disproves. + +// CL-8999: the shell expands a leading $HOME (or ${HOME}) before opening the +// path, so expand it here (string-only, from the environment — never shell +// eval) and let the normal legs judge the result. $USER (or ${USER}) gets the +// same treatment. Only these two prefixes expand: arbitrary variables stay +// unexpanded and fail closed to ask as path-shaped tokens below. +function expandLeadingDollarToken(token: string): string { + const home = process.env.HOME; + if (home !== undefined && home.length > 0) { + if (token === "$HOME" || token === "${HOME}") return home; + if (token.startsWith("$HOME/")) return home + token.slice("$HOME".length); + if (token.startsWith("${HOME}/")) + return home + token.slice("${HOME}".length); + } + const user = process.env.USER ?? process.env.USERNAME; + if (user !== undefined && user.length > 0) { + if (token === "$USER" || token === "${USER}") return user; + if (token.startsWith("$USER/")) return user + token.slice("$USER".length); + if (token.startsWith("${USER}/")) + return user + token.slice("${USER}".length); + } + return token; +} + function isPathLikeShellToken(token: string): boolean { if ( token.startsWith("-") || @@ -303,9 +333,25 @@ export function isSensitiveShellToken( isExtraDenied: (value: string) => boolean = () => false, dialect: ShellDialect = nativeShellDialect(process.platform), ): boolean { - const expanded = expandHome(token); + const expanded = expandHome(expandLeadingDollarToken(token)); if (isSensitivePath(expanded, dialect)) return true; if (isExtraDenied(expanded)) return true; + // CL-8999: a token the shell could still expand at runtime into a path + // ($BASE/mylink, or $HOME/mylink with HOME unset) never reaches either leg + // below — `$` tokens skip both the path-like and bare filters — so fail + // closed to ask when the `$` starts a live variable expansion ($VAR or + // ${VAR}) joined to a path separator. Quoted dollars (`'$'.envrc`), command + // substitutions (`$(...)`), ANSI-C quotes (`$'...'`), and special parameters + // ($?, $$, $1) never match, and neither do ADS streams (`.env.sample::$DATA`, + // no separator) or bare `$VAR` tokens with no path shape: a bare variable + // can never name a file the shell opens without expanding into something + // the matcher sees. + if ( + expanded.includes("$") && + /\$[{]?[A-Za-z_]/.test(expanded) && + (expanded.includes("/") || expanded.includes("\\")) + ) + return true; if (!resolveSymlinks) { if (!isBareProbeCandidate(expanded)) return false; return isExtraDenied(