Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 85 additions & 0 deletions src/core/auto-approval/__tests__/mcp.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
import { isMcpToolAlwaysAllowed } from "../mcp"
import type { McpServerUse, McpServer, McpTool } from "@roo-code/types"

function makeServerUse(serverName: string, toolName: string): McpServerUse {
return { type: "use_mcp_tool", serverName, toolName }
}

function makeTool(name: string, alwaysAllow: boolean): McpTool {
return { name, description: "test", inputSchema: {}, alwaysAllow } as McpTool
}

function makeServer(name: string, tools: McpTool[], config: string): McpServer {
return { name, config, tools } as McpServer
}

describe("isMcpToolAlwaysAllowed", () => {
describe("primary check: tool-level alwaysAllow flag", () => {
it("returns true when tool has alwaysAllow set", () => {
const server = makeServer("my-server", [makeTool("my-tool", true)], "{}")
expect(isMcpToolAlwaysAllowed(makeServerUse("my-server", "my-tool"), [server])).toBe(true)
})

it("returns false when tool has alwaysAllow unset", () => {
const server = makeServer("my-server", [makeTool("my-tool", false)], "{}")
expect(isMcpToolAlwaysAllowed(makeServerUse("my-server", "my-tool"), [server])).toBe(false)
})

it("returns false when server not found", () => {
expect(isMcpToolAlwaysAllowed(makeServerUse("missing-server", "my-tool"), [])).toBe(false)
})

it("returns false when mcpServers is undefined", () => {
expect(isMcpToolAlwaysAllowed(makeServerUse("my-server", "my-tool"), undefined)).toBe(false)
})

it("returns false for non use_mcp_tool type", () => {
const serverUse = { type: "access_mcp_resource", serverName: "s", toolName: "t" } as unknown as McpServerUse
const server = makeServer("s", [makeTool("t", true)], "{}")
expect(isMcpToolAlwaysAllowed(serverUse, [server])).toBe(false)
})
})

describe("fallback: server config alwaysAllow", () => {
it("returns true when tool missing alwaysAllow but server config has wildcard", () => {
const config = JSON.stringify({ alwaysAllow: ["*"] })
const server = makeServer("my-server", [makeTool("my-tool", false)], config)
expect(isMcpToolAlwaysAllowed(makeServerUse("my-server", "my-tool"), [server])).toBe(true)
})

it("returns true when tool missing alwaysAllow but server config has tool name", () => {
const config = JSON.stringify({ alwaysAllow: ["my-tool"] })
const server = makeServer("my-server", [makeTool("my-tool", false)], config)
expect(isMcpToolAlwaysAllowed(makeServerUse("my-server", "my-tool"), [server])).toBe(true)
})

it("returns false when tool missing alwaysAllow and config has different tools", () => {
const config = JSON.stringify({ alwaysAllow: ["other-tool"] })
const server = makeServer("my-server", [makeTool("my-tool", false)], config)
expect(isMcpToolAlwaysAllowed(makeServerUse("my-server", "my-tool"), [server])).toBe(false)
})

it("returns false when config has empty alwaysAllow", () => {
const config = JSON.stringify({ alwaysAllow: [] })
const server = makeServer("my-server", [makeTool("my-tool", false)], config)
expect(isMcpToolAlwaysAllowed(makeServerUse("my-server", "my-tool"), [server])).toBe(false)
})

it("returns false when config has no alwaysAllow field", () => {
const config = JSON.stringify({ command: "node", args: ["server.js"] })
const server = makeServer("my-server", [makeTool("my-tool", false)], config)
expect(isMcpToolAlwaysAllowed(makeServerUse("my-server", "my-tool"), [server])).toBe(false)
})

it("returns false when config is invalid JSON", () => {
const server = makeServer("my-server", [makeTool("my-tool", false)], "not-json")
expect(isMcpToolAlwaysAllowed(makeServerUse("my-server", "my-tool"), [server])).toBe(false)
})

it("returns true with wildcard when tool is not in tools list at all", () => {
const config = JSON.stringify({ alwaysAllow: ["*"] })
const server = makeServer("my-server", [], config)
expect(isMcpToolAlwaysAllowed(makeServerUse("my-server", "unlisted-tool"), [server])).toBe(true)
})
})
})
20 changes: 18 additions & 2 deletions src/core/auto-approval/mcp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,24 @@ import type { McpServerUse, McpServer, McpTool } from "@roo-code/types"
export function isMcpToolAlwaysAllowed(mcpServerUse: McpServerUse, mcpServers: McpServer[] | undefined): boolean {
if (mcpServerUse.type === "use_mcp_tool" && mcpServerUse.toolName) {
const server = mcpServers?.find((s: McpServer) => s.name === mcpServerUse.serverName)
const tool = server?.tools?.find((t: McpTool) => t.name === mcpServerUse.toolName)
return tool?.alwaysAllow || false
if (!server) return false

// Primary check: tool-level flag set by fetchToolsList()
const tool = server.tools?.find((t: McpTool) => t.name === mcpServerUse.toolName)
if (tool?.alwaysAllow) return true

// Fallback: check the server's stored config directly.
// server.config is the JSON-stringified validated config set at connection
// creation time. It includes the alwaysAllow array from mcp_settings.json.
// This handles race conditions where fetchToolsList() hasn't completed or
// its config file read failed silently.
try {
const config = JSON.parse(server.config)
const alwaysAllowConfig: string[] = config.alwaysAllow || []
return alwaysAllowConfig.includes("*") || alwaysAllowConfig.includes(mcpServerUse.toolName)
} catch {
return false
}
}

return false
Expand Down
Loading