From 1f2536935cf7e41b3292469cd8046834699a89c8 Mon Sep 17 00:00:00 2001 From: Maxwell Calkin Date: Sun, 8 Mar 2026 11:43:48 -0400 Subject: [PATCH] fix: handle 404 errors in checkWritePermissions for non-user actors The collaborator permissions API returns 404 for actors like "Copilot" that are not regular GitHub users. Previously, only actors ending in "[bot]" were bypassed, causing workflows triggered by Copilot reviews to fail with "Copilot is not a user" errors. This adds 404 error handling in the catch block so that non-user actors are recognized and allowed through, matching the existing behavior for [bot] actors. Fixes #1018 Co-Authored-By: Claude Opus 4.6 --- src/github/validation/permissions.ts | 13 +++++- test/permissions.test.ts | 69 +++++++++++++++++++++++++++- 2 files changed, 80 insertions(+), 2 deletions(-) diff --git a/src/github/validation/permissions.ts b/src/github/validation/permissions.ts index 731fcd41c..6a71bfc7b 100644 --- a/src/github/validation/permissions.ts +++ b/src/github/validation/permissions.ts @@ -66,7 +66,18 @@ export async function checkWritePermissions( core.warning(`Actor has insufficient permissions: ${permissionLevel}`); return false; } - } catch (error) { + } catch (error: any) { + // Handle 404 errors for non-user actors (e.g., "Copilot") + // The collaborator permission API returns 404 when the actor is not a + // regular GitHub user. These are GitHub system actors that inherently + // operate with the permissions granted by the workflow configuration. + if (error?.status === 404) { + core.info( + `Actor "${actor}" is not a GitHub user (received 404). Treating as a non-user actor and allowing.`, + ); + return true; + } + core.error(`Failed to check permissions: ${error}`); throw new Error(`Failed to check permissions for ${actor}: ${error}`); } diff --git a/test/permissions.test.ts b/test/permissions.test.ts index cf2efdb0e..0050a1bce 100644 --- a/test/permissions.test.ts +++ b/test/permissions.test.ts @@ -140,7 +140,53 @@ describe("checkWritePermissions", () => { expect(result).toBe(true); }); - test("should throw error when permission check fails", async () => { + test("should return true when API returns 404 for non-user actor", async () => { + const httpError = Object.assign( + new Error("Copilot is not a user"), + { status: 404 }, + ); + const mockOctokit = { + repos: { + getCollaboratorPermissionLevel: async () => { + throw httpError; + }, + }, + } as any; + const context = createContext(); + context.actor = "Copilot"; + + const result = await checkWritePermissions(mockOctokit, context); + + expect(result).toBe(true); + expect(coreInfoSpy).toHaveBeenCalledWith( + 'Actor "Copilot" is not a GitHub user (received 404). Treating as a non-user actor and allowing.', + ); + }); + + test("should return true when API returns 404 for any non-user actor name", async () => { + const httpError = Object.assign( + new Error("some-service is not a user"), + { status: 404 }, + ); + const mockOctokit = { + repos: { + getCollaboratorPermissionLevel: async () => { + throw httpError; + }, + }, + } as any; + const context = createContext(); + context.actor = "some-service"; + + const result = await checkWritePermissions(mockOctokit, context); + + expect(result).toBe(true); + expect(coreInfoSpy).toHaveBeenCalledWith( + 'Actor "some-service" is not a GitHub user (received 404). Treating as a non-user actor and allowing.', + ); + }); + + test("should throw error when permission check fails with non-404 error", async () => { const error = new Error("API error"); const mockOctokit = { repos: { @@ -160,6 +206,27 @@ describe("checkWritePermissions", () => { ); }); + test("should throw error for 500 server errors", async () => { + const httpError = Object.assign( + new Error("Internal Server Error"), + { status: 500 }, + ); + const mockOctokit = { + repos: { + getCollaboratorPermissionLevel: async () => { + throw httpError; + }, + }, + } as any; + const context = createContext(); + + await expect(checkWritePermissions(mockOctokit, context)).rejects.toThrow( + "Failed to check permissions for test-user", + ); + + expect(coreErrorSpy).toHaveBeenCalled(); + }); + test("should call API with correct parameters", async () => { let capturedParams: any; const mockOctokit = {