fix(forge): require a host in --public-url, bound the startup-settings test

crates/forge/Cargo.toml

@@ -30,6 +30,7 @@ rusqlite = { version = "0.40.2", features = ["bundled"] }
 serde_json = "1.0.154"
 thiserror = "2.0.18"
 tokio = { version = "1.54.1", features = ["rt-multi-thread", "net", "io-util", "signal", "time", "macros", "sync"] }
+url = "2.5.8"
 
 [lints]
 workspace = true

crates/forge/src/cli.rs

@@ -200,7 +200,8 @@ fn run_admin(cmd: AdminCmd) -> Result<()> {
 }
 
 fn run_serve(a: ServeArgs) -> Result<()> {
-    scl_forge::server::validate_public_url(&a.public_url).map_err(anyhow::Error::msg)?;
+    let public_url =
+        scl_forge::server::validate_public_url(&a.public_url).map_err(anyhow::Error::msg)?;
     if a.max_connections == 0 {
         bail!("--max-connections must be at least 1");
     }
@@ -225,7 +226,7 @@ fn run_serve(a: ServeArgs) -> Result<()> {
         cidrs,
     };
     db::open_for_serve(&a.data_dir)?;
-    let app = make_app(a.data_dir, &a.public_url, proxies, a.argon2_concurrency);
+    let app = make_app(a.data_dir, &public_url, proxies, a.argon2_concurrency);
     let limits = Limits {
         max_body: a.max_body_bytes,
         max_connections: a.max_connections,

crates/forge/src/server.rs

@@ -137,18 +137,55 @@ fn to_response(r: Resp) -> Response<Full<Bytes>> {
     })
 }
 
-/// Check `--public-url` at startup: an `http(s)://` URL of visible ASCII only, so
-/// it can be placed in `Link` headers and bodies without surprises.
-pub fn validate_public_url(url: &str) -> Result<(), String> {
-    let rest = url
+/// Parse and normalize `--public-url`: `http(s)://host[:port][/path]` with a real host,
+/// no user-info, no query or fragment. Returns the normalized base (percent-encoded,
+/// no trailing slash) that every URL in a body or `Link` header is built from, so a
+/// raw space or angle bracket in the path can never reach a header.
+pub fn validate_public_url(input: &str) -> Result<String, String> {
+    let bad = |why: &str| format!("--public-url {input:?} is not usable: {why}");
+    // WHATWG parsing forgives extra or backward slashes (`http:///x` becomes host `x`);
+    // an operator typing that made a mistake, so require the literal `scheme://host` shape.
+    let after = input
         .strip_prefix("http://")
-        .or_else(|| url.strip_prefix("https://"));
-    match rest {
-        Some(r) if !r.is_empty() && r.bytes().all(|b| b.is_ascii_graphic()) => Ok(()),
-        _ => Err(format!(
-            "--public-url {url:?} must be an http:// or https:// URL of visible ASCII characters (percent-encode anything else)"
-        )),
+        .or_else(|| input.strip_prefix("https://"));
+    if after.is_none_or(|r| r.starts_with(['/', '\\'])) {
+        return Err(bad(
+            "it must start with http:// or https:// followed by a host",
+        ));
     }
+    // `http://@host` parses as an empty user name; any `@` before the path is user-info.
+    let authority = after
+        .and_then(|r| r.split(['/', '?', '#']).next())
+        .unwrap_or("");
+    if authority.contains('@') {
+        return Err(bad("it must not contain user-info"));
+    }
+    if authority.ends_with(':') {
+        return Err(bad("the port after ':' is empty"));
+    }
+    let url = url::Url::parse(input).map_err(|e| bad(&e.to_string()))?;
+    if !matches!(url.scheme(), "http" | "https") {
+        return Err(bad("the scheme must be http or https"));
+    }
+    if !url.username().is_empty() || url.password().is_some() {
+        return Err(bad("it must not contain user-info"));
+    }
+    if url.query().is_some() || url.fragment().is_some() {
+        return Err(bad("it must not contain a query or fragment"));
+    }
+    match url.host() {
+        Some(url::Host::Domain(d)) => {
+            // WHATWG parsing tolerates empty labels and edge hyphens; a base URL should not.
+            if d.split('.')
+                .any(|l| l.is_empty() || l.starts_with('-') || l.ends_with('-'))
+            {
+                return Err(bad("the host has an empty or hyphen-edged label"));
+            }
+        }
+        Some(_) => {}
+        None => return Err(bad("it has no host")),
+    }
+    Ok(url.as_str().trim_end_matches('/').to_string())
 }
 
 /// Serve until `shutdown` resolves, then wait up to `drain` for in-flight
@@ -203,18 +240,58 @@ mod tests {
     use super::*;
 
     #[test]
-    fn public_url_must_be_header_safe() {
-        assert!(validate_public_url("http://127.0.0.1:3000").is_ok());
-        assert!(validate_public_url("https://git.example.com/forge").is_ok());
+    fn public_url_is_parsed_and_normalized() {
+        for ok in [
+            "http://127.0.0.1:3000",
+            "https://git.example.com/forge",
+            "http://[::1]:3000",
+            "http://[2001:db8::1]/forge",
+            "http://localhost",
+        ] {
+            assert!(validate_public_url(ok).is_ok(), "{ok}");
+        }
+        assert_eq!(
+            validate_public_url("https://git.example.com/forge/").unwrap(),
+            "https://git.example.com/forge"
+        );
+        assert_eq!(
+            validate_public_url("http://h/a b<c>").unwrap(),
+            "http://h/a%20b%3Cc%3E",
+            "raw characters are encoded, not passed through"
+        );
+        assert_eq!(validate_public_url("http://h").unwrap(), "http://h");
+        assert_eq!(
+            validate_public_url("http://exämple.com").unwrap(),
+            "http://xn--exmple-cua.com",
+            "unicode hosts become their ASCII form"
+        );
         for bad in [
             "",
             "ftp://x",
             "http://",
+            "http:///forge",
+            "http://:3000",
+            "http://@host",
+            "http://user@/forge",
+            "http://user:pw@example.com",
+            "http://[]/forge",
+            "http://[::1/forge",
+            "http://host:",
+            "http://host:abc",
+            "http://host:70000",
+            "http://host:65536",
+            "http://h?x",
+            "http://h/p#f",
+            "http://h#f",
+            "http://api..example.com",
+            "http://api.-example.com",
+            "http://-a.example.com",
+            "http://a-.example.com",
             "http://ex ample",
-            "http://exämple.com",
             "http://x\r\nSet-Cookie: a",
         ] {
             assert!(validate_public_url(bad).is_err(), "{bad:?}");
         }
+        assert!(validate_public_url("http://host:65535").is_ok());
     }
 }

crates/forge/tests/e2e_a.rs

@@ -1176,24 +1176,42 @@ fn a_database_with_the_right_version_but_no_tables_is_not_ready() {
 fn serve_refuses_unusable_settings_at_startup() {
     let dir = std::env::temp_dir().join(format!("scl-forge-serve-args-{}", std::process::id()));
     let bin = env!("CARGO_BIN_EXE_sc-forge");
+    // Bounded wait: if a bad setting were accepted the server would run until a signal,
+    // so kill and reap it instead of stalling the suite.
     let run = |extra: &[&str]| {
-        std::process::Command::new(bin)
+        let mut child = std::process::Command::new(bin)
             .args(["serve", "--listen", "127.0.0.1:0", "--data-dir"])
             .arg(&dir)
             .args(extra)
-            .output()
-            .unwrap()
+            .stderr(std::process::Stdio::piped())
+            .spawn()
+            .unwrap();
+        let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10);
+        let status = loop {
+            if let Some(st) = child.try_wait().unwrap() {
+                break Some(st);
+            }
+            if std::time::Instant::now() > deadline {
+                child.kill().unwrap();
+                let _ = child.wait();
+                break None;
+            }
+            std::thread::sleep(std::time::Duration::from_millis(20));
+        };
+        let mut err = String::new();
+        std::io::Read::read_to_string(child.stderr.as_mut().unwrap(), &mut err).unwrap();
+        (status.is_some_and(|s| !s.success()), err)
     };
-    let o = run(&[
+    let (refused, err) = run(&[
         "--public-url",
         "http://127.0.0.1:1",
         "--max-connections",
         "0",
     ]);
-    assert!(
-        !o.status.success() && String::from_utf8_lossy(&o.stderr).contains("--max-connections")
-    );
-    let o = run(&["--public-url", "http://exämple.com"]);
-    assert!(!o.status.success() && String::from_utf8_lossy(&o.stderr).contains("--public-url"));
+    assert!(refused && err.contains("--max-connections"), "{err}");
+    for bad in ["http://ex ample", "http:///forge"] {
+        let (refused, err) = run(&["--public-url", bad]);
+        assert!(refused && err.contains("--public-url"), "{bad}: {err}");
+    }
     let _ = std::fs::remove_dir_all(&dir);
 }