From 23efdeaf3584f842a1d774e7992edd8d74fe8bd5 Mon Sep 17 00:00:00 2001 From: Matt Rubens Date: Fri, 13 Dec 2024 17:16:27 -0500 Subject: [PATCH] More safety around always allowing MCP --- src/core/prompts/system.ts | 2 ++ src/core/webview/ClineProvider.ts | 10 ++++++++ src/shared/ExtensionMessage.ts | 1 + src/shared/WebviewMessage.ts | 1 + webview-ui/src/components/chat/ChatView.tsx | 6 ++--- webview-ui/src/components/mcp/McpToolRow.tsx | 5 ++-- webview-ui/src/components/mcp/McpView.tsx | 7 +++--- .../src/components/settings/SettingsView.tsx | 25 +++++++++++++++++++ .../src/context/ExtensionStateContext.tsx | 2 ++ 9 files changed, 51 insertions(+), 8 deletions(-) diff --git a/src/core/prompts/system.ts b/src/core/prompts/system.ts index 88cf9f6b5c..25e693a369 100644 --- a/src/core/prompts/system.ts +++ b/src/core/prompts/system.ts @@ -633,6 +633,8 @@ npm run build 5. Install the MCP Server by adding the MCP server configuration to the settings file located at '${await mcpHub.getMcpSettingsFilePath()}'. The settings file may have other MCP servers already configured, so you would read it first and then add your new server to the existing \`mcpServers\` object. +IMPORTANT: Regardless of what else you see in the settings file, you must not set any defaults for the \`alwaysAllow\` array in the newly added MCP server. + \`\`\`json { "mcpServers": { diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 4218ab60f4..5877f4fd1d 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -67,6 +67,7 @@ type GlobalStateKey = | "allowedCommands" | "soundEnabled" | "diffEnabled" + | "alwaysAllowMcp" export const GlobalFileNames = { apiConversationHistory: "api_conversation_history.json", @@ -456,6 +457,10 @@ export class ClineProvider implements vscode.WebviewViewProvider { await this.updateGlobalState("alwaysAllowBrowser", message.bool ?? undefined) await this.postStateToWebview() break + case "alwaysAllowMcp": + await this.updateGlobalState("alwaysAllowMcp", message.bool) + await this.postStateToWebview() + break case "askResponse": this.cline?.handleWebviewAskResponse(message.askResponse!, message.text, message.images) break @@ -904,6 +909,7 @@ export class ClineProvider implements vscode.WebviewViewProvider { alwaysAllowWrite, alwaysAllowExecute, alwaysAllowBrowser, + alwaysAllowMcp, soundEnabled, diffEnabled, taskHistory, @@ -921,6 +927,7 @@ export class ClineProvider implements vscode.WebviewViewProvider { alwaysAllowWrite: alwaysAllowWrite ?? false, alwaysAllowExecute: alwaysAllowExecute ?? false, alwaysAllowBrowser: alwaysAllowBrowser ?? false, + alwaysAllowMcp: alwaysAllowMcp ?? false, uriScheme: vscode.env.uriScheme, clineMessages: this.cline?.clineMessages || [], taskHistory: (taskHistory || []) @@ -1017,6 +1024,7 @@ export class ClineProvider implements vscode.WebviewViewProvider { alwaysAllowWrite, alwaysAllowExecute, alwaysAllowBrowser, + alwaysAllowMcp, taskHistory, allowedCommands, soundEnabled, @@ -1053,6 +1061,7 @@ export class ClineProvider implements vscode.WebviewViewProvider { this.getGlobalState("alwaysAllowWrite") as Promise, this.getGlobalState("alwaysAllowExecute") as Promise, this.getGlobalState("alwaysAllowBrowser") as Promise, + this.getGlobalState("alwaysAllowMcp") as Promise, this.getGlobalState("taskHistory") as Promise, this.getGlobalState("allowedCommands") as Promise, this.getGlobalState("soundEnabled") as Promise, @@ -1107,6 +1116,7 @@ export class ClineProvider implements vscode.WebviewViewProvider { alwaysAllowWrite: alwaysAllowWrite ?? false, alwaysAllowExecute: alwaysAllowExecute ?? false, alwaysAllowBrowser: alwaysAllowBrowser ?? false, + alwaysAllowMcp: alwaysAllowMcp ?? false, taskHistory, allowedCommands, soundEnabled, diff --git a/src/shared/ExtensionMessage.ts b/src/shared/ExtensionMessage.ts index c38cb63e80..608b5e5bb8 100644 --- a/src/shared/ExtensionMessage.ts +++ b/src/shared/ExtensionMessage.ts @@ -47,6 +47,7 @@ export interface ExtensionState { alwaysAllowWrite?: boolean alwaysAllowExecute?: boolean alwaysAllowBrowser?: boolean + alwaysAllowMcp?: boolean uriScheme?: string allowedCommands?: string[] soundEnabled?: boolean diff --git a/src/shared/WebviewMessage.ts b/src/shared/WebviewMessage.ts index fd5b63efc9..e7cfe43184 100644 --- a/src/shared/WebviewMessage.ts +++ b/src/shared/WebviewMessage.ts @@ -29,6 +29,7 @@ export interface WebviewMessage { | "cancelTask" | "refreshOpenRouterModels" | "alwaysAllowBrowser" + | "alwaysAllowMcp" | "playSound" | "soundEnabled" | "diffEnabled" diff --git a/webview-ui/src/components/chat/ChatView.tsx b/webview-ui/src/components/chat/ChatView.tsx index 08780ddab9..e4e0880ba7 100644 --- a/webview-ui/src/components/chat/ChatView.tsx +++ b/webview-ui/src/components/chat/ChatView.tsx @@ -37,7 +37,7 @@ interface ChatViewProps { export const MAX_IMAGES_PER_MESSAGE = 20 // Anthropic limits to 20 images const ChatView = ({ isHidden, showAnnouncement, hideAnnouncement, showHistoryView }: ChatViewProps) => { - const { version, clineMessages: messages, taskHistory, apiConfiguration, mcpServers, alwaysAllowBrowser, alwaysAllowReadOnly, alwaysAllowWrite, alwaysAllowExecute, allowedCommands } = useExtensionState() + const { version, clineMessages: messages, taskHistory, apiConfiguration, mcpServers, alwaysAllowBrowser, alwaysAllowReadOnly, alwaysAllowWrite, alwaysAllowExecute, alwaysAllowMcp, allowedCommands } = useExtensionState() //const task = messages.length > 0 ? (messages[0].say === "task" ? messages[0] : undefined) : undefined) : undefined const task = useMemo(() => messages.at(0), [messages]) // leaving this less safe version here since if the first message is not a task, then the extension is in a bad state and needs to be debugged (see Cline.abort) @@ -803,11 +803,11 @@ const ChatView = ({ isHidden, showAnnouncement, hideAnnouncement, showHistoryVie (alwaysAllowReadOnly && clineAsk === "tool" && isReadOnlyToolAction()) || (alwaysAllowWrite && clineAsk === "tool" && isWriteToolAction()) || (alwaysAllowExecute && clineAsk === "command" && isAllowedCommand()) || - (clineAsk === "use_mcp_server" && isMcpToolAlwaysAllowed()) + (alwaysAllowMcp && clineAsk === "use_mcp_server" && isMcpToolAlwaysAllowed()) ) { handlePrimaryButtonClick() } - }, [clineAsk, enableButtons, handlePrimaryButtonClick, alwaysAllowBrowser, alwaysAllowReadOnly, alwaysAllowWrite, alwaysAllowExecute, messages, allowedCommands, mcpServers]) + }, [clineAsk, enableButtons, handlePrimaryButtonClick, alwaysAllowBrowser, alwaysAllowReadOnly, alwaysAllowWrite, alwaysAllowExecute, alwaysAllowMcp, messages, allowedCommands, mcpServers]) return (
{ +const McpToolRow = ({ tool, serverName, alwaysAllowMcp }: McpToolRowProps) => { const handleAlwaysAllowChange = () => { if (!serverName) return; @@ -33,7 +34,7 @@ const McpToolRow = ({ tool, serverName }: McpToolRowProps) => { {tool.name}
- {serverName && ( + {serverName && alwaysAllowMcp && ( { - const { mcpServers: servers } = useExtensionState() + const { mcpServers: servers, alwaysAllowMcp } = useExtensionState() // const [servers, setServers] = useState([ // // Add some mock servers for testing // { @@ -126,7 +126,7 @@ const McpView = ({ onDone }: McpViewProps) => { {servers.length > 0 && (
{servers.map((server) => ( - + ))}
)} @@ -152,7 +152,7 @@ const McpView = ({ onDone }: McpViewProps) => { } // Server Row Component -const ServerRow = ({ server }: { server: McpServer }) => { +const ServerRow = ({ server, alwaysAllowMcp }: { server: McpServer, alwaysAllowMcp?: boolean }) => { const [isExpanded, setIsExpanded] = useState(false) const getStatusColor = () => { @@ -260,6 +260,7 @@ const ServerRow = ({ server }: { server: McpServer }) => { key={tool.name} tool={tool} serverName={server.name} + alwaysAllowMcp={alwaysAllowMcp} /> ))} diff --git a/webview-ui/src/components/settings/SettingsView.tsx b/webview-ui/src/components/settings/SettingsView.tsx index acff00cec4..0290974b4a 100644 --- a/webview-ui/src/components/settings/SettingsView.tsx +++ b/webview-ui/src/components/settings/SettingsView.tsx @@ -25,6 +25,8 @@ const SettingsView = ({ onDone }: SettingsViewProps) => { setAlwaysAllowExecute, alwaysAllowBrowser, setAlwaysAllowBrowser, + alwaysAllowMcp, + setAlwaysAllowMcp, soundEnabled, setSoundEnabled, diffEnabled, @@ -50,6 +52,7 @@ const SettingsView = ({ onDone }: SettingsViewProps) => { vscode.postMessage({ type: "alwaysAllowWrite", bool: alwaysAllowWrite }) vscode.postMessage({ type: "alwaysAllowExecute", bool: alwaysAllowExecute }) vscode.postMessage({ type: "alwaysAllowBrowser", bool: alwaysAllowBrowser }) + vscode.postMessage({ type: "alwaysAllowMcp", bool: alwaysAllowMcp }) vscode.postMessage({ type: "allowedCommands", commands: allowedCommands ?? [] }) vscode.postMessage({ type: "soundEnabled", bool: soundEnabled }) vscode.postMessage({ type: "diffEnabled", bool: diffEnabled }) @@ -195,7 +198,29 @@ const SettingsView = ({ onDone }: SettingsViewProps) => { color: "var(--vscode-errorForeground)", }}> ⚠️ WARNING: When enabled, Cline will automatically perform browser actions without requiring approval. This is potentially very dangerous and could lead to unwanted system modifications or security risks. Enable only if you fully trust the AI and understand the risks.

NOTE: The checkbox only applies when the model supports computer use. +

+ +
+ { + setAlwaysAllowMcp(e.target.checked) + vscode.postMessage({ type: "alwaysAllowMcp", bool: e.target.checked }) + }}> + Always approve MCP tools + +

+ ⚠️ WARNING: When enabled, you can set individual MCP tools to auto-approve in the MCP Servers view. A tool will only be auto-approved if both this setting and the tool's individual "Always allow" checkbox are enabled. This is potentially very dangerous and could lead to unwanted system modifications or security risks. Enable only if you fully trust the AI and understand the risks.

diff --git a/webview-ui/src/context/ExtensionStateContext.tsx b/webview-ui/src/context/ExtensionStateContext.tsx index 18345734bd..f9690b60c2 100644 --- a/webview-ui/src/context/ExtensionStateContext.tsx +++ b/webview-ui/src/context/ExtensionStateContext.tsx @@ -25,6 +25,7 @@ export interface ExtensionStateContextType extends ExtensionState { setAlwaysAllowWrite: (value: boolean) => void setAlwaysAllowExecute: (value: boolean) => void setAlwaysAllowBrowser: (value: boolean) => void + setAlwaysAllowMcp: (value: boolean) => void setShowAnnouncement: (value: boolean) => void setAllowedCommands: (value: string[]) => void setSoundEnabled: (value: boolean) => void @@ -134,6 +135,7 @@ export const ExtensionStateContextProvider: React.FC<{ children: React.ReactNode setAlwaysAllowWrite: (value) => setState((prevState) => ({ ...prevState, alwaysAllowWrite: value })), setAlwaysAllowExecute: (value) => setState((prevState) => ({ ...prevState, alwaysAllowExecute: value })), setAlwaysAllowBrowser: (value) => setState((prevState) => ({ ...prevState, alwaysAllowBrowser: value })), + setAlwaysAllowMcp: (value) => setState((prevState) => ({ ...prevState, alwaysAllowMcp: value })), setShowAnnouncement: (value) => setState((prevState) => ({ ...prevState, shouldShowAnnouncement: value })), setAllowedCommands: (value) => setState((prevState) => ({ ...prevState, allowedCommands: value })), setSoundEnabled: (value) => setState((prevState) => ({ ...prevState, soundEnabled: value })),