feat(hyperlinks): route CLI open requests through the host link handler

[kshivang]

Summary

CLI commands that spawn open <url|file> / xdg-open / $BROWSER (e.g. claude /login, gh auth login) used to launch the OS default handler directly — the terminal never saw the request, so embedding hosts couldn't offer their link-open UX.

• Shell-integration shim: bin/boss-open is extracted with the integration scripts and installed as open (macOS), xdg-open, and $BROWSER at the front of PATH for BossTerm sessions (bash/zsh/fish). Plain single-target URL/existing-file invocations are forwarded to the terminal as OSC 1341;OpenTarget;<base64>; everything else (option flags, multi-arg, directories, app names, non-existing paths) passes through to the real binary unchanged.
• Guards: intercepts only directly inside BossTerm (TERM_PROGRAM=BossTerm, writable /dev/tty), skips tmux/screen; kill switch BOSSTERM_DISABLE_OPEN_INTERCEPT=1; existing $BROWSER is never clobbered.
• New OpenTargetOSCListener classifies the decoded target into HyperlinkInfo (patternId osc:open-target) and dispatches it through the same onLinkClick path as Ctrl/Cmd+click, falling back to HyperlinkDetector.openUrl (system default) when unhandled — standalone BossTerm behavior is unchanged.
• Wiring: listeners are registered at session creation — TabController.openTargetLinkHandler (set by TabbedTerminal's extracted shared linkOpenHandler, which also keeps the SSH-URL wrapping in one place) and EmbeddableTerminalState.openTargetLinkHandler — so requests from background/uncomposed panes are still routed.

In BossConsole this makes CLI-originated opens show the terminal-link dialog (companion PR: risa-labs-inc/BossConsole#863 adds a System Default option there). Ships to terminal-tab via the usual auto-bump on the next BossTerm release.

Testing

• New OpenTargetOSCListenerTest (classification + dispatch + malformed payloads); full compose-ui desktopTest suite green.
• Shim exercised at the shell level: 7 passthrough scenarios verified, and under a real pty it emits correct OSC payloads (URL preserved, relative file paths absolutized). PATH/$BROWSER wiring verified in zsh and bash (no duplicate prepend on re-source).

🤖 Generated with Claude Code

[claude]

Review: route CLI open requests through the host link handler

Nice, cohesive feature. The shell shim's guard set is thoughtful (single-arg only, existing-file check, tmux/screen/TERM_PROGRAM gating, kill switch, $BROWSER never clobbered), the linkOpenHandler extraction removes real duplication, and the listener is registered at session creation so background/uncomposed panes still route. A few things worth addressing before merge.

1. Auto-open with no user interaction is a terminal-injection risk (highest priority)

Unlike Ctrl/Cmd+click, OpenTargetOSCListener.process() fires an open the moment the OSC arrives — no click, no confirmation. Any process (or cat-ing a crafted file, curling attacker-controlled output, a log line) that writes \033]1341;OpenTarget;<base64>\007 to the tty triggers it. The shim's guards only limit what the shim emits; they don't constrain what the listener honors.

Two concrete escalations:

• Standalone BossTerm: the handler returns false, so it falls straight to HyperlinkDetector.openUrl(info.url) → launches the system browser/opener for an arbitrary URL, or opens an arbitrary local file, unprompted.
• SSH auto-connect: classifyOpenTarget sets scheme = \"ssh\" for an ssh:// target, and TabbedTerminal.linkOpenHandler opens a new tab and runs the SSH command for info.scheme == \"ssh\". So an injected OSC 1341;OpenTarget;base64(\"ssh://attacker-host\") can spawn a tab that connects out — even though the shim itself never forwards ssh://.

Clickable OSC-8 hyperlinks are safe because they require a deliberate click; this path removes that gate. Suggestions: consider constraining the listener to a safe scheme allow-list (http/https/file/mailto) and explicitly not honoring ssh/custom schemes from this OSC, and/or reusing whatever trust/confirmation policy governs clicked links. At minimum, the SSH auto-connect via injected OSC should be closed off.

2. Handler invoked on the emulator thread, but it mutates UI/tab state

process() runs on the emulator processing thread and calls handlerProvider()?.invoke(info). For the tabbed path that lands in linkOpenHandler -> tabController.createTab(...), which mutates SnapshotStateList/activeTabIndex, launches coroutines, and starts a PTY process — all normally driven from the UI thread (Ctrl+click arrives on the UI thread). The existing OSC listeners only touch a thread-safe MutableState; this one does far more. Recommend marshaling the dispatch onto the main/Swing thread (or the session scope) so both entry points behave identically and you avoid subtle races in tab creation/focus. The KDoc correctly warns "Invoked on the emulator thread," which underscores the mismatch.

3. classifyOpenTarget duplicates toHyperlinkInfo

It re-implements most of Hyperlink.toHyperlinkInfo() (scheme extraction, File(URI(...)), isFile/isFolder, type mapping) with slightly different rules. Two copies of "classify a URL/path" will drift. Worth factoring the shared logic into one helper both call. Minor related notes:

• File(trimmed).absolutePath resolves relative paths against the JVM working directory, not the shell's CWD. The shim absolutizes before sending so it's fine in practice, but an injected relative path would resolve wrong — another reason to be strict about inputs.
• catch (e: Exception) around File(URI(...)) is broad; catching IllegalArgumentException/URISyntaxException would be tighter.

4. Shell shim — minor

• Solid POSIX sh. It depends on base64 being on PATH; ubiquitous on macOS/Linux but worth a mental note.
• On macOS, intercepting a plain open somefile.pdf changes long-standing behavior (it no longer immediately opens the default app; it routes to the host). This is intended per the PR, but it's a behavior change users may not expect — standalone falls back correctly via openUrl, so the net effect there is unchanged.
• The shim always exit 0 on success; callers that check the real open/xdg-open exit code won't see failures. Low impact.

Tests

Good coverage of classifyOpenTarget and dispatch, including malformed/null base64 payloads. Gaps: no test for the "unhandled -> openUrl fallback" branch, and the shim itself is only manually verified (understandable, but a couple of shell-level scenario tests would guard the passthrough matrix against regressions). Adding a case that pins the ssh-scheme behavior (see #1) would be valuable once that's decided.

Overall the mechanism is well-built; #1 and #2 are the ones I'd want resolved before this ships.

🤖 Generated with Claude Code

[kshivang]

Addressed the review in 0a34c4f:

1. Injection risk → two layers. (a) Token gate: sessions now get a process-wide secret as BOSSTERM_OPEN_TOKEN; the shim echoes it in the payload (OSC 1341;OpenTarget;<token>;<b64>) and the listener silently ignores requests without it — displayed content (cat/curl/logs) can't know the token. (b) Allow-list: only http/https URLs and existing absolute filesystem paths are honored; ssh://, mailto:, ftp://, custom schemes, and relative/non-existing paths are refused — the ssh:// auto-connect escalation is closed (pinned by a test).
2. Threading → dispatch is marshaled onto Dispatchers.Main (injectable scope for tests), so handlers see the same threading as Ctrl/Cmd+click.
3. Duplication → classifyOpenTarget is now a deliberately strict validator rather than a mirror of toHyperlinkInfo (validation vs. classification semantics diverge by design, documented); URI catches narrowed; relative paths refused instead of resolved against the JVM cwd.
4. Shim minors → added a base64 availability guard (passthrough when missing); exit-0 semantics kept as-is.
5. Tests → added token verification, refused-scheme cases (incl. the ssh pin), and the unhandled → system-opener fallback via an injectable fallbackOpener. Full compose-ui desktopTest suite green; shim re-verified under a pty (no-token → passthrough, token round-trips).

🤖 Generated with Claude Code

[claude]

Review: route CLI open requests through the host link handler

Nicely designed change — the security model is the strongest part: gating auto-open behind a per-process BOSSTERM_OPEN_TOKEN that displayed content can't know, checking the token before decoding the base64 payload, and refusing everything outside an explicit allow-list (http/https/existing absolute path). The multi-shell shim guards (TERM_PROGRAM, writable /dev/tty, tmux/screen skip, kill switch, non-clobbering $BROWSER, single-arg-only) are careful and well-commented, and the OpenTargetOSCListenerTest coverage of token auth + allow-list refusal + malformed payloads is solid. A few things worth considering:

Correctness / robustness

1. Handler exceptions can escape the dispatch coroutine. In OpenTargetOSCListener.process, scope.launch { handlerProvider()?.invoke(info) ... } runs on Dispatchers.Main with a bare SupervisorJob and no CoroutineExceptionHandler. If a host onLinkClick throws, the exception propagates to the default uncaught handler on the main thread. This path can fire from background/uncomposed panes, so consider wrapping the invoke/fallbackOpener in a try/catch. (Related: BossTerminal.processCustomCommand iterates listeners without a try/catch, so a synchronous throw would skip sibling listeners — the launch mostly insulates that, but the coroutine crash remains.)

2. Blocking filesystem I/O on the emulator parse thread. classifyOpenTarget calls file.isFile/file.isDirectory synchronously inside process(), which runs on the terminal data-stream thread. For a target on a hung/stale network mount this could stall terminal output parsing. It's only reachable by a locally-executed (token-bearing) command and is microseconds for local paths, so low severity — but moving the classification inside the scope.launch would keep the parse thread clean.

3. Shim absolutization can silently drop a valid open. In boss-open:

   target=\$(CDPATH= cd -- "\$(dirname -- "\$target")" && pwd)/\$(basename -- "\$target")

   If the cd fails (parent dir not traversable), the command substitution yields empty and target becomes /basename, which won't exist and gets refused by classifyOpenTarget — so a plain open somefile silently does nothing instead of falling through to the real opener. Rare (the file already passed -f), but a guarded fallback (|| passthrough "\$@") on that assignment would be safer than emitting a bad path.

Minor

4. Token comparison isn't constant-time. args[1] != OpenTargetToken.value uses ordinary String.equals. There's no realistic timing oracle here (no measurable response to injected content), so this is informational only.

5. Registration/wiring race window. The listener is registered at session creation reading openTargetLinkHandler via the provider, while the handler is set later in a SideEffect. Between session start and first composition the handler is null, so an early open request routes to system default rather than the host. Acceptable given the design intent (documented), just flagging it.

Nits

• installOpenShims's catch logs to System.err — consider the project's logger for consistency with the rest of ShellIntegrationInjector.
• Good call keeping openTargetLinkHandler on the state/controller rather than the composition so background panes stay routed; the doc comments explain the "why" well.

Overall this looks safe to ship — items (1) and (3) are the two I'd address before merge; the rest are optional. Nice work on the token gating and the shared linkOpenHandler refactor that keeps SSH-URL handling in one place.

[kshivang]

Addressed the second review round in 7893bca:

1. Handler exceptions → the handler invocation and fallback are wrapped in try/catch inside the dispatch coroutine; failures log via slf4j instead of crashing the main thread.
2. Blocking I/O on the parse thread → decode + classification moved into the coroutine behind a withContext(Dispatchers.IO) hop — the emulator parse thread only does the (constant-time) token check now, and Main never touches the filesystem either. ioDispatcher is injectable so the tests stay fully synchronous.
3. Shim absolutization → the cd is now guarded: an untraversable parent falls through to the real opener instead of emitting a bogus /basename path.
4. Token comparison → switched to MessageDigest.isEqual (agreed it's informational; it's cheap).
5. Wiring race → acknowledged as designed: before the host wires a handler, requests fall back to the system default — the pre-feature behavior, never a dropped open.
6. Nit → installOpenShims now logs through the class logger.

Verified: gradle exit code checked directly this round (BUILD SUCCESSFUL, 11/11 listener tests per the JUnit XML), shim re-verified under a pty including relative-path absolutization.

🤖 Generated with Claude Code

[claude]

Review: route CLI open requests through the host link handler

Overall this is a well-designed, security-conscious feature. The token-based authentication (so displayed content cannot forge open requests), the deliberate allow-list in classifyOpenTarget, the constant-time token compare, running FS classification off the parse thread, and the shim conservative "only plain single-target open" heuristic all reflect careful threat modeling. Test coverage of classification + dispatch + malformed payloads is good. A few things worth considering:

Behavior / correctness

1. Silent-drop when the shim intercepts but the JVM classifier refuses. The shim exits 0 after emitting the OSC, so the calling CLI believes the open succeeded. If classifyOpenTarget then returns null, the request vanishes — nothing opens and there is no fallback. The two classifiers can diverge for file: URLs (OpenTargetOSCListener.kt:154 does File(URI(trimmed)), which throws and yields null for authority-bearing URLs like file://host/path, query strings, etc.) and for a plain file deleted between the shim [ -f ] check and JVM classification (TOCTOU). The shim intercepts any file://* (bin/boss-open) but the JVM honors only a subset. Consider (a) keeping the two allow-lists tightly in sync, or (b) having the shim confirm success before exiting 0 / falling through otherwise. At minimum a LOG.debug on the dropped-target path would help diagnose "the link dialog just did not appear."

2. classifyOpenTarget accepts directories (FOLDER) but the shim never sends bare directories (it passes them through); the only way a FOLDER arrives is a file:// URL pointing at a dir. Not a bug, but the asymmetry between the shim rules and the JVM rules is a maintenance hazard — worth a comment noting the JVM side is intentionally broader.

Robustness (bin/boss-open)

3. No explicit recursion guard. self_dir is derived from dirname "$0". In the normal exec path $0 is a full path so this is fine, but if $0 ever arrives without a slash (some execlp/system call styles), dirname yields ".", self_dir becomes the cwd, and find_real would no longer skip the real shim dir — it could re-exec itself. A cheap belt-and-suspenders fix is a sentinel env var (check _BOSS_OPEN_REENTRY then passthrough; export it) before the intercept logic.

Lifecycle

4. OpenTargetOSCListener creates a CoroutineScope(SupervisorJob() + Dispatchers.Main) that is never cancelled, and there is no matching removeCustomCommandListener on tab close (three add sites in TabController.kt: ~415, ~794, ~1041). Low-impact in practice since no long-lived coroutines are retained, but for symmetry with tab teardown consider cancelling the scope / removing the listener on close.

Minor

5. The process-wide OpenTargetToken is shared across all tabs/sessions — right granularity for the stated threat, but note any local process that can read the env var can forge a request (acceptable, since it could invoke open directly). Worth one line in the token KDoc making that boundary explicit.

6. fallbackOpener default (HyperlinkDetector.openUrl) uses Runtime.exec, which returns immediately, so invoking it on Dispatchers.Main is fine — calling it out as verified, not a concern.

Nice work overall — the design mirrors the Ctrl/Cmd+click path cleanly and the KDoc is excellent. Items 1 and 3 are the two I would most want addressed before merge.

Generated with Claude Code

[claude]

PR Review: Route CLI open requests through the host link handler

Thorough, well-documented change. The two-commit hardening pass (token gate, allow-list, Main-thread dispatch) resolves the obvious injection vectors, and the design cleanly reuses the existing onLinkClick path so standalone behavior is preserved. A few things worth considering before merge.

Security — solid, with two minor notes

• Token gate plus allow-list is the right model. Displayed content (cat/curl output) cannot know BOSSTERM_OPEN_TOKEN, and refusing everything but http(s) and existing-absolute-path closes the ssh:// auto-connect escalation through the SSH wrapper. Good call constraining classifyOpenTarget to a strict validator rather than reusing toHyperlinkInfo().
• The process-wide token is readable from a process environment listing and printed by env/printenv. This is within the stated threat model (a same-uid process already has full control, and injected content still cannot emit a valid OSC), so I would only suggest a one-line comment in OpenTargetToken clarifying that the token defends against displayed content, not against executed processes — which can call the real open regardless.
• The token is written to the controlling tty inside the OSC. If a session is recorded with script/tee capturing the tty, the token leaks into the recording. Low severity (rotates per process), but worth being aware of.

Correctness / behavior

• Intercepted opens always exit 0 (boss-open: emit OSC then exit 0). Real open/xdg-open return non-zero on failure, so a script doing open URL && echo ok or checking the exit status now always sees success even when the host declines or the target cannot be opened. Probably acceptable for the use case, but it is a genuine semantics change for scripted callers — worth a note in the shim comment and docs.
• Fallback re-entrancy. The unhandled fallback calls HyperlinkDetector.openUrl, which execs bare open/xdg-open resolved against the app process PATH. In normal launches (desktop/launcher) the shim is not on PATH and TERM_PROGRAM is not BossTerm, so this is fine. But if the app is started from a BossTerm shell (e.g. ./gradlew run in a dev tab) the JVM inherits the shim on PATH plus the token plus a writable controlling tty, so the fallback re-enters the shim and emits an OSC to the outer terminal. It is bounded (no infinite loop) but surprising. Consider having the fallback resolve an absolute system opener or clear BOSSTERM_OPEN_TOKEN for the exec-d child to guarantee it bypasses the shim.

Design / quality

• The listener registration is duplicated across all three session-creation sites in TabController (createTab, ~789, createTabWithPreConnect), each next to the WorkingDirectoryOSCListener registration. That mirrors the existing pattern so it is consistent, but both listeners could be folded into a single registerOscListeners(terminal) helper to keep the third site from drifting later.
• OpenTargetOSCListener creates its own uncancelled CoroutineScope(SupervisorJob() + Dispatchers.Main) per session. It is cheap and GC-d with the terminal, but tying it to the session existing coroutineScope would be tidier and avoid an orphan scope per tab.
• classifyOpenTarget runs File.isFile/isDirectory (a blocking stat) synchronously on the emulator data-stream thread inside process(). Fine for local paths; just noting it happens off the dispatch thread.

Docs

• .claude/rules/shell-integration.md and CLAUDE.md are not updated. Since this globally intercepts every open/xdg-open/BROWSER invocation in a BossTerm session (not just auth flows), the kill switch BOSSTERM_DISABLE_OPEN_INTERCEPT=1 and the new bin/boss-open shim deserve a mention there.

Tests

• Good coverage of classification, token auth (wrong/missing/null), refused schemes including ssh, malformed base64, and the unhandled to system-opener fallback via the injectable opener. The Dispatchers.Unconfined plus Harness setup is clean.
• Gaps worth a line each: file:// pointing at a non-existent path being refused (only bare relative/absolute non-existent are tested), and a case asserting the file/folder fallback receives the resolved absolute path (not the raw file: URL). The shim itself (boss-open) has no automated test — the passthrough matrix is only manually verified per the description; a small shell test would lock in the guard conditions (options, multi-arg, tmux/screen, missing token, directory).

Overall a clean, security-conscious implementation. The exit 0 semantics and the dev-mode fallback re-entrancy are the two items I would most want addressed or explicitly acknowledged; the rest is polish.

🤖 Generated with Claude Code