edits: don't prompt every edit for a folder within a default-system path (#1982)

Closes https://github.com/microsoft/vscode/issues/276193
This commit is contained in:
Connor Peet
2025-11-13 17:41:09 +00:00
committed by GitHub
parent 7d0512d26a
commit 4572f4ee80
2 changed files with 383 additions and 12 deletions
@@ -25,6 +25,7 @@ import { ResourceMap } from '../../../util/vs/base/common/map';
import { Schemas } from '../../../util/vs/base/common/network';
import { isMacintosh, isWindows } from '../../../util/vs/base/common/platform';
import { extUriBiasedIgnorePathCase, normalizePath } from '../../../util/vs/base/common/resources';
import { isDefined } from '../../../util/vs/base/common/types';
import { URI } from '../../../util/vs/base/common/uri';
import { Position as EditorPosition } from '../../../util/vs/editor/common/core/position';
import { ServicesAccessor } from '../../../util/vs/platform/instantiation/common/instantiation';
@@ -619,16 +620,20 @@ const ALWAYS_CHECKED_EDIT_PATTERNS: Readonly<Record<string, boolean>> = {
'**/.vscode/*.json': false,
};
const allPlatformPatterns = [homedir() + '/.*', homedir() + '/.*/**'];
const allPlatformPatterns: (glob.ParsedPattern | string)[] = [
glob.parse(homedir() + '/.*'),
glob.parse(homedir() + '/.*/**'),
];
const specializedPatterns: (glob.ParsedPattern | string | undefined)[] =
isWindows
? [process.env.APPDATA, process.env.LOCALAPPDATA]
: isMacintosh
? [homedir() + '/Library']
: [];
// Path prefixes under which confirmation is unconditionally required
const platformConfirmationRequiredPaths = (
isWindows
? [process.env.APPDATA + '/**', process.env.LOCALAPPDATA + '/**']
: isMacintosh
? [homedir() + '/Library/**']
: []
).concat(allPlatformPatterns).map(p => glob.parse(p));
const platformConfirmationRequiredPaths = specializedPatterns.filter(isDefined).concat(allPlatformPatterns);
/**
* Validates that a path doesn't contain suspicious characters that could be used
@@ -692,7 +697,7 @@ export function assertPathIsSafe(fsPath: string, _isWindows = isWindows): void {
}
}
const enum ConfirmationCheckResult {
export const enum ConfirmationCheckResult {
NoConfirmation,
NoPermissions,
Sensitive,
@@ -704,7 +709,7 @@ const enum ConfirmationCheckResult {
* Returns a function that returns whether a URI is approved for editing without
* further user confirmation.
*/
function makeUriConfirmationChecker(configuration: IConfigurationService, workspaceService: IWorkspaceService, customInstructionsService: ICustomInstructionsService) {
export function makeUriConfirmationChecker(configuration: IConfigurationService, workspaceService: IWorkspaceService, customInstructionsService: ICustomInstructionsService) {
const patterns = configuration.getNonExtensionConfig<Record<string, boolean>>('chat.tools.edits.autoApprove');
const checks = new ResourceMap<{ patterns: { pattern: glob.ParsedPattern; isApproved: boolean }[]; ignoreCasing: boolean }>();
@@ -739,7 +744,21 @@ function makeUriConfirmationChecker(configuration: IConfigurationService, worksp
assertPathIsSafe(fsPath);
if (platformConfirmationRequiredPaths.some(p => p(fsPath))) {
const platformCheckFailed = platformConfirmationRequiredPaths.some(p => {
if (typeof p === 'function') {
return p(fsPath);
}
const parentURI = URI.file(p);
if (extUriBiasedIgnorePathCase.isEqualOrParent(uri, parentURI)) {
// If the workspace is opened in the restricted folder, still allow edits within that workspace
return workspaceFolder && extUriBiasedIgnorePathCase.isEqualOrParent(workspaceFolder, parentURI) ? false : true;
}
return false;
});
if (platformCheckFailed) {
return ConfirmationCheckResult.SystemFile;
}
@@ -4,17 +4,22 @@
*--------------------------------------------------------------------------------------------*/
import * as fs from 'fs';
import { homedir } from 'os';
import { beforeEach, describe, expect, test } from 'vitest';
import { DefaultsOnlyConfigurationService } from '../../../../platform/configuration/common/defaultsOnlyConfigurationService';
import { InMemoryConfigurationService } from '../../../../platform/configuration/test/common/inMemoryConfigurationService';
import type { ICustomInstructionsService } from '../../../../platform/customInstructions/common/customInstructionsService';
import { IAlternativeNotebookContentService } from '../../../../platform/notebook/common/alternativeContent';
import { MockAlternativeNotebookContentService } from '../../../../platform/notebook/common/mockAlternativeContentService';
import { INotebookService } from '../../../../platform/notebook/common/notebookService';
import { TestWorkspaceService } from '../../../../platform/test/node/testWorkspaceService';
import { WorkspaceEdit as WorkspaceEditShim } from '../../../../util/common/test/shims/editing';
import { createTextDocumentData, IExtHostDocumentData, setDocText } from '../../../../util/common/test/shims/textDocument';
import { isMacintosh } from '../../../../util/vs/base/common/platform';
import { URI } from '../../../../util/vs/base/common/uri';
import { WorkspaceEdit } from '../../../../vscodeTypes';
import { applyEdits as applyTextEdits } from '../../../prompt/node/intents';
import { applyEdit, assertPathIsSafe, ContentFormatError, MultipleMatchesError, NoChangeError, NoMatchError } from '../editFileToolUtils';
import { applyEdit, assertPathIsSafe, ConfirmationCheckResult, ContentFormatError, makeUriConfirmationChecker, MultipleMatchesError, NoChangeError, NoMatchError } from '../editFileToolUtils';
describe('replace_string_in_file - applyEdit', () => {
let workspaceEdit: WorkspaceEdit;
@@ -402,3 +407,350 @@ describe('assertPathIsSafe (Windows scenarios)', () => {
expect(() => assertPathIsSafe('C:\\Users\\me\\my~folder\\file.txt', true)).not.toThrow();
});
});
describe('makeUriConfirmationChecker', async () => {
// Mock custom instructions service
class MockCustomInstructionsService implements ICustomInstructionsService {
declare readonly _serviceBrand: undefined;
private externalFiles = new Set<string>();
setExternalFiles(uris: URI[]) {
this.externalFiles.clear();
uris.forEach(uri => this.externalFiles.add(uri.toString()));
}
isExternalInstructionsFile(uri: URI): boolean {
return this.externalFiles.has(uri.toString());
}
fetchInstructionsFromSetting(): Promise<any[]> {
return Promise.resolve([]);
}
fetchInstructionsFromFile(): Promise<any> {
return Promise.resolve(undefined);
}
getAgentInstructions(): Promise<URI[]> {
return Promise.resolve([]);
}
}
let configService: InMemoryConfigurationService;
let workspaceService: TestWorkspaceService;
let customInstructionsService: MockCustomInstructionsService;
beforeEach(() => {
configService = new InMemoryConfigurationService(new DefaultsOnlyConfigurationService());
workspaceService = new TestWorkspaceService([], []);
customInstructionsService = new MockCustomInstructionsService();
});
test('allows files within workspace folder', async () => {
const workspaceFolder = URI.file('/workspace');
workspaceService = new TestWorkspaceService([workspaceFolder], []);
const checker = makeUriConfirmationChecker(configService, workspaceService, customInstructionsService);
const fileInWorkspace = URI.file('/workspace/src/file.ts');
const result = await checker(fileInWorkspace);
expect(result).toBe(ConfirmationCheckResult.NoConfirmation);
});
test('rejects files outside workspace', async () => {
const workspaceFolder = URI.file('/workspace');
workspaceService = new TestWorkspaceService([workspaceFolder], []);
const checker = makeUriConfirmationChecker(configService, workspaceService, customInstructionsService);
const fileOutsideWorkspace = URI.file('/other/file.ts');
const result = await checker(fileOutsideWorkspace);
expect(result).toBe(ConfirmationCheckResult.OutsideWorkspace); // OutsideWorkspace
});
test('allows untitled files', async () => {
workspaceService = new TestWorkspaceService([], []);
const checker = makeUriConfirmationChecker(configService, workspaceService, customInstructionsService);
const untitledFile = URI.parse('untitled:Untitled-1');
const result = await checker(untitledFile);
expect(result).toBe(ConfirmationCheckResult.NoConfirmation);
});
test('allows external instructions files', async () => {
const workspaceFolder = URI.file('/workspace');
workspaceService = new TestWorkspaceService([workspaceFolder], []);
const externalInstruction = URI.file('/external/instruction.md');
customInstructionsService.setExternalFiles([externalInstruction]);
const checker = makeUriConfirmationChecker(configService, workspaceService, customInstructionsService);
const result = await checker(externalInstruction);
expect(result).toBe(ConfirmationCheckResult.NoConfirmation);
});
test('respects autoApprove patterns - allows matching files', async () => {
const workspaceFolder = URI.file('/workspace');
workspaceService = new TestWorkspaceService([workspaceFolder], []);
await configService.setNonExtensionConfig('chat.tools.edits.autoApprove', {
'**/*.test.ts': true,
});
const checker = makeUriConfirmationChecker(configService, workspaceService, customInstructionsService);
const testFile = URI.file('/workspace/src/app.test.ts');
const result = await checker(testFile);
expect(result).toBe(ConfirmationCheckResult.NoConfirmation);
});
test('respects autoApprove patterns - allows non-matching files by default', async () => {
const workspaceFolder = URI.file('/workspace');
workspaceService = new TestWorkspaceService([workspaceFolder], []);
await configService.setNonExtensionConfig('chat.tools.edits.autoApprove', {
'**/*.test.ts': true,
});
const checker = makeUriConfirmationChecker(configService, workspaceService, customInstructionsService);
const prodFile = URI.file('/workspace/src/app.ts');
const result = await checker(prodFile);
// Files in workspace are allowed by default unless explicitly blocked
expect(result).toBe(ConfirmationCheckResult.NoConfirmation);
});
test('respects autoApprove patterns - blocks explicitly denied files', async () => {
const workspaceFolder = URI.file('/workspace');
workspaceService = new TestWorkspaceService([workspaceFolder], []);
await configService.setNonExtensionConfig('chat.tools.edits.autoApprove', {
'**/*.env': false,
});
const checker = makeUriConfirmationChecker(configService, workspaceService, customInstructionsService);
const envFile = URI.file('/workspace/.env');
const result = await checker(envFile);
expect(result).toBe(ConfirmationCheckResult.Sensitive); // Sensitive
});
test('always checks .vscode/*.json files', async () => {
const workspaceFolder = URI.file('/workspace');
workspaceService = new TestWorkspaceService([workspaceFolder], []);
const checker = makeUriConfirmationChecker(configService, workspaceService, customInstructionsService);
const settingsFile = URI.file('/workspace/.vscode/settings.json');
const result = await checker(settingsFile);
expect(result).toBe(ConfirmationCheckResult.Sensitive); // Sensitive - always requires confirmation
});
test('pattern precedence - later patterns override earlier ones', async () => {
const workspaceFolder = URI.file('/workspace');
workspaceService = new TestWorkspaceService([workspaceFolder], []);
await configService.setNonExtensionConfig('chat.tools.edits.autoApprove', {
'**/*.ts': true,
'**/secret.ts': false, // More specific pattern should win
});
const checker = makeUriConfirmationChecker(configService, workspaceService, customInstructionsService);
const secretFile = URI.file('/workspace/src/secret.ts');
const result = await checker(secretFile);
expect(result).toBe(ConfirmationCheckResult.Sensitive); // Sensitive - specific pattern blocks
});
test('handles invalid paths with security checks', async () => {
const workspaceFolder = URI.file('/workspace');
workspaceService = new TestWorkspaceService([workspaceFolder], []);
const checker = makeUriConfirmationChecker(configService, workspaceService, customInstructionsService);
const invalidFile = URI.file('/workspace/file\0.ts');
await expect(checker(invalidFile)).rejects.toThrow();
});
test('multiple workspace folders - allows files in any folder', async () => {
const workspace1 = URI.file('/workspace1');
const workspace2 = URI.file('/workspace2');
workspaceService = new TestWorkspaceService([workspace1, workspace2], []);
const checker = makeUriConfirmationChecker(configService, workspaceService, customInstructionsService);
const fileInWorkspace1 = URI.file('/workspace1/file.ts');
const fileInWorkspace2 = URI.file('/workspace2/file.ts');
expect(await checker(fileInWorkspace1)).toBe(ConfirmationCheckResult.NoConfirmation);
expect(await checker(fileInWorkspace2)).toBe(ConfirmationCheckResult.NoConfirmation);
});
test('caches patterns per workspace folder', async () => {
const workspaceFolder = URI.file('/workspace');
workspaceService = new TestWorkspaceService([workspaceFolder], []);
await configService.setNonExtensionConfig('chat.tools.edits.autoApprove', {
'**/*.test.ts': true,
});
const checker = makeUriConfirmationChecker(configService, workspaceService, customInstructionsService);
// First call should compute patterns
const file1 = URI.file('/workspace/test1.test.ts');
const result1 = await checker(file1);
expect(result1).toBe(ConfirmationCheckResult.NoConfirmation);
// Second call should use cached patterns
const file2 = URI.file('/workspace/test2.test.ts');
const result2 = await checker(file2);
expect(result2).toBe(ConfirmationCheckResult.NoConfirmation);
});
test('case sensitivity handling', async () => {
const workspaceFolder = URI.file('/workspace');
workspaceService = new TestWorkspaceService([workspaceFolder], []);
await configService.setNonExtensionConfig('chat.tools.edits.autoApprove', {
'**/Test.ts': true,
});
const checker = makeUriConfirmationChecker(configService, workspaceService, customInstructionsService);
// Case handling should depend on platform
const testFile = URI.file('/workspace/Test.ts');
const result = await checker(testFile);
expect(result).toBe(ConfirmationCheckResult.NoConfirmation);
});
test('empty autoApprove config - blocks all non-workspace files', async () => {
const workspaceFolder = URI.file('/workspace');
workspaceService = new TestWorkspaceService([workspaceFolder], []);
// No autoApprove config set
const checker = makeUriConfirmationChecker(configService, workspaceService, customInstructionsService);
const file = URI.file('/workspace/src/file.ts');
const result = await checker(file);
// Without explicit approval, files should still be allowed if not sensitive
expect(result).toBe(ConfirmationCheckResult.NoConfirmation);
});
test('workspace folder excluded by pattern - still allows workspace edits', async () => {
const workspaceFolder = URI.file('/workspace');
workspaceService = new TestWorkspaceService([workspaceFolder], []);
await configService.setNonExtensionConfig('chat.tools.edits.autoApprove', {
'/workspace/**': false,
});
const checker = makeUriConfirmationChecker(configService, workspaceService, customInstructionsService);
// Pattern matching the workspace folder itself should not be included
const file = URI.file('/workspace/file.ts');
const result = await checker(file);
// The pattern should be ignored because it matches the workspace root
expect(result).toBe(ConfirmationCheckResult.NoConfirmation);
});
if (isMacintosh) {
test('pattern matching macOS Library path', async () => {
// Simulate a workspace opened in ~/Library (which is normally restricted)
const workspaceFolder = URI.file('/');
workspaceService = new TestWorkspaceService([workspaceFolder], []);
await configService.setNonExtensionConfig('chat.tools.edits.autoApprove', {});
const checker = makeUriConfirmationChecker(configService, workspaceService, customInstructionsService);
const normalFile = URI.file(`${homedir()}/Library/MyApp/src/app.ts`);
expect(await checker(normalFile)).toBe(ConfirmationCheckResult.SystemFile);
});
test('pattern matching workspace folder on macOS Library path', async () => {
// Simulate a workspace opened in ~/Library (which is normally restricted)
const libraryWorkspace = URI.file(`${homedir()}/Library/MyApp`);
workspaceService = new TestWorkspaceService([libraryWorkspace], []);
await configService.setNonExtensionConfig('chat.tools.edits.autoApprove', {
'**/*.config': false,
});
const checker = makeUriConfirmationChecker(configService, workspaceService, customInstructionsService);
const normalFile = URI.file(`${homedir()}/Library/MyApp/src/app.ts`);
const configFile = URI.file(`${homedir()}/Library/MyApp/settings.config`);
expect(await checker(normalFile)).toBe(ConfirmationCheckResult.NoConfirmation);
expect(await checker(configFile)).toBe(ConfirmationCheckResult.Sensitive);
});
}
test('nested pattern matching', async () => {
const workspaceFolder = URI.file('/workspace');
workspaceService = new TestWorkspaceService([workspaceFolder], []);
await configService.setNonExtensionConfig('chat.tools.edits.autoApprove', {
'**/config/**': false,
'**/config/test/**': true, // More specific pattern
});
const checker = makeUriConfirmationChecker(configService, workspaceService, customInstructionsService);
// More specific pattern should override the general one
const testConfigFile = URI.file('/workspace/config/test/settings.json');
const result = await checker(testConfigFile);
expect(result).toBe(ConfirmationCheckResult.NoConfirmation); // allowed by more specific pattern
});
test('handles relative workspace patterns correctly', async () => {
const workspaceFolder = URI.file('/workspace');
workspaceService = new TestWorkspaceService([workspaceFolder], []);
await configService.setNonExtensionConfig('chat.tools.edits.autoApprove', {
'src/**/*.ts': true,
'dist/**': false,
});
const checker = makeUriConfirmationChecker(configService, workspaceService, customInstructionsService);
const srcFile = URI.file('/workspace/src/app.ts');
const distFile = URI.file('/workspace/dist/app.js');
expect(await checker(srcFile)).toBe(ConfirmationCheckResult.NoConfirmation);
expect(await checker(distFile)).toBe(ConfirmationCheckResult.Sensitive); // Sensitive - explicitly blocked
});
test('pattern matching is workspace-relative', async () => {
const workspace1 = URI.file('/workspace1');
const workspace2 = URI.file('/workspace2');
workspaceService = new TestWorkspaceService([workspace1, workspace2], []);
await configService.setNonExtensionConfig('chat.tools.edits.autoApprove', {
'secrets/**': false,
});
const checker = makeUriConfirmationChecker(configService, workspaceService, customInstructionsService);
const secretsInWorkspace1 = URI.file('/workspace1/secrets/api-key.txt');
const secretsInWorkspace2 = URI.file('/workspace2/secrets/token.txt');
// Pattern should apply to both workspaces
expect(await checker(secretsInWorkspace1)).toBe(ConfirmationCheckResult.Sensitive); // Sensitive
expect(await checker(secretsInWorkspace2)).toBe(ConfirmationCheckResult.Sensitive); // Sensitive
});
test('complex glob patterns', async () => {
const workspaceFolder = URI.file('/workspace');
workspaceService = new TestWorkspaceService([workspaceFolder], []);
await configService.setNonExtensionConfig('chat.tools.edits.autoApprove', {
'**/*.{env,secret,key}': false,
'**/test/**/*.env': true, // Exception for test env files
});
const checker = makeUriConfirmationChecker(configService, workspaceService, customInstructionsService);
const prodEnv = URI.file('/workspace/.env');
const testEnv = URI.file('/workspace/test/integration.env');
const apiKey = URI.file('/workspace/config/api.key');
expect(await checker(prodEnv)).toBe(ConfirmationCheckResult.Sensitive); // Sensitive - matches block pattern
expect(await checker(testEnv)).toBe(ConfirmationCheckResult.NoConfirmation); // exception pattern
expect(await checker(apiKey)).toBe(ConfirmationCheckResult.Sensitive); // Sensitive - matches block pattern
});
});