Warn on inherited npm advisories when the lockfile is unchanged

GHSA-vfj7-8cjw-p6xm (braces <= 3.0.3, no patched release) reached the frontend graph only through dev-only tailwindcss 3 and jscpd 4, and from 3 October it failed the required Frontend check on every open pull request, Go-only ones included. The production audit is clean, and a change that leaves package.json and package-lock.json untouched has the base commit's graph, so it can neither add a finding nor remove one. Failing it blocked every fix and made nothing safer. The runner now applies the split it already used for an unreachable endpoint to findings too. A change that moves the graph, or edits the runner, still fails on any finding at any severity. A change whose graph is the base commit's warns, naming every finding. Only an explicit NPM_AUDIT_REQUIRE_RESULT=false selects that mode; an empty or unrecognised value, as a missing workflow output would give, stays strict, which also tightens the outage path. The scheduled npm-audit scan and the daily dependency advisory watch keep the strict default, so an inherited advisory still fails somewhere every day until the dependency graph is fixed. No severity threshold, allowlist or suppression is added.
2026-10-24 04:33:17 +00:00 · 2026-10-03 12:08:37 +01:00
parent 74fd189fd0
commit 237130a2cb
9 changed files with 215 additions and 82 deletions
@@ -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<major>.<minor>` 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
@@ -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
@@ -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
@@ -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`,
@@ -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
@@ -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
@@ -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
@@ -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):
@@ -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(