diff --git a/scripts/dependabot-alerts.mjs b/scripts/dependabot-alerts.mjs index a8fce781..1dd384b9 100644 --- a/scripts/dependabot-alerts.mjs +++ b/scripts/dependabot-alerts.mjs @@ -435,6 +435,7 @@ export function remediation(affected, declared) { */ export function buildIssueBody(group, { affected, declared, ghsas }) { const covered = ghsas ?? group.ghsas; + const applying = group.advisories.length; // ⚠️ Every free-form cell is escaped, the RANGE included: a semver range is // allowed to contain `||`, so a disjoint advisory range like // `>= 1.0, < 2.0 || >= 3.0, < 3.5` would otherwise inject two extra column @@ -478,7 +479,10 @@ export function buildIssueBody(group, { affected, declared, ghsas }) { return [ buildMarker({ ...group, ghsas: covered }), - `Filed automatically from ${covered.length} open Dependabot ${covered.length === 1 ? "alert" : "alerts"} (#2233). Dependabot opens no security-update PRs on this repo; the fix is written by hand against \`v2/main\`.`, + // Counts what APPLIES, like the title and the table — `covered` is the + // marker's monotonic history and would keep counting an advisory that has + // since closed (Copilot). + `Filed automatically from ${applying} open Dependabot ${applying === 1 ? "alert" : "alerts"} (#2233). Dependabot opens no security-update PRs on this repo; the fix is written by hand against \`${TARGET_BRANCH}\`.`, "", "| | |", "| --- | --- |", @@ -504,6 +508,32 @@ export function buildIssueBody(group, { affected, declared, ghsas }) { ].join("\n"); } +/** + * The body an issue is rewritten to once its exposure is gone. + * + * The marker is retained, so the sweep still recognises this issue and will not + * file a fresh one if the same advisory comes back into range. The issue is NOT + * closed automatically: whether the exposure went away because a PR fixed it or + * because the dependency was dropped decides whether the board card is moved to + * Done or deleted, and that is a judgement the sweep cannot make. + * + * @param {ReturnType[number]} group + * @param {{ghsas: string[], reason: string, today: string}} context + * @returns {string} + */ +export function buildClearedBody(group, { ghsas, reason, today }) { + return [ + buildMarker({ ...group, ghsas }), + `**No longer applicable on \`${TARGET_BRANCH}\` as of ${today}** — ${reason}.`, + "", + `Nothing here needs bumping any more: \`${group.package}\` is no longer exposed to ${ghsas.length === 1 ? "the advisory" : "the advisories"} below on the branch we ship from. This body is rewritten in place rather than the issue being closed, because whether the card belongs in **Done** or should be **deleted** depends on why the exposure went away — a merged fix shipped something, a dropped dependency did not.`, + "", + `Previously covered: ${ghsas.map((g) => `\`${g}\``).join(", ")}.`, + "", + "If the same advisory comes back into range, this issue is reused rather than a new one filed.", + ].join("\n"); +} + /** * The marker that makes a "new advisories" comment idempotent on its own. * @@ -883,7 +913,11 @@ function createIssue(repo, group, body, spawn) { return { url, milestone }; } -export function main(repo = process.env.GITHUB_REPOSITORY, spawn = spawnSync) { +export function main( + repo = process.env.GITHUB_REPOSITORY, + spawn = spawnSync, + today = new Date().toISOString().slice(0, 10), +) { if (!repo) throw new Error("repo not specified (GITHUB_REPOSITORY unset)"); checkSecurityPrsStillDisabled(repo, spawn); @@ -903,6 +937,43 @@ export function main(repo = process.env.GITHUB_REPOSITORY, spawn = spawnSync) { const boardProblems = []; for (const rawGroup of groups) { + // ⚠️ Resolved BEFORE the skips below, not after. An issue filed yesterday + // is still open today, and if the manifest has since gone or every copy has + // moved out of range, skipping straight past it leaves its body asserting a + // vulnerability that no longer exists and its Todo/High card live forever + // (Copilot). Matched on the full grouping key, `fixedIn` included: a second + // bump of the same package is a different issue, not an update to this one. + const existing = existingIssues.find( + (i) => + i.marker?.package === rawGroup.package && + i.marker?.manifestPath === rawGroup.manifestPath && + i.marker?.fixedIn === rawGroup.fixedIn, + ); + + /** Rewrite an open issue to its cleared state, once. */ + const clear = (reason) => { + if (!existing) return; + const body = buildClearedBody(rawGroup, { + ghsas: existing.marker.ghsas, + reason, + today, + }); + if (existing.body === body) return; + const edit = gh(spawn, [ + "issue", + "edit", + String(existing.number), + "--repo", + repo, + "--body", + body, + ]); + if (edit.status !== 0) { + throw new Error(`gh issue edit failed: ${(edit.stderr ?? "").trim()}`); + } + console.log(`dependabot-alerts: cleared #${existing.number} — ${reason}`); + }; + if (!manifests.has(rawGroup.manifestPath)) { manifests.set(rawGroup.manifestPath, readManifest(rawGroup.manifestPath)); } @@ -911,6 +982,7 @@ export function main(repo = process.env.GITHUB_REPOSITORY, spawn = spawnSync) { console.log( `dependabot-alerts: ${rawGroup.manifestPath} absent on ${TARGET_BRANCH} — skipping ${rawGroup.package}`, ); + clear(`\`${rawGroup.manifestPath}\` is no longer part of this repo`); continue; } @@ -921,6 +993,11 @@ export function main(repo = process.env.GITHUB_REPOSITORY, spawn = spawnSync) { console.log( `dependabot-alerts: ${rawGroup.package}@${seen.join("/") || "(absent)"} is already out of range on ${TARGET_BRANCH} — skipping`, ); + clear( + seen.length > 0 + ? `every installed copy is out of range (${seen.map((v) => `\`${v}\``).join(", ")})` + : "the package is no longer installed at all", + ); continue; } // From here on `group` carries only the advisories that apply to this @@ -928,14 +1005,6 @@ export function main(repo = process.env.GITHUB_REPOSITORY, spawn = spawnSync) { const { group, affected } = applicable; const declared = isDirectDependency(lock, group.package); - // Matched on the full grouping key, `fixedIn` included: a second bump of - // the same package is a different issue, not an update to this one. - const existing = existingIssues.find( - (i) => - i.marker?.package === group.package && - i.marker?.manifestPath === group.manifestPath && - i.marker?.fixedIn === group.fixedIn, - ); if (!existing) { const { url, milestone } = createIssue( diff --git a/scripts/dependabot-alerts.test.mjs b/scripts/dependabot-alerts.test.mjs index 59317924..85c4bac0 100644 --- a/scripts/dependabot-alerts.test.mjs +++ b/scripts/dependabot-alerts.test.mjs @@ -15,6 +15,7 @@ import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { dirname, join } from "node:path"; import { tmpdir } from "node:os"; import { + buildClearedBody, buildCommentMarker, pickMilestone, narrowToApplicable, @@ -1010,6 +1011,111 @@ test("main refreshes an issue whose exposure shrank, without commenting", () => assert.ok(log.some((l) => l.includes("refreshed #41"))); }); +test("main clears an open issue when the manifest is gone", () => { + const [group] = groupAlerts([ + alert({ ghsa: "GHSA-a", manifest: "clients/gone/package-lock.json" }), + ]); + const spawn = fakeSpawn({ + alertPages: [ + [alert({ ghsa: "GHSA-a", manifest: "clients/gone/package-lock.json" })], + ], + issues: [ + { + number: 41, + title: buildIssueTitle(group), + body: buildIssueBody(group, asInstalled()), + }, + ], + }); + const log = inTempRepo({}, () => + withoutProjectToken(() => + captureLog(() => main("o/r", spawn, "2026-09-04")), + ), + ); + + const edit = ghCall(spawn, "edit"); + assert.ok(edit, "the stale issue is rewritten, not silently skipped"); + const body = edit.args[edit.args.indexOf("--body") + 1]; + assert.match(body, /No longer applicable on `v2\/main` as of 2026-09-04/); + assert.match(body, /no longer part of this repo/); + // The marker survives, so the issue is reused if the advisory comes back. + assert.deepEqual(parseMarker(body).ghsas, ["GHSA-a"]); + assert.equal(ghCall(spawn, "create"), undefined); + assert.ok(log.some((l) => l.includes("cleared #41"))); +}); + +test("main clears an open issue when every copy moved out of range", () => { + const [group] = groupAlerts([alert({ ghsa: "GHSA-a" })]); + const spawn = fakeSpawn({ + alertPages: [[alert({ ghsa: "GHSA-a" })]], + issues: [ + { + number: 41, + title: buildIssueTitle(group), + body: buildIssueBody(group, asInstalled()), + }, + ], + }); + const log = inTempRepo( + { "package-lock.json": lockWith("fast-uri", "3.1.6") }, + () => + withoutProjectToken(() => + captureLog(() => main("o/r", spawn, "2026-09-04")), + ), + ); + const edit = ghCall(spawn, "edit"); + assert.ok(edit); + const body = edit.args[edit.args.indexOf("--body") + 1]; + assert.match(body, /every installed copy is out of range \(`3\.1\.6`\)/); + assert.ok(log.some((l) => l.includes("cleared #41"))); +}); + +test("main does not re-clear an issue it already cleared", () => { + // The second run of a cleared sweep must touch nothing at all. + const [group] = groupAlerts([alert({ ghsa: "GHSA-a" })]); + const spawn = fakeSpawn({ + alertPages: [[alert({ ghsa: "GHSA-a" })]], + issues: [ + { + number: 41, + title: buildIssueTitle(group), + body: buildClearedBody(group, { + ghsas: ["GHSA-a"], + reason: "every installed copy is out of range (`3.1.6`)", + today: "2026-09-04", + }), + }, + ], + }); + inTempRepo({ "package-lock.json": lockWith("fast-uri", "3.1.6") }, () => + withoutProjectToken(() => + captureLog(() => main("o/r", spawn, "2026-09-04")), + ), + ); + assert.equal(ghCall(spawn, "edit"), undefined); + assert.equal(ghCall(spawn, "create"), undefined); +}); + +test("main skips quietly when nothing applies and no issue is open", () => { + const spawn = fakeSpawn({ alertPages: [[alert({ ghsa: "GHSA-a" })]] }); + inTempRepo({ "package-lock.json": lockWith("fast-uri", "3.1.6") }, () => + withoutProjectToken(() => captureLog(() => main("o/r", spawn))), + ); + assert.equal(ghCall(spawn, "edit"), undefined); + assert.equal(ghCall(spawn, "create"), undefined); +}); + +test("buildIssueBody counts the applicable alerts in its prose, not the marker", () => { + const [group] = groupAlerts([alert({ ghsa: "GHSA-a" })]); + // The marker carries a second, since-closed advisory; the prose must not. + const body = buildIssueBody(group, { + ...nested(), + ghsas: ["GHSA-a", "GHSA-closed"], + }); + assert.match(body, /Filed automatically from 1 open Dependabot alert\b/); + assert.deepEqual(parseMarker(body).ghsas, ["GHSA-a", "GHSA-closed"]); +}); + test("main reads every page of open dependabot issues", () => { // The second page holds the matching marker. Truncating the lookup would // file a duplicate issue rather than recognising this one.