mirror of
https://github.com/openai/codex.git
synced 2026-04-30 11:21:34 +03:00
feat: support allowed_sandbox_modes in requirements.toml (#8298)
This adds support for `allowed_sandbox_modes` in `requirements.toml` and provides legacy support for constraining sandbox modes in `managed_config.toml`. This is converted to `Constrained<SandboxPolicy>` in `ConfigRequirements` and applied to `Config` such that constraints are enforced throughout the harness. Note that, because `managed_config.toml` is deprecated, we do not add support for the new `external-sandbox` variant recently introduced in https://github.com/openai/codex/pull/8290. As noted, that variant is not supported in `config.toml` today, but can be configured programmatically via app server.
This commit is contained in:
@@ -1,4 +1,6 @@
|
||||
use codex_protocol::config_types::SandboxMode;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use serde::Deserialize;
|
||||
|
||||
use crate::config::Constrained;
|
||||
@@ -9,12 +11,14 @@ use crate::config::ConstraintError;
|
||||
#[derive(Debug, Clone, PartialEq)]
|
||||
pub struct ConfigRequirements {
|
||||
pub approval_policy: Constrained<AskForApproval>,
|
||||
pub sandbox_policy: Constrained<SandboxPolicy>,
|
||||
}
|
||||
|
||||
impl Default for ConfigRequirements {
|
||||
fn default() -> Self {
|
||||
Self {
|
||||
approval_policy: Constrained::allow_any_from_default(),
|
||||
sandbox_policy: Constrained::allow_any(SandboxPolicy::ReadOnly),
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -23,6 +27,34 @@ impl Default for ConfigRequirements {
|
||||
#[derive(Deserialize, Debug, Clone, Default, PartialEq)]
|
||||
pub struct ConfigRequirementsToml {
|
||||
pub allowed_approval_policies: Option<Vec<AskForApproval>>,
|
||||
pub allowed_sandbox_modes: Option<Vec<SandboxModeRequirement>>,
|
||||
}
|
||||
|
||||
/// Currently, `external-sandbox` is not supported in config.toml, but it is
|
||||
/// supported through programmatic use.
|
||||
#[derive(Deserialize, Debug, Clone, Copy, PartialEq)]
|
||||
pub enum SandboxModeRequirement {
|
||||
#[serde(rename = "read-only")]
|
||||
ReadOnly,
|
||||
|
||||
#[serde(rename = "workspace-write")]
|
||||
WorkspaceWrite,
|
||||
|
||||
#[serde(rename = "danger-full-access")]
|
||||
DangerFullAccess,
|
||||
|
||||
#[serde(rename = "external-sandbox")]
|
||||
ExternalSandbox,
|
||||
}
|
||||
|
||||
impl From<SandboxMode> for SandboxModeRequirement {
|
||||
fn from(mode: SandboxMode) -> Self {
|
||||
match mode {
|
||||
SandboxMode::ReadOnly => SandboxModeRequirement::ReadOnly,
|
||||
SandboxMode::WorkspaceWrite => SandboxModeRequirement::WorkspaceWrite,
|
||||
SandboxMode::DangerFullAccess => SandboxModeRequirement::DangerFullAccess,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl ConfigRequirementsToml {
|
||||
@@ -41,7 +73,7 @@ impl ConfigRequirementsToml {
|
||||
};
|
||||
}
|
||||
|
||||
fill_missing_take!(self, other, { allowed_approval_policies });
|
||||
fill_missing_take!(self, other, { allowed_approval_policies, allowed_sandbox_modes });
|
||||
}
|
||||
}
|
||||
|
||||
@@ -49,12 +81,13 @@ impl TryFrom<ConfigRequirementsToml> for ConfigRequirements {
|
||||
type Error = ConstraintError;
|
||||
|
||||
fn try_from(toml: ConfigRequirementsToml) -> Result<Self, Self::Error> {
|
||||
let approval_policy: Constrained<AskForApproval> = match toml.allowed_approval_policies {
|
||||
let ConfigRequirementsToml {
|
||||
allowed_approval_policies,
|
||||
allowed_sandbox_modes,
|
||||
} = toml;
|
||||
let approval_policy: Constrained<AskForApproval> = match allowed_approval_policies {
|
||||
Some(policies) => {
|
||||
let default_value = AskForApproval::default();
|
||||
if policies.contains(&default_value) {
|
||||
Constrained::allow_values(default_value, policies)?
|
||||
} else if let Some(first) = policies.first() {
|
||||
if let Some(first) = policies.first() {
|
||||
Constrained::allow_values(*first, policies)?
|
||||
} else {
|
||||
return Err(ConstraintError::empty_field("allowed_approval_policies"));
|
||||
@@ -62,7 +95,51 @@ impl TryFrom<ConfigRequirementsToml> for ConfigRequirements {
|
||||
}
|
||||
None => Constrained::allow_any_from_default(),
|
||||
};
|
||||
Ok(ConfigRequirements { approval_policy })
|
||||
|
||||
// TODO(gt): `ConfigRequirementsToml` should let the author specify the
|
||||
// default `SandboxPolicy`? Should do this for `AskForApproval` too?
|
||||
//
|
||||
// Currently, we force ReadOnly as the default policy because two of
|
||||
// the other variants (WorkspaceWrite, ExternalSandbox) require
|
||||
// additional parameters. Ultimately, we should expand the config
|
||||
// format to allow specifying those parameters.
|
||||
let default_sandbox_policy = SandboxPolicy::ReadOnly;
|
||||
let sandbox_policy: Constrained<SandboxPolicy> = match allowed_sandbox_modes {
|
||||
Some(modes) => {
|
||||
if !modes.contains(&SandboxModeRequirement::ReadOnly) {
|
||||
return Err(ConstraintError::invalid_value(
|
||||
"allowed_sandbox_modes",
|
||||
"must include 'read-only' to allow any SandboxPolicy",
|
||||
));
|
||||
};
|
||||
|
||||
Constrained::new(default_sandbox_policy, move |candidate| {
|
||||
let mode = match candidate {
|
||||
SandboxPolicy::ReadOnly => SandboxModeRequirement::ReadOnly,
|
||||
SandboxPolicy::WorkspaceWrite { .. } => {
|
||||
SandboxModeRequirement::WorkspaceWrite
|
||||
}
|
||||
SandboxPolicy::DangerFullAccess => SandboxModeRequirement::DangerFullAccess,
|
||||
SandboxPolicy::ExternalSandbox { .. } => {
|
||||
SandboxModeRequirement::ExternalSandbox
|
||||
}
|
||||
};
|
||||
if modes.contains(&mode) {
|
||||
Ok(())
|
||||
} else {
|
||||
Err(ConstraintError::invalid_value(
|
||||
format!("{candidate:?}"),
|
||||
format!("{modes:?}"),
|
||||
))
|
||||
}
|
||||
})?
|
||||
}
|
||||
None => Constrained::allow_any(default_sandbox_policy),
|
||||
};
|
||||
Ok(ConfigRequirements {
|
||||
approval_policy,
|
||||
sandbox_policy,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -70,6 +147,8 @@ impl TryFrom<ConfigRequirementsToml> for ConfigRequirements {
|
||||
mod tests {
|
||||
use super::*;
|
||||
use anyhow::Result;
|
||||
use codex_protocol::protocol::NetworkAccess;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use pretty_assertions::assert_eq;
|
||||
use toml::from_str;
|
||||
|
||||
@@ -104,4 +183,105 @@ mod tests {
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn deserialize_allowed_approval_policies() -> Result<()> {
|
||||
let toml_str = r#"
|
||||
allowed_approval_policies = ["untrusted", "on-request"]
|
||||
"#;
|
||||
let config: ConfigRequirementsToml = from_str(toml_str)?;
|
||||
let requirements = ConfigRequirements::try_from(config)?;
|
||||
|
||||
assert_eq!(
|
||||
requirements.approval_policy.value(),
|
||||
AskForApproval::UnlessTrusted,
|
||||
"currently, there is no way to specify the default value for approval policy in the toml, so it picks the first allowed value"
|
||||
);
|
||||
assert!(
|
||||
requirements
|
||||
.approval_policy
|
||||
.can_set(&AskForApproval::UnlessTrusted)
|
||||
.is_ok()
|
||||
);
|
||||
assert_eq!(
|
||||
requirements
|
||||
.approval_policy
|
||||
.can_set(&AskForApproval::OnFailure),
|
||||
Err(ConstraintError::InvalidValue {
|
||||
candidate: "OnFailure".into(),
|
||||
allowed: "[UnlessTrusted, OnRequest]".into(),
|
||||
})
|
||||
);
|
||||
assert!(
|
||||
requirements
|
||||
.approval_policy
|
||||
.can_set(&AskForApproval::OnRequest)
|
||||
.is_ok()
|
||||
);
|
||||
assert_eq!(
|
||||
requirements.approval_policy.can_set(&AskForApproval::Never),
|
||||
Err(ConstraintError::InvalidValue {
|
||||
candidate: "Never".into(),
|
||||
allowed: "[UnlessTrusted, OnRequest]".into(),
|
||||
})
|
||||
);
|
||||
assert!(
|
||||
requirements
|
||||
.sandbox_policy
|
||||
.can_set(&SandboxPolicy::ReadOnly)
|
||||
.is_ok()
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn deserialize_allowed_sandbox_modes() -> Result<()> {
|
||||
let toml_str = r#"
|
||||
allowed_sandbox_modes = ["read-only", "workspace-write"]
|
||||
"#;
|
||||
let config: ConfigRequirementsToml = from_str(toml_str)?;
|
||||
let requirements = ConfigRequirements::try_from(config)?;
|
||||
|
||||
let root = if cfg!(windows) { "C:\\repo" } else { "/repo" };
|
||||
assert!(
|
||||
requirements
|
||||
.sandbox_policy
|
||||
.can_set(&SandboxPolicy::ReadOnly)
|
||||
.is_ok()
|
||||
);
|
||||
assert!(
|
||||
requirements
|
||||
.sandbox_policy
|
||||
.can_set(&SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: vec![AbsolutePathBuf::from_absolute_path(root)?],
|
||||
network_access: false,
|
||||
exclude_tmpdir_env_var: false,
|
||||
exclude_slash_tmp: false,
|
||||
})
|
||||
.is_ok()
|
||||
);
|
||||
assert_eq!(
|
||||
requirements
|
||||
.sandbox_policy
|
||||
.can_set(&SandboxPolicy::DangerFullAccess),
|
||||
Err(ConstraintError::InvalidValue {
|
||||
candidate: "DangerFullAccess".into(),
|
||||
allowed: "[ReadOnly, WorkspaceWrite]".into(),
|
||||
})
|
||||
);
|
||||
assert_eq!(
|
||||
requirements
|
||||
.sandbox_policy
|
||||
.can_set(&SandboxPolicy::ExternalSandbox {
|
||||
network_access: NetworkAccess::Restricted,
|
||||
}),
|
||||
Err(ConstraintError::InvalidValue {
|
||||
candidate: "ExternalSandbox { network_access: Restricted }".into(),
|
||||
allowed: "[ReadOnly, WorkspaceWrite]".into(),
|
||||
})
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
@@ -14,6 +14,7 @@ use crate::config::CONFIG_TOML_FILE;
|
||||
use crate::config_loader::config_requirements::ConfigRequirementsToml;
|
||||
use crate::config_loader::layer_io::LoadedConfigLayers;
|
||||
use codex_app_server_protocol::ConfigLayerSource;
|
||||
use codex_protocol::config_types::SandboxMode;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use serde::Deserialize;
|
||||
@@ -238,17 +239,23 @@ async fn load_requirements_from_legacy_scheme(
|
||||
#[derive(Deserialize, Debug, Clone, Default, PartialEq)]
|
||||
struct LegacyManagedConfigToml {
|
||||
approval_policy: Option<AskForApproval>,
|
||||
sandbox_mode: Option<SandboxMode>,
|
||||
}
|
||||
|
||||
impl From<LegacyManagedConfigToml> for ConfigRequirementsToml {
|
||||
fn from(legacy: LegacyManagedConfigToml) -> Self {
|
||||
let mut config_requirements_toml = ConfigRequirementsToml::default();
|
||||
|
||||
let LegacyManagedConfigToml { approval_policy } = legacy;
|
||||
let LegacyManagedConfigToml {
|
||||
approval_policy,
|
||||
sandbox_mode,
|
||||
} = legacy;
|
||||
if let Some(approval_policy) = approval_policy {
|
||||
config_requirements_toml.allowed_approval_policies = Some(vec![approval_policy]);
|
||||
}
|
||||
|
||||
if let Some(sandbox_mode) = sandbox_mode {
|
||||
config_requirements_toml.allowed_sandbox_modes = Some(vec![sandbox_mode.into()]);
|
||||
}
|
||||
config_requirements_toml
|
||||
}
|
||||
}
|
||||
|
||||
@@ -176,7 +176,7 @@ allowed_approval_policies = ["never", "on-request"]
|
||||
let config_requirements: ConfigRequirements = config_requirements_toml.try_into()?;
|
||||
assert_eq!(
|
||||
config_requirements.approval_policy.value(),
|
||||
AskForApproval::OnRequest
|
||||
AskForApproval::Never
|
||||
);
|
||||
config_requirements
|
||||
.approval_policy
|
||||
|
||||
Reference in New Issue
Block a user