fix(guardrails): close PowerShell-tool bypass of Bash-matched guards - #960
Conversation
The git/commit guards matched only the Bash tool, so `git commit --no-verify` (and the other guarded forms) ran unblocked through Claude Code's opt-in PowerShell tool, which surfaces its command in the same tool_input.command field — a bypass proven live on Windows (#912 f1). Widen the PreToolUse matchers for block-no-verify, block-noncanonical- commit, block-dangerous-git, block-hook-bypass, and flag-commit-pr-skill- bypass from `Bash` to `Bash|PowerShell`, and add a guardrails-local classifier (lib/powershell/ps-command.sh) that reduces a PowerShell command to a Bash-tokenizer-faithful form or fails closed: - Here-strings are blanked to an inert placeholder, so the canonical PowerShell commit form (a here-string piped to `git commit -F -`) reduces to `<placeholder> | git commit -F -` and is allowed exactly as the Bash `-F -` form; `git commit -m @'...'@` reduces to `git commit -m <placeholder>` and is blocked. - A `git commit`/`git push`-shaped PowerShell command carrying a construct the Bash tokenizer cannot faithfully handle (backtick, `--%`, subexpression, script-block grouping, or an unbalanced here-string) is blocked fail-closed rather than waved through. The blanker is over-block- never-under-block on ambiguity, so a trailing pipeline can never be swallowed into the placeholder. - block-hook-bypass gains PowerShell file-write coverage (Set-Content, Add-Content, Out-File, Tee-Object, and content-producer `>`/`>>` redirects), producer-scoped like the Bash detection. - Block messages are shell-agnostic (f3): block-noncanonical-commit shows the here-string form on the PowerShell tool, not a Bash heredoc. Full PowerShell grammar parsing (faithful here-strings beyond the canonical form, backticks, `--%`, subexpressions) and content scanning of PowerShell writes are deferred to the A2b follow-up. Closes #915. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
|
![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: 518fce7677
ℹ️ 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".
block-dangerous-git owns destructive non-commit forms (reset --hard, clean -fd, checkout/restore), but the PowerShell classifier deferred (allowed) every non-commit/push command it could not parse. An unparseable git-shaped PowerShell command such as `git --% reset --hard` or a backtick-continued `git reset --hard` therefore ran unblocked — a fail-open in a security guard. Add a `git` danger-shape to ps::classify_git_command (via ps::is_git_shaped) and point block-dangerous-git at it, so the guard fails closed on ANY git-shaped PowerShell it cannot faithfully tokenize, not only commit/push. Non-git unparseable PowerShell stays this guard's non-concern (allowed). A dedicated ps::print_unparseable_git_block_message replaces the commit/push-worded message on this guard's block path. The commit/push guards keep the default shape and are unchanged.
|
Coverage-bar decision needed (tower, ~07:25Z) — morning-queue ratify item next to #903. After three fix rounds, the PowerShell-tool guard's classifier kept yielding new invocation vectors (iex, Start-Process/saps, pwsh/cmd -Command, cmdlet aliases) — the same unbounded-blocklist wall #903 hit. Recalibration (advisor-confirmed): the guard's bar is Bash-parity, not airtight. These guards are accidental-destruction friction, not a boundary against deliberate evasion — and the Bash guard #960 extends does NOT stop deliberate evasion either (e.g. The working bar (fixer completing one bounded parity round): PS guard covers what the BASH guard sees through — Operator decision at ritual: ratify the Bash-parity + documented-residuals coverage bar for the PowerShell guard (consistent with the friction-not-boundary threat model your #915 confirm established) — or direct a different bar (e.g. a fail-closed/allowlist posture on the PS surface, which over-blocks a general shell and was advised against). #960 stays operator-held (area:security) regardless; this makes it coherent + ratifiable rather than an endless vector chase. One open FACT the fixer is verifying: whether the Claude Code PowerShell tool can run Windows PowerShell 5.1 (where |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8301587023
ℹ️ 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".
|
|
…ed net Branch hygiene: this branch had two parallel commits on the round-1 base (518fce7) — a laptop lane's "fail closed on any git-shaped unparseable PowerShell" (8301587) and the tower's Bash-parity hardening. Fold them into one coherent branch so the PR is a clean, comparable single-branch implementation. - ps-command.sh: the tower's git-presence sink (ps::might_invoke_git) is the base — it already fails closed on ANY git-shaped unparseable PowerShell for every git guard, so the lane's separate `shape` parameter / ps::is_git_shaped is redundant and dropped. The lane's dedicated block message (ps::print_unparseable_git_block_message, which names the destructive reset/clean/checkout forms) is kept — a genuine improvement over the generic commit-form message for block-dangerous-git. - block-dangerous-git.sh: calls the classifier without the now-redundant shape arg and uses the dedicated message. - block-dangerous-git.test.sh: the lane's five PowerShell regression tests are kept verbatim (git --% reset --hard, git --% clean -fd, backtick-continued reset --hard, checkout via subexpression, and a non-git negative control) — all pass on the tower's sink (200 -> 205, no failures), the behavioral proof the git-presence sink subsumes the lane's git-shaped fix. - CHANGELOG: one unified 0.9.9 entry crediting both the parity hardening and the wider git-shaped fail-closed net, plus the Export-ModuleMember over-block note. All suites green: block-dangerous-git 205, block-no-verify 112, block-hook-bypass 141, block-noncanonical-commit 57, flag-commit-pr-skill-bypass 27 (0 failures). shellcheck/shfmt clean; sync-hook-utils and changelog-parity green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1f802291d8
ℹ️ 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: 2dfea0438f
ℹ️ 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".
|
Claude finished @kyle-sexton's task in 5m 43s —— View job PR Review: fix(guardrails): close PowerShell-tool bypass of Bash-matched guards
|
|
Claude finished @kyle-sexton's task in 10m 40s —— View job Security Review — PR #960
Two confirmed bypass vulnerabilities in CRITICAL — False here-string opener swallows hidden git commands into the placeholderSeverity: CRITICAL · Confidence: CONFIRMED The opener detection A single-quoted PowerShell string like Confirmed bypass — valid PowerShell, passes every git guard: $x = 'foo @'
git commit --no-verify
$y = @'
commit message
'@ | git commit -F -Blanker trace:
PowerShell actually executes The same construction hides Root cause: The opener check operates on the raw (non-quote-stripped) line. A Mitigation: Strip single-quoted spans (and only single-quoted ones, where nesting is impossible in PS) before testing for the opener. A line-final IMPORTANT —
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dd550e026e
ℹ️ 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".
…nd 4 Bot review on the restacked #960 surfaced five within-parity gaps in covered constructs: .exe-suffixed launcher spellings and the `start` alias skipped the fail-closed launcher sink; the `write` alias of Write-Output was not a redirect producer; module-qualified writer spellings missed the cmdlet match; parenthesized redirect producers fell through the head-only check; and a call/dot-source of a quoted writer name was erased by quote-blanking before detection. All five fixed red-first in ps-command.sh; quoted arbitrary-program calls stay allowed (documented shared residual, Bash-parity bar per the operator ruling on #915). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 5m 4s —— View job PR Review — HEAD
|
| Finding | Fix in ps-command.sh |
|---|---|
.exe-suffix launchers (cmd.exe, pwsh.exe, powershell.exe) |
(\.exe)? appended to launcher alternation — line 234 |
start alias for Start-Process |
Added |start to ps::has_launcher — same line |
write alias for Write-Output |
Added to redirect-producer case arm — line 413 |
Module-qualified writers (Microsoft.PowerShell.Management\Set-Content) |
\\ added to boundary class — line 348 |
Parenthesized redirect producers (('secret') > f) |
Paren-stripping before head check — lines 405–406 |
& 'Set-Content' / & 'Invoke-Expression' quoted writer calls |
Pre-blanking regex on lcq — lines 328–332 |
Each has a new test (block-dangerous-git, block-hook-bypass). ✅
Version is correctly at 0.10.3 vs. 0.10.2 on main; the 0.9.9 conflict noted in earlier comments is resolved.
Still open: two security findings from the prior review
The prior security review identified two vulnerabilities in ps::blank_herestrings. Neither is addressed in this commit. Both are confirmed against the current HEAD.
1. False here-string opener swallows hidden git commands (CRITICAL, unaddressed)
if [[ "$line" == *"@'" || "$line" == *'@"' ]]; thenAny line whose last two characters are @' triggers in_hs=1 — including a valid PS string literal like $x = 'foo @'. That's legal PowerShell: string value foo @. Once the false opener fires, every subsequent line is silently dropped as "body" until the real '@ of a later here-string fires the false close.
Confirmed bypass — valid PowerShell, no flags, all guards pass:
$x = 'foo @'
git commit --no-verify
$y = @'
commit message
'@ | git commit -F -Blanker trace against current code:
- Line 1 (
$x = 'foo @'): ends@'→ false opener.pending = "$x = 'foo __GUARDRAILS_PS_HERESTRING__",in_hs=1. - Line 2 (
git commit --no-verify): body — dropped. - Lines 3–4: body — dropped.
- Line 5 (
'@ | git commit -F -): false close.PS_BLANKED = "$x = 'foo __GUARDRAILS_PS_HERESTRING__ | git commit -F -".PS_HERESTRING_UNBALANCED=0.
No backtick, no (){}, no launcher, no dynamic invocation → classify_git_command returns 0. Bash tokenizer sees the second pipe segment git commit -F - (canonical stdin form) → guard allows. PowerShell simultaneously ran git commit --no-verify on line 2.
Root cause: The opener test runs on the raw (unquoted) line. The actual PowerShell grammar rule: the opener @' or @" must be the very first content of the token starting the sequence — it is not valid if it appears after other content in a quoting context. A line-final @' that follows an unmatched ' is inside a string literal, not a here-string opener.
Mitigation: Strip single-quoted spans from the raw line before testing for @'; a @' that disappears after stripping was inside a string literal. Alternatively, require that the @ immediately following a quote character is not preceded by any alphanumeric or _ character (the invariant that holds in practice for every here-string opener in the wild). Either approach is a one-pass sed pre-filter on $line.
2. Double-quoted here-strings (@"..."@) blank subexpressions before detection (IMPORTANT, unaddressed)
The blanker treats @"..."@ (double-quoted, interpolating) identically to @'...'@ (single-quoted, literal). In PowerShell, $(...) subexpressions inside @"..."@ execute. The blanker drops the body before ps::has_special_constructs can see the ( character — defeating the detection that would otherwise route to the fail-closed sink.
Confirmed bypass — valid PowerShell, all git guards and write gate pass:
$x = @"
$(git commit --no-verify)
"@Blanker trace: line 1 sets hs_quote='"', line 2 (body) is dropped, line 3 ("@) closes. PS_BLANKED = "$x = __GUARDRAILS_PS_HERESTRING__". No ( → has_special_constructs returns false → guard allows. PowerShell executed git commit --no-verify via the subexpression. The same construct hides git reset --hard, Set-Content, or any other guarded form.
Root cause: The single-quoted @'...'@ is the canonical commit-message form and safe to blank because its body is literal (no execution). The double-quoted @"..."@ is not literal — its body executes. Blanking it silences execution evidence.
Mitigation: Do not blank @"..."@. When hs_quote == '"', leave the body in place. ps::has_special_constructs then sees the $(...) and routes to the fail-closed branch. The canonical here-string commit form uses @'...'@ (literal), so this change does not affect the canonical pass-through — only @"..."@ changes behavior, and it becomes fail-closed rather than false-allowed.
Minor note (unchanged from prior review)
block-noncanonical-commit.sh:368: comment reads # non-commit PowerShell with an A2b-deferred construct. Return code 1 from ps::classify_git_command means "provably git-free" — it could be git log | Where-Object { … } (git, not commit). The logic is correct; the comment should read # provably git-free PowerShell with an A2b-deferred construct.
Summary
The round 4 commit closes all six correctness gaps from the prior review cleanly and with tests. The two security findings from the prior review (false here-string opener and @"..."@ subexpression bypass) remain open in the current HEAD and are the only material issues blocking merge. Both are fixable with targeted changes to ps::blank_herestrings (roughly 5–10 lines each), and the double-quoted fix actually simplifies the blanker. Neither requires any change to the fail-closed sink or the Bash-parity scope.
|
Claude finished @kyle-sexton's task in 7m 41s —— View job Security Review — PR #960 (round 5, HEAD c49a120)Tasks
Round 4 fixes confirmed (c49a120 vs dd550e0): Two prior security findings remain unpatched. One new finding was introduced by c49a120. CRITICAL — False here-string opener swallows hidden git commands into the placeholder (UNPATCHED)Severity: CRITICAL · Confidence: CONFIRMED if [[ "$line" == *"@'" || "$line" == *'@"' ]]; thenThis check is identical to dd550e0 — the finding from the prior security review is unpatched. The opener test fires on any raw line whose last two characters are Confirmed exploit — valid PowerShell, bypasses all five guards: $x = 'foo @'
git commit --no-verify
$y = @'
commit message
'@ | git commit -F -Blanker trace:
The Bash parser sees a pipeline with two segments: The same construction hides Root cause: The opener check operates on the raw, unquoted line. A Mitigation (from prior review, still applicable): Before testing for the opener, strip single-quoted spans from the raw line ( IMPORTANT —
|
| File | Line | Stale call |
|---|---|---|
block-no-verify.sh:234 |
234 | ps::print_unparseable_block_message |
block-noncanonical-commit.sh:364 |
364 | ps::print_unparseable_block_message |
block-dangerous-git.sh:758 |
758 | ps::print_unparseable_git_block_message |
The hooks do not use set -e, so the undefined-function call prints command not found to stderr and continues; exit 2 still fires and the block still works. This is not a security bypass, but users receive no informative block message when a PowerShell command hits the fail-closed sink on these three guards.
No other findings
The rest of the surface — ps::might_invoke_git (backtick recovery, quote-intact git probe, dynamic-invocation detection), ps::has_dynamic_invocation (iex / & 'str' / . 'str' forms), ps::has_launcher (now covers .exe suffix and start alias), ps::write_bypass (cmdlet detection, module-qualified spellings via \\ boundary, paren-unwrapping of redirect producers, & 'quoted-cmdlet' call-operator pre-check), and the hook injection surfaces (jq --arg for TOOL_NAME/COMMAND, CLAUDE_PLUGIN_ROOT sourcing) — carries no additional security issues beyond the three above.
Summary for operator
The CRITICAL and IMPORTANT findings were reported on dd550e0 and remain unfixed in c49a120, which addressed only the five code-review gaps from the bot pass. These two bypasses require deliberate multi-line PowerShell construction but use fully valid PS syntax and succeed against every hook widened by this PR. The SUGGESTION is a cosmetic regression from the c49a120 rename — blocks work but messages don't print.
|
CLAIM (control tower, 2026-07-22): this PR is tower-managed through merge per operator ruling (#607 decision batch). Head dd550e0 adjudicated canonical. Lanes: do NOT re-merge, restack, resolve threads, or merge here — merge-staleness protocol does not apply to this PR; the tower merges after suite certification + thread resolution. Claim releases at MERGED. |
|
Claude finished @kyle-sexton's task in 2m 31s —— View job Security Review — PR #960 (HEAD
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 82f0e43d39
ℹ️ 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".
…er shapes
Round-5 bot review: a launcher with a computed program name
(Start-Process ('g'+'it')), a call operator with a computed writer name
(& ('Set-'+'Content')), and expression-literal redirect producers
(36 > out.txt, [char]65 > out.txt) all evaluated expressions the guards
cannot resolve and slipped through as provably-safe. Each shape now takes
the fail-closed branch, mirroring the existing iex / call-of-variable
posture; attached-digit stream redirects (2>err.txt) and tool-output
redirects stay allowed. Red-first regressions in both suites.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 5m 26s —— View job PR Review — HEAD
|
| Finding | Fix |
|---|---|
Expression-valued launcher target (Start-Process ('g'+'it')) |
ps::might_invoke_git now routes any launcher whose argument is a (-expression or $var to fail-closed — line 198 |
Computed writer command name (& ('Set-'+'Content')) |
ps::write_bypass calls computed-target check on quote-intact lcq before blanking erases the expression — line 342 |
Expression-literal redirect producers (36 > out.txt, [char]65 > out.txt) |
'['* cast-head and spaced-numeric head now count as content producers in the redirect gate — lines 427–433 |
Tests added for all three — block-dangerous-git.test.sh and block-hook-bypass.test.sh both have green regression coverage. ✅
The 82f0e43 SUGGESTION (stale unparseable function call sites) remains fixed as confirmed last round. ✅
Two prior security findings remain unpatched
The CRITICAL and IMPORTANT findings have been reported on dd550e0, c49a120, dd0f4a8, 82f0e43, and now 058e09d without being addressed. The blanker code is unchanged from every prior round.
CRITICAL — False here-string opener swallows hidden git commands (UNPATCHED)
Confidence: CONFIRMED
if [[ "$line" == *"@'" || "$line" == *'@"' ]]; thenThis check fires on any raw line whose last two characters are @', including a valid single-quoted PowerShell string like $x = 'foo @' (value: foo @; last chars: @'). The false opener sets in_hs=1 and drops every subsequent line as "body" until the '@ of a real, legitimately-later here-string fires the false close — at which point that here-string's suffix (e.g. | git commit -F -) is grafted onto the pending line, producing output that looks like the canonical form and is allowed.
Confirmed bypass — valid PowerShell, all five widened guards pass at 058e09d:
$x = 'foo @'
git commit --no-verify
$y = @'
commit message
'@ | git commit -F -Blanker trace:
$x = 'foo @'— last two chars@', false opener fires.pending="$x = 'foo __GUARDRAILS_PS_HERESTRING__",in_hs=1.git commit --no-verify—in_hs=1, dropped.$y = @'— body, dropped.commit message— body, dropped.'@ | git commit -F -—first2="'@"matches false closer.out="$x = 'foo __GUARDRAILS_PS_HERESTRING__ | git commit -F -".in_hs=0.
PS_BLANKED="$x = 'foo __GUARDRAILS_PS_HERESTRING__ | git commit -F -". blank_quoted_spans: the unmatched ' before foo has no closing partner on the same line, so it is not stripped — the | git commit -F - suffix remains. has_special_constructs → false. has_dynamic_invocation → false. has_launcher → false. classify_git_command returns 0. Bash parser sees git commit -F - (canonical stdin form) as the second pipeline segment → all guards allow. PowerShell simultaneously ran git commit --no-verify on line 2.
The same construction hides git reset --hard, git push -f, or Set-Content from every guard widened by this PR.
Mitigation (unchanged from prior rounds): Strip single-quoted spans from $line before testing for the opener (stripped=$(printf '%s' "$line" | sed "s/'[^']*'//g")), then check [[ "$stripped" == *"@'" ]]. A @' that disappears after stripping was inside a string literal, not a real opener.
local stripped
stripped=$(printf '%s' "$line" | sed "s/'[^']*'//g")
if [[ "$stripped" == *"@'" || "$stripped" == *'@"' ]]; thenIMPORTANT — @"..."@ body blanked before subexpression detection (UNPATCHED)
Confidence: CONFIRMED
The body loop issues an unconditional continue for every in_hs line regardless of hs_quote. Single-quoted @'...'@ bodies are literal (safe to drop). Double-quoted @"..."@ bodies execute $(...) subexpressions in PowerShell. Dropping the body removes the ( characters that ps::has_special_constructs would otherwise catch to route the command into the fail-closed sink.
Confirmed bypass — valid PowerShell, all git guards and write gate pass at 058e09d:
$x = @"
$(git commit --no-verify)
"@Blanker trace: $x = @" ends with @" → real opener, hs_quote='"'. Body line $(git commit --no-verify) → in_hs=1, dropped (unconditional continue). "@ closes. PS_BLANKED="$x = __GUARDRAILS_PS_HERESTRING__". No ( → has_special_constructs returns false → all guards allow. PowerShell executed git commit --no-verify via the subexpression.
The same construction hides git reset --hard, Set-Content, or any guarded form inside @"..."@.
Mitigation (unchanged from prior rounds): Blank only @'...'@ body lines. For hs_quote == '"', keep body lines in out so $(...) remains visible to has_special_constructs, which routes to the fail-closed sink. The canonical commit form uses @'...'@ (literal), so the pass-through is unaffected — only @"..."@ changes behavior.
# In the body-drop path (line ~113, before `continue`):
if [[ "$hs_quote" == '"' ]]; then
out+="${line}"$'\n' # double-quoted body executes $(...) — keep it visible
continue
fi
# single-quoted body is literal — safe to dropNEW P2 — Separator-adjacent call operator bypasses write gate (introduced by boundary inconsistency)
Confidence: CONFIRMED
ps::has_launcher (line 239) correctly uses the wide boundary (^|[[:space:]\;\|\&\(]) for the preceding character before the launcher name. However, the call-operator (&) / dot-source (.) checks in three other functions use the narrower boundary (^|[[:space:]]), missing ;/|/&/( as valid preceding characters:
| Function | Line | Boundary used |
|---|---|---|
ps::might_invoke_git — call/dot-source check |
193 | `(^ |
ps::has_dynamic_invocation — call/dot-source check |
219 | `(^ |
ps::write_bypass — quoted writer call check |
335 | `(^ |
ps::write_bypass — computed call-target check |
342 | `(^ |
Confirmed write-gate bypass — valid PowerShell, block-hook-bypass exits 0:
Write-Host ok;& 'Set-Content' -Path f.txt -Value xTrace for ps::write_bypass:
PS_BLANKED = "Write-Host ok;& 'Set-Content' -Path f.txt -Value x".lcq(lowercase, backticks removed):write-host ok;& 'set-content' -path f.txt -value x.- Quoted-writer regex
(^|[[:space:]])[.\&]…set-content…:&preceded by;, not[[:space:]]→ no match. - Computed-target regex
(^|[[:space:]])[.\&][[:space:]]*[\(\$]: same → no match. scan = ps::blank_quoted_spans(PS_BLANKED)→write-host ok;& -path f.txt -value x('Set-Content'stripped).- Cmdlet check:
set-contentno longer inlcs→ no match. - No
>redirect → redirect gate not triggered. Returns 1 (allowed). PowerShell wrotef.txt.
The same gap applies to ps::has_dynamic_invocation for git guards: git log;& 'iex' 'git commit --no-verify' — & after ; is not caught by the call/dot-source check, and iex is inside a quoted string erased by blank_quoted_spans, so the guard allows.
Fix: Widen the boundary in all four locations from (^|[[:space:]]) to (^|[[:space:]\;\|\&\(]), consistent with ps::has_launcher and all the cmdlet-detection regexes in ps::write_bypass (which already use this wider boundary).
Minor note (unchanged from prior rounds)
block-noncanonical-commit.sh:368: the comment # non-commit PowerShell with an A2b-deferred construct is inaccurate — return code 1 from ps::classify_git_command means "provably git-free," not specifically "non-commit" (it could be git log | Where-Object {…}). Should read # provably git-free PowerShell with an A2b-deferred construct. Logic is correct; the comment is misleading.
Summary for operator
058e09d is a targeted correctness commit that closes all three Codex P2 findings from the last round (expression-valued launchers, expression-literal producers, computed call-target writers), each with a correct fix and green regression tests.
Three findings remain open. The CRITICAL (false here-string opener) and IMPORTANT (@"..."@ subexpression bypass) have been reported on five consecutive HEADs without being patched — both are targeted fixes to ps::blank_herestrings totaling ~10 lines. The NEW P2 (separator-adjacent call operator boundary inconsistency) is a one-line fix at four sites, consistent with the boundary already used by ps::has_launcher and the cmdlet-detection regexes. None of the three affects the canonical commit form pass-through.
|
Claude finished @kyle-sexton's task in 4m 48s —— View job Security Review — PR #960 (HEAD
|
| Finding | Fix |
|---|---|
Computed launcher target — Start-Process ('g'+'it') -ArgumentList 'reset --hard' reached ps::might_invoke_git with no contiguous git → allowed |
New launcher+computed-target guard at ps-command.sh:198: launcher followed by a ( or $ target is treated as possibly-git → fail-closed ✅ |
Expression-literal redirect producers — 36 > out.txt, [char]65 > f bypassed write gate |
Numeric-head check at ps-command.sh:433 and '['* case at ps-command.sh:427 ✅ |
Computed call-operator writer — & ('Set-'+'Content') -Path f.txt -Value x evaded the quoted-literal pre-check |
Computed-call fail-closed at ps-command.sh:342–344 ✅ |
The stale unparseable → unparsable function-name callers from the round-5 SUGGESTION are correct at block-no-verify.sh:234, block-noncanonical-commit.sh:364, and block-dangerous-git.sh:758. ✅
No new security surface was introduced in 058e09d.
Two prior security findings remain unpatched
ps::blank_herestrings is identical to every prior round. Both CRITICAL and IMPORTANT findings are confirmed against the current HEAD.
CRITICAL — False here-string opener swallows hidden git commands and write cmdlets (UNPATCHED)
Severity: CRITICAL · Confidence: CONFIRMED
if [[ "$line" == *"@'" || "$line" == *'@"' ]]; thenThe opener test fires on any raw line whose last two characters are @' — including a valid single-quoted PowerShell string like $x = 'foo @' (string value foo @; the trailing @' is inside the string, not a here-string opener). The blanker sets in_hs=1 and drops every subsequent line as "body" until the '@ of a real, later here-string fires the false close — at which point that real here-string's suffix is grafted onto the pending line.
Confirmed bypass — valid PowerShell, bypasses all five widened guards at 058e09d:
$x = 'foo @'
git commit --no-verify
$y = @'
commit message
'@ | git commit -F -Blanker trace at current HEAD:
$x = 'foo @'— last two chars@'→ false opener.pending="$x = 'foo __GUARDRAILS_PS_HERESTRING__",in_hs=1.git commit --no-verify—in_hs=1→ dropped.$y = @'— body → dropped.commit message— body → dropped.'@ | git commit -F -—first2="'@"fires false close.out="$x = 'foo __GUARDRAILS_PS_HERESTRING__ | git commit -F -".PS_HERESTRING_UNBALANCED=0.
PS_BLANKED="$x = 'foo __GUARDRAILS_PS_HERESTRING__ | git commit -F -". The unmatched ' before foo has no closing partner so blank_quoted_spans leaves the suffix intact. has_special_constructs → false. has_dynamic_invocation → false. has_launcher → false. classify_git_command returns 0. Bash parser sees git commit -F - (canonical stdin form) as the second pipeline segment → all guards allow. PowerShell ran git commit --no-verify on line 2.
The same construction hides git reset --hard, git push -f, or a write cmdlet (Set-Content, Out-File, etc.) from all five widened guards and the write gate — ps::write_bypass calls the same ps::blank_herestrings at ps-command.sh:325 and returns 1 (no bypass) when the write cmdlet is swallowed as a body line.
Root cause: The opener test operates on the raw unquoted line. Per PowerShell's about_Quoting_Rules, a here-string opener @'/@" must be the last syntactic token on its line — @' that is already inside a string literal is NOT an opener, but the blanker cannot distinguish this without parsing the quoting.
Mitigation (unchanged): Strip single-quoted spans from $line before testing for the opener. A @' that disappears after sed "s/'[^']*'//g" was inside a string literal.
# Before the opener test at line 116:
local stripped
stripped=$(printf '%s' "$line" | sed "s/'[^']*'//g")
if [[ "$stripped" == *"@'" || "$stripped" == *'@"' ]]; thenIMPORTANT — @"..."@ double-quoted here-string body blanked; $(cmd) subexpressions execute and evade all guards (UNPATCHED)
Severity: IMPORTANT · Confidence: CONFIRMED
if ((in_hs)); then
...
# A body line (no column-zero closer) is dropped.
continue
fiThe continue at line 113 is unconditional — body lines are dropped regardless of hs_quote. Single-quoted @'...'@ bodies are literal (safe to blank). Double-quoted @"..."@ bodies execute $(...) subexpressions inline in PowerShell. Blanking the body removes the ( characters that ps::has_special_constructs would catch to route to the fail-closed sink, preventing detection that would otherwise fire.
Confirmed bypass — valid PowerShell, bypasses all guards at 058e09d:
$x = @"
$(git commit --no-verify)
"@Blanker trace: $x = @" (ends @") → real opener, hs_quote='"'. Body line $(git commit --no-verify) → in_hs=1 → dropped (unconditional continue). "@ closes. PS_BLANKED="$x = __GUARDRAILS_PS_HERESTRING__". No ( visible → has_special_constructs returns false → all guards allow. PowerShell executed git commit --no-verify via the subexpression.
The same construction hides git reset --hard, Set-Content -Path creds.txt -Value secret, or any other guarded form inside @"..."@. For the write gate: ps::write_bypass calls ps::blank_herestrings at line 325; the blanked result contains only __GUARDRAILS_PS_HERESTRING__ with no write cmdlet visible, so it returns 1 (not a bypass) and the write proceeds undetected.
Root cause: @'...'@ is literal (safe to blank); @"..."@ evaluates $(...) inline (NOT safe to blank — blanking removes execution evidence). The blanker conflates both types.
Mitigation (unchanged): Do not blank @"..."@ body lines. When hs_quote == '"', keep body lines in out so $(...) remains visible to has_special_constructs. The canonical commit form uses single-quoted @'...'@, so the intended pass-through is unaffected — only @"..."@ changes, becoming fail-closed whenever the body contains a guarded form.
# In the body-drop path, before the final `continue` at line 113:
if [[ "$hs_quote" == '"' ]]; then
out+="${line}"$'\n'
fiNo new findings
The 058e09d additions are all fail-closed: the computed-launcher extension to ps::might_invoke_git (line 198), the computed-call fail-closed in ps::write_bypass (lines 342–344), the numeric-expression and cast-expression redirect producer cases (lines 427, 433). None introduce new bypass vectors. The rest of the surface — has_special_constructs, has_dynamic_invocation, has_launcher, the module-qualified boundary class, the paren-unwrap producer logic, and all hook injection surfaces — carries no new issues.
Summary for operator
058e09d correctly closes the three Codex P2 gaps (computed launcher, expression-literal redirect producers, computed call-operator writer). The CRITICAL and IMPORTANT findings from the prior security review remain the only items blocking merge. Both are in ps::blank_herestrings (~5–10 lines each); the @"..."@ fix simplifies the body loop; neither affects the canonical @'...'@ commit form pass-through.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 058e09dd7d
ℹ️ 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".
Bot round 6, all verified live: (1) a quoted string ending in the
characters @' read as a here-string opener, swallowing following code
lines into a phantom body (git/write lines unseen — P1); the opener test
now strips paired quote spans first. (2) Backslash path-qualified git
(C:\Git\cmd\git.exe reset --hard) tokenized through Bash escapes to a
non-git word (P1); the reduced command now normalizes \ to / so basename
matching works, keeping safe path-qualified calls allowed. (3)+(5) The
call/dot-source probes (git side and both write-gate checks) only fired
after whitespace, missing separator-adjacent forms (;& ('g'+'it') …).
(4) Non-success stream producers (Write-Error 2>, Write-Warning 3>,
verbose/debug/information) were not counted as redirect content writes.
All red-first; suites 251/157 green.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 5m 45s —— View job Claude finished @kyle-sexton's task in 5m 26s —— PR Review — HEAD
|
| Finding | Fix |
|---|---|
CRITICAL — false here-string opener ('foo @' → false in_hs=1) |
opener_scan now strips paired quote spans before testing: sed "s/'[^']*'//g" + `sed -E 's/"([^"\\] |
P1 — backslash path-qualified git (C:\Git\cmd\git.exe reset --hard → Bash tokenizer eats backslashes, C:Gitcmdgit.exe not recognized) |
PS_SAFE_COMMAND="${PS_BLANKED//\\//}" normalizes \ to / so hook::git_is_bin sees the correct basename. — ps-command.sh:290 ✅ |
P2 — separator-adjacent call operators (Write-Host ok;& 'Set-Content' — & after ; not caught) |
All four call/dot-source boundary patterns widened from `(^ |
P2 — non-success stream producers (Write-Error 2> creds.txt, Write-Warning 3> f) bypass write gate |
write-error, write-warning, write-verbose, write-debug, write-information added to the redirect producer case arm. — ps-command.sh:445 ✅ |
Regression tests for all four added to block-dangerous-git.test.sh and block-hook-bypass.test.sh. The SUGGESTION finding (stale unparseable function names) was already fixed in 82f0e43. All prior P2 findings from rounds 5–8 are confirmed fixed.
One prior security finding remains unpatched
IMPORTANT — @"..."@ body blanked before subexpression detection (UNPATCHED)
Confidence: CONFIRMED
if ((in_hs)); then
...
if [[ "$first2" == "$closer" ]]; then
...
in_hs=0
fi
# A body line (no column-zero closer) is dropped.
continue # ← unconditional regardless of hs_quote
fiThe continue at line 113 is still unconditional — body lines are dropped for both @'...'@ (single-quoted, literal, safe to blank) and @"..."@ (double-quoted, expandable, executes $(...) subexpressions in PowerShell). Blanking the double-quoted body removes the ( characters that ps::has_special_constructs would otherwise catch to route the command into the fail-closed sink.
Confirmed bypass — valid PowerShell, all five git guards and write gate pass at 7d82a00:
$x = @"
$(git commit --no-verify)
"@Blanker trace at current HEAD:
$x = @"—opener_scanruns sed on$x = @". No paired double-quoted spans to strip (the@"has no closing"), soopener_scan = "$x = @". Ends with@"→ real opener fires.hs_quote='"',pending="$x = __GUARDRAILS_PS_HERESTRING__",in_hs=1.$(git commit --no-verify)—in_hs=1,first2="$(",closer='"@'→ no match. Dropped (unconditionalcontinue)."@—in_hs=1,first2='"@',closer='"@'→ match.rest="".out="$x = __GUARDRAILS_PS_HERESTRING__".in_hs=0.
PS_BLANKED="$x = __GUARDRAILS_PS_HERESTRING__". scan=ps::blank_quoted_spans(PS_BLANKED) → no change. has_special_constructs(scan) → false (no (/) visible — they were in the dropped body). has_dynamic_invocation(PS_BLANKED) → false. has_launcher(PS_BLANKED) → false. classify_git_command returns 0. Bash tokenizer sees $x = __GUARDRAILS_PS_HERESTRING__, no git → all guards allow. PowerShell executed git commit --no-verify via the subexpression.
The same construction hides git reset --hard, git push -f, or Set-Content -Path creds.txt -Value secret inside @"..."@ — the write gate calls ps::blank_herestrings at the same code path and is equally bypassed.
Root cause: @'...'@ is literal (safe to blank — no evaluation). @"..."@ executes $(...) subexpressions inline. Blanking its body removes the structural ( evidence. Single-quoted here-strings are the canonical commit form and the only here-string form the PR aims to pass through — double-quoted ones should route to the fail-closed branch.
Fix: Blank body lines only for single-quoted here-strings. For hs_quote == '"', keep body lines in out so $(...) remains visible to has_special_constructs, which routes to the fail-closed sink. The canonical commit form (@'...'@) is unaffected — only @"..."@ changes, becoming fail-closed whenever its body contains a (, which is the right outcome.
# In the body-drop path — change the unconditional continue at line 113:
if [[ "$hs_quote" == "'" ]]; then
continue # single-quoted body is literal — safe to drop
fi
# double-quoted body executes $(...) — keep it visible to has_special_constructs
out+="${line}"$'\n'Minor note (unchanged from prior rounds)
block-noncanonical-commit.sh:368: the comment # non-commit PowerShell with an A2b-deferred construct is inaccurate — return code 1 from ps::classify_git_command means "provably git-free" (it could be git log | Where-Object { … } — git, not commit). Should read # provably git-free PowerShell with an A2b-deferred construct. Logic is correct; the comment misleads.
Summary for operator
7d82a00 closes all four round-8 findings correctly, with red-first regression tests confirming each. The CRITICAL false-opener finding (reported over five prior HEADs) is now fixed. The IMPORTANT @"..."@ body subexpression bypass is the only remaining item blocking merge — it requires a ~5-line targeted change to ps::blank_herestrings, does not affect the canonical @'...'@ commit form, and actually simplifies the body loop (the single-quoted body is explicitly continued; the double-quoted body falls through to out +=).
|
Claude finished @kyle-sexton's task in 6m 7s —— View job Security Review — PR #960 (HEAD
|
| Finding | Fix |
|---|---|
CRITICAL: False here-string opener ('foo @' triggering in_hs=1) |
opener_scan variable strips paired quote spans before the opener test — ps-command.sh:121 |
P1: Path-qualified git executable (C:\Git\cmd\git.exe reset --hard passed to Bash tokenizer with backslashes intact) |
PS_SAFE_COMMAND="${PS_BLANKED//\\//}" normalizes to forward slashes — ps-command.sh:290 |
P2: Separator-adjacent call operators (Write-Host ok;& 'Set-Content' … bypassed write gate) |
Boundary widened from `(^ |
P2: Non-success stream producers (Write-Error 2>, Write-Warning 3> not blocked) |
Full Write-{Error,Warning,Verbose,Debug,Information} producer list added — ps-command.sh:445 |
CRITICAL finding resolved ✅
False here-string opener (reported rounds 4–8, confirmed against every prior HEAD) — FIXED.
The fix at ps-command.sh:121:
opener_scan=$(printf '%s' "$line" | sed "s/'[^']*'//g" | sed -E 's/"([^"\\]|\\.)*"//g')
if [[ "$opener_scan" == *"@'" || "$opener_scan" == *'@"' ]]; thenStripping '[^']*' (single-quoted spans) before the test means $x = 'foo @' loses the paired span and becomes $x = , which no longer ends with @'. A real here-string opener like $y = @' has an unpaired ' — there is no matching ' before @' on the same line — so @' survives the strip. The double-quote strip ("([^"\\]|\\.)*") applies the same logic for @".
Tracing the original bypass:
$x = 'foo @'→opener_scan = "$x = "→ no false opener. Line passes through toout.git commit --no-verify→ passes through toout.$y = @'→opener_scan = "$y = @'"→ real opener. Here-string correctly blanked.commit message→ body, dropped.'@ | git commit -F -→ closer.outends withgit commit --no-verify\n$y = __HS__ | git commit -F -.
PS_BLANKED now contains git commit --no-verify — the guard sees it and blocks. ✅
The fix is also injection-safe: printf '%s' "$line" prevents format-string injection, and the sed scripts are hardcoded.
IMPORTANT finding — still unpatched
@"..."@ body blanked before subexpression detection (UNPATCHED)
Severity: IMPORTANT · Confidence: CONFIRMED
if ((in_hs)); then
first2="${line:0:2}"
closer="${hs_quote}@"
if [[ "$first2" == "$closer" ]]; then
rest="${line:2}"
out+="${pending}${rest}"$'\n'
pending=""
in_hs=0
hs_quote=""
fi
# A body line (no column-zero closer) is dropped.
continue
fiThe continue is unconditional — body lines are dropped for both @'...'@ (single-quoted, literal, safe to blank) and @"..."@ (double-quoted, executes $(...) subexpressions inline). Blanking the body of @"..."@ removes the ( characters that ps::has_special_constructs would catch to route to the fail-closed sink.
Confirmed bypass — valid PowerShell, all five widened guards and the write gate pass at 7d82a00:
$x = @"
$(git commit --no-verify)
"@Blanker trace:
$x = @"—opener_scan: after stripping,"at end has no closing"pair →opener_scan = "$x = @"→ ends with@"→ real opener.hs_quote='"',pending="$x = __GUARDRAILS_PS_HERESTRING__",in_hs=1.$(git commit --no-verify)—in_hs=1,first2="$("≠"@"→ dropped (unconditionalcontinue)."@—first2='"@'matchescloser='"@'→out="$x = __GUARDRAILS_PS_HERESTRING__".in_hs=0.
PS_BLANKED = "$x = __GUARDRAILS_PS_HERESTRING__". No ( visible → has_special_constructs returns false. No launcher, no dynamic invocation → classify_git_command returns 0. All guards allow. PowerShell executed git commit --no-verify via the subexpression.
The same construction hides git reset --hard, Set-Content, or any guarded form inside @"..."@. The @"..."@ form is valid PowerShell and the bypass requires no malformed syntax.
Mitigation (unchanged from prior rounds): Do not blank @"..."@ body lines. When hs_quote == '"', add body lines to out unmodified — has_special_constructs then sees the $(...) and routes to the fail-closed sink. The canonical commit form uses single-quoted @'...'@ (literal, no execution), so the canonical pass-through is unaffected.
# In the body-drop path — change the unconditional continue:
if [[ "$hs_quote" == "'" ]]; then
continue # single-quoted body is literal — safe to drop
fi
out+="${line}"$'\n' # double-quoted body executes $(...) — keep visibleNo new findings
All other changes in 7d82a00 are evaluated below and carry no new security issues.
opener_scan fix correctness: The sed pipeline sed "s/'[^']*'//g" | sed -E 's/"([^"\\]|\\.)*"//g' correctly disambiguates false openers. Since PowerShell uses backtick (not backslash) as its escape character, and backtick anywhere routes to the fail-closed sink via has_special_constructs, the double-quote sed pattern's backslash-escape convention only diverges from PowerShell for strings containing backtick — which never reach this path. No bypass surface.
PS_SAFE_COMMAND="${PS_BLANKED//\\//}": Backslash is PowerShell's path separator (not its escape character — that is backtick), so normalizing \ → / lets hook::git_is_bin extract the real basename from a path-qualified command. The Bash tokenizer sees C:/Git/cmd/git.exe reset --hard as expected. No bypass: a backslash embedded in a flag like --ha\rd normalizes to --ha/rd, which Git does not interpret as --hard, so this cannot be used to smuggle a flag past the guard.
Separator boundary widening ((^|[[:space:]\;\{\}\(\|\&])): Consistent application across ps::might_invoke_git, ps::has_dynamic_invocation, and both call-operator checks in ps::write_bypass. No new surface.
Write-{Error,Warning,Verbose,Debug,Information} producer additions: Additive, fail-closed direction. No bypass surface.
Summary for operator
7d82a00 fixes the CRITICAL false here-string opener finding that has been reported across five prior heads — the opener test now correctly strips paired quote spans so 'foo @' does not trigger in_hs=1. All four changes in the commit are correct and no new security surface was introduced.
The IMPORTANT finding (@"..."@ body subexpression bypass) is the sole remaining open issue. It has been confirmed against dd550e0, c49a120, dd0f4a8, 82f0e43, 058e09d, and now 7d82a00 — six consecutive HEADs — without being patched. The fix is approximately five lines in the body loop of ps::blank_herestrings, and it does not affect the canonical single-quoted @'...'@ commit form pass-through.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7d82a00995
ℹ️ 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".
…lock producers, POSIX git.exe
(1) The round-5 numeric-producer check over-blocked tool captures: the &
inside 2>&1 split a phantom "1 > file" segment; fd-dup merge redirects
now strip before segmenting. (2) Invoked script blocks (& { Write-Output
secret } > f) unwrap like parenthesized producers; grouped tool runs stay
allowed. (3) CI (Linux) exposed that git.exe basenames only matched on
the msys branch of hook::git_is_bin — the PS reduction now normalizes
.exe-suffixed git spellings so the path-qualified regressions hold on a
POSIX-run hook too. All red-first; 162/251/90 green.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 4m 31s —— View job PR Review — HEAD
|
| Finding | Fix |
|---|---|
P2 — fd-dup merge redirects (2>&1, *>&1) produced phantom 1 > file segment — the & inside 2>&1 split at the statement-separator pass, generating a 1 > file segment that the numeric-producer check wrongly flagged, so git status 2>&1 > out.txt was blocked |
ps-command.sh:433: lcs=$(printf '%s' "$lcs" | sed -E 's/[0-9*]*>&[0-9]+//g') strips fd-dup forms before the `[ |
P2 — script-block redirect producers (& { Write-Output secret } > creds.txt) bypassed write gate — {/} were not unwrapped alongside ()/) in the producer check, so head { didn't match any producer |
ps-command.sh:445–447: seg="${seg//\{/}" / seg="${seg//\}/}" added after the paren-unwrap, so & { Write-Output secret } exposes Write-Output as head. ✅ |
P1 — POSIX-host git.exe normalization — on a Linux/macOS hook host, hook::git_is_bin strips .exe only on its msys/cygwin branch, so C:/Git/cmd/git.exe reset --hard (after \→/ normalize) left a basename git.exe that matched nothing, allowing the guarded form through |
ps-command.sh:292: sed -E 's/[Gg][Ii][Tt]\.[Ee][Xx][Ee]/git/g' normalizes git.exe (any case) to git unconditionally. ✅ |
Regression tests for all three are present in block-hook-bypass.test.sh (fd-dup allowed, echo x 2>&1 > f blocked, & { Write-Output secret } > f blocked, & { git diff } > f allowed). ✅
Correctness audit of the new code
fd-dup strip ([0-9*]*>&[0-9]+): strips 2>&1, >&1, *>&1, 2>&2. Applied to lcs (the already quote-stripped, backtick-removed text) before the segment split. echo x 2>&1 > f → echo x > f → head echo → blocked correctly. git status 2>&1 > out.txt → git status > out.txt → head git → not a producer → allowed correctly. The pattern [0-9*]* means an empty prefix also matches (>&1), which is the >&1 merge-to-stdout form — equally safe to strip.
Brace unwrap: seg="${seg//\{/}" and seg="${seg//\}/}" applied after the paren-unwrap and before re-ltrim. & { Write-Output secret } > creds.txt → after splitting at & → segment { Write-Output secret } > creds.txt → after brace strip → Write-Output secret > creds.txt → head Write-Output → blocked. & { git diff } > out.txt → after split at & → { git diff } > out.txt → brace strip → git diff > out.txt → head git → not a producer → allowed. Correct in both cases.
One interaction to note: the segment split itself uses [|;&], which includes &, so & { ... } > f correctly splits & as a separator, leaving the { ... } > f part as the segment to examine. Verified correct.
git.exe normalization: runs on the forward-slash-normalized PS_BLANKED after all herestring/quote analysis. Case-insensitive match. mygit.exe → mygit (not matched as git by hook::git_is_bin). git.exe.bak → git.bak (similarly not matched). The pattern [Gg][Ii][Tt]\.[Ee][Xx][Ee] does not require a word boundary, so substring matches within a longer basename are possible (e.g. legitgit.exe → legitgit) — but none of those produce a false positive at hook::git_is_bin. A false block is impossible by the same reasoning. Clean.
One prior security finding remains unpatched
IMPORTANT — @"..."@ double-quoted here-string body blanked before subexpression detection (UNPATCHED across all 10 rounds)
Confidence: CONFIRMED
# A body line (no column-zero closer) is dropped.
continueThe continue at line 113 is unconditional — body lines are dropped for both @'...'@ (single-quoted, literal, safe to blank) and @"..."@ (double-quoted, expandable, executes $(...) subexpressions inline). Blanking the body removes the ( characters that ps::has_special_constructs would catch at line 169–172 to route the command into the fail-closed sink.
Confirmed bypass — valid PowerShell, all five widened guards and the write gate pass at 72ee510:
$x = @"
$(git commit --no-verify)
"@Blanker trace:
$x = @"—opener_scanstrips paired spans. No paired double-quoted span on this line (the"is not closed on the same line), soopener_scan = "$x = @"→ ends with@"→ real opener.hs_quote='"',pending="$x = __GUARDRAILS_PS_HERESTRING__",in_hs=1.$(git commit --no-verify)—in_hs=1,first2="$("≠"@"→ dropped (unconditionalcontinue)."@—first2='"@'matchescloser='"@'→out="$x = __GUARDRAILS_PS_HERESTRING__".in_hs=0.
PS_BLANKED = "$x = __GUARDRAILS_PS_HERESTRING__". scan = ps::blank_quoted_spans(PS_BLANKED) → no change. has_special_constructs(scan) → false (no (/) visible — dropped in the body). has_dynamic_invocation → false. has_launcher → false. classify_git_command returns 0. Bash tokenizer sees $x = __GUARDRAILS_PS_HERESTRING__, no git → all guards allow. PowerShell executed git commit --no-verify via the subexpression.
The same construction hides git reset --hard, git push -f, or $(Set-Content creds.txt secret) from every guard widened by this PR. The write gate calls ps::write_bypass, which also calls ps::blank_herestrings — the write detection sees only __GUARDRAILS_PS_HERESTRING__, no write cmdlet visible.
Root cause: @'...'@ is literal (safe to blank — no evaluation). @"..."@ executes $(...) subexpressions inline (not safe to blank — blanking removes execution evidence). The blanker conflates both types at line 113.
Fix (~5 lines): Do not blank @"..."@ body lines. Replace the unconditional continue with a branch on hs_quote:
# Single-quoted body is literal — safe to drop.
# Double-quoted body executes $(...) — keep it visible to has_special_constructs.
if [[ "$hs_quote" == "'" ]]; then
continue
fi
out+="${line}"$'\n'This leaves $(git commit --no-verify) in PS_BLANKED, so has_special_constructs sees the (, routes to the fail-closed sink, might_invoke_git finds git, and the command is blocked. The canonical commit form uses single-quoted @'...'@ — it is unaffected. Only @"..."@ changes, becoming fail-closed whenever the body contains (. No change to the canonical pass-through.
Minor note (unchanged from prior rounds)
block-noncanonical-commit.sh:368: the comment still reads # non-commit PowerShell with an A2b-deferred construct. Return code 1 from ps::classify_git_command means "provably git-free" — it could be git log | Where-Object { … } (a git command, but not a commit). The correct wording is # provably git-free PowerShell with an A2b-deferred construct. Logic is correct; the comment misleads.
Summary for operator
72ee510 correctly closes all three round-8 open findings:
- fd-dup false-block (
git status 2>&1 > out.txtwas wrongly blocked) — fixed with a pre-splitsedstrip of[0-9*]*>&[0-9]+forms - Script-block redirect producers (
& { Write-Output secret } > fbypassed write gate) — fixed by extending the existing paren-unwrap to also strip{/} - POSIX git.exe normalization (
git.exeon a non-msys hook host not recognized byhook::git_is_bin) — fixed with a case-insensitivesedsubstitution
All three fixes are correct with appropriate regression tests.
The IMPORTANT @"..."@ body subexpression bypass is the sole remaining item blocking merge. It has now been confirmed unpatched across ten consecutive HEADs (dd550e0, c49a120, dd0f4a8, 82f0e43, 058e09d, 7d82a00, 72ee510 — plus intermediate comment/typo-fix commits). The fix is approximately five lines in the body loop of ps::blank_herestrings, does not affect the canonical @'...'@ commit-form pass-through, and simplifies the logic (the single-quoted body is explicitly continued; the double-quoted body falls through to out +=).
|
Claude finished @kyle-sexton's task in 5m 23s —— View job Security Review — PR #960 (HEAD
|
| Finding | Fix |
|---|---|
fd-dup merge redirects (2>&1, *>&1) were split on &, creating a phantom 1 > file segment that wrongly blocked benign tool captures (git status 2>&1 > out.txt) |
sed -E 's/[0-9*]*>&[0-9]+//g' strips fd-dup redirects from lcs before the segment split — ps-command.sh:433 ✅ |
Invoked script blocks (& { Write-Output secret } > creds.txt) reached the redirect loop with a { head and were allowed |
Script-block braces {/} are now stripped alongside ()/) in the per-segment unwrap — ps-command.sh:446–447 ✅ |
Path-qualified git.exe (C:\Git\cmd\git.exe reset --hard) after \→/ normalization still carried .exe, which hook::git_is_bin strips only on its msys/cygwin branch |
sed -E 's/[Gg][Ii][Tt]\.[Ee][Xx][Ee]/git/g' collapses git.exe (any case) to git before handing PS_SAFE_COMMAND to the Bash guards — ps-command.sh:292 ✅ |
Regression tests for all three added in block-hook-bypass.test.sh (fd-dup tool-capture allowed, script-block producer blocked, & { git diff } > out tool-producer allowed). ✅
The CRITICAL false-opener finding (fixed in 7d82a00) and SUGGESTION stale function names (fixed in 82f0e43) both remain correct in the current HEAD. ✅
No new security issues in 72ee510
The three changes are audited below and introduce no new bypass surface:
fd-dup stripping ([0-9*]*>&[0-9]+): The pattern strips 2>&1, *>&1, >&1 (empty prefix allowed by [0-9*]*) but requires a digit after &, so >&somefile is not stripped and stays visible to the redirect check. Producer forms that happen to precede a fd-dup (write-output x >&2 > f) correctly lose the >&2 and retain the file redirect. No bypass.
Script-block brace unwrapping: Braces are stripped per-segment after the [|;&] split. & { write-output x } > f splits on the leading &, then the { write-output x } > f segment has braces removed → write-output x > f → head write-output → blocked. & { git diff } > out → brace-stripped → git diff > out → head git (not a content producer) → allowed. No bypass.
git.exe normalization: The sed replacement git.exe → git runs without a word boundary (xgit.exe → xgit — a different non-git tool, not detected as git by hook::git_is_bin). No bypass or false-positive path that enables evasion.
IMPORTANT — @"..."@ body blanked before subexpression detection (UNPATCHED)
Severity: IMPORTANT · Confidence: CONFIRMED
if ((in_hs)); then
first2="${line:0:2}"
closer="${hs_quote}@"
if [[ "$first2" == "$closer" ]]; then
rest="${line:2}"
out+="${pending}${rest}"$'\n'
...
fi
# A body line (no column-zero closer) is dropped.
continue # ← line 113 — unconditional regardless of hs_quote
fiThe continue is unconditional — body lines are dropped for both @'...'@ (single-quoted, literal, safe) and @"..."@ (double-quoted, executes $(...) subexpressions in PowerShell). Blanking the double-quoted body removes the ( characters that ps::has_special_constructs would catch to route to the fail-closed sink.
Confirmed bypass — valid PowerShell, all five widened guards and the write gate pass at 72ee510:
$x = @"
$(git commit --no-verify)
"@Blanker trace at current HEAD:
$x = @"—opener_scanstrips paired quote spans: no complete"..."pairs on this line, soopener_scan = "$x = @". Ends with@"→ real opener.hs_quote='"',pending="$x = __GUARDRAILS_PS_HERESTRING__",in_hs=1.$(git commit --no-verify)—in_hs=1,first2="$("≠"@"closer → dropped (unconditionalcontinue, line 113)."@—first2='"@'matchescloser='"@'→out="$x = __GUARDRAILS_PS_HERESTRING__".in_hs=0.
PS_BLANKED = "$x = __GUARDRAILS_PS_HERESTRING__". blank_quoted_spans → no change. has_special_constructs → false (no ( visible — dropped in step 2). has_dynamic_invocation → false. has_launcher → false. classify_git_command returns 0. Bash tokenizer sees $x = __GUARDRAILS_PS_HERESTRING__, no git → all guards allow. PowerShell executed git commit --no-verify via the subexpression.
The same construction hides git reset --hard, git push -f, or Set-Content -Path creds.txt -Value secret from every guard. For the write gate: ps::write_bypass calls ps::blank_herestrings at line 345; the blanked result contains only __GUARDRAILS_PS_HERESTRING__ with no write cmdlet or $(...) visible, so it returns 1 (allowed) — block-hook-bypass exits 0.
Root cause: @'...'@ is literal (no evaluation — safe to blank). @"..."@ evaluates $(...) subexpressions inline in PowerShell. The blanker conflates both; blanking @"..."@ body lines removes the structural ( evidence.
Mitigation: Do not blank @"..."@ body lines. In the if ((in_hs)) block, check hs_quote before the unconditional continue:
# Replace the unconditional `continue` at line 113 with:
if [[ "$hs_quote" == "'" ]]; then
continue # single-quoted body is literal — safe to drop
fi
# double-quoted body executes $(...) — keep it visible to has_special_constructs
out+="${line}"$'\n'The canonical commit form uses @'...'@ (literal), so its pass-through is unaffected. Only @"..."@ changes behavior — it becomes fail-closed whenever the body contains a guarded form. This is the right outcome, and it actually simplifies the body loop.
Summary for operator
72ee510 correctly closes the three round-8/9 Codex findings (fd-dup redirect splitting, script-block redirect producers, POSIX git.exe normalization), each with regression tests. No new security surface is introduced.
The IMPORTANT finding (@"..."@ body subexpression bypass) is the sole remaining open issue. It has been confirmed against seven consecutive HEADs (dd550e0 → c49a120 → dd0f4a8 → 82f0e43 → 058e09d → 7d82a00 → 72ee510) without being patched. The fix is approximately five lines in the body loop of ps::blank_herestrings, does not affect the canonical single-quoted @'...'@ commit form pass-through, and simplifies the body loop.
|
CONVERGENCE GATE (control tower, per operator-ratified Bash-parity bar — #915 comment 5046269643, #607 decision batch): Round 7 is in. The ratified bar for this PR is Bash-parity, not airtight: the Bash guard never evaluates Gate, effective now:
Driving lane: also note your prompt file is stale (round-14; current is round-16 — see #502). Do not restack or merge this PR; tower claim (comment 5049967417) stands through merge. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 72ee510f52
ℹ️ 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".
Round-8 bot finding: Microsoft.PowerShell.Utility\Write-Output secret > f.txt fell through the producer head check because the module prefix defeated the exact-name case match. The head now compares its cmdlet basename. Red-first regressions for the success- and error-stream forms. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 4m 19s —— View job PR Review — HEAD
|
| Finding | Fix |
|---|---|
Module-qualified redirect producers (Microsoft.PowerShell.Utility\Write-Output secret > f.txt) bypassed the head-only check because \ was not an allowed boundary |
ps-command.sh:452: head="${head##*\\}" strips the module prefix from the head before the case match, so microsoft.powershell.utility\write-output → write-output and microsoft.powershell.utility\write-error → write-error both match their case arms. Two regression tests added. ✅ |
Correctness audit of the new code
Trace — Microsoft.PowerShell.Utility\Write-Output secret > f.txt:
lcs = "microsoft.powershell.utility\write-output secret > f.txt"(quote-stripped, lowercased).- fd-dup strip:
2>&1patterns; none here. lcs unchanged. - Segment split on
[|;&]: one segment. [[ "$seg" == *'>'* ]]→ true.- Paren/brace unwrap: no change.
head = "microsoft.powershell.utility\write-output".head="${head##*\\}"→head = "write-output". ✅case "$seg": does not start with>→ falls through.case "$head":write-outputmatches →return 0(blocked). ✅
Trace — Microsoft.PowerShell.Utility\Write-Error secret 2> f.txt:
Same path; 2> is NOT a fd-dup ([0-9*]*>&[0-9]+ requires >&n), so it stays in the segment. After basename strip, head = "write-error" → blocked. ✅
No unintended side-effects: For a tool-capture segment like git diff > out.txt, head = "git" after the ##*\\ strip (no backslash, unchanged) — git is not in the producer case arms → falls through → not flagged. ✅ The ##*\\ strip is a no-op when there is no backslash, so no regression on existing forms.
IMPORTANT finding — @"..."@ body blanked before subexpression detection (UNPATCHED)
Severity: IMPORTANT · Confidence: CONFIRMED · Convergence gate: within-parity fail-open — fix required
# A body line (no column-zero closer) is dropped.
continue # ← line 113 — unconditional regardless of hs_quoteThe continue is unconditional for both @'...'@ (single-quoted, literal, safe) and @"..."@ (double-quoted, executes $(...) subexpressions inline in PowerShell). Blanking the double-quoted body removes the ( characters that ps::has_special_constructs would otherwise catch to route the command into the fail-closed sink.
Confirmed bypass — valid PowerShell, all five widened guards and the write gate pass at 8674de0:
$x = @"
$(git commit --no-verify)
"@Blanker trace:
$x = @"—opener_scanstrips paired quote spans: no complete pair on this line →opener_scan = "$x = @"→ ends with@"→ real opener.hs_quote='"',pending="$x = __GUARDRAILS_PS_HERESTRING__",in_hs=1.$(git commit --no-verify)—in_hs=1,first2="$("≠"@"→ dropped (unconditionalcontinue, line 113)."@—first2='"@'matchescloser='"@'→out="$x = __GUARDRAILS_PS_HERESTRING__".in_hs=0.
PS_BLANKED = "$x = __GUARDRAILS_PS_HERESTRING__". No ( visible → has_special_constructs returns false. No launcher, no dynamic invocation → classify_git_command returns 0. All guards allow. PowerShell executed git commit --no-verify via the subexpression.
The same construction hides git reset --hard, git push -f, or Set-Content creds.txt secret inside @"..."@. For the write gate: ps::write_bypass calls ps::blank_herestrings; the blanked result exposes no write cmdlet or $(...) signature, so the write gate exits 0.
Convergence gate classification — within-parity fail-open: The Bash guard catches $(git commit --no-verify) appearing anywhere in the command text via has_special_constructs (which fires on any $( character). The PowerShell guard's blanker REMOVES the $( from PS_BLANKED before has_special_constructs runs — the blanker is actively defeating detection that would otherwise fire. This is not a documented "expression-evaluation / computed-form" residual: the existing fail-closed infrastructure WOULD catch it, but the blanker prevents it from doing so. The fix restores parity without adding any new detection logic.
Fix (~5 lines): Replace the unconditional continue at line 113 with a branch on hs_quote. Single-quoted body lines are still dropped (literal — no execution); double-quoted body lines are kept visible so $(...) reaches has_special_constructs:
# Single-quoted body is literal — safe to drop.
# Double-quoted body executes $(...) — keep visible for has_special_constructs.
if [[ "$hs_quote" == "'" ]]; then
continue
fi
out+="${line}"$'\n'The canonical commit form uses single-quoted @'...'@ — the pass-through is unaffected. Only @"..."@ changes behavior, becoming fail-closed whenever the body contains (, which is the right outcome and actually simplifies the body loop.
Minor note (unchanged from prior rounds)
block-noncanonical-commit.sh:368: comment reads # non-commit PowerShell with an A2b-deferred construct. Return code 1 from ps::classify_git_command means "provably git-free" — it could be git log | Where-Object { … } (a git command, but not a commit). Should read # provably git-free PowerShell with an A2b-deferred construct. Logic is correct; comment misleads.
Summary for operator
8674de0 correctly closes the Codex P2 module-qualified redirect producer gap from round 10 — baseline stripping via head="${head##*\\}" is injection-safe (a Bash parameter expansion with no external input, not a subprocess call) and has no side-effects on non-qualified forms. Both regression tests confirm the block.
The IMPORTANT @"..."@ body subexpression bypass is the sole remaining item blocking merge. Per the convergence gate criteria it is a within-parity fail-open (the Bash guard catches $( in command text via has_special_constructs; the PowerShell blanker removes that $( before the check runs, defeating existing detection). The fix is approximately five lines in the body loop of ps::blank_herestrings, simplifies the logic, and does not affect the canonical single-quoted @'...'@ commit form.
|
Claude finished @kyle-sexton's task in 4m 31s —— View job Security Review — PR #960 (HEAD
|
| Finding | Fix |
|---|---|
Microsoft.PowerShell.Utility\Write-Output secret > f.txt fell through the case "$head" in producer check because head retained the full module-qualified form, and the case arm only matched bare cmdlet names |
head="${head##*\\}" strips everything through the last \, reducing Microsoft.PowerShell.Utility\Write-Output → Write-Output before the case arm — ps-command.sh:452 ✅ |
Two regression tests added in block-hook-bypass.test.sh: success-stream (\Write-Output … > f) and error-stream (\Write-Error … 2> f) forms both confirmed to block. ✅
Fix correctness: head="${head##*\\}" is a correct greedy-strip from the left. Edge cases:
- No
\in head → no change ✓ - Trailing
\(malformed) → head becomes empty → no case match → segment skipped → not a security gap (a trailing-backslash-only form cannot execute a writer cmdlet) - Multiple
\(deep module path) → last component extracted ✓
No new security surface introduced by this commit.
IMPORTANT — @"..."@ body blanked before subexpression detection (UNPATCHED)
Severity: IMPORTANT · Confidence: CONFIRMED
# A body line (no column-zero closer) is dropped.
continue # ← unconditional regardless of hs_quoteBody lines of both @'...'@ (single-quoted, literal) and @"..."@ (double-quoted, expandable) are unconditionally dropped. $(...) subexpressions inside a @"..."@ body execute in PowerShell; blanking the body before any detection runs means ps::has_special_constructs never sees the ( that would route to the fail-closed sink.
Confirmed bypass — valid PowerShell, all five widened guards and write gate pass at 8674de0:
$x = @"
$(git commit --no-verify)
"@$x = @" ends with @" → real opener, hs_quote='"'. Body $(git commit --no-verify) → dropped (unconditional continue). "@ closes. PS_BLANKED = "$x = __GUARDRAILS_PS_HERESTRING__". No ( visible → has_special_constructs returns false → all guards allow. PowerShell executed git commit --no-verify via the subexpression.
The same construction hides git reset --hard, git push -f, or any write cmdlet from all five guards and the write gate.
Convergence-gate assessment
The file header at ps-command.sh:22–24 claims this class as an accepted residual:
"any shell variable / command substitution (never evaluated, the same residual as Bash)"
Under the ratified bar, whether the @"..."@ bypass is within-parity or documented residual turns on one factual question: does the Bash guard see raw here-doc body text when scanning?
- If yes: the Bash guard would detect
git commit --no-verifyin the literal body text of an expandable Bash here-doc (<<EOF…EOF); the PS blanker actively removes the equivalent body, creating an asymmetry — within-parity fail-open. - If no (i.e., the Bash guard also doesn't catch subcommands in expandable here-docs): then the behavior is symmetric and the finding is a documented residual consistent with the file header's claim.
This is the operator's call under the convergence gate. The fix (~5 lines) remains trivially available either way — blank only @'...'@ body lines, keep @"..."@ body lines visible:
# Replace the unconditional continue at line 113:
if [[ "$hs_quote" == "'" ]]; then
continue # literal body — safe to drop
fi
out+="${line}"$'\n' # expandable body executes $(...) — keep visibleThe canonical commit form (@'...'@) is entirely unaffected by this change.
No other findings
All prior findings are resolved:
- CRITICAL false here-string opener → fixed
7d82a00✅ - SUGGESTION stale
unparseablefunction names → fixed82f0e43✅ - P2
.exe-suffix launchers /startalias /writealias / parenthesized producers / module-qualified direct writers / call-operator boundary / computed launchers / computed writers / expression-literal redirect producers / fd-dup plumbing / script-block producers / POSIX git.exe normalization / separator-adjacent call operators / non-success stream producers → all fixed across rounds 4–10 ✅ 8674de0module-qualified redirect producers → fixed in this commit ✅
The rest of the surface — ps::might_invoke_git, ps::has_dynamic_invocation, ps::has_launcher, ps::has_special_constructs, the opener_scan false-opener fix, all hook injection surfaces (jq --arg TOOL_NAME/COMMAND, CLAUDE_PLUGIN_ROOT sourcing) — carries no additional security issues.
Summary for operator
8674de0 correctly closes the sole round-10 code-review finding (module-qualified redirect producer head comparison). The IMPORTANT @"..."@ body subexpression bypass is the only remaining security issue. Under the convergence gate, classifying it as a documented residual vs. a within-parity fix is the operator's call and hinges on how the Bash guard handles expandable here-doc bodies in the raw command text. Whichever way the gate lands, the 8674de0 change itself is correct and clean.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8674de09aa
ℹ️ 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".
| # matched only in its unambiguous Set-Content form. | ||
| # `\\` in the boundary class admits module-qualified spellings | ||
| # (`Microsoft.PowerShell.Management\Set-Content`) — same cmdlet, same write. | ||
| if [[ "$lcs" =~ (^|[[:space:]\;\|\&\(\\])(set-content|add-content|out-file|tee-object|ac|tee)([[:space:]]|$) ]]; then |
There was a problem hiding this comment.
When a writer name or short alias appears as an unquoted argument to another PowerShell command, such as git grep tee or Get-Command Set-Content, it is not invoking that writer. This regex matches any such token after whitespace, so block-hook-bypass.sh exits 2 and blocks benign searches/help commands; please segment to actual command positions before treating the token as a file write.
Useful? React with 👍 / 👎.
| '$'*) return 0 ;; # a variable / subexpression value redirected to a file | ||
| '['*) return 0 ;; # a cast/type expression value ([char]65 > f) |
There was a problem hiding this comment.
When the redirected producer is a PowerShell array or hashtable literal, e.g. @{a=1} > out.txt or @(1,2) > out.txt, > still writes expression output to a file. After the brace/paren unwrapping above, those heads become @a=1 / @1,2, but this expression-head check only catches $ and [, so the hook returns 0 and the Write/Edit gate can still be bypassed; include @ expression literals or fail closed on redirected expression heads.
Useful? React with 👍 / 👎.
|
TOWER CLAIM RELEASED → handed to merge-drive lane (tower clean-stop). Conditions for the lane to merge, all binding:
Operator ruling basis for the merge itself: #607 decision batch (tower #960 = the #915 fix, Bash-parity bar ratified). |
…1071) ## Summary Audit finding f4 (#912): the tracked convention config enforced nothing on its own — any direct `git commit` / `gh pr create` path skipped it, so the file was load-bearing only at skill draft time. This delivers the CC-layer content gate, the zero-setup half of the f4 design (`#919`'s opt-in commit-msg hook is the depth layer and ships separately). **New guard `block-convention-violation.sh`** (PreToolUse, `Bash|PowerShell`): - Validates the **commit subject** of the canonical stdin form — first non-empty line of the Bash heredoc / PowerShell here-string body — against the team-tracked `subject_pattern`, and the **`gh pr create --title`/`-t` value** against `pr_title_pattern` (incl. the `` Same as `subject_pattern`. `` deferral). - Reads patterns via the **vendored enforcement resolver** (`resolve-convention-pattern.sh`, synced from `lib/` — first registered consumer in `scripts/sync-resolve-convention-pattern.sh`, per the commit-convention seam #913/PR #925). - **Unresolved = no enforcement** — no team-tracked pattern / non-ERE pattern → no-op; never gates against the bundled Conventional Commits default (#912 contract 2). - **Never blocks `gh pr create` itself** — only a present-and-violating title; the documented inline fallback stays usable. - **Inherits `block-noncanonical-commit`'s exemption taxonomy** — `--amend`, `-C`/`-c`, `--fixup`/`--squash`, `-F <path>`, sequencer-in-progress — so rebases/merges are never content-gated. - **Declared bypass coverage** (out of scope, documented in the hook header): `gh pr edit --title`, `--fill`, direct API calls, babysit retitles, non-heredoc stdin producers. - Kill switch `block_convention_gate_enabled` (default true). PowerShell traffic reduces through the bundled classifier first; unparsable PS never reaches a content decision here. guardrails `0.10.3` → `0.11.0` with CHANGELOG entry. Grounded against the official hooks reference (matcher semantics, PreToolUse payload): <https://code.claude.com/docs/en/hooks>. ## Test plan - [x] New 25-case contract suite `block-convention-violation.test.sh` — unresolved/PCRE no-ops, subject block/allow, full exemption taxonomy, sequencer, non-heredoc skip, `--title` forms incl. deferral + edit/out-of-scope, PowerShell here-string forms, kill switch - [x] Sibling suites unchanged: block-noncanonical-commit 90/0, block-dangerous-git 251/0, flag-commit-pr-skill-bypass 27/0 - [x] `scripts/sync-resolve-convention-pattern.sh --check` — copy matches source - [x] `scripts/check-changelog-parity.sh --check-bump main` — pass - [x] shellcheck clean ## Related - Closes #914 - Refs #912 (locked contract), #913 / #925 (seam + resolver), #919 (depth layer, ships last), #960 (PowerShell classifier this gate composes with) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…t enforcement (#1077) ## Summary Audit f1's depth layer / f4's backstop (#912): a git `commit-msg` hook enforcing the team-tracked subject pattern **regardless of tool or shell** — editor commits, `git commit -F <file>`, IDE integrations, humans outside Claude — where the Claude-Code-layer gates see nothing. **New installable template** `lib/git-hooks/commit-msg-convention.sh` + **`/guardrails:setup apply install-commit-msg`** (explicit opt-in; bare `apply` still writes nothing): - **Chain-or-refuse.** Managed repos (`core.hooksPath`, lefthook, husky, pre-commit) → refused with the manager-side remediation. An existing `commit-msg` hook is never overwritten: chained as `commit-msg.pre-guardrails` (runs first, its rejection is final) or the install refuses. Covers the operator's machine-local commit-msg gate. - **Sentinel-marked** (`guardrails-commit-msg-convention`) so convention-inference tooling excludes the installed hook as a signal — it is derived FROM the tracked config; counting it would echo-cycle. Sentinel re-install is idempotent. - **Personal `.git/hooks/` lane only.** `core.hooksPath`, hook-manager configs, and tracked files are never touched — the committed team lane is a human PR decision, and `core.hooksPath` changes are exactly the shape `block-no-verify` refuses (interaction resolved by not going there). - **Unresolved = no enforcement** (never the bundled CC default); resolver copy removed → fail open, never block blind; `fixup!`/`squash!`/`amend!` exempt (autosquash). - **Deadlock designed out.** The rejection message instructs fixing the subject and never suggests `--no-verify` (which `block-no-verify` refuses in-session anyway); in Claude sessions the CC-layer gate (#914/PR #1071) blocks first, making this hook the cross-tool backstop. Reads the same resolver contract as the CC-layer gate (copied beside the hook at install time — an installed consumer-repo hook cannot participate in the sync seam, so it carries its own unedited copy). guardrails `0.11.0` → `0.12.0` with CHANGELOG entry. ## Test plan - [x] New 15-case contract suite `lib/git-hooks/commit-msg-convention.test.sh` — unresolved/PCRE pass-through, enforce block/allow, comment-line skipping, autosquash exemptions, empty message, resolver-removed fail-open, chain rejection-final + pass-through, sentinel presence - [x] `scripts/check-changed-skills.sh main` — setup skill PASS (0 errors) - [x] `scripts/check-changelog-parity.sh --check-bump main` — pass - [x] shellcheck clean; exec bits set ## Related - Closes #919 - Refs #912 (locked contract), #913/#925 (resolver seam), #1071 (CC-layer gate this backstops), #915/#960 (tool-bypass class this closes for non-CC surfaces) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
POST-MERGE SUITE CERTIFICATION (control tower, session 9) — conditions comment 5051463647 closed out. Timeline reconstruction: this PR was merged at 2026-07-22T20:40:30Z (squash, merge commit c87b07e, actor kyle-sexton) — 17 minutes BEFORE the tower posted the merge-drive handoff conditions (20:57:44Z). Merge-drive lane telemetry (#502, cycle 7, 20:12Z) records #960 as tower-managed/untouched, and the babysit safe tier honored the tower-lock — no lane claims the merge. Most probable actor: operator manual merge after the round-11 clean reviews on final head 8674de0 (20:35Z). Operator merges are always in-protocol; the conditions comment was written against a stale snapshot. Condition status:
All binding conditions now satisfied (2 retroactively). #964 is unblocked and claimed by the tower (issue comment 5051750345); fixer dispatched. Operator veto window: if the 20:40 merge was NOT yours, flag it — attribution shares the kyle-sexton login (#450). |
1 participant
What
Closes the proven-live security bypass in #912 f1: the guardrails git/commit guards matched only the Bash tool, so
git commit --no-verify(and every other guarded form) ran unblocked through Claude Code's opt-in PowerShell tool, which surfaces its command in the sametool_input.commandfield.block-no-verify,block-noncanonical-commit,block-dangerous-git,block-hook-bypass, andflag-commit-pr-skill-bypassfromBashtoBash|PowerShell.lib/powershell/ps-command.sh, that reduces a PowerShell command to a Bash-tokenizer-faithful form or fails closed.How it stays correct
git commit -F -— reduces to<placeholder> | git commit -F -and is allowed exactly as the Bash-F -form.git commit -m @'...'@reduces togit commit -m <placeholder>and is blocked byblock-noncanonical-commit.git commit/git push-shaped command carrying a construct the Bash tokenizer cannot faithfully handle (backtick,--%, subexpression, script-block grouping, or an unbalanced here-string) is blocked rather than waved through.block-dangerous-gitadditionally owns destructive non-commit forms (reset --hard,clean -fd,checkout/restore), so its fail-closed net is wider: it blocks any git-shaped PowerShell it cannot parse, not only commit/push — an unparseablegit --% reset --hardcannot slip through. The blanker is over-block-never-under-block on ambiguity: an unclosed here-string is left un-blanked so a trailing| git commit --no-verifycan never be swallowed into the placeholder.Set-Content,Add-Content,Out-File,Tee-Object, and content-producer>/>>redirects) are covered byblock-hook-bypass, producer-scoped like the Bash detection (a tool's own output redirect —git diff > out.txt— is still allowed).block-noncanonical-commitshows the here-string form on the PowerShell tool, not a Bash heredoc.Matcher token — doc basis
The official hooks reference confirms the matcher is exact-match with
|alternation and that the Bash tool's command lives intool_input.command. The hooks doc does not enumerate a "PowerShell" tool (it is opt-in viaCLAUDE_CODE_USE_POWERSHELL_TOOL=1); the tool namePowerShelland its identicaltool_input.commandfield are confirmed from anthropics/claude-code#57137. PowerShell here-string delimiter rules (opener ends its line; closer at column zero) are from about_Quoting_Rules. The empty-command bug in #57137 is the declarativeif-clause evaluator, not the matcher + stdin payload these hooks use (they buffer stdin andjqthe field themselves), so it does not affect this fix.Deferred to A2b (per #915)
Faithful parsing of the full PowerShell grammar (here-strings beyond the canonical form, backticks,
--%, subexpressions) and secret-pattern / hardcoded-path CONTENT scanning of PowerShell writes (those guards stayWrite|Edit-matched). These are exactly the constructs that trigger the fail-closed block here.Tests
Existing Bash cases unchanged and green; PowerShell cases added to each guard. Totals (FAIL=0 each): block-no-verify 93, block-noncanonical-commit 57, block-dangerous-git 205, block-hook-bypass 124, flag-commit-pr-skill-bypass 27. shellcheck / shfmt clean;
validate-plugins, changelog-parity (--check+--check-bump),sync-hook-utils --check, cross-plugin-source-drift, orphaned-fixtures, and silent-skips all pass. guardrails bumped 0.9.8 → 0.9.9.Related