openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-25 18:28:38 +00:00
Code Issues Packages Projects Releases Wiki Activity

Preserve complete shell snapshot exports through filtering and replay (#43907)

Browse Source 
## Why

Line-based export parsing can truncate multiline values or mistake their contents for declarations. Snapshot replay also needs to restore Bash options before parsing functions that use extended glob syntax.

## What changed

- Capture shell state, aliases, and complete export records with NUL-delimited boundaries, separating capture, credential preparation, and rendering.
- Apply credential and environment policy to whole exports, preserving multiline values and making credential aliases follow allowed overrides.
- Restore Bash `shopt` options before functions, including for Bash running as `sh`.
- Support non-evaluating POSIX `ENV` path expansion and preserve native environment metadata and unset exports.
- Move core and exec-server snapshot consumers to the shared capture parser. Allow capture overhead while enforcing the exec-server's 512 KiB state-and-environment limit before filtering.

## Testing

Add regression coverage for complete capture records, multiline export filtering and replay, credential alias overrides, `ENV` expansion, and `PATH` export state. Extend exec-server tests to cover Bash-backed `sh`, `extglob`, `nocasematch`, `set -u`, and large environments.

GitOrigin-RevId: 7589ab4d137529a395dcee10b5162718de24a621
This commit is contained in:
Winston Howes
2026-09-08 20:41:01 +00:00
committed by copyberry
parent f046cf35df
commit 1530f828cb
11 changed files with 1472 additions and 300 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/exec-server/src/shell_snapshot.rs
+39 -32
View File
@@ -10,7 +10,10 @@ use codex_network_proxy::strip_managed_proxy_env;
use codex_protocol::config_types::ShellEnvironmentPolicyInherit;
use codex_protocol::shell_environment;
use codex_shell_command::shell_detect::ShellType;
use codex_shell_command::shell_snapshot::snapshot_state_and_environment_script;
use codex_shell_command::shell_snapshot::CapturedSnapshot;
use codex_shell_command::shell_snapshot::SnapshotCaptureOptions;
use codex_shell_command::shell_snapshot::SnapshotStartup;
use codex_shell_command::shell_snapshot::snapshot_capture_script;
use codex_utils_path_uri::PathUri;
use tokio::io::AsyncReadExt;
use tokio::process::Command;
@@ -30,6 +33,8 @@ use crate::telemetry::ExecServerTelemetry;
const MAX_CACHED_SNAPSHOTS: usize = 16;
const MAX_SNAPSHOT_BYTES: usize = 512 * 1024;
// Capture also includes quoted export records and an optional pre-startup environment.
const MAX_SNAPSHOT_CAPTURE_BYTES: usize = 8 * MAX_SNAPSHOT_BYTES;
const MAX_SNAPSHOT_ENV_VALUE_BYTES: usize = 60 * 1024;
const MAX_SNAPSHOT_SCOPE_BYTES: usize = 256;
const SNAPSHOT_TIMEOUT: Duration = Duration::from_secs(10);
@@ -255,7 +260,15 @@ async fn capture_snapshot(
prepared: &PreparedExecRequest,
shell_type: ShellType,
) -> CaptureResult {
let script = snapshot_state_and_environment_script(shell_type).ok_or_else(|| {
let script = snapshot_capture_script(
shell_type,
SnapshotCaptureOptions {
startup: SnapshotStartup::Interactive,
declarations: false,
environment: true,
},
)
.ok_or_else(|| {
(
"unsupported_shell",
invalid_params("unsupported shell snapshot script".to_string()),
@@ -299,7 +312,7 @@ async fn capture_snapshot(
let capture = async {
let mut output = Vec::new();
stdout
.take((MAX_SNAPSHOT_BYTES + 1) as u64)
.take((MAX_SNAPSHOT_CAPTURE_BYTES + 1) as u64)
.read_to_end(&mut output)
.await
.map_err(|err| {
@@ -308,10 +321,12 @@ async fn capture_snapshot(
internal_error(format!("cannot read shell snapshot: {err}")),
)
})?;
if output.len() > MAX_SNAPSHOT_BYTES {
if output.len() > MAX_SNAPSHOT_CAPTURE_BYTES {
return Err((
"too_large",
internal_error(format!("shell snapshot exceeds {MAX_SNAPSHOT_BYTES} bytes")),
internal_error(format!(
"shell snapshot capture exceeds {MAX_SNAPSHOT_CAPTURE_BYTES} bytes"
)),
));
}
let status = child.wait().await.map_err(|err| {
@@ -337,35 +352,30 @@ async fn capture_snapshot(
)
})??;
parse_snapshot(&output, params.env_policy.as_ref())
parse_snapshot(shell_type, &output, params.env_policy.as_ref())
}
fn parse_snapshot(output: &[u8], env_policy: Option<&ExecEnvPolicy>) -> CaptureResult {
let separator = output.iter().position(|byte| *byte == 0).ok_or_else(|| {
fn parse_snapshot(
shell_type: ShellType,
output: &[u8],
env_policy: Option<&ExecEnvPolicy>,
) -> CaptureResult {
let captured = CapturedSnapshot::parse(shell_type, output).ok_or_else(|| {
(
"missing_environment",
internal_error("shell snapshot is missing its environment".to_string()),
)
})?;
let state = &output[..separator];
let marker = b"# Snapshot file";
let start = state
.windows(marker.len())
.position(|window| window == marker)
.ok_or_else(|| {
(
"missing_state",
internal_error("shell snapshot is missing its state marker".to_string()),
)
})?;
let state = std::str::from_utf8(&state[start..]).map_err(|err| {
(
"invalid_utf8",
internal_error(format!("shell snapshot state is not UTF-8: {err}")),
"invalid_capture",
internal_error("invalid shell snapshot capture".to_string()),
)
})?;
let state = captured.render_state();
if state.len().saturating_add(captured.environment.len()) > MAX_SNAPSHOT_BYTES {
return Err((
"too_large",
internal_error(format!("shell snapshot exceeds {MAX_SNAPSHOT_BYTES} bytes")),
));
}
let mut environment = output[separator + 1..]
let mut environment = captured
.environment
.split(|byte| *byte == 0)
.filter(|entry| !entry.is_empty())
.filter_map(|entry| {
@@ -388,10 +398,7 @@ fn parse_snapshot(output: &[u8], env_policy: Option<&ExecEnvPolicy>) -> CaptureR
environment.remove("OLDPWD");
environment.retain(|name, _| !shell_environment::is_non_inheritable_env_var(name));
Ok(ShellSnapshot {
state: state.to_string(),
environment,
})
Ok(ShellSnapshot { state, environment })
}
#[cfg(test)]
codex-rs/exec-server/src/shell_snapshot_tests.rs
+24 -6
View File
@@ -12,6 +12,7 @@ use test_case::test_case;
use super::CapturePurpose;
use super::MAX_SNAPSHOT_ATTEMPTS;
use super::MAX_SNAPSHOT_BYTES;
use super::SNAPSHOT_RETRY_BACKOFF;
use super::ShellSnapshotCache;
use super::parse_snapshot;
@@ -228,6 +229,20 @@ async fn snapshot_failure_retries_are_bounded_and_single_flight(
Ok(())
}
#[test]
fn snapshot_size_limit_counts_state_and_environment_before_filtering() {
let half = "x".repeat(MAX_SNAPSHOT_BYTES / 2);
let oversized = format!("# Snapshot file\n# {half}\n\0\0\0FILTERED={half}\0");
let policy = ExecEnvPolicy {
inherit: ShellEnvironmentPolicyInherit::All,
ignore_default_excludes: false,
exclude: vec!["FILTERED".to_string()],
r#set: HashMap::new(),
include_only: Vec::new(),
};
assert!(parse_snapshot(ShellType::Bash, oversized.as_bytes(), Some(&policy)).is_err());
}
#[test]
fn snapshot_filters_profile_exports_after_capture() {
let policy = ExecEnvPolicy {
@@ -238,7 +253,8 @@ fn snapshot_filters_profile_exports_after_capture() {
include_only: vec!["PROFILE_*".to_string()],
};
let snapshot = parse_snapshot(
b"profile noise\n# Snapshot file\nfunction profile_helper() { :; }\n\0PROFILE_ALLOWED=profile\0PROFILE_DENIED=denied\0PROFILE_SECRET=secret\0PWD=/tmp\0",
ShellType::Bash,
b"profile \xff noise\n# Snapshot file\nfunction profile_helper() { :; }\n\0alias profile_alias='profile_helper'\n\0PROFILE_DENIED\0export PROFILE_DENIED=denied\n\0NON_UTF8\0export NON_UTF8='\xff'\n\0\0PROFILE_ALLOWED=profile\0PROFILE_DENIED=denied\0PROFILE_SECRET=secret\0PWD=/tmp\0NON_UTF8=\xff\0",
Some(&policy),
)
.expect("snapshot should parse");
@@ -249,7 +265,7 @@ fn snapshot_filters_profile_exports_after_capture() {
);
assert_eq!(
snapshot.state,
"# Snapshot file\nfunction profile_helper() { :; }\n"
"# Snapshot file\nfunction profile_helper() { :; }\nalias profile_alias='profile_helper'\n"
);
}
@@ -267,7 +283,8 @@ fn snapshot_preserves_profile_exports_with_restrictive_inheritance() {
include_only: Vec::new(),
};
let snapshot = parse_snapshot(
b"# Snapshot file\n\0PROFILE_ALLOWED=profile\0SDKROOT=/sdk\0PROFILE_SECRET=secret\0PROFILE_DENIED=denied\0",
ShellType::Bash,
b"# Snapshot file\n\0\0\0PROFILE_ALLOWED=profile\0SDKROOT=/sdk\0PROFILE_SECRET=secret\0PROFILE_DENIED=denied\0",
Some(&policy),
)
.expect("snapshot should parse");
@@ -300,10 +317,11 @@ fn snapshot_caches_only_unmanaged_proxy_state() {
]),
),
] {
let output = format!("# Snapshot file\n\0{exports}");
let snapshot =
parse_snapshot(output.as_bytes(), /*env_policy*/ None).expect("snapshot should parse");
let output = format!("# Snapshot file\n\0\0\0{exports}");
let snapshot = parse_snapshot(ShellType::Bash, output.as_bytes(), /*env_policy*/ None)
.expect("snapshot should parse");
assert_eq!(snapshot.environment, expected);
}
}
use codex_shell_command::shell_detect::ShellType;
codex-rs/exec-server/tests/exec_process.rs
+36 -9
View File
@@ -1,7 +1,7 @@
mod common;
use std::collections::HashMap;
#[cfg(target_os = "linux")]
#[cfg(unix)]
use std::os::unix::fs::PermissionsExt;
use std::sync::Arc;
@@ -181,6 +181,7 @@ async fn codex_home_symlink_opt_out_respects_host_config_and_scope() -> Result<(
#[test_case(true, true, false, false, "bash"; "remote_tty")]
#[test_case(true, false, true, false, "bash"; "remote_sandbox")]
#[test_case(false, false, false, false, "sh"; "local_sh_pipe")]
#[test_case(false, false, false, false, "bash-sh"; "local_bash_backed_sh")]
#[test_case(false, false, false, true, "bash"; "local_bash_env")]
#[test_case(true, false, false, true, "bash"; "remote_bash_env")]
#[cfg_attr(
@@ -219,6 +220,7 @@ async fn shell_snapshot_v2_filters_profile_exports_and_stays_in_memory(
"bash" if automatic_startup => ("/bin/bash", ".bash-env"),
"bash" => ("/bin/bash", ".bashrc"),
"sh" => ("/bin/sh", ".snapshot-env"),
"bash-sh" => ("/bin/bash", ".snapshot-env"),
"zsh" if automatic_startup => ("/bin/zsh", ".zshenv"),
"zsh" => ("/bin/zsh", ".zshrc"),
name => anyhow::bail!("unsupported test shell {name}"),
@@ -226,6 +228,14 @@ async fn shell_snapshot_v2_filters_profile_exports_and_stays_in_memory(
let profile_path = home.path().join(profile_name);
let profile_path_entry = home.path().join("profile-bin");
let runtime_path_entry = home.path().join("runtime-bin");
std::fs::create_dir(&profile_path_entry)?;
let wc = profile_path_entry.join("wc");
std::fs::write(
&wc,
"#!/bin/sh\nprintf x >> \"$HOME/tool-captures\"\nexec /usr/bin/wc \"$@\"\n",
)?;
std::fs::set_permissions(&wc, std::fs::Permissions::from_mode(0o755))?;
let posix_shell = matches!(shell_name, "sh" | "bash-sh");
let padding = if !use_remote && !tty && shell_name == "bash" {
format!(
"snapshot_padding() {{ printf '%s' '{}'; }}\n",
@@ -234,7 +244,7 @@ async fn shell_snapshot_v2_filters_profile_exports_and_stays_in_memory(
} else {
String::new()
};
let shadowed_builtins = if shell_name == "sh" {
let shadowed_builtins = if posix_shell {
""
} else {
"unset() { exit 41; }\nbuiltin() { :; }\n"
@@ -242,7 +252,7 @@ async fn shell_snapshot_v2_filters_profile_exports_and_stays_in_memory(
std::fs::write(
&profile_path,
format!(
"printf x >> \"$HOME/captures\"\nexport PATH=\"$HOME/profile-bin:/usr/bin:/bin\"\nexport PROFILE_ALLOWED=profile\nexport PROFILE_SECRET=secret\nexport PROFILE_DENIED=denied\nprofile_helper() {{ printf helper; }}\n{shadowed_builtins}{padding}"
"printf x >> \"$HOME/captures\"\nexport PATH=\"$HOME/profile-bin:/usr/bin:/bin\"\nexport PROFILE_ALLOWED=profile\nexport PROFILE_SECRET=secret\nexport PROFILE_DENIED=denied\nprofile_helper() {{ printf helper; }}\nif [ -n \"${{BASH_VERSION-}}\" ]; then\n shopt -s extglob nocasematch\n eval 'profile_helper() {{ case $1 in @(foo|bar)*) printf helper ;; *) return 1 ;; esac; }}'\nfi\nset -u\n{shadowed_builtins}{padding}"
),
)?;
if shell_name == "zsh" && automatic_startup {
@@ -255,11 +265,15 @@ async fn shell_snapshot_v2_filters_profile_exports_and_stays_in_memory(
"HOME".to_string(),
home.path().to_string_lossy().into_owned(),
)]);
if shell_name == "sh" {
if posix_shell {
configured_environment.insert(
"ENV".to_string(),
profile_path.to_string_lossy().into_owned(),
"${XDG_CONFIG_HOME:-$HOME}/.snapshot-env".to_string(),
);
// Keep coverage for large values alongside the many-small-entry case below.
for index in 0..3 {
configured_environment.insert(format!("PROFILE_SDK_{index}"), "x".repeat(60 * 1024));
}
}
if shell_name == "bash" && automatic_startup {
configured_environment.insert(
@@ -267,6 +281,13 @@ async fn shell_snapshot_v2_filters_profile_exports_and_stays_in_memory(
profile_path.to_string_lossy().into_owned(),
);
}
// Many small entries exercise capture overhead separately from the byte limit above.
let many_entries = !use_remote && !tty && !automatic_startup;
if many_entries {
configured_environment.extend(
(0..1_000).map(|index| (format!("PROFILE_ENTRY_{index}"), format!("value-{index}"))),
);
}
let policy = ExecEnvPolicy {
inherit: ShellEnvironmentPolicyInherit::All,
ignore_default_excludes: false,
@@ -283,10 +304,15 @@ async fn shell_snapshot_v2_filters_profile_exports_and_stays_in_memory(
let (command_prefix, expected_prefix) = if shell_name == "sh" {
("", "")
} else {
("profile_helper; ", "helper")
("profile_helper FOObar; ", "helper")
};
let entry_check = if many_entries {
"[ \"${PROFILE_ENTRY_999-missing}\" = value-999 ] || exit 43; "
} else {
""
};
let command = format!(
"export PATH='{}':\"$PATH\"; {command_prefix}printf '|%s|%s|%s|%s|%s|%s' \"$PROFILE_ALLOWED\" \"${{PROFILE_SECRET-missing}}\" \"${{PROFILE_DENIED-missing}}\" \"$PATH\" \"${{__CODEX_SHELL_SNAPSHOT_STATE_0-missing}}\" \"${{__CODEX_SHELL_SNAPSHOT_STATE_1-missing}}\"",
"case $- in *u*) ;; *) exit 42 ;; esac; {entry_check}export PATH='{}':\"$PATH\"; {command_prefix}printf '|%s|%s|%s|%s|%s|%s' \"$PROFILE_ALLOWED\" \"${{PROFILE_SECRET-missing}}\" \"${{PROFILE_DENIED-missing}}\" \"$PATH\" \"${{__CODEX_SHELL_SNAPSHOT_STATE_0-missing}}\" \"${{__CODEX_SHELL_SNAPSHOT_STATE_1-missing}}\"",
runtime_path_entry.display(),
);
let expected_stdout = format!(
@@ -307,14 +333,14 @@ async fn shell_snapshot_v2_filters_profile_exports_and_stays_in_memory(
shell_snapshot: Some(ShellSnapshotRequest {
scope_id: "attachment-1".to_string(),
shell: ShellInfo {
name: shell_name.to_string(),
name: if posix_shell { "sh" } else { shell_name }.to_string(),
path: shell_path.to_string(),
},
}),
env: HashMap::new(),
tty,
pipe_stdin: false,
arg0: None,
arg0: (shell_name == "bash-sh").then(|| "sh".to_string()),
sandbox: (use_sandbox && attempt == 0).then(|| {
FileSystemSandboxContext::from_permission_profile_with_cwd(
PermissionProfile::read_only(),
@@ -335,6 +361,7 @@ async fn shell_snapshot_v2_filters_profile_exports_and_stays_in_memory(
}
assert_eq!(std::fs::read_to_string(home.path().join("captures"))?, "x");
assert!(!std::fs::read(home.path().join("tool-captures"))?.is_empty());
if let Some(server) = context._server {
assert!(!server.codex_home().join("shell_snapshots").exists());
}