From 2f321e98ced82519618065e841f13f6f28f8427e Mon Sep 17 00:00:00 2001 From: hannesrudolph Date: Tue, 15 Jul 2025 17:03:57 -0600 Subject: [PATCH] fix: address PR #5491 review feedback - Standardized terminology from 'whitelisted/blacklisted' to 'allowed/denied' across all i18n files - Removed unused _isExpanded and terminalShellIntegrationDisabled variables in CommandExecution.tsx - Added comprehensive JSDoc documentation to complex algorithms in commandPatterns.ts - Consolidated redundant command parsing logic into unified commandUtils module - Updated all imports to use the new centralized utilities - Maintained backward compatibility with re-exports where needed All tests passing after refactoring. --- .../__tests__/webviewMessageHandler.spec.ts | 24 +- src/core/webview/webviewMessageHandler.ts | 6 +- src/i18n/locales/ca/common.json | 2 +- src/i18n/locales/de/common.json | 2 +- src/i18n/locales/en/common.json | 2 +- src/i18n/locales/es/common.json | 2 +- src/i18n/locales/fr/common.json | 2 +- src/i18n/locales/hi/common.json | 2 +- src/i18n/locales/id/common.json | 2 +- src/i18n/locales/it/common.json | 2 +- src/i18n/locales/ja/common.json | 2 +- src/i18n/locales/ko/common.json | 2 +- src/i18n/locales/nl/common.json | 2 +- src/i18n/locales/pl/common.json | 2 +- src/i18n/locales/pt-BR/common.json | 2 +- src/i18n/locales/ru/common.json | 2 +- src/i18n/locales/tr/common.json | 2 +- src/i18n/locales/vi/common.json | 2 +- src/i18n/locales/zh-CN/common.json | 2 +- src/i18n/locales/zh-TW/common.json | 2 +- src/shared/WebviewMessage.ts | 4 +- .../src/components/chat/CommandExecution.tsx | 23 +- .../__tests__/command-validation.spec.ts | 2 +- webview-ui/src/utils/command-validation.ts | 120 +------- webview-ui/src/utils/commandParsing.ts | 63 +--- webview-ui/src/utils/commandPatterns.ts | 138 ++++----- webview-ui/src/utils/commandUtils.ts | 286 ++++++++++++++++++ 27 files changed, 405 insertions(+), 297 deletions(-) create mode 100644 webview-ui/src/utils/commandUtils.ts diff --git a/src/core/webview/__tests__/webviewMessageHandler.spec.ts b/src/core/webview/__tests__/webviewMessageHandler.spec.ts index 0ae4db7423..4d83a0f544 100644 --- a/src/core/webview/__tests__/webviewMessageHandler.spec.ts +++ b/src/core/webview/__tests__/webviewMessageHandler.spec.ts @@ -22,8 +22,8 @@ vi.mock("vscode", () => ({ // Mock i18n vi.mock("../../../i18n", () => ({ t: vi.fn((key: string, params?: any) => { - if (key === "common:info.command_whitelisted" && params?.pattern) { - return `Command pattern "${params.pattern}" has been whitelisted` + if (key === "common:info.command_allowed" && params?.pattern) { + return `Command pattern "${params.pattern}" has been allowed` } return key }), @@ -36,7 +36,7 @@ vi.mock("../../../shared/package", () => ({ }, })) -describe("webviewMessageHandler - whitelistCommand", () => { +describe("webviewMessageHandler - allowCommand", () => { let mockProvider: any let mockContextProxy: any let mockConfigUpdate: any @@ -74,7 +74,7 @@ describe("webviewMessageHandler - whitelistCommand", () => { // Create message const message = { - type: "whitelistCommand", + type: "allowCommand", pattern: "npm run build", } @@ -90,7 +90,7 @@ describe("webviewMessageHandler - whitelistCommand", () => { // Verify user was notified expect(vscode.window.showInformationMessage).toHaveBeenCalledWith( - 'Command pattern "npm run build" has been whitelisted', + 'Command pattern "npm run build" has been allowed', ) // Verify state was posted to webview @@ -103,7 +103,7 @@ describe("webviewMessageHandler - whitelistCommand", () => { // Create message with duplicate pattern const message = { - type: "whitelistCommand", + type: "allowCommand", pattern: "npm run build", } @@ -126,7 +126,7 @@ describe("webviewMessageHandler - whitelistCommand", () => { // Create message const message = { - type: "whitelistCommand", + type: "allowCommand", pattern: "echo 'Hello, World!'", } @@ -138,7 +138,7 @@ describe("webviewMessageHandler - whitelistCommand", () => { // Verify user was notified expect(vscode.window.showInformationMessage).toHaveBeenCalledWith( - `Command pattern "echo 'Hello, World!'" has been whitelisted`, + `Command pattern "echo 'Hello, World!'" has been allowed`, ) }) @@ -148,7 +148,7 @@ describe("webviewMessageHandler - whitelistCommand", () => { // Create message const message = { - type: "whitelistCommand", + type: "allowCommand", pattern: "npm run dev", } @@ -166,7 +166,7 @@ describe("webviewMessageHandler - whitelistCommand", () => { it("should handle missing pattern gracefully", async () => { // Create message without pattern const message = { - type: "whitelistCommand", + type: "allowCommand", } // Call handler @@ -181,7 +181,7 @@ describe("webviewMessageHandler - whitelistCommand", () => { it("should handle non-string pattern gracefully", async () => { // Create message with non-string pattern const message = { - type: "whitelistCommand", + type: "allowCommand", pattern: 123, // Invalid type } @@ -200,7 +200,7 @@ describe("webviewMessageHandler - whitelistCommand", () => { // Create message with complex pattern const message = { - type: "whitelistCommand", + type: "allowCommand", pattern: 'echo "Hello, World!" && echo $HOME', } diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index dc9df9384a..ed7b264d1d 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -771,7 +771,7 @@ export const webviewMessageHandler = async ( break } - case "whitelistCommand": { + case "allowCommand": { // Add a command pattern to the allowed commands list if (message.pattern && typeof message.pattern === "string") { const currentCommands = getGlobalState("allowedCommands") ?? [] @@ -786,9 +786,7 @@ export const webviewMessageHandler = async ( await updateGlobalState("allowedCommands", validCommands) // Show confirmation to the user - vscode.window.showInformationMessage( - t("common:info.command_whitelisted", { pattern: message.pattern }), - ) + vscode.window.showInformationMessage(t("common:info.command_allowed", { pattern: message.pattern })) // Update the webview state await provider.postStateToWebview() diff --git a/src/i18n/locales/ca/common.json b/src/i18n/locales/ca/common.json index 244bd961a3..c2b2c1790d 100644 --- a/src/i18n/locales/ca/common.json +++ b/src/i18n/locales/ca/common.json @@ -107,7 +107,7 @@ "public_share_link_copied": "Enllaç de compartició pública copiat al porta-retalls!", "mode_exported": "Mode '{{mode}}' exportat correctament", "mode_imported": "Mode importat correctament", - "command_whitelisted": "El patró d'ordres '{{pattern}}' s'ha afegit a la llista d'ordres permeses", + "command_allowed": "El patró d'ordres '{{pattern}}' s'ha afegit a la llista d'ordres permeses", "command_denied": "El patró de comanda '{{pattern}}' s'ha afegit a la llista de comandes denegades" }, "answers": { diff --git a/src/i18n/locales/de/common.json b/src/i18n/locales/de/common.json index ff966a465b..6ffc05a4c1 100644 --- a/src/i18n/locales/de/common.json +++ b/src/i18n/locales/de/common.json @@ -103,7 +103,7 @@ "public_share_link_copied": "Öffentlicher Freigabelink in die Zwischenablage kopiert!", "mode_exported": "Modus '{{mode}}' erfolgreich exportiert", "mode_imported": "Modus erfolgreich importiert", - "command_whitelisted": "Befehlsmuster '{{pattern}}' wurde zur Liste der erlaubten Befehle hinzugefügt", + "command_allowed": "Befehlsmuster '{{pattern}}' wurde zur Liste der erlaubten Befehle hinzugefügt", "command_denied": "Das Befehlsmuster '{{pattern}}' wurde zur Liste der verweigerten Befehle hinzugefügt" }, "answers": { diff --git a/src/i18n/locales/en/common.json b/src/i18n/locales/en/common.json index 5d45e98e70..fa3ca9ecc6 100644 --- a/src/i18n/locales/en/common.json +++ b/src/i18n/locales/en/common.json @@ -103,7 +103,7 @@ "image_saved": "Image saved to {{path}}", "mode_exported": "Mode '{{mode}}' exported successfully", "mode_imported": "Mode imported successfully", - "command_whitelisted": "Command pattern '{{pattern}}' has been added to the allowed commands list", + "command_allowed": "Command pattern '{{pattern}}' has been added to the allowed commands list", "command_denied": "Command pattern '{{pattern}}' has been added to the denied commands list" }, "answers": { diff --git a/src/i18n/locales/es/common.json b/src/i18n/locales/es/common.json index d1b27de899..8983ec71dd 100644 --- a/src/i18n/locales/es/common.json +++ b/src/i18n/locales/es/common.json @@ -103,7 +103,7 @@ "public_share_link_copied": "¡Enlace de compartición pública copiado al portapapeles!", "mode_exported": "Modo '{{mode}}' exportado correctamente", "mode_imported": "Modo importado correctamente", - "command_whitelisted": "El patrón de comando '{{pattern}}' se ha añadido a la lista de comandos permitidos", + "command_allowed": "El patrón de comando '{{pattern}}' se ha añadido a la lista de comandos permitidos", "command_denied": "El patrón de comando '{{pattern}}' se ha añadido a la lista de comandos denegados" }, "answers": { diff --git a/src/i18n/locales/fr/common.json b/src/i18n/locales/fr/common.json index 8c788688bd..32cfbdcacf 100644 --- a/src/i18n/locales/fr/common.json +++ b/src/i18n/locales/fr/common.json @@ -103,7 +103,7 @@ "public_share_link_copied": "Lien de partage public copié dans le presse-papiers !", "mode_exported": "Mode '{{mode}}' exporté avec succès", "mode_imported": "Mode importé avec succès", - "command_whitelisted": "Le modèle de commande '{{pattern}}' a été ajouté à la liste des commandes autorisées", + "command_allowed": "Le modèle de commande '{{pattern}}' a été ajouté à la liste des commandes autorisées", "command_denied": "Le modèle de commande '{{pattern}}' a été ajouté à la liste des commandes refusées" }, "answers": { diff --git a/src/i18n/locales/hi/common.json b/src/i18n/locales/hi/common.json index b3e895781b..a1d8ec4701 100644 --- a/src/i18n/locales/hi/common.json +++ b/src/i18n/locales/hi/common.json @@ -103,7 +103,7 @@ "public_share_link_copied": "सार्वजनिक साझाकरण लिंक क्लिपबोर्ड में कॉपी किया गया!", "mode_exported": "मोड '{{mode}}' सफलतापूर्वक निर्यात किया गया", "mode_imported": "मोड सफलतापूर्वक आयात किया गया", - "command_whitelisted": "कमांड पैटर्न '{{pattern}}' को अनुमत कमांड सूची में जोड़ा गया है", + "command_allowed": "कमांड पैटर्न '{{pattern}}' को अनुमत कमांड सूची में जोड़ा गया है", "command_denied": "कमांड पैटर्न '{{pattern}}' को अस्वीकृत कमांड सूची में जोड़ा गया है" }, "answers": { diff --git a/src/i18n/locales/id/common.json b/src/i18n/locales/id/common.json index 297d28d3fc..5ce927dee6 100644 --- a/src/i18n/locales/id/common.json +++ b/src/i18n/locales/id/common.json @@ -103,7 +103,7 @@ "public_share_link_copied": "Tautan berbagi publik disalin ke clipboard!", "mode_exported": "Mode '{{mode}}' berhasil diekspor", "mode_imported": "Mode berhasil diimpor", - "command_whitelisted": "Pola perintah '{{pattern}}' telah ditambahkan ke daftar perintah yang diizinkan", + "command_allowed": "Pola perintah '{{pattern}}' telah ditambahkan ke daftar perintah yang diizinkan", "command_denied": "Pola perintah '{{pattern}}' telah ditambahkan ke daftar perintah yang ditolak" }, "answers": { diff --git a/src/i18n/locales/it/common.json b/src/i18n/locales/it/common.json index e65dddbdfa..266c05f4a3 100644 --- a/src/i18n/locales/it/common.json +++ b/src/i18n/locales/it/common.json @@ -103,7 +103,7 @@ "public_share_link_copied": "Link di condivisione pubblica copiato negli appunti!", "mode_exported": "Modalità '{{mode}}' esportata con successo", "mode_imported": "Modalità importata con successo", - "command_whitelisted": "Il modello di comando '{{pattern}}' è stato aggiunto all'elenco dei comandi consentiti", + "command_allowed": "Il modello di comando '{{pattern}}' è stato aggiunto all'elenco dei comandi consentiti", "command_denied": "Il pattern di comando '{{pattern}}' è stato aggiunto all'elenco dei comandi negati" }, "answers": { diff --git a/src/i18n/locales/ja/common.json b/src/i18n/locales/ja/common.json index c96b21fab1..afb8f73434 100644 --- a/src/i18n/locales/ja/common.json +++ b/src/i18n/locales/ja/common.json @@ -103,7 +103,7 @@ "public_share_link_copied": "公開共有リンクがクリップボードにコピーされました！", "mode_exported": "モード「{{mode}}」が正常にエクスポートされました", "mode_imported": "モードが正常にインポートされました", - "command_whitelisted": "コマンドパターン '{{pattern}}' が許可されたコマンドリストに追加されました", + "command_allowed": "コマンドパターン '{{pattern}}' が許可されたコマンドリストに追加されました", "command_denied": "コマンドパターン '{{pattern}}' が拒否されたコマンドリストに追加されました" }, "answers": { diff --git a/src/i18n/locales/ko/common.json b/src/i18n/locales/ko/common.json index 5013a0d38e..e6e9db8ea3 100644 --- a/src/i18n/locales/ko/common.json +++ b/src/i18n/locales/ko/common.json @@ -103,7 +103,7 @@ "public_share_link_copied": "공개 공유 링크가 클립보드에 복사되었습니다!", "mode_exported": "'{{mode}}' 모드가 성공적으로 내보내졌습니다", "mode_imported": "모드를 성공적으로 가져왔습니다", - "command_whitelisted": "명령 패턴 '{{pattern}}'이(가) 허용된 명령 목록에 추가되었습니다", + "command_allowed": "명령 패턴 '{{pattern}}'이(가) 허용된 명령 목록에 추가되었습니다", "command_denied": "명령 패턴 '{{pattern}}'이(가) 거부된 명령 목록에 추가되었습니다" }, "answers": { diff --git a/src/i18n/locales/nl/common.json b/src/i18n/locales/nl/common.json index 41d35adcce..eef641c77f 100644 --- a/src/i18n/locales/nl/common.json +++ b/src/i18n/locales/nl/common.json @@ -103,7 +103,7 @@ "public_share_link_copied": "Openbare deel-link gekopieerd naar klembord!", "mode_exported": "Modus '{{mode}}' succesvol geëxporteerd", "mode_imported": "Modus succesvol geïmporteerd", - "command_whitelisted": "Commandopatroon '{{pattern}}' is toegevoegd aan de lijst met toegestane commando's", + "command_allowed": "Commandopatroon '{{pattern}}' is toegevoegd aan de lijst met toegestane commando's", "command_denied": "Commandopatroon '{{pattern}}' is toegevoegd aan de lijst met geweigerde commando's" }, "answers": { diff --git a/src/i18n/locales/pl/common.json b/src/i18n/locales/pl/common.json index dc45919573..d5535ead7c 100644 --- a/src/i18n/locales/pl/common.json +++ b/src/i18n/locales/pl/common.json @@ -103,7 +103,7 @@ "public_share_link_copied": "Publiczny link udostępniania skopiowany do schowka!", "mode_exported": "Tryb '{{mode}}' pomyślnie wyeksportowany", "mode_imported": "Tryb pomyślnie zaimportowany", - "command_whitelisted": "Wzór polecenia '{{pattern}}' został dodany do listy dozwolonych poleceń", + "command_allowed": "Wzór polecenia '{{pattern}}' został dodany do listy dozwolonych poleceń", "command_denied": "Wzór polecenia '{{pattern}}' został dodany do listy odrzuconych poleceń" }, "answers": { diff --git a/src/i18n/locales/pt-BR/common.json b/src/i18n/locales/pt-BR/common.json index 3c90f05188..8d60c7aaa3 100644 --- a/src/i18n/locales/pt-BR/common.json +++ b/src/i18n/locales/pt-BR/common.json @@ -107,7 +107,7 @@ "public_share_link_copied": "Link de compartilhamento público copiado para a área de transferência!", "mode_exported": "Modo '{{mode}}' exportado com sucesso", "mode_imported": "Modo importado com sucesso", - "command_whitelisted": "O padrão de comando '{{pattern}}' foi adicionado à lista de comandos permitidos", + "command_allowed": "O padrão de comando '{{pattern}}' foi adicionado à lista de comandos permitidos", "command_denied": "O padrão de comando '{{pattern}}' foi adicionado à lista de comandos negados" }, "answers": { diff --git a/src/i18n/locales/ru/common.json b/src/i18n/locales/ru/common.json index b396b1737f..f0d0f1bab5 100644 --- a/src/i18n/locales/ru/common.json +++ b/src/i18n/locales/ru/common.json @@ -103,7 +103,7 @@ "public_share_link_copied": "Публичная ссылка для совместного доступа скопирована в буфер обмена!", "mode_exported": "Режим '{{mode}}' успешно экспортирован", "mode_imported": "Режим успешно импортирован", - "command_whitelisted": "Шаблон команды '{{pattern}}' добавлен в список разрешенных команд", + "command_allowed": "Шаблон команды '{{pattern}}' добавлен в список разрешенных команд", "command_denied": "Шаблон команды '{{pattern}}' добавлен в список запрещенных команд" }, "answers": { diff --git a/src/i18n/locales/tr/common.json b/src/i18n/locales/tr/common.json index a395525342..2eaaeebb57 100644 --- a/src/i18n/locales/tr/common.json +++ b/src/i18n/locales/tr/common.json @@ -103,7 +103,7 @@ "public_share_link_copied": "Herkese açık paylaşım bağlantısı panoya kopyalandı!", "mode_exported": "'{{mode}}' modu başarıyla dışa aktarıldı", "mode_imported": "Mod başarıyla içe aktarıldı", - "command_whitelisted": "'{{pattern}}' komut deseni izin verilen komutlar listesine eklendi", + "command_allowed": "'{{pattern}}' komut deseni izin verilen komutlar listesine eklendi", "command_denied": "'{{pattern}}' komut deseni reddedilen komutlar listesine eklendi" }, "answers": { diff --git a/src/i18n/locales/vi/common.json b/src/i18n/locales/vi/common.json index a0bb311542..1419c624df 100644 --- a/src/i18n/locales/vi/common.json +++ b/src/i18n/locales/vi/common.json @@ -103,7 +103,7 @@ "public_share_link_copied": "Liên kết chia sẻ công khai đã được sao chép vào clipboard!", "mode_exported": "Chế độ '{{mode}}' đã được xuất thành công", "mode_imported": "Chế độ đã được nhập thành công", - "command_whitelisted": "Mẫu lệnh '{{pattern}}' đã được thêm vào danh sách lệnh được phép", + "command_allowed": "Mẫu lệnh '{{pattern}}' đã được thêm vào danh sách lệnh được phép", "command_denied": "Mẫu lệnh '{{pattern}}' đã được thêm vào danh sách lệnh bị từ chối" }, "answers": { diff --git a/src/i18n/locales/zh-CN/common.json b/src/i18n/locales/zh-CN/common.json index 1aa7a17660..00a8d71450 100644 --- a/src/i18n/locales/zh-CN/common.json +++ b/src/i18n/locales/zh-CN/common.json @@ -108,7 +108,7 @@ "public_share_link_copied": "公开分享链接已复制到剪贴板！", "mode_exported": "模式 '{{mode}}' 已成功导出", "mode_imported": "模式已成功导入", - "command_whitelisted": "命令模式 '{{pattern}}' 已添加到允许的命令列表中", + "command_allowed": "命令模式 '{{pattern}}' 已添加到允许的命令列表中", "command_denied": "命令模式 '{{pattern}}' 已被添加到拒绝命令列表中" }, "answers": { diff --git a/src/i18n/locales/zh-TW/common.json b/src/i18n/locales/zh-TW/common.json index ed2af3f8c1..00ff592210 100644 --- a/src/i18n/locales/zh-TW/common.json +++ b/src/i18n/locales/zh-TW/common.json @@ -103,7 +103,7 @@ "public_share_link_copied": "公開分享連結已複製到剪貼簿！", "mode_exported": "模式 '{{mode}}' 已成功匯出", "mode_imported": "模式已成功匯入", - "command_whitelisted": "命令模式 '{{pattern}}' 已新增至允許的命令清單中", + "command_allowed": "命令模式 '{{pattern}}' 已新增至允許的命令清單中", "command_denied": "命令模式 '{{pattern}}' 已新增至拒絕的命令清單中" }, "answers": { diff --git a/src/shared/WebviewMessage.ts b/src/shared/WebviewMessage.ts index d884c30770..40ed7a8c5f 100644 --- a/src/shared/WebviewMessage.ts +++ b/src/shared/WebviewMessage.ts @@ -36,7 +36,7 @@ export interface WebviewMessage { | "getListApiConfiguration" | "customInstructions" | "allowedCommands" - | "whitelistCommand" + | "allowCommand" | "deniedCommands" | "denyCommand" | "alwaysAllowReadOnly" @@ -237,7 +237,7 @@ export interface WebviewMessage { visibility?: ShareVisibility // For share visibility hasContent?: boolean // For checkRulesDirectoryResult checkOnly?: boolean // For deleteCustomMode check - pattern?: string // For whitelistCommand + pattern?: string // For allowCommand codeIndexSettings?: { // Global state settings codebaseIndexEnabled: boolean diff --git a/webview-ui/src/components/chat/CommandExecution.tsx b/webview-ui/src/components/chat/CommandExecution.tsx index 664b13fd88..5c8e3a4dfb 100644 --- a/webview-ui/src/components/chat/CommandExecution.tsx +++ b/webview-ui/src/components/chat/CommandExecution.tsx @@ -26,19 +26,12 @@ interface CommandExecutionProps { export const CommandExecution = ({ executionId, text, icon, title }: CommandExecutionProps) => { const { t } = useAppTranslation() - const { - terminalShellIntegrationDisabled = false, - allowedCommands = [], - deniedCommands = [], - setAllowedCommands, - setDeniedCommands, - } = useExtensionState() + const { allowedCommands = [], deniedCommands = [], setAllowedCommands, setDeniedCommands } = useExtensionState() const { command, output: parsedOutput, suggestions } = useMemo(() => parseCommandAndOutput(text), [text]) - // If we aren't opening the VSCode terminal for this command then we default - // to expanding the command execution output. - const [_isExpanded, setIsExpanded] = useState(terminalShellIntegrationDisabled) + // Note: isExpanded state removed as it was unused. The setIsExpanded in fallback case + // now directly sets isOutputExpanded instead. const [streamingOutput, setStreamingOutput] = useState("") const [status, setStatus] = useState(null) // Separate state for output expansion - default to closed @@ -98,7 +91,7 @@ export const CommandExecution = ({ executionId, text, icon, title }: CommandExec setStreamingOutput(data.output) break case "fallback": - setIsExpanded(true) + setIsOutputExpanded(true) break default: setStatus(data) @@ -116,10 +109,10 @@ export const CommandExecution = ({ executionId, text, icon, title }: CommandExec (pattern: string) => { if (!pattern) return - const isWhitelisted = allowedCommands.includes(pattern) + const isAllowed = allowedCommands.includes(pattern) - if (isWhitelisted) { - // Remove from whitelist + if (isAllowed) { + // Remove from allowed list const updatedAllowedCommands = allowedCommands.filter((p) => p !== pattern) setAllowedCommands(updatedAllowedCommands) vscode.postMessage({ @@ -127,7 +120,7 @@ export const CommandExecution = ({ executionId, text, icon, title }: CommandExec commands: updatedAllowedCommands, }) } else { - // Add to whitelist + // Add to allowed list const updatedAllowedCommands = [...allowedCommands, pattern] setAllowedCommands(updatedAllowedCommands) vscode.postMessage({ diff --git a/webview-ui/src/utils/__tests__/command-validation.spec.ts b/webview-ui/src/utils/__tests__/command-validation.spec.ts index e926561682..d69698cfb8 100644 --- a/webview-ui/src/utils/__tests__/command-validation.spec.ts +++ b/webview-ui/src/utils/__tests__/command-validation.spec.ts @@ -3,7 +3,6 @@ // npx vitest src/utils/__tests__/command-validation.spec.ts import { - parseCommand, isAutoApprovedSingleCommand, isAutoDeniedSingleCommand, isAutoApprovedCommand, @@ -14,6 +13,7 @@ import { CommandValidator, createCommandValidator, } from "../command-validation" +import { parseCommand } from "../commandUtils" describe("Command Validation", () => { describe("parseCommand", () => { diff --git a/webview-ui/src/utils/command-validation.ts b/webview-ui/src/utils/command-validation.ts index 1243cd1ee2..059314f8f3 100644 --- a/webview-ui/src/utils/command-validation.ts +++ b/webview-ui/src/utils/command-validation.ts @@ -1,6 +1,7 @@ -import { parse } from "shell-quote" +import { parseCommand, hasSubshellExpressions, removeRedirections } from "./commandUtils" -type ShellToken = string | { op: string } | { command: string } +// Re-export parseCommand for backward compatibility +export { parseCommand } /** * # Command Denylist Feature - Longest Prefix Match Strategy @@ -58,103 +59,6 @@ type ShellToken = string | { op: string } | { command: string } * This allows users to have personal defaults while projects can define specific restrictions. */ -/** - * Split a command string into individual sub-commands by - * chaining operators (&&, ||, ;, or |). - * - * Uses shell-quote to properly handle: - * - Quoted strings (preserves quotes) - * - Subshell commands ($(cmd) or `cmd`) - * - PowerShell redirections (2>&1) - * - Chain operators (&&, ||, ;, |) - */ -export function parseCommand(command: string): string[] { - if (!command?.trim()) return [] - - // Storage for replaced content - const redirections: string[] = [] - const subshells: string[] = [] - const quotes: string[] = [] - const arrayIndexing: string[] = [] - - // First handle PowerShell redirections by temporarily replacing them - let processedCommand = command.replace(/\d*>&\d*/g, (match) => { - redirections.push(match) - return `__REDIR_${redirections.length - 1}__` - }) - - // Handle array indexing expressions: ${array[...]} pattern and partial expressions - processedCommand = processedCommand.replace(/\$\{[^}]*\[[^\]]*(\]([^}]*\})?)?/g, (match) => { - arrayIndexing.push(match) - return `__ARRAY_${arrayIndexing.length - 1}__` - }) - - // Then handle subshell commands - processedCommand = processedCommand - .replace(/\$\((.*?)\)/g, (_, inner) => { - subshells.push(inner.trim()) - return `__SUBSH_${subshells.length - 1}__` - }) - .replace(/`(.*?)`/g, (_, inner) => { - subshells.push(inner.trim()) - return `__SUBSH_${subshells.length - 1}__` - }) - - // Then handle quoted strings - processedCommand = processedCommand.replace(/"[^"]*"/g, (match) => { - quotes.push(match) - return `__QUOTE_${quotes.length - 1}__` - }) - - const tokens = parse(processedCommand) as ShellToken[] - const commands: string[] = [] - let currentCommand: string[] = [] - - for (const token of tokens) { - if (typeof token === "object" && "op" in token) { - // Chain operator - split command - if (["&&", "||", ";", "|"].includes(token.op)) { - if (currentCommand.length > 0) { - commands.push(currentCommand.join(" ")) - currentCommand = [] - } - } else { - // Other operators (>, &) are part of the command - currentCommand.push(token.op) - } - } else if (typeof token === "string") { - // Check if it's a subshell placeholder - const subshellMatch = token.match(/__SUBSH_(\d+)__/) - if (subshellMatch) { - if (currentCommand.length > 0) { - commands.push(currentCommand.join(" ")) - currentCommand = [] - } - commands.push(subshells[parseInt(subshellMatch[1])]) - } else { - currentCommand.push(token) - } - } - } - - // Add any remaining command - if (currentCommand.length > 0) { - commands.push(currentCommand.join(" ")) - } - - // Restore quotes and redirections - return commands.map((cmd) => { - let result = cmd - // Restore quotes - result = result.replace(/__QUOTE_(\d+)__/g, (_, i) => quotes[parseInt(i)]) - // Restore redirections - result = result.replace(/__REDIR_(\d+)__/g, (_, i) => redirections[parseInt(i)]) - // Restore array indexing expressions - result = result.replace(/__ARRAY_(\d+)__/g, (_, i) => arrayIndexing[parseInt(i)]) - return result - }) -} - /** * Find the longest matching prefix from a list of prefixes for a given command. * @@ -288,7 +192,7 @@ export function isAutoApprovedCommand(command: string, allowedCommands: string[] if (!command?.trim()) return true // Only block subshell execution attempts if there's a denylist configured - if ((command.includes("$(") || command.includes("`")) && deniedCommands?.length) { + if (hasSubshellExpressions(command) && deniedCommands?.length) { return false } @@ -298,7 +202,7 @@ export function isAutoApprovedCommand(command: string, allowedCommands: string[] // Ensure every sub-command is auto-approved return subCommands.every((cmd) => { // Remove simple PowerShell-like redirections (e.g. 2>&1) before checking - const cmdWithoutRedirection = cmd.replace(/\d*>&\d*/, "").trim() + const cmdWithoutRedirection = removeRedirections(cmd) return isAutoApprovedSingleCommand(cmdWithoutRedirection, allowedCommands, deniedCommands) }) @@ -313,7 +217,7 @@ export function isAutoDeniedCommand(command: string, allowedCommands: string[], if (!command?.trim()) return false // Only block subshell execution attempts if there's a denylist configured - if ((command.includes("$(") || command.includes("`")) && deniedCommands?.length) { + if (hasSubshellExpressions(command) && deniedCommands?.length) { return true } @@ -323,7 +227,7 @@ export function isAutoDeniedCommand(command: string, allowedCommands: string[], // Auto-deny if any sub-command is auto-denied return subCommands.some((cmd) => { // Remove simple PowerShell-like redirections (e.g. 2>&1) before checking - const cmdWithoutRedirection = cmd.replace(/\d*>&\d*/, "").trim() + const cmdWithoutRedirection = removeRedirections(cmd) return isAutoDeniedSingleCommand(cmdWithoutRedirection, allowedCommands, deniedCommands) }) @@ -385,7 +289,7 @@ export function getCommandDecision( if (!command?.trim()) return "auto_approve" // Only block subshell execution attempts if there's a denylist configured - if ((command.includes("$(") || command.includes("`")) && deniedCommands?.length) { + if (hasSubshellExpressions(command) && deniedCommands?.length) { return "auto_deny" } @@ -395,7 +299,7 @@ export function getCommandDecision( // Check each sub-command and collect decisions const decisions: CommandDecision[] = subCommands.map((cmd) => { // Remove simple PowerShell-like redirections (e.g. 2>&1) before checking - const cmdWithoutRedirection = cmd.replace(/\d*>&\d*/, "").trim() + const cmdWithoutRedirection = removeRedirections(cmd) return getSingleCommandDecision(cmdWithoutRedirection, allowedCommands, deniedCommands) }) @@ -561,16 +465,16 @@ export class CommandValidator { hasSubshells: boolean } { const subCommands = parseCommand(command) - const hasSubshells = command.includes("$(") || command.includes("`") + const hasSubshells = hasSubshellExpressions(command) const allowedMatches = subCommands.map((cmd) => ({ command: cmd, - match: findLongestPrefixMatch(cmd.replace(/\d*>&\d*/, "").trim(), this.allowedCommands), + match: findLongestPrefixMatch(removeRedirections(cmd), this.allowedCommands), })) const deniedMatches = subCommands.map((cmd) => ({ command: cmd, - match: findLongestPrefixMatch(cmd.replace(/\d*>&\d*/, "").trim(), this.deniedCommands || []), + match: findLongestPrefixMatch(removeRedirections(cmd), this.deniedCommands || []), })) return { diff --git a/webview-ui/src/utils/commandParsing.ts b/webview-ui/src/utils/commandParsing.ts index 4d0b4351c9..347715a16d 100644 --- a/webview-ui/src/utils/commandParsing.ts +++ b/webview-ui/src/utils/commandParsing.ts @@ -1,61 +1,2 @@ -// Define the constant locally since it's a simple string -const COMMAND_OUTPUT_STRING = "Output:" - -export interface ParsedCommand { - command: string - output: string - suggestions: string[] -} - -/** - * Parses command text to extract the command, output, and suggestions. - * Supports both JSON array format and individual tags. - */ -export const parseCommandAndOutput = (text: string | undefined): ParsedCommand => { - if (!text) { - return { command: "", output: "", suggestions: [] } - } - - // First, extract suggestions from the text - const suggestions: string[] = [] - - // Parse tag with JSON array - const suggestionsMatch = text.match(/([\s\S]*?)<\/suggestions>/) - if (suggestionsMatch) { - try { - const parsed = JSON.parse(suggestionsMatch[1]) - if (Array.isArray(parsed)) { - suggestions.push(...parsed.filter((s: any) => typeof s === "string" && s.trim())) - } - } catch { - // Invalid JSON, ignore - } - // Remove the suggestions tag from text - text = text.replace(/[\s\S]*?<\/suggestions>/, "") - } - - // Parse individual tags - let suggestMatch - const suggestRegex = /([\s\S]*?)<\/suggest>/g - while ((suggestMatch = suggestRegex.exec(text)) !== null) { - const suggestion = suggestMatch[1].trim() - if (suggestion) { - suggestions.push(suggestion) - } - } - // Remove all suggest tags from text - text = text.replace(/[\s\S]*?<\/suggest>/g, "") - - // Now parse command and output - const index = text.indexOf(COMMAND_OUTPUT_STRING) - - if (index === -1) { - return { command: text.trim(), output: "", suggestions } - } - - return { - command: text.slice(0, index).trim(), - output: text.slice(index + COMMAND_OUTPUT_STRING.length), - suggestions, - } -} +// Re-export from the unified commandUtils module +export { parseCommandAndOutput, type ParsedCommand } from "./commandUtils" diff --git a/webview-ui/src/utils/commandPatterns.ts b/webview-ui/src/utils/commandPatterns.ts index 6a0cfb02dc..9259233eff 100644 --- a/webview-ui/src/utils/commandPatterns.ts +++ b/webview-ui/src/utils/commandPatterns.ts @@ -1,9 +1,10 @@ import { parse } from "shell-quote" +import { parseCommand } from "./commandUtils" /** * Extracts command patterns from a command string using shell-quote parser. * This provides a robust, deterministic way to extract patterns that can be - * used for whitelisting similar commands. + * used for allowing similar commands. * * @param command The full command string to extract patterns from * @returns Array of unique command patterns sorted alphabetically @@ -13,9 +14,8 @@ export function extractCommandPatterns(command: string): string[] { const patterns = new Set() - // Handle command chains (&&, ||, ;, |) - const chainOperators = ["&&", "||", ";", "|"] - const commands = splitByOperators(command, chainOperators) + // Handle command chains (&&, ||, ;, |) using the unified parseCommand function + const commands = parseCommand(command) for (const cmd of commands) { const cmdPatterns = extractSingleCommandPattern(cmd.trim()) @@ -30,80 +30,66 @@ export function extractCommandPatterns(command: string): string[] { return Array.from(patterns).sort() } -/** - * Split command by operators while respecting shell syntax - */ -function splitByOperators(command: string, operators: string[]): string[] { - const commands: string[] = [] - let current = "" - let inSingleQuote = false - let inDoubleQuote = false - let escapeNext = false - - for (let i = 0; i < command.length; i++) { - const char = command[i] - - if (escapeNext) { - current += char - escapeNext = false - continue - } - - if (char === "\\") { - escapeNext = true - current += char - continue - } - - if (char === "'" && !inDoubleQuote) { - inSingleQuote = !inSingleQuote - current += char - continue - } - - if (char === '"' && !inSingleQuote) { - inDoubleQuote = !inDoubleQuote - current += char - continue - } - - // Check for operators outside quotes - if (!inSingleQuote && !inDoubleQuote) { - let foundOperator = false - for (const op of operators) { - if (command.substring(i, i + op.length) === op) { - // Found an operator, save current command - if (current.trim()) { - commands.push(current.trim()) - } - current = "" - i += op.length - 1 // -1 because the loop will increment - foundOperator = true - break - } - } - if (foundOperator) continue - } - - current += char - } - - // Don't forget the last command - if (current.trim()) { - commands.push(current.trim()) - } - - // If no commands were found, return the whole command - if (commands.length === 0) { - commands.push(command) - } - - return commands -} - /** * Extract patterns from a single command (not chained) - * Returns an array of patterns instead of a single pattern + * + * This function implements a sophisticated pattern extraction algorithm that: + * 1. Parses the command using shell-quote for accurate tokenization + * 2. Identifies the base command and relevant subcommands + * 3. Generates progressively more specific patterns + * 4. Handles special cases for common tools (npm, git, docker, etc.) + * + * ## Pattern Extraction Strategy: + * + * The algorithm generates multiple patterns from least to most specific: + * - Base command only (e.g., "git") + * - Command + subcommand (e.g., "git push") + * - Stops at flags, paths, or complex arguments + * + * ## Special Command Handling: + * + * **Package Managers (npm, yarn, pnpm, bun):** + * - Extracts base command and subcommand + * - Special handling for "run" to allow any script + * - Example: "npm install" → ["npm", "npm install"] + * + * **Version Control (git):** + * - Extracts git + subcommand only + * - Example: "git push origin main" → ["git", "git push"] + * + * **Container/Orchestration (docker, kubectl, helm):** + * - Similar to git, extracts command + subcommand + * - Example: "docker build -t app ." → ["docker", "docker build"] + * + * **Interpreters (python, node, ruby, etc.):** + * - Only extracts the interpreter name + * - Example: "python script.py --arg" → ["python"] + * + * **Dangerous Commands (rm, mv, chmod, etc.):** + * - Only extracts the base command for safety + * - Example: "rm -rf /tmp/*" → ["rm"] + * + * **Script Files:** + * - If command is a path or has script extension, returns as-is + * - Example: "./deploy.sh" → ["./deploy.sh"] + * + * ## Examples: + * ```typescript + * extractSingleCommandPattern("npm install express") + * // Returns: ["npm", "npm install"] + * + * extractSingleCommandPattern("git push --force origin main") + * // Returns: ["git", "git push"] + * + * extractSingleCommandPattern("rm -rf node_modules") + * // Returns: ["rm"] + * + * extractSingleCommandPattern("./scripts/build.sh --prod") + * // Returns: ["./scripts/build.sh"] + * ``` + * + * @param command - Single command string to extract patterns from + * @returns Array of patterns from least to most specific */ function extractSingleCommandPattern(command: string): string[] { if (!command) return [] diff --git a/webview-ui/src/utils/commandUtils.ts b/webview-ui/src/utils/commandUtils.ts new file mode 100644 index 0000000000..13d7abba82 --- /dev/null +++ b/webview-ui/src/utils/commandUtils.ts @@ -0,0 +1,286 @@ +import { parse } from "shell-quote" + +type ShellToken = string | { op: string } | { command: string } + +/** + * # Unified Command Utilities Module + * + * This module consolidates all command parsing and manipulation utilities + * that were previously scattered across multiple files. It provides a single + * source of truth for command-related operations. + * + * ## Key Features: + * - Command splitting by shell operators (&&, ||, ;, |) + * - Proper handling of quoted strings and escape sequences + * - Subshell command detection and handling + * - Command output parsing with suggestion extraction + * - Pattern extraction from commands + * + * ## Migration Notes: + * - `parseCommand` replaces both `parseCommand` from command-validation.ts + * and `splitByOperators` from commandPatterns.ts + * - `parseCommandAndOutput` moved from commandParsing.ts + * - All command-related utilities are now centralized here + */ + +/** + * Split a command string into individual sub-commands by + * chaining operators (&&, ||, ;, or |). + * + * This is the unified implementation that replaces both: + * - `parseCommand` from command-validation.ts + * - `splitByOperators` from commandPatterns.ts + * + * Uses shell-quote to properly handle: + * - Quoted strings (preserves quotes) + * - Subshell commands ($(cmd) or `cmd`) + * - PowerShell redirections (2>&1) + * - Chain operators (&&, ||, ;, |) + * - Array indexing expressions (${array[...]}) + * + * @param command - The command string to split + * @returns Array of individual commands with operators removed + */ +export function parseCommand(command: string): string[] { + if (!command?.trim()) return [] + + // Storage for replaced content + const redirections: string[] = [] + const subshells: string[] = [] + const quotes: string[] = [] + const arrayIndexing: string[] = [] + + // First handle PowerShell redirections by temporarily replacing them + let processedCommand = command.replace(/\d*>&\d*/g, (match) => { + redirections.push(match) + return `__REDIR_${redirections.length - 1}__` + }) + + // Handle array indexing expressions: ${array[...]} pattern and partial expressions + processedCommand = processedCommand.replace(/\$\{[^}]*\[[^\]]*(\]([^}]*\})?)?/g, (match) => { + arrayIndexing.push(match) + return `__ARRAY_${arrayIndexing.length - 1}__` + }) + + // Then handle subshell commands + processedCommand = processedCommand + .replace(/\$\((.*?)\)/g, (_, inner) => { + subshells.push(inner.trim()) + return `__SUBSH_${subshells.length - 1}__` + }) + .replace(/`(.*?)`/g, (_, inner) => { + subshells.push(inner.trim()) + return `__SUBSH_${subshells.length - 1}__` + }) + + // Then handle quoted strings + processedCommand = processedCommand.replace(/"[^"]*"/g, (match) => { + quotes.push(match) + return `__QUOTE_${quotes.length - 1}__` + }) + + const tokens = parse(processedCommand) as ShellToken[] + const commands: string[] = [] + let currentCommand: string[] = [] + + for (const token of tokens) { + if (typeof token === "object" && "op" in token) { + // Chain operator - split command + if (["&&", "||", ";", "|"].includes(token.op)) { + if (currentCommand.length > 0) { + commands.push(currentCommand.join(" ")) + currentCommand = [] + } + } else { + // Other operators (>, &) are part of the command + currentCommand.push(token.op) + } + } else if (typeof token === "string") { + // Check if it's a subshell placeholder + const subshellMatch = token.match(/__SUBSH_(\d+)__/) + if (subshellMatch) { + if (currentCommand.length > 0) { + commands.push(currentCommand.join(" ")) + currentCommand = [] + } + commands.push(subshells[parseInt(subshellMatch[1])]) + } else { + currentCommand.push(token) + } + } + } + + // Add any remaining command + if (currentCommand.length > 0) { + commands.push(currentCommand.join(" ")) + } + + // Restore quotes and redirections + return commands.map((cmd) => { + let result = cmd + // Restore quotes + result = result.replace(/__QUOTE_(\d+)__/g, (_, i) => quotes[parseInt(i)]) + // Restore redirections + result = result.replace(/__REDIR_(\d+)__/g, (_, i) => redirections[parseInt(i)]) + // Restore array indexing expressions + result = result.replace(/__ARRAY_(\d+)__/g, (_, i) => arrayIndexing[parseInt(i)]) + return result + }) +} + +/** + * Legacy alias for parseCommand to maintain backward compatibility + * @deprecated Use parseCommand instead + */ +export const splitByOperators = (command: string, _operators?: string[]): string[] => { + console.warn("splitByOperators is deprecated. Use parseCommand instead.") + return parseCommand(command) +} + +/** + * Check if a command contains subshell expressions + * @param command - The command to check + * @returns True if the command contains $() or `` subshell syntax + */ +export function hasSubshellExpressions(command: string): boolean { + return command.includes("$(") || command.includes("`") +} + +/** + * Remove PowerShell-style redirections from a command + * @param command - The command to clean + * @returns Command with redirections removed + */ +export function removeRedirections(command: string): string { + return command.replace(/\d*>&\d*/g, "").trim() +} + +// Define the constant locally since it's a simple string +const COMMAND_OUTPUT_STRING = "Output:" + +export interface ParsedCommand { + command: string + output: string + suggestions: string[] +} + +/** + * Parses command text to extract the command, output, and suggestions. + * Supports both JSON array format and individual tags. + * + * @param text - The text containing command, output, and suggestions + * @returns Parsed command data with command, output, and suggestions array + */ +export const parseCommandAndOutput = (text: string | undefined): ParsedCommand => { + if (!text) { + return { command: "", output: "", suggestions: [] } + } + + // First, extract suggestions from the text + const suggestions: string[] = [] + + // Parse tag with JSON array + const suggestionsMatch = text.match(/([\s\S]*?)<\/suggestions>/) + if (suggestionsMatch) { + try { + const parsed = JSON.parse(suggestionsMatch[1]) + if (Array.isArray(parsed)) { + suggestions.push(...parsed.filter((s: any) => typeof s === "string" && s.trim())) + } + } catch { + // Invalid JSON, ignore + } + // Remove the suggestions tag from text + text = text.replace(/[\s\S]*?<\/suggestions>/, "") + } + + // Parse individual tags + let suggestMatch + const suggestRegex = /([\s\S]*?)<\/suggest>/g + while ((suggestMatch = suggestRegex.exec(text)) !== null) { + const suggestion = suggestMatch[1].trim() + if (suggestion) { + suggestions.push(suggestion) + } + } + // Remove all suggest tags from text + text = text.replace(/[\s\S]*?<\/suggest>/g, "") + + // Now parse command and output + const index = text.indexOf(COMMAND_OUTPUT_STRING) + + if (index === -1) { + return { command: text.trim(), output: "", suggestions } + } + + return { + command: text.slice(0, index).trim(), + output: text.slice(index + COMMAND_OUTPUT_STRING.length), + suggestions, + } +} + +/** + * Extract the base command from a full command string + * (e.g., "git push origin main" -> "git") + * + * @param command - The full command string + * @returns The base command + */ +export function extractBaseCommand(command: string): string { + const trimmed = command.trim() + const spaceIndex = trimmed.indexOf(" ") + return spaceIndex === -1 ? trimmed : trimmed.substring(0, spaceIndex) +} + +/** + * Check if a command matches a pattern (case-insensitive prefix match) + * + * @param command - The command to check + * @param pattern - The pattern to match against + * @returns True if the command starts with the pattern + */ +export function commandMatchesPattern(command: string, pattern: string): boolean { + return command.trim().toLowerCase().startsWith(pattern.toLowerCase()) +} + +/** + * Normalize a command by trimming whitespace and converting to lowercase + * Useful for consistent command comparison + * + * @param command - The command to normalize + * @returns Normalized command string + */ +export function normalizeCommand(command: string): string { + return command.trim().toLowerCase() +} + +/** + * Get all subcommands from a command string, including those in subshells + * + * @param command - The command to analyze + * @returns Array of all commands including subshell commands + */ +export function getAllSubcommands(command: string): string[] { + const mainCommands = parseCommand(command) + const allCommands: string[] = [] + + for (const cmd of mainCommands) { + allCommands.push(cmd) + + // Extract subshell commands using regex exec + const patterns = [/\$\((.*?)\)/g, /`(.*?)`/g] + + for (const pattern of patterns) { + let match + while ((match = pattern.exec(cmd)) !== null) { + if (match[1]) { + // Recursively get subcommands from the subshell + allCommands.push(...getAllSubcommands(match[1])) + } + } + } + } + + return allCommands +}