From f1aeb241577bc03f941eba52dfe017c543daf09c Mon Sep 17 00:00:00 2001 From: funman300 Date: Tue, 5 May 2026 17:26:14 +0000 Subject: [PATCH] fix(core): validate moved tableau stack forms a legal run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit move_cards only checked that the *bottom* card of a moved stack landed legally on the destination — the cards above the bottom went through unverified. A player could lift an arbitrary selection from one column and drop it on another whenever the bottom happened to match, even if the upper cards didn't form a descending alternating-colour sequence. Adds is_valid_tableau_sequence(&[Card]) -> bool to rules.rs (4 lines) and one call site in move_cards's tableau-destination branch. One focused test covering single-card / valid-run / same-colour / rank-gap cases. Reported by Quat: "stack 4 onto stack 2" was accepted when illegal. Co-Authored-By: Claude Opus 4.7 (1M context) --- solitaire_core/src/game_state.rs | 14 ++++++++++++- solitaire_core/src/rules.rs | 34 ++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/solitaire_core/src/game_state.rs b/solitaire_core/src/game_state.rs index d45f215..5001dba 100644 --- a/solitaire_core/src/game_state.rs +++ b/solitaire_core/src/game_state.rs @@ -4,7 +4,7 @@ use crate::card::Card; use crate::deck::{deal_klondike, Deck}; use crate::error::MoveError; use crate::pile::{Pile, PileType}; -use crate::rules::{can_place_on_foundation, can_place_on_tableau}; +use crate::rules::{can_place_on_foundation, can_place_on_tableau, is_valid_tableau_sequence}; use crate::scoring::{compute_time_bonus as scoring_time_bonus, score_move, score_undo as scoring_undo}; const MAX_UNDO_STACK: usize = 64; @@ -283,6 +283,18 @@ impl GameState { if !can_place_on_tableau(&bottom_card, dest) { return Err(MoveError::RuleViolation("invalid tableau placement".into())); } + // The previous check only validates that the *bottom* of the + // moved stack lands on the destination's top card. Without + // this guard, a player could lift an arbitrary multi-card + // selection from one column and drop it onto another whenever + // the bottom card happens to match — even if the cards + // above the bottom don't form a legal descending + // alternating-colour run. + if !is_valid_tableau_sequence(&from_pile.cards[start..]) { + return Err(MoveError::RuleViolation( + "moved cards must form a valid tableau run".into(), + )); + } } _ => return Err(MoveError::InvalidDestination), } diff --git a/solitaire_core/src/rules.rs b/solitaire_core/src/rules.rs index 56b48ec..66b0b9c 100644 --- a/solitaire_core/src/rules.rs +++ b/solitaire_core/src/rules.rs @@ -30,6 +30,18 @@ pub fn can_place_on_tableau(card: &Card, pile: &Pile) -> bool { } } +/// Returns `true` if `cards` is a legal tableau run on its own — every +/// adjacent pair descends by one rank and alternates colour. A single +/// card is trivially valid. The destination check is separate; this +/// only validates the sequence's *internal* structure, which the tableau +/// move path must enforce so a player can't smuggle an arbitrary stack +/// onto another column when the bottom card happens to land legally. +pub fn is_valid_tableau_sequence(cards: &[Card]) -> bool { + cards.windows(2).all(|w| { + w[0].rank.value() == w[1].rank.value() + 1 && w[0].suit.is_red() != w[1].suit.is_red() + }) +} + #[cfg(test)] mod tests { use super::*; @@ -174,4 +186,26 @@ mod tests { let p = pile_with(PileType::Tableau(0), vec![top]); assert!(!can_place_on_tableau(&c, &p)); } + + #[test] + fn tableau_sequence_validation() { + // Single card is trivially a valid sequence. + assert!(is_valid_tableau_sequence(&[card(Suit::Hearts, Rank::Five)])); + // Valid descending alternating-colour run K♠ Q♥ J♣. + assert!(is_valid_tableau_sequence(&[ + card(Suit::Spades, Rank::King), + card(Suit::Hearts, Rank::Queen), + card(Suit::Clubs, Rank::Jack), + ])); + // Same colour twice (Q♠ on K♠) — invalid. + assert!(!is_valid_tableau_sequence(&[ + card(Suit::Spades, Rank::King), + card(Suit::Spades, Rank::Queen), + ])); + // Rank gap (K♠ → J♥) — invalid. + assert!(!is_valid_tableau_sequence(&[ + card(Suit::Spades, Rank::King), + card(Suit::Hearts, Rank::Jack), + ])); + } }