Fix audit findings: logout notice language after uid unmap, gRPC rehash reports load failure and emits the staff-log line, cap reflected CTCP PING token
All checks were successful
CI / check (push) Successful in 5m47s

This commit is contained in:
Jean Chevronnet 2026-07-20 18:26:49 +00:00
parent fadd4f37cd
commit a2bf5ed553
No known key found for this signature in database
5 changed files with 50 additions and 12 deletions

View file

@ -12,7 +12,17 @@ impl Engine {
let arg = parts.next().unwrap_or(""); let arg = parts.next().unwrap_or("");
let response = match cmd.as_str() { let response = match cmd.as_str() {
"VERSION" => format!("VERSION {}", crate::version::short()), "VERSION" => format!("VERSION {}", crate::version::short()),
"PING" => format!("PING {arg}"), "PING" => {
// Reflect the client's token, but cap it: an over-long token would
// push the client-facing line past 512 once the ircd expands our
// short service source into a full nick!user@host, stranding the
// closing \x01. A real PING token is a short timestamp.
let mut n = arg.len().min(200);
while !arg.is_char_boundary(n) {
n -= 1;
}
format!("PING {}", &arg[..n])
}
"TIME" => format!("TIME {}", echo_api::human_time(self.now_secs())), "TIME" => format!("TIME {}", echo_api::human_time(self.now_secs())),
"CLIENTINFO" => "CLIENTINFO ACTION CLIENTINFO PING TIME VERSION".to_string(), "CLIENTINFO" => "CLIENTINFO ACTION CLIENTINFO PING TIME VERSION".to_string(),
_ => return None, // ACTION and unknown CTCPs: stay silent _ => return None, // ACTION and unknown CTCPs: stay silent

View file

@ -770,7 +770,9 @@ impl Engine {
self.network.clear_account(uid); self.network.clear_account(uid);
self.emit_irc(NetAction::Metadata { target: uid.to_string(), key: "accountname".to_string(), value: String::new() }); self.emit_irc(NetAction::Metadata { target: uid.to_string(), key: "accountname".to_string(), value: String::new() });
if let Some(ns) = self.nick_service.clone() { if let Some(ns) = self.nick_service.clone() {
let lang = self.lang_for_uid(uid); // `clear_account(uid)` above already dropped the uid→account map, so
// resolve the language from the account name, not the (now-anonymous) uid.
let lang = self.lang_for_account(account);
let reason = echo_api::render(&lang, reason, &[]); let reason = echo_api::render(&lang, reason, &[]);
let text = echo_api::render(&lang, "Your account \x02{account}\x02 {reason}. You have been logged out.", &[("account", account.to_string()), ("reason", reason)]); let text = echo_api::render(&lang, "Your account \x02{account}\x02 {reason}. You have been logged out.", &[("account", account.to_string()), ("reason", reason)]);
self.emit_irc(NetAction::Notice { from: ns, to: uid.to_string(), text }); self.emit_irc(NetAction::Notice { from: ns, to: uid.to_string(), text });
@ -964,7 +966,10 @@ impl Engine {
// keeps the running configuration untouched (validate-or-keep, like an ircd // keeps the running configuration untouched (validate-or-keep, like an ircd
// rehash). Server identity (name/sid/protocol/uplink), service umodes and the // rehash). Server identity (name/sid/protocol/uplink), service umodes and the
// keycard endpoint are NOT reloadable — they need a restart. // keycard endpoint are NOT reloadable — they need a restart.
pub fn rehash(&mut self, requester: &str, agent: &str) -> Vec<NetAction> { // Reload config.toml and apply the reloadable settings live. Returns whether
// the new config actually loaded (a parse error keeps the running config) plus
// the notices to deliver to the requester and the staff log channel.
pub fn rehash(&mut self, requester: &str, agent: &str) -> (bool, Vec<NetAction>) {
let path = self.config_path.clone(); let path = self.config_path.clone();
// Who ran it, for the DebugServ log-channel line (account, else nick). // Who ran it, for the DebugServ log-channel line (account, else nick).
let oper = self let oper = self
@ -982,7 +987,7 @@ impl Engine {
out.push(line); out.push(line);
} }
out.push(notice(format!("REHASH failed: {e}. The running configuration is unchanged."))); out.push(notice(format!("REHASH failed: {e}. The running configuration is unchanged.")));
return out; return (false, out);
} }
}; };
let opers = cfg.opers(); let opers = cfg.opers();
@ -1022,7 +1027,21 @@ impl Engine {
echo_api::Priv::valid_names() echo_api::Priv::valid_names()
))); )));
} }
out (true, out)
}
// Rehash driven by the gRPC control CLI. There's no oper session to notice, so
// the per-requester replies are dropped, but the staff log-channel line is still
// delivered and the real load result is reported back to the caller.
pub fn control_rehash(&mut self) -> (bool, String) {
let (ok, actions) = self.rehash("gRPC control", "");
for action in actions {
if matches!(action, NetAction::Privmsg { .. }) {
self.emit_irc(action);
}
}
let msg = if ok { "configuration reloaded" } else { "reload failed; running configuration unchanged" };
(ok, msg.to_string())
} }
// A user arrived on (or changed to) a registered nick: if they aren't logged // A user arrived on (or changed to) a registered nick: if they aren't logged

View file

@ -819,13 +819,21 @@
e.set_debugserv_uid("42SAAAAAO"); e.set_debugserv_uid("42SAAAAAO");
e.set_log_channel(Some("#services".to_string())); e.set_log_channel(Some("#services".to_string()));
let out = e.rehash("000AAAAAB", "42SAAAAAH"); let (ok, out) = e.rehash("000AAAAAB", "42SAAAAAH");
assert!(ok, "a valid config reloads successfully");
let feed = out.iter().find_map(|a| match a { let feed = out.iter().find_map(|a| match a {
NetAction::Privmsg { from, to, text } if from == "42SAAAAAO" && to == "#services" => Some(text.clone()), NetAction::Privmsg { from, to, text } if from == "42SAAAAAO" && to == "#services" => Some(text.clone()),
_ => None, _ => None,
}).expect("DebugServ announces the reload in the log channel"); }).expect("DebugServ announces the reload in the log channel");
assert!(feed.contains("reloaded the configuration"), "{feed}"); assert!(feed.contains("reloaded the configuration"), "{feed}");
assert!(feed.contains("#svc"), "the reloaded services channel is applied and shown: {feed}"); assert!(feed.contains("#svc"), "the reloaded services channel is applied and shown: {feed}");
// A parse error must report failure (not a false "reloaded") so the control
// CLI shows ✗, and must keep the running config.
std::fs::write(&cfgpath, "this is not = valid toml [[[").unwrap();
let (ok, out) = e.rehash("000AAAAAB", "42SAAAAAH");
assert!(!ok, "a broken config reports failure");
assert!(out.iter().any(|a| matches!(a, NetAction::Notice { text, .. } if text.contains("REHASH failed"))), "{out:?}");
let _ = std::fs::remove_file(&cfgpath); let _ = std::fs::remove_file(&cfgpath);
} }

View file

@ -432,11 +432,12 @@ impl Admin for AdminService {
async fn rehash(&self, req: Request<AdminRequest>) -> Result<Response<AdminResponse>, Status> { async fn rehash(&self, req: Request<AdminRequest>) -> Result<Response<AdminResponse>, Status> {
authorize(&req, &self.token)?; authorize(&req, &self.token)?;
// Same reload the OperServ REHASH path runs; the returned staff-feed notice // Same reload the OperServ REHASH path runs. control_rehash applies the
// has no oper to reach over gRPC, so it's dropped (the reload is logged). // config, delivers the staff-log announcement, and reports whether the new
let _ = self.engine.lock().await.rehash("gRPC control", ""); // config actually loaded (a parse error keeps the running config).
tracing::info!("configuration reloaded via gRPC control"); let (ok, message) = self.engine.lock().await.control_rehash();
Ok(Response::new(AdminResponse { ok: true, message: "configuration reloaded".to_string() })) tracing::info!(ok, "configuration reload requested via gRPC control");
Ok(Response::new(AdminResponse { ok, message }))
} }
} }

View file

@ -186,7 +186,7 @@ pub async fn run(mut proto: Box<dyn Protocol>, engine: Arc<Mutex<Engine>>, addr:
// OperServ REHASH: re-read config.toml and apply the // OperServ REHASH: re-read config.toml and apply the
// reloadable settings live, reporting back to the oper. // reloadable settings live, reporting back to the oper.
NetAction::Rehash { requester, agent } => { NetAction::Rehash { requester, agent } => {
engine.lock().await.rehash(&requester, &agent) engine.lock().await.rehash(&requester, &agent).1
} }
action => vec![action], action => vec![action],
}; };