diff --git a/public/link-paths.ts b/public/link-paths.ts new file mode 100644 index 0000000..b8e8b5c --- /dev/null +++ b/public/link-paths.ts @@ -0,0 +1,91 @@ +/** + * public/link-paths.ts — pure file-path matcher for the terminal link provider (W1). + * + * Finds `src/app.ts:42`, `README.md`, `../lib/util.rs:10:5` etc. in a line of + * terminal text so the terminal-session link provider can turn them into + * clickable "open in editor" links. Deliberately DOM-free / xterm-free so it is + * unit-testable in node and reusable by the diff/approval-preview views later. + * + * A candidate token is: an optional `./`, `../` or `dir/…/` prefix, a filename + * with a dot-extension, and an optional `:line` / `:line:col` suffix. A candidate + * is only linked when it is *probably* a real path — it has a `/` separator, OR a + * `:line` suffix, OR its extension is in the CODE_EXT allowlist. This links code + * paths while ignoring `example.com`, `v1.2.3`, `foo.bar`, and `12:34`. + */ + +/** File extensions that mark a bare (slash-less, line-less) token as a code path. */ +export const CODE_EXT: ReadonlySet = new Set([ + 'ts', 'tsx', 'js', 'jsx', 'mjs', 'cjs', + 'py', 'go', 'rs', 'rb', 'java', 'kt', + 'c', 'h', 'cpp', 'hpp', 'cc', 'cs', 'php', 'swift', + 'css', 'scss', 'html', 'json', 'yaml', 'yml', 'toml', + 'md', 'txt', 'sh', 'sql', 'vue', 'svelte', +]) + +export interface PathMatch { + /** Exact matched substring, incl. any `:line[:col]` suffix (e.g. "src/app.ts:42"). */ + text: string + /** The file-path portion only, without the line/column suffix (e.g. "src/app.ts"). */ + path: string + /** 1-based line number, when a `:line` suffix was present. */ + line?: number + /** 1-based column, when a `:line:col` suffix was present. */ + column?: number + /** 1-based column of the first char (maps to an xterm range.start.x). */ + startX: number + /** 1-based column of the last char, inclusive (maps to an xterm range.end.x). */ + endX: number +} + +// A path candidate. The leading negative lookbehind rejects tokens glued to a +// preceding word char, '.', '/', ':', '@' or '-' — this keeps the matcher from +// grabbing the `path.html` tail of a `https://host/path.html` URL (which the +// WebLinksAddon owns) or the middle of a larger identifier. The optional prefix +// admits an absolute `/`, a `./` or a `../` before the dir segments. +// group 1 = the path (prefix + dirs + filename.ext), no line/col +// group 2 = line digits (optional) +// group 3 = column digits (optional) +const PATH_RE = + /(? = new Set(['http:', 'https:', 'mailto:']) + +/** + * Open a URL clicked in the terminal (W1 hardening). Allowlists the scheme — + * blocking `javascript:` / `data:` / `file:` URIs — and opens with + * `noopener,noreferrer` to prevent reverse-tabnabbing. Exported for unit tests. + */ +export function openWebLink(uri: string): void { + let protocol: string + try { + protocol = new URL(uri).protocol + } catch { + return // not a parseable absolute URL → ignore + } + if (!WEB_LINK_SCHEMES.has(protocol)) return + window.open(uri, '_blank', 'noopener,noreferrer') +} + +/** POSIX absolute path check (host is macOS/Linux — see out-of-scope notes). */ +function isAbsolutePath(p: string): boolean { + return p.startsWith('/') +} + +/** + * Resolve a path emitted in terminal output to an absolute path. Absolute paths + * pass through; relative paths are joined onto `cwd` (from OSC-7) and normalized + * (`.`/`..` collapsed). Returns null when the path is relative but no cwd is + * known yet — the caller then declines to open and shows a status line. + */ +function resolvePath(cwd: string | null, rawPath: string): string | null { + if (isAbsolutePath(rawPath)) return rawPath + if (cwd === null) return null + const base = cwd.endsWith('/') ? cwd.slice(0, -1) : cwd + const segments = base.split('/') + for (const seg of rawPath.split('/')) { + if (seg === '' || seg === '.') continue + if (seg === '..') { + if (segments.length > 1) segments.pop() + continue + } + segments.push(seg) + } + const joined = segments.join('/') + return joined.startsWith('/') ? joined : `/${joined}` +} + /** Connection state, surfaced as a colored status dot on the tab. */ export type SessionStatus = 'connecting' | 'connected' | 'reconnecting' | 'exited' @@ -112,6 +160,8 @@ export class TerminalSession { private resizeTimer: ReturnType | null = null private initialInputTimer: ReturnType | null = null private exitListener: { dispose(): void } | null = null + private linkProviderDisposable: IDisposable | null = null + private openPathInFlight = false private isConnecting = false private disposed = false private lastCols = 0 @@ -145,8 +195,13 @@ export class TerminalSession { this.term.loadAddon(this.fitAddon) this.searchAddon = new SearchAddon() this.term.loadAddon(this.searchAddon) - this.term.loadAddon(new WebLinksAddon()) // M2: tap a URL Claude prints to open it + // M2/W1: tap a URL Claude prints to open it — hardened handler (scheme + // allowlist + noopener,noreferrer) instead of the addon's default activation. + this.term.loadAddon(new WebLinksAddon((_event, uri) => openWebLink(uri))) this.term.open(this.el) + // W1: clickable file paths (`src/app.ts:42`) → jump to file:line in the host + // editor. Registered after open() so the linkifier service is available. + this.linkProviderDisposable = this.term.registerLinkProvider(this.makePathLinkProvider()) this.term.onData((data) => this.send(data)) // Tab title = current folder, derived from the shell's OSC title. @@ -378,6 +433,69 @@ export class TerminalSession { this.sendMsg({ type: 'input', data }) } + /** + * W1: a link provider that turns file paths in terminal output into clickable + * links. xterm passes a 1-based buffer row; getLine wants a 0-based index, and + * the range's y is the same 1-based row. + */ + private makePathLinkProvider(): ILinkProvider { + return { + provideLinks: (bufferLineNumber, callback) => { + const line = this.term.buffer.active.getLine(bufferLineNumber - 1) + if (!line) { + callback(undefined) + return + } + const matches = findPathMatches(line.translateToString(true)) + if (matches.length === 0) { + callback(undefined) + return + } + const links: ILink[] = matches.map((match) => ({ + text: match.text, + range: { + start: { x: match.startX, y: bufferLineNumber }, + end: { x: match.endX, y: bufferLineNumber }, + }, + activate: () => this.openPath(match), + })) + callback(links) + }, + } + } + + /** + * Open a clicked file path in the host editor (W1). Relative paths are resolved + * against the OSC-7 cwd; if no cwd is known the click is declined with a status + * line. An in-flight guard stops rapid clicks from spawning many editor + * processes. Failures are shown as a non-blocking status line, never thrown. + */ + private openPath(match: PathMatch): void { + const abs = resolvePath(this.cwdValue, match.path) + if (abs === null) { + this.term.write(statusLine(`cannot open ${match.path}: working dir unknown`)) + return + } + if (this.openPathInFlight) return + this.openPathInFlight = true + const body: { file: string; line?: number } = + match.line !== undefined ? { file: abs, line: match.line } : { file: abs } + fetch('/open-in-editor', { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify(body), + }) + .then((res) => { + if (!res.ok) this.term.write(statusLine(`cannot open ${match.path} (${res.status})`)) + }) + .catch(() => { + this.term.write(statusLine(`cannot open ${match.path}: request failed`)) + }) + .finally(() => { + this.openPathInFlight = false + }) + } + /** Resolve a held PermissionRequest (H3). Optionally relay a permission-mode * change (B4): mode is included only when explicitly provided so the server * can distinguish "approve with default" from "approve, keep existing mode". */ @@ -464,6 +582,10 @@ export class TerminalSession { this.initialInputTimer = null } this.resizeObserver.disconnect() + if (this.linkProviderDisposable !== null) { + this.linkProviderDisposable.dispose() + this.linkProviderDisposable = null + } if (this.ws !== null) { try { this.ws.close() diff --git a/src/http/editor.ts b/src/http/editor.ts index 0c98a1a..dd7b6bf 100644 --- a/src/http/editor.ts +++ b/src/http/editor.ts @@ -61,3 +61,86 @@ export async function openInEditor(cfg: Config, rawPath: unknown): Promise:` to jump to a line. */ +const GOTO_EDITORS: ReadonlySet = new Set([ + 'code', 'code-insiders', 'codium', 'vscodium', 'cursor', 'windsurf', +]) + +const MIN_LINE = 1 +const MAX_LINE = 1_000_000 + +/** + * True iff `editorCmd`'s basename is an editor that understands `--goto file:line`. + * Unknown editors get the bare file (never a stray `--goto` argv they'd misread + * as a filename). A trailing .cmd/.exe/.bat (Windows shim) is stripped first. + */ +export function isGotoEditor(editorCmd: string): boolean { + const base = path.basename(editorCmd).toLowerCase().replace(/\.(cmd|exe|bat)$/, '') + return GOTO_EDITORS.has(base) +} + +/** + * Validate `rawFile` (absolute, existing regular file) and optional `rawLine` + * (integer 1..1_000_000), then launch the editor at that file:line. Never throws + * — returns a structured result the route maps to an HTTP response. The line is + * validated to an integer before interpolation into `${file}:${line}`, so it can + * never smuggle shell metacharacters (and there's no shell anyway — execFile argv). + */ +export async function openFileInEditor( + cfg: Config, + rawFile: unknown, + rawLine?: unknown, +): Promise { + if (typeof rawFile !== 'string' || rawFile.trim() === '') { + return { ok: false, status: 400, error: 'file is required' } + } + if (!path.isAbsolute(rawFile)) { + return { ok: false, status: 400, error: 'file must be absolute' } + } + + let line: number | undefined + if (rawLine !== undefined && rawLine !== null) { + if ( + typeof rawLine !== 'number' || + !Number.isInteger(rawLine) || + rawLine < MIN_LINE || + rawLine > MAX_LINE + ) { + return { ok: false, status: 400, error: `line must be an integer ${MIN_LINE}..${MAX_LINE}` } + } + line = rawLine + } + + let stat + try { + stat = await fs.stat(rawFile) + } catch { + return { ok: false, status: 404, error: 'file not found' } + } + if (!stat.isFile()) { + return { ok: false, status: 400, error: 'path is not a file' } + } + + const args = + isGotoEditor(cfg.editorCmd) && line !== undefined + ? ['--goto', `${rawFile}:${line}`] + : [rawFile] + + try { + const child = execFile(cfg.editorCmd, args, { windowsHide: true }) + child.on('error', (err) => { + console.error('[editor] failed to launch', JSON.stringify(cfg.editorCmd), '-', err.message) + }) + child.unref() + return { ok: true, status: 204 } + } catch (err) { + return { ok: false, status: 500, error: err instanceof Error ? err.message : String(err) } + } +} diff --git a/src/server.ts b/src/server.ts index 8bfc84c..3fef1b1 100644 --- a/src/server.ts +++ b/src/server.ts @@ -37,7 +37,7 @@ import { isOriginAllowed } from './http/origin.js' import { parseHookEvent } from './http/hook.js' import { listSessions } from './http/history.js' import { buildProjects, buildProjectDetail } from './http/projects.js' -import { openInEditor } from './http/editor.js' +import { openInEditor, openFileInEditor } from './http/editor.js' import { getDiff } from './http/diff.js' import { parseStatusLine } from './http/statusline.js' import { createWorktree } from './http/worktrees.js' @@ -380,11 +380,18 @@ export function startServer(cfg: Config): { close(): Promise } { // Open a project in the host's desktop editor (v0.6 Projects panel — VS Code // logo). State-changing (spawns a GUI process), so it carries the same Origin // guard as the DELETE routes. The path is validated + passed via execFile (no - // shell) in openInEditor. + // shell) in openInEditor / openFileInEditor. + // + // W1: two modes on one route. `file` present ⇒ open that FILE at `line` (a + // clicked terminal path like `src/app.ts:42`); else the original directory mode + // (`path`). Discriminated by body shape so the Projects panel is unchanged. app.post('/open-in-editor', express.json({ limit: '4kb' }), async (req, res) => { if (!requireAllowedOrigin(req, res)) return const body = (req.body ?? {}) as Record - const result = await openInEditor(cfg, body['path']) + const result = + body['file'] !== undefined + ? await openFileInEditor(cfg, body['file'], body['line']) + : await openInEditor(cfg, body['path']) if (result.ok) { res.status(204).end() return diff --git a/test/editor.test.ts b/test/editor.test.ts index a96da7c..7e25391 100644 --- a/test/editor.test.ts +++ b/test/editor.test.ts @@ -9,7 +9,7 @@ import { describe, it, expect, beforeAll, afterAll } from 'vitest' import fs from 'node:fs/promises' import os from 'node:os' import path from 'node:path' -import { openInEditor } from '../src/http/editor.js' +import { openInEditor, openFileInEditor, isGotoEditor } from '../src/http/editor.js' import type { Config } from '../src/types.js' function cfg(editorCmd: string): Config { @@ -66,3 +66,104 @@ describe('openInEditor — launch', () => { expect(r.status).toBe(204) }) }) + +// ── W1: openFileInEditor (open a FILE at a line) ────────────────────────────── + +/** Poll for a file to exist + be non-empty (the editor spawn is fire-and-forget). */ +async function waitForFile(p: string, timeoutMs = 3_000): Promise { + const start = Date.now() + for (;;) { + try { + const content = await fs.readFile(p, 'utf8') + if (content.length > 0) return content + } catch { + // not written yet + } + if (Date.now() - start > timeoutMs) throw new Error(`timeout waiting for ${p}`) + await new Promise((r) => setTimeout(r, 25)) + } +} + +describe('isGotoEditor', () => { + it('is true for the VS Code family (code/cursor/windsurf/codium)', () => { + expect(isGotoEditor('code')).toBe(true) + expect(isGotoEditor('/usr/local/bin/cursor')).toBe(true) + expect(isGotoEditor('windsurf')).toBe(true) + expect(isGotoEditor('codium')).toBe(true) + expect(isGotoEditor('code.cmd')).toBe(true) // Windows shim + }) + + it('is false for editors without --goto (vim/nano/true)', () => { + expect(isGotoEditor('vim')).toBe(false) + expect(isGotoEditor('nano')).toBe(false) + expect(isGotoEditor('true')).toBe(false) + }) +}) + +describe('openFileInEditor — validation', () => { + it('rejects a missing / empty / non-string file with 400', async () => { + expect((await openFileInEditor(cfg('true'), undefined)).status).toBe(400) + expect((await openFileInEditor(cfg('true'), '')).status).toBe(400) + expect((await openFileInEditor(cfg('true'), ' ')).status).toBe(400) + expect((await openFileInEditor(cfg('true'), 123)).status).toBe(400) + }) + + it('rejects a relative file with 400', async () => { + expect((await openFileInEditor(cfg('true'), 'rel/a.ts')).status).toBe(400) + }) + + it('returns 404 for a non-existent absolute file', async () => { + const r = await openFileInEditor(cfg('true'), path.join(tmpDir, 'nope.ts')) + expect(r.status).toBe(404) + }) + + it('returns 400 when the path is a directory, not a file', async () => { + const r = await openFileInEditor(cfg('true'), tmpDir) + expect(r.status).toBe(400) + expect(r.error).toContain('not a file') + }) + + it('rejects a non-integer / zero / out-of-range / non-number line with 400', async () => { + expect((await openFileInEditor(cfg('true'), tmpFile, 1.5)).status).toBe(400) + expect((await openFileInEditor(cfg('true'), tmpFile, 0)).status).toBe(400) + expect((await openFileInEditor(cfg('true'), tmpFile, 1_000_000_001)).status).toBe(400) + expect((await openFileInEditor(cfg('true'), tmpFile, 'x')).status).toBe(400) + }) +}) + +describe('openFileInEditor — launch', () => { + it('spawns the editor for a valid file with no line (204)', async () => { + const r = await openFileInEditor(cfg('true'), tmpFile) + expect(r.ok).toBe(true) + expect(r.status).toBe(204) + }) + + it('spawns the editor for a valid file + line (204)', async () => { + const r = await openFileInEditor(cfg('true'), tmpFile, 42) + expect(r.status).toBe(204) + }) + + it('passes `--goto :` for a goto-capable editor (basename code)', async () => { + const argsOut = path.join(tmpDir, 'goto-args.txt') + const recorder = path.join(tmpDir, 'code') // basename 'code' → isGotoEditor true + await fs.writeFile(recorder, `#!/bin/sh\nprintf '%s\\n' "$@" > "${argsOut}"\n`) + await fs.chmod(recorder, 0o755) + + const r = await openFileInEditor(cfg(recorder), tmpFile, 42) + expect(r.status).toBe(204) + const recorded = (await waitForFile(argsOut)).trim().split('\n') + expect(recorded).toEqual(['--goto', `${tmpFile}:42`]) + }) + + it('passes only the bare file for a non-goto editor (drops the line)', async () => { + const argsOut = path.join(tmpDir, 'bare-args.txt') + const recorder = path.join(tmpDir, 'myeditor') // basename not in the goto set + await fs.writeFile(recorder, `#!/bin/sh\nprintf '%s\\n' "$@" > "${argsOut}"\n`) + await fs.chmod(recorder, 0o755) + + const r = await openFileInEditor(cfg(recorder), tmpFile, 42) + expect(r.status).toBe(204) + const recorded = (await waitForFile(argsOut)).trim().split('\n') + expect(recorded).toEqual([tmpFile]) + }) +}) diff --git a/test/integration/server.test.ts b/test/integration/server.test.ts index 4162278..85e11a1 100644 --- a/test/integration/server.test.ts +++ b/test/integration/server.test.ts @@ -25,6 +25,9 @@ */ import net from 'node:net' +import fs from 'node:fs/promises' +import os from 'node:os' +import path from 'node:path' import { execFileSync } from 'node:child_process' import { afterEach, beforeEach, describe, expect, it } from 'vitest' @@ -103,6 +106,9 @@ function makeTestConfig(port: number, shellPath: string, maxPayloadBytes = 512 * IDLE_TTL: '86400', MAX_PAYLOAD_BYTES: String(maxPayloadBytes), USE_TMUX: '0', // default off so most cases don't spawn/leak tmux sessions + // W1: /open-in-editor spawns EDITOR_CMD. Use the POSIX no-op so tests never + // pop a real editor (and it's a non-goto basename → bare-file argv). + EDITOR_CMD: 'true', }) } @@ -403,6 +409,58 @@ describe('startServer — integration', () => { } }, 15_000) + // ── W1: POST /open-in-editor file mode + CSRF guard ──────────────────────── + // EDITOR_CMD is 'true' (see makeTestConfig) so no real editor is launched. + // These don't need a PTY — they exercise the plain HTTP route + Origin guard. + + it('W1: opens a FILE at a line with a valid Origin (204)', async () => { + const dir = await fs.mkdtemp(path.join(os.tmpdir(), 'oie-file-')) + const file = path.join(dir, 'x.ts') + await fs.writeFile(file, 'hi') + try { + const res = await fetch(`http://127.0.0.1:${port}/open-in-editor`, { + method: 'POST', + headers: { 'content-type': 'application/json', Origin: `http://127.0.0.1:${port}` }, + body: JSON.stringify({ file, line: 3 }), + }) + expect(res.status).toBe(204) + } finally { + await fs.rm(dir, { recursive: true, force: true }) + } + }) + + it('W1: rejects file mode from a foreign Origin (403 — CSRF guard covers the new branch)', async () => { + const res = await fetch(`http://127.0.0.1:${port}/open-in-editor`, { + method: 'POST', + headers: { 'content-type': 'application/json', Origin: 'http://evil.com' }, + body: JSON.stringify({ file: '/tmp/x.ts', line: 1 }), + }) + expect(res.status).toBe(403) + }) + + it('W1: still supports directory mode (regression — Projects panel intact)', async () => { + const dir = await fs.mkdtemp(path.join(os.tmpdir(), 'oie-dir-')) + try { + const res = await fetch(`http://127.0.0.1:${port}/open-in-editor`, { + method: 'POST', + headers: { 'content-type': 'application/json', Origin: `http://127.0.0.1:${port}` }, + body: JSON.stringify({ path: dir }), + }) + expect(res.status).toBe(204) + } finally { + await fs.rm(dir, { recursive: true, force: true }) + } + }) + + it('W1: 404 for a file that does not exist (validated server-side)', async () => { + const res = await fetch(`http://127.0.0.1:${port}/open-in-editor`, { + method: 'POST', + headers: { 'content-type': 'application/json', Origin: `http://127.0.0.1:${port}` }, + body: JSON.stringify({ file: '/no/such/file/here.ts', line: 1 }), + }) + expect(res.status).toBe(404) + }) + // ── ⑤ attach → attached → output (needs real PTY — sandbox-off) ────────── // // NOTE (sandbox-off): This test spawns a real shell process via node-pty. diff --git a/test/link-paths.test.ts b/test/link-paths.test.ts new file mode 100644 index 0000000..6320147 --- /dev/null +++ b/test/link-paths.test.ts @@ -0,0 +1,123 @@ +/** + * test/link-paths.test.ts — pure findPathMatches matcher (W1). + * + * Node environment (no DOM): the matcher is deliberately DOM-free so it can be + * unit-tested directly and reused by later diff/preview views. + */ + +import { describe, it, expect } from 'vitest' +import { findPathMatches, CODE_EXT } from '../public/link-paths.js' + +describe('findPathMatches — file:line paths', () => { + it('matches a src/app.ts:42 path with correct 1-based range', () => { + const line = 'see src/app.ts:42 for' + const matches = findPathMatches(line) + expect(matches).toHaveLength(1) + const m = matches[0]! + expect(m.text).toBe('src/app.ts:42') + expect(m.path).toBe('src/app.ts') + expect(m.line).toBe(42) + expect(m.column).toBeUndefined() + // 'see ' = 4 chars → startX = 5 (1-based), endX inclusive = 5 + 13 - 1 = 17. + expect(m.startX).toBe(5) + expect(m.endX).toBe(17) + expect(line.slice(m.startX - 1, m.endX)).toBe('src/app.ts:42') + }) + + it('parses :line:col into line + column', () => { + const matches = findPathMatches('./a/b.tsx:10:5') + expect(matches).toHaveLength(1) + const m = matches[0]! + expect(m.path).toBe('./a/b.tsx') + expect(m.line).toBe(10) + expect(m.column).toBe(5) + }) + + it('matches a bare filename with an allowlisted extension (README.md)', () => { + const matches = findPathMatches('open README.md please') + expect(matches).toHaveLength(1) + expect(matches[0]!.path).toBe('README.md') + expect(matches[0]!.line).toBeUndefined() + }) + + it('matches main.rs:10 via its :line suffix', () => { + const matches = findPathMatches('at main.rs:10') + expect(matches).toHaveLength(1) + expect(matches[0]!.path).toBe('main.rs') + expect(matches[0]!.line).toBe(10) + }) + + it('matches an absolute path with a leading slash', () => { + const matches = findPathMatches('Error at /home/u/app.ts:42 now') + expect(matches).toHaveLength(1) + const m = matches[0]! + expect(m.path).toBe('/home/u/app.ts') + expect(m.line).toBe(42) + }) +}) + +describe('findPathMatches — non-paths are rejected', () => { + it('does not match a bare domain (example.com)', () => { + expect(findPathMatches('visit example.com today')).toHaveLength(0) + }) + + it('does not match a version string (v1.2.3)', () => { + expect(findPathMatches('version v1.2.3 released')).toHaveLength(0) + }) + + it('does not match a foo.bar token (unknown extension, no slash/line)', () => { + expect(findPathMatches('the foo.bar thing')).toHaveLength(0) + }) + + it('does not match a bare timestamp (12:34)', () => { + expect(findPathMatches('at 12:34 today')).toHaveLength(0) + }) +}) + +describe('findPathMatches — URL guard', () => { + it('does not double-link the path tail of an http URL (WebLinksAddon owns it)', () => { + expect(findPathMatches('https://host/path.html')).toHaveLength(0) + }) + + it('does not match a path glued to a URL host (a.ts inside http://x/a.ts)', () => { + expect(findPathMatches('http://example.com/src/a.ts')).toHaveLength(0) + }) +}) + +describe('findPathMatches — multiple + relative paths', () => { + it('returns two disjoint matches for two paths on one line', () => { + const matches = findPathMatches('edit src/a.ts:1 and lib/b.rs:2') + expect(matches).toHaveLength(2) + expect(matches[0]!.path).toBe('src/a.ts') + expect(matches[1]!.path).toBe('lib/b.rs') + // Ranges are disjoint and ordered. + expect(matches[0]!.endX).toBeLessThan(matches[1]!.startX) + }) + + it('keeps a ../ prefix in the path portion', () => { + const matches = findPathMatches('see ../lib/util.ts:7') + expect(matches).toHaveLength(1) + expect(matches[0]!.path).toBe('../lib/util.ts') + expect(matches[0]!.line).toBe(7) + }) + + it('matches a nested config.test.ts filename (multi-dot) with the final extension', () => { + const matches = findPathMatches('run test/config.test.ts') + expect(matches).toHaveLength(1) + expect(matches[0]!.path).toBe('test/config.test.ts') + }) + + it('returns [] for a line with no paths', () => { + expect(findPathMatches('just some plain words here')).toEqual([]) + }) +}) + +describe('CODE_EXT allowlist', () => { + it('contains common code extensions and not arbitrary ones', () => { + expect(CODE_EXT.has('ts')).toBe(true) + expect(CODE_EXT.has('md')).toBe(true) + expect(CODE_EXT.has('rs')).toBe(true) + expect(CODE_EXT.has('com')).toBe(false) + expect(CODE_EXT.has('bar')).toBe(false) + }) +}) diff --git a/test/terminal-session.test.ts b/test/terminal-session.test.ts index 63148b7..d72afe3 100644 --- a/test/terminal-session.test.ts +++ b/test/terminal-session.test.ts @@ -10,12 +10,47 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' +// Captures the handler passed to WebLinksAddon so a test can assert the hardened +// activation handler is wired (W1). vi.hoisted so the mock factory can see it. +const webLinksState = vi.hoisted( + () => ({ handler: undefined as ((e: MouseEvent, uri: string) => void) | undefined }), +) + +// A link registered via the (mocked) provider — the shape terminal-session builds. +interface TestLink { + text: string + range: { start: { x: number; y: number }; end: { x: number; y: number } } + activate: (event: MouseEvent, text: string) => void +} +interface TestLinkProvider { + provideLinks: (bufferLineNumber: number, callback: (links: TestLink[] | undefined) => void) => void +} + // ── Stub xterm + addons (heavy DOM/canvas deps we don't need here) ──────────── class FakeTerminal { options: Record = {} cols = 80 rows = 24 - parser = { registerOscHandler: vi.fn() } + /** Text the fake buffer returns for any getLine() (drives the link provider). */ + lineText = '' + /** The link provider registered by TerminalSession (W1). */ + captured: TestLinkProvider | undefined = undefined + /** OSC handlers registered by TerminalSession, keyed by code (7 = cwd). */ + oscHandlers: Record boolean> = {} + parser = { + registerOscHandler: vi.fn((code: number, handler: (data: string) => boolean) => { + this.oscHandlers[code] = handler + }), + } + buffer = { + active: { + getLine: (_y: number) => ({ translateToString: (_trim?: boolean) => this.lineText }), + }, + } + registerLinkProvider = vi.fn((p: TestLinkProvider) => { + this.captured = p + return { dispose: vi.fn() } + }) loadAddon = vi.fn() open = vi.fn() write = vi.fn() @@ -45,7 +80,13 @@ vi.mock('@xterm/addon-search', () => ({ clearDecorations = vi.fn() }, })) -vi.mock('@xterm/addon-web-links', () => ({ WebLinksAddon: class {} })) +vi.mock('@xterm/addon-web-links', () => ({ + WebLinksAddon: class { + constructor(handler?: (e: MouseEvent, uri: string) => void) { + webLinksState.handler = handler + } + }, +})) // ── Mock WebSocket ──────────────────────────────────────────────────────────── class MockWebSocket { @@ -109,7 +150,20 @@ afterEach(() => { vi.useRealTimers() }) -const { TerminalSession } = await import('../public/terminal-session.js') +const { TerminalSession, openWebLink } = await import('../public/terminal-session.js') + +function term(s: unknown): FakeTerminal { + return (s as unknown as { term: FakeTerminal }).term +} + +/** Drive the captured link provider for one buffer row, returning its links. */ +function provideLinks(t: FakeTerminal, bufferLineNumber = 1): TestLink[] | undefined { + let result: TestLink[] | undefined + t.captured!.provideLinks(bufferLineNumber, (links) => { + result = links + }) + return result +} function setLocation(protocol: string, host: string): void { vi.stubGlobal('location', { protocol, host }) @@ -611,3 +665,168 @@ describe('approve(mode?) — B4 permission-mode relay', () => { expect(s.pendingApproval).toBe(false) }) }) + +// ── W1 clickable file paths (custom link provider) ──────────────────────────── + +describe('clickable file paths (W1)', () => { + beforeEach(() => setLocation('http:', 'lan:3000')) + + it('registers a link provider on construction', () => { + const s = new TerminalSession({ sessionId: null, onSessionId: vi.fn() }) + expect(term(s).registerLinkProvider).toHaveBeenCalledOnce() + expect(term(s).captured).toBeTruthy() + }) + + it('provideLinks yields one link matching findPathMatches (text + 1-based range)', () => { + const s = new TerminalSession({ sessionId: null, onSessionId: vi.fn() }) + const t = term(s) + t.lineText = 'see src/app.ts:42 for' + const links = provideLinks(t, 1)! + expect(links).toHaveLength(1) + expect(links[0]!.text).toBe('src/app.ts:42') + expect(links[0]!.range).toEqual({ start: { x: 5, y: 1 }, end: { x: 17, y: 1 } }) + }) + + it('provideLinks returns undefined for a line with no paths', () => { + const s = new TerminalSession({ sessionId: null, onSessionId: vi.fn() }) + const t = term(s) + t.lineText = 'just some plain words' + expect(provideLinks(t, 1)).toBeUndefined() + }) + + it('provideLinks returns undefined when the buffer row is missing', () => { + const s = new TerminalSession({ sessionId: null, onSessionId: vi.fn() }) + const t = term(s) + t.buffer.active.getLine = () => undefined as never + expect(provideLinks(t, 999)).toBeUndefined() + }) + + it('activate() POSTs {file,line} for an absolute path (no cwd needed)', () => { + const fetchMock = vi.fn(async (_u: string, _o: RequestInit) => ({ ok: true })) + vi.stubGlobal('fetch', fetchMock) + const s = new TerminalSession({ sessionId: null, onSessionId: vi.fn() }) + const t = term(s) + t.lineText = 'open /home/u/app.ts:42 now' + const links = provideLinks(t, 1)! + links[0]!.activate({} as MouseEvent, links[0]!.text) + expect(fetchMock).toHaveBeenCalledOnce() + const call = fetchMock.mock.calls[0]! + expect(call[0]).toBe('/open-in-editor') + expect(call[1].method).toBe('POST') + expect(JSON.parse(String(call[1].body))).toEqual({ file: '/home/u/app.ts', line: 42 }) + }) + + it('resolves a relative path against the OSC-7 cwd', () => { + const fetchMock = vi.fn(async (_u: string, _o: RequestInit) => ({ ok: true })) + vi.stubGlobal('fetch', fetchMock) + const s = new TerminalSession({ sessionId: null, onSessionId: vi.fn() }) + const t = term(s) + t.oscHandlers[7]!('file:///work/here') // set cwd via the real OSC-7 handler + t.lineText = 'edit src/app.ts:42' + const links = provideLinks(t, 1)! + links[0]!.activate({} as MouseEvent, links[0]!.text) + expect(JSON.parse(String(fetchMock.mock.calls[0]![1].body))).toEqual({ + file: '/work/here/src/app.ts', + line: 42, + }) + }) + + it('collapses ../ against the cwd', () => { + const fetchMock = vi.fn(async (_u: string, _o: RequestInit) => ({ ok: true })) + vi.stubGlobal('fetch', fetchMock) + const s = new TerminalSession({ sessionId: null, onSessionId: vi.fn() }) + const t = term(s) + t.oscHandlers[7]!('file:///work/here') + t.lineText = 'see ../lib/util.ts:7' + const links = provideLinks(t, 1)! + links[0]!.activate({} as MouseEvent, links[0]!.text) + expect(JSON.parse(String(fetchMock.mock.calls[0]![1].body))).toEqual({ + file: '/work/lib/util.ts', + line: 7, + }) + }) + + it('relative path with null cwd → no fetch, writes a status line', () => { + const fetchMock = vi.fn(async () => ({ ok: true })) + vi.stubGlobal('fetch', fetchMock) + const s = new TerminalSession({ sessionId: null, onSessionId: vi.fn() }) + const t = term(s) + t.lineText = 'edit src/app.ts:42' + const links = provideLinks(t, 1)! + t.write.mockClear() + links[0]!.activate({} as MouseEvent, links[0]!.text) + expect(fetchMock).not.toHaveBeenCalled() + expect(t.write).toHaveBeenCalled() + expect(String(t.write.mock.calls[0]![0])).toContain('working dir unknown') + }) + + it('in-flight guard: two rapid activate calls → one fetch', () => { + const fetchMock = vi.fn(() => new Promise(() => {})) // never resolves + vi.stubGlobal('fetch', fetchMock) + const s = new TerminalSession({ sessionId: null, onSessionId: vi.fn() }) + const t = term(s) + t.lineText = 'open /a/b.ts:1' + const links = provideLinks(t, 1)! + links[0]!.activate({} as MouseEvent, links[0]!.text) + links[0]!.activate({} as MouseEvent, links[0]!.text) + expect(fetchMock).toHaveBeenCalledOnce() + }) + + it('writes a status line when the server rejects the open (non-ok)', async () => { + const fetchMock = vi.fn(async () => ({ ok: false, status: 404 })) + vi.stubGlobal('fetch', fetchMock) + const s = new TerminalSession({ sessionId: null, onSessionId: vi.fn() }) + const t = term(s) + t.lineText = 'open /home/u/app.ts:42' + const links = provideLinks(t, 1)! + t.write.mockClear() + links[0]!.activate({} as MouseEvent, links[0]!.text) + await new Promise((r) => setTimeout(r, 0)) // flush the fetch().then() chain + expect(t.write).toHaveBeenCalled() + expect(String(t.write.mock.calls[0]![0])).toContain('404') + }) + + it('disposes the link provider on dispose()', () => { + const s = new TerminalSession({ sessionId: null, onSessionId: vi.fn() }) + const disposable = term(s).registerLinkProvider.mock.results[0]!.value as { + dispose: ReturnType + } + s.dispose() + expect(disposable.dispose).toHaveBeenCalled() + }) +}) + +// ── W1 URL activation hardening ─────────────────────────────────────────────── + +describe('openWebLink (W1 URL hardening)', () => { + it('passes a handler function to WebLinksAddon (not the default activation)', () => { + new TerminalSession({ sessionId: null, onSessionId: vi.fn() }) + expect(typeof webLinksState.handler).toBe('function') + }) + + it('opens http/https/mailto with noopener,noreferrer', () => { + const openSpy = vi.fn() + vi.stubGlobal('open', openSpy) + openWebLink('https://example.com/x') + expect(openSpy).toHaveBeenCalledWith('https://example.com/x', '_blank', 'noopener,noreferrer') + openWebLink('http://lan:3000') + openWebLink('mailto:a@b.com') + expect(openSpy).toHaveBeenCalledTimes(3) + }) + + it('blocks javascript: / data: / file: URIs (no window.open)', () => { + const openSpy = vi.fn() + vi.stubGlobal('open', openSpy) + openWebLink('javascript:alert(1)') + openWebLink('data:text/html,') + openWebLink('file:///etc/passwd') + expect(openSpy).not.toHaveBeenCalled() + }) + + it('ignores an unparseable URI without throwing', () => { + const openSpy = vi.fn() + vi.stubGlobal('open', openSpy) + expect(() => openWebLink('not a url')).not.toThrow() + expect(openSpy).not.toHaveBeenCalled() + }) +}) diff --git a/vitest.config.ts b/vitest.config.ts index f55beaf..37d23f8 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -23,6 +23,7 @@ export default defineConfig({ include: [ 'src/**/*.ts', 'public/terminal-session.ts', + 'public/link-paths.ts', 'public/tabs.ts', 'public/grid-layout.ts', 'public/grid-presets.ts',