Uh oh!
There was an error while loading. Please reload this page.
feat(sandbox): support rootfs tar as --from source for VM driver - #2863
feat(sandbox): support rootfs tar as --from source for VM driver#2863feloy wants to merge 5 commits into
Conversation
johntmyers
left a comment
There was a problem hiding this comment.
gator-agent
PR Review Status
The VM rootfs-tar feature is project-valid and documented, but this head has three blocking implementation issues: the new raw host-path field crosses the driver trust boundary, the CLI encodes that field outside the required vm configuration object, and the cache key can collide or remain stale.
Action required: @feloy, please address GATOR-28b5152e-01, GATOR-28b5152e-02, and GATOR-28b5152e-03, including the requested regression coverage.
Blocking findings:
GATOR-28b5152e-01: caller-controlled host paths reach privileged VM-driver filesystem reads.GATOR-28b5152e-02: the CLI-generated rootfs path is discarded by driver-config selection.GATOR-28b5152e-03: rootfs archive cache identities are collision-prone, stale at sub-second updates, and unbounded.
Carried findings:
- None
Gator metadata
- Validation: Project-valid implementation of maintainer-supported issue #2175, scoped to local VM gateways.
- Docs: Fern sandbox documentation updated for the direct UX change.
- Checks: Current required Branch Checks and Helm Lint have not run and remain pending.
- E2E:
test:e2eis required for VM sandbox lifecycle behavior but is not dispatched while review blockers remain. - Head SHA:
28b5152ee0e3bc377b555cdbb89faa7cc8bc1f5c - Base SHA:
40d1b4866691be03b5617089739c7f2325487479 - Merge base SHA:
40d1b4866691be03b5617089739c7f2325487479 - Patch ID:
4f052d6ec908bc7bf57cfeb49f381929c826deee - Gator payload:
7 - Review mode:
initial - Previous reviewed SHA: none
- Review budget exhausted: no
- Maintainer decision required: no
- Next state:
gator:in-review
| ("image_identity".to_string(), cache_identity.clone()), | ||
| ]), | ||
| ); | ||
| if let Err(err) = tokio::fs::copy(tar_path, &rootfs_archive).await { |
There was a problem hiding this comment.
gator-agent
Critical — GATOR-28b5152e-01 · Caller-controlled host paths reach the VM driver
Summary: An authenticated sandbox creator can set driver_config.vm.rootfs_tar_path directly, bypassing the CLI-only locality check. The privileged VM driver then copies that host path; a special or oversized file can exhaust gateway storage, and a readable host tar can be imported into the requester’s sandbox.
Fix: Replace the caller-controlled raw path with a server-created trusted staging reference. At the driver boundary, canonicalize it, require a regular file inside that staging root, enforce a configured size limit, and reject other paths before I/O.
Verify: Submit direct API requests using /dev/zero and a readable path outside staging; both must be rejected before metadata or copy operations, while a staged regular archive succeeds.
Agent context
- Agent path:
CreateSandboxdriver config →VmSandboxDriverConfig→tokio::fs::copy - Ownership: This PR adds the accepted path field and privileged copy sink.
- Location:
crates/openshell-driver-vm/src/driver.rs:2636
| /// Build a `driver_config` struct carrying the rootfs tar path for the VM driver. | ||
| fn rootfs_tar_driver_config(tar_path: &Path) -> Result<prost_types::Struct> { | ||
| let fields = serde_json::Map::from_iter([( | ||
| "rootfs_tar_path".to_string(), |
There was a problem hiding this comment.
gator-agent
Warning — GATOR-28b5152e-02 · Rootfs configuration is dropped before the VM driver
Summary: An operator using the documented --from ./rootfs.tar flow gets a top-level rootfs_tar_path, but the server forwards only driver_config.vm. The new field is therefore discarded, and the image-less VM template fails validation instead of creating a sandbox.
Fix: Encode the generated value as {"vm": {"rootfs_tar_path": path}} and deep-merge it into an existing vm object so other VM settings remain intact.
Verify: Pass the CLI-produced template through server driver selection and assert the VM driver receives both rootfs_tar_path and an existing VM field; the included end-to-end rootfs-tar scenario must then provision successfully.
Agent context
- Ownership: This helper is the new feature’s only archive-path transport.
- Location:
crates/openshell-cli/src/run.rs:1288
| .duration_since(std::time::SystemTime::UNIX_EPOCH) | ||
| .unwrap_or_default() | ||
| .as_secs(); | ||
| let tar_identity = format!("rootfs-tar:{}:{mtime}", tar_path.display()); |
There was a problem hiding this comment.
gator-agent
Warning — GATOR-28b5152e-03 · Rootfs archive cache keys can collide or remain stale
Summary: Archive identity combines an unbounded path with mtime rounded to seconds, then maps all punctuation to -. Distinct paths such as /tmp/a/b.tar and /tmp/a-b.tar can reuse the same prepared disk, same-second rewrites can keep stale contents, and long valid paths can exceed filesystem component limits.
Fix: Use a fixed-length cryptographic hash of canonical path bytes, file length, and the full available modified timestamp. Re-stat after copying and retry or reject if those attributes changed during staging.
Verify: Cover separator-colliding paths with equal mtimes, a same-second rewrite, and a long canonical path; each distinct archive version must select a distinct bounded key and produce its own filesystem.
Agent context
- Ownership: This PR newly feeds arbitrary filesystem paths and second-truncated metadata into the existing sanitizer.
- Location:
crates/openshell-driver-vm/src/driver.rs:2589
Accept flat rootfs tar archives (.tar, .tar.gz, .tgz) via the --from flag for VM-backed gateways. The CLI detects the archive extension, validates that the gateway uses the VM compute driver, and passes the tar path through driver_config. The VM driver copies the tar into its staging area and feeds it into the existing rootfs extraction and ext4 disk creation pipeline, skipping the container image pull/export steps. ClosesNVIDIA#2175 Signed-off-by: Philippe Martin <phmartin@redhat.com>
The rootfs_tar_path field in driver_config was passed from the API caller directly to tokio::fs::copy without validation. An authenticated user bypassing the CLI could supply arbitrary host paths (e.g. /dev/zero for disk exhaustion, or readable host files for data exfiltration). Introduce a trusted staging directory that the VM driver creates on startup and advertises via GetCapabilities. The CLI now copies the tar into the staging directory before creating the sandbox, and the driver validates that the received path is a regular file inside the staging root and within a configurable size limit (default 10 GiB) before any I/O. New VmDriverConfig options: - rootfs_tar_staging_dir: override the staging directory (default: <state_dir>/rootfs-tar-staging) - rootfs_tar_max_bytes: override the size limit (default: 10 GiB) Addresses GATOR-28b5152e-01. Signed-off-by: Philippe Martin <phmartin@redhat.com>
johntmyers
left a comment
There was a problem hiding this comment.
gator-agent
PR Review Status
Thanks @feloy, I checked the new staging-boundary validation against the three existing findings. The new head narrows arbitrary host-path access, but the documented CLI still copies archives into shared persistent staging before enforcing the configured limit, still encodes the rootfs path outside driver_config.vm, and leaves the cache identity and copy-consistency defects unchanged. No new blockers were added.
Action required: @feloy, please finish the three carried obligations and their requested regression coverage.
Blocking findings:
- No new blocking findings.
Carried findings:
GATOR-28b5152e-01: use request-bound server-controlled staging, enforce the byte limit while receiving the archive, prevent cross-request path selection, and clean up staged data.GATOR-28b5152e-02: nestrootfs_tar_pathunderdriver_config.vm, deep-merge existing VM settings, and cover the CLI-to-driver transport.GATOR-28b5152e-03: use a fixed-length collision-resistant version identity and reject or retry if the archive changes during copy.
Gator metadata
- Validation: Project-valid implementation of maintainer-supported issue #2175, scoped to local VM gateways.
- Docs: Fern sandbox documentation is present for the direct UX change.
- Checks: Current-head Branch Checks and Helm Lint are pending; pipeline dispatch waits for review blockers to resolve.
- E2E:
test:e2eremains required for VM sandbox lifecycle behavior and will be dispatched after review blockers resolve. - Head SHA:
75e1f2be4b522e53381ab4e2a958c6dba7e031ff - Base SHA:
8be8b62ab58e42ee08213494b0d556ebc829e5de - Merge base SHA:
8be8b62ab58e42ee08213494b0d556ebc829e5de - Patch ID:
2c200e7c9e38781640f7fbeaa9eab2919ebce81b - Gator payload:
7 - Review mode:
follow_up - Previous reviewed SHA:
28b5152ee0e3bc377b555cdbb89faa7cc8bc1f5c - Review budget exhausted:
no - Maintainer decision required:
no - Next state:
gator:in-review
… rootfs tar Tighten the rootfs tar staging flow to address the remaining GATOR-01 obligations: - Request-scoped staging: the CLI creates a unique per-request subdirectory (req-<pid>) under the staging root instead of placing files directly in the shared directory. The driver enforces that the tar path is at depth 2 (staging_root/<subdir>/<file>), preventing cross-request path selection. - Size pre-check: the driver advertises rootfs_tar_max_bytes via GetCapabilities. The CLI reads this limit and rejects oversized files before copying, avoiding disk exhaustion in the staging directory. - Cleanup: the driver removes the request staging subdirectory after consuming the tar (on cache hit, copy success, or copy failure), ensuring staged data does not persist beyond the request. Signed-off-by: Philippe Martin <phmartin@redhat.com>
johntmyers
left a comment
There was a problem hiding this comment.
gator-agent
PR Review Status
Thanks @feloy, I checked the latest staging-limit, request-directory, and cleanup changes against the three existing obligations. The raw staging reference is still not request-bound or bounded while copying, the CLI transport and cache identity remain unchanged, and the new cleanup makes a successfully created rootfs-tar sandbox unable to restart or restore.
Action required: @feloy, please address GATOR-168b9210-01 and finish the three carried obligations with their requested regression coverage.
Blocking findings:
GATOR-168b9210-01: deleting the one-shot staged archive leaves persisted sandbox state pointing to a path that restart and restoration must canonicalize.
Carried findings:
GATOR-28b5152e-01: replace caller-selected staging paths with request-bound server-controlled staging, enforce the byte limit while receiving, and clean up every terminal path safely.GATOR-28b5152e-02: nestrootfs_tar_pathunderdriver_config.vm, deep-merge existing VM settings, and cover CLI-to-driver transport.GATOR-28b5152e-03: use a fixed-length collision-resistant archive-version identity and reject or retry if the source changes during copy.
Gator metadata
- Validation: Project-valid implementation of maintainer-supported issue #2175, scoped to local VM gateways.
- Docs: Fern sandbox documentation remains present for the direct UX change.
- Checks: Current-head Branch Checks and Helm Lint are pending; pipeline dispatch remains gated on review blockers.
- E2E:
test:e2eis required for VM sandbox lifecycle behavior but must not be dispatched while review blockers remain. - Head SHA:
168b9210cc70a99b003a18b18b2bead91d30f0b9 - Base SHA:
8be8b62ab58e42ee08213494b0d556ebc829e5de - Merge base SHA:
8be8b62ab58e42ee08213494b0d556ebc829e5de - Patch ID:
1f7bcbdefada29ff2276cb444f62a4b8984857ae - Gator payload:
7 - Review mode:
follow_up - Previous reviewed SHA:
75e1f2be4b522e53381ab4e2a958c6dba7e031ff - Review budget exhausted:
yes - Maintainer decision required:
no - Next state:
gator:in-review
Uh oh!
There was an error while loading. Please reload this page.
On restore or restart, the one-shot staged tar archive has already been cleaned up. Reading the persisted image identity from the sandbox state directory and resolving the cached disk path directly avoids re-accessing the deleted staging path. Addresses GATOR-168b9210-01. Signed-off-by: Philippe Martin <phmartin@redhat.com>
johntmyers
commented
Aug 26, 2026
Maintainer Convergence DecisionThanks @feloy. I checked the new persisted-image restoration path at head Root-cause findings:
Scope growth:
Reviewer-quality signals:
Maintainer action: @NVIDIA/openshell-maintainers, please decide whether Gator metadata
|
…s tar copy Replace PID-based request staging directories with tempfile-generated random names to prevent collisions and make paths unpredictable. Replace bare tokio::fs::copy with a streaming copy loop that enforces the advertised max_bytes limit during transfer, closing the TOCTOU gap between the pre-copy size check and the actual copy. Signed-off-by: Philippe Martin <phmartin@nvidia.com> Signed-off-by: Philippe Martin <phmartin@redhat.com>
johntmyers
commented
Aug 26, 2026
Maintainer Convergence DecisionThanks @feloy. I checked the random request-staging directory and bounded-copy update at head Root-cause findings:
Scope growth:
Reviewer-quality signals:
Maintainer action: @NVIDIA/openshell-maintainers, please decide whether the remaining part of Gator metadata
|
Summary
.tar,.tar.gz,.tgz) as a new--fromsource for VM-driver sandboxes, enabling daemon-free sandbox creation from flat filesystem archives produced bydocker export,podman export, orbuildah mount+tardriver_configRelated Issue
Closes#2175
Changes
CLI (
crates/openshell-cli/)ResolvedSource::RootfsTarvariant inresolve_from()with extension-based detectionvalidate_rootfs_tar_source()checks local-gateway and VM-driver constraints viaGetGatewayInfoRPCrootfs_tar_driver_config()andmerge_driver_config()encode the tar path intodriver_config--fromhelp text and error messagesVM driver (
crates/openshell-driver-vm/)rootfs_tar_pathfield onVmSandboxDriverConfigensure_prepared_rootfs_tar_disk()with double-checked cache locking and mtime-based identityvalidate_sandbox(),create_sandbox(),prepare_runtime_images(), and reconciliation to accept rootfs tar as an alternative to image referencesbootstrap_image_ref_default()for reuse in rootfs tar and reconciliation pathsDocs (
docs/sandboxes/manage-sandboxes.mdx)--fromdocumentation with rootfs tar example and VM-driver constraintTests
run.rs: extension detection for.tar/.tar.gz/.tgz, missing archive rejection,filename_looks_like_rootfs_tarcoveragee2e/rust/tests/rootfs_tar.rs): builds a Docker image, exports a flat rootfs tar, creates a VM sandbox from it, and verifies a marker fileTesting
mise run pre-commitpassesChecklist
driver_configcarries the tar path)