mirror of
https://github.com/openai/codex.git
synced 2026-06-01 19:02:59 +00:00
exec-server: carry filesystem sandbox profiles (#18276)
## Why The exec-server still needs platform sandbox inputs, but the migration should preserve the `PermissionProfile` that produced them. Keeping only the derived legacy sandbox map would keep `SandboxPolicy` as the effective abstraction and would make full-disk vs. restricted profiles harder to preserve as the permissions stack starts round-tripping profiles. `PermissionProfile` entries can also be cwd-sensitive (`:cwd`, `:project_roots`, relative globs), so the exec-server must carry the request sandbox cwd instead of resolving those entries against the long-lived exec-server process cwd. ## What changed `FileSystemSandboxContext` now carries `permissions: PermissionProfile` plus an optional `cwd`: - removed `sandboxPolicy`, `sandboxPolicyCwd`, `fileSystemSandboxPolicy`, and `additionalPermissions` - added `permissions` and `cwd` - kept the platform knobs `windowsSandboxLevel`, `windowsSandboxPrivateDesktop`, and `useLegacyLandlock` Core turn and apply-patch paths populate the context from the active runtime permissions and request cwd. Exec-server derives platform `SandboxPolicy`/`FileSystemSandboxPolicy` at the filesystem boundary, adds helper runtime reads there, and rejects cwd-dependent profiles that arrive without a cwd. The legacy `FileSystemSandboxContext::new(SandboxPolicy)` constructor now preserves the old workspace-write conversion semantics for compatibility tests/callers. ## Verification - `cargo test -p codex-exec-server` - `cargo test -p codex-exec-server sandbox_cwd -- --nocapture` - `cargo test -p codex-exec-server sandbox_context_new_preserves_legacy_workspace_write_read_only_subpaths -- --nocapture` - `cargo test -p codex-core --lib file_system_sandbox_context_uses_active_attempt -- --nocapture`
This commit is contained in:
@@ -24,7 +24,6 @@ use codex_protocol::error::SandboxErr;
|
||||
use codex_protocol::exec_output::ExecToolCallOutput;
|
||||
use codex_protocol::exec_output::StreamOutput;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::permissions::FileSystemSandboxPolicy;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::Event;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
@@ -34,6 +33,7 @@ use codex_protocol::protocol::FileChange;
|
||||
use codex_protocol::protocol::ReviewDecision;
|
||||
use codex_sandboxing::SandboxType;
|
||||
use codex_sandboxing::SandboxablePreference;
|
||||
use codex_sandboxing::policy_transforms::merge_permission_profiles;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use futures::future::BoxFuture;
|
||||
use std::path::PathBuf;
|
||||
@@ -77,22 +77,19 @@ impl ApplyPatchRuntime {
|
||||
return None;
|
||||
}
|
||||
|
||||
let legacy_file_system_sandbox_policy = FileSystemSandboxPolicy::from_legacy_sandbox_policy(
|
||||
attempt.policy,
|
||||
attempt.sandbox_cwd,
|
||||
let base_permissions = PermissionProfile::from_runtime_permissions(
|
||||
attempt.file_system_policy,
|
||||
attempt.network_policy,
|
||||
);
|
||||
let file_system_sandbox_policy = (attempt.file_system_policy
|
||||
!= &legacy_file_system_sandbox_policy)
|
||||
.then(|| attempt.file_system_policy.clone());
|
||||
|
||||
let permissions =
|
||||
merge_permission_profiles(Some(&base_permissions), req.additional_permissions.as_ref())
|
||||
.unwrap_or(base_permissions);
|
||||
Some(FileSystemSandboxContext {
|
||||
sandbox_policy: attempt.policy.clone(),
|
||||
sandbox_policy_cwd: Some(attempt.sandbox_cwd.clone()),
|
||||
file_system_sandbox_policy,
|
||||
permissions,
|
||||
cwd: Some(attempt.sandbox_cwd.clone()),
|
||||
windows_sandbox_level: attempt.windows_sandbox_level,
|
||||
windows_sandbox_private_desktop: attempt.windows_sandbox_private_desktop,
|
||||
use_legacy_landlock: attempt.use_legacy_landlock,
|
||||
additional_permissions: req.additional_permissions.clone(),
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -3,15 +3,13 @@ use crate::tools::sandboxing::SandboxAttempt;
|
||||
use codex_protocol::config_types::WindowsSandboxLevel;
|
||||
use codex_protocol::models::FileSystemPermissions;
|
||||
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::NetworkSandboxPolicy;
|
||||
use codex_protocol::protocol::GranularApprovalConfig;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use codex_sandboxing::SandboxManager;
|
||||
use codex_sandboxing::SandboxType;
|
||||
use codex_sandboxing::policy_transforms::merge_permission_profiles;
|
||||
use core_test_support::PathBufExt;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::collections::HashMap;
|
||||
@@ -135,12 +133,7 @@ fn file_system_sandbox_context_uses_active_attempt() {
|
||||
permissions_preapproved: false,
|
||||
};
|
||||
let sandbox_policy = SandboxPolicy::new_read_only_policy();
|
||||
let mut file_system_policy =
|
||||
FileSystemSandboxPolicy::from_legacy_sandbox_policy(&sandbox_policy, path.as_path());
|
||||
file_system_policy.entries.push(FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Path { path: path.clone() },
|
||||
access: FileSystemAccessMode::None,
|
||||
});
|
||||
let file_system_policy = FileSystemSandboxPolicy::from(&sandbox_policy);
|
||||
let manager = SandboxManager::new();
|
||||
let attempt = SandboxAttempt {
|
||||
sandbox: SandboxType::MacosSeatbelt,
|
||||
@@ -159,13 +152,17 @@ fn file_system_sandbox_context_uses_active_attempt() {
|
||||
let sandbox = ApplyPatchRuntime::file_system_sandbox_context_for_attempt(&req, &attempt)
|
||||
.expect("sandbox context");
|
||||
|
||||
assert_eq!(sandbox.sandbox_policy, sandbox_policy);
|
||||
assert_eq!(sandbox.sandbox_policy_cwd, Some(path.clone()));
|
||||
assert_eq!(
|
||||
sandbox.file_system_sandbox_policy,
|
||||
Some(file_system_policy.clone())
|
||||
let base_permissions = PermissionProfile::from_runtime_permissions(
|
||||
&file_system_policy,
|
||||
NetworkSandboxPolicy::Restricted,
|
||||
);
|
||||
assert_eq!(sandbox.additional_permissions, Some(additional_permissions));
|
||||
let Some(expected_permissions) =
|
||||
merge_permission_profiles(Some(&base_permissions), Some(&additional_permissions))
|
||||
else {
|
||||
panic!("merged permissions should not be empty");
|
||||
};
|
||||
assert_eq!(sandbox.permissions, expected_permissions);
|
||||
assert_eq!(sandbox.cwd, Some(path.clone()));
|
||||
assert_eq!(
|
||||
sandbox.windows_sandbox_level,
|
||||
WindowsSandboxLevel::RestrictedToken
|
||||
@@ -174,47 +171,6 @@ fn file_system_sandbox_context_uses_active_attempt() {
|
||||
assert_eq!(sandbox.use_legacy_landlock, true);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn file_system_sandbox_context_omits_legacy_equivalent_policy() {
|
||||
let path = std::env::temp_dir()
|
||||
.join("apply-patch-runtime-legacy-equivalent.txt")
|
||||
.abs();
|
||||
let req = ApplyPatchRequest {
|
||||
action: ApplyPatchAction::new_add_for_test(&path, "hello".to_string()),
|
||||
file_paths: vec![path.clone()],
|
||||
changes: HashMap::new(),
|
||||
exec_approval_requirement: ExecApprovalRequirement::Skip {
|
||||
bypass_sandbox: false,
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
additional_permissions: None,
|
||||
permissions_preapproved: false,
|
||||
};
|
||||
let sandbox_policy = SandboxPolicy::new_read_only_policy();
|
||||
let file_system_policy =
|
||||
FileSystemSandboxPolicy::from_legacy_sandbox_policy(&sandbox_policy, path.as_path());
|
||||
let manager = SandboxManager::new();
|
||||
let attempt = SandboxAttempt {
|
||||
sandbox: SandboxType::MacosSeatbelt,
|
||||
policy: &sandbox_policy,
|
||||
file_system_policy: &file_system_policy,
|
||||
network_policy: NetworkSandboxPolicy::Restricted,
|
||||
enforce_managed_network: false,
|
||||
manager: &manager,
|
||||
sandbox_cwd: &path,
|
||||
codex_linux_sandbox_exe: None,
|
||||
use_legacy_landlock: true,
|
||||
windows_sandbox_level: WindowsSandboxLevel::RestrictedToken,
|
||||
windows_sandbox_private_desktop: true,
|
||||
};
|
||||
|
||||
let sandbox = ApplyPatchRuntime::file_system_sandbox_context_for_attempt(&req, &attempt)
|
||||
.expect("sandbox context");
|
||||
|
||||
assert_eq!(sandbox.sandbox_policy_cwd, Some(path));
|
||||
assert_eq!(sandbox.file_system_sandbox_policy, None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn no_sandbox_attempt_has_no_file_system_context() {
|
||||
let path = std::env::temp_dir()
|
||||
|
||||
Reference in New Issue
Block a user