fix(admin): revalidate ownership transfer candidates

.cargo/audit.toml

@@ -0,0 +1,9 @@
+# `h2` is only present through the off-chain `credence_admin_cli` RPC client:
+# soroban-client 0.5.8 -> reqwest 0.11 -> hyper 0.14 -> h2 0.3. The advisory
+# is an unbounded-empty-DATA-frame client availability issue; it cannot affect
+# contract WASM, contract state, or submitted transactions. The upstream
+# soroban-client release line still requires reqwest 0.11, while the advisory
+# fix requires h2 >= 0.4.16. Reassess this exception when soroban-client moves
+# to reqwest/h2 0.4 or later.
+[advisories]
+ignore = ["RUSTSEC-2026-0258"]

contracts/admin/ATOMICITY.md

@@ -0,0 +1,22 @@
+# Admin operation atomicity
+
+Administrative transactions execute in Soroban's atomic invocation boundary:
+if an entrypoint returns an error, its storage writes and contract events are
+rolled back together. The admin contract validates all ownership-transfer
+preconditions before its first ownership write.
+
+## Ownership invariant
+
+`transfer_ownership` records only a pending proposal. `accept_ownership`
+rechecks that the pending candidate is currently an active, unsuspended
+`SuperAdmin` after the timelock. A candidate removed, demoted, deactivated, or
+suspended during that window cannot receive ownership. The failed acceptance
+leaves the owner, pending owner, proposal timestamp, and emitted event set
+unchanged, so the current owner can recover by cancelling or replacing the
+proposal through the existing transfer flow.
+
+This is compatible with the public interface and storage layout: no migration
+is required. It intentionally adds one failure condition at acceptance for a
+proposal whose candidate is no longer currently eligible. The security model
+assumes Soroban preserves atomicity for a failed invocation and that the
+ledger timestamp is the authoritative timelock and suspension clock.

contracts/admin/src/lib.rs

@@ -23,6 +23,8 @@
 
 pub mod pausable;
 
+#[cfg(test)]
+mod test_atomic_rollback;
 /// Event schema regression tests — verifies topic/data layout for every
 /// role event without involving contract storage.
 #[cfg(test)]
@@ -773,20 +775,10 @@ impl AdminContract {
             panic_with_error!(&e, ContractError::InvalidPauseAction);
         }
 
-        // Verify new owner is a SuperAdmin
-        let new_owner_info: AdminInfo = e
-            .storage()
-            .instance()
-            .get(&DataKey::AdminInfo(new_owner.clone()))
-            .unwrap_or_else(|| panic_with_error!(&e, ContractError::NotAdmin));
-
-        if new_owner_info.role != AdminRole::SuperAdmin {
-            panic_with_error!(&e, ContractError::NotAdmin);
-        }
-
-        if !new_owner_info.active {
-            panic_with_error!(&e, ContractError::AlreadyDeactivated);
-        }
+        // A suspended or deactivated admin must not be able to receive durable
+        // ownership. This check is repeated by `accept_ownership`, because the
+        // candidate's status can change during the timelock.
+        Self::require_effective_super_admin(&e, &new_owner);
 
         // Store pending owner and proposal timestamp for timelock
         e.storage()
@@ -852,6 +844,12 @@ impl AdminContract {
             panic_with_error!(&e, ContractError::TimelockNotReady);
         }
 
+        // Revalidate immediately before the first ownership write. A proposal
+        // is only an intent: its candidate may have been removed, deactivated,
+        // suspended, or demoted while the timelock elapsed. Failing here leaves
+        // the owner, pending owner, timestamp, and event stream untouched.
+        Self::require_effective_super_admin(&e, &pending_owner);
+
         // Get current owner for event emission
         let previous_owner: Address = e
             .storage()
@@ -1157,6 +1155,30 @@ impl AdminContract {
         }
     }
 
+    /// Require an owner candidate to be an effective SuperAdmin now.
+    ///
+    /// Ownership proposals are intentionally revalidated at acceptance, not
+    /// trusted based on the state at proposal time. This preserves the
+    /// two-step transfer API while preventing a stale proposal from granting
+    /// durable authority to an inactive, suspended, removed, or demoted admin.
+    fn require_effective_super_admin(e: &Env, candidate: &Address) {
+        let admin_info: AdminInfo = e
+            .storage()
+            .instance()
+            .get(&DataKey::AdminInfo(candidate.clone()))
+            .unwrap_or_else(|| panic_with_error!(e, ContractError::NotAdmin));
+
+        if admin_info.role != AdminRole::SuperAdmin {
+            panic_with_error!(e, ContractError::NotAdmin);
+        }
+        if !admin_info.active {
+            panic_with_error!(e, ContractError::AlreadyDeactivated);
+        }
+        if e.ledger().timestamp() < admin_info.suspended_until {
+            panic_with_error!(e, ContractError::AdminSuspended);
+        }
+    }
+
     /// Historical role check: verify that `actor` held at least `role` at
     /// ledger timestamp `at_ledger`.
     ///

contracts/admin/src/test_atomic_rollback.rs

@@ -0,0 +1,70 @@
+//! Regression tests for atomic ownership-transfer failure handling.
+//!
+//! These use the generated contract client (`try_accept_ownership`) so the
+//! assertions exercise Soroban's transaction boundary rather than a direct
+//! Rust call. A rejected acceptance must retain the current owner, proposal,
+//! and event stream exactly as they were before the attempted transaction.
+
+use crate::*;
+use soroban_sdk::{
+    testutils::{Address as _, Ledger as _},
+    Address, Env,
+};
+
+fn setup() -> (Env, Address, AdminContractClient<'static>, Address, Address) {
+    let env = Env::default();
+    let contract_id = env.register_contract(None, AdminContract);
+    let client = AdminContractClient::new(&env, &contract_id);
+    let owner = Address::generate(&env);
+    let candidate = Address::generate(&env);
+
+    env.mock_all_auths();
+    client.initialize(&owner, &1, &100);
+    client.add_admin(&owner, &candidate, &AdminRole::SuperAdmin);
+    client.transfer_ownership(&owner, &candidate);
+    env.ledger()
+        .with_mut(|ledger| ledger.timestamp += OWNERSHIP_TRANSFER_TIMELOCK);
+
+    (env, contract_id, client, owner, candidate)
+}
+
+#[test]
+fn rejected_acceptance_rolls_back_when_candidate_is_deactivated() {
+    let (env, contract_id, client, owner, candidate) = setup();
+    // Failure injection at the acceptance boundary: a candidate can become
+    // inactive through a future administrative recovery path. We write that
+    // terminal state directly because peer SuperAdmins cannot deactivate one
+    // another through the public permission model.
+    env.as_contract(&contract_id, || {
+        let mut info: AdminInfo = env
+            .storage()
+            .instance()
+            .get(&DataKey::AdminInfo(candidate.clone()))
+            .unwrap();
+        info.active = false;
+        env.storage()
+            .instance()
+            .set(&DataKey::AdminInfo(candidate.clone()), &info);
+    });
+
+    let events_before = env.events().all().len();
+    assert!(client.try_accept_ownership(&candidate).is_err());
+
+    assert_eq!(client.get_owner(), owner);
+    assert_eq!(client.get_pending_owner(), Some(candidate));
+    assert_eq!(env.events().all().len(), events_before);
+}
+
+#[test]
+fn rejected_acceptance_rolls_back_when_candidate_is_suspended() {
+    let (env, _contract_id, client, owner, candidate) = setup();
+    let suspension_end = env.ledger().timestamp() + 1;
+    client.suspend_admin(&owner, &candidate, &suspension_end);
+
+    let events_before = env.events().all().len();
+    assert!(client.try_accept_ownership(&candidate).is_err());
+
+    assert_eq!(client.get_owner(), owner);
+    assert_eq!(client.get_pending_owner(), Some(candidate));
+    assert_eq!(env.events().all().len(), events_before);
+}