Skip to content

Send the versioned User-Agent from every CLI HTTP client - #2096

Open
Soph wants to merge 1 commit into
mainfrom
soph/user-agent-everywhere
Open

Send the versioned User-Agent from every CLI HTTP client#2096
Soph wants to merge 1 commit into
mainfrom
soph/user-agent-everywhere

Conversation

@Soph

@Soph Soph commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator

https://entire.io/gh/entireio/cli/trails/1122

Summary

Follow-up to #2049. That PR's premise — that api.Client already stamped entire-cli/<version> on "every other CLI request" — wasn't true. The biggest hole was the control plane: coreapi.newCrossJurisHTTPClient wrapped httpclient.NewTransport with no UserAgentTransport, so every entire org|project|repo|grant|search|api|auth request went out as Go-http-client/2.0.

Fixed at the client constructors, not the call sites, so a new request site can't silently regress — the structural version of a nit raised on #2049.

What was uncovered

Client Reaches Was sending
coreapi.newCrossJurisHTTPClient every control-plane call (~20 files' worth of commands) Go-http-client/2.0
auth's 3 bare clients, which carry clusterdiscovery's well-known fetch and the cluster-catalog GET https://entire.io/.well-known/entire-api.json, core /api/v1/clusters Go-http-client/2.0
pluginHTTPClient plugin release assets + checksums.txt Go-http-client/2.0

Already covered, unchanged: api.Client and its bearerTransport (which is also what entire-api cell clients use), versioncheck, dispatch (#2049), and git-remote-entire + remotehelper/transport, which pass their own binary identity.

Audited by enumerating every non-test http.Client{} construction and every http.NewRequest* site under cmd/ and internal/; each one is now either transport-wrapped or sets the header itself.

New helper

versioninfo.WrapTransport binds httpclient.UserAgentTransport to UserAgent() in one place rather than repeating the same two lines in three packages. httpclient stays binary-agnostic (it still takes UA as a field, which is how git-remote-entire supplies its own).

internal/coreapi importing cmd/entire/cli/versioninfo follows what that file already does — it imports cmd/entire/cli/auth today — and avoids a startup-order footgun: nothing has to remember to call a setter before the first request.

The load-bearing detail: wrap innermost

The wrapper goes at the base of a transport chain, never on top. crossJurisRoundTripper builds two requests of its own — the RFC 8693 token exchange and the federation manifest fetch — and sends both straight to t.base, bypassing anything wrapped outside it.

TestNewCrossJurisHTTPClient_StampsUserAgentOnEveryHop pins this. Moving the wrapper outside the round tripper makes it fail:

federation request 0: User-Agent = "Go-http-client/1.1", want "entire-cli/dev"
exchange request 0:   User-Agent = "Go-http-client/1.1", want "entire-cli/dev"

It drives the real newCrossJurisHTTPClient rather than the existing transportFor(t) helper, because the wiring is what's under test.

Test plan

  • TestWrapTransport_StampsUserAgent (table: unset / Go default / caller value) + asserts the caller's own request is not mutated, and TestWrapTransport_NilNextUsesDefaultTransport.
  • TestNewCrossJurisHTTPClient_StampsUserAgentOnEveryHop — 421 → federation manifest → bare 401 → RFC 8693 exchange → retry, asserting the UA on all four hop kinds and failing if any hop stops happening. Mutation-verified above.
  • TestAuthHTTPClients_SendVersionedUserAgent — table over dataAPIDiscoveryClient (https and loopback-http branches, the latter checking the stamp survives dataAPIHTTPDiscoveryTransport layered on top) and cellExchangeHTTPClient.
  • TestPluginHTTPClient_SendsVersionedUserAgent.
  • New recorders are mutex-guarded, per the authRecorder convention in internal/coreapi: HTTP completion is not a happens-before edge the race detector recognises.
  • mise run fmt && mise run lint clean; mise run test:ci green.

Left alone on purpose

  • The OAuth flows send an unversioned UA. auth's device-flow, auth-code, and refresh clients set UserAgent: oauthClientID, i.e. User-Agent: entire-cli with no version. That value is deliberately the OAuth client ID and it is read by the login server, not the BFF, so retargeting it is a server-coupled product call rather than part of a mechanical sweep. Worth deciding separately if login traffic ever needs version attribution.
  • entire-cli/dev is not semver. Local and go build binaries send it, and entire.io#3783's gate passes non-semver untouched by design.

Sequencing note before flipping CLI_MIN_VERSION

This makes a large amount of previously-anonymous traffic gate-visible, which is the point, but it also means the blast radius of flipping CLI_MIN_VERSION grows the moment it ships. Worth letting version telemetry show the real distribution of old clients first. Also worth confirming server-side that the semver parse tolerates prerelease suffixes — nightlies send entire-cli/0.6.2-nightly.202605160654.ddf1a331.

🤖 Generated with Claude Code


Note

Medium Risk
Touches every control-plane request plus discovery and plugin downloads by changing shared HTTP transports. Behavior is header-only, but wrapping order in the cross-jurisdiction client is load-bearing.

Overview
Stamps entire-cli/<version> on CLI HTTP clients that were still sending Go's default User-Agent, by wrapping transports at construction time rather than at each call site.

Adds versioninfo.WrapTransport and applies it to the control-plane client (innermost, so federation fetch and RFC 8693 exchange are covered), auth discovery/cell-exchange clients, and plugin asset downloads. Tests pin the header on those paths, including every hop of a 421 → federation → exchange chain.

Reviewed by Cursor Bugbot for commit b37febe. Configure here.

#2049 fixed dispatch on the premise that api.Client already stamped
entire-cli/<version> on every other CLI request. It didn't. The largest
gap was the control plane: newCrossJurisHTTPClient wrapped
httpclient.NewTransport with no UserAgentTransport, so every
org/project/repo/grant/search/api/auth request went out as
Go-http-client/2.0. The auth package's three bare clients (carrying
clusterdiscovery's well-known fetch and the cluster-catalog GET) and the
plugin asset client were unstamped too.

Fix at the client constructors rather than the call sites, so a new
request site can't silently regress. versioninfo.WrapTransport binds
httpclient.UserAgentTransport to UserAgent() in one place instead of
repeating it in three packages; httpclient keeps taking UA as a field, so
git-remote-entire still supplies its own identity.

Wrap innermost, at the base of a transport chain rather than on top:
crossJurisRoundTripper builds the RFC 8693 exchange and the federation
manifest fetch itself and sends both straight to t.base, so an outer
wrapper would miss them. Moving the wrapper outside makes
TestNewCrossJurisHTTPClient_StampsUserAgentOnEveryHop report
Go-http-client/1.1 on those two hops.

The OAuth device-flow, auth-code, and refresh clients are left alone:
they set UserAgent to the OAuth client ID ("entire-cli", unversioned),
which the login server reads, so retargeting it is a server-coupled
decision rather than part of this sweep.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@Soph
Soph requested a review from a team as a code owner August 21, 2026 13:20
Copilot AI lite review requested due to automatic review settings August 21, 2026 13:20

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit b37febe. Configure here.

var pluginHTTPClient = &http.Client{
Timeout: 5 * time.Minute,
Timeout: 5 * time.Minute,
Transport: versioninfo.WrapTransport(nil),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale User-Agent at package init

Medium Severity

pluginHTTPClient calls versioninfo.WrapTransport in a package-level var initializer, which freezes UserAgent() before main runs versioninfo.Load(). For go install @&lt;version&gt; builds (no ldflags), plugin asset and checksum requests keep sending entire-cli/dev while every lazily built client sends the real version. The new test cannot catch this because both sides still read dev under go test.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit b37febe. Configure here.

Copilot AI 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.

Pull request overview

This PR standardizes outbound HTTP identification by ensuring CLI HTTP clients consistently send a versioned User-Agent (entire-cli/<version>), including control-plane cross-jurisdiction hops, auth discovery/exchange clients, and plugin asset downloads. It does this primarily by wrapping transports at client-construction time to prevent future call sites from silently omitting the header.

Changes:

  • Add versioninfo.WrapTransport to stamp a versioned User-Agent via httpclient.UserAgentTransport, and apply it to several HTTP client constructors.
  • Fix control-plane cross-jurisdiction client wiring so the federation manifest fetch and RFC 8693 token exchange also carry the versioned User-Agent (wrapper placed under the cross-juris transport).
  • Add targeted tests to pin User-Agent behavior across these clients and hops.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
internal/coreapi/cross_juris_useragent_test.go Adds an integration-style test asserting the control-plane client stamps User-Agent on every hop (wrong core, federation fetch, home core, exchange).
internal/coreapi/cross_juris_transport.go Wraps the base transport with the versioned User-Agent wrapper under the cross-juris transport to cover synthesized requests.
cmd/entire/cli/versioninfo/transport.go Introduces versioninfo.WrapTransport helper to attach httpclient.UserAgentTransport using versioninfo.UserAgent().
cmd/entire/cli/versioninfo/transport_test.go Adds unit tests verifying WrapTransport stamps User-Agent, doesn’t mutate caller requests, and handles nil next.
cmd/entire/cli/plugin_fetch.go Stamps plugin download traffic by adding a transport wrapper to the shared plugin download client.
cmd/entire/cli/plugin_fetch_useragent_test.go Adds a test asserting plugin asset/download requests include the versioned User-Agent.
cmd/entire/cli/auth/useragent_test.go Adds tests asserting auth discovery and cell-exchange HTTP clients send the versioned User-Agent.
cmd/entire/cli/auth/data_api.go Wraps the data API discovery client transport and simplifies the plain-HTTP discovery transport wiring.
cmd/entire/cli/auth/control_plane.go Wraps the control-plane cluster discovery HTTP client transport to stamp the versioned User-Agent.
cmd/entire/cli/auth/cell_data_api.go Wraps the cell exchange HTTP client (including the test seam transport) to stamp the versioned User-Agent.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines 97 to 101
var pluginHTTPClient = &http.Client{
Timeout: 5 * time.Minute,
Timeout: 5 * time.Minute,
Transport: versioninfo.WrapTransport(nil),
CheckRedirect: func(req *http.Request, via []*http.Request) error {
if len(via) >= maxAssetRedirects {
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants