feat(sandbox): validate configuration before workload activation - #3259
feat(sandbox): validate configuration before workload activation#3259johntmyers wants to merge 2 commits into
Conversation
Closes #3145 Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com>
E2E Test AttestationLocal E2E checks passed for commit
Tests executed
No selected scenarios were skipped. The Kubernetes command intentionally selects standalone conformance only, with no additional Rust E2E features. Its existing dev/CI overlay disables binary-aware network policy; this does not attest the strict cross-container The first Kubernetes attempt was blocked by k3d's multi-platform image import missing a digest. Before the successful fresh run, imported the cached base image into containerd explicitly with Full |
Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com>
|
🌿 Preview your docs: https://nvidia-preview-pr-3259.docs.buildwithfern.com/openshell |
johntmyers
added
the
gator:in-review
johntmyers
left a comment
johntmyers
left a comment
There was a problem hiding this comment.
gator-agent
PR Review Status
The accepted issue and updated architecture, Fern docs, skills, and tests make this cross-cutting admission change project-valid. The initial review found two blocking startup-lifecycle correctness issues.
Action required: address GATOR-bde03eff-01 and GATOR-bde03eff-02, then push an updated head for a focused follow-up review.
Blocking findings:
GATOR-bde03eff-01: non-repairable startup and sidecar failures can wait forever.GATOR-bde03eff-02: restart admission can reopen static-policy replacement after a workload has already activated.
Carried findings:
- None
Gator metadata
- Validation: Implements accepted issue #3145 with explicit architecture and user documentation.
- Docs: Fern docs updated for admission status and repair workflow.
- Checks: Current-head branch, Helm, Trivy, and DCO gates are green; required E2E dispatch is deferred while blocking review findings remain.
- E2E:
test:e2erequired for sandbox lifecycle, gateway/supervisor, policy, and provider-flow changes; not yet applied. - Head SHA:
bde03eff0530e92a1aaa43f50df4c8a64dfb0474 - Base SHA:
0357daee316f32a4d5c312174d68672cb0f4d389 - Merge base SHA:
0357daee316f32a4d5c312174d68672cb0f4d389 - Patch ID:
7d3bc99d684bba94391baaf2eb70dc73e59b1103 - Gator payload:
8 - Review mode:
initial - Previous reviewed SHA: none
- Review budget exhausted: no
- Maintainer decision required: no
- Next state:
gator:in-review
| let registration_snapshot = loop { | ||
| match gateway.snapshot(id).await { | ||
| Ok(snapshot) => break snapshot, | ||
| Err(_) => tokio::time::sleep(Duration::from_secs(2)).await, |
There was a problem hiding this comment.
gator-agent
Warning — GATOR-bde03eff-01 · Non-repairable startup failures wait forever
Summary: An operator with an incompatible or unauthorized gateway, a fenced supervisor, or a missing process sidecar can leave a sandbox in Provisioning forever. These new loops discard all gateway errors, while sidecar discovery has no timeout, so failures that policy/provider repair cannot fix neither surface an actionable error nor let the supervisor restart.
Fix: Retry only classified transient failures with bounded backoff; return non-transient or exhausted failures, terminate rejected registrations, and bound sidecar discovery with the existing ready timeout. Keep indefinite waiting only after the gateway explicitly reports a repairable configuration rejection.
Verify: Make StartupGateway::snapshot return permission-denied or not-found and assert startup returns that error; start a pending sidecar server without a client and assert it fails after the ready timeout.
Agent context
- Location:
crates/openshell-sandbox/src/lib.rs:2581 - Sibling sites: registration at lines 2584–2598, later snapshots at 2602–2605, and sidecar discovery at 174–180.
| admission.state | ||
| != i32::from(openshell_core::proto::ConfigurationAdmissionState::Accepted) | ||
| }); | ||
| let should_backfill_policy = if startup_blocked && !sandbox_caller { |
There was a problem hiding this comment.
gator-agent
Warning — GATOR-bde03eff-02 · Restart reopens immutable static policy
Summary: An authorized operator can stop and restart a sandbox that already ran, hold restart admission in Pending or Rejected, and replace filesystem, Landlock, or process fields. Restart resets admission to Pending, but this predicate interprets any non-Accepted state as proof that no child has consumed static restrictions, breaking the documented immutable-after-activation contract.
Fix: Gate static replacement on durable evidence that the sandbox has never activated, not the current admission state. Use the retained process-instance identity only if its lifecycle guarantees this distinction; otherwise persist an explicit first-activation marker. Apply the predicate to every projection path.
Verify: Activate, stop, and restart a sandbox into Pending/Rejected admission, then submit a policy with changed static fields; the update must still be rejected.
Agent context
- Location:
crates/openshell-server/src/grpc/policy.rs:3778 - Sibling sites: admission reset in
compute/mod.rs:1487and projection paths inpolicy.rs:2324andpolicy_store.rs:64.
johntmyers
added
gator:blocked
1 participant
Summary
An image policy can be valid alone but become invalid after credentialed-provider composition. Keep the workload stopped until the supervisor validates and accepts the exact effective policy and matching provider environment. Expose a repairable configuration error instead of starting the workload and discovering the problem on its first connection.
Related Issue
Closes #3145
Changes
Scope and compatibility
Gateway-managed supervisors require the admission contract; missing admission from older gateways is fail-closed. Explicit user/global policy precedence remains unchanged. Image/host setup failures such as unresolved OCI users retain their existing startup error behavior. Sidecar disconnects retain existing fail-closed behavior and do not add transparent independent restart recovery.
Testing
mise run pre-commitmise run ci(full Rust, Python, TypeScript, and Go checks/tests)OPENSHELL_E2E_DOCKER_TEST=policy_activation mise run e2e:docker: conformance smoke and activation/repair/restart scenario passedOPENSHELL_E2E_KUBE_EXTRA_VALUES=deploy/helm/openshell/ci/values-sidecar.yaml OPENSHELL_E2E_KUBERNETES_FEATURES='' mise run e2e:kubernetes: sidecar conformance smoke passedFull CI used a command-local
tag.gpgSign=falseoverride because the workstation's global signing setting interferes with temporary Git fixture repositories. The existing SBOM timing test passed in the completed run; no SBOM code was changed.For Kubernetes, imported the base image explicitly with containerd's
--platform linux/arm64before sandbox creation: k3d's default multi-platform archive import reported a missing digest. The completed smoke run required no pod intervention. The dev/CI sidecar overlay uses its existingprocessBinaryAwareNetworkPolicy: falsesetting.E2E results are attested against implementation commit
43095b382e62f489a50849b0c043599bb0b86006. After merging main0357daee, regenerated the conflicting Go bindings, reconciled the new schema inventory, added a legacy-storage fixture, and reran full CI successfully. E2E lanes were not rerun after that merge.Added coverage includes rejected image/provider composition and atomic repair, stale supervisor/generation reports, readiness across restart, pending/repaired sidecar bootstrap, provider-policy publication, CLI/Go admission output, and a Docker workload-marker scenario proving no launch before repair followed by successful activation and restart.
Checklist