diff --git a/AGENTS.md b/AGENTS.md index 71288c60..34487261 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -52,7 +52,8 @@ inspector/ │ ├── react/ React hooks over the state stores (read during render — see React instructions) │ └── storage/ File I/O helpers for the OAuth persist backends ├── test-servers/ Composable MCP test servers + JSON configs -├── scripts/ Root build/verify tooling: install cascade, smokes, verify:* guards +├── scripts/ Root build/verify tooling (install cascade, smokes, verify:* guards) +│ plus repo automation run from CI (the dependency + alert sweeps) ├── docs/ Task-oriented guides ├── specification/ Design/build specifications └── .claude/skills/ The procedures (see the index above) diff --git a/README.md b/README.md index 4593d217..42ae110b 100644 --- a/README.md +++ b/README.md @@ -54,6 +54,7 @@ inspector/ ├── core/ Shared code consumed via the `@inspector/core` alias (no package.json) ├── test-servers/ Composable MCP test servers + fixtures used by integration and smoke tests ├── scripts/ Root build/verify tooling (install cascade, smokes, the verify:* guards) +│ and repo automation run from CI (the dependency and Dependabot-alert sweeps) ├── docs/ Task-oriented guides — see below ├── specification/ Design/build specifications ├── .claude/skills/ Agent skills: the repo's procedures, invokable by name diff --git a/scripts/dependabot-alerts.mjs b/scripts/dependabot-alerts.mjs index 480d2174..8f3159f0 100644 --- a/scripts/dependabot-alerts.mjs +++ b/scripts/dependabot-alerts.mjs @@ -79,6 +79,19 @@ export const BOARD_PRIORITY = "High"; */ export const TARGET_BRANCH = "v2/main"; +/** + * The one ecosystem this sweep can act on. + * + * ⚠️ Dependabot alerts are NOT npm-only. This repo has a `Dockerfile` and + * GitHub Actions workflows, and an alert against either arrives in the same + * feed with a `manifest_path` that is not a lockfile — which the JSON parse + * would reject, aborting the whole daily sweep before any npm group was + * processed (Copilot). Everything downstream reads npm lockfiles, so a non-npm + * alert is reported and skipped rather than guessed at: filing it properly + * means knowing how to fix it, which is different work per ecosystem. + */ +export const SUPPORTED_ECOSYSTEM = "npm"; + const MARKER_RE = /^/; @@ -267,6 +280,7 @@ export function groupAlerts(alerts) { for (const alert of alerts) { if (alert.state !== "open") continue; const pkg = alert.dependency?.package?.name; + const ecosystem = alert.dependency?.package?.ecosystem ?? "unknown"; const manifestPath = alert.dependency?.manifest_path; const fixedIn = alert.security_vulnerability?.first_patched_version?.identifier; @@ -298,6 +312,7 @@ export function groupAlerts(alerts) { groups.set(key, { key, package: pkg, + ecosystem, manifestPath, fixedIn, scope: alert.dependency?.scope ?? "runtime", @@ -485,7 +500,10 @@ export function remediation(affected, declared) { * rewritten to cover advisories it did not originally name. * @returns {string} */ -export function buildIssueBody(group, { affected, declared, ghsas }) { +export function buildIssueBody( + group, + { affected, declared, ghsas, securityPrsOff = true }, +) { const covered = ghsas ?? group.ghsas; const applying = group.advisories.length; // ⚠️ Every free-form cell is escaped, the RANGE included: a semver range is @@ -540,7 +558,11 @@ export function buildIssueBody(group, { affected, declared, ghsas }) { // 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}\`.`, + // ⚠️ The security-PR claim is only made when the run actually READ the + // setting. The guard degrades to UNVERIFIED when the token cannot see it, + // and an issue asserting what the run explicitly could not confirm is worse + // than one that says so (Copilot). + `Filed automatically from ${applying} open Dependabot ${applying === 1 ? "alert" : "alerts"} (#2233). ${securityPrsOff ? "Dependabot opens no security-update PRs on this repo; the" : "The"} fix is written by hand against \`${TARGET_BRANCH}\`.`, "", "| | |", "| --- | --- |", @@ -750,12 +772,23 @@ function openAlerts(repo, spawn) { * against a manifest this branch does not have is not actionable. */ function readManifest(manifestPath) { + let raw; try { - return JSON.parse(readFileSync(manifestPath, "utf8")); + raw = readFileSync(manifestPath, "utf8"); } catch (error) { if (error.code === "ENOENT") return null; throw error; } + try { + return JSON.parse(raw); + } catch { + // Belt and braces behind the ecosystem filter: whatever this is, it is not + // an npm lockfile, and one unparseable manifest must not abort the sweep. + console.log( + `dependabot-alerts: ${manifestPath} is not JSON — skipping (not an npm lockfile)`, + ); + return null; + } } /** @@ -984,7 +1017,7 @@ export function main( ) { if (!repo) throw new Error("repo not specified (GITHUB_REPOSITORY unset)"); - checkSecurityPrsStillDisabled(repo, spawn); + const securityPrsOff = checkSecurityPrsStillDisabled(repo, spawn); const groups = groupAlerts(openAlerts(repo, spawn)); @@ -1003,9 +1036,20 @@ export function main( const boardProblems = []; /** Grouping keys this run actually saw in the open feed. */ const seenKeys = new Set(); + /** Every GHSA still open, in ANY group — the check a vanished key needs. */ + const openGhsas = new Set(groups.flatMap((g) => g.ghsas)); for (const rawGroup of groups) { seenKeys.add(rawGroup.key); + + if (rawGroup.ecosystem !== SUPPORTED_ECOSYSTEM) { + // Loud, not silent: nothing else will file this, so a human has to. + console.log( + `dependabot-alerts: ${rawGroup.package} (${rawGroup.ecosystem}, ${rawGroup.manifestPath}) is not an npm dependency — this sweep cannot file it, raise it by hand: ${rawGroup.ghsas.join(", ")}`, + ); + continue; + } + // ⚠️ 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 @@ -1079,7 +1123,7 @@ export function main( const { url, milestone } = createIssue( repo, group, - buildIssueBody(group, { affected, declared }), + buildIssueBody(group, { affected, declared, securityPrsOff }), spawn, ); // `Incoming` <=> no milestone, everything past it <=> milestoned. With no @@ -1103,6 +1147,7 @@ export function main( affected, declared, ghsas: merged, + securityPrsOff, }); // ⚠️ "Nothing NEW" is not the same as "nothing CHANGED" (Copilot). An issue @@ -1177,17 +1222,25 @@ export function main( issue.marker.fixedIn, ); if (seenKeys.has(key)) continue; + + // ⚠️ A vanished KEY is not the same as a closed ADVISORY. GitHub can revise + // an alert's `first_patched_version`, which moves it to a different key + // while the GHSA stays open — reporting that as "fixed or dismissed" would + // stand down a live exposure (Copilot). So the reason is decided by whether + // the GHSAs are still in the open feed, not by the key's absence. + const stillOpen = issue.marker.ghsas.filter((g) => openGhsas.has(g)); + const reason = + stillOpen.length > 0 + ? `this bump was superseded — ${stillOpen.map((g) => `\`${g}\``).join(", ")} ${stillOpen.length === 1 ? "is" : "are"} still open under a different patched version, and ${stillOpen.length === 1 ? "has" : "have"} their own issue` + : "every alert it tracked has been fixed or dismissed"; + const body = buildClearedBody( { package: issue.marker.package, manifestPath: issue.marker.manifestPath, fixedIn: issue.marker.fixedIn, }, - { - ghsas: issue.marker.ghsas, - reason: "every alert it tracked has been fixed or dismissed", - today, - }, + { ghsas: issue.marker.ghsas, reason, today }, ); if (issue.body === body) continue; const edit = gh(spawn, [ diff --git a/scripts/dependabot-alerts.test.mjs b/scripts/dependabot-alerts.test.mjs index 7f334879..f8325b22 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 { + SUPPORTED_ECOSYSTEM, PRIORITY_FIELD_ID, STATUS_FIELD_ID, buildClearedBody, @@ -51,12 +52,13 @@ function alert({ scope = "runtime", cve = null, state = "open", + ecosystem = "npm", }) { return { state, html_url: `https://github.com/o/r/security/dependabot/${ghsa}`, dependency: { - package: { name: pkg }, + package: { name: pkg, ecosystem }, manifest_path: manifest, scope, }, @@ -1295,6 +1297,124 @@ test("scopedOverrideExample nests each vulnerable copy under its parents", () => }); }); +test("groupAlerts records the ecosystem so non-npm alerts are identifiable", () => { + const [docker] = groupAlerts([ + alert({ + ghsa: "GHSA-d", + pkg: "node", + manifest: "Dockerfile", + ecosystem: "docker", + }), + ]); + assert.equal(docker.ecosystem, "docker"); + assert.notEqual(docker.ecosystem, SUPPORTED_ECOSYSTEM); +}); + +test("main skips a non-npm alert loudly instead of crashing on its manifest", () => { + // ⚠️ This repo has a Dockerfile, so this is reachable. Parsing it as a + // lockfile threw and aborted the entire sweep before any npm group ran. + const spawn = fakeSpawn({ + alertPages: [ + [ + alert({ + ghsa: "GHSA-docker", + pkg: "node", + manifest: "Dockerfile", + ecosystem: "docker", + }), + alert({ ghsa: "GHSA-a" }), + ], + ], + }); + const log = inTempRepo( + { + "package-lock.json": lockWith("fast-uri", "3.1.5"), + }, + () => withoutProjectToken(() => captureLog(() => main("o/r", spawn))), + ); + + // The npm bump is still filed — the non-npm alert must not abort the run. + const created = ghCalls(spawn, "create"); + assert.equal(created.length, 1); + assert.match( + created[0].args[created[0].args.indexOf("--title") + 1], + /`fast-uri`/, + ); + // ...and the skipped one is named, with its GHSA, so a human can file it. + assert.ok( + log.some( + (l) => + l.includes("docker") && + l.includes("Dockerfile") && + l.includes("GHSA-docker") && + l.includes("raise it by hand"), + ), + `expected a loud skip line, got: ${log.join(" | ")}`, + ); +}); + +test("main says superseded, not fixed, when the GHSA is still open elsewhere", () => { + // GitHub revised `first_patched_version`, so the advisory moved to a new key + // while staying open. Calling that "fixed or dismissed" would stand down a + // live exposure. + const [filed] = groupAlerts([alert({ ghsa: "GHSA-a", fixed: "3.1.6" })]); + const spawn = fakeSpawn({ + alertPages: [[alert({ ghsa: "GHSA-a", fixed: "3.1.7", range: "< 3.1.7" })]], + issues: [ + { + number: 41, + title: buildIssueTitle(filed), + body: buildIssueBody(filed, asInstalled()), + }, + ], + }); + inTempRepo({ "package-lock.json": lockWith("fast-uri", "3.1.5") }, () => + 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, /superseded/); + assert.match(body, /`GHSA-a` is still open/); + assert.doesNotMatch(body, /fixed or dismissed/); + // ...and the new bump gets its own issue. + assert.ok(ghCall(spawn, "create")); +}); + +test("buildIssueBody does not claim security PRs are off when unverified", () => { + const [group] = groupAlerts([alert({ ghsa: "GHSA-a" })]); + const verified = buildIssueBody(group, nested()); + assert.match(verified, /Dependabot opens no security-update PRs/); + + const unverified = buildIssueBody(group, { + ...nested(), + securityPrsOff: false, + }); + assert.doesNotMatch(unverified, /opens no security-update PRs/); + assert.match(unverified, /The fix is written by hand/); +}); + +test("main omits the security-PR claim when the token could not read it", () => { + const spawn = fakeSpawn({ + securityFixesStatus: 1, + securityFixesStderr: "gh: HTTP 403: Resource not accessible by integration", + alertPages: [[alert({ ghsa: "GHSA-a" })]], + }); + const log = inTempRepo( + { "package-lock.json": lockWith("fast-uri", "3.1.5") }, + () => withoutProjectToken(() => captureLog(() => main("o/r", spawn))), + ); + assert.ok(log.some((l) => l.includes("UNVERIFIED"))); + const create = ghCall(spawn, "create"); + assert.ok(create); + assert.doesNotMatch( + create.args[create.args.indexOf("--body") + 1], + /opens no security-update PRs/, + ); +}); + 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.