From 6dff8916344a394c9166221df7e1d2345cefec9c Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Mon, 17 Aug 2026 10:22:40 -0600 Subject: [PATCH 01/11] Isolate the legacy GHCR mirror in its own release job (#1620) --- .github/workflows/publish-selfhost-docker.yml | 23 +++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/.github/workflows/publish-selfhost-docker.yml b/.github/workflows/publish-selfhost-docker.yml index 51715ff860..1da6470320 100644 --- a/.github/workflows/publish-selfhost-docker.yml +++ b/.github/workflows/publish-selfhost-docker.yml @@ -281,9 +281,6 @@ jobs: runs-on: blacksmith-4vcpu-ubuntu-2404 permissions: packages: write - env: - HAS_LEGACY_TOKEN: ${{ secrets.GHCR_LEGACY_TOKEN != '' }} - steps: - name: Download image digests uses: actions/download-artifact@v4 @@ -330,10 +327,28 @@ jobs: docker buildx imagetools create "${tag_args[@]}" "${sources[@]}" + # Mirrors each release to the pre-org-move namespace + # (ghcr.io/rhyssullivan/executor-selfhost), which GHCR cannot redirect. + # Deliberately a separate job: the canonical publish above never depends on + # it, and a failure here (an expired GHCR_LEGACY_TOKEN, historically) shows + # as one red job pointing at exactly what to fix. + mirror-legacy: + needs: + - metadata + - merge + runs-on: blacksmith-4vcpu-ubuntu-2404 + env: + HAS_LEGACY_TOKEN: ${{ secrets.GHCR_LEGACY_TOKEN != '' }} + + steps: - name: Skip legacy GHCR mirror (GHCR_LEGACY_TOKEN not configured) if: env.HAS_LEGACY_TOKEN != 'true' run: echo "GHCR_LEGACY_TOKEN is not configured; skipping legacy GHCR mirror." + - name: Set up Docker Buildx + if: env.HAS_LEGACY_TOKEN == 'true' + uses: docker/setup-buildx-action@v3 + - name: Log in to legacy GHCR namespace if: env.HAS_LEGACY_TOKEN == 'true' uses: docker/login-action@v3 @@ -342,7 +357,7 @@ jobs: username: rhyssullivan password: ${{ secrets.GHCR_LEGACY_TOKEN }} - - name: Mirror self-host image to legacy GHCR namespace + - name: Mirror release tags to the legacy namespace if: env.HAS_LEGACY_TOKEN == 'true' shell: bash env: From 86c68afef9bf8b7c19ab58f59acfedca0b3c4ca7 Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Mon, 17 Aug 2026 10:48:21 -0600 Subject: [PATCH 02/11] Stamp client identity on MCP execution spans (#1621) --- .changeset/mcp-client-span-attribution.md | 8 ++ apps/cloud/src/mcp/session-durable-object.ts | 4 + .../src/mcp/agent-session-durable-object.ts | 54 ++++++++++++ packages/hosts/mcp/src/tool-server-core.ts | 84 ++++++++++++++++++- packages/hosts/mcp/src/tool-server.test.ts | 74 ++++++++++++++++ packages/hosts/mcp/src/tool-server.ts | 43 ++++++++++ 6 files changed, 263 insertions(+), 4 deletions(-) create mode 100644 .changeset/mcp-client-span-attribution.md diff --git a/.changeset/mcp-client-span-attribution.md b/.changeset/mcp-client-span-attribution.md new file mode 100644 index 0000000000..c3fc4cda24 --- /dev/null +++ b/.changeset/mcp-client-span-attribution.md @@ -0,0 +1,8 @@ +--- +"@executor-js/host-mcp": patch +"@executor-js/cloudflare": patch +--- + +**MCP execution spans now carry the client identity (`mcp.client.*`)** + +The `clientInfo` a client self-reports at `initialize` (or in a modern request's `_meta`) previously existed only on the initialize request itself, which has no session id yet, so execution telemetry could not be segmented by client. Execute, execute-action, and resume spans (and their descendants) now carry `mcp.client.name` / `mcp.client.version` / `mcp.client.title` alongside the existing session join keys. Cloudflare session Durable Objects persist the reported identity in session meta, so attribution survives cold restores; it feeds telemetry only, never behavior. diff --git a/apps/cloud/src/mcp/session-durable-object.ts b/apps/cloud/src/mcp/session-durable-object.ts index c83a5eddb5..b2bdbe11d2 100644 --- a/apps/cloud/src/mcp/session-durable-object.ts +++ b/apps/cloud/src/mcp/session-durable-object.ts @@ -379,6 +379,10 @@ export class McpSessionDOSqlite extends McpAgentSessionDOBase self.persistAppsEnabled(appsEnabled), + // Same restore contract for the client identity that keys the + // `mcp.client.*` span attribution on execution spans. + ...(sessionMeta.clientInfo ? { restoredClientInfo: sessionMeta.clientInfo } : {}), + onClientInfoChange: (clientInfo) => self.persistClientInfo(clientInfo), appsEnabled: false, sessionful: true, requestStateSigningKey: self.modernRequestStateSigningKey(), diff --git a/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.ts b/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.ts index d2454fabb9..79b3e3c5ef 100644 --- a/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.ts +++ b/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.ts @@ -24,11 +24,13 @@ import { import { appsEnabledForClientCapabilities, clientCapabilitiesFromRequestBody, + clientInfoFromRequestBody, mcpRequestStateBindingFromBody, PAUSED_APPROVAL_TIMEOUT_MS, formatMcpExecutionOutcome, mcpRequestStatePrincipal, requestBodyFromRequest, + type McpClientInfo, type PausedExecutionHooks, type ResumeFallbackOutcome, } from "@executor-js/host-mcp/tool-server"; @@ -147,6 +149,14 @@ export interface SessionMeta { * unknown, which behaves as disabled until the next `initialize`. */ readonly appsEnabled?: boolean; + /** + * The client identity (`clientInfo`) self-reported at `initialize` or in a + * modern request's `_meta`. Persisted for the same reason as + * {@link appsEnabled}: a cold-restored server never sees an `initialize`, + * and without this the execution spans' `mcp.client.*` attribution vanishes + * mid-conversation. Telemetry/display only, never behavior. + */ + readonly clientInfo?: McpClientInfo; /** Creation time of this session, retained across isolate eviction. */ readonly createdAtMs?: number; } @@ -164,6 +174,8 @@ export interface ModernMcpServerRequestOptions { readonly requestStateSigningKey: Uint8Array | string; readonly requestStatePrincipal: string; readonly requestStateBinding?: string; + /** Client identity for span attribution: this request's `_meta`, else the session's persisted copy. */ + readonly restoredClientInfo?: McpClientInfo; } /** Long-lived DO execution runtime shared by per-request MCP servers. */ @@ -515,6 +527,34 @@ export abstract class McpAgentSessionDOBase< ); } + /** + * Persist the client identity self-reported at `initialize` (or in a modern + * request's `_meta`), so a cold restore keeps span attribution. Subclasses + * hand this to `buildMcpServer` as `onClientInfoChange`; the modern request + * path calls it directly. Same no-op-before-meta contract as + * {@link persistAppsEnabled}. + */ + protected persistClientInfo(clientInfo: McpClientInfo): Effect.Effect { + const self = this; + return Effect.gen(function* () { + const stored = yield* self.loadSessionMeta(); + if ( + !stored || + (stored.clientInfo?.name === clientInfo.name && + stored.clientInfo?.version === clientInfo.version && + stored.clientInfo?.title === clientInfo.title) + ) { + return; + } + yield* Effect.promise(() => self.saveSessionMeta({ ...stored, clientInfo })); + }).pipe( + Effect.withSpan("mcp.session.persist_client_info", { + attributes: { "mcp.client.name": clientInfo.name }, + }), + Effect.ignoreCause({ log: false }), + ); + } + private async markActivity(now = Date.now()): Promise { this.lastActivityMs = now; const key = @@ -606,6 +646,7 @@ export abstract class McpAgentSessionDOBase< ...resolved, ...(token.webOrigin ? { webOrigin: token.webOrigin } : {}), appsEnabled: stored?.appsEnabled ?? false, + ...(stored?.clientInfo ? { clientInfo: stored.clientInfo } : {}), createdAtMs: stored?.createdAtMs ?? Date.now(), }; yield* Effect.promise(() => self.saveSessionMeta(sessionMeta)).pipe( @@ -816,6 +857,7 @@ export abstract class McpAgentSessionDOBase< const parsedBody = self.modernRequestBodies.get(request); const propagation = self.modernRequestPropagation.get(request); const capabilities = clientCapabilitiesFromRequestBody(parsedBody); + const clientInfo = clientInfoFromRequestBody(parsedBody) ?? sessionMeta.clientInfo; return Effect.runPromise( Effect.gen(function* () { const requestStatePrincipal = mcpRequestStatePrincipal({ @@ -834,6 +876,7 @@ export abstract class McpAgentSessionDOBase< requestStateSigningKey: self.modernRequestStateSigningKey(), requestStatePrincipal, ...(requestStateBinding === null ? {} : { requestStateBinding }), + ...(clientInfo === undefined ? {} : { restoredClientInfo: clientInfo }), }); }).pipe( (effect) => self.withTelemetry(effect, propagation), @@ -1282,6 +1325,17 @@ export abstract class McpAgentSessionDOBase< }); } + // Modern clients self-report identity per request (`_meta` clientInfo) or + // at `initialize`; persist it so meta-less requests and cold restores keep + // their span attribution. Best-effort by construction (persistClientInfo + // swallows failures) and a storage no-op unless the identity changed. + const reportedClientInfo = clientInfoFromRequestBody(parsedBody); + if (reportedClientInfo) { + await Effect.runPromise( + this.withTelemetry(this.persistClientInfo(reportedClientInfo), props.propagation), + ); + } + this.modernRequestBodies.set(request, parsedBody); this.modernRequestPropagation.set(request, props.propagation); this.modernRunningRequestCount += 1; diff --git a/packages/hosts/mcp/src/tool-server-core.ts b/packages/hosts/mcp/src/tool-server-core.ts index ccb07e2f8c..1d77f17130 100644 --- a/packages/hosts/mcp/src/tool-server-core.ts +++ b/packages/hosts/mcp/src/tool-server-core.ts @@ -196,6 +196,30 @@ type SharedMcpServerConfig = { * restore. Best-effort: failures are swallowed and never affect the session. */ readonly onAppsEnabledChange?: (appsEnabled: boolean) => Effect.Effect; + /** + * The client identity self-reported at a previous `initialize` (or in a + * modern request's `_meta`), restored for the same reason as + * {@link restoredAppsEnabled}. Feeds only the `mcp.client.*` span + * attributes on execution spans, never behavior or security decisions. + */ + readonly restoredClientInfo?: McpClientInfo; + /** + * Called when `initialize` reports the client identity, so the host can + * persist it for {@link restoredClientInfo} on a later cold restore. + * Best-effort: failures are swallowed and never affect the session. + */ + readonly onClientInfoChange?: (clientInfo: McpClientInfo) => Effect.Effect; +}; + +/** + * Client software identity as self-reported over MCP (`clientInfo` at + * `initialize`, `_meta` on modern requests). Display and telemetry vocabulary + * only: the spec forbids relying on it for behavior or security. + */ +export type McpClientInfo = { + readonly name: string; + readonly version?: string; + readonly title?: string; }; /** @@ -340,6 +364,8 @@ export type ExecutorMcpAssembly unknown | null; + /** The live `initialize`-reported client identity, when the SDK has one. */ + readonly getClientInfo: () => McpClientInfo | null; readonly getElicitationSupport: () => { readonly form: boolean; readonly url: boolean }; readonly getUiCapability: () => { readonly mimeTypes?: readonly string[] } | undefined; readonly onInitialized: (callback: () => void) => void; @@ -782,6 +808,21 @@ const joinKeyAttributes = (joinKeys: McpRequestJoinKeys): Record => + clientInfo === null + ? {} + : { + "mcp.client.name": clientInfo.name, + ...(clientInfo.version !== undefined ? { "mcp.client.version": clientInfo.version } : {}), + ...(clientInfo.title !== undefined ? { "mcp.client.title": clientInfo.title } : {}), + }; + const startMarker = (name: string, attributes: Record): Effect.Effect => Effect.void.pipe(Effect.withSpan(name, { attributes })); @@ -1110,6 +1151,15 @@ export const buildExecutorMcpTools = < ); const server = assembly.server; + // Seeded from the host's persisted copy; a live `initialize` on this + // instance replaces it via `syncClientInfo` below. Read lazily at each + // tool call so spans always carry the newest identity. + let clientInfo: McpClientInfo | null = config.restoredClientInfo ?? null; + const requestSpanAttributes = (joinKeys: McpRequestJoinKeys): Record => ({ + ...joinKeyAttributes(joinKeys), + ...clientInfoAttributes(clientInfo), + }); + const executeWithNativeElicitation = ( code: string, extra: RequestContext, @@ -1179,7 +1229,7 @@ export const buildExecutorMcpTools = < "mcp.execute.code_length": code.length, }, }), - Effect.annotateSpans(joinKeyAttributes(extra)), + Effect.annotateSpans(requestSpanAttributes(extra)), ); /** What the caller could bind an unresolved role to. Best effort: the @@ -1340,7 +1390,7 @@ export const buildExecutorMcpTools = < "mcp.execute.execution_id": executionId, }, }), - Effect.annotateSpans(joinKeyAttributes(extra)), + Effect.annotateSpans(requestSpanAttributes(extra)), ); const requireUserResumeApproval = (executionId: string): Effect.Effect => @@ -1419,7 +1469,7 @@ export const buildExecutorMcpTools = < "mcp.execute.execution_id": executionId, }, }), - Effect.annotateSpans(joinKeyAttributes(extra)), + Effect.annotateSpans(requestSpanAttributes(extra)), ); // --- tools --- @@ -2169,9 +2219,35 @@ export const buildExecutorMcpTools = < }); }; + // Client identity arrives with the same `initialize` that carries the + // capabilities above. An absent live value (cold restore, construction + // time) is not evidence the client changed, so the restored value stands, + // the same asymmetry `syncToolAvailability` documents for capabilities. + const syncClientInfo = () => { + const live = assembly.getClientInfo(); + if (live === null) return; + const changed = + live.name !== clientInfo?.name || + live.version !== clientInfo?.version || + live.title !== clientInfo?.title; + if (!changed) return; + clientInfo = live; + const onClientInfoChange = config.onClientInfoChange; + if (onClientInfoChange) { + // oxlint-disable-next-line executor/no-effect-escape-hatch -- boundary: `oninitialized` is a sync SDK hook; persistence is fire-and-forget and its failure must not fail the session + void Effect.runPromiseWith(context)( + onClientInfoChange(live).pipe(Effect.ignoreCause({ log: false })), + ); + } + }; + yield* Effect.sync(() => { syncToolAvailability(); - assembly.onInitialized(syncToolAvailability); + syncClientInfo(); + assembly.onInitialized(() => { + syncToolAvailability(); + syncClientInfo(); + }); }).pipe(Effect.withSpan("mcp.host.sync_tool_availability")); return server; diff --git a/packages/hosts/mcp/src/tool-server.test.ts b/packages/hosts/mcp/src/tool-server.test.ts index 2a0febebd1..14b6bb4c3d 100644 --- a/packages/hosts/mcp/src/tool-server.test.ts +++ b/packages/hosts/mcp/src/tool-server.test.ts @@ -17,8 +17,10 @@ import type { ExecutionEngine, ExecutionResult } from "@executor-js/execution"; import { buildMcpServer, + clientInfoFromRequestBody, formatMcpExecutionOutcome, type ExecutorMcpServerConfig, + type McpClientInfo, } from "./tool-server"; // --------------------------------------------------------------------------- @@ -71,6 +73,7 @@ type TestServerConfig = Pick< | "pausedExecutionHooks" | "pausedExecutionLeaseMs" | "resumeFallback" + | "onClientInfoChange" >; /** Connect a real MCP Client to our executor MCP server over in-memory transports. */ @@ -1610,6 +1613,77 @@ describe("MCP host server — skills tool", () => { }); }); +describe("MCP host server — client attribution", () => { + it("stamps the initialize-reported client identity on execution spans", async () => { + await withTracedClient(makeStubEngine({}), async (client, spans) => { + await client.callTool({ name: "execute", arguments: { code: "1+1" } }); + + const execute = spans.find((span) => span.name === "mcp.host.tool.execute"); + expectDefined(execute); + expect(execute.attributes.get("mcp.client.name")).toBe("test-client"); + expect(execute.attributes.get("mcp.client.version")).toBe("1.0.0"); + }); + }); + + it("reports the initialize-reported identity to onClientInfoChange once", async () => { + const reported: McpClientInfo[] = []; + await withClient( + makeStubEngine({}), + NO_CAPS, + async (client) => { + await client.callTool({ name: "execute", arguments: { code: "1+1" } }); + expect(reported).toEqual([{ name: "test-client", version: "1.0.0" }]); + }, + { + onClientInfoChange: (clientInfo) => + Effect.sync(() => { + reported.push(clientInfo); + }), + }, + ); + }); +}); + +describe("clientInfoFromRequestBody", () => { + const META_KEY = "io.modelcontextprotocol/clientInfo"; + + it("prefers the per-request _meta identity", () => { + expect( + clientInfoFromRequestBody({ + method: "tools/call", + params: { _meta: { [META_KEY]: { name: "meta-client", version: "2.0.0" } } }, + }), + ).toEqual({ name: "meta-client", version: "2.0.0" }); + }); + + it("falls back to the initialize body's native clientInfo", () => { + expect( + clientInfoFromRequestBody({ + method: "initialize", + params: { clientInfo: { name: "init-client", version: "1.2.3", title: "Init Client" } }, + }), + ).toEqual({ name: "init-client", version: "1.2.3", title: "Init Client" }); + }); + + it("decodes a malformed or absent identity to null", () => { + expect(clientInfoFromRequestBody(null)).toBeNull(); + expect(clientInfoFromRequestBody({ method: "tools/call", params: {} })).toBeNull(); + expect( + clientInfoFromRequestBody({ + method: "tools/call", + params: { _meta: { [META_KEY]: { version: "no-name" } } }, + }), + ).toBeNull(); + // Native clientInfo is only trusted on `initialize`, where the spec puts it. + expect( + clientInfoFromRequestBody({ + method: "tools/call", + params: { clientInfo: { name: "misplaced" } }, + }), + ).toBeNull(); + }); +}); + describe("MCP host server — hang-visibility tracing", () => { it("execute emits a start marker and stamps the JSON-RPC id on execution spans", async () => { const engine = makeStubEngine({}); diff --git a/packages/hosts/mcp/src/tool-server.ts b/packages/hosts/mcp/src/tool-server.ts index 3458f27797..4d3469eec1 100644 --- a/packages/hosts/mcp/src/tool-server.ts +++ b/packages/hosts/mcp/src/tool-server.ts @@ -9,12 +9,14 @@ import * as Cause from "effect/Cause"; import { acceptedContent, CLIENT_CAPABILITIES_META_KEY, + CLIENT_INFO_META_KEY, createRequestStateCodec, fromJsonSchema, inputRequired, inputResponse, McpServer, type CallToolResult, + type Implementation, type InputRequiredResult, type ServerContext, } from "@modelcontextprotocol/server"; @@ -37,6 +39,7 @@ import { buildExecutorMcpTools, type ExecutorMcpAssembly, type ExecutorMcpToolConfig, + type McpClientInfo, type McpHandlerResult, type McpRequestJoinKeys, type McpToolResult, @@ -48,6 +51,7 @@ export type { BrowserApprovalStore, ExecutorMcpToolConfig, McpArtifactsPort, + McpClientInfo, McpConnectionsPort, McpToolResult, PausedExecutionHooks, @@ -249,6 +253,43 @@ const elicitationSupportFromUnknown = ( }; }; +const ClientInfoSchema = Schema.Struct({ + name: Schema.String, + version: Schema.optional(Schema.String), + title: Schema.optional(Schema.String), +}); +const decodeClientInfo = Schema.decodeUnknownOption(ClientInfoSchema); + +const toClientInfo = (value: unknown): McpClientInfo | null => + Option.getOrNull(decodeClientInfo(value)); + +const clientInfoFromImplementation = ( + implementation: Implementation | undefined, +): McpClientInfo | null => + implementation === undefined + ? null + : { + name: implementation.name, + version: implementation.version, + ...(implementation.title !== undefined ? { title: implementation.title } : {}), + }; + +/** + * Parse the self-reported client identity from a modern request's `_meta` + * envelope (`io.modelcontextprotocol/clientInfo`, sent per request), falling + * back to the `initialize` body's native `params.clientInfo`. Malformed or + * absent identity decodes to null: it is display/telemetry data, never load-bearing. + */ +export const clientInfoFromRequestBody = (body: unknown): McpClientInfo | null => { + if (!isRecord(body)) return null; + const params = body.params; + if (!isRecord(params)) return null; + const metadata = params._meta; + const fromMeta = isRecord(metadata) ? toClientInfo(metadata[CLIENT_INFO_META_KEY]) : null; + if (fromMeta) return fromMeta; + return body.method === "initialize" ? toClientInfo(params.clientInfo) : null; +}; + /** Parse the MCP Apps capability subset from an already-decoded modern body. */ export const clientCapabilitiesFromRequestBody = ( body: unknown, @@ -464,6 +505,8 @@ const createMcpAssembly = ( initialAppsEnabled, getClientCapabilities: () => sessionful ? (server.server.getClientCapabilities() ?? null) : null, + getClientInfo: () => + sessionful ? clientInfoFromImplementation(server.server.getClientVersion()) : null, getElicitationSupport: () => sessionful ? elicitationSupportFromUnknown(server.server.getClientCapabilities()) From 19a1d21370de7b2ab7b2c4ff5155fea87768c59d Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Mon, 17 Aug 2026 11:20:38 -0600 Subject: [PATCH 03/11] Improve trace quality: redact headers, fix DO span parenting, cut noise (#1623) * Redact all non-allowlisted headers on spans Effect's HttpClient tracer records every request and response header on the span, masking only a four-name default blocklist. Upstream integrations carry credentials in provider-specific headers, so any name outside the list shipped verbatim to the trace backend. Invert the model: redact everything except a short allowlist of structurally safe headers. * Parent DO tool spans under the live request currentParentSpan() was a stub returning undefined, so runToolEffect always fell back to the Effect context captured when the session server was built. Every tool execution span in a legacy session parented under the long-closed construction span instead of the request that triggered it. Track the most recent request's traceparent in the DO and resolve it to an external parent span. The grammar moves to do-headers beside the producer; the worker edge parser delegates to it. * Drop zero-duration bootstrap and annotation spans mcp.host.register_tool, mcp.host.register_resource, mcp.host.create_server, mcp.request.annotate, and the executor.stack sub-spans wrap synchronous context lookups and registrations. Together they were about a fifth of weekly span ingest at exactly 0ms each, with zero recorded errors. The stack keeps its executor.stack.build umbrella span; mcp.request keeps the fingerprint attributes the annotate wrapper was duplicating onto itself. * Name workos and user_store spans by operation The two highest-volume span names in the corpus were bare "workos" and "user_store" wrappers with no attributes, swallowing every distinct SDK and store call. use() now takes the operation name, so spans read workos.userManagement.getUser and user_store.getOrganization, and failure logs say which call failed. The WorkOSClient layer construction span goes away with them: synchronous, per-build, and told us nothing. * Record exceptions on worker http.server error spans The worker-boundary catch set ERROR with no message and no exception event, leaving those spans with zero diagnostic content. Record the exception and a status message, and stamp HTTP on the 5xx path. * Clarify where header-name lowercasing happens --- .../account/org-api-key-revoke.node.test.ts | 2 +- .../src/account/workos-account-service.ts | 4 +- .../api/protected-api-key-auth.node.test.ts | 2 +- .../src/api/protected-jwt-auth.node.test.ts | 2 +- apps/cloud/src/auth/context.ts | 7 +- apps/cloud/src/auth/handlers.ts | 10 +- .../src/auth/org-api-key-auth.node.test.ts | 2 +- .../src/auth/org-selector-auth.node.test.ts | 2 +- apps/cloud/src/auth/organization.ts | 8 +- apps/cloud/src/auth/ssr-gate.ts | 2 +- apps/cloud/src/auth/workos.ts | 92 ++++++++++++------- .../src/extensions/billing/route.node.test.ts | 2 +- apps/cloud/src/mcp/auth-provider.ts | 5 +- apps/cloud/src/mcp/auth.ts | 4 +- apps/cloud/src/mcp/telemetry.ts | 1 - apps/cloud/src/mcp/traceparent.ts | 19 ++-- .../src/observability/header-redaction.ts | 65 +++++++++++++ apps/cloud/src/observability/telemetry.ts | 25 +++-- apps/cloud/src/server.ts | 22 ++++- .../core/api/src/server/execution-stack.ts | 10 +- .../core/api/src/server/scoped-executor.ts | 16 +--- .../src/mcp/agent-session-durable-object.ts | 32 ++++++- .../hosts/cloudflare/src/mcp/do-headers.ts | 27 ++++++ packages/hosts/mcp/src/tool-server-core.ts | 46 +--------- 24 files changed, 265 insertions(+), 142 deletions(-) create mode 100644 apps/cloud/src/observability/header-redaction.ts diff --git a/apps/cloud/src/account/org-api-key-revoke.node.test.ts b/apps/cloud/src/account/org-api-key-revoke.node.test.ts index 5deb62f6b8..ca8d3e0522 100644 --- a/apps/cloud/src/account/org-api-key-revoke.node.test.ts +++ b/apps/cloud/src/account/org-api-key-revoke.node.test.ts @@ -77,7 +77,7 @@ const stubWorkOS = Layer.succeed( ); const stubUsers = Layer.succeed(UserStoreService)({ - use: (fn) => + use: (_op, fn) => Effect.promise(() => fn({ ensureAccount: async (id: string) => ({ id, createdAt }), diff --git a/apps/cloud/src/account/workos-account-service.ts b/apps/cloud/src/account/workos-account-service.ts index 739e91d27b..6e70a5b9a1 100644 --- a/apps/cloud/src/account/workos-account-service.ts +++ b/apps/cloud/src/account/workos-account-service.ts @@ -386,7 +386,9 @@ export const workosAccountProvider: Layer.Layer< .updateOrganization(org.id, name) .pipe(Effect.catchTag("WorkOSError", toAccountError)); yield* users - .use((s) => s.upsertOrganization({ id: updated.id, name: updated.name })) + .use("upsertOrganization", (s) => + s.upsertOrganization({ id: updated.id, name: updated.name }), + ) .pipe(Effect.catchTag("UserStoreError", toAccountError)); return { name: updated.name }; }), diff --git a/apps/cloud/src/api/protected-api-key-auth.node.test.ts b/apps/cloud/src/api/protected-api-key-auth.node.test.ts index 6027133349..3ba01a61b4 100644 --- a/apps/cloud/src/api/protected-api-key-auth.node.test.ts +++ b/apps/cloud/src/api/protected-api-key-auth.node.test.ts @@ -47,7 +47,7 @@ const stubWorkOS = Layer.succeed( ); const stubUsers = Layer.succeed(UserStoreService)({ - use: (fn) => + use: (_op, fn) => Effect.promise(() => fn({ ensureAccount: async (id: string) => ({ id, createdAt }), diff --git a/apps/cloud/src/api/protected-jwt-auth.node.test.ts b/apps/cloud/src/api/protected-jwt-auth.node.test.ts index 2120c9004c..dbf35e1c2c 100644 --- a/apps/cloud/src/api/protected-jwt-auth.node.test.ts +++ b/apps/cloud/src/api/protected-jwt-auth.node.test.ts @@ -64,7 +64,7 @@ const stubWorkOS = Layer.succeed( ); const stubUsers = Layer.succeed(UserStoreService)({ - use: (fn) => + use: (_op, fn) => Effect.promise(() => fn({ ensureAccount: async (id: string) => ({ id, createdAt }), diff --git a/apps/cloud/src/auth/context.ts b/apps/cloud/src/auth/context.ts index bfd3ea25c0..dd713b189d 100644 --- a/apps/cloud/src/auth/context.ts +++ b/apps/cloud/src/auth/context.ts @@ -9,10 +9,13 @@ import { UserStoreError, tryPromiseService, withServiceLogging } from "./errors" type RawStore = ReturnType; +// `op` names the store call so every span reads `user_store.` +// instead of one undifferentiated "user_store" bucket, and failures log which +// query actually failed. const makeService = (store: RawStore) => ({ - use: (fn: (s: RawStore) => Promise) => + use: (op: string, fn: (s: RawStore) => Promise) => withServiceLogging( - "user_store", + `user_store.${op}`, () => new UserStoreError(), tryPromiseService(() => fn(store)), ), diff --git a/apps/cloud/src/auth/handlers.ts b/apps/cloud/src/auth/handlers.ts index 35d18cfa66..d948787351 100644 --- a/apps/cloud/src/auth/handlers.ts +++ b/apps/cloud/src/auth/handlers.ts @@ -202,7 +202,7 @@ export const CloudAuthPublicHandlers = HttpApiBuilder.group( const result = yield* workos.authenticateWithCode(query.code); // Mirror the account locally - yield* users.use((s) => s.ensureAccount(result.user.id)); + yield* users.use("ensureAccount", (s) => s.ensureAccount(result.user.id)); let sealedSession = result.sealedSession; @@ -420,7 +420,7 @@ export const CloudSessionAuthHandlers = HttpApiBuilder.group( const org = yield* workos.createOrganization(name); yield* workos.createMembership(org.id, session.accountId, "admin"); // `upsertOrganization` mints the slug at insert — no separate heal step. - const mirrored = yield* users.use((s) => + const mirrored = yield* users.use("upsertOrganization", (s) => s.upsertOrganization({ id: org.id, name: org.name }), ); @@ -474,7 +474,7 @@ export const CloudSessionAuthHandlers = HttpApiBuilder.group( // The typed confirmation must match the org's current name — the same // label the settings page shows. Trimmed on both sides. - const org = yield* users.use((s) => s.getOrganization(organizationId)); + const org = yield* users.use("getOrganization", (s) => s.getOrganization(organizationId)); if (!org || payload.confirmName.trim() !== org.name.trim()) { return yield* new OrganizationDeletionForbidden(); } @@ -492,7 +492,7 @@ export const CloudSessionAuthHandlers = HttpApiBuilder.group( // everyone (unreachable) but its secrets/tenant rows linger orphaned — // alert loudly so that window gets swept, then surface the failure. yield* users - .use((s) => s.deleteOrganizationCascade(organizationId)) + .use("deleteOrganizationCascade", (s) => s.deleteOrganizationCascade(organizationId)) .pipe( Effect.tapError((error) => Effect.logError( @@ -593,7 +593,7 @@ export const CloudSessionAuthHandlers = HttpApiBuilder.group( // Mirror the org locally so domain tables can FK against it; the // upsert mints the slug at insert — no separate heal step. const org = yield* workos.getOrganization(invitation.organizationId); - const mirrored = yield* users.use((s) => + const mirrored = yield* users.use("upsertOrganization", (s) => s.upsertOrganization({ id: org.id, name: org.name }), ); diff --git a/apps/cloud/src/auth/org-api-key-auth.node.test.ts b/apps/cloud/src/auth/org-api-key-auth.node.test.ts index 1d1e00b9ad..ec87511c3b 100644 --- a/apps/cloud/src/auth/org-api-key-auth.node.test.ts +++ b/apps/cloud/src/auth/org-api-key-auth.node.test.ts @@ -62,7 +62,7 @@ const stubWorkOS = Layer.succeed( ); const stubUsers = Layer.succeed(UserStoreService)({ - use: (fn) => + use: (_op, fn) => Effect.promise(() => fn({ ensureAccount: async (id: string) => ({ id, createdAt }), diff --git a/apps/cloud/src/auth/org-selector-auth.node.test.ts b/apps/cloud/src/auth/org-selector-auth.node.test.ts index 5c59ba7de4..ead56fb893 100644 --- a/apps/cloud/src/auth/org-selector-auth.node.test.ts +++ b/apps/cloud/src/auth/org-selector-auth.node.test.ts @@ -64,7 +64,7 @@ const stubWorkOS = Layer.succeed( ); const stubUsers = Layer.succeed(UserStoreService)({ - use: (fn) => + use: (_op, fn) => Effect.promise(() => fn({ ensureAccount: async (id: string) => ({ id, createdAt }), diff --git a/apps/cloud/src/auth/organization.ts b/apps/cloud/src/auth/organization.ts index 51944cfbbe..073dfacb32 100644 --- a/apps/cloud/src/auth/organization.ts +++ b/apps/cloud/src/auth/organization.ts @@ -38,12 +38,14 @@ import { WorkOSClient } from "./workos"; export const resolveOrganization = (organizationId: string) => Effect.gen(function* () { const users = yield* UserStoreService; - const existing = yield* users.use((s) => s.getOrganization(organizationId)); + const existing = yield* users.use("getOrganization", (s) => s.getOrganization(organizationId)); if (existing) return existing; const workos = yield* WorkOSClient; const fresh = yield* workos.getOrganization(organizationId); - return yield* users.use((s) => s.upsertOrganization({ id: fresh.id, name: fresh.name })); + return yield* users.use("upsertOrganization", (s) => + s.upsertOrganization({ id: fresh.id, name: fresh.name }), + ); }); // --------------------------------------------------------------------------- @@ -125,7 +127,7 @@ export const authorizeOrganizationSelector = (userId: string, selector: string) return yield* authorizeOrganization(userId, selector); } const users = yield* UserStoreService; - const org = yield* users.use((s) => s.getOrganizationBySlug(selector)); + const org = yield* users.use("getOrganizationBySlug", (s) => s.getOrganizationBySlug(selector)); if (!org) return null; return yield* authorizeOrganization(userId, org.id); }); diff --git a/apps/cloud/src/auth/ssr-gate.ts b/apps/cloud/src/auth/ssr-gate.ts index a95c225dff..a85f9ffe1e 100644 --- a/apps/cloud/src/auth/ssr-gate.ts +++ b/apps/cloud/src/auth/ssr-gate.ts @@ -153,7 +153,7 @@ const organizationDisplay = async ( ): Promise<{ name: string; slug: string }> => { const exit = await getRuntime().runPromiseExit( Effect.flatMap(UserStoreService.asEffect(), (users) => - users.use((store) => store.getOrganization(organizationId)), + users.use("getOrganization", (store) => store.getOrganization(organizationId)), ).pipe(Effect.provide(Layer.provide(makeUserStoreLayer(), makeDbLayer()))), ); return Exit.isSuccess(exit) diff --git a/apps/cloud/src/auth/workos.ts b/apps/cloud/src/auth/workos.ts index 88281115af..51959784a8 100644 --- a/apps/cloud/src/auth/workos.ts +++ b/apps/cloud/src/auth/workos.ts @@ -338,9 +338,12 @@ const make = Effect.gen(function* () { // exception had one (all its typed exceptions do), so consumers can tell a // definitive WorkOS denial (401/403/404 — fail closed) from a transient // failure (429/5xx/network — retryable). - const use = (fn: (wos: WorkOS) => Promise) => + // `op` names the SDK call (mirroring its `namespace.method` path) so every + // span reads `workos.` instead of one undifferentiated "workos" + // bucket, and failures log which call actually failed. + const use = (op: string, fn: (wos: WorkOS) => Promise) => withServiceLogging( - "workos", + `workos.${op}`, workosErrorFromFailure, tryPromiseService(() => fn(workos)), ); @@ -376,7 +379,7 @@ const make = Effect.gen(function* () { if (isLocalSessionInvalidCookie(local)) return null; // Try refreshing - const refreshed = yield* use(() => session.refresh()).pipe( + const refreshed = yield* use("session.refresh", () => session.refresh()).pipe( Effect.orElseSucceed(() => ({ authenticated: false as const })), ); @@ -405,7 +408,7 @@ const make = Effect.gen(function* () { }), authenticateWithCode: (code: string) => - use((wos) => + use("userManagement.authenticateWithCode", (wos) => wos.userManagement.authenticateWithCode({ code, clientId, @@ -415,11 +418,13 @@ const make = Effect.gen(function* () { /** Create a new organization in WorkOS. */ createOrganization: (name: string) => - use((wos) => wos.organizations.createOrganization({ name })), + use("organizations.createOrganization", (wos) => + wos.organizations.createOrganization({ name }), + ), /** Add a user to an organization. */ createMembership: (organizationId: string, userId: string, roleSlug?: string) => - use((wos) => + use("userManagement.createOrganizationMembership", (wos) => wos.userManagement.createOrganizationMembership({ organizationId, userId, @@ -429,7 +434,7 @@ const make = Effect.gen(function* () { /** List organization memberships for a user. */ listUserMemberships: (userId: string) => - use(async (wos) => + use("userManagement.listOrganizationMemberships", async (wos) => collectWorkOSList( await wos.userManagement.listOrganizationMemberships({ userId, @@ -448,7 +453,7 @@ const make = Effect.gen(function* () { sessionData, cookiePassword, }); - const refreshed = yield* use(() => + const refreshed = yield* use("session.refresh", () => session.refresh(organizationId ? { organizationId } : undefined), ); if (!refreshed.authenticated || !("sealedSession" in refreshed)) return null; @@ -517,10 +522,13 @@ const make = Effect.gen(function* () { * auth/api-keys.ts. */ validateApiKey: (value: string) => - use((wos) => wos.apiKeys.validateApiKey({ value }) as Promise), + use( + "apiKeys.validateApiKey", + (wos) => wos.apiKeys.validateApiKey({ value }) as Promise, + ), listUserApiKeys: (userId: string, organizationId: string) => - use(async (wos) => { + use("userManagement.listUserApiKeys", async (wos) => { const raw = wos as RawWorkOS; return collectRawWorkOSList(async (after) => { const response = await raw.get(`/user_management/users/${userId}/api_keys`, { @@ -535,7 +543,7 @@ const make = Effect.gen(function* () { }), createUserApiKey: (params: { userId: string; organizationId: string; name: string }) => - use(async (wos) => { + use("userManagement.createUserApiKey", async (wos) => { const raw = wos as RawWorkOS; const response = await raw.post(`/user_management/users/${params.userId}/api_keys`, { name: params.name, @@ -552,7 +560,7 @@ const make = Effect.gen(function* () { * other key response. */ listOrgApiKeys: (organizationId: string) => - use(async (wos) => + use("organizations.listOrganizationApiKeys", async (wos) => collectWorkOSList(await wos.organizations.listOrganizationApiKeys({ organizationId })), ), @@ -564,6 +572,7 @@ const make = Effect.gen(function* () { */ createOrgApiKey: (params: { organizationId: string; name: string }) => use( + "organizations.createOrganizationApiKey", (wos) => wos.organizations.createOrganizationApiKey({ organizationId: params.organizationId, @@ -571,11 +580,12 @@ const make = Effect.gen(function* () { }) as Promise, ), - deleteApiKey: (id: string) => use((wos) => wos.apiKeys.deleteApiKey(id)), + deleteApiKey: (id: string) => + use("apiKeys.deleteApiKey", (wos) => wos.apiKeys.deleteApiKey(id)), /** List organization memberships with user details. */ listOrgMembers: (organizationId: string) => - use(async (wos) => + use("userManagement.listOrganizationMemberships", async (wos) => collectWorkOSList( await wos.userManagement.listOrganizationMemberships({ organizationId, @@ -586,7 +596,7 @@ const make = Effect.gen(function* () { /** Get a user's membership in an organization. */ getUserOrgMembership: (organizationId: string, userId: string) => - use(async (wos) => { + use("userManagement.listOrganizationMemberships", async (wos) => { const response = await wos.userManagement.listOrganizationMemberships({ organizationId, userId, @@ -596,11 +606,12 @@ const make = Effect.gen(function* () { }), /** Get a user by ID. */ - getUser: (userId: string) => use((wos) => wos.userManagement.getUser(userId)), + getUser: (userId: string) => + use("userManagement.getUser", (wos) => wos.userManagement.getUser(userId)), /** List users matching an email within one organization. */ listUsers: (params: { email: string; organizationId: string }) => - use(async (wos) => + use("userManagement.listUsers", async (wos) => collectWorkOSList( await wos.userManagement.listUsers({ email: params.email, @@ -611,7 +622,7 @@ const make = Effect.gen(function* () { /** Send an organization invitation. */ sendInvitation: (params: { email: string; organizationId: string; roleSlug?: string }) => - use((wos) => + use("userManagement.sendInvitation", (wos) => wos.userManagement.sendInvitation({ email: params.email, organizationId: params.organizationId, @@ -625,7 +636,7 @@ const make = Effect.gen(function* () { * API level, so we filter after. */ listPendingInvitations: (organizationId: string) => - use(async (wos) => + use("userManagement.listInvitations", async (wos) => collectWorkOSList( await wos.userManagement.listInvitations({ organizationId, @@ -640,7 +651,7 @@ const make = Effect.gen(function* () { /** List invitations for an email address (across all orgs). */ listInvitationsByEmail: (email: string) => - use(async (wos) => + use("userManagement.listInvitations", async (wos) => collectWorkOSList( await wos.userManagement.listInvitations({ email, @@ -650,19 +661,25 @@ const make = Effect.gen(function* () { /** Accept an invitation; returns the (now accepted) invitation. */ acceptInvitation: (invitationId: string) => - use((wos) => wos.userManagement.acceptInvitation(invitationId)), + use("userManagement.acceptInvitation", (wos) => + wos.userManagement.acceptInvitation(invitationId), + ), /** Remove an organization membership. */ deleteOrgMembership: (membershipId: string) => - use((wos) => wos.userManagement.deleteOrganizationMembership(membershipId)), + use("userManagement.deleteOrganizationMembership", (wos) => + wos.userManagement.deleteOrganizationMembership(membershipId), + ), /** Get the role for a membership. */ getOrgMembership: (membershipId: string) => - use((wos) => wos.userManagement.getOrganizationMembership(membershipId)), + use("userManagement.getOrganizationMembership", (wos) => + wos.userManagement.getOrganizationMembership(membershipId), + ), /** Update a membership's role. */ updateOrgMembershipRole: (membershipId: string, roleSlug: string) => - use((wos) => + use("userManagement.updateOrganizationMembership", (wos) => wos.userManagement.updateOrganizationMembership(membershipId, { roleSlug, }), @@ -670,15 +687,19 @@ const make = Effect.gen(function* () { /** List available roles for an organization. */ listOrgRoles: (organizationId: string) => - use((wos) => wos.organizations.listOrganizationRoles({ organizationId })), + use("organizations.listOrganizationRoles", (wos) => + wos.organizations.listOrganizationRoles({ organizationId }), + ), /** Get an organization (includes domains). */ getOrganization: (organizationId: string) => - use((wos) => wos.organizations.getOrganization(organizationId)), + use("organizations.getOrganization", (wos) => + wos.organizations.getOrganization(organizationId), + ), /** Update an organization. */ updateOrganization: (organizationId: string, name: string) => - use((wos) => + use("organizations.updateOrganization", (wos) => wos.organizations.updateOrganization({ organization: organizationId, name, @@ -690,11 +711,13 @@ const make = Effect.gen(function* () { * invitations, and domains go with it, so every member loses access. */ deleteOrganization: (organizationId: string) => - use((wos) => wos.organizations.deleteOrganization(organizationId)), + use("organizations.deleteOrganization", (wos) => + wos.organizations.deleteOrganization(organizationId), + ), /** Generate an Admin Portal link for domain verification. */ generateDomainVerificationPortalLink: (organizationId: string, returnUrl: string) => - use((wos) => + use("portal.generateLink", (wos) => wos.portal.generateLink({ organization: organizationId, intent: GeneratePortalLinkIntent.DomainVerification, @@ -704,11 +727,11 @@ const make = Effect.gen(function* () { /** Get a domain by ID. */ getOrganizationDomain: (domainId: string) => - use((wos) => wos.organizationDomains.get(domainId)), + use("organizationDomains.get", (wos) => wos.organizationDomains.get(domainId)), /** Delete a domain claim. */ deleteOrganizationDomain: (domainId: string) => - use((wos) => wos.organizationDomains.delete(domainId)), + use("organizationDomains.delete", (wos) => wos.organizationDomains.delete(domainId)), }; }); @@ -717,9 +740,10 @@ export type WorkOSClientService = Effect.Success; export class WorkOSClient extends Context.Service()( "@executor-js/cloud/WorkOSClient", ) { - static Default = Layer.effect(this)(make).pipe( - Layer.withSpan("WorkOSClient", { attributes: { module: "WorkOSClient" } }), - ); + // Deliberately unspanned: client construction is synchronous and ran per + // layer build, which produced one of the highest-volume zero-duration span + // names in the whole trace corpus while telling us nothing. + static Default = Layer.effect(this)(make); } // The boot-scoped WorkOS client root — the one neutral service the stateless diff --git a/apps/cloud/src/extensions/billing/route.node.test.ts b/apps/cloud/src/extensions/billing/route.node.test.ts index f6274facc7..dc2a5a7316 100644 --- a/apps/cloud/src/extensions/billing/route.node.test.ts +++ b/apps/cloud/src/extensions/billing/route.node.test.ts @@ -34,7 +34,7 @@ const stubWorkOS = Layer.succeed( ); const stubUsers = Layer.succeed(UserStoreService)({ - use: (fn) => + use: (_op, fn) => Effect.promise(() => fn({ ensureAccount: async (id: string) => ({ id, createdAt }), diff --git a/apps/cloud/src/mcp/auth-provider.ts b/apps/cloud/src/mcp/auth-provider.ts index 628007a2fd..8a5944f052 100644 --- a/apps/cloud/src/mcp/auth-provider.ts +++ b/apps/cloud/src/mcp/auth-provider.ts @@ -21,8 +21,9 @@ // clearExistingSession. // - verified + org allowed -> Authenticated(principal) // -// The rich `mcp.request.annotate` client-fingerprint span (cloud-specific, no -// envelope seam) is emitted from here so telemetry parity is preserved. +// The rich client-fingerprint annotations (cloud-specific, no envelope seam) +// are stamped onto the `mcp.request` span from here so telemetry parity is +// preserved. // // The OAuth endpoints (/authorize, /token, /register) are NOT cloud's — they // live at WorkOS/AuthKit (external); only the two discovery docs are mounted. diff --git a/apps/cloud/src/mcp/auth.ts b/apps/cloud/src/mcp/auth.ts index 6ffdcf4383..347a79faf7 100644 --- a/apps/cloud/src/mcp/auth.ts +++ b/apps/cloud/src/mcp/auth.ts @@ -204,7 +204,9 @@ const resolveOrgSelector = (selector: string) => ? Effect.succeed(selector) : Effect.gen(function* () { const users = yield* UserStoreService; - const org = yield* users.use((s) => s.getOrganizationBySlug(selector)); + const org = yield* users.use("getOrganizationBySlug", (s) => + s.getOrganizationBySlug(selector), + ); return org?.id ?? null; }); diff --git a/apps/cloud/src/mcp/telemetry.ts b/apps/cloud/src/mcp/telemetry.ts index 1e2d631335..de53c3a49c 100644 --- a/apps/cloud/src/mcp/telemetry.ts +++ b/apps/cloud/src/mcp/telemetry.ts @@ -254,5 +254,4 @@ export const annotateMcpRequest = ( }; yield* Effect.annotateCurrentSpan(attrs); - yield* Effect.annotateCurrentSpan(attrs).pipe(Effect.withSpan("mcp.request.annotate")); }); diff --git a/apps/cloud/src/mcp/traceparent.ts b/apps/cloud/src/mcp/traceparent.ts index 0be604025a..72555df4cf 100644 --- a/apps/cloud/src/mcp/traceparent.ts +++ b/apps/cloud/src/mcp/traceparent.ts @@ -1,13 +1,13 @@ // --------------------------------------------------------------------------- // W3C traceparent parsing shared by the worker edge (server.ts), the MCP agent -// handler, and the session DO. Single-sourced so the producer (the worker span -// stamping traceparent onto the forwarded request) and the consumers (the -// Effect programs joining that span) cannot drift on the header grammar. +// handler, and the session DO. The header grammar itself is single-sourced in +// `@executor-js/cloudflare/mcp/do-headers` beside the producer that stamps the +// header; this module only layers the OTel `tracestate` carrier on top for the +// consumers that join the span via the OTel API. // --------------------------------------------------------------------------- import { createTraceState } from "@opentelemetry/api"; - -const TRACEPARENT_PATTERN = /^([0-9a-f]{2})-([0-9a-f]{32})-([0-9a-f]{16})-([0-9a-f]{2})$/; +import { parseTraceparentHeader } from "@executor-js/cloudflare/mcp/do-headers"; export type IncomingSpanContext = { readonly traceId: string; @@ -20,13 +20,10 @@ export const parseTraceparent = ( traceparent: string | null | undefined, tracestate: string | null | undefined, ): IncomingSpanContext | null => { - if (!traceparent) return null; - const match = TRACEPARENT_PATTERN.exec(traceparent); - if (!match) return null; + const parsed = parseTraceparentHeader(traceparent); + if (!parsed) return null; return { - traceId: match[2]!, - spanId: match[3]!, - traceFlags: parseInt(match[4]!, 16), + ...parsed, ...(tracestate ? { traceState: createTraceState(tracestate) } : {}), }; }; diff --git a/apps/cloud/src/observability/header-redaction.ts b/apps/cloud/src/observability/header-redaction.ts new file mode 100644 index 0000000000..188046682c --- /dev/null +++ b/apps/cloud/src/observability/header-redaction.ts @@ -0,0 +1,65 @@ +// --------------------------------------------------------------------------- +// Span header redaction. +// +// Effect's HttpClient/HttpServer tracer records every request and response +// header as a span attribute, masking only the names in +// `Headers.CurrentRedactedNames` (default: authorization, cookie, set-cookie, +// x-api-key). That blocklist can never be right here: executor's whole job is +// calling arbitrary upstream APIs whose credentials ride in provider-specific +// headers (x-goog-api-key, api-key, x-auth-token, ...), and any name missing +// from the list ships a live secret to the trace backend verbatim. +// +// So the override inverts the model: every header is redacted unless its name +// is on the allowlist of structurally safe, diagnostically useful headers. +// The alternative of enumerating known secret-bearing names was rejected: new +// integrations add new credential headers faster than a blocklist can learn +// them, and one miss is a leaked customer credential. +// --------------------------------------------------------------------------- + +import { Layer } from "effect"; +import { Headers } from "effect/unstable/http"; + +// Names that never carry credentials and are worth reading on a span while +// debugging: negotiation, caching, routing, and the tracing headers +// themselves. Everything else renders as ``. +const SAFE_HEADER_NAMES = [ + "accept", + "accept-encoding", + "accept-language", + "age", + "cache-control", + "connection", + "content-encoding", + "content-length", + "content-type", + "date", + "etag", + "expires", + "host", + "if-modified-since", + "if-none-match", + "last-modified", + "location", + "mcp-protocol-version", + "mcp-session-id", + "origin", + "referer", + "retry-after", + "traceparent", + "tracestate", + "transfer-encoding", + "user-agent", + "vary", + "via", + "x-request-id", +] as const; + +// Header names are lowercased at `Headers` construction, and `Headers.redact` +// masks every name a RegExp entry matches. One negative lookahead turns the +// allowlist into "redact everything else". +const redactAllButSafe = new RegExp(`^(?!(?:${SAFE_HEADER_NAMES.join("|")})$)`); + +export const SpanHeaderRedactionLive: Layer.Layer = Layer.succeed( + Headers.CurrentRedactedNames, + [redactAllButSafe], +); diff --git a/apps/cloud/src/observability/telemetry.ts b/apps/cloud/src/observability/telemetry.ts index 7bd5baa589..e71701d7f1 100644 --- a/apps/cloud/src/observability/telemetry.ts +++ b/apps/cloud/src/observability/telemetry.ts @@ -45,6 +45,7 @@ import { ATTR_SERVICE_NAME, ATTR_SERVICE_VERSION } from "@opentelemetry/semantic import { env } from "cloudflare:workers"; import { Effect, Layer } from "effect"; +import { SpanHeaderRedactionLive } from "./header-redaction"; import { CountingSpanExporter, CountingSpanProcessor, @@ -124,15 +125,21 @@ export const flushTracerProvider = async (): Promise => { }; const makeTelemetryLive = (): Layer.Layer => - Layer.unwrap( - Effect.sync(() => - ensureGlobalTracerProvider() - ? OtelTracer.layerGlobal.pipe( - Layer.provide( - Resource.layer({ serviceName: SERVICE_NAME, serviceVersion: SERVICE_VERSION }), - ), - ) - : Layer.empty, + Layer.mergeAll( + // Redaction applies even when the exporter is not installed: Effect still + // builds spans (and their header attributes) in-memory, and any future + // consumer of those spans must never observe an unredacted credential. + SpanHeaderRedactionLive, + Layer.unwrap( + Effect.sync(() => + ensureGlobalTracerProvider() + ? OtelTracer.layerGlobal.pipe( + Layer.provide( + Resource.layer({ serviceName: SERVICE_NAME, serviceVersion: SERVICE_VERSION }), + ), + ) + : Layer.empty, + ), ), ); diff --git a/apps/cloud/src/server.ts b/apps/cloud/src/server.ts index 12aac6773d..8443a9d8b7 100644 --- a/apps/cloud/src/server.ts +++ b/apps/cloud/src/server.ts @@ -172,11 +172,18 @@ const cloudflareHandler: ExportedHandler = { const response = await mcpAgentHandler(new Request(forwarded, { headers }), env, ctx); span.setAttribute(ATTR_HTTP_RESPONSE_STATUS_CODE, response.status); if (response.status >= 500) { - span.setStatus({ code: SpanStatusCode.ERROR }); + span.setStatus({ code: SpanStatusCode.ERROR, message: `HTTP ${response.status}` }); } return response; } catch (err) { - span.setStatus({ code: SpanStatusCode.ERROR }); + // Record the exception itself, not just the status bit: without it + // these spans are ERROR with zero diagnostic content. + // oxlint-disable-next-line executor/no-instanceof-error, executor/no-unknown-error-message -- adapter boundary: Cloudflare's fetch callback throws untyped; normalized only for the OTel span record, the original error is rethrown below + const cause = err instanceof Error ? err : String(err); + span.recordException(cause); + // oxlint-disable-next-line executor/no-unknown-error-message -- adapter boundary: same normalization as the recordException line above + const message = typeof cause === "string" ? cause : cause.message; + span.setStatus({ code: SpanStatusCode.ERROR, message }); // oxlint-disable-next-line executor/no-try-catch-or-throw -- adapter boundary; preserve original error to Cloudflare runtime throw err; } finally { @@ -237,11 +244,18 @@ const cloudflareHandler: ExportedHandler = { const response = await fetchHandler(request, env, ctx); span.setAttribute(ATTR_HTTP_RESPONSE_STATUS_CODE, response.status); if (response.status >= 500) { - span.setStatus({ code: SpanStatusCode.ERROR }); + span.setStatus({ code: SpanStatusCode.ERROR, message: `HTTP ${response.status}` }); } return response; } catch (err) { - span.setStatus({ code: SpanStatusCode.ERROR }); + // Record the exception itself, not just the status bit: without it + // these spans are ERROR with zero diagnostic content. + // oxlint-disable-next-line executor/no-instanceof-error, executor/no-unknown-error-message -- adapter boundary: Cloudflare's fetch callback throws untyped; normalized only for the OTel span record, the original error is rethrown below + const cause = err instanceof Error ? err : String(err); + span.recordException(cause); + // oxlint-disable-next-line executor/no-unknown-error-message -- adapter boundary: same normalization as the recordException line above + const message = typeof cause === "string" ? cause : cause.message; + span.setStatus({ code: SpanStatusCode.ERROR, message }); // oxlint-disable-next-line executor/no-try-catch-or-throw -- adapter boundary; preserve original error to Cloudflare runtime throw err; } finally { diff --git a/packages/core/api/src/server/execution-stack.ts b/packages/core/api/src/server/execution-stack.ts index eb7756a51d..d951f3b813 100644 --- a/packages/core/api/src/server/execution-stack.ts +++ b/packages/core/api/src/server/execution-stack.ts @@ -124,13 +124,9 @@ export const makeExecutionStack = < organizationId, organizationName, { plugins: { mcpResource: options?.mcpResource } }, - ).pipe(Effect.withSpan("executor.stack.scoped_executor")); - const codeExecutor = yield* CodeExecutorProvider.asEffect().pipe( - Effect.withSpan("executor.stack.code_executor"), - ); - const { decorate } = yield* EngineDecorator.asEffect().pipe( - Effect.withSpan("executor.stack.decorator"), ); + const codeExecutor = yield* CodeExecutorProvider.asEffect(); + const { decorate } = yield* EngineDecorator.asEffect(); const engine = yield* Effect.sync(() => decorate( createExecutionEngine({ executor, codeExecutor }), @@ -141,7 +137,7 @@ export const makeExecutionStack = < }, { mcpResource: options?.mcpResource }, ), - ).pipe(Effect.withSpan("executor.stack.engine.init")); + ); return { executor, engine }; }).pipe(Effect.withSpan("executor.stack.build")); diff --git a/packages/core/api/src/server/scoped-executor.ts b/packages/core/api/src/server/scoped-executor.ts index ea0e33ce61..53f14592f8 100644 --- a/packages/core/api/src/server/scoped-executor.ts +++ b/packages/core/api/src/server/scoped-executor.ts @@ -221,13 +221,9 @@ export const makeScopedExecutor = < options?: { readonly plugins?: PluginsProviderContext }, ): Effect.Effect, StorageFailure, DbProvider | PluginsProvider | HostConfig> => Effect.gen(function* () { - const { db, blobs } = yield* DbProvider.asEffect().pipe( - Effect.withSpan("executor.stack.db_provider"), - ); - const { plugins: pluginsFactory } = yield* PluginsProvider.asEffect().pipe( - Effect.withSpan("executor.stack.plugins_provider"), - ); - const config = yield* HostConfig.asEffect().pipe(Effect.withSpan("executor.stack.host_config")); + const { db, blobs } = yield* DbProvider.asEffect(); + const { plugins: pluginsFactory } = yield* PluginsProvider.asEffect(); + const config = yield* HostConfig.asEffect(); // Explicit config wins; otherwise fall back to the request origin if a host // provided one (HTTP middleware / MCP session DO). Stays `undefined` for // non-request callers — `coreTools.webBaseUrl` is optional and only the @@ -263,9 +259,7 @@ export const makeScopedExecutor = < oauthCallbackPath: config.oauthCallbackPath, }); - const plugins = yield* Effect.sync(() => pluginsFactory(options?.plugins)).pipe( - Effect.withSpan("executor.plugins.init"), - ); + const plugins = yield* Effect.sync(() => pluginsFactory(options?.plugins)); const hostedHttpOptions = { allowLocalNetwork: config.allowLocalNetwork, }; @@ -292,7 +286,7 @@ export const makeScopedExecutor = < orgSlug, includeProviders: config.exposeCredentialProviders ?? true, }, - }).pipe(Effect.withSpan("executor.stack.create_executor")); + }); // Record the sighting. THIS is the seam every HTTP request and MCP session // on every host passes through, so it is where the `subject` table gets // populated: a principal earns a row the first time it authenticates, diff --git a/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.ts b/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.ts index 79b3e3c5ef..937b9928d7 100644 --- a/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.ts +++ b/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.ts @@ -1,6 +1,6 @@ import { DurableObject } from "cloudflare:workers"; import { Cause, Data, Deferred, Effect, Option, Schema } from "effect"; -import type * as Tracer from "effect/Tracer"; +import * as Tracer from "effect/Tracer"; import { createMcpHandler, DEFAULT_NEGOTIATED_PROTOCOL_VERSION, @@ -41,6 +41,7 @@ import { type McpResource, } from "@executor-js/host-mcp"; import { + parseTraceparentHeader, readArtifactsEnabled, readElicitationMode, verifiedMcpRequestHeaders, @@ -313,6 +314,15 @@ export abstract class McpAgentSessionDOBase< private modernRunningRequestCount = 0; private modernRequestBodies = new WeakMap(); private modernRequestPropagation = new WeakMap(); + // Trace context of the most recently entered MCP request, kept for + // `currentParentSpan`. Deliberately not cleared when the request settles: + // deferred SDK callbacks fire after the response resolves, and parenting + // them under the latest request is right far more often than falling back + // to the session's construction-time span. Concurrent requests on one DO + // can mis-parent to each other's trace (last writer wins); threading a + // per-request context through the MCP SDK's callback surface is the + // rejected-for-now alternative. + private lastIncomingTrace: IncomingTraceHeaders | undefined; private legacyRunningRequestCount = 0; private activeLegacyStreamCount = 0; private keepAliveCount = 0; @@ -381,8 +391,20 @@ export abstract class McpAgentSessionDOBase< return this.ctx.id.toString(); } + // Parent for spans opened by deferred MCP SDK callbacks (tool handlers run + // through `runToolEffect`, which otherwise inherits the Effect context + // captured when the server was BUILT). The legacy sessionful runtime builds + // its server once per session and reuses it across every request, so + // without this the whole tool execution tree parents under the long-closed + // session-construction span instead of the request that triggered it. protected currentParentSpan(): Tracer.AnySpan | undefined { - return undefined; + const parsed = parseTraceparentHeader(this.lastIncomingTrace?.traceparent); + if (!parsed) return undefined; + return Tracer.externalSpan({ + traceId: parsed.traceId, + spanId: parsed.spanId, + sampled: (parsed.traceFlags & 1) === 1, + }); } protected sessionTimeoutMs(): number { @@ -1244,6 +1266,11 @@ export abstract class McpAgentSessionDOBase< cors: false, }); } + this.lastIncomingTrace = { + traceparent: request.headers.get("traceparent") ?? undefined, + tracestate: request.headers.get("tracestate") ?? undefined, + baggage: request.headers.get("baggage") ?? undefined, + }; if ((await this.ctx.storage.get(DESTROY_PENDING_KEY)) === true) { return jsonRpcErrorBody(404, -32001, "Session timed out, please reconnect", { cors: false, @@ -1338,6 +1365,7 @@ export abstract class McpAgentSessionDOBase< this.modernRequestBodies.set(request, parsedBody); this.modernRequestPropagation.set(request, props.propagation); + this.lastIncomingTrace = props.propagation; this.modernRunningRequestCount += 1; // oxlint-disable-next-line executor/no-try-catch-or-throw -- adapter boundary: the RPC must decrement its in-memory running lease on both handler resolution and rejection try { diff --git a/packages/hosts/cloudflare/src/mcp/do-headers.ts b/packages/hosts/cloudflare/src/mcp/do-headers.ts index 36e62b2d47..1a20b44715 100644 --- a/packages/hosts/cloudflare/src/mcp/do-headers.ts +++ b/packages/hosts/cloudflare/src/mcp/do-headers.ts @@ -52,6 +52,33 @@ export type IncomingPropagationHeaders = { readonly baggage?: string; }; +// W3C traceparent grammar, owned here beside the producer that stamps the +// header (`currentPropagationHeaders`) so producer and consumers cannot +// drift. The worker edge's richer parser (apps/cloud, which also carries +// tracestate into the OTel API) delegates its grammar to this one. +const TRACEPARENT_PATTERN = /^([0-9a-f]{2})-([0-9a-f]{32})-([0-9a-f]{16})-([0-9a-f]{2})$/; + +export type ParsedTraceparent = { + readonly traceId: string; + readonly spanId: string; + readonly traceFlags: number; +}; + +export const parseTraceparentHeader = ( + traceparent: string | null | undefined, +): ParsedTraceparent | null => { + if (!traceparent) return null; + const match = TRACEPARENT_PATTERN.exec(traceparent); + if (!match) return null; + // SAFETY: the pattern has exactly four capture groups, so a match always + // fills indices 1-4. + return { + traceId: match[2]!, + spanId: match[3]!, + traceFlags: parseInt(match[4]!, 16), + }; +}; + // `currentParentSpan`, not `currentSpan`: the worker's MCP handler joins the // edge `http.server` span via `OtelTracer.withSpanContext`, which provides an // ExternalSpan as the fiber's ParentSpan without opening an Effect-local span. diff --git a/packages/hosts/mcp/src/tool-server-core.ts b/packages/hosts/mcp/src/tool-server-core.ts index 1d77f17130..a4cf5eab48 100644 --- a/packages/hosts/mcp/src/tool-server-core.ts +++ b/packages/hosts/mcp/src/tool-server-core.ts @@ -1146,9 +1146,7 @@ export const buildExecutorMcpTools = < ), ); - const assembly = yield* Effect.sync(createAssembly).pipe( - Effect.withSpan("mcp.host.create_server"), - ); + const assembly = yield* Effect.sync(createAssembly); const server = assembly.server; // Seeded from the host's persisted copy; a live `initialize` on this @@ -1483,10 +1481,6 @@ export const buildExecutorMcpTools = < }, ({ code }, extra) => runToolEffect(executeCode(code, extra)), ), - ).pipe( - Effect.withSpan("mcp.host.register_tool", { - attributes: { "mcp.tool.name": "execute" }, - }), ); yield* Effect.sync(() => @@ -1508,10 +1502,6 @@ export const buildExecutorMcpTools = < ({ name }) => runToolEffect(Effect.succeed(skillsResult(name, executeInventory, skillCatalog))), ), - ).pipe( - Effect.withSpan("mcp.host.register_tool", { - attributes: { "mcp.tool.name": "skills" }, - }), ); yield* Effect.sync(() => { @@ -1559,11 +1549,7 @@ export const buildExecutorMcpTools = < }, ({ executionId }, extra) => runToolEffect(resumeAfterBrowserApproval(executionId, extra)), ); - }).pipe( - Effect.withSpan("mcp.host.register_tool", { - attributes: { "mcp.tool.name": "resume" }, - }), - ); + }); // --- artifacts / MCP Apps --- // @@ -1942,11 +1928,7 @@ export const buildExecutorMcpTools = < ], }), ); - }).pipe( - Effect.withSpan("mcp.host.register_resource", { - attributes: { "mcp.resource.uri": MCP_APPS_SHELL_RESOURCE_URI }, - }), - ); + }); yield* Effect.sync(() => assembly.registerAppTool( @@ -2002,10 +1984,6 @@ export const buildExecutorMcpTools = < ({ code, title, description, connections, artifactId }) => runToolEffect(createArtifact({ code, title, description, connections, artifactId })), ), - ).pipe( - Effect.withSpan("mcp.host.register_tool", { - attributes: { "mcp.tool.name": "create-artifact" }, - }), ); yield* Effect.sync(() => @@ -2067,10 +2045,6 @@ export const buildExecutorMcpTools = < ({ artifactId, edits, connections, title, description }) => runToolEffect(editArtifact({ artifactId, edits, connections, title, description })), ), - ).pipe( - Effect.withSpan("mcp.host.register_tool", { - attributes: { "mcp.tool.name": "edit-artifact" }, - }), ); yield* Effect.sync(() => @@ -2085,10 +2059,6 @@ export const buildExecutorMcpTools = < }, () => runToolEffect(listArtifacts()), ), - ).pipe( - Effect.withSpan("mcp.host.register_tool", { - attributes: { "mcp.tool.name": "list-artifacts" }, - }), ); yield* Effect.sync(() => @@ -2109,10 +2079,6 @@ export const buildExecutorMcpTools = < }, ({ id }) => runToolEffect(showArtifact(id)), ), - ).pipe( - Effect.withSpan("mcp.host.register_tool", { - attributes: { "mcp.tool.name": "show-artifact" }, - }), ); yield* Effect.sync(() => { @@ -2163,11 +2129,7 @@ export const buildExecutorMcpTools = < resumeExecution(executionId, action, parseJsonContent(rawContent), extra), ), ); - }).pipe( - Effect.withSpan("mcp.host.register_tool", { - attributes: { "mcp.tool.name": "execute-action" }, - }), - ); + }); } // Client capabilities only exist after `initialize`, and `tools/list` is From a8d3d3c19218c561cef751943680d4d34873b095 Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Mon, 17 Aug 2026 11:38:07 -0600 Subject: [PATCH 04/11] Sign out browsers whose session has already ended (#1624) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test: repro sign-out answering with a raw Unauthorized page * fix: sign out browsers whose session has already ended Sign-out was declared on the session-authenticated API group, so a browser whose sealed session no longer authenticated at click time got a 401 from SessionAuth instead of being signed out. The console posts sign-out as a top-level form navigation, so that error body was rendered as the page: a screenful of {"_tag":"Unauthorized"}. Move the endpoint to the public group and read the sealed session off the request cookie. Nothing here needs authorizing — the request can only end the session whose cookie it presents — and the handler already fell back to "/" when the cookie would not unseal. Cookies are dropped only when the request actually presented one, so the now-public endpoint cannot be used cross-site to sign a user out. --- apps/cloud/src/auth/api.ts | 10 +- apps/cloud/src/auth/handlers.ts | 69 ++++++----- e2e/cloud/logout-stale-session.test.ts | 165 +++++++++++++++++++++++++ 3 files changed, 212 insertions(+), 32 deletions(-) create mode 100644 e2e/cloud/logout-stale-session.test.ts diff --git a/apps/cloud/src/auth/api.ts b/apps/cloud/src/auth/api.ts index 9d14c8d643..5c64be3c5c 100644 --- a/apps/cloud/src/auth/api.ts +++ b/apps/cloud/src/auth/api.ts @@ -181,6 +181,15 @@ const McpApprovalErrors = [ /** Public auth endpoints — no authentication required */ export class CloudAuthPublicApi extends HttpApiGroup.make("cloudAuthPublic") .add(HttpApiEndpoint.get("login", "/auth/login", { query: AuthLoginSearch })) + // Sign-out is PUBLIC on purpose. The console posts it as a top-level form + // navigation (the WorkOS hop is cross-origin, so it can't be a fetch), which + // means an error response is rendered as the page — and a browser whose + // session has already ended is exactly the browser most likely to click it + // (a second tab, a re-submit from history, an expired or revoked session). + // Behind SessionAuth all of those got a raw `{"_tag":"Unauthorized"}` screen + // instead of being signed out. There is nothing to authorize here anyway: + // the request can only end the session whose cookie it presents. + .add(HttpApiEndpoint.post("logout", "/auth/logout")) .add( HttpApiEndpoint.get("callback", "/auth/callback", { query: AuthCallbackSearch, @@ -201,7 +210,6 @@ export class CloudAuthApi extends HttpApiGroup.make("cloudAuth") error: AuthErrors, }), ) - .add(HttpApiEndpoint.post("logout", "/auth/logout")) .add( HttpApiEndpoint.get("organizations", "/auth/organizations", { success: AuthOrganizationsResponse, diff --git a/apps/cloud/src/auth/handlers.ts b/apps/cloud/src/auth/handlers.ts index d948787351..6445393b61 100644 --- a/apps/cloud/src/auth/handlers.ts +++ b/apps/cloud/src/auth/handlers.ts @@ -144,8 +144,8 @@ const deleteResponseCookie = (response: HttpServerResponse.HttpServerResponse, n HttpServerResponse.setCookieUnsafe(response, name, "", DELETE_COOKIE_OPTIONS); // --------------------------------------------------------------------------- -// Single non-protected API surface — public (login/callback) + session -// (me/logout/organizations/switch-organization). The session group has SessionAuth on it. +// Single non-protected API surface — public (login/callback/logout) + session +// (me/organizations/switch-organization). The session group has SessionAuth on it. // --------------------------------------------------------------------------- export const NonProtectedApi = HttpApi.make("cloudWeb").add(CloudAuthPublicApi).add(CloudAuthApi); @@ -274,6 +274,42 @@ export const CloudAuthPublicHandlers = HttpApiBuilder.group( ); }), ) + .handleRaw("logout", ({ request }) => + Effect.gen(function* () { + const workos = yield* WorkOSClient; + // The session this browser presents, NOT one the middleware vouched + // for — signing out of a session that has already ended must still + // sign the browser out (see the group declaration in ./api.ts). + const sealedSession = request.cookies["wos-session"] ?? ""; + + // WorkOS's documented sign-out: send the browser through the WorkOS + // logout endpoint, which ends the AuthKit session upstream and then + // redirects to the registered sign-out URL. Without this hop, the + // hosted session survives and the next "Sign in" silently + // re-authenticates (issue #1445). Fail-open when the cookie won't + // unseal — there is then nothing to end upstream, and local sign-out + // must still complete, so fall back to "/". + const origin = env.VITE_PUBLIC_SITE_URL ?? ""; + const logoutUrl = sealedSession + ? yield* workos.logoutUrl(sealedSession, origin ? `${origin}/` : undefined) + : null; + + const response = HttpServerResponse.redirect(logoutUrl ?? "/", { status: 302 }); + + // Drop only what this browser actually presented. Both cookies are + // SameSite=Lax, so a cross-site form POST carries neither — it gets + // the bare redirect and cannot be used to sign anyone out. + if (!sealedSession && request.cookies[AUTH_HINT_COOKIE] === undefined) return response; + + // The auth-hint travels with the session: leaving it behind would + // make the next page load optimistically paint the app shell for a + // signed-out browser. + return deleteResponseCookie( + deleteResponseCookie(response, "wos-session"), + AUTH_HINT_COOKIE, + ); + }), + ) // CLI device-login discovery. The WorkOS device endpoints live on the // WorkOS API host (`WORKOS_API_URL`, or api.workos.com in production, // the SAME base the SDK uses, so e2e points the CLI at the emulator with @@ -319,35 +355,6 @@ export const CloudSessionAuthHandlers = HttpApiBuilder.group( }; }), ) - .handleRaw("logout", () => - Effect.gen(function* () { - const workos = yield* WorkOSClient; - const session = yield* SessionContext; - - // WorkOS's documented sign-out: send the browser through the WorkOS - // logout endpoint, which ends the AuthKit session upstream and then - // redirects to the registered sign-out URL. Without this hop, the - // hosted session survives and the next "Sign in" silently - // re-authenticates (issue #1445). Fail-open when the cookie won't - // unseal: local sign-out must still complete, so fall back to "/". - const origin = env.VITE_PUBLIC_SITE_URL ?? ""; - const logoutUrl = yield* workos.logoutUrl( - session.sealedSession, - origin ? `${origin}/` : undefined, - ); - - // The auth-hint travels with the session: leaving it behind would - // make the next page load optimistically paint the app shell for a - // signed-out browser. - return deleteResponseCookie( - deleteResponseCookie( - HttpServerResponse.redirect(logoutUrl ?? "/", { status: 302 }), - "wos-session", - ), - AUTH_HINT_COOKIE, - ); - }), - ) .handle("organizations", () => Effect.gen(function* () { const workos = yield* WorkOSClient; diff --git a/e2e/cloud/logout-stale-session.test.ts b/e2e/cloud/logout-stale-session.test.ts new file mode 100644 index 0000000000..8093cc24d2 --- /dev/null +++ b/e2e/cloud/logout-stale-session.test.ts @@ -0,0 +1,165 @@ +// Cloud-only: signing out must ALWAYS sign the browser out. The shell's +// sign-out is a top-level form POST (the WorkOS hop is cross-origin, so it +// can't be a fetch), which means whatever `/api/auth/logout` answers is +// rendered AS THE PAGE. An error response is therefore not a silent failure — +// it is a screenful of raw API JSON where the homepage should be. Reported +// from the field as `{"_tag":"Unauthorized"}`. +// +// The endpoint used to sit behind SessionAuth, so a browser whose sealed +// session no longer authenticated AT THE MOMENT OF THE CLICK got the +// middleware's 401 instead of being signed out. Reaching that state needs +// nothing exotic: a second open tab does it (sign out in one, the other's +// shell is still painted but its cookie is gone), and an expired or +// upstream-revoked session lands on the same branch. Signing out of a session +// that is already over is the one request that must never fail — the user is +// asking for the state they are already in. +import { expect } from "@effect/vitest"; +import { Effect } from "effect"; +import type { Page } from "playwright"; + +import { scenario } from "../src/scenario"; +import { Api, Browser, Target } from "../src/services"; + +/** The display-only identity cookie the SSR gate mints (non-HttpOnly). */ +const HINT_COOKIE = "executor-auth-hint"; + +/** First `Set-Cookie` header for `name`, as the raw header string. */ +const setCookieFor = (response: Response, name: string): string => { + for (const header of response.headers.getSetCookie()) { + if (header.startsWith(`${name}=`)) return header; + } + return ""; +}; + +// Open the account dropdown and sign out. Bounded retry: a click can land +// while the shell is still hydrating and the radix menu never opens (same +// idiom as auth-hint / org-switcher). +const clickSignOut = async (page: Page) => { + for (let attempt = 1; ; attempt++) { + try { + await page.keyboard.press("Escape"); + await page.getByRole("button", { name: /Test User/ }).click(); + await page.getByRole("menuitem", { name: "Sign out" }).click({ timeout: 5_000 }); + return; + } catch (error) { + if (attempt >= 3) throw error; + } + } +}; + +scenario( + "Sign-out · a browser whose session is already over is signed out, not handed an API error page", + {}, + Effect.gen(function* () { + // Gate: the REST API plane is mounted on this target. + yield* Api; + const target = yield* Target; + + const signOut = (cookie?: string) => + Effect.promise(() => + fetch(new URL("/api/auth/logout", target.baseUrl), { + method: "POST", + redirect: "manual", + ...(cookie ? { headers: { cookie } } : {}), + }), + ); + + // A session cookie that no longer authenticates — expired, or revoked + // upstream while the tab sat open. There is nothing to end at WorkOS, but + // this browser must still leave signed out: a stale session cookie left in + // place keeps routing / into the app instead of marketing. + const stale = yield* signOut("wos-session=stale-sealed-session"); + const staleBody = yield* Effect.promise(() => stale.text()); + + // The response IS the page the user reads. This is the reported symptom. + expect(staleBody, "sign-out never renders an API error envelope").not.toContain("_tag"); + expect(stale.status, "sign-out sends the browser onward").toBe(302); + expect( + new URL(stale.headers.get("location") ?? "", target.baseUrl).pathname, + "sign-out lands the browser home", + ).toBe("/"); + expect(setCookieFor(stale, "wos-session"), "the dead session cookie is dropped").toContain( + "Max-Age=0", + ); + expect( + setCookieFor(stale, HINT_COOKIE), + "the auth hint never outlives the session it describes", + ).toContain("Max-Age=0"); + + // A POST carrying no cookies at all is what a CROSS-SITE form reaching + // this endpoint looks like: both cookies are SameSite=Lax, so another + // origin's POST arrives bare. Signing out is public, so it must answer + // such a request without touching this browser's session — otherwise any + // site could sign our users out. + const bare = yield* signOut(); + const bareBody = yield* Effect.promise(() => bare.text()); + + expect(bareBody, "a session-less sign-out renders no error envelope either").not.toContain( + "_tag", + ); + expect(bare.status, "it is sent home like any other sign-out").toBe(302); + expect( + new URL(bare.headers.get("location") ?? "", target.baseUrl).pathname, + "it lands home", + ).toBe("/"); + expect( + bare.headers.getSetCookie(), + "a request that presented no session cannot expire anyone else's", + ).toEqual([]); + }), +); + +scenario( + "Sign-out · a second open tab signs out too instead of showing raw JSON", + {}, + Effect.gen(function* () { + const browser = yield* Browser; + const target = yield* Target; + const identity = yield* target.newIdentity(); + + // `page` is the tab the user is LOOKING at when it breaks — the run's + // video and screenshots follow it, so the artifacts show the reported + // symptom rather than the tab that already signed out cleanly. + yield* browser.session(identity, async ({ page, step }) => { + const otherTab = await page.context().newPage(); + + await step("The user has the app open in two tabs", async () => { + await page.goto("/", { waitUntil: "commit" }); + await page.getByRole("link", { name: "Policies" }).waitFor(); + await otherTab.goto("/", { waitUntil: "commit" }); + await otherTab.getByRole("link", { name: "Policies" }).waitFor(); + }); + + await step("Sign out in the other tab", async () => { + await clickSignOut(otherTab); + await otherTab.waitForURL((url) => url.pathname === "/" || url.pathname === "/login", { + timeout: 15_000, + }); + }); + + // This tab was never told: its shell is still painted, and its sign-out + // button still works the only way it knows how. + const before = page.url(); + await step("Sign out in this tab, which still shows the app", async () => { + await clickSignOut(page); + await page.waitForURL((url) => url.toString() !== before, { + timeout: 15_000, + }); + await page.waitForLoadState("networkidle"); + }); + + const shown = (await page.locator("body").innerText()).trim(); + expect(shown, "sign-out never renders the API's error envelope as a page").not.toContain( + "_tag", + ); + expect( + new URL(page.url()).pathname, + "this tab lands on a signed-out page like the other one", + ).toMatch(/^\/(login)?$/); + expect( + (await page.context().cookies()).map((cookie) => cookie.name), + "the browser is left with no session either way", + ).not.toContain("wos-session"); + }); + }), +); From f3ec48d495ff6b2ee8fcbc6c7016ababa12ff244 Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Mon, 17 Aug 2026 11:55:13 -0600 Subject: [PATCH 05/11] Answer dead-session standalone GETs with 405 to stop reconnect loops (#1622) --- apps/cloud/src/mcp/agent-handler.ts | 27 ++++++++++++-- apps/host-cloudflare/src/mcp/agent-handler.ts | 25 +++++++++++-- .../mcp/agent-session-durable-object.test.ts | 28 ++++++++++++++ .../src/mcp/agent-session-durable-object.ts | 22 ++++++++++- packages/hosts/mcp/src/envelope.test.ts | 37 +++++++++++++++++++ packages/hosts/mcp/src/envelope.ts | 32 +++++++++++++--- 6 files changed, 156 insertions(+), 15 deletions(-) diff --git a/apps/cloud/src/mcp/agent-handler.ts b/apps/cloud/src/mcp/agent-handler.ts index c4abd5afa2..a2e7a92404 100644 --- a/apps/cloud/src/mcp/agent-handler.ts +++ b/apps/cloud/src/mcp/agent-handler.ts @@ -45,6 +45,25 @@ const jsonRpcResponse = ( ? jsonRpcErrorBody(status, code, message) : jsonRpcErrorBody(status, code, message, { challenge }); +/** + * A dead session id answers by request method. POST/DELETE keep the 404 that + * tells a compliant client to re-initialize. A standalone GET gets 405: the + * v1 SDK treats that as "no SSE stream offered" and stops retrying quietly, + * which breaks the reconnect loops of pre-cutover always-on deployments — + * their GET-404 path never re-initialized, it just retried forever. + */ +const deadSessionResponse = (method: string, message: string): Response => + method === "GET" + ? new Response(JSON.stringify({ jsonrpc: "2.0", error: { code: -32001, message }, id: null }), { + status: 405, + headers: { + "content-type": "application/json", + allow: "POST, DELETE", + "access-control-allow-origin": "*", + }, + }) + : jsonRpcResponse(404, -32001, message); + const renderAuthError = ( auth: McpAuthProvider["Service"], request: Request, @@ -217,7 +236,7 @@ export const makeCloudMcpAgentHandler = () => { const existingSession = sessionId ? mcpSessionStub(env.MCP_SESSION, sessionId) : null; if (sessionId && !existingSession) { - return jsonRpcResponse(404, -32001, "Session not found"); + return deadSessionResponse(request.method, "Session not found"); } if (existingSession) { const owner = await existingSession.validateMcpSessionOwner({ @@ -225,13 +244,13 @@ export const makeCloudMcpAgentHandler = () => { organizationId: outcome.principal.organizationId, }); if (owner === "not_found") { - return jsonRpcResponse(404, -32001, "Session not found"); + return deadSessionResponse(request.method, "Session not found"); } if (owner === "terminated") { // DELETE-condemned but the deferred destroy alarm hasn't wiped storage // yet. Same envelope as the post-destroy race below: the client must // treat the id as dead and reconnect. - return jsonRpcResponse(404, -32001, "Session timed out, please reconnect"); + return deadSessionResponse(request.method, "Session timed out, please reconnect"); } if (owner === "forbidden") { return jsonRpcResponse(403, -32003, "MCP session does not belong to the current bearer"); @@ -267,7 +286,7 @@ export const makeCloudMcpAgentHandler = () => { // client to be told to reconnect, matching a timed-out session). // oxlint-disable-next-line executor/no-unknown-error-message -- adapter boundary: the abort reason is a plain runtime Error whose message IS the signal if (Predicate.isError(error) && error.message === "destroyed") { - return jsonRpcResponse(404, -32001, "Session timed out, please reconnect"); + return deadSessionResponse(request.method, "Session timed out, please reconnect"); } // oxlint-disable-next-line executor/no-try-catch-or-throw -- adapter boundary: rethrow anything that isn't the condemned-DO abort to the Workers runtime unchanged throw error; diff --git a/apps/host-cloudflare/src/mcp/agent-handler.ts b/apps/host-cloudflare/src/mcp/agent-handler.ts index 3a729f86c9..277d34542a 100644 --- a/apps/host-cloudflare/src/mcp/agent-handler.ts +++ b/apps/host-cloudflare/src/mcp/agent-handler.ts @@ -41,6 +41,25 @@ const jsonRpcResponse = ( ? jsonRpcErrorBody(status, code, message) : jsonRpcErrorBody(status, code, message, { challenge }); +/** + * A dead session id answers by request method. POST/DELETE keep the 404 that + * tells a compliant client to re-initialize. A standalone GET gets 405: the + * v1 SDK treats that as "no SSE stream offered" and stops retrying quietly, + * which breaks the reconnect loops of pre-cutover always-on deployments — + * their GET-404 path never re-initialized, it just retried forever. + */ +const deadSessionResponse = (method: string, message: string): Response => + method === "GET" + ? new Response(JSON.stringify({ jsonrpc: "2.0", error: { code: -32001, message }, id: null }), { + status: 405, + headers: { + "content-type": "application/json", + allow: "POST, DELETE", + "access-control-allow-origin": "*", + }, + }) + : jsonRpcResponse(404, -32001, message); + const renderAuthError = ( auth: McpAuthProvider["Service"], request: Request, @@ -145,7 +164,7 @@ export const makeCloudflareMcpAgentHandler = (config: CloudflareConfig) => { const existingSession = sessionId ? mcpSessionStub(env.MCP_SESSION, sessionId) : null; if (sessionId && !existingSession) { - return jsonRpcResponse(404, -32001, "Session not found"); + return deadSessionResponse(request.method, "Session not found"); } if (existingSession) { const owner = await existingSession.validateMcpSessionOwner({ @@ -153,12 +172,12 @@ export const makeCloudflareMcpAgentHandler = (config: CloudflareConfig) => { organizationId: outcome.principal.organizationId, }); if (owner === "not_found") { - return jsonRpcResponse(404, -32001, "Session not found"); + return deadSessionResponse(request.method, "Session not found"); } if (owner === "terminated") { // DELETE-condemned but the deferred destroy alarm hasn't wiped storage // yet; the terminated id must read as dead immediately. - return jsonRpcResponse(404, -32001, "Session timed out, please reconnect"); + return deadSessionResponse(request.method, "Session timed out, please reconnect"); } if (owner === "forbidden") { return jsonRpcResponse(403, -32003, "MCP session does not belong to the current bearer"); diff --git a/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.test.ts b/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.test.ts index 100068f7e8..5fec25ea0a 100644 --- a/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.test.ts +++ b/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.test.ts @@ -576,4 +576,32 @@ describe("McpAgentSessionDOBase session serving", () => { error: { code: -32001, message: "Session not found" }, }); }); + + it("answers a dead-session standalone GET with 405 so old clients stop retrying", async () => { + const state = new MemoryDurableObjectState(); + await state.storage.put("session-meta", { + organizationId: ORGANIZATION_ID, + organizationName: "Old Agent Org", + userId: ACCOUNT_ID, + resource: defaultMcpResource, + } satisfies SessionMeta); + const session = new HarnessSession(state, {} as Cloudflare.Env); + const request = verifiedRequest( + new Request("https://executor.test/mcp", { + method: "GET", + headers: { + accept: "text/event-stream", + "mcp-session-id": SESSION_ID, + }, + }), + ); + + const response = await session.fetch(request); + + expect(response.status).toBe(405); + expect(response.headers.get("allow")).toBe("POST, DELETE"); + await expect(response.json()).resolves.toMatchObject({ + error: { code: -32001, message: "Session not found" }, + }); + }); }); diff --git a/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.ts b/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.ts index 937b9928d7..a27213eaf4 100644 --- a/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.ts +++ b/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.ts @@ -298,6 +298,24 @@ type QueuedTransportMessage = { readonly extra?: MessageExtraInfo; }; +/** + * Dead-session answer by method: POST/DELETE keep the 404 that drives client + * re-initialization; a standalone GET gets 405, which the v1 SDK reads as + * "no SSE stream offered" and stops retrying — breaking the reconnect loops + * of pre-cutover deployments whose GET-404 path never re-initialized. + */ +const deadSessionDoResponse = (method: string): Response => + method === "GET" + ? new Response( + JSON.stringify({ + jsonrpc: "2.0", + error: { code: -32001, message: "Session not found" }, + id: null, + }), + { status: 405, headers: { "content-type": "application/json", allow: "POST, DELETE" } }, + ) + : jsonRpcErrorBody(404, -32001, "Session not found", { cors: false }); + export abstract class McpAgentSessionDOBase< Env extends Cloudflare.Env = Cloudflare.Env, TDbHandle extends SessionDbHandle = SessionDbHandle, @@ -1182,7 +1200,7 @@ export abstract class McpAgentSessionDOBase< return this.serializedTransportRequest(async () => { const transport = this.transport; if (!transport) { - return jsonRpcErrorBody(404, -32001, "Session not found", { cors: false }); + return deadSessionDoResponse(request.method); } if (request.method === "GET") { const lastEventId = request.headers.get("last-event-id"); @@ -1282,7 +1300,7 @@ export abstract class McpAgentSessionDOBase< if (!stored) { if (!isInitializeBody(parsedBody)) { return request.headers.has("mcp-session-id") - ? jsonRpcErrorBody(404, -32001, "Session not found", { cors: false }) + ? deadSessionDoResponse(request.method) : jsonRpcErrorBody(400, -32000, "Bad Request: Server not initialized", { cors: false, }); diff --git a/packages/hosts/mcp/src/envelope.test.ts b/packages/hosts/mcp/src/envelope.test.ts index 90c0a38e8b..65e2828376 100644 --- a/packages/hosts/mcp/src/envelope.test.ts +++ b/packages/hosts/mcp/src/envelope.test.ts @@ -252,6 +252,43 @@ describe("McpServingRoutes envelope", () => { }); }); + it("answers a dead-session standalone GET with 405 so old clients stop retrying", async () => { + const NotFoundStoreLive = Layer.succeed(McpSessionStore)({ + dispatch: (): Effect.Effect => Effect.succeed("not-found"), + dispose: () => Effect.void, + }); + const handler = buildHandler(NotFoundStoreLive, McpErrorReporterNoop); + + const get = await handler( + new Request("https://host.test/mcp", { + method: "GET", + headers: { + authorization: "Bearer x", + accept: "text/event-stream", + "mcp-session-id": "dead-session", + "mcp-protocol-version": "2025-06-18", + }, + }), + ); + expect(get.status).toBe(405); + expect(get.headers.get("allow")).toBe("POST, DELETE"); + expect(await get.json()).toMatchObject({ error: { code: -32001 } }); + + const post = await handler( + new Request("https://host.test/mcp", { + method: "POST", + headers: { + authorization: "Bearer x", + "content-type": "application/json", + "mcp-session-id": "dead-session", + "mcp-protocol-version": "2025-06-18", + }, + body: JSON.stringify({ jsonrpc: "2.0", id: 1, method: "tools/list" }), + }), + ); + expect(post.status).toBe(404); + }); + it("404s a modern request whose toolkit route is not served", async () => { const handler = buildHandler(OkStoreLive, McpErrorReporterNoop); const response = await handler(modernRequest("https://host.test/mcp/toolkits/unknown/extra")); diff --git a/packages/hosts/mcp/src/envelope.ts b/packages/hosts/mcp/src/envelope.ts index d0db0e8d74..a32fd8213e 100644 --- a/packages/hosts/mcp/src/envelope.ts +++ b/packages/hosts/mcp/src/envelope.ts @@ -238,11 +238,29 @@ const renderAuthError = ( Match.exhaustive, ); -/** Render a non-`Response` {@link McpDispatchResult} discriminant. */ -const renderDispatchError = (lookup: "not-found" | "forbidden"): Response => - lookup === "not-found" - ? jsonRpcResponse(404, -32001, "Session not found") - : jsonRpcResponse(403, -32003, "MCP session does not belong to the current bearer"); +/** + * Render a non-`Response` {@link McpDispatchResult} discriminant. A dead + * session answers by method: POST/DELETE keep the 404 that drives client + * re-initialization; a standalone GET gets 405, which the v1 SDK reads as + * "no SSE stream offered" and stops retrying — breaking pre-cutover + * reconnect loops whose GET-404 path never re-initialized. + */ +const renderDispatchError = (lookup: "not-found" | "forbidden", method: string): Response => { + if (lookup === "forbidden") { + return jsonRpcResponse(403, -32003, "MCP session does not belong to the current bearer"); + } + if (method === "GET") { + return new Response( + JSON.stringify({ + jsonrpc: "2.0", + error: { code: -32001, message: "Session not found" }, + id: null, + }), + { status: 405, headers: { "content-type": "application/json", allow: "POST, DELETE" } }, + ); + } + return jsonRpcResponse(404, -32001, "Session not found"); +}; const withModernMcpCors = (response: Response): Response => { const headers = new Headers(response.headers); @@ -398,7 +416,9 @@ const mcpDispatch = (resource: McpResource, modern: ModernMcpRouter) => sessionId, method: request.method, }); - return fromWebResponse(result instanceof Response ? result : renderDispatchError(result)); + return fromWebResponse( + result instanceof Response ? result : renderDispatchError(result, request.method), + ); }); /** From dd8aab7cb19e348a4ad6f71d2e0251184f779560 Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Mon, 17 Aug 2026 12:34:50 -0600 Subject: [PATCH 06/11] Pin worker logpush in wrangler config (#1625) The account Logpush job exports workers_trace_events to Axiom, but the script-level logpush flag was lost in the July deploy migration and the export silently stopped. Pin it in config so deploys preserve it; the flag was re-enabled via API today. --- apps/cloud/wrangler.jsonc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/apps/cloud/wrangler.jsonc b/apps/cloud/wrangler.jsonc index ac00660556..a38224c957 100644 --- a/apps/cloud/wrangler.jsonc +++ b/apps/cloud/wrangler.jsonc @@ -21,6 +21,12 @@ "observability": { "enabled": true, }, + // Script-level logpush feeds the account's workers_trace_events Logpush job + // (invocation logs, outcomes like exceededMemory, console output) into + // Axiom. Pinned here because the setting lives on the script: a deploy that + // omits it can reset it to false, which is how the export silently died in + // the July deploy-tooling migration. + "logpush": true, "durable_objects": { "bindings": [ { From d3f0617deec06c57e0d6e1479fe668f79daf977d Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Mon, 17 Aug 2026 12:38:01 -0600 Subject: [PATCH 07/11] Bind span header redaction to the hosted HTTP client (#1626) * Bind span header redaction to the hosted HTTP client Live verification after the telemetry-layer redaction shipped showed post-deploy http.client spans still recording non-allowlisted headers: consumers capture the client value at construction and execute requests on fibers a host telemetry layer never reaches, so providing the redaction reference by layer was not enough. Bind it to the client itself so every request effect carries it, and keep the cloud layer as the server-side half. Regression test drives a request through the real client and asserts on the recorded span attributes. * Add DOM.Iterable to the mcp plugin tsconfig The package's tsc build failed on Headers.entries() and the iterable protocol: its lib had DOM without DOM.Iterable, so the fetch Headers type lacked both. Latent on main; every CI run served the package build from the turbo cache until the sdk change invalidated it. Same lib pairing hosts/cloudflare already uses. --- .../hosted-client-span-header-redaction.md | 5 ++ .../src/observability/header-redaction.ts | 66 +++-------------- packages/core/sdk/src/host-internal.ts | 1 + .../core/sdk/src/hosted-http-client.test.ts | 68 +++++++++++++++++ packages/core/sdk/src/hosted-http-client.ts | 73 ++++++++++++++++++- packages/plugins/mcp/tsconfig.json | 2 +- 6 files changed, 156 insertions(+), 59 deletions(-) create mode 100644 .changeset/hosted-client-span-header-redaction.md diff --git a/.changeset/hosted-client-span-header-redaction.md b/.changeset/hosted-client-span-header-redaction.md new file mode 100644 index 0000000000..ac53f0f276 --- /dev/null +++ b/.changeset/hosted-client-span-header-redaction.md @@ -0,0 +1,5 @@ +--- +"@executor-js/sdk": patch +--- + +Redact every span header attribute outside a safe allowlist on the hosted HTTP client. The tracer's default four-name blocklist let provider-specific credential headers reach the trace backend verbatim; the hosted client now inverts the model and masks everything except structurally safe negotiation, caching, and tracing headers. diff --git a/apps/cloud/src/observability/header-redaction.ts b/apps/cloud/src/observability/header-redaction.ts index 188046682c..8c7fd3fc08 100644 --- a/apps/cloud/src/observability/header-redaction.ts +++ b/apps/cloud/src/observability/header-redaction.ts @@ -1,65 +1,19 @@ // --------------------------------------------------------------------------- -// Span header redaction. -// -// Effect's HttpClient/HttpServer tracer records every request and response -// header as a span attribute, masking only the names in -// `Headers.CurrentRedactedNames` (default: authorization, cookie, set-cookie, -// x-api-key). That blocklist can never be right here: executor's whole job is -// calling arbitrary upstream APIs whose credentials ride in provider-specific -// headers (x-goog-api-key, api-key, x-auth-token, ...), and any name missing -// from the list ships a live secret to the trace backend verbatim. -// -// So the override inverts the model: every header is redacted unless its name -// is on the allowlist of structurally safe, diagnostically useful headers. -// The alternative of enumerating known secret-bearing names was rejected: new -// integrations add new credential headers faster than a blocklist can learn -// them, and one miss is a leaked customer credential. +// Span header redaction for fibers composed under the cloud telemetry layers +// (the Effect HTTP server tracer and any request-scoped client use). The +// allowlist itself lives beside the hosted HTTP client in +// `@executor-js/sdk/host-internal`, which also binds it directly to that +// client: consumers capture the client value at construction, so a context +// layer alone cannot reach their request fibers. This layer is the +// server-side/defense-in-depth half of the same decision; see +// hosted-http-client.ts for the rationale and the allowlist. // --------------------------------------------------------------------------- import { Layer } from "effect"; import { Headers } from "effect/unstable/http"; - -// Names that never carry credentials and are worth reading on a span while -// debugging: negotiation, caching, routing, and the tracing headers -// themselves. Everything else renders as ``. -const SAFE_HEADER_NAMES = [ - "accept", - "accept-encoding", - "accept-language", - "age", - "cache-control", - "connection", - "content-encoding", - "content-length", - "content-type", - "date", - "etag", - "expires", - "host", - "if-modified-since", - "if-none-match", - "last-modified", - "location", - "mcp-protocol-version", - "mcp-session-id", - "origin", - "referer", - "retry-after", - "traceparent", - "tracestate", - "transfer-encoding", - "user-agent", - "vary", - "via", - "x-request-id", -] as const; - -// Header names are lowercased at `Headers` construction, and `Headers.redact` -// masks every name a RegExp entry matches. One negative lookahead turns the -// allowlist into "redact everything else". -const redactAllButSafe = new RegExp(`^(?!(?:${SAFE_HEADER_NAMES.join("|")})$)`); +import { spanRedactedHeaderNames } from "@executor-js/sdk/host-internal"; export const SpanHeaderRedactionLive: Layer.Layer = Layer.succeed( Headers.CurrentRedactedNames, - [redactAllButSafe], + spanRedactedHeaderNames, ); diff --git a/packages/core/sdk/src/host-internal.ts b/packages/core/sdk/src/host-internal.ts index e62cb15bbb..834e0e4e81 100644 --- a/packages/core/sdk/src/host-internal.ts +++ b/packages/core/sdk/src/host-internal.ts @@ -33,6 +33,7 @@ export { HostedOutboundRequestBlocked, makeHostedFetch, makeHostedHttpClientLayer, + spanRedactedHeaderNames, type HostedHttpClientOptions, } from "./hosted-http-client"; diff --git a/packages/core/sdk/src/hosted-http-client.test.ts b/packages/core/sdk/src/hosted-http-client.test.ts index 61f4590e08..bdf553f8f1 100644 --- a/packages/core/sdk/src/hosted-http-client.test.ts +++ b/packages/core/sdk/src/hosted-http-client.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from "@effect/vitest"; import { Effect, Predicate, Result } from "effect"; +import type * as Tracer from "effect/Tracer"; import { HttpClient, HttpClientRequest } from "effect/unstable/http"; import { @@ -266,4 +267,71 @@ describe("hosted outbound HTTP client", () => { expect(calls).toBe(1); }), ); + + it.effect("redacts every span header attribute outside the safe allowlist", () => + Effect.gen(function* () { + const spanAttributes = new Map(); + const recordingTracer: Tracer.Tracer = { + span: (options) => { + let status: Tracer.SpanStatus = { _tag: "Started", startTime: options.startTime }; + return { + _tag: "Span", + name: options.name, + spanId: "0000000000000001", + traceId: "00000000000000000000000000000001", + parent: options.parent, + annotations: options.annotations, + get status() { + return status; + }, + attributes: spanAttributes, + links: options.links, + sampled: options.sampled, + kind: options.kind, + end: (endTime, exit) => { + status = { _tag: "Ended", startTime: options.startTime, endTime, exit }; + }, + attribute: (key, value) => { + spanAttributes.set(key, value); + }, + event: () => undefined, + addLinks: () => undefined, + }; + }, + }; + + const fakeFetch: typeof globalThis.fetch = (async () => + new Response("{}", { + status: 200, + headers: { + "content-type": "application/json", + "cf-ray": "a2ca5b47bb7f3550", + }, + })) as typeof globalThis.fetch; + + yield* Effect.gen(function* () { + const client = yield* HttpClient.HttpClient; + return yield* client.execute( + HttpClientRequest.get("https://api.example/data").pipe( + HttpClientRequest.setHeaders({ + accept: "application/json", + "x-goog-api-key": "live-credential", + }), + ), + ); + }).pipe( + Effect.provide( + makeHostedHttpClientLayer({ fetch: fakeFetch, resolveHostname: publicResolver }), + ), + Effect.withTracer(recordingTracer), + ); + + expect(String(spanAttributes.get("http.request.header.accept"))).toBe("application/json"); + expect(String(spanAttributes.get("http.request.header.x-goog-api-key"))).toBe(""); + expect(String(spanAttributes.get("http.response.header.content-type"))).toBe( + "application/json", + ); + expect(String(spanAttributes.get("http.response.header.cf-ray"))).toBe(""); + }), + ); }); diff --git a/packages/core/sdk/src/hosted-http-client.ts b/packages/core/sdk/src/hosted-http-client.ts index 37f1ea427c..25c226078c 100644 --- a/packages/core/sdk/src/hosted-http-client.ts +++ b/packages/core/sdk/src/hosted-http-client.ts @@ -1,5 +1,5 @@ import { Effect, Layer, Schema } from "effect"; -import { FetchHttpClient, HttpClient } from "effect/unstable/http"; +import { FetchHttpClient, Headers as HttpHeaders, HttpClient } from "effect/unstable/http"; export class HostedOutboundRequestBlocked extends Schema.TaggedErrorClass()( "HostedOutboundRequestBlocked", @@ -250,10 +250,79 @@ export const makeHostedFetch = (options: HostedHttpClientOptions = {}): typeof g // oxlint-disable-next-line executor/no-raw-fetch -- boundary: exposes a guarded Fetch API adapter for libraries that require fetch guardFetch(options.fetch ?? globalThis.fetch, options); +// --------------------------------------------------------------------------- +// Span header redaction. +// +// The HttpClient tracer records every request and response header as a span +// attribute, masking only the names in `Headers.CurrentRedactedNames` +// (default: authorization, cookie, set-cookie, x-api-key). That blocklist can +// never be right for a hosted client whose whole job is calling arbitrary +// upstream APIs: credentials ride in provider-specific headers +// (x-goog-api-key, api-key, x-auth-token, ...), and any name missing from the +// list ships a live secret to the trace backend verbatim. So the model is +// inverted: every header is redacted unless its name is on the allowlist of +// structurally safe, diagnostically useful headers. Enumerating known +// secret-bearing names was rejected: new integrations add credential headers +// faster than a blocklist can learn them, and one miss is a leaked customer +// credential. +// +// Bound to the client itself (not a host telemetry layer): consumers capture +// the client value at construction and execute requests on fibers whose +// context a host layer never sees, so the reference must travel with every +// request effect. +// --------------------------------------------------------------------------- + +const SPAN_SAFE_HEADER_NAMES = [ + "accept", + "accept-encoding", + "accept-language", + "age", + "cache-control", + "connection", + "content-encoding", + "content-length", + "content-type", + "date", + "etag", + "expires", + "host", + "if-modified-since", + "if-none-match", + "last-modified", + "location", + "mcp-protocol-version", + "mcp-session-id", + "origin", + "referer", + "retry-after", + "traceparent", + "tracestate", + "transfer-encoding", + "user-agent", + "vary", + "via", + "x-request-id", +] as const; + +// Header names are lowercased at `Headers` construction, and `Headers.redact` +// masks every name a RegExp entry matches. One negative lookahead turns the +// allowlist into "redact everything else". +export const spanRedactedHeaderNames: readonly (string | RegExp)[] = [ + new RegExp(`^(?!(?:${SPAN_SAFE_HEADER_NAMES.join("|")})$)`), +]; + +const withSpanHeaderRedaction = (client: HttpClient.HttpClient): HttpClient.HttpClient => + HttpClient.transformResponse(client, (effect) => + Effect.provideService(effect, HttpHeaders.CurrentRedactedNames, spanRedactedHeaderNames), + ); + export const makeHostedHttpClientLayer = ( options: HostedHttpClientOptions = {}, ): Layer.Layer => - FetchHttpClient.layer.pipe( + Layer.effect(HttpClient.HttpClient)( + Effect.map(Effect.service(HttpClient.HttpClient), withSpanHeaderRedaction), + ).pipe( + Layer.provide(FetchHttpClient.layer), Layer.provide( options.fetch ? Layer.succeed(FetchHttpClient.Fetch)(guardFetch(options.fetch, options)) diff --git a/packages/plugins/mcp/tsconfig.json b/packages/plugins/mcp/tsconfig.json index c9ad068608..1a31057f9d 100644 --- a/packages/plugins/mcp/tsconfig.json +++ b/packages/plugins/mcp/tsconfig.json @@ -5,7 +5,7 @@ "moduleResolution": "Bundler", "strict": true, "skipLibCheck": true, - "lib": ["ES2022", "DOM"], + "lib": ["ES2022", "DOM", "DOM.Iterable"], "types": ["bun-types", "node"], "noUnusedLocals": true, "noImplicitOverride": true, From 2ea74946cbbb2b9ca6bb8df4d8a5b7f54522b4f0 Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Mon, 17 Aug 2026 12:53:06 -0600 Subject: [PATCH 08/11] Negative-cache dead MCP session ids in the workers (#1627) --- apps/cloud/src/mcp/agent-handler.test.ts | 146 +++++++++++++++++ apps/cloud/src/mcp/agent-handler.ts | 100 ++++++++++-- apps/cloud/src/server.ts | 151 +++++++++-------- apps/host-cloudflare/src/app.ts | 5 +- .../src/mcp/agent-handler.test.ts | 153 ++++++++++++++++++ apps/host-cloudflare/src/mcp/agent-handler.ts | 88 +++++++++- .../test-stubs/cloudflare-workers.ts | 7 + apps/host-cloudflare/vitest.config.ts | 7 + 8 files changed, 571 insertions(+), 86 deletions(-) create mode 100644 apps/cloud/src/mcp/agent-handler.test.ts create mode 100644 apps/host-cloudflare/src/mcp/agent-handler.test.ts create mode 100644 apps/host-cloudflare/test-stubs/cloudflare-workers.ts diff --git a/apps/cloud/src/mcp/agent-handler.test.ts b/apps/cloud/src/mcp/agent-handler.test.ts new file mode 100644 index 0000000000..0f6e539a53 --- /dev/null +++ b/apps/cloud/src/mcp/agent-handler.test.ts @@ -0,0 +1,146 @@ +import { beforeEach, describe, expect, it, vi } from "@effect/vitest"; +import { Effect, Layer } from "effect"; + +import { + authenticated, + McpAuthProvider, + unauthorized, + type Principal, +} from "@executor-js/host-mcp"; + +import { cloudDeadSessionCacheForTest, makeCloudMcpAgentHandler } from "./agent-handler"; + +const principal: Principal = { + accountId: "acct_test", + organizationId: "org_test", + organizationName: "Test Org", + email: "test@example.com", + name: "Test", + avatarUrl: null, + roles: ["member"], +}; + +const AuthProviderLive = Layer.succeed(McpAuthProvider)({ + discoveryRoutes: [], + resourceMetadataUrl: (request) => new URL("/.well-known/mcp", request.url).toString(), + authenticate: (request) => + Effect.succeed( + request.headers.has("authorization") ? authenticated(principal) : unauthorized(), + ), +}); + +const requestFor = (method: "GET" | "POST", sessionId: string, authenticated = true): Request => + new Request("https://executor.test/mcp", { + method, + headers: { + ...(authenticated ? { authorization: "Bearer test" } : {}), + "mcp-session-id": sessionId, + ...(method === "POST" ? { "content-type": "application/json" } : {}), + }, + ...(method === "POST" + ? { + body: JSON.stringify({ jsonrpc: "2.0", id: 1, method: "tools/list", params: {} }), + } + : {}), + }); + +const makeHarness = (owner: "not_found" | "terminated" = "not_found") => { + const ownerChecks = { count: 0 }; + const traces = { count: 0 }; + const stub = { + validateMcpSessionOwner: async () => { + ownerChecks.count += 1; + return owner; + }, + }; + const namespace = { + idFromString: (sessionId: string) => sessionId, + get: () => stub, + }; + // oxlint-disable-next-line executor/no-double-cast -- test boundary: the handler only reads the MCP_SESSION namespace in these legacy dead-session cases + const env = { MCP_SESSION: namespace } as unknown as Env; + // oxlint-disable-next-line executor/no-double-cast -- test boundary: no ExecutionContext capability is used before the dead-session response + const ctx = {} as unknown as ExecutionContext; + const handler = makeCloudMcpAgentHandler({ + authProvider: AuthProviderLive, + makeModernServerBuilder: () => ({ build: () => Effect.die("unused modern builder") }), + traceRequest: async (request, _env, _ctx, handle) => { + traces.count += 1; + return handle(request); + }, + }); + return { ctx, env, handler, ownerChecks, traces }; +}; + +describe("cloud MCP dead-session negative cache", () => { + beforeEach(() => { + vi.useRealTimers(); + cloudDeadSessionCacheForTest.clear(); + }); + + it.each([ + { method: "GET" as const, status: 405 }, + { method: "POST" as const, status: 404 }, + ])("serves a repeated $method without another DO lookup", async ({ method, status }) => { + const { ctx, env, handler, ownerChecks, traces } = makeHarness(); + const sessionId = `dead-${method.toLowerCase()}`; + + const first = await handler(requestFor(method, sessionId), env, ctx); + const second = await handler(requestFor(method, sessionId), env, ctx); + + expect(first.status).toBe(status); + expect(second.status).toBe(status); + expect(ownerChecks.count).toBe(1); + expect(traces.count).toBe(1); + }); + + it("keeps authentication ahead of cached session existence", async () => { + const { ctx, env, handler, ownerChecks, traces } = makeHarness(); + const sessionId = "dead-auth"; + + expect((await handler(requestFor("GET", sessionId), env, ctx)).status).toBe(405); + const unauthenticated = await handler(requestFor("GET", sessionId, false), env, ctx); + + expect(unauthenticated.status).toBe(401); + expect(ownerChecks.count).toBe(1); + expect(traces.count).toBe(1); + }); + + it("preserves the terminated-session response on a cached hit", async () => { + const { ctx, env, handler, ownerChecks, traces } = makeHarness("terminated"); + const sessionId = "dead-terminated"; + + const first = await handler(requestFor("POST", sessionId), env, ctx); + const second = await handler(requestFor("POST", sessionId), env, ctx); + + expect(first.status).toBe(404); + expect(second.status).toBe(404); + expect(await second.text()).toBe(await first.text()); + expect(ownerChecks.count).toBe(1); + expect(traces.count).toBe(1); + }); + + it("consults the DO again after five minutes", async () => { + vi.useFakeTimers(); + vi.setSystemTime(0); + const { ctx, env, handler, ownerChecks } = makeHarness(); + const sessionId = "dead-expiry"; + + expect((await handler(requestFor("GET", sessionId), env, ctx)).status).toBe(405); + vi.advanceTimersByTime(5 * 60 * 1_000 + 1); + expect((await handler(requestFor("GET", sessionId), env, ctx)).status).toBe(405); + + expect(ownerChecks.count).toBe(2); + }); + + it("evicts the oldest entry without exceeding 4,096 sessions", () => { + for (let index = 0; index <= 4_096; index += 1) { + cloudDeadSessionCacheForTest.remember(`dead-${index}`, 0); + } + + expect(cloudDeadSessionCacheForTest.size()).toBe(4_096); + expect(cloudDeadSessionCacheForTest.has("dead-0", 1)).toBe(false); + expect(cloudDeadSessionCacheForTest.has("dead-1", 1)).toBe(true); + expect(cloudDeadSessionCacheForTest.has("dead-4096", 1)).toBe(true); + }); +}); diff --git a/apps/cloud/src/mcp/agent-handler.ts b/apps/cloud/src/mcp/agent-handler.ts index a2e7a92404..0a8980bfe9 100644 --- a/apps/cloud/src/mcp/agent-handler.ts +++ b/apps/cloud/src/mcp/agent-handler.ts @@ -1,5 +1,5 @@ import * as OtelTracer from "@effect/opentelemetry/Tracer"; -import { Effect, Predicate } from "effect"; +import { Effect, Layer, Predicate } from "effect"; import { McpAuthProvider, @@ -8,6 +8,7 @@ import { defaultMcpResource, UNAVAILABLE_RETRY_AFTER_SECONDS, type AuthOutcome, + type McpModernServerBuilder, type McpResource, } from "@executor-js/host-mcp"; import { requestBodyFromRequest } from "@executor-js/host-mcp/tool-server"; @@ -32,9 +33,53 @@ import { createMcpSessionStub, mcpSessionStub } from "@executor-js/cloudflare/mc import { wrapMcpSseResponse } from "../observability/memory-metrics"; import { WorkerTelemetryLive } from "../observability/telemetry"; import { cloudMcpAuth } from "./auth-provider"; -import { makeCloudModernMcpServerBuilder } from "./session-durable-object"; import { parseTraceparent } from "./traceparent"; +const DEAD_SESSION_CACHE_TTL_MS = 5 * 60 * 1_000; +const DEAD_SESSION_CACHE_MAX_ENTRIES = 4_096; +const deadSessionExpiries = new Map(); +const timedOutSessionIds = new Set(); + +type DeadSessionReason = "not_found" | "timed_out"; + +const isDeadSessionCached = (sessionId: string, now = Date.now()): boolean => { + const expiry = deadSessionExpiries.get(sessionId); + if (expiry === undefined) return false; + if (expiry > now) return true; + deadSessionExpiries.delete(sessionId); + timedOutSessionIds.delete(sessionId); + return false; +}; + +const cacheDeadSession = (sessionId: string, reason: DeadSessionReason, now = Date.now()): void => { + deadSessionExpiries.delete(sessionId); + if (deadSessionExpiries.size >= DEAD_SESSION_CACHE_MAX_ENTRIES) { + const oldestSessionId = deadSessionExpiries.keys().next().value; + if (oldestSessionId !== undefined) { + deadSessionExpiries.delete(oldestSessionId); + timedOutSessionIds.delete(oldestSessionId); + } + } + deadSessionExpiries.set(sessionId, now + DEAD_SESSION_CACHE_TTL_MS); + if (reason === "timed_out") timedOutSessionIds.add(sessionId); + else timedOutSessionIds.delete(sessionId); +}; + +const cachedDeadSessionMessage = (sessionId: string): string => + timedOutSessionIds.has(sessionId) ? "Session timed out, please reconnect" : "Session not found"; + +/** Test-only access to reset and verify the isolate-local dead-session cache. */ +export const cloudDeadSessionCacheForTest = { + clear: (): void => { + deadSessionExpiries.clear(); + timedOutSessionIds.clear(); + }, + remember: (sessionId: string, now?: number): void => + cacheDeadSession(sessionId, "not_found", now), + has: isDeadSessionCached, + size: (): number => deadSessionExpiries.size, +}; + const jsonRpcResponse = ( status: number, code: number, @@ -97,12 +142,12 @@ const renderAuthError = ( }); }; -const authenticate = (request: Request) => +const authenticate = (request: Request, authProvider: Layer.Layer) => Effect.gen(function* () { const auth = yield* McpAuthProvider; const outcome = yield* auth.authenticate(request); return { auth, outcome }; - }).pipe(Effect.provide(cloudMcpAuth)); + }).pipe(Effect.provide(authProvider)); // The earlier shared envelope ran the MCP auth path inside the Effect app, whose // HttpMiddleware provided the OTEL tracer — that is where the `mcp.request` @@ -126,6 +171,21 @@ const runTraced = (request: Request, program: Effect.Effect): Promise = ); }; +type TraceCloudMcpRequest = ( + request: Request, + env: Env, + ctx: ExecutionContext, + handle: (tracedRequest: Request) => Promise, +) => Promise; + +interface CloudMcpAgentHandlerOptions { + readonly makeModernServerBuilder: ( + session: McpSessionProps["session"], + ) => McpModernServerBuilder["Service"]; + readonly authProvider?: Layer.Layer; + readonly traceRequest?: TraceCloudMcpRequest; +} + // The MCP resource the request targets. `server.ts` routes both the bare `/mcp` // and `/mcp/toolkits/` to this handler (`prepareMcpOrgScope` strips the org // selector but keeps the toolkit segment), so a session minted on a toolkit path @@ -158,11 +218,14 @@ const propsForPrincipal = ( }; }); -export const makeCloudMcpAgentHandler = () => { +/** Build the cloud worker's authenticated legacy/modern MCP request handler. */ +export const makeCloudMcpAgentHandler = (options: CloudMcpAgentHandlerOptions) => { + const authProvider = options.authProvider ?? cloudMcpAuth; + const traceRequest = options.traceRequest ?? ((request, _env, _ctx, handle) => handle(request)); const modern = makeMcpModernRequestRouter(); const ALLOWED_METHODS = new Set(["GET", "POST", "DELETE", "OPTIONS"]); - return async (request: Request, env: Env, ctx: ExecutionContext): Promise => { + const handle = async (request: Request, env: Env, ctx: ExecutionContext): Promise => { if (request.method === "OPTIONS") { return mcpCorsPreflightResponse(request.headers.get("access-control-request-headers")); } @@ -173,7 +236,7 @@ export const makeCloudMcpAgentHandler = () => { } const sessionId = request.headers.get("mcp-session-id"); - const { auth, outcome } = await runTraced(request, authenticate(request)); + const { auth, outcome } = await runTraced(request, authenticate(request, authProvider)); if (!Predicate.isTagged(outcome, "Authenticated")) { // Destroying a live session on auth grounds requires a POSITIVE // determination that access is genuinely gone — only `Forbidden` carries @@ -218,7 +281,7 @@ export const makeCloudMcpAgentHandler = () => { resource, props, requestStateSigningKey: requireMcpRequestStateKey(env.MCP_REQUEST_STATE_KEY), - builder: makeCloudModernMcpServerBuilder(props.session), + builder: options.makeModernServerBuilder(props.session), sessions: env.MCP_SESSION, executionOwners: mcpExecutionOwnerDirectoryFromNamespace(env.MCP_EXECUTION_OWNER), }); @@ -238,18 +301,20 @@ export const makeCloudMcpAgentHandler = () => { if (sessionId && !existingSession) { return deadSessionResponse(request.method, "Session not found"); } - if (existingSession) { + if (existingSession && sessionId) { const owner = await existingSession.validateMcpSessionOwner({ accountId: outcome.principal.accountId, organizationId: outcome.principal.organizationId, }); if (owner === "not_found") { + cacheDeadSession(sessionId, "not_found"); return deadSessionResponse(request.method, "Session not found"); } if (owner === "terminated") { // DELETE-condemned but the deferred destroy alarm hasn't wiped storage // yet. Same envelope as the post-destroy race below: the client must // treat the id as dead and reconnect. + cacheDeadSession(sessionId, "timed_out"); return deadSessionResponse(request.method, "Session timed out, please reconnect"); } if (owner === "forbidden") { @@ -286,6 +351,7 @@ export const makeCloudMcpAgentHandler = () => { // client to be told to reconnect, matching a timed-out session). // oxlint-disable-next-line executor/no-unknown-error-message -- adapter boundary: the abort reason is a plain runtime Error whose message IS the signal if (Predicate.isError(error) && error.message === "destroyed") { + if (sessionId) cacheDeadSession(sessionId, "timed_out"); return deadSessionResponse(request.method, "Session timed out, please reconnect"); } // oxlint-disable-next-line executor/no-try-catch-or-throw -- adapter boundary: rethrow anything that isn't the condemned-DO abort to the Workers runtime unchanged @@ -293,4 +359,20 @@ export const makeCloudMcpAgentHandler = () => { } return withMcpResponseHeaders(wrapMcpSseResponse(request, env, response)); }; + + return async (request: Request, env: Env, ctx: ExecutionContext): Promise => { + const sessionId = request.headers.get("mcp-session-id"); + const cacheEligible = request.method !== "OPTIONS" && ALLOWED_METHODS.has(request.method); + if (cacheEligible && sessionId && isDeadSessionCached(sessionId)) { + return Effect.runPromise( + Effect.gen(function* () { + const { auth, outcome } = yield* authenticate(request, authProvider); + return Predicate.isTagged(outcome, "Authenticated") + ? deadSessionResponse(request.method, cachedDeadSessionMessage(sessionId)) + : renderAuthError(auth, request, outcome); + }).pipe(Effect.withTracerEnabled(false)), + ); + } + return traceRequest(request, env, ctx, (tracedRequest) => handle(tracedRequest, env, ctx)); + }; }; diff --git a/apps/cloud/src/server.ts b/apps/cloud/src/server.ts index 8443a9d8b7..549d35f43d 100644 --- a/apps/cloud/src/server.ts +++ b/apps/cloud/src/server.ts @@ -15,7 +15,10 @@ import { isAppOwnedPath } from "./app-paths"; import { makeCloudMcpAgentHandler } from "./mcp/agent-handler"; import { classifyMcpPath, prepareMcpOrgScope } from "./mcp/mount"; import { parseTraceparent } from "./mcp/traceparent"; -import { McpSessionDOSqlite as McpSessionDOBase } from "./mcp/session-durable-object"; +import { + makeCloudModernMcpServerBuilder, + McpSessionDOSqlite as McpSessionDOBase, +} from "./mcp/session-durable-object"; import { beforeSendWithOtelCorrelation, captureCause, @@ -85,15 +88,12 @@ export { McpExecutionOwnerDirectoryDO } from "@executor-js/cloudflare/mcp/execut // migration — without the OTel-SDK version-conflict that package would now // drag in (it pins `@opentelemetry/otlp-* ^0.200.0`, we ship ^0.214.0). // -// ONLY for paths the Effect app does not own. App-owned paths (/api/*, /mcp, -// /.well-known/* — see app-paths.ts) get their `http.server` span from -// Effect's own HttpMiddleware.tracer, which parses `traceparent` itself and -// parents the workos/store/db child spans. Wrapping those here too produced -// two identical sibling `http.server` spans per request (scope -// `executor-cloud-worker` next to scope `executor-cloud`) — double ingest, -// and the waterfall showed a childless twin. The worker span remains for -// everything Effect never sees: Start SSR, the marketing proxy, /_astro -// assets. +// App-owned paths (/api/* and /.well-known/* — see app-paths.ts) get their +// `http.server` span from Effect's HttpMiddleware tracer. `/mcp` is dispatched +// directly and uses `traceCloudMcpRequest` below so the agent handler can skip +// the entire span envelope for negative-cache hits. Other paths keep this +// worker span. Wrapping Effect-owned paths here too produced duplicate sibling +// spans per request. // // SimpleSpanProcessor exports synchronously at span end but the underlying // `fetch()` to Axiom is fire-and-forget; the Worker may terminate before it @@ -108,7 +108,70 @@ const fetchHandler = handler.fetch as ( ) => Response | Promise; const tracer = trace.getTracer("executor-cloud-worker"); -const mcpAgentHandler = makeCloudMcpAgentHandler(); + +const traceCloudMcpRequest = async ( + request: Request, + _env: Env, + ctx: ExecutionContext, + handle: (tracedRequest: Request) => Promise, +): Promise => { + if (!installTracerProvider()) return handle(request); + + const url = new URL(request.url); + const inbound = parseTraceparent(request.headers.get("traceparent"), null); + const parentContext = inbound + ? trace.setSpanContext(context.active(), { + traceId: inbound.traceId, + spanId: inbound.spanId, + traceFlags: inbound.traceFlags, + isRemote: true, + }) + : context.active(); + + return tracer.startActiveSpan( + `http.server ${request.method}`, + { kind: SpanKind.SERVER }, + parentContext, + async (span) => { + span.setAttribute(ATTR_HTTP_REQUEST_METHOD, request.method); + span.setAttribute(ATTR_URL_FULL, request.url); + span.setAttribute(ATTR_URL_PATH, url.pathname); + span.setAttribute(ATTR_URL_SCHEME, url.protocol.replace(/:$/, "")); + const spanContext = span.spanContext(); + const headers = new Headers(request.headers); + headers.set( + "traceparent", + `00-${spanContext.traceId}-${spanContext.spanId}-${(spanContext.traceFlags & 0xff).toString(16).padStart(2, "0")}`, + ); + // oxlint-disable-next-line executor/no-try-catch-or-throw -- adapter boundary; observe response/error for span status, keep trace export alive after the Agents bridge resolves or rejects + try { + const response = await handle(new Request(request, { headers })); + span.setAttribute(ATTR_HTTP_RESPONSE_STATUS_CODE, response.status); + if (response.status >= 500) { + span.setStatus({ code: SpanStatusCode.ERROR, message: `HTTP ${response.status}` }); + } + return response; + } catch (err) { + // oxlint-disable-next-line executor/no-instanceof-error, executor/no-unknown-error-message -- adapter boundary: Cloudflare's fetch callback throws untyped; normalized only for the OTel span record, the original error is rethrown below + const cause = err instanceof Error ? err : String(err); + span.recordException(cause); + // oxlint-disable-next-line executor/no-unknown-error-message -- adapter boundary: same normalization as the recordException line above + const message = typeof cause === "string" ? cause : cause.message; + span.setStatus({ code: SpanStatusCode.ERROR, message }); + // oxlint-disable-next-line executor/no-try-catch-or-throw -- adapter boundary; preserve original error to Cloudflare runtime + throw err; + } finally { + span.end(); + ctx.waitUntil(flushTracerProvider()); + } + }, + ); +}; + +const mcpAgentHandler = makeCloudMcpAgentHandler({ + makeModernServerBuilder: makeCloudModernMcpServerBuilder, + traceRequest: traceCloudMcpRequest, +}); const cloudflareHandler: ExportedHandler = { fetch: async (request, env, ctx) => { @@ -120,10 +183,17 @@ const cloudflareHandler: ExportedHandler = { // The MCP dispatch is classified up front, independent of whether // telemetry installs — an unset `AXIOM_TOKEN` (tracer not installed) must // never take /mcp requests down with it. See `installTracerProvider`'s - // early return below: it only governs the tracing envelope for - // non-MCP paths. + // early return below: the handler invokes it for uncached MCP traffic, and + // this entry invokes it for non-MCP paths. const url = new URL(request.url); const mcpRoute = classifyMcpPath(url.pathname); + if (mcpRoute?.kind === "mcp") { + // The Cloudflare Agents MCP bridge needs the platform ExecutionContext + // to pass authenticated session props into the hibernatable DO. + // Discovery docs still flow through the app-level MCP envelope. + const forwarded = prepareMcpOrgScope(request); + return mcpAgentHandler(forwarded, env, ctx); + } const tracingInstalled = installTracerProvider(); // Join the caller's W3C trace when the request carries one — the web UI // sends traceparent on every API fetch, so the browser's spans and this @@ -138,61 +208,6 @@ const cloudflareHandler: ExportedHandler = { isRemote: true, }) : context.active(); - if (mcpRoute?.kind === "mcp") { - // The Cloudflare Agents MCP bridge needs the platform ExecutionContext - // to pass authenticated session props into the hibernatable DO. - // Discovery docs still flow through the app-level MCP envelope. - const forwarded = prepareMcpOrgScope(request); - if (!tracingInstalled) { - return mcpAgentHandler(forwarded, env, ctx); - } - // /mcp left the Effect app in the Agents-bridge migration, so no - // downstream HttpMiddleware.tracer opens the request envelope anymore — - // this worker span is now THE `http.server` span for MCP traffic. Its - // context is stamped onto the forwarded request's traceparent so the - // agent handler's Effect programs (mcp.request and children) and the - // session DO parent under it instead of exporting orphaned roots. - return tracer.startActiveSpan( - `http.server ${request.method}`, - { kind: SpanKind.SERVER }, - parentContext, - async (span) => { - span.setAttribute(ATTR_HTTP_REQUEST_METHOD, request.method); - span.setAttribute(ATTR_URL_FULL, request.url); - span.setAttribute(ATTR_URL_PATH, url.pathname); - span.setAttribute(ATTR_URL_SCHEME, url.protocol.replace(/:$/, "")); - const spanContext = span.spanContext(); - const headers = new Headers(forwarded.headers); - headers.set( - "traceparent", - `00-${spanContext.traceId}-${spanContext.spanId}-${(spanContext.traceFlags & 0xff).toString(16).padStart(2, "0")}`, - ); - // oxlint-disable-next-line executor/no-try-catch-or-throw -- adapter boundary; observe response/error for span status, keep trace export alive after the Agents bridge resolves or rejects - try { - const response = await mcpAgentHandler(new Request(forwarded, { headers }), env, ctx); - span.setAttribute(ATTR_HTTP_RESPONSE_STATUS_CODE, response.status); - if (response.status >= 500) { - span.setStatus({ code: SpanStatusCode.ERROR, message: `HTTP ${response.status}` }); - } - return response; - } catch (err) { - // Record the exception itself, not just the status bit: without it - // these spans are ERROR with zero diagnostic content. - // oxlint-disable-next-line executor/no-instanceof-error, executor/no-unknown-error-message -- adapter boundary: Cloudflare's fetch callback throws untyped; normalized only for the OTel span record, the original error is rethrown below - const cause = err instanceof Error ? err : String(err); - span.recordException(cause); - // oxlint-disable-next-line executor/no-unknown-error-message -- adapter boundary: same normalization as the recordException line above - const message = typeof cause === "string" ? cause : cause.message; - span.setStatus({ code: SpanStatusCode.ERROR, message }); - // oxlint-disable-next-line executor/no-try-catch-or-throw -- adapter boundary; preserve original error to Cloudflare runtime - throw err; - } finally { - span.end(); - ctx.waitUntil(flushTracerProvider()); - } - }, - ); - } if (!tracingInstalled) { return fetchHandler(request, env, ctx); } diff --git a/apps/host-cloudflare/src/app.ts b/apps/host-cloudflare/src/app.ts index 8c3eebf789..7a2b1bbfa5 100644 --- a/apps/host-cloudflare/src/app.ts +++ b/apps/host-cloudflare/src/app.ts @@ -16,6 +16,7 @@ import { ErrorCaptureLive } from "./observability"; import { cloudflareAccountMiddleware } from "./account/account-provider"; import { makeCloudflareApprovalHandler } from "./mcp"; import { makeCloudflareMcpAgentHandler } from "./mcp/agent-handler"; +import { makeCloudflareModernMcpServerBuilder } from "./mcp/session-durable-object"; import { preloadQuickJs } from "./quickjs"; // =========================================================================== @@ -45,7 +46,9 @@ export const makeCloudflareApp = async (env: CloudflareEnv) => { // handle the per-request scoped executor reads through the DbProvider seam. const dbHandle = await createD1ExecutorDb(env.DB, env.BLOBS); const identityLayer = cloudflareAccessIdentityLayer(config); - const mcpAgentHandler = makeCloudflareMcpAgentHandler(config); + const mcpAgentHandler = makeCloudflareMcpAgentHandler(config, { + makeModernServerBuilder: makeCloudflareModernMcpServerBuilder, + }); const approvalHandler = makeCloudflareApprovalHandler(config, env); const { appLayer, toWebHandler } = ExecutorApp.make({ diff --git a/apps/host-cloudflare/src/mcp/agent-handler.test.ts b/apps/host-cloudflare/src/mcp/agent-handler.test.ts new file mode 100644 index 0000000000..0d21240c53 --- /dev/null +++ b/apps/host-cloudflare/src/mcp/agent-handler.test.ts @@ -0,0 +1,153 @@ +import { beforeEach, describe, expect, it, vi } from "@effect/vitest"; +import { Effect, Layer } from "effect"; + +import { + authenticated, + McpAuthProvider, + unauthorized, + type Principal, +} from "@executor-js/host-mcp"; + +import type { CloudflareConfig, CloudflareEnv } from "../config"; +import { cloudflareDeadSessionCacheForTest, makeCloudflareMcpAgentHandler } from "./agent-handler"; + +const principal: Principal = { + accountId: "acct_test", + organizationId: "org_test", + organizationName: "Test Org", + email: "test@example.com", + name: "Test", + avatarUrl: null, + roles: ["member"], +}; + +const config = { + accessTeamDomain: "test.cloudflareaccess.com", + accessAud: "aud_test", + accessNameClaim: "name", + accessGroupsClaim: "groups", + adminEmails: [], + organizationId: "org_test", + organizationName: "Test Org", + organizationSlug: "test-org", + secretKey: "test-secret-key-0123456789", + allowLocalNetwork: false, + enableDevAuth: false, +} satisfies CloudflareConfig; + +const AuthProviderLive = Layer.succeed(McpAuthProvider)({ + discoveryRoutes: [], + resourceMetadataUrl: (request) => new URL("/.well-known/mcp", request.url).toString(), + authenticate: (request) => + Effect.succeed( + request.headers.has("authorization") ? authenticated(principal) : unauthorized(), + ), +}); + +const requestFor = (method: "GET" | "POST", sessionId: string, authenticated = true): Request => + new Request("https://executor.test/mcp", { + method, + headers: { + ...(authenticated ? { authorization: "Bearer test" } : {}), + "mcp-session-id": sessionId, + ...(method === "POST" ? { "content-type": "application/json" } : {}), + }, + ...(method === "POST" + ? { + body: JSON.stringify({ jsonrpc: "2.0", id: 1, method: "tools/list", params: {} }), + } + : {}), + }); + +const makeHarness = (owner: "not_found" | "terminated" = "not_found") => { + const ownerChecks = { count: 0 }; + const stub = { + validateMcpSessionOwner: async () => { + ownerChecks.count += 1; + return owner; + }, + }; + const namespace = { + idFromString: (sessionId: string) => sessionId, + get: () => stub, + }; + // oxlint-disable-next-line executor/no-double-cast -- test boundary: the handler only reads the MCP_SESSION namespace in these legacy dead-session cases + const env = { MCP_SESSION: namespace } as unknown as CloudflareEnv; + // oxlint-disable-next-line executor/no-double-cast -- test boundary: no ExecutionContext capability is used before the dead-session response + const ctx = {} as unknown as ExecutionContext; + const handler = makeCloudflareMcpAgentHandler(config, { + authProvider: AuthProviderLive, + makeModernServerBuilder: () => ({ build: () => Effect.die("unused modern builder") }), + }); + return { ctx, env, handler, ownerChecks }; +}; + +describe("standalone Cloudflare MCP dead-session negative cache", () => { + beforeEach(() => { + vi.useRealTimers(); + cloudflareDeadSessionCacheForTest.clear(); + }); + + it.each([ + { method: "GET" as const, status: 405 }, + { method: "POST" as const, status: 404 }, + ])("serves a repeated $method without another DO lookup", async ({ method, status }) => { + const { ctx, env, handler, ownerChecks } = makeHarness(); + const sessionId = `dead-${method.toLowerCase()}`; + + const first = await handler(requestFor(method, sessionId), env, ctx); + const second = await handler(requestFor(method, sessionId), env, ctx); + + expect(first.status).toBe(status); + expect(second.status).toBe(status); + expect(ownerChecks.count).toBe(1); + }); + + it("keeps authentication ahead of cached session existence", async () => { + const { ctx, env, handler, ownerChecks } = makeHarness(); + const sessionId = "dead-auth"; + + expect((await handler(requestFor("GET", sessionId), env, ctx)).status).toBe(405); + const unauthenticated = await handler(requestFor("GET", sessionId, false), env, ctx); + + expect(unauthenticated.status).toBe(401); + expect(ownerChecks.count).toBe(1); + }); + + it("preserves the terminated-session response on a cached hit", async () => { + const { ctx, env, handler, ownerChecks } = makeHarness("terminated"); + const sessionId = "dead-terminated"; + + const first = await handler(requestFor("POST", sessionId), env, ctx); + const second = await handler(requestFor("POST", sessionId), env, ctx); + + expect(first.status).toBe(404); + expect(second.status).toBe(404); + expect(await second.text()).toBe(await first.text()); + expect(ownerChecks.count).toBe(1); + }); + + it("consults the DO again after five minutes", async () => { + vi.useFakeTimers(); + vi.setSystemTime(0); + const { ctx, env, handler, ownerChecks } = makeHarness(); + const sessionId = "dead-expiry"; + + expect((await handler(requestFor("GET", sessionId), env, ctx)).status).toBe(405); + vi.advanceTimersByTime(5 * 60 * 1_000 + 1); + expect((await handler(requestFor("GET", sessionId), env, ctx)).status).toBe(405); + + expect(ownerChecks.count).toBe(2); + }); + + it("evicts the oldest entry without exceeding 4,096 sessions", () => { + for (let index = 0; index <= 4_096; index += 1) { + cloudflareDeadSessionCacheForTest.remember(`dead-${index}`, 0); + } + + expect(cloudflareDeadSessionCacheForTest.size()).toBe(4_096); + expect(cloudflareDeadSessionCacheForTest.has("dead-0", 1)).toBe(false); + expect(cloudflareDeadSessionCacheForTest.has("dead-1", 1)).toBe(true); + expect(cloudflareDeadSessionCacheForTest.has("dead-4096", 1)).toBe(true); + }); +}); diff --git a/apps/host-cloudflare/src/mcp/agent-handler.ts b/apps/host-cloudflare/src/mcp/agent-handler.ts index 277d34542a..9fcdbd47cf 100644 --- a/apps/host-cloudflare/src/mcp/agent-handler.ts +++ b/apps/host-cloudflare/src/mcp/agent-handler.ts @@ -1,4 +1,4 @@ -import { Effect, Predicate } from "effect"; +import { Effect, Layer, Predicate } from "effect"; import { McpAuthProvider, @@ -6,6 +6,7 @@ import { mcpModernDisabledResponse, defaultMcpResource, type AuthOutcome, + type McpModernServerBuilder, type Principal, } from "@executor-js/host-mcp"; import { requestBodyFromRequest } from "@executor-js/host-mcp/tool-server"; @@ -29,7 +30,51 @@ import { createMcpSessionStub, mcpSessionStub } from "@executor-js/cloudflare/mc import type { CloudflareConfig, CloudflareEnv } from "../config"; import { cloudflareAccessMcpAuth } from "./auth"; -import { makeCloudflareModernMcpServerBuilder } from "./session-durable-object"; + +const DEAD_SESSION_CACHE_TTL_MS = 5 * 60 * 1_000; +const DEAD_SESSION_CACHE_MAX_ENTRIES = 4_096; +const deadSessionExpiries = new Map(); +const timedOutSessionIds = new Set(); + +type DeadSessionReason = "not_found" | "timed_out"; + +const isDeadSessionCached = (sessionId: string, now = Date.now()): boolean => { + const expiry = deadSessionExpiries.get(sessionId); + if (expiry === undefined) return false; + if (expiry > now) return true; + deadSessionExpiries.delete(sessionId); + timedOutSessionIds.delete(sessionId); + return false; +}; + +const cacheDeadSession = (sessionId: string, reason: DeadSessionReason, now = Date.now()): void => { + deadSessionExpiries.delete(sessionId); + if (deadSessionExpiries.size >= DEAD_SESSION_CACHE_MAX_ENTRIES) { + const oldestSessionId = deadSessionExpiries.keys().next().value; + if (oldestSessionId !== undefined) { + deadSessionExpiries.delete(oldestSessionId); + timedOutSessionIds.delete(oldestSessionId); + } + } + deadSessionExpiries.set(sessionId, now + DEAD_SESSION_CACHE_TTL_MS); + if (reason === "timed_out") timedOutSessionIds.add(sessionId); + else timedOutSessionIds.delete(sessionId); +}; + +const cachedDeadSessionMessage = (sessionId: string): string => + timedOutSessionIds.has(sessionId) ? "Session timed out, please reconnect" : "Session not found"; + +/** Test-only access to reset and verify the isolate-local dead-session cache. */ +export const cloudflareDeadSessionCacheForTest = { + clear: (): void => { + deadSessionExpiries.clear(); + timedOutSessionIds.clear(); + }, + remember: (sessionId: string, now?: number): void => + cacheDeadSession(sessionId, "not_found", now), + has: isDeadSessionCached, + size: (): number => deadSessionExpiries.size, +}; const jsonRpcResponse = ( status: number, @@ -79,12 +124,12 @@ const renderAuthError = ( return jsonRpcResponse(503, -32001, outcome.message); }; -const authenticate = (request: Request, config: CloudflareConfig) => +const authenticate = (request: Request, authProvider: Layer.Layer) => Effect.gen(function* () { const auth = yield* McpAuthProvider; const outcome = yield* auth.authenticate(request); return { auth, outcome }; - }).pipe(Effect.provide(cloudflareAccessMcpAuth(config))); + }).pipe(Effect.provide(authProvider)); const propsForPrincipal = ( request: Request, @@ -108,7 +153,21 @@ const propsForPrincipal = ( }; }); -export const makeCloudflareMcpAgentHandler = (config: CloudflareConfig) => { +interface CloudflareMcpAgentHandlerOptions { + readonly makeModernServerBuilder: ( + env: CloudflareEnv, + config: CloudflareConfig, + session: McpSessionProps["session"], + ) => McpModernServerBuilder["Service"]; + readonly authProvider?: Layer.Layer; +} + +/** Build the standalone Cloudflare worker's authenticated MCP request handler. */ +export const makeCloudflareMcpAgentHandler = ( + config: CloudflareConfig, + options: CloudflareMcpAgentHandlerOptions, +) => { + const authProvider = options.authProvider ?? cloudflareAccessMcpAuth(config); const modern = makeMcpModernRequestRouter(); return async (request: Request, env: CloudflareEnv, ctx: ExecutionContext): Promise => { if (request.method === "OPTIONS") { @@ -116,7 +175,18 @@ export const makeCloudflareMcpAgentHandler = (config: CloudflareConfig) => { } const sessionId = request.headers.get("mcp-session-id"); - const { auth, outcome } = await Effect.runPromise(authenticate(request, config)); + if (sessionId && isDeadSessionCached(sessionId)) { + return Effect.runPromise( + Effect.gen(function* () { + const { auth, outcome } = yield* authenticate(request, authProvider); + return Predicate.isTagged(outcome, "Authenticated") + ? deadSessionResponse(request.method, cachedDeadSessionMessage(sessionId)) + : renderAuthError(auth, request, outcome); + }).pipe(Effect.withTracerEnabled(false)), + ); + } + + const { auth, outcome } = await Effect.runPromise(authenticate(request, authProvider)); if (!Predicate.isTagged(outcome, "Authenticated")) { if (Predicate.isTagged(outcome, "Forbidden") && sessionId) { const session = mcpSessionStub(env.MCP_SESSION, sessionId); @@ -152,7 +222,7 @@ export const makeCloudflareMcpAgentHandler = (config: CloudflareConfig) => { resource: defaultMcpResource, props, requestStateSigningKey: requireMcpRequestStateKey(env.MCP_REQUEST_STATE_KEY), - builder: makeCloudflareModernMcpServerBuilder(env, config, props.session), + builder: options.makeModernServerBuilder(env, config, props.session), sessions: env.MCP_SESSION, executionOwners: mcpExecutionOwnerDirectoryFromNamespace(env.MCP_EXECUTION_OWNER), }); @@ -166,17 +236,19 @@ export const makeCloudflareMcpAgentHandler = (config: CloudflareConfig) => { if (sessionId && !existingSession) { return deadSessionResponse(request.method, "Session not found"); } - if (existingSession) { + if (existingSession && sessionId) { const owner = await existingSession.validateMcpSessionOwner({ accountId: outcome.principal.accountId, organizationId: outcome.principal.organizationId, }); if (owner === "not_found") { + cacheDeadSession(sessionId, "not_found"); return deadSessionResponse(request.method, "Session not found"); } if (owner === "terminated") { // DELETE-condemned but the deferred destroy alarm hasn't wiped storage // yet; the terminated id must read as dead immediately. + cacheDeadSession(sessionId, "timed_out"); return deadSessionResponse(request.method, "Session timed out, please reconnect"); } if (owner === "forbidden") { diff --git a/apps/host-cloudflare/test-stubs/cloudflare-workers.ts b/apps/host-cloudflare/test-stubs/cloudflare-workers.ts new file mode 100644 index 0000000000..7c4b769fc8 --- /dev/null +++ b/apps/host-cloudflare/test-stubs/cloudflare-workers.ts @@ -0,0 +1,7 @@ +/** Node-test stand-ins for Cloudflare module exports reached during handler imports. */ +export const env: Record = {}; +export class WorkerEntrypoint {} +export class DurableObject {} +export class WorkflowEntrypoint {} +export class RpcTarget {} +export const exports: Record = {}; diff --git a/apps/host-cloudflare/vitest.config.ts b/apps/host-cloudflare/vitest.config.ts index 5bfa2d586e..44cc8cc150 100644 --- a/apps/host-cloudflare/vitest.config.ts +++ b/apps/host-cloudflare/vitest.config.ts @@ -1,6 +1,13 @@ +import { resolve } from "node:path"; + import { defineConfig } from "vitest/config"; export default defineConfig({ + resolve: { + alias: { + "cloudflare:workers": resolve(__dirname, "./test-stubs/cloudflare-workers.ts"), + }, + }, test: { include: ["src/**/*.test.ts"], passWithNoTests: true, From 4263f3384b4ae72d38848ac91348194f4980e55d Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Mon, 17 Aug 2026 13:37:21 -0600 Subject: [PATCH 09/11] Warm the Start server graph on isolate first-fetch (#1628) --- apps/cloud/src/server.ts | 44 +++++++++++++++++++++++ apps/cloud/src/start-virtual-entries.d.ts | 8 +++++ 2 files changed, 52 insertions(+) create mode 100644 apps/cloud/src/start-virtual-entries.d.ts diff --git a/apps/cloud/src/server.ts b/apps/cloud/src/server.ts index 549d35f43d..04d0002f11 100644 --- a/apps/cloud/src/server.ts +++ b/apps/cloud/src/server.ts @@ -173,8 +173,51 @@ const mcpAgentHandler = makeCloudMcpAgentHandler({ traceRequest: traceCloudMcpRequest, }); +// --------------------------------------------------------------------------- +// Start server-graph warmup +// +// TanStack Start's server entry loads the router and start instance behind a +// dynamic import on the first Start-handled request per isolate +// (start-server-core's `loadEntries`). That graph is the whole app bundle +// (React SSR + the Effect app), so the request that pays the load stalls for +// seconds on production metal. Under heavy MCP traffic the worker spreads +// across enough isolates that nearly every page/asset request IS such a first +// request — the 2026-08-16 session-reset reconnect storm took the homepage +// p50 from ~15ms to ~2.6s exactly this way. +// +// So: on each isolate's first fetch, kick the same imports off in the +// background. MCP traffic then pre-warms an isolate long before a page +// request reaches it. The specifiers are the virtual module ids the Start +// vite plugin registers for `loadEntries`' own imports, so both resolve to +// the same chunk and `loadEntries`' cache finds it already evaluated. +// +// Deliberately NOT at module scope: a full warmup there trips workerd's +// global-scope I/O restriction, and DO-only isolates should not carry the +// SSR graph in memory. +// --------------------------------------------------------------------------- +let startGraphWarm = false; +let startGraphWarmupStarted = false; +const warmStartGraph = () => { + if (startGraphWarmupStarted) return; + startGraphWarmupStarted = true; + // oxlint-disable-next-line executor/no-promise-catch -- adapter boundary; fire-and-forget warmup outside any Effect runtime + void Promise.all([import("#tanstack-router-entry"), import("#tanstack-start-entry")]) + .then(() => { + startGraphWarm = true; + }) + .catch(() => { + // Advisory only — the request path still loads the graph lazily. + startGraphWarmupStarted = false; + }); +}; + const cloudflareHandler: ExportedHandler = { fetch: async (request, env, ctx) => { + warmStartGraph(); + // Captured before any await: whether the Start graph was already + // evaluated when this request arrived (stamped on the page span below — + // a cold graph is the known multi-second page-latency mode). + const startGraphWasWarm = startGraphWarm; // Browser OTLP ingress — before the server span opens: exporter traffic // must never trace itself (the browser already excludes /v1/traces from // its own tracing for the same reason). @@ -234,6 +277,7 @@ const cloudflareHandler: ExportedHandler = { span.setAttribute(ATTR_URL_FULL, request.url); span.setAttribute(ATTR_URL_PATH, url.pathname); span.setAttribute(ATTR_URL_SCHEME, url.protocol.replace(/:$/, "")); + span.setAttribute("executor.start_graph.warm", startGraphWasWarm); // Adapter boundary: Cloudflare's fetch handler is a Promise-based // callback and the OTel span lifecycle needs to observe both the // resolved response and any thrown error before `span.end()`. Sentry's diff --git a/apps/cloud/src/start-virtual-entries.d.ts b/apps/cloud/src/start-virtual-entries.d.ts new file mode 100644 index 0000000000..9fb43d8206 --- /dev/null +++ b/apps/cloud/src/start-virtual-entries.d.ts @@ -0,0 +1,8 @@ +// TanStack Start's internal virtual server-entry modules (registered by the +// Start vite plugin; the same ids `start-server-core`'s `loadEntries` +// imports). server.ts imports them for the isolate warmup — only the +// module-evaluation side effect matters there, so the value shape is left +// untyped. Kept in a standalone declaration file: shorthand ambient modules +// only register from a non-module file (env-augment.d.ts is a module). +declare module "#tanstack-router-entry"; +declare module "#tanstack-start-entry"; From 003fb07d2e8ad20796f7cf827fb6266ac46048f6 Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Mon, 17 Aug 2026 14:36:21 -0600 Subject: [PATCH 10/11] First-party OAuth clients (#1527) * Add first-party OAuth clients (host-operated GitHub/Google apps) * Allow endpoint overrides for the first-party GitHub app in tests/dev * Launch built-in Google OAuth for Calendar and Sheets --- apps/cloud/src/engine/execution-stack.ts | 52 ++- apps/cloud/src/env-augment.d.ts | 15 + apps/marketing/src/pages/privacy.astro | 20 ++ e2e/scenarios/first-party-oauth.test.ts | 252 ++++++++++++++ e2e/setup/cloud.boot.ts | 15 + packages/core/api/src/oauth/api.ts | 9 + .../core/api/src/server/scoped-executor.ts | 10 + packages/core/sdk/src/executor.ts | 86 ++++- packages/core/sdk/src/index.ts | 5 + packages/core/sdk/src/oauth-client.ts | 69 +++- .../core/sdk/src/oauth-first-party.test.ts | 324 ++++++++++++++++++ packages/core/sdk/src/oauth-service.ts | 134 +++++++- packages/core/sdk/src/shared.ts | 4 + packages/core/sdk/src/test-config.ts | 2 + .../openapi/src/providers/google/index.ts | 1 + .../openapi/src/providers/google/presets.ts | 13 +- packages/plugins/openapi/src/sdk/presets.ts | 16 + .../src/components/add-account-modal.tsx | 16 +- .../use-effective-oauth-client.test.ts | 82 +++++ .../plugins/use-effective-oauth-client.tsx | 74 +++- 20 files changed, 1152 insertions(+), 47 deletions(-) create mode 100644 e2e/scenarios/first-party-oauth.test.ts create mode 100644 packages/core/sdk/src/oauth-first-party.test.ts diff --git a/apps/cloud/src/engine/execution-stack.ts b/apps/cloud/src/engine/execution-stack.ts index 869bf58167..ecbdee7142 100644 --- a/apps/cloud/src/engine/execution-stack.ts +++ b/apps/cloud/src/engine/execution-stack.ts @@ -42,8 +42,9 @@ import { PluginsProvider, collectTables, } from "@executor-js/api/server"; +import { googleCatalogOAuthScopesForPreset } from "@executor-js/plugin-openapi/providers/google"; import { makeDynamicWorkerExecutor } from "@executor-js/runtime-dynamic-worker"; -import type { AnyPlugin } from "@executor-js/sdk"; +import type { AnyPlugin, FirstPartyOAuthClientConfig } from "@executor-js/sdk"; import executorConfig from "../../executor.config"; import { DbService } from "../db/db"; @@ -88,6 +89,54 @@ export const CloudPluginsProvider: Layer.Layer = Layer.succeed( */ export const CLOUD_MOUNT_PREFIX = "/api" as const; +// Initial Google launch boundary. Calendar + Sheets are sensitive scopes but +// not restricted Workspace scopes; Gmail and account-wide Drive remain absent +// until their separate verification/security work is complete. The same scope +// source builds the catalog auth templates, preventing config drift. +const GOOGLE_FIRST_PARTY_ALLOWED_SCOPES: readonly string[] = [ + ...new Set([ + ...googleCatalogOAuthScopesForPreset("google-calendar"), + ...googleCatalogOAuthScopesForPreset("google-sheets"), + ]), +]; + +// Executor-owned provider apps, enabled per provider by setting BOTH env vars +// (id + secret). Each provider-side registration must list +// `${VITE_PUBLIC_SITE_URL}/api/oauth/callback` as its callback; the org slug +// travels inside OAuth `state`, so the single static callback serves every org. +// +// The endpoint URLs default to the real provider; the `_AUTHORIZE_URL` / +// `_TOKEN_URL` overrides exist so tests and dev instances can point the app at +// an emulated provider (`@executor-js/emulate`) and run the complete flow. +// Production leaves them unset. +const cloudFirstPartyOAuthClients = (): readonly FirstPartyOAuthClientConfig[] => [ + ...(env.FIRST_PARTY_GITHUB_CLIENT_ID && env.FIRST_PARTY_GITHUB_CLIENT_SECRET + ? [ + { + name: "github", + authorizationUrl: + env.FIRST_PARTY_GITHUB_AUTHORIZE_URL ?? "https://github.com/login/oauth/authorize", + tokenUrl: + env.FIRST_PARTY_GITHUB_TOKEN_URL ?? "https://github.com/login/oauth/access_token", + clientId: env.FIRST_PARTY_GITHUB_CLIENT_ID, + clientSecret: env.FIRST_PARTY_GITHUB_CLIENT_SECRET, + }, + ] + : []), + ...(env.FIRST_PARTY_GOOGLE_CLIENT_ID && env.FIRST_PARTY_GOOGLE_CLIENT_SECRET + ? [ + { + name: "google", + authorizationUrl: "https://accounts.google.com/o/oauth2/v2/auth", + tokenUrl: "https://oauth2.googleapis.com/token", + clientId: env.FIRST_PARTY_GOOGLE_CLIENT_ID, + clientSecret: env.FIRST_PARTY_GOOGLE_CLIENT_SECRET, + allowedScopes: GOOGLE_FIRST_PARTY_ALLOWED_SCOPES, + }, + ] + : []), +]; + export const CloudHostConfig: Layer.Layer = Layer.sync(HostConfig, () => ({ // SSRF / private-network egress guard. Config-driven, NOT a test flag: // production leaves `ALLOW_LOCAL_NETWORK` unset so the guard stays ON (`false`); @@ -99,6 +148,7 @@ export const CloudHostConfig: Layer.Layer = Layer.sync(HostConfig, ( // WorkOS Vault is cloud's credential storage implementation detail, not a // user-selectable provider surface. exposeCredentialProviders: false, + firstPartyOAuthClients: cloudFirstPartyOAuthClients(), })); export const CloudCodeExecutorProvider: Layer.Layer = Layer.sync( diff --git a/apps/cloud/src/env-augment.d.ts b/apps/cloud/src/env-augment.d.ts index 5b27d19de0..c9d76063e2 100644 --- a/apps/cloud/src/env-augment.d.ts +++ b/apps/cloud/src/env-augment.d.ts @@ -53,6 +53,21 @@ declare global { // number to drive the backstop. Production leaves it unset. EXECUTION_RATE_LIMIT_PER_HOUR?: string; + // First-party OAuth apps (executor-owned provider registrations). Each + // pair enables one-click connect through `first-party:`; an + // unset pair simply ships no first-party app for that provider. The + // registered callback on the provider side must be + // `${VITE_PUBLIC_SITE_URL}/api/oauth/callback`. + FIRST_PARTY_GITHUB_CLIENT_ID?: string; + FIRST_PARTY_GITHUB_CLIENT_SECRET?: string; + // Endpoint overrides for the GitHub first-party app, so tests/dev can + // point it at an emulated provider and complete the whole flow. Unset in + // production (the real github.com endpoints are the defaults). + FIRST_PARTY_GITHUB_AUTHORIZE_URL?: string; + FIRST_PARTY_GITHUB_TOKEN_URL?: string; + FIRST_PARTY_GOOGLE_CLIENT_ID?: string; + FIRST_PARTY_GOOGLE_CLIENT_SECRET?: string; + // Billing AUTUMN_SECRET_KEY?: string; /** Optional Autumn base-URL override (Autumn emulator in tests/dev). */ diff --git a/apps/marketing/src/pages/privacy.astro b/apps/marketing/src/pages/privacy.astro index b7d0c56b17..143c58ebe9 100644 --- a/apps/marketing/src/pages/privacy.astro +++ b/apps/marketing/src/pages/privacy.astro @@ -53,6 +53,26 @@ import LegalLayout from "../components/LegalLayout.astro"; a call, run code, or execute a workflow.

+

Google Workspace API data

+

+ If you connect a Google Workspace service, Executor uses the permissions you grant to perform the actions you + request through that integration. Depending on the service and permissions you choose, this may include accessing + or modifying Google Calendar events, Google Sheets spreadsheets, or other Google Workspace content. Executor stores + OAuth credentials and connection metadata so the integration can continue working, and processes Google Workspace + content when carrying out your requested tool calls and returning their results to you or the agent you directed to + make the call. +

+

+ We do not use Google Workspace API data for advertising or to train generalized artificial-intelligence or + machine-learning models. We disclose it only as needed to provide the user-requested integration, operate and secure + the Services, comply with law, or as otherwise described in this Policy with the user's consent. +

+

+ Executor's use and transfer of information received from Google Workspace APIs adheres to the + Google API Services User Data Policy, + including its Limited Use requirements. +

+

Website inputs and support communications

If you submit a URL to our website's API detection tool, contact us by email, or otherwise communicate with us, diff --git a/e2e/scenarios/first-party-oauth.test.ts b/e2e/scenarios/first-party-oauth.test.ts new file mode 100644 index 0000000000..cb483882c5 --- /dev/null +++ b/e2e/scenarios/first-party-oauth.test.ts @@ -0,0 +1,252 @@ +// First-party OAuth clients: the cloud host declares executor-owned apps via +// env (`FIRST_PARTY_GITHUB_CLIENT_ID/SECRET`, set by the e2e cloud boot), and +// every org can connect through them with nothing to paste. Three guarantees: +// +// 1. Listing: `oauth.listClients` surfaces `first-party:github` with a +// `first_party` origin and its public client id — no create call ever ran. +// 2. Flow: `oauth.start` through the first-party slug redirects to the +// provider's authorize endpoint carrying the env-configured client id and +// this platform's `/api/oauth/callback` — proof the config-resolved +// identity (not a stored row) drives the flow. The redirect is asserted, +// never followed: github.com is not visited. +// 3. Guardrails: the reserved `first-party:` namespace is rejected by +// createClient, so no org can shadow the host's app with its own row. +import { randomBytes } from "node:crypto"; + +import { expect } from "@effect/vitest"; +import { Effect } from "effect"; +import { composePluginApi } from "@executor-js/api/server"; +import { openApiHttpPlugin } from "@executor-js/plugin-openapi/api"; +import { + AuthTemplateSlug, + ConnectionName, + IntegrationSlug, + OAuthClientSlug, +} from "@executor-js/sdk/shared"; + +import { scenario } from "../src/scenario"; +import { Api, Target } from "../src/services"; + +const api = composePluginApi([openApiHttpPlugin()] as const); + +const unique = (prefix: string) => `${prefix}_${randomBytes(4).toString("hex")}`; + +/** A minimal integration whose OAuth template points at GitHub's endpoints, so + * the first-party `first-party:github` app is the matching client for it. */ +const githubShapedIntegrationSpec = { + spec: { + kind: "blob" as const, + value: JSON.stringify({ + openapi: "3.0.3", + info: { title: "GitHub-shaped API", version: "1.0.0" }, + paths: { + "/user": { + get: { + operationId: "getUser", + tags: ["default"], + responses: { "200": { description: "the caller" } }, + }, + }, + }, + }), + }, + baseUrl: "https://api.github.com", + authenticationTemplate: [ + { + slug: "oauth", + kind: "oauth2" as const, + authorizationUrl: "https://github.com/login/oauth/authorize", + tokenUrl: "https://github.com/login/oauth/access_token", + scopes: ["repo", "read:org"], + }, + ], +} as const; + +const googleShapedIntegrationSpec = (scopes: readonly string[]) => ({ + spec: { + kind: "blob" as const, + value: JSON.stringify({ + openapi: "3.0.3", + info: { title: "Google-shaped API", version: "1.0.0" }, + paths: { + "/resource": { + get: { + operationId: "getResource", + tags: ["default"], + responses: { "200": { description: "a Google resource" } }, + }, + }, + }, + }), + }, + baseUrl: "https://www.googleapis.com", + authenticationTemplate: [ + { + slug: "oauth", + kind: "oauth2" as const, + authorizationUrl: "https://accounts.google.com/o/oauth2/v2/auth", + tokenUrl: "https://oauth2.googleapis.com/token", + scopes, + }, + ], +}); + +scenario( + "First-party OAuth · the host-declared GitHub app is listed and drives the authorize redirect", + {}, + Effect.scoped( + Effect.gen(function* () { + const target = yield* Target; + // First-party registrations are a cloud-host capability. This scenario + // intentionally does not apply to self-host, whose operator supplies + // their own OAuth apps through its existing registration flow. + if (target.name !== "cloud") return; + const { client: makeApiClient } = yield* Api; + const identity = yield* target.newIdentity(); + const client = yield* makeApiClient(api, identity); + + // 1. The config-declared app appears in listings with its public id. + const clients = yield* client.oauth.listClients(); + const firstParty = clients.find((c) => String(c.slug) === "first-party:github"); + expect(firstParty, "the env-declared first-party GitHub app is listed").toBeDefined(); + expect(firstParty?.origin.kind).toBe("first_party"); + expect(firstParty?.clientId).toBe("e2e-first-party-github"); + + // 2. A start through the first-party slug builds GitHub's authorize URL + // from the config identity and this platform's served callback. + const integration = IntegrationSlug.make(unique("fpgh")); + yield* client.openapi.addSpec({ + payload: { ...githubShapedIntegrationSpec, slug: integration }, + }); + const started = yield* client.oauth.start({ + payload: { + client: OAuthClientSlug.make("first-party:github"), + clientOwner: "org", + owner: "org", + name: ConnectionName.make("main"), + integration, + template: AuthTemplateSlug.make("oauth"), + }, + }); + expect(started.status, "oauth.start redirects to the provider").toBe("redirect"); + const authorizationUrl = started.status === "redirect" ? started.authorizationUrl : ""; + const authorize = new URL(authorizationUrl); + expect(authorize.origin + authorize.pathname).toBe( + "https://github.com/login/oauth/authorize", + ); + expect(authorize.searchParams.get("client_id")).toBe("e2e-first-party-github"); + expect(authorize.searchParams.get("redirect_uri")).toBe( + new URL("/api/oauth/callback", target.baseUrl).toString(), + ); + + // 3. The reserved namespace cannot be shadowed by a stored row. The + // server rejects with a StorageError, which the HTTP edge scrubs to an + // opaque InternalError — assert the rejection, then prove the listed + // app is still the config-declared one (same public id, same origin). + yield* client.oauth + .createClient({ + payload: { + owner: "org", + slug: OAuthClientSlug.make("first-party:github"), + authorizationUrl: "https://github.com/login/oauth/authorize", + tokenUrl: "https://github.com/login/oauth/access_token", + grant: "authorization_code", + clientId: "impostor", + clientSecret: "impostor-secret", + }, + }) + .pipe(Effect.flip); + const after = yield* client.oauth.listClients(); + const survivors = after.filter((c) => String(c.slug) === "first-party:github"); + expect(survivors, "exactly one first-party:github remains listed").toHaveLength(1); + expect(survivors[0]?.origin.kind).toBe("first_party"); + expect(survivors[0]?.clientId, "the impostor never shadowed the host's app").toBe( + "e2e-first-party-github", + ); + }), + ), +); + +scenario( + "First-party OAuth · Google offers Calendar and Sheets but refuses Gmail scopes", + {}, + Effect.scoped( + Effect.gen(function* () { + const target = yield* Target; + if (target.name !== "cloud") return; + const { client: makeApiClient } = yield* Api; + const identity = yield* target.newIdentity(); + const client = yield* makeApiClient(api, identity); + + const clients = yield* client.oauth.listClients(); + const google = clients.find((candidate) => String(candidate.slug) === "first-party:google"); + expect(google, "the env-declared first-party Google app is listed").toBeDefined(); + expect(google?.origin.kind).toBe("first_party"); + if (google?.origin.kind !== "first_party") return; + expect(google.origin.allowedScopes).toContain("https://www.googleapis.com/auth/calendar"); + expect(google.origin.allowedScopes).toContain("https://www.googleapis.com/auth/spreadsheets"); + expect(google.origin.allowedScopes).not.toContain("https://mail.google.com/"); + expect(google.origin.allowedScopes).not.toContain("https://www.googleapis.com/auth/drive"); + + const calendar = IntegrationSlug.make(unique("google_calendar")); + yield* client.openapi.addSpec({ + payload: { + ...googleShapedIntegrationSpec([ + "openid", + "email", + "profile", + "https://www.googleapis.com/auth/calendar", + ]), + slug: calendar, + }, + }); + const started = yield* client.oauth.start({ + payload: { + client: OAuthClientSlug.make("first-party:google"), + clientOwner: "org", + owner: "org", + name: ConnectionName.make("calendar"), + integration: calendar, + template: AuthTemplateSlug.make("oauth"), + }, + }); + expect(started.status).toBe("redirect"); + const authorizationUrl = started.status === "redirect" ? started.authorizationUrl : ""; + const authorize = new URL(authorizationUrl); + expect(authorize.origin + authorize.pathname).toBe( + "https://accounts.google.com/o/oauth2/v2/auth", + ); + expect(authorize.searchParams.get("client_id")).toBe("e2e-first-party-google"); + expect(authorize.searchParams.get("access_type")).toBe("offline"); + expect(new Set(authorize.searchParams.get("scope")?.split(" ") ?? [])).toEqual( + new Set(["openid", "email", "profile", "https://www.googleapis.com/auth/calendar"]), + ); + + const gmail = IntegrationSlug.make(unique("google_gmail")); + yield* client.openapi.addSpec({ + payload: { + ...googleShapedIntegrationSpec([ + "openid", + "email", + "profile", + "https://mail.google.com/", + ]), + slug: gmail, + }, + }); + const blocked = yield* client.oauth + .start({ + payload: { + client: OAuthClientSlug.make("first-party:google"), + clientOwner: "org", + owner: "org", + name: ConnectionName.make("gmail"), + integration: gmail, + template: AuthTemplateSlug.make("oauth"), + }, + }) + .pipe(Effect.flip); + expect(blocked).toBeDefined(); + }), + ), +); diff --git a/e2e/setup/cloud.boot.ts b/e2e/setup/cloud.boot.ts index 3c072a2f4b..3544afabe0 100644 --- a/e2e/setup/cloud.boot.ts +++ b/e2e/setup/cloud.boot.ts @@ -94,6 +94,21 @@ export const bootCloud = async (options: CloudBootOptions): Promise MCP_SESSION_TIMEOUT_MS: process.env.MCP_SESSION_TIMEOUT_MS, MCP_PAUSED_SESSION_IDLE_TIMEOUT_MS: process.env.MCP_PAUSED_SESSION_IDLE_TIMEOUT_MS, ALLOW_LOCAL_NETWORK: "true", + // A first-party GitHub app for the first-party-oauth scenario: proves the + // env → HostConfig → executor plumbing end to end. The scenario asserts the + // authorize REDIRECT only (client id + callback), never visits github.com. + FIRST_PARTY_GITHUB_CLIENT_ID: "e2e-first-party-github", + FIRST_PARTY_GITHUB_CLIENT_SECRET: "e2e-first-party-github-secret", + // Optional endpoint overrides pass through so a dev instance (`cli up + // cloud`) can point the first-party app at an emulated GitHub and run the + // complete authorize → token dance instead of stopping at github.com. + FIRST_PARTY_GITHUB_AUTHORIZE_URL: process.env.FIRST_PARTY_GITHUB_AUTHORIZE_URL, + FIRST_PARTY_GITHUB_TOKEN_URL: process.env.FIRST_PARTY_GITHUB_TOKEN_URL, + // Fake Google registration for redirect-only first-party coverage. The + // scenario never visits Google or exchanges a code; it proves cloud env, + // scope policy, and authorization URL construction end to end. + FIRST_PARTY_GOOGLE_CLIENT_ID: "e2e-first-party-google", + FIRST_PARTY_GOOGLE_CLIENT_SECRET: "e2e-first-party-google-secret", // Shrink the per-org hourly execution cap (prod default 1000) to a number // the rate-limit-backstop scenario can actually exhaust with real // executions — but see execution-limits.ts: it must stay above every other diff --git a/packages/core/api/src/oauth/api.ts b/packages/core/api/src/oauth/api.ts index dcb71ccd96..6a54d46426 100644 --- a/packages/core/api/src/oauth/api.ts +++ b/packages/core/api/src/oauth/api.ts @@ -121,6 +121,15 @@ const OAuthClientSummaryResponse = Schema.Struct({ kind: Schema.Literal("dynamic_client_registration"), integration: Schema.optional(Schema.NullOr(IntegrationSlug)), }), + /** Host-operated app declared in executor config — every org connects + * through it; nothing to paste. `integrations` ranks it as the default + * for those integrations; `allowedScopes` is the host-enforced scope + * boundary the picker mirrors before offering it. */ + Schema.Struct({ + kind: Schema.Literal("first_party"), + integrations: Schema.optional(Schema.Array(IntegrationSlug)), + allowedScopes: Schema.optional(Schema.Array(Schema.String)), + }), ]), }); diff --git a/packages/core/api/src/server/scoped-executor.ts b/packages/core/api/src/server/scoped-executor.ts index 53f14592f8..6c2b342311 100644 --- a/packages/core/api/src/server/scoped-executor.ts +++ b/packages/core/api/src/server/scoped-executor.ts @@ -41,6 +41,7 @@ import { type AnyPlugin, type Executor, type ExecutorConfig, + type FirstPartyOAuthClientConfig, type StorageFailure, } from "@executor-js/sdk"; import { @@ -97,6 +98,14 @@ export interface HostConfigShape { * Hosts that record product analytics supply it; omitted -> no observation. */ readonly onIntegrationChange?: ExecutorConfig["onIntegrationChange"]; + /** + * Host-operated OAuth apps (`first-party:`), threaded verbatim into + * `createExecutor`. Declared here — not per-request — because the registered + * redirect URI on the provider side is fixed per deployment, and both request + * planes (HTTP API, MCP session DO) must resolve the same apps. Hosts that + * ship none simply omit it. + */ + readonly firstPartyOAuthClients?: readonly FirstPartyOAuthClientConfig[]; } export class HostConfig extends Context.Service()( @@ -281,6 +290,7 @@ export const makeScopedExecutor = < onElicitation: "accept-all", redirectUri, oauthCallbackStateOrgSlug: orgSlug, + firstPartyOAuthClients: config.firstPartyOAuthClients, coreTools: { webBaseUrl, orgSlug, diff --git a/packages/core/sdk/src/executor.ts b/packages/core/sdk/src/executor.ts index 4a962c3ab6..04c742063d 100644 --- a/packages/core/sdk/src/executor.ts +++ b/packages/core/sdk/src/executor.ts @@ -108,7 +108,8 @@ import { type MintOAuthConnectionInput, type OAuthScopePolicy, } from "./oauth-service"; -import type { OAuthService } from "./oauth-client"; +import { isFirstPartyOAuthClientSlug, type OAuthService } from "./oauth-client"; +import type { FirstPartyOAuthClientConfig } from "./oauth-client"; import { comparePolicyRow, isValidPattern, @@ -625,6 +626,14 @@ export interface ExecutorConfig`. Users connect through them with + * nothing to paste. Config-resolved — never persisted; secrets stay in host + * env and are never written to a credential provider or returned over any + * read surface. Minted connections and their tokens remain per-owner. + */ + readonly firstPartyOAuthClients?: readonly FirstPartyOAuthClientConfig[]; /** * Enable the built-in `core-tools` plugin which contributes agent-facing * static tools over the v2 surface (integrations / connections / policies). @@ -1753,6 +1762,23 @@ export const createExecutor = b.and(byOwner(owner)(b), b("slug", "=", slug)), }); + // Config-declared first-party apps, keyed by prefixed slug — the refresh + // path's counterpart to the OAuth service's config-first resolution. + const firstPartyOAuthBySlug = new Map( + (config.firstPartyOAuthClients ?? []).map((client) => [`first-party:${client.name}`, client]), + ); + + /** The app identity a refresh runs against, uniformly resolved: a stored + * row's secret comes out of the credential provider by item id; a + * first-party app's comes from host config and never touches a provider. */ + interface RefreshClient { + readonly clientId: string; + readonly clientSecret: string; + readonly tokenUrl: string; + readonly grant: string; + readonly resource: string | null; + } + /** What drove a refresh: the pre-call expiry check (`proactive`), or an * upstream 401 on a token we believed was still valid (`reactive`). */ type RefreshTrigger = "proactive" | "reactive"; @@ -1839,19 +1865,42 @@ export const createExecutor = + slug.startsWith(FIRST_PARTY_OAUTH_CLIENT_PREFIX); + +export const firstPartyOAuthClientSlug = (name: string): OAuthClientSlug => + OAuthClientSlug.make(`${FIRST_PARTY_OAUTH_CLIENT_PREFIX}${name}`); + +/** A first-party OAuth app the HOST declares at composition time — the + * deployment operator's own registered app for a provider. The secret comes + * from host env/config and stays in memory: it is never written to a + * credential provider and never surfaced over any read surface. Minted + * connections (and their tokens) remain per-owner exactly as with BYO apps; + * only the app identity is shared. */ +export interface FirstPartyOAuthClientConfig { + /** Unprefixed name, e.g. `"github"`; addressed as `first-party:github`. */ + readonly name: string; + readonly authorizationUrl: string; + readonly tokenUrl: string; + readonly clientId: string; + /** Literal secret from host env. Empty string for a public/PKCE client. */ + readonly clientSecret: string; + /** Integrations this app is intended for, used by pickers to rank it as the + * exact-match default for those integrations. Endpoint-host matching still + * applies when omitted. */ + readonly integrations?: readonly IntegrationSlug[]; + /** OAuth scopes this deployment permits the app to request. Omit to allow + * every scope declared by a matching integration. When present, OAuth start + * and completion fail unless every requested scope belongs to this set. */ + readonly allowedScopes?: readonly string[]; +} + +/** Whether a first-party app may request an integration's complete OAuth scope + * set. An omitted policy preserves provider-wide clients; an explicit policy + * is fail-closed and requires every requested scope to be listed. */ +export const firstPartyOAuthClientAllowsScopes = ( + config: Pick, + requestedScopes: readonly string[], +): boolean => { + if (config.allowedScopes === undefined) return true; + const allowed = new Set(config.allowedScopes); + return requestedScopes.every((scope) => allowed.has(scope)); +}; + export type CreateOAuthClientInput = OAuthClient & { - readonly origin?: OAuthClientOrigin; + /** Stored-row origins only — `first_party` is config-declared, never created + * through this surface (the service also rejects the slug namespace). */ + readonly origin?: Exclude; readonly originIssuer?: string | null; /** The redirect URI a DCR registration sent as the client's `redirect_uris` * entry. Persisted so reuse can detect a changed callback (strict servers diff --git a/packages/core/sdk/src/oauth-first-party.test.ts b/packages/core/sdk/src/oauth-first-party.test.ts new file mode 100644 index 0000000000..193b3b4fc2 --- /dev/null +++ b/packages/core/sdk/src/oauth-first-party.test.ts @@ -0,0 +1,324 @@ +import { describe, expect, it } from "@effect/vitest"; +import { Effect, Predicate } from "effect"; + +import { + AuthTemplateSlug, + ConnectionName, + IntegrationSlug, + OAuthClientSlug, + ToolAddress, + ToolName, +} from "./ids"; +import { + firstPartyOAuthClientSlug, + type FirstPartyOAuthClientConfig, + type OAuthStartError, +} from "./oauth-client"; +import { definePlugin } from "./plugin"; +import { makeTestWorkspaceHarness, memoryCredentialsPlugin } from "./test-config"; +import { serveOAuthTestServer } from "./testing/oauth-test-server"; + +// First-party OAuth clients: host-operated apps declared in executor config +// (`firstPartyOAuthClients`), addressed as `first-party:`. Resolved from +// config, never storage — these tests prove the whole lifecycle (start → +// complete → execute → refresh) runs off the config-declared identity, that the +// client CRUD surface rejects the reserved namespace, and that listings project +// the app without ever having written its secret to a credential provider. + +const INTEG = IntegrationSlug.make("acme"); +const TEMPLATE = AuthTemplateSlug.make("oauth"); +const FIRST_PARTY = firstPartyOAuthClientSlug("acme"); + +const oauthPlugin = definePlugin(() => ({ + id: "acme" as const, + storage: () => ({}), + resolveTools: () => + Effect.succeed({ + tools: [{ name: ToolName.make("whoami"), description: "whoami" }], + }), + describeAuthMethods: (record) => { + const config = record.config as { readonly scopes?: readonly string[] } | null; + return [ + { + id: "oauth", + label: "OAuth2", + kind: "oauth" as const, + template: String(TEMPLATE), + oauth: { scopes: config?.scopes ?? [] }, + }, + ]; + }, + invokeTool: ({ credential }) => Effect.succeed({ token: credential.value }), + checkHealth: ({ credential }) => + Effect.succeed({ + status: credential.value === null ? "expired" : "healthy", + checkedAt: Date.now(), + }), + extension: (ctx) => ({ + seed: (scopes: readonly string[] = []) => + ctx.core.integrations.register({ + slug: INTEG, + description: "Acme", + config: { scopes }, + }), + }), +}))(); + +const plugins = [memoryCredentialsPlugin(), oauthPlugin] as const; + +const firstPartyClientFor = (server: { + readonly authorizationEndpoint: string; + readonly tokenEndpoint: string; +}): FirstPartyOAuthClientConfig => ({ + name: "acme", + authorizationUrl: server.authorizationEndpoint, + tokenUrl: server.tokenEndpoint, + clientId: "test-client", + clientSecret: "test-secret", + integrations: [INTEG], +}); + +describe("first-party oauth clients", () => { + it.effect( + "start → complete through a config-declared client mints an executable connection", + () => + Effect.scoped( + Effect.gen(function* () { + const server = yield* serveOAuthTestServer({ scopes: ["read"] }); + const { executor } = yield* makeTestWorkspaceHarness({ + plugins, + firstPartyOAuthClients: [firstPartyClientFor(server)], + }); + yield* executor.acme.seed(); + + // No createClient call — the app exists purely in config. + const started = yield* executor.oauth.start({ + owner: "org", + client: FIRST_PARTY, + clientOwner: "org", + name: ConnectionName.make("main-account"), + integration: INTEG, + template: TEMPLATE, + }); + expect(started.status).toBe("redirect"); + if (started.status !== "redirect") return; + + const callback = yield* server.completeAuthorizationCodeFlow({ + authorizationUrl: started.authorizationUrl, + }); + const connection = yield* executor.oauth.complete({ + state: started.state, + code: callback.code, + }); + expect(String(connection.address)).toBe("tools.acme.org.mainAccount"); + + const out = (yield* executor.execute( + ToolAddress.make("tools.acme.org.mainAccount.whoami"), + {}, + )) as { token: string }; + expect(out.token).toMatch(/^at_/); + expect(yield* server.acceptsAccessToken(out.token)).toBe(true); + }), + ), + ); + + it.effect("refresh resolves the config-declared client (no oauth_client row exists)", () => + Effect.scoped( + Effect.gen(function* () { + const server = yield* serveOAuthTestServer({ scopes: ["read"] }); + const harness = yield* makeTestWorkspaceHarness({ + plugins, + firstPartyOAuthClients: [firstPartyClientFor(server)], + }); + const { executor, config } = harness; + yield* executor.acme.seed(); + + const started = yield* executor.oauth.start({ + owner: "org", + client: FIRST_PARTY, + clientOwner: "org", + name: ConnectionName.make("main"), + integration: INTEG, + template: TEMPLATE, + }); + if (started.status !== "redirect") return; + const callback = yield* server.completeAuthorizationCodeFlow({ + authorizationUrl: started.authorizationUrl, + }); + yield* executor.oauth.complete({ state: started.state, code: callback.code }); + + const firstToken = (yield* executor.execute( + ToolAddress.make("tools.acme.org.main.whoami"), + {}, + )) as { token: string }; + + // Force expiry so the next resolve refreshes through the config client. + yield* Effect.promise(() => + config.db.updateMany("connection", { + where: (b) => b("name", "=", "main"), + set: { expires_at: Date.now() - 60_000 }, + }), + ); + + const refreshedToken = (yield* executor.execute( + ToolAddress.make("tools.acme.org.main.whoami"), + {}, + )) as { token: string }; + expect(refreshedToken.token).not.toBe(firstToken.token); + expect(yield* server.acceptsAccessToken(refreshedToken.token)).toBe(true); + }), + ), + ); + + it.effect("listClients projects the first-party app ahead of stored rows, secretless", () => + Effect.scoped( + Effect.gen(function* () { + const server = yield* serveOAuthTestServer({ scopes: ["read"] }); + const { executor } = yield* makeTestWorkspaceHarness({ + plugins, + firstPartyOAuthClients: [ + { ...firstPartyClientFor(server), allowedScopes: ["openid", "read"] }, + ], + }); + yield* executor.acme.seed(); + + yield* executor.oauth.createClient({ + owner: "org", + slug: OAuthClientSlug.make("byo-app"), + authorizationUrl: server.authorizationEndpoint, + tokenUrl: server.tokenEndpoint, + grant: "authorization_code", + clientId: "byo-client", + clientSecret: "byo-secret", + }); + + const clients = yield* executor.oauth.listClients(); + expect(clients.map((c) => String(c.slug))).toEqual(["first-party:acme", "byo-app"]); + const firstParty = clients[0]!; + expect(firstParty.origin).toEqual({ + kind: "first_party", + integrations: [INTEG], + allowedScopes: ["openid", "read"], + }); + expect(firstParty.clientId).toBe("test-client"); + expect("clientSecret" in firstParty).toBe(false); + }), + ), + ); + + it.effect("a scope-limited first-party app rejects an integration outside its policy", () => + Effect.scoped( + Effect.gen(function* () { + const server = yield* serveOAuthTestServer({ scopes: ["read", "write"] }); + const { executor } = yield* makeTestWorkspaceHarness({ + plugins, + firstPartyOAuthClients: [{ ...firstPartyClientFor(server), allowedScopes: ["read"] }], + }); + yield* executor.acme.seed(["write"]); + + const error = yield* executor.oauth + .start({ + owner: "org", + client: FIRST_PARTY, + clientOwner: "org", + name: ConnectionName.make("blocked"), + integration: INTEG, + template: TEMPLATE, + }) + .pipe(Effect.flip); + + expect(Predicate.isTagged("OAuthStartError")(error)).toBe(true); + const startError = error as OAuthStartError; + expect(startError.message).toContain("not enabled for integration acme"); + }), + ), + ); + + it.effect("createClient and removeClient reject the reserved first-party namespace", () => + Effect.scoped( + Effect.gen(function* () { + const server = yield* serveOAuthTestServer({ scopes: ["read"] }); + const { executor } = yield* makeTestWorkspaceHarness({ + plugins, + firstPartyOAuthClients: [firstPartyClientFor(server)], + }); + + const createError = yield* executor.oauth + .createClient({ + owner: "org", + slug: FIRST_PARTY, + authorizationUrl: server.authorizationEndpoint, + tokenUrl: server.tokenEndpoint, + grant: "authorization_code", + clientId: "impostor", + clientSecret: "impostor-secret", + }) + .pipe(Effect.flip); + expect(createError.message).toContain("reserved first-party namespace"); + + const removeError = yield* executor.oauth + .removeClient("org", FIRST_PARTY) + .pipe(Effect.flip); + expect(removeError.message).toContain("cannot be removed"); + }), + ), + ); + + it.effect("start with an undeclared first-party slug fails as client-not-found", () => + Effect.scoped( + Effect.gen(function* () { + yield* serveOAuthTestServer({ scopes: ["read"] }); + const { executor } = yield* makeTestWorkspaceHarness({ plugins }); + yield* executor.acme.seed(); + + const error = yield* executor.oauth + .start({ + owner: "org", + client: firstPartyOAuthClientSlug("nope"), + clientOwner: "org", + name: ConnectionName.make("main"), + integration: INTEG, + template: TEMPLATE, + }) + .pipe(Effect.flip); + // `OAuthStartError` carries a typed `message`; the `Predicate.isTagged` + // guard narrows the union so this read is on a typed failure. + expect(Predicate.isTagged("OAuthStartError")(error)).toBe(true); + const startError = error as OAuthStartError; + expect(startError.message).toContain("not found"); + }), + ), + ); + + it.effect("a Personal connection can mint through a first-party app", () => + Effect.scoped( + Effect.gen(function* () { + const server = yield* serveOAuthTestServer({ scopes: ["read"] }); + const { executor } = yield* makeTestWorkspaceHarness({ + plugins, + firstPartyOAuthClients: [firstPartyClientFor(server)], + }); + yield* executor.acme.seed(); + + const started = yield* executor.oauth.start({ + owner: "user", + client: FIRST_PARTY, + clientOwner: "org", + name: ConnectionName.make("mine"), + integration: INTEG, + template: TEMPLATE, + }); + expect(started.status).toBe("redirect"); + if (started.status !== "redirect") return; + const callback = yield* server.completeAuthorizationCodeFlow({ + authorizationUrl: started.authorizationUrl, + }); + const connection = yield* executor.oauth.complete({ + state: started.state, + code: callback.code, + }); + expect(String(connection.address)).toBe("tools.acme.user.mine"); + }), + ), + ); +}); diff --git a/packages/core/sdk/src/oauth-service.ts b/packages/core/sdk/src/oauth-service.ts index 742cf65adb..a4dd8ebfa0 100644 --- a/packages/core/sdk/src/oauth-service.ts +++ b/packages/core/sdk/src/oauth-service.ts @@ -36,8 +36,12 @@ import { OAuthRegisterDynamicError, OAuthSessionNotFoundError, OAuthStartError, + firstPartyOAuthClientAllowsScopes, + firstPartyOAuthClientSlug, + isFirstPartyOAuthClientSlug, type ConnectResult, type CreateOAuthClientInput, + type FirstPartyOAuthClientConfig, type OAuthClientOrigin, type OAuthClientSummary, type OAuthCompleteInput, @@ -174,6 +178,12 @@ export interface OAuthServiceDeps { readonly redirectUri: string | null; /** URL selected organization slug to round-trip through OAuth `state`. */ readonly callbackStateOrgSlug?: string | null; + /** Host-operated apps declared at composition time (`first-party:` + * slugs). Resolved from config, never from storage: `loadClient` intercepts + * the prefix ahead of the DB, `listClients` appends their summaries, and the + * client CRUD surface rejects the namespace. Empty/omitted on hosts that + * ship no first-party apps. */ + readonly firstPartyClients?: readonly FirstPartyOAuthClientConfig[]; } type LooseDb = { @@ -490,9 +500,42 @@ const validateClientEndpoints = ( } }); +/** Resolve a config-declared first-party app to the loaded-client shape the + * flow/refresh paths consume. First-party apps are authorization_code only: + * client_credentials mints machine tokens under the OPERATOR's app identity, + * which must never be shared across tenants. */ +export const loadedFirstPartyClient = ( + config: FirstPartyOAuthClientConfig, +): { + readonly slug: string; + readonly authorizationUrl: string; + readonly tokenUrl: string; + readonly grant: OAuthGrant; + readonly clientId: string; + readonly clientSecret: string; + readonly resource: null; +} => ({ + slug: String(firstPartyOAuthClientSlug(config.name)), + authorizationUrl: config.authorizationUrl, + tokenUrl: config.tokenUrl, + grant: "authorization_code", + clientId: config.clientId, + clientSecret: config.clientSecret, + resource: null, +}); + export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { const httpClientLayer = deps.httpClientLayer ?? FetchHttpClient.layer; const fetch = deps.fetch; + // Config-declared first-party apps, keyed by their prefixed slug. Config is + // the source of truth — no row exists, so every stored-row path (CRUD, GC) + // is bypassed by construction, and rotating a secret is an env change. + const firstPartyBySlug = new Map( + (deps.firstPartyClients ?? []).map((client) => [ + String(firstPartyOAuthClientSlug(client.name)), + client, + ]), + ); // EXPLICIT — no localhost default. `null` means this executor has no OAuth // callback; redirect-requiring flows fail loudly via `requireRedirectUri`. const redirectUri = deps.redirectUri; @@ -597,6 +640,15 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { input: CreateOAuthClientInput, ): Effect.Effect => Effect.gen(function* () { + // The `first-party:` namespace is reserved for config-declared apps — a + // stored row under it would be shadowed by (or worse, impersonate) the + // host's own app. + if (isFirstPartyOAuthClientSlug(String(input.slug))) { + return yield* new StorageError({ + message: `OAuth client slug "${String(input.slug)}" uses the reserved first-party namespace.`, + cause: undefined, + }); + } yield* validateClientEndpoints(input, deps.endpointUrlPolicy); const keys = yield* Effect.try({ try: () => deps.ownedKeys(input.owner), @@ -682,6 +734,15 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { // ----------------------------------------------------------------------- const removeClient = (owner: Owner, slug: OAuthClientSlug): Effect.Effect => Effect.gen(function* () { + // Config-declared apps have no row to remove; removing one is an env + // change on the host, not a storage operation. Fail loudly rather than + // returning a success that changed nothing. + if (isFirstPartyOAuthClientSlug(String(slug))) { + return yield* new StorageError({ + message: `OAuth client "${String(slug)}" is a first-party app declared in host config; it cannot be removed through this surface.`, + cause: undefined, + }); + } yield* deps.fuma .use("oauth_client.delete", (db) => looseDb(db).deleteMany("oauth_client", { @@ -953,8 +1014,28 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { // tenant's org rows + this subject's own user rows, so no explicit filter is // needed. The `client_secret` column is deliberately never projected. // ----------------------------------------------------------------------- - const listClients = (): Effect.Effect => - deps.fuma + const listClients = (): Effect.Effect => { + // First-party apps lead the list: config-resolved, visible to every caller, + // and projected exactly like stored rows — clientId only, never the secret. + // Owner is reported as "org" (the widest visibility the summary shape can + // express); the flow itself ignores owner for first-party slugs. + const firstPartySummaries: readonly OAuthClientSummary[] = [...firstPartyBySlug.values()].map( + (config) => ({ + owner: "org", + slug: firstPartyOAuthClientSlug(config.name), + grant: "authorization_code", + authorizationUrl: config.authorizationUrl, + tokenUrl: config.tokenUrl, + resource: null, + clientId: config.clientId, + origin: { + kind: "first_party", + ...(config.integrations !== undefined ? { integrations: config.integrations } : {}), + ...(config.allowedScopes !== undefined ? { allowedScopes: config.allowedScopes } : {}), + }, + }), + ); + return deps.fuma .use("oauth_client.findMany", (db) => looseDb(db).findMany("oauth_client", {})) .pipe( Effect.flatMap((rows) => @@ -982,7 +1063,9 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { } satisfies OAuthClientSummary); }), ), + Effect.map((stored) => [...firstPartySummaries, ...stored]), ); + }; // ----------------------------------------------------------------------- // Load an oauth_client row by (owner, slug). @@ -990,8 +1073,15 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { const loadClient = ( owner: Owner, slug: OAuthClientSlug, - ): Effect.Effect => - deps.fuma + ): Effect.Effect => { + // First-party apps resolve from config, never storage. Owner is irrelevant: + // the app belongs to the DEPLOYMENT, and visibility policy has nothing to + // narrow — only the minted connection (and its tokens) is owner-scoped. + if (isFirstPartyOAuthClientSlug(String(slug))) { + const config = firstPartyBySlug.get(String(slug)); + return Effect.succeed(config ? loadedFirstPartyClient(config) : null); + } + return deps.fuma .use("oauth_client.findFirst", (db) => looseDb(db).findFirst("oauth_client", { where: (b: any) => b.and(b("owner", "=", owner), b("slug", "=", String(slug))), @@ -1038,6 +1128,7 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { }); }), ); + }; // ----------------------------------------------------------------------- // start — begin a flow through a client to mint a connection. @@ -1058,7 +1149,10 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { // cannot be backed by a member's private (user) app. The connection owner // and the app owner are otherwise independent — a Personal connection // through a shared Workspace app is the supported cross-owner case. - if (input.owner === "org" && input.clientOwner === "user") { + // First-party apps are deployment-owned, outside the owner lattice + // entirely, so the rule does not apply to them. + const firstPartyFlow = isFirstPartyOAuthClientSlug(String(input.client)); + if (!firstPartyFlow && input.owner === "org" && input.clientOwner === "user") { return yield* new OAuthStartError({ message: "A Workspace connection must use a Workspace app.", }); @@ -1129,6 +1223,22 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { ) : dedupeScopes(scopePolicy.scopes); + // An explicitly scope-limited first-party app is an authorization + // boundary, not picker decoration. Endpoint matching can associate one + // Google client with every Google API, so enforce the complete requested + // set here before persisting an OAuth session or redirecting the browser. + if (firstPartyFlow) { + const firstParty = firstPartyBySlug.get(String(input.client)); + if ( + firstParty !== undefined && + !firstPartyOAuthClientAllowsScopes(firstParty, requestedScopes) + ) { + return yield* new OAuthStartError({ + message: `The built-in OAuth app is not enabled for integration ${input.integration}.`, + }); + } + } + // client_credentials: exchange immediately and mint the connection. if (client.grant === "client_credentials") { const token = yield* exchangeClientCredentials({ @@ -1310,6 +1420,20 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { restartRequired: true, }); } + if (isFirstPartyOAuthClientSlug(String(session.clientSlug))) { + const firstParty = firstPartyBySlug.get(String(session.clientSlug)); + if ( + firstParty !== undefined && + firstParty.allowedScopes !== undefined && + (session.requestedScopes === null || + !firstPartyOAuthClientAllowsScopes(firstParty, session.requestedScopes)) + ) { + return yield* new OAuthCompleteError({ + message: `The built-in OAuth app is no longer enabled for integration ${session.integration}; restart the flow.`, + restartRequired: true, + }); + } + } // The PKCE verifier is minted by `start` for every authorization_code // session. A null/missing one means a corrupt session row — exchanging diff --git a/packages/core/sdk/src/shared.ts b/packages/core/sdk/src/shared.ts index 615c7c6c57..4a13eac961 100644 --- a/packages/core/sdk/src/shared.ts +++ b/packages/core/sdk/src/shared.ts @@ -144,6 +144,10 @@ export { // OAuth wire contracts (data + tagged errors; the flow impl is server-only). export { + FIRST_PARTY_OAUTH_CLIENT_PREFIX, + firstPartyOAuthClientSlug, + isFirstPartyOAuthClientSlug, + type FirstPartyOAuthClientConfig, type OAuthGrant, type OAuthAuthentication, type OAuthClient, diff --git a/packages/core/sdk/src/test-config.ts b/packages/core/sdk/src/test-config.ts index 9ad7945ff2..4cac77d711 100644 --- a/packages/core/sdk/src/test-config.ts +++ b/packages/core/sdk/src/test-config.ts @@ -123,6 +123,7 @@ export type TestConfigOptions["onIntegrationChange"]; + readonly firstPartyOAuthClients?: ExecutorConfig["firstPartyOAuthClients"]; }; export const makeTestConfig = ( @@ -163,6 +164,7 @@ export const makeTestConfig = > = { "google-cloud-resource-manager": { operation: "cloudresourcemanager.projects.list" }, }; +/** Complete Google OAuth scope set requested by a catalog preset, including + * identity scopes used to label and distinguish connected accounts. */ +export const googleCatalogOAuthScopesForPreset = (presetId: string): readonly string[] => + compactGoogleOAuthScopes([ + ...GOOGLE_IDENTITY_SCOPES, + ...googleOAuthConsentScopesForPreset(presetId), + ]); + const googleCatalogAuthTemplate = (presetId: string) => [ { slug: GOOGLE_OAUTH_SECURITY_SCHEME, kind: "oauth2" as const, authorizationUrl: GOOGLE_OAUTH_AUTHORIZATION_URL, tokenUrl: GOOGLE_OAUTH_TOKEN_URL, - scopes: compactGoogleOAuthScopes([ - ...GOOGLE_IDENTITY_SCOPES, - ...googleOAuthConsentScopesForPreset(presetId), - ]), + scopes: googleCatalogOAuthScopesForPreset(presetId), }, ]; diff --git a/packages/plugins/openapi/src/sdk/presets.ts b/packages/plugins/openapi/src/sdk/presets.ts index dcc2f81b75..6ecf2cd3dd 100644 --- a/packages/plugins/openapi/src/sdk/presets.ts +++ b/packages/plugins/openapi/src/sdk/presets.ts @@ -16,6 +16,21 @@ export interface OpenApiPreset { readonly healthCheck?: HealthCheckSpec; } +/** GitHub OAuth-app flow (classic apps, not GitHub Apps): scopes are requested + * at authorize time, tokens don't expire, and the token endpoint returns + * form-encoded unless the request sends `Accept: application/json` — which the + * SDK's token exchange always does (oauth4webapi sets it). `repo` + `read:org` + * + `user:email` covers the common repo/issue/PR surface without admin grants. */ +export const GITHUB_SUPPORTED_OAUTH_SCOPES = ["repo", "read:org", "user:email"] as const; + +export const GITHUB_OAUTH_TEMPLATE: IntegrationPresetAuthentication = { + slug: "oauth2", + kind: "oauth2", + authorizationUrl: "https://github.com/login/oauth/authorize", + tokenUrl: "https://github.com/login/oauth/access_token", + scopes: GITHUB_SUPPORTED_OAUTH_SCOPES, +}; + export const FIGMA_SUPPORTED_OAUTH_SCOPES = [ "current_user:read", "file_comments:read", @@ -68,6 +83,7 @@ const openApiOnlyPresets: readonly OpenApiPreset[] = [ url: "https://raw.githubusercontent.com/github/rest-api-description/main/descriptions/api.github.com/api.github.com.json", icon: "https://svgl.app/library/github_dark.svg", featured: true, + authTemplate: [GITHUB_OAUTH_TEMPLATE], }, { id: "vercel", diff --git a/packages/react/src/components/add-account-modal.tsx b/packages/react/src/components/add-account-modal.tsx index a79eafe560..21e6beb490 100644 --- a/packages/react/src/components/add-account-modal.tsx +++ b/packages/react/src/components/add-account-modal.tsx @@ -876,11 +876,18 @@ function OAuthAppRadioRow(props: { {clientDisplayName(String(app.slug))} - {clientHost(app.tokenUrl)} ·{" "} - {app.grant === "client_credentials" ? "app-to-app" : "you'll sign in"} + {app.origin.kind === "first_party" + ? "No setup needed · you'll sign in" + : `${clientHost(app.tokenUrl)} · ${ + app.grant === "client_credentials" ? "app-to-app" : "you'll sign in" + }`} - {showOwnerLabel ? {ownerLabel(app.owner)} : null} + {app.origin.kind === "first_party" ? ( + Built-in + ) : showOwnerLabel ? ( + {ownerLabel(app.owner)} + ) : null} {onManage ? ( @@ -1509,6 +1516,7 @@ function AddAccountModalView(props: AddAccountModalProps) { // unrelated provider's app. tokenUrl: method?.oauth?.tokenUrl ?? oauthFallbackProbe?.tokenUrl, authorizationUrl: method?.oauth?.authorizationUrl ?? oauthFallbackProbe?.authorizationUrl, + scopes: method?.oauth?.scopes, // Recorded intent: a manual app registered from THIS integration's dialog is // a tier-1 match regardless of host. integration, @@ -1595,6 +1603,8 @@ function AddAccountModalView(props: AddAccountModalProps) { const manageHandlersFor = ( appOption: OAuthClientOption, ): { readonly onEdit: () => void; readonly onRemove: () => void } | undefined => { + // First-party apps are host config, not rows: nothing to edit or remove. + if (appOption.origin.kind === "first_party") return undefined; const summary = clientSummaries.find( (c: OAuthClientSummary) => c.owner === appOption.owner && String(c.slug) === String(appOption.slug), diff --git a/packages/react/src/plugins/use-effective-oauth-client.test.ts b/packages/react/src/plugins/use-effective-oauth-client.test.ts index 6bdbf612f8..d240298a7d 100644 --- a/packages/react/src/plugins/use-effective-oauth-client.test.ts +++ b/packages/react/src/plugins/use-effective-oauth-client.test.ts @@ -267,6 +267,88 @@ describe("selectClientsForEndpoints", () => { "spotify-app-2", ]); }); + + it("ranks a first-party app above BYO apps when both match", () => { + const integration = IntegrationSlug.make("github_rest"); + const byo = app("my-github-app", { + authorizationUrl: "https://github.com/login/oauth/authorize", + tokenUrl: "https://github.com/login/oauth/access_token", + }); + const firstParty = app("first-party:github", { + owner: "org", + authorizationUrl: "https://github.com/login/oauth/authorize", + tokenUrl: "https://github.com/login/oauth/access_token", + origin: { kind: "first_party", integrations: [integration] }, + }); + const result = selectClientsForEndpoints([byo, firstParty], { + authorizationUrl: "https://github.com/login/oauth/authorize", + tokenUrl: "https://github.com/login/oauth/access_token", + integration, + }); + expect(result.endpointMatched).toBe(true); + // First-party leads despite being org-owned (user-owned normally sorts first). + expect(result.matched.map((a: OAuthClientOption) => String(a.slug))).toEqual([ + "first-party:github", + "my-github-app", + ]); + }); + + it("hides a scope-limited first-party app from another API on the same provider", () => { + const firstParty = app("first-party:google", { + owner: "org", + authorizationUrl: "https://accounts.google.com/o/oauth2/v2/auth", + tokenUrl: "https://oauth2.googleapis.com/token", + origin: { + kind: "first_party", + allowedScopes: ["openid", "email", "https://www.googleapis.com/auth/calendar"], + }, + }); + const result = selectClientsForEndpoints([firstParty], { + authorizationUrl: "https://accounts.google.com/o/oauth2/v2/auth", + tokenUrl: "https://oauth2.googleapis.com/token", + integration: IntegrationSlug.make("renamed_gmail"), + scopes: ["openid", "email", "https://mail.google.com/"], + }); + + expect(result.endpointMatched).toBe(false); + expect(result.matched).toEqual([]); + expect(result.nearMatches).toEqual([]); + expect(result.unmatched).toEqual([]); + }); + + it("intent-matches a first-party app to its declared integrations even without endpoints", () => { + const integration = IntegrationSlug.make("github_rest"); + const firstParty = app("first-party:github", { + owner: "org", + authorizationUrl: "https://github.com/login/oauth/authorize", + tokenUrl: "https://github.com/login/oauth/access_token", + origin: { kind: "first_party", integrations: [integration] }, + }); + const result = selectClientsForEndpoints([firstParty], { + requireEndpointMatch: true, + integration, + }); + expect(result.endpointMatched).toBe(true); + expect(result.matched.map((a: OAuthClientOption) => String(a.slug))).toEqual([ + "first-party:github", + ]); + }); + + it("does not surface a first-party app for an unrelated integration", () => { + const firstParty = app("first-party:github", { + owner: "org", + authorizationUrl: "https://github.com/login/oauth/authorize", + tokenUrl: "https://github.com/login/oauth/access_token", + origin: { kind: "first_party", integrations: [IntegrationSlug.make("github_rest")] }, + }); + const result = selectClientsForEndpoints([firstParty], { + authorizationUrl: "https://accounts.spotify.com/authorize", + tokenUrl: "https://accounts.spotify.com/api/token", + integration: IntegrationSlug.make("spotify"), + }); + expect(result.endpointMatched).toBe(false); + expect(result.matched).toEqual([]); + }); }); describe("selectDcrClientsForIntegration", () => { diff --git a/packages/react/src/plugins/use-effective-oauth-client.tsx b/packages/react/src/plugins/use-effective-oauth-client.tsx index 0c0467254b..8c8fae9013 100644 --- a/packages/react/src/plugins/use-effective-oauth-client.tsx +++ b/packages/react/src/plugins/use-effective-oauth-client.tsx @@ -47,6 +47,25 @@ export interface OAuthClientOption { export const isDcrClient = (app: OAuthClientOption): boolean => app.origin.kind === "dynamic_client_registration"; +/** True for host-operated first-party apps (config-declared, `first-party:` + * slugs). They rank ABOVE user/workspace apps when they match an integration: + * the one-click "nothing to paste" path is the default, BYO the escape hatch. */ +export const isFirstPartyClient = (app: OAuthClientOption): boolean => + app.origin.kind === "first_party"; + +/** Mirror the host's first-party scope boundary in the picker. The server is + * authoritative; this prevents offering a built-in app for a flow it will + * reject. An explicit policy with unknown scopes fails closed. */ +const firstPartyClientAllowsScopes = ( + app: OAuthClientOption, + requestedScopes: readonly string[] | undefined, +): boolean => { + if (app.origin.kind !== "first_party" || app.origin.allowedScopes === undefined) return true; + if (requestedScopes === undefined) return false; + const allowed = new Set(app.origin.allowedScopes); + return requestedScopes.every((scope) => allowed.has(scope)); +}; + const hostOf = (url: string): string | undefined => { // oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: URL() throws on invalid input; treat as "no host" try { @@ -115,12 +134,14 @@ const EMPTY_CLIENTS: readonly OAuthClientOption[] = []; const hostEq = (a: string | undefined, b: string | undefined): boolean => a !== undefined && b !== undefined && a === b; -/** Sort apps user-owned first (so the user's own apps surface before shared - * workspace apps). */ +/** Sort first-party apps first (the one-click default), then user-owned before + * shared workspace apps. */ const sortUserFirst = (apps: readonly OAuthClientOption[]): readonly OAuthClientOption[] => - [...apps].sort((a: OAuthClientOption, b: OAuthClientOption) => - a.owner === b.owner ? 0 : a.owner === "user" ? -1 : 1, - ); + [...apps].sort((a: OAuthClientOption, b: OAuthClientOption) => { + const aFirstParty = isFirstPartyClient(a); + if (aFirstParty !== isFirstPartyClient(b)) return aFirstParty ? -1 : 1; + return a.owner === b.owner ? 0 : a.owner === "user" ? -1 : 1; + }); /** * Pure matcher (no React/atoms) — split owner-visible apps into three honest @@ -154,6 +175,9 @@ export function selectClientsForEndpoints( /** The integration whose picker this is. A manual app stamped with this * integration (recorded intent) is a tier-1 match regardless of host. */ readonly integration?: IntegrationSlug; + /** Complete scope set declared by the selected OAuth auth method. Used to + * hide scope-limited first-party apps that the host will reject. */ + readonly scopes?: readonly string[]; /** When set, an integration that targets a SPECIFIC server (MCP, whose * endpoints are discovered at connect) must match by endpoint — absent * endpoints mean NO match (show the register CTA), never "every app @@ -167,14 +191,24 @@ export function selectClientsForEndpoints( readonly endpointMatched: boolean; } { // DCR clients are plumbing, never picker options. - const manual = all.filter((app) => !isDcrClient(app)); + const manual = all.filter( + (app) => !isDcrClient(app) && firstPartyClientAllowsScopes(app, endpoints.scopes), + ); const intent = endpoints.integration; - const matchesIntent = (app: OAuthClientOption): boolean => - intent != null && - app.origin.kind === "manual" && - app.origin.integration != null && - app.origin.integration === intent; + const matchesIntent = (app: OAuthClientOption): boolean => { + if (intent == null) return false; + // A first-party app declaring this integration is intent-matched the same + // way a BYO app registered from this dialog is. + if (app.origin.kind === "first_party") { + return (app.origin.integrations ?? []).includes(intent); + } + return ( + app.origin.kind === "manual" && + app.origin.integration != null && + app.origin.integration === intent + ); + }; const wantedTokenHost = endpoints.tokenUrl ? hostOf(endpoints.tokenUrl) : undefined; const wantedAuthorizationHost = endpoints.authorizationUrl @@ -246,6 +280,7 @@ export function useOAuthClientsForIntegration(opts: { readonly tokenUrl?: string; readonly authorizationUrl?: string; readonly integration?: IntegrationSlug; + readonly scopes?: readonly string[]; readonly requireEndpointMatch?: boolean; }): UseOAuthClientsResult { // Read the optimistic list so a just-registered/edited/removed app paints @@ -267,9 +302,17 @@ export function useOAuthClientsForIntegration(opts: { tokenUrl: opts.tokenUrl, authorizationUrl: opts.authorizationUrl, integration: opts.integration, + scopes: opts.scopes, requireEndpointMatch: opts.requireEndpointMatch, }), - [all, opts.tokenUrl, opts.authorizationUrl, opts.integration, opts.requireEndpointMatch], + [ + all, + opts.tokenUrl, + opts.authorizationUrl, + opts.integration, + opts.scopes, + opts.requireEndpointMatch, + ], ); if (!loaded) { @@ -377,9 +420,12 @@ export function optimisticDcrClientSlug(issuerOrEndpoint: string): OAuthClientSl return OAuthClientSlug.make(`dcr-${base || "authorization-server"}`); } -/** Humanize a client slug for display ("spotify-prod" → "Spotify prod"). */ +/** Humanize a client slug for display ("spotify-prod" → "Spotify prod"). + * First-party slugs drop their namespace prefix ("first-party:github" → + * "Github") — the row's badge already says it's the built-in app. */ export function clientDisplayName(slug: string): string { - const text = slug.replace(/[-_]/g, " ").trim(); + const bare = slug.startsWith("first-party:") ? slug.slice("first-party:".length) : slug; + const text = bare.replace(/[-_]/g, " ").trim(); return text.length > 0 ? text.charAt(0).toUpperCase() + text.slice(1) : slug; } From 75a3c4536ecdc062c44dff2f4258e3173956e728 Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Mon, 17 Aug 2026 14:42:26 -0600 Subject: [PATCH 11/11] Keep the standalone MCP SSE listener open after undelivered replay (#1629) --- .../mcp/agent-session-durable-object.test.ts | 14 ++++- .../src/mcp/agent-session-durable-object.ts | 56 +++++++++++++------ 2 files changed, 52 insertions(+), 18 deletions(-) diff --git a/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.test.ts b/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.test.ts index 5fec25ea0a..f0d579217b 100644 --- a/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.test.ts +++ b/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.test.ts @@ -418,9 +418,21 @@ describe("McpAgentSessionDOBase session serving", () => { }), ), ); - const standaloneReplayBody = await standaloneReplay.text(); + // The standalone listener replays undelivered responses as its opening + // frames and then STAYS OPEN (a close-after-replay here put every active + // client into a permanent reconnect loop), so read incrementally instead + // of draining to EOF. + const standaloneReader = standaloneReplay.body?.getReader(); + const decoder = new TextDecoder(); + let standaloneReplayBody = ""; + while (!standaloneReplayBody.includes("slow result")) { + const next = await standaloneReader?.read(); + if (!next || next.done) break; + standaloneReplayBody += decoder.decode(next.value, { stream: true }); + } expect(standaloneReplayBody).toContain("slow result"); expect(standaloneReplayBody).toContain("event: message"); + await standaloneReader?.cancel("test complete"); await state.flushWaitUntil(); }); diff --git a/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.ts b/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.ts index a27213eaf4..8e949fd90e 100644 --- a/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.ts +++ b/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.ts @@ -1148,14 +1148,36 @@ export abstract class McpAgentSessionDOBase< }, onClose: (reason) => { this.activeLegacyStreamCount = Math.max(0, this.activeLegacyStreamCount - 1); - if (reason === "complete" && options.acknowledge && options.acknowledge.length > 0) { + // "complete": the source body drained to its natural end. "rotate": + // the client stayed attached for the whole max-age window, so the + // initial replay frames were drained long before. Both count as + // delivered. "cancel"/"error" leave the streams replayable for the + // client's reconnect GET. + if ( + (reason === "complete" || reason === "rotate") && + options.acknowledge && + options.acknowledge.length > 0 + ) { this.ctx.waitUntil(this.eventStore.acknowledgeUndeliveredStreams(options.acknowledge)); } }, }); - private async replayUndeliveredOnStandaloneGet(request: Request): Promise { - if (request.method !== "GET" || request.headers.has("last-event-id")) return null; + /** + * Collect every undelivered stream's events as one SSE frame block, for + * prepending onto the standalone GET stream. Returning a FINITE response + * here instead (the pre-2026-08-17 behavior) was catastrophic: the client's + * standalone listener closed the moment the replay flushed, and because + * every POST marks its stream undelivered until acknowledged, an active + * session ALWAYS had something to replay — so every listener GET became a + * ~3s reconnect short-poll. Fleet-wide, that reconnect storm multiplied + * /mcp volume ~15x, drove the worker into memory-limit kills, and (by + * recycling isolates) pushed homepage TTFB from ~15ms to ~3s. + */ + private async collectUndeliveredReplay(): Promise<{ + readonly frame: Uint8Array; + readonly streamIds: readonly string[]; + } | null> { const frames: Uint8Array[] = []; const streamIds = await this.eventStore.replayUndeliveredStreams({ send: (eventId, message) => { @@ -1164,18 +1186,7 @@ export abstract class McpAgentSessionDOBase< }, }); if (frames.length === 0) return null; - return this.trackedLegacyResponse( - new Response(combineFrames(frames), { - headers: { - "content-type": "text/event-stream", - "cache-control": "no-cache, no-transform", - connection: "keep-alive", - "x-accel-buffering": "no", - "mcp-session-id": this.sessionId, - }, - }), - { acknowledge: streamIds }, - ); + return { frame: new Uint8Array(combineFrames(frames)), streamIds }; } private async serializedTransportRequest(run: () => Promise): Promise { @@ -1211,8 +1222,19 @@ export abstract class McpAgentSessionDOBase< // through every workerd/Vite streaming hop, so explicitly retire a // stale standalone mapping before opening its replacement. transport.closeStandaloneSSEStream(); - const replay = await this.replayUndeliveredOnStandaloneGet(request); - if (replay) return replay; + const replay = await this.collectUndeliveredReplay(); + if (replay) { + // Prepend the replay onto the transport's own long-lived + // standalone stream — the stream MUST stay open afterwards (see + // collectUndeliveredReplay). Acknowledgement moves to stream end: + // "complete"/"rotate" imply the client stayed attached long + // enough to have drained the prepended frames. + const response = await transport.handleRequest(request); + return this.trackedLegacyResponse(response, { + initialFrame: replay.frame, + acknowledge: replay.streamIds, + }); + } } } const toolCallIds = legacyToolCallRequestIds(parsedBody);