From 425656b0c3c589f9bbd68e02a65cb115822199cf Mon Sep 17 00:00:00 2001 From: dzerik Date: Fri, 15 May 2026 11:07:44 +0300 Subject: [PATCH 01/12] ffi: add SANDLOCK_EXCEPTION_DENY_EIO policy variant Per the maintainer's RFC #43 response on Q5 (let audit-only handlers opt down to Errno(EIO) or Continue), add a fourth exception policy discriminant for EIO. Python audit handlers idiomatically prefer EIO because it propagates as OSError rather than PermissionError, which is closer to what callers expect from a failed syscall. Reserves discriminant 3 (after KILL=0, DENY_EPERM=1, CONTINUE=2 to preserve ABI stability with the merged Handler C ABI). --- crates/sandlock-ffi/include/sandlock.h | 4 +++ crates/sandlock-ffi/src/handler/abi.rs | 8 +++++- crates/sandlock-ffi/src/handler/adapter.rs | 1 + crates/sandlock-ffi/tests/handler_smoke.rs | 29 ++++++++++++++++++++-- 4 files changed, 39 insertions(+), 3 deletions(-) diff --git a/crates/sandlock-ffi/include/sandlock.h b/crates/sandlock-ffi/include/sandlock.h index 208744a..ec12efb 100644 --- a/crates/sandlock-ffi/include/sandlock.h +++ b/crates/sandlock-ffi/include/sandlock.h @@ -245,6 +245,10 @@ typedef enum sandlock_exception_policy { SANDLOCK_EXCEPTION_DENY_EPERM = 1, /** Let the syscall continue unchanged (explicit fail-open). */ SANDLOCK_EXCEPTION_CONTINUE = 2, + /** Fail the syscall with EIO. Idiomatic for audit-only handlers that + * propagate the failure as a plain OSError rather than + * PermissionError. */ + SANDLOCK_EXCEPTION_DENY_EIO = 3, } sandlock_exception_policy_t; /** Opaque handler container. diff --git a/crates/sandlock-ffi/src/handler/abi.rs b/crates/sandlock-ffi/src/handler/abi.rs index 5241674..51761cf 100644 --- a/crates/sandlock-ffi/src/handler/abi.rs +++ b/crates/sandlock-ffi/src/handler/abi.rs @@ -333,6 +333,11 @@ pub enum sandlock_exception_policy_t { /// only safe when the syscall is *also* allowed by the BPF filter and /// Landlock layer (e.g. observability handlers). Continue = 2, + /// Treat the failure as `NotifAction::Errno(EIO)`. Idiomatic for + /// audit-only handlers: EIO propagates to the caller as a plain + /// `OSError` rather than `PermissionError`, which is closer to what + /// callers expect from a failed syscall. + DenyEio = 3, } /// C-callable handler entry point. @@ -417,7 +422,7 @@ impl Drop for sandlock_handler_t { /// (b) the supervisor takes ownership via `sandlock_run_with_handlers` /// and the run completes. /// If `on_exception` does not match a defined `sandlock_exception_policy_t` -/// discriminant (0, 1, or 2), the call returns null and no allocation occurs. +/// discriminant (0, 1, 2, or 3), the call returns null and no allocation occurs. #[no_mangle] pub unsafe extern "C" fn sandlock_handler_new( handler_fn: Option, @@ -432,6 +437,7 @@ pub unsafe extern "C" fn sandlock_handler_new( 0 => sandlock_exception_policy_t::Kill, 1 => sandlock_exception_policy_t::DenyEperm, 2 => sandlock_exception_policy_t::Continue, + 3 => sandlock_exception_policy_t::DenyEio, // Reject out-of-range discriminants at the FFI boundary so we never // store an invalid enum value into the struct — reading one later // via `match` would be undefined behaviour. diff --git a/crates/sandlock-ffi/src/handler/adapter.rs b/crates/sandlock-ffi/src/handler/adapter.rs index 82f4d21..88f42ed 100644 --- a/crates/sandlock-ffi/src/handler/adapter.rs +++ b/crates/sandlock-ffi/src/handler/adapter.rs @@ -77,6 +77,7 @@ impl FfiHandler { } } sandlock_exception_policy_t::DenyEperm => NotifAction::Errno(libc::EPERM), + sandlock_exception_policy_t::DenyEio => NotifAction::Errno(libc::EIO), sandlock_exception_policy_t::Continue => NotifAction::Continue, } } diff --git a/crates/sandlock-ffi/tests/handler_smoke.rs b/crates/sandlock-ffi/tests/handler_smoke.rs index 2e41224..b4cc05f 100644 --- a/crates/sandlock-ffi/tests/handler_smoke.rs +++ b/crates/sandlock-ffi/tests/handler_smoke.rs @@ -241,10 +241,10 @@ fn handler_new_and_free_round_trip() { #[test] fn handler_new_rejects_invalid_exception_policy() { - // Cover the boundary (one past the highest valid Continue=2), + // Cover the boundary (one past the highest valid DenyEio=3), // a mid-range value, and the extreme u32::MAX. A mutation that // rejects only specific values would fail at least one of these. - for bad in [3u32, 4u32, 99u32, u32::MAX] { + for bad in [4u32, 5u32, 99u32, u32::MAX] { let h = unsafe { sandlock_handler_new( Some(test_handler as sandlock_handler_fn_t), @@ -2095,3 +2095,28 @@ fn a5_handler_free_unwinds_on_panicking_dropper() { "expected sandlock_handler_free to unwind a panicking dropper instead of aborting", ); } + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn ffi_handler_deny_eio_policy_on_callback_rc_nonzero() { + extern "C-unwind" fn returns_error( + _ud: *mut std::ffi::c_void, + _n: *const sandlock_ffi::notif_repr::sandlock_notif_data_t, + _m: *mut sandlock_ffi::handler::sandlock_mem_handle_t, + _out: *mut sandlock_ffi::handler::sandlock_action_out_t, + ) -> i32 { + -1 + } + let raw = unsafe { + sandlock_ffi::handler::sandlock_handler_new( + Some(returns_error), + std::ptr::null_mut(), + None, + sandlock_ffi::handler::sandlock_exception_policy_t::DenyEio as u32, + ) + }; + let h = unsafe { sandlock_ffi::handler::FfiHandler::from_raw(raw) }; + let cx = fake_ctx(); + let action = h.handle(&cx).await; + assert!(matches!(action, NotifAction::Errno(e) if e == libc::EIO), + "expected Errno(EIO), got {:?}", action); +} From f84006405190432a053f98a54bd6119b2502dea7 Mon Sep 17 00:00:00 2001 From: dzerik Date: Fri, 15 May 2026 11:13:10 +0300 Subject: [PATCH 02/12] python: introduce NotifAction value-object Discriminated dataclass mirroring sandlock_action_out_t. Constructed via classmethod factories. Discriminant values match the C-side SANDLOCK_ACTION_* constants 1:1 so the trampoline (a later task) can translate directly. --- python/src/sandlock/handler.py | 95 ++++++++++++++++++++++++++++++ python/tests/test_handler_smoke.py | 52 ++++++++++++++++ 2 files changed, 147 insertions(+) create mode 100644 python/src/sandlock/handler.py create mode 100644 python/tests/test_handler_smoke.py diff --git a/python/src/sandlock/handler.py b/python/src/sandlock/handler.py new file mode 100644 index 0000000..0dcbc89 --- /dev/null +++ b/python/src/sandlock/handler.py @@ -0,0 +1,95 @@ +# SPDX-License-Identifier: Apache-2.0 +"""Python wrapper for the sandlock Handler ABI. + +The C ABI (see ``crates/sandlock-ffi/include/sandlock.h``) is mapped via +ctypes; this module exposes a pythonic Handler base class and a +NotifAction value-object. + +The wrapper is strictly minimal — ergonomic helpers (path readers, +preset handlers, asyncio adapters) are deferred to a follow-up. +""" + +from __future__ import annotations + +import enum +from dataclasses import dataclass + + +# Discriminant values mirror SANDLOCK_ACTION_* in sandlock.h. +class _ActionKind(enum.IntEnum): + UNSET = 0 + CONTINUE = 1 + ERRNO = 2 + RETURN_VALUE = 3 + INJECT_FD_SEND = 4 + INJECT_FD_SEND_TRACKED = 5 # reserved; setter not exposed + HOLD = 6 + KILL = 7 + + +@dataclass(frozen=True) +class NotifAction: + """Decision returned from a Python ``Handler.handle`` call. + + Construct via the factory classmethods (``NotifAction.continue_()``, + ``NotifAction.errno(13)``, etc.); do not instantiate directly. + + Field semantics depend on ``kind``: + + - CONTINUE: no payload fields used. + - ERRNO: ``errno_value`` set. + - RETURN_VALUE: ``return_value`` set (factory: ``return_value_``). + - INJECT_FD_SEND: ``srcfd``, ``newfd_flags`` set; the supervisor + takes ownership of the fd on dispatch. + - HOLD: no payload fields used. + - KILL: ``sig``, ``pgid`` set. ``pgid == 0`` substitutes the + supervisor-resolved child pgid; if the supervisor cannot safely + resolve one, the action is refused and the exception policy + applies. + + ``srcfd`` defaults to ``-1`` (not a valid fd) for every action + kind other than INJECT_FD_SEND. + """ + + kind: int # discriminant; values from _ActionKind / sandlock_action_kind_t + errno_value: int = 0 + return_value: int = 0 + srcfd: int = -1 + newfd_flags: int = 0 + sig: int = 0 + pgid: int = 0 + + @classmethod + def continue_(cls) -> "NotifAction": + return cls(kind=int(_ActionKind.CONTINUE)) + + @classmethod + def errno(cls, value: int) -> "NotifAction": + return cls(kind=int(_ActionKind.ERRNO), errno_value=value) + + @classmethod + def return_value_(cls, value: int) -> "NotifAction": + return cls(kind=int(_ActionKind.RETURN_VALUE), return_value=value) + + @classmethod + def hold(cls) -> "NotifAction": + return cls(kind=int(_ActionKind.HOLD)) + + @classmethod + def kill(cls, sig: int, pgid: int = 0) -> "NotifAction": + return cls(kind=int(_ActionKind.KILL), sig=sig, pgid=pgid) + + @classmethod + def inject_fd_send(cls, srcfd: int, newfd_flags: int = 0) -> "NotifAction": + """Inject a file descriptor into the child. + + Ownership of ``srcfd`` transfers to the supervisor on successful + dispatch. The Python caller must NOT close ``srcfd`` after + returning this action, regardless of whether the dispatch + actually fires (the supervisor handles cleanup on all paths). + """ + return cls( + kind=int(_ActionKind.INJECT_FD_SEND), + srcfd=srcfd, + newfd_flags=newfd_flags, + ) diff --git a/python/tests/test_handler_smoke.py b/python/tests/test_handler_smoke.py new file mode 100644 index 0000000..5cdfe37 --- /dev/null +++ b/python/tests/test_handler_smoke.py @@ -0,0 +1,52 @@ +# SPDX-License-Identifier: Apache-2.0 +"""Smoke tests for the sandlock Python handler wrapper.""" + +from sandlock.handler import NotifAction + + +def test_notif_action_continue_has_continue_kind(): + a = NotifAction.continue_() + assert a.kind == 1 # SANDLOCK_ACTION_CONTINUE + + +def test_notif_action_errno_carries_value(): + a = NotifAction.errno(13) + assert a.kind == 2 + assert a.errno_value == 13 + + +def test_notif_action_kill_carries_sig_and_pgid(): + a = NotifAction.kill(9, 0) + assert a.kind == 7 + assert a.sig == 9 + assert a.pgid == 0 + + +def test_notif_action_return_value_carries_value(): + a = NotifAction.return_value_(42) + assert a.kind == 3 + assert a.return_value == 42 # field, not the classmethod + + +def test_notif_action_inject_fd_send_carries_srcfd(): + a = NotifAction.inject_fd_send(7) + assert a.kind == 4 + assert a.srcfd == 7 + assert a.newfd_flags == 0 + + +def test_notif_action_inject_fd_send_with_flags(): + a = NotifAction.inject_fd_send(7, newfd_flags=0o2000000) # O_CLOEXEC + assert a.srcfd == 7 + assert a.newfd_flags == 0o2000000 + + +def test_notif_action_is_frozen(): + import dataclasses + a = NotifAction.continue_() + try: + a.kind = 999 # type: ignore[misc] + except dataclasses.FrozenInstanceError: + pass + else: + raise AssertionError("NotifAction must be frozen (immutable)") From 5a1f78347ecedc947a115a2cfe999a33bb3815cf Mon Sep 17 00:00:00 2001 From: dzerik Date: Fri, 15 May 2026 11:20:47 +0300 Subject: [PATCH 03/12] python: add Handler base class and ExceptionPolicy enum Default exception policy is KILL (fail-closed) per RFC #43 Q5 = D. Subclasses can override via class attribute. ExceptionPolicy enum discriminants are stable across the C ABI (KILL=0, DENY_EPERM=1, CONTINUE=2, DENY_EIO=3). --- python/src/sandlock/handler.py | 53 +++++++++++++++++++++++++---- python/tests/test_handler_smoke.py | 54 +++++++++++++++++++++++++++++- 2 files changed, 100 insertions(+), 7 deletions(-) diff --git a/python/src/sandlock/handler.py b/python/src/sandlock/handler.py index 0dcbc89..ff3fab6 100644 --- a/python/src/sandlock/handler.py +++ b/python/src/sandlock/handler.py @@ -60,27 +60,27 @@ class NotifAction: pgid: int = 0 @classmethod - def continue_(cls) -> "NotifAction": + def continue_(cls) -> NotifAction: return cls(kind=int(_ActionKind.CONTINUE)) @classmethod - def errno(cls, value: int) -> "NotifAction": + def errno(cls, value: int) -> NotifAction: return cls(kind=int(_ActionKind.ERRNO), errno_value=value) @classmethod - def return_value_(cls, value: int) -> "NotifAction": + def return_value_(cls, value: int) -> NotifAction: return cls(kind=int(_ActionKind.RETURN_VALUE), return_value=value) @classmethod - def hold(cls) -> "NotifAction": + def hold(cls) -> NotifAction: return cls(kind=int(_ActionKind.HOLD)) @classmethod - def kill(cls, sig: int, pgid: int = 0) -> "NotifAction": + def kill(cls, sig: int, pgid: int = 0) -> NotifAction: return cls(kind=int(_ActionKind.KILL), sig=sig, pgid=pgid) @classmethod - def inject_fd_send(cls, srcfd: int, newfd_flags: int = 0) -> "NotifAction": + def inject_fd_send(cls, srcfd: int, newfd_flags: int = 0) -> NotifAction: """Inject a file descriptor into the child. Ownership of ``srcfd`` transfers to the supervisor on successful @@ -93,3 +93,44 @@ def inject_fd_send(cls, srcfd: int, newfd_flags: int = 0) -> "NotifAction": srcfd=srcfd, newfd_flags=newfd_flags, ) + + +class ExceptionPolicy(enum.IntEnum): + """Maps to sandlock_exception_policy_t in the C ABI. + + Applied when a handler's ``handle()`` raises, returns an invalid + value, or the trampoline cannot reach the Python interpreter + (e.g. ``Py_FinalizeEx``). See ``crates/sandlock-ffi/include/sandlock.h`` + for the supervisor's exact behaviour per policy. + """ + KILL = 0 + DENY_EPERM = 1 + CONTINUE = 2 + DENY_EIO = 3 + + +class Handler: + """Base class for Python sandlock handlers. + + Subclass and override ``handle()``. Optionally override + ``on_exception`` to choose what the supervisor does when this + handler errors. Default is ``ExceptionPolicy.KILL`` (fail-closed). + + Lifetime: a Handler instance must outlive any Sandbox run it is + registered with. The Sandbox holds a Python-side reference for the + duration of the run; the underlying C container's ``ud_drop`` + releases that reference when the run completes (or fails). + """ + + on_exception: ExceptionPolicy = ExceptionPolicy.KILL + + def handle(self, ctx: HandlerCtx) -> NotifAction: + """Override in a subclass to inspect ``ctx`` and return a NotifAction. + + Raising an exception triggers the configured ``on_exception`` + policy. Returning a non-NotifAction value is treated as an + exception. The default implementation raises NotImplementedError. + """ + raise NotImplementedError( + "Handler subclasses must override handle(ctx) -> NotifAction" + ) diff --git a/python/tests/test_handler_smoke.py b/python/tests/test_handler_smoke.py index 5cdfe37..44b2986 100644 --- a/python/tests/test_handler_smoke.py +++ b/python/tests/test_handler_smoke.py @@ -1,7 +1,7 @@ # SPDX-License-Identifier: Apache-2.0 """Smoke tests for the sandlock Python handler wrapper.""" -from sandlock.handler import NotifAction +from sandlock.handler import ExceptionPolicy, Handler, NotifAction def test_notif_action_continue_has_continue_kind(): @@ -50,3 +50,55 @@ def test_notif_action_is_frozen(): pass else: raise AssertionError("NotifAction must be frozen (immutable)") + + +def test_exception_policy_enum_values_match_c_header(): + # Must match include/sandlock.h SANDLOCK_EXCEPTION_* discriminants. + assert ExceptionPolicy.KILL == 0 + assert ExceptionPolicy.DENY_EPERM == 1 + assert ExceptionPolicy.CONTINUE == 2 + assert ExceptionPolicy.DENY_EIO == 3 + + +def test_handler_subclass_has_default_kill_policy(): + class MyHandler(Handler): + def handle(self, ctx): + return NotifAction.continue_() + + h = MyHandler() + assert h.on_exception == ExceptionPolicy.KILL # fail-closed default + + +def test_handler_subclass_can_override_exception_policy(): + class AuditHandler(Handler): + on_exception = ExceptionPolicy.CONTINUE + + def handle(self, ctx): + return NotifAction.continue_() + + h = AuditHandler() + assert h.on_exception == ExceptionPolicy.CONTINUE + + +def test_base_handler_handle_raises_not_implemented(): + h = Handler() + try: + h.handle(None) + except NotImplementedError: + pass + else: + raise AssertionError("base Handler.handle must raise NotImplementedError") + + +def test_action_kind_enum_values_match_c_header(): + # Must match SANDLOCK_ACTION_* in crates/sandlock-ffi/include/sandlock.h. + from sandlock.handler import _ActionKind + + assert _ActionKind.UNSET == 0 + assert _ActionKind.CONTINUE == 1 + assert _ActionKind.ERRNO == 2 + assert _ActionKind.RETURN_VALUE == 3 + assert _ActionKind.INJECT_FD_SEND == 4 + assert _ActionKind.INJECT_FD_SEND_TRACKED == 5 + assert _ActionKind.HOLD == 6 + assert _ActionKind.KILL == 7 From b6938c80a5e6360e40f065565be4a9b7192dad69 Mon Sep 17 00:00:00 2001 From: dzerik Date: Fri, 15 May 2026 11:27:06 +0300 Subject: [PATCH 04/12] python: add HandlerCtx accessor Read-only snapshot of the seccomp notification plus an opaque mem handle for child-memory access. The read_cstr/read/write methods short-circuit to a falsy result when no mem handle is present (test contexts); the real accessors are deferred to the ctypes glue module. --- python/src/sandlock/handler.py | 77 ++++++++++++++++++++++++++++++ python/tests/test_handler_smoke.py | 51 ++++++++++++++++++++ 2 files changed, 128 insertions(+) diff --git a/python/src/sandlock/handler.py b/python/src/sandlock/handler.py index ff3fab6..8bac98c 100644 --- a/python/src/sandlock/handler.py +++ b/python/src/sandlock/handler.py @@ -134,3 +134,80 @@ def handle(self, ctx: HandlerCtx) -> NotifAction: raise NotImplementedError( "Handler subclasses must override handle(ctx) -> NotifAction" ) + + +@dataclass(frozen=True) +class HandlerCtx: + """Read-only snapshot of the seccomp notification the supervisor + received, plus an opaque handle for child-memory access. + + Field names match ``sandlock_notif_data_t`` in the C header. The + ``_mem_handle`` field is an implementation detail (a ctypes + pointer); use ``read_cstr``, ``read``, ``write`` to access child + memory. + + Do not retain a HandlerCtx beyond the ``handle()`` call — the mem + handle is valid only for the duration of the callback. + """ + id: int + pid: int + flags: int + syscall_nr: int + arch: int + instruction_pointer: int + args: tuple[int, int, int, int, int, int] # the six syscall args + + # Set by the trampoline; opaque to user code. None in test contexts. + _mem_handle: object = None + + @classmethod + def _for_test( + cls, + *, + id: int, + pid: int, + flags: int, + syscall_nr: int, + arch: int, + instruction_pointer: int, + args: tuple[int, int, int, int, int, int], + ) -> HandlerCtx: + """Test-only constructor with no mem handle.""" + return cls( + id=id, pid=pid, flags=flags, syscall_nr=syscall_nr, + arch=arch, instruction_pointer=instruction_pointer, + args=args, _mem_handle=None, + ) + + def read_cstr(self, addr: int, max_len: int) -> str | None: + """Read a NUL-terminated string from the child at ``addr``. + + Returns the decoded string on success, or None on failure + (invalid address, target string longer than max_len, race with + child exit, or no mem handle). ``max_len`` must be at least 1 + to fit the NUL terminator. + """ + if self._mem_handle is None: + return None + from . import _handler_ffi + return _handler_ffi.mem_read_cstr(self._mem_handle, addr, max_len) + + def read(self, addr: int, length: int) -> bytes | None: + """Read ``length`` raw bytes from the child at ``addr``. + + Returns bytes on success, or None on failure. + """ + if self._mem_handle is None: + return None + from . import _handler_ffi + return _handler_ffi.mem_read(self._mem_handle, addr, length) + + def write(self, addr: int, data: bytes) -> bool: + """Write ``data`` into the child at ``addr``. + + Returns True on success, False on failure. + """ + if self._mem_handle is None: + return False + from . import _handler_ffi + return _handler_ffi.mem_write(self._mem_handle, addr, data) diff --git a/python/tests/test_handler_smoke.py b/python/tests/test_handler_smoke.py index 44b2986..101077b 100644 --- a/python/tests/test_handler_smoke.py +++ b/python/tests/test_handler_smoke.py @@ -102,3 +102,54 @@ def test_action_kind_enum_values_match_c_header(): assert _ActionKind.INJECT_FD_SEND_TRACKED == 5 assert _ActionKind.HOLD == 6 assert _ActionKind.KILL == 7 + + +def test_handler_ctx_exposes_notif_fields(): + from sandlock.handler import HandlerCtx + + # Construct via the test helper; the production constructor is + # called only from the trampoline. + ctx = HandlerCtx._for_test( + id=42, pid=1234, flags=0, + syscall_nr=39, arch=0xC000003E, + instruction_pointer=0xDEADBEEF, + args=(1, 2, 3, 4, 5, 6), + ) + assert ctx.id == 42 + assert ctx.pid == 1234 + assert ctx.flags == 0 + assert ctx.syscall_nr == 39 + assert ctx.arch == 0xC000003E + assert ctx.instruction_pointer == 0xDEADBEEF + assert ctx.args == (1, 2, 3, 4, 5, 6) + + +def test_handler_ctx_mem_methods_return_falsy_without_handle(): + from sandlock.handler import HandlerCtx + + # _for_test ctx has no mem handle — accessors must degrade safely, + # not crash. + ctx = HandlerCtx._for_test( + id=1, pid=1, flags=0, syscall_nr=0, arch=0, + instruction_pointer=0, args=(0, 0, 0, 0, 0, 0), + ) + assert ctx.read_cstr(0x1000, 64) is None + assert ctx.read(0x1000, 16) is None + assert ctx.write(0x1000, b"x") is False + + +def test_handler_ctx_is_frozen(): + import dataclasses + + from sandlock.handler import HandlerCtx + + ctx = HandlerCtx._for_test( + id=1, pid=1, flags=0, syscall_nr=0, arch=0, + instruction_pointer=0, args=(0, 0, 0, 0, 0, 0), + ) + try: + ctx.pid = 999 # type: ignore[misc] + except dataclasses.FrozenInstanceError: + pass + else: + raise AssertionError("HandlerCtx must be frozen (immutable)") From a8e28812159f1213e8ec4407c86f86c1d46a63eb Mon Sep 17 00:00:00 2001 From: dzerik Date: Fri, 15 May 2026 11:39:41 +0300 Subject: [PATCH 05/12] python: trampoline + Sandbox.run_with_handlers ctypes bindings for the merged Handler C ABI. The trampoline dispatches via an integer-id registry lookup, relies on ctypes' implicit GIL acquisition for CFUNCTYPE callbacks, checks Py_IsInitialized() defensively, catches handler exceptions (routing to the configured on_exception policy), and translates NotifAction into setter calls. Adds Sandbox.run_with_handlers and exports Handler/NotifAction/ HandlerCtx/ExceptionPolicy from the package root. Includes the RFC #43 audit smoke test counting SYS_openat interceptions. --- python/src/sandlock/__init__.py | 6 + python/src/sandlock/_handler_ffi.py | 211 ++++++++++++++++++++++++++++ python/src/sandlock/_sdk.py | 143 +++++++++++++++++++ python/src/sandlock/sandbox.py | 129 +++++++++++++++++ python/tests/test_handler_smoke.py | 118 ++++++++++++++++ 5 files changed, 607 insertions(+) create mode 100644 python/src/sandlock/_handler_ffi.py diff --git a/python/src/sandlock/__init__.py b/python/src/sandlock/__init__.py index e39ed69..fba0c52 100644 --- a/python/src/sandlock/__init__.py +++ b/python/src/sandlock/__init__.py @@ -12,6 +12,7 @@ landlock_abi_version, min_landlock_abi, confine, ) from .inputs import inputs +from .handler import Handler, NotifAction, HandlerCtx, ExceptionPolicy from .sandbox import Sandbox, FsIsolation, BranchAction, parse_ports, Change, DryRunResult from ._profile import load_profile, list_profiles from .exceptions import ( @@ -48,6 +49,11 @@ "parse_ports", "Change", "DryRunResult", + # Handler ABI + "Handler", + "NotifAction", + "HandlerCtx", + "ExceptionPolicy", # Platform "landlock_abi_version", "min_landlock_abi", diff --git a/python/src/sandlock/_handler_ffi.py b/python/src/sandlock/_handler_ffi.py new file mode 100644 index 0000000..aa79811 --- /dev/null +++ b/python/src/sandlock/_handler_ffi.py @@ -0,0 +1,211 @@ +# SPDX-License-Identifier: Apache-2.0 +"""Internal trampoline bridging the C Handler ABI to Python ``Handler``. + +Not part of the public API — see ``handler.py`` for the public surface +and ``_sdk.py`` for the raw ctypes bindings. + +GIL and interpreter safety +-------------------------- +- ctypes' ``CFUNCTYPE`` callback wrappers acquire the GIL automatically + (via ``PyGILState_Ensure``) before invoking the Python callback, even + when the call originates on a thread the interpreter has never seen. + The supervisor dispatches handler callbacks from its own worker + threads, so this implicit acquisition is what makes the trampoline + safe; no manual ``PyGILState_Ensure`` is needed. +- We additionally check ``Py_IsInitialized()`` before touching any + Python state, in case the interpreter is being finalized while the + supervisor is mid-dispatch. On false, the trampoline returns ``-1`` + (rc != 0), routing the notification through the configured exception + policy. +- Native crashes (SIGSEGV) inside a handler are NOT recoverable; that is + documented as caller responsibility. + +Handler dispatch +---------------- +Each registered ``Handler`` is stored in a process-global registry keyed +by an integer id. The ``ud`` slot handed to the C ABI is that id cast to +``c_void_p`` — never a raw ``PyObject*``. When the supervisor frees a +handler container it invokes the ``ud_drop`` callback, which removes the +registry entry. +""" + +from __future__ import annotations + +import ctypes +import threading +from typing import Dict + +from . import _sdk +from .handler import Handler, HandlerCtx, NotifAction, _ActionKind + + +# ---------------------------------------------------------------- +# Handler registry +# ---------------------------------------------------------------- + +# Strong references to every registered Handler, keyed by integer id. +# The C ABI's ``ud`` pointer is this id; ``ud_drop`` removes the entry. +_HANDLERS: Dict[int, Handler] = {} +_REGISTRY_LOCK = threading.Lock() +_NEXT_ID = 1 + + +def _register_handler(handler: Handler) -> int: + """Insert ``handler`` into the registry and return its integer id.""" + global _NEXT_ID + with _REGISTRY_LOCK: + hid = _NEXT_ID + _NEXT_ID += 1 + _HANDLERS[hid] = handler + return hid + + +def _unregister_handler(hid: int) -> None: + """Remove the handler with id ``hid`` from the registry, if present.""" + with _REGISTRY_LOCK: + _HANDLERS.pop(hid, None) + + +# ---------------------------------------------------------------- +# Trampoline + ud_drop +# ---------------------------------------------------------------- + +def _trampoline_impl(ud, notif_ptr, mem_ptr, out_ptr) -> int: + """C-ABI handler callback. Returns 0 on success, -1 on any failure. + + A -1 return routes the notification through the handler's configured + ``on_exception`` policy (the supervisor owns that decision). + """ + # The interpreter may be finalizing while the supervisor dispatches. + if not ctypes.pythonapi.Py_IsInitialized(): + return -1 + + # ``ud`` arrives as a Python int (or None for a null pointer). + if ud is None: + return -1 + with _REGISTRY_LOCK: + handler = _HANDLERS.get(int(ud)) + if handler is None: + return -1 # registration gone — race with sandbox teardown + + notif = notif_ptr.contents + ctx = HandlerCtx( + id=notif.id, + pid=notif.pid, + flags=notif.flags, + syscall_nr=notif.syscall_nr, + arch=notif.arch, + instruction_pointer=notif.instruction_pointer, + args=tuple(notif.args), + _mem_handle=mem_ptr, + ) + + try: + action = handler.handle(ctx) + except BaseException: + # Any exception → defer to the configured on_exception policy. + return -1 + + if not isinstance(action, NotifAction): + return -1 # contract violation: handle() must return a NotifAction + + kind = action.kind + if kind == int(_ActionKind.CONTINUE): + _sdk._lib.sandlock_action_set_continue(out_ptr) + elif kind == int(_ActionKind.ERRNO): + _sdk._lib.sandlock_action_set_errno(out_ptr, action.errno_value) + elif kind == int(_ActionKind.RETURN_VALUE): + _sdk._lib.sandlock_action_set_return_value(out_ptr, action.return_value) + elif kind == int(_ActionKind.HOLD): + _sdk._lib.sandlock_action_set_hold(out_ptr) + elif kind == int(_ActionKind.KILL): + _sdk._lib.sandlock_action_set_kill(out_ptr, action.sig, action.pgid) + elif kind == int(_ActionKind.INJECT_FD_SEND): + _sdk._lib.sandlock_action_set_inject_fd_send( + out_ptr, action.srcfd, action.newfd_flags, + ) + else: + # UNSET, INJECT_FD_SEND_TRACKED (no setter), or an unknown tag. + return -1 + return 0 + + +def _ud_drop_impl(ud) -> None: + """C-ABI destructor: drop the handler's registry entry on free. + + Fires exactly once per container — including when ``ud`` is null + (the C ABI guarantees this). + """ + if not ctypes.pythonapi.Py_IsInitialized(): + return + if ud is None: + return + _unregister_handler(int(ud)) + + +# A single trampoline and ud_drop pair is reused across every handler +# registration — dispatch is by the ``ud`` integer-id lookup. These +# ctypes callback objects MUST stay alive for as long as the C side may +# invoke them, so they are bound at module scope (the supervisor only +# touches them between sandlock_run_with_handlers entry and return). +_TRAMPOLINE = _sdk._HANDLER_FN_TYPE(_trampoline_impl) +_UD_DROP = _sdk._UD_DROP_FN_TYPE(_ud_drop_impl) + + +def _make_trampoline(): + """Return the shared C-callable handler trampoline.""" + return _TRAMPOLINE + + +def _make_ud_drop(): + """Return the shared C-callable ud_drop destructor.""" + return _UD_DROP + + +# ---------------------------------------------------------------- +# Child-memory accessors (back HandlerCtx.read_cstr / read / write) +# ---------------------------------------------------------------- + +def mem_read_cstr(mem_handle, addr: int, max_len: int) -> str | None: + """Read a NUL-terminated string from the child at ``addr``. + + Returns the decoded string on success, or None on failure. + """ + if mem_handle is None or max_len < 1: + return None + buf = (ctypes.c_uint8 * max_len)() + out_len = ctypes.c_size_t(0) + rc = _sdk._lib.sandlock_mem_read_cstr( + mem_handle, addr, buf, max_len, ctypes.byref(out_len), + ) + if rc != 0: + return None + return bytes(buf[:out_len.value]).decode("utf-8", errors="replace") + + +def mem_read(mem_handle, addr: int, length: int) -> bytes | None: + """Read ``length`` raw bytes from the child at ``addr``. + + Returns the bytes copied on success, or None on failure. + """ + if mem_handle is None or length < 1: + return b"" if length == 0 else None + buf = (ctypes.c_uint8 * length)() + out_len = ctypes.c_size_t(0) + rc = _sdk._lib.sandlock_mem_read( + mem_handle, addr, buf, length, ctypes.byref(out_len), + ) + if rc != 0: + return None + return bytes(buf[:out_len.value]) + + +def mem_write(mem_handle, addr: int, data: bytes) -> bool: + """Write ``data`` into the child at ``addr``. Returns True on success.""" + if mem_handle is None: + return False + if len(data) == 0: + return True + buf = (ctypes.c_uint8 * len(data)).from_buffer_copy(data) + rc = _sdk._lib.sandlock_mem_write(mem_handle, addr, buf, len(data)) + return rc == 0 diff --git a/python/src/sandlock/_sdk.py b/python/src/sandlock/_sdk.py index 2659080..286fe75 100644 --- a/python/src/sandlock/_sdk.py +++ b/python/src/sandlock/_sdk.py @@ -398,6 +398,149 @@ def confine(policy: "PolicyDataclass") -> None: _lib.sandlock_checkpoint_free.argtypes = [_c_checkpoint_p] +# ---------------------------------------------------------------- +# Handler ABI — extension handlers for seccomp-notif syscalls. +# +# Structures mirror the C ABI in crates/sandlock-ffi/include/sandlock.h; +# the trampoline that drives these bindings lives in _handler_ffi.py. +# ---------------------------------------------------------------- + +# sandlock_notif_data_t — kernel seccomp-notification snapshot. The +# `args` array is fixed at 6 entries (the syscall ABI maximum). +class _SandlockNotifData(ctypes.Structure): + _fields_ = [ + ("id", ctypes.c_uint64), + ("pid", ctypes.c_uint32), + ("flags", ctypes.c_uint32), + ("syscall_nr", ctypes.c_int32), + ("arch", ctypes.c_uint32), + ("instruction_pointer", ctypes.c_uint64), + ("args", ctypes.c_uint64 * 6), + ] + + +# sandlock_action_payload_t — the tagged union the setters fill in. The +# trampoline never reads these fields directly (it only ever calls the +# setters), but the layout must match so the struct is sized correctly. +class _SandlockActionPayload(ctypes.Union): + _fields_ = [ + ("none", ctypes.c_uint64), + ("errno_value", ctypes.c_int32), + ("return_value", ctypes.c_int64), + # inject_send: { int32 srcfd; uint32 newfd_flags; } + ("inject_send", ctypes.c_uint32 * 2), + # inject_send_tracked: { int32; uint32; uint64; } — reserved. + ("inject_send_tracked", ctypes.c_uint64 * 2), + # kill: { int32 sig; int32 pgid; } + ("kill", ctypes.c_int32 * 2), + ] + + +# sandlock_action_out_t — the slot a handler writes its decision into. +class _SandlockActionOut(ctypes.Structure): + _fields_ = [ + ("kind", ctypes.c_uint32), + ("payload", _SandlockActionPayload), + ] + + +# sandlock_handler_registration_t — one (syscall_nr, handler) pair. +class _SandlockHandlerRegistration(ctypes.Structure): + _fields_ = [ + ("syscall_nr", ctypes.c_int64), + ("handler", ctypes.c_void_p), + ] + + +_c_mem_handle_p = ctypes.c_void_p + +# C handler signature: +# int (*)(void *ud, const sandlock_notif_data_t *notif, +# sandlock_mem_handle_t *mem, sandlock_action_out_t *out) +_HANDLER_FN_TYPE = ctypes.CFUNCTYPE( + ctypes.c_int, + ctypes.c_void_p, # ud + ctypes.POINTER(_SandlockNotifData), # notif + _c_mem_handle_p, # mem + ctypes.POINTER(_SandlockActionOut), # out +) + +# void (*)(void *ud) +_UD_DROP_FN_TYPE = ctypes.CFUNCTYPE(None, ctypes.c_void_p) + +_c_handler_p = ctypes.c_void_p + +_lib.sandlock_handler_new.restype = _c_handler_p +_lib.sandlock_handler_new.argtypes = [ + _HANDLER_FN_TYPE, ctypes.c_void_p, _UD_DROP_FN_TYPE, ctypes.c_uint32, +] + +_lib.sandlock_handler_free.restype = None +_lib.sandlock_handler_free.argtypes = [_c_handler_p] + +_lib.sandlock_run_with_handlers.restype = _c_result_p +_lib.sandlock_run_with_handlers.argtypes = [ + _c_policy_p, ctypes.c_char_p, + ctypes.POINTER(ctypes.c_char_p), ctypes.c_uint, + ctypes.POINTER(_SandlockHandlerRegistration), ctypes.c_size_t, +] + +_lib.sandlock_run_interactive_with_handlers.restype = _c_result_p +_lib.sandlock_run_interactive_with_handlers.argtypes = [ + _c_policy_p, ctypes.c_char_p, + ctypes.POINTER(ctypes.c_char_p), ctypes.c_uint, + ctypes.POINTER(_SandlockHandlerRegistration), ctypes.c_size_t, +] + +# Action setters — exactly one per action, called from the trampoline. +_lib.sandlock_action_set_continue.restype = None +_lib.sandlock_action_set_continue.argtypes = [ctypes.POINTER(_SandlockActionOut)] + +_lib.sandlock_action_set_errno.restype = None +_lib.sandlock_action_set_errno.argtypes = [ + ctypes.POINTER(_SandlockActionOut), ctypes.c_int32, +] + +_lib.sandlock_action_set_return_value.restype = None +_lib.sandlock_action_set_return_value.argtypes = [ + ctypes.POINTER(_SandlockActionOut), ctypes.c_int64, +] + +_lib.sandlock_action_set_inject_fd_send.restype = None +_lib.sandlock_action_set_inject_fd_send.argtypes = [ + ctypes.POINTER(_SandlockActionOut), ctypes.c_int32, ctypes.c_uint32, +] + +_lib.sandlock_action_set_hold.restype = None +_lib.sandlock_action_set_hold.argtypes = [ctypes.POINTER(_SandlockActionOut)] + +_lib.sandlock_action_set_kill.restype = None +_lib.sandlock_action_set_kill.argtypes = [ + ctypes.POINTER(_SandlockActionOut), ctypes.c_int32, ctypes.c_int32, +] + +# Child-memory accessors — valid only for the duration of a callback. +_lib.sandlock_mem_read_cstr.restype = ctypes.c_int +_lib.sandlock_mem_read_cstr.argtypes = [ + _c_mem_handle_p, ctypes.c_uint64, + ctypes.POINTER(ctypes.c_uint8), ctypes.c_size_t, + ctypes.POINTER(ctypes.c_size_t), +] + +_lib.sandlock_mem_read.restype = ctypes.c_int +_lib.sandlock_mem_read.argtypes = [ + _c_mem_handle_p, ctypes.c_uint64, + ctypes.POINTER(ctypes.c_uint8), ctypes.c_size_t, + ctypes.POINTER(ctypes.c_size_t), +] + +_lib.sandlock_mem_write.restype = ctypes.c_int +_lib.sandlock_mem_write.argtypes = [ + _c_mem_handle_p, ctypes.c_uint64, + ctypes.POINTER(ctypes.c_uint8), ctypes.c_size_t, +] + + # ---------------------------------------------------------------- # SyscallEvent & PolicyContext (Python wrappers for policy_fn) # ---------------------------------------------------------------- diff --git a/python/src/sandlock/sandbox.py b/python/src/sandlock/sandbox.py index 6efb578..74f42b0 100644 --- a/python/src/sandlock/sandbox.py +++ b/python/src/sandlock/sandbox.py @@ -518,6 +518,135 @@ def run(self, cmd: Sequence[str], timeout: float | None = None): stderr=stderr, ) + def run_with_handlers( + self, + cmd: Sequence[str], + handlers: Sequence, + name: str | None = None, + ): + """Run ``cmd`` under this sandbox with extra seccomp-notif handlers. + + ``handlers`` is a sequence of ``(syscall_nr, Handler)`` pairs; + ``syscall_nr`` is the kernel syscall number to intercept (e.g. + ``257`` for ``openat`` on x86_64) and ``Handler`` is an instance + of :class:`sandlock.handler.Handler`. See that class for handler + semantics. + + Ownership of every ``Handler`` is held by the sandlock supervisor + for the duration of the run; the Python-side reference is held in + an internal registry and released when the run completes (success + or failure). + + The underlying C ABI builds and drives its own runtime for each + call. Do not invoke this method from a thread that already runs a + Tokio runtime — the FFI panics in that case, and the panic + propagates as a Python exception via ``extern "C-unwind"``. + + Args: + cmd: Command and arguments to execute. + handlers: Sequence of ``(syscall_nr, Handler)`` pairs. + name: Optional sandbox name. ``None`` resolves to the same + auto-generated name as :meth:`run`. + + Returns: + A :class:`Result` describing the run. + """ + import ctypes + + from . import _handler_ffi + from ._sdk import ( + _SandlockHandlerRegistration, + _encode, + _lib, + _make_argv, + _read_result_bytes, + Result, + ) + + if self._handle is not None: + raise RuntimeError("sandbox is already running") + + handlers = list(handlers) + native = self._ensure_native() + argv, argc = _make_argv(list(cmd)) + resolved_name = name if name is not None else self._resolve_name() + + trampoline = _handler_ffi._make_trampoline() + ud_drop = _handler_ffi._make_ud_drop() + + # Build the registration array. Handler containers allocated here + # are consumed by sandlock_run_with_handlers — on a successful or + # failed call the supervisor frees them (and fires ud_drop). We + # must NOT call sandlock_handler_free on any container handed in. + regs = (_SandlockHandlerRegistration * len(handlers))() + registered_ids: list[int] = [] + try: + for i, (syscall_nr, handler) in enumerate(handlers): + hid = _handler_ffi._register_handler(handler) + registered_ids.append(hid) + container = _lib.sandlock_handler_new( + trampoline, + ctypes.c_void_p(hid), + ud_drop, + int(handler.on_exception), + ) + if not container: + raise RuntimeError( + "sandlock_handler_new returned NULL for syscall " + f"{syscall_nr}" + ) + regs[i].syscall_nr = int(syscall_nr) + regs[i].handler = container + except BaseException: + # Roll back: free every handler container already allocated + # in this loop. sandlock_handler_free fires the container's + # ud_drop, which removes that handler from the registry — so + # we must NOT also call _unregister_handler for those. + # + # BaseException (not Exception) so a KeyboardInterrupt or + # SystemExit raised mid-loop still triggers cleanup before it + # propagates. + for j in range(i): + if regs[j].handler: + _lib.sandlock_handler_free(regs[j].handler) + # The current slot `i` registered a handler id but never got + # a container (sandlock_handler_new failed or a later line + # raised), so its ud_drop will never fire — drop it by hand. + if i < len(registered_ids): + _handler_ffi._unregister_handler(registered_ids[i]) + raise + + result_p = _lib.sandlock_run_with_handlers( + native.ptr, + _encode(resolved_name), + argv, + argc, + regs, + len(handlers), + ) + # Ownership of every container has transferred to the supervisor; + # it has also invoked each ud_drop, clearing the registry entries. + + if not result_p: + return Result( + success=False, + exit_code=-1, + error="sandlock_run_with_handlers failed", + ) + + exit_code = _lib.sandlock_result_exit_code(result_p) + success = _lib.sandlock_result_success(result_p) + stdout = _read_result_bytes(result_p, _lib.sandlock_result_stdout_bytes) + stderr = _read_result_bytes(result_p, _lib.sandlock_result_stderr_bytes) + _lib.sandlock_result_free(result_p) + + return Result( + success=bool(success), + exit_code=exit_code, + stdout=stdout, + stderr=stderr, + ) + def create(self, cmd: Sequence[str]) -> None: """Fork the sandboxed child and install policy. The child is parked between policy install and ``execve``; call ``start()`` diff --git a/python/tests/test_handler_smoke.py b/python/tests/test_handler_smoke.py index 101077b..425e695 100644 --- a/python/tests/test_handler_smoke.py +++ b/python/tests/test_handler_smoke.py @@ -153,3 +153,121 @@ def test_handler_ctx_is_frozen(): pass else: raise AssertionError("HandlerCtx must be frozen (immutable)") + + +# ---------------------------------------------------------------- +# End-to-end audit smoke test (RFC #43 §Phasing item 2). +# ---------------------------------------------------------------- + +import os + +import pytest + +import sandlock + + +# Standard readable paths for a sandboxed python3 child, mirroring +# tests/test_sandbox.py's _PYTHON_READABLE helper. +_PYTHON_READABLE = ["/usr", "/lib", "/lib64", "/bin", "/etc", "/proc", "/dev"] + +# Use a system interpreter that lives inside the readable tree above. +# sys.executable may point at a venv outside the sandbox (e.g. under +# the developer's home directory), which the child cannot exec. +_SYSTEM_PYTHON = "/usr/bin/python3" + + +class _OpenatCounter(Handler): + on_exception = ExceptionPolicy.CONTINUE # audit-only — never block + + def __init__(self): + self.count = 0 + + def handle(self, ctx): + self.count += 1 + return NotifAction.continue_() + + +def test_smoke_audit_openat(): + """RFC #43 phasing item 2: audit handler counting SYS_openat.""" + if not os.path.exists(_SYSTEM_PYTHON): + pytest.skip(f"{_SYSTEM_PYTHON} not available") + + SYS_openat = 257 # x86_64 + + sb = sandlock.Sandbox(fs_readable=_PYTHON_READABLE) + counter = _OpenatCounter() + + result = sb.run_with_handlers( + cmd=[_SYSTEM_PYTHON, "-c", + "import os\n" + "for _ in range(3):\n" + " fd = os.open('/etc/hostname', os.O_RDONLY)\n" + " os.close(fd)\n"], + handlers=[(SYS_openat, counter)], + ) + + assert result.success, result + assert counter.count >= 3, ( + f"expected >=3 openat intercepts, got {counter.count}" + ) + + +# ---------------------------------------------------------------- +# End-to-end failure-path tests: a handler that raises exercises the +# trampoline's exception path (handler raises -> rc -1 -> the +# supervisor applies the configured on_exception policy). +# ---------------------------------------------------------------- + + +class _RaisingHandler(Handler): + """Handler that always raises — exercises the trampoline's + exception path. on_exception is set per-test.""" + + def __init__(self, on_exception): + self.on_exception = on_exception + + def handle(self, ctx): + raise RuntimeError("intentional handler failure") + + +def test_handler_exception_continue_policy_lets_child_run(): + """A handler that raises, with on_exception=CONTINUE, must let the + child run to completion — the trampoline catches the exception and + the supervisor applies the Continue policy.""" + if not os.path.exists(_SYSTEM_PYTHON): + pytest.skip(f"{_SYSTEM_PYTHON} not available") + + SYS_openat = 257 # x86_64 + sb = sandlock.Sandbox(fs_readable=_PYTHON_READABLE) + handler = _RaisingHandler(ExceptionPolicy.CONTINUE) + result = sb.run_with_handlers( + cmd=[_SYSTEM_PYTHON, "-c", + "import os\nfd = os.open('/etc/hostname', os.O_RDONLY)\nos.close(fd)\n"], + handlers=[(SYS_openat, handler)], + ) + # Continue policy: the openat proceeds despite the handler raising, + # so the child completes normally. + assert result.success, result + + +def test_handler_exception_kill_policy_terminates_child(): + """A handler that raises, with on_exception=KILL, must terminate the + child — the trampoline catches the exception and the supervisor + applies the Kill policy.""" + if not os.path.exists(_SYSTEM_PYTHON): + pytest.skip(f"{_SYSTEM_PYTHON} not available") + + SYS_openat = 257 + sb = sandlock.Sandbox(fs_readable=_PYTHON_READABLE) + handler = _RaisingHandler(ExceptionPolicy.KILL) + result = sb.run_with_handlers( + cmd=[_SYSTEM_PYTHON, "-c", + "import os\nfd = os.open('/etc/hostname', os.O_RDONLY)\nos.close(fd)\n"], + handlers=[(SYS_openat, handler)], + ) + # Kill policy: the first intercepted openat that the raising handler + # touches kills the child's process group, so the run does not + # succeed. + assert not result.success, ( + f"expected child terminated by Kill policy, got success: {result}" + ) From d40245bc68e2d46e0922b17d5939818afb71e807 Mon Sep 17 00:00:00 2001 From: dzerik Date: Fri, 15 May 2026 11:50:30 +0300 Subject: [PATCH 06/12] docs: Python wrapper section with threading & safety contract Documents the four cross-cutting concerns from RFC #43: GIL contention, interpreter finalization, native crashes, and Tokio runtime reentrancy. Plus ownership rules for Handler instances and injected file descriptors, and a minimal copy-pasteable example. --- docs/extension-handlers.md | 67 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/docs/extension-handlers.md b/docs/extension-handlers.md index 724f0de..26f3cb3 100644 --- a/docs/extension-handlers.md +++ b/docs/extension-handlers.md @@ -652,3 +652,70 @@ an opaque `void*`; the responsibility is on the C side. See `crates/sandlock-ffi/tests/c/handler_smoke.c` for the canonical end-to-end example. + +## Python wrapper + +The `sandlock.handler` module provides a Python-side wrapper on top of +the C ABI. See `python/tests/test_handler_smoke.py` for working +examples. + +### Minimal example + +```python +import sandlock +from sandlock.handler import ExceptionPolicy, Handler, NotifAction + +class AuditOpens(Handler): + on_exception = ExceptionPolicy.CONTINUE # audit-only — never block + + def handle(self, ctx): + path = ctx.read_cstr(ctx.args[1], max_len=4096) + print(f"opening {path!r}") + return NotifAction.continue_() + +sb = sandlock.Sandbox(fs_readable=["/usr", "/etc", "/lib", "/lib64", "/bin"]) +sb.run_with_handlers( + cmd=["/usr/bin/cat", "/etc/hostname"], + handlers=[(257, AuditOpens())], # 257 = x86_64 SYS_openat +) +``` + +### Threading & safety contract + +- **GIL contention.** Each handler dispatch holds the GIL for the + duration of `handle()`. The supervisor may dispatch handler + callbacks concurrently across different notifications, so design + `handle()` to be fast (sub-millisecond) and to protect any mutable + handler state with your own synchronization. High-frequency + interception (e.g. per-`SYS_openat` audit on a busy workload) will + serialize on the GIL and can stall the supervisor. + +- **Interpreter finalization.** If `Py_FinalizeEx` runs while the + sandbox is still alive (e.g. the main thread exits with handlers + still registered), the trampoline checks `Py_IsInitialized()` and + returns an error, routing the notification through the handler's + `on_exception` policy. Do not rely on this for clean shutdown — wait + for the run to finish before tearing down the interpreter. + +- **Native crashes inside `handle()`.** A segfault inside a Python + handler is not recoverable: the supervisor task hangs and the + trapped child is held indefinitely. Write defensive handlers; this + is a user responsibility. + +- **Tokio runtime reentrancy.** The C ABI's `sandlock_run_with_handlers` + builds and drives its own Tokio runtime internally. Do not call + `Sandbox.run_with_handlers` from a thread that already runs a Tokio + runtime — the FFI will panic, and the panic surfaces as a Python + exception. Pure-Python use (the common case) is unaffected. + +### Ownership rules + +- **Handler instances** must outlive the run. The Sandbox holds a + strong reference for the duration of the run; the reference is + released when the run completes (success or failure). + +- **File descriptors** passed via `NotifAction.inject_fd_send(srcfd)` + transfer ownership to the supervisor on dispatch. The Python caller + must NOT close `srcfd` afterwards, regardless of whether the action + was actually dispatched — the supervisor handles cleanup on all + paths. From 4acaae54fd131f9f457d6cc1ec7ea0be2d8c80ad Mon Sep 17 00:00:00 2001 From: dzerik Date: Fri, 15 May 2026 13:49:59 +0300 Subject: [PATCH 07/12] fix: invalidate HandlerCtx mem handle after the callback returns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The trampoline handed the raw sandlock_mem_handle_t* to HandlerCtx. That C struct is a stack local in the supervisor's spawn_blocking closure — it is freed the instant the callback returns. A handler that stored its HandlerCtx and called read/read_cstr/write afterwards dereferenced a dangling pointer (use-after-free). Wrap the handle in a mutable _MemHandle cell that the trampoline invalidates in a finally block. A retained HandlerCtx now fails safe (accessors return None/False) instead of dereferencing freed memory. --- python/src/sandlock/_handler_ffi.py | 70 +++++++++++++++++------------ python/src/sandlock/handler.py | 61 ++++++++++++++++++++----- python/tests/test_handler_smoke.py | 51 +++++++++++++++++++++ 3 files changed, 142 insertions(+), 40 deletions(-) diff --git a/python/src/sandlock/_handler_ffi.py b/python/src/sandlock/_handler_ffi.py index aa79811..e4765fc 100644 --- a/python/src/sandlock/_handler_ffi.py +++ b/python/src/sandlock/_handler_ffi.py @@ -36,7 +36,7 @@ from typing import Dict from . import _sdk -from .handler import Handler, HandlerCtx, NotifAction, _ActionKind +from .handler import Handler, HandlerCtx, NotifAction, _ActionKind, _MemHandle # ---------------------------------------------------------------- @@ -89,6 +89,12 @@ def _trampoline_impl(ud, notif_ptr, mem_ptr, out_ptr) -> int: return -1 # registration gone — race with sandbox teardown notif = notif_ptr.contents + # ``mem_ptr`` is a raw sandlock_mem_handle_t* pointing at a stack + # local in the supervisor — valid ONLY while handle() is running. + # Wrap it in a fresh liveness cell and invalidate that cell in the + # finally below, so a HandlerCtx that escapes the callback fails + # safe instead of dereferencing a dangling pointer. + mem_cell = _MemHandle(mem_ptr) ctx = HandlerCtx( id=notif.id, pid=notif.pid, @@ -97,37 +103,43 @@ def _trampoline_impl(ud, notif_ptr, mem_ptr, out_ptr) -> int: arch=notif.arch, instruction_pointer=notif.instruction_pointer, args=tuple(notif.args), - _mem_handle=mem_ptr, + _mem_handle=mem_cell, ) try: - action = handler.handle(ctx) - except BaseException: - # Any exception → defer to the configured on_exception policy. - return -1 - - if not isinstance(action, NotifAction): - return -1 # contract violation: handle() must return a NotifAction - - kind = action.kind - if kind == int(_ActionKind.CONTINUE): - _sdk._lib.sandlock_action_set_continue(out_ptr) - elif kind == int(_ActionKind.ERRNO): - _sdk._lib.sandlock_action_set_errno(out_ptr, action.errno_value) - elif kind == int(_ActionKind.RETURN_VALUE): - _sdk._lib.sandlock_action_set_return_value(out_ptr, action.return_value) - elif kind == int(_ActionKind.HOLD): - _sdk._lib.sandlock_action_set_hold(out_ptr) - elif kind == int(_ActionKind.KILL): - _sdk._lib.sandlock_action_set_kill(out_ptr, action.sig, action.pgid) - elif kind == int(_ActionKind.INJECT_FD_SEND): - _sdk._lib.sandlock_action_set_inject_fd_send( - out_ptr, action.srcfd, action.newfd_flags, - ) - else: - # UNSET, INJECT_FD_SEND_TRACKED (no setter), or an unknown tag. - return -1 - return 0 + try: + action = handler.handle(ctx) + except BaseException: + # Any exception → defer to the configured on_exception policy. + return -1 + + if not isinstance(action, NotifAction): + return -1 # contract violation: handle() must return a NotifAction + + kind = action.kind + if kind == int(_ActionKind.CONTINUE): + _sdk._lib.sandlock_action_set_continue(out_ptr) + elif kind == int(_ActionKind.ERRNO): + _sdk._lib.sandlock_action_set_errno(out_ptr, action.errno_value) + elif kind == int(_ActionKind.RETURN_VALUE): + _sdk._lib.sandlock_action_set_return_value(out_ptr, action.return_value) + elif kind == int(_ActionKind.HOLD): + _sdk._lib.sandlock_action_set_hold(out_ptr) + elif kind == int(_ActionKind.KILL): + _sdk._lib.sandlock_action_set_kill(out_ptr, action.sig, action.pgid) + elif kind == int(_ActionKind.INJECT_FD_SEND): + _sdk._lib.sandlock_action_set_inject_fd_send( + out_ptr, action.srcfd, action.newfd_flags, + ) + else: + # UNSET, INJECT_FD_SEND_TRACKED (no setter), or an unknown tag. + return -1 + return 0 + finally: + # The mem handle is dead the instant the callback returns. + # Runs on every exit path — exception, normal return, all of + # it — so any HandlerCtx the handler stashed becomes inert. + mem_cell.invalidate() def _ud_drop_impl(ud) -> None: diff --git a/python/src/sandlock/handler.py b/python/src/sandlock/handler.py index 8bac98c..1795830 100644 --- a/python/src/sandlock/handler.py +++ b/python/src/sandlock/handler.py @@ -136,18 +136,53 @@ def handle(self, ctx: HandlerCtx) -> NotifAction: ) +class _MemHandle: + """Mutable wrapper around the opaque child-memory handle. + + The raw ``sandlock_mem_handle_t*`` is valid only for the duration + of a single ``Handler.handle`` call — it points at a stack local + in the supervisor. The trampoline invalidates this cell when the + callback returns, so a HandlerCtx that escapes its ``handle()`` + call fails fast on the next memory access instead of dereferencing + a dangling pointer. + """ + + __slots__ = ("_ptr", "_live") + + def __init__(self, ptr: object) -> None: + self._ptr = ptr + self._live = True + + def invalidate(self) -> None: + self._live = False + self._ptr = None + + @property + def live(self) -> bool: + return self._live + + @property + def ptr(self) -> object: + return self._ptr + + @dataclass(frozen=True) class HandlerCtx: """Read-only snapshot of the seccomp notification the supervisor received, plus an opaque handle for child-memory access. Field names match ``sandlock_notif_data_t`` in the C header. The - ``_mem_handle`` field is an implementation detail (a ctypes - pointer); use ``read_cstr``, ``read``, ``write`` to access child - memory. + ``_mem_handle`` field is an implementation detail (a ``_MemHandle`` + liveness cell); use ``read_cstr``, ``read``, ``write`` to access + child memory. Do not retain a HandlerCtx beyond the ``handle()`` call — the mem - handle is valid only for the duration of the callback. + handle is valid only for the duration of the callback. The wrapper + now ENFORCES this: the trampoline invalidates the underlying + ``_MemHandle`` cell once the callback returns, so a retained + HandlerCtx fails safe — its memory accessors return ``None`` / + ``False`` rather than dereferencing a dangling pointer (a + use-after-free in C). """ id: int pid: int @@ -157,7 +192,8 @@ class HandlerCtx: instruction_pointer: int args: tuple[int, int, int, int, int, int] # the six syscall args - # Set by the trampoline; opaque to user code. None in test contexts. + # Set by the trampoline to a ``_MemHandle`` liveness cell; opaque to + # user code. None in test contexts. _mem_handle: object = None @classmethod @@ -187,27 +223,30 @@ def read_cstr(self, addr: int, max_len: int) -> str | None: child exit, or no mem handle). ``max_len`` must be at least 1 to fit the NUL terminator. """ - if self._mem_handle is None: + cell = self._mem_handle + if cell is None or not cell.live: return None from . import _handler_ffi - return _handler_ffi.mem_read_cstr(self._mem_handle, addr, max_len) + return _handler_ffi.mem_read_cstr(cell.ptr, addr, max_len) def read(self, addr: int, length: int) -> bytes | None: """Read ``length`` raw bytes from the child at ``addr``. Returns bytes on success, or None on failure. """ - if self._mem_handle is None: + cell = self._mem_handle + if cell is None or not cell.live: return None from . import _handler_ffi - return _handler_ffi.mem_read(self._mem_handle, addr, length) + return _handler_ffi.mem_read(cell.ptr, addr, length) def write(self, addr: int, data: bytes) -> bool: """Write ``data`` into the child at ``addr``. Returns True on success, False on failure. """ - if self._mem_handle is None: + cell = self._mem_handle + if cell is None or not cell.live: return False from . import _handler_ffi - return _handler_ffi.mem_write(self._mem_handle, addr, data) + return _handler_ffi.mem_write(cell.ptr, addr, data) diff --git a/python/tests/test_handler_smoke.py b/python/tests/test_handler_smoke.py index 425e695..000cecc 100644 --- a/python/tests/test_handler_smoke.py +++ b/python/tests/test_handler_smoke.py @@ -219,6 +219,57 @@ def test_smoke_audit_openat(): # ---------------------------------------------------------------- +def test_handler_ctx_mem_handle_invalidated_after_handle(): + """A HandlerCtx that escapes its handle() call must have its mem + accessors fail safe (return None/False) rather than dereference a + dangling C pointer.""" + if not os.path.exists(_SYSTEM_PYTHON): + pytest.skip(f"{_SYSTEM_PYTHON} not available") + + SYS_openat = 257 # x86_64 + + captured = {} + + class _EscapingHandler(Handler): + on_exception = ExceptionPolicy.CONTINUE + + def handle(self, ctx): + captured["ctx"] = ctx # escape the HandlerCtx + # While inside handle(), the handle is live — a read may + # succeed or fail depending on the address, but it must not + # be inert yet: + captured["live_during"] = ( + ctx._mem_handle is not None and ctx._mem_handle.live + ) + return NotifAction.continue_() + + sb = sandlock.Sandbox(fs_readable=_PYTHON_READABLE) + handler = _EscapingHandler() + result = sb.run_with_handlers( + cmd=[_SYSTEM_PYTHON, "-c", + "import os\nfd = os.open('/etc/hostname', os.O_RDONLY)\nos.close(fd)\n"], + handlers=[(SYS_openat, handler)], + ) + assert result.success, result + + escaped = captured["ctx"] + assert captured["live_during"] is True, "mem handle should be live during handle()" + # After the run, the escaped ctx must be INERT — the trampoline's + # finally must have invalidated the cell. This is the load-bearing + # assertion: if the cell is still 'live' the accessors would + # dereference a dangling supervisor pointer (use-after-free). + assert escaped._mem_handle is not None + assert escaped._mem_handle.live is False, ( + "trampoline must invalidate the mem handle once handle() returns; " + "a live cell here means accessors deref freed memory (UAF)" + ) + assert escaped._mem_handle.ptr is None + # And the accessors must fail safe rather than deref a dangling ptr. + assert escaped.read_cstr(0x1000, 64) is None + assert escaped.read(0x1000, 16) is None + assert escaped.write(0x1000, b"x") is False + + class _RaisingHandler(Handler): """Handler that always raises — exercises the trampoline's exception path. on_exception is set per-test.""" From a4c069025f3c5ca5c681f3071e48d08940f5d5a0 Mon Sep 17 00:00:00 2001 From: dzerik Date: Fri, 15 May 2026 13:53:51 +0300 Subject: [PATCH 08/12] fix: plug rollback container leak, harden handler contract T1: run_with_handlers' registration-loop rollback freed only the containers already stored in the regs array. A container created by sandlock_handler_new but not yet stored (e.g. int(syscall_nr) raised in between) leaked. Track the pending container in a local and free it in the rollback path. Also: document that handle() may run concurrently for the same instance and must not block; reject a negative srcfd in NotifAction.inject_fd_send before it reaches the C setter; bind the run name bytes to a local for lifetime clarity; pin Py_IsInitialized's restype explicitly. --- python/src/sandlock/_handler_ffi.py | 7 +++++++ python/src/sandlock/handler.py | 16 +++++++++++++++ python/src/sandlock/sandbox.py | 30 +++++++++++++++++++++++++---- python/tests/test_handler_smoke.py | 6 ++++++ 4 files changed, 55 insertions(+), 4 deletions(-) diff --git a/python/src/sandlock/_handler_ffi.py b/python/src/sandlock/_handler_ffi.py index e4765fc..9018e33 100644 --- a/python/src/sandlock/_handler_ffi.py +++ b/python/src/sandlock/_handler_ffi.py @@ -39,6 +39,13 @@ from .handler import Handler, HandlerCtx, NotifAction, _ActionKind, _MemHandle +# ``ctypes.pythonapi`` is a process-global ``PyDLL``; its function +# objects are shared with every other module in the process. Pin +# ``Py_IsInitialized``'s restype explicitly so dispatch never relies on +# the default (``c_int``) that another module could overwrite. +ctypes.pythonapi.Py_IsInitialized.restype = ctypes.c_int + + # ---------------------------------------------------------------- # Handler registry # ---------------------------------------------------------------- diff --git a/python/src/sandlock/handler.py b/python/src/sandlock/handler.py index 1795830..b2a30a6 100644 --- a/python/src/sandlock/handler.py +++ b/python/src/sandlock/handler.py @@ -88,6 +88,11 @@ def inject_fd_send(cls, srcfd: int, newfd_flags: int = 0) -> NotifAction: returning this action, regardless of whether the dispatch actually fires (the supervisor handles cleanup on all paths). """ + if not isinstance(srcfd, int) or srcfd < 0: + raise ValueError( + f"inject_fd_send: srcfd must be a non-negative int, " + f"got {srcfd!r}" + ) return cls( kind=int(_ActionKind.INJECT_FD_SEND), srcfd=srcfd, @@ -120,6 +125,17 @@ class Handler: registered with. The Sandbox holds a Python-side reference for the duration of the run; the underlying C container's ``ud_drop`` releases that reference when the run completes (or fails). + + Concurrency: the supervisor MAY invoke ``handle()`` concurrently for + the same Handler instance, on different worker threads, for + different notifications. If ``handle()`` mutates instance state, + guard it with your own synchronization — the wrapper does not + serialize handler dispatch. + + Promptness: ``handle()`` must return quickly. It runs synchronously + inside the supervisor's dispatch path while holding the GIL; a + handler that blocks (a long sleep, a blocking I/O call, an infinite + loop) stalls the supervisor and can wedge the entire run. """ on_exception: ExceptionPolicy = ExceptionPolicy.KILL diff --git a/python/src/sandlock/sandbox.py b/python/src/sandlock/sandbox.py index 74f42b0..c4878e6 100644 --- a/python/src/sandlock/sandbox.py +++ b/python/src/sandlock/sandbox.py @@ -580,6 +580,14 @@ def run_with_handlers( # must NOT call sandlock_handler_free on any container handed in. regs = (_SandlockHandlerRegistration * len(handlers))() registered_ids: list[int] = [] + # ``i`` is referenced in the rollback path; keep it bound even if + # ``handlers`` is empty and the loop never runs. + i = 0 + # A container produced by sandlock_handler_new but not yet stored + # into ``regs`` is owned by neither the regs array nor the + # supervisor. Track it here so the rollback path can free it; + # clear it back to None the instant ownership moves into regs. + container = None try: for i, (syscall_nr, handler) in enumerate(handlers): hid = _handler_ffi._register_handler(handler) @@ -597,6 +605,10 @@ def run_with_handlers( ) regs[i].syscall_nr = int(syscall_nr) regs[i].handler = container + # Ownership now lives in regs[i]; clear the pending ref so + # the rollback path does not double-free it (regs[i] is + # already covered by the range(i) loop below). + container = None except BaseException: # Roll back: free every handler container already allocated # in this loop. sandlock_handler_free fires the container's @@ -609,16 +621,26 @@ def run_with_handlers( for j in range(i): if regs[j].handler: _lib.sandlock_handler_free(regs[j].handler) - # The current slot `i` registered a handler id but never got - # a container (sandlock_handler_new failed or a later line - # raised), so its ud_drop will never fire — drop it by hand. + # A container created by sandlock_handler_new in the failing + # iteration but not yet stored into regs[i] (e.g. + # int(syscall_nr) raised between the alloc and the store). + # It is owned by nothing else — free it here. After a + # successful store ``container`` is None, so this branch + # never double-frees a container already covered above. + if container: + _lib.sandlock_handler_free(container) + # The current slot `i` registered a handler id but its + # container's ud_drop will never fire (either no container + # was created, or the one created above is freed by hand and + # its ud_drop only clears that same id) — drop it by hand. if i < len(registered_ids): _handler_ffi._unregister_handler(registered_ids[i]) raise + name_b = _encode(resolved_name) result_p = _lib.sandlock_run_with_handlers( native.ptr, - _encode(resolved_name), + name_b, argv, argc, regs, diff --git a/python/tests/test_handler_smoke.py b/python/tests/test_handler_smoke.py index 000cecc..5587754 100644 --- a/python/tests/test_handler_smoke.py +++ b/python/tests/test_handler_smoke.py @@ -41,6 +41,12 @@ def test_notif_action_inject_fd_send_with_flags(): assert a.newfd_flags == 0o2000000 +def test_inject_fd_send_rejects_negative_srcfd(): + import pytest + with pytest.raises(ValueError): + NotifAction.inject_fd_send(-1) + + def test_notif_action_is_frozen(): import dataclasses a = NotifAction.continue_() From 045e46976cb98179fc5f6cea1c9c4a6cd88eecd3 Mon Sep 17 00:00:00 2001 From: dzerik Date: Fri, 15 May 2026 14:09:00 +0300 Subject: [PATCH 09/12] test: end-to-end coverage for non-Continue handler actions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The deep review found the trampoline's NotifAction kind-dispatch (errno / return_value / kill / inject_fd) had zero end-to-end test coverage — a trampoline reduced to "always Continue" passed the whole suite. Add real-child integration tests: - errno action: child observes EPERM from a denied openat - return_value action: child's getpid returns the synthetic 777 - kill action returned directly: child terminated (on_exception set to Continue to prove the kill came from the action, not a policy fallback) - mem read_cstr: handler decodes the real openat path from child memory and denies a specific file - run_with_handlers argument validation: empty list runs cleanly, a non-Handler entry raises All four behavioural tests were destructively verified — neutering the trampoline's kind-dispatch makes each one fail. --- python/tests/test_handler_smoke.py | 196 +++++++++++++++++++++++++++++ 1 file changed, 196 insertions(+) diff --git a/python/tests/test_handler_smoke.py b/python/tests/test_handler_smoke.py index 5587754..4160562 100644 --- a/python/tests/test_handler_smoke.py +++ b/python/tests/test_handler_smoke.py @@ -328,3 +328,199 @@ def test_handler_exception_kill_policy_terminates_child(): assert not result.success, ( f"expected child terminated by Kill policy, got success: {result}" ) + + +# ---------------------------------------------------------------- +# End-to-end coverage for the trampoline's NotifAction kind-dispatch. +# +# The tests above only ever return NotifAction.continue_() or exercise +# the exception-POLICY path. The branch in _handler_ffi.py that +# translates a RETURNED non-Continue action (errno / return_value / +# kill / inject_fd) into the matching sandlock_action_set_* call had no +# end-to-end coverage — a trampoline reduced to "always Continue" passed +# the whole suite. The tests below make a handler RETURN a non-Continue +# action and observe the child behave accordingly, so a neutered +# kind-dispatch fails them. +# ---------------------------------------------------------------- + + +def test_handler_errno_action_makes_child_observe_eperm(tmp_dir): + """A handler returning NotifAction.errno(EPERM) must make the child's + openat fail with errno EPERM — only reachable if the trampoline + translates the Errno action into sandlock_action_set_errno. + + The handler targets one probe file by path: denying *every* openat + would kill the dynamic loader before the child runs (the test would + then observe EPERM from ld.so, not from the child's own open). It + also avoids /etc/hostname and other supervisor-virtualized paths, + which a builtin handler intercepts before any user handler runs. + """ + if not os.path.exists(_SYSTEM_PYTHON): + pytest.skip(f"{_SYSTEM_PYTHON} not available") + + SYS_openat = 257 # x86_64 + import errno as _errno + + probe = tmp_dir / "probe.txt" + probe.write_text("payload\n") + + class _DenyProbe(Handler): + on_exception = ExceptionPolicy.KILL # handler is correct; policy unused + + def handle(self, ctx): + # openat(dirfd, pathname, flags, ...) -> pathname is args[1]. + path = ctx.read_cstr(ctx.args[1], max_len=4096) + if path == str(probe): + return NotifAction.errno(_errno.EPERM) + return NotifAction.continue_() + + sb = sandlock.Sandbox(fs_readable=[*_PYTHON_READABLE, str(tmp_dir)]) + # Child opens the probe file and reports the errno it gets. + script = ( + "import os, sys\n" + "try:\n" + " os.open(%r, os.O_RDONLY)\n" + " sys.exit(0)\n" + "except OSError as e:\n" + " sys.stderr.write('errno=%%d' %% e.errno)\n" + " sys.exit(3)\n" % str(probe) + ) + result = sb.run_with_handlers( + cmd=[_SYSTEM_PYTHON, "-c", script], + handlers=[(SYS_openat, _DenyProbe())], + ) + # Child caught OSError(EPERM) -> exit 3, stderr 'errno=1'. + assert not result.success, result + assert b"errno=%d" % _errno.EPERM in result.stderr, result.stderr + + +def test_handler_return_value_action_overrides_getpid(): + """A handler returning NotifAction.return_value_(777) must make the + child's os.getpid() return the synthetic 777 — only reachable if the + trampoline translates the ReturnValue action into + sandlock_action_set_return_value.""" + if not os.path.exists(_SYSTEM_PYTHON): + pytest.skip(f"{_SYSTEM_PYTHON} not available") + + SYS_getpid = 39 # x86_64 + + class _FakePid(Handler): + on_exception = ExceptionPolicy.KILL + + def handle(self, ctx): + return NotifAction.return_value_(777) + + sb = sandlock.Sandbox(fs_readable=_PYTHON_READABLE) + result = sb.run_with_handlers( + cmd=[_SYSTEM_PYTHON, "-c", "import os; print(os.getpid())"], + handlers=[(SYS_getpid, _FakePid())], + ) + assert result.success, result + assert result.stdout.strip() == b"777", result.stdout + + +def test_handler_kill_action_terminates_child(): + """A handler returning NotifAction.kill(SIGKILL, 0) directly must + terminate the child. on_exception is CONTINUE here deliberately: if + the trampoline failed to translate the Kill action, the action would + be Unset, the exception policy CONTINUE would let the child survive, + and this test would fail — which makes it discriminating.""" + if not os.path.exists(_SYSTEM_PYTHON): + pytest.skip(f"{_SYSTEM_PYTHON} not available") + + SYS_openat = 257 + import signal + + class _KillOnOpen(Handler): + on_exception = ExceptionPolicy.CONTINUE # NOT used — handler returns cleanly + + def handle(self, ctx): + return NotifAction.kill(signal.SIGKILL, 0) # pgid 0 -> supervisor resolves + + sb = sandlock.Sandbox(fs_readable=_PYTHON_READABLE) + result = sb.run_with_handlers( + cmd=[_SYSTEM_PYTHON, "-c", + "import os; os.open('/etc/hostname', os.O_RDONLY)"], + handlers=[(SYS_openat, _KillOnOpen())], + ) + assert not result.success, ( + f"child must be killed by the handler's Kill action; got {result}" + ) + + +def test_handler_mem_read_cstr_reads_child_path(tmp_dir): + """A handler reads the openat path argument from child memory via + ctx.read_cstr and denies a specific file. This exercises the real + sandlock_mem_read_cstr ctypes round-trip (the other tests only cover + read_cstr with _mem_handle=None). + + The probe is a plain file under tmp_dir, not /etc/hostname: the + supervisor virtualizes /etc/hostname with a builtin openat handler + that intercepts the notification before any user handler runs, so a + user handler never observes that path. + """ + if not os.path.exists(_SYSTEM_PYTHON): + pytest.skip(f"{_SYSTEM_PYTHON} not available") + + SYS_openat = 257 + import errno as _errno + + probe = tmp_dir / "probe.txt" + probe.write_text("payload\n") + + seen_paths = [] + + class _PathReader(Handler): + on_exception = ExceptionPolicy.CONTINUE + + def handle(self, ctx): + # openat(dirfd, pathname, flags, ...) -> pathname is args[1]. + path = ctx.read_cstr(ctx.args[1], max_len=4096) + seen_paths.append(path) + if path == str(probe): + return NotifAction.errno(_errno.EACCES) + return NotifAction.continue_() + + sb = sandlock.Sandbox(fs_readable=[*_PYTHON_READABLE, str(tmp_dir)]) + script = ( + "import os, sys\n" + "try:\n" + " os.open(%r, os.O_RDONLY)\n" + " sys.exit(0)\n" + "except OSError as e:\n" + " sys.exit(7 if e.errno == %d else 8)\n" % (str(probe), _errno.EACCES) + ) + result = sb.run_with_handlers( + cmd=[_SYSTEM_PYTHON, "-c", script], + handlers=[(SYS_openat, _PathReader())], + ) + # Handler read the real probe path from child memory and denied it. + assert str(probe) in seen_paths, seen_paths + assert not result.success, result + + +def test_run_with_handlers_empty_handler_list(): + """An empty handler list should run cleanly (degenerate but valid).""" + if not os.path.exists(_SYSTEM_PYTHON): + pytest.skip(f"{_SYSTEM_PYTHON} not available") + + sb = sandlock.Sandbox(fs_readable=_PYTHON_READABLE) + result = sb.run_with_handlers( + cmd=[_SYSTEM_PYTHON, "-c", "pass"], + handlers=[], + ) + assert result.success, result + + +def test_run_with_handlers_rejects_non_handler(): + """A non-Handler object in the list must raise a clear error, not + crash deep in ctypes.""" + if not os.path.exists(_SYSTEM_PYTHON): + pytest.skip(f"{_SYSTEM_PYTHON} not available") + + sb = sandlock.Sandbox(fs_readable=_PYTHON_READABLE) + with pytest.raises((TypeError, AttributeError, ValueError)): + sb.run_with_handlers( + cmd=[_SYSTEM_PYTHON, "-c", "pass"], + handlers=[(257, "not a handler")], + ) From 65b103331a516f5592b17e99154a2e065e29c931 Mon Sep 17 00:00:00 2001 From: dzerik Date: Fri, 15 May 2026 15:33:38 +0300 Subject: [PATCH 10/12] test: strengthen six for-show tests to catch real breakage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A mutation-based audit of the handler smoke suite found six tests that passed even when the production code they claimed to verify was broken: - exception KILL/CONTINUE policy tests asserted only the run's pass/fail, not that the exception path was exercised or that the child was killed AT the intercepted syscall. Now the child prints before/after markers and the raising handler records that it ran. - the two enum/C-header "match" tests asserted hardcoded numbers against hardcoded numbers. Now they parse the discriminants out of sandlock.h so an ABI drift is caught. - the RFC audit smoke test counted interpreter-startup openat noise. Now it opens a unique probe file and counts only that path. - the HandlerCtx field-exposure test exercised _for_test, not the trampoline's notification unpacking. Now a real run inspects the fields the trampoline built. Each rewrite was destructively verified — the corresponding production mutation makes the test fail. --- python/tests/test_handler_smoke.py | 297 ++++++++++++++++++++++------- 1 file changed, 226 insertions(+), 71 deletions(-) diff --git a/python/tests/test_handler_smoke.py b/python/tests/test_handler_smoke.py index 4160562..b1a73f7 100644 --- a/python/tests/test_handler_smoke.py +++ b/python/tests/test_handler_smoke.py @@ -59,11 +59,28 @@ def test_notif_action_is_frozen(): def test_exception_policy_enum_values_match_c_header(): - # Must match include/sandlock.h SANDLOCK_EXCEPTION_* discriminants. - assert ExceptionPolicy.KILL == 0 - assert ExceptionPolicy.DENY_EPERM == 1 - assert ExceptionPolicy.CONTINUE == 2 - assert ExceptionPolicy.DENY_EIO == 3 + """ExceptionPolicy discriminants must match SANDLOCK_EXCEPTION_* in + the C header. Parse the real header so an ABI drift is caught.""" + import re + from pathlib import Path + + header = ( + Path(__file__).resolve().parents[2] + / "crates" / "sandlock-ffi" / "include" / "sandlock.h" + ) + text = header.read_text() + pairs = dict( + (m.group(1), int(m.group(2))) + for m in re.finditer(r"SANDLOCK_EXCEPTION_(\w+)\s*=\s*(\d+)", text) + ) + assert pairs, "no SANDLOCK_EXCEPTION_* discriminants found in sandlock.h" + for c_name, c_val in pairs.items(): + py_member = getattr(ExceptionPolicy, c_name) + assert int(py_member) == c_val, ( + f"ExceptionPolicy.{c_name}={int(py_member)} != C header {c_val}" + ) + for member in ExceptionPolicy: + assert member.name in pairs, f"ExceptionPolicy.{member.name} not in C header" def test_handler_subclass_has_default_kill_policy(): @@ -97,20 +114,40 @@ def test_base_handler_handle_raises_not_implemented(): def test_action_kind_enum_values_match_c_header(): - # Must match SANDLOCK_ACTION_* in crates/sandlock-ffi/include/sandlock.h. + """_ActionKind discriminants must match SANDLOCK_ACTION_* in the C + header. Parse the real header so an ABI drift is caught.""" + import re + from pathlib import Path + from sandlock.handler import _ActionKind - assert _ActionKind.UNSET == 0 - assert _ActionKind.CONTINUE == 1 - assert _ActionKind.ERRNO == 2 - assert _ActionKind.RETURN_VALUE == 3 - assert _ActionKind.INJECT_FD_SEND == 4 - assert _ActionKind.INJECT_FD_SEND_TRACKED == 5 - assert _ActionKind.HOLD == 6 - assert _ActionKind.KILL == 7 + header = ( + Path(__file__).resolve().parents[2] + / "crates" / "sandlock-ffi" / "include" / "sandlock.h" + ) + text = header.read_text() + # Extract `SANDLOCK_ACTION_ = ` pairs. + pairs = dict( + (m.group(1), int(m.group(2))) + for m in re.finditer(r"SANDLOCK_ACTION_(\w+)\s*=\s*(\d+)", text) + ) + assert pairs, "no SANDLOCK_ACTION_* discriminants found in sandlock.h" + # Map C name -> Python _ActionKind member. + for c_name, c_val in pairs.items(): + py_member = getattr(_ActionKind, c_name) + assert int(py_member) == c_val, ( + f"_ActionKind.{c_name}={int(py_member)} != C header {c_val}" + ) + # And every _ActionKind member must exist in the header. + for member in _ActionKind: + assert member.name in pairs, f"_ActionKind.{member.name} not in C header" -def test_handler_ctx_exposes_notif_fields(): +def test_handler_ctx_dataclass_stores_fields(): + """Pure unit test of HandlerCtx as a dataclass: _for_test stores the + kwargs 1:1. This covers storage only — the trampoline's notification + unpacking is covered by + test_handler_ctx_notif_fields_populated_from_real_notification.""" from sandlock.handler import HandlerCtx # Construct via the test helper; the production constructor is @@ -182,39 +219,57 @@ def test_handler_ctx_is_frozen(): _SYSTEM_PYTHON = "/usr/bin/python3" -class _OpenatCounter(Handler): - on_exception = ExceptionPolicy.CONTINUE # audit-only — never block - - def __init__(self): - self.count = 0 - - def handle(self, ctx): - self.count += 1 - return NotifAction.continue_() +def test_smoke_audit_openat(tmp_dir): + """RFC #43 phasing item 2: an audit handler counts the child's + SYS_openat calls. Counts only opens of a unique probe file so the + assertion is not satisfiable by interpreter-startup openat noise. - -def test_smoke_audit_openat(): - """RFC #43 phasing item 2: audit handler counting SYS_openat.""" + The probe is a plain file under tmp_dir, mirroring + test_handler_mem_read_cstr_reads_child_path: /etc/hostname is + virtualized by a builtin supervisor handler that intercepts the + notification before any user handler runs. + """ if not os.path.exists(_SYSTEM_PYTHON): pytest.skip(f"{_SYSTEM_PYTHON} not available") SYS_openat = 257 # x86_64 - sb = sandlock.Sandbox(fs_readable=_PYTHON_READABLE) - counter = _OpenatCounter() + probe = tmp_dir / "audit-probe-file" + probe.write_text("x") + probe_path = str(probe) + class _OpenatCounter(Handler): + on_exception = ExceptionPolicy.CONTINUE # audit-only — never block + + def __init__(self, target): + self.target = target + self.count = 0 + + def handle(self, ctx): + # openat(dirfd, pathname, flags, ...) -> pathname is args[1]. + path = ctx.read_cstr(ctx.args[1], max_len=4096) + if path == self.target: + self.count += 1 + return NotifAction.continue_() + + sb = sandlock.Sandbox(fs_readable=[*_PYTHON_READABLE, str(tmp_dir)]) + counter = _OpenatCounter(probe_path) + script = ( + "import os\n" + "for _ in range(3):\n" + " fd = os.open(%r, os.O_RDONLY)\n" + " os.close(fd)\n" % probe_path + ) result = sb.run_with_handlers( - cmd=[_SYSTEM_PYTHON, "-c", - "import os\n" - "for _ in range(3):\n" - " fd = os.open('/etc/hostname', os.O_RDONLY)\n" - " os.close(fd)\n"], + cmd=[_SYSTEM_PYTHON, "-c", script], handlers=[(SYS_openat, counter)], ) assert result.success, result - assert counter.count >= 3, ( - f"expected >=3 openat intercepts, got {counter.count}" + # Counts only opens of the unique probe path — interpreter-startup + # openat noise targets other paths and is excluded. + assert counter.count == 3, ( + f"expected exactly 3 opens of the probe file, got {counter.count}" ) @@ -276,58 +331,101 @@ def handle(self, ctx): assert escaped.write(0x1000, b"x") is False -class _RaisingHandler(Handler): - """Handler that always raises — exercises the trampoline's - exception path. on_exception is set per-test.""" +def test_handler_exception_continue_policy_lets_child_run(tmp_dir): + """A handler that RAISES, with on_exception=CONTINUE, lets the child + complete — and we verify the exception path was actually exercised, + not just that the child happened to succeed. - def __init__(self, on_exception): - self.on_exception = on_exception - - def handle(self, ctx): - raise RuntimeError("intentional handler failure") - - -def test_handler_exception_continue_policy_lets_child_run(): - """A handler that raises, with on_exception=CONTINUE, must let the - child run to completion — the trampoline catches the exception and - the supervisor applies the Continue policy.""" + The handler targets a probe file under tmp_dir, not /etc/hostname: + that path is virtualized by a builtin supervisor handler that + intercepts the notification before any user handler runs. + """ if not os.path.exists(_SYSTEM_PYTHON): pytest.skip(f"{_SYSTEM_PYTHON} not available") SYS_openat = 257 # x86_64 - sb = sandlock.Sandbox(fs_readable=_PYTHON_READABLE) - handler = _RaisingHandler(ExceptionPolicy.CONTINUE) + + probe = tmp_dir / "probe.txt" + probe.write_text("payload\n") + probe_path = str(probe) + + class _RaisingHandler(Handler): + on_exception = ExceptionPolicy.CONTINUE + + def __init__(self, target): + self.target = target + self.raised = 0 + + def handle(self, ctx): + # Only act on the probe path; the loader's own openat calls + # hit other paths and must not be raised on. + path = ctx.read_cstr(ctx.args[1], max_len=4096) + if path == self.target: + self.raised += 1 + raise RuntimeError("intentional handler failure") + return NotifAction.continue_() + + sb = sandlock.Sandbox(fs_readable=[*_PYTHON_READABLE, str(tmp_dir)]) + handler = _RaisingHandler(probe_path) result = sb.run_with_handlers( cmd=[_SYSTEM_PYTHON, "-c", - "import os\nfd = os.open('/etc/hostname', os.O_RDONLY)\nos.close(fd)\n"], + "import os; fd = os.open(%r, os.O_RDONLY); os.close(fd)" % probe_path], handlers=[(SYS_openat, handler)], ) - # Continue policy: the openat proceeds despite the handler raising, - # so the child completes normally. + # The handler must have been called and must have raised — proving + # the trampoline's except-path + CONTINUE policy were exercised. + assert handler.raised >= 1, "handler.handle was never invoked / never raised" + # And with CONTINUE the raised exception did not block the child. assert result.success, result -def test_handler_exception_kill_policy_terminates_child(): - """A handler that raises, with on_exception=KILL, must terminate the - child — the trampoline catches the exception and the supervisor - applies the Kill policy.""" +def test_handler_exception_kill_policy_terminates_child(tmp_dir): + """A raising handler with on_exception=KILL terminates the child AT + the intercepted syscall — not merely 'the run failed somehow'. + + The handler targets a probe file under tmp_dir, not /etc/hostname: + that path is virtualized by a builtin supervisor handler that + intercepts the notification before any user handler runs. + """ if not os.path.exists(_SYSTEM_PYTHON): pytest.skip(f"{_SYSTEM_PYTHON} not available") - SYS_openat = 257 - sb = sandlock.Sandbox(fs_readable=_PYTHON_READABLE) - handler = _RaisingHandler(ExceptionPolicy.KILL) - result = sb.run_with_handlers( - cmd=[_SYSTEM_PYTHON, "-c", - "import os\nfd = os.open('/etc/hostname', os.O_RDONLY)\nos.close(fd)\n"], - handlers=[(SYS_openat, handler)], + SYS_openat = 257 # x86_64 + + probe = tmp_dir / "probe.txt" + probe.write_text("payload\n") + probe_path = str(probe) + + class _RaisingHandler(Handler): + on_exception = ExceptionPolicy.KILL + + def __init__(self, target): + self.target = target + + def handle(self, ctx): + path = ctx.read_cstr(ctx.args[1], max_len=4096) + if path == self.target: + raise RuntimeError("intentional handler failure") + return NotifAction.continue_() + + sb = sandlock.Sandbox(fs_readable=[*_PYTHON_READABLE, str(tmp_dir)]) + # BEFORE proves the child reached the open; AFTER proves it did NOT + # proceed past it. A run that crashes before the child starts shows + # neither marker -> the test fails, as it should. + script = ( + "import os, sys\n" + "sys.stdout.write('BEFORE\\n'); sys.stdout.flush()\n" + "os.open(%r, os.O_RDONLY)\n" + "sys.stdout.write('AFTER\\n'); sys.stdout.flush()\n" % probe_path ) - # Kill policy: the first intercepted openat that the raising handler - # touches kills the child's process group, so the run does not - # succeed. - assert not result.success, ( - f"expected child terminated by Kill policy, got success: {result}" + result = sb.run_with_handlers( + cmd=[_SYSTEM_PYTHON, "-c", script], + handlers=[(SYS_openat, _RaisingHandler(probe_path))], ) + stdout = result.stdout + assert b"BEFORE" in stdout, f"child never reached the syscall: {stdout!r}" + assert b"AFTER" not in stdout, f"child proceeded past the kill point: {stdout!r}" + assert not result.success, result # ---------------------------------------------------------------- @@ -499,6 +597,63 @@ def handle(self, ctx): assert not result.success, result +def test_handler_ctx_notif_fields_populated_from_real_notification(tmp_dir): + """HandlerCtx fields must be unpacked correctly from the C + notification by the trampoline — not just stored by the dataclass. + Exercise the real run_with_handlers -> trampoline path. + + The child opens a probe file under tmp_dir, not /etc/hostname: + that path is virtualized by a builtin supervisor handler that + intercepts the notification before any user handler runs. + """ + if not os.path.exists(_SYSTEM_PYTHON): + pytest.skip(f"{_SYSTEM_PYTHON} not available") + + SYS_openat = 257 # x86_64 + + probe = tmp_dir / "probe.txt" + probe.write_text("payload\n") + probe_path = str(probe) + + seen = {} + + class _FieldInspector(Handler): + on_exception = ExceptionPolicy.CONTINUE + + def __init__(self, target): + self.target = target + + def handle(self, ctx): + # Record the notification the trampoline built — but only for + # the probe open, so the loader's own openat calls (other + # paths) do not overwrite the recorded fields. + path = ctx.read_cstr(ctx.args[1], max_len=4096) + if path == self.target: + seen["syscall_nr"] = ctx.syscall_nr + seen["pid"] = ctx.pid + seen["args_len"] = len(ctx.args) + seen["arg1_is_ptr"] = ctx.args[1] # openat pathname pointer + return NotifAction.continue_() + + sb = sandlock.Sandbox(fs_readable=[*_PYTHON_READABLE, str(tmp_dir)]) + result = sb.run_with_handlers( + cmd=[_SYSTEM_PYTHON, "-c", + "import os; fd = os.open(%r, os.O_RDONLY); os.close(fd)" % probe_path], + handlers=[(SYS_openat, _FieldInspector(probe_path))], + ) + assert result.success, result + # The handler ran for the probe's SYS_openat: syscall_nr must equal + # the registered number — a field-swap in the trampoline (e.g. + # syscall_nr <- arch) would make this fail. + assert seen.get("syscall_nr") == SYS_openat, seen + # pid must be a real, positive child pid. + assert isinstance(seen.get("pid"), int) and seen["pid"] > 0, seen + # openat takes 6 syscall args; args[1] (pathname pointer) must be a + # non-zero userspace address. + assert seen.get("args_len") == 6, seen + assert isinstance(seen.get("arg1_is_ptr"), int) and seen["arg1_is_ptr"] > 0, seen + + def test_run_with_handlers_empty_handler_list(): """An empty handler list should run cleanly (degenerate but valid).""" if not os.path.exists(_SYSTEM_PYTHON): From e239a0217caf5c49623dc878cf9f66828a00a271 Mon Sep 17 00:00:00 2001 From: dzerik Date: Fri, 15 May 2026 16:17:45 +0300 Subject: [PATCH 11/12] fix: sweep handler registry on panic; harden run_with_handlers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit run_with_handlers inserts each Handler into a process-global registry and relies on the C ABI's ud_drop to remove the entry on completion. sandlock_run_with_handlers is extern "C-unwind", so a panic (e.g. invoked from within an existing Tokio runtime) propagates as a Python exception before the supervisor fires ud_drop — orphaning every entry. Wrap the call in try/finally and sweep the registered ids unconditionally; _unregister_handler is idempotent, so the sweep is a no-op once ud_drop has already run. Also from a self-review pass: - mem_read now fails (returns None) on a null handle regardless of length, mirroring mem_write — a dead context yields no child-memory access. A zero-length read on a live handle stays the trivial b"". - Drop the test-only HandlerCtx._for_test classmethod; _mem_handle already defaults to None, so tests construct HandlerCtx directly. - Add two registry-hygiene tests: a completed run leaves the registry empty, and a mid-loop registration failure rolls back with no orphaned entry. The rollback test is destructively verified. --- python/src/sandlock/_handler_ffi.py | 10 +++- python/src/sandlock/handler.py | 22 +------ python/src/sandlock/sandbox.py | 34 +++++++---- python/tests/test_handler_smoke.py | 93 +++++++++++++++++++++++++---- 4 files changed, 117 insertions(+), 42 deletions(-) diff --git a/python/src/sandlock/_handler_ffi.py b/python/src/sandlock/_handler_ffi.py index 9018e33..90d2f4d 100644 --- a/python/src/sandlock/_handler_ffi.py +++ b/python/src/sandlock/_handler_ffi.py @@ -205,9 +205,15 @@ def mem_read_cstr(mem_handle, addr: int, max_len: int) -> str | None: def mem_read(mem_handle, addr: int, length: int) -> bytes | None: """Read ``length`` raw bytes from the child at ``addr``. - Returns the bytes copied on success, or None on failure. + Returns the bytes copied on success, or None on failure. A null + handle always fails (returns None), mirroring ``mem_write`` — a + dead/absent context yields no child-memory access regardless of + the requested length. A zero-length read on a live handle is the + trivial success ``b""``. """ - if mem_handle is None or length < 1: + if mem_handle is None: + return None + if length < 1: return b"" if length == 0 else None buf = (ctypes.c_uint8 * length)() out_len = ctypes.c_size_t(0) diff --git a/python/src/sandlock/handler.py b/python/src/sandlock/handler.py index b2a30a6..3f76281 100644 --- a/python/src/sandlock/handler.py +++ b/python/src/sandlock/handler.py @@ -209,28 +209,10 @@ class HandlerCtx: args: tuple[int, int, int, int, int, int] # the six syscall args # Set by the trampoline to a ``_MemHandle`` liveness cell; opaque to - # user code. None in test contexts. + # user code. Defaults to None — a HandlerCtx built without one (the + # trampoline always supplies it) has inert child-memory accessors. _mem_handle: object = None - @classmethod - def _for_test( - cls, - *, - id: int, - pid: int, - flags: int, - syscall_nr: int, - arch: int, - instruction_pointer: int, - args: tuple[int, int, int, int, int, int], - ) -> HandlerCtx: - """Test-only constructor with no mem handle.""" - return cls( - id=id, pid=pid, flags=flags, syscall_nr=syscall_nr, - arch=arch, instruction_pointer=instruction_pointer, - args=args, _mem_handle=None, - ) - def read_cstr(self, addr: int, max_len: int) -> str | None: """Read a NUL-terminated string from the child at ``addr``. diff --git a/python/src/sandlock/sandbox.py b/python/src/sandlock/sandbox.py index c4878e6..15c295a 100644 --- a/python/src/sandlock/sandbox.py +++ b/python/src/sandlock/sandbox.py @@ -638,16 +638,30 @@ def run_with_handlers( raise name_b = _encode(resolved_name) - result_p = _lib.sandlock_run_with_handlers( - native.ptr, - name_b, - argv, - argc, - regs, - len(handlers), - ) - # Ownership of every container has transferred to the supervisor; - # it has also invoked each ud_drop, clearing the registry entries. + try: + result_p = _lib.sandlock_run_with_handlers( + native.ptr, + name_b, + argv, + argc, + regs, + len(handlers), + ) + finally: + # The registry exists only to route dispatch DURING the run; + # once sandlock_run_with_handlers returns, no handler can be + # invoked again. On the normal and early-return paths the + # supervisor has already fired every ud_drop, emptying these + # slots. On a panic — the entry point is extern "C-unwind", + # so a panic (e.g. called from within an existing Tokio + # runtime) propagates here as a Python exception — it may + # not have. Sweep unconditionally so a panic cannot orphan + # entries in the process-global registry; + # _unregister_handler is idempotent (pop(.., None)), so this + # is a no-op on the paths where ud_drop already ran. + for hid in registered_ids: + _handler_ffi._unregister_handler(hid) + # Ownership of every container has transferred to the supervisor. if not result_p: return Result( diff --git a/python/tests/test_handler_smoke.py b/python/tests/test_handler_smoke.py index b1a73f7..390245f 100644 --- a/python/tests/test_handler_smoke.py +++ b/python/tests/test_handler_smoke.py @@ -144,15 +144,15 @@ def test_action_kind_enum_values_match_c_header(): def test_handler_ctx_dataclass_stores_fields(): - """Pure unit test of HandlerCtx as a dataclass: _for_test stores the - kwargs 1:1. This covers storage only — the trampoline's notification - unpacking is covered by + """Pure unit test of HandlerCtx as a dataclass: the constructor + stores the kwargs 1:1. This covers storage only — the trampoline's + notification unpacking is covered by test_handler_ctx_notif_fields_populated_from_real_notification.""" from sandlock.handler import HandlerCtx - # Construct via the test helper; the production constructor is - # called only from the trampoline. - ctx = HandlerCtx._for_test( + # ``_mem_handle`` defaults to None, so the dataclass constructor is + # usable directly here; the trampoline is its only other caller. + ctx = HandlerCtx( id=42, pid=1234, flags=0, syscall_nr=39, arch=0xC000003E, instruction_pointer=0xDEADBEEF, @@ -170,9 +170,9 @@ def test_handler_ctx_dataclass_stores_fields(): def test_handler_ctx_mem_methods_return_falsy_without_handle(): from sandlock.handler import HandlerCtx - # _for_test ctx has no mem handle — accessors must degrade safely, - # not crash. - ctx = HandlerCtx._for_test( + # A ctx built without a mem handle (_mem_handle defaults to None) — + # accessors must degrade safely, not crash. + ctx = HandlerCtx( id=1, pid=1, flags=0, syscall_nr=0, arch=0, instruction_pointer=0, args=(0, 0, 0, 0, 0, 0), ) @@ -186,7 +186,7 @@ def test_handler_ctx_is_frozen(): from sandlock.handler import HandlerCtx - ctx = HandlerCtx._for_test( + ctx = HandlerCtx( id=1, pid=1, flags=0, syscall_nr=0, arch=0, instruction_pointer=0, args=(0, 0, 0, 0, 0, 0), ) @@ -679,3 +679,76 @@ def test_run_with_handlers_rejects_non_handler(): cmd=[_SYSTEM_PYTHON, "-c", "pass"], handlers=[(257, "not a handler")], ) + + +# ---------------------------------------------------------------- +# Handler-registry hygiene. run_with_handlers inserts each Handler +# into a process-global registry keyed by integer id; the C ABI's +# ud_drop removes the entry when the supervisor frees the container. +# These tests pin the "no orphaned entry" invariant the PR body +# claims — on both the mid-loop rollback path and a completed run. +# ---------------------------------------------------------------- + + +def test_run_with_handlers_rollback_leaves_registry_clean(): + """A mid-loop registration failure must roll back cleanly: every + handler container freed and the process-global handler registry + left with no orphaned entries. + + The second entry carries a syscall_nr that ``int()`` rejects, so + handler 0 is fully registered while handler 1 fails AFTER + _register_handler + sandlock_handler_new but before the regs store + — the exact mid-loop window the rollback code in run_with_handlers + handles. The failure is raised before sandlock_run_with_handlers is + reached, so no kernel/child is involved. + + Destructive check: deleting the rollback ``except`` block in + run_with_handlers leaves handler 0's id orphaned and the assert + fails with a count of before+1. + """ + from sandlock import _handler_ffi + + class _Noop(Handler): + def handle(self, ctx): + return NotifAction.continue_() + + sb = sandlock.Sandbox(fs_readable=_PYTHON_READABLE) + before = len(_handler_ffi._HANDLERS) + with pytest.raises(ValueError): + sb.run_with_handlers( + cmd=["/bin/true"], + handlers=[(257, _Noop()), ("not-a-syscall-number", _Noop())], + ) + assert len(_handler_ffi._HANDLERS) == before, ( + "mid-loop rollback orphaned handler registry entries: " + f"{len(_handler_ffi._HANDLERS)} != {before}" + ) + + +def test_run_with_handlers_clears_registry_after_run(): + """After a completed run, the process-global handler registry must + be empty again — no entry survives the run that registered it. + Guards against unbounded registry growth across repeated + run_with_handlers calls.""" + if not os.path.exists(_SYSTEM_PYTHON): + pytest.skip(f"{_SYSTEM_PYTHON} not available") + + from sandlock import _handler_ffi + + class _Noop(Handler): + on_exception = ExceptionPolicy.CONTINUE + + def handle(self, ctx): + return NotifAction.continue_() + + sb = sandlock.Sandbox(fs_readable=_PYTHON_READABLE) + before = len(_handler_ffi._HANDLERS) + result = sb.run_with_handlers( + cmd=[_SYSTEM_PYTHON, "-c", "pass"], + handlers=[(257, _Noop())], + ) + assert result.success, result + assert len(_handler_ffi._HANDLERS) == before, ( + "a registry entry survived a completed run: " + f"{len(_handler_ffi._HANDLERS)} != {before}" + ) From 45bc910c0d6ec42ad17ae2d404dac43266f79db7 Mon Sep 17 00:00:00 2001 From: dzerik Date: Fri, 15 May 2026 16:38:34 +0300 Subject: [PATCH 12/12] test: resolve syscall numbers per host arch The handler smoke tests hardcoded x86_64 syscall numbers (openat=257, getpid=39). On aarch64 those are wrong: the handler registers on an unrelated syscall and never fires, and 39 lands in the aarch64 deny list, which fails the run outright. All nine kernel-dependent tests failed on the ubuntu-24.04-arm CI runner while passing on x86_64. The Rust side resolves these via libc::SYS_*; Python has no such table, so add a small arch->number map (x86_64, aarch64) and a _syscall_nr helper that skips on an unmapped arch rather than silently registering on the wrong syscall. --- python/tests/test_handler_smoke.py | 47 +++++++++++++++++++++++------- 1 file changed, 37 insertions(+), 10 deletions(-) diff --git a/python/tests/test_handler_smoke.py b/python/tests/test_handler_smoke.py index 390245f..fe47d0d 100644 --- a/python/tests/test_handler_smoke.py +++ b/python/tests/test_handler_smoke.py @@ -203,6 +203,7 @@ def test_handler_ctx_is_frozen(): # ---------------------------------------------------------------- import os +import platform import pytest @@ -219,6 +220,27 @@ def test_handler_ctx_is_frozen(): _SYSTEM_PYTHON = "/usr/bin/python3" +# Syscall numbers are architecture-specific. run_with_handlers registers +# on raw kernel syscall numbers (the C ABI takes a plain int), so the +# number must match the host arch — x86_64 and aarch64 differ. The Rust +# side gets this for free via libc::SYS_*; Python has no such table, so +# the test resolves it explicitly. An unmapped arch skips rather than +# silently registering on the wrong syscall. +_SYSCALL_NRS = { + "x86_64": {"openat": 257, "getpid": 39}, + "aarch64": {"openat": 56, "getpid": 172}, +} + + +def _syscall_nr(name: str) -> int: + """Return the kernel syscall number for ``name`` on the host arch.""" + machine = platform.machine() + arch = _SYSCALL_NRS.get(machine) + if arch is None or name not in arch: + pytest.skip(f"syscall {name!r} not mapped for arch {machine!r}") + return arch[name] + + def test_smoke_audit_openat(tmp_dir): """RFC #43 phasing item 2: an audit handler counts the child's SYS_openat calls. Counts only opens of a unique probe file so the @@ -232,7 +254,7 @@ def test_smoke_audit_openat(tmp_dir): if not os.path.exists(_SYSTEM_PYTHON): pytest.skip(f"{_SYSTEM_PYTHON} not available") - SYS_openat = 257 # x86_64 + SYS_openat = _syscall_nr("openat") probe = tmp_dir / "audit-probe-file" probe.write_text("x") @@ -287,7 +309,7 @@ def test_handler_ctx_mem_handle_invalidated_after_handle(): if not os.path.exists(_SYSTEM_PYTHON): pytest.skip(f"{_SYSTEM_PYTHON} not available") - SYS_openat = 257 # x86_64 + SYS_openat = _syscall_nr("openat") captured = {} @@ -343,7 +365,7 @@ def test_handler_exception_continue_policy_lets_child_run(tmp_dir): if not os.path.exists(_SYSTEM_PYTHON): pytest.skip(f"{_SYSTEM_PYTHON} not available") - SYS_openat = 257 # x86_64 + SYS_openat = _syscall_nr("openat") probe = tmp_dir / "probe.txt" probe.write_text("payload\n") @@ -390,7 +412,7 @@ def test_handler_exception_kill_policy_terminates_child(tmp_dir): if not os.path.exists(_SYSTEM_PYTHON): pytest.skip(f"{_SYSTEM_PYTHON} not available") - SYS_openat = 257 # x86_64 + SYS_openat = _syscall_nr("openat") probe = tmp_dir / "probe.txt" probe.write_text("payload\n") @@ -456,7 +478,7 @@ def test_handler_errno_action_makes_child_observe_eperm(tmp_dir): if not os.path.exists(_SYSTEM_PYTHON): pytest.skip(f"{_SYSTEM_PYTHON} not available") - SYS_openat = 257 # x86_64 + SYS_openat = _syscall_nr("openat") import errno as _errno probe = tmp_dir / "probe.txt" @@ -500,7 +522,7 @@ def test_handler_return_value_action_overrides_getpid(): if not os.path.exists(_SYSTEM_PYTHON): pytest.skip(f"{_SYSTEM_PYTHON} not available") - SYS_getpid = 39 # x86_64 + SYS_getpid = _syscall_nr("getpid") class _FakePid(Handler): on_exception = ExceptionPolicy.KILL @@ -526,7 +548,7 @@ def test_handler_kill_action_terminates_child(): if not os.path.exists(_SYSTEM_PYTHON): pytest.skip(f"{_SYSTEM_PYTHON} not available") - SYS_openat = 257 + SYS_openat = _syscall_nr("openat") import signal class _KillOnOpen(Handler): @@ -560,7 +582,7 @@ def test_handler_mem_read_cstr_reads_child_path(tmp_dir): if not os.path.exists(_SYSTEM_PYTHON): pytest.skip(f"{_SYSTEM_PYTHON} not available") - SYS_openat = 257 + SYS_openat = _syscall_nr("openat") import errno as _errno probe = tmp_dir / "probe.txt" @@ -609,7 +631,7 @@ def test_handler_ctx_notif_fields_populated_from_real_notification(tmp_dir): if not os.path.exists(_SYSTEM_PYTHON): pytest.skip(f"{_SYSTEM_PYTHON} not available") - SYS_openat = 257 # x86_64 + SYS_openat = _syscall_nr("openat") probe = tmp_dir / "probe.txt" probe.write_text("payload\n") @@ -717,6 +739,9 @@ def handle(self, ctx): with pytest.raises(ValueError): sb.run_with_handlers( cmd=["/bin/true"], + # The first syscall number is arbitrary — registration fails + # on the second entry, before the kernel is ever reached, so + # no arch-correct number is needed here. handlers=[(257, _Noop()), ("not-a-syscall-number", _Noop())], ) assert len(_handler_ffi._HANDLERS) == before, ( @@ -745,7 +770,9 @@ def handle(self, ctx): before = len(_handler_ffi._HANDLERS) result = sb.run_with_handlers( cmd=[_SYSTEM_PYTHON, "-c", "pass"], - handlers=[(257, _Noop())], + # A real run, so the syscall number must be valid for the host + # arch — openat is always notif-eligible and never deny-listed. + handlers=[(_syscall_nr("openat"), _Noop())], ) assert result.success, result assert len(_handler_ffi._HANDLERS) == before, (