diff --git a/crates/myownmesh-core/src/engine/governance.rs b/crates/myownmesh-core/src/engine/governance.rs index 952983f..6d5359f 100644 --- a/crates/myownmesh-core/src/engine/governance.rs +++ b/crates/myownmesh-core/src/engine/governance.rs @@ -1336,11 +1336,19 @@ fn member_tier_timestamp(state: &Arc, variant: &TransitionVariant) target, role: Role::Member, } => target.as_str(), - TransitionVariant::RoleRevoke { target } | TransitionVariant::Evict { target } - if gov.role_of(target) == Role::Member => - { + // A revoke rides the member log (and needs the monotonic stamp) only when + // it targets a plain member; a revoke of an owner/manager stays governance + // tier and just demotes, so it takes the wall clock below. + TransitionVariant::RoleRevoke { target } if gov.role_of(target) == Role::Member => { target.as_str() } + // An evict is tombstoned in the member log at *either* tier (see + // `try_ratify`) — to suppress the target's member-tier admit — so it must + // ALWAYS be stamped strictly past the target's newest member-log entry. + // Otherwise a same-second admit wins the last-writer-wins tie and the + // evicted device survives: a promoted owner/manager keeps the plain-member + // admit it was given before promotion, and re-appears fleet-wide. + TransitionVariant::Evict { target } => target.as_str(), _ => return now, }; let newest = gov @@ -1437,6 +1445,21 @@ async fn try_ratify(state: &Arc, proposal_id: &str) -> Result<()> { // Apply to the governance log (also advances `gov.roles`). let after = network_state::apply_transition(gov.clone(), &transition); *gov = after; + // Evicting a device promoted past plain member (an owner or manager) + // still leaves its *original* member-tier admit in the member log. + // On its own the governance-log evict removes the role but not that + // admit, so any peer that re-derives membership from the signed logs + // — the gossip-adoption path a co-owner runs after being offline for + // the kick — folds the stale admit back in and resurrects the evicted + // device as a plain member: it lingers in the roster, still + // authorised, and nobody but the evicting owner sees it gone. Record + // the evict in the union-merged member log too, so it tombstones that + // admit; the removal then converges network-wide and survives + // concurrent authors, exactly like a plain-member evict. + if matches!(&transition.variant, TransitionVariant::Evict { .. }) { + gov.member_log.push(transition.clone()); + gov.roles = project_roles(&state.network_id, &gov.transitions, &gov.member_log); + } } gov.pending.retain(|p| p.id != proposal_id); network_state::save(&gov)?; @@ -1458,6 +1481,20 @@ async fn try_ratify(state: &Arc, proposal_id: &str) -> Result<()> { crate::roster::set_role_in(&mut roster, target, *role); crate::roster::save(&roster)?; } + if let TransitionVariant::RoleRevoke { target } = &transition.variant { + // Withdrawal back to a plain member: unlike an evict the device stays + // in the roster, but its cached authority tag has to drop to `member` + // so this node's peer rows — and everything that reads the roster role, + // including the fleet UI's grant/withdraw controls — reflect the + // demotion at once. The gossip-adoption path already reprojects the + // whole role map onto the roster; the local ratify path open-codes + // per-variant mirrors and previously skipped revoke entirely, so on the + // very device that authored the withdrawal the role never "took". + let mut roster = state.roster.write(); + if crate::roster::set_role_in(&mut roster, target, Role::Member) { + crate::roster::save(&roster)?; + } + } if let TransitionVariant::KindChange { to: NetworkKind::Closed, } = &transition.variant diff --git a/crates/myownmesh-core/src/engine/mod.rs b/crates/myownmesh-core/src/engine/mod.rs index 0832997..609fb3c 100644 --- a/crates/myownmesh-core/src/engine/mod.rs +++ b/crates/myownmesh-core/src/engine/mod.rs @@ -3390,10 +3390,15 @@ mod tests { let state = build_test_state("evicted-no-reconnect"); let net = state.network_id.clone(); - // Lex-greater than any base32 identity, so `we_offer` holds; dash-free, - // so `pubkey_part` leaves the id whole and the evict target matches. + // Both must sort lex-greater than any base32 identity so `we_offer` + // holds (we're the offerer) and dash-free so `pubkey_part` leaves the id + // whole and the evict target matches. base32-lowercase tops out at 'z' in + // a 52-char id, so any all-'z' string longer than 52 chars clears every + // identity deterministically — a plain "y"*60 did not (a ~3 % of ephemeral + // identities that happen to start with 'z' sort above it, flaking the + // precondition assert). Distinct lengths keep the two ids distinct. let evicted = "z".repeat(60); - let live = "y".repeat(60); + let live = "z".repeat(61); assert!( state.identity.public_id() < evicted.as_str() && state.identity.public_id() < live.as_str(), diff --git a/crates/myownmesh-core/tests/closed_network_governance.rs b/crates/myownmesh-core/tests/closed_network_governance.rs index e4ff4dc..bbe3318 100644 --- a/crates/myownmesh-core/tests/closed_network_governance.rs +++ b/crates/myownmesh-core/tests/closed_network_governance.rs @@ -752,6 +752,201 @@ async fn re_admitting_an_evicted_member_supersedes_the_tombstone() { ); } +#[tokio::test] +async fn evicting_a_promoted_member_tombstones_its_member_admit() { + // Regression for "I removed an owner/manager, but it stays controllable and + // the other owners still see it in the fleet." + // + // A device promoted past plain member (admitted as Member, then granted + // Controller/Owner) still carries its original member-tier admit in the + // member log. Evicting it extends the owner (governance) log — but any peer + // that re-derives membership straight from the signed logs, which is exactly + // what a co-owner does when it adopts the log via gossip (e.g. it was offline + // during the kick), folds that stale admit back in and resurrects the evicted + // device as a plain member: it lingers in the roster, still authorised to + // control the fleet, and every such owner keeps seeing it. The evict must + // tombstone the admit so the projected membership drops the device and the + // roster mirror prunes it — the same convergence a plain-member evict gets. + // + // This drives the projection functions the gossip-adoption path + // (`project_roles` / the roster mirror in `adopt_transition_log`) is built + // on, so it fails deterministically without the fix — unlike an online peer, + // which ratifies the evict incrementally and never hits the resurrecting + // re-projection. + shared_home(); + + let transport = Transport::new().expect("transport"); + let alice_id = Arc::new(Identity::ephemeral()); + let carol_id = Arc::new(Identity::ephemeral()); + let carol_pk = carol_id.public_id().to_string(); + + let network_id = "evict-promoted-projection-net"; + let (alice_state, _ad) = spawn_network( + fresh_network("alice", network_id), + alice_id.clone(), + transport.clone(), + ) + .await + .expect("alice engine"); + + use myownmesh_core::engine::governance::propose; + propose( + &alice_state, + TransitionVariant::KindChange { + to: NetworkKind::Closed, + }, + None, + ) + .await + .expect("found"); + // Admit Carol as a plain member (member log), then promote her to manager + // (governance log) — so her stale member-tier admit outlives the promotion. + propose( + &alice_state, + TransitionVariant::RoleGrant { + target: carol_pk.clone(), + role: Role::Member, + }, + None, + ) + .await + .expect("admit carol"); + propose( + &alice_state, + TransitionVariant::RoleGrant { + target: carol_pk.clone(), + role: Role::Controller, + }, + None, + ) + .await + .expect("promote carol"); + assert_eq!( + alice_state.governance_state.read().role_of(&carol_pk), + Role::Controller, + "carol should be a manager after promotion" + ); + + // Evict the manager. + propose( + &alice_state, + TransitionVariant::Evict { + target: carol_pk.clone(), + }, + None, + ) + .await + .expect("evict carol"); + + // Re-derive membership from the signed logs exactly as a co-owner adopting + // via gossip does. Carol must be gone from the projected membership and — via + // the removed set the roster mirror prunes by — not resurrected by her stale + // member-tier admit. + let g = alice_state.governance_state.read(); + let verified = myownmesh_core::network_state::verify_log(network_id, &g.transitions) + .expect("owner log verifies"); + let members = + myownmesh_core::network_state::verify_member_log(&verified, &g.member_log, network_id); + assert!( + !members.contains(&carol_pk), + "an evicted manager must not project back as a member from its stale admit" + ); + let removed = + myownmesh_core::network_state::member_log_removed(&verified, &g.member_log, network_id); + assert!( + removed.contains(&carol_pk), + "an evicted manager must land in the member-log removed set so the roster \ + mirror prunes it on every peer, not just the owner that authored the evict" + ); +} + +#[tokio::test] +async fn withdrawing_a_role_updates_the_local_roster_tag() { + // Regression: withdrawing a peer's role (owner/manager → plain member) must + // update the *authoring* device's cached roster tag, not just the projected + // `roles` map. The gossip-adoption path reprojects the whole role map onto + // the roster, but the local ratify path open-coded per-variant mirrors and + // skipped RoleRevoke entirely — so on the device that authored the + // withdrawal, the peer's row kept rendering the old authority and the + // downgrade "didn't take". Single engine: the owner authors the whole chain + // and we read the on-disk roster tag it mirrors for its own peer rows. + shared_home(); + + let transport = Transport::new().expect("transport"); + let alice_id = Arc::new(Identity::ephemeral()); + let bob_id = Arc::new(Identity::ephemeral()); + let bob_pk = bob_id.public_id().to_string(); + + let network_id = "withdraw-role-net"; + let (alice_state, _ad) = spawn_network( + fresh_network("alice", network_id), + alice_id.clone(), + transport.clone(), + ) + .await + .expect("alice engine"); + + use myownmesh_core::engine::governance::propose; + propose( + &alice_state, + TransitionVariant::KindChange { + to: NetworkKind::Closed, + }, + None, + ) + .await + .expect("found"); + propose( + &alice_state, + TransitionVariant::RoleGrant { + target: bob_pk.clone(), + role: Role::Member, + }, + None, + ) + .await + .expect("admit bob"); + propose( + &alice_state, + TransitionVariant::RoleGrant { + target: bob_pk.clone(), + role: Role::Controller, + }, + None, + ) + .await + .expect("promote bob"); + // The mirrored roster tag should read controller on the authoring device. + wait_for(Duration::from_secs(5), || { + roster_role(&alice_state, &bob_pk) == Some(Role::Controller) + }) + .await; + + // Withdraw Bob's role back to a plain member. + propose( + &alice_state, + TransitionVariant::RoleRevoke { + target: bob_pk.clone(), + }, + None, + ) + .await + .expect("withdraw bob"); + + // The cached roster tag must drop to member on the authoring device — the + // withdrawal has to "take" right where the owner performed it. + assert_eq!( + roster_role(&alice_state, &bob_pk), + Some(Role::Member), + "withdrawing a role must reset the authoring device's roster tag to member" + ); + // ...and Bob stays in the roster — a withdraw demotes, it doesn't remove. + assert!( + rostered(&alice_state, &bob_pk), + "a withdrawn member stays in the fleet — only its authority drops" + ); +} + #[tokio::test] async fn evicted_offline_device_learns_on_reconnect_and_stands_down() { // The "offline and lost devices just keep showing back up" loop, killed @@ -1159,6 +1354,21 @@ fn rostered(state: &Arc, id: &str) myownmesh_core::roster::is_authorized(&state.roster.read(), id) } +/// The cached authority tag for `id` in `state`'s on-disk roster, if the peer +/// is present. This is the projection the fleet UI renders each member's +/// grant/withdraw controls from, so a role change that doesn't reach here +/// "doesn't take" on the device that authored it. +fn roster_role(state: &Arc, id: &str) -> Option { + let pk = myownmesh_core::signing::pubkey_part(id); + state + .roster + .read() + .authorized_devices + .iter() + .find(|p| p.device_id == pk) + .map(|p| p.role) +} + /// All tests in this file share ONE `MYOWNMESH_HOME` for the process lifetime. /// Each `#[tokio::test]` runs on its own thread, but `MYOWNMESH_HOME` is a /// process-global env var — per-test tempdirs would clobber each other, and diff --git a/crates/myownmesh/src/control.rs b/crates/myownmesh/src/control.rs index 5cfa240..4f3bc40 100644 --- a/crates/myownmesh/src/control.rs +++ b/crates/myownmesh/src/control.rs @@ -1114,7 +1114,25 @@ async fn dispatch(state: &Arc, req: Request) -> Response { // ---- governance ---- Request::GovernanceState { network } => match state.registry.get(&network) { Some(net) => match net.governance_state().await { - Ok(s) => Response::ok(serde_json::json!({ "state": s })), + Ok(s) => { + // The devices the signed logs have **removed** (evicted, or a + // member-tier revoke) — the authoritative "no longer in the + // fleet" set, projected from the same member log membership + // rides. Surfaced alongside the state so a client can prune + // its own local bookkeeping for a device *another* owner + // evicted: that eviction converges the signed roster but never + // touches the evicting-from-afar owner's local claimed-list, + // which would otherwise re-admit the device on the next + // re-assertion. + let evicted: Vec = myownmesh_core::network_state::member_log_removed( + &s, + &s.member_log, + &network, + ) + .into_iter() + .collect(); + Response::ok(serde_json::json!({ "state": s, "evicted": evicted })) + } Err(e) => Response::err(e.to_string()), }, None => Response::err(format!("unknown network: {network}")),