diff --git a/.github/workflows/README.md b/.github/workflows/README.md index ed901df9f..485e156bf 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -105,12 +105,15 @@ 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. +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 +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. 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 a92debd99..155b7d24a 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 @@ -131,17 +144,24 @@ jobs: # npm ci's duplicate best-effort audit POST and its five-minute timeout. 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. - name: Audit complete frontend dependency graph id: audit-complete continue-on-error: true working-directory: frontend-modern - run: '"${GITHUB_WORKSPACE}/scripts/npm-audit-retry.sh"' + 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 id: audit-production continue-on-error: true working-directory: frontend-modern - run: '"${GITHUB_WORKSPACE}/scripts/npm-audit-retry.sh" --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/.github/workflows/security-scan.yml b/.github/workflows/security-scan.yml index ffef28e83..bfc74a0ca 100644 --- a/.github/workflows/security-scan.yml +++ b/.github/workflows/security-scan.yml @@ -386,13 +386,13 @@ jobs: id: audit-complete continue-on-error: true working-directory: ${{ matrix.directory }} - run: '"${GITHUB_WORKSPACE}/scripts/npm-audit-retry.sh" --package-lock-only' + run: bash "$GITHUB_WORKSPACE/scripts/npm-audit-retry.sh" all --package-lock-only - name: Audit production dependencies id: audit-production continue-on-error: true working-directory: ${{ matrix.directory }} - run: '"${GITHUB_WORKSPACE}/scripts/npm-audit-retry.sh" --package-lock-only --omit=dev' + run: bash "$GITHUB_WORKSPACE/scripts/npm-audit-retry.sh" production --package-lock-only - name: Require dependency audits if: ${{ !cancelled() }} diff --git a/docs/release-control/v6/internal/subsystems/deployment-installability.md b/docs/release-control/v6/internal/subsystems/deployment-installability.md index 09e5f5ad5..a25039c2e 100644 --- a/docs/release-control/v6/internal/subsystems/deployment-installability.md +++ b/docs/release-control/v6/internal/subsystems/deployment-installability.md @@ -4038,7 +4038,25 @@ 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, even +if the same response also carries a transport error, so a severity threshold +must never be introduced; only an unreachable endpoint is retried through +bounded one-minute attempts. 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 8f2de6ca8..e4de28935 100644 --- a/scripts/installtests/build_release_assets_test.go +++ b/scripts/installtests/build_release_assets_test.go @@ -3663,9 +3663,13 @@ func TestFrontendDependencySecurityAuditsAreRequired(t *testing.T) { for _, needle := range []string{ `run: npm ci --no-audit`, `- name: Audit complete frontend dependency graph`, - `run: '"${GITHUB_WORKSPACE}/scripts/npm-audit-retry.sh"'`, + `npm-audit-retry.sh" all`, `- name: Audit production frontend dependencies`, - `run: '"${GITHUB_WORKSPACE}/scripts/npm-audit-retry.sh" --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 }}`, `continue-on-error: true`, `- name: Require frontend dependency audits`, `if: ${{ !cancelled() }}`, @@ -3676,27 +3680,38 @@ func TestFrontendDependencySecurityAuditsAreRequired(t *testing.T) { t.Fatalf("build-and-test frontend job missing dependency audit contract: %s", needle) } } + assertFileContainsAll(t, buildWorkflowPath, + `frontend_deps: ${{ steps.filter.outputs.frontend_deps }}`, + ) 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'`, + `npm-audit-retry.sh" all --package-lock-only`, + `npm-audit-retry.sh" production --package-lock-only`, `- 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}" "$@"`, - `grep -Fq '# npm audit report'`, - `if ! grep -Eiq`, - `if (( attempt == max_attempts )); then`, + // 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, + `NPM_AUDIT_REQUIRE_RESULT:-true`, + `NPM_AUDIT_FETCH_TIMEOUT_MS:-60000`, + `AUDIT_ARGS=("$@")`, + `if isinstance(vulns, dict) and "total" in vulns:`, + `print("vulnerable" if total else "clean")`, ) } diff --git a/scripts/npm-audit-retry.sh b/scripts/npm-audit-retry.sh index 343848735..bf7715a72 100755 --- a/scripts/npm-audit-retry.sh +++ b/scripts/npm-audit-retry.sh @@ -1,48 +1,151 @@ #!/usr/bin/env bash -# Retry transient npm audit service failures without hiding vulnerability findings. +# npm-audit-retry.sh — Run npm audit, separating a real advisory from an +# unreachable advisory endpoint. +# +# Usage: scripts/npm-audit-retry.sh [npm-audit-args...] +# +# `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_FETCH_TIMEOUT_MS per-attempt npm fetch timeout (default 60000) +# 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 -euo pipefail +set -uo pipefail + +SCOPE="${1:-}" +case "${SCOPE}" in + all) SCOPE_ARGS=() ;; + production) SCOPE_ARGS=(--omit=dev) ;; + *) + echo "Usage: $0 [npm-audit-args...]" >&2 + exit 2 + ;; +esac +shift +AUDIT_ARGS=("$@") + +ATTEMPTS="${NPM_AUDIT_ATTEMPTS:-3}" +FETCH_TIMEOUT_MS="${NPM_AUDIT_FETCH_TIMEOUT_MS:-60000}" +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) + +meta = report.get("metadata") if isinstance(report, dict) else None +vulns = meta.get("vulnerabilities") if isinstance(meta, dict) else None +if isinstance(vulns, dict) and "total" in vulns: + # A usable advisory verdict takes precedence even if npm also includes a + # transport error. Never turn a real finding into a retryable outage. + 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}") + sys.exit(0) + +if isinstance(report, dict) and report.get("error"): + # npm reports an unusable endpoint as an error object, ENOAUDIT being the + # code it uses for 5xx, timeouts and offline runs alike. + print("unreachable") + sys.exit(0) + +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) + +' +} + +report_file="$(mktemp)" +trap 'rm -f "${report_file}"' EXIT -readonly max_attempts=3 -readonly fetch_timeout_ms=60000 attempt=1 -output="$(mktemp)" -trap 'rm -f -- "${output}"' EXIT +delay="${DELAY}" +while [ "${attempt}" -le "${ATTEMPTS}" ]; do + echo "npm audit (${SCOPE}) attempt ${attempt}/${ATTEMPTS}" + "${NPM_BIN}" audit --json --fetch-timeout="${FETCH_TIMEOUT_MS}" "${AUDIT_ARGS[@]}" "${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')" -while (( attempt <= max_attempts )); do - : >"${output}" - set +e - npm audit --fetch-timeout="${fetch_timeout_ms}" "$@" 2>&1 | tee "${output}" - status=${PIPESTATUS[0]} - set -e + 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 --fetch-timeout="${FETCH_TIMEOUT_MS}" "${AUDIT_ARGS[@]}" "${SCOPE_ARGS[@]}" || true + exit 1 + ;; + *) + echo "npm audit (${SCOPE}): advisory endpoint did not return a usable result" + ;; + esac - if (( status == 0 )); then - exit 0 + if [ "${attempt}" -lt "${ATTEMPTS}" ]; then + echo "retrying in ${delay}s" + sleep "${delay}" + delay=$((delay * 2)) fi - - # An advisory result takes precedence if npm also emits a transport warning. - # Never turn a real vulnerability finding into a retryable service failure. - if grep -Fq '# npm audit report' "${output}"; then - exit "${status}" - 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)) + 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" diff --git a/scripts/tests/test_npm_audit_retry.py b/scripts/tests/test_npm_audit_retry.py index c1e45c930..c366989f8 100755 --- a/scripts/tests/test_npm_audit_retry.py +++ b/scripts/tests/test_npm_audit_retry.py @@ -17,12 +17,11 @@ SCRIPT = ROOT / "scripts" / "npm-audit-retry.sh" class NpmAuditRetryTest(unittest.TestCase): - def run_check(self, mode: str, *arguments: str): + def run_check(self, mode: str, *arguments: str, require: str = "true"): 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( @@ -33,36 +32,37 @@ class NpmAuditRetryTest(unittest.TestCase): count=$((count + 1)) printf '%s\n' "$count" > "$FAKE_NPM_COUNT" printf '%s\n' "$*" >> "$FAKE_NPM_CALLS" + clean='{"metadata":{"vulnerabilities":{"info":0,"low":0,"moderate":0,"high":0,"critical":0,"total":0}}}' + unavailable='{"error":{"code":"ENOAUDIT","summary":"503 Service Unavailable"}}' + vulnerable='{"metadata":{"vulnerabilities":{"info":0,"low":0,"moderate":0,"high":1,"critical":0,"total":1}}}' + vulnerable_with_error='{"error":{"code":"ETIMEDOUT"},"metadata":{"vulnerabilities":{"info":0,"low":0,"moderate":0,"high":1,"critical":0,"total":1}}}' case "$FAKE_NPM_MODE" in success) - echo 'found 0 vulnerabilities' + printf '%s\n' "$clean" 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' + printf '%s\n' "$unavailable" exit 1 fi - echo 'found 0 vulnerabilities' + printf '%s\n' "$clean" exit 0 ;; transient-failure) - echo 'npm error code ETIMEDOUT' - echo 'npm error audit endpoint returned an error' + printf '%s\n' "$unavailable" exit 42 ;; vulnerability) - echo '# npm audit report' - echo 'example <2.0.0' - echo '1 high severity vulnerability' + printf '%s\n' "$vulnerable" exit 1 ;; - vulnerability-and-transient) - echo '# npm audit report' - echo 'example <2.0.0' - echo '1 high severity vulnerability' - echo 'npm error code ETIMEDOUT' + vulnerability-with-error) + printf '%s\n' "$vulnerable_with_error" + exit 1 + ;; + garbage) + echo 'not json' exit 1 ;; esac @@ -72,20 +72,16 @@ class NpmAuditRetryTest(unittest.TestCase): 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']}", + "NPM_AUDIT_ATTEMPTS": "3", + "NPM_AUDIT_CMD": str(fake_npm), + "NPM_AUDIT_REQUIRE_RESULT": require, + "NPM_AUDIT_RETRY_DELAY": "0", } ) result = subprocess.run( @@ -101,91 +97,111 @@ class NpmAuditRetryTest(unittest.TestCase): 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 + return result, recorded_calls - 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" + def test_passes_a_clean_production_audit_and_forwards_arguments(self) -> None: + result, calls = self.run_check( + "success", "production", "--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", - ], + ["audit --json --fetch-timeout=60000 --package-lock-only --omit=dev"], ) - self.assertEqual(sleeps, ["5"]) - self.assertIn("retrying", result.stderr) + + def test_retries_an_unavailable_audit_endpoint(self) -> None: + result, calls = self.run_check("transient-success", "all") + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual( + calls, + ["audit --json --fetch-timeout=60000"] * 2, + ) + self.assertIn("retrying", result.stdout) def test_does_not_retry_a_vulnerability_report(self) -> None: - result, calls, sleeps = self.run_check("vulnerability") + result, calls = self.run_check("vulnerability", "all") 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) + self.assertEqual( + calls, + [ + "audit --json --fetch-timeout=60000", + "audit --fetch-timeout=60000", + ], + ) + self.assertIn("vulnerabilities present", result.stdout) + self.assertNotIn("retrying", result.stdout) - def test_vulnerability_report_takes_precedence_over_transport_marker( - self, - ) -> None: - result, calls, sleeps = self.run_check("vulnerability-and-transient") + def test_vulnerability_verdict_precedes_a_transport_error(self) -> None: + result, calls = self.run_check("vulnerability-with-error", "all") 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) + self.assertEqual(len(calls), 2) + self.assertIn("vulnerabilities present", result.stdout) + self.assertNotIn("retrying", result.stdout) - 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_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 --fetch-timeout=60000"] * 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( + "transient-failure", "all", require="false" + ) + self.assertEqual(result.returncode, 0) + self.assertEqual(calls, ["audit --json --fetch-timeout=60000"] * 3) + self.assertIn("::warning::", result.stdout) + + def test_unparseable_output_never_passes_as_clean(self) -> None: + result, calls = self.run_check("garbage", "all") + self.assertEqual(result.returncode, 1) + self.assertEqual(calls, ["audit --json --fetch-timeout=60000"] * 3) + + def test_rejects_an_unknown_scope(self) -> None: + result, calls = self.run_check("success", "unknown") + self.assertEqual(result.returncode, 2) + self.assertEqual(calls, []) 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"), - ): + expected = { + ".github/workflows/build-and-test.yml": { + "job": "frontend", + "runs": [ + 'bash "$GITHUB_WORKSPACE/scripts/npm-audit-retry.sh" all', + 'bash "$GITHUB_WORKSPACE/scripts/npm-audit-retry.sh" production', + ], + "require_env": True, + }, + ".github/workflows/security-scan.yml": { + "job": "npm-audit", + "runs": [ + 'bash "$GITHUB_WORKSPACE/scripts/npm-audit-retry.sh" all --package-lock-only', + 'bash "$GITHUB_WORKSPACE/scripts/npm-audit-retry.sh" production --package-lock-only', + ], + "require_env": False, + }, + } + for relative, contract in expected.items(): 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") + steps = workflow["jobs"][contract["job"]]["steps"] audits = [ step for step in steps if step.get("id") in {"audit-complete", "audit-production"} ] - self.assertEqual(len(audits), 2) + self.assertEqual([step["run"] for step in audits], contract["runs"]) 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 - ) + all(step.get("continue-on-error") is True for step in audits) ) + for step in audits: + env = step.get("env", {}) + self.assertEqual( + "NPM_AUDIT_REQUIRE_RESULT" in env, + contract["require_env"], + ) verdict = steps[-1] self.assertIn("Require", verdict["name"]) self.assertEqual(verdict["if"], "${{ !cancelled() }}")