stacklok / stacklok/toolhive

Consolidate duplicate JWKS fetch and cache machinery

Open
#6,319 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

authentication enhancement go needs-triage
Dominant language
Go
Stars
2.2k
Forks
300
Avg merge
1d 15h
Merged PRs (30d)
184

Description

Description

ToolHive fetches and caches remote JWKS in two places, built independently on
the same libraries and converging on the same design. Neither knows about the
other.

pkg/auth/token.go — the inbound TokenValidator used by every proxy and CLI
path. Holds a jwk.Cache (jwx v3) created over
httprc.NewClient(httprc.WithHTTPClient(...)), registers the JWKS URL in
ensureJWKSRegistered (:698), and resolves keys in getKeyFromJWKS (:896) via
Lookup then LookupKeyID. Its HTTP client comes from
networking.NewHttpClientBuilder(), gated by AllowPrivateIP and
InsecureAllowHTTP. Discovery retries with cenkalti/backoff.

pkg/authserver/server/tokenexchange/multi_issuer_validator.go — the
token-exchange subject-token validator. Holds a jwk.Cache per issuer
(externalIssuerConfig.jwksCache, :212), registered in registerOrRefresh
(:631) and reached through ensureRegistered (:589) and lookupJWKS (:700).
Adds a rate-limited refreshOnUnknownKid (:757), a fetch-failure backoff,
discoverJWKSURL (:803), ValidateJWKSURL (:860), a limitedBodyTransport
response cap, and per-issuer InsecureAllowHTTP / AllowPrivateIPs.

Same problem, same libraries, two solutions. The second is the more complete
one, and its extra machinery — the response cap, the rate-limited kid refresh,
per-issuer rather than global HTTP and private-IP flags — is exactly the kind
of hardening the first would want and does not have.

Scope

Extract one shared JWKS fetch-and-cache type and move both call sites onto it.

The API surface both implementations already imply:

  • resolve a key set for an issuer or JWKS URL, with caching
  • look up by kid, refreshing once on a miss, rate-limited
  • serve the last known good set when a fetch fails
  • an SSRF-guarded HTTP client from networking, with a bounded response body
  • HTTP and private-IP relaxations declared per issuer, never globally

Take the extraction from the two implementations on main. Do not regress
anything the token-exchange validator already does.

Explicitly out of scope

pkg/authserver/upstream/oidc.go is a third mechanism: it uses
coreos/go-oidc, whose IDTokenVerifier carries its own RemoteKeySet with
its own cache and refresh policy. It already receives an SSRF-guarded client
through oidc.ClientContext (:200). Replacing it means reimplementing ID token
verification, nonce handling and the rest of the verifier, which is a much
larger change with real risk for no clear gain. Leave it alone and say so in
the code.

Prior art, and what not to lift

The unpushed xaa-spike-1 branch has pkg/oauthproto/jwks.go (commit
6fc952750), a JWKSFetcher with per-issuer caching, refresh on kid miss,
HTTPS enforcement, a dial-time private-address guard and a stale-on-error path.

Do not cherry-pick it. It was written on 2026-08-04, before #6149 rebuilt
the token-exchange validator, and it is hand-rolled sync.Mutex caching over
go-jose where both live implementations use jwx jwk.Cache + httprc.
Adopting it converts working code backwards: it would drop httprc's background
refresh, the response cap, and the validator's per-issuer flags in favour of a
single global one.

What it did get right is the API shape — resolve by issuer, look up by kid,
refresh once on miss, serve stale on error. Use it as a sketch of the interface
and nothing else.

Acceptance criteria

  • One shared type, both call sites converted, no third implementation left.
  • Per-issuer InsecureAllowHTTP and AllowPrivateIPs survive; neither becomes
    a process-wide switch.
  • The response-body cap applies to both call sites.
  • Refresh on unknown kid stays rate-limited, so an attacker supplying random
    kid values cannot drive unbounded fetches.
  • A fetch failure serves the last known good key set rather than failing all
    validation.
  • Key rotation still works end to end for both call sites.
  • No key material, token, or assertion is logged.
  • Existing token validation behaviour is unchanged; this is a refactor, not a
    behaviour change.

Why separately

pkg/auth/token.go is on the request path for every proxied workload, so this
wants its own review and its own test story rather than riding along inside a
feature branch.

Related

  • #6149 rebuilt the token-exchange validator's JWKS handling; this consolidates
    what that left duplicated.
  • Not a dependency of the SPIFFE client-auth epic (#6199). SPIFFE JWT-SVID
    authorities come from the trust bundle, not from a JWKS URI, so
    jwtsvid.ParseAndValidate makes no HTTP request. #6203's stated dependency on
    a shared JWKS fetcher is mistaken and is being corrected separately.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reading pkg/auth/token.go at ensureJWKSRegistered and getKeyFromJWKS, then compare pkg/authserver/server/tokenexchange/multi_issuer_validator.go at ensureRegistered, registerOrRefresh, lookupJWKS, refreshOnUnknownKid, and discoverJWKSURL. Trace the existing validation tests and verify that both call sites use one shared implementation while preserving per-issuer protections, bounded responses, rate-limited refresh, stale-on-error behavior, key rotation, and unchanged validation behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
authentication, backend, security
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.