From 114d8664a22b48c666d9f88fec3e82ea9ed03ece Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Tue, 28 Jul 2026 12:06:02 -0700 Subject: [PATCH] Scrub credentials from span attributes Strip OAuth authorization codes and CSRF state from exported span URL attributes, and sanitize user-supplied MCP endpoints before stamping them. --- .changeset/olive-moons-shake.md | 8 + .../oauth-callback-telemetry.test.ts | 114 +++++++++++ .../observability/redact-span-urls.test.ts | 126 ++++++++++++ .../src/observability/redact-span-urls.ts | 189 ++++++++++++++++++ apps/cloud/src/observability/telemetry.ts | 8 +- packages/core/sdk/src/index.ts | 4 + .../core/sdk/src/telemetry-endpoint.test.ts | 68 +++++++ packages/core/sdk/src/telemetry-endpoint.ts | 54 +++++ packages/plugins/graphql/src/sdk/invoke.ts | 10 +- .../plugins/graphql/src/sdk/plugin.test.ts | 2 +- packages/plugins/mcp/src/sdk/plugin.test.ts | 84 +++++++- packages/plugins/mcp/src/sdk/plugin.ts | 12 +- 12 files changed, 666 insertions(+), 13 deletions(-) create mode 100644 .changeset/olive-moons-shake.md create mode 100644 apps/cloud/src/observability/oauth-callback-telemetry.test.ts create mode 100644 apps/cloud/src/observability/redact-span-urls.test.ts create mode 100644 apps/cloud/src/observability/redact-span-urls.ts create mode 100644 packages/core/sdk/src/telemetry-endpoint.test.ts create mode 100644 packages/core/sdk/src/telemetry-endpoint.ts diff --git a/.changeset/olive-moons-shake.md b/.changeset/olive-moons-shake.md new file mode 100644 index 000000000..5f4ec7584 --- /dev/null +++ b/.changeset/olive-moons-shake.md @@ -0,0 +1,8 @@ +--- +"executor": patch +--- + +Stop exporting credential-bearing URLs in span attributes. OAuth callback +authorization codes and CSRF state are stripped from span URL attributes before +export, and user-supplied MCP endpoints are sanitized of query-string and +userinfo credentials before being stamped. diff --git a/apps/cloud/src/observability/oauth-callback-telemetry.test.ts b/apps/cloud/src/observability/oauth-callback-telemetry.test.ts new file mode 100644 index 000000000..d96e905ef --- /dev/null +++ b/apps/cloud/src/observability/oauth-callback-telemetry.test.ts @@ -0,0 +1,114 @@ +// --------------------------------------------------------------------------- +// OAuth callback × telemetry — the authorization code must never be exported. +// +// `/api/oauth/callback` is an app-owned path, so Effect's +// `HttpMiddleware.tracer` opens its `http.server` span and stamps `url.full` +// and `url.query` unconditionally (`effect/unstable/http/HttpMiddleware.ts`). +// It redacts URL userinfo and configured header names — nothing else — so the +// provider's `?code=…&state=…` reached the trace backend on every connect. +// +// This drives a real callback request through the real core API web handler, +// with the real Effect→OTel tracer bridge exporting into an in-memory +// exporter behind the production span-processor chain, and asserts that no +// exported span attribute contains the code or the state. +// --------------------------------------------------------------------------- + +import { describe, expect, it } from "@effect/vitest"; +import * as Resource from "@effect/opentelemetry/Resource"; +import * as OtelTracer from "@effect/opentelemetry/Tracer"; +import { + InMemorySpanExporter, + SimpleSpanProcessor, + type ReadableSpan, +} from "@opentelemetry/sdk-trace-base"; +import { BasicTracerProvider } from "@opentelemetry/sdk-trace-base"; +import { Context, Effect, Layer } from "effect"; +import { HttpRouter, HttpServer } from "effect/unstable/http"; +import { HttpApiBuilder } from "effect/unstable/httpapi"; + +import { ExecutorApi, observabilityMiddleware } from "@executor-js/api"; +import { CoreHandlers, ExecutionEngineService, ExecutorService } from "@executor-js/api/server"; +import { createExecutor } from "@executor-js/sdk"; +import { makeTestConfig } from "@executor-js/sdk/testing"; + +import { UrlRedactingSpanProcessor } from "./redact-span-urls"; + +// Synthetic placeholders — never a real authorization code or CSRF state. +const CODE = "synthetic-authorization-code-9f2c"; +const STATE = "synthetic-csrf-state-4b7e"; + +const makeTracing = () => { + const exporter = new InMemorySpanExporter(); + const provider = new BasicTracerProvider({ + // The same wrapper order production installs in `telemetry.ts`: the + // redactor sits outermost, so nothing downstream ever sees the secret. + spanProcessors: [new UrlRedactingSpanProcessor(new SimpleSpanProcessor(exporter))], + }); + const tracerLayer = OtelTracer.layer.pipe( + Layer.provide(Layer.succeed(OtelTracer.OtelTracerProvider)(provider)), + Layer.provide(Resource.layer({ serviceName: "executor-cloud-test" })), + ); + return { exporter, provider, tracerLayer }; +}; + +describe("oauth callback telemetry", () => { + it.effect("exports no span attribute containing the authorization code or state", () => + Effect.gen(function* () { + const { exporter, provider, tracerLayer } = makeTracing(); + const executor = yield* createExecutor(makeTestConfig({})); + + const web = yield* Effect.acquireRelease( + Effect.sync(() => + HttpRouter.toWebHandler( + HttpApiBuilder.layer(ExecutorApi).pipe( + Layer.provide(CoreHandlers), + Layer.provide(observabilityMiddleware(ExecutorApi)), + Layer.provide(Layer.succeed(ExecutorService)(executor)), + Layer.provide( + Layer.succeed(ExecutionEngineService)({} as ExecutionEngineService["Service"]), + ), + Layer.provideMerge(HttpServer.layerServices), + Layer.provideMerge(Layer.succeed(HttpRouter.RouterConfig)({ maxParamLength: 1000 })), + // The Effect→OTel bridge: HttpMiddleware.tracer's span is created + // by this tracer, so it lands in the exporter below. + Layer.provideMerge(tracerLayer), + ), + { disableLogger: true }, + ), + ), + (handle) => Effect.promise(() => handle.dispose()), + ); + + const context = Context.make(ExecutorService, executor).pipe( + Context.add(ExecutionEngineService, {} as ExecutionEngineService["Service"]), + ); + + // The provider round-trip: a real callback carries the grant and the + // CSRF state in the query string. + yield* Effect.promise(() => + web.handler( + new Request( + `http://app.test/oauth/callback?code=${CODE}&state=${STATE}&domain=example.test`, + ), + context, + ), + ); + + yield* Effect.promise(() => provider.forceFlush()); + const spans: readonly ReadableSpan[] = exporter.getFinishedSpans(); + + // The middleware must actually have opened a server span — otherwise + // this test would pass vacuously. + const serverSpan = spans.find((span) => span.name.startsWith("http.server")); + expect(serverSpan).toBeDefined(); + + const serialized = JSON.stringify(spans.map((span) => span.attributes)); + expect(serialized).not.toContain(CODE); + expect(serialized).not.toContain(STATE); + + // Route-level visibility survives the scrub. + expect(serverSpan?.attributes["url.path"]).toBe("/oauth/callback"); + expect(String(serverSpan?.attributes["url.full"] ?? "")).toContain("/oauth/callback"); + }), + ); +}); diff --git a/apps/cloud/src/observability/redact-span-urls.test.ts b/apps/cloud/src/observability/redact-span-urls.test.ts new file mode 100644 index 000000000..d48a07e92 --- /dev/null +++ b/apps/cloud/src/observability/redact-span-urls.test.ts @@ -0,0 +1,126 @@ +import { describe, expect, it } from "@effect/vitest"; +import { + BasicTracerProvider, + InMemorySpanExporter, + SimpleSpanProcessor, + type ReadableSpan, +} from "@opentelemetry/sdk-trace-base"; + +import { + redactSpanUrlAttributes, + STRIPPED_QUERY_ATTRIBUTE, + UrlRedactingSpanProcessor, +} from "./redact-span-urls"; + +// Synthetic placeholders only — never a real authorization code or state. +const CODE = "synthetic-authorization-code"; +const STATE = "synthetic-csrf-state"; + +const callbackUrl = `https://app.test/api/oauth/callback?code=${CODE}&state=${STATE}&domain=example.test`; + +/** Ends one real SDK span carrying `attributes` through the redacting + * processor, and returns what the exporter actually received. Using the real + * provider (rather than a hand-built span) exercises the `onEnding` → `onEnd` + * hook sequence exactly as production does. */ +const exportSpanWith = (attributes: Record): ReadableSpan | undefined => { + const exporter = new InMemorySpanExporter(); + const provider = new BasicTracerProvider({ + spanProcessors: [new UrlRedactingSpanProcessor(new SimpleSpanProcessor(exporter))], + }); + const span = provider.getTracer("test").startSpan("http.server GET"); + span.setAttributes(attributes); + span.end(); + return exporter.getFinishedSpans()[0]; +}; + +describe("redactSpanUrlAttributes", () => { + it("strips the authorization code and state from url.full and url.query", () => { + const attributes: Record = { + "url.full": callbackUrl, + "url.query": `code=${CODE}&state=${STATE}&domain=example.test`, + "url.path": "/api/oauth/callback", + "http.request.method": "GET", + }; + + const stripped = redactSpanUrlAttributes(attributes); + + expect(stripped).toEqual(["code", "state"]); + expect(JSON.stringify(attributes)).not.toContain(CODE); + expect(JSON.stringify(attributes)).not.toContain(STATE); + // Route-level visibility is preserved. + expect(attributes["url.path"]).toBe("/api/oauth/callback"); + expect(attributes["url.full"]).toBe("https://app.test/api/oauth/callback?domain=example.test"); + expect(attributes["url.query"]).toBe("domain=example.test"); + }); + + it("strips a code nested inside the login redirect's returnTo parameter", () => { + // `/login` is not an app-owned path, so its span comes from the worker + // boundary — the callback query rides along inside `returnTo` + // (auth/return-to.ts + the sign-in redirect in start.ts). + const returnTo = encodeURIComponent(`/api/oauth/callback?code=${CODE}&state=${STATE}`); + const attributes: Record = { + "url.full": `https://app.test/login?returnTo=${returnTo}`, + "url.query": `returnTo=${returnTo}`, + }; + + const stripped = redactSpanUrlAttributes(attributes); + + expect(stripped).toEqual(["returnTo.code", "returnTo.state"]); + expect(JSON.stringify(attributes)).not.toContain(CODE); + expect(JSON.stringify(attributes)).not.toContain(STATE); + expect(String(attributes["url.full"])).toContain("%2Fapi%2Foauth%2Fcallback"); + }); + + it("leaves a span with no sensitive parameters untouched", () => { + const attributes: Record = { + "url.full": "https://app.test/api/integrations?owner=org", + "url.query": "owner=org", + "url.path": "/api/integrations", + }; + + expect(redactSpanUrlAttributes(attributes)).toEqual([]); + expect(attributes["url.full"]).toBe("https://app.test/api/integrations?owner=org"); + expect(attributes["url.query"]).toBe("owner=org"); + }); + + it("strips the other sensitive OAuth parameters", () => { + const attributes: Record = { + "url.query": + "id_token=synthetic-id-token&session_state=synthetic-session&error_description=synthetic-detail&error=access_denied", + }; + + expect(redactSpanUrlAttributes(attributes)).toEqual([ + "error_description", + "id_token", + "session_state", + ]); + // `error` is an enumerable code, not a secret — it stays. + expect(attributes["url.query"]).toBe("error=access_denied"); + }); +}); + +describe("UrlRedactingSpanProcessor", () => { + it("scrubs the span before the exporter sees it", () => { + const exported = exportSpanWith({ + "url.full": callbackUrl, + "url.query": `code=${CODE}&state=${STATE}`, + "url.path": "/api/oauth/callback", + }); + + expect(exported).toBeDefined(); + expect(JSON.stringify(exported?.attributes)).not.toContain(CODE); + expect(JSON.stringify(exported?.attributes)).not.toContain(STATE); + expect(exported?.attributes["url.path"]).toBe("/api/oauth/callback"); + expect(exported?.attributes[STRIPPED_QUERY_ATTRIBUTE]).toBe("code,state"); + }); + + it("leaves a span with no sensitive parameters unchanged", () => { + const exported = exportSpanWith({ + "url.full": "https://app.test/api/integrations?owner=org", + "url.query": "owner=org", + }); + + expect(exported?.attributes["url.full"]).toBe("https://app.test/api/integrations?owner=org"); + expect(exported?.attributes[STRIPPED_QUERY_ATTRIBUTE]).toBeUndefined(); + }); +}); diff --git a/apps/cloud/src/observability/redact-span-urls.ts b/apps/cloud/src/observability/redact-span-urls.ts new file mode 100644 index 000000000..12f6921f9 --- /dev/null +++ b/apps/cloud/src/observability/redact-span-urls.ts @@ -0,0 +1,189 @@ +// --------------------------------------------------------------------------- +// URL-attribute redaction for exported spans. +// +// Effect's `HttpMiddleware.tracer` stamps `url.full` and `url.query` +// unconditionally on every `http.server` span (see +// `effect/unstable/http/HttpMiddleware.ts` — it redacts URL userinfo and +// configured header names, and nothing else). `/api/oauth/callback` is an +// app-owned path, so its span carried the provider's `?code=…&state=…` +// verbatim to the trace backend on every OAuth connect. +// +// The scrub runs at the span-processor seam rather than at the route, because +// this is the only chokepoint every span in the isolate must pass through on +// its way to the exporter — worker spans, Effect spans, Durable Object spans, +// and any route added later. A per-route middleware or a `TracerDisabledWhen` +// override would only cover the routes someone remembered to wire it into, and +// overriding the middleware's attribute handling would mean forking Effect +// internals. +// +// `url.path` is deliberately preserved: route-level visibility is what makes +// these traces worth exporting at all. Only the sensitive query parameters are +// removed, and their presence is recorded as a stripped-key list so a trace +// still shows that the request carried a `code`, without its value. +// --------------------------------------------------------------------------- + +import type { Context } from "@opentelemetry/api"; +import type { ReadableSpan, Span, SpanProcessor } from "@opentelemetry/sdk-trace-base"; + +/** Query parameters that are credentials, single-use grants, or CSRF secrets. + * Matched case-insensitively against the parameter name. */ +const SENSITIVE_QUERY_KEYS: ReadonlySet = new Set([ + "access_token", + "client_secret", + "code", + "code_verifier", + "error_description", + "id_token", + "refresh_token", + "session_state", + "state", + "token", +]); + +/** Span attributes whose value is a whole URL. */ +const URL_ATTRIBUTES = ["url.full", "http.url"] as const; +const QUERY_ATTRIBUTE = "url.query"; +/** Names of the parameters removed from this span's URL attributes. Non-secret + * by construction — it is the key list, never the values. */ +export const STRIPPED_QUERY_ATTRIBUTE = "url.query.stripped_keys"; + +const isSensitive = (key: string): boolean => SENSITIVE_QUERY_KEYS.has(key.toLowerCase()); + +/** How deep to follow a query parameter whose value is itself a URL or path + * with a query string. Depth 2 covers the real nesting in this app — + * `/login?returnTo=%2Fapi%2Foauth%2Fcallback%3Fcode%3D…` (see + * `auth/return-to.ts` and the sign-in redirect in `start.ts`) — with headroom, + * and bounds the work per span. */ +const MAX_NESTED_DEPTH = 2; + +/** The query string with every sensitive parameter dropped, plus the names + * dropped. Parsed with `URLSearchParams` so encoding round-trips correctly. + * + * A parameter whose own value carries a nested query string is redacted + * recursively rather than dropped: the login redirect round-trips the whole + * OAuth callback path through `returnTo`, so the credential rides inside + * another parameter's value and a top-level key check alone would miss it. */ +const redactQuery = ( + query: string, + depth = 0, +): { readonly query: string; readonly stripped: readonly string[] } => { + const params = new URLSearchParams(query); + const stripped = new Set(); + for (const key of Array.from(new Set(params.keys()))) { + if (isSensitive(key)) { + stripped.add(key); + params.delete(key); + continue; + } + if (depth >= MAX_NESTED_DEPTH) continue; + const values = params.getAll(key); + const rewritten = values.map((value) => { + const separator = value.indexOf("?"); + if (separator === -1) return value; + const nested = redactQuery(value.slice(separator + 1), depth + 1); + for (const name of nested.stripped) stripped.add(`${key}.${name}`); + return nested.stripped.length === 0 + ? value + : `${value.slice(0, separator)}${nested.query === "" ? "" : `?${nested.query}`}`; + }); + if (rewritten.every((value, index) => value === values[index])) continue; + params.delete(key); + for (const value of rewritten) params.append(key, value); + } + return stripped.size === 0 + ? { query, stripped: [] } + : { query: params.toString(), stripped: Array.from(stripped).sort() }; +}; + +/** The URL with every sensitive query parameter dropped. Path, host, and + * scheme are untouched. Unparseable values are dropped entirely rather than + * passed through — if it cannot be parsed it cannot be proven safe. */ +const redactUrl = ( + value: string, +): { readonly url: string; readonly stripped: readonly string[] } => { + if (!URL.canParse(value)) { + const { query, stripped } = redactQuery(value); + return { url: stripped.length === 0 ? value : query, stripped }; + } + const url = new URL(value); + if (url.search === "") return { url: value, stripped: [] }; + const { query, stripped } = redactQuery(url.search.slice(1)); + if (stripped.length === 0) return { url: value, stripped: [] }; + url.search = query; + return { url: url.toString(), stripped }; +}; + +/** Rewrites the URL-bearing attributes of an in-flight span in place, dropping + * sensitive query parameters. Returns the parameter names removed. */ +export const redactSpanUrlAttributes = (attributes: Record): readonly string[] => { + const stripped = new Set(); + for (const name of URL_ATTRIBUTES) { + const value = attributes[name]; + if (typeof value !== "string") continue; + const result = redactUrl(value); + for (const key of result.stripped) stripped.add(key); + if (result.stripped.length > 0) attributes[name] = result.url; + } + const query = attributes[QUERY_ATTRIBUTE]; + if (typeof query === "string") { + const result = redactQuery(query); + for (const key of result.stripped) stripped.add(key); + if (result.stripped.length > 0) attributes[QUERY_ATTRIBUTE] = result.query; + } + return Array.from(stripped).sort(); +}; + +/** Wraps a span processor so every span is scrubbed of credential-bearing URL + * query parameters before the inner processor (and therefore the exporter) + * sees it. + * + * The rewrite happens in `onEnding`, the last hook the OTel SDK calls while + * the span is still mutable (`Span.end()` runs `onEnding` before setting + * `_ended`, so `setAttribute` still applies); `onEnd` receives a frozen + * `ReadableSpan`. `onEnding` is optional in the SpanProcessor interface, so + * `onEnd` re-checks the attribute bag and mutates it directly as a backstop + * for any SDK path that skips the earlier hook. */ +export class UrlRedactingSpanProcessor implements SpanProcessor { + constructor(private readonly inner: SpanProcessor) {} + + forceFlush(): Promise { + return this.inner.forceFlush(); + } + + onStart(span: Span, parentContext: Context): void { + this.inner.onStart(span, parentContext); + } + + onEnding(span: Span): void { + this.redact(span.attributes, (key, value) => span.setAttribute(key, value)); + this.inner.onEnding?.(span); + } + + onEnd(span: ReadableSpan): void { + const attributes = span.attributes as Record; + this.redact(attributes, (key, value) => { + attributes[key] = value; + }); + this.inner.onEnd(span); + } + + shutdown(): Promise { + return this.inner.shutdown(); + } + + private redact( + attributes: Record, + write: (key: string, value: string) => void, + ): void { + // Work on a copy so the redaction decision is made from the current values + // and applied through the caller's writer (span API vs direct mutation). + const draft: Record = { ...attributes }; + const stripped = redactSpanUrlAttributes(draft); + if (stripped.length === 0) return; + for (const name of [...URL_ATTRIBUTES, QUERY_ATTRIBUTE]) { + const value = draft[name]; + if (typeof value === "string" && value !== attributes[name]) write(name, value); + } + write(STRIPPED_QUERY_ATTRIBUTE, stripped.join(",")); + } +} diff --git a/apps/cloud/src/observability/telemetry.ts b/apps/cloud/src/observability/telemetry.ts index 7bd5baa58..5b0c11dc1 100644 --- a/apps/cloud/src/observability/telemetry.ts +++ b/apps/cloud/src/observability/telemetry.ts @@ -51,6 +51,7 @@ import { OTEL_MAX_SPAN_QUEUE_SIZE, recordForceFlush, } from "./memory-metrics"; +import { UrlRedactingSpanProcessor } from "./redact-span-urls"; const SERVICE_NAME = "executor-cloud"; const SERVICE_VERSION = "1.0.0"; @@ -93,7 +94,12 @@ const ensureGlobalTracerProvider = (): boolean => { }), OTEL_MAX_SPAN_QUEUE_SIZE, ); - return [countingProcessor]; + // Outermost wrapper: every span the isolate produces passes through here + // before it is queued for export, so credential-bearing query parameters + // (OAuth `code`/`state` on `/api/oauth/callback`, which Effect's + // HttpMiddleware.tracer stamps into `url.full`/`url.query` + // unconditionally) are stripped no matter which route emitted the span. + return [new UrlRedactingSpanProcessor(countingProcessor)]; })(), }); // Skip `provider.register()` — its StackContextManager / W3C propagator diff --git a/packages/core/sdk/src/index.ts b/packages/core/sdk/src/index.ts index 73a8e9f6c..2bf3d8797 100644 --- a/packages/core/sdk/src/index.ts +++ b/packages/core/sdk/src/index.ts @@ -412,3 +412,7 @@ export { insufficientScopeFromEmbeddedJson, type InsufficientScopeDetection, } from "./insufficient-scope"; + +// Endpoint sanitization for span attributes — plugins stamping a user-supplied +// endpoint must strip its credential-bearing parts first. +export { endpointForTelemetry, endpointTelemetryAttributes } from "./telemetry-endpoint"; diff --git a/packages/core/sdk/src/telemetry-endpoint.test.ts b/packages/core/sdk/src/telemetry-endpoint.test.ts new file mode 100644 index 000000000..3d9a8ac62 --- /dev/null +++ b/packages/core/sdk/src/telemetry-endpoint.test.ts @@ -0,0 +1,68 @@ +import { describe, expect, it } from "@effect/vitest"; + +import { endpointForTelemetry, endpointTelemetryAttributes } from "./telemetry-endpoint"; + +// Synthetic placeholders only. +const QUERY_TOKEN = "synthetic-query-token"; +const USERINFO_PASSWORD = "synthetic-userinfo-password"; + +describe("endpointForTelemetry", () => { + it("strips a credential carried in the query string", () => { + // The shape the MCP preset list ships and the add-flow passes through raw. + expect(endpointForTelemetry(`https://mcp.example.test/mcp?token=${QUERY_TOKEN}`)).toBe( + "https://mcp.example.test/mcp", + ); + }); + + it("strips a credential carried in URL userinfo", () => { + expect(endpointForTelemetry(`https://svc-user:${USERINFO_PASSWORD}@mcp.example.test/mcp`)).toBe( + "https://mcp.example.test/mcp", + ); + }); + + it("strips query, fragment, and userinfo together", () => { + const scrubbed = endpointForTelemetry( + `https://svc-user:${USERINFO_PASSWORD}@mcp.example.test/mcp?token=${QUERY_TOKEN}#frag`, + ); + expect(scrubbed).toBe("https://mcp.example.test/mcp"); + expect(scrubbed).not.toContain(QUERY_TOKEN); + expect(scrubbed).not.toContain(USERINFO_PASSWORD); + }); + + it("leaves a credential-free endpoint intact", () => { + expect(endpointForTelemetry("https://mcp.example.test/mcp")).toBe( + "https://mcp.example.test/mcp", + ); + }); + + it("returns unparseable input as-is", () => { + expect(endpointForTelemetry("not a url")).toBe("not a url"); + }); +}); + +describe("endpointTelemetryAttributes", () => { + it("keeps the endpoint debuggable without exposing the credential", () => { + const attributes = endpointTelemetryAttributes( + "mcp.endpoint", + `https://svc-user:${USERINFO_PASSWORD}@mcp.example.test/mcp?token=${QUERY_TOKEN}`, + ); + + expect(attributes).toEqual({ + "mcp.endpoint": "https://mcp.example.test/mcp", + "mcp.endpoint.origin": "https://mcp.example.test", + "mcp.endpoint.has_query": true, + "mcp.endpoint.has_userinfo": true, + }); + expect(JSON.stringify(attributes)).not.toContain(QUERY_TOKEN); + expect(JSON.stringify(attributes)).not.toContain(USERINFO_PASSWORD); + }); + + it("reports absence of both credential shapes for a plain endpoint", () => { + expect(endpointTelemetryAttributes("mcp.endpoint", "https://mcp.example.test/mcp")).toEqual({ + "mcp.endpoint": "https://mcp.example.test/mcp", + "mcp.endpoint.origin": "https://mcp.example.test", + "mcp.endpoint.has_query": false, + "mcp.endpoint.has_userinfo": false, + }); + }); +}); diff --git a/packages/core/sdk/src/telemetry-endpoint.ts b/packages/core/sdk/src/telemetry-endpoint.ts new file mode 100644 index 000000000..cba48a721 --- /dev/null +++ b/packages/core/sdk/src/telemetry-endpoint.ts @@ -0,0 +1,54 @@ +// --------------------------------------------------------------------------- +// Endpoint sanitization for span attributes. +// +// User-supplied endpoints are a credential carrier: `?token=…` in the query +// string and `user:pass@host` userinfo are both first-class supported input +// shapes (the MCP preset list ships a query-token URL, and the add-flow passes +// the raw paste straight through). Stamping such a URL verbatim onto a span +// ships the credential to the trace backend. +// +// Span attributes may carry hostnames, paths, and booleans; they may never +// carry the secret-bearing parts of a URL. `endpointForTelemetry` keeps the +// scheme/host/path — the parts that make a trace debuggable — and drops the +// query, fragment, and userinfo. `endpointTelemetryAttributes` adds the +// non-sensitive companions (origin, and whether a query string was present) so +// "the user pasted a URL with credentials in it" stays diagnosable without the +// credential itself. +// --------------------------------------------------------------------------- + +/** The endpoint with every credential-bearing component removed: query string, + * fragment, and `user:pass@` userinfo. Unparseable input is returned as-is — + * it is not a URL, so there is nothing to strip, and callers still want the + * literal for debugging a malformed paste. */ +export const endpointForTelemetry = (endpoint: string): string => { + if (!URL.canParse(endpoint)) return endpoint; + const url = new URL(endpoint); + url.search = ""; + url.hash = ""; + url.username = ""; + url.password = ""; + return url.toString(); +}; + +/** Span attributes describing an endpoint without exposing its credentials. + * `` is the sanitized URL, `.origin` the scheme+host, and the + * two booleans record that a query string / userinfo was stripped. */ +export const endpointTelemetryAttributes = ( + prefix: string, + endpoint: string, +): Record => { + if (!URL.canParse(endpoint)) { + return { + [prefix]: endpoint, + [`${prefix}.has_query`]: false, + [`${prefix}.has_userinfo`]: false, + }; + } + const url = new URL(endpoint); + return { + [prefix]: endpointForTelemetry(endpoint), + [`${prefix}.origin`]: url.origin, + [`${prefix}.has_query`]: url.search !== "", + [`${prefix}.has_userinfo`]: url.username !== "" || url.password !== "", + }; +}; diff --git a/packages/plugins/graphql/src/sdk/invoke.ts b/packages/plugins/graphql/src/sdk/invoke.ts index 819acefb0..70ca61150 100644 --- a/packages/plugins/graphql/src/sdk/invoke.ts +++ b/packages/plugins/graphql/src/sdk/invoke.ts @@ -1,6 +1,8 @@ import { Effect, Layer, Option } from "effect"; import { HttpClient, HttpClientRequest } from "effect/unstable/http"; +import { endpointForTelemetry } from "@executor-js/sdk/core"; + import { GraphqlInvocationError } from "./errors"; import { type OperationBinding, InvocationResult } from "./types"; @@ -13,14 +15,6 @@ const endpointWithQueryParams = (endpoint: string, queryParams: Record { - if (!URL.canParse(endpoint)) return endpoint; - const url = new URL(endpoint); - url.search = ""; - url.hash = ""; - return url.toString(); -}; - /** The operation string to send for a call. A caller-supplied `select` overrides * the default scalar-leaf selection: it is spliced into the field's selection * set (`field {