mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-09-09 18:15:50 +00:00
Merge remote-tracking branch 'origin/main'
# Conflicts: # .github/workflows/build-and-test.yml # docs/release-control/v6/internal/subsystems/deployment-installability.md # scripts/installtests/build_release_assets_test.go # scripts/npm-audit-retry.sh Change-source: pulse-maintainer
This commit is contained in:
@@ -121,10 +121,11 @@ jobs:
|
||||
needs: changes
|
||||
if: needs.changes.outputs.code == 'true'
|
||||
runs-on: ubuntu-24.04
|
||||
# The two npm audit steps retry through registry outages (they burned
|
||||
# 14m34s on 2026-09-04), which no longer fits a 25 minute budget: the job
|
||||
# was cancelled mid type-check with all 1183 test files already passing.
|
||||
timeout-minutes: 40
|
||||
# Everything except the audit runs in ~11m. The audit is now bounded to a
|
||||
# 4 minute wall-clock budget, so 30 leaves real headroom while staying a
|
||||
# meaningful ceiling: an unbounded 40 would have absorbed the 2026-09-04
|
||||
# stall instead of reporting it.
|
||||
timeout-minutes: 30
|
||||
env:
|
||||
FRONTEND_DIR: frontend-modern
|
||||
|
||||
@@ -143,13 +144,21 @@ jobs:
|
||||
|
||||
- name: Install frontend dependencies
|
||||
working-directory: frontend-modern
|
||||
# The explicit bounded audits below own the advisory verdict. Avoid
|
||||
# npm ci's duplicate best-effort audit POST and its five-minute timeout.
|
||||
# The explicit bounded audit below owns the advisory verdict. Avoid
|
||||
# npm ci's duplicate best-effort audit POST and its hidden 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.
|
||||
#
|
||||
# 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
|
||||
# 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
|
||||
id: audit-complete
|
||||
continue-on-error: true
|
||||
@@ -158,14 +167,6 @@ jobs:
|
||||
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
|
||||
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
|
||||
# the backstop that keeps `make format` a no-op on a clean tree.
|
||||
@@ -200,17 +201,15 @@ 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
|
||||
# An audit service failure or real advisory stays gating, but it must not
|
||||
# suppress formatting, tests, type-checking, or production-build proof.
|
||||
- name: Require frontend dependency audit
|
||||
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."
|
||||
if [ "${COMPLETE_AUDIT_RESULT}" != success ]; then
|
||||
echo "::error::The frontend dependency audit failed."
|
||||
exit 1
|
||||
fi
|
||||
|
||||
@@ -310,21 +309,13 @@ jobs:
|
||||
needs: changes
|
||||
if: needs.changes.outputs.code == 'true'
|
||||
runs-on: ubuntu-24.04
|
||||
timeout-minutes: 35
|
||||
timeout-minutes: 30
|
||||
steps:
|
||||
- name: Checkout repository
|
||||
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
|
||||
with:
|
||||
persist-credentials: false
|
||||
|
||||
- name: Checkout pull request base for paired comparison
|
||||
if: github.event_name == 'pull_request'
|
||||
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
|
||||
with:
|
||||
persist-credentials: false
|
||||
ref: ${{ github.event.pull_request.base.sha }}
|
||||
path: benchmark-base
|
||||
|
||||
- name: Set up Go
|
||||
uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6.4.0
|
||||
with:
|
||||
@@ -336,48 +327,64 @@ jobs:
|
||||
mkdir -p internal/api/frontend-modern/dist
|
||||
[ -f internal/api/frontend-modern/dist/index.html ] || \
|
||||
printf '<!doctype html><title>ci embed stub</title>\n' > internal/api/frontend-modern/dist/index.html
|
||||
if [ -d benchmark-base ]; then
|
||||
mkdir -p benchmark-base/internal/api/frontend-modern/dist
|
||||
[ -f benchmark-base/internal/api/frontend-modern/dist/index.html ] || \
|
||||
printf '<!doctype html><title>ci embed stub</title>\n' \
|
||||
> benchmark-base/internal/api/frontend-modern/dist/index.html
|
||||
fi
|
||||
|
||||
- name: Go benchmarks (non-PR)
|
||||
if: github.event_name != 'pull_request'
|
||||
timeout-minutes: 15
|
||||
- name: Go benchmarks
|
||||
timeout-minutes: 20
|
||||
env:
|
||||
PULSE_BENCH_SAMPLE_COUNT: "10"
|
||||
run: bash scripts/run-ci-benchmarks.sh
|
||||
PULSE_DATA_DIR: /tmp/pulse-bench-data
|
||||
run: |
|
||||
set -o pipefail
|
||||
go test -bench=. -benchmem -count=5 -run=^$ -benchtime=100ms -timeout=5m \
|
||||
./pkg/metrics/ \
|
||||
./pkg/auth/ \
|
||||
./internal/api/ \
|
||||
./internal/monitoring/ \
|
||||
./internal/unifiedresources/ \
|
||||
./internal/dockeragent/ \
|
||||
./cmd/pulse-agent/ \
|
||||
./internal/hostagent/ \
|
||||
./internal/hostmetrics/ \
|
||||
| tee bench-results.txt
|
||||
|
||||
- name: Paired base and candidate benchmarks (pull requests)
|
||||
if: github.event_name == 'pull_request'
|
||||
timeout-minutes: 25
|
||||
env:
|
||||
PULSE_BENCH_BASELINE_DIR: ${{ github.workspace }}/benchmark-base
|
||||
PULSE_BENCH_SAMPLE_COUNT: "10"
|
||||
run: bash scripts/run-ci-benchmarks.sh
|
||||
- name: Upload benchmark results
|
||||
if: always()
|
||||
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
|
||||
with:
|
||||
name: bench-results
|
||||
path: bench-results.txt
|
||||
retention-days: 7
|
||||
|
||||
- name: Install benchstat
|
||||
run: go install golang.org/x/perf/cmd/benchstat@v0.0.0-20260211190930-8161c38c6cdc
|
||||
|
||||
- name: Save benchmark baseline (main branch)
|
||||
if: github.ref == 'refs/heads/main'
|
||||
run: cp bench-results.txt bench-baseline.txt
|
||||
|
||||
- name: Cache benchmark baseline (main branch)
|
||||
if: github.ref == 'refs/heads/main'
|
||||
uses: actions/cache/save@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5
|
||||
with:
|
||||
path: bench-baseline.txt
|
||||
key: go-bench-baseline-${{ github.sha }}
|
||||
|
||||
- name: Restore benchmark baseline (PRs)
|
||||
if: github.event_name == 'pull_request'
|
||||
uses: actions/cache/restore@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5
|
||||
with:
|
||||
path: bench-baseline.txt
|
||||
key: go-bench-baseline-
|
||||
restore-keys: go-bench-baseline-
|
||||
|
||||
- name: Compare benchmarks against baseline
|
||||
if: github.event_name == 'pull_request'
|
||||
run: |
|
||||
set -eo pipefail
|
||||
if [ ! -f bench-baseline.txt ]; then
|
||||
echo "No benchmark baseline found (first PR against main?). Skipping comparison."
|
||||
exit 0
|
||||
fi
|
||||
echo "=== Benchmark comparison (baseline vs current) ==="
|
||||
benchstat bench-baseline.txt bench-results.txt | tee bench-comparison.txt
|
||||
echo ""
|
||||
bash scripts/check-bench-regression.sh bench-comparison.txt
|
||||
|
||||
- name: Upload benchmark evidence
|
||||
if: always()
|
||||
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
|
||||
with:
|
||||
name: bench-results
|
||||
path: |
|
||||
bench-results.txt
|
||||
bench-baseline.txt
|
||||
bench-comparison.txt
|
||||
if-no-files-found: warn
|
||||
retention-days: 7
|
||||
|
||||
@@ -1082,12 +1082,10 @@ artifact-selection behaviour.
|
||||
source-built and exact-candidate images must recreate and qualify all three;
|
||||
otherwise the download handler rejects the local Windows agent and falls
|
||||
through to a release asset with the wrong filename identity.
|
||||
Helm Pages convergence must recover the immutable chart package from the
|
||||
OCI digest produced and qualified by the exact create-release run. It must
|
||||
verify that digest's hosted-workflow provenance and bind the package to the
|
||||
activated source run, tag, commit, and activation marker; a transient
|
||||
Actions artifact is not a recovery boundary. It must not repeat chart
|
||||
packaging or the pre-activation kind install/upgrade smoke.
|
||||
Helm Pages convergence must promote the immutable chart artifact produced
|
||||
and qualified by the exact create-release run. It must bind that artifact
|
||||
to the activated source run, tag, commit, and activation marker, and must
|
||||
not repeat chart packaging or the pre-activation kind install/upgrade smoke.
|
||||
Release-to-convergence and cross-repository child-run observation should
|
||||
use short bounded polls so GitHub indexing cannot add tens of seconds after
|
||||
a required exact run or activation marker has already completed.
|
||||
@@ -1873,18 +1871,6 @@ artifact-selection behaviour.
|
||||
inputs recovered from the activation marker. Pre-commit owner renewal remains
|
||||
limited to the original run while its exact source release run is active and
|
||||
does not consume the post-commit convergence-debt budget.
|
||||
An immutable release blocked only by the private paid-runtime credential-
|
||||
containment gate must not consume attempts through unattended replay while
|
||||
the failed private run's containment checker and operator checklist are
|
||||
byte-identical to their current private-main versions. That terminal
|
||||
classification requires authenticated private-repository evidence tying the
|
||||
public paid-runtime job to exactly one canonical failed private promotion run,
|
||||
exactly one failed containment job, and the explicit blocked marker in that
|
||||
job's log. Missing, inaccessible, malformed, or ambiguous evidence remains a
|
||||
normal fail-closed convergence failure. A change to either private
|
||||
containment input or to the public default-branch controls rearms the bounded
|
||||
retry budget for that control revision; it never weakens the containment gate
|
||||
or marks the customer surface converged.
|
||||
A support-only private Pro prerelease image is a narrower exception for
|
||||
customer verification of an already-fixed defect. It may dispatch the private
|
||||
`Build Pro Release` workflow with `publish_docker_image=true`,
|
||||
@@ -2141,16 +2127,6 @@ artifact-selection behaviour.
|
||||
secrets, and attacker-controlled event metadata must enter generated runner
|
||||
scripts through explicit environment variables; `${{ }}` interpolation in
|
||||
a `run` program is not an acceptable data boundary.
|
||||
Exact-SHA payload compilation and pre-publication release smoke jobs must
|
||||
explicitly disable setup-node automatic package-manager caching and must not
|
||||
opt into setup-node dependency caching. Those jobs consume or qualify the
|
||||
candidate at a release trust boundary, so mutable cache contents must not
|
||||
become an undeclared release input.
|
||||
Release-candidate artifact transfers must use the reviewed Node 24
|
||||
`actions/download-artifact` v8.0.1 pin. Issue automation must use the
|
||||
reviewed Node 24 `actions/github-script` v8.0.0 pin. The workflow trust
|
||||
audit must reject older pins after GitHub's announced Node 20 removal so a
|
||||
release cannot become unbuildable through action-runtime retirement.
|
||||
Whenever that policy changes, update the owning workflow/install proof files
|
||||
in `scripts/installtests/build_release_assets_test.go` and
|
||||
`scripts/release_control/release_promotion_policy_*` in the same slice.
|
||||
@@ -2196,21 +2172,6 @@ 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. If npm emits an advisory report together with a transport
|
||||
marker, the advisory result takes precedence and must not be retried. 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
|
||||
|
||||
@@ -3224,15 +3185,15 @@ vulnerabilities in the current patch level, the canonical fix is to advance the
|
||||
governed release toolchain and immutable Go builder digest together, not to
|
||||
suppress the scanner or produce release artifacts with an older patched-over
|
||||
runtime.
|
||||
As of 2026-09-04, the governed release floor is Go `1.26.8`, the current
|
||||
supported `1.26` patch release. It retains the security corrections that made
|
||||
`1.26.7` the previous floor and adds the upstream compiler, runtime, cgo,
|
||||
`debug/elf`, and `os` fixes shipped in `1.26.8`. Source-built Alpine container
|
||||
stages pin the Docker Official Images manifest list
|
||||
`sha256:ce864e7223ac17b1775e6fd0b4c0db580c2eb50e7953a427916379e4b92a1628`;
|
||||
As of 2026-08-27, the governed release floor is Go `1.26.7`. It supersedes
|
||||
`1.26.5`, whose standard library is reachable through seven vulnerable Pulse
|
||||
call paths reported by `govulncheck`, including HTTP/TLS, URL parsing, SAML XML
|
||||
decoding, HTML templating, and public-key parsing. Both source-built container
|
||||
stages pin the Docker Official Images Linux amd64 manifest
|
||||
`sha256:28d89ee9cc0ff9fec75c82ca201e6bf7fdf9a679d4b7b24dfa04f2bb766bb468`;
|
||||
the checked-in toolchain files and release-script guards must reject an older
|
||||
compiler so local, exact-candidate, provider control-plane, and container builds
|
||||
cannot silently reintroduce a superseded runtime.
|
||||
cannot silently reintroduce the vulnerable runtime.
|
||||
That same dev-runtime dependency-manifest boundary now also owns the maintained
|
||||
Docker engine module floor. `go.mod`, `go.sum`, and
|
||||
`internal/cloudcp/docker/manager.go` must route hosted runtime orchestration
|
||||
@@ -4051,27 +4012,45 @@ resolved package version and integrity that the release build will actually
|
||||
consume.
|
||||
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, through `scripts/npm-audit-retry.sh`. That runner exists because
|
||||
`.github/workflows/build-and-test.yml` frontend job must run the complete
|
||||
`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
|
||||
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
|
||||
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
|
||||
must never be introduced; only an unreachable endpoint is
|
||||
retried. Retrying is bounded by wall clock and not by attempt count alone,
|
||||
because npm's own `fetch-timeout` defaults to five minutes and it retries
|
||||
internally: on 2026-09-04 three attempts against a hanging endpoint ran for
|
||||
10m56s and cancelled the Frontend job at its own timeout with every test
|
||||
already passing, so a green run was reported as a failed required check. Each
|
||||
attempt is therefore bounded, npm's internal retry loop is disabled in favour
|
||||
of the runner's own, and the sequence stops at a total deadline. That budget
|
||||
must stay well inside the job timeout; it may never be raised to the point
|
||||
where an unreachable endpoint can consume the job. When retries are exhausted,
|
||||
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,
|
||||
not of a per-pull-request audit. `scripts/tests/test-npm-audit-retry.sh` pins
|
||||
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 and that an unparseable or unrecognised report is never read as
|
||||
clean. `frontend-modern/src/security/__tests__/dependencySecurity.test.ts`
|
||||
is active, 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`,
|
||||
including DOMPurify `GHSA-55q2-fjhq-7xh7`, brace-expansion
|
||||
`GHSA-mh99-v99m-4gvg` and `GHSA-rgw5-rvv9-x895`, and nanoid
|
||||
@@ -5072,21 +5051,11 @@ Every repository checkout in build, packaging, publication, qualification,
|
||||
recovery, and deployment automation uses the reviewed immutable
|
||||
`actions/checkout` v7.0.1 pin. That baseline refuses fork pull-request checkout
|
||||
on privileged events unless a workflow explicitly opts out; Pulse prohibits
|
||||
that opt-out. The sole `pull_request_target` exception is the machine-checked,
|
||||
metadata-only closed-PR capacity reclaimer: it runs only the protected
|
||||
default-branch helper and has no secret, checkout, cache, artifact, container,
|
||||
or shell ingress. Pull-request workflows may not reference repository secrets,
|
||||
including values treated as public configuration. Exact-SHA compilation and
|
||||
pre-publication release smoke explicitly disable setup-node's automatic npm
|
||||
cache and do not request dependency caches, preventing mutable cache state from
|
||||
becoming release input. Candidate artifact downloads use the reviewed Node 24
|
||||
`actions/download-artifact` v8.0.1 pin, and issue automation uses the reviewed
|
||||
Node 24 `actions/github-script` v8.0.0 pin; older runtime pins are rejected
|
||||
before GitHub's 23 September 2026 Node 20 removal. Dependency refreshes must
|
||||
that opt-out and the `pull_request_target` trigger. Dependency refreshes must
|
||||
update the central workflow-trust allowlist and its regression proof together,
|
||||
so a routine pin change cannot silently remove this release-automation trust
|
||||
boundary.
|
||||
`scripts/check_workflow_trust.py`, `scripts/tests/test_workflow_trust.py`, and
|
||||
boundary. `scripts/check_workflow_trust.py`,
|
||||
`scripts/tests/test_workflow_trust.py`, and
|
||||
`scripts/installtests/build_release_assets_test.go` pin the policy and the
|
||||
release-workflow integration.
|
||||
|
||||
|
||||
@@ -552,3 +552,30 @@ retained only for callers that never had the error value.
|
||||
`internal/notifications/failure_class_test.go` pins the precedence order, the
|
||||
SMTP reply-code mapping, and the rule that response-body text cannot steer the
|
||||
recorded class.
|
||||
|
||||
### The retry ladder is gated on the failure class
|
||||
|
||||
Delivery retries are for conditions that can clear. `authentication`,
|
||||
`configuration`, and `rejected` are verdicts about the request itself: the same
|
||||
payload, sent again to the same destination with the same credentials, gets the
|
||||
same answer. Those three dead-letter on the attempt that produced them, without
|
||||
consuming the remaining ladder. `connectivity`, `rate_limited`, `server_error`,
|
||||
`tls`, and an unclassified failure keep the full ladder, because nothing about
|
||||
them proves a later attempt fails. TLS is deliberately on the retrying side: a
|
||||
handshake can fail transiently during rotation, and the cost of one wasted
|
||||
ladder is lower than the cost of dropping a recoverable notification.
|
||||
|
||||
`NotificationFailureClass.Retryable` is the single owner of that split. Neither
|
||||
the queue nor any destination type may keep its own list. This generalises to
|
||||
every destination the decision webhook delivery already made for HTTP 4xx in
|
||||
`isRetryableWebhookError`.
|
||||
|
||||
Dead-lettering early must not lose the notification: `RetryTerminalFailures`
|
||||
remains the operator's recovery path, returning retained terminal failures to
|
||||
the queue with a fresh budget once the credentials or configuration are fixed.
|
||||
A dead-letter row records `failureClass` and a `deadLetterReason` of
|
||||
`failure_class_not_retryable` or `max_retries_exhausted` so the two are
|
||||
distinguishable in local logs.
|
||||
|
||||
`internal/notifications/failure_class_test.go` pins the retryable split and
|
||||
that a deterministic failure dead-letters on its first attempt.
|
||||
|
||||
@@ -433,8 +433,24 @@ func TestPatrolHTTPJSONObserverUsesScopedDiscoveryOriginSecretReferenceAndWakes(
|
||||
case <-time.After(3 * time.Second):
|
||||
t.Fatal("HTTP JSON observer did not execute")
|
||||
}
|
||||
deadline := time.Now().Add(2 * time.Second)
|
||||
for tm.GetPendingCount() != 1 && time.Now().Before(deadline) {
|
||||
// The HTTP sample runs on its own goroutine and persists the health lease
|
||||
// strictly after it queues the wake, so the lease is the one side effect
|
||||
// that orders the whole sample against these assertions. Gating on the wake
|
||||
// instead let a loaded runner read a half-applied sample: a queued trigger
|
||||
// with a still-nil lease, reported as degraded/observer_health_unknown.
|
||||
// Coverage is evaluated at a fixed instant derived from the sample time so
|
||||
// the verdict never depends on how long the goroutine took to get there.
|
||||
evaluatedAt := now.Add(2 * time.Second)
|
||||
deadline := time.Now().Add(10 * time.Second)
|
||||
var installed PatrolObjective
|
||||
for {
|
||||
installed, _ = store.Get(objective.ID, evaluatedAt)
|
||||
if installed.Observer != nil && installed.Observer.ValidUntil != nil {
|
||||
break
|
||||
}
|
||||
if !time.Now().Before(deadline) {
|
||||
t.Fatalf("HTTP JSON observer never persisted a health lease: %+v", installed.Observer)
|
||||
}
|
||||
time.Sleep(5 * time.Millisecond)
|
||||
}
|
||||
if tm.GetPendingCount() != 1 {
|
||||
@@ -444,8 +460,7 @@ func TestPatrolHTTPJSONObserverUsesScopedDiscoveryOriginSecretReferenceAndWakes(
|
||||
if queued.ObjectiveContext == nil || !strings.Contains(queued.ObjectiveContext.Evidence, "buffering_sessions") || !strings.Contains(queued.ObjectiveContext.Evidence, "actual_1") {
|
||||
t.Fatalf("HTTP JSON objective evidence = %+v", queued.ObjectiveContext)
|
||||
}
|
||||
installed, _ := store.Get(objective.ID, time.Now().UTC())
|
||||
if installed.Observer == nil || installed.Observer.State != PatrolObserverInstalled || installed.Coverage.State != PatrolObjectiveCovered {
|
||||
if installed.Observer.State != PatrolObserverInstalled || installed.Coverage.State != PatrolObjectiveCovered || installed.Coverage.ReasonCode != "observer_healthy" {
|
||||
t.Fatalf("HTTP observer coverage = %+v / %+v", installed.Observer, installed.Coverage)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -170,3 +170,30 @@ func ClassifyNotificationFailureError(err error) NotificationFailureClass {
|
||||
|
||||
return ClassifyNotificationFailure(err.Error())
|
||||
}
|
||||
|
||||
// Retryable reports whether another delivery attempt could plausibly succeed.
|
||||
//
|
||||
// Authentication, configuration and rejection are verdicts about the request
|
||||
// itself: the same payload, sent again to the same destination with the same
|
||||
// credentials, gets the same answer. Retrying them spends attempts, delays the
|
||||
// dead letter the operator needs to see, and learns nothing. Connectivity,
|
||||
// rate limiting and server errors describe conditions that clear on their own,
|
||||
// and an unclassified failure is retried because nothing proves it will not
|
||||
// succeed.
|
||||
//
|
||||
// This generalises to every destination type the decision webhook delivery
|
||||
// already made for HTTP 4xx in isRetryableWebhookError.
|
||||
//
|
||||
// A dead letter is not the end of the road: once the operator fixes the
|
||||
// credentials or the configuration, RetryTerminalFailures returns retained
|
||||
// terminal failures to the queue with a fresh budget.
|
||||
func (class NotificationFailureClass) Retryable() bool {
|
||||
switch class {
|
||||
case NotificationFailureAuthentication,
|
||||
NotificationFailureConfiguration,
|
||||
NotificationFailureRejected:
|
||||
return false
|
||||
default:
|
||||
return true
|
||||
}
|
||||
}
|
||||
|
||||
@@ -240,3 +240,114 @@ func TestRecordAuditErrorPersistsDeclaredClass(t *testing.T) {
|
||||
t.Errorf("unknown = %d, want 0", stats.FailureClasses.Unknown)
|
||||
}
|
||||
}
|
||||
|
||||
func TestFailureClassRetryable(t *testing.T) {
|
||||
cases := map[NotificationFailureClass]bool{
|
||||
NotificationFailureAuthentication: false,
|
||||
NotificationFailureConfiguration: false,
|
||||
NotificationFailureRejected: false,
|
||||
NotificationFailureConnectivity: true,
|
||||
NotificationFailureRateLimited: true,
|
||||
NotificationFailureServerError: true,
|
||||
NotificationFailureTLS: true,
|
||||
NotificationFailureUnknown: true,
|
||||
NotificationFailureClass(""): true,
|
||||
}
|
||||
for class, want := range cases {
|
||||
if got := class.Retryable(); got != want {
|
||||
t.Errorf("%q.Retryable() = %v, want %v", class, got, want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// A deterministic failure must dead-letter on the first attempt rather than
|
||||
// spend the whole ladder re-asking a question that already has an answer.
|
||||
func TestProcessNotificationDeadLettersDeterministicFailureImmediately(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
sendErr error
|
||||
wantStatus NotificationQueueStatus
|
||||
wantCallCount int
|
||||
}{
|
||||
{
|
||||
name: "authentication does not retry",
|
||||
sendErr: FailWithClass(NotificationFailureAuthentication, errors.New("smtp 535 authentication failed")),
|
||||
wantStatus: QueueStatusDLQ,
|
||||
wantCallCount: 1,
|
||||
},
|
||||
{
|
||||
name: "configuration does not retry",
|
||||
sendErr: FailWithClass(NotificationFailureConfiguration, errors.New("no Apprise targets configured for CLI delivery")),
|
||||
wantStatus: QueueStatusDLQ,
|
||||
wantCallCount: 1,
|
||||
},
|
||||
{
|
||||
name: "rejection does not retry",
|
||||
sendErr: FailfWithClass(ClassFromHTTPStatus(422), "webhook returned HTTP 422: unprocessable"),
|
||||
wantStatus: QueueStatusDLQ,
|
||||
wantCallCount: 1,
|
||||
},
|
||||
{
|
||||
name: "connectivity still retries",
|
||||
sendErr: FailWithClass(NotificationFailureConnectivity, errors.New("dial tcp: connection refused")),
|
||||
wantStatus: QueueStatusPending,
|
||||
wantCallCount: 1,
|
||||
},
|
||||
{
|
||||
name: "server error still retries",
|
||||
sendErr: FailfWithClass(ClassFromHTTPStatus(503), "webhook returned HTTP 503: unavailable"),
|
||||
wantStatus: QueueStatusPending,
|
||||
wantCallCount: 1,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
nq, err := NewNotificationQueue(t.TempDir())
|
||||
if err != nil {
|
||||
t.Fatalf("NewNotificationQueue: %v", err)
|
||||
}
|
||||
defer func() { _ = nq.Stop() }()
|
||||
|
||||
futureRetry := time.Now().Add(time.Hour)
|
||||
notif := &QueuedNotification{
|
||||
ID: "deterministic-failure",
|
||||
Type: "webhook",
|
||||
Status: QueueStatusPending,
|
||||
MaxAttempts: 3,
|
||||
Config: []byte(`{}`),
|
||||
NextRetryAt: &futureRetry,
|
||||
}
|
||||
if err := nq.Enqueue(notif); err != nil {
|
||||
t.Fatalf("enqueue: %v", err)
|
||||
}
|
||||
|
||||
calls := 0
|
||||
nq.SetProcessor(func(*QueuedNotification) error {
|
||||
calls++
|
||||
return tc.sendErr
|
||||
})
|
||||
|
||||
nq.processNotification(notif)
|
||||
|
||||
if calls != tc.wantCallCount {
|
||||
t.Errorf("processor calls = %d, want %d", calls, tc.wantCallCount)
|
||||
}
|
||||
|
||||
if notif.Status != tc.wantStatus {
|
||||
t.Errorf("status = %q, want %q", notif.Status, tc.wantStatus)
|
||||
}
|
||||
if notif.Attempts != 1 {
|
||||
t.Errorf("attempts = %d, want 1 (one delivery attempt was made)", notif.Attempts)
|
||||
}
|
||||
|
||||
stats, err := nq.GetQueueStats()
|
||||
if err != nil {
|
||||
t.Fatalf("GetQueueStats: %v", err)
|
||||
}
|
||||
if stats[string(tc.wantStatus)] != 1 {
|
||||
t.Errorf("persisted queue stats = %#v, want one row in %q", stats, tc.wantStatus)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1767,8 +1767,10 @@ func (nq *NotificationQueue) processNotification(notif *QueuedNotification) {
|
||||
|
||||
success := err == nil
|
||||
errorMsg := ""
|
||||
failureClass := NotificationFailureClass("")
|
||||
if err != nil {
|
||||
errorMsg = err.Error()
|
||||
failureClass = ClassifyNotificationFailureError(err)
|
||||
}
|
||||
|
||||
if success {
|
||||
@@ -1801,8 +1803,12 @@ func (nq *NotificationQueue) processNotification(notif *QueuedNotification) {
|
||||
Int("maxAttempts", notif.MaxAttempts).
|
||||
Msg("Notification sent successfully")
|
||||
} else {
|
||||
// Check if we should retry or move to DLQ
|
||||
if notif.Attempts >= notif.MaxAttempts {
|
||||
// Check if we should retry or move to DLQ. A deterministic failure
|
||||
// class is dead-lettered on the spot: retrying an authentication,
|
||||
// configuration, or rejection verdict cannot change the answer, and
|
||||
// only delays the dead letter the operator needs to act on.
|
||||
retryable := failureClass.Retryable()
|
||||
if notif.Attempts >= notif.MaxAttempts || !retryable {
|
||||
// Move to DLQ
|
||||
if dlqErr := nq.MoveToDLQ(notif.ID, errorMsg); dlqErr != nil {
|
||||
log.Error().
|
||||
@@ -1823,6 +1829,10 @@ func (nq *NotificationQueue) processNotification(notif *QueuedNotification) {
|
||||
operationaltrust.NotificationDeadLetter,
|
||||
completedAt,
|
||||
)
|
||||
deadLetterReason := "max_retries_exhausted"
|
||||
if !retryable {
|
||||
deadLetterReason = "failure_class_not_retryable"
|
||||
}
|
||||
log.Warn().
|
||||
Str("component", "notification_queue").
|
||||
Str("action", "move_to_dlq").
|
||||
@@ -1830,8 +1840,10 @@ func (nq *NotificationQueue) processNotification(notif *QueuedNotification) {
|
||||
Str("type", notif.Type).
|
||||
Int("attempts", notif.Attempts).
|
||||
Int("maxAttempts", notif.MaxAttempts).
|
||||
Str("failureClass", string(failureClass)).
|
||||
Str("deadLetterReason", deadLetterReason).
|
||||
Str("error", errorMsg).
|
||||
Msg("notification moved to DLQ after max retries")
|
||||
Msg("notification moved to DLQ")
|
||||
}
|
||||
} else {
|
||||
// Schedule retry
|
||||
|
||||
@@ -1606,7 +1606,7 @@ func TestDockerBuildUsesCanonicalReleaseLdflags(t *testing.T) {
|
||||
dockerfile := string(dockerfileBytes)
|
||||
dockerRequired := []string{
|
||||
`FROM --platform=linux/amd64 node:24-alpine@sha256:`,
|
||||
`FROM --platform=linux/amd64 golang:1.26.8-alpine@sha256:`,
|
||||
`FROM --platform=linux/amd64 golang:1.26.7-alpine@sha256:`,
|
||||
`FROM backend-builder AS release-assets-builder`,
|
||||
`AS agent_runtime`,
|
||||
`AS pulse-runtime-foundation`,
|
||||
@@ -1641,7 +1641,7 @@ func TestDockerBuildUsesCanonicalReleaseLdflags(t *testing.T) {
|
||||
}
|
||||
}
|
||||
assertDigestPinnedDockerStage(t, dockerfile, `FROM --platform=linux/amd64 node:24-alpine@sha256:`, ` AS frontend-builder`)
|
||||
assertDigestPinnedDockerStage(t, dockerfile, `FROM --platform=linux/amd64 golang:1.26.8-alpine@sha256:`, ` AS backend-builder`)
|
||||
assertDigestPinnedDockerStage(t, dockerfile, `FROM --platform=linux/amd64 golang:1.26.7-alpine@sha256:`, ` AS backend-builder`)
|
||||
assertDigestPinnedDockerStage(t, dockerfile, `FROM alpine:3.24@sha256:`, ` AS agent_runtime`)
|
||||
assertDigestPinnedDockerStage(t, dockerfile, `FROM alpine:3.24@sha256:`, ` AS pulse-runtime-foundation`)
|
||||
hostedStart := strings.Index(dockerfile, `FROM pulse-runtime-base AS hosted_runtime`)
|
||||
@@ -1654,7 +1654,7 @@ func TestDockerBuildUsesCanonicalReleaseLdflags(t *testing.T) {
|
||||
t.Fatalf("hosted_runtime target must not depend on installer rendering or embedded agent artifacts:\n%s", hostedStage)
|
||||
}
|
||||
if strings.Contains(dockerfile, `FROM --platform=linux/amd64 node:24-alpine AS frontend-builder`) ||
|
||||
strings.Contains(dockerfile, `FROM --platform=linux/amd64 golang:1.26.8-alpine AS backend-builder`) ||
|
||||
strings.Contains(dockerfile, `FROM --platform=linux/amd64 golang:1.26.7-alpine AS backend-builder`) ||
|
||||
strings.Contains(dockerfile, `FROM alpine:3.24 AS agent_runtime`) ||
|
||||
strings.Contains(dockerfile, `FROM alpine:3.24 AS pulse-runtime-base`) {
|
||||
t.Fatal("Dockerfile base images must be pinned by immutable @sha256 digests")
|
||||
@@ -2098,7 +2098,8 @@ func TestDeploymentDefaultsPinVersionedImagesAndHelmDocsChecksum(t *testing.T) {
|
||||
`Require activated GitHub release and source run`,
|
||||
`release-activation.json`,
|
||||
`.github/workflows/create-release.yml`,
|
||||
`"${GITHUB_REPOSITORY}" "${CHART_DIGEST}" "${CHART_PATH}"`,
|
||||
`gh run download "${SOURCE_RELEASE_RUN_ID}"`,
|
||||
`--name "pulse-chart-${VERSION}"`,
|
||||
`qualified chart metadata does not match the activated release`,
|
||||
`name: Publish chart release and merge Pages index`,
|
||||
`gh release create "${chart_release}" "${chart_path}"`,
|
||||
@@ -2115,7 +2116,6 @@ func TestDeploymentDefaultsPinVersionedImagesAndHelmDocsChecksum(t *testing.T) {
|
||||
}
|
||||
for _, forbidden := range []string{
|
||||
"workflow_run:",
|
||||
`gh run download "${SOURCE_RELEASE_RUN_ID}"`,
|
||||
`Smoke test with kind`,
|
||||
`Install helm-docs`,
|
||||
`helm package deploy/helm/pulse`,
|
||||
@@ -2829,10 +2829,7 @@ func TestReleaseAssetCommonRunsUpdateKeyThroughModulePath(t *testing.T) {
|
||||
t.Skip("go not installed")
|
||||
}
|
||||
|
||||
// Keep the toolchain selected by the test runner. A login shell may source a
|
||||
// developer's stale mise/asdf profile and replace setup-go's release
|
||||
// toolchain while leaving its GOROOT behind.
|
||||
cmd := exec.Command("bash", "-c", "source ./scripts/release_asset_common.sh; pulse_release_go_run_update_key")
|
||||
cmd := exec.Command("bash", "-lc", "source ./scripts/release_asset_common.sh; pulse_release_go_run_update_key")
|
||||
cmd.Dir = repoFile()
|
||||
output, err := cmd.CombinedOutput()
|
||||
if err == nil {
|
||||
@@ -2864,7 +2861,7 @@ func TestReleaseAssetCommonRejectsUnexpectedUpdateSigningPublicKey(t *testing.T)
|
||||
t.Fatalf("generate unexpected public key: %v", err)
|
||||
}
|
||||
|
||||
cmd := exec.Command("bash", "-c", "source ./scripts/release_asset_common.sh; pulse_release_prepare_signing_state pulse-installer pulse-install")
|
||||
cmd := exec.Command("bash", "-lc", "source ./scripts/release_asset_common.sh; pulse_release_prepare_signing_state pulse-installer pulse-install")
|
||||
cmd.Dir = repoFile()
|
||||
cmd.Env = append(os.Environ(),
|
||||
"PULSE_UPDATE_SIGNING_KEY="+base64.StdEncoding.EncodeToString(privateKey),
|
||||
@@ -3243,13 +3240,6 @@ func TestReleasePipelinePromotesOneImmutableCandidate(t *testing.T) {
|
||||
|
||||
createWorkflow := string(createBytes)
|
||||
candidateWorkflow := string(candidateBytes)
|
||||
node24DownloadArtifact := "actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1"
|
||||
if count := strings.Count(candidateWorkflow, node24DownloadArtifact); count != 5 {
|
||||
t.Fatalf("release candidate must use the reviewed Node 24 artifact downloader for all five signed-artifact transfers, got %d", count)
|
||||
}
|
||||
if strings.Contains(candidateWorkflow, "actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093") {
|
||||
t.Fatal("release candidate must not retain the Node 20 artifact downloader")
|
||||
}
|
||||
compilerWorkflow := string(compilerBytes)
|
||||
compileScriptBytes, err := os.ReadFile(repoFile("scripts", "build-release-binaries.sh"))
|
||||
if err != nil {
|
||||
@@ -3276,7 +3266,6 @@ func TestReleasePipelinePromotesOneImmutableCandidate(t *testing.T) {
|
||||
frontendBundleJob := workflowJobBlock(t, createWorkflow, "frontend_bundle")
|
||||
backendJob := workflowJobBlock(t, createWorkflow, "backend_tests")
|
||||
integrationJob := workflowJobBlock(t, createWorkflow, "integration_tests")
|
||||
releaseSmokeJob := workflowJobBlock(t, createWorkflow, "release_smoke")
|
||||
validationJob := workflowJobBlock(t, createWorkflow, "validate_release_assets")
|
||||
privateStageJob := workflowJobBlock(t, createWorkflow, "stage_private_pro_runtime")
|
||||
readinessJob := workflowJobBlock(t, createWorkflow, "release_readiness")
|
||||
@@ -3289,8 +3278,6 @@ func TestReleasePipelinePromotesOneImmutableCandidate(t *testing.T) {
|
||||
helmPagesJob := workflowJobBlock(t, convergenceWorkflow, "publish_helm_pages")
|
||||
demoJob := workflowJobBlock(t, convergenceWorkflow, "update_stable_demo")
|
||||
compileJob := workflowJobBlock(t, compilerWorkflow, "compile-release-payload")
|
||||
compileSetupNodeStep := workflowStepBlock(t, compileJob, "Set up Node.js")
|
||||
releaseSmokeSetupNodeStep := workflowStepBlock(t, releaseSmokeJob, "Set up Node.js")
|
||||
obtainPayloadJob := workflowJobBlock(t, candidateWorkflow, "obtain-release-payload")
|
||||
candidateBuildJob := workflowJobBlock(t, candidateWorkflow, "build")
|
||||
compiledPayloadVerificationStep := workflowStepBlock(t, candidateBuildJob, "Verify exact-SHA compiled payload")
|
||||
@@ -3384,16 +3371,8 @@ func TestReleasePipelinePromotesOneImmutableCandidate(t *testing.T) {
|
||||
t.Fatalf("%s runner selection must not depend on the Windows-signing decision", label)
|
||||
}
|
||||
}
|
||||
for label, step := range map[string]string{
|
||||
"exact-SHA release compilation": compileSetupNodeStep,
|
||||
"pre-publication release smoke": releaseSmokeSetupNodeStep,
|
||||
} {
|
||||
if !strings.Contains(step, "package-manager-cache: false") {
|
||||
t.Fatalf("%s must disable setup-node automatic package-manager caching", label)
|
||||
}
|
||||
if strings.Contains(step, "\n cache:") || strings.Contains(step, "cache-dependency-path:") {
|
||||
t.Fatalf("%s must not opt into setup-node dependency caching", label)
|
||||
}
|
||||
if !strings.Contains(compileJob, "cache: false") || strings.Contains(compileJob, "cache: 'npm'") {
|
||||
t.Fatal("release compilation must avoid Actions cache archival")
|
||||
}
|
||||
if strings.Contains(frontendBundleJob, "cache: 'npm'") {
|
||||
t.Fatal("PVE frontend bundle must use its persistent runner-local npm cache")
|
||||
@@ -3661,42 +3640,27 @@ func TestReleasePipelinePromotesOneImmutableCandidate(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestFrontendDependencySecurityAuditsAreRequired(t *testing.T) {
|
||||
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{
|
||||
workflowPath := repoFile(".github", "workflows", "build-and-test.yml")
|
||||
assertFileContainsAll(t, workflowPath,
|
||||
`run: npm ci --no-audit`,
|
||||
`- name: Audit complete frontend dependency graph`,
|
||||
`npm-audit-retry.sh" all`,
|
||||
`- name: Audit production frontend dependencies`,
|
||||
`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 }}`,
|
||||
`continue-on-error: true`,
|
||||
`- name: Require frontend dependency audits`,
|
||||
`- name: Require frontend dependency audit`,
|
||||
`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)
|
||||
}
|
||||
}
|
||||
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,
|
||||
`npm-audit-retry.sh" all --package-lock-only`,
|
||||
// The production-only audit reports a subset of the complete audit's
|
||||
// advisories and cannot gate anything the complete audit did not already
|
||||
// fail on, so it is off the per-pull-request path. It must still run
|
||||
// somewhere: the scheduled scan owns the dev-versus-production split.
|
||||
assertFileContainsAll(t, repoFile(".github", "workflows", "security-scan.yml"),
|
||||
`- name: Audit production dependencies`,
|
||||
`npm-audit-retry.sh" production --package-lock-only`,
|
||||
`- name: Require dependency audits`,
|
||||
`COMPLETE_AUDIT_RESULT: ${{ steps.audit-complete.outcome }}`,
|
||||
@@ -3715,11 +3679,30 @@ func TestFrontendDependencySecurityAuditsAreRequired(t *testing.T) {
|
||||
}
|
||||
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")`,
|
||||
)
|
||||
// Retrying must be bounded by wall clock, not by attempt count alone.
|
||||
// npm's own fetch-timeout defaults to five minutes and it retries
|
||||
// internally, so three unbounded attempts once ran for 10m56s and
|
||||
// cancelled the Frontend job with every test already passing.
|
||||
assertFileContainsAll(t, runnerPath,
|
||||
`NPM_AUDIT_MAX_SECONDS`,
|
||||
`NPM_AUDIT_ATTEMPT_TIMEOUT`,
|
||||
`export npm_config_fetch_retries=0`,
|
||||
`DEADLINE=`,
|
||||
)
|
||||
// The job budget has to stay above the audit budget by a wide margin, or
|
||||
// a stalled endpoint reappears as a cancelled job rather than a warning.
|
||||
workflow, err := os.ReadFile(workflowPath)
|
||||
if err != nil {
|
||||
t.Fatalf("read %s: %v", workflowPath, err)
|
||||
}
|
||||
frontendJob := workflowJobBlock(t, string(workflow), "frontend")
|
||||
if !strings.Contains(frontendJob, "timeout-minutes: 30") {
|
||||
t.Fatal("frontend job must keep a bounded timeout above the audit budget")
|
||||
}
|
||||
}
|
||||
|
||||
func TestReleaseCutGatesCriticalFrontendAndWindowsRuntimeProof(t *testing.T) {
|
||||
|
||||
@@ -20,6 +20,16 @@
|
||||
# change touches the dependency graph (NPM_AUDIT_REQUIRE_RESULT=true) and
|
||||
# warns without failing when it does not.
|
||||
#
|
||||
# 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
|
||||
# five minutes and it retries internally, so a single `npm audit` against a
|
||||
# hanging endpoint can sit for minutes before this script sees a verdict. On
|
||||
# 2026-09-04 that produced a 10m56s audit step (two 5m00s attempts, then a
|
||||
# 9s success) and cancelled the Frontend job at its 25m limit with every test
|
||||
# already passing — a green run reported as a failed required check. So each
|
||||
# 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
|
||||
# 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
|
||||
@@ -30,9 +40,11 @@
|
||||
#
|
||||
# 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_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_CMD npm executable to invoke (test seam)
|
||||
@@ -52,11 +64,53 @@ shift
|
||||
AUDIT_ARGS=("$@")
|
||||
|
||||
ATTEMPTS="${NPM_AUDIT_ATTEMPTS:-3}"
|
||||
FETCH_TIMEOUT_MS="${NPM_AUDIT_FETCH_TIMEOUT_MS:-60000}"
|
||||
DELAY="${NPM_AUDIT_RETRY_DELAY:-15}"
|
||||
ATTEMPT_TIMEOUT="${NPM_AUDIT_ATTEMPT_TIMEOUT:-60}"
|
||||
MAX_SECONDS="${NPM_AUDIT_MAX_SECONDS:-240}"
|
||||
REQUIRE_RESULT="${NPM_AUDIT_REQUIRE_RESULT:-true}"
|
||||
NPM_BIN="${NPM_AUDIT_CMD:-npm}"
|
||||
|
||||
# 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.
|
||||
export npm_config_fetch_retries=0
|
||||
export npm_config_fetch_timeout=$((ATTEMPT_TIMEOUT * 1000))
|
||||
|
||||
DEADLINE=$(( $(date +%s) + MAX_SECONDS ))
|
||||
|
||||
# Run one audit under a hard wall-clock bound, portably: `timeout` is not
|
||||
# present on every developer machine, so a watchdog subshell kills the npm
|
||||
# process if it outlives the limit. Blocking on `wait` for the real child
|
||||
# avoids the zombie-liveness race that a `kill -0` poll would hit.
|
||||
run_audit() {
|
||||
local limit="$1" out="$2"
|
||||
|
||||
: >"${out}"
|
||||
"${NPM_BIN}" audit --json "${AUDIT_ARGS[@]}" "${SCOPE_ARGS[@]}" >"${out}" 2>/dev/null &
|
||||
local npm_pid=$!
|
||||
|
||||
(
|
||||
sleep "${limit}"
|
||||
kill -TERM "${npm_pid}" 2>/dev/null
|
||||
sleep 2
|
||||
kill -KILL "${npm_pid}" 2>/dev/null
|
||||
) >/dev/null 2>&1 &
|
||||
local killer_pid=$!
|
||||
|
||||
wait "${npm_pid}" 2>/dev/null
|
||||
local status=$?
|
||||
|
||||
kill -TERM "${killer_pid}" 2>/dev/null
|
||||
wait "${killer_pid}" 2>/dev/null
|
||||
|
||||
# 143 = SIGTERM, 137 = SIGKILL: the watchdog fired. The report is then
|
||||
# empty or truncated, which classify_report already reads as unreachable.
|
||||
if [ "${status}" -eq 143 ] || [ "${status}" -eq 137 ]; then
|
||||
return 124
|
||||
fi
|
||||
return 0
|
||||
}
|
||||
|
||||
# Classify one audit run. Prints a verdict word on stdout:
|
||||
# clean — audit completed, no vulnerabilities
|
||||
# vulnerable — audit completed, vulnerabilities present
|
||||
@@ -109,9 +163,25 @@ trap 'rm -f "${report_file}"' EXIT
|
||||
|
||||
attempt=1
|
||||
delay="${DELAY}"
|
||||
budget_exhausted=false
|
||||
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
|
||||
remaining=$(( DEADLINE - $(date +%s) ))
|
||||
if [ "${remaining}" -le 0 ]; then
|
||||
echo "npm audit (${SCOPE}): ${MAX_SECONDS}s retry budget exhausted before attempt ${attempt}"
|
||||
budget_exhausted=true
|
||||
break
|
||||
fi
|
||||
|
||||
# Never let one attempt outlive the overall budget.
|
||||
attempt_limit="${ATTEMPT_TIMEOUT}"
|
||||
if [ "${attempt_limit}" -gt "${remaining}" ]; then
|
||||
attempt_limit="${remaining}"
|
||||
fi
|
||||
|
||||
echo "npm audit (${SCOPE}) attempt ${attempt}/${ATTEMPTS} (limit ${attempt_limit}s, ${remaining}s of budget left)"
|
||||
if ! run_audit "${attempt_limit}" "${report_file}"; then
|
||||
echo "npm audit (${SCOPE}): attempt ${attempt} exceeded ${attempt_limit}s and was stopped"
|
||||
fi
|
||||
verdict_output="$(classify_report <"${report_file}")"
|
||||
verdict="$(printf '%s\n' "${verdict_output}" | head -1)"
|
||||
summary="$(printf '%s\n' "${verdict_output}" | sed -n '2p')"
|
||||
@@ -126,7 +196,7 @@ while [ "${attempt}" -le "${ATTEMPTS}" ]; do
|
||||
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
|
||||
"${NPM_BIN}" audit "${AUDIT_ARGS[@]}" "${SCOPE_ARGS[@]}" || true
|
||||
exit 1
|
||||
;;
|
||||
*)
|
||||
@@ -135,6 +205,12 @@ while [ "${attempt}" -le "${ATTEMPTS}" ]; do
|
||||
esac
|
||||
|
||||
if [ "${attempt}" -lt "${ATTEMPTS}" ]; then
|
||||
remaining=$(( DEADLINE - $(date +%s) ))
|
||||
if [ "${delay}" -ge "${remaining}" ]; then
|
||||
echo "npm audit (${SCOPE}): ${MAX_SECONDS}s retry budget exhausted"
|
||||
budget_exhausted=true
|
||||
break
|
||||
fi
|
||||
echo "retrying in ${delay}s"
|
||||
sleep "${delay}"
|
||||
delay=$((delay * 2))
|
||||
@@ -142,10 +218,16 @@ while [ "${attempt}" -le "${ATTEMPTS}" ]; do
|
||||
attempt=$((attempt + 1))
|
||||
done
|
||||
|
||||
if [ "${budget_exhausted}" = "true" ]; then
|
||||
gave_up="within its ${MAX_SECONDS}s retry budget"
|
||||
else
|
||||
gave_up="after ${ATTEMPTS} attempts"
|
||||
fi
|
||||
|
||||
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."
|
||||
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 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."
|
||||
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."
|
||||
exit 0
|
||||
|
||||
@@ -52,6 +52,8 @@ run_case() {
|
||||
out="$(NPM_AUDIT_CMD="${npm_bin}" \
|
||||
NPM_AUDIT_RETRY_DELAY=0 \
|
||||
NPM_AUDIT_ATTEMPTS="${NPM_AUDIT_ATTEMPTS:-3}" \
|
||||
NPM_AUDIT_ATTEMPT_TIMEOUT="${NPM_AUDIT_ATTEMPT_TIMEOUT:-60}" \
|
||||
NPM_AUDIT_MAX_SECONDS="${NPM_AUDIT_MAX_SECONDS:-240}" \
|
||||
NPM_AUDIT_REQUIRE_RESULT="${require}" \
|
||||
bash "${SCRIPT}" all 2>&1)"
|
||||
status=$?
|
||||
@@ -117,6 +119,54 @@ run_case "unknown payload shape is not treated as clean" 1 \
|
||||
"$(make_fake_npm npm-shape '{"metadata":{}}')" true \
|
||||
"could not reach the advisory endpoint"
|
||||
|
||||
# A hung endpoint must be cut off per attempt rather than inheriting npm's
|
||||
# own five-minute fetch timeout. This is the regression that cancelled the
|
||||
# Frontend job on 2026-09-04: three "attempts" ran for eleven minutes.
|
||||
hanging_npm="${WORK_DIR}/npm-hang"
|
||||
cat > "${hanging_npm}" <<'SH'
|
||||
#!/usr/bin/env bash
|
||||
sleep 300
|
||||
SH
|
||||
chmod +x "${hanging_npm}"
|
||||
|
||||
started=$(date +%s)
|
||||
NPM_AUDIT_ATTEMPT_TIMEOUT=1 NPM_AUDIT_MAX_SECONDS=10 \
|
||||
run_case "a hung audit is stopped at the per-attempt limit" 1 \
|
||||
"${hanging_npm}" true "was stopped" "could not reach the advisory endpoint"
|
||||
elapsed=$(( $(date +%s) - started ))
|
||||
if [ "${elapsed}" -gt 30 ]; then
|
||||
echo "FAIL: hung audit took ${elapsed}s; the per-attempt limit did not bound it"
|
||||
failures=$((failures + 1))
|
||||
else
|
||||
echo "ok: hung audit bounded in ${elapsed}s"
|
||||
fi
|
||||
|
||||
# The total budget, not just the attempt count, has to end the sequence, and
|
||||
# an exhausted budget must still respect the fail-closed/fail-open split.
|
||||
started=$(date +%s)
|
||||
NPM_AUDIT_ATTEMPTS=50 NPM_AUDIT_ATTEMPT_TIMEOUT=1 NPM_AUDIT_MAX_SECONDS=3 \
|
||||
run_case "the wall-clock budget ends the retry sequence" 1 \
|
||||
"${hanging_npm}" true "retry budget exhausted"
|
||||
elapsed=$(( $(date +%s) - started ))
|
||||
if [ "${elapsed}" -gt 25 ]; then
|
||||
echo "FAIL: 50 attempts under a 3s budget took ${elapsed}s; the budget did not bound them"
|
||||
failures=$((failures + 1))
|
||||
else
|
||||
echo "ok: wall-clock budget bounded 50 attempts in ${elapsed}s"
|
||||
fi
|
||||
|
||||
# An exhausted budget is still tolerated when the dependency graph is unchanged.
|
||||
NPM_AUDIT_ATTEMPTS=50 NPM_AUDIT_ATTEMPT_TIMEOUT=1 NPM_AUDIT_MAX_SECONDS=3 \
|
||||
run_case "an exhausted budget warns when dependencies unchanged" 0 \
|
||||
"${hanging_npm}" false "::warning::" "retry budget"
|
||||
|
||||
# A real advisory must still fail even under a tight budget: the bound may
|
||||
# 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 \
|
||||
"vulnerabilities present"
|
||||
|
||||
if [ "${failures}" -ne 0 ]; then
|
||||
echo "${failures} test(s) failed"
|
||||
exit 1
|
||||
|
||||
Reference in New Issue
Block a user