fix(id): refuse non-player signers in sign_genesis/sign_claim (#6424)

pkgs/id/src/attestation.rs

@@ -169,8 +169,10 @@ pub enum MatchState {
 /// # Errors
 ///
 /// Returns [`SignError::DuplicateSeat`] if both seats name the same
-/// player, a chain error if the extension or backlink is invalid, or a
-/// JSON error if the agreement cannot be serialized.
+/// player, [`SignError::NotAPlayer`] if the signing key is neither seat
+/// (no viewer would ever count its genesis, #6424), a chain error if the
+/// extension or backlink is invalid, or a JSON error if the agreement
+/// cannot be serialized.
 pub fn sign_genesis(
     key: &SigningKey,
     seq_num: SeqNum,
@@ -182,6 +184,10 @@ pub fn sign_genesis(
     if first == second {
         return Err(SignError::DuplicateSeat);
     }
+    let signer = player_id(key);
+    if signer != first && signer != second {
+        return Err(SignError::NotAPlayer);
+    }
     let body = agreement.body()?;
     Ok(chain::sign(
         key,
@@ -212,10 +218,38 @@ pub fn sign_genesis(
 /// knowable only against the agreement and stays `compute_state`'s
 /// call, so any well-formed hex signs here.
 ///
+/// The claim is checked against the `agreement` it answers: the signing
+/// key must be one of the agreement's players, since [`compute_state`]
+/// ignores claims from non-players (#6424).
+///
 /// # Errors
 ///
-/// As [`sign_genesis`], plus [`SignError::InvalidOutcome`] above.
+/// As [`sign_genesis`] for the chain and JSON cases, plus
+/// [`SignError::InvalidOutcome`] above, [`SignError::AgreementMismatch`]
+/// if the claim does not answer `agreement`, and
+/// [`SignError::NotAPlayer`] if the signing key is not one of its players.
 pub fn sign_claim(
+    key: &SigningKey,
+    seq_num: SeqNum,
+    backlink: Option<Hash>,
+    agreement: &GenesisAgreement,
+    claim: &ResultClaim,
+    after: Vec<Hash>,
+) -> Result<Event, SignError> {
+    if claim.agreement != agreement.agreement_hash()? {
+        return Err(SignError::AgreementMismatch);
+    }
+    if !agreement.players.contains(&player_id(key)) {
+        return Err(SignError::NotAPlayer);
+    }
+    sign_claim_unchecked(key, seq_num, backlink, claim, after)
+}
+
+/// Signs a claim without checking the signer against an agreement; the
+/// outcome shape is still checked. Crate-private so only tests can
+/// build claims the viewer must ignore (the `sign_unchecked` pattern of
+/// the artifact module).
+pub(crate) fn sign_claim_unchecked(
     key: &SigningKey,
     seq_num: SeqNum,
     backlink: Option<Hash>,
@@ -335,7 +369,8 @@ pub fn compute_state(
 }
 
 /// A signing failure: the object would not serialize, the chain
-/// refused the event, or the object names the same player in both seats.
+/// refused the event, the object names the same player in both seats,
+/// or the signer is not a player of the agreement.
 #[derive(Debug)]
 pub enum SignError {
     /// The object would not serialize to its canonical body.
@@ -347,6 +382,12 @@ pub enum SignError {
     /// The claim's outcome is neither `"draw"` nor a 64-character
     /// lowercase hex player id, so no view could ever count it (#6441).
     InvalidOutcome,
+    /// The signing key is not one of the agreement's two players, so no
+    /// view would ever count the object (#6424).
+    NotAPlayer,
+    /// The claim names a different agreement than the one it was
+    /// checked against.
+    AgreementMismatch,
 }
 
 impl From<serde_json::Error> for SignError {
@@ -369,6 +410,12 @@ impl std::fmt::Display for SignError {
             Self::DuplicateSeat => {
                 write!(f, "both seats of the agreement name the same player")
             }
+            Self::NotAPlayer => {
+                write!(f, "signing key is not one of the agreement's players")
+            }
+            Self::AgreementMismatch => {
+                write!(f, "claim does not answer the given agreement")
+            }
             Self::InvalidOutcome => {
                 write!(
                     f,

pkgs/id/src/world_net.rs

@@ -655,7 +655,7 @@ impl RecordsBudget {
         // Check ordering: keys must be strictly ascending
         if let Some(last) = &self.last_key {
             anyhow::ensure!(
-                key > last,
+                key > last.as_str(),
                 "records are out of order: key {key:?} <= last key {last:?}"
             );
         }
@@ -1618,7 +1618,7 @@ impl WorldClient {
                     // Validate that the cursor advanced from what we sent
                     if let Some(cursor) = &after {
                         anyhow::ensure!(
-                            next > cursor,
+                            next > cursor.as_str(),
                             "records cursor did not advance: next_after={next:?}, previous cursor={cursor:?}"
                         );
                     }

pkgs/id/tests/attestation_outcome_sign.rs

@@ -48,7 +48,7 @@ fn sign_with_outcome(outcome: &str) -> Result<(), SignError> {
     let genesis = sign_genesis(&x, 0, None, &agreement, vec![])?;
     claim.outcome = outcome.to_owned();
     claim.frontiers = vec![genesis.id];
-    sign_claim(&x, 1, Some(genesis.id), &claim, vec![]).map(|_| ())
+    sign_claim(&x, 1, Some(genesis.id), &agreement, &claim, vec![]).map(|_| ())
 }
 
 #[test]
@@ -100,8 +100,8 @@ fn controls_sign_ok_and_compute_agreed_end_to_end() {
         let mut claim = base.clone();
         claim.outcome = outcome.to_owned();
         claim.frontiers = vec![gx.id, go.id];
-        let cx = sign_claim(&x, 1, Some(gx.id), &claim, vec![]).unwrap();
-        let co = sign_claim(&o, 1, Some(go.id), &claim, vec![]).unwrap();
+        let cx = sign_claim(&x, 1, Some(gx.id), &agreement, &claim, vec![]).unwrap();
+        let co = sign_claim(&o, 1, Some(go.id), &agreement, &claim, vec![]).unwrap();
         let _ = (cx, co);
         let genesis = vec![
             (author_id(&gx), agreement.clone()),

pkgs/id/tests/attestation_poc.rs

@@ -88,6 +88,7 @@ fn play_match(x_outcome: &str, o_outcome: &str) -> Match {
         &x,
         1,
         Some(genesis_x.id),
+        &agreement,
         &claim_x,
         vec![genesis_x.id, genesis_o.id],
     )
@@ -96,6 +97,7 @@ fn play_match(x_outcome: &str, o_outcome: &str) -> Match {
         &o,
         1,
         Some(genesis_o.id),
+        &agreement,
         &claim_o,
         vec![genesis_x.id, genesis_o.id],
     )

pkgs/id/tests/attestation_signer_player.rs

@@ -0,0 +1,87 @@
+//! #6424: sign_genesis / sign_claim refuse signers that are not players
+//! of the agreement, since compute_state never counts their objects.
+
+#![allow(clippy::unwrap_used, clippy::expect_used)]
+
+use p2panda_core::{Hash, SigningKey};
+
+use id::attestation::{
+    GenesisAgreement, MatchState, ResultClaim, SignError, author_id, compute_state, player_id,
+    sign_claim, sign_genesis,
+};
+
+fn key(seed: u8) -> SigningKey {
+    SigningKey::from_bytes(&std::array::from_fn(|i| {
+        seed.wrapping_add(u8::try_from(i).unwrap_or(0))
+    }))
+}
+
+fn setup() -> (SigningKey, SigningKey, GenesisAgreement, ResultClaim) {
+    let x = key(0);
+    let o = key(100);
+    let agreement = GenesisAgreement {
+        game_id: "tic-tac-toe".to_owned(),
+        logic_hash: Hash::digest(b"logic"),
+        players: [player_id(&x), player_id(&o)],
+        witness: None,
+        objection_window_secs: 3600,
+    };
+    let claim = ResultClaim {
+        agreement: agreement.agreement_hash().unwrap(),
+        outcome: "draw".to_owned(),
+        final_state_hash: Hash::digest(b"final"),
+        frontiers: vec![],
+    };
+    (x, o, agreement, claim)
+}
+
+#[test]
+fn sign_genesis_refuses_non_player() {
+    let (_x, _o, agreement, _claim) = setup();
+    let stranger = key(200);
+    let result = sign_genesis(&stranger, 0, None, &agreement, vec![]);
+    assert!(
+        matches!(result, Err(SignError::NotAPlayer)),
+        "expected NotAPlayer, got {result:?}"
+    );
+}
+
+#[test]
+fn sign_claim_refuses_non_player() {
+    let (_x, _o, agreement, claim) = setup();
+    let stranger = key(200);
+    let result = sign_claim(&stranger, 1, None, &agreement, &claim, vec![]);
+    assert!(
+        matches!(result, Err(SignError::NotAPlayer)),
+        "expected NotAPlayer, got {result:?}"
+    );
+}
+
+#[test]
+fn sign_claim_refuses_claim_for_another_agreement() {
+    let (x, _o, agreement, mut claim) = setup();
+    claim.agreement = Hash::digest(b"some other agreement");
+    let result = sign_claim(&x, 1, None, &agreement, &claim, vec![]);
+    assert!(
+        matches!(result, Err(SignError::AgreementMismatch)),
+        "expected AgreementMismatch, got {result:?}"
+    );
+}
+
+#[test]
+fn both_players_still_sign_and_compute_agreed() {
+    let (x, o, agreement, claim) = setup();
+    let gx = sign_genesis(&x, 0, None, &agreement, vec![]).unwrap();
+    let go = sign_genesis(&o, 0, None, &agreement, vec![]).unwrap();
+    sign_claim(&x, 1, Some(gx.id), &agreement, &claim, vec![gx.id, go.id]).unwrap();
+    sign_claim(&o, 1, Some(go.id), &agreement, &claim, vec![gx.id, go.id]).unwrap();
+    let genesis = vec![
+        (author_id(&gx), agreement.clone()),
+        (author_id(&go), agreement.clone()),
+    ];
+    let claims = vec![(player_id(&x), claim.clone()), (player_id(&o), claim)];
+    assert_eq!(
+        compute_state(&agreement, &genesis, &claims),
+        MatchState::Agreed
+    );
+}