mirror of
https://github.com/openai/codex.git
synced 2026-10-19 16:17:49 +00:00
Preserve Windows startup variables and MXC child environment defaults (#54027)
## Why
Windows MXC needs valid host startup variables, while an empty child environment must remain empty for the SDK to select Windows user-profile defaults.
## What changed
- Replace caller overrides of `SystemDrive` and `LOCALAPPDATA` case-insensitively with nonempty Unicode host values, failing before launch if either is unavailable.
- Track whether the original child environment was empty, ignoring launcher transport keys. Supply host `SystemRoot` for launcher bootstrap in that case, then clear the bootstrap environment before the SDK selects defaults.
## Testing
Extend transport tests to cover host-value replacement and preservation of empty child environments. Strengthen the PowerShell fallback RPC test to require `SystemDrive`, `LOCALAPPDATA`, `USERPROFILE`, and `PATH` in the child.
GitOrigin-RevId: a097e1079069ef897ebb5ba5e63546328c5faeaa
(cherry picked from commit 81aee8c86b)
This commit is contained in:
committed by
David Wiesen
parent
703d253ed6
commit
f234b9b82d
@@ -48,7 +48,7 @@ async fn powershell_alias_falls_back_without_primary_runtime_over_rpc(
|
||||
.into_owned(),
|
||||
"-NoProfile".to_owned(),
|
||||
"-Command".to_owned(),
|
||||
"Write-Output 'fallback-ok'".to_owned(),
|
||||
"if (!$env:SystemDrive -or !$env:LOCALAPPDATA -or !$env:USERPROFILE -or !$env:PATH) { throw 'missing Windows defaults' }; Write-Output 'fallback-ok'".to_owned(),
|
||||
],
|
||||
cwd,
|
||||
env_policy: None,
|
||||
|
||||
@@ -45,6 +45,13 @@ request: the canonical `PermissionProfile`, policy cwd, proxy context, and exact
|
||||
argv. The helper inherits command cwd, environment, and stdio from the shared
|
||||
process path; policy cwd can differ from command cwd.
|
||||
|
||||
Windows MXC reserves `SystemDrive` and `LOCALAPPDATA` for the host's validated,
|
||||
nonempty values in both wrapper and explicit child environments. Caller
|
||||
overrides are replaced case-insensitively; unavailable host values fail before
|
||||
launch with a diagnostic naming the variable.
|
||||
Windows may virtualize the child's `LOCALAPPDATA` into a sandbox directory
|
||||
beneath the host profile.
|
||||
|
||||
The request uses launcher-only environment chunks to leave Windows' command-line
|
||||
budget to the workload. Payloads are limited to 1,000,000 UTF-8 bytes and chunks
|
||||
to 4096 bytes; the helper removes them before native process creation. Use
|
||||
@@ -70,8 +77,10 @@ to 4096 bytes; the helper removes them before native process creation. Use
|
||||
command are not implicitly granted.
|
||||
- The native API represents paths and environment values as Unicode strings.
|
||||
Non-Unicode values fail instead of undergoing lossy conversion.
|
||||
- An empty child environment uses the Windows user-profile defaults. Nonempty
|
||||
environments remain explicit.
|
||||
- An empty child environment still uses the Windows user-profile defaults.
|
||||
Supplying only two keys would turn it into an incomplete explicit environment,
|
||||
so the wrapper's bootstrap values are removed before the SDK selects defaults.
|
||||
Nonempty environments remain explicit except for these two host-owned values.
|
||||
- The upstream runner terminates remaining descendants when the foreground
|
||||
process exits, as well as on cancellation. Both existing Windows backends
|
||||
preserve descendants after normal exit, so detached servers currently lose
|
||||
|
||||
@@ -18,6 +18,7 @@ use anyhow::Result;
|
||||
use anyhow::ensure;
|
||||
use codex_network_proxy::ManagedNetworkSandboxContext;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_windows_sandbox::environment_transport;
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
|
||||
@@ -46,6 +47,8 @@ struct MxcCommand {
|
||||
sandbox_policy_cwd: PathBuf,
|
||||
managed_network: Option<ManagedNetworkSandboxContext>,
|
||||
command: Vec<String>,
|
||||
#[serde(default)]
|
||||
default_child_env: bool,
|
||||
}
|
||||
|
||||
/// Inputs used to build an MXC helper invocation.
|
||||
@@ -80,6 +83,9 @@ pub fn create_command_args(args: CreateMxcCommandArgsParams<'_>) -> Result<Vec<S
|
||||
sandbox_policy_cwd: sandbox_policy_cwd.to_owned(),
|
||||
managed_network: managed_network.cloned(),
|
||||
command,
|
||||
default_child_env: env
|
||||
.keys()
|
||||
.all(|key| environment_transport::is_key(key.as_ref())),
|
||||
},
|
||||
env,
|
||||
)?;
|
||||
|
||||
@@ -155,6 +155,7 @@ fn command(permissions: &PermissionProfile, cwd: &Path) -> MxcCommand {
|
||||
sandbox_policy_cwd: cwd.to_owned(),
|
||||
managed_network: None,
|
||||
command: vec!["program.exe".to_owned(), "--arg".to_owned()],
|
||||
default_child_env: false,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -173,6 +174,7 @@ fn wrapper_preserves_exact_argv_and_separate_command_cwd() -> Result<()> {
|
||||
"quotes\" and slash\\".to_owned(),
|
||||
];
|
||||
let mut env = HashMap::new();
|
||||
codex_windows_sandbox::environment_transport::encode("stale", &mut env)?;
|
||||
let wrapped = create_command_args(CreateMxcCommandArgsParams {
|
||||
command: argv.clone(),
|
||||
permission_profile: &profile,
|
||||
@@ -182,6 +184,8 @@ fn wrapper_preserves_exact_argv_and_separate_command_cwd() -> Result<()> {
|
||||
})?;
|
||||
assert_eq!(wrapped, vec![crate::CODEX_WINDOWS_MXC_ARG1]);
|
||||
let parsed = crate::transport::decode(&mut env)?;
|
||||
assert!(parsed.default_child_env);
|
||||
assert!(env.is_empty());
|
||||
assert_eq!(
|
||||
(
|
||||
&parsed.permissions,
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
//! Bounded launcher-only environment transport for policies too large for
|
||||
//! Windows command lines. Every transport variable is removed before spawn.
|
||||
//! Bounded launcher transport. Windows startup variables are host-owned;
|
||||
//! an original-empty marker preserves the SDK's user-profile defaults.
|
||||
|
||||
use std::collections::HashMap;
|
||||
|
||||
@@ -11,6 +11,19 @@ use codex_windows_sandbox::environment_transport;
|
||||
use crate::MxcCommand;
|
||||
|
||||
pub(super) fn encode(command: &MxcCommand, env: &mut HashMap<String, String>) -> Result<()> {
|
||||
// SDK profile defaults depend on the launcher's SystemRoot.
|
||||
#[cfg(windows)]
|
||||
for key in ["SystemDrive", "LOCALAPPDATA"]
|
||||
.into_iter()
|
||||
.chain(command.default_child_env.then_some("SystemRoot"))
|
||||
{
|
||||
let value = std::env::var(key)
|
||||
.ok()
|
||||
.filter(|value| !value.is_empty())
|
||||
.with_context(|| format!("MXC requires nonempty Unicode host {key}"))?;
|
||||
env.retain(|name, _| !name.eq_ignore_ascii_case(key));
|
||||
env.insert(key.to_owned(), value);
|
||||
}
|
||||
let payload = serde_json::to_string(command)?;
|
||||
environment_transport::encode(&payload, env)
|
||||
}
|
||||
@@ -18,7 +31,12 @@ pub(super) fn encode(command: &MxcCommand, env: &mut HashMap<String, String>) ->
|
||||
#[cfg(any(windows, test))]
|
||||
pub(super) fn decode(env: &mut HashMap<String, String>) -> Result<MxcCommand> {
|
||||
let payload = environment_transport::decode_and_scrub(env)?;
|
||||
serde_json::from_str(&payload).context("invalid MXC launcher request")
|
||||
let command: MxcCommand =
|
||||
serde_json::from_str(&payload).context("invalid MXC launcher request")?;
|
||||
if command.default_child_env {
|
||||
env.clear();
|
||||
}
|
||||
Ok(command)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
|
||||
@@ -19,6 +19,7 @@ fn command(args: Vec<String>) -> MxcCommand {
|
||||
sandbox_policy_cwd: PathBuf::from("workspace"),
|
||||
managed_network: None,
|
||||
command: args,
|
||||
default_child_env: false,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -31,6 +32,10 @@ fn large_unicode_launch_roundtrips_and_never_reaches_child_environment() -> Resu
|
||||
];
|
||||
let expected_env = HashMap::from([("CUSTOM".to_owned(), "value".to_owned())]);
|
||||
let mut env = expected_env.clone();
|
||||
#[cfg(windows)]
|
||||
env.extend(
|
||||
["systemdrive", "LocalAppData"].map(|key| (key.to_owned(), "caller-only".to_owned())),
|
||||
);
|
||||
environment_transport::encode("stale", &mut env)?;
|
||||
let request = command(args);
|
||||
encode(&request, &mut env)?;
|
||||
@@ -42,6 +47,10 @@ fn large_unicode_launch_roundtrips_and_never_reaches_child_environment() -> Resu
|
||||
serde_json::to_value(decode(&mut env)?)?,
|
||||
serde_json::to_value(request)?
|
||||
);
|
||||
#[cfg(windows)]
|
||||
for key in ["SystemDrive", "LOCALAPPDATA"] {
|
||||
assert_eq!(env.remove(key), Some(std::env::var(key)?));
|
||||
}
|
||||
assert_eq!(env, expected_env);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user