Set a base User-Agent and append agent/<id> when a coding agent is detected - #509
Draft
ctufts wants to merge 4 commits into
Draft
Set a base User-Agent and append agent/<id> when a coding agent is detected#509ctufts wants to merge 4 commits into
ctufts wants to merge 4 commits into
Conversation
…tected The Node SDK sends no User-Agent today, so requests fall back to got's default. Establish mapbox-sdk-js/<version> as the base UA on every request, and append " agent/<id>" when a coding-agent env indicator is present (Node only; the allowlist + precedence mirrors mapbox/tilesets-cli's agent_detect.py). The browser client explicitly filters the user-agent header back out, since browsers forbid script-set UA via XHR.
ctufts
added
the
locationai-team-skills
Per review feedback, the fallback detector should only check whether AI_AGENT/AGENT is present, never read and forward its value - an arbitrary, unvalidated string should not become an "agent id" in production telemetry. Fold the fallback into the allowlist itself as its lowest-precedence entry, returning a fixed `custom-agent` id on presence instead of the variable's value. This also removes the charset/length validation that existed only to sanitize that value, since every returned id is now a fixed literal. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
OX Security reviewed this pull request — nothing to fix.
Branch |
The allowlist's presence-check branch (expectedValue === null) required a non-empty, non-whitespace value, so an env var explicitly set to "" or whitespace was treated as unset. Per feedback, existence is all that should matter - whether the key is present at all, not what it's set to. Check membership directly instead of trimming and testing truthiness. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Every allowlist entry now tests presence only, never a value: collapse the table from (agentId, [[envVar, expectedValueOrNull], ...]) to a flat (agentId, [envVar, ...]), and drop the equality-check branch in scanEnv entirely. The warp entry compared TERM_PROGRAM against "WarpTerminal" - dropped outright, since TERM_PROGRAM is set by most terminal emulators (iTerm2, Apple Terminal, VS Code, Hyper, ...), not just Warp, and an existence-only check on it would misidentify most terminal sessions. vtcode's VTCODE has no such collision risk and stays as a plain presence check. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The Node SDK doesn't send its own
User-Agenttoday; requests just getgot's default (got/11.8.5 ...). This PR gives us a real one:mapbox-sdk-js/<version>on every request, withagent/<id>appended when the process env shows we're running under a coding agent (Claude Code, Codex, Cursor, etc.).This changes the User-Agent on every Node request the SDK makes, which shows up in server-side logs for every consumer. Calling that out explicitly since it needs reviewer sign-off before merge.
Changes
lib/helpers/agent-detect.js(new): Node-onlydetectAgent(). Checks a hardcoded allowlist of about 24 known coding-agent env indicators in priority order, then falls back toAI_AGENT/AGENTif set. Fallback values get validated against a safe charset (/^[\w.-]{1,64}$/) first, since an env var could contain anything and a stray newline shouldn't be able to crash every request. Returnsnulloutside Node (browser bundle) or if readingprocess.envthrows.lib/helpers/sdk-version.js(new): resolvesmapbox-sdk-js/<version>frompackage.jsonat runtime so the UA can't drift from the published version.lib/classes/mapi-request.js:defaultHeadersnow always setsuser-agent, appendingagent/<id>when detected. A caller-suppliedUser-Agentstill overrides it, no duplicate header.lib/browser/browser-layer.js: filtersuser-agentout before callingxhr.setRequestHeader, since browsers won't let scripts set User-Agent via XHR. This is what broke while testing the change (see Testing below). Browser agent-tagging itself is out of scope here; that'll need its own mechanism.rollup.config.js/package.json: addedrollup-plugin-jsonso the UMD browser bundle can inline the version forsdk-version.js.Testing
Full suite passes (490/490), with new coverage for the allowlist, the
AI_AGENT/AGENTfallback and its charset validation (including a header-injection attempt with an embedded newline), the Node/browser guard, and the header merge/override path. Lint and bundle both clean. Manually verified against a realMapiRequest:CLAUDECODE=1producesmapbox-sdk-js/0.16.3 agent/claude-code, a clean env produces just the base UA, and anAI_AGENTvalue with a newline gets rejected without crashing.Downstream impact
Every Node consumer's requests to
api.mapbox.comwill show a new User-Agent going forward. Worth knowing if anyone has UA-based logging, analytics, or allowlisting downstream. No change for browser consumers (the header gets filtered before it reaches the network, same as today). No change to auth, retries, or other request semantics.Operational considerations
None. Pure client-side header change, no deploy or feature flag needed. Takes effect on the next version bump.