#46 Refuse forge frontmatter paths that escape out/ (#60)

src/forge.rs+13 −0
diff --git a/src/forge.rs b/src/forge.rsindex 6044b16..dda2cf3 100644--- a/src/forge.rs+++ b/src/forge.rs@@ -579,6 +579,10 @@ fn load_issues(root: &Path) -> Vec<Issue> {579579         let Some(fm): Option<IssueFm> = parse_frontmatter(yaml, &label) else {580580             continue;581581         };582+        if !crate::paths::is_safe_id(&fm.id) {583+            eprintln!("hu13: forge: skipping {label}: unsafe id {:?}", fm.id);584+            continue;585+        }582586         let comments = load_comments(583587             &dir.join(&uuid).join("comments"),584588             &format!("issues/{uuid}/comments"),@@ -620,6 +624,10 @@ fn load_prs(root: &Path) -> Vec<Pr> {620624         let Some(fm): Option<PrFm> = parse_frontmatter(yaml, &label) else {621625             continue;622626         };627+        if !crate::paths::is_safe_id(&fm.id) {628+            eprintln!("hu13: forge: skipping {label}: unsafe id {:?}", fm.id);629+            continue;630+        }623631         let patch = std::fs::read(dir.join(&uuid).join("patch.diff"))624632             .ok()625633             .map(|bytes| String::from_utf8_lossy(&bytes).into_owned());@@ -1278,6 +1286,11 @@ fn render_ci_index(12781286 }12791287 12801288 fn write_page(out: &Path, rel: &str, html: &str) -> Result<(), String> {1289+    if !crate::paths::is_safe_relative(rel) {1290+        return Err(format!(1291+            "hu13: refusing to write unsafe output path `{rel}`"1292+        ));1293+    }12811294     let dest = out.join(rel);12821295     if let Some(parent) = dest.parent() {12831296         std::fs::create_dir_all(parent).map_err(|e| format!("hu13: create {rel} parent: {e}"))?;
src/main.rs+1 −0
diff --git a/src/main.rs b/src/main.rsindex 148c678..d2ee742 100644--- a/src/main.rs+++ b/src/main.rs@@ -8,6 +8,7 @@ mod gitx;88 mod inbox;99 mod layout;1010 mod logo;11+mod paths;1112 mod releases;1213 mod serve;1314 mod wiki;
src/paths.rs+74 −0
diff --git a/src/paths.rs b/src/paths.rsnew file mode 100644index 0000000..437caaa--- /dev/null+++ b/src/paths.rs@@ -0,0 +1,74 @@1+//! Output-path safety (f0rge #60).2+//!3+//! Every path that becomes a file under the build output dir must be4+//! checked here first: `.forge/` frontmatter (`id`, release `version`) is5+//! untrusted input, and `out.join("issues/{id}.html")` with6+//! `id = "../../tmp/x"` writes outside `out/`.7+//!8+//! Rules: no absolute paths, no `\` or NUL, no empty/`.`/`..` segments.9+//! The checks are lexical on purpose — they run before anything is10+//! created, so there is nothing to canonicalize yet.11+12+/// `true` when `segment` is a single safe path component.13+#[must_use]14+pub fn is_safe_segment(segment: &str) -> bool {15+    !segment.is_empty() && segment != "." && segment != ".." && !segment.contains(['/', '\\', '\0'])16+}17+18+/// `true` when `rel` is a relative path made only of safe segments.19+#[must_use]20+pub fn is_safe_relative(rel: &str) -> bool {21+    !rel.is_empty()22+        && !rel.starts_with('/')23+        && !rel.contains(['\\', '\0'])24+        && rel.split('/').all(is_safe_segment)25+}26+27+/// `true` when `id` is safe both as a path segment and as a displayed28+/// forge id: ASCII alphanumerics, `-`, `_` only.29+#[must_use]30+pub fn is_safe_id(id: &str) -> bool {31+    is_safe_segment(id)32+        && id33+            .bytes()34+            .all(|b| b.is_ascii_alphanumeric() || b == b'-' || b == b'_')35+}36+37+#[cfg(test)]38+mod tests {39+    use super::*;40+41+    #[test]42+    fn rejects_traversal_and_absolute() {43+        for bad in [44+            "..",45+            "../x",46+            "a/../../b",47+            "/etc/passwd",48+            "a\\b",49+            "a\0b",50+            "",51+            "a//b",52+            "./a",53+            "a/",54+        ] {55+            assert!(!is_safe_relative(bad), "{bad:?} must be rejected");56+        }57+    }58+59+    #[test]60+    fn accepts_normal_relative_paths() {61+        for ok in ["a", "a/b", "a-b_c.html", "issues/1.html"] {62+            assert!(is_safe_relative(ok), "{ok:?} must be accepted");63+        }64+    }65+66+    #[test]67+    fn ids_are_strict() {68+        assert!(is_safe_id("06b5602f-4327-4377-99f6-3dca67b5bd75"));69+        assert!(is_safe_id("PWNED2"));70+        for bad in ["../../x", "a/b", "a b", "a.b", "..", "", "a\\b"] {71+            assert!(!is_safe_id(bad), "{bad:?} must be rejected");72+        }73+    }74+}
src/releases.rs+26 −17
diff --git a/src/releases.rs b/src/releases.rsindex 03ca8e5..12fbf5a 100644--- a/src/releases.rs+++ b/src/releases.rs@@ -261,20 +261,23 @@ pub fn load(repo: &Path) -> Vec<Release> {261261                 continue;262262             }263263         };264+        // `version` becomes a path segment (`releases/<v>.html`,265+        // `releases/<v>/`): untrusted frontmatter, so reject traversal266+        // loudly instead of writing outside the output dir (f0rge #60).267+        if !crate::paths::is_safe_segment(&fm.version) {268+            eprintln!(269+                "hu13: releases: skipping {label}: unsafe version {:?}",270+                fm.version271+            );272+            continue;273+        }264274         // Artifacts: present only when `out/<version>/` exists at build.265275         // Absent -> notes-only, no warning.266-        let artifacts = if fm.version.contains('/')267-            || fm.version.contains('\\')268-            || fm.version.split('/').any(|p| p == "..")269-        {270-            Vec::new()276+        let out_dir = root.join("out").join(&fm.version);277+        let artifacts = if out_dir.is_dir() {278+            collect_artifacts(&out_dir)271279         } else {272-            let out_dir = root.join("out").join(&fm.version);273-            if out_dir.is_dir() {274-                collect_artifacts(&out_dir)275-            } else {276-                Vec::new()277-            }280+            Vec::new()278281         };279282         out.push(Release {280283             version: fm.version,@@ -424,13 +427,19 @@ pub fn render_loaded(424427     std::fs::write(&dest, index).map_err(|e| format!("hu13: write releases index: {e}"))?;425428 426429     for rel in releases {430+        // `load` already rejected unsafe versions; keep the guard here431+        // too so a future caller cannot bypass it (defense in depth).432+        if !crate::paths::is_safe_segment(&rel.version) {433+            return Err(format!(434+                "hu13: refusing to write release with unsafe version `{}`",435+                rel.version436+            ));437+        }427438         // Copy the whole out dir when present (no size cap).428-        if !rel.version.contains('/') && !rel.version.contains('\\') {429-            let src_dir = root.join("out").join(&rel.version);430-            if src_dir.is_dir() {431-                let dest_dir = out.join("releases").join(&rel.version);432-                copy_tree(&src_dir, &dest_dir)?;433-            }439+        let src_dir = root.join("out").join(&rel.version);440+        if src_dir.is_dir() {441+            let dest_dir = out.join("releases").join(&rel.version);442+            copy_tree(&src_dir, &dest_dir)?;434443         }435444         let html = render_release_page(rel, site_name, nav, branch, short_sha, description);436445         let page_dest = out.join("releases").join(format!("{}.html", rel.version));
tests/path_safety.rs+134 −1
diff --git a/tests/path_safety.rs b/tests/path_safety.rsnew file mode 100644index 0000000..d56023f--- /dev/null+++ b/tests/path_safety.rs@@ -0,0 +1,134 @@1+//! f0rge #60: `.forge` frontmatter is untrusted and must never escape the2+//! output dir. Issue/PR `id` and release `version` become path segments3+//! (`issues/<id>.html`, `releases/<version>.html`, `releases/<version>/`),4+//! so a `../../..` value used to write outside `-o`.5+6+use std::path::{Path, PathBuf};7+use std::process::Command;8+9+fn git(repo: &Path, args: &[&str]) {10+    let st = Command::new("git")11+        .arg("-C")12+        .arg(repo)13+        .args(args)14+        .status()15+        .expect("spawn git");16+    assert!(st.success(), "git {args:?} failed");17+}18+19+fn fresh_dir(prefix: &str) -> PathBuf {20+    let dir = std::env::temp_dir().join(format!(21+        "hu13-paths-{prefix}-{}-{}",22+        std::process::id(),23+        std::time::SystemTime::now()24+            .duration_since(std::time::UNIX_EPOCH)25+            .map(|d| d.as_nanos())26+            .unwrap_or(0)27+    ));28+    std::fs::create_dir_all(&dir).unwrap();29+    dir30+}31+32+fn write(repo: &Path, rel: &str, content: &str) {33+    let dest = repo.join(rel);34+    std::fs::create_dir_all(dest.parent().expect("parent")).unwrap();35+    std::fs::write(&dest, content).unwrap();36+}37+38+#[test]39+fn hostile_frontmatter_cannot_write_outside_out() {40+    let repo = fresh_dir("repo");41+    let out = fresh_dir("out");42+    let marker = format!(43+        "hu13-pwn-{}-{}",44+        std::process::id(),45+        std::time::SystemTime::now()46+            .duration_since(std::time::UNIX_EPOCH)47+            .map(|d| d.as_nanos())48+            .unwrap_or(0)49+    );50+    let escaped = |name: &str| PathBuf::from(format!("/tmp/{marker}-{name}.html"));51+52+    git(&repo, &["init", "-b", "main"]);53+    git(&repo, &["config", "user.name", "hu13-test"]);54+    git(&repo, &["config", "user.email", "[email protected]"]);55+    git(&repo, &["config", "commit.gpgsign", "false"]);56+    write(&repo, "README.md", "# path safety\n");57+58+    // Issue whose id escapes 6 levels (out lives in /tmp, so /../../.. > /).59+    write(60+        &repo,61+        ".forge/issues/aaaaaaaa-1111-4111-8111-111111111111/issue.md",62+        &format!(63+            "---\nid: ../../../../../../tmp/{marker}-issue\ntitle: pwn\n\64+             state: open\ncreated: 2026-09-10T03:00:00Z\n---\nbody\n"65+        ),66+    );67+    // PR whose id escapes.68+    write(69+        &repo,70+        ".forge/prs/bbbbbbbb-2222-4222-8222-222222222222/pr.md",71+        &format!(72+            "---\nid: ../../../../../../tmp/{marker}-pr\ntitle: pwn\n\73+             state: open\nbase: main\nhead: x\ncreated: 2026-09-10T03:00:00Z\n---\nd\n"74+        ),75+    );76+    // Release whose version is absolute / traversal.77+    write(78+        &repo,79+        ".forge/releases/r1.md",80+        &format!(81+            "---\nversion: /tmp/{marker}-release\ntitle: r1\n\82+             created: 2026-09-10T03:00:00Z\n---\nnotes\n"83+        ),84+    );85+    write(86+        &repo,87+        ".forge/releases/r2.md",88+        "---\nversion: ../../../../../../tmp/{marker}-dotdot\ntitle: r2\n\89+         created: 2026-09-10T03:00:00Z\n---\nnotes\n",90+    );91+    // A valid release still has to render.92+    write(93+        &repo,94+        ".forge/releases/r3.md",95+        "---\nversion: v1.0.0\ntitle: good\ncreated: 2026-09-10T04:00:00Z\n---\nnotes\n",96+    );97+98+    git(&repo, &["add", "-A"]);99+    git(&repo, &["commit", "-m", "hostile fixture"]);100+101+    let produced = Command::new(env!("CARGO_BIN_EXE_hu13"))102+        .arg("build")103+        .arg(&repo)104+        .arg("-o")105+        .arg(&out)106+        .output()107+        .expect("spawn hu13 build");108+    let stderr = String::from_utf8_lossy(&produced.stderr);109+    assert!(110+        produced.status.success(),111+        "hostile frontmatter must not fail the build: {stderr}"112+    );113+114+    for name in ["issue", "pr", "release", "dotdot"] {115+        let p = escaped(name);116+        assert!(117+            !p.exists(),118+            "build wrote outside out/: {} exists",119+            p.display()120+        );121+    }122+    // Nothing landed at /tmp directly either (traversal is blocked, not123+    // merely clamped).124+    assert!(!Path::new(&format!("/tmp/{marker}-issue.html")).exists());125+126+    // Refused entries are warned about, naming the bad value.127+    assert!(stderr.contains("unsafe id"), "stderr: {stderr}");128+    assert!(stderr.contains("unsafe version"), "stderr: {stderr}");129+130+    // Valid content survives.131+    assert!(out.join("releases/v1.0.0.html").is_file());132+    // Build still completes with an output tree.133+    assert!(out.join("index.html").is_file());134+}0-- 2.56.0