Skip to content

feat(auth): mint Docker tokens from the stored access token - #3935

Open
dgageot wants to merge 1 commit into
docker:mainfrom
dgageot:new-auth
Open

feat(auth): mint Docker tokens from the stored access token#3935
dgageot wants to merge 1 commit into
docker:mainfrom
dgageot:new-auth

Conversation

@dgageot

@dgageot dgageot commented Aug 6, 2026

Copy link
Copy Markdown
Member

Docker Desktop's backend API only hands out its own access token — valid
for 15 minutes — and never the refresh token behind it, so an expired JWT
cannot be renewed: a stuck refresher on Desktop's side leaves every
caller with the same dead token. The access token docker login (and
Desktop's own sign-in) leaves in the credential store is long-lived, and
Docker Hub exchanges it for a fresh token with no user interaction.

This package owns that exchange: an in-memory token renewed ahead of its
expiry, credential re-checks so a logout or an account switch is noticed,
a shared cache so sibling processes don't each mint their own, retries
for transient failures (honouring Retry-After), a long back-off when the
token is refused, issuer and audience validation, and clock-skew
correction learned from Hub's Date header so expiry decisions follow the
issuer's clock rather than a drifting local one.

The access token itself never leaves the process except in the exchange
request: the endpoint is pinned to a Docker host, redirects are not
followed, and account passwords are never sent. Set
DOCKER_AGENT_NO_TOKEN_EXCHANGE to opt out entirely.

@dgageot
dgageot requested a review from a team as a code owner August 6, 2026 15:55
@aheritier aheritier added area/cli CLI commands, flags, output formatting area/core Core agent runtime, session management area/providers/openai For features/issues/fixes related to the usage of OpenAI models area/providers/anthropic For features/issues/fixes related to the usage of Anthropic models area/providers/gemini Google Gemini provider support kind/feat PR adds a new feature (maps to feat:). Use on PRs only. labels Aug 6, 2026
Sayt-0
Sayt-0 previously approved these changes Aug 6, 2026
aheritier
aheritier previously approved these changes Aug 6, 2026

@docker-agent docker-agent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 No issues found — LGTM! View logs.

@dgageot
dgageot dismissed stale reviews from Sayt-0 and aheritier via 0968511 August 8, 2026 19:46

@aheritier aheritier left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review of #3935 — head 0968511

CI on 0968511 is green (build-and-test, windows-tests, lint, license-check, build-image ×2; the three push-only jobs skipped). mergeable_state: blocked is REVIEW_REQUIRED, not a conflict.

This is a well-built package: singleflighted exchange, redirects refused, endpoint pinned to docker.com, passwords never exchanged, a 0600 token file in a 0700 directory, clock-skew learning, and genuinely thorough tests (go test -race ./pkg/hubauth/... ./pkg/desktop/... ./pkg/httpclient/... ./pkg/model/provider/base/... ./pkg/modelsgateway/... ./cmd/root/... passes locally). Two behavioural holes in pkg/desktop's new cache need fixing first.

[blocking] A refused token is served again — and then pinned in the cache for its whole life

GetTokenWithSource protects the direct fetch with wasRejected (pkg/desktop/login.go:78) but not the forced-refresh result: runTokenRefresh accepts any non-expiring token Desktop hands back (login.go:304) and login.go:85 passes it straight to remember. cached() (login.go:118-126) doesn't consult cache.rejected either, so the refused token is then served from memory without even an attempt to mint.

Here is the failing case — the token Docker refused comes back one log line later, same fingerprint:

WARN Docker Desktop served a token Docker refused fingerprint=081e6ec5
WARN Forcing a Docker Desktop token refresh
INFO Recovered a fresh token from Docker Desktop fingerprint=081e6ec5
    served token == refused? true (source "docker desktop")
    second call served refused token? true, mint attempted 0 time(s)
// pkg/desktop
func TestRejectedTokenIsNeverServedAgain(t *testing.T) {
	refused := makeToken(t, time.Now().Add(time.Hour))
	backend := &fakeBackend{token: refused, loggedIn: true}
	installFakeBackend(t, backend) // minting unavailable, as on a password-only login

	require.Equal(t, refused, GetToken(t.Context()))
	InvalidateToken(refused) // the gateway answered 401

	token, _ := GetTokenWithSource(t.Context())
	assert.NotEqual(t, refused, token, "a refused token must never be served again")
}

This is reached whenever minting is unavailable at that moment: DOCKER_AGENT_NO_TOKEN_EXCHANGE=1 (deterministic), a password-only docker login, an unreachable Hub, or the 5-minute rejectedCooldown after Hub itself refused the PAT. It also makes recovery worse than on main: once remembered, cached() short-circuits, so Desktop is never asked again for the remainder of that token's life (~15 min) and a fresh token Desktop does eventually produce is ignored — where today every call re-fetched.

Smallest fix: refuse to remember a rejected token (if wasRejected(token) { return "", SourceNone } at the top of remember) and skip rejected tokens in runTokenRefresh's accept condition.

[should-fix] docker logout / an account switch goes unnoticed while a minted token is cached

hubauth re-checks credentials every 30s on purpose (pkg/hubauth/token.go:35-39, token.go:88 — "so a docker logout or an account switch is picked up quickly"), but pkg/desktop's cache sits in front of it and its only exit condition is expiry (login.go:122). While it holds a minted token, hubauth.Token is never called, so the credential store is never consulted.

Here is the failing case:

minted := makeToken(t, time.Now().Add(4*time.Hour))
installFakeBackend(t, &fakeBackend{}) // Desktop signed out / absent
mintToken = func(context.Context) (string, error) { return minted, nil }
require.Equal(t, minted, GetToken(t.Context()))

// docker logout
mintToken = func(context.Context) (string, error) { return "", errors.New("no access token") }
assert.Empty(t, GetToken(t.Context()), "a logout must stop the previous account's token from being served")
Error: Should be empty, but was <the pre-logout token>
still serving the pre-logout token? true; credential store consulted 0 time(s)

After an account switch, requests keep going out as the previous account for the remainder of the minted token's lifetime. Either give the cached minted token its own re-check window, or don't cache minted tokens here at all and let hubauth.Token — already in-memory and singleflighted — be the cache, since it does the credential check itself.

[should-fix] New user-facing env vars and changed behaviour aren't documented

The description presents DOCKER_AGENT_NO_TOKEN_EXCHANGE as the way to opt out, but nothing under docs/ mentions it (nor DOCKER_AGENT_HUB_LOGIN_URL); the runtime-override table at docs/configuration/overview/index.md:159-164 is where readers look for it. Two more places now contradict the code:

  • docs/features/cli/index.md:676debug auth is documented as printing "Docker Desktop authentication info"; it now also reports minted tokens and prints a new Source field.
  • docs/guides/secrets/index.md:185 — presents Docker Desktop as how signed-in users are authenticated, with no mention that the stored access token is exchanged with Hub or that the minted bearer token is cached on disk.

Since this reads a credential from the user's store and sends it to Hub, the opt-out in particular deserves a documented home.

[optional] Shared cache and clock skew

  • pkg/hubauth/cache.go:74 writes the bearer token to disk in cleartext, while MCP OAuth tokens go through the keyring first and fall back to a 0600 file only when it is unavailable (pkg/tools/mcp/keyringstore/tokenstore.go:154). The 0700-dir reasoning is sound; reusing the keyring path would just be stronger. Relatedly, load (cache.go:51-56) trusts the fingerprint and expiry alone and skips the issuer/audience validate() a fresh exchange gets — free defence in depth.
  • learnClockSkew(resp.Header) runs before the status check (pkg/hubauth/exchange.go:164), so an error page from a TLS-terminating corporate proxy can shift now() process-wide — which also moves Expiring decisions for Desktop's tokens. Learning it only from a 200 would keep the source narrow.

Questions

  • Minted tokens carry aud: https://hub.docker.com, and DOCKER_TOKEN is consumed outside the models gateway too: injected into sandboxes (cmd/root/sandbox.go:232) and eval subprocesses (pkg/evaluation/eval.go:420), and forwarded when pulling agents from *.docker.com (pkg/config/sources.go:454). Have those paths been exercised with a minted token?
  • Minted Hub tokens outlive Desktop's 15-minute one. Is that longer TTL acceptable for the copy handed into a sandbox container?

@dgageot

dgageot commented Aug 9, 2026

Copy link
Copy Markdown
Member Author

Thanks — both behavioural findings reproduced exactly as written, log lines and all. All four are addressed in ad1e9e0, and the branch is now a single commit rebased on main.

[blocking] A refused token is served again

Fixed, and wider than the two spots you pointed at. usable(token) (token != "" && !Expiring && !rejected) now gates every path that can hand a token out: the direct fetch, the mint result, the refresh poll in runTokenRefresh, and the rate-limited reuse of refreshState.result — that last one was reachable too, since a token cached there can be refused after the refresh that produced it.

Two things I changed beyond the suggestion:

  • The check and the write are one operation. if wasRejected(token) { … } at the top of remember leaves a window: InvalidateToken can land between the check and the store, and that caller still gets the refused token once. remember now takes the lock, tests the tombstone and stores under it, returning ok=false so the caller falls through to the rest of the ladder. (As written it would also have deadlocked if placed after remember's own cache.Lock()wasRejected takes the same non-reentrant mutex.)
  • All refused tokens stay refused, not just the last one. cache.rejected was a single string, so with two tokens refused in a row the second overwrote the first's tombstone and Desktop could serve the first again. It is now a set, pruned of expired entries on insert.

cached() also skips a rejected token, so nothing that slipped in can be served later. Three tests: minting unavailable, reuse of the last refresh result, and Desktop regressing to an earlier refused token.

[should-fix] docker logout / account switch unnoticed

Fixed with the first of your two options: the cache entry now carries a staleAt, and cacheTTL = 30s matches hubauth's credCheckTTL, so the credential store is consulted at least as often as hubauth intends. I kept a cache in front of hubauth rather than removing it: without one, every LLM call pays the Desktop round-trip and logs a WARN from logUnusableToken whenever Desktop's token is unusable — which is exactly when the minted path is in play. The TTL applies to Desktop-sourced tokens too, so the same staleness bound covers both.

[should-fix] Docs

DOCKER_AGENT_NO_TOKEN_EXCHANGE and DOCKER_AGENT_HUB_LOGIN_URL are now in the runtime-override table, the debug auth row mentions the source, and docs/guides/secrets/index.md gains a Docker Authentication section covering the exchange, the cache and the opt-out. debug auth's own Short said "Docker Desktop" too, so that moved as well.

[optional] Both taken

  • load now runs the same validate() as a fresh exchange (issuer, audience, readability, renewal), so the file gets no more trust than the wire.
  • learnClockSkew moved after the status check. That exposed a wrinkle worth flagging: the date form of Retry-After was being resolved against now(), which on the error path is either the skew the error response just taught us (before) or none at all (after). It is now resolved against that response's own Date header — the clock the date was written in — with now() only as a fallback. Test covers a server an hour ahead and an hour behind.

Questions

  • DOCKER_TOKEN consumers: pkg/model/provider/base, the sandbox, evals and *.docker.com pulls all take whatever desktop.GetToken returns and send it as a bearer to a Docker host, so a minted token is the same shape of credential with the same audience; the gateway path is covered by tests. The sandbox and eval paths I exercised by hand only against Desktop's token — happy to sanity-check them with DOCKER_AGENT_NO_TOKEN_EXCHANGE unset and Desktop signed out before merge if you'd rather.
  • Longer TTL in a sandbox: agreed that it deserves a second look, but the copy handed in is one-shot and unrefreshable either way, so a longer-lived token is the difference between the sandbox working for its session and dying mid-run. I'd keep it as is and revisit if we ever hand sandboxes a token they can renew.

One nit I left alone: the legacy CAGENT_* note is already loose for DOCKER_AGENT_AUTO_UPDATE (no alias in code) — pre-existing, not something this PR should quietly redefine.

Docker Desktop hands out a 15-minute token it never renews, so a stuck
refresher leaves every caller with a dead one. The access token `docker
login` stores is long-lived, and Docker Hub exchanges it for a fresh one
with no user interaction.

pkg/hubauth owns that exchange: a token renewed ahead of its expiry,
credential re-checks so a logout is noticed, a cache shared with sibling
processes, retries, issuer and audience validation, and clock-skew
correction learned from Hub. Desktop's token still comes first while it
is usable; one the gateway refuses is never served again and the request
is replayed once with a fresh one. `docker agent debug auth` reports
which token is in use and where it came from, and
DOCKER_AGENT_NO_TOKEN_EXCHANGE opts out entirely.

Signed-off-by: David Gageot <david.gageot@docker.com>

@aheritier aheritier left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. All four findings are addressed — I checked each against the code, and two of the fixes are better than what I suggested: folding the tombstone check into remember() under one lock closes the check-then-store window, and making cache.rejected a set closes a hole I missed (Desktop regressing to an earlier refused token). Your learnClockSkew placement is also right: after the status check but still before validate(), which needs the skew.

CI is green; go test -race -count=2 on pkg/hubauth, pkg/desktop and pkg/httpclient is clean here too.

On the sandbox TTL question: it's moot — LoginKit declares DOCKER_TOKEN as a proxyManaged sentinel, so the real token never enters the sandbox. Only pkg/evaluation/eval.go:420 copies it into a container env, which is fine for a local eval run.

One non-blocking wording nit inline.

@@ -40,6 +42,22 @@ func GatewayAuthToken(ctx context.Context, env environment.Provider, gateway str
return token, nil

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Three messages still point at Docker Desktop as the only remedy, on the path where docker login is now the fix:

  • base.go:11NoDesktopTokenErrorMessage, returned on line 40 just above
  • gateway.go:26"sorry, you first need to sign in Docker Desktop to use the Docker AI Gateway"
  • config.go:177 — same string

They're reachable with a valid docker login in the store (DOCKER_AGENT_NO_TOKEN_EXCHANGE=1, or a password-only credential), so they send the user to the wrong remedy. You already fixed the equivalent wording in doctor.go and debug auth. This PR or a follow-up, either is fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/cli CLI commands, flags, output formatting area/core Core agent runtime, session management area/providers/anthropic For features/issues/fixes related to the usage of Anthropic models area/providers/gemini Google Gemini provider support area/providers/openai For features/issues/fixes related to the usage of OpenAI models kind/feat PR adds a new feature (maps to feat:). Use on PRs only.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants