mirror of
https://github.com/openai/codex.git
synced 2026-04-25 15:15:15 +00:00
## Why After `#13449`, the Linux helper could receive split filesystem and network policies, but the bubblewrap mount builder still reconstructed filesystem access from the legacy `SandboxPolicy`. That loses explicit unreadable carveouts under writable roots, and it also mishandles `Root` read access paired with explicit deny carveouts. In those cases bubblewrap could still expose paths that the split filesystem policy intentionally blocked. ## What changed - switched bubblewrap mount generation to consume `FileSystemSandboxPolicy` directly at the implementation boundary; legacy `SandboxPolicy` configs still flow through the existing `FileSystemSandboxPolicy::from(&sandbox_policy)` bridge before reaching bwrap - kept the Linux helper and preflight path on the split filesystem policy all the way into bwrap - re-applied explicit unreadable carveouts after readable and writable mounts so blocked subpaths still win under bubblewrap - masked denied directories with `--tmpfs` plus `--remount-ro` and denied files with `--ro-bind-data`, preserving the backing fd until exec - added comments in the unreadable-root masking block to explain why the mount order and directory/file split are intentional - updated Linux helper call sites and tests for the split-policy bwrap path ## Verification - added protocol coverage for root carveouts staying scoped - added core coverage that root-write plus deny carveouts still requires a platform sandbox - added bwrap unit coverage for reapplying blocked carveouts after writable binds - added Linux integration coverage for explicit split-policy carveouts under bubblewrap - validated the final branch state with `cargo test -p codex-linux-sandbox`, `cargo clippy -p codex-linux-sandbox --all-targets -- -D warnings`, and the PR CI reruns --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/13453). * __->__ #13453 * #13452 * #13451 * #13449 * #13448 * #13445 * #13440 * #13439 --------- Co-authored-by: viyatb-oai <viyatb@openai.com>
258 lines
8.3 KiB
Rust
258 lines
8.3 KiB
Rust
#[cfg(test)]
|
|
use super::*;
|
|
#[cfg(test)]
|
|
use codex_protocol::protocol::FileSystemSandboxPolicy;
|
|
#[cfg(test)]
|
|
use codex_protocol::protocol::NetworkSandboxPolicy;
|
|
#[cfg(test)]
|
|
use codex_protocol::protocol::SandboxPolicy;
|
|
#[cfg(test)]
|
|
use pretty_assertions::assert_eq;
|
|
|
|
#[test]
|
|
fn detects_proc_mount_invalid_argument_failure() {
|
|
let stderr = "bwrap: Can't mount proc on /newroot/proc: Invalid argument";
|
|
assert!(is_proc_mount_failure(stderr));
|
|
}
|
|
|
|
#[test]
|
|
fn detects_proc_mount_operation_not_permitted_failure() {
|
|
let stderr = "bwrap: Can't mount proc on /newroot/proc: Operation not permitted";
|
|
assert!(is_proc_mount_failure(stderr));
|
|
}
|
|
|
|
#[test]
|
|
fn detects_proc_mount_permission_denied_failure() {
|
|
let stderr = "bwrap: Can't mount proc on /newroot/proc: Permission denied";
|
|
assert!(is_proc_mount_failure(stderr));
|
|
}
|
|
|
|
#[test]
|
|
fn ignores_non_proc_mount_errors() {
|
|
let stderr = "bwrap: Can't bind mount /dev/null: Operation not permitted";
|
|
assert!(!is_proc_mount_failure(stderr));
|
|
}
|
|
|
|
#[test]
|
|
fn inserts_bwrap_argv0_before_command_separator() {
|
|
let sandbox_policy = SandboxPolicy::new_read_only_policy();
|
|
let argv = build_bwrap_argv(
|
|
vec!["/bin/true".to_string()],
|
|
&FileSystemSandboxPolicy::from(&sandbox_policy),
|
|
Path::new("/"),
|
|
BwrapOptions {
|
|
mount_proc: true,
|
|
network_mode: BwrapNetworkMode::FullAccess,
|
|
},
|
|
)
|
|
.args;
|
|
assert_eq!(
|
|
argv,
|
|
vec![
|
|
"bwrap".to_string(),
|
|
"--new-session".to_string(),
|
|
"--die-with-parent".to_string(),
|
|
"--ro-bind".to_string(),
|
|
"/".to_string(),
|
|
"/".to_string(),
|
|
"--dev".to_string(),
|
|
"/dev".to_string(),
|
|
"--unshare-user".to_string(),
|
|
"--unshare-pid".to_string(),
|
|
"--proc".to_string(),
|
|
"/proc".to_string(),
|
|
"--argv0".to_string(),
|
|
"codex-linux-sandbox".to_string(),
|
|
"--".to_string(),
|
|
"/bin/true".to_string(),
|
|
]
|
|
);
|
|
}
|
|
|
|
#[test]
|
|
fn inserts_unshare_net_when_network_isolation_requested() {
|
|
let sandbox_policy = SandboxPolicy::new_read_only_policy();
|
|
let argv = build_bwrap_argv(
|
|
vec!["/bin/true".to_string()],
|
|
&FileSystemSandboxPolicy::from(&sandbox_policy),
|
|
Path::new("/"),
|
|
BwrapOptions {
|
|
mount_proc: true,
|
|
network_mode: BwrapNetworkMode::Isolated,
|
|
},
|
|
)
|
|
.args;
|
|
assert!(argv.contains(&"--unshare-net".to_string()));
|
|
}
|
|
|
|
#[test]
|
|
fn inserts_unshare_net_when_proxy_only_network_mode_requested() {
|
|
let sandbox_policy = SandboxPolicy::new_read_only_policy();
|
|
let argv = build_bwrap_argv(
|
|
vec!["/bin/true".to_string()],
|
|
&FileSystemSandboxPolicy::from(&sandbox_policy),
|
|
Path::new("/"),
|
|
BwrapOptions {
|
|
mount_proc: true,
|
|
network_mode: BwrapNetworkMode::ProxyOnly,
|
|
},
|
|
)
|
|
.args;
|
|
assert!(argv.contains(&"--unshare-net".to_string()));
|
|
}
|
|
|
|
#[test]
|
|
fn proxy_only_mode_takes_precedence_over_full_network_policy() {
|
|
let mode = bwrap_network_mode(NetworkSandboxPolicy::Enabled, true);
|
|
assert_eq!(mode, BwrapNetworkMode::ProxyOnly);
|
|
}
|
|
|
|
#[test]
|
|
fn managed_proxy_preflight_argv_is_wrapped_for_full_access_policy() {
|
|
let mode = bwrap_network_mode(NetworkSandboxPolicy::Enabled, true);
|
|
let argv = build_preflight_bwrap_argv(
|
|
Path::new("/"),
|
|
&FileSystemSandboxPolicy::from(&SandboxPolicy::DangerFullAccess),
|
|
mode,
|
|
)
|
|
.args;
|
|
assert!(argv.iter().any(|arg| arg == "--"));
|
|
}
|
|
|
|
#[test]
|
|
fn managed_proxy_inner_command_includes_route_spec() {
|
|
let sandbox_policy = SandboxPolicy::new_read_only_policy();
|
|
let args = build_inner_seccomp_command(InnerSeccompCommandArgs {
|
|
sandbox_policy_cwd: Path::new("/tmp"),
|
|
sandbox_policy: &sandbox_policy,
|
|
file_system_sandbox_policy: &FileSystemSandboxPolicy::from(&sandbox_policy),
|
|
network_sandbox_policy: NetworkSandboxPolicy::Restricted,
|
|
use_bwrap_sandbox: true,
|
|
allow_network_for_proxy: true,
|
|
proxy_route_spec: Some("{\"routes\":[]}".to_string()),
|
|
command: vec!["/bin/true".to_string()],
|
|
});
|
|
|
|
assert!(args.iter().any(|arg| arg == "--proxy-route-spec"));
|
|
assert!(args.iter().any(|arg| arg == "{\"routes\":[]}"));
|
|
}
|
|
|
|
#[test]
|
|
fn inner_command_includes_split_policy_flags() {
|
|
let sandbox_policy = SandboxPolicy::new_read_only_policy();
|
|
let args = build_inner_seccomp_command(InnerSeccompCommandArgs {
|
|
sandbox_policy_cwd: Path::new("/tmp"),
|
|
sandbox_policy: &sandbox_policy,
|
|
file_system_sandbox_policy: &FileSystemSandboxPolicy::from(&sandbox_policy),
|
|
network_sandbox_policy: NetworkSandboxPolicy::Restricted,
|
|
use_bwrap_sandbox: true,
|
|
allow_network_for_proxy: false,
|
|
proxy_route_spec: None,
|
|
command: vec!["/bin/true".to_string()],
|
|
});
|
|
|
|
assert!(args.iter().any(|arg| arg == "--file-system-sandbox-policy"));
|
|
assert!(args.iter().any(|arg| arg == "--network-sandbox-policy"));
|
|
}
|
|
|
|
#[test]
|
|
fn non_managed_inner_command_omits_route_spec() {
|
|
let sandbox_policy = SandboxPolicy::new_read_only_policy();
|
|
let args = build_inner_seccomp_command(InnerSeccompCommandArgs {
|
|
sandbox_policy_cwd: Path::new("/tmp"),
|
|
sandbox_policy: &sandbox_policy,
|
|
file_system_sandbox_policy: &FileSystemSandboxPolicy::from(&sandbox_policy),
|
|
network_sandbox_policy: NetworkSandboxPolicy::Restricted,
|
|
use_bwrap_sandbox: true,
|
|
allow_network_for_proxy: false,
|
|
proxy_route_spec: None,
|
|
command: vec!["/bin/true".to_string()],
|
|
});
|
|
|
|
assert!(!args.iter().any(|arg| arg == "--proxy-route-spec"));
|
|
}
|
|
|
|
#[test]
|
|
fn managed_proxy_inner_command_requires_route_spec() {
|
|
let result = std::panic::catch_unwind(|| {
|
|
let sandbox_policy = SandboxPolicy::new_read_only_policy();
|
|
build_inner_seccomp_command(InnerSeccompCommandArgs {
|
|
sandbox_policy_cwd: Path::new("/tmp"),
|
|
sandbox_policy: &sandbox_policy,
|
|
file_system_sandbox_policy: &FileSystemSandboxPolicy::from(&sandbox_policy),
|
|
network_sandbox_policy: NetworkSandboxPolicy::Restricted,
|
|
use_bwrap_sandbox: true,
|
|
allow_network_for_proxy: true,
|
|
proxy_route_spec: None,
|
|
command: vec!["/bin/true".to_string()],
|
|
})
|
|
});
|
|
assert!(result.is_err());
|
|
}
|
|
|
|
#[test]
|
|
fn resolve_sandbox_policies_derives_split_policies_from_legacy_policy() {
|
|
let sandbox_policy = SandboxPolicy::new_read_only_policy();
|
|
|
|
let resolved =
|
|
resolve_sandbox_policies(Path::new("/tmp"), Some(sandbox_policy.clone()), None, None);
|
|
|
|
assert_eq!(resolved.sandbox_policy, sandbox_policy);
|
|
assert_eq!(
|
|
resolved.file_system_sandbox_policy,
|
|
FileSystemSandboxPolicy::from(&sandbox_policy)
|
|
);
|
|
assert_eq!(
|
|
resolved.network_sandbox_policy,
|
|
NetworkSandboxPolicy::from(&sandbox_policy)
|
|
);
|
|
}
|
|
|
|
#[test]
|
|
fn resolve_sandbox_policies_derives_legacy_policy_from_split_policies() {
|
|
let sandbox_policy = SandboxPolicy::new_read_only_policy();
|
|
let file_system_sandbox_policy = FileSystemSandboxPolicy::from(&sandbox_policy);
|
|
let network_sandbox_policy = NetworkSandboxPolicy::from(&sandbox_policy);
|
|
|
|
let resolved = resolve_sandbox_policies(
|
|
Path::new("/tmp"),
|
|
None,
|
|
Some(file_system_sandbox_policy.clone()),
|
|
Some(network_sandbox_policy),
|
|
);
|
|
|
|
assert_eq!(resolved.sandbox_policy, sandbox_policy);
|
|
assert_eq!(
|
|
resolved.file_system_sandbox_policy,
|
|
file_system_sandbox_policy
|
|
);
|
|
assert_eq!(resolved.network_sandbox_policy, network_sandbox_policy);
|
|
}
|
|
|
|
#[test]
|
|
fn resolve_sandbox_policies_rejects_partial_split_policies() {
|
|
let result = std::panic::catch_unwind(|| {
|
|
resolve_sandbox_policies(
|
|
Path::new("/tmp"),
|
|
Some(SandboxPolicy::new_read_only_policy()),
|
|
Some(FileSystemSandboxPolicy::default()),
|
|
None,
|
|
)
|
|
});
|
|
|
|
assert!(result.is_err());
|
|
}
|
|
|
|
#[test]
|
|
fn apply_seccomp_then_exec_without_bwrap_panics() {
|
|
let result = std::panic::catch_unwind(|| ensure_inner_stage_mode_is_valid(true, false));
|
|
assert!(result.is_err());
|
|
}
|
|
|
|
#[test]
|
|
fn valid_inner_stage_modes_do_not_panic() {
|
|
ensure_inner_stage_mode_is_valid(false, false);
|
|
ensure_inner_stage_mode_is_valid(false, true);
|
|
ensure_inner_stage_mode_is_valid(true, true);
|
|
}
|