mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-05 08:10:14 +00:00
fix: restore MCP auto-approval behavior to approve all tools when enabled
- Changed MCP auto-approval logic from AND to OR condition - When alwaysAllowMcp is enabled, all MCP tools are auto-approved - Individual tool alwaysAllow flag can still approve specific tools - Added comprehensive tests for MCP auto-approval scenarios Fixes #9190
This commit is contained in:
Roo Code
parent
69d4efc335
commit
00102b686c
2 changed files with 218 additions and 1 deletions
214
src/core/auto-approval/__tests__/index.spec.ts
Normal file
214
src/core/auto-approval/__tests__/index.spec.ts
Normal file
|
|
@ -0,0 +1,214 @@
|
|||
import { describe, it, expect, vi } from "vitest"
|
||||
import { checkAutoApproval } from "../index"
|
||||
import type { ExtensionState } from "../../../shared/ExtensionMessage"
|
||||
import type { McpServerUse } from "@roo-code/types"
|
||||
|
||||
describe("checkAutoApproval", () => {
|
||||
describe("MCP auto-approval", () => {
|
||||
it("should approve MCP tool when alwaysAllowMcp is enabled", async () => {
|
||||
const mcpServerUse: McpServerUse = {
|
||||
type: "use_mcp_tool",
|
||||
serverName: "test-server",
|
||||
toolName: "test-tool",
|
||||
}
|
||||
|
||||
const state: Partial<ExtensionState> = {
|
||||
autoApprovalEnabled: true,
|
||||
alwaysAllowMcp: true,
|
||||
mcpServers: [
|
||||
{
|
||||
name: "test-server",
|
||||
config: "test-config",
|
||||
status: "connected",
|
||||
tools: [
|
||||
{
|
||||
name: "test-tool",
|
||||
alwaysAllow: false, // Tool does NOT have alwaysAllow flag
|
||||
},
|
||||
],
|
||||
},
|
||||
] as any,
|
||||
}
|
||||
|
||||
const result = await checkAutoApproval({
|
||||
state: state as ExtensionState,
|
||||
ask: "use_mcp_server",
|
||||
text: JSON.stringify(mcpServerUse),
|
||||
})
|
||||
|
||||
// Should approve because alwaysAllowMcp is true, regardless of tool's alwaysAllow flag
|
||||
expect(result).toEqual({ decision: "approve" })
|
||||
})
|
||||
|
||||
it("should approve MCP tool when tool has alwaysAllow flag even if alwaysAllowMcp is false", async () => {
|
||||
const mcpServerUse: McpServerUse = {
|
||||
type: "use_mcp_tool",
|
||||
serverName: "test-server",
|
||||
toolName: "test-tool",
|
||||
}
|
||||
|
||||
const state: Partial<ExtensionState> = {
|
||||
autoApprovalEnabled: true,
|
||||
alwaysAllowMcp: false, // Global MCP auto-approval is disabled
|
||||
mcpServers: [
|
||||
{
|
||||
name: "test-server",
|
||||
config: "test-config",
|
||||
status: "connected",
|
||||
tools: [
|
||||
{
|
||||
name: "test-tool",
|
||||
alwaysAllow: true, // Tool has individual alwaysAllow flag
|
||||
},
|
||||
],
|
||||
},
|
||||
] as any,
|
||||
}
|
||||
|
||||
const result = await checkAutoApproval({
|
||||
state: state as ExtensionState,
|
||||
ask: "use_mcp_server",
|
||||
text: JSON.stringify(mcpServerUse),
|
||||
})
|
||||
|
||||
// Should approve because tool has alwaysAllow flag
|
||||
expect(result).toEqual({ decision: "approve" })
|
||||
})
|
||||
|
||||
it("should ask for approval when neither alwaysAllowMcp nor tool alwaysAllow is set", async () => {
|
||||
const mcpServerUse: McpServerUse = {
|
||||
type: "use_mcp_tool",
|
||||
serverName: "test-server",
|
||||
toolName: "test-tool",
|
||||
}
|
||||
|
||||
const state: Partial<ExtensionState> = {
|
||||
autoApprovalEnabled: true,
|
||||
alwaysAllowMcp: false,
|
||||
mcpServers: [
|
||||
{
|
||||
name: "test-server",
|
||||
config: "test-config",
|
||||
status: "connected",
|
||||
tools: [
|
||||
{
|
||||
name: "test-tool",
|
||||
alwaysAllow: false,
|
||||
},
|
||||
],
|
||||
},
|
||||
] as any,
|
||||
}
|
||||
|
||||
const result = await checkAutoApproval({
|
||||
state: state as ExtensionState,
|
||||
ask: "use_mcp_server",
|
||||
text: JSON.stringify(mcpServerUse),
|
||||
})
|
||||
|
||||
// Should ask because neither condition is met
|
||||
expect(result).toEqual({ decision: "ask" })
|
||||
})
|
||||
|
||||
it("should approve MCP resource access when alwaysAllowMcp is enabled", async () => {
|
||||
const mcpServerUse: McpServerUse = {
|
||||
type: "access_mcp_resource",
|
||||
serverName: "test-server",
|
||||
uri: "test://resource",
|
||||
}
|
||||
|
||||
const state: Partial<ExtensionState> = {
|
||||
autoApprovalEnabled: true,
|
||||
alwaysAllowMcp: true,
|
||||
}
|
||||
|
||||
const result = await checkAutoApproval({
|
||||
state: state as ExtensionState,
|
||||
ask: "use_mcp_server",
|
||||
text: JSON.stringify(mcpServerUse),
|
||||
})
|
||||
|
||||
// Should approve resource access when alwaysAllowMcp is true
|
||||
expect(result).toEqual({ decision: "approve" })
|
||||
})
|
||||
|
||||
it("should ask for MCP resource access when alwaysAllowMcp is disabled", async () => {
|
||||
const mcpServerUse: McpServerUse = {
|
||||
type: "access_mcp_resource",
|
||||
serverName: "test-server",
|
||||
uri: "test://resource",
|
||||
}
|
||||
|
||||
const state: Partial<ExtensionState> = {
|
||||
autoApprovalEnabled: true,
|
||||
alwaysAllowMcp: false,
|
||||
}
|
||||
|
||||
const result = await checkAutoApproval({
|
||||
state: state as ExtensionState,
|
||||
ask: "use_mcp_server",
|
||||
text: JSON.stringify(mcpServerUse),
|
||||
})
|
||||
|
||||
// Should ask for resource access when alwaysAllowMcp is false
|
||||
expect(result).toEqual({ decision: "ask" })
|
||||
})
|
||||
|
||||
it("should handle missing text gracefully", async () => {
|
||||
const state: Partial<ExtensionState> = {
|
||||
autoApprovalEnabled: true,
|
||||
alwaysAllowMcp: true,
|
||||
}
|
||||
|
||||
const result = await checkAutoApproval({
|
||||
state: state as ExtensionState,
|
||||
ask: "use_mcp_server",
|
||||
text: undefined,
|
||||
})
|
||||
|
||||
expect(result).toEqual({ decision: "ask" })
|
||||
})
|
||||
|
||||
it("should handle invalid JSON gracefully", async () => {
|
||||
const state: Partial<ExtensionState> = {
|
||||
autoApprovalEnabled: true,
|
||||
alwaysAllowMcp: true,
|
||||
}
|
||||
|
||||
const result = await checkAutoApproval({
|
||||
state: state as ExtensionState,
|
||||
ask: "use_mcp_server",
|
||||
text: "invalid json",
|
||||
})
|
||||
|
||||
expect(result).toEqual({ decision: "ask" })
|
||||
})
|
||||
})
|
||||
|
||||
describe("General auto-approval settings", () => {
|
||||
it("should ask when autoApprovalEnabled is false", async () => {
|
||||
const state: Partial<ExtensionState> = {
|
||||
autoApprovalEnabled: false,
|
||||
alwaysAllowMcp: true,
|
||||
}
|
||||
|
||||
const result = await checkAutoApproval({
|
||||
state: state as ExtensionState,
|
||||
ask: "use_mcp_server",
|
||||
text: JSON.stringify({ type: "use_mcp_tool", serverName: "test", toolName: "test" }),
|
||||
})
|
||||
|
||||
expect(result).toEqual({ decision: "ask" })
|
||||
})
|
||||
|
||||
it("should ask when state is undefined", async () => {
|
||||
const result = await checkAutoApproval({
|
||||
state: undefined,
|
||||
ask: "use_mcp_server",
|
||||
text: JSON.stringify({ type: "use_mcp_tool", serverName: "test", toolName: "test" }),
|
||||
})
|
||||
|
||||
expect(result).toEqual({ decision: "ask" })
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
@ -99,7 +99,10 @@ export async function checkAutoApproval({
|
|||
const mcpServerUse = JSON.parse(text) as McpServerUse
|
||||
|
||||
if (mcpServerUse.type === "use_mcp_tool") {
|
||||
return state.alwaysAllowMcp === true && isMcpToolAlwaysAllowed(mcpServerUse, state.mcpServers)
|
||||
// Auto-approve if either:
|
||||
// 1. alwaysAllowMcp is enabled (approves all MCP tools)
|
||||
// 2. The specific tool has alwaysAllow flag set
|
||||
return state.alwaysAllowMcp === true || isMcpToolAlwaysAllowed(mcpServerUse, state.mcpServers)
|
||||
? { decision: "approve" }
|
||||
: { decision: "ask" }
|
||||
} else if (mcpServerUse.type === "access_mcp_resource") {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue