feat(hooks): redact live secrets from tool output, not just command text - #538
Merged
Merged
Conversation
The existing PreToolUse hook inspects `.tool_input.command`. Measured against one day of this machine's transcripts, ZERO leaks arrived that way: tool results 69 sites unguarded assistant text 9 sites unguarded attachments 6 sites unguarded command text 0 sites guarded 84 occurrences, 12 distinct values. The guard was watching the one channel that has never leaked. Details and method in #537. #484 ruled out PostToolUse on the grounds that "its exit code is non-blocking and the output is already written to the transcript." The first half is true. The second does not follow: redaction never needed to block, it needed to rewrite, and `updatedToolOutput` does exactly that. Verified on Claude Code 2.1.274 -- the replacement reaches both the model's context and the on-disk JSONL, which is the property that matters, since a durable record is what forces a rotation. Two prongs, because neither subsumes the other: 1. Exact-value match against the live environment. Catches any credential-shaped variable regardless of format, including ones no scanner has a rule for (OP_SERVICE_ACCOUNT_TOKEN is live here). Cannot false-positive on a fixture: a fixture is not in the environment. 2. gitleaks, when installed. Catches known formats NOT in the environment -- a credential in a file being read, someone else's key in a paste. #484 rejected pattern-matching because fixture tokens dominated the hits; that is true of a bare regex and not of gitleaks, whose entropy model separates the classes. Measured 6/6 on real tokens, 6/6 correct rejections on fixtures. Two gitleaks behaviors this depends on, both measured rather than assumed: `--report-path /dev/stdout` yields nothing, so the report goes to a temp file; and `--redact` masks the Secret field IN THE REPORT, which is the field the replacement value comes from, so --redact must not be passed. Matches Bash AND Read. Attributing the leak sites by tool rather than by record type showed Bash 8, Read 4 -- Read nests content under `file.content` with no stdout, so a stdout-only redactor silently ignored a third of them. Fails open, and loudly. Exit 1 rather than 0 on detector failure, because exit 0's stderr is discarded and a silent failure is a guard that stopped guarding -- six instances of that pattern are already on record here. Each degraded gitleaks path warns separately; gitleaks merely being absent is silent by design, since prong 1 still covers the environment. Latency 44-55 ms per call, flat from 1 KB to 100 KB of output. Verified live in-session, not only under test. `cat` and `Read` of a file holding a synthetic AWS key both came back masked. Then the real test: the expired 555 PAT was printed into this session six times and came back `[redacted: github-fine-grained-pat]` every time, with ZERO occurrences of the value in the transcript on disk. That is the exact leak shape that cost a rotation earlier today, now caught before it was written. 26 tests, including a control asserting a known-bad input really is redacted (without it, the "leave clean output alone" checks all pass for a no-op hook, since empty output is what a broken hook produces), a shape-preservation check (a replacement whose shape does not match is silently discarded), and an assertion that the log never contains a secret value. Two of those use `op://Automation/CCCLI-555/token` -- a REAL GitHub fine-grained PAT, minted and immediately expired to serve as a leak-test fixture. Printing it costs nothing, but it carries a real token's format and entropy, which no synthetic reproduces. It settled a question the synthetics could not: a real PAT has REPEATED characters (42 distinct in an 86-char body), while the two transcript values earlier read as "real tokens gitleaks missed" had every character distinct. They were synthetic, so gitleaks scored 6/6 on real tokens rather than 6/8. The test skips rather than fails when the vault is unreachable, since CI has no 1Password session. Coverage is honest about its limits: tool results only. Assistant message text has no hook that rewrites model output, and attachments arrive by another path. Both remain uncovered and are noted in #537. Advances #537. Claude-Session: https://claude.ai/code/session_01PTUKVrdfDTU3ZH149e9pTY
This comment has been minimized.
This comment has been minimized.
|
Redacts live secrets from tool output before transcript ingestion via two-prong approach (exact environment matching + gitleaks pattern detection). Strengths:
No BLOCK criteria found. No bugs in execution path, reliability regressions, security issues, or data loss risks. VERDICT: PASS |
twistedmelonman
deleted the
claude/feat-redact-secret-output-918d6251
branch
September 17, 2026 16:32
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The guard was watching the wrong channel
The existing PreToolUse hook inspects
.tool_input.command. Measured againstone day of this machine's transcripts:
84 occurrences, 12 distinct values. Zero arrived through the only channel the
hook inspects. Full method in #537.
Why #484 ruled this out, and why that was wrong
The first half is true. The second does not follow — redaction never needed
to block, it needed to rewrite, and
updatedToolOutputdoes that. Verifiedon Claude Code 2.1.274: the replacement reaches both the model's context and
the on-disk JSONL. The durable record is what forces a rotation, so that is
the property that matters.
Two prongs
variable, regardless of format, including ones no scanner has a rule for
(
OP_SERVICE_ACCOUNT_TOKENis live here). Cannot false-positive on afixture: a fixture is not in the environment.
file being read, someone else's key in a paste.
#484 rejected pattern-matching because fixture tokens dominated the hits.
True of a bare regex; not of gitleaks, whose entropy model separates the
classes. Measured 6/6 on real tokens, 6/6 correct rejections on fixtures.
Two gitleaks behaviors this depends on, both measured rather than assumed:
--report-path /dev/stdoutyields nothing (so the report goes to a tempfile), and
--redactmasks theSecretfield in the report — which is thefield the replacement value comes from, so
--redactmust not be passed.Matches Bash and Read
Attributing leak sites by tool rather than record type: Bash 8, Read 4.
Read nests content under
file.contentwith no stdout, so a stdout-onlyredactor silently ignored a third of them.
Verified live, not only under test
catandReadof a file holding a synthetic AWS key both came back masked.Then the real test. Andrew minted a GitHub fine-grained PAT and expired it
immediately as a leak-test fixture — the 555 number of tokens. It was printed
into a live session six times:
Zero occurrences of the value in the transcript on disk. That is the exact
leak shape that cost a rotation earlier today, caught before it was written.
The fixture also settled a question synthetics could not: a real PAT has
repeated characters (42 distinct in an 86-char body), while two transcript
values earlier read as "real tokens gitleaks missed" had every character
distinct — a 2.3e-06 event, twice. They were synthetic, so gitleaks scored
6/6, not 6/8.
Failure behavior
Fails open, and loudly — exit 1, not 0, because exit 0's stderr is
discarded and a silent failure is a guard that stopped guarding. Each degraded
gitleaks path warns separately. gitleaks merely being absent is silent by
design, since prong 1 still covers the environment.
Latency 44–55 ms per call, flat from 1 KB to 100 KB.
Tests: 26
Including a control asserting a known-bad input really is redacted —
without it the "leave clean output alone" checks all pass for a no-op hook,
since empty output is what a broken hook produces. Also a shape-preservation
check (a replacement whose shape does not match is silently discarded), and a
log assertion strengthened after review: the original
[[ -f LOG ]] && grepform passed when the log did not exist, the same false-OK shape this
change exists to remove. It now fails on a missing log, verified by running
under a fake
HOME.Two things worth knowing for future edits
file returns its AWS fixtures redacted; an edit written against that text
replaces them with placeholder strings, which then fails the test. It
happened during development and is now documented in the file itself.
the source trips GitHub push protection — correctly. Concatenation keeps the
scanner quiet while gitleaks still receives a complete key.
Coverage limits
Tool results only, and honest about it. Assistant message text has no hook
that rewrites model output; attachments arrive by another path. Both remain
uncovered, noted in #537.
Advances #537.
https://claude.ai/code/session_01PTUKVrdfDTU3ZH149e9pTY