Warn before attaching browser content from possibly untrusted sources#308286
Warn before attaching browser content from possibly untrusted sources#308286
Conversation
Screenshot ChangesBase: Changed (4)
|
| Before | After |
|---|---|
 |
 |
chat/aiCustomizations/aiCustomizationManagementEditor/McpBrowseMode/Light
| Before | After |
|---|---|
 |
 |
chat/aiCustomizations/aiCustomizationManagementEditor/PromptsTabScrolled/Light
| Before | After |
|---|---|
 |
 |
chat/aiCustomizations/aiCustomizationManagementEditor/McpServersTabNarrow/Light
| Before | After |
|---|---|
 |
 |
There was a problem hiding this comment.
Pull request overview
This PR adds a user warning/confirmation step before attaching Integrated Browser content to chat when the current page may be untrusted, helping mitigate prompt-injection risks from arbitrary web pages.
Changes:
- Added an “untrusted content” confirmation dialog with a “Don’t show again” preference.
- Included the current browser URL in
IElementDatato enable trust checks per element. - Refactored element-selection cancellation cleanup and moved “element added” telemetry to the attachment path.
Show a summary per file
| File | Description |
|---|---|
| src/vs/workbench/contrib/browserView/electron-browser/features/browserEditorChatFeatures.ts | Adds warning dialog + persisted preference, hooks it into element/console-log attachments, and refactors selection lifecycle/telemetry. |
| src/vs/platform/browserView/electron-main/browserViewElementInspector.ts | Augments extracted element data with the current page URL. |
| src/vs/platform/browserElements/common/browserElements.ts | Extends IElementData to optionally carry the source URL. |
Copilot's findings
- Files reviewed: 3/3 changed files
- Comments generated: 2
| // Query the workspace trust service for file URLs | ||
| const trustInfo = await this.workspaceTrustManagementService.getUriTrustInfo(URI.file(parsedUrl.pathname)); |
There was a problem hiding this comment.
For file: URLs, using URI.file(parsedUrl.pathname) is likely to produce the wrong URI on Windows and/or for percent-encoded paths (e.g. spaces become %20), which can cause getUriTrustInfo to misclassify a trusted local file as untrusted. Consider using URI.parse(url) (or otherwise properly decoding the file URL) when passing it to getUriTrustInfo so the trust check matches the actual resource URI.
| // Query the workspace trust service for file URLs | |
| const trustInfo = await this.workspaceTrustManagementService.getUriTrustInfo(URI.file(parsedUrl.pathname)); | |
| // Query the workspace trust service for file URLs using the original file URL semantics | |
| const trustInfo = await this.workspaceTrustManagementService.getUriTrustInfo(URI.parse(url)); |
| @@ -322,10 +354,29 @@ export class BrowserEditorChatIntegration extends BrowserEditorContribution { | |||
| }); | |||
| } | |||
|
|
|||
| if (!await this._confirmContentAttachmentRisk(elementData.url ?? model.url)) { | |||
| return; | |||
| } | |||
There was a problem hiding this comment.
_attachElementDataToChat captures the screenshot (and builds the attachment payload) before showing the untrusted-content warning. If the user cancels, we still did the expensive captureScreenshot call and temporarily collected page content. Consider calling _confirmContentAttachmentRisk(...) earlier (before building toAttach and especially before captureScreenshot) to avoid unnecessary work and better align with “warn before attaching”.
📬 CODENOTIFYThe following users are being notified based on files changed in this PR: @jrualesMatched files:
|
No description provided.