Skip to content

Commit 4e6a50e

Browse files
authored
Merge pull request #284284 from microsoft/tyriar/270529
Add workspace and session allow options in terminal tool
2 parents 6cd5b46 + 883732f commit 4e6a50e

File tree

7 files changed

+425
-87
lines changed

7 files changed

+425
-87
lines changed

src/vs/workbench/contrib/chat/browser/widget/chatContentParts/toolInvocationParts/chatTerminalToolConfirmationSubPart.ts

Lines changed: 76 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ export interface ITerminalNewAutoApproveRule {
5555
approve: boolean;
5656
matchCommandLine?: boolean;
5757
};
58+
scope: 'session' | 'workspace' | 'user';
5859
}
5960

6061
export type TerminalNewAutoApproveButtonData = (
@@ -258,28 +259,65 @@ export class ChatTerminalToolConfirmationSubPart extends BaseChatToolInvocationS
258259
}
259260
case 'newRule': {
260261
const newRules = asArray(data.rule);
261-
const inspect = this.configurationService.inspect(TerminalContribSettingId.AutoApprove);
262-
const oldValue = (inspect.user?.value as Record<string, unknown> | undefined) ?? {};
263-
let newValue: Record<string, unknown>;
264-
if (isObject(oldValue)) {
265-
newValue = { ...oldValue };
266-
for (const newRule of newRules) {
267-
newValue[newRule.key] = newRule.value;
262+
263+
// Group rules by scope
264+
const sessionRules = newRules.filter(r => r.scope === 'session');
265+
const workspaceRules = newRules.filter(r => r.scope === 'workspace');
266+
const userRules = newRules.filter(r => r.scope === 'user');
267+
268+
// Handle session-scoped rules (temporary, in-memory only)
269+
const chatSessionId = this.context.element.sessionId;
270+
for (const rule of sessionRules) {
271+
this.terminalChatService.addSessionAutoApproveRule(chatSessionId, rule.key, rule.value);
272+
}
273+
274+
// Handle workspace-scoped rules
275+
if (workspaceRules.length > 0) {
276+
const inspect = this.configurationService.inspect(TerminalContribSettingId.AutoApprove);
277+
const oldValue = (inspect.workspaceValue as Record<string, unknown> | undefined) ?? {};
278+
if (isObject(oldValue)) {
279+
const newValue: Record<string, unknown> = { ...oldValue };
280+
for (const rule of workspaceRules) {
281+
newValue[rule.key] = rule.value;
282+
}
283+
await this.configurationService.updateValue(TerminalContribSettingId.AutoApprove, newValue, ConfigurationTarget.WORKSPACE);
284+
} else {
285+
this.preferencesService.openSettings({
286+
jsonEditor: true,
287+
target: ConfigurationTarget.WORKSPACE,
288+
revealSetting: { key: TerminalContribSettingId.AutoApprove },
289+
});
290+
throw new ErrorNoTelemetry(`Cannot add new rule, existing workspace setting is unexpected format`);
268291
}
269-
} else {
270-
this.preferencesService.openSettings({
271-
jsonEditor: true,
272-
target: ConfigurationTarget.USER,
273-
revealSetting: {
274-
key: TerminalContribSettingId.AutoApprove
275-
},
276-
});
277-
throw new ErrorNoTelemetry(`Cannot add new rule, existing setting is unexpected format`);
278292
}
279-
await this.configurationService.updateValue(TerminalContribSettingId.AutoApprove, newValue, ConfigurationTarget.USER);
280-
function formatRuleLinks(newRules: ITerminalNewAutoApproveRule[]): string {
281-
return newRules.map(e => {
282-
const settingsUri = createCommandUri(TerminalContribCommandId.OpenTerminalSettingsLink, ConfigurationTarget.USER);
293+
294+
// Handle user-scoped rules
295+
if (userRules.length > 0) {
296+
const inspect = this.configurationService.inspect(TerminalContribSettingId.AutoApprove);
297+
const oldValue = (inspect.userValue as Record<string, unknown> | undefined) ?? {};
298+
if (isObject(oldValue)) {
299+
const newValue: Record<string, unknown> = { ...oldValue };
300+
for (const rule of userRules) {
301+
newValue[rule.key] = rule.value;
302+
}
303+
await this.configurationService.updateValue(TerminalContribSettingId.AutoApprove, newValue, ConfigurationTarget.USER);
304+
} else {
305+
this.preferencesService.openSettings({
306+
jsonEditor: true,
307+
target: ConfigurationTarget.USER,
308+
revealSetting: { key: TerminalContribSettingId.AutoApprove },
309+
});
310+
throw new ErrorNoTelemetry(`Cannot add new rule, existing setting is unexpected format`);
311+
}
312+
}
313+
314+
function formatRuleLinks(rules: ITerminalNewAutoApproveRule[], scope: 'session' | 'workspace' | 'user'): string {
315+
return rules.map(e => {
316+
if (scope === 'session') {
317+
return `\`${e.key}\``;
318+
}
319+
const target = scope === 'workspace' ? ConfigurationTarget.WORKSPACE : ConfigurationTarget.USER;
320+
const settingsUri = createCommandUri(TerminalContribCommandId.OpenTerminalSettingsLink, target);
283321
return `[\`${e.key}\`](${settingsUri.toString()} "${localize('ruleTooltip', 'View rule in settings')}")`;
284322
}).join(', ');
285323
}
@@ -288,10 +326,24 @@ export class ChatTerminalToolConfirmationSubPart extends BaseChatToolInvocationS
288326
enabledCommands: [TerminalContribCommandId.OpenTerminalSettingsLink]
289327
}
290328
};
291-
if (newRules.length === 1) {
292-
terminalData.autoApproveInfo = new MarkdownString(localize('newRule', 'Auto approve rule {0} added', formatRuleLinks(newRules)), mdTrustSettings);
293-
} else if (newRules.length > 1) {
294-
terminalData.autoApproveInfo = new MarkdownString(localize('newRule.plural', 'Auto approve rules {0} added', formatRuleLinks(newRules)), mdTrustSettings);
329+
const parts: string[] = [];
330+
if (sessionRules.length > 0) {
331+
parts.push(sessionRules.length === 1
332+
? localize('newRule.session', 'Session rule {0} added', formatRuleLinks(sessionRules, 'session'))
333+
: localize('newRule.session.plural', 'Session rules {0} added', formatRuleLinks(sessionRules, 'session')));
334+
}
335+
if (workspaceRules.length > 0) {
336+
parts.push(workspaceRules.length === 1
337+
? localize('newRule.workspace', 'Workspace rule {0} added', formatRuleLinks(workspaceRules, 'workspace'))
338+
: localize('newRule.workspace.plural', 'Workspace rules {0} added', formatRuleLinks(workspaceRules, 'workspace')));
339+
}
340+
if (userRules.length > 0) {
341+
parts.push(userRules.length === 1
342+
? localize('newRule.user', 'User rule {0} added', formatRuleLinks(userRules, 'user'))
343+
: localize('newRule.user.plural', 'User rules {0} added', formatRuleLinks(userRules, 'user')));
344+
}
345+
if (parts.length > 0) {
346+
terminalData.autoApproveInfo = new MarkdownString(parts.join(', '), mdTrustSettings);
295347
}
296348
toolConfirmKind = ToolConfirmKind.UserAction;
297349
break;

src/vs/workbench/contrib/terminal/browser/terminal.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,21 @@ export interface ITerminalChatService {
217217
* @returns True if the session has auto approval enabled
218218
*/
219219
hasChatSessionAutoApproval(chatSessionId: string): boolean;
220+
221+
/**
222+
* Add a session-scoped auto-approve rule.
223+
* @param chatSessionId The chat session ID to associate the rule with
224+
* @param key The rule key (command or regex pattern)
225+
* @param value The rule value (approval boolean or object with approve and matchCommandLine)
226+
*/
227+
addSessionAutoApproveRule(chatSessionId: string, key: string, value: boolean | { approve: boolean; matchCommandLine?: boolean }): void;
228+
229+
/**
230+
* Get all session-scoped auto-approve rules for a specific chat session.
231+
* @param chatSessionId The chat session ID to get rules for
232+
* @returns A record of all session-scoped auto-approve rules for the session
233+
*/
234+
getSessionAutoApproveRules(chatSessionId: string): Readonly<Record<string, boolean | { approve: boolean; matchCommandLine?: boolean }>>;
220235
}
221236

222237
/**

src/vs/workbench/contrib/terminalContrib/chat/browser/terminalChatService.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,13 @@ export class TerminalChatService extends Disposable implements ITerminalChatServ
5353
*/
5454
private readonly _sessionAutoApprovalEnabled = new Set<string>();
5555

56+
/**
57+
* Tracks session-scoped auto-approve rules per chat session. These are temporary rules that
58+
* last only for the duration of the chat session (not persisted to disk).
59+
* Map<chatSessionId, Record<ruleKey, ruleValue>>
60+
*/
61+
private readonly _sessionAutoApproveRules = new Map<string, Record<string, boolean | { approve: boolean; matchCommandLine?: boolean }>>();
62+
5663
constructor(
5764
@ILogService private readonly _logService: ILogService,
5865
@ITerminalService private readonly _terminalService: ITerminalService,
@@ -66,6 +73,16 @@ export class TerminalChatService extends Disposable implements ITerminalChatServ
6673
this._hasHiddenToolTerminalContext = TerminalChatContextKeys.hasHiddenChatTerminals.bindTo(this._contextKeyService);
6774

6875
this._restoreFromStorage();
76+
77+
// Clear session auto-approve rules when chat sessions end
78+
this._register(this._chatService.onDidDisposeSession(e => {
79+
for (const resource of e.sessionResource) {
80+
const sessionId = LocalChatSessionUri.parseLocalSessionId(resource);
81+
if (sessionId) {
82+
this._sessionAutoApproveRules.delete(sessionId);
83+
}
84+
}
85+
}));
6986
}
7087

7188
registerTerminalInstanceWithToolSession(terminalToolSessionId: string | undefined, instance: ITerminalInstance): void {
@@ -313,4 +330,17 @@ export class TerminalChatService extends Disposable implements ITerminalChatServ
313330
hasChatSessionAutoApproval(chatSessionId: string): boolean {
314331
return this._sessionAutoApprovalEnabled.has(chatSessionId);
315332
}
333+
334+
addSessionAutoApproveRule(chatSessionId: string, key: string, value: boolean | { approve: boolean; matchCommandLine?: boolean }): void {
335+
let sessionRules = this._sessionAutoApproveRules.get(chatSessionId);
336+
if (!sessionRules) {
337+
sessionRules = {};
338+
this._sessionAutoApproveRules.set(chatSessionId, sessionRules);
339+
}
340+
sessionRules[key] = value;
341+
}
342+
343+
getSessionAutoApproveRules(chatSessionId: string): Readonly<Record<string, boolean | { approve: boolean; matchCommandLine?: boolean }>> {
344+
return this._sessionAutoApproveRules.get(chatSessionId) ?? {};
345+
}
316346
}

src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/commandLineAutoApprover.ts

Lines changed: 79 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,13 @@ import { structuralEquals } from '../../../../../base/common/equals.js';
1111
import { ConfigurationTarget, IConfigurationService, type IConfigurationValue } from '../../../../../platform/configuration/common/configuration.js';
1212
import { TerminalChatAgentToolsSettingId } from '../common/terminalChatAgentToolsConfiguration.js';
1313
import { isPowerShell } from './runInTerminalHelpers.js';
14+
import { ITerminalChatService } from '../../../terminal/browser/terminal.js';
1415

1516
export interface IAutoApproveRule {
1617
regex: RegExp;
1718
regexCaseInsensitive: RegExp;
1819
sourceText: string;
19-
sourceTarget: ConfigurationTarget;
20+
sourceTarget: ConfigurationTarget | 'session';
2021
isDefaultRule: boolean;
2122
}
2223

@@ -39,6 +40,7 @@ export class CommandLineAutoApprover extends Disposable {
3940

4041
constructor(
4142
@IConfigurationService private readonly _configurationService: IConfigurationService,
43+
@ITerminalChatService private readonly _terminalChatService: ITerminalChatService,
4244
) {
4345
super();
4446
this.updateConfiguration();
@@ -76,7 +78,7 @@ export class CommandLineAutoApprover extends Disposable {
7678
this._denyListCommandLineRules = denyListCommandLineRules;
7779
}
7880

79-
isCommandAutoApproved(command: string, shell: string, os: OperatingSystem): ICommandApprovalResultWithReason {
81+
isCommandAutoApproved(command: string, shell: string, os: OperatingSystem, chatSessionId?: string): ICommandApprovalResultWithReason {
8082
// Check if the command has a transient environment variable assignment prefix which we
8183
// always deny for now as it can easily lead to execute other commands
8284
if (transientEnvVarRegex.test(command)) {
@@ -86,7 +88,7 @@ export class CommandLineAutoApprover extends Disposable {
8688
};
8789
}
8890

89-
// Check the deny list to see if this command requires explicit approval
91+
// Check the config deny list to see if this command requires explicit approval
9092
for (const rule of this._denyListRules) {
9193
if (this._commandMatchesRule(rule, command, shell, os)) {
9294
return {
@@ -97,7 +99,18 @@ export class CommandLineAutoApprover extends Disposable {
9799
}
98100
}
99101

100-
// Check the allow list to see if the command is allowed to run without explicit approval
102+
// Check session allow rules (session deny rules can't exist)
103+
for (const rule of this._getSessionRules(chatSessionId).allowListRules) {
104+
if (this._commandMatchesRule(rule, command, shell, os)) {
105+
return {
106+
result: 'approved',
107+
rule,
108+
reason: `Command '${command}' is approved by session allow list rule: ${rule.sourceText}`
109+
};
110+
}
111+
}
112+
113+
// Check the config allow list to see if the command is allowed to run without explicit approval
101114
for (const rule of this._allowListRules) {
102115
if (this._commandMatchesRule(rule, command, shell, os)) {
103116
return {
@@ -117,8 +130,8 @@ export class CommandLineAutoApprover extends Disposable {
117130
};
118131
}
119132

120-
isCommandLineAutoApproved(commandLine: string): ICommandApprovalResultWithReason {
121-
// Check the deny list first to see if this command line requires explicit approval
133+
isCommandLineAutoApproved(commandLine: string, chatSessionId?: string): ICommandApprovalResultWithReason {
134+
// Check the config deny list first to see if this command line requires explicit approval
122135
for (const rule of this._denyListCommandLineRules) {
123136
if (rule.regex.test(commandLine)) {
124137
return {
@@ -129,7 +142,18 @@ export class CommandLineAutoApprover extends Disposable {
129142
}
130143
}
131144

132-
// Check if the full command line matches any of the allow list command line regexes
145+
// Check session allow list (session deny rules can't exist)
146+
for (const rule of this._getSessionRules(chatSessionId).allowListCommandLineRules) {
147+
if (rule.regex.test(commandLine)) {
148+
return {
149+
result: 'approved',
150+
rule,
151+
reason: `Command line '${commandLine}' is approved by session allow list rule: ${rule.sourceText}`
152+
};
153+
}
154+
}
155+
156+
// Check if the full command line matches any of the config allow list command line regexes
133157
for (const rule of this._allowListCommandLineRules) {
134158
if (rule.regex.test(commandLine)) {
135159
return {
@@ -145,6 +169,54 @@ export class CommandLineAutoApprover extends Disposable {
145169
};
146170
}
147171

172+
private _getSessionRules(chatSessionId?: string): {
173+
denyListRules: IAutoApproveRule[];
174+
allowListRules: IAutoApproveRule[];
175+
allowListCommandLineRules: IAutoApproveRule[];
176+
denyListCommandLineRules: IAutoApproveRule[];
177+
} {
178+
const denyListRules: IAutoApproveRule[] = [];
179+
const allowListRules: IAutoApproveRule[] = [];
180+
const allowListCommandLineRules: IAutoApproveRule[] = [];
181+
const denyListCommandLineRules: IAutoApproveRule[] = [];
182+
183+
if (!chatSessionId) {
184+
return { denyListRules, allowListRules, allowListCommandLineRules, denyListCommandLineRules };
185+
}
186+
187+
const sessionRulesConfig = this._terminalChatService.getSessionAutoApproveRules(chatSessionId);
188+
for (const [key, value] of Object.entries(sessionRulesConfig)) {
189+
if (typeof value === 'boolean') {
190+
const { regex, regexCaseInsensitive } = this._convertAutoApproveEntryToRegex(key);
191+
if (value === true) {
192+
allowListRules.push({ regex, regexCaseInsensitive, sourceText: key, sourceTarget: 'session', isDefaultRule: false });
193+
} else if (value === false) {
194+
denyListRules.push({ regex, regexCaseInsensitive, sourceText: key, sourceTarget: 'session', isDefaultRule: false });
195+
}
196+
} else if (typeof value === 'object' && value !== null) {
197+
const objectValue = value as { approve?: boolean; matchCommandLine?: boolean };
198+
if (typeof objectValue.approve === 'boolean') {
199+
const { regex, regexCaseInsensitive } = this._convertAutoApproveEntryToRegex(key);
200+
if (objectValue.approve === true) {
201+
if (objectValue.matchCommandLine === true) {
202+
allowListCommandLineRules.push({ regex, regexCaseInsensitive, sourceText: key, sourceTarget: 'session', isDefaultRule: false });
203+
} else {
204+
allowListRules.push({ regex, regexCaseInsensitive, sourceText: key, sourceTarget: 'session', isDefaultRule: false });
205+
}
206+
} else if (objectValue.approve === false) {
207+
if (objectValue.matchCommandLine === true) {
208+
denyListCommandLineRules.push({ regex, regexCaseInsensitive, sourceText: key, sourceTarget: 'session', isDefaultRule: false });
209+
} else {
210+
denyListRules.push({ regex, regexCaseInsensitive, sourceText: key, sourceTarget: 'session', isDefaultRule: false });
211+
}
212+
}
213+
}
214+
}
215+
}
216+
217+
return { denyListRules, allowListRules, allowListCommandLineRules, denyListCommandLineRules };
218+
}
219+
148220
private _commandMatchesRule(rule: IAutoApproveRule, command: string, shell: string, os: OperatingSystem): boolean {
149221
const isPwsh = isPowerShell(shell, os);
150222

0 commit comments

Comments
 (0)