From 68e4cfa41ee073bfdfed4f7f7cfc9386cecd8129 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Thu, 30 Oct 2025 03:54:35 -0700 Subject: [PATCH 1/5] Pull auto approve into new analyzer --- .../commandLineAnalyzer.ts | 10 +- .../commandLineAutoApproveAnalyzer.ts | 235 +++++++++++++++ .../commandLineFileWriteAnalyzer.ts | 4 +- .../browser/tools/runInTerminalTool.ts | 268 ++++++------------ .../runInTerminalTool.test.ts | 2 +- 5 files changed, 335 insertions(+), 184 deletions(-) create mode 100644 src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineAnalyzer/commandLineAutoApproveAnalyzer.ts diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineAnalyzer/commandLineAnalyzer.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineAnalyzer/commandLineAnalyzer.ts index 01edfddae46..e3f8494b7c0 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineAnalyzer/commandLineAnalyzer.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineAnalyzer/commandLineAnalyzer.ts @@ -3,11 +3,14 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +import type { IMarkdownString } from '../../../../../../../base/common/htmlContent.js'; +import type { IDisposable } from '../../../../../../../base/common/lifecycle.js'; import type { OperatingSystem } from '../../../../../../../base/common/platform.js'; +import type { ToolConfirmationAction } from '../../../../../chat/common/languageModelToolsService.js'; import type { ITerminalInstance } from '../../../../../terminal/browser/terminal.js'; import type { TreeSitterCommandParserLanguage } from '../../treeSitterCommandParser.js'; -export interface ICommandLineAnalyzer { +export interface ICommandLineAnalyzer extends IDisposable { analyze(options: ICommandLineAnalyzerOptions): Promise; } @@ -17,9 +20,12 @@ export interface ICommandLineAnalyzerOptions { shell: string; os: OperatingSystem; treeSitterLanguage: TreeSitterCommandParserLanguage; + terminalToolSessionId: string; } export interface ICommandLineAnalyzerResult { readonly isAutoApproveAllowed: boolean; - readonly disclaimers: string[]; + readonly disclaimers?: readonly string[]; + readonly autoApproveInfo?: IMarkdownString; + readonly customActions?: ToolConfirmationAction[]; } diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineAnalyzer/commandLineAutoApproveAnalyzer.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineAnalyzer/commandLineAutoApproveAnalyzer.ts new file mode 100644 index 00000000000..a5b26284048 --- /dev/null +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineAnalyzer/commandLineAutoApproveAnalyzer.ts @@ -0,0 +1,235 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { asArray } from '../../../../../../../base/common/arrays.js'; +import { createCommandUri, MarkdownString, type IMarkdownString } from '../../../../../../../base/common/htmlContent.js'; +import { Disposable } from '../../../../../../../base/common/lifecycle.js'; +import type { SingleOrMany } from '../../../../../../../base/common/types.js'; +import { localize } from '../../../../../../../nls.js'; +import { IConfigurationService } from '../../../../../../../platform/configuration/common/configuration.js'; +import { IInstantiationService } from '../../../../../../../platform/instantiation/common/instantiation.js'; +import { IStorageService, StorageScope } from '../../../../../../../platform/storage/common/storage.js'; +import { TerminalToolConfirmationStorageKeys } from '../../../../../chat/browser/chatContentParts/toolInvocationParts/chatTerminalToolConfirmationSubPart.js'; +import { openTerminalSettingsLinkCommandId } from '../../../../../chat/browser/chatContentParts/toolInvocationParts/chatTerminalToolProgressPart.js'; +import { ChatConfiguration } from '../../../../../chat/common/constants.js'; +import type { ToolConfirmationAction } from '../../../../../chat/common/languageModelToolsService.js'; +import { TerminalChatAgentToolsSettingId } from '../../../common/terminalChatAgentToolsConfiguration.js'; +import { CommandLineAutoApprover, type IAutoApproveRule, type ICommandApprovalResult, type ICommandApprovalResultWithReason } from '../../commandLineAutoApprover.js'; +import { dedupeRules, generateAutoApproveActions, isPowerShell } from '../../runInTerminalHelpers.js'; +import type { RunInTerminalToolTelemetry } from '../../runInTerminalToolTelemetry.js'; +import { type TreeSitterCommandParser } from '../../treeSitterCommandParser.js'; +import type { ICommandLineAnalyzer, ICommandLineAnalyzerOptions, ICommandLineAnalyzerResult } from './commandLineAnalyzer.js'; + +const promptInjectionWarningCommandsLower = [ + 'curl', + 'wget', +]; +const promptInjectionWarningCommandsLowerPwshOnly = [ + 'invoke-restmethod', + 'invoke-webrequest', + 'irm', + 'iwr', +]; + +export class CommandLineAutoApproveAnalyzer extends Disposable implements ICommandLineAnalyzer { + private readonly _commandLineAutoApprover: CommandLineAutoApprover; + + constructor( + private readonly _treeSitterCommandParser: TreeSitterCommandParser, + private readonly _telemetry: RunInTerminalToolTelemetry, + private readonly _log: (message: string, ...args: unknown[]) => void, + @IConfigurationService private readonly _configurationService: IConfigurationService, + @IInstantiationService instantiationService: IInstantiationService, + @IStorageService private readonly _storageService: IStorageService, + ) { + super(); + this._commandLineAutoApprover = this._register(instantiationService.createInstance(CommandLineAutoApprover)); + } + + async analyze(options: ICommandLineAnalyzerOptions): Promise { + let subCommands: string[] | undefined; + try { + subCommands = await this._treeSitterCommandParser.extractSubCommands(options.treeSitterLanguage, options.commandLine); + this._log(`Parsed sub-commands via ${options.treeSitterLanguage} grammar`, subCommands); + } catch (e) { + console.error(e); + this._log(`Failed to parse sub-commands via ${options.treeSitterLanguage} grammar`); + } + + let isAutoApproved = false; + let autoApproveInfo: IMarkdownString | undefined; + let customActions: ToolConfirmationAction[] | undefined; + + if (!subCommands) { + return { + isAutoApproveAllowed: false, + disclaimers: [], + }; + } + + const subCommandResults = subCommands.map(e => this._commandLineAutoApprover.isCommandAutoApproved(e, options.shell, options.os)); + const commandLineResult = this._commandLineAutoApprover.isCommandLineAutoApproved(options.commandLine); + const autoApproveReasons: string[] = [ + ...subCommandResults.map(e => e.reason), + commandLineResult.reason, + ]; + + let isDenied = false; + let autoApproveReason: 'subCommand' | 'commandLine' | undefined; + let autoApproveDefault: boolean | undefined; + + const deniedSubCommandResult = subCommandResults.find(e => e.result === 'denied'); + if (deniedSubCommandResult) { + this._log('Sub-command DENIED auto approval'); + isDenied = true; + autoApproveDefault = deniedSubCommandResult.rule?.isDefaultRule; + autoApproveReason = 'subCommand'; + } else if (commandLineResult.result === 'denied') { + this._log('Command line DENIED auto approval'); + isDenied = true; + autoApproveDefault = commandLineResult.rule?.isDefaultRule; + autoApproveReason = 'commandLine'; + } else { + if (subCommandResults.every(e => e.result === 'approved')) { + this._log('All sub-commands auto-approved'); + autoApproveReason = 'subCommand'; + isAutoApproved = true; + autoApproveDefault = subCommandResults.every(e => e.rule?.isDefaultRule); + } else { + this._log('All sub-commands NOT auto-approved'); + if (commandLineResult.result === 'approved') { + this._log('Command line auto-approved'); + autoApproveReason = 'commandLine'; + isAutoApproved = true; + autoApproveDefault = commandLineResult.rule?.isDefaultRule; + } else { + this._log('Command line NOT auto-approved'); + } + } + } + + // Log detailed auto approval reasoning + for (const reason of autoApproveReasons) { + this._log(`- ${reason}`); + } + + // Apply auto approval or force it off depending on enablement/opt-in state + const isAutoApproveEnabled = this._configurationService.getValue(TerminalChatAgentToolsSettingId.EnableAutoApprove) === true; + const isAutoApproveWarningAccepted = this._storageService.getBoolean(TerminalToolConfirmationStorageKeys.TerminalAutoApproveWarningAccepted, StorageScope.APPLICATION, false); + if (isAutoApproveEnabled && isAutoApproved) { + autoApproveInfo = this._createAutoApproveInfo( + isAutoApproved, + isDenied, + autoApproveReason, + subCommandResults, + commandLineResult, + ); + } else { + isAutoApproved = false; + } + + // Send telemetry about auto approval process + this._telemetry.logPrepare({ + terminalToolSessionId: options.terminalToolSessionId, + subCommands, + autoApproveAllowed: !isAutoApproveEnabled ? 'off' : isAutoApproveWarningAccepted ? 'allowed' : 'needsOptIn', + autoApproveResult: isAutoApproved ? 'approved' : isDenied ? 'denied' : 'manual', + autoApproveReason, + autoApproveDefault + }); + + // Add disclaimers for various security concerns + const disclaimers: string[] = []; + // disclaimers.push(...commandLineAnalyzerResults.map(e => e.disclaimers).flat()); + + // Prompt injection warning for common commands that return content from the web + const subCommandsLowerFirstWordOnly = subCommands.map(command => command.split(' ')[0].toLowerCase()); + if (!isAutoApproved && ( + subCommandsLowerFirstWordOnly.some(command => promptInjectionWarningCommandsLower.includes(command)) || + (isPowerShell(options.shell, options.os) && subCommandsLowerFirstWordOnly.some(command => promptInjectionWarningCommandsLowerPwshOnly.includes(command))) + )) { + disclaimers.push(localize('runInTerminal.promptInjectionDisclaimer', 'Web content may contain malicious code or attempt prompt injection attacks.')); + } + + if (!isAutoApproved && isAutoApproveEnabled) { + customActions = generateAutoApproveActions(options.commandLine, subCommands, { subCommandResults, commandLineResult }); + } + + return { + isAutoApproveAllowed: isAutoApproved, + disclaimers, + autoApproveInfo, + customActions, + }; + } + + private _createAutoApproveInfo( + isAutoApproved: boolean, + isDenied: boolean, + autoApproveReason: 'subCommand' | 'commandLine' | undefined, + subCommandResults: ICommandApprovalResultWithReason[], + commandLineResult: ICommandApprovalResultWithReason, + ): IMarkdownString | undefined { + const formatRuleLinks = (result: SingleOrMany<{ result: ICommandApprovalResult; rule?: IAutoApproveRule; reason: string }>): string => { + return asArray(result).map(e => { + const settingsUri = createCommandUri(openTerminalSettingsLinkCommandId, e.rule!.sourceTarget); + return `[\`${e.rule!.sourceText}\`](${settingsUri.toString()} "${localize('ruleTooltip', 'View rule in settings')}")`; + }).join(', '); + }; + + const mdTrustSettings = { + isTrusted: { + enabledCommands: [openTerminalSettingsLinkCommandId] + } + }; + + const config = this._configurationService.inspect>(ChatConfiguration.GlobalAutoApprove); + const isGlobalAutoApproved = config?.value ?? config.defaultValue; + if (isGlobalAutoApproved) { + const settingsUri = createCommandUri(openTerminalSettingsLinkCommandId, 'global'); + return new MarkdownString(`${localize('autoApprove.global', 'Auto approved by setting {0}', `[\`${ChatConfiguration.GlobalAutoApprove}\`](${settingsUri.toString()} "${localize('ruleTooltip.global', 'View settings')}")`)}`, mdTrustSettings); + } + + if (isAutoApproved) { + switch (autoApproveReason) { + case 'commandLine': { + if (commandLineResult.rule) { + return new MarkdownString(localize('autoApprove.rule', 'Auto approved by rule {0}', formatRuleLinks(commandLineResult)), mdTrustSettings); + } + break; + } + case 'subCommand': { + const uniqueRules = dedupeRules(subCommandResults); + if (uniqueRules.length === 1) { + return new MarkdownString(localize('autoApprove.rule', 'Auto approved by rule {0}', formatRuleLinks(uniqueRules)), mdTrustSettings); + } else if (uniqueRules.length > 1) { + return new MarkdownString(localize('autoApprove.rules', 'Auto approved by rules {0}', formatRuleLinks(uniqueRules)), mdTrustSettings); + } + break; + } + } + } else if (isDenied) { + switch (autoApproveReason) { + case 'commandLine': { + if (commandLineResult.rule) { + return new MarkdownString(localize('autoApproveDenied.rule', 'Auto approval denied by rule {0}', formatRuleLinks(commandLineResult)), mdTrustSettings); + } + break; + } + case 'subCommand': { + const uniqueRules = dedupeRules(subCommandResults.filter(e => e.result === 'denied')); + if (uniqueRules.length === 1) { + return new MarkdownString(localize('autoApproveDenied.rule', 'Auto approval denied by rule {0}', formatRuleLinks(uniqueRules))); + } else if (uniqueRules.length > 1) { + return new MarkdownString(localize('autoApproveDenied.rules', 'Auto approval denied by rules {0}', formatRuleLinks(uniqueRules))); + } + break; + } + } + } + + return undefined; + } +} diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineAnalyzer/commandLineFileWriteAnalyzer.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineAnalyzer/commandLineFileWriteAnalyzer.ts index bafcd3c939d..bddbdcd9f68 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineAnalyzer/commandLineFileWriteAnalyzer.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineAnalyzer/commandLineFileWriteAnalyzer.ts @@ -3,6 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +import { Disposable } from '../../../../../../../base/common/lifecycle.js'; import { URI } from '../../../../../../../base/common/uri.js'; import { localize } from '../../../../../../../nls.js'; import { IConfigurationService } from '../../../../../../../platform/configuration/common/configuration.js'; @@ -12,7 +13,7 @@ import { TerminalChatAgentToolsSettingId } from '../../../common/terminalChatAge import type { TreeSitterCommandParser } from '../../treeSitterCommandParser.js'; import type { ICommandLineAnalyzer, ICommandLineAnalyzerOptions, ICommandLineAnalyzerResult } from './commandLineAnalyzer.js'; -export class CommandLineFileWriteAnalyzer implements ICommandLineAnalyzer { +export class CommandLineFileWriteAnalyzer extends Disposable implements ICommandLineAnalyzer { constructor( private readonly _treeSitterCommandParser: TreeSitterCommandParser, private readonly _log: (message: string, ...args: unknown[]) => void, @@ -20,6 +21,7 @@ export class CommandLineFileWriteAnalyzer implements ICommandLineAnalyzer { @IHistoryService private readonly _historyService: IHistoryService, @IWorkspaceContextService private readonly _workspaceContextService: IWorkspaceContextService, ) { + super(); } async analyze(options: ICommandLineAnalyzerOptions): Promise { diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts index 32e111b66e2..7d85ab59173 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts @@ -4,18 +4,17 @@ *--------------------------------------------------------------------------------------------*/ import type { IMarker as IXtermMarker } from '@xterm/xterm'; -import { asArray } from '../../../../../../base/common/arrays.js'; import { timeout } from '../../../../../../base/common/async.js'; import { CancellationToken } from '../../../../../../base/common/cancellation.js'; import { Codicon } from '../../../../../../base/common/codicons.js'; import { CancellationError } from '../../../../../../base/common/errors.js'; import { Event } from '../../../../../../base/common/event.js'; -import { createCommandUri, MarkdownString, type IMarkdownString } from '../../../../../../base/common/htmlContent.js'; +import { MarkdownString, type IMarkdownString } from '../../../../../../base/common/htmlContent.js'; import { Disposable, DisposableStore } from '../../../../../../base/common/lifecycle.js'; import { basename } from '../../../../../../base/common/path.js'; import { OperatingSystem, OS } from '../../../../../../base/common/platform.js'; import { count } from '../../../../../../base/common/strings.js'; -import type { DeepImmutable, SingleOrMany } from '../../../../../../base/common/types.js'; +import type { DeepImmutable } from '../../../../../../base/common/types.js'; import { generateUuid } from '../../../../../../base/common/uuid.js'; import { localize } from '../../../../../../nls.js'; import { IConfigurationService } from '../../../../../../platform/configuration/common/configuration.js'; @@ -25,30 +24,28 @@ import { TerminalCapability } from '../../../../../../platform/terminal/common/c import { ITerminalLogService, ITerminalProfile } from '../../../../../../platform/terminal/common/terminal.js'; import { IRemoteAgentService } from '../../../../../services/remote/common/remoteAgentService.js'; import { TerminalToolConfirmationStorageKeys } from '../../../../chat/browser/chatContentParts/toolInvocationParts/chatTerminalToolConfirmationSubPart.js'; -import { openTerminalSettingsLinkCommandId } from '../../../../chat/browser/chatContentParts/toolInvocationParts/chatTerminalToolProgressPart.js'; import { IChatService, type IChatTerminalToolInvocationData } from '../../../../chat/common/chatService.js'; -import { ChatConfiguration } from '../../../../chat/common/constants.js'; -import { CountTokensCallback, ILanguageModelToolsService, IPreparedToolInvocation, IToolData, IToolImpl, IToolInvocation, IToolInvocationPreparationContext, IToolResult, ToolDataSource, ToolInvocationPresentation, ToolProgress, type IToolConfirmationMessages, type ToolConfirmationAction } from '../../../../chat/common/languageModelToolsService.js'; -import { ITerminalService, type ITerminalInstance, ITerminalChatService } from '../../../../terminal/browser/terminal.js'; +import { CountTokensCallback, ILanguageModelToolsService, IPreparedToolInvocation, IToolData, IToolImpl, IToolInvocation, IToolInvocationPreparationContext, IToolResult, ToolDataSource, ToolInvocationPresentation, ToolProgress, type IToolConfirmationMessages } from '../../../../chat/common/languageModelToolsService.js'; +import { ITerminalChatService, ITerminalService, type ITerminalInstance } from '../../../../terminal/browser/terminal.js'; import type { XtermTerminal } from '../../../../terminal/browser/xterm/xtermTerminal.js'; import { ITerminalProfileResolverService } from '../../../../terminal/common/terminal.js'; import { TerminalChatAgentToolsSettingId } from '../../common/terminalChatAgentToolsConfiguration.js'; import { getRecommendedToolsOverRunInTerminal } from '../alternativeRecommendation.js'; -import { CommandLineAutoApprover, type IAutoApproveRule, type ICommandApprovalResult, type ICommandApprovalResultWithReason } from '../commandLineAutoApprover.js'; import { CommandSimplifier } from '../commandSimplifier.js'; import { BasicExecuteStrategy } from '../executeStrategy/basicExecuteStrategy.js'; import type { ITerminalExecuteStrategy } from '../executeStrategy/executeStrategy.js'; import { NoneExecuteStrategy } from '../executeStrategy/noneExecuteStrategy.js'; import { RichExecuteStrategy } from '../executeStrategy/richExecuteStrategy.js'; import { getOutput } from '../outputHelpers.js'; -import { dedupeRules, generateAutoApproveActions, isFish, isPowerShell, isWindowsPowerShell, isZsh } from '../runInTerminalHelpers.js'; +import { isFish, isPowerShell, isWindowsPowerShell, isZsh } from '../runInTerminalHelpers.js'; import { RunInTerminalToolTelemetry } from '../runInTerminalToolTelemetry.js'; import { ShellIntegrationQuality, ToolTerminalCreator, type IToolTerminal } from '../toolTerminalCreator.js'; -import { OutputMonitor } from './monitoring/outputMonitor.js'; -import { IPollingResult, OutputMonitorState } from './monitoring/types.js'; import { TreeSitterCommandParser, TreeSitterCommandParserLanguage } from '../treeSitterCommandParser.js'; import { type ICommandLineAnalyzer, type ICommandLineAnalyzerOptions } from './commandLineAnalyzer/commandLineAnalyzer.js'; +import { CommandLineAutoApproveAnalyzer } from './commandLineAnalyzer/commandLineAutoApproveAnalyzer.js'; import { CommandLineFileWriteAnalyzer } from './commandLineAnalyzer/commandLineFileWriteAnalyzer.js'; +import { OutputMonitor } from './monitoring/outputMonitor.js'; +import { IPollingResult, OutputMonitorState } from './monitoring/types.js'; // #region Tool data @@ -247,16 +244,6 @@ const telemetryIgnoredSequences = [ '\x1b[O', // Focus out ]; -const promptInjectionWarningCommandsLower = [ - 'curl', - 'wget', -]; -const promptInjectionWarningCommandsLowerPwshOnly = [ - 'invoke-restmethod', - 'invoke-webrequest', - 'irm', - 'iwr', -]; export class RunInTerminalTool extends Disposable implements IToolImpl { @@ -265,7 +252,6 @@ export class RunInTerminalTool extends Disposable implements IToolImpl { private readonly _treeSitterCommandParser: TreeSitterCommandParser; private readonly _telemetry: RunInTerminalToolTelemetry; private readonly _commandLineAnalyzers: ICommandLineAnalyzer[]; - protected readonly _commandLineAutoApprover: CommandLineAutoApprover; protected readonly _profileFetcher: TerminalProfileFetcher; protected readonly _sessionTerminalAssociations: Map = new Map(); @@ -301,9 +287,9 @@ export class RunInTerminalTool extends Disposable implements IToolImpl { this._commandSimplifier = _instantiationService.createInstance(CommandSimplifier, this._osBackend, this._treeSitterCommandParser); this._telemetry = _instantiationService.createInstance(RunInTerminalToolTelemetry); this._commandLineAnalyzers = [ - this._instantiationService.createInstance(CommandLineFileWriteAnalyzer, this._treeSitterCommandParser, (message, args) => this._logService.info(`CommandLineFileWriteAnalyzer: ${message}`, args)), + this._register(this._instantiationService.createInstance(CommandLineFileWriteAnalyzer, this._treeSitterCommandParser, (message, args) => this._logService.info(`RunInTerminalTool#CommandLineFileWriteAnalyzer: ${message}`, args))), + this._register(this._instantiationService.createInstance(CommandLineAutoApproveAnalyzer, this._treeSitterCommandParser, this._telemetry, (message, args) => this._logService.info(`RunInTerminalTool#CommandLineAutoApproveAnalyzer: ${message}`, args))), ]; - this._commandLineAutoApprover = this._register(_instantiationService.createInstance(CommandLineAutoApprover)); this._profileFetcher = _instantiationService.createInstance(TerminalProfileFetcher); // Clear out warning accepted state if the setting is disabled @@ -359,13 +345,9 @@ export class RunInTerminalTool extends Disposable implements IToolImpl { // commands that would be auto approved if it were enabled. const actualCommand = toolEditedCommand ?? args.command; - let disclaimer: IMarkdownString | undefined; - let customActions: ToolConfirmationAction[] | undefined; - const isAutoApproveEnabled = this._configurationService.getValue(TerminalChatAgentToolsSettingId.EnableAutoApprove) === true; const isAutoApproveWarningAccepted = this._storageService.getBoolean(TerminalToolConfirmationStorageKeys.TerminalAutoApproveWarningAccepted, StorageScope.APPLICATION, false); const isAutoApproveAllowed = isAutoApproveEnabled && isAutoApproveWarningAccepted; - let isAutoApproved = false; let subCommands: string[] | undefined; const treeSitterLanguage = isPowerShell(shell, os) ? TreeSitterCommandParserLanguage.PowerShell : TreeSitterCommandParserLanguage.Bash; @@ -383,102 +365,28 @@ export class RunInTerminalTool extends Disposable implements IToolImpl { os, shell, treeSitterLanguage, + terminalToolSessionId, }; const commandLineAnalyzerResults = await Promise.all(this._commandLineAnalyzers.map(e => e.analyze(commandLineAnalyzerOptions))); - if (subCommands) { - const subCommandResults = subCommands.map(e => this._commandLineAutoApprover.isCommandAutoApproved(e, shell, os)); - const commandLineResult = this._commandLineAutoApprover.isCommandLineAutoApproved(actualCommand); - const autoApproveReasons: string[] = [ - ...subCommandResults.map(e => e.reason), - commandLineResult.reason, - ]; + // TODO: Should this require auto approve and veto instead? + const isAutoApproved = commandLineAnalyzerResults.every(e => e.isAutoApproveAllowed); - let isDenied = false; - let autoApproveReason: 'subCommand' | 'commandLine' | undefined; - let autoApproveDefault: boolean | undefined; + // Add disclaimers for various security concerns + const disclaimers: string[] = []; + disclaimers.push(...commandLineAnalyzerResults.map(e => e.disclaimers ?? []).flat()); - const deniedSubCommandResult = subCommandResults.find(e => e.result === 'denied'); - if (deniedSubCommandResult) { - this._logService.info('RunInTerminalTool: autoApprove: Sub-command DENIED auto approval'); - isDenied = true; - autoApproveDefault = deniedSubCommandResult.rule?.isDefaultRule; - autoApproveReason = 'subCommand'; - } else if (commandLineResult.result === 'denied') { - this._logService.info('RunInTerminalTool: autoApprove: Command line DENIED auto approval'); - isDenied = true; - autoApproveDefault = commandLineResult.rule?.isDefaultRule; - autoApproveReason = 'commandLine'; - } else { - if (subCommandResults.every(e => e.result === 'approved')) { - this._logService.info('RunInTerminalTool: autoApprove: All sub-commands auto-approved'); - autoApproveReason = 'subCommand'; - isAutoApproved = true; - autoApproveDefault = subCommandResults.every(e => e.rule?.isDefaultRule); - } else { - this._logService.info('RunInTerminalTool: autoApprove: All sub-commands NOT auto-approved'); - if (commandLineResult.result === 'approved') { - this._logService.info('RunInTerminalTool: autoApprove: Command line auto-approved'); - autoApproveReason = 'commandLine'; - isAutoApproved = true; - autoApproveDefault = commandLineResult.rule?.isDefaultRule; - } else { - this._logService.info('RunInTerminalTool: autoApprove: Command line NOT auto-approved'); - } - } - } - - // Log detailed auto approval reasoning - for (const reason of autoApproveReasons) { - this._logService.info(`RunInTerminalTool: autoApprove: - ${reason}`); - } - - // Apply auto approval or force it off depending on enablement/opt-in state - if (isAutoApproveEnabled && commandLineAnalyzerResults.every(r => r.isAutoApproveAllowed)) { - autoApproveInfo = this._createAutoApproveInfo( - isAutoApproved, - isDenied, - autoApproveReason, - subCommandResults, - commandLineResult, - ); - } else { - isAutoApproved = false; - } - - // Send telemetry about auto approval process - this._telemetry.logPrepare({ - terminalToolSessionId, - subCommands, - autoApproveAllowed: !isAutoApproveEnabled ? 'off' : isAutoApproveWarningAccepted ? 'allowed' : 'needsOptIn', - autoApproveResult: isAutoApproved ? 'approved' : isDenied ? 'denied' : 'manual', - autoApproveReason, - autoApproveDefault - }); - - // Add disclaimers for various security concerns - const disclaimers: string[] = []; - disclaimers.push(...commandLineAnalyzerResults.map(e => e.disclaimers).flat()); - - // Prompt injection warning for common commands that return content from the web - const subCommandsLowerFirstWordOnly = subCommands.map(command => command.split(' ')[0].toLowerCase()); - if (!isAutoApproved && ( - subCommandsLowerFirstWordOnly.some(command => promptInjectionWarningCommandsLower.includes(command)) || - (isPowerShell(shell, os) && subCommandsLowerFirstWordOnly.some(command => promptInjectionWarningCommandsLowerPwshOnly.includes(command))) - )) { - disclaimers.push(localize('runInTerminal.promptInjectionDisclaimer', 'Web content may contain malicious code or attempt prompt injection attacks.')); - } - - // Combine disclaimers - if (disclaimers.length > 0) { - disclaimer = new MarkdownString(`$(${Codicon.info.id}) ` + disclaimers.join(' '), { supportThemeIcons: true }); - } - - if (!isAutoApproved && isAutoApproveEnabled) { - customActions = generateAutoApproveActions(actualCommand, subCommands, { subCommandResults, commandLineResult }); - } + // Combine disclaimers + let disclaimer: IMarkdownString | undefined; + if (disclaimers.length > 0) { + disclaimer = new MarkdownString(`$(${Codicon.info.id}) ` + disclaimers.join(' '), { supportThemeIcons: true }); } + const customActions = commandLineAnalyzerResults.map(e => e.customActions ?? []).flat(); + + // TODO: This isn't great + autoApproveInfo = commandLineAnalyzerResults.find(e => e.autoApproveInfo)?.autoApproveInfo; + let shellType = basename(shell, '.exe'); if (shellType === 'powershell') { shellType = 'pwsh'; @@ -925,73 +833,73 @@ export class RunInTerminalTool extends Disposable implements IToolImpl { // #region Auto approve - private _createAutoApproveInfo( - isAutoApproved: boolean, - isDenied: boolean, - autoApproveReason: 'subCommand' | 'commandLine' | undefined, - subCommandResults: ICommandApprovalResultWithReason[], - commandLineResult: ICommandApprovalResultWithReason, - ): MarkdownString | undefined { - const formatRuleLinks = (result: SingleOrMany<{ result: ICommandApprovalResult; rule?: IAutoApproveRule; reason: string }>): string => { - return asArray(result).map(e => { - const settingsUri = createCommandUri(openTerminalSettingsLinkCommandId, e.rule!.sourceTarget); - return `[\`${e.rule!.sourceText}\`](${settingsUri.toString()} "${localize('ruleTooltip', 'View rule in settings')}")`; - }).join(', '); - }; + // private _createAutoApproveInfo( + // isAutoApproved: boolean, + // isDenied: boolean, + // autoApproveReason: 'subCommand' | 'commandLine' | undefined, + // subCommandResults: ICommandApprovalResultWithReason[], + // commandLineResult: ICommandApprovalResultWithReason, + // ): MarkdownString | undefined { + // const formatRuleLinks = (result: SingleOrMany<{ result: ICommandApprovalResult; rule?: IAutoApproveRule; reason: string }>): string => { + // return asArray(result).map(e => { + // const settingsUri = createCommandUri(openTerminalSettingsLinkCommandId, e.rule!.sourceTarget); + // return `[\`${e.rule!.sourceText}\`](${settingsUri.toString()} "${localize('ruleTooltip', 'View rule in settings')}")`; + // }).join(', '); + // }; - const mdTrustSettings = { - isTrusted: { - enabledCommands: [openTerminalSettingsLinkCommandId] - } - }; + // const mdTrustSettings = { + // isTrusted: { + // enabledCommands: [openTerminalSettingsLinkCommandId] + // } + // }; - const config = this._configurationService.inspect>(ChatConfiguration.GlobalAutoApprove); - const isGlobalAutoApproved = config?.value ?? config.defaultValue; - if (isGlobalAutoApproved) { - const settingsUri = createCommandUri(openTerminalSettingsLinkCommandId, 'global'); - return new MarkdownString(`${localize('autoApprove.global', 'Auto approved by setting {0}', `[\`${ChatConfiguration.GlobalAutoApprove}\`](${settingsUri.toString()} "${localize('ruleTooltip.global', 'View settings')}")`)}`, mdTrustSettings); - } + // const config = this._configurationService.inspect>(ChatConfiguration.GlobalAutoApprove); + // const isGlobalAutoApproved = config?.value ?? config.defaultValue; + // if (isGlobalAutoApproved) { + // const settingsUri = createCommandUri(openTerminalSettingsLinkCommandId, 'global'); + // return new MarkdownString(`${localize('autoApprove.global', 'Auto approved by setting {0}', `[\`${ChatConfiguration.GlobalAutoApprove}\`](${settingsUri.toString()} "${localize('ruleTooltip.global', 'View settings')}")`)}`, mdTrustSettings); + // } - if (isAutoApproved) { - switch (autoApproveReason) { - case 'commandLine': { - if (commandLineResult.rule) { - return new MarkdownString(localize('autoApprove.rule', 'Auto approved by rule {0}', formatRuleLinks(commandLineResult)), mdTrustSettings); - } - break; - } - case 'subCommand': { - const uniqueRules = dedupeRules(subCommandResults); - if (uniqueRules.length === 1) { - return new MarkdownString(localize('autoApprove.rule', 'Auto approved by rule {0}', formatRuleLinks(uniqueRules)), mdTrustSettings); - } else if (uniqueRules.length > 1) { - return new MarkdownString(localize('autoApprove.rules', 'Auto approved by rules {0}', formatRuleLinks(uniqueRules)), mdTrustSettings); - } - break; - } - } - } else if (isDenied) { - switch (autoApproveReason) { - case 'commandLine': { - if (commandLineResult.rule) { - return new MarkdownString(localize('autoApproveDenied.rule', 'Auto approval denied by rule {0}', formatRuleLinks(commandLineResult)), mdTrustSettings); - } - break; - } - case 'subCommand': { - const uniqueRules = dedupeRules(subCommandResults.filter(e => e.result === 'denied')); - if (uniqueRules.length === 1) { - return new MarkdownString(localize('autoApproveDenied.rule', 'Auto approval denied by rule {0}', formatRuleLinks(uniqueRules))); - } else if (uniqueRules.length > 1) { - return new MarkdownString(localize('autoApproveDenied.rules', 'Auto approval denied by rules {0}', formatRuleLinks(uniqueRules))); - } - break; - } - } - } + // if (isAutoApproved) { + // switch (autoApproveReason) { + // case 'commandLine': { + // if (commandLineResult.rule) { + // return new MarkdownString(localize('autoApprove.rule', 'Auto approved by rule {0}', formatRuleLinks(commandLineResult)), mdTrustSettings); + // } + // break; + // } + // case 'subCommand': { + // const uniqueRules = dedupeRules(subCommandResults); + // if (uniqueRules.length === 1) { + // return new MarkdownString(localize('autoApprove.rule', 'Auto approved by rule {0}', formatRuleLinks(uniqueRules)), mdTrustSettings); + // } else if (uniqueRules.length > 1) { + // return new MarkdownString(localize('autoApprove.rules', 'Auto approved by rules {0}', formatRuleLinks(uniqueRules)), mdTrustSettings); + // } + // break; + // } + // } + // } else if (isDenied) { + // switch (autoApproveReason) { + // case 'commandLine': { + // if (commandLineResult.rule) { + // return new MarkdownString(localize('autoApproveDenied.rule', 'Auto approval denied by rule {0}', formatRuleLinks(commandLineResult)), mdTrustSettings); + // } + // break; + // } + // case 'subCommand': { + // const uniqueRules = dedupeRules(subCommandResults.filter(e => e.result === 'denied')); + // if (uniqueRules.length === 1) { + // return new MarkdownString(localize('autoApproveDenied.rule', 'Auto approval denied by rule {0}', formatRuleLinks(uniqueRules))); + // } else if (uniqueRules.length > 1) { + // return new MarkdownString(localize('autoApproveDenied.rules', 'Auto approval denied by rules {0}', formatRuleLinks(uniqueRules))); + // } + // break; + // } + // } + // } - return undefined; - } + // return undefined; + // } // #endregion } diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/electron-browser/runInTerminalTool.test.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/electron-browser/runInTerminalTool.test.ts index 0b2958cc989..f5b22542405 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/electron-browser/runInTerminalTool.test.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/electron-browser/runInTerminalTool.test.ts @@ -36,7 +36,7 @@ import { arch } from '../../../../../../base/common/process.js'; class TestRunInTerminalTool extends RunInTerminalTool { protected override _osBackend: Promise = Promise.resolve(OperatingSystem.Windows); - get commandLineAutoApprover() { return this._commandLineAutoApprover; } + // get commandLineAutoApprover() { return this._commandLineAutoApprover; } get sessionTerminalAssociations() { return this._sessionTerminalAssociations; } get profileFetcher() { return this._profileFetcher; } From d32bd36721346edc37e2f88909a37ba66da25ea0 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Thu, 30 Oct 2025 04:00:58 -0700 Subject: [PATCH 2/5] More simplification of terminal tool prepare call --- .../commandLineAutoApproveAnalyzer.ts | 5 +- .../browser/tools/runInTerminalTool.ts | 143 ++++++++---------- 2 files changed, 66 insertions(+), 82 deletions(-) diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineAnalyzer/commandLineAutoApproveAnalyzer.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineAnalyzer/commandLineAutoApproveAnalyzer.ts index a5b26284048..c4b9810bb00 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineAnalyzer/commandLineAutoApproveAnalyzer.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineAnalyzer/commandLineAutoApproveAnalyzer.ts @@ -140,11 +140,8 @@ export class CommandLineAutoApproveAnalyzer extends Disposable implements IComma autoApproveDefault }); - // Add disclaimers for various security concerns - const disclaimers: string[] = []; - // disclaimers.push(...commandLineAnalyzerResults.map(e => e.disclaimers).flat()); - // Prompt injection warning for common commands that return content from the web + const disclaimers: string[] = []; const subCommandsLowerFirstWordOnly = subCommands.map(command => command.split(' ')[0].toLowerCase()); if (!isAutoApproved && ( subCommandsLowerFirstWordOnly.some(command => promptInjectionWarningCommandsLower.includes(command)) || diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts index 7d85ab59173..bf011b0e93f 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts @@ -25,7 +25,7 @@ import { ITerminalLogService, ITerminalProfile } from '../../../../../../platfor import { IRemoteAgentService } from '../../../../../services/remote/common/remoteAgentService.js'; import { TerminalToolConfirmationStorageKeys } from '../../../../chat/browser/chatContentParts/toolInvocationParts/chatTerminalToolConfirmationSubPart.js'; import { IChatService, type IChatTerminalToolInvocationData } from '../../../../chat/common/chatService.js'; -import { CountTokensCallback, ILanguageModelToolsService, IPreparedToolInvocation, IToolData, IToolImpl, IToolInvocation, IToolInvocationPreparationContext, IToolResult, ToolDataSource, ToolInvocationPresentation, ToolProgress, type IToolConfirmationMessages } from '../../../../chat/common/languageModelToolsService.js'; +import { CountTokensCallback, ILanguageModelToolsService, IPreparedToolInvocation, IToolData, IToolImpl, IToolInvocation, IToolInvocationPreparationContext, IToolResult, ToolDataSource, ToolInvocationPresentation, ToolProgress } from '../../../../chat/common/languageModelToolsService.js'; import { ITerminalChatService, ITerminalService, type ITerminalInstance } from '../../../../terminal/browser/terminal.js'; import type { XtermTerminal } from '../../../../terminal/browser/xterm/xtermTerminal.js'; import { ITerminalProfileResolverService } from '../../../../terminal/common/terminal.js'; @@ -320,9 +320,6 @@ export class RunInTerminalTool extends Disposable implements IToolImpl { async prepareToolInvocation(context: IToolInvocationPreparationContext, token: CancellationToken): Promise { const args = context.parameters as IRunInTerminalInputParams; - const alternativeRecommendation = getRecommendedToolsOverRunInTerminal(args.command, this._languageModelToolsService); - const presentation = alternativeRecommendation ? ToolInvocationPresentation.Hidden : undefined; - const os = await this._osBackend; const shell = await this._profileFetcher.getCopilotShell(); const language = os === OperatingSystem.Windows ? 'pwsh' : 'sh'; @@ -334,87 +331,77 @@ export class RunInTerminalTool extends Disposable implements IToolImpl { if (toolEditedCommand === args.command) { toolEditedCommand = undefined; } + const toolSpecificData: IChatTerminalToolInvocationData = { + kind: 'terminal', + terminalToolSessionId, + commandLine: { + original: args.command, + toolEdited: toolEditedCommand + }, + language, + }; - let autoApproveInfo: IMarkdownString | undefined; - let confirmationMessages: IToolConfirmationMessages | undefined; + // HACK: Exit early if there's an alternative recommendation, this is a little hacky but + // it's the current mechanism for re-routing terminal tool calls to something else. + const alternativeRecommendation = getRecommendedToolsOverRunInTerminal(args.command, this._languageModelToolsService); if (alternativeRecommendation) { - confirmationMessages = undefined; - } else { - // Determine auto approval, this happens even when auto approve is off to that reasoning - // can be reviewed in the terminal channel. It also allows gauging the effective set of - // commands that would be auto approved if it were enabled. - const actualCommand = toolEditedCommand ?? args.command; - - const isAutoApproveEnabled = this._configurationService.getValue(TerminalChatAgentToolsSettingId.EnableAutoApprove) === true; - const isAutoApproveWarningAccepted = this._storageService.getBoolean(TerminalToolConfirmationStorageKeys.TerminalAutoApproveWarningAccepted, StorageScope.APPLICATION, false); - const isAutoApproveAllowed = isAutoApproveEnabled && isAutoApproveWarningAccepted; - - let subCommands: string[] | undefined; - const treeSitterLanguage = isPowerShell(shell, os) ? TreeSitterCommandParserLanguage.PowerShell : TreeSitterCommandParserLanguage.Bash; - try { - subCommands = await this._treeSitterCommandParser.extractSubCommands(treeSitterLanguage, actualCommand); - this._logService.info(`RunInTerminalTool: autoApprove: Parsed sub-commands via ${treeSitterLanguage} grammar`, subCommands); - } catch (e) { - console.error(e); - this._logService.info(`RunInTerminalTool: autoApprove: Failed to parse sub-commands via ${treeSitterLanguage} grammar`); - } - - const commandLineAnalyzerOptions: ICommandLineAnalyzerOptions = { - instance, - commandLine: actualCommand, - os, - shell, - treeSitterLanguage, - terminalToolSessionId, - }; - const commandLineAnalyzerResults = await Promise.all(this._commandLineAnalyzers.map(e => e.analyze(commandLineAnalyzerOptions))); - - // TODO: Should this require auto approve and veto instead? - const isAutoApproved = commandLineAnalyzerResults.every(e => e.isAutoApproveAllowed); - - // Add disclaimers for various security concerns - const disclaimers: string[] = []; - disclaimers.push(...commandLineAnalyzerResults.map(e => e.disclaimers ?? []).flat()); - - // Combine disclaimers - let disclaimer: IMarkdownString | undefined; - if (disclaimers.length > 0) { - disclaimer = new MarkdownString(`$(${Codicon.info.id}) ` + disclaimers.join(' '), { supportThemeIcons: true }); - } - - const customActions = commandLineAnalyzerResults.map(e => e.customActions ?? []).flat(); - - // TODO: This isn't great - autoApproveInfo = commandLineAnalyzerResults.find(e => e.autoApproveInfo)?.autoApproveInfo; - - let shellType = basename(shell, '.exe'); - if (shellType === 'powershell') { - shellType = 'pwsh'; - } - confirmationMessages = (isAutoApproved && isAutoApproveAllowed && commandLineAnalyzerResults.every(r => r.isAutoApproveAllowed)) ? undefined : { - title: args.isBackground - ? localize('runInTerminal.background', "Run `{0}` command? (background terminal)", shellType) - : localize('runInTerminal', "Run `{0}` command?", shellType), - message: new MarkdownString(args.explanation), - disclaimer, - terminalCustomActions: customActions, + toolSpecificData.alternativeRecommendation = alternativeRecommendation; + return { + confirmationMessages: undefined, + presentation: ToolInvocationPresentation.Hidden, + toolSpecificData, }; } + // Determine auto approval, this happens even when auto approve is off to that reasoning + // can be reviewed in the terminal channel. It also allows gauging the effective set of + // commands that would be auto approved if it were enabled. + const actualCommand = toolEditedCommand ?? args.command; + + const isAutoApproveEnabled = this._configurationService.getValue(TerminalChatAgentToolsSettingId.EnableAutoApprove) === true; + const isAutoApproveWarningAccepted = this._storageService.getBoolean(TerminalToolConfirmationStorageKeys.TerminalAutoApproveWarningAccepted, StorageScope.APPLICATION, false); + const isAutoApproveAllowed = isAutoApproveEnabled && isAutoApproveWarningAccepted; + + const commandLineAnalyzerOptions: ICommandLineAnalyzerOptions = { + instance, + commandLine: actualCommand, + os, + shell, + treeSitterLanguage: isPowerShell(shell, os) ? TreeSitterCommandParserLanguage.PowerShell : TreeSitterCommandParserLanguage.Bash, + terminalToolSessionId, + }; + const commandLineAnalyzerResults = await Promise.all(this._commandLineAnalyzers.map(e => e.analyze(commandLineAnalyzerOptions))); + + // Add disclaimers for various security concerns + const disclaimers: string[] = []; + disclaimers.push(...commandLineAnalyzerResults.map(e => e.disclaimers ?? []).flat()); + + // Combine disclaimers + let disclaimer: IMarkdownString | undefined; + if (disclaimers.length > 0) { + disclaimer = new MarkdownString(`$(${Codicon.info.id}) ` + disclaimers.join(' '), { supportThemeIcons: true }); + } + + const isAutoApproved = commandLineAnalyzerResults.every(e => e.isAutoApproveAllowed); + const customActions = commandLineAnalyzerResults.map(e => e.customActions ?? []).flat(); + toolSpecificData.autoApproveInfo = commandLineAnalyzerResults.find(e => e.autoApproveInfo)?.autoApproveInfo; + + let shellType = basename(shell, '.exe'); + if (shellType === 'powershell') { + shellType = 'pwsh'; + } + const confirmationMessages = (isAutoApproved && isAutoApproveAllowed && commandLineAnalyzerResults.every(r => r.isAutoApproveAllowed)) ? undefined : { + title: args.isBackground + ? localize('runInTerminal.background', "Run `{0}` command? (background terminal)", shellType) + : localize('runInTerminal', "Run `{0}` command?", shellType), + message: new MarkdownString(args.explanation), + disclaimer, + terminalCustomActions: customActions, + }; + return { confirmationMessages, - presentation, - toolSpecificData: { - kind: 'terminal', - terminalToolSessionId, - commandLine: { - original: args.command, - toolEdited: toolEditedCommand - }, - language, - alternativeRecommendation, - autoApproveInfo, - } + toolSpecificData, }; } From 62f86ba471a8a3ae5e5508cdfd13ec97dbe390c3 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Thu, 30 Oct 2025 04:36:04 -0700 Subject: [PATCH 3/5] Further simplify code flow --- .../browser/tools/runInTerminalTool.ts | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts index bf011b0e93f..3074815f1b8 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts @@ -356,7 +356,7 @@ export class RunInTerminalTool extends Disposable implements IToolImpl { // Determine auto approval, this happens even when auto approve is off to that reasoning // can be reviewed in the terminal channel. It also allows gauging the effective set of // commands that would be auto approved if it were enabled. - const actualCommand = toolEditedCommand ?? args.command; + const commandLine = toolEditedCommand ?? args.command; const isAutoApproveEnabled = this._configurationService.getValue(TerminalChatAgentToolsSettingId.EnableAutoApprove) === true; const isAutoApproveWarningAccepted = this._storageService.getBoolean(TerminalToolConfirmationStorageKeys.TerminalAutoApproveWarningAccepted, StorageScope.APPLICATION, false); @@ -364,7 +364,7 @@ export class RunInTerminalTool extends Disposable implements IToolImpl { const commandLineAnalyzerOptions: ICommandLineAnalyzerOptions = { instance, - commandLine: actualCommand, + commandLine, os, shell, treeSitterLanguage: isPowerShell(shell, os) ? TreeSitterCommandParserLanguage.PowerShell : TreeSitterCommandParserLanguage.Bash, @@ -372,17 +372,12 @@ export class RunInTerminalTool extends Disposable implements IToolImpl { }; const commandLineAnalyzerResults = await Promise.all(this._commandLineAnalyzers.map(e => e.analyze(commandLineAnalyzerOptions))); - // Add disclaimers for various security concerns - const disclaimers: string[] = []; - disclaimers.push(...commandLineAnalyzerResults.map(e => e.disclaimers ?? []).flat()); - - // Combine disclaimers + const disclaimersRaw = commandLineAnalyzerResults.map(e => e.disclaimers ?? []).flat(); let disclaimer: IMarkdownString | undefined; - if (disclaimers.length > 0) { - disclaimer = new MarkdownString(`$(${Codicon.info.id}) ` + disclaimers.join(' '), { supportThemeIcons: true }); + if (disclaimersRaw.length > 0) { + disclaimer = new MarkdownString(`$(${Codicon.info.id}) ` + disclaimersRaw.join(' '), { supportThemeIcons: true }); } - const isAutoApproved = commandLineAnalyzerResults.every(e => e.isAutoApproveAllowed); const customActions = commandLineAnalyzerResults.map(e => e.customActions ?? []).flat(); toolSpecificData.autoApproveInfo = commandLineAnalyzerResults.find(e => e.autoApproveInfo)?.autoApproveInfo; @@ -390,7 +385,9 @@ export class RunInTerminalTool extends Disposable implements IToolImpl { if (shellType === 'powershell') { shellType = 'pwsh'; } - const confirmationMessages = (isAutoApproved && isAutoApproveAllowed && commandLineAnalyzerResults.every(r => r.isAutoApproveAllowed)) ? undefined : { + + const isFinalAutoApproved = isAutoApproveAllowed && commandLineAnalyzerResults.every(e => e.isAutoApproveAllowed); + const confirmationMessages = isFinalAutoApproved ? undefined : { title: args.isBackground ? localize('runInTerminal.background', "Run `{0}` command? (background terminal)", shellType) : localize('runInTerminal', "Run `{0}` command?", shellType), From 7dcd13ac70adb364eba40b95fd2817e1f8dcc7b0 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Thu, 30 Oct 2025 04:37:39 -0700 Subject: [PATCH 4/5] Remove now unused comments --- .../browser/tools/runInTerminalTool.ts | 72 ------------------- .../runInTerminalTool.test.ts | 1 - 2 files changed, 73 deletions(-) diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts index 3074815f1b8..caae5e741c0 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts @@ -814,78 +814,6 @@ export class RunInTerminalTool extends Disposable implements IToolImpl { } // #endregion - - // #region Auto approve - - // private _createAutoApproveInfo( - // isAutoApproved: boolean, - // isDenied: boolean, - // autoApproveReason: 'subCommand' | 'commandLine' | undefined, - // subCommandResults: ICommandApprovalResultWithReason[], - // commandLineResult: ICommandApprovalResultWithReason, - // ): MarkdownString | undefined { - // const formatRuleLinks = (result: SingleOrMany<{ result: ICommandApprovalResult; rule?: IAutoApproveRule; reason: string }>): string => { - // return asArray(result).map(e => { - // const settingsUri = createCommandUri(openTerminalSettingsLinkCommandId, e.rule!.sourceTarget); - // return `[\`${e.rule!.sourceText}\`](${settingsUri.toString()} "${localize('ruleTooltip', 'View rule in settings')}")`; - // }).join(', '); - // }; - - // const mdTrustSettings = { - // isTrusted: { - // enabledCommands: [openTerminalSettingsLinkCommandId] - // } - // }; - - // const config = this._configurationService.inspect>(ChatConfiguration.GlobalAutoApprove); - // const isGlobalAutoApproved = config?.value ?? config.defaultValue; - // if (isGlobalAutoApproved) { - // const settingsUri = createCommandUri(openTerminalSettingsLinkCommandId, 'global'); - // return new MarkdownString(`${localize('autoApprove.global', 'Auto approved by setting {0}', `[\`${ChatConfiguration.GlobalAutoApprove}\`](${settingsUri.toString()} "${localize('ruleTooltip.global', 'View settings')}")`)}`, mdTrustSettings); - // } - - // if (isAutoApproved) { - // switch (autoApproveReason) { - // case 'commandLine': { - // if (commandLineResult.rule) { - // return new MarkdownString(localize('autoApprove.rule', 'Auto approved by rule {0}', formatRuleLinks(commandLineResult)), mdTrustSettings); - // } - // break; - // } - // case 'subCommand': { - // const uniqueRules = dedupeRules(subCommandResults); - // if (uniqueRules.length === 1) { - // return new MarkdownString(localize('autoApprove.rule', 'Auto approved by rule {0}', formatRuleLinks(uniqueRules)), mdTrustSettings); - // } else if (uniqueRules.length > 1) { - // return new MarkdownString(localize('autoApprove.rules', 'Auto approved by rules {0}', formatRuleLinks(uniqueRules)), mdTrustSettings); - // } - // break; - // } - // } - // } else if (isDenied) { - // switch (autoApproveReason) { - // case 'commandLine': { - // if (commandLineResult.rule) { - // return new MarkdownString(localize('autoApproveDenied.rule', 'Auto approval denied by rule {0}', formatRuleLinks(commandLineResult)), mdTrustSettings); - // } - // break; - // } - // case 'subCommand': { - // const uniqueRules = dedupeRules(subCommandResults.filter(e => e.result === 'denied')); - // if (uniqueRules.length === 1) { - // return new MarkdownString(localize('autoApproveDenied.rule', 'Auto approval denied by rule {0}', formatRuleLinks(uniqueRules))); - // } else if (uniqueRules.length > 1) { - // return new MarkdownString(localize('autoApproveDenied.rules', 'Auto approval denied by rules {0}', formatRuleLinks(uniqueRules))); - // } - // break; - // } - // } - // } - - // return undefined; - // } - - // #endregion } class BackgroundTerminalExecution extends Disposable { diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/electron-browser/runInTerminalTool.test.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/electron-browser/runInTerminalTool.test.ts index f5b22542405..995f72a352a 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/electron-browser/runInTerminalTool.test.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/electron-browser/runInTerminalTool.test.ts @@ -36,7 +36,6 @@ import { arch } from '../../../../../../base/common/process.js'; class TestRunInTerminalTool extends RunInTerminalTool { protected override _osBackend: Promise = Promise.resolve(OperatingSystem.Windows); - // get commandLineAutoApprover() { return this._commandLineAutoApprover; } get sessionTerminalAssociations() { return this._sessionTerminalAssociations; } get profileFetcher() { return this._profileFetcher; } From cbe79e3b173ee3503acea54559e04b2a50241c1d Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Thu, 30 Oct 2025 04:40:48 -0700 Subject: [PATCH 5/5] Simplify disclaimer mapping suggestion --- .../chatAgentTools/browser/tools/runInTerminalTool.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts index caae5e741c0..0dd4a1779f6 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts @@ -372,7 +372,7 @@ export class RunInTerminalTool extends Disposable implements IToolImpl { }; const commandLineAnalyzerResults = await Promise.all(this._commandLineAnalyzers.map(e => e.analyze(commandLineAnalyzerOptions))); - const disclaimersRaw = commandLineAnalyzerResults.map(e => e.disclaimers ?? []).flat(); + const disclaimersRaw = commandLineAnalyzerResults.filter(e => e.disclaimers).flatMap(e => e.disclaimers); let disclaimer: IMarkdownString | undefined; if (disclaimersRaw.length > 0) { disclaimer = new MarkdownString(`$(${Codicon.info.id}) ` + disclaimersRaw.join(' '), { supportThemeIcons: true });