From 72d0232ead16d4bd1a4682814b2d6110bfcd8f8a Mon Sep 17 00:00:00 2001 From: devmaxxx Date: Mon, 19 Oct 2026 14:41:27 +0200 Subject: [PATCH 1/2] fix: close the 0.7.0 release audit findings Redaction keeps provider rules only. The catch-all rules (generic-api-key, kingfisher.generic/http/auth/curl, curl-auth-*, hashicorp-tf-password, credentials.1, uri.1) flagged 53 spans on the bench fixture, none a secret, and one cost FR-CAL-155 its edge. A span that holds a graph or requirement id is never redacted, and a database URL on a loopback host or with an interpolated or masked password is left as written. The fixture now redacts nothing. redact_secrets = false in zerograph.toml, or ZEROGRAPH_REDACT_SECRETS for one run, turns redaction off. zerograph families now redacts the lines it prints, as the extractor does. The plugin hook stays silent on every event that committed 0.6.x entries in .claude/settings.json still answer, and under Codex on the events left in $CODEX_HOME/hooks.json, so a session hears one brief. npm-pack rewrites an HTML src="..." to the tagged raw URL, so the README logo shows on the npm page. The 0.7.0 notes name the redaction, the two MCP tool changes, the double-hook fix and the GRAMMAR 6 to 7 re-read. The mcp help names the plugins. The comparison GIF shows each frame 10 s. --- agent/claude-code/scripts/hook.mjs | 37 +++++++- agent/claude-code/scripts/hook.test.mjs | 57 +++++++++++- agent/codex/scripts/hook.mjs | 37 +++++++- docs/assets/comparison.gif | Bin 245062 -> 245062 bytes docs/configuration.md | 28 +++++- docs/install.md | 4 +- docs/releases/0.7.0.md | 25 +++++ scripts/comparison-gif.py | 2 +- scripts/npm-pack.sh | 4 +- src/config.rs | 15 ++- src/families.rs | 31 ++++++- src/lib.rs | 14 ++- src/secrets.rs | 117 +++++++++++++++++++++++- tests/common/mod.rs | 1 + tests/redaction.rs | 76 +++++++++++++++ 15 files changed, 423 insertions(+), 25 deletions(-) create mode 100644 tests/redaction.rs diff --git a/agent/claude-code/scripts/hook.mjs b/agent/claude-code/scripts/hook.mjs index 6c37a918..cc04f38d 100644 --- a/agent/claude-code/scripts/hook.mjs +++ b/agent/claude-code/scripts/hook.mjs @@ -21,7 +21,7 @@ import { existsSync, readFileSync, writeFileSync, mkdirSync, realpathSync, lstat import { spawn, spawnSync } from 'node:child_process'; import { join, dirname, relative, isAbsolute } from 'node:path'; import { fileURLToPath } from 'node:url'; -import { tmpdir } from 'node:os'; +import { tmpdir, homedir } from 'node:os'; import { createHash } from 'node:crypto'; import { binaryOf, checkoutOf as repositoryOf, childEnv as cutEnv, commandOf, pathKey } from './launcher.mjs'; @@ -547,6 +547,40 @@ const handlers = { PostToolUse: (p, root, key) => (built(root) ? riskLine(p, root, key) : null), }; +const LEGACY_HOOK = 'zerograph-hook.mjs'; + +/** The commands a harness hooks file runs for `event`, or none when the file is absent or unreadable. */ +function hookCommands(file, event) { + let parsed; + try { parsed = JSON.parse(readFileSync(file, 'utf8')); } catch { return []; } + const groups = parsed?.hooks?.[event]; + if (!Array.isArray(groups)) return []; + return groups.flatMap((g) => (Array.isArray(g?.hooks) ? g.hooks : [])) + .map((h) => [h?.command, ...(Array.isArray(h?.args) ? h.args : [])].filter((w) => typeof w === 'string').join(' ')); +} + +/** + * Whether a hook an earlier `install-agent` wired answers `event` already, so this one stays + * silent rather than put the brief, the seeds and the risk line in context twice. In Claude Code + * that is a repository's committed `.claude/settings.json`, which `install-agent` leaves for the + * team to retire; in Codex it is the machine's `$CODEX_HOME/hooks.json`, which nothing a + * repository ships can touch. Either way the old script has to be there too: an entry whose + * script is gone answers nothing. + */ +function legacyAnswers(event, root) { + if (!process.env.CLAUDE_PLUGIN_ROOT && process.env.PLUGIN_ROOT) { + const home = process.env.CODEX_HOME || join(homedir(), '.codex'); + return hookCommands(join(home, 'hooks.json'), event).some((cmd) => { + const script = cmd.match(/(?:"([^"]*zerograph-hook\.mjs)"|'([^']*zerograph-hook\.mjs)'|(\S*zerograph-hook\.mjs))/); + const path = script && (script[1] ?? script[2] ?? script[3]); + return Boolean(path) && isAbsolute(path) && existsSync(path); + }); + } + const project = process.env.CLAUDE_PROJECT_DIR || root; + return existsSync(join(project, '.claude', 'hooks', LEGACY_HOOK)) + && hookCommands(join(project, '.claude', 'settings.json'), event).some((cmd) => cmd.includes(LEGACY_HOOK)); +} + async function readStdin() { const chunks = []; for await (const chunk of process.stdin) chunks.push(chunk); @@ -561,6 +595,7 @@ async function main() { if (!handler) return; // The harness's cwd follows a `cd` into a subdirectory, which has no `.zerograph/` of its own. const root = (payload.cwd && indexRoot(payload.cwd)) || payload.cwd || process.env.CLAUDE_PROJECT_DIR || process.cwd(); + if (legacyAnswers(event, root)) { debug('legacy hook answers', event); return; } // The subagent's own id is in the key: a subagent shares its parent's session_id, so a counter // keyed by session alone finds the count already past zero on a subagent's first call and says // nothing — which is a second reason 140 unprimed subagents went on grepping. diff --git a/agent/claude-code/scripts/hook.test.mjs b/agent/claude-code/scripts/hook.test.mjs index afa079a3..981375d6 100644 --- a/agent/claude-code/scripts/hook.test.mjs +++ b/agent/claude-code/scripts/hook.test.mjs @@ -3,7 +3,7 @@ // Run: `node --test agent/claude-code/scripts/hook.test.mjs`, on every platform the hook is installed on. import { test } from 'node:test'; import assert from 'node:assert'; -import { mkdtempSync, mkdirSync, writeFileSync, readFileSync, existsSync, chmodSync, copyFileSync, symlinkSync, statSync, realpathSync } from 'node:fs'; +import { mkdtempSync, mkdirSync, writeFileSync, readFileSync, existsSync, chmodSync, copyFileSync, symlinkSync, statSync, realpathSync, rmSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join, dirname, delimiter } from 'node:path'; import { fileURLToPath } from 'node:url'; @@ -58,12 +58,21 @@ function world({ store = true, answer = '', status = 0 } = {}) { return { root, bin, launcher, argvLog, budgetLog, cwdLog, pathLog }; } +/** + * The environment less what says which harness runs the hook and where its project is: a suite run + * from inside a Claude Code or Codex session would otherwise read that session's settings. + */ +function harnessless(env) { + const { CLAUDE_PROJECT_DIR, CLAUDE_PLUGIN_ROOT, PLUGIN_ROOT, CODEX_HOME, ...rest } = env; + return rest; +} + function fire(w, payload, env = {}, hook = HOOK) { const r = spawnSync(process.execPath, [hook], { input: JSON.stringify({ cwd: w.root, session_id: 'sess', ...payload }), encoding: 'utf8', env: { - ...process.env, + ...harnessless(process.env), PATH: `${w.bin}${delimiter}${process.env.PATH}`, // Windows resolves a bare name through `PATHEXT` only inside a shell, so `PATH` alone would // never find `zerograph.cmd`; the variable the hook already honours names it instead. @@ -535,6 +544,50 @@ test('every call the hook waits on refreshes in process for less than its timeou } }); +/** A checkout carrying what a 0.6.x `install-agent` committed: the script and its entries for `events`. */ +function legacyClaude(w, events) { + mkdirSync(join(w.root, '.claude', 'hooks'), { recursive: true }); + writeFileSync(join(w.root, '.claude', 'hooks', 'zerograph-hook.mjs'), '// 0.6.x\n'); + const entry = { hooks: [{ type: 'command', command: 'node "$CLAUDE_PROJECT_DIR/.claude/hooks/zerograph-hook.mjs"', timeout: 10 }] }; + const hooks = Object.fromEntries(events.map((e) => [e, [{ matcher: '.*', ...entry }]])); + writeFileSync(join(w.root, '.claude', 'settings.json'), JSON.stringify({ hooks })); +} + +test('a committed 0.6.x hook answers alone: the plugin is silent on the events it registers', () => { + const w = world({ answer: 'zerograph: 3076 doc nodes, 5240 code nodes' }); + legacyClaude(w, ['SessionStart', 'PreToolUse', 'PostToolUse']); + const env = { CLAUDE_PROJECT_DIR: w.root, ZEROGRAPH_HOOK_SERVE: '0' }; + assert.equal(fire(w, { hook_event_name: 'SessionStart', source: 'startup' }, env), null); + assert.equal(fire(w, { hook_event_name: 'PreToolUse', tool_name: 'Bash', tool_input: { command: 'rg -n cancellation' } }, env), null); + assert.deepEqual(argv(w), [], 'a silent hook asks the binary nothing either'); + assert.equal(context(fire(w, { hook_event_name: 'SubagentStart', agent_id: 'a1' }, env)), RULE, + 'an event the old entries never registered is still the plugin\'s'); +}); + +test('entries whose script is gone leave the plugin answering', () => { + const w = world({ answer: 'zerograph: 3076 doc nodes, 5240 code nodes' }); + legacyClaude(w, ['SessionStart']); + const env = { CLAUDE_PROJECT_DIR: w.root, ZEROGRAPH_HOOK_SERVE: '0' }; + rmSync(join(w.root, '.claude', 'hooks', 'zerograph-hook.mjs')); + assert.ok(context(fire(w, { hook_event_name: 'SessionStart', source: 'startup' }, env))?.startsWith('zerograph:')); +}); + +test('under Codex the machine hooks file decides, and a repository\'s .claude settings do not', () => { + const w = world({ answer: 'zerograph: 3076 doc nodes, 5240 code nodes' }); + legacyClaude(w, ['SessionStart']); + const home = mkdtempSync(join(tmpdir(), 'zerograph-codex-home-')); + const env = { PLUGIN_ROOT: join(home, 'plugin'), CODEX_HOME: home, ZEROGRAPH_HOOK_SERVE: '0' }; + assert.ok(context(fire(w, { hook_event_name: 'SessionStart', source: 'startup' }, env))?.startsWith('zerograph:'), + 'Codex never runs .claude/settings.json'); + + const script = join(w.root, '.codex', 'hooks', 'zerograph-hook.mjs'); + mkdirSync(dirname(script), { recursive: true }); + writeFileSync(script, '// 0.6.x\n'); + writeFileSync(join(home, 'hooks.json'), JSON.stringify({ hooks: { SessionStart: [{ matcher: 'startup', hooks: [{ type: 'command', command: `node '${script}'` }] }] } })); + assert.equal(fire(w, { hook_event_name: 'SessionStart', source: 'startup' }, env), null); + assert.equal(context(fire(w, { hook_event_name: 'SubagentStart', agent_id: 'a1' }, env)), RULE); +}); + test('SubagentStart carries the rule', () => { const w = world(); const c = context(fire(w, { hook_event_name: 'SubagentStart', agent_id: 'a1' })); diff --git a/agent/codex/scripts/hook.mjs b/agent/codex/scripts/hook.mjs index 6c37a918..cc04f38d 100644 --- a/agent/codex/scripts/hook.mjs +++ b/agent/codex/scripts/hook.mjs @@ -21,7 +21,7 @@ import { existsSync, readFileSync, writeFileSync, mkdirSync, realpathSync, lstat import { spawn, spawnSync } from 'node:child_process'; import { join, dirname, relative, isAbsolute } from 'node:path'; import { fileURLToPath } from 'node:url'; -import { tmpdir } from 'node:os'; +import { tmpdir, homedir } from 'node:os'; import { createHash } from 'node:crypto'; import { binaryOf, checkoutOf as repositoryOf, childEnv as cutEnv, commandOf, pathKey } from './launcher.mjs'; @@ -547,6 +547,40 @@ const handlers = { PostToolUse: (p, root, key) => (built(root) ? riskLine(p, root, key) : null), }; +const LEGACY_HOOK = 'zerograph-hook.mjs'; + +/** The commands a harness hooks file runs for `event`, or none when the file is absent or unreadable. */ +function hookCommands(file, event) { + let parsed; + try { parsed = JSON.parse(readFileSync(file, 'utf8')); } catch { return []; } + const groups = parsed?.hooks?.[event]; + if (!Array.isArray(groups)) return []; + return groups.flatMap((g) => (Array.isArray(g?.hooks) ? g.hooks : [])) + .map((h) => [h?.command, ...(Array.isArray(h?.args) ? h.args : [])].filter((w) => typeof w === 'string').join(' ')); +} + +/** + * Whether a hook an earlier `install-agent` wired answers `event` already, so this one stays + * silent rather than put the brief, the seeds and the risk line in context twice. In Claude Code + * that is a repository's committed `.claude/settings.json`, which `install-agent` leaves for the + * team to retire; in Codex it is the machine's `$CODEX_HOME/hooks.json`, which nothing a + * repository ships can touch. Either way the old script has to be there too: an entry whose + * script is gone answers nothing. + */ +function legacyAnswers(event, root) { + if (!process.env.CLAUDE_PLUGIN_ROOT && process.env.PLUGIN_ROOT) { + const home = process.env.CODEX_HOME || join(homedir(), '.codex'); + return hookCommands(join(home, 'hooks.json'), event).some((cmd) => { + const script = cmd.match(/(?:"([^"]*zerograph-hook\.mjs)"|'([^']*zerograph-hook\.mjs)'|(\S*zerograph-hook\.mjs))/); + const path = script && (script[1] ?? script[2] ?? script[3]); + return Boolean(path) && isAbsolute(path) && existsSync(path); + }); + } + const project = process.env.CLAUDE_PROJECT_DIR || root; + return existsSync(join(project, '.claude', 'hooks', LEGACY_HOOK)) + && hookCommands(join(project, '.claude', 'settings.json'), event).some((cmd) => cmd.includes(LEGACY_HOOK)); +} + async function readStdin() { const chunks = []; for await (const chunk of process.stdin) chunks.push(chunk); @@ -561,6 +595,7 @@ async function main() { if (!handler) return; // The harness's cwd follows a `cd` into a subdirectory, which has no `.zerograph/` of its own. const root = (payload.cwd && indexRoot(payload.cwd)) || payload.cwd || process.env.CLAUDE_PROJECT_DIR || process.cwd(); + if (legacyAnswers(event, root)) { debug('legacy hook answers', event); return; } // The subagent's own id is in the key: a subagent shares its parent's session_id, so a counter // keyed by session alone finds the count already past zero on a subagent's first call and says // nothing — which is a second reason 140 unprimed subagents went on grepping. diff --git a/docs/assets/comparison.gif b/docs/assets/comparison.gif index 842e8edbaa9c195dcd0605f71f2e274b697e2fa6..2767051871bba07439a71e110a29a0e593aeb8fd 100644 GIT binary patch delta 88 zcmX^1i|^Pkz6np5UNCQbcKS4s+PwLU+gcz!{Xiv?=yriB#$By&-oyWlcPoLs?LfKG YY?$zLpdz;IK~I@ZWq{=5{FpWJ0c->&+W-In delta 88 zcmX^1i|^Pkz6np5BA7NlJAE2RZQgvwZ7q2e#{#A0IjSd` wrote vectors, and the `serve` poll wrote the graph, without `writer.lock`, so a detached `update` could write the same files meanwhile and `status` reported no refresh. `embed` and `model ` now wait for a running @@ -145,6 +165,11 @@ links to. ## Before upgrading +- **The first `update` after the upgrade re-reads the whole tree, once.** The store's grammar moves + from 6 to 7 because a file is now redacted before it is read, and a store stamped 6 still holds + whatever credentials its unchanged files carried. Expect the first refresh to take as long as a + `build` minus the embedding, which the vector cache keeps for rows whose text did not move. + - **Set `ZEROGRAPH_BIN`** where zerograph is a project dependency and not on `PATH`, or the session hook and the mod find no binary. A `PATH` entry inside the checkout — `node_modules/.bin` added by direnv or an `.envrc` — is now skipped without a word, so a checkout that relied on one needs diff --git a/scripts/comparison-gif.py b/scripts/comparison-gif.py index 4b1ad318..caa93d48 100644 --- a/scripts/comparison-gif.py +++ b/scripts/comparison-gif.py @@ -122,7 +122,7 @@ def main(): canvas = Image.new("RGB", (s.size[0], h), BG) canvas.paste(s, (0, 0)) frames.append(canvas.quantize(colors=64, method=Image.MEDIANCUT)) - frames[0].save(OUT, save_all=True, append_images=frames[1:], duration=6000, loop=0, optimize=True) + frames[0].save(OUT, save_all=True, append_images=frames[1:], duration=10000, loop=0, optimize=True) print(OUT, frames[0].size, f"{os.path.getsize(OUT) // 1024} KiB", file=sys.stderr) diff --git a/scripts/npm-pack.sh b/scripts/npm-pack.sh index 1c08cfe9..86314450 100755 --- a/scripts/npm-pack.sh +++ b/scripts/npm-pack.sh @@ -49,10 +49,12 @@ add() { # The npm page shows the README packed with the version, so it is the repository's own, cut at # pack time: a hand-kept copy drifted from it. npm resolves no relative path, so each one is -# pinned to this version's tag — images to the raw file, links to the GitHub page. +# pinned to this version's tag — images to the raw file, links to the GitHub page. An HTML +# `src="…"`, as the logo is written, is an image like any other. repo=https://github.com/devmaxxx/zerograph sed -E \ -e "s#!\[([^]]*)\]\(([^):\#][^):]*)\)#![\1](https://raw.githubusercontent.com/devmaxxx/zerograph/v$version/\2)#g" \ + -e "s#src=\"([^\":\#/][^\":]*)\"#src=\"https://raw.githubusercontent.com/devmaxxx/zerograph/v$version/\1\"#g" \ -e "s#\]\(([^):\#][^):]*)\)#]($repo/blob/v$version/\1)#g" \ README.md >npm/zerograph/README.md diff --git a/src/config.rs b/src/config.rs index 7ccb343a..f4b4a98b 100644 --- a/src/config.rs +++ b/src/config.rs @@ -82,6 +82,11 @@ pub struct Config { /// must not be able to empty it or grow it past what the reader chose. #[serde(deserialize_with = "byte_size")] pub vector_cache_max: u64, + /// Whether provider credentials written inside a read file are replaced by `[REDACTED_SECRET]` + /// before the text reaches the store, an index or a rerank prompt. On by default; a project + /// that indexes fixtures of fake keys on purpose turns it off. `ZEROGRAPH_REDACT_SECRETS` + /// overrides it for one run. + pub redact_secrets: bool, } /// A size as a whole number of bytes or a string of digits with a binary unit — `K`, `M`, `G` or @@ -207,6 +212,7 @@ impl Default for Config { reader_budget: READER_BUDGET, mcp_reranks: MCP_RERANKS, vector_cache_max: crate::index::cache::DEFAULT_MAX, + redact_secrets: true, } } } @@ -369,6 +375,13 @@ impl Config { if let Some(v) = crate::env("READER_BUDGET").filter(|v| !v.trim().is_empty()) { cfg.reader_budget = v.trim().parse().with_context(|| format!("{}={v:?} is not a whole number of seconds", crate::env_name("READER_BUDGET")))?; } + if let Some(v) = crate::env("REDACT_SECRETS").filter(|v| !v.trim().is_empty()) { + cfg.redact_secrets = match v.trim().to_ascii_lowercase().as_str() { + "1" | "true" | "on" | "yes" => true, + "0" | "false" | "off" | "no" => false, + _ => anyhow::bail!("{}={v:?} is neither true nor false", crate::env_name("REDACT_SECRETS")), + }; + } cfg.rerank_command = rerank_template.replace(MODEL_SLOT, &cfg.rerank_model); if names_model { match trusted_spelling(&cfg.embed_model) { @@ -1027,7 +1040,7 @@ mod tests { } // Safety: the lock above makes this the only thread touching the environment. unsafe { - for k in ["CONFIG", "RERANK_MODEL", "RESOURCES", "READER_BUDGET", "CODE_GLOBS", "TEXT_GLOBS"] { + for k in ["CONFIG", "RERANK_MODEL", "RESOURCES", "READER_BUDGET", "CODE_GLOBS", "TEXT_GLOBS", "REDACT_SECRETS"] { std::env::remove_var(format!("ZEROGRAPH_{k}")); } std::env::set_var("ZEROGRAPH_CONFIG", &path); diff --git a/src/families.rs b/src/families.rs index 8964712b..92d2b6a0 100644 --- a/src/families.rs +++ b/src/families.rs @@ -235,8 +235,26 @@ impl Scan { /// The id-like prefixes the walked tree writes, with where each was first written. The only pass /// over the tree this module still makes, and `zerograph families` is its one caller. -pub fn survey(repo: &Path, entries: &[Entry]) -> Result> { +pub fn survey(repo: &Path, entries: &[Entry], redact_secrets: bool) -> Result> { let mut scan = Scan::default(); + // Each prefix prints the line it was first written on, so the text is redacted as the + // extractor redacts it: a key on the line that cites an id would otherwise reach the terminal. + let mut batch: Vec<(&Entry, String)> = Vec::new(); + let mut batch_bytes = 0; + let tally = |scan: &mut Scan, batch: &mut Vec<(&Entry, String)>| { + if batch.is_empty() { return; } + let redactor = match redact_secrets { + true => crate::secrets::Redactor::for_texts(batch.iter().map(|(_, t)| t.as_str())), + false => crate::secrets::Redactor::off(), + }; + for (e, text) in batch.drain(..) { + let (text, _) = redactor.redact(&e.rel, &text); + match e.kind { + FileKind::Doc => scan.doc(&e.rel, &text), + FileKind::Code | FileKind::Registry | FileKind::Text => scan.tally_lines(&e.rel, &text), + } + } + }; for e in entries { // A text node has no edge out of it, so what it says cites nothing a reader can follow, // and reading every one would make `families` pay for the whole tree. @@ -257,11 +275,14 @@ pub fn survey(repo: &Path, entries: &[Entry]) -> Result> { eprintln!("families: skipping {:?}: not UTF-8", e.rel); continue; }; - match e.kind { - FileKind::Doc => scan.doc(&e.rel, &text), - FileKind::Code | FileKind::Registry | FileKind::Text => scan.tally_lines(&e.rel, &text), + batch_bytes += text.len(); + batch.push((e, text)); + if batch_bytes >= crate::secrets::BATCH_BYTES { + tally(&mut scan, &mut batch); + batch_bytes = 0; } } + tally(&mut scan, &mut batch); Ok(scan.finish()) } @@ -362,7 +383,7 @@ pub fn run(repo: &Path, cfg: &crate::config::Config, json: bool) -> Result<()> { eprintln!("families: the store is {behind} file{} behind the tree — run `zerograph update`", if behind == 1 { "" } else { "s" }); } - let r = report(&graph, survey(repo, &entries)?); + let r = report(&graph, survey(repo, &entries, cfg.redact_secrets)?); match json { true => println!("{}", serde_json::to_string_pretty(&r)?), false => print!("{}", render(&r)), diff --git a/src/lib.rs b/src/lib.rs index 6756efe4..be89b03d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -230,7 +230,8 @@ enum Cmd { }, /// A stdio MCP server: `ask`, `explain`, `impact`, `trace` and `changes` as tools, plus /// `status` (what the index is behind by and how far a background refresh has got), `reindex`, - /// `cache_status` and `switch_model`. Protocol on stdout, diagnostics on stderr; `install-agent` registers it. + /// `cache_status` and `switch_model`. Protocol on stdout, diagnostics on stderr. The Claude Code and + /// Codex plugins start it; `install-agent` registers it for the other editors. Mcp, } @@ -264,6 +265,7 @@ pub(crate) struct Extractors { pub code: Box, pub registry: Box, pub text: Box, + pub redact_secrets: bool, } /// What `enrich` wrote before 0.6.0 removed it; named so the update that finds one can say so. @@ -280,8 +282,6 @@ pub(crate) struct UpdateReport { pub graph_at: Option, } -/// How much file text is held before it is scanned for secrets and extracted. -const REDACT_BATCH_BYTES: usize = 64 << 20; /// Re-extracts what the diff names, drops what is gone, and writes the store back. Nothing else /// is read: every citation a file holds was extracted when the file was read, whatever prefix it @@ -360,7 +360,10 @@ pub(crate) fn apply_diff(repo: &std::path::Path, store: &store::Store, graph: &m let mut batch_bytes = 0; let mut extract_batch = |batch: &mut Vec<(&walk::Entry, String)>| { if batch.is_empty() { return; } - let redactor = secrets::Redactor::for_texts(batch.iter().map(|(_, text)| text.as_str())); + let redactor = match ex.redact_secrets { + true => secrets::Redactor::for_texts(batch.iter().map(|(_, text)| text.as_str())), + false => secrets::Redactor::off(), + }; for (e, text) in batch.drain(..) { let (text, redacted) = redactor.redact(&e.rel, &text); if redacted > 0 { eprintln!("{}", log_line(&format!("{}: {redacted} secret(s) redacted", e.rel))); } @@ -386,7 +389,7 @@ pub(crate) fn apply_diff(repo: &std::path::Path, store: &store::Store, graph: &m }; batch_bytes += text.len(); batch.push((e, text)); - if batch_bytes >= REDACT_BATCH_BYTES { + if batch_bytes >= secrets::BATCH_BYTES { extract_batch(&mut batch); batch_bytes = 0; } @@ -661,6 +664,7 @@ pub(crate) fn extractors(repo: &std::path::Path, cfg: &config::Config) -> anyhow code: Box::new(code::CodeExtractor::new(resolver)), registry: Box::new(doc::registry::RegistryExtractor), text: Box::new(text::TextExtractor), + redact_secrets: cfg.redact_secrets, }) } diff --git a/src/secrets.rs b/src/secrets.rs index f752b6ec..5b592716 100644 --- a/src/secrets.rs +++ b/src/secrets.rs @@ -1,6 +1,12 @@ //! Credentials written inside a file that is read. `walk` refuses the files that exist to hold //! them (`.env`, `*.pem`); this catches the key pasted into a source file, a README or a shell //! script, before its text reaches `graph.json`, the lexical and dense indexes, or a rerank prompt. +//! +//! Coverage is by provider pattern: a token whose own shape names who issued it, a private key +//! block, a JWT, a database URL with a password in it. The catch-all rules that guess from the +//! word written beside a value are left out, so `password = "hunter2"` is not caught — on a real +//! repository they flagged 53 spans and none was a secret, while one of them cost a requirement +//! its citation. use aho_corasick::AhoCorasick; use secrets_scanner::{RedactionMode, ScanConfig, Scanner}; @@ -8,6 +14,10 @@ use std::sync::OnceLock; pub const MARKER: &str = "[REDACTED_SECRET]"; +/// How much file text is held before it is scanned for secrets: one scanner is compiled per batch, +/// and every text of a tree at once would be the largest allocation of a build. +pub const BATCH_BYTES: usize = 64 << 20; + /// The bundled rules, split so a scanner can be built from any subset of them. Compiling all of /// them costs 1.8 s, and the scanner already skips a rule whose keywords a text does not hold — /// so a scanner of only the rules a text's keywords name finds exactly what the full one does. @@ -26,7 +36,8 @@ fn catalog() -> &'static Catalog { // The bundled rules only: `Scanner::new` would also read `$SECRETS_SCANNER_RULES` and a // cache in the user's data directory, so what is redacted would vary by machine. let mut base: toml::Table = toml::from_str(secrets_scanner::rules::BUNDLED_RULES).expect("the bundled rules parse"); - let rules = match base.remove("rules") { Some(toml::Value::Array(r)) => r, _ => Vec::new() }; + let rules: Vec = match base.remove("rules") { Some(toml::Value::Array(r)) => r, _ => Vec::new() }; + let rules: Vec = rules.into_iter().filter(|r| r.get("id").and_then(|i| i.as_str()).is_some_and(|id| !is_catch_all(id))).collect(); let (mut words, mut rule_of_keyword, mut unkeyworded) = (Vec::new(), Vec::new(), Vec::new()); for (i, rule) in rules.iter().enumerate() { let listed: Vec<&str> = rule.get("keywords").and_then(|k| k.as_array()).into_iter().flatten().filter_map(|k| k.as_str()).collect(); @@ -38,6 +49,23 @@ fn catalog() -> &'static Catalog { }) } +/// Rules that match a value by the word beside it — `key`, `password`, `Authorization`, a URL's +/// `user:pass@` on any host — rather than by a shape a provider issues. On the bench fixture +/// they read `key: 'visit.cancel'` and a type parameter as keys, and one ran across lines into +/// the code below it. +fn is_catch_all(id: &str) -> bool { + const RULES: &[&str] = &[ + "generic-api-key", + "hashicorp-tf-password", + "curl-auth-header", + "curl-auth-user", + "kingfisher.credentials.1", + "kingfisher.uri.1", + ]; + const FAMILIES: &[&str] = &["kingfisher.generic.", "kingfisher.http.", "kingfisher.auth.", "kingfisher.curl."]; + RULES.contains(&id) || FAMILIES.iter().any(|f| id.starts_with(f)) +} + /// A scanner for one pass's texts, holding only the rules their keywords name. pub struct Redactor(Option); @@ -73,17 +101,31 @@ impl Redactor { let Some(scanner) = &self.0 else { return (text.to_string(), 0) }; redact_found(text, &scanner.scan_content_detailed(rel, text).findings) } + + /// A redactor that changes nothing, for a project whose `zerograph.toml` says + /// `redact_secrets = false`. + pub fn off() -> Redactor { Redactor(None) } } fn redact_found(text: &str, found: &[secrets_scanner::Finding]) -> (String, usize) { - let mut spans: Vec<(usize, usize)> = found.iter() - .map(|f| (f.secret_start_offset, f.secret_end_offset)) + redact_spans(text, found.iter().map(|f| (f.secret_start_offset, f.secret_end_offset)).collect()) +} + +fn redact_spans(text: &str, found: Vec<(usize, usize)>) -> (String, usize) { + let mut spans: Vec<(usize, usize)> = found.into_iter() .filter(|(s, e)| s < e && *e <= text.len()) .map(|(s, e)| (floor_boundary(text, s), ceil_boundary(text, e))) .collect(); if spans.is_empty() { return (text.to_string(), 0); } + // An id is what the graph is made of: a span holding one would take the node or the edge + // with it, and no provider issues a token shaped like `FR-CAL-155`. + let ids: Vec<(usize, usize)> = crate::ids::generic().find_all(text).into_iter().map(|h| (h.start, h.start + h.id.len())).collect(); + spans.retain(|&(s, e)| !ids.iter().any(|&(a, b)| a < e && s < b) && !local_or_placeholder(text, s, e)); + if spans.is_empty() { + return (text.to_string(), 0); + } spans.sort_unstable(); // Two rules often match one key: overlapping or touching spans become one marker. let mut merged: Vec<(usize, usize)> = Vec::with_capacity(spans.len()); @@ -106,6 +148,26 @@ fn redact_found(text: &str, found: &[secrets_scanner::Finding]) -> (String, usiz (out, count) } +/// Whether the span sits in a connection URL that opens nothing outside the reader's machine: a +/// loopback host, or a password that is an interpolation or a mask. CI files, compose files and +/// `.env.example` write these by the dozen, and every one of them is meant to be read. +fn local_or_placeholder(text: &str, start: usize, end: usize) -> bool { + let line_start = text[..start].rfind('\n').map_or(0, |i| i + 1); + let Some(scheme_end) = text[line_start..end].rfind("://").map(|i| line_start + i + 3) else { return false }; + let authority_len = text[scheme_end..].find(|c: char| c.is_whitespace() || matches!(c, '/' | '"' | '\'' | '`' | '?' | '#')).unwrap_or(text.len() - scheme_end); + let authority = &text[scheme_end..scheme_end + authority_len]; + if start >= scheme_end + authority_len { return false; } + let Some((userinfo, host)) = authority.rsplit_once('@') else { return false }; + let Some((_, password)) = userinfo.split_once(':') else { return false }; + let host = match host.strip_prefix('[') { + Some(v6) => v6.split(']').next().unwrap_or(""), + None => host.split(':').next().unwrap_or(""), + }; + let loopback = host.eq_ignore_ascii_case("localhost") || host.starts_with("127.") || host == "::1" || host == "0.0.0.0"; + let placeholder = password.starts_with(['$', '{', '<']) || password.chars().all(|c| c == '*' || c == '.'); + loopback || placeholder +} + fn floor_boundary(text: &str, mut i: usize) -> usize { while !text.is_char_boundary(i) { i -= 1; } i @@ -170,11 +232,58 @@ mod tests { assert_eq!(redact("src/main.rs", text), (text.to_string(), 0)); } + /// What the catch-all rules read as keys on the bench fixture, verbatim: none is a secret, and + /// the first cost `FR-CAL-155` its citation. + #[test] + fn the_catch_all_false_positives_of_the_fixture_come_back_unchanged() { + for (rel, text) in [ + ("AvailabilityRules.kt", " * Against `when.now`, which is the whole reason that key exists: `FR-CAL-155` makes this a pure\n"), + ("past-time.yaml", "# The comparison is against `when.now`, which is the whole reason that key exists: FR-CAL-155\n"), + ("rbac.repository.ts", " const scoped = await tx.query<{ key: PermissionKey }>(\n `SELECT key FROM permissions`,\n );\n"), + ("permissions.ts", " {\n key: 'visit.cancel',\n description: 'отменить визит',\n },\n"), + ("connection.spec.ts", concat!( + " const { username, password } = new URL(DEV_MIGRATOR_CONNECTION_STRING);\n", + " expect({ username, password }).toEqual({ username, password: roles.get(username) });\n", + " const { username, password } = new URL(DEV_SYSTEM_CONNECTION_STRING);\n", + )), + ("config.md", " password: process.env.DB_PASSWORD,\n password: config.get('database.password'),\n"), + ] { + assert_eq!(redact(rel, text), (text.to_string(), 0), "{rel}"); + } + } + + #[test] + fn a_span_holding_an_id_is_never_redacted() { + let text = format!("token {} FR-CAL-155\n", github_token()); + assert_eq!(super::redact_spans(&text, vec![(6, text.len() - 1)]), (text.clone(), 0)); + assert_eq!(super::redact_spans(&text, vec![(6, 6 + github_token().len())]).1, 1, "the same token without the id beside it goes"); + } + + #[test] + fn a_database_url_is_redacted_unless_it_opens_only_the_readers_own_machine() { + let password = "Xk29vLq7Rm4t"; + let remote = format!("DATABASE_URL=postgres://app:{password}@db.prod.example.net:5432/crm\n"); + let (out, n) = redact("deploy.sh", &remote); + assert_eq!(n, 1, "{out}"); + assert!(!out.contains(password), "{out}"); + for local in [ + "DATABASE_URL_SALON=postgres://salon_ctx:salon_ctx@localhost:5432/beauty_crm\n", + "url: 'postgres://app_system:app_system@127.0.0.1:5432/beauty_crm',\n", + " DATABASE_URL_SALON: postgres://salon_ctx:${SALON_CTX_PASSWORD:-salon_ctx}@postgres:5432/beauty_crm\n", + "db:migrate: chain applied on postgres://app_migrator:***@db.prod.example.net:55440/beauty_crm\n", + ] { + assert_eq!(redact("ci.yml", local), (local.to_string(), 0), "{local}"); + } + } + /// The subset is only an optimisation if it finds what the whole ruleset finds, on real text /// that names many providers' keywords: this repository's own sources and documents. #[test] fn a_scanner_of_the_rules_a_text_names_finds_what_the_whole_ruleset_finds() { - let full = secrets_scanner::Scanner::from_toml(secrets_scanner::rules::BUNDLED_RULES).expect("the whole ruleset passes the strict gate"); + let c = super::catalog(); + let mut whole = c.base.clone(); + whole.insert("rules".into(), toml::Value::Array(c.rules.clone())); + let full = secrets_scanner::Scanner::from_toml(&toml::to_string(&whole).unwrap()).expect("the provider ruleset passes the strict gate"); let root = std::path::Path::new(env!("CARGO_MANIFEST_DIR")); let spans = |f: Vec| { let mut s: Vec<_> = f.into_iter().map(|f| (f.rule_id, f.secret_start_offset, f.secret_end_offset)).collect(); s.sort(); s }; let mut paths: Vec<_> = ignore::Walk::new(root).flatten().map(|e| e.into_path()) diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 9ffcdcc8..42d710f1 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -26,6 +26,7 @@ pub fn run(repo: &Path, args: &[&str]) -> Output { .env_remove("ZEROGRAPH_CODE_GLOBS") .env_remove("ZEROGRAPH_TEXT_GLOBS") .env_remove("ZEROGRAPH_READER_BUDGET") + .env_remove("ZEROGRAPH_REDACT_SECRETS") .arg("--no-dense") .arg("--repo") .arg(repo) diff --git a/tests/redaction.rs b/tests/redaction.rs new file mode 100644 index 00000000..4b1abbc9 --- /dev/null +++ b/tests/redaction.rs @@ -0,0 +1,76 @@ +//! Credentials inside indexed files, end to end: what a build stores, what `families` prints, and +//! the switch a project turns redaction off with. + +mod common; + +use common::{run, write}; + +// Assembled at run time so this file holds no string a scanner of this repository flags. +fn token() -> String { format!("ghp_{}", "R7xK2mQ9vT4wL8nB3cF6hJ1pS5dG0aZyEuXo") } + +fn ops(token: &str) -> String { format!("# Ops\n\n**OPS-1 · MUST · Rotate {token} monthly**\n\nDeploy key for NEW-5: {token}\n") } + +fn out(repo: &std::path::Path, args: &[&str], env: &[(&str, &str)]) -> String { + let mut cmd = std::process::Command::new(env!("CARGO_BIN_EXE_zerograph")); + cmd.env_remove("ZEROGRAPH_REDACT_SECRETS").env_remove("ZEROGRAPH_EMBED_MODEL").env_remove("ZEROGRAPH_BENCH_REPO") + .env_remove("ZEROGRAPH_CODE_GLOBS").env_remove("ZEROGRAPH_TEXT_GLOBS").env_remove("ZEROGRAPH_READER_BUDGET"); + for (k, v) in env { cmd.env(k, v); } + let o = cmd.arg("--no-dense").arg("--repo").arg(repo).args(args).output().unwrap(); + let stdout = String::from_utf8_lossy(&o.stdout).into_owned(); + assert!(o.status.success(), "{args:?}: {stdout}{}", String::from_utf8_lossy(&o.stderr)); + stdout +} + +#[test] +fn a_comment_that_says_key_beside_a_requirement_keeps_its_reference() { + let dir = tempfile::tempdir().unwrap(); + let repo = dir.path(); + write(repo, "docs/cal.md", "# Calendar\n\n**FR-CAL-155 · MUST · Past slots are refused**\n"); + write(repo, "src/rules.ts", "export function rules() {\n /**\n * Against `when.now`, which is the whole reason that key exists: `FR-CAL-155` makes this a pure\n */\n return 1;\n}\n"); + write(repo, "src/rbac.ts", "export async function scoped(tx: Tx) {\n return tx.query<{ key: PermissionKey }>(`SELECT key FROM permissions`);\n}\n"); + let built = run(repo, &["build"]); + let err = String::from_utf8_lossy(&built.stderr); + assert!(built.status.success(), "{err}"); + assert!(!err.contains("redacted"), "nothing in these files is a secret: {err}"); + let explained = out(repo, &["explain", "FR-CAL-155"], &[]); + assert!(explained.contains("References ← sym:src/rules.ts::rules"), "{explained}"); +} + +#[test] +fn a_provider_token_is_redacted_unless_the_project_or_the_run_turns_it_off() { + let dir = tempfile::tempdir().unwrap(); + let repo = dir.path(); + write(repo, "docs/ops.md", &ops(&token())); + + out(repo, &["build"], &[]); + let explained = out(repo, &["explain", "OPS-1"], &[]); + assert!(explained.contains("Rotate [REDACTED_SECRET] monthly") && !explained.contains(&token()), "{explained}"); + + write(repo, "zerograph.toml", "redact_secrets = false\n"); + out(repo, &["build"], &[]); + let explained = out(repo, &["explain", "OPS-1"], &[]); + assert!(explained.contains(&format!("Rotate {} monthly", token())), "the project's switch: {explained}"); + + out(repo, &["build"], &[("ZEROGRAPH_REDACT_SECRETS", "true")]); + let explained = out(repo, &["explain", "OPS-1"], &[]); + assert!(!explained.contains(&token()), "the environment outranks the file for one run: {explained}"); + + let refused = std::process::Command::new(env!("CARGO_BIN_EXE_zerograph")) + .env("ZEROGRAPH_REDACT_SECRETS", "sometimes").arg("--no-dense").arg("--repo").arg(repo).arg("build").output().unwrap(); + assert!(!refused.status.success() && String::from_utf8_lossy(&refused.stderr).contains("ZEROGRAPH_REDACT_SECRETS"), "a value that is neither is refused by name"); +} + +#[test] +fn families_prints_its_example_line_redacted() { + let dir = tempfile::tempdir().unwrap(); + let repo = dir.path(); + write(repo, "docs/ops.md", &ops(&token())); + out(repo, &["build"], &[]); + + let report = out(repo, &["families"], &[]); + assert!(report.contains("Deploy key for NEW-5: [REDACTED_SECRET]"), "{report}"); + assert!(!report.contains("ghp_"), "{report}"); + + let report = out(repo, &["families"], &[("ZEROGRAPH_REDACT_SECRETS", "0")]); + assert!(report.contains("Deploy key for NEW-5: ghp_"), "and the switch reaches it too: {report}"); +} From 96045f9a7e9b8cb3689b1bc2a78545aa2863f0ca Mon Sep 17 00:00:00 2001 From: devmaxxx Date: Mon, 19 Oct 2026 14:58:56 +0200 Subject: [PATCH 2/2] fix: close the review findings on the audit fixes Codex sets CLAUDE_PLUGIN_ROOT as well as PLUGIN_ROOT, so the hook could not tell the harnesses apart and under Codex read Claude's legacy settings. The Codex plugin's hook command now passes --codex (main({harness:'codex'}) on Windows). A legacy entry silences the plugin only where its matcher matches the tool, the session source or the agent type, and a Codex entry spelled with ~, $HOME or ${HOME} is found. The loopback and placeholder exemption applies to the database URL rules only, so a provider token in a local URL is still redacted. The host is parsed as an address rather than matched by prefix, and a password counts as a placeholder only when the whole of it has that shape. cache_status is listed as a new tool and reindex as a change. --- agent/claude-code/scripts/hook.mjs | 49 ++++++++++---- agent/claude-code/scripts/hook.test.mjs | 52 ++++++++++++--- agent/codex/hooks/hooks.json | 6 +- agent/codex/plugin.test.mjs | 4 +- agent/codex/scripts/hook.mjs | 49 ++++++++++---- docs/configuration.md | 7 +- docs/releases/0.7.0.md | 10 +-- src/secrets.rs | 87 +++++++++++++++++++++---- 8 files changed, 210 insertions(+), 54 deletions(-) diff --git a/agent/claude-code/scripts/hook.mjs b/agent/claude-code/scripts/hook.mjs index cc04f38d..46095067 100644 --- a/agent/claude-code/scripts/hook.mjs +++ b/agent/claude-code/scripts/hook.mjs @@ -549,36 +549,61 @@ const handlers = { const LEGACY_HOOK = 'zerograph-hook.mjs'; -/** The commands a harness hooks file runs for `event`, or none when the file is absent or unreadable. */ -function hookCommands(file, event) { +/** + * What a group's `matcher` is tested against for `event`, as both harnesses do: the tool for the + * tool events, how the session began for `SessionStart`, the subagent's type for `SubagentStart`. + */ +function matchSubject(event, payload) { + if (event === 'PreToolUse' || event === 'PostToolUse') return payload.tool_name ?? ''; + if (event === 'SessionStart') return payload.source ?? ''; + if (event === 'SubagentStart') return payload.agent_type ?? ''; + return null; +} + +/** A matcher absent, empty or `*` matches everything; any other is a regex over the whole subject. */ +function matches(matcher, subject) { + if (subject === null || matcher === undefined || matcher === null || matcher === '' || matcher === '*') return true; + try { return new RegExp(`^(?:${matcher})$`).test(subject); } catch { return matcher === subject; } +} + +/** The commands a harness hooks file runs for this payload's event, or none when the file is absent or unreadable. */ +function hookCommands(file, event, payload) { let parsed; try { parsed = JSON.parse(readFileSync(file, 'utf8')); } catch { return []; } const groups = parsed?.hooks?.[event]; if (!Array.isArray(groups)) return []; - return groups.flatMap((g) => (Array.isArray(g?.hooks) ? g.hooks : [])) + const subject = matchSubject(event, payload); + return groups.filter((g) => matches(g?.matcher, subject)) + .flatMap((g) => (Array.isArray(g?.hooks) ? g.hooks : [])) .map((h) => [h?.command, ...(Array.isArray(h?.args) ? h.args : [])].filter((w) => typeof w === 'string').join(' ')); } +/** `~`, `$HOME` and `${HOME}` as the shell running a hook command would expand them. */ +function expandHome(path) { + return path.replace(/^(?:~|\$\{HOME\}|\$HOME)(?=\/|\\|$)/, homedir()); +} + /** - * Whether a hook an earlier `install-agent` wired answers `event` already, so this one stays + * Whether a hook an earlier `install-agent` wired answers this event already, so this one stays * silent rather than put the brief, the seeds and the risk line in context twice. In Claude Code * that is a repository's committed `.claude/settings.json`, which `install-agent` leaves for the * team to retire; in Codex it is the machine's `$CODEX_HOME/hooks.json`, which nothing a * repository ships can touch. Either way the old script has to be there too: an entry whose - * script is gone answers nothing. + * script is gone answers nothing. The harness is named by the plugin's own hook command, since + * Codex sets `CLAUDE_PLUGIN_ROOT` as well and no variable tells the two apart. */ -function legacyAnswers(event, root) { - if (!process.env.CLAUDE_PLUGIN_ROOT && process.env.PLUGIN_ROOT) { +function legacyAnswers(event, payload, root, harness) { + if (harness === 'codex') { const home = process.env.CODEX_HOME || join(homedir(), '.codex'); - return hookCommands(join(home, 'hooks.json'), event).some((cmd) => { + return hookCommands(join(home, 'hooks.json'), event, payload).some((cmd) => { const script = cmd.match(/(?:"([^"]*zerograph-hook\.mjs)"|'([^']*zerograph-hook\.mjs)'|(\S*zerograph-hook\.mjs))/); - const path = script && (script[1] ?? script[2] ?? script[3]); + const path = script && expandHome(script[1] ?? script[2] ?? script[3]); return Boolean(path) && isAbsolute(path) && existsSync(path); }); } const project = process.env.CLAUDE_PROJECT_DIR || root; return existsSync(join(project, '.claude', 'hooks', LEGACY_HOOK)) - && hookCommands(join(project, '.claude', 'settings.json'), event).some((cmd) => cmd.includes(LEGACY_HOOK)); + && hookCommands(join(project, '.claude', 'settings.json'), event, payload).some((cmd) => cmd.includes(LEGACY_HOOK)); } async function readStdin() { @@ -587,7 +612,7 @@ async function readStdin() { return Buffer.concat(chunks).toString('utf8'); } -async function main() { +async function main({ harness = process.argv.includes('--codex') ? 'codex' : 'claude' } = {}) { let payload; try { payload = JSON.parse(await readStdin()); } catch { return; } const event = payload.hook_event_name; @@ -595,7 +620,7 @@ async function main() { if (!handler) return; // The harness's cwd follows a `cd` into a subdirectory, which has no `.zerograph/` of its own. const root = (payload.cwd && indexRoot(payload.cwd)) || payload.cwd || process.env.CLAUDE_PROJECT_DIR || process.cwd(); - if (legacyAnswers(event, root)) { debug('legacy hook answers', event); return; } + if (legacyAnswers(event, payload, root, harness)) { debug('legacy hook answers', event); return; } // The subagent's own id is in the key: a subagent shares its parent's session_id, so a counter // keyed by session alone finds the count already past zero on a subagent's first call and says // nothing — which is a second reason 140 unprimed subagents went on grepping. diff --git a/agent/claude-code/scripts/hook.test.mjs b/agent/claude-code/scripts/hook.test.mjs index 981375d6..eb2dae8f 100644 --- a/agent/claude-code/scripts/hook.test.mjs +++ b/agent/claude-code/scripts/hook.test.mjs @@ -67,8 +67,8 @@ function harnessless(env) { return rest; } -function fire(w, payload, env = {}, hook = HOOK) { - const r = spawnSync(process.execPath, [hook], { +function fire(w, payload, env = {}, hook = HOOK, args = []) { + const r = spawnSync(process.execPath, [hook, ...args], { input: JSON.stringify({ cwd: w.root, session_id: 'sess', ...payload }), encoding: 'utf8', env: { @@ -545,11 +545,11 @@ test('every call the hook waits on refreshes in process for less than its timeou }); /** A checkout carrying what a 0.6.x `install-agent` committed: the script and its entries for `events`. */ -function legacyClaude(w, events) { +function legacyClaude(w, events, matchers = {}) { mkdirSync(join(w.root, '.claude', 'hooks'), { recursive: true }); writeFileSync(join(w.root, '.claude', 'hooks', 'zerograph-hook.mjs'), '// 0.6.x\n'); const entry = { hooks: [{ type: 'command', command: 'node "$CLAUDE_PROJECT_DIR/.claude/hooks/zerograph-hook.mjs"', timeout: 10 }] }; - const hooks = Object.fromEntries(events.map((e) => [e, [{ matcher: '.*', ...entry }]])); + const hooks = Object.fromEntries(events.map((e) => [e, [{ matcher: matchers[e] ?? '.*', ...entry }]])); writeFileSync(join(w.root, '.claude', 'settings.json'), JSON.stringify({ hooks })); } @@ -572,20 +572,54 @@ test('entries whose script is gone leave the plugin answering', () => { assert.ok(context(fire(w, { hook_event_name: 'SessionStart', source: 'startup' }, env))?.startsWith('zerograph:')); }); +test('a legacy entry silences the plugin only where its matcher matches', () => { + const w = world({ answer: 'zerograph: 3076 doc nodes, 5240 code nodes' }); + legacyClaude(w, ['SessionStart', 'PreToolUse'], { SessionStart: 'startup', PreToolUse: 'Grep|Glob|Agent' }); + const env = { CLAUDE_PROJECT_DIR: w.root, ZEROGRAPH_HOOK_SERVE: '0' }; + assert.equal(fire(w, { hook_event_name: 'SessionStart', source: 'startup' }, env), null); + assert.ok(context(fire(w, { hook_event_name: 'SessionStart', source: 'resume', session_id: 's2' }, env))?.startsWith('zerograph:'), + 'a resume the old entry does not match is the plugin\'s'); + assert.equal(fire(w, { hook_event_name: 'PreToolUse', tool_name: 'Grep', tool_input: { pattern: 'cancellation' } }, env), null); + const w2 = world({ answer: 'FR-PAY-22 docs/a.md:1 cancellation' }); + legacyClaude(w2, ['PreToolUse'], { PreToolUse: 'Grep|Glob|Agent' }); + fire(w2, { hook_event_name: 'PreToolUse', tool_name: 'Bash', tool_input: { command: 'rg -n cancellation' } }, { CLAUDE_PROJECT_DIR: w2.root, ZEROGRAPH_HOOK_SERVE: '0' }); + assert.ok(argv(w2).some((a) => a.includes('ask')), 'Bash is not in Grep|Glob|Agent, so the plugin asks'); +}); + +/** A machine `hooks.json` as 0.6.x printed it for Codex, running `script` as spelled. */ +function legacyCodex(home, spelled, matcher = 'startup|resume|clear|compact') { + writeFileSync(join(home, 'hooks.json'), JSON.stringify({ hooks: { SessionStart: [{ matcher, hooks: [{ type: 'command', command: `node '${spelled}'` }] }] } })); +} + test('under Codex the machine hooks file decides, and a repository\'s .claude settings do not', () => { const w = world({ answer: 'zerograph: 3076 doc nodes, 5240 code nodes' }); legacyClaude(w, ['SessionStart']); const home = mkdtempSync(join(tmpdir(), 'zerograph-codex-home-')); - const env = { PLUGIN_ROOT: join(home, 'plugin'), CODEX_HOME: home, ZEROGRAPH_HOOK_SERVE: '0' }; - assert.ok(context(fire(w, { hook_event_name: 'SessionStart', source: 'startup' }, env))?.startsWith('zerograph:'), + // What Codex really sets: both roots, so only the plugin's `--codex` argument names the harness. + const env = { PLUGIN_ROOT: join(home, 'plugin'), CLAUDE_PLUGIN_ROOT: join(home, 'plugin'), CODEX_HOME: home, ZEROGRAPH_HOOK_SERVE: '0' }; + const codex = (payload) => fire(w, payload, env, HOOK, ['--codex']); + assert.ok(context(codex({ hook_event_name: 'SessionStart', source: 'startup' }))?.startsWith('zerograph:'), 'Codex never runs .claude/settings.json'); const script = join(w.root, '.codex', 'hooks', 'zerograph-hook.mjs'); mkdirSync(dirname(script), { recursive: true }); writeFileSync(script, '// 0.6.x\n'); - writeFileSync(join(home, 'hooks.json'), JSON.stringify({ hooks: { SessionStart: [{ matcher: 'startup', hooks: [{ type: 'command', command: `node '${script}'` }] }] } })); - assert.equal(fire(w, { hook_event_name: 'SessionStart', source: 'startup' }, env), null); - assert.equal(context(fire(w, { hook_event_name: 'SubagentStart', agent_id: 'a1' }, env)), RULE); + legacyCodex(home, script); + assert.equal(codex({ hook_event_name: 'SessionStart', source: 'startup', session_id: 's2' }), null); + assert.equal(context(codex({ hook_event_name: 'SubagentStart', agent_id: 'a1' })), RULE); +}); + +test('under Codex a legacy entry spelled from the home directory is found', { skip: WIN && 'HOME is not the home directory there' }, () => { + const w = world({ answer: 'zerograph: 3076 doc nodes, 5240 code nodes' }); + const home = realpathSync(mkdtempSync(join(tmpdir(), 'zerograph-home-'))); + const codexHome = join(home, '.codex'); + mkdirSync(join(codexHome, 'hooks'), { recursive: true }); + writeFileSync(join(codexHome, 'hooks', 'zerograph-hook.mjs'), '// 0.6.x\n'); + const env = { HOME: home, PLUGIN_ROOT: join(home, 'plugin'), CLAUDE_PLUGIN_ROOT: join(home, 'plugin'), ZEROGRAPH_HOOK_SERVE: '0' }; + ['~/.codex/hooks/zerograph-hook.mjs', '$HOME/.codex/hooks/zerograph-hook.mjs', '${HOME}/.codex/hooks/zerograph-hook.mjs'].forEach((spelled, i) => { + legacyCodex(codexHome, spelled); + assert.equal(fire(w, { hook_event_name: 'SessionStart', source: 'startup', session_id: `s${i}` }, env, HOOK, ['--codex']), null, spelled); + }); }); test('SubagentStart carries the rule', () => { diff --git a/agent/codex/hooks/hooks.json b/agent/codex/hooks/hooks.json index 2e23c5b3..51dc9ab4 100644 --- a/agent/codex/hooks/hooks.json +++ b/agent/codex/hooks/hooks.json @@ -1,7 +1,7 @@ { "hooks": { - "SessionStart": [{ "matcher": "startup|resume|clear|compact", "hooks": [{ "type": "command", "command": "node \"${PLUGIN_ROOT}/scripts/hook.mjs\"", "commandWindows": "node -e \"import(require('url').pathToFileURL(require('path').join(process.env.PLUGIN_ROOT,'scripts','hook.mjs')).href).then(m=>m.main())\"", "timeout": 10 }] }], - "SubagentStart": [{ "hooks": [{ "type": "command", "command": "node \"${PLUGIN_ROOT}/scripts/hook.mjs\"", "commandWindows": "node -e \"import(require('url').pathToFileURL(require('path').join(process.env.PLUGIN_ROOT,'scripts','hook.mjs')).href).then(m=>m.main())\"", "timeout": 5 }] }], - "PostToolUse": [{ "matcher": "apply_patch|Edit|Write", "hooks": [{ "type": "command", "command": "node \"${PLUGIN_ROOT}/scripts/hook.mjs\"", "commandWindows": "node -e \"import(require('url').pathToFileURL(require('path').join(process.env.PLUGIN_ROOT,'scripts','hook.mjs')).href).then(m=>m.main())\"", "timeout": 10 }] }] + "SessionStart": [{ "matcher": "startup|resume|clear|compact", "hooks": [{ "type": "command", "command": "node \"${PLUGIN_ROOT}/scripts/hook.mjs\" --codex", "commandWindows": "node -e \"import(require('url').pathToFileURL(require('path').join(process.env.PLUGIN_ROOT,'scripts','hook.mjs')).href).then(m=>m.main({harness:'codex'}))\"", "timeout": 10 }] }], + "SubagentStart": [{ "hooks": [{ "type": "command", "command": "node \"${PLUGIN_ROOT}/scripts/hook.mjs\" --codex", "commandWindows": "node -e \"import(require('url').pathToFileURL(require('path').join(process.env.PLUGIN_ROOT,'scripts','hook.mjs')).href).then(m=>m.main({harness:'codex'}))\"", "timeout": 5 }] }], + "PostToolUse": [{ "matcher": "apply_patch|Edit|Write", "hooks": [{ "type": "command", "command": "node \"${PLUGIN_ROOT}/scripts/hook.mjs\" --codex", "commandWindows": "node -e \"import(require('url').pathToFileURL(require('path').join(process.env.PLUGIN_ROOT,'scripts','hook.mjs')).href).then(m=>m.main({harness:'codex'}))\"", "timeout": 10 }] }] } } diff --git a/agent/codex/plugin.test.mjs b/agent/codex/plugin.test.mjs index 4c994259..16e7e853 100644 --- a/agent/codex/plugin.test.mjs +++ b/agent/codex/plugin.test.mjs @@ -53,7 +53,9 @@ test('the hooks run the shipped hook, and the marketplace points at this directo assert.deepEqual(Object.keys(hooks).sort(), ['PostToolUse', 'SessionStart', 'SubagentStart']); for (const groups of Object.values(hooks)) { for (const h of groups.flatMap((g) => g.hooks)) { - assert.equal(h.command, 'node "${PLUGIN_ROOT}/scripts/hook.mjs"'); + // Codex sets CLAUDE_PLUGIN_ROOT too, so the command is what tells the hook which harness runs it. + assert.equal(h.command, 'node "${PLUGIN_ROOT}/scripts/hook.mjs" --codex'); + assert.match(h.commandWindows, /main\(\{harness:'codex'\}\)/); // `${PLUGIN_ROOT}` is a POSIX shell's expansion; the Windows form reads the variable in node. assert.doesNotMatch(h.commandWindows, /\$\{|%PLUGIN_ROOT%|\$env:/); assert.match(h.commandWindows, /process\.env\.PLUGIN_ROOT/); diff --git a/agent/codex/scripts/hook.mjs b/agent/codex/scripts/hook.mjs index cc04f38d..46095067 100644 --- a/agent/codex/scripts/hook.mjs +++ b/agent/codex/scripts/hook.mjs @@ -549,36 +549,61 @@ const handlers = { const LEGACY_HOOK = 'zerograph-hook.mjs'; -/** The commands a harness hooks file runs for `event`, or none when the file is absent or unreadable. */ -function hookCommands(file, event) { +/** + * What a group's `matcher` is tested against for `event`, as both harnesses do: the tool for the + * tool events, how the session began for `SessionStart`, the subagent's type for `SubagentStart`. + */ +function matchSubject(event, payload) { + if (event === 'PreToolUse' || event === 'PostToolUse') return payload.tool_name ?? ''; + if (event === 'SessionStart') return payload.source ?? ''; + if (event === 'SubagentStart') return payload.agent_type ?? ''; + return null; +} + +/** A matcher absent, empty or `*` matches everything; any other is a regex over the whole subject. */ +function matches(matcher, subject) { + if (subject === null || matcher === undefined || matcher === null || matcher === '' || matcher === '*') return true; + try { return new RegExp(`^(?:${matcher})$`).test(subject); } catch { return matcher === subject; } +} + +/** The commands a harness hooks file runs for this payload's event, or none when the file is absent or unreadable. */ +function hookCommands(file, event, payload) { let parsed; try { parsed = JSON.parse(readFileSync(file, 'utf8')); } catch { return []; } const groups = parsed?.hooks?.[event]; if (!Array.isArray(groups)) return []; - return groups.flatMap((g) => (Array.isArray(g?.hooks) ? g.hooks : [])) + const subject = matchSubject(event, payload); + return groups.filter((g) => matches(g?.matcher, subject)) + .flatMap((g) => (Array.isArray(g?.hooks) ? g.hooks : [])) .map((h) => [h?.command, ...(Array.isArray(h?.args) ? h.args : [])].filter((w) => typeof w === 'string').join(' ')); } +/** `~`, `$HOME` and `${HOME}` as the shell running a hook command would expand them. */ +function expandHome(path) { + return path.replace(/^(?:~|\$\{HOME\}|\$HOME)(?=\/|\\|$)/, homedir()); +} + /** - * Whether a hook an earlier `install-agent` wired answers `event` already, so this one stays + * Whether a hook an earlier `install-agent` wired answers this event already, so this one stays * silent rather than put the brief, the seeds and the risk line in context twice. In Claude Code * that is a repository's committed `.claude/settings.json`, which `install-agent` leaves for the * team to retire; in Codex it is the machine's `$CODEX_HOME/hooks.json`, which nothing a * repository ships can touch. Either way the old script has to be there too: an entry whose - * script is gone answers nothing. + * script is gone answers nothing. The harness is named by the plugin's own hook command, since + * Codex sets `CLAUDE_PLUGIN_ROOT` as well and no variable tells the two apart. */ -function legacyAnswers(event, root) { - if (!process.env.CLAUDE_PLUGIN_ROOT && process.env.PLUGIN_ROOT) { +function legacyAnswers(event, payload, root, harness) { + if (harness === 'codex') { const home = process.env.CODEX_HOME || join(homedir(), '.codex'); - return hookCommands(join(home, 'hooks.json'), event).some((cmd) => { + return hookCommands(join(home, 'hooks.json'), event, payload).some((cmd) => { const script = cmd.match(/(?:"([^"]*zerograph-hook\.mjs)"|'([^']*zerograph-hook\.mjs)'|(\S*zerograph-hook\.mjs))/); - const path = script && (script[1] ?? script[2] ?? script[3]); + const path = script && expandHome(script[1] ?? script[2] ?? script[3]); return Boolean(path) && isAbsolute(path) && existsSync(path); }); } const project = process.env.CLAUDE_PROJECT_DIR || root; return existsSync(join(project, '.claude', 'hooks', LEGACY_HOOK)) - && hookCommands(join(project, '.claude', 'settings.json'), event).some((cmd) => cmd.includes(LEGACY_HOOK)); + && hookCommands(join(project, '.claude', 'settings.json'), event, payload).some((cmd) => cmd.includes(LEGACY_HOOK)); } async function readStdin() { @@ -587,7 +612,7 @@ async function readStdin() { return Buffer.concat(chunks).toString('utf8'); } -async function main() { +async function main({ harness = process.argv.includes('--codex') ? 'codex' : 'claude' } = {}) { let payload; try { payload = JSON.parse(await readStdin()); } catch { return; } const event = payload.hook_event_name; @@ -595,7 +620,7 @@ async function main() { if (!handler) return; // The harness's cwd follows a `cd` into a subdirectory, which has no `.zerograph/` of its own. const root = (payload.cwd && indexRoot(payload.cwd)) || payload.cwd || process.env.CLAUDE_PROJECT_DIR || process.cwd(); - if (legacyAnswers(event, root)) { debug('legacy hook answers', event); return; } + if (legacyAnswers(event, payload, root, harness)) { debug('legacy hook answers', event); return; } // The subagent's own id is in the key: a subagent shares its parent's session_id, so a counter // keyed by session alone finds the count already past zero on a subagent's first call and says // nothing — which is a second reason 140 unprimed subagents went on grepping. diff --git a/docs/configuration.md b/docs/configuration.md index 5196a4e7..6fe7e6db 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -53,8 +53,11 @@ an `https://` URL are **not** caught: on a real repository they flagged dozens o a secret, among them a type parameter, a permission name and a comment whose requirement id lost its citation with it. Nothing that reads as an id (`FR-CAL-155`, `ADR-024`) is redacted, whatever rule matched it. A database URL whose host is `localhost` or a loopback address, or whose password -is an interpolation (`${DB_PASSWORD:-dev}`) or a mask (`***`), is left as written: it opens -nothing outside the reader's own machine, and CI files and compose files write them by the dozen. +is wholly an interpolation (`${DB_PASSWORD:-dev}`, `$DB_PASSWORD`, `%DB_PASSWORD%`), a +placeholder (``) or a mask (`***`, `xxx`), is left as written: it opens nothing outside +the reader's own machine, and CI files and compose files write them by the dozen. That holds for +the database URL rules only: a provider token used as the password, or in any other URL on +`localhost`, is still redacted by the provider's own rule. An inline `gitleaks:allow` does not exempt one; the rules' own path allowlist does — images, fonts, lockfiles, `vendor/` and `node_modules/` trees are not scanned. A run compiles only the rules whose keywords the diff --git a/docs/releases/0.7.0.md b/docs/releases/0.7.0.md index 6c4e4171..495c7428 100644 --- a/docs/releases/0.7.0.md +++ b/docs/releases/0.7.0.md @@ -92,12 +92,12 @@ links to. and several hundred more): a generic `password = "..."`, `api_key: "..."` or `Bearer` header is not caught, because the catch-all rules that would catch it flagged dozens of spans on a real repository and none of them was a secret. A span that holds an id is never redacted, and a - database URL on `localhost` or with an interpolated password is left as written. + database URL on a loopback host or with a whole placeholder password (`${VAR}`, `$VAR`, ``, + `***`) is left as written, unless the password is itself a provider token. `redact_secrets = false` in `zerograph.toml`, or `ZEROGRAPH_REDACT_SECRETS=false` for one run, turns it off ([configuration](../configuration.md#credentials-inside-files)). -- **Two MCP tools moved.** `cache_status` reports the machine's vector cache as `zerograph cache - status` does, and `reindex` starts a detached `build` where no store exists instead of failing - ([MCP](../mcp.md)). +- **A `cache_status` MCP tool** reports the machine's vector cache as `zerograph cache status` does: + where it is, its cap, and its rows and bytes per model ([MCP](../mcp.md)). - **A store records the release that wrote it.** `manifest.json` carries `release`, `status` shows it, and a store a newer release wrote has a `newer_grammar` state of its own, which the Claude Code mod displays ([commands](../commands.md)). @@ -112,6 +112,8 @@ links to. ([configuration](../configuration.md)). The session hook resolves the same root, so inside a git checkout it asks about the top level whatever subdirectory the session is in ([keeping fresh](../keeping-fresh.md#the-session-hook)). +- **The `reindex` MCP tool builds a checkout that has no store** with a detached `build`, where it + used to fail; with a store it starts a detached `update` as before ([MCP](../mcp.md)). - **The Claude Code mod reads files instead of polling `status --json`.** Every 10 s it reads `background.log` and stats the socket the last report named; the full report runs at session start, every 5 minutes, once after a refresh ends and after `/zerograph` diff --git a/src/secrets.rs b/src/secrets.rs index 5b592716..537e8a25 100644 --- a/src/secrets.rs +++ b/src/secrets.rs @@ -108,13 +108,26 @@ impl Redactor { } fn redact_found(text: &str, found: &[secrets_scanner::Finding]) -> (String, usize) { - redact_spans(text, found.iter().map(|f| (f.secret_start_offset, f.secret_end_offset)).collect()) + redact_spans(text, found.iter().map(|f| (f.secret_start_offset, f.secret_end_offset, is_database_url(&f.rule_id))).collect()) } -fn redact_spans(text: &str, found: Vec<(usize, usize)>) -> (String, usize) { - let mut spans: Vec<(usize, usize)> = found.into_iter() - .filter(|(s, e)| s < e && *e <= text.len()) - .map(|(s, e)| (floor_boundary(text, s), ceil_boundary(text, e))) +/// Rules that match a database connection URL by its scheme, whatever the password in it. Only +/// these may be let through for a local host or a placeholder password: a provider's token in +/// the same place is still matched by its own rule, and that span is never let through. +fn is_database_url(id: &str) -> bool { + const FAMILIES: &[&str] = &[ + "kingfisher.postgres.", "kingfisher.mysql.", "kingfisher.mariadb.", "kingfisher.mongodb.", "kingfisher.redis.", + "kingfisher.rabbitmq.", "kingfisher.jdbc.", "kingfisher.mssql.", "kingfisher.db2.", "kingfisher.odbc.", + ]; + const RULES: &[&str] = &["mongodb-atlas-db-uri-with-credentials", "snowflake-postgres-connection-string"]; + RULES.contains(&id) || FAMILIES.iter().any(|f| id.starts_with(f)) +} + +/// Spans as `(start, end, from a database URL rule)`. +fn redact_spans(text: &str, found: Vec<(usize, usize, bool)>) -> (String, usize) { + let mut spans: Vec<(usize, usize, bool)> = found.into_iter() + .filter(|(s, e, _)| s < e && *e <= text.len()) + .map(|(s, e, db)| (floor_boundary(text, s), ceil_boundary(text, e), db)) .collect(); if spans.is_empty() { return (text.to_string(), 0); @@ -122,7 +135,8 @@ fn redact_spans(text: &str, found: Vec<(usize, usize)>) -> (String, usize) { // An id is what the graph is made of: a span holding one would take the node or the edge // with it, and no provider issues a token shaped like `FR-CAL-155`. let ids: Vec<(usize, usize)> = crate::ids::generic().find_all(text).into_iter().map(|h| (h.start, h.start + h.id.len())).collect(); - spans.retain(|&(s, e)| !ids.iter().any(|&(a, b)| a < e && s < b) && !local_or_placeholder(text, s, e)); + spans.retain(|&(s, e, db)| !ids.iter().any(|&(a, b)| a < e && s < b) && !(db && local_or_placeholder(text, s, e))); + let mut spans: Vec<(usize, usize)> = spans.into_iter().map(|(s, e, _)| (s, e)).collect(); if spans.is_empty() { return (text.to_string(), 0); } @@ -163,9 +177,24 @@ fn local_or_placeholder(text: &str, start: usize, end: usize) -> bool { Some(v6) => v6.split(']').next().unwrap_or(""), None => host.split(':').next().unwrap_or(""), }; - let loopback = host.eq_ignore_ascii_case("localhost") || host.starts_with("127.") || host == "::1" || host == "0.0.0.0"; - let placeholder = password.starts_with(['$', '{', '<']) || password.chars().all(|c| c == '*' || c == '.'); - loopback || placeholder + let loopback = host.eq_ignore_ascii_case("localhost") + || host.parse::().is_ok_and(|ip| ip.is_loopback() || ip.is_unspecified()); + loopback || placeholder(password) +} + +/// A password that names where the real one comes from, or masks it, rather than being one. The +/// whole password has to be the shape: `$Xk29…` is a password that happens to start with `$`. +fn placeholder(password: &str) -> bool { + static SHAPES: OnceLock = OnceLock::new(); + let shapes = SHAPES.get_or_init(|| regex::Regex::new(concat!( + r"^(?:\$\{[^{}\s]+\}", // ${VAR}, ${VAR:-default} + r"|\$[A-Z_][A-Z0-9_]*", // $VAR, upper case as environment variables are named + r"|%[A-Za-z_][A-Za-z0-9_]*%", // %VAR% + r"|\{\{[^{}]+\}\}", // {{ template }} + r"|<[^<>]+>", // + r"|\*{3,}|[xX]{3,}|\.{3,})$", // ***, xxx, ... + )).expect("the placeholder shapes compile")); + shapes.is_match(password) } fn floor_boundary(text: &str, mut i: usize) -> usize { @@ -255,8 +284,8 @@ mod tests { #[test] fn a_span_holding_an_id_is_never_redacted() { let text = format!("token {} FR-CAL-155\n", github_token()); - assert_eq!(super::redact_spans(&text, vec![(6, text.len() - 1)]), (text.clone(), 0)); - assert_eq!(super::redact_spans(&text, vec![(6, 6 + github_token().len())]).1, 1, "the same token without the id beside it goes"); + assert_eq!(super::redact_spans(&text, vec![(6, text.len() - 1, false)]), (text.clone(), 0)); + assert_eq!(super::redact_spans(&text, vec![(6, 6 + github_token().len(), false)]).1, 1, "the same token without the id beside it goes"); } #[test] @@ -276,6 +305,42 @@ mod tests { } } + #[test] + fn a_provider_token_in_a_local_or_masked_url_is_still_redacted() { + for text in [ + format!("git clone https://x-access-token:{}@localhost:3000/org/repo.git\n", github_token()), + format!("BILLING_URL=https://api:{}@127.0.0.1:8080/v1/charges\n", aws_key()), + format!("DATABASE_URL=postgres://app:{}@localhost:5432/crm\n", github_token()), + ] { + let (out, n) = redact("ops.sh", &text); + assert!(n >= 1 && !out.contains(&github_token()) && !out.contains(&aws_key()), "{out}"); + } + } + + #[test] + fn only_a_real_loopback_host_or_a_whole_placeholder_lets_a_database_url_through() { + let scheme = format!("{}://", "postgres"); + for (url, kept) in [ + (format!("{scheme}app:Xk29vLq7Rm4t@127.0.0.1:5432/crm"), true), + (format!("{scheme}app:Xk29vLq7Rm4t@127.8.9.10:5432/crm"), true), + (format!("{scheme}app:Xk29vLq7Rm4t@[::1]:5432/crm"), true), + (format!("{scheme}app:Xk29vLq7Rm4t@LOCALHOST:5432/crm"), true), + (format!("{scheme}app:Xk29vLq7Rm4t@127.attacker.example.net:5432/crm"), false), + (format!("{scheme}app:Xk29vLq7Rm4t@localhost.example.net:5432/crm"), false), + (format!("{scheme}app:$DB_PASSWORD@db.prod.example.net:5432/crm"), true), + (format!("{scheme}app:%DB_PASSWORD%@db.prod.example.net:5432/crm"), true), + (format!("{scheme}app:@db.prod.example.net:5432/crm"), true), + (format!("{scheme}app:xxxx@db.prod.example.net:5432/crm"), true), + (format!("{scheme}app:$Xk29vLq7Rm4t@db.prod.example.net:5432/crm"), false), + (format!("{scheme}app:{{Xk29vLq7Rm4t@db.prod.example.net:5432/crm"), false), + (format!("{scheme}app:***Xk29vLq7Rm4t@db.prod.example.net:5432/crm"), false), + ] { + let text = format!("DATABASE_URL={url}\n"); + let (out, n) = redact("deploy.sh", &text); + assert_eq!(n == 0, kept, "{url} -> {out}"); + } + } + /// The subset is only an optimisation if it finds what the whole ruleset finds, on real text /// that names many providers' keywords: this repository's own sources and documents. #[test]