feat(core) Introduce Feature::RequestPermissions (#11871)

## Summary
Introduces the initial implementation of Feature::RequestPermissions.
RequestPermissions allows the model to request that a command be run
inside the sandbox, with additional permissions, like writing to a
specific folder. Eventually this will include other rules as well, and
the ability to persist these permissions, but this PR is already quite
large - let's get the core flow working and go from there!

<img width="1279" height="541" alt="Screenshot 2026-02-15 at 2 26 22 PM"
src="https://github.com/user-attachments/assets/0ee3ec0f-02ec-4509-91a2-809ac80be368"
/>

## Testing
- [x] Added tests
- [x] Tested locally
- [x] Feature
This commit is contained in:
Dylan Hurd
2026-02-24 09:48:57 -08:00
committed by GitHub
Unverified
parent 9a8adbf6e5
commit f6053fdfb3
43 changed files with 1471 additions and 77 deletions
@@ -5,6 +5,23 @@
"description": "A path that is guaranteed to be absolute and normalized (though it is not guaranteed to be canonicalized or exist on the filesystem).\n\nIMPORTANT: When deserializing an `AbsolutePathBuf`, a base path must be set using [AbsolutePathBufGuard::new]. If no base path is set, the deserialization will fail unless the path being deserialized is already absolute.",
"type": "string"
},
"AdditionalPermissions": {
"properties": {
"fs_read": {
"items": {
"type": "string"
},
"type": "array"
},
"fs_write": {
"items": {
"type": "string"
},
"type": "array"
}
},
"type": "object"
},
"AgentMessageContent": {
"oneOf": [
{
@@ -1613,6 +1630,17 @@
},
{
"properties": {
"additional_permissions": {
"anyOf": [
{
"$ref": "#/definitions/AdditionalPermissions"
},
{
"type": "null"
}
],
"description": "Optional additional filesystem permissions requested for this command."
},
"approval_id": {
"description": "Identifier for this specific approval callback.\n\nWhen absent, the approval is for the command item itself (`call_id`). This is present for subcommand approvals (via execve intercept).",
"type": [
@@ -6914,6 +6942,17 @@
},
{
"properties": {
"additional_permissions": {
"anyOf": [
{
"$ref": "#/definitions/AdditionalPermissions"
},
{
"type": "null"
}
],
"description": "Optional additional filesystem permissions requested for this command."
},
"approval_id": {
"description": "Identifier for this specific approval callback.\n\nWhen absent, the approval is for the command item itself (`call_id`). This is present for subcommand approvals (via execve intercept).",
"type": [
@@ -1,6 +1,23 @@
{
"$schema": "http://json-schema.org/draft-07/schema#",
"definitions": {
"AdditionalPermissions": {
"properties": {
"fs_read": {
"items": {
"type": "string"
},
"type": "array"
},
"fs_write": {
"items": {
"type": "string"
},
"type": "array"
}
},
"type": "object"
},
"AgentMessageContent": {
"oneOf": [
{
@@ -2674,6 +2691,17 @@
},
{
"properties": {
"additional_permissions": {
"anyOf": [
{
"$ref": "#/definitions/AdditionalPermissions"
},
{
"type": "null"
}
],
"description": "Optional additional filesystem permissions requested for this command."
},
"approval_id": {
"description": "Identifier for this specific approval callback.\n\nWhen absent, the approval is for the command item itself (`call_id`). This is present for subcommand approvals (via execve intercept).",
"type": [
@@ -0,0 +1,5 @@
// GENERATED CODE! DO NOT MODIFY BY HAND!
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.
export type AdditionalPermissions = { fs_read?: Array<string>, fs_write?: Array<string>, };
@@ -1,6 +1,7 @@
// GENERATED CODE! DO NOT MODIFY BY HAND!
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.
import type { AdditionalPermissions } from "./AdditionalPermissions";
import type { ExecPolicyAmendment } from "./ExecPolicyAmendment";
import type { NetworkApprovalContext } from "./NetworkApprovalContext";
import type { NetworkPolicyAmendment } from "./NetworkPolicyAmendment";
@@ -46,4 +47,8 @@ proposed_execpolicy_amendment?: ExecPolicyAmendment,
/**
* Proposed network policy amendments (for example allow/deny this host in future).
*/
proposed_network_policy_amendments?: Array<NetworkPolicyAmendment>, parsed_cmd: Array<ParsedCommand>, };
proposed_network_policy_amendments?: Array<NetworkPolicyAmendment>,
/**
* Optional additional filesystem permissions requested for this command.
*/
additional_permissions?: AdditionalPermissions, parsed_cmd: Array<ParsedCommand>, };
@@ -3,6 +3,7 @@
export type { AbsolutePathBuf } from "./AbsolutePathBuf";
export type { AddConversationListenerParams } from "./AddConversationListenerParams";
export type { AddConversationSubscriptionResponse } from "./AddConversationSubscriptionResponse";
export type { AdditionalPermissions } from "./AdditionalPermissions";
export type { AgentMessageContent } from "./AgentMessageContent";
export type { AgentMessageContentDeltaEvent } from "./AgentMessageContentDeltaEvent";
export type { AgentMessageDeltaEvent } from "./AgentMessageDeltaEvent";
@@ -478,6 +478,7 @@ fn assert_permissions_message(item: &ResponseItem) {
AskForApproval::Never,
&Policy::empty(),
&PathBuf::from("/tmp"),
false,
)
.into_text();
assert_eq!(
+6
View File
@@ -370,6 +370,9 @@
"remote_models": {
"type": "boolean"
},
"request_permissions": {
"type": "boolean"
},
"request_rule": {
"type": "boolean"
},
@@ -1657,6 +1660,9 @@
"remote_models": {
"type": "boolean"
},
"request_permissions": {
"type": "boolean"
},
"request_rule": {
"type": "boolean"
},
+4
View File
@@ -70,6 +70,7 @@ use codex_protocol::items::PlanItem;
use codex_protocol::items::TurnItem;
use codex_protocol::items::UserMessageItem;
use codex_protocol::mcp::CallToolResult;
use codex_protocol::models::AdditionalPermissions;
use codex_protocol::models::BaseInstructions;
use codex_protocol::models::format_allow_prefixes;
use codex_protocol::openai_models::ModelInfo;
@@ -2573,6 +2574,7 @@ impl Session {
reason: Option<String>,
network_approval_context: Option<NetworkApprovalContext>,
proposed_execpolicy_amendment: Option<ExecPolicyAmendment>,
additional_permissions: Option<AdditionalPermissions>,
) -> ReviewDecision {
// command-level approvals use `call_id`.
// `approval_id` is only present for subcommand callbacks (execve intercept)
@@ -2616,6 +2618,7 @@ impl Session {
network_approval_context,
proposed_execpolicy_amendment,
proposed_network_policy_amendments,
additional_permissions,
parsed_cmd,
});
self.send_event(turn_context, event).await;
@@ -2999,6 +3002,7 @@ impl Session {
turn_context.approval_policy.value(),
self.services.exec_policy.current().as_ref(),
&turn_context.cwd,
turn_context.features.enabled(Feature::RequestPermissions),
)
.into(),
);
+2
View File
@@ -320,6 +320,7 @@ async fn handle_exec_approval(
reason,
network_approval_context,
proposed_execpolicy_amendment,
additional_permissions,
..
} = event;
// Race approval with cancellation and timeout to avoid hangs.
@@ -332,6 +333,7 @@ async fn handle_exec_approval(
reason,
network_approval_context,
proposed_execpolicy_amendment,
additional_permissions,
);
let decision = await_approval_with_cancel(
approval_fut,
@@ -1,5 +1,6 @@
use crate::codex::TurnContext;
use crate::environment_context::EnvironmentContext;
use crate::features::Feature;
use crate::shell::Shell;
use codex_execpolicy::Policy;
use codex_protocol::config_types::Personality;
@@ -43,6 +44,7 @@ fn build_permissions_update_item(
next.approval_policy.value(),
exec_policy,
&next.cwd,
next.features.enabled(Feature::RequestPermissions),
)
.into(),
)
+6 -1
View File
@@ -206,6 +206,7 @@ pub async fn process_exec_tool_call(
env,
expiration,
sandbox_permissions,
additional_permissions: None,
justification,
};
@@ -225,7 +226,7 @@ pub async fn process_exec_tool_call(
.map_err(CodexErr::from)?;
// Route through the sandboxing module for a single, unified execution path.
crate::sandboxing::execute_env(exec_req, sandbox_policy, stdout_stream).await
crate::sandboxing::execute_env(exec_req, stdout_stream).await
}
pub(crate) async fn execute_exec_env(
@@ -242,6 +243,7 @@ pub(crate) async fn execute_exec_env(
sandbox,
windows_sandbox_level,
sandbox_permissions,
sandbox_policy: _sandbox_policy_from_env,
justification,
arg0,
} = env;
@@ -517,6 +519,9 @@ pub(crate) mod errors {
SandboxTransformError::SeatbeltUnavailable => CodexErr::UnsupportedOperation(
"seatbelt sandbox is only available on macOS".to_string(),
),
SandboxTransformError::InvalidAdditionalPermissionsPath(path) => {
CodexErr::InvalidRequest(format!("invalid additional_permissions path: {path}"))
}
}
}
}
+2 -2
View File
@@ -539,7 +539,7 @@ pub fn render_decision_for_unmatched_command(
// In restricted sandboxes (ReadOnly/WorkspaceWrite), do not prompt for
// nonescalated, nondangerous commands — let the sandbox enforce
// restrictions (e.g., block network/write) without a user prompt.
if sandbox_permissions.requires_escalated_permissions() {
if sandbox_permissions.requires_additional_permissions() {
Decision::Prompt
} else {
Decision::Allow
@@ -554,7 +554,7 @@ pub fn render_decision_for_unmatched_command(
Decision::Allow
}
SandboxPolicy::ReadOnly { .. } | SandboxPolicy::WorkspaceWrite { .. } => {
if sandbox_permissions.requires_escalated_permissions() {
if sandbox_permissions.requires_additional_permissions() {
Decision::Prompt
} else {
Decision::Allow
+8
View File
@@ -88,6 +88,8 @@ pub enum Feature {
ShellZshFork,
/// Include the freeform apply_patch tool.
ApplyPatchFreeform,
/// Allow requesting additional filesystem permissions while staying sandboxed.
RequestPermissions,
/// Allow the model to request web searches that fetch live content.
WebSearchRequest,
/// Allow the model to request web searches that fetch cached content.
@@ -520,6 +522,12 @@ pub const FEATURES: &[FeatureSpec] = &[
stage: Stage::UnderDevelopment,
default_enabled: false,
},
FeatureSpec {
id: Feature::RequestPermissions,
key: "request_permissions",
stage: Stage::UnderDevelopment,
default_enabled: false,
},
FeatureSpec {
id: Feature::UseLinuxSandboxBwrap,
key: "use_linux_sandbox_bwrap",
+190 -5
View File
@@ -24,8 +24,12 @@ use crate::spawn::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR;
use crate::tools::sandboxing::SandboxablePreference;
use codex_network_proxy::NetworkProxy;
use codex_protocol::config_types::WindowsSandboxLevel;
use codex_protocol::models::AdditionalPermissions;
pub use codex_protocol::models::SandboxPermissions;
use codex_protocol::protocol::ReadOnlyAccess;
use codex_utils_absolute_path::AbsolutePathBuf;
use std::collections::HashMap;
use std::collections::HashSet;
use std::path::Path;
use std::path::PathBuf;
@@ -37,6 +41,7 @@ pub struct CommandSpec {
pub env: HashMap<String, String>,
pub expiration: ExecExpiration,
pub sandbox_permissions: SandboxPermissions,
pub additional_permissions: Option<AdditionalPermissions>,
pub justification: Option<String>,
}
@@ -50,6 +55,7 @@ pub struct ExecRequest {
pub sandbox: SandboxType,
pub windows_sandbox_level: WindowsSandboxLevel,
pub sandbox_permissions: SandboxPermissions,
pub sandbox_policy: SandboxPolicy,
pub justification: Option<String>,
pub arg0: Option<String>,
}
@@ -81,11 +87,183 @@ pub enum SandboxPreference {
pub(crate) enum SandboxTransformError {
#[error("missing codex-linux-sandbox executable path")]
MissingLinuxSandboxExecutable,
#[error("invalid additional permissions path: {0}")]
InvalidAdditionalPermissionsPath(String),
#[cfg(not(target_os = "macos"))]
#[error("seatbelt sandbox is only available on macOS")]
SeatbeltUnavailable,
}
pub(crate) fn normalize_additional_permissions(
additional_permissions: AdditionalPermissions,
command_cwd: &Path,
) -> Result<AdditionalPermissions, String> {
let fs_read =
normalize_permission_paths(additional_permissions.fs_read, command_cwd, "fs_read")?;
let fs_write =
normalize_permission_paths(additional_permissions.fs_write, command_cwd, "fs_write")?;
Ok(AdditionalPermissions { fs_read, fs_write })
}
fn normalize_permission_paths(
paths: Vec<PathBuf>,
command_cwd: &Path,
permission_kind: &str,
) -> Result<Vec<PathBuf>, String> {
let mut out = Vec::with_capacity(paths.len());
let mut seen = HashSet::new();
for path in paths {
if path.as_os_str().is_empty() {
return Err(format!("{permission_kind} contains an empty path"));
}
let resolved = if path.is_absolute() {
AbsolutePathBuf::from_absolute_path(path.clone()).map_err(|err| {
format!(
"{permission_kind} path `{}` is invalid: {err}",
path.display()
)
})?
} else {
AbsolutePathBuf::resolve_path_against_base(&path, command_cwd).map_err(|err| {
format!(
"{permission_kind} path `{}` cannot be resolved against cwd `{}`: {err}",
path.display(),
command_cwd.display()
)
})?
};
let canonicalized = resolved
.as_path()
.canonicalize()
.ok()
.and_then(|path| AbsolutePathBuf::from_absolute_path(path).ok())
.unwrap_or(resolved);
let canonicalized = canonicalized.to_path_buf();
if seen.insert(canonicalized.clone()) {
out.push(canonicalized);
}
}
Ok(out)
}
fn dedup_absolute_paths(paths: Vec<AbsolutePathBuf>) -> Vec<AbsolutePathBuf> {
let mut out = Vec::with_capacity(paths.len());
let mut seen = HashSet::new();
for path in paths {
if seen.insert(path.to_path_buf()) {
out.push(path);
}
}
out
}
fn additional_permission_roots(
additional_permissions: &AdditionalPermissions,
) -> Result<(Vec<AbsolutePathBuf>, Vec<AbsolutePathBuf>), SandboxTransformError> {
let to_abs = |paths: &[PathBuf]| {
let mut out = Vec::with_capacity(paths.len());
for path in paths {
let absolute = AbsolutePathBuf::from_absolute_path(path.clone()).map_err(|err| {
SandboxTransformError::InvalidAdditionalPermissionsPath(format!(
"`{}`: {err}",
path.display()
))
})?;
out.push(absolute);
}
Ok(dedup_absolute_paths(out))
};
Ok((
to_abs(&additional_permissions.fs_read)?,
to_abs(&additional_permissions.fs_write)?,
))
}
fn merge_read_only_access_with_additional_reads(
read_only_access: &ReadOnlyAccess,
extra_reads: Vec<AbsolutePathBuf>,
) -> ReadOnlyAccess {
match read_only_access {
ReadOnlyAccess::FullAccess => ReadOnlyAccess::FullAccess,
ReadOnlyAccess::Restricted {
include_platform_defaults,
readable_roots,
} => {
let mut merged = readable_roots.clone();
merged.extend(extra_reads);
ReadOnlyAccess::Restricted {
include_platform_defaults: *include_platform_defaults,
readable_roots: dedup_absolute_paths(merged),
}
}
}
}
fn sandbox_policy_with_additional_permissions(
sandbox_policy: &SandboxPolicy,
additional_permissions: &AdditionalPermissions,
) -> Result<SandboxPolicy, SandboxTransformError> {
if additional_permissions.is_empty() {
return Ok(sandbox_policy.clone());
}
let (extra_reads, extra_writes) = additional_permission_roots(additional_permissions)?;
let policy = match sandbox_policy {
SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } => {
sandbox_policy.clone()
}
SandboxPolicy::WorkspaceWrite {
writable_roots,
read_only_access,
network_access,
exclude_tmpdir_env_var,
exclude_slash_tmp,
} => {
let mut merged_writes = writable_roots.clone();
merged_writes.extend(extra_writes);
SandboxPolicy::WorkspaceWrite {
writable_roots: dedup_absolute_paths(merged_writes),
read_only_access: merge_read_only_access_with_additional_reads(
read_only_access,
extra_reads,
),
network_access: *network_access,
exclude_tmpdir_env_var: *exclude_tmpdir_env_var,
exclude_slash_tmp: *exclude_slash_tmp,
}
}
SandboxPolicy::ReadOnly { access } => {
if extra_writes.is_empty() {
SandboxPolicy::ReadOnly {
access: merge_read_only_access_with_additional_reads(access, extra_reads),
}
} else {
// todo(dylan) - for now, this grants more access than the request. We should restrict this,
// but we should add a new SandboxPolicy variant to handle this. While the feature is still
// UnderDevelopment, it's a useful approximation of the desired behavior.
SandboxPolicy::WorkspaceWrite {
writable_roots: dedup_absolute_paths(extra_writes),
read_only_access: merge_read_only_access_with_additional_reads(
access,
extra_reads,
),
network_access: false,
exclude_tmpdir_env_var: false,
exclude_slash_tmp: false,
}
}
}
};
Ok(policy)
}
#[derive(Default)]
pub struct SandboxManager;
@@ -145,8 +323,14 @@ impl SandboxManager {
use_linux_sandbox_bwrap,
windows_sandbox_level,
} = request;
let effective_policy =
if let Some(additional_permissions) = spec.additional_permissions.take() {
sandbox_policy_with_additional_permissions(policy, &additional_permissions)?
} else {
policy.clone()
};
let mut env = spec.env;
if !policy.has_full_network_access() {
if !effective_policy.has_full_network_access() {
env.insert(
CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR.to_string(),
"1".to_string(),
@@ -171,7 +355,7 @@ impl SandboxManager {
.map_or_else(Vec::new, |path| vec![path.clone()]);
let mut args = create_seatbelt_command_args(
command.clone(),
policy,
&effective_policy,
sandbox_policy_cwd,
enforce_managed_network,
network,
@@ -190,7 +374,7 @@ impl SandboxManager {
let allow_proxy_network = allow_network_for_proxy(enforce_managed_network);
let mut args = create_linux_sandbox_command_args(
command.clone(),
policy,
&effective_policy,
sandbox_policy_cwd,
use_linux_sandbox_bwrap,
allow_proxy_network,
@@ -225,6 +409,7 @@ impl SandboxManager {
sandbox,
windows_sandbox_level,
sandbox_permissions: spec.sandbox_permissions,
sandbox_policy: effective_policy,
justification: spec.justification,
arg0: arg0_override,
})
@@ -237,10 +422,10 @@ impl SandboxManager {
pub async fn execute_env(
env: ExecRequest,
policy: &SandboxPolicy,
stdout_stream: Option<StdoutStream>,
) -> crate::error::Result<ExecToolCallOutput> {
execute_exec_env(env, policy, stdout_stream).await
let effective_policy = env.sandbox_policy.clone();
execute_exec_env(env, &effective_policy, stdout_stream).await
}
#[cfg(test)]
+2 -1
View File
@@ -147,6 +147,7 @@ pub(crate) async fn execute_user_shell_command(
)
.await;
let sandbox_policy = SandboxPolicy::DangerFullAccess;
let exec_env = ExecRequest {
command: exec_command.clone(),
cwd: cwd.clone(),
@@ -161,6 +162,7 @@ pub(crate) async fn execute_user_shell_command(
sandbox: SandboxType::None,
windows_sandbox_level: turn_context.windows_sandbox_level,
sandbox_permissions: SandboxPermissions::UseDefault,
sandbox_policy: sandbox_policy.clone(),
justification: None,
arg0: None,
};
@@ -171,7 +173,6 @@ pub(crate) async fn execute_user_shell_command(
tx_event: session.get_tx_event(),
});
let sandbox_policy = SandboxPolicy::DangerFullAccess;
let exec_result = execute_exec_env(exec_env, &sandbox_policy, stdout_stream)
.or_cancel(&cancellation_token)
.await;
+60
View File
@@ -17,9 +17,14 @@ mod view_image;
pub use plan::PLAN_TOOL;
use serde::Deserialize;
use std::path::Path;
use crate::function_tool::FunctionCallError;
use crate::sandboxing::SandboxPermissions;
use crate::sandboxing::normalize_additional_permissions;
pub use apply_patch::ApplyPatchHandler;
use codex_protocol::models::AdditionalPermissions;
use codex_protocol::protocol::AskForApproval;
pub use dynamic::DynamicToolHandler;
pub use grep_files::GrepFilesHandler;
pub use js_repl::JsReplHandler;
@@ -49,3 +54,58 @@ where
FunctionCallError::RespondToModel(format!("failed to parse function arguments: {err}"))
})
}
/// Validates feature/policy constraints for `with_additional_permissions` and
/// returns normalized absolute paths. Errors if paths are invalid.
pub(super) fn normalize_and_validate_additional_permissions(
request_permission_enabled: bool,
approval_policy: AskForApproval,
sandbox_permissions: SandboxPermissions,
additional_permissions: Option<AdditionalPermissions>,
cwd: &Path,
) -> Result<Option<AdditionalPermissions>, String> {
let uses_additional_permissions = matches!(
sandbox_permissions,
SandboxPermissions::WithAdditionalPermissions
);
if !request_permission_enabled
&& (uses_additional_permissions || additional_permissions.is_some())
{
return Err(
"additional permissions are disabled; enable `features.request_permission` before using `with_additional_permissions`"
.to_string(),
);
}
if uses_additional_permissions {
if !matches!(approval_policy, AskForApproval::OnRequest) {
return Err(format!(
"approval policy is {approval_policy:?}; reject command — you cannot request additional permissions unless the approval policy is OnRequest"
));
}
let Some(additional_permissions) = additional_permissions else {
return Err(
"missing `additional_permissions`; provide `fs_read` and/or `fs_write` when using `with_additional_permissions`"
.to_string(),
);
};
let normalized = normalize_additional_permissions(additional_permissions, cwd)?;
if normalized.is_empty() {
return Err(
"`additional_permissions` must include at least one path in `fs_read` or `fs_write`"
.to_string(),
);
}
return Ok(Some(normalized));
}
if additional_permissions.is_some() {
Err(
"`additional_permissions` requires `sandbox_permissions` set to `with_additional_permissions`"
.to_string(),
)
} else {
Ok(None)
}
}
+22 -1
View File
@@ -9,6 +9,7 @@ use crate::codex::TurnContext;
use crate::exec::ExecParams;
use crate::exec_env::create_env;
use crate::exec_policy::ExecApprovalRequest;
use crate::features::Feature;
use crate::function_tool::FunctionCallError;
use crate::is_safe_command::is_known_safe_command;
use crate::protocol::ExecCommandSource;
@@ -20,6 +21,7 @@ use crate::tools::context::ToolPayload;
use crate::tools::events::ToolEmitter;
use crate::tools::events::ToolEventCtx;
use crate::tools::handlers::apply_patch::intercept_apply_patch;
use crate::tools::handlers::normalize_and_validate_additional_permissions;
use crate::tools::handlers::parse_arguments;
use crate::tools::orchestrator::ToolOrchestrator;
use crate::tools::registry::ToolHandler;
@@ -27,6 +29,7 @@ use crate::tools::registry::ToolKind;
use crate::tools::runtimes::shell::ShellRequest;
use crate::tools::runtimes::shell::ShellRuntime;
use crate::tools::sandboxing::ToolCtx;
use codex_protocol::models::AdditionalPermissions;
pub struct ShellHandler;
@@ -35,6 +38,7 @@ pub struct ShellCommandHandler;
struct RunExecLikeArgs {
tool_name: String,
exec_params: ExecParams,
additional_permissions: Option<AdditionalPermissions>,
prefix_rule: Option<Vec<String>>,
session: Arc<crate::codex::Session>,
turn: Arc<TurnContext>,
@@ -150,6 +154,7 @@ impl ToolHandler for ShellHandler {
Self::run_exec_like(RunExecLikeArgs {
tool_name: tool_name.clone(),
exec_params,
additional_permissions: params.additional_permissions.clone(),
prefix_rule,
session,
turn,
@@ -165,6 +170,7 @@ impl ToolHandler for ShellHandler {
Self::run_exec_like(RunExecLikeArgs {
tool_name: tool_name.clone(),
exec_params,
additional_permissions: None,
prefix_rule: None,
session,
turn,
@@ -247,6 +253,7 @@ impl ToolHandler for ShellCommandHandler {
ShellHandler::run_exec_like(RunExecLikeArgs {
tool_name,
exec_params,
additional_permissions: params.additional_permissions.clone(),
prefix_rule,
session,
turn,
@@ -263,6 +270,7 @@ impl ShellHandler {
let RunExecLikeArgs {
tool_name,
exec_params,
additional_permissions,
prefix_rule,
session,
turn,
@@ -284,10 +292,20 @@ impl ShellHandler {
}
}
let request_permission_enabled = session.features().enabled(Feature::RequestPermissions);
let normalized_additional_permissions = normalize_and_validate_additional_permissions(
request_permission_enabled,
turn.approval_policy.value(),
exec_params.sandbox_permissions,
additional_permissions,
&exec_params.cwd,
)
.map_err(FunctionCallError::RespondToModel)?;
// Approval policy guard for explicit escalation in non-OnRequest modes.
if exec_params
.sandbox_permissions
.requires_escalated_permissions()
.requires_additional_permissions()
&& !matches!(
turn.approval_policy.value(),
codex_protocol::protocol::AskForApproval::OnRequest
@@ -345,6 +363,7 @@ impl ShellHandler {
explicit_env_overrides,
network: exec_params.network.clone(),
sandbox_permissions: exec_params.sandbox_permissions,
additional_permissions: normalized_additional_permissions,
justification: exec_params.justification.clone(),
exec_approval_requirement,
};
@@ -466,6 +485,7 @@ mod tests {
login,
timeout_ms,
sandbox_permissions: Some(sandbox_permissions),
additional_permissions: None,
prefix_rule: None,
justification: justification.clone(),
};
@@ -525,6 +545,7 @@ mod tests {
login: None,
timeout_ms: None,
sandbox_permissions: None,
additional_permissions: None,
prefix_rule: None,
justification: None,
};
@@ -1,3 +1,4 @@
use crate::features::Feature;
use crate::function_tool::FunctionCallError;
use crate::is_safe_command::is_known_safe_command;
use crate::protocol::EventMsg;
@@ -10,6 +11,7 @@ use crate::tools::context::ToolInvocation;
use crate::tools::context::ToolOutput;
use crate::tools::context::ToolPayload;
use crate::tools::handlers::apply_patch::intercept_apply_patch;
use crate::tools::handlers::normalize_and_validate_additional_permissions;
use crate::tools::handlers::parse_arguments;
use crate::tools::registry::ToolHandler;
use crate::tools::registry::ToolKind;
@@ -19,6 +21,7 @@ use crate::unified_exec::UnifiedExecProcessManager;
use crate::unified_exec::UnifiedExecResponse;
use crate::unified_exec::WriteStdinRequest;
use async_trait::async_trait;
use codex_protocol::models::AdditionalPermissions;
use codex_protocol::models::FunctionCallOutputBody;
use serde::Deserialize;
use std::path::PathBuf;
@@ -44,6 +47,8 @@ pub(crate) struct ExecCommandArgs {
#[serde(default)]
sandbox_permissions: SandboxPermissions,
#[serde(default)]
additional_permissions: Option<AdditionalPermissions>,
#[serde(default)]
justification: Option<String>,
#[serde(default)]
prefix_rule: Option<Vec<String>>,
@@ -153,12 +158,16 @@ impl ToolHandler for UnifiedExecHandler {
yield_time_ms,
max_output_tokens,
sandbox_permissions,
additional_permissions,
justification,
prefix_rule,
..
} = args;
if sandbox_permissions.requires_escalated_permissions()
let request_permission_enabled =
session.features().enabled(Feature::RequestPermissions);
if sandbox_permissions.requires_additional_permissions()
&& !matches!(
context.turn.approval_policy.value(),
codex_protocol::protocol::AskForApproval::OnRequest
@@ -175,6 +184,20 @@ impl ToolHandler for UnifiedExecHandler {
let workdir = workdir.map(|dir| context.turn.resolve_path(Some(dir)));
let cwd = workdir.clone().unwrap_or_else(|| context.turn.cwd.clone());
let normalized_additional_permissions =
match normalize_and_validate_additional_permissions(
request_permission_enabled,
context.turn.approval_policy.value(),
sandbox_permissions,
additional_permissions,
&cwd,
) {
Ok(normalized) => normalized,
Err(err) => {
manager.release_process_id(&process_id).await;
return Err(FunctionCallError::RespondToModel(err));
}
};
if let Some(output) = intercept_apply_patch(
&command,
@@ -203,6 +226,7 @@ impl ToolHandler for UnifiedExecHandler {
network: context.turn.network.clone(),
tty,
sandbox_permissions,
additional_permissions: normalized_additional_permissions,
justification,
prefix_rule,
},
+1
View File
@@ -588,6 +588,7 @@ impl JsReplManager {
env,
expiration: ExecExpiration::DefaultTimeout,
sandbox_permissions: SandboxPermissions::UseDefault,
additional_permissions: None,
justification: None,
};
@@ -341,6 +341,7 @@ impl NetworkApprovalService {
Some(prompt_reason),
Some(network_approval_context.clone()),
None,
None,
)
.await;
+1
View File
@@ -124,6 +124,7 @@ impl ToolRouter {
workdir: exec.working_directory,
timeout_ms: exec.timeout_ms,
sandbox_permissions: Some(SandboxPermissions::UseDefault),
additional_permissions: None,
prefix_rule: None,
justification: None,
};
@@ -66,6 +66,7 @@ impl ApplyPatchRuntime {
// Run apply_patch with a minimal environment for determinism and to avoid leaks.
env: HashMap::new(),
sandbox_permissions: SandboxPermissions::UseDefault,
additional_permissions: None,
justification: None,
})
}
@@ -162,7 +163,7 @@ impl ToolRuntime<ApplyPatchRequest, ExecToolCallOutput> for ApplyPatchRuntime {
let env = attempt
.env_for(spec, None)
.map_err(|err| ToolError::Codex(err.into()))?;
let out = execute_env(env, attempt.policy, Self::stdout_stream(ctx))
let out = execute_env(env, Self::stdout_stream(ctx))
.await
.map_err(ToolError::Codex)?;
Ok(out)
+3
View File
@@ -10,6 +10,7 @@ use crate::sandboxing::CommandSpec;
use crate::sandboxing::SandboxPermissions;
use crate::shell::Shell;
use crate::tools::sandboxing::ToolError;
use codex_protocol::models::AdditionalPermissions;
use std::collections::HashMap;
use std::path::Path;
@@ -25,6 +26,7 @@ pub(crate) fn build_command_spec(
env: &HashMap<String, String>,
expiration: ExecExpiration,
sandbox_permissions: SandboxPermissions,
additional_permissions: Option<AdditionalPermissions>,
justification: Option<String>,
) -> Result<CommandSpec, ToolError> {
let (program, args) = command
@@ -37,6 +39,7 @@ pub(crate) fn build_command_spec(
env: env.clone(),
expiration,
sandbox_permissions,
additional_permissions,
justification,
})
}
+10 -14
View File
@@ -25,9 +25,11 @@ use crate::tools::sandboxing::SandboxablePreference;
use crate::tools::sandboxing::ToolCtx;
use crate::tools::sandboxing::ToolError;
use crate::tools::sandboxing::ToolRuntime;
use crate::tools::sandboxing::sandbox_override_for_first_attempt;
use crate::tools::sandboxing::with_cached_approval;
use crate::zsh_exec_bridge::ZSH_EXEC_BRIDGE_WRAPPER_SOCKET_ENV_VAR;
use codex_network_proxy::NetworkProxy;
use codex_protocol::models::AdditionalPermissions;
use codex_protocol::protocol::ReviewDecision;
use futures::future::BoxFuture;
use std::path::PathBuf;
@@ -41,6 +43,7 @@ pub struct ShellRequest {
pub explicit_env_overrides: std::collections::HashMap<String, String>,
pub network: Option<NetworkProxy>,
pub sandbox_permissions: SandboxPermissions,
pub additional_permissions: Option<AdditionalPermissions>,
pub justification: Option<String>,
pub exec_approval_requirement: ExecApprovalRequirement,
}
@@ -53,6 +56,7 @@ pub(crate) struct ApprovalKey {
command: Vec<String>,
cwd: PathBuf,
sandbox_permissions: SandboxPermissions,
additional_permissions: Option<AdditionalPermissions>,
}
impl ShellRuntime {
@@ -86,6 +90,7 @@ impl Approvable<ShellRequest> for ShellRuntime {
command: canonicalize_command_for_approval(&req.command),
cwd: req.cwd.clone(),
sandbox_permissions: req.sandbox_permissions,
additional_permissions: req.additional_permissions.clone(),
}]
}
@@ -118,6 +123,7 @@ impl Approvable<ShellRequest> for ShellRuntime {
req.exec_approval_requirement
.proposed_execpolicy_amendment()
.cloned(),
req.additional_permissions.clone(),
)
.await
})
@@ -130,19 +136,7 @@ impl Approvable<ShellRequest> for ShellRuntime {
}
fn sandbox_mode_for_first_attempt(&self, req: &ShellRequest) -> SandboxOverride {
if req.sandbox_permissions.requires_escalated_permissions()
|| matches!(
req.exec_approval_requirement,
ExecApprovalRequirement::Skip {
bypass_sandbox: true,
..
}
)
{
SandboxOverride::BypassSandboxFirstAttempt
} else {
SandboxOverride::NoOverride
}
sandbox_override_for_first_attempt(req.sandbox_permissions, &req.exec_approval_requirement)
}
}
@@ -198,6 +192,7 @@ impl ToolRuntime<ShellRequest, ExecToolCallOutput> for ShellRuntime {
&zsh_fork_env,
req.timeout_ms.into(),
req.sandbox_permissions,
req.additional_permissions.clone(),
req.justification.clone(),
)?;
let env = attempt
@@ -217,12 +212,13 @@ impl ToolRuntime<ShellRequest, ExecToolCallOutput> for ShellRuntime {
&req.env,
req.timeout_ms.into(),
req.sandbox_permissions,
req.additional_permissions.clone(),
req.justification.clone(),
)?;
let env = attempt
.env_for(spec, req.network.as_ref())
.map_err(|err| ToolError::Codex(err.into()))?;
let out = execute_env(env, attempt.policy, Self::stdout_stream(ctx))
let out = execute_env(env, Self::stdout_stream(ctx))
.await
.map_err(ToolError::Codex)?;
Ok(out)
@@ -26,11 +26,13 @@ use crate::tools::sandboxing::SandboxablePreference;
use crate::tools::sandboxing::ToolCtx;
use crate::tools::sandboxing::ToolError;
use crate::tools::sandboxing::ToolRuntime;
use crate::tools::sandboxing::sandbox_override_for_first_attempt;
use crate::tools::sandboxing::with_cached_approval;
use crate::unified_exec::UnifiedExecError;
use crate::unified_exec::UnifiedExecProcess;
use crate::unified_exec::UnifiedExecProcessManager;
use codex_network_proxy::NetworkProxy;
use codex_protocol::models::AdditionalPermissions;
use codex_protocol::protocol::ReviewDecision;
use futures::future::BoxFuture;
use std::collections::HashMap;
@@ -45,6 +47,7 @@ pub struct UnifiedExecRequest {
pub network: Option<NetworkProxy>,
pub tty: bool,
pub sandbox_permissions: SandboxPermissions,
pub additional_permissions: Option<AdditionalPermissions>,
pub justification: Option<String>,
pub exec_approval_requirement: ExecApprovalRequirement,
}
@@ -55,6 +58,7 @@ pub struct UnifiedExecApprovalKey {
pub cwd: PathBuf,
pub tty: bool,
pub sandbox_permissions: SandboxPermissions,
pub additional_permissions: Option<AdditionalPermissions>,
}
pub struct UnifiedExecRuntime<'a> {
@@ -86,6 +90,7 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
cwd: req.cwd.clone(),
tty: req.tty,
sandbox_permissions: req.sandbox_permissions,
additional_permissions: req.additional_permissions.clone(),
}]
}
@@ -118,6 +123,7 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
req.exec_approval_requirement
.proposed_execpolicy_amendment()
.cloned(),
req.additional_permissions.clone(),
)
.await
})
@@ -133,19 +139,7 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
}
fn sandbox_mode_for_first_attempt(&self, req: &UnifiedExecRequest) -> SandboxOverride {
if req.sandbox_permissions.requires_escalated_permissions()
|| matches!(
req.exec_approval_requirement,
ExecApprovalRequirement::Skip {
bypass_sandbox: true,
..
}
)
{
SandboxOverride::BypassSandboxFirstAttempt
} else {
SandboxOverride::NoOverride
}
sandbox_override_for_first_attempt(req.sandbox_permissions, &req.exec_approval_requirement)
}
}
@@ -194,6 +188,7 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
&env,
ExecExpiration::DefaultTimeout,
req.sandbox_permissions,
req.additional_permissions.clone(),
req.justification.clone(),
)
.map_err(|_| ToolError::Rejected("missing command line for PTY".to_string()))?;
+37
View File
@@ -10,6 +10,7 @@ use crate::error::CodexErr;
use crate::protocol::SandboxPolicy;
use crate::sandboxing::CommandSpec;
use crate::sandboxing::SandboxManager;
use crate::sandboxing::SandboxPermissions;
use crate::sandboxing::SandboxTransformError;
use crate::state::SessionServices;
use crate::tools::network_approval::NetworkApprovalSpec;
@@ -200,6 +201,27 @@ pub(crate) enum SandboxOverride {
BypassSandboxFirstAttempt,
}
pub(crate) fn sandbox_override_for_first_attempt(
sandbox_permissions: SandboxPermissions,
exec_approval_requirement: &ExecApprovalRequirement,
) -> SandboxOverride {
// ExecPolicy `Allow` can intentionally imply full trust (Skip + bypass_sandbox=true),
// which supersedes `with_additional_permissions` sandboxed execution hints.
if sandbox_permissions.requires_escalated_permissions()
|| matches!(
exec_approval_requirement,
ExecApprovalRequirement::Skip {
bypass_sandbox: true,
..
}
)
{
SandboxOverride::BypassSandboxFirstAttempt
} else {
SandboxOverride::NoOverride
}
}
pub(crate) trait Approvable<Req> {
type ApprovalKey: Hash + Eq + Clone + Debug + Serialize;
@@ -328,6 +350,7 @@ impl<'a> SandboxAttempt<'a> {
#[cfg(test)]
mod tests {
use super::*;
use crate::sandboxing::SandboxPermissions;
use codex_protocol::protocol::NetworkAccess;
use codex_protocol::protocol::RejectConfig;
use pretty_assertions::assert_eq;
@@ -400,4 +423,18 @@ mod tests {
}
);
}
#[test]
fn additional_permissions_allow_bypass_sandbox_first_attempt_when_execpolicy_skips() {
assert_eq!(
sandbox_override_for_first_attempt(
SandboxPermissions::WithAdditionalPermissions,
&ExecApprovalRequirement::Skip {
bypass_sandbox: true,
proposed_execpolicy_amendment: None,
},
),
SandboxOverride::BypassSandboxFirstAttempt
);
}
}
+92 -26
View File
@@ -40,6 +40,7 @@ pub(crate) struct ToolsConfig {
pub web_search_mode: Option<WebSearchMode>,
pub agent_roles: BTreeMap<String, AgentRoleConfig>,
pub search_tool: bool,
pub request_permission_enabled: bool,
pub js_repl_enabled: bool,
pub js_repl_tools_only: bool,
pub collab_tools: bool,
@@ -67,6 +68,7 @@ impl ToolsConfig {
let include_collab_tools = features.enabled(Feature::Collab);
let include_collaboration_modes_tools = true;
let include_search_tool = features.enabled(Feature::Apps);
let request_permission_enabled = features.enabled(Feature::RequestPermissions);
let shell_type = if !features.enabled(Feature::ShellTool) {
ConfigShellToolType::Disabled
@@ -102,6 +104,7 @@ impl ToolsConfig {
web_search_mode: *web_search_mode,
agent_roles: BTreeMap::new(),
search_tool: include_search_tool,
request_permission_enabled,
js_repl_enabled: include_js_repl,
js_repl_tools_only: include_js_repl_tools_only,
collab_tools: include_collab_tools,
@@ -177,14 +180,18 @@ impl From<JsonSchema> for AdditionalProperties {
}
}
fn create_approval_parameters() -> BTreeMap<String, JsonSchema> {
fn create_approval_parameters(request_permission_enabled: bool) -> BTreeMap<String, JsonSchema> {
let mut properties = BTreeMap::from([
(
"sandbox_permissions".to_string(),
JsonSchema::String {
description: Some(
"Sandbox permissions for the command. Set to \"require_escalated\" to request running without sandbox restrictions; defaults to \"use_default\"."
.to_string(),
if request_permission_enabled {
"Sandbox permissions for the command. Use \"with_additional_permissions\" to request additional sandboxed filesystem access (preferred), or \"require_escalated\" to request running without sandbox restrictions; defaults to \"use_default\"."
} else {
"Sandbox permissions for the command. Set to \"require_escalated\" to request running without sandbox restrictions; defaults to \"use_default\"."
}
.to_string(),
),
},
),
@@ -201,24 +208,55 @@ fn create_approval_parameters() -> BTreeMap<String, JsonSchema> {
),
},
),
(
"prefix_rule".to_string(),
JsonSchema::Array {
items: Box::new(JsonSchema::String { description: None }),
description: Some(
r#"Only specify when sandbox_permissions is `require_escalated`.
Suggest a prefix command pattern that will allow you to fulfill similar requests from the user in the future.
Should be a short but reasonable prefix, e.g. [\"git\", \"pull\"] or [\"uv\", \"run\"] or [\"pytest\"]."#.to_string(),
),
},
)
]);
properties.insert(
"prefix_rule".to_string(),
JsonSchema::Array {
items: Box::new(JsonSchema::String { description: None }),
description: Some(
r#"Only specify when sandbox_permissions is `require_escalated`.
Suggest a prefix command pattern that will allow you to fulfill similar requests from the user in the future.
Should be a short but reasonable prefix, e.g. [\"git\", \"pull\"] or [\"uv\", \"run\"] or [\"pytest\"]."#.to_string(),
),
},
);
if request_permission_enabled {
properties.insert(
"additional_permissions".to_string(),
JsonSchema::Object {
properties: BTreeMap::from([
(
"fs_read".to_string(),
JsonSchema::Array {
items: Box::new(JsonSchema::String { description: None }),
description: Some(
"Additional filesystem paths to grant read access for this command."
.to_string(),
),
},
),
(
"fs_write".to_string(),
JsonSchema::Array {
items: Box::new(JsonSchema::String { description: None }),
description: Some(
"Additional filesystem paths to grant write access for this command."
.to_string(),
),
},
),
]),
required: None,
additional_properties: Some(false.into()),
},
);
}
properties
}
fn create_exec_command_tool(allow_login_shell: bool) -> ToolSpec {
fn create_exec_command_tool(allow_login_shell: bool, request_permission_enabled: bool) -> ToolSpec {
let mut properties = BTreeMap::from([
(
"cmd".to_string(),
@@ -278,7 +316,7 @@ fn create_exec_command_tool(allow_login_shell: bool) -> ToolSpec {
},
);
}
properties.extend(create_approval_parameters());
properties.extend(create_approval_parameters(request_permission_enabled));
ToolSpec::Function(ResponsesApiTool {
name: "exec_command".to_string(),
@@ -341,7 +379,7 @@ fn create_write_stdin_tool() -> ToolSpec {
})
}
fn create_shell_tool() -> ToolSpec {
fn create_shell_tool(request_permission_enabled: bool) -> ToolSpec {
let mut properties = BTreeMap::from([
(
"command".to_string(),
@@ -363,7 +401,7 @@ fn create_shell_tool() -> ToolSpec {
},
),
]);
properties.extend(create_approval_parameters());
properties.extend(create_approval_parameters(request_permission_enabled));
let description = if cfg!(windows) {
r#"Runs a Powershell command (Windows) and returns its output. Arguments to `shell` will be passed to CreateProcessW(). Most commands should be prefixed with ["powershell.exe", "-Command"].
@@ -394,7 +432,10 @@ Examples of valid command strings:
})
}
fn create_shell_command_tool(allow_login_shell: bool) -> ToolSpec {
fn create_shell_command_tool(
allow_login_shell: bool,
request_permission_enabled: bool,
) -> ToolSpec {
let mut properties = BTreeMap::from([
(
"command".to_string(),
@@ -428,7 +469,7 @@ fn create_shell_command_tool(allow_login_shell: bool) -> ToolSpec {
},
);
}
properties.extend(create_approval_parameters());
properties.extend(create_approval_parameters(request_permission_enabled));
let description = if cfg!(windows) {
r#"Runs a Powershell command (Windows) and returns its output.
@@ -1464,17 +1505,21 @@ pub(crate) fn build_specs(
let search_tool_handler = Arc::new(SearchToolBm25Handler);
let js_repl_handler = Arc::new(JsReplHandler);
let js_repl_reset_handler = Arc::new(JsReplResetHandler);
let request_permission_enabled = config.request_permission_enabled;
match &config.shell_type {
ConfigShellToolType::Default => {
builder.push_spec_with_parallel_support(create_shell_tool(), true);
builder.push_spec_with_parallel_support(
create_shell_tool(request_permission_enabled),
true,
);
}
ConfigShellToolType::Local => {
builder.push_spec_with_parallel_support(ToolSpec::LocalShell {}, true);
}
ConfigShellToolType::UnifiedExec => {
builder.push_spec_with_parallel_support(
create_exec_command_tool(config.allow_login_shell),
create_exec_command_tool(config.allow_login_shell, request_permission_enabled),
true,
);
builder.push_spec(create_write_stdin_tool());
@@ -1486,7 +1531,7 @@ pub(crate) fn build_specs(
}
ConfigShellToolType::ShellCommand => {
builder.push_spec_with_parallel_support(
create_shell_command_tool(config.allow_login_shell),
create_shell_command_tool(config.allow_login_shell, request_permission_enabled),
true,
);
}
@@ -1833,7 +1878,7 @@ mod tests {
// Build expected from the same helpers used by the builder.
let mut expected: BTreeMap<String, ToolSpec> = BTreeMap::from([]);
for spec in [
create_exec_command_tool(true),
create_exec_command_tool(true, false),
create_write_stdin_tool(),
PLAN_TOOL.clone(),
create_request_user_input_tool(),
@@ -2749,7 +2794,7 @@ mod tests {
#[test]
fn test_shell_tool() {
let tool = super::create_shell_tool();
let tool = super::create_shell_tool(false);
let ToolSpec::Function(ResponsesApiTool {
description, name, ..
}) = &tool
@@ -2777,9 +2822,30 @@ Examples of valid command strings:
assert_eq!(description, &expected);
}
#[test]
fn shell_tool_with_request_permission_includes_additional_permissions() {
let tool = super::create_shell_tool(true);
let ToolSpec::Function(ResponsesApiTool { parameters, .. }) = tool else {
panic!("expected function tool");
};
let JsonSchema::Object { properties, .. } = parameters else {
panic!("expected object parameters");
};
assert!(properties.contains_key("additional_permissions"));
let Some(JsonSchema::String {
description: Some(description),
}) = properties.get("sandbox_permissions")
else {
panic!("expected sandbox_permissions description");
};
assert!(description.contains("with_additional_permissions"));
}
#[test]
fn test_shell_command_tool() {
let tool = super::create_shell_command_tool(true);
let tool = super::create_shell_command_tool(true, false);
let ToolSpec::Function(ResponsesApiTool {
description, name, ..
}) = &tool
+3
View File
@@ -29,6 +29,7 @@ use std::sync::Weak;
use std::time::Duration;
use codex_network_proxy::NetworkProxy;
use codex_protocol::models::AdditionalPermissions;
use rand::Rng;
use rand::rng;
use tokio::sync::Mutex;
@@ -89,6 +90,7 @@ pub(crate) struct ExecCommandRequest {
pub network: Option<NetworkProxy>,
pub tty: bool,
pub sandbox_permissions: SandboxPermissions,
pub additional_permissions: Option<AdditionalPermissions>,
pub justification: Option<String>,
pub prefix_rule: Option<Vec<String>>,
}
@@ -229,6 +231,7 @@ mod tests {
network: None,
tty: true,
sandbox_permissions: SandboxPermissions::UseDefault,
additional_permissions: None,
justification: None,
prefix_rule: None,
},
@@ -590,6 +590,7 @@ impl UnifiedExecProcessManager {
network: request.network.clone(),
tty: request.tty,
sandbox_permissions: request.sandbox_permissions,
additional_permissions: request.additional_permissions.clone(),
justification: request.justification.clone(),
exec_approval_requirement,
};
+1
View File
@@ -366,6 +366,7 @@ impl ZshExecBridge {
approval_reason,
None,
None::<ExecPolicyAmendment>,
None,
)
.await;
+2 -2
View File
@@ -211,7 +211,7 @@ fn shell_event_with_prefix_rule(
"command": command,
"timeout_ms": timeout_ms,
});
if sandbox_permissions.requires_escalated_permissions() {
if sandbox_permissions.requires_additional_permissions() {
args["sandbox_permissions"] = json!(sandbox_permissions);
}
if let Some(prefix_rule) = prefix_rule {
@@ -234,7 +234,7 @@ fn exec_command_event(
if let Some(yield_time_ms) = yield_time_ms {
args["yield_time_ms"] = json!(yield_time_ms);
}
if sandbox_permissions.requires_escalated_permissions() {
if sandbox_permissions.requires_additional_permissions() {
args["sandbox_permissions"] = json!(sandbox_permissions);
let reason = justification.unwrap_or(DEFAULT_UNIFIED_EXEC_JUSTIFICATION);
args["justification"] = json!(reason);
+2
View File
@@ -99,6 +99,8 @@ mod read_file;
mod realtime_conversation;
mod remote_models;
mod request_compression;
#[cfg(not(target_os = "windows"))]
mod request_permissions;
mod request_user_input;
mod resume;
mod resume_warning;
@@ -489,6 +489,7 @@ async fn permissions_message_includes_writable_roots() -> Result<()> {
AskForApproval::OnRequest,
&Policy::empty(),
test.config.cwd.as_path(),
false,
)
.into_text();
// Normalize line endings to handle Windows vs Unix differences
@@ -0,0 +1,595 @@
#![allow(clippy::unwrap_used, clippy::expect_used)]
use anyhow::Result;
use codex_core::config::Constrained;
use codex_core::features::Feature;
use codex_core::sandboxing::SandboxPermissions;
use codex_protocol::config_types::ReasoningSummary;
use codex_protocol::models::AdditionalPermissions;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::ExecApprovalRequestEvent;
use codex_protocol::protocol::Op;
use codex_protocol::protocol::ReviewDecision;
use codex_protocol::protocol::SandboxPolicy;
use codex_protocol::user_input::UserInput;
use core_test_support::responses::ev_assistant_message;
use core_test_support::responses::ev_completed;
use core_test_support::responses::ev_function_call;
use core_test_support::responses::ev_response_created;
use core_test_support::responses::mount_sse_once;
use core_test_support::responses::sse;
use core_test_support::responses::start_mock_server;
use core_test_support::skip_if_no_network;
#[cfg(target_os = "macos")]
use core_test_support::skip_if_sandbox;
use core_test_support::test_codex::TestCodex;
use core_test_support::test_codex::test_codex;
use core_test_support::wait_for_event;
use pretty_assertions::assert_eq;
use regex_lite::Regex;
use serde_json::Value;
use serde_json::json;
use std::fs;
struct CommandResult {
exit_code: Option<i64>,
stdout: String,
}
fn parse_result(item: &Value) -> CommandResult {
let output_str = item
.get("output")
.and_then(Value::as_str)
.expect("shell output payload");
match serde_json::from_str::<Value>(output_str) {
Ok(parsed) => {
let exit_code = parsed["metadata"]["exit_code"].as_i64();
let stdout = parsed["output"].as_str().unwrap_or_default().to_string();
CommandResult { exit_code, stdout }
}
Err(_) => {
let structured = Regex::new(r"(?s)^Exit code:\s*(-?\d+).*?Output:\n(.*)$").unwrap();
let regex =
Regex::new(r"(?s)^.*?Process exited with code (\d+)\n.*?Output:\n(.*)$").unwrap();
if let Some(captures) = structured.captures(output_str) {
let exit_code = captures.get(1).unwrap().as_str().parse::<i64>().unwrap();
let output = captures.get(2).unwrap().as_str();
CommandResult {
exit_code: Some(exit_code),
stdout: output.to_string(),
}
} else if let Some(captures) = regex.captures(output_str) {
let exit_code = captures.get(1).unwrap().as_str().parse::<i64>().unwrap();
let output = captures.get(2).unwrap().as_str();
CommandResult {
exit_code: Some(exit_code),
stdout: output.to_string(),
}
} else {
CommandResult {
exit_code: None,
stdout: output_str.to_string(),
}
}
}
}
}
fn shell_event_with_request_permissions(
call_id: &str,
command: &str,
additional_permissions: &AdditionalPermissions,
) -> Result<Value> {
let args = json!({
"command": command,
"timeout_ms": 1_000_u64,
"sandbox_permissions": SandboxPermissions::WithAdditionalPermissions,
"additional_permissions": additional_permissions,
});
let args_str = serde_json::to_string(&args)?;
Ok(ev_function_call(call_id, "shell_command", &args_str))
}
async fn submit_turn(
test: &TestCodex,
prompt: &str,
approval_policy: AskForApproval,
sandbox_policy: SandboxPolicy,
) -> Result<()> {
let session_model = test.session_configured.model.clone();
test.codex
.submit(Op::UserTurn {
items: vec![UserInput::Text {
text: prompt.into(),
text_elements: Vec::new(),
}],
final_output_json_schema: None,
cwd: test.cwd.path().to_path_buf(),
approval_policy,
sandbox_policy,
model: session_model,
effort: None,
summary: ReasoningSummary::Auto,
collaboration_mode: None,
personality: None,
})
.await?;
Ok(())
}
async fn wait_for_completion(test: &TestCodex) {
wait_for_event(&test.codex, |event| {
matches!(event, EventMsg::TurnComplete(_))
})
.await;
}
async fn expect_exec_approval(
test: &TestCodex,
expected_command: &str,
) -> ExecApprovalRequestEvent {
let event = wait_for_event(&test.codex, |event| {
matches!(
event,
EventMsg::ExecApprovalRequest(_) | EventMsg::TurnComplete(_)
)
})
.await;
match event {
EventMsg::ExecApprovalRequest(approval) => {
let last_arg = approval
.command
.last()
.map(String::as_str)
.unwrap_or_default();
assert_eq!(last_arg, expected_command);
approval
}
EventMsg::TurnComplete(_) => panic!("expected approval request before completion"),
other => panic!("unexpected event: {other:?}"),
}
}
fn workspace_write_excluding_tmp() -> SandboxPolicy {
SandboxPolicy::WorkspaceWrite {
writable_roots: vec![],
read_only_access: Default::default(),
network_access: false,
exclude_tmpdir_env_var: true,
exclude_slash_tmp: true,
}
}
#[tokio::test(flavor = "current_thread")]
#[cfg(target_os = "macos")]
async fn with_additional_permissions_requires_approval_under_on_request() -> Result<()> {
skip_if_no_network!(Ok(()));
skip_if_sandbox!(Ok(()));
let server = start_mock_server().await;
let approval_policy = AskForApproval::OnRequest;
let sandbox_policy = SandboxPolicy::new_read_only_policy();
let sandbox_policy_for_config = sandbox_policy.clone();
let mut builder = test_codex().with_config(move |config| {
config.permissions.approval_policy = Constrained::allow_any(approval_policy);
config.permissions.sandbox_policy = Constrained::allow_any(sandbox_policy_for_config);
config.features.enable(Feature::RequestPermissions);
});
let test = builder.build(&server).await?;
let requested_write = test.workspace_path("requested-but-unused.txt");
let _ = fs::remove_file(&requested_write);
let call_id = "request_permissions_skip_approval";
let command = "touch requested-but-unused.txt";
let requested_permissions = AdditionalPermissions {
fs_read: vec![],
fs_write: vec![requested_write.clone()],
};
let event = shell_event_with_request_permissions(call_id, command, &requested_permissions)?;
let _ = mount_sse_once(
&server,
sse(vec![
ev_response_created("resp-1"),
event,
ev_completed("resp-1"),
]),
)
.await;
let results = mount_sse_once(
&server,
sse(vec![
ev_assistant_message("msg-1", "done"),
ev_completed("resp-2"),
]),
)
.await;
submit_turn(&test, call_id, approval_policy, sandbox_policy.clone()).await?;
let approval = expect_exec_approval(&test, command).await;
assert_eq!(
approval.additional_permissions,
Some(requested_permissions.clone())
);
test.codex
.submit(Op::ExecApproval {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Approved,
})
.await?;
wait_for_completion(&test).await;
let result = parse_result(&results.single_request().function_call_output(call_id));
assert!(
result.exit_code.is_none() || result.exit_code == Some(0),
"unexpected exit code/output: {:?} {}",
result.exit_code,
result.stdout
);
assert!(
requested_write.exists(),
"touch command should create requested path"
);
Ok(())
}
#[tokio::test(flavor = "current_thread")]
#[cfg(target_os = "macos")]
async fn read_only_with_additional_permissions_widens_to_unrequested_cwd_write() -> Result<()> {
skip_if_no_network!(Ok(()));
skip_if_sandbox!(Ok(()));
let server = start_mock_server().await;
let approval_policy = AskForApproval::OnRequest;
let sandbox_policy = SandboxPolicy::new_read_only_policy();
let sandbox_policy_for_config = sandbox_policy.clone();
let mut builder = test_codex().with_config(move |config| {
config.permissions.approval_policy = Constrained::allow_any(approval_policy);
config.permissions.sandbox_policy = Constrained::allow_any(sandbox_policy_for_config);
config.features.enable(Feature::RequestPermissions);
});
let test = builder.build(&server).await?;
let requested_write = test.workspace_path("requested-only-cwd.txt");
let unrequested_write = test.workspace_path("unrequested-cwd-write.txt");
let _ = fs::remove_file(&requested_write);
let _ = fs::remove_file(&unrequested_write);
let call_id = "request_permissions_cwd_widening";
let command = format!(
"printf {:?} > {:?} && cat {:?}",
"cwd-widened", unrequested_write, unrequested_write
);
let requested_permissions = AdditionalPermissions {
fs_read: vec![],
fs_write: vec![requested_write.clone()],
};
let event = shell_event_with_request_permissions(call_id, &command, &requested_permissions)?;
let _ = mount_sse_once(
&server,
sse(vec![
ev_response_created("resp-cwd-1"),
event,
ev_completed("resp-cwd-1"),
]),
)
.await;
let results = mount_sse_once(
&server,
sse(vec![
ev_assistant_message("msg-cwd-1", "done"),
ev_completed("resp-cwd-2"),
]),
)
.await;
submit_turn(&test, call_id, approval_policy, sandbox_policy.clone()).await?;
let approval = expect_exec_approval(&test, &command).await;
assert_eq!(
approval.additional_permissions,
Some(requested_permissions.clone())
);
test.codex
.submit(Op::ExecApproval {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Approved,
})
.await?;
wait_for_completion(&test).await;
let result = parse_result(&results.single_request().function_call_output(call_id));
assert!(
result.exit_code.is_none() || result.exit_code == Some(0),
"unexpected exit code/output: {:?} {}",
result.exit_code,
result.stdout
);
assert!(result.stdout.contains("cwd-widened"));
assert_eq!(fs::read_to_string(&unrequested_write)?, "cwd-widened");
assert!(
!requested_write.exists(),
"only the unrequested cwd path should have been written"
);
let _ = fs::remove_file(unrequested_write);
let _ = fs::remove_file(requested_write);
Ok(())
}
#[tokio::test(flavor = "current_thread")]
#[cfg(target_os = "macos")]
async fn read_only_with_additional_permissions_widens_to_unrequested_tmp_write() -> Result<()> {
skip_if_no_network!(Ok(()));
skip_if_sandbox!(Ok(()));
let server = start_mock_server().await;
let approval_policy = AskForApproval::OnRequest;
let sandbox_policy = SandboxPolicy::new_read_only_policy();
let sandbox_policy_for_config = sandbox_policy.clone();
let mut builder = test_codex().with_config(move |config| {
config.permissions.approval_policy = Constrained::allow_any(approval_policy);
config.permissions.sandbox_policy = Constrained::allow_any(sandbox_policy_for_config);
config.features.enable(Feature::RequestPermissions);
});
let test = builder.build(&server).await?;
let requested_write = test.workspace_path("requested-only-tmp.txt");
let tmp_dir = tempfile::tempdir()?;
let tmp_write = tmp_dir.path().join("tmp-widening.txt");
let _ = fs::remove_file(&requested_write);
let _ = fs::remove_file(&tmp_write);
let call_id = "request_permissions_tmp_widening";
let command = format!(
"printf {:?} > {:?} && cat {:?}",
"tmp-widened", tmp_write, tmp_write
);
let requested_permissions = AdditionalPermissions {
fs_read: vec![],
fs_write: vec![requested_write.clone()],
};
let event = shell_event_with_request_permissions(call_id, &command, &requested_permissions)?;
let _ = mount_sse_once(
&server,
sse(vec![
ev_response_created("resp-tmp-1"),
event,
ev_completed("resp-tmp-1"),
]),
)
.await;
let results = mount_sse_once(
&server,
sse(vec![
ev_assistant_message("msg-tmp-1", "done"),
ev_completed("resp-tmp-2"),
]),
)
.await;
submit_turn(&test, call_id, approval_policy, sandbox_policy.clone()).await?;
let approval = expect_exec_approval(&test, &command).await;
assert_eq!(
approval.additional_permissions,
Some(requested_permissions.clone())
);
test.codex
.submit(Op::ExecApproval {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Approved,
})
.await?;
wait_for_completion(&test).await;
let result = parse_result(&results.single_request().function_call_output(call_id));
assert!(
result.exit_code.is_none() || result.exit_code == Some(0),
"unexpected exit code/output: {:?} {}",
result.exit_code,
result.stdout
);
assert!(result.stdout.contains("tmp-widened"));
assert_eq!(fs::read_to_string(&tmp_write)?, "tmp-widened");
assert!(
!requested_write.exists(),
"only the unrequested tmp path should have been written"
);
let _ = fs::remove_file(tmp_write);
let _ = fs::remove_file(requested_write);
Ok(())
}
#[tokio::test(flavor = "current_thread")]
#[cfg(target_os = "macos")]
async fn workspace_write_with_additional_permissions_can_write_outside_cwd() -> Result<()> {
skip_if_no_network!(Ok(()));
skip_if_sandbox!(Ok(()));
let server = start_mock_server().await;
let approval_policy = AskForApproval::OnRequest;
let sandbox_policy = workspace_write_excluding_tmp();
let sandbox_policy_for_config = sandbox_policy.clone();
let mut builder = test_codex().with_config(move |config| {
config.permissions.approval_policy = Constrained::allow_any(approval_policy);
config.permissions.sandbox_policy = Constrained::allow_any(sandbox_policy_for_config);
config.features.enable(Feature::RequestPermissions);
});
let test = builder.build(&server).await?;
let outside_dir = tempfile::tempdir()?;
let outside_write = outside_dir.path().join("workspace-write-outside.txt");
let placeholder = test.workspace_path("workspace-write-placeholder.txt");
let _ = fs::remove_file(&outside_write);
let _ = fs::remove_file(&placeholder);
let call_id = "request_permissions_workspace_write_outside";
let command = format!(
"printf {:?} > {:?} && cat {:?}",
"outside-cwd-ok", outside_write, outside_write
);
let requested_permissions = AdditionalPermissions {
fs_read: vec![],
fs_write: vec![outside_dir.path().to_path_buf()],
};
let normalized_requested_permissions = AdditionalPermissions {
fs_read: vec![],
fs_write: vec![outside_dir.path().canonicalize()?],
};
let event = shell_event_with_request_permissions(call_id, &command, &requested_permissions)?;
let _ = mount_sse_once(
&server,
sse(vec![
ev_response_created("resp-ww-1"),
event,
ev_completed("resp-ww-1"),
]),
)
.await;
let results = mount_sse_once(
&server,
sse(vec![
ev_assistant_message("msg-ww-1", "done"),
ev_completed("resp-ww-2"),
]),
)
.await;
submit_turn(&test, call_id, approval_policy, sandbox_policy.clone()).await?;
let approval = expect_exec_approval(&test, &command).await;
assert_eq!(
approval.additional_permissions,
Some(normalized_requested_permissions)
);
test.codex
.submit(Op::ExecApproval {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Approved,
})
.await?;
wait_for_completion(&test).await;
let result = parse_result(&results.single_request().function_call_output(call_id));
assert!(
result.exit_code.is_none() || result.exit_code == Some(0),
"unexpected exit code/output: {:?} {}",
result.exit_code,
result.stdout
);
assert!(result.stdout.contains("outside-cwd-ok"));
assert_eq!(fs::read_to_string(&outside_write)?, "outside-cwd-ok");
assert!(
!placeholder.exists(),
"placeholder path should remain untouched"
);
let _ = fs::remove_file(outside_write);
let _ = fs::remove_file(placeholder);
Ok(())
}
#[tokio::test(flavor = "current_thread")]
#[cfg(unix)]
async fn with_additional_permissions_denied_approval_blocks_execution() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
let approval_policy = AskForApproval::OnRequest;
let sandbox_policy = workspace_write_excluding_tmp();
let sandbox_policy_for_config = sandbox_policy.clone();
let mut builder = test_codex().with_config(move |config| {
config.permissions.approval_policy = Constrained::allow_any(approval_policy);
config.permissions.sandbox_policy = Constrained::allow_any(sandbox_policy_for_config);
config.features.enable(Feature::RequestPermissions);
});
let test = builder.build(&server).await?;
let outside_dir = tempfile::tempdir()?;
let outside_write = outside_dir.path().join("workspace-write-denied.txt");
let _ = fs::remove_file(&outside_write);
let call_id = "request_permissions_denied";
let command = format!(
"printf {:?} > {:?} && cat {:?}",
"should-not-write", outside_write, outside_write
);
let requested_permissions = AdditionalPermissions {
fs_read: vec![],
fs_write: vec![outside_dir.path().to_path_buf()],
};
let normalized_requested_permissions = AdditionalPermissions {
fs_read: vec![],
fs_write: vec![outside_dir.path().canonicalize()?],
};
let event = shell_event_with_request_permissions(call_id, &command, &requested_permissions)?;
let _ = mount_sse_once(
&server,
sse(vec![
ev_response_created("resp-denied-1"),
event,
ev_completed("resp-denied-1"),
]),
)
.await;
let results = mount_sse_once(
&server,
sse(vec![
ev_assistant_message("msg-denied-1", "done"),
ev_completed("resp-denied-2"),
]),
)
.await;
submit_turn(&test, call_id, approval_policy, sandbox_policy.clone()).await?;
let approval = expect_exec_approval(&test, &command).await;
assert_eq!(
approval.additional_permissions,
Some(normalized_requested_permissions)
);
test.codex
.submit(Op::ExecApproval {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Denied,
})
.await?;
wait_for_completion(&test).await;
let result = parse_result(&results.single_request().function_call_output(call_id));
assert_ne!(
result.exit_code,
Some(0),
"denied command should not succeed"
);
assert!(
result.stdout.contains("rejected by user"),
"unexpected denial output: {}",
result.stdout
);
assert!(
!outside_write.exists(),
"denied command should not create file"
);
let _ = fs::remove_file(outside_write);
Ok(())
}
@@ -226,6 +226,7 @@ async fn run_codex_tool_session_inner(
proposed_network_policy_amendments: _,
parsed_cmd,
network_approval_context: _,
additional_permissions: _,
} = ev;
handle_exec_approval_request(
command,
+5
View File
@@ -2,6 +2,7 @@ use std::collections::HashMap;
use std::path::PathBuf;
use crate::mcp::RequestId;
use crate::models::AdditionalPermissions;
use crate::parse_command::ParsedCommand;
use crate::protocol::FileChange;
use schemars::JsonSchema;
@@ -102,6 +103,10 @@ pub struct ExecApprovalRequestEvent {
#[serde(default, skip_serializing_if = "Option::is_none")]
#[ts(optional)]
pub proposed_network_policy_amendments: Option<Vec<NetworkPolicyAmendment>>,
/// Optional additional filesystem permissions requested for this command.
#[serde(default, skip_serializing_if = "Option::is_none")]
#[ts(optional)]
pub additional_permissions: Option<AdditionalPermissions>,
pub parsed_cmd: Vec<ParsedCommand>,
}
+71 -4
View File
@@ -1,5 +1,6 @@
use std::collections::HashMap;
use std::path::Path;
use std::path::PathBuf;
use codex_utils_image::load_and_resize_to_fit;
use serde::Deserialize;
@@ -35,12 +36,34 @@ pub enum SandboxPermissions {
UseDefault,
/// Request to run outside the sandbox
RequireEscalated,
/// Request to run in the sandbox with additional per-command permissions.
WithAdditionalPermissions,
}
impl SandboxPermissions {
/// True if SandboxPermissions requires full unsandboxed execution (i.e. RequireEscalated)
pub fn requires_escalated_permissions(self) -> bool {
matches!(self, SandboxPermissions::RequireEscalated)
}
/// True if SandboxPermissions requires permissions beyond UseDefault
pub fn requires_additional_permissions(self) -> bool {
!matches!(self, SandboxPermissions::UseDefault)
}
}
#[derive(Debug, Clone, Default, Eq, Hash, PartialEq, Serialize, Deserialize, JsonSchema, TS)]
pub struct AdditionalPermissions {
#[serde(default, skip_serializing_if = "Vec::is_empty")]
pub fs_read: Vec<PathBuf>,
#[serde(default, skip_serializing_if = "Vec::is_empty")]
pub fs_write: Vec<PathBuf>,
}
impl AdditionalPermissions {
pub fn is_empty(&self) -> bool {
self.fs_read.is_empty() && self.fs_write.is_empty()
}
}
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, JsonSchema, TS)]
@@ -227,6 +250,8 @@ const APPROVAL_POLICY_ON_FAILURE: &str =
include_str!("prompts/permissions/approval_policy/on_failure.md");
const APPROVAL_POLICY_ON_REQUEST_RULE: &str =
include_str!("prompts/permissions/approval_policy/on_request_rule.md");
const APPROVAL_POLICY_ON_REQUEST_RULE_REQUEST_PERMISSION: &str =
include_str!("prompts/permissions/approval_policy/on_request_rule_request_permission.md");
const SANDBOX_MODE_DANGER_FULL_ACCESS: &str =
include_str!("prompts/permissions/sandbox_mode/danger_full_access.md");
@@ -239,16 +264,25 @@ impl DeveloperInstructions {
Self { text: text.into() }
}
pub fn from(approval_policy: AskForApproval, exec_policy: &Policy) -> DeveloperInstructions {
pub fn from(
approval_policy: AskForApproval,
exec_policy: &Policy,
request_permission_enabled: bool,
) -> DeveloperInstructions {
let on_request_instructions = || {
let on_request_rule = if request_permission_enabled {
APPROVAL_POLICY_ON_REQUEST_RULE_REQUEST_PERMISSION
} else {
APPROVAL_POLICY_ON_REQUEST_RULE
};
let command_prefixes = format_allow_prefixes(exec_policy.get_allowed_prefixes());
match command_prefixes {
Some(prefixes) => {
format!(
"{APPROVAL_POLICY_ON_REQUEST_RULE}\n## Approved command prefixes\nThe following prefix rules have already been approved: {prefixes}"
"{on_request_rule}\n## Approved command prefixes\nThe following prefix rules have already been approved: {prefixes}"
)
}
None => APPROVAL_POLICY_ON_REQUEST_RULE.to_string(),
None => on_request_rule.to_string(),
}
};
let text = match approval_policy {
@@ -306,6 +340,7 @@ impl DeveloperInstructions {
approval_policy: AskForApproval,
exec_policy: &Policy,
cwd: &Path,
request_permission_enabled: bool,
) -> Self {
let network_access = if sandbox_policy.has_full_network_access() {
NetworkAccess::Enabled
@@ -329,6 +364,7 @@ impl DeveloperInstructions {
approval_policy,
exec_policy,
writable_roots,
request_permission_enabled,
)
}
@@ -352,6 +388,7 @@ impl DeveloperInstructions {
approval_policy: AskForApproval,
exec_policy: &Policy,
writable_roots: Option<Vec<WritableRoot>>,
request_permission_enabled: bool,
) -> Self {
let start_tag = DeveloperInstructions::new("<permissions instructions>");
let end_tag = DeveloperInstructions::new("</permissions instructions>");
@@ -360,7 +397,11 @@ impl DeveloperInstructions {
sandbox_mode,
network_access,
))
.concat(DeveloperInstructions::from(approval_policy, exec_policy))
.concat(DeveloperInstructions::from(
approval_policy,
exec_policy,
request_permission_enabled,
))
.concat(DeveloperInstructions::from_writable_roots(writable_roots))
.concat(end_tag)
}
@@ -757,6 +798,9 @@ pub struct ShellToolCallParams {
#[serde(default, skip_serializing_if = "Option::is_none")]
#[ts(optional)]
pub prefix_rule: Option<Vec<String>>,
#[serde(default, skip_serializing_if = "Option::is_none")]
#[ts(optional)]
pub additional_permissions: Option<AdditionalPermissions>,
#[serde(skip_serializing_if = "Option::is_none")]
pub justification: Option<String>,
}
@@ -780,6 +824,9 @@ pub struct ShellCommandToolCallParams {
#[serde(default, skip_serializing_if = "Option::is_none")]
#[ts(optional)]
pub prefix_rule: Option<Vec<String>>,
#[serde(default, skip_serializing_if = "Option::is_none")]
#[ts(optional)]
pub additional_permissions: Option<AdditionalPermissions>,
#[serde(skip_serializing_if = "Option::is_none")]
pub justification: Option<String>,
}
@@ -1205,6 +1252,7 @@ mod tests {
AskForApproval::OnRequest,
&Policy::empty(),
None,
false,
);
let text = instructions.into_text();
@@ -1233,6 +1281,7 @@ mod tests {
AskForApproval::UnlessTrusted,
&Policy::empty(),
&PathBuf::from("/tmp"),
false,
);
let text = instructions.into_text();
assert!(text.contains("Network access is enabled."));
@@ -1254,6 +1303,7 @@ mod tests {
AskForApproval::OnRequest,
&exec_policy,
None,
false,
);
let text = instructions.into_text();
@@ -1262,6 +1312,22 @@ mod tests {
assert!(text.contains(r#"["git", "pull"]"#));
}
#[test]
fn includes_request_permission_rule_instructions_for_on_request_when_enabled() {
let instructions = DeveloperInstructions::from_permissions_with_network(
SandboxMode::WorkspaceWrite,
NetworkAccess::Enabled,
AskForApproval::OnRequest,
&Policy::empty(),
None,
true,
);
let text = instructions.into_text();
assert!(text.contains("with_additional_permissions"));
assert!(text.contains("additional_permissions"));
}
#[test]
fn render_command_prefix_list_sorts_by_len_then_total_len_then_alphabetical() {
let prefixes = vec![
@@ -1572,6 +1638,7 @@ mod tests {
timeout_ms: Some(1000),
sandbox_permissions: None,
prefix_rule: None,
additional_permissions: None,
justification: None,
},
params
@@ -0,0 +1,30 @@
# Permission Requests
Commands may require user approval before execution. Prefer requesting sandboxed additional permissions instead of asking to run fully outside the sandbox.
## Preferred request mode
When you need extra filesystem access for one command, use:
- `sandbox_permissions: "with_additional_permissions"`
- `additional_permissions` with one or both fields:
- `fs_read`: list of paths that need read access
- `fs_write`: list of paths that need write access
This keeps execution inside the current sandbox policy, while adding only the requested permissions for that command, unless an exec-policy allow rule applies and authorizes running the command outside the sandbox.
If the command already matches an exec-policy allow rule, the command can be auto-approved without an extra prompt. In that case, exec-policy allow behavior (including any sandbox bypass) takes precedence.
## Escalation Requests
Use full escalation only when sandboxed additional permissions cannot satisfy the task.
- `sandbox_permissions: "require_escalated"`
- Include `justification` as a short question asking for approval.
- Optionally include `prefix_rule` to suggest a reusable allow rule.
## Command segmentation reminder
The command string is split into independent command segments at shell control operators, including pipes (`|`), logical operators (`&&`, `||`), command separators (`;`), and subshell boundaries (`(...)`, `$()`).
Each segment is evaluated independently for sandbox restrictions and approval requirements.
@@ -18,6 +18,7 @@ use crate::render::renderable::ColumnRenderable;
use crate::render::renderable::Renderable;
use codex_core::features::Features;
use codex_protocol::mcp::RequestId;
use codex_protocol::models::AdditionalPermissions;
use codex_protocol::protocol::ElicitationAction;
use codex_protocol::protocol::ExecPolicyAmendment;
use codex_protocol::protocol::FileChange;
@@ -45,6 +46,7 @@ pub(crate) enum ApprovalRequest {
reason: Option<String>,
network_approval_context: Option<NetworkApprovalContext>,
proposed_execpolicy_amendment: Option<ExecPolicyAmendment>,
additional_permissions: Option<AdditionalPermissions>,
},
ApplyPatch {
id: String,
@@ -112,11 +114,13 @@ impl ApprovalOverlay {
ApprovalVariant::Exec {
network_approval_context,
proposed_execpolicy_amendment,
additional_permissions,
..
} => (
exec_options(
proposed_execpolicy_amendment.clone(),
network_approval_context.as_ref(),
additional_permissions.as_ref(),
),
network_approval_context.as_ref().map_or_else(
|| "Would you like to run the following command?".to_string(),
@@ -358,18 +362,29 @@ impl From<ApprovalRequest> for ApprovalRequestState {
reason,
network_approval_context,
proposed_execpolicy_amendment,
additional_permissions,
} => {
let mut header: Vec<Line<'static>> = Vec::new();
if let Some(reason) = reason {
header.push(Line::from(vec!["Reason: ".into(), reason.italic()]));
header.push(Line::from(""));
}
if let Some(ref additional_permissions) = additional_permissions
&& let Some(rule_line) =
format_additional_permissions_rule(additional_permissions)
{
header.push(Line::from(vec![
"Permission rule: ".into(),
rule_line.cyan(),
]));
header.push(Line::from(""));
}
let full_cmd = strip_bash_lc_and_escape(&command);
let mut full_cmd_lines = highlight_bash_to_lines(&full_cmd);
if let Some(first) = full_cmd_lines.first_mut() {
first.spans.insert(0, Span::from("$ "));
}
if network_approval_context.is_none() {
let full_cmd = strip_bash_lc_and_escape(&command);
let mut full_cmd_lines = highlight_bash_to_lines(&full_cmd);
if let Some(first) = full_cmd_lines.first_mut() {
first.spans.insert(0, Span::from("$ "));
}
header.extend(full_cmd_lines);
}
Self {
@@ -378,6 +393,7 @@ impl From<ApprovalRequest> for ApprovalRequestState {
command,
network_approval_context,
proposed_execpolicy_amendment,
additional_permissions,
},
header: Box::new(Paragraph::new(header).wrap(Wrap { trim: false })),
}
@@ -434,6 +450,7 @@ enum ApprovalVariant {
command: Vec<String>,
network_approval_context: Option<NetworkApprovalContext>,
proposed_execpolicy_amendment: Option<ExecPolicyAmendment>,
additional_permissions: Option<AdditionalPermissions>,
},
ApplyPatch {
id: String,
@@ -469,6 +486,7 @@ impl ApprovalOption {
fn exec_options(
proposed_execpolicy_amendment: Option<ExecPolicyAmendment>,
network_approval_context: Option<&NetworkApprovalContext>,
additional_permissions: Option<&AdditionalPermissions>,
) -> Vec<ApprovalOption> {
if network_approval_context.is_some() {
return vec![
@@ -493,6 +511,23 @@ fn exec_options(
];
}
if additional_permissions.is_some() {
return vec![
ApprovalOption {
label: "Yes, proceed".to_string(),
decision: ApprovalDecision::Review(ReviewDecision::Approved),
display_shortcut: None,
additional_shortcuts: vec![key_hint::plain(KeyCode::Char('y'))],
},
ApprovalOption {
label: "No, and tell Codex what to do differently".to_string(),
decision: ApprovalDecision::Review(ReviewDecision::Abort),
display_shortcut: Some(key_hint::plain(KeyCode::Esc)),
additional_shortcuts: vec![key_hint::plain(KeyCode::Char('n'))],
},
];
}
vec![ApprovalOption {
label: "Yes, proceed".to_string(),
decision: ApprovalDecision::Review(ReviewDecision::Approved),
@@ -526,6 +561,36 @@ fn exec_options(
.collect()
}
fn format_additional_permissions_rule(
additional_permissions: &AdditionalPermissions,
) -> Option<String> {
let mut parts = Vec::new();
if !additional_permissions.fs_read.is_empty() {
let reads = additional_permissions
.fs_read
.iter()
.map(|path| format!("`{}`", path.display()))
.collect::<Vec<_>>()
.join(", ");
parts.push(format!("read {reads}"));
}
if !additional_permissions.fs_write.is_empty() {
let writes = additional_permissions
.fs_write
.iter()
.map(|path| format!("`{}`", path.display()))
.collect::<Vec<_>>()
.join(", ");
parts.push(format!("write {writes}"));
}
if parts.is_empty() {
None
} else {
Some(parts.join("; "))
}
}
fn patch_options() -> Vec<ApprovalOption> {
vec![
ApprovalOption {
@@ -577,9 +642,26 @@ mod tests {
use super::*;
use crate::app_event::AppEvent;
use codex_protocol::protocol::NetworkApprovalProtocol;
use insta::assert_snapshot;
use pretty_assertions::assert_eq;
use tokio::sync::mpsc::unbounded_channel;
fn render_overlay_lines(view: &ApprovalOverlay, width: u16) -> String {
let height = view.desired_height(width);
let mut buf = Buffer::empty(Rect::new(0, 0, width, height));
view.render(Rect::new(0, 0, width, height), &mut buf);
(0..buf.area.height)
.map(|row| {
(0..buf.area.width)
.map(|col| buf[(col, row)].symbol().to_string())
.collect::<String>()
.trim_end()
.to_string()
})
.collect::<Vec<_>>()
.join("\n")
}
fn make_exec_request() -> ApprovalRequest {
ApprovalRequest::Exec {
id: "test".to_string(),
@@ -587,6 +669,7 @@ mod tests {
reason: Some("reason".to_string()),
network_approval_context: None,
proposed_execpolicy_amendment: None,
additional_permissions: None,
}
}
@@ -632,6 +715,7 @@ mod tests {
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![
"echo".to_string(),
])),
additional_permissions: None,
},
tx,
Features::with_defaults(),
@@ -669,6 +753,7 @@ mod tests {
reason: None,
network_approval_context: None,
proposed_execpolicy_amendment: None,
additional_permissions: None,
};
let view = ApprovalOverlay::new(exec_request, tx, Features::with_defaults());
@@ -699,6 +784,7 @@ mod tests {
let options = exec_options(
Some(ExecPolicyAmendment::new(vec!["curl".to_string()])),
Some(&network_context),
None,
);
let labels: Vec<String> = options.into_iter().map(|option| option.label).collect();
@@ -712,6 +798,83 @@ mod tests {
);
}
#[test]
fn additional_permissions_exec_options_hide_execpolicy_amendment() {
let additional_permissions = AdditionalPermissions {
fs_read: vec![PathBuf::from("/tmp/readme.txt")],
fs_write: vec![PathBuf::from("/tmp/out.txt")],
};
let options = exec_options(None, None, Some(&additional_permissions));
let labels: Vec<String> = options.into_iter().map(|option| option.label).collect();
assert_eq!(
labels,
vec![
"Yes, proceed".to_string(),
"No, and tell Codex what to do differently".to_string(),
]
);
}
#[test]
fn additional_permissions_prompt_shows_permission_rule_line() {
let (tx, _rx) = unbounded_channel::<AppEvent>();
let tx = AppEventSender::new(tx);
let exec_request = ApprovalRequest::Exec {
id: "test".into(),
command: vec!["cat".into(), "/tmp/readme.txt".into()],
reason: None,
network_approval_context: None,
proposed_execpolicy_amendment: None,
additional_permissions: Some(AdditionalPermissions {
fs_read: vec![PathBuf::from("/tmp/readme.txt")],
fs_write: vec![PathBuf::from("/tmp/out.txt")],
}),
};
let view = ApprovalOverlay::new(exec_request, tx, Features::with_defaults());
let mut buf = Buffer::empty(Rect::new(0, 0, 120, view.desired_height(120)));
view.render(Rect::new(0, 0, 120, view.desired_height(120)), &mut buf);
let rendered: Vec<String> = (0..buf.area.height)
.map(|row| {
(0..buf.area.width)
.map(|col| buf[(col, row)].symbol().to_string())
.collect()
})
.collect();
assert!(
rendered
.iter()
.any(|line| line.contains("Permission rule:")),
"expected permission-rule line, got {rendered:?}"
);
}
#[test]
fn additional_permissions_prompt_snapshot() {
let (tx, _rx) = unbounded_channel::<AppEvent>();
let tx = AppEventSender::new(tx);
let exec_request = ApprovalRequest::Exec {
id: "test".into(),
command: vec!["cat".into(), "/tmp/readme.txt".into()],
reason: Some("need filesystem access".into()),
network_approval_context: None,
proposed_execpolicy_amendment: None,
additional_permissions: Some(AdditionalPermissions {
fs_read: vec![PathBuf::from("/tmp/readme.txt")],
fs_write: vec![PathBuf::from("/tmp/out.txt")],
}),
};
let view = ApprovalOverlay::new(exec_request, tx, Features::with_defaults());
assert_snapshot!(
"approval_overlay_additional_permissions_prompt",
render_overlay_lines(&view, 120)
);
}
#[test]
fn network_exec_prompt_title_includes_host() {
let (tx, _rx) = unbounded_channel::<AppEvent>();
@@ -725,6 +888,7 @@ mod tests {
protocol: NetworkApprovalProtocol::Https,
}),
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec!["curl".into()])),
additional_permissions: None,
};
let view = ApprovalOverlay::new(exec_request, tx, Features::with_defaults());
+1
View File
@@ -1084,6 +1084,7 @@ mod tests {
reason: None,
network_approval_context: None,
proposed_execpolicy_amendment: None,
additional_permissions: None,
}
}
@@ -0,0 +1,17 @@
---
source: tui/src/bottom_pane/approval_overlay.rs
expression: "render_overlay_lines(&view, 120)"
---
Would you like to run the following command?
Reason: need filesystem access
Permission rule: read `/tmp/readme.txt`; write `/tmp/out.txt`
$ cat /tmp/readme.txt
1. Yes, proceed (y)
2. No, and tell Codex what to do differently (esc)
Press enter to confirm or esc to cancel
+1
View File
@@ -2552,6 +2552,7 @@ impl ChatWidget {
reason: ev.reason,
network_approval_context: ev.network_approval_context,
proposed_execpolicy_amendment: ev.proposed_execpolicy_amendment,
additional_permissions: ev.additional_permissions,
};
self.bottom_pane
.push_approval_request(request, &self.config.features);
+7
View File
@@ -2776,6 +2776,7 @@ async fn exec_approval_emits_proposed_command_and_decision_history() {
network_approval_context: None,
proposed_execpolicy_amendment: None,
proposed_network_policy_amendments: None,
additional_permissions: None,
parsed_cmd: vec![],
};
chat.handle_codex_event(Event {
@@ -2823,6 +2824,7 @@ async fn exec_approval_decision_truncates_multiline_and_long_commands() {
network_approval_context: None,
proposed_execpolicy_amendment: None,
proposed_network_policy_amendments: None,
additional_permissions: None,
parsed_cmd: vec![],
};
chat.handle_codex_event(Event {
@@ -2876,6 +2878,7 @@ async fn exec_approval_decision_truncates_multiline_and_long_commands() {
network_approval_context: None,
proposed_execpolicy_amendment: None,
proposed_network_policy_amendments: None,
additional_permissions: None,
parsed_cmd: vec![],
};
chat.handle_codex_event(Event {
@@ -6474,6 +6477,7 @@ async fn approval_modal_exec_snapshot() -> anyhow::Result<()> {
"world".into(),
])),
proposed_network_policy_amendments: None,
additional_permissions: None,
parsed_cmd: vec![],
};
chat.handle_codex_event(Event {
@@ -6533,6 +6537,7 @@ async fn approval_modal_exec_without_reason_snapshot() -> anyhow::Result<()> {
"world".into(),
])),
proposed_network_policy_amendments: None,
additional_permissions: None,
parsed_cmd: vec![],
};
chat.handle_codex_event(Event {
@@ -6579,6 +6584,7 @@ async fn approval_modal_exec_multiline_prefix_hides_execpolicy_option_snapshot()
network_approval_context: None,
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(command)),
proposed_network_policy_amendments: None,
additional_permissions: None,
parsed_cmd: vec![],
};
chat.handle_codex_event(Event {
@@ -6944,6 +6950,7 @@ async fn status_widget_and_approval_modal_snapshot() {
"hello world".into(),
])),
proposed_network_policy_amendments: None,
additional_permissions: None,
parsed_cmd: vec![],
};
chat.handle_codex_event(Event {