Skip to content

base red: cmux-wg-tun reads its buffers without indexing (panic ratchet indexing_slicing 4 > 0, new crate) - #34220

Merged
lawrencecchen merged 1 commit into
feat-cmux-nextfrom
base-red-wg-tun-panic
Nov 3, 2026
Merged

lawrencecchen merged 1 commit into
feat-cmux-nextfrom
base-red-wg-tun-panic

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Summary

The tip panic ratchet is red: cmux-wg-tun: clippy::indexing_slicing 4 > new crate (must be 0) (fleet job e3cabfe8f3a4526406d20a58). cmux-wg-tun came in lawrencecchen's direct push ad2c76731f28..cc33e089376e (commits b4f6d32 and d2b1b1c).

Neither #34198 (the revert, which keeps the Rust crate) nor #34204 (the extension forward fix) changes this crate, so this PR fixes the red alone. It does not depend on either of them.

The four indexes now read with get(), with no allow:

  • utun.rs interface_name: the name takes the whole buffer when the kernel reports a longer length (as the old min() did) and ends at its first NUL.
  • engine.rs drain_tun: a packet range outside the frame buffer, or an empty one, skips that packet.
  • engine.rs drain_socket: a datagram length outside its buffer stops the burst instead of panicking.

Behavior inside the bounds is unchanged.

Changelog

none

Testing

  • python3 scripts/cmux-next/prepush-gate.py: no new violations.
  • Fleet, macOS: cmux-tui-rust-check.sh test engine::tests::, step f74200465feb95b217cf67d8, green. It ran all 6 cmux-wg-tun tests (engine::tests::*) among 35 matched. 5 passed in the full run. a_socketpair_is_not_mistaken_for_utun failed in the full run and passed alone (nextest FLAKY). That test calls only interface_name, whose result this PR leaves unchanged for every in-bounds length, so the flake is in the crate from the push and not from this change; the crate owner must fix it. An earlier step a809c4f688e9 used the filter cmux_wg_tun, which selected 0 tests (exit 4) and proved nothing.
  • Fleet, Linux: panic ratchet cmux-tui-rust-check.sh panic, step 16044e3cf8c0c17c9078de52. Result in a comment.

Observed / Not run

  • Observed: the prepush gate; fleet results in the comment.
  • Not run: a live tunnel.

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


Summary by cubic

Fixes the clippy::indexing_slicing panic ratchet violation in cmux-wg-tun by replacing direct slice indexing with get() reads, so the new crate meets the required zero-violation budget.

  • The utun interface name now takes the whole buffer when the kernel reports a longer length and ends at its first NUL.
  • Packet ranges or datagram lengths outside their buffer now skip that read instead of panicking.
  • Behavior inside the buffer bounds is unchanged.

Written for commit 6277862. Summary will update on new commits.

View guided diff Turn on auto-fix

…et: clippy::indexing_slicing 4 in a new crate, must be 0)

The direct push ad2c767..cc33e08 added cmux-wg-tun with four slice
indexes (utun.rs interface_name x2, engine.rs drain_tun and drain_socket). The
tip panic ratchet (fleet job e3cabfe8f3a4526406d20a58) counts them against a new
crate's budget of 0. Each now reads with get(): the utun name takes the whole
buffer when the kernel reports a longer length (as the min() did) and ends at its
first NUL; a packet range or datagram length outside its buffer skips that read
instead of panicking. No allow attributes.
@lawrencecchen lawrencecchen added the risk:normal Risk tier (docs/ci/risk-tiers.md) label Nov 3, 2026
@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

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

Review 6277862: 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=pr34220 -f ref=base-red-wg-tun-panic -f platform=macos -f backend_mode=local

Shipped in:

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (claude-opus-reviewer): SHIP at 6277862

Independent review (read-only). By reading, it compiles: the borrows are disjoint fields, Default holds for &[u8], and as_slice works on the array. Behavior inside the bounds is unchanged: the utun name equals the old min() and position() result, and an empty packet is still skipped. The only index expressions left are constant indexes into fixed arrays (fds[0..2]), which clippy does not lint. The PR adds no allow attribute and no other deny-lint site.

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Panic ratchet at the head 6277862: fleet step 16044e3cf8c0c17c9078de52 (hq run 41712271411).

  • cmux-wg-tun is at 0 indexing_slicing at the head. The only cmux-wg-tun lines are warnings, not RED: cmux-wg-tun: clippy::arithmetic_side_effects 1 > new crate (must be 0) and cmux-wg-tun: clippy::cast_possible_truncation 3 > new crate (must be 0).
  • The RED lines are base reds and are not in files this PR touches: acpmux: clippy::indexing_slicing 688 > baseline 686 and cmux-tui: clippy::indexing_slicing 835 > baseline 831. The tip b8cf524 has the same reds (acpmux 688 > 686), and the BASE RED TIP owner is working on them.

macOS tests: step f74200465feb95b217cf67d8 is green with all 6 cmux-wg-tun tests (one FLAKY: a_socketpair_is_not_mistaken_for_utun failed in the full run and passed alone; see the body). The SHIP receipt for this head is above.

@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 Nov 3, 2026
@lawrencecchen
lawrencecchen merged commit db85008 into feat-cmux-next Nov 3, 2026
270 of 282 checks passed
@lawrencecchen
lawrencecchen deleted the base-red-wg-tun-panic branch November 3, 2026 21:32

This branch was previously deployed

3 inactive deployments
ci-routing-agent — 62778625 Deployed Nov 3, 2026 by github-actions[bot] via cmux-next macOS placement #67168
artifacts — 62778625 Deployed Nov 3, 2026 by lawrencecchen via publish tree-addressed cmux-tui #107560
sentry-dsyms — 62778625 Deployed Nov 3, 2026 by lawrencecchen via upload cmux-tui dSYMs to Sentry #107560
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.

1 participant