feat(docker): isolate workloads behind the host supervisor - #2965
Conversation
|
🌿 Preview your docs: https://nvidia-preview-pr-2965.docs.buildwithfern.com/openshell |
3f387e4 to
9f08093
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. |
drew
changed the title
0236ea1 to
7f9fa25
Compare
7f9fa25 to
e3b8a72
Compare
2026bdc to
f3fd9d9
Compare
drew
changed the title
d992755 to
f6bc10a
Compare
f6bc10a to
c63a604
Compare
|
Label |
c63a604 to
c3a22d3
Compare
|
/ok to test |
3 similar comments
|
/ok to test |
|
/ok to test |
|
/ok to test |
ae3939c to
a62c4b7
Compare
drew
changed the title
a62c4b7 to
c59f844
Compare
c59f844 to
744d9e9
Compare
744d9e9 to
3fe2973
Compare
3fe2973 to
b5fd2f2
Compare
480c8a0 to
c8e388e
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>
johntmyers
left a comment
johntmyers
left a comment
There was a problem hiding this comment.
gator-agent
PR Review Status
The incremental Docker layer is project-valid and its split-runtime direction matches the accepted RFC 0012 stack, but this head has four concrete blockers. The most serious lets a supported workload identity collide with the fixed supervisor identity and read supervisor-only authentication material; the remaining findings cover cross-workspace lifecycle selection, incompatible schema-v2 configuration changes, and an E2E image that cannot execute its dynamically linked supervisor.
Action required: @drew, please address GATOR-134c96ed-01 through GATOR-134c96ed-04 and push an updated head for a focused follow-up review.
Blocking findings:
GATOR-134c96ed-01: supervisor credentials are exposed to workloads running as UID/GID 65534.GATOR-134c96ed-02: ID-or-name pending lookup can select another workspace's same-named sandbox.GATOR-134c96ed-03: the Docker configuration parser, first-party config, docs, RFC text, and operator guidance disagree.GATOR-134c96ed-04: the generated E2E supervisor image omits its GNU runtime dependencies.
Carried findings:
- None.
Non-blocking suggestions:
- None.
Gator metadata
- Validation: Maintainer-authored Docker implementation layer in the accepted RFC 0012 isolation stack rooted at #2942 and linked to #1737/#2048.
- Docs: Updated, but materially inconsistent with the implemented schema and split runtime images; see
GATOR-134c96ed-03. - Checks: Current-head
OpenShell / Branch Checksis failing, including the RPC schema inventory check. - E2E:
test:e2eis applied and current-headOpenShell / E2Ewas dispatched but is failing across Docker lanes. - Head SHA:
134c96ed917bb36bcb63571334f1907da4eaf91e - Base SHA:
d88b88430c289691f373487e91c9129fa902b98d - Merge base SHA:
d88b88430c289691f373487e91c9129fa902b98d - Patch ID:
6fbdd8a9b1e664f97dd33dd1a3ded9d679c48e72 - Gator payload:
8 - Review mode:
initial - Previous reviewed SHA: none
- Review budget exhausted: no
- Maintainer decision required: no
- Next state:
gator:in-review
| let mut archive = tar::Builder::new(Vec::new()); | ||
| append_docker_archive_directory( | ||
| &mut archive, | ||
| ".openshell/channel/supervisor", |
There was a problem hiding this comment.
gator-agent
Critical — GATOR-134c96ed-01 · Workload can read supervisor credentials
Summary: A supported Docker workload running as UID/GID 65534 shares the fixed identity used to own auth.json and optional gateway TLS keys. Because the entire channel volume is writable in the workload, its code can read the launch-scoped supervisor JWT and gateway client private key, then overwrite state consumed by a restarted supervisor.
Fix: Put supervisor-only files in storage never mounted into the workload, such as direct supervisor staging or a separate supervisor-only volume. Add coverage using workload UID/GID 65534.
Verify: Launch an image or policy as 65534:65534, then test /.openshell/channel/supervisor/auth.json and the configured TLS key. Both must be absent or unreadable, including after supervisor restart.
Agent context
- Location:
crates/openshell-driver-docker/src/lib.rs:4245 - Ownership: This PR adds the fixed supervisor UID, credential archive, and shared workload-writable channel volume; the shared identity contract rejects only UID/GID zero.
| } | ||
| Ok(Some(id)) | ||
| fn pending_sandbox_matches(sandbox: &DriverSandbox, sandbox_id: &str, sandbox_name: &str) -> bool { | ||
| (!sandbox_id.is_empty() && sandbox.id == sandbox_id) |
There was a problem hiding this comment.
gator-agent
Warning — GATOR-134c96ed-02 · Pending lookup can target another workspace
Summary: When the normal get or delete request carries both an ID and a workspace-scoped name, this predicate matches either value. If another workspace is provisioning a same-named sandbox, HashMap iteration can select and abort that unrelated sandbox even though the supplied ID names a different one, leaving gateway state inconsistent.
Fix: Restore ID-first resolution: match only a non-empty ID; otherwise require a unique name match. Allow duplicate names across workspaces and restore the removed ambiguity tests.
Verify: Store pending sbx-beta/demo, then remove using sbx-alpha/demo; beta must remain. With two demo entries in different workspaces, a name-only lookup must return FailedPrecondition rather than an arbitrary record.
Agent context
- Location:
crates/openshell-driver-docker/src/lib.rs:3274 - Ownership: This PR replaces the base's ID-authoritative resolver and removes its cross-workspace regression tests.
| @@ -128,20 +161,18 @@ pub struct DockerComputeConfig { | |||
| pub default_image: String, | |||
|
|
|||
| /// Image pull policy for sandbox images. | |||
| pub image_pull_policy: ImagePullPolicy, | |||
| pub image_pull_policy: String, | |||
There was a problem hiding this comment.
gator-agent
Warning — GATOR-134c96ed-03 · Docker schema-v2 configuration is incompatible
Summary: Existing Docker gateways and this repository's E2E config fail under deny_unknown_fields because this diff replaces supported fields such as sandbox_label; after those are removed, the documented if_not_present pull policy is rejected during first provisioning. Changed docs also disagree about the gateway selector and split runtime images.
Fix: Preserve the base schema contract and shared ImagePullPolicy parser, or add explicit compatible aliases for intentional renames. Reconcile the first-party E2E config, gateway/driver docs, RFC text, and debug-openshell-cluster guidance with the resulting split-image schema.
Verify: Deserialize e2e/configs/gateway/docker.toml through the real driver context and provision with image_pull_policy="if_not_present"; startup and provisioning must accept it, and every published example must use the same accepted fields.
Agent context
- Location:
crates/openshell-driver-docker/src/lib.rs:164 - Ownership: This incremental layer changes the field/type contract and deletes the base configuration regression tests.
| mkdir -p "${supervisor_rootfs}" | ||
| install -m 0555 "${linux_sandbox_bin}" "${supervisor_rootfs}/openshell-sandbox" | ||
| tar -C "${supervisor_rootfs}" -cf "${supervisor_archive}" openshell-sandbox | ||
| install -m 0555 "${linux_supervisor_bin}" "${supervisor_rootfs}/openshell-supervisor" |
There was a problem hiding this comment.
gator-agent
Warning — GATOR-134c96ed-04 · Generated supervisor image cannot execute
Summary: The standard Docker/Podman E2E runner now builds openshell-supervisor for a dynamically linked GNU target, but imports a root filesystem containing only that executable. Without its ELF interpreter, glibc libraries, or CA certificates, the companion exits before readiness, so the required driver E2E lanes cannot validate this isolation layer.
Fix: Build the test image from the existing Debian-based supervisor Dockerfile, or package the GNU interpreter, required libraries, and CA certificates. Add an image preflight that executes the supervisor before gateway launch.
Verify: Inspect the built binary's interpreter, import the generated image, and run its supervisor health/version entry point. It must execute successfully in host and VM Docker/Podman paths before the gateway starts.
Agent context
- Location:
e2e/run.sh:319 - Ownership: This PR replaces the base's static musl one-file image with a GNU binary without adding its runtime filesystem.
2 participants
Summary
Adopt the RFC 0012 split in the Docker driver. The workload container runs
openshell-sandboxwith Docker networking disabled; the host supervisor owns policy and all mediated TCP and DNS access.The driver provisions a private Unix socket. The protocol uses a fresh pinned TLS server identity and launch-scoped sandbox JWT over that socket, with new material on every start after stop.
Related Issue
Part of #1737.
Changes
network_mode=none.Testing
mise run pre-commitChecklist
Stack