Skip to content

Commit b8fb045

Browse files
authored
feat(safe-outputs): add allowed-events filter to submit-pull-request-review (#25484)
1 parent 613551f commit b8fb045

File tree

9 files changed

+327
-5
lines changed

9 files changed

+327
-5
lines changed

actions/setup/js/submit_pr_review.cjs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@ async function main(config = {}) {
3333
const { defaultTargetRepo, allowedRepos } = resolveTargetRepoConfig(config);
3434
const githubClient = await createAuthenticatedGitHubClient(config);
3535

36+
// Build the allowed events set from config (empty set means all events are allowed)
37+
const allowedEvents = new Set(Array.isArray(config.allowed_events) && config.allowed_events.length > 0 ? config.allowed_events.map(e => String(e).toUpperCase()) : []);
38+
3639
if (!buffer) {
3740
core.warning("submit_pull_request_review: No PR review buffer provided in config");
3841
return async function handleSubmitPRReview() {
@@ -45,6 +48,9 @@ async function main(config = {}) {
4548
if (allowedRepos.size > 0) {
4649
core.info(`Allowed repos: ${Array.from(allowedRepos).join(", ")}`);
4750
}
51+
if (allowedEvents.size > 0) {
52+
core.info(`Allowed review events: ${Array.from(allowedEvents).join(", ")}`);
53+
}
4854

4955
// Propagate per-handler staged flag to the shared PR review buffer
5056
if (config.staged === true) {
@@ -81,6 +87,16 @@ async function main(config = {}) {
8187
};
8288
}
8389

90+
// Enforce allowed-events filter (infrastructure-level enforcement)
91+
if (allowedEvents.size > 0 && !allowedEvents.has(event)) {
92+
const allowedList = Array.from(allowedEvents).join(", ");
93+
core.warning(`Review event '${event}' is not allowed. Allowed events: ${allowedList}`);
94+
return {
95+
success: false,
96+
error: `Review event '${event}' is not allowed by safe-outputs configuration. Allowed events: ${allowedList}`,
97+
};
98+
}
99+
84100
// Body is required for REQUEST_CHANGES per GitHub API docs;
85101
// optional for APPROVE and COMMENT
86102
const body = message.body || "";

actions/setup/js/submit_pr_review.test.cjs

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,83 @@ describe("submit_pr_review (Handler Factory Architecture)", () => {
173173
expect(result.error).toContain("Invalid review event");
174174
});
175175

176+
it("should allow all events when allowed_events not configured", async () => {
177+
const { main } = require("./submit_pr_review.cjs");
178+
179+
for (const event of ["APPROVE", "COMMENT", "REQUEST_CHANGES"]) {
180+
const localBuffer = createReviewBuffer();
181+
const h = await main({ max: 5, _prReviewBuffer: localBuffer });
182+
const result = await h({ event, body: event === "REQUEST_CHANGES" ? "body" : "" }, {});
183+
expect(result.success).toBe(true);
184+
expect(result.event).toBe(event);
185+
}
186+
});
187+
188+
it("should block APPROVE when allowed_events is [COMMENT, REQUEST_CHANGES]", async () => {
189+
const { main } = require("./submit_pr_review.cjs");
190+
const localBuffer = createReviewBuffer();
191+
const localHandler = await main({
192+
max: 5,
193+
_prReviewBuffer: localBuffer,
194+
allowed_events: ["COMMENT", "REQUEST_CHANGES"],
195+
});
196+
197+
const result = await localHandler({ event: "APPROVE", body: "" }, {});
198+
199+
expect(result.success).toBe(false);
200+
expect(result.error).toContain("not allowed");
201+
expect(result.error).toContain("APPROVE");
202+
});
203+
204+
it("should allow COMMENT when allowed_events is [COMMENT, REQUEST_CHANGES]", async () => {
205+
const { main } = require("./submit_pr_review.cjs");
206+
const localBuffer = createReviewBuffer();
207+
const localHandler = await main({
208+
max: 5,
209+
_prReviewBuffer: localBuffer,
210+
allowed_events: ["COMMENT", "REQUEST_CHANGES"],
211+
});
212+
213+
const result = await localHandler({ event: "COMMENT", body: "some feedback" }, {});
214+
215+
expect(result.success).toBe(true);
216+
expect(result.event).toBe("COMMENT");
217+
});
218+
219+
it("should normalize event case before checking allowed_events", async () => {
220+
const { main } = require("./submit_pr_review.cjs");
221+
const localBuffer = createReviewBuffer();
222+
const localHandler = await main({
223+
max: 5,
224+
_prReviewBuffer: localBuffer,
225+
allowed_events: ["COMMENT"],
226+
});
227+
228+
const result = await localHandler({ event: "comment", body: "feedback" }, {});
229+
230+
expect(result.success).toBe(true);
231+
expect(result.event).toBe("COMMENT");
232+
});
233+
234+
it("should not consume max count slot when event is blocked by allowed_events", async () => {
235+
const { main } = require("./submit_pr_review.cjs");
236+
const localBuffer = createReviewBuffer();
237+
const localHandler = await main({
238+
max: 1,
239+
_prReviewBuffer: localBuffer,
240+
allowed_events: ["COMMENT"],
241+
});
242+
243+
// Blocked event should not consume a slot
244+
const blocked = await localHandler({ event: "APPROVE", body: "" }, {});
245+
expect(blocked.success).toBe(false);
246+
247+
// Allowed event should still succeed since blocked one didn't count
248+
const allowed = await localHandler({ event: "COMMENT", body: "feedback" }, {});
249+
expect(allowed.success).toBe(true);
250+
expect(allowed.event).toBe("COMMENT");
251+
});
252+
176253
it("should allow empty body for APPROVE event", async () => {
177254
const message = {
178255
type: "submit_pull_request_review",

actions/setup/js/types/safe-outputs-config.d.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@ interface SubmitPullRequestReviewConfig extends SafeOutputConfig {
109109
target?: string;
110110
"target-repo"?: string;
111111
"allowed-repos"?: string[];
112+
"allowed-events"?: Array<"APPROVE" | "COMMENT" | "REQUEST_CHANGES">;
112113
footer?: boolean | "always" | "none" | "if-body";
113114
}
114115

docs/src/content/docs/reference/frontmatter-full.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4039,6 +4039,13 @@ safe-outputs:
40394039
allowed-repos: []
40404040
# Array of strings
40414041

4042+
# Optional list of allowed review event types. If omitted, all event types
4043+
# (APPROVE, COMMENT, REQUEST_CHANGES) are allowed. Use this to restrict the agent
4044+
# to specific event types at the infrastructure level.
4045+
# (optional)
4046+
allowed-events: []
4047+
# Array of strings (APPROVE, COMMENT, REQUEST_CHANGES)
4048+
40424049
# GitHub token to use for this specific output type. Overrides global github-token
40434050
# if specified.
40444051
# (optional)

docs/src/content/docs/reference/safe-outputs.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -734,9 +734,12 @@ safe-outputs:
734734
target: "triggering" # or "*", or e.g. ${{ github.event.inputs.pr_number }} when not in pull_request trigger
735735
target-repo: "owner/repo" # cross-repository: submit review on PR in another repo
736736
allowed-repos: ["org/repo1", "org/repo2"] # additional allowed repositories
737+
allowed-events: [COMMENT, REQUEST_CHANGES] # restrict allowed review event types (default: all allowed)
737738
footer: false # omit AI-generated footer from review body (default: true)
738739
```
739740

741+
Use `allowed-events` to restrict which review event types the agent can submit. This provides infrastructure-level enforcement — for example, `allowed-events: [COMMENT, REQUEST_CHANGES]` prevents the agent from submitting APPROVE reviews regardless of what the agent attempts to output. If omitted, all event types (APPROVE, COMMENT, REQUEST_CHANGES) are allowed.
742+
740743
### Resolve PR Review Thread (`resolve-pull-request-review-thread:`)
741744

742745
Resolves review threads on pull requests. Allows AI agents to mark review conversations as resolved after addressing the feedback. Uses the GitHub GraphQL API with the `resolveReviewThread` mutation.

pkg/parser/schemas/main_workflow_schema.json

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1887,7 +1887,7 @@
18871887
},
18881888
"status-comment": {
18891889
"type": "boolean",
1890-
"description": "Whether to post status comments (started/completed) on the triggering item. When true, adds a comment with workflow run link and updates it on completion. When false or not specified, no status comments are posted. Automatically enabled for slash_command and label_command triggers manual configuration is only needed for other trigger types.",
1890+
"description": "Whether to post status comments (started/completed) on the triggering item. When true, adds a comment with workflow run link and updates it on completion. When false or not specified, no status comments are posted. Automatically enabled for slash_command and label_command triggers \u2014 manual configuration is only needed for other trigger types.",
18911891
"examples": [true, false]
18921892
},
18931893
"github-token": {
@@ -5907,6 +5907,15 @@
59075907
},
59085908
"description": "List of additional repositories in format 'owner/repo' that PR reviews can be submitted in. When specified, the agent can use a 'repo' field in the output to specify which repository to submit the review in. The target repository (current or target-repo) is always implicitly allowed."
59095909
},
5910+
"allowed-events": {
5911+
"type": "array",
5912+
"items": {
5913+
"type": "string",
5914+
"enum": ["APPROVE", "COMMENT", "REQUEST_CHANGES"]
5915+
},
5916+
"description": "Optional list of allowed review event types. If omitted, all event types (APPROVE, COMMENT, REQUEST_CHANGES) are allowed. Use this to restrict the agent to specific event types, e.g. [COMMENT, REQUEST_CHANGES] to prevent approvals.",
5917+
"minItems": 1
5918+
},
59105919
"github-token": {
59115920
"$ref": "#/$defs/github_token",
59125921
"description": "GitHub token to use for this specific output type. Overrides global github-token if specified."
@@ -7553,12 +7562,16 @@
75537562
"properties": {
75547563
"include": {
75557564
"type": "array",
7556-
"items": { "type": "string" },
7565+
"items": {
7566+
"type": "string"
7567+
},
75577568
"description": "Glob patterns for files to include"
75587569
},
75597570
"exclude": {
75607571
"type": "array",
7561-
"items": { "type": "string" },
7572+
"items": {
7573+
"type": "string"
7574+
},
75627575
"description": "Glob patterns for files to exclude"
75637576
}
75647577
},

pkg/workflow/compiler_safe_outputs_config.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -461,6 +461,7 @@ var handlerRegistry = map[string]handlerBuilder{
461461
AddIfNotEmpty("target", c.Target).
462462
AddIfNotEmpty("target-repo", c.TargetRepoSlug).
463463
AddStringSlice("allowed_repos", c.AllowedRepos).
464+
AddStringSlice("allowed_events", c.AllowedEvents).
464465
AddIfNotEmpty("github-token", c.GitHubToken).
465466
AddStringPtr("footer", getEffectiveFooterString(c.Footer, cfg.Footer)).
466467
AddIfTrue("staged", c.Staged).

pkg/workflow/submit_pr_review.go

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package workflow
22

33
import (
4+
"strings"
5+
46
"github.com/github/gh-aw/pkg/logger"
57
)
68

@@ -13,7 +15,8 @@ var submitPRReviewLog = logger.New("workflow:submit_pr_review")
1315
type SubmitPullRequestReviewConfig struct {
1416
BaseSafeOutputConfig `yaml:",inline"`
1517
SafeOutputTargetConfig `yaml:",inline"`
16-
Footer *string `yaml:"footer,omitempty"` // Controls when to show footer in PR review body: "always" (default), "none", or "if-body" (only when review has body text)
18+
Footer *string `yaml:"footer,omitempty"` // Controls when to show footer in PR review body: "always" (default), "none", or "if-body" (only when review has body text)
19+
AllowedEvents []string `yaml:"allowed-events,omitempty"` // Optional list of allowed review event types: APPROVE, COMMENT, REQUEST_CHANGES. If omitted, all event types are allowed.
1720
}
1821

1922
// parseSubmitPullRequestReviewConfig handles submit-pull-request-review configuration
@@ -71,7 +74,33 @@ func (c *Compiler) parseSubmitPullRequestReviewConfig(outputMap map[string]any)
7174
}
7275
}
7376

74-
submitPRReviewLog.Printf("Parsed submit-pull-request-review config: max=%d, target=%s, target_repo=%s", templatableIntValue(config.Max), config.Target, config.TargetRepoSlug)
77+
// Parse allowed-events configuration
78+
if allowedEvents, exists := configMap["allowed-events"]; exists {
79+
eventsSlice, ok := allowedEvents.([]any)
80+
if !ok {
81+
submitPRReviewLog.Printf("Invalid allowed-events configuration: must be a list of review event types")
82+
return nil
83+
}
84+
85+
validEvents := map[string]bool{"APPROVE": true, "COMMENT": true, "REQUEST_CHANGES": true}
86+
for _, e := range eventsSlice {
87+
if eventStr, ok := e.(string); ok {
88+
upper := strings.ToUpper(eventStr)
89+
if validEvents[upper] {
90+
config.AllowedEvents = append(config.AllowedEvents, upper)
91+
} else {
92+
submitPRReviewLog.Printf("Ignoring invalid allowed-events value: %s", eventStr)
93+
}
94+
}
95+
}
96+
97+
if len(config.AllowedEvents) == 0 {
98+
submitPRReviewLog.Printf("Invalid allowed-events configuration: at least one valid event type is required when allowed-events is specified")
99+
return nil
100+
}
101+
}
102+
103+
submitPRReviewLog.Printf("Parsed submit-pull-request-review config: max=%d, target=%s, target_repo=%s, allowed_events=%v", templatableIntValue(config.Max), config.Target, config.TargetRepoSlug, config.AllowedEvents)
75104
} else {
76105
// If configData is nil or not a map, set the default max
77106
config.Max = defaultIntStr(1)

0 commit comments

Comments
 (0)