mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
feat(cli): add explicit sandbox permission profiles (#20117)
## Why `codex sandbox` is useful for exercising sandbox behavior directly, but before this stack the CLI only picked up permission profiles indirectly from the active config. The existing debug-sandbox path already compiled `[permissions]` profiles through normal config loading, as covered by the existing profile tests in [`debug_sandbox.rs`](https://github.com/openai/codex/blob/de2ccf94735a3d8a2a7077e6a5292026413867cf/codex-rs/cli/src/debug_sandbox.rs#L715-L760). This adds the smallest stable entry point first: an explicit profile selector that reuses the same config machinery as normal Codex config, so standalone testing becomes possible without changing current no-selector behavior. ## What changed - Add additive `--permissions-profile NAME` support to `codex sandbox macos|linux|windows`. - Resolve built-in and user-defined profile names by feeding `default_permissions` through the existing config compilation path instead of inventing a sandbox-only parser. - Make an explicit selector win over an ambient active profile's legacy `sandbox_mode`. - Keep the existing no-selector behavior unchanged. ## Stack 1. #20117 `sandbox-ui-profile` --> this PR 2. #20118 `sandbox-ui-config` Both PRs are additive. Replay JSON is intentionally deferred to a follow-up design pass. ## Tests ran - `cargo test -p codex-cli debug_sandbox` - `cargo test -p codex-cli sandbox_macos_parses_permissions_profile` - `cargo test -p codex-core cli_override_takes_precedence_over_profile_sandbox_mode` - macOS branch-binary smoke on the rebased top of stack: built-in `:workspace` and user-defined profiles both executed successfully through `--permissions-profile`. - Linux devbox branch-binary smoke on the rebased top of stack: built-in `:workspace` and user-defined profiles both executed successfully through `--permissions-profile`.
This commit is contained in:
committed by
GitHub
Unverified
parent
3d10ba9f36
commit
6ed0440611
@@ -42,12 +42,14 @@ pub async fn run_command_under_seatbelt(
|
||||
codex_linux_sandbox_exe: Option<PathBuf>,
|
||||
) -> anyhow::Result<()> {
|
||||
let SeatbeltCommand {
|
||||
permissions_profile,
|
||||
allow_unix_sockets,
|
||||
log_denials,
|
||||
config_overrides,
|
||||
command,
|
||||
} = command;
|
||||
run_command_under_sandbox(
|
||||
permissions_profile,
|
||||
command,
|
||||
config_overrides,
|
||||
codex_linux_sandbox_exe,
|
||||
@@ -71,10 +73,12 @@ pub async fn run_command_under_landlock(
|
||||
codex_linux_sandbox_exe: Option<PathBuf>,
|
||||
) -> anyhow::Result<()> {
|
||||
let LandlockCommand {
|
||||
permissions_profile,
|
||||
config_overrides,
|
||||
command,
|
||||
} = command;
|
||||
run_command_under_sandbox(
|
||||
permissions_profile,
|
||||
command,
|
||||
config_overrides,
|
||||
codex_linux_sandbox_exe,
|
||||
@@ -90,10 +94,12 @@ pub async fn run_command_under_windows(
|
||||
codex_linux_sandbox_exe: Option<PathBuf>,
|
||||
) -> anyhow::Result<()> {
|
||||
let WindowsCommand {
|
||||
permissions_profile,
|
||||
config_overrides,
|
||||
command,
|
||||
} = command;
|
||||
run_command_under_sandbox(
|
||||
permissions_profile,
|
||||
command,
|
||||
config_overrides,
|
||||
codex_linux_sandbox_exe,
|
||||
@@ -112,6 +118,7 @@ enum SandboxType {
|
||||
}
|
||||
|
||||
async fn run_command_under_sandbox(
|
||||
permissions_profile: Option<String>,
|
||||
command: Vec<String>,
|
||||
config_overrides: CliConfigOverrides,
|
||||
codex_linux_sandbox_exe: Option<PathBuf>,
|
||||
@@ -125,6 +132,7 @@ async fn run_command_under_sandbox(
|
||||
.parse_overrides()
|
||||
.map_err(anyhow::Error::msg)?,
|
||||
codex_linux_sandbox_exe,
|
||||
permissions_profile,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -563,20 +571,30 @@ mod windows_stdio_bridge {
|
||||
async fn load_debug_sandbox_config(
|
||||
cli_overrides: Vec<(String, TomlValue)>,
|
||||
codex_linux_sandbox_exe: Option<PathBuf>,
|
||||
permissions_profile: Option<String>,
|
||||
) -> anyhow::Result<Config> {
|
||||
load_debug_sandbox_config_with_codex_home(
|
||||
cli_overrides,
|
||||
codex_linux_sandbox_exe,
|
||||
permissions_profile,
|
||||
/*codex_home*/ None,
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
async fn load_debug_sandbox_config_with_codex_home(
|
||||
cli_overrides: Vec<(String, TomlValue)>,
|
||||
mut cli_overrides: Vec<(String, TomlValue)>,
|
||||
codex_linux_sandbox_exe: Option<PathBuf>,
|
||||
permissions_profile: Option<String>,
|
||||
codex_home: Option<PathBuf>,
|
||||
) -> anyhow::Result<Config> {
|
||||
if let Some(permissions_profile) = permissions_profile {
|
||||
cli_overrides.push((
|
||||
"default_permissions".to_string(),
|
||||
TomlValue::String(permissions_profile),
|
||||
));
|
||||
}
|
||||
|
||||
// For legacy configs, `codex sandbox` historically defaulted to read-only
|
||||
// instead of inheriting ambient `sandbox_mode` settings from user/system
|
||||
// config. Keep that behavior unless this invocation explicitly passes a
|
||||
@@ -698,6 +716,7 @@ mod tests {
|
||||
let config = load_debug_sandbox_config_with_codex_home(
|
||||
Vec::new(),
|
||||
/*codex_linux_sandbox_exe*/ None,
|
||||
/*permissions_profile*/ None,
|
||||
Some(codex_home_path),
|
||||
)
|
||||
.await?;
|
||||
@@ -748,6 +767,7 @@ mod tests {
|
||||
let config = load_debug_sandbox_config_with_codex_home(
|
||||
cli_overrides,
|
||||
/*codex_linux_sandbox_exe*/ None,
|
||||
/*permissions_profile*/ None,
|
||||
Some(codex_home_path),
|
||||
)
|
||||
.await?;
|
||||
@@ -797,6 +817,7 @@ mod tests {
|
||||
let config = load_debug_sandbox_config_with_codex_home(
|
||||
Vec::new(),
|
||||
/*codex_linux_sandbox_exe*/ None,
|
||||
/*permissions_profile*/ None,
|
||||
Some(codex_home_path),
|
||||
)
|
||||
.await?;
|
||||
@@ -809,4 +830,88 @@ mod tests {
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn debug_sandbox_honors_explicit_builtin_permission_profile() -> anyhow::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
|
||||
let config = load_debug_sandbox_config_with_codex_home(
|
||||
Vec::new(),
|
||||
/*codex_linux_sandbox_exe*/ None,
|
||||
Some(":workspace".to_string()),
|
||||
Some(codex_home.path().to_path_buf()),
|
||||
)
|
||||
.await?;
|
||||
|
||||
assert_eq!(
|
||||
config.permissions.file_system_sandbox_policy(),
|
||||
codex_protocol::models::PermissionProfile::workspace_write()
|
||||
.file_system_sandbox_policy()
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn explicit_permission_profile_overrides_active_profile_sandbox_mode()
|
||||
-> anyhow::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
std::fs::write(
|
||||
codex_home.path().join("config.toml"),
|
||||
"profile = \"legacy\"\n\
|
||||
\n\
|
||||
[profiles.legacy]\n\
|
||||
sandbox_mode = \"danger-full-access\"\n",
|
||||
)?;
|
||||
|
||||
let config = load_debug_sandbox_config_with_codex_home(
|
||||
Vec::new(),
|
||||
/*codex_linux_sandbox_exe*/ None,
|
||||
Some(":workspace".to_string()),
|
||||
Some(codex_home.path().to_path_buf()),
|
||||
)
|
||||
.await?;
|
||||
|
||||
assert_eq!(
|
||||
config.permissions.file_system_sandbox_policy(),
|
||||
codex_protocol::models::PermissionProfile::workspace_write()
|
||||
.file_system_sandbox_policy()
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn debug_sandbox_honors_explicit_named_permission_profile() -> anyhow::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let sandbox_paths = TempDir::new()?;
|
||||
let docs = sandbox_paths.path().join("docs");
|
||||
let private = docs.join("private");
|
||||
write_permissions_profile_config(&codex_home, &docs, &private)?;
|
||||
|
||||
let config = load_debug_sandbox_config_with_codex_home(
|
||||
Vec::new(),
|
||||
/*codex_linux_sandbox_exe*/ None,
|
||||
Some("limited-read-test".to_string()),
|
||||
Some(codex_home.path().to_path_buf()),
|
||||
)
|
||||
.await?;
|
||||
|
||||
let expected = build_debug_sandbox_config(
|
||||
vec![(
|
||||
"default_permissions".to_string(),
|
||||
TomlValue::String("limited-read-test".to_string()),
|
||||
)],
|
||||
ConfigOverrides::default(),
|
||||
Some(codex_home.path().to_path_buf()),
|
||||
)
|
||||
.await?;
|
||||
|
||||
assert_eq!(
|
||||
config.permissions.file_system_sandbox_policy(),
|
||||
expected.permissions.file_system_sandbox_policy()
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
@@ -21,6 +21,10 @@ pub use login::run_logout;
|
||||
|
||||
#[derive(Debug, Parser)]
|
||||
pub struct SeatbeltCommand {
|
||||
/// Named permissions profile to apply from the active configuration stack.
|
||||
#[arg(long = "permissions-profile", value_name = "NAME")]
|
||||
pub permissions_profile: Option<String>,
|
||||
|
||||
/// Allow the sandboxed command to bind/connect AF_UNIX sockets rooted at this path. Relative paths are resolved against the current directory. Repeat to allow multiple paths.
|
||||
#[arg(long = "allow-unix-socket", value_parser = parse_allow_unix_socket_path)]
|
||||
pub allow_unix_sockets: Vec<AbsolutePathBuf>,
|
||||
@@ -44,6 +48,10 @@ fn parse_allow_unix_socket_path(raw: &str) -> Result<AbsolutePathBuf, String> {
|
||||
|
||||
#[derive(Debug, Parser)]
|
||||
pub struct LandlockCommand {
|
||||
/// Named permissions profile to apply from the active configuration stack.
|
||||
#[arg(long = "permissions-profile", value_name = "NAME")]
|
||||
pub permissions_profile: Option<String>,
|
||||
|
||||
#[clap(skip)]
|
||||
pub config_overrides: CliConfigOverrides,
|
||||
|
||||
@@ -54,6 +62,10 @@ pub struct LandlockCommand {
|
||||
|
||||
#[derive(Debug, Parser)]
|
||||
pub struct WindowsCommand {
|
||||
/// Named permissions profile to apply from the active configuration stack.
|
||||
#[arg(long = "permissions-profile", value_name = "NAME")]
|
||||
pub permissions_profile: Option<String>,
|
||||
|
||||
#[clap(skip)]
|
||||
pub config_overrides: CliConfigOverrides,
|
||||
|
||||
|
||||
@@ -1922,6 +1922,30 @@ mod tests {
|
||||
assert!(matches!(cli.subcommand, Some(Subcommand::Update)));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sandbox_macos_parses_permissions_profile() {
|
||||
let cli = MultitoolCli::try_parse_from([
|
||||
"codex",
|
||||
"sandbox",
|
||||
"macos",
|
||||
"--permissions-profile",
|
||||
":workspace",
|
||||
"--",
|
||||
"echo",
|
||||
])
|
||||
.expect("parse");
|
||||
|
||||
let Some(Subcommand::Sandbox(SandboxArgs {
|
||||
cmd: SandboxCommand::Macos(command),
|
||||
})) = cli.subcommand
|
||||
else {
|
||||
panic!("expected sandbox macos command");
|
||||
};
|
||||
|
||||
assert_eq!(command.permissions_profile.as_deref(), Some(":workspace"));
|
||||
assert_eq!(command.command, vec!["echo"]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn plugin_marketplace_remove_parses_under_plugin() {
|
||||
let cli =
|
||||
|
||||
@@ -8,6 +8,7 @@ use crate::windows_sandbox::WindowsSandboxLevelExt;
|
||||
use crate::windows_sandbox::resolve_windows_sandbox_mode;
|
||||
use crate::windows_sandbox::resolve_windows_sandbox_private_desktop;
|
||||
use codex_config::CloudRequirementsLoader;
|
||||
use codex_config::ConfigLayerSource;
|
||||
use codex_config::ConfigLayerStack;
|
||||
use codex_config::ConfigLayerStackOrdering;
|
||||
use codex_config::ConfigRequirements;
|
||||
@@ -1531,7 +1532,30 @@ fn resolve_permission_config_syntax(
|
||||
sandbox_mode_override: Option<SandboxMode>,
|
||||
profile_sandbox_mode: Option<SandboxMode>,
|
||||
) -> Option<PermissionConfigSyntax> {
|
||||
if sandbox_mode_override.is_some() || profile_sandbox_mode.is_some() {
|
||||
if sandbox_mode_override.is_some() {
|
||||
return Some(PermissionConfigSyntax::Legacy);
|
||||
}
|
||||
|
||||
let session_flags_select_profiles = config_layer_stack
|
||||
.get_layers(
|
||||
ConfigLayerStackOrdering::HighestPrecedenceFirst,
|
||||
/*include_disabled*/ false,
|
||||
)
|
||||
.into_iter()
|
||||
.find(|layer| matches!(layer.name, ConfigLayerSource::SessionFlags))
|
||||
.and_then(|layer| {
|
||||
layer
|
||||
.config
|
||||
.clone()
|
||||
.try_into::<PermissionSelectionToml>()
|
||||
.ok()
|
||||
})
|
||||
.is_some_and(|selection| selection.default_permissions.is_some());
|
||||
if session_flags_select_profiles {
|
||||
return Some(PermissionConfigSyntax::Profiles);
|
||||
}
|
||||
|
||||
if profile_sandbox_mode.is_some() {
|
||||
return Some(PermissionConfigSyntax::Legacy);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user