diff --git a/CHANGELOG.md b/CHANGELOG.md index 97ba6a70..6ea83e61 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,8 @@ ### 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 trace 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=***`. + - **urlSafety**: `isSafeExternalUrl()` (used to validate `static_map_image_tool` custom-marker overlay URLs) now uses `ipaddr.js` to parse IPv6 literals and correctly identify IPv4 addresses embedded via any standard encoding (IPv4-mapped, IPv4-compatible, 6to4, NAT64, IPv4-translated/SIIT), instead of pattern-matching a subset of string forms. The previous regex-based check missed the bare IPv4-compatible form and the 6to4/NAT64/SIIT forms, letting blocked IPv4 ranges (loopback, private, link-local, etc.) through when expressed as one of those encodings. The embedded address is now detected from the parsed address's raw bytes rather than by enumerating each encoding's string shape. - **map_matching_tool**: When the Map Matching API can't match a trace (e.g. `code: "NoMatch"` for distant/unmatchable coordinates), the tool now returns a clear `isError` text result instead of crashing with `MCP error -32602: Output validation error` — the API omits `tracepoints`/`matchings` in this case, which previously violated the tool's output schema and was returned as `structuredContent` anyway, triggering the MCP SDK's output validation. The same schema-violating `structuredContent` could also be returned for a `code: "Ok"` response that otherwise failed schema validation (e.g. a `confidence` out of range); that fallback now also returns a graceful `isError` result instead of the raw invalid payload. `tracepoints` and `matchings` are also now `.optional()` in the output schema as a defensive measure. (AGI-1021) - **render_map_tool: map previews no longer break after a server restart or when reopening an old conversation.** Rendering previously depended entirely on in-memory server state (a 30-minute-TTL `Map`) that doesn't survive a process restart — including `render_map_tool`'s own merged-output ref, the one every MCP App host actually re-fetches when a map card is redisplayed. That ref is now self-describing (the whole small payload is encoded directly into the ref) instead of pointing at that ephemeral store, whenever the payload is small enough to inline. diff --git a/docs/tracing.md b/docs/tracing.md index 54892e14..95dd1db8 100644 --- a/docs/tracing.md +++ b/docs/tracing.md @@ -41,7 +41,7 @@ This MCP server uses **stdio transport exclusively** for communication. Console ### 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 @@ -137,13 +137,11 @@ The server supports **any OTLP-compatible observability backend**. Configuration ### Production Cloud Providers - **AWS X-Ray**: AWS-native distributed tracing - - Endpoint: AWS Distro for OpenTelemetry Collector - Auth: IAM credentials - [Setup Guide](https://aws-otel.github.io/docs/getting-started/collector) - **Azure Monitor**: Azure Application Insights - - Endpoint: `https://.livediagnostics.monitor.azure.com` - Auth: Connection string or AAD token - [Setup Guide](https://learn.microsoft.com/en-us/azure/azure-monitor/app/opentelemetry-enable) @@ -156,13 +154,11 @@ The server supports **any OTLP-compatible observability backend**. Configuration ### Production SaaS Observability Platforms - **Datadog**: Full-stack observability platform - - Endpoint: `https://api.datadoghq.com/api/v2/traces` or local agent - Auth: API key - [Setup Guide](https://docs.datadoghq.com/tracing/trace_collection/opentelemetry/) - **New Relic**: Application performance monitoring - - Endpoint: `https://otlp.nr-data.net:4318` (US) or `https://otlp.eu01.nr-data.net:4318` (EU) - Auth: License key - [Setup Guide](https://docs.newrelic.com/docs/more-integrations/open-source-telemetry-integrations/opentelemetry/opentelemetry-setup/) @@ -352,6 +348,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 trace backend, 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/search-and-geocode-tool/SearchAndGeocodeTool.ts b/src/tools/search-and-geocode-tool/SearchAndGeocodeTool.ts index aef7d69c..1b9a2d82 100644 --- a/src/tools/search-and-geocode-tool/SearchAndGeocodeTool.ts +++ b/src/tools/search-and-geocode-tool/SearchAndGeocodeTool.ts @@ -4,6 +4,7 @@ import type { z } from 'zod'; import { MapboxApiBasedTool } from '../MapboxApiBasedTool.js'; import type { HttpRequest } from '../../utils/types.js'; +import { redactToken } from '../../utils/redactToken.js'; import { SearchAndGeocodeInputSchema } from './SearchAndGeocodeTool.input.schema.js'; import { SearchBoxResponseSchema, @@ -113,6 +114,11 @@ export class SearchAndGeocodeTool extends MapboxApiBasedTool< apiEndpoint: MapboxApiBasedTool.mapboxApiEndpoint }); + this.log( + 'info', + `SearchAndGeocodeTool: Fetching from URL: ${redactToken(url)}` + ); + const response = await this.httpRequest(url); if (!response.ok) { diff --git a/src/utils/index.ts b/src/utils/index.ts index 480c19c5..bf92913a 100644 --- a/src/utils/index.ts +++ b/src/utils/index.ts @@ -46,6 +46,12 @@ export { // Export types export type { HttpRequest, TracedRequestInit } from './types.js'; +// Export tracing helpers for consumers running their own OpenTelemetry setup. +// Wrap your span exporter in RedactingSpanExporter to keep Mapbox access tokens, +// which travel as a URL query parameter, out of exported span attributes. +export { RedactingSpanExporter } from './redactingSpanExporter.js'; +export { redactToken } from './redactToken.js'; + // Export version utilities export { getVersionInfo } from './versionUtils.js'; export type { VersionInfo } from './versionUtils.js'; diff --git a/src/utils/redactToken.ts b/src/utils/redactToken.ts new file mode 100644 index 00000000..7b2887ca --- /dev/null +++ b/src/utils/redactToken.ts @@ -0,0 +1,65 @@ +// Copyright (c) Mapbox, Inc. +// Licensed under the MIT License. + +/** 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. This + * bounds what a token payload can put into a span attribute or log line — the + * value comes from decoding untrusted JWT payload data, so the allowlist is a + * safety gate against log/span injection and oversized values, not a + * statement of Mapbox's actual 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, + (_match, token: string) => `access_token=${maskTokenValue(token)}` + ); +} diff --git a/src/utils/redactingSpanExporter.ts b/src/utils/redactingSpanExporter.ts new file mode 100644 index 00000000..c41f8717 --- /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 './redactToken.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 d9fdeb80..bad6dcda 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'; @@ -207,7 +208,9 @@ export async function initializeTracing(): Promise { // Create SDK instance sdk = new NodeSDK({ resource, - traceExporter: exporters[0], + // Wrap the exporter so access tokens recorded in HTTP client span + // attributes (url.full, url.query) never reach the trace backend + traceExporter: new RedactingSpanExporter(exporters[0]), instrumentations: [ getNodeAutoInstrumentations({ // Disable instrumentations that might be too noisy diff --git a/test/utils/redactToken.test.ts b/test/utils/redactToken.test.ts new file mode 100644 index 00000000..56e4b0e8 --- /dev/null +++ b/test/utils/redactToken.test.ts @@ -0,0 +1,62 @@ +// Copyright (c) Mapbox, Inc. +// Licensed under the MIT License. + +import { describe, it, expect } from 'vitest'; +import { redactToken } from '../../src/utils/redactToken.js'; + +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/directions/v5/mapbox/driving/0,0;1,1?access_token=${PUBLIC_TOKEN}&geometries=geojson` + ) + ).toBe( + 'https://api.mapbox.com/directions/v5/mapbox/driving/0,0;1,1?access_token=pk.example-account.redacted&geometries=geojson' + ); + }); + + 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/isochrone/v1/mapbox/driving') + ).toBe('https://api.mapbox.com/isochrone/v1/mapbox/driving'); + }); +}); diff --git a/test/utils/redactingSpanExporter.test.ts b/test/utils/redactingSpanExporter.test.ts new file mode 100644 index 00000000..6b0c81ed --- /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(); + }); +});