kai/web-terminal
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
Files
675de771c70803f1ad1047687a3a21e73e4bc9da
web-terminal/docs/plans/w1-clickable-links.md

14 KiB
Raw Blame History

Clickable URLs & file paths in the terminal

Status of premise (read first). Two independent capabilities are bundled here:

  1. URLs — already 90% shipped. public/terminal-session.ts:148 already does this.term.loadAddon(new WebLinksAddon()), and @xterm/addon-web-links@^0.12.0 is already in package.json. Remaining work is a security hardening pass on link activation (scheme allowlist + noopener). Pure frontend.

  2. File paths → editor — needs a custom link provider and a small, additive server change. The task brief says "POST to the existing /open-in-editor … no server change," but the existing route cannot open a file at a line: openInEditor (src/http/editor.ts:31) rejects anything that is not an absolute existing directory (lines 35, 45–47) and spawns code <dir> with no --goto/line (line 51). Feeding it src/app.ts:42 fails three validators at once. See the Decision callout below — I recommend a tiny additive openFileInEditor alongside the untouched openInEditor. A strict "frontend-only" fallback exists but degrades to "open the containing folder, no line jump."

src/types.ts is not touched — this is an HTTP JSON body, not a WS protocol message; no shared-contract change. The byte-shuttle terminal stream is untouched.

Decision: how file-path clicks reach the editor

Option What clicking src/app.ts:42 does Server change Recommendation
A — additive openFileInEditor (recommended) Opens the file at line 42 (code --goto /abs/src/app.ts:42) +~35 lines in src/http/editor.ts, +1 branch in the server.ts:384 route. Backward-compatible; openInEditor + its tests untouched Yes. Only option that delivers the actual feature (jump to file:line). Additive and low-risk.
B — strict frontend-only Resolves to the file's parent dir and calls existing openInEditor → opens the repo/folder, no file, no line None Fallback only. Poor UX; loses the whole point (line jump). Document but don't ship as primary.

The rest of this plan assumes Option A. The frontend work is identical either way; only the POST body and server branch differ.

Contract

Route (extended, backward-compatible)

POST /open-in-editor (src/server.ts:384) — unchanged guards: express.json({ limit: '4kb' }), requireAllowedOrigin (CSRF, src/server.ts:352). New branch on body shape:

  • Existing (directory) — { "path": "<abs dir>" } → openInEditor(cfg, body.path) (unchanged; Projects panel keeps working).
  • New (file+line) — { "file": "<abs file>", "line": <int?>, "column": <int?> } → openFileInEditor(cfg, body.file, body.line).
  • Response envelope unchanged: 204 on success; { error } + 4xx/5xx on failure (mirrors src/server.ts:388–392).

Body is a discriminated request: file present ⇒ file mode; else path mode. If neither present → 400 { error: 'path or file is required' }.

New server function (src/http/editor.ts)

export async function openFileInEditor(
  cfg: Config, rawFile: unknown, rawLine?: unknown
): Promise<OpenEditorResult>

Reuses the existing OpenEditorResult type (editor.ts:20). Validates: string + non-empty (400), path.isAbsolute (400), fs.stat exists (404), stat.isFile() (400 'path is not a file'), and line (when present) is an integer in 1..1_000_000 else 400. Spawns via execFile(cfg.editorCmd, args) (no shell, same as line 51) where:

  • args = isGotoEditor(cfg.editorCmd) && line !== undefined ? ['--goto', ${file}:${line}] : [file]
  • isGotoEditor = basename ∈ {code, code-insiders, codium, cursor, windsurf} (the editors that accept --goto). Unknown editors open the bare file (never pass a bogus --goto argv).

New frontend module (public/link-paths.ts) — pure, node-testable

export interface PathMatch {
  text: string        // exact matched substring (e.g. "src/app.ts:42")
  path: string        // "src/app.ts"
  line?: number
  column?: number
  startX: number      // 1-based column of first char (xterm range.start.x)
  endX: number        // 1-based column of last char  (xterm range.end.x, inclusive)
}
export function findPathMatches(lineText: string): PathMatch[]

Matching rules (concrete): a token is a path candidate iff it has a filename with a dot-extension, optionally preceded by ./, ../, or dir/…/ segments, optionally suffixed :line and :line:col. A candidate becomes a match iff (has a / separator) OR (has a :line suffix) OR (extension ∈ CODE_EXT allowlist) — this links src/app.ts:42, README.md, main.rs:10 while rejecting example.com, v1.2, foo.bar. Reject candidates immediately preceded by / or : (avoids grabbing the tail of a https://host/path.html URL, which WebLinksAddon owns). CODE_EXT = a named const 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 …).

Env vars

None new. EDITOR_CMD (default 'code') already exists (src/config.ts:49, src/types.ts:42) and is reused.

WS protocol / src/types.ts

No change. No new client→server or server→client message types.

Files to change

Path Change
public/link-paths.ts New. Pure findPathMatches + CODE_EXT const + PathMatch type. No DOM. (Keeps matcher unit-testable in node and file <150 lines.)
public/terminal-session.ts Replace bare new WebLinksAddon() at line 148 with a hardened handler (scheme allowlist + window.open(uri,'_blank','noopener,noreferrer')). After term.open (line 149) register a path link provider via this.term.registerLinkProvider(...); store the returned IDisposable. Add private openPath(m: PathMatch) (resolve rel→abs via this.cwdValue, in-flight guard, fetch('/open-in-editor', …), error→statusLine toast). Dispose the provider in dispose() (line 452). Small helpers openWebLink, makePathLinkProvider.
src/http/editor.ts Add openFileInEditor + isGotoEditor helper. openInEditor unchanged (Projects panel + its tests keep passing).
src/server.ts In the /open-in-editor handler (line 384–393) branch: body.file present → openFileInEditor(cfg, body.file, body.line); else existing openInEditor(cfg, body.path).
src/types.ts Not touched — noted here only to confirm no shared-contract edit is required.

TDD steps (ordered)

Run with npm test (vitest). New frontend-logic tests are node (pure matcher); DOM-wiring tests reuse the existing jsdom harness in test/terminal-session.test.ts.

  1. test/link-paths.test.ts (new, node). RED→GREEN for findPathMatches:

    • 'see src/app.ts:42 for' → one match, path:'src/app.ts', line:42, startX/endX correct (1-based, startX = index+1, endX = index+len).
    • './a/b.tsx:10:5' → line:10, column:5.
    • 'README.md' (allowlisted ext, no slash) → matched; 'example.com' and 'v1.2.3' → no match.
    • URL guard: 'https://host/path.html' → no path match (preceded-by-/ rule).
    • Two paths on one line → two matches with disjoint ranges. Then implement public/link-paths.ts to green.

  2. test/editor.test.ts (extend, node — mirror existing spawn-a-harmless-process style, editorCmd:'true'). Add a describe('openFileInEditor'):

    • relative → 400; missing → 404; directory → 400 'is not a file'; non-int/0/1e9+ line → 400.
    • success on a real temp file (editorCmd 'true') → status 204.
    • argv assertion: write a tiny recorder script into the temp dir (#!/bin/sh; printf '%s\n' "$@" > "$ARGS_OUT"), set editorCmd to it, call with line:42, assert the recorded argv is --goto, <file>:42; call an unknown editor name → argv is just <file> (no --goto). Then implement openFileInEditor + isGotoEditor to green.

  3. test/integration/server.test.ts (extend; harness at line 229). Boot the app, POST /open-in-editor:

    • {file:<abs tmp file>, line:3} with a valid Origin → 204.
    • foreign/missing Origin → 403 (proves the CSRF guard still covers the new branch).
    • {path:<abs tmp dir>} still 204 (regression: directory mode intact). Wire the server.ts branch to green.

  4. test/terminal-session.test.ts (extend, jsdom). Extend FakeTerminal (line 14) with registerLinkProvider = vi.fn(p => { this.captured = p; return {dispose:vi.fn()} }) and a buffer = { active: { getLine: (y)=>({ translateToString:()=> this.lineText }) } }. Keep the WebLinksAddon mock (line 48) but assert the constructor received a handler fn. Tests:

    • after construct, a link provider is registered; feeding a line with src/app.ts:42 → provideLinks callback yields one ILink whose text/range match findPathMatches.
    • calling link.activate(mouseEvent, text) when cwd is set (drive an OSC-7 via the captured handler, or set via a resolved path) → fetch (stub via vi.stubGlobal('fetch', …) as in test/preview-grid.test.ts:140) called once with '/open-in-editor', method POST, body {file:<abs>, line:42}.
    • relative path with null cwd → no fetch; a statusLine is written to the terminal (assert term.write).
    • in-flight guard: two rapid activate calls → one fetch.
    • openWebLink('javascript:alert(1)') → window.open not called; openWebLink('https://x') → window.open('https://x','_blank','noopener,noreferrer') called. Wire terminal-session.ts to green.

Coverage: the matcher (branch-heavy) is fully covered by the node test; the DOM wiring by jsdom; the server branch + validators by editor/integration tests. This keeps the 80% gate comfortably.

Edge cases & failure modes

  • No cwd yet (OSC-7 never fired, this.cwdValue === null, terminal-session.ts:97) → relative paths are unresolvable → skip activation, write a one-line statusLine('cannot open <path>: working dir unknown'). Absolute paths still work.
  • Path doesn't exist / is a dir → server returns 404/400; frontend shows a non-blocking statusLine toast, no throw (mirror the existing openProjectInEditor catch that only console.errors).
  • Wide (CJK) glyphs before a path shift xterm columns vs JS string index. Paths are ASCII, but a preceding CJK run offsets startX. Acceptable v1 caveat; note it. (Fixable later by walking cells; YAGNI now.)
  • URL/path overlap (https://host/a.ts:5) — WebLinksAddon links the URL; the /-preceded guard stops the path provider from double-linking the tail. Verify the two providers don't both underline.
  • provideLinks line indexing — pass terminal.buffer.active.getLine(bufferLineNumber - 1) and set range.{start,end}.y = bufferLineNumber (xterm gives a 1-based buffer row). Verify once in a real browser — the single indexing footgun.
  • Rapid clicks spawn N detached GUI processes on the host → in-flight boolean guard (+ optional 500 ms cooldown) on the frontend.
  • Non-code editor without --goto support → isGotoEditor returns false → open bare file (never inject a stray --goto argv the editor would treat as a filename).
  • Line-only false positives like 12:34 (a timestamp) — filtered because the token needs a filename-with-extension before the :line.

Security

  • Origin/CSRF: unchanged. /open-in-editor stays behind requireAllowedOrigin (src/server.ts:385, :352). The frontend POST is same-origin, so the browser sends Origin and passes; a foreign page's no-preflight POST is rejected 403. Integration test #3 asserts this on the new branch.
  • No shell / no injection: openFileInEditor uses execFile with an argv array (as editor.ts:51); the file path and <file>:<line> are argv elements, never a command line. line is validated to an integer before interpolation, so ${file}:${line} can't smuggle shell metacharacters via the line field.
  • Path containment: the resolved path is validated absolute + existing + isFile() server-side. Terminal output is attacker-influenced (a malicious repo could print ../../etc/hosts:1), but opening a file the user could already cat in the shell this app already grants adds no privilege (threat model: LAN, no auth, full shell). Optional hardening (note, not required v1): reject resolved paths that escape the session cwd root.
  • URL activation hardening (the real new surface): custom WebLinksAddon handler (replacing the default at line 148) allowlists schemes to http:/https:/mailto: and opens with window.open(uri, '_blank', 'noopener,noreferrer') — blocks javascript:/data:/file: URIs and prevents reverse-tabnabbing. Activation is gated on the click (a genuine user gesture); no hover/auto-open.
  • Rate-limit: frontend in-flight guard caps editor-spawn fan-out; the route's express.json({limit:'4kb'}) bounds body size. No new secrets, no logging of paths beyond the existing console.error on failure.

Effort & dependencies

  • Effort: ~1.5–2 days. Matcher + tests (0.5d), frontend wiring + jsdom tests + hardened URL handler (0.5d), server openFileInEditor + editor/integration tests (0.5d), manual browser verify of link indexing/overlap (0.25d).
  • Depends on: nothing — OSC-7 cwd capture (terminal-session.ts:155–159), the /open-in-editor route, and the WebLinksAddon dependency all already exist. Self-contained, W1.
  • Unlocks / synergy: the Approval preview (W1, task #7) and diff viewer (W4, task #13) can reuse findPathMatches + openFileInEditor to make paths in a diff/command preview clickable. link-paths.ts is deliberately a standalone pure module for that reuse.
  • Scope flag for the orchestrator: Option A adds a ~35-line server function (contradicting the brief's "no server change"). It is additive and backward-compatible, but it is a deviation — record it in PROGRESS_LOG.md. If the "no server change" constraint is hard, ship Option B (folder-open fallback) and defer file:line jump.