mirror of
https://github.com/openai/codex.git
synced 2026-10-19 17:28:46 +00:00
exec-server: materialize filesystem workspace roots (#31892)
Filesystem helper requests currently turn symbolic `:workspace_roots` permissions into a sandbox policy before applying the workspace roots from the filesystem sandbox context. This can accidentally broaden filesystem access to the cwd instead of limiting it to the selected workspace roots. Materialize project-root permissions using the context workspace roots before deriving the filesystem sandbox policy. The same converted roots are then reused when constructing the sandbox command, keeping policy enforcement and process setup aligned. Adds a remote filesystem integration test that verifies a file inside the selected workspace root is readable while a sibling under the cwd is denied. ## Validation - `just test -p codex-exec-server remote_read_file_materializes_environment_workspace_roots` (macOS, outside the outer Seatbelt sandbox)
This commit is contained in:
@@ -67,10 +67,22 @@ impl FileSystemSandboxRunner {
|
||||
request: FsHelperRequest,
|
||||
) -> Result<FsHelperPayload, JSONRPCErrorError> {
|
||||
let cwd = sandbox_cwd(sandbox)?;
|
||||
let native_workspace_roots = sandbox
|
||||
.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()
|
||||
};
|
||||
let native_permissions: PermissionProfile =
|
||||
sandbox.permissions.clone().try_into().map_err(|err| {
|
||||
invalid_request(format!("invalid sandbox permission path URI: {err}"))
|
||||
})?;
|
||||
let native_permissions =
|
||||
native_permissions.materialize_project_roots_with_workspace_roots(workspace_roots);
|
||||
let mut file_system_policy = native_permissions.file_system_sandbox_policy();
|
||||
let helper_read_roots = if sandbox.use_legacy_landlock {
|
||||
Vec::new()
|
||||
@@ -89,7 +101,8 @@ impl FileSystemSandboxRunner {
|
||||
&file_system_policy,
|
||||
network_policy,
|
||||
);
|
||||
let command = self.sandbox_exec_request(&permission_profile, &cwd, sandbox)?;
|
||||
let command =
|
||||
self.sandbox_exec_request(&permission_profile, &cwd, workspace_roots, sandbox)?;
|
||||
let request_json = serde_json::to_vec(&request).map_err(json_error)?;
|
||||
run_command(command, request_json).await
|
||||
}
|
||||
@@ -98,6 +111,7 @@ impl FileSystemSandboxRunner {
|
||||
&self,
|
||||
permission_profile: &PermissionProfile,
|
||||
cwd: &SandboxCwd,
|
||||
workspace_roots: &[AbsolutePathBuf],
|
||||
sandbox_context: &FileSystemSandboxContext,
|
||||
) -> Result<SandboxExecRequest, JSONRPCErrorError> {
|
||||
let helper = &self.runtime_paths.codex_self_exe;
|
||||
@@ -118,16 +132,6 @@ impl FileSystemSandboxRunner {
|
||||
managed_network: None,
|
||||
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_for_direct_spawn(SandboxDirectSpawnTransformRequest {
|
||||
workspace_roots,
|
||||
@@ -560,7 +564,12 @@ mod tests {
|
||||
};
|
||||
|
||||
let request = runner
|
||||
.sandbox_exec_request(&permission_profile, &sandbox_cwd, &sandbox_context)
|
||||
.sandbox_exec_request(
|
||||
&permission_profile,
|
||||
&sandbox_cwd,
|
||||
std::slice::from_ref(&sandbox_cwd.native),
|
||||
&sandbox_context,
|
||||
)
|
||||
.expect("sandbox exec request");
|
||||
|
||||
assert_eq!(request.env.get(&path_key), Some(&path));
|
||||
|
||||
@@ -23,11 +23,19 @@ use codex_exec_server::CreateDirectoryOptions;
|
||||
#[cfg(target_os = "linux")]
|
||||
use codex_exec_server::Environment;
|
||||
use codex_exec_server::FileMetadata;
|
||||
use codex_exec_server::FileSystemSandboxContext;
|
||||
use codex_exec_server::RemoveOptions;
|
||||
use codex_exec_server::WalkEntry;
|
||||
use codex_exec_server::WalkEntryKind;
|
||||
use codex_exec_server::WalkOptions;
|
||||
use codex_exec_server::WalkOutcome;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::permissions::FileSystemAccessMode;
|
||||
use codex_protocol::permissions::FileSystemPath;
|
||||
use codex_protocol::permissions::FileSystemSandboxEntry;
|
||||
use codex_protocol::permissions::FileSystemSandboxPolicy;
|
||||
use codex_protocol::permissions::FileSystemSpecialPath;
|
||||
use codex_protocol::permissions::NetworkSandboxPolicy;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tempfile::TempDir;
|
||||
@@ -210,6 +218,51 @@ async fn sandboxed_file_system_helper_finds_bwrap_on_preserved_path() -> Result<
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn remote_read_file_materializes_environment_workspace_roots() -> Result<()> {
|
||||
let context = create_file_system_context(FileSystemImplementation::Remote).await?;
|
||||
let file_system = context.file_system;
|
||||
let tmp = TempDir::new()?;
|
||||
let workspace = tmp.path().join("workspace");
|
||||
let workspace_file = workspace.join("included.txt");
|
||||
let excluded_file = tmp.path().join("excluded.txt");
|
||||
std::fs::create_dir(&workspace)?;
|
||||
std::fs::write(&workspace_file, b"included")?;
|
||||
std::fs::write(&excluded_file, b"excluded")?;
|
||||
|
||||
let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Special {
|
||||
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
|
||||
},
|
||||
access: FileSystemAccessMode::Read,
|
||||
}]);
|
||||
let mut sandbox = FileSystemSandboxContext::from_permission_profile_with_cwd(
|
||||
PermissionProfile::from_runtime_permissions(&policy, NetworkSandboxPolicy::Restricted),
|
||||
PathUri::from_host_native_path(tmp.path())?,
|
||||
);
|
||||
sandbox.workspace_roots = vec![PathUri::from_host_native_path(&workspace)?];
|
||||
|
||||
assert_eq!(
|
||||
file_system
|
||||
.read_file(
|
||||
&PathUri::from_host_native_path(&workspace_file)?,
|
||||
Some(&sandbox),
|
||||
)
|
||||
.await?,
|
||||
b"included"
|
||||
);
|
||||
let error = file_system
|
||||
.read_file(
|
||||
&PathUri::from_host_native_path(&excluded_file)?,
|
||||
Some(&sandbox),
|
||||
)
|
||||
.await
|
||||
.expect_err("read outside environment workspace roots should fail");
|
||||
assert_sandbox_denied(&error);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test_case(FileSystemImplementation::Local ; "local")]
|
||||
#[test_case(FileSystemImplementation::Remote ; "remote")]
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
|
||||
Reference in New Issue
Block a user