From b52d2f08f51840eb386b4829b20656b239d13bee Mon Sep 17 00:00:00 2001 From: joshavant <830519+joshavant@users.noreply.github.com> Date: Thu, 20 Aug 2026 15:08:39 -0500 Subject: [PATCH] fix(ci): trust maintainer-authored dependency changes --- scripts/github/dependency-guard.mjs | 25 +++++++-- scripts/github/guard-shared.mjs | 21 +++---- test/scripts/dependency-guard-script.test.ts | 59 +++++++++++++++++++- 3 files changed, 89 insertions(+), 16 deletions(-) diff --git a/scripts/github/dependency-guard.mjs b/scripts/github/dependency-guard.mjs index dcd65557b567..662da4e56e83 100644 --- a/scripts/github/dependency-guard.mjs +++ b/scripts/github/dependency-guard.mjs @@ -324,7 +324,7 @@ export function renderTrustedDependencyComment({ actor, headSha }) { "", "### Dependency graph changes noted", "", - "This PR includes dependency graph changes. The dependency guard is informational because the PR author is a repository admin or a member of `@openclaw/openclaw-secops`.", + "This PR includes dependency graph changes. The dependency guard is informational because the PR author is a repository admin, a member of `@openclaw/openclaw-secops`, or an OpenClaw organization member with Maintain or Admin repository access.", "", `- Current SHA: ${markdownCode(headSha ?? "")}`, `- Trusted actor: @${sanitizeGuardDisplayValue(actor.login)}`, @@ -497,12 +497,27 @@ export function dependencyGuardTrustedActorCandidates({ pullRequest, event, curr /** * @param {{ * candidates: GuardActorCandidate[], + * pullRequest: { author_association?: string }, * isDependencyApprover: (login: string) => Promise, + * getRepositoryRoleName: (login: string) => Promise, * }} options */ -export async function findTrustedDependencyGuardActor({ candidates, isDependencyApprover }) { +export async function findTrustedDependencyGuardActor({ + candidates, + pullRequest, + isDependencyApprover, + getRepositoryRoleName, +}) { for (const candidate of candidates) { - const role = await isDependencyApprover(candidate.login); + let role = await isDependencyApprover(candidate.login); + if (!role && pullRequest.author_association === "MEMBER") { + // GitHub's MEMBER association excludes outside collaborators. Keep this role path separate + // from override approvers so Maintain authors cannot authorize another contributor's PR. + const repositoryRole = await getRepositoryRoleName(candidate.login); + if (repositoryRole === "maintain" || repositoryRole === "admin") { + role = `OpenClaw organization member with repository ${repositoryRole} role`; + } + } if (role) { return { login: candidate.login, @@ -787,7 +802,7 @@ async function main() { return; } - const { isSecurityMember, isRepositoryAdmin } = createGuardApproverChecks({ + const { getRepositoryRoleName, isSecurityMember, isRepositoryAdmin } = createGuardApproverChecks({ api, owner, repo, @@ -820,7 +835,9 @@ async function main() { } const trustedActor = await findTrustedDependencyGuardActor({ candidates: dependencyGuardTrustedActorCandidates({ pullRequest, event, currentHeadSha }), + pullRequest, isDependencyApprover, + getRepositoryRoleName, }); if (trustedActor) { if (mode === "detect") { diff --git a/scripts/github/guard-shared.mjs b/scripts/github/guard-shared.mjs index 0c5cb94b36bd..2eb81fa9f8e2 100644 --- a/scripts/github/guard-shared.mjs +++ b/scripts/github/guard-shared.mjs @@ -157,7 +157,7 @@ export function createGuardApproverChecks({ warn = console.warn, }) { const membershipCache = new Map(); - const permissionCache = new Map(); + const repositoryRoleCache = new Map(); const isSecurityMember = async (login) => { const normalizedLogin = login.toLowerCase(); if (explicitSecurityApprovers.has(normalizedLogin)) { @@ -181,27 +181,28 @@ export function createGuardApproverChecks({ return false; } }; - const isRepositoryAdmin = async (login) => { + const getRepositoryRoleName = async (login) => { const normalizedLogin = login.toLowerCase(); - if (permissionCache.has(normalizedLogin)) { - return permissionCache.get(normalizedLogin); + if (repositoryRoleCache.has(normalizedLogin)) { + return repositoryRoleCache.get(normalizedLogin); } try { const result = await api.request( `/repos/${owner}/${repo}/collaborators/${encodeURIComponent(login)}/permission`, ); - const allowed = result?.permission === "admin"; - permissionCache.set(normalizedLogin, allowed); - return allowed; + const roleName = typeof result?.role_name === "string" ? result.role_name : null; + repositoryRoleCache.set(normalizedLogin, roleName); + return roleName; } catch (error) { if (error?.status !== 404) { warn(`Could not verify repository permission for ${login}: ${error.message}`); } - permissionCache.set(normalizedLogin, false); - return false; + repositoryRoleCache.set(normalizedLogin, null); + return null; } }; - return { isSecurityMember, isRepositoryAdmin }; + const isRepositoryAdmin = async (login) => (await getRepositoryRoleName(login)) === "admin"; + return { getRepositoryRoleName, isSecurityMember, isRepositoryAdmin }; } function githubErrorBodyTooLarge(maxBytes) { diff --git a/test/scripts/dependency-guard-script.test.ts b/test/scripts/dependency-guard-script.test.ts index f85149381041..9990497e78f4 100644 --- a/test/scripts/dependency-guard-script.test.ts +++ b/test/scripts/dependency-guard-script.test.ts @@ -31,6 +31,7 @@ import { securityApproverSet, shouldAutoscrubDependencyLockfiles, } from "../../scripts/github/dependency-guard.mjs"; +import { createGuardApproverChecks } from "../../scripts/github/guard-shared.mjs"; const headSha = "a".repeat(40); const staleSha = "b".repeat(40); @@ -240,31 +241,85 @@ describe("dependency guard script", () => { await expect( findTrustedDependencyGuardActor({ candidates: untrustedAuthorCandidate, + pullRequest: { author_association: "COLLABORATOR" }, isDependencyApprover: async (login) => login === "security-user" || login === "repo-admin" ? "openclaw-secops" : null, + getRepositoryRoleName: async () => "maintain", }), ).resolves.toBeNull(); await expect( findTrustedDependencyGuardActor({ candidates: sameActorCandidates, + pullRequest: { author_association: "MEMBER" }, isDependencyApprover: async (login) => (login === "repo-admin" ? "repository admin" : null), + getRepositoryRoleName: async () => null, }), ).resolves.toEqual({ login: "repo-admin", reason: "pull request author; repository admin", }); + + await expect( + findTrustedDependencyGuardActor({ + candidates: [{ login: "maintainer", source: "pull request author" }], + pullRequest: { author_association: "MEMBER" }, + isDependencyApprover: async () => null, + getRepositoryRoleName: async () => "maintain", + }), + ).resolves.toEqual({ + login: "maintainer", + reason: "pull request author; OpenClaw organization member with repository maintain role", + }); + + const rejectedAuthorRoles: Array<[string, string]> = [ + ["COLLABORATOR", "maintain"], + ["MEMBER", "write"], + ]; + for (const [authorAssociation, repositoryRole] of rejectedAuthorRoles) { + await expect( + findTrustedDependencyGuardActor({ + candidates: [{ login: "contributor", source: "pull request author" }], + pullRequest: { author_association: authorAssociation }, + isDependencyApprover: async () => null, + getRepositoryRoleName: async () => repositoryRole, + }), + ).resolves.toBeNull(); + } + }); + + it("uses GitHub role_name without granting Maintain users comment authority", async () => { + const request = vi + .fn() + .mockResolvedValueOnce({ permission: "write", role_name: "maintain" }) + .mockResolvedValueOnce({ permission: "admin", role_name: "admin" }); + const checks = createGuardApproverChecks({ + api: { request }, + owner: "openclaw", + repo: "openclaw", + securityTeamSlug: "openclaw-secops", + explicitSecurityApprovers: new Set(), + }); + + await expect(checks.getRepositoryRoleName("maintainer")).resolves.toBe("maintain"); + await expect(checks.isRepositoryAdmin("maintainer")).resolves.toBe(false); + await expect(checks.isRepositoryAdmin("admin")).resolves.toBe(true); + expect(request).toHaveBeenCalledTimes(2); }); it("renders trusted dependency graph comments without blocker language", () => { const body = renderTrustedDependencyComment({ - actor: { login: "repo-admin", reason: "pull request author; repository admin" }, + actor: { + login: "maintainer", + reason: "pull request author; OpenClaw organization member with repository maintain role", + }, headSha, }); expect(body).toContain(""); expect(body).toContain("Dependency graph changes noted"); expect(body).toContain("informational"); - expect(body).toContain("@repo-admin"); + expect(body).toContain("OpenClaw organization member with Maintain or Admin repository access"); + expect(body).toContain("@maintainer"); expect(body).toContain(headSha); expect(body).not.toContain("are blocked"); expect(body).not.toContain("/allow-dependencies-change");