fix(security): bound the loop-shortcut iteration count (pre-existing remote DoS in #5672)

[lgray]

🤖 AI text below 🤖

Summary

Bounds the loop-shortcut iteration count — a pre-existing remote DoS merged in #5672. GameAction::DeclareShortcut carried an unvalidated IterationCount::Fixed(u32): the server payload guard discarded count with .. (its "count is a small enum — nothing unbounded" comment was false), handle_declare_shortcut moved it into the proposal unchecked, and materialize_fixed_shortcut drove for i in 0..n — one GameState clone + beat drive per cycle, up to ~4.3e9. The fix is engine-authoritative (one MAX_SHORTCUT_CYCLES cap enforced at the single proposal-build authority, so every transport — WS, WASM, Tauri, local — is covered) with a wire defense-in-depth belt on the multiplayer server.

Predecessor: #5672 (this is a security follow-up to that merged PR).

Threat model (honest, three-way — the multiplayer server applies an 8 KB inbound WS frame cap, crates/phase-server/src/main.rs:409/:1420, before deserialize; guard_game_action_payload runs post-deserialize, client_message_wire_guard.rs:50)

1. Fixed(u32::MAX) — the one catastrophic remote vector. A u32 scalar-encodes up to ~4.3e9 cycles in ~10 JSON bytes, so it sails through the 8 KB frame cap and forces ~4.3e9 GameState clones. A byte cap cannot see it; only a count cap can. Closed by the engine MAX_SHORTCUT_CYCLES reject-to-handback + a wire bound_batch_count belt.
2. Templated UntilLethal drive period (shortcut_drive_period = the client template schedule length, driving apply_until_lethal_shortcut's for i in 0..period). Structurally unbounded in the engine, but the 8 KB WS frame cap bounds a hostile schedule to a few hundred entries (~1–2 s stall) on the remote transport — not a million-cycle remote DoS. Closed at the source for all callers (incl. in-process WASM/Tauri/local that bypass the WS cap) by clamping the shared shortcut_drive_period helper.
3. Nested template vecs (a Targets pin's Vec<TargetPin>, each RoundRobin/Piecewise schedule vec). Defense-in-depth only: the 8 KB WS cap already keeps a remote nested payload to a few hundred structs, and the guard runs post-deserialize, so the added bound_lists bound downstream compute/clone work (not the transient serde allocation) for in-process callers past the frame cap.

Single-authority: the cap is checked once, before the sole ShortcutProposal build site; both confirmation paths and both materializers (materialize_fixed_shortcut, materialize_object_growth_shortcut) read the bounded n and never re-check.

Implementation method (required)

Method: /engine-implementer

CR references

• CR 732.2a — a shortcut is "a loop that repeats a specified number of times"; the CR places no board-relative upper bound, so MAX_SHORTCUT_CYCLES is annotated as an implementation safety limit, not a rules limit.
• CR 800.4a — an over-cap count hands priority to the next living seat (living_priority_seat).
• CR 704.5a — referenced by the unchanged UntilLethal terminator.

Verification

• Required checks ran clean (worktree cargo — own target/, no Tilt lock contention).

• Gate A output below is for the current committed head.

• Final review-impl below is clean for the current committed head.

• Both anchors cite existing analogous code at the same seam.

• cargo fmt --all — clean

• cargo clippy -p engine -p server-core --all-targets --features engine/proptest -- -D warnings — exit 0, no diagnostics (rebased head)

• cargo nextest run -p engine -p server-core (loop_shortcut + guard suite) — 63/63 pass, 0 failed (rebased head; includes the 5 new tests + the co-resident sibling-fix(ai): score the CR 732.2a loop-shortcut offer + cover the winner-liveness conjunct (#5672 follow-ups) #5748 tests merged in by the rebase)

• Discriminating tests (5 new, each revert-probe MEASURED FAIL→PASS):

  • over_cap_fixed_count_hands_back_with_no_drive (engine integration) — asserts the DRIVE did not run (life(P1)==l0, no GameOver); waiting_for is a sanity check only (handback and cap-absent stop-short both land on Priority — vacuity trap documented in the test ///). Revert-probe (delete the engine cap, measured pre-rebase on byte-identical code): assert_eq!(life(P1), l0) FAILS left: 0, right: 19 — the drain drove to lethal in 0.128 s (CrossLethal commits+stops, so u32::MAX reverted does not hang). Re-confirmed on the rebased head by the passing discrimination pair on the same fixture: b3_materialize_stop_short (Fixed(3), under-cap → drives) vs this test (Fixed(u32::MAX), over-cap → no drive) — only the count relative to the cap differs.
  • shortcut_drive_period_is_schedule_max (engine unit) — RoundRobin(MAX+5) ⇒ MAX_SHORTCUT_CYCLES. Revert-probe (drop .clamp): FAILS 1005 ≠ 1000.
  • rejects_over_cap_fixed_shortcut_count (server-core) — Fixed(u32::MAX) rejected. Revert-probe (restore ..): returns Ok → FAILS.
  • accepts_realistic_fixed_shortcut_count (server-core) — Fixed(50) accepted (non-vacuous). Revert-probe (threshold→0): FAILS.
  • rejects_over_cap_shortcut_schedule (server-core) — over-cap nested RoundRobin rejected. Revert-probe (disable schedule bounds): FAILS.

• Stated residual: no end-to-end runtime drive test of the templated-UntilLethal for i in 0..period path — a real over-cap drive would be the DoS. Coverage rests on the shortcut_drive_period clamp unit test + the structural fact that both consumers (validate_pins, apply_until_lethal_shortcut) are literally for i in 0..period, so a bounded period ⇒ a bounded drive by construction.

Gate A

Gate A PASS head=e8a857bdf5eebf15ad49858564c176d683e40748 base=8a1dfd7041297b48f7d3dfea6095b06c730edfd2

Anchored on

• crates/engine/src/game/engine.rs (handle_declare_shortcut pin-validation handback) — existing fail-closed reset_priority → WaitingFor::Priority { living_priority_seat } → Ok idiom reused for the over-cap count handback.
• crates/server-core/src/game_action_payload_guard.rs (ChooseManaColor.count → bound_batch_count) — existing u32-count wire bound reused for DeclareShortcut.count.

Final review-impl

Final review-impl PASS head=e8a857bdf5eebf15ad49858564c176d683e40748

[gemini-code-assist]

Code Review

This pull request implements a safety limit (CR 732.2a) to prevent remote Denial of Service (DoS) attacks via unbounded loop-shortcut declarations. It introduces a MAX_SHORTCUT_CYCLES cap of 1,000 in the engine, clamping the drive period and rejecting over-cap Fixed iteration counts before a proposal is built. Additionally, it adds payload-guard validation in server-core to bound the shortcut count and nested template vectors at the WebSocket transport layer, accompanied by comprehensive unit and integration tests. No review comments were provided, so there is no feedback to address.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[github-actions]

Parse changes introduced by this PR

Baseline pending for 57651955b5501d23add07901ea4649ef3f398b6b — this populates once main publishes its coverage snapshot (a few minutes after that commit landed).