diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 4769fa6b49..f28df761e2 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -120,10 +120,12 @@ exposes independent marker damage in the same evidence packet; it never admits the release or enables later delivery checks unless both trust checks pass. Scheduled and push-time npm audits classify JSON results, use one-minute registry attempts, and retry an unavailable audit endpoint. Advisory findings -still fail immediately, even if the same response contains a transport error. The -scheduled security scan and dependency-changing builds fail if three attempts -produce no result; a build with an unchanged dependency graph warns and uses -the base commit's passing answer. Audit steps defer their aggregate verdict so +are conclusive immediately, even if the same response contains a transport error. The +scheduled security scan, the dependency advisory watch and dependency-changing +builds fail on any finding and when three attempts produce no result. A build +with an unchanged dependency graph warns on both instead and names every +finding, because its graph is the base commit's, so any advisory it sees is one +the base already has and that only a dependency change can remove. Audit steps defer their aggregate verdict so an unavailable endpoint cannot suppress independent frontend checks or the production bundle build; the preceding clean install disables npm's duplicate best-effort audit request. @@ -170,8 +172,8 @@ one that passed. It uploads nothing and gates nothing. `dependency-advisory-watch.yml` runs the required build-and-test frontend audit daily against `main` and every active `release/v.` line (the latest stable's line and newer), because a new npm advisory against an -unchanged lockfile otherwise surfaces only when it fails every pull request on -that branch. Each line is a separate matrix job that reads only that line's +unchanged lockfile otherwise surfaces only as a warning on that branch's pull +requests; this watch is where it fails. Each line is a separate matrix job that reads only that line's `frontend-modern/package.json` and `package-lock.json` (it never checks out, installs or runs another branch's code in the default branch's scope) and audits them with the same Node.js pin and `scripts/npm-audit-retry.sh all`. A failing job names diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 701861305e..7a0d49b3a2 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -105,10 +105,12 @@ jobs: fi echo "code=$code" >> "$GITHUB_OUTPUT" # Whether this change moves the frontend dependency graph. It decides - # what an unreachable npm advisory endpoint means: unknown answer - # (must block) versus the base commit's already-passing answer. A - # change to the audit runner itself also demands a real result, so it - # can never be relaxed under cover of its own tolerant mode. + # whether the audit blocks it: a change that moves the graph answers + # for any finding and for an unreachable endpoint, while one that + # leaves the graph alone only warns, because its findings are the + # base commit's. A change to the audit runner itself also demands a + # clean result, so it can never be relaxed under cover of its own + # tolerant mode. if printf '%s\n' "$changed" | grep -qE '^frontend-modern/package(-lock)?\.json$|^scripts/npm-audit-retry\.sh$'; then frontend_deps=true else @@ -193,14 +195,15 @@ jobs: run: npm ci --no-audit # npm audit exits 1 both for a real advisory and for an unreachable - # advisory endpoint. The runner keeps the advisory verdict exactly as - # strict and only retries the endpoint being down; see the script header. + # advisory endpoint. The runner retries the endpoint being down, fails a + # change that moves the dependency graph on any finding, and only warns, + # naming every finding, for a change whose graph is the base commit's; + # see the script header. # # Only the complete graph is audited here. `--omit=dev` audits a subset - # of these packages, so it can only report a subset of these advisories; - # and because the complete audit fails the job on any finding, the - # production step could only ever run in the cases where it was already - # guaranteed clean. The dev-versus-production split is still reported + # of these packages, so it can only report a subset of these advisories, + # under the same blocking rule; a separate production step could never + # block a change the complete audit lets through. The dev-versus-production split is still reported # for every workspace by the scheduled npm-audit job in # security-scan.yml, where it informs rather than blocks delivery. - name: Audit complete frontend dependency graph diff --git a/.github/workflows/dependency-advisory-watch.yml b/.github/workflows/dependency-advisory-watch.yml index 94bf5aeacf..966a3dbfb0 100644 --- a/.github/workflows/dependency-advisory-watch.yml +++ b/.github/workflows/dependency-advisory-watch.yml @@ -1,12 +1,14 @@ name: Dependency Advisory Watch -# A new npm advisory against an unchanged lockfile fails the required +# A new npm advisory against an unchanged lockfile used to fail the required # "Audit complete frontend dependency graph" step in build-and-test.yml on # every pull request to that branch. On 30 Sep-1 Oct 2026 two advisories did # exactly that to main, release/v6.4 and release/v6.5 at once, and nothing -# owned the fix. This watch runs the same audit daily against main and every -# active release line, fails when any of them would block, and names the -# branch, the advisories and the fix in the step summary. Scheduled workflows +# owned the fix. Since 3 Oct 2026 that step only warns for a pull request that +# leaves the lockfile alone, so this watch is where such an advisory fails: it +# runs the same audit daily against main and every active release line with +# NPM_AUDIT_REQUIRE_RESULT=true, fails on any finding, and names the branch, +# the advisories and the fix in the step summary. Scheduled workflows # only run on the default branch. The audit needs only each line's # frontend-modern/package.json and package-lock.json, so this job reads those # two files with git and never checks out, installs or executes another diff --git a/docs/release-control/v6/internal/subsystems/deployment-installability.md b/docs/release-control/v6/internal/subsystems/deployment-installability.md index efa5d4e8c8..43ec755a66 100644 --- a/docs/release-control/v6/internal/subsystems/deployment-installability.md +++ b/docs/release-control/v6/internal/subsystems/deployment-installability.md @@ -4788,17 +4788,17 @@ borrowing the local dev-runtime orchestration tests. The canonical `npm audit` after a clean install, through `scripts/npm-audit-retry.sh`. The production-only `npm audit --omit=dev` is deliberately not on that per-pull-request path: it audits a subset of the same packages, so it can only -report a subset of the same advisories, and because the complete audit fails -the job on any finding, the production step could only ever execute in the -cases where it was already guaranteed clean. The dev-versus-production split +report a subset of the same advisories under the same blocking rule, so a +production step could never block a change the complete audit lets through. The dev-versus-production split is reported instead by the scheduled `npm-audit` job in `.github/workflows/security-scan.yml`, which covers every npm workspace and informs rather than blocks delivery. That runner exists because `npm audit` exits non-zero both for a real advisory and for an unreachable advisory endpoint: on 2026-09-03 registry.npmjs.org returned 503s and timeouts for over an hour and no pull request could land, including changes that touch -no JavaScript. It separates the two and nothing else. A conclusive result is -acted on immediately and any vulnerability at any severity still fails, even +no JavaScript. It separates the two, and decides whether a finding blocks the change by +whether the change moves the dependency graph. A conclusive result is +acted on immediately and any vulnerability at any severity is a finding, even if the same response also carries a transport error, so a severity threshold must never be introduced; only an unreachable endpoint is retried. A nonempty package-finding map or any positive integer @@ -4835,13 +4835,29 @@ by attempt count or by budget, the run fails if the change touches `frontend-modern/package.json`, `frontend-modern/package-lock.json`, or the runner itself, because then the answer is genuinely unknown and the runner may never be relaxed under cover of its own tolerant mode, and warns without -failing when it does not, because the dependency graph is then identical to the base commit that -already produced a passing answer. Advisories published later against -unchanged dependencies are the responsibility of Dependabot security updates -and the scheduled scan, not of a per-pull-request audit. -`scripts/tests/test-npm-audit-retry.sh` pins -that split, including that a real advisory fails even when the tolerant mode -is active, that an unparseable or unrecognised report is never read as +failing when it does not, because the dependency graph is then identical to its +base commit. The same split governs a finding. A change that moves the graph, +or edits the runner, fails on any finding. A change that leaves +`package.json` and `package-lock.json` untouched warns instead, still printing +every finding, because any advisory it sees is one its base commit already has +and only a dependency change can remove it. Only `NPM_AUDIT_REQUIRE_RESULT=false` +selects that mode; an empty or unrecognised value keeps the strict verdict. On +3 October 2026 the unpatched `braces` advisory `GHSA-vfj7-8cjw-p6xm`, reached +only through the dev-only `tailwindcss` 3 and `jscpd` 4 chains with a clean +`--omit=dev` audit, failed the required Frontend check on every open pull +request, Go-only ones included, which made nothing safer and blocked every +fix. Advisories against unchanged dependencies are owned by the jobs that audit +the graph as it stands: the scheduled `npm-audit` scan and the daily +`dependency-advisory-watch.yml`, which keep the strict default and fail on any +finding, and Dependabot security updates; +`scripts/tests/test_dependency_advisory_watch.py` runs the real runner with the +watch's own environment and proves an advisory that only warns a pull request +fails the watch. +`scripts/tests/test-npm-audit-retry.sh` and +`scripts/tests/test_npm_audit_retry.py` pin that split, including that a real +advisory fails whenever the graph moves or the mode value is not exactly +`false`, that an inherited advisory warns and is still named, that an +unparseable or unrecognised report is never read as clean, and that neither a hung attempt nor an exhausted budget can outlive its bound. `frontend-modern/src/security/__tests__/dependencySecurity.test.ts` pins the known safe floors for advisories remediated by commit `6ba85a185`, diff --git a/scripts/installtests/build_release_assets_test.go b/scripts/installtests/build_release_assets_test.go index 94688451c0..c7d55a6c30 100644 --- a/scripts/installtests/build_release_assets_test.go +++ b/scripts/installtests/build_release_assets_test.go @@ -4213,8 +4213,11 @@ func TestFrontendDependencySecurityAuditsAreRequired(t *testing.T) { ) // The gate itself must stay strict. An unreachable endpoint may be // retried, but no severity threshold may be introduced that lets a real - // advisory through. Any positive package, severity or total evidence - // must fail, even when the summary is missing or contradictory. + // advisory through. Any positive package, severity or total evidence is + // a finding, even when the summary is missing or contradictory, and it + // fails every change that moves the dependency graph. Only an explicit + // NPM_AUDIT_REQUIRE_RESULT=false, a graph identical to its base, may turn + // a finding the base already has into a warning. runnerPath := repoFile("scripts", "npm-audit-retry.sh") runner, err := os.ReadFile(runnerPath) if err != nil { @@ -4233,6 +4236,7 @@ func TestFrontendDependencySecurityAuditsAreRequired(t *testing.T) { `isinstance(report, dict) and not report.get("error")`, `len(counts) == len(count_names) and all(count == 0 for count in counts.values())`, `and ("vulnerabilities" not in report or isinstance(findings, dict))`, + `if [ "${REQUIRE_RESULT}" = "false" ]; then`, ) // Retrying must be bounded by wall clock, not by attempt count alone. // npm's own fetch-timeout defaults to five minutes and it retries diff --git a/scripts/npm-audit-retry.sh b/scripts/npm-audit-retry.sh index 95ddc1d2d9..c80d6529f8 100755 --- a/scripts/npm-audit-retry.sh +++ b/scripts/npm-audit-retry.sh @@ -10,15 +10,18 @@ # returned 503s and timeouts for over an hour and no pull request could land, # including Go-only ones. Four consecutive failures, zero advisories. # -# This keeps the gate exactly as strict about advisories — any vulnerability at -# any severity still fails, and suppression is never a valid closure — and -# changes only what happens when npm cannot answer: +# Any vulnerability at any severity is still reported, no severity threshold +# exists, and suppression is never a valid closure. What depends on the change +# is only whether the verdict blocks it: # -# * a conclusive answer is acted on immediately, pass or fail; +# * a conclusive answer is acted on immediately; # * an unreachable endpoint is retried with backoff; -# * if it is still unreachable after every attempt, the run fails when this -# change touches the dependency graph (NPM_AUDIT_REQUIRE_RESULT=true) and -# warns without failing when it does not. +# * a change that moves the dependency graph (NPM_AUDIT_REQUIRE_RESULT=true, +# or any value other than exactly "false") fails on any finding and on an +# endpoint that never answers; +# * a change that leaves the graph identical to its base +# (NPM_AUDIT_REQUIRE_RESULT=false) warns on both instead, naming every +# finding, because any finding it sees is one the base commit already has. # # The retry budget is wall-clock, not just an attempt count, because attempt # count alone does not bound anything: npm's own `fetch-timeout` defaults to @@ -30,13 +33,19 @@ # attempt is bounded, npm's internal retry loop is disabled in favour of this # one, and the whole sequence stops at a deadline. # -# That last split is the whole safety argument. When package.json and +# That split is the whole safety argument. When package.json and # package-lock.json are untouched, the audit answer for this change is the one -# the base commit already produced, so skipping it adds no risk from this -# change; advisories published later against unchanged dependencies are caught -# by Dependabot security updates, not by a per-pull-request audit. When the -# dependency graph does move, the answer is unknown and only then does an -# unreachable endpoint have to block. +# the base commit would produce, so this change can neither add a finding nor +# remove one. Failing it on an advisory the base already has blocks unrelated +# work without making anything safer: on 2026-10-03 GHSA-vfj7-8cjw-p6xm +# (braces, reached only through dev-only tailwindcss and jscpd, with no patched +# release) failed the required Frontend check on every open pull request, +# Go-only ones included. Advisories against unchanged dependencies are owned by +# the jobs that audit the graph as it stands: the scheduled npm-audit scan and +# the dependency advisory watch, which keep the strict default and fail, and +# Dependabot security updates. When the dependency graph does move, the +# change is answerable for the result, so any finding and any missing answer +# block it. # # Env: # NPM_AUDIT_ATTEMPTS attempts before giving up (default 3) @@ -45,8 +54,11 @@ # NPM_AUDIT_ATTEMPT_TIMEOUT seconds one npm invocation may run (default 60) # NPM_AUDIT_MAX_SECONDS total wall-clock budget for all attempts # (default 240) -# NPM_AUDIT_REQUIRE_RESULT "true" to fail when no answer was obtained -# (default true — the safe default) +# NPM_AUDIT_REQUIRE_RESULT "false" only when this change leaves +# package.json and package-lock.json identical to +# its base; findings and a missing answer then +# warn. Any other value fails on both (default +# true — the safe default) # NPM_AUDIT_CMD npm executable to invoke (test seam) set -uo pipefail @@ -70,6 +82,13 @@ MAX_SECONDS="${NPM_AUDIT_MAX_SECONDS:-240}" REQUIRE_RESULT="${NPM_AUDIT_REQUIRE_RESULT:-true}" NPM_BIN="${NPM_AUDIT_CMD:-npm}" +# Only an explicit "false" may relax anything. An empty or misspelled value, +# for instance from a missing workflow output, keeps the strict verdict. +graph_unchanged=false +if [ "${REQUIRE_RESULT}" = "false" ]; then + graph_unchanged=true +fi + # This script is the retry layer. npm's own fetch retry loop would multiply # every attempt by an unbounded amount of hidden waiting, which is exactly # what made a bounded-looking three attempts run for eleven minutes. @@ -240,10 +259,14 @@ while [ "${attempt}" -le "${ATTEMPTS}" ]; do ;; vulnerable) echo "npm audit (${SCOPE}): vulnerabilities present (${summary})" - echo "::error::npm audit (${SCOPE}) found vulnerabilities: ${summary}" # Keep diagnostics bound to the conclusive response, with no extra # registry request outside the attempt/total wall-clock limits. printf '%s\n' "${verdict_output}" | tail -n +3 + if [ "${graph_unchanged}" = "true" ]; then + echo "::warning::npm audit (${SCOPE}) found vulnerabilities the base commit already has: ${summary}. This change does not touch package.json or package-lock.json, so it neither introduced them nor can remove them; the scheduled audit and the dependency advisory watch keep failing until the dependency graph is fixed." + exit 0 + fi + echo "::error::npm audit (${SCOPE}) found vulnerabilities: ${summary}" exit 1 ;; *) @@ -271,10 +294,10 @@ else gave_up="after ${ATTEMPTS} attempts" fi -if [ "${REQUIRE_RESULT}" = "true" ]; then +if [ "${graph_unchanged}" != "true" ]; then echo "::error::npm audit (${SCOPE}) could not reach the advisory endpoint ${gave_up}, and this change touches the dependency graph, so the result cannot be assumed." exit 1 fi -echo "::warning::npm audit (${SCOPE}) could not reach the advisory endpoint ${gave_up}. This change does not touch package.json or package-lock.json, so the dependency graph is identical to the base commit that already passed; continuing without a fresh result." +echo "::warning::npm audit (${SCOPE}) could not reach the advisory endpoint ${gave_up}. This change does not touch package.json or package-lock.json, so the dependency graph is identical to its base commit; continuing without a fresh result." exit 0 diff --git a/scripts/tests/test-npm-audit-retry.sh b/scripts/tests/test-npm-audit-retry.sh index e005e7e9d4..165af753a7 100755 --- a/scripts/tests/test-npm-audit-retry.sh +++ b/scripts/tests/test-npm-audit-retry.sh @@ -1,7 +1,8 @@ #!/usr/bin/env bash # -# Smoke tests for scripts/npm-audit-retry.sh — the gate must stay exactly as -# strict about advisories and only tolerate an unreachable endpoint. +# Smoke tests for scripts/npm-audit-retry.sh — a change that moves the +# dependency graph must fail on any advisory and on an unreachable endpoint; +# only a change whose graph is the base commit's may warn instead. set -euo pipefail @@ -79,10 +80,28 @@ run_case() { run_case "clean audit passes" 0 "$(make_fake_npm npm-clean "${CLEAN}")" true \ "no vulnerabilities" -# A vulnerability fails, and must fail even when the dependency graph is -# untouched — the outage tolerance must never soften a real finding. -run_case "vulnerability fails" 1 "$(make_fake_npm npm-vuln "${VULN}")" false \ - "vulnerabilities present" +# A vulnerability fails a change that moves the dependency graph. +run_case "vulnerability fails when dependencies changed" 1 \ + "$(make_fake_npm npm-vuln "${VULN}")" true \ + "vulnerabilities present" "::error::" + +# A change whose graph is the base commit's cannot have introduced the +# finding, so it warns and still names it rather than reading it as clean. +run_case "inherited vulnerability warns when dependencies unchanged" 0 \ + "$(make_fake_npm npm-vuln-inherited "${VULN}")" false \ + "vulnerabilities present" "::warning::" "base commit already has" + +# Only an explicit "false" relaxes the verdict. An empty or misspelled value, +# as a missing workflow output would produce, keeps it strict. +run_case "empty mode value keeps a vulnerability failing" 1 \ + "$(make_fake_npm npm-vuln-empty "${VULN}")" "" \ + "vulnerabilities present" "::error::" +run_case "unrecognised mode value keeps a vulnerability failing" 1 \ + "$(make_fake_npm npm-vuln-typo "${VULN}")" False \ + "vulnerabilities present" "::error::" +run_case "empty mode value keeps an outage failing" 1 \ + "$(make_fake_npm npm-out-empty "${ENOAUDIT}")" "" \ + "could not reach the advisory endpoint" # A transient endpoint failure that clears on retry passes. run_case "retry recovers from a transient outage" 0 \ @@ -164,7 +183,7 @@ NPM_AUDIT_ATTEMPTS=50 NPM_AUDIT_ATTEMPT_TIMEOUT=1 NPM_AUDIT_MAX_SECONDS=3 \ # only ever change what happens to an unreachable endpoint. NPM_AUDIT_ATTEMPT_TIMEOUT=1 NPM_AUDIT_MAX_SECONDS=3 \ run_case "a vulnerability still fails under a tight budget" 1 \ - "$(make_fake_npm npm-vuln-budget "${VULN}")" false \ + "$(make_fake_npm npm-vuln-budget "${VULN}")" true \ "vulnerabilities present" if [ "${failures}" -ne 0 ]; then diff --git a/scripts/tests/test_dependency_advisory_watch.py b/scripts/tests/test_dependency_advisory_watch.py index 9019c1818c..6cdd6b6beb 100644 --- a/scripts/tests/test_dependency_advisory_watch.py +++ b/scripts/tests/test_dependency_advisory_watch.py @@ -4,6 +4,7 @@ from __future__ import annotations import importlib.util import json +import os from pathlib import Path import re import subprocess @@ -202,6 +203,38 @@ class WorkflowContractTest(unittest.TestCase): self.assertEqual(step(audit, "Audit complete frontend dependency graph")["env"]["NPM_AUDIT_REQUIRE_RESULT"], "true") + def test_watch_fails_on_an_advisory_pull_requests_only_warn_on(self) -> None: + # A pull request that leaves the lockfile alone only warns on an + # advisory its base already has; this watch is where that advisory + # must fail. Run the real runner with the watch's own environment. + audit_step = next( + item for item in self.workflow["jobs"]["audit"]["steps"] + if item.get("name") == "Audit complete frontend dependency graph" + ) + vulnerable = json.dumps({ + "metadata": {"vulnerabilities": { + "info": 0, "low": 0, "moderate": 0, "high": 1, "critical": 0, "total": 1, + }}, + "vulnerabilities": {"braces": {"name": "braces", "severity": "high"}}, + }) + with tempfile.TemporaryDirectory() as directory: + fake_npm = Path(directory) / "npm" + fake_npm.write_text(f"#!/bin/sh\nprintf '%s\\n' '{vulnerable}'\nexit 1\n", encoding="utf-8") + fake_npm.chmod(0o755) + outcomes = {} + for mode in (audit_step["env"]["NPM_AUDIT_REQUIRE_RESULT"], "false"): + result = subprocess.run( + ["bash", str(ROOT / "scripts" / "npm-audit-retry.sh"), "all"], + env={**os.environ, "NPM_AUDIT_CMD": str(fake_npm), + "NPM_AUDIT_REQUIRE_RESULT": mode, "NPM_AUDIT_RETRY_DELAY": "0"}, + text=True, capture_output=True, check=False, + ) + outcomes[mode] = result + self.assertEqual(1, outcomes["true"].returncode, outcomes["true"].stdout) + self.assertIn("::error::", outcomes["true"].stdout) + self.assertEqual(0, outcomes["false"].returncode, outcomes["false"].stdout) + self.assertIn("::warning::", outcomes["false"].stdout) + def test_checkout_pins_match_build_and_test(self) -> None: pins = set(re.findall(r"uses: (\S+@[0-9a-f]{40})", BUILD_AND_TEST.read_text(encoding="utf-8"))) for pin in re.findall(r"uses: (\S+@\S+)", self.text): diff --git a/scripts/tests/test_npm_audit_retry.py b/scripts/tests/test_npm_audit_retry.py index 667a4754ff..3bcb17c7cb 100755 --- a/scripts/tests/test_npm_audit_retry.py +++ b/scripts/tests/test_npm_audit_retry.py @@ -146,8 +146,29 @@ class NpmAuditRetryTest(unittest.TestCase): ["audit --json"], ) self.assertIn("vulnerabilities present", result.stdout) + self.assertIn("::error::", result.stdout) self.assertNotIn("retrying", result.stdout) + def test_inherited_vulnerability_warns_for_an_unchanged_dependency_graph(self) -> None: + result, calls = self.run_check("vulnerability", "all", require="false") + self.assertEqual(result.returncode, 0, result.stdout) + self.assertEqual(calls, ["audit --json"]) + self.assertIn("vulnerabilities present", result.stdout) + self.assertIn("::warning::", result.stdout) + self.assertIn("base commit already has", result.stdout) + self.assertNotIn("::error::", result.stdout) + self.assertNotIn("no vulnerabilities", result.stdout) + self.assertNotIn("retrying", result.stdout) + + def test_only_an_explicit_false_relaxes_a_vulnerability(self) -> None: + for require in ["", "False", "0", "no", "true"]: + with self.subTest(require=require): + result, calls = self.run_check("vulnerability", "all", require=require) + self.assertEqual(result.returncode, 1, result.stdout) + self.assertEqual(calls, ["audit --json"]) + self.assertIn("::error::", result.stdout) + self.assertNotIn("::warning::", result.stdout) + def test_vulnerability_verdict_precedes_a_transport_error(self) -> None: result, calls = self.run_check("vulnerability-with-error", "all") self.assertEqual(result.returncode, 1) @@ -188,10 +209,13 @@ class NpmAuditRetryTest(unittest.TestCase): }, "error": {"detail": "must not be printed"}, } + # An inherited finding only warns, but it must be named exactly as a + # blocking one would be. result, calls = self.run_check( "captured-vulnerability", "all", require="false", report=report ) - self.assertEqual(result.returncode, 1) + self.assertEqual(result.returncode, 0, result.stdout) + self.assertIn("::warning::", result.stdout) self.assertEqual(calls, ["audit --json"]) finding_lines = [ line.removeprefix("audit finding ") @@ -241,7 +265,7 @@ class NpmAuditRetryTest(unittest.TestCase): self.assertNotIn("\n::error::injected", result.stdout) self.assertNotIn("\x1b", result.stdout) - def test_package_findings_fail_even_when_summary_is_missing_or_zero(self) -> None: + def test_package_findings_count_even_when_summary_is_missing_or_zero(self) -> None: for metadata in [None, {}, {"vulnerabilities": {"total": 0}}]: for require in ["true", "false"]: with self.subTest(metadata=metadata, require=require): @@ -255,30 +279,35 @@ class NpmAuditRetryTest(unittest.TestCase): result, calls = self.run_check( "captured-vulnerability", "all", require=require, report=report ) - self.assertEqual(result.returncode, 1, result.stdout) + self.assertEqual( + result.returncode, 1 if require == "true" else 0, result.stdout + ) self.assertEqual(calls, ["audit --json"], result.stdout) self.assertIn("vulnerabilities present", result.stdout) self.assertIn("package_records=1", result.stdout) self.assertNotIn("no vulnerabilities", result.stdout) - def test_positive_severity_counts_fail_without_a_consistent_total(self) -> None: + def test_positive_severity_counts_count_without_a_consistent_total(self) -> None: for severity in ["info", "low", "moderate", "high", "critical"]: for total in [None, 0]: - with self.subTest(severity=severity, total=total): - counts = {severity: 1} - if total is not None: - counts["total"] = total - report = { - "metadata": {"vulnerabilities": counts}, - "error": {"code": "ETIMEDOUT"}, - } - result, calls = self.run_check( - "captured-vulnerability", "production", - require="false", report=report, - ) - self.assertEqual(result.returncode, 1, result.stdout) - self.assertEqual(calls, ["audit --json --omit=dev"], result.stdout) - self.assertIn("vulnerabilities present", result.stdout) + for require in ["true", "false"]: + with self.subTest(severity=severity, total=total, require=require): + counts = {severity: 1} + if total is not None: + counts["total"] = total + report = { + "metadata": {"vulnerabilities": counts}, + "error": {"code": "ETIMEDOUT"}, + } + result, calls = self.run_check( + "captured-vulnerability", "production", + require=require, report=report, + ) + self.assertEqual( + result.returncode, 1 if require == "true" else 0, result.stdout + ) + self.assertEqual(calls, ["audit --json --omit=dev"], result.stdout) + self.assertIn("vulnerabilities present", result.stdout) def test_malformed_zero_verdicts_cannot_pass_as_clean(self) -> None: zero_counts = dict.fromkeys( @@ -340,10 +369,12 @@ class NpmAuditRetryTest(unittest.TestCase): self.assertIn("no vulnerabilities", result.stdout) def test_persistent_outage_fails_when_a_result_is_required(self) -> None: - result, calls = self.run_check("transient-failure", "all") - self.assertEqual(result.returncode, 1) - self.assertEqual(calls, ["audit --json"] * 3) - self.assertIn("could not reach", result.stdout) + for require in ["true", "", "False"]: + with self.subTest(require=require): + result, calls = self.run_check("transient-failure", "all", require=require) + self.assertEqual(result.returncode, 1) + self.assertEqual(calls, ["audit --json"] * 3) + self.assertIn("could not reach", result.stdout) def test_persistent_outage_warns_for_an_unchanged_dependency_graph(self) -> None: result, calls = self.run_check(