From cdd636d146ea38d153cec4b35808baf86411096c Mon Sep 17 00:00:00 2001 From: "Dustin L. Howett" Date: Tue, 7 Apr 2026 11:23:15 -0500 Subject: [PATCH] Display a warning dialog for unsafe URLs (#20065) 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 81170aff78dc10c5673930b4cd107cf3d3705336) Service-Card-Id: PVTI_lADOAF3p4s4BBcTlzgpOYmk Service-Version: 1.24 --- .../Resources/en-US/Resources.resw | 8 ++- src/cascadia/TerminalApp/TerminalPage.cpp | 59 +++++++++++++++++-- src/cascadia/TerminalApp/TerminalPage.h | 5 +- src/cascadia/TerminalApp/TerminalPage.xaml | 8 +-- src/cascadia/TerminalControl/TermControl.cpp | 2 - src/inc/til/string.h | 4 +- 6 files changed, 70 insertions(+), 16 deletions(-) diff --git a/src/cascadia/TerminalApp/Resources/en-US/Resources.resw b/src/cascadia/TerminalApp/Resources/en-US/Resources.resw index 0dd84b19a4..a750631cfe 100644 --- a/src/cascadia/TerminalApp/Resources/en-US/Resources.resw +++ b/src/cascadia/TerminalApp/Resources/en-US/Resources.resw @@ -715,9 +715,15 @@ This link type is currently not supported: - + Cancel + + 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. + + + Open anyway + Settings diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 07c9797f90..d6c01539ec 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -3032,18 +3032,42 @@ namespace winrt::TerminalApp::implementation } CATCH_LOG(); - void TerminalPage::_OpenHyperlinkHandler(const IInspectable /*sender*/, const Microsoft::Terminal::Control::OpenHyperlinkEventArgs eventArgs) + safe_void_coroutine TerminalPage::_OpenHyperlinkHandler(const IInspectable /*sender*/, const Microsoft::Terminal::Control::OpenHyperlinkEventArgs eventArgs) { try { - auto parsed = winrt::Windows::Foundation::Uri(eventArgs.Uri()); + auto uriString{ eventArgs.Uri() }; + auto parsed = winrt::Windows::Foundation::Uri(uriString); if (_IsUriSupported(parsed)) { - ShellExecute(nullptr, L"open", eventArgs.Uri().c_str(), nullptr, nullptr, SW_SHOWNORMAL); + bool shouldLaunch{ _IsUriConsideredSomewhatSafe(parsed) }; + + if (!shouldLaunch) + { + if (auto presenter{ _dialogPresenter.get() }) + { + // FindName needs to be called first to actually load the xaml object + auto unopenedUriDialog = FindName(L"UriErrorDialog").try_as(); + + // Insert the reason and the URI + unopenedUriDialog.SecondaryButtonText(RS_(L"UnsafeUrlConfirmAllowAction")); + CouldNotOpenUriReason().Text(RS_(L"UnsafeUrlConfirmText")); + UnopenedUri().Text(uriString); + + // Show the dialog + auto result = co_await presenter.ShowDialog(unopenedUriDialog); + shouldLaunch = result == ContentDialogResult::Secondary; + } + } + + if (shouldLaunch) + { + ShellExecuteW(nullptr, L"open", uriString.c_str(), nullptr, nullptr, SW_SHOWNORMAL); + } } else { - _ShowCouldNotOpenDialog(RS_(L"UnsupportedSchemeText"), eventArgs.Uri()); + _ShowCouldNotOpenDialog(RS_(L"UnsupportedSchemeText"), uriString); } } catch (...) @@ -3063,9 +3087,10 @@ namespace winrt::TerminalApp::implementation if (auto presenter{ _dialogPresenter.get() }) { // FindName needs to be called first to actually load the xaml object - auto unopenedUriDialog = FindName(L"CouldNotOpenUriDialog").try_as(); + auto unopenedUriDialog = FindName(L"UriErrorDialog").try_as(); // Insert the reason and the URI + unopenedUriDialog.SecondaryButtonText({}); CouldNotOpenUriReason().Text(reason); UnopenedUri().Text(uri); @@ -3118,6 +3143,30 @@ namespace winrt::TerminalApp::implementation return true; } + bool TerminalPage::_IsUriConsideredSomewhatSafe(const winrt::Windows::Foundation::Uri& parsedUri) + { + if (parsedUri.SchemeName() == L"http" || parsedUri.SchemeName() == L"https") + { + return true; + } + if (parsedUri.SchemeName() == L"file") + { + static const auto pathext{ wil::TryGetEnvironmentVariableW(L"PATHEXT") }; + const auto filename = parsedUri.Path(); + for (const auto& e : til::split_iterator{ std::wstring_view{ pathext }, L';' }) + { + if (til::ends_with_insensitive_ascii(filename, e)) + { + return false; + } + } + + return true; + } + + return false; + } + // Important! Don't take this eventArgs by reference, we need to extend the // lifetime of it to the other side of the co_await! safe_void_coroutine TerminalPage::_ControlNoticeRaisedHandler(const IInspectable /*sender*/, diff --git a/src/cascadia/TerminalApp/TerminalPage.h b/src/cascadia/TerminalApp/TerminalPage.h index 22b73f89d8..3c19648975 100644 --- a/src/cascadia/TerminalApp/TerminalPage.h +++ b/src/cascadia/TerminalApp/TerminalPage.h @@ -416,8 +416,9 @@ namespace winrt::TerminalApp::implementation safe_void_coroutine _PasteFromClipboardHandler(const IInspectable sender, const Microsoft::Terminal::Control::PasteFromClipboardEventArgs eventArgs); - void _OpenHyperlinkHandler(const IInspectable sender, const Microsoft::Terminal::Control::OpenHyperlinkEventArgs eventArgs); - bool _IsUriSupported(const winrt::Windows::Foundation::Uri& parsedUri); + safe_void_coroutine _OpenHyperlinkHandler(const IInspectable sender, const Microsoft::Terminal::Control::OpenHyperlinkEventArgs eventArgs); + static bool _IsUriSupported(const winrt::Windows::Foundation::Uri& parsedUri); + static bool _IsUriConsideredSomewhatSafe(const winrt::Windows::Foundation::Uri& parsedUri); void _ShowCouldNotOpenDialog(winrt::hstring reason, winrt::hstring uri); bool _CopyText(bool dismissSelection, bool singleLine, bool withControlSequences, Microsoft::Terminal::Control::CopyFormat formats); diff --git a/src/cascadia/TerminalApp/TerminalPage.xaml b/src/cascadia/TerminalApp/TerminalPage.xaml index 40e3b838c3..68396d01d5 100644 --- a/src/cascadia/TerminalApp/TerminalPage.xaml +++ b/src/cascadia/TerminalApp/TerminalPage.xaml @@ -144,17 +144,17 @@ - + DefaultButton="Close"> - + diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index 61ef1eaadf..a0b5a1d280 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -1993,8 +1993,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation // - args: event data void TermControl::_TappedHandler(const IInspectable& /*sender*/, const TappedRoutedEventArgs& e) { - Focus(FocusState::Pointer); - if (e.PointerDeviceType() == Windows::Devices::Input::PointerDeviceType::Touch) { // Normally TSF would be responsible for showing the touch keyboard, but it's buggy for us: diff --git a/src/inc/til/string.h b/src/inc/til/string.h index 824df3212b..5f05820a49 100644 --- a/src/inc/til/string.h +++ b/src/inc/til/string.h @@ -228,7 +228,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" constexpr bool ends_with_insensitive_ascii(const std::basic_string_view& str, const std::basic_string_view& suffix) noexcept { #pragma warning(suppress : 26481) // Don't use pointer arithmetic. Use span instead (bounds.1). - return str.size() >= suffix.size() && equals_insensitive_ascii<>({ str.data() - suffix.size(), suffix.size() }, suffix); + return str.size() >= suffix.size() && equals_insensitive_ascii<>({ str.data() + str.size() - suffix.size(), suffix.size() }, suffix); } constexpr bool ends_with_insensitive_ascii(const std::string_view& str, const std::string_view& prefix) noexcept @@ -238,7 +238,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" constexpr bool ends_with_insensitive_ascii(const std::wstring_view& str, const std::wstring_view& prefix) noexcept { - return ends_with<>(str, prefix); + return ends_with_insensitive_ascii<>(str, prefix); } template