fix(github): allowlist remote detection in triage-prs - #85
Conversation
The skill's pre-execution remote detection failed its permission check on every invocation. The checker splits compound commands on `||` and validates each part independently, and the skill's `allowed-tools` had no `Bash(git remote ...)` entry at all, so both `git remote get-url` parts came back as requiring approval. The trailing `echo` was already safe by default. Allowlist the split parts rather than restructuring the command. The chained form is deterministic — the shell picks `upstream` over `origin` and emits a single URL or the `__NO_REMOTE__` sentinel, including when run outside a git repository — so Phase 1 parses one value instead of choosing between remotes itself. `get-url` is read-only, so a wildcard on the remote name covers both parts without widening beyond reads. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195QVgZcRJjTBKKDJQjoEr3
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe GitHub plugin version changed from ChangesGitHub plugin update
Estimated code review effort: 1 (Trivial) | ~3 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Fixes /github:triage-prs remote detection by granting the required read-only Git command permission.
Changes:
- Allowlisted
git remote get-urlcommands. - Bumped the GitHub plugin patch version to
1.3.1.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
github/skills/triage-prs/SKILL.md |
Adds the remote URL lookup permission. |
github/.claude-plugin/plugin.json |
Applies the patch version bump. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Problem
/github:triage-prsfailed on every invocation, before doing any work:The skill's Repo Context section detects the target repo with a pre-execution
!command. The permission checker splits compound commands on||andvalidates each part independently, and the skill's
allowed-toolshad noBash(git remote ...)entry at all — so bothgit remote get-urlparts cameback as requiring approval. The trailing
echowas already safe by default,which is why the error names only the two git parts.
Fix
Allowlist the split parts rather than restructuring the command:
The command in the skill body is unchanged. Keeping the chained form keeps
detection deterministic — the shell picks
upstreamoveroriginand emits asingle URL or the
__NO_REMOTE__sentinel, including when run outside a gitrepository — so Phase 1 parses one value instead of choosing between remotes
itself.
A wildcard on the remote name covers both parts in one entry without widening
beyond reads:
get-urlonly accepts a remote name plus--push/--alland hasno mutating form (that's
set-url/add/remove), and operator injection iscaught by the same command splitting that caused this bug. It also matches the
existing convention in
review-pr, which usesBash(git diff *).Two alternatives were tried and dropped: restructuring to a single
git remote -vmoved theupstream/originpreference out of the shell andinto the model's reading of a multi-line table, and leaked
fatal: not a git repositoryinto context where the sentinel had handled it;listing
upstreamandoriginas separate exact entries needed three lines todo what one does.
Version bumped
1.3.0→1.3.1(patch — bug fix, no behavior change to theskill's logic).
Validation
claude plugin validate .— passedclaude plugin validate ./github— passednpx markdownlint-cli2 --config .markdownlint-cli2.jsonc "github/**/*.md"— exit 0uvx skillsaw --strict— 0 errors, 0 warnings, grade ANote
pr-review-toolkit/skills/review-pr/SKILL.md:30has the same latent failure —!`git remote get-url origin 2>/dev/null || echo __NO_ORIGIN_REMOTE__`with noBash(git remote ...)entry in itsallowed-tools. Left out of this PR sinceit's a separate plugin needing its own version bump; the same one-line entry
would fix it.
🤖 Generated with Claude Code
https://claude.ai/code/session_0195QVgZcRJjTBKKDJQjoEr3
Generated by Claude Code
Summary by CodeRabbit