mirror of
https://github.com/openai/codex.git
synced 2026-11-09 20:50:33 +00:00
Bind permission grants to the originating turn (#49880)
## Why Background code-mode cells can continue running after a new turn starts. Looking up grants and strict review requirements through the active turn can make those cells lose their approved access or use another turn's permissions. ## What changed - Keep turn-scoped grants and strict review state in `TurnContext`, retained by captured steps and background cells. - Record permission responses against the requesting turn, including requests made after a later turn starts. - Share live session-scoped grants across turns while keeping turn-scoped grants isolated. - Use the originating turn's permissions and review requirements for tool execution, patches, MCP calls, file uploads, and terminal input. ## Testing Add regression coverage for background cells retaining grants across turn changes, turn versus session scope, permission requests after the next turn starts, and strict review enabled while `write_stdin` is queued. GitOrigin-RevId: 641502fbfd43553c2c42424d4aaa53ae8020c850
This commit is contained in:
committed by
copyberry
parent
d2b254fd17
commit
1f52d40704
@@ -53,7 +53,6 @@ use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::FileChange;
|
||||
use codex_protocol::protocol::PatchApplyUpdatedEvent;
|
||||
use codex_sandboxing::policy_transforms::effective_file_system_sandbox_policy;
|
||||
use codex_sandboxing::policy_transforms::merge_permission_profiles;
|
||||
use codex_sandboxing::policy_transforms::normalize_additional_permissions;
|
||||
use codex_sandboxing::policy_transforms::normalize_additional_permissions_with_context;
|
||||
use codex_tools::ToolName;
|
||||
@@ -520,18 +519,10 @@ async fn execute_verified_patch(
|
||||
};
|
||||
let environment_id = turn_environment.selection.environment_id.as_str();
|
||||
let file_paths = file_paths_for_action(&action);
|
||||
let granted_permissions = merge_permission_profiles(
|
||||
tool_ctx
|
||||
.session
|
||||
.granted_session_permissions(environment_id)
|
||||
.await
|
||||
.as_ref(),
|
||||
tool_ctx
|
||||
.session
|
||||
.granted_turn_permissions(environment_id)
|
||||
.await
|
||||
.as_ref(),
|
||||
);
|
||||
let granted_permissions = tool_ctx
|
||||
.step_context
|
||||
.turn
|
||||
.granted_permissions(environment_id);
|
||||
let base_file_system_sandbox_policy = turn_environment
|
||||
.permission_profile()
|
||||
.file_system_sandbox_policy();
|
||||
@@ -551,13 +542,12 @@ async fn execute_verified_patch(
|
||||
FunctionCallError::RespondToModel(format!("failed to check patch permissions: {error}"))
|
||||
})?;
|
||||
let effective_additional_permissions = apply_granted_turn_permissions(
|
||||
tool_ctx.session.as_ref(),
|
||||
&tool_ctx.step_context,
|
||||
&turn_environment,
|
||||
&cwd,
|
||||
crate::sandboxing::SandboxPermissions::UseDefault,
|
||||
additional_permissions,
|
||||
)
|
||||
.await;
|
||||
);
|
||||
let apply = apply_patch::prepare_apply_patch(
|
||||
&tool_ctx.step_context,
|
||||
&turn_environment,
|
||||
|
||||
@@ -187,13 +187,12 @@ async fn to_extension_call(invocation: &ToolInvocation) -> ExtensionToolCall<'_>
|
||||
let mut environments = Vec::new();
|
||||
for environment in invocation.step_context.environments.turn_environments() {
|
||||
let additional_permissions = apply_granted_turn_permissions(
|
||||
invocation.session.as_ref(),
|
||||
&invocation.step_context,
|
||||
environment,
|
||||
environment.cwd(),
|
||||
SandboxPermissions::UseDefault,
|
||||
/*additional_permissions*/ None,
|
||||
)
|
||||
.await
|
||||
.additional_permissions;
|
||||
let file_system_sandbox_context = environment.sandbox_context(additional_permissions);
|
||||
environments.push(ToolEnvironment {
|
||||
|
||||
@@ -50,7 +50,7 @@ use serde_json::Value;
|
||||
use crate::environment_selection::TurnEnvironmentSnapshot;
|
||||
use crate::function_tool::FunctionCallError;
|
||||
use crate::sandboxing::SandboxPermissions;
|
||||
use crate::session::session::Session;
|
||||
use crate::session::step_context::StepContext;
|
||||
use crate::session::turn_context::TurnEnvironment;
|
||||
pub(crate) use crate::tools::code_mode::CodeModeExecuteHandler;
|
||||
pub(crate) use crate::tools::code_mode::CodeModeWaitHandler;
|
||||
@@ -267,8 +267,8 @@ pub(super) fn implicit_granted_permissions(
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) async fn apply_granted_turn_permissions(
|
||||
session: &Session,
|
||||
pub(super) fn apply_granted_turn_permissions(
|
||||
step_context: &StepContext,
|
||||
environment: &TurnEnvironment,
|
||||
cwd: &PathUri,
|
||||
sandbox_permissions: SandboxPermissions,
|
||||
@@ -283,12 +283,7 @@ pub(super) async fn apply_granted_turn_permissions(
|
||||
}
|
||||
|
||||
let environment_id = &environment.selection.environment_id;
|
||||
let granted_session_permissions = session.granted_session_permissions(environment_id).await;
|
||||
let granted_turn_permissions = session.granted_turn_permissions(environment_id).await;
|
||||
let granted_permissions = merge_permission_profiles(
|
||||
granted_session_permissions.as_ref(),
|
||||
granted_turn_permissions.as_ref(),
|
||||
);
|
||||
let granted_permissions = step_context.turn.granted_permissions(environment_id);
|
||||
let effective_permissions = merge_permission_profiles(
|
||||
additional_permissions.as_ref(),
|
||||
granted_permissions.as_ref(),
|
||||
|
||||
@@ -325,13 +325,12 @@ impl ExecCommandHandler {
|
||||
turn_environment.sandbox_context(/*additional_permissions*/ None);
|
||||
let permission_context = file_system_sandbox_policy_context_for_cwd(&sandbox_context, &cwd);
|
||||
let effective_additional_permissions = apply_granted_turn_permissions(
|
||||
context.session.as_ref(),
|
||||
&context.step_context,
|
||||
turn_environment,
|
||||
&cwd,
|
||||
sandbox_permissions,
|
||||
additional_permissions,
|
||||
)
|
||||
.await;
|
||||
);
|
||||
let additional_permissions_allowed = exec_permission_approvals_enabled
|
||||
|| (session.features().enabled(Feature::RequestPermissionsTool)
|
||||
&& effective_additional_permissions.permissions_preapproved);
|
||||
|
||||
@@ -489,16 +489,6 @@ async fn exec_command_reuses_foreign_windows_grant() {
|
||||
)),
|
||||
..Default::default()
|
||||
};
|
||||
*session.active_turn.lock().await = Some(crate::state::ActiveTurn::default());
|
||||
let turn_state = {
|
||||
let active_turn = session.active_turn.lock().await;
|
||||
Arc::clone(&active_turn.as_ref().expect("active turn").turn_state)
|
||||
};
|
||||
turn_state.lock().await.record_granted_permissions(
|
||||
codex_exec_server::REMOTE_ENVIRONMENT_ID,
|
||||
granted_permissions.clone(),
|
||||
);
|
||||
|
||||
{
|
||||
let turn = Arc::get_mut(&mut turn).expect("turn should be uniquely owned");
|
||||
let TurnEnvironmentState::Ready(environment) = turn
|
||||
@@ -519,10 +509,16 @@ async fn exec_command_reuses_foreign_windows_grant() {
|
||||
);
|
||||
}
|
||||
|
||||
let step_context = StepContext::for_test(Arc::clone(&turn));
|
||||
step_context.turn.record_granted_permissions(
|
||||
codex_exec_server::REMOTE_ENVIRONMENT_ID,
|
||||
granted_permissions.clone(),
|
||||
/*strict_auto_review*/ false,
|
||||
);
|
||||
let response = ExecCommandHandler::default()
|
||||
.handle(ToolInvocation {
|
||||
session: Arc::clone(&session),
|
||||
step_context: StepContext::for_test(Arc::clone(&turn)),
|
||||
step_context,
|
||||
turn,
|
||||
cancellation_token: tokio_util::sync::CancellationToken::new(),
|
||||
tracker: Arc::new(Mutex::new(TurnDiffTracker::new())),
|
||||
|
||||
@@ -136,7 +136,7 @@ impl ToolOrchestrator {
|
||||
let otel = turn_ctx.session_telemetry.clone();
|
||||
let otel_tn = flat_tool_name(&tool_ctx.tool_name).into_owned();
|
||||
let otel_ci = &tool_ctx.call_id;
|
||||
let strict_auto_review = tool_ctx.session.strict_auto_review_enabled().await;
|
||||
let strict_auto_review = turn_ctx.strict_auto_review_enabled();
|
||||
// 1) Approval
|
||||
let mut already_approved = false;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user