mirror of
https://github.com/openclaw/openclaw.git
synced 2026-06-02 03:44:57 +00:00
ci: guard dependency graph PR changes (#87791)
This commit is contained in:
Dallin Romney
279
test/scripts/dependency-change-awareness-script.test.ts
Normal file
279
test/scripts/dependency-change-awareness-script.test.ts
Normal file
@@ -0,0 +1,279 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import {
|
||||
dependencyGuardCommentHeadSha,
|
||||
dependencyFieldChanges,
|
||||
dependencyOverrideExpectedSha,
|
||||
findDependencyOverrideCommand,
|
||||
findDependencyOverrideCommandAsync,
|
||||
isDependencyGuardAuthorizedForHead,
|
||||
isDependencyFile,
|
||||
isDependencyManifest,
|
||||
isPackageLockfile,
|
||||
renderAuthorizedDependencyComment,
|
||||
renderBlockedDependencyComment,
|
||||
renderClearedDependencyGuardComment,
|
||||
sanitizeDisplayValue,
|
||||
securityApproverSet,
|
||||
} from "../../scripts/github/dependency-change-awareness.mjs";
|
||||
|
||||
const headSha = "a".repeat(40);
|
||||
const staleSha = "b".repeat(40);
|
||||
|
||||
describe("dependency change awareness script", () => {
|
||||
it("detects dependency awareness file surfaces", () => {
|
||||
expect(isDependencyFile("pnpm-lock.yaml")).toBe(true);
|
||||
expect(isDependencyFile("package.json")).toBe(false);
|
||||
expect(isDependencyFile("ui/package.json")).toBe(false);
|
||||
expect(isDependencyFile("packages/core/package.json")).toBe(false);
|
||||
expect(isDependencyFile("qa/convex-credential-broker/package.json")).toBe(false);
|
||||
expect(isDependencyFile("extensions/slack/npm-shrinkwrap.json")).toBe(true);
|
||||
expect(isDependencyFile("tools/nested/pnpm-lock.yaml")).toBe(true);
|
||||
expect(isDependencyFile("src/index.ts")).toBe(false);
|
||||
expect(isPackageLockfile("pnpm-lock.yaml")).toBe(true);
|
||||
expect(isPackageLockfile("extensions/slack/npm-shrinkwrap.json")).toBe(true);
|
||||
expect(isPackageLockfile("package.json")).toBe(false);
|
||||
});
|
||||
|
||||
it("compares package manifest fields that can affect dependency resolution", () => {
|
||||
expect(isDependencyManifest("package.json")).toBe(true);
|
||||
expect(isDependencyManifest("extensions/slack/package.json")).toBe(true);
|
||||
expect(isDependencyManifest("qa/convex-credential-broker/package.json")).toBe(true);
|
||||
expect(isDependencyManifest("src/index.ts")).toBe(false);
|
||||
expect(
|
||||
dependencyFieldChanges(
|
||||
{ scripts: { test: "old" }, dependencies: { a: "1" } },
|
||||
{ scripts: { test: "new" }, dependencies: { a: "1" } },
|
||||
),
|
||||
).toEqual([]);
|
||||
expect(
|
||||
dependencyFieldChanges(
|
||||
{ dependencies: { a: "1" }, devDependencies: { b: "1" } },
|
||||
{ dependencies: { a: "2" }, devDependencies: { b: "1", c: "1" } },
|
||||
),
|
||||
).toEqual(["dependencies", "devDependencies"]);
|
||||
expect(
|
||||
dependencyFieldChanges(
|
||||
{
|
||||
optionalDependencies: { a: "1" },
|
||||
peerDependencies: { b: "1" },
|
||||
overrides: { c: "1" },
|
||||
packageManager: "pnpm@10.0.0",
|
||||
pnpm: { patchedDependencies: { d: "patches/d.patch" } },
|
||||
scripts: { test: "old" },
|
||||
},
|
||||
{
|
||||
optionalDependencies: { a: "2" },
|
||||
peerDependencies: { b: "2" },
|
||||
overrides: { c: "2" },
|
||||
packageManager: "pnpm@10.1.0",
|
||||
pnpm: { patchedDependencies: { d: "patches/d2.patch" } },
|
||||
scripts: { test: "new" },
|
||||
},
|
||||
),
|
||||
).toEqual([
|
||||
"optionalDependencies",
|
||||
"peerDependencies",
|
||||
"overrides",
|
||||
"packageManager",
|
||||
"pnpm",
|
||||
]);
|
||||
});
|
||||
|
||||
it("accepts only security-member override commands for the current head sha", () => {
|
||||
const comments = [
|
||||
{
|
||||
body: "/allow-dependencies-change not enough",
|
||||
created_at: "2026-05-28T20:00:00Z",
|
||||
user: { login: "not-security" },
|
||||
},
|
||||
{
|
||||
body: "/allow-dependencies-change stale approval",
|
||||
created_at: "2026-05-28T20:01:00Z",
|
||||
user: { login: "security-user" },
|
||||
},
|
||||
{
|
||||
body: "/allow-dependencies-change reviewed dependency graph",
|
||||
created_at: "2026-05-28T20:03:00Z",
|
||||
html_url: "https://example.test/comment",
|
||||
user: { login: "security-user" },
|
||||
},
|
||||
];
|
||||
|
||||
const override = findDependencyOverrideCommand({
|
||||
comments,
|
||||
expectedSha: headSha,
|
||||
isSecurityMember: (login) => login === "security-user",
|
||||
newerThan: "2026-05-28T20:02:00Z",
|
||||
});
|
||||
|
||||
expect(override).toEqual({
|
||||
login: "security-user",
|
||||
reason: "reviewed dependency graph",
|
||||
sha: headSha,
|
||||
url: "https://example.test/comment",
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects stale or non-security override commands", async () => {
|
||||
const comments = [
|
||||
{
|
||||
body: "/allow-dependencies-change stale approval",
|
||||
created_at: "2026-05-28T20:00:00Z",
|
||||
user: { login: "security-user" },
|
||||
},
|
||||
{
|
||||
body: "/allow-dependencies-change not enough",
|
||||
created_at: "2026-05-28T20:02:00Z",
|
||||
user: { login: "not-security" },
|
||||
},
|
||||
];
|
||||
|
||||
await expect(
|
||||
findDependencyOverrideCommandAsync({
|
||||
comments,
|
||||
expectedSha: headSha,
|
||||
isSecurityMember: async (login) => login === "security-user",
|
||||
newerThan: "2026-05-28T20:01:00Z",
|
||||
}),
|
||||
).resolves.toBeNull();
|
||||
});
|
||||
|
||||
it("rejects override commands without a freshness barrier", () => {
|
||||
const override = findDependencyOverrideCommand({
|
||||
comments: [
|
||||
{
|
||||
body: "/allow-dependencies-change",
|
||||
created_at: "2026-05-28T20:03:00Z",
|
||||
user: { login: "security-user" },
|
||||
},
|
||||
],
|
||||
expectedSha: headSha,
|
||||
isSecurityMember: (login) => login === "security-user",
|
||||
});
|
||||
|
||||
expect(override).toBeNull();
|
||||
});
|
||||
|
||||
it("accepts override commands without a reason", () => {
|
||||
const override = findDependencyOverrideCommand({
|
||||
comments: [
|
||||
{
|
||||
body: "/allow-dependencies-change",
|
||||
created_at: "2026-05-28T20:03:00Z",
|
||||
user: { login: "security-user" },
|
||||
},
|
||||
],
|
||||
expectedSha: headSha,
|
||||
isSecurityMember: (login) => login === "security-user",
|
||||
newerThan: "2026-05-28T20:02:00Z",
|
||||
});
|
||||
|
||||
expect(override).toEqual({
|
||||
login: "security-user",
|
||||
reason: null,
|
||||
sha: headSha,
|
||||
url: undefined,
|
||||
});
|
||||
});
|
||||
|
||||
it("binds override commands to the head sha in the blocked guard comment", () => {
|
||||
const blockedComment = {
|
||||
body: renderBlockedDependencyComment({
|
||||
baseBranch: "main",
|
||||
headSha,
|
||||
lockfileChanges: ["pnpm-lock.yaml"],
|
||||
dependencyManifestChanges: [],
|
||||
}),
|
||||
};
|
||||
const staleBlockedComment = {
|
||||
body: renderBlockedDependencyComment({
|
||||
baseBranch: "main",
|
||||
headSha: staleSha,
|
||||
lockfileChanges: ["pnpm-lock.yaml"],
|
||||
dependencyManifestChanges: [],
|
||||
}),
|
||||
};
|
||||
|
||||
expect(dependencyGuardCommentHeadSha(blockedComment)).toBe(headSha);
|
||||
expect(dependencyOverrideExpectedSha(blockedComment, headSha)).toBe(headSha);
|
||||
expect(dependencyOverrideExpectedSha(staleBlockedComment, headSha)).toBeNull();
|
||||
});
|
||||
|
||||
it("preserves same-head authorization across reruns", () => {
|
||||
const authorizedComment = {
|
||||
body: renderAuthorizedDependencyComment({
|
||||
login: "security-user",
|
||||
reason: null,
|
||||
sha: headSha,
|
||||
}),
|
||||
};
|
||||
|
||||
expect(dependencyGuardCommentHeadSha(authorizedComment)).toBe(headSha);
|
||||
expect(isDependencyGuardAuthorizedForHead(authorizedComment, headSha)).toBe(true);
|
||||
expect(isDependencyGuardAuthorizedForHead(authorizedComment, staleSha)).toBe(false);
|
||||
expect(dependencyOverrideExpectedSha(authorizedComment, headSha)).toBeNull();
|
||||
});
|
||||
|
||||
it("renders deterministic removal guidance for blocked lockfile changes", () => {
|
||||
const body = renderBlockedDependencyComment({
|
||||
baseBranch: "main",
|
||||
headSha,
|
||||
lockfileChanges: ["pnpm-lock.yaml", "extensions/slack/npm-shrinkwrap.json"],
|
||||
dependencyManifestChanges: [
|
||||
{
|
||||
path: "package.json",
|
||||
fields: ["dependencies"],
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
expect(body).toContain("<!-- openclaw:dependency-graph-guard -->");
|
||||
expect(body).toContain("Dependency graph changes are blocked");
|
||||
expect(body).toContain("`pnpm-lock.yaml` changed.");
|
||||
expect(body).toContain("`extensions/slack/npm-shrinkwrap.json` changed.");
|
||||
expect(body).toContain("`package.json` changed `dependencies`.");
|
||||
expect(body).toContain(
|
||||
"git checkout 'origin/main' -- 'pnpm-lock.yaml' 'extensions/slack/npm-shrinkwrap.json'",
|
||||
);
|
||||
expect(body).toContain("/allow-dependencies-change");
|
||||
expect(body).toContain(`current head SHA (\`${headSha}\`)`);
|
||||
expect(body).toContain("A later push requires a fresh approval.");
|
||||
});
|
||||
|
||||
it("shell-quotes PR-controlled paths in removal guidance", () => {
|
||||
const body = renderBlockedDependencyComment({
|
||||
baseBranch: "release/canary branch",
|
||||
headSha,
|
||||
lockfileChanges: [
|
||||
"dir with spaces/pnpm-lock.yaml",
|
||||
"safe/quote'$(touch bad);/package-lock.json",
|
||||
],
|
||||
dependencyManifestChanges: [],
|
||||
});
|
||||
|
||||
expect(body).toContain(
|
||||
"git checkout 'origin/release/canary branch' -- 'dir with spaces/pnpm-lock.yaml' 'safe/quote'\\''$(touch bad);/package-lock.json'",
|
||||
);
|
||||
});
|
||||
|
||||
it("renders a cleared guard comment that preserves approval freshness", () => {
|
||||
const body = renderClearedDependencyGuardComment({ headSha });
|
||||
|
||||
expect(body).toContain("<!-- openclaw:dependency-graph-guard -->");
|
||||
expect(body).toContain("Dependency graph guard cleared");
|
||||
expect(body).toContain(headSha);
|
||||
expect(body).toContain("requires a fresh `/allow-dependencies-change` comment");
|
||||
});
|
||||
|
||||
it("parses explicit security approver allowlists", () => {
|
||||
expect(securityApproverSet("vincentkoc, steipete\njoshavant")).toEqual(
|
||||
new Set(["vincentkoc", "steipete", "joshavant"]),
|
||||
);
|
||||
});
|
||||
|
||||
it("sanitizes display values", () => {
|
||||
expect(sanitizeDisplayValue("abc\u0000def")).toBe("abc?def");
|
||||
expect(sanitizeDisplayValue("x".repeat(300))).toHaveLength(240);
|
||||
});
|
||||
});
|
||||
@@ -31,23 +31,22 @@ describe("dependency change awareness workflow", () => {
|
||||
const parsed = readWorkflow();
|
||||
|
||||
expect(workflow).toContain("pull_request_target:");
|
||||
expect(workflow).toContain("metadata-only workflow; no checkout or untrusted code execution");
|
||||
expect(workflow).toContain("checks trusted base script only; never checks out PR head");
|
||||
expect(parsed.permissions).toEqual({
|
||||
contents: "read",
|
||||
"pull-requests": "write",
|
||||
issues: "write",
|
||||
});
|
||||
});
|
||||
|
||||
it("does not checkout or execute PR-controlled code", () => {
|
||||
it("checks out only trusted base scripts and does not execute PR-controlled code", () => {
|
||||
const workflow = readFileSync(WORKFLOW, "utf8");
|
||||
const forbiddenSnippets = [
|
||||
"actions/checkout",
|
||||
"github.event.pull_request.head",
|
||||
"pullRequest.head",
|
||||
"pnpm install",
|
||||
"npm install",
|
||||
"pnpm dlx",
|
||||
"contents: write",
|
||||
"actions: write",
|
||||
"id-token: write",
|
||||
"secrets.",
|
||||
@@ -59,44 +58,53 @@ describe("dependency change awareness workflow", () => {
|
||||
}
|
||||
|
||||
const steps = readWorkflow().jobs?.["dependency-change-awareness"]?.steps ?? [];
|
||||
expect(steps).toHaveLength(1);
|
||||
expect(steps[0].run).toBeUndefined();
|
||||
expect(steps).toHaveLength(2);
|
||||
expect(steps[0].uses).toBe("actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd");
|
||||
expect(steps[0].with?.ref).toBe("${{ github.event.pull_request.base.sha }}");
|
||||
expect(steps[0].with?.["persist-credentials"]).toBe(false);
|
||||
expect(steps[1].run).toBe("node scripts/github/dependency-change-awareness.mjs");
|
||||
});
|
||||
|
||||
it("uses a pinned GitHub Script action and bounded sticky comments", () => {
|
||||
it("uses a dedicated checked-in script and bounded sticky comments", () => {
|
||||
const workflow = readFileSync(WORKFLOW, "utf8");
|
||||
const steps = readWorkflow().jobs?.["dependency-change-awareness"]?.steps ?? [];
|
||||
const step = steps[0];
|
||||
const runStep = steps[1];
|
||||
const script = readFileSync("scripts/github/dependency-change-awareness.mjs", "utf8");
|
||||
|
||||
expect(step.uses).toBe("actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3");
|
||||
expect(step.with?.script).toContain("<!-- openclaw:dependency-change-awareness -->");
|
||||
expect(step.with?.script).toContain("const maxListedFiles = 25;");
|
||||
expect(step.with?.script).toContain("const sanitizeDisplayValue = (value)");
|
||||
expect(step.with?.script).toContain('.replace(/[\\u0000-\\u001f\\u007f]/gu, "?")');
|
||||
expect(step.with?.script).toContain(".slice(0, 240)");
|
||||
expect(step.with?.script).toContain('comment.user?.login === "github-actions[bot]"');
|
||||
expect(step.with?.script).toContain("github.rest.pulls.listFiles");
|
||||
expect(step.with?.script).toContain("github.rest.issues.createComment");
|
||||
expect(step.with?.script).toContain("github.rest.issues.updateComment");
|
||||
expect(step.with?.script).toContain("github.rest.issues.deleteComment");
|
||||
expect(step.with?.script).toContain("ignoreUnavailableWritePermission");
|
||||
expect(step.with?.script).toContain("error?.status === 403");
|
||||
expect(workflow).toContain('"dependencies-changed"');
|
||||
expect(runStep.env?.OPENCLAW_SECURITY_TEAM_SLUG).toBe("openclaw-secops");
|
||||
expect(runStep.env?.OPENCLAW_SECURITY_APPROVERS).toBe("vincentkoc,steipete,joshavant");
|
||||
expect(workflow).toContain("scripts/github/dependency-change-awareness.mjs");
|
||||
expect(script).toContain('"dependencies-changed"');
|
||||
expect(script).not.toContain('"blocked: dependencies"');
|
||||
});
|
||||
|
||||
it("detects the intended dependency-related file surfaces", () => {
|
||||
const script = readWorkflow().jobs?.["dependency-change-awareness"]?.steps?.[0].with?.script;
|
||||
expect(script).toContain('filename === "package.json"');
|
||||
expect(script).toContain('filename === "package-lock.json"');
|
||||
expect(script).toContain('filename === "npm-shrinkwrap.json"');
|
||||
expect(script).toContain('filename === "pnpm-lock.yaml"');
|
||||
const script = readFileSync("scripts/github/dependency-change-awareness.mjs", "utf8");
|
||||
expect(script).toContain('filename.endsWith("package.json")');
|
||||
expect(script).toContain('filename.endsWith("package-lock.json")');
|
||||
expect(script).toContain('filename.endsWith("npm-shrinkwrap.json")');
|
||||
expect(script).toContain('filename.endsWith("pnpm-lock.yaml")');
|
||||
expect(script).toContain('filename === "pnpm-workspace.yaml"');
|
||||
expect(script).toContain('filename === "ui/package.json"');
|
||||
expect(script).toContain('filename.startsWith("patches/")');
|
||||
expect(script).toContain("^packages\\/[^/]+\\/package\\.json$");
|
||||
expect(script).toContain("^extensions\\/[^/]+\\/package-lock\\.json$");
|
||||
expect(script).toContain("^extensions\\/[^/]+\\/npm-shrinkwrap\\.json$");
|
||||
expect(script).toContain("^extensions\\/[^/]+\\/package\\.json$");
|
||||
expect(script).toContain("dependencyGraphFiles");
|
||||
});
|
||||
|
||||
it("blocks package lockfile and manifest graph changes unless secops approves the current head sha", () => {
|
||||
const script = readFileSync("scripts/github/dependency-change-awareness.mjs", "utf8");
|
||||
expect(script).toContain('filename.endsWith("pnpm-lock.yaml")');
|
||||
expect(script).toContain('filename.endsWith("package-lock.json")');
|
||||
expect(script).toContain('filename.endsWith("npm-shrinkwrap.json")');
|
||||
expect(script).toContain('"optionalDependencies"');
|
||||
expect(script).toContain('"peerDependencies"');
|
||||
expect(script).toContain('"overrides"');
|
||||
expect(script).toContain('"packageManager"');
|
||||
expect(script).toContain("/allow-dependencies-change");
|
||||
expect(script).toContain("openclaw-secops");
|
||||
expect(script).toContain("securityApproverSet");
|
||||
expect(script).toContain("/memberships/");
|
||||
expect(script).toContain("isCommentNewerThan");
|
||||
expect(script).toContain("A later push requires a fresh approval.");
|
||||
expect(script).toContain("process.exitCode = 1");
|
||||
});
|
||||
|
||||
it("requires secops review for future workflow or guard changes", () => {
|
||||
@@ -107,6 +115,9 @@ describe("dependency change awareness workflow", () => {
|
||||
expect(codeowners).toContain(
|
||||
"/test/scripts/dependency-change-awareness-workflow.test.ts @openclaw/openclaw-secops",
|
||||
);
|
||||
expect(codeowners).toContain(
|
||||
"/scripts/github/dependency-change-awareness.mjs @openclaw/openclaw-secops",
|
||||
);
|
||||
expect(codeowners).toContain("/package-lock.json @openclaw/openclaw-secops");
|
||||
expect(codeowners).toContain("/npm-shrinkwrap.json @openclaw/openclaw-secops");
|
||||
expect(codeowners).toContain("/extensions/*/package-lock.json @openclaw/openclaw-secops");
|
||||
|
||||
Reference in New Issue
Block a user