Repository navigation
cmux vm token: one macOS lock per user; revoke removes an unreadable token - #29719
Conversation
…able token (#29705, #29706) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-75b7a03d1a2084cf16af
… unreadable token (#29705, #29706) On macOS the token is one login Keychain item per user, so the store lock now follows the account (its home from the user database), not HOME or XDG_CONFIG_HOME. A stored token that cannot be read is removed by `token revoke` (its key cannot be revoked without it and expires within its TTL), and the error for it says so. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-75b7a03d1a2084cf16af
|
cmux status (one comment, edited in place; each line is updated by the job that owns it) Review App of this head (app.zip, kept 1 day)
red: cmux-next checks (god files, concurrency, crash safety, l10n); base red: the same failure on the base 4510f25; does not block Shipped in:
|
…s home Review finding: falling back to HOME/XDG_CONFIG_HOME when the user database has no home would split the lock again. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-75b7a03d1a2084cf16af
|
Review (codex gpt-6.1-sol xhigh, next-review): BLOCK at 70bcc7a BLOCK at 70bcc7a Visual verdict: no images; Leo tries the build
Cargo tests could not run because the box has no Rust toolchain. |
|
Review (codex gpt-6-astra high fast): SHIP at 70bcc7a SHIP Findings No [blocking] or [follow-up] findings. The earlier fallback issue is fixed: macOS lock creation fails closed when the user database has no absolute home and never consults Unreadable-token removal rechecks under the shared store lock, while readable-token revocation rechecks the key before deleting local state. An earlier review at 257bad7 blocked on the macOS lock falling back to HOME/XDG_CONFIG_HOME without a user-database home; 70bcc7a makes it fail closed, and this review is at that head. Green compile: Linux CI run 41245035670 (cmux-vm tests and clippy, cmux-tui cli::vm) and cmux-tui artifacts aarch64/x86_64-apple-darwin builds at the head. |
lifecycle.rs: both sides added tests at the end (revoke_removes_an_unreadable_stored_token here, #29722's kept-run archive tests on base); both are kept. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-75b7a03d1a2084cf16af
|
Found 8 test failures on Blacksmith runners: Failures
|
|
Review (codex gpt-6-astra high fast): SHIP at 00dd1a0 SHIP Findings: No verified blocking or follow-up defects. The macOS lock fails closed without an account home, unreadable-token removal is lock-protected and revalidated, readable revocation remains server-first, callers and base archive/CLI changes integrate correctly, and both locked workspaces resolve metadata successfully. Verification: reviewed both merge diffs and callers; This head merges feat-cmux-next into 70bcc7a (SHIP, no findings); the one conflict, tests/lifecycle.rs, keeps both sides' new tests. Tests at this head (cmux-ci run 41260858997): Not this PR's red: |
Bugbot needs on-demand usage enabledBugbot uses usage-based billing for this team and requires on-demand usage to be enabled. A team admin can enable on-demand usage in the Cursor dashboard. |
A newer verdict blocks 9d9c1cc (no independent SHIP at the head).
|
Review (codex gpt-6-astra high fast; carried by patch-id): SHIP at 9d9c1cc This head is cmux-bot's base merge of 00dd1a0, reviewed SHIP there (issuecomment-6552119419). The PR's change against its base has the same |
|
Review (codex gpt-6-astra high fast; carried by patch-id): SHIP at ca989b5 This head is cmux-bot's base merge of 9d9c1cc (itself a base merge of the SHIP-reviewed 00dd1a0, issuecomment-6552119419). The PR's change against its base still has |
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
Two follow-ups from the review of #27603, both in the
cmux vm tokenstore (workers/cmux-vm/cli/crates/cmux-vm/src/token.rs, dispatch inlib.rs).#29705, one lock per user for the macOS Keychain item. The issue: "On macOS the token lives in one login Keychain item per user (fixed service and account), so two creates run with different
XDG_CONFIG_HOMEorHOMEvalues take different locks, can both see an empty store, both mint, and one overwrites the other's stored key, leaving that key live with nothing able to revoke it." On macOS the store lock now follows the account:<home>/.config/cmux/vm-token.lock, with the home from the user database (getpwuid(geteuid())throughnix's safeUser::from_uid; the crate forbidsunsafe). In the usual environment (HOME is the account's home, no XDG_CONFIG_HOME) that is the same file as before, so an older cmux running at the same time still shares it. With no user-database entry (or a home that is not absolute) the macOS lock fails closed with an error instead of falling back to the environment, which would split the lock again. Linux and other platforms keep the lock beside the file store: that store's file itself follows XDG_CONFIG_HOME or HOME, so another environment is another store and nothing is overwritten.#29706, an unreadable stored token names a recovery that works. The issue: "When the stored VM token's JSON cannot be read, the error recommends
cmux vm token revoke. Buttoken revokeloads the same stored token before it runs, so it fails with the same error." Nowtoken revokeon a store holding something that is not a token (bad JSON, not text) removes that local copy, under the store lock and only while it is still unreadable, and says its key was not revoked and expires within its TTL (at most 24h), which deletes its VMs.--jsonprints{"id": null, "revoked": false, "removed": true}. The error now says exactly that. A store that cannot be read at all (Keychain refusal, a file other users can read) is still an error; the world-readable-file error no longer suggests a revoke that would fail, and says to chmod 600 it first.Adds
nix0.31 (featureuser, macOS only) to cmux-vm; cmux-tui's lock already has nix 0.31.3.Fixes #29705
Fixes #29706
Changelog
cmux vm token createtakes one lock per macOS user, andcmux vm token revokeremoves a stored token it cannot read instead of failing.Tests
cargo test --locked -p cmux-vm --test lifecycle revoke_removes_an_unreadable_stored_token(inworkers/cmux-vm/cli): fails at the test commit 5b12a6e, passes at the head.cargo test --locked -p cmux-vm --lib the_lock_follows_the_account_not_the_environment: does not compile at the test commit (no account-keyed lock existed), passes at the head.two_environments_of_one_user_share_the_macos_lockruns on macOS only.Observed / Not run
Observed on Linux CI (run 41245035670, head 70bcc7a): at the test commit the integration test FAILED (exit 101) and the unit test did not compile (exit 101); at the head both pass,
cargo test --locked -p cmux-vmpasses (45 unit, 83 lifecycle and the rest),cargo clippy --locked -p cmux-vm --all-targets -- -D warningspasses, andcargo test --locked -p cmux-tui cli::vmpasses (25). The macOS code compiles in the PR'scmux-tui artifacts / build aarch64-apple-darwinandx86_64-apple-darwinjobs (green). Not run: the macOS-only test on a Mac, and a live Keychain (a cross-targetcargo checkon Linux had no macOS std installed).No capture: CLI only, no UI change.
🤖 Generated with Claude Code
Agent session: agent session