Link preview / protection in workbench

[octref]

With Markdown support in HTML/CSS extensions, users would see links going to external websites such as https://developer.mozilla.org/en-US/ and https://www.w3.org/. When displaying these links, we need to protect users as we don't control the linked websites.

Previous proposal. After you click link:

Notification asks you to whitelist domains:

After that, a setting would be written to css.whiteListDomains. Next time you click open that link, it opens in browser directly.

We had some concerns this complicates a simple link opening. Here's my alternative proposal:

This applies to JS/TS as well:

For command links (GitLens), this would also give more context as to what command would be run. Maybe we can truncate the command to only show command but not its args.

[octref]

/cc @aeschli Since you didn't like the notification either.

[jrieken]

For external links, can’t we copy what websites like wikipedia do and add an ‘external’ icon to the text. Like so:

For command links (GitLens), this would also give more context as to what command would be run. Maybe we can truncate the command to only show command but not its args.

Strong disagree. I think that would ruin the command link experience. Also, I don't understand how command links (which only work when markdown content is marked as trusted) are the same problem as external websites.

[octref]

@jrieken Would showing the link preview only for http/https links work for you?

We discussed about showing just an icon. It's a good idea and we can even show icons of different colors depending on scheme (http, command, etc). But it doesn't reveal the link and some of us feel we should make sure link location is shown before user can click (explicit consent & assume the risk).

I can bring this up in next week's UX review.

[jrieken]

Would showing the link preview only for http/https links work for you?

Yeah, foremost I think this should only be done for http/https links, not for file- or command-links. I don't think colors help because in contrast to said icons, there is no well-accepted color metaphor for external links.

Instead of showing this so prominent, wouldn't it be enough to show its href as title/hover thing? That what I would have accepted...

[octref]

Currently if you hover over it for 3 seconds there's the default tooltip over the link.

We could override it to show a quicker, themed tooltip.

Instead of showing this so prominent

I'll prepare some alternatives that make this less prominent for next UX meeting.

[jrieken]

Currently if you hover over it for 3 seconds there's the default tooltip over the link.

Which is what I am used to tbh

[octref]

Current dialog:

• Open Link only opens link once
• Trust links on domain is a checkbox, as we want user to explicitly opt-in
• No "Trust all links" or "Trust all domains" button, as requested by @isidorn. You can only configure this manually by adding a * to the http.trustedDomains setting. I also know that @aeschli and @jrieken prefer *. Maybe you three can discuss a bit?

@Tyriar raised the point that extensions can modify setting through API, so from a security perspective this should be local storage item instead of a setting. I tried implementing a command to show quick pick, but this doesn't handle add/edit cases. I still think setting is fine, as the most evil thing an extension can do would be allow all links, which is as secure as we have it today.

[isidorn]

@octref great. As for the quick pick I would have it very simple if we decide to do it. Command is something like "Edit trusted links". Which opens command palette again with two options "Reset" and "Allow all" with no checkboxes just text. I understand this does not allow fine tuning but I do not really see this scenario.
But in the end I still prefer putting this to settings if we think that is fine from the security standpoint

I would just change the wording "Trust all links on" -> "Trust all links from"
I also do not understand why do we put the link two times in the dialog? Shuoldn't just once in bold be enough?

[jrieken]

so from a security perspective this should be local storage item instead of a setting.

Does that mean I will get prompted for each and every window? Or will this be a global choice? How do we ship a list of allows domain? Not at all, hardcode in the sources or via product.json?

[jrieken]

No "Trust all links" or "Trust all domains" button, as requested by @isidorn.

@isidorn Strong objection. Even tho this issue feels like bike shedding but we are talking about opening links in a browser. Why can't we have an option in the dialog (not the default) that says 'trust all links'?

[isidorn]

@jrieken I just wanted us to be on the safe side, and that the "trust all links" is a hidden option.
I felt like that it does not happen that often that user opens multiple domains quickly and that it would not be a total pain in the ass to add all those domains one by one.
However if you expect users to open different domains from vscode all the time then yeah making the "Trust all domains" more visible makes sense.
I personally only open vscode links from vscode, though I understand other users might be diferent.

[jrieken]

I would say it's pretty common to have comments with links to SO, GH but also to all kinds of internal company sides and I would says it is PITA if you constantly get prompted. This is links we are talking about, not executables or such.

[octref]

One downside of dropping "Trust all links" is * becomes a hidden feature buried in one month's release notes. If we don't have that button, I'd want some indication in the dialog for *.

[octref]

Current flow:

• One command "Configure Trusted Domains"
• User can toggle on and off *
• User can delete but cannot add/edit existing entries

[octref]

@isidorn @jrieken Would this be a fine compromise between you two?

[isidorn]

@octref the only downside of that approach is that I can not open the link and tell you to allow it for the future in one go. So I have to first configure, than click on the link again to open.
It would be cool if after configuring and if the link is allowed that we still open the link.

[octref]

Make sense, added description and open link after trusting the current domain:

[jrieken]

One more nit: We shouldn't say "without protection" but "without prompt", e.g "Open all links without prompt".

[octref]

Updated as requested.