From 86fd7054e1eddcf64b7461ba04d273b57eba4807 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 23 Sep 2026 20:48:25 +0000 Subject: [PATCH 1/7] fix(guards): match protect-paths secret paths per path token The Bash rule matched `\.env(\.[\w-]+)?\b` anywhere in a command, so read-only commands that merely mentioned an env accessor were blocked: `grep -rn process.env src`, `rg 'import\.meta\.env'`, `git log --grep='.env handling'`, `cat messages.key.ts`, plus Reads of `.env.example`, a plain project `.npmrc` and a Next.js `app/docs/secrets/page.tsx` route. Real reads slipped through instead: `sed -n p .env`, `awk 1 .env`, `tac .env`, `... < .env`, `cat .e*v`. The command is now split into shell words (quotes, `$'..'`, `$(..)`, backticks, heredocs and comments honoured) and only each simple command's FILE operands are tested against one secretKind() predicate shared with tool paths. A grep/rg/sed/awk/jq pattern, a commit message or a `--grep=` value is never a path. - `.env.example`/`.sample`/`.template`/`.dist` are templates; `*.key.ts` is code; source files under a `secrets/` directory are code. - A project `.npmrc` is protected only when it holds a literal `_authToken`/`_auth`/`_password` (an `${NPM_TOKEN}` reference is not a secret); the user-level `~/.npmrc` always is. - New readers: sed awk tac sort uniq cut paste bat jq diff cmp fold rev hexdump `dd if=`; input redirection from a secret; globs that select a secret name. - `rm -rf` of `.`, `..`, `./` or `*`; `git checkout .` and `git restore .` (not `--staged` only); `| sudo sh`, `| python3`. - The protect-paths matcher covers Read|Grep|Glob|NotebookRead in all three hook manifests, and the Grep tool's `glob` filter is checked. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01UUhB8JaPayd43w37dxiXrW Signed-off-by: Claude --- .claude/settings.json | 2 +- global/guards/protect-paths.mjs | 837 +++++++++++++++++++++++++++++--- global/settings.template.json | 2 +- hooks/hooks.json | 2 +- test/guards.test.js | 12 +- test/protect_paths.test.js | 436 +++++++++++++++++ test/settings_template.test.js | 19 + 7 files changed, 1234 insertions(+), 76 deletions(-) create mode 100644 test/protect_paths.test.js diff --git a/.claude/settings.json b/.claude/settings.json index 750341f7..311e770b 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -110,7 +110,7 @@ ] }, { - "matcher": "Read", + "matcher": "Read|Grep|Glob|NotebookRead", "hooks": [ { "type": "command", diff --git a/global/guards/protect-paths.mjs b/global/guards/protect-paths.mjs index 953ab1d2..8163d761 100644 --- a/global/guards/protect-paths.mjs +++ b/global/guards/protect-paths.mjs @@ -10,75 +10,739 @@ // 3. FAIL CLOSED. Exit 1 is a non-blocking hook error in Claude Code; `set -e` turned every // internal hiccup into a silent pass. Here any error denies with a reason (exit 2). // -// SCOPE: pattern matching, not a sandbox. Regex cannot parse shell, so interpreter-driven -// access (`python -c 'open(".env")…'`, `node -e …`) is DELIBERATELY out of scope. This layer -// sits behind the permission system and secret-redact.sh: best-effort hardening, never a -// boundary. - -// ── File rules. Paths are normalized to forward slashes (Claude Code sends native Windows -// paths) and matched case-insensitively (NTFS is; a `.ENV` is still a secret on POSIX). -const FILE_RULES = [ - { re: /\.env($|\.)/i, what: "env file" }, - { - re: /\.pem($|\.)|(^|\/)id_rsa($|\.)|(^|\/)id_ed25519($|\.)|\.key($|\.)/i, - what: "credential/key file", +// Secret paths are matched per PATH TOKEN, never as a substring of the command: the command is +// split into shell words (quotes honoured), each simple command's FILE operands are found, and +// only those are tested. So `grep -rn process.env src`, `rg 'import\.meta\.env'`, `.env.example` +// and `messages.key.ts` pass, while `.env`, `.env.local`, `id_rsa`, `*.pem` and `*.key` stay +// blocked. +// +// SCOPE: pattern matching, not a sandbox. A word splitter cannot evaluate shell, so indirection +// (`f=.env; cat $f`, brace expansion, `find … -exec cat {}`) and interpreter-driven access +// (`python -c 'open(".env")…'`, `node -e …`) are DELIBERATELY out of scope. This layer sits +// behind the permission system and secret-redact.sh: best-effort hardening, never a boundary. +import { closeSync, openSync, readSync, statSync } from "node:fs"; +import { homedir } from "node:os"; +import { resolve } from "node:path"; + +/** + * @typedef {{cwd?: string, home?: string, readText?: (absPath: string) => string | null}} FsCtx + * @typedef {FsCtx & {bash?: boolean}} PathCtx + * @typedef {{text: string, glob: boolean, quoted: boolean}} Word + * @typedef {{op: string, target: Word}} Redirect + * @typedef {{words: Word[], redirs: Redirect[]}} Segment + */ + +// ── What is a secret path. ONE predicate for a tool's path (Read/Edit/Grep/…) and for every +// file operand of a Bash command. Paths are compared with forward slashes (Claude Code sends +// native Windows paths) and case-insensitively (NTFS is; a `.ENV` is still a secret on POSIX). + +/** Suffixes that mark a committed TEMPLATE of an env file (`.env.example`), never the real one. */ +const ENV_TEMPLATE = new Set(["example", "sample", "template", "dist", "defaults"]); +/** Source and docs a `secrets/` directory legitimately holds in an app — a Next.js + * `app/docs/secrets/page.tsx` route, a Django `secrets` app. Code, not a secret store. */ +const CODE_FILE = + /\.(?:[cm]?[jt]sx?|vue|svelte|astro|mdx?|css|scss|sass|less|html?|py|rb|go|rs|java|kts?|swift|php|cs|c|cc|cpp|h|hpp|dart|exs?|scala|lua)$/i; +const SECRET_DIR = "path under secrets/ or .ssh/"; + +/** Forward slashes, no trailing slash (a trailing slash names the directory itself). + * @param {string} s */ +const slashes = (s) => + String(s) + .replaceAll("\\", "/") + .replace(/(.)\/+$/, "$1"); + +/** + * The kind of protected secret `raw` names, or null. In a Bash word (`ctx.bash`) a backslash is + * usually regex text (`process\.env`), so only a drive/UNC-shaped word is re-slashed, and a bare + * `name.env` is NOT an env file there: it is indistinguishable from `process.env`. + * @param {string} raw + * @param {PathCtx} [ctx] + * @returns {string | null} + */ +export function secretKind(raw, ctx = {}) { + const text = String(raw ?? ""); + const p = + !ctx.bash || /^[A-Za-z]:\\|^\\\\/.test(text) ? slashes(text) : text.replace(/(.)\/+$/, "$1"); + if (!p) return null; + const segs = p.split("/"); + const base = segs[segs.length - 1]; + const lower = segs.map((s) => s.toLowerCase()); + const env = /^\.env((?:\.[\w-]+)*)~?$/i.exec(base); + if (env) { + if (!ENV_TEMPLATE.has(env[1].split(".").pop()?.toLowerCase() ?? "")) return "env file"; + } else if (/.\.env$/i.test(base) && (!ctx.bash || segs.length > 1)) { + return "env file"; // `prod.env`, `docker/app.env` + } + if ( + /^id_(?:rsa|dsa|ecdsa|ed25519)(?:_sk)?$/i.test(base) || + /\.(?:pem|key)(?:\.(?:bak|old|orig|backup|save|tmp))?~?$/i.test(base) + ) + return "credential/key file"; + if (lower.includes(".ssh")) return SECRET_DIR; + if (lower.slice(0, -1).includes("secrets") && !CODE_FILE.test(base)) return SECRET_DIR; + if ( + /^(?:\.netrc|_netrc|\.git-credentials)$/i.test(base) || + (lower.at(-2) === ".aws" && lower.at(-1) === "credentials") + ) + return "credential store"; + if (/^\.npmrc$/i.test(base) && npmrcIsSecret(p, ctx)) return "credential store"; + return null; +} + +/** + * True iff an npmrc's text carries a literal registry credential. `${NPM_TOKEN}` — the usual CI + * spelling — references the environment and holds no secret, so it does not count. + * @param {string} text + */ +export function npmrcHasToken(text) { + const auth = /^[ \t]*(?:[^\s=#;]*:)?_(?:authToken|auth|password)[ \t]*=[ \t]*(.*)$/gim; + for (const m of String(text).matchAll(auth)) { + const v = m[1].trim().replace(/^(["'])(.*)\1$/, "$2"); + if (v && !/^\$\{[A-Za-z_]\w*\}$/.test(v)) return true; + } + return false; +} + +/** + * A project `.npmrc` is ordinary config (`registry=`, `engine-strict=`) and reading it is + * routine; only one holding a literal token is a credential store. The user-level npmrc is where + * `npm login` writes the token, so it is always protected, and so is any spelling this guard + * cannot resolve (`$DIR/.npmrc`): fail closed. + * @param {string} p forward-slashed path + * @param {FsCtx} ctx + */ +function npmrcIsSecret(p, ctx) { + if (/^~[^/]*\//.test(p) || /^\$\{?HOME\}?\//.test(p)) return true; + if (/[$`]/.test(p)) return true; + const home = slashes(ctx.home ?? homedir()).toLowerCase(); + const abs = slashes(/^(?:[A-Za-z]:)?\//.test(p) ? p : resolve(ctx.cwd ?? process.cwd(), p)); + if (abs.slice(0, abs.lastIndexOf("/")).toLowerCase() === home) return true; + return npmrcHasToken((ctx.readText ?? readHead)(abs) ?? ""); +} + +/** + * The first MiB of a regular file, or null. Never opens a FIFO or a device: a blocking read + * would hang the hook. + * @param {string} p + */ +function readHead(p) { + let fd; + try { + if (!statSync(p).isFile()) return null; + fd = openSync(p, "r"); + const buf = Buffer.alloc(1 << 20); + return buf.toString("utf8", 0, readSync(fd, buf, 0, buf.length, 0)); + } catch { + return null; + } finally { + if (fd !== undefined) closeSync(fd); + } +} + +// ── Globs. `cat .e*v` reads `.env` without ever spelling it, so a wildcard operand is tested +// against representative secret names. The shell never expands `*` to a dotfile, so a `.`-led +// name is only reachable by a glob that itself starts with `.`; a glob with no literal +// characters (`*`, `*.*`) is not a targeted read, so `grep -r x *` stays allowed. +const GLOB_SAMPLES = [ + ".env", + ".env.local", + ".env.production", + "prod.env", + "id_rsa", + "id_ed25519", + "server.pem", + "server.key", + ".netrc", + "_netrc", + ".git-credentials", +]; + +/** @param {string} glob one path segment */ +function globRegExp(glob) { + let re = ""; + for (let i = 0; i < glob.length; i++) { + const c = glob[i]; + const close = c === "[" ? glob.indexOf("]", i + 2) : -1; + if (c === "*") re += "[^/]*"; + else if (c === "?") re += "[^/]"; + else if (close > 0) { + const body = glob.slice(i + 1, close).replace(/^[!^]/, "^"); + re += `[${body.replaceAll("\\", "\\\\")}]`; + i = close; + } else re += c.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + } + return new RegExp(`^${re}$`, "i"); +} + +/** + * True iff the wildcard pattern `glob` (a shell glob, an `rg -g` glob, the Grep tool's `glob`) + * can select a protected secret file by name. + * @param {string} glob + */ +export function globMatchesSecret(glob) { + const g = slashes(glob); + if (g.startsWith("!")) return false; // an exclusion (`rg -g '!.env'`) never selects a file + const base = g.split("/").pop() ?? ""; + if (!/[*?[]/.test(base)) return false; // no wildcard in the name: the literal check covers it + let re; + try { + re = globRegExp(base); + } catch { + return true; // an unparsable class: fail closed + } + const dotted = base.startsWith("."); + const literal = base.replace(/\[[^\]]*\]|[*?.]/g, ""); + return GLOB_SAMPLES.some((s) => (s.startsWith(".") ? dotted : literal.length > 0) && re.test(s)); +} + +// ── Shell words. Enough of POSIX sh to find each simple command, its operands and its +// redirections — NOT a parser. Quotes are honoured, so a quoted `;`, `>` or space never splits a +// word, and a quoted `*` is not a glob. + +/** + * Decode a `$'…'` (ANSI-C) string that starts at `cmd[i] === "$"`: `$'\x2eenv'` is `.env`. + * @param {string} cmd + * @param {number} i + */ +function ansiC(cmd, i) { + /** @type {Record} */ + const simple = { n: "\n", t: "\t", r: "\r", a: "\x07", b: "\b", e: "\x1b", f: "\f", v: "\v" }; + let s = ""; + let j = i + 2; + for (; j < cmd.length && cmd[j] !== "'"; j++) { + if (cmd[j] !== "\\" || j + 1 >= cmd.length) { + s += cmd[j]; + continue; + } + const e = cmd[++j]; + const hex = /^[0-9a-fA-F]+/.exec(cmd.slice(j + 1, j + (e === "x" ? 3 : 5))); + const oct = /^[0-7]{1,3}/.exec(cmd.slice(j, j + 3)); + if ((e === "x" || e === "u") && hex) { + s += String.fromCharCode(Number.parseInt(hex[0], 16)); + j += hex[0].length; + } else if (oct) { + s += String.fromCharCode(Number.parseInt(oct[0], 8)); + j += oct[0].length - 1; + } else s += simple[e] ?? e; + } + return { text: s, end: j }; +} + +/** Shells that execute a heredoc body as code — theirs is scanned instead of skipped. */ +const SHELLS = new Set(["sh", "bash", "zsh", "dash", "ksh", "mksh", "fish"]); + +/** `/usr/bin/CAT.exe` → `cat`. @param {string} t */ +const cmdName = (t) => + (t.replaceAll("\\", "/").split("/").pop() ?? "").toLowerCase().replace(/\.exe$/, ""); + +/** + * Split a Bash command into simple-command segments. `;` `&` `|` newlines and `( )` end a + * segment; `$( … )`, backticks and `<( … )` are scanned as nested segments; a heredoc body is + * skipped as data unless it is fed to a shell. + * @param {string} cmd + * @returns {Segment[]} + */ +export function shellSegments(cmd) { + /** @type {Segment[]} */ + const done = []; + /** @returns {{words: Word[], redirs: Redirect[], cur: Word | null, redir: string | null}} */ + const fresh = () => ({ words: [], redirs: [], cur: null, redir: null }); + let ctx = fresh(); + /** @type {{ctx: ReturnType, kind: string, dq: boolean}[]} */ + const stack = []; + let dq = false; // inside "…" + /** @type {{delim: string, strip: boolean, exec: boolean}[]} */ + const heredocs = []; + /** @param {string} s @param {boolean} [glob] @param {boolean} [quoted] */ + const add = (s, glob = false, quoted = false) => { + ctx.cur ??= { text: "", glob: false, quoted: false }; + ctx.cur.text += s; + if (glob) ctx.cur.glob = true; + if (quoted) ctx.cur.quoted = true; + }; + const endWord = () => { + const w = ctx.cur; + if (!w) return; + ctx.cur = null; + const op = ctx.redir; + if (!op) { + ctx.words.push(w); + return; + } + ctx.redir = null; + if (op === "<<" || op === "<<-") { + const first = ctx.words.find((x) => !/^[A-Za-z_]\w*=/.test(x.text)); + heredocs.push({ + delim: w.text, + strip: op === "<<-", + exec: SHELLS.has(cmdName(first?.text ?? "")), + }); + } else ctx.redirs.push({ op, target: w }); + }; + const endSeg = () => { + endWord(); + if (ctx.words.length || ctx.redirs.length) done.push({ words: ctx.words, redirs: ctx.redirs }); + ctx.words = []; + ctx.redirs = []; + ctx.redir = null; + }; + /** @param {string} kind */ + const open = (kind) => { + stack.push({ ctx, kind, dq }); + ctx = fresh(); + dq = false; + }; + const close = () => { + endSeg(); + const f = /** @type {(typeof stack)[number]} */ (stack.pop()); + ctx = f.ctx; + dq = f.dq; + if (f.kind !== "(") add("$()"); // an expansion inside a word: no longer a literal path + }; + /** @param {number} i index of the newline ending the line that owns the heredocs */ + const skipHeredocs = (i) => { + let pos = i + 1; + for (const h of heredocs) { + while (pos < cmd.length) { + let e = cmd.indexOf("\n", pos); + if (e < 0) e = cmd.length; + const line = cmd.slice(pos, e); + pos = e + 1; + if ((h.strip ? line.replace(/^\t+/, "") : line) === h.delim) break; + } + } + heredocs.length = 0; + return pos - 1; + }; + for (let i = 0; i < cmd.length; i++) { + const c = cmd[i]; + const next = cmd[i + 1]; + if (dq) { + if (c === '"') dq = false; + else if (c === "\\" && next !== undefined && '"\\$`\n'.includes(next)) { + if (next !== "\n") add(next, false, true); + i++; + } else if (c === "$" && next === "(") { + open("$("); + i++; + } else if (c === "`") open("`"); + else add(c, false, true); + continue; + } + if (c === "\\") { + if (next !== undefined && next !== "\n") add(next, false, true); + i++; + } else if (c === "'") { + const j = cmd.indexOf("'", i + 1); + const end = j < 0 ? cmd.length : j; + add(cmd.slice(i + 1, end), false, true); + i = end; + } else if (c === '"') { + dq = true; + add("", false, true); + } else if (c === "$" && next === "'") { + const { text, end } = ansiC(cmd, i); + add(text, false, true); + i = end; + } else if (c === "$" && next === "(") { + open("$("); + i++; + } else if (c === "`") { + if (stack.at(-1)?.kind === "`") close(); + else open("`"); + } else if (c === "(") open("("); + else if (c === ")") { + if (stack.length && stack.at(-1)?.kind !== "`") close(); + else endSeg(); + } else if (c === "\n") { + endSeg(); + if (heredocs.length) { + if (heredocs.some((h) => h.exec)) heredocs.length = 0; + else i = skipHeredocs(i); + } + } else if (c === " " || c === "\t" || c === "\r") endWord(); + else if (c === "#" && !ctx.cur) { + const e = cmd.indexOf("\n", i); + i = (e < 0 ? cmd.length : e) - 1; // a comment runs to the end of the line + } else if (c === ";" || c === "|") endSeg(); + else if (c === "&") { + if (next === ">") { + endWord(); + i += cmd[i + 2] === ">" ? 2 : 1; + ctx.redir = ">"; + } else endSeg(); + } else if (c === "<" || c === ">") { + // `2>file`: a bare number right before the operator is the fd, not an operand. + if (ctx.cur && !ctx.cur.quoted && /^\d+$/.test(ctx.cur.text)) ctx.cur = null; + else endWord(); + if (next === "(") { + open(`${c}(`); // process substitution: a nested command used as a file argument + i++; + continue; + } + let op = c; + if (c === "<" && cmd.startsWith("<<<", i)) { + op = "<<<"; + i += 2; + } else if (c === "<" && next === "<") { + op = "<<"; + i++; + if (cmd[i + 1] === "-") { + op = "<<-"; + i++; + } + } else if (next === ">") { + op = c === "<" ? "<>" : ">>"; + i++; + } else if (c === ">" && next === "|") i++; + else if (next === "&") { + op = `${c}&`; + i++; + } + ctx.redir = op; + } else add(c, c === "*" || c === "?" || c === "["); + } + while (stack.length) close(); + endSeg(); + return done; +} + +// ── Commands: which word runs, and which of its arguments are FILES it reads or writes. + +/** Shell keywords that may precede the command word. */ +const KEYWORDS = new Set(["if", "then", "else", "elif", "do", "while", "until", "!", "{"]); +/** Wrappers that run the next word as the command → their options that take a value. + * @type {Record} */ +const WRAPPERS = { + sudo: ["-u", "-g", "-C", "-D", "-h", "-p", "-r", "-t", "-U", "-T", "--user", "--group"], + doas: ["-u", "-C"], + env: ["-u", "-C", "-S", "--unset", "--chdir", "--split-string"], + command: [], + builtin: [], + busybox: [], + exec: ["-a"], + nohup: [], + nice: ["-n", "--adjustment"], + ionice: ["-c", "-n", "-p"], + stdbuf: ["-i", "-o", "-e"], + time: ["-f", "-o", "--format", "--output"], + timeout: ["-s", "-k", "--signal", "--kill-after"], + xargs: ["-a", "-d", "-E", "-I", "-L", "-n", "-P", "-s", "--arg-file", "--delimiter"], +}; + +/** + * The command a segment runs, past `VAR=val` prefixes, keywords and wrappers. + * @param {Word[]} words + * @returns {{name: string, args: Word[]} | null} + */ +function commandOf(words) { + let i = 0; + while (i < words.length) { + const w = words[i]; + // `FOO="a b" cat .env`: an assignment even when its value is quoted. + if (/^[A-Za-z_]\w*(\[[^\]]*\])?\+?=/.test(w.text) || KEYWORDS.has(w.text)) { + i++; + continue; + } + const name = cmdName(w.text); + const values = WRAPPERS[name]; + if (!values) return { name, args: words.slice(i + 1) }; + let positional = name === "timeout" ? 1 : 0; // timeout DURATION cmd… + for (i++; i < words.length; ) { + const a = words[i].text; + if (a === "--") { + i++; + break; + } + if (a.length > 1 && a.startsWith("-")) i += values.includes(a) ? 2 : 1; + else if (positional-- > 0) i++; + else break; + } + } + return null; +} + +/** + * How a command's arguments map to files. `patternFirst`: its first operand is a pattern or a + * program (grep, sed, awk, jq), not a file. `opts` names the options that take a value: + * file — a path (checked); it also supplies the pattern/program (`grep -f`, `sed -f`) + * pattern — a pattern or program, not a path; it supplies the pattern (`grep -e`, `sed -e`) + * glob — a file glob, checked as one (`rg -g`, `grep --include`) + * skip — any other non-path value (`git log --grep`, `rg -t`, `head -n`) + * skip2 / file2 — two-word options (`jq --arg k v` / `jq --rawfile k FILE`) + * Any option not listed is a flag (or a glued `-Xvalue`). + * @typedef {{patternFirst?: boolean, opts?: Record}} ArgSpec + */ +/** @param {string} kind @param {string[]} names */ +const kinds = (kind, names) => Object.fromEntries(names.map((n) => [n, kind])); + +/** @type {ArgSpec} */ +const GREP = { + patternFirst: true, + opts: { + ...kinds("pattern", ["-e", "--regexp"]), + ...kinds("file", ["-f", "--file"]), + ...kinds("glob", ["--include"]), + ...kinds("skip", ["-A", "-B", "-C", "-m", "-d", "-D", "--exclude", "--exclude-dir"]), + ...kinds("skip", ["--context", "--after-context", "--before-context", "--max-count"]), + ...kinds("skip", ["--label", "--binary-files", "--devices", "--directories"]), }, - { re: /\/secrets\/|\/\.ssh\//i, what: "path under secrets/ or .ssh/" }, - { - re: /(^|\/)(\.aws\/credentials|\.netrc|_netrc|\.npmrc|\.git-credentials)$/i, - what: "credential store", +}; +/** @type {ArgSpec} */ +const RG = { + patternFirst: true, + opts: { + ...kinds("pattern", ["-e", "--regexp"]), + ...kinds("file", ["-f", "--file"]), + ...kinds("glob", ["-g", "--glob", "--iglob"]), + ...kinds("skip", ["-A", "-B", "-C", "-m", "-t", "-T", "-r", "-E", "-M", "-j", "-d"]), + ...kinds("skip", ["--type", "--type-not", "--type-add", "--type-clear", "--replace"]), + ...kinds("skip", ["--context", "--after-context", "--before-context", "--max-count"]), + ...kinds("skip", ["--encoding", "--engine", "--sort", "--sortr", "--color", "--colors"]), + ...kinds("skip", ["--max-columns", "--threads", "--max-depth", "--max-filesize", "--pre"]), + ...kinds("skip", ["--pre-glob", "--path-separator", "--context-separator"]), }, -]; +}; +/** @type {ArgSpec} */ +const AG = { + patternFirst: true, + opts: kinds("skip", ["-A", "-B", "-C", "-m", "-G", "-g", "--ignore", "--ignore-dir", "--depth"]), +}; +/** @type {ArgSpec} */ +const SED = { + patternFirst: true, + opts: { + ...kinds("pattern", ["-e", "--expression"]), + ...kinds("file", ["-f", "--file"]), + ...kinds("skip", ["-l", "--line-length"]), + }, +}; +/** @type {ArgSpec} */ +const AWK = { + patternFirst: true, + opts: { + ...kinds("pattern", ["-e", "--source"]), + ...kinds("file", ["-f", "--file"]), + ...kinds("skip", ["-v", "-F", "--assign", "--field-separator"]), + }, +}; +/** @type {ArgSpec} */ +const JQ = { + patternFirst: true, + opts: { + ...kinds("file", ["-f", "--from-file"]), + ...kinds("skip", ["--indent", "-L"]), + ...kinds("skip2", ["--arg", "--argjson"]), + ...kinds("file2", ["--slurpfile", "--rawfile"]), + }, +}; +/** @type {ArgSpec} */ +const PLAIN = { + opts: kinds("skip", ["-n", "-c", "--lines", "--bytes", "-k", "-t", "-S", "-T", "-w", "-d"]), +}; + +/** Commands that print file content → how to find their file operands. + * @type {Record} */ +const READERS = { + ...Object.fromEntries( + [ + ...["cat", "tac", "nl", "head", "tail", "less", "more", "most", "bat", "batcat"], + ...["xxd", "od", "hexdump", "strings", "base64", "sort", "uniq", "cut", "paste"], + ...["fold", "rev", "diff", "cmp", "dd"], + ].map((n) => [n, PLAIN]), + ), + ...Object.fromEntries(["grep", "egrep", "fgrep"].map((n) => [n, GREP])), + rg: RG, + ag: AG, + ack: AG, + sed: SED, + ...Object.fromEntries(["awk", "gawk", "mawk", "nawk"].map((n) => [n, AWK])), + jq: JQ, + yq: JQ, +}; +/** git subcommands that print file or history content (RA-05, HI-07). */ +const GIT_READERS = new Set([ + ...["show", "log", "diff", "stash", "cat-file", "archive", "grep", "blame", "annotate"], + ...["show-index", "bundle", "whatchanged"], +]); +/** @type {ArgSpec} */ +const GIT_LOG = { + opts: { + ...kinds("skip", ["--grep", "--author", "--committer", "--format", "--pretty", "-S", "-G"]), + ...kinds("skip", ["-n", "--max-count", "--skip", "--since", "--until", "--date", "-U"]), + }, +}; +/** @type {ArgSpec} */ +const GIT_GREP = { + patternFirst: true, + opts: { + ...kinds("pattern", ["-e"]), + ...kinds("file", ["-f"]), + ...kinds("skip", ["-A", "-B", "-C", "-m", "-O", "--max-depth", "--threads"]), + }, +}; +/** git's global options that take a value (`git -C dir show …`). */ +const GIT_GLOBAL_VALUES = new Set(["-C", "-c", "--git-dir", "--work-tree", "--namespace"]); +/** Commands that WRITE to their file operands (HI-06). */ +const WRITERS = new Set(["tee", "cp", "mv", "install", "ln", "truncate"]); + +/** + * A word's candidate paths: itself, the value of `key=path` (`dd of=`), the path of `REV:path` + * (`git show HEAD:.env`) and of `@file` (`curl -d @file`). + * @param {string} t + */ +function pathCandidates(t) { + const out = [t]; + const eq = /^[A-Za-z_][\w.-]*=([\s\S]*)$/.exec(t); + if (eq) out.push(eq[1]); + const colon = t.indexOf(":"); + if (colon > 0 && !/^[A-Za-z]:[\\/]/.test(t)) out.push(t.slice(colon + 1)); + if (t.startsWith("@")) out.push(t.slice(1)); + return out; +} + +/** + * The file operands among a command's arguments, under `spec`. + * @param {Word[]} args + * @param {ArgSpec} spec + * @returns {{word: Word, glob?: boolean}[]} + */ +function operands(args, spec) { + const opts = spec.opts ?? {}; + /** @type {{word: Word, glob?: boolean}[]} */ + const out = []; + /** @type {Word[]} */ + const positional = []; + let patternGiven = false; + let endOfOpts = false; + /** @param {string} kind @param {Word} word */ + const value = (kind, word) => { + if (kind === "file" || kind === "pattern") patternGiven = true; + if (kind === "glob") out.push({ word, glob: true }); + else if (kind === "file" || kind === "file2") out.push({ word }); + }; + for (let i = 0; i < args.length; i++) { + const w = args[i]; + const t = w.text; + if (endOfOpts || t.length < 2 || !t.startsWith("-")) { + positional.push(w); + continue; + } + if (t === "--") { + endOfOpts = true; + continue; + } + const eq = t.indexOf("="); + const name = eq > 0 ? t.slice(0, eq) : t; + const kind = opts[name]; + if (!kind) continue; + if (eq > 0) value(kind, { ...w, text: t.slice(eq + 1) }); + else if (kind === "skip2" || kind === "file2") { + if (kind === "file2" && args[i + 2]) value(kind, args[i + 2]); + i += 2; + } else if (args[i + 1]) value(kind, args[++i]); + } + if (spec.patternFirst && !patternGiven) positional.shift(); + return [...out, ...positional.map((word) => ({ word }))]; +} -// ── Command rules. A command word starts at the line start, after a separator, or after -// whitespace (so `sudo rm`, `env rm` and `/bin/rm` are all caught). +/** + * @param {Word} word + * @param {PathCtx} ctx + * @param {boolean} [asGlob] + */ +function isSecretWord(word, ctx, asGlob = false) { + return pathCandidates(word.text).some( + (c) => secretKind(c, ctx) !== null || ((asGlob || word.glob) && globMatchesSecret(c)), + ); +} + +const READ_REASON = + "reading a protected secret path via Bash is blocked. Read it yourself if intended."; +const REDIRECT_REASON = + "reading a protected secret path via input redirection is blocked. Read it yourself if intended."; +const WRITE_REASON = + "writing to a protected secret path via Bash is blocked. Edit it yourself if intended."; + +/** + * The reason a Bash command reads or writes a protected secret path (P0-04, HI-06), or null. + * Checked per simple command: redirection targets, a writer's operands and a reader's FILE + * operands, never a grep pattern, a commit message or a `--grep=` value. + * @param {string} cmd + * @param {FsCtx} [fsCtx] + * @returns {string | null} + */ +export function secretShellAccess(cmd, fsCtx = {}) { + /** @type {PathCtx} */ + const ctx = { ...fsCtx, bash: true }; + /** @param {{word: Word, glob?: boolean}[]} ops */ + const anySecret = (ops) => ops.some((o) => isSecretWord(o.word, ctx, o.glob)); + for (const { words, redirs } of shellSegments(String(cmd))) { + for (const { op, target } of redirs) { + const fd = op.endsWith("&") && /^(\d+-?|-)$/.test(target.text); // `2>&1`: no file + if (op === "<<<" || fd || !isSecretWord(target, ctx)) continue; + return op.startsWith(">") ? WRITE_REASON : REDIRECT_REASON; + } + const c = commandOf(words); + if (!c) continue; + const { name, args } = c; + if (WRITERS.has(name) && anySecret(operands(args, PLAIN))) return WRITE_REASON; + const inPlace = args.some((a) => /^-[A-Za-z]*i|^--in-place/.test(a.text)); + if (name === "sed" && inPlace && anySecret(operands(args, SED))) return WRITE_REASON; + const target = args.find((a) => a.text.startsWith("of=")); + if (name === "dd" && target && isSecretWord({ ...target, text: target.text.slice(3) }, ctx)) + return WRITE_REASON; + const spec = READERS[name]; + if (spec && anySecret(operands(args, spec))) return READ_REASON; + if (name === "git") { + let i = 0; + while (i < args.length && args[i].text.startsWith("-")) + i += GIT_GLOBAL_VALUES.has(args[i].text) ? 2 : 1; + const sub = args[i]?.text ?? ""; + const rest = args.slice(i + 1); + if (GIT_READERS.has(sub) && anySecret(operands(rest, sub === "grep" ? GIT_GREP : GIT_LOG))) + return READ_REASON; + } + } + return null; +} + +// ── Destructive-command rules. A command word starts at the line start, after a separator, or +// after whitespace (so `sudo rm`, `env rm` and `/bin/rm` are all caught). const B = "(^|[^A-Za-z0-9_.-])([^\\s;&|]*/)?"; const SEG = "([^;&|]*\\s)?"; // further args inside the SAME command segment -const TARGET = "[\"']?(/|~|\\$HOME|\\$\\{HOME\\})"; // an absolute/home path operand +const END = "(?=\\s|$|[;&|)])"; +// An absolute/home path operand, or the working tree itself: `.`, `..`, `./`, `./*`, `*`. +const TARGET = `["']?(/|~|\\$HOME|\\$\\{HOME\\}|(\\.\\.?/?\\*?|\\*)["']?${END})`; const RECUR = "(-[A-Za-z]*[rR][A-Za-z]*|--recursive)"; -// Git readers that can print file or history content (RA-05, HI-07), behind an optional -// `env `/`command `/`VAR=val ` prefix, an absolute path, and git's own global options. +// git behind an optional `env `/`command `/`VAR=val ` prefix, an absolute path, and git's own +// global options (HI-07). const gitpfx = "([A-Za-z0-9_]+=\\S+\\s+|(env|command|sudo)\\s+)*(\\S*/)?git\\s+"; const gitopt = "(-C\\s+\\S+\\s+|--no-pager\\s+|-c\\s+\\S+\\s+|--git-dir=\\S+\\s+|--work-tree=\\S+\\s+)*"; -const gitsub = "(show|log|diff|stash|cat-file|archive|grep|blame|show-index|bundle)(\\s|$)"; -const READER = `(^|[;&|])\\s*((cat|less|more|head|tail|nl|xxd|od|strings|base64|rg|grep|ag)\\s|${gitpfx}${gitopt}${gitsub})`; -// \b anchors the extensions so `.key` matches a real key file but NOT `Object.keys`, and -// `.env` matches `.env`/`.env.prod` but NOT `.environment`. -const SECRET_TOKEN = - "(\\.env(\\.[A-Za-z0-9_-]+)?\\b|id_rsa\\b|id_ed25519\\b|\\.pem\\b|\\.key\\b|/secrets/|/\\.ssh/|\\.netrc\\b|_netrc\\b|\\.npmrc\\b|\\.git-credentials\\b|\\.aws/credentials\\b)"; -// A protected path as a redirection target, or as an argument to a mutating command. Each -// alternative embeds the token, so a bare `echo hi > out.txt` is never blocked. -const WRITE = [ - `>>?\\s*["']?[^\\s<>|;&]*${SECRET_TOKEN}`, - `(^|[;&|])\\s*(${gitpfx})?(tee(\\s+-a)?|cp|mv|install)\\s+[^;&|]*${SECRET_TOKEN}`, - `(^|[;&|])\\s*sed\\s+[^;&|]*-i[^;&|]*${SECRET_TOKEN}`, - `(^|[;&|])\\s*dd\\s+([^;&|]*\\s)?of=\\S*${SECRET_TOKEN}`, -].join("|"); - -/** @type {{all: RegExp[], reason: string}[]} — first match wins; protected paths first, so - * `dd if=x of=.env` reads as a secret write rather than as a generic `dd of=`. */ +// A pathspec naming the whole tree. +const ALL_PATHS = `["']?(\\.|\\./|:/|\\*)["']?${END}`; +/** A flag (`--long` or a short letter, possibly grouped) somewhere in the same segment. + * @param {string} long @param {string} short */ +const flagIn = (long, short) => + `(?:[^;&|]*\\s)?(?:--${long}|-[A-Za-z]*${short}[A-Za-z]*)(?:\\s|$|[;&|])`; +// `git restore --staged .` only unstages; the working tree is untouched, so it stays allowed. +const STAGED_ONLY = `(?=${flagIn("staged", "S")})(?!${flagIn("worktree", "W")})`; +// The far end of a pipe, behind `sudo`/`doas`/`env`/`command` wrappers. +const PIPED = + "\\|&?\\s*((sudo|doas)(\\s+(-[ugCDhprtUT]\\s+\\S+|-\\S+))*\\s+|env(\\s+-\\S+)*(\\s+[A-Za-z_]\\w*=\\S*)*\\s+|(command|exec|nohup)\\s+)*(\\S*/)?"; + +/** @type {{all: RegExp[], reason: string}[]} — first match wins. */ const COMMAND_RULES = [ { - // Close the Bash secret-READ bypass (P0-04): the Read tool denies .env/keys, but a shell - // `cat .env` / `git show HEAD:.env` sidesteps that. A reader command AND a protected - // path token — so prose in a quoted arg (a commit message naming ".env") is not a hit. - all: [new RegExp(READER), new RegExp(SECRET_TOKEN)], - reason: "reading a protected secret path via Bash is blocked. Read it yourself if intended.", - }, - { - // Close the Bash secret-WRITE bypass (HI-06). - all: [new RegExp(WRITE)], - reason: "writing to a protected secret path via Bash is blocked. Edit it yourself if intended.", - }, - { - // Recursive delete of an absolute/home path, flags in any order or grouping. + // Recursive delete of an absolute/home path or the whole working tree, flags in any order. all: [ new RegExp( `${B}rm\\s+${SEG}${RECUR}(\\s[^;&|]*)?\\s${TARGET}|${B}rm\\s+${SEG}${TARGET}[^;&|]*\\s${RECUR}(\\s|$)|${B}rm\\s+${SEG}--no-preserve-root`, ), ], - reason: "destructive rm (recursive delete of an absolute/home path) detected.", + reason: + "destructive rm (recursive delete of an absolute/home path or the working tree) detected.", }, { // `--force-with-lease` / `--force-if-includes` are the SAFE variants and stay allowed. @@ -93,6 +757,16 @@ const COMMAND_RULES = [ all: [new RegExp(`${gitpfx}${gitopt}reset\\s${SEG}--hard(\\s|$)`)], reason: "`git reset --hard` discards uncommitted work. Ask the user first.", }, + { + // The same data loss as `reset --hard`, spelled as a whole-tree checkout or restore. + all: [ + new RegExp( + `${gitpfx}${gitopt}(checkout\\s${SEG}${ALL_PATHS}|restore\\s(?!${STAGED_ONLY})${SEG}${ALL_PATHS})`, + ), + ], + reason: + "`git checkout .` / `git restore .` discard every uncommitted change. Ask the user first.", + }, { all: [ new RegExp(`${gitpfx}${gitopt}clean\\s${SEG}(-[A-Za-z0-9]*f[A-Za-z0-9]*|--force)(\\s|$)`), @@ -118,33 +792,53 @@ const COMMAND_RULES = [ reason: "destructive SQL detected. Confirm with the user.", }, { - // Pipe-to-shell (curl … | sh). Boundary-aware so `… | shellcheck` is not caught. - all: [/\|\s*(sh|bash|zsh)(\s|$)/], + // Pipe-to-shell (`curl … | sh`, `| sudo bash`), or to an interpreter that runs its stdin as + // the program (`| python3`, `| node -`). Boundary-aware, so `… | shellcheck` and + // `… | python3 -m json.tool` are not caught. + all: [ + new RegExp( + `${PIPED}((sh|bash|zsh|dash|ksh|mksh|fish)(\\s|$|[;&|)])|(python[0-9.]*|node|nodejs|perl|ruby|php|deno|bun)(\\s+-)?\\s*($|[;&|)]))`, + ), + ], reason: "piping content to a shell is blocked.", }, ]; /** - * PURE decision over one tool call — the testable core. - * @param {{toolName?: string, filePath?: string, command?: string}} call + * PURE decision over one tool call — the testable core. Its one filesystem touch is reading a + * project `.npmrc` to see whether it holds a token (`readText`, injectable). + * @param {{toolName?: string, filePath?: string, command?: string, glob?: string} & FsCtx} [call] * @returns {{block: boolean, reason?: string}} */ -export function protectPathsDecision({ toolName = "", filePath = "", command = "" } = {}) { - const path = String(filePath).replaceAll("\\", "/"); - if (path) { - // A plugin install carries no `permissions.deny` block, so for Read this guard is the - // only thing between the agent and `.env`. - const verb = /^(Read|Grep|Glob|NotebookRead)$/.test(String(toolName)) ? "read" : "modify"; - for (const { re, what } of FILE_RULES) { - if (re.test(path)) - return { - block: true, - reason: `refusing to ${verb} ${what} (${filePath}). Handle it yourself if intended.`, - }; - } +export function protectPathsDecision({ + toolName = "", + filePath = "", + command = "", + glob = "", + ...fsCtx +} = {}) { + // A plugin install carries no `permissions.deny` block, so for Read/Grep this guard is the + // only thing between the agent and `.env`. + const verb = /^(Read|Grep|Glob|NotebookRead)$/.test(String(toolName)) ? "read" : "modify"; + if (filePath) { + const what = secretKind(String(filePath), fsCtx); + if (what) + return { + block: true, + reason: `refusing to ${verb} ${what} (${filePath}). Handle it yourself if intended.`, + }; } + // A literal name (`.env`) or a wildcard that can select one; an exclusion (`!.env`) never does. + const g = String(glob); + if (g && !g.startsWith("!") && (secretKind(g, fsCtx) || globMatchesSecret(g))) + return { + block: true, + reason: `refusing to ${verb} files matching ${glob}: it selects a protected secret path. Handle it yourself if intended.`, + }; const cmd = String(command); if (cmd) { + const io = secretShellAccess(cmd, fsCtx); + if (io) return { block: true, reason: io }; for (const rule of COMMAND_RULES) { if (rule.all.every((re) => re.test(cmd))) return { block: true, reason: rule.reason }; } @@ -185,6 +879,9 @@ async function main() { toolName: data.tool_name, filePath: inp.file_path ?? inp.notebook_path ?? inp.path ?? "", command: inp.command ?? "", + // The Grep tool's `glob` filter selects the files it reads; Glob only lists names. + glob: data.tool_name === "Grep" ? (inp.glob ?? "") : "", + cwd: typeof data.cwd === "string" && data.cwd ? data.cwd : process.cwd(), }); if (d.block) deny(String(d.reason)); } diff --git a/global/settings.template.json b/global/settings.template.json index 54d18c0f..a4a2dc3d 100644 --- a/global/settings.template.json +++ b/global/settings.template.json @@ -181,7 +181,7 @@ ] }, { - "matcher": "Read", + "matcher": "Read|Grep|Glob|NotebookRead", "hooks": [ { "type": "command", diff --git a/hooks/hooks.json b/hooks/hooks.json index 90b02931..f8c0239e 100644 --- a/hooks/hooks.json +++ b/hooks/hooks.json @@ -93,7 +93,7 @@ ] }, { - "matcher": "Read", + "matcher": "Read|Grep|Glob|NotebookRead", "hooks": [ { "type": "command", diff --git a/test/guards.test.js b/test/guards.test.js index b9e1aeb0..a5349669 100644 --- a/test/guards.test.js +++ b/test/guards.test.js @@ -352,6 +352,9 @@ test("protect-paths rules: destructive commands the literal substrings missed (B }); test("protect-paths protects the credential stores and Read itself (B6)", () => { + // `/home/u` is HOME for the guard, so `/home/u/.npmrc` is the USER-level npmrc — where + // `npm login` writes the token — and stays protected with no file on disk. + const env = { ...process.env, HOME: "/home/u", USERPROFILE: "/home/u" }; for (const file_path of [ "/home/u/.aws/credentials", "/home/u/.netrc", @@ -361,14 +364,17 @@ test("protect-paths protects the credential stores and Read itself (B6)", () => "C:\\proj\\.env", ]) { for (const tool_name of ["Write", "Read"]) { - const r = runGuard("protect-paths.sh", { tool_name, tool_input: { file_path } }); + const r = runGuard("protect-paths.sh", { tool_name, tool_input: { file_path } }, { env }); assert.equal(r.code, 2, `must block ${tool_name} of ${file_path}`); assert.match(r.err, tool_name === "Read" ? /refusing to read/ : /refusing to modify/); } } - // Bash readers/writers of the same stores are blocked too. + // Bash readers/writers of the same stores are blocked too — a project `.npmrc` when it + // holds a literal token. + const token = () => "//registry.npmjs.org/:_authToken=npm_abc123"; for (const command of ["cat ~/.netrc", "cat .npmrc", "echo x > ~/.git-credentials"]) { - assert.equal(protectPathsDecision({ toolName: "Bash", command }).block, true, command); + const d = protectPathsDecision({ toolName: "Bash", command, readText: token }); + assert.equal(d.block, true, command); } // …and an ordinary source file is still untouched. assert.equal( diff --git a/test/protect_paths.test.js b/test/protect_paths.test.js new file mode 100644 index 00000000..7b6aad2d --- /dev/null +++ b/test/protect_paths.test.js @@ -0,0 +1,436 @@ +// protect-paths: secret paths match per PATH TOKEN, not as a substring of the command. +// +// The old rule matched `\.env(\.[\w-]+)?\b` anywhere in a Bash command, so every read-only +// command that merely MENTIONED an env accessor was blocked — `grep -rn process.env src`, +// `rg 'import\.meta\.env'`, `git log --grep='.env handling'` — along with `.env.example`, +// `messages.key.ts`, a plain project `.npmrc` and a Next.js `app/docs/secrets/page.tsx` route. +// Meanwhile real reads slipped through: `sed -n p .env`, `awk 1 .env`, `tac .env`, +// `… < .env`, `cat .e*v`. These tests pin both halves: every false positive from the review +// stays allowed, and every secret read / destructive command stays (or is now) blocked. +import assert from "node:assert/strict"; +import { spawnSync } from "node:child_process"; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; +import { test } from "node:test"; +import { fileURLToPath } from "node:url"; + +import { + globMatchesSecret, + npmrcHasToken, + protectPathsDecision, + secretKind, + secretShellAccess, + shellSegments, +} from "../global/guards/protect-paths.mjs"; + +const guards = join(dirname(fileURLToPath(import.meta.url)), "..", "global", "guards"); +const noFile = () => null; +/** @param {string} command */ +const bash = (command, extra = {}) => + protectPathsDecision({ toolName: "Bash", command, readText: noFile, ...extra }); + +/** Run the real guard end to end (node twin, no bash), hook JSON on stdin. */ +function hook(payload) { + const r = spawnSync(process.execPath, [join(guards, "protect-paths.mjs")], { + input: JSON.stringify(payload), + encoding: "utf8", + }); + return { code: r.status, err: r.stderr ?? "" }; +} + +// ── The review's false positives: read-only commands and files that must be ALLOWED. + +test("false positives from the review are allowed: env accessors, templates, code files", () => { + for (const command of [ + "grep -rn process.env src", + "rg 'process\\.env\\.WHMCS' src/lib", + "grep -rn import.meta.env src", + "git log -p --grep='.env handling'", + "git log -p --grep=.env", + "git show HEAD:src/lib/whmcs.ts | grep process.env", + "npx tsc --noEmit | grep -i process.env", + "cat src/i18n/messages.key.ts", + "cat .env.example", + "cat .env.sample", + "cat .env.template", + "cat .env.local.example", + "cat src/app/docs/secrets/page.tsx", + "cat id_rsa.pub", // a PUBLIC key + ]) { + const d = bash(command); + assert.equal(d.block, false, `must not block: ${command} (${d.reason})`); + } + for (const [tool, file_path] of [ + ["Read", "/p/.env.example"], + ["Read", "/p/.env.sample"], + ["Read", "/p/.env.template"], + ["Read", "/p/src/app/docs/secrets/page.tsx"], + ["Read", "/p/src/i18n/messages.key.ts"], + ["Read", "/p/src/config.env.ts"], + ["Read", "/p/.env/lib/python3.12/site.py"], // a virtualenv named `.env` + ["Edit", "/p/src/app/docs/secrets/page.tsx"], + ["Grep", "/p/src/app/docs/secrets"], + ]) { + const d = protectPathsDecision({ toolName: tool, filePath: file_path, readText: noFile }); + assert.equal(d.block, false, `must not block ${tool} of ${file_path} (${d.reason})`); + } +}); + +test("a grep/rg/sed/awk/jq PATTERN is not a path — only file operands are checked", () => { + for (const command of [ + 'grep -rn ".env" src', + "rg -n '\\.env' src", + "grep -e .env -r src", + "jq -r .env config.json", + "awk '/.env/' notes.txt", + "sed -n 's/.env/x/p' README.md", + "rg -g '!.env' TOKEN", // an exclusion glob + "rg -g '*.ts' process.env", + "git grep -n process.env -- src", + 'git commit -m "block cat .env and git show HEAD:.env reads"', + "cat < notes.md\ncat .env\nEOF\necho done", // a heredoc body is data + "echo hi # cat .env", // a comment + ]) { + const d = bash(command); + assert.equal(d.block, false, `must not block: ${command} (${d.reason})`); + } + // …while the same tools reading a secret FILE are still blocked. + for (const command of [ + "grep password .env", + "grep -e KEY .env", + "grep -f .env src/a.js", // -f FILE reads the file + "rg KEY .env.local", + "jq . .env", + "awk 1 .env", + "sed -n p .env", + "git grep -n KEY -- .env", + ]) { + assert.equal(bash(command).block, true, `must block: ${command}`); + } +}); + +// ── Still blocked: real secrets, through every reader the review found missing. + +test("real secret files stay blocked: .env, .env.local, .env.production, id_rsa, *.key, *.pem", () => { + for (const f of [".env", ".env.local", ".env.production", ".env.production.local"]) { + assert.equal(bash(`cat ${f}`).block, true, `cat ${f}`); + assert.equal(bash(`cat ./${f}`).block, true, `cat ./${f}`); + assert.equal( + protectPathsDecision({ toolName: "Read", filePath: `/p/${f}` }).block, + true, + `Read ${f}`, + ); + } + for (const f of ["id_rsa", "id_ed25519", "server.key", "certs/server.pem", "tls.key.bak"]) { + assert.equal(bash(`cat ${f}`).block, true, `cat ${f}`); + assert.equal(protectPathsDecision({ toolName: "Read", filePath: `/p/${f}` }).block, true, f); + } + for (const f of ["/p/prod.env", "/p/secrets/db.txt", "/run/secrets/db_password", "/h/.ssh"]) { + assert.equal(protectPathsDecision({ toolName: "Read", filePath: f }).block, true, f); + } +}); + +test("the readers the review found missing are blocked: sed awk tac sort uniq cut paste bat jq diff cmp fold rev", () => { + for (const command of [ + "sed -n p .env", + "awk 1 .env", + "tac .env", + "sort .env", + "uniq .env", + "cut -d= -f1 .env", + "paste .env", + "bat .env", + "jq . .env", + "diff .env .env.example", + "cmp .env other", + "fold .env", + "rev .env", + "hexdump -C .env", + "dd if=.env", + ]) { + const d = bash(command); + assert.equal(d.block, true, `must block: ${command}`); + assert.match(String(d.reason), /protected secret path/); + } +}); + +test("input redirection from a secret is blocked; a heredoc, a herestring and fd dups are not", () => { + for (const command of [ + "while read l; do echo $l; done < .env", + "node script.js < .env.local", + "x=$(< .env)", + "mysql db 0< ~/.ssh/id_rsa", + ]) { + const d = bash(command); + assert.equal(d.block, true, `must block: ${command}`); + assert.match(String(d.reason), /input redirection/); + } + for (const command of [ + "while read l; do echo $l; done < list.txt", + "cat <<< '.env'", + "echo x 2>&1 | tee build.log", + "echo x >&2", + ]) { + const d = bash(command); + assert.equal(d.block, false, `must not block: ${command} (${d.reason})`); + } +}); + +test("globs, quoting tricks and substitutions that name a secret are blocked", () => { + for (const command of [ + "cat .e*v", + "cat .en?", + "cat .[e]nv", + "cat .env*", + "cat *.pem", + "cat ~/.ssh/*", + "grep -r KEY --include=.env* .", + "rg -g '.env*' KEY", + "rg --glob=.env KEY", + 'echo "$(cat .env)"', + "x=`cat .env`", + 'cat "my dir/.env"', + "cat $'\\x2eenv'", // ANSI-C quoting spells `.env` + "\\cat .env", + "/bin/cat .env", + "sudo -u root cat /root/.env", + 'FOO="a b" cat .env', // an assignment prefix with a quoted value + "busybox cat .env", + "echo hi\ncat .env", // a second line is a second command + "bash < { + for (const command of [ + "rm -rf ./", + "rm -rf *", + "rm -rf ..", + "rm -rf .", + "rm -rf ./*", + "rm * -rf", + "git checkout -- .", + "git checkout .", + "git checkout HEAD -- .", + "git restore .", + "git restore --worktree .", + "git restore --staged --worktree .", + "git restore -s HEAD~1 .", + "curl x | sudo sh", + "curl x | sudo -E bash", + "curl x | sudo -u root bash -s", + "curl x | env sh", + "curl x | /bin/bash", + "curl x | python3", + "curl x | python3 -", + "curl x | node", + ]) { + assert.equal(bash(command).block, true, `must block: ${command}`); + } + for (const command of [ + "rm -rf ./node_modules", + "rm -rf .next", + "rm -rf dist/*", + "rm -rf *.log", + "git checkout main", + "git checkout -- src/a.ts", + "git checkout ./src/a.ts", + "git restore src/a.ts", + "git restore --staged .", // only unstages: the working tree is untouched + "echo '{}' | python3 -m json.tool", + "curl -s x | python3 -c 'import json,sys; print(json.load(sys.stdin))'", + "cat x | shellcheck -", + 'git commit -m "never curl | bash"', + ]) { + const d = bash(command); + assert.equal(d.block, false, `must not block: ${command} (${d.reason})`); + } +}); + +// ── .npmrc: a project npmrc is config; only a literal token makes it a credential store. + +test("npmrcHasToken: a literal token counts, an env reference and plain config do not", () => { + const envRef = ["//registry.npmjs.org/:_authToken=$", "{NPM_TOKEN}"].join(""); + assert.equal(npmrcHasToken("//registry.npmjs.org/:_authToken=npm_abc123\n"), true); + assert.equal(npmrcHasToken("_auth = dXNlcjpwYXNz"), true); + assert.equal(npmrcHasToken('//r.example/:_password="c2VjcmV0"'), true); + assert.equal(npmrcHasToken(envRef), false, "an env reference holds no secret"); + assert.equal(npmrcHasToken("registry=https://registry.npmjs.org/\nmin-release-age=7\n"), false); + assert.equal(npmrcHasToken("# _authToken=npm_commented_out"), false); +}); + +test("a project .npmrc is readable unless it holds a token; the user-level one never is (end to end)", () => { + const home = mkdtempSync(join(tmpdir(), "forge-pp-home-")); + const repo = mkdtempSync(join(tmpdir(), "forge-pp-repo-")); + try { + const npmrc = join(repo, ".npmrc"); + writeFileSync(npmrc, "registry=https://registry.npmjs.org/\nengine-strict=true\n"); + const ctx = { cwd: repo, home }; + assert.equal(protectPathsDecision({ toolName: "Read", filePath: npmrc, ...ctx }).block, false); + assert.equal( + protectPathsDecision({ toolName: "Bash", command: "cat .npmrc", ...ctx }).block, + false, + ); + // Through the real hook: `cwd` comes from the payload. + assert.equal( + hook({ tool_name: "Bash", cwd: repo, tool_input: { command: "cat .npmrc" } }).code, + 0, + ); + + writeFileSync(npmrc, "//registry.npmjs.org/:_authToken=npm_abc123\n"); + assert.equal(protectPathsDecision({ toolName: "Read", filePath: npmrc, ...ctx }).block, true); + assert.equal( + protectPathsDecision({ toolName: "Bash", command: "cat .npmrc", ...ctx }).block, + true, + ); + const r = hook({ tool_name: "Bash", cwd: repo, tool_input: { command: "cat .npmrc" } }); + assert.equal(r.code, 2); + assert.match(r.err, /protected secret path/); + + // The user-level npmrc is protected whatever it holds, even when absent. + const user = join(home, ".npmrc"); + assert.equal(protectPathsDecision({ toolName: "Read", filePath: user, ...ctx }).block, true); + for (const command of ["cat ~/.npmrc", "cat $HOME/.npmrc", "cat $DIR/.npmrc"]) + assert.equal( + protectPathsDecision({ toolName: "Bash", command, ...ctx }).block, + true, + command, + ); + } finally { + rmSync(home, { recursive: true, force: true }); + rmSync(repo, { recursive: true, force: true }); + } +}); + +// ── The Grep / Glob / NotebookRead tools reach the guard (the manifests now route them). + +test("Grep, Glob and NotebookRead: secret paths and a secret-selecting Grep glob are blocked (end to end)", () => { + for (const payload of [ + { tool_name: "Grep", tool_input: { pattern: "KEY", path: "/p/.env" } }, + { tool_name: "Grep", tool_input: { pattern: "KEY", path: "/home/u/.ssh" } }, + { tool_name: "Grep", tool_input: { pattern: "KEY", path: "/p", glob: ".env*" } }, + { tool_name: "Grep", tool_input: { pattern: "KEY", glob: "**/*.pem" } }, + { tool_name: "Glob", tool_input: { pattern: "*", path: "/home/u/.ssh" } }, + { tool_name: "NotebookRead", tool_input: { notebook_path: "/p/secrets/creds.ipynb" } }, + ]) { + const r = hook(payload); + assert.equal(r.code, 2, `must block: ${JSON.stringify(payload)}`); + assert.match(r.err, /refusing to read/); + } + for (const payload of [ + { tool_name: "Grep", tool_input: { pattern: "process.env", path: "/p/src" } }, + { tool_name: "Grep", tool_input: { pattern: "KEY", path: "/p", glob: "*.ts" } }, + { tool_name: "Grep", tool_input: { pattern: ".env", path: "/p", glob: "!.env" } }, + { tool_name: "Glob", tool_input: { pattern: "**/.env*" } }, // names only, no content + { tool_name: "NotebookRead", tool_input: { notebook_path: "/p/analysis.ipynb" } }, + ]) { + assert.equal(hook(payload).code, 0, `must not block: ${JSON.stringify(payload)}`); + } +}); + +// ── The building blocks. + +test("secretKind: one path predicate for tool paths and Bash words", () => { + assert.equal(secretKind("/p/.env"), "env file"); + assert.equal(secretKind("/p/.env.example"), null); + assert.equal(secretKind("/p/prod.env"), "env file"); + assert.equal( + secretKind("prod.env", { bash: true }), + null, + "a bare x.env word may be process.env", + ); + assert.equal(secretKind("config/prod.env", { bash: true }), "env file"); + assert.equal(secretKind("process.env", { bash: true }), null); + assert.equal( + secretKind("process\\.env", { bash: true }), + null, + "regex text is not a Windows path", + ); + assert.equal(secretKind("C:\\proj\\.env", { bash: true }), "env file"); + assert.equal(secretKind("/p/foo.key.ts"), null); + assert.equal(secretKind("/p/foo.key"), "credential/key file"); + assert.equal(secretKind("/p/secrets/page.tsx"), null); + assert.equal(secretKind("/p/secrets/db.yaml"), "path under secrets/ or .ssh/"); + assert.equal(secretKind("C:\\Users\\u\\.aws\\credentials"), "credential store"); + assert.equal(secretKind("/x/.npmrc", { home: "/h", readText: noFile }), null); + assert.equal(secretKind("/h/.npmrc", { home: "/h", readText: noFile }), "credential store"); +}); + +test("globMatchesSecret: dotfiles need a dot-led glob; wildcard-only globs are not targeted", () => { + for (const g of [".e*v", ".env*", ".*", "*.pem", "**/*.key", "id_*", ".[e]nv", "*.env"]) + assert.equal(globMatchesSecret(g), true, g); + for (const g of ["*", "*.*", "*.ts", "src/**/*.tsx", "!.env*", ".env", "*.md"]) + assert.equal(globMatchesSecret(g), false, g); +}); + +test("shellSegments: quotes, separators, substitutions and redirections", () => { + const segs = shellSegments(`a "b c" 'd;e' > out.txt; f $(g .env) | h 2>&1 < in`); + const words = segs.map((s) => s.words.map((w) => w.text)); + assert.deepEqual(words, [["a", "b c", "d;e"], ["g", ".env"], ["f", "$()"], ["h"]]); + assert.deepEqual( + segs.flatMap((s) => s.redirs.map((r) => `${r.op}${r.target.text}`)), + [">out.txt", ">&1", " s.words[0].text), + ["cat", "ls"], + "a heredoc body is skipped", + ); +}); + +test("secretShellAccess: reports read vs write, per simple command", () => { + assert.match(String(secretShellAccess("cat .env")), /reading/); + assert.match(String(secretShellAccess("echo x > .env")), /writing/); + assert.match(String(secretShellAccess("sed -i s/a/b/ .env")), /writing/); + assert.match(String(secretShellAccess("truncate -s0 .env")), /writing/); + assert.equal(secretShellAccess("cat README.md; echo .env"), null, "echo is not a reader"); + assert.equal(secretShellAccess("cat .env.example > .env.sample"), null); +}); + +test("the protect-paths hook still fails CLOSED on a payload it cannot parse (node twin)", () => { + const r = spawnSync(process.execPath, [join(guards, "protect-paths.mjs")], { + input: "not json", + encoding: "utf8", + }); + assert.equal(r.status, 2); + assert.match(r.stderr, /fail closed/); +}); + +test("a large command is still checked end to end", () => { + const big = `echo start\n${Array.from({ length: 20000 }, (_, i) => `# note ${i}`).join("\n")}\ncat .env`; + const r = hook({ tool_name: "Bash", tool_input: { command: big } }); + assert.equal(r.code, 2); +}); + +test("a real .env inside a temp repo: Read blocked, .env.example allowed (end to end)", () => { + const repo = mkdtempSync(join(tmpdir(), "forge-pp-env-")); + try { + mkdirSync(join(repo, "src")); + writeFileSync(join(repo, ".env"), "KEY=1\n"); + writeFileSync(join(repo, ".env.example"), "KEY=\n"); + assert.equal( + hook({ tool_name: "Read", tool_input: { file_path: join(repo, ".env") } }).code, + 2, + ); + assert.equal( + hook({ tool_name: "Read", tool_input: { file_path: join(repo, ".env.example") } }).code, + 0, + ); + } finally { + rmSync(repo, { recursive: true, force: true }); + } +}); diff --git a/test/settings_template.test.js b/test/settings_template.test.js index 33893e58..6e5d115b 100644 --- a/test/settings_template.test.js +++ b/test/settings_template.test.js @@ -141,6 +141,25 @@ test("PreToolUse protect-paths also covers Read in both manifests (B6)", () => { } }); +// The review: protectPathsDecision handled Grep/Glob/NotebookRead, but no manifest ever sent +// them to it — the Grep tool could read `.env` (or everything under `~/.ssh`) unchecked. +test("PreToolUse protect-paths also covers Grep, Glob and NotebookRead in all three manifests", () => { + const project = JSON.parse( + readFileSync(new URL("../.claude/settings.json", import.meta.url), "utf8"), + ); + for (const [name, manifest] of [ + ["settings.template.json", template], + ["hooks.json", pluginHooks], + [".claude/settings.json", project], + ]) { + const tools = (manifest.hooks?.PreToolUse ?? []) + .filter((g) => (g.hooks ?? []).some((h) => hookText(h).includes("protect-paths.sh"))) + .flatMap((g) => (g.matcher ?? "").split("|")); + for (const tool of ["Read", "Grep", "Glob", "NotebookRead"]) + assert.ok(tools.includes(tool), `${name}: protect-paths must run on ${tool} (got: ${tools})`); + } +}); + test("the credential stores the guard protects are denied for Read too (B6)", () => { const deny = template.permissions?.deny ?? []; for (const rule of [ From 04b245a997f9eac39a858fc04f6e376257c50fec Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 23 Sep 2026 20:48:58 +0000 Subject: [PATCH 2/7] fix(guards): run protect-paths on node and fail closed With no bash on PATH the hook launcher exited 1, which Claude Code treats as a NON-blocking hook error, so the PreToolUse secret guard was silently off; a signal-killed guard also returned 1. run.mjs now has a GUARD_POLICY: protect-paths.sh is only a thin launcher over protect-paths.mjs, so the launcher runs that twin on its own node (bash leaves the path, one process fewer per tool call), and the guard is fail-closed: no interpreter, a spawn error, a signal or an exit other than 0/2 becomes exit 2 with the reason on stderr. `node run.mjs --fail-closed .sh` opts any other guard in. Advisory guards keep the visible, non-blocking exit 1. The hook manifests keep `run.mjs .../protect-paths.sh`, so guard identity (guardKey) is unchanged and existing settings.json installs get the fix with the package, no re-merge needed. doctor now requires guards/protect-paths.mjs in an install and says protect-paths still blocks when bash is missing. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01UUhB8JaPayd43w37dxiXrW Signed-off-by: Claude --- global/guards/protect-paths.sh | 4 +- global/guards/run.mjs | 84 ++++++++++++++++++------ src/doctor.js | 6 +- test/hook_launcher.test.js | 116 +++++++++++++++++++++++++++++---- 4 files changed, 176 insertions(+), 34 deletions(-) diff --git a/global/guards/protect-paths.sh b/global/guards/protect-paths.sh index 27625729..f4e65e51 100755 --- a/global/guards/protect-paths.sh +++ b/global/guards/protect-paths.sh @@ -2,7 +2,9 @@ # PreToolUse hook: block reads/edits of secret/credential files and obviously destructive Bash. # Thin launcher: the payload parsing and the whole rule set live in protect-paths.mjs (Node), # the same split secret-redact.sh uses — one real JSON parser instead of jq-or-a-regex, and no -# shell pipeline that can lose a match to SIGPIPE under `pipefail`. +# shell pipeline that can lose a match to SIGPIPE under `pipefail`. The hook launcher (run.mjs) +# skips this file and runs protect-paths.mjs on node directly, fail-closed; this shim remains for +# direct `bash protect-paths.sh` callers. # Exit 2 = block the tool call and feed the reason back to Claude (works across versions). # FAIL CLOSED: exit 1 is a NON-blocking hook error in Claude Code, so a guard that cannot # evaluate the call must deny, never fall through. diff --git a/global/guards/run.mjs b/global/guards/run.mjs index 03fe1dad..a2919609 100755 --- a/global/guards/run.mjs +++ b/global/guards/run.mjs @@ -17,9 +17,16 @@ // exits 1: the same visible, non-blocking hook error the ENOENT was, minus the mystery. Node // built-ins only — a launcher that itself failed to load would be exactly the silent no-op the // guards exist to prevent. +// +// SECURITY guards are the exception to "exit 1": for a PreToolUse guard, exit 1 lets the tool call +// through, so a guard that cannot run would be silently OFF. A fail-closed guard (GUARD_POLICY, or +// `--fail-closed` before the guard path) turns every failure to reach a verdict — no interpreter, a +// spawn error, a signal, an exit other than 0/2 — into exit 2: a block, with the reason on stderr. +// protect-paths also skips bash entirely: its `.sh` is a thin launcher over a Node twin, which runs +// on this very node, so the guard works where bash does not. import { spawnSync } from "node:child_process"; import { existsSync } from "node:fs"; -import { basename, win32 } from "node:path"; +import { basename, dirname, join, win32 } from "node:path"; import { fileURLToPath } from "node:url"; export const NO_BASH_HINT = @@ -117,35 +124,74 @@ export function resolveBash({ const toPosix = (p) => String(p).replaceAll("\\", "/"); /** - * Spawn `bash