[Search Subagent] Don't disclose paths outside of current workspace (#317213)

* wip

* filter out files outside of workspace from subagent response

* revert settings.json

* address code review comments

* retrigger gh check

Author: guomaggie

extensions/copilot/src/extension/tools/node/searchSubagentTool.ts

@@ -25,6 +25,7 @@ import { IBuildPromptContext } from '../../prompt/common/intents';
 import { SearchSubagentToolCallingLoop } from '../../prompt/node/searchSubagentToolCallingLoop';
 import { ToolName } from '../common/toolNames';
 import { CopilotToolMode, ICopilotTool, ICopilotToolCtor, ToolRegistry } from '../common/toolsRegistry';
+import { assertFileOkForTool, isFileExternalAndNeedsConfirmation } from './toolUtils';
 
 export interface ISearchSubagentParams {
 
@@ -174,7 +175,7 @@ class SearchSubagentTool implements ICopilotTool<ISearchSubagentParams> {
 			subagentResponse = `The search subagent request failed with this message:\n${loopResult.response.type}: ${loopResult.response.reason}`;
 		}
 		// Parse and hydrate code snippets from <final_answer> tags
-		const hydratedResponse = await this.parseFinalAnswerAndHydrate(subagentResponse, cwd, token);
+		const hydratedResponse = await this.parseFinalAnswerAndHydrate(subagentResponse, cwd, options.workingDirectory, token);
 
 		// toolMetadata will be automatically included in exportAllPromptLogsAsJsonCommand
 		const result = new ExtendedLanguageModelToolResult([new LanguageModelTextPart(hydratedResponse)]);
@@ -187,10 +188,11 @@ class SearchSubagentTool implements ICopilotTool<ISearchSubagentParams> {
 	 * Parse the path and line range subagent response and hydrate code snippets
 	 * @param response The subagent response containing paths and line ranges
 	 * @param cwd The current working directory to prepend to relative paths
+	 * @param workingDirectory The working directory URI from tool invocation context
 	 * @param token Cancellation token
 	 * @returns The response with actual code snippets appended to file paths
 	 */
-	private async parseFinalAnswerAndHydrate(response: string, cwd: string | undefined, token: vscode.CancellationToken): Promise<string> {
+	private async parseFinalAnswerAndHydrate(response: string, cwd: string | undefined, workingDirectory: URI | undefined, token: vscode.CancellationToken): Promise<string> {
 		const lines = response.split('\n');
 
 		// Parse file:line-line format
@@ -211,12 +213,17 @@ class SearchSubagentTool implements ICopilotTool<ISearchSubagentParams> {
 			const startLine = parseInt(startLineStr, 10);
 			const endLine = parseInt(endLineStr, 10);
 
+			// Resolve the candidate URI up front so we can reference it from both the
+			// try and the catch block (for the external-file check below).
+			const uri = (!path.isAbsolute(filePath) && cwd)
+				? URI.joinPath(URI.file(cwd), filePath)
+				: URI.file(filePath);
+
 			try {
-				// For relative paths, immediately resolve against cwd.
-				// For absolute paths, use as-is and let openTextDocument throw if not found.
-				const uri = (!path.isAbsolute(filePath) && cwd)
-					? URI.joinPath(URI.file(cwd), filePath)
-					: URI.file(filePath);
+				// Enforce read-only file access via shared toolUtils guards before hydrating.
+				await this.instantiationService.invokeFunction(accessor =>
+					assertFileOkForTool(accessor, uri, this._inputContext, { readOnly: true, workingDirectory })
+				);
 				const document = await this.workspaceService.openTextDocument(uri);
 
 				const snapshot = TextDocumentSnapshot.create(document);
@@ -232,9 +239,24 @@ class SearchSubagentTool implements ICopilotTool<ISearchSubagentParams> {
 				const code = snapshot.getText(range);
 				processedLines.push(`File: \`${uri.fsPath}\`, lines ${clampedStartLine}-${clampedEndLine}:\n\`\`\`\n${code}\n\`\`\``);
 			} catch {
-				// If hydration fails (e.g. the captured path didn't resolve because the model's formatting drifted),
-				// keep the original line so the main agent still gets the model's answer instead of a noisy error suffix.
-				processedLines.push(line);
+				// Drop the line entirely for files outside the workspace so we don't
+				// disclose the path back to the model. For inside-workspace failures
+				// (e.g. file missing), keep the original line with the error.
+				let isExternal = false;
+				try {
+					isExternal = await this.instantiationService.invokeFunction(accessor =>
+						isFileExternalAndNeedsConfirmation(accessor, uri, this._inputContext, { readOnly: true, workingDirectory })
+					);
+				} catch {
+					// isFileExternalAndNeedsConfirmation throws for nonexistent files;
+					// treat that as "not external" so the original line is preserved.
+				}
+
+				if (!isExternal) {
+					// If hydration fails (e.g. the captured path didn't resolve because the model's formatting drifted),
+					// keep the original line so the main agent still gets the model's answer instead of a noisy error suffix.
+					processedLines.push(line);
+				}
 			}
 
 			if (token.isCancellationRequested) {

extensions/copilot/src/extension/tools/node/test/searchSubagentTool.spec.ts

@@ -6,12 +6,39 @@
 import type * as vscode from 'vscode';
 import { expect, suite, test } from 'vitest';
 import { ConfigKey } from '../../../../platform/configuration/common/configurationService';
+import { URI } from '../../../../util/vs/base/common/uri';
 import { toolCategories, ToolCategory, ToolName } from '../../common/toolNames';
 import { ToolRegistry } from '../../common/toolsRegistry';
 
 // Ensure side-effect registration
 import '../searchSubagentTool';
 
+/**
+ * Returns an invokeFunction stub that dequeues outcomes in call order.
+ * Each outcome is either a value to resolve with, or a thunk that throws.
+ */
+function sequencedInvokeFunction(...outcomes: Array<unknown | (() => never)>) {
+	let i = 0;
+	return async (_fn: unknown) => {
+		const outcome = outcomes[i++];
+		if (typeof outcome === 'function') {
+			return (outcome as () => unknown)();
+		}
+		return outcome;
+	};
+}
+
+/** Minimal vscode.TextDocument-shaped object that satisfies TextDocumentSnapshot.create. */
+function makeFakeDocument(uri: URI, text: string) {
+	return {
+		uri,
+		getText: () => text,
+		languageId: 'typescript',
+		eol: 1,
+		version: 0,
+	} as unknown as vscode.TextDocument;
+}
+
 /** Minimal stub for LanguageModelToolInformation */
 function makeToolInfo(overrides: Partial<vscode.LanguageModelToolInformation> = {}): vscode.LanguageModelToolInformation {
 	return {
@@ -25,7 +52,14 @@ function makeToolInfo(overrides: Partial<vscode.LanguageModelToolInformation> =
 }
 
 /** Returns an instance of the (private) SearchSubagentTool via the registry */
-function makeToolInstance(thoroughnessEnabled: boolean, toolCallLimit: number = 4) {
+function makeToolInstance(
+	thoroughnessEnabled: boolean,
+	toolCallLimit: number = 4,
+	overrides: {
+		invokeFunction?: (fn: unknown) => Promise<unknown>;
+		openTextDocument?: (uri: URI) => Promise<vscode.TextDocument>;
+	} = {},
+) {
 	const toolCtor = ToolRegistry.getTools().find(t => t.toolName === ToolName.SearchSubagent)!;
 
 	const configService = {
@@ -49,12 +83,18 @@ function makeToolInstance(thoroughnessEnabled: boolean, toolCallLimit: number =
 			// Return a minimal stub that exposes run()
 			return { run: async () => ({ response: { type: 'error', reason: 'stub' }, toolCallRounds: [], round: { response: '' } }) };
 		},
+		invokeFunction: overrides.invokeFunction ?? (async () => { throw new Error('invokeFunction not stubbed'); }),
+	};
+
+	const workspaceService = {
+		getWorkspaceFolders: () => [],
+		openTextDocument: overrides.openTextDocument ?? (async () => { throw new Error('openTextDocument not stubbed'); }),
 	};
 
 	const tool = new (toolCtor as any)(
 		instantiationService,
 		{ captureInvocation: async (_token: unknown, fn: () => unknown) => fn() }, // requestLogger
-		{ getWorkspaceFolders: () => [], openTextDocument: async () => { throw new Error('stub'); } }, // workspaceService
+		workspaceService,
 		configService,
 		experimentationService,
 	);
@@ -182,21 +222,54 @@ suite('SearchSubagentTool', () => {
 				'- /workspace/other.ts (lines 30-40): test2',
 			].join('\n');
 
-			const result = await tool['parseFinalAnswerAndHydrate'](response, '/workspace', notCancelled);
+			const result = await tool['parseFinalAnswerAndHydrate'](response, '/workspace', undefined, notCancelled);
 
 			expect(result).toBe(response);
 		});
 
-		test('keeps a matching line verbatim when the captured path fails to open (no error suffix)', async () => {
-			const { tool } = makeToolInstance(false);
-			const response = [
-				'- /workspace/file.ts:10-20'
-			].join('\n');
+		test('drops the line when the path is outside the workspace', async () => {
+			const { tool } = makeToolInstance(false, 4, {
+				invokeFunction: sequencedInvokeFunction(
+					() => { throw new Error('outside workspace'); },
+					true,
+				),
+			});
+
+			const response = '/external/secret.ts:5-10';
+			const result = await tool['parseFinalAnswerAndHydrate'](response, '/workspace', undefined, notCancelled);
+
+			expect(result).toBe('');
+		});
+
+		test('keeps the original line when an inside-workspace path fails to open', async () => {
+			const { tool } = makeToolInstance(false, 4, {
+				invokeFunction: sequencedInvokeFunction(
+					undefined,
+					false,
+				),
+				openTextDocument: async () => { throw new Error('file not found'); },
+			});
 
-			const result = await tool['parseFinalAnswerAndHydrate'](response, '/workspace', notCancelled);
+			const response = 'inside/file.ts:5-10';
+			const result = await tool['parseFinalAnswerAndHydrate'](response, '/workspace', undefined, notCancelled);
 
 			expect(result).toBe(response);
-			expect(result).not.toContain('unable to read file');
+		});
+
+		test('hydrates the line with code when an inside-workspace path opens', async () => {
+			const cwd = '/workspace';
+			const filePath = 'inside/file.ts';
+			const fileText = 'line1\nline2';
+			const uri = URI.joinPath(URI.file(cwd), filePath);
+
+			const { tool } = makeToolInstance(false, 4, {
+				invokeFunction: sequencedInvokeFunction(undefined),
+				openTextDocument: async () => makeFakeDocument(uri, fileText),
+			});
+
+			const result = await tool['parseFinalAnswerAndHydrate'](`${filePath}:1-2`, cwd, undefined, notCancelled);
+
+			expect(result).toBe(`File: \`${uri.fsPath}\`, lines 1-2:\n\`\`\`\n${fileText}\n\`\`\``);
 		});
 	});
 });