Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 27 additions & 15 deletions codex-rs/core/tests/suite/remote_env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,26 +59,38 @@ fn absolute_path(path: PathBuf) -> AbsolutePathBuf {
}

fn read_only_sandbox(readable_root: PathBuf) -> FileSystemSandboxContext {
FileSystemSandboxContext::new(SandboxPolicy::ReadOnly {
access: ReadOnlyAccess::Restricted {
include_platform_defaults: false,
readable_roots: vec![absolute_path(readable_root)],
let readable_root = absolute_path(readable_root);
// The policy is evaluated in the remote container, so use a container path
// for cwd instead of capturing the local test runner cwd.
FileSystemSandboxContext::from_legacy_sandbox_policy(
SandboxPolicy::ReadOnly {
access: ReadOnlyAccess::Restricted {
include_platform_defaults: false,
readable_roots: vec![readable_root.clone()],
},
network_access: false,
},
network_access: false,
})
readable_root,
)
}

fn workspace_write_sandbox(writable_root: PathBuf) -> FileSystemSandboxContext {
FileSystemSandboxContext::new(SandboxPolicy::WorkspaceWrite {
writable_roots: vec![absolute_path(writable_root)],
read_only_access: ReadOnlyAccess::Restricted {
include_platform_defaults: false,
readable_roots: vec![],
let writable_root = absolute_path(writable_root);
// The policy is evaluated in the remote container, so use a container path
// for cwd instead of capturing the local test runner cwd.
FileSystemSandboxContext::from_legacy_sandbox_policy(
SandboxPolicy::WorkspaceWrite {
writable_roots: vec![writable_root.clone()],
read_only_access: ReadOnlyAccess::Restricted {
include_platform_defaults: false,
readable_roots: vec![],
},
network_access: false,
exclude_tmpdir_env_var: true,
exclude_slash_tmp: true,
},
network_access: false,
exclude_tmpdir_env_var: true,
exclude_slash_tmp: true,
})
writable_root,
)
}

fn assert_normalized_path_rejected(error: &std::io::Error) {
Expand Down
28 changes: 28 additions & 0 deletions codex-rs/exec-server/src/file_system.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
use async_trait::async_trait;
use codex_protocol::config_types::WindowsSandboxLevel;
use codex_protocol::models::PermissionProfile;
use codex_protocol::permissions::FileSystemPath;
use codex_protocol::permissions::FileSystemSandboxKind;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::permissions::FileSystemSpecialPath;
use codex_protocol::permissions::NetworkSandboxPolicy;
use codex_protocol::protocol::SandboxPolicy;
use codex_utils_absolute_path::AbsolutePathBuf;
use std::path::Path;
use tokio::io;

#[derive(Clone, Copy, Debug, Eq, PartialEq)]
Expand Down Expand Up @@ -103,6 +106,31 @@ impl FileSystemSandboxContext {
matches!(file_system_policy.kind, FileSystemSandboxKind::Restricted)
&& !file_system_policy.has_full_disk_write_access()
}

pub(crate) fn drop_cwd_if_unused(mut self) -> Self {
let file_system_policy = self.permissions.file_system_sandbox_policy();
if !file_system_policy_has_cwd_dependent_entries(&file_system_policy) {
self.cwd = None;
}
self
}
}

pub(crate) fn file_system_policy_has_cwd_dependent_entries(
file_system_policy: &FileSystemSandboxPolicy,
) -> bool {
file_system_policy
.entries
.iter()
.any(|entry| match &entry.path {
FileSystemPath::GlobPattern { pattern } => !Path::new(pattern).is_absolute(),
FileSystemPath::Special {
value:
FileSystemSpecialPath::CurrentWorkingDirectory
| FileSystemSpecialPath::ProjectRoots { .. },
} => true,
FileSystemPath::Path { .. } | FileSystemPath::Special { .. } => false,
})
}

pub type FileSystemResult<T> = io::Result<T>;
Expand Down
113 changes: 69 additions & 44 deletions codex-rs/exec-server/src/fs_sandbox.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use tokio::process::Command;

use crate::ExecServerRuntimePaths;
use crate::FileSystemSandboxContext;
use crate::file_system::file_system_policy_has_cwd_dependent_entries;
use crate::fs_helper::CODEX_FS_HELPER_ARG1;
use crate::fs_helper::FsHelperPayload;
use crate::fs_helper::FsHelperRequest;
Expand Down Expand Up @@ -52,12 +53,12 @@ impl FileSystemSandboxRunner {
) -> Result<FsHelperPayload, JSONRPCErrorError> {
let cwd = sandbox_cwd(sandbox)?;
let mut file_system_policy = sandbox.permissions.file_system_sandbox_policy();
let helper_read_root = if sandbox.use_legacy_landlock {
None
let helper_read_roots = if sandbox.use_legacy_landlock {
Vec::new()
} else {
helper_read_root(&self.runtime_paths)
helper_read_roots(&self.runtime_paths)
};
add_helper_runtime_permissions(&mut file_system_policy, helper_read_root, cwd.as_path());
add_helper_runtime_permissions(&mut file_system_policy, &helper_read_roots, cwd.as_path());
normalize_file_system_policy_root_aliases(&mut file_system_policy);
let network_policy = NetworkSandboxPolicy::Restricted;
let sandbox_policy =
Expand Down Expand Up @@ -133,33 +134,24 @@ fn sandbox_cwd(sandbox: &FileSystemSandboxContext) -> Result<AbsolutePathBuf, JS
.map_err(|err| invalid_request(format!("current directory is not absolute: {err}")))
}

fn file_system_policy_has_cwd_dependent_entries(
file_system_policy: &FileSystemSandboxPolicy,
) -> bool {
file_system_policy
.entries
.iter()
.any(|entry| match &entry.path {
FileSystemPath::GlobPattern { pattern } => !std::path::Path::new(pattern).is_absolute(),
FileSystemPath::Special {
value:
FileSystemSpecialPath::CurrentWorkingDirectory
| FileSystemSpecialPath::ProjectRoots { .. },
} => true,
FileSystemPath::Path { .. } | FileSystemPath::Special { .. } => false,
})
}

fn helper_read_root(runtime_paths: &ExecServerRuntimePaths) -> Option<AbsolutePathBuf> {
runtime_paths
.codex_self_exe
.parent()
.and_then(|path| AbsolutePathBuf::from_absolute_path(path).ok())
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())
.chain(runtime_paths.codex_linux_sandbox_exe.as_deref().into_iter())
{
if let Some(parent) = path.parent()
&& let Ok(root) = AbsolutePathBuf::from_absolute_path(parent)
&& !roots.contains(&root)
{
roots.push(root);
}
}
roots
}

fn add_helper_runtime_permissions(
file_system_policy: &mut FileSystemSandboxPolicy,
helper_read_root: Option<AbsolutePathBuf>,
helper_read_roots: &[AbsolutePathBuf],
cwd: &std::path::Path,
) {
if !file_system_policy.has_full_disk_read_access() {
Expand All @@ -174,19 +166,18 @@ fn add_helper_runtime_permissions(
}
}

let Some(helper_read_root) = helper_read_root else {
return;
};
if file_system_policy.can_read_path_with_cwd(helper_read_root.as_path(), cwd) {
return;
}
for helper_read_root in helper_read_roots {
if file_system_policy.can_read_path_with_cwd(helper_read_root.as_path(), cwd) {
continue;
}

file_system_policy.entries.push(FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: helper_read_root,
},
access: FileSystemAccessMode::Read,
});
file_system_policy.entries.push(FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: helper_read_root.clone(),
},
access: FileSystemAccessMode::Read,
});
}
}

fn compatibility_sandbox_policy(
Expand Down Expand Up @@ -371,7 +362,7 @@ mod tests {
use super::helper_env;
use super::helper_env_from_vars;
use super::helper_env_key_is_allowed;
use super::helper_read_root;
use super::helper_read_roots;
use super::sandbox_cwd;

#[test]
Expand All @@ -388,7 +379,7 @@ mod tests {
let mut policy =
FileSystemSandboxPolicy::from_legacy_sandbox_policy(&sandbox_policy, cwd.as_path());

add_helper_runtime_permissions(&mut policy, /*helper_read_root*/ None, cwd.as_path());
add_helper_runtime_permissions(&mut policy, /*helper_read_roots*/ &[], cwd.as_path());

assert!(policy.include_platform_defaults());
}
Expand All @@ -410,7 +401,7 @@ mod tests {
let mut policy =
FileSystemSandboxPolicy::from_legacy_sandbox_policy(&sandbox_policy, cwd.as_path());

add_helper_runtime_permissions(&mut policy, /*helper_read_root*/ None, cwd.as_path());
add_helper_runtime_permissions(&mut policy, /*helper_read_roots*/ &[], cwd.as_path());

assert!(policy.include_platform_defaults());
}
Expand Down Expand Up @@ -449,7 +440,7 @@ mod tests {

add_helper_runtime_permissions(
&mut policy,
helper_read_root(&runtime_paths),
&helper_read_roots(&runtime_paths),
cwd.as_path(),
);

Expand Down Expand Up @@ -611,10 +602,44 @@ mod tests {

add_helper_runtime_permissions(
&mut policy,
helper_read_root(&runtime_paths),
&helper_read_roots(&runtime_paths),
cwd.as_path(),
);

assert!(policy.can_read_path_with_cwd(readable.as_path(), cwd.as_path()));
}

#[test]
fn helper_permissions_include_linux_sandbox_alias_parent() {
let root = tempfile::tempdir().expect("temp dir");
let codex_self_exe = root.path().join("bin").join("codex");
let codex_linux_sandbox_exe = root.path().join("aliases").join("codex-linux-sandbox");
let runtime_paths =
ExecServerRuntimePaths::new(codex_self_exe, Some(codex_linux_sandbox_exe))
.expect("runtime paths");
let cwd = AbsolutePathBuf::from_absolute_path(std::env::temp_dir().as_path())
.expect("absolute cwd");
let sandbox_policy = SandboxPolicy::ReadOnly {
access: ReadOnlyAccess::Restricted {
include_platform_defaults: false,
readable_roots: Vec::new(),
},
network_access: false,
};
let mut policy =
FileSystemSandboxPolicy::from_legacy_sandbox_policy(&sandbox_policy, cwd.as_path());
let codex_parent = AbsolutePathBuf::from_absolute_path(root.path().join("bin"))
.expect("absolute codex parent");
let alias_parent = AbsolutePathBuf::from_absolute_path(root.path().join("aliases"))
.expect("absolute alias parent");

add_helper_runtime_permissions(
&mut policy,
&helper_read_roots(&runtime_paths),
cwd.as_path(),
);

assert!(policy.can_read_path_with_cwd(codex_parent.as_path(), cwd.as_path()));
assert!(policy.can_read_path_with_cwd(alias_parent.as_path(), cwd.as_path()));
}
}
Loading
Loading