mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Run fs helper through Windows sandbox wrapper (#28359)
## Why This is the final PR in the Windows fs-helper sandbox stack and contains the actual bug fix. The exec-server filesystem helper is a direct-spawn path: it asks `SandboxManager` for a `SandboxExecRequest`, then launches the returned argv itself. That works on macOS and Linux because the transformed argv is already a self-contained sandbox wrapper. On Windows, the transformed request carried `WindowsRestrictedToken` metadata, but the direct-spawn fs-helper runner still launched the helper argv directly. That means Windows filesystem built-ins backed by the fs-helper could run with the parent Codex process permissions instead of the configured Windows sandbox. This PR makes the direct-spawn transform produce a self-contained Windows wrapper argv before fs-helper launches it. ## What Changed - Added `SandboxManager::transform_for_direct_spawn()` for callers that launch the returned argv themselves. - Wrapped Windows restricted-token direct-spawn requests with `codex.exe --run-as-windows-sandbox` and then marked the outer request as unsandboxed, matching the macOS/Linux wrapper argv shape. - Updated `exec-server/src/fs_sandbox.rs` to use the direct-spawn transform for fs-helper launches. - Materialized the inner `codex.exe --codex-run-as-fs-helper` executable into `.sandbox-bin` so the sandboxed user can run it. - Carried runtime workspace roots through `FileSystemSandboxContext` as `PathUri` values so `:workspace_roots` policies resolve correctly without sending native client paths over exec-server JSON. - Preserved wrapper setup identity environment needed by Windows sandbox setup without changing the serialized inner helper environment. ## Verification - `just bazel-lock-update` - `just bazel-lock-check` - `just test -p codex-sandboxing transform_for_direct_spawn_windows` - `just test -p codex-exec-server fs_sandbox::tests` - `just fix -p codex-windows-sandbox -p codex-sandboxing -p codex-exec-server -p codex-core -p codex-file-system` Local note: `just fmt` completed Rust formatting, but this workstation still fails the non-Rust formatter phases because uv cannot open its cache and the local buildifier/dotslash path is missing.
This commit is contained in:
committed by
GitHub
Unverified
parent
c78911e37f
commit
ef75171f18
@@ -9,6 +9,7 @@ use codex_protocol::permissions::FileSystemSandboxPolicy;
|
||||
use codex_protocol::permissions::FileSystemSpecialPath;
|
||||
use codex_protocol::permissions::NetworkSandboxPolicy;
|
||||
use codex_sandboxing::SandboxCommand;
|
||||
use codex_sandboxing::SandboxDirectSpawnTransformRequest;
|
||||
use codex_sandboxing::SandboxExecRequest;
|
||||
use codex_sandboxing::SandboxManager;
|
||||
use codex_sandboxing::SandboxTransformRequest;
|
||||
@@ -88,7 +89,7 @@ impl FileSystemSandboxRunner {
|
||||
&file_system_policy,
|
||||
network_policy,
|
||||
);
|
||||
let command = self.sandbox_exec_request(&permission_profile, &cwd.uri, sandbox)?;
|
||||
let command = self.sandbox_exec_request(&permission_profile, &cwd, sandbox)?;
|
||||
let request_json = serde_json::to_vec(&request).map_err(json_error)?;
|
||||
run_command(command, request_json).await
|
||||
}
|
||||
@@ -96,7 +97,7 @@ impl FileSystemSandboxRunner {
|
||||
fn sandbox_exec_request(
|
||||
&self,
|
||||
permission_profile: &PermissionProfile,
|
||||
cwd: &PathUri,
|
||||
cwd: &SandboxCwd,
|
||||
sandbox_context: &FileSystemSandboxContext,
|
||||
) -> Result<SandboxExecRequest, JSONRPCErrorError> {
|
||||
let helper = &self.runtime_paths.codex_self_exe;
|
||||
@@ -112,22 +113,36 @@ impl FileSystemSandboxRunner {
|
||||
let command = SandboxCommand {
|
||||
program: helper.as_path().as_os_str().to_owned(),
|
||||
args: vec![CODEX_FS_HELPER_ARG1.to_string()],
|
||||
cwd: cwd.clone(),
|
||||
cwd: cwd.uri.clone(),
|
||||
env: self.helper_env.clone(),
|
||||
additional_permissions: None,
|
||||
};
|
||||
let native_workspace_roots = sandbox_context
|
||||
.workspace_roots
|
||||
.iter()
|
||||
.map(native_workspace_root)
|
||||
.collect::<Result<Vec<_>, _>>()?;
|
||||
let workspace_roots = if native_workspace_roots.is_empty() {
|
||||
std::slice::from_ref(&cwd.native)
|
||||
} else {
|
||||
native_workspace_roots.as_slice()
|
||||
};
|
||||
sandbox_manager
|
||||
.transform(SandboxTransformRequest {
|
||||
command,
|
||||
permissions: permission_profile,
|
||||
sandbox,
|
||||
enforce_managed_network: false,
|
||||
network: None,
|
||||
sandbox_policy_cwd: cwd,
|
||||
codex_linux_sandbox_exe: self.runtime_paths.codex_linux_sandbox_exe.as_deref(),
|
||||
use_legacy_landlock: sandbox_context.use_legacy_landlock,
|
||||
windows_sandbox_level: sandbox_context.windows_sandbox_level,
|
||||
windows_sandbox_private_desktop: sandbox_context.windows_sandbox_private_desktop,
|
||||
.transform_for_direct_spawn(SandboxDirectSpawnTransformRequest {
|
||||
workspace_roots,
|
||||
transform: SandboxTransformRequest {
|
||||
command,
|
||||
permissions: permission_profile,
|
||||
sandbox,
|
||||
enforce_managed_network: false,
|
||||
network: None,
|
||||
sandbox_policy_cwd: &cwd.uri,
|
||||
codex_linux_sandbox_exe: self.runtime_paths.codex_linux_sandbox_exe.as_deref(),
|
||||
use_legacy_landlock: sandbox_context.use_legacy_landlock,
|
||||
windows_sandbox_level: sandbox_context.windows_sandbox_level,
|
||||
windows_sandbox_private_desktop: sandbox_context
|
||||
.windows_sandbox_private_desktop,
|
||||
},
|
||||
})
|
||||
.map_err(|err| invalid_request(format!("failed to prepare fs sandbox: {err}")))
|
||||
}
|
||||
@@ -158,6 +173,14 @@ fn native_sandbox_cwd(cwd: &PathUri) -> Result<AbsolutePathBuf, JSONRPCErrorErro
|
||||
.map_err(|err| invalid_request(err.to_string()))
|
||||
}
|
||||
|
||||
fn native_workspace_root(root: &PathUri) -> Result<AbsolutePathBuf, JSONRPCErrorError> {
|
||||
root.to_abs_path().map_err(|err| {
|
||||
invalid_request(format!(
|
||||
"file system sandbox workspace root is not native to this exec-server host: {err}"
|
||||
))
|
||||
})
|
||||
}
|
||||
|
||||
fn helper_read_roots(runtime_paths: &ExecServerRuntimePaths) -> Vec<AbsolutePathBuf> {
|
||||
let mut roots = Vec::new();
|
||||
for path in std::iter::once(runtime_paths.codex_self_exe.as_path())
|
||||
@@ -517,15 +540,21 @@ mod tests {
|
||||
let runner = FileSystemSandboxRunner::new(runtime_paths);
|
||||
let native_cwd = AbsolutePathBuf::current_dir().expect("cwd");
|
||||
let cwd = PathUri::from_abs_path(&native_cwd);
|
||||
let file_system_policy =
|
||||
restricted_policy(vec![path_entry(native_cwd, FileSystemAccessMode::Write)]);
|
||||
let file_system_policy = restricted_policy(vec![path_entry(
|
||||
native_cwd.clone(),
|
||||
FileSystemAccessMode::Write,
|
||||
)]);
|
||||
let network_policy = NetworkSandboxPolicy::Restricted;
|
||||
let permission_profile =
|
||||
PermissionProfile::from_runtime_permissions(&file_system_policy, network_policy);
|
||||
let sandbox_context = sandbox_context_with_cwd(&file_system_policy, cwd.clone());
|
||||
let sandbox_cwd = SandboxCwd {
|
||||
uri: cwd,
|
||||
native: native_cwd,
|
||||
};
|
||||
|
||||
let request = runner
|
||||
.sandbox_exec_request(&permission_profile, &cwd, &sandbox_context)
|
||||
.sandbox_exec_request(&permission_profile, &sandbox_cwd, &sandbox_context)
|
||||
.expect("sandbox exec request");
|
||||
|
||||
assert_eq!(request.env.get(&path_key), Some(&path));
|
||||
|
||||
@@ -19,6 +19,7 @@ pub(crate) mod exec_server;
|
||||
pub(crate) const DELAYED_OUTPUT_AFTER_EXIT_PARENT_ARG: &str =
|
||||
"--codex-test-delayed-output-after-exit-parent";
|
||||
|
||||
const CODEX_WINDOWS_SANDBOX_ARG1: &str = "--run-as-windows-sandbox";
|
||||
const DELAYED_OUTPUT_AFTER_EXIT_CHILD_ARG: &str = "--codex-test-delayed-output-after-exit-child";
|
||||
|
||||
#[ctor]
|
||||
@@ -27,6 +28,9 @@ pub static TEST_BINARY_DISPATCH_GUARD: Option<TestBinaryDispatchGuard> = {
|
||||
if argv1 == Some(CODEX_FS_HELPER_ARG1) {
|
||||
return TestBinaryDispatchMode::DispatchArg0Only;
|
||||
}
|
||||
if argv1 == Some(CODEX_WINDOWS_SANDBOX_ARG1) {
|
||||
return TestBinaryDispatchMode::DispatchArg0Only;
|
||||
}
|
||||
if exe_name == CODEX_LINUX_SANDBOX_ARG0 {
|
||||
return TestBinaryDispatchMode::DispatchArg0Only;
|
||||
}
|
||||
|
||||
@@ -12,9 +12,14 @@ use std::path::Path;
|
||||
use std::process::Command;
|
||||
|
||||
use anyhow::Result;
|
||||
use codex_exec_server::FileSystemSandboxContext;
|
||||
use codex_protocol::config_types::WindowsSandboxLevel;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use test_case::test_case;
|
||||
|
||||
use crate::support::FileSystemImplementation;
|
||||
use crate::support::create_file_system_context;
|
||||
|
||||
fn create_directory_junction(target: &Path, alias: &Path) -> Result<()> {
|
||||
let output = Command::new("cmd")
|
||||
@@ -54,3 +59,58 @@ async fn file_system_sandboxed_canonicalize_resolves_directory_junction(
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn file_system_remote_fs_helper_respects_windows_sandbox_write_policy() -> Result<()> {
|
||||
let context = create_file_system_context(FileSystemImplementation::Remote).await?;
|
||||
let file_system = context.file_system;
|
||||
let tmp = tempfile::TempDir::new()?;
|
||||
let readonly_dir = tmp.path().join("readonly");
|
||||
std::fs::create_dir_all(&readonly_dir)?;
|
||||
|
||||
let mut sandbox = read_only_sandbox_for_cwd(readonly_dir.clone())?;
|
||||
sandbox.windows_sandbox_level = WindowsSandboxLevel::RestrictedToken;
|
||||
|
||||
let readable_file = readonly_dir.join("readable.txt");
|
||||
std::fs::write(&readable_file, b"readable")?;
|
||||
let read_result = file_system
|
||||
.read_file(&PathUri::from_path(&readable_file)?, Some(&sandbox))
|
||||
.await;
|
||||
// Some local Windows hosts cannot create restricted tokens. Reaching that
|
||||
// error still proves the remote fs helper went through the Windows sandbox
|
||||
// launcher; before the wrapper fix this read would have run unsandboxed.
|
||||
if is_unsupported_restricted_token_host(&read_result) {
|
||||
return Ok(());
|
||||
}
|
||||
assert_eq!(read_result?, b"readable");
|
||||
|
||||
let blocked_file = readonly_dir.join("blocked.txt");
|
||||
let error = file_system
|
||||
.write_file(
|
||||
&PathUri::from_path(&blocked_file)?,
|
||||
b"blocked".to_vec(),
|
||||
Some(&sandbox),
|
||||
)
|
||||
.await
|
||||
.expect_err("write outside the sandbox should fail");
|
||||
assert!(
|
||||
!blocked_file.exists(),
|
||||
"sandboxed fs helper must not create blocked file after error: {error}"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn read_only_sandbox_for_cwd(cwd: std::path::PathBuf) -> Result<FileSystemSandboxContext> {
|
||||
Ok(FileSystemSandboxContext::from_legacy_sandbox_policy(
|
||||
SandboxPolicy::new_read_only_policy(),
|
||||
PathUri::from_path(cwd)?,
|
||||
)?)
|
||||
}
|
||||
|
||||
fn is_unsupported_restricted_token_host<T>(result: &std::io::Result<T>) -> bool {
|
||||
result.as_ref().err().is_some_and(|err| {
|
||||
err.to_string()
|
||||
.contains("windows sandbox failed: CreateRestrictedToken failed: 87")
|
||||
})
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user