Repository navigation
Conversation
…rence Replace raw argument-directory inference with a typed launch spec that validates every input/output/restore/logPipe path against the pinned child-root contract ($TMPDIR/ml-child-ipc/<child-id>): rejects relative, root, out-of-root, dot-dot, duplicate, and mutable-symlink/alias paths before any policy is built. Adds the minimized fixed-mount enumeration (narrows /etc to individually justified files, never binds host /proc//sys), a private bounded tmpfs at /tmp, and a purpose-built allowlisted mechanism probe (ml_sandbox_probe) proving allowed IPC access, denied host reads/writes, mount enumeration, and external-egress denial. Validator logic verified standalone on this host (non-Sandbox2 path): compiles clean with -Wall -Wextra, and a driver exercising every V16 rejection case (relative/root/dot-dot/duplicate/aliased/wrong-depth/ child-id-mismatch) plus the valid multi-pipe case all pass. The Sandbox2-gated PolicyBuilder path and the Linux mechanism-probe integration test are unverified in this session (no Linux/Sandbox2 toolchain on this host) and need a devbox or Buildkite pass before V4/V5(policy half)/V7/V16 can be marked closed - see elastic-workspace-3b59.4.
Contributor
Author
This was referenced Sep 9, 2026
valeriy42
added a commit
that referenced
this pull request
Sep 23, 2026
…#3187) ## Summary Rebuilds `CSandboxedProcessSpawner` around an explicit lifecycle state machine (`Prepared -> Launched -> IdentityCaptured -> Registered -> Monitoring -> TerminationRequested -> CleanupRequired -> Reaped/Failed`), stacked on #3184. - Non-throwing kill-and-reap guard, armed immediately after launch and PID capture, holding its own owning reference to the sandbox handle so cleanup is order-independent during unwinding. - Four injectable seams (pidfd acquisition, registry allocation, monitor-thread launch, sandbox completion) for deterministic fault injection. - Explicit pidfd outcome classification: a kernel lacking pidfd support (`ENOSYS`) is the *only* case that falls back to signalling the sandboxee through its owned monitor handle (SIGKILL, identity-safe, no numeric-PID lookup); every other pidfd error is treated as a resource/identity failure and fails registration outright. Numeric `kill(pid, ...)` does not appear anywhere in this file. - A CAS-controlled one-shot latch replacing two-boolean coordination for the timeout-vs-completion race. - `CSandboxedProcessSpawnerLifecycleTest_Linux.cc`: fault-injection coverage for every pidfd class, allocation/resource failures, stale-generation protection, PID-reuse identity binding, the CAS race, descriptor-baseline cleanup, and destructor-latency. ## Notes - `E_Launched`/`E_CleanupRequired` lifecycle states are declared but intentionally left unassigned (no natural single point without broader restructuring) — not silently dropped. - The SIGKILL-only assumption for the owned-monitor termination path (no graceful-SIGTERM variant is buildable against the pinned Sandbox2 dependency version) was cross-checked against the vendored monitor source during review.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacks on #3182 (PR B). PR C of the Sandbox2 clean rebuild plan (see the elastic-workspace harness, docs/projects/mlcpp-sandbox2-pr2873).
Replaces raw argument-directory inference in
CPytorchInferenceSandboxPolicywith a typed launch spec: everyinput/output/restore/logPipepath is validated against the pinned child-root contract ($TMPDIR/ml-child-ipc/<child-id>) before any policy is built. Rejects relative, root-level, dot-dot, out-of-root, wrong-depth, duplicate, mutable-symlink/alias, and cross-option child-id-mismatch paths - never widens a mount to recover a rejected argument.Also minimizes the filesystem policy: enumerates and justifies all seven historically bulk-mounted fixed directories, replaces whole
/etcwith five individually justified files, never binds host/proc//sys(relies on Sandbox2's own namespaced procfs/sysfs), uses a private bounded tmpfs at/tmpinstead of the host's, and consumes the syscall allowlist already shared with the legacy BPF filter instead of hand-duplicating it.Adds a purpose-built allowlisted mechanism probe (
ml_sandbox_probe) proving allowed IPC access, denied host reads, denied external egress, loopback reachability, and mount enumeration, plus a portable validator unit-test suite and a Linux-only mechanism integration test.Verified this session (macOS host, no Sandbox2/Linux toolchain available): the validator compiles clean with
-Wall -Wextraoutside the ml-cpp build system, and a standalone driver exercises every rejection/acceptance path against realmkdtemp/mkdir/symlinkfixtures - all pass. Not yet verified: theSANDBOX2_AVAILABLEbuild, the CMake wiring, and the mechanism-probe integration test itself - needs a Linux CI/devbox pass. Opening as draft for that reason.