merge #60 path traversal guard

2d4e35fgl0w

all commits

Message

merge #60 path traversal guard

Diff

.forge/merges/2026-10-27T15-34-02.513214394Z.md+9 −0
diff --git a/.forge/merges/2026-10-27T15-34-02.513214394Z.md b/.forge/merges/2026-10-27T15-34-02.513214394Z.mdnew file mode 100644index 0000000..af96799--- /dev/null+++ b/.forge/merges/2026-10-27T15-34-02.513214394Z.md@@ -0,0 +1,9 @@1+---2+pr: 44ea71b1-a7ab-408b-8618-92b4dbbe0e853+applied: true4+verdict: pass5+resolved: []6+created: 2026-10-27T15:34:02.513214394Z7+---8+clean apply9+patch: c6a16089b1cfab4ad0f6865e0623db62136d76b8
.forge/prs/44ea71b1-a7ab-408b-8618-92b4dbbe0e85/patch.diff+360 −0
diff --git a/.forge/prs/44ea71b1-a7ab-408b-8618-92b4dbbe0e85/patch.diff b/.forge/prs/44ea71b1-a7ab-408b-8618-92b4dbbe0e85/patch.diffnew file mode 100644index 0000000..cdeb0a6--- /dev/null+++ b/.forge/prs/44ea71b1-a7ab-408b-8618-92b4dbbe0e85/patch.diff@@ -0,0 +1,360 @@1+From f88f679b30b91964b59f9d03ade26d612e076149 Mon Sep 17 00:00:00 20012+From: hu13-test <[email protected]>3+Date: Tue, 27 Oct 2026 15:33:24 +00004+Subject: [PATCH] refuse forge frontmatter paths that escape out/ (#60)5+6+Issue/PR id and release version become path segments7+(issues/<id>.html, releases/<v>.html, releases/<v>/), so a8+'../../tmp/x' value wrote outside the build output. Validate ids9+([A-Za-z0-9_-]) and versions (single safe segment) at load time,10+guard write_page() with a lexical relative-path check, and refuse11+unsafe release versions in render too. Refused entries warn and are12+skipped; the build still succeeds.13+---14+ src/forge.rs         |  13 +++++15+ src/main.rs          |   1 +16+ src/paths.rs         |  74 ++++++++++++++++++++++++17+ src/releases.rs      |  43 ++++++++------18+ tests/path_safety.rs | 134 +++++++++++++++++++++++++++++++++++++++++++19+ 5 files changed, 248 insertions(+), 17 deletions(-)20+ create mode 100644 src/paths.rs21+ create mode 100644 tests/path_safety.rs22+23+diff --git a/src/forge.rs b/src/forge.rs24+index 6044b16..dda2cf3 10064425+--- a/src/forge.rs26++++ b/src/forge.rs27+@@ -579,6 +579,10 @@ fn load_issues(root: &Path) -> Vec<Issue> {28+         let Some(fm): Option<IssueFm> = parse_frontmatter(yaml, &label) else {29+             continue;30+         };31++        if !crate::paths::is_safe_id(&fm.id) {32++            eprintln!("hu13: forge: skipping {label}: unsafe id {:?}", fm.id);33++            continue;34++        }35+         let comments = load_comments(36+             &dir.join(&uuid).join("comments"),37+             &format!("issues/{uuid}/comments"),38+@@ -620,6 +624,10 @@ fn load_prs(root: &Path) -> Vec<Pr> {39+         let Some(fm): Option<PrFm> = parse_frontmatter(yaml, &label) else {40+             continue;41+         };42++        if !crate::paths::is_safe_id(&fm.id) {43++            eprintln!("hu13: forge: skipping {label}: unsafe id {:?}", fm.id);44++            continue;45++        }46+         let patch = std::fs::read(dir.join(&uuid).join("patch.diff"))47+             .ok()48+             .map(|bytes| String::from_utf8_lossy(&bytes).into_owned());49+@@ -1278,6 +1286,11 @@ fn render_ci_index(50+ }51+ 52+ fn write_page(out: &Path, rel: &str, html: &str) -> Result<(), String> {53++    if !crate::paths::is_safe_relative(rel) {54++        return Err(format!(55++            "hu13: refusing to write unsafe output path `{rel}`"56++        ));57++    }58+     let dest = out.join(rel);59+     if let Some(parent) = dest.parent() {60+         std::fs::create_dir_all(parent).map_err(|e| format!("hu13: create {rel} parent: {e}"))?;61+diff --git a/src/main.rs b/src/main.rs62+index 148c678..d2ee742 10064463+--- a/src/main.rs64++++ b/src/main.rs65+@@ -8,6 +8,7 @@ mod gitx;66+ mod inbox;67+ mod layout;68+ mod logo;69++mod paths;70+ mod releases;71+ mod serve;72+ mod wiki;73+diff --git a/src/paths.rs b/src/paths.rs74+new file mode 10064475+index 0000000..437caaa76+--- /dev/null77++++ b/src/paths.rs78+@@ -0,0 +1,74 @@79++//! Output-path safety (f0rge #60).80++//!81++//! Every path that becomes a file under the build output dir must be82++//! checked here first: `.forge/` frontmatter (`id`, release `version`) is83++//! untrusted input, and `out.join("issues/{id}.html")` with84++//! `id = "../../tmp/x"` writes outside `out/`.85++//!86++//! Rules: no absolute paths, no `\` or NUL, no empty/`.`/`..` segments.87++//! The checks are lexical on purpose — they run before anything is88++//! created, so there is nothing to canonicalize yet.89++90++/// `true` when `segment` is a single safe path component.91++#[must_use]92++pub fn is_safe_segment(segment: &str) -> bool {93++    !segment.is_empty() && segment != "." && segment != ".." && !segment.contains(['/', '\\', '\0'])94++}95++96++/// `true` when `rel` is a relative path made only of safe segments.97++#[must_use]98++pub fn is_safe_relative(rel: &str) -> bool {99++    !rel.is_empty()100++        && !rel.starts_with('/')101++        && !rel.contains(['\\', '\0'])102++        && rel.split('/').all(is_safe_segment)103++}104++105++/// `true` when `id` is safe both as a path segment and as a displayed106++/// forge id: ASCII alphanumerics, `-`, `_` only.107++#[must_use]108++pub fn is_safe_id(id: &str) -> bool {109++    is_safe_segment(id)110++        && id111++            .bytes()112++            .all(|b| b.is_ascii_alphanumeric() || b == b'-' || b == b'_')113++}114++115++#[cfg(test)]116++mod tests {117++    use super::*;118++119++    #[test]120++    fn rejects_traversal_and_absolute() {121++        for bad in [122++            "..",123++            "../x",124++            "a/../../b",125++            "/etc/passwd",126++            "a\\b",127++            "a\0b",128++            "",129++            "a//b",130++            "./a",131++            "a/",132++        ] {133++            assert!(!is_safe_relative(bad), "{bad:?} must be rejected");134++        }135++    }136++137++    #[test]138++    fn accepts_normal_relative_paths() {139++        for ok in ["a", "a/b", "a-b_c.html", "issues/1.html"] {140++            assert!(is_safe_relative(ok), "{ok:?} must be accepted");141++        }142++    }143++144++    #[test]145++    fn ids_are_strict() {146++        assert!(is_safe_id("06b5602f-4327-4377-99f6-3dca67b5bd75"));147++        assert!(is_safe_id("PWNED2"));148++        for bad in ["../../x", "a/b", "a b", "a.b", "..", "", "a\\b"] {149++            assert!(!is_safe_id(bad), "{bad:?} must be rejected");150++        }151++    }152++}153+diff --git a/src/releases.rs b/src/releases.rs154+index 03ca8e5..12fbf5a 100644155+--- a/src/releases.rs156++++ b/src/releases.rs157+@@ -261,20 +261,23 @@ pub fn load(repo: &Path) -> Vec<Release> {158+                 continue;159+             }160+         };161++        // `version` becomes a path segment (`releases/<v>.html`,162++        // `releases/<v>/`): untrusted frontmatter, so reject traversal163++        // loudly instead of writing outside the output dir (f0rge #60).164++        if !crate::paths::is_safe_segment(&fm.version) {165++            eprintln!(166++                "hu13: releases: skipping {label}: unsafe version {:?}",167++                fm.version168++            );169++            continue;170++        }171+         // Artifacts: present only when `out/<version>/` exists at build.172+         // Absent -> notes-only, no warning.173+-        let artifacts = if fm.version.contains('/')174+-            || fm.version.contains('\\')175+-            || fm.version.split('/').any(|p| p == "..")176+-        {177+-            Vec::new()178++        let out_dir = root.join("out").join(&fm.version);179++        let artifacts = if out_dir.is_dir() {180++            collect_artifacts(&out_dir)181+         } else {182+-            let out_dir = root.join("out").join(&fm.version);183+-            if out_dir.is_dir() {184+-                collect_artifacts(&out_dir)185+-            } else {186+-                Vec::new()187+-            }188++            Vec::new()189+         };190+         out.push(Release {191+             version: fm.version,192+@@ -424,13 +427,19 @@ pub fn render_loaded(193+     std::fs::write(&dest, index).map_err(|e| format!("hu13: write releases index: {e}"))?;194+ 195+     for rel in releases {196++        // `load` already rejected unsafe versions; keep the guard here197++        // too so a future caller cannot bypass it (defense in depth).198++        if !crate::paths::is_safe_segment(&rel.version) {199++            return Err(format!(200++                "hu13: refusing to write release with unsafe version `{}`",201++                rel.version202++            ));203++        }204+         // Copy the whole out dir when present (no size cap).205+-        if !rel.version.contains('/') && !rel.version.contains('\\') {206+-            let src_dir = root.join("out").join(&rel.version);207+-            if src_dir.is_dir() {208+-                let dest_dir = out.join("releases").join(&rel.version);209+-                copy_tree(&src_dir, &dest_dir)?;210+-            }211++        let src_dir = root.join("out").join(&rel.version);212++        if src_dir.is_dir() {213++            let dest_dir = out.join("releases").join(&rel.version);214++            copy_tree(&src_dir, &dest_dir)?;215+         }216+         let html = render_release_page(rel, site_name, nav, branch, short_sha, description);217+         let page_dest = out.join("releases").join(format!("{}.html", rel.version));218+diff --git a/tests/path_safety.rs b/tests/path_safety.rs219+new file mode 100644220+index 0000000..d56023f221+--- /dev/null222++++ b/tests/path_safety.rs223+@@ -0,0 +1,134 @@224++//! f0rge #60: `.forge` frontmatter is untrusted and must never escape the225++//! output dir. Issue/PR `id` and release `version` become path segments226++//! (`issues/<id>.html`, `releases/<version>.html`, `releases/<version>/`),227++//! so a `../../..` value used to write outside `-o`.228++229++use std::path::{Path, PathBuf};230++use std::process::Command;231++232++fn git(repo: &Path, args: &[&str]) {233++    let st = Command::new("git")234++        .arg("-C")235++        .arg(repo)236++        .args(args)237++        .status()238++        .expect("spawn git");239++    assert!(st.success(), "git {args:?} failed");240++}241++242++fn fresh_dir(prefix: &str) -> PathBuf {243++    let dir = std::env::temp_dir().join(format!(244++        "hu13-paths-{prefix}-{}-{}",245++        std::process::id(),246++        std::time::SystemTime::now()247++            .duration_since(std::time::UNIX_EPOCH)248++            .map(|d| d.as_nanos())249++            .unwrap_or(0)250++    ));251++    std::fs::create_dir_all(&dir).unwrap();252++    dir253++}254++255++fn write(repo: &Path, rel: &str, content: &str) {256++    let dest = repo.join(rel);257++    std::fs::create_dir_all(dest.parent().expect("parent")).unwrap();258++    std::fs::write(&dest, content).unwrap();259++}260++261++#[test]262++fn hostile_frontmatter_cannot_write_outside_out() {263++    let repo = fresh_dir("repo");264++    let out = fresh_dir("out");265++    let marker = format!(266++        "hu13-pwn-{}-{}",267++        std::process::id(),268++        std::time::SystemTime::now()269++            .duration_since(std::time::UNIX_EPOCH)270++            .map(|d| d.as_nanos())271++            .unwrap_or(0)272++    );273++    let escaped = |name: &str| PathBuf::from(format!("/tmp/{marker}-{name}.html"));274++275++    git(&repo, &["init", "-b", "main"]);276++    git(&repo, &["config", "user.name", "hu13-test"]);277++    git(&repo, &["config", "user.email", "[email protected]"]);278++    git(&repo, &["config", "commit.gpgsign", "false"]);279++    write(&repo, "README.md", "# path safety\n");280++281++    // Issue whose id escapes 6 levels (out lives in /tmp, so /../../.. > /).282++    write(283++        &repo,284++        ".forge/issues/aaaaaaaa-1111-4111-8111-111111111111/issue.md",285++        &format!(286++            "---\nid: ../../../../../../tmp/{marker}-issue\ntitle: pwn\n\287++             state: open\ncreated: 2026-09-10T03:00:00Z\n---\nbody\n"288++        ),289++    );290++    // PR whose id escapes.291++    write(292++        &repo,293++        ".forge/prs/bbbbbbbb-2222-4222-8222-222222222222/pr.md",294++        &format!(295++            "---\nid: ../../../../../../tmp/{marker}-pr\ntitle: pwn\n\296++             state: open\nbase: main\nhead: x\ncreated: 2026-09-10T03:00:00Z\n---\nd\n"297++        ),298++    );299++    // Release whose version is absolute / traversal.300++    write(301++        &repo,302++        ".forge/releases/r1.md",303++        &format!(304++            "---\nversion: /tmp/{marker}-release\ntitle: r1\n\305++             created: 2026-09-10T03:00:00Z\n---\nnotes\n"306++        ),307++    );308++    write(309++        &repo,310++        ".forge/releases/r2.md",311++        "---\nversion: ../../../../../../tmp/{marker}-dotdot\ntitle: r2\n\312++         created: 2026-09-10T03:00:00Z\n---\nnotes\n",313++    );314++    // A valid release still has to render.315++    write(316++        &repo,317++        ".forge/releases/r3.md",318++        "---\nversion: v1.0.0\ntitle: good\ncreated: 2026-09-10T04:00:00Z\n---\nnotes\n",319++    );320++321++    git(&repo, &["add", "-A"]);322++    git(&repo, &["commit", "-m", "hostile fixture"]);323++324++    let produced = Command::new(env!("CARGO_BIN_EXE_hu13"))325++        .arg("build")326++        .arg(&repo)327++        .arg("-o")328++        .arg(&out)329++        .output()330++        .expect("spawn hu13 build");331++    let stderr = String::from_utf8_lossy(&produced.stderr);332++    assert!(333++        produced.status.success(),334++        "hostile frontmatter must not fail the build: {stderr}"335++    );336++337++    for name in ["issue", "pr", "release", "dotdot"] {338++        let p = escaped(name);339++        assert!(340++            !p.exists(),341++            "build wrote outside out/: {} exists",342++            p.display()343++        );344++    }345++    // Nothing landed at /tmp directly either (traversal is blocked, not346++    // merely clamped).347++    assert!(!Path::new(&format!("/tmp/{marker}-issue.html")).exists());348++349++    // Refused entries are warned about, naming the bad value.350++    assert!(stderr.contains("unsafe id"), "stderr: {stderr}");351++    assert!(stderr.contains("unsafe version"), "stderr: {stderr}");352++353++    // Valid content survives.354++    assert!(out.join("releases/v1.0.0.html").is_file());355++    // Build still completes with an output tree.356++    assert!(out.join("index.html").is_file());357++}358+-- 359+2.56.0360+
.forge/prs/44ea71b1-a7ab-408b-8618-92b4dbbe0e85/pr.md+13 −0
diff --git a/.forge/prs/44ea71b1-a7ab-408b-8618-92b4dbbe0e85/pr.md b/.forge/prs/44ea71b1-a7ab-408b-8618-92b4dbbe0e85/pr.mdnew file mode 100644index 0000000..22517be--- /dev/null+++ b/.forge/prs/44ea71b1-a7ab-408b-8618-92b4dbbe0e85/pr.md@@ -0,0 +1,13 @@1+---2+id: 44ea71b1-a7ab-408b-8618-92b4dbbe0e853+title: Refuse forge frontmatter paths that escape out/ (#60)4+state: merged5+base: master6+head: issue-60-path-traversal7+draft: false8+issues:9+- 0c944797-1025-43bb-a382-686669c85c6910+ci: pass11+created: 2026-10-27T15:33:27.908708853Z12+updated: 2026-10-27T15:34:02.513214394Z13+---