[pimlock]

Summary

A retained sandbox stays reachable after its main process exits, but only through some of the ways into it. This PR puts every way in behind one access check, so they all agree on when a sandbox can be used.

Related Issue

Closes #4949

What's inconsistent today

Each entry point checks the sandbox phase on its own, and they don't agree:

Entry point	Ready	Completed / Error
SSH, file transfer, port forward	allowed	allowed if the supervisor is connected to the replica that got the request
Exec, interactive exec	allowed	refused
Exposed services	allowed	refused
CLI exec (client-side check)	allowed	refused before calling the gateway

For example, a job writes result.txt and exits 0. The sandbox moves to Completed and its supervisor stays connected. Then:

openshell sandbox download job result.txt ./   # works
openshell sandbox exec --name job -- cat result.txt
# Error: sandbox 'job' is not ready (phase: Completed); wait for it to reach Ready state

With several gateway replicas, even the paths that do work after exit are unreliable. They only look for the supervisor on the replica that received the request, so the same download works or fails depending on where the load balancer sends it.

What changes

One rule now covers every way in:

• Ready: reachable.
• Completed or Error: reachable while the supervisor is connected to any replica.
• Anything else: refused.

Exposed services have one extra limit. They don't authenticate callers, so they reach an Error sandbox only when its main process failed. A sandbox in Error because of an infrastructure failure stays reachable for debugging only through the authenticated paths.

The rule lives in a new gateway module, sandbox_access. Every entry point calls sandbox_access::acquire and opens its relay through the handle it returns, so none of them can skip the check:

flowchart LR
  exec[ExecSandbox] --> acquire
  iexec[ExecSandboxInteractive] --> acquire
  ssh[CreateSshSession] --> acquire
  fwd[ForwardTcp<br/>SSH, file transfer, port forward] --> acquire
  svc[Exposed services<br/>every request] --> acquire
  acquire{"sandbox_access::acquire<br/>phase + supervisor on any replica"} -->|SandboxAccess| relay[open_relay] --> sup[Supervisor]
  acquire -->|refused| err[FailedPrecondition<br/>sandbox is not ready]

Loading

The same example after this PR:

openshell sandbox exec --name job -- cat result.txt   # prints the result
openshell sandbox delete job

The gateway is the only place that decides. The CLI no longer checks the phase before exec. When the gateway refuses, the CLI error still names the phase it saw and what to do about it. A service forward in the CLI or the TypeScript SDK stops only when its sandbox is stopping, stopped or being deleted, and leaves the per-connection decision to the gateway.

New ways into a sandbox should go through acquire as well. So should anything that has to happen before a sandbox can be used, such as resuming a suspended sandbox.

Changes

• Gateway: new sandbox_access module. Exec, interactive exec, SSH sessions, port forwarding and exposed services go through it. Exposed services check access on every request, including requests served over a pooled upstream connection. supervisor_session::supervisor_connected checks all replicas through the owner record.
• CLI and TypeScript SDK: leave the access decision to the gateway.
• Docs and the CLI skill: what still works after a retained sandbox's main process exits, and that --no-keep sandboxes are deleted.
• Conformance: two CLI tests in tests/suites/conformance/tests/cli.rs, each run for exit codes 0 and 3. A --no-keep sandbox is deleted. A retained sandbox records the exit code and still accepts exec.

Testing

• Checks appropriate to the affected code and behavior pass
• Unit tests added/updated
• E2E tests added/updated

I ran the conformance checks against a local Docker gateway before the sandbox_access refactor and before the conformance suite moved to testkit. Unit tests and the CLI exec integration tests cover the refactor. I haven't run the testkit version of the conformance tests, and I haven't tested Kubernetes, Podman or a real multi-replica gateway live.

Still broken and tracked in #4949: after a Docker gateway restart, Completed and Error sandboxes lose their supervisor and can't be reached.

Checklist

• Follows Conventional Commits
• Commits are signed off (DCO)
• Architecture docs updated (if applicable)

[github-actions]

🌿 Preview your docs: https://nvidia-preview-pr-4950.docs.buildwithfern.com/openshell

[pimlock]

Error covers two cases here: a main process that exited nonzero (MainProcessFailed) and an infrastructure failure. We considered giving a failed or exited main process its own phase, and also reusing Completed for every exit. We decided against both for this PR:

• Older SDKs treat Completed as success in wait_ready, so a failed run would be reported as a successful one.
• A new phase reaches older clients as an unknown value, and they keep polling until they time out.

This PR keeps the existing phases. The access rule doesn't need a new phase to be correct.

[elezar]

Does this function not represent a decision by the CLI? Would adding a property to the sandbox or a filter on the GetSandbox request make sense?

[elezar]

Thanks for tackling this. A single acquire gate with a capability handle is a good fit for the problem, and the multi-replica owner-record handling reads correctly. I have no blockers on the gateway logic. My comments fall into two groups:

1. Exposed services after exit. This is the one behavioural expansion I'd like an explicit decision on (comments on service_routing.rs and sandbox_access.rs).
2. Clients re-implementing the gateway's rules. The CLI and SDK keep their own phase lists, which weakens the "gateway is the only place that decides" design. Carrying the refusal reason in the error would remove them (comments on sandbox_access.rs, run.rs, client.ts). These are non-blocking.

Test coverage. CI conformance passes on Docker, Podman, k3s and k3s-ha (three replicas), which is good coverage, and the new retained-sandbox tests run there. Two gaps remain:

• It would be worth confirming that the k3s-ha run served a request on a replica other than the one holding the supervisor, or adding a test that forces it. Otherwise only the peer_holds_session unit test covers the cross-replica path directly.
• I don't see end-to-end coverage for exposed services on a Completed sandbox. Only the unit test completed_sandbox_routes_services_only_while_supervisor_is_live covers it.

[elezar]

This is the one change I'd like an explicit decision on. Before, a service request to a Completed sandbox (or an Error one whose main process failed) was refused. Now it's relayed through the retained supervisor, and the gateway doesn't authenticate service callers (auth is the service's own, per ServiceAuthorizationMode). If anything a job left behind is still listening on the endpoint's port, anyone with the service URL can reach it after exit.

Two questions: (a) does the supervisor tear down the workload's process tree on main-process exit, so nothing can be listening? If so, a test or a note here would settle it. (b) What's the use case for serving after exit? #4949 needs exec, SSH and file transfer, which are all authenticated. If services don't need it, I'd limit acquire_for_exposed_service to Ready. Otherwise I'd make it opt-in per endpoint.

[elezar]

Related to the above. The failed-main-process Error carve-out for exposed services is the most permissive unauthenticated case, so it's worth stating the reasoning in the doc comment. If post-exit service access is deliberate, say why. If it isn't, this branch goes away together with the previous comment.

[elezar]

Suggestion: let NotReachable carry a reason and attach it as a google.rpc.ErrorInfo on the existing FailedPrecondition, keeping the message "sandbox is not ready". acquire already knows which branch it took. The repo already does this (openshell-core::rpc_error, and provider.rs keys on an ErrorInfo reason), so it follows convention.

Something like: PhaseNotServing, Stopped, Deleting, SupervisorDisconnected, ServiceInfraError, with the phase in metadata. That needs splitting Stopped/Deleting out of the _ => false arm. Older clients are unaffected, and newer clients against an older gateway fall back to the raw status.

[elezar]

This matches the gateway's message text ("sandbox is not ready") and picks a hint from a phase fetched earlier. Two problems: a reworded gateway message silently drops the hint, and "supervisor is no longer connected" can be wrong when the refusal had another cause (a Lookup error, or the phase changed since the fetch). With the reason from the sandbox_access.rs comment, this becomes a match on the validated ErrorInfo, in the style of provider_mutation_is_uncertain, and uses the gateway's phase.

[elezar]

This and the SDK check (sdk/typescript/src/client.ts:1754) keep their own copy of "stopping, stopped or deleting means no connection can succeed", which is a reachability rule outside the gateway. It'll drift (Suspended already differs). Related: a forward against Starting or Suspended used to fail up front with "not ready". Now it binds and every connection fails at the gateway.

Suggestion: at startup, ask the gateway and act on the reason from the sandbox_access.rs comment. Give up on Stopped or Deleting. For phases that can still become reachable, wait with a bounded retry and then fail with the gateway's reason. That removes both phase lists and restores a clear startup error. If you'd rather keep the lists, a comment pointing at sandbox_access.rs and a parity test would reduce the drift risk.

[elezar]

Same point as the run.rs comment on forward_ended_by_phase: this is a second copy of the reachability phase list, and it moves the failure for Starting/Suspended from startup to each connection. Reading the reason from the gateway would let both clients drop it.

[elezar]

Good unit coverage for the gate. Since this is the expansion with the most security weight (see the comment on proxy_to_endpoint), an end-to-end check would be worth having, in either direction: that a service on a Completed sandbox is refused (if you take the Ready-only option), or that it's served and nothing unintended is reachable (if you keep it). Right now CI has no conformance test that touches it.

[elezar]

#5277 adds a features/host-gateway test that currently works around the multi-replica attach race this PR fixes (#4949). It runs its client through sandbox exec in a Ready sandbox, because on k3s-ha a short main process can finish before the CLI attaches and the receiving replica answers "sandbox is not ready".

If this PR lands first, update tests/suites/features/host-gateway/tests/host_gateway.rs to run the client as the main process of sandbox create and remove the workaround comment. The k3s-ha row would then confirm the fix on a real multi-replica gateway, which this PR's description says hasn't been tested.