mirror of
https://github.com/microsoft/vscode.git
synced 2026-07-15 00:14:20 +01:00
fix: fetch tool should not warn when reading files inside the workspace (#303789)
This commit is contained in:
@@ -13,6 +13,7 @@ import { URI } from '../../../../../base/common/uri.js';
|
||||
import { localize } from '../../../../../nls.js';
|
||||
import { IFileService } from '../../../../../platform/files/common/files.js';
|
||||
import { IWebContentExtractorService, WebContentExtractResult } from '../../../../../platform/webContentExtractor/common/webContentExtractor.js';
|
||||
import { IWorkspaceContextService } from '../../../../../platform/workspace/common/workspace.js';
|
||||
import { detectEncodingFromBuffer } from '../../../../services/textfile/common/encoding.js';
|
||||
import { ITrustedDomainService } from '../../../url/browser/trustedDomainService.js';
|
||||
import { IChatService } from '../../common/chatService/chatService.js';
|
||||
@@ -56,6 +57,7 @@ export class FetchWebPageTool implements IToolImpl {
|
||||
@IFileService private readonly _fileService: IFileService,
|
||||
@ITrustedDomainService private readonly _trustedDomainService: ITrustedDomainService,
|
||||
@IChatService private readonly _chatService: IChatService,
|
||||
@IWorkspaceContextService private readonly _workspaceContextService: IWorkspaceContextService,
|
||||
) { }
|
||||
|
||||
async invoke(invocation: IToolInvocation, _countTokens: CountTokensCallback, _progress: ToolProgress, token: CancellationToken): Promise<IToolResult> {
|
||||
@@ -170,7 +172,15 @@ export class FetchWebPageTool implements IToolImpl {
|
||||
}
|
||||
|
||||
const invalid = [...Array.from(invalidUris), ...additionalInvalidUrls];
|
||||
const urlsNeedingConfirmation = new ResourceSet([...webUris.values(), ...validFileUris]);
|
||||
// All valid URIs (web + file) for display in messages
|
||||
const allFetchedUris = new ResourceSet([...webUris.values(), ...validFileUris]);
|
||||
// File URIs that are inside the workspace don't need confirmation — they're already accessible
|
||||
// and don't carry the web content risks (prompt injection, malicious redirects).
|
||||
// File URIs outside the workspace are treated like web URIs and require confirmation.
|
||||
const fileUrisOutsideWorkspace = validFileUris.filter(
|
||||
uri => !this._workspaceContextService.getWorkspaceFolder(uri)
|
||||
);
|
||||
const urlsNeedingConfirmation = new ResourceSet([...webUris.values(), ...fileUrisOutsideWorkspace]);
|
||||
|
||||
const pastTenseMessage = invalid.length
|
||||
? invalid.length > 1
|
||||
@@ -178,7 +188,7 @@ export class FetchWebPageTool implements IToolImpl {
|
||||
? new MarkdownString(
|
||||
localize(
|
||||
'fetchWebPage.pastTenseMessage.plural',
|
||||
'Fetched {0} resources, but the following were invalid URLs:\n\n{1}\n\n', urlsNeedingConfirmation.size, invalid.map(url => `- ${url}`).join('\n')
|
||||
'Fetched {0} resources, but the following were invalid URLs:\n\n{1}\n\n', allFetchedUris.size, invalid.map(url => `- ${url}`).join('\n')
|
||||
))
|
||||
// If there is only one invalid URL, show it
|
||||
: new MarkdownString(
|
||||
@@ -190,11 +200,11 @@ export class FetchWebPageTool implements IToolImpl {
|
||||
: new MarkdownString();
|
||||
|
||||
const invocationMessage = new MarkdownString();
|
||||
if (urlsNeedingConfirmation.size > 1) {
|
||||
pastTenseMessage.appendMarkdown(localize('fetchWebPage.pastTenseMessageResult.plural', 'Fetched {0} resources', urlsNeedingConfirmation.size));
|
||||
invocationMessage.appendMarkdown(localize('fetchWebPage.invocationMessage.plural', 'Fetching {0} resources', urlsNeedingConfirmation.size));
|
||||
} else if (urlsNeedingConfirmation.size === 1) {
|
||||
const url = Iterable.first(urlsNeedingConfirmation)!.toString(true);
|
||||
if (allFetchedUris.size > 1) {
|
||||
pastTenseMessage.appendMarkdown(localize('fetchWebPage.pastTenseMessageResult.plural', 'Fetched {0} resources', allFetchedUris.size));
|
||||
invocationMessage.appendMarkdown(localize('fetchWebPage.invocationMessage.plural', 'Fetching {0} resources', allFetchedUris.size));
|
||||
} else if (allFetchedUris.size === 1) {
|
||||
const url = Iterable.first(allFetchedUris)!.toString(true);
|
||||
// If the URL is too long or it's a file url, show it as a link... otherwise, show it as plain text
|
||||
if (url.length > 400 || validFileUris.length === 1) {
|
||||
pastTenseMessage.appendMarkdown(localize({
|
||||
|
||||
+117
-1
@@ -11,8 +11,9 @@ import { ResourceMap } from '../../../../../../../base/common/map.js';
|
||||
import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../../base/test/common/utils.js';
|
||||
import { IFileContent, IReadFileOptions } from '../../../../../../../platform/files/common/files.js';
|
||||
import { IWebContentExtractorService, WebContentExtractResult } from '../../../../../../../platform/webContentExtractor/common/webContentExtractor.js';
|
||||
import { testWorkspace } from '../../../../../../../platform/workspace/test/common/testWorkspace.js';
|
||||
import { FetchWebPageTool } from '../../../../electron-browser/builtInTools/fetchPageTool.js';
|
||||
import { TestFileService } from '../../../../../../test/common/workbenchTestServices.js';
|
||||
import { TestContextService, TestFileService } from '../../../../../../test/common/workbenchTestServices.js';
|
||||
import { MockTrustedDomainService } from '../../../../../url/test/browser/mockTrustedDomainService.js';
|
||||
import { InternalFetchWebPageToolId } from '../../../../common/tools/builtinTools/tools.js';
|
||||
import { MockChatService } from '../../../common/chatService/mockChatService.js';
|
||||
@@ -91,6 +92,7 @@ suite('FetchWebPageTool', () => {
|
||||
new ExtendedTestFileService(fileContentMap),
|
||||
new MockTrustedDomainService(),
|
||||
new MockChatService(),
|
||||
new TestContextService(),
|
||||
);
|
||||
|
||||
const testUrls = [
|
||||
@@ -140,6 +142,7 @@ suite('FetchWebPageTool', () => {
|
||||
new ExtendedTestFileService(new ResourceMap<string | VSBuffer>()),
|
||||
new MockTrustedDomainService([]),
|
||||
new MockChatService(),
|
||||
new TestContextService(),
|
||||
);
|
||||
|
||||
// Test empty array
|
||||
@@ -189,6 +192,7 @@ suite('FetchWebPageTool', () => {
|
||||
new ExtendedTestFileService(fileContentMap),
|
||||
new MockTrustedDomainService(),
|
||||
new MockChatService(),
|
||||
new TestContextService(),
|
||||
);
|
||||
|
||||
const preparation = await tool.prepareToolInvocation(
|
||||
@@ -203,6 +207,100 @@ suite('FetchWebPageTool', () => {
|
||||
assert.ok(messageText.includes('invalid://invalid'), 'Should mention invalid URL');
|
||||
});
|
||||
|
||||
test('should not show confirmation dialog for file URIs inside the workspace', async () => {
|
||||
// Use a workspace rooted at /workspaceRoot
|
||||
const workspaceRoot = URI.file('/workspaceRoot');
|
||||
const workspaceContextService = new TestContextService(testWorkspace(workspaceRoot));
|
||||
|
||||
const fileContentMap = new ResourceMap<string | VSBuffer>([
|
||||
[URI.file('/workspaceRoot/plan.md'), 'Plan content'],
|
||||
[URI.file('/workspaceRoot/subdir/notes.txt'), 'Notes content'],
|
||||
]);
|
||||
|
||||
const tool = new FetchWebPageTool(
|
||||
new TestWebContentExtractorService(new ResourceMap<string>()),
|
||||
new ExtendedTestFileService(fileContentMap),
|
||||
new MockTrustedDomainService([]),
|
||||
new MockChatService(),
|
||||
workspaceContextService,
|
||||
);
|
||||
|
||||
// File inside workspace - should NOT trigger confirmation
|
||||
const preparation = await tool.prepareToolInvocation(
|
||||
{ parameters: { urls: [URI.file('/workspaceRoot/plan.md').toString()] }, toolCallId: 'test-file-in-ws', chatSessionResource: undefined },
|
||||
CancellationToken.None
|
||||
);
|
||||
assert.ok(preparation, 'Should return prepared invocation');
|
||||
assert.strictEqual(preparation.confirmationMessages?.title, undefined, 'File inside workspace should not show confirmation dialog');
|
||||
assert.strictEqual(preparation.confirmationMessages?.confirmResults, false, 'File inside workspace should not require post-confirmation');
|
||||
});
|
||||
|
||||
test('should show confirmation dialog for file URIs outside the workspace', async () => {
|
||||
// Use a workspace rooted at /workspaceRoot
|
||||
const workspaceRoot = URI.file('/workspaceRoot');
|
||||
const workspaceContextService = new TestContextService(testWorkspace(workspaceRoot));
|
||||
|
||||
const fileContentMap = new ResourceMap<string | VSBuffer>([
|
||||
[URI.file('/tmp/external-plan.md'), 'External plan content'],
|
||||
]);
|
||||
|
||||
const tool = new FetchWebPageTool(
|
||||
new TestWebContentExtractorService(new ResourceMap<string>()),
|
||||
new ExtendedTestFileService(fileContentMap),
|
||||
new MockTrustedDomainService([]),
|
||||
new MockChatService(),
|
||||
workspaceContextService,
|
||||
);
|
||||
|
||||
// File outside workspace - should still trigger confirmation
|
||||
const preparation = await tool.prepareToolInvocation(
|
||||
{ parameters: { urls: [URI.file('/tmp/external-plan.md').toString()] }, toolCallId: 'test-file-outside-ws', chatSessionResource: undefined },
|
||||
CancellationToken.None
|
||||
);
|
||||
assert.ok(preparation, 'Should return prepared invocation');
|
||||
assert.ok(preparation.confirmationMessages?.title, 'File outside workspace should show confirmation dialog');
|
||||
assert.strictEqual(preparation.confirmationMessages?.confirmResults, true, 'File outside workspace should require post-confirmation');
|
||||
});
|
||||
|
||||
test('workspace file mixed with untrusted web URI: only web URI triggers confirmation', async () => {
|
||||
const workspaceRoot = URI.file('/workspaceRoot');
|
||||
const workspaceContextService = new TestContextService(testWorkspace(workspaceRoot));
|
||||
|
||||
const webContentMap = new ResourceMap<string>([
|
||||
[URI.parse('https://example.com'), 'Web content']
|
||||
]);
|
||||
const fileContentMap = new ResourceMap<string | VSBuffer>([
|
||||
[URI.file('/workspaceRoot/plan.md'), 'Plan content']
|
||||
]);
|
||||
|
||||
const tool = new FetchWebPageTool(
|
||||
new TestWebContentExtractorService(webContentMap),
|
||||
new ExtendedTestFileService(fileContentMap),
|
||||
new MockTrustedDomainService([]), // No trusted domains
|
||||
new MockChatService(),
|
||||
workspaceContextService,
|
||||
);
|
||||
|
||||
// Mix: one untrusted web URI + one workspace file URI
|
||||
const preparation = await tool.prepareToolInvocation(
|
||||
{
|
||||
parameters: { urls: ['https://example.com', URI.file('/workspaceRoot/plan.md').toString()] },
|
||||
toolCallId: 'test-mixed',
|
||||
chatSessionResource: undefined
|
||||
},
|
||||
CancellationToken.None
|
||||
);
|
||||
assert.ok(preparation, 'Should return prepared invocation');
|
||||
// Confirmation should only be for the web URI
|
||||
assert.ok(preparation.confirmationMessages?.title, 'Should show confirmation for untrusted web URI');
|
||||
// The confirmation message should mention only the web URI, not the workspace file
|
||||
const msgValue = typeof preparation.confirmationMessages?.message === 'string'
|
||||
? preparation.confirmationMessages.message
|
||||
: preparation.confirmationMessages?.message?.value ?? '';
|
||||
assert.ok(!msgValue.includes('/workspaceRoot/'), 'Confirmation message should not mention workspace file');
|
||||
assert.ok(msgValue.includes('example.com'), 'Confirmation message should mention web URI');
|
||||
});
|
||||
|
||||
test('should approve when all URLs were mentioned in chat', async () => {
|
||||
const webContentMap = new ResourceMap<string>([
|
||||
[URI.parse('https://valid.com'), 'Valid content']
|
||||
@@ -227,6 +325,7 @@ suite('FetchWebPageTool', () => {
|
||||
};
|
||||
},
|
||||
}),
|
||||
new TestContextService(),
|
||||
);
|
||||
|
||||
const preparation1 = await tool.prepareToolInvocation(
|
||||
@@ -261,6 +360,7 @@ suite('FetchWebPageTool', () => {
|
||||
new ExtendedTestFileService(fileContentMap),
|
||||
new MockTrustedDomainService(),
|
||||
new MockChatService(),
|
||||
new TestContextService(),
|
||||
);
|
||||
|
||||
const result = await tool.invoke(
|
||||
@@ -309,6 +409,7 @@ suite('FetchWebPageTool', () => {
|
||||
new ExtendedTestFileService(fileContentMap),
|
||||
new MockTrustedDomainService(),
|
||||
new MockChatService(),
|
||||
new TestContextService(),
|
||||
);
|
||||
|
||||
const result = await tool.invoke(
|
||||
@@ -350,6 +451,7 @@ suite('FetchWebPageTool', () => {
|
||||
new ExtendedTestFileService(fileContentMap),
|
||||
new MockTrustedDomainService(),
|
||||
new MockChatService(),
|
||||
new TestContextService(),
|
||||
);
|
||||
|
||||
const result = await tool.invoke(
|
||||
@@ -397,6 +499,7 @@ suite('FetchWebPageTool', () => {
|
||||
new ExtendedTestFileService(fileContentMap),
|
||||
new MockTrustedDomainService(),
|
||||
new MockChatService(),
|
||||
new TestContextService(),
|
||||
);
|
||||
|
||||
const result = await tool.invoke(
|
||||
@@ -463,6 +566,7 @@ suite('FetchWebPageTool', () => {
|
||||
new ExtendedTestFileService(fileContentMap),
|
||||
new MockTrustedDomainService(),
|
||||
new MockChatService(),
|
||||
new TestContextService(),
|
||||
);
|
||||
|
||||
const result = await tool.invoke(
|
||||
@@ -503,6 +607,7 @@ suite('FetchWebPageTool', () => {
|
||||
new ExtendedTestFileService(fileContentMap),
|
||||
new MockTrustedDomainService(),
|
||||
new MockChatService(),
|
||||
new TestContextService(),
|
||||
);
|
||||
|
||||
const result = await tool.invoke(
|
||||
@@ -547,6 +652,7 @@ suite('FetchWebPageTool', () => {
|
||||
new ExtendedTestFileService(fileContentMap),
|
||||
new MockTrustedDomainService(),
|
||||
new MockChatService(),
|
||||
new TestContextService(),
|
||||
);
|
||||
|
||||
const testUrls = [
|
||||
@@ -606,6 +712,7 @@ suite('FetchWebPageTool', () => {
|
||||
new ExtendedTestFileService(new ResourceMap<string | VSBuffer>()),
|
||||
new MockTrustedDomainService([]),
|
||||
new MockChatService(),
|
||||
new TestContextService(),
|
||||
);
|
||||
|
||||
const testUrls = [
|
||||
@@ -642,6 +749,7 @@ suite('FetchWebPageTool', () => {
|
||||
new ExtendedTestFileService(fileContentMap),
|
||||
new MockTrustedDomainService(),
|
||||
new MockChatService(),
|
||||
new TestContextService(),
|
||||
);
|
||||
|
||||
const testUrls = [
|
||||
@@ -684,6 +792,7 @@ suite('FetchWebPageTool', () => {
|
||||
new ExtendedTestFileService(fileContentMap),
|
||||
new MockTrustedDomainService(),
|
||||
new MockChatService(),
|
||||
new TestContextService(),
|
||||
);
|
||||
|
||||
const testUrls = [
|
||||
@@ -732,6 +841,7 @@ suite('FetchWebPageTool', () => {
|
||||
new ExtendedTestFileService(new ResourceMap<string | VSBuffer>()), // Empty - all file ,
|
||||
new MockTrustedDomainService([]),
|
||||
new MockChatService(),
|
||||
new TestContextService(),
|
||||
);
|
||||
|
||||
const testUrls = [
|
||||
@@ -766,6 +876,7 @@ suite('FetchWebPageTool', () => {
|
||||
new ExtendedTestFileService(new ResourceMap<string | VSBuffer>()),
|
||||
new MockTrustedDomainService([]),
|
||||
new MockChatService(),
|
||||
new TestContextService(),
|
||||
);
|
||||
|
||||
const result = await tool.invoke(
|
||||
@@ -792,6 +903,7 @@ suite('FetchWebPageTool', () => {
|
||||
new ExtendedTestFileService(fileContentMap),
|
||||
new MockTrustedDomainService(),
|
||||
new MockChatService(),
|
||||
new TestContextService(),
|
||||
);
|
||||
|
||||
const result = await tool.invoke(
|
||||
@@ -829,6 +941,7 @@ suite('FetchWebPageTool', () => {
|
||||
new ExtendedTestFileService(new ResourceMap<string | VSBuffer>()),
|
||||
new MockTrustedDomainService(),
|
||||
new MockChatService(),
|
||||
new TestContextService(),
|
||||
);
|
||||
|
||||
const result = await tool.invoke(
|
||||
@@ -856,6 +969,7 @@ suite('FetchWebPageTool', () => {
|
||||
new ExtendedTestFileService(new ResourceMap<string | VSBuffer>()),
|
||||
new MockTrustedDomainService(),
|
||||
new MockChatService(),
|
||||
new TestContextService(),
|
||||
);
|
||||
|
||||
const result = await tool.invoke(
|
||||
@@ -888,6 +1002,7 @@ suite('FetchWebPageTool', () => {
|
||||
new ExtendedTestFileService(new ResourceMap<string | VSBuffer>()),
|
||||
new MockTrustedDomainService(),
|
||||
new MockChatService(),
|
||||
new TestContextService(),
|
||||
);
|
||||
|
||||
const result = await tool.invoke(
|
||||
@@ -914,6 +1029,7 @@ suite('FetchWebPageTool', () => {
|
||||
new ExtendedTestFileService(new ResourceMap<string | VSBuffer>()),
|
||||
new MockTrustedDomainService(),
|
||||
new MockChatService(),
|
||||
new TestContextService(),
|
||||
);
|
||||
|
||||
const result = await tool.invoke(
|
||||
|
||||
Reference in New Issue
Block a user