Added support for sandbox_mode in profiles (#5686)
Currently, `approval_policy` is supported in profiles, but `sandbox_mode` is not. This PR adds support for `sandbox_mode`. Note: a fix for this was submitted in [this PR](https://github.com/openai/codex/pull/2397), but the underlying code has changed significantly since then. This addresses issue #3034
This commit is contained in:
@@ -1027,9 +1027,11 @@ impl ConfigToml {
|
|||||||
fn derive_sandbox_policy(
|
fn derive_sandbox_policy(
|
||||||
&self,
|
&self,
|
||||||
sandbox_mode_override: Option<SandboxMode>,
|
sandbox_mode_override: Option<SandboxMode>,
|
||||||
|
profile_sandbox_mode: Option<SandboxMode>,
|
||||||
resolved_cwd: &Path,
|
resolved_cwd: &Path,
|
||||||
) -> SandboxPolicy {
|
) -> SandboxPolicy {
|
||||||
let resolved_sandbox_mode = sandbox_mode_override
|
let resolved_sandbox_mode = sandbox_mode_override
|
||||||
|
.or(profile_sandbox_mode)
|
||||||
.or(self.sandbox_mode)
|
.or(self.sandbox_mode)
|
||||||
.or_else(|| {
|
.or_else(|| {
|
||||||
// if no sandbox_mode is set, but user has marked directory as trusted, use WorkspaceWrite
|
// if no sandbox_mode is set, but user has marked directory as trusted, use WorkspaceWrite
|
||||||
@@ -1219,7 +1221,8 @@ impl Config {
|
|||||||
.get_active_project(&resolved_cwd)
|
.get_active_project(&resolved_cwd)
|
||||||
.unwrap_or(ProjectConfig { trust_level: None });
|
.unwrap_or(ProjectConfig { trust_level: None });
|
||||||
|
|
||||||
let mut sandbox_policy = cfg.derive_sandbox_policy(sandbox_mode, &resolved_cwd);
|
let mut sandbox_policy =
|
||||||
|
cfg.derive_sandbox_policy(sandbox_mode, config_profile.sandbox_mode, &resolved_cwd);
|
||||||
if let SandboxPolicy::WorkspaceWrite { writable_roots, .. } = &mut sandbox_policy {
|
if let SandboxPolicy::WorkspaceWrite { writable_roots, .. } = &mut sandbox_policy {
|
||||||
for path in additional_writable_roots {
|
for path in additional_writable_roots {
|
||||||
if !writable_roots.iter().any(|existing| existing == &path) {
|
if !writable_roots.iter().any(|existing| existing == &path) {
|
||||||
@@ -1242,8 +1245,8 @@ impl Config {
|
|||||||
.is_some()
|
.is_some()
|
||||||
|| config_profile.approval_policy.is_some()
|
|| config_profile.approval_policy.is_some()
|
||||||
|| cfg.approval_policy.is_some()
|
|| cfg.approval_policy.is_some()
|
||||||
// TODO(#3034): profile.sandbox_mode is not implemented
|
|
||||||
|| sandbox_mode.is_some()
|
|| sandbox_mode.is_some()
|
||||||
|
|| config_profile.sandbox_mode.is_some()
|
||||||
|| cfg.sandbox_mode.is_some();
|
|| cfg.sandbox_mode.is_some();
|
||||||
|
|
||||||
let mut model_providers = built_in_model_providers();
|
let mut model_providers = built_in_model_providers();
|
||||||
@@ -1603,8 +1606,11 @@ network_access = false # This should be ignored.
|
|||||||
let sandbox_mode_override = None;
|
let sandbox_mode_override = None;
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
SandboxPolicy::DangerFullAccess,
|
SandboxPolicy::DangerFullAccess,
|
||||||
sandbox_full_access_cfg
|
sandbox_full_access_cfg.derive_sandbox_policy(
|
||||||
.derive_sandbox_policy(sandbox_mode_override, &PathBuf::from("/tmp/test"))
|
sandbox_mode_override,
|
||||||
|
None,
|
||||||
|
&PathBuf::from("/tmp/test")
|
||||||
|
)
|
||||||
);
|
);
|
||||||
|
|
||||||
let sandbox_read_only = r#"
|
let sandbox_read_only = r#"
|
||||||
@@ -1619,8 +1625,11 @@ network_access = true # This should be ignored.
|
|||||||
let sandbox_mode_override = None;
|
let sandbox_mode_override = None;
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
SandboxPolicy::ReadOnly,
|
SandboxPolicy::ReadOnly,
|
||||||
sandbox_read_only_cfg
|
sandbox_read_only_cfg.derive_sandbox_policy(
|
||||||
.derive_sandbox_policy(sandbox_mode_override, &PathBuf::from("/tmp/test"))
|
sandbox_mode_override,
|
||||||
|
None,
|
||||||
|
&PathBuf::from("/tmp/test")
|
||||||
|
)
|
||||||
);
|
);
|
||||||
|
|
||||||
let sandbox_workspace_write = r#"
|
let sandbox_workspace_write = r#"
|
||||||
@@ -1644,8 +1653,11 @@ exclude_slash_tmp = true
|
|||||||
exclude_tmpdir_env_var: true,
|
exclude_tmpdir_env_var: true,
|
||||||
exclude_slash_tmp: true,
|
exclude_slash_tmp: true,
|
||||||
},
|
},
|
||||||
sandbox_workspace_write_cfg
|
sandbox_workspace_write_cfg.derive_sandbox_policy(
|
||||||
.derive_sandbox_policy(sandbox_mode_override, &PathBuf::from("/tmp/test"))
|
sandbox_mode_override,
|
||||||
|
None,
|
||||||
|
&PathBuf::from("/tmp/test")
|
||||||
|
)
|
||||||
);
|
);
|
||||||
|
|
||||||
let sandbox_workspace_write = r#"
|
let sandbox_workspace_write = r#"
|
||||||
@@ -1672,8 +1684,11 @@ trust_level = "trusted"
|
|||||||
exclude_tmpdir_env_var: true,
|
exclude_tmpdir_env_var: true,
|
||||||
exclude_slash_tmp: true,
|
exclude_slash_tmp: true,
|
||||||
},
|
},
|
||||||
sandbox_workspace_write_cfg
|
sandbox_workspace_write_cfg.derive_sandbox_policy(
|
||||||
.derive_sandbox_policy(sandbox_mode_override, &PathBuf::from("/tmp/test"))
|
sandbox_mode_override,
|
||||||
|
None,
|
||||||
|
&PathBuf::from("/tmp/test")
|
||||||
|
)
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -1765,6 +1780,75 @@ trust_level = "trusted"
|
|||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn profile_sandbox_mode_overrides_base() -> 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,
|
||||||
|
profile: Some("work".to_string()),
|
||||||
|
sandbox_mode: Some(SandboxMode::ReadOnly),
|
||||||
|
..Default::default()
|
||||||
|
};
|
||||||
|
|
||||||
|
let config = Config::load_from_base_config_with_overrides(
|
||||||
|
cfg,
|
||||||
|
ConfigOverrides::default(),
|
||||||
|
codex_home.path().to_path_buf(),
|
||||||
|
)?;
|
||||||
|
|
||||||
|
assert!(matches!(
|
||||||
|
config.sandbox_policy,
|
||||||
|
SandboxPolicy::DangerFullAccess
|
||||||
|
));
|
||||||
|
assert!(config.did_user_set_custom_approval_policy_or_sandbox_mode);
|
||||||
|
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn cli_override_takes_precedence_over_profile_sandbox_mode() -> 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,
|
||||||
|
profile: Some("work".to_string()),
|
||||||
|
..Default::default()
|
||||||
|
};
|
||||||
|
|
||||||
|
let overrides = ConfigOverrides {
|
||||||
|
sandbox_mode: Some(SandboxMode::WorkspaceWrite),
|
||||||
|
..Default::default()
|
||||||
|
};
|
||||||
|
|
||||||
|
let config = Config::load_from_base_config_with_overrides(
|
||||||
|
cfg,
|
||||||
|
overrides,
|
||||||
|
codex_home.path().to_path_buf(),
|
||||||
|
)?;
|
||||||
|
|
||||||
|
assert!(matches!(
|
||||||
|
config.sandbox_policy,
|
||||||
|
SandboxPolicy::WorkspaceWrite { .. }
|
||||||
|
));
|
||||||
|
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn feature_table_overrides_legacy_flags() -> std::io::Result<()> {
|
fn feature_table_overrides_legacy_flags() -> std::io::Result<()> {
|
||||||
let codex_home = TempDir::new()?;
|
let codex_home = TempDir::new()?;
|
||||||
|
|||||||
@@ -4,6 +4,7 @@ use std::path::PathBuf;
|
|||||||
use crate::protocol::AskForApproval;
|
use crate::protocol::AskForApproval;
|
||||||
use codex_protocol::config_types::ReasoningEffort;
|
use codex_protocol::config_types::ReasoningEffort;
|
||||||
use codex_protocol::config_types::ReasoningSummary;
|
use codex_protocol::config_types::ReasoningSummary;
|
||||||
|
use codex_protocol::config_types::SandboxMode;
|
||||||
use codex_protocol::config_types::Verbosity;
|
use codex_protocol::config_types::Verbosity;
|
||||||
|
|
||||||
/// Collection of common configuration options that a user can define as a unit
|
/// Collection of common configuration options that a user can define as a unit
|
||||||
@@ -15,6 +16,7 @@ pub struct ConfigProfile {
|
|||||||
/// [`ModelProviderInfo`] to use.
|
/// [`ModelProviderInfo`] to use.
|
||||||
pub model_provider: Option<String>,
|
pub model_provider: Option<String>,
|
||||||
pub approval_policy: Option<AskForApproval>,
|
pub approval_policy: Option<AskForApproval>,
|
||||||
|
pub sandbox_mode: Option<SandboxMode>,
|
||||||
pub model_reasoning_effort: Option<ReasoningEffort>,
|
pub model_reasoning_effort: Option<ReasoningEffort>,
|
||||||
pub model_reasoning_summary: Option<ReasoningSummary>,
|
pub model_reasoning_summary: Option<ReasoningSummary>,
|
||||||
pub model_verbosity: Option<Verbosity>,
|
pub model_verbosity: Option<Verbosity>,
|
||||||
|
|||||||
Reference in New Issue
Block a user