Conversation
Add protection against the Dependabot Confused Deputy Injection technique documented at https://labs.boostsecurity.io/articles/weaponizing-dependabot-pwn-request-at-its-finest/ An attacker can trigger @dependabot recreate (for pull_request events) or @dependabot show (for issue_comment events) to make dependabot appear as github.actor on an arbitrary PR, bypassing permission checks that rely solely on github.actor. Changes: - Add isConfusedDeputyAttack() to check_permissions_utils.cjs: detects when actor != PR author (pull_request events) or comment author (issue_comment events) - Guard check_membership.cjs: deny access with 'confused_deputy' result when attack is detected before any permission or bot-allowlist checks - Guard check_skip_bots.cjs: skip the bot-skip rule when confused deputy is detected, preventing attackers from suppressing workflow runs - Fix trigger_parser.go 'dependabot pull request' condition to also require github.event.pull_request.user.login == 'dependabot[bot]' - Add comprehensive tests for all changed components Agent-Logs-Url: https://github.com/github/gh-aw/sessions/ea6c8a30-d88d-4c83-a604-7678ebce227b Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot make sure this works with workflow_call and agent context |
Add tests documenting that isConfusedDeputyAttack correctly returns false
for workflow_call events, preventing false positives in reusable workflow
scenarios where context.payload = { inputs: { aw_context: "..." } } and
has no pull_request.user or comment.user fields.
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/31d9390c-d678-4471-93aa-2edc4f23bfc7
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
For Added explicit tests documenting this behaviour in commit 94dd4d3:
For the "agent context" (the agent job that runs after activation): the confused deputy check only runs at activation time in |
There was a problem hiding this comment.
Pull request overview
This PR hardens workflow actor/bot filtering against the “Dependabot Confused Deputy” injection technique, preventing attackers from spoofing github.actor via Dependabot commands to bypass permission checks or suppress runs.
Changes:
- Add
isConfusedDeputyAttack(actor, eventName, payload)helper and integrate it into membership and skip-bots checks. - Tighten the
"dependabot pull request"trigger shorthand to require both actor and PR author to be Dependabot. - Add/adjust unit tests for the new confused-deputy protection paths.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/trigger_parser.go | Strengthens Dependabot PR shorthand condition to also validate PR author. |
| pkg/workflow/trigger_parser_test.go | Updates expected shorthand condition string for Dependabot PR trigger. |
| actions/setup/js/check_permissions_utils.cjs | Introduces isConfusedDeputyAttack helper and exports it. |
| actions/setup/js/check_permissions_utils.test.cjs | Adds unit tests for isConfusedDeputyAttack. |
| actions/setup/js/check_membership.cjs | Denies early when a confused-deputy pattern is detected. |
| actions/setup/js/check_membership.test.cjs | Adds tests covering confused-deputy denial/allowance scenarios. |
| actions/setup/js/check_skip_bots.cjs | Prevents skip-bots suppression when a confused-deputy pattern is detected. |
| actions/setup/js/check_skip_bots.test.cjs | Adds tests ensuring confused-deputy prevents skipping and avoids false positives for workflow_call. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 8/8 changed files
- Comments generated: 2
| // For pull_request, pull_request_review, and pull_request_review_comment events, | ||
| // the PR author must match the actor. @dependabot recreate triggers a synchronize | ||
| // event with actor=dependabot[bot] but pull_request.user = original human author. | ||
| const prEvents = ["pull_request", "pull_request_review", "pull_request_review_comment"]; | ||
| if (prEvents.includes(eventName)) { | ||
| const prAuthor = payload.pull_request?.user?.login; | ||
| if (prAuthor !== undefined && prAuthor !== actor) { | ||
| return true; |
There was a problem hiding this comment.
isConfusedDeputyAttack treats pull_request_review and pull_request_review_comment as if the PR author must match github.actor, but for these events the actor is typically the reviewer/commenter and will not match payload.pull_request.user.login in normal operation. This causes false positives that will deny check_membership and bypass check_skip_bots for essentially all review/review-comment triggers. Use the event-specific author fields instead (e.g., payload.review.user.login for pull_request_review and payload.comment.user.login for pull_request_review_comment), and only fall back to payload.pull_request.user.login for pull_request events.
| it("should return true for pull_request_review when actor differs from PR author", () => { | ||
| const payload = { pull_request: { user: { login: "attacker" } } }; | ||
| expect(isConfusedDeputyAttack("dependabot[bot]", "pull_request_review", payload)).toBe(true); | ||
| }); | ||
|
|
||
| it("should return true for pull_request_review_comment when actor differs from PR author", () => { | ||
| const payload = { pull_request: { user: { login: "attacker" } } }; |
There was a problem hiding this comment.
These new tests assert that isConfusedDeputyAttack() should return true for pull_request_review / pull_request_review_comment when github.actor differs from the PR author. For these event types, the relevant author to compare against is the review author (payload.review.user.login) or the review comment author (payload.comment.user.login), not payload.pull_request.user.login. As written, the tests codify the same false-positive behavior that will break review/review-comment triggered workflows.
| it("should return true for pull_request_review when actor differs from PR author", () => { | |
| const payload = { pull_request: { user: { login: "attacker" } } }; | |
| expect(isConfusedDeputyAttack("dependabot[bot]", "pull_request_review", payload)).toBe(true); | |
| }); | |
| it("should return true for pull_request_review_comment when actor differs from PR author", () => { | |
| const payload = { pull_request: { user: { login: "attacker" } } }; | |
| it("should return false for pull_request_review when actor matches review author even if PR author differs", () => { | |
| const payload = { | |
| pull_request: { user: { login: "attacker" } }, | |
| review: { user: { login: "dependabot[bot]" } }, | |
| }; | |
| expect(isConfusedDeputyAttack("dependabot[bot]", "pull_request_review", payload)).toBe(false); | |
| }); | |
| it("should return true for pull_request_review when actor differs from review author even if PR author matches", () => { | |
| const payload = { | |
| pull_request: { user: { login: "dependabot[bot]" } }, | |
| review: { user: { login: "attacker" } }, | |
| }; | |
| expect(isConfusedDeputyAttack("dependabot[bot]", "pull_request_review", payload)).toBe(true); | |
| }); | |
| it("should return false for pull_request_review_comment when actor matches comment author even if PR author differs", () => { | |
| const payload = { | |
| pull_request: { user: { login: "attacker" } }, | |
| comment: { user: { login: "dependabot[bot]" } }, | |
| }; | |
| expect(isConfusedDeputyAttack("dependabot[bot]", "pull_request_review_comment", payload)).toBe(false); | |
| }); | |
| it("should return true for pull_request_review_comment when actor differs from comment author even if PR author matches", () => { | |
| const payload = { | |
| pull_request: { user: { login: "dependabot[bot]" } }, | |
| comment: { user: { login: "attacker" } }, | |
| }; |
There was a problem hiding this comment.
Great suggestion! Using event-specific author fields (payload.review.user.login for pull_request_review) instead of always comparing against the PR author is the correct approach to avoid false positives on normal review workflows.
📰 BREAKING: Report filed by Smoke Copilot · ● 1.3M
|
@copilot review all comments |
For pull_request_review, compare actor against payload.review.user.login (the reviewer) rather than payload.pull_request.user.login (the PR author). For pull_request_review_comment, compare actor against payload.comment.user.login (the comment author), not the PR author. Using pull_request.user.login for all three event types caused false positives on every review/review-comment workflow run, since reviewers are almost never the PR author. Update tests to cover the correct event-specific author comparisons, including explicit false-positive prevention cases. Agent-Logs-Url: https://github.com/github/gh-aw/sessions/f5e1c9c7-f623-40ba-8b83-d460dec714be Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Addressed the code reviewer's finding in commit 7df1e43:
Updated tests explicitly cover: (1) actor = reviewer, PR author differs → |
|
@copilot do a deep review, update ADR |
…uty injection Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/8584a602-dd33-4665-9942-c3c48abcef7e Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Deep review complete — no issues found beyond what was already fixed in commit 7df1e43. ADR added in commit 108cfce: The ADR covers:
|
|
@copilot review this scenario where the workflow is triggered by a label. Be more precise for this check. https://github.com/github/gh-aw/actions/runs/25202808475/job/73897140423#step:4:1 |
…only Agent-Logs-Url: https://github.com/github/gh-aw/sessions/db69096a-e3b8-445e-b24a-4a1f5aea3d19 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Fixed in commit 4479d66. The log showed The attack specifically exploits the Tests updated to include Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
|
|
|
|
📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing... |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
🚀 Smoke OpenCode MISSION COMPLETE! OpenCode delivered. 🔥 |
Agent Container Tool Check
Result: 11/12 tools available Overall Status: FAIL —
|
pelikhan
deleted the
copilot/guard-bot-filtering-against-dependabot-attacks
branch
|
Smoke Test Codex 25203411968: FAIL Warning Firewall blocked 2 domainsThe following domains were blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "ab.chatgpt.com"
- "chatgpt.com"See Network Configuration for more information.
|
|
🤖 Smoke test agent was here! The tests are running strong and all systems are green. 🎉 Greetings from the automation depths!
|
Smoke Test: Copilot - 25203411945PR: security: guard bot filtering against dependabot confused deputy attacks
Overall: PASS ✅
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Smoke test review: The security changes look solid. The narrowing of confused deputy checks to only 'synchronize' action events is a good fix. The new isConfusedDeputyAttack helper is well-structured.
📰 BREAKING: Report filed by Smoke Copilot · ● 1.3M
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
Smoke Test 25203411948 — PARTIAL PASS Core (#1–12): ✅✅✅✅✅✅✅✅✅❌✅✅ (11/12 — AW MCP status tool unavailable) PR used: #29431 (open) — triggering PR #29432 merged mid-run.
|
Summary
Guards bot filtering against the Dependabot Confused Deputy Injection technique documented in https://labs.boostsecurity.io/articles/weaponizing-dependabot-pwn-request-at-its-finest/
Attack Vector
An attacker can trigger
@dependabot recreate(forpull_requestevents) or@dependabot show(forissue_commentevents) to makedependabot[bot]appear asgithub.actoron an arbitrary PR. Because dependabot typically haswriteaccess to the repository, this bypasses the permission check incheck_membership.cjsand allows the attacker's unauthorized PR to activate the agentic workflow.Changes
check_permissions_utils.cjs: AddisConfusedDeputyAttack(actor, eventName, payload)helper that returnstruewhen the current actor does not match the event-specific author:pull_requestwith actionsynchronize: actor must matchpayload.pull_request.user.login(PR author). The check is scoped to thesynchronizeaction only — the documented attack vector (@dependabot recreate). Otherpull_requestactions (labeled,unlabeled,assigned,review_requested, etc.) legitimately have the actor differ from the PR author and are excluded.pull_request_review: actor must matchpayload.review.user.login(reviewer)pull_request_review_comment: actor must matchpayload.comment.user.login(comment author)issue_comment: actor must matchpayload.comment.user.login(comment author)check_membership.cjs: Before any permission or bot-allowlist check, callisConfusedDeputyAttack. If detected, deny with resultconfused_deputyand an actionable error message.check_skip_bots.cjs: Before applying skip-bots rules, callisConfusedDeputyAttack. If detected, do not skip the workflow — this prevents an attacker from suppressing the workflow run by making a skip-bot appear as the actor.trigger_parser.go: The compiled condition for the"dependabot pull request"trigger shorthand now requires bothgithub.actor == 'dependabot[bot]'andgithub.event.pull_request.user.login == 'dependabot[bot]', ensuring the PR was actually authored by Dependabot and not merely synchronized by it.docs/adr/29450-guard-bot-filter-against-dependabot-confused-deputy-injection.md: ADR documenting the decision, attack vectors, rejected alternatives, and RFC 2119 normative specification.Tests
check_permissions_utils.test.cjs: Tests forisConfusedDeputyAttackcovering PRsynchronizeevents (attack detected), PRlabeled/unlabeled/review_requestedevents (no false positive),pull_request_review(actor = reviewer, not PR author),pull_request_review_comment(actor = comment author, not PR author),issue_commentevents,workflow_callevents (no false positives), and edge cases.check_membership.test.cjs: Tests covering confused deputy denial (pull_request synchronize, issue_comment), no false positive for pull_request:labeled, genuine dependabot PR allowance, safe event bypass, non-PR events, andworkflow_callwithaw_context(no false positive).check_skip_bots.test.cjs: Tests covering confused deputy skip prevention (synchronize), genuine bot skip, pull_request:labeled skip-bots applied normally, issue_comment, no-payload cases, andworkflow_callcompatibility.trigger_parser_test.go: Updated expected condition for"dependabot pull request"trigger.✨ PR Review Safe Output Test - Run 25203411948