Container Dev Mode: host CLI, embedded registry, and VM push path#184
Open
jetm wants to merge 24 commits into
Open
Container Dev Mode: host CLI, embedded registry, and VM push path#184jetm wants to merge 24 commits into
jetm wants to merge 24 commits into
Conversation
Container Dev Mode is a safety-critical change whose Phase 0 gate must de-risk the core premises before any Phase 1 code ships. The evidence was scattered across an in-session spike and a maintainer's lab, with no durable record of what was actually verified versus attested. Record the Phase 0 decision as GO with explicit provenance: the two-socket separation plus auth matrix (1.11) and the docker arm of the loopback credential path (1.4) were verified in-session with tool output; the remaining hardware spikes (1.1-1.10) are maintainer-attested in-lab. Separating verified from attested keeps the gate honest at this risk tier. Signed-off-by: Javier Tia <javier@peridio.com>
There is currently no foundation for the planned Container Dev Mode feature, which requires an embedded OCI Distribution registry server with TLS and WebSocket support running inside the CLI. Without the necessary dependencies and module structure in place, subsequent tasks implementing config parsing, registry listeners, engine-driver watching, and sync orchestration have nowhere to build on. Introduce the axum, rustls, tokio-rustls, rcgen, and tokio-tungstenite crates as dependencies, all pinned to the aws-lc-rs crypto provider already present via reqwest. This avoids pulling in a second C-based crypto library and keeps the build requirement footprint unchanged. A new container_dev module is added under src/utils as scaffolding, exposing a clear location for the registry server surface and dev loop logic to land in follow-up tasks. Signed-off-by: Javier Tia <javier@peridio.com>
Without a typed configuration structure, the container_dev feature has no way to be selectively enabled per runtime or carry validated parameters such as the watched image list and registry port. Any downstream implementation task would be building on an undefined interface. Introduce ContainerDevConfig as the canonical typed representation of the runtimes.<name>.container_dev YAML block, and wire it into RuntimeConfig as an optional field. Presence of the field enables the feature for that runtime; absence leaves it off. This structural gate is intentional — a container_dev block placed anywhere other than under a runtime is ignored by the parser. The default registry port is set to 5599, explicitly avoiding 5000 which conflicts with the macOS AirPlay Receiver as established during phase 0 task 1.6. The key is also registered with the external-config-ref scanner so the scanner does not recurse into the images list and misinterpret shaped YAML fragments as extension dependency references. Signed-off-by: Javier Tia <javier@peridio.com>
There is currently no entry point for Container Dev Mode in the CLI. Without the command tree in place, the subcommand hierarchy, --help output, and shell completion cannot be exercised or validated before the underlying orchestration logic (host-side registry, engine-driver watcher, device bootstrap) is built. Introduce the `avocado container dev` subcommand family with five verbs: `up`, `sync`, `status`, `down`, and `prune`. Each handler returns a not-yet-implemented error at this stage. This establishes the full dispatch path through the clap enum hierarchy so that integration points and help text can be reviewed independently of the host-side implementation work that follows. Signed-off-by: Javier Tia <javier@peridio.com>
Container Dev Mode needs a local registry to cache OCI blobs and manifest tags between push/pull cycles. Without a dedicated store, there is no safe place to persist layer data across operations, and nothing prevents one project's blobs from polluting another's cache. Introduce a content-addressed BlobStore rooted at `~/.avocado/container-dev/<project>/registry/` that stores blobs under a `blobs/<algorithm>/<hex>` layout matching the OCI image layout spec. Writes are deduplicated by digest so a blob that is already present is never stored a second time, and all writes are atomic via a temp-file-then-rename sequence to avoid partial blobs on crash. Tag pointers live under `manifests/tags/<tag>` and are similarly written atomically. Per-project namespacing is enforced structurally so that GC in one project can never sweep another project's blobs. Digest and tag inputs are validated to reject path-separator and traversal sequences before they can influence filesystem paths. Signed-off-by: Javier Tia <javier@peridio.com>
Without HTTP handlers exposing the content-addressed store built in task 3.1, a device engine has no way to pull images from the embedded registry during a dev-mode iteration. The store exists but is entirely unreachable over the network until the read half of the OCI Distribution protocol is implemented. Introduce the registry read module covering the three endpoints a container engine exercises on a pull: the v2 base check, manifest retrieval by tag or digest, and blob retrieval with Range support. Tags are resolved through the existing BlobStore and manifests are served with the correct media type read from their stored content, ensuring an engine can correctly distinguish a single-platform manifest from a multi-arch image index. Ranged blob fetches return 206 Partial Content with a Content-Range header, matching the resumable-download behavior engines rely on for large layers. HEAD requests are handled transparently by axum's routing without a separate handler. The assembled Router is exported here but intentionally left unbound; it will be mounted onto the dedicated bulk read listener in task 3.7. Signed-off-by: Javier Tia <javier@peridio.com>
Currently the embedded Container Dev Mode registry only serves read requests (blob/manifest GET and partial-content streaming). There is no write path, so a container engine cannot push images to the registry, making the dev-loop push-pull cycle impossible. Add a host-only write token credential type (HTTP Basic, fixed username, password equal to the write token) and a separate write router covering the full OCI push surface: monolithic and chunked blob upload (POST/PATCH/PUT on `.../blobs/uploads/...`), manifest PUT, and the push-side blob dedup HEAD. The write router is gated entirely behind a middleware that validates the Basic credential and rejects anything else — including a Bearer read/control token with the same secret value — before any handler is reached. This enforces the design constraint that a device, which only ever receives a Bearer read/control token, cannot reach a write route regardless of topology. The write router is intentionally kept on a distinct router object to be bound onto a separate listener by later tasks (3.6/3.7); it is never merged onto the bulk read listener. Signed-off-by: Javier Tia <javier@peridio.com>
Previously the auth module only implemented the write-side Basic validator. The read listener and the future control-WebSocket upgrade (task 5.1) had no auth surface defined, meaning any future binding of the read routes would have been open or would have required a separate, independently implemented credential check — risking the two surfaces diverging (design concern G-5). Introduce a single authorization seam, `read_request_authorized`, that both the bulk read listener middleware (`require_bearer_read`) and the control-WS upgrade handler will call. Because both surfaces funnel through the same predicate the WS upgrade cannot accidentally implement a looser or different check. The validator enforces scheme separation (M-2): a Basic write credential is rejected on a read route by construction, since `Bearer` and `Basic` are distinct schemes. The `read_router` public constructor is updated to accept a `ReadToken` and apply the gate, while the internal `read_routes` assembly remains ungated so existing handler-semantics tests are not burdened with auth noise. Signed-off-by: Javier Tia <javier@peridio.com>
Without a sweep path, the per-project blob store accumulates layers indefinitely. Orphaned blobs left behind by retagged or abandoned pushes are never reclaimed, growing disk usage without bound. There is also no protection against a `prune` command racing with a device actively pulling layers, which could sweep a blob the pull still needs and leave the device with a broken image. Introduce an explicit GC path (`collect_garbage`) that walks all currently-tagged manifests, follows their references transitively (including multi-arch indices to their sub-manifests), and removes any blob on disk that is not reachable from a live tag. GC is deliberately invoked only from `prune` and `down`, never implicitly during a push or on a timer, to keep the sweep boundary predictable. To guard against the mid-pull race, a reference-counted `PullGuard` RAII type is added; `begin_pull` increments a shared atomic counter and the guard decrements it on drop. `prune` checks the counter and returns `PruneWhilePulling` if any pull is in flight, while `collect_garbage` (used by `down`, which tears down listeners first) bypasses that check unconditionally. Signed-off-by: Javier Tia <javier@peridio.com>
Container Dev Mode needs mutually-authenticated TLS between the host registry and the device, but previously had no mechanism to generate per-project certificates or session tokens. Without this, the bulk-read and control-WS listeners have no credential material to bind against, and the bootstrap payload delivered to the device has nothing to pin the host with. Introduce the tls module (task 3.6) which mints, in a single operation, a per-project CA and a CA-signed server leaf whose SANs cover the runtime name, the QEMU guest-to-host alias (10.0.2.2), and the loopback address. The notBefore is backdated to year 2000 so that RTC-less devices that cold-boot at the Unix epoch or their firmware build date still fall inside the validity window. The CA private key is consumed during leaf signing and deliberately never stored as a struct field, making it impossible for it to reach any serialized payload. The device receives only the CA certificate and the read/control Bearer token via BootstrapPayload; the host-only Basic write token and the CA key are structurally excluded from that type. Both tokens carry 256 bits of randomness and rotate independently on every mint. Signed-off-by: Javier Tia <javier@peridio.com>
The OCI read router previously had no dedicated bound socket, meaning bulk blob transfers had no guaranteed separation from the control WebSocket channel. Without this separation, a large layer pull could head-of-line-block control frames on the same byte stream, violating the three-listener isolation model required by the session design. Introduce BulkListener, a self-contained handle that binds a TCP socket, wraps it with TLS termination using the per-session leaf certificate, and serves the token-gated OCI read router exclusively on that socket. The TLS layer is handled by a custom axum Listener implementation that absorbs transient accept errors with a brief back-off and silently drops failed handshakes, keeping the server loop running without surfacing per-connection noise. Because bulk reads live on their own socket, devices handed only this endpoint cannot reach the write listener, and no blob body can arrive as a WebSocket frame. Dropping the BulkListener handle aborts the backing task, so the socket lifetime is tied directly to the session lifecycle. Signed-off-by: Javier Tia <javier@peridio.com>
The Container Dev Mode watcher needs to observe the host container
engine for image tag events so it can trigger layer sync to the device
on rebuild. Without a well-defined engine abstraction, both the event
stream logic and credential injection would need to be duplicated or
entangled across Docker and Podman code paths.
Introduce an EngineDriver trait that captures everything engine-specific
about watching for tag events and injecting push credentials. Two
concrete drivers are provided: DockerDriver, which drives `docker events
--format {{json .}}` and injects credentials via an ephemeral
DOCKER_CONFIG directory, and PodmanDriver, which drives `podman events
--format json` and injects credentials via per-invocation `--creds`.
Both drivers communicate exclusively through the engine CLI subprocess,
never through the API socket, so a rootless Podman installation without
a running `podman.socket` works correctly. The engine-agnostic
`forward_tag_events` and `watch_tag_events` helpers drive whichever
engine driver they receive, meaning the watcher orchestration and push
wiring added in subsequent tasks reuse this plumbing unchanged.
Signed-off-by: Javier Tia <javier@peridio.com>
Currently there is no mechanism to detect image rebuild events, select the correct transfer strategy for the host topology, and notify the device after layers are synced. Without this, the container dev-mode loop has no way to react to a `docker build` or `podman build` and propagate the result to the embedded registry on the device. Introduce the watcher module that consumes tag events from the engine driver, debounces rapid rebuilds to a single sync of the latest tag, and supersedes any in-flight transfer when a newer event arrives. The sync strategy is chosen by explicit host-topology detection rather than emergent behavior: native Linux and the avocado-vm take a delta PUSH into the embedded registry, while Docker Desktop and podman-machine without the VM take a full-image INGEST export as the only reachable fallback. The notifier and syncer are expressed as seams so the control WebSocket (task 5.1) and the concrete engine transfer can be wired in later without coupling this module to either. Signed-off-by: Javier Tia <javier@peridio.com>
Syncing a container image built for one CPU architecture to a device running a different architecture results in a silent wrong-arch delivery. The container engine on the device may fail to run the image, or in the case of a multi-arch manifest, pull silently but never execute correctly. There was no check to catch this before the image was pushed or the device was notified. Introduce an architecture guard decorator that sits in the sync path and probes the image's platform architecture before delegating to the real syncer. The guard compares the image's canonical GOARCH architecture against every connected device's reported architecture, sourced from their `hello` control frames. A single mismatched device refuses the entire sync with an actionable error that names the device's target platform and provides the correct `docker buildx build --platform` invocation. Because the guard returns an error before the wrapped syncer runs, neither the push nor the device notification is ever triggered for a mismatched image. An arch normalization layer reconciles `uname -m` spellings (e.g. `aarch64`, `x86_64`) with GOARCH spellings (e.g. `arm64`, `amd64`) so comparisons are reliable regardless of which convention the source uses. Signed-off-by: Javier Tia <javier@peridio.com>
Without a control channel, the host has no way to push image availability notifications to connected devices or reconcile a device that reconnects with a stale running digest. Bulk transfers are handled by a dedicated HTTPS listener, but there is no signalling path to tell a device when and what to pull, nor to detect that a device came back out of sync. Introduce a control-only WebSocket server that exchanges lightweight control frames between host and device. The host sends a single Sync frame carrying image coordinates and a digest reference; the device responds with Hello, Progress, and Status frames. Blob bytes never travel this channel by construction: HostFrame has no variant that could carry them, making an accidental bulk transfer a compile-time error rather than a runtime constraint. Two invariants are load-bearing. Desired state is re-derived entirely from the engine's current watched tags at every up, with no disk-restore path, so a digest that changed while the host was down is always reflected rather than silently restored from a stale snapshot. On each device reconnect the host compares the reported running digest against the desired state and issues reconcile Sync frames for every entry that does not match, driving the device back to current without manual intervention. Authentication reuses the shared read/control-token validator that the bulk listener's middleware already calls. The WebSocket upgrade is an HTTP GET carrying the same Authorization header, so the upgrade callback delegates directly to that function rather than implementing a separate auth surface. ControlServer also implements the watcher's Notifier seam, broadcasting a Sync frame to every connected device when the watcher reports a new tag and updating the desired state so later reconciles compare against the freshly pushed digest. Signed-off-by: Javier Tia <javier@peridio.com>
Previously the `avocado container dev up`, `down`, and `status` subcommands were unimplemented stubs that returned a not-yet-implemented error. The command tree, `--help`, and completion wiring could be exercised, but no actual dev session could be started, monitored, or stopped. This left the Container Dev Mode feature entirely inoperable. Implement the full `up`/`down`/`status` lifecycle. `up` mints fresh TLS material and both session tokens, binds a dedicated bulk read listener on all interfaces and a loopback-only write listener on a separate port, starts the engine-driver watcher and control WebSocket, auto-detects the reachable host IP against the target device (with `AVOCADO_CONTAINER_DEV_HOST`/`PORT` overrides), delivers the bootstrap payload once over SSH, and then runs in the foreground until SIGINT or SIGTERM. `down` signals the foreground `up` process to shut down via SIGTERM and clears the session state file. `status` reads that same state file and surfaces a re-bootstrap warning when any device has presented a stale token. The accompanying `bootstrap` module provides the testable primitives these commands depend on. `DeviceBootstrap` is structurally constrained to carry only the bulk endpoint, the read/control token, and the CA certificate — there is no field for the write token or write-listener address, so neither can ever appear in a serialized payload delivered to a device. `WriteListenerGuard` runs its teardown closure from `Drop`, ensuring the routable write listener is torn down on any exit path, clean or unclean. `TokenRegistry` keeps a rotated-out read token valid until its in-flight bulk connections drain to zero or a hard ceiling elapses, rather than using a fixed timer that would issue a mid-stream 401 to an in-flight image pull on a slow link. Stale tokens surface as `TokenStatus::NeedsReBootstrap` in `DevStatus` rather than silently retrying. Signed-off-by: Javier Tia <javier@peridio.com>
Previously, `container dev sync` and `container dev prune` both exited immediately with a "not implemented yet" error. Users had no way to manually trigger a one-shot re-push of a watched image to a connected device, nor any way to reclaim disk space used by unreferenced blobs in the per-project store. Implement `sync` as a signal-based trigger: a separate `sync` invocation sends SIGUSR1 to the running `up` process, which drives one pass of the same push-then-notify pipeline the file watcher uses on every rebuild. This reuses the live session's registry write listener, engine syncer, and control WebSocket, so the notification reaches the device without requiring additional SSH connections. If no active `up` session exists, `sync` reports this clearly rather than silently doing nothing. A failed re-push surfaces as an error and suppresses the device notification, preserving the invariant that a device is never told an image is ready when the push did not land. Implement `prune` as a thin wrapper over the existing per-project store GC policy: it sweeps blobs unreferenced by any currently-tagged manifest, refuses to run while a device is mid-pull to avoid sweeping a blob a pull still needs, and deliberately leaves the session token and CA material untouched since those live outside the store's registry tree. Signed-off-by: Javier Tia <javier@peridio.com>
The control WebSocket accepted plain TCP and ran the WebSocket upgrade directly on the raw stream, so the host<->device control channel carried sync frames and the reconcile handshake in cleartext. The design binds the control channel to the same per-project pinned-CA TLS the bulk listener uses (D8/D9), and the device agent dials wss:// pinning the session CA -- against a plaintext listener that connection cannot complete and the pinned-CA guarantee does not hold. Make the connection-handling core generic over the transport and add a serve_tls entry that handshakes each accepted stream with the session leaf before the upgrade, mirroring the bulk listener's TlsListener. The plain-TCP serve entry is gated cfg(test) so no production path can bind the control WS in cleartext, and the shared read/control-token validator seam is preserved rather than duplicated per transport. Signed-off-by: Javier Tia <javier@peridio.com>
The container dev mode registry and control WebSocket expose two distinct authentication surfaces: a Bearer-gated TLS bulk read listener and a Basic-gated plain-HTTP write listener. Without interface-level tests exercising the real listeners, regressions that accidentally serve unauthenticated requests or accept the wrong credential type on the wrong interface would go undetected until runtime. Add a dedicated integration test file that spins up live listeners from a real DevSession and drives them with a pinned-CA client, asserting that each auth gate rejects exactly the requests it must reject. The suite covers five failure modes: unauthenticated reads and WS upgrades being served, unauthenticated writes succeeding on either interface, the Bearer read token being honored on write routes (compromised-device scenario), a wrong-password Basic credential being accepted on a write route, and the Basic write token being accepted on a read route. Each test is written as a falsifier so that any gate removal turns a passing assertion into a failure. Signed-off-by: Javier Tia <javier@peridio.com>
Without integration-level coverage, the arch guard introduced in task 4.3 could regress silently: a future change might accidentally allow a mismatched-arch image to pass through to a device that cannot run it, and no test would catch it. Unit tests on individual functions are insufficient because they do not verify that the guard, the device-arch book, and the syncer compose correctly under realistic call patterns. Add a dedicated integration test file that exercises the real guard types (`ArchGuardSyncer`, `HelloArchBook`, `check_arch`, `ArchMismatch`, `DeviceArch`) end-to-end using only two narrow test doubles: a fixed `ImageArchProbe` standing in for an engine `image inspect`, and a `ShipRecorder` that counts actual sync calls. This pairing makes refusals and passes concretely observable: a refusal is proven by an `ArchMismatch` error AND a ship count of zero, while a pass is proven by a ship count of exactly one. The suite covers the full decision matrix: single-device mismatch, single-device match, mixed fleet where one mismatched device refuses the whole sync, homogeneous matching fleet, and the canonical `uname`/GOARCH spelling equivalence between `aarch64` and `arm64` that must not trigger a spurious refusal. Signed-off-by: Javier Tia <javier@peridio.com>
Previously the control WebSocket listener was bound on an ephemeral port (`0.0.0.0:0`), meaning the assigned port was unknown at bootstrap time. The device agent receives its connection parameters once, at bootstrap delivery, so a port that is only discovered after binding can never be communicated to the device. This made the control channel unreachable in practice. Bind the control WS on a fixed, configurable port (default 5600, overridable via `AVOCADO_CONTAINER_DEV_WS_PORT`) and include its device-reachable address as a first-class `ws_endpoint` field in `DeviceBootstrap`. The endpoint is resolved using the same device-reachable host as the bulk listener, keeping the two endpoints symmetric and consistent with the existing host-resolution logic. The control-WS listener remains structurally distinct from both the bulk read listener and the write listener; the write-listener address is still never disclosed to a device, and the bootstrap payload still carries no field for it. Signed-off-by: Javier Tia <javier@peridio.com>
The container dev mode write listener served plain HTTP, which worked for native Linux because Docker's built-in loopback exemption (127.0.0.0/8) allows insecure connections. However, when the container engine runs inside an avocado-vm guest, it reaches the host listener through the QEMU SLIRP alias 10.0.2.2, which is not inside Docker's trusted loopback range. The guest daemon therefore requires HTTPS, and before this change any push from an avocado-vm engine would fail at the transport layer. A second problem existed even when the transport was corrected: axum's default 2 MiB body limit caused any real image layer upload to be rejected with 413 mid-stream, making the write path unusable for non-trivial images regardless of topology. Introduce topology detection that selects either the native loopback plain-HTTP path or the avocado-vm authenticated HTTPS path at `up` time. On the VM path the write listener terminates the same per-project leaf TLS as the bulk and control listeners, bound to a known port so the guest's certs.d trust directory and the pushed image tag can both be keyed on the identical 10.0.2.2:<port> address. The per-project CA is delivered into the guest engine's docker trust store over SSH at `up`, never baked into the VM overlay, satisfying the per-connection delivery requirement. The write path is authenticated with a host-only Basic write token; the device-delivered read token is never accepted on a write route. Lift the body limit on the write router so large image layers are accepted. Add a verification script and tests covering each falsifiable property of the VM write path.
Without integration-level tests, the two core properties of the container dev mode sync loop have no falsifiable coverage: that a one-line change transfers only the changed layer (not the whole image), and that a device reporting a stale digest receives a sync frame over the control WebSocket prompting it to pull and restart. Add an end-to-end test suite that exercises both properties against the real listeners a device talks to in production. The delta-pull test drives the write listener and bulk TLS listener over a shared content-addressed store, verifying that byte-identical blobs are never re-transferred after a top-layer change. The control-WS test dials the TLS-upgraded WebSocket with a pinned session CA and a Bearer token, sends a Hello carrying the stale running digest, and asserts that the host responds with a sync frame carrying the new digest. Together these tests catch regressions in the store deduplication logic and the reconciliation path without requiring a real device or container runtime. Signed-off-by: Javier Tia <javier@peridio.com>
The authenticated VM write path (task 7.1) was validated against a local QEMU engine VM, but only the verify script was committed - the harness that provisions that VM lived as untracked local scratch, so the end-to-end path could not be reproduced from a clean checkout. Commit the provisioner (idempotent Debian 12 + docker + ssh engine VM under QEMU SLIRP, guest reachable at 10.0.2.2 like the macOS avocado-vm), its example container_dev config, and a README. Generated state (the qcow2 overlay, ssh key, cloud-init seed, env.sh) is kept in a work dir outside the repo so a ~900 MB overlay never lands in git; only the base image download is a manual one-time step. Signed-off-by: Javier Tia <javier@peridio.com>
jetm
commented
Jul 24, 2026
jetm
left a comment
jetm
left a comment
Contributor
Author
There was a problem hiding this comment.
Self-review (cold read of my own +10k PR). The security core is clean: TLS drops the CA key (bootstrap carries only CA cert + read token), tokens are 256-bit CSPRNG, the store closes digest/tag path-traversal, every docker/podman call is argv-only (no shell), and the bind addresses match the threat model. Six correctness/robustness findings inline - the two worth acting on first are the recycled-PID signal hazard in down/sync and the cross-arch guard never being wired into the live up path (confirm whether that's intentionally deferred - if so it shouldn't read as complete).
| /// Signal the recorded `up` process to shut down (SIGTERM), driving its graceful | ||
| /// teardown (and, on any unclean exit, its [`WriteListenerGuard`]). | ||
| #[cfg(unix)] | ||
| fn signal_shutdown(pid: u32) { |
Contributor
Author
There was a problem hiding this comment.
Stale session.json PID -> down/sync signals an arbitrary process.
up removes session.json only on the graceful path (remove_file at ~425, after wait_for_shutdown, not in a Drop guard), so a panic or SIGKILL leaves the file with a dead PID. down then libc::kill(pid, SIGTERM) (~688) and sync libc::kill(pid, SIGUSR1) (~748) with no liveness/ownership check. If the OS recycles that PID to an unrelated process, sync delivers SIGUSR1 (default disposition: terminate) to an innocent process, killing it; down has the same hazard with SIGTERM. The "stale pid yields ESRCH" assumption only holds when the PID is truly gone, not recycled. Record a start-time/owner and verify it before signalling, or remove session.json from a Drop/finally guard.
| .parent() | ||
| .expect("store root has a per-project parent") | ||
| .to_path_buf(); | ||
| let syncer = Arc::new(EngineSyncer::new( |
Contributor
Author
There was a problem hiding this comment.
Cross-arch guard is never wired into the live up path.
up hands EngineSyncer straight to run_watcher unwrapped; ArchGuardSyncer/EngineArchProbe (the whole watcher::arch_guard module) is never instantiated or imported here, and the HelloArchBook ControlServer populates from hello.arch is never read. So on an amd64 host targeting an aarch64 device, container dev up rebuilds and pushes an amd64 image and notifies the device to pull it - the exact silent wrong-arch delivery the guard exists to refuse - while tests/container_dev_arch.rs and the guard's own unit tests pass. The module carries a blanket #[allow(dead_code)] and mod.rs says wiring lands "later", so this may be intentionally deferred - if so, it shouldn't be presented as complete; if not, wire the guard before the live push.
|
|
||
| // Record the running session (with this process's pid) so `status`/`down` | ||
| // in a separate invocation can find and signal it. | ||
| let state = SessionState { |
Contributor
Author
There was a problem hiding this comment.
container dev status is static and can never surface a stale device; drain-based token rotation is unwired.
SessionState.status.devices is written once as Vec::new() and never updated, and TokenRegistry (drain-based rotation + TokenStatus, D5/H-2) is fully implemented and tested in bootstrap.rs but never constructed in the live path. registry_running/watcher_running/last_sync are frozen at up-time. So a device presenting a rotated-out token after a re-up never surfaces via status.needs_rebootstrap() (hard-coded false on empty devices), and container dev status reports a clean static state. Same deferred-wiring caveat as the arch guard - confirm whether this is intentionally not-yet-live.
| /// receives a `401` with a Basic challenge. This router is bound onto the | ||
| /// DISTINCT write listener (design D9); it is never merged onto the bulk read | ||
| /// listener. | ||
| pub fn write_router(store: Arc<BlobStore>, write_token: WriteToken) -> Router { |
Contributor
Author
There was a problem hiding this comment.
Write router disables the body limit and never GCs abandoned upload sessions.
DefaultBodyLimit::disable() plus buffering bodies as Bytes/Vec<u8>; a POST .../blobs/uploads/ that opens a session and PATCHes chunks but never PUTs leaves its buffer in the UploadSessions map forever (no timeout/eviction). Repeated interrupted pushes grow host memory unbounded across a long up session. Low security impact (loopback-only + Basic-token-gated, auth runs before buffering), but a real memory-growth path - add a session TTL/eviction and a sane body cap.
|
|
||
| /// `HEAD /v2/<name>/blobs/<digest>` — the push-side dedup probe: `200` when the | ||
| /// blob already exists so the engine skips re-uploading it, else `404`. | ||
| async fn head_route(State(state): State<WriteState>, Path(rest): Path<String>) -> Response { |
Contributor
Author
There was a problem hiding this comment.
HEAD dedup probe reads the entire blob into memory just to report Content-Length.
head_route calls state.store.read_blob(digest) and uses only bytes.len(); the store has no size/stat path, so the whole (potentially hundreds-of-MB) layer is read into RAM and discarded. A docker push HEADs every layer for dedup before uploading, so each probe reads a full existing layer into memory on the hot push path. Add a stat/size path to the store and use it here. Efficiency, not correctness.
| //! CA certificate. Steady-state sync then rides the control WS with no further | ||
| //! SSH (design D5). | ||
| //! | ||
| //! `down` stops all listeners AND tears down the routable write listener + its |
Contributor
Author
There was a problem hiding this comment.
Comment drift: docs describe a 0.0.0.0 write forward this code doesn't create.
Several comments and the WriteListenerGuard doc claim down tears down "the routable write listener and its 0.0.0.0 forward", but the write listener is bound 127.0.0.1 only (~232) with no 0.0.0.0 forward anywhere in this PR. No runtime bug (the code is safer than the comment), but it misleads a reviewer about the write listener's exposure - update the comment to match the loopback-only bind.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Container Dev Mode is the inner dev loop for a containerized app on an
immutable Avocado OS device: edit on the host, and only the changed image
layer is hot-reloaded onto the running device — no reflash, no full re-ship.
This adds the host half:
a control WebSocket (three-listener model), all over a per-project pinned CA;
selects PUSH vs INGEST by host topology, plus a cross-arch guard;
avocado container dev up/sync/status/down/prune;a routable HTTPS write listener whose per-project CA is delivered at
up,never baked.
Verification: unit + integration suites (
container_devlib, pluscontainer_dev_e2e,container_dev_security,container_dev_arch) all pass;the VM write path was validated end-to-end 8/8 against a local QEMU docker+ssh
engine VM. Phase 0 de-risk findings are recorded in
docs/container-dev/phase0-findings.md.