From 8e812dd2a9fe6d2a2e264a57d5a87452b9dba6cc Mon Sep 17 00:00:00 2001 From: hu13-test Date: Tue, 27 Oct 2026 18:19:18 +0000 Subject: [PATCH] review #4: harden release artifacts, links, origin, CI redaction (#97-#105) --- .forge/wiki/deploy | 7 +- docs/cloudflare.md | 10 +- functions/inbox/[kind].js | 14 +- src/build.rs | 42 +++-- src/feed.rs | 13 +- src/forge.rs | 10 +- src/releases.rs | 33 +++- src/serve.rs | 206 ++++++++++++++++++---- src/wiki.rs | 6 +- tests/pages_inbox.test.mjs | 8 + tests/review4_hardening.rs | 344 +++++++++++++++++++++++++++++++++++++ 11 files changed, 620 insertions(+), 73 deletions(-) create mode 100644 tests/review4_hardening.rs diff --git a/.forge/wiki/deploy b/.forge/wiki/deploy index 1fd24c2..ac9b9d3 100644 --- a/.forge/wiki/deploy +++ b/.forge/wiki/deploy @@ -66,9 +66,10 @@ What happens inside (`entrypoint.sh`): `/srv/www`) so the derived `repo.git` directory never looks tracked. - Static hosting (Cloudflare Pages/nginx): publish the whole `dist/` directory. `dist/repo.git` is plain files + `update-server-info`, and - `dist/_headers` carries the `/raw/**` download policy for Pages. On - nginx, add the equivalent `add_header Content-Disposition attachment` - for `/raw/` — committed HTML/SVG must never render from that path. + `dist/_headers` carries the `/raw/**` and artifact + `/releases//**` download policy for Pages. On nginx, add the + equivalent `add_header Content-Disposition attachment` for `/raw/` and + release artifact directories — committed HTML/SVG must never render. - Cloudflare Pages inbox: the static `dist/` alone has **no inbox**. The Pages Function in `functions/inbox/[kind].js` + R2 binding `INBOX` and secret `INBOX_SALT` implement the same contract. See diff --git a/docs/cloudflare.md b/docs/cloudflare.md index dedbd9d..b561c9e 100644 --- a/docs/cloudflare.md +++ b/docs/cloudflare.md @@ -12,8 +12,8 @@ cargo build --release --public-origin https://hu13.example.com ``` -`--public-origin` makes the clone command, `og:image`, Atom links and -canonical URLs absolute. The build refuses output over Cloudflare's +`--public-origin` makes the clone command, `og:image`, and Atom links +absolute. The build refuses output over Cloudflare's limits (20,000 files, 25 MiB/file) and writes `_headers` and `_routes.json` into `dist/`. @@ -91,5 +91,7 @@ it does not read R2. Copy objects down if you want the terminal reader. `dist/` (drag-and-drop or a plain static host) gives a dead inbox. - `repo.git` is a dumb-HTTP repo: cloning works on any static host, no smart server. No GitHub dependency. -- Committed `/raw/*` bytes always download (`Content-Disposition: - attachment`) on both `serve` and Pages. +- Committed `/raw/*` bytes and release artifacts under + `/releases//*` always download (`Content-Disposition: + attachment`) on both `serve` and Pages, so hostile HTML/SVG can never + execute in the inbox origin. diff --git a/functions/inbox/[kind].js b/functions/inbox/[kind].js index d7592b8..1fa9037 100644 --- a/functions/inbox/[kind].js +++ b/functions/inbox/[kind].js @@ -83,20 +83,26 @@ function sleep(ms) { } // Same-origin gate: browsers send Origin/Referer; agents (curl) send -// neither and pass. `null`/unparsable/mismatched hosts fail closed. +// neither and pass. `null`/unparsable/mismatched host or scheme fails +// closed; `.host` keeps the port, so another local port is rejected. function originAllowed(request) { - let host; + let target; try { - host = new URL(request.url).host.toLowerCase(); + target = new URL(request.url); } catch { return false; } + const host = target.host.toLowerCase(); + const protocol = target.protocol; for (const name of ["origin", "referer"]) { const value = request.headers.get(name); if (value === null) continue; if (!value || value.trim().toLowerCase() === "null") return false; try { - if (new URL(value).host.toLowerCase() !== host) return false; + const claimed = new URL(value); + if (claimed.host.toLowerCase() !== host || claimed.protocol !== protocol) { + return false; + } } catch { return false; } diff --git a/src/build.rs b/src/build.rs index 532802d..4274e88 100644 --- a/src/build.rs +++ b/src/build.rs @@ -415,7 +415,7 @@ pub fn is_markdown(path: &str) -> bool { /// browser truncates a link at `#`/`?` (f0rge #89). `/` stays literal /// so file paths remain hierarchical; everything outside the unreserved /// set becomes `%XX` of its UTF-8 bytes. -fn encode_path(path: &str) -> String { +pub(crate) fn encode_path(path: &str) -> String { let mut out = String::with_capacity(path.len()); for &b in path.as_bytes() { if b.is_ascii_alphanumeric() || matches!(b, b'-' | b'_' | b'.' | b'~' | b'/') { @@ -427,7 +427,7 @@ fn encode_path(path: &str) -> String { out } -fn href_esc(path: &str) -> String { +pub(crate) fn href_esc(path: &str) -> String { esc(&encode_path(path)) } @@ -1148,10 +1148,15 @@ pub const ROUTES_FILE: &str = /// inbox endpoints live. `serve` sets the same policy directly /// (f0rge #61); this file gives Pages the equivalent. pub const HEADERS_FILE: &str = "\ -# generated by hu13: committed files are untrusted (f0rge #61) +# generated by hu13: committed files and release artifacts are untrusted /raw/* Content-Disposition: attachment X-Content-Type-Options: nosniff +# Artifacts are build outputs copied from .forge/releases/out//; +# never let an artifact .html/.svg execute in the site origin (f0rge #97). +/releases/*/* + Content-Disposition: attachment + X-Content-Type-Options: nosniff /* X-Content-Type-Options: nosniff Referrer-Policy: strict-origin-when-cross-origin @@ -1324,14 +1329,29 @@ fn absolutize_social_meta(out: &Path, origin: &str) -> Result<(), String> { let path = ent.path(); if path.is_dir() { stack.push(path); - } else if path.extension().is_some_and(|x| x == "html") { - let html = std::fs::read_to_string(&path) - .map_err(|e| format!("hu13: social meta read {}: {e}", path.display()))?; - let replaced = html.replace("content=\"/og.png\"", &absolute); - if replaced != html { - std::fs::write(&path, replaced) - .map_err(|e| format!("hu13: social meta write {}: {e}", path.display()))?; - } + continue; + } + if !path.extension().is_some_and(|x| x == "html") { + continue; + } + let rel = path.strip_prefix(out).unwrap_or(&path); + let rel = rel.to_string_lossy().replace('\\', "/"); + // Only generated pages carry this marker. `/raw/` and release + // artifacts are byte-for-byte copies of untrusted inputs: reading + // them as UTF-8 can fail the build and rewriting them corrupts the + // download (f0rge #100). `repo.git` is git internals, never HTML. + if rel.starts_with("raw/") + || rel.starts_with("repo.git/") + || crate::releases::is_artifact_path(&rel) + { + continue; + } + let html = std::fs::read_to_string(&path) + .map_err(|e| format!("hu13: social meta read {}: {e}", path.display()))?; + let replaced = html.replace("content=\"/og.png\"", &absolute); + if replaced != html { + std::fs::write(&path, replaced) + .map_err(|e| format!("hu13: social meta write {}: {e}", path.display()))?; } } } diff --git a/src/feed.rs b/src/feed.rs index 24be85d..f50a956 100644 --- a/src/feed.rs +++ b/src/feed.rs @@ -7,6 +7,7 @@ use std::path::Path; +use crate::build::encode_path; use crate::gitx; use crate::releases; @@ -89,17 +90,15 @@ pub fn read_release_entries(repo: &Path) -> Vec { let summary = if first_line.is_empty() { format!("release {} ({})", r.version, r.created) } else { - let mut s: String = first_line.chars().take(200).collect(); - if s.len() > 200 { - s.truncate(200); - } - s + // 200 Unicode scalar values, never a byte offset: a + // multibyte cut used to panic in `truncate` (f0rge #99). + first_line.chars().take(200).collect() }; FeedEntry { title: r.display_title().to_string(), - id: format!("urn:release:{}", r.version), + id: format!("urn:release:{}", encode_path(&r.version)), updated: r.created.clone(), - link: format!("/releases/{}.html", r.version), + link: format!("/releases/{}.html", encode_path(&r.version)), summary, } }) diff --git a/src/forge.rs b/src/forge.rs index c9f7ce6..ecdbcba 100644 --- a/src/forge.rs +++ b/src/forge.rs @@ -770,10 +770,12 @@ fn load_milestones(root: &Path) -> Vec { /// the canonical repo root with ``, then `$HOME` with `~`. fn redact_paths(text: &str, root: &Path) -> String { let mut out = text.to_string(); - let root_abs = root.canonicalize().unwrap_or_else(|_| root.to_path_buf()); - for p in [root_abs.as_path(), root] { - let needle = p.to_string_lossy(); - if !needle.is_empty() && needle != "/" { + // Only replace a canonical absolute path. `root` may be `.forge` under a + // relative repo argument (`.`); using the raw relative form as a needle + // would replace every dot in `v0.1.0`/`00.log` (f0rge #103). + if let Ok(root_abs) = root.canonicalize() { + let needle = root_abs.to_string_lossy(); + if !needle.is_empty() && needle != "/" && needle != "." && needle != ".." { out = out.replace(needle.as_ref(), ""); } } diff --git a/src/releases.rs b/src/releases.rs index 4ef2d1b..75acf0c 100644 --- a/src/releases.rs +++ b/src/releases.rs @@ -21,7 +21,7 @@ use std::path::{Path, PathBuf}; use serde::Deserialize; -use crate::build::markdown_to_html; +use crate::build::{href_esc, markdown_to_html}; use crate::layout::{esc, page_site_with_provenance}; /// `.forge/` files larger than this are skipped (schema SEC-01: 256 KiB). @@ -326,8 +326,21 @@ pub fn sha256_lines(body: &str) -> Vec { .collect() } +/// True for `releases//...` artifact paths (but not the generated +/// `releases/.html` page). Both the server and the Pages header +/// policy use this to keep artifacts downloads, never active documents. +#[must_use] +pub fn is_artifact_path(path: &str) -> bool { + let mut parts = path.split('/'); + matches!(parts.next(), Some("releases")) + && matches!(parts.next(), Some(version) if !version.is_empty()) + && parts.next().is_some() +} + fn artifact_href(version: &str, path: &str) -> String { - format!("/releases/{}/{path}", esc(version)) + // Version and artifact path are repo-controlled: percent-encode both + // (`#`/`?`/quotes) before HTML-escaping the attribute (f0rge #98/#104). + href_esc(&format!("/releases/{version}/{path}")) } fn render_release_page( @@ -422,8 +435,9 @@ pub fn render_loaded( } else { body.push_str("
\n\n\n"); for rel in releases { + let href = href_esc(&format!("/releases/{}.html", rel.version)); body.push_str(&format!( - "\n", + "\n", v = esc(&rel.version), ve = esc(&rel.version), t = esc(rel.display_title()), @@ -490,6 +504,19 @@ mod tests { assert!(sha256_lines("no hashes here\n").is_empty()); } + #[test] + fn artifact_paths_and_links_are_classified_and_encoded() { + assert!(is_artifact_path("releases/v1.0/app.bin")); + assert!(is_artifact_path("releases/v1.0/nested/app.bin")); + assert!(!is_artifact_path("releases/v1.0.html")); + assert!(!is_artifact_path("releases/index.html")); + assert!(!is_artifact_path("raw/releases/v1/app.bin")); + + let href = artifact_href("v1#x", "a\" b#c?.txt"); + assert!(href.starts_with("/releases/v1%23x/a%22%20b%23c%3F.txt")); + assert!(!href.contains('"'), "quotes are encoded"); + } + #[test] fn display_title_falls_back_to_version() { let r = Release { diff --git a/src/serve.rs b/src/serve.rs index 85df371..9edb446 100644 --- a/src/serve.rs +++ b/src/serve.rs @@ -170,11 +170,36 @@ fn absolutize(p: &Path) -> PathBuf { } } +/// Resolve symlinks in the deepest existing ancestor of `p`, then append +/// the non-existent suffix. Startup may run before the inbox dir exists. +fn resolve_existing(p: &Path) -> PathBuf { + let abs = absolutize(p); + let mut suffix: Vec = Vec::new(); + let mut cur = abs.as_path(); + loop { + if let Ok(mut resolved) = std::fs::canonicalize(cur) { + for name in suffix.iter().rev() { + resolved.push(name); + } + return resolved; + } + let Some(name) = cur.file_name() else { + return abs; + }; + suffix.push(name.to_os_string()); + let Some(parent) = cur.parent() else { + return abs; + }; + cur = parent; + } +} + /// Refuse to start when the inbox dir is inside (or equal to) www. -/// Inbox files must never be reachable over HTTP. +/// Inbox files must never be reachable over HTTP. Resolve symlinks: a +/// lexical-only check lets `--inbox-dir link-to-www/private` through. pub fn ensure_inbox_outside_www(www: &Path, inbox_dir: &Path) -> Result<(), String> { - let www = absolutize(www); - let inbox = absolutize(inbox_dir); + let www = resolve_existing(www); + let inbox = resolve_existing(inbox_dir); if inbox == www || inbox.starts_with(&www) { return Err(format!( "hu13: refusing to start: inbox-dir {} is inside www {} (inbox must never be served)", @@ -216,6 +241,12 @@ pub fn run(opts: &ServeOptions) -> i32 { return 1; } } + // Re-check after creation: a dangling symlink may only reveal that its + // target is inside www once `create_dir_all` materializes it. + if let Err(e) = ensure_inbox_outside_www(&opts.www, &opts.inbox_dir) { + eprintln!("{e}"); + return 1; + } let listener = match TcpListener::bind(&opts.listen) { Ok(l) => l, Err(e) => { @@ -451,6 +482,16 @@ fn content_type_for(name: &str) -> &'static str { "text/css; charset=utf-8" } else if lower.ends_with(".svg") { "image/svg+xml" + } else if lower.ends_with(".png") { + "image/png" + } else if lower.ends_with(".jpg") || lower.ends_with(".jpeg") { + "image/jpeg" + } else if lower.ends_with(".gif") { + "image/gif" + } else if lower.ends_with(".webp") { + "image/webp" + } else if lower.ends_with(".ico") { + "image/x-icon" } else if lower.ends_with(".webmanifest") { "application/manifest+json" } else if lower.ends_with(".json") { @@ -593,48 +634,77 @@ fn decode_path(s: &str) -> Option { String::from_utf8(bytes).ok() } -/// Hostname of an `Origin`/`Referer` value (`scheme://host[:port][/...]`) -/// or of a `Host` header (`host[:port]`), lowercased, brackets stripped. -/// `None` = unparsable (fail closed when it came from the browser). -fn host_of_url(value: &str) -> Option { +/// Normalized authority (`host` + effective port) of an `Origin`/`Referer` +/// URL (`scheme://host[:port][/...]`) or a `Host` header (`host[:port]`). +/// Scheme defaults matter: `http` → 80, `https` → 443, and a bare `Host` +/// on this plain-HTTP server → 80. `None` = unparsable/unsupported → fail +/// closed when supplied by the browser. +#[derive(Debug, PartialEq, Eq)] +struct Authority { + scheme: String, + host: String, + port: u16, +} + +fn authority_of_url(value: &str, default_scheme: &str) -> Option { let v = value.trim(); - if (v.is_empty()) || v.eq_ignore_ascii_case("null") { + if v.is_empty() || v.eq_ignore_ascii_case("null") { return None; } - let after_scheme = match v.split_once("://") { - Some((_, rest)) => rest, - None => v, + let (scheme, rest) = match v.split_once("://") { + Some((scheme, rest)) => { + let scheme = scheme.to_ascii_lowercase(); + if scheme != "http" && scheme != "https" { + return None; + } + (scheme, rest) + } + // A bare `Host` has no scheme; use the request scheme (plain HTTP, + // or `X-Forwarded-Proto` when behind a TLS proxy). + None => (default_scheme.to_string(), v), }; - let hostport = after_scheme.split('/').next().unwrap_or(""); + let hostport = rest.split('/').next().unwrap_or(""); let hostport = hostport.split('@').next_back().unwrap_or(""); if hostport.is_empty() { return None; } - // Split port (last `:` with only digits after, or `[v6]:port`). - let host = if let Some(stripped) = hostport.strip_prefix('[') { - match stripped.split_once(']') { - Some((h, _)) => h, - None => return None, - } + let (host, port) = if let Some(stripped) = hostport.strip_prefix('[') { + let (host, after) = stripped.split_once(']')?; + let port = if after.is_empty() { + None + } else { + Some(after.strip_prefix(':')?) + }; + (host, port) } else if hostport.matches(':').count() == 1 { - match hostport.split_once(':') { - Some((h, port)) if !port.is_empty() && port.bytes().all(|b| b.is_ascii_digit()) => h, - Some(_) => return None, - None => hostport, + let (host, port) = hostport.split_once(':')?; + if port.is_empty() { + return None; } + (host, Some(port)) } else { - hostport + (hostport, None) }; let host = host.trim().trim_matches('.').to_ascii_lowercase(); if host.is_empty() { return None; } - Some(host) + let port = match port { + Some(port) => port.parse::().ok()?, + None => match scheme.as_str() { + "https" => 443, + "http" => 80, + _ => return None, + }, + }; + Some(Authority { scheme, host, port }) } /// Same-origin gate for the two POST routes: no `Origin` and no /// `Referer` (curl/agents) → allow; browser-supplied `Origin` (else -/// `Referer`) must match the `Host` hostname → else `403`. Never echoes. +/// `Referer`) must match the `Host` authority, including port → else +/// `403`. Comparing only hostnames lets another local port forge a +/// same-origin request (f0rge #101). Never echoes. #[must_use] fn origin_allowed(head: &Head) -> bool { let origin = header(head, "origin"); @@ -649,9 +719,15 @@ fn origin_allowed(head: &Head) -> bool { let Some(claimed) = claimed else { return true; // curl sends none }; + let scheme = header(head, "x-forwarded-proto") + .and_then(|v| v.split(',').next()) + .map(str::trim) + .map(str::to_ascii_lowercase) + .filter(|v| v == "http" || v == "https") + .unwrap_or_else(|| "http".to_string()); let (Some(want), Some(got)) = ( - header(head, "host").and_then(host_of_url), - host_of_url(claimed), + header(head, "host").and_then(|h| authority_of_url(h, &scheme)), + authority_of_url(claimed, &scheme), ) else { return false; // unparsable Host or Origin → fail closed }; @@ -706,8 +782,9 @@ fn handle_conn(mut stream: TcpStream, shared: &Shared) { } else { rel.to_string() }; - let dest = absolutize(&shared.www.join(&file_rel)); - if dest != absolutize(&shared.www) && !dest.starts_with(absolutize(&shared.www)) { + let www = resolve_existing(&shared.www); + let dest = resolve_existing(&shared.www.join(&file_rel)); + if dest != www && !dest.starts_with(&www) { static_not_found(&mut stream, &head, shared); return; } @@ -726,13 +803,21 @@ fn handle_conn(mut stream: TcpStream, shared: &Shared) { static_error(&mut stream, 405); return; } - let is_raw = file_rel.starts_with("raw/"); - let (ctype, disposition) = if is_raw { - raw_content_type(&file_rel) + // Classify by the resolved path, not the raw request string: a request + // like `/releases//v1/evil.html` or `/./releases/...` maps to the same + // file but would bypass a string-prefix check (f0rge #97). + let served_rel = dest + .strip_prefix(&www) + .map(|p| p.to_string_lossy().replace('\\', "/")) + .unwrap_or_else(|_| file_rel.clone()); + let is_raw = served_rel.starts_with("raw/"); + let is_artifact = crate::releases::is_artifact_path(&served_rel); + let (ctype, disposition) = if is_raw || is_artifact { + raw_content_type(&served_rel) } else { - (content_type_for(&file_rel), "inline") + (content_type_for(&served_rel), "inline") }; - let extra: &[(&str, &str)] = if is_raw { + let extra: &[(&str, &str)] = if is_raw || is_artifact { &[("Content-Disposition", disposition)] } else { &[] @@ -1195,6 +1280,9 @@ mod tests { assert!(content_type_for("f.atom").contains("atom")); // m6 favicon (f0rge issue #16): svg icon + web app manifest. assert_eq!(content_type_for("favicon.svg"), "image/svg+xml"); + assert_eq!(content_type_for("og.png"), "image/png"); + assert_eq!(content_type_for("photo.JPG"), "image/jpeg"); + assert_eq!(content_type_for("icon.ico"), "image/x-icon"); assert_eq!( content_type_for("manifest.webmanifest"), "application/manifest+json" @@ -1202,6 +1290,35 @@ mod tests { assert_eq!(content_type_for("b.bundle"), "application/octet-stream"); } + #[cfg(unix)] + #[test] + fn inbox_symlink_into_www_refused() { + use std::os::unix::fs::symlink; + let base = std::env::temp_dir().join(format!( + "hu13-inbox-symlink-{}-{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_nanos()) + .unwrap_or(0) + )); + let www = base.join("www"); + let private = www.join("private"); + std::fs::create_dir_all(&private).unwrap(); + let inbox = base.join("inbox-link"); + symlink(&private, &inbox).unwrap(); + assert!(ensure_inbox_outside_www(&www, &inbox).is_err()); + + let parent = base.join("linked-parent"); + symlink(&www, &parent).unwrap(); + assert!(ensure_inbox_outside_www(&www, &parent.join("inbox")).is_err()); + + let www_link = base.join("www-link"); + symlink(&www, &www_link).unwrap(); + assert!(ensure_inbox_outside_www(&www_link, &www.join("inbox")).is_err()); + std::fs::remove_dir_all(&base).ok(); + } + #[test] fn bad_paths_rejected() { assert!(decode_path("/%2e%2e/x").is_some()); // decodes; `..` blocked later @@ -1238,6 +1355,27 @@ mod tests { Some("https://evil.example"), None ))); + // Different port or scheme is a different origin (f0rge #101). + assert!(!origin_allowed(&head_with( + "127.0.0.1:39313", + Some("http://127.0.0.1:9999"), + None + ))); + assert!(!origin_allowed(&head_with( + "127.0.0.1:39313", + Some("https://127.0.0.1:39313"), + None + ))); + assert!(!origin_allowed(&head_with( + "a.example:443", + Some("https://a.example"), + None + ))); + let mut proxied = head_with("a.example", Some("https://a.example"), None); + proxied + .headers + .push(("x-forwarded-proto".to_string(), "https".to_string())); + assert!(origin_allowed(&proxied), "TLS proxy can declare https"); assert!(!origin_allowed(&head_with( "127.0.0.1:1", Some("null"), diff --git a/src/wiki.rs b/src/wiki.rs index a0e8d7e..25bfc48 100644 --- a/src/wiki.rs +++ b/src/wiki.rs @@ -14,7 +14,7 @@ use std::collections::BTreeMap; use std::path::Path; -use crate::build::markdown_to_html; +use crate::build::{href_esc, markdown_to_html}; use crate::layout::{esc, page_site_with_provenance}; /// Wiki files larger than this are skipped (schema SEC-01: 256 KiB). @@ -288,9 +288,9 @@ pub fn render( } else { String::new() }; + let href = href_esc(&format!("/wiki/{}.html", p.path)); body.push_str(&format!( - "
  • {}{hint}
    • \n", - esc(&p.path), + "
  • {}{hint}
    • \n", esc(&p.title), )); } diff --git a/tests/pages_inbox.test.mjs b/tests/pages_inbox.test.mjs index f9625c8..a825c71 100644 --- a/tests/pages_inbox.test.mjs +++ b/tests/pages_inbox.test.mjs @@ -128,6 +128,14 @@ test("cross-origin browser POST is rejected; curl (no Origin) passes", async () assert.equal(bad.res.status, 403); const nullOrigin = await post("issue", validIssue, { origin: "null" }); assert.equal(nullOrigin.res.status, 403); + const badPort = await post("issue", validIssue, { + origin: "https://hu13.test:444", + }); + assert.equal(badPort.res.status, 403); + const badScheme = await post("issue", validIssue, { + origin: "http://hu13.test", + }); + assert.equal(badScheme.res.status, 403); const same = await post("issue", validIssue, { origin: "https://hu13.test" }); assert.equal(same.res.status, 200); }); diff --git a/tests/review4_hardening.rs b/tests/review4_hardening.rs new file mode 100644 index 0000000..961f0bc --- /dev/null +++ b/tests/review4_hardening.rs @@ -0,0 +1,344 @@ +//! Review #4 hardening (f0rge #97–#105): release artifacts are downloads, +//! artifact/feed/wiki URLs are encoded, social-meta postprocessing skips +//! copied bytes, origin checks include port/scheme, and relative CI logs +//! keep their dots. + +use std::io::{BufRead, BufReader, Read, Write}; +use std::net::TcpStream; +use std::path::{Path, PathBuf}; +use std::process::{Child, Command, Stdio}; +use std::sync::atomic::{AtomicU64, Ordering}; +use std::time::Duration; + +static SEQ: AtomicU64 = AtomicU64::new(0); + +fn fresh(prefix: &str) -> PathBuf { + let dir = std::env::temp_dir().join(format!( + "hu13-review4-{prefix}-{}-{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_nanos()) + .unwrap_or(0) + .wrapping_add(u128::from(SEQ.fetch_add(1, Ordering::Relaxed))) + )); + std::fs::create_dir_all(&dir).unwrap(); + dir +} + +fn git(repo: &Path, args: &[&str]) { + let st = Command::new("git") + .arg("-C") + .arg(repo) + .args(args) + .status() + .expect("spawn git"); + assert!(st.success(), "git {args:?} failed"); +} + +fn write(repo: &Path, rel: &str, body: &str) { + let dest = repo.join(rel); + std::fs::create_dir_all(dest.parent().expect("parent")).unwrap(); + std::fs::write(dest, body).unwrap(); +} + +fn write_bytes(repo: &Path, rel: &str, body: &[u8]) { + let dest = repo.join(rel); + std::fs::create_dir_all(dest.parent().expect("parent")).unwrap(); + std::fs::write(dest, body).unwrap(); +} + +fn read_bytes(path: &Path) -> Vec { + std::fs::read(path).unwrap_or_else(|e| panic!("read {}: {e}", path.display())) +} + +fn read(path: &Path) -> String { + String::from_utf8(read_bytes(path)).expect("utf8") +} + +const VERSION: &str = "v1#x"; +const ARTIFACT_EVIL: &str = "evil\" onmouseover=\"alert(1).html"; +const ARTIFACT_HASH: &str = "hash#q?%.txt"; + +fn fixture() -> PathBuf { + let repo = fresh("repo"); + git(&repo, &["init", "-b", "main"]); + git(&repo, &["config", "user.name", "hu13-test"]); + git(&repo, &["config", "user.email", "hu13@test.local"]); + git(&repo, &["config", "commit.gpgsign", "false"]); + write(&repo, "README.md", "# review4 fixture\n"); + write( + &repo, + "raw/x.html", + "", + ); + write_bytes(&repo, "raw/bad.html", &[0xff, 0xfe, b'<', b'x', b'>']); + write( + &repo, + ".forge/releases/v1#x.md", + &format!( + "---\nversion: \"{VERSION}\"\ntitle: Frag\ncreated: 2026-09-10T02:00:00Z\n---\n{}\nnotes\n", + "€".repeat(200) + ), + ); + let artifact_dir = format!(".forge/releases/out/{VERSION}"); + write( + &repo, + &format!("{artifact_dir}/{ARTIFACT_EVIL}"), + "", + ); + write(&repo, &format!("{artifact_dir}/{ARTIFACT_HASH}"), "hash\n"); + write( + &repo, + &format!("{artifact_dir}/artifact.html"), + "", + ); + write(&repo, ".forge/wiki/a#b.md", "# Hash wiki\n\nbody\n"); + write( + &repo, + ".forge/ci/runs/2026-09-10T00-00-00Z/summary.md", + "---\nrun: 2026-09-10T00-00-00Z\nverdict: pass\ncreated: 2026-09-10T00:00:00Z\nsteps:\n - name: 00_test.sh\n exit: 0\n duration_ms: 3\n---\nsummary\n", + ); + write( + &repo, + ".forge/ci/runs/2026-09-10T00-00-00Z/steps/00_test.sh.log", + &format!("Checking hu13 v0.1.0 at {}\n", repo.display()), + ); + git(&repo, &["add", "-A"]); + git(&repo, &["commit", "-m", "review4 fixture"]); + repo +} + +fn build(repo: &Path, out: &Path) -> std::process::Output { + Command::new(env!("CARGO_BIN_EXE_hu13")) + .arg("build") + .arg(repo) + .arg("-o") + .arg(out) + .arg("--public-origin") + .arg("https://hu13.example") + .output() + .expect("spawn hu13 build") +} + +struct Server { + child: Child, + addr: String, +} + +impl Drop for Server { + fn drop(&mut self) { + self.child.kill().ok(); + self.child.wait().ok(); + } +} + +fn start(www: &Path, inbox: &Path, extra: &[&str]) -> Server { + let mut cmd = Command::new(env!("CARGO_BIN_EXE_hu13")); + cmd.arg("serve") + .arg("--www") + .arg(www) + .arg("--inbox-dir") + .arg(inbox) + .arg("--listen") + .arg("127.0.0.1:0") + .args(extra) + .stdout(Stdio::piped()) + .stderr(Stdio::null()); + let mut child = cmd.spawn().expect("spawn serve"); + let stdout = child.stdout.take().expect("stdout"); + let mut addr = String::new(); + for line in BufReader::new(stdout).lines().take(50).flatten() { + if let Some(rest) = line.split("listening on ").nth(1) { + addr = rest.trim().to_string(); + break; + } + } + assert!(!addr.is_empty(), "serve printed no listening line"); + Server { child, addr } +} + +fn get(addr: &str, path: &str) -> String { + let mut s = TcpStream::connect(addr).expect("connect"); + s.set_read_timeout(Some(Duration::from_secs(10))).unwrap(); + write!( + s, + "GET {path} HTTP/1.1\r\nHost: {addr}\r\nConnection: close\r\n\r\n" + ) + .unwrap(); + let mut out = Vec::new(); + s.read_to_end(&mut out).unwrap(); + String::from_utf8_lossy(&out).into_owned() +} + +fn post(addr: &str, origin: Option<&str>) -> String { + let body = "title=Hello&body=World&license=agree"; + let origin_line = origin + .map(|o| format!("Origin: {o}\r\n")) + .unwrap_or_default(); + let req = format!( + "POST /inbox/issue HTTP/1.1\r\nHost: {addr}\r\n{origin_line}Content-Type: application/x-www-form-urlencoded\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{body}", + body.len() + ); + let mut s = TcpStream::connect(addr).expect("connect"); + s.set_read_timeout(Some(Duration::from_secs(10))).unwrap(); + s.write_all(req.as_bytes()).unwrap(); + let mut out = Vec::new(); + s.read_to_end(&mut out).unwrap(); + String::from_utf8_lossy(&out).into_owned() +} + +#[test] +fn review4_build_and_release_hardening() { + let repo = fixture(); + let out = fresh("out"); + let res = build(&repo, &out); + assert!( + res.status.success(), + "build failed: {}", + String::from_utf8_lossy(&res.stderr) + ); + + let release = read(&out.join(format!("releases/{VERSION}.html"))); + assert!( + release.contains("href=\"/releases/v1%23x/evil%22%20onmouseover"), + "{release}" + ); + assert!(!release.contains("onmouseover=\"alert(1)\"")); + assert!( + release.contains("hash%23q%3F%25.txt"), + "artifact path encoded" + ); + + let index = read(&out.join("releases/index.html")); + assert!(index.contains("href=\"/releases/v1%23x.html\""), "{index}"); + let wiki = read(&out.join("wiki/index.html")); + assert!(wiki.contains("href=\"/wiki/a%23b.md.html\""), "{wiki}"); + let feed = read(&out.join("feed.atom")); + assert!(feed.contains("/releases/v1%23x.html"), "{feed}"); + assert!( + feed.contains(&"€".repeat(200)), + "multibyte summary truncation must not panic or corrupt text" + ); + + let headers = read(&out.join("_headers")); + assert!(headers.contains("/releases/*/*"), "{headers}"); + assert!( + headers.contains("Content-Disposition: attachment"), + "{headers}" + ); + + // --public-origin only rewrites generated pages; copied bytes stay exact. + let raw = read_bytes(&out.join("raw/raw/x.html")); + assert_eq!( + raw, b"", + "raw HTML bytes preserved" + ); + let bad = read_bytes(&out.join("raw/raw/bad.html")); + assert_eq!(bad, vec![0xff, 0xfe, b'<', b'x', b'>']); + let artifact = read_bytes(&out.join(format!("releases/{VERSION}/artifact.html"))); + assert_eq!( + artifact, b"", + "artifact bytes preserved" + ); + assert!(read(&out.join("index.html")).contains("content=\"https://hu13.example/og.png\"")); + + let inbox = fresh("inbox"); + let srv = start(&out, &inbox, &[]); + std::thread::sleep(Duration::from_millis(100)); + + let evil = get( + &srv.addr, + "/releases/v1%23x/evil%22%20onmouseover%3D%22alert%281%29.html", + ); + assert!(evil.starts_with("HTTP/1.1 200"), "{evil}"); + assert!( + evil.contains("Content-Type: application/octet-stream"), + "{evil}" + ); + assert!(evil.contains("Content-Disposition: attachment"), "{evil}"); + + for path in [ + "/releases//v1%23x/evil%22%20onmouseover%3D%22alert%281%29.html", + "/./releases/v1%23x/evil%22%20onmouseover%3D%22alert%281%29.html", + ] { + let bypass = get(&srv.addr, path); + assert!(bypass.starts_with("HTTP/1.1 200"), "{path}: {bypass}"); + assert!( + bypass.contains("Content-Disposition: attachment"), + "{path}: {bypass}" + ); + } + + let page = get(&srv.addr, "/releases/v1%23x.html"); + assert!(page.starts_with("HTTP/1.1 200"), "{page}"); + assert!(page.contains("Content-Type: text/html"), "{page}"); + assert!(!page.contains("Content-Disposition: attachment"), "{page}"); + + let og = get(&srv.addr, "/og.png"); + assert!(og.contains("Content-Type: image/png"), "{og}"); +} + +#[test] +fn review4_relative_repo_keeps_log_dots() { + let repo = fixture(); + let out = fresh("rel-out"); + let res = Command::new(env!("CARGO_BIN_EXE_hu13")) + .current_dir(&repo) + .arg("build") + .arg(".") + .arg("-o") + .arg(&out) + .output() + .expect("spawn build"); + assert!( + res.status.success(), + "build failed: {}", + String::from_utf8_lossy(&res.stderr) + ); + let ci = read(&out.join("ci/index.html")); + assert!(ci.contains("v0.1.0"), "version dots preserved: {ci}"); + assert!( + !ci.contains("v010"), + "relative dot not redacted" + ); + assert!( + ci.contains("<repo>") || ci.contains(""), + "absolute builder path still redacted" + ); +} + +#[test] +fn review4_origin_and_static_symlink_gate() { + let inbox = fresh("origin-inbox"); + let www = fresh("origin-www"); + write(&www, "index.html", "

    origin

    \n"); + let srv = start(&www, &inbox, &["--rate-per-hour", "100", "--quota", "100"]); + std::thread::sleep(Duration::from_millis(100)); + + let bad_port = post(&srv.addr, Some("http://127.0.0.1:1")); + assert!(bad_port.starts_with("HTTP/1.1 403"), "{bad_port}"); + let bad_scheme = post(&srv.addr, Some(&format!("https://{}", srv.addr))); + assert!(bad_scheme.starts_with("HTTP/1.1 403"), "{bad_scheme}"); + let good = post(&srv.addr, Some(&format!("http://{}", srv.addr))); + assert!(good.starts_with("HTTP/1.1 200"), "{good}"); + assert_eq!( + std::fs::read_dir(&inbox).unwrap().count(), + 1, + "only same-origin submission stored" + ); + + // A symlink inside www must not escape to an outside file. + let outside = fresh("outside"); + write(&outside, "secret.txt", "SECRET\n"); + #[cfg(unix)] + std::os::unix::fs::symlink(&outside, www.join("escape")).unwrap(); + #[cfg(unix)] + { + let srv = start(&www, &inbox, &[]); + std::thread::sleep(Duration::from_millis(100)); + let escaped = get(&srv.addr, "/escape/secret.txt"); + assert!(escaped.starts_with("HTTP/1.1 404"), "{escaped}"); + assert!(!escaped.contains("SECRET")); + } +} -- 2.56.0
    titleversioncreatedartifacts
    {t}{ve}{ca}{n}
    {t}{ve}{ca}{n}