-
Notifications
You must be signed in to change notification settings - Fork 526
Add safe output for approving fork pull request workflow runs #52541
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 2 commits
Commits
Show all changes
32 commits
Select commit
Hold shift + click to select a range
34e5934
Add approve workflow run safe output
Copilot 529f1d2
Preserve approval budget on validation failures
Copilot a2b80a6
Add draft ADR for approve-workflow-run safe output (PR #52541)
github-actions[bot] 274953a
Merge branch 'main' into copilot/add-safe-output-type
github-actions[bot] 6ccb138
Fix workflow-run approval validation
Copilot ab48671
Merge branch 'main' into copilot/add-safe-output-type
github-actions[bot] 267816b
Merge branch 'main' into copilot/add-safe-output-type
github-actions[bot] 9158266
Fix workflow approval status guard
Copilot 6277b83
Merge branch 'main' into copilot/add-safe-output-type
github-actions[bot] 6ce18e1
Merge branch 'main' into copilot/add-safe-output-type
github-actions[bot] 12d020e
Merge branch 'main' into copilot/add-safe-output-type
github-actions[bot] 243e934
Require explicit workflow approval credentials
Copilot 74c285b
Merge branch 'main' into copilot/add-safe-output-type
github-actions[bot] 58860c4
Merge branch 'main' into copilot/add-safe-output-type
github-actions[bot] ca18c9d
Merge branch 'main' into copilot/add-safe-output-type
github-actions[bot] eaa21dc
Correct workflow approval status documentation
Copilot fe3df73
Authorize workflow run approvals by pull request
Copilot 833dc9e
Add workflow approval integration test
Copilot b1c23eb
Document workflow run approval output
Copilot d5a9ebd
Mark workflow approval experimental
Copilot e7937a1
Block approval for protected pull request changes
Copilot 94f3339
Require explicit fork approval opt-in
Copilot 1d65b92
Merge branch 'main' into copilot/add-safe-output-type
github-actions[bot] 90b0c57
Merge branch 'main' into copilot/add-safe-output-type
github-actions[bot] bc028d2
Merge branch 'main' into copilot/add-safe-output-type
github-actions[bot] b32f118
Preserve workflow approval retries
Copilot 3473c92
Merge branch 'main' into copilot/add-safe-output-type
github-actions[bot] 1be5064
Define PR-scoped workflow run approvals
Copilot 711d477
Merge branch 'main' into copilot/add-safe-output-type
github-actions[bot] 8bf4c20
Restrict workflow run approvals by workflow
Copilot ef4b786
Merge branch 'main' into copilot/add-safe-output-type
github-actions[bot] a93d775
Fix JavaScript lint formatting
Copilot File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,96 @@ | ||
| // @ts-check | ||
| /// <reference types="@actions/github-script" /> | ||
|
|
||
| /** | ||
| * @typedef {import('./types/handler-factory').HandlerFactoryFunction} HandlerFactoryFunction | ||
| */ | ||
|
|
||
| const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs"); | ||
| const { getErrorMessage } = require("./error_helpers.cjs"); | ||
| const { isStagedMode } = require("./safe_output_helpers.cjs"); | ||
| const { logStagedPreviewInfo } = require("./staged_preview.cjs"); | ||
|
|
||
| /** @type {string} Safe output type handled by this module */ | ||
| const HANDLER_TYPE = "approve_workflow_run"; | ||
|
|
||
| /** | ||
| * @param {unknown} value | ||
| * @returns {number | undefined} | ||
| */ | ||
| function parseRunId(value) { | ||
| if (typeof value !== "number" && typeof value !== "string") return undefined; | ||
| const normalized = typeof value === "string" ? value.trim() : value; | ||
| if (normalized === "") return undefined; | ||
| const runId = Number(normalized); | ||
| if (!Number.isSafeInteger(runId) || runId <= 0) return undefined; | ||
| return runId; | ||
| } | ||
|
|
||
| /** | ||
| * Main handler factory for approve_workflow_run. | ||
| * @type {HandlerFactoryFunction} | ||
| */ | ||
| async function main(config = {}) { | ||
| const maxCount = config.max || 1; | ||
| const githubClient = await createAuthenticatedGitHubClient(config); | ||
| const isStaged = isStagedMode(config); | ||
| let processedCount = 0; | ||
|
|
||
| core.info(`Approve workflow run configuration: max=${maxCount}`); | ||
|
|
||
| return async function handleApproveWorkflowRun(message) { | ||
| if (processedCount >= maxCount) { | ||
| core.warning(`Skipping ${HANDLER_TYPE}: max count of ${maxCount} reached`); | ||
| return { success: false, error: `Max count of ${maxCount} reached` }; | ||
| } | ||
|
|
||
| const runId = parseRunId(message.run_id); | ||
| if (!runId) { | ||
| const error = "run_id must be a positive integer"; | ||
| core.warning(error); | ||
| return { success: false, error }; | ||
| } | ||
|
|
||
| try { | ||
| const { data: run } = await githubClient.rest.actions.getWorkflowRun({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| run_id: runId, | ||
|
github-actions[bot] marked this conversation as resolved.
|
||
| }); | ||
|
|
||
| if (run.event !== "pull_request" || !Array.isArray(run.pull_requests) || run.pull_requests.length === 0) { | ||
| const error = `Workflow run ${runId} is not associated with a pull request`; | ||
| core.warning(error); | ||
| return { success: false, error }; | ||
|
github-actions[bot] marked this conversation as resolved.
|
||
| } | ||
|
|
||
| if (run.conclusion !== "action_required") { | ||
|
github-actions[bot] marked this conversation as resolved.
Outdated
|
||
| const error = `Workflow run ${runId} is not awaiting approval (conclusion: ${run.conclusion || "none"})`; | ||
| core.warning(error); | ||
| return { success: false, error }; | ||
| } | ||
|
|
||
| processedCount++; | ||
|
github-actions[bot] marked this conversation as resolved.
|
||
|
|
||
| if (isStaged) { | ||
| logStagedPreviewInfo(`Would approve workflow run ${runId}`); | ||
| return { success: true, staged: true, run_id: runId, url: run.html_url }; | ||
| } | ||
|
|
||
|
github-actions[bot] marked this conversation as resolved.
|
||
| await githubClient.rest.actions.approveWorkflowRun({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| run_id: runId, | ||
| }); | ||
|
|
||
| core.info(`Approved workflow run ${runId}: ${run.html_url}`); | ||
| return { success: true, run_id: runId, url: run.html_url }; | ||
| } catch (error) { | ||
| const errorMessage = getErrorMessage(error); | ||
| core.error(`Failed to approve workflow run ${runId}: ${errorMessage}`); | ||
| return { success: false, error: errorMessage }; | ||
| } | ||
| }; | ||
| } | ||
|
|
||
| module.exports = { main, parseRunId }; | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,142 @@ | ||
| // @ts-check | ||
| import { beforeEach, describe, expect, it, vi } from "vitest"; | ||
|
|
||
| const mockGetWorkflowRun = vi.fn(); | ||
| const mockApproveWorkflowRun = vi.fn(); | ||
|
|
||
| global.core = { | ||
| info: vi.fn(), | ||
| warning: vi.fn(), | ||
| error: vi.fn(), | ||
| }; | ||
|
|
||
| global.context = { | ||
| repo: { owner: "test-owner", repo: "test-repo" }, | ||
| }; | ||
|
|
||
| global.github = { | ||
| rest: { | ||
| actions: { | ||
| getWorkflowRun: mockGetWorkflowRun, | ||
| approveWorkflowRun: mockApproveWorkflowRun, | ||
| }, | ||
| }, | ||
| }; | ||
|
|
||
| const pendingPullRequestRun = { | ||
| event: "pull_request", | ||
| conclusion: "action_required", | ||
| html_url: "https://github.com/test-owner/test-repo/actions/runs/123", | ||
| pull_requests: [{ number: 42 }], | ||
| }; | ||
|
|
||
| describe("approve_workflow_run", () => { | ||
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
| mockGetWorkflowRun.mockResolvedValue({ data: pendingPullRequestRun }); | ||
| mockApproveWorkflowRun.mockResolvedValue({ status: 201 }); | ||
| }); | ||
|
|
||
| it("approves an eligible pull request workflow run", async () => { | ||
| const { main } = require("./approve_workflow_run.cjs"); | ||
| const handler = await main(); | ||
|
|
||
| const result = await handler({ run_id: 123 }, {}); | ||
|
|
||
| expect(result).toEqual({ | ||
| success: true, | ||
| run_id: 123, | ||
| url: pendingPullRequestRun.html_url, | ||
| }); | ||
| expect(mockApproveWorkflowRun).toHaveBeenCalledWith({ | ||
| owner: "test-owner", | ||
| repo: "test-repo", | ||
| run_id: 123, | ||
| }); | ||
| }); | ||
|
|
||
| it("accepts a decimal run ID string", async () => { | ||
| const { main } = require("./approve_workflow_run.cjs"); | ||
| const handler = await main(); | ||
|
|
||
| const result = await handler({ run_id: "123" }, {}); | ||
|
|
||
| expect(result.success).toBe(true); | ||
| expect(mockGetWorkflowRun).toHaveBeenCalledWith(expect.objectContaining({ run_id: 123 })); | ||
| }); | ||
|
|
||
| it.each([undefined, "", 0, -1, 1.5, "abc", "12abc"])("rejects invalid run ID %j", async runId => { | ||
| const { main } = require("./approve_workflow_run.cjs"); | ||
| const handler = await main(); | ||
|
|
||
| const result = await handler({ run_id: runId }, {}); | ||
|
|
||
| expect(result.success).toBe(false); | ||
| expect(result.error).toContain("positive integer"); | ||
| expect(mockGetWorkflowRun).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("rejects runs that are not associated with a pull request", async () => { | ||
| mockGetWorkflowRun.mockResolvedValue({ | ||
| data: { ...pendingPullRequestRun, event: "push", pull_requests: [] }, | ||
| }); | ||
| const { main } = require("./approve_workflow_run.cjs"); | ||
| const handler = await main(); | ||
|
|
||
| const result = await handler({ run_id: 123 }, {}); | ||
|
|
||
| expect(result.success).toBe(false); | ||
| expect(result.error).toContain("not associated with a pull request"); | ||
| expect(mockApproveWorkflowRun).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("rejects runs that are not awaiting approval", async () => { | ||
| mockGetWorkflowRun.mockResolvedValue({ | ||
| data: { ...pendingPullRequestRun, conclusion: "success" }, | ||
| }); | ||
| const { main } = require("./approve_workflow_run.cjs"); | ||
| const handler = await main(); | ||
|
|
||
| const result = await handler({ run_id: 123 }, {}); | ||
|
|
||
| expect(result.success).toBe(false); | ||
| expect(result.error).toContain("not awaiting approval"); | ||
| expect(mockApproveWorkflowRun).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("previews without approving in staged mode", async () => { | ||
| const { main } = require("./approve_workflow_run.cjs"); | ||
| const handler = await main({ staged: true }); | ||
|
|
||
| const result = await handler({ run_id: 123 }, {}); | ||
|
|
||
| expect(result.success).toBe(true); | ||
| expect(result.staged).toBe(true); | ||
| expect(mockApproveWorkflowRun).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("enforces the configured maximum", async () => { | ||
| const { main } = require("./approve_workflow_run.cjs"); | ||
| const handler = await main({ max: 1 }); | ||
|
|
||
| expect((await handler({ run_id: 123 }, {})).success).toBe(true); | ||
| const result = await handler({ run_id: 124 }, {}); | ||
|
|
||
| expect(result.success).toBe(false); | ||
| expect(result.error).toContain("Max count of 1 reached"); | ||
| }); | ||
|
|
||
| it("does not consume the maximum for an ineligible run", async () => { | ||
| mockGetWorkflowRun | ||
| .mockResolvedValueOnce({ | ||
| data: { ...pendingPullRequestRun, conclusion: "success" }, | ||
| }) | ||
| .mockResolvedValueOnce({ data: pendingPullRequestRun }); | ||
| const { main } = require("./approve_workflow_run.cjs"); | ||
| const handler = await main({ max: 1 }); | ||
|
github-actions[bot] marked this conversation as resolved.
Outdated
|
||
|
|
||
| expect((await handler({ run_id: 122 }, {})).success).toBe(false); | ||
| expect((await handler({ run_id: 123 }, {})).success).toBe(true); | ||
| expect(mockApproveWorkflowRun).toHaveBeenCalledTimes(1); | ||
| }); | ||
| }); | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.