HostServ: normalise vhosts + enforce uniqueness

Canonicalise a requested vhost the way the ircd displays it (a disallowed
underscore becomes a hyphen) before storing and applying, so what we keep
matches what shows on the network. A vhost can no longer be assigned to two
accounts: prepare_vhost normalises, validates, and checks vhost_owner at
every assignment point (SET/REQUEST/ACTIVATE/TAKE/DEFAULT), so two inputs
that collapse to the same host are caught as one.

Approach recommended by siniStar (the ns_nethost normalisation logic).

api/src/lib.rs

@@ -601,6 +601,7 @@ pub trait Store {
     fn del_vhost(&mut self, account: &str) -> Result<bool, RegError>;
     fn vhost(&self, account: &str) -> Option<VhostView>;
     fn vhosts(&self) -> Vec<VhostView>;
+    fn vhost_owner(&self, host: &str) -> Option<String>;
     fn request_vhost(&mut self, account: &str, host: &str) -> Result<(), RegError>;
     fn vhost_request_wait(&self, account: &str) -> u64;
     fn take_vhost_request(&mut self, account: &str) -> Result<Option<String>, RegError>;

hostserv/src/approve.rs

@@ -26,6 +26,14 @@ pub fn handle(me: &str, from: &Sender, args: &[&str], ctx: &mut ServiceCtx, net:
         ctx.notice(me, from.uid, format!("Rejected \x02{account}\x02's vhost request."));
         return;
     }
+    // Re-check the requested host now (another account may have taken it since).
+    let host = match super::prepare_vhost(&host, account, db) {
+        Ok(h) => h,
+        Err(msg) => {
+            ctx.notice(me, from.uid, format!("Can't activate: {msg}"));
+            return;
+        }
+    };
     match db.set_vhost(account, &host, from.nick, None) {
         Ok(()) => {
             for uid in net.uids_logged_into(account) {

hostserv/src/default.rs

@@ -17,11 +17,14 @@ pub fn handle(me: &str, from: &Sender, ctx: &mut ServiceCtx, db: &mut dyn Store)
         ctx.notice(me, from.uid, "Your account name has no usable characters for a vhost.");
         return;
     }
-    let host = template.replace("$account", &label);
-    if !super::valid_vhost(&host) || db.vhost_is_forbidden(&host) {
+    let generated = template.replace("$account", &label);
+    let host = match super::prepare_vhost(&generated, account, db) {
+        Ok(h) if !db.vhost_is_forbidden(&h) => h,
+        _ => {
             ctx.notice(me, from.uid, "Sorry, a vhost couldn't be generated for your account.");
             return;
         }
+    };
     match db.set_vhost(account, &host, "template", None) {
         Ok(()) => {
             ctx.apply_vhost(from.uid, &host);

hostserv/src/lib.rs

@@ -86,6 +86,37 @@ fn require_oper(me: &str, from: &Sender, ctx: &mut ServiceCtx) -> bool {
     false
 }
 
+// Canonicalise a vhost the way the ircd will actually display it: the host part
+// only allows letters, digits, dots and hyphens, so a disallowed character it
+// would rewrite (an underscore becomes a hyphen) is rewritten here first. This
+// keeps what we store identical to what shows on the network, so two inputs
+// that would collapse to the same host are detected as one.
+// (Recommended by siniStar — the ns_nethost normalisation approach.)
+pub(crate) fn normalize_vhost(spec: &str) -> String {
+    let (ident, host) = match spec.split_once('@') {
+        Some((i, h)) => (Some(i), h),
+        None => (None, spec),
+    };
+    let norm_host: String = host.chars().map(|c| if c == '_' { '-' } else { c }).collect();
+    match ident {
+        Some(i) => format!("{i}@{norm_host}"),
+        None => norm_host,
+    }
+}
+
+// Normalise a requested vhost and check it's valid and not already another
+// account's. Returns the canonical spec to store, or a message to show.
+pub(crate) fn prepare_vhost(spec: &str, account: &str, db: &dyn Store) -> Result<String, String> {
+    let host = normalize_vhost(spec);
+    if !valid_vhost(&host) {
+        return Err(format!("\x02{host}\x02 isn't a valid host (letters, digits, hyphens and dots)."));
+    }
+    if db.vhost_owner(&host).is_some_and(|owner| !owner.eq_ignore_ascii_case(account)) {
+        return Err(format!("\x02{host}\x02 is already in use. Please choose another."));
+    }
+    Ok(host)
+}
+
 // Whether `spec` is a valid vhost: an optional `ident@` (letters, digits, a few
 // punctuation) followed by a hostname of dot-separated alphanumeric/hyphen labels.
 pub(crate) fn valid_vhost(spec: &str) -> bool {

hostserv/src/request.rs

@@ -10,11 +10,14 @@ pub fn handle(me: &str, from: &Sender, args: &[&str], ctx: &mut ServiceCtx, db:
         ctx.notice(me, from.uid, "Syntax: REQUEST <host>");
         return;
     };
-    if !super::valid_vhost(host) {
-        ctx.notice(me, from.uid, format!("\x02{host}\x02 isn't a valid host (letters, digits, hyphens and dots)."));
+    let host = match super::prepare_vhost(host, account, db) {
+        Ok(h) => h,
+        Err(msg) => {
+            ctx.notice(me, from.uid, msg);
             return;
         }
-    if db.vhost_is_forbidden(host) {
+    };
+    if db.vhost_is_forbidden(&host) {
         ctx.notice(me, from.uid, format!("\x02{host}\x02 isn't allowed here. Please choose another."));
         return;
     }
@@ -23,7 +26,7 @@ pub fn handle(me: &str, from: &Sender, args: &[&str], ctx: &mut ServiceCtx, db:
         ctx.notice(me, from.uid, format!("Please wait \x02{wait}\x02s before requesting another vhost."));
         return;
     }
-    match db.request_vhost(account, host) {
+    match db.request_vhost(account, &host) {
         Ok(()) => ctx.notice(me, from.uid, format!("Requested vhost \x02{host}\x02 — an operator will review it.")),
         Err(_) => ctx.notice(me, from.uid, "Sorry, that didn't work. Please try again in a moment."),
     }

hostserv/src/set.rs

@@ -11,19 +11,22 @@ pub fn handle(me: &str, from: &Sender, args: &[&str], ctx: &mut ServiceCtx, net:
         ctx.notice(me, from.uid, "Syntax: SET <account> <host> [duration]");
         return;
     };
-    if !super::valid_vhost(host) {
-        ctx.notice(me, from.uid, format!("\x02{host}\x02 isn't a valid host (letters, digits, hyphens and dots)."));
-        return;
-    }
     if db.account(account).is_none() {
         ctx.notice(me, from.uid, format!("\x02{account}\x02 isn't registered."));
         return;
     }
+    let host = match super::prepare_vhost(host, account, db) {
+        Ok(h) => h,
+        Err(msg) => {
+            ctx.notice(me, from.uid, msg);
+            return;
+        }
+    };
     let ttl = args.get(3).and_then(|s| parse_duration(s));
-    match db.set_vhost(account, host, from.nick, ttl) {
+    match db.set_vhost(account, &host, from.nick, ttl) {
         Ok(()) => {
             for uid in net.uids_logged_into(account) {
-                ctx.apply_vhost(&uid, host);
+                ctx.apply_vhost(&uid, &host);
             }
             let when = args.get(3).filter(|_| ttl.is_some()).map(|d| format!(" (expires in {d})")).unwrap_or_default();
             ctx.notice(me, from.uid, format!("Vhost \x02{host}\x02 assigned to \x02{account}\x02{when}."));

hostserv/src/take.rs

@@ -10,10 +10,17 @@ pub fn handle(me: &str, from: &Sender, args: &[&str], ctx: &mut ServiceCtx, db:
         ctx.notice(me, from.uid, "Syntax: TAKE <number> (see OFFERLIST)");
         return;
     };
-    let Some(host) = n.checked_sub(1).and_then(|i| db.vhost_offers().into_iter().nth(i)) else {
+    let Some(offer) = n.checked_sub(1).and_then(|i| db.vhost_offers().into_iter().nth(i)) else {
         ctx.notice(me, from.uid, format!("There's no offer #\x02{n}\x02. See \x02OFFERLIST\x02."));
         return;
     };
+    let host = match super::prepare_vhost(&offer, account, db) {
+        Ok(h) => h,
+        Err(msg) => {
+            ctx.notice(me, from.uid, msg);
+            return;
+        }
+    };
     match db.set_vhost(account, &host, "offer", None) {
         Ok(()) => {
             ctx.apply_vhost(from.uid, &host);

src/engine/db.rs

@@ -1356,6 +1356,15 @@ impl Db {
         self.host_cfg.template.as_deref()
     }
 
+    /// The account whose current (non-expired) vhost is `host`, if any — so a
+    /// vhost can't be assigned to two accounts and collide on the network.
+    pub fn vhost_owner(&self, host: &str) -> Option<String> {
+        self.accounts
+            .values()
+            .find(|a| a.vhost.as_ref().is_some_and(|v| v.host.eq_ignore_ascii_case(host) && v.expires.is_none_or(|e| e > now())))
+            .map(|a| a.name.clone())
+    }
+
     /// Every account that has a vhost, as (account, host, setter, expires).
     pub fn vhosts(&self) -> Vec<(String, String, String, Option<u64>)> {
         let mut out: Vec<(String, String, String, Option<u64>)> = self
@@ -2532,6 +2541,9 @@ impl Store for Db {
     fn vhosts(&self) -> Vec<VhostView> {
         Db::vhosts(self).into_iter().map(|(account, host, setter, expires)| VhostView { account, host, setter, expires }).collect()
     }
+    fn vhost_owner(&self, host: &str) -> Option<String> {
+        Db::vhost_owner(self, host)
+    }
     fn request_vhost(&mut self, account: &str, host: &str) -> Result<(), RegError> {
         Db::request_vhost(self, account, host)
     }

src/engine/mod.rs

@@ -2686,6 +2686,50 @@ mod tests {
         assert!(hs(&mut e, "000AAAAAB", "SET alice not a host").iter().any(|a| matches!(a, NetAction::Notice { text, .. } if text.contains("isn't a valid host"))), "host validated");
     }
 
+    // A vhost is normalised the way the ircd displays it (_ -> -) and can't be
+    // assigned to two accounts (uniqueness on the normalised form).
+    #[test]
+    fn hostserv_normalises_and_dedupes() {
+        use fedserv_hostserv::HostServ;
+        use fedserv_nickserv::NickServ;
+        let path = std::env::temp_dir().join("fedserv-hsnorm.jsonl");
+        let _ = std::fs::remove_file(&path);
+        let mut db = Db::open(&path, "42S");
+        db.scram_iterations = 4096;
+        db.register("boss", "password1", None).unwrap();
+        db.register("alice", "password1", None).unwrap();
+        db.register("bob", "password1", None).unwrap();
+        let mut e = Engine::new(
+            vec![
+                Box::new(NickServ { uid: "42SAAAAAA".into(), guest_nick: "Guest".into(), guest_seq: 0 }),
+                Box::new(HostServ { uid: "42SAAAAAG".into() }),
+            ],
+            db,
+        );
+        e.set_sid("42S".into());
+        let mut opers = std::collections::HashMap::new();
+        opers.insert("boss".to_string(), Privs::default().with(fedserv_api::Priv::Admin));
+        e.set_opers(opers);
+        let ns = |e: &mut Engine, uid: &str, t: &str| e.handle(NetEvent::Privmsg { from: uid.into(), to: "42SAAAAAA".into(), text: t.into() });
+        let hs = |e: &mut Engine, uid: &str, t: &str| e.handle(NetEvent::Privmsg { from: uid.into(), to: "42SAAAAAG".into(), text: t.into() });
+        e.handle(NetEvent::UserConnect { uid: "000AAAAAB".into(), nick: "boss".into(), host: "realhost".into() });
+        e.handle(NetEvent::UserConnect { uid: "000AAAAAV".into(), nick: "alice".into(), host: "realhost".into() });
+        e.handle(NetEvent::UserConnect { uid: "000AAAAAW".into(), nick: "bob".into(), host: "realhost".into() });
+        ns(&mut e, "000AAAAAB", "IDENTIFY password1");
+        ns(&mut e, "000AAAAAV", "IDENTIFY password1");
+        ns(&mut e, "000AAAAAW", "IDENTIFY password1");
+        let says = |out: &[NetAction], n: &str| out.iter().any(|a| matches!(a, NetAction::Notice { text, .. } if text.contains(n)));
+
+        // The underscore is normalised to a hyphen, so what's applied matches the
+        // ircd's display.
+        let out = hs(&mut e, "000AAAAAB", "SET alice cool_user.example");
+        assert!(out.iter().any(|a| matches!(a, NetAction::SetHost { host, .. } if host == "cool-user.example")), "normalised: {out:?}");
+        // bob can't take the same host (even spelled with the underscore).
+        assert!(says(&hs(&mut e, "000AAAAAB", "SET bob cool_user.example"), "already in use"), "uniqueness enforced");
+        // A distinct host is fine.
+        assert!(hs(&mut e, "000AAAAAB", "SET bob other.example").iter().any(|a| matches!(a, NetAction::SetHost { host, .. } if host == "other.example")), "distinct host ok");
+    }
+
     // A temporary vhost applies while valid; an already-expired one is ignored.
     #[test]
     fn hostserv_vhost_expiry() {