mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Disable env-bound tools when exec server is none (#16349)
## Summary - make `CODEX_EXEC_SERVER_URL=none` map to an explicit disabled environment mode instead of inferring from a missing URL - expose environment capabilities (`exec_enabled`, `filesystem_enabled`) so tool building can gate behavior explicitly and future multi-environment work has a clearer seam - suppress env-backed tools when the relevant capability is unavailable, including exec tools, `js_repl`, `apply_patch`, `list_dir`, and `view_image` - keep handler/runtime backstops so disabled environments still reject execution if a tool path somehow bypasses registration ## Testing - `just fmt` - `cargo test -p codex-exec-server` - `cargo test -p codex-tools disabled_environment_omits_environment_backed_tools` - `cargo test -p codex-tools environment_capabilities_gate_exec_and_filesystem_tools_independently` - remote devbox Bazel build via `codex-applied-devbox`: `//codex-rs/cli:cli`
This commit is contained in:
@@ -862,7 +862,7 @@ pub(crate) struct TurnContext {
|
||||
pub(crate) reasoning_effort: Option<ReasoningEffortConfig>,
|
||||
pub(crate) reasoning_summary: ReasoningSummaryConfig,
|
||||
pub(crate) session_source: SessionSource,
|
||||
pub(crate) environment: Arc<Environment>,
|
||||
pub(crate) environment: Option<Arc<Environment>>,
|
||||
/// The session's absolute working directory. All relative paths provided
|
||||
/// by the model as well as sandbox policies are resolved against this path
|
||||
/// instead of `std::env::current_dir()`.
|
||||
@@ -958,6 +958,7 @@ impl TurnContext {
|
||||
.with_unified_exec_shell_mode(self.tools_config.unified_exec_shell_mode.clone())
|
||||
.with_web_search_config(self.tools_config.web_search_config.clone())
|
||||
.with_allow_login_shell(self.tools_config.allow_login_shell)
|
||||
.with_has_environment(self.tools_config.has_environment)
|
||||
.with_agent_type_description(crate::agent::role::spawn_tool_spec::build(
|
||||
&config.agent_roles,
|
||||
));
|
||||
@@ -977,7 +978,7 @@ impl TurnContext {
|
||||
reasoning_effort,
|
||||
reasoning_summary: self.reasoning_summary,
|
||||
session_source: self.session_source.clone(),
|
||||
environment: Arc::clone(&self.environment),
|
||||
environment: self.environment.clone(),
|
||||
cwd: self.cwd.clone(),
|
||||
current_date: self.current_date.clone(),
|
||||
timezone: self.timezone.clone(),
|
||||
@@ -1406,7 +1407,7 @@ impl Session {
|
||||
model_info: ModelInfo,
|
||||
models_manager: &ModelsManager,
|
||||
network: Option<NetworkProxy>,
|
||||
environment: Arc<Environment>,
|
||||
environment: Option<Arc<Environment>>,
|
||||
sub_id: String,
|
||||
js_repl: Arc<JsReplHandle>,
|
||||
skills_outcome: Arc<SkillLoadOutcome>,
|
||||
@@ -1439,6 +1440,7 @@ impl Session {
|
||||
)
|
||||
.with_web_search_config(per_turn_config.web_search_config.clone())
|
||||
.with_allow_login_shell(per_turn_config.permissions.allow_login_shell)
|
||||
.with_has_environment(environment.is_some())
|
||||
.with_agent_type_description(crate::agent::role::spawn_tool_spec::build(
|
||||
&per_turn_config.agent_roles,
|
||||
));
|
||||
@@ -2542,7 +2544,7 @@ impl Session {
|
||||
.network_proxy
|
||||
.as_ref()
|
||||
.map(StartedNetworkProxy::proxy),
|
||||
Arc::clone(&self.services.environment),
|
||||
self.services.environment.clone(),
|
||||
sub_id,
|
||||
Arc::clone(&self.js_repl),
|
||||
skills_outcome,
|
||||
@@ -5614,6 +5616,7 @@ async fn spawn_review_thread(
|
||||
)
|
||||
.with_web_search_config(/*web_search_config*/ None)
|
||||
.with_allow_login_shell(config.permissions.allow_login_shell)
|
||||
.with_has_environment(parent_turn_context.environment.is_some())
|
||||
.with_agent_type_description(crate::agent::role::spawn_tool_spec::build(
|
||||
&config.agent_roles,
|
||||
));
|
||||
@@ -5672,7 +5675,7 @@ async fn spawn_review_thread(
|
||||
reasoning_effort,
|
||||
reasoning_summary,
|
||||
session_source,
|
||||
environment: Arc::clone(&parent_turn_context.environment),
|
||||
environment: parent_turn_context.environment.clone(),
|
||||
tools_config,
|
||||
features: parent_turn_context.features.clone(),
|
||||
ghost_snapshot: parent_turn_context.ghost_snapshot.clone(),
|
||||
|
||||
@@ -78,8 +78,8 @@ pub(crate) async fn run_codex_thread_interactive(
|
||||
config,
|
||||
auth_manager,
|
||||
models_manager,
|
||||
environment_manager: Arc::new(EnvironmentManager::new(
|
||||
parent_ctx.environment.exec_server_url().map(str::to_owned),
|
||||
environment_manager: Arc::new(EnvironmentManager::from_environment(
|
||||
parent_ctx.environment.as_deref(),
|
||||
)),
|
||||
skills_manager: Arc::clone(&parent_session.services.skills_manager),
|
||||
plugins_manager: Arc::clone(&parent_session.services.plugins_manager),
|
||||
|
||||
@@ -2770,7 +2770,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
|
||||
code_mode_service: crate::tools::code_mode::CodeModeService::new(
|
||||
config.js_repl_node_path.clone(),
|
||||
),
|
||||
environment: Arc::clone(&environment),
|
||||
environment: Some(Arc::clone(&environment)),
|
||||
};
|
||||
let js_repl = Arc::new(JsReplHandle::with_node_path(
|
||||
config.js_repl_node_path.clone(),
|
||||
@@ -2797,7 +2797,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
|
||||
model_info,
|
||||
&models_manager,
|
||||
/*network*/ None,
|
||||
environment,
|
||||
Some(environment),
|
||||
"turn_id".to_string(),
|
||||
Arc::clone(&js_repl),
|
||||
skills_outcome,
|
||||
@@ -3610,7 +3610,7 @@ pub(crate) async fn make_session_and_context_with_dynamic_tools_and_rx(
|
||||
code_mode_service: crate::tools::code_mode::CodeModeService::new(
|
||||
config.js_repl_node_path.clone(),
|
||||
),
|
||||
environment: Arc::clone(&environment),
|
||||
environment: Some(Arc::clone(&environment)),
|
||||
};
|
||||
let js_repl = Arc::new(JsReplHandle::with_node_path(
|
||||
config.js_repl_node_path.clone(),
|
||||
@@ -3637,7 +3637,7 @@ pub(crate) async fn make_session_and_context_with_dynamic_tools_and_rx(
|
||||
model_info,
|
||||
&models_manager,
|
||||
/*network*/ None,
|
||||
environment,
|
||||
Some(environment),
|
||||
"turn_id".to_string(),
|
||||
Arc::clone(&js_repl),
|
||||
skills_outcome,
|
||||
|
||||
@@ -57,5 +57,5 @@ pub(crate) struct SessionServices {
|
||||
/// Session-scoped model client shared across turns.
|
||||
pub(crate) model_client: ModelClient,
|
||||
pub(crate) code_mode_service: CodeModeService,
|
||||
pub(crate) environment: Arc<Environment>,
|
||||
pub(crate) environment: Option<Arc<Environment>>,
|
||||
}
|
||||
|
||||
@@ -91,9 +91,13 @@ impl ToolHandler for ViewImageHandler {
|
||||
AbsolutePathBuf::try_from(turn.resolve_path(Some(args.path))).map_err(|error| {
|
||||
FunctionCallError::RespondToModel(format!("unable to resolve image path: {error}"))
|
||||
})?;
|
||||
let Some(environment) = turn.environment.as_ref() else {
|
||||
return Err(FunctionCallError::RespondToModel(
|
||||
"view_image is unavailable in this session".to_string(),
|
||||
));
|
||||
};
|
||||
|
||||
let metadata = turn
|
||||
.environment
|
||||
let metadata = environment
|
||||
.get_filesystem()
|
||||
.get_metadata(&abs_path)
|
||||
.await
|
||||
@@ -110,8 +114,7 @@ impl ToolHandler for ViewImageHandler {
|
||||
abs_path.display()
|
||||
)));
|
||||
}
|
||||
let file_bytes = turn
|
||||
.environment
|
||||
let file_bytes = environment
|
||||
.get_filesystem()
|
||||
.read_file(&abs_path)
|
||||
.await
|
||||
|
||||
@@ -239,7 +239,12 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
|
||||
.await?
|
||||
{
|
||||
Some(prepared) => {
|
||||
if ctx.turn.environment.exec_server_url().is_some() {
|
||||
let Some(environment) = ctx.turn.environment.as_ref() else {
|
||||
return Err(ToolError::Rejected(
|
||||
"exec_command is unavailable in this session".to_string(),
|
||||
));
|
||||
};
|
||||
if environment.is_remote() {
|
||||
return Err(ToolError::Rejected(
|
||||
"unified_exec zsh-fork is not supported when exec_server_url is configured".to_string(),
|
||||
));
|
||||
@@ -251,7 +256,7 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
|
||||
&prepared.exec_request,
|
||||
req.tty,
|
||||
prepared.spawn_lifecycle,
|
||||
ctx.turn.environment.as_ref(),
|
||||
environment.as_ref(),
|
||||
)
|
||||
.await
|
||||
.map_err(|err| match err {
|
||||
@@ -281,13 +286,18 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
|
||||
let exec_env = attempt
|
||||
.env_for(command, options, req.network.as_ref())
|
||||
.map_err(|err| ToolError::Codex(err.into()))?;
|
||||
let Some(environment) = ctx.turn.environment.as_ref() else {
|
||||
return Err(ToolError::Rejected(
|
||||
"exec_command is unavailable in this session".to_string(),
|
||||
));
|
||||
};
|
||||
self.manager
|
||||
.open_session_with_exec_env(
|
||||
req.process_id,
|
||||
&exec_env,
|
||||
req.tty,
|
||||
Box::new(NoopSpawnLifecycle),
|
||||
ctx.turn.environment.as_ref(),
|
||||
environment.as_ref(),
|
||||
)
|
||||
.await
|
||||
.map_err(|err| match err {
|
||||
|
||||
@@ -98,7 +98,7 @@ async fn exec_command_with_tty(
|
||||
&request,
|
||||
tty,
|
||||
Box::new(NoopSpawnLifecycle),
|
||||
turn.environment.as_ref(),
|
||||
turn.environment.as_ref().expect("turn environment"),
|
||||
)
|
||||
.await?,
|
||||
);
|
||||
@@ -593,7 +593,7 @@ async fn remote_exec_server_rejects_inherited_fd_launches() -> anyhow::Result<()
|
||||
|
||||
let remote_test_env = remote_test_env().await?;
|
||||
let (_, mut turn) = make_session_and_context().await;
|
||||
turn.environment = Arc::new(remote_test_env.environment().clone());
|
||||
turn.environment = Some(Arc::new(remote_test_env.environment().clone()));
|
||||
|
||||
let request = test_exec_request(
|
||||
&turn,
|
||||
@@ -611,7 +611,7 @@ async fn remote_exec_server_rejects_inherited_fd_launches() -> anyhow::Result<()
|
||||
Box::new(TestSpawnLifecycle {
|
||||
inherited_fds: vec![42],
|
||||
}),
|
||||
turn.environment.as_ref(),
|
||||
turn.environment.as_ref().expect("turn environment"),
|
||||
)
|
||||
.await
|
||||
.expect_err("expected inherited fd rejection");
|
||||
|
||||
@@ -593,7 +593,7 @@ impl UnifiedExecProcessManager {
|
||||
.ok_or(UnifiedExecError::MissingCommandLine)?;
|
||||
let inherited_fds = spawn_lifecycle.inherited_fds();
|
||||
|
||||
if environment.exec_server_url().is_some() {
|
||||
if environment.is_remote() {
|
||||
if !inherited_fds.is_empty() {
|
||||
return Err(UnifiedExecError::create_process(
|
||||
"remote exec-server does not support inherited file descriptors".to_string(),
|
||||
|
||||
Reference in New Issue
Block a user