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 2077bf38abb78..fd8a338ac8a85 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 @@ -10,12 +10,16 @@ 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 { TreeSitterCommandParserLanguage, 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'; import { ILabelService } from '../../../../../../../platform/label/common/label.js'; +const nullDevice = Symbol('null device'); + +type FileWrite = URI | string | typeof nullDevice; + export class CommandLineFileWriteAnalyzer extends Disposable implements ICommandLineAnalyzer { constructor( private readonly _treeSitterCommandParser: TreeSitterCommandParser, @@ -28,7 +32,7 @@ export class CommandLineFileWriteAnalyzer extends Disposable implements ICommand } async analyze(options: ICommandLineAnalyzerOptions): Promise { - let fileWrites: URI[] | string[]; + let fileWrites: FileWrite[]; try { fileWrites = await this._getFileWrites(options); } catch (e) { @@ -41,14 +45,18 @@ export class CommandLineFileWriteAnalyzer extends Disposable implements ICommand return this._getResult(options, fileWrites); } - private async _getFileWrites(options: ICommandLineAnalyzerOptions): Promise { - let fileWrites: URI[] | string[] = []; - const capturedFileWrites = await this._treeSitterCommandParser.getFileWrites(options.treeSitterLanguage, options.commandLine); + private async _getFileWrites(options: ICommandLineAnalyzerOptions): Promise { + let fileWrites: FileWrite[] = []; + const capturedFileWrites = (await this._treeSitterCommandParser.getFileWrites(options.treeSitterLanguage, options.commandLine)) + .map(this._mapNullDevice.bind(this, options)); if (capturedFileWrites.length) { const cwd = options.cwd; if (cwd) { this._log('Detected cwd', cwd.toString()); fileWrites = capturedFileWrites.map(e => { + if (e === nullDevice) { + return e; + } const isAbsolute = options.os === OperatingSystem.Windows ? win32.isAbsolute(e) : posix.isAbsolute(e); if (isAbsolute) { return URI.file(e); @@ -65,7 +73,18 @@ export class CommandLineFileWriteAnalyzer extends Disposable implements ICommand return fileWrites; } - private _getResult(options: ICommandLineAnalyzerOptions, fileWrites: URI[] | string[]): ICommandLineAnalyzerResult { + private _mapNullDevice(options: ICommandLineAnalyzerOptions, rawFileWrite: string): string | typeof nullDevice { + if (options.treeSitterLanguage === TreeSitterCommandParserLanguage.PowerShell) { + return rawFileWrite === '$null' + ? nullDevice + : rawFileWrite; + } + return rawFileWrite === '/dev/null' + ? nullDevice + : rawFileWrite; + } + + private _getResult(options: ICommandLineAnalyzerOptions, fileWrites: FileWrite[]): ICommandLineAnalyzerResult { let isAutoApproveAllowed = true; if (fileWrites.length > 0) { const blockDetectedFileWrites = this._configurationService.getValue(TerminalChatAgentToolsSettingId.BlockDetectedFileWrites); @@ -79,6 +98,11 @@ export class CommandLineFileWriteAnalyzer extends Disposable implements ICommand const workspaceFolders = this._workspaceContextService.getWorkspace().folders; if (workspaceFolders.length > 0) { for (const fileWrite of fileWrites) { + if (fileWrite === nullDevice) { + this._log('File write to null device allowed', URI.isUri(fileWrite) ? fileWrite.toString() : fileWrite); + continue; + } + if (isString(fileWrite)) { const isAbsolute = options.os === OperatingSystem.Windows ? win32.isAbsolute(fileWrite) : posix.isAbsolute(fileWrite); if (!isAbsolute) { @@ -106,9 +130,12 @@ export class CommandLineFileWriteAnalyzer extends Disposable implements ICommand } } } else { - // No workspace folders, consider all writes as outside workspace - isAutoApproveAllowed = false; - this._log('File writes blocked - no workspace folders'); + // No workspace folders, allow safe null device paths even without workspace + const hasOnlyNullDevices = fileWrites.every(fw => fw === nullDevice); + if (!hasOnlyNullDevices) { + isAutoApproveAllowed = false; + this._log('File writes blocked - no workspace folders'); + } } break; } @@ -121,7 +148,7 @@ export class CommandLineFileWriteAnalyzer extends Disposable implements ICommand const disclaimers: string[] = []; if (fileWrites.length > 0) { - const fileWritesList = fileWrites.map(fw => `\`${URI.isUri(fw) ? this._labelService.getUriLabel(fw) : fw}\``).join(', '); + const fileWritesList = fileWrites.map(fw => `\`${URI.isUri(fw) ? this._labelService.getUriLabel(fw) : fw === nullDevice ? '/dev/null' : fw.toString()}\``).join(', '); if (!isAutoApproveAllowed) { disclaimers.push(localize('runInTerminal.fileWriteBlockedDisclaimer', 'File write operations detected that cannot be auto approved: {0}', fileWritesList)); } else { 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 cfc24b2e34ff6..1d94d81a6ef70 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 @@ -110,10 +110,11 @@ import { Workspace } from '../../../../../../../platform/workspace/test/common/t 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)); + test('absolute path - /dev/null - allow (null device)', () => t('echo hello > /dev/null', 'outsideWorkspace', true, 1)); // Special cases test('no workspace folders - block', () => t('echo hello > file.txt', 'outsideWorkspace', false, 1, [])); + test('no workspace folders - /dev/null allowed', () => t('echo hello > /dev/null', 'outsideWorkspace', true, 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)); @@ -131,6 +132,7 @@ import { Workspace } from '../../../../../../../platform/workspace/test/common/t 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)); + test('error output to /dev/null - allow', () => t('cat missing.txt 2> /dev/null', 'outsideWorkspace', true, 1)); }); suite('no cwd provided', () => { @@ -235,7 +237,7 @@ import { Workspace } from '../../../../../../../platform/workspace/test/common/t }); suite('edge cases', () => { - test('redirection to $null (variable) - block', () => t('Write-Host "hello" > $null', 'outsideWorkspace', false, 1)); + test('redirection to $null (PowerShell null device) - allow', () => t('Write-Host "hello" > $null', 'outsideWorkspace', true, 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));