Repository navigation
cmux-host: verify fresh enrolled identity for Cloud triggers - #30362
Merged
Merged
Conversation
Contributor
|
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 2a2aa0c; does not block Shipped in:
|
teamleaderleo
temporarily deployed
to
ci-routing-agent
November 1, 2026 07:04 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
8 issues found and verified against the latest diff
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="docs/cloud-trigger-identity.md">
<violation number="1" location="docs/cloud-trigger-identity.md:24">
P2: A bind can return success without writing this receipt, but the documentation presents the write as guaranteed. Say the host attempts the write and that identity queries fail closed when it cannot save the receipt, or make receipt failure fail the bind.</violation>
<violation number="2" location="docs/cloud-trigger-identity.md:26">
P3: The doc names the receipt and socket paths but not `bound.json`. Add its path, `/var/lib/cmux/bound.json`, where the requirement is stated so operators can check it.</violation>
</file>
<file name="cmux-tui/crates/cmux-host/src/identity/tests.rs">
<violation number="1" location="cmux-tui/crates/cmux-host/src/identity/tests.rs:49">
P3: The new tests never cover the `receipt.machine != bound.machine` branch in `enrolled()`. Add a case that rewrites `bound.json` with a different `machine` while keeping the receipt, and assert `enrolled` fails. Also cover an empty `bound.machine` and an empty `bound.install`.</violation>
</file>
<file name="cmux-tui/crates/cmux-host/src/agent.rs">
<violation number="1" location="cmux-tui/crates/cmux-host/src/agent.rs:199">
P2: Marking `Wake::Identity` as a clone signal fences the machine on any failed metadata read during a query, not only after a clone. That clears `identity_settled()` and stops the periodic announce loop until a later successful read. If retries run out first, it stays stopped until another wake. Confirm this fencing is intended, or drop the `clone_signal` flag for identity observations and keep the refusal in the reply path.</violation>
</file>
<file name="cmux-tui/crates/cmux-host/src/cloud/client.rs">
<violation number="1" location="cmux-tui/crates/cmux-host/src/cloud/client.rs:218">
P2: A receipt-write failure consumes `bind.json` and returns `Bound`, but no later path retries `identity::record`, so a transient filesystem error permanently leaves the new identity IPC unavailable. Preserve a durable retry or attempt receipt creation when resuming an otherwise bound guest.</violation>
<violation number="2" location="cmux-tui/crates/cmux-host/src/cloud/client.rs:219">
P3: The receipt-failure branch in `bind_machine` has no test. Add a case where the install key file is unreadable or the receipt write fails, and assert that bind still returns `Bound`, `bind.json` is removed, and the receipt is gone.</violation>
</file>
<file name="cmux-tui/crates/cmux-host/src/cli.rs">
<violation number="1" location="cmux-tui/crates/cmux-host/src/cli.rs:362">
P3: The module doc at the top of this file still lists only `cloud daemon-info | probe-activity`. Add `cloud identity` to that list so it matches USAGE and the new branch.</violation>
<violation number="2" location="cmux-tui/crates/cmux-host/src/cli.rs:371">
P3: An unreachable identity socket (host not running) exits 1 (internal). `cmux host status` uses exit 3 (not found) for the same condition. Map the not-running case to code(3) so callers can tell it apart from an internal failure.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| Wake::Identity => { | ||
| observe = true; | ||
| // Failure must fence an inherited identity, just like a clone signal. | ||
| clone_signal = true; |
There was a problem hiding this comment.
P2: Marking Wake::Identity as a clone signal fences the machine on any failed metadata read during a query, not only after a clone. That clears identity_settled() and stops the periodic announce loop until a later successful read. If retries run out first, it stays stopped until another wake. Confirm this fencing is intended, or drop the clone_signal flag for identity observations and keep the refusal in the reply path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At cmux-tui/crates/cmux-host/src/agent.rs, line 199:
<comment>Marking `Wake::Identity` as a clone signal fences the machine on any failed metadata read during a query, not only after a clone. That clears `identity_settled()` and stops the periodic announce loop until a later successful read. If retries run out first, it stays stopped until another wake. Confirm this fencing is intended, or drop the `clone_signal` flag for identity observations and keep the refusal in the reply path.</comment>
<file context>
@@ -184,6 +193,11 @@ impl<P: Platform> Agent<P> {
+ Wake::Identity => {
+ observe = true;
+ // Failure must fence an inherited identity, just like a clone signal.
+ clone_signal = true;
+ }
Wake::ClockSet => {
</file context>
Comment on lines
+24
to
+25
| A successful bind writes `/var/lib/cmux/install/identity.json`, a public receipt | ||
| of the machine, instance and install-key fingerprint, mode 0600. The query |
There was a problem hiding this comment.
P2: A bind can return success without writing this receipt, but the documentation presents the write as guaranteed. Say the host attempts the write and that identity queries fail closed when it cannot save the receipt, or make receipt failure fail the bind.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At docs/cloud-trigger-identity.md, line 24:
<comment>A bind can return success without writing this receipt, but the documentation presents the write as guaranteed. Say the host attempts the write and that identity queries fail closed when it cannot save the receipt, or make receipt failure fail the bind.</comment>
<file context>
@@ -0,0 +1,39 @@
+The client verifies root `SO_PEERCRED`, bounds the entire read to five seconds
+and the reply to 2 KiB, and fails immediately on a full connect backlog.
+
+A successful bind writes `/var/lib/cmux/install/identity.json`, a public receipt
+of the machine, instance and install-key fingerprint, mode 0600. The query
+requires that receipt, the installed key, `bound.json` and newly observed
</file context>
Suggested change
| A successful bind writes `/var/lib/cmux/install/identity.json`, a public receipt | |
| of the machine, instance and install-key fingerprint, mode 0600. The query | |
| A successful bind attempts to write `/var/lib/cmux/install/identity.json`, a public receipt | |
| of the machine, instance and install-key fingerprint, mode 0600; if it cannot be saved, | |
| identity queries fail closed. |
| } | ||
| // The new identity IPC stays unavailable if the receipt cannot be saved. | ||
| // Do not retry a spent bind token or disturb the ordinary bound machine. | ||
| if crate::identity::record(store, &bound.machine, key).is_err() { |
There was a problem hiding this comment.
P2: A receipt-write failure consumes bind.json and returns Bound, but no later path retries identity::record, so a transient filesystem error permanently leaves the new identity IPC unavailable. Preserve a durable retry or attempt receipt creation when resuming an otherwise bound guest.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At cmux-tui/crates/cmux-host/src/cloud/client.rs, line 218:
<comment>A receipt-write failure consumes `bind.json` and returns `Bound`, but no later path retries `identity::record`, so a transient filesystem error permanently leaves the new identity IPC unavailable. Preserve a durable retry or attempt receipt creation when resuming an otherwise bound guest.</comment>
<file context>
@@ -213,6 +213,11 @@ pub fn bind_machine(
}
+ // The new identity IPC stays unavailable if the receipt cannot be saved.
+ // Do not retry a spent bind token or disturb the ordinary bound machine.
+ if crate::identity::record(store, &bound.machine, key).is_err() {
+ store.remove(crate::identity::RECEIPT);
+ }
</file context>
|
|
||
| A successful bind writes `/var/lib/cmux/install/identity.json`, a public receipt | ||
| of the machine, instance and install-key fingerprint, mode 0600. The query | ||
| requires that receipt, the installed key, `bound.json` and newly observed |
There was a problem hiding this comment.
P3: The doc names the receipt and socket paths but not bound.json. Add its path, /var/lib/cmux/bound.json, where the requirement is stated so operators can check it.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At docs/cloud-trigger-identity.md, line 26:
<comment>The doc names the receipt and socket paths but not `bound.json`. Add its path, `/var/lib/cmux/bound.json`, where the requirement is stated so operators can check it.</comment>
<file context>
@@ -0,0 +1,39 @@
+
+A successful bind writes `/var/lib/cmux/install/identity.json`, a public receipt
+of the machine, instance and install-key fingerprint, mode 0600. The query
+requires that receipt, the installed key, `bound.json` and newly observed
+provider instance all agree. Resuming the same instance keeps the key. A clone
+cannot combine an inherited `bound.json` with a newly rotated install key.
</file context>
| } | ||
|
|
||
| #[test] | ||
| fn identity_requires_receipt_current_binding_and_key() { |
There was a problem hiding this comment.
P3: The new tests never cover the receipt.machine != bound.machine branch in enrolled(). Add a case that rewrites bound.json with a different machine while keeping the receipt, and assert enrolled fails. Also cover an empty bound.machine and an empty bound.install.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At cmux-tui/crates/cmux-host/src/identity/tests.rs, line 49:
<comment>The new tests never cover the `receipt.machine != bound.machine` branch in `enrolled()`. Add a case that rewrites `bound.json` with a different `machine` while keeping the receipt, and assert `enrolled` fails. Also cover an empty `bound.machine` and an empty `bound.install`.</comment>
<file context>
@@ -0,0 +1,73 @@
+}
+
+#[test]
+fn identity_requires_receipt_current_binding_and_key() {
+ let (mut store, _) = enrolled_store();
+ store.write(BIND_FILE, "{}", 0o600).unwrap();
</file context>
| // The new identity IPC stays unavailable if the receipt cannot be saved. | ||
| // Do not retry a spent bind token or disturb the ordinary bound machine. | ||
| if crate::identity::record(store, &bound.machine, key).is_err() { | ||
| store.remove(crate::identity::RECEIPT); |
There was a problem hiding this comment.
P3: The receipt-failure branch in bind_machine has no test. Add a case where the install key file is unreadable or the receipt write fails, and assert that bind still returns Bound, bind.json is removed, and the receipt is gone.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At cmux-tui/crates/cmux-host/src/cloud/client.rs, line 219:
<comment>The receipt-failure branch in `bind_machine` has no test. Add a case where the install key file is unreadable or the receipt write fails, and assert that bind still returns `Bound`, `bind.json` is removed, and the receipt is gone.</comment>
<file context>
@@ -213,6 +213,11 @@ pub fn bind_machine(
+ // The new identity IPC stays unavailable if the receipt cannot be saved.
+ // Do not retry a spent bind token or disturb the ordinary bound machine.
+ if crate::identity::record(store, &bound.machine, key).is_err() {
+ store.remove(crate::identity::RECEIPT);
+ }
// Best effort: the token is spent, so a retry would be refused and lose
</file context>
| } | ||
| Err(_) => code(1), | ||
| }, | ||
| Err(e) => { |
There was a problem hiding this comment.
P3: An unreachable identity socket (host not running) exits 1 (internal). cmux host status uses exit 3 (not found) for the same condition. Map the not-running case to code(3) so callers can tell it apart from an internal failure.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At cmux-tui/crates/cmux-host/src/cli.rs, line 371:
<comment>An unreachable identity socket (host not running) exits 1 (internal). `cmux host status` uses exit 3 (not found) for the same condition. Map the not-running case to code(3) so callers can tell it apart from an internal failure.</comment>
<file context>
@@ -358,6 +358,22 @@ fn rekey_verb(args: &[String]) -> u8 {
+ }
+ Err(_) => code(1),
+ },
+ Err(e) => {
+ eprintln!("cmux host cloud identity: {e}");
+ code(1)
</file context>
| #[cfg(target_os = "linux")] | ||
| fn cloud_verb(args: &[String]) -> u8 { | ||
| // Identity authority has no --root or environment override. | ||
| if args == ["identity"] { |
There was a problem hiding this comment.
P3: The module doc at the top of this file still lists only cloud daemon-info | probe-activity. Add cloud identity to that list so it matches USAGE and the new branch.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At cmux-tui/crates/cmux-host/src/cli.rs, line 362:
<comment>The module doc at the top of this file still lists only `cloud daemon-info | probe-activity`. Add `cloud identity` to that list so it matches USAGE and the new branch.</comment>
<file context>
@@ -358,6 +358,22 @@ fn rekey_verb(args: &[String]) -> u8 {
#[cfg(target_os = "linux")]
fn cloud_verb(args: &[String]) -> u8 {
+ // Identity authority has no --root or environment override.
+ if args == ["identity"] {
+ return match crate::identity::query() {
+ Ok(identity) => match serde_json::to_string(&identity) {
</file context>
There was a problem hiding this comment.
3 existing issues remain and 1 new issue found across 12 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="docs/cloud-trigger-identity.md">
<violation number="1" location="docs/cloud-trigger-identity.md:13">
P2: This absolute claim is false because enrollment sends `bind_token` to `/v1/cloud/bind`. Limit it to the identity response: it includes no install token, grant, or private key.</violation>
</file>
Requires human review: Auto-approval blocked because this review re-detected 3 unresolved issues already reported by Cubic.
View guided diff | Turn on auto-fix | Re-trigger cubic
|
|
||
| `guestKey` is SHA-256 of the DER SubjectPublicKeyInfo of the existing P-256 | ||
| install key (id-ecPublicKey / prime256v1, uncompressed point), lowercase hex. | ||
| It is not a credential. No install token, grant or private key leaves the host. |
There was a problem hiding this comment.
P2: This absolute claim is false because enrollment sends bind_token to /v1/cloud/bind. Limit it to the identity response: it includes no install token, grant, or private key.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At docs/cloud-trigger-identity.md, line 13:
<comment>This absolute claim is false because enrollment sends `bind_token` to `/v1/cloud/bind`. Limit it to the identity response: it includes no install token, grant, or private key.</comment>
<file context>
@@ -0,0 +1,39 @@
+
+`guestKey` is SHA-256 of the DER SubjectPublicKeyInfo of the existing P-256
+install key (id-ecPublicKey / prime256v1, uncompressed point), lowercase hex.
+It is not a credential. No install token, grant or private key leaves the host.
+
+The fixed Unix socket is `/run/cmux-host/identity.sock`, root-owned, mode 0660
</file context>
Suggested change
| It is not a credential. No install token, grant or private key leaves the host. | |
| It is not a credential. The identity response includes no install token, grant or private key. |
teamleaderleo
temporarily deployed
to
ci-routing-agent
November 1, 2026 07:17 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
|
Found 9 test failures on Blacksmith runners: Failures
|
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
This was referenced Nov 1, 2026
This was referenced Nov 1, 2026
Contributor
|
This PR left macOS clippy red on feat-cmux-next: |
lawrencecchen
added a commit
that referenced
this pull request
Nov 1, 2026
…30555) * fix(cmux-tui): clear base clippy reds from #30361, #30362 and #30363 cargo clippy -D warnings is red on feat-cmux-next 16b7cc2. Fix forward (each culprit is a whole feature; every error is a lint, not behavior): - cmux-host identity::enrolled (#30362): its only caller is the Linux host loop, so it was dead code on macOS. Compile it on Linux and in tests, and import its wire names inside it so macOS has no unused imports. - cmux-host github_repos_tests (#30361): name the fake's state tuple (type_complexity). - acpmux agent_tools, cloud_repo_mcp and cmux-tui-core cloud_repos engine and runtime (#30361): collapse nested ifs into let chains; add the missing semicolon after the clone wait sleep. Same conditions, same order. - cmux-tui-core agent_session_folders test (#30363): drop a redundant clone. - acpmux remote_guard (#30363): move cloud_chat_tests to the end of the file (items_after_test_module). No code change in the module. * fix(cmux-host): identity tests import the wire file names they write The previous commit moved BIND_FILE and BOUND_FILE out of identity.rs's module imports into enrolled(), so the tests' use super::* lost them (E0425 on macOS and Linux test builds). Import them in the tests. * fix(cmux-tui): cloud repo CLI validation uses is_none_or (nonminimal_bool, #30361) !x.is_some_and(|ms| ms <= MAX) is x.is_none_or(|ms| ms > MAX), and !s.is_some_and(|v| !v.trim().is_empty()) is s.is_none_or(|v| v.trim().is_empty()). Same accepted and refused values.
4 tasks
This branch was previously deployed
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
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.
Cloud trigger tokens need to bind to the receiving machine after snapshot restore. A copied status file or machine ID cannot establish that identity.
Adds
cmux host cloud identityandcmux_host::identity::query(). The system host accepts the request before reading provider metadata through its existing sole reader, then checks its settled identity against the current install key and a receipt written only after successful Cloud enrollment. It returns machine, provider instance and SHA-256 of the existing P-256 public key DER SPKI. The Unix client verifies root peer credentials, uses a fixed socket, and bounds reply size and total read time. No credential leaves the host.Requested behavior: “an agent principal: a scoped, short-lived trigger token minted per trigger and bound to its owner.” This PR supplies the independent receiving-machine identity for that token.
Security limits: this query is a point-in-time observation, not an authorization lease. Trigger callers must recheck after asynchronous work and before mutation, and abort stale operations. This change does not claim inherited acpmux processes stop or that an operation already executing during a memory snapshot is atomic. Old guests without a successful enrollment receipt fail closed for this new capability; existing attach/chat paths are unaffected. Root compromise remains outside the local boundary. See docs/cloud-trigger-identity.md.
Validation: focused unit coverage includes same-instance resume, clone/key mismatch, stale or missing enrollment, failed metadata, identity transition, peer identity rejection, oversized replies and a slow-byte deadline. Fleet cmux-ci test and clippy are being dispatched at this head; results will be recorded before landing. Static prepush checks pass. Broad verify-local finds existing localization debt and missing CmuxDeviceHub workspace grouping; no Swift or project files change.
Prior art read: #30318 (login sync, separate bind seam), #28761 (whole-request deadline and bounded replies retained), #25039 (existing enrollment credential boundary), #26863 (no credential export), #25682 (snapshot trajectory restore). No UI change.
Agent session: agent session