Warn on inherited npm advisories when the lockfile is unchanged (#2442)

Since 3 October the required Frontend check has failed on every open
pull request, including Go-only ones, because of GHSA-vfj7-8cjw-p6xm
(`braces` <= 3.0.3, high, no patched release). `braces` reaches the
frontend graph only through dev-only `tailwindcss` 3 and `jscpd` 4. `npm
audit --omit=dev` is clean, so the shipped bundle is not affected.

A pull request that leaves `package.json` and `package-lock.json`
untouched has the same dependency graph as its base commit. It can
neither add a finding nor remove one, so failing it blocks every fix
without making anything safer.

## What changes

`scripts/npm-audit-retry.sh` now treats findings the same way it already
treated an unreachable advisory endpoint:

- **The change moves the dependency graph, or edits the runner**
(`NPM_AUDIT_REQUIRE_RESULT=true`): any finding at any severity fails, as
before. A missing answer also fails.
- **The graph is the base commit's** (`NPM_AUDIT_REQUIRE_RESULT=false`):
the run warns instead, still naming every finding. A missing answer also
only warns, as before.
- **Only an explicit `false` relaxes the verdict.** An empty or
unrecognised value stays strict. This also tightens the outage path,
which previously relaxed on any value other than `true`.

The scheduled `npm-audit` scan and the daily dependency advisory watch
keep the strict default. An inherited advisory therefore still fails
once a day until the graph is fixed; `test_dependency_advisory_watch.py`
now proves this. No severity threshold, allowlist or suppression is
added.

## Landing

This change edits the runner, so its own Frontend audit runs in strict
mode and fails on this same advisory. It cannot pass the required
checks, so it lands only by an explicit maintainer merge. That is
intentional: the policy is relaxed by a human decision, never by a
change passing its own check.

Removing `braces` for real means migrating to Tailwind 4 and jscpd
5.4.0; that is tracked separately. Dependency PRs (#2352, #2278) stay
blocked until that lands.

## Verification

- `scripts/tests/test-npm-audit-retry.sh`: 19 cases pass.
- `test_npm_audit_retry.py` and `test_dependency_advisory_watch.py`
pass.
- `TestFrontendDependencySecurityAuditsAreRequired` passes.
- `check_workflow_trust.py` passes.
- The real runner against main's lockfile exits 0 with the warning and
all 7 findings named when `NPM_AUDIT_REQUIRE_RESULT=false`, and exits 1
with `::error::` when it is `true`.

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

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

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

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):

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(