From 3258e0139ed6aab9f400f983b052dc77328c4bca Mon Sep 17 00:00:00 2001 From: Rob Macrae Date: Thu, 9 Jul 2026 15:00:25 +0100 Subject: [PATCH] fix(desktop): propagate surface token to WS + CLI; harden pull/cleanup/health MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses a code review of the desktop surface-token gate + robustness: P1 (WS omit surface token): the surface gate wasn't propagated to WebSocket URL builders, so terminal streaming, live collaboration, and session-control WS all 401 on desktop (WS can't send the header). Append ?surface= (+ user_id for the control WS, which had no params). Fixes desktop terminals/collaboration. P1 (CLI): cp_call and open_pty_ws now send the surface token (header) like apply_auth already did — CLI/TUI commands were 401ing under the gate. P2 (pull symlink): route the pull write through an O_NOFOLLOW open (new safe_write), mirroring the folder-import path, so a pre-planted terminal-component symlink can't be written through (safe_workspace_dest already guards ancestors). P2 (PID reuse): cleanup_stale_processes verifies a PID's command line is actually ours (data-dir/bundle/binary markers) before SIGTERM/SIGKILL — no longer kills an unrelated process that recycled the PID after a crash. P3 (health): wait_for_health now requires a real HTTP response (any status — the d1-shim/frontend legitimately 404), not just a TCP connect, so a stray listener isn't mistaken for readiness. Both bins + frontend typecheck clean. Revision markers bumped. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_014t8Ukp4NPWtqJ55X261wMZ --- desktop/app/src-tauri/src/bin/orcabot.rs | 43 +++++++++-- desktop/app/src-tauri/src/main.rs | 71 +++++++++++++++---- frontend/src/lib/api/cloudflare/dashboards.ts | 22 +++++- frontend/src/lib/ws/TerminalWSManager.ts | 11 ++- 4 files changed, 128 insertions(+), 19 deletions(-) diff --git a/desktop/app/src-tauri/src/bin/orcabot.rs b/desktop/app/src-tauri/src/bin/orcabot.rs index 9af3d912..7d57f8a9 100644 --- a/desktop/app/src-tauri/src/bin/orcabot.rs +++ b/desktop/app/src-tauri/src/bin/orcabot.rs @@ -1,4 +1,4 @@ -// REVISION: orcabot-cli-v19-pull-path-guard +// REVISION: orcabot-cli-v20-surface-token-ws // // `orcabot` — command-line control for the Orcabot desktop stack. // @@ -54,7 +54,7 @@ const CONTROLPLANE_PORT: u16 = 8787; const SANDBOX_PORT: u16 = 8080; const FRONTEND_PORT: u16 = 8788; const VZ_CONSOLE_LOG: &str = "/tmp/vz-console.log"; -const REVISION: &str = "orcabot-cli-v19-pull-path-guard"; +const REVISION: &str = "orcabot-cli-v20-surface-token-ws"; pub fn run() { let args: Vec = std::env::args().collect(); @@ -900,6 +900,29 @@ fn safe_workspace_dest(ws_canon: &std::path::Path, rel: &str) -> Option Some(dest) } +/// Write bytes to `dest` without following a symlink at the final path component. +/// safe_workspace_dest verifies existing ancestors resolve inside the workspace; +/// O_NOFOLLOW closes the last gap — a pre-planted `dest -> /elsewhere` symlink +/// fails with ELOOP instead of being written through. Mirrors safe_copy_file in +/// commands.rs (the Tauri folder-import path). +#[cfg(unix)] +fn safe_write(dest: &std::path::Path, data: &[u8]) -> std::io::Result<()> { + use std::io::Write; + use std::os::unix::fs::OpenOptionsExt; + let mut f = std::fs::OpenOptions::new() + .write(true) + .create(true) + .truncate(true) + .custom_flags(libc::O_NOFOLLOW) + .open(dest)?; + f.write_all(data) +} + +#[cfg(not(unix))] +fn safe_write(dest: &std::path::Path, data: &[u8]) -> std::io::Result<()> { + std::fs::write(dest, data) +} + /// Recreate a dashboard (items + edges, old->new item-id remap) on `dest`. /// Returns (new dashboard id, idmap, edges created). fn recreate_dashboard( @@ -1282,7 +1305,7 @@ fn cmd_pull(rest: &[String]) -> i32 { if let Some(parent) = dest_path.parent() { let _ = fs::create_dir_all(parent); } - if fs::write(&dest_path, &data).is_ok() { + if safe_write(&dest_path, &data).is_ok() { n += 1; bytes += data.len() as u64; if n % 50 == 0 { @@ -1670,12 +1693,17 @@ const DEV_NAME: &str = "Desktop User"; fn cp_call(method: &str, path: &str, body: Option) -> Result { let url = format!("http://127.0.0.1:{}{}", CONTROLPLANE_PORT, path); - let req = agent(Duration::from_secs(15)) + let mut req = agent(Duration::from_secs(15)) .request(method, &url) .set("X-User-ID", DEV_USER) .set("X-User-Email", DEV_EMAIL) .set("X-User-Name", DEV_NAME) .set("Content-Type", "application/json"); + // The local desktop control plane may gate dev-auth behind the per-boot + // surface token; include it (the CLI is a trusted host client that can read it). + if let Some(t) = read_surface_token() { + req = req.set("X-Orcabot-Surface", &t); + } let resp = match body { Some(b) => req.send_json(b), None => req.call(), @@ -1977,6 +2005,13 @@ fn open_pty_ws( "X-User-Name", tungstenite::http::HeaderValue::from_static(DEV_NAME), ); + // The local desktop control plane may gate dev-auth behind the per-boot + // surface token; send it as a header (this native client can, unlike a browser). + if let Some(t) = read_surface_token() { + if let Ok(v) = tungstenite::http::HeaderValue::from_str(&t) { + req.headers_mut().insert("X-Orcabot-Surface", v); + } + } req.headers_mut().insert( "Origin", tungstenite::http::HeaderValue::from_static("http://localhost:8788"), diff --git a/desktop/app/src-tauri/src/main.rs b/desktop/app/src-tauri/src/main.rs index 39fb0ea4..0f75a9da 100644 --- a/desktop/app/src-tauri/src/main.rs +++ b/desktop/app/src-tauri/src/main.rs @@ -1,7 +1,7 @@ #![cfg_attr(not(debug_assertions), windows_subsystem = "windows")] -// REVISION: main-v11-vm-image-ondemand -const MODULE_REVISION: &str = "main-v11-vm-image-ondemand"; +// REVISION: main-v12-surface-ws-hardening +const MODULE_REVISION: &str = "main-v12-surface-ws-hardening"; mod commands; mod vm; @@ -167,15 +167,23 @@ fn cleanup_stale_processes(data_dir: &Path) { for line in contents.lines() { if let Ok(pid) = line.trim().parse::() { - // Check if the process is still alive #[cfg(unix)] { - if unsafe { libc::kill(pid, 0) } == 0 { - eprintln!("[cleanup] Killing stale process {}", pid); - unsafe { libc::kill(pid, libc::SIGTERM) }; - // Give it a moment to exit gracefully, then force kill - std::thread::sleep(Duration::from_millis(500)); - unsafe { libc::kill(pid, libc::SIGKILL) }; + if unsafe { libc::kill(pid, 0) } != 0 { + continue; // not alive + } + // Verify the PID is actually one of ours before signaling. After a crash + // the OS may have recycled the PID for an unrelated process, and blindly + // SIGKILLing it would be a nasty bug. + match proc_command(pid) { + Some(cmd) if is_orcabot_process(&cmd, data_dir) => { + eprintln!("[cleanup] Killing stale Orcabot process {pid}"); + unsafe { libc::kill(pid, libc::SIGTERM) }; + std::thread::sleep(Duration::from_millis(500)); + unsafe { libc::kill(pid, libc::SIGKILL) }; + } + Some(_) => eprintln!("[cleanup] Skipping PID {pid} — not an Orcabot process (PID reused?)"), + None => eprintln!("[cleanup] Skipping PID {pid} — could not verify its identity"), } } } @@ -184,6 +192,38 @@ fn cleanup_stale_processes(data_dir: &Path) { let _ = std::fs::remove_file(&pid_path); } +/// The full command line of a running PID (via `ps`), or None if unreadable/gone. +#[cfg(unix)] +fn proc_command(pid: i32) -> Option { + let out = std::process::Command::new("ps") + .args(["-p", &pid.to_string(), "-o", "command="]) + .output() + .ok()?; + if !out.status.success() { + return None; + } + let cmd = String::from_utf8_lossy(&out.stdout).trim().to_string(); + if cmd.is_empty() { + None + } else { + Some(cmd) + } +} + +/// Whether a command line looks like one of Orcabot's own children. Our workerd, +/// d1-shim, and vz-helper run from the data dir, and the headless backend runs +/// from the com.orcabot bundle — install-specific markers, so a recycled PID +/// running an unrelated program is not matched. +#[cfg(unix)] +fn is_orcabot_process(cmd: &str, data_dir: &Path) -> bool { + let dd = data_dir.to_string_lossy(); + (!dd.is_empty() && cmd.contains(dd.as_ref())) + || cmd.contains("com.orcabot") + || cmd.contains("orcabot-desktop") + || cmd.contains("d1-shim") + || cmd.contains("vz-helper") +} + /// Write all tracked child PIDs to the PID file. fn write_pid_file(data_dir: &Path, children: &[Child], vm_pid: Option) { let pid_path = pid_file_path(data_dir); @@ -765,10 +805,17 @@ fn wait_for_health(port: &str) { let addr = format!("127.0.0.1:{}", port); for _ in 0..10 { if let Ok(mut stream) = std::net::TcpStream::connect(&addr) { - let _ = stream.write_all(b"GET /health HTTP/1.1\r\nHost: localhost\r\n\r\n"); + let _ = stream.set_read_timeout(Some(Duration::from_secs(2))); + let _ = stream.write_all(b"GET /health HTTP/1.1\r\nHost: localhost\r\nConnection: close\r\n\r\n"); let mut buf = [0u8; 128]; - let _ = stream.read(&mut buf); - return; + let n = stream.read(&mut buf).unwrap_or(0); + // Ready only on a real HTTP response. We accept ANY status (the d1-shim and + // frontend workerd legitimately 404 on /health) but require the "HTTP/" + // status line, so a stray non-HTTP listener on the port isn't mistaken for + // a healthy service. + if String::from_utf8_lossy(&buf[..n]).starts_with("HTTP/") { + return; + } } std::thread::sleep(Duration::from_millis(500)); } diff --git a/frontend/src/lib/api/cloudflare/dashboards.ts b/frontend/src/lib/api/cloudflare/dashboards.ts index 3712dd07..cc168ab2 100644 --- a/frontend/src/lib/api/cloudflare/dashboards.ts +++ b/frontend/src/lib/api/cloudflare/dashboards.ts @@ -1,9 +1,12 @@ // Copyright 2026 Rob Macrae. All rights reserved. // SPDX-License-Identifier: LicenseRef-Proprietary +// REVISION: dashboards-api-v1-surface-token-ws + import { API } from "@/config/env"; import { apiGet, apiPost, apiPut, apiDelete } from "../client"; import { useAuthStore } from "@/stores/auth-store"; +import { getCachedSurfaceToken } from "@/lib/tauri-bridge"; import type { Dashboard, DashboardItem, @@ -242,7 +245,18 @@ export async function updateSessionEnv( payload: { set?: Record; unset?: string[]; applyNow?: boolean } ): Promise { const base = API.cloudflare.base.replace(/^http/, "ws"); - const wsUrl = `${base}/sessions/${sessionId}/control`; + // Desktop/dev-auth can't send auth headers over a WebSocket, so pass identity + // + the surface token as query params (ignored by cookie-authed web). + let wsUrl = `${base}/sessions/${sessionId}/control`; + const user = useAuthStore.getState()?.user; + const surface = getCachedSurfaceToken(); + if (user?.id || surface) { + const params = new URLSearchParams(); + if (user?.id) params.set("user_id", user.id); + if (user?.email) params.set("user_email", user.email); + if (surface) params.set("surface", surface); + wsUrl += `?${params.toString()}`; + } const applyNow = Boolean(payload.applyNow); const message = { type: "env", @@ -556,6 +570,12 @@ export function getCollaborationWsUrl( if (email) { url += `&user_email=${encodeURIComponent(email)}`; } + // Desktop gates dev-auth on the surface token; WebSockets can't send the + // header, so pass it as a query param (null → omitted on web). + const surface = getCachedSurfaceToken(); + if (surface) { + url += `&surface=${encodeURIComponent(surface)}`; + } return url; } diff --git a/frontend/src/lib/ws/TerminalWSManager.ts b/frontend/src/lib/ws/TerminalWSManager.ts index a4795627..45c885f0 100644 --- a/frontend/src/lib/ws/TerminalWSManager.ts +++ b/frontend/src/lib/ws/TerminalWSManager.ts @@ -1,8 +1,8 @@ // Copyright 2026 Rob Macrae. All rights reserved. // SPDX-License-Identifier: LicenseRef-Proprietary -// REVISION: terminal-ws-v3-session-expired-recovery -const MODULE_REVISION_TERMINAL_WS = "terminal-ws-v3-session-expired-recovery"; +// REVISION: terminal-ws-v4-surface-token +const MODULE_REVISION_TERMINAL_WS = "terminal-ws-v4-surface-token"; console.log(`[TerminalWS] REVISION: ${MODULE_REVISION_TERMINAL_WS} loaded at ${new Date().toISOString()}`); /** @@ -30,6 +30,7 @@ import type { } from "@/types/terminal"; import { API, DESKTOP_MODE, DEV_MODE_ENABLED } from "@/config/env"; import { useAuthStore } from "@/stores/auth-store"; +import { getCachedSurfaceToken } from "@/lib/tauri-bridge"; export interface TerminalWSConfig extends WebSocketConfig { @@ -90,6 +91,12 @@ export class TerminalWSManager extends BaseWebSocketManager { url += `&user_email=${encodeURIComponent(email)}`; } } + // Desktop gates dev-auth on the surface token; WebSockets can't send the + // header, so pass it as a query param (no-op on web, where it's null). + const surface = getCachedSurfaceToken(); + if (surface) { + url += `&surface=${encodeURIComponent(surface)}`; + } super(url, config); this.sessionId = sessionId;