Skip to content

ci: execute Cloud host identity tests on relevant PRs - #30420

Merged
teamleaderleo merged 1 commit into
feat-cmux-nextfrom
lane/cloud-trigger-identity-ci
Nov 1, 2026
Merged

teamleaderleo merged 1 commit into
feat-cmux-nextfrom
lane/cloud-trigger-identity-ci

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Nov 1, 2026 •

Copy link
Copy Markdown
Collaborator

Cloud host identity tests currently compile in PR clippy but execute only on a manual fleet run. Run them automatically when the host, its path dependencies, build inputs, or this workflow change. Reuse the existing dependency classifier, Rust setup, runner and warm target directory; reject an empty test selection.

Follows merged #30362. This changes only 23 workflow lines and adds no Rust source changes. It reuses the existing runner, setup and warm target directory. No runner, token, trust or permission changes.

Validation: the parent implementation passed all 79 host library tests (0.30s), including all 7 identity tests (0.10s); shell syntax, whitespace and static prepush checks pass. PR CI run 41285756259 executed cargo test --locked -p cmux-host --lib identity: all 7 identity tests passed, zero failures, in 0.10s; the clippy job passed. Prior art: #28644 (dependency-aware test selection), #28169 (keep PR panic checks), #27599 (reject empty selections).

Agent session: agent session

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review (claude-opus-5-5 medium, cloud-reviewer): SHIP at b787da1

This carries my review at 5f5c8bd to b787da1, which is the same commit rebased onto the merged host change (8c6f5ee). I checked that the 23 workflow lines are identical and that no other file changes. I reviewed only the child diff: 23 added lines in .github/workflows/cmux-tui-spec.yml, stacked on the reviewed host-identity head 0d70c5e. The cmux-tui tree at this head is identical to 0d70c5e's (3cec7ea5), so no product code changed.

  • The new step reuses the clippy job's pinned Rust, the cache and the steps.base output. There are no runner, trust, permission or secret changes.
  • The gate is the existing cargo_test_verdict.py touches cmux-tui/crates/cmux-host, which follows path dependencies, the workspace manifest, the lockfile and the toolchain. If the base SHA is missing or the diff fails, the step runs the tests anyway rather than skipping them, so it fails safe.
  • set -o pipefail with tee keeps a failing cargo test red. The test result: ok. [1-9]… passed grep rejects a filter that selects zero tests. --lib builds a single test binary, so there is only one result line to match.

Follow-ups:

  • Because of if: !cancelled(), the step still runs after clippy fails. That costs a test build on an already-red job but is harmless.

Host identity receipt for #30362 (merged; the broker refuses comments on closed PRs, so it is recorded here)

Review (claude-opus-5-5 medium, cloud-reviewer): SHIP at b4ffb20

This is a full security review at b2ea9c4, carried to the final head through three deltas I checked:

  • b2ea9c4..2d12fbd: a cloud identity usage line in cli.rs. The dripping-reply test now sends for about 2 s, so the 1 s assertion tells the total deadline apart from a per-read timeout.
  • 2d12fbd..0d70c5e (ipc.rs only): as casts become checked try_from, Instant + timeout becomes checked_add, and &bytes[..n] becomes take(n). Behaviour is unchanged, and every new error path fails closed.
  • 0d70c5e..b4ffb20: std::mem::size_of_val becomes the prelude size_of_val, and the two socket tests set their tempdir to 0700. This test-only fix is needed because the production Listener correctly refuses group-writable parents. Production behaviour is unchanged.

Full review at b2ea9c4 follows.

Code review of the diff against feat-cmux-next (merge base 078389c), with docs/cloud-trigger-identity.md and the surrounding machine.rs / agent.rs / linux/mod.rs read. I did not build it; rustfmt, clippy and the cmux-tui tests were still pending when I read it. All the logic is in Rust; nothing touches Swift.

What I checked:

  • Peer check. The client calls SO_PEERCRED on the connected stream, which on Linux returns the credentials the listener had at listen(). It requires uid 0, checks the returned length, and reads nothing before the check passes. A non-root process can't fake it. There is no path or env override, and cloud identity is matched before root_arg. The negative path is tested (identity_socket_rejects_an_untrusted_peer_before_reading).
  • Socket path. The listener only binds when the host runs as euid 0 with the system root. The parent dir is checked with symlink_metadata, so a symlinked dir is rejected, and it must be a dir owned by euid with no group or other write bit (ensure_agent_dir makes /run/cmux-host 0755). Because of that, the unlink-then-bind sequence can't race. The socket is chowned to root:work group and set to 0660. A socket squatted before the host starts would fail the client's peer check anyway.
  • Freshness. Every Wake::Identity forces platform.observe() in the same turn. The client is accepted before that read. The reply needs identity_settled() (not parked, roles running, no verify_pending), and the platform also needs observed_instance == current_id. A failed read sets clone_signal and with it verify_pending, so the reply is the error object. The agent test covers fresh read, failed read and clone transition.
  • Clone and key mismatch. enrolled() requires the fresh instance to match the install-key record's instance and the receipt's instance. It requires receipt.machine == bound.machine and no bind.json. It also rebuilds the receipt from the installed key's SPKI and compares the whole struct. So both cases fail closed: the same state with a rotated key, and the same key read under a new instance. Tests cover both, before and after rotation, plus each missing file. If writing the receipt fails at bind, the receipt is removed and the spent token is not retried.
  • Fingerprint. The SPKI DER prefix is the standard P-256 id-ecPublicKey header (0x59 total length, 0x42 bit string), followed by ring's 65-byte uncompressed point. It is canonical.
  • Secrets. The receipt and reply hold only machine, instanceId and guestKey, and deny_unknown_fields is set. The grant is asserted absent. The socket error is one generic object, so no internal error strings leak.
  • Bounded I/O. The client connects nonblocking (a full backlog fails at once), reads under one 5 s overall deadline (a drip test exists) and caps the reply at 2 KiB. The server accepts at most 8 clients per wake and writes once, nonblocking, then drops the stream.
  • Upgrades. A new client against an old host gets ENOENT, an error. A new host on a guest with no receipt also returns an error until a fresh enrollment. Both fail closed, and the existing paths are untouched.

No blocking findings.

Follow-ups:

  • [follow-up] Any work-group process can make the host main loop run back-to-back provider metadata reads, one per turn, each with the full retry budget. If a read fails, the host enters verify_pending, which delays Resumed and announce until a later read succeeds. Consider a small rate limit on identity-driven observations (a minimum gap, answering the queued clients after the next permitted read) so a busy caller can't hit the metadata service's limits or delay other wakes.
  • [follow-up] A failure in Listener::bind is propagated with ? from LinuxPlatform::new, so an identity-socket problem stops the whole host from starting. Failing closed for identity alone (log it and run without the listener) would keep a new optional capability from taking down attach and chat.
  • [follow-up] Tests are missing for Listener::bind refusing a symlinked, foreign-owned or group/world-writable parent, and for LinuxPlatform::reply_identity's observed_instance != settled filter.
  • [follow-up] public_spki() could assert that the public key is 65 bytes starting with 0x04, so a future ring or curve change can't silently produce a malformed SPKI.

@teamleaderleo
teamleaderleo merged commit 7cde2f3 into feat-cmux-next Nov 1, 2026
67 of 74 checks passed
@teamleaderleo
teamleaderleo deleted the lane/cloud-trigger-identity-ci branch November 1, 2026 07:46
@github-actions

github-actions Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

cmux status (one comment, edited in place; each line is updated by the job that owns it)

Review b787da1: HQ feed card · gallery before/after runs · checks

App of this head (app.zip, kept 1 day)

gh workflow run reload-build.yml -R manaflow-ai/cmux --ref feat-cmux-next -f tag=pr30420 -f ref=lane/cloud-trigger-identity-ci -f platform=macos -f backend_mode=local

Shipped in:

@github-actions github-actions Bot added the risk:normal Risk tier (docs/ci/risk-tiers.md) label Nov 1, 2026

This branch was previously deployed

1 inactive deployment
ci-routing-agent — b787da17 Deployed Nov 1, 2026 by teamleaderleo via cmux-tui tree preflight #94588
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk:normal Risk tier (docs/ci/risk-tiers.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant