Skip to content

Commit 5d7c22f

Browse files
committed
fix(terminal): open links externally
1 parent eb60531 commit 5d7c22f

9 files changed

Lines changed: 256 additions & 15 deletions

File tree

electron/ipc/channels.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ export enum IPC {
8484
ShellReveal = '__shell_reveal',
8585
ShellOpenFile = '__shell_open_file',
8686
ShellOpenInEditor = '__shell_open_in_editor',
87+
ShellOpenExternal = '__shell_open_external',
8788

8889
// Arena
8990
SaveArenaData = 'save_arena_data',

electron/ipc/register.test.ts

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { describe, expect, it } from 'vitest';
2-
import { selectMcpJsonDir } from './register.js';
2+
import { openExternalHttpUrl, selectMcpJsonDir, validateExternalHttpUrl } from './register.js';
33

44
describe('selectMcpJsonDir', () => {
55
it('returns worktreePath when defined', () => {
@@ -14,3 +14,62 @@ describe('selectMcpJsonDir', () => {
1414
expect(selectMcpJsonDir('', '/project')).toBe('');
1515
});
1616
});
17+
18+
describe('validateExternalHttpUrl', () => {
19+
it('accepts http and https URLs', () => {
20+
expect(validateExternalHttpUrl('https://example.com/path?q=1')).toBe(
21+
'https://example.com/path?q=1',
22+
);
23+
expect(validateExternalHttpUrl('http://example.com/')).toBe('http://example.com/');
24+
});
25+
26+
it('normalizes protocol and host casing', () => {
27+
expect(validateExternalHttpUrl('HTTPS://EXAMPLE.COM/pr/1')).toBe('https://example.com/pr/1');
28+
});
29+
30+
it('rejects non-web protocols', () => {
31+
expect(() => validateExternalHttpUrl('file:///etc/passwd')).toThrow(
32+
'url must use http or https',
33+
);
34+
expect(() => validateExternalHttpUrl('javascript:alert(1)')).toThrow(
35+
'url must use http or https',
36+
);
37+
});
38+
39+
it('rejects invalid and non-string values', () => {
40+
expect(() => validateExternalHttpUrl('not a url')).toThrow('url must be a valid URL');
41+
expect(() => validateExternalHttpUrl(undefined)).toThrow('url must be a string');
42+
});
43+
});
44+
45+
describe('openExternalHttpUrl', () => {
46+
it('opens the normalized URL', async () => {
47+
const opened: string[] = [];
48+
49+
await openExternalHttpUrl('HTTPS://EXAMPLE.COM/pr/1', async (url) => {
50+
opened.push(url);
51+
});
52+
53+
expect(opened).toEqual(['https://example.com/pr/1']);
54+
});
55+
56+
it('does not call the opener for invalid URLs', async () => {
57+
const opened: string[] = [];
58+
59+
await expect(
60+
openExternalHttpUrl('file:///etc/passwd', async (url) => {
61+
opened.push(url);
62+
}),
63+
).rejects.toThrow('url must use http or https');
64+
65+
expect(opened).toEqual([]);
66+
});
67+
68+
it('does not include the URL in opener failure errors', async () => {
69+
await expect(
70+
openExternalHttpUrl('https://example.com/?token=secret', async (url) => {
71+
throw new Error(`OS refused ${url}`);
72+
}),
73+
).rejects.toThrow('Failed to open external URL');
74+
});
75+
});

electron/ipc/register.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,32 @@ function validateRelativePath(p: unknown, label: string): void {
219219
if (p.includes('..')) throw new Error(`${label} must not contain ".."`);
220220
}
221221

222+
export function validateExternalHttpUrl(rawUrl: unknown): string {
223+
assertString(rawUrl, 'url');
224+
let parsed: URL;
225+
try {
226+
parsed = new URL(rawUrl);
227+
} catch {
228+
throw new Error('url must be a valid URL');
229+
}
230+
if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') {
231+
throw new Error('url must use http or https');
232+
}
233+
return parsed.href;
234+
}
235+
236+
export async function openExternalHttpUrl(
237+
rawUrl: unknown,
238+
openExternal: (url: string) => Promise<void> = shell.openExternal,
239+
): Promise<void> {
240+
const url = validateExternalHttpUrl(rawUrl);
241+
try {
242+
await openExternal(url);
243+
} catch {
244+
throw new Error('Failed to open external URL');
245+
}
246+
}
247+
222248
const validateBranchName = sharedValidateBranchName;
223249

224250
/** Reject commit hashes that are not valid hex strings. */
@@ -1119,6 +1145,10 @@ export function registerAllHandlers(win: BrowserWindow): void {
11191145
return shell.openPath(path.join(args.worktreePath, args.filePath));
11201146
});
11211147

1148+
ipcMain.handle(IPC.ShellOpenExternal, async (_e, args) => {
1149+
await openExternalHttpUrl(args.url);
1150+
});
1151+
11221152
ipcMain.handle(IPC.ShellOpenInEditor, (_e, args) => {
11231153
validatePath(args.worktreePath, 'worktreePath');
11241154
if (typeof args.editorCommand !== 'string' || !args.editorCommand.trim()) {

electron/ipc/trace.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ const NEVER_SAFE: ReadonlySet<string> = new Set<string>([
3434
IPC.ShellOpenInEditor,
3535
IPC.ShellOpenFile,
3636
IPC.ShellReveal,
37+
IPC.ShellOpenExternal,
3738
IPC.DialogOpen,
3839
IPC.OpenPath,
3940
IPC.ReadFileText,

electron/preload.cjs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ const ALLOWED_CHANNELS = new Set([
8282
'__shell_reveal',
8383
'__shell_open_file',
8484
'__shell_open_in_editor',
85+
'__shell_open_external',
8586
// Arena
8687
'save_arena_data',
8788
'load_arena_data',

src/components/TerminalView.tsx

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import {
3131
import { dataTransferToShellArgs, escapePath } from '../lib/terminalDrop';
3232
import { cleanCopiedTerminalText } from '../lib/copy-text';
3333
import { hasTerminalUserActivity, nextTerminalInputPending } from '../lib/terminalInputPending';
34+
import { createTerminalHttpLinkHandler } from '../lib/terminalLinks';
3435
import type { PtyOutput } from '../ipc/types';
3536

3637
let windowUnloading = false;
@@ -90,6 +91,13 @@ interface TerminalViewProps {
9091
// expensive full-chunk decoding during large terminal bursts.
9192
const STATUS_ANALYSIS_MAX_BYTES = 8 * 1024;
9293

94+
const openTerminalHttpLinkWithModifier = createTerminalHttpLinkHandler({
95+
isMac,
96+
requireModifier: true,
97+
openExternal: (url) => invoke(IPC.ShellOpenExternal, { url }),
98+
onOpenError: () => logWarn('terminal.link', 'Failed to open external URL'),
99+
});
100+
93101
/** Terminal-layer bindings — filtered from resolved bindings.
94102
* Called in the key handler (hot path); resolveBindings walks the full
95103
* defaults list on each call, which is fine at human typing speed. */
@@ -137,24 +145,15 @@ export function TerminalView(props: TerminalViewProps) {
137145
allowProposedApi: true,
138146
scrollback: TERMINAL_SCROLLBACK_LINES,
139147
disableStdin: taskPtyDetached(),
148+
linkHandler: {
149+
activate: openTerminalHttpLinkWithModifier,
150+
allowNonHttpProtocols: false,
151+
},
140152
});
141153

142154
fitAddon = new FitAddon();
143155
term.loadAddon(fitAddon);
144-
term.loadAddon(
145-
new WebLinksAddon((event, uri) => {
146-
// Require Cmd+click (Mac) or Ctrl+click (Linux) to open links
147-
if (!(isMac ? event.metaKey : event.ctrlKey)) return;
148-
try {
149-
const parsed = new URL(uri);
150-
if (parsed.protocol === 'http:' || parsed.protocol === 'https:') {
151-
window.open(uri, '_blank');
152-
}
153-
} catch {
154-
// Invalid URL, ignore
155-
}
156-
}),
157-
);
156+
term.loadAddon(new WebLinksAddon(openTerminalHttpLinkWithModifier));
158157

159158
term.open(containerRef);
160159

src/lib/terminalLinks.test.ts

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
import { describe, expect, it, vi } from 'vitest';
2+
import { createTerminalHttpLinkHandler, normalizeHttpUrl } from './terminalLinks';
3+
4+
function event(overrides: Partial<MouseEvent> = {}) {
5+
return {
6+
ctrlKey: false,
7+
metaKey: false,
8+
preventDefault: vi.fn(),
9+
stopPropagation: vi.fn(),
10+
...overrides,
11+
};
12+
}
13+
14+
describe('normalizeHttpUrl', () => {
15+
it('accepts and normalizes web URLs', () => {
16+
expect(normalizeHttpUrl('HTTPS://EXAMPLE.COM/pr/1')).toBe('https://example.com/pr/1');
17+
});
18+
19+
it('rejects invalid and non-web URLs', () => {
20+
expect(normalizeHttpUrl('file:///etc/passwd')).toBeNull();
21+
expect(normalizeHttpUrl('javascript:alert(1)')).toBeNull();
22+
expect(normalizeHttpUrl('not a url')).toBeNull();
23+
});
24+
});
25+
26+
describe('createTerminalHttpLinkHandler', () => {
27+
it('does nothing for unmodified desktop clicks when a modifier is required', () => {
28+
const openExternal = vi.fn();
29+
const e = event();
30+
const handler = createTerminalHttpLinkHandler({
31+
isMac: false,
32+
requireModifier: true,
33+
openExternal,
34+
});
35+
36+
handler(e, 'https://example.com/');
37+
38+
expect(openExternal).not.toHaveBeenCalled();
39+
expect(e.preventDefault).not.toHaveBeenCalled();
40+
});
41+
42+
it('opens with Ctrl on non-mac platforms', () => {
43+
const openExternal = vi.fn();
44+
const e = event({ ctrlKey: true });
45+
const handler = createTerminalHttpLinkHandler({
46+
isMac: false,
47+
requireModifier: true,
48+
openExternal,
49+
});
50+
51+
handler(e, 'HTTPS://EXAMPLE.COM/pr/1');
52+
53+
expect(openExternal).toHaveBeenCalledWith('https://example.com/pr/1');
54+
expect(e.preventDefault).toHaveBeenCalledTimes(1);
55+
expect(e.stopPropagation).not.toHaveBeenCalled();
56+
});
57+
58+
it('opens with Cmd on macOS', () => {
59+
const openExternal = vi.fn();
60+
const e = event({ metaKey: true });
61+
const handler = createTerminalHttpLinkHandler({
62+
isMac: true,
63+
requireModifier: true,
64+
openExternal,
65+
});
66+
67+
handler(e, 'https://example.com/');
68+
69+
expect(openExternal).toHaveBeenCalledWith('https://example.com/');
70+
});
71+
72+
it('blocks invalid URLs after taking over the click', () => {
73+
const openExternal = vi.fn();
74+
const e = event({ ctrlKey: true });
75+
const handler = createTerminalHttpLinkHandler({
76+
isMac: false,
77+
requireModifier: true,
78+
openExternal,
79+
});
80+
81+
handler(e, 'javascript:alert(1)');
82+
83+
expect(openExternal).not.toHaveBeenCalled();
84+
expect(e.preventDefault).toHaveBeenCalledTimes(1);
85+
});
86+
87+
it('reports opener failures without throwing', async () => {
88+
const onOpenError = vi.fn();
89+
const handler = createTerminalHttpLinkHandler({
90+
isMac: false,
91+
openExternal: () => Promise.reject(new Error('failed')),
92+
onOpenError,
93+
});
94+
95+
handler(event(), 'https://example.com/');
96+
await Promise.resolve();
97+
98+
expect(onOpenError).toHaveBeenCalledTimes(1);
99+
});
100+
});

src/lib/terminalLinks.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
export interface TerminalLinkMouseEvent {
2+
ctrlKey: boolean;
3+
metaKey: boolean;
4+
preventDefault: () => void;
5+
}
6+
7+
export interface TerminalLinkHandlerOptions {
8+
isMac: boolean;
9+
openExternal: (url: string) => Promise<void> | void;
10+
onOpenError?: () => void;
11+
requireModifier?: boolean;
12+
}
13+
14+
export function normalizeHttpUrl(uri: string): string | null {
15+
try {
16+
const parsed = new URL(uri);
17+
if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') return null;
18+
return parsed.href;
19+
} catch {
20+
return null;
21+
}
22+
}
23+
24+
export function createTerminalHttpLinkHandler(options: TerminalLinkHandlerOptions) {
25+
return (event: TerminalLinkMouseEvent, uri: string): void => {
26+
if (options.requireModifier && !(options.isMac ? event.metaKey : event.ctrlKey)) return;
27+
28+
event.preventDefault();
29+
const url = normalizeHttpUrl(uri);
30+
if (!url) return;
31+
32+
try {
33+
Promise.resolve(options.openExternal(url)).catch(() => options.onOpenError?.());
34+
} catch {
35+
options.onOpenError?.();
36+
}
37+
};
38+
}

src/remote/AgentDetail.tsx

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { onMount, onCleanup, createSignal, Show } from 'solid-js';
22
import { Terminal } from '@xterm/xterm';
33
import { FitAddon } from '@xterm/addon-fit';
44
import { TERMINAL_SCROLLBACK_LINES, base64ToUint8Array } from '../lib/terminalConstants';
5+
import { createTerminalHttpLinkHandler } from '../lib/terminalLinks';
56
import { subscribeAgent, unsubscribeAgent, onOutput, onScrollback, agents, status } from './ws';
67

78
interface AgentDetailProps {
@@ -10,6 +11,13 @@ interface AgentDetailProps {
1011
onBack: () => void;
1112
}
1213

14+
const openRemoteHttpLink = createTerminalHttpLinkHandler({
15+
isMac: false,
16+
openExternal: (url) => {
17+
window.open(url, '_blank', 'noopener,noreferrer');
18+
},
19+
});
20+
1321
export function AgentDetail(props: AgentDetailProps) {
1422
let termContainer: HTMLDivElement | undefined;
1523
let term: Terminal | undefined;
@@ -41,6 +49,10 @@ export function AgentDetail(props: AgentDetailProps) {
4149
cursorBlink: false,
4250
disableStdin: true,
4351
convertEol: false,
52+
linkHandler: {
53+
activate: openRemoteHttpLink,
54+
allowNonHttpProtocols: false,
55+
},
4456
});
4557

4658
fitAddon = new FitAddon();

0 commit comments

Comments
 (0)