fix(cloud): require CSRF state in the WorkOS login callback

.changeset/tidy-login-state.md

@@ -0,0 +1,17 @@
+---
+"@executor-js/cloud": patch
+---
+
+fix: make login CSRF state mandatory in the WorkOS callback
+
+The callback previously skipped its CSRF check whenever the redirect carried
+no `state` value ("some WorkOS-initiated redirects don't include one"). That
+bypass let an attacker complete their own OAuth round-trip and redirect a
+victim's browser through the callback with the attacker's `code` and no
+`state`, silently signing the victim into the attacker's account (login CSRF).
+
+The check is now unconditional: a callback without a state matching the
+`wos-login-state` cookie set on `/login` is rejected with 400. This is a
+breaking change for any client relying on the undocumented no-state entry
+path; server-initiated flows that cannot carry state must be redesigned with
+a signed nonce instead of re-adding the bypass.

apps/cloud/src/auth/handlers.ts

@@ -189,17 +189,17 @@ export const CloudAuthPublicHandlers = HttpApiBuilder.group(
           const workos = yield* WorkOSClient;
           const users = yield* UserStoreService;
           const cookieState = request.cookies[STATE_COOKIE] ?? null;
-          // CSRF check is only enforced when the redirect carries a state
-          // value — some WorkOS-initiated redirects don't include one.
-          // When state is present, it MUST match the cookie we set on
-          // /login.
-          if (query.state !== undefined) {
-            if (!cookieState || !timingSafeEqual(cookieState, query.state)) {
-              return deleteResponseCookie(
-                HttpServerResponse.text("Invalid login state", { status: 400 }),
-                STATE_COOKIE,
-              );
-            }
+          // CSRF is unconditional: every callback must carry a state that
+          // matches the cookie set on /login. There is no legitimate
+          // no-state entry path — omitting state previously allowed an
+          // attacker to complete their own OAuth round-trip and redirect a
+          // victim's browser through this callback, signing the victim into
+          // the attacker's account (login CSRF).
+          if (!cookieState || !timingSafeEqual(cookieState, query.state ?? "")) {
+            return deleteResponseCookie(
+              HttpServerResponse.text("Invalid login state", { status: 400 }),
+              STATE_COOKIE,
+            );
           }
 
           const result = yield* workos.authenticateWithCode(query.code);
@@ -210,7 +210,7 @@ export const CloudAuthPublicHandlers = HttpApiBuilder.group(
           let sealedSession = result.sealedSession;
 
           // Resume where the SSR gate interrupted them. The state passed the
-          // CSRF check above whenever it's present, but it's still a
+          // CSRF check above, but it's still a
           // round-tripped value, so the returnTo inside it is re-validated like
           // any other untrusted path.
           const returnTo = safeReturnTo(decodeLoginState(query.state)?.returnTo) ?? "/";

apps/cloud/src/auth/workos-callback-state.node.test.ts

@@ -0,0 +1,166 @@
+// ---------------------------------------------------------------------------
+// Focused tests — the WorkOS login callback's CSRF gate.
+//
+// The callback's CSRF check must be unconditional: no state ⇒ 400 before any
+// WorkOS call; a replayed (already consumed) state ⇒ 400; a fresh state
+// matching the cookie ⇒ 302 + session.
+//
+// Test seams follow repo conventions: @effect/vitest, Layer.succeed stubs
+// (see org-selector-auth.node.test.ts), and HttpRouter.toWebHandler for the
+// HTTP surface (see api.request-scope.node.test.ts).
+// ---------------------------------------------------------------------------
+
+import { afterAll, describe, expect, it } from "@effect/vitest";
+import { Effect, Layer } from "effect";
+import { HttpRouter, HttpServer } from "effect/unstable/http";
+import { HttpApiBuilder } from "effect/unstable/httpapi";
+import { HttpApi } from "effect/unstable/httpapi";
+
+import { CloudAuthPublicHandlers } from "./handlers";
+import { CloudAuthPublicApi } from "./api";
+import { UserStoreService } from "./context";
+import { WorkOSClient, type WorkOSClientService } from "./workos";
+import { encodeLoginState } from "./login-state";
+
+// The route under test serves under the `/api` prefix in the composed app;
+// toWebHandler mounts the raw group, so paths here are relative to the group.
+const SESSION_COOKIE = "wos-session";
+const STATE_COOKIE = "wos-login-state";
+
+const STUB_USER_ID = "user_test";
+const STUB_SESSION = "sealed-session-stub";
+const STUB_ORG_ID = "org_test";
+
+const stubWorkOS = Layer.succeed(
+  WorkOSClient,
+  new Proxy({} as WorkOSClientService, {
+    get: (_t, prop) => {
+      if (prop === "authenticateWithCode") {
+        return () =>
+          Effect.succeed({
+            user: { id: STUB_USER_ID, email: "u@test" },
+            organizationId: STUB_ORG_ID,
+            sealedSession: STUB_SESSION,
+          });
+      }
+      if (prop === "listUserMemberships") {
+        return () => Effect.succeed({ data: [] });
+      }
+      return () => Effect.die(`unexpected WorkOSClient.${String(prop)} call`);
+    },
+  }),
+);
+
+const stubUsers = Layer.succeed(UserStoreService)({
+  use: (_op, fn) =>
+    Effect.promise(() =>
+      fn({
+        ensureAccount: async (id: string) => ({ id, createdAt: new Date() }),
+        getAccount: async (id: string) => ({ id, createdAt: new Date() }),
+        upsertOrganization: async (org: { id: string; name: string }) => ({
+          ...org,
+          slug: org.id,
+          createdAt: new Date(),
+        }),
+        getOrganization: async (id: string) => ({
+          id,
+          name: "Org " + id,
+          slug: id,
+          createdAt: new Date(),
+        }),
+        getOrganizationBySlug: async (slug: string) => ({
+          id: slug,
+          name: slug,
+          slug,
+          createdAt: new Date(),
+        }),
+        deleteOrganizationCascade: async () => {},
+      }),
+    ),
+});
+
+// Only the public group is under test; the session group (and its SessionAuth
+// middleware, which needs a live DB) is out of scope — the callback route lives
+// in CloudAuthPublicApi and requires no middleware.
+const PublicApi = HttpApi.make("cloudWeb").add(CloudAuthPublicApi);
+
+const App = HttpApiBuilder.layer(PublicApi).pipe(
+  Layer.provide(CloudAuthPublicHandlers),
+  Layer.provide(stubWorkOS),
+  Layer.provide(stubUsers),
+  Layer.provide(HttpServer.layerServices),
+);
+
+const app = HttpRouter.toWebHandler(App, { disableLogger: true });
+afterAll(() => app.dispose());
+
+const run = (request: Request) => {
+  // beta.59: the handler type expects a context argument; this layer stack
+  // needs none at runtime — pass undefined like the api.request-scope tests.
+  return app.handler(request, undefined as never);
+};
+
+const callbackUrl = (state?: string, code = "code_1") =>
+  `https://executor.test/auth/callback${state ? `?state=${encodeURIComponent(state)}` : ""}${state ? "&" : "?"}code=${code}`;
+
+describe("workos callback · CSRF state hardening", () => {
+  it("rejects a callback with NO state (the former bypass) before any WorkOS call", async () => {
+    const res = await run(new Request(callbackUrl(undefined), { redirect: "manual" }));
+    expect(res.status).toBe(400);
+    expect(await res.text()).toContain("Invalid login state");
+    expect(res.headers.get("set-cookie") ?? "").not.toContain(SESSION_COOKIE);
+  });
+
+  it("rejects missing state even when the browser has a login cookie", async () => {
+    const res = await run(
+      new Request(callbackUrl(undefined), {
+        headers: { cookie: `${STATE_COOKIE}=victim-login-state` },
+        redirect: "manual",
+      }),
+    );
+    expect(res.status).toBe(400);
+    expect(await res.text()).toBe("Invalid login state");
+    expect(res.headers.get("set-cookie") ?? "").not.toContain(SESSION_COOKIE);
+  });
+
+  it("rejects a state that does not match the login cookie", async () => {
+    const res = await run(
+      new Request(callbackUrl("attacker-controlled-state"), {
+        headers: { cookie: `${STATE_COOKIE}=victim-login-state` },
+        redirect: "manual",
+      }),
+    );
+    expect(res.status).toBe(400);
+    expect(await res.text()).toContain("Invalid login state");
+  });
+
+  it("accepts a fresh state matching the cookie and issues a session (302 + cookie)", async () => {
+    // /login sets the cookie; simulate its value for this callback.
+    const state = encodeLoginState({ nonce: "nonce-123", returnTo: "/" });
+    const res = await run(
+      new Request(callbackUrl(state), {
+        headers: { cookie: `${STATE_COOKIE}=${state}` },
+        redirect: "manual",
+      }),
+    );
+    expect(res.status).toBe(302);
+    expect(res.headers.get("set-cookie") ?? "").toContain(SESSION_COOKIE);
+  });
+
+  it("rejects a replayed state (single-use contract preserved downstream)", async () => {
+    // Replay of a state whose cookie is gone (already consumed by the login
+    // round-trip) must fail closed.
+    const state = encodeLoginState({ nonce: "nonce-replay", returnTo: "/" });
+    const first = await run(
+      new Request(callbackUrl(state), {
+        headers: { cookie: `${STATE_COOKIE}=${state}` },
+        redirect: "manual",
+      }),
+    );
+    expect(first.status).toBe(302);
+
+    // Second callback: same state, no cookie (session-store consumed it).
+    const replay = await run(new Request(callbackUrl(state), { redirect: "manual" }));
+    expect(replay.status).toBe(400);
+  });
+});

apps/cloud/src/mcp/session-build-semaphore.test.ts

@@ -1,4 +1,4 @@
-import { describe, expect, it, beforeEach } from "@effect/vitest";
+import { describe, expect, it, beforeEach, afterEach, vi } from "@effect/vitest";
 
 import {
   acquireBuildSlot,
@@ -13,6 +13,10 @@ describe("session-build-semaphore", () => {
     resetBuildSlotsForTest();
   });
 
+  afterEach(() => {
+    vi.useRealTimers();
+  });
+
   it("grants up to the cap immediately, with no wait", async () => {
     const results = await Promise.all([
       acquireBuildSlot().promise,
@@ -214,6 +218,7 @@ describe("session-build-semaphore", () => {
   });
 
   it("proceeds without a slot when the queue wait exceeds the timeout, and does not count it as active", async () => {
+    vi.useFakeTimers();
     await Promise.all([
       acquireBuildSlot().promise,
       acquireBuildSlot().promise,
@@ -223,6 +228,10 @@ describe("session-build-semaphore", () => {
     expect(currentActiveBuildsForTest()).toBe(4);
 
     const timedOutHandle = acquireBuildSlot(10);
+    await vi.advanceTimersByTimeAsync(9);
+    expect(currentQueueLengthForTest()).toBe(1);
+    expect(currentActiveBuildsForTest()).toBe(4);
+    await vi.advanceTimersByTimeAsync(1);
     const result = await timedOutHandle.promise;
 
     expect(result).toEqual({ acquired: false, waitMs: expect.any(Number), timedOut: true });

e2e/cloud/login-csrf.test.ts

@@ -0,0 +1,75 @@
+import { randomUUID } from "node:crypto";
+
+import { expect } from "@effect/vitest";
+import { Effect } from "effect";
+
+import { scenario } from "../src/scenario";
+import { Browser, Target } from "../src/services";
+
+scenario(
+  "Login CSRF · state is required, bound to the browser, and consumed after login",
+  { timeout: 180_000 },
+  Effect.gen(function* () {
+    const target = yield* Target;
+    const browser = yield* Browser;
+    const email = `csrf-${randomUUID()}@e2e.test`;
+    yield* browser.session({ label: "anonymous" }, async ({ page, step }) => {
+      const interceptCallback = async (): Promise<string> => {
+        let callback: string | undefined;
+        // Pause the real provider response before its redirect reaches the app.
+        // Playwright does not route subsequent hops of a redirect chain.
+        await page.route("**/user_management/authorize/submit", async (route) => {
+          const response = await route.fetch({ maxRedirects: 0 });
+          expect(response.status()).toBe(302);
+          callback = response.headers().location;
+          await route.fulfill({
+            status: 200,
+            contentType: "text/plain",
+            body: "Authorization ready for callback validation",
+          });
+        });
+        await page.goto(new URL("/api/auth/login", target.baseUrl).toString());
+        await page.getByPlaceholder("new-user@example.com").fill(email);
+        await page.getByRole("button", { name: /Continue/ }).click();
+        await expect.poll(() => callback).toBeDefined();
+        await page.unroute("**/user_management/authorize/submit");
+        if (!callback) throw new Error("AuthKit did not return a callback");
+        return callback;
+      };
+      await step("Refuse a valid authorization code with no state", async () => {
+        const callback = new URL(await interceptCallback());
+        callback.searchParams.delete("state");
+        const response = await page.request.get(callback.toString(), { maxRedirects: 0 });
+        expect(response.status()).toBe(400);
+        expect(await response.text()).toBe("Invalid login state");
+        expect(
+          (await page.context().cookies()).some((cookie) => cookie.name === "wos-session"),
+        ).toBe(false);
+      });
+      await step("Refuse a state from another login", async () => {
+        const callback = new URL(await interceptCallback());
+        callback.searchParams.set("state", "another-browser-state");
+        const response = await page.request.get(callback.toString(), { maxRedirects: 0 });
+        expect(response.status()).toBe(400);
+        expect(await response.text()).toBe("Invalid login state");
+        expect(
+          (await page.context().cookies()).some((cookie) => cookie.name === "wos-session"),
+        ).toBe(false);
+      });
+      await step("Complete a fresh login, then reject the same callback again", async () => {
+        const callback = await interceptCallback();
+        await page.goto(callback);
+        await page.waitForURL((url) => url.pathname === "/create-org", { timeout: 30_000 });
+        const cookies = await page.context().cookies();
+        expect(cookies.some((cookie) => cookie.name === "wos-session")).toBe(true);
+        expect(cookies.some((cookie) => cookie.name === "wos-login-state")).toBe(false);
+        const me = await page.request.get(new URL("/api/auth/me", target.baseUrl).toString());
+        expect(me.status()).toBe(200);
+        expect(await me.json()).toMatchObject({ user: { email } });
+        const replay = await page.request.get(callback, { maxRedirects: 0 });
+        expect(replay.status()).toBe(400);
+        expect(await replay.text()).toBe("Invalid login state");
+      });
+    });
+  }),
+);

e2e/cloud/org-api-keys-console.test.ts

@@ -110,7 +110,9 @@ scenario(
           .getByRole("heading", { name: "Revoke organization key" })
           .waitFor({ state: "hidden", timeout: 30_000 });
 
-        // The revoked value no longer authenticates.
+        // The dialog closes when revocation starts. Wait for the confirmed
+        // provider mutation before asserting the key no longer authenticates.
+        await page.getByText("Revoked e2e backend reader", { exact: true }).waitFor();
         const after = await fetch(new URL("/api/admin/users", target.baseUrl), {
           headers: { authorization: `Bearer ${mintedValue}` },
         });