diff --git a/TODO.md b/TODO.md index 7c3a74a..545cd3b 100644 --- a/TODO.md +++ b/TODO.md @@ -1,3 +1,3 @@ # Todo -- preallow: generate the credential rules from [Betterleaks](https://github.com/betterleaks/betterleaks) instead of gitleaks, which now takes only security fixes, once Betterleaks 2 is released. Its `sourcegraph-access-token` rule no longer matches any 40-character hex string, so outputs holding both a git commit hash and the word `sourcegraph` stop being withheld. Most of its rules filter a match with an Expr expression and some score it by tokens (`tokenRatio`), so `scripts/gitleaks.ts` has to turn those into JavaScript or leave such rules out, and the keyword budget test still has to pass. +- preallow: generate the credential rules from [Betterleaks](https://github.com/betterleaks/betterleaks) instead of gitleaks, which now takes only security fixes, once Betterleaks 2 is released. Most of its rules filter a match with an Expr expression and some score it by tokens (`tokenRatio`), so `scripts/gitleaks.ts` has to turn those into JavaScript or leave such rules out, and the keyword budget test still has to pass. diff --git a/plugins/preallow/.claude-plugin/plugin.json b/plugins/preallow/.claude-plugin/plugin.json index 164a3ae..72714ef 100644 --- a/plugins/preallow/.claude-plugin/plugin.json +++ b/plugins/preallow/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "preallow", - "version": "0.0.9", + "version": "0.0.10", "description": "Lets Claude Code auto mode run the steps of the task you gave without the classifier: a model judge allows what is reversible or what you explicitly asked for", "author": { "name": "Entwining" diff --git a/plugins/preallow/AGENTS.md b/plugins/preallow/AGENTS.md index b7b7a8a..4b6ba6c 100644 --- a/plugins/preallow/AGENTS.md +++ b/plugins/preallow/AGENTS.md @@ -4,7 +4,7 @@ The README states what the plugin allows and what it leaves to native approval; The plugin only loosens. `tool.check` turns the core's `ask` into `allow` or returns the core's answer unchanged; it never touches an `allow`, a `deny`, or an ask whose result names a decider other than the classifier (`OWNS_ASK` in `hooks/register.ts`), or whose input names a host as `approver`. Preserve this ownership boundary when the engine adds metadata. An unknown field leaves the ask to the judge, since sending every ask with an unknown field to the classifier would bring back the false denials the plugin exists to remove. A permission hook that throws or overruns its budget is skipped and the core's answer stands. -One judge for every tool it is asked about: no per-tool or per-command policy branches or allowlists in its policy or around it, and the policy text names no project, command, or wording taken from real data. A tool whose verdict the engine keeps whatever a hook answers (`RESERVED_TOOLS` in `hooks/register.ts`) never reaches the judge, since a judge call there spends seconds on an allow the engine rejects or ignores; add a tool there only for that engine behavior, as a `rejected` log line or the engine's documentation shows it. A Bash, PowerShell, or command-monitor call requesting a non-empty `allowed_domains` list also stays with the classifier, whose approval registers the network grants. Nor does a call whose input asks to run in the background, since its output arrives where the withholding scan cannot see it; a tool whose output arrives later without such a flag (`Monitor`) is a known gap in the README until the engine signals it. +One judge for every tool it is asked about: no per-tool or per-command policy branches or allowlists in its policy or around it, and the policy text names no project, command, or wording taken from real data. A tool whose verdict the engine keeps whatever a hook answers (`RESERVED_TOOLS` in `hooks/register.ts`) never reaches the judge, since a judge call there spends seconds on an allow the engine rejects or ignores; add a tool there only for that engine behavior, as a `rejected` log line or the engine's documentation shows it. A Bash, PowerShell, or command-monitor call requesting a non-empty `allowed_domains` list also stays with the classifier, whose approval registers the network grants. Nor does a call whose input asks to run in the background, since its output arrives where the output scan cannot see it; a tool whose output arrives later without such a flag (`Monitor`) is a known gap in the README until the engine signals it. `JUDGE_SYSTEM` (`hooks/policy.ts`) decides what is allowed. Check a change to it or to either stage's default model or effort in `hooks/register.ts` against real denials kept outside this repository, on a split not used while writing the change, and compare allow and review rates before and after. @@ -18,6 +18,6 @@ Only a reply that ends with a JSON object answering `allow` allows: text may com `judge` (`hooks/judge.ts`) stays one stage, since the evaluation outside the engine calls it once per stage; the second stage lives in `judgeCall` (`hooks/stages.ts`). The `model`, `effort`, `escalation_model`, and `escalation_effort` settings reach `$.model.complete` as the user wrote them: no aliases, option lists, or validation. -A failed output scan withholds the output of a call the plugin allowed; the call has already run, so recovery must not execute it again. Every pattern in `hooks/withhold.ts` stays linear in the output's length, and `scripts/gitleaks.ts` strips the name gitleaks matches before a value under a keyword, which backtracks for seconds over a crafted megabyte, so a scan ends within the budget instead of withholding large outputs; the test that scans every rule's keyword fails when it comes back. A pattern needs a key's body, not only the words that name its format, and `scripts/gitleaks.ts` appends a token body to a gitleaks rule that is only a literal prefix, since the plugin's own files name every format; `bun run test:sources` fails when reading one of them would be withheld. +The output scan answers a call the plugin allowed with a new `{ result }` holding the redacted record and no `ref`, since core reuses the messages a `ref` names and the model would read the original, or, after an error, with a deny holding the redacted error text. Core keeps whole the context a hook below attached, so a credential there withholds the output. A redacted call stays in `allowedCalls` for the rest of the session, because the engine refuses an answer only after the hook has returned it, then skips the hook and lets the unredacted result stand unless `.catch` withholds it; `claude plugin test` does not validate a hook's result against the tool's schema, so no test reaches that refusal. A failed scan withholds the output too; the call has already run, so recovery must not execute it again. Every pattern in `hooks/withhold.ts` stays linear in the output's length, and `scripts/gitleaks.ts` strips the name gitleaks matches before a value under a keyword, which backtracks for seconds over a crafted megabyte, so a scan ends within the budget instead of withholding large outputs; the test that scans every rule's keyword fails when it comes back. A pattern needs a key's body, not only the words that name its format, and `scripts/gitleaks.ts` appends a token body to a gitleaks rule that is only a literal prefix, since the plugin's own files name every format; `bun run test:sources` fails when reading one of them would be redacted. `hooks/gitleaks.generated.ts` is written by `bun run gitleaks` (`scripts/gitleaks.ts`, which pins the gitleaks release and rewrites its Go regexes for JavaScript); change the script and regenerate instead of editing the file. diff --git a/plugins/preallow/CHANGELOG.md b/plugins/preallow/CHANGELOG.md index 9430ade..f964695 100644 --- a/plugins/preallow/CHANGELOG.md +++ b/plugins/preallow/CHANGELOG.md @@ -1,6 +1,13 @@ # Changelog -## 0.0.9 - unreleased +## 0.0.10 - 2026-10-29 + +- An output holding something credential-shaped has each such value replaced with `[redacted by preallow: ]`, wherever it appears in the result, instead of being withheld whole, so the agent keeps the rest of it; a note beside the result tells it what was replaced. The whole output is still withheld when the full output a large result leaves in a file holds a credential, when a note another hook adds beside the result holds one, or when the text the agent reads shows one the tool's result does not. A credential the scan does not know now reaches the agent even beside one it found. The log records `redacted` with the kinds it replaced. +- The output scan also reads the tool's result, not only the text the agent reads, so a credential that line numbers or JSON escaping hide from its patterns, such as a password in a netrc file the agent reads with Read, is found. +- An empty assignment such as `API_KEY=` is no longer taken for a credential whose value is the name on the next line, as in an `.env.example` file. +- A git commit hash in an output that names Sourcegraph is no longer taken for a Sourcegraph access token; a token without the `sgp_` prefix is still found under a name such as `SRC_ACCESS_TOKEN`. + +## 0.0.9 - 2026-10-29 - The judge runs on `haiku` at `high` effort instead of `opus` at `medium`, and a second judge on `opus` at `medium` judges again, from the start, every call the first did not allow, including one it gave no verdict on; the second judge's verdict is final. The new `escalation_model` and `escalation_effort` settings choose the second judge; if you set `model` and leave `escalation_model` empty, there is none. Each `judged` line in the log keeps the first judge's fields and gives the second judge's in `escalation`. - The judge's instructions are rewritten as decisions taken in order: leaks first, then whether the call serves your task and respects what you ruled out, then work on your own repositories, then whether each effect can be undone, and last whether your words explicitly cover the irreversible ones. They describe each field the judge reads and each fact a check returns, including what a fact does not establish, such as file times saying nothing about who made a file. The wording was chosen by evaluating it against real calls. diff --git a/plugins/preallow/README.md b/plugins/preallow/README.md index 6ad2e7c..96bf9b7 100644 --- a/plugins/preallow/README.md +++ b/plugins/preallow/README.md @@ -33,7 +33,7 @@ Everything else is allowed, including commands outside the sandbox, loosening pe - An `ask` from a settings rule, from a hook in your settings or another plugin, from a setting's own check (the read block, `permissions.blockReadsOutsideWorkingDirectories`, which asks you rather than the classifier), from an organization's ceiling for the tool, or one put to a host that answers the session's asks. - A call run in the background (`run_in_background`), such as a dev server or a watcher: its output reaches the model after the call returns, where the plugin cannot scan it. - A Bash, PowerShell, or command-monitor call with a non-empty `allowed_domains` list: the classifier reviews the command and registers approved network grants; a hook allow approves the command without granting the hosts. -- A call whose output the plugin withheld, or whose allow the engine rejected, when the same tool runs again with the same input. +- A call whose output the plugin redacted or withheld, or whose allow the engine rejected, when the same tool runs again with the same input. - A call auto mode denied after the judge left it to review, when the agent makes it again with the same input, in the same directory and, for a subagent, with the same brief, before you write again. Judging it again could only repeat the review or undo the denial with nothing new from you; once you have written, the call is judged again with your new words. - A tool whose verdict the engine keeps whatever a hook answers: a subagent's report to its parent (`SubagentHandback`), which only the classifier may allow, and a question to you or a plan for your approval (`AskUserQuestion`, `ExitPlanMode`), which wait for you. The judge is not asked, since its allow would be rejected or ignored. @@ -76,11 +76,13 @@ The verdict is the JSON object that ends the judge's reply: text may come before The policy and your kept messages open every judge's prompt and are marked for the prompt cache, so a call soon after another, and the call after a check, can read them from the cache until you write again; the `usage` in each `judged` line shows what was read and written. -## Output it withholds +## Output it redacts -The judge decides before the call runs and cannot see what it will print. When what the model would read from a call the plugin allowed, including the full output a large result leaves in a file, a result another plugin's hook gave in the tool's place, and notes a hook adds beside the result, holds what looks like a credential (a private key, a token in a provider format that [gitleaks](https://github.com/gitleaks/gitleaks) knows or one starting `sk-`, a JSON web token, a netrc or registry login, a session cookie, or a password or token in a URL, header, or assignment), the plugin withholds it. The result is also withheld when the scan itself fails or runs out of time. +The judge decides before the call runs and cannot see what it will print. When what the model would read from a call the plugin allowed, including a result another plugin's hook gave in the tool's place, holds what looks like a credential (a private key, a token in a provider format that [gitleaks](https://github.com/gitleaks/gitleaks) knows or one starting `sk-`, a JSON web token, a netrc or registry login, a session cookie, or a password or token in a URL, header, or assignment), the plugin replaces each such value with `[redacted by preallow: ]`, wherever in the result the same value appears, and tells the agent beside the result. It scans the tool's result as well as the text the agent reads, so line numbers in a file the agent reads do not hide a credential from it. A failed call's error text is redacted the same way. -The call has already been made, and withholding its output does not undo it: the agent is warned that running it again repeats its effects, and the same call run again goes to the classifier. A call that moves to the background has its hand-off scanned in the same way, including the output returned before the move; a withheld hand-off warns that withholding did not stop the task, and gives its scanned task ID to follow, or asks you to inspect `/tasks` when that ID cannot be read safely. +The plugin withholds the whole output instead when it cannot redact it: when the full output a large result leaves in a file holds a credential, since the agent can read that file later; when a note a hook adds beside the result holds one, since a hook cannot change another's notes; when the text the agent reads shows a credential that the tool's result, from which Claude Code renders that text, does not hold; and when the scan fails or runs out of time, or Claude Code refuses the redacted result. + +The call has already been made, and redacting or withholding its output does not undo it: the agent is warned that running it again repeats its effects, and the same call run again goes to the classifier. A call that moves to the background has its hand-off scanned in the same way, including the output returned before the move; a withheld hand-off warns that withholding did not stop the task, and gives its scanned task ID to follow, or asks you to inspect `/tasks` when that ID cannot be read safely. ## Reading the log @@ -88,7 +90,7 @@ The plugin logs one JSON line per event to `~/.claude/preallow/.json To find calls the judge should not have allowed, read the `judged` lines whose deciding verdict is `allow`: `escalation.verdict` where the line has an `escalation`, `verdict` otherwise. -The events; all but `rejected`, `denied`, and `withheld` also give why the core asked, in `coreReason`: +The events; all but `rejected`, `denied`, `redacted`, and `withheld` also give why the core asked, in `coreReason`: - `judged`: a call the judge was asked about, or whose input was too long to ask it about, with the tool, its input (up to 2,000 characters), and the first judge's fields listed below. When a second judge ran, `escalation` holds the same fields for it, and its verdict decided the call. - `owned`: an `ask` its decider owns, left to it without asking the judge (see [What it leaves to native approval](#what-it-leaves-to-native-approval)), with the field that named the decider in `ownedBy` and its value. @@ -98,6 +100,7 @@ The events; all but `rejected`, `denied`, and `withheld` also give why the core - `repeated`: a call auto mode denied, made again unchanged before you wrote again, left to the classifier without asking the judge. - `rejected`: the engine rejected the plugin's allow of a call because only the classifier may allow that tool, with the start of its message in `reason`. The call failed, and the same call run again goes to the classifier; that tool belongs with the reserved ones, so report it. - `denied`: auto mode denied a call the judge left to review, with the start of its reason in `reason`; auto mode can deny without a classifier verdict. The matching `judged` line says why the judge did not allow it. +- `redacted`: the plugin redacted the output of a call it allowed, with the kinds of value it replaced in `kinds` (never the values). - `withheld`: the plugin withheld the output of a call it allowed, and why in `reason` (never the output). A judge's fields in a `judged` line: @@ -116,12 +119,14 @@ A judge's fields in a `judged` line: - On Bedrock, Vertex, Foundry, or behind a gateway, Claude Code finds cached text again only while the prompt opens the same way, and the judge's prompt opens with your kept messages, so after each message you write the judge's next call writes its instructions to the cache again along with your messages. - A conversation forked into a new session, or one you last wrote in before the 50 most recent, starts with none of your earlier messages, so the judge may allow a call your earlier words ruled out. - `/rewind` does not take your messages back from the judge: the plugin learns of no rewind, so a request you rewound still counts as yours. To withdraw one, say so in a new message. -- A call that only reads, such as printing a log, listing processes, showing proxy variables, or capturing a terminal pane, does not show that its output will hold a secret, so the judge allows it as it would any read; keeping the secret out of the conversation is left to the [output scan](#output-it-withholds), with the limits below. -- The credential scan knows a fixed list of formats (`hooks/withhold.ts` and gitleaks' provider rules); a credential in any other format, such as a password of letters only, or a value under a name the list does not know, slips through, and so do a token holding the word `false`, which gitleaks' allowlist reads as a boolean, and a value in a Kubernetes Secret that follows an empty one. JSON escaping can also hide a quoted password's shape. A private key slips through when more than about 200 characters of headers come before its body, or when a body wrapped narrower than 40 characters has indented or blank lines. -- Some text that is not a credential is taken for one, such as a private key's armor around 40 or more letters and digits that are not its body, a placeholder spread over lines for example, or, under gitleaks' rules, almost any 40-character hex string, a git commit hash included, in an output that also holds the word `sourcegraph`; the output is withheld, and the same call run again goes to the classifier. +- A call that only reads, such as printing a log, listing processes, showing proxy variables, or capturing a terminal pane, does not show that its output will hold a secret, so the judge allows it as it would any read; keeping the secret out of the conversation is left to the [output scan](#output-it-redacts), with the limits below. +- The credential scan knows a fixed list of formats (`hooks/withhold.ts` and gitleaks' provider rules); a credential in any other format, such as a password of letters only, or a value under a name the list does not know, slips through, even in an output where the scan redacted another, and so do a token holding the word `false`, which gitleaks' allowlist reads as a boolean, and a value in a Kubernetes Secret that follows an empty one. JSON escaping can also hide a quoted password's shape. A private key slips through when more than about 200 characters of headers come before its body, or when a body wrapped narrower than 40 characters has indented or blank lines. A key whose lines start with something else, such as line numbers, keeps its lines after the first when no end marker follows within about 10,000 characters, as when its output was cut off. +- Some text that is not a credential is taken for one, such as a private key's armor around 40 or more letters and digits that are not its body, a placeholder spread over lines for example, or the output between a key cut off before its end marker and another key's end marker within about 10,000 characters; it is redacted wherever its text appears, even inside a longer word, and the same call run again goes to the classifier. +- A private key wrapped narrower than 16 characters a line, in a result another plugin's hook gave in the tool's place, is withheld instead of redacted. +- A large result whose full output in its file passes 4 MiB is withheld, since the plugin cannot read a file that large to scan it. - For a call moved to the background while it runs, the plugin scans only the hand-off. Output delivered later, including task notifications, is outside that scan; a later read is scanned only if the plugin allows that call. - A `Monitor` call starts a watch whose events reach the model later, one line at a time; the judge may allow it, and the plugin does not scan those events. -- A call the plugin allowed runs again under the classifier after a withheld output only when its input is the same; a rerun with any input change is judged again. +- A call the plugin allowed runs again under the classifier after a redacted or withheld output only when its input is the same; a rerun with any input change is judged again. ## License diff --git a/plugins/preallow/hooks/gitleaks.generated.ts b/plugins/preallow/hooks/gitleaks.generated.ts index fce141d..ebfb7d1 100644 --- a/plugins/preallow/hooks/gitleaks.generated.ts +++ b/plugins/preallow/hooks/gitleaks.generated.ts @@ -1034,7 +1034,7 @@ export const rules = [ { id: 'jwt-base64', regex: - '\\bZXlK(?:(?aGJHY2lPaU)|(?aGNIVWlPaU)|(?aGNIWWlPaU)|(?aGRXUWlPaU)|(?aU5qUWlP)|(?amNtbDBJanBi)|(?amRIa2lPaU)|(?bGNHc2lPbn)|(?bGJtTWlPaU)|(?cWEzVWlPaU)|(?cWQyc2lPb)|(?cGMzTWlPaU)|(?cGRpSTZJ)|(?cmFXUWlP)|(?clpYbGZiM0J6SWpwY)|(?cmRIa2lPaUp)|(?dWIyNWpaU0k2)|(?d01tTWlP)|(?d01uTWlPaU)|(?d2NIUWlPaU)|(?emRXSWlPaU)|(?emRuUWlP)|(?MFlXY2lPaU)|(?MGVYQWlPaUp)|(?MWNtd2l)|(?MWMyVWlPaUp)|(?MlpYSWlPaU)|(?MlpYSnphVzl1SWpv)|(?NElqb2)|(?NE5XTWlP)|(?NE5YUWlPaU)|(?NE5YUWpVekkxTmlJNkl)|(?NE5YVWlPaU)|(?NmFYQWlPaU))[a-zA-Z0-9\\/\\\\_+\\-\\r\\n]{40,}={0,2}', + '\\bZXlK(?:(?:aGJHY2lPaU)|(?:aGNIVWlPaU)|(?:aGNIWWlPaU)|(?:aGRXUWlPaU)|(?:aU5qUWlP)|(?:amNtbDBJanBi)|(?:amRIa2lPaU)|(?:bGNHc2lPbn)|(?:bGJtTWlPaU)|(?:cWEzVWlPaU)|(?:cWQyc2lPb)|(?:cGMzTWlPaU)|(?:cGRpSTZJ)|(?:cmFXUWlP)|(?:clpYbGZiM0J6SWpwY)|(?:cmRIa2lPaUp)|(?:dWIyNWpaU0k2)|(?:d01tTWlP)|(?:d01uTWlPaU)|(?:d2NIUWlPaU)|(?:emRXSWlPaU)|(?:emRuUWlP)|(?:MFlXY2lPaU)|(?:MGVYQWlPaUp)|(?:MWNtd2l)|(?:MWMyVWlPaUp)|(?:MlpYSWlPaU)|(?:MlpYSnphVzl1SWpv)|(?:NElqb2)|(?:NE5XTWlP)|(?:NE5YUWlPaU)|(?:NE5YUWpVekkxTmlJNkl)|(?:NE5YVWlPaU)|(?:NmFYQWlPaU))[a-zA-Z0-9\\/\\\\_+\\-\\r\\n]{40,}={0,2}', keywords: ['zxlk'], entropy: 2, secretGroup: 0, @@ -1252,7 +1252,7 @@ export const rules = [ { id: 'microsoft-teams-webhook', regex: - 'https://[a-z0-9]+\\.webhook\\.office\\.com/webhookb2/[a-z0-9]{8}-([a-z0-9]{4}-){3}[a-z0-9]{12}@[a-z0-9]{8}-([a-z0-9]{4}-){3}[a-z0-9]{12}/IncomingWebhook/[a-z0-9]{32}/[a-z0-9]{8}-([a-z0-9]{4}-){3}[a-z0-9]{12}', + 'https://[a-z0-9]+\\.webhook\\.office\\.com/webhookb2/[a-z0-9]{8}-(?:[a-z0-9]{4}-){3}[a-z0-9]{12}@[a-z0-9]{8}-(?:[a-z0-9]{4}-){3}[a-z0-9]{12}/IncomingWebhook/[a-z0-9]{32}/[a-z0-9]{8}-(?:[a-z0-9]{4}-){3}[a-z0-9]{12}', keywords: ['webhook.office.com', 'webhookb2', 'incomingwebhook'], entropy: 0, secretGroup: 0, @@ -1738,8 +1738,8 @@ export const rules = [ { id: 'sourcegraph-access-token', regex: - '(?i:\\b(\\b(sgp_(?:[a-fA-F0-9]{16}|local)_[a-fA-F0-9]{40}|sgp_[a-fA-F0-9]{40}|[a-fA-F0-9]{40})\\b)(?![\\w-]))', - keywords: ['sgp_', 'sourcegraph'], + '(?i:\\b(\\b(sgp_(?:[a-fA-F0-9]{16}|local)_[a-fA-F0-9]{40}|sgp_[a-fA-F0-9]{40})\\b)(?![\\w-]))', + keywords: ['sgp_'], entropy: 3, secretGroup: 0, allowlists: [], diff --git a/plugins/preallow/hooks/gitleaks.ts b/plugins/preallow/hooks/gitleaks.ts index 355ea3d..8b91d3f 100644 --- a/plugins/preallow/hooks/gitleaks.ts +++ b/plugins/preallow/hooks/gitleaks.ts @@ -12,7 +12,7 @@ const compile = (list: Allowlist) => ({ ...list, regexes: list.regexes.map((r) = const allowlist = compile(ruleset.allowlist); const rules = ruleset.rules.map((rule) => ({ ...rule, - regex: new RegExp(rule.regex, 'g'), + regex: new RegExp(rule.regex, 'dg'), allowlists: [allowlist, ...rule.allowlists.map(compile)], })); @@ -25,24 +25,29 @@ function entropy(text: string): number { return bits; } -// The id of the first rule that finds a secret in the text. -export function findLeak(text: string): string | undefined { +// A credential-shaped value's place in a text, and its kind: the name of a +// shape in `withhold.ts`, or the id of a gitleaks rule. +export type Finding = { kind: string; start: number; end: number }; + +export function findLeaks(text: string): Finding[] { const lower = text.toLowerCase(); + const found: Finding[] = []; for (const rule of rules) { if (!rule.keywords.some((k) => lower.includes(k))) continue; for (const match of text.matchAll(rule.regex)) { // The secret is the rule's secret group, else the first group that // matched, as gitleaks takes it. - const secret = - (rule.secretGroup > 0 ? match[rule.secretGroup] : match.slice(1).find(Boolean)) ?? match[0]; + const group = rule.secretGroup > 0 ? rule.secretGroup : match.findIndex((g, i) => i > 0 && g); + const [start, end] = match.indices![group] ?? match.indices![0]!; + const secret = text.slice(start, end); if (rule.entropy > 0 && entropy(secret) <= rule.entropy) continue; const allowed = rule.allowlists.some( (list) => list.regexes.some((r) => r.test(list.regexTarget === 'match' ? match[0] : secret)) || list.stopwords.some((word) => secret.toLowerCase().includes(word)), ); - if (!allowed) return rule.id; + if (!allowed) found.push({ kind: rule.id, start, end }); } } - return undefined; + return found; } diff --git a/plugins/preallow/hooks/register.ts b/plugins/preallow/hooks/register.ts index 2d9cf56..f2853e6 100644 --- a/plugins/preallow/hooks/register.ts +++ b/plugins/preallow/hooks/register.ts @@ -5,7 +5,7 @@ import type { Modes } from '../types'; import { judgeCase, judgePrompt } from './judge.ts'; import { judgeCall, type JudgeSettings } from './stages.ts'; import { loggedInput, record } from './log.ts'; -import { rerunWarning, WITHHELD, withholding } from './withhold.ts'; +import { rerunWarning, screen, WITHHELD } from './withhold.ts'; import { ANSWER_PREFIX, remember } from './messages.ts'; const modesRef = atom({ plugin: 'preallow', key: 'modes' } as const, {} as Modes); @@ -58,10 +58,10 @@ export const register: Register = (on, options) => { }; // Calls the plugin allowed, by tool_use_id, so their output is scanned // before the model reads it; calls left to the core when they run again, - // after their output was withheld or the engine rejected the plugin's allow; - // the calls the judge left to review, by tool_use_id, with a digest of the - // judge's first prompt, so a denial of one is logged; and the digests of - // those auto mode denied. + // after their output was redacted or withheld or the engine rejected the + // plugin's allow; the calls the judge left to review, by tool_use_id, with + // a digest of the judge's first prompt, so a denial of one is logged; and + // the digests of those auto mode denied. const allowedCalls = new Map(); const coreCalls = new Set(); const reviewCalls = new Map(); @@ -194,9 +194,9 @@ export const register: Register = (on, options) => { return result; }); - // A call the plugin allowed has its output withheld when it holds a - // credential-shaped value, since the classifier never saw the call; run - // again, it goes to the classifier. + // A call the plugin allowed has each credential-shaped value in its output + // redacted, or the output withheld, since the classifier never saw the + // call; run again, it goes to the classifier. on('tool.call', async ($, e, next) => { const result = await next(e); const id = e.tool_use_id; @@ -211,28 +211,41 @@ export const register: Register = (on, options) => { return result; } - const why = await withholding($, result.text, result.result, result.context); - const warning = why === undefined ? undefined : rerunWarning(result.result); // Still listed while the scan runs, so `.catch` withholds the output if // the scan fails or runs out of time. - allowedCalls.delete(id); - if (why === undefined) return result; - record($, { event: 'withheld', tool_use_id: id, reason: why }); + const screened = await screen($, result); + if (screened === undefined) { + allowedCalls.delete(id); + return result; + } coreCalls.add(key); + if ('redacted' in screened) { + record($, { event: 'redacted', tool_use_id: id, kinds: screened.kinds }); + // A redacted result stays listed: the engine skips a hook whose answer + // it refuses and lets the unredacted result stand, unless `.catch` + // withholds it. + return screened.redacted; + } + allowedCalls.delete(id); + record($, { event: 'withheld', tool_use_id: id, reason: screened.withheld }); // After a call that answered without error the engine already tells the // model that it ran and a plugin withheld its result. const prefix = result.isError === true ? WITHHELD : 'preallow:'; - return { deny: `${prefix} ${why}. ${warning}` }; + return { deny: `${prefix} ${screened.withheld}. ${rerunWarning(result.result)}` }; }).catch(async ($, e, next) => { - // The tool has run: a scan that failed or ran out of time must not show - // the output it was scanning. + // The tool has run: a scan that failed or ran out of time, or a redacted + // result the engine refused, must not show the output it was scanning. const key = allowedCalls.get(e.tool_use_id); if (key === undefined || !next.called) return next(e); allowedCalls.delete(e.tool_use_id); coreCalls.add(key); - record($, { event: 'withheld', tool_use_id: e.tool_use_id, reason: 'scanning it failed' }); + record($, { + event: 'withheld', + tool_use_id: e.tool_use_id, + reason: 'scanning or redacting it failed', + }); // In a catch handler, next replays the settled result without running the tool again. const warning = rerunWarning((await next(e)).result); - return { deny: `${WITHHELD} scanning it failed. ${warning}` }; + return { deny: `${WITHHELD} scanning or redacting it failed. ${warning}` }; }); }; diff --git a/plugins/preallow/hooks/withhold.ts b/plugins/preallow/hooks/withhold.ts index a2fc487..3697c51 100644 --- a/plugins/preallow/hooks/withhold.ts +++ b/plugins/preallow/hooks/withhold.ts @@ -2,9 +2,9 @@ // model. The judge decides before the call runs and cannot see what it will // print, and the classifier, which weighs that, never saw the call. -import type { EngineInterface } from 'claude-code'; +import type { EngineInterface, ToolCallResult } from 'claude-code'; -import { findLeak } from './gitleaks.ts'; +import { type Finding, findLeaks } from './gitleaks.ts'; // Every quantifier that could start at each character of a long run is // bounded or stops at the end of its line, so a scan stays linear on a large @@ -14,29 +14,48 @@ import { findLeak } from './gitleaks.ts'; // syntax, so a placeholder such as `Bearer ${token}` or `replace-with-your-key` // does not match. Where code assigns a bare value, a dot makes it a property // path such as `req.headers.token`; a quoted value or a token sent in a -// header may hold dots, and a sentence may end right after a value. +// header may hold dots, and a sentence may end right after a value. The +// `secret` group is what gets redacted, so the name before it stays readable. function value(min: number, hasDots = false): string { const char = hasDots ? '[A-Za-z0-9._~+/=-]' : '[A-Za-z0-9_~+/=-]'; - return String.raw`(?=${char}{0,256}\d)(?=${char}{0,256}[A-Za-z])${char}{${min},}(?!\w|\.\w)`; + return String.raw`(?(?=${char}{0,256}\d)(?=${char}{0,256}[A-Za-z])${char}{${min},})(?!\w|\.\w)`; } // A variable or key name for a secret, such as `DB_PASSWORD`, `apiKey`, or // `aws_secret_access_key`. const SECRET_NAME = String.raw`\b[\w-]{0,40}(?:token|secret|passw(?:or)?d|credential|api[_-]?key|private[_-]?key)[\w-]{0,40}`; +// The `=` or `:` before a value, which starts on the same line, or on a later +// one only after a quote, as when a formatter wraps a long string: otherwise +// an empty `API_KEY=` would take the name on the next line for its value. +const ASSIGNMENT = String.raw`[:=](?:\s*["']|[ \t]*)`; + +const LINE_BREAK = String.raw`(?:\r\n?|\n|\\r(?:\\n)?|\\n)`; +// The rest of a key past the 40 body characters its shape needs, where no end +// marker closes it: each line after that is one base64 run, so a line of +// text such as `Private-MAC: ...` stays. +const KEY_LINES = String.raw`[A-Za-z0-9+/=]*(?:[ \t]*${LINE_BREAK}{1,2}[ \t]*[A-Za-z0-9+/=]+(?=[ \t]*(?:[\r\n\\"']|$)))*`; + // A key needs its body, not only the words that name its format, so text // about a format, this file's included, is not taken for a key. A key's // newlines may be escaped, as in JSON. const SECRET_SHAPES: ReadonlyArray<[string, RegExp]> = [ // PEM and OpenPGP armor, then within 200 characters, past any headers, 40 // characters of its base64 body, counted across lines since parsers accept - // any width, any line ending, and blanks ending a line. + // any width, any line ending, and blanks ending a line. The rest of the key + // runs to its end marker, whatever starts its lines, such as line numbers, + // looked for within the length of an 8192-bit key with numbered lines. [ 'private key', - /-----BEGIN[ A-Z0-9_-]{0,100}PRIVATE KEY(?: BLOCK)?-----[\s\S]{0,200}?(?:\s|\\[rn])(?:[A-Za-z0-9+/](?:[ \t]{0,64}(?:[\r\n]|\\[rn]){1,2})?){40}/, + new RegExp( + String.raw`-----BEGIN[ A-Z0-9_-]{0,100}PRIVATE KEY(?: BLOCK)?-----[\s\S]{0,200}?(?:\s|\\[rn])(?:[A-Za-z0-9+/](?:[ \t]{0,64}(?:[\r\n]|\\[rn]){1,2})?){40}(?:[\s\S]{0,10000}?-----END[ A-Z0-9_-]{0,100}PRIVATE KEY(?: BLOCK)?-----|${KEY_LINES})`, + ), ], // A PuTTY key file's private part. - ['private key', /\bPrivate-Lines: \d{1,3}(?:\s|\\[rn]){1,4}[A-Za-z0-9+/]{40}/], + [ + 'private key', + new RegExp(String.raw`\bPrivate-Lines: \d{1,3}(?:\s|\\[rn]){1,4}[A-Za-z0-9+/]{40}${KEY_LINES}`), + ], // An age identity: its Bech32 body. ['private key', /AGE-SECRET-KEY-1[QPZRY9X8GF2TVDW0S3JN54KHCE6MUA7L]{58}/], // Providers that share the `sk-` prefix in formats gitleaks does not know. @@ -50,7 +69,7 @@ const SECRET_SHAPES: ReadonlyArray<[string, RegExp]> = [ [ 'auth header', new RegExp( - String.raw`\b(?:authorization|x-api-key|x-auth-token)["']?\s*[:=]\s*["']?(?:(?:bearer|basic|token)\s+)?${value(16, true)}`, + String.raw`\b(?:authorization|x-api-key|x-auth-token)["']?\s*${ASSIGNMENT}(?:(?:bearer|basic|token)\s+)?${value(16, true)}`, 'i', ), ], @@ -61,34 +80,70 @@ const SECRET_SHAPES: ReadonlyArray<[string, RegExp]> = [ [ 'session cookie', new RegExp( - String.raw`(?:sess|(?(?![\da-f]{8}-[\da-f]{4}-[\da-f]{4}-[\da-f]{4}-[\da-f]{12}(?![%\w./+=~-]))(?=[%\w./+=~-]{0,256}\d)(?=[%\w./+=~-]{0,256}[a-z])[%\w./+=~-]{16,})`, 'i', ), ], [ 'secret assignment', - new RegExp(String.raw`${SECRET_NAME}["']?\s*[:=]\s*["']?${value(16)}`, 'i'), + new RegExp(String.raw`${SECRET_NAME}["']?\s*${ASSIGNMENT}${value(16)}`, 'i'), ], [ 'secret assignment', new RegExp(String.raw`${SECRET_NAME}["']?\s*[:=]\s*(["'])${value(16, true)}\1`, 'i'), ], // An environment variable named for a key, such as `SERVICE_KEY=...`. - ['secret assignment', new RegExp(String.raw`\b[A-Z0-9_]{0,40}_KEY\s*[:=]\s*["']?${value(16)}`)], + ['secret assignment', new RegExp(String.raw`\b[A-Z0-9_]{0,40}_KEY\s*${ASSIGNMENT}${value(16)}`)], // Azure storage connection strings. ['secret assignment', new RegExp(String.raw`\bAccountKey=${value(16)}`)], - ['netrc password', /^[ \t]*(?:machine\s+\S+|default)\s[\s\S]{0,200}?\bpassword\s+\S+/m], - ['registry credential', /"auth"\s*:\s*"[A-Za-z0-9+/]{16,}={0,2}"/], + [ + 'netrc password', + /^[ \t]*(?:machine\s+\S+|default)\s[\s\S]{0,200}?\bpassword\s+(?\S+)/m, + ], + ['registry credential', /"auth"\s*:\s*"(?[A-Za-z0-9+/]{16,}={0,2})"/], ]; -// The kind of credential-shaped value the output holds: a shape above, in -// order, or else the id of a gitleaks rule for a provider's token format. -export function findSecret(text: string): string | undefined { - return SECRET_SHAPES.find(([, shape]) => shape.test(text))?.[0] ?? findLeak(text); +const SHAPES = SECRET_SHAPES.map( + ([kind, shape]) => [kind, new RegExp(shape.source, `${shape.flags}dg`)] as const, +); + +// Every credential-shaped value in the text: the shapes above, in order, then +// gitleaks' rules for providers' token formats. +export function findSecrets(text: string): Finding[] { + const found: Finding[] = []; + for (const [kind, shape] of SHAPES) { + for (const match of text.matchAll(shape)) { + const [start, end] = match.indices!.groups?.secret ?? match.indices![0]!; + found.push({ kind, start, end }); + } + } + return [...found, ...findLeaks(text)]; } -// What the model reads in place of a withheld output. The call was made, so a -// rerun repeats its effects, such as a message sent or a second server started. +// Each value gives way to a marker naming its kind, keeping the value's line +// breaks so the lines after it keep their numbers, as a Read result counts them. +export function redact(text: string, found: readonly Finding[]): string { + let out = ''; + let at = 0; + for (const { kind, start, end } of [...found].sort((a, b) => a.start - b.start)) { + if (start >= at) { + out += `${text.slice(at, start)}[redacted by preallow: ${kind}]`; + at = start; + } + if (end > at) { + out += text.slice(at, end).replaceAll(/[^\n]/g, ''); + at = end; + } + } + return out + text.slice(at); +} + +// What the model reads beside a result the plugin redacted. The call was made, +// so a rerun repeats its effects, such as a message sent or a second server started. +const REDACTED = + 'The call already ran. preallow replaced each value it detected as a possible credential with "[redacted by preallow: ]", where names the pattern that matched; a marker is not a usable value. Continue with the rest of the output. Refer to a credential by where it is kept, such as an environment variable or a file, not by its value, and run no call to reveal a redacted value; if the task needs the value itself, ask the user to look at it outside this conversation. Running the same call again with identical input repeats its effects and leaves it to the auto-mode classifier.'; + +// What the model reads in place of a withheld output. export const WITHHELD = 'The call was made, and preallow withheld its output:'; const RERUN = 'Running it again repeats its effects, and leaves it to the auto-mode classifier.'; @@ -97,36 +152,129 @@ export function rerunWarning(output: unknown): string { if (backgroundTaskId === undefined) return RERUN; // The task id goes back to the model even when the rest of the output cannot. const recovery = - typeof backgroundTaskId === 'string' && findSecret(backgroundTaskId) === undefined + typeof backgroundTaskId === 'string' && findSecrets(backgroundTaskId).length === 0 ? `Check task ${JSON.stringify(backgroundTaskId)} before starting another run.` : 'Ask the user to inspect /tasks before starting another run.'; return `The task may still be running in the background. Withholding its output did not stop it. ${recovery}`; } -// Why the output of a call the plugin allowed must be withheld, if it must: -// what the model would read, including the full output a large result leaves -// in a file, holds a credential-shaped value, or the output cannot be scanned. -// The model reads the core's `text`, or a hook's own result, which carries -// none, and any context beside it. -export async function withholding( - $: EngineInterface, - text: string | undefined, - output: unknown, - context: readonly string[] | undefined, -): Promise { - const { persistedOutputPath } = (output ?? {}) as { - persistedOutputPath?: unknown; - }; +const mapStrings = (value: unknown, change: (text: string) => string): unknown => + typeof value === 'string' + ? change(value) + : Array.isArray(value) + ? value.map((item) => mapStrings(item, change)) + : typeof value === 'object' && value !== null + ? Object.fromEntries( + Object.entries(value).map(([k, v]) => [change(k), mapStrings(v, change)]), + ) + : value; + +const holds = (kind: string) => `it holds what looks like a credential (${kind})`; + +// Every place a found value appears in the text, as a copy the shapes do not +// find beside it is the same credential. +function copies(text: string, values: ReadonlyMap): Finding[] { + const found: Finding[] = []; + for (const [value, kind] of values) + for (let at = text.indexOf(value); at !== -1; at = text.indexOf(value, at + value.length)) + found.push({ kind, start: at, end: at + value.length }); + return found; +} - let full = [text ?? JSON.stringify(output) ?? '', ...(context ?? [])].join('\n'); +// How another text shows a value: a value across lines by the runs on its +// lines, since a key in a Read result has line numbers between them and one in +// JSON has escaped line breaks; any other value whole. +function parts(value: string): string[] { + const lines = value.split(new RegExp(LINE_BREAK)); + const runs = + lines.length > 1 ? lines.flatMap((line) => line.match(/[A-Za-z0-9+/]{16,}/g) ?? []) : []; + return runs.length > 0 ? runs : [value]; +} + +// What the model may read of the output of a call the plugin allowed: nothing +// to change, the result with each credential-shaped value redacted, or why it +// must be withheld whole. The model reads the core's `text`, or a hook's own +// result, which carries none, and any context beside it. +export async function screen( + $: EngineInterface, + answer: ToolCallResult, +): Promise { + // The full output a large result leaves in a file, which the model can read + // later without the plugin; the plugin cannot redact the engine's file. + const { persistedOutputPath } = (answer.result ?? {}) as { persistedOutputPath?: unknown }; if (typeof persistedOutputPath === 'string') { + let saved: string; try { - full += `\n${await $.fs.read(persistedOutputPath)}`; + saved = await $.fs.read(persistedOutputPath); } catch { - return 'its full output could not be read to scan it'; + return { withheld: 'its full output could not be read to scan it' }; } + const [first] = findSecrets(saved); + if (first !== undefined) return { withheld: holds(first.kind) }; + } + + const text = answer.text ?? JSON.stringify(answer.result) ?? ''; + const shown = findSecrets(text); + + const values = new Map(); + const add = (value: string, kind: string) => { + for (const part of parts(value)) values.set(part, kind); + }; + for (const { kind, start, end } of shown) add(text.slice(start, end), kind); + + // The record is scanned too, since line numbers or JSON escapes in the text + // can hide a shape the record holds. Core's text is all the model reads, so + // a value counts only where that text shows it, by its runs or, on lines + // too narrow for runs, line by line, and not in a field such as the file an + // Edit result keeps. Every value is found before any is redacted, so that + // each copy of it, wherever it comes, is redacted or withholds the output. + const held = new Map(); + if (answer.isError !== true) + mapStrings(answer.result, (leaf) => { + const found = findSecrets(leaf).filter(({ start, end }) => { + if (answer.text === undefined) return true; + const value = leaf.slice(start, end); + return ( + parts(value).some((part) => values.has(part) || text.includes(part)) || + value.split(new RegExp(LINE_BREAK)).every((line) => text.includes(line.trim())) + ); + }); + held.set(leaf, found); + for (const { kind, start, end } of found) add(leaf.slice(start, end), kind); + return leaf; + }); + + // Core keeps whole the context a hook below attached. + for (const entry of answer.context ?? []) { + const [first] = [...findSecrets(entry), ...copies(entry, values)]; + if (first !== undefined) return { withheld: holds(first.kind) }; } - const secret = findSecret(full); - return secret === undefined ? undefined : `it holds what looks like a credential (${secret})`; + // An error's text is all the model reads of it. + if (answer.isError === true) { + if (shown.length === 0) return undefined; + return { + redacted: { deny: `${redact(text, [...shown, ...copies(text, values)])}\n\n${REDACTED}` }, + kinds: [...new Set(shown.map(({ kind }) => kind))], + }; + } + + const leaves: string[] = []; + const kinds = new Set(); + const result = mapStrings(answer.result, (leaf) => { + const found = [...held.get(leaf)!, ...copies(leaf, values)]; + for (const { kind } of found) kinds.add(kind); + leaves.push(leaf); + return redact(leaf, found); + }); + // The core renders its text from the record, so a value only the text holds + // cannot be redacted from it. + for (const [value, kind] of values) + if (!leaves.some((leaf) => leaf.includes(value))) return { withheld: holds(kind) }; + if (kinds.size === 0) return undefined; + + return { + redacted: { result, context: [...(answer.context ?? []), REDACTED] }, + kinds: [...kinds], + }; } diff --git a/plugins/preallow/tests/background.test.ts b/plugins/preallow/tests/background.test.ts index 03d8e2b..55503a3 100644 --- a/plugins/preallow/tests/background.test.ts +++ b/plugins/preallow/tests/background.test.ts @@ -25,7 +25,7 @@ describe('background handoff', () => { expect((await check($, 'Bash', bash('run stage'))).decision).toBe('allow'); }); - test('a credential in the initial handoff is withheld without inviting another run', async ($, on) => { + test("a credential the handoff's text shows and its record does not hold is withheld without inviting another run", async ($, on) => { const w = world(on, { output: `${HANDOFF}\n${FAKE_TOKEN}`, record: { stdout: 'ready', stderr: '', interrupted: false, backgroundTaskId: 'b1' }, @@ -59,7 +59,7 @@ describe('background handoff', () => { command: 'run stage', tool_use_id: 't1', } as never); - expect(result.deny).toMatch(/^The call was made, .* scanning it failed/); + expect(result.deny).toMatch(/^The call was made, .* scanning or redacting it failed/); expect(result.deny).toContain( background ? 'Check task "b1" before starting another run' @@ -68,7 +68,7 @@ describe('background handoff', () => { expect(w.ran).toBe(1); expect((await w.logged(2))[1]).toMatchObject({ event: 'withheld', - reason: 'scanning it failed', + reason: 'scanning or redacting it failed', }); expect((await check($, 'Bash', bash('run stage'))).decision).toBe('ask'); }); @@ -76,8 +76,9 @@ describe('background handoff', () => { test('a task id that holds a credential is not echoed in the recovery message', async ($, on) => { const w = world(on, { - output: FAKE_TOKEN, - record: { backgroundTaskId: FAKE_TOKEN }, + output: 'Output too large, saved to /out/b1.txt', + record: { persistedOutputPath: '/out/b1.txt', backgroundTaskId: FAKE_TOKEN }, + files: new Map([['/out/b1.txt', FAKE_TOKEN]]), }); await auto($); const result = await $.tool.call({ diff --git a/plugins/preallow/tests/output.test.ts b/plugins/preallow/tests/output.test.ts new file mode 100644 index 0000000..c002b26 --- /dev/null +++ b/plugins/preallow/tests/output.test.ts @@ -0,0 +1,185 @@ +import { describe, expect, test } from 'claude-code/testing'; +import { auto, bash, check } from './session.ts'; +import { world } from './world.ts'; + +const RANDOM = 'q7Rk2Vx9Lm4Np8Ts3Wz6Yb1Hc5'; +const FAKE_TOKEN = `sk-${RANDOM}`; +const KEY_LINES = Array.from({ length: 4 }, (_, i) => `${'MIIEpAIBAAKCAQEA'.slice(i)}${RANDOM}`); +const KEY = [ + `-----BEGIN RSA ${'PRIVATE KEY'}-----`, + ...KEY_LINES, + `-----END RSA ${'PRIVATE KEY'}-----`, +]; + +describe('output', () => { + test('a call the plugin allowed has each credential in its output redacted, and runs again under the classifier', async ($, on) => { + const w = world(on, { + output: `ok\nTOKEN=${FAKE_TOKEN}`, + record: { stdout: `ok\nTOKEN=${FAKE_TOKEN}`, stderr: '', interrupted: false }, + }); + await auto($); + const result = await $.tool.call({ + tool: 'Bash', + command: 'cat config.json', + tool_use_id: 't1', + } as never); + expect(w.ran).toBe(1); + expect(result.result).toEqual({ + stdout: 'ok\nTOKEN=[redacted by preallow: API key]', + stderr: '', + interrupted: false, + }); + expect(result.text).toBeUndefined(); + expect(result.ref).toBeUndefined(); + expect(result.context).toHaveLength(1); + const lines = await w.logged(2); + expect(lines[1]).toMatchObject({ + event: 'redacted', + tool_use_id: 't1', + kinds: ['API key', 'secret assignment'], + }); + expect(JSON.stringify([result, lines])).not.toContain(FAKE_TOKEN); + expect((await check($, 'Bash', bash('cat config.json'))).decision).toBe('ask'); + expect((await check($, 'Bash', bash('cat other.json'))).decision).toBe('allow'); + + // A failed call's text is all the model reads of it. + w.callFails = true; + w.output = `password=${RANDOM}, retried with ${RANDOM}`; + const failed = await $.tool.call({ + tool: 'Bash', + command: 'cat secret.json', + tool_use_id: 't3', + } as never); + expect(failed.deny).toContain( + 'password=[redacted by preallow: secret assignment], retried with [redacted by preallow: secret assignment]', + ); + }); + + test('output is passed on when it holds no credential, only a field the model does not read holds one, or the plugin did not allow the call', async ($, on) => { + const w = world(on, { output: 'version 1.2.3' }); + await auto($); + const call = (id: string) => + $.tool.call({ tool: 'Bash', command: 'cat config.json', tool_use_id: id } as never); + expect((await call('t1')).text).toBe('version 1.2.3'); + w.callFails = true; + expect((await call('t2')).text).toBe('version 1.2.3'); + w.callFails = false; + // An Edit result keeps the file it changed, which its text does not show. + w.record = { filePath: '/w/app.env', originalFile: `TOKEN=${FAKE_TOKEN}` }; + expect((await call('t4')).result).toEqual(w.record); + w.record = undefined; + w.output = `TOKEN=${FAKE_TOKEN}`; + w.core = { decision: 'allow' }; + expect((await call('t3')).text).toContain(FAKE_TOKEN); + }); + + test('a key shown with line numbers is redacted whole, in a Read record and in Bash output', async ($, on) => { + const content = `before\n${KEY.join('\n')}\nafter`; + const numbered = content + .split('\n') + .map((line, i) => `${String(i + 1).padStart(6)}\t${line}`) + .join('\n'); + const w = world(on, { output: numbered, record: { type: 'text', file: { content } } }); + await auto($); + const result = await $.tool.call({ + tool: 'Read', + file_path: '/w/key.pem', + tool_use_id: 't1', + } as never); + const redacted = (result.result as { file: { content: string } }).file.content; + expect(redacted).toBe(`before\n[redacted by preallow: private key]${'\n'.repeat(5)}\nafter`); + expect(w.ran).toBe(1); + + w.record = { stdout: numbered, stderr: '', interrupted: false }; + const printed = await $.tool.call({ + tool: 'Bash', + command: 'cat -n key.pem', + tool_use_id: 't2', + } as never); + expect((printed.result as { stdout: string }).stdout).toBe( + ` 1\tbefore\n 2\t[redacted by preallow: private key]${'\n'.repeat(6)} 8\tafter`, + ); + }); + + test('a credential that line numbers hide from the shapes in the text is redacted in the record, with its copies', async ($, on) => { + const numbered = (content: string) => + content + .split('\n') + .map((line, i) => `${String(i + 1).padStart(6)}\t${line}`) + .join('\n'); + const content = `machine example.com login u password ${RANDOM}`; + // The copy comes before the value the shapes find. + const w = world(on, { + output: numbered(content), + record: { type: 'text', [RANDOM]: 'name', file: { content } }, + }); + await auto($); + const read = (id: string) => + $.tool.call({ tool: 'Read', file_path: `/w/${id}`, tool_use_id: id } as never); + const marker = '[redacted by preallow: netrc password]'; + expect((await read('t1')).result).toEqual({ + type: 'text', + [marker]: 'name', + file: { content: `machine example.com login u password ${marker}` }, + }); + w.context = [`retried with ${RANDOM}`]; + expect((await read('t2')).deny).toMatch(/^preallow: it holds .*netrc password/); + w.context = undefined; + + // A key wrapped too narrow to give a line its own run of the body. + const key = `-----BEGIN ${'PRIVATE KEY'}-----\n${'q7Rk2Vx9\n'.repeat(8)}-----END ${'PRIVATE KEY'}-----`; + w.output = numbered(key); + w.record = { type: 'text', file: { content: key } }; + expect((await read('t3')).result).toEqual({ + type: 'text', + file: { content: `[redacted by preallow: private key]${'\n'.repeat(9)}` }, + }); + }); + + test('a value found once is redacted where the shapes do not find it', async ($, on) => { + // The shapes find the value by the name before it, which only the text + // shows; the record also holds it under another name, and as a name. + world(on, { + output: `{"password": "${RANDOM}"}`, + record: { password: RANDOM, copy: RANDOM, [RANDOM]: 'name' }, + }); + await auto($); + const result = await $.tool.call({ tool: 'mcp__srv__lookup', tool_use_id: 't1' } as never); + const marker = '[redacted by preallow: secret assignment]'; + expect(result.result).toEqual({ password: marker, copy: marker, [marker]: 'name' }); + }); + + test("output the model would read later, from a file, from a hook's own result, or beside it is scanned, and redacted or withheld", async ($, on) => { + const w = world(on, { + output: 'Output too large, saved to /out/t1.txt', + record: { stdout: '', persistedOutputPath: '/out/t1.txt' }, + }); + await auto($); + const call = (command: string, id: string) => + $.tool.call({ tool: 'Bash', command, tool_use_id: id } as never); + w.files.set('/out/t1.txt', `line\nTOKEN=${FAKE_TOKEN}\n`); + expect((await call('cat a.log', 't1')).deny).toContain('API key'); + w.files.set('/out/t1.txt', 'line\n'.repeat(3)); + expect((await call('cat b.log', 't2')).text).toBe('Output too large, saved to /out/t1.txt'); + w.files.delete('/out/t1.txt'); + expect((await call('cat c.log', 't3')).deny).toContain('could not be read'); + w.output = undefined; + w.record = { stdout: `TOKEN=${FAKE_TOKEN}` }; + expect((await call('cat e.log', 't5')).result).toEqual({ + stdout: 'TOKEN=[redacted by preallow: API key]', + }); + // A key wrapped too narrow to give a line its own run of the body. + w.output = `-----BEGIN ${'PRIVATE KEY'}-----\n${'q7Rk2Vx9\n'.repeat(8)}`; + expect((await call('cat k.log', 't9')).deny).toMatch(/^preallow: it holds .*private key/); + w.output = `password=${RANDOM}`; + w.record = undefined; + w.context = [`retried with ${RANDOM}`]; + expect((await call('cat f.log', 't6')).deny).toMatch(/^preallow: it holds .*secret assignment/); + w.output = 'ok'; + w.context = [`TOKEN=${FAKE_TOKEN}`]; + expect((await call('cat g.log', 't7')).deny).toMatch(/^preallow: it holds .*API key/); + // A failed call carries no word from the engine that it ran. + w.callFails = true; + expect((await call('cat h.log', 't8')).deny).toMatch(/^The call was made, .*API key/); + }); +}); diff --git a/plugins/preallow/tests/withhold.test.ts b/plugins/preallow/tests/withhold.test.ts index 2a0ab1c..378a6fb 100644 --- a/plugins/preallow/tests/withhold.test.ts +++ b/plugins/preallow/tests/withhold.test.ts @@ -1,21 +1,19 @@ import { describe, expect, test } from 'claude-code/testing'; import { rules } from '../hooks/gitleaks.generated.ts'; -import { findSecret } from '../hooks/withhold.ts'; -import { auto, bash, check } from './session.ts'; -import { world } from './world.ts'; +import { findSecrets, redact } from '../hooks/withhold.ts'; // Values are built at run time, so no scanner takes the source for a leak. const RANDOM = 'q7Rk2Vx9Lm4Np8Ts3Wz6Yb1Hc5'; -const FAKE_TOKEN = `sk-${RANDOM}`; const B62 = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789'; const BECH32 = 'QPZRY9X8GF2TVDW0S3JN54KHCE6MUA7L'; const DIGITS = '1234567890'; +const HEX = '0123456789abcdef'; // A value spread evenly over its alphabet, so it passes the entropy check of // gitleaks' rules as a generated token does. const mixed = (n: number, alphabet = B62) => Array.from({ length: n }, (_, i) => alphabet[(i * 7) % alphabet.length]).join(''); -describe('findSecret', () => { +describe('findSecrets', () => { const found: Array<[string, string]> = [ ['private key', `-----BEGIN OPENSSH ${'PRIVATE KEY'}-----\n${mixed(70)}\n`], ['API key', `sk-ant-api03-${RANDOM}`], @@ -25,6 +23,7 @@ describe('findSecret', () => { ['auth header', `x-api-key: ${RANDOM}.${RANDOM}`], ['bearer token', `curl -H "x: Bearer ya29.${RANDOM}"`], ['secret assignment', `DB_PASSWORD="${RANDOM}"`], + ['secret assignment', `const apiKey =\n "${RANDOM}`], ['private key', `-----BEGIN PGP ${'PRIVATE KEY'} BLOCK-----\nVersion: v1\n\n${mixed(64)}`], ['private key', `-----BEGIN SM2 ${'PRIVATE KEY'}-----\n${mixed(64)}`], ['private key', `-----BEGIN ${'PRIVATE KEY'}-----\n${`${mixed(8)}\n`.repeat(8)}`], @@ -62,10 +61,14 @@ describe('findSecret', () => { ['npm-access-token', `The token is npm_${mixed(36)}.`], ['atlassian-api-token', `ATATT3${mixed(186)}`], ['okta-access-token', `OKTA = 00${mixed(40)}`], + ['sourcegraph-access-token', `sgp_${mixed(16, HEX)}_${mixed(40, HEX)}`], + // A legacy Sourcegraph token has no prefix; only its name tells it from a + // commit hash. + ['secret assignment', `SRC_ACCESS_TOKEN=${mixed(40, HEX)}`], ]; for (const [kind, text] of found) { test(`${kind}: ${text}`, () => { - expect(findSecret(text)).toBe(kind); + expect(findSecrets(text)[0]?.kind).toBe(kind); }); } @@ -84,6 +87,8 @@ describe('findSecret', () => { 'The default timeout applies; see the password reset guide.', `sessionId=${['3f2a9c4e', '7b1d', '4e8a', '9c2f', '5d6e7a8b9c0d'].join('-')}`, 'session_timeout=3600', + // Empty assignments, each followed by a name of letters and digits. + 'API_TOKEN=\nAWS_S3_REGION_2=\nSERVICE_KEY=\nS3_BUCKET_NAME_2024=\nAUTHORIZATION=\nS3_ACCESS_KEY_ID2=\n', `considered=${RANDOM}`, 'const PASSWORD_MISSING = "password_missing";', // A hostname's dots match only dots. @@ -103,6 +108,7 @@ describe('findSecret', () => { `hf_${'ABCDEFGHIJKLMNOPQRSTUVWXYZ'}abcdefgh`, `sonar.login=${'abcdefghijklmnopqrstuvwxyz'}0123456789abcd`, `data:\n color: ${'Ymx1ZQ'.repeat(3)}\n---\napiVersion: v1\nkind: Secret\ntype: Opaque`, + `Sourcegraph search, commit ${mixed(40, HEX)}`, ]; for (const [name, run] of [ ['word characters and hyphens', 'a-'.repeat(500_000)], @@ -112,11 +118,15 @@ describe('findSecret', () => { ['secret names', 'A_KEY='.repeat(100_000)], ['cookie names', 'sess'.repeat(250_000)], ['key armor', `-----BEGIN ${'PRIVATE KEY'}-----\n${'A\n'.repeat(39)}`.repeat(9_500)], + [ + 'key armor cut off before its end', + `-----BEGIN ${'PRIVATE KEY'}-----\n${'A'.repeat(40)}\n`.repeat(15_000), + ], ['PuTTY private lines', `Private-Lines: 1\n${'A'.repeat(39)}\n`.repeat(17_000)], ] as const) { test(`a long run of ${name} scans in linear time`, () => { const start = performance.now(); - findSecret(run); + findSecrets(run); expect(performance.now() - start).toBeLessThan(2000); }); } @@ -126,81 +136,66 @@ describe('findSecret', () => { for (const run of ['.', 'a.', ' ']) { const unit = keywords.map((k) => `${k}${run.repeat(400)}`).join(' '); const start = performance.now(); - expect(findSecret(unit.repeat(Math.ceil(1_000_000 / unit.length)))).toBeUndefined(); + expect(findSecrets(unit.repeat(Math.ceil(1_000_000 / unit.length)))).toEqual([]); expect(performance.now() - start).toBeLessThan(2000); } }); for (const text of clean) { test(`no secret: ${text}`, () => { - expect(findSecret(text)).toBeUndefined(); + expect(findSecrets(text)).toEqual([]); }); } }); -describe('output', () => { - test('a call the plugin allowed has its output withheld when it holds a credential, and runs again under the classifier', async ($, on) => { - const w = world(on, { output: `TOKEN=${FAKE_TOKEN}` }); - await auto($); - const result = await $.tool.call({ - tool: 'Bash', - command: 'cat config.json', - tool_use_id: 't1', - } as never); - expect(w.ran).toBe(1); - expect(result.deny).toContain('API key'); - expect(result.deny).toMatch(/^preallow: it holds/); - expect(JSON.stringify(result)).not.toContain(FAKE_TOKEN); - const lines = await w.logged(2); - expect([lines[1]!.event, lines[1]!.reason]).toEqual([ - 'withheld', - 'it holds what looks like a credential (API key)', - ]); - expect(JSON.stringify(lines)).not.toContain(FAKE_TOKEN); - expect((await check($, 'Bash', bash('cat config.json'))).decision).toBe('ask'); - expect((await check($, 'Bash', bash('cat other.json'))).decision).toBe('allow'); +describe('redact', () => { + const redacted = (text: string) => redact(text, findSecrets(text)); - // A failed call carries no word from the engine that it ran. - w.callFails = true; - const failed = await $.tool.call({ - tool: 'Bash', - command: 'cat secret.json', - tool_use_id: 't3', - } as never); - expect(failed.deny).toMatch(/^The call was made, .*API key/); + test('each value gives way to a marker, keeping its line breaks and what surrounds it', () => { + const body = Array.from({ length: 3 }, (_, i) => mixed(64 - i)); + const pem = `-----BEGIN ${'PRIVATE KEY'}-----\n${body.join('\n')}\n-----END ${'PRIVATE KEY'}-----`; + expect(redacted(`a\n${pem}\nTOKEN=${RANDOM} b`)).toBe( + `a\n[redacted by preallow: private key]${'\n'.repeat(5)}TOKEN=[redacted by preallow: secret assignment] b`, + ); + const escaped = JSON.stringify({ key: pem }); + expect(redacted(escaped)).toBe('{"key":"[redacted by preallow: private key]"}'); }); - test('output is passed on when it holds no credential, or the plugin did not allow the call', async ($, on) => { - const w = world(on, { output: 'version 1.2.3' }); - await auto($); - const call = (id: string) => - $.tool.call({ tool: 'Bash', command: 'cat config.json', tool_use_id: id } as never); - expect((await call('t1')).text).toBe('version 1.2.3'); - w.output = `TOKEN=${FAKE_TOKEN}`; - w.core = { decision: 'allow' }; - expect((await call('t2')).text).toContain(FAKE_TOKEN); + test('a key runs to its end marker, or else through its last line of base64, and no further', () => { + const body = `${mixed(64)}\n${mixed(20)}`; + expect(redacted(`-----BEGIN ${'PRIVATE KEY'}-----\n${body}\nOutput truncated.`)).toBe( + '[redacted by preallow: private key]\n\n\nOutput truncated.', + ); + const putty = `Private-Lines: 2\n${body}\nPrivate-MAC: ${mixed(64, HEX)}`; + expect(redacted(putty)).toBe( + `[redacted by preallow: private key]\n\n\nPrivate-MAC: ${mixed(64, HEX)}`, + ); + expect(redacted(`${'AGE-SECRET-KEY'}-1${mixed(58, BECH32)}\nREADME\nDone`)).toBe( + '[redacted by preallow: private key]\nREADME\nDone', + ); }); - test("output the model would read later, from a file, from a hook's own result, or beside it is scanned or withheld", async ($, on) => { - const w = world(on, { - output: 'Output too large, saved to /out/t1.txt', - record: { stdout: '', persistedOutputPath: '/out/t1.txt' }, - }); - await auto($); - const call = (command: string, id: string) => - $.tool.call({ tool: 'Bash', command, tool_use_id: id } as never); - w.files.set('/out/t1.txt', `line\nTOKEN=${FAKE_TOKEN}\n`); - expect((await call('cat a.log', 't1')).deny).toContain('API key'); - w.files.set('/out/t1.txt', 'line\n'.repeat(3)); - expect((await call('cat b.log', 't2')).text).toBe('Output too large, saved to /out/t1.txt'); - w.files.delete('/out/t1.txt'); - expect((await call('cat c.log', 't3')).deny).toContain('could not be read'); - w.output = undefined; - w.record = { stdout: `TOKEN=${FAKE_TOKEN}` }; - expect((await call('cat e.log', 't5')).deny).toContain('API key'); - w.output = 'ok'; - w.record = undefined; - w.context = [`TOKEN=${FAKE_TOKEN}`]; - expect((await call('cat f.log', 't6')).deny).toContain('API key'); + test('a gitleaks rule whose groups hold parts of its token has the whole token redacted', () => { + const uuid = `${mixed(8, HEX)}-${'a1b2-'.repeat(3)}${mixed(12, HEX)}`; + const webhook = `https://example.webhook.office.com/webhookb2/${uuid}@${uuid}/IncomingWebhook/${mixed(32, HEX)}/${uuid}`; + expect(redacted(`url: ${webhook}`)).toBe( + 'url: [redacted by preallow: microsoft-teams-webhook]', + ); + expect(redacted(`${'ZXlKaGJHY2lPaU'}${mixed(48)} b`)).toBe( + '[redacted by preallow: jwt-base64] b', + ); + }); + + test("a gitleaks rule's match keeps the name before its secret", () => { + expect(redacted(`OKTA = 00${mixed(40)}`)).toBe( + 'OKTA = [redacted by preallow: okta-access-token]', + ); + }); + + test('values that overlap give way to one marker', () => { + // One shape takes the value up to the dot, another the whole quoted value. + expect(redacted(`"token": "${RANDOM}==.${RANDOM}"`)).toBe( + '"token": "[redacted by preallow: secret assignment]"', + ); }); }); diff --git a/plugins/preallow/tests/world.ts b/plugins/preallow/tests/world.ts index 5c940cc..919b850 100644 --- a/plugins/preallow/tests/world.ts +++ b/plugins/preallow/tests/world.ts @@ -192,8 +192,15 @@ export function world(on: On, partial: Partial = {}): World { on('tool.call', async () => { if (w.engineRejects) return { isError: true, result: REJECTION, text: REJECTION } as never; w.ran++; - if (w.callFails) return { isError: true, result: w.output, text: w.output } as never; - return { result: w.record ?? w.output, text: w.output, context: w.context } as never; + if (w.callFails) + return { + ref: 1, + isError: true, + result: w.output, + text: w.output, + context: w.context, + } as never; + return { ref: 1, result: w.record ?? w.output, text: w.output, context: w.context } as never; }); return w; } diff --git a/scripts/gitleaks.ts b/scripts/gitleaks.ts index b579d14..5223158 100644 --- a/scripts/gitleaks.ts +++ b/scripts/gitleaks.ts @@ -1,5 +1,5 @@ // Writes `plugins/preallow/hooks/gitleaks.generated.ts`: the rules of a -// gitleaks release that preallow's withholding scan applies after its own +// gitleaks release that preallow's output scan applies after its own // shapes, with each regex rewritten from Go's syntax to JavaScript's. It is a // module, not JSON, because a hooks module may import only code. @@ -21,6 +21,22 @@ const IDENTIFIER = String.raw`[\w.-]{0,50}?`; // between a key file's markers, such as a placeholder sentence, for a key. const LEFT_OUT = new Set(['generic-api-key', 'hashicorp-tf-password', 'private-key']); +// An alternative taken out of a rule, with the keyword only it needed. +// `sourcegraph-access-token` also takes any 40-character hex string in a text +// that names Sourcegraph, so every git commit hash beside the name. A legacy +// token without the `sgp_` prefix is told from a hash only by a name such as +// `SRC_ACCESS_TOKEN` before it, which `withhold.ts` reads as a secret +// assignment; Betterleaks' rule keeps only the `sgp_` forms too. +const NARROWED: Record = { + 'sourcegraph-access-token': { alternative: '|[a-fA-F0-9]{40}', keyword: 'sourcegraph' }, +}; + +// Rules whose secret is their whole match though they hold groups: gitleaks +// takes the first group that matched for the secret, here one header field of +// a JWT or one segment of a UUID, and the scan redacts the secret, so their +// groups become non-capturing. +const WHOLE_MATCH = new Set(['jwt-base64', 'microsoft-teams-webhook']); + // How gitleaks ends a provider token, consuming the character after it since // Go's engine has no lookahead. That misses a token followed by a full stop, // a comma, a bracket, a colon, or `@`, and takes the separator a following @@ -44,8 +60,8 @@ const POSIX: Record = { alnum: 'A-Za-z0-9', alpha: 'A-Za-z', dig // Go applies an inline flag such as `(?i)` to the rest of its group, across // alternatives; JavaScript only has the scoped form `(?i:...)`, so each one // opens a scoped group that is closed and reopened at every `|` and closed -// with the enclosing group. -function toJs(go: string): string { +// with the enclosing group. Without `capture`, no group captures. +function toJs(go: string, capture = true): string { const src = go.replaceAll('(?P<', '(?<'); const flagGroups: string[][] = [[]]; let out = ''; @@ -69,8 +85,13 @@ function toJs(go: string): string { }); i = j; } else if (c === '(') { + const group = capture ? null : /^\((?:\?<\w+>)?(?!\?)/.exec(src.slice(i)); const flags = /^\(\?([a-z]+)\)/.exec(src.slice(i)); - if (flags === null) { + if (group !== null) { + flagGroups.push([]); + out += '(?:'; + i += group[0].length - 1; + } else if (flags === null) { flagGroups.push([]); out += c; } else { @@ -115,7 +136,7 @@ function allowlist(list: GoAllowlist) { throw new Error(`hooks/gitleaks.ts reads no allowlist condition ${list.condition}`); return { regexTarget, - regexes: (list.regexes ?? []).filter((r) => !isLiteral(r)).map(toJs), + regexes: (list.regexes ?? []).filter((r) => !isLiteral(r)).map((r) => toJs(r)), stopwords: list.stopwords ?? [], }; } @@ -143,18 +164,27 @@ const rules = config.rules // A rule without a regex matches file paths, not text. rule.regex === undefined || LEFT_OUT.has(rule.id) ? [] : [{ ...rule, regex: rule.regex }], ) - .map((rule) => ({ - id: rule.id, - regex: - toJs(rule.regex.replaceAll(IDENTIFIER, '')).replaceAll(TOKEN_END, String.raw`(?![\w-])`) + - (isLiteral(rule.regex) ? TOKEN_BODY : ''), - keywords: (rule.keywords ?? []).map((k) => k.toLowerCase()), - entropy: rule.entropy ?? 0, - secretGroup: rule.secretGroup ?? 0, - allowlists: (rule.allowlists ?? []) - .map(allowlist) - .filter((list) => list.regexes.length > 0 || list.stopwords.length > 0), - })); + .map((rule) => { + const narrowed = NARROWED[rule.id]; + if (narrowed !== undefined && !rule.regex.includes(narrowed.alternative)) + throw new Error(`${rule.id} no longer has the alternative ${narrowed.alternative}`); + const stripped = rule.regex.replaceAll(IDENTIFIER, ''); + const regex = narrowed === undefined ? stripped : stripped.replace(narrowed.alternative, ''); + return { + id: rule.id, + regex: + toJs(regex, !WHOLE_MATCH.has(rule.id)).replaceAll(TOKEN_END, String.raw`(?![\w-])`) + + (isLiteral(rule.regex) ? TOKEN_BODY : ''), + keywords: (rule.keywords ?? []) + .map((k) => k.toLowerCase()) + .filter((k) => k !== narrowed?.keyword), + entropy: rule.entropy ?? 0, + secretGroup: rule.secretGroup ?? 0, + allowlists: (rule.allowlists ?? []) + .map(allowlist) + .filter((list) => list.regexes.length > 0 || list.stopwords.length > 0), + }; + }); const source = `https://github.com/gitleaks/gitleaks/blob/${VERSION}/config/gitleaks.toml`; const license = (await fetchText('LICENSE')).trim(); diff --git a/scripts/sources.ts b/scripts/sources.ts index e8164b6..b44845f 100644 --- a/scripts/sources.ts +++ b/scripts/sources.ts @@ -1,10 +1,10 @@ -// Fails when preallow's withholding scan finds a credential in one of the +// Fails when preallow's output scan finds a credential in one of the // plugin's own files: reading the plugin's source, as an agent changing it -// does, would have the output withheld. +// does, would have the output redacted. import { $ } from 'bun'; -import { findSecret } from '../plugins/preallow/hooks/withhold.ts'; +import { findSecrets } from '../plugins/preallow/hooks/withhold.ts'; const root = `${import.meta.dir}/../`; const files = (await $`git ls-files -z plugins/preallow`.cwd(root).text()) @@ -13,10 +13,10 @@ const files = (await $`git ls-files -z plugins/preallow`.cwd(root).text()) const found: string[] = []; for (const file of files) { - const kind = findSecret(await Bun.file(`${root}${file}`).text()); + const kind = findSecrets(await Bun.file(`${root}${file}`).text())[0]?.kind; if (kind !== undefined) found.push(`${file}: ${kind}`); } if (found.length > 0) { - console.error(`preallow's scan would withhold these files:\n${found.join('\n')}`); + console.error(`preallow's scan would redact these files:\n${found.join('\n')}`); process.exit(1); }