From 1c019517d5d7d3cdcbd936aa0b1933aa97715922 Mon Sep 17 00:00:00 2001 From: coderdailyone Date: Sun, 6 Sep 2026 15:28:49 +0000 Subject: [PATCH 1/4] fix(sandbox): refuse FIFO, socket, and device nodes in UnixLocal read/write instead of blocking the event loop Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01DFSMrLZZ3oq1dKmJFAofz6 --- src/agents/sandbox/sandboxes/unix_local.py | 33 ++++++++++++++- tests/sandbox/test_unix_local.py | 49 +++++++++++++++++++++- 2 files changed, 80 insertions(+), 2 deletions(-) diff --git a/src/agents/sandbox/sandboxes/unix_local.py b/src/agents/sandbox/sandboxes/unix_local.py index ea7f83e9d8..b1129e3d1f 100644 --- a/src/agents/sandbox/sandboxes/unix_local.py +++ b/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,34 @@ 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"), +) + + +def _raise_if_special_file(workspace_path: Path, *, path: Path, for_write: bool) -> None: + """Refuse to open a FIFO, socket, or device node as a workspace file. + + `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. Missing paths and directories keep their existing `open()` error handling. + """ + try: + mode = workspace_path.stat().st_mode + except OSError: + return + kind = next((name for predicate, name in _SPECIAL_FILE_KINDS if predicate(mode)), 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) + + class UnixLocalSandboxSession(BaseSandboxSession): """ Unix-only session implementation that runs commands on the host and uses the host filesystem @@ -984,6 +1013,7 @@ async def read(self, path: Path, *, user: str | User | None = None) -> io.IOBase await self._check_read_with_exec(path, user=user) workspace_path = self.normalize_path(path) + _raise_if_special_file(workspace_path, path=path, for_write=False) try: return workspace_path.open("rb") except FileNotFoundError as e: @@ -1001,6 +1031,7 @@ async def write( payload = coerce_write_payload(path=path, data=data) workspace_path = self.normalize_path(path, for_write=True) + _raise_if_special_file(workspace_path, path=workspace_path, for_write=True) if user is not None: await self._write_stream_with_exec(workspace_path, payload.stream, user=user) return diff --git a/tests/sandbox/test_unix_local.py b/tests/sandbox/test_unix_local.py index fb13be3c51..9d3b8d91a0 100644 --- a/tests/sandbox/test_unix_local.py +++ b/tests/sandbox/test_unix_local.py @@ -2,6 +2,7 @@ import asyncio import io +import os import signal import tarfile import threading @@ -13,7 +14,11 @@ 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 +518,45 @@ 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() From 40d2fcb2fd63d853572958fbac323265e4c7020c Mon Sep 17 00:00:00 2001 From: coderdailyone Date: Sun, 6 Sep 2026 15:41:09 +0000 Subject: [PATCH 2/4] fix(sandbox): classify the opened descriptor instead of a pre-check stat Open non-blocking and judge with fstat so a concurrent replacement cannot slip a FIFO past the check; probe the user-scoped write target the same way. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01DFSMrLZZ3oq1dKmJFAofz6 --- src/agents/sandbox/sandboxes/unix_local.py | 77 +++++++++++++++++----- tests/sandbox/test_unix_local.py | 28 ++++++++ 2 files changed, 89 insertions(+), 16 deletions(-) diff --git a/src/agents/sandbox/sandboxes/unix_local.py b/src/agents/sandbox/sandboxes/unix_local.py index b1129e3d1f..84dda679ed 100644 --- a/src/agents/sandbox/sandboxes/unix_local.py +++ b/src/agents/sandbox/sandboxes/unix_local.py @@ -161,24 +161,68 @@ class _UnixPtyProcessEntry: ) -def _raise_if_special_file(workspace_path: Path, *, path: Path, for_write: bool) -> None: - """Refuse to open a FIFO, socket, or device node as a workspace file. +def _special_file_kind(mode: int) -> str | None: + return next((name for predicate, name in _SPECIAL_FILE_KINDS if predicate(mode)), None) + + +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. Missing paths and directories keep their existing `open()` error handling. + process. The descriptor is opened non-blocking and classified with `fstat()` so a + concurrent replacement of the entry cannot slip past the check. Missing paths keep + their existing error handling; a directory is reported like the blocking `open()` did. """ + flags = os.O_NONBLOCK | getattr(os, "O_CLOEXEC", 0) + if for_write: + flags |= os.O_WRONLY | os.O_CREAT | os.O_TRUNC + else: + flags |= os.O_RDONLY + fd = os.open(workspace_path, flags, 0o666) try: - mode = workspace_path.stat().st_mode - except OSError: - return - kind = next((name for predicate, name in _SPECIAL_FILE_KINDS if predicate(mode)), None) - if kind is None: + mode = os.fstat(fd).st_mode + if stat.S_ISDIR(mode): + raise IsADirectoryError(errno.EISDIR, os.strerror(errno.EISDIR), str(workspace_path)) + kind = _special_file_kind(mode) + if kind is not None: + context = {"reason": f"not a regular file: {kind}"} + if for_write: + raise WorkspaceArchiveWriteError(path=path, context=context) + raise WorkspaceArchiveReadError(path=path, context=context) + # 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 + + +def _raise_if_existing_special_file(workspace_path: Path) -> None: + """Refuse a user-scoped write whose existing target is a FIFO, socket, or device node. + + The write itself runs as the requested user (`cat > "$1"`), which would block on a + FIFO. Opening the current entry non-blocking classifies it without blocking: a FIFO + or socket without a peer fails with ENXIO, anything else is judged by `fstat()`. + """ + try: + fd = os.open(workspace_path, os.O_WRONLY | os.O_NONBLOCK | getattr(os, "O_CLOEXEC", 0)) + except FileNotFoundError: return - context = {"reason": f"not a regular file: {kind}"} - if for_write: - raise WorkspaceArchiveWriteError(path=path, context=context) - raise WorkspaceArchiveReadError(path=path, context=context) + except OSError as e: + if e.errno == errno.ENXIO: + raise WorkspaceArchiveWriteError( + path=workspace_path, context={"reason": "not a regular file: fifo or socket"} + ) from e + return # let the user-scoped exec report permission and type errors + try: + kind = _special_file_kind(os.fstat(fd).st_mode) + finally: + os.close(fd) + if kind is not None: + raise WorkspaceArchiveWriteError( + path=workspace_path, context={"reason": f"not a regular file: {kind}"} + ) class UnixLocalSandboxSession(BaseSandboxSession): @@ -1013,13 +1057,13 @@ async def read(self, path: Path, *, user: str | User | None = None) -> io.IOBase await self._check_read_with_exec(path, user=user) workspace_path = self.normalize_path(path) - _raise_if_special_file(workspace_path, path=path, for_write=False) 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, @@ -1031,14 +1075,15 @@ async def write( payload = coerce_write_payload(path=path, data=data) workspace_path = self.normalize_path(path, for_write=True) - _raise_if_special_file(workspace_path, path=workspace_path, for_write=True) if user is not None: + _raise_if_existing_special_file(workspace_path) await self._write_stream_with_exec(workspace_path, payload.stream, user=user) return 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 diff --git a/tests/sandbox/test_unix_local.py b/tests/sandbox/test_unix_local.py index 9d3b8d91a0..9d2d0b956e 100644 --- a/tests/sandbox/test_unix_local.py +++ b/tests/sandbox/test_unix_local.py @@ -560,3 +560,31 @@ async def test_unix_local_read_and_write_refuse_a_fifo_instead_of_blocking( 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() From 700a0805d5038cd307c98472565fc774d70c15b8 Mon Sep 17 00:00:00 2001 From: coderdailyone Date: Mon, 7 Sep 2026 04:26:25 +0000 Subject: [PATCH 3/4] fix(sandbox): classify special files with stat before opening and under permission denials Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01DFSMrLZZ3oq1dKmJFAofz6 --- src/agents/sandbox/sandboxes/unix_local.py | 50 ++++++++++++++-------- tests/sandbox/test_unix_local.py | 45 ++++++++++++++++++- 2 files changed, 77 insertions(+), 18 deletions(-) diff --git a/src/agents/sandbox/sandboxes/unix_local.py b/src/agents/sandbox/sandboxes/unix_local.py index 84dda679ed..bbc4fcfc97 100644 --- a/src/agents/sandbox/sandboxes/unix_local.py +++ b/src/agents/sandbox/sandboxes/unix_local.py @@ -165,15 +165,32 @@ 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 _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. The descriptor is opened non-blocking and classified with `fstat()` so a - concurrent replacement of the entry cannot slip past the check. Missing paths keep - their existing error handling; a directory is reported like the blocking `open()` did. + process. The entry is classified with `stat()` before it is opened at all, so a device + node is never invoked (some drivers block or act on open regardless of `O_NONBLOCK`), + and the descriptor is opened non-blocking and classified again with `fstat()` so a + replacement between the two calls cannot slip past. Missing paths keep their existing + error handling; a directory is reported like the blocking `open()` did. """ + try: + _raise_for_special_file( + _special_file_kind(workspace_path.stat().st_mode), path=path, for_write=for_write + ) + except OSError: + pass # missing or unreadable: let the open below report it flags = os.O_NONBLOCK | getattr(os, "O_CLOEXEC", 0) if for_write: flags |= os.O_WRONLY | os.O_CREAT | os.O_TRUNC @@ -184,12 +201,7 @@ def _open_regular_file(workspace_path: Path, *, path: Path, for_write: bool) -> mode = os.fstat(fd).st_mode if stat.S_ISDIR(mode): raise IsADirectoryError(errno.EISDIR, os.strerror(errno.EISDIR), str(workspace_path)) - kind = _special_file_kind(mode) - if kind is not None: - context = {"reason": f"not a regular file: {kind}"} - if for_write: - raise WorkspaceArchiveWriteError(path=path, context=context) - raise WorkspaceArchiveReadError(path=path, context=context) + _raise_for_special_file(_special_file_kind(mode), 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: @@ -202,27 +214,31 @@ def _raise_if_existing_special_file(workspace_path: Path) -> None: """Refuse a user-scoped write whose existing target is a FIFO, socket, or device node. The write itself runs as the requested user (`cat > "$1"`), which would block on a - FIFO. Opening the current entry non-blocking classifies it without blocking: a FIFO - or socket without a peer fails with ENXIO, anything else is judged by `fstat()`. + FIFO. The entry is classified with `stat()`, which needs only search permission on the + parent, so a target the SDK identity cannot open (for example a mode-0200 FIFO owned by + the requested user) is still recognized; a FIFO or socket without a peer is also caught + by the non-blocking open failing with ENXIO. """ try: - fd = os.open(workspace_path, os.O_WRONLY | os.O_NONBLOCK | getattr(os, "O_CLOEXEC", 0)) + kind = _special_file_kind(workspace_path.stat().st_mode) except FileNotFoundError: return + except OSError: + kind = None + _raise_for_special_file(kind, path=workspace_path, for_write=True) + try: + fd = os.open(workspace_path, os.O_WRONLY | os.O_NONBLOCK | getattr(os, "O_CLOEXEC", 0)) except OSError as e: if e.errno == errno.ENXIO: raise WorkspaceArchiveWriteError( path=workspace_path, context={"reason": "not a regular file: fifo or socket"} ) from e - return # let the user-scoped exec report permission and type errors + return # missing, or a permission error the user-scoped exec will report itself try: kind = _special_file_kind(os.fstat(fd).st_mode) finally: os.close(fd) - if kind is not None: - raise WorkspaceArchiveWriteError( - path=workspace_path, context={"reason": f"not a regular file: {kind}"} - ) + _raise_for_special_file(kind, path=workspace_path, for_write=True) class UnixLocalSandboxSession(BaseSandboxSession): diff --git a/tests/sandbox/test_unix_local.py b/tests/sandbox/test_unix_local.py index 9d2d0b956e..f89424c831 100644 --- a/tests/sandbox/test_unix_local.py +++ b/tests/sandbox/test_unix_local.py @@ -1,6 +1,7 @@ from __future__ import annotations import asyncio +import errno import io import os import signal @@ -9,7 +10,7 @@ import time from pathlib import Path from types import SimpleNamespace -from typing import cast +from typing import Any, cast import pytest @@ -588,3 +589,45 @@ async def test_unix_local_refuses_a_fifo_without_a_peer_and_a_directory_read( 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_classifies_a_fifo_before_opening_it( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + fifo = tmp_path / "pipe" + os.mkfifo(fifo) + real_open = os.open + + def guarded_open(path: object, *args: object, **kwargs: object) -> int: + if Path(str(path)) == fifo: + raise AssertionError("the FIFO must be rejected without being opened") + return real_open(cast(Any, path), *cast(Any, args), **cast(Any, kwargs)) + + 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) + + +@pytest.mark.skipif(not hasattr(os, "mkfifo"), reason="requires FIFO support") +def test_user_scoped_write_refuses_a_fifo_the_sdk_identity_cannot_open( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + fifo = tmp_path / "pipe" + os.mkfifo(fifo) + + def denied_open(path: object, *args: object, **kwargs: object) -> int: + raise PermissionError(errno.EACCES, os.strerror(errno.EACCES), str(path)) + + monkeypatch.setattr(unix_local_module.os, "open", denied_open) + + with pytest.raises(WorkspaceArchiveWriteError) as write_error: + unix_local_module._raise_if_existing_special_file(fifo) + assert write_error.value.context["reason"] == "not a regular file: fifo" + + # A missing target is left to the user-scoped exec, which creates it. + unix_local_module._raise_if_existing_special_file(tmp_path / "new.txt") From 7c58cec1b3a03ffeff7861de65dd4c1b742ee8ce Mon Sep 17 00:00:00 2001 From: coderdailyone Date: Tue, 8 Sep 2026 16:20:56 +0000 Subject: [PATCH 4/4] fix(sandbox): classify special files inside the user-scoped writer and pin inodes with O_PATH The user-scoped write now opens the existing entry read-write as the requested user and tests the descriptor's type through /dev/fd before cat runs, so a FIFO only that user can reach is refused instead of blocking. In-process I/O pins the entry with O_PATH where available, classifies it with fstat, and reopens the same inode through /proc/self/fd; missing targets are created with O_EXCL. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_019m9gE9ZQnx3UeRYXggw7TN --- src/agents/sandbox/sandboxes/unix_local.py | 120 ++++++++++++--------- tests/sandbox/test_unix_local.py | 94 +++++++++++++--- 2 files changed, 149 insertions(+), 65 deletions(-) diff --git a/src/agents/sandbox/sandboxes/unix_local.py b/src/agents/sandbox/sandboxes/unix_local.py index bbc4fcfc97..40befdb15e 100644 --- a/src/agents/sandbox/sandboxes/unix_local.py +++ b/src/agents/sandbox/sandboxes/unix_local.py @@ -159,6 +159,24 @@ class _UnixPtyProcessEntry: (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: @@ -174,34 +192,68 @@ def _raise_for_special_file(kind: str | None, *, path: Path, for_write: bool) -> 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. The entry is classified with `stat()` before it is opened at all, so a device - node is never invoked (some drivers block or act on open regardless of `O_NONBLOCK`), - and the descriptor is opened non-blocking and classified again with `fstat()` so a - replacement between the two calls cannot slip past. Missing paths keep their existing - error handling; a directory is reported like the blocking `open()` did. + 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: - _raise_for_special_file( - _special_file_kind(workspace_path.stat().st_mode), path=path, for_write=for_write + _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 = os.O_NONBLOCK | getattr(os, "O_CLOEXEC", 0) + flags = io_flags | os.O_NONBLOCK | cloexec if for_write: - flags |= os.O_WRONLY | os.O_CREAT | os.O_TRUNC - else: - flags |= os.O_RDONLY + flags |= os.O_CREAT fd = os.open(workspace_path, flags, 0o666) try: - mode = os.fstat(fd).st_mode - 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) + _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: @@ -210,37 +262,6 @@ def _open_regular_file(workspace_path: Path, *, path: Path, for_write: bool) -> return fd -def _raise_if_existing_special_file(workspace_path: Path) -> None: - """Refuse a user-scoped write whose existing target is a FIFO, socket, or device node. - - The write itself runs as the requested user (`cat > "$1"`), which would block on a - FIFO. The entry is classified with `stat()`, which needs only search permission on the - parent, so a target the SDK identity cannot open (for example a mode-0200 FIFO owned by - the requested user) is still recognized; a FIFO or socket without a peer is also caught - by the non-blocking open failing with ENXIO. - """ - try: - kind = _special_file_kind(workspace_path.stat().st_mode) - except FileNotFoundError: - return - except OSError: - kind = None - _raise_for_special_file(kind, path=workspace_path, for_write=True) - try: - fd = os.open(workspace_path, os.O_WRONLY | os.O_NONBLOCK | getattr(os, "O_CLOEXEC", 0)) - except OSError as e: - if e.errno == errno.ENXIO: - raise WorkspaceArchiveWriteError( - path=workspace_path, context={"reason": "not a regular file: fifo or socket"} - ) from e - return # missing, or a permission error the user-scoped exec will report itself - try: - kind = _special_file_kind(os.fstat(fd).st_mode) - finally: - os.close(fd) - _raise_for_special_file(kind, path=workspace_path, for_write=True) - - class UnixLocalSandboxSession(BaseSandboxSession): """ Unix-only session implementation that runs commands on the host and uses the host filesystem @@ -1092,7 +1113,6 @@ async def write( workspace_path = self.normalize_path(path, for_write=True) if user is not None: - _raise_if_existing_special_file(workspace_path) await self._write_stream_with_exec(workspace_path, payload.stream, user=user) return @@ -1116,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, @@ -1154,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, diff --git a/tests/sandbox/test_unix_local.py b/tests/sandbox/test_unix_local.py index f89424c831..33c1e2073b 100644 --- a/tests/sandbox/test_unix_local.py +++ b/tests/sandbox/test_unix_local.py @@ -1,11 +1,13 @@ from __future__ import annotations import asyncio -import errno import io import os +import shutil import signal +import subprocess import tarfile +import tempfile import threading import time from pathlib import Path @@ -592,17 +594,18 @@ async def test_unix_local_refuses_a_fifo_without_a_peer_and_a_directory_read( @pytest.mark.skipif(not hasattr(os, "mkfifo"), reason="requires FIFO support") -def test_open_regular_file_classifies_a_fifo_before_opening_it( +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, *args: object, **kwargs: object) -> int: - if Path(str(path)) == fifo: - raise AssertionError("the FIFO must be rejected without being opened") - return real_open(cast(Any, path), *cast(Any, args), **cast(Any, kwargs)) + 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) @@ -611,23 +614,80 @@ def guarded_open(path: object, *args: object, **kwargs: object) -> int: 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_scoped_write_refuses_a_fifo_the_sdk_identity_cannot_open( - tmp_path: Path, monkeypatch: pytest.MonkeyPatch -) -> None: +def test_user_write_script_refuses_a_fifo_and_writes_regular_files(tmp_path: Path) -> None: fifo = tmp_path / "pipe" os.mkfifo(fifo) - def denied_open(path: object, *args: object, **kwargs: object) -> int: - raise PermissionError(errno.EACCES, os.strerror(errno.EACCES), str(path)) + 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" - monkeypatch.setattr(unix_local_module.os, "open", denied_open) + 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" - with pytest.raises(WorkspaceArchiveWriteError) as write_error: - unix_local_module._raise_if_existing_special_file(fifo) - assert write_error.value.context["reason"] == "not a regular file: fifo" - # A missing target is left to the user-scoped exec, which creates it. - unix_local_module._raise_if_existing_special_file(tmp_path / "new.txt") +@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)