stacklok / toolhive / 40981886298

73%

Build Type

push

github

Committed by web-flow

Commit Message

Keep RetrieveError in failed upstream refresh errors (#6999)

* Keep RetrieveError in upstream refresh errors

formatOAuth2Error flattened every token endpoint failure except
invalid_grant into plain text, so callers of GetValidTokens and
GetAllUpstreamCredentials could not tell a provider outage from a
definitive OAuth verdict such as invalid_client, even though
pkg/auth/oautherr already classifies those responses.

Implements changes for issue #6998:
- Return an unexported wrapper whose Error() is the existing sanitized
  text and whose Unwrap() exposes both that error (keeping
  ErrGrantRejected) and a body-scrubbed copy of the RetrieveError
- Drop formatOAuth2Error's prefix parameter, which every caller set to
  the same value
- Test the oautherr predicates end to end through the refresher and
  upstreamtoken service, and pin the unchanged error text
- Document the contract on ErrRefreshFailed, the service interface and
  the auth server storage architecture doc

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Clarify tokenRequestError docs and test coverage

Address low-severity review notes for issue #6998: note on
formatOAuth2Error that the RetrieveError stays in the chain, say why
the chained copy keeps Response, and assert the GetValidTokens message
also excludes the response body.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Narrow chained RetrieveError and harden tests

Addresses stacklok/toolhive#6999 review comments:
- MEDIUM pkg/authserver/upstream/oauth2.go (4415964810): reduce the
  chained Response to StatusCode and Status so the outgoing request and
  its credentials are unreachable
- LOW pkg/authserver/upstream/oauth2.go (4415964835): correct the type
  comment and state why it copies and why Unwrap is multi-valued
- LOW pkg/authserver/upstream/oauth2.go (4415964846): test the
  authorization code exchange path
- LOW pkg/authserver/upstream_refresh_rejection_test.go (4415964861):
  assert the RetrieveError,... (continued)

Coverage Stats

17 of 17 new or added lines in 1 file covered. (100.0%)

1 existing line in 1 file now uncovered.

91023 of 126221 relevant lines covered (72.11%)

131.29 hits per line