github/codex
4
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-20 00:18:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

Honor shell environment policy in legacy snapshots (#48099)

Browse Source 
## Why

Legacy shell snapshots could retain exports excluded by `shell_environment_policy` and restore captured values over explicit overrides when replayed.

## What changed

- Build snapshots with the resolved environment's policy and omit filtered exports and original values for variables overridden by `set`.
- Skip eager snapshot creation when configuration is unavailable, and reject snapshots whose policy differs from the current turn's policy.

## Testing

Add a regression test covering default and custom exclusions, `include_only`, preservation of multiline values, and `set` overrides that survive snapshot replay. Verify excluded and overridden captured values are absent from the snapshot file.

GitOrigin-RevId: 4b785130f6c5a4d268ab86fd36ab9d77d8c4a3b9
This commit is contained in:
jif
2026-09-25 14:15:05 +00:00
committed by copyberry
parent b35a7afbe8
commit b63a296775
7 changed files with 180 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/core/src/environment_selection.rs
+11 -2
View File
@@ -17,7 +17,6 @@ use codex_exec_server::ExecutorFileSystem;
use codex_exec_server::SelectedCapabilityRootsStatus;
use codex_protocol::capabilities::CapabilityRootLocation;
use codex_protocol::capabilities::SelectedCapabilityRoot;
use codex_protocol::config_types::ShellEnvironmentPolicy;
use codex_protocol::error::CodexErr;
use codex_protocol::error::Result as CodexResult;
use codex_protocol::models::PermissionProfile;
@@ -344,18 +343,22 @@ impl ThreadEnvironments {
environment: Arc<Environment>,
cwd: PathUri,
shell: Option<Shell>,
config: &EnvironmentConfigState,
) -> ShellSnapshotTask {
// Protected snapshots require the command's sandbox and are captured lazily.
if shell_snapshot.should_rebuild_inherited() {
return futures::future::ready(None).boxed().shared();
}
let EnvironmentConfigState::Ready(config) = config else {
return futures::future::ready(None).boxed().shared();
};
let shell_snapshot = shell_snapshot
.build(
environment,
cwd,
shell,
/*allow_login_shell*/ true,
ShellEnvironmentPolicy::default(),
config.shell_environment_policy.clone(),
/*sandbox*/ None,
)
.boxed()
@@ -610,6 +613,7 @@ impl ThreadEnvironments {
Arc::clone(&resolved.environment),
selected.selection.cwd.clone(),
resolved.shell.clone(),
&selected.selection.config,
);
selected.resolution = futures::future::ready(Ok(ResolvedEnvironment {
shell_snapshot,
@@ -803,11 +807,16 @@ impl ThreadEnvironments {
)
};
let shell_snapshot_builder = shell_snapshot.clone();
let snapshot_config = installed_config
.clone()
.map(EnvironmentConfigState::Ready)
.unwrap_or(selection.config);
let task = Self::start_shell_snapshot_task(
shell_snapshot,
Arc::clone(&environment),
selection.cwd,
shell.clone(),
&snapshot_config,
);
let shell_snapshot_v2_supported = snapshot_v2
&& (environment.is_remote() || !shell_snapshot_builder.should_rebuild_inherited());
codex-rs/core/src/session/turn_context.rs
+3 -1
View File
@@ -196,7 +196,9 @@ impl TurnEnvironment {
}
None
} else {
self.shell_snapshot.peek()?.clone()
self.shell_snapshot.peek()?.clone().filter(|snapshot| {
&snapshot.shell_environment_policy == self.shell_environment_policy()
})
}
}
codex-rs/core/src/shell_snapshot.rs
+22 -4
View File
@@ -72,6 +72,7 @@ pub(crate) enum SnapshotCredentialBrokerState {
pub(crate) struct ShellSnapshotFile {
path: AbsolutePathBuf,
credentials: Option<SnapshotCredentials>,
pub(crate) shell_environment_policy: ShellEnvironmentPolicy,
}
struct SnapshotCredentials {
@@ -205,7 +206,7 @@ impl ShellSnapshot {
SnapshotCredentialBrokerState::Ready(network_proxy) if sandbox.is_some() => {
Some(SnapshotCredentialBroker {
network_proxy,
shell_environment_policy,
shell_environment_policy: shell_environment_policy.clone(),
allow_login_shell,
})
}
@@ -221,6 +222,7 @@ impl ShellSnapshot {
&config.codex_home,
config.session_id,
&cwd,
&shell_environment_policy,
&shell,
config.state_db.clone(),
credential_broker,
@@ -242,10 +244,12 @@ impl ShellSnapshot {
.await
}
#[allow(clippy::too_many_arguments)]
async fn try_create(
codex_home: &AbsolutePathBuf,
session_id: ThreadId,
session_cwd: &AbsolutePathBuf,
shell_environment_policy: &ShellEnvironmentPolicy,
shell: &Shell,
state_db: Option<StateDbHandle>,
credential_broker: Option<SnapshotCredentialBroker>,
@@ -283,6 +287,7 @@ impl ShellSnapshot {
shell,
&temp_path,
session_cwd,
shell_environment_policy,
credential_broker.as_ref(),
sandbox,
)
@@ -319,7 +324,11 @@ impl ShellSnapshot {
return Err("write_failed");
}
Ok(ShellSnapshotFile { path, credentials })
Ok(ShellSnapshotFile {
path,
credentials,
shell_environment_policy: shell_environment_policy.clone(),
})
}
}
@@ -531,6 +540,7 @@ async fn write_shell_snapshot(
shell: &Shell,
output_path: &AbsolutePathBuf,
cwd: &AbsolutePathBuf,
shell_environment_policy: &ShellEnvironmentPolicy,
credential_broker: Option<&SnapshotCredentialBroker>,
sandbox: Option<&ShellSnapshotSandbox>,
) -> Result<Option<SnapshotCredentials>> {
@@ -538,7 +548,14 @@ async fn write_shell_snapshot(
if shell_type == ShellType::PowerShell || shell_type == ShellType::Cmd {
bail!("Shell snapshot not supported yet for {shell_type:?}");
}
let (snapshot, credentials) = capture_snapshot(shell, cwd, credential_broker, sandbox).await?;
let (snapshot, credentials) = capture_snapshot(
shell,
cwd,
shell_environment_policy,
credential_broker,
sandbox,
)
.await?;
if let Some(parent) = output_path.parent() {
let parent_display = parent.display();
@@ -558,6 +575,7 @@ async fn write_shell_snapshot(
async fn capture_snapshot(
shell: &Shell,
cwd: &AbsolutePathBuf,
shell_environment_policy: &ShellEnvironmentPolicy,
credential_broker: Option<&SnapshotCredentialBroker>,
sandbox: Option<&ShellSnapshotSandbox>,
) -> Result<(String, Option<SnapshotCredentials>)> {
@@ -597,7 +615,7 @@ async fn capture_snapshot(
let capture = CapturedSnapshot::parse(shell_type, raw_snapshot.as_bytes())
.ok_or_else(|| anyhow!("invalid shell snapshot capture"))?;
let Some(credential_broker) = credential_broker else {
return Ok((capture.render_script(), None));
return Ok((capture.render_script(shell_environment_policy), None));
};
let original_env = std::str::from_utf8(capture.environment)?
codex-rs/core/src/shell_snapshot_tests.rs
+24 -1
View File
@@ -154,6 +154,7 @@ async fn get_snapshot(shell_type: ShellType) -> Result<String> {
&shell,
&path.abs(),
&dir.path().abs(),
&ShellEnvironmentPolicy::default(),
/*credential_broker*/ None,
/*sandbox*/ None,
)
@@ -728,6 +729,7 @@ async fn snapshot_discovers_and_redacts_shell_initialized_credentials() -> Resul
&shell,
&path,
&dir.path().abs(),
&credential_broker.shell_environment_policy,
Some(&credential_broker),
/*sandbox*/ None,
)
@@ -825,6 +827,7 @@ async fn snapshot_discovers_and_redacts_shell_initialized_credentials() -> Resul
&shell,
&filtered_startup_path,
&dir.path().abs(),
&filtered_startup_broker.shell_environment_policy,
Some(&filtered_startup_broker),
/*sandbox*/ None,
)
@@ -847,6 +850,7 @@ async fn snapshot_discovers_and_redacts_shell_initialized_credentials() -> Resul
let (_, excluded_credentials) = capture_snapshot(
&shell,
&dir.path().abs(),
&excluded_credential_broker.shell_environment_policy,
Some(&excluded_credential_broker),
/*sandbox*/ None,
)
@@ -916,7 +920,11 @@ async fn snapshot_discovers_and_redacts_shell_initialized_credentials() -> Resul
);
assert!(!inherited_snapshot.contains(inherited_secret));
let snapshot_file = ShellSnapshotFile { path, credentials };
let snapshot_file = ShellSnapshotFile {
path,
credentials,
shell_environment_policy: ShellEnvironmentPolicy::default(),
};
let unset_credential_keys = snapshot_file
.credentials
.as_ref()
@@ -1212,6 +1220,7 @@ async fn snapshot_discovers_and_redacts_shell_initialized_credentials() -> Resul
&shell,
&partially_filtered_path,
&dir.path().abs(),
&partially_filtered_credential_broker.shell_environment_policy,
Some(&partially_filtered_credential_broker),
/*sandbox*/ None,
)
@@ -1230,6 +1239,7 @@ async fn snapshot_discovers_and_redacts_shell_initialized_credentials() -> Resul
"Bearer ghp_filtered_dummy\nunset"
);
let filtered_snapshot = ShellSnapshotFile {
shell_environment_policy: ShellEnvironmentPolicy::default(),
path: partially_filtered_path,
credentials: filtered_credentials,
};
@@ -1270,6 +1280,7 @@ async fn snapshot_protects_posix_startup_only_when_it_contains_credentials() ->
&posix_shell,
&posix_snapshot_path,
&dir.path().abs(),
&posix_credential_broker.shell_environment_policy,
Some(&posix_credential_broker),
/*sandbox*/ None,
)
@@ -1293,6 +1304,7 @@ async fn snapshot_protects_posix_startup_only_when_it_contains_credentials() ->
&posix_shell,
&application_snapshot_path,
&dir.path().abs(),
&posix_credential_broker.shell_environment_policy,
Some(&posix_credential_broker),
/*sandbox*/ None,
)
@@ -1344,6 +1356,7 @@ async fn snapshot_discovers_and_restores_inherited_credential_aliases() -> Resul
&shell,
&inherited_path,
&dir.path().abs(),
&inherited_credential_broker.shell_environment_policy,
Some(&inherited_credential_broker),
/*sandbox*/ None,
)
@@ -1366,6 +1379,7 @@ async fn snapshot_discovers_and_restores_inherited_credential_aliases() -> Resul
);
let snapshot_file = ShellSnapshotFile {
shell_environment_policy: ShellEnvironmentPolicy::default(),
path: inherited_path,
credentials: inherited_credentials,
};
@@ -1434,6 +1448,7 @@ async fn snapshot_discovers_and_restores_inherited_credential_aliases() -> Resul
let residual_credential = capture_snapshot(
&shell,
&dir.path().abs(),
&inherited_credential_broker.shell_environment_policy,
Some(&inherited_credential_broker),
/*sandbox*/ None,
)
@@ -1450,6 +1465,7 @@ async fn snapshot_discovers_and_restores_inherited_credential_aliases() -> Resul
let (unset_path_snapshot, _) = capture_snapshot(
&shell,
&dir.path().abs(),
&inherited_credential_broker.shell_environment_policy,
Some(&inherited_credential_broker),
/*sandbox*/ None,
)
@@ -1471,12 +1487,14 @@ async fn snapshot_discovers_and_restores_inherited_credential_aliases() -> Resul
let (snapshot, credentials) = capture_snapshot(
&shell,
&dir.path().abs(),
&inherited_credential_broker.shell_environment_policy,
Some(&inherited_credential_broker),
/*sandbox*/ None,
)
.await?;
assert!(!snapshot.contains(real));
let snapshot_file = ShellSnapshotFile {
shell_environment_policy: ShellEnvironmentPolicy::default(),
path: dir.path().join("hostless-snapshot.sh").abs(),
credentials,
};
@@ -1527,12 +1545,14 @@ async fn snapshot_discovers_and_restores_inherited_credential_aliases() -> Resul
let (snapshot, credentials) = capture_snapshot(
&shell,
&dir.path().abs(),
&broker.shell_environment_policy,
Some(&broker),
/*sandbox*/ None,
)
.await?;
assert!(!snapshot.contains(real));
let snapshot_file = ShellSnapshotFile {
shell_environment_policy: ShellEnvironmentPolicy::default(),
path: dir.path().join("hostless-alias-snapshot.sh").abs(),
credentials,
};
@@ -1587,6 +1607,7 @@ async fn try_create_creates_and_deletes_snapshot_file() -> Result<()> {
&dir.path().abs(),
ThreadId::new(),
&dir.path().abs(),
&ShellEnvironmentPolicy::default(),
&shell,
/*state_db*/ None,
/*credential_broker*/ None,
@@ -1618,6 +1639,7 @@ async fn try_create_uses_distinct_generation_paths() -> Result<()> {
&dir.path().abs(),
session_id,
&dir.path().abs(),
&ShellEnvironmentPolicy::default(),
&shell,
/*state_db*/ None,
/*credential_broker*/ None,
@@ -1629,6 +1651,7 @@ async fn try_create_uses_distinct_generation_paths() -> Result<()> {
&dir.path().abs(),
session_id,
&dir.path().abs(),
&ShellEnvironmentPolicy::default(),
&shell,
/*state_db*/ None,
/*credential_broker*/ None,
codex-rs/core/tests/suite/shell_snapshot.rs
+103 -1
View File
@@ -14,9 +14,13 @@ use codex_network_proxy::NetworkProxyConfig;
#[cfg(unix)]
use codex_protocol::config_types::ApprovalsReviewer;
use codex_protocol::config_types::CollaborationMode;
#[cfg(unix)]
use codex_protocol::config_types::EnvironmentVariablePattern;
use codex_protocol::config_types::ModeKind;
use codex_protocol::config_types::Settings;
#[cfg(unix)]
use codex_protocol::config_types::ShellEnvironmentPolicy;
#[cfg(unix)]
use codex_protocol::config_types::TrustLevel;
#[cfg(unix)]
use codex_protocol::config_types::WindowsSandboxLevel;
@@ -59,7 +63,7 @@ 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")]
#[cfg(unix)]
use std::os::unix::fs::PermissionsExt;
use std::path::Path;
use std::path::PathBuf;
@@ -752,6 +756,104 @@ async fn shell_snapshot_v2_guardian_uses_its_resolved_permissions_and_tools(
Ok(())
}
#[cfg(unix)]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn legacy_snapshot_omits_filtered_and_overridden_exports() -> Result<()> {
skip_if_remote!(Ok(()), "legacy snapshots require a local environment");
let home = tempfile::tempdir()?;
let shell_path = home.path().join("bash");
// Seed the capture shell without changing the test process environment. Replay uses -c.
fs::write(
&shell_path,
r#"#!/bin/sh
if [ "$1" = -lc ]; then
export HOME="${0%/*}"
export PROFILE_ALLOWED='first
second'
export PROFILE_TOKEN=token-sentinel
export PROFILE_DENIED=denied-sentinel
export OTHER=other-sentinel
export PROFILE_SECRET=original-secret-sentinel
fi
exec /bin/bash --noprofile --norc "$@"
"#,
)
.await?;
fs::set_permissions(&shell_path, std::fs::Permissions::from_mode(/*mode*/ 0o755)).await?;
let builder = test_codex()
.with_user_shell(
codex_shell_command::shell_detect::DetectedShell {
shell_type: codex_shell_command::shell_detect::ShellType::Bash,
shell_path,
}
.into(),
)
.with_config(|config| {
config.features.enable(Feature::ShellSnapshot).unwrap();
config.features.enable(Feature::UnifiedExec).unwrap();
config.features.disable(Feature::ShellSnapshotV2).unwrap();
config
.permissions
.set_permission_profile(PermissionProfile::Disabled)
.unwrap();
config.permissions.shell_environment_policy = ShellEnvironmentPolicy {
ignore_default_excludes: false,
exclude: vec![EnvironmentVariablePattern::new_case_insensitive(
"PROFILE_DENIED",
)],
include_only: ["PATH", "PROFILE_*"]
.map(EnvironmentVariablePattern::new_case_insensitive)
.to_vec(),
r#set: HashMap::from([("PROFILE_SECRET".to_string(), "dummy".to_string())]),
..Default::default()
};
});
let harness = TestCodexHarness::with_auto_env_builder(builder).await?;
let snapshot_path = wait_for_snapshot(harness.test().home.path()).await?;
let snapshot = fs::read_to_string(&snapshot_path).await?;
for sentinel in [
"token-sentinel",
"denied-sentinel",
"other-sentinel",
"original-secret-sentinel",
] {
assert!(!snapshot.contains(sentinel));
}
let snapshot_path = shlex::try_quote(snapshot_path.to_str().unwrap())?;
let args = json!({
"cmd": format!("printf '%s|%s|%s|%s|%s' \"$PROFILE_ALLOWED\" \"${{PROFILE_TOKEN-missing}}\" \"${{PROFILE_DENIED-missing}}\" \"${{OTHER-missing}}\" \"$PROFILE_SECRET\"; . {snapshot_path}; printf '|%s' \"$PROFILE_SECRET\""),
"shell": "/bin/bash",
"yield_time_ms": 1_000,
});
mount_sse_sequence(
harness.server(),
vec![
sse(vec![
ev_function_call("snapshot-policy", "exec_command", &args.to_string()),
ev_completed("tool"),
]),
sse(vec![ev_completed("done")]),
],
)
.await;
// Keep the environment selection unchanged so the readiness wait refers to this turn's snapshot.
harness
.test()
.submit_text_turn("verify snapshot filtering")
.await?;
let output = harness.function_call_stdout("snapshot-policy").await;
assert_eq!(
normalize_newlines(&output)
.split_once("Output:\n")
.unwrap()
.1
.trim(),
"first\nsecond|missing|missing|missing|dummy|dummy"
);
harness.test().codex.shutdown_and_wait().await?;
Ok(())
}
#[cfg(unix)]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn shell_snapshot_v2_filters_profile_secrets_without_creating_files() -> Result<()> {
codex-rs/shell-command/src/shell_snapshot_render.rs
+16 -4
View File
@@ -1,7 +1,9 @@
//! Serialize prepared snapshot values without inspecting credentials or executing shell code.
//! Serialize prepared snapshot values and apply export-name policy without executing shell code.
//! Native declarations stay unchanged unless the credential stage supplies a replacement.
use super::capture::CapturedSnapshot;
use codex_protocol::config_types::ShellEnvironmentPolicy;
use codex_protocol::shell_environment::create_env_from_vars;
impl CapturedSnapshot<'_> {
/// Render native functions, options, and aliases; the executor restores environment separately.
@@ -9,12 +11,22 @@ impl CapturedSnapshot<'_> {
format!("{}{}", self.state, self.aliases)
}
/// Render a complete replay script without applying credential or environment policy.
pub fn render_script(&self) -> String {
/// Persist only exports admitted by the policy. Explicit overrides are supplied
/// by the command environment; never retain their original captured values.
pub fn render_script(&self, policy: &ShellEnvironmentPolicy) -> String {
let allowed = create_env_from_vars(
self.exports
.iter()
.map(|export| (export.key.to_string(), String::new())),
policy,
/*thread_id*/ None,
);
let mut script = self.render_state();
script.push_str("# exports (native declarations)\n");
for export in &self.exports {
script.push_str(&export.source);
if allowed.contains_key(export.key) && !policy.r#set.contains_key(export.key) {
script.push_str(&export.source);
}
}
script
}
codex-rs/shell-command/src/shell_snapshot_tests.rs
+1 -1
View File
@@ -2231,7 +2231,7 @@ fn rewrite_snapshot_credentials(
fn captured_script(shell_type: ShellType, source: &str) -> Result<String> {
let captured =
CapturedSnapshot::parse(shell_type, source.as_bytes()).context("invalid native capture")?;
Ok(captured.render_script())
Ok(captured.render_script(&codex_protocol::config_types::ShellEnvironmentPolicy::default()))
}
#[test]