test(contract): pin per-record ban signature checks at the cap; fix debug bans the same way

cli/CHANGELOG.md

@@ -2,6 +2,14 @@
 
 All notable changes to riverctl will be documented in this file.
 
+## [Unreleased]
+
+### Fixed
+- `debug bans` judges each ban record by its own signature. A record that
+  copies another ban's signature was reported with that ban's status
+  (freenet/river#797 follow-up). Display only: the contract already drops
+  such records.
+
 ## [0.2.22] - 2026-10-16
 
 ### Changed

cli/src/commands/debug.rs

@@ -4,7 +4,7 @@ use crate::output::OutputFormat;
 use anyhow::{anyhow, Result};
 use clap::Subcommand;
 use ed25519_dalek::VerifyingKey;
-use river_core::room_state::ban::AuthorizedUserBan;
+use river_core::room_state::ban::{AuthorizedUserBan, BansV1};
 use river_core::room_state::member::{AuthorizedMember, BanResolution, MemberId};
 use river_core::room_state::{ChatRoomParametersV1, ChatRoomStateV1};
 use serde::Serialize;
@@ -160,12 +160,15 @@ struct BanInfo {
 ///
 /// - A ban naming the owner is `Inert` and permanently dead: the owner can
 ///   never be a target.
-/// - A ban whose signature was not verified against its issuer's key is
+/// - A ban whose own signature does not verify against its issuer's key is
 ///   `Inert`. That covers a forgery (the check must come FIRST, since the
 ///   authority check knows nothing about who signed: pinned by
 ///   `ban_signed_by_someone_other_than_its_attributed_banner_is_inert`), an
 ///   issuer whose record is held nowhere (reversible: if they rejoin with the
 ///   same key their bans verify again), and an issuer the owner has banned.
+///   Each record is checked itself: `resolution` is keyed by `BanId`, a hash
+///   of the signature alone, so a record copying another ban's signature
+///   would otherwise read that ban's verdict.
 /// - A ban the resolution ranks owner, effective or mutual is `Enforcing`:
 ///   it removes its target, a mutual-ban cycle's bans included.
 /// - Any other ban takes no effect today. With its target's record known it
@@ -176,8 +179,12 @@ fn classify_ban(
     resolution: &BanResolution,
     known: &HashMap<MemberId, &AuthorizedMember>,
     owner_id: MemberId,
+    owner_vk: &VerifyingKey,
 ) -> BanEnforcement {
-    if ban.ban.banned_user == owner_id || !resolution.verified.contains(&ban.id()) {
+    if ban.ban.banned_user == owner_id
+        || !BansV1::ban_signature_matches_current_key(ban, known, owner_id, owner_vk)
+        || !resolution.verified.contains(&ban.id())
+    {
         return BanEnforcement::Inert;
     }
     if resolution.tier(&ban.id()) > BanResolution::INERT {
@@ -220,7 +227,7 @@ fn collect_ban_infos(room_state: &ChatRoomStateV1, owner_vk: &VerifyingKey) -> V
                 banned_user_id: ban.ban.banned_user.to_string(),
                 banned_by_id: ban.banned_by.to_string(),
                 banned_at_secs,
-                enforcement: classify_ban(ban, &resolution, &known, owner_id),
+                enforcement: classify_ban(ban, &resolution, &known, owner_id, owner_vk),
             }
         })
         .collect()
@@ -1116,6 +1123,45 @@ mod tests {
         assert_eq!(verdict(&state, &owner), BanEnforcement::Inert);
     }
 
+    #[test]
+    fn record_copying_a_genuine_bans_signature_is_inert() {
+        // A `BanId` hashes the signature alone, so a record that reuses a
+        // genuine ban's signature with a different target shares its id. The
+        // classifier must judge each record by its own signature, not read
+        // the genuine ban's verdict through the shared id.
+        let owner = key(1);
+        let alice = key(2);
+        let bob = key(3);
+
+        let mut state = ChatRoomStateV1::default();
+        push_member(&mut state, &owner, &owner, &alice);
+        push_member(&mut state, &owner, &owner, &bob);
+        push_ban(&mut state, &owner, &owner, &alice);
+        let genuine = state.bans.0[0].clone();
+        let copy = AuthorizedUserBan::with_signature(
+            UserBan {
+                owner_member_id: id(&owner),
+                banned_at: genuine.ban.banned_at + std::time::Duration::from_secs(1),
+                banned_user: id(&bob),
+            },
+            id(&owner),
+            genuine.signature,
+        );
+        assert_eq!(copy.id(), genuine.id(), "precondition: the ids collide");
+        state.bans.0.push(copy);
+
+        let infos = collect_ban_infos(&state, &owner.verifying_key());
+        let of = |target: &SigningKey| {
+            infos
+                .iter()
+                .find(|i| i.banned_user_id == id(target).to_string())
+                .expect("ban listed")
+                .enforcement
+        };
+        assert_eq!(of(&alice), BanEnforcement::Enforcing);
+        assert_eq!(of(&bob), BanEnforcement::Inert);
+    }
+
     #[test]
     fn ban_naming_the_room_owner_is_inert_not_undetermined() {
         // `is_ban_authorized` denies `target == owner_id` outright, before any

common/tests/mutual_rejection_test.rs

@@ -3152,3 +3152,126 @@ fn evidence_member_info_is_kept_up_to_the_first_present_ancestor() {
         }
     }
 }
+
+// --- freenet/river#797: each ban record is judged by its own signature ---
+//
+// A `BanId` hashes the signature alone, and records that share an id but
+// differ in content are all stored. `enforce_user_ban_cap` must not credit a
+// record with the verdict `resolve_bans` reached for another record of the
+// same id: a record whose own signature does not verify against its claimed
+// issuer is dead. Each case is run in two input orders and compares whole
+// records, since the forged record and the genuine one share an id.
+type BanRecord = (MemberId, std::time::SystemTime, MemberId, MemberId);
+fn ban_record(b: &AuthorizedUserBan) -> BanRecord {
+    (
+        b.banned_by,
+        b.ban.banned_at,
+        b.ban.banned_user,
+        b.ban.owner_member_id,
+    )
+}
+fn copy_signature(
+    sig_of: &AuthorizedUserBan,
+    issuer: MemberId,
+    target: MemberId,
+    at: u64,
+    owner: MemberId,
+) -> AuthorizedUserBan {
+    AuthorizedUserBan::with_signature(
+        UserBan {
+            owner_member_id: owner,
+            banned_at: std::time::UNIX_EPOCH + std::time::Duration::from_secs(1_800_000_000 + at),
+            banned_user: target,
+        },
+        issuer,
+        sig_of.signature,
+    )
+}
+fn cap_both_orders(
+    room: &Room,
+    s: &ChatRoomStateV1,
+    bans: Vec<AuthorizedUserBan>,
+    cap: usize,
+) -> Vec<BanRecord> {
+    let mut out = Vec::new();
+    for rev in [false, true] {
+        let mut b = bans.clone();
+        if rev {
+            b.reverse();
+        }
+        BansV1::enforce_user_ban_cap(
+            &mut b,
+            cap,
+            &s.members,
+            &s.ban_evidence,
+            &s.member_info,
+            &room.params,
+        );
+        out.push(b.iter().map(ban_record).collect::<Vec<_>>());
+    }
+    assert_eq!(out[0], out[1], "input order changed the result");
+    out.pop().unwrap()
+}
+#[test]
+fn a_record_copying_an_owner_bans_signature_is_dead_at_the_cap() {
+    let room = capped_room(20, 2);
+    let s = room.modded_state(&[], &[], vec![]);
+    let (x, y, z) = (room.person(), room.person(), room.person());
+    let b1 = room.owner_ban(&x, 10);
+    let b2 = room.owner_ban(&y, 11);
+    let forged = copy_signature(&b1, room.owner_id, z.id, 99, room.owner_id);
+    let got = cap_both_orders(&room, &s, vec![b1.clone(), b2.clone(), forged], 2);
+    assert_eq!(got, vec![ban_record(&b1), ban_record(&b2)]);
+}
+#[test]
+fn a_record_copying_a_bans_signature_cannot_take_its_pair() {
+    // An older live ban H competes for the one slot, so the result depends on
+    // the first-of-pair filter alone: with it, the copy is dead and goes,
+    // then H (older than G) goes, leaving G. Without it, the copy takes G's
+    // pair, G and the copy both go as dead, and H is left.
+    let room = capped_room(20, 1);
+    let a = room.person();
+    let s = room.modded_state(&[&a], &[], vec![]);
+    let (t, t2) = (room.person(), room.person());
+    let g = room.ban(&a, &t, 20);
+    let h = room.ban(&a, &t2, 15);
+    let forged = copy_signature(&g, a.id, t.id, 10, room.owner_id);
+    let got = cap_both_orders(&room, &s, vec![g.clone(), h, forged], 1);
+    assert_eq!(got, vec![ban_record(&g)]);
+}
+
+/// A copy identical to a genuine ban in issuer, target and date, differing
+/// only in `owner_member_id`, matches the genuine ban's first-of-pair rank
+/// exactly. Only the per-record signature check in `live` makes it dead.
+#[test]
+fn a_record_differing_only_in_owner_member_id_is_dead_at_the_cap() {
+    let room = capped_room(20, 2);
+    let s = room.modded_state(&[], &[], vec![]);
+    let (x, y) = (room.person(), room.person());
+    let b1 = room.owner_ban(&x, 10);
+    let b2 = room.owner_ban(&y, 11);
+    let forged = AuthorizedUserBan::with_signature(
+        UserBan {
+            owner_member_id: room.person().id,
+            banned_at: b1.ban.banned_at,
+            banned_user: x.id,
+        },
+        room.owner_id,
+        b1.signature,
+    );
+    let got = cap_both_orders(&room, &s, vec![b1.clone(), b2.clone(), forged], 2);
+    assert_eq!(got, vec![ban_record(&b1), ban_record(&b2)]);
+}
+
+#[test]
+fn a_record_copying_a_bans_signature_is_not_live_on_an_unknown_target() {
+    let room = capped_room(20, 2);
+    let a = room.person();
+    let s = room.modded_state(&[&a], &[], vec![]);
+    let (t1, t2, t3) = (room.person(), room.person(), room.person());
+    let g1 = room.ban(&a, &t1, 10);
+    let g2 = room.ban(&a, &t2, 11);
+    let forged = copy_signature(&g1, a.id, t3.id, 99, room.owner_id);
+    let got = cap_both_orders(&room, &s, vec![g1.clone(), g2.clone(), forged], 2);
+    assert_eq!(got, vec![ban_record(&g1), ban_record(&g2)]);
+}