diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index b123824e68..c2bb6a331a 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -4133,6 +4133,7 @@ dependencies = [ "serde", "serde_json", "shlex", + "tempfile", "tree-sitter", "tree-sitter-bash", "tree-sitter-powershell", diff --git a/codex-rs/core/src/shell_snapshot_tests.rs b/codex-rs/core/src/shell_snapshot_tests.rs index 2752bf8b6c..f27e27d446 100644 --- a/codex-rs/core/src/shell_snapshot_tests.rs +++ b/codex-rs/core/src/shell_snapshot_tests.rs @@ -5,8 +5,6 @@ use pretty_assertions::assert_eq; #[cfg(unix)] use std::os::unix::ffi::OsStrExt; use std::path::PathBuf; -#[cfg(unix)] -use std::process::Command; #[cfg(target_os = "linux")] use std::process::Command as StdCommand; @@ -123,163 +121,6 @@ fn snapshot_file_name_parser_supports_legacy_and_suffixed_names() { ); } -#[cfg(unix)] -#[test] -fn bash_snapshot_filters_invalid_exports() -> Result<()> { - let output = Command::new("/bin/bash") - .arg("-c") - .arg(snapshot_script(ShellType::Bash).expect("bash supports snapshots")) - .env("BASH_ENV", "/dev/null") - .env("VALID_NAME", "ok") - .env("PWD", "/tmp/stale") - .env("NEXTEST_BIN_EXE_codex-write-config-schema", "/path/to/bin") - .env("BAD-NAME", "broken") - .output()?; - - assert!(output.status.success()); - - let stdout = String::from_utf8_lossy(&output.stdout); - assert!(stdout.contains("VALID_NAME")); - assert!(!stdout.contains("PWD=/tmp/stale")); - assert!(!stdout.contains("NEXTEST_BIN_EXE_codex-write-config-schema")); - assert!(!stdout.contains("BAD-NAME")); - - Ok(()) -} - -#[cfg(unix)] -#[test] -fn bash_snapshot_preserves_multiline_exports() -> Result<()> { - let multiline_cert = "-----BEGIN CERTIFICATE-----\nabc\n-----END CERTIFICATE-----"; - let output = Command::new("/bin/bash") - .arg("-c") - .arg(snapshot_script(ShellType::Bash).expect("bash supports snapshots")) - .env("BASH_ENV", "/dev/null") - .env("MULTILINE_CERT", multiline_cert) - .output()?; - - assert!(output.status.success()); - - let stdout = String::from_utf8_lossy(&output.stdout); - assert!( - stdout.contains("MULTILINE_CERT=") || stdout.contains("MULTILINE_CERT"), - "snapshot should include the multiline export name" - ); - - let dir = tempdir()?; - let snapshot_path = dir.path().join("snapshot.sh"); - std::fs::write(&snapshot_path, stdout.as_bytes())?; - - let validate = Command::new("/bin/bash") - .arg("-c") - .arg("set -e; . \"$1\"") - .arg("bash") - .arg(&snapshot_path) - .env("BASH_ENV", "/dev/null") - .output()?; - - assert!( - validate.status.success(), - "snapshot validation failed: {}", - String::from_utf8_lossy(&validate.stderr) - ); - - Ok(()) -} - -#[cfg(target_os = "macos")] -#[test] -fn zsh_snapshot_restores_tied_path() -> Result<()> { - let dir = tempdir()?; - let path_with_spaces = dir.path().join("path with spaces").join("bin"); - let plain_path = dir.path().join("plain-path").join("bin"); - let expected_path = format!( - "{}:{}:/usr/bin:/bin", - path_with_spaces.display(), - plain_path.display() - ); - let zshrc = format!( - "export -UT PATH path=('{}' '{}' '{}' /usr/bin /bin)\n", - path_with_spaces.display(), - plain_path.display(), - plain_path.display() - ); - std::fs::write(dir.path().join(".zshrc"), zshrc)?; - - let snapshot = Command::new("/bin/zsh") - .arg("-f") - .arg("-c") - .arg(snapshot_script(ShellType::Zsh).expect("zsh supports snapshots")) - .env_clear() - .env("PATH", "/usr/bin:/bin") - .env("ZDOTDIR", dir.path()) - .output()?; - assert!(snapshot.status.success()); - - let snapshot_path = dir.path().join("snapshot.sh"); - std::fs::write(&snapshot_path, &snapshot.stdout)?; - - let restored = Command::new("/bin/zsh") - .arg("-f") - .arg("-c") - .arg("set -e; . \"$1\"; print -r -- \"$PATH\"") - .arg("zsh") - .arg(&snapshot_path) - .env_clear() - .env("PATH", "/usr/bin:/bin") - .output()?; - assert!(restored.status.success()); - assert_eq!( - String::from_utf8(restored.stdout)?.trim_end(), - expected_path - ); - - let snapshot = String::from_utf8(snapshot.stdout)?; - assert!( - snapshot - .lines() - .any(|line| line.starts_with("export -UT PATH path=")), - "snapshot should capture the tied PATH export" - ); - - std::fs::write(dir.path().join(".zshrc"), "readonly PATH\n")?; - let readonly_snapshot = Command::new("/bin/zsh") - .arg("-f") - .arg("-c") - .arg(snapshot_script(ShellType::Zsh).expect("zsh supports snapshots")) - .env_clear() - .env("PATH", "/usr/bin:/bin") - .env("ZDOTDIR", dir.path()) - .output()?; - assert!(readonly_snapshot.status.success()); - std::fs::write(&snapshot_path, &readonly_snapshot.stdout)?; - - let readonly_restored = Command::new("/bin/zsh") - .arg("-f") - .arg("-c") - .arg("set -e; . \"$1\"; export PATH='/codex-path':\"$PATH\"; print -r -- \"$PATH\"") - .arg("zsh") - .arg(&snapshot_path) - .env_clear() - .env("PATH", "/usr/bin:/bin") - .output()?; - assert!(readonly_restored.status.success()); - assert_eq!( - String::from_utf8(readonly_restored.stdout)?.trim_end(), - "/codex-path:/usr/bin:/bin" - ); - - let readonly_snapshot = String::from_utf8(readonly_snapshot.stdout)?; - assert!( - !readonly_snapshot - .lines() - .any(|line| line.starts_with("export -rT PATH path=")), - "snapshot should not capture the readonly tied PATH export" - ); - - Ok(()) -} - #[cfg(unix)] #[tokio::test] async fn try_create_creates_and_deletes_snapshot_file() -> Result<()> { diff --git a/codex-rs/shell-command/Cargo.toml b/codex-rs/shell-command/Cargo.toml index c5947d9155..999295ec31 100644 --- a/codex-rs/shell-command/Cargo.toml +++ b/codex-rs/shell-command/Cargo.toml @@ -26,6 +26,7 @@ which = { workspace = true } [dev-dependencies] anyhow = { workspace = true } pretty_assertions = { workspace = true } +tempfile = { workspace = true } [lib] doctest = false diff --git a/codex-rs/shell-command/src/shell_snapshot.rs b/codex-rs/shell-command/src/shell_snapshot.rs index 9eac209bc4..62e4af7837 100644 --- a/codex-rs/shell-command/src/shell_snapshot.rs +++ b/codex-rs/shell-command/src/shell_snapshot.rs @@ -1,5 +1,9 @@ use crate::shell_detect::ShellType; +#[cfg(all(test, unix))] +#[path = "shell_snapshot_tests.rs"] +mod tests; + const EXCLUDED_EXPORT_VARS: &[&str] = &["PWD", "OLDPWD"]; /// Returns the shell-native script used to capture restorable shell state. diff --git a/codex-rs/shell-command/src/shell_snapshot_tests.rs b/codex-rs/shell-command/src/shell_snapshot_tests.rs new file mode 100644 index 0000000000..b280c54e34 --- /dev/null +++ b/codex-rs/shell-command/src/shell_snapshot_tests.rs @@ -0,0 +1,162 @@ +use super::snapshot_script; +use crate::shell_detect::ShellType; +use anyhow::Result; +#[cfg(target_os = "macos")] +use pretty_assertions::assert_eq; +use std::process::Command; +use tempfile::tempdir; + +#[test] +fn bash_snapshot_filters_invalid_exports() -> Result<()> { + let output = Command::new("/bin/bash") + .arg("-c") + .arg(snapshot_script(ShellType::Bash).expect("bash supports snapshots")) + .env("BASH_ENV", "/dev/null") + .env("VALID_NAME", "ok") + .env("PWD", "/tmp/stale") + .env("NEXTEST_BIN_EXE_codex-write-config-schema", "/path/to/bin") + .env("BAD-NAME", "broken") + .output()?; + + assert!(output.status.success()); + + let stdout = String::from_utf8_lossy(&output.stdout); + assert!(stdout.contains("VALID_NAME")); + assert!(!stdout.contains("PWD=/tmp/stale")); + assert!(!stdout.contains("NEXTEST_BIN_EXE_codex-write-config-schema")); + assert!(!stdout.contains("BAD-NAME")); + + Ok(()) +} + +#[test] +fn bash_snapshot_preserves_multiline_exports() -> Result<()> { + let multiline_cert = "-----BEGIN CERTIFICATE-----\nabc\n-----END CERTIFICATE-----"; + let output = Command::new("/bin/bash") + .arg("-c") + .arg(snapshot_script(ShellType::Bash).expect("bash supports snapshots")) + .env("BASH_ENV", "/dev/null") + .env("MULTILINE_CERT", multiline_cert) + .output()?; + + assert!(output.status.success()); + + let stdout = String::from_utf8_lossy(&output.stdout); + assert!( + stdout.contains("MULTILINE_CERT=") || stdout.contains("MULTILINE_CERT"), + "snapshot should include the multiline export name" + ); + + let dir = tempdir()?; + let snapshot_path = dir.path().join("snapshot.sh"); + std::fs::write(&snapshot_path, stdout.as_bytes())?; + + let validate = Command::new("/bin/bash") + .arg("-c") + .arg("set -e; . \"$1\"") + .arg("bash") + .arg(&snapshot_path) + .env("BASH_ENV", "/dev/null") + .output()?; + + assert!( + validate.status.success(), + "snapshot validation failed: {}", + String::from_utf8_lossy(&validate.stderr) + ); + + Ok(()) +} + +#[cfg(target_os = "macos")] +#[test] +fn zsh_snapshot_restores_tied_path() -> Result<()> { + let dir = tempdir()?; + let path_with_spaces = dir.path().join("path with spaces").join("bin"); + let plain_path = dir.path().join("plain-path").join("bin"); + let expected_path = format!( + "{}:{}:/usr/bin:/bin", + path_with_spaces.display(), + plain_path.display() + ); + let zshrc = format!( + "export -UT PATH path=('{}' '{}' '{}' /usr/bin /bin)\n", + path_with_spaces.display(), + plain_path.display(), + plain_path.display() + ); + std::fs::write(dir.path().join(".zshrc"), zshrc)?; + + let snapshot = Command::new("/bin/zsh") + .arg("-f") + .arg("-c") + .arg(snapshot_script(ShellType::Zsh).expect("zsh supports snapshots")) + .env_clear() + .env("PATH", "/usr/bin:/bin") + .env("ZDOTDIR", dir.path()) + .output()?; + assert!(snapshot.status.success()); + + let snapshot_path = dir.path().join("snapshot.sh"); + std::fs::write(&snapshot_path, &snapshot.stdout)?; + + let restored = Command::new("/bin/zsh") + .arg("-f") + .arg("-c") + .arg("set -e; . \"$1\"; print -r -- \"$PATH\"") + .arg("zsh") + .arg(&snapshot_path) + .env_clear() + .env("PATH", "/usr/bin:/bin") + .output()?; + assert!(restored.status.success()); + assert_eq!( + String::from_utf8(restored.stdout)?.trim_end(), + expected_path + ); + + let snapshot = String::from_utf8(snapshot.stdout)?; + assert!( + snapshot + .lines() + .any(|line| line.starts_with("export -UT PATH path=")), + "snapshot should capture the tied PATH export" + ); + + std::fs::write(dir.path().join(".zshrc"), "readonly PATH\n")?; + let readonly_snapshot = Command::new("/bin/zsh") + .arg("-f") + .arg("-c") + .arg(snapshot_script(ShellType::Zsh).expect("zsh supports snapshots")) + .env_clear() + .env("PATH", "/usr/bin:/bin") + .env("ZDOTDIR", dir.path()) + .output()?; + assert!(readonly_snapshot.status.success()); + std::fs::write(&snapshot_path, &readonly_snapshot.stdout)?; + + let readonly_restored = Command::new("/bin/zsh") + .arg("-f") + .arg("-c") + .arg("set -e; . \"$1\"; export PATH='/codex-path':\"$PATH\"; print -r -- \"$PATH\"") + .arg("zsh") + .arg(&snapshot_path) + .env_clear() + .env("PATH", "/usr/bin:/bin") + .output()?; + assert!(readonly_restored.status.success()); + assert_eq!( + String::from_utf8(readonly_restored.stdout)?.trim_end(), + "/codex-path:/usr/bin:/bin" + ); + + let readonly_snapshot = String::from_utf8(readonly_snapshot.stdout)?; + assert!( + !readonly_snapshot + .lines() + .any(|line| line.starts_with("export -rT PATH path=")), + "snapshot should not capture the readonly tied PATH export" + ); + + Ok(()) +}