Skip to content

Commit 4e0659c

Browse files
authored
feat(chat): Add validation to fileRead, ListDir and ExecBash tools (aws#7008)
## Problem We should ask for consensus tools accessing files/dirs outside of workspace ## Solution Add validation step when accessing files/dirs outside of workspace using fsRead or listDirectories tools. --- - 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 4b23600 commit 4e0659c

File tree

13 files changed

+249
-8
lines changed

13 files changed

+249
-8
lines changed

packages/core/src/amazonq/webview/ui/apps/cwChatConnector.ts

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -323,7 +323,14 @@ export class Connector extends BaseConnector {
323323

324324
if (
325325
!this.onChatAnswerUpdated ||
326-
!['accept-code-diff', 'reject-code-diff', 'run-shell-command', 'reject-shell-command'].includes(action.id)
326+
![
327+
'accept-code-diff',
328+
'reject-code-diff',
329+
'run-shell-command',
330+
'reject-shell-command',
331+
'confirm-tool-use',
332+
'reject-tool-use',
333+
].includes(action.id)
327334
) {
328335
return
329336
}
@@ -381,6 +388,28 @@ export class Connector extends BaseConnector {
381388
},
382389
}
383390
break
391+
case 'confirm-tool-use':
392+
answer.header = {
393+
icon: 'shell' as MynahIconsType,
394+
body: 'shell',
395+
status: {
396+
icon: 'ok' as MynahIconsType,
397+
text: 'Accepted',
398+
status: 'success',
399+
},
400+
}
401+
break
402+
case 'reject-tool-use':
403+
answer.header = {
404+
icon: 'shell' as MynahIconsType,
405+
body: 'shell',
406+
status: {
407+
icon: 'cancel' as MynahIconsType,
408+
text: 'Rejected',
409+
status: 'error',
410+
},
411+
}
412+
break
384413
default:
385414
break
386415
}

packages/core/src/codewhispererChat/controllers/chat/controller.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -827,7 +827,12 @@ export class ChatController {
827827

828828
const session = this.sessionStorage.getSession(message.tabID!)
829829
const currentToolUse = session.toolUseWithError?.toolUse
830-
if (currentToolUse && currentToolUse.name === ToolType.ExecuteBash) {
830+
if (
831+
currentToolUse &&
832+
(currentToolUse.name === ToolType.ExecuteBash ||
833+
currentToolUse.name === ToolType.FsRead ||
834+
currentToolUse.name === ToolType.ListDirectory)
835+
) {
831836
session.toolUseWithError.error = new Error('Tool use was rejected by the user.')
832837
} else {
833838
getLogger().error(
@@ -843,6 +848,7 @@ export class ChatController {
843848
break
844849
case 'run-shell-command':
845850
case 'generic-tool-execution':
851+
case 'confirm-tool-use':
846852
await this.processToolUseMessage(message)
847853
if (message.action.id === 'run-shell-command' && message.action.text === 'Run') {
848854
this.telemetryHelper.recordInteractionWithAgenticChat(
@@ -860,6 +866,7 @@ export class ChatController {
860866
this.telemetryHelper.recordInteractionWithAgenticChat(AgenticChatInteractionType.RejectDiff, message)
861867
break
862868
case 'reject-shell-command':
869+
case 'reject-tool-use':
863870
await this.rejectShellCommand(message)
864871
await this.processToolUseMessage(message)
865872
break

packages/core/src/codewhispererChat/controllers/chat/messenger/messenger.ts

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -573,13 +573,13 @@ export class Messenger {
573573
const buttons: ChatItemButton[] = [
574574
{
575575
id: 'reject-shell-command',
576-
text: localize('AWS.amazonq.executeBash.reject', 'Reject'),
576+
text: localize('AWS.generic.reject', 'Reject'),
577577
status: 'clear',
578578
icon: 'cancel' as MynahIconsType,
579579
},
580580
{
581581
id: 'run-shell-command',
582-
text: localize('AWS.amazonq.executeBash.run', 'Run'),
582+
text: localize('AWS.generic.run', 'Run'),
583583
status: 'clear',
584584
icon: 'play' as MynahIconsType,
585585
},
@@ -624,6 +624,28 @@ export class Messenger {
624624
buttons,
625625
fileList,
626626
}
627+
} else if (toolUse?.name === ToolType.ListDirectory || toolUse?.name === ToolType.FsRead) {
628+
if (validation.requiresAcceptance) {
629+
const buttons: ChatItemButton[] = [
630+
{
631+
id: 'confirm-tool-use',
632+
text: localize('AWS.generic.run', 'Run'),
633+
status: 'main',
634+
icon: 'play' as MynahIconsType,
635+
},
636+
{
637+
id: 'reject-tool-use',
638+
text: localize('AWS.generic.reject', 'Reject'),
639+
status: 'clear',
640+
icon: 'cancel' as MynahIconsType,
641+
},
642+
]
643+
header = {
644+
icon: 'shell' as MynahIconsType,
645+
body: 'shell',
646+
buttons,
647+
}
648+
}
627649
}
628650

629651
this.dispatcher.sendChatMessage(

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@ import { fs } from '../../shared/fs/fs'
99
import { ChildProcess, ChildProcessOptions } from '../../shared/utilities/processUtils'
1010
import { InvokeOutput, OutputKind, sanitizePath } from './toolShared'
1111
import { split } from 'shlex'
12+
import path from 'path'
13+
import * as vscode from 'vscode'
14+
import { isInDirectory } from '../../shared/filesystemUtilities'
1215

1316
export enum CommandCategory {
1417
ReadOnly,
@@ -185,6 +188,27 @@ export class ExecuteBash {
185188
return { requiresAcceptance: true }
186189
}
187190

191+
// For each command, validate arguments for path safety within workspace
192+
for (const arg of cmdArgs) {
193+
if (this.looksLikePath(arg)) {
194+
// If not absolute, resolve using workingDirectory if available.
195+
let fullPath = arg
196+
if (!path.isAbsolute(arg) && this.workingDirectory) {
197+
fullPath = path.join(this.workingDirectory, arg)
198+
}
199+
const workspaceFolders = vscode.workspace.workspaceFolders
200+
if (!workspaceFolders || workspaceFolders.length === 0) {
201+
return { requiresAcceptance: true, warning: destructiveCommandWarningMessage }
202+
}
203+
const isInWorkspace = workspaceFolders.some((folder) =>
204+
isInDirectory(folder.uri.fsPath, fullPath)
205+
)
206+
if (!isInWorkspace) {
207+
return { requiresAcceptance: true, warning: destructiveCommandWarningMessage }
208+
}
209+
}
210+
}
211+
188212
const command = cmdArgs[0]
189213
const category = commandCategories.get(command)
190214

@@ -371,4 +395,8 @@ export class ExecuteBash {
371395
updates.write('```shell\n' + this.command + '\n```')
372396
updates.end()
373397
}
398+
399+
private looksLikePath(arg: string): boolean {
400+
return arg.startsWith('/') || arg.startsWith('./') || arg.startsWith('../')
401+
}
374402
}

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

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,10 @@
55
import * as vscode from 'vscode'
66
import { getLogger } from '../../shared/logger/logger'
77
import fs from '../../shared/fs/fs'
8-
import { InvokeOutput, OutputKind, sanitizePath } from './toolShared'
98
import { Writable } from 'stream'
109
import path from 'path'
10+
import { InvokeOutput, OutputKind, sanitizePath, CommandValidation } from './toolShared'
11+
import { isInDirectory } from '../../shared/filesystemUtilities'
1112

1213
export interface FsReadParams {
1314
path: string
@@ -68,6 +69,18 @@ export class FsRead {
6869
updates.end()
6970
}
7071

72+
public requiresAcceptance(): CommandValidation {
73+
const workspaceFolders = vscode.workspace.workspaceFolders
74+
if (!workspaceFolders || workspaceFolders.length === 0) {
75+
return { requiresAcceptance: true }
76+
}
77+
const isInWorkspace = workspaceFolders.some((folder) => isInDirectory(folder.uri.fsPath, this.fsPath))
78+
if (!isInWorkspace) {
79+
return { requiresAcceptance: true }
80+
}
81+
return { requiresAcceptance: false }
82+
}
83+
7184
public async invoke(updates?: Writable): Promise<InvokeOutput> {
7285
try {
7386
const fileUri = vscode.Uri.file(this.fsPath)

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

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,10 @@ import * as vscode from 'vscode'
66
import { getLogger } from '../../shared/logger/logger'
77
import { readDirectoryRecursively } from '../../shared/utilities/workspaceUtils'
88
import fs from '../../shared/fs/fs'
9-
import { InvokeOutput, OutputKind, sanitizePath } from './toolShared'
109
import { Writable } from 'stream'
1110
import path from 'path'
11+
import { InvokeOutput, OutputKind, sanitizePath, CommandValidation } from './toolShared'
12+
import { isInDirectory } from '../../shared/filesystemUtilities'
1213

1314
export interface ListDirectoryParams {
1415
path: string
@@ -61,6 +62,18 @@ export class ListDirectory {
6162
updates.end()
6263
}
6364

65+
public requiresAcceptance(): CommandValidation {
66+
const workspaceFolders = vscode.workspace.workspaceFolders
67+
if (!workspaceFolders || workspaceFolders.length === 0) {
68+
return { requiresAcceptance: true }
69+
}
70+
const isInWorkspace = workspaceFolders.some((folder) => isInDirectory(folder.uri.fsPath, this.fsPath))
71+
if (!isInWorkspace) {
72+
return { requiresAcceptance: true }
73+
}
74+
return { requiresAcceptance: false }
75+
}
76+
6477
public async invoke(updates?: Writable): Promise<InvokeOutput> {
6578
try {
6679
const fileUri = vscode.Uri.file(this.fsPath)

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,3 +32,8 @@ export function sanitizePath(inputPath: string): string {
3232
}
3333
return sanitized
3434
}
35+
36+
export interface CommandValidation {
37+
requiresAcceptance: boolean
38+
warning?: string
39+
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,13 +40,13 @@ export class ToolUtils {
4040
static requiresAcceptance(tool: Tool): CommandValidation {
4141
switch (tool.type) {
4242
case ToolType.FsRead:
43-
return { requiresAcceptance: false }
43+
return tool.tool.requiresAcceptance()
4444
case ToolType.FsWrite:
4545
return { requiresAcceptance: false }
4646
case ToolType.ExecuteBash:
4747
return tool.tool.requiresAcceptance()
4848
case ToolType.ListDirectory:
49-
return { requiresAcceptance: false }
49+
return tool.tool.requiresAcceptance()
5050
}
5151
}
5252

packages/core/src/shared/localizedText.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ export const invalidArn = localize('AWS.error.invalidArn', 'Invalid ARN')
1818
export const localizedDelete = localize('AWS.generic.delete', 'Delete')
1919
export const cancel = localize('AWS.generic.cancel', 'Cancel')
2020
export const help = localize('AWS.generic.help', 'Help')
21+
export const run = localize('AWS.generic.run', 'Run')
22+
export const reject = localize('AWS.generic.reject', 'Reject')
2123
export const invalidNumberWarning = localize('AWS.validateTime.error.invalidNumber', 'Input must be a positive number')
2224
export const viewDocs = localize('AWS.generic.viewDocs', 'View Documentation')
2325
export const recentlyUsed = localize('AWS.generic.recentlyUsed', 'recently used')

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

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { strict as assert } from 'assert'
77
import sinon from 'sinon'
88
import { destructiveCommandWarningMessage, ExecuteBash } from '../../../codewhispererChat/tools/executeBash'
99
import { ChildProcess } from '../../../shared/utilities/processUtils'
10+
import * as vscode from 'vscode'
1011

1112
describe('ExecuteBash Tool', () => {
1213
let runStub: sinon.SinonStub
@@ -114,4 +115,55 @@ describe('ExecuteBash Tool', () => {
114115

115116
assert.strictEqual(invokeStub.callCount, 1)
116117
})
118+
119+
it('requires acceptance if the command references an absolute file path outside the workspace', () => {
120+
// Stub workspace folders to simulate a workspace at '/workspace/folder'
121+
const workspaceStub = sinon
122+
.stub(vscode.workspace, 'workspaceFolders')
123+
.value([{ uri: { fsPath: '/workspace/folder' } } as any])
124+
// Command references an absolute path outside the workspace
125+
const execBash = new ExecuteBash({ command: 'cat /not/in/workspace/file.txt', cwd: '/workspace/folder' })
126+
const result = execBash.requiresAcceptance()
127+
128+
assert.equal(
129+
result.requiresAcceptance,
130+
true,
131+
'Should require acceptance for an absolute path outside of workspace'
132+
)
133+
workspaceStub.restore()
134+
})
135+
136+
it('does NOT require acceptance if the command references a relative file path inside the workspace', () => {
137+
// Stub workspace folders to simulate a workspace at '/workspace/folder'
138+
const workspaceStub = sinon
139+
.stub(vscode.workspace, 'workspaceFolders')
140+
.value([{ uri: { fsPath: '/workspace/folder' } } as any])
141+
142+
// Command references a relative path that resolves within the workspace
143+
const execBash = new ExecuteBash({ command: 'cat ./file.txt', cwd: '/workspace/folder' })
144+
const result = execBash.requiresAcceptance()
145+
146+
assert.equal(result.requiresAcceptance, false, 'Relative path inside workspace should not require acceptance')
147+
148+
workspaceStub.restore()
149+
})
150+
151+
it('does NOT require acceptance if there is no path-like token in the command', () => {
152+
// Stub workspace folders (even though they are not used in this case)
153+
const workspaceStub = sinon
154+
.stub(vscode.workspace, 'workspaceFolders')
155+
.value([{ uri: { fsPath: '/workspace/folder' } } as any])
156+
157+
// Command with tokens that do not look like file paths
158+
const execBash = new ExecuteBash({ command: 'echo hello world', cwd: '/workspace/folder' })
159+
const result = execBash.requiresAcceptance()
160+
161+
assert.equal(
162+
result.requiresAcceptance,
163+
false,
164+
'A command without any path-like token should not require acceptance'
165+
)
166+
167+
workspaceStub.restore()
168+
})
117169
})

0 commit comments

Comments
 (0)