From 2c8ba4460dc74635336574567bddc60880d1cfc8 Mon Sep 17 00:00:00 2001 From: Jean Date: Fri, 17 Jul 2026 13:26:16 +0000 Subject: [PATCH] forbid: type the FORBID kind as an enum at the store boundary, string only in storage --- api/src/lib.rs | 53 ++++++++++++++++++++++++++++++---- modules/chanserv/src/lib.rs | 4 +-- modules/nickserv/src/set.rs | 4 +-- modules/operserv/src/forbid.rs | 28 +++++++----------- src/engine/db/mod.rs | 2 +- src/engine/db/network.rs | 29 ++++++++++++------- src/engine/db/store.rs | 6 ++-- src/engine/db/tests.rs | 8 ++--- src/engine/register.rs | 4 +-- src/engine/tests.rs | 2 +- 10 files changed, 91 insertions(+), 49 deletions(-) diff --git a/api/src/lib.rs b/api/src/lib.rs index 689b0ac..1bb12b8 100644 --- a/api/src/lib.rs +++ b/api/src/lib.rs @@ -795,10 +795,41 @@ pub struct AkillView { pub expires: Option, } -// A registration ban held by OperServ FORBID (kind = NICK/CHAN/EMAIL). +// A registration-ban target for OperServ FORBID. The persistence/gossip layer +// stores the wire token (`wire()`); the Store API is typed so a call site can't +// pass a mistyped kind like "NCK" (it wouldn't compile). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ForbidKind { + Nick, + Chan, + Email, +} + +impl ForbidKind { + /// The stored/display token: "NICK" / "CHAN" / "EMAIL". + pub fn wire(self) -> &'static str { + match self { + Self::Nick => "NICK", + Self::Chan => "CHAN", + Self::Email => "EMAIL", + } + } + + /// Parse a FORBID kind argument (case-insensitive, with common aliases). + pub fn from_name(s: &str) -> Option { + match s.to_ascii_uppercase().as_str() { + "NICK" | "NICKNAME" | "ACCOUNT" => Some(Self::Nick), + "CHAN" | "CHANNEL" => Some(Self::Chan), + "EMAIL" | "MAIL" => Some(Self::Email), + _ => None, + } + } +} + +// A registration ban held by OperServ FORBID. #[derive(Debug, Clone)] pub struct ForbidView { - pub kind: String, + pub kind: ForbidKind, pub mask: String, pub setter: String, pub reason: String, @@ -1140,10 +1171,10 @@ pub trait Store { fn akill_del(&mut self, kind: &str, mask: &str) -> Result; fn akills(&self) -> Vec; // Registration bans (OperServ FORBID). - fn forbid_add(&mut self, kind: &str, mask: &str, setter: &str, reason: &str) -> Result; - fn forbid_del(&mut self, kind: &str, mask: &str) -> Result; + fn forbid_add(&mut self, kind: ForbidKind, mask: &str, setter: &str, reason: &str) -> Result; + fn forbid_del(&mut self, kind: ForbidKind, mask: &str) -> Result; fn forbids(&self) -> Vec; - fn is_forbidden(&self, kind: &str, name: &str) -> Option; + fn is_forbidden(&self, kind: ForbidKind, name: &str) -> Option; // Services ignores (node-local, oper-only). fn ignore_add(&mut self, mask: &str, reason: &str, expires: Option); fn ignore_del(&mut self, mask: &str) -> bool; @@ -1522,6 +1553,18 @@ mod tests { assert_eq!(Priv::from_name("nope"), None); assert_eq!(Priv::valid_names(), "auspex, suspend, admin"); } + + #[test] + fn forbid_kind_parses_aliases_and_round_trips_its_wire_token() { + assert_eq!(ForbidKind::from_name("nickname"), Some(ForbidKind::Nick)); + assert_eq!(ForbidKind::from_name("ACCOUNT"), Some(ForbidKind::Nick)); + assert_eq!(ForbidKind::from_name("channel"), Some(ForbidKind::Chan)); + assert_eq!(ForbidKind::from_name("mail"), Some(ForbidKind::Email)); + assert_eq!(ForbidKind::from_name("nope"), None); + // The wire token is the stable persisted/gossiped form. + assert_eq!(ForbidKind::Nick.wire(), "NICK"); + assert_eq!(ForbidKind::from_name(ForbidKind::Email.wire()), Some(ForbidKind::Email)); + } } #[cfg(test)] diff --git a/modules/chanserv/src/lib.rs b/modules/chanserv/src/lib.rs index ec0982f..be40efc 100644 --- a/modules/chanserv/src/lib.rs +++ b/modules/chanserv/src/lib.rs @@ -1,4 +1,4 @@ -use echo_api::{ChanError, ChannelView, Priv, Store}; +use echo_api::{ChanError, ChannelView, ForbidKind, Priv, Store}; use echo_api::{HelpEntry, Sender, Service, ServiceCtx}; use echo_api::NetView; @@ -125,7 +125,7 @@ impl Service for ChanServ { ctx.notice(me, from.uid, format!("You must be a channel operator (\x02@\x02) in \x02{chan}\x02 to register it.")); return; } - if let Some(reason) = db.is_forbidden("CHAN", chan) { + if let Some(reason) = db.is_forbidden(ForbidKind::Chan, chan) { ctx.notice(me, from.uid, format!("\x02{chan}\x02 can't be registered: {reason}")); return; } diff --git a/modules/nickserv/src/set.rs b/modules/nickserv/src/set.rs index e4fd689..ea645c3 100644 --- a/modules/nickserv/src/set.rs +++ b/modules/nickserv/src/set.rs @@ -1,4 +1,4 @@ -use echo_api::Store; +use echo_api::{ForbidKind, Store}; use echo_api::{Sender, ServiceCtx}; // SET PASSWORD | SET EMAIL [address]: change your account settings. @@ -30,7 +30,7 @@ pub fn handle(me: &str, from: &Sender, args: &[&str], ctx: &mut ServiceCtx, db: let email = args.get(2).map(|s| s.to_string()); let cleared = email.is_none(); if let Some(addr) = &email { - if db.is_forbidden("EMAIL", addr).is_some() { + if db.is_forbidden(ForbidKind::Email, addr).is_some() { ctx.notice(me, from.uid, "That email address is forbidden by network policy. Use a different one."); return; } diff --git a/modules/operserv/src/forbid.rs b/modules/operserv/src/forbid.rs index fa8efd2..7b9f985 100644 --- a/modules/operserv/src/forbid.rs +++ b/modules/operserv/src/forbid.rs @@ -1,4 +1,4 @@ -use echo_api::{human_time, Priv, Sender, ServiceCtx, Store}; +use echo_api::{human_time, ForbidKind, Priv, Sender, ServiceCtx, Store}; // FORBID ADD | DEL | LIST // Bans a nick, channel, or email pattern from being registered. Admin-only. @@ -9,7 +9,7 @@ pub fn handle(me: &str, from: &Sender, args: &[&str], ctx: &mut ServiceCtx, db: } match args.get(1).map(|s| s.to_ascii_uppercase()).as_deref() { Some("ADD") => { - let (Some(kind), Some(&mask)) = (args.get(2).and_then(|k| norm_kind(k)), args.get(3)) else { + let (Some(kind), Some(&mask)) = (args.get(2).and_then(|k| ForbidKind::from_name(k)), args.get(3)) else { ctx.notice(me, from.uid, "Syntax: FORBID ADD "); return; }; @@ -19,20 +19,22 @@ pub fn handle(me: &str, from: &Sender, args: &[&str], ctx: &mut ServiceCtx, db: } let reason = args[4..].join(" "); let setter = from.account.unwrap_or(from.nick); + let label = kind.wire().to_lowercase(); match db.forbid_add(kind, mask, setter, &reason) { - Ok(true) => ctx.notice(me, from.uid, format!("Forbade {} \x02{mask}\x02.", kind.to_lowercase())), - Ok(false) => ctx.notice(me, from.uid, format!("Updated the forbid on {} \x02{mask}\x02.", kind.to_lowercase())), + Ok(true) => ctx.notice(me, from.uid, format!("Forbade {label} \x02{mask}\x02.")), + Ok(false) => ctx.notice(me, from.uid, format!("Updated the forbid on {label} \x02{mask}\x02.")), Err(_) => ctx.notice(me, from.uid, "Sorry, that didn't work. Please try again in a moment."), } } Some("DEL") | Some("REMOVE") => { - let (Some(kind), Some(&mask)) = (args.get(2).and_then(|k| norm_kind(k)), args.get(3)) else { + let (Some(kind), Some(&mask)) = (args.get(2).and_then(|k| ForbidKind::from_name(k)), args.get(3)) else { ctx.notice(me, from.uid, "Syntax: FORBID DEL "); return; }; + let label = kind.wire().to_lowercase(); match db.forbid_del(kind, mask) { - Ok(true) => ctx.notice(me, from.uid, format!("Removed the forbid on {} \x02{mask}\x02.", kind.to_lowercase())), - Ok(false) => ctx.notice(me, from.uid, format!("No forbid on {} \x02{mask}\x02.", kind.to_lowercase())), + Ok(true) => ctx.notice(me, from.uid, format!("Removed the forbid on {label} \x02{mask}\x02.")), + Ok(false) => ctx.notice(me, from.uid, format!("No forbid on {label} \x02{mask}\x02.")), Err(_) => ctx.notice(me, from.uid, "Sorry, that didn't work. Please try again in a moment."), } } @@ -44,19 +46,9 @@ pub fn handle(me: &str, from: &Sender, args: &[&str], ctx: &mut ServiceCtx, db: } ctx.notice(me, from.uid, "Registration bans:"); for f in &forbids { - ctx.notice(me, from.uid, format!(" [{}] \x02{}\x02 by {} ({}) — {}", f.kind, f.mask, f.setter, human_time(f.ts), f.reason)); + ctx.notice(me, from.uid, format!(" [{}] \x02{}\x02 by {} ({}) — {}", f.kind.wire(), f.mask, f.setter, human_time(f.ts), f.reason)); } } _ => ctx.notice(me, from.uid, "Syntax: FORBID ADD | DEL | LIST"), } } - -// Canonicalise the kind argument to NICK / CHAN / EMAIL. -fn norm_kind(k: &str) -> Option<&'static str> { - match k.to_ascii_uppercase().as_str() { - "NICK" | "NICKNAME" | "ACCOUNT" => Some("NICK"), - "CHAN" | "CHANNEL" => Some("CHAN"), - "EMAIL" | "MAIL" => Some("EMAIL"), - _ => None, - } -} diff --git a/src/engine/db/mod.rs b/src/engine/db/mod.rs index 98c2a93..b39940a 100644 --- a/src/engine/db/mod.rs +++ b/src/engine/db/mod.rs @@ -40,7 +40,7 @@ pub(crate) use event::{apply, Scope}; // echo-api SDK crate; re-exported so the engine keeps naming them locally and // modules importing `crate::engine::db::{ChanError, ...}` are unaffected. pub use echo_api::{ - AccountView, AjoinView, AkillView, BotView, Caps, ForbidView, GroupView, HelpView, IgnoreView, MemoView, NewsView, Privs, ReportView, SuspensionView, ChanAccessView, ChanAkickView, ChanError, ChanSetting, ChannelView, CertError, CodeKind, Kicker, RegError, Store, TriggerView, VhostView, + AccountView, AjoinView, AkillView, BotView, Caps, ForbidKind, ForbidView, GroupView, HelpView, IgnoreView, MemoView, NewsView, Privs, ReportView, SuspensionView, ChanAccessView, ChanAkickView, ChanError, ChanSetting, ChannelView, CertError, CodeKind, Kicker, RegError, Store, TriggerView, VhostView, }; #[derive(Debug, Clone, Serialize, Deserialize)] diff --git a/src/engine/db/network.rs b/src/engine/db/network.rs index 3cbf172..1fbe755 100644 --- a/src/engine/db/network.rs +++ b/src/engine/db/network.rs @@ -37,41 +37,48 @@ impl Db { /// Add a registration ban of `kind` ("NICK"/"CHAN"/"EMAIL") for `mask`. /// Returns whether it was new. - pub fn forbid_add(&mut self, kind: &str, mask: &str, setter: &str, reason: &str) -> Result { - let same = |f: &Forbid| f.kind == kind && f.mask.eq_ignore_ascii_case(mask); + pub fn forbid_add(&mut self, kind: ForbidKind, mask: &str, setter: &str, reason: &str) -> Result { + // The store persists/gossips the wire token; the API is typed. + let wire = kind.wire(); + let same = |f: &Forbid| f.kind == wire && f.mask.eq_ignore_ascii_case(mask); let fresh = !self.net.forbids.iter().any(same); self.log - .append(Event::ForbidAdded { kind: kind.to_string(), mask: mask.to_string(), setter: setter.to_string(), reason: reason.to_string(), ts: now() }) + .append(Event::ForbidAdded { kind: wire.to_string(), mask: mask.to_string(), setter: setter.to_string(), reason: reason.to_string(), ts: now() }) .map_err(|_| RegError::Internal)?; self.net.forbids.retain(|f| !same(f)); - self.net.forbids.push(Forbid { kind: kind.to_string(), mask: mask.to_string(), setter: setter.to_string(), reason: reason.to_string(), ts: now() }); + self.net.forbids.push(Forbid { kind: wire.to_string(), mask: mask.to_string(), setter: setter.to_string(), reason: reason.to_string(), ts: now() }); Ok(fresh) } /// Remove a registration ban of `kind` for `mask`. Returns whether one existed. - pub fn forbid_del(&mut self, kind: &str, mask: &str) -> Result { - let same = |f: &Forbid| f.kind == kind && f.mask.eq_ignore_ascii_case(mask); + pub fn forbid_del(&mut self, kind: ForbidKind, mask: &str) -> Result { + let wire = kind.wire(); + let same = |f: &Forbid| f.kind == wire && f.mask.eq_ignore_ascii_case(mask); if !self.net.forbids.iter().any(same) { return Ok(false); } - self.log.append(Event::ForbidRemoved { kind: kind.to_string(), mask: mask.to_string() }).map_err(|_| RegError::Internal)?; + self.log.append(Event::ForbidRemoved { kind: wire.to_string(), mask: mask.to_string() }).map_err(|_| RegError::Internal)?; self.net.forbids.retain(|f| !same(f)); Ok(true) } - /// All registration bans, oldest first. + /// All registration bans, oldest first. A stored kind that no longer parses + /// (corrupt/old log) is skipped rather than shown. pub fn forbids(&self) -> Vec { self.net.forbids .iter() - .map(|f| ForbidView { kind: f.kind.clone(), mask: f.mask.clone(), setter: f.setter.clone(), reason: f.reason.clone(), ts: f.ts }) + .filter_map(|f| { + ForbidKind::from_name(&f.kind).map(|kind| ForbidView { kind, mask: f.mask.clone(), setter: f.setter.clone(), reason: f.reason.clone(), ts: f.ts }) + }) .collect() } /// The reason `name` is forbidden for `kind` registration (glob), if it is. - pub fn is_forbidden(&self, kind: &str, name: &str) -> Option { + pub fn is_forbidden(&self, kind: ForbidKind, name: &str) -> Option { + let wire = kind.wire(); self.net.forbids .iter() - .find(|f| f.kind == kind && glob_match(&f.mask, name)) + .find(|f| f.kind == wire && glob_match(&f.mask, name)) .map(|f| f.reason.clone()) } diff --git a/src/engine/db/store.rs b/src/engine/db/store.rs index c0526ed..b715b1f 100644 --- a/src/engine/db/store.rs +++ b/src/engine/db/store.rs @@ -218,16 +218,16 @@ impl Store for Db { fn akills(&self) -> Vec { Db::akills(self) } - fn forbid_add(&mut self, kind: &str, mask: &str, setter: &str, reason: &str) -> Result { + fn forbid_add(&mut self, kind: ForbidKind, mask: &str, setter: &str, reason: &str) -> Result { Db::forbid_add(self, kind, mask, setter, reason) } - fn forbid_del(&mut self, kind: &str, mask: &str) -> Result { + fn forbid_del(&mut self, kind: ForbidKind, mask: &str) -> Result { Db::forbid_del(self, kind, mask) } fn forbids(&self) -> Vec { Db::forbids(self) } - fn is_forbidden(&self, kind: &str, name: &str) -> Option { + fn is_forbidden(&self, kind: ForbidKind, name: &str) -> Option { Db::is_forbidden(self, kind, name) } fn ignore_add(&mut self, mask: &str, reason: &str, expires: Option) { diff --git a/src/engine/db/tests.rs b/src/engine/db/tests.rs index 5d0e3e2..9495e5b 100644 --- a/src/engine/db/tests.rs +++ b/src/engine/db/tests.rs @@ -610,7 +610,7 @@ db.group_register("!other", "bob").unwrap(); db.group_set_flags("!other", "alice", "").unwrap(); db.akill_add("G", "*@evil", "oper", "spam", None).unwrap(); - db.forbid_add("NICK", "bad*", "oper", "squat").unwrap(); + db.forbid_add(ForbidKind::Nick, "bad*", "oper", "squat").unwrap(); db.vhost_offer_add("cool.vhost").unwrap(); db.vhost_forbid_add("(?i)admin").unwrap(); db.set_vhost_template(Some("$account.users.example".into())).unwrap(); @@ -642,8 +642,8 @@ db.unsuspend_account("carol").unwrap(); db.akill_add("Q", "spam*", "oper", "nick", None).unwrap(); db.akill_del("Q", "spam*").unwrap(); - db.forbid_add("CHAN", "#evil*", "oper", "bad").unwrap(); - db.forbid_del("CHAN", "#evil*").unwrap(); + db.forbid_add(ForbidKind::Chan, "#evil*", "oper", "bad").unwrap(); + db.forbid_del(ForbidKind::Chan, "#evil*").unwrap(); let nid = db.news_add("logon", "temp notice", "oper"); db.news_del(nid); db.certfp_add("bob", "1122334455667788990011223344556677889900").unwrap(); @@ -704,7 +704,7 @@ a.set_account_kill("alice", false).unwrap(); a.suspend_account("bob", "op", "bad", None).unwrap(); a.akill_add("G", "*@evil", "op", "spam", None).unwrap(); - a.forbid_add("NICK", "bad*", "op", "squat").unwrap(); + a.forbid_add(ForbidKind::Nick, "bad*", "op", "squat").unwrap(); a.group_register("!g", "alice").unwrap(); a.group_set_flags("!g", "bob", "").unwrap(); diff --git a/src/engine/register.rs b/src/engine/register.rs index 0aa08b5..ed4cb96 100644 --- a/src/engine/register.rs +++ b/src/engine/register.rs @@ -214,7 +214,7 @@ impl Engine { if self.db.exists(account) { return Some(reg_reply(reply, RegOutcome::Exists, account)); } - if self.db.is_forbidden("NICK", account).is_some() { + if self.db.is_forbidden(echo_api::ForbidKind::Nick, account).is_some() { return Some(reg_reply(reply, RegOutcome::Forbidden, account)); } if !self.reg_limiter.allow() { @@ -230,7 +230,7 @@ impl Engine { }; // A forbidden email pattern (OperServ FORBID EMAIL) blocks registration. if let Some(addr) = &email { - if self.db.is_forbidden("EMAIL", addr).is_some() { + if self.db.is_forbidden(echo_api::ForbidKind::Email, addr).is_some() { return reg_reply(&reply, RegOutcome::ForbiddenEmail, account); } } diff --git a/src/engine/tests.rs b/src/engine/tests.rs index 7916f23..3ff574d 100644 --- a/src/engine/tests.rs +++ b/src/engine/tests.rs @@ -1740,7 +1740,7 @@ let reply = crate::proto::RegReply::NickServ { agent: "42SAAAAAA".into(), uid: "000AAAAAC".into(), nick: "evilbob".into() }; assert!(e.pre_register_check("evilbob", &reply).is_some(), "forbidden nick refused"); assert!(e.pre_register_check("cleanname", &reply).is_none(), "clean nick allowed"); - assert!(e.db.is_forbidden("CHAN", "#warez").is_some(), "channel is forbidden"); + assert!(e.db.is_forbidden(echo_api::ForbidKind::Chan, "#warez").is_some(), "channel is forbidden"); // EMAIL forbids block registration and SET EMAIL. assert!(notice(&os(&mut e, "FORBID ADD EMAIL *@spam.tld disposable"), "Forbade"), "email forbid added");