From 276167a6fd5fafc22953bd4986cc97ed28781653 Mon Sep 17 00:00:00 2001 From: Lawrence Chen <54008264+lawrencecchen@users.noreply.github.com> Date: Tue, 1 Sep 2026 03:07:10 -0700 Subject: [PATCH 1/6] Harden cmux-tui executable resolution before spawn --- cmux-tui/crates/chatmux-relay/src/pty_deps.rs | 23 ++++++++++++------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/cmux-tui/crates/chatmux-relay/src/pty_deps.rs b/cmux-tui/crates/chatmux-relay/src/pty_deps.rs index 970bd7834615..820d050e1861 100644 --- a/cmux-tui/crates/chatmux-relay/src/pty_deps.rs +++ b/cmux-tui/crates/chatmux-relay/src/pty_deps.rs @@ -845,12 +845,11 @@ impl PtyDeps for RealPtyDeps { if let Some(override_path) = self.env.get("CHATMUX_RELAY_CMUX_TUI").filter(|value| !value.trim().is_empty()) { - let path = override_path.trim(); - return if is_executable(Path::new(path)).await { - Some(CmuxTui { file: path.to_owned(), prefix: Vec::new() }) - } else { - None - }; + let path = Path::new(override_path.trim()); + return canonical_executable(path).await.map(|file| CmuxTui { + file: file.to_string_lossy().into_owned(), + prefix: Vec::new(), + }); } // Never a bare `cmux` on PATH — that name is ambiguous; only cmux-tui. for dir in self.env.get("PATH").map(String::as_str).unwrap_or("").split(':') { @@ -858,9 +857,9 @@ impl PtyDeps for RealPtyDeps { continue; } let candidate = Path::new(dir).join("cmux-tui"); - if is_executable(&candidate).await { + if let Some(file) = canonical_executable(&candidate).await { return Some(CmuxTui { - file: candidate.to_string_lossy().into_owned(), + file: file.to_string_lossy().into_owned(), prefix: Vec::new(), }); } @@ -988,6 +987,14 @@ async fn is_executable(path: &Path) -> bool { } } +/// Resolve and validate a PATH candidate before handing it to `Command`. +/// Keeping the canonical absolute path in `CmuxTui` avoids a second PATH +/// lookup after validation, so a changed PATH cannot select another binary. +async fn canonical_executable(path: &Path) -> Option { + let canonical = tokio::fs::canonicalize(path).await.ok()?; + is_executable(&canonical).await.then_some(canonical) +} + /// Session-name validity is re-exported so the daemon path can reject early. pub fn valid_session(name: &str) -> bool { session_name_ok(name) From 20e322f7a737b56cac1bb6566b646e829c4305bb Mon Sep 17 00:00:00 2001 From: Lawrence Chen <54008264+lawrencecchen@users.noreply.github.com> Date: Tue, 1 Sep 2026 03:26:20 -0700 Subject: [PATCH 2/6] Fix relay callback clippy findings --- cmux-tui/crates/chatmux-relay/src/pty_deps.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cmux-tui/crates/chatmux-relay/src/pty_deps.rs b/cmux-tui/crates/chatmux-relay/src/pty_deps.rs index 820d050e1861..da6ceeda9f10 100644 --- a/cmux-tui/crates/chatmux-relay/src/pty_deps.rs +++ b/cmux-tui/crates/chatmux-relay/src/pty_deps.rs @@ -1112,10 +1112,10 @@ mod tests { let exit_seen = TestArc::clone(&seen); output.subscribe( TestArc::new(move |chunk| { - data_seen.lock().expect("seen lock").push(format!("data:{}", chunk.len())) + data_seen.lock().expect("seen lock").push(format!("data:{}", chunk.len())); }), TestArc::new(move |code| { - exit_seen.lock().expect("seen lock").push(format!("exit:{code}")) + exit_seen.lock().expect("seen lock").push(format!("exit:{code}")); }), ); assert_eq!( @@ -1201,10 +1201,10 @@ mod tests { data_seen .lock() .expect("seen lock") - .push(String::from_utf8_lossy(&chunk).into_owned()) + .push(String::from_utf8_lossy(&chunk).into_owned()); }), TestArc::new(move |code| { - exit_seen.lock().expect("seen lock").push(format!("exit:{code}")) + exit_seen.lock().expect("seen lock").push(format!("exit:{code}")); }), ); From efcc834194002197b455d30f4ca326b28426489b Mon Sep 17 00:00:00 2001 From: Lawrence Chen <54008264+lawrencecchen@users.noreply.github.com> Date: Tue, 1 Sep 2026 17:05:53 -0700 Subject: [PATCH 3/6] fix relay PATH resolution relative to request cwd --- cmux-tui/crates/chatmux-relay/src/pty.rs | 6 ++-- cmux-tui/crates/chatmux-relay/src/pty_deps.rs | 30 +++++++++++++++---- .../chatmux-relay/src/tunnel_terminal.rs | 2 +- 3 files changed, 29 insertions(+), 9 deletions(-) diff --git a/cmux-tui/crates/chatmux-relay/src/pty.rs b/cmux-tui/crates/chatmux-relay/src/pty.rs index 79e088499840..a4901703b617 100644 --- a/cmux-tui/crates/chatmux-relay/src/pty.rs +++ b/cmux-tui/crates/chatmux-relay/src/pty.rs @@ -246,7 +246,7 @@ pub struct EnsureDaemon { #[async_trait] pub trait PtyDeps: Send + Sync { async fn spawn_pty(&self, spec: SpawnSpec) -> PtyHandle; - async fn resolve_cmux_tui(&self) -> Option; + async fn resolve_cmux_tui(&self, cwd: &Path) -> Option; async fn ensure_daemon( &self, cmux_tui: &CmuxTui, @@ -677,7 +677,7 @@ impl Inner { }; let env = pty_env(&self.env); - let cmux_tui = self.deps.resolve_cmux_tui().await; + let cmux_tui = self.deps.resolve_cmux_tui(&cwd).await; let opened = if let (Some(cmux_tui), Some(surface_ref)) = (cmux_tui.as_ref(), surface_ref.as_ref()) { @@ -2125,7 +2125,7 @@ mod tests { let output: Arc = Arc::new(pty); PtyHandle { control, output, banner: None } } - async fn resolve_cmux_tui(&self) -> Option { + async fn resolve_cmux_tui(&self, _cwd: &Path) -> Option { self.resolve.clone() } async fn ensure_daemon( diff --git a/cmux-tui/crates/chatmux-relay/src/pty_deps.rs b/cmux-tui/crates/chatmux-relay/src/pty_deps.rs index da6ceeda9f10..c11d2cb8231a 100644 --- a/cmux-tui/crates/chatmux-relay/src/pty_deps.rs +++ b/cmux-tui/crates/chatmux-relay/src/pty_deps.rs @@ -841,12 +841,12 @@ impl PtyDeps for RealPtyDeps { }) } - async fn resolve_cmux_tui(&self) -> Option { + async fn resolve_cmux_tui(&self, cwd: &Path) -> Option { if let Some(override_path) = self.env.get("CHATMUX_RELAY_CMUX_TUI").filter(|value| !value.trim().is_empty()) { let path = Path::new(override_path.trim()); - return canonical_executable(path).await.map(|file| CmuxTui { + return canonical_executable(path, cwd).await.map(|file| CmuxTui { file: file.to_string_lossy().into_owned(), prefix: Vec::new(), }); @@ -857,7 +857,7 @@ impl PtyDeps for RealPtyDeps { continue; } let candidate = Path::new(dir).join("cmux-tui"); - if let Some(file) = canonical_executable(&candidate).await { + if let Some(file) = canonical_executable(&candidate, cwd).await { return Some(CmuxTui { file: file.to_string_lossy().into_owned(), prefix: Vec::new(), @@ -990,8 +990,9 @@ async fn is_executable(path: &Path) -> bool { /// Resolve and validate a PATH candidate before handing it to `Command`. /// Keeping the canonical absolute path in `CmuxTui` avoids a second PATH /// lookup after validation, so a changed PATH cannot select another binary. -async fn canonical_executable(path: &Path) -> Option { - let canonical = tokio::fs::canonicalize(path).await.ok()?; +async fn canonical_executable(path: &Path, cwd: &Path) -> Option { + let resolved = if path.is_absolute() { path.to_path_buf() } else { cwd.join(path) }; + let canonical = tokio::fs::canonicalize(resolved).await.ok()?; is_executable(&canonical).await.then_some(canonical) } @@ -1053,6 +1054,25 @@ mod tests { assert!(error.contains("invalid session")); } + #[tokio::test] + async fn canonical_executable_resolves_relative_path_against_request_cwd() { + use std::os::unix::fs::PermissionsExt; + + let root = std::env::temp_dir().join(format!("cmux-relay-cwd-test-{}", std::process::id())); + let cwd = root.join("request"); + let bin = cwd.join("bin"); + let executable = bin.join("cmux-tui"); + tokio::fs::create_dir_all(&bin).await.unwrap(); + tokio::fs::write(&executable, b"#!/bin/sh\n").await.unwrap(); + tokio::fs::set_permissions(&executable, std::fs::Permissions::from_mode(0o755)) + .await + .unwrap(); + + let resolved = canonical_executable(Path::new("bin/cmux-tui"), &cwd).await; + assert_eq!(resolved, Some(std::fs::canonicalize(&executable).unwrap())); + let _ = tokio::fs::remove_dir_all(root).await; + } + #[test] fn subscribe_replay_stays_ahead_of_concurrent_output_and_exit() { let output = ThreadOutput::new(); diff --git a/cmux-tui/crates/chatmux-relay/src/tunnel_terminal.rs b/cmux-tui/crates/chatmux-relay/src/tunnel_terminal.rs index a6ce9140b457..317a61fa6146 100644 --- a/cmux-tui/crates/chatmux-relay/src/tunnel_terminal.rs +++ b/cmux-tui/crates/chatmux-relay/src/tunnel_terminal.rs @@ -711,7 +711,7 @@ mod tests { self.spawned.lock().unwrap().push(pty.clone()); PtyHandle { control: Arc::new(pty.clone()), output: Arc::new(pty), banner: None } } - async fn resolve_cmux_tui(&self) -> Option { + async fn resolve_cmux_tui(&self, _cwd: &Path) -> Option { None } async fn ensure_daemon( From f93a93aedbf7273b28d68cbb2e3b6694bd2297e6 Mon Sep 17 00:00:00 2001 From: Lawrence Chen <54008264+lawrencecchen@users.noreply.github.com> Date: Wed, 2 Sep 2026 04:28:43 -0700 Subject: [PATCH 4/6] test relay rejects relative executable sources --- cmux-tui/crates/chatmux-relay/src/pty_deps.rs | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/cmux-tui/crates/chatmux-relay/src/pty_deps.rs b/cmux-tui/crates/chatmux-relay/src/pty_deps.rs index c11d2cb8231a..cc2e736597aa 100644 --- a/cmux-tui/crates/chatmux-relay/src/pty_deps.rs +++ b/cmux-tui/crates/chatmux-relay/src/pty_deps.rs @@ -1073,6 +1073,35 @@ mod tests { let _ = tokio::fs::remove_dir_all(root).await; } + #[tokio::test] + async fn resolver_rejects_relative_override_and_path_entries() { + use std::os::unix::fs::PermissionsExt; + + let root = std::env::temp_dir().join(format!( + "cmux-relay-relative-executable-policy-{}-{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos() + )); + let cwd = root.join("launch"); + let executable = cwd.join("bin/cmux-tui"); + tokio::fs::create_dir_all(executable.parent().unwrap()).await.unwrap(); + tokio::fs::write(&executable, b"#!/bin/sh\n").await.unwrap(); + tokio::fs::set_permissions(&executable, std::fs::Permissions::from_mode(0o755)) + .await + .unwrap(); + + let mut env = HashMap::new(); + env.insert("CHATMUX_RELAY_CMUX_TUI".to_owned(), "bin/cmux-tui".to_owned()); + env.insert("PATH".to_owned(), "bin".to_owned()); + let deps = RealPtyDeps::new(env); + + assert!(deps.resolve_cmux_tui(&cwd).await.is_none()); + let _ = tokio::fs::remove_dir_all(root).await; + } + #[test] fn subscribe_replay_stays_ahead_of_concurrent_output_and_exit() { let output = ThreadOutput::new(); From ac2025a52746792358d0857c92e8e40915fc32da Mon Sep 17 00:00:00 2001 From: Lawrence Chen <54008264+lawrencecchen@users.noreply.github.com> Date: Wed, 2 Sep 2026 04:30:26 -0700 Subject: [PATCH 5/6] fix relay executable resolution policy --- cmux-tui/crates/chatmux-relay/src/pty.rs | 6 +-- cmux-tui/crates/chatmux-relay/src/pty_deps.rs | 43 +++++++++++++------ .../chatmux-relay/src/tunnel_terminal.rs | 2 +- 3 files changed, 33 insertions(+), 18 deletions(-) diff --git a/cmux-tui/crates/chatmux-relay/src/pty.rs b/cmux-tui/crates/chatmux-relay/src/pty.rs index a4901703b617..79e088499840 100644 --- a/cmux-tui/crates/chatmux-relay/src/pty.rs +++ b/cmux-tui/crates/chatmux-relay/src/pty.rs @@ -246,7 +246,7 @@ pub struct EnsureDaemon { #[async_trait] pub trait PtyDeps: Send + Sync { async fn spawn_pty(&self, spec: SpawnSpec) -> PtyHandle; - async fn resolve_cmux_tui(&self, cwd: &Path) -> Option; + async fn resolve_cmux_tui(&self) -> Option; async fn ensure_daemon( &self, cmux_tui: &CmuxTui, @@ -677,7 +677,7 @@ impl Inner { }; let env = pty_env(&self.env); - let cmux_tui = self.deps.resolve_cmux_tui(&cwd).await; + let cmux_tui = self.deps.resolve_cmux_tui().await; let opened = if let (Some(cmux_tui), Some(surface_ref)) = (cmux_tui.as_ref(), surface_ref.as_ref()) { @@ -2125,7 +2125,7 @@ mod tests { let output: Arc = Arc::new(pty); PtyHandle { control, output, banner: None } } - async fn resolve_cmux_tui(&self, _cwd: &Path) -> Option { + async fn resolve_cmux_tui(&self) -> Option { self.resolve.clone() } async fn ensure_daemon( diff --git a/cmux-tui/crates/chatmux-relay/src/pty_deps.rs b/cmux-tui/crates/chatmux-relay/src/pty_deps.rs index cc2e736597aa..1b6f4df82233 100644 --- a/cmux-tui/crates/chatmux-relay/src/pty_deps.rs +++ b/cmux-tui/crates/chatmux-relay/src/pty_deps.rs @@ -841,12 +841,12 @@ impl PtyDeps for RealPtyDeps { }) } - async fn resolve_cmux_tui(&self, cwd: &Path) -> Option { + async fn resolve_cmux_tui(&self) -> Option { if let Some(override_path) = self.env.get("CHATMUX_RELAY_CMUX_TUI").filter(|value| !value.trim().is_empty()) { let path = Path::new(override_path.trim()); - return canonical_executable(path, cwd).await.map(|file| CmuxTui { + return canonical_executable(path).await.map(|file| CmuxTui { file: file.to_string_lossy().into_owned(), prefix: Vec::new(), }); @@ -857,7 +857,7 @@ impl PtyDeps for RealPtyDeps { continue; } let candidate = Path::new(dir).join("cmux-tui"); - if let Some(file) = canonical_executable(&candidate, cwd).await { + if let Some(file) = canonical_executable(&candidate).await { return Some(CmuxTui { file: file.to_string_lossy().into_owned(), prefix: Vec::new(), @@ -987,12 +987,15 @@ async fn is_executable(path: &Path) -> bool { } } -/// Resolve and validate a PATH candidate before handing it to `Command`. -/// Keeping the canonical absolute path in `CmuxTui` avoids a second PATH -/// lookup after validation, so a changed PATH cannot select another binary. -async fn canonical_executable(path: &Path, cwd: &Path) -> Option { - let resolved = if path.is_absolute() { path.to_path_buf() } else { cwd.join(path) }; - let canonical = tokio::fs::canonicalize(resolved).await.ok()?; +/// Resolve and validate an operator-selected executable before handing it to +/// `Command`. Relative sources are rejected because the relay's launch cwd can +/// be caller-controlled. Keeping the canonical absolute path in `CmuxTui` +/// avoids a second PATH lookup after validation. +async fn canonical_executable(path: &Path) -> Option { + if !path.is_absolute() { + return None; + } + let canonical = tokio::fs::canonicalize(path).await.ok()?; is_executable(&canonical).await.then_some(canonical) } @@ -1055,7 +1058,7 @@ mod tests { } #[tokio::test] - async fn canonical_executable_resolves_relative_path_against_request_cwd() { + async fn canonical_executable_rejects_relative_path() { use std::os::unix::fs::PermissionsExt; let root = std::env::temp_dir().join(format!("cmux-relay-cwd-test-{}", std::process::id())); @@ -1068,8 +1071,8 @@ mod tests { .await .unwrap(); - let resolved = canonical_executable(Path::new("bin/cmux-tui"), &cwd).await; - assert_eq!(resolved, Some(std::fs::canonicalize(&executable).unwrap())); + let resolved = canonical_executable(Path::new("bin/cmux-tui")).await; + assert_eq!(resolved, None); let _ = tokio::fs::remove_dir_all(root).await; } @@ -1096,9 +1099,21 @@ mod tests { let mut env = HashMap::new(); env.insert("CHATMUX_RELAY_CMUX_TUI".to_owned(), "bin/cmux-tui".to_owned()); env.insert("PATH".to_owned(), "bin".to_owned()); - let deps = RealPtyDeps::new(env); + let mut deps = RealPtyDeps::new(env); + + assert!(deps.resolve_cmux_tui().await.is_none()); + + deps.env.remove("CHATMUX_RELAY_CMUX_TUI"); + assert!(deps.resolve_cmux_tui().await.is_none()); - assert!(deps.resolve_cmux_tui(&cwd).await.is_none()); + deps.env.insert( + "CHATMUX_RELAY_CMUX_TUI".to_owned(), + executable.to_string_lossy().into_owned(), + ); + assert_eq!( + deps.resolve_cmux_tui().await.map(|resolved| resolved.file), + Some(std::fs::canonicalize(&executable).unwrap().to_string_lossy().into_owned()) + ); let _ = tokio::fs::remove_dir_all(root).await; } diff --git a/cmux-tui/crates/chatmux-relay/src/tunnel_terminal.rs b/cmux-tui/crates/chatmux-relay/src/tunnel_terminal.rs index 317a61fa6146..a6ce9140b457 100644 --- a/cmux-tui/crates/chatmux-relay/src/tunnel_terminal.rs +++ b/cmux-tui/crates/chatmux-relay/src/tunnel_terminal.rs @@ -711,7 +711,7 @@ mod tests { self.spawned.lock().unwrap().push(pty.clone()); PtyHandle { control: Arc::new(pty.clone()), output: Arc::new(pty), banner: None } } - async fn resolve_cmux_tui(&self, _cwd: &Path) -> Option { + async fn resolve_cmux_tui(&self) -> Option { None } async fn ensure_daemon( From 45d4d55162549e7fac91b86ec9a499daff3bb014 Mon Sep 17 00:00:00 2001 From: Lawrence Chen <54008264+lawrencecchen@users.noreply.github.com> Date: Wed, 2 Sep 2026 04:34:37 -0700 Subject: [PATCH 6/6] fmt relay executable policy test --- cmux-tui/crates/chatmux-relay/src/pty_deps.rs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/cmux-tui/crates/chatmux-relay/src/pty_deps.rs b/cmux-tui/crates/chatmux-relay/src/pty_deps.rs index 1b6f4df82233..bf2d096b4732 100644 --- a/cmux-tui/crates/chatmux-relay/src/pty_deps.rs +++ b/cmux-tui/crates/chatmux-relay/src/pty_deps.rs @@ -1083,10 +1083,7 @@ mod tests { let root = std::env::temp_dir().join(format!( "cmux-relay-relative-executable-policy-{}-{}", std::process::id(), - std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap() - .as_nanos() + std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).unwrap().as_nanos() )); let cwd = root.join("launch"); let executable = cwd.join("bin/cmux-tui"); @@ -1106,10 +1103,8 @@ mod tests { deps.env.remove("CHATMUX_RELAY_CMUX_TUI"); assert!(deps.resolve_cmux_tui().await.is_none()); - deps.env.insert( - "CHATMUX_RELAY_CMUX_TUI".to_owned(), - executable.to_string_lossy().into_owned(), - ); + deps.env + .insert("CHATMUX_RELAY_CMUX_TUI".to_owned(), executable.to_string_lossy().into_owned()); assert_eq!( deps.resolve_cmux_tui().await.map(|resolved| resolved.file), Some(std::fs::canonicalize(&executable).unwrap().to_string_lossy().into_owned())