Support hyperlink ansi escapes

[sindresorhus]

Terminal emulators are starting to support hyperlinks. While many terminals have long detected URL's and linkified them, allowing you to Command-Click or Control-Click them to open a browser, you were forced to print the long unsightly URL's on the screen. As of spring 2017 a few terminals began supporting HTML like links, where the link text and destination could be specified separately.

Example from iTerm2 3.1:

[mofux]

Good idea, PR welcome!

[chabou]

It could become a standard and make parsing easier.
Related issue: #583

[sindresorhus]

The biggest benefit is that we can linkify text without having to spam the terminal with a long URL. For example, one could link to the documentation in error messages without making it super verbose.

[albinekb]

This should be implemented with care, you need to be able to hover the link to see where it leads, otherwise you could link users unknowingly to malicious sites. This is already a huge problem in email phishing.

For example you could make a link that had the title "login.facebook.com" but actually link it to login.faceboook.com, having a page that looks exactly the same, but saving the users password on login.

[sindresorhus]

iTerm2 shows the URL like this:

This is also similar to how Chrome shows the URL when you hover links.

[jamestalmage]

If / when this is implemented, please submit an issue or PR to supports-hyperlinks, which is intended to detect support for this capability.

[Tyriar]

@jamestalmage this will probably need to be left up to xterm.js embedders, right now the environment is owned by the terminal emulators that use xterm.js. For example Hyper and VS Code each individually set TERM_PROGRAM.

[jamestalmage]

@Tyriar - In that case, perhaps just include a reference to supports-hyperlinks in the commit that lands this, and in the release notes when it goes wide. I'm not sure if there are supports-hyperlinks equivalents for other languages, but if so, probably worth mentioning them as well.

[PerBothner]

I have proof-of-concept implementation:

links.txt

There is a fundamental problem in that these links not stored in the buffer in any real way, so they go away if the window is re-sized. Instead, the link information is stored in the MouseZoneManager which is cleared by _mouseZoneManager.clearAll.

I believe a real fix has to wait until the Buffer re-implementation is done. We need a mechanism to annotate cells and/or cell ranges, which is stable on resize.

(Or maybe I'm just missing something, being new to this code-base.)

[Tyriar]

I believe a real fix has to wait until the Buffer re-implementation is done. We need a mechanism to annotate cells and/or cell ranges, which is stable on resize.

@PerBothner yeah you're probably right. Another thing we need to think about before this is shippable is how to is the underlying URL gets exposed from xterm.js so embedders can display it in the UI. We would probably also want to turn it off by default for security reasons (since the URL would not be surfaced by default).

[PerBothner]

I updated my patch and pushed it into a branch: https://github.com/PerBothner/xterm.js/tree/hyperlinks

Not ready for a pull request, though - various problems mentioned earlier have not been addressed.

[Tyriar]

Proposal for the API to use this:

export class Terminal {
	/**
	 * Adds a handler for the ANSI hyperlink escape `\x1b]8;;url\alabel\x1b]8;;`, you should use
	 * this API to display the full URL to the user. Note that ANSI hyperlinks will only work if
	 * there is a handler for security reasons.
	 * @param onHover The callback that fires when the mouse enters a link's zone.
	 * @param onLeave The callback that fires when the mouse leaves a link's zone.
	 * @return An IDisposable which can be used to disable the handler.
	 */
	addAnsiHyperlinkHandler(onHover: (event: MouseEvent, url: string) => void, onLeave: () => void): IDisposable;
}

@mofux @jerch feedback on API shape? I can't find that thread talking about the form of these, but maybe this should be set instead of add as it only makes sense to have 1.

Also it might be better to use something like ILinkHoverEvent:

interface ILinkHoverEvent {
  // Maybe the cell the mouse is under is also needed?
  x1: number;
  y1: number;
  x2: number;
  y2: number;
  url: string;
}

export class Terminal {
	addAnsiHyperlinkHandler(onHover: (event: ILinkHoverEvent) => void, onLeave: () => void): IDisposable;
}

Another alternative:

interface ILinkHoverEvent {
  linkStart: Cell;
  linkEnd: Cell;
  mousePosition: Cell;
  url: string
}

[mofux]

@Tyriar IMO it doesn't make much sense to create a separate handler for this. The likely consumer of this API would be our renderer that renders the url in a tooltip if we hover the linked text, wouldn't it? Maybe it would make sense to extend our linkifier with the aforementioned hooks for onLinkHover and onLinkLeave, and have those ansi hyperlinks handled like we do with web links right now?

[Tyriar]

The likely consumer of this API would be our renderer that renders the url in a tooltip

@mofux I was thinking more like the search addon, the embedder provides the UI in whatever style they want, we just provide the callbacks to do so. Good point that this doesn't let you handle things yet. I guess merging in to the existing link API and having a setting ITerminalOptions.enableAnsiHyperlinks for opt-in and details the why?

[mofux]

I guess merging in to the existing link API and having a setting ITerminalOptions.enableAnsiHyperlinks for opt-in and details the why?

How would be the story for embedders to support this feature? Opening the (invisible) link when clicking the linked text is potentially dangerous - especially if we don't show the linked URL anywhere 🤔

[egmontkob]

While showing the target upfront is indeed nice, I don't think there's a problem in opening "unknown" URLs. The security aspects have been discussed in the comments under the specs. Browsers open "unknown" URLs all the time, e.g. when there's a JS code that alters the target just before opening it, often to show a "nice" URL to the user, but actually open it through a redirector.

Speaking of which... something that hasn't occurred to me so far...

If xterm.js is running inside an actual browser, and opens the link in let's say a new tab of that browser: I guess leaking xterm.js's URL via the Referer field is something to worry about. Not sure about the current support for rel="noreferrer", but seems like something that should be used.

[PerBothner]

See this related discussion of hover text. That is issue is about annotating sections of the output with "tool-tops" and other mouse-over popups, which is not the same thing as displaying the URL when you hover over a link. However, both may want to use the same mechanism to display the actual hover text. Both would probably make use of a MouseZoneManager.

[jerch]

Do we have any XSS experts hanging around? Maybe we need an audit, lol.

For my limited knowledge about browser security the main attack vector with xterm.js is data crossing the terminal/browser border:

• XSS from the browser(JS) into the terminal(data)
  Thats already possible and imho the integrator's responsibility (xterm.js cannot control this by any means). Rule of thumb - never ever import scripts from any untrusted source on the terminal page or the pty websocket and thus the system that hosts the pty is lost. Dont allow unfiltered user content to be inserted into the page and such - basically all the typical XSS things, or the pty is lost.
• XSS from the terminal(data) into the browser(JS)
  Thats a new quality we might enter with URLs, if we dont make sure that the terminal data will never reach the JS context of the embedding page (thus the terminal object itself). If thats possible (lets assume that for a second) an attacker can gain access to the browser page, and thus do a terminal(data) - browser(JS) - terminal(data) attack. Lets further assume xterm.js runs alot in cloud orchestrating service portals and the admin uses terminal sessions to different machines. Outch. Once the attacker gains access to the embedding browser page kinda all cloud services the admin has access to are in danger.

The question is - are there possibilities to cross the terminal(data) - browser(JS) border with URLs? Again my limited knowledge about browser security does not help here much. Only scenario I can think of are bookmarklets that inject themselves into the page's JS. I have no clue if we can avoid this by always open stuff in a new window/tab only (guess the session will still leak, if not httpOnly? What about the websocket connection?) Which makes me think that we have to parse the URL and strip any "JS looking content", oh dear...

Edit: Note that websockets miss most of the browser's security settings, they dont even have a reliable same origin check (go with ajax long polling if you need this lol). Hmm...

[egmontkob]

bookmarklets [...] strip any "JS looking content"

I think it's a good idea to whitelist only URLs beginning with "http://", "https://", maybe "ftp://" and reject anything else. Or at the very least blacklist the lack of scheme as well as "javascript:".

[Tyriar]

VS Code now has support for showing detailed hovers for links, so hooking this up to that will solve the problem:

It's a shame it's a bit of a weird sequences as our parser hooks don't cover it. If they did this could be done entirely as an addon using parser hooks, markers and the link provider APIs.

[jerch]

@Tyriar Also with #2751 the extended attr storage can be used to annotate URL stuff to the buffer cells.

It's a shame it's a bit of a weird sequences as our parser hooks don't cover it. If they did this could be done entirely as an addon using parser hooks, markers and the link provider APIs.

Yeah thats a problem and imho cannot be solved at addon level, even if we'd expose a print handler hook. I think this has to go directly into the codebase plus a few exceptional conditions (like any non print action before the finalizer is acknowledged should break the URL marking of cells and such).

[Tyriar]

Took a shot at this as a weekend project, will probably need another weekend to finish it up 🙂 #3904

[ssbarnea]

Thanks! You will make some many users happy.

[cidrblock]

This will be great! TY