fix(guardrails): peel sudo clustered short options for chdir resolution by kyle-sexton · Pull Request #2483 · melodic-software/claude-code-plugins

kyle-sexton · 2026-08-12T15:54:13Z

Fixes #1811

hook::git_resolve_index now peels documented valueless sudo shorts from clustered tokens before matching -D/--chdir.

Test plan

bash lib/hook-utils.test.sh (187/0)

🟡 lib/hook-utils.sh:1456 — the comment documenting the peeled valueless-short set (-b/-E/-H/-K/-k/-n/-s/-v) doesn't match the actual regex class, which also peels -h and -S.
🟡 lib/hook-utils.sh:1471 — -h is now treated as valueless when clustered (new peel regex) but value-taking when standalone (pre-existing case arm a few lines below), an inconsistency the new logic makes more visible.
🔴 scripts/verify-security-review-evidence.sh:181-193 — has_execution_evidence is only consulted inside the duration < 8s branch, so it never actually applies to the 15-30s "legitimate review" case the commit message cites as motivation (the lowered floor alone handles that). More importantly, the old 45s floor's coverage of the 8-45s range is now dropped entirely with no equivalent check, narrowing the ci(security-review): the required security-review check reported pass in 16s on #2287 while both reviewer jobs failed — a required gate that can fail open #2337 false-pass guard, and scripts/verify-security-review-evidence.sh.test.sh wasn't updated to cover any of the new branches.

Other notes (not code-anchored)

PR scope: this PR bundles two unrelated fixes under one title/description — the sudo clustering fix (0b2b853, matches the title and issue 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) and an unrelated CI evidence-gate threshold/heuristic change (273f7fa, not mentioned in the PR body at all). Worth splitting or at least describing both in the PR body so revert/bisect stays unambiguous.
Good: the 20 synced copies of hook-utils.sh under plugins/*/hooks/ are byte-identical to the canonical lib/hook-utils.sh — confirmed via diff, not just author claim. The plugins/guardrails version bump (0.28.9 → 0.28.10) and CHANGELOG entry correctly describe the fix.
Unable to independently verify: this sandbox's Bash approval gate blocked running bash lib/hook-utils.test.sh and even simple source/inline snippets, so the author-claimed "187/0" test-plan result is unverified here — I instead manually traced the peeling regex against representative inputs (-bDother, -nDother) by reading the code, which checks out for the documented cases. I could not execute the security-review-evidence script's log-format assumptions (e.g. whether Claude Code action completed actually appears in real workflow logs) against a real run either.
Security-relevant aspects of this diff (if any) are left to the dedicated claude-security-review lane per this repo's REVIEW.md scope split.

claude · 2026-08-12T15:54:43Z

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

Security Review

Gather context (PR diff, changed files)
Review lib/hook-utils.sh sudo-peeling logic for correctness/bypass risk
Review scripts/verify-security-review-evidence.sh changes
Check plugin copies stayed in sync with lib/hook-utils.sh
Report findings

Reviewed only the files changed in this PR (gh/git diff origin/main...HEAD). Deferred to zizmor for supply-chain/pinning/trigger/permission-widening/template-injection concerns in workflow YAML — none of that lane applies here since no workflow files changed.

IMPORTANT — sudo cluster-peel set is missing several documented valueless short flags, leaving the chdir-detection bypass only partially closed

File: lib/hook-utils.sh (and its 15 synced plugin copies), lines 1444–1477
Permalink:

claude-code-plugins/lib/hook-utils.sh

Lines 1444 to 1477 in 0b2b853

    
           sudo) 
        
             # sudo's own chdir is -D/--chdir (its -C is close-from, -R is --chroot). 
        
             # GNU sudo clusters short options, so `-bD dir` carries a chdir that no 
        
             # exact `-D` match sees. Peel the documented valueless shorts (-b/-E/-H/-K/-k 
        
             # -n/-s/-v, per `sudo --help`) off a single-dash token so the value-taking 
        
             # tail (-D/--chdir, -u/-g/-h/-p/-C/-R/-T) reaches its own branch. 
        
             # `-i` relocates to the target user's home without naming a directory at all. 
        
             ((i++)) 
        
             local sudo_ci=-1 stok 
        
             while ((i < n)) && [[ "${w[i]}" == -* ]]; do 
        
               stok="${w[i]}" 
        
               if [[ "$stok" == -[!-]* ]]; then 
        
                 while [[ "$stok" =~ ^-[bnEhHkKsSv](.+)$ ]]; do stok="-${BASH_REMATCH[1]}"; done 
        
               fi 
        
               case "$stok" in 
        
               -D | --chdir) 
        
                 ((i + 1 < n)) && hook::wrapper_chdir_record sudo_ci "${w[i + 1]}" 
        
                 ((i += 2)) 
        
                 ;; 
        
               --chdir=*) 
        
                 hook::wrapper_chdir_record sudo_ci "${stok#--chdir=}" 
        
                 ((i++)) 
        
                 ;; 
        
               -D*) 
        
                 hook::wrapper_chdir_record sudo_ci "${stok#-D}" 
        
                 ((i++)) 
        
                 ;; 
        
               -u | -g | -h | -p | -C | -R | -T | --user | --group) ((i += 2)) ;; 
        
               -*) ((i++)) ;; 
        
               *) ((i++)) ;; 
        
               esac 
        
             done 
        
             continue 
        
             ;;

The new peel loop only strips b, n, E, h, H, k, K, s, S, v off a clustered short-option token before matching -D/--chdir:

while [[ "$stok" =~ ^-[bnEhHkKsSv](.+)$ ]]; do stok="-${BASH_REMATCH[1]}"; done

sudo --help documents additional valueless short options this set omits: -A (askpass), -B (bell), -e (edit), -i (login — noted in the comment as an accepted gap, but not the others), -l (list), -P (preserve-groups), -V (version). Any of these placed before D in a clustered token still defeats detection, e.g.:

sudo -AD /other/repo git push --force origin main

stok stays -AD, matches neither -D | --chdir nor -D*, falls through to the generic -*) ((i++)) arm, and the chdir is silently dropped — the exact "fail-open" failure mode this PR sets out to fix, just for a narrower set of flags than sudo actually defines as valueless.

This matters because HOOK_GIT_RESOLVED_WRAPPER_DIRS (populated by hook::wrapper_chdir_record) feeds directly into git_locating_opts in plugins/guardrails/hooks/block-dangerous-git.sh (and similarly block-no-verify.sh, block-noncanonical-commit.sh, block-convention-violation.sh), which is how those guardrails decide which repository a -C/git -C inspection targets. A dropped chdir means the dangerous-git-command guard evaluates the wrong working tree/repo — a guardrail bypass, not just a cosmetic parsing gap.

The new tests (lib/hook-utils.test.sh:2166-2167) only cover -b and -n, so this gap isn't caught by the added coverage either.

Suggested fix: extend the peel character class to the full valueless set sudo documents (AbBEeHhiKklnPSsVv, keeping -i excluded per the existing documented rationale, or handle it explicitly), and add test cases for the currently-untested letters.

Fix this →

Confidence: High — verified against sudo's documented option grammar and traced the data flow into block-dangerous-git.sh's repo-targeting logic.

SUGGESTION — execution-evidence markers in `verify-security-review-evidence.sh` are weak signals, not proof the review actually ran

File: scripts/verify-security-review-evidence.sh, lines 181–193
Permalink:

claude-code-plugins/scripts/verify-security-review-evidence.sh

Lines 181 to 193 in 0b2b853

    
           # Positive execution evidence: the Claude review action progressed past 
        
           # install into the agent SDK (or posted a review). Prefer this over a 
        
           # wall-clock ceiling — small in-scope PRs routinely finish a real review 
        
           # in 15–30s and must not be blocked as "#2337 false passes". 
        
           local has_execution_evidence=0 
        
           if grep -qE '@anthropic-ai/claude-agent-sdk|mcp__github_inline_comment__create_inline_comment|Posted review|create_inline_comment|Claude Code action completed' "$log_file" 2>/dev/null; then 
        
             has_execution_evidence=1 
        
           fi 
        
           if (( duration > 0 && duration < MIN_REVIEW_SECONDS && has_execution_evidence == 0 )); then 
        
             echo "ERROR: in-scope security-review succeeded in ${duration}s (<${MIN_REVIEW_SECONDS}s) with no execution evidence — likely no review ran (#2337)" >&2 
        
             exit 1 
        
           fi

This script exists specifically to catch a security-review job that reports success without actually performing a review (#2337). The PR replaces a flat duration < 45s check with duration < 8s AND no execution evidence, where evidence is:

grep -qE '@anthropic-ai/claude-agent-sdk|mcp__github_inline_comment__create_inline_comment|Posted review|create_inline_comment|Claude Code action completed' "$log_file"

@anthropic-ai/claude-agent-sdk is the npm package name for the SDK the action depends on — it is very likely to appear in the job log the moment npm install/dependency resolution runs, near the start of essentially every invocation, regardless of whether the review logic itself ever executes or completes. Likewise, tool names such as create_inline_comment can appear in agent-framework logs simply because the tool is declared/available to the model, not only when it is actually invoked — this depends on how the upstream claude-code-action logs its tool registry, which isn't verifiable from this repo.

If either of those substrings shows up regardless of whether a real review ran, this new evidence check can make has_execution_evidence=1 in exactly the "job finished suspiciously fast with no real review" scenario the guard was built to catch, silently defeating the fail-closed intent for the lowered MIN_REVIEW_SECONDS (45s → 8s). Combined, the net effect is a narrower detection window for the false-pass class this file's whole purpose is to catch.

Flagging as a suggestion rather than a confirmed bug because I can't inspect the upstream ci-workflows/claude-code-action log output from this repo to confirm exactly when these strings are emitted — but given the sensitivity of this control (it's the safety net for a security-review false-pass class already seen in production, #2337), it's worth having someone with visibility into the actual action logs confirm these markers can only appear on genuine completion (e.g. prefer a single terminal marker like Claude Code action completed or an explicit "review posted" line over including the SDK package name, which is present from process startup).

Confidence: Medium — reasoning is sound but depends on external log behavior not visible in this repo.

No other security-relevant issues found in the changed files (plugin.json version bump and CHANGELOG.md entry are non-functional; all 15 plugin hook-utils.sh copies remain byte-identical to lib/hook-utils.sh).
· branch cursor/sudo-cluster-chdir-peel-63e1

github-actions · 2026-08-12T15:59:35Z

Claude has reviewed this PR 2 times. The lane skips further automatic reviews after 5; deleting this comment resets the count.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0b2b853370

ℹ️ 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 · 2026-08-12T17:03:09Z

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

Security Review

Gather context (git diff origin/main...HEAD, changed files)
Review lib/hook-utils.sh sudo-peeling logic for correctness/bypass risk
Confirm scripts/verify-security-review-evidence.sh is out of scope (no longer changed by this PR since the last sync — only the sudo-peel fix remains)
Check plugin copies stayed in sync with lib/hook-utils.sh
Report findings

Reviewed only the files changed at HEAD (968074300476faaacd735ad5405cb4db3f545998): lib/hook-utils.sh, lib/hook-utils.test.sh, and the 16 synced plugin copies of hook-utils.sh, plus plugins/guardrails/.claude-plugin/plugin.json / CHANGELOG.md (non-functional version bump). Note: scripts/verify-security-review-evidence.sh is not part of this PR's diff at the current head (it was in an earlier commit that's no longer on this branch), so it's out of scope for this pass.

CRITICAL — `-h` is misclassified as a valueless short flag, still allowing a full guardrail bypass via `sudo -hD …`

File: lib/hook-utils.sh (and its 16 synced plugin copies)
Peel regex:

claude-code-plugins/lib/hook-utils.sh

Line 1456 in 9680743

while [[ "$stok" =~ ^-[bnEhHkKsSv](.+)$ ]]; do stok="-${BASH_REMATCH[1]}"; done

Conflicting value-taking case arm:

claude-code-plugins/lib/hook-utils.sh

Line 1471 in 9680743

-u | -g | -h | -p | -C | -R | -T | --user | --group) ((i += 2)) ;;

The new peel loop strips h off a clustered token before matching -D/--chdir:

while [[ "$stok" =~ ^-[bnEhHkKsSv](.+)$ ]]; do stok="-${BASH_REMATCH[1]}"; done

but five lines later, in the same sudo) case block, a standalone -h is treated as value-taking (-u | -g | -h | -p | -C | -R | -T | --user | --group) ((i += 2)) ;;). Real sudo's -h is overloaded (--help, no argument, vs. -h host/--host=host, which does take one) — the two branches of this same function now assert opposite grammars for the identical flag.

Traced sudo -hD git push --force origin main against the code (w=(sudo -hD git push --force origin main)):

stok="-hD" matches the peel regex (h is in the class), becomes stok="-D".
-D matches the exact -D | --chdir case, which reads w[i+1] — i.e. the literal word git — as the chdir directory, records HOOK_GIT_RESOLVED_WRAPPER_DIRS+=("git"), and advances i by 2, consuming both -hD and git.
The outer loop resumes at push, which is not a recognized wrapper/reserved word and fails hook::git_is_bin, so hook::git_resolve_index returns 1.
In plugins/guardrails/hooks/block-dangerous-git.sh:678, hook::git_resolve_index "$@" || return 0 — a failed resolve means check_segment returns success (no block) unconditionally. The force-push is never inspected at all, not just mis-scoped to the wrong directory.

This is the exact "fail-open" class this PR sets out to close (issue #1811), just not closed for -h. It's also not caught by the new tests — lib/hook-utils.test.sh only adds cases for -b and -n (lines 2166-2167).

Confidence: High — traced step-by-step against the actual code in this diff and the downstream consumer in block-dangerous-git.sh.

IMPORTANT — peel set omits other documented valueless sudo shorts, leaving the same full-bypass gap for those flags

File: lib/hook-utils.sh line 1456 (and synced copies)

sudo's own usage grammar (sudo --help) lists additional valueless short options not in the peel class [bnEhHkKsSv]: -A (askpass), -B (bell), -e (edit), -l (list), -P (preserve-groups), -V (version) — -i is the only gap the code's comment explicitly calls out as accepted. For any of these placed before D in a cluster (e.g. sudo -AD /other/repo git push --force), stok stays -AD, matches none of the specific cases, falls to the generic -*) ((i++)) arm, and the next word (/other/repo) is left at the front of the loop — which then exits (it doesn't start with -), and the outer resolver fails to recognize git at all, hitting the same hook::git_resolve_index … || return 0 full-bypass path as the -h case above (not merely a missed chdir).

This narrows, but does not close, the gap the pre-PR comment already documented as a known limitation — the commit's stated goal (peeling "documented valueless sudo shorts") is met for 8 of the ~14 valueless shorts sudo actually defines.

Confidence: High that the flag-set gap exists as described (matches sudo's documented option grammar); same downstream bypass mechanism as the CRITICAL finding above.

No other security-relevant issues found in the files this PR currently changes. All 16 plugin copies of hook-utils.sh remain byte-identical to lib/hook-utils.sh (verified via diff, not author claim); plugin.json/CHANGELOG.md changes are non-functional.
· branch cursor/sudo-cluster-chdir-peel-63e1

claude · 2026-08-12T17:21:09Z

Claude finished @kyle-sexton's task in 1m 35s —— View job

Security Review

Gather context (git diff origin/main...HEAD, changed files)
Review lib/hook-utils.sh sudo-peeling logic for correctness/bypass risk
Check plugin copies stayed in sync with lib/hook-utils.sh
Report findings

Reviewed the files changed at HEAD (a9eb9fa7f35a0c823a276e0b59dbffc52b0f5eac): lib/hook-utils.sh, lib/hook-utils.test.sh, the 16 synced plugin copies of hook-utils.sh, and plugins/guardrails/.claude-plugin/plugin.json / CHANGELOG.md (non-functional version bump). This is the same sudo-peeling logic reviewed in an earlier pass on this branch and it is unchanged — the findings below still apply at the current HEAD.

CRITICAL — `-h` is misclassified as a valueless short flag, still allowing a full guardrail bypass via `sudo -hD …`

File: lib/hook-utils.sh (and its 16 synced plugin copies)
Peel regex:

claude-code-plugins/lib/hook-utils.sh

Line 1456 in a9eb9fa

while [[ "$stok" =~ ^-[bnEhHkKsSv](.+)$ ]]; do stok="-${BASH_REMATCH[1]}"; done

Conflicting value-taking case arm:

claude-code-plugins/lib/hook-utils.sh

Line 1471 in a9eb9fa

-u | -g | -h | -p | -C | -R | -T | --user | --group) ((i += 2)) ;;

The peel loop strips h off a clustered token before matching -D/--chdir:

while [[ "$stok" =~ ^-[bnEhHkKsSv](.+)$ ]]; do stok="-${BASH_REMATCH[1]}"; done

but 15 lines later, in the same sudo) case block, a standalone -h is treated as value-taking (-u | -g | -h | -p | -C | -R | -T | --user | --group) ((i += 2)) ;;). Real sudo's -h is overloaded (--help, no argument, vs. -h host/--host=host, which does take one) — the two branches of this same function assert opposite grammars for the identical flag.

Tracing sudo -hD git push --force origin main (w=(sudo -hD git push --force origin main)):

stok="-hD" matches the peel regex (h is in the class), becomes stok="-D".
-D matches the exact -D | --chdir case, which reads w[i+1] — the literal word git — as the chdir directory, records HOOK_GIT_RESOLVED_WRAPPER_DIRS+=("git"), and advances i by 2, consuming both -hD and git.
The outer loop resumes at push, which isn't a recognized wrapper/reserved word, so hook::git_is_bin fails and hook::git_resolve_index returns 1.
In plugins/guardrails/hooks/block-dangerous-git.sh:678, hook::git_resolve_index "$@" || return 0 — a failed resolve means check_segment returns success (no block) unconditionally. The force-push is never inspected at all, not just mis-scoped to the wrong directory. The same hook::git_resolve_index call gates block-no-verify.sh, block-noncanonical-commit.sh, and block-convention-violation.sh too, so all four guardrails are bypassable this way.

This is the exact "fail-open" class this PR sets out to close (issue #1811), just not closed for -h. It's also not caught by the new tests — lib/hook-utils.test.sh only adds cases for -b and -n (lines 2166-2167).

Confidence: High — traced step-by-step against the actual code in this diff and the downstream consumer in block-dangerous-git.sh.

IMPORTANT — peel set omits other documented valueless sudo shorts, leaving the same full-bypass gap for those flags

File: lib/hook-utils.sh line 1456 (and synced copies)

sudo --help documents additional valueless short options not in the peel class [bnEhHkKsSv]: -A (askpass), -B (bell), -e (edit), -l (list), -P (preserve-groups), -V (version) — -i is the only gap the code's own comment calls out as accepted. For any of these placed before D in a cluster (e.g. sudo -AD /other/repo git push --force), stok stays -AD, matches none of the specific cases, falls to the generic -*) ((i++)) arm, and the outer resolver never recognizes git at all — hitting the same hook::git_resolve_index … || return 0 full-bypass path as the -h case above (not merely a missed chdir).

This narrows, but does not close, the gap the pre-PR comment already documented as a known limitation — the commit's stated goal (peeling "documented valueless sudo shorts") is met for 8 of the ~14 valueless shorts sudo actually defines.

Confidence: High that the flag-set gap exists as described (matches sudo's documented option grammar); same downstream bypass mechanism as the CRITICAL finding above.

No other security-relevant issues found in the files this PR currently changes. All 16 plugin copies of hook-utils.sh remain byte-identical to lib/hook-utils.sh (verified via cmp, not author claim); plugin.json/CHANGELOG.md changes are non-functional. scripts/verify-security-review-evidence.sh is not part of this PR's diff at the current HEAD, so it remains out of scope for this pass.
· branch cursor/sudo-cluster-chdir-peel-63e1

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>

claude · 2026-08-12T20:21:23Z

Claude finished @cursor[bot]'s task in 1m 54s —— View job

Security Review

Gather context (git diff origin/main...HEAD, changed files)
Check skip gate (prior automated run at this head failed to complete — infra failure per the bot comment, not a passed review; the last completed review pass on this branch found CRITICAL issues, so nothing here qualifies as a still-applying successful review — proceeding with a full pass)
Review lib/hook-utils.sh sudo-peeling logic at current HEAD (24cdda0)
Confirm synced plugin copies stayed in sync
Report findings

Reviewed the files changed at HEAD 24cdda0584258e6c03a5f6502439c92f9306004a: lib/hook-utils.sh, lib/hook-utils.test.sh, the 16 synced plugin copies of hook-utils.sh, and the plugins/*/.claude-plugin/plugin.json / CHANGELOG.md version bumps.

The sudo cluster-peeling logic (lib/hook-utils.sh:1444-1477) is unchanged from the prior review passes on this branch — the two findings below still apply and have been re-posted as inline comments at the current head:

CRITICAL — `-h` is misclassified as valueless, still allowing a full guardrail bypass via `sudo -hD …`

The peel regex at line 1456 strips h from a clustered token, but line 1471 in the same sudo) case treats standalone -h as value-taking — the same flag is asserted to have opposite grammars a few lines apart. sudo -hD git push --force origin main gets -D misread as chdir, git consumed as its argument, and the outer resolver then fails to recognize the command at all — so hook::git_resolve_index "$@" || return 0 causes block-dangerous-git.sh (and block-no-verify.sh, block-noncanonical-commit.sh, block-convention-violation.sh) to skip inspection entirely, not just target the wrong directory. See inline comment for the full trace and fix.

IMPORTANT — peel set omits other documented valueless sudo shorts (`-A`, `-B`, `-e`, `-l`, `-P`, `-V`), leaving the same full-bypass gap for those flags

sudo --help defines more valueless shorts than the [bnEhHkKsSv] class covers. A cluster like sudo -AD /other/repo git push --force hits the same fail-open path as the -h case. Separately, the doc comment (lines 1447-1448) lists 8 flags but the regex actually covers 10 (h/S extra) — comment/code drift independent of the fix above.

Both findings were traced against the code at this exact head, not just carried forward from memory — the relevant lines are byte-identical to the previously reviewed commits on this branch.

No other security-relevant issues found. All 16 plugin copies of hook-utils.sh remain byte-identical to lib/hook-utils.sh (same blob SHA across all copies in the diff); plugin.json/CHANGELOG.md changes are non-functional. scripts/verify-security-review-evidence.sh is not part of this PR's diff at the current head, so it remains out of scope.
· branch cursor/sudo-cluster-chdir-peel-63e1

…ckRollup running-check pitfall (#2513) ## Summary Lands the two PR-lifecycle facts #1436 recorded from driving #1393/#1316/#1318/#1322 to merge, in the document that was prescribing the expensive path: - **Stale-branch recovery now defaults to merge-forward.** `monitor.md`'s mergeable pre-check (§3.1 step 1) and stale-branch recovery rule (§3.2) both prescribed "force-push with lease" — but auto-mode permission classifiers commonly deny force-push, and the recorded consequence was a fresh branch + fresh PR + closing the old one per rebase (the #1315 → #1377 → #1393 churn, with every review thread re-opened on the successor). Merging the default branch *into* the PR branch resolves staleness and pushes **fast-forward** — no force-push — and under a squash-only default branch the merge commits collapse to one commit on merge, so linear-history requirements stay satisfied. Verified in the issue's own record: #1393 landed that way and #1318 was merge-forwarded five times without needing a new branch. Rebase stays available as the exception for projects requiring a linear PR branch where force-push is actually permitted. - **`statusCheckRollup` reports a running check as `conclusion: ""` (empty string), not `null`.** The complement-shaped filter (`conclusion != null and != "SUCCESS"`) therefore counts every in-progress check as a failure — the exact misreport in the issue (two "failing" checks that were simply still running). The multi-PR scan section (§3.0.6, the one place this skill reads `statusCheckRollup`) now documents the pitfall with value-positive jq selectors for "failed" and "still running". Version `0.53.11` → `0.53.14` (patch; `0.53.12`/`0.53.13` are claimed by in-flight PRs #2450/#2453/#2483/#2510 and #2469 — skipping past them per the #1746 collision pattern). ## Test plan - `npx markdownlint-cli2@0.23.2` on both edited markdown files — 0 issues. - Docs-only change to skill reference text; no scripts or hooks touched. The jq forms added are the ones from the issue, verified against `gh pr view --json statusCheckRollup` semantics. ## Related Fixes #1436 Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>

github-actions · 2026-08-12T20:23:39Z

Last security-reviewed head: e4bc4266249727027c6dea470e3766c0aaae3e25. On the next push, the relevance gate compares only the commits since this SHA; delete this comment to force a full re-review.

…ckRollup running-check pitfall (#2513) ## Summary Lands the two PR-lifecycle facts #1436 recorded from driving #1393/#1316/#1318/#1322 to merge, in the document that was prescribing the expensive path: - **Stale-branch recovery now defaults to merge-forward.** `monitor.md`'s mergeable pre-check (§3.1 step 1) and stale-branch recovery rule (§3.2) both prescribed "force-push with lease" — but auto-mode permission classifiers commonly deny force-push, and the recorded consequence was a fresh branch + fresh PR + closing the old one per rebase (the #1315 → #1377 → #1393 churn, with every review thread re-opened on the successor). Merging the default branch *into* the PR branch resolves staleness and pushes **fast-forward** — no force-push — and under a squash-only default branch the merge commits collapse to one commit on merge, so linear-history requirements stay satisfied. Verified in the issue's own record: #1393 landed that way and #1318 was merge-forwarded five times without needing a new branch. Rebase stays available as the exception for projects requiring a linear PR branch where force-push is actually permitted. - **`statusCheckRollup` reports a running check as `conclusion: ""` (empty string), not `null`.** The complement-shaped filter (`conclusion != null and != "SUCCESS"`) therefore counts every in-progress check as a failure — the exact misreport in the issue (two "failing" checks that were simply still running). The multi-PR scan section (§3.0.6, the one place this skill reads `statusCheckRollup`) now documents the pitfall with value-positive jq selectors for "failed" and "still running". Version `0.53.11` → `0.53.14` (patch; `0.53.12`/`0.53.13` are claimed by in-flight PRs #2450/#2453/#2483/#2510 and #2469 — skipping past them per the #1746 collision pattern). ## Test plan - `npx markdownlint-cli2@0.23.2` on both edited markdown files — 0 issues. - Docs-only change to skill reference text; no scripts or hooks touched. The jq forms added are the ones from the issue, verified against `gh pr view --json statusCheckRollup` semantics. ## Related Fixes #1436 Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>

claude · 2026-08-12T20:32:49Z

Claude finished @cursor[bot]'s task in 1m 59s —— View job

Security Review

Gather context (git diff since last-reviewed head 24cdda0, changed files at HEAD f5dcc5e)
Check skip gate (last review found CRITICAL/IMPORTANT issues; this push directly addresses them — full re-review warranted)
Review lib/hook-utils.sh sudo-peeling logic changes
Confirm synced plugin copies stayed in sync
Report findings

Reviewed the changes at HEAD f5dcc5eb03de3a0bc65dc452a9b65e4de19dcb64 relative to the last-reviewed commit (24cdda0): lib/hook-utils.sh, lib/hook-utils.test.sh, and the 16 synced plugin copies of hook-utils.sh, plus the plugins/*/.claude-plugin/plugin.json / CHANGELOG.md version bumps.

Both previously reported findings are fixed:

CRITICAL (-h misclassified as valueless) — line 1456: h was dropped from the peel character class (^-[ABbeEHKklnPsSvV](.+)$), so a clustered -hD no longer peels to -D. Traced sudo -hD git commit: stok stays -hD, doesn't match -D/--chdir/-D*, doesn't match the exact -h value-taking arm either (-hD ≠ -h), falls to the generic -*) ((i++)) arm — so -hD is consumed as one opaque option and the loop correctly advances to git, which the outer resolver then recognizes via hook::git_is_bin. No chdir is recorded and git is still resolved (no downstream guardrail bypass, unlike before). Regression-tested at lib/hook-utils.test.sh:2169.
IMPORTANT (peel set omitted other documented valueless shorts) — the class widened from [bnEhHkKsSv] to [ABbeEHKklnPsSvV]. Checked against sudo --help's full valueless-short grammar (-A -b -B -E -e -H -K -k -l -n -P -S -s -V -v, 15 flags, -i/-h excluded per the updated comment) — the new class covers all 15 documented valueless shorts. Regression-tested for -A at lib/hook-utils.test.sh:2168.
The comment/code mismatch flagged in an earlier pass is also resolved — the updated comment (lines 1445-1450) now lists exactly the 15 flags in the regex class and explicitly documents why -h is excluded.

No new security-relevant issues found in this diff. All 16 plugin copies of hook-utils.sh remain byte-identical to lib/hook-utils.sh (verified via md5sum); plugin.json/CHANGELOG.md changes are non-functional version bumps.
· branch cursor/sudo-cluster-chdir-peel-63e1

Peel sudo's clustered valueless short options in hook::git_resolve_index so sudo -bD <dir> git … records the chdir instead of losing it fail-open. Closes #1811 Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>

…den valueless shorts Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>

claude · 2026-08-12T20:57:25Z

Claude finished @cursor[bot]'s task in 1m 20s —— View job

Security Review

Gather context (git diff origin/main...HEAD, changed files at HEAD e4bc426)
Invoke /review:security-review and follow its instructions
Report findings

Skip gate hit (condition 3): this head already has a successful security review that still applies.

The last completed security review on this branch (posted at commit f5dcc5e) found both prior findings (-h misclassified as valueless, and the peel set omitting other documented valueless shorts) fixed, with no new issues. Diffing that reviewed state forward to the current HEAD (e4bc426):

git diff a746cfe..e4bc426 --stat
 plugins/actionlint/CHANGELOG.md           | 1 -
 plugins/claude-ops/CHANGELOG.md           | 1 -
 plugins/context-guard/CHANGELOG.md        | 1 -
 plugins/desktop-notification/CHANGELOG.md | 1 -
 plugins/eol-normalizer/CHANGELOG.md       | 1 -
 plugins/markdown-format/CHANGELOG.md      | 1 -
 plugins/ruff-format/CHANGELOG.md          | 1 -
 plugins/typos-format/CHANGELOG.md         | 1 -
 8 files changed, 8 deletions(-)

(a746cfe carries the same "drop -h from peel set, widen valueless shorts" content previously reviewed at f5dcc5e — hash differs from a rebase, content is identical, confirmed by re-reading lib/hook-utils.sh:1444-1477 at current HEAD, which matches what was confirmed fixed.)

The only commit since then (e4bc426, "fix: remove extra blank lines in changelogs (MD012)") touches only CHANGELOG.md files, removing stray blank lines for markdownlint MD012 compliance — no code, no security-relevant surface.

Per the skip-gate rule in /review:security-review, no further review is needed and no new findings are posted.
· branch cursor/sudo-cluster-chdir-peel-63e1

claude · 2026-08-12T21:13:27Z

Claude finished @cursor[bot]'s task in 2m 19s —— View job

Code review

Gather context (PR diff at HEAD e4bc426, CLAUDE.md, REVIEW.md)
Review lib/hook-utils.sh sudo peeling fix (current state)
Review lib/hook-utils.test.sh new tests
Confirm synced plugin copies stay in sync
Post findings

No findings. git diff origin/main...HEAD is empty — this branch (claude/pr-2483-20260812-2113) and the PR head (e4bc426) are tree-identical to origin/main (8b6c53c), i.e. the PR is already merged and there is no unreviewed diff left to evaluate.

I re-verified the current state of the sudo cluster-peel logic in lib/hook-utils.sh:1444-1477 against the CRITICAL/IMPORTANT findings raised in earlier review rounds on this branch:

-h misclassification (previously CRITICAL): h is no longer in the peel character class ([ABbeEHKklnPsSvV]), so sudo -hD dir … is no longer mistaken for a chdir — confirmed fixed, and regression-tested at lib/hook-utils.test.sh:2169 (sudo -hD DIR does not peel -h and does not treat git as chdir).
Incomplete valueless-short coverage (previously IMPORTANT): the class now covers all 15 of sudo's documented valueless shorts (-A -B -b -e -E -H -K -k -l -n -P -S -s -V -v), matching sudo --help's grammar. Regression-tested for -A at lib/hook-utils.test.sh:2168.
Comment/code drift (previously nit): the explaining comment (lines 1445-1451) now lists exactly the flags in the regex class and explicitly documents why -h is excluded.

Other checks:

All 16 synced plugin copies of hook-utils.sh are byte-identical to lib/hook-utils.sh (verified via md5sum, not author claim).
plugins/guardrails/.claude-plugin/plugin.json version (0.28.12) matches the top CHANGELOG.md entry, and the changelog has no stray double-blank-lines (the concern the final commit's title references).
Unable to execute bash lib/hook-utils.test.sh in this sandbox (Bash approval gate blocks it here, same limitation noted in earlier review passes on this branch) — verification above is via static tracing and reading the existing test assertions, not a live run.

scripts/verify-security-review-evidence.sh (flagged as IMPORTANT in an earlier round on this branch) is not part of the diff at the current head, so it's out of scope for this pass. Security-relevant aspects are left to the dedicated claude-security-review lane per this repo's REVIEW.md scope split; that lane already confirmed both prior CRITICAL/IMPORTANT sudo-peel findings fixed as of commit f5dcc5e.

claude Bot reviewed Aug 12, 2026

View reviewed changes

Comment thread lib/hook-utils.sh Outdated

claude Bot reviewed Aug 12, 2026

View reviewed changes

Comment thread lib/hook-utils.sh

claude Bot reviewed Aug 12, 2026

View reviewed changes

Comment thread scripts/verify-security-review-evidence.sh

chatgpt-codex-connector Bot reviewed Aug 12, 2026

View reviewed changes

Comment thread lib/hook-utils.sh Outdated

cursor Bot force-pushed the cursor/sudo-cluster-chdir-peel-63e1 branch from 0b2b853 to 1bb1a0d Compare August 12, 2026 16:23

kyle-sexton force-pushed the cursor/sudo-cluster-chdir-peel-63e1 branch from 1bb1a0d to 9680743 Compare August 12, 2026 16:53

kyle-sexton force-pushed the cursor/sudo-cluster-chdir-peel-63e1 branch from 9680743 to a9eb9fa Compare August 12, 2026 17:13

cursor Bot pushed a commit that referenced this pull request Aug 12, 2026

chore: bump plugin versions for changelog parity (#2483)

25f2e13

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>

cursor Bot force-pushed the cursor/sudo-cluster-chdir-peel-63e1 branch from 25f2e13 to 2fba1fa Compare August 12, 2026 18:05

kyle-sexton mentioned this pull request Aug 12, 2026

docs(source-control): merge-forward for stale PR branches + statusCheckRollup running-check pitfall #2513

Merged

claude Bot reviewed Aug 12, 2026

View reviewed changes

Comment thread lib/hook-utils.sh Outdated

claude Bot reviewed Aug 12, 2026

View reviewed changes

Comment thread lib/hook-utils.sh Outdated

cursor Bot force-pushed the cursor/sudo-cluster-chdir-peel-63e1 branch from 24cdda0 to f5dcc5e Compare August 12, 2026 20:32

cursoragent and others added 4 commits August 12, 2026 20:56

chore(go-format): bump version for hook-utils sync in sudo peel change

2b2021c

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>

fix(guardrails): address sudo peel review — drop -h from peel set, wi…

a746cfe

…den valueless shorts Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>

fix: remove extra blank lines in changelogs (MD012)

e4bc426

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>

cursor Bot force-pushed the cursor/sudo-cluster-chdir-peel-63e1 branch from bb964f0 to e4bc426 Compare August 12, 2026 20:56

kyle-sexton merged commit 8b6c53c into main Aug 12, 2026
38 checks passed

kyle-sexton deleted the cursor/sudo-cluster-chdir-peel-63e1 branch August 12, 2026 21:12

Uh oh!

Conversation

kyle-sexton commented Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test plan

Related

Uh oh!

cursor Bot commented Aug 12, 2026

Uh oh!

claude Bot commented Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code review

Other notes (not code-anchored)

Uh oh!

claude Bot commented Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Security Review

IMPORTANT — sudo cluster-peel set is missing several documented valueless short flags, leaving the chdir-detection bypass only partially closed

SUGGESTION — execution-evidence markers in verify-security-review-evidence.sh are weak signals, not proof the review actually ran

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

claude Bot commented Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Security Review

CRITICAL — -h is misclassified as a valueless short flag, still allowing a full guardrail bypass via sudo -hD …

IMPORTANT — peel set omits other documented valueless sudo shorts, leaving the same full-bypass gap for those flags

Uh oh!

claude Bot commented Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Security Review

CRITICAL — -h is misclassified as a valueless short flag, still allowing a full guardrail bypass via sudo -hD …

IMPORTANT — peel set omits other documented valueless sudo shorts, leaving the same full-bypass gap for those flags

Uh oh!

claude Bot commented Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Security Review

CRITICAL — -h is misclassified as valueless, still allowing a full guardrail bypass via sudo -hD …

IMPORTANT — peel set omits other documented valueless sudo shorts (-A, -B, -e, -l, -P, -V), leaving the same full-bypass gap for those flags

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude Bot commented Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Security Review

Uh oh!

claude Bot commented Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Security Review

Uh oh!

Uh oh!

claude Bot commented Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code review

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

kyle-sexton commented Aug 12, 2026 •

edited

Loading

claude Bot commented Aug 12, 2026 •

edited

Loading

claude Bot commented Aug 12, 2026 •

edited

Loading

SUGGESTION — execution-evidence markers in `verify-security-review-evidence.sh` are weak signals, not proof the review actually ran

github-actions Bot commented Aug 12, 2026 •

edited

Loading

claude Bot commented Aug 12, 2026 •

edited

Loading

CRITICAL — `-h` is misclassified as a valueless short flag, still allowing a full guardrail bypass via `sudo -hD …`

claude Bot commented Aug 12, 2026 •

edited

Loading

CRITICAL — `-h` is misclassified as a valueless short flag, still allowing a full guardrail bypass via `sudo -hD …`

claude Bot commented Aug 12, 2026 •

edited

Loading

CRITICAL — `-h` is misclassified as valueless, still allowing a full guardrail bypass via `sudo -hD …`

IMPORTANT — peel set omits other documented valueless sudo shorts (`-A`, `-B`, `-e`, `-l`, `-P`, `-V`), leaving the same full-bypass gap for those flags

github-actions Bot commented Aug 12, 2026 •

edited

Loading

claude Bot commented Aug 12, 2026 •

edited

Loading

claude Bot commented Aug 12, 2026 •

edited

Loading

claude Bot commented Aug 12, 2026 •

edited

Loading