From fbabc5c7ef00008917dbe69d4936c5fbbc5bd086 Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Tue, 31 Mar 2026 21:36:45 -0700 Subject: [PATCH] agentHost: auto-approve terminal commands using treesitter (#306992) * agentHost: auto-approve terminal commands using treesitter Co-authored-by: Copilot * Cleanup Co-authored-by: Copilot * Add tests Co-authored-by: Copilot * Tests Co-authored-by: Copilot --------- Co-authored-by: Copilot --- eslint.config.js | 3 +- .../platform/agentHost/node/agentService.ts | 13 +- .../agentHost/node/agentSideEffects.ts | 96 +++- .../agentHost/node/commandAutoApprover.ts | 418 ++++++++++++++++++ .../test/node/agentSideEffects.test.ts | 13 +- .../test/node/commandAutoApprover.test.ts | 141 ++++++ .../platform/agentHost/test/node/mockAgent.ts | 36 ++ .../node/protocolWebSocket.integrationTest.ts | 65 ++- 8 files changed, 756 insertions(+), 29 deletions(-) create mode 100644 src/vs/platform/agentHost/node/commandAutoApprover.ts create mode 100644 src/vs/platform/agentHost/test/node/commandAutoApprover.test.ts diff --git a/eslint.config.js b/eslint.config.js index 28e0f00ce4c..df163b7a06d 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -1623,7 +1623,8 @@ export default tseslint.config( 'tas-client', // node module allowed even in /common/ '@microsoft/1ds-core-js', // node module allowed even in /common/ '@microsoft/1ds-post-js', // node module allowed even in /common/ - '@xterm/headless' // node module allowed even in /common/ + '@xterm/headless', // node module allowed even in /common/ + '@vscode/tree-sitter-wasm' // used by agentHost for command auto-approval ] }, { diff --git a/src/vs/platform/agentHost/node/agentService.ts b/src/vs/platform/agentHost/node/agentService.ts index a5c017323db..66b56fa4edd 100644 --- a/src/vs/platform/agentHost/node/agentService.ts +++ b/src/vs/platform/agentHost/node/agentService.ts @@ -168,8 +168,19 @@ export class AgentService extends Disposable implements IAgentService { if (!provider) { throw new Error(`No agent provider registered for: ${providerId ?? '(none)'}`); } + + // Ensure the command auto-approver is ready before any session events + // can arrive. This makes shell command auto-approval fully synchronous. + // Safe to run in parallel with createSession since no events flow until + // sendMessage() is called. + this._logService.trace(`[AgentService] createSession: initializing auto-approver and creating session...`); + const [, session] = await Promise.all([ + this._sideEffects.initialize(), + provider.createSession(config), + ]); + this._logService.trace(`[AgentService] createSession: initialization complete`); + this._logService.trace(`[AgentService] createSession: provider=${provider.id} model=${config?.model ?? '(default)'}`); - const session = await provider.createSession(config); this._sessionToProvider.set(session.toString(), provider.id); this._logService.trace(`[AgentService] createSession returned: ${session.toString()}`); diff --git a/src/vs/platform/agentHost/node/agentSideEffects.ts b/src/vs/platform/agentHost/node/agentSideEffects.ts index cb209197399..03fa37affca 100644 --- a/src/vs/platform/agentHost/node/agentSideEffects.ts +++ b/src/vs/platform/agentHost/node/agentSideEffects.ts @@ -6,10 +6,11 @@ import { match as globMatch } from '../../../base/common/glob.js'; import { Disposable, DisposableStore, IDisposable } from '../../../base/common/lifecycle.js'; import { autorun, IObservable } from '../../../base/common/observable.js'; +import { extUriBiasedIgnorePathCase, normalizePath } from '../../../base/common/resources.js'; import { URI } from '../../../base/common/uri.js'; import { generateUuid } from '../../../base/common/uuid.js'; import { ILogService } from '../../log/common/log.js'; -import { IAgent, IAgentAttachment } from '../common/agentService.js'; +import { IAgent, IAgentAttachment, IAgentProgressEvent } from '../common/agentService.js'; import { ISessionDataService } from '../common/sessionDataService.js'; import { ActionType, ISessionAction } from '../common/state/sessionActions.js'; import { @@ -20,6 +21,7 @@ import { type URI as ProtocolURI, } from '../common/state/sessionState.js'; import { AgentEventMapper } from './agentEventMapper.js'; +import { CommandAutoApprover } from './commandAutoApprover.js'; import { SessionStateManager } from './sessionStateManager.js'; /** @@ -50,6 +52,8 @@ export class AgentSideEffects extends Disposable { private readonly _toolCallAgents = new Map(); /** Per-agent event mapper instances (stateful for partId tracking). */ private readonly _eventMappers = new Map(); + /** Auto-approver for shell commands parsed via tree-sitter. */ + private readonly _commandAutoApprover: CommandAutoApprover; constructor( private readonly _stateManager: SessionStateManager, @@ -57,6 +61,7 @@ export class AgentSideEffects extends Disposable { private readonly _logService: ILogService, ) { super(); + this._commandAutoApprover = this._register(new CommandAutoApprover(this._logService)); // Whenever the agents observable changes, publish to root state. this._register(autorun(reader => { @@ -118,6 +123,61 @@ export class AgentSideEffects extends Disposable { return approved; } + /** + * Initializes async resources (tree-sitter WASM) used for command + * auto-approval. Await this before any session events can arrive to + * guarantee that {@link _tryAutoApproveToolReady} is fully synchronous. + */ + initialize(): Promise { + return this._commandAutoApprover.initialize(); + } + + /** + * Synchronously attempts to auto-approve a `tool_ready` event based on + * permission kind. Returns `true` if auto-approved (event should not be + * dispatched to the state manager), or `false` to proceed normally. + */ + private _tryAutoApproveToolReady( + e: { readonly toolCallId: string; readonly session: URI; readonly permissionKind?: string; readonly permissionPath?: string; readonly toolInput?: string }, + sessionKey: ProtocolURI, + agent: IAgent, + ): boolean { + // Write auto-approval: only within the session's working directory, + // then apply the default glob patterns for protected files. + if (e.permissionKind === 'write' && e.permissionPath) { + const sessionState = this._stateManager.getSessionState(sessionKey); + const workDir = sessionState?.workingDirectory ?? sessionState?.summary.workingDirectory; + const workingDirectory = workDir ? URI.parse(workDir) : undefined; + if (workingDirectory && extUriBiasedIgnorePathCase.isEqualOrParent(normalizePath(URI.file(e.permissionPath)), workingDirectory)) { + if (this._shouldAutoApproveEdit(e.permissionPath)) { + this._logService.trace(`[AgentSideEffects] Auto-approving write to ${e.permissionPath}`); + this._toolCallAgents.delete(`${sessionKey}:${e.toolCallId}`); + agent.respondToPermissionRequest(e.toolCallId, true); + return true; + } + } + return false; + } + + // Shell auto-approval: parse the command via tree-sitter (synchronous + // after initialize() has been awaited) and match against default rules. + if (e.permissionKind === 'shell' && e.toolInput) { + const result = this._commandAutoApprover.shouldAutoApprove(e.toolInput); + if (result === 'approved') { + this._logService.trace(`[AgentSideEffects] Auto-approving shell command`); + this._toolCallAgents.delete(`${sessionKey}:${e.toolCallId}`); + agent.respondToPermissionRequest(e.toolCallId, true); + return true; + } + if (result === 'denied') { + this._logService.trace(`[AgentSideEffects] Shell command denied by rule`); + } + return false; + } + + return false; + } + // ---- Agent registration ------------------------------------------------- /** @@ -143,26 +203,15 @@ export class AgentSideEffects extends Disposable { const sessionKey = e.session.toString(); const turnId = this._stateManager.getActiveTurnId(sessionKey); if (turnId) { - // Check if this is a write permission request that can be auto-approved - // based on the built-in default patterns. - if (e.type === 'tool_ready' && e.permissionKind === 'write' && e.permissionPath) { - if (this._shouldAutoApproveEdit(e.permissionPath)) { - this._logService.trace(`[AgentSideEffects] Auto-approving write to ${e.permissionPath}`); - agent.respondToPermissionRequest(e.toolCallId, true); + // Auto-approve tool_ready events synchronously before dispatching. + // Tree-sitter is pre-warmed via initialize(), so this is fully sync. + if (e.type === 'tool_ready') { + if (this._tryAutoApproveToolReady(e, sessionKey, agent)) { return; } } - const actions = agentMapper.mapProgressEventToActions(e, sessionKey, turnId); - if (actions) { - if (Array.isArray(actions)) { - for (const action of actions) { - this._stateManager.dispatchServerAction(action); - } - } else { - this._stateManager.dispatchServerAction(actions); - } - } + this._dispatchProgressActions(agentMapper, e, sessionKey, turnId); } // After a turn completes (idle event), try to consume the next queued message @@ -185,6 +234,19 @@ export class AgentSideEffects extends Disposable { // ---- Side-effect handlers -------------------------------------------------- + private _dispatchProgressActions(mapper: AgentEventMapper, e: IAgentProgressEvent, sessionKey: ProtocolURI, turnId: string): void { + const actions = mapper.mapProgressEventToActions(e, sessionKey, turnId); + if (actions) { + if (Array.isArray(actions)) { + for (const action of actions) { + this._stateManager.dispatchServerAction(action); + } + } else { + this._stateManager.dispatchServerAction(actions); + } + } + } + handleAction(action: ISessionAction): void { switch (action.type) { case ActionType.SessionTurnStarted: { diff --git a/src/vs/platform/agentHost/node/commandAutoApprover.ts b/src/vs/platform/agentHost/node/commandAutoApprover.ts new file mode 100644 index 00000000000..f0ada0fe936 --- /dev/null +++ b/src/vs/platform/agentHost/node/commandAutoApprover.ts @@ -0,0 +1,418 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import type { Language, Parser, Query, QueryCapture } from '@vscode/tree-sitter-wasm'; +import * as fs from 'fs'; +import { Disposable, toDisposable } from '../../../base/common/lifecycle.js'; +import { FileAccess } from '../../../base/common/network.js'; +import { escapeRegExpCharacters, regExpLeadsToEndlessLoop } from '../../../base/common/strings.js'; +import { URI } from '../../../base/common/uri.js'; +import { ILogService } from '../../log/common/log.js'; + +/** Pattern that detects compound commands (&&, ||, ;, |, backtick, $()) */ +const compoundCommandPattern = /&&|\|\||[;|]|`|\$\(/; + +/** + * Result of a command auto-approval check. + * - `approved`: all sub-commands match allow rules and none are denied + * - `denied`: at least one sub-command matches a deny rule + * - `noMatch`: no rule matched — requires user confirmation + */ +export type CommandApprovalResult = 'approved' | 'denied' | 'noMatch'; + +interface IAutoApproveRule { + readonly regex: RegExp; +} + +const neverMatchRegex = /(?!.*)/; +const transientEnvVarRegex = /^[A-Z_][A-Z0-9_]*=/i; + +/** + * Auto-approves or denies shell commands based on default rules. + * + * Uses tree-sitter to parse compound commands (`foo && bar`) into + * sub-commands that are individually checked against allow/deny lists. + * The default rules mirror the VS Code `chat.tools.terminal.autoApprove` + * setting defaults. + * + * Tree-sitter is initialized eagerly; call {@link initialize} and await the + * result before using {@link shouldAutoApprove} to guarantee synchronous + * parsing. If tree-sitter failed to load, compound commands fall back to + * `noMatch` (user confirmation required). + */ +export class CommandAutoApprover extends Disposable { + + private _allowRules: IAutoApproveRule[] | undefined; + private _denyRules: IAutoApproveRule[] | undefined; + private _parser: Parser | undefined; + private _bashLanguage: Language | undefined; + private _queryClass: typeof Query | undefined; + private readonly _initPromise: Promise; + + constructor( + private readonly _logService: ILogService, + ) { + super(); + this._initPromise = this._initTreeSitter(); + } + + /** + * Returns a promise that resolves once tree-sitter WASM has been loaded. + * Await this before processing any events to guarantee that + * {@link shouldAutoApprove} can parse commands synchronously. + */ + initialize(): Promise { + return this._initPromise; + } + + /** + * Synchronously check whether the given command line should be auto-approved. + * Uses tree-sitter (if loaded) to parse compound commands into sub-commands. + */ + shouldAutoApprove(commandLine: string): CommandApprovalResult { + const trimmed = commandLine.trimStart(); + if (trimmed.length === 0) { + return 'approved'; + } + + this._ensureRules(); + + // Try to extract sub-commands via tree-sitter + const subCommands = this._extractSubCommands(trimmed); + if (subCommands && subCommands.length > 0) { + return this._matchSubCommands(subCommands); + } + + // Fallback: if this looks like a compound command but tree-sitter + // failed to parse it, require user confirmation rather than risking + // auto-approving a dangerous sub-command. + if (compoundCommandPattern.test(trimmed)) { + this._logService.trace('[CommandAutoApprover] Compound command without tree-sitter, requiring confirmation'); + return 'noMatch'; + } + + // Simple single command — match against rules + return this._matchCommandLine(trimmed); + } + + private _matchSubCommands(subCommands: string[]): CommandApprovalResult { + let allApproved = true; + for (const subCommand of subCommands) { + // Deny transient env var assignments + if (transientEnvVarRegex.test(subCommand)) { + return 'denied'; + } + + const result = this._matchSingleCommand(subCommand); + if (result === 'denied') { + return 'denied'; + } + if (result !== 'approved') { + allApproved = false; + } + } + return allApproved ? 'approved' : 'noMatch'; + } + + private _matchCommandLine(commandLine: string): CommandApprovalResult { + if (transientEnvVarRegex.test(commandLine)) { + return 'denied'; + } + return this._matchSingleCommand(commandLine); + } + + private _matchSingleCommand(command: string): CommandApprovalResult { + // Check deny rules first + for (const rule of this._denyRules!) { + if (rule.regex.test(command)) { + return 'denied'; + } + } + + // Then check allow rules + for (const rule of this._allowRules!) { + if (rule.regex.test(command)) { + return 'approved'; + } + } + + return 'noMatch'; + } + + // ---- Tree-sitter -------------------------------------------------------- + + private _extractSubCommands(commandLine: string): string[] | undefined { + if (!this._parser || !this._bashLanguage || !this._queryClass) { + return undefined; + } + + try { + this._parser.setLanguage(this._bashLanguage); + const tree = this._parser.parse(commandLine); + if (!tree) { + return undefined; + } + + try { + const query = new this._queryClass(this._bashLanguage, '(command) @command'); + const captures: QueryCapture[] = query.captures(tree.rootNode); + const subCommands = captures.map(c => c.node.text); + query.delete(); + return subCommands.length > 0 ? subCommands : undefined; + } finally { + tree.delete(); + } + } catch (err) { + this._logService.warn('[CommandAutoApprover] Tree-sitter parsing failed', err); + return undefined; + } + } + + private async _initTreeSitter(): Promise { + try { + const TreeSitter = await import('@vscode/tree-sitter-wasm'); + + // Resolve WASM files from node_modules + const moduleRoot = URI.joinPath(FileAccess.asFileUri(''), '..', 'node_modules', '@vscode', 'tree-sitter-wasm', 'wasm'); + const wasmPath = URI.joinPath(moduleRoot, 'tree-sitter.wasm').fsPath; + + await TreeSitter.Parser.init({ + locateFile() { + return wasmPath; + } + }); + + const parser = new TreeSitter.Parser(); + this._register(toDisposable(() => parser.delete())); + + // Load bash grammar + const bashWasmPath = URI.joinPath(moduleRoot, 'tree-sitter-bash.wasm').fsPath; + const bashWasm = await fs.promises.readFile(bashWasmPath); + const bashLanguage = await TreeSitter.Language.load(new Uint8Array(bashWasm.buffer, bashWasm.byteOffset, bashWasm.byteLength)); + + this._parser = parser; + this._bashLanguage = bashLanguage; + this._queryClass = TreeSitter.Query; + this._logService.info('[CommandAutoApprover] Tree-sitter initialized successfully'); + } catch (err) { + this._logService.warn('[CommandAutoApprover] Failed to initialize tree-sitter', err); + } + } + + // ---- Rules -------------------------------------------------------------- + + private _ensureRules(): void { + if (this._allowRules && this._denyRules) { + return; + } + + const allowRules: IAutoApproveRule[] = []; + const denyRules: IAutoApproveRule[] = []; + + for (const [key, value] of Object.entries(DEFAULT_TERMINAL_AUTO_APPROVE_RULES)) { + const regex = convertAutoApproveEntryToRegex(key); + if (value === true) { + allowRules.push({ regex }); + } else if (value === false) { + denyRules.push({ regex }); + } + } + + this._allowRules = allowRules; + this._denyRules = denyRules; + } +} + +// ---- Regex conversion ------------------------------------------------------- + +function convertAutoApproveEntryToRegex(value: string): RegExp { + // If wrapped in `/`, treat as regex + const regexMatch = value.match(/^\/(?.+)\/(?[dgimsuvy]*)$/); + const regexPattern = regexMatch?.groups?.pattern; + if (regexPattern) { + let flags = regexMatch.groups?.flags; + if (flags) { + flags = flags.replaceAll('g', ''); + } + + if (regexPattern === '.*') { + return new RegExp(regexPattern); + } + + try { + const regex = new RegExp(regexPattern, flags || undefined); + if (regExpLeadsToEndlessLoop(regex)) { + return neverMatchRegex; + } + return regex; + } catch { + return neverMatchRegex; + } + } + + if (value === '') { + return neverMatchRegex; + } + + let sanitizedValue: string; + + // Match both path separators if it looks like a path + if (value.includes('/') || value.includes('\\')) { + let pattern = value.replace(/[/\\]/g, '%%PATH_SEP%%'); + pattern = escapeRegExpCharacters(pattern); + pattern = pattern.replace(/%%PATH_SEP%%*/g, '[/\\\\]'); + sanitizedValue = `^(?:\\.[/\\\\])?${pattern}`; + } else { + sanitizedValue = escapeRegExpCharacters(value); + } + + return new RegExp(`^${sanitizedValue}\\b`); +} + +// ---- Default rules ---------------------------------------------------------- +// +// These mirror the VS Code `chat.tools.terminal.autoApprove` setting defaults. +// Kept in sync manually — the actual setting will be wired up later. + +const DEFAULT_TERMINAL_AUTO_APPROVE_RULES: Readonly> = { + // Safe readonly commands + cd: true, + echo: true, + ls: true, + dir: true, + pwd: true, + cat: true, + head: true, + tail: true, + findstr: true, + wc: true, + tr: true, + cut: true, + cmp: true, + which: true, + basename: true, + dirname: true, + realpath: true, + readlink: true, + stat: true, + file: true, + od: true, + du: true, + df: true, + sleep: true, + nl: true, + + grep: true, + + // Safe git sub-commands + '/^git(\\s+(-C\\s+\\S+|--no-pager))*\\s+status\\b/': true, + '/^git(\\s+(-C\\s+\\S+|--no-pager))*\\s+log\\b/': true, + '/^git(\\s+(-C\\s+\\S+|--no-pager))*\\s+log\\b.*\\s--output(=|\\s|$)/': false, + '/^git(\\s+(-C\\s+\\S+|--no-pager))*\\s+show\\b/': true, + '/^git(\\s+(-C\\s+\\S+|--no-pager))*\\s+diff\\b/': true, + '/^git(\\s+(-C\\s+\\S+|--no-pager))*\\s+ls-files\\b/': true, + '/^git(\\s+(-C\\s+\\S+|--no-pager))*\\s+grep\\b/': true, + '/^git(\\s+(-C\\s+\\S+|--no-pager))*\\s+branch\\b/': true, + '/^git(\\s+(-C\\s+\\S+|--no-pager))*\\s+branch\\b.*\\s-(d|D|m|M|-delete|-force)\\b/': false, + + // Docker readonly sub-commands + '/^docker\\s+(ps|images|info|version|inspect|logs|top|stats|port|diff|search|events)\\b/': true, + '/^docker\\s+(container|image|network|volume|context|system)\\s+(ls|ps|inspect|history|show|df|info)\\b/': true, + '/^docker\\s+compose\\s+(ps|ls|top|logs|images|config|version|port|events)\\b/': true, + + // PowerShell + 'Get-ChildItem': true, + 'Get-Content': true, + 'Get-Date': true, + 'Get-Random': true, + 'Get-Location': true, + 'Set-Location': true, + 'Write-Host': true, + 'Write-Output': true, + 'Out-String': true, + 'Split-Path': true, + 'Join-Path': true, + 'Start-Sleep': true, + 'Where-Object': true, + '/^Select-[a-z0-9]/i': true, + '/^Measure-[a-z0-9]/i': true, + '/^Compare-[a-z0-9]/i': true, + '/^Format-[a-z0-9]/i': true, + '/^Sort-[a-z0-9]/i': true, + + // Package manager read-only commands + '/^npm\\s+(ls|list|outdated|view|info|show|explain|why|root|prefix|bin|search|doctor|fund|repo|bugs|docs|home|help(-search)?)\\b/': true, + '/^npm\\s+config\\s+(list|get)\\b/': true, + '/^npm\\s+pkg\\s+get\\b/': true, + '/^npm\\s+audit$/': true, + '/^npm\\s+cache\\s+verify\\b/': true, + '/^yarn\\s+(list|outdated|info|why|bin|help|versions)\\b/': true, + '/^yarn\\s+licenses\\b/': true, + '/^yarn\\s+audit\\b(?!.*\\bfix\\b)/': true, + '/^yarn\\s+config\\s+(list|get)\\b/': true, + '/^yarn\\s+cache\\s+dir\\b/': true, + '/^pnpm\\s+(ls|list|outdated|why|root|bin|doctor)\\b/': true, + '/^pnpm\\s+licenses\\b/': true, + '/^pnpm\\s+audit\\b(?!.*\\bfix\\b)/': true, + '/^pnpm\\s+config\\s+(list|get)\\b/': true, + + // Safe lockfile-only installs + 'npm ci': true, + '/^yarn\\s+install\\s+--frozen-lockfile\\b/': true, + '/^pnpm\\s+install\\s+--frozen-lockfile\\b/': true, + + // Safe commands with dangerous arg blocking + column: true, + '/^column\\b.*\\s-c\\s+[0-9]{4,}/': false, + date: true, + '/^date\\b.*\\s(-s|--set)\\b/': false, + find: true, + '/^find\\b.*\\s-(delete|exec|execdir|fprint|fprintf|fls|ok|okdir)\\b/': false, + rg: true, + '/^rg\\b.*\\s(--pre|--hostname-bin)\\b/': false, + sed: true, + '/^sed\\b.*\\s(-[a-zA-Z]*(e|f)[a-zA-Z]*|--expression|--file)\\b/': false, + '/^sed\\b.*s\\/.*\\/.*\\/[ew]/': false, + '/^sed\\b.*;W/': false, + sort: true, + '/^sort\\b.*\\s-(o|S)\\b/': false, + tree: true, + '/^tree\\b.*\\s-o\\b/': false, + '/^xxd$/': true, + '/^xxd\\b(\\s+-\\S+)*\\s+[^-\\s]\\S*$/': true, + + // Dangerous commands + rm: false, + rmdir: false, + del: false, + 'Remove-Item': false, + ri: false, + rd: false, + erase: false, + dd: false, + kill: false, + ps: false, + top: false, + 'Stop-Process': false, + spps: false, + taskkill: false, + 'taskkill.exe': false, + curl: false, + wget: false, + 'Invoke-RestMethod': false, + 'Invoke-WebRequest': false, + irm: false, + iwr: false, + chmod: false, + chown: false, + 'Set-ItemProperty': false, + sp: false, + 'Set-Acl': false, + jq: false, + xargs: false, + eval: false, + 'Invoke-Expression': false, + iex: false, +}; diff --git a/src/vs/platform/agentHost/test/node/agentSideEffects.test.ts b/src/vs/platform/agentHost/test/node/agentSideEffects.test.ts index 5f69e50f652..7b91db50c95 100644 --- a/src/vs/platform/agentHost/test/node/agentSideEffects.test.ts +++ b/src/vs/platform/agentHost/test/node/agentSideEffects.test.ts @@ -40,7 +40,7 @@ suite('AgentSideEffects', () => { const sessionUri = AgentSession.uri('mock', 'session-1'); - function setupSession(): void { + function setupSession(workingDirectory?: string): void { stateManager.createSession({ resource: sessionUri.toString(), provider: 'mock', @@ -48,6 +48,7 @@ suite('AgentSideEffects', () => { status: SessionStatus.Idle, createdAt: Date.now(), modifiedAt: Date.now(), + workingDirectory, }); stateManager.dispatchServerAction({ type: ActionType.SessionReady, session: sessionUri.toString() }); } @@ -615,7 +616,7 @@ suite('AgentSideEffects', () => { suite('edit auto-approve', () => { test('auto-approves writes to regular source files', async () => { - setupSession(); + setupSession(URI.file('/workspace').toString()); startTurn('turn-1'); disposables.add(sideEffects.registerProgressListener(agent)); @@ -644,7 +645,7 @@ suite('AgentSideEffects', () => { }); test('blocks writes to .env files', () => { - setupSession(); + setupSession(URI.file('/workspace').toString()); startTurn('turn-1'); disposables.add(sideEffects.registerProgressListener(agent)); @@ -679,7 +680,7 @@ suite('AgentSideEffects', () => { }); test('blocks writes to package.json', () => { - setupSession(); + setupSession(URI.file('/workspace').toString()); startTurn('turn-1'); disposables.add(sideEffects.registerProgressListener(agent)); @@ -706,7 +707,7 @@ suite('AgentSideEffects', () => { }); test('blocks writes to .lock files', () => { - setupSession(); + setupSession(URI.file('/workspace').toString()); startTurn('turn-1'); disposables.add(sideEffects.registerProgressListener(agent)); @@ -733,7 +734,7 @@ suite('AgentSideEffects', () => { }); test('blocks writes to .git directory', () => { - setupSession(); + setupSession(URI.file('/workspace').toString()); startTurn('turn-1'); disposables.add(sideEffects.registerProgressListener(agent)); diff --git a/src/vs/platform/agentHost/test/node/commandAutoApprover.test.ts b/src/vs/platform/agentHost/test/node/commandAutoApprover.test.ts new file mode 100644 index 00000000000..c890ca7a60e --- /dev/null +++ b/src/vs/platform/agentHost/test/node/commandAutoApprover.test.ts @@ -0,0 +1,141 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import assert from 'assert'; +import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js'; +import { NullLogService } from '../../../log/common/log.js'; +import { CommandAutoApprover } from '../../node/commandAutoApprover.js'; + +suite('CommandAutoApprover', () => { + + const disposables = ensureNoDisposablesAreLeakedInTestSuite(); + + let approver: CommandAutoApprover; + + setup(() => { + approver = disposables.add(new CommandAutoApprover(new NullLogService())); + }); + + suite('shouldAutoApprove', () => { + + test('approves empty command', () => { + assert.strictEqual(approver.shouldAutoApprove(''), 'approved'); + assert.strictEqual(approver.shouldAutoApprove(' '), 'approved'); + }); + + // Safe readonly commands + test('approves allowed readonly commands', () => { + assert.strictEqual(approver.shouldAutoApprove('ls'), 'approved'); + assert.strictEqual(approver.shouldAutoApprove('ls -la'), 'approved'); + assert.strictEqual(approver.shouldAutoApprove('cat file.txt'), 'approved'); + assert.strictEqual(approver.shouldAutoApprove('head -n 10 file.txt'), 'approved'); + assert.strictEqual(approver.shouldAutoApprove('tail -f log.txt'), 'approved'); + assert.strictEqual(approver.shouldAutoApprove('pwd'), 'approved'); + assert.strictEqual(approver.shouldAutoApprove('echo hello'), 'approved'); + assert.strictEqual(approver.shouldAutoApprove('grep -r pattern .'), 'approved'); + assert.strictEqual(approver.shouldAutoApprove('wc -l file.txt'), 'approved'); + assert.strictEqual(approver.shouldAutoApprove('which node'), 'approved'); + }); + + // Dangerous commands + test('denies denied commands', () => { + assert.strictEqual(approver.shouldAutoApprove('rm file.txt'), 'denied'); + assert.strictEqual(approver.shouldAutoApprove('rm -rf /'), 'denied'); + assert.strictEqual(approver.shouldAutoApprove('rmdir folder'), 'denied'); + assert.strictEqual(approver.shouldAutoApprove('kill -9 1234'), 'denied'); + assert.strictEqual(approver.shouldAutoApprove('curl http://evil.com'), 'denied'); + assert.strictEqual(approver.shouldAutoApprove('wget http://evil.com'), 'denied'); + assert.strictEqual(approver.shouldAutoApprove('chmod 777 file'), 'denied'); + assert.strictEqual(approver.shouldAutoApprove('chown root file'), 'denied'); + assert.strictEqual(approver.shouldAutoApprove('eval "bad stuff"'), 'denied'); + assert.strictEqual(approver.shouldAutoApprove('xargs rm'), 'denied'); + assert.strictEqual(approver.shouldAutoApprove('dd if=/dev/zero of=/dev/sda'), 'denied'); + }); + + // Safe git sub-commands + test('approves allowed git sub-commands', () => { + assert.strictEqual(approver.shouldAutoApprove('git status'), 'approved'); + assert.strictEqual(approver.shouldAutoApprove('git log --oneline'), 'approved'); + assert.strictEqual(approver.shouldAutoApprove('git diff HEAD'), 'approved'); + assert.strictEqual(approver.shouldAutoApprove('git show HEAD'), 'approved'); + assert.strictEqual(approver.shouldAutoApprove('git ls-files'), 'approved'); + assert.strictEqual(approver.shouldAutoApprove('git branch'), 'approved'); + }); + + // Unsafe git sub-commands + test('denies denied git operations', () => { + assert.strictEqual(approver.shouldAutoApprove('git branch -D main'), 'denied'); + assert.strictEqual(approver.shouldAutoApprove('git branch --delete main'), 'denied'); + assert.strictEqual(approver.shouldAutoApprove('git log --output=/tmp/out'), 'denied'); + }); + + // Safe commands with dangerous arg blocking + test('handles find with blocked args', () => { + assert.strictEqual(approver.shouldAutoApprove('find . -name "*.ts"'), 'approved'); + assert.strictEqual(approver.shouldAutoApprove('find . -delete'), 'denied'); + // find -exec with ; is treated as a compound command, requiring confirmation + assert.strictEqual(approver.shouldAutoApprove('find . -exec rm {} ;'), 'noMatch'); + }); + + test('handles sed with blocked args', () => { + assert.strictEqual(approver.shouldAutoApprove('sed "s/foo/bar/g" file.txt'), 'approved'); + assert.strictEqual(approver.shouldAutoApprove('sed -e "s/foo/bar/"'), 'denied'); + assert.strictEqual(approver.shouldAutoApprove('sed --expression "s/foo/bar/"'), 'denied'); + }); + + // npm/package managers + test('approves allowed npm commands', () => { + assert.strictEqual(approver.shouldAutoApprove('npm ci'), 'approved'); + assert.strictEqual(approver.shouldAutoApprove('npm ls'), 'approved'); + assert.strictEqual(approver.shouldAutoApprove('npm audit'), 'approved'); + }); + + // Unknown commands get noMatch + test('returns noMatch for unknown commands', () => { + assert.strictEqual(approver.shouldAutoApprove('my-custom-script'), 'noMatch'); + assert.strictEqual(approver.shouldAutoApprove('python script.py'), 'noMatch'); + assert.strictEqual(approver.shouldAutoApprove('node index.js'), 'noMatch'); + }); + + // Transient env vars + test('denies transient environment variable assignments', () => { + assert.strictEqual(approver.shouldAutoApprove('FOO=bar some-command'), 'denied'); + assert.strictEqual(approver.shouldAutoApprove('PATH=/evil:$PATH ls'), 'denied'); + }); + + // PowerShell + test('approves allowed PowerShell commands', () => { + assert.strictEqual(approver.shouldAutoApprove('Get-ChildItem'), 'approved'); + assert.strictEqual(approver.shouldAutoApprove('Get-Content file.txt'), 'approved'); + assert.strictEqual(approver.shouldAutoApprove('Write-Host "hello"'), 'approved'); + assert.strictEqual(approver.shouldAutoApprove('Select-Object Name'), 'approved'); + }); + + test('PowerShell case-insensitive rules work', () => { + // Rules with /i flag (like Select-*, Measure-*, etc.) are case-insensitive + assert.strictEqual(approver.shouldAutoApprove('select-object Name'), 'approved'); + assert.strictEqual(approver.shouldAutoApprove('SELECT-OBJECT Name'), 'approved'); + assert.strictEqual(approver.shouldAutoApprove('Measure-Command'), 'approved'); + assert.strictEqual(approver.shouldAutoApprove('measure-command'), 'approved'); + }); + + test('denies denied PowerShell commands', () => { + assert.strictEqual(approver.shouldAutoApprove('Remove-Item file.txt'), 'denied'); + assert.strictEqual(approver.shouldAutoApprove('Invoke-Expression "bad"'), 'denied'); + assert.strictEqual(approver.shouldAutoApprove('Invoke-WebRequest http://evil.com'), 'denied'); + assert.strictEqual(approver.shouldAutoApprove('Stop-Process -Id 1234'), 'denied'); + }); + + // Compound commands containing denied sub-commands should never be auto-approved, + // regardless of whether tree-sitter is available (with tree-sitter they are + // 'denied', without they are 'noMatch' — both are safe). + test('compound commands with denied sub-commands are not auto-approved', () => { + assert.notStrictEqual(approver.shouldAutoApprove('echo ok && rm -rf /'), 'approved'); + assert.notStrictEqual(approver.shouldAutoApprove('ls || curl evil.com'), 'approved'); + assert.notStrictEqual(approver.shouldAutoApprove('cat file; rm file'), 'approved'); + assert.notStrictEqual(approver.shouldAutoApprove('echo $(whoami)'), 'approved'); + }); + }); +}); diff --git a/src/vs/platform/agentHost/test/node/mockAgent.ts b/src/vs/platform/agentHost/test/node/mockAgent.ts index 2d751c9ee05..dadc996f2a7 100644 --- a/src/vs/platform/agentHost/test/node/mockAgent.ts +++ b/src/vs/platform/agentHost/test/node/mockAgent.ts @@ -293,6 +293,42 @@ export class ScriptedMockAgent implements IAgent { break; } + case 'run-safe-command': { + // Fire tool_start + tool_ready with shell permission for an allowed command (should be auto-approved) + (async () => { + await timeout(10); + this._onDidSessionProgress.fire({ type: 'tool_start', session, toolCallId: 'tc-shell-1', toolName: 'bash', displayName: 'Run Command', invocationMessage: 'Run command' }); + await timeout(5); + this._onDidSessionProgress.fire({ type: 'tool_ready', session, toolCallId: 'tc-shell-1', invocationMessage: 'ls -la', permissionKind: 'shell', toolInput: 'ls -la' }); + // Auto-approved shell commands resolve immediately + await timeout(10); + this._fireSequence(session, [ + { type: 'tool_complete', session, toolCallId: 'tc-shell-1', result: { pastTenseMessage: 'Ran command', content: [{ type: ToolResultContentType.Text, text: 'file1.ts\nfile2.ts' }], success: true } }, + { type: 'idle', session }, + ]); + })(); + break; + } + + case 'run-dangerous-command': { + // Fire tool_start + tool_ready with shell permission for a denied command (should require confirmation) + (async () => { + await timeout(10); + this._onDidSessionProgress.fire({ type: 'tool_start', session, toolCallId: 'tc-shell-deny-1', toolName: 'bash', displayName: 'Run Command', invocationMessage: 'Run command' }); + await timeout(5); + this._onDidSessionProgress.fire({ type: 'tool_ready', session, toolCallId: 'tc-shell-deny-1', invocationMessage: 'rm -rf /', permissionKind: 'shell', toolInput: 'rm -rf /', confirmationTitle: 'Run in terminal' }); + })(); + this._pendingPermissions.set('tc-shell-deny-1', (approved) => { + if (approved) { + this._fireSequence(session, [ + { type: 'tool_complete', session, toolCallId: 'tc-shell-deny-1', result: { pastTenseMessage: 'Ran command', content: [{ type: ToolResultContentType.Text, text: '' }], success: true } }, + { type: 'idle', session }, + ]); + } + }); + break; + } + case 'with-usage': this._fireSequence(session, [ { type: 'delta', session, messageId: 'msg-1', content: 'Usage response.' }, diff --git a/src/vs/platform/agentHost/test/node/protocolWebSocket.integrationTest.ts b/src/vs/platform/agentHost/test/node/protocolWebSocket.integrationTest.ts index 12f4a9b73e2..5d6833254f6 100644 --- a/src/vs/platform/agentHost/test/node/protocolWebSocket.integrationTest.ts +++ b/src/vs/platform/agentHost/test/node/protocolWebSocket.integrationTest.ts @@ -241,10 +241,10 @@ function getActionEnvelope(n: IAhpNotification): IActionEnvelope { } /** Perform handshake, create a session, subscribe, and return its URI. */ -async function createAndSubscribeSession(c: TestProtocolClient, clientId: string): Promise { +async function createAndSubscribeSession(c: TestProtocolClient, clientId: string, workingDirectory?: string): Promise { await c.call('initialize', { protocolVersion: PROTOCOL_VERSION, clientId }); - await c.call('createSession', { session: nextSessionUri(), provider: 'mock' }); + await c.call('createSession', { session: nextSessionUri(), provider: 'mock', workingDirectory }); const notif = await c.waitForNotification(n => n.method === 'notification' && (n.params as INotificationBroadcastParams).notification.type === 'notify/sessionAdded' @@ -926,7 +926,7 @@ suite('Protocol WebSocket E2E', function () { test('auto-approves write to regular file (no pending confirmation)', async function () { this.timeout(10_000); - const sessionUri = await createAndSubscribeSession(client, 'test-autoapprove'); + const sessionUri = await createAndSubscribeSession(client, 'test-autoapprove', 'file:///workspace'); client.clearReceived(); // Start a turn that triggers a write permission request for a regular .ts file @@ -952,7 +952,7 @@ suite('Protocol WebSocket E2E', function () { test('blocks write to .env file (requires manual confirmation)', async function () { this.timeout(10_000); - const sessionUri = await createAndSubscribeSession(client, 'test-autoapprove-deny'); + const sessionUri = await createAndSubscribeSession(client, 'test-autoapprove-deny', 'file:///workspace'); client.clearReceived(); // Start a turn that tries to write .env (blocked by default patterns) @@ -977,4 +977,61 @@ suite('Protocol WebSocket E2E', function () { await client.waitForNotification(n => isActionNotification(n, 'session/turnComplete')); }); + + // ---- Shell auto-approve ------------------------------------------------- + + test('auto-approves allowed shell command (no pending confirmation)', async function () { + this.timeout(10_000); + + const sessionUri = await createAndSubscribeSession(client, 'test-shell-approve'); + client.clearReceived(); + + // Start a turn that triggers a shell permission request for "ls -la" (allowed command) + dispatchTurnStarted(client, sessionUri, 'turn-shell-approve', 'run-safe-command', 1); + + // The shell command should be auto-approved — we should see tool_start, tool_complete, and turn_complete + // but NOT a pending-confirmation toolCallReady. + await client.waitForNotification(n => isActionNotification(n, 'session/toolCallStart')); + await client.waitForNotification(n => isActionNotification(n, 'session/toolCallComplete')); + await client.waitForNotification(n => isActionNotification(n, 'session/turnComplete')); + + // Verify no pending-confirmation toolCallReady was received + const pendingConfirmNotifs = client.receivedNotifications(n => { + if (!isActionNotification(n, 'session/toolCallReady')) { + return false; + } + const action = getActionEnvelope(n).action as { confirmed?: string }; + return !action.confirmed; + }); + assert.strictEqual(pendingConfirmNotifs.length, 0, 'should not have received pending-confirmation toolCallReady for allowed shell command'); + }); + + test('blocks denied shell command (requires manual confirmation)', async function () { + this.timeout(10_000); + + const sessionUri = await createAndSubscribeSession(client, 'test-shell-deny'); + client.clearReceived(); + + // Start a turn that triggers a shell permission request for "rm -rf /" (denied command) + dispatchTurnStarted(client, sessionUri, 'turn-shell-deny', 'run-dangerous-command', 1); + + // The denied command should NOT be auto-approved — we should see toolCallReady (pending confirmation) + await client.waitForNotification(n => isActionNotification(n, 'session/toolCallStart')); + await client.waitForNotification(n => isActionNotification(n, 'session/toolCallReady')); + + // Confirm it manually to let the turn complete + client.notify('dispatchAction', { + clientSeq: 2, + action: { + type: 'session/toolCallConfirmed', + session: sessionUri, + turnId: 'turn-shell-deny', + toolCallId: 'tc-shell-deny-1', + approved: true, + confirmed: 'user-action', + }, + }); + + await client.waitForNotification(n => isActionNotification(n, 'session/turnComplete')); + }); });