Unified · Split
| @@ -0,0 +1,13 @@ | ||
| 1 | +--- | |
| 2 | +"executor": patch | |
| 3 | +--- | |
| 4 | + | |
| 5 | +Close the remaining credential leaks at the telemetry export seam. Span events | |
| 6 | +and status messages are now scrubbed alongside URL attributes, so the | |
| 7 | +exception messages, stack traces, and pretty-printed causes every failed span | |
| 8 | +carries no longer ship query-string credentials, and unbounded stack text is | |
| 9 | +capped. The browser tracer applies the same scrub as the server (previously it | |
| 10 | +exported unredacted), the tracer's redacted-header list is widened past | |
| 11 | +Effect's four defaults to cover the header names an integration's auth | |
| 12 | +placement can mint, and the OpenAPI invoke span's `base_url` is sanitized like | |
| 13 | +the mcp and graphql endpoints already were. | |
| @@ -102,7 +102,21 @@ describe("oauth callback telemetry", () => { · 101 unmodified lines | ||
| 102 | 102 | const serverSpan = spans.find((span) => span.name.startsWith("http.server")); |
| 103 | 103 | expect(serverSpan).toBeDefined(); |
| 104 | 104 | |
| 105 | − const serialized = JSON.stringify(spans.map((span) => span.attributes)); | |
| 105 | + // Every surface that leaves the isolate, not just the attributes: a | |
| 106 | + // failed span carries the grant a second time through its exception | |
| 107 | + // events and status message (Effect's tracer logger and the OTel bridge's | |
| 108 | + // recordException), which an attribute-only assertion cannot see. Named | |
| 109 | + // fields rather than the whole `ReadableSpan` because the SDK's span | |
| 110 | + // objects hold a back-reference to their processor and do not serialize. | |
| 111 | + const serialized = JSON.stringify( | |
| 112 | + spans.map((span) => ({ | |
| 113 | + name: span.name, | |
| 114 | + attributes: span.attributes, | |
| 115 | + events: span.events, | |
| 116 | + status: span.status, | |
| 117 | + links: span.links, | |
| 118 | + })), | |
| 119 | + ); | |
| 106 | 120 | expect(serialized).not.toContain(CODE); |
| 107 | 121 | expect(serialized).not.toContain(STATE); |
| 108 | 122 | |
| @@ -1,16 +1,21 @@ | ||
| 1 | +// --------------------------------------------------------------------------- | |
| 2 | +// The OTel span-processor seam. The redaction POLICY is covered by | |
| 3 | +// `packages/core/sdk/src/span-redaction.test.ts`; what is under test here is | |
| 4 | +// that the processor reaches every surface a span ships credentials on — | |
| 5 | +// attributes, events, and `status.message` — before the exporter sees it. | |
| 6 | +// --------------------------------------------------------------------------- | |
| 7 | + | |
| 1 | 8 | import { describe, expect, it } from "@effect/vitest"; |
| 9 | +import { SpanStatusCode } from "@opentelemetry/api"; | |
| 2 | 10 | import { |
| 3 | 11 | BasicTracerProvider, |
| 4 | 12 | InMemorySpanExporter, |
| 5 | 13 | SimpleSpanProcessor, |
| 6 | 14 | type ReadableSpan, |
| 15 | + type Span, | |
| 7 | 16 | } from "@opentelemetry/sdk-trace-base"; |
| 8 | 17 | |
| 9 | −import { | |
| 10 | − redactSpanUrlAttributes, | |
| 11 | − STRIPPED_QUERY_ATTRIBUTE, | |
| 12 | − UrlRedactingSpanProcessor, | |
| 13 | −} from "./redact-span-urls"; | |
| 18 | +import { STRIPPED_QUERY_ATTRIBUTE, UrlRedactingSpanProcessor } from "./redact-span-urls"; | |
| 14 | 19 | |
| 15 | 20 | // Synthetic placeholders only — never a real authorization code or state. |
| 16 | 21 | const CODE = "synthetic-authorization-code"; |
| @@ -18,89 +23,26 @@ const STATE = "synthetic-csrf-state"; · 1 unmodified line | ||
| 18 | 23 | |
| 19 | 24 | const callbackUrl = `https://app.test/api/oauth/callback?code=${CODE}&state=${STATE}&domain=example.test`; |
| 20 | 25 | |
| 21 | −/** Ends one real SDK span carrying `attributes` through the redacting | |
| 22 | − * processor, and returns what the exporter actually received. Using the real | |
| 23 | − * provider (rather than a hand-built span) exercises the `onEnding` → `onEnd` | |
| 24 | − * hook sequence exactly as production does. */ | |
| 25 | −const exportSpanWith = (attributes: Record<string, string>): ReadableSpan | undefined => { | |
| 26 | +/** Ends one real SDK span through the redacting processor and returns what the | |
| 27 | + * exporter actually received. Using the real provider (rather than a | |
| 28 | + * hand-built span) exercises the `onEnding` → `onEnd` hook sequence exactly as | |
| 29 | + * production does. */ | |
| 30 | +const exportSpan = (record: (span: Span) => void): ReadableSpan | undefined => { | |
| 26 | 31 | const exporter = new InMemorySpanExporter(); |
| 27 | 32 | const provider = new BasicTracerProvider({ |
| 28 | 33 | spanProcessors: [new UrlRedactingSpanProcessor(new SimpleSpanProcessor(exporter))], |
| 29 | 34 | }); |
| 30 | − const span = provider.getTracer("test").startSpan("http.server GET"); | |
| 31 | − span.setAttributes(attributes); | |
| 35 | + const span = provider.getTracer("test").startSpan("http.server GET") as Span; | |
| 36 | + record(span); | |
| 32 | 37 | span.end(); |
| 33 | 38 | return exporter.getFinishedSpans()[0]; |
| 34 | 39 | }; |
| 35 | 40 | |
| 36 | −describe("redactSpanUrlAttributes", () => { | |
| 37 | − it("strips the authorization code and state from url.full and url.query", () => { | |
| 38 | − const attributes: Record<string, unknown> = { | |
| 39 | − "url.full": callbackUrl, | |
| 40 | − "url.query": `code=${CODE}&state=${STATE}&domain=example.test`, | |
| 41 | − "url.path": "/api/oauth/callback", | |
| 42 | − "http.request.method": "GET", | |
| 43 | − }; | |
| 44 | − | |
| 45 | − const stripped = redactSpanUrlAttributes(attributes); | |
| 46 | − | |
| 47 | − expect(stripped).toEqual(["code", "state"]); | |
| 48 | − expect(JSON.stringify(attributes)).not.toContain(CODE); | |
| 49 | − expect(JSON.stringify(attributes)).not.toContain(STATE); | |
| 50 | − // Route-level visibility is preserved. | |
| 51 | − expect(attributes["url.path"]).toBe("/api/oauth/callback"); | |
| 52 | − expect(attributes["url.full"]).toBe("https://app.test/api/oauth/callback?domain=example.test"); | |
| 53 | − expect(attributes["url.query"]).toBe("domain=example.test"); | |
| 54 | − }); | |
| 55 | − | |
| 56 | − it("strips a code nested inside the login redirect's returnTo parameter", () => { | |
| 57 | − // `/login` is not an app-owned path, so its span comes from the worker | |
| 58 | − // boundary — the callback query rides along inside `returnTo` | |
| 59 | − // (auth/return-to.ts + the sign-in redirect in start.ts). | |
| 60 | − const returnTo = encodeURIComponent(`/api/oauth/callback?code=${CODE}&state=${STATE}`); | |
| 61 | − const attributes: Record<string, unknown> = { | |
| 62 | − "url.full": `https://app.test/login?returnTo=${returnTo}`, | |
| 63 | − "url.query": `returnTo=${returnTo}`, | |
| 64 | − }; | |
| 65 | − | |
| 66 | − const stripped = redactSpanUrlAttributes(attributes); | |
| 67 | − | |
| 68 | − expect(stripped).toEqual(["returnTo.code", "returnTo.state"]); | |
| 69 | − expect(JSON.stringify(attributes)).not.toContain(CODE); | |
| 70 | − expect(JSON.stringify(attributes)).not.toContain(STATE); | |
| 71 | − expect(String(attributes["url.full"])).toContain("%2Fapi%2Foauth%2Fcallback"); | |
| 72 | − }); | |
| 73 | − | |
| 74 | − it("leaves a span with no sensitive parameters untouched", () => { | |
| 75 | − const attributes: Record<string, unknown> = { | |
| 76 | − "url.full": "https://app.test/api/integrations?owner=org", | |
| 77 | − "url.query": "owner=org", | |
| 78 | − "url.path": "/api/integrations", | |
| 79 | − }; | |
| 80 | − | |
| 81 | − expect(redactSpanUrlAttributes(attributes)).toEqual([]); | |
| 82 | − expect(attributes["url.full"]).toBe("https://app.test/api/integrations?owner=org"); | |
| 83 | − expect(attributes["url.query"]).toBe("owner=org"); | |
| 84 | − }); | |
| 85 | − | |
| 86 | − it("strips the other sensitive OAuth parameters", () => { | |
| 87 | − const attributes: Record<string, unknown> = { | |
| 88 | − "url.query": | |
| 89 | − "id_token=synthetic-id-token&session_state=synthetic-session&error_description=synthetic-detail&error=access_denied", | |
| 90 | − }; | |
| 91 | − | |
| 92 | − expect(redactSpanUrlAttributes(attributes)).toEqual([ | |
| 93 | − "error_description", | |
| 94 | − "id_token", | |
| 95 | − "session_state", | |
| 96 | − ]); | |
| 97 | − // `error` is an enumerable code, not a secret — it stays. | |
| 98 | − expect(attributes["url.query"]).toBe("error=access_denied"); | |
| 99 | − }); | |
| 100 | −}); | |
| 41 | +const exportSpanWith = (attributes: Record<string, string>): ReadableSpan | undefined => | |
| 42 | + exportSpan((span) => span.setAttributes(attributes)); | |
| 101 | 43 | |
| 102 | 44 | describe("UrlRedactingSpanProcessor", () => { |
| 103 | − it("scrubs the span before the exporter sees it", () => { | |
| 45 | + it("scrubs the span attributes before the exporter sees them", () => { | |
| 104 | 46 | const exported = exportSpanWith({ |
| 105 | 47 | "url.full": callbackUrl, |
| 106 | 48 | "url.query": `code=${CODE}&state=${STATE}`, |
| @@ -123,4 +65,77 @@ describe("UrlRedactingSpanProcessor", () => { · 16 unmodified lines | ||
| 123 | 65 | expect(exported?.attributes["url.full"]).toBe("https://app.test/api/integrations?owner=org"); |
| 124 | 66 | expect(exported?.attributes[STRIPPED_QUERY_ATTRIBUTE]).toBeUndefined(); |
| 125 | 67 | }); |
| 68 | + | |
| 69 | + it("scrubs an exception event carrying the code in its message and stacktrace", () => { | |
| 70 | + // The shape `@effect/opentelemetry`'s `recordException` produces on every | |
| 71 | + // failed span — the URL scrubber never sees these, they are event | |
| 72 | + // attributes, not span attributes. | |
| 73 | + const exported = exportSpan((span) => { | |
| 74 | + span.recordException({ | |
| 75 | + name: "RequestError", | |
| 76 | + message: `POST ${callbackUrl} failed with 400`, | |
| 77 | + stack: `RequestError: POST ${callbackUrl}\n at fetch (executor.js:1:1)`, | |
| 78 | + }); | |
| 79 | + }); | |
| 80 | + | |
| 81 | + const serialized = JSON.stringify(exported?.events); | |
| 82 | + expect(serialized).not.toContain(CODE); | |
| 83 | + expect(serialized).not.toContain(STATE); | |
| 84 | + // The diagnosable part survives. | |
| 85 | + expect(serialized).toContain("/api/oauth/callback"); | |
| 86 | + expect(serialized).toContain("failed with 400"); | |
| 87 | + }); | |
| 88 | + | |
| 89 | + it("scrubs a log event whose NAME is the already-interpolated message", () => { | |
| 90 | + // Effect's default tracer logger writes the interpolated message as the | |
| 91 | + // event name and the pretty cause as an attribute. | |
| 92 | + const exported = exportSpan((span) => { | |
| 93 | + span.addEvent(`redirecting to /oauth/callback?code=${CODE}`, { | |
| 94 | + "effect.cause": `Error: GET ${callbackUrl}`, | |
| 95 | + "effect.logLevel": "ERROR", | |
| 96 | + }); | |
| 97 | + }); | |
| 98 | + | |
| 99 | + const serialized = JSON.stringify(exported?.events); | |
| 100 | + expect(serialized).not.toContain(CODE); | |
| 101 | + expect(serialized).not.toContain(STATE); | |
| 102 | + expect(exported?.events[0]?.name).toBe("redirecting to /oauth/callback"); | |
| 103 | + expect(exported?.events[0]?.attributes?.["effect.logLevel"]).toBe("ERROR"); | |
| 104 | + }); | |
| 105 | + | |
| 106 | + it("caps an unbounded stacktrace", () => { | |
| 107 | + const exported = exportSpan((span) => { | |
| 108 | + span.recordException({ | |
| 109 | + name: "DeepError", | |
| 110 | + message: "failed", | |
| 111 | + stack: `DeepError: failed\n${" at frame (executor.js:1:1)\n".repeat(2_000)}`, | |
| 112 | + }); | |
| 113 | + }); | |
| 114 | + | |
| 115 | + const stacktrace = String(exported?.events[0]?.attributes?.["exception.stacktrace"] ?? ""); | |
| 116 | + expect(stacktrace).toContain("truncated"); | |
| 117 | + expect(stacktrace.length).toBeLessThan(10_000); | |
| 118 | + }); | |
| 119 | + | |
| 120 | + it("scrubs status.message", () => { | |
| 121 | + // `@effect/opentelemetry` sets this to the first error's message, which is | |
| 122 | + // the same free text the exception event carries. | |
| 123 | + const exported = exportSpan((span) => { | |
| 124 | + span.setStatus({ code: SpanStatusCode.ERROR, message: `GET ${callbackUrl} failed` }); | |
| 125 | + }); | |
| 126 | + | |
| 127 | + expect(exported?.status.message).not.toContain(CODE); | |
| 128 | + expect(exported?.status.message).toContain("/api/oauth/callback"); | |
| 129 | + }); | |
| 130 | + | |
| 131 | + it("leaves a successful span's events and status alone", () => { | |
| 132 | + const exported = exportSpan((span) => { | |
| 133 | + span.addEvent("tool.invoked", { "tool.name": "list_things" }); | |
| 134 | + span.setStatus({ code: SpanStatusCode.OK }); | |
| 135 | + }); | |
| 136 | + | |
| 137 | + expect(exported?.events[0]?.name).toBe("tool.invoked"); | |
| 138 | + expect(exported?.events[0]?.attributes?.["tool.name"]).toBe("list_things"); | |
| 139 | + expect(exported?.status.message).toBeUndefined(); | |
| 140 | + }); | |
| 126 | 141 | }); |
| @@ -1,12 +1,10 @@ | ||
| 1 | 1 | // --------------------------------------------------------------------------- |
| 2 | −// URL-attribute redaction for exported spans. | |
| 2 | +// Export-seam redaction for the Workers isolates' OTel span pipeline. | |
| 3 | 3 | // |
| 4 | −// Effect's `HttpMiddleware.tracer` stamps `url.full` and `url.query` | |
| 5 | −// unconditionally on every `http.server` span (see | |
| 6 | −// `effect/unstable/http/HttpMiddleware.ts` — it redacts URL userinfo and | |
| 7 | −// configured header names, and nothing else). `/api/oauth/callback` is an | |
| 8 | −// app-owned path, so its span carried the provider's `?code=…&state=…` | |
| 9 | −// verbatim to the trace backend on every OAuth connect. | |
| 4 | +// The decisions (which query parameters are sensitive, how free text is | |
| 5 | +// scrubbed and capped) live in `@executor-js/sdk/shared` so the browser | |
| 6 | +// isolate's exporter applies the identical policy. This file is only the OTel | |
| 7 | +// `SpanProcessor` shell around them. | |
| 10 | 8 | // |
| 11 | 9 | // The scrub runs at the span-processor seam rather than at the route, because |
| 12 | 10 | // this is the only chokepoint every span in the isolate must pass through on |
| @@ -16,6 +14,17 @@ · 3 unmodified lines | ||
| 16 | 14 | // overriding the middleware's attribute handling would mean forking Effect |
| 17 | 15 | // internals. |
| 18 | 16 | // |
| 17 | +// Three surfaces carry credentials out, not one: | |
| 18 | +// | |
| 19 | +// - Attributes. `HttpMiddleware.tracer` stamps `url.full` and `url.query` | |
| 20 | +// unconditionally on every `http.server` span, so `/api/oauth/callback` | |
| 21 | +// carried the provider's `?code=…&state=…` verbatim on every OAuth connect. | |
| 22 | +// - Events. Effect's default tracer logger writes the interpolated log | |
| 23 | +// message as the EVENT NAME and the whole `effect.cause` as an event | |
| 24 | +// attribute; the OTel bridge's `recordException` writes `exception.message` | |
| 25 | +// and `exception.stacktrace` the same way. Every failed span has them. | |
| 26 | +// - `status.message`, which the bridge sets to the first error's message. | |
| 27 | +// | |
| 19 | 28 | // `url.path` is deliberately preserved: route-level visibility is what makes |
| 20 | 29 | // these traces worth exporting at all. Only the sensitive query parameters are |
| 21 | 30 | // removed, and their presence is recorded as a stripped-key list so a trace |
| @@ -25,124 +34,48 @@ · 3 unmodified lines | ||
| 25 | 34 | import type { Context } from "@opentelemetry/api"; |
| 26 | 35 | import type { ReadableSpan, Span, SpanProcessor } from "@opentelemetry/sdk-trace-base"; |
| 27 | 36 | |
| 28 | −/** Query parameters that are credentials, single-use grants, or CSRF secrets. | |
| 29 | − * Matched case-insensitively against the parameter name. */ | |
| 30 | −const SENSITIVE_QUERY_KEYS: ReadonlySet<string> = new Set([ | |
| 31 | − "access_token", | |
| 32 | − "client_secret", | |
| 33 | − "code", | |
| 34 | − "code_verifier", | |
| 35 | − "error_description", | |
| 36 | − "id_token", | |
| 37 | − "refresh_token", | |
| 38 | − "session_state", | |
| 39 | − "state", | |
| 40 | − "token", | |
| 41 | −]); | |
| 42 | − | |
| 43 | −/** Span attributes whose value is a whole URL. */ | |
| 44 | −const URL_ATTRIBUTES = ["url.full", "http.url"] as const; | |
| 45 | −const QUERY_ATTRIBUTE = "url.query"; | |
| 46 | −/** Names of the parameters removed from this span's URL attributes. Non-secret | |
| 47 | − * by construction — it is the key list, never the values. */ | |
| 48 | −export const STRIPPED_QUERY_ATTRIBUTE = "url.query.stripped_keys"; | |
| 49 | − | |
| 50 | −const isSensitive = (key: string): boolean => SENSITIVE_QUERY_KEYS.has(key.toLowerCase()); | |
| 37 | +import { | |
| 38 | + SPAN_QUERY_ATTRIBUTE, | |
| 39 | + SPAN_URL_ATTRIBUTES, | |
| 40 | + STRIPPED_QUERY_ATTRIBUTE, | |
| 41 | + redactSpanUrlAttributes, | |
| 42 | + scrubSpanText, | |
| 43 | +} from "@executor-js/sdk/shared"; | |
| 51 | 44 | |
| 52 | −/** How deep to follow a query parameter whose value is itself a URL or path | |
| 53 | − * with a query string. Depth 2 covers the real nesting in this app — | |
| 54 | − * `/login?returnTo=%2Fapi%2Foauth%2Fcallback%3Fcode%3D…` (see | |
| 55 | − * `auth/return-to.ts` and the sign-in redirect in `start.ts`) — with headroom, | |
| 56 | − * and bounds the work per span. */ | |
| 57 | −const MAX_NESTED_DEPTH = 2; | |
| 45 | +export { STRIPPED_QUERY_ATTRIBUTE, redactSpanUrlAttributes }; | |
| 58 | 46 | |
| 59 | −/** The query string with every sensitive parameter dropped, plus the names | |
| 60 | − * dropped. Parsed with `URLSearchParams` so encoding round-trips correctly. | |
| 61 | − * | |
| 62 | − * A parameter whose own value carries a nested query string is redacted | |
| 63 | − * recursively rather than dropped: the login redirect round-trips the whole | |
| 64 | − * OAuth callback path through `returnTo`, so the credential rides inside | |
| 65 | − * another parameter's value and a top-level key check alone would miss it. */ | |
| 66 | −const redactQuery = ( | |
| 67 | − query: string, | |
| 68 | − depth = 0, | |
| 69 | −): { readonly query: string; readonly stripped: readonly string[] } => { | |
| 70 | − const params = new URLSearchParams(query); | |
| 71 | − const stripped = new Set<string>(); | |
| 72 | − for (const key of Array.from(new Set(params.keys()))) { | |
| 73 | − if (isSensitive(key)) { | |
| 74 | − stripped.add(key); | |
| 75 | − params.delete(key); | |
| 76 | − continue; | |
| 77 | − } | |
| 78 | − if (depth >= MAX_NESTED_DEPTH) continue; | |
| 79 | − const values = params.getAll(key); | |
| 80 | − const rewritten = values.map((value) => { | |
| 81 | − const separator = value.indexOf("?"); | |
| 82 | − if (separator === -1) return value; | |
| 83 | − const nested = redactQuery(value.slice(separator + 1), depth + 1); | |
| 84 | − for (const name of nested.stripped) stripped.add(`${key}.${name}`); | |
| 85 | − return nested.stripped.length === 0 | |
| 86 | − ? value | |
| 87 | − : `${value.slice(0, separator)}${nested.query === "" ? "" : `?${nested.query}`}`; | |
| 88 | − }); | |
| 89 | − if (rewritten.every((value, index) => value === values[index])) continue; | |
| 90 | − params.delete(key); | |
| 91 | − for (const value of rewritten) params.append(key, value); | |
| 92 | − } | |
| 93 | − return stripped.size === 0 | |
| 94 | − ? { query, stripped: [] } | |
| 95 | − : { query: params.toString(), stripped: Array.from(stripped).sort() }; | |
| 96 | −}; | |
| 97 | − | |
| 98 | −/** The URL with every sensitive query parameter dropped. Path, host, and | |
| 99 | − * scheme are untouched. Unparseable values are dropped entirely rather than | |
| 100 | − * passed through — if it cannot be parsed it cannot be proven safe. */ | |
| 101 | −const redactUrl = ( | |
| 102 | − value: string, | |
| 103 | −): { readonly url: string; readonly stripped: readonly string[] } => { | |
| 104 | − if (!URL.canParse(value)) { | |
| 105 | − const { query, stripped } = redactQuery(value); | |
| 106 | − return { url: stripped.length === 0 ? value : query, stripped }; | |
| 107 | − } | |
| 108 | − const url = new URL(value); | |
| 109 | − if (url.search === "") return { url: value, stripped: [] }; | |
| 110 | − const { query, stripped } = redactQuery(url.search.slice(1)); | |
| 111 | − if (stripped.length === 0) return { url: value, stripped: [] }; | |
| 112 | − url.search = query; | |
| 113 | − return { url: url.toString(), stripped }; | |
| 114 | −}; | |
| 115 | − | |
| 116 | −/** Rewrites the URL-bearing attributes of an in-flight span in place, dropping | |
| 117 | − * sensitive query parameters. Returns the parameter names removed. */ | |
| 118 | −export const redactSpanUrlAttributes = (attributes: Record<string, unknown>): readonly string[] => { | |
| 119 | − const stripped = new Set<string>(); | |
| 120 | − for (const name of URL_ATTRIBUTES) { | |
| 121 | − const value = attributes[name]; | |
| 47 | +/** Rewrite every free-text value of a span event in place: the event name | |
| 48 | + * (Effect's log message) and each string attribute. Event attributes are not | |
| 49 | + * parseable URLs — `exception.message`, `exception.stacktrace`, and | |
| 50 | + * `effect.cause` are prose that may quote one — so they go through the text | |
| 51 | + * policy (URL query parameters stripped, then capped) rather than the URL | |
| 52 | + * parser. */ | |
| 53 | +const scrubEvent = (event: { name: string; attributes?: Record<string, unknown> }): void => { | |
| 54 | + event.name = scrubSpanText(event.name); | |
| 55 | + const attributes = event.attributes; | |
| 56 | + if (attributes === undefined) return; | |
| 57 | + for (const [key, value] of Object.entries(attributes)) { | |
| 122 | 58 | if (typeof value !== "string") continue; |
| 123 | − const result = redactUrl(value); | |
| 124 | − for (const key of result.stripped) stripped.add(key); | |
| 125 | − if (result.stripped.length > 0) attributes[name] = result.url; | |
| 126 | − } | |
| 127 | − const query = attributes[QUERY_ATTRIBUTE]; | |
| 128 | − if (typeof query === "string") { | |
| 129 | − const result = redactQuery(query); | |
| 130 | − for (const key of result.stripped) stripped.add(key); | |
| 131 | − if (result.stripped.length > 0) attributes[QUERY_ATTRIBUTE] = result.query; | |
| 59 | + attributes[key] = scrubSpanText(value); | |
| 132 | 60 | } |
| 133 | − return Array.from(stripped).sort(); | |
| 134 | 61 | }; |
| 135 | 62 | |
| 136 | −/** Wraps a span processor so every span is scrubbed of credential-bearing URL | |
| 137 | − * query parameters before the inner processor (and therefore the exporter) | |
| 138 | − * sees it. | |
| 63 | +/** Wraps a span processor so every span is scrubbed of credential-bearing | |
| 64 | + * values before the inner processor (and therefore the exporter) sees it. | |
| 139 | 65 | * |
| 140 | − * The rewrite happens in `onEnding`, the last hook the OTel SDK calls while | |
| 141 | − * the span is still mutable (`Span.end()` runs `onEnding` before setting | |
| 66 | + * Attributes are rewritten in `onEnding`, the last hook the OTel SDK calls | |
| 67 | + * while the span is still mutable (`Span.end()` runs `onEnding` before setting | |
| 142 | 68 | * `_ended`, so `setAttribute` still applies); `onEnd` receives a frozen |
| 143 | 69 | * `ReadableSpan`. `onEnding` is optional in the SpanProcessor interface, so |
| 144 | 70 | * `onEnd` re-checks the attribute bag and mutates it directly as a backstop |
| 145 | − * for any SDK path that skips the earlier hook. */ | |
| 71 | + * for any SDK path that skips the earlier hook. | |
| 72 | + * | |
| 73 | + * Events and `status.message` are scrubbed in `onEnd` only: `recordException` | |
| 74 | + * and `setStatus` both run inside `Span.end()`'s caller, so at `onEnding` time | |
| 75 | + * the events array is not yet final. `ReadableSpan` is readonly by TYPE, not | |
| 76 | + * frozen at runtime — the arrays and objects the SDK exposes are the live ones | |
| 77 | + * the exporter serializes, so mutating them here is what keeps the secret out | |
| 78 | + * of the export. */ | |
| 146 | 79 | export class UrlRedactingSpanProcessor implements SpanProcessor { |
| 147 | 80 | constructor(private readonly inner: SpanProcessor) {} |
| 148 | 81 | |
| @@ -155,15 +88,18 @@ export class UrlRedactingSpanProcessor implements SpanProcessor { · 6 unmodified lines | ||
| 155 | 88 | } |
| 156 | 89 | |
| 157 | 90 | onEnding(span: Span): void { |
| 158 | − this.redact(span.attributes, (key, value) => span.setAttribute(key, value)); | |
| 91 | + this.redactAttributes(span.attributes, (key, value) => span.setAttribute(key, value)); | |
| 159 | 92 | this.inner.onEnding?.(span); |
| 160 | 93 | } |
| 161 | 94 | |
| 162 | 95 | onEnd(span: ReadableSpan): void { |
| 163 | 96 | const attributes = span.attributes as Record<string, unknown>; |
| 164 | − this.redact(attributes, (key, value) => { | |
| 97 | + this.redactAttributes(attributes, (key, value) => { | |
| 165 | 98 | attributes[key] = value; |
| 166 | 99 | }); |
| 100 | + for (const event of span.events) scrubEvent(event); | |
| 101 | + const status = span.status as { code: number; message?: string }; | |
| 102 | + if (typeof status.message === "string") status.message = scrubSpanText(status.message); | |
| 167 | 103 | this.inner.onEnd(span); |
| 168 | 104 | } |
| 169 | 105 | |
| @@ -171,7 +107,7 @@ export class UrlRedactingSpanProcessor implements SpanProcessor { · 1 unmodified line | ||
| 171 | 107 | return this.inner.shutdown(); |
| 172 | 108 | } |
| 173 | 109 | |
| 174 | − private redact( | |
| 110 | + private redactAttributes( | |
| 175 | 111 | attributes: Record<string, unknown>, |
| 176 | 112 | write: (key: string, value: string) => void, |
| 177 | 113 | ): void { |
| @@ -180,7 +116,7 @@ export class UrlRedactingSpanProcessor implements SpanProcessor { · 2 unmodified lines | ||
| 180 | 116 | const draft: Record<string, unknown> = { ...attributes }; |
| 181 | 117 | const stripped = redactSpanUrlAttributes(draft); |
| 182 | 118 | if (stripped.length === 0) return; |
| 183 | − for (const name of [...URL_ATTRIBUTES, QUERY_ATTRIBUTE]) { | |
| 119 | + for (const name of [...SPAN_URL_ATTRIBUTES, SPAN_QUERY_ATTRIBUTE]) { | |
| 184 | 120 | const value = draft[name]; |
| 185 | 121 | if (typeof value === "string" && value !== attributes[name]) write(name, value); |
| 186 | 122 | } |
| @@ -45,6 +45,8 @@ import { ATTR_SERVICE_NAME, ATTR_SERVICE_VERSION } from "@opentelemetry/semantic · 44 unmodified lines | ||
| 45 | 45 | import { env } from "cloudflare:workers"; |
| 46 | 46 | import { Effect, Layer } from "effect"; |
| 47 | 47 | |
| 48 | +import { RedactedHeaderNamesLive } from "@executor-js/sdk/http-auth"; | |
| 49 | + | |
| 48 | 50 | import { |
| 49 | 51 | CountingSpanExporter, |
| 50 | 52 | CountingSpanProcessor, |
| @@ -129,6 +131,10 @@ export const flushTracerProvider = async (): Promise<void> => { · 78 unmodified lines | ||
| 129 | 131 | } |
| 130 | 132 | }; |
| 131 | 133 | |
| 134 | +// The widened redacted-header list travels WITH the tracer, not just with the | |
| 135 | +// app layer: the MCP worker handler and the session Durable Object each run | |
| 136 | +// their programs under this layer directly (`Effect.runPromise` from the raw | |
| 137 | +// worker entry), and those fibers open client spans too. | |
| 132 | 138 | const makeTelemetryLive = (): Layer.Layer<never> => |
| 133 | 139 | Layer.unwrap( |
| 134 | 140 | Effect.sync(() => |
| @@ -137,8 +143,9 @@ const makeTelemetryLive = (): Layer.Layer<never> => · 2 unmodified lines | ||
| 137 | 143 | Layer.provide( |
| 138 | 144 | Resource.layer({ serviceName: SERVICE_NAME, serviceVersion: SERVICE_VERSION }), |
| 139 | 145 | ), |
| 146 | + Layer.merge(RedactedHeaderNamesLive), | |
| 140 | 147 | ) |
| 141 | − : Layer.empty, | |
| 148 | + : RedactedHeaderNamesLive, | |
| 142 | 149 | ), |
| 143 | 150 | ); |
| 144 | 151 | |
| @@ -43,6 +43,7 @@ import { HttpRouter } from "effect/unstable/http"; · 42 unmodified lines | ||
| 43 | 43 | import { Effect, Layer } from "effect"; |
| 44 | 44 | |
| 45 | 45 | import type { AnyPlugin } from "@executor-js/sdk"; |
| 46 | +import { RedactedHeaderNamesLive } from "@executor-js/sdk/http-auth"; | |
| 46 | 47 | import type { DbProvider } from "./executor-fuma-db"; |
| 47 | 48 | import { HostConfig } from "./scoped-executor"; |
| 48 | 49 | import type { PluginsProvider } from "./scoped-executor"; |
| @@ -584,7 +585,16 @@ export const make = < · 535 unmodified lines | ||
| 584 | 585 | // socket ONCE at boot. It is folded into the execution-stack middleware (above) |
| 585 | 586 | // and into the account middleware + extension routes (the host self-combines |
| 586 | 587 | // those) so each rebuilds per request. `boot` is the long-lived context. |
| 587 | − const appLayer: AppRouteLayer = merged.pipe(Layer.provideMerge(options.boot)); | |
| 588 | + // | |
| 589 | + // `RedactedHeaderNamesLive` rides on `boot` so it reaches every fiber this | |
| 590 | + // host serves: `HttpMiddleware.tracer` reads `Headers.CurrentRedactedNames` | |
| 591 | + // for the inbound span, and every outbound `HttpClient` built under the same | |
| 592 | + // context reads it for the client span. Effect's default list stops at | |
| 593 | + // authorization/cookie/set-cookie/x-api-key, which is narrower than the | |
| 594 | + // header names an integration's auth placement can mint. | |
| 595 | + const appLayer: AppRouteLayer = merged.pipe( | |
| 596 | + Layer.provideMerge(Layer.merge(options.boot, RedactedHeaderNamesLive)), | |
| 597 | + ); | |
| 588 | 598 | |
| 589 | 599 | return { |
| 590 | 600 | api: protectedApi.api, |
| @@ -14,6 +14,10 @@ export { · 13 unmodified lines | ||
| 14 | 14 | type RenderedAuthPlacements, |
| 15 | 15 | } from "./auth-method"; |
| 16 | 16 | |
| 17 | +// Tracer header redaction — a placement can mint any header name, so the | |
| 18 | +// tracer's redacted-name list has to be widened past Effect's default. | |
| 19 | +export { REDACTED_HEADER_NAMES, RedactedHeaderNamesLive } from "./redacted-headers"; | |
| 20 | + | |
| 17 | 21 | // Request-shaped authoring dialect — accepted on every plugin's auth inputs, |
| 18 | 22 | // normalized to canonical placements at the boundary. |
| 19 | 23 | export { |
| @@ -0,0 +1,75 @@ | ||
| 1 | +// --------------------------------------------------------------------------- | |
| 2 | +// The tracer's redacted-header list must cover the headers THIS app mints. | |
| 3 | +// | |
| 4 | +// Effect's tracing seams stamp every header as a span attribute and wrap only | |
| 5 | +// the names in `Headers.CurrentRedactedNames`. An `AuthPlacement` names its own | |
| 6 | +// header, so the list has to cover names a spec's `secretHeaders` produced — | |
| 7 | +// not just Effect's four defaults. Values here are synthetic. | |
| 8 | +// --------------------------------------------------------------------------- | |
| 9 | + | |
| 10 | +import { describe, expect, it } from "@effect/vitest"; | |
| 11 | +import { Redacted } from "effect"; | |
| 12 | +import { Headers } from "effect/unstable/http"; | |
| 13 | + | |
| 14 | +import { REDACTED_HEADER_NAMES } from "./redacted-headers"; | |
| 15 | + | |
| 16 | +const SECRET = "synthetic-credential-value"; | |
| 17 | + | |
| 18 | +const redactedNames = (headers: Record<string, string>): readonly string[] => { | |
| 19 | + const redacted = Headers.redact(Headers.fromRecordUnsafe(headers), REDACTED_HEADER_NAMES); | |
| 20 | + return Object.entries(redacted) | |
| 21 | + .filter(([, value]) => Redacted.isRedacted(value)) | |
| 22 | + .map(([name]) => name) | |
| 23 | + .sort(); | |
| 24 | +}; | |
| 25 | + | |
| 26 | +describe("REDACTED_HEADER_NAMES", () => { | |
| 27 | + it("covers the header names an auth placement mints", () => { | |
| 28 | + // Every name here is reachable from a real placement: `authoring.ts` | |
| 29 | + // accepts an arbitrary header name, and `derive-auth.ts` builds placements | |
| 30 | + // straight from a spec's `secretHeaders`. | |
| 31 | + const minted = { | |
| 32 | + authorization: `Bearer ${SECRET}`, | |
| 33 | + "x-api-key": SECRET, | |
| 34 | + "api-key": SECRET, | |
| 35 | + "x-figma-token": SECRET, | |
| 36 | + "dd-api-key": SECRET, | |
| 37 | + "private-token": SECRET, | |
| 38 | + "x-auth-token": SECRET, | |
| 39 | + "x-access-token": SECRET, | |
| 40 | + "x-session-token": SECRET, | |
| 41 | + "x-hub-signature": SECRET, | |
| 42 | + "client-secret": SECRET, | |
| 43 | + }; | |
| 44 | + | |
| 45 | + expect(redactedNames(minted)).toEqual(Object.keys(minted).sort()); | |
| 46 | + }); | |
| 47 | + | |
| 48 | + it("covers Effect's own defaults plus the standard auth headers it omits", () => { | |
| 49 | + expect( | |
| 50 | + redactedNames({ | |
| 51 | + authorization: `Bearer ${SECRET}`, | |
| 52 | + cookie: `wos-session=${SECRET}`, | |
| 53 | + "set-cookie": `wos-session=${SECRET}`, | |
| 54 | + "proxy-authorization": `Basic ${SECRET}`, | |
| 55 | + "www-authenticate": `Bearer realm="executor", error="invalid_token"`, | |
| 56 | + }), | |
| 57 | + ).toEqual(["authorization", "cookie", "proxy-authorization", "set-cookie", "www-authenticate"]); | |
| 58 | + }); | |
| 59 | + | |
| 60 | + it("leaves ordinary headers stamped in plaintext", () => { | |
| 61 | + // Over-matching is cheap (a redacted attribute still shows the header was | |
| 62 | + // present) but it must not swallow the headers that make a trace useful. | |
| 63 | + expect( | |
| 64 | + redactedNames({ | |
| 65 | + "content-type": "application/json", | |
| 66 | + "user-agent": "executor/1.0", | |
| 67 | + "x-request-id": "synthetic-request-id", | |
| 68 | + accept: "application/json", | |
| 69 | + "cache-control": "no-cache", | |
| 70 | + "x-executor-organization": "acme", | |
| 71 | + traceparent: "00-0af7651916cd43dd8448eb211c80319c-b7ad6b7169203331-01", | |
| 72 | + }), | |
| 73 | + ).toEqual([]); | |
| 74 | + }); | |
| 75 | +}); | |
| @@ -0,0 +1,46 @@ | ||
| 1 | +// --------------------------------------------------------------------------- | |
| 2 | +// Which header names a tracer must never stamp verbatim. | |
| 3 | +// | |
| 4 | +// Effect's tracing seams (`HttpMiddleware.tracer` inbound, `HttpClient` | |
| 5 | +// outbound) stamp EVERY header as a span attribute and wrap only the names in | |
| 6 | +// `Headers.CurrentRedactedNames` — whose default is just authorization, cookie, | |
| 7 | +// set-cookie, and x-api-key. This app mints credential headers well outside | |
| 8 | +// that list: an `AuthPlacement` names its own header (`authoring.ts` / | |
| 9 | +// `legacy.ts`), so a connection's key rides on whatever the integration | |
| 10 | +// declared — `X-Figma-Token`, `DD-API-KEY`, `Private-Token`, anything a spec's | |
| 11 | +// `secretHeaders` produced. Left at the default, every one of those was stamped | |
| 12 | +// in plaintext on the `http.client` span of every tool invocation. | |
| 13 | +// | |
| 14 | +// Placement names are user-supplied, so they cannot be enumerated. They are | |
| 15 | +// matched by SHAPE instead, and the match is deliberately loose: a redacted | |
| 16 | +// span attribute still records that the header was present (its value renders | |
| 17 | +// as `<redacted>`), so over-matching costs nothing, while under-matching ships | |
| 18 | +// a credential. | |
| 19 | +// --------------------------------------------------------------------------- | |
| 20 | + | |
| 21 | +import { Headers } from "effect/unstable/http"; | |
| 22 | +import { Layer } from "effect"; | |
| 23 | + | |
| 24 | +/** Credential vocabulary, matched against a whole header name segment so | |
| 25 | + * `x-request-id` and `content-type` are untouched while `api-key`, | |
| 26 | + * `x-figma-token`, and `private-token` are not. */ | |
| 27 | +const CREDENTIAL_HEADER_SHAPE = | |
| 28 | + /(^|-)(api[-_]?key|access[-_]?key|auth|authentication|auth[-_]?token|access[-_]?token|credential|key|password|pat|secret|session|signature|token)(-|$)/i; | |
| 29 | + | |
| 30 | +/** The names this app's tracers redact: Effect's defaults, the standard auth | |
| 31 | + * headers it omits, and the credential shape an `AuthPlacement` can mint. */ | |
| 32 | +export const REDACTED_HEADER_NAMES: readonly (string | RegExp)[] = [ | |
| 33 | + "authorization", | |
| 34 | + "cookie", | |
| 35 | + "set-cookie", | |
| 36 | + "proxy-authorization", | |
| 37 | + "www-authenticate", | |
| 38 | + CREDENTIAL_HEADER_SHAPE, | |
| 39 | +]; | |
| 40 | + | |
| 41 | +/** Provide the widened list wherever an HTTP layer is assembled — inbound | |
| 42 | + * server middleware and outbound clients alike. Both read the same reference, | |
| 43 | + * so one layer covers a surface's spans in both directions. */ | |
| 44 | +export const RedactedHeaderNamesLive: Layer.Layer<never> = Layer.succeed( | |
| 45 | + Headers.CurrentRedactedNames, | |
| 46 | +)(REDACTED_HEADER_NAMES); | |
| @@ -179,3 +179,22 @@ export { · 178 unmodified lines | ||
| 179 | 179 | type OAuthPopupResult, |
| 180 | 180 | isOAuthPopupResult, |
| 181 | 181 | } from "./oauth-popup-types"; |
| 182 | + | |
| 183 | +// Span-value redaction. Browser-safe on purpose: the web client's OTLP | |
| 184 | +// exporter and the Workers isolates' OTel span processor apply the SAME | |
| 185 | +// policy, so it cannot live in either isolate. | |
| 186 | +export { | |
| 187 | + MAX_SPAN_TEXT_CHARS, | |
| 188 | + SPAN_QUERY_ATTRIBUTE, | |
| 189 | + SPAN_URL_ATTRIBUTES, | |
| 190 | + STRIPPED_QUERY_ATTRIBUTE, | |
| 191 | + redactQuery, | |
| 192 | + redactSensitiveKeyValuesInText, | |
| 193 | + redactSpanUrlAttribute, | |
| 194 | + redactSpanUrlAttributes, | |
| 195 | + redactUrl, | |
| 196 | + redactUrlQueryInText, | |
| 197 | + scrubSpanText, | |
| 198 | + truncateSpanText, | |
| 199 | + type SpanRedaction, | |
| 200 | +} from "./span-redaction"; | |
| @@ -0,0 +1,170 @@ | ||
| 1 | +import { describe, expect, it } from "@effect/vitest"; | |
| 2 | + | |
| 3 | +import { | |
| 4 | + MAX_SPAN_TEXT_CHARS, | |
| 5 | + STRIPPED_QUERY_ATTRIBUTE, | |
| 6 | + redactSensitiveKeyValuesInText, | |
| 7 | + redactSpanUrlAttribute, | |
| 8 | + redactSpanUrlAttributes, | |
| 9 | + redactUrlQueryInText, | |
| 10 | + scrubSpanText, | |
| 11 | + truncateSpanText, | |
| 12 | +} from "./span-redaction"; | |
| 13 | + | |
| 14 | +// Synthetic placeholders only — never a real authorization code or state. | |
| 15 | +const CODE = "synthetic-authorization-code"; | |
| 16 | +const STATE = "synthetic-csrf-state"; | |
| 17 | +const TOKEN = "synthetic-endpoint-token"; | |
| 18 | + | |
| 19 | +describe("redactSpanUrlAttributes", () => { | |
| 20 | + it("strips the authorization code and state from url.full and url.query", () => { | |
| 21 | + const attributes: Record<string, unknown> = { | |
| 22 | + "url.full": `https://app.test/api/oauth/callback?code=${CODE}&state=${STATE}&domain=example.test`, | |
| 23 | + "url.query": `code=${CODE}&state=${STATE}&domain=example.test`, | |
| 24 | + "url.path": "/api/oauth/callback", | |
| 25 | + }; | |
| 26 | + | |
| 27 | + expect(redactSpanUrlAttributes(attributes)).toEqual(["code", "state"]); | |
| 28 | + expect(JSON.stringify(attributes)).not.toContain(CODE); | |
| 29 | + expect(JSON.stringify(attributes)).not.toContain(STATE); | |
| 30 | + // Route-level visibility is preserved. | |
| 31 | + expect(attributes["url.path"]).toBe("/api/oauth/callback"); | |
| 32 | + expect(attributes["url.full"]).toBe("https://app.test/api/oauth/callback?domain=example.test"); | |
| 33 | + }); | |
| 34 | + | |
| 35 | + it("strips a code nested inside the login redirect's returnTo parameter", () => { | |
| 36 | + const returnTo = encodeURIComponent(`/api/oauth/callback?code=${CODE}&state=${STATE}`); | |
| 37 | + const attributes: Record<string, unknown> = { | |
| 38 | + "url.full": `https://app.test/login?returnTo=${returnTo}`, | |
| 39 | + }; | |
| 40 | + | |
| 41 | + expect(redactSpanUrlAttributes(attributes)).toEqual(["returnTo.code", "returnTo.state"]); | |
| 42 | + expect(JSON.stringify(attributes)).not.toContain(CODE); | |
| 43 | + expect(String(attributes["url.full"])).toContain("%2Fapi%2Foauth%2Fcallback"); | |
| 44 | + }); | |
| 45 | + | |
| 46 | + it("leaves a span with no sensitive parameters untouched", () => { | |
| 47 | + const attributes: Record<string, unknown> = { "url.query": "owner=org" }; | |
| 48 | + expect(redactSpanUrlAttributes(attributes)).toEqual([]); | |
| 49 | + expect(attributes["url.query"]).toBe("owner=org"); | |
| 50 | + }); | |
| 51 | +}); | |
| 52 | + | |
| 53 | +describe("redactSpanUrlAttribute", () => { | |
| 54 | + it("returns null for an attribute that is not URL-bearing", () => { | |
| 55 | + expect(redactSpanUrlAttribute("db.statement", `select ${CODE}`)).toBeNull(); | |
| 56 | + expect(redactSpanUrlAttribute(STRIPPED_QUERY_ATTRIBUTE, "code")).toBeNull(); | |
| 57 | + }); | |
| 58 | + | |
| 59 | + it("returns null for a URL attribute carrying nothing sensitive", () => { | |
| 60 | + expect(redactSpanUrlAttribute("url.full", "https://app.test/api?owner=org")).toBeNull(); | |
| 61 | + }); | |
| 62 | + | |
| 63 | + it("returns the scrubbed value and the stripped keys", () => { | |
| 64 | + expect(redactSpanUrlAttribute("url.full", `https://mcp.test/mcp?token=${TOKEN}`)).toEqual({ | |
| 65 | + value: "https://mcp.test/mcp", | |
| 66 | + stripped: ["token"], | |
| 67 | + }); | |
| 68 | + }); | |
| 69 | + | |
| 70 | + it("ignores a non-string value", () => { | |
| 71 | + expect(redactSpanUrlAttribute("url.full", 42)).toBeNull(); | |
| 72 | + }); | |
| 73 | +}); | |
| 74 | + | |
| 75 | +describe("redactUrlQueryInText", () => { | |
| 76 | + it("strips a credential from a URL quoted inside a message", () => { | |
| 77 | + const message = `GET https://mcp.test/mcp?token=${TOKEN}&team=acme failed with 401`; | |
| 78 | + const scrubbed = redactUrlQueryInText(message); | |
| 79 | + | |
| 80 | + expect(scrubbed).not.toContain(TOKEN); | |
| 81 | + expect(scrubbed).toBe("GET https://mcp.test/mcp?team=acme failed with 401"); | |
| 82 | + }); | |
| 83 | + | |
| 84 | + it("strips a credential from a bare path with a query string", () => { | |
| 85 | + expect(redactUrlQueryInText(`redirected to /oauth/callback?code=${CODE} and stopped`)).toBe( | |
| 86 | + "redirected to /oauth/callback and stopped", | |
| 87 | + ); | |
| 88 | + }); | |
| 89 | + | |
| 90 | + it("leaves text with no URL untouched", () => { | |
| 91 | + expect(redactUrlQueryInText("the upstream rejected the request")).toBe( | |
| 92 | + "the upstream rejected the request", | |
| 93 | + ); | |
| 94 | + }); | |
| 95 | + | |
| 96 | + it("leaves a URL with no query string untouched", () => { | |
| 97 | + expect(redactUrlQueryInText("GET https://api.test/v1/things failed")).toBe( | |
| 98 | + "GET https://api.test/v1/things failed", | |
| 99 | + ); | |
| 100 | + }); | |
| 101 | + | |
| 102 | + it("scrubs every URL in a multi-line pretty cause", () => { | |
| 103 | + const pretty = [ | |
| 104 | + `RequestError: POST https://api.test/token?code=${CODE}`, | |
| 105 | + ` at fetch (https://app.test/assets/index.js?state=${STATE})`, | |
| 106 | + ].join("\n"); | |
| 107 | + | |
| 108 | + const scrubbed = redactUrlQueryInText(pretty); | |
| 109 | + | |
| 110 | + expect(scrubbed).not.toContain(CODE); | |
| 111 | + expect(scrubbed).not.toContain(STATE); | |
| 112 | + expect(scrubbed).toContain("https://api.test/token"); | |
| 113 | + }); | |
| 114 | +}); | |
| 115 | + | |
| 116 | +describe("redactSensitiveKeyValuesInText", () => { | |
| 117 | + it("replaces a credential-named value in serialized JSON", () => { | |
| 118 | + // The exact shape Effect's tracer logger produces for a logged cause: the | |
| 119 | + // tagged error is serialized INTO the event name, so no URL is involved. | |
| 120 | + const logged = `[\n "OAuth callback completion failed",\n {\n "state": "${STATE}",\n "_tag": "OAuthSessionNotFoundError"\n }\n]`; | |
| 121 | + const scrubbed = redactSensitiveKeyValuesInText(logged); | |
| 122 | + | |
| 123 | + expect(scrubbed).not.toContain(STATE); | |
| 124 | + expect(scrubbed).toContain('"state": "[redacted]"'); | |
| 125 | + // The part that makes the failure diagnosable survives. | |
| 126 | + expect(scrubbed).toContain('"_tag": "OAuthSessionNotFoundError"'); | |
| 127 | + expect(scrubbed).toContain("OAuth callback completion failed"); | |
| 128 | + }); | |
| 129 | + | |
| 130 | + it("replaces unquoted and single-quoted values", () => { | |
| 131 | + expect(redactSensitiveKeyValuesInText(`code=${CODE} status=400`)).toBe( | |
| 132 | + "code=[redacted] status=400", | |
| 133 | + ); | |
| 134 | + expect(redactSensitiveKeyValuesInText(`token: '${TOKEN}'`)).toBe("token: '[redacted]'"); | |
| 135 | + }); | |
| 136 | + | |
| 137 | + it("keeps a numeric value under a credential name", () => { | |
| 138 | + // `code: 404` is the readable part of a failure message, not a grant. | |
| 139 | + expect(redactSensitiveKeyValuesInText("failed with code: 404")).toBe("failed with code: 404"); | |
| 140 | + }); | |
| 141 | + | |
| 142 | + it("leaves non-credential keys alone", () => { | |
| 143 | + expect( | |
| 144 | + redactSensitiveKeyValuesInText('{"slug": "acme", "status": "failed", "encoding": "utf-8"}'), | |
| 145 | + ).toBe('{"slug": "acme", "status": "failed", "encoding": "utf-8"}'); | |
| 146 | + }); | |
| 147 | +}); | |
| 148 | + | |
| 149 | +describe("truncateSpanText", () => { | |
| 150 | + it("caps text past the limit and records how much went", () => { | |
| 151 | + const capped = truncateSpanText("x".repeat(MAX_SPAN_TEXT_CHARS + 100)); | |
| 152 | + expect(capped.length).toBeLessThan(MAX_SPAN_TEXT_CHARS + 100); | |
| 153 | + expect(capped).toContain("truncated 100 chars"); | |
| 154 | + }); | |
| 155 | + | |
| 156 | + it("leaves text within the limit exactly as-is", () => { | |
| 157 | + expect(truncateSpanText("short")).toBe("short"); | |
| 158 | + }); | |
| 159 | +}); | |
| 160 | + | |
| 161 | +describe("scrubSpanText", () => { | |
| 162 | + it("applies both policies: the credential goes and the tail is capped", () => { | |
| 163 | + const stack = `Error: POST https://api.test/token?code=${CODE}\n${"frame\n".repeat(3_000)}`; | |
| 164 | + const scrubbed = scrubSpanText(stack); | |
| 165 | + | |
| 166 | + expect(scrubbed).not.toContain(CODE); | |
| 167 | + expect(scrubbed).toContain("truncated"); | |
| 168 | + expect(scrubbed.length).toBeLessThan(stack.length); | |
| 169 | + }); | |
| 170 | +}); | |
| @@ -0,0 +1,217 @@ | ||
| 1 | +// --------------------------------------------------------------------------- | |
| 2 | +// Span-value redaction — the pure core every export seam shares. | |
| 3 | +// | |
| 4 | +// Two classes of value reach a trace backend carrying credentials, and neither | |
| 5 | +// is fixable by wrapping the secret in `Redacted`: the framework stamps them | |
| 6 | +// itself, from values that are already plain strings by the time it sees them. | |
| 7 | +// | |
| 8 | +// 1. URL attributes. Effect's `HttpMiddleware.tracer` and `HttpClient` stamp | |
| 9 | +// `url.full` / `url.query` unconditionally (they redact URL userinfo and | |
| 10 | +// configured header names, and nothing else), so an OAuth callback's | |
| 11 | +// `?code=…&state=…` and a user-supplied `?token=…` endpoint ride out with | |
| 12 | +// the span. | |
| 13 | +// 2. Free text. Effect's tracer logger puts the already-interpolated message | |
| 14 | +// in the event name and the whole `effect.cause` in an event attribute; | |
| 15 | +// the OTel bridge's `recordException` does the same for `exception.message` | |
| 16 | +// and `exception.stacktrace`. Any URL those quote comes with its query | |
| 17 | +// string attached. | |
| 18 | +// | |
| 19 | +// This module owns the decisions — which parameters are sensitive, how deep to | |
| 20 | +// follow nesting, how much stack text is worth keeping — so the isolate-specific | |
| 21 | +// wrappers (an OTel `SpanProcessor` in the Workers isolates, an OTLP | |
| 22 | +// serialization wrapper in the browser) stay thin and cannot drift apart. | |
| 23 | +// --------------------------------------------------------------------------- | |
| 24 | + | |
| 25 | +/** Query parameters that are credentials, single-use grants, or CSRF secrets. | |
| 26 | + * Matched case-insensitively against the parameter name. */ | |
| 27 | +const SENSITIVE_QUERY_KEYS: ReadonlySet<string> = new Set([ | |
| 28 | + "access_token", | |
| 29 | + "client_secret", | |
| 30 | + "code", | |
| 31 | + "code_verifier", | |
| 32 | + "error_description", | |
| 33 | + "id_token", | |
| 34 | + "refresh_token", | |
| 35 | + "session_state", | |
| 36 | + "state", | |
| 37 | + "token", | |
| 38 | +]); | |
| 39 | + | |
| 40 | +/** Span attributes whose value is a whole URL. */ | |
| 41 | +export const SPAN_URL_ATTRIBUTES = ["url.full", "http.url"] as const; | |
| 42 | +/** The span attribute holding a bare query string. */ | |
| 43 | +export const SPAN_QUERY_ATTRIBUTE = "url.query"; | |
| 44 | +/** Names of the parameters removed from this span's URL attributes. Non-secret | |
| 45 | + * by construction — it is the key list, never the values. */ | |
| 46 | +export const STRIPPED_QUERY_ATTRIBUTE = "url.query.stripped_keys"; | |
| 47 | + | |
| 48 | +const isSensitive = (key: string): boolean => SENSITIVE_QUERY_KEYS.has(key.toLowerCase()); | |
| 49 | + | |
| 50 | +/** How deep to follow a query parameter whose value is itself a URL or path | |
| 51 | + * with a query string. Depth 2 covers the real nesting in this app — | |
| 52 | + * `/login?returnTo=%2Fapi%2Foauth%2Fcallback%3Fcode%3D…` — with headroom, and | |
| 53 | + * bounds the work per span. */ | |
| 54 | +const MAX_NESTED_DEPTH = 2; | |
| 55 | + | |
| 56 | +/** A scrubbed value plus the names of the parameters that were removed. */ | |
| 57 | +export interface SpanRedaction { | |
| 58 | + readonly value: string; | |
| 59 | + readonly stripped: readonly string[]; | |
| 60 | +} | |
| 61 | + | |
| 62 | +/** The query string with every sensitive parameter dropped, plus the names | |
| 63 | + * dropped. Parsed with `URLSearchParams` so encoding round-trips correctly. | |
| 64 | + * | |
| 65 | + * A parameter whose own value carries a nested query string is redacted | |
| 66 | + * recursively rather than dropped: the login redirect round-trips the whole | |
| 67 | + * OAuth callback path through `returnTo`, so the credential rides inside | |
| 68 | + * another parameter's value and a top-level key check alone would miss it. */ | |
| 69 | +export const redactQuery = (query: string, depth = 0): SpanRedaction => { | |
| 70 | + const params = new URLSearchParams(query); | |
| 71 | + const stripped = new Set<string>(); | |
| 72 | + for (const key of Array.from(new Set(params.keys()))) { | |
| 73 | + if (isSensitive(key)) { | |
| 74 | + stripped.add(key); | |
| 75 | + params.delete(key); | |
| 76 | + continue; | |
| 77 | + } | |
| 78 | + if (depth >= MAX_NESTED_DEPTH) continue; | |
| 79 | + const values = params.getAll(key); | |
| 80 | + const rewritten = values.map((value) => { | |
| 81 | + const separator = value.indexOf("?"); | |
| 82 | + if (separator === -1) return value; | |
| 83 | + const nested = redactQuery(value.slice(separator + 1), depth + 1); | |
| 84 | + for (const name of nested.stripped) stripped.add(`${key}.${name}`); | |
| 85 | + return nested.stripped.length === 0 | |
| 86 | + ? value | |
| 87 | + : `${value.slice(0, separator)}${nested.value === "" ? "" : `?${nested.value}`}`; | |
| 88 | + }); | |
| 89 | + if (rewritten.every((value, index) => value === values[index])) continue; | |
| 90 | + params.delete(key); | |
| 91 | + for (const value of rewritten) params.append(key, value); | |
| 92 | + } | |
| 93 | + return stripped.size === 0 | |
| 94 | + ? { value: query, stripped: [] } | |
| 95 | + : { value: params.toString(), stripped: Array.from(stripped).sort() }; | |
| 96 | +}; | |
| 97 | + | |
| 98 | +/** The URL with every sensitive query parameter dropped. Path, host, and | |
| 99 | + * scheme are untouched. Unparseable values are treated as a bare query string | |
| 100 | + * and dropped entirely rather than passed through — if it cannot be parsed it | |
| 101 | + * cannot be proven safe. */ | |
| 102 | +export const redactUrl = (url: string): SpanRedaction => { | |
| 103 | + if (!URL.canParse(url)) return redactQuery(url); | |
| 104 | + const parsed = new URL(url); | |
| 105 | + if (parsed.search === "") return { value: url, stripped: [] }; | |
| 106 | + const query = redactQuery(parsed.search.slice(1)); | |
| 107 | + if (query.stripped.length === 0) return { value: url, stripped: [] }; | |
| 108 | + parsed.search = query.value; | |
| 109 | + return { value: parsed.toString(), stripped: query.stripped }; | |
| 110 | +}; | |
| 111 | + | |
| 112 | +/** Scrub one span attribute by name. Returns `null` when the attribute is not | |
| 113 | + * URL-bearing, is not a string, or carried nothing sensitive — so a non-null | |
| 114 | + * result always means "rewrite this value". */ | |
| 115 | +export const redactSpanUrlAttribute = (name: string, value: unknown): SpanRedaction | null => { | |
| 116 | + if (typeof value !== "string") return null; | |
| 117 | + const isUrl = (SPAN_URL_ATTRIBUTES as readonly string[]).includes(name); | |
| 118 | + if (!isUrl && name !== SPAN_QUERY_ATTRIBUTE) return null; | |
| 119 | + const result = isUrl ? redactUrl(value) : redactQuery(value); | |
| 120 | + return result.stripped.length === 0 ? null : result; | |
| 121 | +}; | |
| 122 | + | |
| 123 | +/** Rewrites the URL-bearing attributes of a span's attribute bag in place, | |
| 124 | + * dropping sensitive query parameters. Returns the parameter names removed. */ | |
| 125 | +export const redactSpanUrlAttributes = (attributes: Record<string, unknown>): readonly string[] => { | |
| 126 | + const stripped = new Set<string>(); | |
| 127 | + for (const name of [...SPAN_URL_ATTRIBUTES, SPAN_QUERY_ATTRIBUTE]) { | |
| 128 | + const result = redactSpanUrlAttribute(name, attributes[name]); | |
| 129 | + if (result === null) continue; | |
| 130 | + for (const key of result.stripped) stripped.add(key); | |
| 131 | + attributes[name] = result.value; | |
| 132 | + } | |
| 133 | + return Array.from(stripped).sort(); | |
| 134 | +}; | |
| 135 | + | |
| 136 | +// --------------------------------------------------------------------------- | |
| 137 | +// Free text — event names, exception messages, pretty-printed causes | |
| 138 | +// --------------------------------------------------------------------------- | |
| 139 | + | |
| 140 | +/** A URL-shaped substring that HAS a query string: an absolute URL or an | |
| 141 | + * absolute path, up to the first `?`, then the query up to the next | |
| 142 | + * whitespace or closing delimiter. Requiring the `?` is the point — a URL with | |
| 143 | + * no query string has nothing to strip, so it is left alone. */ | |
| 144 | +const URL_WITH_QUERY = /(?:[a-z][a-z0-9+.-]*:\/\/|\/)[^\s"'<>)\]}\\?]*\?[^\s"'<>)\]}\\]*/gi; | |
| 145 | + | |
| 146 | +/** Drop sensitive query parameters from every URL-shaped substring of free | |
| 147 | + * text (a log message, an exception message, a pretty-printed cause). */ | |
| 148 | +export const redactUrlQueryInText = (text: string): string => | |
| 149 | + text.replace(URL_WITH_QUERY, (match) => { | |
| 150 | + const separator = match.indexOf("?"); | |
| 151 | + const { value, stripped } = redactQuery(match.slice(separator + 1)); | |
| 152 | + if (stripped.length === 0) return match; | |
| 153 | + return value === "" ? match.slice(0, separator) : `${match.slice(0, separator)}?${value}`; | |
| 154 | + }); | |
| 155 | + | |
| 156 | +/** The marker left where a credential-named value was removed from free text. | |
| 157 | + * Deliberately not `Redacted`'s own "<redacted>" rendering, so a scrubbed span | |
| 158 | + * value is never mistaken for a value serialized while still wrapped. */ | |
| 159 | +const TEXT_SCRUB_MARKER = "[redacted]"; | |
| 160 | + | |
| 161 | +/** A `<name><separator><value>` pair, where the separator is `=` or `:` with | |
| 162 | + * optional quoting and whitespace around either side. Covers the three shapes | |
| 163 | + * free text actually arrives in: the JSON a serialized error renders to | |
| 164 | + * (`"state": "…"`), form-encoded prose (`code=…`), and hand-written messages | |
| 165 | + * (`token: …`). The value runs to the closing quote, or to the next | |
| 166 | + * whitespace/delimiter when unquoted. */ | |
| 167 | +const KEY_VALUE_PAIR = | |
| 168 | + /(["']?)([a-z_][a-z0-9_]*)\1(\s*[=:]\s*)(?:"([^"]*)"|'([^']*)'|([^\s,;)\]}]+))/gi; | |
| 169 | + | |
| 170 | +/** A value that is purely numeric. Kept verbatim: `code: 404` and | |
| 171 | + * `status_code=200` are the readable part of a failure message, and a bare | |
| 172 | + * integer is not a credential. */ | |
| 173 | +const NUMERIC_VALUE = /^\d+$/; | |
| 174 | + | |
| 175 | +/** Replace the value of any credential-named key in free text. | |
| 176 | + * | |
| 177 | + * This is the second half of the free-text backstop, and it exists because the | |
| 178 | + * first half is not enough: Effect's tracer logger serializes a logged CAUSE | |
| 179 | + * into the event name, so a tagged error's `state` / `code` / `token` field | |
| 180 | + * reaches the exporter as JSON inside a string — no URL involved, nothing for | |
| 181 | + * `Redacted` to have wrapped, and no chance for the URL scrub to see it. | |
| 182 | + * | |
| 183 | + * Matching is by KEY NAME, using the same sensitive set as the query scrub, so | |
| 184 | + * it cannot depend on recognizing a secret by shape. A non-credential value | |
| 185 | + * that happens to sit under one of those names is lost from the trace; that is | |
| 186 | + * the accepted cost of the backstop, bounded by keeping numeric values. */ | |
| 187 | +export const redactSensitiveKeyValuesInText = (text: string): string => | |
| 188 | + text.replace(KEY_VALUE_PAIR, (match, quote, key, separator, doubleQuoted, singleQuoted, bare) => { | |
| 189 | + if (!isSensitive(key)) return match; | |
| 190 | + const value = doubleQuoted ?? singleQuoted ?? bare ?? ""; | |
| 191 | + if (value === "" || NUMERIC_VALUE.test(value)) return match; | |
| 192 | + const rendered = | |
| 193 | + doubleQuoted !== undefined | |
| 194 | + ? `"${TEXT_SCRUB_MARKER}"` | |
| 195 | + : singleQuoted !== undefined | |
| 196 | + ? `'${TEXT_SCRUB_MARKER}'` | |
| 197 | + : TEXT_SCRUB_MARKER; | |
| 198 | + return `${quote}${key}${quote}${separator}${rendered}`; | |
| 199 | + }); | |
| 200 | + | |
| 201 | +/** The cap applied to free-text span values. Stack traces and pretty-printed | |
| 202 | + * causes are unbounded — a deep Effect cause runs to tens of kilobytes — and | |
| 203 | + * the tail is where framework frames, echoed request bodies, and quoted | |
| 204 | + * upstream responses live. Truncating is the policy rather than parsing: the | |
| 205 | + * head carries the failure, the tail carries the risk. */ | |
| 206 | +export const MAX_SPAN_TEXT_CHARS = 8_000; | |
| 207 | + | |
| 208 | +/** Cap free text, recording how much was dropped. */ | |
| 209 | +export const truncateSpanText = (text: string, maxChars = MAX_SPAN_TEXT_CHARS): string => | |
| 210 | + text.length <= maxChars | |
| 211 | + ? text | |
| 212 | + : `${text.slice(0, maxChars)}\n…[truncated ${text.length - maxChars} chars]`; | |
| 213 | + | |
| 214 | +/** The whole policy for a free-text value leaving the process on a span: URL | |
| 215 | + * query parameters stripped, credential-named values replaced, then capped. */ | |
| 216 | +export const scrubSpanText = (text: string, maxChars = MAX_SPAN_TEXT_CHARS): string => | |
| 217 | + truncateSpanText(redactSensitiveKeyValuesInText(redactUrlQueryInText(text)), maxChars); | |
| @@ -0,0 +1,59 @@ | ||
| 1 | +// --------------------------------------------------------------------------- | |
| 2 | +// `recordCauseOnSpan` stamps the failing cause as plain span ATTRIBUTES, not as | |
| 3 | +// an exception event, so neither the export seam's URL scrubber (which only | |
| 4 | +// looks at URL attributes) nor its event scrub reaches them. It runs the text | |
| 5 | +// through the shared scrub itself. | |
| 6 | +// --------------------------------------------------------------------------- | |
| 7 | + | |
| 8 | +import { describe, expect, it } from "@effect/vitest"; | |
| 9 | +import { Cause } from "effect"; | |
| 10 | + | |
| 11 | +import { causeSpanAttributes } from "./agent-session-durable-object"; | |
| 12 | + | |
| 13 | +// Synthetic placeholders only. | |
| 14 | +const QUERY_TOKEN = "synthetic-endpoint-token"; | |
| 15 | +const CODE = "synthetic-authorization-code"; | |
| 16 | + | |
| 17 | +describe("causeSpanAttributes", () => { | |
| 18 | + it("scrubs a credential out of the message and the pretty cause", () => { | |
| 19 | + const attributes = causeSpanAttributes( | |
| 20 | + Cause.fail( | |
| 21 | + // oxlint-disable-next-line executor/no-error-constructor -- the failure text is what is under test; a tagged error would hide it | |
| 22 | + new Error(`POST https://mcp.test/mcp?token=${QUERY_TOKEN} failed with 401`), | |
| 23 | + ), | |
| 24 | + ); | |
| 25 | + | |
| 26 | + expect(attributes).not.toBeNull(); | |
| 27 | + expect(attributes?.["exception.message"]).not.toContain(QUERY_TOKEN); | |
| 28 | + expect(attributes?.["exception.stacktrace"]).not.toContain(QUERY_TOKEN); | |
| 29 | + // The diagnosable part survives. | |
| 30 | + expect(attributes?.["exception.message"]).toContain("https://mcp.test/mcp"); | |
| 31 | + expect(attributes?.["exception.message"]).toContain("failed with 401"); | |
| 32 | + expect(attributes?.["exception.type"]).toBe("Error"); | |
| 33 | + }); | |
| 34 | + | |
| 35 | + it("scrubs a credential-named field of a serialized failure", () => { | |
| 36 | + const attributes = causeSpanAttributes( | |
| 37 | + // oxlint-disable-next-line executor/no-error-constructor -- the failure text is what is under test | |
| 38 | + Cause.fail(new Error(`callback rejected {"code": "${CODE}", "_tag": "OAuthError"}`)), | |
| 39 | + ); | |
| 40 | + | |
| 41 | + expect(attributes?.["exception.message"]).not.toContain(CODE); | |
| 42 | + expect(attributes?.["exception.message"]).toContain('"_tag": "OAuthError"'); | |
| 43 | + }); | |
| 44 | + | |
| 45 | + it("caps an unbounded pretty cause", () => { | |
| 46 | + // oxlint-disable-next-line executor/no-error-constructor -- a deep synthetic stack is the point | |
| 47 | + const deep = new Error("deep failure"); | |
| 48 | + deep.stack = `Error: deep failure\n${" at frame (do.js:1:1)\n".repeat(2_000)}`; | |
| 49 | + | |
| 50 | + const stacktrace = causeSpanAttributes(Cause.fail(deep))?.["exception.stacktrace"] ?? ""; | |
| 51 | + | |
| 52 | + expect(stacktrace).toContain("truncated"); | |
| 53 | + expect(stacktrace.length).toBeLessThan(10_000); | |
| 54 | + }); | |
| 55 | + | |
| 56 | + it("returns null for a cause with no errors", () => { | |
| 57 | + expect(causeSpanAttributes(Cause.empty)).toBeNull(); | |
| 58 | + }); | |
| 59 | +}); | |
| @@ -19,6 +19,7 @@ import { · 18 unmodified lines | ||
| 19 | 19 | type ResumeFallbackOutcome, |
| 20 | 20 | } from "@executor-js/host-mcp/tool-server"; |
| 21 | 21 | import { defaultMcpResource, type McpResource } from "@executor-js/host-mcp"; |
| 22 | +import { scrubSpanText } from "@executor-js/sdk/shared"; | |
| 22 | 23 | |
| 23 | 24 | import type { IncomingPropagationHeaders, McpElicitationMode } from "./do-headers"; |
| 24 | 25 | import type { |
| @@ -36,6 +37,26 @@ import { · 11 unmodified lines | ||
| 36 | 37 | |
| 37 | 38 | export type IncomingTraceHeaders = IncomingPropagationHeaders; |
| 38 | 39 | |
| 40 | +/** The `exception.*` attributes for a failing cause, or `null` when the cause | |
| 41 | + * carries no error. | |
| 42 | + * | |
| 43 | + * These land as plain span ATTRIBUTES, not as exception events and not as URL | |
| 44 | + * attributes, so neither the URL scrubber nor the event scrub at the export | |
| 45 | + * seam reaches them: the message and the pretty cause are free text that may | |
| 46 | + * quote an upstream request URL with its query string, and the pretty cause is | |
| 47 | + * unbounded. `scrubSpanText` is the same policy the export seam applies to | |
| 48 | + * event text. */ | |
| 49 | +export const causeSpanAttributes = (cause: Cause.Cause<unknown>): Record<string, string> | null => { | |
| 50 | + const errors = Cause.prettyErrors(cause); | |
| 51 | + if (errors.length === 0) return null; | |
| 52 | + const first = errors[0]; | |
| 53 | + return { | |
| 54 | + "exception.type": first?.name ?? "Error", | |
| 55 | + "exception.message": scrubSpanText(first?.message ?? "unknown"), | |
| 56 | + "exception.stacktrace": scrubSpanText(Cause.pretty(cause)), | |
| 57 | + }; | |
| 58 | +}; | |
| 59 | + | |
| 39 | 60 | export interface McpSessionInit { |
| 40 | 61 | readonly organizationId: string; |
| 41 | 62 | readonly userId: string; |
| @@ -471,14 +492,8 @@ export abstract class McpAgentSessionDOBase< · 429 unmodified lines | ||
| 471 | 492 | } |
| 472 | 493 | |
| 473 | 494 | private recordCauseOnSpan(cause: Cause.Cause<unknown>): Effect.Effect<void> { |
| 474 | − const errors = Cause.prettyErrors(cause); | |
| 475 | − if (errors.length === 0) return Effect.void; | |
| 476 | − const first = errors[0]; | |
| 477 | − return Effect.annotateCurrentSpan({ | |
| 478 | − "exception.type": first?.name ?? "Error", | |
| 479 | − "exception.message": first?.message ?? "unknown", | |
| 480 | − "exception.stacktrace": Cause.pretty(cause), | |
| 481 | − }); | |
| 495 | + const attributes = causeSpanAttributes(cause); | |
| 496 | + return attributes === null ? Effect.void : Effect.annotateCurrentSpan(attributes); | |
| 482 | 497 | } |
| 483 | 498 | |
| 484 | 499 | private logExecutionOwnerDirectoryFailure(input: { |
| @@ -0,0 +1,87 @@ | ||
| 1 | +// --------------------------------------------------------------------------- | |
| 2 | +// The invoke span must not carry the connection's base URL verbatim. | |
| 3 | +// | |
| 4 | +// A connection `baseUrl` is a user-supplied URL from the same class as the mcp | |
| 5 | +// and graphql endpoints already covered: `?token=…` in the query string and | |
| 6 | +// `user:pass@host` userinfo are both supported input shapes. Stamping it raw on | |
| 7 | +// `plugin.openapi.invoke` shipped the credential to the trace backend on every | |
| 8 | +// tool call. Synthetic placeholders only. | |
| 9 | +// --------------------------------------------------------------------------- | |
| 10 | + | |
| 11 | +import { describe, expect, it } from "@effect/vitest"; | |
| 12 | +import { Effect, Option, Tracer } from "effect"; | |
| 13 | +import { FetchHttpClient } from "effect/unstable/http"; | |
| 14 | + | |
| 15 | +import { invokeWithLayer } from "./invoke"; | |
| 16 | +import { OperationBinding } from "./types"; | |
| 17 | + | |
| 18 | +const QUERY_TOKEN = "synthetic-endpoint-token"; | |
| 19 | +const USERINFO_PASSWORD = "synthetic-endpoint-password"; | |
| 20 | + | |
| 21 | +/** Records every span the program opens so the stamped attributes can be read | |
| 22 | + * back. Port 1 connection-refuses immediately, so the invocation resolves | |
| 23 | + * without any network dependency. */ | |
| 24 | +const recordingTracer = (spans: Array<Tracer.NativeSpan>) => | |
| 25 | + Tracer.make({ | |
| 26 | + span: (options) => { | |
| 27 | + const span = new Tracer.NativeSpan(options); | |
| 28 | + spans.push(span); | |
| 29 | + return span; | |
| 30 | + }, | |
| 31 | + context: (primitive, fiber) => primitive["~effect/Effect/evaluate"](fiber), | |
| 32 | + }); | |
| 33 | + | |
| 34 | +const operation = OperationBinding.make({ | |
| 35 | + method: "get", | |
| 36 | + servers: [], | |
| 37 | + pathTemplate: "/things", | |
| 38 | + requestBody: Option.none(), | |
| 39 | + responseBody: Option.none(), | |
| 40 | + parameters: [], | |
| 41 | +}); | |
| 42 | + | |
| 43 | +const invokeAgainst = (baseUrl: string) => | |
| 44 | + Effect.gen(function* () { | |
| 45 | + const spans: Array<Tracer.NativeSpan> = []; | |
| 46 | + yield* invokeWithLayer(operation, {}, baseUrl, {}, {}, FetchHttpClient.layer).pipe( | |
| 47 | + Effect.exit, | |
| 48 | + Effect.provideService(Tracer.Tracer, recordingTracer(spans)), | |
| 49 | + ); | |
| 50 | + return spans; | |
| 51 | + }); | |
| 52 | + | |
| 53 | +describe("openapi invoke telemetry", () => { | |
| 54 | + it.effect("stamps a sanitized base_url on the invoke span", () => | |
| 55 | + Effect.gen(function* () { | |
| 56 | + const spans = yield* invokeAgainst( | |
| 57 | + `http://svc-user:${USERINFO_PASSWORD}@127.0.0.1:1/v1?token=${QUERY_TOKEN}`, | |
| 58 | + ); | |
| 59 | + | |
| 60 | + const invoke = spans.find((span) => span.name === "plugin.openapi.invoke"); | |
| 61 | + expect(invoke).toBeDefined(); | |
| 62 | + expect(invoke?.attributes.get("plugin.openapi.base_url")).toBe("http://127.0.0.1:1/v1"); | |
| 63 | + | |
| 64 | + // Scoped to the plugin's own spans. Effect's HttpClient separately stamps | |
| 65 | + // `url.full`/`url.query` on its outgoing client spans; those are scrubbed | |
| 66 | + // downstream by the export pipeline's redaction, which is not installed | |
| 67 | + // at this level. | |
| 68 | + const serialized = JSON.stringify( | |
| 69 | + spans | |
| 70 | + .filter((span) => span.name.startsWith("plugin.openapi.")) | |
| 71 | + .map((span) => Object.fromEntries(span.attributes.entries())), | |
| 72 | + ); | |
| 73 | + expect(serialized).not.toContain(QUERY_TOKEN); | |
| 74 | + expect(serialized).not.toContain(USERINFO_PASSWORD); | |
| 75 | + }), | |
| 76 | + ); | |
| 77 | + | |
| 78 | + it.effect("leaves a credential-free base URL intact", () => | |
| 79 | + Effect.gen(function* () { | |
| 80 | + const spans = yield* invokeAgainst("http://127.0.0.1:1/v1"); | |
| 81 | + | |
| 82 | + const invoke = spans.find((span) => span.name === "plugin.openapi.invoke"); | |
| 83 | + expect(invoke?.attributes.get("plugin.openapi.base_url")).toBe("http://127.0.0.1:1/v1"); | |
| 84 | + expect(invoke?.attributes.get("plugin.openapi.path_template")).toBe("/things"); | |
| 85 | + }), | |
| 86 | + ); | |
| 87 | +}); | |
| @@ -1,6 +1,6 @@ | ||
| 1 | 1 | import { Effect, Exit, Fiber, Layer, Option, Schema, Stream } from "effect"; |
| 2 | 2 | import { HttpClient, HttpClientRequest, HttpClientResponse } from "effect/unstable/http"; |
| 3 | −import type { ToolFileValue } from "@executor-js/sdk/core"; | |
| 3 | +import { endpointForTelemetry, type ToolFileValue } from "@executor-js/sdk/core"; | |
| 4 | 4 | |
| 5 | 5 | import { OpenApiInvocationError } from "./errors"; |
| 6 | 6 | import { isNdjsonMediaType, NDJSON_MEDIA_TYPES, resolveServerUrl } from "./openapi-utils"; |
| @@ -1293,7 +1293,10 @@ export const invokeWithLayer = ( · 1286 unmodified lines | ||
| 1293 | 1293 | attributes: { |
| 1294 | 1294 | "plugin.openapi.method": operation.method.toUpperCase(), |
| 1295 | 1295 | "plugin.openapi.path_template": operation.pathTemplate, |
| 1296 | − "plugin.openapi.base_url": effectiveBaseUrl, | |
| 1296 | + // The connection's `baseUrl` is a user-supplied URL from the same class | |
| 1297 | + // as the mcp/graphql endpoints: it may carry `?token=…` or `user:pass@` | |
| 1298 | + // userinfo. Sanitize before stamping. | |
| 1299 | + "plugin.openapi.base_url": endpointForTelemetry(effectiveBaseUrl), | |
| 1297 | 1300 | }, |
| 1298 | 1301 | }), |
| 1299 | 1302 | ); |
| @@ -4,6 +4,7 @@ import { FetchHttpClient, HttpClient, HttpClientRequest } from "effect/unstable/ · 3 unmodified lines | ||
| 4 | 4 | import * as HttpClientError from "effect/unstable/http/HttpClientError"; |
| 5 | 5 | import { OtlpSerialization, OtlpTracer } from "effect/unstable/observability"; |
| 6 | 6 | import { ExecutorApi } from "@executor-js/api/client"; |
| 7 | +import { RedactedHeaderNamesLive } from "@executor-js/sdk/http-auth"; | |
| 7 | 8 | import * as Cause from "effect/Cause"; |
| 8 | 9 | import * as Effect from "effect/Effect"; |
| 9 | 10 | import * as Layer from "effect/Layer"; |
| @@ -11,6 +12,7 @@ import * as Option from "effect/Option"; · 1 unmodified line | ||
| 11 | 12 | import * as Schema from "effect/Schema"; |
| 12 | 13 | |
| 13 | 14 | import { reportHandledFrontendError } from "./error-reporting"; |
| 15 | +import { layerRedacted } from "./otlp-redaction"; | |
| 14 | 16 | import { notifyLocalAuthRequired } from "./local-auth"; |
| 15 | 17 | import { |
| 16 | 18 | EXECUTOR_ORG_HEADER, |
| @@ -105,8 +107,20 @@ if (otlpTracesUrl && typeof document !== "undefined" && Math.random() < otlpSamp · 88 unmodified lines | ||
| 105 | 107 | // Browser sessions are short; the 5s default loses the tail spans |
| 106 | 108 | // when the tab closes. |
| 107 | 109 | exportInterval: "1 second", |
| 108 | − }).pipe(Layer.provide(OtlpSerialization.layerJson), Layer.provide(FetchHttpClient.layer)), | |
| 110 | + }).pipe( | |
| 111 | + // The browser's spans go to the same Axiom dataset the worker's do, so | |
| 112 | + // they get the same scrub: HttpClient stamps `url.full`/`url.query` on | |
| 113 | + // every API request, and a failed span carries the interpolated message | |
| 114 | + // and cause as event text. Wrapping the serializer is the browser's | |
| 115 | + // equivalent of the worker's outermost span processor. | |
| 116 | + Layer.provide(layerRedacted(OtlpSerialization.layerJson)), | |
| 117 | + Layer.provide(FetchHttpClient.layer), | |
| 118 | + ), | |
| 109 | 119 | Layer.succeed(HttpClient.TracerDisabledWhen, (request) => request.url.includes("/v1/traces")), |
| 120 | + // Widen the header names the tracer wraps: an integration's API key rides | |
| 121 | + // on whatever header its auth placement names, and Effect's default list | |
| 122 | + // is only authorization/cookie/set-cookie/x-api-key. | |
| 123 | + RedactedHeaderNamesLive, | |
| 110 | 124 | ), |
| 111 | 125 | ); |
| 112 | 126 | } |
| @@ -0,0 +1,130 @@ | ||
| 1 | +// --------------------------------------------------------------------------- | |
| 2 | +// The BROWSER export seam. The web client ships spans to /v1/traces, which the | |
| 3 | +// worker forwards to the same Axiom dataset the worker's own spans land in, so | |
| 4 | +// the browser must apply the same scrub — the redaction POLICY is covered by | |
| 5 | +// `packages/core/sdk/src/span-redaction.test.ts`. | |
| 6 | +// | |
| 7 | +// This drives the real `OtlpTracer` (the layer `client.tsx` installs) with the | |
| 8 | +// redacting serializer in front of a fake fetch, and asserts on the bytes that | |
| 9 | +// would have gone over the wire. | |
| 10 | +// --------------------------------------------------------------------------- | |
| 11 | + | |
| 12 | +import { describe, expect, it } from "@effect/vitest"; | |
| 13 | +import { Effect, Layer } from "effect"; | |
| 14 | +import { FetchHttpClient } from "effect/unstable/http"; | |
| 15 | +import { OtlpSerialization, OtlpTracer } from "effect/unstable/observability"; | |
| 16 | + | |
| 17 | +import { layerRedacted } from "./otlp-redaction"; | |
| 18 | + | |
| 19 | +// Synthetic placeholders only — never a real authorization code or token. | |
| 20 | +const CODE = "synthetic-authorization-code"; | |
| 21 | +const TOKEN = "synthetic-endpoint-token"; | |
| 22 | + | |
| 23 | +const EXPORT_URL = "https://app.test/v1/traces"; | |
| 24 | + | |
| 25 | +/** Runs `program` under a real `OtlpTracer` whose exporter posts through a fake | |
| 26 | + * fetch, and returns every exported body. The exporter flushes from a scope | |
| 27 | + * finalizer, so closing the scope drains the batch. */ | |
| 28 | +const exportedBodies = ( | |
| 29 | + program: Effect.Effect<void>, | |
| 30 | + serialization: Layer.Layer<OtlpSerialization.OtlpSerialization>, | |
| 31 | +): Effect.Effect<readonly string[]> => | |
| 32 | + Effect.gen(function* () { | |
| 33 | + const decoder = new TextDecoder(); | |
| 34 | + const bodies: string[] = []; | |
| 35 | + const fetchLayer = Layer.succeed(FetchHttpClient.Fetch)((( | |
| 36 | + _input: RequestInfo | URL, | |
| 37 | + init?: RequestInit, | |
| 38 | + ) => { | |
| 39 | + // `OtlpSerialization.layerJson` produces an `HttpBody.Uint8Array`, so the | |
| 40 | + // wire bytes arrive already encoded. | |
| 41 | + const body = init?.body; | |
| 42 | + bodies.push(body instanceof Uint8Array ? decoder.decode(body) : String(body ?? "")); | |
| 43 | + return Promise.resolve(new Response(null, { status: 200 })); | |
| 44 | + }) as typeof globalThis.fetch); | |
| 45 | + | |
| 46 | + yield* Effect.scoped( | |
| 47 | + Effect.provide( | |
| 48 | + program, | |
| 49 | + OtlpTracer.layer({ | |
| 50 | + url: EXPORT_URL, | |
| 51 | + resource: { serviceName: "executor-web-test" }, | |
| 52 | + exportInterval: "1 minute", | |
| 53 | + }).pipe( | |
| 54 | + Layer.provide(serialization), | |
| 55 | + Layer.provide(FetchHttpClient.layer.pipe(Layer.provide(fetchLayer))), | |
| 56 | + ), | |
| 57 | + ), | |
| 58 | + ); | |
| 59 | + | |
| 60 | + return bodies; | |
| 61 | + }); | |
| 62 | + | |
| 63 | +/** One span carrying the attributes `HttpClient` stamps on every API request. */ | |
| 64 | +const callbackSpan = Effect.void.pipe( | |
| 65 | + Effect.withSpan("http.client GET", { | |
| 66 | + attributes: { | |
| 67 | + "url.full": `https://app.test/api/oauth/callback?code=${CODE}&domain=example.test`, | |
| 68 | + "url.query": `code=${CODE}&domain=example.test`, | |
| 69 | + "url.path": "/api/oauth/callback", | |
| 70 | + }, | |
| 71 | + }), | |
| 72 | +); | |
| 73 | + | |
| 74 | +describe("layerRedacted", () => { | |
| 75 | + it.effect("scrubs url.full and url.query out of the exported payload", () => | |
| 76 | + Effect.gen(function* () { | |
| 77 | + const bodies = yield* exportedBodies( | |
| 78 | + callbackSpan, | |
| 79 | + layerRedacted(OtlpSerialization.layerJson), | |
| 80 | + ); | |
| 81 | + | |
| 82 | + expect(bodies).toHaveLength(1); | |
| 83 | + expect(bodies[0]).not.toContain(CODE); | |
| 84 | + // Route-level visibility survives, and the stripped keys are recorded. | |
| 85 | + expect(bodies[0]).toContain("/api/oauth/callback"); | |
| 86 | + expect(bodies[0]).toContain("url.query.stripped_keys"); | |
| 87 | + }), | |
| 88 | + ); | |
| 89 | + | |
| 90 | + it.effect("the unwrapped serializer exports the code — the wrapper is load-bearing", () => | |
| 91 | + Effect.gen(function* () { | |
| 92 | + const bodies = yield* exportedBodies(callbackSpan, OtlpSerialization.layerJson); | |
| 93 | + | |
| 94 | + expect(bodies[0]).toContain(CODE); | |
| 95 | + }), | |
| 96 | + ); | |
| 97 | + | |
| 98 | + it.effect("scrubs a failed span's exception event and status message", () => | |
| 99 | + Effect.gen(function* () { | |
| 100 | + // A failing span: `OtlpTracer` renders the cause into an `exception` | |
| 101 | + // event AND into `status.message`, neither of which is a URL attribute. | |
| 102 | + const failing = Effect.fail( | |
| 103 | + // oxlint-disable-next-line executor/no-error-constructor -- the failure text is what is under test; a tagged error would hide it | |
| 104 | + new Error(`POST https://mcp.test/mcp?token=${TOKEN} failed with 401`), | |
| 105 | + ).pipe(Effect.withSpan("http.client POST"), Effect.ignore); | |
| 106 | + | |
| 107 | + const bodies = yield* exportedBodies(failing, layerRedacted(OtlpSerialization.layerJson)); | |
| 108 | + | |
| 109 | + expect(bodies[0]).not.toContain(TOKEN); | |
| 110 | + expect(bodies[0]).toContain("https://mcp.test/mcp"); | |
| 111 | + expect(bodies[0]).toContain("failed with 401"); | |
| 112 | + }), | |
| 113 | + ); | |
| 114 | + | |
| 115 | + it.effect("leaves a span with nothing sensitive untouched", () => | |
| 116 | + Effect.gen(function* () { | |
| 117 | + const bodies = yield* exportedBodies( | |
| 118 | + Effect.void.pipe( | |
| 119 | + Effect.withSpan("http.client GET", { | |
| 120 | + attributes: { "url.full": "https://app.test/api/integrations?owner=org" }, | |
| 121 | + }), | |
| 122 | + ), | |
| 123 | + layerRedacted(OtlpSerialization.layerJson), | |
| 124 | + ); | |
| 125 | + | |
| 126 | + expect(bodies[0]).toContain("https://app.test/api/integrations?owner=org"); | |
| 127 | + expect(bodies[0]).not.toContain("url.query.stripped_keys"); | |
| 128 | + }), | |
| 129 | + ); | |
| 130 | +}); | |
| @@ -0,0 +1,90 @@ | ||
| 1 | +// --------------------------------------------------------------------------- | |
| 2 | +// Export-seam redaction for the BROWSER isolate's OTLP pipeline. | |
| 3 | +// | |
| 4 | +// The web client's `OtlpTracer` stamps the same credential-bearing values the | |
| 5 | +// worker's tracer does — `HttpClient` writes `url.full` and `url.query` on | |
| 6 | +// every `http.client` span, and a failed span carries the interpolated log | |
| 7 | +// message, `effect.cause`, and `exception.*` as event text — and ships them to | |
| 8 | +// `/v1/traces`, which the worker forwards to Axiom. The policy is shared with | |
| 9 | +// the Workers isolates (`@executor-js/sdk/shared`); only the seam differs. | |
| 10 | +// | |
| 11 | +// `OtlpTracer` has no processor hook: it builds the OTLP payload itself and | |
| 12 | +// hands it to `OtlpSerialization`. That handoff is the one chokepoint every | |
| 13 | +// exported span crosses, so the redaction wraps the serializer rather than the | |
| 14 | +// tracer — a tracer wrapper would have to re-implement `Tracer.Span`, and a | |
| 15 | +// per-request `HttpClient` transform would only cover the clients someone | |
| 16 | +// remembered to wire it into. | |
| 17 | +// --------------------------------------------------------------------------- | |
| 18 | + | |
| 19 | +import * as Effect from "effect/Effect"; | |
| 20 | +import * as Layer from "effect/Layer"; | |
| 21 | +import { OtlpSerialization } from "effect/unstable/observability/OtlpSerialization"; | |
| 22 | +import type { TraceData } from "effect/unstable/observability/OtlpTracer"; | |
| 23 | + | |
| 24 | +import { | |
| 25 | + STRIPPED_QUERY_ATTRIBUTE, | |
| 26 | + redactSpanUrlAttribute, | |
| 27 | + scrubSpanText, | |
| 28 | +} from "@executor-js/sdk/shared"; | |
| 29 | + | |
| 30 | +/** The OTLP span, taken off `TraceData` rather than redeclared — the payload | |
| 31 | + * shape is the contract this wrapper sits on. */ | |
| 32 | +type OtlpSpan = TraceData["resourceSpans"][number]["scopeSpans"][number]["spans"][number]; | |
| 33 | + | |
| 34 | +/** Rewrite one span's payload in place. The `TraceData` handed to the | |
| 35 | + * serializer is freshly built per export batch and is not shared with the | |
| 36 | + * tracer's own span objects, so mutating it affects only what is serialized. | |
| 37 | + * | |
| 38 | + * An OTLP attribute value is a tagged union (`stringValue`, `intValue`, …); | |
| 39 | + * only the string arm can carry a URL or free text. */ | |
| 40 | +const scrubSpan = (span: OtlpSpan): void => { | |
| 41 | + const stripped = new Set<string>(); | |
| 42 | + for (const attribute of span.attributes) { | |
| 43 | + const value = attribute.value.stringValue; | |
| 44 | + if (typeof value !== "string") continue; | |
| 45 | + const result = redactSpanUrlAttribute(attribute.key, value); | |
| 46 | + if (result === null) continue; | |
| 47 | + attribute.value.stringValue = result.value; | |
| 48 | + for (const key of result.stripped) stripped.add(key); | |
| 49 | + } | |
| 50 | + if (stripped.size > 0) { | |
| 51 | + span.attributes.push({ | |
| 52 | + key: STRIPPED_QUERY_ATTRIBUTE, | |
| 53 | + value: { stringValue: Array.from(stripped).sort().join(",") }, | |
| 54 | + }); | |
| 55 | + } | |
| 56 | + span.events.forEach((event, index) => { | |
| 57 | + for (const attribute of event.attributes) { | |
| 58 | + const value = attribute.value.stringValue; | |
| 59 | + if (typeof value !== "string") continue; | |
| 60 | + attribute.value.stringValue = scrubSpanText(value); | |
| 61 | + } | |
| 62 | + // `name` is readonly on the event, so the entry is replaced rather than | |
| 63 | + // assigned through — the attribute bags above are shared with the copy. | |
| 64 | + span.events[index] = { ...event, name: scrubSpanText(event.name) }; | |
| 65 | + }); | |
| 66 | + if (typeof span.status.message === "string") { | |
| 67 | + span.status.message = scrubSpanText(span.status.message); | |
| 68 | + } | |
| 69 | +}; | |
| 70 | + | |
| 71 | +/** Wrap an `OtlpSerialization` layer so every span it serializes is scrubbed | |
| 72 | + * first. Metrics and logs pass through untouched — neither carries a span's | |
| 73 | + * URL attributes, and the log exporter is not installed on this surface. */ | |
| 74 | +export const layerRedacted = ( | |
| 75 | + inner: Layer.Layer<OtlpSerialization>, | |
| 76 | +): Layer.Layer<OtlpSerialization> => | |
| 77 | + Layer.effect( | |
| 78 | + OtlpSerialization, | |
| 79 | + Effect.map(OtlpSerialization.asEffect(), (serialization) => ({ | |
| 80 | + ...serialization, | |
| 81 | + traces: (data) => { | |
| 82 | + for (const resourceSpan of data.resourceSpans) { | |
| 83 | + for (const scopeSpan of resourceSpan.scopeSpans) { | |
| 84 | + for (const span of scopeSpan.spans) scrubSpan(span); | |
| 85 | + } | |
| 86 | + } | |
| 87 | + return serialization.traces(data); | |
| 88 | + }, | |
| 89 | + })), | |
| 90 | + ).pipe(Layer.provide(inner)); | |