fix(prd-1): fix CI test failures and Forgejo permission check
Two issues found during M8 E2E testing on Forgejo:
1. CI test failures (31 tests): Unit tests assume GitHub context but
Forgejo runner sets GITHUB_SERVER_URL=git.wxs.ro, causing
detectPlatform() to return "forgejo" and module-level constants in
config.ts to use Forgejo URLs. Fixed by setting GITHUB_SERVER_URL
in the test step env. Forgejo-specific tests already override with
PLATFORM=forgejo in their beforeEach.
2. Permission check 403: Forgejo's getCollaboratorPermissionLevel
requires admin access. Added checkForgejoPermissions() that uses
GET /repos/{owner}/{repo} instead — returns a permissions object
with { admin, push, pull } booleans for the authenticated token.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
1 parent
c61073da42
commit
e26d08e666
3 files changed
+195
No files matched your search
@@ -36,6 +36,12 @@ jobs:
|
||||
|
||||
- name: Run tests
|
||||
run: bun test
|
||||
env:
|
||||
# Unit tests assume GitHub context. Override Forgejo runner's
|
||||
# GITHUB_SERVER_URL so module-level constants (config.ts) and
|
||||
# detectPlatform() default to GitHub. Forgejo-specific tests
|
||||
# explicitly set PLATFORM=forgejo in their own beforeEach.
|
||||
GITHUB_SERVER_URL: https://github.com
|
||||
|
||||
prettier:
|
||||
runs-on: grunt
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import * as core from "@actions/core";
|
||||
import type { ParsedGitHubContext } from "../context";
|
||||
import type { Octokit } from "@octokit/rest";
|
||||
import { detectPlatform } from "../../platform/detector";
|
||||
|
||||
/**
|
||||
* Check if the actor has write permissions to the repository
|
||||
@@ -49,6 +50,15 @@ export async function checkWritePermissions(
|
||||
return true;
|
||||
}
|
||||
|
||||
const platform = detectPlatform();
|
||||
|
||||
if (platform === "forgejo") {
|
||||
// Forgejo's getCollaboratorPermissionLevel requires admin access.
|
||||
// Use GET /repos/{owner}/{repo} instead — it returns a `permissions`
|
||||
// object for the authenticated token (push = write access).
|
||||
return await checkForgejoPermissions(octokit, repository, actor);
|
||||
}
|
||||
|
||||
// Check permissions directly using the permission endpoint
|
||||
const response = await octokit.repos.getCollaboratorPermissionLevel({
|
||||
owner: repository.owner,
|
||||
@@ -71,3 +81,43 @@ export async function checkWritePermissions(
|
||||
throw new Error(`Failed to check permissions for ${actor}: ${error}`);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Forgejo permission check using the repo endpoint.
|
||||
* GET /repos/{owner}/{repo} returns a `permissions` object for the
|
||||
* authenticated token with { admin, push, pull } booleans. We check
|
||||
* that the token has push (write) access.
|
||||
*/
|
||||
async function checkForgejoPermissions(
|
||||
octokit: Octokit,
|
||||
repository: { owner: string; repo: string },
|
||||
actor: string,
|
||||
): Promise<boolean> {
|
||||
const response = await octokit.repos.get({
|
||||
owner: repository.owner,
|
||||
repo: repository.repo,
|
||||
});
|
||||
|
||||
const permissions = (response.data as any).permissions as
|
||||
| { admin?: boolean; push?: boolean; pull?: boolean }
|
||||
| undefined;
|
||||
|
||||
if (!permissions) {
|
||||
core.warning(
|
||||
`Forgejo did not return permissions for repo ${repository.owner}/${repository.repo}`,
|
||||
);
|
||||
return false;
|
||||
}
|
||||
|
||||
if (permissions.admin || permissions.push) {
|
||||
core.info(
|
||||
`Forgejo token has write access (admin=${permissions.admin}, push=${permissions.push}) for actor: ${actor}`,
|
||||
);
|
||||
return true;
|
||||
}
|
||||
|
||||
core.warning(
|
||||
`Forgejo token has insufficient permissions (admin=${permissions.admin}, push=${permissions.push}) for actor: ${actor}`,
|
||||
);
|
||||
return false;
|
||||
}
|
||||
@@ -302,4 +302,143 @@ describe("checkWritePermissions", () => {
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("Forgejo permission check", () => {
|
||||
let savedPlatform: string | undefined;
|
||||
let savedForgejo: string | undefined;
|
||||
let savedServerUrl: string | undefined;
|
||||
|
||||
beforeEach(() => {
|
||||
savedPlatform = process.env.PLATFORM;
|
||||
savedForgejo = process.env.FORGEJO;
|
||||
savedServerUrl = process.env.GITHUB_SERVER_URL;
|
||||
process.env.PLATFORM = "forgejo";
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
if (savedPlatform !== undefined) process.env.PLATFORM = savedPlatform;
|
||||
else delete process.env.PLATFORM;
|
||||
if (savedForgejo !== undefined) process.env.FORGEJO = savedForgejo;
|
||||
else delete process.env.FORGEJO;
|
||||
if (savedServerUrl !== undefined)
|
||||
process.env.GITHUB_SERVER_URL = savedServerUrl;
|
||||
else delete process.env.GITHUB_SERVER_URL;
|
||||
});
|
||||
|
||||
const createForgejoOctokit = (permissions: {
|
||||
admin?: boolean;
|
||||
push?: boolean;
|
||||
pull?: boolean;
|
||||
}) => {
|
||||
return {
|
||||
repos: {
|
||||
get: async () => ({
|
||||
data: { permissions },
|
||||
}),
|
||||
getCollaboratorPermissionLevel: async () => {
|
||||
throw new Error("Should not be called on Forgejo");
|
||||
},
|
||||
},
|
||||
} as any;
|
||||
};
|
||||
|
||||
test("should return true when Forgejo token has push access", async () => {
|
||||
const mockOctokit = createForgejoOctokit({
|
||||
admin: false,
|
||||
push: true,
|
||||
pull: true,
|
||||
});
|
||||
const context = createContext();
|
||||
|
||||
const result = await checkWritePermissions(mockOctokit, context);
|
||||
|
||||
expect(result).toBe(true);
|
||||
expect(coreInfoSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining("Forgejo token has write access"),
|
||||
);
|
||||
});
|
||||
|
||||
test("should return true when Forgejo token has admin access", async () => {
|
||||
const mockOctokit = createForgejoOctokit({
|
||||
admin: true,
|
||||
push: false,
|
||||
pull: true,
|
||||
});
|
||||
const context = createContext();
|
||||
|
||||
const result = await checkWritePermissions(mockOctokit, context);
|
||||
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
|
||||
test("should return false when Forgejo token has only pull access", async () => {
|
||||
const mockOctokit = createForgejoOctokit({
|
||||
admin: false,
|
||||
push: false,
|
||||
pull: true,
|
||||
});
|
||||
const context = createContext();
|
||||
|
||||
const result = await checkWritePermissions(mockOctokit, context);
|
||||
|
||||
expect(result).toBe(false);
|
||||
expect(coreWarningSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining("insufficient permissions"),
|
||||
);
|
||||
});
|
||||
|
||||
test("should return false when permissions object is missing", async () => {
|
||||
const mockOctokit = {
|
||||
repos: {
|
||||
get: async () => ({ data: {} }),
|
||||
},
|
||||
} as any;
|
||||
const context = createContext();
|
||||
|
||||
const result = await checkWritePermissions(mockOctokit, context);
|
||||
|
||||
expect(result).toBe(false);
|
||||
expect(coreWarningSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining("did not return permissions"),
|
||||
);
|
||||
});
|
||||
|
||||
test("should not call getCollaboratorPermissionLevel on Forgejo", async () => {
|
||||
let collaboratorCalled = false;
|
||||
const mockOctokit = {
|
||||
repos: {
|
||||
get: async () => ({
|
||||
data: { permissions: { admin: false, push: true, pull: true } },
|
||||
}),
|
||||
getCollaboratorPermissionLevel: async () => {
|
||||
collaboratorCalled = true;
|
||||
return { data: { permission: "write" } };
|
||||
},
|
||||
},
|
||||
} as any;
|
||||
const context = createContext();
|
||||
|
||||
await checkWritePermissions(mockOctokit, context);
|
||||
|
||||
expect(collaboratorCalled).toBe(false);
|
||||
});
|
||||
|
||||
test("should still bypass for allowed_non_write_users on Forgejo", async () => {
|
||||
const mockOctokit = createForgejoOctokit({
|
||||
admin: false,
|
||||
push: false,
|
||||
pull: true,
|
||||
});
|
||||
const context = createContext();
|
||||
|
||||
const result = await checkWritePermissions(
|
||||
mockOctokit,
|
||||
context,
|
||||
"test-user",
|
||||
true,
|
||||
);
|
||||
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user