Skip to content

Commit 18be691

Browse files
ashishrp-awsashishrp-aws
authored
chore(amazonq): revert for emergency reverts for auto-approve feature (aws#1973)
* Revert "revert: revert for all commits for emergency deployment (aws#1966)" This reverts commit 519f75d. * Revert "fix(amazonq): revert commit f17b631 (aws#1965)" This reverts commit 8c2cab6. * fix: fix for CI test failure
1 parent 64d4e3e commit 18be691

File tree

16 files changed

+618
-108
lines changed

16 files changed

+618
-108
lines changed

‎chat-client/src/client/tabs/tabFactory.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ export class TabFactory {
187187
tabBarButtons.push({
188188
id: McpServerTabButtonId,
189189
icon: MynahIcons.TOOLS,
190-
description: 'Configure MCP servers',
190+
description: 'Configure MCP servers and Built-in tools',
191191
})
192192
}
193193

‎server/aws-lsp-codewhisperer/src/language-server/agenticChat/agenticChatController.ts‎

Lines changed: 38 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,9 @@ import {
185185
DEFAULT_WINDOW_REJECT_SHORTCUT,
186186
DEFAULT_MACOS_STOP_SHORTCUT,
187187
DEFAULT_WINDOW_STOP_SHORTCUT,
188+
OUT_OF_WORKSPACE_WARNING_MSG,
189+
CREDENTIAL_FILE_WARNING_MSG,
190+
BINARY_FILE_WARNING_MSG,
188191
} from './constants/constants'
189192
import {
190193
AgenticChatError,
@@ -1692,9 +1695,9 @@ export class AgenticChatController implements ChatHandlers {
16921695
const tool = new Tool(this.#features)
16931696

16941697
// For MCP tools, get the permission from McpManager
1695-
// const permission = McpManager.instance.getToolPerm('Built-in', toolUse.name)
1698+
const permission = McpManager.instance.getToolPerm('Built-in', toolUse.name)
16961699
// If permission is 'alwaysAllow', we don't need to ask for acceptance
1697-
// const builtInPermission = permission !== 'alwaysAllow'
1700+
const builtInPermission = permission !== 'alwaysAllow'
16981701

16991702
// Get the approved paths from the session
17001703
const approvedPaths = session.approvedPaths
@@ -1705,19 +1708,40 @@ export class AgenticChatController implements ChatHandlers {
17051708
approvedPaths
17061709
)
17071710

1711+
const isExecuteBash = toolUse.name === EXECUTE_BASH
1712+
1713+
// check if tool execution's path is out of workspace
1714+
const isOutOfWorkSpace = warning === OUT_OF_WORKSPACE_WARNING_MSG
1715+
// check if tool involved secured files
1716+
const isSecuredFilesInvoled =
1717+
warning === BINARY_FILE_WARNING_MSG || warning === CREDENTIAL_FILE_WARNING_MSG
1718+
17081719
// Honor built-in permission if available, otherwise use tool's requiresAcceptance
1709-
// const requiresAcceptance = builtInPermission || toolRequiresAcceptance
1720+
let toolRequiresAcceptance =
1721+
(builtInPermission || isOutOfWorkSpace || isSecuredFilesInvoled) ?? requiresAcceptance
1722+
1723+
// if the command is read-only and in-workspace --> flip back to no approval needed
1724+
if (
1725+
isExecuteBash &&
1726+
commandCategory === CommandCategory.ReadOnly &&
1727+
!isOutOfWorkSpace &&
1728+
!requiresAcceptance
1729+
) {
1730+
toolRequiresAcceptance = false
1731+
}
17101732

1711-
if (requiresAcceptance || toolUse.name === EXECUTE_BASH) {
1733+
if (toolRequiresAcceptance || isExecuteBash) {
17121734
// for executeBash, we till send the confirmation message without action buttons
17131735
const confirmationResult = this.#processToolConfirmation(
17141736
toolUse,
1715-
requiresAcceptance,
1737+
toolRequiresAcceptance,
17161738
warning,
1717-
commandCategory
1739+
commandCategory,
1740+
toolUse.name,
1741+
builtInPermission
17181742
)
17191743
cachedButtonBlockId = await chatResultStream.writeResultBlock(confirmationResult)
1720-
const isExecuteBash = toolUse.name === EXECUTE_BASH
1744+
17211745
if (isExecuteBash) {
17221746
this.#telemetryController.emitInteractWithAgenticChat(
17231747
'GeneratedCommand',
@@ -1728,7 +1752,7 @@ export class AgenticChatController implements ChatHandlers {
17281752
this.#abTestingAllocation?.userVariation
17291753
)
17301754
}
1731-
if (requiresAcceptance) {
1755+
if (toolRequiresAcceptance) {
17321756
await this.waitForToolApproval(
17331757
toolUse,
17341758
chatResultStream,
@@ -2699,7 +2723,7 @@ export class AgenticChatController implements ChatHandlers {
26992723
body = builtInPermission
27002724
? `I need permission to read files.\n${formattedPaths.join('\n')}`
27012725
: `I need permission to read files outside the workspace.\n${formattedPaths.join('\n')}`
2702-
} else {
2726+
} else if (toolName === 'listDirectory') {
27032727
const readFilePath = (toolUse.input as unknown as ListDirectoryParams).path
27042728

27052729
// Validate the path using our synchronous utility
@@ -2709,6 +2733,11 @@ export class AgenticChatController implements ChatHandlers {
27092733
body = builtInPermission
27102734
? `I need permission to list directories.\n\`${readFilePath}\``
27112735
: `I need permission to list directories outside the workspace.\n\`${readFilePath}\``
2736+
} else {
2737+
const readFilePath = (toolUse.input as unknown as ListDirectoryParams).path
2738+
body = builtInPermission
2739+
? `I need permission to search files.\n\`${readFilePath}\``
2740+
: `I need permission to search files outside the workspace.\n\`${readFilePath}\``
27122741
}
27132742
break
27142743
}

‎server/aws-lsp-codewhisperer/src/language-server/agenticChat/constants/constants.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,3 +88,11 @@ export const DEFAULT_LINUX_STOP_SHORTCUT = 'Meta + ⇧ + ⌫'
8888
export const DEFAULT_MACOS_REJECT_SHORTCUT = '⇧ ⌘ R'
8989
export const DEFAULT_WINDOW_REJECT_SHORTCUT = 'Ctrl + ⇧ + R'
9090
export const DEFAULT_LINUX_REJECT_SHORTCUT = 'Meta + ⇧ + R'
91+
92+
// Warning Message Constants
93+
export const DESTRUCTIVE_COMMAND_WARNING_MSG = 'WARNING: Potentially destructive command detected:\n\n'
94+
export const MUTATE_COMMAND_WARNING_MSG = 'Mutation command:\n\n'
95+
export const OUT_OF_WORKSPACE_WARNING_MSG = 'Execution out of workspace scope:\n\n'
96+
export const CREDENTIAL_FILE_WARNING_MSG =
97+
'WARNING: Command involves credential files that require secure permissions:\n\n'
98+
export const BINARY_FILE_WARNING_MSG = 'WARNING: Command involves binary files that require secure permissions:\n\n'

‎server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/executeBash.ts‎

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,15 @@ import { getWorkspaceFolderPaths } from '@aws/lsp-core/out/util/workspaceUtils'
1313
// eslint-disable-next-line import/no-nodejs-modules
1414
import { existsSync, statSync } from 'fs'
1515

16+
// Warning message
17+
import {
18+
BINARY_FILE_WARNING_MSG,
19+
CREDENTIAL_FILE_WARNING_MSG,
20+
DESTRUCTIVE_COMMAND_WARNING_MSG,
21+
MUTATE_COMMAND_WARNING_MSG,
22+
OUT_OF_WORKSPACE_WARNING_MSG,
23+
} from '../constants/constants'
24+
1625
export enum CommandCategory {
1726
ReadOnly,
1827
Mutate,
@@ -108,12 +117,6 @@ export const commandCategories = new Map<string, CommandCategory>([
108117
])
109118
export const maxToolResponseSize: number = 1024 * 1024 // 1MB
110119
export const lineCount: number = 1024
111-
export const destructiveCommandWarningMessage = 'WARNING: Potentially destructive command detected:\n\n'
112-
export const mutateCommandWarningMessage = 'Mutation command:\n\n'
113-
export const outOfWorkspaceWarningmessage = 'Execution out of workspace scope:\n\n'
114-
export const credentialFileWarningMessage =
115-
'WARNING: Command involves credential files that require secure permissions:\n\n'
116-
export const binaryFileWarningMessage = 'WARNING: Command involves binary files that require secure permissions:\n\n'
117120

118121
/**
119122
* Parameters for executing a command on the system shell.
@@ -232,7 +235,7 @@ export class ExecuteBash {
232235
// Treat tilde paths as absolute paths (they will be expanded by the shell)
233236
return {
234237
requiresAcceptance: true,
235-
warning: destructiveCommandWarningMessage,
238+
warning: DESTRUCTIVE_COMMAND_WARNING_MSG,
236239
commandCategory: CommandCategory.Destructive,
237240
}
238241
} else if (!isAbsolute(arg) && params.cwd) {
@@ -255,7 +258,7 @@ export class ExecuteBash {
255258
this.logging.info(`Detected credential file in command: ${fullPath}`)
256259
return {
257260
requiresAcceptance: true,
258-
warning: credentialFileWarningMessage,
261+
warning: CREDENTIAL_FILE_WARNING_MSG,
259262
commandCategory: CommandCategory.Mutate,
260263
}
261264
}
@@ -265,7 +268,7 @@ export class ExecuteBash {
265268
this.logging.info(`Detected binary file in command: ${fullPath}`)
266269
return {
267270
requiresAcceptance: true,
268-
warning: binaryFileWarningMessage,
271+
warning: BINARY_FILE_WARNING_MSG,
269272
commandCategory: CommandCategory.Mutate,
270273
}
271274
}
@@ -282,7 +285,7 @@ export class ExecuteBash {
282285
if (!isInWorkspace) {
283286
return {
284287
requiresAcceptance: true,
285-
warning: outOfWorkspaceWarningmessage,
288+
warning: OUT_OF_WORKSPACE_WARNING_MSG,
286289
commandCategory: highestCommandCategory,
287290
}
288291
}
@@ -306,13 +309,13 @@ export class ExecuteBash {
306309
case CommandCategory.Destructive:
307310
return {
308311
requiresAcceptance: true,
309-
warning: destructiveCommandWarningMessage,
312+
warning: DESTRUCTIVE_COMMAND_WARNING_MSG,
310313
commandCategory: CommandCategory.Destructive,
311314
}
312315
case CommandCategory.Mutate:
313316
return {
314317
requiresAcceptance: true,
315-
warning: mutateCommandWarningMessage,
318+
warning: MUTATE_COMMAND_WARNING_MSG,
316319
commandCategory: CommandCategory.Mutate,
317320
}
318321
case CommandCategory.ReadOnly:
@@ -334,7 +337,7 @@ export class ExecuteBash {
334337
if (!workspaceFolders || workspaceFolders.length === 0) {
335338
return {
336339
requiresAcceptance: true,
337-
warning: outOfWorkspaceWarningmessage,
340+
warning: OUT_OF_WORKSPACE_WARNING_MSG,
338341
commandCategory: highestCommandCategory,
339342
}
340343
}
@@ -351,7 +354,7 @@ export class ExecuteBash {
351354
if (!isInWorkspace) {
352355
return {
353356
requiresAcceptance: true,
354-
warning: outOfWorkspaceWarningmessage,
357+
warning: OUT_OF_WORKSPACE_WARNING_MSG,
355358
commandCategory: highestCommandCategory,
356359
}
357360
}

‎server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/grepSearch.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { ChildProcess, ChildProcessOptions } from '@aws/lsp-core/out/util/proces
77
import path = require('path')
88
import { dirname } from 'path'
99
import { pathToFileURL } from 'url'
10+
import { OUT_OF_WORKSPACE_WARNING_MSG } from '../constants/constants'
1011

1112
export interface GrepSearchParams {
1213
path?: string
@@ -81,7 +82,8 @@ export class GrepSearch {
8182

8283
public async requiresAcceptance(params: GrepSearchParams): Promise<CommandValidation> {
8384
const path = this.getSearchDirectory(params.path)
84-
return { requiresAcceptance: !workspaceUtils.isInWorkspace(getWorkspaceFolderPaths(this.workspace), path) }
85+
const isInWorkspace = workspaceUtils.isInWorkspace(getWorkspaceFolderPaths(this.workspace), path)
86+
return { requiresAcceptance: !isInWorkspace, warning: !isInWorkspace ? OUT_OF_WORKSPACE_WARNING_MSG : '' }
8587
}
8688

8789
public async invoke(params: GrepSearchParams, token?: CancellationToken): Promise<InvokeOutput> {

‎server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/mcp/mcpEventHandler.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ describe('McpEventHandler error handling', () => {
4949
},
5050
agent: {
5151
getTools: sinon.stub().returns([]),
52+
getBuiltInToolNames: sinon.stub().returns([]),
5253
},
5354
lsp: {},
5455
telemetry: {

0 commit comments

Comments
 (0)