Repository navigation
refactor(supervisor): isolate state per sandbox instance - #4701
Merged
Merged
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
drew
added this pull request to stack #4698
October 19, 2026 02:17
drew
force-pushed
the
feat/4699-supervisor-session-state/drew
branch
from
October 20, 2026 16:24
0e074fb to
e2b27bd
Compare
drew
force-pushed
the
feat/4699-supervisor-session-state/drew
branch
3 times, most recently
from
October 20, 2026 22:10
bec05a1 to
d823dac
Compare
drew
force-pushed
the
feat/4699-supervisor-session-state/drew
branch
6 times, most recently
from
October 21, 2026 01:05
4a7e9d5 to
00d9769
Compare
Base automatically changed from
feat/4694-experimental-supervisor-topology/drew
to
main
October 21, 2026 01:48
drew
force-pushed
the
feat/4699-supervisor-session-state/drew
branch
from
October 21, 2026 01:48
00d9769 to
9ca201c
Compare
drew
changed the title
3 tasks done
drew
marked this pull request as ready for review
October 22, 2026 00:14
A supervisor process that hosts several sandboxes cannot use one process-wide OCSF context: every event would carry the wrong sandbox identity, and middleware requests read their sandbox identity from the same context. Builders now accept a ContextRef, either a borrowed context or a context shared by a hosted session, so per-session contexts need not be leaked. ctx() returns the context bound to the calling thread when present and falls back to the process-wide context otherwise. OCSF JSON output settings can likewise be bound per thread, and OpenTelemetry can parent a span from an explicit W3C trace context instead of process environment. A dedicated supervisor keeps using the process-wide context. Signed-off-by: Drew Newberry <anewberry@nvidia.com>
…a session The supervisor kept its gateway bearer, Sandbox Protocol bearer, and renewal task in process-wide slots, and the network supervisor read provider token grants, proxy CA files, and the gateway identity from process environment. A second sandbox in the same process would have replaced or rotated the first sandbox's credentials, reused its provider access tokens, and exposed its denials through policy.local. GatewayCredentials now holds one session's gateway and Sandbox Protocol credentials and their renewal; every gateway client receives a GatewayEndpoint that pairs the URL with them, including log push, ConnectSupervisor and its redirects, and extension-credential rotation. run_networking takes a NetworkSession with the sandbox identity, the session's gateway access, its Workload API socket, its policy.local denial source, and its proxy CA files. Provider token grants use a cache and HTTP client owned by the session and fail closed without a Workload API socket. run_session supervises one generation from an explicit SessionConfig and owner-provided SessionControls. run_sandbox_with_backend remains the dedicated entry point: it keeps the process-wide OCSF context, reads the same environment, binds readiness endpoints to session readiness, and stops on SIGTERM or SIGINT. The binary shares one set of credentials between log push and supervision, as before. Signed-off-by: Drew Newberry <anewberry@nvidia.com>
SessionHost runs several supervisor sessions in one process. Each session gets its own Tokio runtime, and every thread that runtime starts, plus the thread driving it, is bound to the session's OCSF context, OCSF JSON settings, log sink, and denial log. Tasks, blocking work, reconnects, and credential renewal spawned by a session therefore keep its identity, while threads outside any session carry none and push no logs. Gateway credentials stay explicit per session. Removing a session stops its workload gracefully, or cancels it at once while it is still starting, then shuts down only its runtime and revokes its gateway credentials. The key stays registered until the session thread has finished, so the same generation cannot gain a second owner while one is stopping. A panicking session reports a failure, clears its readiness, and is revoked the same way. Hosted sessions get a private proxy trust directory and serve policy.local denials from their own log. A focused test runs two sessions concurrently and checks that nested and blocking tasks see their own identity, gateway requests carry their own bearer, log lines reach only their own stream, and removing one leaves the other supervising. Closes #4699 Signed-off-by: Drew Newberry <anewberry@nvidia.com>
A sandbox instance is one running generation of a stable Sandbox resource. Gateway transport connections and the gateway supervisor sessions they carry can reconnect within one instance, so the hosted object, its configuration, controls, host, and per-instance network, denial, and token-grant state now use the sandbox instance name. Rename session_host to sandbox_instance_host, run_session to run_sandbox_instance, and SessionConfig, SessionControls, SessionHost, and related hosted types to their SandboxInstance equivalents. Gateway supervisor sessions, SSH sessions, and authentication session types keep their names. No wire, configuration, or lifecycle behavior changes. Signed-off-by: Drew Newberry <anewberry@nvidia.com>
…epared bindings Replace SandboxRunConfig and its field-by-field copy into SandboxInstanceConfig with a single launch configuration. Live per-instance state (gateway credentials, OCSF context and JSON settings, log sink, and denial log) is now an explicit SandboxInstanceBindings value that the owner prepares once, instead of placeholders the runner or host overwrote. The dedicated binary loads launch inputs from its arguments and environment and shares one GatewayCredentials with log push; run_sandbox_with_backend keeps the process-wide OCSF context, health endpoints, and signal handling. The instance host binds the same SandboxInstanceBindings to its runtime threads and revokes their credentials when the instance ends. Signed-off-by: Drew Newberry <anewberry@nvidia.com>
TaylorMutch
requested changes
Oct 22, 2026
TaylorMutch
left a comment
TaylorMutch
left a comment
Collaborator
There was a problem hiding this comment.
Found one Windows test compilation regression. The remaining credential, policy/provider state, logging, and lifecycle changes have no other actionable findings from this static review. Tests and a Windows build were not run.
drew
force-pushed
the
feat/4699-supervisor-session-state/drew
branch
from
October 22, 2026 00:57
fc7caf2 to
e1be7e9
Compare
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
TaylorMutch
approved these changes
Oct 22, 2026
drew
added
test:e2e
|
Label |
|
Label |
Collaborator
Author
|
/ok to test 34d2ee0 |
drew
added a commit
that referenced
this pull request
Oct 22, 2026
… into provider token resolvers Signed-off-by: Drew Newberry <anewberry@nvidia.com>
2 of 7 tasks
krishicks
added a commit
that referenced
this pull request
Oct 23, 2026
The sandbox_instance_host tests added with per-instance supervisor state (#4701) admit hosted instances through SandboxInstanceHost::new, which prepares a private directory per instance. Hosted sandbox instances are Unix-only: on other hosts prepare_private_dir returns "hosted sandbox instances require a Unix host". Prior to this change, the test module still compiled and ran on Windows, so all three tests failed while creating their host. Now, the test module is gated to Unix, and the Unix-only permission check inside one test no longer needs its own cfg attribute. Signed-off-by: Kris Hicks <khicks@nvidia.com>
krishicks
added a commit
that referenced
this pull request
Oct 23, 2026
The sandbox_instance_host tests added with per-instance supervisor state (#4701) admit hosted instances through SandboxInstanceHost::new, which prepares a private directory per instance. Hosted sandbox instances are Unix-only: on other hosts prepare_private_dir returns "hosted sandbox instances require a Unix host". Prior to this change, the test module still compiled and ran on Windows, so all three tests failed while creating their host. Now, the test module is gated to Unix, and the Unix-only permission check inside one test no longer needs its own cfg attribute. Signed-off-by: Kris Hicks <khicks@nvidia.com>
purp
pushed a commit
to purp/OpenShell
that referenced
this pull request
Oct 23, 2026
* fix(isolation): validate Unix socket paths the same way on every host Prior to this change, SandboxTransport::validate checked Unix socket paths with Path::is_absolute(). On Windows that requires a drive or UNC prefix, so a POSIX path such as /run/backend.sock was rejected as "invalid boundary control transport endpoint". The isolation backend registration work in NVIDIA#3898 added tests that register Unix endpoints in the core registration types, gateway config, gateway config preflight, and supervisor backend setup; all of them failed on the Windows MSVC runners. The Windows test job only runs on PRs labeled test:windows, so the break went unnoticed on main. Unix transport is refused at runtime on non-Unix hosts, so these paths are always POSIX paths regardless of where they are validated. Now, both the core registration validator and the sandbox backend protocol validator use a new openshell_core::paths::is_posix_absolute helper, which applies the POSIX rule (the path starts with '/') on every host. On Unix this matches is_absolute() exactly. Signed-off-by: Kris Hicks <khicks@nvidia.com> * test(supervisor): run baseline write-back cases only on Unix hosts The startup policy write-refusal tests from NVIDIA#3785 rely on enrich_proto_baseline_paths adding the proxy baseline paths (/usr, /tmp, /dev/null, ...) to a policy, which triggers a write-back that the fake gateway then refuses. Enrichment only adds paths that exist on the host. Prior to this change, the tests assumed those paths exist, so on Windows enrichment added nothing, the precondition assertions failed, and the baseline case of permanent_codes_still_exit never saw a write to refuse. Now, the four baseline write-back tests are gated to Unix, and permanent_codes_still_exit checks the image policy upload case on every host and the baseline write-back case only on Unix. Signed-off-by: Kris Hicks <khicks@nvidia.com> * test(server): scale the SQLite concurrent-write test for Windows fsync sqlite_concurrent_writes_return_their_own_resource_version (NVIDIA#4408) runs 8 writers that each make 25 unconditional and 25 version-checked writes against one row of an on-disk WAL database with synchronous=FULL. Writers serialize on the SQLite write lock and wait out sqlx's default 5s busy_timeout. Prior to this change, the per-commit fsync cost on the Windows runners was high enough that a waiting writer starved past that timeout and failed with "database is locked" after about 7s. Now, each writer makes 5 writes on Windows and 25 elsewhere. The test still races every writer on the same row, which is what it checks. Signed-off-by: Kris Hicks <khicks@nvidia.com> * test(server): hold scope-mix locks for wall-clock time, not timer ticks random_scope_mix_never_deadlocks (NVIDIA#4147) runs 64 tasks of 50 random lock operations and holds each operation's locks for 0-2 ms, failing if the mix does not finish within 20s. Prior to this change, the hold was a tokio::time::sleep. Windows timers round a 1 ms sleep up to about 16 ms, and a quarter of the operations take the exclusive global lock, so the serialized holds alone took several seconds and the full mix exceeded the 20s deadline on the Windows runners. Now, occupy yields until a std::time::Instant deadline, so holds last 0-2 ms of wall-clock time on every platform and the deadline means the same thing everywhere. Signed-off-by: Kris Hicks <khicks@nvidia.com> * test(supervisor): run sandbox instance host tests only on Unix hosts The sandbox_instance_host tests added with per-instance supervisor state (NVIDIA#4701) admit hosted instances through SandboxInstanceHost::new, which prepares a private directory per instance. Hosted sandbox instances are Unix-only: on other hosts prepare_private_dir returns "hosted sandbox instances require a Unix host". Prior to this change, the test module still compiled and ran on Windows, so all three tests failed while creating their host. Now, the test module is gated to Unix, and the Unix-only permission check inside one test no longer needs its own cfg attribute. Signed-off-by: Kris Hicks <khicks@nvidia.com> --------- Signed-off-by: Kris Hicks <khicks@nvidia.com>
|
A question on the shared-supervisor mode. Some deployments use the sandbox ID on OCSF events as an audit key (as recommended in #4658). For them, will per-session attribution be stated as a guarantee in the docs? #4701 reads as fixing exactly the case where a second sandbox in the same process could put the wrong identity on events. The new kubernetes_shared_supervisor e2e test seems to focus on route and TLS identity. Is there, or will there be, a test asserting that events from two sandboxes on one supervisor always carry their own sandbox ID? |
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.
Summary
Make supervisor state belong to a sandbox instance (one running generation), so a single process can host several sandboxes without sharing gateway credentials, provider grants, policy decisions, or log identity. Dedicated supervision continues to run one instance; Kubernetes shared hosting is added in #4702.
Related Issue
Closes #4699
Changes
SandboxInstanceConfigfor dedicated and hosted launches, with explicitSandboxInstanceBindingsfor prepared credentials and logging state. Remove the duplicate launch config and long argument-copying wrapper; keep process health and signals in the dedicated wrapper.A sandbox is the stable resource; an instance is one running generation. Transport and authentication sessions may reconnect within that instance. Existing wire/storage names, environment variables, CLI flags, and defaults remain compatible.
Release Notes
Testing
1d880c9ac, preserving all 38 commits and the merge-only changes. Current full-stack Rust tests: 176 supervisor, 429 server authorization/token/expiry, and 2 runtime-routing tests passed; 2 server tests ignored.The focused shared-supervisor, SPIFFE, and dedicated lifecycle scenarios compile. A fresh-cluster live run was blocked before OpenShell deployment: containerd could not create an inotify watcher (
too many open files; hostfs.inotify.max_user_instances=128), so the node never registered. No live scenarios ran; the temporary cluster was removed. Windows MSVC compilation was not run locally.Checklist