Repository: microsoft/terminal (language: cpp)

Issue:
[DISCUSSION] Allow more URI schemes
<!-- 
🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨

I ACKNOWLEDGE THE FOLLOWING BEFORE PROCEEDING:
1. If I delete this entire template and go my own path, the core team may close my issue without further explanation or engagement.
2. If I list multiple bugs/concerns in this one issue, the core team may close my issue without further explanation or engagement.
3. If I write an issue that has many duplicates, the core team may close my issue without further explanation or engagement (and without necessarily spending time to find the exact duplicate ID number).
4. If I leave the title incomplete when filing the issue, the core team may close my issue without further explanation or engagement.
5. If I file something completely blank in the body, the core team may close my issue without further explanation or engagement.

All good? Then proceed!
-->

# Description of the new feature/enhancement
We recently added support for hyperlinks (but only certain schemes so far). We should add support for more URI schemes. 

Discussion points:
- Which schemes should we support?
- Which schemes can be easily allowed without much security risks and which would require more thought?
- Is it on us to deal with the security concerns with regards to clicking links or is that on the user?

<!-- 
A clear and concise description of what the problem is that the new feature would solve.
Describe why and how a user would use this new functionality (if applicable).
-->




Discussion / hints:
What about a more generic implementation of URI parsing to match anything that fits the RFC, and leave handling actually opening it to Windows? Artificially implementing schemes, explicitly, on a one-off basis, seems limiting and more complex.
You are completely correct that this way is more complex/limiting. However there are security concerns with regards to allowing any scheme to be opened (see the discussion in #7526 for example). 

That being said, I have marked this issue as a generic place to discuss adding support for more URI schemes, and further concerns regarding which schemes we can easily support and which schemes would require more thought with regards to security/viability/etc can be talked about here. 

@j4james, would love to hear your input on this!
Well taken.

I tend to be of a mind aligning with the third bullet point above, for a few reasons:

- This particular feature *is* just a convenience to the user, and isn’t likely to be a primary means of interacting with it.
- A user who has downloaded, installed, and is using Terminal is significantly less likely to be a novice user who doesn’t understand the repercussions of clicking random links.
- The text, as displayed, *is* the link, is it not? In other words, it’s not like an HTML page, where the displayed text may not match the actual target of the hyperlink.
- I *highly* doubt a malicious actor would go through the effort of having someone open up Terminal in order to carry out their dirty deeds, when there are *far* simpler ways of getting someone to click a nasty link.
- In my opinion, it isn’t Terminal’s responsibility or place to protect a user from themselves any more than any other part of the shell, so, if an action is possible via a click elsewhere, it feels like it should be possible, here, too, to me.

A question/thought, though: If clicked hyperlinks are launched in such a way that UAC will be invoked, regardless of the permissions Terminal was launched with, would that not also be a fairly decent safeguard, leaving all responsibility then up to the user to confirm they want to do something potentially dangerous? Is that behavior possible?
I think by default most protocols should warn the user before opening, similar to the way browsers do, with a dialog telling them exactly which application is going to be opened, and the url that triggered the event. We can whitelist a few protocols by default (http being an obvious candidate), but I think that list should be really short. Users can always choose to whitelist individual protocols, but I wouldn't make that too easy for them, otherwise they'll just blindly whitelist everything.

When it comes to the file protocol in particular, we need to be even more careful. I don't think it's good enough to have a blanket whitelist for all file urls. One possibility would be to whitelist particular file extensions in the same way we would whitelist the protocols. So we default to popping up a warning - again letting the user know exactly which application is going to be opened - and then they can always have an option to whitelist trusted file extensions if they really want to.

Another thought I had was to `ShellExecute` the file urls using an `edit` verb. So for example if you clicked on a file url for a Python script, ideally we want to allow that to open in an editor, but never allow it to execute. I don't know how feasible that is in practice, though. Does `edit` guarantee that nothing will every be executed? And are there any file extensions that we would want to support that couldn't be opened that way? That needs to be investigated.

Finally, I should point out that I'm not a security expert, but in my experience security people tend to be way more paranoid than me about stuff like this, and I'm terrified of this functionality. If I were you, I wouldn't want to implement anything that hasn't been signed off by someone senior in Microsoft's security team, especially if we're planning to add hyperlink support to conhost as well, or if Windows Terminals is going to be become the default Windows console one day.
I looked for ways to avoid having a list of safe URI schemes in Windows Terminal, but didn't find any.

* [IAttachmentExecute](https://docs.microsoft.com/windows/desktop/api/shobjidl_core/nn-shobjidl_core-iattachmentexecute): Requires a file name.
* [ASSOCDATA_EDITFLAGS](https://docs.microsoft.com/windows/win32/api/shlwapi/ne-shlwapi-assocdata) (FTA_OpenIsSafe, FTA_AlwaysUnsafe) or [AssocIsDangerous](https://docs.microsoft.com/windows/win32/api/shlwapi/nf-shlwapi-associsdangerous): Most ProgID keys that have a "URL Protocol" value do not have any "EditFlags" value. Some have FTA_Show = 0x00000002 or FTA_SafeForElevation = 0x00200000.
* [Bind Context String Keys](https://docs.microsoft.com/windows/win32/shell/str-constants) (perhaps an IBindCtx could be provided via [SEE_MASK_FLAG_HINST_IS_SITE](https://docs.microsoft.com/windows/win32/api/shellapi/ns-shellapi-shellexecuteinfow#see_mask_flag_hinst_is_site-0x08000000)): Nothing suitable.
* [STARTF_UNTRUSTEDSOURCE](https://docs.microsoft.com/windows/win32/api/processthreadsapi/ns-processthreadsapi-startupinfoa#STARTF_UNTRUSTEDSOURCE): Could be added via [ICreateProcessInputs::AddStartupFlags](https://docs.microsoft.com/windows/win32/api/shobjidl_core/nf-shobjidl_core-icreateprocessinputs-addstartupflags), but ShellExecuteEx might not create a process, and the URL handler application might not support the flag.

[ASSOCSTR_FRIENDLYAPPNAME](https://docs.microsoft.com/windows/win32/api/shlwapi/ne-shlwapi-assocstr) and [IHandlerActivationHost](https://docs.microsoft.com/windows/win32/api/shobjidl_core/nn-shobjidl_core-ihandleractivationhost) seem to be ways to get the name of the URL handler application, if Windows Terminal wants to display that in the warning.
So, in general I agree with the arguments made on behalf of security here. However, there's one issue that tips me in the other direction:

Given the basic premise that a remote application can't generate meaningful file URIs (it lacks knowledge of the layout of the remote system and any ability to drop files in known places), the concern about an application printing a link to an executable or executable document applies only to _local_ applications.

But then... would such a local application that can produce a path to a malicious executable not itself simply _do the bad thing_? It's running with the user's integrity level and identity already. It can do far, far worse things than trick the user into clicking a link to another EXE file. It _is_, or can just directly spawn, an EXE file.

The same holds for URIs handled by applications that act immediately (like a hypothetical `ms-settings:reset_system?confirmed=yes&confirm_token=aaaaaa` (which doesn't exist, but illustrates the point). Those applications are already potential vectors for attack _regardless_ of who's calling them.

So I guess my question is ... what's the attack, and how does Terminal launching file URIs amplify it?
If we were talking about terminal automatically fetching (or launching!) URLs as directed by an application, I'd be more concerned...
I've located an internal contact related to URI handling and reputation. We'll reach out to the experts at Microsoft who have already handled this for other products and get some advice.
> Given the basic premise that a remote application can't generate meaningful file URIs (it lacks knowledge of the layout of the remote system and any ability to drop files in known places),

If it knows you're on a Windows box, it can reasonably assume it'll be able to find a bunch of executables under c:\Windows, or /mnt/c/Windows. It doesn't have to be right 100% of the time.

Also, note that most exploits are a combination of vulnerabilities. You may be thinking this isn't a problem because an attacker can only execute files that are already on the system, but can't drop files there. But there is likely also a developer somewhere thinking they don't have a problem because their vulnerability only allows the attacker to drop files on the system, but not execute them. Combine the two and the user is screwed.

I should also mention that the original proposed hyperlink code would already have allowed a file url to run remote executables using a UNC path to a public SMB share. In my tests, the OS did at least seem to detect that as a remote executable and provide a warning, but I wouldn't want to rely on that being an insurmountable defence. Anything that lets an attacker get closer to an exploit is a bad thing, even if it doesn't seem to go all the way.
I would love it if the windows terminal supported URIs that point to a specific file resource like sftp. A use case might be to allow someone to script up an alternative form of ls/gci that would output sftp hyperlinks that point to the files listed. One could then ctrl+click to download said file locally rather than having to fire up WinSCP, navigating to the file, and then downloading it to the device.

For that matter if windows terminal supported URIs that pointed to file locations, a future version of Windows terminal could have an option to make the linked item draggable to the desktop.  Could you imagine SSH into a host, cd to the log directory, get a listing of files, and then dragging and dropping a listing item to the desktop!? 🤤 Or maybe being able to right click and select, "Open with VS Code" 🤩 

I've gone way off topic, but supporting URI/URLs that point to specific file resources would be a dream come true.
I found this while trying to use vscode://file/[path to file]:[line] URIs using ANSI Hyperlinks. It would be very helpful to use already registered system handlers. 

I don't see why it needs to be any more secure than a browser (e.g. Are you sure, Choose Application, etc.) but if it was necessary maybe the configuration file can whitelist allowed schemes? (at a minimum, maybe the configuration approach can at least allow a shorter track to allow people to take advantage of them while the rest of the UX is figured out?)

It would also be super handy to have a URI that could enter text as input on the current terminal as I'd love to make my powershell scripts more interactive with less text on the screen. Seems like this could go really well with VCS tooling, powerlines, directory listings (e.g. a hyperlink that change directory to a specific parent) and such. 

One of the first things that came to mind to test this with was adding a little config gear to my powerline and making it a hyperlink to `vscode://file/$PROFILE` or to edit a json config file similarly.
I would love to be able to create **mailto:** and **msteams://** clickable links similar to how we open https:// hyperlinks in MS Terminal.  I can't tell from the above discussion if we will see this capability or not.  Hopefully, there will at least be _some_ way to achieve this through a work-around or official feature.
_note to self: ```Write-Output "`e]8;;msteams://foo`e\This is a link to teams?`e]8;;`e\"``` doesn't work, cause we manually block that._ 

I don't think we ever came to a decision one way or the other. We need to find someone who's an actual expert in this area to give us a bit more feedback. I don't think we're opposed to the idea, but we just don't want to go into this blindly 😄 
Would love to see support for [::{CLSID}](https://www.autohotkey.com/docs/v2/misc/CLSID-List.htm) and [`shell:`](https://ss64.com/nt/shell.html) protocols

For example, I have an interactive directory explorer (https://github.com/Araxeus/ls-interactive) and I'm trying to make everything a link
The problem is linking to `My Computer` - there isn't any valid path to it, only one of the schemes mentioned above
```
shell:MyComputerFolder
```
```
::{20D04FE0-3AEA-1069-A2D8-08002B30309D}
```
@zadjii-msft I think there shouldn't be security problems with those since they are built in windows

----
[`shell:`](https://ss64.com/nt/shell.html) constants:
https://learn.microsoft.com/en-us/windows/win32/shell/search-protocol-crumb#constants-for-common-folders
saved here:
HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows\CurrentVersion\Explorer\FolderDescriptions\


https://learn.microsoft.com/en-us/windows/win32/shell/known-folders

https://support.microsoft.com/en-us/topic/how-to-redirect-user-shell-folders-to-a-specified-path-by-using-profile-maker-ed6289ae-1f9c-b874-4e8c-20d23ea65b2e

https://docs.rainmeter.net/tips/launching-windows-special-folders/


Big 👍 here.

If there's uncertainty over which schemes are "safe" to automatically allow, it seems to me a configuration option to enter a list of schemes (or a URL regex) would be an effective stopgap. I have scripts which output `vscode://` and similar URLs as mentioned in a comment above and it's ineffective in Terminal.
Maybe some URI's that aren't a security concern can get added first?  Who actually decides what can get added and what can't?
I mean, ultimately we as the maintainers decide what can and what can't, though, with feedback from the community. I kinda come down to two trains of thought:

* What schemes are safe for us to enable always? `https`, sure, but the rest I honestly don't know enough about the threat space here to make any sort of decision. That's where I'd love input from someone who _is_ an expert, and that we haven't had the cycles to dig up.
* How can we conditionally add support for additional schemes? Even if we don't allow all of them by default (and I'm sure we never will), what's the best way of enabling users to pick and choose which _are_ enabled? Is this just a list of well-known URI schemes that we let folks opt in to? Or maybe more broadly a whole list of regexes? How do these layer (i.e. profile-specific settings on top of `profiles.defaults` ones)? Another question I certainly haven't had the cycles for recently. 

<hr>

edit: notes here so I don't ping folks:

The relevant security comments: https://github.com/microsoft/terminal/pull/7526#issuecomment-764160208, https://github.com/microsoft/terminal/pull/7526#discussion_r564742669

> I finally get to come back with a bit of an answer to this.
> 
> > Also, using ShellExecute on a file url seems risky from a security point of view.
> 
> We chatted with the defender/application security folks about this. At the end of the day, `ShellExecute` ends up using the same reputation services as the rest of Windows. Defender/ATM/"Sense" gets a chance to intervene if the executable is unsigned or untrustworthy.
> 
> Looking at it all-up:
> 
> * this doesn't let a would-be attacker specify command-line arguments (ala "cmd.exe /s /c do_a_bad_thing") (using somebody else's reputation to cause mayhem)
> * Links require affirmative (Ctrl+click) action to activate, and I will staunchly refuse to add support for opening one without a modifier key
> * For links that the user has vetted:
> 	* A local application that can overwrite a linked region with another link is running at-or-above user permission and can just `do_the_bad_thing()` (or the reputation service kicks in)
> 	* A remote application that can overwrite a linked region either...
> 		1. can manipulate a UNC path and replace an executable (network permission required; reputation service kicks in)
> 		2. can make a user launch an otherwise-acquired local executable (reputation service kicks in)
> * `ShellExecute` de-elevates because it bounces a launch request off the shell
> 
> I'm less concerned about the security aspect of this feature.
> 
> Now- for WSL paths. "Works predictably for 15% of applications" (h/t to PhMajerus' AXSH, and other on-Windows requestors) is better in so many ways than "Works for 0% of applications", in my estimation. Incremental progress :smile: while we work on features that'll make it even more broadly applicable.
> 
> I'm inclined to call in favor of merging this.

> Wouldn't it be better to use the "edit" verb here for file URLs? Perhaps having two separate conditions, like `IsUriSupportedForOpen`, and `IsUriSupportedForEdit`. Because if a user is clicking on a link to a `.py` file, I think it's more likely they would want the file edited rather than executed (which is what the _open_ verb tends to do).
> 
> I don't know if there are any downsides to using the _edit_ verb though. I did bring this up in the #7562 discussion thread, but I don't think anyone commented on the merits of the idea one way or the other.

> I'm not in love with the idea of using the edit verb. I just gave it a test on a couple of the files I had laying around:
> 
> extension|open behavior|edit behavior
> -|-|-
> ps1|notepad|powershell ISE (?!)
> txt|notepad|notepad
> cpp|visual studio preview, which is not my default VS (argh)|notepad
> _no extension_|"how do you want to open this?" dialog|an error message from ShellExecute
> .gitconfig, .wslconifg, etc|same as above|same as above
> 
> It's great if somebody's registered for it, but I'm bothered that the OS doesn't give it the same "do what with?" treatment.

<hr>

more notes

```pwsh
Write-Output "`e]8;;msteams://`e\This link will open Teams`e]8;;`e\"
Write-Output "`e]8;;ms-settings://`e\This link will open the Settings app`e]8;;`e\"
Write-Output "`e]8;;shell:MyComputerFolder`e\This link will open 'My Computer'?`e]8;;`e\"
Write-Output "`e]8;;vscode://file/d:/100k.py:5:10`e\This link will open '100k.py' to line 5, col 10`e]8;;`e\"
```

```bash
printf "\n\x1b]8;;file://wsl$/$WSL_DISTRO_NAME$PWD\x1b\\$P
WD\x1b]8;;\x1b\\ \n\n"
```

those look like the sensible ones I have installed

@zadjii-msft IMO enabling people to add schemes at least (much simpler than allowing complete URI regexes, and I don't really see the benefit as the URI regex should match all of them anyway) would already be a good 80-90% solution. 

I'd add `phpstorm` or `vscode` to an array of allowed schemes in my config (augmenting the defaults of `https`, `http`, `file`) and would become able to click those links. By doing this I'm also taking the responsibility for possibly adding threat vectors, but I doubt there are many ways URIs can be abused, as links are so prevalent online.

Given the utility this brings for developers opening files directly in editors from CLI output, I think it's well worth the addition and the minimal risk this carries.

I fully understand you don't want to spend too much time on a 100% solution here, but it seems like adding the config option for schemes and supporting it in https://github.com/microsoft/terminal/blob/143127d6c4810488df851ae99abe5789fec11e10/src/cascadia/TerminalApp/TerminalPage.cpp#L2461 and wherever the highlighting-on-hover/parsing happens (I could not find this, grepping for `http` or `file` is a nightmare 😄) is reasonably low effort and would resolve the issue for most I'm sure.
You know, there's a good point that I think has maybe been lost in the last two years. I think we long ago conflated "autodetected URIs" with "manually emitted" ones. 

I think there's a lot more minutia that would go into adding more URIs to auto-detect, and how that plays with some other regex asks. 

But I think I've lost the part of the discussion where we only allowed OSC8 links to `https:`, `http:` and `file:`. I'll need to dig for why that was. Seems like from the other discussion notes there's no reason to filter out the other URI schemes, we just only ever supported those to start with...

Fix patch:
diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp
index 94e9865e63a..f20c106ee0b 100644
--- a/src/cascadia/TerminalApp/TerminalPage.cpp
+++ b/src/cascadia/TerminalApp/TerminalPage.cpp
@@ -2473,14 +2473,27 @@ namespace winrt::TerminalApp::implementation
             {
                 return true;
             }
+
+            // GH#10188: WSL paths are okay. We'll let those through.
+            if (host == L"wsl$" || host == L"wsl.localhost")
+            {
+                return true;
+            }
+
             // TODO: by the OSC 8 spec, if a hostname (other than localhost) is provided, we _should_ be
             // comparing that value against what is returned by GetComputerNameExW and making sure they match.
             // However, ShellExecute does not seem to be happy with file URIs of the form
             //          file://{hostname}/path/to/file.ext
             // and so while we could do the hostname matching, we do not know how to actually open the URI
             // if its given in that form. So for now we ignore all hostnames other than localhost
+            return false;
         }
-        return false;
+
+        // In this case, the app manually output a URI other than file:// or
+        // http(s)://. We'll trust the user knows what they're doing when
+        // clicking on those sorts of links.
+        // See discussion in GH#7562 for more details.
+        return true;
     }
 
     // Important! Don't take this eventArgs by reference, we need to extend the


Test patch: