Honor bot allowlists before confused-deputy checks

actions/setup/js/check_membership.cjs

@@ -203,12 +203,43 @@ async function main() {
     return;
   }
 
+  // Allow trusted bots other than Dependabot to synchronize same-repository PRs they
+  // did not open. Cross-repository PRs still require provenance validation because an
+  // attacker may induce an allowlisted bot to update code from their fork.
+  const isPullRequestSynchronization = (eventName === "pull_request" || eventName === "pull_request_target") && context.payload?.action === "synchronize";
+  const pullRequestHeadRepository = context.payload?.pull_request?.head?.repo;
+  const pullRequestBaseRepository = context.payload?.pull_request?.base?.repo;
+  const hasRepositoryIds = Number.isInteger(pullRequestHeadRepository?.id) && Number.isInteger(pullRequestBaseRepository?.id);
+  const isSameRepositoryPullRequest = hasRepositoryIds
+    ? pullRequestHeadRepository.id === pullRequestBaseRepository.id
+    : typeof pullRequestHeadRepository?.full_name === "string" && pullRequestHeadRepository.full_name.toLowerCase() === `${owner}/${repo}`.toLowerCase();
+  const pullRequestAuthor = context.payload?.pull_request?.user?.login;
+  const isAllowlistedBotSynchronizationMismatch = isPullRequestSynchronization && typeof pullRequestAuthor === "string" && pullRequestAuthor !== actorToValidate && isAllowedBot(actorToValidate, allowedBots);
+  const canAuthorizeBotBeforeConfusedDeputyCheck = isAllowlistedBotSynchronizationMismatch && isSameRepositoryPullRequest && actorToValidate !== "dependabot[bot]";
+  if (isAllowlistedBotSynchronizationMismatch) {
+    core.info(
+      `Evaluating allowlisted bot synchronization for actor '${actorToValidate}' on ${eventName}: ` + `PR author '${pullRequestAuthor}', same repository: ${isSameRepositoryPullRequest}, Dependabot: ${actorToValidate === "dependabot[bot]"}`
+    );
+  }
+  if (canAuthorizeBotBeforeConfusedDeputyCheck) {
+    const authorPermission = await checkRepositoryPermission(pullRequestAuthor, owner, repo, requiredPermissions);
+    if (authorPermission.authorized) {
+      core.info(`PR author '${pullRequestAuthor}' is trusted; checking whether bot '${actorToValidate}' is active`);
+      const botResult = await checkBotAllowlistAuthorization(actorToValidate, allowedBots, owner, repo);
+      if (botResult.handled) {
+        return;
+      }
+    } else {
+      core.info(`PR author '${pullRequestAuthor}' is not trusted; continuing with confused-deputy validation`);
+    }
+  }
+
   // Guard against Dependabot Confused Deputy attacks.
   // An attacker can trigger @dependabot recreate (for pull_request events) or
   // @dependabot show (for issue_comment events) to make dependabot appear as the
   // actor, bypassing permission checks that rely solely on github.actor.
   // Reference: https://labs.boostsecurity.io/articles/weaponizing-dependabot-pwn-request-at-its-finest/
-  if (isConfusedDeputyAttack(actorToValidate, eventName, context.payload)) {
+  if (isConfusedDeputyAttack(actorToValidate, eventName, context.payload) || isAllowlistedBotSynchronizationMismatch) {
     const errorMessage = `Access denied: Potential confused deputy attack detected. Actor '${actorToValidate}' does not match the event author. The workflow may have been triggered indirectly via a bot command.`;
     core.warning(errorMessage);
     core.setOutput("is_team_member", "false");
@@ -218,11 +249,7 @@ async function main() {
     return;
   }
 
-  // If the actor is in the bots allowlist, skip the roles check entirely and go straight
-  // to bot-status verification. A bot listed in on.bots: is an explicit grant; the roles
-  // mismatch (bots typically have "none" repo permission) is expected and not actionable.
-  // Checking bots first also avoids a spurious "permission does not meet requirements"
-  // warning that would otherwise be emitted by the roles check before authorization succeeds.
+  // For all other events, preserve confused-deputy validation before bot authorization.
   const botResult = await checkBotAllowlistAuthorization(actorToValidate, allowedBots, owner, repo);
   if (botResult.handled) {
     return;

actions/setup/js/check_membership.test.cjs

@@ -464,6 +464,143 @@ describe("check_membership.cjs", () => {
       expect(mockCore.setOutput).toHaveBeenCalledWith("result", "confused_deputy");
     });
 
+    it.each(["pull_request", "pull_request_target"])("should authorize an active allowlisted bot when actor differs from PR author (%s synchronize event)", async eventName => {
+      process.env.GH_AW_ALLOWED_BOTS = "my-fixup-bot[bot]";
+      mockContext.actor = "my-fixup-bot[bot]";
+      mockContext.eventName = eventName;
+      mockContext.payload = {
+        action: "synchronize",
+        pull_request: {
+          user: { login: "human-author" },
+          head: { repo: { id: 123, full_name: "testorg/testrepo" } },
+          base: { repo: { id: 123, full_name: "testorg/testrepo" } },
+        },
+      };
+      mockGithub.rest.repos.getCollaboratorPermissionLevel.mockResolvedValueOnce({ data: { permission: "write" } }).mockResolvedValueOnce({ data: { permission: "none" } });
+
+      await runScript();
+
+      expect(mockCore.info).toHaveBeenCalledWith(`Evaluating allowlisted bot synchronization for actor 'my-fixup-bot[bot]' on ${eventName}: PR author 'human-author', same repository: true, Dependabot: false`);
+      expect(mockCore.info).toHaveBeenCalledWith("PR author 'human-author' is trusted; checking whether bot 'my-fixup-bot[bot]' is active");
+      expect(mockCore.setOutput).toHaveBeenCalledWith("is_team_member", "true");
+      expect(mockCore.setOutput).toHaveBeenCalledWith("result", "authorized_bot");
+      expect(mockCore.setOutput).not.toHaveBeenCalledWith("result", "confused_deputy");
+    });
+
+    it.each(["pull_request", "pull_request_target"])("should deny an active allowlisted bot when synchronizing a cross-repository PR (%s event)", async eventName => {
+      process.env.GH_AW_ALLOWED_BOTS = "my-fixup-bot[bot]";
+      mockContext.actor = "my-fixup-bot";
+      mockContext.eventName = eventName;
+      mockContext.payload = {
+        action: "synchronize",
+        pull_request: {
+          user: { login: "attacker" },
+          head: { repo: { id: 456, full_name: "attacker/fork" } },
+          base: { repo: { id: 123, full_name: "testorg/testrepo" } },
+        },
+      };
+      await runScript();
+
+      expect(mockCore.info).toHaveBeenCalledWith(`Evaluating allowlisted bot synchronization for actor 'my-fixup-bot' on ${eventName}: PR author 'attacker', same repository: false, Dependabot: false`);
+      expect(mockCore.setOutput).toHaveBeenCalledWith("is_team_member", "false");
+      expect(mockCore.setOutput).toHaveBeenCalledWith("result", "confused_deputy");
+      expect(mockCore.setOutput).not.toHaveBeenCalledWith("result", "authorized_bot");
+    });
+
+    it("should deny an active allowlisted bot when the same-repository PR author lacks the required role", async () => {
+      process.env.GH_AW_ALLOWED_BOTS = "my-fixup-bot[bot]";
+      mockContext.actor = "my-fixup-bot[bot]";
+      mockContext.eventName = "pull_request_target";
+      mockContext.payload = {
+        action: "synchronize",
+        pull_request: {
+          user: { login: "untrusted-author" },
+          head: { repo: { id: 123, full_name: "testorg/testrepo" } },
+          base: { repo: { id: 123, full_name: "testorg/testrepo" } },
+        },
+      };
+      mockGithub.rest.repos.getCollaboratorPermissionLevel.mockResolvedValueOnce({ data: { permission: "read" } });
+
+      await runScript();
+
+      expect(mockCore.info).toHaveBeenCalledWith("PR author 'untrusted-author' is not trusted; continuing with confused-deputy validation");
+      expect(mockCore.setOutput).toHaveBeenCalledWith("is_team_member", "false");
+      expect(mockCore.setOutput).toHaveBeenCalledWith("result", "confused_deputy");
+      expect(mockCore.setOutput).not.toHaveBeenCalledWith("result", "authorized_bot");
+    });
+
+    it("should deny an active allowlisted bot when the PR head repository is unavailable", async () => {
+      process.env.GH_AW_ALLOWED_BOTS = "my-fixup-bot[bot]";
+      mockContext.actor = "my-fixup-bot[bot]";
+      mockContext.eventName = "pull_request_target";
+      mockContext.payload = {
+        action: "synchronize",
+        pull_request: {
+          user: { login: "attacker" },
+          head: { repo: null },
+        },
+      };
+
+      await runScript();
+
+      expect(mockCore.setOutput).toHaveBeenCalledWith("is_team_member", "false");
+      expect(mockCore.setOutput).toHaveBeenCalledWith("result", "confused_deputy");
+      expect(mockCore.setOutput).not.toHaveBeenCalledWith("result", "authorized_bot");
+    });
+
+    it("should compare same-repository PR names case-insensitively", async () => {
+      process.env.GH_AW_ALLOWED_BOTS = "my-fixup-bot[bot]";
+      mockContext.actor = "my-fixup-bot[bot]";
+      mockContext.eventName = "pull_request";
+      mockContext.payload = {
+        action: "synchronize",
+        pull_request: {
+          user: { login: "human-author" },
+          head: { repo: { full_name: "TestOrg/TestRepo" } },
+        },
+      };
+      mockGithub.rest.repos.getCollaboratorPermissionLevel.mockResolvedValueOnce({ data: { permission: "write" } }).mockResolvedValueOnce({ data: { permission: "none" } });
+
+      await runScript();
+
+      expect(mockCore.setOutput).toHaveBeenCalledWith("is_team_member", "true");
+      expect(mockCore.setOutput).toHaveBeenCalledWith("result", "authorized_bot");
+    });
+
+    it.each(["pull_request", "pull_request_target"])("should deny allowlisted dependabot when actor differs from PR author (%s synchronize event)", async eventName => {
+      process.env.GH_AW_ALLOWED_BOTS = "dependabot[bot]";
+      mockContext.actor = "dependabot[bot]";
+      mockContext.eventName = eventName;
+      mockContext.payload = { action: "synchronize", pull_request: { user: { login: "attacker" } } };
+
+      await runScript();
+
+      expect(mockCore.setOutput).toHaveBeenCalledWith("is_team_member", "false");
+      expect(mockCore.setOutput).toHaveBeenCalledWith("result", "confused_deputy");
+      expect(mockCore.setOutput).not.toHaveBeenCalledWith("result", "authorized_bot");
+    });
+
+    it("should deny an inactive allowlisted bot when actor differs from PR author", async () => {
+      process.env.GH_AW_ALLOWED_BOTS = "my-fixup-bot[bot]";
+      mockContext.actor = "my-fixup-bot[bot]";
+      mockContext.eventName = "pull_request_target";
+      mockContext.payload = {
+        action: "synchronize",
+        pull_request: {
+          user: { login: "human-author" },
+          head: { repo: { id: 123, full_name: "testorg/testrepo" } },
+          base: { repo: { id: 123, full_name: "testorg/testrepo" } },
+        },
+      };
+      mockGithub.rest.repos.getCollaboratorPermissionLevel.mockResolvedValueOnce({ data: { permission: "write" } }).mockRejectedValue({ status: 404, message: "Not Found" });
+
+      await runScript();
+
+      expect(mockCore.setOutput).toHaveBeenCalledWith("is_team_member", "false");
+      expect(mockCore.setOutput).toHaveBeenCalledWith("result", "bot_not_active");
+      expect(mockCore.setOutput).not.toHaveBeenCalledWith("result", "authorized_bot");
+    });
+
     it("should allow access when actor matches PR author (genuine dependabot PR synchronize)", async () => {
       mockContext.actor = "dependabot[bot]";
       mockContext.eventName = "pull_request";
@@ -480,6 +617,7 @@ describe("check_membership.cjs", () => {
     });
 
     it("should deny access when actor differs from comment author (issue_comment event)", async () => {
+      process.env.GH_AW_ALLOWED_BOTS = "dependabot[bot]";
       mockContext.actor = "dependabot[bot]";
       mockContext.eventName = "issue_comment";
       mockContext.payload = { comment: { user: { login: "attacker" } } };

specs/compiler-threat-detection-compliance/README.md

@@ -33,8 +33,9 @@ Baseline rule implementation and test-file locations are maintained in [spec §7
 | CTR-023 | T-CTR-023 |
 | CTR-025 | T-CTR-039 |
 | CTR-026 | T-CTR-041 |
+| CTR-027 | T-CTR-042 |
 
-Note: `CTR-025` maps to `T-CTR-039` and `CTR-026` maps to `T-CTR-041` because `T-CTR-024` through `T-CTR-038` and `T-CTR-040` were already allocated to Section 6 false-positive and optimizer protocol norms. The shared `T-CTR-*` sequence is intentionally non-sequential with respect to `CTR-*` rule IDs.
+Note: `CTR-025` maps to `T-CTR-039`, `CTR-026` maps to `T-CTR-041`, and `CTR-027` maps to `T-CTR-042` because `T-CTR-024` through `T-CTR-038` and `T-CTR-040` were already allocated to Section 6 false-positive and optimizer protocol norms. The shared `T-CTR-*` sequence is intentionally non-sequential with respect to `CTR-*` rule IDs.
 
 The test triggers, expected compiler actions, and stable diagnostics are defined in [Section 8.1](../compiler-threat-detection-spec.md#81-test-id-catalog). The implementation and concrete test-file mappings are defined in [Section 7.1](../compiler-threat-detection-spec.md#71-baseline-rule-mapping).
 

specs/compiler-threat-detection-spec.md

@@ -7,7 +7,7 @@ sidebar:
 
 # GitHub Actions Compiler Threat Detection Specification
 
-**Version**: 1.0.33
+**Version**: 1.0.34
 **Status**: Candidate Recommendation  
 **Latest Version**: https://github.com/github/gh-aw/blob/main/specs/compiler-threat-detection-spec.md  
 **Editors**: GitHub Next (GitHub, Inc.)
@@ -32,6 +32,7 @@ Each version maps to the minimum compatible binary. A version change MUST update
 
 | Versions | Minimum gh-aw | Compatibility |
 |---|---:|---|
+| `1.0.34` | `v0.87.9` | Adds CTR-027; allowlisted bot synchronization requires trusted same-repository provenance. |
 | `1.0.33` | `v0.87.9` | Audit-only; no new CTR rule or lock-file schema change. |
 | `1.0.32` | `v0.87.9` | Audit-only; no new CTR rule or lock-file schema change. |
 | `1.0.31` | `v0.87.9` | Audit-only; no new CTR rule or lock-file schema change. |
@@ -83,6 +84,7 @@ Each rule has a stable `CTR-*` ID, threat class, trigger, compiler action, diagn
 - **CTR-023 Bash Command Allowlist Illusion**: Reject explicit bash restrictions for engines that cannot enforce them.
 - **CTR-025 Framework Self-Prompt Misattribution**: Strip only a leading framework `<system>` block before analysis.
 - **CTR-026 Generated Job Timeout Expression Injection**: Reject non-positive or expression job timeout values.
+- **CTR-027 Allowlisted Bot Synchronization Provenance**: Deny bot-driven PR synchronization when the actor differs from the PR author unless the bot is explicitly allowlisted and active, the PR is from the base repository, the PR author satisfies the configured roles, and the actor is not Dependabot.
 
 ### 5.2 Compiler Response Requirements
 
@@ -150,13 +152,16 @@ Every active rule MUST map to implementation and test coverage. References are p
 | CTR-023 Bash Command Allowlist Illusion | `agent_validation.go`, `agentic_engine.go`, `pkg/gitutil/gitutil.go` | workflow-run, bash-allowlist, gitutil, and download tests |
 | CTR-025 Framework Self-Prompt Misattribution | `actions/setup/js/setup_threat_detection.cjs` | `setup_threat_detection.test.cjs` |
 | CTR-026 Generated Job Timeout Expression Injection | custom-job properties and timeout resolution | custom-job and timeout tests |
+| CTR-027 Allowlisted Bot Synchronization Provenance | `actions/setup/js/check_membership.cjs`, `actions/setup/js/check_permissions_utils.cjs` | `actions/setup/js/check_membership.test.cjs`, `actions/setup/js/check_permissions_utils.test.cjs` |
 
 ### 7.3 Mapping Audit (2026-09-11)
 
 Issue #59894 (`[sighthound] Security findings in github/gh-aw`) reported a Critical CWE-78 command-injection finding claiming untrusted `commentBody`/`params` reach `exec` in `actions/setup/js/close_issue.cjs` (and a `pkg/workflow/js/close_issue.cjs` mirror) around lines 5 and 1110. Verification against conformance scope found this to be a false positive: `close_issue.cjs` is 402 lines total (no line 1110 exists), contains no `exec`/`child_process`/`spawn` call anywhere, and `pkg/workflow/js/close_issue.cjs` does not exist in the repository. `commentBody` in `close_entity_helpers.cjs` flows only into the authenticated GitHub API client (`callbacks.addComment`), never into a shell or subprocess argument, so CTR-006 (Template Injection) and CTR-013 (Argument Injection via Package/Image Names) triggers do not apply and no new `CTR-*` rule is warranted. No `threat-detection-suppress` annotation was added because the finding does not correspond to any real code path the compiler needs to suppress — it is an inapplicable external scan result about nonexistent code, not an active false-positive-prone compiler rule.
 
 Open high/critical code-scanning alerts (#675–#677 `go/allocation-size-overflow`) remain unchanged from the 2026-09-10 audit disposition: in-process, schema-validated capacity-hint computations, not exploitable by untrusted input. No live `threat-detection-suppress` annotation exists in any workflow frontmatter. No compiler/parser source diff exists beyond the single squashed commit state, so no candidate threat surfaced from source changes.
 
+CTR-027 records the runtime pre-activation control for allowlisted bots that synchronize PRs they did not open. The allowlist alone MUST NOT override confused-deputy protection. Authorization is permitted only for `pull_request` or `pull_request_target` `synchronize` events when repository IDs match (or the head repository name matches the base repository if IDs are unavailable), the original PR author satisfies `on.roles`, the bot is installed and active, and the actor is not `dependabot[bot]`. Missing provenance, fork PRs, untrusted authors, inactive bots, and Dependabot author mismatches fail closed. The implementation logs the decision inputs and trust outcome without logging credentials or event payload contents.
+
 ### 7.2 Mapping Audit (2026-09-10)
 
 CTR-001–026 have implementation and test references with no `TODO` placeholders. The available repository history is a single squashed commit (`099efdd`, dated 2026-09-09 17:19 -0700); no additional compiler/parser diff exists beyond that state, so no new candidate threat surfaced from source changes. No live `threat-detection-suppress` annotation exists in any workflow frontmatter (only illustrative documentation examples in `.github/aw/syntax-agentic.md` and reference docs), so no `SLA_BREACH` applies.
@@ -198,6 +203,7 @@ Each active rule MUST have at least one deterministic test that covers its prima
 | **T-CTR-023** | CTR-023 Bash Command Allowlist Illusion | Reject explicit bash restrictions for engines that cannot enforce them | Reject explicit bash restrictions for engines that cannot enforce them. | `CTR-023` |
 | **T-CTR-039** | CTR-025 Framework Self-Prompt Misattribution | Strip only a leading framework `<system>` block before analysis | Strip only a leading framework `<system>` block before analysis. | `CTR-025` |
 | **T-CTR-041** | CTR-026 Generated Job Timeout Expression Injection | Reject non-positive or expression job timeout values | Reject non-positive or expression job timeout values. | `CTR-026` |
+| **T-CTR-042** | CTR-027 Allowlisted Bot Synchronization Provenance | An allowlisted bot synchronizes a PR authored by another actor | Authorize only when the bot, repository provenance, and PR author satisfy all trust requirements; otherwise deny with `confused_deputy` or `bot_not_active`. | `CTR-027` |
 
 The core tests exercise their catalog trigger and assert the expected rejection, warning, rewrite, or runtime-safe output.
 
@@ -223,6 +229,7 @@ A test ID that is deprecated under Section 5.4 MUST remain listed in Section 8.1
 
 | Version | Change |
 |---|---|
+| 1.0.34 | Added CTR-027 for trusted same-repository allowlisted bot synchronization and fail-closed confused-deputy handling. |
 | 1.0.33 | Audit-only review; issue #59894's `close_issue.cjs` command-injection claim is a false positive (no `exec`/subprocess call exists in the file). |
 | 1.0.32 | Audit-only review; #675–677 and #667–669/#674 are not new threat classes. |
 | 1.0.31 | Audit-only review; #672 is not a new threat class. |