fix(audit,query): redact secrets in audit log fields and @query errors (#137)#160
Open
tcconnally wants to merge 1 commit into
Open
fix(audit,query): redact secrets in audit log fields and @query errors (#137)#160tcconnally wants to merge 1 commit into
tcconnally wants to merge 1 commit into
Conversation
#137) Pre-1.0.6, calls like `@query "curl -H 'Authorization: Bearer ghp_…'"` produced correctly-redacted render output BUT persisted the raw bearer token in `~/.perseus/audit_log.jsonl` (via the `command` field), and leaked the same secret in `@query` error/timeout/no-output messages back into render output. Render-time redaction only applies to the final assembled output, not to audit fields or to error strings constructed before the redaction pass runs. Two fixes: src/perseus/audit.py: - audit_event() now passes every user-supplied field value through redact_text() before serializing to JSONL. Structural fields (directive, exit_code, duration_ms, pid, etc.) are exempt via an explicit allowlist (_AUDIT_NEVER_REDACT_KEYS) — they are never user-supplied secrets. - New _audit_redact_value() helper walks nested dicts and lists recursively. - New config knob audit.redact_fields (default true). Operators can set false for forensic mode where the audit log is itself the secured artifact. - On redact_text failure, fall back to raw value rather than dropping the audit entry — observability beats perfect redaction, and rendered output is the primary defense. src/perseus/directives/query.py: - Exit-nonzero header: cmd + stderr now passed through redact_text before interpolation. - No-output message: cmd redacted. - TimeoutExpired branch: cmd redacted. - Generic exception branch: str(exc) redacted (exc.args often includes the full cmd via shell argv). Tests (tests/test_audit_log.py): - test_audit_event_redacts_aws_key_in_command_field - test_audit_event_redacts_bearer_token_in_command_field - test_audit_event_does_not_redact_structural_fields - test_audit_event_redact_fields_can_be_disabled (forensic opt-out) - test_audit_event_walks_nested_dict_fields Test results: - All 5 new regression tests pass. - tests/test_audit_log.py parity vs main confirmed (6 pre-existing failures on both branches — unrelated to this change; tracked separately). - tests/test_redaction.py: 21/21 pass (unchanged). Closes #137 Refs milestone v1.0.6
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
2 participants
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.
Summary
Closes #137 — second item in the v1.0.6 hotfix bundle (after #136 / PR #159).
Pre-1.0.6,
@query "curl -H 'Authorization: Bearer ghp_…'"produced correctly-redacted rendered output BUT:~/.perseus/audit_log.jsonl(via thecommandfield of theshell_execaudit event)@queryerror/timeout/no-output messages back into render outputRender-time redaction only runs on the final assembled output — not on audit fields, and not on error strings constructed before the redaction pass.
Fix
src/perseus/audit.pyaudit_event()now passes every user-supplied field value throughredact_text()before serializing to JSONLdirective,exit_code,duration_ms,pid, etc.) are exempt via explicit allowlist_AUDIT_NEVER_REDACT_KEYS— they are never user-supplied secrets_audit_redact_value()walks nested dicts and lists recursivelyaudit.redact_fields(defaulttrue); operators can opt out for forensic mode where the audit log is itself the secured artifactredact_textfailure, fall back to raw value rather than dropping the audit entry — observability beats perfect redactionsrc/perseus/directives/query.pycmd+stderrpassed throughredact_textbefore interpolationcmdredactedTimeoutExpiredbranch:cmdredactedstr(exc)redacted (exc.argsoften includes fullcmdvia shell argv)Tests
5 new regression tests in
tests/test_audit_log.py:test_audit_event_redacts_aws_key_in_command_fieldtest_audit_event_redacts_bearer_token_in_command_fieldtest_audit_event_does_not_redact_structural_fieldstest_audit_event_redact_fields_can_be_disabled(forensic opt-out)test_audit_event_walks_nested_dict_fieldsTest results
tests/test_audit_log.pyparity vsmainconfirmed: 6 pre-existing failures on both branches (tracked separately; unrelated to this change)tests/test_redaction.py: 21/21 pass unchangedCHANGELOG
New
[1.0.6] — UNRELEASEDsection added. Note: PR #159 (for #136) also adds a 1.0.6 section. Whichever PR merges second should reconcile the two entries.Migration
No config breaking changes. The new
audit.redact_fieldsdefault oftrueis strictly more secure than pre-1.0.6 behavior.Forensic Mode
Operators who specifically need the audit log to contain raw secrets (for incident forensics where the log is itself secured and access-controlled) can opt out:
Second of 12 PRs in the v1.0.6 milestone. Suggested next: #129 (trust profile override) or #138 (query timeout leak).