mirror of
https://github.com/openai/codex.git
synced 2026-05-05 05:42:33 +03:00
fix: move inline codex-rs/core unit tests into sibling files (#14444)
## Why PR #13783 moved the `codex.rs` unit tests into `codex_tests.rs`. This applies the same extraction pattern across the rest of `codex-rs/core` so the production modules stay focused on runtime code instead of large inline test blocks. Keeping the tests in sibling files also makes follow-up edits easier to review because product changes no longer have to share a file with hundreds or thousands of lines of test scaffolding. ## What changed - replaced each inline `mod tests { ... }` in `codex-rs/core/src/**` with a path-based module declaration - moved each extracted unit test module into a sibling `*_tests.rs` file, using `mod_tests.rs` for `mod.rs` modules - preserved the existing `cfg(...)` guards and module-local structure so the refactor remains structural rather than behavioral ## Testing - `cargo test -p codex-core --lib` (`1653 passed; 0 failed; 5 ignored`) - `just fix -p codex-core` - `cargo fmt --check` - `cargo shear`
This commit is contained in:
Michael Bolin
GitHub
@@ -180,259 +180,5 @@ fn is_write_patch_constrained_to_writable_paths(
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use codex_protocol::protocol::FileSystemAccessMode;
|
||||
use codex_protocol::protocol::FileSystemPath;
|
||||
use codex_protocol::protocol::FileSystemSandboxEntry;
|
||||
use codex_protocol::protocol::FileSystemSpecialPath;
|
||||
use codex_protocol::protocol::RejectConfig;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tempfile::TempDir;
|
||||
|
||||
#[test]
|
||||
fn test_writable_roots_constraint() {
|
||||
// Use a temporary directory as our workspace to avoid touching
|
||||
// the real current working directory.
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let cwd = tmp.path().to_path_buf();
|
||||
let parent = cwd.parent().unwrap().to_path_buf();
|
||||
|
||||
// Helper to build a single‑entry patch that adds a file at `p`.
|
||||
let make_add_change = |p: PathBuf| ApplyPatchAction::new_add_for_test(&p, "".to_string());
|
||||
|
||||
let add_inside = make_add_change(cwd.join("inner.txt"));
|
||||
let add_outside = make_add_change(parent.join("outside.txt"));
|
||||
|
||||
// Policy limited to the workspace only; exclude system temp roots so
|
||||
// only `cwd` is writable by default.
|
||||
let policy_workspace_only = SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: vec![],
|
||||
read_only_access: Default::default(),
|
||||
network_access: false,
|
||||
exclude_tmpdir_env_var: true,
|
||||
exclude_slash_tmp: true,
|
||||
};
|
||||
|
||||
assert!(is_write_patch_constrained_to_writable_paths(
|
||||
&add_inside,
|
||||
&FileSystemSandboxPolicy::from(&policy_workspace_only),
|
||||
&cwd,
|
||||
));
|
||||
|
||||
assert!(!is_write_patch_constrained_to_writable_paths(
|
||||
&add_outside,
|
||||
&FileSystemSandboxPolicy::from(&policy_workspace_only),
|
||||
&cwd,
|
||||
));
|
||||
|
||||
// With the parent dir explicitly added as a writable root, the
|
||||
// outside write should be permitted.
|
||||
let policy_with_parent = SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: vec![AbsolutePathBuf::try_from(parent).unwrap()],
|
||||
read_only_access: Default::default(),
|
||||
network_access: false,
|
||||
exclude_tmpdir_env_var: true,
|
||||
exclude_slash_tmp: true,
|
||||
};
|
||||
assert!(is_write_patch_constrained_to_writable_paths(
|
||||
&add_outside,
|
||||
&FileSystemSandboxPolicy::from(&policy_with_parent),
|
||||
&cwd,
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn external_sandbox_auto_approves_in_on_request() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let cwd = tmp.path().to_path_buf();
|
||||
let add_inside = ApplyPatchAction::new_add_for_test(&cwd.join("inner.txt"), "".to_string());
|
||||
|
||||
let policy = SandboxPolicy::ExternalSandbox {
|
||||
network_access: codex_protocol::protocol::NetworkAccess::Enabled,
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
assess_patch_safety(
|
||||
&add_inside,
|
||||
AskForApproval::OnRequest,
|
||||
&policy,
|
||||
&FileSystemSandboxPolicy::from(&policy),
|
||||
&cwd,
|
||||
WindowsSandboxLevel::Disabled
|
||||
),
|
||||
SafetyCheck::AutoApprove {
|
||||
sandbox_type: SandboxType::None,
|
||||
user_explicitly_approved: false,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn reject_with_all_flags_false_matches_on_request_for_out_of_root_patch() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let cwd = tmp.path().to_path_buf();
|
||||
let parent = cwd.parent().unwrap().to_path_buf();
|
||||
let add_outside =
|
||||
ApplyPatchAction::new_add_for_test(&parent.join("outside.txt"), "".to_string());
|
||||
let policy_workspace_only = SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: vec![],
|
||||
read_only_access: Default::default(),
|
||||
network_access: false,
|
||||
exclude_tmpdir_env_var: true,
|
||||
exclude_slash_tmp: true,
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
assess_patch_safety(
|
||||
&add_outside,
|
||||
AskForApproval::OnRequest,
|
||||
&policy_workspace_only,
|
||||
&FileSystemSandboxPolicy::from(&policy_workspace_only),
|
||||
&cwd,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
),
|
||||
SafetyCheck::AskUser,
|
||||
);
|
||||
assert_eq!(
|
||||
assess_patch_safety(
|
||||
&add_outside,
|
||||
AskForApproval::Reject(RejectConfig {
|
||||
sandbox_approval: false,
|
||||
rules: false,
|
||||
skill_approval: false,
|
||||
request_permissions: false,
|
||||
mcp_elicitations: false,
|
||||
}),
|
||||
&policy_workspace_only,
|
||||
&FileSystemSandboxPolicy::from(&policy_workspace_only),
|
||||
&cwd,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
),
|
||||
SafetyCheck::AskUser,
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn reject_sandbox_approval_rejects_out_of_root_patch() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let cwd = tmp.path().to_path_buf();
|
||||
let parent = cwd.parent().unwrap().to_path_buf();
|
||||
let add_outside =
|
||||
ApplyPatchAction::new_add_for_test(&parent.join("outside.txt"), "".to_string());
|
||||
let policy_workspace_only = SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: vec![],
|
||||
read_only_access: Default::default(),
|
||||
network_access: false,
|
||||
exclude_tmpdir_env_var: true,
|
||||
exclude_slash_tmp: true,
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
assess_patch_safety(
|
||||
&add_outside,
|
||||
AskForApproval::Reject(RejectConfig {
|
||||
sandbox_approval: true,
|
||||
rules: false,
|
||||
skill_approval: false,
|
||||
request_permissions: false,
|
||||
mcp_elicitations: false,
|
||||
}),
|
||||
&policy_workspace_only,
|
||||
&FileSystemSandboxPolicy::from(&policy_workspace_only),
|
||||
&cwd,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
),
|
||||
SafetyCheck::Reject {
|
||||
reason: "writing outside of the project; rejected by user approval settings"
|
||||
.to_string(),
|
||||
},
|
||||
);
|
||||
}
|
||||
#[test]
|
||||
fn explicit_unreadable_paths_prevent_auto_approval_for_external_sandbox() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let cwd = tmp.path().to_path_buf();
|
||||
let blocked_path = cwd.join("blocked.txt");
|
||||
let blocked_absolute = AbsolutePathBuf::from_absolute_path(blocked_path.clone()).unwrap();
|
||||
let action = ApplyPatchAction::new_add_for_test(&blocked_path, "".to_string());
|
||||
let sandbox_policy = SandboxPolicy::ExternalSandbox {
|
||||
network_access: codex_protocol::protocol::NetworkAccess::Restricted,
|
||||
};
|
||||
let file_system_sandbox_policy = FileSystemSandboxPolicy::restricted(vec![
|
||||
FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Special {
|
||||
value: FileSystemSpecialPath::Root,
|
||||
},
|
||||
access: FileSystemAccessMode::Write,
|
||||
},
|
||||
FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Path {
|
||||
path: blocked_absolute,
|
||||
},
|
||||
access: FileSystemAccessMode::None,
|
||||
},
|
||||
]);
|
||||
|
||||
assert!(!is_write_patch_constrained_to_writable_paths(
|
||||
&action,
|
||||
&file_system_sandbox_policy,
|
||||
&cwd,
|
||||
));
|
||||
assert_eq!(
|
||||
assess_patch_safety(
|
||||
&action,
|
||||
AskForApproval::OnRequest,
|
||||
&sandbox_policy,
|
||||
&file_system_sandbox_policy,
|
||||
&cwd,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
),
|
||||
SafetyCheck::AskUser,
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn explicit_read_only_subpaths_prevent_auto_approval_for_external_sandbox() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let cwd = tmp.path().to_path_buf();
|
||||
let blocked_path = cwd.join("docs").join("blocked.txt");
|
||||
let docs_absolute = AbsolutePathBuf::resolve_path_against_base("docs", &cwd).unwrap();
|
||||
let action = ApplyPatchAction::new_add_for_test(&blocked_path, "".to_string());
|
||||
let sandbox_policy = SandboxPolicy::ExternalSandbox {
|
||||
network_access: codex_protocol::protocol::NetworkAccess::Restricted,
|
||||
};
|
||||
let file_system_sandbox_policy = FileSystemSandboxPolicy::restricted(vec![
|
||||
FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Special {
|
||||
value: FileSystemSpecialPath::CurrentWorkingDirectory,
|
||||
},
|
||||
access: FileSystemAccessMode::Write,
|
||||
},
|
||||
FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Path {
|
||||
path: docs_absolute,
|
||||
},
|
||||
access: FileSystemAccessMode::Read,
|
||||
},
|
||||
]);
|
||||
|
||||
assert!(!is_write_patch_constrained_to_writable_paths(
|
||||
&action,
|
||||
&file_system_sandbox_policy,
|
||||
&cwd,
|
||||
));
|
||||
assert_eq!(
|
||||
assess_patch_safety(
|
||||
&action,
|
||||
AskForApproval::OnRequest,
|
||||
&sandbox_policy,
|
||||
&file_system_sandbox_policy,
|
||||
&cwd,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
),
|
||||
SafetyCheck::AskUser,
|
||||
);
|
||||
}
|
||||
}
|
||||
#[path = "safety_tests.rs"]
|
||||
mod tests;
|
||||
|
||||
Reference in New Issue
Block a user