Skip to content

Display a warning dialog for unsafe URLs#20065

Open
DHowett wants to merge 2 commits intomainfrom
dev/duhowett/they-just-keep-filing-security-reports
Open

Display a warning dialog for unsafe URLs#20065
DHowett wants to merge 2 commits intomainfrom
dev/duhowett/they-just-keep-filing-security-reports

Conversation

@DHowett
Copy link
Copy Markdown
Member

@DHowett DHowett commented Apr 3, 2026

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
<value>This link type is currently not supported:</value>
</data>
<data name="CouldNotOpenUriDialog.PrimaryButtonText" xml:space="preserve">
<data name="UriErrorDialog.CloseButtonText" xml:space="preserve">
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

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.

<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>
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

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

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.

1 participant