stacklok / stacklok/toolhive

DCR credential resolution never refreshes after a replica's startup, allowing cross-replica generation drift

Open
#6,496 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

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

Description

Summary

Each replica resolves its outbound DCR (Dynamic Client Registration) credentials for an upstream OAuth provider exactly once, at startup (buildUpstreamConfigs, called from the EmbeddedAuthServer constructor in pkg/authserver/runner/embeddedauthserver.go). The resolved client_id/client_secret are baked into the replica's in-memory OAuth2Config for the life of the process. Nothing ever re-resolves or refreshes it.

Separately, the Redis-backed DCRCredentialStore ties a cached credential's TTL to the upstream-supplied client_secret_expires_at (pkg/authserver/storage/redis.go, since #5195).

Combined, this means: if the upstream issues a time-limited client secret, and that TTL lapses while a replica is still running, a later-starting replica (rolling deploy straggler, autoscale, crash-restart) will see a cache miss, register a new client with the upstream ("generation B"), while the still-running replica keeps using its original registration ("generation A") for the rest of its life. Both generations are then live simultaneously behind the load balancer.

Failure mode

A user's login flow can start on a generation-A replica (redirected to the upstream using generation A's client_id) and have its callback land on a generation-B replica (no session affinity across the OAuth redirect). The generation-B replica tries to redeem the authorization code with its own (different) client credentials, which the upstream rejects — typically invalid_grant/unauthorized_client. This is a visible login failure for the affected request, not a silent security issue.

Reachability

Only occurs when:

  1. The upstream IdP sets a non-zero client_secret_expires_at on DCR registration (common for enterprise IdPs, not universal — many dev/test and some production IdPs issue non-expiring secrets, in which case this can't happen at all), AND
  2. Replicas are restarted/added at staggered times relative to each other, straddling that expiry.

Why this isn't a quick fix

This needs one of:

  • A scheduled re-resolution/refresh task tied to the credential's expiry, with safe hot-swapping of the in-memory OAuth2Config under concurrent use, or
  • A push-based invalidation mechanism (e.g. pub/sub) so a replica that (re)registers tells its siblings to refresh, or
  • Avoiding the problem by proactively rotating/refreshing well before expiry from one coordinated place, rather than reactively on whichever replica happens to hit a cache miss first.

All three require new infrastructure beyond the existing single-shot-resolve-at-startup architecture — not a bounded fix to the storage layer.

Origin

Flagged during review of #6474 (a fix for a different, narrower DCR cache-fill race — the simultaneous-first-fill race between replicas starting at the same time, fixed via PutIfAbsent/create-if-absent semantics). Confirmed via git log that both the single-shot-resolution architecture (#3540, #5044) and the TTL/expiry mechanism (#5195) predate #6474 by a wide margin — #6474 did not introduce or worsen this gap, it only fixed a different race in the same area.

Suggested first step

Before designing the full fix, check whether the upstream's invalid_grant/unauthorized_client rejection currently surfaces as a clear, actionable error to the end user/operator, or gets swallowed into a generic failure — a "fail loudly with a clear re-authenticate message" may be a reasonable interim mitigation even before real cross-replica refresh coordination is built.

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 with buildUpstreamConfigs and the EmbeddedAuthServer constructor in pkg/authserver/runner/embeddedauthserver.go, then trace DCRCredentialStore and its TTL handling in pkg/authserver/storage/redis.go. Inspect how upstream invalid_grant or unauthorized_client failures reach users and operators. Done would require an agreed approach for cross-replica credential refresh, or a clearly actionable interim failure message.

Written by the indexing model from the issue text.

Assessment

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.