From f52b16a7fbeafa810ee4e3a7441cf038b9b3853e Mon Sep 17 00:00:00 2001 From: Randall-Jiang Date: Fri, 18 Apr 2025 08:11:48 -0700 Subject: [PATCH 1/4] remove destructive warning for the commands not in the list --- packages/core/src/codewhispererChat/tools/executeBash.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/src/codewhispererChat/tools/executeBash.ts b/packages/core/src/codewhispererChat/tools/executeBash.ts index 77390ebdb7d..e7fbcf3d134 100644 --- a/packages/core/src/codewhispererChat/tools/executeBash.ts +++ b/packages/core/src/codewhispererChat/tools/executeBash.ts @@ -200,13 +200,13 @@ export class ExecuteBash { } const workspaceFolders = vscode.workspace.workspaceFolders if (!workspaceFolders || workspaceFolders.length === 0) { - return { requiresAcceptance: true, warning: destructiveCommandWarningMessage } + return { requiresAcceptance: true } } const isInWorkspace = workspaceFolders.some((folder) => isInDirectory(folder.uri.fsPath, fullPath) ) if (!isInWorkspace) { - return { requiresAcceptance: true, warning: destructiveCommandWarningMessage } + return { requiresAcceptance: true } } } } From 27817641700d686a92402182ef393af23469553a Mon Sep 17 00:00:00 2001 From: Randall-Jiang Date: Fri, 18 Apr 2025 12:00:36 -0700 Subject: [PATCH 2/4] change the test case --- .../core/src/test/codewhispererChat/tools/executeBash.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/test/codewhispererChat/tools/executeBash.test.ts b/packages/core/src/test/codewhispererChat/tools/executeBash.test.ts index 54dfb3f02a0..95b97a66431 100644 --- a/packages/core/src/test/codewhispererChat/tools/executeBash.test.ts +++ b/packages/core/src/test/codewhispererChat/tools/executeBash.test.ts @@ -44,7 +44,7 @@ describe('ExecuteBash Tool', () => { }) it('set requiresAcceptance=true if the command has dangerous patterns', () => { - const execBash = new ExecuteBash({ command: 'ls && rm -rf /' }) + const execBash = new ExecuteBash({ command: 'rm -rf /' }) const needsAcceptance = execBash.requiresAcceptance().requiresAcceptance assert.equal(needsAcceptance, true, 'Should require acceptance for dangerous pattern') assert.equal( From 0dc4ce8f5425715f4fff84127d38d86bc3f473ac Mon Sep 17 00:00:00 2001 From: Randall-Jiang Date: Fri, 18 Apr 2025 12:38:19 -0700 Subject: [PATCH 3/4] fix the test fail --- .../core/src/codewhispererChat/tools/executeBash.ts | 11 +++++++---- .../test/codewhispererChat/tools/executeBash.test.ts | 2 +- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/packages/core/src/codewhispererChat/tools/executeBash.ts b/packages/core/src/codewhispererChat/tools/executeBash.ts index e7fbcf3d134..79e42bfcb54 100644 --- a/packages/core/src/codewhispererChat/tools/executeBash.ts +++ b/packages/core/src/codewhispererChat/tools/executeBash.ts @@ -189,24 +189,24 @@ export class ExecuteBash { if (cmdArgs.length === 0) { return { requiresAcceptance: true } } - + let hasOutsideWorkspacePath = false // For each command, validate arguments for path safety within workspace for (const arg of cmdArgs) { if (this.looksLikePath(arg)) { - // If not absolute, resolve using workingDirectory if available. let fullPath = arg if (!path.isAbsolute(arg) && this.workingDirectory) { fullPath = path.join(this.workingDirectory, arg) } const workspaceFolders = vscode.workspace.workspaceFolders if (!workspaceFolders || workspaceFolders.length === 0) { - return { requiresAcceptance: true } + hasOutsideWorkspacePath = true + continue } const isInWorkspace = workspaceFolders.some((folder) => isInDirectory(folder.uri.fsPath, fullPath) ) if (!isInWorkspace) { - return { requiresAcceptance: true } + hasOutsideWorkspacePath = true } } } @@ -220,6 +220,9 @@ export class ExecuteBash { case CommandCategory.Mutate: return { requiresAcceptance: true, warning: mutateCommandWarningMessage } case CommandCategory.ReadOnly: + if (hasOutsideWorkspacePath) { + return { requiresAcceptance: true } + } continue default: return { requiresAcceptance: true } diff --git a/packages/core/src/test/codewhispererChat/tools/executeBash.test.ts b/packages/core/src/test/codewhispererChat/tools/executeBash.test.ts index 95b97a66431..54dfb3f02a0 100644 --- a/packages/core/src/test/codewhispererChat/tools/executeBash.test.ts +++ b/packages/core/src/test/codewhispererChat/tools/executeBash.test.ts @@ -44,7 +44,7 @@ describe('ExecuteBash Tool', () => { }) it('set requiresAcceptance=true if the command has dangerous patterns', () => { - const execBash = new ExecuteBash({ command: 'rm -rf /' }) + const execBash = new ExecuteBash({ command: 'ls && rm -rf /' }) const needsAcceptance = execBash.requiresAcceptance().requiresAcceptance assert.equal(needsAcceptance, true, 'Should require acceptance for dangerous pattern') assert.equal( From 0997a16e68562da93be22ad7dac01926ee2230a8 Mon Sep 17 00:00:00 2001 From: Randall-Jiang Date: Fri, 18 Apr 2025 15:06:42 -0700 Subject: [PATCH 4/4] add test for cmd outside workspace --- .../src/codewhispererChat/tools/executeBash.ts | 14 +++++++++----- .../codewhispererChat/tools/executeBash.test.ts | 10 ++++++++++ 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/packages/core/src/codewhispererChat/tools/executeBash.ts b/packages/core/src/codewhispererChat/tools/executeBash.ts index 79e42bfcb54..8701b4b99dc 100644 --- a/packages/core/src/codewhispererChat/tools/executeBash.ts +++ b/packages/core/src/codewhispererChat/tools/executeBash.ts @@ -189,31 +189,34 @@ export class ExecuteBash { if (cmdArgs.length === 0) { return { requiresAcceptance: true } } + + const command = cmdArgs[0] + const category = commandCategories.get(command) let hasOutsideWorkspacePath = false - // For each command, validate arguments for path safety within workspace + for (const arg of cmdArgs) { if (this.looksLikePath(arg)) { let fullPath = arg if (!path.isAbsolute(arg) && this.workingDirectory) { fullPath = path.join(this.workingDirectory, arg) } + const workspaceFolders = vscode.workspace.workspaceFolders if (!workspaceFolders || workspaceFolders.length === 0) { hasOutsideWorkspacePath = true - continue + break } + const isInWorkspace = workspaceFolders.some((folder) => isInDirectory(folder.uri.fsPath, fullPath) ) if (!isInWorkspace) { hasOutsideWorkspacePath = true + break } } } - const command = cmdArgs[0] - const category = commandCategories.get(command) - switch (category) { case CommandCategory.Destructive: return { requiresAcceptance: true, warning: destructiveCommandWarningMessage } @@ -228,6 +231,7 @@ export class ExecuteBash { return { requiresAcceptance: true } } } + return { requiresAcceptance: false } } catch (error) { this.logger.warn(`Error while checking acceptance: ${(error as Error).message}`) diff --git a/packages/core/src/test/codewhispererChat/tools/executeBash.test.ts b/packages/core/src/test/codewhispererChat/tools/executeBash.test.ts index 54dfb3f02a0..fade87bcbfc 100644 --- a/packages/core/src/test/codewhispererChat/tools/executeBash.test.ts +++ b/packages/core/src/test/codewhispererChat/tools/executeBash.test.ts @@ -54,6 +54,16 @@ describe('ExecuteBash Tool', () => { ) }) + it('requiresAcceptance=true without destructive warning for read-only command outside workspace', () => { + const execBash = new ExecuteBash({ + command: 'ls /', + cwd: '/do/not/exist/dir', + }) + const result = execBash.requiresAcceptance() + assert.equal(result.requiresAcceptance, true, 'Should require acceptance due to path outside workspace') + assert.equal(result.warning, undefined, 'Should not show destructive warning for read-only command') + }) + it('set requiresAcceptance=false if it is a read-only command', () => { const execBash = new ExecuteBash({ command: 'cat file.txt' }) const needsAcceptance = execBash.requiresAcceptance().requiresAcceptance