diff --git a/agent/claude-code/scripts/hook.mjs b/agent/claude-code/scripts/hook.mjs index 6c37a918..46095067 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,13 +547,72 @@ const handlers = { PostToolUse: (p, root, key) => (built(root) ? riskLine(p, root, key) : null), }; +const LEGACY_HOOK = 'zerograph-hook.mjs'; + +/** + * 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 []; + 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 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. 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, payload, root, harness) { + if (harness === 'codex') { + const home = process.env.CODEX_HOME || join(homedir(), '.codex'); + 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 && 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, payload).some((cmd) => cmd.includes(LEGACY_HOOK)); +} + async function readStdin() { const chunks = []; for await (const chunk of process.stdin) chunks.push(chunk); 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; @@ -561,6 +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, 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 afa079a3..eb2dae8f 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 }; } -function fire(w, payload, env = {}, hook = HOOK) { - const r = spawnSync(process.execPath, [hook], { +/** + * 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, args = []) { + const r = spawnSync(process.execPath, [hook, ...args], { 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,84 @@ 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, 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: matchers[e] ?? '.*', ...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('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-')); + // 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'); + 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', () => { const w = world(); const c = context(fire(w, { hook_event_name: 'SubagentStart', agent_id: 'a1' })); 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 6c37a918..46095067 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,13 +547,72 @@ const handlers = { PostToolUse: (p, root, key) => (built(root) ? riskLine(p, root, key) : null), }; +const LEGACY_HOOK = 'zerograph-hook.mjs'; + +/** + * 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 []; + 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 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. 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, payload, root, harness) { + if (harness === 'codex') { + const home = process.env.CODEX_HOME || join(homedir(), '.codex'); + 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 && 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, payload).some((cmd) => cmd.includes(LEGACY_HOOK)); +} + async function readStdin() { const chunks = []; for await (const chunk of process.stdin) chunks.push(chunk); 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; @@ -561,6 +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, 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/assets/comparison.gif b/docs/assets/comparison.gif index 842e8edb..27670518 100644 Binary files a/docs/assets/comparison.gif and b/docs/assets/comparison.gif differ diff --git a/docs/configuration.md b/docs/configuration.md index 5777f41d..6fe7e6db 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -21,6 +21,7 @@ in full, not an empty config: | `vector_cache_max` | `"4G"` — bytes the machine's vector cache may hold before the least recently used rows go; a whole number of bytes or `K`/`M`/`G`/`T`; `0` turns the cache off. Machine file only: the cache is every checkout's, see [The vector cache](keeping-fresh.md#the-vector-cache). `ZEROGRAPH_VECTOR_CACHE` moves the cache to another absolute directory | | `reader_budget` | `10` — seconds a reader may spend refreshing before it answers from the store as it stands and leaves the rest to a detached `update`; `0` never refreshes inline. The machine file may set it; `ZEROGRAPH_READER_BUDGET` overrides, see [Keeping it fresh](keeping-fresh.md#keeping-it-fresh) | | `resources` | `"balanced"` = 40% of the logical cores; `"low"` a sixth, `"full"` a half — how much of the machine a run may take, see [Resources](configuration.md#resources) | +| `redact_secrets` | `true` — provider credentials inside a read file become `[REDACTED_SECRET]` before extraction; `false` keeps the text as written. `ZEROGRAPH_REDACT_SECRETS` (`true`/`false`, `1`/`0`) overrides it for one run, see [Credentials inside files](configuration.md#credentials-inside-files) | `ZEROGRAPH_CODE_GLOBS`, whitespace-separated, replaces `code_globs` for one run. It is a measurement's switch, as `ZEROGRAPH_EMBED_MODEL` is, so a reading can name other globs without writing a @@ -34,15 +35,39 @@ keys and keystores, `.npmrc`, `.pypirc`, `.netrc`, `.git-credentials`, cloud cre `*.tfvars`, `*.tfstate` and `secrets.{yaml,json,toml,…}` — their content would otherwise reach the store, a `--rerank` prompt sent to a model, and the answers. +### Credentials inside files + A credential written inside a file that is read — a token in a source file, a key exported in a shell script, a private key pasted into a README — is replaced with `[REDACTED_SECRET]` before the file is extracted, by the bundled rules of [secrets_scanner](https://crates.io/crates/secrets_scanner) (gitleaks-compatible). The rest of the -file is still read and keeps its line numbers; `update` logs how many were redacted in which file. +file is still read and keeps its line numbers; `update` logs how many were redacted in which file, +and `zerograph families` prints its example lines through the same redaction. + +Coverage is by provider pattern: a token whose own shape says who issued it (GitHub, GitLab, AWS, +Slack, Stripe, OpenAI, Anthropic, Google, npm, PyPI, SendGrid, Twilio, Hugging Face, Discord and +several hundred more), a JWT, a private key block, and a database connection URL with a password +in it. The catch-all rules that judge a value by the word written beside it are left out, so +`password = "hunter2"`, `api_key: "…"`, an `Authorization: Bearer …` header or a `user:pass@` in +an `https://` URL are **not** caught: on a real repository they flagged dozens of spans, none of them +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 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 -files it re-reads contain — a few dozen for a one-file update, tens of milliseconds, where all ~1,000 -would cost 1.8 s. +files it re-reads contain — a few dozen for a one-file update, tens of milliseconds, where all of them +would cost over a second. + +`redact_secrets = false` turns redaction off for a project that indexes fake keys on purpose — a +scanner's own fixtures, say. It applies to files read from then on: `zerograph build` re-reads the +whole tree under the new setting. `ZEROGRAPH_REDACT_SECRETS=false` (or `true`) overrides the file +for one run. The store writes its own `.gitignore` holding `*`, so `.zerograph/` stays out of a commit in a repository that never listed it. diff --git a/docs/install.md b/docs/install.md index 017da42a..85702a49 100644 --- a/docs/install.md +++ b/docs/install.md @@ -67,7 +67,9 @@ A hook script goes only after the entries that run it are gone. A hook script, s somebody edited, a link, or a directory with files of its own is left and named. A file git tracks is left too, and listed in one line: it is the team's, and serves every teammate who has no plugin yet. Once all of them have it, `zerograph install-agent --claude --retire-repo` (or `--codex ---retire-repo`) takes those out as well, for a commit of their own. Both take the `post-merge`/`post-checkout` block an earlier release wrote out of the git +--retire-repo`) takes those out as well, for a commit of their own. Until then the plugin's hook +stays silent on every event the committed entries in `.claude/settings.json` still answer — in +Codex, the entries left in `$CODEX_HOME/hooks.json` — so a session hears one brief, not two. Both take the `post-merge`/`post-checkout` block an earlier release wrote out of the git hooks, deleting a hook that held nothing else. The install runs only once the plugin installed: when the `claude` or `codex` CLI is missing or a step fails, it prints the commands to run by hand and leaves the older install in place. diff --git a/docs/releases/0.7.0.md b/docs/releases/0.7.0.md index 5a1f1cfa..495c7428 100644 --- a/docs/releases/0.7.0.md +++ b/docs/releases/0.7.0.md @@ -83,6 +83,21 @@ links to. - **A Codex plugin**, `agent/codex/`: the MCP server, the session hook on `SessionStart`, `SubagentStart` and after an edit, and the commands as skills, `$zerograph:ask` to `$zerograph:refresh` ([install](../install.md#codex-plugin)). +- **Credentials written inside an indexed file are redacted.** A provider token in a source file, a + key exported in a shell script, a private key pasted into a README or a database URL with a + password in it becomes `[REDACTED_SECRET]` before the file is extracted, so it never reaches the + store, an index, a rerank prompt or `zerograph families`; the file keeps its line numbers and + `update` names the file. Coverage is by provider pattern only (GitHub, GitLab, AWS, Slack, Stripe, + OpenAI, Anthropic, Google, npm, PyPI, SendGrid, Twilio, Hugging Face, Discord, JWTs, private keys + 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 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)). +- **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)). @@ -97,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` @@ -125,6 +142,11 @@ links to. naming both generations; `update`, `build`, `embed` and the model switch refuse to write. `ask` reads the manifest again before it writes, whether or not it could take the writer lock, so a newer `update` that saves in between is not stamped over ([commands](../commands.md)). +- **A checkout that still commits the 0.6.x hook hears one brief, not two.** `install-agent` leaves + a `.claude/settings.json` git tracks for the team, and its entries ran the old hook beside the + plugin's, so every session start and every search got two answers. The plugin's hook is now + silent on each event those entries still answer, and the same for entries left in + `$CODEX_HOME/hooks.json` ([install](../install.md#upgrading)). - **Every writer takes the writer lock.** `embed` and `model ` 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 +167,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..537e8a25 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,45 @@ 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)) - .filter(|(s, e)| s < e && *e <= text.len()) - .map(|(s, e)| (floor_boundary(text, s), ceil_boundary(text, e))) + redact_spans(text, found.iter().map(|f| (f.secret_start_offset, f.secret_end_offset, is_database_url(&f.rule_id))).collect()) +} + +/// 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); } + // 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, 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); + } 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 +162,41 @@ 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.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 { while !text.is_char_boundary(i) { i -= 1; } i @@ -170,11 +261,94 @@ 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, 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] + 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}"); + } + } + + #[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] 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}"); +}