diff --git a/.changeset/large-crews-hunt.md b/.changeset/large-crews-hunt.md new file mode 100644 index 0000000..5bf3c6d --- /dev/null +++ b/.changeset/large-crews-hunt.md @@ -0,0 +1,5 @@ +--- +"roo-cline": patch +--- + +Improved regex for auto-execution of chained commands diff --git a/webview-ui/src/components/chat/ChatView.tsx b/webview-ui/src/components/chat/ChatView.tsx index 13c0fa8..fcbbf5b 100644 --- a/webview-ui/src/components/chat/ChatView.tsx +++ b/webview-ui/src/components/chat/ChatView.tsx @@ -523,7 +523,7 @@ const ChatView = ({ isHidden, showAnnouncement, hideAnnouncement, showHistoryVie } // Split command by chaining operators - const commands = command.split(/&&|\|\||;|\||\$\(|`/).map(cmd => cmd.trim()) + const commands = command.split(/&&|\|\||;|(? cmd.trim()) // Check if all individual commands are allowed return commands.every((cmd) => { diff --git a/webview-ui/src/components/chat/__tests__/ChatView.test.tsx b/webview-ui/src/components/chat/__tests__/ChatView.test.tsx index 9e2d2c4..63c1ac6 100644 --- a/webview-ui/src/components/chat/__tests__/ChatView.test.tsx +++ b/webview-ui/src/components/chat/__tests__/ChatView.test.tsx @@ -415,7 +415,7 @@ describe('ChatView - Auto Approval Tests', () => { it('auto-approves chained commands when all parts are allowed', async () => { render( - {}} @@ -429,7 +429,12 @@ describe('ChatView - Auto Approval Tests', () => { 'npm test && npm run build', 'npm test; npm run build', 'npm test || npm run build', - 'npm test | npm run build' + 'npm test | npm run build', + // Add test for quoted pipes which should be treated as part of the command, not as a chain operator + 'echo "hello | world"', + 'npm test "param with | inside" && npm run build', + // PowerShell command with Select-String + 'npm test 2>&1 | Select-String -NotMatch "node_modules" | Select-String "FAIL|Error"' ] for (const command of allowedChainedCommands) { @@ -438,7 +443,7 @@ describe('ChatView - Auto Approval Tests', () => { // First hydrate state with initial task mockPostMessage({ alwaysAllowExecute: true, - allowedCommands: ['npm test', 'npm run build'], + allowedCommands: ['npm test', 'npm run build', 'echo', 'Select-String'], clineMessages: [ { type: 'say', @@ -452,7 +457,7 @@ describe('ChatView - Auto Approval Tests', () => { // Then send the chained command ask message mockPostMessage({ alwaysAllowExecute: true, - allowedCommands: ['npm test', 'npm run build'], + allowedCommands: ['npm test', 'npm run build', 'echo', 'Select-String'], clineMessages: [ { type: 'say', @@ -483,7 +488,7 @@ describe('ChatView - Auto Approval Tests', () => { it('does not auto-approve chained commands when any part is disallowed', () => { render( - {}} @@ -498,15 +503,20 @@ describe('ChatView - Auto Approval Tests', () => { 'npm test; rm -rf /', 'npm test || rm -rf /', 'npm test | rm -rf /', + // Test subshell execution using $() and backticks 'npm test $(echo dangerous)', - 'npm test `echo dangerous`' + 'npm test `echo dangerous`', + // Test unquoted pipes with disallowed commands + 'npm test | rm -rf /', + // Test PowerShell command with disallowed parts + 'npm test 2>&1 | Select-String -NotMatch "node_modules" | rm -rf /' ] disallowedChainedCommands.forEach(command => { // First hydrate state with initial task mockPostMessage({ alwaysAllowExecute: true, - allowedCommands: ['npm test'], + allowedCommands: ['npm test', 'Select-String'], clineMessages: [ { type: 'say', @@ -520,7 +530,7 @@ describe('ChatView - Auto Approval Tests', () => { // Then send the chained command ask message mockPostMessage({ alwaysAllowExecute: true, - allowedCommands: ['npm test'], + allowedCommands: ['npm test', 'Select-String'], clineMessages: [ { type: 'say', @@ -545,6 +555,121 @@ describe('ChatView - Auto Approval Tests', () => { }) }) }) + + it('handles complex PowerShell command chains correctly', async () => { + render( + + {}} + showHistoryView={() => {}} + /> + + ) + + // Test PowerShell specific command chains + const powershellCommands = { + allowed: [ + 'npm test 2>&1 | Select-String -NotMatch "node_modules"', + 'npm test 2>&1 | Select-String "FAIL|Error"', + 'npm test 2>&1 | Select-String -NotMatch "node_modules" | Select-String "FAIL|Error"' + ], + disallowed: [ + 'npm test 2>&1 | Select-String -NotMatch "node_modules" | rm -rf /', + 'npm test 2>&1 | Select-String "FAIL|Error" && del /F /Q *', + 'npm test 2>&1 | Select-String -NotMatch "node_modules" | Remove-Item -Recurse' + ] + } + + // Test allowed PowerShell commands + for (const command of powershellCommands.allowed) { + jest.clearAllMocks() + + mockPostMessage({ + alwaysAllowExecute: true, + allowedCommands: ['npm test', 'Select-String'], + clineMessages: [ + { + type: 'say', + say: 'task', + ts: Date.now() - 2000, + text: 'Initial task' + } + ] + }) + + mockPostMessage({ + alwaysAllowExecute: true, + allowedCommands: ['npm test', 'Select-String'], + clineMessages: [ + { + type: 'say', + say: 'task', + ts: Date.now() - 2000, + text: 'Initial task' + }, + { + type: 'ask', + ask: 'command', + ts: Date.now(), + text: command, + partial: false + } + ] + }) + + await waitFor(() => { + expect(vscode.postMessage).toHaveBeenCalledWith({ + type: 'askResponse', + askResponse: 'yesButtonClicked' + }) + }) + } + + // Test disallowed PowerShell commands + for (const command of powershellCommands.disallowed) { + jest.clearAllMocks() + + mockPostMessage({ + alwaysAllowExecute: true, + allowedCommands: ['npm test', 'Select-String'], + clineMessages: [ + { + type: 'say', + say: 'task', + ts: Date.now() - 2000, + text: 'Initial task' + } + ] + }) + + mockPostMessage({ + alwaysAllowExecute: true, + allowedCommands: ['npm test', 'Select-String'], + clineMessages: [ + { + type: 'say', + say: 'task', + ts: Date.now() - 2000, + text: 'Initial task' + }, + { + type: 'ask', + ask: 'command', + ts: Date.now(), + text: command, + partial: false + } + ] + }) + + expect(vscode.postMessage).not.toHaveBeenCalledWith({ + type: 'askResponse', + askResponse: 'yesButtonClicked' + }) + } + }) }) })