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/.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/.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: 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/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/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..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); @@ -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; @@ -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; @@ -420,7 +427,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 +481,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 +499,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 +600,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/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/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/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 c4abd5afa2..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, @@ -45,6 +90,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, @@ -78,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` @@ -107,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 @@ -139,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")); } @@ -154,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 @@ -199,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), }); @@ -217,21 +299,23 @@ 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) { + if (existingSession && sessionId) { const owner = await existingSession.validateMcpSessionOwner({ accountId: outcome.principal.accountId, organizationId: outcome.principal.organizationId, }); if (owner === "not_found") { - return jsonRpcResponse(404, -32001, "Session 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. - return jsonRpcResponse(404, -32001, "Session timed out, please reconnect"); + cacheDeadSession(sessionId, "timed_out"); + 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,11 +351,28 @@ 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"); + 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 throw error; } 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/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/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/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..8c7fd3fc08 --- /dev/null +++ b/apps/cloud/src/observability/header-redaction.ts @@ -0,0 +1,19 @@ +// --------------------------------------------------------------------------- +// 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"; +import { spanRedactedHeaderNames } from "@executor-js/sdk/host-internal"; + +export const SpanHeaderRedactionLive: Layer.Layer = Layer.succeed( + Headers.CurrentRedactedNames, + spanRedactedHeaderNames, +); 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..04d0002f11 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,10 +108,116 @@ 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, +}); + +// --------------------------------------------------------------------------- +// 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). @@ -120,10 +226,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,54 +251,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 }); - } - return response; - } catch (err) { - span.setStatus({ code: SpanStatusCode.ERROR }); - // 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); } @@ -212,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 @@ -237,11 +303,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/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"; 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": [ { 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 3a729f86c9..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, @@ -41,6 +86,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, @@ -60,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, @@ -89,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") { @@ -97,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); @@ -133,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), }); @@ -145,20 +234,22 @@ 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) { + if (existingSession && sessionId) { const owner = await existingSession.validateMcpSessionOwner({ accountId: outcome.principal.accountId, organizationId: outcome.principal.organizationId, }); if (owner === "not_found") { - return jsonRpcResponse(404, -32001, "Session 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. - return jsonRpcResponse(404, -32001, "Session timed out, please reconnect"); + cacheDeadSession(sessionId, "timed_out"); + 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/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, 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/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"); + }); + }), +); 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/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..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()( @@ -221,13 +230,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 +268,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, }; @@ -287,12 +290,13 @@ export const makeScopedExecutor = < onElicitation: "accept-all", redirectUri, oauthCallbackStateOrgSlug: orgSlug, + firstPartyOAuthClients: config.firstPartyOAuthClients, coreTools: { webBaseUrl, 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/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 = { 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/core/sdk/src/index.ts b/packages/core/sdk/src/index.ts index 9d3cc9f587..c56e3a236a 100644 --- a/packages/core/sdk/src/index.ts +++ b/packages/core/sdk/src/index.ts @@ -291,6 +291,11 @@ export { OAuthProbeError, OAuthRegisterDynamicError, OAuthSessionNotFoundError, + FIRST_PARTY_OAUTH_CLIENT_PREFIX, + firstPartyOAuthClientSlug, + isFirstPartyOAuthClientSlug, + type FirstPartyOAuthClientConfig, + type OAuthClientOrigin, type OAuthGrant, type OAuthAuthentication, type OAuthClient, diff --git a/packages/core/sdk/src/oauth-client.ts b/packages/core/sdk/src/oauth-client.ts index 1e91467170..613b09e5cd 100644 --- a/packages/core/sdk/src/oauth-client.ts +++ b/packages/core/sdk/src/oauth-client.ts @@ -77,10 +77,77 @@ export type OAuthClientOrigin = | { readonly kind: "dynamic_client_registration"; readonly integration?: IntegrationSlug | null; + } + | { + /** A host-operated app declared in executor config (`firstPartyOAuthClients`), + * not a stored row: the deployment operator registered ONE app with the + * provider (GitHub, Google, …) and every org connects through it, so + * users paste nothing. Config-resolved — never persisted, never + * creatable or removable through the client CRUD surface. Plural + * `integrations` (unlike the stored origins' single recorded intent): + * one Google app deliberately backs gmail, calendar, drive, …. */ + readonly kind: "first_party"; + readonly integrations?: readonly IntegrationSlug[]; + /** OAuth scopes this deployment permits the app to request. Omitted means + * the provider app is unrestricted; present means every requested scope + * must be in this set. This is public policy metadata, not a secret. */ + readonly allowedScopes?: readonly string[]; }; +/** Slug namespace separating config-declared first-party apps from stored + * owner-scoped rows. A first-party client declared as `github` is addressed + * everywhere (start input, connection rows' `oauth_client` column, listings) + * as `first-party:github`, so it can never collide with a BYO row and every + * load path can intercept before touching storage. The client CRUD surface + * rejects the prefix on stored rows. */ +export const FIRST_PARTY_OAUTH_CLIENT_PREFIX = "first-party:"; + +export const isFirstPartyOAuthClientSlug = (slug: string): boolean => + 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 = { }), ), ); - 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(); }); @@ -576,4 +588,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 d2454fabb9..8e949fd90e 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, @@ -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"; @@ -39,6 +41,7 @@ import { type McpResource, } from "@executor-js/host-mcp"; import { + parseTraceparentHeader, readArtifactsEnabled, readElicitationMode, verifiedMcpRequestHeaders, @@ -147,6 +150,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 +175,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. */ @@ -285,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, @@ -301,6 +332,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; @@ -369,8 +409,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 { @@ -515,6 +567,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 +686,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 +897,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 +916,7 @@ export abstract class McpAgentSessionDOBase< requestStateSigningKey: self.modernRequestStateSigningKey(), requestStatePrincipal, ...(requestStateBinding === null ? {} : { requestStateBinding }), + ...(clientInfo === undefined ? {} : { restoredClientInfo: clientInfo }), }); }).pipe( (effect) => self.withTelemetry(effect, propagation), @@ -1065,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) => { @@ -1081,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 { @@ -1117,7 +1211,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"); @@ -1128,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); @@ -1201,6 +1306,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, @@ -1212,7 +1322,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, }); @@ -1282,8 +1392,20 @@ 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.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/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), + ); }); /** diff --git a/packages/hosts/mcp/src/tool-server-core.ts b/packages/hosts/mcp/src/tool-server-core.ts index ccb07e2f8c..a4cf5eab48 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 })); @@ -1105,11 +1146,18 @@ 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 + // 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 +1227,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 +1388,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 +1467,7 @@ export const buildExecutorMcpTools = < "mcp.execute.execution_id": executionId, }, }), - Effect.annotateSpans(joinKeyAttributes(extra)), + Effect.annotateSpans(requestSpanAttributes(extra)), ); // --- tools --- @@ -1433,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(() => @@ -1458,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(() => { @@ -1509,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 --- // @@ -1892,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( @@ -1952,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(() => @@ -2017,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(() => @@ -2035,10 +2059,6 @@ export const buildExecutorMcpTools = < }, () => runToolEffect(listArtifacts()), ), - ).pipe( - Effect.withSpan("mcp.host.register_tool", { - attributes: { "mcp.tool.name": "list-artifacts" }, - }), ); yield* Effect.sync(() => @@ -2059,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(() => { @@ -2113,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 @@ -2169,9 +2181,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()) 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, diff --git a/packages/plugins/openapi/src/providers/google/index.ts b/packages/plugins/openapi/src/providers/google/index.ts index 025e0a2030..7cab8d31e3 100644 --- a/packages/plugins/openapi/src/providers/google/index.ts +++ b/packages/plugins/openapi/src/providers/google/index.ts @@ -17,6 +17,7 @@ export { googleStandardUserOAuthPresets, googleOAuthConsentScopes, googleOAuthConsentScopesForPreset, + googleCatalogOAuthScopesForPreset, googleServiceSlug, googleAudienceWarningsForUrls, googlePresetForDiscoveryUrl, diff --git a/packages/plugins/openapi/src/providers/google/presets.ts b/packages/plugins/openapi/src/providers/google/presets.ts index 0cdfc4f997..33fbab3830 100644 --- a/packages/plugins/openapi/src/providers/google/presets.ts +++ b/packages/plugins/openapi/src/providers/google/presets.ts @@ -317,16 +317,21 @@ const GOOGLE_HEALTH_CHECKS: Readonly> = { "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; }