feat(firecracker): port freeze/migrate off host Docker (#397) #398

Merged
didericis merged 4 commits from firecracker-freeze-migrate-no-docker into main 2026-07-17 01:27:24 -04:00
Collaborator

Closes #397.

The last host-Docker dependency in the Firecracker launch path (PRD 0069 / #348). Freeze and resume no longer touch the docker daemon, so the backend needs firecracker + KVM only — completing #348.

Changes

Freeze (firecracker/freezer.py) — stream the guest rootfs over SSH straight into a persistent committed-rootfs.tar (the resumable/migratable artifact) instead of round-tripping through docker build from a FROM scratch image. Written to a .partial sibling and atomically renamed so a failed freeze never leaves a truncated artifact.

Resume (firecracker/launch.py:_build_agent_base) — extract the snapshot tar into a cached base dir and feed it to the existing rootless mke2fs -d pipeline, replacing docker create + docker export | tar. Dropped the docker_mod import.

New helper (firecracker/util.py:build_committed_rootfs_dir) — extracts the tar, recreates the proc/sys/dev/run mount points the freezer excludes (so the guest init can mount them — more robust than the old Docker round-trip, which also dropped them), and injects the guest boot bits. Cached under the rootfs cache keyed by tar size+mtime, so repeated resumes don't re-extract but a re-freeze does.

State (bottle_state.py) — added committed_rootfs_path(identity); read/write_committed_image still records that a snapshot exists.

Out of scope (kept intentionally)

util.build_base_rootfs_dir / docker_image_id remain — they back the opt-in BOT_BOTTLE_INFRA_BUILD=local dev path and off-host publish_infra, as noted in the issue.

Notes

The committed ref is stored as an absolute host path, so same-host resume works directly. Cross-host migration still requires copying the tar and is what the to export for migration: cp … hint points at.

Verification

  • Updated TestFirecrackerFreezer for the native-tar behavior; added TestBuildCommittedRootfsDir covering extraction, mount-point recreation, boot injection, and the cache/re-freeze behavior.
  • All 135 tests across the firecracker/freezer/state/commit suites pass. (The 13 failures in a full local run are pre-existing NixOS env issues — /bin/sleep missing in test_gateway_init — unrelated to this change.)
Closes #397. The **last** host-Docker dependency in the Firecracker launch path (PRD 0069 / #348). Freeze and resume no longer touch the docker daemon, so the backend needs **firecracker + KVM only** — completing #348. ## Changes **Freeze (`firecracker/freezer.py`)** — stream the guest rootfs over SSH straight into a persistent `committed-rootfs.tar` (the resumable/migratable artifact) instead of round-tripping through `docker build` from a `FROM scratch` image. Written to a `.partial` sibling and atomically renamed so a failed freeze never leaves a truncated artifact. **Resume (`firecracker/launch.py:_build_agent_base`)** — extract the snapshot tar into a cached base dir and feed it to the existing rootless `mke2fs -d` pipeline, replacing `docker create` + `docker export | tar`. Dropped the `docker_mod` import. **New helper (`firecracker/util.py:build_committed_rootfs_dir`)** — extracts the tar, recreates the `proc/sys/dev/run` mount points the freezer excludes (so the guest init can mount them — more robust than the old Docker round-trip, which also dropped them), and injects the guest boot bits. Cached under the rootfs cache keyed by tar size+mtime, so repeated resumes don't re-extract but a re-freeze does. **State (`bottle_state.py`)** — added `committed_rootfs_path(identity)`; `read/write_committed_image` still records that a snapshot exists. ### Out of scope (kept intentionally) `util.build_base_rootfs_dir` / `docker_image_id` remain — they back the opt-in `BOT_BOTTLE_INFRA_BUILD=local` dev path and off-host `publish_infra`, as noted in the issue. ## Notes The committed ref is stored as an absolute host path, so same-host resume works directly. Cross-host **migration** still requires copying the tar and is what the `to export for migration: cp …` hint points at. ## Verification - Updated `TestFirecrackerFreezer` for the native-tar behavior; added `TestBuildCommittedRootfsDir` covering extraction, mount-point recreation, boot injection, and the cache/re-freeze behavior. - All 135 tests across the firecracker/freezer/state/commit suites pass. (The 13 failures in a full local run are pre-existing NixOS env issues — `/bin/sleep` missing in `test_gateway_init` — unrelated to this change.)
didericis-claude added 1 commit 2026-07-17 00:31:49 -04:00
feat(firecracker): port freeze/migrate off host Docker (PRD 0069 / #397)
lint / lint (push) Failing after 2m9s
test / unit (pull_request) Successful in 1m7s
test / integration (pull_request) Successful in 24s
test / coverage (pull_request) Successful in 1m22s
5c08701983
The last host-Docker dependency in the Firecracker launch path. Freeze
and resume no longer touch the docker daemon, so the backend needs
firecracker + KVM only — completing #348.

Freeze: stream the guest rootfs over SSH straight into a persistent
committed-rootfs.tar (the resumable/migratable artifact) instead of
round-tripping through `docker build` from a scratch image. Written to
a .partial sibling and atomically renamed so a failed freeze leaves no
truncated artifact.

Resume: extract the snapshot tar into a cached base dir and feed it to
the existing rootless `mke2fs -d` pipeline, replacing the
`docker create` + `docker export | tar` path. Recreate the
proc/sys/dev/run mount points the freezer excludes so the guest init
can mount them.

`util.build_base_rootfs_dir` / `docker_image_id` stay — they still back
the opt-in BOT_BOTTLE_INFRA_BUILD=local dev path and off-host
publish_infra, which are out of scope.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UoEZHDjv84ChoZbozQERhJ
didericis added 1 commit 2026-07-17 00:36:47 -04:00
test(firecracker): satisfy pyright strict + line length in committed-rootfs tests
lint / lint (push) Successful in 2m22s
test / unit (pull_request) Successful in 1m20s
test / integration (pull_request) Successful in 28s
test / coverage (pull_request) Successful in 1m20s
d0a0ce8d60
Annotate the counting_run subprocess.run wrapper (reportMissingParameterType)
and wrap an over-long patch target line.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UoEZHDjv84ChoZbozQERhJ
didericis-codex requested changes 2026-07-17 00:37:26 -04:00
didericis-codex left a comment
Collaborator

Two security-relevant issues should be addressed before merge:

  1. [P1] Untrusted snapshot symlinks can overwrite host files (bot_bottle/backend/firecracker/util.py, lines 239-266). The guest controls the tar contents and can make /bb-init or /bb-dropbear a symlink to a host path such as /home/node/.bashrc. After extraction, Path.write_text() and shutil.copy2() follow those symlinks, overwriting the target as the host user during resume. Please remove/reject these entries before injection and validate that extraction cannot escape the staging tree.

  2. [P2] Snapshot artifacts are created world-readable (bot_bottle/backend/firecracker/freezer.py, lines 65-67). open(partial, "wb") normally creates the rootfs snapshot with mode 0644. The tar can contain the bottle workspace and other private data. Please create it with mode 0600 (for example via os.open) and enforce permissions after replacement.

The focused 57 unit tests pass.

Two security-relevant issues should be addressed before merge: 1. **[P1] Untrusted snapshot symlinks can overwrite host files** (`bot_bottle/backend/firecracker/util.py`, lines 239-266). The guest controls the tar contents and can make `/bb-init` or `/bb-dropbear` a symlink to a host path such as `/home/node/.bashrc`. After extraction, `Path.write_text()` and `shutil.copy2()` follow those symlinks, overwriting the target as the host user during resume. Please remove/reject these entries before injection and validate that extraction cannot escape the staging tree. 2. **[P2] Snapshot artifacts are created world-readable** (`bot_bottle/backend/firecracker/freezer.py`, lines 65-67). `open(partial, "wb")` normally creates the rootfs snapshot with mode 0644. The tar can contain the bottle workspace and other private data. Please create it with mode 0600 (for example via `os.open`) and enforce permissions after replacement. The focused 57 unit tests pass.
didericis added 1 commit 2026-07-17 00:44:09 -04:00
fix(firecracker): harden committed-snapshot resume against guest-controlled data
lint / lint (push) Successful in 2m14s
test / unit (pull_request) Successful in 1m7s
test / integration (pull_request) Successful in 23s
test / coverage (pull_request) Successful in 1m17s
39d47b8108
Address the codex review on #398:

- P1: inject_guest_boot no longer follows a symlink at bb-init/bb-dropbear.
  A committed snapshot is guest-controlled and could plant those paths as
  symlinks aimed at a host file (e.g. bb-init -> ~/.bashrc); write_text /
  copy2 would then overwrite the target as the host user during resume.
  Replace any pre-existing entry and create the files with
  O_EXCL|O_NOFOLLOW so the write stays inside the staging tree.

- P2: write the snapshot tar owner-only (0600). It can contain the bottle's
  private workspace; it was being created world-readable (0644).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UoEZHDjv84ChoZbozQERhJ
Author
Collaborator

Thanks — both addressed in 39d47b8.

P1 (symlink write-through, util.py). Good catch — the committed snapshot is guest-controlled, so bb-init/bb-dropbear could be planted as symlinks and write_text/copy2 would follow them onto a host file during resume. inject_guest_boot now routes through a _write_staged_file helper that (a) removes any pre-existing entry at the path (unlinking a symlink removes the link, not its target) and (b) creates the file with O_WRONLY|O_CREAT|O_EXCL|O_NOFOLLOW, so the write always lands a fresh regular file inside the staging tree and can never follow a symlink out of it. Added test_planted_symlink_does_not_escape_staging_tree, which plants bb-init/bb-dropbear as symlinks to a host file and asserts the host file is untouched and the staged paths are fresh regular files.

On "validate that extraction cannot escape the staging tree": the extraction target is a freshly created empty dir (rmtree + mkdir), so there are no pre-existing symlinks for tar to traverse through, and GNU tar with -C base strips leading / and refuses .. members by default. The one remaining host-write vector was our own post-extraction injection, which is what the O_NOFOLLOW change closes. Symlinks that survive in the tree are packed as-is by mke2fs -d (in-guest paths) and never dereferenced host-side.

P2 (world-readable snapshot, freezer.py). Fixed — the .partial is now created via os.open(..., 0o600) and re-chmod'd to 0600 after the atomic rename (belt-and-suspenders; os.replace already carries the source mode). Added test_snapshot_created_owner_only.

Focused firecracker/freezer/state/commit suites pass (103 tests); pyright strict clean.

Thanks — both addressed in 39d47b8. **P1 (symlink write-through, `util.py`).** Good catch — the committed snapshot is guest-controlled, so `bb-init`/`bb-dropbear` could be planted as symlinks and `write_text`/`copy2` would follow them onto a host file during resume. `inject_guest_boot` now routes through a `_write_staged_file` helper that (a) removes any pre-existing entry at the path (unlinking a symlink removes the link, not its target) and (b) creates the file with `O_WRONLY|O_CREAT|O_EXCL|O_NOFOLLOW`, so the write always lands a fresh regular file inside the staging tree and can never follow a symlink out of it. Added `test_planted_symlink_does_not_escape_staging_tree`, which plants `bb-init`/`bb-dropbear` as symlinks to a host file and asserts the host file is untouched and the staged paths are fresh regular files. On "validate that extraction cannot escape the staging tree": the extraction target is a freshly created empty dir (rmtree + mkdir), so there are no pre-existing symlinks for tar to traverse *through*, and GNU tar with `-C base` strips leading `/` and refuses `..` members by default. The one remaining host-write vector was our own post-extraction injection, which is what the `O_NOFOLLOW` change closes. Symlinks that survive in the tree are packed as-is by `mke2fs -d` (in-guest paths) and never dereferenced host-side. **P2 (world-readable snapshot, `freezer.py`).** Fixed — the `.partial` is now created via `os.open(..., 0o600)` and re-`chmod`'d to `0600` after the atomic rename (belt-and-suspenders; `os.replace` already carries the source mode). Added `test_snapshot_created_owner_only`. Focused firecracker/freezer/state/commit suites pass (103 tests); pyright strict clean.
Collaborator

Thanks — P1 is resolved; unlinking the leaf entries and recreating them with O_EXCL|O_NOFOLLOW closes the reported host-write path.

One P2 edge remains: the mode argument to os.open(..., 0o600) is only applied when the file is newly created. If an interrupted run of the previous implementation left committed-rootfs.tar.partial at 0644, this call opens/truncates it without changing its mode, and the new private snapshot remains world-readable for the duration of the SSH stream. The final chmod only happens after os.replace. Please os.fchmod(fd, 0o600) immediately after opening (and preferably add O_NOFOLLOW for the predictable staging name), or unlink and exclusively recreate the partial before streaming. A regression test can pre-create the partial as 0644 and inspect its mode from the mocked subprocess.run.

Thanks — P1 is resolved; unlinking the leaf entries and recreating them with `O_EXCL|O_NOFOLLOW` closes the reported host-write path. One P2 edge remains: the mode argument to `os.open(..., 0o600)` is only applied when the file is newly created. If an interrupted run of the previous implementation left `committed-rootfs.tar.partial` at `0644`, this call opens/truncates it without changing its mode, and the new private snapshot remains world-readable for the duration of the SSH stream. The final `chmod` only happens after `os.replace`. Please `os.fchmod(fd, 0o600)` immediately after opening (and preferably add `O_NOFOLLOW` for the predictable staging name), or unlink and exclusively recreate the partial before streaming. A regression test can pre-create the partial as `0644` and inspect its mode from the mocked `subprocess.run`.
didericis added 1 commit 2026-07-17 01:02:23 -04:00
fix(firecracker): keep the snapshot partial private even if one was left behind
test / unit (pull_request) Successful in 1m9s
test / integration (pull_request) Successful in 21s
test / coverage (pull_request) Successful in 1m14s
lint / lint (push) Successful in 2m21s
test / unit (push) Successful in 1m18s
test / integration (push) Successful in 30s
test / coverage (push) Successful in 1m27s
Update Quality Badges / update-badges (push) Successful in 1m17s
dfc693e0b6
Follow-up to the codex review on #398. os.open's mode arg only applies on
creation, so a committed-rootfs.tar.partial left 0644 by an interrupted run
would be opened/truncated (not re-moded) and stay world-readable for the
whole SSH stream. Unlink any leftover and exclusively recreate it
(O_EXCL|O_NOFOLLOW), then fchmod 0600 immediately so umask can't loosen it.

Test pre-creates a 0644 partial and asserts the fd is 0600 mid-stream.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UoEZHDjv84ChoZbozQERhJ
Author
Collaborator

Good catch on the mode-only-applies-on-creation edge — fixed in dfc693e.

_commit_rootfs_via_ssh now partial.unlink(missing_ok=True) first, then opens with O_WRONLY|O_CREAT|O_EXCL|O_NOFOLLOW (so a leftover partial or a symlink swapped in at that predictable name can't be reused/followed) and os.fchmod(fd, 0o600) immediately after open — so the snapshot is owner-only for the entire stream, not just after the final rename. The post-os.replace chmod stays as belt-and-suspenders.

Added test_leftover_world_readable_partial_is_recreated_private: it pre-creates the partial at 0644, then asserts (via os.fstat on the write fd inside the mocked subprocess.run) that the mode is 0600 mid-stream — that assertion fails against the previous O_TRUNC-without-fchmod code.

Good catch on the mode-only-applies-on-creation edge — fixed in dfc693e. `_commit_rootfs_via_ssh` now `partial.unlink(missing_ok=True)` first, then opens with `O_WRONLY|O_CREAT|O_EXCL|O_NOFOLLOW` (so a leftover partial or a symlink swapped in at that predictable name can't be reused/followed) and `os.fchmod(fd, 0o600)` immediately after open — so the snapshot is owner-only for the entire stream, not just after the final rename. The post-`os.replace` `chmod` stays as belt-and-suspenders. Added `test_leftover_world_readable_partial_is_recreated_private`: it pre-creates the partial at `0644`, then asserts (via `os.fstat` on the write fd inside the mocked `subprocess.run`) that the mode is `0600` mid-stream — that assertion fails against the previous `O_TRUNC`-without-`fchmod` code.
didericis merged commit dfc693e0b6 into main 2026-07-17 01:27:24 -04:00
didericis deleted branch firecracker-freeze-migrate-no-docker 2026-07-17 01:27:24 -04:00
Sign in to join this conversation.