Display a warning dialog for unsafe URLs by DHowett · Pull Request #20065 · microsoft/terminal

DHowett · 2026-04-03T23:12:07Z

We are getting a sufficient number of LLM-generated security reports telling us that Ctrl+click and a tooltip are insufficient protection from users clicking on links to dangerous things.

This commit displays a warning that prevents users from blindly clicking on dangerous things.

Dangerous things include:

any non-http and non-https and non-file URLs
any file URLs that point to something understandable as a "program" (so, something which resides in PATHEXT.)

In doing this, I learned that til::ends_with_insensitive_ascii was broken.

I also learned that ContentDialogs summoned by any event handler out of TermControl::Pointer* would lose focus immediately. It turns out that in the absolute earliest days of Terminal, when we first created the UserControl that became TermControl, we added our Tapped event handler.

It unconditionally focused the control.

Since Tapped is a higher-level event handler than PointerPressed, it was firing after the gesture that opened the content dialog and stealing focus back.

I'm fairly certain we don't need it.

Refs #7562

We are getting a sufficient number of LLM-generated security reports telling us that Ctrl+click and a tooltip are insufficient protection from users clicking on links to dangerous things. This commit displays a warning that prevents users from blindly clicking on dangerous things. Dangerous things include: - any non-http and non-https and non-file URLs - any file URLs that point to something understandable as a "program" (so, something which resides in `PATHEXT`.) In doing this, I learned that `til::ends_with_insensitive_ascii` was broken. I also learned that ContentDialogs summoned by any event handler out of TermControl::Pointer* would lose focus immediately. It turns out that in the absolute earliest days of Terminal, when we first created the UserControl that became TermControl, we added our Tapped event handler. It unconditionally focused the control. Since `Tapped` is a higher-level event handler than `PointerPressed`, it was firing after the gesture that opened the content dialog and stealing focus back. I'm fairly certain we don't need it. Refs #7562

DHowett · 2026-04-03T23:13:04Z

    <value>This link type is currently not supported:</value>
  </data>
-  <data name="CouldNotOpenUriDialog.PrimaryButtonText" xml:space="preserve">
+  <data name="UriErrorDialog.CloseButtonText" xml:space="preserve">


I had to rename the dialog because of some business about xaml UIDs causing the wrong resources to be loaded when I changed the Cancel button from being the "Primary" button to correctly being the "Close" button.

DHowett · 2026-04-03T23:15:40Z

    <value>Cancel</value>
  </data>
+  <data name="UnsafeUrlConfirmText" xml:space="preserve">
+    <value>This link may lead to an unsafe location. Hyperlinks can be harmful to your computer and data. To protect your computer, only click links from trusted sources.</value>


This messaging was used in Notepad. I do not recommend fighting it.

Don't worry, I got you: I'm fighting it.

sagarbhure-msft · 2026-04-06T17:50:44Z

This looks like a really nice and much-needed improvement.
Not sure if this is important or if I might be missing something here, but I had a small question.

Are we normalizing / percent-decoding the URL before checking the extension? In cases like:

file:///C:/temp/malware%2Eexe
it seems possible that an ends_with-style check could miss it if evaluated before decoding.

Also, I was wondering if UNC paths / remote shares should be treated as higher risk? For example:

file://server/share/payload.exe
\\server\share\payload.bat

Relying on PATHEXT might miss some executable vectors handled via file associations. .lnk (shortcuts) ?

These might be riskier than local paths (e.g., remote execution or credential leaks), so I was curious if they should be handled differently.

DHowett · 2026-04-06T17:54:48Z

it seems possible that an ends_with-style check could miss it if evaluated before decoding.

Evaluation happens after decoding.

Also, I was wondering if UNC paths / remote shares should be treated as higher risk?

We reject file URIs that point to different hosts in an earlier step.

Relying on PATHEXT might miss some executable vectors

Fair, but admittedly if the attacker can place a lnk on the local machine (see second point) they can likely do much worse than "trick the user into ctrl+clicking on it."

carlos-zamora

LGTM! Thanks for doing this!

lhecker · 2026-04-07T16:18:03Z

    <value>Cancel</value>
  </data>
+  <data name="UnsafeUrlConfirmText" xml:space="preserve">
+    <value>This link may lead to an unsafe location. Hyperlinks can be harmful to your computer and data. To protect your computer, only click links from trusted sources.</value>


Don't worry, I got you: I'm fighting it.

We are getting a sufficient number of LLM-generated security reports telling us that Ctrl+click and a tooltip are insufficient protection from users clicking on links to dangerous things. This commit displays a warning that prevents users from blindly clicking on dangerous things. Dangerous things include: - any non-http and non-https and non-file URLs - any file URLs that point to something understandable as a "program" (so, something which resides in `PATHEXT`.) In doing this, I learned that `til::ends_with_insensitive_ascii` was broken. I also learned that ContentDialogs summoned by any event handler out of TermControl::Pointer* would lose focus immediately. It turns out that in the absolute earliest days of Terminal, when we first created the UserControl that became TermControl, we added our Tapped event handler. It unconditionally focused the control. Since `Tapped` is a higher-level event handler than `PointerPressed`, it was firing after the gesture that opened the content dialog and stealing focus back. I'm fairly certain we don't need it. Refs #7562 (cherry picked from commit 81170af) Service-Card-Id: PVTI_lADOAF3p4s4BQX0-zgpOYmc Service-Version: 1.25

We are getting a sufficient number of LLM-generated security reports telling us that Ctrl+click and a tooltip are insufficient protection from users clicking on links to dangerous things. This commit displays a warning that prevents users from blindly clicking on dangerous things. Dangerous things include: - any non-http and non-https and non-file URLs - any file URLs that point to something understandable as a "program" (so, something which resides in `PATHEXT`.) In doing this, I learned that `til::ends_with_insensitive_ascii` was broken. I also learned that ContentDialogs summoned by any event handler out of TermControl::Pointer* would lose focus immediately. It turns out that in the absolute earliest days of Terminal, when we first created the UserControl that became TermControl, we added our Tapped event handler. It unconditionally focused the control. Since `Tapped` is a higher-level event handler than `PointerPressed`, it was firing after the gesture that opened the content dialog and stealing focus back. I'm fairly certain we don't need it. Refs #7562 (cherry picked from commit 81170af) Service-Card-Id: PVTI_lADOAF3p4s4BBcTlzgpOYmk Service-Version: 1.24

We are getting a sufficient number of LLM-generated security reports telling us that Ctrl+click and a tooltip are insufficient protection from users clicking on links to dangerous things. This commit displays a warning that prevents users from blindly clicking on dangerous things. Dangerous things include: - any non-http and non-https and non-file URLs - any file URLs that point to something understandable as a "program" (so, something which resides in `PATHEXT`.) In doing this, I learned that `til::ends_with_insensitive_ascii` was broken. I also learned that ContentDialogs summoned by any event handler out of TermControl::Pointer* would lose focus immediately. It turns out that in the absolute earliest days of Terminal, when we first created the UserControl that became TermControl, we added our Tapped event handler. It unconditionally focused the control. Since `Tapped` is a higher-level event handler than `PointerPressed`, it was firing after the gesture that opened the content dialog and stealing focus back. I'm fairly certain we don't need it. Refs microsoft#7562

This adds a `safeUriSchemes` global setting which lets you define hyperlink URI schemes which the user considers safe. No confirmation dialog will be shown when trying to open hyperlinks which use these schemes. - This solves the root issue, but doesn't introduce any UI or documentation changes. I wanted to validate the approach and implementation with you first. - I closely followed the code handling the `disabledProfileSources` setting, which is of the same type. - This feature does not change the behavior of `http`, `https` and `file` schemes. Validation I ran the dev terminal, and tested the behavior by clicking on `vscode` hyperlinks generated by ripgrep with various `safeUriSchemes` settings: - Setting not defined - asks for confirmation - `["vscode"]` - does not ask for confirmation - `["foo", "vscode"]` - does not ask for confirmation - `["foo"]` - asks for confirmation - `null` - asks for confirmation - `[]` - asks for confirmation - `[""]` - asks for confirmation - `[{"foo": "bar"}]` - fails to deserialize (as expected) A few uinit tests failed, but they seem unrelated to these changes: - `KeyBindingTests` in `UnitTests_SettingsModel`, probably because I use an AZERTY keyboard. - A few `Conhost` tests, but I didn't touch this part Refs #20065 Closes #20191

This adds a `safeUriSchemes` global setting which lets you define hyperlink URI schemes which the user considers safe. No confirmation dialog will be shown when trying to open hyperlinks which use these schemes. - This solves the root issue, but doesn't introduce any UI or documentation changes. I wanted to validate the approach and implementation with you first. - I closely followed the code handling the `disabledProfileSources` setting, which is of the same type. - This feature does not change the behavior of `http`, `https` and `file` schemes. Validation I ran the dev terminal, and tested the behavior by clicking on `vscode` hyperlinks generated by ripgrep with various `safeUriSchemes` settings: - Setting not defined - asks for confirmation - `["vscode"]` - does not ask for confirmation - `["foo", "vscode"]` - does not ask for confirmation - `["foo"]` - asks for confirmation - `null` - asks for confirmation - `[]` - asks for confirmation - `[""]` - asks for confirmation - `[{"foo": "bar"}]` - fails to deserialize (as expected) A few uinit tests failed, but they seem unrelated to these changes: - `KeyBindingTests` in `UnitTests_SettingsModel`, probably because I use an AZERTY keyboard. - A few `Conhost` tests, but I didn't touch this part Refs #20065 Closes #20191 (cherry picked from commit fb71a04) Service-Card-Id: PVTI_lADOAF3p4s4BBcTlzgshlaM Service-Version: 1.24

This adds a `safeUriSchemes` global setting which lets you define hyperlink URI schemes which the user considers safe. No confirmation dialog will be shown when trying to open hyperlinks which use these schemes. - This solves the root issue, but doesn't introduce any UI or documentation changes. I wanted to validate the approach and implementation with you first. - I closely followed the code handling the `disabledProfileSources` setting, which is of the same type. - This feature does not change the behavior of `http`, `https` and `file` schemes. Validation I ran the dev terminal, and tested the behavior by clicking on `vscode` hyperlinks generated by ripgrep with various `safeUriSchemes` settings: - Setting not defined - asks for confirmation - `["vscode"]` - does not ask for confirmation - `["foo", "vscode"]` - does not ask for confirmation - `["foo"]` - asks for confirmation - `null` - asks for confirmation - `[]` - asks for confirmation - `[""]` - asks for confirmation - `[{"foo": "bar"}]` - fails to deserialize (as expected) A few uinit tests failed, but they seem unrelated to these changes: - `KeyBindingTests` in `UnitTests_SettingsModel`, probably because I use an AZERTY keyboard. - A few `Conhost` tests, but I didn't touch this part Refs #20065 Closes #20191 (cherry picked from commit fb71a04) Service-Card-Id: PVTI_lADOAF3p4s4BQX0-zgshlaI Service-Version: 1.25

DHowett commented Apr 3, 2026

View reviewed changes

Comment thread src/cascadia/TerminalApp/Resources/en-US/Resources.resw Outdated

Apply suggestion from @DHowett

37686d0

DHowett commented Apr 3, 2026

View reviewed changes

carlos-zamora approved these changes Apr 6, 2026

View reviewed changes

carlos-zamora added this to 1.25 Servicing Pipeline and 1.24 Servicing Pipeline Apr 6, 2026

github-project-automation Bot moved this to To Cherry Pick in 1.24 Servicing Pipeline Apr 6, 2026

github-project-automation Bot moved this to To Cherry Pick in 1.25 Servicing Pipeline Apr 6, 2026

carlos-zamora moved this from To Cherry Pick to To Consider in 1.24 Servicing Pipeline Apr 6, 2026

carlos-zamora moved this from To Cherry Pick to To Consider in 1.25 Servicing Pipeline Apr 6, 2026

DHowett moved this from To Consider to To Cherry Pick in 1.25 Servicing Pipeline Apr 6, 2026

lhecker approved these changes Apr 7, 2026

View reviewed changes

DHowett merged commit 81170af into main Apr 7, 2026
20 checks passed

DHowett deleted the dev/duhowett/they-just-keep-filing-security-reports branch April 7, 2026 16:23

DHowett moved this from To Cherry Pick to Cherry Picked in 1.25 Servicing Pipeline Apr 16, 2026

DHowett moved this from To Consider to To Cherry Pick in 1.24 Servicing Pipeline Apr 30, 2026

DHowett moved this from To Cherry Pick to Cherry Picked in 1.24 Servicing Pipeline Apr 30, 2026

This was referenced May 6, 2026

Make URL safety configurable #20191

Closed

Add safeUriSchemes setting #20207

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Display a warning dialog for unsafe URLs#20065

Display a warning dialog for unsafe URLs#20065
DHowett merged 2 commits into
mainfrom
dev/duhowett/they-just-keep-filing-security-reports

DHowett commented Apr 3, 2026

Uh oh!

DHowett Apr 3, 2026

Uh oh!

Uh oh!

DHowett Apr 3, 2026

Uh oh!

lhecker Apr 7, 2026

Uh oh!

sagarbhure-msft commented Apr 6, 2026 •

edited

Loading

Uh oh!

DHowett commented Apr 6, 2026

Uh oh!

carlos-zamora left a comment

Uh oh!

lhecker Apr 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

DHowett commented Apr 3, 2026

Uh oh!

DHowett Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

DHowett Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

lhecker Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

sagarbhure-msft commented Apr 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DHowett commented Apr 6, 2026

Uh oh!

carlos-zamora left a comment

Choose a reason for hiding this comment

Uh oh!

lhecker Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sagarbhure-msft commented Apr 6, 2026 •

edited

Loading