diff --git a/CHANGELOG.md b/CHANGELOG.md index 9334a82..861ea42 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 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 bdc8280..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 +- **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 @@ -369,6 +369,15 @@ 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 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..2e8bde3 100644 --- a/src/tools/MapboxApiBasedTool.ts +++ b/src/tools/MapboxApiBasedTool.ts @@ -13,9 +13,65 @@ 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']); + +/** + * 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}$/; + +/** + * 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 new file mode 100644 index 0000000..f5e4cf7 --- /dev/null +++ b/src/utils/redactingSpanExporter.ts @@ -0,0 +1,119 @@ +// 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 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 + * 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/tools/MapboxApiBasedTool.test.ts b/test/tools/MapboxApiBasedTool.test.ts index c877770..606cb33 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,63 @@ class TestTool extends MapboxApiBasedTool { } } +describe('redactToken', () => { + 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.example-account.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/example-account?access_token=${PUBLIC_TOKEN}&limit=5` + ) + ).toBe( + 'https://api.mapbox.com/tokens/v2/example-account?access_token=pk.example-account.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.example-account.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/example-account') + ).toBe('https://api.mapbox.com/tokens/v2/example-account'); + }); +}); + describe('MapboxApiBasedTool', () => { let testTool: TestTool; const originalEnv = process.env; diff --git a/test/utils/redactingSpanExporter.test.ts b/test/utils/redactingSpanExporter.test.ts new file mode 100644 index 0000000..6b0c81e --- /dev/null +++ b/test/utils/redactingSpanExporter.test.ts @@ -0,0 +1,162 @@ +// 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'; +const MASKED = 'sk.testuser.redacted'; + +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=${MASKED}` + ); + expect(attributes['url.query']).toBe(`access_token=${MASKED}`); + 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=${MASKED}&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(); + }); +});