Skip to content

security(audit): redact nested arguments and result previews (#397, #525) - #657

Open
harrylettering wants to merge 1 commit into
andrewyng:mainfrom
harrylettering:fix/audit-log-redaction
Open

security(audit): redact nested arguments and result previews (#397, #525)#657
harrylettering wants to merge 1 commit into
andrewyng:mainfrom
harrylettering:fix/audit-log-redaction

Conversation

@harrylettering

Copy link
Copy Markdown

Two independent leaks put cleartext into audit_events in coworker.db.

#397 — arguments were only checked one level deep

_sanitize_args inspected top-level keys and handed everything else to _summarize, which recursed into dicts and lists without re-applying the policy. A credential in a nested headers / auth / config object was written verbatim, and _truncate keeps 500 characters, so a bearer token survived whole. The key list also missed authorization, cookie, credential and private_key — none of which contain token.

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_preview was stored with _truncate and nothing else, while the same data was being redacted on the argument side. An email_read body, a message's text, and run_shell stdout from printenv all persisted in the clear and were re-served by AuditStore.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:

email_read  -> {"ok": true, "subject": "Q3 numbers", "body": "[redacted body]"}
run_shell   -> {"command": "printenv", "exit_code": 0, "output": "[redacted body]"}

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: nested headers / config credentials, the four newly covered key names, a nested body, a credential below the walk limit, browser_type input, 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

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant