fix(core): enforce mcp tool permissions (#35345)
This commit is contained in:
parent
c590e27639
commit
b99759c7de
2 changed files with 132 additions and 3 deletions
|
|
@ -6,6 +6,7 @@ import { Effect, Exit, type JsonSchema, Layer, Scope, Semaphore, Stream } from "
|
|||
import { makeLocationNode } from "../effect/app-node"
|
||||
import { EventV2 } from "../event"
|
||||
import { MCP } from "../mcp"
|
||||
import { PermissionV2 } from "../permission"
|
||||
import { Tool } from "./tool"
|
||||
import { Tools } from "./tools"
|
||||
import { ToolRegistry } from "./registry"
|
||||
|
|
@ -21,6 +22,7 @@ export const layer = Layer.effectDiscard(
|
|||
const mcp = yield* MCP.Service
|
||||
const tools = yield* Tools.Service
|
||||
const events = yield* EventV2.Service
|
||||
const permission = yield* PermissionV2.Service
|
||||
const scope = yield* Scope.Scope
|
||||
const lock = Semaphore.makeUnsafe(1)
|
||||
let current: Scope.Closeable | undefined
|
||||
|
|
@ -41,8 +43,21 @@ export const layer = Layer.effectDiscard(
|
|||
properties: schema.properties ?? {},
|
||||
additionalProperties: false,
|
||||
},
|
||||
execute: (input) =>
|
||||
execute: (input, context) =>
|
||||
Effect.gen(function* () {
|
||||
yield* permission.assert({
|
||||
action: name(tool.server, tool.name),
|
||||
resources: ["*"],
|
||||
save: ["*"],
|
||||
metadata: {},
|
||||
sessionID: context.sessionID,
|
||||
agent: context.agent,
|
||||
source: {
|
||||
type: "tool",
|
||||
messageID: context.assistantMessageID,
|
||||
callID: context.toolCallID,
|
||||
},
|
||||
})
|
||||
const result = yield* mcp
|
||||
.callTool({
|
||||
server: tool.server,
|
||||
|
|
@ -72,7 +87,13 @@ export const layer = Layer.effectDiscard(
|
|||
: { type: "file" as const, data: part.data, mime: part.mimeType },
|
||||
),
|
||||
}
|
||||
}),
|
||||
}).pipe(
|
||||
Effect.mapError((error) =>
|
||||
error instanceof ToolFailure
|
||||
? error
|
||||
: new ToolFailure({ message: `Unable to execute ${name(tool.server, tool.name)}` }),
|
||||
),
|
||||
),
|
||||
})
|
||||
groups.set(tool.server, group)
|
||||
}
|
||||
|
|
@ -96,5 +117,5 @@ export const layer = Layer.effectDiscard(
|
|||
export const node = makeLocationNode({
|
||||
name: "mcp-tools",
|
||||
layer,
|
||||
deps: [ToolRegistry.toolsNode, MCP.node, EventV2.node],
|
||||
deps: [ToolRegistry.toolsNode, MCP.node, EventV2.node, PermissionV2.node],
|
||||
})
|
||||
|
|
|
|||
|
|
@ -1,7 +1,61 @@
|
|||
import { describe, expect, test } from "bun:test"
|
||||
import { AppNodeBuilder } from "@opencode-ai/core/effect/app-node-builder"
|
||||
import { LayerNode } from "@opencode-ai/core/effect/layer-node"
|
||||
import { EventV2 } from "@opencode-ai/core/event"
|
||||
import { MCP } from "@opencode-ai/core/mcp/index"
|
||||
import { MCPClient } from "@opencode-ai/core/mcp/client"
|
||||
import { PermissionV2 } from "@opencode-ai/core/permission"
|
||||
import { SessionV2 } from "@opencode-ai/core/session"
|
||||
import { McpTool } from "@opencode-ai/core/tool/mcp"
|
||||
import { ToolRegistry } from "@opencode-ai/core/tool/registry"
|
||||
import { ToolOutputStore } from "@opencode-ai/core/tool-output-store"
|
||||
import { Deferred, Effect, Fiber, Layer, Stream } from "effect"
|
||||
import { testEffect } from "./lib/effect"
|
||||
import { settleTool, toolIdentity, waitForTool } from "./lib/tool"
|
||||
|
||||
let assertion: Deferred.Deferred<PermissionV2.AssertInput> | undefined
|
||||
let decision: Effect.Effect<void, PermissionV2.Error> = Effect.void
|
||||
let calls = 0
|
||||
|
||||
const mcp = Layer.mock(MCP.Service, {
|
||||
tools: () =>
|
||||
Effect.succeed([
|
||||
new MCP.Tool({
|
||||
server: MCP.ServerName.make("demo"),
|
||||
name: "search",
|
||||
description: "Search",
|
||||
inputSchema: { type: "object", properties: {} },
|
||||
}),
|
||||
]),
|
||||
callTool: (input) =>
|
||||
Effect.sync(() => {
|
||||
calls += 1
|
||||
return new MCP.ToolResult({
|
||||
server: MCP.ServerName.make(input.server),
|
||||
tool: input.name,
|
||||
isError: false,
|
||||
structured: { ok: true },
|
||||
content: [],
|
||||
})
|
||||
}),
|
||||
})
|
||||
const permissions = Layer.mock(PermissionV2.Service, {
|
||||
assert: (input) =>
|
||||
Effect.gen(function* () {
|
||||
if (!assertion) return yield* Effect.die("Permission test is not initialized")
|
||||
yield* Deferred.succeed(assertion, input)
|
||||
yield* decision
|
||||
}),
|
||||
})
|
||||
const events = Layer.mock(EventV2.Service, { subscribe: () => Stream.never })
|
||||
const it = testEffect(
|
||||
AppNodeBuilder.build(LayerNode.group([ToolRegistry.node, ToolRegistry.toolsNode, McpTool.node]), [
|
||||
[MCP.node, mcp],
|
||||
[PermissionV2.node, permissions],
|
||||
[EventV2.node, events],
|
||||
[ToolOutputStore.node, ToolOutputStore.nodeWithoutConfig],
|
||||
]),
|
||||
)
|
||||
|
||||
describe("MCP errors", () => {
|
||||
test("expose useful messages", () => {
|
||||
|
|
@ -17,3 +71,57 @@ describe("MCP errors", () => {
|
|||
test("MCP tool names match V1 sanitization", () => {
|
||||
expect(McpTool.name("context 7", "resolve.library/id")).toBe("context_7_resolve_library_id")
|
||||
})
|
||||
|
||||
it.effect("waits for permission before calling an MCP tool", () =>
|
||||
Effect.gen(function* () {
|
||||
calls = 0
|
||||
assertion = yield* Deferred.make<PermissionV2.AssertInput>()
|
||||
const permission = yield* Deferred.make<void>()
|
||||
decision = Deferred.await(permission)
|
||||
const registry = yield* ToolRegistry.Service
|
||||
yield* waitForTool(registry, "demo_search")
|
||||
|
||||
const fiber = yield* settleTool(registry, {
|
||||
sessionID: SessionV2.ID.make("ses_mcp_permission"),
|
||||
...toolIdentity,
|
||||
call: { type: "tool-call", id: "call_mcp_permission", name: "demo_search", input: {} },
|
||||
}).pipe(Effect.forkScoped)
|
||||
expect(yield* Deferred.await(assertion)).toEqual({
|
||||
action: "demo_search",
|
||||
resources: ["*"],
|
||||
save: ["*"],
|
||||
metadata: {},
|
||||
sessionID: SessionV2.ID.make("ses_mcp_permission"),
|
||||
agent: toolIdentity.agent,
|
||||
source: {
|
||||
type: "tool",
|
||||
messageID: toolIdentity.assistantMessageID,
|
||||
callID: "call_mcp_permission",
|
||||
},
|
||||
})
|
||||
expect(calls).toBe(0)
|
||||
|
||||
yield* Deferred.succeed(permission, undefined)
|
||||
yield* Fiber.join(fiber)
|
||||
expect(calls).toBe(1)
|
||||
}),
|
||||
)
|
||||
|
||||
it.effect("does not call MCP when permission is rejected", () =>
|
||||
Effect.gen(function* () {
|
||||
calls = 0
|
||||
assertion = yield* Deferred.make<PermissionV2.AssertInput>()
|
||||
decision = Effect.fail(new PermissionV2.RejectedError())
|
||||
const registry = yield* ToolRegistry.Service
|
||||
yield* waitForTool(registry, "demo_search")
|
||||
|
||||
expect(
|
||||
yield* settleTool(registry, {
|
||||
sessionID: SessionV2.ID.make("ses_mcp_rejected"),
|
||||
...toolIdentity,
|
||||
call: { type: "tool-call", id: "call_mcp_rejected", name: "demo_search", input: {} },
|
||||
}),
|
||||
).toEqual({ result: { type: "error", value: "Unable to execute demo_search" } })
|
||||
expect(calls).toBe(0)
|
||||
}),
|
||||
)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue