mirror of
https://github.com/openai/codex.git
synced 2026-08-23 13:09:46 +00:00
Honor request PATH in exec-server shell snapshots (#39917)
## What changed - Apply all per-request environment overrides, including `PATH`, after restoring the captured shell environment. - Remove `runtime_path_prepends` from `ShellSnapshotRequest` and the associated PATH replay and deduplication logic. - Keep the shell snapshot integration test's runtime PATH setup in the command being executed. GitOrigin-RevId: a6f8d2e144bf9977c7434bc58b60ac972f7e0449
This commit is contained in:
@@ -269,9 +269,6 @@ pub struct ShellSnapshotRequest {
|
|||||||
pub scope_id: String,
|
pub scope_id: String,
|
||||||
/// Executor-native shell used to capture and restore the snapshot.
|
/// Executor-native shell used to capture and restore the snapshot.
|
||||||
pub shell: ShellInfo,
|
pub shell: ShellInfo,
|
||||||
/// Runtime-owned PATH entries to replay after restoring profile state.
|
|
||||||
#[serde(default, skip_serializing_if = "Vec::is_empty")]
|
|
||||||
pub runtime_path_prepends: Vec<String>,
|
|
||||||
}
|
}
|
||||||
|
|
||||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||||
|
|||||||
@@ -132,15 +132,9 @@ impl ShellSnapshotCache {
|
|||||||
return Ok(());
|
return Ok(());
|
||||||
};
|
};
|
||||||
|
|
||||||
let restore_snapshot_path = snapshot.environment.contains_key("PATH")
|
|
||||||
&& params
|
|
||||||
.env_policy
|
|
||||||
.as_ref()
|
|
||||||
.is_none_or(|policy| !policy.r#set.contains_key("PATH"));
|
|
||||||
let request_overrides = params
|
let request_overrides = params
|
||||||
.env
|
.env
|
||||||
.iter()
|
.iter()
|
||||||
.filter(|(name, _)| name.as_str() != "PATH" || !restore_snapshot_path)
|
|
||||||
.map(|(name, value)| {
|
.map(|(name, value)| {
|
||||||
(
|
(
|
||||||
name.clone(),
|
name.clone(),
|
||||||
@@ -155,20 +149,6 @@ impl ShellSnapshotCache {
|
|||||||
.map(|(name, value)| (name.clone(), value.clone())),
|
.map(|(name, value)| (name.clone(), value.clone())),
|
||||||
);
|
);
|
||||||
prepared.env.extend(request_overrides);
|
prepared.env.extend(request_overrides);
|
||||||
if restore_snapshot_path && let Some(path) = prepared.env.get_mut("PATH") {
|
|
||||||
for entry in &request.runtime_path_prepends {
|
|
||||||
if entry.is_empty() {
|
|
||||||
continue;
|
|
||||||
}
|
|
||||||
*path = std::iter::once(entry.as_str())
|
|
||||||
.chain(
|
|
||||||
path.split(':')
|
|
||||||
.filter(|existing| !existing.is_empty() && *existing != entry),
|
|
||||||
)
|
|
||||||
.collect::<Vec<_>>()
|
|
||||||
.join(":");
|
|
||||||
}
|
|
||||||
}
|
|
||||||
prepared
|
prepared
|
||||||
.env
|
.env
|
||||||
.retain(|name, _| !shell_environment::is_non_inheritable_env_var(name));
|
.retain(|name, _| !shell_environment::is_non_inheritable_env_var(name));
|
||||||
|
|||||||
@@ -141,8 +141,6 @@ async fn shell_snapshot_v2_filters_profile_exports_and_stays_in_memory(
|
|||||||
let profile_path = home.path().join(profile_name);
|
let profile_path = home.path().join(profile_name);
|
||||||
let profile_path_entry = home.path().join("profile-bin");
|
let profile_path_entry = home.path().join("profile-bin");
|
||||||
let runtime_path_entry = home.path().join("runtime-bin");
|
let runtime_path_entry = home.path().join("runtime-bin");
|
||||||
let inherited_path = std::env::var("PATH")?;
|
|
||||||
let runtime_path = format!("{}:{inherited_path}", runtime_path_entry.display());
|
|
||||||
let padding = if !use_remote && !tty && shell_name == "bash" {
|
let padding = if !use_remote && !tty && shell_name == "bash" {
|
||||||
format!(
|
format!(
|
||||||
"snapshot_padding() {{ printf '%s' '{}'; }}\n",
|
"snapshot_padding() {{ printf '%s' '{}'; }}\n",
|
||||||
@@ -190,7 +188,8 @@ async fn shell_snapshot_v2_filters_profile_exports_and_stays_in_memory(
|
|||||||
("profile_helper; ", "helper")
|
("profile_helper; ", "helper")
|
||||||
};
|
};
|
||||||
let command = format!(
|
let command = format!(
|
||||||
"{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}}\""
|
"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!(
|
let expected_stdout = format!(
|
||||||
"{expected_prefix}|profile|missing|missing|{}:{}:/usr/bin:/bin|missing|missing",
|
"{expected_prefix}|profile|missing|missing|{}:{}:/usr/bin:/bin|missing|missing",
|
||||||
@@ -212,9 +211,8 @@ async fn shell_snapshot_v2_filters_profile_exports_and_stays_in_memory(
|
|||||||
name: shell_name.to_string(),
|
name: shell_name.to_string(),
|
||||||
path: shell_path.to_string(),
|
path: shell_path.to_string(),
|
||||||
},
|
},
|
||||||
runtime_path_prepends: vec![runtime_path_entry.to_string_lossy().into_owned()],
|
|
||||||
}),
|
}),
|
||||||
env: HashMap::from([("PATH".to_string(), runtime_path.clone())]),
|
env: HashMap::new(),
|
||||||
tty,
|
tty,
|
||||||
pipe_stdin: false,
|
pipe_stdin: false,
|
||||||
arg0: None,
|
arg0: None,
|
||||||
@@ -293,7 +291,6 @@ async fn shell_snapshot_v2_remote_managed_proxy_uses_prepared_execution_context(
|
|||||||
name: "bash".to_string(),
|
name: "bash".to_string(),
|
||||||
path: "/bin/bash".to_string(),
|
path: "/bin/bash".to_string(),
|
||||||
},
|
},
|
||||||
runtime_path_prepends: Vec::new(),
|
|
||||||
}),
|
}),
|
||||||
env: HashMap::new(),
|
env: HashMap::new(),
|
||||||
tty: false,
|
tty: false,
|
||||||
@@ -362,7 +359,6 @@ async fn shell_snapshot_v2_capture_failure_falls_back_to_original_command() -> R
|
|||||||
name: "bash".to_string(),
|
name: "bash".to_string(),
|
||||||
path: "/bin/bash".to_string(),
|
path: "/bin/bash".to_string(),
|
||||||
},
|
},
|
||||||
runtime_path_prepends: Vec::new(),
|
|
||||||
}),
|
}),
|
||||||
env: HashMap::new(),
|
env: HashMap::new(),
|
||||||
tty: false,
|
tty: false,
|
||||||
|
|||||||
Reference in New Issue
Block a user