Release per-issuer JWKS refresh workers on auth-server shutdown

[tgrunnagle]

Summary

MultiIssuerTokenValidator starts one jwk.Cache per configured trusted issuer, each running an httprc worker pool (~3 goroutines) rooted at context.Background(). Nothing ever released them: neither Server.Close() nor CloseIdleConnections reached the validator, and jwk.Cache.Shutdown was never called. A long-lived process that reconstructs the auth server to change its upstream set therefore leaked one worker pool per trusted issuer on every reconstruction, unbounded. This is the goroutine half of #6479; #6480 fixed the HTTP-connection half.

• Why: each auth-server reconstruction abandoned every issuer's JWKS refresh worker pool for the life of the process, and a reconstruction that failed after buildUpstreams but during buildProvider abandoned a half-built validator that no deferred cleanup covered.
• What: the validator now owns a scoped context that roots every per-issuer cache, exposes Close() to tear them all down, and is held on server so both normal shutdown and the failed-reconstruction path release it.

Closes #6482

Type of change

• Bug fix
• New feature
• Refactoring (no behavior change)
• Dependency update
• Documentation
• Other (describe):

Test plan

• Unit tests (task test)
• E2E tests (task test-e2e)
• Linting (task lint-fix)
• Manual testing (describe below)

Tests for pkg/authserver and pkg/authserver/server/tokenexchange pass under -race. The new Close() unit test is deterministic: jwk.Cache.Shutdown returning nil proves its workers drained within the timeout. A server-wiring test confirms the validator is built, held on server, and shut down by Close().

Changes

File	Change
pkg/authserver/server/tokenexchange/multi_issuer_validator.go	Add a validator-scoped context.CancelFunc; root each per-issuer jwk.Cache in that context; add Close() (cancel first, then drain each cache via Shutdown) and shutdownJWKSCache(); clean up already-started caches on partial construction failure.
pkg/authserver/server_impl.go	Hold the validator on server; build it eagerly in buildProvider and return it; call Close() from server.Close() and newServer's deferred error path; join shutdown and storage-close errors.
pkg/authserver/server.go	Update the Scope: note on CloseIdleConnections to explain it deliberately does not release the JWKS workers.
pkg/authserver/server/tokenexchange/multi_issuer_validator_test.go	Close() unit test; t.Cleanup in the newMultiValidator test helper.
pkg/authserver/server_test.go	Server-wiring test covering validator ownership and shutdown.

Does this introduce a user-facing change?

No.

Implementation plan

Approved implementation plan

1. Add a validator-scoped context.CancelFunc field to MultiIssuerTokenValidator; root each per-issuer jwk.Cache in that context; add Close() that shuts down each cache and cancels the context; add construction-error cleanup; update stale comments.
2. Hold the validator on server; have buildProvider build it whenever trusted issuers exist and return it; call Close() from server.Close() and newServer's deferred error path.
3. Update the Scope: comment on CloseIdleConnections.
4. Tests: Close() unit test (deterministic — Shutdown returning nil proves workers drained) plus a server-wiring test.

Special notes for reviewers

• Cancel-first, then drain: Close() cancels the shared context up front so a slow-draining cache is already unwinding by the time its Shutdown is reached — Close does not serialize on each pool's full timeout. shutdownJWKSCache passes a fresh timeout context to Shutdown (not the cancelled validator-scoped one), or httprc's Controller.ShutdownContext would return immediately without waiting.
• CloseIdleConnections intentionally does not release the workers: that path must stay safe on a live server, which needs the workers to keep refreshing external issuers' keys. Its Scope: doc note was updated to say so.
• Out of scope: pkg/auth/token.go's TokenValidator has the same shape but already roots its cache in a caller-supplied context; per the issue's "may want to cover both" wording it is left alone here.

Generated with Claude Code

[codecov]

Codecov Report

❌ Patch coverage is 72.60274% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.23%. Comparing base (2ad87a3) to head (8a7e7bd).

Files with missing lines	Patch %	Lines
pkg/authserver/server_impl.go	62.16%	14 Missing ⚠️
...ver/server/tokenexchange/multi_issuer_validator.go	85.18%	4 Missing ⚠️
...hserver/server/tokenexchange/jwt_bearer_handler.go	33.33%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6493      +/-   ##
==========================================
+ Coverage   78.18%   78.23%   +0.05%     
==========================================
  Files         769      769              
  Lines       75049    75087      +38     
==========================================
+ Hits        58680    58748      +68     
+ Misses      16364    16334      -30     
  Partials        5        5              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[tgrunnagle]

Multi-agent review — Release per-issuer JWKS refresh workers on shutdown

Recommendation: COMMENT (non-blocking). The implementation is correct and safe to merge as-is. Correctness was independently confirmed by concurrency, security, and Go-correctness specialists across every traced path (fail-closed shutdown, no double-close, no leak path, idempotent, per-issuer SSRF isolation and may_act consent checks unchanged). All actionable findings are about test coverage plus one out-of-diff scope observation — none are defects in the shipped code.

5 specialist reviewers (concurrency & lifecycle, security, Go correctness, test coverage, general quality). Codex cross-review skipped (CLI not installed). 0 HIGH, 3 MEDIUM, 3 LOW.

#	Severity	Location	Summary
T1	MEDIUM	multi_issuer_validator_test.go	Close test's nil-return can't detect a Close that stops draining
T2	MEDIUM	multi_issuer_validator.go	Partial-construction cleanup loop (#6482's reachable leak) untested
C1	MEDIUM	factory.go (out of diff)	Exported Factory/JWTBearerIssuanceFactory fallback still builds un-closeable validators
A	LOW	server_impl.go	Dropped _ = cleanup-shutdown errors not logged/commented
C2	LOW	multi_issuer_validator.go	Sequential per-issuer Shutdown timeouts → worst-case httpTimeout×N
T3	LOW	server_impl.go	Deferred validator-Close error paths untested

Highest-value follow-ups: T1 and T2 — this is a leak-prevention change whose value is a test that fails when the leak returns, and neither currently does. A runtime.NumGoroutine() delta or goleak around the Close test and a two-issuer partial-construction case would close both. Neither reflects a defect in the code being merged.

Nice work — the cancel-first-then-drain ordering and the "fresh context or Shutdown returns immediately" handling are exactly right, and building shared eagerly also closes the token-exchange-only leak path as a bonus.

_{🤖 Multi-agent review via /pr-review}