security(audit): redact nested arguments and result previews (#397, #525) - #657
Open
harrylettering wants to merge 1 commit into
Open
security(audit): redact nested arguments and result previews (#397, #525)#657harrylettering wants to merge 1 commit into
harrylettering wants to merge 1 commit into
Conversation
Two independent leaks put cleartext into `audit_events` in coworker.db. Arguments (andrewyng#397): `_sanitize_args` checked top-level keys only and handed everything else to `_summarize`, which recursed without re-applying the policy, so a credential in a nested `headers` / `config` object was written verbatim. The key list also missed `authorization`, `cookie`, `credential` and `private_key`, none of which contain "token". The walk is now one recursive pass that checks every key at every level, with a depth limit that drops what it stops checking rather than stringifying it through. Result previews (andrewyng#525): `result_preview` was stored with `_truncate` and no redaction at all, so an email body, a message's text, or `run_shell` stdout from `printenv` persisted in the clear. The preview is now rebuilt in the store from the sanitized result structure, under the argument policy plus the result-side content keys (output, stdout, stderr, text, snippet). Envelope fields survive, so a row still says what ran and against what. Fixes andrewyng#397 Fixes andrewyng#525 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HYvy9TcuTD4CZMVcdwwH4D
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.
Two independent leaks put cleartext into
audit_eventsincoworker.db.#397 — arguments were only checked one level deep
_sanitize_argsinspected top-level keys and handed everything else to_summarize, which recursed into dicts and lists without re-applying the policy. A credential in a nestedheaders/auth/configobject was written verbatim, and_truncatekeeps 500 characters, so a bearer token survived whole. The key list also missedauthorization,cookie,credentialandprivate_key— none of which containtoken.The walk is now a single recursive pass that checks every key at every level, for both the secret keys and the body keys. It carries a depth limit; past it the value is dropped as
[nested]rather than stringified through, since the keys are no longer being checked there.#525 — result previews were not redacted at all
result_previewwas stored with_truncateand nothing else, while the same data was being redacted on the argument side. Anemail_readbody, a message's text, andrun_shellstdout fromprintenvall persisted in the clear and were re-served byAuditStore.list().The preview is now rebuilt inside the store from the sanitized result structure, so redaction happens while the keys still exist rather than after the string has been flattened. The policy is the argument one plus the result-side content keys:
output,stdout,stderr,text,snippet. Envelope fields survive, so a row still supports triage:When an event carries no raw
result(the non-tool stages the manager appends), the caller's preview string is stored as before.Tests
tests/test_audit_redaction.py, eight cases: nestedheaders/configcredentials, the four newly covered key names, a nested body, a credential below the walk limit,browser_typeinput, the two result-preview shapes above, and the no-raw-result fallback.Full suite: 28 failures before and after this change on my machine (Slack relay, browser address guard, UI e2e — network and Playwright dependent), no new ones.
Known limit
A tool that returns a bare string result has no key to judge, so its preview is still stored as text. Every result shape in the tree today is a dict.
Fixes #397
Fixes #525
🤖 Generated with Claude Code
https://claude.ai/code/session_01HYvy9TcuTD4CZMVcdwwH4D