Files
codex/codex-rs/sandboxing/src/policy_transforms_tests.rs
viyatb-oai b114781495 fix(permissions): fix symlinked writable roots in sandbox permissions (#15981)
## Summary
- preserve logical symlink paths during permission normalization and
config cwd handling
- bind real targets for symlinked readable/writable roots in bwrap and
remap carveouts and unreadable roots there
- add regressions for symlinked carveouts and nested symlink escape
masking

## Root cause
Permission normalization canonicalized symlinked writable roots and cwd
to their real targets too early. That drifted policy checks away from
the logical paths the sandboxed process can actually address, while
bwrap still needed the real targets for mounts. The mismatch caused
shell and apply_patch failures on symlinked writable roots.

## Impact
Fixes #15781.

Also fixes #17079:
- #17079 is the protected symlinked carveout side: bwrap now binds the
real symlinked writable-root target and remaps carveouts before masking.

Related to #15157:
- #15157 is the broader permission-check side of this path-identity
problem. This PR addresses the shared logical-vs-canonical normalization
issue, but the reported Darwin prompt behavior should be validated
separately before auto-closing it.

This should also fix #14672, #14694, #14715, and #15725:
- #14672, #14694, and #14715 are the same Linux
symlinked-writable-root/bwrap family as #15781.
- #15725 is the protected symlinked workspace path variant; the PR
preserves the protected logical path in policy space while bwrap applies
read-only or unreadable treatment to the resolved target so
file-vs-directory bind mismatches do not abort sandbox setup.

## Notes
- Added Linux-only regressions for symlinked writable ancestors and
protected symlinked directory targets, including nested symlink escape
masking without rebinding the escape target writable.

---------

Co-authored-by: Codex <noreply@openai.com>
2026-04-10 17:00:58 -07:00

430 lines
14 KiB
Rust

use super::effective_file_system_sandbox_policy;
use super::intersect_permission_profiles;
use super::merge_file_system_policy_with_additional_permissions;
use super::normalize_additional_permissions;
use super::sandbox_policy_with_additional_permissions;
use super::should_require_platform_sandbox;
use codex_protocol::models::FileSystemPermissions;
use codex_protocol::models::NetworkPermissions;
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_protocol::protocol::NetworkAccess;
use codex_protocol::protocol::ReadOnlyAccess;
use codex_protocol::protocol::SandboxPolicy;
use codex_utils_absolute_path::AbsolutePathBuf;
use dunce::canonicalize;
use pretty_assertions::assert_eq;
#[cfg(unix)]
use std::path::Path;
use tempfile::TempDir;
#[cfg(unix)]
fn symlink_dir(original: &Path, link: &Path) -> std::io::Result<()> {
std::os::unix::fs::symlink(original, link)
}
#[test]
fn full_access_restricted_policy_skips_platform_sandbox_when_network_is_enabled() {
let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::Root,
},
access: FileSystemAccessMode::Write,
}]);
assert_eq!(
should_require_platform_sandbox(
&policy,
NetworkSandboxPolicy::Enabled,
/*has_managed_network_requirements*/ false
),
false
);
}
#[test]
fn root_write_policy_with_carveouts_still_uses_platform_sandbox() {
let blocked = AbsolutePathBuf::resolve_path_against_base(
"blocked",
std::env::current_dir().expect("current dir"),
);
let policy = FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::Root,
},
access: FileSystemAccessMode::Write,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path { path: blocked },
access: FileSystemAccessMode::None,
},
]);
assert_eq!(
should_require_platform_sandbox(
&policy,
NetworkSandboxPolicy::Enabled,
/*has_managed_network_requirements*/ false
),
true
);
}
#[test]
fn full_access_restricted_policy_still_uses_platform_sandbox_for_restricted_network() {
let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::Root,
},
access: FileSystemAccessMode::Write,
}]);
assert_eq!(
should_require_platform_sandbox(
&policy,
NetworkSandboxPolicy::Restricted,
/*has_managed_network_requirements*/ false
),
true
);
}
#[test]
fn normalize_additional_permissions_preserves_network() {
let temp_dir = TempDir::new().expect("create temp dir");
let path = AbsolutePathBuf::from_absolute_path(
canonicalize(temp_dir.path()).expect("canonicalize temp dir"),
)
.expect("absolute temp dir");
let permissions = normalize_additional_permissions(PermissionProfile {
network: Some(NetworkPermissions {
enabled: Some(true),
}),
file_system: Some(FileSystemPermissions {
read: Some(vec![path.clone()]),
write: Some(vec![path.clone()]),
}),
})
.expect("permissions");
assert_eq!(
permissions.network,
Some(NetworkPermissions {
enabled: Some(true),
})
);
assert_eq!(
permissions.file_system,
Some(FileSystemPermissions {
read: Some(vec![path.clone()]),
write: Some(vec![path]),
})
);
}
#[cfg(unix)]
#[test]
fn normalize_additional_permissions_preserves_symlinked_write_paths() {
let temp_dir = TempDir::new().expect("create temp dir");
let real_root = temp_dir.path().join("real");
let link_root = temp_dir.path().join("link");
let write_dir = real_root.join("write");
std::fs::create_dir_all(&write_dir).expect("create write dir");
symlink_dir(&real_root, &link_root).expect("create symlinked root");
let link_write_dir =
AbsolutePathBuf::from_absolute_path(link_root.join("write")).expect("link write dir");
let permissions = normalize_additional_permissions(PermissionProfile {
file_system: Some(FileSystemPermissions {
read: Some(vec![]),
write: Some(vec![link_write_dir]),
}),
..Default::default()
})
.expect("permissions");
assert_eq!(
permissions.file_system,
Some(FileSystemPermissions {
read: Some(vec![]),
write: Some(vec![
AbsolutePathBuf::from_absolute_path(link_root.join("write"))
.expect("link write dir")
]),
})
);
}
#[test]
fn normalize_additional_permissions_drops_empty_nested_profiles() {
let permissions = normalize_additional_permissions(PermissionProfile {
network: Some(NetworkPermissions { enabled: None }),
file_system: Some(FileSystemPermissions {
read: None,
write: None,
}),
})
.expect("permissions");
assert_eq!(permissions, PermissionProfile::default());
}
#[test]
fn intersect_permission_profiles_preserves_explicit_empty_requested_reads() {
let temp_dir = TempDir::new().expect("create temp dir");
let path = AbsolutePathBuf::from_absolute_path(
canonicalize(temp_dir.path()).expect("canonicalize temp dir"),
)
.expect("absolute temp dir");
let requested = PermissionProfile {
file_system: Some(FileSystemPermissions {
read: Some(vec![]),
write: Some(vec![path]),
}),
..Default::default()
};
let granted = requested.clone();
assert_eq!(
intersect_permission_profiles(requested.clone(), granted),
requested
);
}
#[test]
fn intersect_permission_profiles_drops_ungranted_nonempty_path_requests() {
let temp_dir = TempDir::new().expect("create temp dir");
let path = AbsolutePathBuf::from_absolute_path(
canonicalize(temp_dir.path()).expect("canonicalize temp dir"),
)
.expect("absolute temp dir");
let requested = PermissionProfile {
file_system: Some(FileSystemPermissions {
read: Some(vec![path]),
write: None,
}),
..Default::default()
};
assert_eq!(
intersect_permission_profiles(requested, PermissionProfile::default()),
PermissionProfile::default()
);
}
#[test]
fn intersect_permission_profiles_drops_explicit_empty_reads_without_grant() {
let temp_dir = TempDir::new().expect("create temp dir");
let path = AbsolutePathBuf::from_absolute_path(
canonicalize(temp_dir.path()).expect("canonicalize temp dir"),
)
.expect("absolute temp dir");
let requested = PermissionProfile {
file_system: Some(FileSystemPermissions {
read: Some(vec![]),
write: Some(vec![path]),
}),
..Default::default()
};
assert_eq!(
intersect_permission_profiles(requested, PermissionProfile::default()),
PermissionProfile::default()
);
}
#[test]
fn read_only_additional_permissions_can_enable_network_without_writes() {
let temp_dir = TempDir::new().expect("create temp dir");
let path = AbsolutePathBuf::from_absolute_path(
canonicalize(temp_dir.path()).expect("canonicalize temp dir"),
)
.expect("absolute temp dir");
let policy = sandbox_policy_with_additional_permissions(
&SandboxPolicy::ReadOnly {
access: ReadOnlyAccess::Restricted {
include_platform_defaults: true,
readable_roots: vec![path.clone()],
},
network_access: false,
},
&PermissionProfile {
network: Some(NetworkPermissions {
enabled: Some(true),
}),
file_system: Some(FileSystemPermissions {
read: Some(vec![path.clone()]),
write: Some(Vec::new()),
}),
},
);
assert_eq!(
policy,
SandboxPolicy::ReadOnly {
access: ReadOnlyAccess::Restricted {
include_platform_defaults: true,
readable_roots: vec![path],
},
network_access: true,
}
);
}
#[test]
fn external_sandbox_additional_permissions_can_enable_network() {
let temp_dir = TempDir::new().expect("create temp dir");
let path = AbsolutePathBuf::from_absolute_path(
canonicalize(temp_dir.path()).expect("canonicalize temp dir"),
)
.expect("absolute temp dir");
let policy = sandbox_policy_with_additional_permissions(
&SandboxPolicy::ExternalSandbox {
network_access: NetworkAccess::Restricted,
},
&PermissionProfile {
network: Some(NetworkPermissions {
enabled: Some(true),
}),
file_system: Some(FileSystemPermissions {
read: Some(vec![path]),
write: Some(Vec::new()),
}),
},
);
assert_eq!(
policy,
SandboxPolicy::ExternalSandbox {
network_access: NetworkAccess::Enabled,
}
);
}
#[test]
fn merge_file_system_policy_with_additional_permissions_preserves_unreadable_roots() {
let temp_dir = TempDir::new().expect("create temp dir");
let cwd = AbsolutePathBuf::from_absolute_path(
canonicalize(temp_dir.path()).expect("canonicalize temp dir"),
)
.expect("absolute temp dir");
let allowed_path = cwd.join("allowed");
let denied_path = cwd.join("denied");
let merged_policy = merge_file_system_policy_with_additional_permissions(
&FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::Root,
},
access: FileSystemAccessMode::Read,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: denied_path.clone(),
},
access: FileSystemAccessMode::None,
},
]),
vec![allowed_path.clone()],
Vec::new(),
);
assert_eq!(
merged_policy.entries.contains(&FileSystemSandboxEntry {
path: FileSystemPath::Path { path: denied_path },
access: FileSystemAccessMode::None,
}),
true
);
assert_eq!(
merged_policy.entries.contains(&FileSystemSandboxEntry {
path: FileSystemPath::Path { path: allowed_path },
access: FileSystemAccessMode::Read,
}),
true
);
}
#[test]
fn effective_file_system_sandbox_policy_returns_base_policy_without_additional_permissions() {
let temp_dir = TempDir::new().expect("create temp dir");
let cwd = AbsolutePathBuf::from_absolute_path(
canonicalize(temp_dir.path()).expect("canonicalize temp dir"),
)
.expect("absolute temp dir");
let denied_path = cwd.join("denied");
let base_policy = FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::Root,
},
access: FileSystemAccessMode::Read,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path { path: denied_path },
access: FileSystemAccessMode::None,
},
]);
let effective_policy =
effective_file_system_sandbox_policy(&base_policy, /*additional_permissions*/ None);
assert_eq!(effective_policy, base_policy);
}
#[test]
fn effective_file_system_sandbox_policy_merges_additional_write_roots() {
let temp_dir = TempDir::new().expect("create temp dir");
let cwd = AbsolutePathBuf::from_absolute_path(
canonicalize(temp_dir.path()).expect("canonicalize temp dir"),
)
.expect("absolute temp dir");
let allowed_path = cwd.join("allowed");
let denied_path = cwd.join("denied");
let base_policy = FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::Root,
},
access: FileSystemAccessMode::Read,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: denied_path.clone(),
},
access: FileSystemAccessMode::None,
},
]);
let additional_permissions = PermissionProfile {
file_system: Some(FileSystemPermissions {
read: Some(vec![]),
write: Some(vec![allowed_path.clone()]),
}),
..Default::default()
};
let effective_policy =
effective_file_system_sandbox_policy(&base_policy, Some(&additional_permissions));
assert_eq!(
effective_policy.entries.contains(&FileSystemSandboxEntry {
path: FileSystemPath::Path { path: denied_path },
access: FileSystemAccessMode::None,
}),
true
);
assert_eq!(
effective_policy.entries.contains(&FileSystemSandboxEntry {
path: FileSystemPath::Path { path: allowed_path },
access: FileSystemAccessMode::Write,
}),
true
);
}