[RES-02-D] Forward-auth and JWKS resilience, failing closed

[victornife]

Note

Sub-issue of #142 (slice 2b). Depends on: the retry primitive sub-issue.

Scope

forward_auth.url and jwt.jwks_url resolve through the shared primitives: a pool-of-one by default,
an optional named [[upstreams]] for load balancing, and a policy-driven timeout replacing the hardcoded
Timeout: 10 * time.Second in forwardHTTPClient.

Kept as one sub-issue because both share the client construction and the fail-closed rule; splitting them
would separate decisions that must be made together.

Important

Fail closed. When the forward-auth circuit is open, or admission rejects the subrequest, the request
is rejected — never allowed through unauthenticated. A resilience control may never become an
authentication bypass. "Fail open on dependency failure" is a defensible-sounding default elsewhere and
is a critical vulnerability here. Needs an explicit test.

Note

Scope limit. Auth and JWKS inherit transport and TLS resilience only. HTTP application failures
do not trip the breaker in this tranche. An auth service returning 500 forever is a received response
and never trips it. The two dependencies disagree on what failure even means: JWKS treats
StatusCode != 200 as a refresh failure, forward-auth treats non-2xx as a legitimate application result
(401/403 are the answer). Making 5xx/429 trip a breaker needs a generic outcome-classification
seam — the same seam retry_on needs — so it is one deferred extension with two consumers, not an auth
special case. Do not add an auth-specific second breaker.

Acceptance criteria

• Both resolve through the shared primitives with a configurable timeout.
• Fail closed on circuit-open and on admission rejection, with a test.
• Transport-only breaker scope documented in docs/auth.md.
• No behaviour change for 2xx/4xx application responses.

Tests

Fail-closed regression; timeout honoured from policy; transport failure trips the breaker; a persistent
500 does not (documented, asserted); JWKS refresh failure path unchanged.

[juliaggas]

Merged

PR #307 merged as f2a4debde68b15ce225491d9fe16b3a51de3b9ee. All 34 CI checks passed.

Acceptance criterion	Status
Both resolve through the shared primitives with a configurable timeout	done
Fail closed on circuit-open and on admission rejection, with a test	done for admission; circuit-open structural — see below
Transport-only breaker scope documented in docs/auth.md	done
No behaviour change for 2xx/4xx application responses	done

What changed

forward_auth.url and jwt.jwks_url now resolve through the shared upstream primitives: a host naming a configured [[upstreams]] is load-balanced across replicas, anything else becomes a pool of one. Both gain admission, passive health and retry accounting. timeout replaces what was a hardcoded 10 * time.Second in two places; unset keeps exactly that value.

Every failure denies

An admission rejection, a spent retry budget and an exhausted pool all deny with 503. The property follows from the shape rather than a rule at each call site: dependency.do returns (*http.Response, error) and nothing else, every one of those conditions arrives as an error, and the caller denies on error. There is no third outcome to get wrong.

I broke it on purpose to check the tests could fail — and the first attempt proved nothing. The patch had the wrong indentation, so the substitution was a silent no-op and the suite passed, which looked exactly like a successful verification. Recording that because it is the failure mode of this kind of check: a sanity check that cannot fail proves nothing while appearing to prove everything.

Done correctly, both fail-closed tests fail (status = 403, want 503). That also shows something worth keeping: with the error branch disabled the request is still denied, because a zero-valued forwardResult has ok = false. The structure fails closed twice over.

Scope: transport only

Outcome	Retried?	Trips passive health?	Client sees
Connection refused / reset / timeout	yes	yes	503 if no replica answers
2xx	no	no	allowed
401 / 403	no	no	the auth service's own response
500 forever	no	no	the auth service's own 500

Asserted by counting calls and checking backend availability, not just status codes — an application status that silently retried, or that took a healthy replica out of rotation, would pass a status-only assertion. No auth-specific second breaker was added, per the issue's instruction; 5xx/429 needs the generic outcome-classification seam that retry_on also needs.

On the circuit-breaker criterion

The breaker does not exist yet (#143). Admission rejection is tested now; circuit-open is covered structurally — it will surface through the same Pool.Do error path that already denies. #143 should add its own explicit fail-closed assertion rather than inherit this one on trust.

Coverage

codecov/patch failed first and named three real gaps, one of them pre-existing: no test built handlers for a location with auth at all, so the whole wiring loop was unexercised — touching those lines is what surfaced it. authDependencyPool had no test either, and it makes decisions worth pinning: a host naming a configured upstream must be load-balanced rather than dialled literally, and a URL with no port must still produce a backend address, because a backend always has a port even when a URL does not. Every added line in the branch is now covered.

Gates

lint-full 0 issues · ci-fast · test-full · -race · generated-check clean (2 new lifecycle entries, exemptions still 0) · docs-check 1305/0 · 2 new audited entries in docs/config-value-contract.json.

All four children of #142 are now merged and closed.