From e7668ae53e2abb16fdeded8b4cf363c9038b99d7 Mon Sep 17 00:00:00 2001 From: rabis7575 <340973947+rabis7575@users.noreply.github.com> Date: Sun, 8 Nov 2026 01:48:57 +0100 Subject: [PATCH] Tests: run the file commands through the hostile-parameter test (#3719) (#5688) * #3719: run the file commands through the hostile-parameter test hostile_params_never_panic skips file.*, insert.picture and insert.textFromFile, the commands that take a path and parse what is there. hostile_file_params_never_panic runs them with the same junk params, and with paths to an empty file, a folder, a missing name and noise under each extension they open or save. Automation roots hold them to a temporary folder. It found no panic, so this adds only the test. Co-Authored-By: Claude Opus 5.5 * #5688: check the whole working directory, on the sample document The working-directory check looked only for hostile-relative names, but an empty path comes out as .pdf or .dotx there. It now compares the whole folder before and after the runs. The runs now use the sample document, like hostile_params_never_panic: on an empty document the export commands had almost nothing to write. The docs and comments name the three groups of commands, because other commands also read files. Co-Authored-By: Claude Opus 5.5 * #5688: a failing file check names the files, not their bytes Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- crates/engine/src/tests.rs | 147 +++++++++++++++++++++++++++++++++++-- docs/architecture.md | 8 +- 2 files changed, 144 insertions(+), 11 deletions(-) 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 |