Skip to content

Commit 462df07

Browse files
authored
feat(chat): Split commands by additonal operators (aws#6908)
## Problem Regular read-only commands are being categorized as high risk. ## Solution - Split commands by additional operators - Do not automatically default to return false on readOnly command ## Testing - Tested manually via plugin build - Updated unit tests <img width="1624" alt="Screenshot 2025-04-01 at 3 50 48 PM" src="https://github.com/user-attachments/assets/8751cf18-948a-4467-8646-e6dc45e62144" /> --- - Treat all work as PUBLIC. Private `feature/x` branches will not be squash-merged at release time. - Your code changes must meet the guidelines in [CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines). - License: I confirm that my contribution is made under the terms of the Apache 2.0 license.
1 parent 62214ee commit 462df07

File tree

2 files changed

+14
-7
lines changed

2 files changed

+14
-7
lines changed

packages/core/src/codewhispererChat/tools/executeBash.ts

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,9 @@ export enum CommandCategory {
1616
Destructive,
1717
}
1818

19-
export const dangerousPatterns = new Set(['<(', '$(', '`', '>', '&&', '||'])
19+
export const dangerousPatterns = new Set(['<(', '$(', '`'])
20+
export const splitOperators = new Set(['|', '&&', '||', '>'])
21+
export const splitOperatorsArray = Array.from(splitOperators)
2022
export const commandCategories = new Map<string, CommandCategory>([
2123
// ReadOnly commands
2224
['ls', CommandCategory.ReadOnly],
@@ -163,17 +165,17 @@ export class ExecuteBash {
163165
return { requiresAcceptance: true }
164166
}
165167

166-
// Split commands by pipe and process each segment
168+
// Split commands by operators and process each segment
167169
let currentCmd: string[] = []
168170
const allCommands: string[][] = []
169171

170172
for (const arg of args) {
171-
if (arg === '|') {
173+
if (splitOperators.has(arg)) {
172174
if (currentCmd.length > 0) {
173175
allCommands.push(currentCmd)
174176
}
175177
currentCmd = []
176-
} else if (arg.includes('|')) {
178+
} else if (splitOperatorsArray.some((op) => arg.includes(op))) {
177179
return { requiresAcceptance: true }
178180
} else {
179181
currentCmd.push(arg)
@@ -208,12 +210,12 @@ export class ExecuteBash {
208210
) {
209211
return { requiresAcceptance: true, warning: highRiskCommandWarningMessage }
210212
}
211-
return { requiresAcceptance: false }
213+
continue
212214
default:
213215
return { requiresAcceptance: true, warning: highRiskCommandWarningMessage }
214216
}
215217
}
216-
return { requiresAcceptance: true }
218+
return { requiresAcceptance: false }
217219
} catch (error) {
218220
this.logger.warn(`Error while checking acceptance: ${(error as Error).message}`)
219221
return { requiresAcceptance: true }

packages/core/src/test/codewhispererChat/tools/executeBash.test.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
import { strict as assert } from 'assert'
77
import sinon from 'sinon'
8-
import { ExecuteBash } from '../../../codewhispererChat/tools/executeBash'
8+
import { destructiveCommandWarningMessage, ExecuteBash } from '../../../codewhispererChat/tools/executeBash'
99
import { ChildProcess } from '../../../shared/utilities/processUtils'
1010

1111
describe('ExecuteBash Tool', () => {
@@ -46,6 +46,11 @@ describe('ExecuteBash Tool', () => {
4646
const execBash = new ExecuteBash({ command: 'ls && rm -rf /' })
4747
const needsAcceptance = execBash.requiresAcceptance().requiresAcceptance
4848
assert.equal(needsAcceptance, true, 'Should require acceptance for dangerous pattern')
49+
assert.equal(
50+
execBash.requiresAcceptance().warning,
51+
destructiveCommandWarningMessage,
52+
'Warning message should match the destructiveCommandWarningMessage'
53+
)
4954
})
5055

5156
it('set requiresAcceptance=false if it is a read-only command', () => {

0 commit comments

Comments
 (0)