mirror of
https://github.com/openai/codex.git
synced 2026-10-21 07:42:01 +00:00
Use turn environment permissions for tool execution (#37038)
## What changed - Derive sandboxing, approval, and permission-profile environment variables from the selected turn environment for shell, unified exec, `apply_patch`, image viewing, and network approval flows. - Materialize project-root permissions against every workspace root in that environment, allowing `apply_patch` to write to secondary workspace roots while preserving protected paths. - Carry the selected environment's permission profile into spawned or resumed agents and Guardian permission context. ## Testing - Add an integration test covering `apply_patch` writes in a secondary workspace root. - Extend tool, Guardian, and multi-agent tests to distinguish environment permissions from thread-level permissions. GitOrigin-RevId: e22da8974bcd3de1e724978b1a7790307a51e4bc
This commit is contained in:
@@ -5,6 +5,7 @@ use crate::session::turn_context::TurnContext;
|
||||
use crate::tools::sandboxing::ExecApprovalRequirement;
|
||||
use codex_apply_patch::ApplyPatchAction;
|
||||
use codex_apply_patch::ApplyPatchFileChange;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::protocol::FileChange;
|
||||
use codex_protocol::protocol::FileSystemSandboxPolicy;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
@@ -20,13 +21,14 @@ pub(crate) struct ApplyPatchRuntimeInvocation {
|
||||
|
||||
pub(crate) fn prepare_apply_patch(
|
||||
turn_context: &TurnContext,
|
||||
permission_profile: &PermissionProfile,
|
||||
file_system_sandbox_policy: &FileSystemSandboxPolicy,
|
||||
action: ApplyPatchAction,
|
||||
) -> Result<ApplyPatchRuntimeInvocation, FunctionCallError> {
|
||||
match assess_patch_safety(
|
||||
&action,
|
||||
turn_context.approval_policy(),
|
||||
&turn_context.permission_profile(),
|
||||
permission_profile,
|
||||
file_system_sandbox_policy,
|
||||
&action.cwd,
|
||||
turn_context.windows_sandbox_level,
|
||||
|
||||
@@ -12,6 +12,7 @@ use crate::compact::content_items_to_text;
|
||||
use crate::event_mapping::is_contextual_user_message_content;
|
||||
use crate::session::session::Session;
|
||||
use crate::session::turn_context::TurnContext;
|
||||
use crate::session::turn_context::TurnEnvironment;
|
||||
use codex_utils_output_truncation::TruncationPolicy;
|
||||
use codex_utils_output_truncation::approx_bytes_for_tokens;
|
||||
use codex_utils_output_truncation::approx_token_count;
|
||||
@@ -254,17 +255,24 @@ pub(crate) async fn build_guardian_prompt_items_with_parent_turn(
|
||||
}
|
||||
|
||||
fn parent_turn_denied_reads_context(turn: &TurnContext) -> Option<String> {
|
||||
// TODO(sayan): Pass StepContext through Guardian so it sees environments that become ready mid-turn.
|
||||
let environment = turn.environments.primary();
|
||||
#[allow(deprecated)]
|
||||
let cwd = &turn.cwd;
|
||||
let file_system_policy = turn.file_system_sandbox_policy();
|
||||
let cwd = environment
|
||||
.and_then(|environment| environment.cwd().to_abs_path().ok())
|
||||
.unwrap_or_else(|| turn.cwd.clone());
|
||||
let permission_profile = environment
|
||||
.map(TurnEnvironment::permission_profile_with_workspace_roots)
|
||||
.unwrap_or_else(|| turn.permission_profile());
|
||||
let file_system_policy = permission_profile.file_system_sandbox_policy();
|
||||
let mut entries = file_system_policy
|
||||
.get_unreadable_roots_with_cwd(cwd)
|
||||
.get_unreadable_roots_with_cwd(&cwd)
|
||||
.into_iter()
|
||||
.map(|root| format!("- path `{}`", root.to_string_lossy()))
|
||||
.collect::<Vec<_>>();
|
||||
entries.extend(
|
||||
file_system_policy
|
||||
.get_unreadable_globs_with_cwd(cwd)
|
||||
.get_unreadable_globs_with_cwd(&cwd)
|
||||
.into_iter()
|
||||
.map(|glob| format!("- glob `{glob}`")),
|
||||
);
|
||||
|
||||
@@ -4,7 +4,9 @@ use crate::config::ConfigOverrides;
|
||||
use crate::config::Constrained;
|
||||
use crate::config::ManagedFeatures;
|
||||
use crate::config::NetworkProxySpec;
|
||||
use crate::config::PermissionProfileSnapshot;
|
||||
use crate::config::test_config;
|
||||
use crate::environment_selection::TurnEnvironmentState;
|
||||
use crate::guardian::approval_request::guardian_request_target_item_id;
|
||||
use crate::guardian::prompt::BUNDLED_GUARDIAN_POLICY;
|
||||
use crate::guardian::prompt::BUNDLED_GUARDIAN_POLICY_TEMPLATE;
|
||||
@@ -12,6 +14,7 @@ use crate::guardian::prompt::guardian_policy_prompt_with_config_and_template;
|
||||
use crate::guardian::review::guardian_review_session_config;
|
||||
use crate::session::session::Session;
|
||||
use crate::session::turn_context::TurnContext;
|
||||
use crate::session::turn_context::TurnEnvironment;
|
||||
use crate::test_support;
|
||||
use codex_analytics::GuardianApprovalRequestSource;
|
||||
use codex_config::ConfigLayerStack;
|
||||
@@ -55,6 +58,7 @@ use codex_protocol::protocol::GuardianUserAuthorization;
|
||||
use codex_protocol::protocol::ReviewDecision;
|
||||
use codex_protocol::protocol::RolloutItem;
|
||||
use codex_protocol::protocol::TurnCompleteEvent;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use core_test_support::PathBufExt;
|
||||
use core_test_support::TempDirExt;
|
||||
use core_test_support::context_snapshot;
|
||||
@@ -544,9 +548,12 @@ async fn build_guardian_prompt_truncates_oversized_approval_reason() -> anyhow::
|
||||
async fn build_guardian_prompt_includes_parent_turn_denied_reads() -> anyhow::Result<()> {
|
||||
let (mut session, mut turn) = crate::session::tests::make_session_and_context().await;
|
||||
session.thread_id = fixed_guardian_parent_session_id();
|
||||
let denied_root = test_path_buf("/repo/private").abs();
|
||||
let workspace_root = test_path_buf("/repo").abs();
|
||||
let second_workspace_root = test_path_buf("/another-repo").abs();
|
||||
let denied_root = workspace_root.join("private");
|
||||
let second_denied_root = second_workspace_root.join("private");
|
||||
let denied_glob = test_path_buf("/repo/private/**").display().to_string();
|
||||
let permission_profile = PermissionProfile::from_runtime_permissions(
|
||||
let environment_permission_profile = PermissionProfile::from_runtime_permissions(
|
||||
&FileSystemSandboxPolicy::restricted(vec![
|
||||
FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Special {
|
||||
@@ -556,8 +563,10 @@ async fn build_guardian_prompt_includes_parent_turn_denied_reads() -> anyhow::Re
|
||||
missing_path_behavior: None,
|
||||
},
|
||||
FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Path {
|
||||
path: denied_root.clone(),
|
||||
path: FileSystemPath::Special {
|
||||
value: codex_protocol::permissions::FileSystemSpecialPath::project_roots(Some(
|
||||
"private".to_string(),
|
||||
)),
|
||||
},
|
||||
access: FileSystemAccessMode::Deny,
|
||||
missing_path_behavior: None,
|
||||
@@ -572,10 +581,22 @@ async fn build_guardian_prompt_includes_parent_turn_denied_reads() -> anyhow::Re
|
||||
]),
|
||||
NetworkSandboxPolicy::Restricted,
|
||||
);
|
||||
Arc::make_mut(&mut turn.config)
|
||||
.permissions
|
||||
.set_permission_profile(permission_profile)
|
||||
.expect("test setup should allow updating permission profile");
|
||||
let TurnEnvironmentState::Ready(environment) = &mut turn.environments.environments[0] else {
|
||||
panic!("parent environment should be ready");
|
||||
};
|
||||
environment.config.permission_profile =
|
||||
PermissionProfileSnapshot::legacy(environment_permission_profile);
|
||||
*environment = TurnEnvironment::new(
|
||||
environment.environment_id.clone(),
|
||||
Arc::clone(&environment.environment),
|
||||
environment.cwd().clone(),
|
||||
vec![
|
||||
PathUri::from_abs_path(&workspace_root),
|
||||
PathUri::from_abs_path(&second_workspace_root),
|
||||
],
|
||||
environment.shell.clone(),
|
||||
environment.config.clone(),
|
||||
);
|
||||
let session = Arc::new(session);
|
||||
let turn = Arc::new(turn);
|
||||
seed_guardian_parent_history(&session, &turn).await;
|
||||
@@ -603,6 +624,7 @@ async fn build_guardian_prompt_includes_parent_turn_denied_reads() -> anyhow::Re
|
||||
assert!(text.contains("PARENT TURN PERMISSION CONTEXT START"));
|
||||
assert!(text.contains("do not approve escalation whose purpose is to read them"));
|
||||
assert!(text.contains(denied_root.to_string_lossy().as_ref()));
|
||||
assert!(text.contains(second_denied_root.to_string_lossy().as_ref()));
|
||||
assert!(text.contains(&format!("glob `{denied_glob}`")));
|
||||
|
||||
Ok(())
|
||||
|
||||
@@ -1195,16 +1195,16 @@ async fn danger_full_access_tool_attempts_do_not_enforce_managed_network() -> an
|
||||
enforce_managed_network: Vec<bool>,
|
||||
}
|
||||
|
||||
impl crate::tools::sandboxing::Approvable<()> for ProbeToolRuntime {
|
||||
impl crate::tools::sandboxing::Approvable<TurnEnvironment> for ProbeToolRuntime {
|
||||
type ApprovalKey = String;
|
||||
|
||||
fn approval_keys(&self, _req: &()) -> Vec<Self::ApprovalKey> {
|
||||
fn approval_keys(&self, _req: &TurnEnvironment) -> Vec<Self::ApprovalKey> {
|
||||
vec!["probe".to_string()]
|
||||
}
|
||||
|
||||
fn start_approval_async<'a>(
|
||||
&'a mut self,
|
||||
_req: &'a (),
|
||||
_req: &'a TurnEnvironment,
|
||||
_ctx: crate::tools::sandboxing::ApprovalCtx<'a>,
|
||||
) -> futures::future::BoxFuture<'a, ReviewDecision> {
|
||||
Box::pin(async { ReviewDecision::Approved })
|
||||
@@ -1212,7 +1212,7 @@ async fn danger_full_access_tool_attempts_do_not_enforce_managed_network() -> an
|
||||
|
||||
fn approval_action(
|
||||
&self,
|
||||
_req: &(),
|
||||
_req: &TurnEnvironment,
|
||||
ctx: &crate::tools::sandboxing::ApprovalCtx<'_>,
|
||||
) -> std::io::Result<crate::tools::sandboxing::ApprovalAction> {
|
||||
Ok(crate::tools::sandboxing::ApprovalAction::Shell {
|
||||
@@ -1234,14 +1234,14 @@ async fn danger_full_access_tool_attempts_do_not_enforce_managed_network() -> an
|
||||
}
|
||||
}
|
||||
|
||||
impl crate::tools::sandboxing::ToolRuntime<(), ()> for ProbeToolRuntime {
|
||||
fn workspace_roots<'a>(&self, _req: &'a ()) -> &'a [PathUri] {
|
||||
&[]
|
||||
impl crate::tools::sandboxing::ToolRuntime<TurnEnvironment, ()> for ProbeToolRuntime {
|
||||
fn turn_environment<'a>(&self, req: &'a TurnEnvironment) -> &'a TurnEnvironment {
|
||||
req
|
||||
}
|
||||
|
||||
async fn run(
|
||||
&mut self,
|
||||
_req: &(),
|
||||
_req: &TurnEnvironment,
|
||||
attempt: &crate::tools::sandboxing::SandboxAttempt<'_>,
|
||||
_ctx: &crate::tools::sandboxing::ToolCtx,
|
||||
) -> Result<(), crate::tools::sandboxing::ToolError> {
|
||||
@@ -1305,7 +1305,9 @@ async fn danger_full_access_tool_attempts_do_not_enforce_managed_network() -> an
|
||||
orchestrator
|
||||
.run(
|
||||
&mut tool,
|
||||
&(),
|
||||
turn.environments
|
||||
.primary()
|
||||
.expect("turn should have a primary environment"),
|
||||
&tool_ctx,
|
||||
turn.as_ref(),
|
||||
AskForApproval::Never,
|
||||
|
||||
@@ -309,6 +309,13 @@ async fn guardian_allows_shell_command_additional_permissions_requests_past_poli
|
||||
.permissions
|
||||
.set_permission_profile(codex_protocol::models::PermissionProfile::Disabled)
|
||||
.expect("test setup should allow disabling the permission profile");
|
||||
let TurnEnvironmentState::Ready(environment) =
|
||||
&mut turn_context_raw.environments.environments[0]
|
||||
else {
|
||||
panic!("primary environment should be ready");
|
||||
};
|
||||
environment.config.permission_profile =
|
||||
config.permissions.permission_profile_state().snapshot();
|
||||
config.codex_linux_sandbox_exe = codex_linux_sandbox_exe_or_skip!();
|
||||
config
|
||||
.features
|
||||
@@ -424,6 +431,13 @@ async fn strict_auto_review_turn_grant_forces_guardian_for_shell_command_policy_
|
||||
.permissions
|
||||
.set_permission_profile(codex_protocol::models::PermissionProfile::Disabled)
|
||||
.expect("test setup should allow disabling the permission profile");
|
||||
let TurnEnvironmentState::Ready(environment) =
|
||||
&mut turn_context_raw.environments.environments[0]
|
||||
else {
|
||||
panic!("primary environment should be ready");
|
||||
};
|
||||
environment.config.permission_profile =
|
||||
config.permissions.permission_profile_state().snapshot();
|
||||
config.approvals_reviewer = ApprovalsReviewer::User;
|
||||
config.model_provider.base_url = Some(format!("{}/v1", server.uri()));
|
||||
let config = Arc::new(config);
|
||||
|
||||
@@ -81,6 +81,25 @@ impl TurnEnvironment {
|
||||
&self.workspace_roots
|
||||
}
|
||||
|
||||
pub(crate) fn permission_profile(&self) -> &PermissionProfile {
|
||||
self.config.permission_profile.permission_profile()
|
||||
}
|
||||
|
||||
pub(crate) fn active_permission_profile(&self) -> Option<ActivePermissionProfile> {
|
||||
self.config.permission_profile.active_permission_profile()
|
||||
}
|
||||
|
||||
pub(crate) fn permission_profile_with_workspace_roots(&self) -> PermissionProfile {
|
||||
let workspace_roots = self
|
||||
.workspace_roots()
|
||||
.iter()
|
||||
.filter_map(|workspace_root| workspace_root.to_abs_path().ok())
|
||||
.collect::<Vec<_>>();
|
||||
self.permission_profile()
|
||||
.clone()
|
||||
.materialize_project_roots_with_workspace_roots(&workspace_roots)
|
||||
}
|
||||
|
||||
pub(crate) fn selection(&self) -> TurnEnvironmentSelection {
|
||||
TurnEnvironmentSelection {
|
||||
environment_id: self.environment_id.clone(),
|
||||
@@ -333,7 +352,7 @@ impl TurnContext {
|
||||
environment: &TurnEnvironment,
|
||||
) -> FileSystemSandboxContext {
|
||||
let permissions = effective_permission_profile(
|
||||
self.config.permissions.permission_profile(),
|
||||
environment.permission_profile(),
|
||||
additional_permissions.as_ref(),
|
||||
);
|
||||
FileSystemSandboxContext {
|
||||
|
||||
@@ -259,8 +259,7 @@ fn apply_patch_payload_command(payload: &ToolPayload) -> Option<String> {
|
||||
|
||||
async fn effective_patch_permissions(
|
||||
session: &Session,
|
||||
turn: &TurnContext,
|
||||
environment_id: &str,
|
||||
environment: &TurnEnvironment,
|
||||
action: &ApplyPatchAction,
|
||||
cwd: &PathUri,
|
||||
) -> std::io::Result<(
|
||||
@@ -268,6 +267,7 @@ async fn effective_patch_permissions(
|
||||
crate::tools::handlers::EffectiveAdditionalPermissions,
|
||||
codex_protocol::permissions::FileSystemSandboxPolicy,
|
||||
)> {
|
||||
let environment_id = environment.environment_id.as_str();
|
||||
let file_paths = file_paths_for_action(action);
|
||||
let native_cwd = cwd.to_abs_path()?;
|
||||
let granted_permissions = merge_permission_profiles(
|
||||
@@ -280,7 +280,9 @@ async fn effective_patch_permissions(
|
||||
.await
|
||||
.as_ref(),
|
||||
);
|
||||
let base_file_system_sandbox_policy = turn.file_system_sandbox_policy();
|
||||
let base_file_system_sandbox_policy = environment
|
||||
.permission_profile_with_workspace_roots()
|
||||
.file_system_sandbox_policy();
|
||||
let file_system_sandbox_policy = effective_file_system_sandbox_policy(
|
||||
&base_file_system_sandbox_policy,
|
||||
granted_permissions.as_ref(),
|
||||
@@ -527,17 +529,12 @@ async fn execute_verified_patch(
|
||||
tool_ctx: ToolCtx,
|
||||
) -> Result<String, FunctionCallError> {
|
||||
let (file_paths, effective_additional_permissions, file_system_sandbox_policy) =
|
||||
effective_patch_permissions(
|
||||
tool_ctx.session.as_ref(),
|
||||
tool_ctx.turn.as_ref(),
|
||||
&turn_environment.environment_id,
|
||||
&action,
|
||||
cwd,
|
||||
)
|
||||
.await
|
||||
.unwrap_or_else(|_| patch_permissions_without_path_matching(&action));
|
||||
effective_patch_permissions(tool_ctx.session.as_ref(), &turn_environment, &action, cwd)
|
||||
.await
|
||||
.unwrap_or_else(|_| patch_permissions_without_path_matching(&action));
|
||||
let apply = apply_patch::prepare_apply_patch(
|
||||
tool_ctx.turn.as_ref(),
|
||||
turn_environment.permission_profile(),
|
||||
&file_system_sandbox_policy,
|
||||
action,
|
||||
)?;
|
||||
|
||||
@@ -2,6 +2,7 @@ use super::*;
|
||||
use crate::agent::next_thread_spawn_depth;
|
||||
use crate::session::session::Session;
|
||||
use crate::session::turn_context::TurnContext;
|
||||
use crate::session::turn_context::TurnEnvironment;
|
||||
use crate::tools::handlers::multi_agents_spec::create_resume_agent_tool;
|
||||
use codex_tools::ToolSpec;
|
||||
use std::sync::Arc;
|
||||
@@ -35,6 +36,7 @@ async fn handle_resume_agent(
|
||||
let ToolInvocation {
|
||||
session,
|
||||
turn,
|
||||
step_context,
|
||||
payload,
|
||||
call_id,
|
||||
..
|
||||
@@ -88,6 +90,7 @@ async fn handle_resume_agent(
|
||||
match Box::pin(try_resume_closed_agent(
|
||||
&session,
|
||||
&turn,
|
||||
step_context.environments.primary(),
|
||||
receiver_thread_id,
|
||||
child_depth,
|
||||
))
|
||||
@@ -188,10 +191,11 @@ impl ToolOutput for ResumeAgentResult {
|
||||
async fn try_resume_closed_agent(
|
||||
session: &Arc<Session>,
|
||||
turn: &Arc<TurnContext>,
|
||||
environment: Option<&TurnEnvironment>,
|
||||
receiver_thread_id: ThreadId,
|
||||
child_depth: i32,
|
||||
) -> Result<(), FunctionCallError> {
|
||||
let config = build_agent_resume_config(turn.as_ref())?;
|
||||
let config = build_agent_resume_config(turn.as_ref(), environment)?;
|
||||
Box::pin(session.services.agent_control.resume_agent_from_rollout(
|
||||
config,
|
||||
receiver_thread_id,
|
||||
|
||||
@@ -34,6 +34,7 @@ impl Handler {
|
||||
let ToolInvocation {
|
||||
session,
|
||||
turn,
|
||||
step_context,
|
||||
payload,
|
||||
call_id,
|
||||
..
|
||||
@@ -48,7 +49,8 @@ impl Handler {
|
||||
.agent_control
|
||||
.get_agent_metadata(receiver_thread_id);
|
||||
if receiver_agent.is_some() {
|
||||
let resume_config = build_agent_resume_config(turn.as_ref())?;
|
||||
let resume_config =
|
||||
build_agent_resume_config(turn.as_ref(), step_context.environments.primary())?;
|
||||
session
|
||||
.services
|
||||
.agent_control
|
||||
|
||||
@@ -86,8 +86,11 @@ async fn handle_spawn_agent(
|
||||
}),
|
||||
)
|
||||
.await;
|
||||
let mut config =
|
||||
build_agent_spawn_config(&session.get_base_instructions().await, turn.as_ref())?;
|
||||
let mut config = build_agent_spawn_config(
|
||||
&session.get_base_instructions().await,
|
||||
turn.as_ref(),
|
||||
step_context.environments.primary(),
|
||||
)?;
|
||||
if let Some(service_tier) = args.service_tier.as_ref() {
|
||||
config.service_tier = Some(service_tier.clone());
|
||||
}
|
||||
@@ -112,7 +115,11 @@ async fn handle_spawn_agent(
|
||||
args.service_tier.as_deref(),
|
||||
)
|
||||
.await?;
|
||||
apply_spawn_agent_runtime_overrides(&mut config, turn.as_ref())?;
|
||||
apply_spawn_agent_runtime_overrides(
|
||||
&mut config,
|
||||
turn.as_ref(),
|
||||
step_context.environments.primary(),
|
||||
)?;
|
||||
|
||||
let result = Box::pin(session.services.agent_control.spawn_agent_with_metadata(
|
||||
config,
|
||||
|
||||
@@ -6,6 +6,7 @@ use crate::config::HARD_MAX_MULTI_AGENT_V2_TIMEOUT_MS;
|
||||
use crate::function_tool::FunctionCallError;
|
||||
use crate::session::session::Session;
|
||||
use crate::session::turn_context::TurnContext;
|
||||
use crate::session::turn_context::TurnEnvironment;
|
||||
use crate::tools::context::FunctionToolOutput;
|
||||
use crate::tools::context::ToolOutput;
|
||||
use crate::tools::context::ToolPayload;
|
||||
@@ -170,27 +171,34 @@ pub(crate) fn parse_collab_input(
|
||||
/// Builds the base config snapshot for a newly spawned sub-agent.
|
||||
///
|
||||
/// The returned config starts from the parent's effective config and then refreshes the
|
||||
/// runtime-owned fields carried on `turn`, including model selection, reasoning settings,
|
||||
/// approval policy, sandbox, and cwd. Role-specific overrides are layered after this step;
|
||||
/// skipping this helper and cloning stale config state directly can send the child agent out with
|
||||
/// the wrong provider or runtime policy.
|
||||
/// runtime-owned fields carried by the turn and selected environment, including model selection,
|
||||
/// reasoning settings, approval policy, sandbox, and cwd. Role-specific overrides are layered
|
||||
/// after this step; skipping this helper and cloning stale config state directly can send the child
|
||||
/// agent out with the wrong provider or runtime policy.
|
||||
pub(crate) fn build_agent_spawn_config(
|
||||
base_instructions: &BaseInstructions,
|
||||
turn: &TurnContext,
|
||||
environment: Option<&TurnEnvironment>,
|
||||
) -> Result<Config, FunctionCallError> {
|
||||
let mut config = build_agent_shared_config(turn)?;
|
||||
let mut config = build_agent_shared_config(turn, environment)?;
|
||||
config.base_instructions = Some(base_instructions.text.clone());
|
||||
Ok(config)
|
||||
}
|
||||
|
||||
pub(crate) fn build_agent_resume_config(turn: &TurnContext) -> Result<Config, FunctionCallError> {
|
||||
let mut config = build_agent_shared_config(turn)?;
|
||||
pub(crate) fn build_agent_resume_config(
|
||||
turn: &TurnContext,
|
||||
environment: Option<&TurnEnvironment>,
|
||||
) -> Result<Config, FunctionCallError> {
|
||||
let mut config = build_agent_shared_config(turn, environment)?;
|
||||
// For resume, keep base instructions sourced from rollout/session metadata.
|
||||
config.base_instructions = None;
|
||||
Ok(config)
|
||||
}
|
||||
|
||||
fn build_agent_shared_config(turn: &TurnContext) -> Result<Config, FunctionCallError> {
|
||||
fn build_agent_shared_config(
|
||||
turn: &TurnContext,
|
||||
environment: Option<&TurnEnvironment>,
|
||||
) -> Result<Config, FunctionCallError> {
|
||||
let base_config = turn.config.clone();
|
||||
let mut config = (*base_config).clone();
|
||||
config.model = Some(turn.model_info.slug.clone());
|
||||
@@ -210,7 +218,7 @@ fn build_agent_shared_config(turn: &TurnContext) -> Result<Config, FunctionCallE
|
||||
{
|
||||
config.developer_instructions = Some(developer_instructions);
|
||||
}
|
||||
apply_spawn_agent_runtime_overrides(&mut config, turn)?;
|
||||
apply_spawn_agent_runtime_overrides(&mut config, turn, environment)?;
|
||||
|
||||
Ok(config)
|
||||
}
|
||||
@@ -228,11 +236,13 @@ pub(crate) fn reject_full_fork_agent_type_override(
|
||||
|
||||
/// Copies runtime-only turn state onto a child config before it is handed to `AgentControl`.
|
||||
///
|
||||
/// These values are chosen by the live turn rather than persisted config, so leaving them stale
|
||||
/// can make a child agent disagree with its parent about approval policy, cwd, or sandboxing.
|
||||
/// These values are chosen by the live turn and selected environment rather than persisted config,
|
||||
/// so leaving them stale can make a child agent disagree with its parent about approval policy,
|
||||
/// cwd, or sandboxing.
|
||||
pub(crate) fn apply_spawn_agent_runtime_overrides(
|
||||
config: &mut Config,
|
||||
turn: &TurnContext,
|
||||
environment: Option<&TurnEnvironment>,
|
||||
) -> Result<(), FunctionCallError> {
|
||||
config
|
||||
.permissions
|
||||
@@ -245,9 +255,12 @@ pub(crate) fn apply_spawn_agent_runtime_overrides(
|
||||
#[allow(deprecated)]
|
||||
let turn_cwd = turn.cwd.clone();
|
||||
config.cwd = turn_cwd;
|
||||
let permission_profile = environment
|
||||
.map(|environment| environment.permission_profile().clone())
|
||||
.unwrap_or_else(|| turn.permission_profile());
|
||||
config
|
||||
.permissions
|
||||
.set_permission_profile(turn.permission_profile())
|
||||
.set_permission_profile(permission_profile)
|
||||
.map_err(|err| {
|
||||
FunctionCallError::RespondToModel(format!("permission_profile is invalid: {err}"))
|
||||
})?;
|
||||
|
||||
@@ -3,6 +3,8 @@ use crate::StartThreadOptions;
|
||||
use crate::ThreadManager;
|
||||
use crate::config::AgentRoleConfig;
|
||||
use crate::config::DEFAULT_AGENT_MAX_DEPTH;
|
||||
use crate::config::PermissionProfileSnapshot;
|
||||
use crate::environment_selection::TurnEnvironmentState;
|
||||
use crate::function_tool::FunctionCallError;
|
||||
use crate::init_state_db;
|
||||
use crate::local_agent_graph_store_from_state_db;
|
||||
@@ -2264,7 +2266,7 @@ async fn spawn_agent_reapplies_runtime_sandbox_after_role_config() {
|
||||
config.approvals_reviewer = ApprovalsReviewer::AutoReview;
|
||||
config
|
||||
.permissions
|
||||
.set_permission_profile(expected_permission_profile.clone())
|
||||
.set_permission_profile(PermissionProfile::Disabled)
|
||||
.expect("test setup should allow updating permission profile");
|
||||
set_turn_config(&mut turn, config);
|
||||
let role_name = install_role_with_model_override(&mut turn).await;
|
||||
@@ -2272,11 +2274,26 @@ async fn spawn_agent_reapplies_runtime_sandbox_after_role_config() {
|
||||
crate::agent::role::apply_role_to_config(&mut role_config, Some(role_name.as_str()))
|
||||
.await
|
||||
.expect("non-empty role config should apply");
|
||||
let TurnEnvironmentState::Ready(environment) = turn
|
||||
.environments
|
||||
.environments
|
||||
.first_mut()
|
||||
.expect("parent environment should exist")
|
||||
else {
|
||||
panic!("parent environment should be ready");
|
||||
};
|
||||
environment.config.permission_profile =
|
||||
PermissionProfileSnapshot::legacy(expected_permission_profile.clone());
|
||||
assert_ne!(
|
||||
role_config.permissions.effective_permission_profile(),
|
||||
expected_permission_profile,
|
||||
"role config must discard the runtime permission override before it is reapplied"
|
||||
);
|
||||
assert_ne!(
|
||||
expected_permission_profile,
|
||||
turn.permission_profile(),
|
||||
"test requires an environment profile that differs from the thread profile"
|
||||
);
|
||||
|
||||
let invocation = invocation(
|
||||
Arc::new(session),
|
||||
@@ -4446,6 +4463,7 @@ async fn build_agent_spawn_config_uses_turn_context_values() {
|
||||
&file_system_sandbox_policy,
|
||||
network_sandbox_policy,
|
||||
);
|
||||
turn.environments.environments.clear();
|
||||
Arc::make_mut(&mut turn.config)
|
||||
.permissions
|
||||
.set_permission_profile(permission_profile)
|
||||
@@ -4456,7 +4474,8 @@ async fn build_agent_spawn_config_uses_turn_context_values() {
|
||||
.set(AskForApproval::OnRequest)
|
||||
.expect("approval policy set");
|
||||
|
||||
let config = build_agent_spawn_config(&base_instructions, &turn).expect("spawn config");
|
||||
let config = build_agent_spawn_config(&base_instructions, &turn, turn.environments.primary())
|
||||
.expect("spawn config");
|
||||
let mut expected = (*turn.config).clone();
|
||||
expected.base_instructions = Some(base_instructions.text);
|
||||
expected.model = Some(turn.model_info.slug.clone());
|
||||
@@ -4491,8 +4510,25 @@ async fn build_agent_resume_config_clears_base_instructions() {
|
||||
.approval_policy
|
||||
.set(AskForApproval::OnRequest)
|
||||
.expect("approval policy set");
|
||||
let environment_permission_profile =
|
||||
if turn.permission_profile() == PermissionProfile::read_only() {
|
||||
PermissionProfile::workspace_write()
|
||||
} else {
|
||||
PermissionProfile::read_only()
|
||||
};
|
||||
let TurnEnvironmentState::Ready(environment) = turn
|
||||
.environments
|
||||
.environments
|
||||
.first_mut()
|
||||
.expect("parent environment should exist")
|
||||
else {
|
||||
panic!("parent environment should be ready");
|
||||
};
|
||||
environment.config.permission_profile =
|
||||
PermissionProfileSnapshot::legacy(environment_permission_profile.clone());
|
||||
|
||||
let config = build_agent_resume_config(&turn).expect("resume config");
|
||||
let config =
|
||||
build_agent_resume_config(&turn, turn.environments.primary()).expect("resume config");
|
||||
|
||||
let mut expected = (*turn.config).clone();
|
||||
expected.base_instructions = None;
|
||||
@@ -4512,7 +4548,7 @@ async fn build_agent_resume_config_clears_base_instructions() {
|
||||
.expect("approval policy set");
|
||||
expected
|
||||
.permissions
|
||||
.set_permission_profile(turn.permission_profile())
|
||||
.set_permission_profile(environment_permission_profile)
|
||||
.expect("permission profile set");
|
||||
assert_eq!(config, expected);
|
||||
}
|
||||
|
||||
@@ -59,6 +59,7 @@ pub(crate) async fn handle_message_string_tool(
|
||||
let ToolInvocation {
|
||||
session,
|
||||
turn,
|
||||
step_context,
|
||||
call_id,
|
||||
source,
|
||||
..
|
||||
@@ -82,7 +83,8 @@ pub(crate) async fn handle_message_string_tool(
|
||||
let receiver_agent_path = receiver_agent.agent_path.clone().ok_or_else(|| {
|
||||
FunctionCallError::RespondToModel("target agent is missing an agent_path".to_string())
|
||||
})?;
|
||||
let resume_config = build_agent_resume_config(turn.as_ref())?;
|
||||
let resume_config =
|
||||
build_agent_resume_config(turn.as_ref(), step_context.environments.primary())?;
|
||||
session
|
||||
.services
|
||||
.agent_control
|
||||
|
||||
@@ -60,8 +60,11 @@ async fn handle_spawn_agent(
|
||||
|
||||
let session_source = turn.session_source.clone();
|
||||
let child_depth = next_thread_spawn_depth(&session_source);
|
||||
let mut config =
|
||||
build_agent_spawn_config(&session.get_base_instructions().await, turn.as_ref())?;
|
||||
let mut config = build_agent_spawn_config(
|
||||
&session.get_base_instructions().await,
|
||||
turn.as_ref(),
|
||||
step_context.environments.primary(),
|
||||
)?;
|
||||
if let Some(service_tier) = args.service_tier.as_ref() {
|
||||
config.service_tier = Some(service_tier.clone());
|
||||
}
|
||||
@@ -87,7 +90,11 @@ async fn handle_spawn_agent(
|
||||
args.service_tier.as_deref(),
|
||||
)
|
||||
.await?;
|
||||
apply_spawn_agent_runtime_overrides(&mut config, turn.as_ref())?;
|
||||
apply_spawn_agent_runtime_overrides(
|
||||
&mut config,
|
||||
turn.as_ref(),
|
||||
step_context.environments.primary(),
|
||||
)?;
|
||||
|
||||
let spawn_source = thread_spawn_source(
|
||||
session.thread_id,
|
||||
|
||||
@@ -178,7 +178,7 @@ async fn run_exec_like(args: RunExecLikeArgs) -> Result<FunctionToolOutput, Func
|
||||
.create_exec_approval_requirement_for_command(ExecApprovalRequest {
|
||||
command: &exec_params.command,
|
||||
approval_policy: turn.approval_policy(),
|
||||
permission_profile: turn.permission_profile(),
|
||||
permission_profile: turn_environment.permission_profile().clone(),
|
||||
windows_sandbox_level: turn.windows_sandbox_level,
|
||||
sandbox_permissions: if effective_additional_permissions.permissions_preapproved {
|
||||
codex_protocol::models::SandboxPermissions::UseDefault
|
||||
|
||||
@@ -105,7 +105,7 @@ impl ShellCommandHandler {
|
||||
&turn_context.config.permissions.shell_environment_policy,
|
||||
Some(session.thread_id),
|
||||
);
|
||||
let active_permission_profile = turn_context.config.permissions.active_permission_profile();
|
||||
let active_permission_profile = turn_environment.active_permission_profile();
|
||||
inject_permission_profile_env(&mut env, active_permission_profile.as_ref());
|
||||
let sandbox_permissions = resolve_sandbox_permissions(
|
||||
params.sandbox_permissions,
|
||||
|
||||
@@ -81,8 +81,8 @@ async fn shell_command_handler_to_exec_params_uses_selected_environment() {
|
||||
Arc::make_mut(&mut turn_context.config)
|
||||
.permissions
|
||||
.set_permission_profile_from_session_snapshot(PermissionProfileSnapshot::active(
|
||||
permission_profile,
|
||||
ActivePermissionProfile::new("test-profile"),
|
||||
permission_profile.clone(),
|
||||
ActivePermissionProfile::new("thread-profile"),
|
||||
))
|
||||
.expect("set active permission profile");
|
||||
|
||||
@@ -100,6 +100,7 @@ async fn shell_command_handler_to_exec_params_uses_selected_environment() {
|
||||
let expected_command = selected_shell.derive_exec_args(&command, /*use_login_shell*/ true);
|
||||
let selected_cwd = turn_context.config.cwd.join("selected-environment");
|
||||
let expected_cwd = selected_cwd.join("subdir");
|
||||
let active_permission_profile = ActivePermissionProfile::new("selected-profile");
|
||||
let selected_environment = TurnEnvironment::new(
|
||||
"selected-environment".to_string(),
|
||||
Arc::clone(
|
||||
@@ -114,19 +115,17 @@ async fn shell_command_handler_to_exec_params_uses_selected_environment() {
|
||||
Some(selected_shell),
|
||||
EnvironmentConfig {
|
||||
allow_login_shell: true,
|
||||
permission_profile: turn_context
|
||||
.config
|
||||
.permissions
|
||||
.permission_profile_state()
|
||||
.snapshot(),
|
||||
permission_profile: PermissionProfileSnapshot::active(
|
||||
permission_profile,
|
||||
active_permission_profile.clone(),
|
||||
),
|
||||
},
|
||||
);
|
||||
let mut expected_env = create_env(
|
||||
&turn_context.config.permissions.shell_environment_policy,
|
||||
Some(session.thread_id),
|
||||
);
|
||||
let active_permission_profile = turn_context.config.permissions.active_permission_profile();
|
||||
inject_permission_profile_env(&mut expected_env, active_permission_profile.as_ref());
|
||||
inject_permission_profile_env(&mut expected_env, Some(&active_permission_profile));
|
||||
|
||||
let params = ShellCommandToolCallParams {
|
||||
command,
|
||||
@@ -154,9 +153,7 @@ async fn shell_command_handler_to_exec_params_uses_selected_environment() {
|
||||
assert_eq!(exec_params.env, expected_env);
|
||||
assert_eq!(
|
||||
exec_params.env.get(CODEX_PERMISSION_PROFILE_ENV_VAR),
|
||||
active_permission_profile
|
||||
.as_ref()
|
||||
.map(|profile| &profile.id)
|
||||
Some(&active_permission_profile.id)
|
||||
);
|
||||
assert_eq!(exec_params.network, turn_context.network);
|
||||
assert_eq!(
|
||||
|
||||
@@ -156,7 +156,7 @@ impl ExecCommandHandler {
|
||||
// permissions config below. Consult the configured platform-sandbox requirement before
|
||||
// deciding whether parsing may continue without that base path.
|
||||
let sandbox = SandboxManager::new().select_initial(
|
||||
&turn.permission_profile(),
|
||||
turn_environment.permission_profile(),
|
||||
SandboxablePreference::Auto,
|
||||
turn.windows_sandbox_level,
|
||||
turn.network.is_some(),
|
||||
|
||||
@@ -244,6 +244,7 @@ impl ToolOutput for ViewImageOutput {
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::config::PermissionProfileSnapshot;
|
||||
use crate::environment_selection::TurnEnvironmentState;
|
||||
use crate::session::step_context::StepContext;
|
||||
use crate::session::tests::make_session_and_context;
|
||||
@@ -318,8 +319,14 @@ mod tests {
|
||||
std::fs::write(image_path.as_path(), b"not a real image").expect("write test image");
|
||||
Arc::make_mut(&mut turn.config)
|
||||
.permissions
|
||||
.set_permission_profile(PermissionProfile::read_only())
|
||||
.expect("test setup should allow updating permission profile");
|
||||
.set_permission_profile(PermissionProfile::Disabled)
|
||||
.expect("set thread permission profile");
|
||||
let TurnEnvironmentState::Ready(environment) = &mut turn.environments.environments[0]
|
||||
else {
|
||||
panic!("primary environment should be ready");
|
||||
};
|
||||
environment.config.permission_profile =
|
||||
PermissionProfileSnapshot::legacy(PermissionProfile::read_only());
|
||||
let turn = Arc::new(turn);
|
||||
|
||||
let result = ViewImageHandler::default()
|
||||
@@ -386,10 +393,12 @@ mod tests {
|
||||
replace_primary_environment_cwd(&mut turn, image_cwd.clone());
|
||||
let image_path = image_cwd.join("image.png");
|
||||
std::fs::write(image_path.as_path(), b"not a real image").expect("write test image");
|
||||
Arc::make_mut(&mut turn.config)
|
||||
.permissions
|
||||
.set_permission_profile(PermissionProfile::Disabled)
|
||||
.expect("set permission profile");
|
||||
let TurnEnvironmentState::Ready(environment) = &mut turn.environments.environments[0]
|
||||
else {
|
||||
panic!("primary environment should be ready");
|
||||
};
|
||||
environment.config.permission_profile =
|
||||
PermissionProfileSnapshot::legacy(PermissionProfile::Disabled);
|
||||
let turn = Arc::new(turn);
|
||||
|
||||
let result = ViewImageHandler::default()
|
||||
|
||||
@@ -7,6 +7,7 @@ use crate::guardian::routes_approval_to_guardian;
|
||||
use crate::hook_runtime::run_permission_request_hooks;
|
||||
use crate::network_policy_decision::denied_network_policy_message;
|
||||
use crate::session::session::Session;
|
||||
use crate::session::turn_context::TurnEnvironment;
|
||||
use crate::tools::events::truncate_rejection_message;
|
||||
use crate::tools::sandboxing::PermissionRequestPayload;
|
||||
use crate::tools::sandboxing::ToolError;
|
||||
@@ -60,6 +61,7 @@ pub(crate) struct NetworkApprovalSpec {
|
||||
pub trigger: GuardianNetworkAccessTrigger,
|
||||
pub command: String,
|
||||
pub environment_id: String,
|
||||
pub permission_profile: PermissionProfile,
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug)]
|
||||
@@ -265,6 +267,7 @@ struct ActiveNetworkApprovalCall {
|
||||
trigger: GuardianNetworkAccessTrigger,
|
||||
command: String,
|
||||
environment_id: String,
|
||||
permission_profile: PermissionProfile,
|
||||
cancellation_token: CancellationToken,
|
||||
}
|
||||
|
||||
@@ -398,28 +401,11 @@ impl NetworkApprovalService {
|
||||
other_approved_hosts.extend(approved_hosts.iter().cloned());
|
||||
}
|
||||
|
||||
async fn register_call(
|
||||
&self,
|
||||
registration_id: String,
|
||||
turn_id: String,
|
||||
trigger: GuardianNetworkAccessTrigger,
|
||||
command: String,
|
||||
environment_id: String,
|
||||
cancellation_token: CancellationToken,
|
||||
) {
|
||||
async fn register_call(&self, call: ActiveNetworkApprovalCall) {
|
||||
let mut calls = self.calls.lock().await;
|
||||
let key = registration_id.clone();
|
||||
calls.active_calls.insert(
|
||||
key,
|
||||
Arc::new(ActiveNetworkApprovalCall {
|
||||
registration_id,
|
||||
turn_id,
|
||||
trigger,
|
||||
command,
|
||||
environment_id,
|
||||
cancellation_token,
|
||||
}),
|
||||
);
|
||||
calls
|
||||
.active_calls
|
||||
.insert(call.registration_id.clone(), Arc::new(call));
|
||||
}
|
||||
|
||||
pub(crate) async fn unregister_call(&self, registration_id: &str) {
|
||||
@@ -711,7 +697,17 @@ impl NetworkApprovalService {
|
||||
.map(|call| call.cancellation_token.clone()),
|
||||
);
|
||||
|
||||
if !permission_profile_allows_network_approval_flow(&turn_context.permission_profile()) {
|
||||
let permission_profile = owner_call
|
||||
.as_ref()
|
||||
.map(|call| &call.permission_profile)
|
||||
.or_else(|| {
|
||||
turn_context
|
||||
.environments
|
||||
.turn_environments()
|
||||
.find(|environment| environment.environment_id == environment_id)
|
||||
.map(TurnEnvironment::permission_profile)
|
||||
});
|
||||
if !permission_profile.is_some_and(permission_profile_allows_network_approval_flow) {
|
||||
if let Some(owner_call) = owner_call.as_ref() {
|
||||
self.record_call_outcome(
|
||||
&owner_call.registration_id,
|
||||
@@ -1084,6 +1080,7 @@ pub(crate) async fn begin_network_approval(
|
||||
trigger,
|
||||
command,
|
||||
environment_id,
|
||||
permission_profile,
|
||||
} = match spec {
|
||||
Some(spec) => spec,
|
||||
None => return Ok(None),
|
||||
@@ -1108,14 +1105,15 @@ pub(crate) async fn begin_network_approval(
|
||||
session
|
||||
.services
|
||||
.network_approval
|
||||
.register_call(
|
||||
registration_id.clone(),
|
||||
turn_id.to_string(),
|
||||
.register_call(ActiveNetworkApprovalCall {
|
||||
registration_id: registration_id.clone(),
|
||||
turn_id: turn_id.to_string(),
|
||||
trigger,
|
||||
command,
|
||||
environment_id,
|
||||
cancellation_token.clone(),
|
||||
)
|
||||
permission_profile,
|
||||
cancellation_token: cancellation_token.clone(),
|
||||
})
|
||||
.await;
|
||||
|
||||
Ok(Some(ActiveNetworkApproval {
|
||||
|
||||
@@ -474,10 +474,10 @@ async fn register_call_with_default_shell_trigger(
|
||||
) -> CancellationToken {
|
||||
let cancellation_token = CancellationToken::new();
|
||||
service
|
||||
.register_call(
|
||||
registration_id.to_string(),
|
||||
"turn-1".to_string(),
|
||||
GuardianNetworkAccessTrigger {
|
||||
.register_call(ActiveNetworkApprovalCall {
|
||||
registration_id: registration_id.to_string(),
|
||||
turn_id: "turn-1".to_string(),
|
||||
trigger: GuardianNetworkAccessTrigger {
|
||||
call_id: "call-1".to_string(),
|
||||
tool_name: "shell_command".to_string(),
|
||||
command: vec!["curl".to_string(), "https://example.com".to_string()],
|
||||
@@ -487,10 +487,11 @@ async fn register_call_with_default_shell_trigger(
|
||||
justification: None,
|
||||
tty: None,
|
||||
},
|
||||
"curl https://example.com".to_string(),
|
||||
"local".to_string(),
|
||||
cancellation_token.clone(),
|
||||
)
|
||||
command: "curl https://example.com".to_string(),
|
||||
environment_id: "local".to_string(),
|
||||
permission_profile: PermissionProfile::workspace_write(),
|
||||
cancellation_token: cancellation_token.clone(),
|
||||
})
|
||||
.await;
|
||||
cancellation_token
|
||||
}
|
||||
@@ -510,14 +511,15 @@ async fn active_call_preserves_triggering_command_context() {
|
||||
};
|
||||
|
||||
service
|
||||
.register_call(
|
||||
"registration-1".to_string(),
|
||||
"turn-1".to_string(),
|
||||
expected.clone(),
|
||||
"curl https://example.com".to_string(),
|
||||
"remote".to_string(),
|
||||
CancellationToken::new(),
|
||||
)
|
||||
.register_call(ActiveNetworkApprovalCall {
|
||||
registration_id: "registration-1".to_string(),
|
||||
turn_id: "turn-1".to_string(),
|
||||
trigger: expected.clone(),
|
||||
command: "curl https://example.com".to_string(),
|
||||
environment_id: "remote".to_string(),
|
||||
permission_profile: PermissionProfile::workspace_write(),
|
||||
cancellation_token: CancellationToken::new(),
|
||||
})
|
||||
.await;
|
||||
|
||||
let call = service
|
||||
|
||||
@@ -35,7 +35,6 @@ use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::ReviewDecision;
|
||||
use codex_sandboxing::SandboxManager;
|
||||
use codex_sandboxing::SandboxType;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use std::time::Instant;
|
||||
|
||||
pub(crate) struct ToolOrchestrator {
|
||||
@@ -152,15 +151,10 @@ impl ToolOrchestrator {
|
||||
// 1) Approval
|
||||
let mut already_approved = false;
|
||||
|
||||
let workspace_roots = tool.workspace_roots(req);
|
||||
let permission_profile = turn_ctx.config.permissions.permission_profile();
|
||||
let materialized_workspace_roots = workspace_roots
|
||||
.iter()
|
||||
.filter_map(|workspace_root| workspace_root.to_abs_path().ok())
|
||||
.collect::<Vec<_>>();
|
||||
let permissions = permission_profile
|
||||
.clone()
|
||||
.materialize_project_roots_with_workspace_roots(&materialized_workspace_roots);
|
||||
let environment = tool.turn_environment(req);
|
||||
let workspace_roots = environment.workspace_roots();
|
||||
let permission_profile = environment.permission_profile();
|
||||
let permissions = environment.permission_profile_with_workspace_roots();
|
||||
let file_system_sandbox_policy = permissions.file_system_sandbox_policy();
|
||||
let requirement = tool.exec_approval_requirement(req).unwrap_or_else(|| {
|
||||
default_exec_approval_requirement(approval_policy, &file_system_sandbox_policy)
|
||||
@@ -247,11 +241,10 @@ impl ToolOrchestrator {
|
||||
|
||||
// Platform-specific flag gating is handled by SandboxManager::select_initial.
|
||||
let use_legacy_landlock = turn_ctx.config.features.use_legacy_landlock();
|
||||
#[allow(deprecated)]
|
||||
let sandbox_policy_cwd = tool
|
||||
.sandbox_cwd(req)
|
||||
.cloned()
|
||||
.unwrap_or_else(|| PathUri::from_abs_path(&turn_ctx.cwd));
|
||||
.unwrap_or_else(|| environment.cwd().clone());
|
||||
let initial_attempt = SandboxAttempt {
|
||||
sandbox: initial_sandbox,
|
||||
sandbox_requested,
|
||||
|
||||
@@ -215,8 +215,8 @@ impl Approvable<ApplyPatchRequest> for ApplyPatchRuntime {
|
||||
}
|
||||
|
||||
impl ToolRuntime<ApplyPatchRequest, ApplyPatchRuntimeOutput> for ApplyPatchRuntime {
|
||||
fn workspace_roots<'a>(&self, req: &'a ApplyPatchRequest) -> &'a [PathUri] {
|
||||
req.turn_environment.workspace_roots()
|
||||
fn turn_environment<'a>(&self, req: &'a ApplyPatchRequest) -> &'a TurnEnvironment {
|
||||
&req.turn_environment
|
||||
}
|
||||
|
||||
fn sandbox_cwd<'a>(&self, req: &'a ApplyPatchRequest) -> Option<&'a PathUri> {
|
||||
|
||||
@@ -213,8 +213,8 @@ impl Approvable<ShellRequest> for ShellRuntime {
|
||||
}
|
||||
|
||||
impl ToolRuntime<ShellRequest, ExecToolCallOutput> for ShellRuntime {
|
||||
fn workspace_roots<'a>(&self, req: &'a ShellRequest) -> &'a [PathUri] {
|
||||
req.turn_environment.workspace_roots()
|
||||
fn turn_environment<'a>(&self, req: &'a ShellRequest) -> &'a TurnEnvironment {
|
||||
&req.turn_environment
|
||||
}
|
||||
|
||||
fn network_approval_spec(
|
||||
@@ -222,7 +222,10 @@ impl ToolRuntime<ShellRequest, ExecToolCallOutput> for ShellRuntime {
|
||||
req: &ShellRequest,
|
||||
ctx: &ToolCtx,
|
||||
) -> Option<NetworkApprovalSpec> {
|
||||
let file_system_sandbox_policy = ctx.turn.file_system_sandbox_policy();
|
||||
let file_system_sandbox_policy = req
|
||||
.turn_environment
|
||||
.permission_profile()
|
||||
.file_system_sandbox_policy();
|
||||
let sandbox_permissions = sandbox_permissions_preserving_denied_reads(
|
||||
req.sandbox_permissions,
|
||||
&file_system_sandbox_policy,
|
||||
@@ -244,6 +247,7 @@ impl ToolRuntime<ShellRequest, ExecToolCallOutput> for ShellRuntime {
|
||||
},
|
||||
command: req.hook_command.clone(),
|
||||
environment_id: req.turn_environment.environment_id.clone(),
|
||||
permission_profile: req.turn_environment.permission_profile().clone(),
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -265,8 +265,8 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
|
||||
}
|
||||
|
||||
impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRuntime<'a> {
|
||||
fn workspace_roots<'b>(&self, req: &'b UnifiedExecRequest) -> &'b [PathUri] {
|
||||
req.turn_environment.workspace_roots()
|
||||
fn turn_environment<'b>(&self, req: &'b UnifiedExecRequest) -> &'b TurnEnvironment {
|
||||
&req.turn_environment
|
||||
}
|
||||
|
||||
fn sandbox_cwd<'b>(&self, req: &'b UnifiedExecRequest) -> Option<&'b PathUri> {
|
||||
@@ -278,7 +278,10 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
|
||||
req: &UnifiedExecRequest,
|
||||
ctx: &ToolCtx,
|
||||
) -> Option<NetworkApprovalSpec> {
|
||||
let file_system_sandbox_policy = ctx.turn.file_system_sandbox_policy();
|
||||
let file_system_sandbox_policy = req
|
||||
.turn_environment
|
||||
.permission_profile()
|
||||
.file_system_sandbox_policy();
|
||||
let sandbox_permissions = sandbox_permissions_preserving_denied_reads(
|
||||
req.sandbox_permissions,
|
||||
&file_system_sandbox_policy,
|
||||
@@ -300,6 +303,7 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
|
||||
},
|
||||
command: req.hook_command.clone(),
|
||||
environment_id: req.turn_environment.environment_id.clone(),
|
||||
permission_profile: req.turn_environment.permission_profile().clone(),
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -8,6 +8,7 @@ use crate::sandboxing::ExecOptions;
|
||||
use crate::sandboxing::SandboxPermissions;
|
||||
use crate::session::session::Session;
|
||||
use crate::session::turn_context::TurnContext;
|
||||
use crate::session::turn_context::TurnEnvironment;
|
||||
use crate::state::SessionServices;
|
||||
use crate::tools::hook_names::HookToolName;
|
||||
use crate::tools::network_approval::NetworkApprovalSpec;
|
||||
@@ -394,7 +395,7 @@ pub(crate) enum ToolError {
|
||||
}
|
||||
|
||||
pub(crate) trait ToolRuntime<Req, Out>: Approvable<Req> + Sandboxable {
|
||||
fn workspace_roots<'a>(&self, req: &'a Req) -> &'a [PathUri];
|
||||
fn turn_environment<'a>(&self, req: &'a Req) -> &'a TurnEnvironment;
|
||||
|
||||
fn network_approval_spec(&self, _req: &Req, _ctx: &ToolCtx) -> Option<NetworkApprovalSpec> {
|
||||
None
|
||||
|
||||
@@ -1175,7 +1175,7 @@ impl UnifiedExecProcessManager {
|
||||
CODEX_THREAD_ID_ENV_VAR.to_string(),
|
||||
context.session.thread_id.to_string(),
|
||||
);
|
||||
let active_permission_profile = context.turn.config.permissions.active_permission_profile();
|
||||
let active_permission_profile = request.turn_environment.active_permission_profile();
|
||||
inject_permission_profile_env(&mut env, active_permission_profile.as_ref());
|
||||
let env = apply_unified_exec_env(env);
|
||||
let exec_server_env_config = ExecServerEnvConfig {
|
||||
@@ -1193,7 +1193,7 @@ impl UnifiedExecProcessManager {
|
||||
.create_exec_approval_requirement_for_command(ExecApprovalRequest {
|
||||
command: &request.command,
|
||||
approval_policy: context.turn.approval_policy(),
|
||||
permission_profile: context.turn.permission_profile(),
|
||||
permission_profile: request.turn_environment.permission_profile().clone(),
|
||||
windows_sandbox_level: context.turn.windows_sandbox_level,
|
||||
sandbox_permissions: if request.additional_permissions_preapproved {
|
||||
crate::sandboxing::SandboxPermissions::UseDefault
|
||||
|
||||
@@ -5,6 +5,7 @@ use codex_exec_server::RemoveOptions;
|
||||
use codex_features::Feature;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::permissions::NetworkSandboxPolicy;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
#[cfg(windows)]
|
||||
use core_test_support::PathExt;
|
||||
@@ -65,6 +66,14 @@ fn outside_workspace_path(test: &TestCodex, file_name: &str) -> Result<PathUri>
|
||||
.map_err(Into::into)
|
||||
}
|
||||
|
||||
fn sibling_workspace_root_name(cwd: &AbsolutePathBuf, suffix: &str) -> String {
|
||||
let cwd_name = cwd
|
||||
.file_name()
|
||||
.expect("test workspace should have a file name")
|
||||
.to_string_lossy();
|
||||
format!("{cwd_name}-{suffix}")
|
||||
}
|
||||
|
||||
fn command_arguments(path: &str, contents: &str) -> Result<String> {
|
||||
let (shell, command) = match test_target_os() {
|
||||
TestTargetOs::Linux | TestTargetOs::MacOs => {
|
||||
@@ -188,6 +197,105 @@ async fn workspace_roots_allow_file_and_command_writes() -> Result<()> {
|
||||
remove_files(&test, &[&patch_path, &command_path]).await
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn workspace_roots_allow_apply_patch_in_secondary_root() -> Result<()> {
|
||||
const SECONDARY_ROOT_NAME: &str = "secondary-workspace-root";
|
||||
|
||||
skip_if_wine_exec!(
|
||||
Ok(()),
|
||||
"Wine does not emulate Windows restricted-token and ACL sandbox semantics"
|
||||
);
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let mut builder = test_codex()
|
||||
.with_config(|config| {
|
||||
#[cfg(windows)]
|
||||
{
|
||||
config.cwd = dunce::canonicalize(config.cwd.as_path())
|
||||
.expect("primary workspace root should be canonicalizable")
|
||||
.abs();
|
||||
}
|
||||
let secondary_root = config
|
||||
.cwd
|
||||
.parent()
|
||||
.expect("workspace should have a parent")
|
||||
.join(sibling_workspace_root_name(
|
||||
&config.cwd,
|
||||
SECONDARY_ROOT_NAME,
|
||||
));
|
||||
config.workspace_roots = vec![config.cwd.clone(), secondary_root];
|
||||
config.set_windows_sandbox_enabled(/*value*/ true);
|
||||
})
|
||||
.with_workspace_setup(|cwd, fs| async move {
|
||||
let secondary_root = cwd
|
||||
.parent()
|
||||
.context("workspace should have a parent")?
|
||||
.join(sibling_workspace_root_name(&cwd, SECONDARY_ROOT_NAME));
|
||||
fs.create_directory(
|
||||
&PathUri::from_abs_path(&secondary_root),
|
||||
CreateDirectoryOptions { recursive: true },
|
||||
/*sandbox*/ None,
|
||||
)
|
||||
.await?;
|
||||
Ok(())
|
||||
});
|
||||
let test = builder.build_with_auto_env(&server).await?;
|
||||
let secondary_root_name = sibling_workspace_root_name(&test.config.cwd, SECONDARY_ROOT_NAME);
|
||||
let secondary_root = PathUri::from_abs_path(&test.config.cwd)
|
||||
.parent()
|
||||
.context("workspace should have a parent")?
|
||||
.join(&secondary_root_name)?;
|
||||
let patch = format!(
|
||||
"*** Begin Patch\n*** Add File: ../{secondary_root_name}/secondary-root-patch.txt\n+secondary root\n*** End Patch\n"
|
||||
);
|
||||
let response_mock = mount_sse_sequence(
|
||||
&server,
|
||||
vec![
|
||||
sse(vec![
|
||||
ev_response_created("resp-1"),
|
||||
ev_apply_patch_custom_tool_call(PATCH_CALL_ID, &patch),
|
||||
ev_completed("resp-1"),
|
||||
]),
|
||||
sse(vec![
|
||||
ev_response_created("resp-2"),
|
||||
ev_assistant_message("msg-1", "done"),
|
||||
ev_completed("resp-2"),
|
||||
]),
|
||||
],
|
||||
)
|
||||
.await;
|
||||
|
||||
submit_workspace_turn(&test, "write in the secondary workspace root").await?;
|
||||
|
||||
let request = response_mock
|
||||
.last_request()
|
||||
.context("model should receive the apply_patch result")?;
|
||||
let (patch_output, patch_success) = request
|
||||
.custom_tool_call_output_content_and_success(PATCH_CALL_ID)
|
||||
.context("patch result should be present")?;
|
||||
assert_ne!(patch_success, Some(false), "{patch_output:?}");
|
||||
assert!(
|
||||
!patch_output
|
||||
.as_deref()
|
||||
.is_some_and(|output| output.contains("patch rejected")),
|
||||
"{patch_output:?}"
|
||||
);
|
||||
let patched_file = secondary_root.join("secondary-root-patch.txt")?;
|
||||
assert_eq!(read_file(&test, &patched_file).await?, "secondary root\n");
|
||||
|
||||
test.fs()
|
||||
.remove(
|
||||
&secondary_root,
|
||||
RemoveOptions {
|
||||
recursive: true,
|
||||
force: true,
|
||||
},
|
||||
/*sandbox*/ None,
|
||||
)
|
||||
.await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn workspace_roots_allow_patches_but_protect_metadata_directories() -> Result<()> {
|
||||
const PATCH_CONTENTS: &str = "workspace root patch access";
|
||||
|
||||
Reference in New Issue
Block a user