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
18 changes: 14 additions & 4 deletions codex-rs/core/src/session/turn_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use super::*;
use codex_model_provider::SharedModelProvider;
use codex_model_provider::create_model_provider;
use codex_protocol::protocol::TurnEnvironmentSelection;
use codex_sandboxing::policy_transforms::merge_permission_profiles;

pub(super) fn image_generation_tool_auth_allowed(auth_manager: Option<&AuthManager>) -> bool {
matches!(
Expand Down Expand Up @@ -83,6 +84,13 @@ pub(crate) struct TurnContext {
pub(crate) turn_timing_state: Arc<TurnTimingState>,
}
impl TurnContext {
pub(crate) fn permission_profile(&self) -> PermissionProfile {
PermissionProfile::from_runtime_permissions(
&self.file_system_sandbox_policy,
self.network_sandbox_policy,
)
}

pub(crate) fn model_context_window(&self) -> Option<i64> {
let effective_context_window_percent = self.model_info.effective_context_window_percent;
self.model_info
Expand Down Expand Up @@ -220,17 +228,19 @@ impl TurnContext {
&self,
additional_permissions: Option<PermissionProfile>,
) -> FileSystemSandboxContext {
let base_permissions = self.permission_profile();
let permissions =
merge_permission_profiles(Some(&base_permissions), additional_permissions.as_ref())
.unwrap_or(base_permissions);
FileSystemSandboxContext {
sandbox_policy: self.sandbox_policy.get().clone(),
sandbox_policy_cwd: Some(self.cwd.clone()),
file_system_sandbox_policy: self.non_legacy_file_system_sandbox_policy(),
permissions,
cwd: Some(self.cwd.clone()),
windows_sandbox_level: self.windows_sandbox_level,
windows_sandbox_private_desktop: self
.config
.permissions
.windows_sandbox_private_desktop,
use_legacy_landlock: self.features.use_legacy_landlock(),
additional_permissions,
}
}

Expand Down
21 changes: 9 additions & 12 deletions codex-rs/core/src/tools/runtimes/apply_patch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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(),
})
}

Expand Down
68 changes: 12 additions & 56 deletions codex-rs/core/src/tools/runtimes/apply_patch_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand All @@ -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()
Expand Down
56 changes: 43 additions & 13 deletions codex-rs/exec-server/src/file_system.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
use async_trait::async_trait;
use codex_protocol::config_types::WindowsSandboxLevel;
use codex_protocol::models::PermissionProfile;
use codex_protocol::permissions::FileSystemSandboxKind;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::permissions::NetworkSandboxPolicy;
use codex_protocol::protocol::SandboxPolicy;
use codex_utils_absolute_path::AbsolutePathBuf;
use tokio::io;
Expand Down Expand Up @@ -41,37 +43,65 @@ pub struct ReadDirectoryEntry {
#[derive(Clone, Debug, Eq, PartialEq, serde::Serialize, serde::Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct FileSystemSandboxContext {
pub sandbox_policy: SandboxPolicy,
pub permissions: PermissionProfile,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub sandbox_policy_cwd: Option<AbsolutePathBuf>,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub file_system_sandbox_policy: Option<FileSystemSandboxPolicy>,
pub cwd: Option<AbsolutePathBuf>,
pub windows_sandbox_level: WindowsSandboxLevel,
#[serde(default)]
pub windows_sandbox_private_desktop: bool,
#[serde(default)]
pub use_legacy_landlock: bool,
pub additional_permissions: Option<PermissionProfile>,
}

impl FileSystemSandboxContext {
pub fn new(sandbox_policy: SandboxPolicy) -> Self {
if let Ok(cwd) = AbsolutePathBuf::current_dir() {
Self::from_legacy_sandbox_policy(sandbox_policy, cwd)
} else {
let permissions = PermissionProfile::from_runtime_permissions(
&FileSystemSandboxPolicy::from(&sandbox_policy),
NetworkSandboxPolicy::from(&sandbox_policy),
);
Self::from_permission_profile(permissions)
}
}

pub fn from_legacy_sandbox_policy(sandbox_policy: SandboxPolicy, cwd: AbsolutePathBuf) -> Self {
let permissions = PermissionProfile::from_runtime_permissions(
&FileSystemSandboxPolicy::from_legacy_sandbox_policy(&sandbox_policy, cwd.as_path()),
NetworkSandboxPolicy::from(&sandbox_policy),
Comment on lines +70 to +72
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve legacy workspace-write conversion in sandbox context

FileSystemSandboxContext::new now builds permissions via FileSystemSandboxPolicy::from(&sandbox_policy), which is the cwd-independent projection and does not apply the legacy from_legacy_sandbox_policy adjustments (e.g., workspace-root read-only carveouts like .git/.codex and other cwd-aware normalization). Before this change, those adjustments were applied later in fs_sandbox::run; now callers that construct contexts through new(SandboxPolicy::WorkspaceWrite { .. }) lose those protections and can write files that were previously blocked.

Useful? React with 👍 / 👎.

);
Self::from_permission_profile_with_cwd(permissions, cwd)
}

pub fn from_permission_profile(permissions: PermissionProfile) -> Self {
Self::from_permissions_and_cwd(permissions, /*cwd*/ None)
}

pub fn from_permission_profile_with_cwd(
permissions: PermissionProfile,
cwd: AbsolutePathBuf,
) -> Self {
Self::from_permissions_and_cwd(permissions, Some(cwd))
}

fn from_permissions_and_cwd(
permissions: PermissionProfile,
cwd: Option<AbsolutePathBuf>,
) -> Self {
Self {
sandbox_policy,
sandbox_policy_cwd: None,
file_system_sandbox_policy: None,
permissions,
cwd,
windows_sandbox_level: WindowsSandboxLevel::Disabled,
windows_sandbox_private_desktop: false,
use_legacy_landlock: false,
additional_permissions: None,
}
}

pub fn should_run_in_sandbox(&self) -> bool {
matches!(
self.sandbox_policy,
SandboxPolicy::ReadOnly { .. } | SandboxPolicy::WorkspaceWrite { .. }
)
let file_system_policy = self.permissions.file_system_sandbox_policy();
matches!(file_system_policy.kind, FileSystemSandboxKind::Restricted)
&& !file_system_policy.has_full_disk_write_access()
}
}

Expand Down
Loading
Loading