mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
permissions: canonicalize workspace_roots and danger-full-access names (#22624)
## Why This is a small precursor to the larger permissions-migration work. Both the comparison stack in [#22401](https://github.com/openai/codex/pull/22401) / [#22402](https://github.com/openai/codex/pull/22402) and the alternate stack in [#22610](https://github.com/openai/codex/pull/22610) / [#22611](https://github.com/openai/codex/pull/22611) / [#22612](https://github.com/openai/codex/pull/22612) are easier to review if the terminology is already settled underneath them. Because `:project_roots` and `:danger-no-sandbox` have not shipped as stable user-facing surface area, carrying them forward as aliases would just add more migration logic to the later stacks. This PR removes that ambiguity now so the follow-on work can rely on one spelling for each built-in concept. ## What Changed - renamed the config-facing special filesystem key from `:project_roots` to `:workspace_roots` - dropped unpublished `:project_roots` parsing support in `core/src/config/permissions.rs`, so new config only recognizes `:workspace_roots` - renamed the built-in full-access permission profile id from `:danger-no-sandbox` to `:danger-full-access` - dropped unpublished `:danger-no-sandbox` support entirely, including the old active-profile canonicalization path, and added explicit rejection coverage for the legacy id - introduced shared built-in permission-profile id constants in `codex-rs/protocol/src/models.rs` - updated `core`, `app-server`, and `tui` call sites that special-case built-in profiles to use the shared constants and canonical ids - updated tests and the Linux sandbox README to use `:workspace_roots` / `:danger-full-access` ## Verification I focused verification on the three places this rename can regress: config parsing, active-profile identity surfaced back out of `core`, and user/server call sites that special-case built-in profiles. Targeted checks: - `config::tests::default_permissions_can_select_builtin_profile_without_permissions_table` - `config::tests::default_permissions_read_only_applies_additional_writable_roots_as_modifications` - `config::tests::default_permissions_can_select_builtin_full_access_profile` - `config::tests::legacy_danger_no_sandbox_is_rejected` - `workspace_root` filtered `codex-core` tests - `request_processors::thread_processor::thread_processor_tests::thread_processor_behavior_tests::requested_permissions_trust_project_uses_permission_profile_intent` - `suite::v2::turn_start::turn_start_rejects_invalid_permission_selection_before_starting_turn` - `status::tests::status_snapshot_shows_auto_review_permissions` - `status::tests::status_permissions_full_disk_managed_with_network_is_danger_full_access` - `app_server_session::tests::embedded_turn_permissions_use_active_profile_selection`
This commit is contained in:
@@ -1592,6 +1592,8 @@ mod tests {
|
||||
use codex_protocol::config_types::ServiceTier;
|
||||
use codex_protocol::config_types::Verbosity;
|
||||
use codex_protocol::config_types::WebSearchMode;
|
||||
use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_READ_ONLY;
|
||||
use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_WORKSPACE;
|
||||
use codex_protocol::openai_models::ReasoningEffort;
|
||||
use codex_utils_absolute_path::test_support::PathBufExt;
|
||||
use codex_utils_absolute_path::test_support::test_path_buf;
|
||||
@@ -1612,7 +1614,7 @@ mod tests {
|
||||
let config = ConfigBuilder::default()
|
||||
.codex_home(temp_dir.path().to_path_buf())
|
||||
.harness_overrides(ConfigOverrides {
|
||||
default_permissions: Some(":workspace".to_string()),
|
||||
default_permissions: Some(BUILT_IN_PERMISSION_PROFILE_WORKSPACE.to_string()),
|
||||
..ConfigOverrides::default()
|
||||
})
|
||||
.build()
|
||||
@@ -1657,7 +1659,8 @@ mod tests {
|
||||
#[test]
|
||||
fn embedded_turn_permissions_use_active_profile_selection() {
|
||||
let cwd = test_path_buf("/workspace/project").abs();
|
||||
let active_permission_profile = ActivePermissionProfile::new(":workspace");
|
||||
let active_permission_profile =
|
||||
ActivePermissionProfile::new(BUILT_IN_PERMISSION_PROFILE_WORKSPACE);
|
||||
let expected_permissions =
|
||||
permissions_selection_from_active_profile(active_permission_profile.clone());
|
||||
|
||||
@@ -1698,7 +1701,9 @@ mod tests {
|
||||
|
||||
let (sandbox_policy, permissions) = turn_permissions_overrides(
|
||||
&PermissionProfile::read_only(),
|
||||
Some(ActivePermissionProfile::new(":read-only")),
|
||||
Some(ActivePermissionProfile::new(
|
||||
BUILT_IN_PERMISSION_PROFILE_READ_ONLY,
|
||||
)),
|
||||
cwd.as_path(),
|
||||
ThreadParamsMode::Remote,
|
||||
);
|
||||
|
||||
@@ -956,7 +956,7 @@ fn special_path_label(value: &FileSystemSpecialPath) -> String {
|
||||
match value {
|
||||
FileSystemSpecialPath::Root => ":root".to_string(),
|
||||
FileSystemSpecialPath::Minimal => ":minimal".to_string(),
|
||||
FileSystemSpecialPath::ProjectRoots { subpath } => path_label(":project_roots", subpath),
|
||||
FileSystemSpecialPath::ProjectRoots { subpath } => path_label(":workspace_roots", subpath),
|
||||
FileSystemSpecialPath::Tmpdir => ":tmpdir".to_string(),
|
||||
FileSystemSpecialPath::SlashTmp => "/tmp".to_string(),
|
||||
FileSystemSpecialPath::Unknown { path, subpath } => path_label(path, subpath),
|
||||
@@ -1771,6 +1771,31 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn additional_permissions_rule_uses_workspace_roots_label() {
|
||||
let additional_permissions = AdditionalPermissionProfile {
|
||||
network: None,
|
||||
file_system: Some(AdditionalFileSystemPermissions {
|
||||
read: None,
|
||||
write: None,
|
||||
entries: Some(vec![FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Special {
|
||||
value: FileSystemSpecialPath::ProjectRoots {
|
||||
subpath: Some(".git".into()),
|
||||
},
|
||||
},
|
||||
access: FileSystemAccessMode::Read,
|
||||
}]),
|
||||
glob_scan_max_depth: None,
|
||||
}),
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
format_additional_permissions_rule(&additional_permissions),
|
||||
Some("read `:workspace_roots/.git`".to_string())
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn permissions_session_shortcut_submits_session_scope() {
|
||||
let (tx, mut rx) = unbounded_channel::<AppEvent>();
|
||||
|
||||
@@ -16,6 +16,9 @@ use codex_protocol::account::PlanType;
|
||||
use codex_protocol::config_types::ApprovalsReviewer;
|
||||
use codex_protocol::models::ActivePermissionProfile;
|
||||
use codex_protocol::models::ActivePermissionProfileModification;
|
||||
use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_DANGER_FULL_ACCESS;
|
||||
use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_READ_ONLY;
|
||||
use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_WORKSPACE;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::openai_models::ReasoningEffort;
|
||||
use codex_utils_sandbox_summary::summarize_permission_profile;
|
||||
@@ -587,7 +590,7 @@ fn status_permissions_label(
|
||||
count => format!(" + {count} writable roots"),
|
||||
};
|
||||
match active_id {
|
||||
Some(":read-only") => {
|
||||
Some(BUILT_IN_PERMISSION_PROFILE_READ_ONLY) => {
|
||||
let label = if sandbox == "read-only with network access" {
|
||||
"Read Only with network access"
|
||||
} else {
|
||||
@@ -595,14 +598,16 @@ fn status_permissions_label(
|
||||
};
|
||||
return format!("{label}{modification_suffix} ({approval})");
|
||||
}
|
||||
Some(":workspace") => match sandbox {
|
||||
Some(BUILT_IN_PERMISSION_PROFILE_WORKSPACE) => match sandbox {
|
||||
"workspace" => return format!("Workspace{modification_suffix} ({approval})"),
|
||||
"workspace with network access" => {
|
||||
return format!("Workspace with network access{modification_suffix} ({approval})");
|
||||
}
|
||||
_ => {}
|
||||
},
|
||||
Some(":danger-no-sandbox") if permission_profile == &PermissionProfile::Disabled => {
|
||||
Some(BUILT_IN_PERMISSION_PROFILE_DANGER_FULL_ACCESS)
|
||||
if permission_profile == &PermissionProfile::Disabled =>
|
||||
{
|
||||
return if approval_policy == AskForApproval::Never {
|
||||
"Full Access".to_string()
|
||||
} else {
|
||||
|
||||
@@ -32,6 +32,8 @@ use codex_protocol::config_types::ApprovalsReviewer;
|
||||
use codex_protocol::config_types::ReasoningSummary;
|
||||
use codex_protocol::models::ActivePermissionProfile;
|
||||
use codex_protocol::models::ActivePermissionProfileModification;
|
||||
use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_READ_ONLY;
|
||||
use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_WORKSPACE;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::openai_models::ReasoningEffort;
|
||||
use codex_protocol::permissions::NetworkSandboxPolicy;
|
||||
@@ -298,7 +300,9 @@ async fn status_permissions_named_read_only_profile_shows_builtin_label() {
|
||||
.permissions
|
||||
.set_permission_profile_with_active_profile(
|
||||
PermissionProfile::read_only(),
|
||||
Some(ActivePermissionProfile::new(":read-only")),
|
||||
Some(ActivePermissionProfile::new(
|
||||
BUILT_IN_PERMISSION_PROFILE_READ_ONLY,
|
||||
)),
|
||||
)
|
||||
.expect("set permission profile");
|
||||
|
||||
@@ -329,11 +333,12 @@ async fn status_permissions_read_only_profile_shows_additional_writable_roots()
|
||||
NetworkSandboxPolicy::Restricted,
|
||||
),
|
||||
Some(
|
||||
ActivePermissionProfile::new(":read-only").with_modifications(vec![
|
||||
ActivePermissionProfileModification::AdditionalWritableRoot {
|
||||
path: extra_root,
|
||||
},
|
||||
]),
|
||||
ActivePermissionProfile::new(BUILT_IN_PERMISSION_PROFILE_READ_ONLY)
|
||||
.with_modifications(vec![
|
||||
ActivePermissionProfileModification::AdditionalWritableRoot {
|
||||
path: extra_root,
|
||||
},
|
||||
]),
|
||||
),
|
||||
)
|
||||
.expect("set permission profile");
|
||||
@@ -357,7 +362,9 @@ async fn status_permissions_named_workspace_profile_shows_builtin_label() {
|
||||
.permissions
|
||||
.set_permission_profile_with_active_profile(
|
||||
PermissionProfile::workspace_write(),
|
||||
Some(ActivePermissionProfile::new(":workspace")),
|
||||
Some(ActivePermissionProfile::new(
|
||||
BUILT_IN_PERMISSION_PROFILE_WORKSPACE,
|
||||
)),
|
||||
)
|
||||
.expect("set permission profile");
|
||||
|
||||
@@ -381,7 +388,9 @@ async fn status_permissions_workspace_auto_review_shows_reviewer_label() {
|
||||
.permissions
|
||||
.set_permission_profile_with_active_profile(
|
||||
PermissionProfile::workspace_write(),
|
||||
Some(ActivePermissionProfile::new(":workspace")),
|
||||
Some(ActivePermissionProfile::new(
|
||||
BUILT_IN_PERMISSION_PROFILE_WORKSPACE,
|
||||
)),
|
||||
)
|
||||
.expect("set permission profile");
|
||||
|
||||
@@ -411,11 +420,12 @@ async fn status_permissions_named_profile_shows_additional_writable_roots() {
|
||||
/*exclude_slash_tmp*/ false,
|
||||
),
|
||||
Some(
|
||||
ActivePermissionProfile::new(":workspace").with_modifications(vec![
|
||||
ActivePermissionProfileModification::AdditionalWritableRoot {
|
||||
path: extra_root,
|
||||
},
|
||||
]),
|
||||
ActivePermissionProfile::new(BUILT_IN_PERMISSION_PROFILE_WORKSPACE)
|
||||
.with_modifications(vec![
|
||||
ActivePermissionProfileModification::AdditionalWritableRoot {
|
||||
path: extra_root,
|
||||
},
|
||||
]),
|
||||
),
|
||||
)
|
||||
.expect("set permission profile");
|
||||
@@ -444,7 +454,9 @@ async fn status_permissions_broadened_workspace_profile_shows_builtin_label() {
|
||||
/*exclude_tmpdir_env_var*/ false,
|
||||
/*exclude_slash_tmp*/ false,
|
||||
),
|
||||
Some(ActivePermissionProfile::new(":workspace")),
|
||||
Some(ActivePermissionProfile::new(
|
||||
BUILT_IN_PERMISSION_PROFILE_WORKSPACE,
|
||||
)),
|
||||
)
|
||||
.expect("set permission profile");
|
||||
|
||||
@@ -580,7 +592,9 @@ async fn status_snapshot_shows_auto_review_permissions() {
|
||||
.permissions
|
||||
.set_permission_profile_with_active_profile(
|
||||
PermissionProfile::workspace_write(),
|
||||
Some(ActivePermissionProfile::new(":workspace")),
|
||||
Some(ActivePermissionProfile::new(
|
||||
BUILT_IN_PERMISSION_PROFILE_WORKSPACE,
|
||||
)),
|
||||
)
|
||||
.expect("set permission profile");
|
||||
|
||||
|
||||
Reference in New Issue
Block a user