Conversation
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/a14eaf5d-e5b2-4191-862b-20f3dbb3463d Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
… bot-menu checkbox pattern Adds an explicit opt-in to bypass the confused-deputy check for the legitimate pattern where a workflow posts a checkbox-menu comment as github-actions[bot] and a human maintainer edits it to tick a box (issue_comment:edited, actor != comment.user.login). Changes: - `aw_context.cjs`: `buildAwContext()` now includes `allow_bot_authored_trigger_comment: true` when the current event is issue_comment:edited with a bot-authored comment (actor differs from comment.user.login). Propagated to child workflows via aw_context. - `check_permissions_utils.cjs`: Adds `readAllowBotAuthoredTriggerComment(payload)` helper that reads the flag from inbound aw_context (workflow_call inputs or repository_dispatch client_payload). Extends `isConfusedDeputyAttack()` with optional 4th param `allowBotAuthoredTriggerComment`; when true and action is 'edited', skips the issue_comment confused-deputy check. The dependabot attack vector (created action) remains fully protected. - `check_membership.cjs` and `check_skip_bots.cjs`: Read the inbound aw_context flag and pass it to `isConfusedDeputyAttack()`. - Tests: Added coverage for the new flag in all four test files. Agent-Logs-Url: https://github.com/github/gh-aw/sessions/a14eaf5d-e5b2-4191-862b-20f3dbb3463d Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…ean expression - readAllowBotAuthoredTriggerComment: catch block now logs at debug level on parse error - aw_context.cjs buildAwContext: extract allow_bot_authored_trigger_comment boolean to named intermediate variables (isIssueCommentEdited, commentAuthor, commentAuthoredByOther) for improved readability Agent-Logs-Url: https://github.com/github/gh-aw/sessions/a14eaf5d-e5b2-4191-862b-20f3dbb3463d Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR aims to prevent false-positive “confused deputy” denials for the workflow pattern where a bot authors a checkbox-menu comment and a human maintainer later edits that comment (triggering issue_comment:edited where actor !== comment.user.login), by introducing an opt-in flag propagated via aw_context and honoring it in the confused-deputy check.
Changes:
- Add
allow_bot_authored_trigger_commenttoaw_contextgeneration and propagate it across workflow hops. - Introduce
readAllowBotAuthoredTriggerComment(...)and thread the flag throughcheck_membership/check_skip_botsintoisConfusedDeputyAttack(...). - Add/extend unit tests covering the new flag behavior and parsing.
- Bump pinned AWF container/image version in
stale-pr-cleanup.lock.yml.
Show a summary per file
| File | Description |
|---|---|
| actions/setup/js/check_skip_bots.cjs | Reads the new allow-flag and passes it into isConfusedDeputyAttack. |
| actions/setup/js/check_permissions_utils.test.cjs | Adds unit tests for the edited-comment opt-in bypass and aw_context flag parsing. |
| actions/setup/js/check_permissions_utils.cjs | Adds readAllowBotAuthoredTriggerComment and an opt-in bypass for issue_comment:edited in isConfusedDeputyAttack. |
| actions/setup/js/check_membership.test.cjs | Adds integration-style tests ensuring bot-menu edited comments can be allowed when opt-in is present. |
| actions/setup/js/check_membership.cjs | Reads the allow-flag and passes it into isConfusedDeputyAttack. |
| actions/setup/js/aw_context.test.cjs | Adds tests for new allow_bot_authored_trigger_comment field in built aw_context. |
| actions/setup/js/aw_context.cjs | Adds allow_bot_authored_trigger_comment field to aw_context output. |
| .github/workflows/stale-pr-cleanup.lock.yml | Bumps AWF/firewall image tag and schema URLs from 0.25.38 to 0.25.39. |
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: 4
| // The allowBotAuthoredTriggerComment flag is propagated via the inbound aw_context | ||
| // when a caller explicitly opts in to the bot-posted-menu / user-checks-box pattern | ||
| // (e.g. a workflow posts a checkbox comment as github-actions[bot] and a maintainer | ||
| // edits it to tick a box, firing issue_comment:edited). | ||
| const allowBotAuthoredTriggerComment = readAllowBotAuthoredTriggerComment(context.payload); | ||
| if (isConfusedDeputyAttack(actor, eventName, context.payload, allowBotAuthoredTriggerComment)) { |
| // | ||
| // The allowBotAuthoredTriggerComment flag is propagated via the inbound aw_context | ||
| // when a caller opts in to the bot-posted-menu / user-checks-box pattern. | ||
| const allowBotAuthoredTriggerComment = readAllowBotAuthoredTriggerComment(context.payload); | ||
| if (isConfusedDeputyAttack(actor, eventName, context.payload, allowBotAuthoredTriggerComment)) { |
| // allow_bot_authored_trigger_comment is set to true when the triggering event is | ||
| // issue_comment:edited and the comment author differs from the actor — the | ||
| // bot-posted-menu / user-checks-box pattern described in gh-aw issue #29480. | ||
| // Propagated to child workflows so their confused-deputy check can recognise | ||
| // this known-safe scenario and skip the actor-vs-comment-author mismatch guard. | ||
| allow_bot_authored_trigger_comment: (() => { | ||
| const isIssueCommentEdited = context.eventName === "issue_comment" && context.payload?.action === "edited"; | ||
| const commentAuthor = context.payload?.comment?.user?.login; | ||
| const commentAuthoredByOther = typeof commentAuthor === "string" && commentAuthor !== (context.actor ?? ""); | ||
| return isIssueCommentEdited && commentAuthoredByOther; |
| // the mismatch is intentional — a workflow posted a checkbox-menu comment (authored | ||
| // by github-actions[bot]) and a human maintainer edited it to tick a box. This | ||
| // pattern is safe because no permission is being bypassed: the human actor is who | ||
| // they appear to be and the bot's role is purely a UI affordance. | ||
| if (eventName === "issue_comment") { | ||
| const commentAuthor = payload.comment?.user?.login; | ||
| if (commentAuthor !== undefined && commentAuthor !== actor) { | ||
| if (allowBotAuthoredTriggerComment && payload.action === "edited") { |
|
@copilot review all comments |
…fusedDeputyAttack
Addresses review feedback:
- The previous aw_context-flag approach only worked for workflow_call/repository_dispatch
payloads; for direct issue_comment triggers (the actual false positive case) there is no
aw_context in the payload so the flag was always false.
- The exception is now derived directly from the native webhook payload:
action==="edited" && commentAuthor.endsWith("[bot]"). This works for all trigger paths.
- Both isConfusedDeputyAttack and buildAwContext now require [bot] suffix on the comment
author, preventing the exception from applying to human-vs-human comment edits.
- Removes readAllowBotAuthoredTriggerComment calls from check_membership and
check_skip_bots; isConfusedDeputyAttack no longer needs the external flag.
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/95b31b1a-526d-4138-8f19-3d2a81d5d386
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
All four review comments addressed in e3abd75:
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:
|
| allow_bot_authored_trigger_comment: (() => { | ||
| const isIssueCommentEdited = context.eventName === "issue_comment" && context.payload?.action === "edited"; | ||
| const commentAuthor = context.payload?.comment?.user?.login; | ||
| const commentAuthoredByBot = typeof commentAuthor === "string" && commentAuthor.endsWith("[bot]"); | ||
| const commentAuthoredByOther = commentAuthoredByBot && commentAuthor !== (context.actor ?? ""); | ||
| return isIssueCommentEdited && commentAuthoredByOther; |
There was a problem hiding this comment.
Done in c272004. The IIFE is now replaced with named local variables (isIssueCommentEdited, triggerCommentAuthor, triggerCommentByBot, allowBotAuthoredTriggerComment) computed before the return object.
…object; add on.allow-bot-authored-trigger-comment frontmatter option - aw_context.cjs: move IIFE to variables computed before the return object (per review comment) - Add on.allow-bot-authored-trigger-comment boolean frontmatter option that compiles to GH_AW_ALLOW_BOT_AUTHORED_TRIGGER_COMMENT env var in check_membership and check_skip_bots steps - When env var is set, isConfusedDeputyAttack allows issue_comment:edited bypass even for bots without the standard [bot] naming convention (broader than auto-detection from payload) - Update schema, WorkflowData, role_checks.go, compiler_orchestrator_workflow.go, compiler_pre_activation_job.go, and tests Agent-Logs-Url: https://github.com/github/gh-aw/sessions/b0fa2a65-65ec-4157-ac46-04d5924846d3 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in c272004. Added ---
on:
issue_comment:
types: [edited]
allow-bot-authored-trigger-comment: true
---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
isConfusedDeputyAttackcheck introduced in v0.71.4 produces a false positive for the pattern where a workflow posts a checkbox-menu comment asgithub-actions[bot]and a human maintainer edits it to tick a box (issue_comment:edited). Becauseactor ≠ comment.user.login, the check denies the run, blocking all downstream jobs.Changes
check_permissions_utils.cjsisConfusedDeputyAttack()now detects the bot-menu exception directly from the native webhook payload:action === "edited"and (commentAuthor.endsWith("[bot]")OR the frontmatter opt-in env var is set). Works for directissue_commenttriggers as well asworkflow_callrelays — no externalaw_contextflag needed.readAllowBotAuthoredTriggerComment(payload)is retained as a utility for reading the metadata flag from inboundaw_context, but is no longer part of the security check path.aw_context.cjsbuildAwContext()includesallow_bot_authored_trigger_comment: booleanas metadata — set totruewheneventName === "issue_comment",action === "edited",comment.user.loginends with[bot], andcomment.user.login ≠ actor. The value is now computed in named local variables before the return object rather than an IIFE.check_membership.cjs/check_skip_bots.cjsisConfusedDeputyAttack(actor, eventName, context.payload)without an external flag; the bot-menu exception is self-contained in the check.Frontmatter:
on: allow-bot-authored-trigger-comment: trueGH_AW_ALLOW_BOT_AUTHORED_TRIGGER_COMMENT=trueas an environment variable on both thecheck_membershipandcheck_skip_botssteps.[bot]naming convention — for anyissue_comment:editedmismatch the check is skipped when the flag is present.main_workflow_schema.json),WorkflowData,role_checks.go,compiler_orchestrator_workflow.go, andcompiler_pre_activation_job.goupdated accordingly.How the exception works
For
issue_commentevents, the confused-deputy check is skipped when all of the following are true:payload.action === "edited"— the Dependabot attack vector always uses"created"payload.comment.user.login.endsWith("[bot]")(auto-detection), orGH_AW_ALLOW_BOT_AUTHORED_TRIGGER_COMMENT=trueis set via theon: allow-bot-authored-trigger-comment: truefrontmatter option (covers non-standard bot naming)The
issue_comment:createdpath and any edited comment authored by a human (without the frontmatter opt-in) remain fully guarded.Usage
Workflows that use standard
[bot]-named GitHub App accounts do not need the frontmatter option — auto-detection handles them automatically.