fix: allow admins to approve dependency guard (#88966)

* fix: allow admins to approve dependency guard * fix: auto-bypass trusted dependency authors
2026-06-03 21:24:05 +00:00 · 2026-06-01 10:17:14 -07:00
parent 469bec97ef
commit e3d24faecd
3 changed files with 261 additions and 33 deletions
--- a/scripts/github/dependency-guard.mjs
+++ b/scripts/github/dependency-guard.mjs
@@ -222,6 +222,14 @@ export function isDependencyGuardAuthorizedForHead(comment, currentHeadSha) {
  );
 }

+export function isDependencyGuardTrustedForHead(comment, currentHeadSha) {
+  return (
+    Boolean(currentHeadSha) &&
+    comment?.body?.includes("### Dependency graph changes noted") === true &&
+    dependencyGuardCommentHeadSha(comment) === currentHeadSha
+  );
+}
+
 export function securityApproverSet(value) {
  return new Set(
    String(value ?? "")
@@ -277,7 +285,7 @@ export function renderAuthorizedDependencyComment(override) {
    "",
    "### Dependency graph change authorized",
    "",
-    "This PR includes dependency graph changes. A member of `@openclaw/openclaw-secops` authorized this exact head SHA with `/allow-dependencies-change`.",
+    "This PR includes dependency graph changes. A repository admin or member of `@openclaw/openclaw-secops` authorized this exact head SHA with `/allow-dependencies-change`.",
    "",
    `- Approved SHA: ${markdownCode(override.sha)}`,
    `- Approved by: @${sanitizeDisplayValue(override.login)}`,
@@ -289,6 +297,22 @@ export function renderAuthorizedDependencyComment(override) {
  return lines.join("\n");
 }

+export function renderTrustedDependencyComment({ actor, headSha }) {
+  return [
+    dependencyGraphGuardMarker,
+    "",
+    "### 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`.",
+    "",
+    `- Current SHA: ${markdownCode(headSha ?? "<head-sha>")}`,
+    `- Trusted actor: @${sanitizeDisplayValue(actor.login)}`,
+    `- Trusted role: ${markdownCode(actor.reason)}`,
+    "",
+    "Security review is still recommended before merge when the dependency graph change is intentional.",
+  ].join("\n");
+}
+
 export function renderAutoscrubbedDependencyComment({ baseBranch, lockfileChanges, commitSha }) {
  const safeBranch = sanitizeDisplayValue(baseBranch ?? "main");
  const fileLines = lockfileChanges.map((path) => `- ${markdownCode(path)}`);
@@ -361,14 +385,14 @@ export function renderBlockedDependencyComment({
    "",
    "### Dependency graph changes are blocked",
    "",
-    "OpenClaw does not accept dependency graph changes through PRs unless security explicitly authorizes the current head SHA. Dependency updates are generated internally by maintainers so external PRs cannot change the resolved graph.",
+    "OpenClaw does not accept dependency graph changes through PRs unless a repository admin or security explicitly authorizes the current head SHA. Dependency updates are generated internally by maintainers so external PRs cannot change the resolved graph.",
    "",
    "Detected dependency graph changes:",
    ...reasons,
    ...autoscrubLines,
    ...removalSteps,
    "",
-    "If this PR intentionally needs a dependency graph change, ask a member of `@openclaw/openclaw-secops` to comment:",
+    "If this PR intentionally needs a dependency graph change, ask a repository admin or member of `@openclaw/openclaw-secops` to comment:",
    "",
    "```text",
    allowDependenciesCommand,
@@ -415,6 +439,44 @@ function renderAutoscrubStatusLines(status) {
  return [];
 }

+export function dependencyGuardTrustedActorCandidates({ pullRequest, event, currentHeadSha }) {
+  const eventHeadSha = event?.pull_request?.head?.sha;
+  const eventAfterSha = event?.after;
+  const eventMatchesCurrentHead =
+    Boolean(currentHeadSha) &&
+    (eventHeadSha === currentHeadSha || eventAfterSha === currentHeadSha);
+  if (!eventMatchesCurrentHead) {
+    return [];
+  }
+  const candidates = [];
+  const seen = new Set();
+  for (const [source, login] of [["pull request author", pullRequest?.user?.login]]) {
+    if (typeof login !== "string" || login.length === 0) {
+      continue;
+    }
+    const normalizedLogin = login.toLowerCase();
+    if (seen.has(normalizedLogin)) {
+      continue;
+    }
+    seen.add(normalizedLogin);
+    candidates.push({ login, source });
+  }
+  return candidates;
+}
+
+export async function findTrustedDependencyGuardActor({ candidates, isDependencyApprover }) {
+  for (const candidate of candidates) {
+    const role = await isDependencyApprover(candidate.login);
+    if (role) {
+      return {
+        login: candidate.login,
+        reason: `${candidate.source}; ${role}`,
+      };
+    }
+  }
+  return null;
+}
+
 function renderManifestChangeLine(change) {
  return `- ${markdownCode(change.path)} changed ${change.fields.map(markdownCode).join(", ")}.`;
 }
@@ -794,6 +856,98 @@ async function main() {
    return;
  }

+  const membershipCache = new Map();
+  const permissionCache = new Map();
+  const isSecurityMember = async (login) => {
+    const normalizedLogin = login.toLowerCase();
+    if (explicitSecurityApprovers.has(normalizedLogin)) {
+      return true;
+    }
+    if (membershipCache.has(normalizedLogin)) {
+      return membershipCache.get(normalizedLogin);
+    }
+    try {
+      const membership = await api.request(
+        `/orgs/${owner}/teams/${securityTeamSlug}/memberships/${encodeURIComponent(login)}`,
+      );
+      const allowed = membership?.state === "active";
+      membershipCache.set(normalizedLogin, allowed);
+      return allowed;
+    } catch (error) {
+      if (error?.status !== 404) {
+        console.warn(`Could not verify ${login} against ${securityTeamSlug}: ${error.message}`);
+      }
+      membershipCache.set(normalizedLogin, false);
+      return false;
+    }
+  };
+  const isRepositoryAdmin = async (login) => {
+    const normalizedLogin = login.toLowerCase();
+    if (permissionCache.has(normalizedLogin)) {
+      return permissionCache.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;
+    } catch (error) {
+      if (error?.status !== 404) {
+        console.warn(`Could not verify repository permission for ${login}: ${error.message}`);
+      }
+      permissionCache.set(normalizedLogin, false);
+      return false;
+    }
+  };
+  const isDependencyApprover = async (login) => {
+    if (await isSecurityMember(login)) {
+      return securityTeamSlug;
+    }
+    if (await isRepositoryAdmin(login)) {
+      return "repository admin";
+    }
+    return null;
+  };
+  const currentHeadSha = pullRequest.head?.sha;
+  if (isDependencyGuardTrustedForHead(existingGuardComment, currentHeadSha)) {
+    if (mode === "detect") {
+      await setOutput("autoscrub", "false");
+    }
+    await writeSummary(
+      [
+        "## Dependency Guard",
+        "",
+        `Dependency graph change remains informational for a trusted actor at ${markdownCode(currentHeadSha)}.`,
+      ].join("\n"),
+    );
+    console.log("Dependency graph change remains informational for this head SHA.");
+    return;
+  }
+  const trustedActor = await findTrustedDependencyGuardActor({
+    candidates: dependencyGuardTrustedActorCandidates({ pullRequest, event, currentHeadSha }),
+    isDependencyApprover,
+  });
+  if (trustedActor) {
+    if (mode === "detect") {
+      await setOutput("autoscrub", "false");
+    }
+    await upsertComment(
+      existingGuardComment,
+      renderTrustedDependencyComment({ actor: trustedActor, headSha: currentHeadSha }),
+    );
+    await writeSummary(
+      [
+        "## Dependency Guard",
+        "",
+        `Dependency graph change noted for trusted actor @${sanitizeDisplayValue(trustedActor.login)} and allowed to continue.`,
+      ].join("\n"),
+    );
+    console.log("Dependency graph change noted for trusted actor; guard is informational.");
+    return;
+  }
+
  const autoscrubCandidate = shouldAutoscrubDependencyLockfiles({
    dependencyFiles,
    lockfileChanges,
@@ -891,32 +1045,6 @@ async function main() {
      };
    }
  }
-
-  const membershipCache = new Map();
-  const isSecurityMember = async (login) => {
-    const normalizedLogin = login.toLowerCase();
-    if (explicitSecurityApprovers.size > 0) {
-      return explicitSecurityApprovers.has(normalizedLogin);
-    }
-    if (membershipCache.has(login)) {
-      return membershipCache.get(login);
-    }
-    try {
-      const membership = await api.request(
-        `/orgs/${owner}/teams/${securityTeamSlug}/memberships/${encodeURIComponent(login)}`,
-      );
-      const allowed = membership?.state === "active";
-      membershipCache.set(login, allowed);
-      return allowed;
-    } catch (error) {
-      if (error?.status !== 404) {
-        console.warn(`Could not verify ${login} against ${securityTeamSlug}: ${error.message}`);
-      }
-      membershipCache.set(login, false);
-      return false;
-    }
-  };
-  const currentHeadSha = pullRequest.head?.sha;
  if (isDependencyGuardAuthorizedForHead(existingGuardComment, currentHeadSha)) {
    await writeSummary(
      [
@@ -931,7 +1059,7 @@ async function main() {
  const override = await findDependencyOverrideCommandAsync({
    comments,
    expectedSha: dependencyOverrideExpectedSha(existingGuardComment, currentHeadSha),
-    isSecurityMember,
+    isSecurityMember: async (login) => Boolean(await isDependencyApprover(login)),
    newerThan: existingGuardComment?.updated_at ?? existingGuardComment?.created_at,
  });
  if (override) {
@@ -943,7 +1071,7 @@ async function main() {
        `Dependency graph change authorized by @${sanitizeDisplayValue(override.login)} for ${markdownCode(override.sha)}.`,
      ].join("\n"),
    );
-    console.log("Dependency graph change authorized by security override.");
+    console.log("Dependency graph change authorized by trusted override.");
    return;
  }

@@ -958,9 +1086,11 @@ async function main() {
    }),
  );
  await writeSummary(
-    "## Dependency Guard\n\nDependency graph changes are blocked without a current secops override.",
+    "## Dependency Guard\n\nDependency graph changes are blocked without a current admin or secops override.",
+  );
+  throw new Error(
+    "Dependency graph changes require removal or a current admin or secops override.",
  );
-  throw new Error("Dependency graph changes require removal or a current secops override.");
 }

 if (import.meta.url === `file://${process.argv[1]}`) {
--- a/test/scripts/dependency-guard-script.test.ts
+++ b/test/scripts/dependency-guard-script.test.ts
@@ -5,22 +5,26 @@ import {
  createAutoscrubCommit,
  dependencyGuardCommentAuthors,
  dependencyGuardCommentHeadSha,
+  dependencyGuardTrustedActorCandidates,
  dependencyFieldChanges,
  dependencyOverrideExpectedSha,
  findDependencyOverrideCommand,
  findDependencyOverrideCommandAsync,
+  findTrustedDependencyGuardActor,
  githubApi,
  isAutoscrubbedDependencyComment,
  isDependencyGuardAuthorizedForHead,
  isDependencyFile,
  isDependencyGuardMarkerComment,
  isDependencyManifest,
+  isDependencyGuardTrustedForHead,
  isPackageLockfile,
  readBoundedGitHubErrorText,
  renderAuthorizedDependencyComment,
  renderAutoscrubbedDependencyComment,
  renderBlockedDependencyComment,
  renderClearedDependencyGuardComment,
+  renderTrustedDependencyComment,
  sanitizeDisplayValue,
  securityApproverSet,
  shouldAutoscrubDependencyLockfiles,
@@ -142,6 +146,89 @@ describe("dependency guard script", () => {
    ).resolves.toBeNull();
  });

+  it("accepts repository admins through the same sha-bound override command", async () => {
+    const comments = [
+      {
+        body: "/allow-dependencies-change admin reviewed",
+        created_at: "2026-05-28T20:03:00Z",
+        html_url: "https://example.test/comment",
+        user: { login: "repo-admin" },
+      },
+    ];
+
+    await expect(
+      findDependencyOverrideCommandAsync({
+        comments,
+        expectedSha: headSha,
+        isSecurityMember: async (login) => login === "repo-admin",
+        newerThan: "2026-05-28T20:02:00Z",
+      }),
+    ).resolves.toEqual({
+      login: "repo-admin",
+      reason: "admin reviewed",
+      sha: headSha,
+      url: "https://example.test/comment",
+    });
+  });
+
+  it("recognizes trusted dependency guard actors automatically", async () => {
+    const sameActorCandidates = dependencyGuardTrustedActorCandidates({
+      pullRequest: { user: { login: "repo-admin" } },
+      event: { pull_request: { head: { sha: headSha } }, sender: { login: "repo-admin" } },
+      currentHeadSha: headSha,
+    });
+    const untrustedAuthorCandidate = dependencyGuardTrustedActorCandidates({
+      pullRequest: { user: { login: "contributor" } },
+      event: { after: headSha, sender: { login: "security-user" } },
+      currentHeadSha: headSha,
+    });
+    const staleAuthorCandidate = dependencyGuardTrustedActorCandidates({
+      pullRequest: { user: { login: "repo-admin" } },
+      event: { pull_request: { head: { sha: staleSha } }, sender: { login: "repo-admin" } },
+      currentHeadSha: headSha,
+    });
+
+    expect(sameActorCandidates).toEqual([{ login: "repo-admin", source: "pull request author" }]);
+    expect(untrustedAuthorCandidate).toEqual([
+      { login: "contributor", source: "pull request author" },
+    ]);
+    expect(staleAuthorCandidate).toEqual([]);
+
+    await expect(
+      findTrustedDependencyGuardActor({
+        candidates: untrustedAuthorCandidate,
+        isDependencyApprover: async (login) =>
+          login === "security-user" || login === "repo-admin" ? "openclaw-secops" : null,
+      }),
+    ).resolves.toBeNull();
+    await expect(
+      findTrustedDependencyGuardActor({
+        candidates: sameActorCandidates,
+        isDependencyApprover: async (login) => (login === "repo-admin" ? "repository admin" : null),
+      }),
+    ).resolves.toEqual({
+      login: "repo-admin",
+      reason: "pull request author; repository admin",
+    });
+  });
+
+  it("renders trusted dependency graph comments without blocker language", () => {
+    const body = renderTrustedDependencyComment({
+      actor: { login: "repo-admin", reason: "pull request author; repository admin" },
+      headSha,
+    });
+
+    expect(body).toContain("<!-- openclaw:dependency-graph-guard -->");
+    expect(body).toContain("Dependency graph changes noted");
+    expect(body).toContain("informational");
+    expect(body).toContain("@repo-admin");
+    expect(body).toContain(headSha);
+    expect(body).not.toContain("are blocked");
+    expect(body).not.toContain("/allow-dependencies-change");
+    expect(isDependencyGuardTrustedForHead({ body }, headSha)).toBe(true);
+    expect(isDependencyGuardTrustedForHead({ body }, staleSha)).toBe(false);
+  });
+
  it("rejects override commands without a freshness barrier", () => {
    const override = findDependencyOverrideCommand({
      comments: [
--- a/test/scripts/dependency-guard-workflow.test.ts
+++ b/test/scripts/dependency-guard-workflow.test.ts
@@ -221,6 +221,17 @@ describe("dependency guard workflow", () => {
    expect(autoscrubCommentIndex).toBeGreaterThan(deleteCommentIndex);
  });

+  it("checks trusted actors before autoscrub can mutate dependency changes", () => {
+    const script = readFileSync("scripts/github/dependency-guard.mjs", "utf8");
+    const trustedActorIndex = script.indexOf("const trustedActor =");
+    const autoscrubCandidateIndex = script.indexOf("const autoscrubCandidate =");
+    const autoscrubOutputIndex = script.indexOf('await setOutput("autoscrub", "true")');
+
+    expect(trustedActorIndex).toBeGreaterThan(0);
+    expect(autoscrubCandidateIndex).toBeGreaterThan(trustedActorIndex);
+    expect(autoscrubOutputIndex).toBeGreaterThan(trustedActorIndex);
+  });
+
  it("requires secops review for future workflow or guard changes", () => {
    const codeowners = readFileSync(CODEOWNERS, "utf8");
    expect(codeowners).toContain(