stacklok / toolhive / 32710296244

70%

Build Type

push

github

Committed by web-flow

Commit Message

Report a rejected stored credential as re-login required (#6389)

* fix(auth): report a rejected stored credential as re-login required

When a non-interactive token source exhausted every cache tier, Token()
returned the last error verbatim. For a refresh token the IdP had rejected
(expired, revoked, or rotated out from under us) that meant the raw
invalid_grant surfaced instead of the caller's FallbackErr sentinel.

That inverted the two outcomes. A cache MISS got the actionable "you must log
in again" error, while a DEAD credential, the case that genuinely requires an
interactive login, got an opaque OAuth error that every downstream consumer
reads as a transient provider fault. The LLM proxy renders it as a 502
server_error rather than the 401 that names `thv llm setup`, so a user whose
refresh token died sees a gateway problem and has nothing to act on.

Token() now classifies the terminal error: a permanent token-endpoint verdict
is reported as FallbackErr with the cause still reachable via errors.As, while
everything else (5xx, 429, a WAF page, a locked keyring) keeps surfacing
verbatim so callers can retry or fix the real problem. The rendered message
carries only the sentinel and the RFC 6749 error code, never the raw
token-endpoint response body, which can echo back bearer material.

The stored credential is deliberately not deleted on a rejection. With an IdP
that rotates refresh tokens, a sibling process may have just written a newer
token under the same key and the next call re-reads the secrets provider;
deleting here would destroy that cross-process recovery.

The transient/permanent rules move to a new leaf package, pkg/auth/oautherr,
so the token source and the workload auth monitor share one implementation
instead of each carrying its own copy. The monitor's private helpers now
delegate to it, leaving its behaviour and tests unchanged.

ErrTokenRequired's wording widens from "no cached credentials found" to "no
usable cached credenti... (continued)

Coverage Stats

42 of 42 new or added lines in 3 files covered. (100.0%)

6 existing lines in 3 files now uncovered.

74532 of 107642 relevant lines covered (69.24%)

88.53 hits per line