From 1fb3615b7e0df6b245be7ab8218bda883df1d4db Mon Sep 17 00:00:00 2001 From: Jean Date: Fri, 17 Jul 2026 12:45:41 +0000 Subject: [PATCH] auth: warn on unknown [[oper]] privilege names instead of silently dropping them --- api/src/lib.rs | 68 +++++++++++++++++++++++++++++++++++++---------- src/config.rs | 12 +++++++++ src/engine/mod.rs | 7 +++++ src/main.rs | 3 +++ 4 files changed, 76 insertions(+), 14 deletions(-) diff --git a/api/src/lib.rs b/api/src/lib.rs index f31518e..689b0ac 100644 --- a/api/src/lib.rs +++ b/api/src/lib.rs @@ -245,6 +245,31 @@ pub enum Priv { Admin, } +impl Priv { + /// Every privilege, for iteration and validation. + pub const ALL: [Priv; 3] = [Priv::Auspex, Priv::Suspend, Priv::Admin]; + + /// This privilege's canonical name — the `[[oper]]` config token. Exhaustive, + /// so adding a variant forces adding its name (no silent gap). + pub fn name(self) -> &'static str { + match self { + Priv::Auspex => "auspex", + Priv::Suspend => "suspend", + Priv::Admin => "admin", + } + } + + /// Parse a privilege name (case-insensitive); `None` if unrecognised. + pub fn from_name(s: &str) -> Option { + Self::ALL.into_iter().find(|p| s.eq_ignore_ascii_case(p.name())) + } + + /// The recognised names as a comma list, for "valid: …" error messages. + pub fn valid_names() -> String { + Self::ALL.iter().map(|p| p.name()).collect::>().join(", ") + } +} + // The set of privileges a sender holds. A plain Copy bitset — empty means "not // an oper". Built from the [[oper]] config and carried on every Sender. #[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] @@ -269,28 +294,30 @@ impl Privs { Privs(self.0 | other.0) } - // Build a set from privilege names ("auspex"/"suspend"/"admin"), ignoring any - // that aren't recognised. Shared by the config loader and runtime OPER grants. - pub fn from_names>(names: &[S]) -> Self { + // Build a set from privilege names, returning the parsed set AND any names that + // weren't recognised — so a caller (config load, REHASH) can surface a typo + // loudly instead of silently granting nothing. + pub fn parse_names>(names: &[S]) -> (Self, Vec) { let mut privs = Privs::default(); + let mut unknown = Vec::new(); for n in names { - match n.as_ref().to_ascii_lowercase().as_str() { - "auspex" => privs = privs.with(Priv::Auspex), - "suspend" => privs = privs.with(Priv::Suspend), - "admin" => privs = privs.with(Priv::Admin), - _ => {} + match Priv::from_name(n.as_ref()) { + Some(p) => privs = privs.with(p), + None => unknown.push(n.as_ref().to_string()), } } - privs + (privs, unknown) + } + + // Build a set from privilege names, ignoring any that aren't recognised. Use + // [`Privs::parse_names`] where a typo should be surfaced. + pub fn from_names>(names: &[S]) -> Self { + Self::parse_names(names).0 } // The privilege names held, for display. pub fn names(self) -> Vec<&'static str> { - [(Priv::Auspex, "auspex"), (Priv::Suspend, "suspend"), (Priv::Admin, "admin")] - .into_iter() - .filter(|(p, _)| self.has(*p)) - .map(|(_, n)| n) - .collect() + Priv::ALL.into_iter().filter(|p| self.has(*p)).map(Priv::name).collect() } } @@ -1482,6 +1509,19 @@ mod tests { assert!(p.any()); assert!(!Privs::default().any(), "empty set is not an oper"); } + + #[test] + fn parse_names_reports_typos_instead_of_dropping_them() { + // Known names are parsed (case-insensitive); unknown ones are collected so + // a config typo is surfaced, not silently ignored. + let (privs, unknown) = Privs::parse_names(&["Admin", "auspex", "admn", "root"]); + assert!(privs.has(Priv::Admin) && privs.has(Priv::Auspex)); + assert!(!privs.has(Priv::Suspend)); + assert_eq!(unknown, vec!["admn".to_string(), "root".to_string()]); + assert_eq!(Priv::from_name("SUSPEND"), Some(Priv::Suspend)); + assert_eq!(Priv::from_name("nope"), None); + assert_eq!(Priv::valid_names(), "auspex, suspend, admin"); + } } #[cfg(test)] diff --git a/src/config.rs b/src/config.rs index 5dacb5d..e1fe456 100644 --- a/src/config.rs +++ b/src/config.rs @@ -130,6 +130,18 @@ impl Config { } map } + + // (account, unrecognised-privilege-name) pairs across all [[oper]] blocks, so + // the caller can warn about a typo that would otherwise silently grant nothing. + pub fn oper_priv_warnings(&self) -> Vec<(String, String)> { + self.oper + .iter() + .flat_map(|o| { + let (_, unknown) = echo_api::Privs::parse_names(&o.privs); + unknown.into_iter().map(move |name| (o.account.clone(), name)) + }) + .collect() + } } // The service modules to bring up at burst. Each name maps to a compiled-in diff --git a/src/engine/mod.rs b/src/engine/mod.rs index 0fa0382..71a91ac 100644 --- a/src/engine/mod.rs +++ b/src/engine/mod.rs @@ -851,6 +851,13 @@ impl Engine { if self.service_oper_type.is_empty() { "(none)" } else { &self.service_oper_type }, ))); out.push(notice("Not reloadable (need a RESTART): server name/SID/protocol, uplink, service umodes, keycard.".to_string())); + // Flag any typo'd privilege names so they don't silently grant nothing. + for (account, name) in cfg.oper_priv_warnings() { + out.push(notice(format!( + "\x02Warning:\x02 oper \x02{account}\x02 has an unknown privilege \x02{name}\x02 (valid: {}) — ignored.", + echo_api::Priv::valid_names() + ))); + } out } diff --git a/src/main.rs b/src/main.rs index 0831809..64fcc19 100644 --- a/src/main.rs +++ b/src/main.rs @@ -187,6 +187,9 @@ async fn main() -> Result<()> { // Channel for services-initiated actions to reach the uplink (drained by the link loop). let (irc_tx, irc_rx) = tokio::sync::mpsc::unbounded_channel(); engine.lock().await.set_irc_out(irc_tx); + for (account, name) in cfg.oper_priv_warnings() { + tracing::warn!(%account, privilege = %name, valid = %echo_api::Priv::valid_names(), "unknown privilege in [[oper]] — ignored (typo?)"); + } engine.lock().await.set_opers(cfg.opers()); engine.lock().await.set_config_path(path.clone()); engine.lock().await.set_sid(cfg.server.sid.clone());