diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 70c20b572..62c2bfdf2 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -71,6 +71,7 @@ jobs: timeout-minutes: 5 outputs: code: ${{ steps.filter.outputs.code }} + frontend_deps: ${{ steps.filter.outputs.frontend_deps }} steps: - name: Checkout repository uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -88,8 +89,9 @@ jobs: base="${{ github.event.before }}" fi if [ -z "$base" ] || [ "$base" = "0000000000000000000000000000000000000000" ] || ! git cat-file -e "$base" 2>/dev/null; then - echo "Base commit unavailable; assuming code changed." + echo "Base commit unavailable; assuming code and dependencies changed." echo "code=true" >> "$GITHUB_OUTPUT" + echo "frontend_deps=true" >> "$GITHUB_OUTPUT" exit 0 fi changed=$(git diff --name-only "$base" "${{ github.sha }}") @@ -102,6 +104,17 @@ jobs: code=false 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. + if printf '%s\n' "$changed" | grep -qE '^frontend-modern/package(-lock)?\.json$|^scripts/npm-audit-retry\.sh$'; then + frontend_deps=true + else + frontend_deps=false + fi + echo "frontend_deps=$frontend_deps" >> "$GITHUB_OUTPUT" frontend: name: Frontend @@ -129,13 +142,20 @@ jobs: working-directory: frontend-modern run: npm ci + # 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. - name: Audit complete frontend dependency graph working-directory: frontend-modern - run: npm audit + env: + NPM_AUDIT_REQUIRE_RESULT: ${{ needs.changes.outputs.frontend_deps }} + run: bash "$GITHUB_WORKSPACE/scripts/npm-audit-retry.sh" all - name: Audit production frontend dependencies working-directory: frontend-modern - run: npm audit --omit=dev + env: + NPM_AUDIT_REQUIRE_RESULT: ${{ needs.changes.outputs.frontend_deps }} + run: bash "$GITHUB_WORKSPACE/scripts/npm-audit-retry.sh" production # 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 diff --git a/docs/release-control/v6/internal/subsystems/deployment-installability.md b/docs/release-control/v6/internal/subsystems/deployment-installability.md index 2ad16bccf..c3eaa1afc 100644 --- a/docs/release-control/v6/internal/subsystems/deployment-installability.md +++ b/docs/release-control/v6/internal/subsystems/deployment-installability.md @@ -4004,7 +4004,24 @@ Frontend dependency-security changes use their own proof route rather than borrowing the local dev-runtime orchestration tests. The canonical `.github/workflows/build-and-test.yml` frontend job must run both the complete `npm audit` and the production-only `npm audit --omit=dev` after a clean -install. `frontend-modern/src/security/__tests__/dependencySecurity.test.ts` +install, through `scripts/npm-audit-retry.sh`. 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, so a +severity threshold must never be introduced; only an unreachable endpoint is +retried. When retries are exhausted, 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, +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 and that an unparseable or unrecognised report is never read as +clean. `frontend-modern/src/security/__tests__/dependencySecurity.test.ts` pins the known safe floors for advisories remediated by commit `6ba85a185`, including DOMPurify `GHSA-55q2-fjhq-7xh7`, brace-expansion `GHSA-mh99-v99m-4gvg` and `GHSA-rgw5-rvv9-x895`, and nanoid diff --git a/scripts/installtests/build_release_assets_test.go b/scripts/installtests/build_release_assets_test.go index ccc452c0f..65d25c4b9 100644 --- a/scripts/installtests/build_release_assets_test.go +++ b/scripts/installtests/build_release_assets_test.go @@ -3643,9 +3643,28 @@ func TestFrontendDependencySecurityAuditsAreRequired(t *testing.T) { workflowPath := repoFile(".github", "workflows", "build-and-test.yml") assertFileContainsAll(t, workflowPath, `- name: Audit complete frontend dependency graph`, - `run: npm audit`, + `npm-audit-retry.sh" all`, `- name: Audit production frontend dependencies`, - `run: npm audit --omit=dev`, + `npm-audit-retry.sh" production`, + // The runner may retry an unreachable advisory endpoint, but only a + // change that leaves the dependency graph untouched may proceed + // without a fresh result. + `NPM_AUDIT_REQUIRE_RESULT: ${{ needs.changes.outputs.frontend_deps }}`, + `frontend_deps: ${{ steps.filter.outputs.frontend_deps }}`, + ) + // 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, and any vulnerability total must still fail. + runnerPath := repoFile("scripts", "npm-audit-retry.sh") + runner, err := os.ReadFile(runnerPath) + if err != nil { + t.Fatalf("read %s: %v", runnerPath, err) + } + if strings.Contains(string(runner), "--audit-level") { + t.Fatalf("%s must not weaken the audit with a severity threshold", runnerPath) + } + assertFileContainsAll(t, runnerPath, + `print("vulnerable" if total else "clean")`, ) } diff --git a/scripts/npm-audit-retry.sh b/scripts/npm-audit-retry.sh new file mode 100755 index 000000000..a28cef045 --- /dev/null +++ b/scripts/npm-audit-retry.sh @@ -0,0 +1,142 @@ +#!/usr/bin/env bash +# npm-audit-retry.sh — Run npm audit, separating a real advisory from an +# unreachable advisory endpoint. +# +# Usage: scripts/npm-audit-retry.sh +# +# `npm audit` exits 1 both when it finds vulnerabilities and when it cannot +# reach registry.npmjs.org. Treating those the same made a required check +# depend on npm's availability: on 2026-09-03 the advisory bulk endpoint +# 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: +# +# * a conclusive answer is acted on immediately, pass or fail; +# * 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. +# +# That last 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. +# +# Env: +# NPM_AUDIT_ATTEMPTS attempts before giving up (default 3) +# NPM_AUDIT_RETRY_DELAY seconds before the first retry, doubled each +# time (default 15) +# NPM_AUDIT_REQUIRE_RESULT "true" to fail when no answer was obtained +# (default true — the safe default) +# NPM_AUDIT_CMD npm executable to invoke (test seam) + +set -uo pipefail + +SCOPE="${1:-}" +case "${SCOPE}" in + all) SCOPE_ARGS=() ;; + production) SCOPE_ARGS=(--omit=dev) ;; + *) + echo "Usage: $0 " >&2 + exit 2 + ;; +esac + +ATTEMPTS="${NPM_AUDIT_ATTEMPTS:-3}" +DELAY="${NPM_AUDIT_RETRY_DELAY:-15}" +REQUIRE_RESULT="${NPM_AUDIT_REQUIRE_RESULT:-true}" +NPM_BIN="${NPM_AUDIT_CMD:-npm}" + +# Classify one audit run. Prints a verdict word on stdout: +# clean — audit completed, no vulnerabilities +# vulnerable — audit completed, vulnerabilities present +# unreachable — npm could not get an answer from the advisory endpoint +classify_report() { + python3 -c ' +import json, sys + +raw = sys.stdin.read().strip() +if not raw: + print("unreachable") + sys.exit(0) +try: + report = json.loads(raw) +except ValueError: + print("unreachable") + sys.exit(0) + +# npm reports an unusable audit endpoint as an error object, ENOAUDIT being +# the code it uses for 5xx, timeouts and offline runs alike. +if isinstance(report, dict) and report.get("error"): + print("unreachable") + sys.exit(0) + +meta = report.get("metadata") if isinstance(report, dict) else None +vulns = meta.get("vulnerabilities") if isinstance(meta, dict) else None +if not isinstance(vulns, dict) or "total" not in vulns: + # No usable verdict in the payload: treat as unreachable rather than + # silently passing on a shape we do not understand. + print("unreachable") + sys.exit(0) + +total = vulns.get("total", 0) +detail = " ".join( + f"{name}={vulns.get(name, 0)}" + for name in ("critical", "high", "moderate", "low", "info") +) +print("vulnerable" if total else "clean") +print(f"total={total} {detail}") +' +} + +report_file="$(mktemp)" +trap 'rm -f "${report_file}"' EXIT + +attempt=1 +delay="${DELAY}" +while [ "${attempt}" -le "${ATTEMPTS}" ]; do + echo "npm audit (${SCOPE}) attempt ${attempt}/${ATTEMPTS}" + "${NPM_BIN}" audit --json "${SCOPE_ARGS[@]}" >"${report_file}" 2>/dev/null + verdict_output="$(classify_report <"${report_file}")" + verdict="$(printf '%s\n' "${verdict_output}" | head -1)" + summary="$(printf '%s\n' "${verdict_output}" | sed -n '2p')" + + case "${verdict}" in + clean) + echo "npm audit (${SCOPE}): no vulnerabilities (${summary})" + exit 0 + ;; + vulnerable) + echo "npm audit (${SCOPE}): vulnerabilities present (${summary})" + echo "::error::npm audit (${SCOPE}) found vulnerabilities: ${summary}" + # Re-run without --json so the log carries the human-readable advisory + # detail a maintainer needs to act on. + "${NPM_BIN}" audit "${SCOPE_ARGS[@]}" || true + exit 1 + ;; + *) + echo "npm audit (${SCOPE}): advisory endpoint did not return a usable result" + ;; + esac + + if [ "${attempt}" -lt "${ATTEMPTS}" ]; then + echo "retrying in ${delay}s" + sleep "${delay}" + delay=$((delay * 2)) + fi + attempt=$((attempt + 1)) +done + +if [ "${REQUIRE_RESULT}" = "true" ]; then + echo "::error::npm audit (${SCOPE}) could not reach the advisory endpoint after ${ATTEMPTS} attempts, 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 after ${ATTEMPTS} attempts. 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." +exit 0 diff --git a/scripts/tests/test-npm-audit-retry.sh b/scripts/tests/test-npm-audit-retry.sh new file mode 100755 index 000000000..df7da2a33 --- /dev/null +++ b/scripts/tests/test-npm-audit-retry.sh @@ -0,0 +1,124 @@ +#!/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. + +set -euo pipefail + +ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +SCRIPT="${ROOT_DIR}/scripts/npm-audit-retry.sh" + +WORK_DIR="$(mktemp -d)" +trap 'rm -rf "${WORK_DIR}"' EXIT + +failures=0 + +# Build a fake npm that emits a canned payload per invocation. Each line of +# the mode list is used for one successive call, so retry behaviour is +# observable. +make_fake_npm() { + local name="$1" + shift + local path="${WORK_DIR}/${name}" + { + printf '#!/usr/bin/env bash\n' + printf 'count_file="%s/${0##*/}.count"\n' "${WORK_DIR}" + printf 'n=$(cat "$count_file" 2>/dev/null || echo 0)\n' + printf 'n=$((n + 1))\n' + printf 'printf "%%s" "$n" > "$count_file"\n' + printf 'case "$n" in\n' + local i=1 + for payload in "$@"; do + printf ' %d) cat <<'"'"'JSON'"'"'\n%s\nJSON\n ;;\n' "${i}" "${payload}" + i=$((i + 1)) + done + printf ' *) cat <<'"'"'JSON'"'"'\n%s\nJSON\n ;;\n' "${!#}" + printf 'esac\n' + printf 'exit 1\n' + } > "${path}" + chmod +x "${path}" + printf '%s' "${path}" +} + +CLEAN='{"metadata":{"vulnerabilities":{"info":0,"low":0,"moderate":0,"high":0,"critical":0,"total":0}}}' +VULN='{"metadata":{"vulnerabilities":{"info":0,"low":0,"moderate":1,"high":0,"critical":0,"total":1}}}' +ENOAUDIT='{"error":{"code":"ENOAUDIT","summary":"503 Service Unavailable","detail":""}}' + +run_case() { + local desc="$1" expected_status="$2" npm_bin="$3" require="$4" + shift 4 + local out status + set +e + out="$(NPM_AUDIT_CMD="${npm_bin}" \ + NPM_AUDIT_RETRY_DELAY=0 \ + NPM_AUDIT_ATTEMPTS="${NPM_AUDIT_ATTEMPTS:-3}" \ + NPM_AUDIT_REQUIRE_RESULT="${require}" \ + bash "${SCRIPT}" all 2>&1)" + status=$? + set -e + if [ "${status}" != "${expected_status}" ]; then + echo "FAIL: ${desc} — exit ${status}, want ${expected_status}" + printf '%s\n' "${out}" | sed 's/^/ /' + failures=$((failures + 1)) + return + fi + for needle in "$@"; do + if ! printf '%s' "${out}" | grep -qF -- "${needle}"; then + echo "FAIL: ${desc} — output missing: ${needle}" + printf '%s\n' "${out}" | sed 's/^/ /' + failures=$((failures + 1)) + return + fi + done + echo "ok: ${desc}" +} + +# A clean audit passes on the first attempt. +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 transient endpoint failure that clears on retry passes. +run_case "retry recovers from a transient outage" 0 \ + "$(make_fake_npm npm-recover "${ENOAUDIT}" "${CLEAN}")" true \ + "did not return a usable result" "no vulnerabilities" + +# A vulnerability found only after a transient failure still fails. +run_case "retry surfacing a vulnerability fails" 1 \ + "$(make_fake_npm npm-late-vuln "${ENOAUDIT}" "${VULN}")" true \ + "vulnerabilities present" + +# A sustained outage fails when this change touches the dependency graph. +run_case "sustained outage fails when dependencies changed" 1 \ + "$(make_fake_npm npm-out-required "${ENOAUDIT}")" true \ + "could not reach the advisory endpoint" + +# A sustained outage warns but passes when the dependency graph is unchanged. +run_case "sustained outage warns when dependencies unchanged" 0 \ + "$(make_fake_npm npm-out-optional "${ENOAUDIT}")" false \ + "::warning::" "does not touch package.json" + +# Unparseable output is treated as unreachable, never as a pass. +run_case "garbage output is not treated as clean" 1 \ + "$(make_fake_npm npm-garbage "not json at all")" true \ + "could not reach the advisory endpoint" + +# An empty report is likewise not a pass. +run_case "empty output is not treated as clean" 1 \ + "$(make_fake_npm npm-empty "")" true \ + "could not reach the advisory endpoint" + +# An unknown payload shape must not pass silently. +run_case "unknown payload shape is not treated as clean" 1 \ + "$(make_fake_npm npm-shape '{"metadata":{}}')" true \ + "could not reach the advisory endpoint" + +if [ "${failures}" -ne 0 ]; then + echo "${failures} test(s) failed" + exit 1 +fi +echo "all npm-audit-retry tests passed"