mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
core tests: submit turns with permission profiles (#20010)
## Summary
- Add `PermissionProfile`-based turn submission helpers to
`core_test_support`, while keeping the legacy `SandboxPolicy` helper for
tests that intentionally exercise legacy fallback behavior.
- Switch the default `TestCodex::submit_turn()` path to send a real
`PermissionProfile` plus the required legacy compatibility projection in
`Op::UserTurn`.
- Migrate straightforward app/search/shell/truncation tests from
`SandboxPolicy::{DangerFullAccess, ReadOnly}` to
`PermissionProfile::{Disabled, read_only}`.
- Add a TUI compatibility projection helper for legacy app-server fields
so non-legacy writable roots are preserved instead of being downgraded
to read-only.
- Fix remote start/resume/fork sandbox-mode projection to classify any
managed profile with writable roots as workspace-write, not only
profiles that can write `cwd`.
- Reduce `SandboxPolicy` references in `codex-rs/core/tests` from 47
files to 41 files without changing production behavior.
## Testing
- `cargo check -p codex-core --tests`
- `cargo test -p codex-tui
compatibility_profile_preserves_unbridgeable_write_roots`
- `cargo test -p codex-tui
sandbox_mode_preserves_non_cwd_write_roots_for_remote_sessions`
- `just fmt`
- `just fix -p core_test_support`
- `just fix -p codex-core`
This commit is contained in:
committed by
GitHub
Unverified
parent
2dbde94aa9
commit
891722849d
@@ -23,6 +23,8 @@ use codex_protocol::user_input::UserInput;
|
||||
use serde::Serialize;
|
||||
use serde_json::Value;
|
||||
|
||||
use crate::permission_compat::legacy_compatible_permission_profile;
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Serialize)]
|
||||
pub(crate) struct AppCommand(Op);
|
||||
|
||||
@@ -148,18 +150,12 @@ impl AppCommand {
|
||||
collaboration_mode: Option<CollaborationMode>,
|
||||
personality: Option<Personality>,
|
||||
) -> Self {
|
||||
let sandbox_policy = permission_profile
|
||||
let legacy_profile =
|
||||
legacy_compatible_permission_profile(&permission_profile, cwd.as_path());
|
||||
let sandbox_policy = legacy_profile
|
||||
.to_legacy_sandbox_policy(cwd.as_path())
|
||||
.unwrap_or_else(|err| {
|
||||
tracing::warn!(
|
||||
%err,
|
||||
"permission profile cannot be projected to legacy UserTurn sandbox; using read-only compatibility fallback"
|
||||
);
|
||||
PermissionProfile::read_only()
|
||||
.to_legacy_sandbox_policy(cwd.as_path())
|
||||
.unwrap_or_else(|err| {
|
||||
unreachable!("read-only permissions must be legacy-compatible: {err}")
|
||||
})
|
||||
unreachable!("legacy-compatible permissions must project to legacy policy: {err}")
|
||||
});
|
||||
Self(Op::UserTurn {
|
||||
items,
|
||||
|
||||
@@ -3,6 +3,7 @@ use crate::bottom_pane::FeedbackAudience;
|
||||
use crate::legacy_core::append_message_history_entry;
|
||||
use crate::legacy_core::config::Config;
|
||||
use crate::legacy_core::message_history_metadata;
|
||||
use crate::permission_compat::legacy_compatible_permission_profile;
|
||||
use crate::status::StatusAccountDisplay;
|
||||
use crate::status::plan_type_display_name;
|
||||
use codex_app_server_client::AppServerClient;
|
||||
@@ -542,22 +543,15 @@ impl AppServerSession {
|
||||
) -> Result<TurnStartResponse> {
|
||||
let request_id = self.next_request_id();
|
||||
let sandbox_policy = if matches!(self.thread_params_mode(), ThreadParamsMode::Remote) {
|
||||
let policy =
|
||||
permission_profile
|
||||
.to_legacy_sandbox_policy(cwd.as_path())
|
||||
.unwrap_or_else(|err| {
|
||||
tracing::warn!(
|
||||
%err,
|
||||
"permission profile cannot be projected for remote turn/start; using read-only compatibility fallback"
|
||||
);
|
||||
PermissionProfile::read_only()
|
||||
.to_legacy_sandbox_policy(cwd.as_path())
|
||||
.unwrap_or_else(|err| {
|
||||
unreachable!(
|
||||
"read-only permissions must be legacy-compatible: {err}"
|
||||
)
|
||||
})
|
||||
});
|
||||
let legacy_profile =
|
||||
legacy_compatible_permission_profile(&permission_profile, cwd.as_path());
|
||||
let policy = legacy_profile
|
||||
.to_legacy_sandbox_policy(cwd.as_path())
|
||||
.unwrap_or_else(|err| {
|
||||
unreachable!(
|
||||
"legacy-compatible permissions must project to legacy policy: {err}"
|
||||
)
|
||||
});
|
||||
Some(policy.into())
|
||||
} else {
|
||||
None
|
||||
@@ -1514,6 +1508,12 @@ mod tests {
|
||||
use codex_app_server_protocol::ThreadStatus;
|
||||
use codex_app_server_protocol::Turn;
|
||||
use codex_app_server_protocol::TurnStatus;
|
||||
use codex_protocol::models::ManagedFileSystemPermissions;
|
||||
use codex_protocol::permissions::FileSystemAccessMode;
|
||||
use codex_protocol::permissions::FileSystemPath;
|
||||
use codex_protocol::permissions::FileSystemSandboxEntry;
|
||||
use codex_protocol::permissions::FileSystemSpecialPath;
|
||||
use codex_protocol::permissions::NetworkSandboxPolicy;
|
||||
use codex_utils_absolute_path::test_support::PathBufExt;
|
||||
use codex_utils_absolute_path::test_support::test_path_buf;
|
||||
use pretty_assertions::assert_eq;
|
||||
@@ -1606,6 +1606,65 @@ mod tests {
|
||||
assert_eq!(fork.permission_profile, None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sandbox_mode_does_not_project_non_cwd_write_roots_for_remote_sessions() {
|
||||
let cwd = test_path_buf("/workspace/project").abs();
|
||||
let extra_root = test_path_buf("/workspace/cache").abs();
|
||||
let permission_profile = PermissionProfile::Managed {
|
||||
file_system: ManagedFileSystemPermissions::Restricted {
|
||||
entries: vec![
|
||||
FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Special {
|
||||
value: FileSystemSpecialPath::Root,
|
||||
},
|
||||
access: FileSystemAccessMode::Read,
|
||||
},
|
||||
FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Path { path: extra_root },
|
||||
access: FileSystemAccessMode::Write,
|
||||
},
|
||||
],
|
||||
glob_scan_max_depth: None,
|
||||
},
|
||||
network: NetworkSandboxPolicy::Restricted,
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
sandbox_mode_from_permission_profile(&permission_profile, cwd.as_path()),
|
||||
Some(codex_app_server_protocol::SandboxMode::ReadOnly)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sandbox_mode_projects_cwd_write_for_remote_sessions() {
|
||||
let cwd = test_path_buf("/workspace/project").abs();
|
||||
let permission_profile = PermissionProfile::Managed {
|
||||
file_system: ManagedFileSystemPermissions::Restricted {
|
||||
entries: vec![
|
||||
FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Special {
|
||||
value: FileSystemSpecialPath::Root,
|
||||
},
|
||||
access: FileSystemAccessMode::Read,
|
||||
},
|
||||
FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Special {
|
||||
value: FileSystemSpecialPath::ProjectRoots { subpath: None },
|
||||
},
|
||||
access: FileSystemAccessMode::Write,
|
||||
},
|
||||
],
|
||||
glob_scan_max_depth: None,
|
||||
},
|
||||
network: NetworkSandboxPolicy::Restricted,
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
sandbox_mode_from_permission_profile(&permission_profile, cwd.as_path()),
|
||||
Some(codex_app_server_protocol::SandboxMode::WorkspaceWrite)
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn thread_lifecycle_params_forward_explicit_remote_cwd_override_for_remote_sessions() {
|
||||
let temp_dir = tempfile::tempdir().expect("tempdir");
|
||||
|
||||
@@ -150,6 +150,7 @@ mod npm_registry;
|
||||
pub(crate) mod onboarding;
|
||||
mod oss_selection;
|
||||
mod pager_overlay;
|
||||
mod permission_compat;
|
||||
pub(crate) mod public_widgets;
|
||||
mod render;
|
||||
mod resize_reflow_cap;
|
||||
|
||||
@@ -0,0 +1,105 @@
|
||||
//! Compatibility projections from the canonical permission profile model into
|
||||
//! legacy shapes still required by older or remote app-server APIs.
|
||||
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::permissions::FileSystemSandboxPolicy;
|
||||
use codex_protocol::permissions::NetworkSandboxPolicy;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use std::path::Path;
|
||||
|
||||
pub(crate) fn legacy_compatible_permission_profile(
|
||||
permission_profile: &PermissionProfile,
|
||||
cwd: &Path,
|
||||
) -> PermissionProfile {
|
||||
if permission_profile.to_legacy_sandbox_policy(cwd).is_ok() {
|
||||
return permission_profile.clone();
|
||||
}
|
||||
|
||||
let file_system_policy = permission_profile.file_system_sandbox_policy();
|
||||
compatibility_workspace_write_profile(
|
||||
&file_system_policy,
|
||||
permission_profile.network_sandbox_policy(),
|
||||
cwd,
|
||||
)
|
||||
}
|
||||
|
||||
fn compatibility_workspace_write_profile(
|
||||
file_system_policy: &FileSystemSandboxPolicy,
|
||||
network_policy: NetworkSandboxPolicy,
|
||||
cwd: &Path,
|
||||
) -> PermissionProfile {
|
||||
let cwd_abs = AbsolutePathBuf::from_absolute_path(cwd).ok();
|
||||
let writable_roots = file_system_policy
|
||||
.get_writable_roots_with_cwd(cwd)
|
||||
.into_iter()
|
||||
.map(|root| root.root)
|
||||
.filter(|root| cwd_abs.as_ref() != Some(root))
|
||||
.collect::<Vec<_>>();
|
||||
let tmpdir_writable = std::env::var_os("TMPDIR")
|
||||
.filter(|tmpdir| !tmpdir.is_empty())
|
||||
.and_then(|tmpdir| {
|
||||
AbsolutePathBuf::from_absolute_path(std::path::PathBuf::from(tmpdir)).ok()
|
||||
})
|
||||
.is_some_and(|tmpdir| file_system_policy.can_write_path_with_cwd(tmpdir.as_path(), cwd));
|
||||
let slash_tmp = Path::new("/tmp");
|
||||
let slash_tmp_writable = slash_tmp.is_absolute()
|
||||
&& slash_tmp.is_dir()
|
||||
&& file_system_policy.can_write_path_with_cwd(slash_tmp, cwd);
|
||||
|
||||
PermissionProfile::workspace_write_with(
|
||||
&writable_roots,
|
||||
network_policy,
|
||||
!tmpdir_writable,
|
||||
!slash_tmp_writable,
|
||||
)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use codex_protocol::models::ManagedFileSystemPermissions;
|
||||
use codex_protocol::permissions::FileSystemAccessMode;
|
||||
use codex_protocol::permissions::FileSystemPath;
|
||||
use codex_protocol::permissions::FileSystemSandboxEntry;
|
||||
use codex_protocol::permissions::FileSystemSpecialPath;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[test]
|
||||
fn compatibility_profile_preserves_unbridgeable_write_roots() {
|
||||
let cwd = AbsolutePathBuf::try_from("/workspace/project").expect("absolute cwd");
|
||||
let extra_root = AbsolutePathBuf::try_from("/workspace/extra").expect("absolute root");
|
||||
let permission_profile = PermissionProfile::Managed {
|
||||
file_system: ManagedFileSystemPermissions::Restricted {
|
||||
entries: vec![
|
||||
FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Special {
|
||||
value: FileSystemSpecialPath::Root,
|
||||
},
|
||||
access: FileSystemAccessMode::Read,
|
||||
},
|
||||
FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Path {
|
||||
path: extra_root.clone(),
|
||||
},
|
||||
access: FileSystemAccessMode::Write,
|
||||
},
|
||||
],
|
||||
glob_scan_max_depth: None,
|
||||
},
|
||||
network: NetworkSandboxPolicy::Restricted,
|
||||
};
|
||||
|
||||
let compatibility_profile =
|
||||
legacy_compatible_permission_profile(&permission_profile, cwd.as_path());
|
||||
let policy = compatibility_profile
|
||||
.to_legacy_sandbox_policy(cwd.as_path())
|
||||
.expect("compatibility profile should project to legacy policy");
|
||||
let roots = policy
|
||||
.get_writable_roots_with_cwd(cwd.as_path())
|
||||
.into_iter()
|
||||
.map(|root| root.root)
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
assert_eq!(roots, vec![extra_root, cwd]);
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user