From 44ca02d3d4186af2f719ba9b73d180b4e2791817 Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Fri, 4 Sep 2026 03:51:20 +0100 Subject: [PATCH] Keep npm audit outages from erasing CI evidence Retry only explicit registry failures with bounded one-minute attempts while preserving immediate advisory failures. Defer the aggregate audit verdict so frontend tests and builds still report during npm service incidents. --- .github/workflows/README.md | 6 + .github/workflows/build-and-test.yml | 26 ++- .github/workflows/security-scan.yml | 20 +- .../subsystems/deployment-installability.md | 13 ++ .../installtests/build_release_assets_test.go | 44 ++++- scripts/npm-audit-retry.sh | 42 ++++ scripts/tests/test_dependabot_config.py | 12 +- scripts/tests/test_npm_audit_retry.py | 185 ++++++++++++++++++ 8 files changed, 336 insertions(+), 12 deletions(-) create mode 100755 scripts/npm-audit-retry.sh create mode 100755 scripts/tests/test_npm_audit_retry.py diff --git a/.github/workflows/README.md b/.github/workflows/README.md index e404963bf..67c6d0c84 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -105,6 +105,12 @@ failed release-trust check still permits activation-marker inspection when the tag, numeric release ID, and exact source SHA are structurally valid. This 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 use one-minute registry attempts and retry +only explicit network or audit-endpoint failures. Advisory findings still fail +without retry, and three unavailable registry responses remain a failed check. +Audit steps defer their aggregate verdict so an unavailable advisory endpoint +cannot suppress independent frontend checks or the production bundle build; +the preceding clean install disables npm's duplicate best-effort audit request. Activation inspection also requires exactly one uploaded marker and compares the downloaded byte count and SHA-256 value with GitHub's release-asset metadata, so a valid-looking JSON response cannot silently replace or truncate diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 70c20b572..a92debd99 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -127,15 +127,21 @@ jobs: - name: Install frontend dependencies working-directory: frontend-modern - run: npm ci + # The explicit bounded audits below own the advisory verdict. Avoid + # npm ci's duplicate best-effort audit POST and its five-minute timeout. + run: npm ci --no-audit - name: Audit complete frontend dependency graph + id: audit-complete + continue-on-error: true working-directory: frontend-modern - run: npm audit + run: '"${GITHUB_WORKSPACE}/scripts/npm-audit-retry.sh"' - name: Audit production frontend dependencies + id: audit-production + continue-on-error: true working-directory: frontend-modern - run: npm audit --omit=dev + run: '"${GITHUB_WORKSPACE}/scripts/npm-audit-retry.sh" --omit=dev' # Whole-tree, not staged-only: the pre-commit formatter only ever sees # staged files, so drift in untouched files is invisible to it. This is @@ -171,6 +177,20 @@ jobs: working-directory: frontend-modern run: npm run check:bundlesize + # Audit service failures and real advisories stay gating, but neither may + # suppress formatting, tests, type-checking, or the production build. + - name: Require frontend dependency audits + if: ${{ !cancelled() }} + env: + COMPLETE_AUDIT_RESULT: ${{ steps.audit-complete.outcome }} + PRODUCTION_AUDIT_RESULT: ${{ steps.audit-production.outcome }} + run: | + if [ "${COMPLETE_AUDIT_RESULT}" != success ] || \ + [ "${PRODUCTION_AUDIT_RESULT}" != success ]; then + echo "::error::One or more frontend dependency audits failed." + exit 1 + fi + backend: name: Backend tests (${{ matrix.shard }}) needs: changes diff --git a/.github/workflows/security-scan.yml b/.github/workflows/security-scan.yml index b63805ed7..ffef28e83 100644 --- a/.github/workflows/security-scan.yml +++ b/.github/workflows/security-scan.yml @@ -383,9 +383,25 @@ jobs: node-version: '24' - name: Audit complete dependency graph + id: audit-complete + continue-on-error: true working-directory: ${{ matrix.directory }} - run: npm audit --package-lock-only + run: '"${GITHUB_WORKSPACE}/scripts/npm-audit-retry.sh" --package-lock-only' - name: Audit production dependencies + id: audit-production + continue-on-error: true working-directory: ${{ matrix.directory }} - run: npm audit --package-lock-only --omit=dev + run: '"${GITHUB_WORKSPACE}/scripts/npm-audit-retry.sh" --package-lock-only --omit=dev' + + - name: Require dependency audits + if: ${{ !cancelled() }} + env: + COMPLETE_AUDIT_RESULT: ${{ steps.audit-complete.outcome }} + PRODUCTION_AUDIT_RESULT: ${{ steps.audit-production.outcome }} + run: | + if [ "${COMPLETE_AUDIT_RESULT}" != success ] || \ + [ "${PRODUCTION_AUDIT_RESULT}" != success ]; then + echo "::error::One or more dependency audits failed." + exit 1 + fi diff --git a/docs/release-control/v6/internal/subsystems/deployment-installability.md b/docs/release-control/v6/internal/subsystems/deployment-installability.md index c16c0f3ab..a6ba37fc2 100644 --- a/docs/release-control/v6/internal/subsystems/deployment-installability.md +++ b/docs/release-control/v6/internal/subsystems/deployment-installability.md @@ -2186,6 +2186,19 @@ artifact-selection behaviour. `scripts/trigger-release.sh` and `scripts/trigger-stable-patch.sh` must send the exact remote candidate SHA they already verified; branch ancestry or a later branch tip is not equivalent release admission. +18. Keep frontend dependency security checks fail-closed without discarding + independent build evidence. Push-time and scheduled complete/production + dependency audits may retry only explicit registry transport or endpoint + failures, with a bounded request timeout and attempt count; an advisory + finding must fail immediately, and exhausted service failures must remain a + failed check. In `.github/workflows/build-and-test.yml`, the aggregate audit + verdict must run after formatting, lint, tests, type-checking, the production + build, and the bundle-size check so an unavailable advisory service cannot + suppress those results. The preceding `npm ci` must disable its duplicate + best-effort audit request because the explicit checks own the security + verdict. Keep this boundary covered in + `scripts/installtests/build_release_assets_test.go` whenever its workflow or + helper wiring changes. ## Current State diff --git a/scripts/installtests/build_release_assets_test.go b/scripts/installtests/build_release_assets_test.go index bb9523f39..323adcf3c 100644 --- a/scripts/installtests/build_release_assets_test.go +++ b/scripts/installtests/build_release_assets_test.go @@ -3640,12 +3640,48 @@ func TestReleasePipelinePromotesOneImmutableCandidate(t *testing.T) { } func TestFrontendDependencySecurityAuditsAreRequired(t *testing.T) { - workflowPath := repoFile(".github", "workflows", "build-and-test.yml") - assertFileContainsAll(t, workflowPath, + buildWorkflowPath := repoFile(".github", "workflows", "build-and-test.yml") + buildWorkflowBytes, err := os.ReadFile(buildWorkflowPath) + if err != nil { + t.Fatalf("read build-and-test workflow: %v", err) + } + frontendJob := workflowJobBlock(t, string(buildWorkflowBytes), "frontend") + for _, needle := range []string{ + `run: npm ci --no-audit`, `- name: Audit complete frontend dependency graph`, - `run: npm audit`, + `run: '"${GITHUB_WORKSPACE}/scripts/npm-audit-retry.sh"'`, `- name: Audit production frontend dependencies`, - `run: npm audit --omit=dev`, + `run: '"${GITHUB_WORKSPACE}/scripts/npm-audit-retry.sh" --omit=dev'`, + `continue-on-error: true`, + `- name: Require frontend dependency audits`, + `if: ${{ !cancelled() }}`, + `COMPLETE_AUDIT_RESULT: ${{ steps.audit-complete.outcome }}`, + `PRODUCTION_AUDIT_RESULT: ${{ steps.audit-production.outcome }}`, + } { + if !strings.Contains(frontendJob, needle) { + t.Fatalf("build-and-test frontend job missing dependency audit contract: %s", needle) + } + } + if verdictIndex, bundleIndex := strings.Index(frontendJob, "- name: Require frontend dependency audits"), strings.Index(frontendJob, "- name: Check frontend bundle size budget"); verdictIndex < bundleIndex || bundleIndex < 0 { + t.Fatal("build-and-test must report independent frontend evidence before requiring audit success") + } + + securityWorkflowPath := repoFile(".github", "workflows", "security-scan.yml") + assertFileContainsAll(t, securityWorkflowPath, + `run: '"${GITHUB_WORKSPACE}/scripts/npm-audit-retry.sh" --package-lock-only'`, + `run: '"${GITHUB_WORKSPACE}/scripts/npm-audit-retry.sh" --package-lock-only --omit=dev'`, + `- name: Require dependency audits`, + `COMPLETE_AUDIT_RESULT: ${{ steps.audit-complete.outcome }}`, + `PRODUCTION_AUDIT_RESULT: ${{ steps.audit-production.outcome }}`, + ) + + auditHelperPath := repoFile("scripts", "npm-audit-retry.sh") + assertFileContainsAll(t, auditHelperPath, + `readonly max_attempts=3`, + `readonly fetch_timeout_ms=60000`, + `npm audit --fetch-timeout="${fetch_timeout_ms}" "$@"`, + `if ! grep -Eiq`, + `if (( attempt == max_attempts )); then`, ) } diff --git a/scripts/npm-audit-retry.sh b/scripts/npm-audit-retry.sh new file mode 100755 index 000000000..94c348768 --- /dev/null +++ b/scripts/npm-audit-retry.sh @@ -0,0 +1,42 @@ +#!/usr/bin/env bash +# Retry transient npm audit service failures without hiding vulnerability findings. + +set -euo pipefail + +readonly max_attempts=3 +readonly fetch_timeout_ms=60000 +attempt=1 +output="$(mktemp)" +trap 'rm -f -- "${output}"' EXIT + +while (( attempt <= max_attempts )); do + : >"${output}" + set +e + npm audit --fetch-timeout="${fetch_timeout_ms}" "$@" 2>&1 | tee "${output}" + status=${PIPESTATUS[0]} + set -e + + if (( status == 0 )); then + exit 0 + fi + + # npm audit uses a registry POST. npm's fetch retries cover idempotent reads, + # so a transient timeout or audit endpoint error otherwise consumes the full + # default five-minute timeout and fails the check without another attempt. + # A real advisory report is not an infrastructure error and must remain an + # immediate, fail-closed result. + if ! grep -Eiq \ + 'npm (warn|error) audit (network|endpoint|429 |5[0-9]{2} )|npm error (code )?(EAI_AGAIN|ECONNRESET|ECONNREFUSED|ENETUNREACH|ETIMEDOUT|ERR_SOCKET_TIMEOUT|FETCH_ERROR)|npm error network|npm error request to .* failed' \ + "${output}"; then + exit "${status}" + fi + + if (( attempt == max_attempts )); then + echo "npm audit registry request failed after ${max_attempts} attempts." >&2 + exit "${status}" + fi + + echo "npm audit registry request failed on attempt ${attempt}; retrying." >&2 + sleep 5 + ((attempt += 1)) +done diff --git a/scripts/tests/test_dependabot_config.py b/scripts/tests/test_dependabot_config.py index 63b990ae5..976e04755 100644 --- a/scripts/tests/test_dependabot_config.py +++ b/scripts/tests/test_dependabot_config.py @@ -155,12 +155,18 @@ class DependabotConfigTest(unittest.TestCase): }, ) scan_steps = jobs["npm-audit"]["steps"] - audit_commands = [step["run"] for step in scan_steps if "run" in step] + audit_commands = [ + step["run"] + for step in scan_steps + if step.get("id") in {"audit-complete", "audit-production"} + ] self.assertEqual( audit_commands, [ - "npm audit --package-lock-only", - "npm audit --package-lock-only --omit=dev", + '"${GITHUB_WORKSPACE}/scripts/npm-audit-retry.sh" ' + "--package-lock-only", + '"${GITHUB_WORKSPACE}/scripts/npm-audit-retry.sh" ' + "--package-lock-only --omit=dev", ], ) diff --git a/scripts/tests/test_npm_audit_retry.py b/scripts/tests/test_npm_audit_retry.py new file mode 100755 index 000000000..3b5a96569 --- /dev/null +++ b/scripts/tests/test_npm_audit_retry.py @@ -0,0 +1,185 @@ +#!/usr/bin/env python3 + +from __future__ import annotations + +import os +from pathlib import Path +import subprocess +import tempfile +import textwrap +import unittest + +import yaml + + +ROOT = Path(__file__).resolve().parents[2] +SCRIPT = ROOT / "scripts" / "npm-audit-retry.sh" + + +class NpmAuditRetryTest(unittest.TestCase): + def run_check(self, mode: str, *arguments: str): + with tempfile.TemporaryDirectory() as directory: + fake_bin = Path(directory) + count = fake_bin / "count" + calls = fake_bin / "calls" + sleep_calls = fake_bin / "sleep-calls" + count.write_text("0\n", encoding="utf-8") + fake_npm = fake_bin / "npm" + fake_npm.write_text( + textwrap.dedent( + """\ + #!/bin/sh + count=$(cat "$FAKE_NPM_COUNT") + count=$((count + 1)) + printf '%s\n' "$count" > "$FAKE_NPM_COUNT" + printf '%s\n' "$*" >> "$FAKE_NPM_CALLS" + case "$FAKE_NPM_MODE" in + success) + echo 'found 0 vulnerabilities' + exit 0 + ;; + transient-success) + if [ "$count" -eq 1 ]; then + echo 'npm warn audit network timeout at: https://registry.npmjs.org/-/npm/v1/security/advisories/bulk' + echo 'npm error audit endpoint returned an error' + exit 1 + fi + echo 'found 0 vulnerabilities' + exit 0 + ;; + transient-failure) + echo 'npm error code ETIMEDOUT' + echo 'npm error audit endpoint returned an error' + exit 42 + ;; + vulnerability) + echo '# npm audit report' + echo 'example <2.0.0' + echo '1 high severity vulnerability' + exit 1 + ;; + esac + exit 64 + """ + ), + encoding="utf-8", + ) + fake_npm.chmod(0o755) + fake_sleep = fake_bin / "sleep" + fake_sleep.write_text( + "#!/bin/sh\nprintf '%s\\n' \"$*\" >> \"$FAKE_SLEEP_CALLS\"\n", + encoding="utf-8", + ) + fake_sleep.chmod(0o755) + env = os.environ.copy() + env.update( + { + "FAKE_NPM_CALLS": str(calls), + "FAKE_NPM_COUNT": str(count), + "FAKE_NPM_MODE": mode, + "FAKE_SLEEP_CALLS": str(sleep_calls), + "PATH": f"{directory}:{env['PATH']}", + } + ) + result = subprocess.run( + [str(SCRIPT), *arguments], + cwd=ROOT, + env=env, + text=True, + capture_output=True, + check=False, + ) + recorded_calls = ( + calls.read_text(encoding="utf-8").splitlines() + if calls.exists() + else [] + ) + recorded_sleeps = ( + sleep_calls.read_text(encoding="utf-8").splitlines() + if sleep_calls.exists() + else [] + ) + return result, recorded_calls, recorded_sleeps + + def test_passes_a_clean_audit_without_retry(self) -> None: + result, calls, sleeps = self.run_check("success", "--omit=dev") + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(calls, ["audit --fetch-timeout=60000 --omit=dev"]) + self.assertEqual(sleeps, []) + + def test_retries_a_transient_audit_endpoint_failure(self) -> None: + result, calls, sleeps = self.run_check( + "transient-success", "--package-lock-only" + ) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual( + calls, + [ + "audit --fetch-timeout=60000 --package-lock-only", + "audit --fetch-timeout=60000 --package-lock-only", + ], + ) + self.assertEqual(sleeps, ["5"]) + self.assertIn("retrying", result.stderr) + + def test_does_not_retry_a_vulnerability_report(self) -> None: + result, calls, sleeps = self.run_check("vulnerability") + self.assertEqual(result.returncode, 1) + self.assertEqual(calls, ["audit --fetch-timeout=60000"]) + self.assertEqual(sleeps, []) + self.assertIn("1 high severity vulnerability", result.stdout) + self.assertNotIn("retrying", result.stderr) + + def test_persistent_registry_failure_remains_fatal(self) -> None: + result, calls, sleeps = self.run_check("transient-failure") + self.assertEqual(result.returncode, 42) + self.assertEqual(len(calls), 3) + self.assertEqual(sleeps, ["5", "5"]) + self.assertIn("failed after 3 attempts", result.stderr) + + def test_all_workflow_audits_use_the_retry_boundary(self) -> None: + for relative, job_name in ( + (".github/workflows/build-and-test.yml", "frontend"), + (".github/workflows/security-scan.yml", "npm-audit"), + ): + with self.subTest(workflow=relative): + workflow = yaml.safe_load( + (ROOT / relative).read_text(encoding="utf-8") + ) + steps = workflow["jobs"][job_name]["steps"] + if job_name == "frontend": + install = next( + step + for step in steps + if step.get("name") == "Install frontend dependencies" + ) + self.assertEqual(install["run"], "npm ci --no-audit") + audits = [ + step + for step in steps + if step.get("id") in {"audit-complete", "audit-production"} + ] + self.assertEqual(len(audits), 2) + self.assertTrue( + all( + step["run"].startswith( + '"${GITHUB_WORKSPACE}/scripts/npm-audit-retry.sh"' + ) + and step.get("continue-on-error") is True + for step in audits + ) + ) + verdict = steps[-1] + self.assertIn("Require", verdict["name"]) + self.assertEqual(verdict["if"], "${{ !cancelled() }}") + self.assertEqual( + verdict["env"], + { + "COMPLETE_AUDIT_RESULT": "${{ steps.audit-complete.outcome }}", + "PRODUCTION_AUDIT_RESULT": "${{ steps.audit-production.outcome }}", + }, + ) + + +if __name__ == "__main__": + unittest.main()