mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
test: reduce core sandbox policy test setup (#23036)
## Why `SandboxPolicy` is a legacy compatibility shape, but several core tests still used it for ordinary turn setup even when the runtime path now carries `PermissionProfile`. With the first cleanup PR merged, this follow-up trims more core test scaffolding so remaining `SandboxPolicy` matches are easier to classify as production compatibility, legacy-boundary coverage, or explicit conversion tests. ## What Changed - Updated apply-patch handler and runtime tests to pass `PermissionProfile` directly. - Changed sandboxing test helpers to build permission profiles without first creating `SandboxPolicy` values. - Converted request-permissions integration turns to pass `PermissionProfile` through the test helper, leaving legacy sandbox projection at the `Op::UserTurn` boundary. - Converted unified exec integration helpers and direct turn submissions to use `PermissionProfile` values instead of `SandboxPolicy` setup. - Removed now-unused `SandboxPolicy` imports from the touched core tests. ## Test Plan - `just fmt` - `cargo test -p codex-core --lib tools::sandboxing::tests` - `cargo test -p codex-core --lib tools::runtimes::apply_patch::tests` - `cargo test -p codex-core --lib tools::handlers::apply_patch::tests` - `cargo test -p codex-core --lib unified_exec::process_manager::tests` - `cargo test -p codex-core --test all request_permissions::` - `cargo test -p codex-core --test all unified_exec::` - `just fix -p codex-core`
This commit is contained in:
committed by
GitHub
Unverified
parent
545ede569c
commit
0a83353ca3
@@ -3,7 +3,6 @@ use codex_apply_patch::MaybeApplyPatchVerified;
|
||||
use codex_exec_server::LOCAL_FS;
|
||||
use codex_protocol::permissions::FileSystemSandboxPolicy;
|
||||
use codex_protocol::protocol::FileChange;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use core_test_support::PathBufExt;
|
||||
use core_test_support::PathExt;
|
||||
use pretty_assertions::assert_eq;
|
||||
@@ -251,12 +250,11 @@ fn write_permissions_for_paths_skip_dirs_already_writable_under_workspace_root()
|
||||
std::fs::create_dir_all(&nested).expect("create nested dir");
|
||||
let file_path = AbsolutePathBuf::try_from(nested.join("file.txt"))
|
||||
.expect("nested file path should be absolute");
|
||||
let sandbox_policy = FileSystemSandboxPolicy::from(&SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: vec![],
|
||||
network_access: false,
|
||||
exclude_tmpdir_env_var: true,
|
||||
exclude_slash_tmp: false,
|
||||
});
|
||||
let sandbox_policy = FileSystemSandboxPolicy::workspace_write(
|
||||
&[],
|
||||
/*exclude_tmpdir_env_var*/ true,
|
||||
/*exclude_slash_tmp*/ false,
|
||||
);
|
||||
|
||||
let permissions = write_permissions_for_paths(&[file_path], &sandbox_policy, &cwd);
|
||||
|
||||
@@ -273,12 +271,11 @@ fn write_permissions_for_paths_keep_dirs_outside_workspace_root() {
|
||||
let file_path = AbsolutePathBuf::try_from(outside.join("file.txt"))
|
||||
.expect("outside file path should be absolute");
|
||||
let cwd_abs = cwd.abs();
|
||||
let sandbox_policy = FileSystemSandboxPolicy::from(&SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: vec![],
|
||||
network_access: false,
|
||||
exclude_tmpdir_env_var: true,
|
||||
exclude_slash_tmp: true,
|
||||
});
|
||||
let sandbox_policy = FileSystemSandboxPolicy::workspace_write(
|
||||
&[],
|
||||
/*exclude_tmpdir_env_var*/ true,
|
||||
/*exclude_slash_tmp*/ true,
|
||||
);
|
||||
|
||||
let permissions = write_permissions_for_paths(&[file_path], &sandbox_policy, &cwd_abs);
|
||||
let expected_outside =
|
||||
|
||||
@@ -7,7 +7,6 @@ use codex_protocol::models::PermissionProfile;
|
||||
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::effective_file_system_sandbox_policy;
|
||||
@@ -201,8 +200,7 @@ async fn file_system_sandbox_context_uses_active_attempt() {
|
||||
additional_permissions: Some(additional_permissions.clone()),
|
||||
permissions_preapproved: false,
|
||||
};
|
||||
let sandbox_policy = SandboxPolicy::new_read_only_policy();
|
||||
let file_system_policy = FileSystemSandboxPolicy::from(&sandbox_policy);
|
||||
let file_system_policy = FileSystemSandboxPolicy::default();
|
||||
let permissions = PermissionProfile::from_runtime_permissions(
|
||||
&file_system_policy,
|
||||
NetworkSandboxPolicy::Restricted,
|
||||
|
||||
@@ -19,8 +19,6 @@ use codex_protocol::permissions::FileSystemSandboxKind;
|
||||
use codex_protocol::permissions::FileSystemSandboxPolicy;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::ReviewDecision;
|
||||
#[cfg(test)]
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use codex_sandboxing::SandboxCommand;
|
||||
use codex_sandboxing::SandboxManager;
|
||||
use codex_sandboxing::SandboxTransformError;
|
||||
|
||||
@@ -5,7 +5,6 @@ use codex_protocol::permissions::FileSystemAccessMode;
|
||||
use codex_protocol::permissions::FileSystemPath;
|
||||
use codex_protocol::permissions::FileSystemSandboxEntry;
|
||||
use codex_protocol::protocol::GranularApprovalConfig;
|
||||
use codex_protocol::protocol::NetworkAccess;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
|
||||
@@ -39,13 +38,10 @@ fn bash_permission_request_payload_includes_description_when_present() {
|
||||
|
||||
#[test]
|
||||
fn external_sandbox_skips_exec_approval_on_request() {
|
||||
let sandbox_policy = SandboxPolicy::ExternalSandbox {
|
||||
network_access: NetworkAccess::Restricted,
|
||||
};
|
||||
assert_eq!(
|
||||
default_exec_approval_requirement(
|
||||
AskForApproval::OnRequest,
|
||||
&FileSystemSandboxPolicy::from(&sandbox_policy),
|
||||
&FileSystemSandboxPolicy::external_sandbox(),
|
||||
),
|
||||
ExecApprovalRequirement::Skip {
|
||||
bypass_sandbox: false,
|
||||
@@ -56,11 +52,10 @@ fn external_sandbox_skips_exec_approval_on_request() {
|
||||
|
||||
#[test]
|
||||
fn restricted_sandbox_requires_exec_approval_on_request() {
|
||||
let sandbox_policy = SandboxPolicy::new_read_only_policy();
|
||||
assert_eq!(
|
||||
default_exec_approval_requirement(
|
||||
AskForApproval::OnRequest,
|
||||
&FileSystemSandboxPolicy::from(&sandbox_policy)
|
||||
&FileSystemSandboxPolicy::default()
|
||||
),
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
reason: None,
|
||||
@@ -79,9 +74,8 @@ fn default_exec_approval_requirement_rejects_sandbox_prompt_when_granular_disabl
|
||||
mcp_elicitations: true,
|
||||
});
|
||||
|
||||
let sandbox_policy = SandboxPolicy::new_read_only_policy();
|
||||
let requirement =
|
||||
default_exec_approval_requirement(policy, &FileSystemSandboxPolicy::from(&sandbox_policy));
|
||||
default_exec_approval_requirement(policy, &FileSystemSandboxPolicy::default());
|
||||
|
||||
assert_eq!(
|
||||
requirement,
|
||||
@@ -101,9 +95,8 @@ fn default_exec_approval_requirement_keeps_prompt_when_granular_allows_sandbox_a
|
||||
mcp_elicitations: false,
|
||||
});
|
||||
|
||||
let sandbox_policy = SandboxPolicy::new_read_only_policy();
|
||||
let requirement =
|
||||
default_exec_approval_requirement(policy, &FileSystemSandboxPolicy::from(&sandbox_policy));
|
||||
default_exec_approval_requirement(policy, &FileSystemSandboxPolicy::default());
|
||||
|
||||
assert_eq!(
|
||||
requirement,
|
||||
|
||||
@@ -71,16 +71,10 @@ fn exec_server_params_use_env_policy_overlay_contract() {
|
||||
.expect("current dir")
|
||||
.try_into()
|
||||
.expect("absolute path");
|
||||
let sandbox_policy = codex_protocol::protocol::SandboxPolicy::DangerFullAccess;
|
||||
let file_system_sandbox_policy =
|
||||
codex_protocol::permissions::FileSystemSandboxPolicy::from(&sandbox_policy);
|
||||
codex_protocol::permissions::FileSystemSandboxPolicy::unrestricted();
|
||||
let network_sandbox_policy = codex_protocol::permissions::NetworkSandboxPolicy::Restricted;
|
||||
let permission_profile =
|
||||
codex_protocol::models::PermissionProfile::from_runtime_permissions_with_enforcement(
|
||||
codex_protocol::models::SandboxEnforcement::from_legacy_sandbox_policy(&sandbox_policy),
|
||||
&file_system_sandbox_policy,
|
||||
network_sandbox_policy,
|
||||
);
|
||||
let permission_profile = codex_protocol::models::PermissionProfile::Disabled;
|
||||
let request = ExecRequest {
|
||||
command: vec!["bash".to_string(), "-lc".to_string(), "true".to_string()],
|
||||
cwd: cwd.clone(),
|
||||
|
||||
Reference in New Issue
Block a user