mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
sandboxing: migrate cwd inputs to PathUri (#27816)
## Why Sandbox cwd values can cross app-server and exec-server host boundaries. They should retain URI semantics until the receiving host validates them instead of being interpreted early as native paths. ## What - Carry `PathUri` through filesystem sandbox contexts, sandbox commands, and transform inputs. - Convert command and policy cwd once in `SandboxManager::transform`, then keep launch requests native. - Preserve sandbox cwd over remote filesystem transport and reject non-native URIs without fallback. - Cache paired native/URI turn-environment cwd values during migration, with immutable access to keep them synchronized. - Extend existing protocol, forwarding, transform, and core runtime tests.
This commit is contained in:
committed by
GitHub
Unverified
parent
84520225b9
commit
52a50aec70
@@ -314,7 +314,7 @@ impl Session {
|
||||
let mcp_runtime_context = match turn_context.environments.primary() {
|
||||
Some(turn_environment) => McpRuntimeContext::new(
|
||||
Arc::clone(&self.services.environment_manager),
|
||||
turn_environment.cwd.to_path_buf(),
|
||||
turn_environment.cwd().to_path_buf(),
|
||||
),
|
||||
None => McpRuntimeContext::new(
|
||||
Arc::clone(&self.services.environment_manager),
|
||||
|
||||
@@ -1135,7 +1135,7 @@ impl Session {
|
||||
let mcp_runtime_context = match turn_environment {
|
||||
Some(turn_environment) => McpRuntimeContext::new(
|
||||
Arc::clone(&sess.services.environment_manager),
|
||||
turn_environment.cwd.to_path_buf(),
|
||||
turn_environment.cwd().to_path_buf(),
|
||||
),
|
||||
None => McpRuntimeContext::new(
|
||||
Arc::clone(&sess.services.environment_manager),
|
||||
|
||||
@@ -4035,12 +4035,15 @@ fn turn_environments_for_tests(
|
||||
cwd: &codex_utils_absolute_path::AbsolutePathBuf,
|
||||
) -> crate::environment_selection::ResolvedTurnEnvironments {
|
||||
crate::environment_selection::ResolvedTurnEnvironments {
|
||||
turn_environments: vec![TurnEnvironment {
|
||||
environment_id: codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(),
|
||||
environment: Arc::clone(environment),
|
||||
cwd: cwd.clone(),
|
||||
shell: None,
|
||||
}],
|
||||
turn_environments: vec![
|
||||
TurnEnvironment::new(
|
||||
codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(),
|
||||
Arc::clone(environment),
|
||||
cwd.clone(),
|
||||
/*shell*/ None,
|
||||
)
|
||||
.expect("turn environment"),
|
||||
],
|
||||
}
|
||||
}
|
||||
|
||||
@@ -5656,8 +5659,14 @@ async fn request_permissions_tool_resolves_relative_paths_against_selected_envir
|
||||
mcp_elicitations: true,
|
||||
}))
|
||||
.expect("test setup should allow updating approval policy");
|
||||
turn_context_mut.environments.turn_environments[0].environment_id = "remote".to_string();
|
||||
turn_context_mut.environments.turn_environments[0].cwd = environment_cwd.clone();
|
||||
let current_environment = turn_context_mut.environments.turn_environments[0].clone();
|
||||
turn_context_mut.environments.turn_environments[0] = TurnEnvironment::new(
|
||||
"remote".to_string(),
|
||||
current_environment.environment,
|
||||
environment_cwd.clone(),
|
||||
current_environment.shell,
|
||||
)
|
||||
.expect("environment cwd URI");
|
||||
|
||||
let call_id = "call-1".to_string();
|
||||
let handler = RequestPermissionsHandler;
|
||||
@@ -6246,15 +6255,15 @@ async fn primary_environment_uses_first_turn_environment() {
|
||||
let first_environment = turn_context.environments.turn_environments[0].clone();
|
||||
#[allow(deprecated)]
|
||||
let second_cwd = turn_context.cwd.join("second");
|
||||
turn_context
|
||||
.environments
|
||||
.turn_environments
|
||||
.push(TurnEnvironment {
|
||||
environment_id: "second".to_string(),
|
||||
environment: Arc::clone(&first_environment.environment),
|
||||
cwd: second_cwd.clone(),
|
||||
shell: None,
|
||||
});
|
||||
turn_context.environments.turn_environments.push(
|
||||
TurnEnvironment::new(
|
||||
"second".to_string(),
|
||||
Arc::clone(&first_environment.environment),
|
||||
second_cwd.clone(),
|
||||
/*shell*/ None,
|
||||
)
|
||||
.expect("turn environment"),
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
turn_context
|
||||
@@ -6271,13 +6280,13 @@ async fn primary_environment_uses_first_turn_environment() {
|
||||
.iter()
|
||||
.find(|environment| environment.environment_id == "second")
|
||||
.expect("second environment")
|
||||
.cwd,
|
||||
second_cwd
|
||||
.cwd(),
|
||||
&second_cwd
|
||||
);
|
||||
assert_eq!(turn_context.environments.turn_environments.len(), 2);
|
||||
assert_eq!(
|
||||
turn_context.environments.turn_environments[1].cwd,
|
||||
second_cwd
|
||||
turn_context.environments.turn_environments[1].cwd(),
|
||||
&second_cwd
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -416,10 +416,10 @@ async fn turn_diff_display_roots(turn_context: &TurnContext) -> Vec<(String, Pat
|
||||
for turn_environment in &turn_context.environments.turn_environments {
|
||||
let root = get_git_repo_root_with_fs(
|
||||
turn_environment.environment.get_filesystem().as_ref(),
|
||||
&turn_environment.cwd,
|
||||
turn_environment.cwd(),
|
||||
)
|
||||
.await
|
||||
.unwrap_or_else(|| turn_environment.cwd.clone())
|
||||
.unwrap_or_else(|| turn_environment.cwd().clone())
|
||||
.into_path_buf();
|
||||
display_roots.push((turn_environment.environment_id.clone(), root));
|
||||
}
|
||||
@@ -630,7 +630,7 @@ async fn build_extension_turn_input_items(
|
||||
.enumerate()
|
||||
.map(|(index, environment)| TurnInputEnvironment {
|
||||
environment_id: environment.environment_id.clone(),
|
||||
cwd: environment.cwd.as_path().to_path_buf(),
|
||||
cwd: environment.cwd().as_path().to_path_buf(),
|
||||
is_primary: index == 0,
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
@@ -17,6 +17,7 @@ use codex_protocol::protocol::TurnEnvironmentSelection;
|
||||
use codex_sandboxing::compatibility_sandbox_policy_for_permission_profile;
|
||||
use codex_sandboxing::policy_transforms::effective_file_system_sandbox_policy;
|
||||
use codex_sandboxing::policy_transforms::effective_network_sandbox_policy;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use std::sync::atomic::AtomicBool;
|
||||
use std::sync::atomic::Ordering;
|
||||
|
||||
@@ -39,11 +40,45 @@ impl TurnSkillsContext {
|
||||
pub(crate) struct TurnEnvironment {
|
||||
pub(crate) environment_id: String,
|
||||
pub(crate) environment: Arc<Environment>,
|
||||
pub(crate) cwd: AbsolutePathBuf,
|
||||
// Keep both representations together while cwd consumers migrate to URI semantics. Keeping
|
||||
// them synchronized means neither representation can be exposed through a mutable reference;
|
||||
// updates must rebuild the validated pair through `TurnEnvironment::new`. Once
|
||||
// `TurnEnvironment::cwd` itself becomes a `PathUri`, convert only at native filesystem and
|
||||
// process-launch boundaries and remove this paired migration state.
|
||||
cwd: AbsolutePathBuf,
|
||||
cwd_uri: PathUri,
|
||||
pub(crate) shell: Option<shell::Shell>,
|
||||
}
|
||||
|
||||
impl TurnEnvironment {
|
||||
pub(crate) fn new(
|
||||
environment_id: String,
|
||||
environment: Arc<Environment>,
|
||||
cwd: AbsolutePathBuf,
|
||||
shell: Option<shell::Shell>,
|
||||
) -> CodexResult<Self> {
|
||||
let cwd_uri = PathUri::from_abs_path(&cwd).map_err(|_| {
|
||||
CodexErr::InvalidRequest(
|
||||
"turn environment cwd cannot be represented as a file URI".to_string(),
|
||||
)
|
||||
})?;
|
||||
Ok(Self {
|
||||
environment_id,
|
||||
environment,
|
||||
cwd,
|
||||
cwd_uri,
|
||||
shell,
|
||||
})
|
||||
}
|
||||
|
||||
pub(crate) fn cwd(&self) -> &AbsolutePathBuf {
|
||||
&self.cwd
|
||||
}
|
||||
|
||||
pub(crate) fn cwd_uri(&self) -> &PathUri {
|
||||
&self.cwd_uri
|
||||
}
|
||||
|
||||
pub(crate) fn selection(&self) -> TurnEnvironmentSelection {
|
||||
TurnEnvironmentSelection {
|
||||
environment_id: self.environment_id.clone(),
|
||||
@@ -291,7 +326,7 @@ impl TurnContext {
|
||||
pub(crate) fn file_system_sandbox_context(
|
||||
&self,
|
||||
additional_permissions: Option<AdditionalPermissionProfile>,
|
||||
cwd: &AbsolutePathBuf,
|
||||
cwd: &PathUri,
|
||||
) -> FileSystemSandboxContext {
|
||||
let (base_file_system_sandbox_policy, base_network_sandbox_policy) =
|
||||
self.permission_profile.to_runtime_permissions();
|
||||
@@ -712,7 +747,7 @@ impl Session {
|
||||
let primary_turn_environment = turn_environments.primary().cloned();
|
||||
let cwd = primary_turn_environment
|
||||
.as_ref()
|
||||
.map(|turn_environment| turn_environment.cwd.clone())
|
||||
.map(|turn_environment| turn_environment.cwd().clone())
|
||||
.unwrap_or_else(|| session_configuration.cwd().clone());
|
||||
let per_turn_config = Self::build_per_turn_config(&session_configuration, cwd.clone());
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user