mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-09-10 02:25:56 +00:00
Merge pull request #1885 from rcourtman/claude/npm-audit-outage-hardening
Separate a real advisory from an unreachable npm audit endpoint
This commit is contained in:
rcourtman
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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")`,
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
Executable
+142
@@ -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 <all|production>
|
||||
#
|
||||
# `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 <all|production>" >&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
|
||||
Executable
+124
@@ -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"
|
||||
Reference in New Issue
Block a user