From d625eb049653a4e702aec9ce073757f44b87e49c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 8 Nov 2025 12:45:43 +0000 Subject: [PATCH 1/4] Initial plan From f310c111876da10c44993ac0c8a8384b6f00e023 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 8 Nov 2025 13:00:52 +0000 Subject: [PATCH 2/4] Add auto-approval for writes to null device files Co-authored-by: Tyriar <2193314+Tyriar@users.noreply.github.com> --- .../commandLineFileWriteAnalyzer.ts | 37 +++++++++++++++++-- .../commandLineFileWriteAnalyzer.test.ts | 7 +++- 2 files changed, 39 insertions(+), 5 deletions(-) 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..67c74e10cae44 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 @@ -65,6 +65,28 @@ export class CommandLineFileWriteAnalyzer extends Disposable implements ICommand return fileWrites; } + private _isSafeNullDevicePath(fileWrite: URI | string): boolean { + const path = URI.isUri(fileWrite) ? fileWrite.fsPath : fileWrite; + const normalizedPath = path.toLowerCase().replace(/\\/g, '/'); + + // Unix/Linux null device + if (normalizedPath === '/dev/null') { + return true; + } + + // Windows CMD null device (case-insensitive) + if (normalizedPath === 'nul' || normalizedPath.endsWith('/nul') || normalizedPath.endsWith('\\nul')) { + return true; + } + + // PowerShell $null variable (appears as "$null" in the command line) + if (path === '$null') { + return true; + } + + return false; + } + private _getResult(options: ICommandLineAnalyzerOptions, fileWrites: URI[] | string[]): ICommandLineAnalyzerResult { let isAutoApproveAllowed = true; if (fileWrites.length > 0) { @@ -79,6 +101,12 @@ export class CommandLineFileWriteAnalyzer extends Disposable implements ICommand const workspaceFolders = this._workspaceContextService.getWorkspace().folders; if (workspaceFolders.length > 0) { for (const fileWrite of fileWrites) { + // Allow safe null device paths (check before variable detection) + if (this._isSafeNullDevicePath(fileWrite)) { + 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 +134,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 => this._isSafeNullDevicePath(fw)); + if (!hasOnlyNullDevices) { + isAutoApproveAllowed = false; + this._log('File writes blocked - no workspace folders'); + } } break; } 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..da1087e499d0e 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,8 @@ 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('redirection to NUL (Windows CMD null device) - allow', () => t('Write-Host "hello" > NUL', '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)); From 9fb8b07610d37eac19d2d07f8de0761e805c4881 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Mon, 10 Nov 2025 07:47:44 -0800 Subject: [PATCH 3/4] Simplify null device detection --- .../commandLineFileWriteAnalyzer.ts | 56 +++++++++---------- .../commandLineFileWriteAnalyzer.test.ts | 1 - 2 files changed, 26 insertions(+), 31 deletions(-) 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 67c74e10cae44..420b9aa4f1868 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,29 +73,18 @@ export class CommandLineFileWriteAnalyzer extends Disposable implements ICommand return fileWrites; } - private _isSafeNullDevicePath(fileWrite: URI | string): boolean { - const path = URI.isUri(fileWrite) ? fileWrite.fsPath : fileWrite; - const normalizedPath = path.toLowerCase().replace(/\\/g, '/'); - - // Unix/Linux null device - if (normalizedPath === '/dev/null') { - return true; + private _mapNullDevice(options: ICommandLineAnalyzerOptions, rawFileWrite: string): string | typeof nullDevice { + if (options.treeSitterLanguage === TreeSitterCommandParserLanguage.PowerShell) { + return rawFileWrite === '$null' + ? nullDevice + : rawFileWrite; } - - // Windows CMD null device (case-insensitive) - if (normalizedPath === 'nul' || normalizedPath.endsWith('/nul') || normalizedPath.endsWith('\\nul')) { - return true; - } - - // PowerShell $null variable (appears as "$null" in the command line) - if (path === '$null') { - return true; - } - - return false; + return rawFileWrite === '/dev/null' + ? nullDevice + : rawFileWrite; } - private _getResult(options: ICommandLineAnalyzerOptions, fileWrites: URI[] | string[]): ICommandLineAnalyzerResult { + private _getResult(options: ICommandLineAnalyzerOptions, fileWrites: FileWrite[]): ICommandLineAnalyzerResult { let isAutoApproveAllowed = true; if (fileWrites.length > 0) { const blockDetectedFileWrites = this._configurationService.getValue(TerminalChatAgentToolsSettingId.BlockDetectedFileWrites); @@ -101,12 +98,11 @@ export class CommandLineFileWriteAnalyzer extends Disposable implements ICommand const workspaceFolders = this._workspaceContextService.getWorkspace().folders; if (workspaceFolders.length > 0) { for (const fileWrite of fileWrites) { - // Allow safe null device paths (check before variable detection) - if (this._isSafeNullDevicePath(fileWrite)) { + 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) { @@ -135,7 +131,7 @@ export class CommandLineFileWriteAnalyzer extends Disposable implements ICommand } } else { // No workspace folders, allow safe null device paths even without workspace - const hasOnlyNullDevices = fileWrites.every(fw => this._isSafeNullDevicePath(fw)); + const hasOnlyNullDevices = fileWrites.every(fw => fw === nullDevice); if (!hasOnlyNullDevices) { isAutoApproveAllowed = false; this._log('File writes blocked - no workspace folders'); @@ -152,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.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 da1087e499d0e..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 @@ -238,7 +238,6 @@ import { Workspace } from '../../../../../../../platform/workspace/test/common/t suite('edge cases', () => { test('redirection to $null (PowerShell null device) - allow', () => t('Write-Host "hello" > $null', 'outsideWorkspace', true, 1)); - test('redirection to NUL (Windows CMD null device) - allow', () => t('Write-Host "hello" > NUL', '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)); From 7afcca68918ac469ad5dddce0b9877207fb1c6a7 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Tue, 11 Nov 2025 06:48:07 -0800 Subject: [PATCH 4/4] 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 420b9aa4f1868..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 @@ -148,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.toString()}\``).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 {