Skip to content

base red: panic ratchet: cmux-rd-link's 7, acpmux's and cmux-tui's new index/slice hits - #29024

Merged
lawrencecchen merged 5 commits into
feat-cmux-nextfrom
lane/panic-ratchet-rd-link
Oct 31, 2026
Merged

lawrencecchen merged 5 commits into
feat-cmux-nextfrom
lane/panic-ratchet-rd-link

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

What

A base red, priority zero. The Rust panic ratchet (scripts/ci/cmux-tui-rust-check.sh panic, the "Rust panic ratchet" step of cmux-tui.yml) is red on feat-cmux-next on three rows. Every PR that touches cmux-tui inherits it.

Fix

Each hit is rewritten with checked access, as #28808 and #28847 did. The baseline is not raised and no allow is added. No behavior change: where the old code would have panicked, the new code gives an empty slice, a null or no match.

  • cmux-rd-link lib.rs: write_handshake truncates the buffer to the length Noise wrote; seal builds the packet with push/extend_from_slice/resize and gives Noise the body through get_mut(9..).
  • cmux-rd-link replay.rs: the window word goes through get/get_mut. slot() always gives a word < WORDS.
  • acpmux host_peering.rs, hub/peers.rs, server/requests.rs: Value indexing becomes pointer/get and JsonSet::set.
  • cmux-tui group_create.rs, chief/mod.rs, chief/pipe.rs: get(n..).unwrap_or_default(). apps_list.rs: get(..).and_then(..).
  • cmux-tui cli/automation.rs take_target: it walks the words with an iterator. A new unit test pins both flag forms, the default, a missing value and a bad target.

The rd-link, acpmux and four cmux-tui hunks are Leo's from #28958 (credited with Co-authored-by; his unused-import fix dc34d45 is cherry-picked with his authorship). The take_target hit came after his count.

Verification

Observed / Not run

  • Observed: the ratchet red on the base and green at 467388d. prepush-gate.py: no new violations. Tests at 467388d (job c285412e8b48acf834fcdfd0): 13508 of 13519 passed. The cmux-rd-link tests and the new take_target test passed. 11 tests failed in acpmux peer_restart/startup_load, cmux-tui cli_audit/setup/apps_store and cmux-tui-core local_resize_order. None of them is in the changed code.
  • Not run: the 11 failing tests on the base, so I did not prove that they are base reds. No Mac build (Rust only, Linux ratchet).

Changelog

none

The Rust panic ratchet is red on feat-cmux-next since the cmux-rd-link crate
landed by direct push (3b9cf69, 6d04ce1, ae8cd21): a new crate
has a baseline of 0, and it had 7 clippy::indexing_slicing hits.

- lib.rs write_handshake: truncate the buffer to the length Noise wrote
  instead of slicing it.
- lib.rs seal: build the packet with push/extend/resize and hand Noise the
  body through get_mut, not three index writes.
- replay.rs: the window word is read and written through get/get_mut; slot()
  always gives a word < WORDS, so the miss branch never runs.

No behavior change. The baseline is not raised and no allow is added.
These are the same hunks as #28958 (cmux-rd-link part), so that PR merges
over this one with no conflict.

Co-authored-by: Leo Li <cheerleaderleo@outlook.com>
@lawrencecchen lawrencecchen added the risk:low Risk tier (docs/ci/risk-tiers.md) label Oct 31, 2026
@cursor

cursor Bot commented Oct 31, 2026

Copy link
Copy Markdown

Bugbot needs on-demand usage enabled

Bugbot 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.

@github-actions

github-actions Bot commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

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

Review 2d634ba: 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=pr29024 -f ref=lane/panic-ratchet-rd-link -f platform=macos -f backend_mode=local

Shipped in:

@github-actions github-actions Bot added risk:normal Risk tier (docs/ci/risk-tiers.md) and removed risk:low Risk tier (docs/ci/risk-tiers.md) labels Oct 31, 2026
lawrencecchen referenced this pull request Oct 31, 2026
…aled datagrams, reliable control)

A pure crate (no I/O, injected clock) for Mac Screen on cmux.rd/1
(plans/cmux-next/remote-desktop-mac-host.md section 7):
- Noise_XX_25519_ChaChaPoly_BLAKE2s with the hub session id as prologue. The link
  accepts only the remote session key the caller expects (the device hub vouches for it:
  each side signs its session key with its device key, checked before the link is built).
  The initiator checks the responder's key on message 2, before it reveals its own.
- Sealed packets carry an explicit 64-bit nonce (stateless transport), so loss, reorder and
  duplicates on any path are fine; a 2048-nonce sliding window drops replays.
- Inside the seal: rd datagrams (best effort), an ordered reliable control channel (window
  128, RTO with backoff; hello, start and stats leave the TCP socket so the session works
  through routers and relays), path probes and replies, and close.
- Handshake retries every 250 ms and gives up after 10 s.
- Packet tags 0xD0..0xD7 sit outside every RFC 7983 range, so one UDP socket also carries
  STUN and TURN.

Tests (behavior at the crate boundary, two links over an in-memory network): handshake,
control and datagrams across 20 % loss with duplication and reordering; a peer the hub did
not vouch for is refused; another session's handshake does not complete; tampered and
replayed packets open nothing; an unanswered handshake times out. rbx (lane mac-screen-rd):
cargo fmt, clippy --locked -D warnings, 5 tests passed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@blacksmith-sh

blacksmith-sh Bot commented Oct 31, 2026 •

Copy link
Copy Markdown

Found 3 test failures on Blacksmith runners:

Failures

Test View Logs
test_a_picked_signing_label_has_a_rerun_alternative (main.RunsOnFallbackTests.test_
a_picked_signing_label_has_a_rerun_alternative)/
test_a_picked_signing_label_has_a_rerun_alternative (main.RunsOnFallbackTests.test_
a_picked_signing_label_has_a_rerun_alternative)
View Logs
test_every_listed_hook_is_one_glaeda_released_with_the_check (main.SigningHooksInSt
epWithGlaedaTests.test_every_listed_hook_is_one_glaeda_released_with_the_check)/
test_every_listed_hook_is_one_glaeda_released_with_the_check (main.SigningHooksInSt
epWithGlaedaTests.test_every_listed_hook_is_one_glaeda_released_with_the_check)
View Logs
test_no_runs_on_chain_ends_on_the_reserved_side_label (main.RunsOnFallbackTests.tes
t_no_runs_on_chain_ends_on_the_reserved_side_label)/
test_no_runs_on_chain_ends_on_the_reserved_side_label (main.RunsOnFallbackTests.tes
t_no_runs_on_chain_ends_on_the_reserved_side_label)
View Logs

Fix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (claude-opus-subagent): SHIP at 19b8aaa

Findings: none blocking. I read the code only; I did not build or run tests.

  • seal(): the bytes are the same as before. The old code made vec![0; 1+8+plain.len()+16] and then wrote the tag and the little-endian nonce into it. The new code pushes the tag, adds the nonce and fills the rest with zeros up to the same length. Noise gets the same body slice (9.., length plain.len()+16), and truncate(9 + n) is the same. get_mut(9..) cannot miss, because resize sets the length to 25 or more first. If it did miss, the empty &mut [u8] default would fail loudly: snow 0.10.0 StatelessTransportState::write_message checks payload.len() + TAGLEN > message.len() and returns Error::Input, so the caller gets LinkError::Noise. You never get a bad packet without an error.
  • write_handshake(): buf.truncate(n) and then extend_from_slice(&buf) gives the same bytes as &buf[..n]. snow returns how many bytes it wrote into buf, so n <= buf.len() (256) is always true.
  • replay.rs: slot() takes nonce % 2048 and divides by 64, so the word is always below 32 (WORDS). get/get_mut always hit. The replay window works the same as before: same floor check, same clearing of slots as the window slides, same setting of the bit after authentication. If a lookup ever missed, check would return false and drop the packet, which is the safe side.
  • Other code I checked: cmux-rd-link on feat-cmux-next has not changed since the merge base, so this PR does not drift from the base. Noise usage is the same as before. Note, not caused by this PR: accept() computes nonce + 1 - self.top, which would overflow at nonce == u64::MAX. This cannot happen, because snow refuses nonce u64::MAX (Exhausted) before accept runs.

@lawrencecchen lawrencecchen added the ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds label Oct 31, 2026
lawrencecchen and others added 3 commits October 31, 2026 06:58
The panic ratchet is also red on two more rows on feat-cmux-next:
acpmux indexing_slicing 692 > baseline 686 (from #28170) and cmux-tui
836 > baseline 831. Each hit is rewritten with checked access (pointer/get,
JsonSet::set, get(n..).unwrap_or_default()); no behavior change: an empty
slice or a null stands where the old code would have panicked. The baseline
is not raised.

These are Leo's hunks from #28958 (e9303c1, 5033a36, 96299c6),
including his compile fix for the Value name inside tracing::warn!.

Co-authored-by: Leo Li <cheerleaderleo@outlook.com>
…s with an iterator (cmux-tui 836 > 831)

7b3b9f7 (cmux automation --target cloud) added args[i] in take_target,
the sixth new cmux-tui indexing_slicing hit. The loop now takes the words
from an iterator and the value after --target from the same iterator; the
words, the target and the missing-value usage error are the same. A unit test
pins both flag forms, the default, a missing value and a bad target.
@lawrencecchen lawrencecchen removed the ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds label Oct 31, 2026
@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Fleet results at head 467388d:

  • Panic ratchet (cmux-tui-rust-check.sh panic, job f8235b86838f87ac3b080fd3, https://github.com/manaflow-ai/cmuxterm-hq/actions/runs/41179291058): green. panic ratchet pass, cmux-tui-rust-check: green. No indexing_slicing row for cmux-rd-link, acpmux or cmux-tui.
  • Tests (cmux-tui-rust-check.sh test, job c285412e8b48acf834fcdfd0, https://github.com/manaflow-ai/cmuxterm-hq/actions/runs/41179290793): 13519 run, 13508 passed, 11 failed (each also failed on its rerun).
    • cmux-rd-link: the 8 endpoint and link tests passed. The new cli::automation::tests::target_is_taken_out_of_the_words_in_both_forms passed.
    • The 11 failures are outside the changed code: acpmux peer_restart close_1012 and startup_load; cmux-tui cli_audit, 6 setup harness tests, apps_store customize folder; cmux-tui-core local_resize_order. I did not run the base for these, so I did not prove that they are base reds.

…::* (unused-imports)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-d9e7e9f7fb2b87a57d5f
@lawrencecchen lawrencecchen changed the title base red: panic ratchet: cmux-rd-link's 7 index/slice hits base red: panic ratchet: cmux-rd-link's 7, acpmux's and cmux-tui's new index/slice hits Oct 31, 2026
@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (claude-opus-subagent): SHIP at 2d634ba

Findings: none. I checked each hunk against the base. I did not build or run tests (cargo is not run locally). The clippy counts are not checked here.

  • host_peering.rs: v["a"]["b"] == true and pointer("/a/b") == Some(&Value::Bool(true)) give the same result for every input. Index gives Null on a missing key or a non-object, and pointer gives None, so neither matches. The same is true for settled == false and handshaking. last_error(Null) returns "" as before.
  • peers.rs and requests.rs: JsonSet::set and IndexMut give the same result on an object or null. Both targets are always json!({...}) objects. JsonSet is in scope in both files: peers.rs has use cmux_json::JsonSet as _, and requests.rs has crate::no_panic::JsonExt, which is pub use cmux_json::JsonSet as JsonExt. It is correct to remove the duplicate import in 2d634ba. In peers.rs, peer.get("state").unwrap_or(&Null) shows the same text as peer["state"].
  • cmux-rd-link seal: push, extend, then resize gives the same buffer of 1+8+len+16 bytes as before. get_mut(9..) is always Some, and &mut [u8]: Default compiles. The borrow stops before truncate. write_handshake: buf is a Vec, and truncate(n) then extend is the same as &buf[..n] (n <= len). replay.rs: slot() always returns word < WORDS, so the checked access always takes the Some path.
  • get(n..).unwrap_or_default() call sites: group_create (args.len() >= 3 because the 3-word prefix matched) and chief acp (guarded by args.first()). In pipe.rs, printed is always <= replies.len() because Turn only pushes. So the empty-slice fallback never runs. The new while let Some(arg) = rest.get(index) loop is the same as index < rest.len(), and every continue path still increments index.
  • take_target: the iterator walk is the same as the old loop. When --target is the last word, it still gives Failure::Usage(missing_value). Because words is used in the body, while_let_on_iterator does not apply. The new test covers both forms, this-mac, a missing value and an unknown target.

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Fleet results at head 2d634ba (Leo's unused-import fix dc34d45, cherry-picked):

Base findings, not caused by this PR:

@lawrencecchen lawrencecchen added the ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds label Oct 31, 2026
@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Owner note for PL-acpmux: #29024 changes three acpmux files with checked access and no behavior change: cli/host_peering.rs (pointer/get in place of Value indexing), hub/peers.rs (get and JsonSet::set), and server/requests.rs (JsonSet::set; a duplicate import is removed). These are Leo's hunks from #28958. They clear the panic-ratchet row acpmux: indexing_slicing 692 > 686 from #28170.

This branch was previously deployed

3 inactive deployments
artifacts — 2d634ba8 Deployed Oct 31, 2026 by lawrencecchen via publish tree-addressed cmux-tui #88226
sentry-dsyms — 2d634ba8 Deployed Oct 31, 2026 by lawrencecchen via upload cmux-tui dSYMs to Sentry #88226
ci-routing-agent — 2d634ba8 Deployed Oct 31, 2026 by lawrencecchen via cmux-tui tree preflight #88226
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds risk:normal Risk tier (docs/ci/risk-tiers.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants