feat(isolation): implement the RFC 0012 sandbox architecture - #2942
feat(isolation): implement the RFC 0012 sandbox architecture#2942drew wants to merge 52 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>
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
| @@ -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
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
Re-check After Author Update
Thanks @drew. I reviewed the terminology and guidance update at d3d92a7c against the four durable obligations and the five downstream RFC 0012 layers. The naming changes remain coherent across the stack, but the three functional findings are untouched and the guidance finding is only partially addressed: the root layer now documents downstream-only sandbox_runtime_image and three-image Helm behavior while its own Helm workflow still has the two-image topology.
Action required: @drew, please address the three functional obligations and make the root-layer operational guidance truthful for behavior present in #2942, then push an updated head for another focused follow-up review.
Blocking findings:
- No new findings.
Carried findings:
GATOR-07441545-01: gateway replacement or replica routing still loses the process-local active authentication session, so a surviving supervisor cannot refresh.GATOR-07441545-02: stop and delete still revoke authentication before the compute result, leaving recovery-to-Ready unable to refresh.GATOR-07441545-03: the replacement PID 1 boundary still lacks managed-child-aware orphan reaping.GATOR-07441545-04: the crate inventory is updated, but root-layer debugging and release guidance now claims driver/Helm behavior supplied only by downstream PRs and retains stale extracted-binary terminology.
Gator metadata
- Validation: Maintainer-authored implementation of accepted RFC 0012 (#2048) for issue #1737; this review remains limited to PR #2942 while checking compatibility with its five downstream stack layers.
- Docs: Architecture inventory and terminology improved; operational skill guidance remains inconsistent with the incremental root layer under GATOR-07441545-04.
- Checks: Current-head Branch Checks and E2E are pending, Trivy Changes is failing, and Helm Lint and DCO pass; pipeline handoff is not active while review findings remain.
- E2E:
test:e2eis present and the current-head E2E workflow is running; no label,/ok to test, or rerun action is needed this cycle. - Head SHA:
d3d92a7ccc70cebbd7f4e1148fb1cdbd5f7f705a - Base SHA:
5b57f0d1549677c7547862d26ff9491285ea0fbc - Merge base SHA:
5b57f0d1549677c7547862d26ff9491285ea0fbc - Patch ID:
573196b2025255ea2a11b81f7e65556c898b54f3 - Gator payload:
8 - Review mode:
follow_up - Previous reviewed SHA:
07441545bab1241ba359eda5154236fbf775adfb - Review budget exhausted: no
- Maintainer decision required: no
- Next state:
gator:in-review
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Maintainer Convergence DecisionThe third finding-bearing review round is complete at the current head. The new scratch-image and CI-policy commits do not resolve the three functional obligations, and the scratch artifact makes the existing operational-guidance obligation more specific. A maintainer decision is now required before another autonomous Warning-bearing review. Root-cause findings:
Scope growth:
Reviewer-quality signals:
Maintainer action: @NVIDIA/openshell-maintainers, confirm that these four obligations must be completed in #2942 and direct @drew to implement them, or explicitly waive/re-scope the specific finding IDs that should not gate this independently mergeable root layer. Gator metadata
|
johntmyers
added
gator:blocked
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Re-check After Maintainer-Author UpdateThanks @drew. I reviewed The focused follow-up found no new blockers. Three durable obligations remain unchanged or became more concrete in this delta. Action required: @drew or another OpenShell maintainer, please implement Blocking findings:
Carried findings:
Gator metadata
|
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.