mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
config: remove dead profile sandbox fallback (#25943)
## Why `profile_sandbox_mode` was left over from the old selected legacy profile path. Production now always derives permissions without that value, and legacy profile contents are ignored, so keeping a parameter that is always `None` makes `derive_permission_profile` look like it still supports a fallback that no longer exists. ## What Changed - Removed the `profile_sandbox_mode` argument from `ConfigToml::derive_permission_profile`. - Updated the production caller and legacy sandbox-policy test helper to match. - Dropped the stale unselected legacy-profile sandbox test that only protected the removed fallback shape. ## Verification - `just test -p codex-config` - `just test -p codex-core 'config::'` --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/25943). * #25926 * __->__ #25943
This commit is contained in:
committed by
GitHub
Unverified
parent
d55e5a9bde
commit
c6d76750e8
@@ -724,16 +724,13 @@ impl ConfigToml {
|
||||
pub async fn derive_permission_profile(
|
||||
&self,
|
||||
sandbox_mode_override: Option<SandboxMode>,
|
||||
profile_sandbox_mode: Option<SandboxMode>,
|
||||
windows_sandbox_level: WindowsSandboxLevel,
|
||||
active_project: Option<&ProjectConfig>,
|
||||
permission_profile_constraint: Option<&crate::Constrained<PermissionProfile>>,
|
||||
) -> PermissionProfile {
|
||||
let sandbox_mode_was_explicit = sandbox_mode_override.is_some()
|
||||
|| profile_sandbox_mode.is_some()
|
||||
|| self.sandbox_mode.is_some();
|
||||
let sandbox_mode_was_explicit =
|
||||
sandbox_mode_override.is_some() || self.sandbox_mode.is_some();
|
||||
let resolved_sandbox_mode = sandbox_mode_override
|
||||
.or(profile_sandbox_mode)
|
||||
.or(self.sandbox_mode)
|
||||
.or(if sandbox_mode_was_explicit {
|
||||
None
|
||||
|
||||
@@ -32,7 +32,6 @@ use codex_config::permissions_toml::NetworkToml;
|
||||
use codex_config::permissions_toml::PermissionProfileToml;
|
||||
use codex_config::permissions_toml::PermissionsToml;
|
||||
use codex_config::permissions_toml::WorkspaceRootsToml;
|
||||
use codex_config::profile_toml::ConfigProfile;
|
||||
use codex_config::types::AppToolApproval;
|
||||
use codex_config::types::ApprovalsReviewer;
|
||||
use codex_config::types::BundledSkillsConfig;
|
||||
@@ -165,7 +164,6 @@ fn http_mcp(url: &str) -> McpServerConfig {
|
||||
async fn derive_legacy_sandbox_policy_for_test(
|
||||
cfg: &ConfigToml,
|
||||
sandbox_mode_override: Option<SandboxMode>,
|
||||
profile_sandbox_mode: Option<SandboxMode>,
|
||||
windows_sandbox_level: WindowsSandboxLevel,
|
||||
active_project: Option<&ProjectConfig>,
|
||||
permission_profile_constraint: Option<&Constrained<PermissionProfile>>,
|
||||
@@ -173,7 +171,6 @@ async fn derive_legacy_sandbox_policy_for_test(
|
||||
let permission_profile = cfg
|
||||
.derive_permission_profile(
|
||||
sandbox_mode_override,
|
||||
profile_sandbox_mode,
|
||||
windows_sandbox_level,
|
||||
active_project,
|
||||
permission_profile_constraint,
|
||||
@@ -3530,7 +3527,6 @@ network_access = false # This should be ignored.
|
||||
let resolution = derive_legacy_sandbox_policy_for_test(
|
||||
&sandbox_full_access_cfg,
|
||||
sandbox_mode_override,
|
||||
/*profile_sandbox_mode*/ None,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
/*active_project*/ None,
|
||||
/*permission_profile_constraint*/ None,
|
||||
@@ -3551,7 +3547,6 @@ network_access = true # This should be ignored.
|
||||
let resolution = derive_legacy_sandbox_policy_for_test(
|
||||
&sandbox_read_only_cfg,
|
||||
sandbox_mode_override,
|
||||
/*profile_sandbox_mode*/ None,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
/*active_project*/ None,
|
||||
/*permission_profile_constraint*/ None,
|
||||
@@ -3583,7 +3578,6 @@ trust_level = "trusted"
|
||||
let resolution = derive_legacy_sandbox_policy_for_test(
|
||||
&sandbox_workspace_write_cfg,
|
||||
sandbox_mode_override,
|
||||
/*profile_sandbox_mode*/ None,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
/*active_project*/ None,
|
||||
/*permission_profile_constraint*/ None,
|
||||
@@ -3623,7 +3617,6 @@ exclude_slash_tmp = true
|
||||
let resolution = derive_legacy_sandbox_policy_for_test(
|
||||
&sandbox_workspace_write_cfg,
|
||||
sandbox_mode_override,
|
||||
/*profile_sandbox_mode*/ None,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
/*active_project*/ None,
|
||||
/*permission_profile_constraint*/ None,
|
||||
@@ -4998,38 +4991,6 @@ model = "gpt-project-local"
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn unselected_profile_sandbox_mode_is_ignored() -> std::io::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let mut profiles = HashMap::new();
|
||||
profiles.insert(
|
||||
"work".to_string(),
|
||||
ConfigProfile {
|
||||
sandbox_mode: Some(SandboxMode::DangerFullAccess),
|
||||
..Default::default()
|
||||
},
|
||||
);
|
||||
let cfg = ConfigToml {
|
||||
profiles,
|
||||
sandbox_mode: Some(SandboxMode::ReadOnly),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
let config = Config::load_from_base_config_with_overrides(
|
||||
cfg,
|
||||
ConfigOverrides::default(),
|
||||
codex_home.abs(),
|
||||
)
|
||||
.await?;
|
||||
|
||||
assert_eq!(
|
||||
config.legacy_sandbox_policy(),
|
||||
SandboxPolicy::new_read_only_policy()
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn feature_table_overrides_legacy_flags() -> std::io::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
@@ -8587,7 +8548,6 @@ trust_level = "untrusted"
|
||||
let resolution = derive_legacy_sandbox_policy_for_test(
|
||||
&cfg,
|
||||
/*sandbox_mode_override*/ None,
|
||||
/*profile_sandbox_mode*/ None,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
Some(&active_project),
|
||||
/*permission_profile_constraint*/ None,
|
||||
@@ -8644,7 +8604,6 @@ async fn derive_sandbox_policy_falls_back_to_read_only_for_implicit_defaults() -
|
||||
let resolution = derive_legacy_sandbox_policy_for_test(
|
||||
&cfg,
|
||||
/*sandbox_mode_override*/ None,
|
||||
/*profile_sandbox_mode*/ None,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
Some(&active_project),
|
||||
Some(&constrained),
|
||||
@@ -8697,7 +8656,6 @@ async fn derive_sandbox_policy_preserves_windows_downgrade_for_unsupported_fallb
|
||||
let resolution = derive_legacy_sandbox_policy_for_test(
|
||||
&cfg,
|
||||
/*sandbox_mode_override*/ None,
|
||||
/*profile_sandbox_mode*/ None,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
Some(&active_project),
|
||||
Some(&constrained),
|
||||
|
||||
@@ -2936,7 +2936,6 @@ impl Config {
|
||||
let mut permission_profile = cfg
|
||||
.derive_permission_profile(
|
||||
sandbox_mode,
|
||||
/*profile_sandbox_mode*/ None,
|
||||
windows_sandbox_level,
|
||||
Some(&active_project),
|
||||
Some(&constrained_permission_profile),
|
||||
|
||||
Reference in New Issue
Block a user