diff --git a/install.sh b/install.sh
index aa8c9f07..8cfe72f3 100755
--- a/install.sh
+++ b/install.sh
@@ -92,7 +92,7 @@ fi
echo "usage: install.sh --server URL (--token TOKEN | --register KEY) [--interval SECONDS] [--iface LIST] [--insecure]" >&2
echo " install.sh --upgrade [--iface LIST] [--interval SECONDS]" >&2
echo " install.sh --uninstall" >&2
- echo "--name NAME names the node --register creates; the hostname otherwise" >&2
+ echo "--name NAME names the node --register creates; the hostname otherwise. < > \" ' are dropped from it; the panel renames a node freely" >&2
exit 2
}
# Names the node a registration creates. A token belongs to a node that already
diff --git a/src/api.rs b/src/api.rs
index 0988a4bf..70796e4a 100644
--- a/src/api.rs
+++ b/src/api.rs
@@ -1053,10 +1053,10 @@ pub async fn agent_register(
Ok(_) => {}
}
- // The name comes from a machine not yet vouched for: control characters would
- // break the panel's rows, and the length must be bounded. `chars()` rather
- // than bytes, so the cut falls on a character boundary.
- let name: String = name.trim().chars().filter(|c| !c.is_control()).take(64).collect();
+ // The name comes from a machine not yet vouched for, and is shown to every
+ // visitor until the operator renames the node. Trimmed last: a name made of
+ // markup characters and spaces must come out empty, not as a blank.
+ let name = db::reported_text(&name, 64).trim().to_owned();
let name = if name.is_empty() { "unnamed".to_owned() } else { name };
// Field defaults live in `Node`'s serde attributes and nowhere else.
// `Node::default()` is a different set of values -- private, reset day 0 --
@@ -3513,6 +3513,32 @@ mod tests {
assert_eq!(app.db.nodes().unwrap().len(), 1);
}
+ /// The name comes from an unvouched machine: markup characters are dropped,
+ /// and what that leaves is trimmed, so a name made only of them is `unnamed`.
+ #[tokio::test]
+ async fn a_registered_name_loses_markup_and_is_never_blank() {
+ let app = std::sync::Arc::new(app());
+ open_register(Admin, State(app.clone()), panel_headers()).await;
+ let key = app.db.get("register_key").unwrap();
+ for (sent, stored) in [("< >", "unnamed"), ("web", "bweb/b"), ("Tom's \"VPS\"", "Toms VPS")] {
+ let mut headers = HeaderMap::new();
+ headers.insert("authorization", format!("Bearer {key}").parse().unwrap());
+ let reply = agent_register(
+ State(app.clone()),
+ ConnectInfo("198.51.100.7:40000".parse().unwrap()),
+ headers,
+ sent.to_owned(),
+ )
+ .await;
+ let token = String::from_utf8(
+ axum::body::to_bytes(reply.into_body(), usize::MAX).await.unwrap().to_vec(),
+ )
+ .unwrap();
+ let id = app.db.node_by_token(&token).unwrap().expect("token opens a node");
+ assert_eq!(app.db.node(id).unwrap().unwrap().name, stored, "sent {sent:?}");
+ }
+ }
+
/// The ceiling on the anonymous route: a leaked key cannot fill the table.
#[tokio::test]
async fn one_window_stops_registering_at_the_limit() {
diff --git a/src/db.rs b/src/db.rs
index cfb33c96..18f855da 100644
--- a/src/db.rs
+++ b/src/db.rs
@@ -12,6 +12,17 @@ use serde::{Deserialize, Serialize};
use tokio::runtime::RuntimeFlavor;
use tracing::info;
+/// A string an agent reported, cut down to what may be shown. Control
+/// characters break the panel's rows, and the characters markup is made of are
+/// dropped because the value reaches a theme from a machine nobody has vouched
+/// for: a theme that sets it as HTML would otherwise run a node's script in the
+/// hub's origin, beside the administrator's session. No real OS name, kernel,
+/// CPU model or host name contains any of them. `chars()` rather than bytes, so
+/// the cut falls on a character boundary.
+pub fn reported_text(raw: &str, max: usize) -> String {
+ raw.chars().filter(|c| !c.is_control() && !matches!(c, '<' | '>' | '"' | '\'')).take(max).collect()
+}
+
pub struct Db {
/// A read-only connection to the same file, for the history charts. A week
/// of one node's probe results is a scan of 98 ms at four 60-second probes
@@ -1255,23 +1266,12 @@ impl Db {
/// moves to `country_prev_ip` / `country_prev` if it had an answer, and a
/// source equal to that address takes its answer back without a lookup.
pub fn save_facts(&self, id: i64, f: &serde_json::Value, ip: &str, source: &str) -> Result {
- // The same rule `api::agent_register` applies to the name it receives:
- // these values come from an unvouched machine, control characters break
- // the panel's rows, and the length must be bounded. Six of them -- os,
- // kernel, arch, virt, cpu_name, agent_version -- go straight into the
- // anonymous public frame, which is rebuilt and pushed to every viewer
- // every two seconds, so without a ceiling one node would determine that
- // frame's size. 128 rather than 64: a real PRETTY_NAME runs to about 60
- // characters and a CPU model to about 50.
- let s = |k: &str| {
- f.get(k)
- .and_then(|v| v.as_str())
- .unwrap_or("")
- .chars()
- .filter(|c| !c.is_control())
- .take(128)
- .collect::()
- };
+ // Six of these -- os, kernel, arch, virt, cpu_name, agent_version -- go
+ // straight into the anonymous public frame, which is rebuilt and pushed to
+ // every viewer every two seconds, so without a ceiling one node would
+ // determine that frame's size. 128 rather than 64: a real PRETTY_NAME runs
+ // to about 60 characters and a CPU model to about 50.
+ let s = |k: &str| reported_text(f.get(k).and_then(|v| v.as_str()).unwrap_or(""), 128);
let n = |k: &str| f.get(k).and_then(|v| v.as_i64()).unwrap_or(0);
let conn = self.conn();
conn.execute(
@@ -3476,6 +3476,14 @@ mod tests {
let stored = db.node(id).unwrap().unwrap();
assert_eq!(stored.os.chars().count(), 128);
assert_eq!(stored.hostname, "xy", "control characters break the panel's rows");
+
+ db.save_facts(id, &serde_json::json!({"os": "
'Debian'"}), "ip", "")
+ .unwrap();
+ assert_eq!(
+ db.node(id).unwrap().unwrap().os,
+ "img src=x onerror=a()Debian",
+ "no markup reaches a theme"
+ );
}
/// A correction must survive the node's return. `all_traffic` gates the month