Skip to content

fix: fetch tool should not warn when reading files inside the workspace#303789

Merged
TylerLeonhardt merged 3 commits intomainfrom
copilot/fix-fetch-tool-permission-warnings
Mar 22, 2026
Merged

fix: fetch tool should not warn when reading files inside the workspace#303789
TylerLeonhardt merged 3 commits intomainfrom
copilot/fix-fetch-tool-permission-warnings

Conversation

Copy link
Contributor

Copilot AI commented Mar 21, 2026

The fetch tool was showing a "Fetch web page?" confirmation dialog for file:// URIs pointing to files inside the currently open workspace. The prompt injection risk warning makes no sense for workspace files that are already accessible.

Root cause

prepareToolInvocation added all URIs — web and file — to urlsNeedingConfirmation. Since trustedDomainService.isValid() only handles HTTP(S) domains, file URIs always failed the trust check and triggered the dialog.

Changes

  • fetchPageTool.ts: Inject IWorkspaceContextService and use getWorkspaceFolder() to classify file URIs:

    • allFetchedUris (web + all file URIs) — message display only
    • fileUrisOutsideWorkspace — file URIs not inside any workspace folder
    • urlsNeedingConfirmation (web URIs + files outside workspace) — trust checks and confirmation dialog

    Workspace file fetches now produce confirmationTitle = undefined and confirmResults = false — no dialog, no post-confirmation. Files outside the workspace still trigger confirmation, as do all web (http/https) URIs.

  • fetchPageTool.test.ts: Added TestContextService to all constructor calls; added tests covering: workspace file (no dialog), file outside workspace (still shows dialog), mixed workspace file + untrusted web URI (only web URI appears in confirmation).

Notes

Symlink resolution was considered but intentionally omitted — no other workspace membership checks in the codebase (e.g. read_file, ChatExternalPathConfirmation) resolve symlinks, and the confirmation dialog here is primarily guarding against web prompt injection rather than local filesystem traversal.


📱 Kick off Copilot coding agent tasks wherever you are with GitHub Mobile, available on iOS and Android.

Co-authored-by: TylerLeonhardt <2644648+TylerLeonhardt@users.noreply.github.com>
Agent-Logs-Url: https://github.com/microsoft/vscode/sessions/d33f23a2-816c-4dec-acca-ac0c989f6467
Copilot AI changed the title [WIP] Fix fetch tool to not warn when reading out of workspace fix: fetch tool should not warn when reading local/non-web files Mar 21, 2026
Copilot AI requested a review from TylerLeonhardt March 21, 2026 22:22
@TylerLeonhardt
Copy link
Member

@copilot I was expecting you to just check if the files are in the currently open workspace

Copy link
Contributor Author

Copilot AI commented Mar 21, 2026

@copilot I was expecting you to just check if the files are in the currently open workspace

Reworked in 9b462d8. Now FetchWebPageTool injects IWorkspaceContextService and checks getWorkspaceFolder() for each file URI — files inside the workspace are excluded from urlsNeedingConfirmation, so no dialog is shown. Files outside the workspace still trigger confirmation, as do all web (http/https) URIs.

Copilot AI changed the title fix: fetch tool should not warn when reading local/non-web files fix: fetch tool should not warn when reading files inside the workspace Mar 21, 2026
@TylerLeonhardt TylerLeonhardt marked this pull request as ready for review March 21, 2026 23:01
Copilot AI review requested due to automatic review settings March 21, 2026 23:02
@TylerLeonhardt TylerLeonhardt enabled auto-merge (squash) March 21, 2026 23:02
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adjusts the internal Chat “fetch” tool’s confirmation behavior so that reading file:// URIs inside the currently open workspace no longer triggers the “Fetch web page?” confirmation prompt, while still requiring confirmation for web URLs and file URIs outside the workspace.

Changes:

  • Update FetchWebPageTool.prepareToolInvocation to classify URIs using IWorkspaceContextService.getWorkspaceFolder() and exclude workspace-contained file URIs from confirmation checks.
  • Update message counts to use “all fetched URIs” (web + all valid file URIs) while keeping confirmations limited to web + outside-workspace file URIs.
  • Extend tests to cover workspace file URIs (no dialog), outside-workspace file URIs (dialog), and mixed workspace file + untrusted web URI (only web prompts).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/vs/workbench/contrib/chat/electron-browser/builtInTools/fetchPageTool.ts Injects workspace context to skip confirmations for workspace-contained file URIs; keeps confirmation for web/outside-workspace URIs.
src/vs/workbench/contrib/chat/test/electron-browser/tools/builtinTools/fetchPageTool.test.ts Updates constructor calls and adds coverage for new workspace vs outside-workspace confirmation behavior.

@TylerLeonhardt
Copy link
Member

@copilot what do you think of this comment ... does it feel contrived?

Copy link
Contributor Author

Copilot AI commented Mar 21, 2026

@copilot what do you think of this comment ... does it feel contrived?

Yes, I think it's contrived for this context. A few reasons:

  1. Inconsistency with existing patterns: The read_file tool and ChatExternalPathConfirmation also use getWorkspaceFolder() for workspace membership checks and don't resolve symlinks either. Adding symlink resolution only here would be inconsistent without fixing the same gap everywhere.

  2. Same threat model: If an agent can create a symlink inside the workspace pointing to a sensitive file, it already has sufficient local access to do much worse. The fetch tool's confirmation dialog is primarily guarding against web prompt injection — that threat doesn't apply to workspace files regardless of symlinks.

  3. fileService.realpath() returns Promise<URI | undefined>, so every file URI fetch would require an additional async stat + realpath call, adding latency for the common (non-symlink) case.

Happy to apply it if you feel strongly, but I'd lean toward leaving it as-is and addressing symlinks globally across all workspace membership checks if/when it's deemed necessary.

@TylerLeonhardt TylerLeonhardt merged commit e428f03 into main Mar 22, 2026
24 checks passed
@TylerLeonhardt TylerLeonhardt deleted the copilot/fix-fetch-tool-permission-warnings branch March 22, 2026 00:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fetch tool should not warn me when reading out of my workspace

4 participants