diff --git a/.github/workflows/dependency-security-scheduled.yml b/.github/workflows/dependency-security-scheduled.yml new file mode 100644 index 00000000..4147a9df --- /dev/null +++ b/.github/workflows/dependency-security-scheduled.yml @@ -0,0 +1,66 @@ +name: Dependency security (scheduled) + +# Section 2 of docs/proposed-dependency-policy.md. +# +# The pull-request audit in dependency-security.yml only ever looks at a branch +# somebody pushed. An advisory published against unchanged code is nobody's +# pull request, so nothing on that trigger can find it: on 2026-09-15 +# RUSTSEC-2026-0285 was published against `rustls` and went unnoticed until the +# next person to open a pull request found the whole pipeline frozen. This job +# looks at the default branch on a clock instead, and puts what it finds where +# a person will see it. +on: + schedule: + - cron: "17 6 * * *" # daily; off the hour, to dodge the scheduler pile-up + workflow_dispatch: + +permissions: + contents: read + issues: write # the one permission beyond the pull-request audit's + +concurrency: + # Deliberately NOT cancel-in-progress, unlike the sibling audit workflow. + # That one only reads; this one writes to an issue, in two steps -- edit then + # comment, or close then comment. Cancelling between them leaves the tracking + # issue half-updated: a body naming a new finding with nothing saying so, or a + # closed issue with no explanation. The job is bounded by timeout-minutes and + # has only two triggers, so letting a second run queue costs little. + group: dependency-security-scheduled + cancel-in-progress: false + +jobs: + audit-master: + name: Audit master and track findings in one issue + runs-on: ubuntu-latest + timeout-minutes: 15 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + with: + persist-credentials: false + - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v6 + with: + node-version-file: .node-version + - uses: taiki-e/install-action@fa23953489c080190314742a9b907f8e97c6767c # v2.87.10 + with: + tool: cargo-audit@0.22.2 + fallback: none + # `|| true` because cargo audit exits non-zero when it FINDS something, + # which here is the case we want to keep going for. It also swallows a + # genuine failure (no network, advisory db unreachable), so the next step + # exists to stop that being mistaken for a clean result. + - name: Audit the default branch + run: cargo audit --file src-tauri/Cargo.lock --json > audit-report.json || true + # Without this, an audit that failed to run writes an empty file, the + # tracker reads "no findings", and it CLOSES a standing advisory's + # tracking issue. Silence about a broken audit is the one outcome worse + # than a noisy one. + - name: Refuse to treat a failed audit as a clean one + run: | + if [ ! -s audit-report.json ]; then + echo "cargo audit produced no report -- treating as a failed run, not a clean one" >&2 + exit 1 + fi + - name: Open, update or close the tracking issue + env: + GH_TOKEN: ${{ github.token }} + run: node scripts/audit-tracking-issue.mjs --report audit-report.json --apply diff --git a/scripts/audit-tracking-issue.mjs b/scripts/audit-tracking-issue.mjs new file mode 100644 index 00000000..0beb706a --- /dev/null +++ b/scripts/audit-tracking-issue.mjs @@ -0,0 +1,263 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// Keeps exactly one open tracking issue in step with what `cargo audit` finds +// on the default branch, for the scheduled audit in +// `.github/workflows/dependency-security-scheduled.yml`. +// +// Why a scheduled audit needs this at all: on 2026-09-15 RUSTSEC-2026-0285 was +// published against `rustls` and nobody knew until the next person to open a +// pull request found the pipeline frozen. An advisory published against +// unchanged code is not a pull request's fault and cannot be discovered by +// anything that only runs on pull requests -- it needs a job that looks at the +// default branch on a clock. That job then has to put its finding somewhere a +// person will see, which is what this script does. +// +// The hard part is not opening an issue, it is not opening one every day. An +// advisory commonly stays unresolved for days while an upgrade is +// investigated, so "file a finding" naively becomes a daily duplicate. This +// script therefore reconciles rather than reports: one open issue per +// repository, found by a hidden marker, updated only when the finding SET +// changes, and closed when the findings are gone. A run that changes nothing +// is the normal case and must stay silent. +// +// Split deliberately into a pure planner and a thin applier: the planner is +// what the tests exercise, so the reconciliation rules are provable without a +// network, a GitHub token, or a Rust toolchain -- `pnpm test` runs every +// scripts/*.test.mjs in the "Frontend build" CI job, which has none of those. + +import { spawnSync } from "node:child_process"; +import { readFileSync } from "node:fs"; + +// A hidden marker rather than a title match: titles get edited by people, and +// a tracking issue whose title someone improved must still be found. +export const MARKER = ""; + +// The label already exists in this repository. A new one would have to be +// created as a side effect of the first run, which is a repository +// configuration change this script has no business making on its own. +const LABEL = "area:security"; + +const TITLE = "Dependency advisories on master"; + +// Mirrors `idsAndDetails` in check-advisory-delta.mjs deliberately rather than +// importing it: that module runs `main()` at import time, so importing it here +// would run a full audit as a side effect. Keep the two in step -- especially +// the synthetic key, which exists because a yanked-crate warning carries no +// advisory at all and would otherwise be invisible to a set comparison. +export function findingsFromReport(report) { + const findings = new Map(); + const record = (id, detail) => { + if (!findings.has(id)) findings.set(id, detail); + }; + for (const vulnerability of report.vulnerabilities?.list ?? []) { + record(vulnerability.advisory.id, { + id: vulnerability.advisory.id, + package: `${vulnerability.package.name}@${vulnerability.package.version}`, + summary: vulnerability.advisory.title, + kind: "vulnerability", + }); + } + for (const kind of Object.keys(report.warnings ?? {})) { + for (const warning of report.warnings[kind] ?? []) { + const id = + warning.advisory?.id ?? `${kind}:${warning.package.name}@${warning.package.version}`; + record(id, { + id, + package: `${warning.package.name}@${warning.package.version}`, + summary: warning.advisory?.title ?? kind, + kind, + }); + } + } + return [...findings.values()].sort((a, b) => a.id.localeCompare(b.id)); +} + +// The recorded set is written into the issue body on its own line so a later +// run can read back exactly what the last run saw. Comparing against the +// rendered prose instead would make an editorial change to the issue look +// like a change in the findings. +const RECORDED_PREFIX = "Tracked advisory ids:"; + +// JSON rather than a comma-separated list. `advisory.id` is free text from the +// community-run RustSec database, and an id containing a comma or a backtick +// round-trips wrong through a hand-rolled encoding -- it reads back as two ids, +// the set compares unequal forever, and the issue collects a "changed" comment +// every single day. That is exactly the noise this script exists to prevent, so +// the encoding may not be the thing that reintroduces it. +export function recordedIds(body) { + const line = (body ?? "") + .split("\n") + .map((entry) => entry.trim()) + .find((entry) => entry.startsWith(RECORDED_PREFIX)); + if (!line) return []; + try { + const parsed = JSON.parse(line.slice(RECORDED_PREFIX.length).trim()); + if (!Array.isArray(parsed)) return []; + return parsed.filter((id) => typeof id === "string").sort(); + } catch { + // An unreadable line is reported as no ids, which `planAction` treats as a + // changed set. That rewrites the body and repairs the line, rather than + // silently believing the issue is up to date. + return []; + } +} + +/// The whole contract, in one pure function. +/// +/// `none` and `unchanged` are distinct on purpose: both do nothing, but the +/// first means "clean, nothing tracked" and the second means "still broken, +/// already tracked". Collapsing them would hide a standing advisory behind the +/// same silence as a clean run. +export function planAction(findings, existing) { + const ids = findings.map((finding) => finding.id).sort(); + if (ids.length === 0) { + return existing ? { action: "close", issue: existing.number } : { action: "none" }; + } + if (!existing) return { action: "create", ids, findings }; + const recorded = [...(existing.ids ?? [])].sort(); + const same = recorded.length === ids.length && recorded.every((id, i) => id === ids[i]); + if (same) return { action: "unchanged", issue: existing.number, ids }; + const added = ids.filter((id) => !recorded.includes(id)); + const removed = recorded.filter((id) => !ids.includes(id)); + // A recorded line that merely repeats an id differs as a list while naming + // the same set. Rewriting the body is right; announcing a change that moved + // nothing is not. + if (added.length === 0 && removed.length === 0) { + return { action: "repair", issue: existing.number, ids, findings }; + } + return { action: "update", issue: existing.number, ids, findings, added, removed }; +} + +export function issueBody(findings) { + const lines = findings.map( + (finding) => `- \`${finding.id}\` (${finding.kind}) ${finding.package} -- ${finding.summary}`, + ); + return [ + MARKER, + "", + "`cargo audit` reports the following on the default branch, after", + "`.cargo/audit.toml`'s ignore list is applied.", + "", + ...lines, + "", + "This issue is maintained by the scheduled dependency audit. It is updated", + "when the set of findings changes and closed automatically when the", + "findings are gone. Edit the title or add comments freely.", + "", + "Three things are load-bearing for finding this issue again: the marker", + `above, the recorded-ids line below, and the \`${LABEL}\` label -- the next`, + "run looks for the marker only among open issues carrying that label.", + "Remove any of the three and the next run opens a duplicate and leaves this", + "one orphaned.", + "", + `${RECORDED_PREFIX} ${JSON.stringify(findings.map((finding) => finding.id).sort())}`, + ].join("\n"); +} + +function gh(args) { + const result = spawnSync("gh", args, { encoding: "utf8", maxBuffer: 32 * 1024 * 1024 }); + if (result.status !== 0) { + throw new Error(`audit-tracking-issue: gh ${args.join(" ")} failed: ${result.stderr?.trim()}`); + } + return result.stdout; +} + +// `run` is injected so the GitHub-touching half is testable without a token or +// a network. It is the half the reconciliation invariant actually lives in, so +// leaving it untestable would mean the tests cover the arithmetic and not the +// thing that can file a duplicate issue. +// +// `gh issue list` rather than `gh api .../issues`: the repository's +// `check-gh-api-pagination` gate exempts gh's own list subcommands because +// their paging is gh-owned, and an explicit --limit says what this one expects. +export function findExisting(run = gh) { + const issues = JSON.parse( + run([ + "issue", + "list", + "--state", + "open", + "--label", + LABEL, + "--limit", + "100", + "--json", + "number,body", + ]), + ); + const matches = issues.filter((issue) => (issue.body ?? "").includes(MARKER)); + // "Exactly one open issue" is this script's whole contract. Picking the first + // of several would honour it silently and wrongly: the others would never be + // updated and never closed, and nothing would ever say so. Refuse instead -- + // a human duplicated something, and a human should decide which survives. + if (matches.length > 1) { + throw new Error( + `audit-tracking-issue: ${matches.length} open issues carry the tracker marker ` + + `(${matches.map((issue) => `#${issue.number}`).join(", ")}). ` + + "Close all but one, then re-run -- this script will not choose for you.", + ); + } + const match = matches[0]; + return match ? { number: match.number, ids: recordedIds(match.body) } : null; +} + +export function apply(plan, run = gh) { + if (plan.action === "none" || plan.action === "unchanged") return; + if (plan.action === "create") { + run(["issue", "create", "--title", TITLE, "--label", LABEL, "--body", issueBody(plan.findings)]); + return; + } + if (plan.action === "repair") { + // The set is the same; only the recorded line was malformed. Rewrite it + // and say nothing, so a cosmetic repair does not read as a new finding. + run(["issue", "edit", String(plan.issue), "--body", issueBody(plan.findings)]); + return; + } + if (plan.action === "update") { + run(["issue", "edit", String(plan.issue), "--body", issueBody(plan.findings)]); + const changed = [ + plan.added.length ? `newly reported: ${plan.added.join(", ")}` : null, + plan.removed.length ? `no longer reported: ${plan.removed.join(", ")}` : null, + ] + .filter(Boolean) + .join("; "); + run(["issue", "comment", String(plan.issue), "--body", `Scheduled audit update -- ${changed}.`]); + return; + } + // Close first, then comment. The other order leaves a "Closing" comment on an + // issue that is still open if the close fails, and the next run recomputes + // the same plan and leaves another -- one redundant comment per day for as + // long as the failure lasts. Closing first means a failed comment leaves the + // issue in the state that was wanted, and the next run sees nothing open and + // does nothing. + run(["issue", "close", String(plan.issue)]); + run([ + "issue", + "comment", + String(plan.issue), + "--body", + "Scheduled audit reports no findings on the default branch. Closed automatically.", + ]); +} + +function main() { + const argv = process.argv.slice(2); + let reportPath = null; + let shouldApply = false; + for (let i = 0; i < argv.length; i += 1) { + if (argv[i] === "--report") reportPath = argv[++i]; + else if (argv[i] === "--apply") shouldApply = true; + else throw new Error(`audit-tracking-issue: unknown argument: ${argv[i]}`); + } + if (!reportPath) throw new Error("audit-tracking-issue: --report is required"); + + const findings = findingsFromReport(JSON.parse(readFileSync(reportPath, "utf8"))); + // Queried in both modes. A preview that assumed no issue existed could only + // ever print "create" or "none", which is not a preview of what --apply would + // do -- it is a different answer that happens to share a format. + const plan = planAction(findings, findExisting()); + process.stdout.write(`${JSON.stringify(plan, null, 2)}\n`); + if (shouldApply) apply(plan); +} + +if (process.argv[1] && process.argv[1].endsWith("audit-tracking-issue.mjs")) main(); diff --git a/scripts/audit-tracking-issue.test.mjs b/scripts/audit-tracking-issue.test.mjs new file mode 100644 index 00000000..d7a1c157 --- /dev/null +++ b/scripts/audit-tracking-issue.test.mjs @@ -0,0 +1,241 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// Contract tests for scripts/audit-tracking-issue.mjs. +// +// The failure this file exists to prevent is not "the issue is wrong". It is +// the scheduled audit filing a fresh issue every single day an advisory stays +// unresolved, which is what a reconciler does the moment it can no longer read +// back what its previous run wrote. So the load-bearing test here is the body +// round trip: `recordedIds(issueBody(findings))` must return exactly the ids +// that went in. Reformat the issue body without keeping that true and this +// suite fails rather than the repository quietly accumulating duplicates. +// +// Everything is pure -- no gh, no network, no Rust toolchain. `pnpm test` +// globs scripts/*.test.mjs in the "Frontend build" CI job, which has none. + +import assert from "node:assert/strict"; +import test from "node:test"; + +import { + MARKER, + findingsFromReport, + issueBody, + planAction, + recordedIds, +} from "./audit-tracking-issue.mjs"; + +const vuln = (id, name, version, title) => ({ + advisory: { id, title }, + package: { name, version }, +}); + +const REPORT = { + vulnerabilities: { + found: true, + list: [vuln("RUSTSEC-2026-0285", "rustls", "0.23.43", "TLS 1.3 handshake messages")], + }, + warnings: { + unmaintained: [ + { advisory: { id: "RUSTSEC-2024-0001", title: "unmaintained crate" }, package: { name: "old", version: "1.0.0" } }, + ], + // A yanked crate carries no advisory at all. Without a synthetic key it + // would be invisible to a set comparison, so a yank appearing or being + // resolved would never move the tracking issue. + yanked: [{ package: { name: "chacha20", version: "0.9.0" } }], + }, +}; + +test("a report becomes a sorted, deduplicated finding set", () => { + const findings = findingsFromReport(REPORT); + assert.deepEqual( + findings.map((finding) => finding.id), + ["RUSTSEC-2024-0001", "RUSTSEC-2026-0285", "yanked:chacha20@0.9.0"], + ); + assert.equal(findings.find((f) => f.id === "RUSTSEC-2026-0285").kind, "vulnerability"); +}); + +test("a warning with no advisory still gets a stable synthetic id", () => { + const findings = findingsFromReport({ vulnerabilities: { list: [] }, warnings: { yanked: [{ package: { name: "c", version: "1.2.3" } }] } }); + assert.deepEqual(findings.map((f) => f.id), ["yanked:c@1.2.3"]); +}); + +test("an empty report yields no findings", () => { + assert.deepEqual(findingsFromReport({ vulnerabilities: { found: false, list: [] } }), []); +}); + +// The one that matters: a body this script wrote must be readable by the next +// run. If this breaks, every scheduled run files a duplicate issue. +test("the issue body round-trips its recorded ids", () => { + const findings = findingsFromReport(REPORT); + const body = issueBody(findings); + assert.ok(body.includes(MARKER), "the body must carry the marker used to find it again"); + assert.deepEqual( + recordedIds(body), + findings.map((f) => f.id).sort(), + ); +}); + +test("a body with no recorded-ids line reads as no ids, not a crash", () => { + assert.deepEqual(recordedIds("nothing here"), []); + assert.deepEqual(recordedIds(undefined), []); +}); + +test("clean run with nothing tracked does nothing", () => { + assert.deepEqual(planAction([], null), { action: "none" }); +}); + +test("clean run with something tracked closes it", () => { + assert.deepEqual(planAction([], { number: 7, ids: ["RUSTSEC-2026-0285"] }), { + action: "close", + issue: 7, + }); +}); + +test("first finding opens an issue", () => { + const findings = findingsFromReport(REPORT); + const plan = planAction(findings, null); + assert.equal(plan.action, "create"); + assert.deepEqual(plan.ids, findings.map((f) => f.id).sort()); +}); + +test("an unchanged finding set is silent rather than a daily duplicate", () => { + const findings = findingsFromReport(REPORT); + const plan = planAction(findings, { number: 7, ids: findings.map((f) => f.id) }); + assert.equal(plan.action, "unchanged"); + assert.equal(plan.issue, 7); +}); + +test("ordering alone is not a change", () => { + const findings = findingsFromReport(REPORT); + const reversed = findings.map((f) => f.id).reverse(); + assert.equal(planAction(findings, { number: 7, ids: reversed }).action, "unchanged"); +}); + +test("a changed finding set updates and names what moved", () => { + const findings = findingsFromReport(REPORT); + const plan = planAction(findings, { number: 7, ids: ["RUSTSEC-2024-0001", "RUSTSEC-1999-0000"] }); + assert.equal(plan.action, "update"); + assert.deepEqual(plan.added, ["RUSTSEC-2026-0285", "yanked:chacha20@0.9.0"]); + assert.deepEqual(plan.removed, ["RUSTSEC-1999-0000"]); +}); + +test("a tracked issue whose ids cannot be read is treated as changed, not as clean", () => { + const findings = findingsFromReport(REPORT); + const plan = planAction(findings, { number: 7, ids: [] }); + assert.equal(plan.action, "update", "an unreadable body must not silently look up to date"); +}); + +// --- the GitHub-touching half ------------------------------------------- +// +// This is where the invariant "exactly one open issue" actually lives, and +// where every duplicate-issue path found in review lived. `run` is injected so +// these need no token and no network; a fake records the argv it was handed. + +import { apply, findExisting } from "./audit-tracking-issue.mjs"; + +const fakeRun = (issues) => { + const calls = []; + const run = (args) => { + calls.push(args); + if (args[0] === "issue" && args[1] === "list") return JSON.stringify(issues ?? []); + return ""; + }; + run.calls = calls; + return run; +}; + +const tracked = (number, ids) => ({ + number, + body: `${MARKER}\nTracked advisory ids: ${JSON.stringify(ids)}`, +}); + +test("an advisory id containing a comma or a backtick still round-trips", () => { + const findings = [ + { id: "RUSTSEC-2026-0001, and more", package: "p@1", summary: "s", kind: "vulnerability" }, + { id: "has`backtick", package: "q@2", summary: "s", kind: "vulnerability" }, + ]; + assert.deepEqual(recordedIds(issueBody(findings)), ["RUSTSEC-2026-0001, and more", "has`backtick"]); +}); + +test("a malformed recorded-ids line reads as no ids rather than throwing", () => { + assert.deepEqual(recordedIds(`${MARKER}\nTracked advisory ids: {not json`), []); + assert.deepEqual(recordedIds(`${MARKER}\nTracked advisory ids: "a string"`), []); +}); + +test("findExisting returns null when no open issue carries the marker", () => { + assert.equal(findExisting(fakeRun([{ number: 1, body: "unrelated" }])), null); +}); + +test("findExisting reads the tracked ids off the matching issue", () => { + const existing = findExisting(fakeRun([{ number: 2, body: "no marker" }, tracked(9, ["B", "A"])])); + assert.deepEqual(existing, { number: 9, ids: ["A", "B"] }); +}); + +// The contract is "exactly one". Quietly picking the first of several would +// honour it wrongly: the rest are never updated and never closed. +test("findExisting refuses rather than choosing between duplicate trackers", () => { + assert.throws( + () => findExisting(fakeRun([tracked(9, ["A"]), tracked(10, ["A"])])), + /2 open issues carry the tracker marker .*#9, #10/, + ); +}); + +test("findExisting scopes its query to open issues carrying the label", () => { + const run = fakeRun([]); + findExisting(run); + const [args] = run.calls; + assert.deepEqual(args.slice(0, 2), ["issue", "list"]); + assert.ok(args.includes("--state") && args[args.indexOf("--state") + 1] === "open"); + assert.ok(args.includes("--label"), "discovery is label-scoped, which the issue body warns about"); + assert.ok(args.includes("--limit"), "an explicit limit, not gh's default"); +}); + +// If the close fails, the wanted end state has not happened and the next run +// recomputes the same plan. Commenting first would leave a 'Closing' comment on +// a still-open issue, once per day, for as long as the failure lasts. +test("close happens before the comment that announces it", () => { + const run = fakeRun([]); + apply({ action: "close", issue: 9 }, run); + assert.deepEqual( + run.calls.map((args) => args[1]), + ["close", "comment"], + ); +}); + +test("a repair rewrites the body and says nothing", () => { + const run = fakeRun([]); + apply({ action: "repair", issue: 9, ids: ["A"], findings: [{ id: "A", package: "p@1", summary: "s", kind: "vulnerability" }] }, run); + assert.deepEqual(run.calls.map((args) => args[1]), ["edit"]); +}); + +test("an update edits the body and names what moved", () => { + const run = fakeRun([]); + apply( + { + action: "update", + issue: 9, + ids: ["A"], + findings: [{ id: "A", package: "p@1", summary: "s", kind: "vulnerability" }], + added: ["A"], + removed: ["B"], + }, + run, + ); + assert.deepEqual(run.calls.map((args) => args[1]), ["edit", "comment"]); + const comment = run.calls[1].at(-1); + assert.match(comment, /newly reported: A/); + assert.match(comment, /no longer reported: B/); +}); + +test("nothing to do means no gh calls at all", () => { + for (const action of ["none", "unchanged"]) { + const run = fakeRun([]); + apply({ action, issue: 9 }, run); + assert.equal(run.calls.length, 0, `${action} must be silent`); + } +}); + +test("a recorded line naming the same set twice repairs without announcing a change", () => { + const findings = [{ id: "A", package: "p@1", summary: "s", kind: "vulnerability" }]; + assert.equal(planAction(findings, { number: 9, ids: ["A", "A"] }).action, "repair"); +});