From 4bc1e2133034b55502b2d6ebd664e6179f9c0317 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Mon, 3 Nov 2025 08:33:21 -0800 Subject: [PATCH 1/3] Block cwds that have unknowns Part of #274167 Part of #274166 --- .../commandLineFileWriteAnalyzer.ts | 35 +- .../commandLineFileWriteAnalyzer.test.ts | 330 ++++++++++++++++++ 2 files changed, 359 insertions(+), 6 deletions(-) create mode 100644 src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/electron-browser/commandLineAnalyzer/commandLineFileWriteAnalyzer.test.ts 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 6365137c2bd..ef41365794d 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 @@ -5,12 +5,15 @@ import { Disposable } from '../../../../../../../base/common/lifecycle.js'; import { URI } from '../../../../../../../base/common/uri.js'; +import { win32, posix } from '../../../../../../../base/common/path.js'; import { localize } from '../../../../../../../nls.js'; import { IConfigurationService } from '../../../../../../../platform/configuration/common/configuration.js'; import { IWorkspaceContextService } from '../../../../../../../platform/workspace/common/workspace.js'; import { TerminalChatAgentToolsSettingId } from '../../../common/terminalChatAgentToolsConfiguration.js'; -import type { TreeSitterCommandParser } from '../../treeSitterCommandParser.js'; +import { type TreeSitterCommandParser } from '../../treeSitterCommandParser.js'; import type { ICommandLineAnalyzer, ICommandLineAnalyzerOptions, ICommandLineAnalyzerResult } from './commandLineAnalyzer.js'; +import { OperatingSystem } from '../../../../../../../base/common/platform.js'; +import { isString } from '../../../../../../../base/common/types.js'; export class CommandLineFileWriteAnalyzer extends Disposable implements ICommandLineAnalyzer { constructor( @@ -23,18 +26,23 @@ export class CommandLineFileWriteAnalyzer extends Disposable implements ICommand } async analyze(options: ICommandLineAnalyzerOptions): Promise { - return this._getResult(await this._getFileWrites(options)); + return this._getResult(options, await this._getFileWrites(options)); } private async _getFileWrites(options: ICommandLineAnalyzerOptions): Promise { let fileWrites: URI[] | string[] = []; const capturedFileWrites = await this._treeSitterCommandParser.getFileWrites(options.treeSitterLanguage, options.commandLine); - // TODO: Handle environment variables https://github.com/microsoft/vscode/issues/274166 - // TODO: Handle command substitions/complex destinations https://github.com/microsoft/vscode/issues/274167 if (capturedFileWrites.length) { const cwd = options.cwd; if (cwd) { - fileWrites = capturedFileWrites.map(e => URI.joinPath(cwd, e)); + fileWrites = capturedFileWrites.map(e => { + const isAbsolute = options.os === OperatingSystem.Windows ? win32.isAbsolute(e) : posix.isAbsolute(e); + if (isAbsolute) { + return URI.file(e); + } else { + return URI.joinPath(cwd, e); + } + }); } else { this._log('Cwd could not be detected'); fileWrites = capturedFileWrites; @@ -44,7 +52,7 @@ export class CommandLineFileWriteAnalyzer extends Disposable implements ICommand return fileWrites; } - private _getResult(fileWrites: URI[] | string[]): ICommandLineAnalyzerResult { + private _getResult(options: ICommandLineAnalyzerOptions, fileWrites: URI[] | string[]): ICommandLineAnalyzerResult { let isAutoApproveAllowed = true; if (fileWrites.length > 0) { const blockDetectedFileWrites = this._configurationService.getValue(TerminalChatAgentToolsSettingId.BlockDetectedFileWrites); @@ -58,7 +66,22 @@ export class CommandLineFileWriteAnalyzer extends Disposable implements ICommand const workspaceFolders = this._workspaceContextService.getWorkspace().folders; if (workspaceFolders.length > 0) { for (const fileWrite of fileWrites) { + if (isString(fileWrite)) { + const isAbsolute = options.os === OperatingSystem.Windows ? win32.isAbsolute(fileWrite) : posix.isAbsolute(fileWrite); + if (!isAbsolute) { + isAutoApproveAllowed = false; + this._log(`File write blocked due to unknown terminal cwd: ${fileWrite.toString()}`); + break; + } + } const fileUri = URI.isUri(fileWrite) ? fileWrite : URI.file(fileWrite); + // TODO: Handle command substitutions/complex destinations properly https://github.com/microsoft/vscode/issues/274167 + // TODO: Handle environment variables properly https://github.com/microsoft/vscode/issues/274166 + if (fileUri.fsPath.match(/[$\(\){}]/)) { + isAutoApproveAllowed = false; + this._log(`File write blocked due to likely containing a variable: ${fileUri.toString()}`); + break; + } const isInsideWorkspace = workspaceFolders.some(folder => folder.uri.scheme === fileUri.scheme && (fileUri.path.startsWith(folder.uri.path + '/') || fileUri.path === folder.uri.path) diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/electron-browser/commandLineAnalyzer/commandLineFileWriteAnalyzer.test.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/electron-browser/commandLineAnalyzer/commandLineFileWriteAnalyzer.test.ts new file mode 100644 index 00000000000..2cf117c06d6 --- /dev/null +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/electron-browser/commandLineAnalyzer/commandLineFileWriteAnalyzer.test.ts @@ -0,0 +1,330 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { strictEqual } from 'assert'; +import { URI } from '../../../../../../../base/common/uri.js'; +import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../../base/test/common/utils.js'; +import type { TestInstantiationService } from '../../../../../../../platform/instantiation/test/common/instantiationServiceMock.js'; +import { workbenchInstantiationService } from '../../../../../../test/browser/workbenchTestServices.js'; +import { ITreeSitterLibraryService } from '../../../../../../../editor/common/services/treeSitter/treeSitterLibraryService.js'; +import { TreeSitterLibraryService } from '../../../../../../services/treeSitter/browser/treeSitterLibraryService.js'; +import { FileService } from '../../../../../../../platform/files/common/fileService.js'; +import { NullLogService } from '../../../../../../../platform/log/common/log.js'; +import { Schemas } from '../../../../../../../base/common/network.js'; +import { TestIPCFileSystemProvider } from '../../../../../../test/electron-browser/workbenchTestServices.js'; +import { TreeSitterCommandParser, TreeSitterCommandParserLanguage } from '../../../browser/treeSitterCommandParser.js'; +import { arch } from '../../../../../../../base/common/process.js'; +import { isWindows, OperatingSystem } from '../../../../../../../base/common/platform.js'; +import { CommandLineFileWriteAnalyzer } from '../../../browser/tools/commandLineAnalyzer/commandLineFileWriteAnalyzer.js'; +import { TestConfigurationService } from '../../../../../../../platform/configuration/test/common/testConfigurationService.js'; +import { IWorkspaceContextService, toWorkspaceFolder } from '../../../../../../../platform/workspace/common/workspace.js'; +import { TerminalChatAgentToolsSettingId } from '../../../common/terminalChatAgentToolsConfiguration.js'; +import type { ICommandLineAnalyzerOptions } from '../../../browser/tools/commandLineAnalyzer/commandLineAnalyzer.js'; +import { TestContextService } from '../../../../../../test/common/workbenchTestServices.js'; +import { Workspace } from '../../../../../../../platform/workspace/test/common/testWorkspace.js'; + +// TODO: The powershell grammar can cause an OOM crash on Windows/arm https://github.com/microsoft/vscode/issues/273177 +(isWindows && (arch === 'arm' || arch === 'arm64') ? suite.skip : suite)('CommandLineFileWriteAnalyzer', () => { + const store = ensureNoDisposablesAreLeakedInTestSuite(); + + let instantiationService: TestInstantiationService; + let parser: TreeSitterCommandParser; + let analyzer: CommandLineFileWriteAnalyzer; + let configurationService: TestConfigurationService; + let workspaceContextService: TestContextService; + + const mockLog = (..._args: unknown[]) => { }; + + setup(() => { + const fileService = store.add(new FileService(new NullLogService())); + const fileSystemProvider = new TestIPCFileSystemProvider(); + store.add(fileService.registerProvider(Schemas.file, fileSystemProvider)); + + configurationService = new TestConfigurationService(); + workspaceContextService = new TestContextService(); + + instantiationService = workbenchInstantiationService({ + fileService: () => fileService, + configurationService: () => configurationService + }, store); + + instantiationService.stub(IWorkspaceContextService, workspaceContextService); + + const treeSitterLibraryService = store.add(instantiationService.createInstance(TreeSitterLibraryService)); + treeSitterLibraryService.isTest = true; + instantiationService.stub(ITreeSitterLibraryService, treeSitterLibraryService); + + parser = store.add(instantiationService.createInstance(TreeSitterCommandParser)); + + analyzer = store.add(instantiationService.createInstance( + CommandLineFileWriteAnalyzer, + parser, + mockLog + )); + }); + + suite('bash', () => { + const cwd = URI.file('/workspace/project'); + + async function t(commandLine: string, blockDetectedFileWrites: 'never' | 'outsideWorkspace' | 'all', expectedAutoApprove: boolean, expectedDisclaimers: number = 0, workspaceFolders: URI[] = [cwd]) { + configurationService.setUserConfiguration(TerminalChatAgentToolsSettingId.BlockDetectedFileWrites, blockDetectedFileWrites); + + // Setup workspace folders + const workspace = new Workspace('test', workspaceFolders.map(uri => toWorkspaceFolder(uri))); + workspaceContextService.setWorkspace(workspace); + + const options: ICommandLineAnalyzerOptions = { + commandLine, + cwd, + shell: 'bash', + os: OperatingSystem.Linux, + treeSitterLanguage: TreeSitterCommandParserLanguage.Bash, + terminalToolSessionId: 'test' + }; + + const result = await analyzer.analyze(options); + strictEqual(result.isAutoApproveAllowed, expectedAutoApprove, `Expected auto approve to be ${expectedAutoApprove} for: ${commandLine}`); + strictEqual((result.disclaimers || []).length, expectedDisclaimers, `Expected ${expectedDisclaimers} disclaimers for: ${commandLine}`); + } + + suite('blockDetectedFileWrites: never', () => { + test('relative path - simple output redirection', () => t('echo hello > file.txt', 'never', true, 1)); + test('relative path - append redirection', () => t('echo hello >> file.txt', 'never', true, 1)); + test('relative paths - multiple redirections', () => t('echo hello > file1.txt && echo world > file2.txt', 'never', true, 1)); + test('relative path - error redirection', () => t('cat missing.txt 2> error.log', 'never', true, 1)); + test('no redirections', () => t('echo hello', 'never', true, 0)); + test('absolute path - /dev/null allowed with never', () => t('echo hello > /dev/null', 'never', true, 1)); + }); + + suite('blockDetectedFileWrites: outsideWorkspace', () => { + // Relative paths (joined with cwd) + test('relative path - file in workspace root - allow', () => t('echo hello > file.txt', 'outsideWorkspace', true, 1)); + test('relative path - file in subdirectory - allow', () => t('echo hello > subdir/file.txt', 'outsideWorkspace', true, 1)); + test('relative path - parent directory - block', () => t('echo hello > ../file.txt', 'outsideWorkspace', false, 1)); + test('relative path - grandparent directory - block', () => t('echo hello > ../../file.txt', 'outsideWorkspace', false, 1)); + + // Absolute paths (parsed as-is) + test('absolute path - /tmp - block', () => t('echo hello > /tmp/file.txt', 'outsideWorkspace', false, 1)); + test('absolute path - /etc - block', () => t('echo hello > /etc/config.txt', 'outsideWorkspace', false, 1)); + test('absolute path - /home - block', () => t('echo hello > /home/user/file.txt', 'outsideWorkspace', false, 1)); + test('absolute path - root - block', () => t('echo hello > /file.txt', 'outsideWorkspace', false, 1)); + test('absolute path - /dev/null - block', () => t('echo hello > /dev/null', 'outsideWorkspace', false, 1)); + + // Special cases + test('no workspace folders - block', () => t('echo hello > file.txt', 'outsideWorkspace', false, 1, [])); + test('no redirections - allow', () => t('echo hello', 'outsideWorkspace', true, 0)); + test('variable in filename - block', () => t('echo hello > $HOME/file.txt', 'outsideWorkspace', false, 1)); + test('command substitution - block', () => t('echo hello > $(pwd)/file.txt', 'outsideWorkspace', false, 1)); + test('brace expansion - block', () => t('echo hello > {a,b}.txt', 'outsideWorkspace', false, 1)); + }); + + suite('blockDetectedFileWrites: all', () => { + test('inside workspace - block', () => t('echo hello > file.txt', 'all', false, 1)); + test('outside workspace - block', () => t('echo hello > /tmp/file.txt', 'all', false, 1)); + test('no redirections - allow', () => t('echo hello', 'all', true, 0)); + test('multiple inside workspace - block', () => t('echo hello > file1.txt && echo world > file2.txt', 'all', false, 1)); + }); + + suite('complex scenarios', () => { + test('pipeline with redirection inside workspace', () => t('cat file.txt | grep "test" > output.txt', 'outsideWorkspace', true, 1)); + test('multiple redirections mixed inside/outside', () => t('echo hello > file.txt && echo world > /tmp/file.txt', 'outsideWorkspace', false, 1)); + test('here-document', () => t('cat > file.txt << EOF\nhello\nEOF', 'outsideWorkspace', true, 1)); + }); + + suite('no cwd provided', () => { + async function tNoCwd(commandLine: string, blockDetectedFileWrites: 'never' | 'outsideWorkspace' | 'all', expectedAutoApprove: boolean, expectedDisclaimers: number = 0) { + configurationService.setUserConfiguration(TerminalChatAgentToolsSettingId.BlockDetectedFileWrites, blockDetectedFileWrites); + + const workspace = new Workspace('test', [toWorkspaceFolder(cwd)]); + workspaceContextService.setWorkspace(workspace); + + const options: ICommandLineAnalyzerOptions = { + commandLine, + cwd: undefined, + shell: 'bash', + os: OperatingSystem.Linux, + treeSitterLanguage: TreeSitterCommandParserLanguage.Bash, + terminalToolSessionId: 'test' + }; + + const result = await analyzer.analyze(options); + strictEqual(result.isAutoApproveAllowed, expectedAutoApprove, `Expected auto approve to be ${expectedAutoApprove} for: ${commandLine}`); + strictEqual((result.disclaimers || []).length, expectedDisclaimers, `Expected ${expectedDisclaimers} disclaimers for: ${commandLine}`); + } + + // When cwd is undefined, relative paths remain as strings and are blocked + test('relative path - never setting - allow', () => tNoCwd('echo hello > file.txt', 'never', true, 1)); + test('relative path - outsideWorkspace setting - block (unknown cwd)', () => tNoCwd('echo hello > file.txt', 'outsideWorkspace', false, 1)); + test('relative path - all setting - block', () => tNoCwd('echo hello > file.txt', 'all', false, 1)); + + // Absolute paths are converted to URIs and checked normally + test('absolute path inside workspace - outsideWorkspace setting - allow', () => tNoCwd('echo hello > /workspace/project/file.txt', 'outsideWorkspace', true, 1)); + test('absolute path outside workspace - outsideWorkspace setting - block', () => tNoCwd('echo hello > /tmp/file.txt', 'outsideWorkspace', false, 1)); + test('absolute path - all setting - block', () => tNoCwd('echo hello > /tmp/file.txt', 'all', false, 1)); + }); + }); + + suite('pwsh', () => { + const cwd = URI.file('C:/workspace/project'); + + async function t(commandLine: string, blockDetectedFileWrites: 'never' | 'outsideWorkspace' | 'all', expectedAutoApprove: boolean, expectedDisclaimers: number = 0, workspaceFolders: URI[] = [cwd]) { + configurationService.setUserConfiguration(TerminalChatAgentToolsSettingId.BlockDetectedFileWrites, blockDetectedFileWrites); + + // Setup workspace folders + const workspace = new Workspace('test', workspaceFolders.map(uri => toWorkspaceFolder(uri))); + workspaceContextService.setWorkspace(workspace); + + const options: ICommandLineAnalyzerOptions = { + commandLine, + cwd, + shell: 'pwsh', + os: OperatingSystem.Windows, + treeSitterLanguage: TreeSitterCommandParserLanguage.PowerShell, + terminalToolSessionId: 'test' + }; + + const result = await analyzer.analyze(options); + strictEqual(result.isAutoApproveAllowed, expectedAutoApprove, `Expected auto approve to be ${expectedAutoApprove} for: ${commandLine}`); + strictEqual((result.disclaimers || []).length, expectedDisclaimers, `Expected ${expectedDisclaimers} disclaimers for: ${commandLine}`); + } + + suite('blockDetectedFileWrites: never', () => { + test('simple output redirection', () => t('Write-Host "hello" > file.txt', 'never', true, 1)); + test('append redirection', () => t('Write-Host "hello" >> file.txt', 'never', true, 1)); + test('multiple redirections', () => t('Write-Host "hello" > file1.txt ; Write-Host "world" > file2.txt', 'never', true, 1)); + test('error redirection', () => t('Get-Content missing.txt 2> error.log', 'never', true, 1)); + test('no redirections', () => t('Write-Host "hello"', 'never', true, 0)); + }); + + suite('blockDetectedFileWrites: outsideWorkspace', () => { + // Relative paths (joined with cwd) + test('relative path - file in workspace root - allow', () => t('Write-Host "hello" > file.txt', 'outsideWorkspace', true, 1)); + test('relative path - file in subdirectory - allow', () => t('Write-Host "hello" > subdir\\file.txt', 'outsideWorkspace', true, 1)); + test('relative path - parent directory - block', () => t('Write-Host "hello" > ..\\file.txt', 'outsideWorkspace', false, 1)); + test('relative path - grandparent directory - block', () => t('Write-Host "hello" > ..\\..\\file.txt', 'outsideWorkspace', false, 1)); + + // Absolute paths - Windows drive letters (parsed as-is) + test('absolute path - C: drive - block', () => t('Write-Host "hello" > C:\\temp\\file.txt', 'outsideWorkspace', false, 1)); + test('absolute path - D: drive - block', () => t('Write-Host "hello" > D:\\data\\config.txt', 'outsideWorkspace', false, 1)); + test('absolute path - different drive than workspace - block', () => t('Write-Host "hello" > E:\\external\\file.txt', 'outsideWorkspace', false, 1)); + + // Absolute paths - UNC paths + test('absolute path - UNC path - block', () => t('Write-Host "hello" > \\\\server\\share\\file.txt', 'outsideWorkspace', false, 1)); + + // Special cases + test('no workspace folders - block', () => t('Write-Host "hello" > file.txt', 'outsideWorkspace', false, 1, [])); + test('no redirections - allow', () => t('Write-Host "hello"', 'outsideWorkspace', true, 0)); + test('variable in filename - block', () => t('Write-Host "hello" > $env:TEMP\\file.txt', 'outsideWorkspace', false, 1)); + test('subexpression - block', () => t('Write-Host "hello" > $(Get-Date).log', 'outsideWorkspace', false, 1)); + }); + + suite('blockDetectedFileWrites: all', () => { + test('inside workspace - block', () => t('Write-Host "hello" > file.txt', 'all', false, 1)); + test('outside workspace - block', () => t('Write-Host "hello" > C:\\temp\\file.txt', 'all', false, 1)); + test('no redirections - allow', () => t('Write-Host "hello"', 'all', true, 0)); + test('multiple inside workspace - block', () => t('Write-Host "hello" > file1.txt ; Write-Host "world" > file2.txt', 'all', false, 1)); + }); + + suite('complex scenarios', () => { + test('pipeline with redirection inside workspace', () => t('Get-Process | Where-Object {$_.CPU -gt 100} > processes.txt', 'outsideWorkspace', true, 1)); + test('multiple redirections mixed inside/outside', () => t('Write-Host "hello" > file.txt ; Write-Host "world" > C:\\temp\\file.txt', 'outsideWorkspace', false, 1)); + test('all streams redirection', () => t('Get-Process *> all.log', 'outsideWorkspace', true, 1)); + test('multiple stream redirections', () => t('Get-Content missing.txt > output.txt 2> error.txt 3> warning.txt', 'outsideWorkspace', true, 1)); + }); + + suite('edge cases', () => { + test('redirection to $null (variable) - block', () => t('Write-Host "hello" > $null', 'outsideWorkspace', false, 1)); + test('relative path with backslashes - allow', () => t('Write-Host "hello" > server\\share\\file.txt', 'outsideWorkspace', true, 1)); + test('quoted filename inside workspace - allow', () => t('Write-Host "hello" > "file with spaces.txt"', 'outsideWorkspace', true, 1)); + test('forward slashes on Windows (relative) - allow', () => t('Write-Host "hello" > subdir/file.txt', 'outsideWorkspace', true, 1)); + }); + }); + + suite('disclaimer messages', () => { + const cwd = URI.file('/workspace/project'); + + async function checkDisclaimer(commandLine: string, blockDetectedFileWrites: 'never' | 'outsideWorkspace' | 'all', expectedContains: string) { + configurationService.setUserConfiguration(TerminalChatAgentToolsSettingId.BlockDetectedFileWrites, blockDetectedFileWrites); + + const workspace = new Workspace('test', [toWorkspaceFolder(cwd)]); + workspaceContextService.setWorkspace(workspace); + + const options: ICommandLineAnalyzerOptions = { + commandLine, + cwd, + shell: 'bash', + os: OperatingSystem.Linux, + treeSitterLanguage: TreeSitterCommandParserLanguage.Bash, + terminalToolSessionId: 'test' + }; + + const result = await analyzer.analyze(options); + const disclaimers = result.disclaimers || []; + strictEqual(disclaimers.length > 0, true, 'Expected at least one disclaimer'); + const combinedDisclaimers = disclaimers.join(' '); + strictEqual(combinedDisclaimers.includes(expectedContains), true, `Expected disclaimer to contain "${expectedContains}" but got: ${combinedDisclaimers}`); + } + + test('blocked disclaimer - absolute path outside workspace', () => checkDisclaimer('echo hello > /tmp/file.txt', 'outsideWorkspace', 'cannot be auto approved')); + test('allowed disclaimer - relative path inside workspace', () => checkDisclaimer('echo hello > file.txt', 'outsideWorkspace', 'File write operations detected')); + test('blocked disclaimer - all setting blocks everything', () => checkDisclaimer('echo hello > file.txt', 'all', 'cannot be auto approved')); + }); + + suite('multiple workspace folders', () => { + const workspace1 = URI.file('/workspace/project1'); + const workspace2 = URI.file('/workspace/project2'); + + async function t(cwd: URI, commandLine: string, expectedAutoApprove: boolean, expectedDisclaimers: number = 0) { + configurationService.setUserConfiguration(TerminalChatAgentToolsSettingId.BlockDetectedFileWrites, 'outsideWorkspace'); + + const workspace = new Workspace('test', [workspace1, workspace2].map(uri => toWorkspaceFolder(uri))); + workspaceContextService.setWorkspace(workspace); + + const options: ICommandLineAnalyzerOptions = { + commandLine, + cwd, + shell: 'bash', + os: OperatingSystem.Linux, + treeSitterLanguage: TreeSitterCommandParserLanguage.Bash, + terminalToolSessionId: 'test' + }; + + const result = await analyzer.analyze(options); + strictEqual(result.isAutoApproveAllowed, expectedAutoApprove, `Expected auto approve to be ${expectedAutoApprove} for: ${commandLine}`); + strictEqual((result.disclaimers || []).length, expectedDisclaimers, `Expected ${expectedDisclaimers} disclaimers for: ${commandLine}`); + } + + test('relative path in same workspace - allow', () => t(workspace1, 'echo hello > file.txt', true, 1)); + test('absolute path to other workspace - allow', () => t(workspace1, 'echo hello > /workspace/project2/file.txt', true, 1)); + test('absolute path outside all workspaces - block', () => t(workspace1, 'echo hello > /tmp/file.txt', false, 1)); + test('relative path to parent of workspace - block', () => t(workspace1, 'echo hello > ../file.txt', false, 1)); + }); + + suite('uri schemes', () => { + async function t(cwdScheme: string, filePath: string, expectedAutoApprove: boolean) { + configurationService.setUserConfiguration(TerminalChatAgentToolsSettingId.BlockDetectedFileWrites, 'outsideWorkspace'); + + const cwd = URI.from({ scheme: cwdScheme, path: '/workspace/project' }); + const workspace = new Workspace('test', [toWorkspaceFolder(cwd)]); + workspaceContextService.setWorkspace(workspace); + + const options: ICommandLineAnalyzerOptions = { + commandLine: `echo hello > ${filePath}`, + cwd, + shell: 'bash', + os: OperatingSystem.Linux, + treeSitterLanguage: TreeSitterCommandParserLanguage.Bash, + terminalToolSessionId: 'test' + }; + + const result = await analyzer.analyze(options); + strictEqual(result.isAutoApproveAllowed, expectedAutoApprove); + } + + test('file scheme - relative path inside workspace', () => t('file', 'file.txt', true)); + test('vscode-remote scheme - relative path inside workspace', () => t('vscode-remote', 'file.txt', true)); + test('vscode-remote scheme - absolute path outside workspace', () => t('vscode-remote', '/tmp/file.txt', false)); + }); +}); From 7b08784a597f61c9975cfa006fa15cf174ba8c03 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Mon, 3 Nov 2025 08:37:12 -0800 Subject: [PATCH 2/3] Conditionally run tests based on OS Depends on URI's impl --- .../commandLineAnalyzer/commandLineFileWriteAnalyzer.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/electron-browser/commandLineAnalyzer/commandLineFileWriteAnalyzer.test.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/electron-browser/commandLineAnalyzer/commandLineFileWriteAnalyzer.test.ts index 2cf117c06d6..cfc24b2e34f 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/electron-browser/commandLineAnalyzer/commandLineFileWriteAnalyzer.test.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/electron-browser/commandLineAnalyzer/commandLineFileWriteAnalyzer.test.ts @@ -65,7 +65,7 @@ import { Workspace } from '../../../../../../../platform/workspace/test/common/t )); }); - suite('bash', () => { + (isWindows ? suite.skip : suite)('bash', () => { const cwd = URI.file('/workspace/project'); async function t(commandLine: string, blockDetectedFileWrites: 'never' | 'outsideWorkspace' | 'all', expectedAutoApprove: boolean, expectedDisclaimers: number = 0, workspaceFolders: URI[] = [cwd]) { @@ -166,7 +166,7 @@ import { Workspace } from '../../../../../../../platform/workspace/test/common/t }); }); - suite('pwsh', () => { + (isWindows ? suite : suite.skip)('pwsh', () => { const cwd = URI.file('C:/workspace/project'); async function t(commandLine: string, blockDetectedFileWrites: 'never' | 'outsideWorkspace' | 'all', expectedAutoApprove: boolean, expectedDisclaimers: number = 0, workspaceFolders: URI[] = [cwd]) { From f88c8fce7910e7e4d51264d228ec8fc36a6c7c0e Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Mon, 3 Nov 2025 08:46:12 -0800 Subject: [PATCH 3/3] Update src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineAnalyzer/commandLineFileWriteAnalyzer.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../tools/commandLineAnalyzer/commandLineFileWriteAnalyzer.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 ef41365794d..0b23293907d 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 @@ -70,7 +70,7 @@ export class CommandLineFileWriteAnalyzer extends Disposable implements ICommand const isAbsolute = options.os === OperatingSystem.Windows ? win32.isAbsolute(fileWrite) : posix.isAbsolute(fileWrite); if (!isAbsolute) { isAutoApproveAllowed = false; - this._log(`File write blocked due to unknown terminal cwd: ${fileWrite.toString()}`); + this._log(`File write blocked due to unknown terminal cwd: ${fileWrite}`); break; } }