fix(ci): stop blocking dependency-only removals (#117172)

* fix(ci): allow dependency removals without approval

* fix(ci): align dependency guard declarations
This commit is contained in:
Patrick Erichsen
2026-08-01 13:58:56 -07:00
committed by GitHub
parent 4b2d78a2ef
commit def80bff90
4 changed files with 105 additions and 0 deletions
+12
View File
@@ -17,6 +17,11 @@ type PullRequest = {
};
type ActorCandidate = { login: string; source: string };
type DependencyGraphChange = {
change_type: string;
manifest?: string | null;
name?: string | null;
};
export function isDependencyFile(filename: string): boolean;
export function isDependencyManifest(filename: string): boolean;
@@ -25,6 +30,9 @@ export function dependencyFieldChanges(
baseManifest: Record<string, unknown>,
headManifest: Record<string, unknown>,
): string[];
export function isRemovalOnlyDependencyGraphChange(
changes: readonly DependencyGraphChange[],
): boolean;
export function shouldAutoscrubDependencyLockfiles(options: {
dependencyFiles?: string[];
lockfileChanges: unknown[];
@@ -76,6 +84,10 @@ export function renderTrustedDependencyComment(options: {
actor: { login: string; reason: string };
headSha: string;
}): string;
export function renderRemovalOnlyDependencyComment(options: {
dependencyGraphChanges: readonly DependencyGraphChange[];
headSha?: string;
}): string;
export function renderAutoscrubbedDependencyComment(options: {
baseBranch: string;
lockfileChanges: string[];
+45
View File
@@ -82,6 +82,10 @@ export function dependencyFieldChanges(baseManifest, headManifest) {
return changes;
}
export function isRemovalOnlyDependencyGraphChange(changes) {
return changes.length > 0 && changes.every((change) => change.change_type === "removed");
}
export function shouldAutoscrubDependencyLockfiles({
dependencyFiles = [],
lockfileChanges,
@@ -308,6 +312,26 @@ export function renderTrustedDependencyComment({ actor, headSha }) {
].join("\n");
}
export function renderRemovalOnlyDependencyComment({ dependencyGraphChanges, headSha }) {
const removalLines = dependencyGraphChanges.map(
(change) =>
`- Removed ${markdownCode(change.name ?? "<unknown dependency>")} from ${markdownCode(change.manifest ?? "<unknown manifest>")}.`,
);
return [
dependencyGraphGuardMarker,
"",
"### Dependency removals noted",
"",
"This PR only removes dependencies from the dependency graph, so the dependency guard is informational and does not require `/allow-dependencies-change`.",
"",
...removalLines,
"",
`- Current SHA: ${markdownCode(headSha ?? "<head-sha>")}`,
"",
"A later push that adds or changes dependency graph entries will require a fresh security approval.",
].join("\n");
}
export function renderAutoscrubbedDependencyComment({ baseBranch, lockfileChanges, commitSha }) {
const safeBranch = sanitizeDisplayValue(baseBranch ?? "main");
const fileLines = lockfileChanges.map((path) => `- ${markdownCode(path)}`);
@@ -725,6 +749,27 @@ async function main() {
return;
}
const dependencyGraphChanges = await api.paginate(
`/repos/${owner}/${repo}/dependency-graph/compare/${pullRequest.base?.sha}...${pullRequest.head?.sha}`,
);
if (isRemovalOnlyDependencyGraphChange(dependencyGraphChanges)) {
if (mode === "detect") {
await setOutput("autoscrub", "false");
}
await upsertComment(
existingGuardComment,
renderRemovalOnlyDependencyComment({
dependencyGraphChanges,
headSha: pullRequest.head?.sha,
}),
);
await writeSummary(
"## Dependency Guard\n\nDependency removals are informational and do not require security approval.",
);
console.log("Dependency removals detected; guard is informational.");
return;
}
const { isSecurityMember, isRepositoryAdmin } = createGuardApproverChecks({
api,
owner,
@@ -21,11 +21,13 @@ import {
isDependencyManifest,
isDependencyGuardTrustedForHead,
isPackageLockfile,
isRemovalOnlyDependencyGraphChange,
readBoundedGitHubErrorText,
renderAuthorizedDependencyComment,
renderAutoscrubbedDependencyComment,
renderBlockedDependencyComment,
renderClearedDependencyGuardComment,
renderRemovalOnlyDependencyComment,
renderTrustedDependencyComment,
sanitizeDisplayValue,
securityApproverSet,
@@ -93,6 +95,43 @@ describe("dependency guard script", () => {
).toEqual(["optionalDependencies", "peerDependencies", "overrides", "packageManager", "pnpm"]);
});
it("allows only dependency graph removals without approval", () => {
expect(
isRemovalOnlyDependencyGraphChange([
{ change_type: "removed", name: "a" },
{ change_type: "removed", name: "b" },
]),
).toBe(true);
expect(
isRemovalOnlyDependencyGraphChange([
{ change_type: "removed", name: "a" },
{ change_type: "added", name: "b" },
]),
).toBe(false);
expect(isRemovalOnlyDependencyGraphChange([{ change_type: "changed", name: "a" }])).toBe(false);
expect(isRemovalOnlyDependencyGraphChange([])).toBe(false);
});
it("renders dependency removals as informational", () => {
const body = renderRemovalOnlyDependencyComment({
dependencyGraphChanges: [
{
change_type: "removed",
manifest: "extensions/example/package.json",
name: "example-dependency",
},
],
headSha,
});
expect(body).toContain("Dependency removals noted");
expect(body).toContain("does not require `/allow-dependencies-change`");
expect(body).toContain("Removed `example-dependency`");
expect(body).toContain("`extensions/example/package.json`");
expect(body).toContain(headSha);
expect(body).not.toContain("changes are blocked");
});
it("accepts only security-member override commands for the current head sha", () => {
const comments = [
{
@@ -221,6 +221,15 @@ describe("dependency guard workflow", () => {
expect(script).toContain("process.exitCode = 1");
});
it("keeps removal-only dependency changes informational", () => {
const script = readFileSync("scripts/github/dependency-guard.mjs", "utf8");
expect(script).toContain("isRemovalOnlyDependencyGraphChange");
expect(script).toContain("Dependency removals detected; guard is informational.");
expect(script).toContain("/dependency-graph/compare/");
expect(script).toContain('change.change_type === "removed"');
});
it("cleans dependency label and guard comment after successful autoscrub", () => {
const script = readFileSync("scripts/github/dependency-guard.mjs", "utf8");
const autoscrubCommitIndex = script.indexOf("const commit = await createAutoscrubCommit");