feat: disposable macOS guests with pre-baked pulled base images#359
feat: disposable macOS guests with pre-baked pulled base images#359PeronGH wants to merge 51 commits into
Conversation
|
Run failed. View the logs →
|
Greptile SummaryThis PR introduces disposable macOS guests as a first-class ArcBox surface: pull a pre-baked base image from ArcBox's CDN, copy-on-write clone it, and boot throwaway VMs. It adds
Confidence Score: 5/5Safe to merge. All previously identified races and integrity issues have been addressed; the new code is well-tested and hardware-verified. The RunningSlot placeholder state machine correctly serializes all concurrent operations without per-machine mutexes. The verify-before-decode approach eliminates stale-data-on-retry bugs. Staging-then-rename landing is used consistently. Name validation blocks path traversal at every entry point. The two observations (noisy log warnings from staging dirs, asymmetric not-found behavior on machine remove) are minor style issues that do not affect correctness or safety. No files require special attention. machine.rs and pull.rs contain the most complex concurrent logic and both look correct. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant CLI as arcbox CLI
participant GRPC as MacosService
participant MGR as MacImageManager
participant DSK as disk.rs
participant FS as Filesystem
CLI->>GRPC: ImagePull(reference)
GRPC->>MGR: pull_remote()
MGR->>MGR: resolve_source() fetch index + manifest
MGR->>MGR: pull_lock(name).lock()
MGR->>MGR: validate_hardware_model()
MGR->>MGR: version check (no-op if same)
MGR->>FS: create staging dir
MGR->>DSK: fetch_disk()
loop per chunk bounded parallel
DSK->>DSK: download into memory
DSK->>DSK: verify SHA-256 before decode
DSK->>FS: sparse write at chunk offset
end
MGR->>FS: fetch+verify aux, write meta
MGR->>FS: rename staging to images/name
MGR-->>GRPC: MacImage
GRPC-->>CLI: stream PullEvent
CLI->>GRPC: CreateMachine(name, image)
GRPC->>MGR: MacMachineManager.create()
MGR->>MGR: validate name + image minimums
MGR->>FS: stage in .create-name-uuid
MGR->>FS: clonefile disk
MGR->>FS: rename under write lock
GRPC-->>CLI: Empty
CLI->>GRPC: StartMachine(name)
GRPC->>MGR: start()
MGR->>MGR: insert Starting slot check cap
MGR->>MGR: boot_reserved MacVm::boot
MGR->>MGR: slot Starting to Running
GRPC-->>CLI: Empty
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant CLI as arcbox CLI
participant GRPC as MacosService
participant MGR as MacImageManager
participant DSK as disk.rs
participant FS as Filesystem
CLI->>GRPC: ImagePull(reference)
GRPC->>MGR: pull_remote()
MGR->>MGR: resolve_source() fetch index + manifest
MGR->>MGR: pull_lock(name).lock()
MGR->>MGR: validate_hardware_model()
MGR->>MGR: version check (no-op if same)
MGR->>FS: create staging dir
MGR->>DSK: fetch_disk()
loop per chunk bounded parallel
DSK->>DSK: download into memory
DSK->>DSK: verify SHA-256 before decode
DSK->>FS: sparse write at chunk offset
end
MGR->>FS: fetch+verify aux, write meta
MGR->>FS: rename staging to images/name
MGR-->>GRPC: MacImage
GRPC-->>CLI: stream PullEvent
CLI->>GRPC: CreateMachine(name, image)
GRPC->>MGR: MacMachineManager.create()
MGR->>MGR: validate name + image minimums
MGR->>FS: stage in .create-name-uuid
MGR->>FS: clonefile disk
MGR->>FS: rename under write lock
GRPC-->>CLI: Empty
CLI->>GRPC: StartMachine(name)
GRPC->>MGR: start()
MGR->>MGR: insert Starting slot check cap
MGR->>MGR: boot_reserved MacVm::boot
MGR->>MGR: slot Starting to Running
GRPC-->>CLI: Empty
Reviews (10): Last reviewed commit: "fix(macos): serialize machine lifecycle ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
PeronGH
force-pushed
the
feat/macos-guest-vz
branch
from
46fb489 to
2c868c0
Compare
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — the schema-2 chunked-CDN refactor of the macOS image pull path (commit 892f5c6), which replaces the single-stream zstd download with parallel per-chunk fetch, decode, and sparse-write.
- New
disk.rschunked restore —fetch_diskdownloads chunks with bounded parallelism (JoinSet+Semaphore, concurrency 4), streams each through zstd into a sharedArc<File>via a zero-skippingSparseWriterwriting atbase = index * chunk_size, and verifies each chunk's compressed size, compressed SHA-256, and exact decompressed length. - Manifest schema v2 —
diskbecomes aDiskManifest(uncompressed_size,chunk_size, orderedchunks) inremote.rs;resolve_sourcenow validates that the chunk list exactly tiles the declared disk size (chunks.len() == div_ceil(uncompressed_size, chunk_size), non-empty, non-zerochunk_size). - Removed single-stream path —
fetch_zstd_to_sparseand its byte-range resume are gone;SparseWriter/verify_sha256/hexmoved frompull.rsintodisk.rs. Per-chunk retry re-downloads the whole chunk (no resume). - CDN base moved —
DEFAULT_IMAGE_BASEis nowhttps://image.arcboxcdn.com/darwin. - Tests — local assembly with a zero gap and partial last chunk, wrong-hash rejection, HTTP chunk retry after a mid-body connection drop, sparse-writer hole allocation at 256 MiB, and inconsistent-chunk-count rejection.
The refactor is coherent and well-covered. Retry is idempotent (a failed attempt flushes only full 64 KiB blocks, and truncated zstd output is a strict prefix of the full decode, so re-download rewrites identical bytes), and the parallel positioned writes target disjoint byte ranges so they never race. No stale references to the removed types remain.
ℹ️ Published whole-disk hash is never verified; interior-chunk ordering is unchecked
The manifest carries a whole-disk uncompressed_sha256, but the client never reads it — integrity rests entirely on per-chunk hashing plus placement by array index. Because every interior chunk shares the same expected_len (chunk_size), two interior chunks swapped in the manifest's chunks array would pass all per-chunk checks (compressed hash, decompressed length, zstd frame checksum) yet land a corrupt disk with no error. The producer is trusted and JSON array order is deterministic, so this is low-probability — worth surfacing so the intent behind the published-but-unused field is explicit.
Technical details
# Published whole-disk hash is never verified; interior-chunk ordering is unchecked
## Affected sites
- `app/arcbox-core/src/macos/remote.rs:187-205` — `DiskManifest` doc notes `uncompressed_sha256` "is not consumed here".
- `app/arcbox-core/src/macos/disk.rs:190-232` — `fetch_disk` places chunk `i` at `base = i * chunk_size` purely by iteration index; no chunk-level verification ties the assembled whole to `uncompressed_sha256`.
- `app/arcbox-core/src/macos/pull.rs:141-157` — `resolve_source` validates chunk *count* but not chunk identity/order.
## Required outcome
- Make the trust model explicit and closed: either drop the unused `uncompressed_sha256` from the consumed schema, or wire in a cheap ordering/identity guard so a mis-ordered manifest cannot land silently.
## Suggested approach (optional)
- A whole-disk re-read defeats the perf goal, so avoid it. A cheap alternative: assert each `chunk.path` matches the expected `disk.{i:03}.zst` for its index during `resolve_source`, coupling array position to filename. This catches reordering without a multi-GB read.
- Alternatively, document in `remote.rs` why the field is retained despite being unverified (e.g. for out-of-band/publisher-side auditing), so a future reader doesn't assume it's a live client check.
## Open questions for the human
- Is chunk placement contractually keyed to array order alone, or should the client enforce the `disk.NNN.zst` naming convention as the ordering source of truth?Claude Opus ｜ 𝕏
|
The check suite failed on Step 6: Check formatting (
|
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — the --no-linux-vm VM-host-only mode added in commit 3172451 (the only new commit since the prior pullfrog review at 892f5c6).
- New
--no-linux-vmdaemon flag +[vm] autostartconfig — flag defaultsautostarttofalse, overriding the config file ininit_runtime; documented on both the CLI (app/arcbox-cli/src/commands/daemon.rs) and daemon (app/arcbox-daemon/src/main.rs) argument surfaces. - Core bootstrap gate —
Runtime::initearly-returns before the Linux/Docker system-VM bootstrap whenautostartis false, so no guest binaries download and no VM lifecycle actor starts; the macOS guest manager is untouched. - Daemon service gating — the Docker API server handle becomes
Option(spawned only when the Linux VM runs), and Docker CLI integration, the Kubernetes proxy, and post-startup recovery are all skipped in host-only mode. - Consistent wiring —
ServiceHandles.dockerand the shutdown drain handle the optional Docker task;pipeline.rsthreads alinux_vmbool so the ready banner omits the Docker/K8s lines and prints the host-only notice.
Every gate reads the same post-override runtime.config().vm.autostart, the changes are coherent across startup, services, recovery, and shutdown, and the optional Docker handle is handled correctly on both the drain and abort paths.
Claude Opus ｜ 𝕏
|
Re: the pullfrog note on the unused whole-disk |
The chunked pull decoded compressed bytes straight into disk.img while downloading, so a failed/corrupt attempt mutated the destination before its hash was ever checked. On retry, SparseWriter skips zero blocks, so a prior attempt's divergent non-zero bytes in a now-zero region would survive — silent base-image corruption, with no whole-disk check to catch it (greptile P1 on PR #359). Reorder the flow to verify-then-decode: download each chunk into memory, check exact size + compressed SHA-256, and only then decompress the proven-correct bytes into the sparse file. A rejected attempt never touches disk.img, so retries are safe by construction, and every byte written comes from correct data — sparse-skipping a zero block can never leave stale bytes. No per-attempt special-casing and no multi-GB re-read. - Buffered downloads are bounded by a byte-budget semaphore (1 GiB) that also caps concurrency (≥ MIN_SLOT per chunk → ≤ 8 connections); a runaway response is capped at the manifest size. - Decode runs on spawn_blocking, parallelizing decompression across cores. - New test corrupt_attempt_never_reaches_disk: a complete-but-wrong first response (non-zero where the correct chunk is zero) is rejected in memory and the retry lands the correct disk — fails under the old streaming path. - Document the integrity/trust model on DiskManifest (per-chunk hash + producer chunk ordering; whole-disk uncompressed_sha256 is for out-of-band auditing).
The positional <name> read like 'what you're pulling' (a version), when it's actually a local label for the base image. Move it to --name and free the positional slot for the eventual version|latest selector.
Replaces the IPSW-install ImagePull RPC with pull-by-reference from the published distribution bucket: MacosService.ImagePull is now server- streaming, the CLI takes a positional stream[@Version] reference (or --manifest for development), and image ls shows version/OS provenance.
The product acquires base images via pull only. install_from_ipsw and the Apple IPSW downloader stay compiled under an off-by-default feature so they remain compiler-verified without being product surface. Docs updated to the pull flow, including the corrected shared-machine-identifier note.
HTTP downloads retry with Range: bytes=N- on transient failures — the zstd decoder state lives in memory, so decoding continues where it left off. A staging Drop guard makes the pull future cancellation-safe: dropping it at any await point removes the partial image.
The file-source branch read with std::fs in a loop with no await points, so the pull future ran to completion inside a single poll — immune to select!-based cancellation and pinning a runtime worker throughout. tokio::fs reads yield, verified by killing a pull mid-flight: staging stops growing and the guard removes it within seconds.
Extracts pull_remote's resolution prefix (index resolve, schema/format/ name checks) into a shared helper and adds MacImageManager::resolve_remote on top of it: the concrete version a pull would land, its requirements, host-support validation, and the locally installed version — so a caller (the fleet agent's settings/prepare path, RUN-35 follow-up) can answer "does this fit here" and "is an update pending" without a download.
ImageResolve exposes core's resolve_remote over MacosService, and ImagePull's stream now ends with a terminal "done" event carrying the registered image's summary, so a caller learns the concrete version a floating reference landed. CLI: new `arcbox macos image resolve`, and `image pull` reports the landed name@version.
…races Address the five open code-review findings on the disposable-macOS-guest work: - Validate image/machine names to a single path component at the directory constructors (image_dir/machine_dir are now fallible), so a crafted --manifest name or a create/remove name with separators cannot escape the managed macos/images and macos/machines roots. - Enforce the image's published CPU/memory minimums in create (fail fast). - Assemble a machine in a unique staging dir and rename it into place under the records lock, so concurrent same-name creates can no longer delete each other's cloned disks. Move cloning to MacImage::clone_into and share the StagingGuard RAII helper via the macos module. - Serialize pulls of the same image with a per-name async lock, so concurrent pulls can no longer corrupt the shared staging directory. - Re-insert the running slot when a stop fails, keeping the guest cap and list() accurate while the VZ guest may still hold a framework slot.
The pull consumer was written against a schema-1 monolithic-disk.img.zst manifest, but the published layout is schema-2: the disk is split into fixed-size, independently decompressible zstd chunks (disk.NNN.zst) served under the darwin/ namespace at image.arcboxcdn.com. The old consumer could not even deserialize a live manifest. - Point DEFAULT_IMAGE_BASE at the real namespace root (https://image.arcboxcdn.com/darwin); the darwin.arcboxcdn.com subdomain never existed. - Accept manifest schema_version 2; replace DiskFile with DiskManifest + DiskChunk (chunk_size + per-chunk path/size/sha256). - Validate that the chunk list exactly tiles the declared disk size before downloading anything. - New macos/disk module: bounded-parallel chunk download (JoinSet + Semaphore), streaming decompression into a positioned sparse writer at index*chunk_size, per-chunk compressed-hash + exact-length verification, and whole-chunk retry on transient failure. Deterministic placement plus per-chunk integrity replaces an end-to-end re-read of the assembled disk. Public API (PullStage, RemoteSource, ResolvedImage, pull_remote, resolve_remote) is unchanged; aux restore is unchanged.
Adds a first-class mode that runs the daemon without the default Linux/Docker system VM. Useful for macOS-guest-only workflows and for hosts that don't need Docker/Kubernetes, and it avoids booting a VM that would otherwise sit idle. - config: `[vm] autostart` (default true). When false, Runtime::init skips the entire Linux bootstrap — guest-binary download, validation, and VM boot — so no VM lifecycle actor is ever created. - daemon: `--no-linux-vm` flag (forwarded by the abctl `daemon start` wrapper) overrides `[vm] autostart` to false. - When disabled, the daemon layer skips the Docker API server, Docker CLI integration, the Kubernetes proxy, and container-networking recovery; the ready banner reflects VM-host-only mode. gRPC (incl. macOS guests), DNS, and machine management are unaffected. ServiceHandles.docker becomes Option to reflect the skipped Docker server.
The chunked pull decoded compressed bytes straight into disk.img while downloading, so a failed/corrupt attempt mutated the destination before its hash was ever checked. On retry, SparseWriter skips zero blocks, so a prior attempt's divergent non-zero bytes in a now-zero region would survive — silent base-image corruption, with no whole-disk check to catch it (greptile P1 on PR #359). Reorder the flow to verify-then-decode: download each chunk into memory, check exact size + compressed SHA-256, and only then decompress the proven-correct bytes into the sparse file. A rejected attempt never touches disk.img, so retries are safe by construction, and every byte written comes from correct data — sparse-skipping a zero block can never leave stale bytes. No per-attempt special-casing and no multi-GB re-read. - Buffered downloads are bounded by a byte-budget semaphore (1 GiB) that also caps concurrency (≥ MIN_SLOT per chunk → ≤ 8 connections); a runaway response is capped at the manifest size. - Decode runs on spawn_blocking, parallelizing decompression across cores. - New test corrupt_attempt_never_reaches_disk: a complete-but-wrong first response (non-zero where the correct chunk is zero) is rejected in memory and the retry lands the correct disk — fails under the old streaming path. - Document the integrity/trust model on DiskManifest (per-chunk hash + producer chunk ordering; whole-disk uncompressed_sha256 is for out-of-band auditing).
PeronGH
force-pushed
the
feat/macos-guest-vz
branch
from
517f9c1 to
cbffb05
Compare
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — the verify-then-decode chunk-integrity fix (cbffb055), the only new commit since the prior pullfrog review at 3172451. It reorders the chunked pull so compressed bytes are proven correct in memory before any byte reaches disk.img.
- Verify-then-decode flow —
fetch_verifiedbuffers each chunk fully in memory and checks exact size + compressed SHA-256 beforedecode_intodecompresses the proven-correct bytes into the sparse file. A rejected attempt never touchesdisk.img, soSparseWriter's zero-block skip can no longer leave a prior attempt's stale non-zero bytes behind (the greptile P1). The single-pass streamingChunkSinkis removed. - Byte-budget concurrency — replaces the fixed
DOWNLOAD_CONCURRENCY = 4count with a1 GiBBUFFER_BUDGETsemaphore where each chunk reservesclamp(chunk.size, MIN_SLOT=128 MiB, BUFFER_BUDGET), capping peak buffered memory at ~1 GiB and concurrent connections at 8. The HTTP loop rejects any response exceeding the manifestchunk.size. - Decode off the async runtime — decompression runs on
spawn_blocking; local-file chunks now read viatokio::fs::readthen verify through the sameverify_chunkpath. - Trust model documented —
DiskManifestdoc inremote.rsnow states integrity rests on per-chunk hashing plus producer offset ordering (disk.NNN.zst), and that the whole-diskuncompressed_sha256is intentionally unconsumed (out-of-band auditing). This closes the prior pullfrog informational finding via its documentation option. - New test —
corrupt_attempt_never_reaches_diskdrives the retry path with a complete-but-wrong first response whose decoded bytes are non-zero exactly where the correct chunk is zero; it passes only because verification precedes decode.
The reorder is correct: cost is clamped to BUFFER_BUDGET, so acquire_many_owned can never deadlock; runaway and short responses both fail the size check and retry; and detached spawn_blocking decodes only ever write verified-correct data into the staging file the StagingGuard removes on cancel, so there is no corruption of any surviving file. All prior pullfrog reviews were body-only, leaving no inline threads to retire.
Claude Opus ｜ 𝕏
…ders stop() vacated the machine's runtime slot before awaiting vm.stop(), so a concurrent start could boot a second VM on the same disks while the VZ guest was still live, and stop's failure re-insert could clobber that start's reservation, orphaning a running guest. remove() never reserved the slot at all, so a racing start could boot from files mid-deletion (or resurrect a half-deleted directory via the MAC backfill) and end up as a running ghost with no record. Make the running-slot map the single serialization point for all lifecycle transitions: stop holds a Stopping placeholder for the whole in-flight stop, remove holds a Removing placeholder while it deletes, and start re-validates the record (and does the MAC backfill) only after reserving its Starting slot. In-flight owners re-check their slot before finalizing so stop_all's shutdown sweep is never clobbered. Found by Greptile on PR #359 (both threads); the placeholder design replaces its proposed per-machine mutex so racing callers fail fast with precise states instead of queueing behind a multi-second boot.
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — the machine-lifecycle serialization refactor in app/arcbox-core/src/macos/machine.rs (443dac5f), the only new commit since the prior pullfrog review at cbffb055. It makes the running slot map the single serialization point for start/stop/remove so the per-host guest cap and mutual exclusion are both enforced through slot reservations held across the multi-second boot/stop windows.
- New placeholder slot states —
RunningSlotgainsStoppingandRemovingvariants (alongsideStarting/Running) plus adescribe()helper for uniform "cannot do that now" errors;is_running/state_of/stop_allupdated to account for them. startsplit into reserve + boot —startdoes a fast not-found check, then atomically checks the cap and reservesStarting, then delegates to the newboot_reserved(record re-read, MAC mint,MacVm::boot) which runs entirely under the reservation so a concurrent remove cannot delete files mid-boot; finalization promotesStarting→Runningor releases the slot on error/shutdown-sweep.stopleaves a live-guest placeholder — a running machine transitionsRunning→Stopping(keeping the cap accurate while the VZ guest may still be live), then clears the placeholder on success or restoresRunningon failure; refuses when another op holds the slot.removereserves before touching disk — non-force refuses any held slot, force stops first then claims the freed slot (aborting cleanly if a concurrent start won it); disk+record deletion extracted intodelete_machine, run under aRemovingreservation and released on success and failure alike.- Tests — four new cases cover held-slot refusal across start/stop/remove, reservation release after remove and after a failed start, and placeholder slots counting against the guest cap.
The serialization is coherent: the RwLock write is only ever held briefly for reservation and finalization (never across an await), the slot itself provides mutual exclusion during async work, and every finalize path re-checks its own placeholder with a matches! guard so a stop_all shutdown sweep is never clobbered. Force-remove ordering is airtight — whoever grabs the slot first wins, and the slot stays held across the dangerous delete/boot window, so a concurrent start can never boot from mid-deletion files. No slot leaks on any success or error path, and the new Stopping/Removing states surface safely through the gRPC layer's {:?}-to-string state mapping.
Claude Opus ｜ 𝕏
What
macOS guest VMs on Apple Silicon as a first-class ArcBox noun: pull a pre-baked base image from ArcBox's distribution bucket, copy-on-write clone it, and boot clean throwaway guests — the Linux "clone, use, discard" model extended to macOS. Foundation for the BYOC macOS Actions runner fleet (Linear RUN-36 tree).
Surface
Served by a dedicated
MacosService(own proto, noMachineServicecoupling), registered only on Apple Silicon hosts.ImagePullis server-streaming (stage + fraction events).Design notes
darwinbucket behind darwin.arcboxcdn.com): no OCI client, no registry auth, no Tart traces in this codebase. Format spec + bake pipeline live inmacos-runner-image-builder(the producer; this repo consumes).macos/remote.rs+macos/pull.rs): index/manifest resolution → hardware-model validation before the multi-GB download → socket → zstd → zero-skipping sparse writes (compressed bytes never touch disk; SHA-256 hashed in flight) → staging dir renamed live only after every check passes.Range: bytes=N-against the live decoder state (no temp file). A daemon restart restarts the pull; resume covers network blips, the failure that matters at 20 GB.tx.closed()+ future drop); aDrop-based staging guard cleans partials at any await point.zstd --sparseunder-recovers holes on macOS (verified: 45 GB actual from a 31 GB original); our writer restores bit-identical sparseness.install_from_ipsw+ Apple IPSW download compile under the off-by-defaultmacos-ipsw-installfeature (clippy-clean both configs) with zero product surface.docs/macos-guest.mdcorrected accordingly.Verification
create+startbooted macOS 26.5 to SSH; stop/rm clean. Kill-mid-pull verified: staging removed within seconds.cargo fmt --check,clippy --workspace --all-targetsandclippy --features macos-ipsw-installat zero warnings, full workspace test suite green.Known gaps (tracked)
--manifestpath is fully E2E-verified).