Invalidate cached api key validations on revoke

.changeset/apikey-revoke-cache-invalidation.md

@@ -0,0 +1,7 @@
+---
+"@executor-js/cloud": patch
+---
+
+**Revoking an API key now invalidates its cached validation**
+
+The per-isolate validation cache made a revoked key keep authenticating for up to 60 seconds. Two fixes close that: the cache map now lives at module scope, shared by every build of the key service (the account middleware rebuilds it per request, so a per-build map left the revoke invalidating a fresh empty cache), and both revoke paths drop the revoked key's entries from it. A revoked key is refused on the next request served by the isolate that processed the revoke; other isolates still age the entry out within the TTL.

apps/cloud/src/auth/api-key-validation-cache.node.test.ts

@@ -171,10 +171,46 @@ describe("makeCachedApiKeyValidate", () => {
       expect(cached.cacheKeys()).not.toContain(rawKey);
     }),
   );
+
+  it.effect("invalidating a key id drops its entries and forces revalidation", () =>
+    Effect.gen(function* () {
+      let calls = 0;
+      const cached = makeCachedApiKeyValidate(() =>
+        Effect.sync(() => {
+          calls += 1;
+          return userOwner("api_key_1");
+        }),
+      );
+
+      yield* cached.validate("sk_test_abc");
+      cached.invalidateKeyId("api_key_1");
+      expect(cached.cacheKeys()).toHaveLength(0);
+
+      yield* cached.validate("sk_test_abc");
+      expect(calls).toBe(2);
+    }),
+  );
+
+  it.effect("invalidating one key id leaves other keys' entries cached", () =>
+    Effect.gen(function* () {
+      const cached = makeCachedApiKeyValidate((value) =>
+        Effect.succeed(userOwner(`key_for_${value}`)),
+      );
+
+      yield* cached.validate("sk_1");
+      yield* cached.validate("sk_2");
+      cached.invalidateKeyId("key_for_sk_1");
+
+      expect(cached.cacheKeys()).toHaveLength(1);
+    }),
+  );
 });
 
+// These tests build the REAL WorkOS layer, whose cache map is module-scope —
+// entries written by one test are visible to the next within this file. Each
+// test therefore uses key values of its own; none may reuse another's.
 describe("ApiKeyService.WorkOS validation cache", () => {
-  it.effect("validates a repeated key through one upstream call per layer build", () =>
+  it.effect("serves a key cached by one layer build from a second build", () =>
     Effect.gen(function* () {
       let calls = 0;
       const layer = ApiKeyService.WorkOS.pipe(
@@ -185,7 +221,7 @@ describe("ApiKeyService.WorkOS validation cache", () => {
                 calls += 1;
                 return {
                   apiKey: {
-                    id: "api_key_123",
+                    id: "api_key_shared",
                     owner: { type: "user", id: "user_123", organizationId: "org_123" },
                   },
                 };
@@ -194,17 +230,74 @@ describe("ApiKeyService.WorkOS validation cache", () => {
         ),
       );
 
-      const { first, second } = yield* Effect.gen(function* () {
+      // Two separate `Effect.provide(layer)` runs are two separate layer
+      // builds — the account middleware rebuilds this layer per request, so
+      // cross-build reuse is exactly what production needs the cache to do.
+      const first = yield* Effect.gen(function* () {
         const apiKeys = yield* ApiKeyService;
-        return {
-          first: yield* apiKeys.validate("sk_test_abc"),
-          second: yield* apiKeys.validate("sk_test_abc"),
-        };
+        return yield* apiKeys.validate("sk_test_cross_build");
+      }).pipe(Effect.provide(layer));
+      const second = yield* Effect.gen(function* () {
+        const apiKeys = yield* ApiKeyService;
+        return yield* apiKeys.validate("sk_test_cross_build");
       }).pipe(Effect.provide(layer));
 
       expect(calls).toBe(1);
-      expect(first?.keyId).toBe("api_key_123");
+      expect(first?.keyId).toBe("api_key_shared");
       expect(second).toEqual(first);
     }),
   );
+
+  it.effect("a revoke in one layer build refuses the cached value in every build", () =>
+    Effect.gen(function* () {
+      // Live until revoked: the stub answers with the key while `revoked` is
+      // false and with a null apiKey afterwards, the same flip the WorkOS
+      // backend performs. The revoke runs in its OWN layer build — the shape
+      // of the real console flow, where the account middleware rebuilds the
+      // layer per request — and must still drop the entry the validating
+      // plane cached.
+      let revoked = false;
+      let calls = 0;
+      const layer = ApiKeyService.WorkOS.pipe(
+        Layer.provide(
+          stubWorkOS({
+            validateApiKey: () =>
+              Effect.sync(() => {
+                calls += 1;
+                return {
+                  apiKey: revoked
+                    ? null
+                    : {
+                        id: "api_key_revoked",
+                        owner: { type: "user", id: "user_123", organizationId: "org_123" },
+                      },
+                };
+              }),
+            deleteApiKey: () =>
+              Effect.sync(() => {
+                revoked = true;
+                return {};
+              }),
+          }),
+        ),
+      );
+
+      const before = yield* Effect.gen(function* () {
+        const apiKeys = yield* ApiKeyService;
+        return yield* apiKeys.validate("sk_test_revocable");
+      }).pipe(Effect.provide(layer));
+      yield* Effect.gen(function* () {
+        const apiKeys = yield* ApiKeyService;
+        return yield* apiKeys.revokeUserKey({ keyId: "api_key_revoked" });
+      }).pipe(Effect.provide(layer));
+      const after = yield* Effect.gen(function* () {
+        const apiKeys = yield* ApiKeyService;
+        return yield* apiKeys.validate("sk_test_revocable");
+      }).pipe(Effect.provide(layer));
+
+      expect(before?.keyId).toBe("api_key_revoked");
+      expect(after, "the revoke dropped the cached entry, not just the upstream key").toBeNull();
+      expect(calls).toBe(2);
+    }),
+  );
 });

apps/cloud/src/auth/api-keys.node.test.ts

@@ -1,9 +1,14 @@
-import { describe, expect, it } from "@effect/vitest";
+import { beforeEach, describe, expect, it } from "@effect/vitest";
 import { Effect, Layer } from "effect";
 
-import { ApiKeyService } from "./api-keys";
+import { ApiKeyService, resetApiKeyValidationCacheForTest } from "./api-keys";
 import { WorkOSClient, type WorkOSClientService } from "./workos";
 
+// Every case validates the same "test_key" value through the REAL layer, whose
+// success cache is module-scope — without a reset, one case's decoded owner
+// would be served to the next in place of its own stubbed response.
+beforeEach(() => resetApiKeyValidationCacheForTest());
+
 const stubWorkOS = (overrides: Partial<WorkOSClientService>) =>
   Layer.succeed(
     WorkOSClient,

apps/cloud/src/auth/api-keys.ts

@@ -296,10 +296,14 @@ const createdFromResponse = (value: unknown): CreatedApiKey | null =>
 //     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.
+//   - REVOCATION WINDOW: the isolate that serves the revoke drops its own
+//     entries for that key id immediately (`invalidateKeyId`, called by both
+//     revoke paths below), so revoking a key through the console refuses it
+//     on the next request wherever the same isolate validates. OTHER isolates
+//     that validated the key before revocation keep serving it for up to the
+//     TTL (60s). That residual window 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
@@ -312,36 +316,68 @@ const API_KEY_VALIDATION_CACHE_TTL_MS = 60_000;
 // map unbounded (mirrors BALANCE_CACHE_MAX_ENTRIES in execution-gate.ts).
 const API_KEY_VALIDATION_CACHE_MAX_ENTRIES = 10_000;
 
+type ApiKeyValidationCacheEntry = {
+  readonly owner: ApiKeyOwner;
+  readonly expiresAtMs: number;
+};
+
+// The map lives at MODULE scope — one per isolate — not inside
+// `makeCachedApiKeyValidate`, because the `ApiKeyService.WorkOS` layer is NOT
+// built once per isolate on every plane: the account middleware rebuilds it
+// per request. A map owned by each build would give the revoke paths a fresh
+// empty cache to invalidate while the long-lived identity plane kept serving
+// the revoked key from its own. Every build shares this map instead (entries
+// are pure validation RESULTS, so which build's WorkOS client wrote them does
+// not matter), which is also what makes the entries survive between requests
+// on the per-request plane at all.
+const isolateApiKeyValidationCache = new Map<string, ApiKeyValidationCacheEntry>();
+
+/** Test-only: module state outlives a single test case (mirrors
+ *  `resetBuildSlotsForTest` in mcp/session-build-semaphore.ts). */
+export const resetApiKeyValidationCacheForTest = (): void => {
+  isolateApiKeyValidationCache.clear();
+};
+
 export type ApiKeyValidate = (
   value: string,
 ) => Effect.Effect<ApiKeyOwner | null, ApiKeyValidationError>;
 
 /**
- * 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.
+ * Wrap a `validate` function with the success cache described above. Exported
+ * for tests only — production use is the `ApiKeyService.WorkOS` layer below,
+ * every build of which shares the module-scope map.
  *
- * The `ttlMs` / `maxEntries` / `now` knobs exist so tests can exercise expiry
- * and the size bound without waiting on wall time; production passes nothing.
+ * The `ttlMs` / `maxEntries` / `now` / `cache` knobs exist so tests can
+ * exercise expiry, the size bound, and map sharing without wall time or
+ * module state; production passes only `cache`.
  */
 export const makeCachedApiKeyValidate = (
   validate: ApiKeyValidate,
   options?: {
     readonly ttlMs?: number;
     readonly maxEntries?: number;
     readonly now?: () => number;
+    /** The map to cache into. Production passes the module-scope
+     *  `isolateApiKeyValidationCache`; tests default to a private map. */
+    readonly cache?: Map<string, ApiKeyValidationCacheEntry>;
   },
 ): {
   readonly validate: ApiKeyValidate;
+  /**
+   * Drop every cached entry that resolved to this key id. The cache is keyed
+   * by the digest of the presented VALUE and a revoke only knows the key ID,
+   * so this walks the map — bounded by `maxEntries`, and revocation is a rare
+   * human action, not a hot path.
+   */
+  readonly invalidateKeyId: (keyId: string) => void;
   /** Test seam: the digests currently cached, so tests can assert the raw
    *  credential never appears as a map key. */
   readonly cacheKeys: () => ReadonlyArray<string>;
 } => {
   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<string, { readonly owner: ApiKeyOwner; readonly expiresAtMs: number }>();
+  const cache = options?.cache ?? new Map<string, ApiKeyValidationCacheEntry>();
 
   const writeCache = (digest: string, owner: ApiKeyOwner, nowMs: number): void => {
     if (cache.size >= maxEntries) {
@@ -356,6 +392,11 @@ export const makeCachedApiKeyValidate = (
 
   return {
     cacheKeys: () => [...cache.keys()],
+    invalidateKeyId: (keyId) => {
+      for (const [digest, entry] of cache) {
+        if (entry.owner.keyId === keyId) cache.delete(digest);
+      }
+    },
     validate: (value) =>
       Effect.gen(function* () {
         const digest = yield* sha256Hex(value);
@@ -414,12 +455,17 @@ 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 })),
-        ),
+      // The cache MAP is the module-scope per-isolate one (see its comment);
+      // this build only contributes the validate function that fills it, so
+      // however many times a plane rebuilds this layer, hits, misses, and
+      // invalidations all land in the same map.
+      const cachedValidate = makeCachedApiKeyValidate(
+        (value) =>
+          workos.validateApiKey(value).pipe(
+            Effect.map(ownerFromResponse),
+            Effect.mapError((cause) => new ApiKeyValidationError({ cause })),
+          ),
+        { cache: isolateApiKeyValidationCache },
       );
       return {
         validate: cachedValidate.validate,
@@ -439,9 +485,10 @@ export class ApiKeyService extends Context.Service<
             }),
           ),
         revokeUserKey: ({ keyId }) =>
-          workos
-            .deleteApiKey(keyId)
-            .pipe(Effect.mapError((cause) => new ApiKeyManagementError({ cause }))),
+          workos.deleteApiKey(keyId).pipe(
+            Effect.mapError((cause) => new ApiKeyManagementError({ cause })),
+            Effect.tap(() => Effect.sync(() => cachedValidate.invalidateKeyId(keyId))),
+          ),
         listOrgKeys: ({ organizationId }) =>
           workos.listOrgApiKeys(organizationId).pipe(
             Effect.map((response) => orgListFromResponse(response, organizationId)),
@@ -475,6 +522,7 @@ export class ApiKeyService extends Context.Service<
             yield* workos
               .deleteApiKey(keyId)
               .pipe(Effect.mapError((cause) => new ApiKeyManagementError({ cause })));
+            cachedValidate.invalidateKeyId(keyId);
           }),
       };
     }),

apps/cloud/src/mcp/agent-handler.ts

@@ -133,25 +133,26 @@ const authenticate = (request: Request) =>
 
 // 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).
+// WorkOS client and `ApiKeyService` — on every MCP request. 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.
+// fresh runtime when the bindings changed. The api-key validation success
+// cache is NOT swapped with it — it is module-scope in api-keys.ts (shared
+// with the /api/* plane so revocation can invalidate it everywhere), so after
+// a WORKOS_API_KEY rotation its remaining entries age out within the 60s TTL
+// rather than dropping instantly. See auth-runtime.ts for the drop-vs-dispose
+// reasoning.
 const mcpAuthRuntimeFor: (
   fingerprint: string,
 ) => ManagedRuntime.ManagedRuntime<McpAuthProvider, never> = makeBindingKeyedRuntime(() =>