mirror of
https://github.com/openai/codex.git
synced 2026-10-20 17:38:45 +00:00
unified-exec: reject foreign background terminal cwd listing
This commit is contained in:
@@ -1753,6 +1753,7 @@ impl ThreadRequestProcessor {
|
||||
let terminals = thread
|
||||
.list_background_terminals()
|
||||
.await
|
||||
.map_err(invalid_request)?
|
||||
.into_iter()
|
||||
.map(|terminal| ThreadBackgroundTerminal {
|
||||
item_id: terminal.item_id,
|
||||
|
||||
@@ -404,7 +404,7 @@ impl CodexThread {
|
||||
self.codex.agent_status().await
|
||||
}
|
||||
|
||||
pub async fn list_background_terminals(&self) -> Vec<BackgroundTerminalInfo> {
|
||||
pub async fn list_background_terminals(&self) -> Result<Vec<BackgroundTerminalInfo>, String> {
|
||||
self.codex.session.list_background_terminals().await
|
||||
}
|
||||
|
||||
|
||||
@@ -785,7 +785,9 @@ impl Session {
|
||||
.await;
|
||||
}
|
||||
|
||||
pub(crate) async fn list_background_terminals(&self) -> Vec<BackgroundTerminalInfo> {
|
||||
pub(crate) async fn list_background_terminals(
|
||||
&self,
|
||||
) -> Result<Vec<BackgroundTerminalInfo>, String> {
|
||||
self.services.unified_exec_manager.list_processes().await
|
||||
}
|
||||
|
||||
|
||||
@@ -162,7 +162,8 @@ struct ProcessEntry {
|
||||
exit_watcher: Option<tokio::task::JoinHandle<()>>,
|
||||
call_id: String,
|
||||
process_id: i32,
|
||||
cwd: AbsolutePathBuf,
|
||||
cwd_uri: PathUri,
|
||||
path_convention: PathConvention,
|
||||
initial_exec_command_active: Arc<std::sync::atomic::AtomicBool>,
|
||||
hook_command: String,
|
||||
tty: bool,
|
||||
|
||||
@@ -129,7 +129,8 @@ async fn exec_command_with_tty(
|
||||
exit_watcher: None,
|
||||
call_id: context.call_id.clone(),
|
||||
process_id,
|
||||
cwd: cwd.clone(),
|
||||
cwd_uri: PathUri::from_abs_path(&cwd),
|
||||
path_convention: PathConvention::native(),
|
||||
initial_exec_command_active: Arc::new(std::sync::atomic::AtomicBool::new(true)),
|
||||
hook_command: cmd.to_string(),
|
||||
tty,
|
||||
@@ -366,7 +367,10 @@ async fn unified_exec_persists_across_requests() -> anyhow::Result<()> {
|
||||
.await?;
|
||||
let process_id = open_shell.process_id.expect("expected process_id");
|
||||
assert_eq!(
|
||||
session.list_background_terminals().await,
|
||||
session
|
||||
.list_background_terminals()
|
||||
.await
|
||||
.expect("local terminal cwd should be listable"),
|
||||
vec![BackgroundTerminalInfo {
|
||||
item_id: "call".to_string(),
|
||||
process_id: process_id.to_string(),
|
||||
@@ -399,7 +403,49 @@ async fn unified_exec_persists_across_requests() -> anyhow::Result<()> {
|
||||
|
||||
assert!(session.terminate_background_terminal(process_id).await);
|
||||
assert!(!session.terminate_background_terminal(process_id).await);
|
||||
assert!(session.list_background_terminals().await.is_empty());
|
||||
assert!(
|
||||
session
|
||||
.list_background_terminals()
|
||||
.await
|
||||
.expect("local terminal cwd should be listable")
|
||||
.is_empty()
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn background_terminal_listing_rejects_foreign_cwd_conventions() -> anyhow::Result<()> {
|
||||
skip_if_sandbox!(Ok(()));
|
||||
|
||||
let (session, turn) = test_session_and_turn().await;
|
||||
let open_shell = exec_command(
|
||||
&session, &turn, "bash -i", /*yield_time_ms*/ 2_500, /*workdir*/ None,
|
||||
)
|
||||
.await?;
|
||||
let process_id = open_shell.process_id.expect("expected process_id");
|
||||
{
|
||||
let mut store = session
|
||||
.services
|
||||
.unified_exec_manager
|
||||
.process_store
|
||||
.lock()
|
||||
.await;
|
||||
let entry = store
|
||||
.processes
|
||||
.get_mut(&process_id)
|
||||
.expect("background process should be stored");
|
||||
entry.cwd_uri = PathUri::parse("file:///C:/workspace")?;
|
||||
entry.path_convention = PathConvention::Windows;
|
||||
}
|
||||
|
||||
assert_eq!(
|
||||
session.list_background_terminals().await,
|
||||
Err(format!(
|
||||
"background terminal process {process_id} uses Windows cwd paths, which cannot be listed on a POSIX host"
|
||||
))
|
||||
);
|
||||
assert!(session.terminate_background_terminal(process_id).await);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
@@ -676,7 +722,8 @@ async fn terminating_initial_exec_command_rechecks_initial_response_state() -> a
|
||||
exit_watcher: None,
|
||||
call_id: "call".to_string(),
|
||||
process_id,
|
||||
cwd,
|
||||
cwd_uri: PathUri::from_abs_path(&cwd),
|
||||
path_convention: PathConvention::native(),
|
||||
initial_exec_command_active: Arc::new(std::sync::atomic::AtomicBool::new(true)),
|
||||
hook_command: "sleep 60".to_string(),
|
||||
tty: true,
|
||||
@@ -750,7 +797,8 @@ async fn terminating_during_stdin_poll_returns_exited_response() -> anyhow::Resu
|
||||
exit_watcher: None,
|
||||
call_id: "call".to_string(),
|
||||
process_id,
|
||||
cwd,
|
||||
cwd_uri: PathUri::from_abs_path(&cwd),
|
||||
path_convention: PathConvention::native(),
|
||||
initial_exec_command_active: Arc::new(std::sync::atomic::AtomicBool::new(false)),
|
||||
hook_command: "sleep 60".to_string(),
|
||||
tty: true,
|
||||
|
||||
@@ -480,7 +480,6 @@ impl UnifiedExecProcessManager {
|
||||
context,
|
||||
&request.command,
|
||||
request.hook_command.clone(),
|
||||
cwd.clone(),
|
||||
request.cwd_uri.clone(),
|
||||
request.path_convention,
|
||||
start,
|
||||
@@ -893,7 +892,6 @@ impl UnifiedExecProcessManager {
|
||||
context: &UnifiedExecContext,
|
||||
command: &[String],
|
||||
hook_command: String,
|
||||
cwd: AbsolutePathBuf,
|
||||
cwd_uri: PathUri,
|
||||
path_convention: PathConvention,
|
||||
started_at: Instant,
|
||||
@@ -909,7 +907,7 @@ impl UnifiedExecProcessManager {
|
||||
Arc::clone(&context.turn),
|
||||
context.call_id.clone(),
|
||||
command.to_vec(),
|
||||
cwd_uri,
|
||||
cwd_uri.clone(),
|
||||
path_convention,
|
||||
process_id,
|
||||
transcript,
|
||||
@@ -920,7 +918,8 @@ impl UnifiedExecProcessManager {
|
||||
exit_watcher: Some(exit_watcher),
|
||||
call_id: context.call_id.clone(),
|
||||
process_id,
|
||||
cwd: cwd.clone(),
|
||||
cwd_uri,
|
||||
path_convention,
|
||||
initial_exec_command_active,
|
||||
hook_command,
|
||||
tty,
|
||||
@@ -1454,7 +1453,7 @@ impl UnifiedExecProcessManager {
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) async fn list_processes(&self) -> Vec<BackgroundTerminalInfo> {
|
||||
pub(crate) async fn list_processes(&self) -> Result<Vec<BackgroundTerminalInfo>, String> {
|
||||
let store = self.process_store.lock().await;
|
||||
let mut entries = store
|
||||
.processes
|
||||
@@ -1464,11 +1463,27 @@ impl UnifiedExecProcessManager {
|
||||
entries.sort_by_key(|entry| entry.process_id);
|
||||
entries
|
||||
.into_iter()
|
||||
.map(|entry| BackgroundTerminalInfo {
|
||||
item_id: entry.call_id.clone(),
|
||||
process_id: entry.process_id.to_string(),
|
||||
command: entry.hook_command.clone(),
|
||||
cwd: entry.cwd.clone(),
|
||||
.map(|entry| {
|
||||
if entry.path_convention != PathConvention::native() {
|
||||
return Err(format!(
|
||||
"background terminal process {} uses {} cwd paths, which cannot be listed on a {} host",
|
||||
entry.process_id,
|
||||
entry.path_convention,
|
||||
PathConvention::native(),
|
||||
));
|
||||
}
|
||||
let cwd = entry.cwd_uri.to_abs_path().map_err(|error| {
|
||||
format!(
|
||||
"background terminal process {} cwd `{}` cannot be represented on this host: {error}",
|
||||
entry.process_id, entry.cwd_uri,
|
||||
)
|
||||
})?;
|
||||
Ok(BackgroundTerminalInfo {
|
||||
item_id: entry.call_id.clone(),
|
||||
process_id: entry.process_id.to_string(),
|
||||
command: entry.hook_command.clone(),
|
||||
cwd,
|
||||
})
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user