From 11bd91e876c87381d95d33a12513cdcbc3964e5f Mon Sep 17 00:00:00 2001 From: Edward Frazer Date: Fri, 17 Apr 2026 16:26:52 -0700 Subject: [PATCH] fix: skip SSH config dependency roots in Windows sandbox --- codex-rs/Cargo.lock | 1 + codex-rs/Cargo.toml | 1 + codex-rs/windows-sandbox-rs/Cargo.toml | 1 + codex-rs/windows-sandbox-rs/src/lib.rs | 3 + .../src/setup_orchestrator.rs | 32 ++ .../src/ssh_config_dependencies.rs | 277 ++++++++++++++++++ 6 files changed, 315 insertions(+) create mode 100644 codex-rs/windows-sandbox-rs/src/ssh_config_dependencies.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index ab65fc0a4d..eb1d7c8758 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -3275,6 +3275,7 @@ dependencies = [ "codex-utils-string", "dirs-next", "dunce", + "glob", "pretty_assertions", "rand 0.8.5", "serde", diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index ed6bdc0fd6..d10716ce4f 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -242,6 +242,7 @@ env_logger = "0.11.9" eventsource-stream = "0.2.3" futures = { version = "0.3", default-features = false } gethostname = "1.1.0" +glob = "0.3" globset = "0.4" hmac = "0.12.1" http = "1.3.1" diff --git a/codex-rs/windows-sandbox-rs/Cargo.toml b/codex-rs/windows-sandbox-rs/Cargo.toml index 6d49f81a31..e7ca02fed2 100644 --- a/codex-rs/windows-sandbox-rs/Cargo.toml +++ b/codex-rs/windows-sandbox-rs/Cargo.toml @@ -31,6 +31,7 @@ codex-utils-pty = { workspace = true } codex-utils-absolute-path = { workspace = true } codex-utils-string = { workspace = true } dunce = "1.0" +glob = { workspace = true } serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" tempfile = "3" diff --git a/codex-rs/windows-sandbox-rs/src/lib.rs b/codex-rs/windows-sandbox-rs/src/lib.rs index 90176b08ea..3b9601b16f 100644 --- a/codex-rs/windows-sandbox-rs/src/lib.rs +++ b/codex-rs/windows-sandbox-rs/src/lib.rs @@ -2,6 +2,9 @@ // from the eventual unsafe cleanup. #![allow(unsafe_op_in_unsafe_fn)] +#[cfg(any(target_os = "windows", test))] +mod ssh_config_dependencies; + macro_rules! windows_modules { ($($name:ident),+ $(,)?) => { $(#[cfg(target_os = "windows")] mod $name;)+ diff --git a/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs b/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs index 06669fff92..c5877a8178 100644 --- a/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs +++ b/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs @@ -21,6 +21,7 @@ use crate::setup_error::SetupFailure; use crate::setup_error::clear_setup_error_report; use crate::setup_error::failure; use crate::setup_error::read_setup_error_report; +use crate::ssh_config_dependencies::ssh_config_dependency_profile_entry_names; use anyhow::Context; use anyhow::Result; use anyhow::anyhow; @@ -328,6 +329,7 @@ fn profile_read_roots(user_profile: &Path) -> Vec { Ok(entries) => entries, Err(_) => return vec![user_profile.to_path_buf()], }; + let ssh_dependency_entry_names = ssh_config_dependency_profile_entry_names(user_profile); entries .filter_map(Result::ok) @@ -337,6 +339,9 @@ fn profile_read_roots(user_profile: &Path) -> Vec { !USERPROFILE_READ_ROOT_EXCLUSIONS .iter() .any(|excluded| name.eq_ignore_ascii_case(excluded)) + && !ssh_dependency_entry_names + .iter() + .any(|excluded| name.eq_ignore_ascii_case(excluded)) }) .map(|(_, path)| path) .collect() @@ -1048,6 +1053,33 @@ mod tests { assert_eq!(expected, actual); } + #[test] + fn profile_read_roots_excludes_ssh_config_dependency_entries() { + let tmp = TempDir::new().expect("tempdir"); + let user_profile = tmp.path(); + let allowed_dir = user_profile.join("Documents"); + let ssh_dir = user_profile.join(".ssh"); + let key_dir = user_profile.join(".keys"); + let include_dir = user_profile.join(".included"); + + fs::create_dir_all(&allowed_dir).expect("create allowed dir"); + fs::create_dir_all(&ssh_dir).expect("create .ssh"); + fs::create_dir_all(&key_dir).expect("create key dir"); + fs::create_dir_all(&include_dir).expect("create include dir"); + fs::write( + ssh_dir.join("config"), + "IdentityFile ~/.keys/id_ed25519\nInclude ~/.included/config\n", + ) + .expect("write ssh config"); + fs::write(include_dir.join("config"), "User git\n").expect("write included config"); + + let roots = profile_read_roots(user_profile); + let actual: HashSet = roots.into_iter().collect(); + let expected: HashSet = [allowed_dir].into_iter().collect(); + + assert_eq!(expected, actual); + } + #[test] fn profile_read_roots_falls_back_to_profile_root_when_enumeration_fails() { let tmp = TempDir::new().expect("tempdir"); diff --git a/codex-rs/windows-sandbox-rs/src/ssh_config_dependencies.rs b/codex-rs/windows-sandbox-rs/src/ssh_config_dependencies.rs new file mode 100644 index 0000000000..0fdc8c938d --- /dev/null +++ b/codex-rs/windows-sandbox-rs/src/ssh_config_dependencies.rs @@ -0,0 +1,277 @@ +use std::collections::BTreeSet; +use std::collections::HashSet; +use std::path::Path; +use std::path::PathBuf; + +const FILE_DIRECTIVES: &[&str] = &[ + "certificatefile", + "controlpath", + "globalknownhostsfile", + "identityagent", + "identityfile", + "pkcs11provider", + "revokedhostkeys", + "securitykeyprovider", + "userknownhostsfile", + "xauthlocation", +]; +const COMMAND_DIRECTIVES: &[&str] = &["knownhostscommand", "localcommand", "proxycommand"]; + +pub(crate) fn ssh_config_dependency_profile_entry_names(user_profile: &Path) -> BTreeSet { + let ssh_dir = user_profile.join(".ssh"); + let mut entries = BTreeSet::from([".ssh".to_string()]); + visit_config( + &ssh_dir.join("config"), + user_profile, + &ssh_dir, + &mut HashSet::new(), + &mut entries, + 0, + ); + entries +} + +fn visit_config( + path: &Path, + user_profile: &Path, + ssh_dir: &Path, + visited: &mut HashSet, + entries: &mut BTreeSet, + depth: usize, +) { + if depth == 32 { + return; + } + let key = std::fs::canonicalize(path).unwrap_or_else(|_| path.to_path_buf()); + if !visited.insert(key) { + return; + } + + let Ok(contents) = std::fs::read_to_string(path) else { + return; + }; + for (key, args) in contents.lines().filter_map(directive) { + match key.to_ascii_lowercase().as_str() { + "include" => { + for arg in args { + for include in include_paths(&arg, user_profile, ssh_dir) { + record_profile_entry(user_profile, &include, entries); + visit_config(&include, user_profile, ssh_dir, visited, entries, depth + 1); + } + } + } + key if FILE_DIRECTIVES.contains(&key) => { + for arg in args { + if let Some(path) = profile_path_arg(&arg, user_profile, None) { + record_profile_entry(user_profile, &path, entries); + } + } + } + key if COMMAND_DIRECTIVES.contains(&key) => { + for arg in args { + for word in words(&arg) { + if let Some(path) = profile_path_arg(&word, user_profile, None) { + record_profile_entry(user_profile, &path, entries); + } + } + } + } + _ => {} + } + } +} + +fn include_paths(arg: &str, user_profile: &Path, ssh_dir: &Path) -> Vec { + let Some(pattern_path) = profile_path_arg(arg, user_profile, Some(ssh_dir)) else { + return Vec::new(); + }; + let pattern = pattern_path.to_string_lossy(); + let Ok(paths) = glob::glob(&pattern) else { + return vec![glob_parent(pattern_path)]; + }; + let paths: Vec = paths.filter_map(Result::ok).collect(); + if paths.is_empty() { + vec![glob_parent(pattern_path)] + } else { + paths + } +} + +fn directive(line: &str) -> Option<(String, Vec)> { + let mut words = words(line); + let first = words.first()?.clone(); + if let Some((key, value)) = first.split_once('=') + && !key.is_empty() + { + let mut args = Vec::new(); + if !value.is_empty() { + args.push(value.to_string()); + } + args.extend(words.drain(1..)); + Some((key.to_string(), args)) + } else { + Some((words.remove(0), words)) + } +} + +fn words(line: &str) -> Vec { + let mut out = Vec::new(); + let mut word = String::new(); + let mut quote = false; + let mut chars = line.chars(); + + while let Some(ch) = chars.next() { + match ch { + '#' if !quote => break, + '"' => quote = !quote, + '\\' if quote => word.extend(chars.next()), + ch if ch.is_whitespace() && !quote => { + if !word.is_empty() { + out.push(std::mem::take(&mut word)); + } + } + ch => word.push(ch), + } + } + if !word.is_empty() { + out.push(word); + } + out +} + +fn profile_path_arg( + arg: &str, + user_profile: &Path, + relative_base: Option<&Path>, +) -> Option { + if arg.eq_ignore_ascii_case("none") { + return None; + } + if arg == "~" || arg == "%d" || arg == "${HOME}" { + return Some(user_profile.to_path_buf()); + } + if let Some(rest) = arg + .strip_prefix("~/") + .or_else(|| arg.strip_prefix(r"~\")) + .or_else(|| arg.strip_prefix("%d/")) + .or_else(|| arg.strip_prefix(r"%d\")) + .or_else(|| arg.strip_prefix("${HOME}/")) + .or_else(|| arg.strip_prefix(r"${HOME}\")) + { + return Some(user_profile.join(rest)); + } + + let path = PathBuf::from(arg); + if path.is_absolute() { + Some(path) + } else { + relative_base.map(|base| base.join(path)) + } +} + +fn record_profile_entry(user_profile: &Path, path: &Path, entries: &mut BTreeSet) { + let profile = user_profile.to_string_lossy().replace('\\', "/"); + let path = path.to_string_lossy().replace('\\', "/"); + let profile = profile.trim_end_matches('/'); + let relative = if path.eq_ignore_ascii_case(profile) { + "" + } else { + let prefix = format!("{profile}/"); + path.strip_prefix(&prefix).unwrap_or_default() + }; + if let Some(entry) = relative.split('/').find(|part| !part.is_empty()) { + entries.insert(entry.to_string()); + } +} + +fn glob_parent(path: PathBuf) -> PathBuf { + let path = path.to_string_lossy(); + PathBuf::from( + path.split(['*', '?', '[']) + .next() + .unwrap_or_default() + .trim_end_matches(['/', '\\']), + ) +} + +#[cfg(test)] +mod tests { + use super::ssh_config_dependency_profile_entry_names; + use pretty_assertions::assert_eq; + use std::collections::BTreeSet; + use std::fs; + use tempfile::TempDir; + + #[test] + fn collects_file_directive_profile_entries() { + let tmp = TempDir::new().expect("tempdir"); + let home = tmp.path(); + fs::create_dir_all(home.join(".ssh")).expect("create .ssh"); + fs::write( + home.join(".ssh/config"), + r#" +Host devbox + IdentityFile ~/.keys/id_ed25519 + CertificateFile %d/.certs/devbox-cert.pub + UserKnownHostsFile ${HOME}/.known_hosts_custom + ControlPath ~/.ssh/control-%h-%p-%r +"#, + ) + .expect("write config"); + + assert_eq!( + BTreeSet::from([ + ".certs".to_string(), + ".keys".to_string(), + ".known_hosts_custom".to_string(), + ".ssh".to_string(), + ]), + ssh_config_dependency_profile_entry_names(home) + ); + } + + #[test] + fn recursively_collects_include_dependencies() { + let tmp = TempDir::new().expect("tempdir"); + let home = tmp.path(); + let ssh_dir = home.join(".ssh"); + fs::create_dir_all(ssh_dir.join("conf.d")).expect("create conf.d"); + fs::write(ssh_dir.join("config"), "Include conf.d/*.conf\n").expect("write config"); + fs::write( + ssh_dir.join("conf.d/devbox.conf"), + "CertificateFile ~/.included/devbox-cert.pub\n", + ) + .expect("write include"); + + assert_eq!( + BTreeSet::from([".included".to_string(), ".ssh".to_string()]), + ssh_config_dependency_profile_entry_names(home) + ); + } + + #[test] + fn command_directives_only_record_explicit_profile_paths() { + let tmp = TempDir::new().expect("tempdir"); + let home = tmp.path(); + fs::create_dir_all(home.join(".ssh")).expect("create .ssh"); + fs::write( + home.join(".ssh/config"), + r#" +Host devbox + ProxyCommand ~/.helpers/proxy --state ${HOME}/.proxy-state %h %p + KnownHostsCommand "%d/.known-hosts/bin" %H +"#, + ) + .expect("write config"); + + assert_eq!( + BTreeSet::from([ + ".helpers".to_string(), + ".known-hosts".to_string(), + ".proxy-state".to_string(), + ".ssh".to_string(), + ]), + ssh_config_dependency_profile_entry_names(home) + ); + } +}