wip(guardrails): wrapper-argv resolver for the env -S / sudo / -C fail-open family

[kyle-sexton]

Summary

DRAFT — work-in-progress save from an autonomous lane stopped at operator wind-down. Refs #1814. Refs #1811. Refs #1810. (Refs, not Fixes: verification is incomplete; a partial merge must not close the security family.)

Implements the wrapper-argv resolver closing the guardrails fail-open family: env -S restart bypass (#1814, critical), sudo option-cluster peeling losing the wrapper chdir (#1811), and block-convention-violation passing whole argv to effective_dir so a wrapper's -C hides a persisted alias (#1810). Implemented in lib/hook-utils.sh with tests and propagated to every plugin's vendored hooks/hook-utils.sh with per-plugin version bumps and CHANGELOG entries (56 files, +2829/-457).

State at stop

• Convention suite green (36/0) before the stop.
• NOT yet done: the remaining per-guard suites, unfixed-hook evidence for the guardrails: block-convention-violation passes its whole argv to effective_dir, so a wrapper's -C hides a persisted alias (fail-open, shared with main) #1810 rows, the delegated-authority decision comment on guardrails: hook::git_resolve_index restarts outside env parsing after -S, so env -S '-i git commit -m x' bypasses every git guard (fail-open, shared with main) #1814, an independent fresh-context review, and the full repo gate battery (changelog parity x3, check-changed-skills, shellcheck across all vendored copies, portability gates).
• Overlap note: PR fix(guardrails): defer unparsable PowerShell commits to the guards that can block them (#1858) #1860 (merged or open) touched the same two guard files on a different code path (rc=2 deferral); rebase over main before finishing.

Test plan

• Full guardrails suite per guard (dangerous-git, no-verify, noncanonical-commit, convention-violation, hook-bypass)
• Pinned bypass shapes: env -S '-i git commit -m x', sudo -bD <dir> git ..., wrapper -C alias-hiding, plus adversarial variants
• Repo gate battery + CI

Related

No linked issue closed by this draft. Refs #1814 #1811 #1810. Resume context: this body + the three issue threads carry everything a cold agent needs; local worktree claude-code-plugins-issue-1814.

[claude]

Claude finished @kyle-sexton's task in 4m 42s —— View job

---

Security Review — PR #1864 (wrapper-argv resolver)

Task list

• Gather context (PR body, changed files list)
• Fetch diff (git diff origin/main...HEAD)
• Review lib/hook-utils.sh resolver logic (env -S / sudo / -C handling)
• Review plugins/guardrails/hooks/block-*.sh call sites
• Review vendored-copy propagation for drift/inconsistency
• Post findings

Scope

Reviewed the diff against origin/main (git diff origin/main...HEAD). The 56 changed files reduce to two units of unique logic: lib/hook-utils.sh (the resolver, vendored byte-identically into 13 plugins per the diff's identical +136/-24/+160 hunks and matching blob SHAs) and the four plugins/guardrails/hooks/block-*.sh call sites plus their test files. .claude-plugin/plugin.json and CHANGELOG.md changes are version bumps and prose only. No workflow files changed.

Findings

IMPORTANT — block-convention-violation.sh silently loses its own bypass coverage when its documented independent kill switch is used as designed

block-convention-violation.sh treats any nonzero hook::git_resolve_index return the same way (skip/allow):

hook::git_resolve_index "$@" || return 0

block-convention-violation.sh#L268

This deliberately folds rc 1 (not-git) and rc 2 (the resolver's new fail-closed refusal — an unparseable wrapper prefix, e.g. sudo -i git commit -m x, with something git-shaped downstream) into one skip path. The inline comment explains why: "rc 2 ... is the MECHANIC guard's fail-closed concern — it blocks the call outright, so this content gate just skips" — i.e., it relies on block-noncanonical-commit.sh (the sibling "mechanic" guard) to have already blocked the same call outright. The same reliance is repeated for the --config-env-shaped alias refusal at L296.

That assumption breaks under a configuration the plugin explicitly documents and supports: each of the twelve guards, including these two, is "independently toggleable" (plugin.json description, and README.md:5), with separate kill switches — block_noncanonical_commit_enabled vs. block_convention_gate_enabled (plugin.json#L53, plugin.json#L59).

Failure scenario: a user/team runs with block_noncanonical_commit_enabled=false (they don't care about the -F - piping mechanic) but keeps block_convention_gate_enabled=true (they do enforce ticket-ID subject conventions via .claude/source-control.md) — a fully supported combination per the plugin's own docs. Under that combination, sudo -i git commit -F - <<'EOF' (or any wrapper shape the resolver refuses, or a --config-env-shaped alias) now bypasses the convention-violation content gate entirely and silently, because the guard whose fail-closed behavior it was implicitly counting on never runs. This is exactly the class of guardrail bypass the #1814/#1811/#1810 family exists to close, reopened at a cross-guard trust boundary that isn't visible from either hook's own code.

The block-dangerous-git.sh and block-no-verify.sh guards do not have this issue — each independently blocks on rc 2 itself (see block-dangerous-git.sh#L444-450 and the equivalent in block-no-verify.sh).

Confidence: High that the coupling exists and is exploitable exactly as designed; the plugin's own README/plugin.json confirm the "independently toggleable" contract that the implicit cross-process dependency violates. Whether this is accepted risk (WIP PR's own body already flags "verification is incomplete") is a product decision, but it's currently undocumented at the code site making the assumption.

Other areas reviewed, no issues found

• env -S/--split-string splice (the guardrails: hook::git_resolve_index restarts outside env parsing after -S, so env -S '-i git commit -m x' bypasses every git guard (fail-open, shared with main) #1814 restart-outside-parsing bug): traced the new w=(env ...); i=1; continue resume-inside-env-parsing logic through nested/attached/chdir-before/chdir-inside cases — no bypass found; matches the added test coverage.
• sudo option-cluster grammar (guardrails: hook::git_resolve_index does not peel sudo option clusters, so sudo -bD <dir> git … loses the wrapper chdir (fail-open, shared with main) #1811): the valueless-short peel list, value-taking short/long classification, and the fail-closed *) backstop (git-shaped substring scan, rc 2) were traced against sudo(8) semantics for clustered/attached/separate forms — no misclassification found that would leave a value word at the command position or swallow the command word. The backstop's case-insensitive substring scan is conservative in the safe direction (may over-block, never under-blocks).
• hook::git_effective_dir / effective_dir composition (guardrails: block-convention-violation passes its whole argv to effective_dir, so a wrapper's -C hides a persisted alias (fail-open, shared with main) #1810): the persisted-alias lookup and sequencer probe now slice only [gi, sub_idx) plus HOOK_GIT_RESOLVED_WRAPPER_DIRS rather than the whole argv; the composed path is passed as a literal git -C argument (no shell re-interpretation), so no injection surface there.
• Vendored propagation: all 13 plugins/*/hooks/hook-utils.sh copies share the identical blob SHA 724b951... — no per-plugin drift introduced.
• No workflow files, permissions, or CI configuration touched by this PR.

Per this lane's scope, unpinned-action/supply-chain risk, dangerous trigger patterns, and template injection are deferred to zizmor's static advisory lane and not re-reported here (none applicable — no workflow files changed anyway).

[kyle-sexton]

Branch refreshed against main — version allocation needs adjudication

Brought current with origin/main (1e08c6a5c6) by three ordinary merge commits. No rebase, no amend, no force-push: the original commit bc8fa79e93 is still the only non-merge commit in origin/main..HEAD, and the push was a fast-forward (bc8fa79e93..0997a0adcb).

Why this needed 16 version decisions, not one

This branch bumps every plugin that vendors hooks/hook-utils.sh. Main bumped all of them too while this branch sat, so all 16 version claims collided — and seven collided exactly, which is worse than stale: they would have re-claimed a number main has already published with different content.

Every number below is now strictly greater than what main has shipped.

plugin	main (shipped)	this branch	note
actionlint	0.7.7	0.7.8	was an exact collision
autonomy	0.12.1	0.12.3	centrally allocated (0.12.2 claimed by docs/fixbatch-goal-budget-cap)
bash-format	0.6.10	0.6.11	provisional
biome-format	0.5.8	0.5.9	was an exact collision
claude-ops	0.27.1	0.27.2	collided with #1945 mid-refresh — see below
context-guard	0.4.5	0.4.6	provisional
desktop-notification	0.5.9	0.5.10	was an exact collision
eol-normalizer	0.5.8	0.5.9	was an exact collision
go-format	0.2.8	0.2.9	was an exact collision
guardrails	0.19.0	0.19.1	provisional
markdown-format	0.9.0	0.9.1	provisional
powershell-format	0.6.5	0.6.6	was an exact collision
rate-limit-guard	0.4.0	0.4.1	provisional
ruff-format	0.5.8	0.5.9	was an exact collision
source-control	0.45.1	0.45.2	provisional
typos-format	0.5.0	0.5.1	provisional

Only autonomy carries a centrally allocated number. The other fifteen are provisional, assigned by the rule "next free patch above main". That rule cannot see what sibling open branches have already claimed, so these fifteen need central adjudication before this leaves draft.

The claude-ops collision is why that matters

Mid-refresh, main merged #1945, which shipped claude-ops 0.27.1 — the exact number this branch had provisionally taken. Re-resolved to 0.27.2.

check-changelog-parity.sh --check-bump cannot catch this. An equal version reads as "not changed", so the entry check is skipped and the gate passes while two different 0.27.1 releases exist. Any future refresh of this branch must compare each plugin's version against main's directly, not rely on the gate.

Conflict resolution

• lib/hook-utils.sh + all 16 vendored copies — auto-merged, both sides intact. Main's addition (hook::under_temp_root and the hook::read_file_path temp exemption, typos-format hook rewrites code identifiers outside any repository, including inside fenced code blocks #1769) sits in a different region from this branch's git_resolve_index / git_effective_dir work. All 16 vendored copies verified byte-identical to lib/hook-utils.sh by cmp and by scripts/sync-hook-utils.sh --check.
• CHANGELOGs — main's structure kept verbatim, including its relocation of the "predate this file" preamble to directly under the header. This branch's entry is re-inserted above main's newest under the new version. Verified as a pure insertion over main for all 16 (zero deleted lines).
• Manifests — main's file, with the version line alone advanced. Confirmed this branch had changed nothing else in any manifest.

Gates

sync-hook-utils.sh --check, check-changelog-parity.sh in all three modes (--check, --check-order, --check-bump origin/main), block-convention-violation.test.sh (36/0), block-noncanonical-commit.test.sh (183/0) — all green.

lib/hook-utils.test.sh fails 2–4 tests in the buffer_stdin wall-clock timing family. Pre-existing on main, not introduced here: hook::buffer_stdin is byte-identical between main and this branch, the branch's only change to that test file is a pure insertion after the affected tests, and two runs of the same baseline commit produced different failing sets. On the guard suites this branch is greener than main (183/0 vs main's 170/2).

Still a draft

The refresh did not touch the test plan in the PR body. The remaining per-guard suites, the unfixed-hook evidence for the #1810 rows, the delegated-authority decision comment on #1814, and an independent review of the resolver logic itself are all still open. "Gates green" here means the merge is clean and the versions are consistent — not that this is ready to merge.

[claude]

Claude finished @kyle-sexton's task in 2m 52s —— View job

---

Security Review — PR #1864 (wrapper-argv resolver)

Task list

• Gather context (PR body, changed files list)
• Fetch diff (git diff origin/main...HEAD at refreshed HEAD b9fdb944)
• Review lib/hook-utils.sh resolver logic (env -S / sudo / -C handling)
• Review plugins/guardrails/hooks/block-*.sh call sites
• Review vendored-copy propagation for drift/inconsistency
• Post findings

Scope

This is a re-review after the branch was refreshed against origin/main (merge commits 0997a0a/b9fdb94, plus a version-allocation commit). The refresh is a merge + version bump, not new logic — hook::git_resolve_index, hook::git_effective_dir, the sudo/env branches, and the three guard call sites are unchanged in substance from the prior review pass. Re-verified against the new HEAD rather than assumed.

Findings

IMPORTANT — block-convention-violation.sh still folds the resolver's fail-closed refusal (rc 2) into its skip path, defeating the guards' own "independently toggleable" contract

hook::git_resolve_index "$@" || return 0

block-convention-violation.sh#L268

Unchanged from the prior pass. block-dangerous-git.sh, block-no-verify.sh, and block-noncanonical-commit.sh were all updated in this refresh to handle rc 2 explicitly and fail closed themselves:

hook::git_resolve_index "$@"
case $? in
0) ;;
2) block "unparseable-wrapper" "BLOCKED: ..." ;;
*) return 0 ;;
esac

block-dangerous-git.sh#L438-L450, block-no-verify.sh#L141-L153, block-noncanonical-commit.sh#L538-L551 — each carrying the comment "per-hook kill switches mean no guard may delegate its fail-closed posture [to a sibling]".

block-convention-violation.sh alone keeps the old delegation, now spelled out explicitly in both the inline comment and the new CHANGELOG entry:

"block-convention-violation skips, per its existing rule that unparseable FORM is the mechanic guard's concern."
plugins/guardrails/CHANGELOG.md#L41-L42

That written rationale directly contradicts the sibling guards' own stated invariant in the same commit ("no guard may delegate its fail-closed posture"), and contradicts the plugin's shipped contract that all twelve guards, including these two, are independently toggleable with separate kill switches — block_noncanonical_commit_enabled vs. block_convention_gate_enabled (plugin.json#L53, #L59). The manifest description confirming this is itself part of this diff and unchanged by the refresh.

Failure scenario, unchanged: a config with block_noncanonical_commit_enabled=false (don't care about the -F - piping mechanic) and block_convention_gate_enabled=true (do enforce ticket-ID subjects) is fully supported per the plugin's own docs. Under that combination, any wrapper shape the resolver refuses on rc 2 — sudo -i git commit -F - <<'EOF', sudo -h host git commit, an unknown/unclassifiable sudo option — now bypasses the convention-violation content gate silently, because the sibling guard whose fail-closed behavior it implicitly relies on never runs. This is the same class of bypass the #1814/#1811/#1810 family exists to close, reopened at a cross-guard trust boundary neither hook's own code makes visible.

No test in block-convention-violation.test.sh exercises this guard with block_noncanonical_commit_enabled=false, so the gap has no regression coverage either.

Confidence: High that the coupling exists and is exploitable exactly as designed and now explicitly documented as accepted, rather than fixed. Whether that's the intended resolution is a product call for the maintainer — the PR body itself still says "verification is incomplete" — but the CHANGELOG language asserting this is "per its existing rule" reads as treating a known gap as settled when the sibling guards' own new comments argue against that resting position.

Other areas reviewed, no issues found

• env -S/--split-string resume-inside-parsing fix (guardrails: hook::git_resolve_index restarts outside env parsing after -S, so env -S '-i git commit -m x' bypasses every git guard (fail-open, shared with main) #1814): the splice now keeps a synthetic leading env word and resumes at i=1 inside env's own option loop rather than restarting the outer command scan. Traced nested/attached/chdir-before/chdir-inside/NAME=value-terminates-option-parsing cases against the new test block in lib/hook-utils.test.sh — behavior matches GNU env's actual continued-argv semantics; no bypass found.
• sudo option-cluster grammar (guardrails: hook::git_resolve_index does not peel sudo option clusters, so sudo -bD <dir> git … loses the wrapper chdir (fail-open, shared with main) #1811): re-checked the valueless-short peel set (-A -B -b -E -e -H -K -k -l -N -n -P -S -s -v -V) and value-taking set (-C -g -p -R -r -T -t -U -u, plus -D/--chdir) against sudo(8) semantics — counts and letters line up correctly, and the deliberately-excluded -h (optional-argument ambiguity) and -i/--login (unrecordable relocation) correctly fall through to the fail-closed backstop rather than being misclassified. The backstop's case-insensitive git substring scan is conservative in the safe direction.
• hook::git_effective_dir / effective_dir composition (guardrails: block-convention-violation passes its whole argv to effective_dir, so a wrapper's -C hides a persisted alias (fail-open, shared with main) #1810): both block-noncanonical-commit.sh and block-convention-violation.sh now delegate to the one shared hook::git_effective_dir and pass only [gi, sub_idx) plus HOOK_GIT_RESOLVED_WRAPPER_DIRS, not the whole argv. New tests (env -u -C git x, env -C other git y) pin both the fixed decoy-argv case and the still-honored genuine-relocation case. Composed path is a literal git -C argument — no shell re-interpretation, no injection surface.
• Vendored propagation: all 13 plugins/*/hooks/hook-utils.sh copies share blob SHA 724b951772dccc04d266a268b7eb58c23eb20a9e per the diff metadata — no per-plugin drift.
• Version-allocation refresh itself: manifest/CHANGELOG changes in this pass are version bumps and prose; no logic changed there. No workflow files touched.

Per this lane's scope, unpinned-action/supply-chain risk, dangerous trigger patterns, and template injection are deferred to zizmor's static advisory lane (not applicable anyway — no workflow files changed).

[kyle-sexton]

Version allocation resolved — supersedes the provisional table above

All sixteen numbers are now centrally allocated, verified against every open PR's actual head claim. Head is b9fdb9443e.

Thirteen of my provisional numbers were confirmed unchanged. Three moved up to clear versions held by open PRs that a "next patch above main" rule could not see:

plugin	provisional	allocated	reason
bash-format	0.6.11	0.6.12	#1867 claims 0.6.11
rate-limit-guard	0.4.1	0.4.2	#1866 claims 0.4.1
source-control	0.45.2	0.46.1	#1830 claims 0.46.0

Final allocation: actionlint 0.7.8 · autonomy 0.12.3 · bash-format 0.6.12 · biome-format 0.5.9 · claude-ops 0.27.2 · context-guard 0.4.6 · desktop-notification 0.5.10 · eol-normalizer 0.5.9 · go-format 0.2.9 · guardrails 0.19.1 · markdown-format 0.9.1 · powershell-format 0.6.6 · rate-limit-guard 0.4.2 · ruff-format 0.5.9 · source-control 0.46.1 · typos-format 0.5.1.

autonomy 0.12.3 was allocated to clear 0.12.2, which docs/fixbatch-goal-budget-cap was holding. That branch has since merged as #1960 and main now ships 0.12.2 — so the allocation anticipated exactly the collision that then occurred, and 0.12.3 still clears it.

Brought current through two more merges (c1abdecc39, then a2a526ad78). All 16 versions verified strictly greater than main by semver comparison — not string comparison, which matters for 0.5.10 > 0.5.9 and 0.6.12 > 0.6.10 — with each CHANGELOG entry ordered above that plugin's newest shipped entry.

An independent fresh-context verifier, rationale withheld and read-only, passed exact-allocation match, changelog ordering, survival of the resolver delta (all 16 vendored copies byte-identical to lib/hook-utils.sh), gates, and integrity. Its only FAIL was ancestry against a main that advanced mid-run; that commit has since been merged.

Still a draft — the PR's own test plan is untouched by this refresh.

[kyle-sexton]

Status note (2026-08-09 board sweep): this draft is the standing WIP artifact for the wrapper-argv fail-open family — issues #1814, #1811, #1810 remain the durable record, and this branch holds the in-progress resolver. Deliberately open-as-draft, not orphaned; it graduates to ready when the resolver covers the env -S / sudo / -C family with tests, or closes if that work restarts fresh.

[kyle-sexton]

Closing superseded WIP — the wrapper-argv resolver work landed on main via merged PRs while this draft stayed open.

Evidence: #1814's env -S fix in PR #2147; #1810's effective_dir scoping in PR #2152 (#2113); shared parser from #1785. This draft explicitly said "Refs, not Fixes" and listed incomplete verification — those fixes are now shipped and tested on main.