fix(opencode): secure manual MCP OAuth callback
This commit is contained in:
@@ -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,18 @@ 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) return false
|
||||
const matches = entry.oauthState === oauthState
|
||||
delete entry.oauthState
|
||||
yield* fs.writeJson(filepath, { ...data, [mcpName]: entry }, 0o600).pipe(Effect.orDie)
|
||||
return matches
|
||||
}).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 +174,7 @@ export const layer = Layer.effect(
|
||||
clearCodeVerifier,
|
||||
updateOAuthState,
|
||||
getOAuthState,
|
||||
consumeOAuthState,
|
||||
clearOAuthState,
|
||||
isTokenExpired,
|
||||
})
|
||||
|
||||
@@ -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,66 @@ 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)
|
||||
const transport = pendingOAuthTransports.get(mcpName)
|
||||
if (!transport) throw new Error(`No pending OAuth flow for MCP server: ${mcpName}`)
|
||||
if (!transport) return yield* new OAuthError({ message: `No pending OAuth flow for MCP server: ${mcpName}` })
|
||||
|
||||
if (!(yield* auth.consumeOAuthState(mcpName, oauthState))) {
|
||||
yield* cleanupAuth(mcpName)
|
||||
return yield* new OAuthError({ message: "Invalid or expired OAuth state - potential CSRF attack" })
|
||||
}
|
||||
|
||||
const result = yield* Effect.tryPromise({
|
||||
try: () => transport.finishAuth(authorizationCode).then(() => true as const),
|
||||
@@ -927,13 +966,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)
|
||||
|
||||
@@ -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),
|
||||
|
||||
@@ -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 } }) {
|
||||
|
||||
Reference in New Issue
Block a user