From the review of #194.
pr-shepherd.yml grants Bash(gh pr comment:*) unconditionally:
--allowedTools "Bash(gh pr list:*),Bash(gh pr view:*),Bash(gh pr diff:*),Bash(gh pr checks:*),Bash(git log:*),Bash(git diff:*),Bash(gh pr comment:*),Read,Grep,Glob"
dry_run defaults to "true", and the only thing stopping a comment in that mode is a prompt instruction:
If DRY_RUN is "true", post NOTHING. Report your assessment as your final message only.
So the default, safest mode is guaranteed by persuasion rather than by capability. A model that misreads the input, or a prompt-injection payload in a PR diff it is reading, could post anyway. The workflow reads untrusted PR content by design, which is exactly the setting where instruction-level limits are weakest.
Why this is worth fixing rather than shrugging at
The PR argues the point itself, and applies it correctly elsewhere. gh api is deliberately excluded from the allowlist with this reasoning:
It is a general HTTP client for the API, so allowlisting it would permit anything the token can do — including submitting an approving review.
That is capability-based reasoning: make the bad action impossible, not merely forbidden. The prompt calls its limits "hard limits, not preferences". dry_run is the one place where that principle is stated but not implemented — and it is the default mode.
Suggested fix
Make the grant conditional, so in dry-run the tool is simply absent:
--allowedTools "Bash(gh pr list:*),Bash(gh pr view:*),Bash(gh pr diff:*),Bash(gh pr checks:*),Bash(git log:*),Bash(git diff:*),Read,Grep,Glob${{ inputs.dry_run == 'false' && ',Bash(gh pr comment:*)' || '' }}"Keep the prompt instruction as well — belt and braces — but the allowlist becomes the thing that actually holds.
Worth confirming the rendered string is well-formed for both input values before merging, since claude_args is a literal block scalar passed straight through as CLI arguments.
Note
The same reasoning applies to any future capability added to this agent: if the prompt says it must not do X, prefer not granting X over asking it not to.
From the review of #194.
pr-shepherd.ymlgrantsBash(gh pr comment:*)unconditionally:dry_rundefaults to"true", and the only thing stopping a comment in that mode is a prompt instruction:So the default, safest mode is guaranteed by persuasion rather than by capability. A model that misreads the input, or a prompt-injection payload in a PR diff it is reading, could post anyway. The workflow reads untrusted PR content by design, which is exactly the setting where instruction-level limits are weakest.
Why this is worth fixing rather than shrugging at
The PR argues the point itself, and applies it correctly elsewhere.
gh apiis deliberately excluded from the allowlist with this reasoning:That is capability-based reasoning: make the bad action impossible, not merely forbidden. The prompt calls its limits "hard limits, not preferences".
dry_runis the one place where that principle is stated but not implemented — and it is the default mode.Suggested fix
Make the grant conditional, so in dry-run the tool is simply absent:
--allowedTools "Bash(gh pr list:*),Bash(gh pr view:*),Bash(gh pr diff:*),Bash(gh pr checks:*),Bash(git log:*),Bash(git diff:*),Read,Grep,Glob${{ inputs.dry_run == 'false' && ',Bash(gh pr comment:*)' || '' }}"Keep the prompt instruction as well — belt and braces — but the allowlist becomes the thing that actually holds.
Worth confirming the rendered string is well-formed for both input values before merging, since
claude_argsis a literal block scalar passed straight through as CLI arguments.Note
The same reasoning applies to any future capability added to this agent: if the prompt says it must not do X, prefer not granting X over asking it not to.