cmux-host, devbox: guest-side vm-kind writers keep the record the create wrote

Merged
TE
10d ago
+699-49

What

The two guest-side writers of /etc/cmux/vm-kind no longer replace the record the Cloud backend writes at create (#26179):

  • The devbox image's cmux-vm-kind-sync wrote team before every fetch of /api/vm/reflection/kind. A failed fetch therefore left an owner VM marked team. It now writes only a kind it fetched, and otherwise leaves the record as it is.
    • The boot's snapshot-clone check relied on that pre-write to reset a clone's carried-over record. It now asks for the reset explicitly with CMUX_VM_KIND_RESET=team.
    • Every write holds a lock (/etc/cmux/vm-kind.lock, flock), and the lock file holds a reset generation. A reset bumps it. A fetched answer is written only when no reset happened since its fetch began. So a sync already fetching when a clone resets (started earlier in the same boot tick, or carried in from the source's snapshot) cannot put back the old answer. The next sync writes the clone's own answer.
    • The lock covers only the write, never the fetch, so it's held briefly. A reset and a sync each wait at most 10 s for it. A sync that can't take it writes nothing.
    • A reset that can't take the lock, or can't write, exits non-zero. The boot checks that and runs CMUX_VM_KIND_RESET=team-unlocked, which renames team and a new generation into place without the lock, logs it, and goes on to start the daemon. So a stuck lock holder never keeps a clone on its source's record or holds up its boot.
    • The fallback writes the new generation and lock file first, then team. A sync that was already past its generation check when the fallback ran can still rename its answer afterwards. So after its rename, every sync checks again that its lock fd is still the lock file's inode ([ /dev/fd/9 -ef LOCK ]) and that the generation is unchanged, and if not it renames team back. Either ordering ends team.
    • The generation file is read whole. Valid is 1 to 9 digits with at most one trailing newline, matched with case, not by stripping characters. The parsed text must also account for every byte of the file, because command substitution drops NUL bytes. Anything else is corrupt: a sync writes nothing, and a reset replaces it with a fresh value.
    • A fetched answer must be exactly owner or team. A body with several JSON values, which jq turns into several lines, writes nothing.
    • Every write is a rename, the locked reset's generation included. Nothing is truncated or written in place, so a lockless reader never sees a half-written generation, and a planted symlink is replaced rather than written through.
  • cmux-host's record_owner_kind (cloud/client.rs, called at bind and at every agent restart from cloud/role.rs) wrote owner over anything but team. It now keeps any existing record and writes owner only when there is none and no team enroll or binding exists.
    • The check is a stat (Store::present). Any error other than "not found" means a record may be there, so nothing is written and the error is logged. A record it can't read as text (invalid UTF-8) is still a record and is kept.
    • The write is atomic and conditional (Store::create_new): a finished 0644 temp file is linked into place, and link() fails if any entry now has that name. So a record the sync or the create writes between the check and the write is kept.
    • The temp file is held by a guard that removes it on drop. So no failure path leaves a .tmp-* file beside the record: the write, permissions, rename and link steps alike. A failed cleanup is logged.
  • DEVBOX_AWAITING_REBAKE (web/scripts/devbox-image-common.ts) moves the desktop sources' current digest to 08eaadaaff2d2bae9b06f0ab85925fcbbfc8b35eea8da89088b58ea92842b0ee. It's recorded the same way earlier unbaked source changes were (#24270, #24491, #25269). No image is baked or promoted here; the promotion that bakes these sources deletes the entry.
  • cloud/wire.rs's VM_KIND_FILE doc says the record is informational, acpmux doesn't trust it (#26227), and the owner write never replaces a record.
  • /api/vm/reflection/kind's comment no longer says the sync writes team before asking. main's copy of that comment is already accurate ("a 503 leaves the guest's record as it was"), so main needs no change.

This was split from #26179 (now landed) to keep that PR under 800 lines.

The record is informational: it shows which kind cmux assigned. Informational only; acpmux no longer trusts it (remote_sandbox.rs, hb-acpmux; #26227). A later re-bake of the devbox image ships the sync change. Until then the backend write in #26179 is what fresh VMs get.

Credit: the record and the vmHost default it drives come from #25749. Related: #25778 (the "Dedicated agent machine" switch) and #24594 (cmux-vm-kind-sync).

Tests

  • web/tests/vm-devbox-vm-kind-sync.test.ts (new) covers:

    • a failed fetch keeps an existing owner record;
    • a failed fetch writes nothing where there is no record;
    • an answer still replaces the record;
    • CMUX_VM_KIND_RESET=team writes team without fetching, and the boot script uses it;
    • a clone reset that lands while a sync is fetching (the fake curl runs the reset before it answers owner) leaves team, and the next sync writes owner;
    • with the lock held (the test waits until the holder has it), a reset waits and then writes team. A sync that can't take the lock meanwhile writes nothing.
    • with a holder that never lets go, the boot's reset (mark_vm_kind_shared, run from the boot script) ends within its bound and logs the fallback. The record is team, the generation is new, and the daemon-start step is reached.
    • when the reset can't take the lock (flock fails) or can't write (the first mktemp fails), the boot falls back to team and goes on.
    • a 32-digit generation is corrupt: a sync writes nothing. The boot's reset writes team and a valid generation, and then a sync writes its answer.
    • a sync held between its check and its rename (a PATH shim on mv waits for a file) while the fallback runs ends team. If the hold times out, the shim fails instead of renaming, so a slow runner fails the test rather than hiding a regression.
    • generation files 1\n2, 7 \n, 7\n\n, 7\r\n, 7\0 and \0 are corrupt. Empty, 7, 7\n and 007\n are valid.
    • an answer of two JSON values writes nothing.

    Without the fix, 3 of the first 4 fail. Without the generation check, the interleaving test fails. Without the reset's bound, the never-released test fails (the boot hangs). Without the boot's fallback, it and the flock/mktemp test fail. Without the nine-digit limit, the corrupt-generation test fails. Without the post-rename check, the held-sync test fails. With the old tr -d '\n' read, the generation-file test fails. Without the one-value check, the two-values test fails.

  • cmux-host cloud::tests:

    • owner_kind_is_written_once_and_never_replaces_a_record: writes owner 0644 with no record; keeps team, owner, empty and unknown records, mode included; writes nothing with a team enroll on disk; a check that errors on the record or on the team enroll writes nothing.
    • owner_kind_keeps_a_record_it_cannot_read (real FileStore): an invalid-UTF-8 record is kept byte for byte; a path that can't be checked returns an error and writes nothing.
    • owner_kind_never_replaces_a_record_written_after_its_check (real FileStore): a team record written right after the check stays team, with no temp file left; with nothing there it writes owner 0644.
    • a_failed_write_leaves_no_temp_file (real FileStore): it injects a failure at each of write, permissions and rename (write), and write, permissions and link (create_new). Each leaves no .tmp-* file and no record.

    Writing with the old rename instead of the link fails the race test. Checking with read().is_some() instead of the stat fails 2 of the 3. Without the guard's cleanup, the temp-file test and the race test fail.

Linux runs:

  • vm-image-manifest (23) and devbox:manifest:check, vm-devbox-vm-kind-sync (12, three runs in a row), vm-devbox-identity (18), vm-devbox-agent-auth (40), vm-devbox-image (57) and vm-kind-route (5) pass, as do sh -n on both scripts, tsgo and eslint.
  • cargo test -p cmux-host --lib cloud:: passes 16/16, with clippy -D warnings and fmt clean.

What I checked in cmux-vm-kind-sync

I read it once as a skeptical reviewer, looking for any write or parse that could leave the wrong value:

  • Writes: the record, the generation (locked reset, lockless fallback) and a sync's answer are all renames of a finished 0644 temp file (mv -fT, so a directory there fails rather than being written into). The temp file is removed by a trap on any exit the shell sees.
  • Lockless paths: the fallback, and the sync's first generation read before its fetch. The fallback is ordered generation, then team. A sync's write is followed by the post-check above. The early read only decides whether a later locked check can match, and a rename can't be read half-written.
  • Parses:
    • the generation file: exact match and byte count, as above;
    • the answer: jq -e plus the exact owner/team match;
    • model-plane.env: sourced, as before. It's root-owned image data and not changed here.
  • Exit paths:
    • under set -e, a failure in the reset exits non-zero, and the boot falls back;
    • a sync that can't take the lock or reads a corrupt generation writes nothing;
    • a missing lock file reads as generation 0.
  • Known limit: a sync's post-check can put team back just after a later sync legitimately wrote owner. That errs toward team, and the next sync (60 s) writes the fetched answer again.

Risk: low. Both writers now do less: they never replace a record they did not fetch. The sync takes a short lock only around its write, never around the fetch, for at most 10 s. A clone's boot falls back to writing team without the lock rather than waiting on it. flock is from util-linux, which the image already needs (cmux-tui-state-mount).

Agent session: agent session

šŸ¤– Generated with Claude Code