diff --git a/Cargo.lock b/Cargo.lock index 3b6340b..90e5186 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -343,6 +343,7 @@ dependencies = [ "crossterm", "dirs-next", "globset", + "interprocess", "json5", "libc", "portable-pty", @@ -528,6 +529,12 @@ dependencies = [ "objc2", ] +[[package]] +name = "doctest-file" +version = "1.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c2db04e74f0a9a93103b50e90b96024c9b2bdca8bce6a632ec71b88736d3d359" + [[package]] name = "document-features" version = "0.2.12" @@ -834,6 +841,19 @@ dependencies = [ "syn 2.0.117", ] +[[package]] +name = "interprocess" +version = "2.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "069323743400cb7ab06a8fe5c1ed911d36b6919ec531661d034c89083629595b" +dependencies = [ + "doctest-file", + "libc", + "recvmsg", + "widestring", + "windows-sys", +] + [[package]] name = "is_terminal_polyfill" version = "1.70.2" @@ -1488,6 +1508,12 @@ dependencies = [ "unicode-width", ] +[[package]] +name = "recvmsg" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d3edd4d5d42c92f0a659926464d4cce56b562761267ecf0f469d85b7de384175" + [[package]] name = "redox_syscall" version = "0.5.18" @@ -2305,6 +2331,12 @@ dependencies = [ "wezterm-dynamic", ] +[[package]] +name = "widestring" +version = "1.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "72069c3113ab32ab29e5584db3c6ec55d416895e60715417b5b883a357c3e471" + [[package]] name = "winapi" version = "0.3.9" diff --git a/crates/coven-cli/Cargo.toml b/crates/coven-cli/Cargo.toml index 0cbf5f3..7b0eee4 100644 --- a/crates/coven-cli/Cargo.toml +++ b/crates/coven-cli/Cargo.toml @@ -35,5 +35,8 @@ textwrap = "0.16" [target.'cfg(unix)'.dependencies] libc = "0.2" +[target.'cfg(windows)'.dependencies] +interprocess = "2" + [dev-dependencies] tempfile = "3" diff --git a/crates/coven-cli/src/api.rs b/crates/coven-cli/src/api.rs index 47535ea..2338e1c 100644 --- a/crates/coven-cli/src/api.rs +++ b/crates/coven-cli/src/api.rs @@ -416,6 +416,18 @@ fn conversation_from_payload(payload: &Value) -> Result .filter(|id| !id.is_empty()) .context("conversation.id is required and must be a non-empty string")? .to_string(); + // The id is forwarded verbatim as the value of the harness CLI's + // `--session-id`/`--resume`/`resume` argument. Restrict it to an unambiguous, + // shell-safe charset so untrusted text can never inject extra arguments or — + // on Windows, where a `.cmd` shim re-parses the command line through cmd.exe — + // shell metacharacters. UUIDs and opaque slugs pass; whitespace and + // metacharacters (& | < > ^ % " $ etc.) do not. + if !id + .chars() + .all(|c| c.is_ascii_alphanumeric() || matches!(c, '-' | '_' | '.')) + { + anyhow::bail!("conversation.id must contain only letters, digits, '-', '_', or '.'"); + } match mode { "init" => Ok(Some(ConversationHint::Init { id })), "resume" => Ok(Some(ConversationHint::Resume { id })), @@ -1425,6 +1437,41 @@ mod tests { Ok(()) } + #[test] + fn conversation_id_rejects_shell_metacharacters() { + // Ids carrying whitespace or shell metacharacters must be rejected so they + // can never reach the harness CLI's `--session-id`/`--resume`/`resume` argv, + // where on Windows a `.cmd` shim would re-parse cmd.exe metacharacters. + for bad in [ + "not-a-uuid & calc.exe", + "a|b", + "a b", + "$(whoami)", + "a\"b", + "a^b", + ] { + let payload = json!({"conversation": {"mode": "resume", "id": bad}}); + assert!( + conversation_from_payload(&payload).is_err(), + "expected {bad:?} to be rejected" + ); + } + // UUIDs and opaque slugs are accepted. + for good in [ + "11111111-2222-3333-4444-555555555555", + "abc-123", + "sess_1.2", + ] { + let payload = json!({"conversation": {"mode": "init", "id": good}}); + assert!( + conversation_from_payload(&payload) + .expect("shell-safe id should parse") + .is_some(), + "expected {good:?} to be accepted" + ); + } + } + #[test] fn launch_request_with_malformed_conversation_mode_returns_400_not_daemon_crash( ) -> anyhow::Result<()> { diff --git a/crates/coven-cli/src/daemon.rs b/crates/coven-cli/src/daemon.rs index a2a828a..9aac184 100644 --- a/crates/coven-cli/src/daemon.rs +++ b/crates/coven-cli/src/daemon.rs @@ -1,12 +1,12 @@ use std::collections::HashMap; use std::io::Write; +#[cfg(unix)] use std::net::{SocketAddr, TcpListener, ToSocketAddrs}; use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; use std::sync::Mutex; use std::time::{Duration, Instant}; -#[cfg(unix)] use std::io::{BufRead, BufReader, Read}; #[cfg(unix)] use std::os::unix::{ @@ -37,6 +37,7 @@ pub enum DaemonStatusState { Stale(DaemonStatus), } +#[cfg(unix)] #[derive(Debug, Deserialize)] struct DaemonHealthStatus { ok: bool, @@ -902,9 +903,12 @@ fn daemon_status_from_health_socket(socket: &str) -> Result // Unix socket — a misbehaving network client can otherwise hold the API // thread indefinitely (slowloris) or force a huge allocation by claiming a // large body. +#[cfg(unix)] pub const TCP_IO_TIMEOUT: Duration = Duration::from_secs(30); +#[cfg(unix)] pub const MAX_TCP_BODY_BYTES: usize = 1024 * 1024; +#[cfg(unix)] fn ensure_loopback_addrs(addrs: &[SocketAddr]) -> Result<()> { if addrs.is_empty() { anyhow::bail!("TCP listener address did not resolve to any sockets"); @@ -927,6 +931,7 @@ fn ensure_loopback_addrs(addrs: &[SocketAddr]) -> Result<()> { Ok(()) } +#[cfg(unix)] pub fn bind_tcp_listener(addr: A) -> Result { let addrs: Vec = addr .to_socket_addrs() @@ -1064,7 +1069,6 @@ pub fn serve_forever(coven_home: &Path, started_at: String, tcp_addr: Option<&st } } -#[cfg(unix)] fn handle_http_stream( read: R, mut write: W, @@ -1125,7 +1129,6 @@ where Ok(()) } -#[cfg(unix)] fn write_payload_too_large(write: &mut W, max: usize) -> Result<()> { let body = format!( "{{\"ok\":false,\"error\":{{\"code\":\"payload_too_large\",\"message\":\"Request body exceeds {max}-byte limit.\"}}}}", @@ -1141,7 +1144,6 @@ fn write_payload_too_large(write: &mut W, max: usize) -> Result<()> { Ok(()) } -#[cfg(unix)] fn host_is_loopback(host: Option<&str>) -> bool { match host { Some(h) => is_loopback_host(strip_port(h.trim())), @@ -1149,7 +1151,6 @@ fn host_is_loopback(host: Option<&str>) -> bool { } } -#[cfg(unix)] fn origin_is_loopback(origin: &str) -> bool { match origin.trim().split_once("://") { Some((_scheme, rest)) => is_loopback_host(strip_port(rest)), @@ -1157,7 +1158,6 @@ fn origin_is_loopback(origin: &str) -> bool { } } -#[cfg(unix)] fn strip_port(authority: &str) -> &str { if let Some(rest) = authority.strip_prefix('[') { // IPv6 literal like [::1]:8080 -> ::1 @@ -1166,7 +1166,6 @@ fn strip_port(authority: &str) -> &str { authority.split(':').next().unwrap_or(authority) } -#[cfg(unix)] fn is_loopback_host(host: &str) -> bool { // Parse as an IP and ask the address itself — never a string prefix. A prefix // test like `starts_with("127.")` would also accept attacker hostnames such as @@ -1179,7 +1178,6 @@ fn is_loopback_host(host: &str) -> bool { .unwrap_or(false) } -#[cfg(unix)] fn write_forbidden(write: &mut W, reason: &str) -> Result<()> { let body = format!("{{\"ok\":false,\"error\":{{\"code\":\"forbidden\",\"message\":\"{reason}\"}}}}"); @@ -1223,7 +1221,6 @@ fn http_reason_phrase(status: u16) -> &'static str { } } -#[cfg(unix)] fn read_http_request_line(reader: &mut R) -> Result { let mut line = String::new(); reader @@ -1235,14 +1232,12 @@ fn read_http_request_line(reader: &mut R) -> Result { Ok(line) } -#[cfg(unix)] struct ParsedHeaders { content_length: usize, host: Option, origin: Option, } -#[cfg(unix)] fn read_http_headers(reader: &mut R) -> Result { let mut headers = ParsedHeaders { content_length: 0, @@ -1273,7 +1268,6 @@ fn read_http_headers(reader: &mut R) -> Result { Ok(headers) } -#[cfg(unix)] fn read_http_body(reader: &mut R, content_length: usize) -> Result> { if content_length == 0 { return Ok(None); @@ -1287,7 +1281,6 @@ fn read_http_body(reader: &mut R, content_length: usize) -> Result Result<(&str, &str)> { let mut parts = line.split_whitespace(); let method = parts.next().context("missing HTTP method")?; @@ -1295,6 +1288,66 @@ fn parse_request_line(line: &str) -> Result<(&str, &str)> { Ok((method, path)) } +#[cfg(windows)] +fn windows_pipe_name(coven_home: &Path) -> String { + use std::hash::{Hash, Hasher}; + let mut h = std::collections::hash_map::DefaultHasher::new(); + coven_home.to_string_lossy().hash(&mut h); + format!("coven-daemon-{:016x}.sock", h.finish()) +} + +#[cfg(windows)] +pub fn serve_forever(coven_home: &Path, started_at: String, tcp_addr: Option<&str>) -> Result<()> { + use interprocess::local_socket::{prelude::*, GenericNamespaced, ListenerOptions}; + use std::sync::Arc; + + let _ = tcp_addr; // TCP not wired on Windows in this prototype + + let status = DaemonStatus { + pid: std::process::id(), + started_at: started_at.clone(), + socket: windows_pipe_name(coven_home), + }; + write_status(coven_home, &status)?; + recover_orphaned_sessions(coven_home, &started_at)?; + + let name = windows_pipe_name(coven_home) + .to_ns_name::() + .context("failed to create named pipe name")?; + let listener = ListenerOptions::new() + .name(name) + .create_sync() + .context("failed to bind Windows named pipe")?; + + let runtime = Arc::new(LiveSessionRuntime::with_coven_home( + coven_home.to_path_buf(), + )); + + for conn in listener.incoming() { + let stream = match conn { + Ok(s) => s, + Err(error) => { + eprintln!("coven daemon: pipe accept error: {error:#}"); + continue; + } + }; + // Stream implements Read + Write via shared reference on Windows. + // The handler reads the full request before writing, so sharing &stream is safe. + if let Err(error) = handle_http_stream( + &stream, + &stream, + coven_home, + Some(status.clone()), + runtime.as_ref(), + None, + false, + ) { + eprintln!("coven daemon: pipe connection error: {error:#}"); + } + } + Ok(()) +} + #[cfg(test)] mod tests { use super::*; @@ -1722,6 +1775,7 @@ mod tests { assert!(response.contains("\"apiVersion\""), "got: {response}"); } + #[cfg(unix)] #[test] fn bind_tcp_listener_rejects_non_loopback() { let error = bind_tcp_listener("0.0.0.0:0").expect_err("should reject wildcard bind"); @@ -2433,4 +2487,54 @@ mod tests { .map(|session| session.status.clone()) .unwrap_or_default() } + + #[cfg(windows)] + #[test] + fn serves_health_over_windows_named_pipe() -> Result<()> { + use interprocess::local_socket::{prelude::*, GenericNamespaced, ListenerOptions, Stream}; + use std::io::{Read, Write}; + use std::thread; + + let temp_dir = tempfile::tempdir()?; + let pipe_name = windows_pipe_name(temp_dir.path()); + + let name = pipe_name + .clone() + .to_ns_name::() + .expect("pipe name"); + let listener = ListenerOptions::new() + .name(name) + .create_sync() + .expect("bind pipe"); + + let status = DaemonStatus { + pid: 12345, + started_at: "2026-04-27T10:00:00Z".to_string(), + socket: pipe_name.clone(), + }; + let home = temp_dir.path().to_path_buf(); + let runtime = LiveSessionRuntime::default(); + let server = thread::spawn(move || { + let conn = listener.incoming().next().expect("accept").expect("stream"); + handle_http_stream(&conn, &conn, &home, Some(status), &runtime, None, false) + }); + + let client_name = pipe_name + .to_ns_name::() + .expect("client pipe name"); + let mut client = Stream::connect(client_name).expect("connect"); + client + .write_all(b"GET /api/v1/health HTTP/1.1\r\nHost: coven\r\n\r\n") + .expect("write request"); + // Flush to ensure the server receives the full request before we start reading. + client.flush().expect("flush"); + let mut response = String::new(); + client.read_to_string(&mut response).expect("read response"); + + server.join().expect("server thread")?; + + assert!(response.starts_with("HTTP/1.1 200 OK"), "got: {response}"); + assert!(response.contains("\"apiVersion\""), "got: {response}"); + Ok(()) + } } diff --git a/crates/coven-cli/src/main.rs b/crates/coven-cli/src/main.rs index d2c8309..98c016d 100644 --- a/crates/coven-cli/src/main.rs +++ b/crates/coven-cli/src/main.rs @@ -1,6 +1,8 @@ use std::collections::HashSet; use std::ffi::OsString; -use std::io::{self, BufRead, IsTerminal, Read, Write}; +#[cfg(unix)] +use std::io::Read; +use std::io::{self, BufRead, IsTerminal, Write}; use std::path::{Path, PathBuf}; use std::thread; use std::time::Duration; @@ -792,11 +794,15 @@ fn run_daemon_command(command: DaemonCommand) -> Result<()> { { daemon::serve_forever(&home, current_timestamp(), tcp.as_deref())?; } - #[cfg(not(unix))] + #[cfg(windows)] + { + daemon::serve_forever(&home, current_timestamp(), tcp.as_deref())?; + } + #[cfg(not(any(unix, windows)))] { let _ = tcp; anyhow::bail!( - "coven daemon server is only implemented on Unix-like systems for now" + "coven daemon server is only implemented on Unix-like systems and Windows for now" ); } } @@ -1345,6 +1351,7 @@ fn send_session_input(_coven_home: &Path, _session_id: &str, _data: &str) -> Res anyhow::bail!("Coven attach input forwarding is only implemented on Unix-like systems for now") } +#[cfg(unix)] fn ensure_successful_http_response(response: &str) -> Result<()> { let status = response .lines() @@ -2167,6 +2174,7 @@ mod tests { ); } + #[cfg(unix)] #[test] fn successful_http_response_accepts_2xx_only() { assert!(ensure_successful_http_response("HTTP/1.1 202 Accepted\r\n\r\n{}").is_ok()); diff --git a/crates/coven-cli/src/tui/chat/client.rs b/crates/coven-cli/src/tui/chat/client.rs index 5c6cb1b..4cd2c5b 100644 --- a/crates/coven-cli/src/tui/chat/client.rs +++ b/crates/coven-cli/src/tui/chat/client.rs @@ -5,10 +5,13 @@ //! ritual verbs use the shared store path/timestamp helpers because they are //! ledger-only mutations. +#[cfg(unix)] use std::io::{Read, Write}; use std::path::{Path, PathBuf}; -use anyhow::{anyhow, Context, Result}; +#[cfg(unix)] +use anyhow::anyhow; +use anyhow::{Context, Result}; use serde::Deserialize; use serde_json::{json, Value}; use uuid::Uuid; @@ -387,6 +390,7 @@ fn request_daemon( anyhow::bail!("Coven daemon chat is only implemented on Unix-like systems for now") } +#[cfg(unix)] fn parse_http_response(response: &str) -> Result { let (head, body) = response .split_once("\r\n\r\n") @@ -407,6 +411,7 @@ fn parse_http_response(response: &str) -> Result { }) } +#[cfg(unix)] fn daemon_error(status: u16, body: &str) -> anyhow::Error { if let Ok(value) = serde_json::from_str::(body) { if let Some(message) = value @@ -444,6 +449,7 @@ fn session_title(prompt: &str) -> String { mod tests { use super::*; + #[cfg(unix)] #[test] fn parses_successful_http_response_body() -> Result<()> { let response = @@ -454,6 +460,7 @@ mod tests { Ok(()) } + #[cfg(unix)] #[test] fn turns_structured_daemon_errors_into_readable_errors() { let error = parse_http_response(