Fix OSC 8 links opening in external browser #3486
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -143,7 +143,14 @@ export class TermWrap { | |
| this.lastCommandAtom = jotai.atom(null) as jotai.PrimitiveAtom<string | null>; | ||
| this.claudeCodeActiveAtom = jotai.atom(false); | ||
| this.webglEnabledAtom = jotai.atom(false) as jotai.PrimitiveAtom<boolean>; | ||
| this.terminal = new Terminal({ | ||
| ...options, | ||
| linkHandler: { | ||
| activate: (_event, uri) => { | ||
| fireAndForget(() => openLink(uri)); | ||
|
Comment on lines
+149
to
+150
Contributor
There was a problem hiding this comment. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- applicable repository guidance ---'
find /tmp/coderabbit-repo-knowledge/wavetermdev-waveterm-904946a9 -maxdepth 2 -type f -name '*.md' -print
printf '%s\n' '--- target file outline ---'
ast-grep outline frontend/app/view/term/termwrap.ts
printf '%s\n' '--- target implementation ---'
sed -n '120,190p' frontend/app/view/term/termwrap.ts
printf '%s\n' '--- direct definitions and callers ---'
rg -n -A8 -B8 'function openLink|const openLink|openLink\\(|linkHandler|WebLinksAddon|createBlock|openExternal' frontend/app frontend 2>/dev/null | head -240Repository: wavetermdev/waveterm Length of output: 4643 🏁 Script executed: #!/bin/bash
set -eu
for f in /tmp/coderabbit-repo-knowledge/wavetermdev-waveterm-904946a9/*/*.md; do
case "$f" in
*convention*|*guideline*|*term*|*link*|*security*) printf '\n--- %s ---\n' "$f"; cat "$f" ;;
esac
doneRepository: wavetermdev/waveterm Length of output: 850 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- imports and link-related code ---'
sed -n '1,85p' frontend/app/view/term/termwrap.ts
rg -n -F 'openLink' frontend/app frontend
rg -n -F 'linkHandler' frontend/app frontend
rg -n -F 'WebLinksAddon' frontend/app frontend
printf '%s\n' '--- xterm dependency declarations ---'
rg -n -F '`@xterm/xterm`' package.json frontend package-lock.json yarn.lock pnpm-lock.yaml 2>/dev/null || trueRepository: wavetermdev/waveterm Length of output: 5466 🌐 Web query:
💡 Result: In xterm.js, there is no built-in "modifier key" property directly on the ILinkHandler interface for OSC 8 hyperlinks [1][2]. Instead, the implementation of modifier-key requirements is handled by the embedder within the activate callback of the ILinkHandler [3][1]. To require a modifier key (such as Ctrl or Cmd) for opening links, you must check the modifier state of the provided MouseEvent inside your activate implementation [3]. Implementation approach: 1. Define a linkHandler object that implements the ILinkHandler interface [1][2]. 2. In the activate(event, text, range) method, check if the desired modifier key is pressed [3]. 3. For cross-platform compatibility, you can check event.ctrlKey for Windows/Linux and event.metaKey for macOS [3]. Example: const linkHandler = { activate: (event, uri) => { // Check if Ctrl (or Cmd on Mac) is pressed const isModifierPressed = isMac? event.metaKey: event.ctrlKey; if (isModifierPressed) { // Open the link window.open(uri, '_blank'); } } }; // Assign the handler to the terminal options terminal.options.linkHandler = linkHandler; Note: The official xterm.js documentation explicitly recommends implementing this check to avoid unintentional window-opening and to improve user security [3][4]. If no link handler is provided, xterm.js defaults to using the browser's confirm API for OSC 8 links, which triggers a warning prompt [5][4]. Historical context: While older or alternative link-matching APIs (such as ILinkMatcher) may have included a willLinkActivate callback for modifier checks [6], the standard OSC 8 linkHandler interface expects the logic to be handled directly within the activate function [3][1]. Citations:
Require Cmd/Ctrl for OSC 8 activation. The 🤖 Prompt for AI AgentsSource: MCP tools
Contributor
There was a problem hiding this comment. 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- repository conventions and learnings ---'
find /tmp/coderabbit-repo-knowledge/wavetermdev-waveterm-904946a9 \
-maxdepth 2 -type f -name '*.md' -print
printf '%s\n' '--- openLink and nearby definitions ---'
sed -n '520,555p' frontend/app/store/global.ts
printf '%s\n' '--- createBlock and openExternal references ---'
rg -n -S 'function createBlock|const createBlock|createBlock\s*=|openExternal' frontend/app frontend/util --glob '*.{ts,tsx}'Repository: wavetermdev/waveterm Length of output: 2844 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- repository convention scope ---'
cat /tmp/coderabbit-repo-knowledge/wavetermdev-waveterm-904946a9/learnings/null.md
printf '%s\n' '--- createBlock implementation ---'
sed -n '400,435p' frontend/app/store/global.ts
printf '%s\n' '--- all openExternal definitions and bridge wiring ---'
rg -n -S 'openExternal' . --glob '!node_modules' --glob '!dist' --glob '!build'
printf '%s\n' '--- web block URL handling ---'
rg -n -S 'meta\.url|blockDef\.meta\.url|url:' frontend/app/view/webview frontend/app/block --glob '*.{ts,tsx}'Repository: wavetermdev/waveterm Length of output: 5779 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- preload openExternal validation ---'
sed -n '1,45p' emain/preload.ts
printf '%s\n' '--- tab-view external opener ---'
sed -n '325,355p' emain/emain-tabview.ts
printf '%s\n' '--- webview navigation handling ---'
sed -n '360,425p' frontend/app/view/webview/webview.tsx
printf '%s\n' '--- webview initialization and URL loading ---'
rg -n -S 'loadURL|loadUrl|handleNavigate|urlAtom|url:' frontend/app/view/webview/webview.tsx | head -40Repository: wavetermdev/waveterm Length of output: 8131 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- open-external IPC handler ---'
sed -n '180,212p' emain/emain-ipc.ts
printf '%s\n' '--- internal webview URL load path ---'
sed -n '425,475p' frontend/app/view/webview/webview.tsx
printf '%s\n' '--- webview event handlers near navigation ---'
sed -n '1000,1040p' frontend/app/view/webview/webview.tsxRepository: wavetermdev/waveterm Length of output: 4942 Other (CWE-20): Improper Input Validation Reachability: External · Exploitability: Moderate Allow only HTTP(S) OSC 8 URIs. The terminal controls 🤖 Prompt for AI AgentsSource: MCP tools |
||
| }, | ||
| }, | ||
| }); | ||
| this.fitAddon = new FitAddon(); | ||
| this.serializeAddon = new SerializeAddon(); | ||
| this.searchAddon = new SearchAddon(); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: wavetermdev/waveterm
Length of output: 4838
🏁 Script executed:
Repository: wavetermdev/waveterm
Length of output: 7830
🌐 Web query:
@xterm/xterm 6.0.0 linkHandler activate confirm fallback TerminalOptions documentation💡 Result:
In
@xterm/xtermversion 6.0.0, the linkHandler option allows developers to customize the behavior of OSC 8 hyperlinks [1][2]. If no custom linkHandler is provided, xterm.js defaults to a built-in activate function that uses the browser's confirm API to display a security warning before navigating to the link [3][2]. The ILinkHandler interface provides methods to control link interactions [1][4]: activate(event, text, range): Triggered when a link is activated (e.g., clicked) [1]. hover(event, text, range): Optional; triggered when the mouse hovers over a link, often used to display a custom tooltip [1][5]. leave(event, text, range): Optional; triggered when the mouse leaves a link [1][6]. For security, the ILinkHandler interface includes an allowNonHttpProtocols property [1][4]. By default, xterm.js restricts links to http: and https: protocols to prevent risks such as XSS (e.g., from javascript: links) [3][7]. If your application requires custom protocols (like vscode:// or others), you must explicitly set allowNonHttpProtocols to true [7][8]. When implementing a custom linkHandler, the documentation strongly advises providing visual feedback, such as a tooltip or prompt, due to the inherent security risks of opening links in a terminal environment [1][2]. The terminal's internal OscLinkProvider uses this handler to route events and enforce the protocol restriction [3].Citations:
Preserve xterm.js link confirmation.
Setting
linkHandlerreplaces xterm.js's default browser confirmation for OSC 8 links. This handler callsopenLink(uri)directly, which can reachcreateBlockoropenExternalwithout confirmation. Preserve the confirmation before opening the URI.🤖 Prompt for AI Agents
Source: MCP tools