mirror of
https://github.com/openai/codex.git
synced 2026-10-18 17:18:52 +00:00
Preserve zsh tied PATH exports in shell snapshots (#34293)
## Why zsh prints tied scalar/array exports such as `PATH` and `path` with `-T` metadata. The snapshot filter rejected that syntax, so commands available only through the configured `PATH` could not be resolved from a restored snapshot. ## What changed - Recognize writable tied exports and retain them in zsh shell snapshots. - Skip readonly exports so restoring a snapshot does not make variables such as `PATH` immutable. ## Testing Added macOS coverage for restoring a tied `PATH`, handling entries with spaces, deduplicating path entries, excluding readonly tied exports, and resolving a command available only through the restored snapshot. GitOrigin-RevId: 297cfae3368230c7f6fe5ef84f0a10b35a2c802c
This commit is contained in:
committed by
copyberry
parent
bf3c1972b7
commit
2deed3fb9c
@@ -342,6 +342,13 @@ export_lines=$(export -p | awk '
|
||||
line=$0
|
||||
name=line
|
||||
sub(/^(export|declare -x|typeset -x) /, "", name)
|
||||
if (name ~ /^-[A-Za-z]*r[A-Za-z]* /) {
|
||||
next
|
||||
}
|
||||
if (name ~ /^-[A-Za-z]*T[A-Za-z]* /) {
|
||||
sub(/^-[A-Za-z]*T[A-Za-z]* /, "", name)
|
||||
sub(/ [A-Za-z_][A-Za-z0-9_]*=.*/, "", name)
|
||||
}
|
||||
sub(/=.*/, "", name)
|
||||
if (name ~ /^(EXCLUDED_EXPORTS)$/) {
|
||||
next
|
||||
|
||||
@@ -187,6 +187,99 @@ fn bash_snapshot_preserves_multiline_exports() -> Result<()> {
|
||||
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(zsh_snapshot_script())
|
||||
.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(zsh_snapshot_script())
|
||||
.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<()> {
|
||||
|
||||
@@ -1,4 +1,6 @@
|
||||
use anyhow::Result;
|
||||
#[cfg(target_os = "macos")]
|
||||
use codex_core::shell::get_shell_by_model_provided_path;
|
||||
use codex_features::Feature;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
@@ -22,6 +24,8 @@ use core_test_support::wait_for_event_match;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
use std::collections::HashMap;
|
||||
#[cfg(target_os = "macos")]
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use tokio::fs;
|
||||
@@ -676,6 +680,74 @@ async fn shell_snapshot_deleted_after_shutdown_with_skills() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(target_os = "macos")]
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn macos_shell_command_resolves_command_from_tied_path_snapshot() -> Result<()> {
|
||||
let builder = test_codex()
|
||||
.with_user_shell(get_shell_by_model_provided_path(&PathBuf::from("/bin/zsh")))
|
||||
.with_config(|config| {
|
||||
config
|
||||
.features
|
||||
.enable(Feature::ShellSnapshot)
|
||||
.expect("test config should allow feature update");
|
||||
});
|
||||
let harness = TestCodexHarness::with_builder(builder).await?;
|
||||
let command_dir = harness
|
||||
.test()
|
||||
.config
|
||||
.cwd
|
||||
.join("path with spaces")
|
||||
.join("bin");
|
||||
fs::create_dir_all(&command_dir).await?;
|
||||
let command_path = command_dir.join("snapshot-only-command");
|
||||
fs::write(&command_path, "#!/bin/sh\nprintf tied-path-command").await?;
|
||||
let mut permissions = fs::metadata(&command_path).await?.permissions();
|
||||
permissions.set_mode(0o755);
|
||||
fs::set_permissions(&command_path, permissions).await?;
|
||||
|
||||
run_tool_turn_on_harness(
|
||||
&harness,
|
||||
"warm up the tied PATH shell snapshot",
|
||||
"shell-snapshot-tied-path-warmup",
|
||||
"shell_command",
|
||||
json!({
|
||||
"command": "printf warmup",
|
||||
"timeout_ms": 5_000,
|
||||
}),
|
||||
)
|
||||
.await?;
|
||||
let snapshot_path = wait_for_snapshot(harness.test().home.path()).await?;
|
||||
fs::write(
|
||||
&snapshot_path,
|
||||
format!(
|
||||
"# Snapshot file\nexport -UT PATH path=('{}' /usr/bin /bin)\n",
|
||||
command_dir.display()
|
||||
),
|
||||
)
|
||||
.await?;
|
||||
|
||||
let end = run_tool_turn_on_harness(
|
||||
&harness,
|
||||
"resolve a command from the tied PATH snapshot",
|
||||
"shell-snapshot-tied-path",
|
||||
"shell_command",
|
||||
json!({
|
||||
"command": "snapshot-only-command",
|
||||
"timeout_ms": 5_000,
|
||||
}),
|
||||
)
|
||||
.await?;
|
||||
|
||||
assert_eq!(
|
||||
end.exit_code, 0,
|
||||
"tied-path command failed: stderr={:?}",
|
||||
end.stderr
|
||||
);
|
||||
assert_eq!(normalize_newlines(&end.stdout).trim(), "tied-path-command");
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg_attr(not(target_os = "macos"), ignore)]
|
||||
#[cfg_attr(
|
||||
target_os = "macos",
|
||||
|
||||
Reference in New Issue
Block a user