Demo/pr 127 coderabbit - #2
Open
evancoderabbit wants to merge 4 commits into
Open
Conversation
Mapbox APIs take the access token as a URL query parameter, and OpenTelemetry's HTTP/undici auto-instrumentation records the full request URL on every client span as `url.full` and `url.query`. Nothing scrubbed those attributes before export, so any operator who followed the tracing setup in docs/tracing.md and set OTEL_EXPORTER_OTLP_ENDPOINT had the caller's token copied verbatim into their telemetry backend on every authenticated tool call. Wrap the OTLP exporter in a RedactingSpanExporter that rewrites `access_token=<value>` to `access_token=***` across all string span attributes (and span event attributes) on the way out, reusing the existing redactToken() helper. Redacting at the exporter rather than in an instrumentation requestHook means the scrub covers every attribute on every span, including attribute names introduced by future semantic-convention or instrumentation changes, instead of only the four URL attribute names known today. Spans needing no redaction are passed through by reference; spans that do are copied field-by-field from the prototype chain rather than mutated, so the SDK's own span state is untouched and the copy survives SDK version differences in span shape. docs/tracing.md previously claimed sensitive data was protected in tracing output without qualification. It now describes what the exporter actually does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Masking every token to `access_token=***` threw away context that is
useful in a trace and not sensitive: which kind of token was used, and
which account the request billed to.
Mapbox tokens are JWTs (`<prefix>.<payload>.<signature>`) whose payload
carries the account name under `u`, so redaction now decodes the payload
and emits `<prefix>.<account>.redacted` — `pk.some-account.redacted`,
`sk.some-account.redacted`, `tk.some-account.redacted`. The signature,
which is the part that actually authenticates, still never leaves the
process.
Anything that does not parse cleanly as such a token — unrecognized
prefix, wrong segment count, payload that is not base64 JSON, payload
with no usable `u` field, or an opaque legacy value — falls back to
`access_token=***`. An unrecognized shape is never partially disclosed on
the assumption it was harmless. The account name is also matched against
`[A-Za-z0-9_-]{1,64}` before being emitted, so a hostile payload cannot
inject arbitrary text into a span attribute.
This changes `redactToken()` itself, so the same enriched placeholder now
applies to the debug logs and MCP client error responses that already
used it, not only to exported spans.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The fixture tokens carried a real token's decoded payload (account name and 'a' claim). The signature was already fake, so they were never usable credentials, but there is no reason for a public test file to name a real account. Switch to 'example-account'. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The comment above ACCOUNT_NAME_PATTERN asserted that Mapbox usernames are lowercase alphanumerics with dashes and underscores. That was not verified against anything, and it contradicted the regex directly below it, which accepts uppercase. Describe what the allowlist is actually for — bounding what a token payload can write into a span attribute or log line — and point at the existing username validation in StyleComparisonTool as the precedent for the character set, rather than claiming a rule about Mapbox account naming. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Owner
Author
|
@coderabbitai review |
Action performedReview triggered.
|
evancoderabbit
marked this pull request as draft
August 10, 2026 15:12
evancoderabbit
marked this pull request as ready for review
August 10, 2026 15:12
Action performedReview triggered.
|
Action performedReview triggered.
|
Owner
Author
|
@coderabbitai review please |
|
Action performedReview triggered.
|
|
Action performedReview triggered.
|
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
2 participants
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.
Description
Testing
Checklist
Additional Notes