fix: allow gh issue/pr commands so bot workflows don't get silently denied - #306
Merged
Conversation
…enied The Stage 1-4 bot workflows (triage, analyze, fix, review) run with --permission-mode bypassPermissions, but every triage run since 2026-07-11 has shown permission_denials_count > 0 and applied zero labels/comments despite the job reporting success. This lines up with claude-code-action@v1 picking up a Claude Code CLI bump (2.1.206 -> 2.1.207) around that date, after which bypassPermissions stopped force-allowing gh commands that weren't already on the project's explicit Bash allow list. Add gh issue/pr subcommands actually used by the workflows to the allow list directly, so the bots don't depend on bypass-mode behavior at all. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
johanzander
added a commit
that referenced
this pull request
Aug 13, 2026
The guard blocked a live beta release. Three commands in a row prompted, all issued from the changelog worktree and all operating inside it: cd <that worktree>; .venv/bin/pytest -m "not slow" -q cd <that worktree>; python3 - <<'EOF' ... rewrite config.yaml ... EOF cd <that worktree>; git add -A; git commit -m "merge: ..." Root cause: the token scan separated on quotes, parens, commas and `=` but not on shell separators, so `cd /path/to/wt; cmd` produced the token "/path/to/wt;" -- semicolon attached. That matches neither "$session_root" nor "$session_root"/*, so a worktree was judged foreign to ITSELF and every `cd <own worktree>; ...` prompted. Separators are now separators. Two further changes, both from the same root problem -- the check was trying to parse shell with regex, and each fix spawned the next false positive. Four distinct shapes reached the user, every one introduced by a FIX to this check rather than by the original code: cmd 2>/dev/null with any $var (fixed earlier) cd into a SIBLING worktree read-only grep whose literal pattern held `$(` and `;;` inspecting files under $HOME So the trigger is narrowed from "any absolute path, any .., any ~" to "can this command WRITE": the write verbs, a repo-redirect flag, or a redirect that creates a file (2>/dev/null and 2>&1 stripped first). A command that cannot write cannot commit the accident being guarded. Interpreters are included wholesale, since `python3 -c "shutil.rmtree(...)"` was the motivating miss for the old verb list. And a sibling worktree now counts as contained. A linked worktree is disposable by exactly the argument that justifies auto-allowing commands inside this one, and agents legitimately reach across them -- cutting a release from a changelog worktree is what triggered this. The checkout the guard exists for is the MAIN one, which is shared, long-lived, and the only place a stale path does lasting damage; it stays excluded explicitly, even though .claude/worktrees/* sits inside it. Known and accepted: an arbitrary-write shape not on the verb list is now auto-allowed inside a worktree. That is the price of the false-positive rate, and the header comment says so rather than implying coverage the check does not have. Verification: all four false-positive shapes and the three release commands are pinned, alongside six main-checkout attacks that must still ask (rm -rf $MAIN/core, sed -i into $MAIN, shutil.rmtree($MAIN), redirect into $MAIN, $HOME/GitHub/bess-manager, ../../../bess-manager) -- all still ask. The fixture gains a second linked worktree so sibling-vs-main is tested rather than asserted. Suite 118 -> 133 cases, all pass; quality-check.sh 0 errors, 0 warnings. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W29UVDwQPwVRRqBn2sMgHa Also in this commit -- the five PR #561 review findings, all verified against the merged hook before fixing: - A missing hook test file was a WARNING, so quality-check.sh still exited 0 with the permission matrix unguarded. #562 converted every other unavailable tool in that script to an ERROR on exactly this principle; now an ERROR. - `gh pr review` was absent from the safe list while settings.json explicitly allows `Bash(gh pr review *)` (added in #306 because the Stage-4 review bot was being denied). A hook decision supersedes a settings rule, so this silently revoked a granted permission. - Only the FIRST `git push` in a compound was scanned, so `git push origin feat/x && git push origin main` was allowed. Every push is now examined. - `mutates_shared_state` is a blocklist, so ref-moving shapes it did not name were allowed: `git branch -f main HEAD`, `git update-ref`, `git symbolic-ref`. Added, and the header no longer claims every check fails closed -- it names this function as the weakest link instead. - The shared-ref regex missed `beta-release-*` branches, which are shared release refs. Refspecs are now compared as TOKENS (src:dst split, `+` and refs/heads/ stripped), so `beta-release-1.2` matches while `feat/main-ish` still does not. - Corrected a comment claiming settings.json has a blanket `Bash(gh *)` entry; it has eight narrow patterns, and the safe list must stay a superset of them.
johanzander
added a commit
that referenced
this pull request
Aug 13, 2026
) The guard blocked a live beta release. Three commands in a row prompted, all issued from the changelog worktree and all operating inside it: cd <that worktree>; .venv/bin/pytest -m "not slow" -q cd <that worktree>; python3 - <<'EOF' ... rewrite config.yaml ... EOF cd <that worktree>; git add -A; git commit -m "merge: ..." Root cause: the token scan separated on quotes, parens, commas and `=` but not on shell separators, so `cd /path/to/wt; cmd` produced the token "/path/to/wt;" -- semicolon attached. That matches neither "$session_root" nor "$session_root"/*, so a worktree was judged foreign to ITSELF and every `cd <own worktree>; ...` prompted. Separators are now separators. Two further changes, both from the same root problem -- the check was trying to parse shell with regex, and each fix spawned the next false positive. Four distinct shapes reached the user, every one introduced by a FIX to this check rather than by the original code: cmd 2>/dev/null with any $var (fixed earlier) cd into a SIBLING worktree read-only grep whose literal pattern held `$(` and `;;` inspecting files under $HOME So the trigger is narrowed from "any absolute path, any .., any ~" to "can this command WRITE": the write verbs, a repo-redirect flag, or a redirect that creates a file (2>/dev/null and 2>&1 stripped first). A command that cannot write cannot commit the accident being guarded. Interpreters are included wholesale, since `python3 -c "shutil.rmtree(...)"` was the motivating miss for the old verb list. And a sibling worktree now counts as contained. A linked worktree is disposable by exactly the argument that justifies auto-allowing commands inside this one, and agents legitimately reach across them -- cutting a release from a changelog worktree is what triggered this. The checkout the guard exists for is the MAIN one, which is shared, long-lived, and the only place a stale path does lasting damage; it stays excluded explicitly, even though .claude/worktrees/* sits inside it. Known and accepted: an arbitrary-write shape not on the verb list is now auto-allowed inside a worktree. That is the price of the false-positive rate, and the header comment says so rather than implying coverage the check does not have. Verification: all four false-positive shapes and the three release commands are pinned, alongside six main-checkout attacks that must still ask (rm -rf $MAIN/core, sed -i into $MAIN, shutil.rmtree($MAIN), redirect into $MAIN, $HOME/GitHub/bess-manager, ../../../bess-manager) -- all still ask. The fixture gains a second linked worktree so sibling-vs-main is tested rather than asserted. Suite 118 -> 133 cases, all pass; quality-check.sh 0 errors, 0 warnings. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W29UVDwQPwVRRqBn2sMgHa Also in this commit -- the five PR #561 review findings, all verified against the merged hook before fixing: - A missing hook test file was a WARNING, so quality-check.sh still exited 0 with the permission matrix unguarded. #562 converted every other unavailable tool in that script to an ERROR on exactly this principle; now an ERROR. - `gh pr review` was absent from the safe list while settings.json explicitly allows `Bash(gh pr review *)` (added in #306 because the Stage-4 review bot was being denied). A hook decision supersedes a settings rule, so this silently revoked a granted permission. - Only the FIRST `git push` in a compound was scanned, so `git push origin feat/x && git push origin main` was allowed. Every push is now examined. - `mutates_shared_state` is a blocklist, so ref-moving shapes it did not name were allowed: `git branch -f main HEAD`, `git update-ref`, `git symbolic-ref`. Added, and the header no longer claims every check fails closed -- it names this function as the weakest link instead. - The shared-ref regex missed `beta-release-*` branches, which are shared release refs. Refspecs are now compared as TOKENS (src:dst split, `+` and refs/heads/ stripped), so `beta-release-1.2` matches while `feat/main-ish` still does not. - Corrected a comment claiming settings.json has a blanket `Bash(gh *)` entry; it has eight narrow patterns, and the safe list must stay a superset of them.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
successbut applied zero labels/comments (permission_denials_countwent from 0 to 2-3 that day, with no repo-side change to explain it).anthropics/claude-code-action@v1(floating tag) picking up a Claude Code CLI bump from 2.1.206 to 2.1.207 in that window —--permission-mode bypassPermissionsappears to no longer force-allow Bash commands (likegh) that aren't already on the project's explicit allow list.gh issue/gh prsubcommands actually used across the bot pipeline (triage/analyze/fix/review) to.claude/settings.json's allow list, so those bots don't depend on bypass-mode behavior at all.Test plan
gh workflow run issue-triage.yml -f issue_number=304) and confirm labels + comment actually land this time