diff --git a/.env.example b/.env.example index 81c60824d..d8884692a 100644 --- a/.env.example +++ b/.env.example @@ -10,6 +10,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 3f29b0767..14c58ff19 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2181,7 +2181,7 @@ version = "3.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "faf9468729b8cbcea668e36183cb69d317348c2e08e994829fb56ebfdfbaac34" dependencies = [ - "windows-sys 0.48.0", + "windows-sys 0.61.2", ] [[package]] @@ -2630,7 +2630,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7ab67060fc6b8ef687992d439ca0fa36e7ed17e9a0b16b25b601e8757df720de" dependencies = [ "data-encoding", - "syn 1.0.109", + "syn 2.0.117", ] [[package]] @@ -3002,7 +3002,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -3484,6 +3484,7 @@ dependencies = [ "tracing", "tracing-subscriber", "unicode-normalization", + "url", "uuid", "zstd", ] @@ -3509,6 +3510,7 @@ dependencies = [ "tokio", "tracing", "tracing-subscriber", + "url", "urlencoding", "uuid", ] @@ -3543,9 +3545,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", @@ -3864,7 +3866,7 @@ dependencies = [ "libc", "percent-encoding", "pin-project-lite", - "socket2 0.5.10", + "socket2 0.6.3", "tokio", "tower-service", "tracing", @@ -5432,7 +5434,7 @@ dependencies = [ "quinn-udp", "rustc-hash", "rustls", - "socket2 0.5.10", + "socket2 0.6.3", "thiserror 2.0.18", "tokio", "tracing", @@ -5469,7 +5471,7 @@ dependencies = [ "cfg_aliases", "libc", "once_cell", - "socket2 0.5.10", + "socket2 0.6.3", "tracing", "windows-sys 0.52.0", ] @@ -5901,7 +5903,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -6795,7 +6797,7 @@ dependencies = [ "getrandom 0.4.2", "once_cell", "rustix", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 9b8b4684a..c2e60d1a0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -54,7 +54,8 @@ 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" # HMAC hmac = "0.12" diff --git a/crates/gitlawb-node/Cargo.toml b/crates/gitlawb-node/Cargo.toml index c583569cb..ecc007c19 100644 --- a/crates/gitlawb-node/Cargo.toml +++ b/crates/gitlawb-node/Cargo.toml @@ -23,6 +23,7 @@ tracing = { workspace = true } tracing-subscriber = { workspace = true } chrono = { workspace = true } uuid = { workspace = true } +url = { 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 1fbf4376e..bbb7698ad 100644 --- a/crates/gitlawb-node/src/config.rs +++ b/crates/gitlawb-node/src/config.rs @@ -63,6 +63,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, @@ -728,7 +734,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> { // A write pins one pooled connection for its whole duration (the // connection-affine advisory lock in repo_store::acquire_write), and // concurrent writes are capped at max_concurrent_git_pushes. If the pool @@ -746,10 +758,84 @@ impl Config { floor )); } + // 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(), + ); + } + Ok(()) +} + #[cfg(test)] mod tests { use super::*; @@ -1375,7 +1461,7 @@ mod tests { .expect("default config must validate"); // An under-sized pool relative to the push cap is rejected (20 < 32 + 8). - let under = Config::parse_from([ + let mut under = Config::parse_from([ "gitlawb-node", "--db-max-connections", "20", @@ -1388,7 +1474,7 @@ mod tests { ); // Exactly at the floor validates (40 == 32 + 8). - let at_floor = Config::parse_from([ + let mut at_floor = Config::parse_from([ "gitlawb-node", "--db-max-connections", "40", @@ -1401,6 +1487,129 @@ mod tests { ); } + #[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" + ); + + // 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 diff --git a/crates/gitlawb-node/src/main.rs b/crates/gitlawb-node/src/main.rs index 66bfa0961..03e6ddcc1 100644 --- a/crates/gitlawb-node/src/main.rs +++ b/crates/gitlawb-node/src/main.rs @@ -49,6 +49,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 @@ -183,6 +184,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(), )); @@ -888,11 +890,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) @@ -909,10 +912,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 @@ -944,6 +952,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 de61fcbe2..7831fd894 100644 --- a/crates/gitlawb-node/src/server.rs +++ b/crates/gitlawb-node/src/server.rs @@ -531,7 +531,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(), @@ -540,7 +540,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 2b973a4c6..f736aefff 100644 --- a/crates/gl/Cargo.toml +++ b/crates/gl/Cargo.toml @@ -26,6 +26,7 @@ clap = { version = "4", features = ["derive", "env"] } dirs = "5" reqwest = { workspace = true } uuid = { workspace = true } +url = { 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 400d3d45e..ea83af01f 100644 --- a/crates/gl/src/mirror.rs +++ b/crates/gl/src/mirror.rs @@ -134,7 +134,11 @@ pub async fn run(args: MirrorArgs) -> Result<()> { println!(); println!("✓ Mirror complete: {name}"); println!(" Clone: git clone {gitlawb_url}"); - println!(" View: https://gitlawb.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 c75a7667c..be6639883 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)] @@ -222,6 +222,89 @@ 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; + let info: Value = match serde_json::from_str(&raw_body.text) { + Ok(json) => json, + Err(_) => return None, + }; + // Missing field / non-string degrade to None without warning — same contract + // as before; only transport- and advertisement-level failures warn there. + let raw = info["web_url"].as_str()?; + let trimmed = raw.trim().trim_end_matches('/'); + if trimmed.is_empty() { + return 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. + let malformed_reason: Option<&str> = if trimmed + .chars() + .any(|c| c.is_control() || gitlawb_core::sanitize::is_bidi_format(c)) + { + Some("contains control or bidi characters") + } else { + match trimmed.parse::() { + Err(_) => Some("not an absolute URL"), + Ok(parsed) if !matches!(parsed.scheme(), "http" | "https") => Some("not http(s)"), + Ok(parsed) if parsed.query().is_some() || parsed.fragment().is_some() => { + Some("contains a query or fragment") + } + // 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") + } + Ok(_) => None, + } + }; + if let Some(reason) = 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. + let shown = sanitize_node_msg(trimmed); + eprintln!( + "warning: node advertised a malformed web_url ({shown:?}, {reason}); skipping View link" + ); + return None; + } + Some(trimmed.to_string()) +} + async fn cmd_create( name: String, description: Option, @@ -265,7 +348,11 @@ async fn cmd_create( println!("✓ Created repository: {name}"); println!(" Clone: git clone {gitlawb_url}"); println!(" HTTP: {clone_url}"); - println!(" View: https://gitlawb.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}"); } @@ -846,6 +933,226 @@ 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") + .with_body(format!(r#"{{"web_url":"{}"}}"#, bad.replace('"', "\\\""))) + .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" + ); + } + } + + #[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}" + ); + } +}