stacklok / toolhive / 25116589293

65%

Build Type

push

github

Committed by web-flow

Commit Message

Fix non-expiring upstream token handling and storage TTL bugs (#5092)

* Remove fabricated 1h expiry for non-expiring upstream tokens

RFC 6749 §5.1 marks expires_in as RECOMMENDED, not REQUIRED. When a
provider omits it, the client has no basis to invent an expiry. Slack
user tokens and GitHub OAuth App tokens deliberately omit expires_in
because they are genuinely non-expiring and have no refresh token.

The fabricated 1h ExpiresAt caused guaranteed hourly re-authorization
failures for those providers: the token appeared expired, a refresh was
attempted with no refresh token, ErrNoRefreshToken was returned, and
the token was dropped from the service.

Remove defaultTokenExpiration and the fabrication block. Pass
token.Expiry (the Go zero value when expires_in is absent) through
directly so downstream code sees zero ExpiresAt and treats the token
as non-expiring. Guard the expires_at log field to emit "none" instead
of the misleading year-0001 timestamp.

Fixes #5046

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* Fix zero-ExpiresAt semantics in IsExpired/IsExpiredAt

Both methods returned true for a zero ExpiresAt value because
now.After(time.Time{}) is always true (year 1 is in the past).
All production call sites guarded with !ExpiresAt.IsZero() before
calling, so the bug was harmless, but the methods were latent traps
for future callers.

Embed the zero guard inside each method: zero ExpiresAt means the
provider did not assert an expiry, so the token is treated as
non-expiring. Add nil-receiver guards for consistency and defensive
correctness.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* Fix storage backends to not evict non-expiring tokens

Both backends applied a default ~31-day TTL when ExpiresAt was zero,
which would silently evict non-expiring tokens and prevent retrieval.

memory.go: split StoreUpstreamTokens logic so zero ExpiresAt leaves
the timedEntry expiry as the zero value (never evicted). Guard
cleanupE... (continued)

Coverage Stats

126 of 134 new or added lines in 9 files covered. (94.03%)

90 existing lines in 3 files now uncovered.

60729 of 94679 relevant lines covered (64.14%)

59.95 hits per line