Set User-Agent on OAuth token refresh requests#5168
Conversation
ToolHive sets User-Agent: ToolHive/1.0 on OAuth/OIDC requests it constructs directly, but token refresh requests go through golang.org/x/oauth2 and fall back to Go's default Go-http-client User-Agent. Server operators investigating WAF blocks, rate limits, or IP allowlists cannot identify the traffic as ToolHive, and the default User-Agent is itself a common bot-detection signal that may trigger false positives. Fixes stacklok#5167 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Signed-off-by: Greg Katz <gkatz@indeed.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5168 +/- ##
==========================================
+ Coverage 67.53% 67.58% +0.04%
==========================================
Files 601 602 +1
Lines 61093 61109 +16
==========================================
+ Hits 41262 41298 +36
+ Misses 16714 16692 -22
- Partials 3117 3119 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
jhrozek
left a comment
There was a problem hiding this comment.
LGTM. The fix is well-scoped: the oauth2.HTTPClient context key is the documented extension point for injecting a custom *http.Client into golang.org/x/oauth2's refresh path, and a wrapping http.RoundTripper is the canonical way to add a header to requests the library builds internally. Test coverage looks good — the table-driven tests cover both the "UA set when missing" and "caller-supplied UA preserved" cases, plus the nil-Base fallback through a real httptest server, and the persisting-token-source test exercises both the standard and resource-aware refresh paths end-to-end.
Summary
User-Agent: ToolHive/1.0on the OAuth/OIDC requests it constructs directly (DCR, discovery, token introspection, GitHub provider), but token refresh requests go throughgolang.org/x/oauth2and fall back to Go's defaultGo-http-client/<version>User-Agent. Server operators investigating WAF blocks, rate limits, or IP allowlists cannot identify the traffic as ToolHive, andGo-http-client/*is itself a common bot-detection signal that may trigger false positives.oauthproto.UserAgentTransport(anhttp.RoundTripperthat sets the ToolHive User-Agent when one is not already set) andoauthproto.NewHTTPClient()(a small constructor that bundles the transport with a 30s timeout).oauth2.TokenSource:pkg/auth/oauth/non_caching_refresher.go,pkg/auth/oauth/flow.go, andpkg/auth/remote/persisting_token_source.go.Fixes #5167
Type of change
Test plan
task test) — including a newTestUserAgentTransport_RoundTriptable test, a newTestUserAgentTransport_NilBaseround-trip test, an extraUser-Agentassertion on the existingTestResourceTokenSource_Token_ExpiredToken, and a newTestCreateTokenSourceFromCached_SetsUserAgentcovering both refresh paths.task lint-fix)thvfrom this branch and ran a standalone harness that importspkg/auth/remote.CreateTokenSourceFromCachedagainst anhttptest.Serverand captures the User-Agent on the wire. Onmain(8c90184) both refresh paths emitGo-http-client/1.1; on this branch both emitToolHive/1.0.API Compatibility
v1beta1API, OR theapi-break-allowedlabel is applied and the migration guidance is described above.Does this introduce a user-facing change?
OAuth token refresh requests now advertise
User-Agent: ToolHive/1.0instead of Go's defaultGo-http-client/<version>. Operators of upstream IdPs can identify ToolHive traffic in logs, WAF rules, and rate-limit dashboards.Implementation plan
Approved implementation plan
Helper —
pkg/oauthproto/useragent.go:UserAgentTransportstruct with a publicBase http.RoundTripperfield (mirrors stdlibhttp.Transportandoauth2.Transportshape). NilBasefalls back tohttp.DefaultTransport.RoundTripclones the request before mutating headers (per the RoundTripper contract) and setsUser-Agentonly when not already set, so layered transports can override.NewHTTPClient()returns an*http.Clientwhose transport is&UserAgentTransport{}and whoseTimeoutis30 * time.Second. Used at all three call sites to lock in a single timeout invariant.Wire-up — three call sites:
pkg/auth/oauth/non_caching_refresher.go—httpClient: oauthproto.NewHTTPClient()on theNonCachingRefresherstruct. Covers both standard and RFC 8707 resource-aware refresh (the refresher injects this client viaoauth2.HTTPClientcontext value before each refresh). This also coverspkg/auth/oauth/resource_token_source.go, which delegates toNonCachingRefresher.pkg/auth/oauth/flow.go(processTokennon-resource branch) — build a*http.Clientviaoauthproto.NewHTTPClient()and inject it viaoauth2.HTTPClientinto the context passed tooauth2Config.TokenSource. Preserves the existingcontext.Background()reasoning (the OAuth callback ctx is cancelled when the callback server shuts down).pkg/auth/remote/persisting_token_source.go(CreateTokenSourceFromCachednon-resource branch) — same pattern; replaces the previouscontext.TODO().Why the helper lives in
pkg/oauthproto: co-located with the existingoauthproto.UserAgentconstant; the package already owns "set the ToolHive UA on outbound OAuth requests" viapkg/oauthproto/dcr.goandpkg/oauthproto/discovery.go. A future PR that fixes non-OAuth gaps (e.g. webhook delivery, transparent proxy health checks, etc.) can import the transport from here or move it topkg/networkingif that ends up being the better long-term home.Special notes for reviewers
UserAgentTransportcalls out the WAF/attribution motivation so the rationale is durable in the source.TestUserAgentTransport_NilBaseexercises thenil → http.DefaultTransportfallback through a realhttptest.Serverround-trip rather than relying on a recorder, since a recorder would mask the fallback.flow.go'sprocessTokenchange — that path requires a full OAuth callback flow to exercise. Thenon_caching_refresherandpersisting_token_sourcetests verify the transport behavior; theflow.gowire-up is the same three-line pattern.pkg/auth/discovery,pkg/authserver/upstream,pkg/webhook).🤖 Generated with Claude Code