Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fix-github-app-review-pr-checkout.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 6 additions & 5 deletions actions/setup/js/checkout_pr_branch.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -291,8 +291,8 @@ async function assertTrustedCheckoutRuntime(awContext, pullRequest) {

// Apps are not collaborators: their permission endpoint returns "none" even
// when they can write to the repository. A bot that triggered a PR event for
// a branch in this repository can have its checkout trusted instead. Comments,
// reviews and dispatches do not prove the sender can write the PR branch.
// a branch in this repository can have its checkout trusted instead. Other
// events require the collaborator check unless explicitly allow-listed below.
if (
(context.eventName === "pull_request" || context.eventName === "pull_request_target") &&
(context.payload.action === "opened" || context.payload.action === "synchronize") &&
Expand All @@ -311,19 +311,20 @@ async function assertTrustedCheckoutRuntime(awContext, pullRequest) {

const runtimeRepositoryId = context.payload.repository?.id;
const senderLogin = context.payload.sender?.login;
const isReview = context.eventName === "pull_request_review";
if (
(context.eventName === "issue_comment" || context.eventName === "pull_request_review_comment") &&
(context.eventName === "issue_comment" || context.eventName === "pull_request_review_comment" || isReview) &&

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added the patch changeset .changeset/fix-github-app-review-pr-checkout.md in commit 2744171.

senderType === "Bot" &&
typeof senderLogin === "string" &&
context.payload.comment?.user?.login === senderLogin &&
(isReview ? context.payload.review?.user?.login : context.payload.comment?.user?.login) === senderLogin &&
canonicalizeBotIdentifier(senderLogin) === canonicalizeBotIdentifier(actor) &&
isAllowedBot(actor, parseAllowedBots()) &&
Number.isSafeInteger(runtimeRepositoryId) &&
runtimeRepositoryId > 0
) {
const details = await fetchPRDetails(pullRequest.number);
if (details.pullRequest.head?.repo?.id === runtimeRepositoryId && details.pullRequest.base?.repo?.id === runtimeRepositoryId) {
core.info(`Runtime safety check passed for same-repository allowed bot comment actor '${actor}'`);
core.info(`Runtime safety check passed for same-repository allowed bot ${isReview ? "review" : "comment"} actor '${actor}'`);
return details;
}
}
Expand Down
32 changes: 25 additions & 7 deletions actions/setup/js/checkout_pr_branch.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,7 @@ If the pull request is still open, verify that:
expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("Internal Server Error"));
});

it.each(["workflow_dispatch", "issue_comment", "pull_request_review_comment"])("should reject bot actor without write-or-higher permission for %s events", async eventName => {
it.each(["workflow_dispatch", "issue_comment", "pull_request_review_comment", "pull_request_review"])("should reject bot actor without write-or-higher permission for %s events", async eventName => {
mockContext.eventName = eventName;
mockContext.actor = "third-party-bot[bot]";
mockContext.payload.sender = { login: "third-party-bot[bot]", type: "Bot" };
Expand Down Expand Up @@ -462,13 +462,13 @@ If the pull request is still open, verify that:
expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("requires write or higher"));
});

it.each(["issue_comment", "pull_request_review_comment"].flatMap(eventName => ["my-app", "my-app[bot]"].flatMap(actor => ["my-app", "my-app[bot]"].map(login => [eventName, actor, login]))))(
"should allow an allow-listed App comment on a same-repository PR for %s with actor %s and login %s",
it.each(["issue_comment", "pull_request_review_comment", "pull_request_review"].flatMap(eventName => ["my-app", "my-app[bot]"].flatMap(actor => ["my-app", "my-app[bot]"].map(login => [eventName, actor, login]))))(
"should allow an allow-listed App comment or review on a same-repository PR for %s with actor %s and login %s",
async (eventName, actor, login) => {
mockContext.eventName = eventName;
mockContext.actor = actor;
mockContext.payload.sender = { login, type: "Bot" };
mockContext.payload.comment = { user: { login } };
mockContext.payload[eventName === "pull_request_review" ? "review" : "comment"] = { user: { login } };
mockContext.payload.repository.id = 42;
mockContext.payload.pull_request = { number: 123, state: "open" };
if (eventName === "issue_comment") {
Expand Down Expand Up @@ -648,13 +648,13 @@ If the pull request is still open, verify that:
},
];

it.each(["issue_comment", "pull_request_review_comment"].flatMap(eventName => commentCheckoutConformanceCases.map(testCase => ({ eventName, ...testCase }))))(
"conforms to the comment checkout policy for $eventName: $name",
it.each(["issue_comment", "pull_request_review_comment", "pull_request_review"].flatMap(eventName => commentCheckoutConformanceCases.map(testCase => ({ eventName, ...testCase }))))(
"conforms to the comment and review checkout policy for $eventName: $name",
async testCase => {
mockContext.eventName = testCase.eventName;
mockContext.actor = testCase.actor;
mockContext.payload.sender = { login: testCase.sender, type: testCase.senderType };
mockContext.payload.comment = { user: { login: testCase.commentAuthor } };
mockContext.payload[testCase.eventName === "pull_request_review" ? "review" : "comment"] = { user: { login: testCase.commentAuthor } };
mockContext.payload.repository.id = 42;
mockContext.payload.pull_request = { number: 123, state: "open" };
if (testCase.eventName === "issue_comment") {
Expand Down Expand Up @@ -689,6 +689,24 @@ If the pull request is still open, verify that:
}
);

it("does not trust a review using a matching comment author instead of the review author", async () => {
mockContext.eventName = "pull_request_review";
mockContext.actor = "my-app[bot]";
mockContext.payload.sender = { login: "my-app[bot]", type: "Bot" };
mockContext.payload.review = { user: { login: "other[bot]" } };
mockContext.payload.comment = { user: { login: "my-app[bot]" } };
mockContext.payload.repository.id = 42;
mockContext.payload.pull_request = { number: 123, state: "open" };
process.env.GH_AW_ALLOWED_BOTS = "my-app";
mockGithub.rest.repos.getCollaboratorPermissionLevel.mockResolvedValue({ data: { permission: "none" } });

await runScript();

expect(mockGithub.rest.repos.getCollaboratorPermissionLevel).toHaveBeenCalled();
expect(mockCore.setOutput).toHaveBeenCalledWith("checkout_pr_success", "false");
expect(mockExec.exec).not.toHaveBeenCalledWith("git", expect.arrayContaining(["fetch"]));
});

it("should validate centralized workflow_dispatch using the originating aw_context actor", async () => {
mockContext.eventName = "workflow_dispatch";
mockContext.actor = "github-actions[bot]";
Expand Down
6 changes: 4 additions & 2 deletions actions/setup/js/checkout_pr_branch_z3.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ describe("RS-05a checkout validator Z3 proof", () => {
"checkout_requires_write_or_same_repo_bot_pr",
"unlisted_same_repo_bot_comment_requires_write_permission",
"allowlisted_same_repo_bot_comment_can_checkout",
"review_with_mismatched_author_requires_write_permission",
"allowlisted_same_repo_bot_review_can_checkout",
"centralized_dispatch_requires_platform_bot_identity",
"centralized_dispatch_requires_command_or_label_marker",
"centralized_dispatch_requires_originating_actor",
Expand All @@ -40,8 +42,8 @@ describe("RS-05a checkout validator Z3 proof", () => {
"workflow_dispatch_uses_refs_pull_checkout",
"non_dispatch_pr_trigger_allows_forked_runtime_after_trust",
]);
expect(expected.filter(result => result.expected === "unsat")).toHaveLength(10);
expect(expected.filter(result => result.expected === "sat")).toHaveLength(2);
expect(expected.filter(result => result.expected === "unsat")).toHaveLength(11);
expect(expected.filter(result => result.expected === "sat")).toHaveLength(3);
});

it.runIf(z3Available())("proves every modeled unsafe checkout state is unreachable", () => {
Expand Down
2 changes: 1 addition & 1 deletion docs/src/content/docs/reference/triggers.md
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,7 @@ The `[bot]` suffix is optional — `github-actions` matches `github-actions[bot]

Allowlisted bots must be installed on the repository, which is verified through the repository collaborator API. The built-in Copilot coding agent identities (`Copilot` and `copilot-swe-agent`, including `copilot-swe-agent[bot]`) are an exception: when explicitly allowlisted, they do not require a collaborator lookup. Other GitHub App identities are not collaborators, so that lookup finds nothing for them. On `repository_dispatch` the check is therefore relaxed: the lookup is still performed, and an allowlisted App is authorized when it conclusively reports that the actor is not a collaborator, because sending the event already requires `contents: write` access to the repository. A lookup that fails (for example during an API outage or when rate limited) is still denied. This is how an App-triggered `repository_dispatch` is admitted — `on.roles:` is matched against repository collaborator roles and never matches an App sender.

For PR branch checkout after an `issue_comment` or `pull_request_review_comment`, an allowlisted Bot sender may bypass the collaborator check only when it authored the comment and the PR head and base are both in the workflow repository. Fork PRs and other actors still require write-or-higher collaborator permission.
For PR branch checkout after an `issue_comment`, `pull_request_review_comment`, or `pull_request_review`, an allowlisted Bot sender may bypass the collaborator check only when it authored the comment or review and the PR head and base are both in the workflow repository. Fork PRs and other actors still require write-or-higher collaborator permission.

### Filtering by Author Associations (`on.skip-author-associations`)

Expand Down
20 changes: 20 additions & 0 deletions specs/compiler-threat-detection-changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ This changelog records the version history and the dated mapping audits for `spe

| Version | Change |
|---|---|
| 1.0.48 | Audit-only clarification that same-repository checkout for allow-listed App comments and reviews is a runtime trust control governed by Security Architecture Specification RS-05a, not a new compiler threat rule. |
| 1.0.47 | Audit-only clarification that same-repository checkout for allow-listed App comments is a runtime trust control governed by Security Architecture Specification RS-05a, not a new compiler threat rule. |
| 1.0.46 | Extended CTR-017 OTLP exclusions to evals AWF sandboxes and mapped evals-enabled/disabled regression coverage. |
| 1.0.45 | Extended CTR-017 with OTLP collector endpoint/header exclusions for agent and inline/external threat-detection AWF sandboxes when `--exclude-env` is supported; retained host-side exporter access and mapped OTLP-enabled/disabled regression coverage. |
Expand Down Expand Up @@ -43,6 +44,25 @@ This changelog records the version history and the dated mapping audits for `spe

## Mapping Audits

### Mapping Audit (2026-10-12)

Reviewed the allow-listed GitHub App checkout exception for `pull_request_review`
in `actions/setup/js/checkout_pr_branch.cjs`, together with its regression tests,
RS-05a requirements and validation mapping, and the comment/review checkout TLA+
model.

Findings and disposition:

- The review exception requires a Bot sender whose login exactly matches the
review author and canonically matches the runtime actor, an explicit `on.bots`
allow-list entry, and a positive runtime repository ID equal to the API-verified
PR head and base repository IDs.
- Fork PRs, unlisted bots, identity mismatches, missing repository provenance,
and other actors retain the collaborator-permission floor.
- The compiler emits no generated-workflow construct for this runtime trust
decision. No `CTR-*` rule is warranted; RS-05a and its formal model specify
the authorization boundary.

### Mapping Audit (2026-10-09)

Reviewed the allow-listed GitHub App checkout exception for `issue_comment` and `pull_request_review_comment` in `actions/setup/js/checkout_pr_branch.cjs`, together with its regression tests and RS-05a validation evidence.
Expand Down
5 changes: 3 additions & 2 deletions specs/compiler-threat-detection-spec.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ sidebar:

# GitHub Actions Compiler Threat Detection Specification

**Version**: 1.0.47
**Version**: 1.0.48
**Status**: Candidate Recommendation
**Latest Version**: https://github.com/github/gh-aw/blob/main/specs/compiler-threat-detection-spec.md
**Editors**: GitHub Next (GitHub, Inc.)
Expand All @@ -32,6 +32,7 @@ Each version maps to the minimum compatible binary. A version change MUST update

| Versions | Minimum gh-aw | Compatibility |
|---|---:|---|
| `1.0.48` | Unreleased | Audit-only; same-repository checkout for allow-listed App comments and reviews is a runtime trust control governed by Security Architecture Specification RS-05a, not a new compiler threat rule. |
| `1.0.47` | Unreleased | Audit-only; same-repository checkout for allow-listed App comments is a runtime trust control governed by Security Architecture Specification RS-05a, not a new compiler threat rule. |
| `1.0.46` | Unreleased | Extends CTR-017 OTLP sandbox exclusions to evals AWF invocations. No lock-file schema change. |
| `1.0.45` | Unreleased | Extends CTR-017 with host-only OTLP exporter configuration: regenerated agent and threat-detection AWF commands exclude collector endpoints and headers when AWF supports `--exclude-env`. No lock-file schema change. |
Expand Down Expand Up @@ -63,7 +64,7 @@ Generated workflows run with elevated permissions and consume untrusted content

Runtime trust boundaries inside the agent workspace are out of scope for this specification. In particular, the safe-outputs repository checkout discovery path (`actions/setup/js/find_repo_checkout.cjs`) resolves an `owner/repo` target from agent-writable workspace state; its normative controls — manifest precedence, workspace confinement, per-invocation `safe.directory` scoping, read-only discovery, remote host constraint, and deferred durable trust — are specified as Threat T7 and requirements RCR1–RCR7 in the Safe Outputs MCP Gateway Specification and as §3.5 of the Checkout Behavior Specification. A conforming compiler is not required to detect this class, because the compiler emits no generated-workflow construct that selects the checkout directory.

PR-branch checkout authorization in `actions/setup/js/checkout_pr_branch.cjs` is also a runtime trust control, not a compiler-detectable threat. The same-repository bot exception covers `pull_request` and `pull_request_target` `opened` or `synchronize` events with matching bot sender and actor plus equal runtime, PR head, and PR base repository IDs. For `issue_comment` and `pull_request_review_comment`, an explicitly `on.bots`-allow-listed App may bypass the collaborator check only when `sender.type` is `Bot`, the sender and comment author logins match exactly, the sender is canonically equivalent to the runtime actor (App slug or `[bot]` form), the runtime repository ID is positive, and the fetched PR head and base repository IDs both match it. Forks, unlisted bots, unverifiable or mismatched identities, and other actors retain the collaborator-permission check. The normative requirements are in the Security Architecture Specification, RS-05a; this control does not add a `CTR-*` rule.
PR-branch checkout authorization in `actions/setup/js/checkout_pr_branch.cjs` is also a runtime trust control, not a compiler-detectable threat. The same-repository bot exception covers `pull_request` and `pull_request_target` `opened` or `synchronize` events with matching bot sender and actor plus equal runtime, PR head, and PR base repository IDs. For `issue_comment`, `pull_request_review_comment`, and `pull_request_review`, an explicitly `on.bots`-allow-listed App may bypass the collaborator check only when `sender.type` is `Bot`, the sender and matching comment or review author logins match exactly, the sender is canonically equivalent to the runtime actor (App slug or `[bot]` form), the runtime repository ID is positive, and the fetched PR head and base repository IDs both match it. Forks, unlisted bots, unverifiable or mismatched identities, and other actors retain the collaborator-permission check. The normative requirements are in the Security Architecture Specification, RS-05a; this control does not add a `CTR-*` rule.

Sandbox bypass includes the provenance of the agent's own configuration. On pull-request triggers the workspace holds head-branch content that the PR author controls, so agent configuration folders and root instruction files are attacker-controlled inputs until the generated job replaces them with the base-branch snapshot captured before that content was checked out.

Expand Down
Loading
Loading