Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 5 additions & 46 deletions extensions/copilot/src/extension/tools/node/editFileToolUtils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,7 @@
*--------------------------------------------------------------------------------------------*/

import { t } from '@vscode/l10n';
import { realpath } from 'fs/promises';
import { homedir } from 'os';
import * as path from 'path';
import type { LanguageModelChat, PreparedToolInvocation } from 'vscode';
import { ToolName } from '../common/toolNames';
import { IConfigurationService } from '../../../platform/configuration/common/configurationService';
Expand Down Expand Up @@ -37,6 +35,7 @@ import { ServicesAccessor } from '../../../util/vs/platform/instantiation/common
import { EndOfLine, Position, Range, TextEdit } from '../../../vscodeTypes';
import { IBuildPromptContext } from '../../prompt/common/intents';
import { formatUriForFileWidget } from '../common/toolUtils';
import { resolveRealPathForNonexistent } from './toolUtils';

// Simplified Hunk type for the patch
interface Hunk {
Expand Down Expand Up @@ -803,46 +802,6 @@ export const enum ConfirmationCheckResult {
OutsideWorkspace,
}

/**
* Resolves the real path of `fsPath`, walking up the parent chain when the path
* (or its ancestors) does not yet exist on disk. This ensures that a symlink at
* any ancestor.
*/
async function resolveRealPathForNonexistent(fsPath: string): Promise<string> {
try {
return await realpath(fsPath);
} catch (e) {
if ((e as NodeJS.ErrnoException).code !== 'ENOENT') {
throw e;
}
}

const tail: string[] = [path.basename(fsPath)];
let current = path.dirname(fsPath);
while (true) {
const parent = path.dirname(current);
if (parent === current) {
// Reached the filesystem root without finding an existing ancestor.
// Don't attempt to resolve the root itself — on Windows, realpath('\\')
// normalizes to a drive letter (e.g. 'C:\\'), which would otherwise look
// like a redirect even though no symlink was involved.
return fsPath;
}
try {
const resolved = await realpath(current);
return path.join(resolved, ...tail);
} catch (e) {
const code = (e as NodeJS.ErrnoException).code;
if (code !== 'ENOENT' && code !== 'ENOTDIR') {
throw e;
}
}
tail.unshift(path.basename(current));
current = parent;
}
}


/**
* Returns a function that returns whether a URI is approved for editing without
* further user confirmation.
Expand Down Expand Up @@ -939,11 +898,11 @@ export function makeUriConfirmationChecker(configuration: IConfigurationService,
const toCheck = [normalizePath(uri)];
if (uri.scheme === Schemas.file) {
try {
const linked = await resolveRealPathForNonexistent(uri.fsPath);
assertPathIsSafe(linked);
const linked = await resolveRealPathForNonexistent(uri);
assertPathIsSafe(linked.fsPath);

if (linked !== uri.fsPath) {
toCheck.push(URI.file(linked));
if (!extUriBiasedIgnorePathCase.isEqual(linked, uri)) {
toCheck.push(linked);
}
} catch (e) {
if ((e as NodeJS.ErrnoException).code === 'EPERM') {
Expand Down
12 changes: 9 additions & 3 deletions extensions/copilot/src/extension/tools/node/readFileTool.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -220,19 +220,25 @@ export class ReadFileTool implements ICopilotTool<ReadFileParams> {
}

// Check if file is external (outside workspace, not open in editor, etc.)
const isExternal = await this.instantiationService.invokeFunction(
const { needsConfirmation, realPath } = await this.instantiationService.invokeFunction(
accessor => isFileExternalAndNeedsConfirmation(accessor, uri!, this._promptContext, { readOnly: true, workingDirectory: options.workingDirectory })
);

if (isExternal) {
if (needsConfirmation) {
// Still check content exclusion (copilot ignore)
await this.instantiationService.invokeFunction(
accessor => assertFileNotContentExcluded(accessor, uri!)
);

const folderUri = dirname(uri);

const message = this.workspaceService.getWorkspaceFolders().length === 1 ? new MarkdownString(l10n.t`${formatUriForFileWidget(uri)} is outside of the current folder in ${formatUriForFileWidget(folderUri)}.`) : new MarkdownString(l10n.t`${formatUriForFileWidget(uri)} is outside of the current workspace in ${formatUriForFileWidget(folderUri)}.`);
const message = realPath
? this.workspaceService.getWorkspaceFolders().length === 1
? new MarkdownString(l10n.t`${formatUriForFileWidget(uri)} links to ${formatUriForFileWidget(realPath)}, which is outside the current folder.`)
: new MarkdownString(l10n.t`${formatUriForFileWidget(uri)} links to ${formatUriForFileWidget(realPath)}, which is outside the current workspace.`)
: this.workspaceService.getWorkspaceFolders().length === 1
? new MarkdownString(l10n.t`${formatUriForFileWidget(uri)} is outside of the current folder in ${formatUriForFileWidget(folderUri)}.`)
: new MarkdownString(l10n.t`${formatUriForFileWidget(uri)} is outside of the current workspace in ${formatUriForFileWidget(folderUri)}.`);

// Return confirmation request for external file
// The folder-based "allow this session" option is provided by the core confirmation contribution
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -278,9 +278,9 @@ class SearchSubagentTool implements ICopilotTool<ISearchSubagentParams> {
// (e.g. file missing), keep the original line with the error.
let isExternal = false;
try {
isExternal = await this.instantiationService.invokeFunction(accessor =>
({ needsConfirmation: isExternal } = await this.instantiationService.invokeFunction(accessor =>
isFileExternalAndNeedsConfirmation(accessor, uri, this._inputContext, { readOnly: true, workingDirectory })
);
));
Comment on lines +281 to +283
} catch {
// isFileExternalAndNeedsConfirmation throws for nonexistent files;
// treat that as "not external" so the original line is preserved.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ suite('SearchSubagentTool', () => {
const { tool } = makeToolInstance(false, 4, {
invokeFunction: sequencedInvokeFunction(
() => { throw new Error('outside workspace'); },
true,
{ needsConfirmation: true, realPath: undefined },
),
});

Expand All @@ -287,7 +287,7 @@ suite('SearchSubagentTool', () => {
const { tool } = makeToolInstance(false, 4, {
invokeFunction: sequencedInvokeFunction(
undefined,
false,
{ needsConfirmation: false, realPath: undefined },
),
openTextDocument: async () => { throw new Error('file not found'); },
});
Expand Down
129 changes: 119 additions & 10 deletions extensions/copilot/src/extension/tools/node/test/toolUtils.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { afterAll, beforeAll, beforeEach, describe, expect, it, suite, test } from 'vitest';
import * as fs from 'fs';
import { tmpdir } from 'os';
import * as path from 'path';
import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, suite, test } from 'vitest';
import { ConfigKey, IConfigurationService } from '../../../../platform/configuration/common/configurationService';
import { InMemoryConfigurationService } from '../../../../platform/configuration/test/common/inMemoryConfigurationService';
import { ICustomInstructionsService } from '../../../../platform/customInstructions/common/customInstructionsService';
Expand All @@ -18,14 +21,15 @@ import { WorkingDirectory } from '../../../../platform/workspace/common/workingD
import { CancellationToken } from '../../../../util/vs/base/common/cancellation';
import { ResourceSet } from '../../../../util/vs/base/common/map';
import { posix } from '../../../../util/vs/base/common/path';
import { isWindows } from '../../../../util/vs/base/common/platform';
import { URI } from '../../../../util/vs/base/common/uri';
import { SyncDescriptor } from '../../../../util/vs/platform/instantiation/common/descriptors';
import { IInstantiationService } from '../../../../util/vs/platform/instantiation/common/instantiation';
import { ChatVariablesCollection, CustomizationsIndexId } from '../../../prompt/common/chatVariablesCollection';
import { IBuildPromptContext } from '../../../prompt/common/intents';
import { createExtensionUnitTestingServices } from '../../../test/node/services';
import { encodeUrlHostname } from '../../common/toolUtils';
import { assertFileOkForTool, inputGlobToPattern, isDirExternalAndNeedsConfirmation, isFileExternalAndNeedsConfirmation } from '../toolUtils';
import { assertFileOkForTool, inputGlobToPattern, isDirExternalAndNeedsConfirmation, isExternalSymlinkedFile, isFileExternalAndNeedsConfirmation } from '../toolUtils';

class TestIgnoreService extends NullIgnoreService {
private readonly _ignoredUris = new Set<string>();
Expand Down Expand Up @@ -159,7 +163,7 @@ suite('toolUtils - additionalReadAccessPaths', () => {

describe('isFileExternalAndNeedsConfirmation', () => {
test('workspace file does not need confirmation', async () => {
expect(await invokeIsFileExternalAndNeedsConfirmation(URI.file('/workspace/file.ts'))).toBe(false);
expect(await invokeIsFileExternalAndNeedsConfirmation(URI.file('/workspace/file.ts'))).toEqual({ needsConfirmation: false, realPath: undefined });
});

test('external file that does not exist throws', async () => {
Expand All @@ -174,17 +178,17 @@ suite('toolUtils - additionalReadAccessPaths', () => {

test('non-existent workspace file does not need confirmation', async () => {
// Non-existent files within the workspace should also not trigger confirmation
expect(await invokeIsFileExternalAndNeedsConfirmation(URI.file('/workspace/nonexistent.ts'))).toBe(false);
expect(await invokeIsFileExternalAndNeedsConfirmation(URI.file('/workspace/nonexistent.ts'))).toEqual({ needsConfirmation: false, realPath: undefined });
});

test('external file under additional paths with readOnly does not need confirmation', async () => {
await configService.setConfig(ConfigKey.AdditionalReadAccessPaths, ['/external']);
expect(await invokeIsFileExternalAndNeedsConfirmation(URI.file('/external/file.ts'), true)).toBe(false);
expect(await invokeIsFileExternalAndNeedsConfirmation(URI.file('/external/file.ts'), true)).toEqual({ needsConfirmation: false, realPath: undefined });
});

test('nested file under additional paths with readOnly does not need confirmation', async () => {
await configService.setConfig(ConfigKey.AdditionalReadAccessPaths, ['/external']);
expect(await invokeIsFileExternalAndNeedsConfirmation(URI.file('/external/deep/nested/file.ts'), true)).toBe(false);
expect(await invokeIsFileExternalAndNeedsConfirmation(URI.file('/external/deep/nested/file.ts'), true)).toEqual({ needsConfirmation: false, realPath: undefined });
});

test('external file under additional paths without readOnly throws when file does not exist', async () => {
Expand Down Expand Up @@ -256,7 +260,7 @@ suite('toolUtils - additionalReadAccessPaths', () => {
});

test('isFileExternalAndNeedsConfirmation: file within workingDirectory is not external', async () => {
expect(await invokeIsFileExternalWithWd(URI.file('/my-project/src/file.ts'))).toBe(false);
expect(await invokeIsFileExternalWithWd(URI.file('/my-project/src/file.ts'))).toEqual({ needsConfirmation: false, realPath: undefined });
});

test('isFileExternalAndNeedsConfirmation: workspace file is external when workingDirectory is set', async () => {
Expand Down Expand Up @@ -383,7 +387,7 @@ suite('toolUtils - external file existence', () => {
test('external file that exists needs confirmation', async () => {
// Mock an external file that actually exists
mockFs.mockFile(URI.file('/external/existing-file.ts'), 'content');
expect(await invokeIsFileExternalAndNeedsConfirmation(URI.file('/external/existing-file.ts'))).toBe(true);
expect(await invokeIsFileExternalAndNeedsConfirmation(URI.file('/external/existing-file.ts'))).toEqual({ needsConfirmation: true, realPath: undefined });
});

test('external file that does not exist throws', async () => {
Expand All @@ -395,12 +399,117 @@ suite('toolUtils - external file existence', () => {
test('workspace file does not need confirmation even if it exists', async () => {
// Mock a workspace file
mockFs.mockFile(URI.file('/workspace/file.ts'), 'content');
expect(await invokeIsFileExternalAndNeedsConfirmation(URI.file('/workspace/file.ts'))).toBe(false);
expect(await invokeIsFileExternalAndNeedsConfirmation(URI.file('/workspace/file.ts'))).toEqual({ needsConfirmation: false, realPath: undefined });
});

test('workspace file does not need confirmation even if it does not exist', async () => {
// Non-existent workspace file
expect(await invokeIsFileExternalAndNeedsConfirmation(URI.file('/workspace/nonexistent.ts'))).toBe(false);
expect(await invokeIsFileExternalAndNeedsConfirmation(URI.file('/workspace/nonexistent.ts'))).toEqual({ needsConfirmation: false, realPath: undefined });
});
});

describe.skipIf(isWindows)('isExternalSymlinkedFile', () => {
let temporaryDirectory: string;
let workspaceDirectory: string;
let externalDirectory: string;

beforeEach(() => {
temporaryDirectory = fs.mkdtempSync(path.join(tmpdir(), 'toolutils-symlink-'));
workspaceDirectory = fs.realpathSync(fs.mkdtempSync(path.join(temporaryDirectory, 'workspace-')));
externalDirectory = fs.realpathSync(fs.mkdtempSync(path.join(temporaryDirectory, 'external-')));
});

afterEach(() => {
fs.rmSync(temporaryDirectory, { recursive: true, force: true });
});

function getFolder(uri: URI): URI | undefined {
const workspaceUri = URI.file(workspaceDirectory);
return uri.fsPath === workspaceDirectory || uri.fsPath.startsWith(`${workspaceDirectory}${path.sep}`) ? workspaceUri : undefined;
}

async function invokeIsFileExternal(uri: URI) {
const services = createExtensionUnitTestingServices();
services.define(IWorkspaceService, new SyncDescriptor(
TestWorkspaceService,
[[URI.file(workspaceDirectory)], []]
));
const accessor = services.createTestingAccessor();
try {
return await accessor.get(IInstantiationService).invokeFunction(acc => isFileExternalAndNeedsConfirmation(acc, uri));
} finally {
accessor.dispose();
}
}

test('returns true when the file symlink points outside the workspace', async () => {
const externalFile = path.join(externalDirectory, 'external.txt');
const symlinkedFile = path.join(workspaceDirectory, 'linked.txt');
fs.writeFileSync(externalFile, 'content');
fs.symlinkSync(externalFile, symlinkedFile);

await expect(isExternalSymlinkedFile(URI.file(symlinkedFile), getFolder)).resolves.toBe(true);
});

test('returns the real path when a workspace symlink points outside the workspace', async () => {
const externalFile = path.join(externalDirectory, 'external.txt');
const symlinkedFile = path.join(workspaceDirectory, 'linked.txt');
fs.writeFileSync(externalFile, 'content');
fs.symlinkSync(externalFile, symlinkedFile);

await expect(invokeIsFileExternal(URI.file(symlinkedFile))).resolves.toEqual({
needsConfirmation: true,
realPath: URI.file(externalFile),
});
});

test('returns true when a parent directory symlink points outside the workspace', async () => {
const externalFile = path.join(externalDirectory, 'external.txt');
const symlinkedDirectory = path.join(workspaceDirectory, 'linked');
fs.writeFileSync(externalFile, 'content');
fs.symlinkSync(externalDirectory, symlinkedDirectory, 'dir');

await expect(isExternalSymlinkedFile(URI.file(path.join(symlinkedDirectory, 'external.txt')), getFolder)).resolves.toBe(true);
});

test('returns true for a nonexistent file under a parent directory symlink that points outside the workspace', async () => {
const symlinkedDirectory = path.join(workspaceDirectory, 'linked');
fs.symlinkSync(externalDirectory, symlinkedDirectory, 'dir');

await expect(isExternalSymlinkedFile(URI.file(path.join(symlinkedDirectory, 'missing.txt')), getFolder)).resolves.toBe(true);
});

test('returns false when the symlink target is inside the workspace', async () => {
const targetFile = path.join(workspaceDirectory, 'target.txt');
const symlinkedFile = path.join(workspaceDirectory, 'linked.txt');
fs.writeFileSync(targetFile, 'content');
fs.symlinkSync(targetFile, symlinkedFile);

await expect(isExternalSymlinkedFile(URI.file(symlinkedFile), getFolder)).resolves.toBe(false);
});

test('returns false when the file is not a symlink', async () => {
const file = path.join(workspaceDirectory, 'file.txt');
fs.writeFileSync(file, 'content');

await expect(isExternalSymlinkedFile(URI.file(file), getFolder)).resolves.toBe(false);
});

test('returns false when the workspace folder itself is symlinked', async () => {
const symlinkedWorkspace = path.join(temporaryDirectory, 'workspace-link');
const workspaceFile = path.join(workspaceDirectory, 'file.txt');
fs.writeFileSync(workspaceFile, 'content');
fs.symlinkSync(workspaceDirectory, symlinkedWorkspace, 'dir');
const symlinkedWorkspaceUri = URI.file(symlinkedWorkspace);

await expect(isExternalSymlinkedFile(
URI.file(path.join(symlinkedWorkspace, 'file.txt')),
uri => uri.fsPath.startsWith(`${symlinkedWorkspace}${path.sep}`) ? symlinkedWorkspaceUri : undefined
)).resolves.toBe(false);
});

test('returns false when the file does not exist', async () => {
await expect(isExternalSymlinkedFile(URI.file(path.join(workspaceDirectory, 'missing.txt')), getFolder)).resolves.toBe(false);
});
});

Expand Down
Loading
Loading