diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index e13cf934b7..316300fca9 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -4681,10 +4681,8 @@ fn typed_paragraphs_are_capped_for_every_caller() { assert!(!body_texts(&s).iter().any(|p| p.contains('y'))); } -#[test] -fn hostile_params_never_panic() { - let reg = cmd::registry(); - let junk = [ +fn hostile_junk() -> [serde_json::Value; 8] { + [ json!(null), json!({}), json!({"text": 5, "size": "x", "path": [], "pos": {"block": 9999, "off": 99999}}), @@ -4693,11 +4691,23 @@ fn hostile_params_never_panic() { json!({"page": 0, "x": 1e30, "y": -1e30, "extend": 1}), json!({"width": 1e-9, "height": 1e30, "top": -1e9, "left": 1e9}), json!({"section": {"pageW": -1e308, "pageH": 0, "marginTop": 1e30, "header": -5, "columns": {"count": 1000000, "space": -1e300, "widths": [[0, 0]]}}}), - ]; + ] +} + +/// `file.*`, `insert.picture` and `insert.textFromFile`: `hostile_file_params_never_panic` runs +/// them, held to a folder of its own, and `hostile_params_never_panic` every other command. +fn reaches_files(id: &str) -> bool { + id.starts_with("file.") || id == "insert.picture" || id == "insert.textFromFile" +} + +#[test] +fn hostile_params_never_panic() { + let reg = cmd::registry(); + let junk = hostile_junk(); for spec in reg.all() { - // These reach outside the session (files). Read Aloud (`review.readAloud`, `readAloud.*`) - // is fuzzed too: under `cfg(test)` its backend is the silent `Hold`, asserted below. - if spec.id.starts_with("file.") || spec.id == "insert.picture" || spec.id == "insert.textFromFile" { + // Read Aloud (`review.readAloud`, `readAloud.*`) is fuzzed too: under `cfg(test)` its + // backend is the silent `Hold`, asserted below. + if reaches_files(spec.id) { continue; } for j in &junk { @@ -4711,6 +4721,127 @@ fn hostile_params_never_panic() { } } +/// `file.*`, `insert.picture` and `insert.textFromFile` take junk params and paths to files they +/// can't use (#3719). They may read only `in` and write only `out`, so a save can't replace a file +/// another run is meant to find broken, and nothing outside those folders is touched. +#[cfg(not(target_arch = "wasm32"))] +#[test] +fn hostile_file_params_never_panic() { + /// What is directly in `dir`: each name with its bytes (`None` for a folder). + fn contents(dir: &std::path::Path) -> std::collections::BTreeMap>> { + let entries = std::fs::read_dir(dir).unwrap().map(|e| e.unwrap().path()); + entries.map(|p| (p.file_name().unwrap().to_string_lossy().into_owned(), std::fs::read(&p).ok())).collect() + } + /// The names whose bytes differ, so a failure lists them instead of printing every file. + fn changed(before: &std::collections::BTreeMap>>, dir: &std::path::Path) -> Vec { + let after = contents(dir); + let names: std::collections::BTreeSet<_> = before.keys().chain(after.keys()).collect(); + names.into_iter().filter(|n| before.get(*n) != after.get(*n)).cloned().collect() + } + let reg = cmd::registry(); + let base = scratch_dir("hostile-files"); + let (input, output, outside) = (base.join("in"), base.join("out"), base.join("outside")); + for dir in [&outside, &input.join("folder.docx"), &output.join("folder.docx")] { + std::fs::create_dir_all(dir).unwrap(); + } + let at = |dir: &std::path::Path, name: &str| dir.join(name).to_string_lossy().into_owned(); + + // Fixed bytes, so a failure comes back on the next run. + let noise: Vec = (0..512u32).map(|i| (i.wrapping_mul(2_654_435_761) >> 24) as u8).collect(); + let picture_exts = ["png", "jpg", "gif", "bmp", "svg", "emf"]; + let mut paths = vec![]; + for ext in crate::io::OPEN_EXTS.iter().chain(&picture_exts) { + let name = format!("random.{ext}"); + std::fs::write(input.join(&name), &noise).unwrap(); + paths.push(at(&input, &name)); + } + paths.extend(crate::io::SAVE_EXTS.iter().map(|ext| at(&output, &format!("new.{ext}")))); + std::fs::write(input.join("empty"), b"").unwrap(); + std::fs::write(input.join("empty.docx"), b"").unwrap(); + std::fs::write(input.join("good.txt"), "Good text").unwrap(); + std::fs::write(input.join("good.png"), tiny_png()).unwrap(); + paths.extend(["empty", "empty.docx", "folder.docx", "missing", "missing.docx"].map(|name| at(&input, name))); + paths.extend(["folder.docx", "missing/new.docx", &format!("{}.docx", "x".repeat(5000))].map(|name| at(&output, name))); + // Outside the roots, however it is spelled, and not paths at all. + paths.extend(["new.docx", "new"].map(|name| at(&outside, name))); + paths.push(format!("{}/../outside/new.docx", input.to_string_lossy())); + paths.extend(["hostile-relative.docx", "hostile-relative", "", "\0", "a\0b.docx"].map(String::from)); + + // Every path alone; each of these also with every other kind of junk param. The numbers that + // cost time (`copies`, `scale`) stay small: huge ones are #3720. + let mut params: Vec = hostile_junk().into(); + params.extend(paths.iter().map(|p| json!({"path": p}))); + for name in [ + "in/random.docx", + "in/empty.docx", + "in/folder.docx", + "in/missing.docx", + "in/good.txt", + "in/good.png", + "out/new.docx", + "out/new.pdf", + "out/new.png", + ] { + let path = at(&base, name); + params.extend([ + json!({"path": path, "password": "x", "format": "wordml2003", "markup": "x", "confirmDamaged": 1}), + json!({"path": path, "password": 5, "format": 5, "markup": true}), + json!({"path": path, "page": 0, "scale": 0, "width": -1e308, "alt": 5}), + json!({"path": path, "data": "not base64!"}), + json!({"path": path, "data": "AAAA"}), + json!({"path": path, "pagesPerSheet": 7, "range": "x", "pages": "9-1,,", "parity": "x", "copies": 3, "paper": "x"}), + json!({"dir": path, "name": path, "template": path, "userTemplatesFolder": path, "id": 1e30, "restore": 1e30, "save": path}), + ]); + } + + let before = contents(&input); + let working_dir = std::path::Path::new("."); + let working_dir_before = contents(working_dir); + let roots = crate::file_access::AutomationRoots::new(Some(&input), Some(&output)).unwrap(); + let confined = |s: &mut Session, id: &str, v: &serde_json::Value| crate::file_access::confine(roots.as_ref(), || s.run(id, v)); + for spec in reg.all() { + if !reaches_files(spec.id) { + continue; + } + for v in ¶ms { + let mut s = Session::new(crate::sample::sample_document()); + let r = confined(&mut s, spec.id, v); + assert!(!caught_panic(®, &r), "{} panicked on {v}: {r:?}", spec.id); + s.clamp_selection(); + let _ = s.layout(); + } + } + + let nothing = Vec::::new(); + assert_eq!(changed(&before, &input), nothing, "the read root is only read"); + assert_eq!(changed(&Default::default(), &outside), nothing, "nothing outside the write root"); + // Every name counts: an empty path comes out as `.pdf` or `.dotx` here. + assert_eq!(changed(&working_dir_before, working_dir), nothing, "nothing in the working directory"); + + // The junk above met the files, not a closed door: usable files work through the same roots... + let mut s = s(); + confined(&mut s, "file.open", &json!({"path": at(&input, "good.txt")})).unwrap(); + assert_eq!(s.doc.plain_text(StoryRef::Body), "Good text"); + confined(&mut s, "insert.textFromFile", &json!({"path": at(&input, "good.txt")})).unwrap(); + confined(&mut s, "insert.picture", &json!({"path": at(&input, "good.png")})).unwrap(); + let saved: [(&str, &str, &[u8]); 3] = + [("file.saveAs", "control.docx", b"PK\x03\x04"), ("file.exportPdf", "control.pdf", b"%PDF-"), ("file.exportPng", "control.png", b"\x89PNG")]; + for (id, name, signature) in saved { + confined(&mut s, id, &json!({"path": at(&output, name)})).unwrap(); + assert_eq!(std::fs::read(output.join(name)).unwrap().get(..signature.len()), Some(signature), "{name}"); + } + // ...the noise is refused by the reader, and only a path outside by the roots. + let refused = |s: &mut Session, path: String| match confined(s, "file.open", &json!({"path": path})) { + Err(crate::CmdError::Failed(m)) => m, + r => panic!("not refused: {r:?}"), + }; + let unreadable = refused(&mut s, at(&input, "random.docx")); + assert!(!unreadable.contains("automation path rejected"), "{unreadable}"); + let rejected = refused(&mut s, at(&outside, "new.docx")); + assert!(rejected.contains("automation path rejected"), "{rejected}"); + let _ = std::fs::remove_dir_all(&base); +} + /// A panic that `Session::run`'s guard turned into an error, in a command or one it ran: still a /// crash path (#3718). Only the guard's own `"{id} panicked: "` counts, not an error that echoes /// those words. diff --git a/docs/architecture.md b/docs/architecture.md index b937424d48..e22d36a891 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -1,6 +1,6 @@ # WordCraft architecture -> **Last reviewed:** 2026-10-28 · **Last updated:** 2026-11-02 · **Change:** trivial (craft-proof's row says what a build without the Language data keeps: English spelling from an installed Hunspell English or the built-in list, rule hyphenation for Bulgarian, Turkish and Russian; previously minor: proofing moved out: `wordcraft-proof` is craft-libs' `craft-proof`, its Language data craft-i18n's `language-data/` embedded from `CRAFT_I18N_DIR`; previously major: crate and layer inventory, source counts, proofing languages and module boundaries re-measured from this checkout; previously minor: `proof` is per language: English and Polish; previously major: first committed version; `plan/architecture.md` is local-only) · **Target:** Microsoft Word (Microsoft 365) for Mac 16.113.4 +> **Last reviewed:** 2026-10-28 · **Last updated:** 2026-11-05 · **Change:** trivial (the hostile-parameter test now has a sibling for `file.*`, `insert.picture` and `insert.textFromFile`; previously trivial: craft-proof's row says what a build without the Language data keeps: English spelling from an installed Hunspell English or the built-in list, rule hyphenation for Bulgarian, Turkish and Russian; previously minor: proofing moved out: `wordcraft-proof` is craft-libs' `craft-proof`, its Language data craft-i18n's `language-data/` embedded from `CRAFT_I18N_DIR`; previously major: crate and layer inventory, source counts, proofing languages and module boundaries re-measured from this checkout; previously minor: `proof` is per language: English and Polish; previously major: first committed version; `plan/architecture.md` is local-only) · **Target:** Microsoft Word (Microsoft 365) for Mac 16.113.4 How WordCraft is built today. The longer design notes in `plan/architecture.md` are gitignored and local to the original author's machine; this file is the @@ -57,8 +57,9 @@ crate name links to its source tree. - 821 `#[test]` functions and 10 property-test blocks: model, layout, DOCX round-trip and hostile input, `.doc`, formats, typing parity (`engine/src/tests/typing.rs`), bidi, i18n catalogs, UI - interaction (egui_kittest), MCP acceptance; `hostile_params_never_panic` runs every command with - junk params. + interaction (egui_kittest), MCP acceptance; `hostile_file_params_never_panic` runs `file.*`, + `insert.picture` and `insert.textFromFile` with junk params and unusable paths, held to a + temporary folder, and `hostile_params_never_panic` every other command with junk params. - `cargo xtask ci`: fmt, clippy `-D warnings`, tests, asset attribution, layering, wasm build. - `cargo xtask parity` regenerates [`parity-checklist.md`](parity-checklist.md); a test keeps the catalog percentage above a floor that only rises. @@ -69,6 +70,7 @@ crate name links to its source tree. | Date | Change | Summary | |---|---|---| +| 2026-11-05 | trivial | `hostile_file_params_never_panic` runs `file.*`, `insert.picture` and `insert.textFromFile` with junk params and unusable paths (#3719) | | 2026-11-02 | trivial | craft-proof's row: without the Language data English spelling falls back to an installed Hunspell English or the built-in Moby list, and only pattern hyphenation is off (#5202 review) | | 2026-11-01 | minor | `proof` left the workspace: it is `craft-proof` in storytold/craft-libs (with its history), and the spelling and hyphenation data is craft-i18n's `language-data/` (`CRAFT_I18N_DIR`); `assets/spelling` and `assets/hyphenation` are gone | | 2026-10-28 | major | Recounted workspace crates, layers and Rust source lines; refreshed proofing languages and documented current engine/layout module boundaries and source links |