mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
core: preserve constrained approval/sandbox policies in TurnContext (#12473)
This commit is contained in:
committed by
GitHub
Unverified
parent
f33ac830aa
commit
66d5d34e6e
@@ -38,8 +38,8 @@ pub(crate) async fn apply_patch(
|
||||
) -> InternalApplyPatchInvocation {
|
||||
match assess_patch_safety(
|
||||
&action,
|
||||
turn_context.approval_policy,
|
||||
&turn_context.sandbox_policy,
|
||||
turn_context.approval_policy.value(),
|
||||
turn_context.sandbox_policy.get(),
|
||||
&turn_context.cwd,
|
||||
turn_context.windows_sandbox_level,
|
||||
) {
|
||||
|
||||
+33
-25
@@ -558,8 +558,8 @@ pub(crate) struct TurnContext {
|
||||
pub(crate) user_instructions: Option<String>,
|
||||
pub(crate) collaboration_mode: CollaborationMode,
|
||||
pub(crate) personality: Option<Personality>,
|
||||
pub(crate) approval_policy: AskForApproval,
|
||||
pub(crate) sandbox_policy: SandboxPolicy,
|
||||
pub(crate) approval_policy: Constrained<AskForApproval>,
|
||||
pub(crate) sandbox_policy: Constrained<SandboxPolicy>,
|
||||
pub(crate) network: Option<NetworkProxy>,
|
||||
pub(crate) windows_sandbox_level: WindowsSandboxLevel,
|
||||
pub(crate) shell_environment_policy: ShellEnvironmentPolicy,
|
||||
@@ -640,7 +640,7 @@ impl TurnContext {
|
||||
user_instructions: self.user_instructions.clone(),
|
||||
collaboration_mode,
|
||||
personality: self.personality,
|
||||
approval_policy: self.approval_policy,
|
||||
approval_policy: self.approval_policy.clone(),
|
||||
sandbox_policy: self.sandbox_policy.clone(),
|
||||
network: self.network.clone(),
|
||||
windows_sandbox_level: self.windows_sandbox_level,
|
||||
@@ -674,8 +674,8 @@ impl TurnContext {
|
||||
TurnContextItem {
|
||||
turn_id: Some(self.sub_id.clone()),
|
||||
cwd: self.cwd.clone(),
|
||||
approval_policy: self.approval_policy,
|
||||
sandbox_policy: self.sandbox_policy.clone(),
|
||||
approval_policy: self.approval_policy.value(),
|
||||
sandbox_policy: self.sandbox_policy.get().clone(),
|
||||
network: self.turn_context_network_item(),
|
||||
model: self.model_info.slug.clone(),
|
||||
personality: self.personality,
|
||||
@@ -980,8 +980,8 @@ impl Session {
|
||||
user_instructions: session_configuration.user_instructions.clone(),
|
||||
collaboration_mode: session_configuration.collaboration_mode.clone(),
|
||||
personality: session_configuration.personality,
|
||||
approval_policy: session_configuration.approval_policy.value(),
|
||||
sandbox_policy: session_configuration.sandbox_policy.get().clone(),
|
||||
approval_policy: session_configuration.approval_policy.clone(),
|
||||
sandbox_policy: session_configuration.sandbox_policy.clone(),
|
||||
network,
|
||||
windows_sandbox_level: session_configuration.windows_sandbox_level,
|
||||
shell_environment_policy: per_turn_config.permissions.shell_environment_policy.clone(),
|
||||
@@ -2713,8 +2713,8 @@ impl Session {
|
||||
let shell = self.user_shell();
|
||||
items.push(
|
||||
DeveloperInstructions::from_policy(
|
||||
&turn_context.sandbox_policy,
|
||||
turn_context.approval_policy,
|
||||
turn_context.sandbox_policy.get(),
|
||||
turn_context.approval_policy.value(),
|
||||
self.services.exec_policy.current().as_ref(),
|
||||
&turn_context.cwd,
|
||||
)
|
||||
@@ -3227,7 +3227,7 @@ impl Session {
|
||||
);
|
||||
let auth_statuses = compute_auth_statuses(mcp_servers.iter(), store_mode).await;
|
||||
let sandbox_state = SandboxState {
|
||||
sandbox_policy: turn_context.sandbox_policy.clone(),
|
||||
sandbox_policy: turn_context.sandbox_policy.get().clone(),
|
||||
codex_linux_sandbox_exe: turn_context.codex_linux_sandbox_exe.clone(),
|
||||
sandbox_cwd: turn_context.cwd.clone(),
|
||||
use_linux_sandbox_bwrap: turn_context.features.enabled(Feature::UseLinuxSandboxBwrap),
|
||||
@@ -4340,7 +4340,7 @@ async fn spawn_review_thread(
|
||||
let turn_metadata_state = Arc::new(TurnMetadataState::new(
|
||||
review_turn_id.clone(),
|
||||
parent_turn_context.cwd.clone(),
|
||||
&parent_turn_context.sandbox_policy,
|
||||
parent_turn_context.sandbox_policy.get(),
|
||||
parent_turn_context.windows_sandbox_level,
|
||||
parent_turn_context
|
||||
.features
|
||||
@@ -4365,7 +4365,7 @@ async fn spawn_review_thread(
|
||||
compact_prompt: parent_turn_context.compact_prompt.clone(),
|
||||
collaboration_mode: parent_turn_context.collaboration_mode.clone(),
|
||||
personality: parent_turn_context.personality,
|
||||
approval_policy: parent_turn_context.approval_policy,
|
||||
approval_policy: parent_turn_context.approval_policy.clone(),
|
||||
sandbox_policy: parent_turn_context.sandbox_policy.clone(),
|
||||
network: parent_turn_context.network.clone(),
|
||||
windows_sandbox_level: parent_turn_context.windows_sandbox_level,
|
||||
@@ -5720,8 +5720,8 @@ async fn try_run_sampling_request(
|
||||
|
||||
feedback_tags!(
|
||||
model = turn_context.model_info.slug.clone(),
|
||||
approval_policy = turn_context.approval_policy,
|
||||
sandbox_policy = turn_context.sandbox_policy,
|
||||
approval_policy = turn_context.approval_policy.value(),
|
||||
sandbox_policy = turn_context.sandbox_policy.get(),
|
||||
effort = turn_context.reasoning_effort,
|
||||
auth_mode = sess.services.auth_manager.auth_mode(),
|
||||
features = sess.features.enabled_features(),
|
||||
@@ -6652,8 +6652,8 @@ mod tests {
|
||||
let previous_context_item = TurnContextItem {
|
||||
turn_id: Some(turn_context.sub_id.clone()),
|
||||
cwd: turn_context.cwd.clone(),
|
||||
approval_policy: turn_context.approval_policy,
|
||||
sandbox_policy: turn_context.sandbox_policy.clone(),
|
||||
approval_policy: turn_context.approval_policy.value(),
|
||||
sandbox_policy: turn_context.sandbox_policy.get().clone(),
|
||||
network: None,
|
||||
model: previous_model.to_string(),
|
||||
personality: turn_context.personality,
|
||||
@@ -6689,8 +6689,8 @@ mod tests {
|
||||
let mut previous_context_item = TurnContextItem {
|
||||
turn_id: Some(turn_context.sub_id.clone()),
|
||||
cwd: turn_context.cwd.clone(),
|
||||
approval_policy: turn_context.approval_policy,
|
||||
sandbox_policy: turn_context.sandbox_policy.clone(),
|
||||
approval_policy: turn_context.approval_policy.value(),
|
||||
sandbox_policy: turn_context.sandbox_policy.get().clone(),
|
||||
network: None,
|
||||
model: previous_model.to_string(),
|
||||
personality: turn_context.personality,
|
||||
@@ -7051,8 +7051,8 @@ mod tests {
|
||||
let previous_context_item = TurnContextItem {
|
||||
turn_id: Some(turn_context.sub_id.clone()),
|
||||
cwd: turn_context.cwd.clone(),
|
||||
approval_policy: turn_context.approval_policy,
|
||||
sandbox_policy: turn_context.sandbox_policy.clone(),
|
||||
approval_policy: turn_context.approval_policy.value(),
|
||||
sandbox_policy: turn_context.sandbox_policy.get().clone(),
|
||||
network: None,
|
||||
model: previous_model.to_string(),
|
||||
personality: turn_context.personality,
|
||||
@@ -8809,7 +8809,10 @@ mod tests {
|
||||
|
||||
let (session, mut turn_context_raw) = make_session_and_context().await;
|
||||
// Ensure policy is NOT OnRequest so the early rejection path triggers
|
||||
turn_context_raw.approval_policy = AskForApproval::OnFailure;
|
||||
turn_context_raw
|
||||
.approval_policy
|
||||
.set(AskForApproval::OnFailure)
|
||||
.expect("test setup should allow updating approval policy");
|
||||
let session = Arc::new(session);
|
||||
let mut turn_context = Arc::new(turn_context_raw);
|
||||
|
||||
@@ -8883,7 +8886,7 @@ mod tests {
|
||||
|
||||
let expected = format!(
|
||||
"approval policy is {policy:?}; reject command — you should not ask for escalated permissions if the approval policy is {policy:?}",
|
||||
policy = turn_context.approval_policy
|
||||
policy = turn_context.approval_policy.value()
|
||||
);
|
||||
|
||||
pretty_assertions::assert_eq!(output, expected);
|
||||
@@ -8892,7 +8895,9 @@ mod tests {
|
||||
// Force DangerFullAccess to avoid platform sandbox dependencies in tests.
|
||||
Arc::get_mut(&mut turn_context)
|
||||
.expect("unique turn context Arc")
|
||||
.sandbox_policy = SandboxPolicy::DangerFullAccess;
|
||||
.sandbox_policy
|
||||
.set(SandboxPolicy::DangerFullAccess)
|
||||
.expect("test setup should allow updating sandbox policy");
|
||||
|
||||
let resp2 = handler
|
||||
.handle(ToolInvocation {
|
||||
@@ -8946,7 +8951,10 @@ mod tests {
|
||||
use crate::turn_diff_tracker::TurnDiffTracker;
|
||||
|
||||
let (session, mut turn_context_raw) = make_session_and_context().await;
|
||||
turn_context_raw.approval_policy = AskForApproval::OnFailure;
|
||||
turn_context_raw
|
||||
.approval_policy
|
||||
.set(AskForApproval::OnFailure)
|
||||
.expect("test setup should allow updating approval policy");
|
||||
let session = Arc::new(session);
|
||||
let turn_context = Arc::new(turn_context_raw);
|
||||
let tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::new()));
|
||||
@@ -8976,7 +8984,7 @@ mod tests {
|
||||
|
||||
let expected = format!(
|
||||
"approval policy is {policy:?}; reject command — you cannot ask for escalated permissions if the approval policy is {policy:?}",
|
||||
policy = turn_context.approval_policy
|
||||
policy = turn_context.approval_policy.value()
|
||||
);
|
||||
|
||||
pretty_assertions::assert_eq!(output, expected);
|
||||
|
||||
@@ -31,14 +31,16 @@ fn build_permissions_update_item(
|
||||
exec_policy: &Policy,
|
||||
) -> Option<ResponseItem> {
|
||||
let prev = previous?;
|
||||
if prev.sandbox_policy == next.sandbox_policy && prev.approval_policy == next.approval_policy {
|
||||
if prev.sandbox_policy == *next.sandbox_policy.get()
|
||||
&& prev.approval_policy == next.approval_policy.value()
|
||||
{
|
||||
return None;
|
||||
}
|
||||
|
||||
Some(
|
||||
DeveloperInstructions::from_policy(
|
||||
&next.sandbox_policy,
|
||||
next.approval_policy,
|
||||
next.sandbox_policy.get(),
|
||||
next.approval_policy.value(),
|
||||
exec_policy,
|
||||
&next.cwd,
|
||||
)
|
||||
|
||||
@@ -32,9 +32,9 @@ const MCP_DEPENDENCY_OPTION_INSTALL: &str = "Install";
|
||||
const MCP_DEPENDENCY_OPTION_SKIP: &str = "Continue anyway";
|
||||
|
||||
fn is_full_access_mode(turn_context: &TurnContext) -> bool {
|
||||
matches!(turn_context.approval_policy, AskForApproval::Never)
|
||||
matches!(turn_context.approval_policy.value(), AskForApproval::Never)
|
||||
&& matches!(
|
||||
turn_context.sandbox_policy,
|
||||
turn_context.sandbox_policy.get(),
|
||||
SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. }
|
||||
)
|
||||
}
|
||||
|
||||
@@ -401,9 +401,9 @@ async fn maybe_request_mcp_tool_approval(
|
||||
}
|
||||
|
||||
fn is_full_access_mode(turn_context: &TurnContext) -> bool {
|
||||
matches!(turn_context.approval_policy, AskForApproval::Never)
|
||||
matches!(turn_context.approval_policy.value(), AskForApproval::Never)
|
||||
&& matches!(
|
||||
turn_context.sandbox_policy,
|
||||
turn_context.sandbox_policy.get(),
|
||||
SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. }
|
||||
)
|
||||
}
|
||||
|
||||
@@ -145,7 +145,13 @@ impl ToolHandler for ApplyPatchHandler {
|
||||
tool_name: tool_name.to_string(),
|
||||
};
|
||||
let out = orchestrator
|
||||
.run(&mut runtime, &req, &tool_ctx, &turn, turn.approval_policy)
|
||||
.run(
|
||||
&mut runtime,
|
||||
&req,
|
||||
&tool_ctx,
|
||||
&turn,
|
||||
turn.approval_policy.value(),
|
||||
)
|
||||
.await
|
||||
.map(|result| result.output);
|
||||
let event_ctx = ToolEventCtx::new(
|
||||
@@ -235,7 +241,13 @@ pub(crate) async fn intercept_apply_patch(
|
||||
tool_name: tool_name.to_string(),
|
||||
};
|
||||
let out = orchestrator
|
||||
.run(&mut runtime, &req, &tool_ctx, turn, turn.approval_policy)
|
||||
.run(
|
||||
&mut runtime,
|
||||
&req,
|
||||
&tool_ctx,
|
||||
turn,
|
||||
turn.approval_policy.value(),
|
||||
)
|
||||
.await
|
||||
.map(|result| result.output);
|
||||
let event_ctx =
|
||||
|
||||
@@ -919,7 +919,7 @@ fn build_agent_shared_config(
|
||||
config
|
||||
.permissions
|
||||
.sandbox_policy
|
||||
.set(turn.sandbox_policy.clone())
|
||||
.set(turn.sandbox_policy.get().clone())
|
||||
.map_err(|err| {
|
||||
FunctionCallError::RespondToModel(format!("sandbox_policy is invalid: {err}"))
|
||||
})?;
|
||||
@@ -1913,10 +1913,13 @@ mod tests {
|
||||
let temp_dir = tempfile::tempdir().expect("temp dir");
|
||||
turn.cwd = temp_dir.path().to_path_buf();
|
||||
turn.codex_linux_sandbox_exe = Some(PathBuf::from("/bin/echo"));
|
||||
turn.sandbox_policy = pick_allowed_sandbox_policy(
|
||||
let sandbox_policy = pick_allowed_sandbox_policy(
|
||||
&turn.config.permissions.sandbox_policy,
|
||||
turn.config.permissions.sandbox_policy.get().clone(),
|
||||
);
|
||||
turn.sandbox_policy
|
||||
.set(sandbox_policy)
|
||||
.expect("sandbox policy set");
|
||||
|
||||
let config = build_agent_spawn_config(&base_instructions, &turn, 0).expect("spawn config");
|
||||
let mut expected = (*turn.config).clone();
|
||||
@@ -1938,7 +1941,7 @@ mod tests {
|
||||
expected
|
||||
.permissions
|
||||
.sandbox_policy
|
||||
.set(turn.sandbox_policy)
|
||||
.set(turn.sandbox_policy.get().clone())
|
||||
.expect("sandbox policy set");
|
||||
assert_eq!(config, expected);
|
||||
}
|
||||
@@ -1987,7 +1990,7 @@ mod tests {
|
||||
expected
|
||||
.permissions
|
||||
.sandbox_policy
|
||||
.set(turn.sandbox_policy)
|
||||
.set(turn.sandbox_policy.get().clone())
|
||||
.expect("sandbox policy set");
|
||||
assert_eq!(config, expected);
|
||||
}
|
||||
|
||||
@@ -281,11 +281,11 @@ impl ShellHandler {
|
||||
.sandbox_permissions
|
||||
.requires_escalated_permissions()
|
||||
&& !matches!(
|
||||
turn.approval_policy,
|
||||
turn.approval_policy.value(),
|
||||
codex_protocol::protocol::AskForApproval::OnRequest
|
||||
)
|
||||
{
|
||||
let approval_policy = turn.approval_policy;
|
||||
let approval_policy = turn.approval_policy.value();
|
||||
return Err(FunctionCallError::RespondToModel(format!(
|
||||
"approval policy is {approval_policy:?}; reject command — you should not ask for escalated permissions if the approval policy is {approval_policy:?}"
|
||||
)));
|
||||
@@ -322,8 +322,8 @@ impl ShellHandler {
|
||||
.exec_policy
|
||||
.create_exec_approval_requirement_for_command(ExecApprovalRequest {
|
||||
command: &exec_params.command,
|
||||
approval_policy: turn.approval_policy,
|
||||
sandbox_policy: &turn.sandbox_policy,
|
||||
approval_policy: turn.approval_policy.value(),
|
||||
sandbox_policy: turn.sandbox_policy.get(),
|
||||
sandbox_permissions: exec_params.sandbox_permissions,
|
||||
prefix_rule,
|
||||
})
|
||||
@@ -349,7 +349,13 @@ impl ShellHandler {
|
||||
tool_name,
|
||||
};
|
||||
let out = orchestrator
|
||||
.run(&mut runtime, &req, &tool_ctx, &turn, turn.approval_policy)
|
||||
.run(
|
||||
&mut runtime,
|
||||
&req,
|
||||
&tool_ctx,
|
||||
&turn,
|
||||
turn.approval_policy.value(),
|
||||
)
|
||||
.await
|
||||
.map(|result| result.output);
|
||||
let event_ctx = ToolEventCtx::new(session.as_ref(), turn.as_ref(), &call_id, None);
|
||||
|
||||
@@ -152,11 +152,11 @@ impl ToolHandler for UnifiedExecHandler {
|
||||
|
||||
if sandbox_permissions.requires_escalated_permissions()
|
||||
&& !matches!(
|
||||
context.turn.approval_policy,
|
||||
context.turn.approval_policy.value(),
|
||||
codex_protocol::protocol::AskForApproval::OnRequest
|
||||
)
|
||||
{
|
||||
let approval_policy = context.turn.approval_policy;
|
||||
let approval_policy = context.turn.approval_policy.value();
|
||||
manager.release_process_id(&process_id).await;
|
||||
return Err(FunctionCallError::RespondToModel(format!(
|
||||
"approval policy is {approval_policy:?}; reject command — you cannot ask for escalated permissions if the approval policy is {approval_policy:?}"
|
||||
|
||||
@@ -1800,8 +1800,12 @@ mod tests {
|
||||
}
|
||||
|
||||
let (session, mut turn) = make_session_and_context().await;
|
||||
turn.approval_policy = AskForApproval::Never;
|
||||
turn.sandbox_policy = SandboxPolicy::DangerFullAccess;
|
||||
turn.approval_policy
|
||||
.set(AskForApproval::Never)
|
||||
.expect("test setup should allow updating approval policy");
|
||||
turn.sandbox_policy
|
||||
.set(SandboxPolicy::DangerFullAccess)
|
||||
.expect("test setup should allow updating sandbox policy");
|
||||
|
||||
let session = Arc::new(session);
|
||||
let turn = Arc::new(turn);
|
||||
@@ -1843,8 +1847,12 @@ mod tests {
|
||||
}
|
||||
|
||||
let (session, mut turn) = make_session_and_context().await;
|
||||
turn.approval_policy = AskForApproval::Never;
|
||||
turn.sandbox_policy = SandboxPolicy::DangerFullAccess;
|
||||
turn.approval_policy
|
||||
.set(AskForApproval::Never)
|
||||
.expect("test setup should allow updating approval policy");
|
||||
turn.sandbox_policy
|
||||
.set(SandboxPolicy::DangerFullAccess)
|
||||
.expect("test setup should allow updating sandbox policy");
|
||||
|
||||
let session = Arc::new(session);
|
||||
let turn = Arc::new(turn);
|
||||
@@ -1891,8 +1899,12 @@ try {
|
||||
}
|
||||
|
||||
let (session, mut turn) = make_session_and_context().await;
|
||||
turn.approval_policy = AskForApproval::Never;
|
||||
turn.sandbox_policy = SandboxPolicy::DangerFullAccess;
|
||||
turn.approval_policy
|
||||
.set(AskForApproval::Never)
|
||||
.expect("test setup should allow updating approval policy");
|
||||
turn.sandbox_policy
|
||||
.set(SandboxPolicy::DangerFullAccess)
|
||||
.expect("test setup should allow updating sandbox policy");
|
||||
|
||||
let session = Arc::new(session);
|
||||
let turn = Arc::new(turn);
|
||||
@@ -1941,8 +1953,12 @@ console.log("cell-complete");
|
||||
{
|
||||
return Ok(());
|
||||
}
|
||||
turn.approval_policy = AskForApproval::Never;
|
||||
turn.sandbox_policy = SandboxPolicy::DangerFullAccess;
|
||||
turn.approval_policy
|
||||
.set(AskForApproval::Never)
|
||||
.expect("test setup should allow updating approval policy");
|
||||
turn.sandbox_policy
|
||||
.set(SandboxPolicy::DangerFullAccess)
|
||||
.expect("test setup should allow updating sandbox policy");
|
||||
|
||||
let session = Arc::new(session);
|
||||
let turn = Arc::new(turn);
|
||||
|
||||
@@ -299,7 +299,7 @@ impl NetworkApprovalService {
|
||||
.await;
|
||||
return NetworkDecision::deny(REASON_NOT_ALLOWED);
|
||||
};
|
||||
if !allows_network_prompt(turn_context.approval_policy) {
|
||||
if !allows_network_prompt(turn_context.approval_policy.value()) {
|
||||
pending.set_decision(PendingApprovalDecision::Deny).await;
|
||||
let mut pending_approvals = self.pending_host_approvals.lock().await;
|
||||
pending_approvals.remove(&key);
|
||||
|
||||
@@ -193,8 +193,12 @@ mod tests {
|
||||
|
||||
async fn test_session_and_turn() -> (Arc<Session>, Arc<TurnContext>) {
|
||||
let (session, mut turn) = make_session_and_context().await;
|
||||
turn.approval_policy = AskForApproval::Never;
|
||||
turn.sandbox_policy = SandboxPolicy::DangerFullAccess;
|
||||
turn.approval_policy
|
||||
.set(AskForApproval::Never)
|
||||
.expect("test setup should allow updating approval policy");
|
||||
turn.sandbox_policy
|
||||
.set(SandboxPolicy::DangerFullAccess)
|
||||
.expect("test setup should allow updating sandbox policy");
|
||||
(Arc::new(session), Arc::new(turn))
|
||||
}
|
||||
|
||||
|
||||
@@ -576,8 +576,8 @@ impl UnifiedExecProcessManager {
|
||||
.exec_policy
|
||||
.create_exec_approval_requirement_for_command(ExecApprovalRequest {
|
||||
command: &request.command,
|
||||
approval_policy: context.turn.approval_policy,
|
||||
sandbox_policy: &context.turn.sandbox_policy,
|
||||
approval_policy: context.turn.approval_policy.value(),
|
||||
sandbox_policy: context.turn.sandbox_policy.get(),
|
||||
sandbox_permissions: request.sandbox_permissions,
|
||||
prefix_rule: request.prefix_rule.clone(),
|
||||
})
|
||||
@@ -605,7 +605,7 @@ impl UnifiedExecProcessManager {
|
||||
&req,
|
||||
&tool_ctx,
|
||||
context.turn.as_ref(),
|
||||
context.turn.approval_policy,
|
||||
context.turn.approval_policy.value(),
|
||||
)
|
||||
.await
|
||||
.map(|result| (result.output, result.deferred_network_approval))
|
||||
|
||||
Reference in New Issue
Block a user