Skip to content

fix(hook): secret redaction was broken — rewrite via Python + add tests#16

Merged
anthroos merged 1 commit intomainfrom
fix/post-tool-use-redaction
May 2, 2026
Merged

fix(hook): secret redaction was broken — rewrite via Python + add tests#16
anthroos merged 1 commit intomainfrom
fix/post-tool-use-redaction

Conversation

@anthroos
Copy link
Copy Markdown
Owner

@anthroos anthroos commented May 2, 2026

TL;DR

The PostToolUse hook claimed to redact Bearer X, api_key=…, password=…, token=… etc from observation summaries before writing them to ~/.openexp/observations/. A code review found the redaction was broken on the most realistic inputs, including Anthropic API keys (sk-ant-api03-…).

The bug

Empirically verified before this PR (run on macOS, but reproduces identically on Linux GNU sed):

Input Old output Status
api_key="abc12345xyz_secretvalue" api_key="abc12345xyz_secretvalue" NOT redacted
Bearer sk-ant-api03-veryverysecret Bearer [REDACTED]-ant-api03-veryverysecret API key leaks
token=secretvalue1234567890 token=[REDACTED]4567890 Tail leaks
password="hunter2hunter2" password=[REDACTED]" Trailing quote leaks
ANTHROPIC_API_KEY=sk-ant-… npm test unchanged Inline env-var form not caught at all

Three concrete root causes:

  1. The keyword pattern used \047 for ' inside a POSIX bracket expression. Both BSD sed (macOS) and GNU sed interpret \047 as the literal characters \, 0, 4, 7 there, not as an escaped single-quote. The character class never matched the quote forms.
  2. The Bearer character class [A-Za-z0-9_\.\-\/+=]+ had unnecessary backslash escapes that fouled the match — only sk consumed before failing on -.
  3. Inline env-var form (MY_TOKEN=abc curl …) was never targeted at all; the previous heuristic only caught export TOKEN=….

Plus two secondary issues caught in the same review:

  • The ls* skip-list pattern matched lsof, lsblk, lsattr and dropped their observations.
  • The Bash branch only redacted SUMMARY. The full COMMAND also gets persisted into context.command in the JSONL — uncovered.

Fix

redact_secrets() rewritten in Python via python3 -c. sed character classes diverge between BSD and GNU; Python regex is portable, readable, and tested. Three rules:

# 1. ANTHROPIC_API_KEY=sk-ant-…, MY_TOKEN=abc, GH_PASSWORD=…
re.sub(r"(^|\s)([A-Z][A-Z0-9_]*(?:TOKEN|SECRET|KEY|PASSWORD|API|PASS|PWD|AUTH)[A-Z0-9_]*)\s*=\s*\S+", r"\1\2=[REDACTED]", s)

# 2. token=val, password="val", api_key: val (case-insensitive)
re.sub(r"(token|password|api[_-]?key|secret|credential|auth)\s*[:=]\s*[\"\x27]?[^\s\"\x27]{4,}[\"\x27]?", lambda m: m.group(1) + "=[REDACTED]", s, flags=re.IGNORECASE)

# 3. Bearer X, sk-…, ghp_…, AKIA…
re.sub(r"Bearer\s+[A-Za-z0-9._/+=\-]+", "Bearer [REDACTED]", s)
re.sub(r"\bsk-[A-Za-z0-9_\-]{16,}", "[REDACTED]", s)
re.sub(r"\bghp_[A-Za-z0-9]{20,}", "[REDACTED]", s)
re.sub(r"\bAKIA[A-Z0-9]{16}\b", "[REDACTED]", s)

Other changes:

  • Skip-list tightened: ls / ls * instead of ls*. pwd / pwd *. Adds grep and rg.
  • Bash branch now redacts COMMAND before deriving SUMMARY. Both on-disk fields are clean.
  • Drops the brittle if grep ... TOKEN|SECRET heuristic — replaced by the rule (a) regex which is more general and tested.

Tests

New tests/test_post_tool_use_hook.sh — 19 assertions covering the six previously-broken redaction cases, the read-only skip list (including the lsof/lsblk regression), and sensitive-file-path skips on Write. Passes 19/19 against this PR; would fail 6+ on the prior version.

=== Secret redaction (Bash) ===
  PASS  api_key= form
  PASS  Bearer sk-ant-...
  PASS  --token=val
  PASS  --password="val"
  PASS  inline ENV=val
  PASS  export GITHUB_TOKEN
=== Read-only commands skipped ===
  PASS  ls -la / find / cat / pwd / echo / head
=== Non-read-only commands captured (regression: ls* glob) ===
  PASS  lsof -i :8080
  PASS  lsblk
=== Sensitive file paths skipped (Write) ===
  PASS  .env / .env.local / credentials.json / .ssh/id_rsa / *.pem

Performance impact

Each Bash tool call now invokes python3 once (~50ms cold). Acceptable for a hook that fires on Edit/Write/Bash. The hook still fast-exits on read-only commands before reaching Python — no overhead for ls -la, cat, find.

Severity / impact on existing users

Anyone running OpenExp with the previous hook has potentially leaked secrets from Bash tool calls into ~/.openexp/observations/observations-YYYY-MM-DD.jsonl on their own disk. The file is in $HOME with default permissions (700 typically), so not network-exposed — but if a user's transcripts get backed up, synced, or shared, the secrets travel with them.

After merge, recommend a follow-up issue to scan/redact existing observation files. Tracked separately.

The PostToolUse hook claimed to redact bearer tokens / api keys /
passwords from observation summaries before writing them to disk.
A code review found the redaction was broken in three concrete ways:

  1. The keyword=value sed pattern used `\047` to mean a single quote
     inside a POSIX bracket expression. Both BSD sed (macOS) and GNU
     sed treat `\047` as literal characters there, so `api_key="..."`
     and `password="..."` patterns never matched. Verified empirically:
     `api_key="abc12345xyz_secretvalue"` came through unchanged.

  2. The Bearer pattern's character class (`[A-Za-z0-9_\.\-\/+=]+`)
     had unnecessary backslash escapes that fouled the match. Result:
     `Bearer sk-ant-api03-realsecret` was redacted as
     `Bearer [REDACTED]-ant-api03-realsecret` — the `sk-` got eaten,
     the rest of the API key leaked into observations on disk. This
     is the actual Anthropic key prefix and the most realistic case.

  3. Inline-env-var assignments (`MY_TOKEN=abc curl ...`) were never
     caught — only `export TOKEN=...` form. Common shell pattern,
     fully bypassed redaction.

Plus secondary issues:

  4. The `ls*` skip-list pattern matched `lsof`, `lsblk`, `lsattr` and
     other non-read-only commands, dropping their observations.
  5. The Bash branch only redacted SUMMARY, not COMMAND. The full
     command also gets persisted into context.command in the JSONL.

Fix:

- `redact_secrets()` helper rewritten in Python via `python3 -c`
  (sed character classes diverge between BSD and GNU; Python regex
  is portable and verified). Three rules:
    a. inline ENV=val with secret-implying variable name → REDACTED
    b. keyword (token/password/api_key/secret/credential/auth) =
       value (4+ non-space/quote chars) → REDACTED
    c. Bearer X, sk-..., ghp_..., AKIA... → REDACTED
- Read-only skip patterns tightened: `ls` and `ls *` instead of `ls*`,
  same for `pwd`. Adds `grep` and `rg` to the skip list.
- Bash branch now redacts COMMAND before deriving SUMMARY, so both
  on-disk fields are clean.
- Drops the brittle `if grep ... TOKEN|SECRET` heuristic — replaced
  by the rule (a) regex, which is more general and tested.

Adds tests/test_post_tool_use_hook.sh — 19 assertions covering all
six redaction cases that previously failed, the read-only skip list
(including the lsof/lsblk regression), and sensitive-file-path skips
on Write. Passes 19/19 against the new hook; would fail 6+ on the
prior version.

Performance: each tool call now invokes python3 once or twice. ~50ms
overhead, acceptable for a hook that fires on Edit/Write/Bash. The
hook is still non-blocking and fast-exits on read-only commands
before reaching Python.
@anthroos anthroos merged commit 5d4c5f9 into main May 2, 2026
@anthroos anthroos deleted the fix/post-tool-use-redaction branch May 2, 2026 05:15
anthroos pushed a commit that referenced this pull request May 2, 2026
Security-relevant: existing users should scrub
~/.openexp/observations/ if they installed before today. PR #16
fixed a redaction regression that let Bearer/api_key/password
values reach disk verbatim. Entry documents the bug, the fix, and
gives users a concrete grep + rm recipe to clean up old
observations.
anthroos added a commit that referenced this pull request May 2, 2026
Security-relevant: existing users should scrub
~/.openexp/observations/ if they installed before today. PR #16
fixed a redaction regression that let Bearer/api_key/password
values reach disk verbatim. Entry documents the bug, the fix, and
gives users a concrete grep + rm recipe to clean up old
observations.

Co-authored-by: Ivan Pasichnyk <ivanpasichnyk@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant