diff --git a/Cargo.lock b/Cargo.lock index 954c769c..b0881d82 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6226,6 +6226,7 @@ dependencies = [ "bincode", "iroh-base", "serde", + "serde_json", "sha2 0.10.9", "tracing", "willow-identity", diff --git a/crates/agent/src/tools.rs b/crates/agent/src/tools.rs index fc20303c..de32f563 100644 --- a/crates/agent/src/tools.rs +++ b/crates/agent/src/tools.rs @@ -549,9 +549,18 @@ impl WillowToolRouter { } "set_permission" => { let p: SetPermissionParams = parse_args(&args)?; + let perm = willow_state::Permission::from_name(&p.permission).ok_or_else(|| { + ErrorData::invalid_params( + format!( + "unknown permission '{}'; valid: SyncProvider, ManageChannels, ManageRoles, SendMessages, CreateInvite", + p.permission + ), + None, + ) + })?; match self .client - .set_permission(&p.role_id, &p.permission, p.granted) + .set_permission(&p.role_id, perm, p.granted) .await { Ok(()) => success_json(serde_json::json!({"success": true})), diff --git a/crates/client/src/actions.rs b/crates/client/src/actions.rs index 24e2f1ed..31123f92 100644 --- a/crates/client/src/actions.rs +++ b/crates/client/src/actions.rs @@ -196,11 +196,10 @@ impl ClientHandle { pub async fn set_permission( &self, role_id: &str, - permission: &str, + permission: willow_state::Permission, granted: bool, ) -> anyhow::Result<()> { let role_id = role_id.to_string(); - let permission = permission.to_string(); let event = self .mutation_handle .build_event(willow_state::EventKind::SetPermission { diff --git a/crates/client/src/views.rs b/crates/client/src/views.rs index de51c3db..0181e03a 100644 --- a/crates/client/src/views.rs +++ b/crates/client/src/views.rs @@ -299,6 +299,9 @@ pub struct RolesView { pub struct RoleEntry { pub id: String, pub name: String, + /// Permission names (string form) in stable, deduplicated order. + /// Surfaced as strings so UI / accessor consumers stay format-stable + /// even as the underlying [`willow_state::Permission`] enum grows. pub permissions: Vec, } @@ -786,7 +789,7 @@ pub fn compute_roles_view(events: &Arc) -> RolesView .map(|role| RoleEntry { id: role.id.clone(), name: role.name.clone(), - permissions: role.permissions.iter().cloned().collect(), + permissions: role.permissions.iter().map(|p| format!("{p:?}")).collect(), }) .collect(); roles.sort_by(|a, b| a.name.cmp(&b.name)); diff --git a/crates/state/Cargo.toml b/crates/state/Cargo.toml index 0c1725e1..24bf8562 100644 --- a/crates/state/Cargo.toml +++ b/crates/state/Cargo.toml @@ -12,3 +12,6 @@ iroh-base = { workspace = true, features = ["key"] } serde = { workspace = true } sha2 = "0.10" willow-identity = { path = "../identity" } + +[dev-dependencies] +serde_json = "1" diff --git a/crates/state/src/event.rs b/crates/state/src/event.rs index 82be5da3..885180c8 100644 --- a/crates/state/src/event.rs +++ b/crates/state/src/event.rs @@ -4,7 +4,7 @@ //! containing an [`EventKind`]. Events are content-addressed — their //! identity is their SHA-256 hash. -use serde::{Deserialize, Serialize}; +use serde::{Deserialize, Deserializer, Serialize}; use willow_identity::{EndpointId, Identity, Signature}; use crate::hash::EventHash; @@ -17,7 +17,7 @@ use crate::hash::EventHash; /// [`ProposedAction`] and the vote path. This structural separation makes /// it impossible for any peer to grant admin via a direct /// [`EventKind::GrantPermission`] event. -#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)] +#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize)] pub enum Permission { /// Can sync/provide full history to other peers. SyncProvider, @@ -30,6 +30,124 @@ pub enum Permission { SendMessages, /// Can create invites. CreateInvite, + /// Sentinel for permission names that an unknown / future client + /// emitted. Reached only via the back-compat string-form deserialize + /// path (e.g. an MCP tool boundary or a legacy JSON snapshot). + /// `apply_event` treats this sentinel as a no-op so the event still + /// joins the DAG — preserving signatures + hash linkage — without + /// mutating any role's permission set. + /// + /// Hidden from generated docs; never emitted by Willow itself. + #[doc(hidden)] + #[serde(skip)] + __UnknownLegacy, +} + +impl Permission { + /// Try to parse a permission name from its string form. + /// + /// Returns `None` for unknown names. Used by the agent MCP tool + /// boundary, which rejects unknown permissions before they enter + /// the DAG. + pub fn from_name(name: &str) -> Option { + match name { + "SyncProvider" => Some(Self::SyncProvider), + "ManageChannels" => Some(Self::ManageChannels), + "ManageRoles" => Some(Self::ManageRoles), + "SendMessages" => Some(Self::SendMessages), + "CreateInvite" => Some(Self::CreateInvite), + _ => None, + } + } +} + +impl<'de> Deserialize<'de> for Permission { + /// Custom deserialize that accepts both the enum form (default for + /// any format — bincode emits a u32 discriminant, JSON emits the + /// variant name) and tolerates unknown variant names by mapping + /// them to the [`Permission::__UnknownLegacy`] sentinel. + /// + /// This lets a peer running an older or rogue client that broadcast + /// an unrecognised permission name still have its event join the + /// DAG; `apply_event` then silently drops the unknown perm so the + /// role's permission set is never polluted. + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + struct PermissionVisitor; + + impl<'de> serde::de::Visitor<'de> for PermissionVisitor { + type Value = Permission; + + fn expecting(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + f.write_str("a Permission variant") + } + + // String form (JSON, MCP boundary, legacy snapshots). + fn visit_str(self, v: &str) -> Result { + Ok(Permission::from_name(v).unwrap_or_else(|| { + tracing::warn!( + permission = %v, + "unknown permission name; mapping to __UnknownLegacy", + ); + Permission::__UnknownLegacy + })) + } + + // Owned-string variant — same handling. + fn visit_string(self, v: String) -> Result { + self.visit_str(&v) + } + + // Bincode encodes a unit-variant enum as the discriminant + // index via `deserialize_enum`, which routes through this + // method. Forward to the standard derived parser. + fn visit_enum(self, data: A) -> Result + where + A: serde::de::EnumAccess<'de>, + { + use serde::de::VariantAccess; + #[derive(Deserialize)] + enum Tag { + SyncProvider, + ManageChannels, + ManageRoles, + SendMessages, + CreateInvite, + } + let (tag, variant) = data.variant::()?; + variant.unit_variant()?; + Ok(match tag { + Tag::SyncProvider => Permission::SyncProvider, + Tag::ManageChannels => Permission::ManageChannels, + Tag::ManageRoles => Permission::ManageRoles, + Tag::SendMessages => Permission::SendMessages, + Tag::CreateInvite => Permission::CreateInvite, + }) + } + } + + if deserializer.is_human_readable() { + // JSON-style formats encode unit variants as strings; route + // through the visitor so unknown names hit the sentinel. + deserializer.deserialize_any(PermissionVisitor) + } else { + // Bincode-style formats encode unit variants as discriminant + // indices; route through the enum visitor. + deserializer.deserialize_enum( + "Permission", + &[ + "SyncProvider", + "ManageChannels", + "ManageRoles", + "SendMessages", + "CreateInvite", + ], + PermissionVisitor, + ) + } + } } // ───── Governance types ──────────────────────────────────────────────────── @@ -125,7 +243,7 @@ pub enum EventKind { /// Set or clear a permission on a role. SetPermission { role_id: String, - permission: String, + permission: Permission, granted: bool, }, /// Assign a role to a member. diff --git a/crates/state/src/materialize.rs b/crates/state/src/materialize.rs index fcba801e..416e6987 100644 --- a/crates/state/src/materialize.rs +++ b/crates/state/src/materialize.rs @@ -418,9 +418,20 @@ fn apply_mutation(state: &mut ServerState, event: &Event) -> ApplyResult { permission, granted, } => { + // Drop the unknown-legacy sentinel produced by the + // back-compat deserialize path so a rogue / future client + // can never inject an unrecognised permission name into a + // role's permission set. + if matches!(permission, Permission::__UnknownLegacy) { + tracing::warn!( + role_id = %role_id, + "SetPermission with unknown legacy permission; dropping", + ); + return ApplyResult::Applied; + } if let Some(role) = state.roles.get_mut(role_id) { if *granted { - role.permissions.insert(permission.clone()); + role.permissions.insert(*permission); } else { role.permissions.remove(permission); } diff --git a/crates/state/src/tests.rs b/crates/state/src/tests.rs index 33c986a6..aa4272a0 100644 --- a/crates/state/src/tests.rs +++ b/crates/state/src/tests.rs @@ -446,7 +446,7 @@ fn set_permission_on_nonexistent_role_is_noop() { &admin, EventKind::SetPermission { role_id: "nonexistent".to_string(), - permission: "SendMessages".to_string(), + permission: Permission::SendMessages, granted: true, }, ); @@ -856,7 +856,7 @@ fn has_permission_ignores_role_based_permissions() { &admin, EventKind::SetPermission { role_id: "r-1".to_string(), - permission: "SendMessages".to_string(), + permission: Permission::SendMessages, granted: true, }, ); @@ -4376,3 +4376,138 @@ fn derive_ephemeral_state_bands() { EphemeralState::Archived ); } + +// ───── SetPermission typed permission round-trip ───────────────────────── + +/// Emit `SetPermission` with a typed `Permission` enum value, serialize +/// via the wire format (`willow-transport` = bincode), deserialize, apply +/// to a fresh DAG, and assert the role's permission set contains the +/// typed permission. +#[test] +fn set_permission_with_typed_permission_round_trips() { + let admin = Identity::generate(); + let mut dag = test_dag(&admin); + + do_emit( + &mut dag, + &admin, + EventKind::CreateRole { + name: "mod".into(), + role_id: "r-1".into(), + }, + ); + let set_event = do_emit( + &mut dag, + &admin, + EventKind::SetPermission { + role_id: "r-1".into(), + permission: Permission::ManageChannels, + granted: true, + }, + ); + + // Wire-round-trip the event through bincode (the format used by + // `willow-transport` and the storage layer) and re-apply. + let bytes = bincode::serialize(&set_event).unwrap(); + let decoded: Event = bincode::deserialize(&bytes).unwrap(); + match &decoded.kind { + EventKind::SetPermission { permission, .. } => { + assert_eq!(*permission, Permission::ManageChannels); + } + other => panic!("expected SetPermission, got {other:?}"), + } + + let state = materialize(&dag); + let role = state.roles.get("r-1").expect("role created"); + assert!(role.permissions.contains(&Permission::ManageChannels)); +} + +/// Synthesize a JSON document carrying the legacy `permission: ""` +/// string form (the shape MCP / agent boundary accepts) and assert the +/// custom deserializer maps it to the typed `Permission::ManageChannels`. +#[test] +fn set_permission_legacy_string_form_still_loads() { + let json = serde_json::json!({ + "SetPermission": { + "role_id": "r-1", + "permission": "ManageChannels", + "granted": true, + } + }); + let kind: EventKind = serde_json::from_value(json).expect("legacy string form must load"); + match kind { + EventKind::SetPermission { permission, .. } => { + assert_eq!(permission, Permission::ManageChannels); + } + other => panic!("expected SetPermission, got {other:?}"), + } +} + +/// Unknown legacy permission strings deserialize successfully (so the +/// event still enters the DAG and the chain is not broken) but apply as +/// a no-op — the unknown name is dropped. +#[test] +fn set_permission_legacy_unknown_string_drops_silently() { + let json = serde_json::json!({ + "SetPermission": { + "role_id": "r-1", + "permission": "FrobnicateWidgets", + "granted": true, + } + }); + let kind: EventKind = + serde_json::from_value(json).expect("unknown legacy string must deserialize, not fail"); + match kind { + EventKind::SetPermission { permission, .. } => { + // Unknown name is mapped to the sentinel that apply_event drops. + assert_eq!(permission, Permission::__UnknownLegacy); + } + other => panic!("expected SetPermission, got {other:?}"), + } + + // Apply path: synthesize the post-deserialize event in memory (the + // sentinel never crosses the wire — it only exists after a custom + // deserialize from an unrecognised string form). Bypass `do_emit` + // (which signs + bincodes the kind) and feed the event directly to + // `apply_incremental`, mirroring what would happen if a JSON + // snapshot containing the unknown name were replayed into state. + use crate::materialize::apply_incremental; + + let admin = Identity::generate(); + let mut dag = test_dag(&admin); + do_emit( + &mut dag, + &admin, + EventKind::CreateRole { + name: "mod".into(), + role_id: "r-1".into(), + }, + ); + let mut state = materialize(&dag); + + // Fabricate an event whose kind carries the sentinel; we reuse a + // valid hash from the genesis chain since `apply_event` does not + // re-verify signatures and we only care about the apply branch. + let signable = EventKind::SetPermission { + role_id: "r-1".into(), + permission: Permission::__UnknownLegacy, + granted: true, + }; + let synthetic = Event { + hash: EventHash::from_bytes(b"synthetic-unknown-legacy"), + author: admin.endpoint_id(), + seq: 99, + prev: EventHash::ZERO, + deps: vec![], + kind: signable, + sig: willow_identity::Signature::from_bytes(&[0u8; 64]), + timestamp_hint_ms: 0, + }; + let _ = apply_incremental(&mut state, &synthetic); + + let role = state.roles.get("r-1").expect("role created"); + assert!( + role.permissions.is_empty(), + "unknown legacy permission must apply as a no-op" + ); +} diff --git a/crates/state/src/types.rs b/crates/state/src/types.rs index a60988d4..d750f98c 100644 --- a/crates/state/src/types.rs +++ b/crates/state/src/types.rs @@ -57,8 +57,8 @@ pub struct Role { pub id: String, /// Human-readable name (e.g. "Moderator"). pub name: String, - /// The set of permission strings this role grants. - pub permissions: BTreeSet, + /// The set of typed permissions this role grants. + pub permissions: BTreeSet, } /// A peer's membership record within a server. diff --git a/crates/web/src/components/roles.rs b/crates/web/src/components/roles.rs index cd2b7e8f..59f0dfb0 100644 --- a/crates/web/src/components/roles.rs +++ b/crates/web/src/components/roles.rs @@ -198,7 +198,14 @@ pub fn RoleManager( let rid = rid_t.clone(); let perm = perm_toggle.clone(); wasm_bindgen_futures::spawn_local(async move { - let _ = h.set_permission(&rid, &perm, granted).await; + // Names come from PERMISSION_NAMES which is + // kept in sync with willow_state::Permission; + // an unparsed name is a bug, not user input. + if let Some(parsed) = + willow_state::Permission::from_name(&perm) + { + let _ = h.set_permission(&rid, parsed, granted).await; + } }); } />