feat(sandbox): pluggable handler for bootstrap-subsystem failures#1549
Open
dims wants to merge 1 commit into
Open
feat(sandbox): pluggable handler for bootstrap-subsystem failures#1549dims wants to merge 1 commit into
dims wants to merge 1 commit into
Conversation
dims
requested review from
a team,
derekwaynecarr,
maxamillion and
mrunalp
as code owners
8 tasks
dims
added a commit
to dims/openshell-driver-substrate
that referenced
this pull request
May 23, 2026
Two upstream PRs are now open against NVIDIA/OpenShell with the same end result but different shapes — #1548 (env-var gate) and #1549 (SandboxFailureHandler trait + setter). Update the README "Companion changes upstream" table and the poc-intro §1 header, §3 components table, §6.1 detail, and §9 "Where to next" item to present both alternatives side by side. Signed-off-by: Davanum Srinivas <dsrinivas@nvidia.com>
Contributor
Author
The supervisor performs three optional hardening steps that the host kernel may refuse: unshare(CLONE_NEWNET), the supervisor seccomp prelude, and the workload seccomp filter. On bare-metal Linux a refusal is fatal. Under an outer sandbox (gVisor, Firecracker, Kata) the host runtime is itself the enforcing boundary and routinely intercepts those syscalls, leaving the supervisor unable to boot. Introduce a pluggable SandboxFailureHandler trait. The default StrictHandler preserves the historical contract — every refusal aborts. Outer-sandbox integrations link this crate and call set_failure_handler once at process start to register a handler that returns Ok(()) for the kinds the host runtime is expected to manage. The three internal call sites in run_sandbox and sandbox::linux::enforce route through failure_handler().handle(kind, err)?. Also make drop_privileges idempotent: when euid/egid already match the resolved target, skip initgroups(3), which would otherwise fail without CAP_SETGID. The stock openshell-sandbox binary never calls set_failure_handler, so its behaviour against this commit is byte-identical to upstream main. All 777 sandbox library tests pass unchanged. Signed-off-by: Davanum Srinivas <dsrinivas@nvidia.com>
dims
force-pushed
the
feat/sandbox-failure-handler
branch
from
fafa9a4 to
e295996
Compare
|
Label |
Collaborator
|
/ok to test e295996 |
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
2 participants
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.
The supervisor performs three optional hardening steps that the host kernel may refuse:
unshare(CLONE_NEWNET), the supervisor seccomp prelude, and the workload seccomp filter. On bare-metal Linux a refusal is fatal — the supervisor cannot meet its hardening contract and aborts. Under an outer sandbox (gVisor, Firecracker, Kata) the host runtime is itself the enforcing boundary and routinely intercepts those syscalls, leaving the supervisor unable to boot.This change introduces a pluggable
SandboxFailureHandlertrait. The defaultStrictHandlerpreserves the historical contract — every refusal aborts. Outer-sandbox integrations link this crate and callset_failure_handleronce at process start to register a handler that returnsOk(())for the kinds the host runtime is expected to manage. The three internal call sites inrun_sandboxandsandbox::linux::enforceroute throughfailure_handler().handle(kind, err)?.The change also makes
drop_privilegesidempotent: wheneuid/egidalready match the resolved target, skipinitgroups(3), which would otherwise fail withoutCAP_SETGID.The stock
openshell-sandboxbinary never callsset_failure_handler, so its behaviour against this commit is byte-identical to upstreammain. All 777 sandbox library tests pass unchanged.Files touched (3 files, +71 / −7):
crates/openshell-sandbox/src/lib.rs— theSandboxFailureKindenum, theSandboxFailureHandlertrait,StrictHandler, the set-once handler slot,set_failure_handler, and the netns-create + supervisor-seccomp call sites.crates/openshell-sandbox/src/sandbox/linux/mod.rs— workload-seccomp call site.crates/openshell-sandbox/src/process.rs— idempotentdrop_privilegesfast-path.Verification:
cargo fmt --all -- --checkclean on the touched files.cargo test -p openshell-sandbox --lib— 777 tests pass.cargo clippy -p openshell-sandbox --lib --tests -- -D warnings— zero new warnings introduced (pre-existing warnings onmainunchanged).mise run pre-commit— to be re-verified by CI after copy-pr-bot mirrors.Motivation: the substrate-based POC at
dims/openshell-driver-substrateneeds the supervisor to boot inside a gVisor actor; the host runtime refuses the three syscalls above. Without a trait-shaped extension point the integration would have to fork the supervisor. This is the smaller, upstream-friendly alternative to the earlier env-variable-gated #1548 — same end result, but expressed as a typed Rust API and capable of selective per-kind policy.Checklist:
feat(sandbox): …