fix(sandbox): refuse FIFO, socket, and device nodes in UnixLocal read/write instead of blocking the event loop

src/agents/sandbox/sandboxes/unix_local.py

@@ -15,13 +15,14 @@
 import shlex
 import shutil
 import signal
+import stat
 import tarfile
 import tempfile
 import termios
 import time
 import uuid
 from collections import deque
-from collections.abc import Collection, Mapping, Sequence
+from collections.abc import Callable, Collection, Mapping, Sequence
 from contextlib import suppress
 from dataclasses import dataclass, field
 from functools import partial
@@ -152,6 +153,115 @@ class _UnixPtyProcessEntry:
     wait_task: asyncio.Task[None] | None = None
 
 
+_SPECIAL_FILE_KINDS: tuple[tuple[Callable[[int], bool], str], ...] = (
+    (stat.S_ISFIFO, "fifo"),
+    (stat.S_ISSOCK, "socket"),
+    (stat.S_ISCHR, "character device"),
+    (stat.S_ISBLK, "block device"),
+)
+_SPECIAL_FILE_EXIT_CODE = 3
+
+# User-scoped writer, run as the requested user via the confined exec path. It does the
+# special-file classification itself (host-side checks cannot see inside a directory only
+# that user may search): an existing entry is opened read-write, which unlike a write-only
+# open never blocks on a FIFO, and the type of the *descriptor* is tested through /dev/fd.
+_USER_WRITE_SCRIPT = (
+    'target="$1"\n'
+    'mkdir -p "$(dirname "$target")" || exit 1\n'
+    'if [ -e "$target" ] || [ -L "$target" ]; then\n'
+    '    exec 3<>"$target" || exit 1\n'
+    "    if [ -p /dev/fd/3 ] || [ -S /dev/fd/3 ] || [ -c /dev/fd/3 ] || [ -b /dev/fd/3 ]; then\n"
+    f"        exit {_SPECIAL_FILE_EXIT_CODE}\n"
+    "    fi\n"
+    "    exec 3>&-\n"
+    "fi\n"
+    'cat > "$target"\n'
+)
+
+
+def _special_file_kind(mode: int) -> str | None:
+    return next((name for predicate, name in _SPECIAL_FILE_KINDS if predicate(mode)), None)
+
+
+def _raise_for_special_file(kind: str | None, *, path: Path, for_write: bool) -> None:
+    if kind is None:
+        return
+    context = {"reason": f"not a regular file: {kind}"}
+    if for_write:
+        raise WorkspaceArchiveWriteError(path=path, context=context)
+    raise WorkspaceArchiveReadError(path=path, context=context)
+
+
+def _classify_mode(mode: int, *, workspace_path: Path, path: Path, for_write: bool) -> None:
+    if stat.S_ISDIR(mode):
+        raise IsADirectoryError(errno.EISDIR, os.strerror(errno.EISDIR), str(workspace_path))
+    _raise_for_special_file(_special_file_kind(mode), path=path, for_write=for_write)
+
+
+def _open_regular_file(workspace_path: Path, *, path: Path, for_write: bool) -> int:
+    """Open a workspace file for in-process I/O, refusing FIFOs, sockets, and device nodes.
+
+    `open()` on a FIFO with no peer blocks the calling thread, and this session performs
+    file I/O synchronously on the event loop, so such an open would stall the whole
+    process; a device node may block or act on open regardless of `O_NONBLOCK`.
+
+    Where the platform offers `O_PATH` (Linux), the entry is pinned with a descriptor that
+    does not open it, classified with `fstat()`, and then that same inode is opened for I/O
+    through `/proc/self/fd`, so a replacement of the path between the two steps cannot
+    reach a blocking open. A missing target is created with `O_EXCL`, which guarantees the
+    created entry is a regular file. Elsewhere the entry is classified with `stat()`
+    before a non-blocking open and again with `fstat()` on the opened descriptor.
+    Missing paths keep their existing error handling; a directory is reported like the
+    blocking `open()` did.
+    """
+    cloexec = getattr(os, "O_CLOEXEC", 0)
+    if for_write:
+        io_flags = os.O_WRONLY | os.O_TRUNC
+    else:
+        io_flags = os.O_RDONLY
+    o_path = getattr(os, "O_PATH", None)
+    if o_path is not None:
+        try:
+            pin = os.open(workspace_path, o_path | cloexec)
+        except FileNotFoundError:
+            if not for_write:
+                raise
+            # Create the file ourselves; O_EXCL means whatever we get back is the
+            # regular file this call created, never an entry swapped in meanwhile.
+            return os.open(workspace_path, os.O_WRONLY | os.O_CREAT | os.O_EXCL | cloexec, 0o666)
+        try:
+            _classify_mode(
+                os.fstat(pin).st_mode, workspace_path=workspace_path, path=path, for_write=for_write
+            )
+            return os.open(f"/proc/self/fd/{pin}", io_flags | cloexec)
+        finally:
+            os.close(pin)
+
+    try:
+        _classify_mode(
+            workspace_path.stat().st_mode,
+            workspace_path=workspace_path,
+            path=path,
+            for_write=for_write,
+        )
+    except OSError:
+        pass  # missing or unreadable: let the open below report it
+    flags = io_flags | os.O_NONBLOCK | cloexec
+    if for_write:
+        flags |= os.O_CREAT
+    fd = os.open(workspace_path, flags, 0o666)
+    try:
+        _classify_mode(
+            os.fstat(fd).st_mode, workspace_path=workspace_path, path=path, for_write=for_write
+        )
+        # Regular files ignore O_NONBLOCK; clear it anyway so the handle behaves like open().
+        fcntl.fcntl(fd, fcntl.F_SETFL, fcntl.fcntl(fd, fcntl.F_GETFL) & ~os.O_NONBLOCK)
+    except BaseException:
+        os.close(fd)
+        raise
+    return fd
+
+
 class UnixLocalSandboxSession(BaseSandboxSession):
     """
     Unix-only session implementation that runs commands on the host and uses the host filesystem
@@ -985,11 +1095,12 @@ async def read(self, path: Path, *, user: str | User | None = None) -> io.IOBase
 
         workspace_path = self.normalize_path(path)
         try:
-            return workspace_path.open("rb")
+            fd = _open_regular_file(workspace_path, path=path, for_write=False)
         except FileNotFoundError as e:
             raise WorkspaceReadNotFoundError(path=path, cause=e) from e
         except OSError as e:
             raise WorkspaceArchiveReadError(path=path, cause=e) from e
+        return os.fdopen(fd, "rb")
 
     async def write(
         self,
@@ -1007,7 +1118,8 @@ async def write(
 
         try:
             workspace_path.parent.mkdir(parents=True, exist_ok=True)
-            with workspace_path.open("wb") as f:
+            fd = _open_regular_file(workspace_path, path=workspace_path, for_write=True)
+            with os.fdopen(fd, "wb") as f:
                 shutil.copyfileobj(payload.stream, f)
         except OSError as e:
             raise WorkspaceArchiveWriteError(path=workspace_path, cause=e) from e
@@ -1024,7 +1136,7 @@ async def _write_stream_with_exec(
         command_parts = self._prepare_exec_command(
             "sh",
             "-c",
-            'mkdir -p "$(dirname "$1")" && cat > "$1"',
+            _USER_WRITE_SCRIPT,
             "sh",
             str(path),
             shell=False,
@@ -1062,6 +1174,10 @@ async def _write_stream_with_exec(
         except OSError as e:
             raise WorkspaceArchiveWriteError(path=path, cause=e) from e
 
+        if proc.returncode == _SPECIAL_FILE_EXIT_CODE:
+            raise WorkspaceArchiveWriteError(
+                path=path, context={"reason": "not a regular file", "user": str(user)}
+            )
         if proc.returncode:
             raise WorkspaceArchiveWriteError(
                 path=path,

tests/sandbox/test_unix_local.py

@@ -2,18 +2,26 @@
 
 import asyncio
 import io
+import os
+import shutil
 import signal
+import subprocess
 import tarfile
+import tempfile
 import threading
 import time
 from pathlib import Path
 from types import SimpleNamespace
-from typing import cast
+from typing import Any, cast
 
 import pytest
 
 from agents.sandbox import SandboxPathGrant
-from agents.sandbox.errors import PtySessionNotFoundError
+from agents.sandbox.errors import (
+    PtySessionNotFoundError,
+    WorkspaceArchiveReadError,
+    WorkspaceArchiveWriteError,
+)
 from agents.sandbox.manifest import Environment, Manifest
 from agents.sandbox.sandboxes import unix_local as unix_local_module
 from agents.sandbox.sandboxes.unix_local import (
@@ -513,3 +521,173 @@ def _slow_extract(tar: object, **kwargs: object) -> None:
     # the workspace root are only released once nothing is still writing to them.
     assert events == ["extract-start", "extract-end"]
     assert not buf.closed
+
+
+@pytest.mark.asyncio
+@pytest.mark.skipif(not hasattr(os, "mkfifo"), reason="requires FIFO support")
+async def test_unix_local_read_and_write_refuse_a_fifo_instead_of_blocking(
+    tmp_path: Path,
+) -> None:
+    workspace = tmp_path / "ws"
+    workspace.mkdir()
+    fifo = workspace / "pipe"
+    os.mkfifo(fifo)
+    # Keep both ends of the pipe open so the previous behaviour (opening the FIFO) returns
+    # instead of blocking the event loop, letting the assertions below fail cleanly.
+    peer_fd = os.open(fifo, os.O_RDWR | os.O_NONBLOCK)
+    try:
+        async with await UnixLocalSandboxClient().create(
+            manifest=Manifest(root=str(workspace)), snapshot=None, options=None
+        ) as session:
+            with pytest.raises(WorkspaceArchiveReadError) as read_error:
+                await session.read(Path("pipe"))
+            assert read_error.value.context["reason"] == "not a regular file: fifo"
+
+            with pytest.raises(WorkspaceArchiveWriteError) as write_error:
+                await session.write(Path("pipe"), io.BytesIO(b"payload"))
+            assert write_error.value.context["reason"] == "not a regular file: fifo"
+
+            # A link to the FIFO resolves to the same entry and is refused the same way.
+            (workspace / "pipe_link").symlink_to(fifo)
+            with pytest.raises(WorkspaceArchiveReadError):
+                await session.read(Path("pipe_link"))
+
+            # Regular-file behaviour is unchanged.
+            await session.write(Path("plain.txt"), io.BytesIO(b"hello"))
+            handle = await session.read(Path("plain.txt"))
+            try:
+                assert handle.read() == b"hello"
+            finally:
+                handle.close()
+    finally:
+        os.close(peer_fd)
+
+    assert fifo.is_fifo()
+
+
+@pytest.mark.asyncio
+@pytest.mark.skipif(not hasattr(os, "mkfifo"), reason="requires FIFO support")
+async def test_unix_local_refuses_a_fifo_without_a_peer_and_a_directory_read(
+    tmp_path: Path,
+) -> None:
+    workspace = tmp_path / "ws"
+    workspace.mkdir()
+    fifo = workspace / "pipe"
+    os.mkfifo(fifo)
+    (workspace / "dir").mkdir()
+
+    async with await UnixLocalSandboxClient().create(
+        manifest=Manifest(root=str(workspace)), snapshot=None, options=None
+    ) as session:
+        # No peer holds the pipe open: a blocking open would never return.
+        with pytest.raises(WorkspaceArchiveWriteError):
+            await asyncio.wait_for(session.write(Path("pipe"), io.BytesIO(b"payload")), 5)
+        with pytest.raises(WorkspaceArchiveReadError) as read_error:
+            await asyncio.wait_for(session.read(Path("pipe")), 5)
+        assert read_error.value.context["reason"] == "not a regular file: fifo"
+
+        with pytest.raises(WorkspaceArchiveReadError) as dir_error:
+            await session.read(Path("dir"))
+        assert isinstance(dir_error.value.__cause__, IsADirectoryError)
+
+    assert fifo.is_fifo()
+
+
+@pytest.mark.skipif(not hasattr(os, "mkfifo"), reason="requires FIFO support")
+def test_open_regular_file_never_opens_a_fifo_for_io(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    fifo = tmp_path / "pipe"
+    os.mkfifo(fifo)
+    real_open = os.open
+    o_path = getattr(os, "O_PATH", 0)
+
+    def guarded_open(path: object, flags: int, *args: object) -> int:
+        if Path(str(path)) == fifo and not (o_path and flags & o_path):
+            raise AssertionError("the FIFO must be classified without an I/O open")
+        return real_open(cast(Any, path), flags, *cast(Any, args))
+
+    monkeypatch.setattr(unix_local_module.os, "open", guarded_open)
+
+    with pytest.raises(WorkspaceArchiveReadError) as read_error:
+        unix_local_module._open_regular_file(fifo, path=Path("pipe"), for_write=False)
+    assert read_error.value.context["reason"] == "not a regular file: fifo"
+    with pytest.raises(WorkspaceArchiveWriteError):
+        unix_local_module._open_regular_file(fifo, path=Path("pipe"), for_write=True)
+    assert fifo.is_fifo()
+
+
+def test_open_regular_file_creates_missing_targets_and_reads_them_back(tmp_path: Path) -> None:
+    target = tmp_path / "new.txt"
+    fd = unix_local_module._open_regular_file(target, path=Path("new.txt"), for_write=True)
+    with os.fdopen(fd, "wb") as handle:
+        handle.write(b"hello")
+    fd = unix_local_module._open_regular_file(target, path=Path("new.txt"), for_write=False)
+    with os.fdopen(fd, "rb") as handle:
+        assert handle.read() == b"hello"
+    with pytest.raises(FileNotFoundError):
+        unix_local_module._open_regular_file(
+            tmp_path / "absent", path=Path("absent"), for_write=False
+        )
+    with pytest.raises(IsADirectoryError):
+        unix_local_module._open_regular_file(tmp_path, path=Path("."), for_write=False)
+
+
+def _run_user_write_script(
+    target: Path, payload: bytes, *, user: int | None
+) -> subprocess.CompletedProcess[bytes]:
+    return subprocess.run(
+        ["sh", "-c", unix_local_module._USER_WRITE_SCRIPT, "sh", str(target)],
+        input=payload,
+        capture_output=True,
+        check=False,
+        timeout=5,  # a regression would block on the FIFO; the watchdog fails the test instead
+        user=user,
+        group=user,
+    )
+
+
+@pytest.mark.skipif(not hasattr(os, "mkfifo"), reason="requires FIFO support")
+def test_user_write_script_refuses_a_fifo_and_writes_regular_files(tmp_path: Path) -> None:
+    fifo = tmp_path / "pipe"
+    os.mkfifo(fifo)
+
+    refused = _run_user_write_script(fifo, b"payload", user=None)
+    assert refused.returncode == unix_local_module._SPECIAL_FILE_EXIT_CODE, refused.stderr
+    assert fifo.is_fifo()
+
+    created = _run_user_write_script(tmp_path / "sub" / "new.txt", b"hello", user=None)
+    assert created.returncode == 0, created.stderr
+    assert (tmp_path / "sub" / "new.txt").read_bytes() == b"hello"
+
+    existing = tmp_path / "existing.txt"
+    existing.write_bytes(b"old content that is longer")
+    rewritten = _run_user_write_script(existing, b"new", user=None)
+    assert rewritten.returncode == 0, rewritten.stderr
+    assert existing.read_bytes() == b"new"
+
+
+@pytest.mark.skipif(not hasattr(os, "mkfifo"), reason="requires FIFO support")
+@pytest.mark.skipif(os.geteuid() != 0, reason="needs root to run the writer as another user")
+def test_user_write_script_refuses_a_fifo_under_a_user_only_parent(tmp_path: Path) -> None:
+    """The requested user, not the SDK identity, is the one that can see and open the FIFO."""
+    nobody = 65534
+    # pytest's tmp_path sits under a root-only directory; the requested user must be able
+    # to reach the parent, so build it under the world-traversable temp root instead.
+    parent = Path(tempfile.mkdtemp(prefix="unix-local-private-"))
+    try:
+        fifo = parent / "pipe"
+        os.mkfifo(fifo)
+        os.chown(fifo, nobody, nobody)
+        os.chown(parent, nobody, nobody)
+        parent.chmod(0o700)
+
+        refused = _run_user_write_script(fifo, b"payload", user=nobody)
+        assert refused.returncode == unix_local_module._SPECIAL_FILE_EXIT_CODE, refused.stderr
+        assert fifo.is_fifo()
+
+        written = _run_user_write_script(parent / "note.txt", b"hello", user=nobody)
+        assert written.returncode == 0, written.stderr
+        assert (parent / "note.txt").read_bytes() == b"hello"
+    finally:
+        shutil.rmtree(parent, ignore_errors=True)