mirror of
https://github.com/openai/codex.git
synced 2026-05-01 09:56:37 +00:00
app-server: accept command permission profiles (#18283)
## Why `command/exec` is another app-server entry point that can run under caller-provided permissions. It needs to accept `PermissionProfile` directly so command execution is not left behind on `SandboxPolicy` while thread APIs move forward. Command-level profiles also need to preserve the semantics clients expect from profile-relative paths. `:cwd` and cwd-relative deny globs should be anchored to the resolved command cwd for a command-specific profile, while configured deny-read restrictions such as `**/*.env = none` still need to be enforced because they can come from config or requirements rather than the command override itself. ## What Changed This adds `permissionProfile` to `CommandExecParams`, rejects requests that combine it with `sandboxPolicy`, and converts accepted profiles into the runtime filesystem/network permissions used for command execution. When a command supplies a profile, the app-server resolves that profile against the command cwd instead of the thread/server cwd. It also preserves configured deny-read entries and `globScanMaxDepth` on the effective filesystem policy so one-off command overrides cannot drop those read protections. The PR also updates app-server docs/schema fixtures and adds command-exec coverage for accepted, rejected, cwd-scoped, and deny-read-preserving profile paths. ## Verification - `cargo test -p codex-app-server command_exec_permission_profile_cwd_uses_command_cwd` - `cargo test -p codex-app-server command_profile_preserves_configured_deny_read_restrictions` - `cargo test -p codex-app-server command_exec_accepts_permission_profile` - `cargo test -p codex-app-server command_exec_rejects_sandbox_policy_with_permission_profile` - `just fix -p codex-app-server` --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/18283). * #18288 * #18287 * #18286 * #18285 * #18284 * __->__ #18283
This commit is contained in:
@@ -13,9 +13,17 @@ use codex_app_server_protocol::CommandExecResponse;
|
||||
use codex_app_server_protocol::CommandExecTerminalSize;
|
||||
use codex_app_server_protocol::CommandExecTerminateParams;
|
||||
use codex_app_server_protocol::CommandExecWriteParams;
|
||||
use codex_app_server_protocol::FileSystemAccessMode;
|
||||
use codex_app_server_protocol::FileSystemPath;
|
||||
use codex_app_server_protocol::FileSystemSandboxEntry;
|
||||
use codex_app_server_protocol::FileSystemSpecialPath;
|
||||
use codex_app_server_protocol::JSONRPCMessage;
|
||||
use codex_app_server_protocol::JSONRPCNotification;
|
||||
use codex_app_server_protocol::PermissionProfile;
|
||||
use codex_app_server_protocol::PermissionProfileFileSystemPermissions;
|
||||
use codex_app_server_protocol::PermissionProfileNetworkPermissions;
|
||||
use codex_app_server_protocol::RequestId;
|
||||
use codex_app_server_protocol::SandboxPolicy;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::collections::HashMap;
|
||||
use tempfile::TempDir;
|
||||
@@ -57,6 +65,7 @@ async fn command_exec_without_streams_can_be_terminated() -> Result<()> {
|
||||
env: None,
|
||||
size: None,
|
||||
sandbox_policy: None,
|
||||
permission_profile: None,
|
||||
})
|
||||
.await?;
|
||||
let terminate_request_id = mcp
|
||||
@@ -109,6 +118,7 @@ async fn command_exec_without_process_id_keeps_buffered_compatibility() -> Resul
|
||||
env: None,
|
||||
size: None,
|
||||
sandbox_policy: None,
|
||||
permission_profile: None,
|
||||
})
|
||||
.await?;
|
||||
|
||||
@@ -167,6 +177,7 @@ async fn command_exec_env_overrides_merge_with_server_environment_and_support_un
|
||||
])),
|
||||
size: None,
|
||||
sandbox_policy: None,
|
||||
permission_profile: None,
|
||||
})
|
||||
.await?;
|
||||
|
||||
@@ -186,6 +197,158 @@ async fn command_exec_env_overrides_merge_with_server_environment_and_support_un
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn command_exec_accepts_permission_profile() -> Result<()> {
|
||||
let server = create_mock_responses_server_sequence_unchecked(Vec::new()).await;
|
||||
let codex_home = TempDir::new()?;
|
||||
create_config_toml(codex_home.path(), &server.uri(), "never")?;
|
||||
let mut mcp = McpProcess::new(codex_home.path()).await?;
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
||||
|
||||
let command_request_id = mcp
|
||||
.send_command_exec_request(CommandExecParams {
|
||||
command: vec![
|
||||
"sh".to_string(),
|
||||
"-lc".to_string(),
|
||||
"printf 'profile'".to_string(),
|
||||
],
|
||||
process_id: None,
|
||||
tty: false,
|
||||
stream_stdin: false,
|
||||
stream_stdout_stderr: false,
|
||||
output_bytes_cap: None,
|
||||
disable_output_cap: false,
|
||||
disable_timeout: false,
|
||||
timeout_ms: None,
|
||||
cwd: None,
|
||||
env: None,
|
||||
size: None,
|
||||
sandbox_policy: None,
|
||||
permission_profile: Some(root_read_only_permission_profile()),
|
||||
})
|
||||
.await?;
|
||||
|
||||
let response = mcp
|
||||
.read_stream_until_response_message(RequestId::Integer(command_request_id))
|
||||
.await?;
|
||||
let response: CommandExecResponse = to_response(response)?;
|
||||
assert_eq!(
|
||||
response,
|
||||
CommandExecResponse {
|
||||
exit_code: 0,
|
||||
stdout: "profile".to_string(),
|
||||
stderr: String::new(),
|
||||
}
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[tokio::test]
|
||||
async fn command_exec_permission_profile_cwd_uses_command_cwd() -> Result<()> {
|
||||
let server = create_mock_responses_server_sequence_unchecked(Vec::new()).await;
|
||||
let codex_home = TempDir::new()?;
|
||||
let command_dir = codex_home.path().join("command-cwd");
|
||||
std::fs::create_dir(&command_dir)?;
|
||||
create_config_toml(codex_home.path(), &server.uri(), "never")?;
|
||||
let mut mcp = McpProcess::new(codex_home.path()).await?;
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
||||
|
||||
let mut permission_profile = root_read_only_permission_profile();
|
||||
permission_profile
|
||||
.file_system
|
||||
.as_mut()
|
||||
.expect("root read-only helper should include filesystem permissions")
|
||||
.entries
|
||||
.push(FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Special {
|
||||
value: FileSystemSpecialPath::CurrentWorkingDirectory,
|
||||
},
|
||||
access: FileSystemAccessMode::Write,
|
||||
});
|
||||
|
||||
let command_request_id = mcp
|
||||
.send_command_exec_request(CommandExecParams {
|
||||
command: vec![
|
||||
"sh".to_string(),
|
||||
"-lc".to_string(),
|
||||
"printf child > child.txt && ! printf parent > ../parent.txt".to_string(),
|
||||
],
|
||||
process_id: None,
|
||||
tty: false,
|
||||
stream_stdin: false,
|
||||
stream_stdout_stderr: false,
|
||||
output_bytes_cap: None,
|
||||
disable_output_cap: false,
|
||||
disable_timeout: false,
|
||||
timeout_ms: None,
|
||||
cwd: Some("command-cwd".into()),
|
||||
env: None,
|
||||
size: None,
|
||||
sandbox_policy: None,
|
||||
permission_profile: Some(permission_profile),
|
||||
})
|
||||
.await?;
|
||||
|
||||
let response = mcp
|
||||
.read_stream_until_response_message(RequestId::Integer(command_request_id))
|
||||
.await?;
|
||||
let response: CommandExecResponse = to_response(response)?;
|
||||
assert_eq!(
|
||||
response.exit_code, 0,
|
||||
"parent cwd write should fail under command-cwd-scoped profile: {response:?}"
|
||||
);
|
||||
assert_eq!(
|
||||
std::fs::read_to_string(command_dir.join("child.txt"))?,
|
||||
"child"
|
||||
);
|
||||
assert!(
|
||||
!codex_home.path().join("parent.txt").exists(),
|
||||
"permissionProfile :cwd write should not grant the server cwd when command cwd differs"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn command_exec_rejects_sandbox_policy_with_permission_profile() -> Result<()> {
|
||||
let server = create_mock_responses_server_sequence_unchecked(Vec::new()).await;
|
||||
let codex_home = TempDir::new()?;
|
||||
create_config_toml(codex_home.path(), &server.uri(), "never")?;
|
||||
let mut mcp = McpProcess::new(codex_home.path()).await?;
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
||||
|
||||
let command_request_id = mcp
|
||||
.send_command_exec_request(CommandExecParams {
|
||||
command: vec!["sh".to_string(), "-lc".to_string(), "true".to_string()],
|
||||
process_id: None,
|
||||
tty: false,
|
||||
stream_stdin: false,
|
||||
stream_stdout_stderr: false,
|
||||
output_bytes_cap: None,
|
||||
disable_output_cap: false,
|
||||
disable_timeout: false,
|
||||
timeout_ms: None,
|
||||
cwd: None,
|
||||
env: None,
|
||||
size: None,
|
||||
sandbox_policy: Some(SandboxPolicy::DangerFullAccess),
|
||||
permission_profile: Some(root_read_only_permission_profile()),
|
||||
})
|
||||
.await?;
|
||||
|
||||
let error = mcp
|
||||
.read_stream_until_error_message(RequestId::Integer(command_request_id))
|
||||
.await?;
|
||||
assert_eq!(
|
||||
error.error.message,
|
||||
"`permissionProfile` cannot be combined with `sandboxPolicy`"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn command_exec_rejects_disable_timeout_with_timeout_ms() -> Result<()> {
|
||||
let server = create_mock_responses_server_sequence_unchecked(Vec::new()).await;
|
||||
@@ -209,6 +372,7 @@ async fn command_exec_rejects_disable_timeout_with_timeout_ms() -> Result<()> {
|
||||
env: None,
|
||||
size: None,
|
||||
sandbox_policy: None,
|
||||
permission_profile: None,
|
||||
})
|
||||
.await?;
|
||||
|
||||
@@ -246,6 +410,7 @@ async fn command_exec_rejects_disable_output_cap_with_output_bytes_cap() -> Resu
|
||||
env: None,
|
||||
size: None,
|
||||
sandbox_policy: None,
|
||||
permission_profile: None,
|
||||
})
|
||||
.await?;
|
||||
|
||||
@@ -283,6 +448,7 @@ async fn command_exec_rejects_negative_timeout_ms() -> Result<()> {
|
||||
env: None,
|
||||
size: None,
|
||||
sandbox_policy: None,
|
||||
permission_profile: None,
|
||||
})
|
||||
.await?;
|
||||
|
||||
@@ -320,6 +486,7 @@ async fn command_exec_without_process_id_rejects_streaming() -> Result<()> {
|
||||
env: None,
|
||||
size: None,
|
||||
sandbox_policy: None,
|
||||
permission_profile: None,
|
||||
})
|
||||
.await?;
|
||||
|
||||
@@ -361,6 +528,7 @@ async fn command_exec_non_streaming_respects_output_cap() -> Result<()> {
|
||||
env: None,
|
||||
size: None,
|
||||
sandbox_policy: None,
|
||||
permission_profile: None,
|
||||
})
|
||||
.await?;
|
||||
|
||||
@@ -408,6 +576,7 @@ async fn command_exec_streaming_does_not_buffer_output() -> Result<()> {
|
||||
env: None,
|
||||
size: None,
|
||||
sandbox_policy: None,
|
||||
permission_profile: None,
|
||||
})
|
||||
.await?;
|
||||
|
||||
@@ -471,6 +640,7 @@ async fn command_exec_pipe_streams_output_and_accepts_write() -> Result<()> {
|
||||
env: None,
|
||||
size: None,
|
||||
sandbox_policy: None,
|
||||
permission_profile: None,
|
||||
})
|
||||
.await?;
|
||||
|
||||
@@ -546,6 +716,7 @@ async fn command_exec_tty_implies_streaming_and_reports_pty_output() -> Result<(
|
||||
env: None,
|
||||
size: None,
|
||||
sandbox_policy: None,
|
||||
permission_profile: None,
|
||||
})
|
||||
.await?;
|
||||
|
||||
@@ -619,6 +790,7 @@ async fn command_exec_tty_supports_initial_size_and_resize() -> Result<()> {
|
||||
cols: 101,
|
||||
}),
|
||||
sandbox_policy: None,
|
||||
permission_profile: None,
|
||||
})
|
||||
.await?;
|
||||
|
||||
@@ -888,6 +1060,23 @@ fn decode_delta_notification(
|
||||
serde_json::from_value(params).context("deserialize command/exec/outputDelta notification")
|
||||
}
|
||||
|
||||
fn root_read_only_permission_profile() -> PermissionProfile {
|
||||
PermissionProfile {
|
||||
network: Some(PermissionProfileNetworkPermissions {
|
||||
enabled: Some(false),
|
||||
}),
|
||||
file_system: Some(PermissionProfileFileSystemPermissions {
|
||||
entries: vec![FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Special {
|
||||
value: FileSystemSpecialPath::Root,
|
||||
},
|
||||
access: FileSystemAccessMode::Read,
|
||||
}],
|
||||
glob_scan_max_depth: None,
|
||||
}),
|
||||
}
|
||||
}
|
||||
|
||||
async fn read_initialize_response(
|
||||
stream: &mut super::connection_handling_websocket::WsClient,
|
||||
request_id: i64,
|
||||
|
||||
Reference in New Issue
Block a user