Skip to content

Commit 807e2c2

Browse files
authored
Add unified exec escalation handling and tests (#6492)
Similar implementation to the shell tool
1 parent ad279ea commit 807e2c2

File tree

8 files changed

+208
-23
lines changed

8 files changed

+208
-23
lines changed

codex-rs/Cargo.lock

Lines changed: 5 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

codex-rs/core/src/codex.rs

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2323,6 +2323,7 @@ mod tests {
23232323
use crate::tools::context::ToolOutput;
23242324
use crate::tools::context::ToolPayload;
23252325
use crate::tools::handlers::ShellHandler;
2326+
use crate::tools::handlers::UnifiedExecHandler;
23262327
use crate::tools::registry::ToolHandler;
23272328
use crate::turn_diff_tracker::TurnDiffTracker;
23282329
use codex_app_server_protocol::AuthMode;
@@ -3062,6 +3063,48 @@ mod tests {
30623063
assert!(exec_output.output.contains("hi"));
30633064
}
30643065

3066+
#[tokio::test]
3067+
async fn unified_exec_rejects_escalated_permissions_when_policy_not_on_request() {
3068+
use crate::protocol::AskForApproval;
3069+
use crate::turn_diff_tracker::TurnDiffTracker;
3070+
3071+
let (session, mut turn_context_raw) = make_session_and_context();
3072+
turn_context_raw.approval_policy = AskForApproval::OnFailure;
3073+
let session = Arc::new(session);
3074+
let turn_context = Arc::new(turn_context_raw);
3075+
let tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::new()));
3076+
3077+
let handler = UnifiedExecHandler;
3078+
let resp = handler
3079+
.handle(ToolInvocation {
3080+
session: Arc::clone(&session),
3081+
turn: Arc::clone(&turn_context),
3082+
tracker: Arc::clone(&tracker),
3083+
call_id: "exec-call".to_string(),
3084+
tool_name: "exec_command".to_string(),
3085+
payload: ToolPayload::Function {
3086+
arguments: serde_json::json!({
3087+
"cmd": "echo hi",
3088+
"with_escalated_permissions": true,
3089+
"justification": "need unsandboxed execution",
3090+
})
3091+
.to_string(),
3092+
},
3093+
})
3094+
.await;
3095+
3096+
let Err(FunctionCallError::RespondToModel(output)) = resp else {
3097+
panic!("expected error result");
3098+
};
3099+
3100+
let expected = format!(
3101+
"approval policy is {policy:?}; reject command — you cannot ask for escalated permissions if the approval policy is {policy:?}",
3102+
policy = turn_context.approval_policy
3103+
);
3104+
3105+
pretty_assertions::assert_eq!(output, expected);
3106+
}
3107+
30653108
#[test]
30663109
fn mcp_init_error_display_prompts_for_github_pat() {
30673110
let server_name = "github";

codex-rs/core/src/tools/handlers/unified_exec.rs

Lines changed: 36 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@ struct ExecCommandArgs {
3636
yield_time_ms: Option<u64>,
3737
#[serde(default)]
3838
max_output_tokens: Option<usize>,
39+
#[serde(default)]
40+
with_escalated_permissions: Option<bool>,
41+
#[serde(default)]
42+
justification: Option<String>,
3943
}
4044

4145
#[derive(Debug, Deserialize)]
@@ -100,8 +104,30 @@ impl ToolHandler for UnifiedExecHandler {
100104
"failed to parse exec_command arguments: {err:?}"
101105
))
102106
})?;
103-
let workdir = args
104-
.workdir
107+
let ExecCommandArgs {
108+
cmd,
109+
workdir,
110+
shell,
111+
login,
112+
yield_time_ms,
113+
max_output_tokens,
114+
with_escalated_permissions,
115+
justification,
116+
} = args;
117+
118+
if with_escalated_permissions.unwrap_or(false)
119+
&& !matches!(
120+
context.turn.approval_policy,
121+
codex_protocol::protocol::AskForApproval::OnRequest
122+
)
123+
{
124+
return Err(FunctionCallError::RespondToModel(format!(
125+
"approval policy is {policy:?}; reject command — you cannot ask for escalated permissions if the approval policy is {policy:?}",
126+
policy = context.turn.approval_policy
127+
)));
128+
}
129+
130+
let workdir = workdir
105131
.as_deref()
106132
.filter(|value| !value.is_empty())
107133
.map(PathBuf::from);
@@ -113,18 +139,20 @@ impl ToolHandler for UnifiedExecHandler {
113139
&context.call_id,
114140
None,
115141
);
116-
let emitter = ToolEmitter::unified_exec(args.cmd.clone(), cwd.clone(), true);
142+
let emitter = ToolEmitter::unified_exec(cmd.clone(), cwd.clone(), true);
117143
emitter.emit(event_ctx, ToolEventStage::Begin).await;
118144

119145
manager
120146
.exec_command(
121147
ExecCommandRequest {
122-
command: &args.cmd,
123-
shell: &args.shell,
124-
login: args.login,
125-
yield_time_ms: args.yield_time_ms,
126-
max_output_tokens: args.max_output_tokens,
148+
command: &cmd,
149+
shell: &shell,
150+
login,
151+
yield_time_ms,
152+
max_output_tokens,
127153
workdir,
154+
with_escalated_permissions,
155+
justification,
128156
},
129157
&context,
130158
)

codex-rs/core/src/tools/runtimes/unified_exec.rs

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ pub struct UnifiedExecRequest {
3434
pub command: Vec<String>,
3535
pub cwd: PathBuf,
3636
pub env: HashMap<String, String>,
37+
pub with_escalated_permissions: Option<bool>,
38+
pub justification: Option<String>,
3739
}
3840

3941
impl ProvidesSandboxRetryData for UnifiedExecRequest {
@@ -49,15 +51,28 @@ impl ProvidesSandboxRetryData for UnifiedExecRequest {
4951
pub struct UnifiedExecApprovalKey {
5052
pub command: Vec<String>,
5153
pub cwd: PathBuf,
54+
pub escalated: bool,
5255
}
5356

5457
pub struct UnifiedExecRuntime<'a> {
5558
manager: &'a UnifiedExecSessionManager,
5659
}
5760

5861
impl UnifiedExecRequest {
59-
pub fn new(command: Vec<String>, cwd: PathBuf, env: HashMap<String, String>) -> Self {
60-
Self { command, cwd, env }
62+
pub fn new(
63+
command: Vec<String>,
64+
cwd: PathBuf,
65+
env: HashMap<String, String>,
66+
with_escalated_permissions: Option<bool>,
67+
justification: Option<String>,
68+
) -> Self {
69+
Self {
70+
command,
71+
cwd,
72+
env,
73+
with_escalated_permissions,
74+
justification,
75+
}
6176
}
6277
}
6378

@@ -84,6 +99,7 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
8499
UnifiedExecApprovalKey {
85100
command: req.command.clone(),
86101
cwd: req.cwd.clone(),
102+
escalated: req.with_escalated_permissions.unwrap_or(false),
87103
}
88104
}
89105

@@ -98,7 +114,10 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
98114
let call_id = ctx.call_id.to_string();
99115
let command = req.command.clone();
100116
let cwd = req.cwd.clone();
101-
let reason = ctx.retry_reason.clone();
117+
let reason = ctx
118+
.retry_reason
119+
.clone()
120+
.or_else(|| req.justification.clone());
102121
let risk = ctx.risk.clone();
103122
Box::pin(async move {
104123
with_cached_approval(&session.services, key, || async move {
@@ -116,7 +135,16 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
116135
policy: AskForApproval,
117136
sandbox_policy: &SandboxPolicy,
118137
) -> bool {
119-
requires_initial_appoval(policy, sandbox_policy, &req.command, false)
138+
requires_initial_appoval(
139+
policy,
140+
sandbox_policy,
141+
&req.command,
142+
req.with_escalated_permissions.unwrap_or(false),
143+
)
144+
}
145+
146+
fn wants_escalated_first_attempt(&self, req: &UnifiedExecRequest) -> bool {
147+
req.with_escalated_permissions.unwrap_or(false)
120148
}
121149
}
122150

@@ -127,8 +155,15 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecSession> for UnifiedExecRunt
127155
attempt: &SandboxAttempt<'_>,
128156
_ctx: &ToolCtx<'_>,
129157
) -> Result<UnifiedExecSession, ToolError> {
130-
let spec = build_command_spec(&req.command, &req.cwd, &req.env, None, None, None)
131-
.map_err(|_| ToolError::Rejected("missing command line for PTY".to_string()))?;
158+
let spec = build_command_spec(
159+
&req.command,
160+
&req.cwd,
161+
&req.env,
162+
None,
163+
req.with_escalated_permissions,
164+
req.justification.clone(),
165+
)
166+
.map_err(|_| ToolError::Rejected("missing command line for PTY".to_string()))?;
132167
let exec_env = attempt
133168
.env_for(&spec)
134169
.map_err(|err| ToolError::Codex(err.into()))?;

codex-rs/core/src/tools/spec.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,24 @@ fn create_exec_command_tool() -> ToolSpec {
177177
),
178178
},
179179
);
180+
properties.insert(
181+
"with_escalated_permissions".to_string(),
182+
JsonSchema::Boolean {
183+
description: Some(
184+
"Whether to request escalated permissions. Set to true if command needs to be run without sandbox restrictions"
185+
.to_string(),
186+
),
187+
},
188+
);
189+
properties.insert(
190+
"justification".to_string(),
191+
JsonSchema::String {
192+
description: Some(
193+
"Only set if with_escalated_permissions is true. 1-sentence explanation of why we want to run this command."
194+
.to_string(),
195+
),
196+
},
197+
);
180198

181199
ToolSpec::Function(ResponsesApiTool {
182200
name: "exec_command".to_string(),

codex-rs/core/src/unified_exec/mod.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,8 @@ pub(crate) struct ExecCommandRequest<'a> {
7171
pub yield_time_ms: Option<u64>,
7272
pub max_output_tokens: Option<usize>,
7373
pub workdir: Option<PathBuf>,
74+
pub with_escalated_permissions: Option<bool>,
75+
pub justification: Option<String>,
7476
}
7577

7678
#[derive(Debug)]
@@ -201,6 +203,8 @@ mod tests {
201203
yield_time_ms,
202204
max_output_tokens: None,
203205
workdir: None,
206+
with_escalated_permissions: None,
207+
justification: None,
204208
},
205209
&context,
206210
)

codex-rs/core/src/unified_exec/session_manager.rs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,13 @@ impl UnifiedExecSessionManager {
5151
];
5252

5353
let session = self
54-
.open_session_with_sandbox(command, cwd.clone(), context)
54+
.open_session_with_sandbox(
55+
command,
56+
cwd.clone(),
57+
request.with_escalated_permissions,
58+
request.justification,
59+
context,
60+
)
5561
.await?;
5662

5763
let max_tokens = resolve_max_tokens(request.max_output_tokens);
@@ -317,6 +323,8 @@ impl UnifiedExecSessionManager {
317323
&self,
318324
command: Vec<String>,
319325
cwd: PathBuf,
326+
with_escalated_permissions: Option<bool>,
327+
justification: Option<String>,
320328
context: &UnifiedExecContext,
321329
) -> Result<UnifiedExecSession, UnifiedExecError> {
322330
let mut orchestrator = ToolOrchestrator::new();
@@ -325,6 +333,8 @@ impl UnifiedExecSessionManager {
325333
command,
326334
cwd,
327335
create_env(&context.turn.shell_environment_policy),
336+
with_escalated_permissions,
337+
justification,
328338
);
329339
let tool_ctx = ToolCtx {
330340
session: context.session.as_ref(),

0 commit comments

Comments
 (0)