fix(apodex): kill the whole command session when run_shell times out

apodex/sandbox.py

@@ -41,9 +41,11 @@
 from __future__ import annotations
 
 import asyncio
+import contextlib
 import logging
 import os
 import shlex
+import signal
 import sys
 from dataclasses import dataclass
 from pathlib import Path
@@ -258,8 +260,22 @@ async def run_shell(
         cwd=cwd,
         stdout=asyncio.subprocess.PIPE,
         stderr=asyncio.subprocess.PIPE,
+        start_new_session=True,
     )
-    out, err = await asyncio.wait_for(proc.communicate(), timeout=timeout)
+    completed = False
+    try:
+        out, err = await asyncio.wait_for(proc.communicate(), timeout=timeout)
+        completed = True
+    finally:
+        if not completed:
+            # Timed out or cancelled: kill the whole session, not just the
+            # shell, which may have exited while its children hold the pipes.
+            # Same contract as ``_CurrentCommands.run`` in plugins.tools._sandbox,
+            # including the bounded wait for a setsid escapee killpg misses.
+            with contextlib.suppress(OSError):
+                os.killpg(proc.pid, signal.SIGKILL)
+            with contextlib.suppress(TimeoutError):
+                await asyncio.wait_for(proc.wait(), timeout=5)
     return (
         proc.returncode or 0,
         out.decode("utf-8", "replace"),

apodex/tests/test_native.py

@@ -4,6 +4,8 @@
 import os
 from pathlib import Path
 
+import pytest
+
 from apodex import cli, docker, sandbox
 from apodex.native import prepare_native_runtime
 from apodex.sandbox import BWRAP, CONTAINER, NATIVE, Strategy, resolve_strategy
@@ -199,6 +201,38 @@ def kill(self):
     assert second.binds == ((str(second_workspace.resolve()),) * 2 + (False,),)
 
 
+@pytest.mark.parametrize(
+    "command",
+    ["(sleep 2; touch marker) & wait", "(sleep 2; touch marker) & exit 0"],
+    ids=["running-shell", "exited-shell"],
+)
+@pytest.mark.parametrize("interruption", ["timeout", "cancellation"])
+async def test_run_shell_kills_the_whole_command_on_interruption(
+    tmp_path, command, interruption,
+) -> None:
+    """An interrupted command must not keep writing to the workspace.
+
+    The subshell is a grandchild holding the output pipes, so killing only the
+    shell would still leave it alive to write the marker. Cleanup must also run
+    when the shell has already exited while its child still holds the pipes.
+    """
+    if interruption == "timeout":
+        with pytest.raises(TimeoutError):
+            await sandbox.run_shell(command, str(tmp_path), 1, Strategy(NATIVE, "test"))
+    else:
+        task = asyncio.create_task(sandbox.run_shell(
+            command, str(tmp_path), 10,
+            Strategy(NATIVE, "test"),
+        ))
+        await asyncio.sleep(1)
+        task.cancel()
+        with pytest.raises(asyncio.CancelledError):
+            await task
+    await asyncio.sleep(2)
+
+    assert not (tmp_path / "marker").exists()
+
+
 def test_macos_falls_back_to_native_when_docker_is_unavailable(
     tmp_path, monkeypatch, capsys,
 ) -> None: