diff --git a/sdk/src/sandbox.ts b/sdk/src/sandbox.ts index e941e2783..d25854aff 100644 --- a/sdk/src/sandbox.ts +++ b/sdk/src/sandbox.ts @@ -437,6 +437,28 @@ export interface SandboxSpawnOptions { signal?: AbortSignal; } +/** + * Inject environment variables into the config's `process.env` field as + * `KEY=VALUE` strings. This is the explicit channel for passing env vars + * to the sandboxed child -- the parent process environment is NOT inherited + * by the sandbox (security: prevents secret leakage). + */ +function injectEnvIntoConfig( + config: ContainerConfig, + env: { [key: string]: string | undefined }, +): void { + if (!config.process) { + config.process = { commandLine: '' }; + } + const entries: string[] = config.process.env ? [...config.process.env] : []; + for (const [key, value] of Object.entries(env)) { + if (value !== undefined) { + entries.push(`${key}=${value}`); + } + } + config.process.env = entries; +} + /** * Internal helper: resolves the executor binary path and spawns a PTY process. */ @@ -446,6 +468,12 @@ function spawnWithConfig( workingDirectory?: string, env?: { [key: string]: string | undefined }, ): pty.IPty { + // Inject env vars into config.process.env so they are passed explicitly to + // the sandboxed child via the JSON config (not via process inheritance). + if (env) { + injectEnvIntoConfig(config, env); + } + const { executablePath, args, logger, startTime } = prepareSpawn(config, options); try { @@ -455,7 +483,6 @@ function spawnWithConfig( rows: 80, ...options.ptyOptions, cwd: workingDirectory || process.cwd(), - env: env ?? options.ptyOptions?.env, }; diagLog(`spawnWithConfig: spawning PTY process, cwd=${ptyOpts.cwd}`); @@ -557,12 +584,17 @@ export function spawnSandboxFromConfig( env?: { [key: string]: string | undefined } ): pty.IPty | ChildProcess { if (options.usePty === false) { + // Inject env vars into config.process.env so they are passed explicitly to + // the sandboxed child via the JSON config (not via process inheritance). + if (env) { + injectEnvIntoConfig(config, env); + } + const { executablePath, args, logger, startTime } = prepareSpawn(config, options); try { const child = spawn(executablePath, args, { cwd: workingDirectory || process.cwd(), stdio: ['pipe', 'pipe', 'pipe'], - ...(env ? { env: { ...process.env, ...env } as NodeJS.ProcessEnv } : {}), }); child.on('close', (code) => { logger?.log('info', 'mxc.spawn.exit', { diff --git a/src/Cargo.toml b/src/Cargo.toml index 8a769ca36..2c3ef5196 100644 --- a/src/Cargo.toml +++ b/src/Cargo.toml @@ -40,6 +40,7 @@ windows = { version = "0.62", features = [ "Win32_Security", "Win32_System_Com", "Win32_System_Console", + "Win32_System_Environment", "Win32_System_Ole", "Win32_System_Threading", "Win32_System_Pipes", diff --git a/src/wxc_common/src/base_container_runner.rs b/src/wxc_common/src/base_container_runner.rs index 85510443d..82af9763f 100644 --- a/src/wxc_common/src/base_container_runner.rs +++ b/src/wxc_common/src/base_container_runner.rs @@ -38,6 +38,29 @@ use sandbox_spec::base_container_layout::{ NetworkPolicy as FbsNetworkPolicy, NetworkPolicyArgs, SandboxSpec, SandboxSpecArgs, }; +/// `CREATE_UNICODE_ENVIRONMENT` -- tells the API that the environment block is UTF-16 encoded. +const CREATE_UNICODE_ENVIRONMENT: u32 = 0x0000_0400; + +/// Serialize `KEY=VALUE` pairs into a double-null-terminated UTF-16 environment block. +/// +/// Entries are sorted case-insensitively by key as required by `CreateProcessW`. +fn encode_env_block(env_vars: &[String]) -> Vec { + let mut entries: Vec<(&str, &str)> = + env_vars.iter().filter_map(|e| e.split_once('=')).collect(); + + entries.sort_by(|(a, _), (b, _)| a.to_ascii_uppercase().cmp(&b.to_ascii_uppercase())); + + let mut block = Vec::new(); + for (key, value) in &entries { + for ch in format!("{}={}", key, value).encode_utf16() { + block.push(ch); + } + block.push(0); + } + block.push(0); + block +} + /// Function pointer type matching `Experimental_CreateProcessInSandbox` from processmodel.dll. type PfnCreateProcessInSandbox = unsafe extern "system" fn( application_name: *const u16, @@ -543,6 +566,28 @@ impl ScriptRunner for BaseContainerRunner { }; let mut pi: PROCESS_INFORMATION = unsafe { std::mem::zeroed() }; + // Environment block for the sandboxed child. + // If the caller specified explicit env vars, use only those. + // Otherwise, pass NULL to let the OS provide the default environment + // for the sandbox (CreateProcessInSandbox handles this internally). + let env_block: Option> = if request.env.is_empty() { + // TODO: consider calling CreateEnvironmentBlock(NULL, FALSE) here + // for a cleansed default env if the OS API doesn't do it for us. + None + } else { + Some(encode_env_block(&request.env)) + }; + + let env_ptr = env_block + .as_ref() + .map(|b| b.as_ptr() as *const c_void) + .unwrap_or(ptr::null()); + let creation_flags = if env_block.is_some() { + CREATE_UNICODE_ENVIRONMENT + } else { + 0 + }; + let _ = writeln!(logger, "launching: {}", request.script_code); let _ = writeln!(logger, "identity: {identity}"); @@ -558,8 +603,8 @@ impl ScriptRunner for BaseContainerRunner { ptr::null(), // processAttributes (must be NULL) ptr::null(), // threadAttributes (must be NULL) 0, // inheritHandles (must be FALSE) - 0, // creationFlags - ptr::null(), // environment (must be NULL) + creation_flags, // creationFlags + env_ptr, // environment cwd_ptr, // currentDirectory &si, // startupInfo identity_wide.as_ptr(), // identity