Skip to content

fix: make OCI staging capsules reproducible and fail-closed - #1070

Open
Koh0920 wants to merge 8 commits into
devfrom
fix/oci-multi-runtime-fallback
Open

fix: make OCI staging capsules reproducible and fail-closed#1070
Koh0920 wants to merge 8 commits into
devfrom
fix/oci-multi-runtime-fallback

Conversation

@Koh0920

@Koh0920 Koh0920 commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Summary

  • select a ready Podman or Docker-compatible provider for OCI service graphs
  • honor materialized ato.lock.json OCI resolutions after registry install
  • resolve digest-pinned emulated images consistently
  • enforce empty egress allowlists with no-default-route/internal networks

Verification

  • cargo check -p cli
  • provider selection targeted tests
  • empty egress network targeted test
  • installed lock precedence targeted test
  • digest-pinned emulation targeted test
  • staging install and repeated launch of koh0920/ossm-vol1-lab 0.1.2

Note

On this macOS host, Podman Machine and Docker Desktop did not forward a host port from the fail-closed network. Container readiness, non-root execution, persistence, and denied egress were verified; browser UI was verified through an SSH tunnel into the Podman VM.

@Koh0920
Koh0920 marked this pull request as ready for review July 14, 2026 22:36
@Koh0920

Koh0920 commented Jul 25, 2026

Copy link
Copy Markdown
Contributor Author

Automated review (Claude Code)

Verdict: changes-requested

What it does
Rewires the multi-service OCI executor (oci_multi_service.rs) to (a) pick whichever of Podman/Docker-compat is actually ready instead of hard-coding PodmanProvider, (b) re-read <workspace>/ato.lock.json from disk and treat it as authoritative over the in-memory plan.lock, gated by a new manifest cross-check, and (c) create the session network as an "internal" network (Docker internal: true / Podman --opt no_default_route=1) whenever egress_allow is empty. It also lets a digest-pinned single-arch image resolve without an explicit platform when allow_emulation = true, and carries four unrelated snapshot/guest-agent commits (Firecracker snapshot-create read timeout, devpts mounts in three init scripts).

Findings

blocker — crates/cli/src/adapters/runtime/executors/oci_multi_service.rs:156: the new authoritative-lock reader rejects the documented on-disk lock shape.
validate_workspace_lock_matches_manifest reads the disk file through resolve_oci_image_for_target, which deserializes each entry into OciImageResolution (crates/capsule/src/foundation/types/oci.rs:17) where platform is an object {os, architecture}. But the on-disk writer for that same JSON key — upsert_oci_lock_facts/write_oci_facts_to_main_lock, crates/capsule/src/contract/ato_lock/oci.rs:354,383 — serializes OciImageLockEntry, whose platform is a String (oci.rs:24), and the governing ADR documents exactly that: "platform": "linux/amd64" (crates/nacelle/docs/adr/ADR-oci-lock-integration.md §5, both schema blocks). The compose and install.sh runners read it back via parse_platform_str (oci_compose_runner.rs:303, install_sh_runner.rs:323) because it is a string.
Failure: any ADR-conformant ato.lock.jsonresolution.oci_images.<target> is invalid: invalid type: string "linux/amd64", expected struct OciPlatform, propagated by the ? at line 156, so authoritative_workspace_lock aborts the whole launch. The documented compat fallback at line 282 (None => resolve from declared ref at run time) becomes unreachable. The new tests can't catch this because the fixture at oci_multi_service.rs:2107 writes the object shape, not the ADR shape. Note docs/rfcs/draft/OCI_PROVIDER_LOCK_IDENTITY_SPEC.md §3.4 shows the object shape — the two documents already disagree; this PR is the change that turns that latent split into a hard launch failure, so it needs to pick one and normalize on read.

blocker — oci_multi_service.rs:646 + :1906-1911: fail-closed network breaks the ingress the readiness probe depends on, on every launch.
policy_mode is hard-coded OciPolicyMode::Strict at line 235, and enforce_multi_service_policy_gate (line 1803) rejects any non-empty egress_allow. So egress_allow is always empty on a launch that gets past the gate → the else branch at line 649 is dead in production and every multi-service OCI launch now takes create_internal_network. run_readiness_probe probes the published host port (http://127.0.0.1:{host_port}{path}, line 1909; TCP variant line 1927). The PR's own verification note says "Podman Machine and Docker Desktop did not forward a host port from the fail-closed network" and that the UI was checked through an SSH tunnel. That is the readiness path failing, for the default configuration, on the primary dev platform. It also directly contradicts the code comment at oci_provider.rs:1336 ("keeps host-published ingress available"). Either gate this behind a flag or demonstrate host-port publishing works on the internal network for both providers.

major — crates/cli/src/adapters/runtime/oci_provider.rs:1169-1180: fabricated platform violates the spec and lands in the identity hash.
For a digest-pinned single-arch manifest with no requested_platform, under AllowEmulation the code invents linux/amd64. OCI_PROVIDER_LOCK_IDENTITY_SPEC.md §3.8 is explicit: "the ref is usable provided a requested_platform is explicitly supplied. Without an explicit platform there is no reliable way to record the platform; resolution fails." The invented value flows through into_lock_resolution() (oci_multi_service.rs:340) into OciServiceLaunchShape.imageOciLaunchEnvelope (crates/capsule/src/foundation/types/oci.rs:159) → the execution-identity hash input (crates/capsule/src/engine/execution_identity/mod.rs:2208). So flipping allow_emulation in capsule.toml changes a recorded "content-addressed" platform for a byte-identical image. It is also simply wrong for an arm64-only single-arch image: the lock records amd64 and create_container is handed platform: linux/amd64 (oci_multi_service.rs:860). The mutable-tag branch below already solves this correctly by pulling and inspecting — do the same here instead of asserting.

major — oci_multi_service.rs:185-198: new host-arch gate hard-fails cross-arch locks with no fallback.
When allow_emulation is false — the default (crates/capsule/src/foundation/types/manifest.rs:1977) — the lock's recorded architecture must equal std::env::consts::ARCH. A v1 lock is single-platform by design; ADR §7 says the remedy is "Move to linux/amd64 host, relock to get correct platform". So a registry-installed capsule locked on an arm64 laptop will refuse to launch on an amd64 runner (and vice versa) — exactly the staging/runner case this PR is aimed at. The launch aborts rather than falling back to live re-resolution, and the error tells the user to edit a capsule.toml they got from the registry.

major — oci_multi_service.rs:116 and :139-144: hard requirements on lock sections other code paths in this repo don't write.
contract.metadata is required unconditionally, but write_oci_facts_to_main_lock starts from AtoLock::default() when the file is absent (crates/capsule/src/contract/ato_lock/oci.rs:391-395), producing a lock with resolution.oci_images and no contract.metadata. resolution.oci_images is likewise required whenever any ato.lock.json exists — a lock carrying only source-side resolution (e.g. persist_provider_authoritative_lock, provider_target.rs:1497) now aborts instead of falling through to the compat path. Both should degrade to the fallback lock, not abort. Related nit: the [targets] check at line 135-138 says "when ato.lock contains OCI resolutions" but runs before oci_images is read.

minor — oci_provider.rs:1343 vs crates/capsule/src/engine/runtime/oci.rs:324: one trait method, two different security postures.
Podman gets --opt no_default_route=1; Docker gets internal: true. These are not equivalent. no_default_route only omits the container's default route — the bridge subnet, netavark's NAT rules and aardvark-dns on the gateway IP remain reachable, so DNS resolution (and DNS-based exfiltration) still works, and a process with CAP_NET_ADMIN can re-add the route. The trait doc at oci.rs:249 ("denying external routing") describes the stronger Docker semantics. Also no_default_route is netavark-only; on older Podman the whole network create fails with an opaque CLI error and no version probe (fails closed, but unhelpfully).

minor — the egress change covers one of five OCI lanes. orchestrator.rs:346 still calls plain create_network; oci_compose_runner, install_sh_runner, oci_single_target are untouched. Relatedly, create_internal_network is not overridden in impl OciRuntimeClient for PodmanProvider (oci_provider.rs:1608) or for RuntimeOciProvider (:2665), so those inherit the always-Err default from crates/capsule/src/engine/runtime/oci.rs:250. Harmless today (nothing calls it through that trait) but it's a trap.

minor — crates/cli/src/application/runtime_setup.rs:1084: the env-lock fix is partial. Taking crate::tests::env_lock() only coordinates with tests using that mutex. The CLI crate has at least nine independent env_lock() definitions (adapters/preview/tests.rs:16, adapters/registry/publish/artifact.rs:924, adapters/runtime/overrides.rs:122, application/source_inference/mod.rs:4368, cli/commands/logs.rs:147, cli/dispatch/run.rs:594, community/prompt.rs:105, application/engine/manager/locks.rs:60). Any of those that sets ATO_HOME still races.

minor — scope. Four of eight files (snapshot/firecracker.rs, snapshot/rootfs_builder.rs, snapshot/docker_import/multi_image.rs, guest-agent/supervisor.rs) are unrelated snapshot/devpts work and are the sole cause of the nightly conflict below. The devpts additions also change the init script baked into every built rootfs, which invalidates previously built rootfs/warmup artifacts — fine, but it deserves its own PR and its own rebuild note rather than riding an OCI-lock fix. The Firecracker timeout change itself looks correct and well-reasoned (FC_SNAPSHOT_CREATE_READ_TIMEOUT, no retry of an in-flight request).

Staleness/mergeability
Base is dev, which is a strict ancestor of nightly (0 ahead, 54 behind) — so the branch is 0 behind its own base but 54 behind nightly, and per CLAUDE.md feature branches should target nightly. GitHub reports MERGEABLE / BLOCKED against dev. Retargeting to nightly does not apply cleanly: git merge-tree origin/nightly origin/fix/oci-multi-runtime-fallback conflicts in crates/snapshot/src/firecracker.rs (that file diverged by ~731 lines between dev and nightly); oci_multi_service.rs and rootfs_builder.rs auto-merge. Nightly has none of these fixes yet (grep FC_SNAPSHOT_CREATE_READ_TIMEOUT and devpts both empty on origin/nightly), so the work is still wanted — but it needs a rebase onto nightly with a manual resolution in firecracker.rs, and splitting the snapshot commits out would remove the conflict entirely. Worth remembering the house caveat: nightly PRs get no workspace Rust CI, and the new lock tests all use a fixture in the non-ADR shape, so green here proves very little about finding #1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant