diff --git a/src/vs/platform/agentHost/node/commandAutoApprover.ts b/src/vs/platform/agentHost/node/commandAutoApprover.ts index 6c713d18688cdb..fb7ddc833a9b82 100644 --- a/src/vs/platform/agentHost/node/commandAutoApprover.ts +++ b/src/vs/platform/agentHost/node/commandAutoApprover.ts @@ -11,6 +11,7 @@ import { escapeRegExpCharacters, regExpLeadsToEndlessLoop } from '../../../base/ import { URI } from '../../../base/common/uri.js'; import { getAppNodeModulesPath } from './appNodeModules.js'; import { ILogService } from '../../log/common/log.js'; +import { SedFileWriteParser } from '../../terminal/common/autoApprove/sedFileWriteParser.js'; import type { AgentHostTerminalAutoApproveRuleValue, AgentHostTerminalAutoApproveRules } from '../common/agentHostSchema.js'; /** @@ -170,6 +171,7 @@ interface IAutoApproveRules { const neverMatchRegex = /(?!.*)/; const transientEnvVarRegex = /^[A-Z_][A-Z0-9_]*=/i; +const sedFileWriteParser = new SedFileWriteParser(); /** * Auto-approves or denies shell commands based on terminal auto-approve rules. @@ -261,6 +263,9 @@ export class CommandAutoApprover extends Disposable { private _matchSubCommands(subCommands: string[], rules: IAutoApproveRules, isPowerShell: boolean): CommandApprovalResult { let allApproved = true; for (const subCommand of subCommands) { + if (sedFileWriteParser.canHandle(subCommand)) { + return 'denied'; + } // Deny transient env var assignments if (transientEnvVarRegex.test(subCommand)) { return 'denied'; diff --git a/src/vs/platform/agentHost/test/node/commandAutoApprover.test.ts b/src/vs/platform/agentHost/test/node/commandAutoApprover.test.ts index e0aa454b3aa058..ba1dfaadd0d2b3 100644 --- a/src/vs/platform/agentHost/test/node/commandAutoApprover.test.ts +++ b/src/vs/platform/agentHost/test/node/commandAutoApprover.test.ts @@ -81,9 +81,39 @@ suite('CommandAutoApprover', () => { }); 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'); + assert.deepStrictEqual([ + approver.shouldAutoApprove('sed "s/foo/bar/g" file.txt'), + approver.shouldAutoApprove('sed -e "s/foo/bar/"'), + approver.shouldAutoApprove('sed --expression "s/foo/bar/"'), + approver.shouldAutoApprove('sed -i "s/foo/bar/" file.txt'), + approver.shouldAutoApprove('sed -I "s/foo/bar/" file.txt'), + approver.shouldAutoApprove('sed -ni "s/foo/bar/" file.txt'), + approver.shouldAutoApprove('sed -i.bak "s/foo/bar/" file.txt'), + approver.shouldAutoApprove('sed -i \'\' "s/foo/bar/" file.txt'), + approver.shouldAutoApprove('sed --in-place "s/foo/bar/" file.txt'), + approver.shouldAutoApprove('sed --in-place=.bak "s/foo/bar/" file.txt'), + ], [ + 'approved', + 'denied', + 'denied', + 'denied', + 'denied', + 'denied', + 'denied', + 'denied', + 'denied', + 'denied', + ]); + }); + + test('sed in-place commands cannot be allowed by a full-command rule', () => { + const commandLine = 'sed -i "s/foo/bar/" file.txt'; + assert.deepStrictEqual(approver.evaluate(commandLine, { + autoApproveRules: { + sed: true, + '/^sed -i "s\\/foo\\/bar\\/" file\\.txt$/': { approve: true, matchCommandLine: true }, + }, + }), { result: 'denied', autoApproveRuleResolvable: false }); }); // npm/package managers diff --git a/src/vs/platform/agentHost/test/node/sessionPermissions.test.ts b/src/vs/platform/agentHost/test/node/sessionPermissions.test.ts index 73df9e4774079c..1755d9c6d9d389 100644 --- a/src/vs/platform/agentHost/test/node/sessionPermissions.test.ts +++ b/src/vs/platform/agentHost/test/node/sessionPermissions.test.ts @@ -239,6 +239,17 @@ suite('SessionPermissionManager', () => { assert.strictEqual(result, ToolCallConfirmationReason.NotNeeded); }); + test('requires confirmation for sed in-place edits', async () => { + const event = shellEvent('sed -i "s/foo/bar/" file.txt', 'bash'); + assert.deepStrictEqual({ + approval: await permissions.getAutoApproval(event, sessionUri), + ruleResolvable: permissions.isAutoApproveRuleResolvable(event, sessionUri), + }, { + approval: undefined, + ruleResolvable: false, + }); + }); + test('uses forwarded terminal auto-approve rules as the source of truth over fallback defaults', async () => { configService.updateRootConfig({ [AgentHostTerminalAutoApproveRulesConfigKey]: {} }); diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/commandParsers/sedFileWriteParser.ts b/src/vs/platform/terminal/common/autoApprove/sedFileWriteParser.ts similarity index 97% rename from src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/commandParsers/sedFileWriteParser.ts rename to src/vs/platform/terminal/common/autoApprove/sedFileWriteParser.ts index f1442781c74273..11ed304322c413 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/commandParsers/sedFileWriteParser.ts +++ b/src/vs/platform/terminal/common/autoApprove/sedFileWriteParser.ts @@ -3,8 +3,6 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { ICommandFileWriteParser } from './commandFileWriteParser.js'; - /** * Parser for detecting file writes from `sed` commands using in-place editing. * @@ -16,7 +14,7 @@ import { ICommandFileWriteParser } from './commandFileWriteParser.js'; * - `sed --in-place=.bak 's/foo/bar/' file.txt` (GNU long form with backup) * - `sed -I 's/foo/bar/' file.txt` (BSD case-insensitive variant) */ -export class SedFileWriteParser implements ICommandFileWriteParser { +export class SedFileWriteParser { readonly commandName = 'sed'; canHandle(commandText: string): boolean { diff --git a/src/vs/platform/terminal/test/common/autoApprove/sedFileWriteParser.test.ts b/src/vs/platform/terminal/test/common/autoApprove/sedFileWriteParser.test.ts new file mode 100644 index 00000000000000..fb4d8c4256d19e --- /dev/null +++ b/src/vs/platform/terminal/test/common/autoApprove/sedFileWriteParser.test.ts @@ -0,0 +1,49 @@ +/*--------------------------------------------------------------------------------------------- + * 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 { SedFileWriteParser } from '../../../common/autoApprove/sedFileWriteParser.js'; + +suite('SedFileWriteParser', () => { + + ensureNoDisposablesAreLeakedInTestSuite(); + + const parser = new SedFileWriteParser(); + + test('detects supported in-place options', () => { + const commandLines = [ + 'sed -i "s/foo/bar/" file.txt', + 'sed -I "s/foo/bar/" file.txt', + 'sed -ni "s/foo/bar/" file.txt', + 'sed -i.bak "s/foo/bar/" file.txt', + 'sed -i \'\' "s/foo/bar/" file.txt', + 'sed --in-place "s/foo/bar/" file.txt', + 'sed --in-place=.bak "s/foo/bar/" file.txt', + ]; + assert.deepStrictEqual(commandLines.map(commandLine => parser.canHandle(commandLine)), commandLines.map(() => true)); + }); + + test('does not classify non-in-place commands', () => { + const commandLines = [ + 'sed "s/foo/bar/" file.txt', + 'sed -n "s/foo/bar/p" file.txt', + 'echo sed -i file.txt', + ]; + assert.deepStrictEqual(commandLines.map(commandLine => parser.canHandle(commandLine)), commandLines.map(() => false)); + }); + + test('extracts in-place file targets', () => { + assert.deepStrictEqual({ + single: parser.extractFileWrites('sed -i "s/foo/bar/" file.txt'), + multiple: parser.extractFileWrites('sed -i "s/foo/bar/" file1.txt file2.txt'), + bsd: parser.extractFileWrites('sed -i \'\' "s/foo/bar/" file.txt'), + }, { + single: ['file.txt'], + multiple: ['file1.txt', 'file2.txt'], + bsd: ['file.txt'], + }); + }); +}); diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/treeSitterCommandParser.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/treeSitterCommandParser.ts index 5c2c7a5adccffb..dfc8623bc8e1e7 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/treeSitterCommandParser.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/treeSitterCommandParser.ts @@ -11,8 +11,8 @@ import { Disposable, MutableDisposable, toDisposable } from '../../../../../base import { posix, win32 } from '../../../../../base/common/path.js'; import { ITreeSitterLibraryService } from '../../../../../editor/common/services/treeSitter/treeSitterLibraryService.js'; import type { ITerminalSandboxCommand } from '../../../../../platform/sandbox/common/terminalSandboxService.js'; +import { SedFileWriteParser } from '../../../../../platform/terminal/common/autoApprove/sedFileWriteParser.js'; import { ICommandFileWriteParser } from './commandParsers/commandFileWriteParser.js'; -import { SedFileWriteParser } from './commandParsers/sedFileWriteParser.js'; export const enum TreeSitterCommandParserLanguage { Bash = 'bash',