From 95b6c2a8d56d47b7268f09a249c76796af74d442 Mon Sep 17 00:00:00 2001 From: Roo Code Date: Mon, 30 Jun 2025 20:17:00 +0000 Subject: [PATCH 1/4] feat: Add 'Add & Run' button to command approval UI - Add third button option to command approval dialog - Implement streamlined whitelisting workflow that adds command to whitelist and executes it - Update UI to show three buttons: 'Run Command', 'Add & Run', and 'Reject' - Add backend logic to detect 'Add & Run' selection and update allowedCommands setting - Enhance message passing system to support tertiary button interactions Fixes #5290 --- .../presentAssistantMessage.ts | 17 +- src/core/tools/executeCommandTool.ts | 37 ++- src/shared/ExtensionMessage.ts | 8 +- src/shared/WebviewMessage.ts | 7 +- webview-ui/src/components/chat/ChatView.tsx | 219 +++++++++++++----- 5 files changed, 222 insertions(+), 66 deletions(-) diff --git a/src/core/assistant-message/presentAssistantMessage.ts b/src/core/assistant-message/presentAssistantMessage.ts index 21c973ab507..de41e91185b 100644 --- a/src/core/assistant-message/presentAssistantMessage.ts +++ b/src/core/assistant-message/presentAssistantMessage.ts @@ -271,7 +271,14 @@ export async function presentAssistantMessage(cline: Task) { isProtected || false, ) - if (response !== "yesButtonClicked") { + if (response === "yesButtonClicked" || response === "addAndRunButtonClicked") { + // Handle yesButtonClicked or addAndRunButtonClicked with text. + if (text) { + await cline.say("user_feedback", text, images) + pushToolResult(formatResponse.toolResult(formatResponse.toolApprovedWithFeedback(text), images)) + } + return true + } else { // Handle both messageResponse and noButtonClicked with text. if (text) { await cline.say("user_feedback", text, images) @@ -282,14 +289,6 @@ export async function presentAssistantMessage(cline: Task) { cline.didRejectTool = true return false } - - // Handle yesButtonClicked with text. - if (text) { - await cline.say("user_feedback", text, images) - pushToolResult(formatResponse.toolResult(formatResponse.toolApprovedWithFeedback(text), images)) - } - - return true } const askFinishSubTaskApproval = async () => { diff --git a/src/core/tools/executeCommandTool.ts b/src/core/tools/executeCommandTool.ts index 795beccc061..e9e9a42a1f4 100644 --- a/src/core/tools/executeCommandTool.ts +++ b/src/core/tools/executeCommandTool.ts @@ -51,9 +51,42 @@ export async function executeCommandTool( cline.consecutiveMistakeCount = 0 command = unescapeHtmlEntities(command) // Unescape HTML entities. - const didApprove = await askApproval("command", command) - if (!didApprove) { + // We need to capture the actual response to check if "Add & Run" was clicked + const { response, text, images } = await cline.ask("command", command) + + if (response === "yesButtonClicked" || response === "addAndRunButtonClicked") { + // Handle yesButtonClicked or addAndRunButtonClicked with text. + if (text) { + await cline.say("user_feedback", text, images) + } + + // Check if user selected "Add & Run" to add command to whitelist + if (response === "addAndRunButtonClicked") { + const clineProvider = await cline.providerRef.deref() + if (clineProvider) { + const state = await clineProvider.getState() + const currentCommands = state.allowedCommands ?? [] + + // Add command to whitelist if not already present + if (!currentCommands.includes(command)) { + const newCommands = [...currentCommands, command] + await clineProvider.setValue("allowedCommands", newCommands) + + // Notify webview of the updated commands + await clineProvider.postMessageToWebview({ + type: "invoke", + invoke: "setChatBoxMessage", + text: `Command "${command}" added to whitelist.`, + }) + } + } + } + } else { + // Handle both messageResponse and noButtonClicked with text. + if (text) { + await cline.say("user_feedback", text, images) + } return } diff --git a/src/shared/ExtensionMessage.ts b/src/shared/ExtensionMessage.ts index 73ebf59d4c8..15f58720c1d 100644 --- a/src/shared/ExtensionMessage.ts +++ b/src/shared/ExtensionMessage.ts @@ -112,7 +112,13 @@ export interface ExtensionMessage { | "didBecomeVisible" | "focusInput" | "switchTab" - invoke?: "newChat" | "sendMessage" | "primaryButtonClick" | "secondaryButtonClick" | "setChatBoxMessage" + invoke?: + | "newChat" + | "sendMessage" + | "primaryButtonClick" + | "secondaryButtonClick" + | "tertiaryButtonClick" + | "setChatBoxMessage" state?: ExtensionState images?: string[] filePaths?: string[] diff --git a/src/shared/WebviewMessage.ts b/src/shared/WebviewMessage.ts index 7efc97e8c77..17a25b6a729 100644 --- a/src/shared/WebviewMessage.ts +++ b/src/shared/WebviewMessage.ts @@ -12,7 +12,12 @@ import { marketplaceItemSchema } from "@roo-code/types" import { Mode } from "./modes" -export type ClineAskResponse = "yesButtonClicked" | "noButtonClicked" | "messageResponse" | "objectResponse" +export type ClineAskResponse = + | "yesButtonClicked" + | "noButtonClicked" + | "addAndRunButtonClicked" + | "messageResponse" + | "objectResponse" export type PromptMode = Mode | "enhance" diff --git a/webview-ui/src/components/chat/ChatView.tsx b/webview-ui/src/components/chat/ChatView.tsx index a4f18c870c1..5ddce925ca6 100644 --- a/webview-ui/src/components/chat/ChatView.tsx +++ b/webview-ui/src/components/chat/ChatView.tsx @@ -139,6 +139,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction(false) const [primaryButtonText, setPrimaryButtonText] = useState(undefined) const [secondaryButtonText, setSecondaryButtonText] = useState(undefined) + const [tertiaryButtonText, setTertiaryButtonText] = useState(undefined) const [didClickCancel, setDidClickCancel] = useState(false) const virtuosoRef = useRef(null) const [expandedRows, setExpandedRows] = useState>({}) @@ -312,7 +313,8 @@ const ChatViewComponent: React.ForwardRefRenderFunction 0)) { + vscode.postMessage({ + type: "askResponse", + askResponse: "addAndRunButtonClicked", + text: trimmedInput, + images: images, + }) + } else { + vscode.postMessage({ type: "askResponse", askResponse: "addAndRunButtonClicked" }) + } + // Clear input state after sending + setInputValue("") + setSelectedImages([]) + break case "tool": case "browser_action_launch": case "use_mcp_server": @@ -639,6 +657,36 @@ const ChatViewComponent: React.ForwardRefRenderFunction { + const trimmedInput = text?.trim() + + switch (clineAsk) { + case "command": + // For command case, tertiary button is "Reject" + // Only send text/images if they exist + if (trimmedInput || (images && images.length > 0)) { + vscode.postMessage({ + type: "askResponse", + askResponse: "noButtonClicked", + text: trimmedInput, + images: images, + }) + } else { + vscode.postMessage({ type: "askResponse", askResponse: "noButtonClicked" }) + } + // Clear input state after sending + setInputValue("") + setSelectedImages([]) + break + } + setSendingDisabled(true) + setClineAsk(undefined) + setEnableButtons(false) + }, + [clineAsk], + ) + const handleTaskCloseButtonClick = useCallback(() => startNewTask(), [startNewTask]) const { info: model } = useSelectedModel(apiConfiguration) @@ -690,6 +738,9 @@ const ChatViewComponent: React.ForwardRefRenderFunction ) : (
- {primaryButtonText && !isStreaming && ( - + {/* Three-button layout for command approval */} + {tertiaryButtonText && !isStreaming ? ( + <> + {/* Top row: Run and Add & Run buttons */} +
+ {primaryButtonText && ( + - handlePrimaryButtonClick(inputValue, selectedImages)}> - {primaryButtonText} - - - )} - {(secondaryButtonText || isStreaming) && ( - - handleSecondaryButtonClick(inputValue, selectedImages)}> - {isStreaming ? t("chat:cancel.title") : secondaryButtonText} - - + }> + + handlePrimaryButtonClick(inputValue, selectedImages) + }> + {primaryButtonText} + + + )} + {secondaryButtonText && ( + + + handleSecondaryButtonClick(inputValue, selectedImages) + }> + {secondaryButtonText} + + + )} +
+ {/* Bottom row: Reject button */} +
+ + handleTertiaryButtonClick(inputValue, selectedImages)}> + {tertiaryButtonText} + + +
+ + ) : ( + /* Two-button layout for other cases */ + <> + {primaryButtonText && !isStreaming && ( + + handlePrimaryButtonClick(inputValue, selectedImages)}> + {primaryButtonText} + + + )} + {(secondaryButtonText || isStreaming) && !tertiaryButtonText && ( + + handleSecondaryButtonClick(inputValue, selectedImages)}> + {isStreaming ? t("chat:cancel.title") : secondaryButtonText} + + + )} + )}
)} From 8fd1210776c63933448cb2294c8f26c8a8906316 Mon Sep 17 00:00:00 2001 From: hannesrudolph Date: Tue, 1 Jul 2025 08:33:09 -0600 Subject: [PATCH 2/4] Address PR review comments - Use extractCommandPattern function to extract base command patterns - Add proper error handling for providerRef.deref() - Use internationalization for user-facing messages - Extract addCommandToWhitelist helper function to reduce duplication - Add comments explaining why we can't use the shared askApproval function --- src/core/tools/executeCommandTool.ts | 137 ++++++++++++++++++--- src/i18n/locales/en/tools.json | 3 + webview-ui/src/utils/command-validation.ts | 45 +++++++ 3 files changed, 165 insertions(+), 20 deletions(-) diff --git a/src/core/tools/executeCommandTool.ts b/src/core/tools/executeCommandTool.ts index e9e9a42a1f4..830dbb80185 100644 --- a/src/core/tools/executeCommandTool.ts +++ b/src/core/tools/executeCommandTool.ts @@ -14,9 +14,116 @@ import { unescapeHtmlEntities } from "../../utils/text-normalization" import { ExitCodeDetails, RooTerminalCallbacks, RooTerminalProcess } from "../../integrations/terminal/types" import { TerminalRegistry } from "../../integrations/terminal/TerminalRegistry" import { Terminal } from "../../integrations/terminal/Terminal" +import { t } from "../../i18n" class ShellIntegrationError extends Error {} +/** + * Extract the base command pattern from a full command string. + * For example: "gh pr checkout 1234" -> "gh pr checkout" + * + * @param command The full command string + * @returns The base command pattern suitable for whitelisting + */ +function extractCommandPattern(command: string): string { + if (!command?.trim()) return "" + + // Split by whitespace, handling quoted strings + const parts: string[] = [] + let current = "" + let inQuotes = false + let quoteChar = "" + + for (let i = 0; i < command.length; i++) { + const char = command[i] + const prevChar = i > 0 ? command[i - 1] : "" + + if ((char === '"' || char === "'") && prevChar !== "\\") { + if (!inQuotes) { + inQuotes = true + quoteChar = char + } else if (char === quoteChar) { + inQuotes = false + quoteChar = "" + } + current += char + } else if (char === " " && !inQuotes) { + if (current) { + parts.push(current) + current = "" + } + } else { + current += char + } + } + + if (current) { + parts.push(current) + } + + // Extract pattern parts, stopping at arguments + const patternParts: string[] = [] + + for (const part of parts) { + // Remove quotes for analysis + const unquoted = part.replace(/^["']|["']$/g, "") + + // Stop at common argument patterns: + // - Pure numbers (like PR numbers, PIDs, etc.) + // - Flags starting with - or -- + // - File paths or URLs + // - Variable assignments (KEY=VALUE) + // - Operators (&&, ||, |, ;, >, <, etc.) + if ( + /^\d+$/.test(unquoted) || + unquoted.startsWith("-") || + unquoted.includes("/") || + unquoted.includes("\\") || + unquoted.includes("=") || + unquoted.startsWith("http") || + unquoted.includes(".") || + ["&&", "||", "|", ";", ">", "<", ">>", "<<", "&"].includes(unquoted) + ) { + // Stop collecting pattern parts + break + } + patternParts.push(part) + } + + // Return the base command pattern + return patternParts.join(" ") +} + +/** + * Adds a command pattern to the whitelist + */ +async function addCommandToWhitelist(cline: Task, command: string): Promise { + const clineProvider = cline.providerRef.deref() + if (!clineProvider) { + console.error("Provider reference is undefined, cannot add command to whitelist") + return + } + + const state = await clineProvider.getState() + const currentCommands = state.allowedCommands ?? [] + + // Extract the base command pattern for whitelisting + const commandPattern = extractCommandPattern(command) + + // Add command pattern to whitelist if not already present + if (commandPattern && !currentCommands.includes(commandPattern)) { + const newCommands = [...currentCommands, commandPattern] + await clineProvider.setValue("allowedCommands", newCommands) + + // Notify webview of the updated commands + await clineProvider.postMessageToWebview({ + type: "invoke", + invoke: "setChatBoxMessage", + text: t("tools:executeCommand.patternAddedToWhitelist", { pattern: commandPattern }), + }) + } +} + export async function executeCommandTool( cline: Task, block: ToolUse, @@ -53,40 +160,30 @@ export async function executeCommandTool( command = unescapeHtmlEntities(command) // Unescape HTML entities. // We need to capture the actual response to check if "Add & Run" was clicked + // Note: We cannot use the provided askApproval function here because we need to + // differentiate between "yesButtonClicked" and "addAndRunButtonClicked" responses const { response, text, images } = await cline.ask("command", command) if (response === "yesButtonClicked" || response === "addAndRunButtonClicked") { - // Handle yesButtonClicked or addAndRunButtonClicked with text. + // Handle yesButtonClicked or addAndRunButtonClicked with text (following askApproval pattern) if (text) { await cline.say("user_feedback", text, images) + pushToolResult(formatResponse.toolResult(formatResponse.toolApprovedWithFeedback(text), images)) } // Check if user selected "Add & Run" to add command to whitelist if (response === "addAndRunButtonClicked") { - const clineProvider = await cline.providerRef.deref() - if (clineProvider) { - const state = await clineProvider.getState() - const currentCommands = state.allowedCommands ?? [] - - // Add command to whitelist if not already present - if (!currentCommands.includes(command)) { - const newCommands = [...currentCommands, command] - await clineProvider.setValue("allowedCommands", newCommands) - - // Notify webview of the updated commands - await clineProvider.postMessageToWebview({ - type: "invoke", - invoke: "setChatBoxMessage", - text: `Command "${command}" added to whitelist.`, - }) - } - } + await addCommandToWhitelist(cline, command) } } else { - // Handle both messageResponse and noButtonClicked with text. + // Handle both messageResponse and noButtonClicked with text (following askApproval pattern) if (text) { await cline.say("user_feedback", text, images) + pushToolResult(formatResponse.toolResult(formatResponse.toolDeniedWithFeedback(text), images)) + } else { + pushToolResult(formatResponse.toolDenied()) } + cline.didRejectTool = true return } diff --git a/src/i18n/locales/en/tools.json b/src/i18n/locales/en/tools.json index 0265a843985..9f3992d4ded 100644 --- a/src/i18n/locales/en/tools.json +++ b/src/i18n/locales/en/tools.json @@ -12,5 +12,8 @@ "errors": { "policy_restriction": "Failed to create new task due to policy restrictions." } + }, + "executeCommand": { + "patternAddedToWhitelist": "Command pattern '{{pattern}}' added to whitelist" } } diff --git a/webview-ui/src/utils/command-validation.ts b/webview-ui/src/utils/command-validation.ts index 7ad21b1675c..d4943ec5af4 100644 --- a/webview-ui/src/utils/command-validation.ts +++ b/webview-ui/src/utils/command-validation.ts @@ -2,6 +2,51 @@ import { parse } from "shell-quote" type ShellToken = string | { op: string } | { command: string } +/** + * Extract the base command pattern from a full command string. + * For example: "gh pr checkout 1234" -> "gh pr checkout" + * + * @param command The full command string + * @returns The base command pattern suitable for whitelisting + */ +export function extractCommandPattern(command: string): string { + if (!command?.trim()) return "" + + // Parse the command to get tokens + const tokens = parse(command.trim()) as ShellToken[] + const patternParts: string[] = [] + + for (const token of tokens) { + if (typeof token === "string") { + // Check if this token looks like an argument (number, flag, etc.) + // Common patterns to stop at: + // - Pure numbers (like PR numbers, PIDs, etc.) + // - Flags starting with - or -- + // - File paths or URLs + // - Variable assignments (KEY=VALUE) + if ( + /^\d+$/.test(token) || + token.startsWith("-") || + token.includes("/") || + token.includes("\\") || + token.includes("=") || + token.startsWith("http") || + token.includes(".") + ) { + // Stop collecting pattern parts + break + } + patternParts.push(token) + } else if (typeof token === "object" && "op" in token) { + // Stop at operators + break + } + } + + // Return the base command pattern + return patternParts.join(" ") +} + /** * Split a command string into individual sub-commands by * chaining operators (&&, ||, ;, or |). From db8c60ceffd1f0a3b6144b40950276ae6095f4e9 Mon Sep 17 00:00:00 2001 From: hannesrudolph Date: Tue, 1 Jul 2025 08:44:12 -0600 Subject: [PATCH 3/4] Add missing translation key to all locale files - Added 'executeCommand.patternAddedToWhitelist' translation to all 17 non-English locale files - This fixes the failing check-translations CI job --- src/i18n/locales/ca/tools.json | 3 +++ src/i18n/locales/de/tools.json | 3 +++ src/i18n/locales/es/tools.json | 3 +++ src/i18n/locales/fr/tools.json | 3 +++ src/i18n/locales/hi/tools.json | 3 +++ src/i18n/locales/id/tools.json | 3 +++ src/i18n/locales/it/tools.json | 3 +++ src/i18n/locales/ja/tools.json | 3 +++ src/i18n/locales/ko/tools.json | 3 +++ src/i18n/locales/nl/tools.json | 3 +++ src/i18n/locales/pl/tools.json | 3 +++ src/i18n/locales/pt-BR/tools.json | 3 +++ src/i18n/locales/ru/tools.json | 3 +++ src/i18n/locales/tr/tools.json | 3 +++ src/i18n/locales/vi/tools.json | 3 +++ src/i18n/locales/zh-CN/tools.json | 3 +++ src/i18n/locales/zh-TW/tools.json | 3 +++ 17 files changed, 51 insertions(+) diff --git a/src/i18n/locales/ca/tools.json b/src/i18n/locales/ca/tools.json index 5b3a228bdec..835523f1473 100644 --- a/src/i18n/locales/ca/tools.json +++ b/src/i18n/locales/ca/tools.json @@ -12,5 +12,8 @@ "errors": { "policy_restriction": "No s'ha pogut crear una nova tasca a causa de restriccions de política." } + }, + "executeCommand": { + "patternAddedToWhitelist": "Command pattern \"{{pattern}}\" has been added to the allowed commands list." } } diff --git a/src/i18n/locales/de/tools.json b/src/i18n/locales/de/tools.json index eb1afbc0821..16e871dc663 100644 --- a/src/i18n/locales/de/tools.json +++ b/src/i18n/locales/de/tools.json @@ -12,5 +12,8 @@ "errors": { "policy_restriction": "Neue Aufgabe konnte aufgrund von Richtlinienbeschränkungen nicht erstellt werden." } + }, + "executeCommand": { + "patternAddedToWhitelist": "Command pattern \"{{pattern}}\" has been added to the allowed commands list." } } diff --git a/src/i18n/locales/es/tools.json b/src/i18n/locales/es/tools.json index 303f5365ed0..539010428ee 100644 --- a/src/i18n/locales/es/tools.json +++ b/src/i18n/locales/es/tools.json @@ -12,5 +12,8 @@ "errors": { "policy_restriction": "No se pudo crear una nueva tarea debido a restricciones de política." } + }, + "executeCommand": { + "patternAddedToWhitelist": "Command pattern \"{{pattern}}\" has been added to the allowed commands list." } } diff --git a/src/i18n/locales/fr/tools.json b/src/i18n/locales/fr/tools.json index a6c71aca333..e8bf5c3ce52 100644 --- a/src/i18n/locales/fr/tools.json +++ b/src/i18n/locales/fr/tools.json @@ -12,5 +12,8 @@ "errors": { "policy_restriction": "Impossible de créer une nouvelle tâche en raison de restrictions de politique." } + }, + "executeCommand": { + "patternAddedToWhitelist": "Command pattern \"{{pattern}}\" has been added to the allowed commands list." } } diff --git a/src/i18n/locales/hi/tools.json b/src/i18n/locales/hi/tools.json index 0cb4aeb14ec..f35a4798a51 100644 --- a/src/i18n/locales/hi/tools.json +++ b/src/i18n/locales/hi/tools.json @@ -12,5 +12,8 @@ "errors": { "policy_restriction": "नीति प्रतिबंधों के कारण नया कार्य बनाने में विफल।" } + }, + "executeCommand": { + "patternAddedToWhitelist": "Command pattern \"{{pattern}}\" has been added to the allowed commands list." } } diff --git a/src/i18n/locales/id/tools.json b/src/i18n/locales/id/tools.json index 2e3c4f0c22e..185a323b1a2 100644 --- a/src/i18n/locales/id/tools.json +++ b/src/i18n/locales/id/tools.json @@ -15,5 +15,8 @@ "errors": { "policy_restriction": "Gagal membuat tugas baru karena pembatasan kebijakan." } + }, + "executeCommand": { + "patternAddedToWhitelist": "Command pattern \"{{pattern}}\" has been added to the allowed commands list." } } diff --git a/src/i18n/locales/it/tools.json b/src/i18n/locales/it/tools.json index ffae474f1db..1733d157aa1 100644 --- a/src/i18n/locales/it/tools.json +++ b/src/i18n/locales/it/tools.json @@ -12,5 +12,8 @@ "errors": { "policy_restriction": "Impossibile creare una nuova attività a causa di restrizioni di policy." } + }, + "executeCommand": { + "patternAddedToWhitelist": "Command pattern \"{{pattern}}\" has been added to the allowed commands list." } } diff --git a/src/i18n/locales/ja/tools.json b/src/i18n/locales/ja/tools.json index 04a5fcc0856..0876a6c4b4e 100644 --- a/src/i18n/locales/ja/tools.json +++ b/src/i18n/locales/ja/tools.json @@ -12,5 +12,8 @@ "errors": { "policy_restriction": "ポリシー制限により新しいタスクを作成できませんでした。" } + }, + "executeCommand": { + "patternAddedToWhitelist": "Command pattern \"{{pattern}}\" has been added to the allowed commands list." } } diff --git a/src/i18n/locales/ko/tools.json b/src/i18n/locales/ko/tools.json index e43a541794a..fb888a231f3 100644 --- a/src/i18n/locales/ko/tools.json +++ b/src/i18n/locales/ko/tools.json @@ -12,5 +12,8 @@ "errors": { "policy_restriction": "정책 제한으로 인해 새 작업을 생성하지 못했습니다." } + }, + "executeCommand": { + "patternAddedToWhitelist": "Command pattern \"{{pattern}}\" has been added to the allowed commands list." } } diff --git a/src/i18n/locales/nl/tools.json b/src/i18n/locales/nl/tools.json index 56a8cdbc466..4157e593403 100644 --- a/src/i18n/locales/nl/tools.json +++ b/src/i18n/locales/nl/tools.json @@ -12,5 +12,8 @@ "errors": { "policy_restriction": "Kan geen nieuwe taak aanmaken vanwege beleidsbeperkingen." } + }, + "executeCommand": { + "patternAddedToWhitelist": "Command pattern \"{{pattern}}\" has been added to the allowed commands list." } } diff --git a/src/i18n/locales/pl/tools.json b/src/i18n/locales/pl/tools.json index 62568826aae..cd7e8a273e0 100644 --- a/src/i18n/locales/pl/tools.json +++ b/src/i18n/locales/pl/tools.json @@ -12,5 +12,8 @@ "errors": { "policy_restriction": "Nie udało się utworzyć nowego zadania z powodu ograniczeń polityki." } + }, + "executeCommand": { + "patternAddedToWhitelist": "Command pattern \"{{pattern}}\" has been added to the allowed commands list." } } diff --git a/src/i18n/locales/pt-BR/tools.json b/src/i18n/locales/pt-BR/tools.json index f74e0f8196e..9855d078752 100644 --- a/src/i18n/locales/pt-BR/tools.json +++ b/src/i18n/locales/pt-BR/tools.json @@ -12,5 +12,8 @@ "errors": { "policy_restriction": "Falha ao criar nova tarefa devido a restrições de política." } + }, + "executeCommand": { + "patternAddedToWhitelist": "Command pattern \"{{pattern}}\" has been added to the allowed commands list." } } diff --git a/src/i18n/locales/ru/tools.json b/src/i18n/locales/ru/tools.json index 1e59d10499c..39de1ef758d 100644 --- a/src/i18n/locales/ru/tools.json +++ b/src/i18n/locales/ru/tools.json @@ -12,5 +12,8 @@ "errors": { "policy_restriction": "Не удалось создать новую задачу из-за ограничений политики." } + }, + "executeCommand": { + "patternAddedToWhitelist": "Command pattern \"{{pattern}}\" has been added to the allowed commands list." } } diff --git a/src/i18n/locales/tr/tools.json b/src/i18n/locales/tr/tools.json index e4c73cdc4b2..f7d8682a9cd 100644 --- a/src/i18n/locales/tr/tools.json +++ b/src/i18n/locales/tr/tools.json @@ -12,5 +12,8 @@ "errors": { "policy_restriction": "Politika kısıtlamaları nedeniyle yeni görev oluşturulamadı." } + }, + "executeCommand": { + "patternAddedToWhitelist": "Command pattern \"{{pattern}}\" has been added to the allowed commands list." } } diff --git a/src/i18n/locales/vi/tools.json b/src/i18n/locales/vi/tools.json index 9811ee12c92..269b0beebb3 100644 --- a/src/i18n/locales/vi/tools.json +++ b/src/i18n/locales/vi/tools.json @@ -12,5 +12,8 @@ "errors": { "policy_restriction": "Không thể tạo nhiệm vụ mới do hạn chế chính sách." } + }, + "executeCommand": { + "patternAddedToWhitelist": "Command pattern \"{{pattern}}\" has been added to the allowed commands list." } } diff --git a/src/i18n/locales/zh-CN/tools.json b/src/i18n/locales/zh-CN/tools.json index 13641b8d43b..e28d3b21137 100644 --- a/src/i18n/locales/zh-CN/tools.json +++ b/src/i18n/locales/zh-CN/tools.json @@ -12,5 +12,8 @@ "errors": { "policy_restriction": "由于策略限制，无法创建新任务。" } + }, + "executeCommand": { + "patternAddedToWhitelist": "Command pattern \"{{pattern}}\" has been added to the allowed commands list." } } diff --git a/src/i18n/locales/zh-TW/tools.json b/src/i18n/locales/zh-TW/tools.json index a726e3c9192..4a20d9bac4b 100644 --- a/src/i18n/locales/zh-TW/tools.json +++ b/src/i18n/locales/zh-TW/tools.json @@ -12,5 +12,8 @@ "errors": { "policy_restriction": "由於政策限制，無法建立新工作。" } + }, + "executeCommand": { + "patternAddedToWhitelist": "Command pattern \"{{pattern}}\" has been added to the allowed commands list." } } From 82c3e927b8d600e58953de820ab53c2d2be56948 Mon Sep 17 00:00:00 2001 From: hannesrudolph Date: Tue, 1 Jul 2025 09:25:40 -0600 Subject: [PATCH 4/4] fix: Update shell-quote to v1.8.3 and use it for command pattern extraction - Updated shell-quote from v1.8.2 to v1.8.3 in webview-ui - Added shell-quote v1.8.3 to backend dependencies - Updated shared extract-command-pattern module to use shell-quote for proper parsing - Added @types/shell-quote for TypeScript support - Ensures consistent command pattern extraction across frontend and backend --- pnpm-lock.yaml | 23 +++--- src/core/tools/executeCommandTool.ts | 77 +-------------------- src/package.json | 2 + src/shared/extract-command-pattern.ts | 50 +++++++++++++ webview-ui/package.json | 2 +- webview-ui/src/components/chat/ChatView.tsx | 2 +- webview-ui/src/i18n/locales/ca/chat.json | 4 ++ webview-ui/src/i18n/locales/de/chat.json | 4 ++ webview-ui/src/i18n/locales/en/chat.json | 4 ++ webview-ui/src/i18n/locales/es/chat.json | 4 ++ webview-ui/src/i18n/locales/fr/chat.json | 4 ++ webview-ui/src/i18n/locales/hi/chat.json | 4 ++ webview-ui/src/i18n/locales/id/chat.json | 4 ++ webview-ui/src/i18n/locales/it/chat.json | 4 ++ webview-ui/src/i18n/locales/ja/chat.json | 4 ++ webview-ui/src/i18n/locales/ko/chat.json | 4 ++ webview-ui/src/i18n/locales/nl/chat.json | 4 ++ webview-ui/src/i18n/locales/pl/chat.json | 4 ++ webview-ui/src/i18n/locales/pt-BR/chat.json | 4 ++ webview-ui/src/i18n/locales/ru/chat.json | 4 ++ webview-ui/src/i18n/locales/tr/chat.json | 4 ++ webview-ui/src/i18n/locales/vi/chat.json | 4 ++ webview-ui/src/i18n/locales/zh-CN/chat.json | 4 ++ webview-ui/src/i18n/locales/zh-TW/chat.json | 4 ++ 24 files changed, 138 insertions(+), 90 deletions(-) create mode 100644 src/shared/extract-command-pattern.ts diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 5030055feac..b6569d4113b 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -723,6 +723,9 @@ importers: serialize-error: specifier: ^12.0.0 version: 12.0.0 + shell-quote: + specifier: 1.8.3 + version: 1.8.3 simple-git: specifier: ^3.27.0 version: 3.27.0 @@ -814,6 +817,9 @@ importers: '@types/ps-tree': specifier: ^1.1.6 version: 1.1.6 + '@types/shell-quote': + specifier: ^1.7.5 + version: 1.7.5 '@types/stream-json': specifier: ^1.7.8 version: 1.7.8 @@ -1028,8 +1034,8 @@ importers: specifier: ^0.6.0 version: 0.6.2 shell-quote: - specifier: ^1.8.2 - version: 1.8.2 + specifier: ^1.8.3 + version: 1.8.3 shiki: specifier: ^3.2.1 version: 3.4.1 @@ -8478,10 +8484,6 @@ packages: resolution: {integrity: sha512-7++dFhtcx3353uBaq8DDR4NuxBetBzC7ZQOhmTQInHEd6bSrXdiEyzCvG07Z44UYdLShWUyXt5M/yhz8ekcb1A==} engines: {node: '>=8'} - shell-quote@1.8.2: - resolution: {integrity: sha512-AzqKpGKjrj7EM6rKVQEPpB288oCfnrEIuyoT9cyF4nmGa7V8Zk6f7RRqYisX8X9m+Q7bd632aZW4ky7EhbQztA==} - engines: {node: '>= 0.4'} - shell-quote@1.8.3: resolution: {integrity: sha512-ObmnIF4hXNg1BqhnHmgbDETF8dLPCggZWBjkQfhZpbszZnYur5DUljTcCHii5LC3J5E0yeO/1LIMyH+UvHQgyw==} engines: {node: '>= 0.4'} @@ -13295,7 +13297,7 @@ snapshots: sirv: 3.0.1 tinyglobby: 0.2.14 tinyrainbow: 2.0.0 - vitest: 3.2.4(@types/debug@4.1.12)(@types/node@22.15.29)(@vitest/ui@3.2.4)(jiti@2.4.2)(jsdom@26.1.0)(lightningcss@1.30.1)(tsx@4.19.4)(yaml@2.8.0) + vitest: 3.2.4(@types/debug@4.1.12)(@types/node@20.17.50)(@vitest/ui@3.2.4)(jiti@2.4.2)(jsdom@26.1.0)(lightningcss@1.30.1)(tsx@4.19.4)(yaml@2.8.0) '@vitest/utils@3.2.4': dependencies: @@ -17293,7 +17295,7 @@ snapshots: minimatch: 10.0.1 pidtree: 0.6.0 read-package-json-fast: 4.0.0 - shell-quote: 1.8.2 + shell-quote: 1.8.3 which: 5.0.0 npm-run-path@4.0.1: @@ -18522,10 +18524,7 @@ snapshots: shebang-regex@3.0.0: {} - shell-quote@1.8.2: {} - - shell-quote@1.8.3: - optional: true + shell-quote@1.8.3: {} shiki@3.4.1: dependencies: diff --git a/src/core/tools/executeCommandTool.ts b/src/core/tools/executeCommandTool.ts index 830dbb80185..80a66834d21 100644 --- a/src/core/tools/executeCommandTool.ts +++ b/src/core/tools/executeCommandTool.ts @@ -15,85 +15,10 @@ import { ExitCodeDetails, RooTerminalCallbacks, RooTerminalProcess } from "../.. import { TerminalRegistry } from "../../integrations/terminal/TerminalRegistry" import { Terminal } from "../../integrations/terminal/Terminal" import { t } from "../../i18n" +import { extractCommandPattern } from "../../shared/extract-command-pattern" class ShellIntegrationError extends Error {} -/** - * Extract the base command pattern from a full command string. - * For example: "gh pr checkout 1234" -> "gh pr checkout" - * - * @param command The full command string - * @returns The base command pattern suitable for whitelisting - */ -function extractCommandPattern(command: string): string { - if (!command?.trim()) return "" - - // Split by whitespace, handling quoted strings - const parts: string[] = [] - let current = "" - let inQuotes = false - let quoteChar = "" - - for (let i = 0; i < command.length; i++) { - const char = command[i] - const prevChar = i > 0 ? command[i - 1] : "" - - if ((char === '"' || char === "'") && prevChar !== "\\") { - if (!inQuotes) { - inQuotes = true - quoteChar = char - } else if (char === quoteChar) { - inQuotes = false - quoteChar = "" - } - current += char - } else if (char === " " && !inQuotes) { - if (current) { - parts.push(current) - current = "" - } - } else { - current += char - } - } - - if (current) { - parts.push(current) - } - - // Extract pattern parts, stopping at arguments - const patternParts: string[] = [] - - for (const part of parts) { - // Remove quotes for analysis - const unquoted = part.replace(/^["']|["']$/g, "") - - // Stop at common argument patterns: - // - Pure numbers (like PR numbers, PIDs, etc.) - // - Flags starting with - or -- - // - File paths or URLs - // - Variable assignments (KEY=VALUE) - // - Operators (&&, ||, |, ;, >, <, etc.) - if ( - /^\d+$/.test(unquoted) || - unquoted.startsWith("-") || - unquoted.includes("/") || - unquoted.includes("\\") || - unquoted.includes("=") || - unquoted.startsWith("http") || - unquoted.includes(".") || - ["&&", "||", "|", ";", ">", "<", ">>", "<<", "&"].includes(unquoted) - ) { - // Stop collecting pattern parts - break - } - patternParts.push(part) - } - - // Return the base command pattern - return patternParts.join(" ") -} - /** * Adds a command pattern to the whitelist */ diff --git a/src/package.json b/src/package.json index 6412b4b858e..79263d8b751 100644 --- a/src/package.json +++ b/src/package.json @@ -407,6 +407,7 @@ "sanitize-filename": "^1.6.3", "say": "^0.16.0", "serialize-error": "^12.0.0", + "shell-quote": "1.8.3", "simple-git": "^3.27.0", "sound-play": "^1.1.0", "stream-json": "^1.8.0", @@ -439,6 +440,7 @@ "@types/node-ipc": "^9.2.3", "@types/proper-lockfile": "^4.1.4", "@types/ps-tree": "^1.1.6", + "@types/shell-quote": "^1.7.5", "@types/stream-json": "^1.7.8", "@types/string-similarity": "^4.0.2", "@types/tmp": "^0.2.6", diff --git a/src/shared/extract-command-pattern.ts b/src/shared/extract-command-pattern.ts new file mode 100644 index 00000000000..6bf16d1d3a1 --- /dev/null +++ b/src/shared/extract-command-pattern.ts @@ -0,0 +1,50 @@ +import { parse } from "shell-quote" + +type ShellToken = string | { op: string } | { command: string } + +/** + * Extract the base command pattern from a full command string. + * For example: "gh pr checkout 1234" -> "gh pr checkout" + * + * Uses shell-quote v1.8.3 for proper shell parsing. + * + * @param command The full command string + * @returns The base command pattern suitable for whitelisting + */ +export function extractCommandPattern(command: string): string { + if (!command?.trim()) return "" + + // Parse the command to get tokens + const tokens = parse(command.trim()) as ShellToken[] + const patternParts: string[] = [] + + for (const token of tokens) { + if (typeof token === "string") { + // Check if this token looks like an argument (number, flag, etc.) + // Common patterns to stop at: + // - Pure numbers (like PR numbers, PIDs, etc.) + // - Flags starting with - or -- + // - File paths or URLs + // - Variable assignments (KEY=VALUE) + if ( + /^\d+$/.test(token) || + token.startsWith("-") || + token.includes("/") || + token.includes("\\") || + token.includes("=") || + token.startsWith("http") || + token.includes(".") + ) { + // Stop collecting pattern parts + break + } + patternParts.push(token) + } else if (typeof token === "object" && "op" in token) { + // Stop at operators + break + } + } + + // Return the base command pattern + return patternParts.join(" ") +} diff --git a/webview-ui/package.json b/webview-ui/package.json index 4c6edc7a2bf..86dd21525d9 100644 --- a/webview-ui/package.json +++ b/webview-ui/package.json @@ -65,7 +65,7 @@ "remark-gfm": "^4.0.1", "remark-math": "^6.0.0", "remove-markdown": "^0.6.0", - "shell-quote": "^1.8.2", + "shell-quote": "^1.8.3", "shiki": "^3.2.1", "source-map": "^0.7.4", "styled-components": "^6.1.13", diff --git a/webview-ui/src/components/chat/ChatView.tsx b/webview-ui/src/components/chat/ChatView.tsx index 5ddce925ca6..dc8c3c7a9ff 100644 --- a/webview-ui/src/components/chat/ChatView.tsx +++ b/webview-ui/src/components/chat/ChatView.tsx @@ -313,7 +313,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction