[codex] Default values from requirements if unset (#10531)
If we don't set any explicit values for sandbox or approval policy, let's try to use a requirements-satisfying value.
This commit is contained in:
parent
477379b83c
commit
8406bd7672
1 changed files with 267 additions and 14 deletions
|
|
@ -1094,7 +1094,11 @@ impl ConfigToml {
|
|||
profile_sandbox_mode: Option<SandboxMode>,
|
||||
windows_sandbox_level: WindowsSandboxLevel,
|
||||
resolved_cwd: &Path,
|
||||
sandbox_policy_constraint: Option<&Constrained<SandboxPolicy>>,
|
||||
) -> SandboxPolicyResolution {
|
||||
let sandbox_mode_was_explicit = sandbox_mode_override.is_some()
|
||||
|| profile_sandbox_mode.is_some()
|
||||
|| self.sandbox_mode.is_some();
|
||||
let resolved_sandbox_mode = sandbox_mode_override
|
||||
.or(profile_sandbox_mode)
|
||||
.or(self.sandbox_mode)
|
||||
|
|
@ -1128,13 +1132,30 @@ impl ConfigToml {
|
|||
SandboxMode::DangerFullAccess => SandboxPolicy::DangerFullAccess,
|
||||
};
|
||||
let mut forced_auto_mode_downgraded_on_windows = false;
|
||||
if cfg!(target_os = "windows")
|
||||
&& matches!(resolved_sandbox_mode, SandboxMode::WorkspaceWrite)
|
||||
// If the experimental Windows sandbox is enabled, do not force a downgrade.
|
||||
&& windows_sandbox_level == codex_protocol::config_types::WindowsSandboxLevel::Disabled
|
||||
let mut downgrade_workspace_write_if_unsupported = |policy: &mut SandboxPolicy| {
|
||||
if cfg!(target_os = "windows")
|
||||
// If the experimental Windows sandbox is enabled, do not force a downgrade.
|
||||
&& windows_sandbox_level
|
||||
== codex_protocol::config_types::WindowsSandboxLevel::Disabled
|
||||
&& matches!(&*policy, SandboxPolicy::WorkspaceWrite { .. })
|
||||
{
|
||||
*policy = SandboxPolicy::new_read_only_policy();
|
||||
forced_auto_mode_downgraded_on_windows = true;
|
||||
}
|
||||
};
|
||||
if matches!(resolved_sandbox_mode, SandboxMode::WorkspaceWrite) {
|
||||
downgrade_workspace_write_if_unsupported(&mut sandbox_policy);
|
||||
}
|
||||
if !sandbox_mode_was_explicit
|
||||
&& let Some(constraint) = sandbox_policy_constraint
|
||||
&& let Err(err) = constraint.can_set(&sandbox_policy)
|
||||
{
|
||||
sandbox_policy = SandboxPolicy::new_read_only_policy();
|
||||
forced_auto_mode_downgraded_on_windows = true;
|
||||
tracing::warn!(
|
||||
error = %err,
|
||||
"default sandbox policy is disallowed by requirements; falling back to required default"
|
||||
);
|
||||
sandbox_policy = constraint.get().clone();
|
||||
downgrade_workspace_write_if_unsupported(&mut sandbox_policy);
|
||||
}
|
||||
SandboxPolicyResolution {
|
||||
policy: sandbox_policy,
|
||||
|
|
@ -1364,6 +1385,9 @@ impl Config {
|
|||
let active_project = cfg
|
||||
.get_active_project(&resolved_cwd)
|
||||
.unwrap_or(ProjectConfig { trust_level: None });
|
||||
let sandbox_mode_was_explicit = sandbox_mode.is_some()
|
||||
|| config_profile.sandbox_mode.is_some()
|
||||
|| cfg.sandbox_mode.is_some();
|
||||
|
||||
let windows_sandbox_level = WindowsSandboxLevel::from_features(&features);
|
||||
let SandboxPolicyResolution {
|
||||
|
|
@ -1374,6 +1398,7 @@ impl Config {
|
|||
config_profile.sandbox_mode,
|
||||
windows_sandbox_level,
|
||||
&resolved_cwd,
|
||||
Some(&requirements.sandbox_policy),
|
||||
);
|
||||
if let SandboxPolicy::WorkspaceWrite { writable_roots, .. } = &mut sandbox_policy {
|
||||
for path in additional_writable_roots {
|
||||
|
|
@ -1382,7 +1407,10 @@ impl Config {
|
|||
}
|
||||
}
|
||||
}
|
||||
let approval_policy = approval_policy_override
|
||||
let approval_policy_was_explicit = approval_policy_override.is_some()
|
||||
|| config_profile.approval_policy.is_some()
|
||||
|| cfg.approval_policy.is_some();
|
||||
let mut approval_policy = approval_policy_override
|
||||
.or(config_profile.approval_policy)
|
||||
.or(cfg.approval_policy)
|
||||
.unwrap_or_else(|| {
|
||||
|
|
@ -1394,16 +1422,20 @@ impl Config {
|
|||
AskForApproval::default()
|
||||
}
|
||||
});
|
||||
if !approval_policy_was_explicit
|
||||
&& let Err(err) = requirements.approval_policy.can_set(&approval_policy)
|
||||
{
|
||||
tracing::warn!(
|
||||
error = %err,
|
||||
"default approval policy is disallowed by requirements; falling back to required default"
|
||||
);
|
||||
approval_policy = requirements.approval_policy.value();
|
||||
}
|
||||
let web_search_mode = resolve_web_search_mode(&cfg, &config_profile, &features);
|
||||
// TODO(dylan): We should be able to leverage ConfigLayerStack so that
|
||||
// we can reliably check this at every config level.
|
||||
let did_user_set_custom_approval_policy_or_sandbox_mode = approval_policy_override
|
||||
.is_some()
|
||||
|| config_profile.approval_policy.is_some()
|
||||
|| cfg.approval_policy.is_some()
|
||||
|| sandbox_mode.is_some()
|
||||
|| config_profile.sandbox_mode.is_some()
|
||||
|| cfg.sandbox_mode.is_some();
|
||||
let did_user_set_custom_approval_policy_or_sandbox_mode =
|
||||
approval_policy_was_explicit || sandbox_mode_was_explicit;
|
||||
|
||||
let mut model_providers = built_in_model_providers();
|
||||
// Merge user-defined providers into the built-in list.
|
||||
|
|
@ -1919,6 +1951,7 @@ network_access = false # This should be ignored.
|
|||
None,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
&PathBuf::from("/tmp/test"),
|
||||
None,
|
||||
);
|
||||
assert_eq!(
|
||||
resolution,
|
||||
|
|
@ -1943,6 +1976,7 @@ network_access = true # This should be ignored.
|
|||
None,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
&PathBuf::from("/tmp/test"),
|
||||
None,
|
||||
);
|
||||
assert_eq!(
|
||||
resolution,
|
||||
|
|
@ -1975,6 +2009,7 @@ exclude_slash_tmp = true
|
|||
None,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
&PathBuf::from("/tmp/test"),
|
||||
None,
|
||||
);
|
||||
if cfg!(target_os = "windows") {
|
||||
assert_eq!(
|
||||
|
|
@ -2024,6 +2059,7 @@ trust_level = "trusted"
|
|||
None,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
&PathBuf::from("/tmp/test"),
|
||||
None,
|
||||
);
|
||||
if cfg!(target_os = "windows") {
|
||||
assert_eq!(
|
||||
|
|
@ -4331,6 +4367,7 @@ trust_level = "untrusted"
|
|||
None,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
&PathBuf::from("/tmp/test"),
|
||||
None,
|
||||
);
|
||||
|
||||
// Verify that untrusted projects get WorkspaceWrite (or ReadOnly on Windows due to downgrade)
|
||||
|
|
@ -4351,6 +4388,103 @@ trust_level = "untrusted"
|
|||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn derive_sandbox_policy_falls_back_to_constraint_value_for_implicit_defaults()
|
||||
-> anyhow::Result<()> {
|
||||
let project_dir = TempDir::new()?;
|
||||
let project_path = project_dir.path().to_path_buf();
|
||||
let project_key = project_path.to_string_lossy().to_string();
|
||||
let cfg = ConfigToml {
|
||||
projects: Some(HashMap::from([(
|
||||
project_key,
|
||||
ProjectConfig {
|
||||
trust_level: Some(TrustLevel::Trusted),
|
||||
},
|
||||
)])),
|
||||
..Default::default()
|
||||
};
|
||||
let constrained = Constrained::new(SandboxPolicy::DangerFullAccess, |candidate| {
|
||||
if matches!(candidate, SandboxPolicy::DangerFullAccess) {
|
||||
Ok(())
|
||||
} else {
|
||||
Err(ConstraintError::InvalidValue {
|
||||
field_name: "sandbox_mode",
|
||||
candidate: format!("{candidate:?}"),
|
||||
allowed: "[DangerFullAccess]".to_string(),
|
||||
requirement_source: RequirementSource::Unknown,
|
||||
})
|
||||
}
|
||||
})?;
|
||||
|
||||
let resolution = cfg.derive_sandbox_policy(
|
||||
None,
|
||||
None,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
&project_path,
|
||||
Some(&constrained),
|
||||
);
|
||||
|
||||
assert_eq!(resolution.policy, SandboxPolicy::DangerFullAccess);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn derive_sandbox_policy_preserves_windows_downgrade_for_unsupported_fallback()
|
||||
-> anyhow::Result<()> {
|
||||
let project_dir = TempDir::new()?;
|
||||
let project_path = project_dir.path().to_path_buf();
|
||||
let project_key = project_path.to_string_lossy().to_string();
|
||||
let cfg = ConfigToml {
|
||||
projects: Some(HashMap::from([(
|
||||
project_key,
|
||||
ProjectConfig {
|
||||
trust_level: Some(TrustLevel::Trusted),
|
||||
},
|
||||
)])),
|
||||
..Default::default()
|
||||
};
|
||||
let constrained =
|
||||
Constrained::new(SandboxPolicy::new_workspace_write_policy(), |candidate| {
|
||||
if matches!(candidate, SandboxPolicy::WorkspaceWrite { .. }) {
|
||||
Ok(())
|
||||
} else {
|
||||
Err(ConstraintError::InvalidValue {
|
||||
field_name: "sandbox_mode",
|
||||
candidate: format!("{candidate:?}"),
|
||||
allowed: "[WorkspaceWrite]".to_string(),
|
||||
requirement_source: RequirementSource::Unknown,
|
||||
})
|
||||
}
|
||||
})?;
|
||||
|
||||
let resolution = cfg.derive_sandbox_policy(
|
||||
None,
|
||||
None,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
&project_path,
|
||||
Some(&constrained),
|
||||
);
|
||||
|
||||
if cfg!(target_os = "windows") {
|
||||
assert_eq!(
|
||||
resolution,
|
||||
SandboxPolicyResolution {
|
||||
policy: SandboxPolicy::ReadOnly,
|
||||
forced_auto_mode_downgraded_on_windows: true,
|
||||
}
|
||||
);
|
||||
} else {
|
||||
assert_eq!(
|
||||
resolution,
|
||||
SandboxPolicyResolution {
|
||||
policy: SandboxPolicy::new_workspace_write_policy(),
|
||||
forced_auto_mode_downgraded_on_windows: false,
|
||||
}
|
||||
);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_resolve_oss_provider_explicit_override() {
|
||||
let config_toml = ConfigToml::default();
|
||||
|
|
@ -4506,6 +4640,125 @@ mcp_oauth_callback_port = 5678
|
|||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn requirements_disallowing_default_sandbox_falls_back_to_required_default()
|
||||
-> std::io::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
|
||||
let config = ConfigBuilder::default()
|
||||
.codex_home(codex_home.path().to_path_buf())
|
||||
.cloud_requirements(CloudRequirementsLoader::new(async {
|
||||
Some(crate::config_loader::ConfigRequirementsToml {
|
||||
allowed_sandbox_modes: Some(vec![
|
||||
crate::config_loader::SandboxModeRequirement::ReadOnly,
|
||||
]),
|
||||
..Default::default()
|
||||
})
|
||||
}))
|
||||
.build()
|
||||
.await?;
|
||||
|
||||
assert_eq!(*config.sandbox_policy.get(), SandboxPolicy::ReadOnly);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn explicit_sandbox_mode_still_errors_when_disallowed_by_requirements()
|
||||
-> std::io::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
std::fs::write(
|
||||
codex_home.path().join(CONFIG_TOML_FILE),
|
||||
r#"sandbox_mode = "danger-full-access"
|
||||
"#,
|
||||
)?;
|
||||
|
||||
let requirements = crate::config_loader::ConfigRequirementsToml {
|
||||
allowed_approval_policies: None,
|
||||
allowed_sandbox_modes: Some(vec![
|
||||
crate::config_loader::SandboxModeRequirement::ReadOnly,
|
||||
]),
|
||||
mcp_servers: None,
|
||||
rules: None,
|
||||
enforce_residency: None,
|
||||
};
|
||||
|
||||
let err = ConfigBuilder::default()
|
||||
.codex_home(codex_home.path().to_path_buf())
|
||||
.fallback_cwd(Some(codex_home.path().to_path_buf()))
|
||||
.cloud_requirements(CloudRequirementsLoader::new(
|
||||
async move { Some(requirements) },
|
||||
))
|
||||
.build()
|
||||
.await
|
||||
.expect_err("explicit disallowed mode should still fail");
|
||||
assert_eq!(err.kind(), std::io::ErrorKind::InvalidInput);
|
||||
let message = err.to_string();
|
||||
assert!(message.contains("invalid value for `sandbox_mode`"));
|
||||
assert!(message.contains("set by cloud requirements"));
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn requirements_disallowing_default_approval_falls_back_to_required_default()
|
||||
-> std::io::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let workspace = TempDir::new()?;
|
||||
let workspace_key = workspace.path().to_string_lossy().replace('\\', "\\\\");
|
||||
std::fs::write(
|
||||
codex_home.path().join(CONFIG_TOML_FILE),
|
||||
format!(
|
||||
r#"
|
||||
[projects."{workspace_key}"]
|
||||
trust_level = "untrusted"
|
||||
"#
|
||||
),
|
||||
)?;
|
||||
|
||||
let config = ConfigBuilder::default()
|
||||
.codex_home(codex_home.path().to_path_buf())
|
||||
.fallback_cwd(Some(workspace.path().to_path_buf()))
|
||||
.cloud_requirements(CloudRequirementsLoader::new(async {
|
||||
Some(crate::config_loader::ConfigRequirementsToml {
|
||||
allowed_approval_policies: Some(vec![AskForApproval::OnRequest]),
|
||||
..Default::default()
|
||||
})
|
||||
}))
|
||||
.build()
|
||||
.await?;
|
||||
|
||||
assert_eq!(config.approval_policy.value(), AskForApproval::OnRequest);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn explicit_approval_policy_still_errors_when_disallowed_by_requirements()
|
||||
-> std::io::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
std::fs::write(
|
||||
codex_home.path().join(CONFIG_TOML_FILE),
|
||||
r#"approval_policy = "untrusted"
|
||||
"#,
|
||||
)?;
|
||||
|
||||
let err = ConfigBuilder::default()
|
||||
.codex_home(codex_home.path().to_path_buf())
|
||||
.fallback_cwd(Some(codex_home.path().to_path_buf()))
|
||||
.cloud_requirements(CloudRequirementsLoader::new(async {
|
||||
Some(crate::config_loader::ConfigRequirementsToml {
|
||||
allowed_approval_policies: Some(vec![AskForApproval::OnRequest]),
|
||||
..Default::default()
|
||||
})
|
||||
}))
|
||||
.build()
|
||||
.await
|
||||
.expect_err("explicit disallowed approval policy should fail");
|
||||
assert_eq!(err.kind(), std::io::ErrorKind::InvalidInput);
|
||||
let message = err.to_string();
|
||||
assert!(message.contains("invalid value for `approval_policy`"));
|
||||
assert!(message.contains("set by cloud requirements"));
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue