From d007c43fccbd95d14d552c219b9297064f21558d Mon Sep 17 00:00:00 2001 From: Valentin Salmanovicius Date: Tue, 28 Jul 2026 17:31:18 +0100 Subject: [PATCH 1/4] fix: strip access tokens from exported span attributes 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=` 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) --- CHANGELOG.md | 1 + docs/tracing.md | 7 +- src/utils/redactingSpanExporter.ts | 118 +++++++++++++++++ src/utils/tracing.ts | 5 +- test/utils/redactingSpanExporter.test.ts | 161 +++++++++++++++++++++++ 5 files changed, 290 insertions(+), 2 deletions(-) create mode 100644 src/utils/redactingSpanExporter.ts create mode 100644 test/utils/redactingSpanExporter.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 9334a82..dcf296f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ### Fixed +- **Tracing**: Access tokens are no longer included in exported spans. Mapbox APIs take the access token as a URL query parameter, and OpenTelemetry's HTTP/undici auto-instrumentation records the full request URL on client spans (`url.full`, `url.query`), so operators who configured `OTEL_EXPORTER_OTLP_ENDPOINT` had tokens copied verbatim into their telemetry backend. The OTLP exporter is now wrapped in a `RedactingSpanExporter` that rewrites `access_token=` to `access_token=***` across all string span attributes before export. - **`validate_expression_tool` / `validate_style_tool`**: Both tools now delegate expression and style validation to the official `@mapbox/mapbox-gl-style-spec` package (the same logic mapbox-gl-js runs at runtime) instead of a hand-rolled reimplementation. Fixes two disagreements with real mapbox-gl-js behavior around the `["zoom"]` expression placement rule (#123): - `validate_expression_tool` no longer false-positives on ordinary zoom-based `interpolate`/`step` expressions (it previously misidentified the interpolation-type argument, e.g. `["linear"]`, as an unknown operator). - `validate_style_tool` now correctly flags `["zoom"]` expressions nested inside e.g. a `case` (invalid per spec — zoom may only be used as the top-level input to `step`/`interpolate`), which it previously missed entirely. diff --git a/docs/tracing.md b/docs/tracing.md index bdc8280..8511940 100644 --- a/docs/tracing.md +++ b/docs/tracing.md @@ -37,7 +37,7 @@ Console output is incompatible with stdio and will corrupt JSON-RPC communicatio ### Security & Performance -- **Sensitive Data Protection**: Input parameters logged by size only, not content +- **Sensitive Data Protection**: Input parameters logged by size only, not content; access tokens are stripped from span attributes before export - **Minimal Overhead**: <1% CPU impact, ~10MB memory for trace buffers - **Configurable Sampling**: Support for production trace volume management - **Graceful Fallback**: No impact on functionality when tracing is disabled @@ -369,6 +369,11 @@ getNodeAutoInstrumentations({ ### Data Privacy - **Input sanitization**: Only input/output sizes are logged, not content +- **Access tokens**: Mapbox APIs take the access token as a URL query parameter, and HTTP + auto-instrumentation records request URLs on client spans (`url.full`, `url.query`). Every + span passes through a redacting exporter that rewrites `access_token=` to + `access_token=***` in all string attributes before anything is sent to your OTLP endpoint, + so tokens are not copied into your telemetry backend - **JWT validation**: Basic format validation only, no secret verification - **Error messages**: Error details are logged but sensitive data is protected diff --git a/src/utils/redactingSpanExporter.ts b/src/utils/redactingSpanExporter.ts new file mode 100644 index 0000000..b12d2b3 --- /dev/null +++ b/src/utils/redactingSpanExporter.ts @@ -0,0 +1,118 @@ +// Copyright (c) Mapbox, Inc. +// Licensed under the MIT License. + +import type { ReadableSpan, SpanExporter } from '@opentelemetry/sdk-trace-base'; +import type { ExportResult } from '@opentelemetry/core'; +import type { Attributes } from '@opentelemetry/api'; +import { redactToken } from '../tools/MapboxApiBasedTool.js'; + +/** + * Rewrites every string attribute value through `redactToken`, returning the + * original object when nothing changed so unaffected spans pass through as-is. + */ +function redactAttributes(attributes: Attributes): Attributes { + let redacted: Attributes | undefined; + + for (const [key, value] of Object.entries(attributes)) { + if (typeof value !== 'string') { + continue; + } + const clean = redactToken(value); + if (clean !== value) { + redacted ??= { ...attributes }; + redacted[key] = clean; + } + } + + return redacted ?? attributes; +} + +/** + * Collect every property name reachable on a span, including accessors defined + * on its prototype chain. Copying by key list rather than a hardcoded field list + * keeps this working across OpenTelemetry SDK versions, which have moved fields + * (e.g. `parentSpanId` to `parentSpanContext`) between releases. + */ +function collectKeys(span: ReadableSpan): Set { + const keys = new Set(); + + for ( + let current: object | null = span; + current && current !== Object.prototype; + current = Object.getPrototypeOf(current) + ) { + for (const key of Object.getOwnPropertyNames(current)) { + if (key !== 'constructor') { + keys.add(key); + } + } + } + + return keys; +} + +/** + * Return a copy of `span` with redacted attributes, or the span itself when no + * attribute needed redaction. The copy is a plain object rather than a mutation + * of the original, so the SDK's own span state is left untouched. + */ +function redactSpan(span: ReadableSpan): ReadableSpan { + const attributes = redactAttributes(span.attributes); + const events = span.events.map((event) => + event.attributes + ? { ...event, attributes: redactAttributes(event.attributes) } + : event + ); + + const attributesChanged = attributes !== span.attributes; + const eventsChanged = events.some( + (event, index) => event !== span.events[index] + ); + + if (!attributesChanged && !eventsChanged) { + return span; + } + + const copy: Record = {}; + for (const key of collectKeys(span)) { + // eslint-disable-next-line @typescript-eslint/no-explicit-any -- generic property copy across unknown SDK span shapes + const value = (span as any)[key]; + copy[key] = typeof value === 'function' ? value.bind(span) : value; + } + copy.attributes = attributes; + copy.events = events; + + return copy as unknown as ReadableSpan; +} + +/** + * Wraps a SpanExporter and strips `access_token` values from span attributes + * before they leave the process. + * + * Auto-instrumentation of `fetch`/`undici` records the full request URL on client + * spans (`url.full`, `url.query`), and Mapbox APIs take the access token as a + * query parameter. Without this wrapper, an operator who configures an OTLP + * endpoint gets those tokens copied verbatim into their telemetry backend. + * + * Redaction happens at the exporter rather than in an instrumentation + * `requestHook` so it covers every attribute on every span, including attribute + * names introduced by future semantic-convention or instrumentation changes. + */ +export class RedactingSpanExporter implements SpanExporter { + constructor(private readonly delegate: SpanExporter) {} + + export( + spans: ReadableSpan[], + resultCallback: (result: ExportResult) => void + ): void { + this.delegate.export(spans.map(redactSpan), resultCallback); + } + + shutdown(): Promise { + return this.delegate.shutdown(); + } + + forceFlush(): Promise { + return this.delegate.forceFlush?.() ?? Promise.resolve(); + } +} diff --git a/src/utils/tracing.ts b/src/utils/tracing.ts index ed599ce..8c83bd3 100644 --- a/src/utils/tracing.ts +++ b/src/utils/tracing.ts @@ -18,6 +18,7 @@ import { DiagLogLevel } from '@opentelemetry/api'; import { getVersionInfo } from './versionUtils.js'; +import { RedactingSpanExporter } from './redactingSpanExporter.js'; import { ATTR_SERVICE_INSTANCE_ID } from '@opentelemetry/semantic-conventions/incubating'; import { type HttpRequest } from './types.js'; @@ -194,7 +195,9 @@ export async function initializeTracing(): Promise { // Create SDK instance sdk = new NodeSDK({ resource, - traceExporter: exporter, + // Wrap the exporter so access tokens recorded in HTTP client span + // attributes (url.full, url.query) never reach the OTLP backend + traceExporter: new RedactingSpanExporter(exporter), instrumentations: [ getNodeAutoInstrumentations({ // Disable instrumentations that might be too noisy diff --git a/test/utils/redactingSpanExporter.test.ts b/test/utils/redactingSpanExporter.test.ts new file mode 100644 index 0000000..262ed2e --- /dev/null +++ b/test/utils/redactingSpanExporter.test.ts @@ -0,0 +1,161 @@ +// Copyright (c) Mapbox, Inc. +// Licensed under the MIT License. + +import { describe, it, expect, vi } from 'vitest'; +import { + BasicTracerProvider, + SimpleSpanProcessor, + type ReadableSpan, + type SpanExporter +} from '@opentelemetry/sdk-trace-base'; +import { ExportResultCode, type ExportResult } from '@opentelemetry/core'; +import { SpanKind } from '@opentelemetry/api'; +import { RedactingSpanExporter } from '../../src/utils/redactingSpanExporter.js'; + +const SECRET = 'sk.eyJ1IjoidGVzdHVzZXIifQ.signaturevalue'; + +class CapturingExporter implements SpanExporter { + readonly captured: ReadableSpan[] = []; + + export( + spans: ReadableSpan[], + resultCallback: (result: ExportResult) => void + ): void { + this.captured.push(...spans); + resultCallback({ code: ExportResultCode.SUCCESS }); + } + + shutdown(): Promise { + return Promise.resolve(); + } + + forceFlush(): Promise { + return Promise.resolve(); + } +} + +/** + * Produce a real ended span carrying the given attributes, so the exporter is + * exercised against the SDK's own span implementation rather than a stub object. + */ +function endedSpanWith( + attributes: Record +): ReadableSpan { + const captured = new CapturingExporter(); + const provider = new BasicTracerProvider({ + spanProcessors: [new SimpleSpanProcessor(captured)] + }); + + const span = provider + .getTracer('test') + .startSpan('GET', { kind: SpanKind.CLIENT, attributes }); + span.end(); + + return captured.captured[0]; +} + +describe('RedactingSpanExporter', () => { + it('redacts access tokens from url.full and url.query', () => { + const delegate = new CapturingExporter(); + const exporter = new RedactingSpanExporter(delegate); + + exporter.export( + [ + endedSpanWith({ + 'url.full': `https://api.mapbox.com/tokens/v2/testuser?access_token=${SECRET}`, + 'url.query': `access_token=${SECRET}` + }) + ], + () => {} + ); + + const attributes = delegate.captured[0].attributes; + expect(attributes['url.full']).toBe( + 'https://api.mapbox.com/tokens/v2/testuser?access_token=***' + ); + expect(attributes['url.query']).toBe('access_token=***'); + expect( + Object.values(attributes).filter((value) => + String(value).includes(SECRET) + ) + ).toEqual([]); + }); + + it('redacts tokens regardless of which attribute carries them', () => { + const delegate = new CapturingExporter(); + const exporter = new RedactingSpanExporter(delegate); + + exporter.export( + [ + endedSpanWith({ + 'some.future.url.attribute': `https://api.mapbox.com/styles/v1/u?access_token=${SECRET}&limit=5` + }) + ], + () => {} + ); + + expect(delegate.captured[0].attributes['some.future.url.attribute']).toBe( + 'https://api.mapbox.com/styles/v1/u?access_token=***&limit=5' + ); + }); + + it('preserves span identity and non-sensitive attributes', () => { + const delegate = new CapturingExporter(); + const exporter = new RedactingSpanExporter(delegate); + const original = endedSpanWith({ + 'url.full': `https://api.mapbox.com/tokens/v2/testuser?access_token=${SECRET}`, + 'server.address': 'api.mapbox.com', + 'http.response.status_code': 200 + }); + + exporter.export([original], () => {}); + + const exported = delegate.captured[0]; + expect(exported.name).toBe(original.name); + expect(exported.kind).toBe(original.kind); + expect(exported.spanContext()).toEqual(original.spanContext()); + expect(exported.startTime).toEqual(original.startTime); + expect(exported.endTime).toEqual(original.endTime); + expect(exported.duration).toEqual(original.duration); + expect(exported.ended).toBe(true); + expect(exported.resource).toBe(original.resource); + expect(exported.status).toEqual(original.status); + expect(exported.attributes['server.address']).toBe('api.mapbox.com'); + expect(exported.attributes['http.response.status_code']).toBe(200); + }); + + it('passes spans through untouched when nothing needs redaction', () => { + const delegate = new CapturingExporter(); + const exporter = new RedactingSpanExporter(delegate); + const original = endedSpanWith({ 'server.address': 'api.mapbox.com' }); + + exporter.export([original], () => {}); + + expect(delegate.captured[0]).toBe(original); + }); + + it('reports the delegate export result to the caller', () => { + const delegate = new CapturingExporter(); + const exporter = new RedactingSpanExporter(delegate); + const resultCallback = vi.fn(); + + exporter.export([endedSpanWith({})], resultCallback); + + expect(resultCallback).toHaveBeenCalledWith({ + code: ExportResultCode.SUCCESS + }); + }); + + it('delegates shutdown and forceFlush', async () => { + const delegate = new CapturingExporter(); + const shutdown = vi.spyOn(delegate, 'shutdown'); + const forceFlush = vi.spyOn(delegate, 'forceFlush'); + const exporter = new RedactingSpanExporter(delegate); + + await exporter.forceFlush(); + await exporter.shutdown(); + + expect(forceFlush).toHaveBeenCalledOnce(); + expect(shutdown).toHaveBeenCalledOnce(); + }); +}); From e167fe14b7be64db0bbff817704fe895013be972 Mon Sep 17 00:00:00 2001 From: Valentin Salmanovicius Date: Wed, 29 Jul 2026 09:16:02 +0100 Subject: [PATCH 2/4] fix: keep token prefix and account name in redacted spans MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 (`..`) whose payload carries the account name under `u`, so redaction now decodes the payload and emits `..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) --- CHANGELOG.md | 2 +- docs/tracing.md | 12 +++-- src/tools/MapboxApiBasedTool.ts | 53 +++++++++++++++++++- src/utils/redactingSpanExporter.ts | 5 +- test/tools/MapboxApiBasedTool.test.ts | 63 +++++++++++++++++++++++- test/utils/redactingSpanExporter.test.ts | 7 +-- 6 files changed, 130 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dcf296f..861ea42 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ### Fixed -- **Tracing**: Access tokens are no longer included in exported spans. Mapbox APIs take the access token as a URL query parameter, and OpenTelemetry's HTTP/undici auto-instrumentation records the full request URL on client spans (`url.full`, `url.query`), so operators who configured `OTEL_EXPORTER_OTLP_ENDPOINT` had tokens copied verbatim into their telemetry backend. The OTLP exporter is now wrapped in a `RedactingSpanExporter` that rewrites `access_token=` to `access_token=***` across all string span attributes before export. +- **Tracing**: Access tokens are no longer included in exported spans. Mapbox APIs take the access token as a URL query parameter, and OpenTelemetry's HTTP/undici auto-instrumentation records the full request URL on client spans (`url.full`, `url.query`), so operators who configured `OTEL_EXPORTER_OTLP_ENDPOINT` had tokens copied verbatim into their telemetry backend. The OTLP exporter is now wrapped in a `RedactingSpanExporter` that strips the token signature from all string span attributes before export. Redaction keeps the token prefix and account name — `pk.eyJ1...xyz.signature` becomes `pk.your-account.redacted` — so spans still distinguish public from secret tokens and show which account a request billed to, without carrying a usable credential. Values that do not parse as a Mapbox token fall back to `access_token=***`. - **`validate_expression_tool` / `validate_style_tool`**: Both tools now delegate expression and style validation to the official `@mapbox/mapbox-gl-style-spec` package (the same logic mapbox-gl-js runs at runtime) instead of a hand-rolled reimplementation. Fixes two disagreements with real mapbox-gl-js behavior around the `["zoom"]` expression placement rule (#123): - `validate_expression_tool` no longer false-positives on ordinary zoom-based `interpolate`/`step` expressions (it previously misidentified the interpolation-type argument, e.g. `["linear"]`, as an unknown operator). - `validate_style_tool` now correctly flags `["zoom"]` expressions nested inside e.g. a `case` (invalid per spec — zoom may only be used as the top-level input to `step`/`interpolate`), which it previously missed entirely. diff --git a/docs/tracing.md b/docs/tracing.md index 8511940..69c37b0 100644 --- a/docs/tracing.md +++ b/docs/tracing.md @@ -37,7 +37,7 @@ Console output is incompatible with stdio and will corrupt JSON-RPC communicatio ### Security & Performance -- **Sensitive Data Protection**: Input parameters logged by size only, not content; access tokens are stripped from span attributes before export +- **Sensitive Data Protection**: Input parameters logged by size only, not content; access token signatures are stripped from span attributes before export - **Minimal Overhead**: <1% CPU impact, ~10MB memory for trace buffers - **Configurable Sampling**: Support for production trace volume management - **Graceful Fallback**: No impact on functionality when tracing is disabled @@ -371,9 +371,13 @@ getNodeAutoInstrumentations({ - **Input sanitization**: Only input/output sizes are logged, not content - **Access tokens**: Mapbox APIs take the access token as a URL query parameter, and HTTP auto-instrumentation records request URLs on client spans (`url.full`, `url.query`). Every - span passes through a redacting exporter that rewrites `access_token=` to - `access_token=***` in all string attributes before anything is sent to your OTLP endpoint, - so tokens are not copied into your telemetry backend + span passes through a redacting exporter that strips the token signature from all string + attributes before anything is sent to your OTLP endpoint, so usable credentials are not + copied into your telemetry backend. The token prefix and account name are kept, so a span + shows `access_token=pk.your-account.redacted` — enough to tell a public token from a secret + one and see which account a request billed to, without the part that authenticates it. + Tokens that do not parse as `..` with a `pk`/`sk`/`tk` prefix + and an account name in the payload are replaced wholesale with `access_token=***` - **JWT validation**: Basic format validation only, no secret verification - **Error messages**: Error details are logged but sensitive data is protected diff --git a/src/tools/MapboxApiBasedTool.ts b/src/tools/MapboxApiBasedTool.ts index eb52d3e..d90ad9e 100644 --- a/src/tools/MapboxApiBasedTool.ts +++ b/src/tools/MapboxApiBasedTool.ts @@ -13,9 +13,60 @@ import { context, trace, SpanStatusCode } from '@opentelemetry/api'; import type { ToolExecutionContext } from '../utils/tracing.js'; import { createToolExecutionContext } from '../utils/tracing.js'; +/** Token prefixes Mapbox uses: public, secret, and temporary. */ +const TOKEN_PREFIXES = new Set(['pk', 'sk', 'tk']); + +/** Mapbox usernames are lowercase alphanumerics with dashes and underscores. */ +const ACCOUNT_NAME_PATTERN = /^[A-Za-z0-9_-]{1,64}$/; + +/** + * Replace a token value with a placeholder that keeps the parts safe to publish. + * + * Mapbox tokens are JWTs (`..`) whose payload carries + * the account name under `u`, so `pk.eyJ1IjoiZXhhbXBsZSJ9.signature` becomes + * `pk.example.redacted`. Keeping the prefix and account name makes traces and logs + * readable — you can still tell a secret token from a public one, and whose account + * a request billed to — while the signature, which is the part that authenticates, + * never leaves the process. + * + * Anything that does not parse cleanly as such a token falls back to `***`, so an + * unrecognized shape is never partially disclosed on the assumption it was harmless. + */ +function maskTokenValue(token: string): string { + const parts = token.split('.'); + if (parts.length !== 3) { + return '***'; + } + + const [prefix, payload] = parts; + if (!TOKEN_PREFIXES.has(prefix)) { + return '***'; + } + + try { + const decoded = JSON.parse( + Buffer.from(payload, 'base64').toString('utf-8') + ) as { u?: unknown }; + + if ( + typeof decoded.u !== 'string' || + !ACCOUNT_NAME_PATTERN.test(decoded.u) + ) { + return '***'; + } + + return `${prefix}.${decoded.u}.redacted`; + } catch { + return '***'; + } +} + /** Remove access_token query parameter values from strings before logging or returning to callers. */ export function redactToken(s: string): string { - return s.replace(/access_token=[^&\s#"']+/g, 'access_token=***'); + return s.replace( + /access_token=([^&\s#"']+)/g, + (_match, token: string) => `access_token=${maskTokenValue(token)}` + ); } /** diff --git a/src/utils/redactingSpanExporter.ts b/src/utils/redactingSpanExporter.ts index b12d2b3..f5e4cf7 100644 --- a/src/utils/redactingSpanExporter.ts +++ b/src/utils/redactingSpanExporter.ts @@ -86,8 +86,9 @@ function redactSpan(span: ReadableSpan): ReadableSpan { } /** - * Wraps a SpanExporter and strips `access_token` values from span attributes - * before they leave the process. + * Wraps a SpanExporter and strips access token signatures from span attributes + * before they leave the process, leaving behind the prefix and account name + * (`access_token=pk.some-account.redacted`). * * Auto-instrumentation of `fetch`/`undici` records the full request URL on client * spans (`url.full`, `url.query`), and Mapbox APIs take the access token as a diff --git a/test/tools/MapboxApiBasedTool.test.ts b/test/tools/MapboxApiBasedTool.test.ts index c877770..376161a 100644 --- a/test/tools/MapboxApiBasedTool.test.ts +++ b/test/tools/MapboxApiBasedTool.test.ts @@ -6,7 +6,10 @@ process.env.MAPBOX_ACCESS_TOKEN = `eyJhbGciOiJIUzI1NiJ9.${payload}.signature`; import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { z } from 'zod'; -import { MapboxApiBasedTool } from '../../src/tools/MapboxApiBasedTool.js'; +import { + MapboxApiBasedTool, + redactToken +} from '../../src/tools/MapboxApiBasedTool.js'; import { CallToolResult } from '@modelcontextprotocol/sdk/types.js'; import type { HttpRequest } from '../../src/utils/types.js'; import { setupHttpRequest } from '../utils/httpPipelineUtils.js'; @@ -42,6 +45,64 @@ class TestTool extends MapboxApiBasedTool { } } +describe('redactToken', () => { + const PUBLIC_TOKEN = + 'pk.eyJ1IjoidmFsaXVuaWEiLCJhIjoiY21yb3FqdWtiMDJobjJ5c2c3NGVxeXphZCJ9.signaturevalue'; + const SECRET_TOKEN = 'sk.eyJ1IjoidGVzdHVzZXIifQ.signaturevalue'; + const TEMP_TOKEN = 'tk.eyJ1IjoidGVtcC11c2VyXzEifQ.signaturevalue'; + + it('keeps the prefix and account name, dropping the signature', () => { + expect(redactToken(`access_token=${PUBLIC_TOKEN}`)).toBe( + 'access_token=pk.valiunia.redacted' + ); + expect(redactToken(`access_token=${SECRET_TOKEN}`)).toBe( + 'access_token=sk.testuser.redacted' + ); + expect(redactToken(`access_token=${TEMP_TOKEN}`)).toBe( + 'access_token=tk.temp-user_1.redacted' + ); + }); + + it('never emits the token signature', () => { + expect( + redactToken( + `https://api.mapbox.com/tokens/v2/valiunia?access_token=${PUBLIC_TOKEN}&limit=5` + ) + ).toBe( + 'https://api.mapbox.com/tokens/v2/valiunia?access_token=pk.valiunia.redacted&limit=5' + ); + }); + + it('redacts every occurrence in a string', () => { + expect( + redactToken( + `first access_token=${PUBLIC_TOKEN} second access_token=${SECRET_TOKEN}` + ) + ).toBe( + 'first access_token=pk.valiunia.redacted second access_token=sk.testuser.redacted' + ); + }); + + it.each([ + ['an unrecognized prefix', 'zz.eyJ1IjoidGVzdHVzZXIifQ.signaturevalue'], + ['too few segments', 'pk.eyJ1IjoidGVzdHVzZXIifQ'], + ['a payload that is not base64 JSON', 'pk.@@@notbase64@@@.signaturevalue'], + [ + 'a payload with no account name', + 'pk.eyJhIjoibm9hY2NvdW50In0.signaturevalue' + ], + ['an opaque value', 'some-legacy-opaque-token'] + ])('falls back to *** for %s', (_case, token) => { + expect(redactToken(`access_token=${token}`)).toBe('access_token=***'); + }); + + it('leaves strings without a token untouched', () => { + expect(redactToken('https://api.mapbox.com/tokens/v2/valiunia')).toBe( + 'https://api.mapbox.com/tokens/v2/valiunia' + ); + }); +}); + describe('MapboxApiBasedTool', () => { let testTool: TestTool; const originalEnv = process.env; diff --git a/test/utils/redactingSpanExporter.test.ts b/test/utils/redactingSpanExporter.test.ts index 262ed2e..6b0c81e 100644 --- a/test/utils/redactingSpanExporter.test.ts +++ b/test/utils/redactingSpanExporter.test.ts @@ -13,6 +13,7 @@ import { SpanKind } from '@opentelemetry/api'; import { RedactingSpanExporter } from '../../src/utils/redactingSpanExporter.js'; const SECRET = 'sk.eyJ1IjoidGVzdHVzZXIifQ.signaturevalue'; +const MASKED = 'sk.testuser.redacted'; class CapturingExporter implements SpanExporter { readonly captured: ReadableSpan[] = []; @@ -71,9 +72,9 @@ describe('RedactingSpanExporter', () => { const attributes = delegate.captured[0].attributes; expect(attributes['url.full']).toBe( - 'https://api.mapbox.com/tokens/v2/testuser?access_token=***' + `https://api.mapbox.com/tokens/v2/testuser?access_token=${MASKED}` ); - expect(attributes['url.query']).toBe('access_token=***'); + expect(attributes['url.query']).toBe(`access_token=${MASKED}`); expect( Object.values(attributes).filter((value) => String(value).includes(SECRET) @@ -95,7 +96,7 @@ describe('RedactingSpanExporter', () => { ); expect(delegate.captured[0].attributes['some.future.url.attribute']).toBe( - 'https://api.mapbox.com/styles/v1/u?access_token=***&limit=5' + `https://api.mapbox.com/styles/v1/u?access_token=${MASKED}&limit=5` ); }); From d472a7c80d653047ddb762ade1a337a5c9aeebf2 Mon Sep 17 00:00:00 2001 From: Valentin Salmanovicius Date: Wed, 29 Jul 2026 09:26:23 +0100 Subject: [PATCH 3/4] test: use a neutral account name in redaction fixtures 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) --- test/tools/MapboxApiBasedTool.test.ts | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/test/tools/MapboxApiBasedTool.test.ts b/test/tools/MapboxApiBasedTool.test.ts index 376161a..606cb33 100644 --- a/test/tools/MapboxApiBasedTool.test.ts +++ b/test/tools/MapboxApiBasedTool.test.ts @@ -46,14 +46,13 @@ class TestTool extends MapboxApiBasedTool { } describe('redactToken', () => { - const PUBLIC_TOKEN = - 'pk.eyJ1IjoidmFsaXVuaWEiLCJhIjoiY21yb3FqdWtiMDJobjJ5c2c3NGVxeXphZCJ9.signaturevalue'; + const PUBLIC_TOKEN = 'pk.eyJ1IjoiZXhhbXBsZS1hY2NvdW50In0.signaturevalue'; const SECRET_TOKEN = 'sk.eyJ1IjoidGVzdHVzZXIifQ.signaturevalue'; const TEMP_TOKEN = 'tk.eyJ1IjoidGVtcC11c2VyXzEifQ.signaturevalue'; it('keeps the prefix and account name, dropping the signature', () => { expect(redactToken(`access_token=${PUBLIC_TOKEN}`)).toBe( - 'access_token=pk.valiunia.redacted' + 'access_token=pk.example-account.redacted' ); expect(redactToken(`access_token=${SECRET_TOKEN}`)).toBe( 'access_token=sk.testuser.redacted' @@ -66,10 +65,10 @@ describe('redactToken', () => { it('never emits the token signature', () => { expect( redactToken( - `https://api.mapbox.com/tokens/v2/valiunia?access_token=${PUBLIC_TOKEN}&limit=5` + `https://api.mapbox.com/tokens/v2/example-account?access_token=${PUBLIC_TOKEN}&limit=5` ) ).toBe( - 'https://api.mapbox.com/tokens/v2/valiunia?access_token=pk.valiunia.redacted&limit=5' + 'https://api.mapbox.com/tokens/v2/example-account?access_token=pk.example-account.redacted&limit=5' ); }); @@ -79,7 +78,7 @@ describe('redactToken', () => { `first access_token=${PUBLIC_TOKEN} second access_token=${SECRET_TOKEN}` ) ).toBe( - 'first access_token=pk.valiunia.redacted second access_token=sk.testuser.redacted' + 'first access_token=pk.example-account.redacted second access_token=sk.testuser.redacted' ); }); @@ -97,9 +96,9 @@ describe('redactToken', () => { }); it('leaves strings without a token untouched', () => { - expect(redactToken('https://api.mapbox.com/tokens/v2/valiunia')).toBe( - 'https://api.mapbox.com/tokens/v2/valiunia' - ); + expect( + redactToken('https://api.mapbox.com/tokens/v2/example-account') + ).toBe('https://api.mapbox.com/tokens/v2/example-account'); }); }); From eccf5a5ec27fcb4c076f28e09fd611c23fcfa744 Mon Sep 17 00:00:00 2001 From: Valentin Salmanovicius Date: Wed, 29 Jul 2026 09:47:40 +0100 Subject: [PATCH 4/4] docs: correct an unverified claim in the redaction comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- src/tools/MapboxApiBasedTool.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/tools/MapboxApiBasedTool.ts b/src/tools/MapboxApiBasedTool.ts index d90ad9e..2e8bde3 100644 --- a/src/tools/MapboxApiBasedTool.ts +++ b/src/tools/MapboxApiBasedTool.ts @@ -16,7 +16,12 @@ import { createToolExecutionContext } from '../utils/tracing.js'; /** Token prefixes Mapbox uses: public, secret, and temporary. */ const TOKEN_PREFIXES = new Set(['pk', 'sk', 'tk']); -/** Mapbox usernames are lowercase alphanumerics with dashes and underscores. */ +/** + * Character allowlist for an account name lifted out of a token payload, matching the + * username validation in StyleComparisonTool. This bounds what a token payload can put + * into a span attribute or log line; it is not a statement of Mapbox's account naming + * rules. A name outside it is not partially disclosed — it falls back to `***`. + */ const ACCOUNT_NAME_PATTERN = /^[A-Za-z0-9_-]{1,64}$/; /**