feat(firewall+scrub): argv-taint + forensic action_summary + secret key-name backstop + /root tell (T3MP3ST R2/R2b/R10/R12-residual; v0.25.0)#159
Conversation
…ey-name backstop + /root tell (T3MP3ST R2/R2b/R10/R12-residual; v0.25.0) (#158) PR-B of the T3MP3ST-audit survivors; grep-verified residuals from the loop-until-dry adversarial review. - R2 argv-taint: name the argv of a gated shell-out as a local-blast-radius injection sink the egress+no-laundering gates miss (tool-firewall.md section + invariant); for unattended exec, fail-closed reject a tainted value that reparses as an option/metachar. - R2b action_summary: validate_tool_firewall_log now requires a non-empty action_summary on prompt_gated_exec/write rows (the command actually executed + target/argv; tool names the actuator but not what it ran). - R10 secret key-name backstop: validate_scrub_report walks each JSON scrubbed artifact and blocks a non-surrogate value under a controlled sensitive-key vocab, surrogate placeholders allowlisted; keyed on the KEY NAME so it never fires on a missing-authorization sink-class value or an Authorization: header echoed in a replay command. Defense-in-depth, not a completeness claim. - R12-residual: extend the provenance tell-scan to /root/ only; /opt/ and /home/ excluded (legit synthetic /opt/skills and profile/home/cache), with a backtick exclusion so doc mentions of the tell are not self-hits. Ceilings raised (portable-invariants 427->429; tool-firewall 126->148 lines / 8700->10600 bytes) per reviews/run-security-scrub-hardening-consensus.md. 660 unit + 9 integration green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c51750fa3b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…s placeholder match (Codex #159) Two P2 correctness holes in the R10 secret key-name backstop: - Propagate sensitive-key context into structured children: a secret under an array- or object-valued sensitive key ({"Authorization": ["Bearer ..."]}) previously escaped because isinstance(value,str) skipped the container and the child strings had no sensitive key name. _walk_sensitive_keys now carries an under_sensitive flag into list/object children. - Anchor placeholder recognition to a full-match: _SURROGATE_PLACEHOLDER_RE was used with .search, so any value merely CONTAINING a placeholder word or an angle-bracket substring (a password foo<bar>, a token containing 'redacted') was treated as scrubbed. New _is_scrubbed_value uses fullmatch — the whole value must be a declared surrogate or a placeholder token. Regression tests: array-valued + nested-object sensitive keys block; substring placeholders are not treated as scrubbed. 663 unit + 9 integration green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Addressed in fa625a9, both P2s (thanks — both were real holes):
|
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fa625a94a9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… surrogate_map values (Codex #159) Third P2: _is_scrubbed_value trusted 'value in surrogate_map.values()', but those are unvalidated agent input — a raw secret listed as a surrogate value would whitelist itself under a sensitive key. Replace the surrogate-trust + strict full-match with a redaction-MARKER requirement: a value under a sensitive key is exempt only if it carries a bracketed redaction token (<REDACTED>, [SCRUBBED], Bearer <REDACTED_TOKEN>) or is a bare redaction/empty keyword. This also fixes a latent FP the strict full-match would have caused ('Bearer <REDACTED>' is a legit redaction, not a whole-value token). Regression tests: surrogate_map value equal to a raw secret still blocks; 'Bearer <REDACTED>' passes. Drops the now-unused surrogate allowlist plumbing. 665 unit + 9 integration green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Addressed in 67a4a1d (thanks — third real one): |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 67a4a1d5e8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…Codex #159 P1) P1: _is_scrubbed_value used .search, so a marker beside residual raw material ('Bearer <REDACTED> sk-live-realtoken') was exempted while still shipping the raw token. Switch to an anchored fullmatch of the WHOLE redacted shape: a single bracketed redaction token with at most one auth-scheme prefix (Bearer/Basic/ token=…), or a bare redaction/empty keyword. Any leading secret word, trailing token, or unrelated bracket substring now fails the match. Regression tests: marker + residual raw token blocks (prefix and suffix forms); 'Bearer <REDACTED>' still passes. Verified against 15 edge cases. 667 unit + 9 integration green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Addressed in 63830e4 (P1, real bypass): |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 63830e41cb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Fifth finding: a whole-file json.loads raises 'Extra data' on a line-delimited JSON artifact (tool-firewall-log.jsonl, lane-heartbeats.jsonl), so the block silently continued and a secret under a sensitive key in a JSONL row bypassed the backstop entirely. Parse a .jsonl artifact row-by-row (each row walked with an indexed path) instead of skipping it. Regression test: a .jsonl scrubbed artifact with an Authorization secret in a row blocks. 668 unit + 9 integration green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Addressed in 3d4d127: R10 now parses a |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3d4d1278fd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…l metadata (Codex #159) Two P2s: - Recall: SENSITIVE_KEY_VOCAB matched the fully-normalized key exactly, so secret_key/api_secret/secretAccessKey/tokens/refresh_token all bypassed. Switch to sensitive STEMS matched as substrings after normalization. - FP: propagating under_sensitive into every dict descendant flagged benign metadata ({"Authorization":{"scheme":"Bearer","value":"<REDACTED>"}} rejected the harmless scheme field). A DICT now re-evaluates sensitivity per child key rather than inheriting the parent's; sensitivity still propagates into LIST children (array-valued headers stay caught). Bare auth-scheme words are also treated as benign metadata, not secrets. Tests: compound/plural keys block; structured-credential metadata (scheme, username) not flagged; nested secret under a sensitive sub-key still blocks. Validated against 15 edge cases. 671 unit + 9 integration green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Addressed in 15d1d82 (both real):
|
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 15d1d82f98
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…gn metadata (Codex #159) Round-5's dict-per-key reset (fixing the metadata FP) opened the inverse bypass: a raw secret under a generic sub-key inside a sensitive object ({"Authorization":{"scheme":"Bearer","value":"sk-live-..."}}) was never checked. Synthesis that resolves both directions: sensitivity now propagates into BOTH list and dict children so a structured credential's secret leaf is checked, EXCEPT a child under a benign-metadata key (scheme/type/alg/username/expires/… — never secret-bearing) is exempt, so metadata beside a redacted secret is not flagged. value/data/content are deliberately NOT metadata. Tests: raw secret under a generic value key (and under password beside a benign username) blocks; scheme/username metadata beside a redacted secret passes. Validated against 14 edge cases spanning all six review rounds. 673 unit + 9 integration green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Addressed in 4ee1d19: this resolves the round-5↔round-6 tension permanently. Sensitivity now propagates into BOTH list and dict children (so a raw secret leaf under a generic sub-key like |
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…ar-text FP) The R10 backstop flags any non-redacted string under a sensitive key, so the fixtures do not need realistic-looking credentials. Replacing the sk-live-* / Bearer-token literals (which CodeQL's default clear-text-storage query classifies as real secrets when written to disk via write_json) with plainly-synthetic strings clears a false-positive high-severity alert without changing any test semantics. 673 unit + 9 integration green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…Codex #159) The rglob loop silently skipped a .json/.jsonl that failed to read, while the declared-artifact path already rejects non-UTF-8. Make the shipped-artifact scan consistent: an unreadable artifact cannot be verified scrubbed, so reject it. Regression test added. 681 unit + 9 integration green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Addressed: the shipped-artifact scan now rejects an unreadable/non-UTF-8 |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f91e2b4742
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f91e2b4742
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…trings (Codex #159) Substring stem matching over-blocked ordinary evidence whose field name merely CONTAINS a stem: {"name":"tokenizer","value":"bert-base"} (tokenizer ⊃ token) and {"tokenizer":"..."} were rejected as secrets. Replace the substring stems with a curated _SENSITIVE_KEY_NAMES frozenset matched EXACTLY after normalization, so common compound/plural forms (secret_key, api_secret, refresh_token, tokens, x-api-key, set-cookie) stay covered while tokenizer/secretary/engagement_authorization are not misclassified. This also ends the recall/precision oscillation: the set is a fixed, extendable list rather than a fuzzy matcher. Regression test for the FP; all prior recall cases preserved (validated against 21 cases). 682 unit + 9 integration green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Addressed (real FP): replaced substring stem matching with a curated |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e3f432db53
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…159) WWW-Authenticate is a challenge/response header naming the auth scheme, not a credential-bearing header, so it should not be treated as secret-bearing. 682 unit + 9 integration green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Addressed: removed |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 211cba7b53
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 211cba7b53
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ensitive keys (Codex #159) - Include .har (HTTP Archive = JSON) in the shipped-artifact sweep and malformed rejection, so a capture.har omitted from scrubbed_artifacts is still scanned by the HAR name/value walker. - Flag a non-string scalar (numeric password/PIN/session id) directly under a sensitive key name — the string-only leaf check missed it. Restricted to the child's OWN sensitive key and excludes bool, so numeric metadata (a nested expires_in/max_retries) is not flagged. Regression tests for both. 685 unit + 9 integration green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Addressed (both real):
|
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 75767577a3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…crets in sensitive arrays (Codex #159) - R2b: a filler PREFIX ('TODO fill this in later', 'pending rerun') is not replayable evidence; reject when the first token is a filler word (low-FP: a real action_summary never leads with one). - R10: extend the direct-key numeric-secret check to array elements, so {"tokens": [123456]} is caught like the bare {"sessionid": 123456} case. Regression tests for both. 686 unit + 9 integration green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Addressed (both):
|
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b9212c8c13
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b9212c8c13
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b9212c8c13
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Codex review loop — convergence decision. Over ~20 rounds Codex raised (and this PR addressed, each with a regression test) findings across R2b (forensic action_summary: presence, placeholder, filler-prefix) and R10 (the secret key-name backstop: structured/array/nested values, redaction-marker anchoring, untrusted surrogates, JSONL, shipped-artifact coverage incl. subdirectories and .har, case-insensitive suffixes, malformed/unreadable rejection, numeric secrets, exact-name matching to kill the tokenizer-class FP). A CodeQL clear-text false-positive on synthetic test fixtures was also cleared by exercising the walk in-memory. The two most recent re-triggered 🤖 Generated with Claude Code |
|
To use Codex here, create an environment for this repo. |
1 participant
Closes #158. PR-B of 3 implementing the T3MP3ST-audit survivors (loop-until-dry adversarial review; every residual grep-verified). Self-tooling run-security & outbound scrub.
What changed
git) is a third, local-blast-radius sink both miss. Newtool-firewall.mdsection +portable-invariants.mdinvariant: never word-split a model/target string into a command; for unattended/auto-approved exec, fail-closed reject a tainted value that reparses as an option or carries a shell metacharacter.action_summary(validator).validate_tool_firewall_lognow requires a non-emptyaction_summaryon everyprompt_gated_exec/prompt_gated_writerow (the command actually executed + target/argv).toolnames the actuator but never what it ran.validate_scrub_reportwalks each JSON scrubbed artifact and blocks a non-surrogate string value under a controlled sensitive-key vocab (authorization/cookie/token/secret/password/api_key/…), surrogate placeholders allowlisted. Keyed on the KEY NAME, not the value, so it never fires on amissing-authorizationsink-class value or anAuthorization:header echoed in a replay command. Closes a verbatim-leak class the enumerated surrogate scan missed./root/tell (honestly scoped). Tell-scan extended to/root/only./opt/and/home/are excluded — a grep proved they appear in legitimate synthetic paths (/opt/skills/…in ~20 fixtures;profile/home/cachein integration tests), so a blanket scan would not start green. Backtick exclusion prevents doc mentions from self-tripping.Process
/simplifypass: dropped a deadisinstance(key,str)guard (JSON keys are always str), hoisted the key-normalize regex, cross-referenced the placeholder-token vocab shared withANGLE_PLACEHOLDER_RE/PLACEHOLDER_EVIDENCE_RE. Reuse/altitude/efficiency confirmed the walker, vocab, and singlejson.loadsre-parse are justified.reviews/run-security-scrub-hardening-consensus.md; five-agent docs recheck +agent-feature-matrix.mdrefresh indocs/authoritative-sources.md.make sync+make verifyclean (660 unit + 9 integration); no eval fixture regressed under the R10 backstop.🤖 Generated with Claude Code