diff --git a/.github/scripts/check_dependency_advisories.py b/.github/scripts/check_dependency_advisories.py new file mode 100644 index 00000000..7b1b1914 --- /dev/null +++ b/.github/scripts/check_dependency_advisories.py @@ -0,0 +1,358 @@ +#!/usr/bin/env python3 +"""Fail the build when this repo's dependency tree carries a known advisory. + +WHY. The production tree of this repo was, until opencharly/docs#159, carrying +GHSA-rj75-hqrm-r3gf (PostCSS quadratic complexity in flat selector parsing) +for weeks: it was visible in `npm audit` and in nobody's build. A tree that +REPORTS a finding on every run trains its readers to ignore the report, and +the finding that mattered was in the output the whole time. opencharly/docs#163 +asks for the control that ends that: the audit becomes a GATE. + +WHAT IT CHECKS. The tree npm resolves from the committed lockfile -- the same +tree `npm ci` installs, read from the lockfile alone, so the check does not +depend on an install having run. BOTH trees are asserted: + + * the PRODUCTION tree -- what the site ships, and what opencharly/docs#159's + kind of advisory lands in; + * the DEV tree -- the deploy tool, `wrangler`, which holds the Cloudflare + Pages token and runs privileged during a deploy. opencharly/docs#163's own + reasoning: "the deploy path is privileged". + +A finding in EITHER tree fails the check. The production tree is what the +site ships; the dev tree is a tool that holds deployment credentials and +parses responses from a remote service, and opencharly/docs#163 cleared it +with a scoped `overrides` entry rather than leaving it to be re-read. + +WHAT IT DOES NOT DO. It does not decide that a finding is acceptable, and it +does not ignore a severity. The remediation for a finding is a dependency +change (a bump, or a scoped `overrides` entry), which is a change to the +lockfile -- and this check reads that lockfile, so the two cannot drift. + +THE REPORT IS A REPORT, NOT A SUBSTITUTE FOR THE GATE. npm's exit code is 1 +for findings and 1 for some failures too, so the DISCRIMINATOR IS THE JSON: +a run whose output does not parse as an audit report is an ERROR and fails +this check loudly. A check that cannot read the report must never read as a +pass -- that is the difference between a gate and a decoration. + +NO MOCKED AUDIT. The self-test below drives the real `npm audit` against real +fixtures, against the real npm advisory database. Nothing here simulates an +advisory. +""" + +from __future__ import annotations + +import json +import subprocess +import sys +import tempfile +from pathlib import Path + +ROOT = Path(__file__).resolve().parent.parent.parent + +# The fixture's vulnerable package, and WHY it is that one: GHSA-rj75-hqrm-r3gf +# covers `postcss-selector-parser < 7.1.6`, a PUBLISHED advisory with a FIXED +# upper bound. Pinning 6.1.4 therefore keeps the fixture in the advisory range +# for as long as the advisory exists -- it is not a fixture that drifts with a +# transitive bump, and it is not a hand-written "advisory" the checker trusts +# because the checker was told about it. +VULNERABLE_PACKAGE = "postcss-selector-parser" +VULNERABLE_VERSION = "6.1.4" +VULNERABLE_ADVISORY = "GHSA-rj75-hqrm-r3gf" + +# A tree npm cannot audit is an error, never a pass. +AUDIT_TIMEOUT_SECONDS = 300 + + +class AuditError(Exception): + """The audit could not produce a report. Never a pass.""" + + +def _run_audit(directory: Path, omit_dev: bool) -> dict: + """Run `npm audit --json` and return the parsed report. + + The report is REQUIRED to carry the keys the check reads. A report that + parses as JSON but carries no `vulnerabilities` map is not a report of + this shape, and believing it would be believing a verdict that was never + produced. + """ + command = ["npm", "audit", "--json"] + if omit_dev: + command.insert(2, "--omit=dev") + try: + completed = subprocess.run( + command, + cwd=directory, + capture_output=True, + text=True, + timeout=AUDIT_TIMEOUT_SECONDS, + ) + except FileNotFoundError as error: + raise AuditError(f"npm is not on PATH: {error}") from error + except subprocess.TimeoutExpired as error: + raise AuditError(f"npm audit did not finish within {AUDIT_TIMEOUT_SECONDS}s") from error + + try: + report = json.loads(completed.stdout) + except json.JSONDecodeError: + report = None + + # ONE guard, ONE error. npm reports its own failures in the same JSON + # channel it reports findings in -- a tree with no lockfile comes back as + # `{"error": {"code": "ENOLOCK", ...}}` -- so a report that does not carry + # a `vulnerabilities` map is npm saying it could not audit, whatever the + # exit code was. That is an ERROR, and the check fails loudly: a check + # that could not read the report must never read as a pass. + if not isinstance(report, dict) or "vulnerabilities" not in report: + raise AuditError( + "npm audit did not produce an audit report " + f"(exit {completed.returncode}): " + f"{(completed.stderr.strip() or completed.stdout.strip())[:400]}" + ) + vulnerabilities = report["vulnerabilities"] + if not isinstance(vulnerabilities, dict): + raise AuditError("npm audit produced a `vulnerabilities` map that is not an object") + return report + + +def _advisories(vulnerability: dict) -> list[dict]: + """The advisory entries of one vulnerable package. + + npm reports a package's `via` as either an ADVISORY (an object with a + title, a severity and a URL) or the NAME of another package that carries + one. Only the first kind is an advisory of this package. + """ + entries = [] + for via in vulnerability.get("via", []) or []: + if isinstance(via, dict): + entries.append( + { + "title": via.get("title", ""), + "url": via.get("url", ""), + "severity": via.get("severity", ""), + "range": via.get("range", ""), + } + ) + return entries + + +def _reached_through(vulnerability: dict) -> list[str]: + """The names of the packages this one carries a finding THROUGH.""" + names = [] + for via in vulnerability.get("via", []) or []: + if isinstance(via, str): + names.append(via) + return names + + +def check_tree(directory: Path) -> list[dict]: + """Every finding in the tree, each labelled with the tree it sits in.""" + full = _run_audit(directory, omit_dev=False) + production = _run_audit(directory, omit_dev=True) + + production_names = set(production["vulnerabilities"].keys()) + findings = [] + for name, vulnerability in sorted(full["vulnerabilities"].items()): + findings.append( + { + "name": name, + "severity": vulnerability.get("severity", "unknown"), + "tree": "production" if name in production_names else "dev", + "range": vulnerability.get("range", ""), + "nodes": vulnerability.get("nodes", []), + "advisories": _advisories(vulnerability), + "through": _reached_through(vulnerability), + "fix": vulnerability.get("fixAvailable"), + } + ) + return findings + + +def _format_findings(findings: list[dict]) -> list[str]: + lines = [] + for finding in findings: + lines.append( + f"- {finding['name']} ({finding['severity']}) -- the {finding['tree']} tree" + ) + for node in finding["nodes"]: + lines.append(f" at {node}") + if finding["range"]: + lines.append(f" affected versions: {finding['range']}") + for advisory in finding["advisories"]: + lines.append(f" {advisory['title']} ({advisory['severity']})") + if advisory["url"]: + lines.append(f" {advisory['url']}") + for through in finding["through"]: + lines.append(f" reached through {through}") + fix = finding["fix"] + if isinstance(fix, dict): + lines.append( + f" npm's suggested fix: {fix.get('name')} {fix.get('version')}" + + (" (a MAJOR move)" if fix.get("isSemVerMajor") else "") + ) + elif isinstance(fix, bool): + lines.append(" npm's suggested fix: a non-breaking upgrade") + return lines + + +def _write_fixture(root: Path, name: str, dependencies: dict, dev_dependencies: dict) -> Path: + """A fixture tree with a real lockfile, built by npm itself.""" + directory = root / name + directory.mkdir(parents=True, exist_ok=True) + manifest = { + "name": name, + "version": "0.0.0", + "private": True, + } + if dependencies: + manifest["dependencies"] = dependencies + if dev_dependencies: + manifest["devDependencies"] = dev_dependencies + (directory / "package.json").write_text(json.dumps(manifest, indent=2) + "\n", encoding="utf-8") + completed = subprocess.run( + ["npm", "install", "--package-lock-only", "--no-audit", "--no-fund"], + cwd=directory, + capture_output=True, + text=True, + timeout=AUDIT_TIMEOUT_SECONDS, + ) + if completed.returncode != 0: + raise AuditError( + f"could not build the fixture lockfile for {name}: {completed.stderr.strip()[:400]}" + ) + return directory + + +def self_test() -> None: + """Prove the checker on real fixtures, against the real advisory database.""" + failures: list[str] = [] + + def expect(label: str, condition: bool, detail: str = "") -> None: + if not condition: + failures.append(f"{label}{': ' + detail if detail else ''}") + + with tempfile.TemporaryDirectory(prefix="audit-gate-self-test-") as temporary: + root = Path(temporary) + + # (1) A PRODUCTION finding must be caught -- this is the arm + # opencharly/docs#163 asks for by name. + vulnerable = _write_fixture( + root, + "vulnerable-production", + {VULNERABLE_PACKAGE: VULNERABLE_VERSION}, + {}, + ) + findings = check_tree(vulnerable) + expect( + "a vulnerable production fixture is reported", + len(findings) == 1, + str([f["name"] for f in findings]), + ) + if findings: + expect( + "the production finding is labelled as the production tree", + findings[0]["tree"] == "production", + findings[0]["tree"], + ) + expect( + f"the production finding names {VULNERABLE_ADVISORY}", + any( + VULNERABLE_ADVISORY in advisory["url"] + for advisory in findings[0]["advisories"] + ), + str(findings[0]["advisories"]), + ) + else: + failures.append("the vulnerable production fixture produced no finding to label") + + # (2) A clean tree must NOT be reported -- a gate that fails on + # everything is not a gate. + clean = _write_fixture(root, "clean", {}, {}) + clean_findings = check_tree(clean) + expect( + "a clean fixture produces no finding", + clean_findings == [], + str([f["name"] for f in clean_findings]), + ) + + # (3) A DEV finding must be caught too: the deploy tool holds the + # Cloudflare token, so its tree is watched as well. + vulnerable_dev = _write_fixture( + root, + "vulnerable-dev", + {}, + {VULNERABLE_PACKAGE: VULNERABLE_VERSION}, + ) + dev_findings = check_tree(vulnerable_dev) + expect( + "a vulnerable dev fixture is reported", + len(dev_findings) == 1, + str([f["name"] for f in dev_findings]), + ) + if dev_findings: + expect( + "the dev finding is labelled as the dev tree", + dev_findings[0]["tree"] == "dev", + dev_findings[0]["tree"], + ) + + # (4) A tree npm cannot audit must FAIL, never pass silently. npm + # reports that in the same channel it reports findings in -- a tree + # with no lockfile comes back as `{"error": {"code": "ENOLOCK"}}` -- + # so the check must read it as an error rather than as an empty audit. + broken = root / "no-lockfile" + broken.mkdir(parents=True, exist_ok=True) + (broken / "package.json").write_text( + json.dumps({"name": "no-lockfile", "version": "0.0.0", "private": True}) + "\n", + encoding="utf-8", + ) + try: + check_tree(broken) + except AuditError: + pass + else: + failures.append("an unauditable tree did not raise an error") + + if failures: + print("check-dependency-advisories self-test: FAILED") + for failure in failures: + print(f"- {failure}") + raise SystemExit(1) + print( + "check-dependency-advisories self-test: OK (a production finding, a dev finding and a " + "clean tree are all told apart, and an unauditable tree is an error)" + ) + + +def main() -> int: + arguments = sys.argv[1:] + if arguments == ["--self-test"]: + self_test() + return 0 + directory = Path(arguments[0]).resolve() if arguments else ROOT + if not (directory / "package-lock.json").is_file(): + print("check-dependency-advisories: FAILED") + print(f"- no package-lock.json in {directory}: there is no tree to audit") + return 1 + try: + findings = check_tree(directory) + except AuditError as error: + print("check-dependency-advisories: FAILED") + print(f"- the tree could not be audited: {error}") + return 1 + if findings: + production = [f for f in findings if f["tree"] == "production"] + dev = [f for f in findings if f["tree"] == "dev"] + print("check-dependency-advisories: FAILED") + print( + f"- {len(findings)} finding(s) in the dependency tree " + f"({len(production)} in the production tree, {len(dev)} in the dev tree)" + ) + for line in _format_findings(findings): + print(line) + return 1 + print("check-dependency-advisories: OK (no known advisory in the production tree or the dev tree)") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/.github/scripts/test_check_dependency_advisories.py b/.github/scripts/test_check_dependency_advisories.py new file mode 100644 index 00000000..5ca166ad --- /dev/null +++ b/.github/scripts/test_check_dependency_advisories.py @@ -0,0 +1,182 @@ +#!/usr/bin/env python3 +"""Prove that the dependency-advisory checker's SELF-TEST can fail — one mechanism at a time. + +WHY THIS EXISTS. `.github/scripts/check_dependency_advisories.py` ships a +`--self-test` mode, and that mode is the committed coverage of the rule: it is +what proves the gate can fail on a production-tree finding, which is the +obligation opencharly/docs#163 asks for. A self-test is only evidence if it can +FAIL — a checker whose self-test passes whatever the code does proves nothing, +and that failure mode is SILENT: the gate stays green while the rule it claims +to enforce has stopped being enforced. + +So each mechanism of the checker is removed, one at a time, and this harness +asserts that the self-test then turns RED on the fixture that covers exactly +that mechanism. A mutant that still passes is a mechanism the self-test does +not cover, and the harness fails on it. + +The mutation is an ASSERTED single-string replacement: the anchor must occur +EXACTLY once in the committed source, and the harness fails loudly when it does +not, so a moved anchor can never masquerade as a passing mutation. + +WHAT IS PROVED. Each mutant removes ONE mechanism: + + mutA the production audit — the second npm audit that tells the + production tree from the dev tree + mutB the findings loop — the read of npm's report at all + mutC the tree label — which tree a finding is reported in + mutD the error path — a tree npm cannot audit is an ERROR, + never a silent pass + mutE the advisory extraction — the advisory titles and URLs a finding + is reported with + +The mutant is written into a temporary tree that MIRRORS this repo's layout +(`.github/scripts/`), because the checker derives its repository root +from its own path. + +usage: python3 .github/scripts/test_check_dependency_advisories.py + python3 .github/scripts/test_check_dependency_advisories.py --emit +exit: 0 = every mutant turned the self-test red; 1 = a mutant still passed. +""" + +from pathlib import Path +import subprocess +import sys +import tempfile + +ROOT = Path(__file__).resolve().parent.parent.parent +SOURCE = ROOT / ".github" / "scripts" / "check_dependency_advisories.py" + +# (name, anchor, replacement, what the mutation removes, the self-test case +# that must turn red). +MUTANTS = [ + ( + "mutA-noproductionaudit", + " production = _run_audit(directory, omit_dev=True)", + " production = {\"vulnerabilities\": {}} # MUTANT: the production audit removed", + "the production audit", + "the production finding is labelled as the production tree", + ), + ( + "mutB-nofindings", + ' for name, vulnerability in sorted(full["vulnerabilities"].items()):', + ' for name, vulnerability in sorted({}.items()): # MUTANT: the findings loop removed', + "the findings loop", + "a vulnerable production fixture is reported", + ), + ( + "mutC-invertedlabel", + ' "tree": "production" if name in production_names else "dev",', + ' "tree": "dev" if name in production_names else "production", # MUTANT: the tree label inverted', + "the tree label", + "the production finding is labelled as the production tree", + ), + ( + "mutD-swallowederror", + ' if not isinstance(report, dict) or "vulnerabilities" not in report:\n' + ' raise AuditError(\n' + ' "npm audit did not produce an audit report "\n' + ' f"(exit {completed.returncode}): "\n' + ' f"{(completed.stderr.strip() or completed.stdout.strip())[:400]}"\n' + ' )', + ' if not isinstance(report, dict) or "vulnerabilities" not in report:\n' + ' return {"vulnerabilities": {}} # MUTANT: the report guard swallowed', + "the report guard", + "an unauditable tree did not raise an error", + ), + ( + "mutE-noadvisories", + " if isinstance(via, dict):", + " if False: # MUTANT: the advisory extraction removed", + "the advisory extraction", + "the production finding names GHSA-rj75-hqrm-r3gf", + ), +] + + +def emit(name: str, destination: str) -> int: + """Write ONE mutant to a path, so another harness can use it as a control. + + The anchor assertion is the same one the harness makes: an anchor that has + moved must FAIL LOUDLY here too, because a mutant that is not the mutation + it claims to be is a control that proves nothing. + """ + text = SOURCE.read_text(encoding="utf-8") + for candidate, anchor, replacement, what, _case in MUTANTS: + if candidate != name: + continue + count = text.count(anchor) + if count != 1: + print( + f"FAIL: the anchor for {what} occurs {count} times, expected 1", + file=sys.stderr, + ) + return 1 + target = Path(destination) + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text(text.replace(anchor, replacement), encoding="utf-8") + print(f"emitted {target} — {what} removed") + return 0 + print(f"FAIL: no mutant named {name!r}", file=sys.stderr) + return 1 + + +def main() -> int: + arguments = sys.argv[1:] + if arguments[:1] == ["--emit"]: + if len(arguments) != 3: + print("usage: test_check_dependency_advisories.py --emit ", file=sys.stderr) + return 2 + return emit(arguments[1], arguments[2]) + + text = SOURCE.read_text(encoding="utf-8") + failures: list[str] = [] + passed = 0 + + for name, anchor, replacement, what, case in MUTANTS: + count = text.count(anchor) + if count != 1: + failures.append( + f"{name}: the anchor for {what} occurs {count} times, expected 1 — " + f"the mutation cannot be applied, so it would prove nothing" + ) + continue + with tempfile.TemporaryDirectory() as temporary: + scripts = Path(temporary) / ".github" / "scripts" + scripts.mkdir(parents=True) + mutant = scripts / f"{name}.py" + mutant.write_text(text.replace(anchor, replacement), encoding="utf-8") + completed = subprocess.run( + [sys.executable, str(mutant), "--self-test"], + capture_output=True, + text=True, + ) + output = completed.stdout + completed.stderr + if completed.returncode == 0: + failures.append( + f"{name}: the self-test still PASSED with {what} removed — " + f"the self-test does not cover that mechanism" + ) + continue + if case not in output: + failures.append( + f"{name}: the self-test failed, but not on the case that covers " + f"{what} (expected {case!r} in the output)" + ) + continue + passed += 1 + print(f"ok {name} — {what} removed, and the self-test turned red on it") + + if failures: + print(f"test_check_dependency_advisories: FAIL ({len(failures)})") + for failure in failures: + print(f"- {failure}") + return 1 + print( + f"test_check_dependency_advisories: OK ({passed} mutants; every one of them turns " + f"the self-test red)" + ) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index eadc654f..e2777717 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -174,6 +174,39 @@ jobs: node-version-file: .node-version cache: npm + # The audit becomes a GATE (opencharly/docs#163). The production tree + # was clean only by the accident of a lockfile: GHSA-rj75-hqrm-r3gf sat + # in it for weeks and was visible only to whoever happened to run + # `npm audit`. This check reads the tree npm resolves from the committed + # lockfile -- the same tree `npm ci` installs, so it does not depend on + # an install having run -- and fails on a finding in EITHER tree: the + # production tree (what the site ships) and the dev tree (the deploy + # tool, which holds the Cloudflare Pages token and runs privileged + # during a deploy). + # + # It runs EARLY, before the expensive steps, because a finding here is + # cheap to find and expensive to discover after a ten-minute + # regeneration. + - name: Check that the dependency tree carries no known advisory + timeout-minutes: 5 + run: python3 .github/scripts/check_dependency_advisories.py + + # A check that cannot fail is not a check. The self-test drives the + # real `npm audit` against real fixtures -- one carrying a published + # advisory in its PRODUCTION tree, one carrying it in its dev tree, one + # clean, and one npm cannot audit at all -- against the real npm + # advisory database. Nothing is simulated. + - name: Prove the dependency-advisory checker rejects a production-tree finding + timeout-minutes: 10 + run: python3 .github/scripts/check_dependency_advisories.py --self-test + + # And a self-test that cannot fail is not a self-test. Each mechanism + # of the checker is removed, one at a time, and the self-test must turn + # RED on the fixture that covers exactly that mechanism. + - name: Prove the dependency-advisory self-test can fail (mutation harness) + timeout-minutes: 10 + run: python3 .github/scripts/test_check_dependency_advisories.py + # setup-go hashes only files inside GITHUB_WORKSPACE. Keep the module it # actually compiles here, rather than keying its cache on /tmp/charly. - name: Fetch the pinned docs generator diff --git a/AGENTS.md b/AGENTS.md index d737da68..494948bb 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -33,6 +33,11 @@ Canonical files: ## Build / validate / test - `npm ci && npm run build` — the local Astro build. +- `python3 .github/scripts/check_dependency_advisories.py` — the + dependency-advisory gate. It audits the tree the committed lockfile resolves + (the same tree `npm ci` installs) and FAILS on a known advisory in the + **production** tree or the **dev** tree. The generation workflow runs it, its + self-test, and the mutation harness that proves the self-test can fail. - The merge gate is the **org-wide** `charly/pr-validator` (required check `validate / validate`, defined in `opencharly/.github`); this repo carries no per-repo candy gate. diff --git a/package-lock.json b/package-lock.json index 726408e9..6a17e011 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4666,9 +4666,9 @@ } }, "node_modules/miniflare/node_modules/undici": { - "version": "7.29.1", - "resolved": "https://registry.npmjs.org/undici/-/undici-7.29.1.tgz", - "integrity": "sha512-RYONW2MeafgYlkVOKYKkA/Ag7BmXqgIWCa8t1m0JcxrQg9pI9lEqRhAOruOBCbAohOa/gkCF+iPi9hrgvTzu6Q==", + "version": "7.30.1", + "resolved": "https://registry.npmjs.org/undici/-/undici-7.30.1.tgz", + "integrity": "sha512-03KrMbjIFDWQR5RNPrSw3bd1r12JpyFQl9/yxjUbJtqkjmllqC5lFjZh6ujeuJCKr8EOQvRW872Oy/az+nLYxg==", "dev": true, "license": "MIT", "engines": { diff --git a/package.json b/package.json index 83bbe295..848c6e96 100644 --- a/package.json +++ b/package.json @@ -18,5 +18,8 @@ }, "devDependencies": { "wrangler": "4.151.0" + }, + "overrides": { + "undici@7.29.1": "7.30.1" } }