diff --git a/.env.example b/.env.example index c453d647b..a4fe15c6e 100644 --- a/.env.example +++ b/.env.example @@ -39,6 +39,12 @@ GITLAWB_KEY=/data/keys/identity.pem # Publicly reachable URL of this node (used in peer announcements) GITLAWB_PUBLIC_URL=https://your-node.example.com +# Base URL for the web view of repos on this node (used by `gl` to print a +# working View: link after repo creation). Omit for nodes with no web front-end. +# Distinct from GITLAWB_PUBLIC_URL: that is API reachability for peers; +# this is browser reachability for humans. +# GITLAWB_WEB_URL=https://gitlawb.com + # ── Server ──────────────────────────────────────────────────────────────── GITLAWB_HOST=0.0.0.0 GITLAWB_PORT=7545 diff --git a/Cargo.lock b/Cargo.lock index 1fd6b7e6b..275c3c3a0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2208,7 +2208,7 @@ version = "3.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "faf9468729b8cbcea668e36183cb69d317348c2e08e994829fb56ebfdfbaac34" dependencies = [ - "windows-sys 0.61.2", + "windows-sys 0.48.0", ] [[package]] @@ -2701,7 +2701,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7ab67060fc6b8ef687992d439ca0fa36e7ed17e9a0b16b25b601e8757df720de" dependencies = [ "data-encoding", - "syn 2.0.117", + "syn 1.0.109", ] [[package]] @@ -3085,7 +3085,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys 0.61.2", + "windows-sys 0.52.0", ] [[package]] @@ -3563,6 +3563,7 @@ dependencies = [ "libp2p-swarm", "mockito", "multiaddr", + "percent-encoding", "prometheus", "reqwest", "serde", @@ -3580,6 +3581,7 @@ dependencies = [ "tracing", "tracing-subscriber", "unicode-normalization", + "url", "uuid", "zeroize", "zstd", @@ -3599,6 +3601,7 @@ dependencies = [ "gitlawb-core", "icaptcha-client", "mockito", + "percent-encoding", "reqwest", "serde", "serde_json", @@ -3607,6 +3610,7 @@ dependencies = [ "tokio", "tracing", "tracing-subscriber", + "url", "urlencoding", "uuid", ] @@ -3642,9 +3646,9 @@ dependencies = [ [[package]] name = "h2" -version = "0.4.18" +version = "0.4.19" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "839c0e8a181239723652be9062bb56ca5bf5f64011f73b623f6f4fc59086a228" +checksum = "ef8e5e5a340588f4452631496976cf8636d4a7ecf600239fdc27615d2530bc16" dependencies = [ "atomic-waker", "bytes", @@ -6223,7 +6227,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys", - "windows-sys 0.61.2", + "windows-sys 0.52.0", ] [[package]] @@ -7157,7 +7161,7 @@ dependencies = [ "getrandom 0.4.2", "once_cell", "rustix", - "windows-sys 0.61.2", + "windows-sys 0.52.0", ] [[package]] @@ -7949,7 +7953,7 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" dependencies = [ - "windows-sys 0.61.2", + "windows-sys 0.48.0", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index c6c3dca32..9d933cf13 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -54,8 +54,10 @@ uuid = { version = "1", features = ["v4"] } # http client reqwest = { version = "0.12", features = ["blocking", "json", "multipart", "rustls-tls"], default-features = false } # URL parsing (what reqwest::Url re-exports, so the shared redirect predicate can -# take a parsed URL without pulling reqwest into gitlawb-core) +# take a parsed URL without pulling reqwest into gitlawb-core; also used for the +# absolute-browser-url validation on GITLAWB_WEB_URL in the node and CLI) url = "2" +percent-encoding = "2" # HMAC hmac = "0.12" diff --git a/crates/gitlawb-node/Cargo.toml b/crates/gitlawb-node/Cargo.toml index 4fc9eb03a..36efc035f 100644 --- a/crates/gitlawb-node/Cargo.toml +++ b/crates/gitlawb-node/Cargo.toml @@ -23,6 +23,8 @@ tracing = { workspace = true } tracing-subscriber = { workspace = true } chrono = { workspace = true } uuid = { workspace = true } +url = { workspace = true } +percent-encoding = { workspace = true } axum = { version = "0.8", features = ["http1", "http2", "json", "ws"] } async-graphql = { version = "7", features = ["chrono", "uuid", "tracing"] } async-graphql-axum = "7" diff --git a/crates/gitlawb-node/src/config.rs b/crates/gitlawb-node/src/config.rs index 35f6ff764..2c5291371 100644 --- a/crates/gitlawb-node/src/config.rs +++ b/crates/gitlawb-node/src/config.rs @@ -66,6 +66,12 @@ pub struct Config { #[arg(long, env = "GITLAWB_PUBLIC_URL")] pub public_url: Option, + /// Base URL for the web view of repos on this node (e.g. https://gitlawb.com). + /// When set, `GET /` advertises it as `web_url` so the CLI can print a + /// working `View:` link. Omit for nodes that have no web front-end. + #[arg(long, env = "GITLAWB_WEB_URL")] + pub web_url: Option, + /// Comma-separated list of bootstrap peer URLs to announce to on startup #[arg(long, env = "GITLAWB_BOOTSTRAP_PEERS", value_delimiter = ',')] pub bootstrap_peers: Vec, @@ -803,7 +809,13 @@ impl Config { /// Cross-field boot validation. Single-field ranges are enforced by clap; this /// catches combinations that ship a denial-of-service under otherwise-valid /// values. Call once at startup and fail fast on `Err`. - pub fn validate(&self) -> Result<(), String> { + /// + /// `&mut self` is required so the validator can normalize `web_url` to + /// its canonical form after validation: the configured value is + /// advertised on public `GET /`, and any consumer reading + /// `config.web_url` later must see the same string that was actually + /// validated. + pub fn validate(&mut self) -> Result<(), String> { // Concurrent git writes pin the dedicated lock pool for their whole // duration (connection-affine advisory lock in acquire_write). The main // pool no longer carries that occupancy. @@ -920,10 +932,101 @@ impl Config { .map_err(|e| e.to_string())?; } + // GITLAWB_WEB_URL is advertised in GET / so the CLI can print a working + // View: link. Clap maps "" to Some("") which would produce a broken URL; + // reject early instead of serving a malformed link. Non-blank values must + // additionally parse as an absolute http(s) URL — the CLI appends + // `/{owner}/{repo}` to it, so scheme-less hosts ("gitlawb.com") and other + // garbage would render links no browser can follow. + if let Some(raw) = &self.web_url { + match validate_web_url(raw) { + Ok(()) => {} + Err(reason) => { + return Err(format!( + "GITLAWB_WEB_URL {reason} — \ + set it to an absolute URL like https://gitlawb.com or leave it unset." + )); + } + } + // Normalize to one canonical form so every consumer (the regular + // and degraded `GET /` handlers, anyone who reads + // `config.web_url` directly) sees the same string that was + // actually validated. The CLI's own defensive trim would paper + // over the divergence on the `View:` line, but that hides the + // bug from any other consumer that parses the value verbatim. + let canonical = raw + .trim() + .parse::() + .map(|u| u.to_string()) + .map_err(|_| "is not a valid absolute URL".to_string())?; + if canonical != *raw { + self.web_url = Some(canonical); + } + } + Ok(()) } } +/// Validate a `web_url` value for use as a browser-reachable base URL. +/// Blank values are rejected (clap maps `--web-url ""` to `Some("")`), and +/// non-blank values must parse as absolute `http`/`https` URLs with no +/// query, fragment, or userinfo — the CLI treats this as a string prefix +/// to append paths to, so anything else produces links no browser can +/// follow (`?a=1/owner/repo` puts the repo path inside the query string); +/// and userinfo is rejected outright because the configured value is +/// published on public `GET /` and reprinted on the CLI's `View:` line, +/// so an accidentally configured password would land in public metadata, +/// terminal scrollback, and CI logs. +pub(crate) fn validate_web_url(raw: &str) -> Result<(), String> { + if raw.trim().is_empty() { + return Err("must not be empty or whitespace-only".into()); + } + let parsed: url::Url = raw + .trim() + .parse() + .map_err(|_| "is not a valid absolute URL".to_string())?; + match parsed.scheme() { + "http" | "https" => {} + other => return Err(format!("must use http or https, got '{other}:'")), + } + // url::Url cannot represent a URL without a host for http/https schemes, so + // reaching here guarantees an absolute browser-usable base. + if parsed.query().is_some() { + return Err( + "must not contain a query string — it is used as a path prefix for View links".into(), + ); + } + if parsed.fragment().is_some() { + return Err( + "must not contain a fragment — it is used as a path prefix for View links".into(), + ); + } + if !parsed.username().is_empty() || parsed.password().is_some() { + return Err( + "must not contain a username or password — it is advertised publicly and printed on the View: line" + .into(), + ); + } + // Bidi/control bytes can hide behind percent-encoding: a literal scan of + // the wire form misses `.../%E2%80%AE` even though a browser decodes it + // into a reordering character on the rendered View: link. Check the + // decoded host and path before the value is advertised on GET / (INV-6). + let host = parsed.host_str().unwrap_or(""); + let decoded_path = percent_encoding::percent_decode_str(parsed.path()).decode_utf8_lossy(); + if host + .chars() + .chain(decoded_path.chars()) + .any(|c| c.is_control() || gitlawb_core::sanitize::is_bidi_format(c)) + { + return Err( + "must not contain control or bidi characters, even percent-encoded — the View: line is terminal output" + .into(), + ); + } + Ok(()) +} + #[cfg(test)] mod tests { use super::*; @@ -1566,7 +1669,7 @@ mod tests { .expect("default config must validate"); // An under-sized lock pool relative to the push cap is rejected. - let under_lock = Config::parse_from([ + let mut under_lock = Config::parse_from([ "gitlawb-node", "--db-lock-pool-max-connections", "16", @@ -1579,7 +1682,7 @@ mod tests { ); // Exactly at the push cap without non-push headroom is rejected. - let push_only = Config::parse_from([ + let mut push_only = Config::parse_from([ "gitlawb-node", "--db-lock-pool-max-connections", "32", @@ -1592,7 +1695,7 @@ mod tests { ); // Main pool can be smaller than pushes + headroom when the lock pool carries writes. - let split = Config::parse_from([ + let mut split = Config::parse_from([ "gitlawb-node", "--db-max-connections", "16", @@ -1607,13 +1710,153 @@ mod tests { ); // Main pool still needs the app headroom floor. - let under_main = Config::parse_from(["gitlawb-node", "--db-max-connections", "4"]); + let mut under_main = Config::parse_from(["gitlawb-node", "--db-max-connections", "4"]); assert!( under_main.validate().is_err(), "db_max_connections below DB_POOL_APP_HEADROOM must be rejected" ); } + #[test] + fn web_url_rejects_empty_and_whitespace() { + // Unset is fine. + Config::parse_from(["gitlawb-node"]) + .validate() + .expect("no web_url must validate"); + + // A real URL validates. + let mut cfg = Config::parse_from(["gitlawb-node"]); + cfg.web_url = Some("https://gitlawb.com".into()); + assert!(cfg.validate().is_ok()); + + // Empty string is rejected. + let mut cfg = Config::parse_from(["gitlawb-node"]); + cfg.web_url = Some("".into()); + assert!(cfg.validate().is_err(), "empty web_url must be rejected"); + + // Whitespace-only is rejected. + let mut cfg = Config::parse_from(["gitlawb-node"]); + cfg.web_url = Some(" ".into()); + assert!( + cfg.validate().is_err(), + "whitespace-only web_url must be rejected" + ); + } + + #[test] + fn web_url_rejects_non_absolute_or_non_browser_urls() { + // Scheme-less host: parses nowhere, renders a broken View: link. + for bad in [ + "gitlawb.com", + "not a url", + "ftp://files.example.com", + "javascript:alert(1)", + "https://", // no host + ] { + let mut cfg = Config::parse_from(["gitlawb-node"]); + cfg.web_url = Some(bad.into()); + assert!(cfg.validate().is_err(), "web_url {bad:?} must be rejected"); + } + + // Query strings and fragments corrupt the appended repo path, since + // web_url is used as a raw string prefix (`?a=1` would swallow + // `/{owner}/{repo}` into the query). + for bad in [ + "https://gitlawb.com?a=1", + "https://gitlawb.com/?utm_source=docs", + "https://gitlawb.com#section", + "https://gitlawb.com/#faq", + ] { + let mut cfg = Config::parse_from(["gitlawb-node"]); + cfg.web_url = Some(bad.into()); + assert!( + cfg.validate().is_err(), + "web_url {bad:?} (query or fragment) must be rejected" + ); + } + + // Surrounding whitespace around a valid URL is tolerated. + let mut cfg = Config::parse_from(["gitlawb-node"]); + cfg.web_url = Some(" https://gitlawb.com ".into()); + assert!( + cfg.validate().is_ok(), + "padded-but-valid web_url must validate" + ); + + // Percent-encoded bidi must not boot: the wire form carries no Cf + // code point, but a browser decodes %E2%80%AE into a reordering + // character on the rendered View: link (INV-6). Literal bidi and + // control bytes are rejected the same way. + for bad in [ + "https://example.com/%E2%80%AE", + "https://example.com/\u{202e}spoof", + "https://example.com/%1b[31m", + ] { + let mut cfg = Config::parse_from(["gitlawb-node"]); + cfg.web_url = Some(bad.into()); + assert!( + cfg.validate().is_err(), + "web_url {bad:?} (control or bidi, even encoded) must be rejected" + ); + } + + // Non-default ports and paths are fine. + let mut cfg = Config::parse_from(["gitlawb-node"]); + cfg.web_url = Some("http://localhost:8080/ui".into()); + assert!(cfg.validate().is_ok(), "port+path web_url must validate"); + + // Userinfo must be rejected: `web_url` is published on public + // `GET /` and reprinted on the CLI's `View:` line, so an + // accidentally configured password would land in public metadata, + // terminal scrollback, and CI logs. Reject username-only and + // user+password forms. + for bad in [ + "https://user@example.com", + "https://user:secret@example.com", + "https://:secret@example.com", + ] { + let mut cfg = Config::parse_from(["gitlawb-node"]); + cfg.web_url = Some(bad.into()); + assert!( + cfg.validate().is_err(), + "web_url {bad:?} (userinfo) must be rejected" + ); + } + } + + /// `validate()` accepts whitespace-padded input but the stored value + /// must match the canonical form actually validated, so every consumer + /// (the regular and degraded `GET /` handlers, anyone who reads + /// `config.web_url` directly) sees the same string. Without this, an + /// operator that sets `GITLAWB_WEB_URL=" https://git.example "` boots + /// a node that advertises `" https://git.example "` on public + /// metadata — different from what was validated, different from what + /// any other consumer would parse. + #[test] + fn web_url_is_normalized_to_canonical_form_after_validation() { + // Padded input is rewritten to the canonical form. `url::Url`'s + // own `to_string` is what we use for the canonicalization, and + // it always appends a path-separator slash to a bare-host URL, + // so the stored value is `https://git.example/` (which is also + // what every browser would normalize the operator-typed form to). + let mut cfg = Config::parse_from(["gitlawb-node"]); + cfg.web_url = Some(" https://git.example ".into()); + cfg.validate().unwrap(); + assert_eq!( + cfg.web_url.as_deref(), + Some("https://git.example/"), + "stored web_url must be the canonical form, not the operator-typed one" + ); + + // Already-canonical input is left alone after a no-op + // re-normalization — same form comes back out, so any consumer + // reading the stored value sees a stable string across calls. + let mut cfg = Config::parse_from(["gitlawb-node"]); + cfg.web_url = Some("https://git.example/".into()); + cfg.validate().unwrap(); + assert_eq!(cfg.web_url.as_deref(), Some("https://git.example/")); + } + /// The DECLARED default, read off the parser rather than out of a parse. /// /// `Config::parse_from` consults the process environment, so on a host that @@ -1878,7 +2121,7 @@ mod tests { std::os::unix::fs::symlink(&target, &link).unwrap(); let key_path = link.join("p2p.key"); - let config = Config::parse_from([ + let mut config = Config::parse_from([ "gitlawb-node", "--p2p-port", "0", @@ -1973,7 +2216,7 @@ mod tests { let key_path = build(base.path()); let before = snapshot(base.path()); - let config = Config::parse_from([ + let mut config = Config::parse_from([ "gitlawb-node", "--p2p-port", "7546", diff --git a/crates/gitlawb-node/src/main.rs b/crates/gitlawb-node/src/main.rs index e41f52594..3914e844e 100644 --- a/crates/gitlawb-node/src/main.rs +++ b/crates/gitlawb-node/src/main.rs @@ -51,6 +51,7 @@ use state::AppState; struct DegradedState { node_did: String, db_startup: Arc, + web_url: Option, } /// Two independent counters with no cross-field invariant — atomics, not a @@ -159,6 +160,7 @@ async fn main() -> Result<()> { degraded_listener, node_did.to_string(), Arc::clone(&db_startup), + config.web_url.clone(), db_ready_rx, shutdown_tx.subscribe(), )); @@ -872,11 +874,12 @@ async fn run_degraded_server( listener: TcpListener, node_did: String, db_startup: Arc, + web_url: Option, mut db_ready_rx: watch::Receiver, mut shutdown_rx: watch::Receiver, ) -> Result<()> { let addr = listener.local_addr().ok(); - let router = build_degraded_router(node_did, db_startup); + let router = build_degraded_router(node_did, db_startup, web_url); info!(?addr, "degraded HTTP server ready"); axum::serve(listener, router) @@ -893,10 +896,15 @@ async fn run_degraded_server( Ok(()) } -fn build_degraded_router(node_did: String, db_startup: Arc) -> Router { +fn build_degraded_router( + node_did: String, + db_startup: Arc, + web_url: Option, +) -> Router { let state = DegradedState { node_did, db_startup, + web_url, }; // Everything answers 503 with the same body — including /health and // /ready, so peer readiness probes and uptime monitors correctly see a @@ -928,6 +936,9 @@ async fn degraded_node_info(State(state): State) -> impl IntoResp obj.insert("name".into(), "gitlawb-node".into()); obj.insert("version".into(), env!("CARGO_PKG_VERSION").into()); obj.insert("did".into(), state.node_did.clone().into()); + if let Some(web_url) = &state.web_url { + obj.insert("web_url".into(), web_url.clone().into()); + } } (StatusCode::SERVICE_UNAVAILABLE, Json(body)) } diff --git a/crates/gitlawb-node/src/server.rs b/crates/gitlawb-node/src/server.rs index d89bf28eb..9000abf03 100644 --- a/crates/gitlawb-node/src/server.rs +++ b/crates/gitlawb-node/src/server.rs @@ -619,7 +619,7 @@ async fn ready(State(state): State) -> axum::response::Response { async fn node_info(State(state): State) -> Json { let p2p_peer_id = state.p2p.as_ref().map(|h| h.local_peer_id.to_string()); - Json(json!({ + let mut body = json!({ "name": "gitlawb-node", "version": env!("CARGO_PKG_VERSION"), "did": state.node_did.to_string(), @@ -628,7 +628,11 @@ async fn node_info(State(state): State) -> Json { "auth": "http-signature-rfc9421", "identity": "ed25519", "p2p_peer_id": p2p_peer_id, - })) + }); + if let Some(web_url) = &state.config.web_url { + body["web_url"] = json!(web_url); + } + Json(body) } pub(crate) async fn stats(State(state): State) -> Json { diff --git a/crates/gl/Cargo.toml b/crates/gl/Cargo.toml index 3df2c76bd..bc870e6df 100644 --- a/crates/gl/Cargo.toml +++ b/crates/gl/Cargo.toml @@ -26,6 +26,8 @@ clap = { version = "4", features = ["derive", "env"] } dirs = "5" reqwest = { workspace = true } uuid = { workspace = true } +url = { workspace = true } +percent-encoding = { workspace = true } urlencoding = "2" alloy = { version = "1", default-features = false, features = [ "contract", diff --git a/crates/gl/src/mirror.rs b/crates/gl/src/mirror.rs index 4dd5371b5..f98419e26 100644 --- a/crates/gl/src/mirror.rs +++ b/crates/gl/src/mirror.rs @@ -142,7 +142,11 @@ pub async fn run(args: MirrorArgs) -> Result<()> { println!(); println!("✓ Mirror complete: {name}"); println!(" Clone: git clone {gitlawb_url}"); - println!(" View: https://twigpine.com/{owner_short}/{name}"); + // Only print View: when the node advertises a web_url — self-hosted nodes + // without a web front-end would otherwise produce a 404 link (#370). + if let Some(web_url) = crate::repo::fetch_node_web_url(&args.node).await { + println!(" View: {web_url}/{owner_short}/{name}"); + } Ok(()) } diff --git a/crates/gl/src/repo.rs b/crates/gl/src/repo.rs index 27b8de254..4db01c78f 100644 --- a/crates/gl/src/repo.rs +++ b/crates/gl/src/repo.rs @@ -5,7 +5,7 @@ use clap::{Args, Subcommand}; use serde_json::{json, Value}; use std::path::PathBuf; -use crate::http::NodeClient; +use crate::http::{sanitize_node_msg, NodeClient}; use crate::identity::load_keypair_from_dir; #[derive(Args)] @@ -274,6 +274,126 @@ async fn resolve_owner_did(_node: &str, dir: Option<&std::path::Path>) -> Result Ok(did.split(':').next_back().unwrap_or(&did).to_string()) } +/// Fetch `GET /` from the node and return the `web_url` if the node advertises one. +/// +/// The View: link is a nice-to-have, so every failure mode degrades to `None` +/// rather than failing the enclosing command — but failures are surfaced as +/// stderr warnings (request error, non-success HTTP status with the status, +/// malformed advertised value), since they all mean the node itself is +/// misbehaving or misconfigured, which the user would otherwise never learn. +/// A successful response that lacks a usable `web_url` (field missing or +/// blank) omits the link silently: self-hosted nodes without a web front-end +/// are expected to not advertise one (#370). +/// +/// The body is treated like every other caller-chosen node reply (INV-6): the +/// read is capped and an accepted `web_url` must be free of control/bidi bytes +/// — it reaches the terminal verbatim through the View: line. +pub(crate) async fn fetch_node_web_url(node: &str) -> Option { + let info_client = NodeClient::new(node, None); + let info_resp = match info_client.get("/").await { + Ok(resp) => resp, + Err(err) => { + eprintln!("warning: node info request failed ({err}); skipping View link"); + return None; + } + }; + let status = info_resp.status(); + if !status.is_success() { + eprintln!("warning: node info request returned {status}; skipping View link"); + return None; + } + // An info reply is a DID, a few URLs and counts — 8 KiB is well past what + // the shape needs; anything longer is hostile or broken (peer.rs precedent). + let raw_body = crate::http::read_body_capped(info_resp, 8 * 1024).await; + match web_url_from_info_body(&raw_body) { + Ok(url) => url, + Err(reason) => { + eprintln!("warning: {reason}; skipping View link"); + None + } + } +} + +/// Classify a capped `GET /` body for the `View:` advertisement. Split out of +/// `fetch_node_web_url` so tests can pin the classification contract directly: +/// a truncated or half-read body is indistinguishable from a node that +/// advertised nothing, so it must warn rather than silently omit the link. +fn web_url_from_info_body( + raw_body: &crate::http::CappedBody, +) -> std::result::Result, String> { + if raw_body.truncated || raw_body.read_failed { + return Err("node info body was oversized or could not be read in full".to_string()); + } + let info: Value = match serde_json::from_str(&raw_body.text) { + Ok(json) => json, + Err(_) => return Err("node info body was not valid JSON".to_string()), + }; + // Missing field / non-string degrade to None without warning — same contract + // as before; only transport- and advertisement-level failures warn there. + let raw = match info["web_url"].as_str() { + Some(v) => v, + None => return Ok(None), + }; + let trimmed = raw.trim().trim_end_matches('/'); + if trimmed.is_empty() { + return Ok(None); + } + // A present-but-malformed value means the node is misconfigured; say so + // instead of quietly dropping the link. Mirrors the node-side boot + // validation: must be an absolute http(s) URL with no query or fragment — + // the link is built by string-appending `/{owner}/{repo}` to it. Control + // and bidi-format bytes are rejected outright (not stripped): they have no + // legitimate place in a base URL and would reach the terminal through the + // View: line — including when they arrive percent-encoded (INV-6). + let malformed_reason: Option = if trimmed + .chars() + .any(|c| c.is_control() || gitlawb_core::sanitize::is_bidi_format(c)) + { + Some("contains control or bidi characters".to_string()) + } else { + match trimmed.parse::() { + Err(_) => Some("not an absolute URL".to_string()), + Ok(parsed) if !matches!(parsed.scheme(), "http" | "https") => { + Some("not http(s)".to_string()) + } + Ok(parsed) if parsed.query().is_some() || parsed.fragment().is_some() => { + Some("contains a query or fragment".to_string()) + } + // userinfo is rejected because the `View:` line prints the + // value verbatim; a remote advertisement carrying + // `user:pass@host` would land that credential in terminal + // scrollback and CI logs. + Ok(parsed) if !parsed.username().is_empty() || parsed.password().is_some() => { + Some("contains a username or password".to_string()) + } + Ok(parsed) + if parsed + .host_str() + .unwrap_or("") + .chars() + .chain( + percent_encoding::percent_decode_str(parsed.path()) + .decode_utf8_lossy() + .chars(), + ) + .any(|c| c.is_control() || gitlawb_core::sanitize::is_bidi_format(c)) => + { + Some("contains control or bidi characters, even percent-encoded".to_string()) + } + Ok(_) => None, + } + }; + match malformed_reason { + // The reason string is ours; the advertised value is not — defang it + // exactly as it would have been defanged had it been accepted. + Some(reason) => Err(format!( + "node advertised a malformed web_url ({:?}, {reason})", + sanitize_node_msg(trimmed) + )), + None => Ok(Some(trimmed.to_string())), + } +} + async fn cmd_create( name: String, description: Option, @@ -311,7 +431,11 @@ async fn cmd_create( println!("✓ Created repository: {name}"); println!(" Clone: git clone {gitlawb_url}"); println!(" HTTP: {clone_url}"); - println!(" View: https://twigpine.com/{owner_short}/{name}"); + // Only print View: when the node advertises a web_url — self-hosted nodes + // without a web front-end would otherwise produce a 404 link (#370). + if let Some(web_url) = fetch_node_web_url(&node).await { + println!(" View: {web_url}/{owner_short}/{name}"); + } if let Some(desc) = payload["description"].as_str().filter(|s| !s.is_empty()) { println!(" Desc: {desc}"); } @@ -848,6 +972,285 @@ mod tests { .unwrap(); } + #[tokio::test] + async fn test_fetch_node_web_url_with_web_url() { + let mut server = mockito::Server::new_async().await; + let _m = server + .mock("GET", "/") + .with_status(200) + .with_header("content-type", "application/json") + .with_body(r#"{"web_url":"https://example.com","did":"did:key:z6Mk"}"#) + .create_async() + .await; + + let web_url = fetch_node_web_url(&server.url()).await; + assert_eq!(web_url.as_deref(), Some("https://example.com")); + } + + #[tokio::test] + async fn test_fetch_node_web_url_trims_trailing_slash() { + let mut server = mockito::Server::new_async().await; + let _m = server + .mock("GET", "/") + .with_status(200) + .with_header("content-type", "application/json") + .with_body(r#"{"web_url":"https://example.com/"}"#) + .create_async() + .await; + + let web_url = fetch_node_web_url(&server.url()).await; + assert_eq!(web_url.as_deref(), Some("https://example.com")); + } + + #[tokio::test] + async fn test_fetch_node_web_url_without_web_url() { + let mut server = mockito::Server::new_async().await; + let _m = server + .mock("GET", "/") + .with_status(200) + .with_header("content-type", "application/json") + .with_body(r#"{"did":"did:key:z6Mk"}"#) + .create_async() + .await; + + let web_url = fetch_node_web_url(&server.url()).await; + assert_eq!(web_url, None); + } + + #[tokio::test] + async fn test_fetch_node_web_url_empty_string() { + let mut server = mockito::Server::new_async().await; + let _m = server + .mock("GET", "/") + .with_status(200) + .with_header("content-type", "application/json") + .with_body(r#"{"web_url":""}"#) + .create_async() + .await; + + let web_url = fetch_node_web_url(&server.url()).await; + assert_eq!(web_url, None); + } + + #[tokio::test] + async fn test_fetch_node_web_url_whitespace_only() { + let mut server = mockito::Server::new_async().await; + let _m = server + .mock("GET", "/") + .with_status(200) + .with_header("content-type", "application/json") + .with_body(r#"{"web_url":" "}"#) + .create_async() + .await; + + let web_url = fetch_node_web_url(&server.url()).await; + assert_eq!(web_url, None); + } + + #[tokio::test] + async fn test_fetch_node_web_url_server_error() { + let mut server = mockito::Server::new_async().await; + let _m = server + .mock("GET", "/") + .with_status(500) + .create_async() + .await; + + let web_url = fetch_node_web_url(&server.url()).await; + assert_eq!(web_url, None); + } + + /// A node advertising a present-but-malformed web_url (not an absolute + /// http(s) URL) must not yield a View link — and must warn, since a + /// malformed advertisement means the node is misconfigured (#370). + #[tokio::test] + async fn test_fetch_node_web_url_malformed_value_is_rejected() { + for bad in [ + "gitlawb.com", + "not a url", + "ftp://files.example.com", + // Query/fragment corrupt the appended /{owner}/{repo} path. + "https://example.com?a=1", + "https://example.com/#faq", + ] { + let mut server = mockito::Server::new_async().await; + let _m = server + .mock("GET", "/") + .with_status(200) + .with_header("content-type", "application/json") + .with_body(format!(r#"{{"web_url":"{bad}"}}"#)) + .create_async() + .await; + + let web_url = fetch_node_web_url(&server.url()).await; + assert_eq!( + web_url, None, + "malformed web_url {bad:?} must not render a View link" + ); + } + } + + /// A transport-level failure (connection refused) degrades to None but is + /// no longer silent: the request error is surfaced on stderr so an + /// unreachable node isn't indistinguishable from one without a web_url. + #[tokio::test] + async fn test_fetch_node_web_url_connection_refused_warns() { + // Port 1 on localhost is reserved (tcpmux) and refuses connections. + let web_url = fetch_node_web_url("http://127.0.0.1:1").await; + assert_eq!(web_url, None); + } + + /// A hostile node can smuggle ANSI/bell/bidi controls inside a web_url that + /// still passes http(s) URL parsing — those bytes would reach the terminal + /// verbatim through the View: line. The advertised value must be rejected + /// outright: no control byte may survive into the returned string. + #[tokio::test] + async fn test_fetch_node_web_url_rejects_control_bytes() { + for bad in [ + "https://example.com/\x1b[31mred", // ANSI CSI escape in path + "https://example.com\x07", // bell + "https://ex\u{202e}ample.com", // bidi override (RLO) + "https://example.com/\u{200f}", // RLM format char + "\x1b]0;title\x07https://evil.example", // OSC title-set prefix + ] { + let mut server = mockito::Server::new_async().await; + let _m = server + .mock("GET", "/") + .with_status(200) + .with_header("content-type", "application/json") + // serde_json escapes the control bytes so the body is VALID + // JSON — a raw literal would make parse fail and the test + // would pass without reaching the control check at all. + .with_body(serde_json::json!({"web_url": bad}).to_string()) + .create_async() + .await; + + let web_url = fetch_node_web_url(&server.url()).await; + assert_eq!( + web_url, None, + "web_url with control bytes {bad:?} must be rejected" + ); + } + } + + /// An oversized or half-read `GET /` body must warn, not silently + /// classify as "no web front-end": a cut body fails JSON parse, which is + /// indistinguishable from a node that advertised nothing without the + /// truncated flag. Removing the flag check makes this test pass + /// vacuously through the parse-fail arm. + #[test] + fn web_url_from_info_body_warns_on_truncated_reads() { + for (body, label) in [ + ( + crate::http::CappedBody { + text: r#"{"web_url":"https://x"#.repeat(2000), + truncated: true, + read_failed: false, + }, + "truncated", + ), + ( + crate::http::CappedBody { + text: r#"{"web_url":"https://x"#.to_string(), + truncated: false, + read_failed: true, + }, + "read_failed", + ), + ] { + let err = web_url_from_info_body(&body) + .expect_err(&format!("{label} body must warn, not silently classify")); + assert!( + err.contains("oversized or could not be read"), + "{label}: got {err}" + ); + } + // A clean body missing web_url still classifies as a silent None. + let clean = crate::http::CappedBody { + text: r#"{"did":"did:key:x"}"#.to_string(), + truncated: false, + read_failed: false, + }; + assert_eq!(web_url_from_info_body(&clean).unwrap(), None); + } + + /// Percent-encoded bidi must not slip past the literal scan: a node + /// advertising `https://example.com/%E2%80%AE` serves wire bytes with no + /// Cf code point, but a browser decodes it into a reordering character + /// on the rendered View: link (INV-6). + #[test] + fn web_url_from_info_body_rejects_percent_encoded_bidi() { + let body = crate::http::CappedBody { + text: r#"{"web_url":"https://example.com/%E2%80%AE"}"#.to_string(), + truncated: false, + read_failed: false, + }; + let err = web_url_from_info_body(&body).expect_err("percent-encoded bidi must be rejected"); + assert!(err.contains("bidi"), "got {err}"); + } + + #[tokio::test] + async fn test_fetch_node_web_url_validates_after_trimming() { + // Trailing slash is trimmed before validation; result stays usable. + let mut server = mockito::Server::new_async().await; + let _m = server + .mock("GET", "/") + .with_status(200) + .with_header("content-type", "application/json") + .with_body(r#"{"web_url":"http://localhost:8080/ui/"}"#) + .create_async() + .await; + + let web_url = fetch_node_web_url(&server.url()).await; + assert_eq!(web_url.as_deref(), Some("http://localhost:8080/ui")); + } + + /// A remote node that advertises `web_url` with userinfo (`user@`, + /// `user:pass@`) must be dropped: the value is reprinted on the `View:` + /// line, and a credential embedded in the URL would land in terminal + /// scrollback and CI logs. Mirrors the node-side boot validation. + #[tokio::test] + async fn test_fetch_node_web_url_rejects_userinfo() { + for bad in [ + "https://user@example.com", + "https://user:secret@example.com", + ] { + let mut server = mockito::Server::new_async().await; + let _m = server + .mock("GET", "/") + .with_status(200) + .with_header("content-type", "application/json") + .with_body(format!(r#"{{"web_url":"{}"}}"#, bad)) + .create_async() + .await; + + let web_url = fetch_node_web_url(&server.url()).await; + assert_eq!( + web_url, None, + "web_url with userinfo {bad:?} must be rejected" + ); + } + } + + /// A non-success status must still return None (View link is cosmetic) but + /// the status surfaces as a user-visible stderr warning instead of being + /// swallowed silently (#370 review). + #[tokio::test] + async fn test_fetch_node_web_url_non_success_warns_with_status() { + let mut server = mockito::Server::new_async().await; + let _m = server + .mock("GET", "/") + .with_status(503) + .create_async() + .await; + + let web_url = fetch_node_web_url(&server.url()).await; + assert_eq!(web_url, None); + // The warning itself goes to stderr; mockito can't capture it here, so + // this asserts only the None contract. Manual check: run against a + // 503-ing node and observe "node info request returned 503" on stderr. + } + #[tokio::test] async fn test_cmd_create_server_error() { let dir = TempDir::new().unwrap(); diff --git a/crates/gl/tests/cmd_create_view_url.rs b/crates/gl/tests/cmd_create_view_url.rs new file mode 100644 index 000000000..aa1bb7302 --- /dev/null +++ b/crates/gl/tests/cmd_create_view_url.rs @@ -0,0 +1,138 @@ +//! End-to-end test for the `View:` line printed by `gl repo create` (#370). +//! +//! `cmd_create` is the load-bearing #370 path: it is the one place the CLI +//! prints a `View:` line, and the only thing keeping it from being a +//! hardcoded `https://gitlawb.com/...` 404 is `fetch_node_web_url`. The +//! helper-level tests in `repo.rs` cover the helper, not the command path — +//! if `cmd_create` went back to a hardcoded constant, every helper test +//! would still pass. +//! +//! This integration test drives the real `gl` binary against a mockito +//! server that answers both `POST /api/v1/repos` and `GET /`. Asserting on +//! the subprocess's stdout is straightforward: bytes are bytes, with no +//! libtest / gag / in-process capture race. +//! +//! CARGO_BIN_EXE_gl is set by Cargo for integration tests of a `[[bin]]` +//! in the same package, so the test always picks up the in-tree build. + +use std::process::Stdio; +use tempfile::TempDir; +use tokio::io::AsyncWriteExt; +use tokio::process::Command; + +fn gl_bin() -> String { + std::env::var("CARGO_BIN_EXE_gl").expect("CARGO_BIN_EXE_gl is unset for this test") +} + +async fn write_identity(dir: &TempDir) { + let kp = gitlawb_core::identity::Keypair::generate(); + let pem = kp.to_pem().unwrap(); + let path = dir.path().join("identity.pem"); + let mut f = tokio::fs::File::create(&path).await.unwrap(); + f.write_all(pem.as_bytes()).await.unwrap(); +} + +/// The trailing key segment of the freshly-generated identity. Mirrors the +/// in-source `resolve_owner_did` test helper but lives in this integration +/// file because that helper is `pub(crate)`. +async fn owner_short(dir: &TempDir) -> String { + let pem = tokio::fs::read_to_string(dir.path().join("identity.pem")) + .await + .unwrap(); + let kp = gitlawb_core::identity::Keypair::from_pem(&pem).unwrap(); + let did = kp.did().to_string(); + did.split(':').next_back().unwrap_or(&did).to_string() +} + +async fn run_gl_create(node_url: &str, dir: &TempDir) -> String { + let output = Command::new(gl_bin()) + .arg("repo") + .arg("create") + .arg("myrepo") + .arg("--private") + .arg("--branch") + .arg("main") + .arg("--node") + .arg(node_url) + .arg("--dir") + .arg(dir.path()) + .env("NO_COLOR", "1") + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .output() + .await + .expect("failed to spawn `gl repo create`"); + assert!( + output.status.success(), + "`gl repo create` failed: stderr:\n{}\nstdout:\n{}", + String::from_utf8_lossy(&output.stderr), + String::from_utf8_lossy(&output.stdout), + ); + String::from_utf8(output.stdout).expect("`gl` wrote non-UTF-8 to stdout") +} + +#[tokio::test] +async fn cmd_create_view_url_tracks_node_advertisement() { + let dir = TempDir::new().unwrap(); + write_identity(&dir).await; + let owner = owner_short(&dir).await; + + // Case 1: node advertises a web_url. The `View:` line must use the + // advertised origin and the resolved owner/short name. + { + let mut server = mockito::Server::new_async().await; + let _m_create = server + .mock("POST", "/api/v1/repos") + .with_status(200) + .with_header("content-type", "application/json") + .with_body(r#"{"name":"myrepo","clone_url":"gitlawb://did:key:z6Mk/myrepo"}"#) + .create_async() + .await; + let _m_info = server + .mock("GET", "/") + .with_status(200) + .with_header("content-type", "application/json") + .with_body(r#"{"web_url":"https://git.example","did":"did:key:z6Mk"}"#) + .create_async() + .await; + let server_url = server.url(); + + let out = run_gl_create(&server_url, &dir).await; + let view_line = out + .lines() + .find(|l| l.trim_start().starts_with("View:")) + .unwrap_or_else(|| panic!("View: line missing from stdout:\n{out}")); + assert!( + view_line.contains(&format!("https://git.example/{owner}/myrepo")), + "View: line must use the advertised origin, got: {view_line:?}" + ); + } + + // Case 2: node does NOT advertise a web_url. The `View:` line must be + // absent, so a self-hosted node without a web front-end never produces + // a 404 link (#370). + { + let mut server = mockito::Server::new_async().await; + let _m_create = server + .mock("POST", "/api/v1/repos") + .with_status(200) + .with_header("content-type", "application/json") + .with_body(r#"{"name":"myrepo","clone_url":"gitlawb://did:key:z6Mk/myrepo"}"#) + .create_async() + .await; + let _m_info = server + .mock("GET", "/") + .with_status(200) + .with_header("content-type", "application/json") + .with_body(r#"{"did":"did:key:z6Mk"}"#) + .create_async() + .await; + let server_url = server.url(); + + let out = run_gl_create(&server_url, &dir).await; + assert!( + !out.lines().any(|l| l.trim_start().starts_with("View:")), + "View: line must be absent when node omits web_url; got:\n{out}" + ); + } +}