Skip to content

Commit 1e83d1a

Browse files
stea9499stea9499
authored
Improved regex for auto-execution of chained commands (#158)
* Improved regex for auto-execution of chained commands
1 parent a982df7 commit 1e83d1a

File tree

3 files changed

+139
-9
lines changed

3 files changed

+139
-9
lines changed

‎.changeset/large-crews-hunt.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"roo-cline": patch
3+
---
4+
5+
Improved regex for auto-execution of chained commands

‎webview-ui/src/components/chat/ChatView.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -523,7 +523,7 @@ const ChatView = ({ isHidden, showAnnouncement, hideAnnouncement, showHistoryVie
523523
}
524524

525525
// Split command by chaining operators
526-
const commands = command.split(/&&|\|\||;|\||\$\(|`/).map(cmd => cmd.trim())
526+
const commands = command.split(/&&|\|\||;|(?<!"[^"]*)\|(?![^"]*")|\$\(|`/).map(cmd => cmd.trim())
527527

528528
// Check if all individual commands are allowed
529529
return commands.every((cmd) => {

‎webview-ui/src/components/chat/__tests__/ChatView.test.tsx‎

Lines changed: 133 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -415,7 +415,7 @@ describe('ChatView - Auto Approval Tests', () => {
415415
it('auto-approves chained commands when all parts are allowed', async () => {
416416
render(
417417
<ExtensionStateContextProvider>
418-
<ChatView
418+
<ChatView
419419
isHidden={false}
420420
showAnnouncement={false}
421421
hideAnnouncement={() => {}}
@@ -429,7 +429,12 @@ describe('ChatView - Auto Approval Tests', () => {
429429
'npm test && npm run build',
430430
'npm test; npm run build',
431431
'npm test || npm run build',
432-
'npm test | npm run build'
432+
'npm test | npm run build',
433+
// Add test for quoted pipes which should be treated as part of the command, not as a chain operator
434+
'echo "hello | world"',
435+
'npm test "param with | inside" && npm run build',
436+
// PowerShell command with Select-String
437+
'npm test 2>&1 | Select-String -NotMatch "node_modules" | Select-String "FAIL|Error"'
433438
]
434439

435440
for (const command of allowedChainedCommands) {
@@ -438,7 +443,7 @@ describe('ChatView - Auto Approval Tests', () => {
438443
// First hydrate state with initial task
439444
mockPostMessage({
440445
alwaysAllowExecute: true,
441-
allowedCommands: ['npm test', 'npm run build'],
446+
allowedCommands: ['npm test', 'npm run build', 'echo', 'Select-String'],
442447
clineMessages: [
443448
{
444449
type: 'say',
@@ -452,7 +457,7 @@ describe('ChatView - Auto Approval Tests', () => {
452457
// Then send the chained command ask message
453458
mockPostMessage({
454459
alwaysAllowExecute: true,
455-
allowedCommands: ['npm test', 'npm run build'],
460+
allowedCommands: ['npm test', 'npm run build', 'echo', 'Select-String'],
456461
clineMessages: [
457462
{
458463
type: 'say',
@@ -483,7 +488,7 @@ describe('ChatView - Auto Approval Tests', () => {
483488
it('does not auto-approve chained commands when any part is disallowed', () => {
484489
render(
485490
<ExtensionStateContextProvider>
486-
<ChatView
491+
<ChatView
487492
isHidden={false}
488493
showAnnouncement={false}
489494
hideAnnouncement={() => {}}
@@ -498,15 +503,20 @@ describe('ChatView - Auto Approval Tests', () => {
498503
'npm test; rm -rf /',
499504
'npm test || rm -rf /',
500505
'npm test | rm -rf /',
506+
// Test subshell execution using $() and backticks
501507
'npm test $(echo dangerous)',
502-
'npm test `echo dangerous`'
508+
'npm test `echo dangerous`',
509+
// Test unquoted pipes with disallowed commands
510+
'npm test | rm -rf /',
511+
// Test PowerShell command with disallowed parts
512+
'npm test 2>&1 | Select-String -NotMatch "node_modules" | rm -rf /'
503513
]
504514

505515
disallowedChainedCommands.forEach(command => {
506516
// First hydrate state with initial task
507517
mockPostMessage({
508518
alwaysAllowExecute: true,
509-
allowedCommands: ['npm test'],
519+
allowedCommands: ['npm test', 'Select-String'],
510520
clineMessages: [
511521
{
512522
type: 'say',
@@ -520,7 +530,7 @@ describe('ChatView - Auto Approval Tests', () => {
520530
// Then send the chained command ask message
521531
mockPostMessage({
522532
alwaysAllowExecute: true,
523-
allowedCommands: ['npm test'],
533+
allowedCommands: ['npm test', 'Select-String'],
524534
clineMessages: [
525535
{
526536
type: 'say',
@@ -545,6 +555,121 @@ describe('ChatView - Auto Approval Tests', () => {
545555
})
546556
})
547557
})
558+
559+
it('handles complex PowerShell command chains correctly', async () => {
560+
render(
561+
<ExtensionStateContextProvider>
562+
<ChatView
563+
isHidden={false}
564+
showAnnouncement={false}
565+
hideAnnouncement={() => {}}
566+
showHistoryView={() => {}}
567+
/>
568+
</ExtensionStateContextProvider>
569+
)
570+
571+
// Test PowerShell specific command chains
572+
const powershellCommands = {
573+
allowed: [
574+
'npm test 2>&1 | Select-String -NotMatch "node_modules"',
575+
'npm test 2>&1 | Select-String "FAIL|Error"',
576+
'npm test 2>&1 | Select-String -NotMatch "node_modules" | Select-String "FAIL|Error"'
577+
],
578+
disallowed: [
579+
'npm test 2>&1 | Select-String -NotMatch "node_modules" | rm -rf /',
580+
'npm test 2>&1 | Select-String "FAIL|Error" && del /F /Q *',
581+
'npm test 2>&1 | Select-String -NotMatch "node_modules" | Remove-Item -Recurse'
582+
]
583+
}
584+
585+
// Test allowed PowerShell commands
586+
for (const command of powershellCommands.allowed) {
587+
jest.clearAllMocks()
588+
589+
mockPostMessage({
590+
alwaysAllowExecute: true,
591+
allowedCommands: ['npm test', 'Select-String'],
592+
clineMessages: [
593+
{
594+
type: 'say',
595+
say: 'task',
596+
ts: Date.now() - 2000,
597+
text: 'Initial task'
598+
}
599+
]
600+
})
601+
602+
mockPostMessage({
603+
alwaysAllowExecute: true,
604+
allowedCommands: ['npm test', 'Select-String'],
605+
clineMessages: [
606+
{
607+
type: 'say',
608+
say: 'task',
609+
ts: Date.now() - 2000,
610+
text: 'Initial task'
611+
},
612+
{
613+
type: 'ask',
614+
ask: 'command',
615+
ts: Date.now(),
616+
text: command,
617+
partial: false
618+
}
619+
]
620+
})
621+
622+
await waitFor(() => {
623+
expect(vscode.postMessage).toHaveBeenCalledWith({
624+
type: 'askResponse',
625+
askResponse: 'yesButtonClicked'
626+
})
627+
})
628+
}
629+
630+
// Test disallowed PowerShell commands
631+
for (const command of powershellCommands.disallowed) {
632+
jest.clearAllMocks()
633+
634+
mockPostMessage({
635+
alwaysAllowExecute: true,
636+
allowedCommands: ['npm test', 'Select-String'],
637+
clineMessages: [
638+
{
639+
type: 'say',
640+
say: 'task',
641+
ts: Date.now() - 2000,
642+
text: 'Initial task'
643+
}
644+
]
645+
})
646+
647+
mockPostMessage({
648+
alwaysAllowExecute: true,
649+
allowedCommands: ['npm test', 'Select-String'],
650+
clineMessages: [
651+
{
652+
type: 'say',
653+
say: 'task',
654+
ts: Date.now() - 2000,
655+
text: 'Initial task'
656+
},
657+
{
658+
type: 'ask',
659+
ask: 'command',
660+
ts: Date.now(),
661+
text: command,
662+
partial: false
663+
}
664+
]
665+
})
666+
667+
expect(vscode.postMessage).not.toHaveBeenCalledWith({
668+
type: 'askResponse',
669+
askResponse: 'yesButtonClicked'
670+
})
671+
}
672+
})
548673
})
549674
})
550675

0 commit comments

Comments
 (0)