diff --git a/crates/engine/src/game/interaction.rs b/crates/engine/src/game/interaction.rs index 0d769c6c2f..1c563d6d97 100644 --- a/crates/engine/src/game/interaction.rs +++ b/crates/engine/src/game/interaction.rs @@ -7924,6 +7924,21 @@ fn validate_response_bounds(response: &InteractionResponse) -> Result<(), Intera } } +/// Wire-boundary bounds for one inbound submission, evaluated without touching +/// game state. +/// +/// Transports call this at the wire so a rejection is answered before the +/// dispatcher does identity work; [`submit_interaction`] re-runs it via +/// [`resolve_interaction_response`], so no caller can skip it and no transport +/// can drift from these bounds by restating them. +pub fn bound_interaction_submission( + submission: &InteractionSubmission, +) -> Result<(), InteractionSubmitError> { + bound_string(submission.interaction_id.as_str())?; + validate_response_bounds(&submission.response)?; + Ok(()) +} + fn slot_for_submission<'a>( state: &'a GameState, actor: PlayerId, @@ -9321,8 +9336,7 @@ pub fn resolve_interaction_response( actor: PlayerId, submission: &InteractionSubmission, ) -> Result { - bound_string(submission.interaction_id.as_str())?; - validate_response_bounds(&submission.response)?; + bound_interaction_submission(submission)?; slot_for_submission(state, actor, &submission.interaction_id)?; let filtered = visibility::filter_state_for_viewer(state, actor); let (action, _) = materialize_response( diff --git a/crates/engine/tests/integration/interaction_contract.rs b/crates/engine/tests/integration/interaction_contract.rs index 30e8179b51..54211f8425 100644 --- a/crates/engine/tests/integration/interaction_contract.rs +++ b/crates/engine/tests/integration/interaction_contract.rs @@ -3021,3 +3021,87 @@ fn deck_partition_schema_publishes_an_interval_not_an_exact_deck_size() { 14 ); } + +/// The interaction contract omits a debug-capability gate at the transport +/// (`SessionManager::handle_interaction`) on the grounds that candidate +/// enumeration never produces one. This converts that "cannot happen" into +/// something that fails the day it starts happening. +/// +/// It asserts on the **client-visible** publication — `derive_viewer_interaction` +/// -> `opportunity_for_slot` -> `actor_candidates` -> `ai_support`'s validated +/// candidate set — rather than on an internal helper, so it covers what a +/// remote seat could actually submit. +/// +/// The sandbox capability is armed *fully* and deliberately: the claim is not +/// that debug actions are unreachable because sandbox mode is off, it is that +/// enumeration ignores the flag even when it is on. All three of +/// `allow_debug_actions`, `debug_mode`, and `debug_permitted` are set because +/// `apply`'s own gate requires the latter two together — arming only one would +/// leave the capability half-granted and the test could pass for the wrong +/// reason. +#[test] +fn published_interaction_choices_never_offer_a_debug_action_in_a_sandbox_game() { + let mut state = GameState::new_two_player(42); + state.format_config.allow_debug_actions = true; + state.debug_mode = true; + state.debug_permitted.insert(P0); + bind(&mut state, "sandbox-debug-enumeration"); + + let view = priority_view(&state); + + // Reach guard (1): a `ViewerInteraction` with `can_submit: false`, or a + // terminal `waiting_for`, publishes no opportunities at all and would + // satisfy the negative below vacuously. + assert!( + !view.opportunities.is_empty(), + "the fixture must publish something for the negative assertion to bite" + ); + + // Reach guard (3): the capability is genuinely in force at assertion time. + assert!( + state.format_config.allow_debug_actions + && state.debug_mode + && state.debug_permitted.contains(&P0), + "the sandbox capability must be armed, or this asserts nothing" + ); + + // Reach guard (2): `WaitingFor::Priority` maps to + // `HumanResponseModel::ExactCandidates`, which is the `actor_candidates` + // branch — the enumerator whose output this test is about. + assert!( + matches!(state.waiting_for, WaitingFor::Priority { .. }), + "the enumerating branch is selected by the waiting_for shape, got {:?}", + state.waiting_for + ); + + let mut saw_choices = false; + for opportunity in &view.opportunities { + let InteractionOpportunityResponse::ExactChoices { choices } = &opportunity.response else { + continue; + }; + saw_choices |= !choices.is_empty(); + for choice in choices { + for surface in &choice.surfaces { + if let InteractionPresentationSurface::Action { code, .. } = surface { + assert!( + !matches!( + code, + InteractionActionCode::Debug + | InteractionActionCode::GrantDebugPermission + | InteractionActionCode::RevokeDebugPermission + ), + "candidate enumeration published a debug action ({code:?}); \ + `SessionManager::handle_interaction`'s missing debug gate is \ + no longer safe" + ); + } + } + } + } + + assert!( + saw_choices, + "an ExactChoices opportunity with real choices is what proves the \ + actor_candidates path ran" + ); +} diff --git a/crates/phase-server/src/main.rs b/crates/phase-server/src/main.rs index 017a49221f..4b914fdd46 100644 --- a/crates/phase-server/src/main.rs +++ b/crates/phase-server/src/main.rs @@ -28,8 +28,10 @@ use engine::database::CardDatabase; use engine::game::derived_views::derive_filtered_views; use engine::game::interaction::{derive_viewer_interaction, object_action_payloads}; use engine::game::validate_name_deck_for_format_full; +use engine::types::actions::GameAction; use engine::types::events::GameEvent; use engine::types::game_state::GameState; +use engine::types::interaction::InteractionSubmission; use engine::types::player::PlayerId; use engine::types::GameLogEntry; use http::{HeaderMap, HeaderValue}; @@ -42,7 +44,7 @@ use seat_reducer::types::{DeckChoice, DeckResolver, ReducerCtx}; use server_core::ai_seats_wire_guard::{guard_create_ai_seats, MAX_FULL_GAME_PLAYER_COUNT}; use server_core::client_hello_guard::guard_client_hello; use server_core::client_message_wire_guard::{ - guard_broker_projection_inbound, guard_client_message_before_dispatch, + guard_broker_projection_inbound, guard_client_message_before_dispatch, wire_rejection_message, }; use server_core::draft_action_payload_guard::guard_draft_action_payload; use server_core::draft_session::{draft_seats_needing_auto_pick, DraftSessionManager}; @@ -56,6 +58,7 @@ use server_core::game_reconnect_guard::guard_game_reconnect; use server_core::game_state_snapshot_wire_guard::{ guard_game_state_for_broadcast, guard_state_snapshot_broadcast, StateSnapshotParts, }; +use server_core::interaction_payload_guard::guard_interaction_submission_payload; use server_core::legacy_deck_guard::guard_legacy_deck; use server_core::legacy_join_guard::guard_legacy_join_game; use server_core::lobby::RegisterGameRequest; @@ -865,6 +868,7 @@ fn reject_if_disabled(msg: &ClientMessage, mode: ServerMode) -> Option<&'static ClientMessage::CreateGame { .. } | ClientMessage::JoinGame { .. } | ClientMessage::Action { .. } + | ClientMessage::Interaction { .. } | ClientMessage::PreviewManaPayment { .. } | ClientMessage::Reconnect { .. } | ClientMessage::AbandonGame @@ -3535,6 +3539,467 @@ async fn broadcast_takeback_approved( } } +/// What a Full-mode game socket submitted: a client-materialized `GameAction`, +/// or an opaque engine-authored interaction response. Both are authenticated, +/// applied, and broadcast identically, so they share one handler. A bool or a +/// pair of `Option`s here would hide that the two are alternatives — and would +/// not give the payload-guard match below a total, wildcard-free form. +#[derive(Debug)] +enum GameSubmission { + Action(GameAction), + Interaction(InteractionSubmission), +} + +impl GameSubmission { + /// Wire-bounds this submission and, on failure, names the channel the + /// rejection is answered on. + /// + /// Defense in depth: `guard_client_message_before_dispatch` already ran + /// these exact bounds before dispatch, so in production this returns `Ok` + /// for every frame that reaches the handler — the same standing as the + /// `Action` guard it replaces. It is kept because the handler must not + /// assume its caller ran them. + /// + /// The channels here MUST agree with + /// `client_message_wire_guard::wire_rejection_message`, which answers the + /// same failure at the wire. An oversized `GameAction` is a malformed + /// frame — a client materializes actions from engine-published legal + /// actions and cannot produce one by accident. An oversized interaction + /// response is a rejected decision: `TextChoiceProjection::allow_arbitrary` + /// accepts free-form text and the bound is 256 bytes, so an ordinary paste + /// trips it and `ServerMessage::Error` would tear the session down. + /// `handler_payload_channels_agree_with_the_wire` pins that agreement + /// directly, by comparing this function's answer with + /// `wire_rejection_message`'s for the same payload. + /// + /// The `Err` is boxed because `ServerMessage` is ~13 KiB (it carries a + /// `GameState`), matching how this file already passes the type around + /// (`game_started_msg: Box`). + /// Stable `kind` label for the diagnostics this handler emits. + /// + /// Both submission variants share one handler, so an unlabelled event is + /// unattributable: an operator triaging an interaction report greps for + /// "interaction" and matches nothing. Deriving the label here keeps the two + /// call sites from restating the variant set. + fn kind(&self) -> &'static str { + match self { + GameSubmission::Action(_) => "action", + GameSubmission::Interaction(_) => "interaction", + } + } + + fn payload_rejection(&self) -> Result<(), Box> { + match self { + GameSubmission::Action(action) => guard_game_action_payload(action) + .map_err(|reason| Box::new(ServerMessage::error(reason))), + GameSubmission::Interaction(submission) => { + guard_interaction_submission_payload(submission) + .map_err(|reason| Box::new(ServerMessage::ActionRejected { reason })) + } + } + } +} + +/// Apply one authenticated game submission from a Full-mode game socket, then +/// broadcast the resulting state to every participant and spectator. +/// +/// Extracted verbatim from the `ClientMessage::Action` arm of +/// [`handle_client_message`] so that `ClientMessage::Interaction` can reuse the +/// identical authorization, application, and fan-out path instead of growing a +/// second ~400-line copy that would drift. +#[allow(clippy::too_many_arguments)] +async fn handle_full_game_submission( + submission: GameSubmission, + socket: &mut WebSocket, + state: &SharedState, + draft_state: &SharedDraftState, + connections: &SharedConnections, + game_db: &SharedGameDb, + game_spectators: &SharedGameSpectators, + // Read-only: this handler reads `game_code`, `player_token`, and `player_id` + // and mutates nothing. `&SocketIdentity` is deliberate, not an oversight -- + // `require_host`, `is_joining_current_game`, and their neighbours already + // take it by shared reference; only `dispatch_broker` and + // `handle_client_message` need `&mut`. Do not "fix" this to `&mut`. + identity: &SocketIdentity, +) { + let kind = submission.kind(); + let game_code = match &identity.game_code { + Some(c) => c.clone(), + None => { + warn!(kind, "game submission received but not in a game"); + let msg = ServerMessage::error("Not in a game".to_string()); + if let Ok(json) = serde_json::to_string(&msg) { + let _ = socket.send(Message::text(json)).await; + } + return; + } + }; + let player_token = match &identity.player_token { + Some(t) => t.clone(), + None => { + let msg = ServerMessage::error("No player token".to_string()); + if let Ok(json) = serde_json::to_string(&msg) { + let _ = socket.send(Message::text(json)).await; + } + return; + } + }; + + debug!(game = %game_code, player = ?identity.player_id, submission = ?submission, "game submission"); + + // Bound client-supplied payload sizes before the clone-heavy engine + // reducers process them (mirrors guard_draft_action_payload for draft + // actions). The channel each variant answers on is declared by + // `GameSubmission::payload_rejection`. + if let Err(msg) = submission.payload_rejection() { + if let Ok(json) = serde_json::to_string(&msg) { + let _ = socket.send(Message::text(json)).await; + } + return; + } + + // Apply human action and collect AI follow-up results while holding the lock. + // Filtering is deferred until after the lock is dropped to reduce contention. + let action_result = { + let lock_start = std::time::Instant::now(); + let mut mgr = state.lock().await; + let applied = match submission { + GameSubmission::Action(action) => mgr.handle_action(&game_code, &player_token, action), + GameSubmission::Interaction(submission) => { + mgr.handle_interaction(&game_code, &player_token, submission) + } + }; + match applied { + Ok(human_result) => { + let human_revision = mgr + .sessions + .get_mut(&game_code) + .expect("handled action must retain its session") + .advance_state_revision(); + // Run AI follow-up actions (still inside lock — needs &mut state) + let ai_results = match mgr.sessions.get_mut(&game_code) { + Some(session) => session.run_ai(), + None => vec![], + }; + let session = mgr.sessions.get(&game_code).unwrap(); + let eliminated = session.state.eliminated_players.clone(); + let player_count = session.player_count; + let game_over_winner = match &session.state.waiting_for { + engine::types::game_state::WaitingFor::GameOver { winner } => Some(*winner), + _ => None, + }; + let terminal = if let Some(winner) = game_over_winner { + info!(game = %game_code, winner = ?winner, reason = "game_rules", "game over"); + let ranked_result = ranked_duel_players(session).and_then(|players| { + ranked_result_for_duel(game_db, &game_code, &players, winner) + }); + terminal_artifact(session, winner, "Game ended".to_string(), ranked_result) + .map(Some) + } else { + persist_full_session_async(game_db, session); + Ok(None) + }; + + let lock_ms = lock_start.elapsed().as_millis(); + info!( + game = %game_code, + kind, + lock_ms, + ai_actions = ai_results.len(), + "game submission processed (lock held)" + ); + + terminal.map(|terminal| { + ( + human_revision, + human_result, + ai_results, + eliminated, + player_count, + game_over_winner, + terminal, + ) + }) + } + Err(e) => Err(e), + } + }; // lock dropped — filtering happens below without blocking other games + + match action_result { + Ok(( + human_revision, + ( + raw_state, + events, + legal_actions, + log_entries, + _auto_pass_rec, + spell_costs, + legal_actions_by_object, + ), + ai_results, + eliminated, + player_count, + game_over_winner, + terminal, + )) => { + if let Err(reason) = guard_state_snapshot_broadcast(StateSnapshotParts { + state: &raw_state, + events: &events, + log_entries: &log_entries, + legal_actions: &legal_actions, + legal_actions_by_object: &legal_actions_by_object, + spell_costs: &spell_costs, + }) { + warn!(game = %game_code, %reason, "action snapshot too large to broadcast"); + let msg = ServerMessage::error(reason); + if let Ok(json) = serde_json::to_string(&msg) { + let _ = socket.send(Message::text(json)).await; + } + return; + } + + let terminal_deliveries = match terminal { + Some(artifact) => match prepare_full_terminal(game_db, artifact).await { + Ok(deliveries) => deliveries, + Err(error) => { + error!(game = %game_code, %error, "terminal preparation failed"); + let msg = ServerMessage::error(error); + if let Ok(json) = serde_json::to_string(&msg) { + let _ = socket.send(Message::text(json)).await; + } + return; + } + }, + None => Vec::new(), + }; + + // Filter state per-player outside the lock + let filtered_states: Vec<(PlayerId, GameState)> = (0..player_count) + .map(|i| { + let pid = PlayerId(i); + (pid, server_core::filter_state_for_player(&raw_state, pid)) + }) + .collect(); + + // Broadcast human action result + { + let conns = connections.lock().await; + if let Some(players) = conns.get(&game_code) { + for (pid, pstate) in &filtered_states { + if let Some(s) = players.get(pid) { + let is_actor = server_core::is_acting(&raw_state, *pid); + let player_legals = if ai_results.is_empty() && is_actor { + legal_actions.clone() + } else { + // AI will act next — don't send legal actions yet + vec![] + }; + let p_auto_pass = if ai_results.is_empty() { + engine_auto_pass_for_viewer(&raw_state, *pid, &legal_actions) + } else { + false + }; + let p_end_continuous_effect_offers = + engine_end_continuous_effect_offers(&player_legals); + let p_mana_payment_shortcut_actions = + if ai_results.is_empty() && is_actor { + engine_mana_payment_shortcut_actions( + &raw_state, + &legal_actions_by_object, + ) + } else { + Vec::new() + }; + let p_spell_costs = if ai_results.is_empty() && is_actor { + spell_costs.clone() + } else { + HashMap::new() + }; + let p_by_object = if ai_results.is_empty() && is_actor { + legal_actions_by_object.clone() + } else { + HashMap::new() + }; + let _ = s.send(ServerMessage::StateUpdate { + state_revision: human_revision, + state: pstate.clone(), + events: server_core::filter_events_for_player( + &events, &raw_state, *pid, + ), + legal_actions: player_legals, + auto_pass_recommended: p_auto_pass, + end_continuous_effect_offers: p_end_continuous_effect_offers, + mana_payment_shortcut_actions: p_mana_payment_shortcut_actions, + eliminated_players: eliminated.clone(), + log_entries: log_entries.clone(), + spell_costs: p_spell_costs, + legal_actions_by_object: object_action_payloads(&p_by_object), + derived: derive_transport_views(&raw_state, pstate, Some(*pid)), + viewer_interaction: derive_viewer_interaction( + &raw_state, pstate, *pid, + ), + }); + } + } + } + } + if let Ok(spectator_msg) = build_spectator_state_update_message( + &raw_state, + &events, + &log_entries, + human_revision, + ) { + let mut specs = game_spectators.lock().await; + if let Some(spectators) = specs.get_mut(&game_code) { + spectators.retain(|sender| sender.send(spectator_msg.clone()).is_ok()); + if spectators.is_empty() { + specs.remove(&game_code); + } + } + } + + // Broadcast AI follow-up results with delays + for (i, (ai_revision, result)) in ai_results.iter().enumerate() { + tokio::time::sleep(std::time::Duration::from_millis(100)).await; + let ( + ai_raw_state, + ai_events, + ai_legal, + ai_log_entries, + _ai_auto_pass, + ai_spell_costs, + ai_by_object, + ) = result; + if guard_state_snapshot_broadcast(StateSnapshotParts { + state: ai_raw_state, + events: ai_events, + log_entries: ai_log_entries, + legal_actions: ai_legal, + legal_actions_by_object: ai_by_object, + spell_costs: ai_spell_costs, + }) + .is_err() + { + continue; + } + let is_last = i == ai_results.len() - 1; + + // Filter AI state per-player outside the lock + let ai_filtered: Vec<(PlayerId, GameState)> = (0..player_count) + .map(|j| { + let pid = PlayerId(j); + (pid, server_core::filter_state_for_player(ai_raw_state, pid)) + }) + .collect(); + + let conns = connections.lock().await; + if let Some(players) = conns.get(&game_code) { + for (pid, pstate) in &ai_filtered { + if let Some(s) = players.get(pid) { + let is_actor = server_core::is_acting(ai_raw_state, *pid); + let player_legals = if is_last && is_actor { + ai_legal.clone() + } else { + vec![] + }; + let p_auto_pass = if is_last { + engine_auto_pass_for_viewer(ai_raw_state, *pid, ai_legal) + } else { + false + }; + let p_end_continuous_effect_offers = + engine_end_continuous_effect_offers(&player_legals); + let p_mana_payment_shortcut_actions = if is_last && is_actor { + engine_mana_payment_shortcut_actions(ai_raw_state, ai_by_object) + } else { + Vec::new() + }; + let p_spell_costs = if is_last && is_actor { + ai_spell_costs.clone() + } else { + HashMap::new() + }; + let p_by_object = if is_last && is_actor { + ai_by_object.clone() + } else { + HashMap::new() + }; + let _ = s.send(ServerMessage::StateUpdate { + state_revision: *ai_revision, + state: pstate.clone(), + events: server_core::filter_events_for_player( + ai_events, + ai_raw_state, + *pid, + ), + legal_actions: player_legals, + auto_pass_recommended: p_auto_pass, + end_continuous_effect_offers: p_end_continuous_effect_offers, + mana_payment_shortcut_actions: p_mana_payment_shortcut_actions, + eliminated_players: eliminated.clone(), + log_entries: ai_log_entries.clone(), + spell_costs: p_spell_costs, + legal_actions_by_object: object_action_payloads(&p_by_object), + derived: derive_transport_views(ai_raw_state, pstate, Some(*pid)), + viewer_interaction: derive_viewer_interaction( + ai_raw_state, + pstate, + *pid, + ), + }); + } + } + } + let (ai_raw_state, ai_events, _, ai_log_entries, _, _, _) = result; + if let Ok(spectator_msg) = build_spectator_state_update_message( + ai_raw_state, + ai_events, + ai_log_entries, + *ai_revision, + ) { + let mut specs = game_spectators.lock().await; + if let Some(spectators) = specs.get_mut(&game_code) { + spectators.retain(|sender| sender.send(spectator_msg.clone()).is_ok()); + if spectators.is_empty() { + specs.remove(&game_code); + } + } + } + } + + if !terminal_deliveries.is_empty() { + let conns = connections.lock().await; + if let Some(players) = conns.get(&game_code) { + for (player, delivery) in &terminal_deliveries { + if let Some(sender) = players.get(player) { + let _ = sender.send(ServerMessage::TerminalResult { + delivery: Some(delivery.clone()), + }); + } + } + } + drop(conns); + report_draft_game_over( + draft_state, + connections, + &game_code, + game_over_winner.flatten(), + ) + .await; + state.lock().await.remove_game(&game_code); + } + } + Err(e) => { + let msg = ServerMessage::ActionRejected { reason: e }; + if let Ok(json) = serde_json::to_string(&msg) { + let _ = socket.send(Message::text(json)).await; + } + } + } +} + #[allow(clippy::too_many_arguments)] async fn handle_client_message( client_msg: ClientMessage, @@ -3632,7 +4097,10 @@ async fn handle_client_message( } if let Err(reason) = guard_client_message_before_dispatch(&client_msg, mode) { - let msg = ServerMessage::error(reason); + // The answer channel is wire policy, declared per variant alongside the + // bounds themselves. Every variant except `Interaction` keeps today's + // `ServerMessage::error`. + let msg = wire_rejection_message(&client_msg, reason); if let Ok(json) = serde_json::to_string(&msg) { let _ = socket.send(Message::text(json)).await; } @@ -3895,404 +4363,31 @@ async fn handle_client_message( } ClientMessage::Action { action } => { - let game_code = match &identity.game_code { - Some(c) => c.clone(), - None => { - warn!("Action received but not in a game"); - let msg = ServerMessage::error("Not in a game".to_string()); - if let Ok(json) = serde_json::to_string(&msg) { - let _ = socket.send(Message::text(json)).await; - } - return; - } - }; - let player_token = match &identity.player_token { - Some(t) => t.clone(), - None => { - let msg = ServerMessage::error("No player token".to_string()); - if let Ok(json) = serde_json::to_string(&msg) { - let _ = socket.send(Message::text(json)).await; - } - return; - } - }; - - debug!(game = %game_code, player = ?identity.player_id, action = ?action, "Action"); - - // Bound client-supplied action payload sizes before the clone-heavy - // engine reducers process them (mirrors guard_draft_action_payload - // for draft actions). - if let Err(reason) = guard_game_action_payload(&action) { - let msg = ServerMessage::error(reason); - if let Ok(json) = serde_json::to_string(&msg) { - let _ = socket.send(Message::text(json)).await; - } - return; - } - - // Apply human action and collect AI follow-up results while holding the lock. - // Filtering is deferred until after the lock is dropped to reduce contention. - let action_result = { - let lock_start = std::time::Instant::now(); - let mut mgr = state.lock().await; - match mgr.handle_action(&game_code, &player_token, action) { - Ok(human_result) => { - let human_revision = mgr - .sessions - .get_mut(&game_code) - .expect("handled action must retain its session") - .advance_state_revision(); - // Run AI follow-up actions (still inside lock — needs &mut state) - let ai_results = match mgr.sessions.get_mut(&game_code) { - Some(session) => session.run_ai(), - None => vec![], - }; - let session = mgr.sessions.get(&game_code).unwrap(); - let eliminated = session.state.eliminated_players.clone(); - let player_count = session.player_count; - let game_over_winner = match &session.state.waiting_for { - engine::types::game_state::WaitingFor::GameOver { winner } => { - Some(*winner) - } - _ => None, - }; - let terminal = if let Some(winner) = game_over_winner { - info!(game = %game_code, winner = ?winner, reason = "game_rules", "game over"); - let ranked_result = ranked_duel_players(session).and_then(|players| { - ranked_result_for_duel(game_db, &game_code, &players, winner) - }); - terminal_artifact( - session, - winner, - "Game ended".to_string(), - ranked_result, - ) - .map(Some) - } else { - persist_full_session_async(game_db, session); - Ok(None) - }; - - let lock_ms = lock_start.elapsed().as_millis(); - info!( - game = %game_code, - lock_ms, - ai_actions = ai_results.len(), - "action processed (lock held)" - ); - - terminal.map(|terminal| { - ( - human_revision, - human_result, - ai_results, - eliminated, - player_count, - game_over_winner, - terminal, - ) - }) - } - Err(e) => Err(e), - } - }; // lock dropped — filtering happens below without blocking other games - - match action_result { - Ok(( - human_revision, - ( - raw_state, - events, - legal_actions, - log_entries, - _auto_pass_rec, - spell_costs, - legal_actions_by_object, - ), - ai_results, - eliminated, - player_count, - game_over_winner, - terminal, - )) => { - if let Err(reason) = guard_state_snapshot_broadcast(StateSnapshotParts { - state: &raw_state, - events: &events, - log_entries: &log_entries, - legal_actions: &legal_actions, - legal_actions_by_object: &legal_actions_by_object, - spell_costs: &spell_costs, - }) { - warn!(game = %game_code, %reason, "action snapshot too large to broadcast"); - let msg = ServerMessage::error(reason); - if let Ok(json) = serde_json::to_string(&msg) { - let _ = socket.send(Message::text(json)).await; - } - return; - } - - let terminal_deliveries = match terminal { - Some(artifact) => match prepare_full_terminal(game_db, artifact).await { - Ok(deliveries) => deliveries, - Err(error) => { - error!(game = %game_code, %error, "terminal preparation failed"); - let msg = ServerMessage::error(error); - if let Ok(json) = serde_json::to_string(&msg) { - let _ = socket.send(Message::text(json)).await; - } - return; - } - }, - None => Vec::new(), - }; - - // Filter state per-player outside the lock - let filtered_states: Vec<(PlayerId, GameState)> = (0..player_count) - .map(|i| { - let pid = PlayerId(i); - (pid, server_core::filter_state_for_player(&raw_state, pid)) - }) - .collect(); - - // Broadcast human action result - { - let conns = connections.lock().await; - if let Some(players) = conns.get(&game_code) { - for (pid, pstate) in &filtered_states { - if let Some(s) = players.get(pid) { - let is_actor = server_core::is_acting(&raw_state, *pid); - let player_legals = if ai_results.is_empty() && is_actor { - legal_actions.clone() - } else { - // AI will act next — don't send legal actions yet - vec![] - }; - let p_auto_pass = if ai_results.is_empty() { - engine_auto_pass_for_viewer( - &raw_state, - *pid, - &legal_actions, - ) - } else { - false - }; - let p_end_continuous_effect_offers = - engine_end_continuous_effect_offers(&player_legals); - let p_mana_payment_shortcut_actions = - if ai_results.is_empty() && is_actor { - engine_mana_payment_shortcut_actions( - &raw_state, - &legal_actions_by_object, - ) - } else { - Vec::new() - }; - let p_spell_costs = if ai_results.is_empty() && is_actor { - spell_costs.clone() - } else { - HashMap::new() - }; - let p_by_object = if ai_results.is_empty() && is_actor { - legal_actions_by_object.clone() - } else { - HashMap::new() - }; - let _ = s.send(ServerMessage::StateUpdate { - state_revision: human_revision, - state: pstate.clone(), - events: server_core::filter_events_for_player( - &events, &raw_state, *pid, - ), - legal_actions: player_legals, - auto_pass_recommended: p_auto_pass, - end_continuous_effect_offers: - p_end_continuous_effect_offers, - mana_payment_shortcut_actions: - p_mana_payment_shortcut_actions, - eliminated_players: eliminated.clone(), - log_entries: log_entries.clone(), - spell_costs: p_spell_costs, - legal_actions_by_object: object_action_payloads( - &p_by_object, - ), - derived: derive_transport_views( - &raw_state, - pstate, - Some(*pid), - ), - viewer_interaction: derive_viewer_interaction( - &raw_state, pstate, *pid, - ), - }); - } - } - } - } - if let Ok(spectator_msg) = build_spectator_state_update_message( - &raw_state, - &events, - &log_entries, - human_revision, - ) { - let mut specs = game_spectators.lock().await; - if let Some(spectators) = specs.get_mut(&game_code) { - spectators.retain(|sender| sender.send(spectator_msg.clone()).is_ok()); - if spectators.is_empty() { - specs.remove(&game_code); - } - } - } - - // Broadcast AI follow-up results with delays - for (i, (ai_revision, result)) in ai_results.iter().enumerate() { - tokio::time::sleep(std::time::Duration::from_millis(100)).await; - let ( - ai_raw_state, - ai_events, - ai_legal, - ai_log_entries, - _ai_auto_pass, - ai_spell_costs, - ai_by_object, - ) = result; - if guard_state_snapshot_broadcast(StateSnapshotParts { - state: ai_raw_state, - events: ai_events, - log_entries: ai_log_entries, - legal_actions: ai_legal, - legal_actions_by_object: ai_by_object, - spell_costs: ai_spell_costs, - }) - .is_err() - { - continue; - } - let is_last = i == ai_results.len() - 1; - - // Filter AI state per-player outside the lock - let ai_filtered: Vec<(PlayerId, GameState)> = (0..player_count) - .map(|j| { - let pid = PlayerId(j); - (pid, server_core::filter_state_for_player(ai_raw_state, pid)) - }) - .collect(); - - let conns = connections.lock().await; - if let Some(players) = conns.get(&game_code) { - for (pid, pstate) in &ai_filtered { - if let Some(s) = players.get(pid) { - let is_actor = server_core::is_acting(ai_raw_state, *pid); - let player_legals = if is_last && is_actor { - ai_legal.clone() - } else { - vec![] - }; - let p_auto_pass = if is_last { - engine_auto_pass_for_viewer(ai_raw_state, *pid, ai_legal) - } else { - false - }; - let p_end_continuous_effect_offers = - engine_end_continuous_effect_offers(&player_legals); - let p_mana_payment_shortcut_actions = if is_last && is_actor { - engine_mana_payment_shortcut_actions( - ai_raw_state, - ai_by_object, - ) - } else { - Vec::new() - }; - let p_spell_costs = if is_last && is_actor { - ai_spell_costs.clone() - } else { - HashMap::new() - }; - let p_by_object = if is_last && is_actor { - ai_by_object.clone() - } else { - HashMap::new() - }; - let _ = s.send(ServerMessage::StateUpdate { - state_revision: *ai_revision, - state: pstate.clone(), - events: server_core::filter_events_for_player( - ai_events, - ai_raw_state, - *pid, - ), - legal_actions: player_legals, - auto_pass_recommended: p_auto_pass, - end_continuous_effect_offers: - p_end_continuous_effect_offers, - mana_payment_shortcut_actions: - p_mana_payment_shortcut_actions, - eliminated_players: eliminated.clone(), - log_entries: ai_log_entries.clone(), - spell_costs: p_spell_costs, - legal_actions_by_object: object_action_payloads( - &p_by_object, - ), - derived: derive_transport_views( - ai_raw_state, - pstate, - Some(*pid), - ), - viewer_interaction: derive_viewer_interaction( - ai_raw_state, - pstate, - *pid, - ), - }); - } - } - } - let (ai_raw_state, ai_events, _, ai_log_entries, _, _, _) = result; - if let Ok(spectator_msg) = build_spectator_state_update_message( - ai_raw_state, - ai_events, - ai_log_entries, - *ai_revision, - ) { - let mut specs = game_spectators.lock().await; - if let Some(spectators) = specs.get_mut(&game_code) { - spectators - .retain(|sender| sender.send(spectator_msg.clone()).is_ok()); - if spectators.is_empty() { - specs.remove(&game_code); - } - } - } - } + handle_full_game_submission( + GameSubmission::Action(action), + socket, + state, + draft_state, + connections, + game_db, + game_spectators, + identity, + ) + .await; + } - if !terminal_deliveries.is_empty() { - let conns = connections.lock().await; - if let Some(players) = conns.get(&game_code) { - for (player, delivery) in &terminal_deliveries { - if let Some(sender) = players.get(player) { - let _ = sender.send(ServerMessage::TerminalResult { - delivery: Some(delivery.clone()), - }); - } - } - } - drop(conns); - report_draft_game_over( - draft_state, - connections, - &game_code, - game_over_winner.flatten(), - ) - .await; - state.lock().await.remove_game(&game_code); - } - } - Err(e) => { - let msg = ServerMessage::ActionRejected { reason: e }; - if let Ok(json) = serde_json::to_string(&msg) { - let _ = socket.send(Message::text(json)).await; - } - } - } + ClientMessage::Interaction { submission } => { + handle_full_game_submission( + GameSubmission::Interaction(submission), + socket, + state, + draft_state, + connections, + game_db, + game_spectators, + identity, + ) + .await; } ClientMessage::Reconnect { @@ -7653,7 +7748,8 @@ mod issue_4548_full_create_tests { } } - async fn spawn_full_mode_server() -> (String, tokio::task::JoinHandle<()>, tempfile::TempDir) { + pub(super) async fn spawn_full_mode_server( + ) -> (String, tokio::task::JoinHandle<()>, tempfile::TempDir) { let temp_dir = tempfile::tempdir().expect("temp dir"); let game_db = Arc::new( persistence::GameDb::open( @@ -7691,7 +7787,7 @@ mod issue_4548_full_create_tests { (format!("ws://{addr}/ws"), handle, temp_dir) } - async fn recv_server_message(socket: &mut WebSocketStream) -> ServerMessage + pub(super) async fn recv_server_message(socket: &mut WebSocketStream) -> ServerMessage where S: AsyncRead + AsyncWrite + Unpin, { @@ -7926,6 +8022,404 @@ mod issue_4548_full_create_tests { } } +/// End-to-end coverage for the shared game-submission handler +/// ([`handle_full_game_submission`]) over a real socket. +/// +/// Before this module existed, `ClientMessage::Action` had **no** end-to-end +/// test through `handle_client_message` at all: every `ClientMessage::Action` +/// occurrence in this file's test modules exercised `reject_if_disabled` or +/// `classify_hello_gate` as pure functions. These tests are what gate the +/// extraction of that arm into a shared handler. +#[cfg(test)] +mod game_submission_tests { + use super::issue_4548_full_create_tests::{recv_server_message, spawn_full_mode_server}; + use super::*; + use engine::game::interaction::MAX_INTERACTION_STRING_LEN; + use engine::types::interaction::{InteractionChoiceId, InteractionId, InteractionResponse}; + use futures_util::SinkExt; + use server_core::game_action_payload_guard::MAX_ACTION_LIST_LEN; + use server_core::protocol::DeckData; + use tokio_tungstenite::tungstenite::Message as WsMessage; + use tokio_tungstenite::MaybeTlsStream; + use tokio_tungstenite::WebSocketStream; + + /// Connect, handshake, and create a two-seat game so the socket carries an + /// authenticated `SocketIdentity` with both a `game_code` and a + /// `player_token`. + /// + /// Seat 1 never joins, so the game never *starts* — which is exactly the + /// reachable surface these tests need: everything up to and including + /// `SessionManager`'s verdict and the `Err(e) => ActionRejected` arm. The + /// `Ok(..)` broadcast fan-out is not reachable over this harness, because + /// `spawn_full_mode_server` builds `AppState` with an empty + /// `CardDatabase::default()`. + async fn create_authenticated_game_socket( + url: String, + ) -> WebSocketStream> { + let (mut socket, _) = tokio_tungstenite::connect_async(url) + .await + .expect("connect"); + + assert!(matches!( + recv_server_message(&mut socket).await, + ServerMessage::ServerHello { .. } + )); + + let hello = ClientMessage::ClientHello { + client_version: env!("CARGO_PKG_VERSION").to_string(), + build_commit: build_commit().to_string(), + protocol_version: PROTOCOL_VERSION, + }; + socket + .send(WsMessage::Text( + serde_json::to_string(&hello).expect("hello json").into(), + )) + .await + .expect("send hello"); + + let create = ClientMessage::CreateGameWithSettings { + deck: DeckData::default(), + display_name: "Alice".to_string(), + public: false, + password: None, + timer_seconds: None, + player_count: 2, + match_config: Default::default(), + ai_seats: Vec::new(), + format_config: None, + room_name: None, + host_peer_id: None, + draft_metadata: None, + start_when_full: true, + ranked: false, + }; + socket + .send(WsMessage::Text( + serde_json::to_string(&create).expect("create json").into(), + )) + .await + .expect("send create"); + + let mut saw_created = false; + let mut saw_slots = false; + while !saw_created || !saw_slots { + match recv_server_message(&mut socket).await { + ServerMessage::GameCreated { .. } => saw_created = true, + ServerMessage::PlayerSlotsUpdate { .. } => saw_slots = true, + _ => {} + } + } + + socket + } + + /// Read frames until one is an `ActionRejected` or an `Error`, ignoring the + /// unrelated broadcasts the session emits. The enclosing + /// `tokio::time::timeout` is the failure mode, as in every sibling test. + async fn recv_submission_answer(socket: &mut WebSocketStream) -> ServerMessage + where + S: tokio::io::AsyncRead + tokio::io::AsyncWrite + Unpin, + { + loop { + let msg = recv_server_message(socket).await; + if matches!( + msg, + ServerMessage::ActionRejected { .. } | ServerMessage::Error { .. } + ) { + return msg; + } + } + } + + /// Gates the extraction of the `ClientMessage::Action` arm body into + /// [`handle_full_game_submission`]: an extraction that broke identity + /// extraction, the payload guard, the lock, `player_for_token`, or the + /// `Err(e) => ActionRejected` arm fails here. + /// + /// `GrantDebugPermission` is chosen over `PassPriority` deliberately. + /// `GameState::new` initializes `waiting_for: WaitingFor::Priority` with the + /// host holding priority, so a `PassPriority` from this socket may well be + /// *accepted* — which would make the assertion pass for the wrong reason. + /// The sandbox refusal, by contrast, is decidable without running the + /// engine: the action is Full-mode allowed, is a payloadless no-op in + /// `guard_game_action_payload`, and hits `handle_action`'s Grant/Revoke gate + /// *first* — before the seat check, before `debug_permitted`, and before + /// `apply` — where `format_config.allow_debug_actions` is `false` because + /// the wire sent `format_config: None` and `FormatConfig::standard()` sets + /// it to `false`. `handle_action`'s `Err(String)` is then answered verbatim, + /// with no `"Engine error: "` prefix. + #[tokio::test] + async fn action_frame_reaches_the_shared_submission_handler() { + let (url, server, _temp_dir) = spawn_full_mode_server().await; + let answer = tokio::time::timeout(Duration::from_secs(5), async { + let mut socket = create_authenticated_game_socket(url).await; + + let action = ClientMessage::Action { + action: GameAction::GrantDebugPermission { + player_id: PlayerId(1), + }, + }; + socket + .send(WsMessage::Text( + serde_json::to_string(&action).expect("action json").into(), + )) + .await + .expect("send action"); + + recv_submission_answer(&mut socket).await + }) + .await; + server.abort(); + + let answer = answer.expect("action frame was never answered"); + match answer { + ServerMessage::ActionRejected { reason } => { + assert_eq!(reason, "Sandbox mode is not enabled for this game"); + } + other => panic!("expected ActionRejected from the shared handler, got {other:?}"), + } + } + + fn submission(response: InteractionResponse) -> InteractionSubmission { + InteractionSubmission { + interaction_id: InteractionId("interaction-1".to_string()), + response, + } + } + + fn oversized_submission() -> InteractionSubmission { + submission(InteractionResponse::Text { + value: "x".repeat(MAX_INTERACTION_STRING_LEN + 1), + }) + } + + /// The real cross-check of the two independent channel declarations. + /// + /// Both `GameSubmission::payload_rejection` and `wire_rejection_message` + /// are pure functions, so they can be compared directly. An end-to-end + /// socket test structurally cannot do this: the wire guard returns before + /// dispatch, so no frame ever reaches both layers. + #[test] + fn handler_payload_channels_agree_with_the_wire() { + let oversized = oversized_submission(); + let oversized_action = GameAction::ReorderHand { + order: vec![engine::types::identifiers::ObjectId(1); MAX_ACTION_LIST_LEN + 1], + }; + + // (i) an oversized interaction answers on the benign channel. + let handler_reason = match *GameSubmission::Interaction(oversized.clone()) + .payload_rejection() + .expect_err("an oversized interaction is refused") + { + ServerMessage::ActionRejected { reason } => reason, + ref other => panic!("an oversized paste must not tear the session down: {other:?}"), + }; + + // (ii) an oversized action stays a malformed frame. + let action_reason = match *GameSubmission::Action(oversized_action.clone()) + .payload_rejection() + .expect_err("an oversized action is refused") + { + ServerMessage::Error { message, .. } => message, + ref other => panic!("an oversized action is a malformed frame, got {other:?}"), + }; + + // (iii) both layers agree, in variant *and* in reason string. + let wire_interaction = ClientMessage::Interaction { + submission: oversized, + }; + let wire_reason = + guard_client_message_before_dispatch(&wire_interaction, ServerMode::Full).unwrap_err(); + match wire_rejection_message(&wire_interaction, wire_reason) { + ServerMessage::ActionRejected { reason } => assert_eq!(reason, handler_reason), + other => panic!("wire and handler disagree on the interaction channel: {other:?}"), + } + + let wire_action = ClientMessage::Action { + action: oversized_action, + }; + let wire_action_reason = + guard_client_message_before_dispatch(&wire_action, ServerMode::Full).unwrap_err(); + match wire_rejection_message(&wire_action, wire_action_reason) { + ServerMessage::Error { message, .. } => assert_eq!(message, action_reason), + other => panic!("wire and handler disagree on the action channel: {other:?}"), + } + + // Reach guard: without it, a `payload_rejection` that always errored + // would satisfy (i) and (ii). + assert!( + GameSubmission::Interaction(submission(InteractionResponse::Choose { + choice_id: InteractionChoiceId("a".to_string()), + })) + .payload_rejection() + .is_ok() + ); + assert!(GameSubmission::Action(GameAction::PassPriority) + .payload_rejection() + .is_ok()); + } + + /// The revert-failing assertion for #6941. + /// + /// Before the fix, the frame never reaches a handler at all: serde answers + /// `ServerMessage::Error { message: "Invalid message: unknown variant + /// `Interaction` ..." }` — a different variant *and* a different string. + /// + /// Asserting the exact reason is the reach guard: only a frame that + /// traversed serde, `reject_if_disabled`, the wire guard, both identity + /// checks, `payload_rejection`, `state.lock()`, `player_for_token`, + /// `submit_interaction`, and `slot_for_submission`'s + /// `.ok_or(StaleInteraction)` can produce it. + #[tokio::test] + async fn interaction_frame_is_accepted_by_the_wire_schema() { + let (url, server, _temp_dir) = spawn_full_mode_server().await; + let answer = tokio::time::timeout(Duration::from_secs(5), async { + let mut socket = create_authenticated_game_socket(url).await; + + let frame = ClientMessage::Interaction { + submission: submission(InteractionResponse::Choose { + choice_id: InteractionChoiceId("no-such-choice".to_string()), + }), + }; + socket + .send(WsMessage::Text( + serde_json::to_string(&frame) + .expect("interaction json") + .into(), + )) + .await + .expect("send interaction"); + + recv_submission_answer(&mut socket).await + }) + .await; + server.abort(); + + let answer = answer.expect("interaction frame was never answered"); + match answer { + ServerMessage::ActionRejected { reason } => { + assert_eq!(reason, "Engine error: StaleInteraction"); + } + other => panic!("the wire schema must accept an Interaction frame, got {other:?}"), + } + } + + /// Scope note: this exercises the **wire** layer only — the guard returns + /// before dispatch, so the handler's `payload_rejection` is never reached + /// by this frame. It makes no claim about the handler layer; that agreement + /// is pinned by `handler_payload_channels_agree_with_the_wire`. + #[tokio::test] + async fn an_oversized_interaction_is_answered_on_the_benign_channel() { + let (url, server, _temp_dir) = spawn_full_mode_server().await; + let answers = tokio::time::timeout(Duration::from_secs(5), async { + let mut socket = create_authenticated_game_socket(url).await; + + let frame = ClientMessage::Interaction { + submission: oversized_submission(), + }; + socket + .send(WsMessage::Text( + serde_json::to_string(&frame) + .expect("oversized json") + .into(), + )) + .await + .expect("send oversized interaction"); + + let first = recv_submission_answer(&mut socket).await; + + // Liveness probe rather than a vacuous `!matches!`: a socket that + // had been answered with `ServerMessage::Error` would have been + // torn down client-side, so a second answer on the same socket is + // what proves the benign channel was used. + let bounded = ClientMessage::Interaction { + submission: submission(InteractionResponse::Choose { + choice_id: InteractionChoiceId("no-such-choice".to_string()), + }), + }; + socket + .send(WsMessage::Text( + serde_json::to_string(&bounded) + .expect("bounded json") + .into(), + )) + .await + .expect("send bounded interaction"); + + (first, recv_submission_answer(&mut socket).await) + }) + .await; + server.abort(); + + let (first, second) = answers.expect("oversized interaction was never answered"); + match first { + ServerMessage::ActionRejected { reason } => { + assert_eq!(reason, "Engine error: PayloadTooLarge"); + } + other => panic!("an oversized paste must not end the match, got {other:?}"), + } + assert!( + matches!(second, ServerMessage::ActionRejected { .. }), + "the socket must still be live after a bounds rejection, got {second:?}" + ); + } + + /// Pins the pre-session rows of the channel table, and is the discriminator + /// for `interaction_frame_is_accepted_by_the_wire_schema`: it proves that + /// test's `ActionRejected` came from an engine verdict rather than being + /// this handler's blanket answer. + #[tokio::test] + async fn a_game_submission_without_a_session_is_answered_on_the_error_channel() { + let (url, server, _temp_dir) = spawn_full_mode_server().await; + let answer = tokio::time::timeout(Duration::from_secs(5), async { + let (mut socket, _) = tokio_tungstenite::connect_async(url) + .await + .expect("connect"); + + assert!(matches!( + recv_server_message(&mut socket).await, + ServerMessage::ServerHello { .. } + )); + + let hello = ClientMessage::ClientHello { + client_version: env!("CARGO_PKG_VERSION").to_string(), + build_commit: build_commit().to_string(), + protocol_version: PROTOCOL_VERSION, + }; + socket + .send(WsMessage::Text( + serde_json::to_string(&hello).expect("hello json").into(), + )) + .await + .expect("send hello"); + + let frame = ClientMessage::Interaction { + submission: submission(InteractionResponse::Choose { + choice_id: InteractionChoiceId("a".to_string()), + }), + }; + socket + .send(WsMessage::Text( + serde_json::to_string(&frame) + .expect("interaction json") + .into(), + )) + .await + .expect("send interaction"); + + recv_submission_answer(&mut socket).await + }) + .await; + server.abort(); + + let answer = answer.expect("sessionless interaction was never answered"); + match answer { + ServerMessage::Error { message, .. } => assert_eq!(message, "Not in a game"), + other => panic!("a pre-session condition is not an engine verdict: {other:?}"), + } + } +} + #[cfg(test)] mod mode_gate_tests { use super::*; @@ -8093,6 +8587,40 @@ mod mode_gate_tests { assert!(reject_if_disabled(&m, ServerMode::Full).is_none()); } } + + fn interaction_frame() -> ClientMessage { + ClientMessage::Interaction { + submission: InteractionSubmission { + interaction_id: engine::types::interaction::InteractionId("i-1".to_string()), + response: engine::types::interaction::InteractionResponse::Choose { + choice_id: engine::types::interaction::InteractionChoiceId("a".to_string()), + }, + }, + } + } + + /// Both halves are required: either alone is satisfiable by a wrong + /// grouping. A LobbyOnly broker runs no engine and holds no + /// `SessionManager`, so it publishes no interactions and no client can hold + /// a live `interaction_id` against it — identical to `Action`. + #[test] + fn interaction_is_full_only() { + assert!(reject_if_disabled(&interaction_frame(), ServerMode::Full).is_none()); + assert!(reject_if_disabled(&interaction_frame(), ServerMode::LobbyOnly).is_some()); + } + + /// Supplies the other half of `server-core`'s + /// `broker_projection_accepts_an_interaction_without_bounding_it`: leaving + /// the projection guard unbounded for this variant is safe only because + /// nothing is ever cloned into the broker. + /// + /// The paired positive is required in the same test so a wholesale-`None` + /// regression in `to_lobby_client_message` cannot satisfy it. + #[test] + fn interaction_is_never_projected_into_the_lobby_broker() { + assert!(to_lobby_client_message(&interaction_frame()).is_none()); + assert!(to_lobby_client_message(&ClientMessage::SubscribeLobby).is_some()); + } } #[cfg(test)] diff --git a/crates/server-core/src/client_message_wire_guard.rs b/crates/server-core/src/client_message_wire_guard.rs index 26f497c7e4..6be4a9a5b6 100644 --- a/crates/server-core/src/client_message_wire_guard.rs +++ b/crates/server-core/src/client_message_wire_guard.rs @@ -2,8 +2,15 @@ //! variant before handler dispatch and broker projection clones. //! //! Individual handlers still run their guards for defense in depth; this layer -//! guarantees a single exhaustive match so new variants must declare wire policy +//! guarantees that every wire policy is declared in an exhaustive, wildcard-free +//! match, so a new `ClientMessage` variant cannot compile until it states one, //! and broker-projected frames are bounded before `to_lobby_client_message` clones. +//! +//! Three such matches live here, one per policy axis: +//! [`guard_client_message_before_dispatch`] (payload bounding), +//! [`wire_rejection_message`] (which channel a rejection is answered on), and +//! [`guard_broker_projection_inbound`] (broker projection). A new variant must +//! declare a policy in all three. use lobby_broker::inbound_guard::{ guard_create_game_settings_inbound, guard_join_game_with_password_inbound, @@ -25,9 +32,10 @@ use crate::draft_wire_guard::{ use crate::emote_guard::guard_emote; use crate::game_action_payload_guard::guard_game_action_payload; use crate::game_reconnect_guard::guard_game_reconnect; +use crate::interaction_payload_guard::guard_interaction_submission_payload; use crate::legacy_deck_guard::guard_legacy_deck; use crate::legacy_join_guard::guard_legacy_join_game; -use crate::protocol::{ClientMessage, ServerMode}; +use crate::protocol::{ClientMessage, ServerMessage, ServerMode}; use crate::seat_mutation_wire_guard::guard_seat_mutation; use crate::spectator_wire_guard::{guard_spectate_draft, guard_spectator_join}; @@ -50,6 +58,9 @@ pub fn guard_client_message_before_dispatch( ClientMessage::Action { action } | ClientMessage::PreviewManaPayment { action, .. } => { guard_game_action_payload(action) } + ClientMessage::Interaction { submission } => { + guard_interaction_submission_payload(submission) + } ClientMessage::Reconnect { game_code, player_token, @@ -193,6 +204,59 @@ pub fn guard_client_message_before_dispatch( } } +/// Answer a frame that [`guard_client_message_before_dispatch`] rejected, on +/// the channel that frame's variant declares. +/// +/// Exhaustive by design, like the two sibling matches in this module: a new +/// variant must declare not only *which* bounds apply at the wire, but *how a +/// rejection is answered*. The native client disposes its adapter on ANY +/// `ServerMessage::Error`, so any variant whose wire bounds a routine, +/// non-hostile client can trip MUST answer on `ActionRejected`. +pub fn wire_rejection_message(msg: &ClientMessage, reason: String) -> ServerMessage { + match msg { + // An oversized interaction response is reachable without hostility: + // `TextChoiceProjection::allow_arbitrary` accepts free-form text and + // `MAX_INTERACTION_STRING_LEN` is 256, so a long paste is a rejected + // decision, not a malformed frame. `ServerMessage::error` here would + // end the match on a paste. + ClientMessage::Interaction { .. } => ServerMessage::ActionRejected { reason }, + + // Every other variant keeps today's behavior exactly: a bounds failure + // on these is a malformed frame, not a rejected decision. + ClientMessage::ClientHello { .. } + | ClientMessage::CreateGame { .. } + | ClientMessage::JoinGame { .. } + | ClientMessage::Action { .. } + | ClientMessage::PreviewManaPayment { .. } + | ClientMessage::Reconnect { .. } + | ClientMessage::AbandonGame + | ClientMessage::SubscribeLobby + | ClientMessage::UnsubscribeLobby + | ClientMessage::CreateGameWithSettings { .. } + | ClientMessage::JoinGameWithPassword { .. } + | ClientMessage::LookupJoinTarget { .. } + | ClientMessage::Concede + | ClientMessage::ConcedeMatch + | ClientMessage::BootstrapTerminalDelivery { .. } + | ClientMessage::ReadTerminalResult { .. } + | ClientMessage::AckTerminalDelivery { .. } + | ClientMessage::Emote { .. } + | ClientMessage::SpectatorJoin { .. } + | ClientMessage::Ping { .. } + | ClientMessage::UpdateLobbyMetadata { .. } + | ClientMessage::SeatMutate { .. } + | ClientMessage::UnregisterLobby { .. } + | ClientMessage::CreateDraftWithSettings { .. } + | ClientMessage::JoinDraftWithPassword { .. } + | ClientMessage::DraftAction { .. } + | ClientMessage::ReconnectDraft { .. } + | ClientMessage::SpectateDraft { .. } + | ClientMessage::RequestTakeback + | ClientMessage::RespondTakeback { .. } + | ClientMessage::CancelTakeback => ServerMessage::error(reason), + } +} + /// Validate broker-projected lobby frames without constructing `LobbyClientMessage`. /// /// Used by `dispatch_broker` before `to_lobby_client_message` clones strings and @@ -269,6 +333,7 @@ pub fn guard_broker_projection_inbound(msg: &ClientMessage) -> Result<(), String ClientMessage::CreateGame { .. } | ClientMessage::JoinGame { .. } | ClientMessage::Action { .. } + | ClientMessage::Interaction { .. } | ClientMessage::PreviewManaPayment { .. } | ClientMessage::Reconnect { .. } | ClientMessage::AbandonGame @@ -298,6 +363,10 @@ mod tests { use engine::types::ability::{TriggerBaseSetInstanceRef, TriggerDefinitionOccurrenceRef}; use engine::types::game_state::ProductionOverride; use engine::types::identifiers::ObjectIncarnationRef; + use engine::types::interaction::{ + InteractionChoiceId, InteractionId, InteractionResponse, InteractionSubmission, + MAX_INTERACTION_LIST_LEN, + }; use engine::types::mana::{ ManaRestriction, ManaSourcePenalty, ManaSourceSelection, ManaType, TapsForManaSelection, }; @@ -427,4 +496,86 @@ mod tests { let err = guard_client_message_before_dispatch(&msg, ServerMode::Full).unwrap_err(); assert!(err.contains("game_code")); } + + fn interaction_frame(response: InteractionResponse) -> ClientMessage { + ClientMessage::Interaction { + submission: InteractionSubmission { + interaction_id: InteractionId("interaction-1".to_string()), + response, + }, + } + } + + fn oversized_interaction_frame() -> ClientMessage { + interaction_frame(InteractionResponse::Select { + choice_ids: vec![InteractionChoiceId("a".to_string()); MAX_INTERACTION_LIST_LEN + 1], + }) + } + + /// Direct sibling of `dispatch_guard_rejects_oversized_game_action_before_handler_work`. + #[test] + fn dispatch_guard_rejects_oversized_interaction_before_handler_work() { + let err = + guard_client_message_before_dispatch(&oversized_interaction_frame(), ServerMode::Full) + .unwrap_err(); + + assert!(err.contains("PayloadTooLarge"), "unexpected reason: {err}"); + } + + /// Non-vacuity guard for the test above, and a statement that the dispatch + /// guard does not double as the mode gate — that is `reject_if_disabled`'s + /// job. + #[test] + fn dispatch_guard_accepts_a_bounded_interaction() { + let msg = interaction_frame(InteractionResponse::Choose { + choice_id: InteractionChoiceId("a".to_string()), + }); + + assert!(guard_client_message_before_dispatch(&msg, ServerMode::Full).is_ok()); + assert!(guard_client_message_before_dispatch(&msg, ServerMode::LobbyOnly).is_ok()); + } + + /// Declared wire policy for the projection boundary: an interaction is a + /// game frame, so `to_lobby_client_message` returns `None` for it and + /// nothing is ever cloned into the broker. Unbounded is safe here *only* + /// because nothing is cloned — which is why this test is meaningless + /// without its pair, `interaction_is_never_projected_into_the_lobby_broker` + /// in `phase-server`. + #[test] + fn broker_projection_accepts_an_interaction_without_bounding_it() { + assert!(guard_broker_projection_inbound(&oversized_interaction_frame()).is_ok()); + } + + /// Both halves are required: the `Interaction` half alone would pass for a + /// function that answered `ActionRejected` for everything, which would + /// change `Action`'s behavior. + #[test] + fn interaction_wire_rejection_answers_on_the_benign_channel() { + let interaction = oversized_interaction_frame(); + let reason = + guard_client_message_before_dispatch(&interaction, ServerMode::Full).unwrap_err(); + + match wire_rejection_message(&interaction, reason.clone()) { + ServerMessage::ActionRejected { reason: answered } => { + assert_eq!(answered, reason); + } + other => panic!("an interaction rejection must not tear the session down: {other:?}"), + } + + let action = ClientMessage::Action { + action: GameAction::ReorderHand { + order: vec![ObjectId(1); MAX_ACTION_LIST_LEN + 1], + }, + }; + let action_reason = + guard_client_message_before_dispatch(&action, ServerMode::Full).unwrap_err(); + + assert!( + matches!( + wire_rejection_message(&action, action_reason), + ServerMessage::Error { .. } + ), + "an oversized action stays a malformed frame" + ); + } } diff --git a/crates/server-core/src/interaction_payload_guard.rs b/crates/server-core/src/interaction_payload_guard.rs new file mode 100644 index 0000000000..b1212828c8 --- /dev/null +++ b/crates/server-core/src/interaction_payload_guard.rs @@ -0,0 +1,150 @@ +//! Wire-payload bounds for inbound `InteractionSubmission` bodies on the native +//! WebSocket path. +//! +//! The engine owns these bounds +//! (`engine::game::interaction::bound_interaction_submission`) and re-runs them +//! inside `submit_interaction`. This module exists so +//! `guard_client_message_before_dispatch` can *declare* the variant's wire +//! policy in its exhaustive match, the way every other arm declares one, rather +//! than declaring `Ok(())` — "unbounded at the wire" — for a variant that +//! carries a client-controlled payload. +//! +//! It deliberately restates no limit of its own: the engine owns the bounds, and +//! a second copy here would drift the first time a response variant changes. +//! +//! The rejection string is byte-identical to `engine-wasm`'s +//! (`format!("Engine error: {:?}", code)`) and to +//! `SessionManager::handle_interaction`'s, so the same engine reason code reads +//! the same on the WASM and WebSocket transports and at every server layer that +//! reports one. + +use engine::game::interaction::bound_interaction_submission; +use engine::types::interaction::InteractionSubmission; + +/// Validate a client-supplied interaction submission before session dispatch. +pub fn guard_interaction_submission_payload( + submission: &InteractionSubmission, +) -> Result<(), String> { + bound_interaction_submission(submission) + .map_err(|error| format!("Engine error: {:?}", error.code)) +} + +#[cfg(test)] +mod tests { + use super::*; + use engine::game::interaction::MAX_INTERACTION_STRING_LEN; + use engine::types::interaction::{ + InteractionChoiceId, InteractionId, InteractionReasonCode, InteractionResponse, + InteractionShortcutDecision, InteractionShortcutPin, MAX_INTERACTION_LIST_LEN, + }; + + fn choice(id: &str) -> InteractionChoiceId { + InteractionChoiceId(id.to_string()) + } + + fn submission(response: InteractionResponse) -> InteractionSubmission { + InteractionSubmission { + interaction_id: InteractionId("interaction-1".to_string()), + response, + } + } + + /// Reach guard for every negative below: the guard is not rejecting + /// everything it is handed. + #[test] + fn accepts_a_realistic_submission() { + let msg = submission(InteractionResponse::Select { + choice_ids: vec![choice("a"), choice("b")], + }); + + assert_eq!(guard_interaction_submission_payload(&msg), Ok(())); + } + + #[test] + fn rejects_an_oversized_select_list() { + let msg = submission(InteractionResponse::Select { + choice_ids: vec![choice("a"); MAX_INTERACTION_LIST_LEN + 1], + }); + + assert!(guard_interaction_submission_payload(&msg).is_err()); + + // Boundary sibling: the bound sits at the constant, not at "any large + // list". + let at_limit = submission(InteractionResponse::Select { + choice_ids: vec![choice("a"); MAX_INTERACTION_LIST_LEN], + }); + assert_eq!(guard_interaction_submission_payload(&at_limit), Ok(())); + } + + /// The routine-reachable case: a `TextChoiceProjection` with + /// `allow_arbitrary` accepts free-form text, so an ordinary paste can + /// exceed the bound. This is why the rejection must not travel on + /// `ServerMessage::Error`. + #[test] + fn rejects_an_oversized_text_value() { + let msg = submission(InteractionResponse::Text { + value: "x".repeat(MAX_INTERACTION_STRING_LEN + 1), + }); + + assert!(guard_interaction_submission_payload(&msg).is_err()); + + let at_limit = submission(InteractionResponse::Text { + value: "x".repeat(MAX_INTERACTION_STRING_LEN), + }); + assert_eq!(guard_interaction_submission_payload(&at_limit), Ok(())); + } + + #[test] + fn rejects_an_oversized_interaction_id() { + let msg = InteractionSubmission { + interaction_id: InteractionId("x".repeat(MAX_INTERACTION_STRING_LEN + 1)), + response: InteractionResponse::Choose { + choice_id: choice("a"), + }, + }; + + assert!(guard_interaction_submission_payload(&msg).is_err()); + } + + /// The `OutboundBudget` cumulative path — the only nested branch in the + /// engine validator, and precisely what a naive per-field guard would miss: + /// no single pin is oversized, only their sum. + #[test] + fn rejects_an_oversized_nested_shortcut_pin_budget() { + let per_pin = MAX_INTERACTION_LIST_LEN / 2; + let pins: Vec = (0..3) + .map(|group| InteractionShortcutPin { + group, + choice_ids: vec![choice("a"); per_pin], + }) + .collect(); + + for pin in &pins { + assert!(pin.choice_ids.len() <= MAX_INTERACTION_LIST_LEN); + } + + let msg = submission(InteractionResponse::Shortcut { + decision: InteractionShortcutDecision::AcceptSuggested, + pins, + }); + + assert!(guard_interaction_submission_payload(&msg).is_err()); + } + + /// Pins the byte-identity claim at this layer so it cannot silently drift + /// from `engine-wasm/src/lib.rs`'s `format!("Engine error: {:?}", code)`. + #[test] + fn rejection_string_matches_the_wasm_transport() { + let msg = submission(InteractionResponse::Select { + choice_ids: vec![choice("a"); MAX_INTERACTION_LIST_LEN + 1], + }); + + assert_eq!( + guard_interaction_submission_payload(&msg), + Err(format!( + "Engine error: {:?}", + InteractionReasonCode::PayloadTooLarge + )) + ); + } +} diff --git a/crates/server-core/src/lib.rs b/crates/server-core/src/lib.rs index 83c2f1098b..96deb36a6c 100644 --- a/crates/server-core/src/lib.rs +++ b/crates/server-core/src/lib.rs @@ -12,6 +12,7 @@ pub mod game_reconnect_guard; pub mod game_state_snapshot_wire_guard; #[cfg(test)] mod harness; +pub mod interaction_payload_guard; pub mod legacy_deck_guard; pub mod legacy_join_guard; pub mod lobby; @@ -29,7 +30,7 @@ pub mod takeback; pub use ai_seats_wire_guard::guard_create_ai_seats; pub use client_hello_guard::guard_client_hello; pub use client_message_wire_guard::{ - guard_broker_projection_inbound, guard_client_message_before_dispatch, + guard_broker_projection_inbound, guard_client_message_before_dispatch, wire_rejection_message, }; pub use deck_resolve::resolve_deck; pub use draft_action_payload_guard::guard_draft_action_payload; diff --git a/crates/server-core/src/protocol.rs b/crates/server-core/src/protocol.rs index f8d96e3bf6..e435a1856d 100644 --- a/crates/server-core/src/protocol.rs +++ b/crates/server-core/src/protocol.rs @@ -6,6 +6,7 @@ use engine::types::events::GameEvent; use engine::types::format::FormatConfig; use engine::types::game_state::GameState; use engine::types::identifiers::ObjectId; +use engine::types::interaction::InteractionSubmission; use engine::types::log::GameLogEntry; use engine::types::mana::ManaCost; use engine::types::match_config::MatchConfig; @@ -157,6 +158,19 @@ pub enum ClientMessage { request_id: u64, action: GameAction, }, + /// One opaque, engine-authored interaction response. The client echoes the + /// submission the engine published in `ViewerInteraction`; it never derives + /// a `GameAction` from the opportunity schema. Like `Action`, the + /// authenticated session — not the payload — determines the acting seat. + /// + /// Unlike `Action`, a bounds rejection on this variant is answered on + /// `ServerMessage::ActionRejected`, not `ServerMessage::Error`: a free-form + /// `Text` response can exceed `MAX_INTERACTION_STRING_LEN` by an ordinary + /// paste, and the native client tears the session down on any `Error`. + /// See `client_message_wire_guard::wire_rejection_message`. + Interaction { + submission: InteractionSubmission, + }, Reconnect { game_code: String, player_token: String, diff --git a/crates/server-core/src/session.rs b/crates/server-core/src/session.rs index 8b33977ad0..2a92c374d3 100644 --- a/crates/server-core/src/session.rs +++ b/crates/server-core/src/session.rs @@ -8,7 +8,7 @@ use engine::database::CardDatabase; use engine::game::deck_loading::{DeckPayload, PlayerDeckPayload}; use engine::game::engine::{apply, start_game}; use engine::game::finalize_public_state; -use engine::game::interaction::bind_interaction_authority; +use engine::game::interaction::{bind_interaction_authority, submit_interaction}; use engine::game::match_flow::apply_trusted_match_forfeit; use engine::game::preview::preview_auto_payment_sources; use engine::game::{load_and_hydrate_decks, rehydrate_game_from_card_db}; @@ -17,7 +17,7 @@ use engine::types::events::GameEvent; use engine::types::format::FormatConfig; use engine::types::game_state::{GameState, PersistedGameState}; use engine::types::identifiers::ObjectId; -use engine::types::interaction::InteractionSessionId; +use engine::types::interaction::{InteractionSessionId, InteractionSubmission}; use engine::types::log::GameLogEntry; use engine::types::mana::ManaCost; use engine::types::match_config::MatchConfig; @@ -1400,6 +1400,116 @@ impl SessionManager { )) } + /// Apply one engine-authored interaction submission from a player. + /// + /// Deliberately shaped as the exact sibling of [`Self::handle_action`]: it + /// returns the same [`ActionResult`], so the transport's broadcast path is + /// shared rather than duplicated. + /// + /// The acting `PlayerId` is resolved from the join-token-authenticated + /// session, never from the payload — the wire frame carries no actor field + /// at all (`protocol.rs`: "The authenticated session, rather than the + /// client, determines the actor"). `submit_interaction` then re-authorizes + /// the actor against the interaction slot inside the engine, so a forged + /// `interaction_id` belonging to another seat is rejected twice. + /// + /// There is deliberately no debug-capability gate here, unlike + /// `handle_action`. `materialize_response` dispatches on + /// `human_response_model`, and its `Choose` fallthrough sources actions from + /// `actor_candidates` -> + /// `ai_support::validated_candidate_actions_for_semantic_owner`. An + /// engine-wide grep shows no production constructor of + /// `GameAction::Debug`, `GrantDebugPermission`, or `RevokeDebugPermission` + /// anywhere in that chain — only classification and ordering matches — so + /// guarding here would be validation for a case that cannot occur. The + /// engine test + /// `published_interaction_choices_never_offer_a_debug_action_in_a_sandbox_game` + /// (`crates/engine/tests/integration/interaction_contract.rs`) fails the day + /// that stops being true. + pub fn handle_interaction( + &mut self, + game_code: &str, + player_token: &str, + submission: InteractionSubmission, + ) -> Result { + let session = self + .sessions + .get_mut(game_code) + .ok_or_else(|| format!("Game not found: {game_code}"))?; + + let player = session + .player_for_token(player_token) + .ok_or_else(|| "Invalid player token".to_string())?; + + // GH #1507: the authoritative state must not move while the table is + // voting on a rollback. Same interlock, same reason, as `handle_action`. + if session.pending_takeback.is_some() { + return Err( + "A takeback request is pending — resolve it before taking further actions" + .to_string(), + ); + } + + // Snapshot BEFORE `log_player_names` is written, matching + // `handle_action`'s order exactly. The two handlers are siblings; the + // ordering is part of that. + // + // The clone is unconditional here where `handle_action`'s is + // conditional on `!action.is_actor_scoped_preference()`, because the + // action is not known until `submit_interaction` returns. That is + // equivalent, not a regression: the seven actions that predicate + // matches are UI preferences that no candidate enumerator or + // `materialize_*` function ever produces, so the predicate is always + // false on this path. The guard below is kept anyway so the two + // handlers stay structurally identical if that ever changes. + let pre_action_state = session.state.clone(); + + // Set player names for log resolution. + session.state.log_player_names = session.display_names.clone(); + + let applied = + submit_interaction(&mut session.state, player, submission).map_err(|error| { + warn!( + game = %game_code, + player = ?player, + code = ?error.code, + reason = "interaction_rejected", + "interaction rejected" + ); + // Byte-identical to the WASM transport + // (`engine-wasm/src/lib.rs`) and to + // `interaction_payload_guard`, so every layer that reports an + // engine reason code, on both transports, classifies the same + // rejection identically. + format!("Engine error: {:?}", error.code) + })?; + + if !applied.action.is_actor_scoped_preference() { + session.push_takeback_state(player, pre_action_state); + } + + info!( + game = %game_code, + player = ?player, + action_type = applied.action.variant_name(), + event_count = applied.result.events.len(), + "interaction applied" + ); + + let (new_legal_actions, spell_costs, by_object) = engine_legal_actions_full(&session.state); + let auto_pass = auto_pass_recommended(&session.state, &new_legal_actions); + + Ok(( + session.state.clone(), + applied.result.events, + new_legal_actions, + applied.result.log_entries, + auto_pass, + spell_costs, + by_object, + )) + } + /// Applies the payload-free match-concede intent after binding its /// requester to an authenticated player token. The closed cause is chosen /// here, never by the wire payload or a game action. @@ -1596,7 +1706,10 @@ mod tests { use engine::types::card::CardFace; use engine::types::card_type::CardType; use engine::types::game_state::{CastPaymentMode, PersistedGameState, WaitingFor}; - use engine::types::interaction::{InteractionAvailability, InteractionReasonCode}; + use engine::types::interaction::{ + InteractionAvailability, InteractionChoiceId, InteractionReasonCode, InteractionResponse, + MAX_INTERACTION_LIST_LEN, + }; use engine::types::mana::ManaCost; use engine::types::phase::{Phase, PhaseStop, PhaseStopScope}; use engine::types::zones::Zone; @@ -4345,4 +4458,174 @@ mod tests { && attacker.attack_target == AttackTarget::Player(PlayerId(2)) })); } + + /// A live, engine-published submission for whichever seat currently holds a + /// complete progress witness, paired with that seat's authenticated token. + /// + /// Deriving the witness from `derive_viewer_interaction` rather than + /// hand-building one is the point: these tests must exercise a capability + /// the engine actually minted, or a `StaleInteraction` would be + /// indistinguishable from a fabricated id. + fn live_witness<'a>( + mgr: &SessionManager, + code: &str, + token0: &'a str, + token1: &'a str, + ) -> (PlayerId, &'a str, &'a str, InteractionSubmission) { + let state = &mgr.sessions.get(code).expect("session").state; + for (player, token, other) in [(PlayerId(0), token0, token1), (PlayerId(1), token1, token0)] + { + let filtered = filter_state_for_player(state, player); + let view = derive_viewer_interaction(state, &filtered, player); + if let InteractionAvailability::ProgressAvailable { witness } = view.availability { + return (player, token, other, witness); + } + } + panic!("a started session must publish a progress witness for some seat"); + } + + fn started_two_seat_game() -> (SessionManager, String, String, String) { + let mut mgr = SessionManager::new(); + let (code, token0) = mgr.create_game(make_deck()); + let (token1, _) = mgr + .join_game(&code, make_deck()) + .expect("second seat joins"); + (mgr, code, token0, token1) + } + + /// The multi-authority hostile fixture: two seats, both legitimately + /// authenticated, one capability. Seat B holding a *valid* token for the + /// same game must not be able to spend seat A's `interaction_id`. + #[test] + fn handle_interaction_binds_the_actor_to_the_authenticated_token() { + let (mut mgr, code, token0, token1) = started_two_seat_game(); + let (_acting, acting_token, other_token, witness) = + live_witness(&mgr, &code, &token0, &token1); + + let before = mgr.sessions[&code].state.active_interaction_slots.clone(); + + let forged = mgr.handle_interaction(&code, other_token, witness.clone()); + let error = forged.expect_err("a valid token for another seat must not spend this slot"); + assert!( + error.contains("NotAuthorized"), + "unexpected reason: {error}" + ); + assert_eq!( + mgr.sessions[&code].state.active_interaction_slots, before, + "a refused submission must not consume the capability" + ); + + // The success half is the reach guard: without it the `Err` above could + // have come from staleness rather than from authorization. + mgr.handle_interaction(&code, acting_token, witness) + .expect("the authenticated owner of the slot may spend it"); + assert_ne!( + mgr.sessions[&code].state.active_interaction_slots, before, + "an accepted submission re-mints the interaction slots" + ); + } + + #[test] + fn handle_interaction_rejects_an_unknown_token() { + let (mut mgr, code, token0, token1) = started_two_seat_game(); + let (_acting, _acting_token, _other, witness) = live_witness(&mgr, &code, &token0, &token1); + + let before_waiting = mgr.sessions[&code].state.waiting_for.clone(); + let before_slots = mgr.sessions[&code].state.active_interaction_slots.clone(); + + let error = mgr + .handle_interaction(&code, "not-a-real-token", witness) + .expect_err("an unknown token is not a seat"); + assert_eq!(error, "Invalid player token"); + + assert_eq!(mgr.sessions[&code].state.waiting_for, before_waiting); + assert_eq!( + mgr.sessions[&code].state.active_interaction_slots, + before_slots + ); + } + + /// Capability consumption. This is the routine condition the benign + /// rejection channel exists for: a double-click produces exactly this. + #[test] + fn handle_interaction_rejects_a_stale_submission_benignly() { + let (mut mgr, code, token0, token1) = started_two_seat_game(); + let (_acting, acting_token, _other, witness) = live_witness(&mgr, &code, &token0, &token1); + + mgr.handle_interaction(&code, acting_token, witness.clone()) + .expect("the first submission spends a live capability"); + + let error = mgr + .handle_interaction(&code, acting_token, witness) + .expect_err("an interaction id is single-use"); + assert!(error.contains("StaleInteraction"), "unexpected: {error}"); + } + + #[test] + fn handle_interaction_rejects_an_oversized_response_before_the_reducer() { + let (mut mgr, code, token0, token1) = started_two_seat_game(); + let (_acting, acting_token, _other, witness) = live_witness(&mgr, &code, &token0, &token1); + + let choice = InteractionChoiceId("a".to_string()); + let oversized = InteractionSubmission { + interaction_id: witness.interaction_id.clone(), + response: InteractionResponse::Select { + choice_ids: vec![choice.clone(); MAX_INTERACTION_LIST_LEN + 1], + }, + }; + + let error = mgr + .handle_interaction(&code, acting_token, oversized) + .expect_err("an oversized response is refused"); + assert_eq!(error, "Engine error: PayloadTooLarge"); + + // Negative sibling: the boundary sits at the constant, not at "any + // large list". The same shape at exactly the limit is still refused, + // but for a different reason and further downstream. + let at_limit = InteractionSubmission { + interaction_id: witness.interaction_id.clone(), + response: InteractionResponse::Select { + choice_ids: vec![choice; MAX_INTERACTION_LIST_LEN], + }, + }; + let error = mgr + .handle_interaction(&code, acting_token, at_limit) + .expect_err("unknown choices are still refused"); + assert!( + !error.contains("PayloadTooLarge"), + "the bound must be the constant, not list size in general: {error}" + ); + } + + #[test] + fn handle_interaction_defers_to_a_pending_takeback() { + let (mut mgr, code, token0, token1) = started_two_seat_game(); + let (acting, acting_token, _other, witness) = live_witness(&mgr, &code, &token0, &token1); + + let target_state = mgr.sessions[&code].state.clone(); + mgr.sessions.get_mut(&code).unwrap().pending_takeback = Some(PendingTakeback { + requested_by: acting, + target_state, + approvals: HashSet::new(), + }); + + let before_slots = mgr.sessions[&code].state.active_interaction_slots.clone(); + let error = mgr + .handle_interaction(&code, acting_token, witness.clone()) + .expect_err("the table is voting; the state must not move"); + assert!( + error.contains("takeback request is pending"), + "unexpected: {error}" + ); + assert_eq!( + mgr.sessions[&code].state.active_interaction_slots, + before_slots + ); + + // Reach guard: the very same submission succeeds once the interlock is + // cleared, so the refusal above is attributable to the takeback. + mgr.sessions.get_mut(&code).unwrap().pending_takeback = None; + mgr.handle_interaction(&code, acting_token, witness) + .expect("with no pending takeback the same submission applies"); + } }