diff --git a/.changeset/apikey-validate-cache.md b/.changeset/apikey-validate-cache.md new file mode 100644 index 000000000..26d3d9a21 --- /dev/null +++ b/.changeset/apikey-validate-cache.md @@ -0,0 +1,9 @@ +--- +"@executor-js/cloud": patch +--- + +**API key validation is cached per isolate** + +Every MCP request and every API-key-authenticated `/api/*` request used to pay a live WorkOS round trip (~100-150ms) to validate the presented key, on every single request. The JWT bearer path beside it already verified locally against a JWKS cached for an hour; API keys had no cache at all. + +Successful validations are now cached in a bounded per-isolate map for 60 seconds, keyed by the SHA-256 digest of the key value (never the raw credential). The MCP handler used to rebuild its whole auth layer (and with it the cache) on every request; it now builds the layer once per isolate, so the cache holds on both the `/api/*` and `/mcp` planes. Invalid keys and upstream failures are never cached, so probing bad keys cannot pollute the map and a freshly created key works immediately. The tradeoff: a revoked key remains usable for up to 60 seconds within an isolate that validated it before revocation — far tighter than the one-hour rotation window the JWT path already accepts. diff --git a/apps/cloud/src/auth/api-key-validation-cache.node.test.ts b/apps/cloud/src/auth/api-key-validation-cache.node.test.ts new file mode 100644 index 000000000..6c1b32eca --- /dev/null +++ b/apps/cloud/src/auth/api-key-validation-cache.node.test.ts @@ -0,0 +1,210 @@ +import { describe, expect, it } from "@effect/vitest"; +import { Effect, Exit, Layer } from "effect"; + +import { + ApiKeyService, + ApiKeyValidationError, + makeCachedApiKeyValidate, + type ApiKeyOwner, +} from "./api-keys"; +import { WorkOSClient, type WorkOSClientService } from "./workos"; + +const stubWorkOS = (overrides: Partial) => + Layer.succeed( + WorkOSClient, + new Proxy({} as WorkOSClientService, { + get: (_target, prop) => { + if (prop in overrides) return overrides[prop as keyof WorkOSClientService]; + return () => Effect.die(`unexpected WorkOSClient.${String(prop)} call`); + }, + }), + ); + +const userOwner = (keyId: string): ApiKeyOwner => ({ + scope: "user", + accountId: "user_123", + organizationId: "org_123", + keyId, +}); + +describe("makeCachedApiKeyValidate", () => { + it.effect("serves a repeat validation from cache without a second upstream call", () => + Effect.gen(function* () { + let calls = 0; + const cached = makeCachedApiKeyValidate(() => + Effect.sync(() => { + calls += 1; + return userOwner("api_key_1"); + }), + ); + + const first = yield* cached.validate("sk_test_abc"); + const second = yield* cached.validate("sk_test_abc"); + + expect(calls).toBe(1); + expect(first).toEqual(userOwner("api_key_1")); + expect(second).toEqual(first); + }), + ); + + it.effect("re-validates once the TTL has expired", () => + Effect.gen(function* () { + let calls = 0; + let nowMs = 0; + const cached = makeCachedApiKeyValidate( + () => + Effect.sync(() => { + calls += 1; + return userOwner("api_key_1"); + }), + { ttlMs: 1_000, now: () => nowMs }, + ); + + yield* cached.validate("sk_test_abc"); + nowMs = 999; + yield* cached.validate("sk_test_abc"); + expect(calls).toBe(1); + + nowMs = 1_000; + yield* cached.validate("sk_test_abc"); + expect(calls).toBe(2); + }), + ); + + it.effect("never caches an invalid key", () => + Effect.gen(function* () { + let calls = 0; + const cached = makeCachedApiKeyValidate(() => + Effect.sync(() => { + calls += 1; + return null; + }), + ); + + const first = yield* cached.validate("sk_test_bad"); + const second = yield* cached.validate("sk_test_bad"); + + expect(first).toBeNull(); + expect(second).toBeNull(); + expect(calls).toBe(2); + expect(cached.cacheKeys()).toHaveLength(0); + }), + ); + + it.effect("never caches an upstream failure", () => + Effect.gen(function* () { + let calls = 0; + const cached = makeCachedApiKeyValidate(() => + Effect.suspend(() => { + calls += 1; + return Effect.fail(new ApiKeyValidationError({ cause: "workos_down" })); + }), + ); + + const first = yield* Effect.exit(cached.validate("sk_test_abc")); + const second = yield* Effect.exit(cached.validate("sk_test_abc")); + + expect(Exit.isFailure(first)).toBe(true); + expect(Exit.isFailure(second)).toBe(true); + expect(calls).toBe(2); + expect(cached.cacheKeys()).toHaveLength(0); + }), + ); + + it.effect("resets a saturated map of live entries rather than growing past the bound", () => + Effect.gen(function* () { + const cached = makeCachedApiKeyValidate((value) => Effect.succeed(userOwner(value)), { + maxEntries: 3, + }); + + for (const value of ["sk_1", "sk_2", "sk_3"]) { + yield* cached.validate(value); + } + expect(cached.cacheKeys()).toHaveLength(3); + + // Nothing is expired, so a fourth live entry resets the map instead of + // growing it (mirrors execution-gate's writeCache). + yield* cached.validate("sk_4"); + expect(cached.cacheKeys()).toHaveLength(1); + + yield* cached.validate("sk_5"); + expect(cached.cacheKeys()).toHaveLength(2); + }), + ); + + it.effect("evicts expired entries before resetting a saturated map", () => + Effect.gen(function* () { + let nowMs = 0; + const cached = makeCachedApiKeyValidate((value) => Effect.succeed(userOwner(value)), { + ttlMs: 1_000, + maxEntries: 3, + now: () => nowMs, + }); + + for (const value of ["sk_1", "sk_2", "sk_3"]) { + yield* cached.validate(value); + } + expect(cached.cacheKeys()).toHaveLength(3); + + // Everything is expired: the sweep drops the stale entries and the new + // one fits without a full reset. + nowMs = 2_000; + yield* cached.validate("sk_4"); + expect(cached.cacheKeys()).toHaveLength(1); + }), + ); + + it.effect("keys the cache by the SHA-256 digest, never the raw key value", () => + Effect.gen(function* () { + const rawKey = "sk_live_super_secret_value"; + const cached = makeCachedApiKeyValidate(() => Effect.succeed(userOwner("api_key_1"))); + + yield* cached.validate(rawKey); + + const expectedDigest = yield* Effect.promise(async () => { + const digest = await crypto.subtle.digest("SHA-256", new TextEncoder().encode(rawKey)); + return Array.from(new Uint8Array(digest), (byte) => + byte.toString(16).padStart(2, "0"), + ).join(""); + }); + expect(cached.cacheKeys()).toEqual([expectedDigest]); + expect(cached.cacheKeys()).not.toContain(rawKey); + }), + ); +}); + +describe("ApiKeyService.WorkOS validation cache", () => { + it.effect("validates a repeated key through one upstream call per layer build", () => + Effect.gen(function* () { + let calls = 0; + const layer = ApiKeyService.WorkOS.pipe( + Layer.provide( + stubWorkOS({ + validateApiKey: () => + Effect.sync(() => { + calls += 1; + return { + apiKey: { + id: "api_key_123", + owner: { type: "user", id: "user_123", organizationId: "org_123" }, + }, + }; + }), + }), + ), + ); + + const { first, second } = yield* Effect.gen(function* () { + const apiKeys = yield* ApiKeyService; + return { + first: yield* apiKeys.validate("sk_test_abc"), + second: yield* apiKeys.validate("sk_test_abc"), + }; + }).pipe(Effect.provide(layer)); + + expect(calls).toBe(1); + expect(first?.keyId).toBe("api_key_123"); + expect(second).toEqual(first); + }), + ); +}); diff --git a/apps/cloud/src/auth/api-keys.ts b/apps/cloud/src/auth/api-keys.ts index 0effa2609..9ff7a1c66 100644 --- a/apps/cloud/src/auth/api-keys.ts +++ b/apps/cloud/src/auth/api-keys.ts @@ -1,5 +1,7 @@ import { Context, Data, Effect, Layer, Option, Schema } from "effect"; +import { sha256Hex } from "@executor-js/sdk"; + import { ApiKeyManagementError } from "./errors"; import { WorkOSClient } from "./workos"; @@ -277,6 +279,98 @@ const createdFromResponse = (value: unknown): CreatedApiKey | null => }, }); +// --------------------------------------------------------------------------- +// Per-isolate validation cache. +// +// Every /mcp request and every api-key-authenticated /api/* request funnels +// through `validate`, and each call is a live WorkOS round trip (~100-150ms). +// The JWT bearer path beside it verifies locally against a JWKS cached for an +// hour; api keys had no cache at all. This is the same bounded TTL map the +// engine already uses for billing outcomes (engine/execution-gate.ts). +// +// Security shape, stated explicitly because this is auth code: +// - The map key is the SHA-256 digest of the presented key value, never the +// raw credential, so the map's keys cannot be reversed into a usable key. +// The digest reaches no log, span, or error message. +// - Only SUCCESSFUL validations are cached. An invalid key and an upstream +// failure always miss: an attacker probing bad keys cannot poison the map +// (the size bound alone handles memory), and a just-created key works +// immediately instead of waiting out a stale negative entry. +// - REVOCATION WINDOW: a revoked key keeps working for up to the TTL (60s) +// in any isolate that validated it before revocation. That is the price +// of the cache, and it is far tighter than the 1h JWKS rotation window +// the JWT path already accepts. +// - Concurrent misses for the same key each call WorkOS; there is no +// in-flight dedupe. The duplicate window is one round trip per key per +// TTL per isolate — exactly the cost EVERY request paid before this +// cache — and dedupe would add interruption-safety machinery (a +// cancelled leader must not strand its waiters) to auth-critical code. +// --------------------------------------------------------------------------- + +const API_KEY_VALIDATION_CACHE_TTL_MS = 60_000; +// Sweep guard so a long-lived isolate serving many keys can't grow the cache +// map unbounded (mirrors BALANCE_CACHE_MAX_ENTRIES in execution-gate.ts). +const API_KEY_VALIDATION_CACHE_MAX_ENTRIES = 10_000; + +export type ApiKeyValidate = ( + value: string, +) => Effect.Effect; + +/** + * Wrap a `validate` function with the per-isolate success cache described + * above. Exported for tests only — production use is the single instance the + * `ApiKeyService.WorkOS` layer builds, which lives in `boot` and is therefore + * constructed once per isolate. + * + * The `ttlMs` / `maxEntries` / `now` knobs exist so tests can exercise expiry + * and the size bound without waiting on wall time; production passes nothing. + */ +export const makeCachedApiKeyValidate = ( + validate: ApiKeyValidate, + options?: { + readonly ttlMs?: number; + readonly maxEntries?: number; + readonly now?: () => number; + }, +): { + readonly validate: ApiKeyValidate; + /** Test seam: the digests currently cached, so tests can assert the raw + * credential never appears as a map key. */ + readonly cacheKeys: () => ReadonlyArray; +} => { + const ttlMs = options?.ttlMs ?? API_KEY_VALIDATION_CACHE_TTL_MS; + const maxEntries = options?.maxEntries ?? API_KEY_VALIDATION_CACHE_MAX_ENTRIES; + const now = options?.now ?? Date.now; + const cache = new Map(); + + const writeCache = (digest: string, owner: ApiKeyOwner, nowMs: number): void => { + if (cache.size >= maxEntries) { + for (const [key, entry] of cache) { + if (entry.expiresAtMs <= nowMs) cache.delete(key); + } + // Still saturated after dropping expired entries: reset rather than grow. + if (cache.size >= maxEntries) cache.clear(); + } + cache.set(digest, { owner, expiresAtMs: nowMs + ttlMs }); + }; + + return { + cacheKeys: () => [...cache.keys()], + validate: (value) => + Effect.gen(function* () { + const digest = yield* sha256Hex(value); + const nowMs = now(); + const cached = cache.get(digest); + if (cached && cached.expiresAtMs > nowMs) return cached.owner; + const owner = yield* validate(value); + // Cache only a resolved owner — see the block comment above for why + // null results and failures always stay misses. + if (owner) writeCache(digest, owner, nowMs); + return owner; + }), + }; +}; + export class ApiKeyService extends Context.Service< ApiKeyService, { @@ -320,12 +414,15 @@ export class ApiKeyService extends Context.Service< static WorkOS = Layer.effect(this)( Effect.gen(function* () { const workos = yield* WorkOSClient; + // Built once with the boot layer, so the success cache is per-isolate. + const cachedValidate = makeCachedApiKeyValidate((value) => + workos.validateApiKey(value).pipe( + Effect.map(ownerFromResponse), + Effect.mapError((cause) => new ApiKeyValidationError({ cause })), + ), + ); return { - validate: (value: string) => - workos.validateApiKey(value).pipe( - Effect.map(ownerFromResponse), - Effect.mapError((cause) => new ApiKeyValidationError({ cause })), - ), + validate: cachedValidate.validate, listUserKeys: ({ accountId, organizationId }) => workos.listUserApiKeys(accountId, organizationId).pipe( Effect.map(listFromResponse), diff --git a/apps/cloud/src/mcp/agent-handler.ts b/apps/cloud/src/mcp/agent-handler.ts index 0ec697c91..59a6e86f7 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, ManagedRuntime, Predicate } from "effect"; import { McpAuthProvider, @@ -27,6 +27,7 @@ import { mcpSessionStub } from "@executor-js/cloudflare/mcp/session-stub"; import { wrapMcpSseResponse } from "../observability/memory-metrics"; import { WorkerTelemetryLive } from "../observability/telemetry"; import { cloudMcpAuth } from "./auth-provider"; +import { makeBindingKeyedRuntime, mcpAuthBindingsFingerprint } from "./auth-runtime"; import { isMcpSessionMetaUnavailable } from "./session-meta"; import { McpSessionDOSqlite } from "./session-durable-object"; import { parseTraceparent } from "./traceparent"; @@ -128,7 +129,34 @@ const authenticate = (request: Request) => const auth = yield* McpAuthProvider; const outcome = yield* auth.authenticate(request); return { auth, outcome }; - }).pipe(Effect.provide(cloudMcpAuth)); + }); + +// ONE auth layer per isolate. `authenticate` used to end in +// `Effect.provide(cloudMcpAuth)`, which rebuilt the entire auth stack — the +// WorkOS client, `ApiKeyService` and with it the api-key validation success +// cache — on every MCP request, so that cache never survived a request on this +// plane (the /api/* plane resolves the same service from the app's `boot` +// layer, built once per isolate). A lazily created ManagedRuntime builds +// `cloudMcpAuth` on the first request and memoizes it; `runTraced` runs every +// program on it. Nothing inside the layer is request-scoped: the WorkOS client +// holds no sockets (just config), the JWKS cache is already module-scope, and +// `McpOrganizationAuthLive` builds its postgres socket FRESH inside every +// `authorize` call precisely so the service itself can outlive a request +// (Cloudflare Workers' I/O isolation — see makeMcpOrganizationAuthServices). +// +// The memo is keyed on the WorkOS binding values the layer captures at build, +// NOT held forever: Cloudflare reuses warm isolates across binding-only +// deployments, so a bare `??=` would keep authenticating with a rotated-out +// WORKOS_API_KEY until isolate eviction. `runTraced` fingerprints the +// request's own `env` (a string compare per request) and the cache swaps in a +// fresh runtime — fresh ApiKeyService validation cache included, as a rotated +// key must not serve cached validations — when the bindings changed. See +// auth-runtime.ts for the drop-vs-dispose reasoning. +const mcpAuthRuntimeFor: ( + fingerprint: string, +) => ManagedRuntime.ManagedRuntime = makeBindingKeyedRuntime(() => + ManagedRuntime.make(cloudMcpAuth), +); // The pre-Agents envelope ran the MCP auth path inside the Effect app, whose // HttpMiddleware provided the OTEL tracer — that is where the `mcp.request` @@ -140,12 +168,21 @@ const authenticate = (request: Request) => // forwarded request's traceparent — which also makes `currentPropagationHeaders` // (via Effect.currentParentSpan) ferry that same trace into the session DO // instead of letting the DO start a fresh root per request. -const runTraced = (request: Request, program: Effect.Effect): Promise => { +// +// Runs on the memoized auth runtime above (rather than a bare +// `Effect.runPromise`) so `authenticate` resolves the once-built +// `McpAuthProvider` from it; the telemetry layer is still provided per run, so +// spans keep exporting exactly as before. +const runTraced = ( + request: Request, + env: Env, + program: Effect.Effect, +): Promise => { const parsed = parseTraceparent( request.headers.get("traceparent"), request.headers.get("tracestate"), ); - return Effect.runPromise( + return mcpAuthRuntimeFor(mcpAuthBindingsFingerprint(env)).runPromise( (parsed ? OtelTracer.withSpanContext(program, parsed) : program).pipe( Effect.provide(WorkerTelemetryLive), ), @@ -221,7 +258,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, env, authenticate(request)); if (!Predicate.isTagged(outcome, "Authenticated")) { // Destroying a live session on auth grounds requires a POSITIVE // determination that access is genuinely gone — only `Forbidden` carries @@ -269,7 +306,11 @@ export const makeCloudMcpAgentHandler = () => { // oxlint-disable-next-line executor/no-try-catch-or-throw -- adapter boundary: an unrecognized failure is a real defect and must reach the runtime unchanged throw error; } - await runTraced(request, recordDurableObjectFailure(failure, "validate_session_owner")); + await runTraced( + request, + env, + recordDurableObjectFailure(failure, "validate_session_owner"), + ); return durableObjectFailureResponse(failure); } if (owner === "not_found") { @@ -287,7 +328,11 @@ export const makeCloudMcpAgentHandler = () => { } const resource = resourceFromPath(request); - const props = await runTraced(request, propsForPrincipal(request, outcome.principal, resource)); + const props = await runTraced( + request, + env, + propsForPrincipal(request, outcome.principal, resource), + ); (ctx as ExecutionContext & { props?: McpSessionProps }).props = props; const forwarded = withVerifiedIdentityHeaders( request, @@ -338,7 +383,7 @@ export const makeCloudMcpAgentHandler = () => { // oxlint-disable-next-line executor/no-try-catch-or-throw -- adapter boundary: rethrow anything that isn't a recognized platform failure to the Workers runtime unchanged throw error; } - await runTraced(request, recordDurableObjectFailure(failure, "session_fetch")); + await runTraced(request, env, recordDurableObjectFailure(failure, "session_fetch")); return durableObjectFailureResponse(failure); } // The agents SDK answers a bare DELETE with 204; the old envelope's diff --git a/apps/cloud/src/mcp/auth-runtime.node.test.ts b/apps/cloud/src/mcp/auth-runtime.node.test.ts new file mode 100644 index 000000000..6fe8a13bd --- /dev/null +++ b/apps/cloud/src/mcp/auth-runtime.node.test.ts @@ -0,0 +1,161 @@ +import { describe, expect, it } from "@effect/vitest"; +import { Effect, Layer, ManagedRuntime } from "effect"; + +import { ApiKeyService } from "../auth/api-keys"; +import { WorkOSClient, type WorkOSClientService } from "../auth/workos"; +import { + makeBindingKeyedRuntime, + mcpAuthBindingsFingerprint, + type McpAuthBindings, +} from "./auth-runtime"; + +const stubWorkOS = (overrides: Partial) => + Layer.succeed( + WorkOSClient, + new Proxy({} as WorkOSClientService, { + get: (_target, prop) => { + if (prop in overrides) return overrides[prop as keyof WorkOSClientService]; + return () => Effect.die(`unexpected WorkOSClient.${String(prop)} call`); + }, + }), + ); + +// The same shape the memoized MCP auth runtime holds in production: the real +// `ApiKeyService.WorkOS` layer (validation cache included) over a counting +// upstream, one fresh build per `build()` call. +const makeCountingAuthRuntime = () => { + const counters = { upstreamCalls: 0, builds: 0 }; + const layer = ApiKeyService.WorkOS.pipe( + Layer.provide( + stubWorkOS({ + validateApiKey: () => + Effect.sync(() => { + counters.upstreamCalls += 1; + return { + apiKey: { + id: "api_key_123", + owner: { type: "user", id: "user_123", organizationId: "org_123" }, + }, + }; + }), + }), + ), + ); + const runtimeFor = makeBindingKeyedRuntime(() => { + counters.builds += 1; + return ManagedRuntime.make(layer); + }); + // One "request": resolve ApiKeyService from the memoized runtime the + // fingerprint selects and validate the same key, as `runTraced` would. + const request = (fingerprint: string) => + Effect.promise(() => + runtimeFor(fingerprint).runPromise( + Effect.flatMap(ApiKeyService.asEffect(), (apiKeys) => apiKeys.validate("sk_test_abc")), + ), + ); + return { counters, runtimeFor, request }; +}; + +const bindings = (overrides?: Partial): McpAuthBindings => ({ + WORKOS_API_KEY: "sk_workos_1", + WORKOS_CLIENT_ID: "client_1", + WORKOS_COOKIE_PASSWORD: "cookie_password_at_least_32_chars!!", + ...overrides, +}); + +describe("makeBindingKeyedRuntime", () => { + it.effect("reuses the runtime — and its validation cache — while bindings are unchanged", () => + Effect.gen(function* () { + const { counters, runtimeFor, request } = makeCountingAuthRuntime(); + + const first = yield* request("fingerprint_a"); + const second = yield* request("fingerprint_a"); + + expect(counters.builds).toBe(1); + // The second request hit the cached validation, not the upstream. + expect(counters.upstreamCalls).toBe(1); + expect(first?.keyId).toBe("api_key_123"); + expect(second).toEqual(first); + expect(runtimeFor("fingerprint_a")).toBe(runtimeFor("fingerprint_a")); + }), + ); + + it.effect("rebuilds a fresh runtime — resetting the validation cache — on a binding change", () => + Effect.gen(function* () { + const { counters, runtimeFor, request } = makeCountingAuthRuntime(); + + const before = runtimeFor("fingerprint_a"); + yield* request("fingerprint_a"); + expect(counters.upstreamCalls).toBe(1); + + // Rotated bindings: a fresh runtime (fresh ApiKeyService, empty cache), + // so the same key must be re-validated upstream. + yield* request("fingerprint_b"); + expect(counters.builds).toBe(2); + expect(counters.upstreamCalls).toBe(2); + expect(runtimeFor("fingerprint_b")).not.toBe(before); + }), + ); + + it.effect("holds only the latest runtime, so flapping back also rebuilds", () => + Effect.gen(function* () { + const { counters, request } = makeCountingAuthRuntime(); + + yield* request("fingerprint_a"); + yield* request("fingerprint_b"); + yield* request("fingerprint_a"); + + expect(counters.builds).toBe(3); + expect(counters.upstreamCalls).toBe(3); + }), + ); +}); + +describe("mcpAuthBindingsFingerprint", () => { + it("is stable for identical bindings", () => { + expect(mcpAuthBindingsFingerprint(bindings())).toBe(mcpAuthBindingsFingerprint(bindings())); + }); + + it("changes when any captured binding changes", () => { + const base = mcpAuthBindingsFingerprint(bindings()); + expect(mcpAuthBindingsFingerprint(bindings({ WORKOS_API_KEY: "sk_workos_2" }))).not.toBe(base); + expect(mcpAuthBindingsFingerprint(bindings({ WORKOS_CLIENT_ID: "client_2" }))).not.toBe(base); + expect( + mcpAuthBindingsFingerprint( + bindings({ WORKOS_COOKIE_PASSWORD: "another_cookie_password_32_chars!!!" }), + ), + ).not.toBe(base); + expect( + mcpAuthBindingsFingerprint(bindings({ WORKOS_API_URL: "http://127.0.0.1:8788" })), + ).not.toBe(base); + }); + + it("treats an unset WORKOS_API_URL the same as the client does (no override)", () => { + // `workosApiUrlOptions` resolves both to "no override", so they may share + // a fingerprint — a rebuild between them would change nothing. + expect(mcpAuthBindingsFingerprint(bindings({ WORKOS_API_URL: "" }))).toBe( + mcpAuthBindingsFingerprint(bindings()), + ); + }); + + it("never collides when a delimiter-like byte sits inside a value", () => { + // Bindings may contain any byte (workerd Text permits embedded NUL). A + // raw joined fingerprint would let a NUL inside one value shift the field + // boundary so two DIFFERENT binding sets collide — suppressing the + // rebuild a rotation requires. JSON encoding escapes every byte, so these + // two sets, which collide under a NUL join, must fingerprint differently. + const a = mcpAuthBindingsFingerprint( + bindings({ + WORKOS_COOKIE_PASSWORD: "cookie\u0000https://old.example/path", + WORKOS_API_URL: "https://new.example", + }), + ); + const b = mcpAuthBindingsFingerprint( + bindings({ + WORKOS_COOKIE_PASSWORD: "cookie", + WORKOS_API_URL: "https://old.example/path\u0000https://new.example", + }), + ); + expect(a).not.toBe(b); + }); +}); diff --git a/apps/cloud/src/mcp/auth-runtime.ts b/apps/cloud/src/mcp/auth-runtime.ts new file mode 100644 index 000000000..3328c554c --- /dev/null +++ b/apps/cloud/src/mcp/auth-runtime.ts @@ -0,0 +1,85 @@ +// --------------------------------------------------------------------------- +// The per-isolate MCP auth runtime memo, keyed on the bindings it captures. +// +// Cloudflare reuses warm isolates across binding-only deployments, and its +// docs call out exactly the `client ??= new Client(env.SECRET)` pattern as +// retaining stale credentials: a plain `runtime ??= ManagedRuntime.make(...)` +// would keep authenticating with the OLD WorkOS credentials after a +// WORKOS_API_KEY rotation until the isolate happened to be evicted — an auth +// outage once the old key is revoked. So the memo is keyed on a fingerprint +// of exactly the binding values the layer captures at build: same +// fingerprint -> reuse (one string compare per request), changed fingerprint +// -> build fresh. The rebuild also resets the ApiKeyService validation cache +// living inside the layer, which is required — a rotated key must not serve +// cached validations. +// +// The superseded runtime is DROPPED, not disposed. `ManagedRuntime.dispose` +// tears down the runtime's scope, and requests already in flight may still be +// running programs against the old runtime's services — pulling the scope out +// from under them is not safe. Nothing is leaked by dropping: the MCP auth +// layer registers no finalizers (the WorkOS client is plain config; the +// org-auth postgres socket is built and closed per `authorize` call), so an +// unreferenced runtime is simply GC'd once the last in-flight request +// finishes. Rotations only happen on deploys, and at most one previous +// runtime is ever still referenced by in-flight work. +// +// This lives in its own `cloudflare:workers`-free leaf so the node-pool unit +// tests can exercise the rebuild semantics; agent-handler.ts is the one +// production consumer. +// --------------------------------------------------------------------------- + +import type { ManagedRuntime } from "effect"; + +/** The env subset the MCP auth layer reads at build time. */ +export type McpAuthBindings = Pick< + Env, + "WORKOS_API_KEY" | "WORKOS_CLIENT_ID" | "WORKOS_COOKIE_PASSWORD" | "WORKOS_API_URL" +>; + +/** + * The exact binding values `cloudMcpAuth` captures when its runtime is built — + * `WorkOSClient`'s `make` (auth/workos.ts) reads precisely these four. A + * joined string is enough to answer "did any of them change"; it CONTAINS the + * live WORKOS_API_KEY, so it must never reach a log, span, or error message. + * `WORKOS_API_URL` is genuinely optional (unset in production), so absence is + * encoded as the empty string — the same "no override" the client resolves it + * to (see `workosApiUrlOptions`). + * + * JSON-encoded rather than delimiter-joined: bindings may contain any byte + * (workerd Text permits embedded NUL), so a raw join is not injective — a + * delimiter byte inside one value could make two different binding sets + * collide and suppress the rebuild a rotation requires. JSON escapes every + * byte, so distinct value tuples always produce distinct fingerprints. + * + * Not included: the module-scope env reads in mcp/auth.ts (AUTHKIT_DOMAIN, + * RESOURCE_ORIGIN, the JWKS cache, the JWT audience). Those are frozen at the + * isolate's first module evaluation, so rebuilding the runtime cannot refresh + * them — keying on them would force rebuilds that change nothing. + */ +export const mcpAuthBindingsFingerprint = (env: McpAuthBindings): string => + JSON.stringify([ + env.WORKOS_API_KEY, + env.WORKOS_CLIENT_ID, + env.WORKOS_COOKIE_PASSWORD, + env.WORKOS_API_URL ?? "", + ]); + +/** + * Memoize one built runtime per fingerprint value (holding only the latest): + * an unchanged fingerprint returns the existing runtime, a changed one drops + * it (see the header for why dropping, not disposing, is correct) and builds + * a fresh runtime via `build`. + */ +export const makeBindingKeyedRuntime = ( + build: () => ManagedRuntime.ManagedRuntime, +): ((fingerprint: string) => ManagedRuntime.ManagedRuntime) => { + let runtime: ManagedRuntime.ManagedRuntime | undefined; + let fingerprint: string | undefined; + return (next) => { + if (runtime === undefined || fingerprint !== next) { + runtime = build(); + fingerprint = next; + } + return runtime; + }; +}; diff --git a/e2e/cloud/api-key-validation-cache.test.ts b/e2e/cloud/api-key-validation-cache.test.ts new file mode 100644 index 000000000..9572bd04e --- /dev/null +++ b/e2e/cloud/api-key-validation-cache.test.ts @@ -0,0 +1,272 @@ +// Cloud: an Executor API key presented as a bearer is validated against +// WorkOS once per TTL window, not once per request. Every API-key- +// authenticated request used to buy a live `POST /api_keys/validations` +// round trip; with the per-isolate success cache, the first request pays it +// and back-to-back follow-ups ride the cached owner. +// +// Pinned black-box at the real upstream: the WorkOS emulator the cloud stack +// boots against keeps a request ledger, and a validation request carries the +// presented key value in its body — so "how many times was THIS key +// validated" is readable from the upstream's own records. The guarantees: +// +// 1. Back-to-back requests on one key land exactly ONE validation on +// WorkOS. Without the cache this is one validation per request. +// 2. A DIFFERENT key is validated on its own first request — the cache is +// keyed by credential, so key B neither rides key A's cached owner +// (which would be a cross-credential auth hole) nor disturbs key A's +// entry. +// +// Two scenarios because the two bearer surfaces wire the cache differently: +// +// - /api/* resolves `ApiKeyService` from the app's boot layer, built once +// per isolate. +// - /mcp is served outside the app envelope by the agent handler, which +// builds `cloudMcpAuth` once per isolate on a memoized ManagedRuntime +// (apps/cloud/src/mcp/agent-handler.ts). It used to provide that layer +// per request — a fresh `ApiKeyService`, a fresh cache map, a WorkOS +// round trip on every MCP request — so this scenario pins the shared +// wiring, not just the cache. +// +// Attribution: the emulator redacts secret-named fields (the response's +// `api_key` object), but the request body's `value` survives, and every key +// minted here is unique to this run — so filtering the shared suite-wide +// ledger by the key's own value is collision-free. +import { expect } from "@effect/vitest"; +import { Effect } from "effect"; +import { AccountHttpApi } from "@executor-js/api"; +import { connectEmulator, type EmulatorClient, type LedgerEntry } from "@executor-js/emulate"; + +import { scenario } from "../src/scenario"; +import { Api, Target } from "../src/services"; +import { WORKOS_EMULATOR_PORT } from "../targets/cloud"; + +const JSON_AND_SSE = "application/json, text/event-stream"; +const PROTOCOL_VERSION = "2025-03-26"; + +/** The WorkOS route `apiKeys.validateApiKey` posts to — the round trip the + * cache exists to avoid. */ +const VALIDATIONS_PATH = "/api_keys/validations"; + +const INITIALIZE_REQUEST = { + jsonrpc: "2.0" as const, + id: 1, + method: "initialize", + params: { + protocolVersion: PROTOCOL_VERSION, + capabilities: {}, + clientInfo: { name: "executor-e2e-apikey-cache", version: "0.0.1" }, + }, +}; + +const INITIALIZED_NOTIFICATION = { + jsonrpc: "2.0" as const, + method: "notifications/initialized", +}; + +const toolsList = (id: number) => ({ + jsonrpc: "2.0" as const, + id, + method: "tools/list", + params: {}, +}); + +const mcpPost = ( + url: string, + init: { + readonly bearer: string; + readonly sessionId?: string; + readonly body: unknown; + }, +): Promise => + fetch(url, { + method: "POST", + headers: { + accept: JSON_AND_SSE, + "content-type": "application/json", + authorization: `Bearer ${init.bearer}`, + ...(init.sessionId ? { "mcp-session-id": init.sessionId } : {}), + }, + body: JSON.stringify(init.body), + }); + +/** initialize → session id → notifications/initialized, all on the API key. */ +const openSession = async (mcpUrl: string, bearer: string): Promise => { + const initialize = await mcpPost(mcpUrl, { bearer, body: INITIALIZE_REQUEST }); + const sessionId = initialize.headers.get("mcp-session-id"); + await initialize.text(); + if (initialize.status !== 200 || !sessionId) { + throw new Error(`openSession: initialize failed (${initialize.status})`); + } + const initialized = await mcpPost(mcpUrl, { + bearer, + sessionId, + body: INITIALIZED_NOTIFICATION, + }); + await initialized.text(); + if (initialized.status !== 202) { + throw new Error(`openSession: notifications/initialized failed (${initialized.status})`); + } + return sessionId; +}; + +/** One tools/list on an open session; the response must be a real tool list, + * so a validation count of one can never come from requests that bounced at + * the door. */ +const listTools = async ( + mcpUrl: string, + init: { readonly bearer: string; readonly sessionId: string; readonly id: number }, +): Promise => { + const response = await mcpPost(mcpUrl, { + bearer: init.bearer, + sessionId: init.sessionId, + body: toolsList(init.id), + }); + const text = await response.text(); + if (response.status !== 200 || !text.includes('"tools"')) { + throw new Error(`listTools(id=${init.id}): failed (${response.status}): ${text.slice(0, 200)}`); + } +}; + +/** One API-key-authenticated read of the protected API; the response must be + * a real integration listing (same reason as `listTools`). */ +const listIntegrations = async (baseUrl: string, bearer: string): Promise => { + const response = await fetch(new URL("/api/integrations", baseUrl), { + headers: { authorization: `Bearer ${bearer}` }, + }); + const text = await response.text(); + if (response.status !== 200 || !text.includes('"slug"')) { + throw new Error(`listIntegrations: failed (${response.status}): ${text.slice(0, 200)}`); + } +}; + +/** The ledger entries in which WorkOS was asked to validate THIS key value. */ +const validationsOf = ( + entries: ReadonlyArray, + keyValue: string, +): ReadonlyArray => + entries.filter( + (entry) => + entry.path === VALIDATIONS_PATH && + typeof entry.request.body === "object" && + entry.request.body !== null && + (entry.request.body as { readonly value?: unknown }).value === keyValue, + ); + +/** The shared fixtures both scenarios start from: two freshly minted keys + * (revoked on exit) and the WorkOS emulator the target validates against. */ +const mintTwoKeys = Effect.gen(function* () { + const target = yield* Target; + const { client: apiClient } = yield* Api; + const identity = yield* target.newIdentity(); + const client = yield* apiClient(AccountHttpApi, identity); + + const workos: EmulatorClient = yield* Effect.promise(() => + connectEmulator({ baseUrl: `http://127.0.0.1:${WORKOS_EMULATOR_PORT}` }), + ); + + const mintKey = (name: string) => + Effect.gen(function* () { + const created = yield* client.account.createApiKey({ payload: { name } }); + yield* Effect.addFinalizer(() => + client.account.revokeApiKey({ params: { apiKeyId: created.id } }).pipe(Effect.ignore), + ); + return created; + }); + + const keyA = yield* mintKey("e2e validation-cache key A"); + const keyB = yield* mintKey("e2e validation-cache key B"); + + const readValidations = (keyValue: string) => + Effect.promise(async () => validationsOf(await workos.ledger.list(500), keyValue)); + + return { target, keyA, keyB, readValidations } as const; +}); + +scenario( + "API keys · back-to-back API requests validate the key with WorkOS once, and each key validates independently", + {}, + Effect.scoped( + Effect.gen(function* () { + const { target, keyA, keyB, readValidations } = yield* mintTwoKeys; + + // ── Key A: two authenticated reads back to back ─────────────────────── + // Without the cache each request is its own WorkOS round trip; with it, + // only the first request's miss reaches WorkOS. + yield* Effect.promise(() => listIntegrations(target.baseUrl, keyA.value)); + yield* Effect.promise(() => listIntegrations(target.baseUrl, keyA.value)); + + const afterA = yield* readValidations(keyA.value); + expect( + afterA.map((entry) => entry.summary), + "back-to-back API requests cost exactly one WorkOS validation", + ).toHaveLength(1); + + // ── Key B: a different credential misses the cache on its own ───────── + // Key B's first request MUST reach WorkOS: a zero here would mean the + // cache handed key A's owner to a different credential. + yield* Effect.promise(() => listIntegrations(target.baseUrl, keyB.value)); + + const afterB = yield* readValidations(keyB.value); + expect( + afterB.map((entry) => entry.summary), + "a different key is validated on its own first request", + ).toHaveLength(1); + + // And key B's request did not spend or duplicate key A's cache entry. + const finalA = yield* readValidations(keyA.value); + expect( + finalA.map((entry) => entry.summary), + "key A's single validation is undisturbed by key B's request", + ).toHaveLength(1); + }), + ), +); + +scenario( + "MCP · back-to-back API-key requests validate the key with WorkOS once, and each key validates independently", + {}, + Effect.scoped( + Effect.gen(function* () { + const { target, keyA, keyB, readValidations } = yield* mintTwoKeys; + + // ── Key A: one session, two tool-surface calls back to back ────────── + // Four API-key-authenticated requests in total (initialize, initialized, + // tools/list ×2). Without the cache each one is its own WorkOS round + // trip; with it, only the first request's miss reaches WorkOS. + const sessionA = yield* Effect.promise(() => openSession(target.mcpUrl, keyA.value)); + yield* Effect.promise(() => + listTools(target.mcpUrl, { bearer: keyA.value, sessionId: sessionA, id: 2 }), + ); + yield* Effect.promise(() => + listTools(target.mcpUrl, { bearer: keyA.value, sessionId: sessionA, id: 3 }), + ); + + const afterA = yield* readValidations(keyA.value); + expect( + afterA.map((entry) => entry.summary), + "the whole API-key session cost exactly one WorkOS validation", + ).toHaveLength(1); + + // ── Key B: a different credential misses the cache on its own ──────── + // Key B's first request MUST reach WorkOS: a zero here would mean the + // cache handed key A's owner to a different credential. + const sessionB = yield* Effect.promise(() => openSession(target.mcpUrl, keyB.value)); + yield* Effect.promise(() => + listTools(target.mcpUrl, { bearer: keyB.value, sessionId: sessionB, id: 2 }), + ); + + const afterB = yield* readValidations(keyB.value); + expect( + afterB.map((entry) => entry.summary), + "a different key is validated on its own first request", + ).toHaveLength(1); + + // And key B's session did not spend or duplicate key A's cache entry. + const finalA = yield* readValidations(keyA.value); + expect( + finalA.map((entry) => entry.summary), + "key A's single validation is undisturbed by key B's session", + ).toHaveLength(1); + }), + ), +);