1 From 8e812dd2a9fe6d2a2e264a57d5a87452b9dba6cc Mon Sep 17 00:00:00 20012 From: hu13-test <[email protected]>3 Date: Tue, 27 Oct 2026 18:19:18 +00004 Subject: [PATCH] review #4: harden release artifacts, links, origin, CI5 redaction (#97-#105)6 7 ---8 .forge/wiki/deploy | 7 +-9 docs/cloudflare.md | 10 +-10 functions/inbox/[kind].js | 14 +-11 src/build.rs | 42 +++--12 src/feed.rs | 13 +-13 src/forge.rs | 10 +-14 src/releases.rs | 33 +++-15 src/serve.rs | 206 ++++++++++++++++++----16 src/wiki.rs | 6 +-17 tests/pages_inbox.test.mjs | 8 +18 tests/review4_hardening.rs | 344 +++++++++++++++++++++++++++++++++++++19 11 files changed, 620 insertions(+), 73 deletions(-)20 create mode 100644 tests/review4_hardening.rs21 22 diff --git a/.forge/wiki/deploy b/.forge/wiki/deploy23 index 1fd24c2..ac9b9d3 10064424 --- a/.forge/wiki/deploy25 +++ b/.forge/wiki/deploy26 @@ -66,9 +66,10 @@ What happens inside (`entrypoint.sh`):27 `/srv/www`) so the derived `repo.git` directory never looks tracked.28 - Static hosting (Cloudflare Pages/nginx): publish the whole `dist/`29 directory. `dist/repo.git` is plain files + `update-server-info`, and30 - `dist/_headers` carries the `/raw/**` download policy for Pages. On31 - nginx, add the equivalent `add_header Content-Disposition attachment`32 - for `/raw/` — committed HTML/SVG must never render from that path.33 + `dist/_headers` carries the `/raw/**` and artifact34 + `/releases/<version>/**` download policy for Pages. On nginx, add the35 + equivalent `add_header Content-Disposition attachment` for `/raw/` and36 + release artifact directories — committed HTML/SVG must never render.37 - Cloudflare Pages inbox: the static `dist/` alone has **no inbox**. The38 Pages Function in `functions/inbox/[kind].js` + R2 binding `INBOX` and39 secret `INBOX_SALT` implement the same contract. See40 diff --git a/docs/cloudflare.md b/docs/cloudflare.md41 index dedbd9d..b561c9e 10064442 --- a/docs/cloudflare.md43 +++ b/docs/cloudflare.md44 @@ -12,8 +12,8 @@ cargo build --release45 --public-origin https://hu13.example.com46 ```47 48 -`--public-origin` makes the clone command, `og:image`, Atom links and49 -canonical URLs absolute. The build refuses output over Cloudflare's50 +`--public-origin` makes the clone command, `og:image`, and Atom links51 +absolute. The build refuses output over Cloudflare's52 limits (20,000 files, 25 MiB/file) and writes `_headers` and53 `_routes.json` into `dist/`.54 55 @@ -91,5 +91,7 @@ it does not read R2. Copy objects down if you want the terminal reader.56 `dist/` (drag-and-drop or a plain static host) gives a dead inbox.57 - `repo.git` is a dumb-HTTP repo: cloning works on any static host, no58 smart server. No GitHub dependency.59 -- Committed `/raw/*` bytes always download (`Content-Disposition:60 - attachment`) on both `serve` and Pages.61 +- Committed `/raw/*` bytes and release artifacts under62 + `/releases/<version>/*` always download (`Content-Disposition:63 + attachment`) on both `serve` and Pages, so hostile HTML/SVG can never64 + execute in the inbox origin.65 diff --git a/functions/inbox/[kind].js b/functions/inbox/[kind].js66 index d7592b8..1fa9037 10064467 --- a/functions/inbox/[kind].js68 +++ b/functions/inbox/[kind].js69 @@ -83,20 +83,26 @@ function sleep(ms) {70 }71 72 // Same-origin gate: browsers send Origin/Referer; agents (curl) send73 -// neither and pass. `null`/unparsable/mismatched hosts fail closed.74 +// neither and pass. `null`/unparsable/mismatched host or scheme fails75 +// closed; `.host` keeps the port, so another local port is rejected.76 function originAllowed(request) {77 - let host;78 + let target;79 try {80 - host = new URL(request.url).host.toLowerCase();81 + target = new URL(request.url);82 } catch {83 return false;84 }85 + const host = target.host.toLowerCase();86 + const protocol = target.protocol;87 for (const name of ["origin", "referer"]) {88 const value = request.headers.get(name);89 if (value === null) continue;90 if (!value || value.trim().toLowerCase() === "null") return false;91 try {92 - if (new URL(value).host.toLowerCase() !== host) return false;93 + const claimed = new URL(value);94 + if (claimed.host.toLowerCase() !== host || claimed.protocol !== protocol) {95 + return false;96 + }97 } catch {98 return false;99 }100 diff --git a/src/build.rs b/src/build.rs101 index 532802d..4274e88 100644102 --- a/src/build.rs103 +++ b/src/build.rs104 @@ -415,7 +415,7 @@ pub fn is_markdown(path: &str) -> bool {105 /// browser truncates a link at `#`/`?` (f0rge #89). `/` stays literal106 /// so file paths remain hierarchical; everything outside the unreserved107 /// set becomes `%XX` of its UTF-8 bytes.108 -fn encode_path(path: &str) -> String {109 +pub(crate) fn encode_path(path: &str) -> String {110 let mut out = String::with_capacity(path.len());111 for &b in path.as_bytes() {112 if b.is_ascii_alphanumeric() || matches!(b, b'-' | b'_' | b'.' | b'~' | b'/') {113 @@ -427,7 +427,7 @@ fn encode_path(path: &str) -> String {114 out115 }116 117 -fn href_esc(path: &str) -> String {118 +pub(crate) fn href_esc(path: &str) -> String {119 esc(&encode_path(path))120 }121 122 @@ -1148,10 +1148,15 @@ pub const ROUTES_FILE: &str =123 /// inbox endpoints live. `serve` sets the same policy directly124 /// (f0rge #61); this file gives Pages the equivalent.125 pub const HEADERS_FILE: &str = "\126 -# generated by hu13: committed files are untrusted (f0rge #61)127 +# generated by hu13: committed files and release artifacts are untrusted128 /raw/*129 Content-Disposition: attachment130 X-Content-Type-Options: nosniff131 +# Artifacts are build outputs copied from .forge/releases/out/<ver>/;132 +# never let an artifact .html/.svg execute in the site origin (f0rge #97).133 +/releases/*/*134 + Content-Disposition: attachment135 + X-Content-Type-Options: nosniff136 /*137 X-Content-Type-Options: nosniff138 Referrer-Policy: strict-origin-when-cross-origin139 @@ -1324,14 +1329,29 @@ fn absolutize_social_meta(out: &Path, origin: &str) -> Result<(), String> {140 let path = ent.path();141 if path.is_dir() {142 stack.push(path);143 - } else if path.extension().is_some_and(|x| x == "html") {144 - let html = std::fs::read_to_string(&path)145 - .map_err(|e| format!("hu13: social meta read {}: {e}", path.display()))?;146 - let replaced = html.replace("content=\"/og.png\"", &absolute);147 - if replaced != html {148 - std::fs::write(&path, replaced)149 - .map_err(|e| format!("hu13: social meta write {}: {e}", path.display()))?;150 - }151 + continue;152 + }153 + if !path.extension().is_some_and(|x| x == "html") {154 + continue;155 + }156 + let rel = path.strip_prefix(out).unwrap_or(&path);157 + let rel = rel.to_string_lossy().replace('\\', "/");158 + // Only generated pages carry this marker. `/raw/` and release159 + // artifacts are byte-for-byte copies of untrusted inputs: reading160 + // them as UTF-8 can fail the build and rewriting them corrupts the161 + // download (f0rge #100). `repo.git` is git internals, never HTML.162 + if rel.starts_with("raw/")163 + || rel.starts_with("repo.git/")164 + || crate::releases::is_artifact_path(&rel)165 + {166 + continue;167 + }168 + let html = std::fs::read_to_string(&path)169 + .map_err(|e| format!("hu13: social meta read {}: {e}", path.display()))?;170 + let replaced = html.replace("content=\"/og.png\"", &absolute);171 + if replaced != html {172 + std::fs::write(&path, replaced)173 + .map_err(|e| format!("hu13: social meta write {}: {e}", path.display()))?;174 }175 }176 }177 diff --git a/src/feed.rs b/src/feed.rs178 index 24be85d..f50a956 100644179 --- a/src/feed.rs180 +++ b/src/feed.rs181 @@ -7,6 +7,7 @@182 183 use std::path::Path;184 185 +use crate::build::encode_path;186 use crate::gitx;187 use crate::releases;188 189 @@ -89,17 +90,15 @@ pub fn read_release_entries(repo: &Path) -> Vec<FeedEntry> {190 let summary = if first_line.is_empty() {191 format!("release {} ({})", r.version, r.created)192 } else {193 - let mut s: String = first_line.chars().take(200).collect();194 - if s.len() > 200 {195 - s.truncate(200);196 - }197 - s198 + // 200 Unicode scalar values, never a byte offset: a199 + // multibyte cut used to panic in `truncate` (f0rge #99).200 + first_line.chars().take(200).collect()201 };202 FeedEntry {203 title: r.display_title().to_string(),204 - id: format!("urn:release:{}", r.version),205 + id: format!("urn:release:{}", encode_path(&r.version)),206 updated: r.created.clone(),207 - link: format!("/releases/{}.html", r.version),208 + link: format!("/releases/{}.html", encode_path(&r.version)),209 summary,210 }211 })212 diff --git a/src/forge.rs b/src/forge.rs213 index c9f7ce6..ecdbcba 100644214 --- a/src/forge.rs215 +++ b/src/forge.rs216 @@ -770,10 +770,12 @@ fn load_milestones(root: &Path) -> Vec<Milestone> {217 /// the canonical repo root with `<repo>`, then `$HOME` with `~`.218 fn redact_paths(text: &str, root: &Path) -> String {219 let mut out = text.to_string();220 - let root_abs = root.canonicalize().unwrap_or_else(|_| root.to_path_buf());221 - for p in [root_abs.as_path(), root] {222 - let needle = p.to_string_lossy();223 - if !needle.is_empty() && needle != "/" {224 + // Only replace a canonical absolute path. `root` may be `.forge` under a225 + // relative repo argument (`.`); using the raw relative form as a needle226 + // would replace every dot in `v0.1.0`/`00.log` (f0rge #103).227 + if let Ok(root_abs) = root.canonicalize() {228 + let needle = root_abs.to_string_lossy();229 + if !needle.is_empty() && needle != "/" && needle != "." && needle != ".." {230 out = out.replace(needle.as_ref(), "<repo>");231 }232 }233 diff --git a/src/releases.rs b/src/releases.rs234 index 4ef2d1b..75acf0c 100644235 --- a/src/releases.rs236 +++ b/src/releases.rs237 @@ -21,7 +21,7 @@ use std::path::{Path, PathBuf};238 239 use serde::Deserialize;240 241 -use crate::build::markdown_to_html;242 +use crate::build::{href_esc, markdown_to_html};243 use crate::layout::{esc, page_site_with_provenance};244 245 /// `.forge/` files larger than this are skipped (schema SEC-01: 256 KiB).246 @@ -326,8 +326,21 @@ pub fn sha256_lines(body: &str) -> Vec<String> {247 .collect()248 }249 250 +/// True for `releases/<version>/...` artifact paths (but not the generated251 +/// `releases/<version>.html` page). Both the server and the Pages header252 +/// policy use this to keep artifacts downloads, never active documents.253 +#[must_use]254 +pub fn is_artifact_path(path: &str) -> bool {255 + let mut parts = path.split('/');256 + matches!(parts.next(), Some("releases"))257 + && matches!(parts.next(), Some(version) if !version.is_empty())258 + && parts.next().is_some()259 +}260 +261 fn artifact_href(version: &str, path: &str) -> String {262 - format!("/releases/{}/{path}", esc(version))263 + // Version and artifact path are repo-controlled: percent-encode both264 + // (`#`/`?`/quotes) before HTML-escaping the attribute (f0rge #98/#104).265 + href_esc(&format!("/releases/{version}/{path}"))266 }267 268 fn render_release_page(269 @@ -422,8 +435,9 @@ pub fn render_loaded(270 } else {271 body.push_str("<div class=\"table-scroll\"><table>\n<thead><tr><th>title</th><th>version</th><th>created</th><th>artifacts</th></tr></thead>\n<tbody>\n");272 for rel in releases {273 + let href = href_esc(&format!("/releases/{}.html", rel.version));274 body.push_str(&format!(275 - "<tr><td class=\"title trunc trunc-title\" data-label=\"title\" title=\"{t}\">{t}</td><td class=\"trunc\" data-label=\"version\" title=\"{v}\"><a href=\"/releases/{v}.html\">{ve}</a></td><td class=\"trunc\" data-label=\"created\" title=\"{c}\">{ca}</td><td class=\"trunc\" data-label=\"artifacts\" title=\"{n}\">{n}</td></tr>\n",276 + "<tr><td class=\"title trunc trunc-title\" data-label=\"title\" title=\"{t}\">{t}</td><td class=\"trunc\" data-label=\"version\" title=\"{v}\"><a href=\"{href}\">{ve}</a></td><td class=\"trunc\" data-label=\"created\" title=\"{c}\">{ca}</td><td class=\"trunc\" data-label=\"artifacts\" title=\"{n}\">{n}</td></tr>\n",277 v = esc(&rel.version),278 ve = esc(&rel.version),279 t = esc(rel.display_title()),280 @@ -490,6 +504,19 @@ mod tests {281 assert!(sha256_lines("no hashes here\n").is_empty());282 }283 284 + #[test]285 + fn artifact_paths_and_links_are_classified_and_encoded() {286 + assert!(is_artifact_path("releases/v1.0/app.bin"));287 + assert!(is_artifact_path("releases/v1.0/nested/app.bin"));288 + assert!(!is_artifact_path("releases/v1.0.html"));289 + assert!(!is_artifact_path("releases/index.html"));290 + assert!(!is_artifact_path("raw/releases/v1/app.bin"));291 +292 + let href = artifact_href("v1#x", "a\" b#c?.txt");293 + assert!(href.starts_with("/releases/v1%23x/a%22%20b%23c%3F.txt"));294 + assert!(!href.contains('"'), "quotes are encoded");295 + }296 +297 #[test]298 fn display_title_falls_back_to_version() {299 let r = Release {300 diff --git a/src/serve.rs b/src/serve.rs301 index 85df371..9edb446 100644302 --- a/src/serve.rs303 +++ b/src/serve.rs304 @@ -170,11 +170,36 @@ fn absolutize(p: &Path) -> PathBuf {305 }306 }307 308 +/// Resolve symlinks in the deepest existing ancestor of `p`, then append309 +/// the non-existent suffix. Startup may run before the inbox dir exists.310 +fn resolve_existing(p: &Path) -> PathBuf {311 + let abs = absolutize(p);312 + let mut suffix: Vec<std::ffi::OsString> = Vec::new();313 + let mut cur = abs.as_path();314 + loop {315 + if let Ok(mut resolved) = std::fs::canonicalize(cur) {316 + for name in suffix.iter().rev() {317 + resolved.push(name);318 + }319 + return resolved;320 + }321 + let Some(name) = cur.file_name() else {322 + return abs;323 + };324 + suffix.push(name.to_os_string());325 + let Some(parent) = cur.parent() else {326 + return abs;327 + };328 + cur = parent;329 + }330 +}331 +332 /// Refuse to start when the inbox dir is inside (or equal to) www.333 -/// Inbox files must never be reachable over HTTP.334 +/// Inbox files must never be reachable over HTTP. Resolve symlinks: a335 +/// lexical-only check lets `--inbox-dir link-to-www/private` through.336 pub fn ensure_inbox_outside_www(www: &Path, inbox_dir: &Path) -> Result<(), String> {337 - let www = absolutize(www);338 - let inbox = absolutize(inbox_dir);339 + let www = resolve_existing(www);340 + let inbox = resolve_existing(inbox_dir);341 if inbox == www || inbox.starts_with(&www) {342 return Err(format!(343 "hu13: refusing to start: inbox-dir {} is inside www {} (inbox must never be served)",344 @@ -216,6 +241,12 @@ pub fn run(opts: &ServeOptions) -> i32 {345 return 1;346 }347 }348 + // Re-check after creation: a dangling symlink may only reveal that its349 + // target is inside www once `create_dir_all` materializes it.350 + if let Err(e) = ensure_inbox_outside_www(&opts.www, &opts.inbox_dir) {351 + eprintln!("{e}");352 + return 1;353 + }354 let listener = match TcpListener::bind(&opts.listen) {355 Ok(l) => l,356 Err(e) => {357 @@ -451,6 +482,16 @@ fn content_type_for(name: &str) -> &'static str {358 "text/css; charset=utf-8"359 } else if lower.ends_with(".svg") {360 "image/svg+xml"361 + } else if lower.ends_with(".png") {362 + "image/png"363 + } else if lower.ends_with(".jpg") || lower.ends_with(".jpeg") {364 + "image/jpeg"365 + } else if lower.ends_with(".gif") {366 + "image/gif"367 + } else if lower.ends_with(".webp") {368 + "image/webp"369 + } else if lower.ends_with(".ico") {370 + "image/x-icon"371 } else if lower.ends_with(".webmanifest") {372 "application/manifest+json"373 } else if lower.ends_with(".json") {374 @@ -593,48 +634,77 @@ fn decode_path(s: &str) -> Option<String> {375 String::from_utf8(bytes).ok()376 }377 378 -/// Hostname of an `Origin`/`Referer` value (`scheme://host[:port][/...]`)379 -/// or of a `Host` header (`host[:port]`), lowercased, brackets stripped.380 -/// `None` = unparsable (fail closed when it came from the browser).381 -fn host_of_url(value: &str) -> Option<String> {382 +/// Normalized authority (`host` + effective port) of an `Origin`/`Referer`383 +/// URL (`scheme://host[:port][/...]`) or a `Host` header (`host[:port]`).384 +/// Scheme defaults matter: `http` → 80, `https` → 443, and a bare `Host`385 +/// on this plain-HTTP server → 80. `None` = unparsable/unsupported → fail386 +/// closed when supplied by the browser.387 +#[derive(Debug, PartialEq, Eq)]388 +struct Authority {389 + scheme: String,390 + host: String,391 + port: u16,392 +}393 +394 +fn authority_of_url(value: &str, default_scheme: &str) -> Option<Authority> {395 let v = value.trim();396 - if (v.is_empty()) || v.eq_ignore_ascii_case("null") {397 + if v.is_empty() || v.eq_ignore_ascii_case("null") {398 return None;399 }400 - let after_scheme = match v.split_once("://") {401 - Some((_, rest)) => rest,402 - None => v,403 + let (scheme, rest) = match v.split_once("://") {404 + Some((scheme, rest)) => {405 + let scheme = scheme.to_ascii_lowercase();406 + if scheme != "http" && scheme != "https" {407 + return None;408 + }409 + (scheme, rest)410 + }411 + // A bare `Host` has no scheme; use the request scheme (plain HTTP,412 + // or `X-Forwarded-Proto` when behind a TLS proxy).413 + None => (default_scheme.to_string(), v),414 };415 - let hostport = after_scheme.split('/').next().unwrap_or("");416 + let hostport = rest.split('/').next().unwrap_or("");417 let hostport = hostport.split('@').next_back().unwrap_or("");418 if hostport.is_empty() {419 return None;420 }421 - // Split port (last `:` with only digits after, or `[v6]:port`).422 - let host = if let Some(stripped) = hostport.strip_prefix('[') {423 - match stripped.split_once(']') {424 - Some((h, _)) => h,425 - None => return None,426 - }427 + let (host, port) = if let Some(stripped) = hostport.strip_prefix('[') {428 + let (host, after) = stripped.split_once(']')?;429 + let port = if after.is_empty() {430 + None431 + } else {432 + Some(after.strip_prefix(':')?)433 + };434 + (host, port)435 } else if hostport.matches(':').count() == 1 {436 - match hostport.split_once(':') {437 - Some((h, port)) if !port.is_empty() && port.bytes().all(|b| b.is_ascii_digit()) => h,438 - Some(_) => return None,439 - None => hostport,440 + let (host, port) = hostport.split_once(':')?;441 + if port.is_empty() {442 + return None;443 }444 + (host, Some(port))445 } else {446 - hostport447 + (hostport, None)448 };449 let host = host.trim().trim_matches('.').to_ascii_lowercase();450 if host.is_empty() {451 return None;452 }453 - Some(host)454 + let port = match port {455 + Some(port) => port.parse::<u16>().ok()?,456 + None => match scheme.as_str() {457 + "https" => 443,458 + "http" => 80,459 + _ => return None,460 + },461 + };462 + Some(Authority { scheme, host, port })463 }464 465 /// Same-origin gate for the two POST routes: no `Origin` and no466 /// `Referer` (curl/agents) → allow; browser-supplied `Origin` (else467 -/// `Referer`) must match the `Host` hostname → else `403`. Never echoes.468 +/// `Referer`) must match the `Host` authority, including port → else469 +/// `403`. Comparing only hostnames lets another local port forge a470 +/// same-origin request (f0rge #101). Never echoes.471 #[must_use]472 fn origin_allowed(head: &Head) -> bool {473 let origin = header(head, "origin");474 @@ -649,9 +719,15 @@ fn origin_allowed(head: &Head) -> bool {475 let Some(claimed) = claimed else {476 return true; // curl sends none477 };478 + let scheme = header(head, "x-forwarded-proto")479 + .and_then(|v| v.split(',').next())480 + .map(str::trim)481 + .map(str::to_ascii_lowercase)482 + .filter(|v| v == "http" || v == "https")483 + .unwrap_or_else(|| "http".to_string());484 let (Some(want), Some(got)) = (485 - header(head, "host").and_then(host_of_url),486 - host_of_url(claimed),487 + header(head, "host").and_then(|h| authority_of_url(h, &scheme)),488 + authority_of_url(claimed, &scheme),489 ) else {490 return false; // unparsable Host or Origin → fail closed491 };492 @@ -706,8 +782,9 @@ fn handle_conn(mut stream: TcpStream, shared: &Shared) {493 } else {494 rel.to_string()495 };496 - let dest = absolutize(&shared.www.join(&file_rel));497 - if dest != absolutize(&shared.www) && !dest.starts_with(absolutize(&shared.www)) {498 + let www = resolve_existing(&shared.www);499 + let dest = resolve_existing(&shared.www.join(&file_rel));500 + if dest != www && !dest.starts_with(&www) {501 static_not_found(&mut stream, &head, shared);502 return;503 }504 @@ -726,13 +803,21 @@ fn handle_conn(mut stream: TcpStream, shared: &Shared) {505 static_error(&mut stream, 405);506 return;507 }508 - let is_raw = file_rel.starts_with("raw/");509 - let (ctype, disposition) = if is_raw {510 - raw_content_type(&file_rel)511 + // Classify by the resolved path, not the raw request string: a request512 + // like `/releases//v1/evil.html` or `/./releases/...` maps to the same513 + // file but would bypass a string-prefix check (f0rge #97).514 + let served_rel = dest515 + .strip_prefix(&www)516 + .map(|p| p.to_string_lossy().replace('\\', "/"))517 + .unwrap_or_else(|_| file_rel.clone());518 + let is_raw = served_rel.starts_with("raw/");519 + let is_artifact = crate::releases::is_artifact_path(&served_rel);520 + let (ctype, disposition) = if is_raw || is_artifact {521 + raw_content_type(&served_rel)522 } else {523 - (content_type_for(&file_rel), "inline")524 + (content_type_for(&served_rel), "inline")525 };526 - let extra: &[(&str, &str)] = if is_raw {527 + let extra: &[(&str, &str)] = if is_raw || is_artifact {528 &[("Content-Disposition", disposition)]529 } else {530 &[]531 @@ -1195,6 +1280,9 @@ mod tests {532 assert!(content_type_for("f.atom").contains("atom"));533 // m6 favicon (f0rge issue #16): svg icon + web app manifest.534 assert_eq!(content_type_for("favicon.svg"), "image/svg+xml");535 + assert_eq!(content_type_for("og.png"), "image/png");536 + assert_eq!(content_type_for("photo.JPG"), "image/jpeg");537 + assert_eq!(content_type_for("icon.ico"), "image/x-icon");538 assert_eq!(539 content_type_for("manifest.webmanifest"),540 "application/manifest+json"541 @@ -1202,6 +1290,35 @@ mod tests {542 assert_eq!(content_type_for("b.bundle"), "application/octet-stream");543 }544 545 + #[cfg(unix)]546 + #[test]547 + fn inbox_symlink_into_www_refused() {548 + use std::os::unix::fs::symlink;549 + let base = std::env::temp_dir().join(format!(550 + "hu13-inbox-symlink-{}-{}",551 + std::process::id(),552 + std::time::SystemTime::now()553 + .duration_since(std::time::UNIX_EPOCH)554 + .map(|d| d.as_nanos())555 + .unwrap_or(0)556 + ));557 + let www = base.join("www");558 + let private = www.join("private");559 + std::fs::create_dir_all(&private).unwrap();560 + let inbox = base.join("inbox-link");561 + symlink(&private, &inbox).unwrap();562 + assert!(ensure_inbox_outside_www(&www, &inbox).is_err());563 +564 + let parent = base.join("linked-parent");565 + symlink(&www, &parent).unwrap();566 + assert!(ensure_inbox_outside_www(&www, &parent.join("inbox")).is_err());567 +568 + let www_link = base.join("www-link");569 + symlink(&www, &www_link).unwrap();570 + assert!(ensure_inbox_outside_www(&www_link, &www.join("inbox")).is_err());571 + std::fs::remove_dir_all(&base).ok();572 + }573 +574 #[test]575 fn bad_paths_rejected() {576 assert!(decode_path("/%2e%2e/x").is_some()); // decodes; `..` blocked later577 @@ -1238,6 +1355,27 @@ mod tests {578 Some("https://evil.example"),579 None580 )));581 + // Different port or scheme is a different origin (f0rge #101).582 + assert!(!origin_allowed(&head_with(583 + "127.0.0.1:39313",584 + Some("http://127.0.0.1:9999"),585 + None586 + )));587 + assert!(!origin_allowed(&head_with(588 + "127.0.0.1:39313",589 + Some("https://127.0.0.1:39313"),590 + None591 + )));592 + assert!(!origin_allowed(&head_with(593 + "a.example:443",594 + Some("https://a.example"),595 + None596 + )));597 + let mut proxied = head_with("a.example", Some("https://a.example"), None);598 + proxied599 + .headers600 + .push(("x-forwarded-proto".to_string(), "https".to_string()));601 + assert!(origin_allowed(&proxied), "TLS proxy can declare https");602 assert!(!origin_allowed(&head_with(603 "127.0.0.1:1",604 Some("null"),605 diff --git a/src/wiki.rs b/src/wiki.rs606 index a0e8d7e..25bfc48 100644607 --- a/src/wiki.rs608 +++ b/src/wiki.rs609 @@ -14,7 +14,7 @@610 use std::collections::BTreeMap;611 use std::path::Path;612 613 -use crate::build::markdown_to_html;614 +use crate::build::{href_esc, markdown_to_html};615 use crate::layout::{esc, page_site_with_provenance};616 617 /// Wiki files larger than this are skipped (schema SEC-01: 256 KiB).618 @@ -288,9 +288,9 @@ pub fn render(619 } else {620 String::new()621 };622 + let href = href_esc(&format!("/wiki/{}.html", p.path));623 body.push_str(&format!(624 - "<li><a href=\"/wiki/{}.html\">{}</a>{hint}</li>\n",625 - esc(&p.path),626 + "<li><a href=\"{href}\">{}</a>{hint}</li>\n",627 esc(&p.title),628 ));629 }630 diff --git a/tests/pages_inbox.test.mjs b/tests/pages_inbox.test.mjs631 index f9625c8..a825c71 100644632 --- a/tests/pages_inbox.test.mjs633 +++ b/tests/pages_inbox.test.mjs634 @@ -128,6 +128,14 @@ test("cross-origin browser POST is rejected; curl (no Origin) passes", async ()635 assert.equal(bad.res.status, 403);636 const nullOrigin = await post("issue", validIssue, { origin: "null" });637 assert.equal(nullOrigin.res.status, 403);638 + const badPort = await post("issue", validIssue, {639 + origin: "https://hu13.test:444",640 + });641 + assert.equal(badPort.res.status, 403);642 + const badScheme = await post("issue", validIssue, {643 + origin: "http://hu13.test",644 + });645 + assert.equal(badScheme.res.status, 403);646 const same = await post("issue", validIssue, { origin: "https://hu13.test" });647 assert.equal(same.res.status, 200);648 });649 diff --git a/tests/review4_hardening.rs b/tests/review4_hardening.rs650 new file mode 100644651 index 0000000..961f0bc652 --- /dev/null653 +++ b/tests/review4_hardening.rs654 @@ -0,0 +1,344 @@655 +//! Review #4 hardening (f0rge #97–#105): release artifacts are downloads,656 +//! artifact/feed/wiki URLs are encoded, social-meta postprocessing skips657 +//! copied bytes, origin checks include port/scheme, and relative CI logs658 +//! keep their dots.659 +660 +use std::io::{BufRead, BufReader, Read, Write};661 +use std::net::TcpStream;662 +use std::path::{Path, PathBuf};663 +use std::process::{Child, Command, Stdio};664 +use std::sync::atomic::{AtomicU64, Ordering};665 +use std::time::Duration;666 +667 +static SEQ: AtomicU64 = AtomicU64::new(0);668 +669 +fn fresh(prefix: &str) -> PathBuf {670 + let dir = std::env::temp_dir().join(format!(671 + "hu13-review4-{prefix}-{}-{}",672 + std::process::id(),673 + std::time::SystemTime::now()674 + .duration_since(std::time::UNIX_EPOCH)675 + .map(|d| d.as_nanos())676 + .unwrap_or(0)677 + .wrapping_add(u128::from(SEQ.fetch_add(1, Ordering::Relaxed)))678 + ));679 + std::fs::create_dir_all(&dir).unwrap();680 + dir681 +}682 +683 +fn git(repo: &Path, args: &[&str]) {684 + let st = Command::new("git")685 + .arg("-C")686 + .arg(repo)687 + .args(args)688 + .status()689 + .expect("spawn git");690 + assert!(st.success(), "git {args:?} failed");691 +}692 +693 +fn write(repo: &Path, rel: &str, body: &str) {694 + let dest = repo.join(rel);695 + std::fs::create_dir_all(dest.parent().expect("parent")).unwrap();696 + std::fs::write(dest, body).unwrap();697 +}698 +699 +fn write_bytes(repo: &Path, rel: &str, body: &[u8]) {700 + let dest = repo.join(rel);701 + std::fs::create_dir_all(dest.parent().expect("parent")).unwrap();702 + std::fs::write(dest, body).unwrap();703 +}704 +705 +fn read_bytes(path: &Path) -> Vec<u8> {706 + std::fs::read(path).unwrap_or_else(|e| panic!("read {}: {e}", path.display()))707 +}708 +709 +fn read(path: &Path) -> String {710 + String::from_utf8(read_bytes(path)).expect("utf8")711 +}712 +713 +const VERSION: &str = "v1#x";714 +const ARTIFACT_EVIL: &str = "evil\" onmouseover=\"alert(1).html";715 +const ARTIFACT_HASH: &str = "hash#q?%.txt";716 +717 +fn fixture() -> PathBuf {718 + let repo = fresh("repo");719 + git(&repo, &["init", "-b", "main"]);720 + git(&repo, &["config", "user.name", "hu13-test"]);721 + git(&repo, &["config", "user.email", "[email protected]"]);722 + git(&repo, &["config", "commit.gpgsign", "false"]);723 + write(&repo, "README.md", "# review4 fixture\n");724 + write(725 + &repo,726 + "raw/x.html",727 + "<meta property=\"og:image\" content=\"/og.png\">",728 + );729 + write_bytes(&repo, "raw/bad.html", &[0xff, 0xfe, b'<', b'x', b'>']);730 + write(731 + &repo,732 + ".forge/releases/v1#x.md",733 + &format!(734 + "---\nversion: \"{VERSION}\"\ntitle: Frag\ncreated: 2026-09-10T02:00:00Z\n---\n{}\nnotes\n",735 + "€".repeat(200)736 + ),737 + );738 + let artifact_dir = format!(".forge/releases/out/{VERSION}");739 + write(740 + &repo,741 + &format!("{artifact_dir}/{ARTIFACT_EVIL}"),742 + "<script>alert(1)</script>",743 + );744 + write(&repo, &format!("{artifact_dir}/{ARTIFACT_HASH}"), "hash\n");745 + write(746 + &repo,747 + &format!("{artifact_dir}/artifact.html"),748 + "<meta property=\"og:image\" content=\"/og.png\">",749 + );750 + write(&repo, ".forge/wiki/a#b.md", "# Hash wiki\n\nbody\n");751 + write(752 + &repo,753 + ".forge/ci/runs/2026-09-10T00-00-00Z/summary.md",754 + "---\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",755 + );756 + write(757 + &repo,758 + ".forge/ci/runs/2026-09-10T00-00-00Z/steps/00_test.sh.log",759 + &format!("Checking hu13 v0.1.0 at {}\n", repo.display()),760 + );761 + git(&repo, &["add", "-A"]);762 + git(&repo, &["commit", "-m", "review4 fixture"]);763 + repo764 +}765 +766 +fn build(repo: &Path, out: &Path) -> std::process::Output {767 + Command::new(env!("CARGO_BIN_EXE_hu13"))768 + .arg("build")769 + .arg(repo)770 + .arg("-o")771 + .arg(out)772 + .arg("--public-origin")773 + .arg("https://hu13.example")774 + .output()775 + .expect("spawn hu13 build")776 +}777 +778 +struct Server {779 + child: Child,780 + addr: String,781 +}782 +783 +impl Drop for Server {784 + fn drop(&mut self) {785 + self.child.kill().ok();786 + self.child.wait().ok();787 + }788 +}789 +790 +fn start(www: &Path, inbox: &Path, extra: &[&str]) -> Server {791 + let mut cmd = Command::new(env!("CARGO_BIN_EXE_hu13"));792 + cmd.arg("serve")793 + .arg("--www")794 + .arg(www)795 + .arg("--inbox-dir")796 + .arg(inbox)797 + .arg("--listen")798 + .arg("127.0.0.1:0")799 + .args(extra)800 + .stdout(Stdio::piped())801 + .stderr(Stdio::null());802 + let mut child = cmd.spawn().expect("spawn serve");803 + let stdout = child.stdout.take().expect("stdout");804 + let mut addr = String::new();805 + for line in BufReader::new(stdout).lines().take(50).flatten() {806 + if let Some(rest) = line.split("listening on ").nth(1) {807 + addr = rest.trim().to_string();808 + break;809 + }810 + }811 + assert!(!addr.is_empty(), "serve printed no listening line");812 + Server { child, addr }813 +}814 +815 +fn get(addr: &str, path: &str) -> String {816 + let mut s = TcpStream::connect(addr).expect("connect");817 + s.set_read_timeout(Some(Duration::from_secs(10))).unwrap();818 + write!(819 + s,820 + "GET {path} HTTP/1.1\r\nHost: {addr}\r\nConnection: close\r\n\r\n"821 + )822 + .unwrap();823 + let mut out = Vec::new();824 + s.read_to_end(&mut out).unwrap();825 + String::from_utf8_lossy(&out).into_owned()826 +}827 +828 +fn post(addr: &str, origin: Option<&str>) -> String {829 + let body = "title=Hello&body=World&license=agree";830 + let origin_line = origin831 + .map(|o| format!("Origin: {o}\r\n"))832 + .unwrap_or_default();833 + let req = format!(834 + "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}",835 + body.len()836 + );837 + let mut s = TcpStream::connect(addr).expect("connect");838 + s.set_read_timeout(Some(Duration::from_secs(10))).unwrap();839 + s.write_all(req.as_bytes()).unwrap();840 + let mut out = Vec::new();841 + s.read_to_end(&mut out).unwrap();842 + String::from_utf8_lossy(&out).into_owned()843 +}844 +845 +#[test]846 +fn review4_build_and_release_hardening() {847 + let repo = fixture();848 + let out = fresh("out");849 + let res = build(&repo, &out);850 + assert!(851 + res.status.success(),852 + "build failed: {}",853 + String::from_utf8_lossy(&res.stderr)854 + );855 +856 + let release = read(&out.join(format!("releases/{VERSION}.html")));857 + assert!(858 + release.contains("href=\"/releases/v1%23x/evil%22%20onmouseover"),859 + "{release}"860 + );861 + assert!(!release.contains("onmouseover=\"alert(1)\""));862 + assert!(863 + release.contains("hash%23q%3F%25.txt"),864 + "artifact path encoded"865 + );866 +867 + let index = read(&out.join("releases/index.html"));868 + assert!(index.contains("href=\"/releases/v1%23x.html\""), "{index}");869 + let wiki = read(&out.join("wiki/index.html"));870 + assert!(wiki.contains("href=\"/wiki/a%23b.md.html\""), "{wiki}");871 + let feed = read(&out.join("feed.atom"));872 + assert!(feed.contains("/releases/v1%23x.html"), "{feed}");873 + assert!(874 + feed.contains(&"€".repeat(200)),875 + "multibyte summary truncation must not panic or corrupt text"876 + );877 +878 + let headers = read(&out.join("_headers"));879 + assert!(headers.contains("/releases/*/*"), "{headers}");880 + assert!(881 + headers.contains("Content-Disposition: attachment"),882 + "{headers}"883 + );884 +885 + // --public-origin only rewrites generated pages; copied bytes stay exact.886 + let raw = read_bytes(&out.join("raw/raw/x.html"));887 + assert_eq!(888 + raw, b"<meta property=\"og:image\" content=\"/og.png\">",889 + "raw HTML bytes preserved"890 + );891 + let bad = read_bytes(&out.join("raw/raw/bad.html"));892 + assert_eq!(bad, vec![0xff, 0xfe, b'<', b'x', b'>']);893 + let artifact = read_bytes(&out.join(format!("releases/{VERSION}/artifact.html")));894 + assert_eq!(895 + artifact, b"<meta property=\"og:image\" content=\"/og.png\">",896 + "artifact bytes preserved"897 + );898 + assert!(read(&out.join("index.html")).contains("content=\"https://hu13.example/og.png\""));899 +900 + let inbox = fresh("inbox");901 + let srv = start(&out, &inbox, &[]);902 + std::thread::sleep(Duration::from_millis(100));903 +904 + let evil = get(905 + &srv.addr,906 + "/releases/v1%23x/evil%22%20onmouseover%3D%22alert%281%29.html",907 + );908 + assert!(evil.starts_with("HTTP/1.1 200"), "{evil}");909 + assert!(910 + evil.contains("Content-Type: application/octet-stream"),911 + "{evil}"912 + );913 + assert!(evil.contains("Content-Disposition: attachment"), "{evil}");914 +915 + for path in [916 + "/releases//v1%23x/evil%22%20onmouseover%3D%22alert%281%29.html",917 + "/./releases/v1%23x/evil%22%20onmouseover%3D%22alert%281%29.html",918 + ] {919 + let bypass = get(&srv.addr, path);920 + assert!(bypass.starts_with("HTTP/1.1 200"), "{path}: {bypass}");921 + assert!(922 + bypass.contains("Content-Disposition: attachment"),923 + "{path}: {bypass}"924 + );925 + }926 +927 + let page = get(&srv.addr, "/releases/v1%23x.html");928 + assert!(page.starts_with("HTTP/1.1 200"), "{page}");929 + assert!(page.contains("Content-Type: text/html"), "{page}");930 + assert!(!page.contains("Content-Disposition: attachment"), "{page}");931 +932 + let og = get(&srv.addr, "/og.png");933 + assert!(og.contains("Content-Type: image/png"), "{og}");934 +}935 +936 +#[test]937 +fn review4_relative_repo_keeps_log_dots() {938 + let repo = fixture();939 + let out = fresh("rel-out");940 + let res = Command::new(env!("CARGO_BIN_EXE_hu13"))941 + .current_dir(&repo)942 + .arg("build")943 + .arg(".")944 + .arg("-o")945 + .arg(&out)946 + .output()947 + .expect("spawn build");948 + assert!(949 + res.status.success(),950 + "build failed: {}",951 + String::from_utf8_lossy(&res.stderr)952 + );953 + let ci = read(&out.join("ci/index.html"));954 + assert!(ci.contains("v0.1.0"), "version dots preserved: {ci}");955 + assert!(956 + !ci.contains("v0<repo>1<repo>0"),957 + "relative dot not redacted"958 + );959 + assert!(960 + ci.contains("<repo>") || ci.contains("<repo>"),961 + "absolute builder path still redacted"962 + );963 +}964 +965 +#[test]966 +fn review4_origin_and_static_symlink_gate() {967 + let inbox = fresh("origin-inbox");968 + let www = fresh("origin-www");969 + write(&www, "index.html", "<p>origin</p>\n");970 + let srv = start(&www, &inbox, &["--rate-per-hour", "100", "--quota", "100"]);971 + std::thread::sleep(Duration::from_millis(100));972 +973 + let bad_port = post(&srv.addr, Some("http://127.0.0.1:1"));974 + assert!(bad_port.starts_with("HTTP/1.1 403"), "{bad_port}");975 + let bad_scheme = post(&srv.addr, Some(&format!("https://{}", srv.addr)));976 + assert!(bad_scheme.starts_with("HTTP/1.1 403"), "{bad_scheme}");977 + let good = post(&srv.addr, Some(&format!("http://{}", srv.addr)));978 + assert!(good.starts_with("HTTP/1.1 200"), "{good}");979 + assert_eq!(980 + std::fs::read_dir(&inbox).unwrap().count(),981 + 1,982 + "only same-origin submission stored"983 + );984 +985 + // A symlink inside www must not escape to an outside file.986 + let outside = fresh("outside");987 + write(&outside, "secret.txt", "SECRET\n");988 + #[cfg(unix)]989 + std::os::unix::fs::symlink(&outside, www.join("escape")).unwrap();990 + #[cfg(unix)]991 + {992 + let srv = start(&www, &inbox, &[]);993 + std::thread::sleep(Duration::from_millis(100));994 + let escaped = get(&srv.addr, "/escape/secret.txt");995 + assert!(escaped.starts_with("HTTP/1.1 404"), "{escaped}");996 + assert!(!escaped.contains("SECRET"));997 + }998 +}999 -- 1000 2.56.01001
/
•
•