Skip to content

Commit 5528af5

Browse files
authored
Merge pull request microsoft#264220 from microsoft/tyriar/global
Warn about global auto approve, rename setting
2 parents b725ded + 78528c7 commit 5528af5

File tree

7 files changed

+97
-25
lines changed

7 files changed

+97
-25
lines changed

src/vs/workbench/contrib/chat/browser/actions/chatAccessibilityHelp.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import { IKeybindingService } from '../../../../../platform/keybinding/common/ke
1515
import { AccessibilityVerbositySettingId } from '../../../accessibility/browser/accessibilityConfiguration.js';
1616
import { INLINE_CHAT_ID } from '../../../inlineChat/common/inlineChat.js';
1717
import { ChatContextKeyExprs, ChatContextKeys } from '../../common/chatContextKeys.js';
18-
import { ChatAgentLocation, ChatModeKind } from '../../common/constants.js';
18+
import { ChatAgentLocation, ChatConfiguration, ChatModeKind } from '../../common/constants.js';
1919
import { IChatWidgetService } from '../chat.js';
2020

2121
export class PanelChatAccessibilityHelp implements IAccessibleViewImplementation {
@@ -99,7 +99,7 @@ export function getAccessibilityHelpText(type: 'panelChat' | 'inlineChat' | 'qui
9999
if (type === 'agentView') {
100100
content.push(localize('chatAgent.userActionRequired', 'An alert will indicate when user action is required. For example, if the agent wants to run something in the terminal, you will hear Action Required: Run Command in Terminal.'));
101101
content.push(localize('chatAgent.runCommand', 'To take the action, use the accept tool command{0}.', '<keybinding:workbench.action.chat.acceptTool>'));
102-
content.push(localize('chatAgent.autoApprove', 'To automatically approve tool actions without manual confirmation, set chat.tools.autoApprove to true in your settings.'));
102+
content.push(localize('chatAgent.autoApprove', 'To automatically approve tool actions without manual confirmation, set {0} to {1} in your settings.', ChatConfiguration.GlobalAutoApprove, 'true'));
103103
content.push(localize('chatAgent.acceptTool', 'To accept a tool action, use the Accept Tool Confirmation command{0}.', '<keybinding:workbench.action.chat.acceptTool>'));
104104
content.push(localize('chatAgent.openEditedFilesSetting', 'By default, when edits are made to files, they will be opened. To change this behavior, set accessibility.openChatEditedFiles to false in your settings.'));
105105
}

src/vs/workbench/contrib/chat/browser/chat.contribution.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ import './contrib/chatInputCompletions.js';
110110
import './contrib/chatInputEditorContrib.js';
111111
import './contrib/chatInputEditorHover.js';
112112
import { ChatRelatedFilesContribution } from './contrib/chatInputRelatedFilesContrib.js';
113-
import { LanguageModelToolsService } from './languageModelToolsService.js';
113+
import { globalAutoApproveDescription, LanguageModelToolsService } from './languageModelToolsService.js';
114114
import './promptSyntax/promptCodingAgentActionContribution.js';
115115
import './promptSyntax/promptToolsCodeLensProvider.js';
116116
import { PromptUrlHandler } from './promptSyntax/promptUrlHandler.js';
@@ -235,11 +235,11 @@ configurationRegistry.registerConfiguration({
235235
description: nls.localize('chat.notifyWindowOnConfirmation', "Controls whether a chat session should notify the user when a confirmation is needed while the window is not in focus. This includes a window badge as well as notification toast."),
236236
default: true,
237237
},
238-
'chat.tools.autoApprove': {
238+
[ChatConfiguration.GlobalAutoApprove]: {
239239
default: false,
240240
// Description is added in for policy parser. See https://github.com/microsoft/vscode/issues/254526
241-
description: nls.localize('chat.tools.autoApprove.description', "Controls whether tool use should be automatically approved. Allow all tools to run automatically without user confirmation, overriding any tool-specific settings such as terminal auto-approval. Use with caution: carefully review selected tools and be extra wary of possible sources of prompt injection!"),
242-
markdownDescription: nls.localize('chat.tools.autoApprove.markdownDescription', "Controls whether tool use should be automatically approved.\n\nAllows _all_ tools to run automatically without user confirmation, overriding any tool-specific settings such as terminal auto-approval.\n\nUse with caution: carefully review selected tools and be extra wary of possible sources of prompt injection!"),
241+
description: globalAutoApproveDescription.value,
242+
markdownDescription: globalAutoApproveDescription.value,
243243
type: 'boolean',
244244
scope: ConfigurationScope.APPLICATION_MACHINE,
245245
tags: ['experimental'],

src/vs/workbench/contrib/chat/browser/chatContentParts/toolInvocationParts/chatTerminalToolProgressPart.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import { TerminalContribSettingId } from '../../../../terminal/terminalContribEx
2424
import { ConfigurationTarget } from '../../../../../../platform/configuration/common/configuration.js';
2525
import { matchesScheme, Schemas } from '../../../../../../base/common/network.js';
2626
import type { ICodeBlockRenderOptions } from '../../codeBlockPart.js';
27+
import { ChatConfiguration } from '../../../common/constants.js';
2728

2829
export class ChatTerminalToolProgressPart extends BaseChatToolInvocationSubPart {
2930
public readonly domNode: HTMLElement;
@@ -96,7 +97,7 @@ export class ChatTerminalToolProgressPart extends BaseChatToolInvocationSubPart
9697
case 'settings': {
9798
if (scopeRaw === 'global') {
9899
preferencesService.openSettings({
99-
query: `@id:chat.tools.autoApprove`
100+
query: `@id:${ChatConfiguration.GlobalAutoApprove}`
100101
});
101102
} else {
102103
const scope = parseInt(scopeRaw);

src/vs/workbench/contrib/chat/browser/languageModelToolsService.ts

Lines changed: 77 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,16 @@ import { CancellationToken, CancellationTokenSource } from '../../../../base/com
1111
import { Codicon } from '../../../../base/common/codicons.js';
1212
import { toErrorMessage } from '../../../../base/common/errorMessage.js';
1313
import { CancellationError, isCancellationError } from '../../../../base/common/errors.js';
14-
import { Emitter } from '../../../../base/common/event.js';
14+
import { Emitter, Event } from '../../../../base/common/event.js';
15+
import { MarkdownString } from '../../../../base/common/htmlContent.js';
1516
import { Iterable } from '../../../../base/common/iterator.js';
1617
import { Lazy } from '../../../../base/common/lazy.js';
1718
import { combinedDisposable, Disposable, DisposableStore, IDisposable, toDisposable } from '../../../../base/common/lifecycle.js';
1819
import { LRUCache } from '../../../../base/common/map.js';
1920
import { IObservable, ObservableSet } from '../../../../base/common/observable.js';
21+
import Severity from '../../../../base/common/severity.js';
2022
import { ThemeIcon } from '../../../../base/common/themables.js';
23+
import { localize, localize2 } from '../../../../nls.js';
2124
import { IAccessibilityService } from '../../../../platform/accessibility/common/accessibility.js';
2225
import { AccessibilitySignal, IAccessibilitySignalService } from '../../../../platform/accessibilitySignal/browser/accessibilitySignalService.js';
2326
import { IConfigurationService } from '../../../../platform/configuration/common/configuration.js';
@@ -50,6 +53,23 @@ interface ITrackedCall {
5053
store: IDisposable;
5154
}
5255

56+
const enum AutoApproveStorageKeys {
57+
GlobalAutoApproveOptIn = 'chat.tools.global.autoApprove.optIn'
58+
}
59+
60+
export const globalAutoApproveDescription = localize2(
61+
{
62+
key: 'autoApprove2.markdown',
63+
comment: [
64+
'{Locked=\'](https://github.com/features/codespaces)\'}',
65+
'{Locked=\'](https://marketplace.visualstudio.com/items?itemName=ms-vscode-remote.remote-containers)\'}',
66+
'{Locked=\'](https://code.visualstudio.com/docs/copilot/security)\'}',
67+
'{Locked=\'**\'}',
68+
]
69+
},
70+
'Global auto approve also known as "YOLO mode" disables manual approval completely for _all tools in all workspaces_, allowing the agent to act fully autonomously. This is extremely dangerous and is *never* recommended, even containerized environments like [Codespaces](https://github.com/features/codespaces) and [Dev Containers](https://marketplace.visualstudio.com/items?itemName=ms-vscode-remote.remote-containers) have user keys forwarded into the container that could be compromised.\n\n**This feature disables [critical security protections](https://code.visualstudio.com/docs/copilot/security) and makes it much easier for an attacker to compromise the machine.**'
71+
);
72+
5373
export class LanguageModelToolsService extends Disposable implements ILanguageModelToolsService {
5474
_serviceBrand: undefined;
5575

@@ -79,7 +99,8 @@ export class LanguageModelToolsService extends Disposable implements ILanguageMo
7999
@ILogService private readonly _logService: ILogService,
80100
@IConfigurationService private readonly _configurationService: IConfigurationService,
81101
@IAccessibilityService private readonly _accessibilityService: IAccessibilityService,
82-
@IAccessibilitySignalService private readonly _accessibilitySignalService: IAccessibilitySignalService
102+
@IAccessibilitySignalService private readonly _accessibilitySignalService: IAccessibilitySignalService,
103+
@IStorageService private readonly _storageService: IStorageService,
83104
) {
84105
super();
85106

@@ -99,6 +120,15 @@ export class LanguageModelToolsService extends Disposable implements ILanguageMo
99120
}
100121
}));
101122

123+
// Clear out warning accepted state if the setting is disabled
124+
this._register(Event.runAndSubscribe(this._configurationService.onDidChangeConfiguration, e => {
125+
if (!e || e.affectsConfiguration(ChatConfiguration.GlobalAutoApprove)) {
126+
if (this._configurationService.getValue(ChatConfiguration.GlobalAutoApprove) !== true) {
127+
this._storageService.remove(AutoApproveStorageKeys.GlobalAutoApproveOptIn, StorageScope.APPLICATION);
128+
}
129+
}
130+
}));
131+
102132
this._ctxToolsCount = ChatContextKeys.Tools.toolsCount.bindTo(_contextKeyService);
103133
}
104134
override dispose(): void {
@@ -290,7 +320,7 @@ export class LanguageModelToolsService extends Disposable implements ILanguageMo
290320
const prepared = await this.prepareToolInvocation(tool, dto, token);
291321
toolInvocation = new ChatToolInvocation(prepared, tool.data, dto.callId);
292322
trackedCall.invocation = toolInvocation;
293-
const autoConfirmed = this.shouldAutoConfirm(tool.data.id, tool.data.runsInWorkspace);
323+
const autoConfirmed = await this.shouldAutoConfirm(tool.data.id, tool.data.runsInWorkspace);
294324
if (autoConfirmed) {
295325
toolInvocation.confirmed.complete(autoConfirmed);
296326
}
@@ -324,7 +354,7 @@ export class LanguageModelToolsService extends Disposable implements ILanguageMo
324354
}
325355
} else {
326356
const prepared = await this.prepareToolInvocation(tool, dto, token);
327-
if (prepared?.confirmationMessages && !this.shouldAutoConfirm(tool.data.id, tool.data.runsInWorkspace)) {
357+
if (prepared?.confirmationMessages && !(await this.shouldAutoConfirm(tool.data.id, tool.data.runsInWorkspace))) {
328358
const result = await this._dialogService.confirm({ message: renderAsPlaintext(prepared.confirmationMessages.title), detail: renderAsPlaintext(prepared.confirmationMessages.message) });
329359
if (!result.confirmed) {
330360
throw new CancellationError();
@@ -411,7 +441,7 @@ export class LanguageModelToolsService extends Disposable implements ILanguageMo
411441
}
412442

413443
private playAccessibilitySignal(toolInvocations: ChatToolInvocation[]): void {
414-
const autoApproved = this._configurationService.getValue('chat.tools.autoApprove');
444+
const autoApproved = this._configurationService.getValue(ChatConfiguration.GlobalAutoApprove);
415445
if (autoApproved) {
416446
return;
417447
}
@@ -453,7 +483,7 @@ export class LanguageModelToolsService extends Disposable implements ILanguageMo
453483
});
454484
}
455485

456-
private shouldAutoConfirm(toolId: string, runsInWorkspace: boolean | undefined): ConfirmedReason | undefined {
486+
private async shouldAutoConfirm(toolId: string, runsInWorkspace: boolean | undefined): Promise<ConfirmedReason | undefined> {
457487
if (this._workspaceToolConfirmStore.value.getAutoConfirm(toolId)) {
458488
return { type: ToolConfirmKind.LmServicePerTool, scope: 'workspace' };
459489
}
@@ -464,7 +494,7 @@ export class LanguageModelToolsService extends Disposable implements ILanguageMo
464494
return { type: ToolConfirmKind.LmServicePerTool, scope: 'session' };
465495
}
466496

467-
const config = this._configurationService.inspect<boolean | Record<string, boolean>>('chat.tools.autoApprove');
497+
const config = this._configurationService.inspect<boolean | Record<string, boolean>>(ChatConfiguration.GlobalAutoApprove);
468498

469499
// If we know the tool runs at a global level, only consider the global config.
470500
// If we know the tool runs at a workspace level, use those specific settings when appropriate.
@@ -478,12 +508,51 @@ export class LanguageModelToolsService extends Disposable implements ILanguageMo
478508

479509
const autoConfirm = value === true || (typeof value === 'object' && value.hasOwnProperty(toolId) && value[toolId] === true);
480510
if (autoConfirm) {
481-
return { type: ToolConfirmKind.Setting, id: 'chat.tools.autoApprove' };
511+
if (await this._checkGlobalAutoApprove()) {
512+
return { type: ToolConfirmKind.Setting, id: ChatConfiguration.GlobalAutoApprove };
513+
}
482514
}
483515

484516
return undefined;
485517
}
486518

519+
private async _checkGlobalAutoApprove(): Promise<boolean> {
520+
const optedIn = this._storageService.getBoolean(AutoApproveStorageKeys.GlobalAutoApproveOptIn, StorageScope.APPLICATION, false);
521+
if (optedIn) {
522+
return true;
523+
}
524+
525+
const promptResult = await this._dialogService.prompt({
526+
type: Severity.Warning,
527+
message: localize('autoApprove2.title', 'Enable global auto approve?'),
528+
buttons: [
529+
{
530+
label: localize('autoApprove2.button.enable', 'Enable'),
531+
run: () => true
532+
},
533+
{
534+
label: localize('autoApprove2.button.disable', 'Disable'),
535+
run: () => false
536+
},
537+
],
538+
custom: {
539+
icon: Codicon.warning,
540+
disableCloseAction: true,
541+
markdownDetails: [{
542+
markdown: new MarkdownString(globalAutoApproveDescription.value),
543+
}],
544+
}
545+
});
546+
547+
if (promptResult.result !== true) {
548+
await this._configurationService.updateValue(ChatConfiguration.GlobalAutoApprove, false);
549+
return false;
550+
}
551+
552+
this._storageService.store(AutoApproveStorageKeys.GlobalAutoApproveOptIn, true, StorageScope.APPLICATION, StorageTarget.USER);
553+
return true;
554+
}
555+
487556
private cleanupCallDisposables(requestId: string | undefined, store: DisposableStore): void {
488557
if (requestId) {
489558
const disposables = this._callsByRequestId.get(requestId);

src/vs/workbench/contrib/chat/common/constants.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ export enum ChatConfiguration {
99
Edits2Enabled = 'chat.edits2.enabled',
1010
ExtensionToolsEnabled = 'chat.extensionTools.enabled',
1111
EditRequests = 'chat.editRequests',
12+
GlobalAutoApprove = 'chat.tools.global.autoApprove',
1213
AutoApproveEdits = 'chat.tools.edits.autoApprove',
1314
EnableMath = 'chat.math.enabled',
1415
CheckpointsEnabled = 'chat.checkpoints.enabled',

src/vs/workbench/contrib/chat/test/browser/languageModelToolsService.test.ts

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -513,7 +513,7 @@ suite('LanguageModelToolsService', () => {
513513
test('accessibility signal for tool confirmation', async () => {
514514
// Create a test configuration service with proper settings
515515
const testConfigService = new TestConfigurationService();
516-
testConfigService.setUserConfiguration('chat.tools.autoApprove', false);
516+
testConfigService.setUserConfiguration('chat.tools.global.autoApprove', false);
517517
testConfigService.setUserConfiguration('accessibility.signals.chatUserActionRequired', { sound: 'auto', announcement: 'auto' });
518518

519519
// Create a test accessibility service that simulates screen reader being enabled
@@ -574,7 +574,7 @@ suite('LanguageModelToolsService', () => {
574574
test('accessibility signal respects autoApprove configuration', async () => {
575575
// Create a test configuration service with auto-approve enabled
576576
const testConfigService = new TestConfigurationService();
577-
testConfigService.setUserConfiguration('chat.tools.autoApprove', true);
577+
testConfigService.setUserConfiguration('chat.tools.global.autoApprove', true);
578578
testConfigService.setUserConfiguration('accessibility.signals.chatUserActionRequired', { sound: 'auto', announcement: 'auto' });
579579

580580
// Create a test accessibility service that simulates screen reader being enabled
@@ -624,7 +624,7 @@ suite('LanguageModelToolsService', () => {
624624
test('shouldAutoConfirm with basic configuration', async () => {
625625
// Test basic shouldAutoConfirm behavior with simple configuration
626626
const testConfigService = new TestConfigurationService();
627-
testConfigService.setUserConfiguration('chat.tools.autoApprove', true); // Global enabled
627+
testConfigService.setUserConfiguration('chat.tools.global.autoApprove', true); // Global enabled
628628

629629
const instaService = workbenchInstantiationService({
630630
contextKeyService: () => store.add(new ContextKeyService(testConfigService)),
@@ -654,7 +654,7 @@ suite('LanguageModelToolsService', () => {
654654
test('shouldAutoConfirm with per-tool configuration object', async () => {
655655
// Test per-tool configuration: { toolId: true/false }
656656
const testConfigService = new TestConfigurationService();
657-
testConfigService.setUserConfiguration('chat.tools.autoApprove', {
657+
testConfigService.setUserConfiguration('chat.tools.global.autoApprove', {
658658
'approvedTool': true,
659659
'deniedTool': false
660660
});
@@ -839,7 +839,7 @@ suite('LanguageModelToolsService', () => {
839839

840840
// Test case 1: Sound enabled, announcement disabled, screen reader off
841841
const testConfigService1 = new TestConfigurationService();
842-
testConfigService1.setUserConfiguration('chat.tools.autoApprove', false);
842+
testConfigService1.setUserConfiguration('chat.tools.global.autoApprove', false);
843843
testConfigService1.setUserConfiguration('accessibility.signals.chatUserActionRequired', { sound: 'on', announcement: 'off' });
844844

845845
const testAccessibilityService1 = new class extends TestAccessibilityService {
@@ -879,7 +879,7 @@ suite('LanguageModelToolsService', () => {
879879

880880
// Test case 2: Sound auto, announcement auto, screen reader on
881881
const testConfigService2 = new TestConfigurationService();
882-
testConfigService2.setUserConfiguration('chat.tools.autoApprove', false);
882+
testConfigService2.setUserConfiguration('chat.tools.global.autoApprove', false);
883883
testConfigService2.setUserConfiguration('accessibility.signals.chatUserActionRequired', { sound: 'auto', announcement: 'auto' });
884884

885885
const testAccessibilityService2 = new class extends TestAccessibilityService {
@@ -920,7 +920,7 @@ suite('LanguageModelToolsService', () => {
920920

921921
// Test case 3: Sound off, announcement off - no signal
922922
const testConfigService3 = new TestConfigurationService();
923-
testConfigService3.setUserConfiguration('chat.tools.autoApprove', false);
923+
testConfigService3.setUserConfiguration('chat.tools.global.autoApprove', false);
924924
testConfigService3.setUserConfiguration('accessibility.signals.chatUserActionRequired', { sound: 'off', announcement: 'off' });
925925

926926
const testAccessibilityService3 = new class extends TestAccessibilityService {
@@ -1318,7 +1318,7 @@ suite('LanguageModelToolsService', () => {
13181318
test('shouldAutoConfirm with workspace-specific tool configuration', async () => {
13191319
const testConfigService = new TestConfigurationService();
13201320
// Configure per-tool settings at different scopes
1321-
testConfigService.setUserConfiguration('chat.tools.autoApprove', { 'workspaceTool': true });
1321+
testConfigService.setUserConfiguration('chat.tools.global.autoApprove', { 'workspaceTool': true });
13221322

13231323
const instaService = workbenchInstantiationService({
13241324
contextKeyService: () => store.add(new ContextKeyService(testConfigService)),

src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ import { Event } from '../../../../../../base/common/event.js';
4545
import { TerminalToolConfirmationStorageKeys } from '../../../../chat/browser/chatContentParts/toolInvocationParts/chatTerminalToolConfirmationSubPart.js';
4646
import { IPollingResult, OutputMonitorState } from './monitoring/types.js';
4747
import { getOutput } from '../outputHelpers.js';
48+
import { ChatConfiguration } from '../../../../chat/common/constants.js';
4849

4950
const enum TerminalToolStorageKeysInternal {
5051
TerminalSession = 'chat.terminalSessions'
@@ -779,10 +780,10 @@ export class RunInTerminalTool extends Disposable implements IToolImpl {
779780
}).join(', ');
780781
}
781782

782-
const config = this._configurationService.inspect<boolean | Record<string, boolean>>('chat.tools.autoApprove');
783+
const config = this._configurationService.inspect<boolean | Record<string, boolean>>(ChatConfiguration.GlobalAutoApprove);
783784
const isGlobalAutoApproved = config?.value ?? config.defaultValue;
784785
if (isGlobalAutoApproved) {
785-
return new MarkdownString(`_${localize('autoApprove.global', 'Auto approved by setting {0}', `[\`chat.tools.autoApprove\`](settings_global "${localize('ruleTooltip.global', 'View settings')}")`)}_`);
786+
return new MarkdownString(`_${localize('autoApprove.global', 'Auto approved by setting {0}', `[\`${ChatConfiguration.GlobalAutoApprove}\`](settings_global "${localize('ruleTooltip.global', 'View settings')}")`)}_`);
786787
}
787788

788789
if (isAutoApproved) {

0 commit comments

Comments
 (0)