feat(isolation): implement the RFC 0012 sandbox architecture - #2942
feat(isolation): implement the RFC 0012 sandbox architecture#2942drew wants to merge 48 commits into
Conversation
fccbbc0 to
5b589ab
Compare
5b589ab to
8818afd
Compare
|
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. |
fbcc382 to
efba1d8
Compare
00b12d3 to
b06e0b1
Compare
|
Label |
b06e0b1 to
8e95cd7
Compare
|
/ok to test |
3 similar comments
|
/ok to test |
|
/ok to test |
|
/ok to test |
e4cc065 to
62a943a
Compare
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
57e7b2d to
9b00cb6
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
johntmyers
left a comment
johntmyers
left a comment
There was a problem hiding this comment.
gator-agent
PR Review Status
The RFC 0012 core is project-valid and its stacked interface is coherent across the VM, Docker, Kubernetes, Podman, and performance layers, but four concrete obligations remain in the core patch: authentication continuity across gateway replacement, lifecycle rollback preserving active authentication, PID 1 orphan reaping, and synchronization of the shipped runtime split with repository/operator guidance.
Action required: @drew, please address the four blocking findings and push an updated head for a focused follow-up review.
Blocking findings:
GATOR-07441545-01: preserve refreshable launch authentication across a gateway restart or supported replica routing.GATOR-07441545-02: do not revoke an active launch when stop/delete rolls back to Ready.GATOR-07441545-03: restore managed-child-aware orphan reaping whenopenshell-sandboxis PID 1.GATOR-07441545-04: synchronize the new crates and split runtime artifacts with agent/operator workflows.
Carried findings:
- None
Non-blocking suggestions:
- None
Gator metadata
- Validation: Maintainer-authored implementation of accepted RFC 0012 (#2048) for issue #1737; this review is limited to PR #2942 while checking compatibility with its five downstream stack layers.
- Docs: Architecture documents are updated; driver-specific Fern UX documentation remains owned by the downstream integration layers, while the core runtime split still needs the workflow/skill synchronization in GATOR-07441545-04.
- Checks: Current-head Branch Checks and Trivy Changes are failing; Helm Lint and DCO pass. Pipeline handoff is not active while review findings remain.
- E2E:
test:e2eis present and the current-headOpenShell / E2Egate is green; no label,/ok to test, or rerun action is needed this cycle. - Head SHA:
07441545bab1241ba359eda5154236fbf775adfb - Base SHA:
5b57f0d1549677c7547862d26ff9491285ea0fbc - Merge base SHA:
5b57f0d1549677c7547862d26ff9491285ea0fbc - Patch ID:
7e44fa1b2153c53661068195adc76311e8448d3c - Gator payload:
8 - Review mode:
initial - Previous reviewed SHA: none
- Review budget exhausted: no
- Maintainer decision required: no
- Next state:
gator:in-review
| @@ -423,6 +429,10 @@ impl ServerState { | |||
| middleware_registry: Arc::new(MiddlewareRegistry::default()), | |||
| oidc_cache, | |||
| sandbox_jwt_issuer: None, | |||
| sandbox_session_jwt_authority: None, | |||
| sandbox_auth_sessions: Arc::new( | |||
There was a problem hiding this comment.
gator-agent
Warning — GATOR-07441545-01 · Gateway replacement strands live authentication
Summary: A live externally managed sandbox can survive a gateway restart, but the replacement gateway creates an empty in-memory session registry. Its supervisor’s still-valid refresh request is then rejected as inactive; when the launch tokens expire, the protected channel closes and the healthy workload freezes and is eventually terminated.
Fix: Persist or reconstruct the active session identity, credential epoch, current token identity, and retry-idempotence state so single-gateway restart—and supported replica routing—preserves refresh and revocation semantics. Add a two-ServerState restart regression test over one durable store.
Verify: Create a sandbox, retain its durable row and valid gateway token, replace ServerState, then refresh through the new state. It must rotate credentials successfully rather than return sandbox session is not active.
Agent context
- Ownership: This PR makes the new process-local registry authoritative for refresh; the base refresh path had no volatile launch-state dependency.
- Location:
crates/openshell-server/src/lib.rs:433
| @@ -1374,6 +1400,8 @@ async fn handle_stop_sandbox_inner( | |||
| let workspace = super::workspace::resolve_workspace(state.store.as_ref(), &authz.workspace) | |||
| .await? | |||
| .name; | |||
| let current = sandbox_by_name(state, &workspace, &req.name).await?; | |||
| state.sandbox_auth_sessions.deactivate(current.object_id()); | |||
There was a problem hiding this comment.
gator-agent
Warning — GATOR-07441545-02 · Failed lifecycle calls revoke a still-running sandbox
Summary: If an operator stops or deletes a sandbox and the driver returns an ambiguous/transient error, compute recovery can restore the sandbox to Ready. This line has already deactivated its authentication, however, so the surviving supervisor’s next refresh fails; expiry then disconnects and terminates a workload the API says remains ready. Delete has the same ordering defect.
Fix: Couple deactivation to the durable lifecycle outcome. Preserve or restore the exact prior active session when recovery returns to Ready, and revoke it only after stop, deletion, or an unresolved terminal transition is committed. Cover stop and delete rollback paths.
Verify: Make a fake driver fail stop/delete while its later snapshot remains running. After phase recovery returns Ready, refreshing with the current token must still succeed.
Agent context
- Sibling site:
crates/openshell-server/src/grpc/sandbox.rs:1354 - Location:
crates/openshell-server/src/grpc/sandbox.rs:1404
| let bytes = std::fs::read(config_path) | ||
| .map_err(|error| format!("read boundary config {}: {error}", config_path.display()))?; | ||
| let config: BoundaryConfig = serde_json::from_slice(&bytes).map_err(|error| { | ||
| format!("decode boundary config {}: {error}", config_path.display()) |
There was a problem hiding this comment.
gator-agent
Warning — GATOR-07441545-03 · The replacement PID 1 runtime does not reap orphans
Summary: Ordinary sandboxed code can double-fork background children. When openshell-sandbox is PID 1, exited descendants are adopted by it, but this replacement startup has no SIGCHLD orphan-reaping loop. Zombies can accumulate to the namespace/cgroup PID limit and prevent later fork or exec operations.
Fix: Move the prior managed-child-aware SIGCHLD reaper into boundary startup. Coordinate it with the existing managed-child registry so it reaps only orphan descendants and cannot steal statuses from explicit process waiters. Add a PID-namespace regression test.
Verify: Run the boundary as PID 1, repeatedly double-fork short-lived children, and confirm no adopted zombies remain; simultaneously retain a managed direct child and confirm its explicit waiter receives the correct status.
Agent context
- Ownership: This PR removes the prior runtime containing the reaper and adds this replacement boundary startup without moving that responsibility.
- Location:
crates/openshell-sandbox/src/boundary_server.rs:119
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| [package] | ||
| name = "openshell-supervisor" |
There was a problem hiding this comment.
gator-agent
Warning — GATOR-07441545-04 · Runtime split leaves operational guidance materially stale
Summary: Contributors and operators following repository guidance do not see four new security-sensitive crates, while troubleshooting and release skills still describe the old combined supervisor image—for example, expecting /openshell-sandbox inside it—and omit the separately shipped sandbox artifact. That makes the published operational workflow false for this PR’s topology.
Fix: Add openshell-binary-identity, openshell-isolation-interface, openshell-sandbox-backend, and openshell-supervisor to the architecture inventory. Update debug-openshell-cluster, helm-dev-environment, test-release-canary, and sbom to distinguish and inventory the sandbox and supervisor artifacts, then run the sync-agent-infra checklist.
Verify: Compare crates/* with the architecture table and search the mapped skills for image inventories and /openshell-sandbox; every new crate and separately published image must be represented without the obsolete combined-image diagnostic.
Agent context
- Location:
crates/openshell-supervisor/Cargo.toml:5 - Sibling sites:
skills/debug-openshell-cluster/SKILL.md:216,.agents/skills/helm-dev-environment/SKILL.md:82,.agents/skills/test-release-canary/SKILL.md:95,.agents/skills/sbom/SKILL.md:18
johntmyers
added
the
gator:in-review
3 participants
Summary
Implement RFC 0012 as one self-contained core change. This PR defines the isolation interface and adds the OpenShell implementation: the Sandbox Protocol,
OpenShellRuntimeBackend, and separateopenshell-supervisorandopenshell-sandboxbinaries.The compute-driver integrations remain in their own stacked PRs so each runtime can demonstrate the same interface without adding driver-specific behavior to the core implementation.
Related Issue
Part of #1737. Implements the architecture accepted in #2048.
Changes
openshell-isolation-interfacecrate and backend conformance tests.OpenShellRuntimeBackend, the supervisor-side implementation of the isolation interface.openshell-supervisorpolicy and access process.openshell-sandboxworkload runtime with process ownership, binary identity, TCP and DNS mediation, and fail-closed enforcement.Testing
Checklist
Stack
#2941 is superseded by the canonical RFC merged in #2048. #3151 was folded into this PR.