fix(opencode): preserve valid MCP OAuth state

packages/opencode/src/mcp/auth.ts

@@ -49,6 +49,7 @@ export interface Interface {
   readonly clearCodeVerifier: (mcpName: string) => Effect.Effect<void>
   readonly updateOAuthState: (mcpName: string, oauthState: string) => Effect.Effect<void>
   readonly getOAuthState: (mcpName: string) => Effect.Effect<string | undefined>
+  readonly consumeOAuthState: (mcpName: string, oauthState: string) => Effect.Effect<boolean>
   readonly clearOAuthState: (mcpName: string) => Effect.Effect<void>
   readonly isTokenExpired: (mcpName: string) => Effect.Effect<boolean | null>
 }
@@ -142,6 +143,17 @@ export const layer = Layer.effect(
       return entry?.oauthState
     })
 
+    const consumeOAuthState = Effect.fn("McpAuth.consumeOAuthState")(function* (mcpName: string, oauthState: string) {
+      return yield* Effect.gen(function* () {
+        const data = yield* read()
+        const entry = data[mcpName]
+        if (entry?.oauthState !== oauthState) return false
+        delete entry.oauthState
+        yield* fs.writeJson(filepath, { ...data, [mcpName]: entry }, 0o600).pipe(Effect.orDie)
+        return true
+      }).pipe(flock.withLock(lockKey), Effect.orDie)
+    })
+
     const isTokenExpired = Effect.fn("McpAuth.isTokenExpired")(function* (mcpName: string) {
       const entry = yield* get(mcpName)
       if (!entry?.tokens) return null
@@ -161,6 +173,7 @@ export const layer = Layer.effect(
       clearCodeVerifier,
       updateOAuthState,
       getOAuthState,
+      consumeOAuthState,
       clearOAuthState,
       isTokenExpired,
     })

packages/opencode/src/mcp/index.ts

@@ -71,6 +71,10 @@ export class NotFoundError extends Schema.TaggedErrorClass<NotFoundError>()("MCP
   name: Schema.String,
 }) {}
 
+export class OAuthError extends Schema.TaggedErrorClass<OAuthError>()("MCP.OAuthError", {
+  message: Schema.String,
+}) {}
+
 type MCPClient = Client
 
 function createClient(directory: string) {
@@ -180,7 +184,11 @@ export interface Interface {
     mcpName: string,
   ) => Effect.Effect<{ authorizationUrl: string; oauthState: string }, NotFoundError>
   readonly authenticate: (mcpName: string) => Effect.Effect<Status, NotFoundError>
-  readonly finishAuth: (mcpName: string, authorizationCode: string) => Effect.Effect<Status, NotFoundError>
+  readonly finishAuth: (
+    mcpName: string,
+    authorizationCode: string,
+    oauthState: string,
+  ) => Effect.Effect<Status, NotFoundError | OAuthError>
   readonly removeAuth: (mcpName: string) => Effect.Effect<void>
   readonly supportsOAuth: (mcpName: string) => Effect.Effect<boolean, NotFoundError>
   readonly hasStoredTokens: (mcpName: string) => Effect.Effect<boolean>
@@ -793,6 +801,16 @@ export const layer = Layer.effect(
       return mcpConfig
     })
 
+    const cleanupAuth = Effect.fnUntraced(function* (mcpName: string) {
+      const transport = pendingOAuthTransports.get(mcpName)
+      pendingOAuthTransports.delete(mcpName)
+      yield* Effect.all([
+        auth.clearOAuthState(mcpName),
+        auth.clearCodeVerifier(mcpName),
+        Effect.tryPromise(() => transport?.close() ?? Promise.resolve()).pipe(Effect.ignore),
+      ])
+    })
+
     const startAuth = Effect.fn("MCP.startAuth")(function* (mcpName: string) {
       const mcpConfig = yield* requireMcpConfig(mcpName)
       if (mcpConfig.type !== "remote") throw new Error(`MCP server ${mcpName} is not a remote server`)
@@ -808,8 +826,7 @@ export const layer = Layer.effect(
         oauthConfig?.redirectUri ??
         (oauthConfig?.callbackPort ? `http://127.0.0.1:${oauthConfig.callbackPort}${OAUTH_CALLBACK_PATH}` : undefined)
 
-      // Start the callback server with custom redirectUri if configured
-      yield* Effect.promise(() => McpOAuthCallback.ensureRunning(effectiveRedirectUri))
+      yield* cleanupAuth(mcpName)
 
       const oauthState = Array.from(crypto.getRandomValues(new Uint8Array(32)))
         .map((b) => b.toString(16).padStart(2, "0"))
@@ -853,7 +870,7 @@ export const layer = Layer.effect(
             pendingOAuthTransports.set(mcpName, transport)
             return Effect.succeed({ authorizationUrl: capturedUrl.toString(), oauthState } satisfies AuthResult)
           }
-          return Effect.die(error)
+          return cleanupAuth(mcpName).pipe(Effect.andThen(Effect.die(error)))
         }),
       )
     })
@@ -881,44 +898,68 @@ export const layer = Layer.effect(
         return yield* storeClient(s, mcpName, client, listed, client.getInstructions()?.trim(), mcpConfig.timeout)
       }
 
-      const callbackPromise = McpOAuthCallback.waitForCallback(result.oauthState, mcpName)
+      return yield* Effect.gen(function* () {
+        const mcpConfig = yield* requireMcpConfig(mcpName)
+        if (mcpConfig.type !== "remote") throw new Error(`MCP server ${mcpName} is not a remote server`)
+        const oauthConfig = typeof mcpConfig.oauth === "object" ? mcpConfig.oauth : undefined
+        yield* Effect.promise(() =>
+          McpOAuthCallback.ensureRunning(
+            oauthConfig?.redirectUri ??
+              (oauthConfig?.callbackPort
+                ? `http://127.0.0.1:${oauthConfig.callbackPort}${OAUTH_CALLBACK_PATH}`
+                : undefined),
+          ),
+        )
 
-      yield* Effect.tryPromise(() => open(result.authorizationUrl)).pipe(
-        Effect.flatMap((subprocess) =>
-          Effect.callback<void, Error>((resume) => {
-            const timer = setTimeout(() => resume(Effect.void), 500)
-            subprocess.on("error", (err) => {
-              clearTimeout(timer)
-              resume(Effect.fail(err))
-            })
-            subprocess.on("exit", (code) => {
-              if (code !== null && code !== 0) {
+        const callbackPromise = McpOAuthCallback.waitForCallback(result.oauthState, mcpName)
+
+        yield* Effect.tryPromise(() => open(result.authorizationUrl)).pipe(
+          Effect.flatMap((subprocess) =>
+            Effect.callback<void, Error>((resume) => {
+              const timer = setTimeout(() => resume(Effect.void), 500)
+              subprocess.on("error", (err) => {
                 clearTimeout(timer)
-                resume(Effect.fail(new Error(`Browser open failed with exit code ${code}`)))
-              }
-            })
+                resume(Effect.fail(err))
+              })
+              subprocess.on("exit", (code) => {
+                if (code !== null && code !== 0) {
+                  clearTimeout(timer)
+                  resume(Effect.fail(new Error(`Browser open failed with exit code ${code}`)))
+                }
+              })
+            }),
+          ),
+          Effect.catch(() => {
+            return events.publish(BrowserOpenFailed, { mcpName, url: result.authorizationUrl }).pipe(Effect.ignore)
           }),
+        )
+
+        const code = yield* Effect.promise(() => callbackPromise)
+        return yield* finishAuth(mcpName, code, result.oauthState).pipe(
+          Effect.catchTag("MCP.OAuthError", (error) => Effect.die(error)),
+        )
+      }).pipe(
+        Effect.ensuring(
+          Effect.sync(() => McpOAuthCallback.cancelPending(mcpName)).pipe(Effect.andThen(cleanupAuth(mcpName))),
         ),
-        Effect.catch(() => {
-          return events.publish(BrowserOpenFailed, { mcpName, url: result.authorizationUrl }).pipe(Effect.ignore)
-        }),
       )
-
-      const code = yield* Effect.promise(() => callbackPromise)
-
-      const storedState = yield* auth.getOAuthState(mcpName)
-      if (storedState !== result.oauthState) {
-        yield* auth.clearOAuthState(mcpName)
-        throw new Error("OAuth state mismatch - potential CSRF attack")
-      }
-      yield* auth.clearOAuthState(mcpName)
-      return yield* finishAuth(mcpName, code)
     })
 
-    const finishAuth = Effect.fn("MCP.finishAuth")(function* (mcpName: string, authorizationCode: string) {
+    const finishAuth = Effect.fn("MCP.finishAuth")(function* (
+      mcpName: string,
+      authorizationCode: string,
+      oauthState: string,
+    ) {
       yield* requireMcpConfig(mcpName)
+      if (!(yield* auth.consumeOAuthState(mcpName, oauthState))) {
+        return yield* new OAuthError({ message: "Invalid or expired OAuth state - potential CSRF attack" })
+      }
+
       const transport = pendingOAuthTransports.get(mcpName)
-      if (!transport) throw new Error(`No pending OAuth flow for MCP server: ${mcpName}`)
+      if (!transport) {
+        yield* cleanupAuth(mcpName)
+        return yield* new OAuthError({ message: `No pending OAuth flow for MCP server: ${mcpName}` })
+      }
 
       const result = yield* Effect.tryPromise({
         try: () => transport.finishAuth(authorizationCode).then(() => true as const),
@@ -927,13 +968,11 @@ export const layer = Layer.effect(
         },
       }).pipe(Effect.option)
 
+      yield* cleanupAuth(mcpName)
       if (Option.isNone(result)) {
         return { status: "failed", error: "OAuth completion failed" } satisfies Status
       }
 
-      yield* auth.clearCodeVerifier(mcpName)
-      pendingOAuthTransports.delete(mcpName)
-
       const mcpConfig = yield* requireMcpConfig(mcpName)
 
       return yield* createAndStore(mcpName, mcpConfig)

packages/opencode/src/server/routes/instance/httpapi/groups/mcp.ts

@@ -20,6 +20,7 @@ export const AuthStartResponse = Schema.Struct({
 })
 export const AuthCallbackPayload = Schema.Struct({
   code: Schema.String,
+  state: Schema.String,
 })
 export const AuthRemoveResponse = Schema.Struct({
   success: Schema.Literal(true),
@@ -87,7 +88,7 @@ export const McpApi = HttpApi.make("mcp")
             identifier: "mcp.auth.callback",
             summary: "Complete MCP OAuth",
             description:
-              "Complete OAuth authentication for a Model Context Protocol (MCP) server using the authorization code.",
+              "Complete OAuth authentication for a Model Context Protocol (MCP) server using the authorization code and state returned by the start endpoint.",
           }),
         ),
         HttpApiEndpoint.post("authAuthenticate", McpPaths.authAuthenticate, {

packages/opencode/src/server/routes/instance/httpapi/handlers/mcp.ts

@@ -37,15 +37,15 @@ export const mcpHandlers = HttpApiBuilder.group(InstanceHttpApi, "mcp", (handler
       params: { name: string }
       payload: typeof AuthCallbackPayload.Type
     }) {
-      return yield* mcp
-        .finishAuth(ctx.params.name, ctx.payload.code)
-        .pipe(
-          Effect.catchTag("MCP.NotFoundError", (error) =>
+      return yield* mcp.finishAuth(ctx.params.name, ctx.payload.code, ctx.payload.state).pipe(
+        Effect.catchTags({
+          "MCP.NotFoundError": (error) =>
             Effect.fail(
               new McpServerNotFoundError({ name: error.name, message: `MCP server not found: ${error.name}` }),
             ),
-          ),
-        )
+          "MCP.OAuthError": () => Effect.fail(new HttpApiError.BadRequest({})),
+        }),
+      )
     })
 
     const authAuthenticate = Effect.fn("McpHttpApi.authAuthenticate")(function* (ctx: { params: { name: string } }) {

packages/opencode/test/server/httpapi-mcp-oauth.test.ts

@@ -1,73 +1,197 @@
-import { NodeHttpServer } from "@effect/platform-node"
-import { Session } from "@/session/session"
+import { NodeHttpServer, NodeServices } from "@effect/platform-node"
+import Http from "node:http"
+import path from "node:path"
 import { describe, expect } from "bun:test"
-import { Effect, Layer } from "effect"
-import { HttpClient, HttpClientRequest, HttpRouter } from "effect/unstable/http"
-import { HttpApi, HttpApiBuilder } from "effect/unstable/httpapi"
-import { McpApi, McpPaths } from "../../src/server/routes/instance/httpapi/groups/mcp"
-import { Authorization } from "../../src/server/routes/instance/httpapi/middleware/authorization"
-import { InstanceContextMiddleware } from "../../src/server/routes/instance/httpapi/middleware/instance-context"
+import { Config, Context, Effect, Layer, Ref } from "effect"
 import {
-  WorkspaceRouteContext,
-  WorkspaceRoutingMiddleware,
-} from "../../src/server/routes/instance/httpapi/middleware/workspace-routing"
+  HttpClient,
+  HttpClientRequest,
+  HttpRouter,
+  HttpServer,
+  HttpServerRequest,
+  HttpServerResponse,
+} from "effect/unstable/http"
+import { McpOAuthCallback } from "../../src/mcp/oauth-callback"
+import { McpPaths } from "../../src/server/routes/instance/httpapi/groups/mcp"
+import { HttpApiApp } from "../../src/server/routes/instance/httpapi/server"
+import { tmpdirScoped } from "../fixture/fixture"
 import { testEffect } from "../lib/effect"
 
-const TestHttpApi = HttpApi.make("opencode-instance").addHttpApi(McpApi)
-const fakeSession = Layer.mock(Session.Service)({})
-const testMcpHandlers = HttpApiBuilder.group(TestHttpApi, "mcp", (handlers) =>
-  Effect.succeed(
-    handlers
-      .handle("status", () => Effect.die("unexpected MCP status"))
-      .handle("add", () => Effect.die("unexpected MCP add"))
-      .handle("authStart", () =>
-        Effect.succeed({ authorizationUrl: "https://auth.example/start", oauthState: "state-123" }),
-      )
-      .handle("authCallback", () => Effect.die("unexpected MCP authCallback"))
-      .handle("authAuthenticate", () => Effect.die("unexpected MCP authAuthenticate"))
-      .handle("authRemove", () => Effect.die("unexpected MCP authRemove"))
-      .handle("connect", () => Effect.die("unexpected MCP connect"))
-      .handle("disconnect", () => Effect.die("unexpected MCP disconnect")),
-  ),
-)
-
-const passthroughAuthorization = Layer.succeed(
-  Authorization,
-  Authorization.of((effect) => effect),
-)
-
-const passthroughInstanceContext = Layer.succeed(
-  InstanceContextMiddleware,
-  InstanceContextMiddleware.of((effect) => effect),
-)
-
-const testWorkspaceRouting = Layer.succeed(
-  WorkspaceRoutingMiddleware,
-  WorkspaceRoutingMiddleware.of((effect) =>
-    effect.pipe(Effect.provideService(WorkspaceRouteContext, WorkspaceRouteContext.of({ directory: process.cwd() }))),
-  ),
+const servedRoutes: Layer.Layer<never, Config.ConfigError, HttpServer.HttpServer> = HttpRouter.serve(
+  HttpApiApp.routes,
+  {
+    disableListenLog: true,
+    disableLogger: true,
+  },
 )
 
 const it = testEffect(
-  HttpRouter.serve(
-    HttpApiBuilder.layer(TestHttpApi).pipe(
-      Layer.provide(testMcpHandlers),
-      Layer.provide([passthroughAuthorization, passthroughInstanceContext, testWorkspaceRouting, fakeSession]),
-    ),
-    { disableListenLog: true, disableLogger: true },
-  ).pipe(Layer.provideMerge(NodeHttpServer.layerTest)),
+  servedRoutes.pipe(Layer.provideMerge(NodeHttpServer.layerTest), Layer.provideMerge(NodeServices.layer)),
 )
 
-describe("mcp HttpApi OAuth", () => {
-  it.live("preserves oauth state when starting OAuth", () =>
-    Effect.gen(function* () {
-      const response = yield* HttpClientRequest.post(McpPaths.auth.replace(":name", "demo")).pipe(HttpClient.execute)
+const callbackPath = McpPaths.authCallback.replace(":name", "secure-oauth")
+const authPath = McpPaths.auth.replace(":name", "secure-oauth")
 
-      expect(response.status).toBe(200)
-      expect(yield* response.json).toEqual({
-        authorizationUrl: "https://auth.example/start",
-        oauthState: "state-123",
-      })
+function listenOAuthServer(tokenCalls: Ref.Ref<number>) {
+  return Effect.gen(function* () {
+    const context = yield* Layer.build(NodeHttpServer.layer(Http.createServer, { host: "127.0.0.1", port: 0 }))
+    const server = Context.get(context, HttpServer.HttpServer)
+    const origin = HttpServer.formatAddress(server.address)
+    yield* server.serve(
+      HttpServerRequest.HttpServerRequest.use((request) => {
+        const url = new URL(request.url, origin)
+        if (url.pathname === "/.well-known/oauth-protected-resource/mcp")
+          return HttpServerResponse.json({
+            resource: `${origin}/mcp`,
+            authorization_servers: [origin],
+          })
+        if (url.pathname === "/.well-known/oauth-authorization-server")
+          return HttpServerResponse.json({
+            issuer: origin,
+            authorization_endpoint: `${origin}/authorize`,
+            token_endpoint: `${origin}/token`,
+            response_types_supported: ["code"],
+            grant_types_supported: ["authorization_code", "refresh_token"],
+            code_challenge_methods_supported: ["S256"],
+          })
+        if (url.pathname === "/token")
+          return Effect.gen(function* () {
+            yield* request.text
+            yield* Ref.update(tokenCalls, (value) => value + 1)
+            return yield* HttpServerResponse.json({ access_token: "access-token", token_type: "Bearer" })
+          })
+        if (url.pathname !== "/mcp") return Effect.succeed(HttpServerResponse.empty({ status: 404 }))
+        if (request.headers.authorization !== "Bearer access-token")
+          return Effect.succeed(
+            HttpServerResponse.empty({
+              status: 401,
+              headers: {
+                "www-authenticate": `Bearer resource_metadata="${origin}/.well-known/oauth-protected-resource/mcp"`,
+              },
+            }),
+          )
+        return Effect.gen(function* () {
+          const body = yield* request.json
+          if (
+            typeof body === "object" &&
+            body !== null &&
+            "method" in body &&
+            body.method === "notifications/initialized"
+          )
+            return HttpServerResponse.empty({ status: 202 })
+          if (typeof body === "object" && body !== null && "method" in body && body.method === "tools/list")
+            return yield* HttpServerResponse.json({
+              jsonrpc: "2.0",
+              id: "id" in body ? body.id : null,
+              result: { tools: [] },
+            })
+          return yield* HttpServerResponse.json({
+            jsonrpc: "2.0",
+            id: typeof body === "object" && body !== null && "id" in body ? body.id : null,
+            result: {
+              protocolVersion: "2025-03-26",
+              capabilities: { tools: {} },
+              serverInfo: { name: "oauth-test", version: "1.0.0" },
+            },
+          })
+        })
+      }),
+    )
+    return origin
+  })
+}
+
+function availablePort() {
+  return Effect.promise(
+    () =>
+      new Promise<number>((resolve, reject) => {
+        const server = Http.createServer()
+        server.on("error", reject)
+        server.listen(0, "127.0.0.1", () => {
+          const address = server.address()
+          if (!address || typeof address === "string") return reject(new Error("Failed to allocate callback port"))
+          server.close((error) => (error ? reject(error) : resolve(address.port)))
+        })
+      }),
+  )
+}
+
+function assertPortAvailable(port: number) {
+  return Effect.promise(
+    () =>
+      new Promise<void>((resolve, reject) => {
+        const server = Http.createServer()
+        server.on("error", reject)
+        server.listen(port, "127.0.0.1", () => server.close((error) => (error ? reject(error) : resolve())))
+      }),
+  )
+}
+
+function setup() {
+  return Effect.gen(function* () {
+    const directory = yield* tmpdirScoped({ git: true })
+    const tokenCalls = yield* Ref.make(0)
+    const upstream = yield* listenOAuthServer(tokenCalls)
+    const callbackPort = yield* availablePort()
+    yield* Effect.promise(() =>
+      Bun.write(
+        path.join(directory, "opencode.json"),
+        JSON.stringify({
+          formatter: false,
+          lsp: false,
+          mcp: {
+            "secure-oauth": {
+              type: "remote",
+              url: `${upstream}/mcp`,
+              oauth: { clientId: "test-client", callbackPort },
+            },
+          },
+        }),
+      ),
+    )
+    return { directory, tokenCalls, callbackPort }
+  })
+}
+
+function request(directory: string, route: string, payload?: object) {
+  const base = HttpClientRequest.post(route).pipe(HttpClientRequest.setHeader("x-opencode-directory", directory))
+  if (!payload) return HttpClient.execute(base)
+  return base.pipe(HttpClientRequest.bodyJson(payload), Effect.flatMap(HttpClient.execute))
+}
+
+describe("mcp HttpApi OAuth", () => {
+  it.live("requires, validates, consumes, and rejects replayed callback state", () =>
+    Effect.gen(function* () {
+      const test = yield* setup()
+      const started = yield* request(test.directory, authPath)
+      expect(started.status).toBe(200)
+      const first = (yield* started.json) as { oauthState: string }
+
+      const missing = yield* request(test.directory, callbackPath, { code: "missing-state" })
+      expect(missing.status).toBe(400)
+      expect(yield* Ref.get(test.tokenCalls)).toBe(0)
+
+      const wrong = yield* request(test.directory, callbackPath, { code: "wrong-state", state: "wrong" })
+      expect(wrong.status).toBe(400)
+      expect(yield* Ref.get(test.tokenCalls)).toBe(0)
+
+      const correct = yield* request(test.directory, callbackPath, { code: "valid-code", state: first.oauthState })
+      expect(correct.status).toBe(200)
+      expect(yield* Ref.get(test.tokenCalls)).toBe(1)
+
+      const replayed = yield* request(test.directory, callbackPath, { code: "replayed-code", state: first.oauthState })
+      expect(replayed.status).toBe(400)
+      expect(yield* Ref.get(test.tokenCalls)).toBe(1)
+    }),
+  )
+
+  it.live("does not bind the browser callback listener during manual start", () =>
+    Effect.gen(function* () {
+      const test = yield* setup()
+      const started = yield* request(test.directory, authPath)
+      expect(started.status).toBe(200)
+      expect(McpOAuthCallback.isRunning()).toBe(false)
+      yield* assertPortAvailable(test.callbackPort)
     }),
   )
 })

packages/opencode/test/server/httpapi-mcp.test.ts

@@ -199,7 +199,11 @@ describe("mcp HttpApi", () => {
         for (const input of [
           { method: "POST", route: "/mcp/missing/auth" },
           { method: "POST", route: "/mcp/missing/auth/authenticate" },
-          { method: "POST", route: "/mcp/missing/auth/callback", body: JSON.stringify({ code: "code" }) },
+          {
+            method: "POST",
+            route: "/mcp/missing/auth/callback",
+            body: JSON.stringify({ code: "code", state: "state" }),
+          },
           { method: "DELETE", route: "/mcp/missing/auth" },
           { method: "POST", route: "/mcp/missing/connect" },
           { method: "POST", route: "/mcp/missing/disconnect" },

packages/sdk/js/src/v2/gen/sdk.gen.ts

@@ -2296,7 +2296,7 @@ export class Auth2 extends HeyApiClient {
   /**
    * Complete MCP OAuth
    *
-   * Complete OAuth authentication for a Model Context Protocol (MCP) server using the authorization code.
+   * Complete OAuth authentication for a Model Context Protocol (MCP) server using the authorization code and state returned by the start endpoint.
    */
   public callback<ThrowOnError extends boolean = false>(
     parameters: {
@@ -2304,6 +2304,7 @@ export class Auth2 extends HeyApiClient {
       directory?: string
       workspace?: string
       code?: string
+      state?: string
     },
     options?: Options<never, ThrowOnError>,
   ) {
@@ -2316,6 +2317,7 @@ export class Auth2 extends HeyApiClient {
             { in: "query", key: "directory" },
             { in: "query", key: "workspace" },
             { in: "body", key: "code" },
+            { in: "body", key: "state" },
           ],
         },
       ],

packages/sdk/js/src/v2/gen/types.gen.ts

@@ -8407,6 +8407,7 @@ export type McpAuthStartResponse = McpAuthStartResponses[keyof McpAuthStartRespo
 export type McpAuthCallbackData = {
   body?: {
     code: string
+    state: string
   }
   path: {
     name: string