From f16b5d9331da5583fa9abdb973010f358aa5c60f Mon Sep 17 00:00:00 2001 From: eliets2 Date: Thu, 5 Nov 2026 21:52:06 +0200 Subject: [PATCH] M13 batch: test that results hold no planted secret, and record network-crate reachability (#6297) * M13 batch: test that results hold no planted secret, and record network-crate reachability * netcheck: list socket2, report a walk past the cap, test by the gate rule socket2 can open sockets, so it belongs in the list. A walk past the cap now fails instead of reading as unreachable. The baseline test follows check_rows, so a dependency update that moves a path no longer fails cargo test. Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: eliets2 <287273517+eliets2@users.noreply.github.com> Co-authored-by: FredApps <153173854+FredApps@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 --- crates/automation/tests/it/batch_secrets.rs | 84 +++++ crates/automation/tests/it/main.rs | 1 + xtask/baselines/network-crates.json | 74 +++++ xtask/src/main.rs | 6 + xtask/src/netcheck.rs | 344 ++++++++++++++++++++ 5 files changed, 509 insertions(+) create mode 100644 crates/automation/tests/it/batch_secrets.rs create mode 100644 xtask/baselines/network-crates.json create mode 100644 xtask/src/netcheck.rs diff --git a/crates/automation/tests/it/batch_secrets.rs b/crates/automation/tests/it/batch_secrets.rs new file mode 100644 index 0000000000..08223f0955 --- /dev/null +++ b/crates/automation/tests/it/batch_secrets.rs @@ -0,0 +1,84 @@ +//! Tool results and errors from the folder tools carry no known test password. +//! +//! The passwords are set on the document and passed to the tool, and must never come back out: a +//! result that echoes one has leaked it into logs, transcripts and MCP callers. + +use pdfcraft_automation::{Automation, Content}; +use pdfcraft_engine::{Changes, Edit, Protection, Session}; +use serde_json::{Value, json}; + +/// The known test passwords; neither may appear in a result or an error. +const OPEN_PASSWORD: &str = "PLANT-OPEN-5f2c9a"; +const OWNER_PASSWORD: &str = "PLANT-OWNER-71de0b"; + +/// A one-page PDF locked with the known test passwords. +/// +/// The open password is the only way in. The batch bates tool rewrites page content, and +/// `Changes::None` forbids that, so this fixture permits every change except extracting pages; +/// the restrictions stay real, since the open password never lifts them. +fn protected_pdf() -> Vec { + let mut session = Session::new(); + let bytes = session.create_from_text("Batch secrets", "Planted password test").unwrap(); + let id = session.open("locked.pdf", None, bytes, None).unwrap(); + session + .apply( + id, + Edit::Protect(Protection { + open_password: Some(OPEN_PASSWORD.into()), + permissions_password: Some(OWNER_PASSWORD.into()), + changes: Changes::AnyExceptExtract, + ..Default::default() + }), + ) + .unwrap(); + session.save_bytes(id).unwrap().as_ref().clone() +} + +fn json_result(a: &mut Automation, tool: &str, args: Value) -> Value { + match a.call(tool, &args) { + Ok(mut content) => match content.remove(0) { + Content::Json(value) => value, + other => panic!("{tool}: expected JSON, got {other:?}"), + }, + Err(error) => panic!("{tool} {args}: {error}"), + } +} + +#[test] +fn results_and_errors_hold_no_planted_password_for_bates_or_batch() { + let dir = tempfile::tempdir().unwrap(); + std::fs::write(dir.path().join("locked.pdf"), protected_pdf()).unwrap(); + let mut a = Automation::new().with_root(dir.path()).unwrap(); + + // The right open password: numbering succeeds and reports the numbers only. + let numbered = + json_result(&mut a, "doc_bates_multi", json!({"files":[{"input":"locked.pdf","output":"locked-numbered.pdf","password":OPEN_PASSWORD}]})); + assert_eq!(numbered["completed"], 1, "{numbered}"); + assert_eq!(numbered["files"][0]["status"], "ok", "{numbered}"); + // The report carries the per-file entry the Bates tool builds, so a field added to that entry + // lands in the text checked below. + assert_eq!(numbered["files"][0]["first"], 1, "{numbered}"); + assert!(numbered["files"][0]["output"].as_str().is_some_and(|path| path.ends_with("locked-numbered.pdf")), "{numbered}"); + assert!(dir.path().join("locked-numbered.pdf").exists(), "the numbered copy is written"); + + // The wrong open password: the tool refuses the file with an error. + let refused = match a + .call("doc_bates_multi", &json!({"files":[{"input":"locked.pdf","output":"locked-refused.pdf","password":"not-the-open-password"}]})) + { + Err(error) => error.to_string(), + Ok(value) => panic!("a wrong open password must be refused: {value:?}"), + }; + + // The folder job refuses the encrypted file without a password, in its per-file report. + let batch = json_result(&mut a, "doc_batch", json!({"paths":["locked.pdf"],"steps":["optimize"],"out_dir":"out"})); + assert_eq!(batch["failed"], 1, "{batch}"); + assert_eq!(batch["files"][0]["status"], "error", "{batch}"); + assert!(batch["files"][0]["error"].as_str().is_some_and(|text| !text.is_empty()), "{batch}"); + assert!(!dir.path().join("out/locked.pdf").exists(), "a refused file publishes nothing"); + + for report in [serde_json::to_string(&numbered).unwrap(), refused, serde_json::to_string(&batch).unwrap()] { + for secret in [OPEN_PASSWORD, OWNER_PASSWORD] { + assert!(!report.contains(secret), "a result or error echoed a planted password: {report}"); + } + } +} diff --git a/crates/automation/tests/it/main.rs b/crates/automation/tests/it/main.rs index a3cd55623c..13f24013cd 100644 --- a/crates/automation/tests/it/main.rs +++ b/crates/automation/tests/it/main.rs @@ -7,6 +7,7 @@ mod automation_roots; mod backdrop; mod batch; mod batch_failures; +mod batch_secrets; mod button_caption; mod form_bulk; mod hostile_args; diff --git a/xtask/baselines/network-crates.json b/xtask/baselines/network-crates.json new file mode 100644 index 0000000000..13f24c43a7 --- /dev/null +++ b/xtask/baselines/network-crates.json @@ -0,0 +1,74 @@ +[ + { + "crate": "pdfcraft-cli", + "name": "hyper", + "reachable": true, + "path": "pdfcraft-cli -> pdfcraft-engine -> pdfcraft-print -> craft-print -> winprint -> reqwest -> hyper" + }, + { + "crate": "pdfcraft-cli", + "name": "reqwest", + "reachable": true, + "path": "pdfcraft-cli -> pdfcraft-engine -> pdfcraft-print -> craft-print -> winprint -> reqwest" + }, + { + "crate": "pdfcraft-cli", + "name": "socket2", + "reachable": true, + "path": "pdfcraft-cli -> pdfcraft-engine -> pdfcraft-print -> craft-print -> winprint -> reqwest -> hyper-util -> socket2" + }, + { + "crate": "pdfcraft-cli", + "name": "tokio", + "reachable": true, + "path": "pdfcraft-cli -> pdfcraft-engine -> pdfcraft-print -> craft-print -> winprint -> reqwest -> tokio" + }, + { + "crate": "pdfcraft-automation", + "name": "hyper", + "reachable": true, + "path": "pdfcraft-automation -> pdfcraft-engine -> pdfcraft-print -> craft-print -> winprint -> reqwest -> hyper" + }, + { + "crate": "pdfcraft-automation", + "name": "reqwest", + "reachable": true, + "path": "pdfcraft-automation -> pdfcraft-engine -> pdfcraft-print -> craft-print -> winprint -> reqwest" + }, + { + "crate": "pdfcraft-automation", + "name": "socket2", + "reachable": true, + "path": "pdfcraft-automation -> pdfcraft-engine -> pdfcraft-print -> craft-print -> winprint -> reqwest -> hyper-util -> socket2" + }, + { + "crate": "pdfcraft-automation", + "name": "tokio", + "reachable": true, + "path": "pdfcraft-automation -> pdfcraft-engine -> pdfcraft-print -> craft-print -> winprint -> reqwest -> tokio" + }, + { + "crate": "pdfcraft-engine", + "name": "hyper", + "reachable": true, + "path": "pdfcraft-engine -> pdfcraft-print -> craft-print -> winprint -> reqwest -> hyper" + }, + { + "crate": "pdfcraft-engine", + "name": "reqwest", + "reachable": true, + "path": "pdfcraft-engine -> pdfcraft-print -> craft-print -> winprint -> reqwest" + }, + { + "crate": "pdfcraft-engine", + "name": "socket2", + "reachable": true, + "path": "pdfcraft-engine -> pdfcraft-print -> craft-print -> winprint -> reqwest -> hyper-util -> socket2" + }, + { + "crate": "pdfcraft-engine", + "name": "tokio", + "reachable": true, + "path": "pdfcraft-engine -> pdfcraft-print -> craft-print -> winprint -> reqwest -> tokio" + } +] diff --git a/xtask/src/main.rs b/xtask/src/main.rs index 8e3255d5e6..01beb95ecf 100644 --- a/xtask/src/main.rs +++ b/xtask/src/main.rs @@ -10,6 +10,7 @@ mod gates; mod layers; mod license_check; mod macos_integration; +mod netcheck; mod parity; mod screenshots; mod version; @@ -35,6 +36,11 @@ const COMMANDS: &[(&str, &str, Command)] = &[ ("check", "Robustness sweep over corpus/ with pdfcraft-cli; fails on crashes or regressions vs xtask/baselines", gates::check), ("fuzz", "Mutation fuzzing of open/render/edit/save in child processes; findings in fuzz-out/ (--time 300)", fuzz::run), ("parity", "Validate parity/acrobat-features.toml against the registry, tools and tests; report progress (--json, --partial)", parity::run), + ( + "netcheck", + "Record which network-capable crates the batch crates can reach in Cargo.lock, and fail on any reachable pair not in xtask/baselines/network-crates.json", + netcheck::run, + ), ( "macos-integration", "Report macOS bundle, Quick Look, Spotlight and PDF integration diagnostics without mutating system state", diff --git a/xtask/src/netcheck.rs b/xtask/src/netcheck.rs new file mode 100644 index 0000000000..0d29a461f1 --- /dev/null +++ b/xtask/src/netcheck.rs @@ -0,0 +1,344 @@ +//! `cargo xtask netcheck`: which network-capable crates the batch path can reach. +//! +//! The batch folder job runs `pdfcraft-cli`, which drives `pdfcraft-automation` over +//! `pdfcraft-engine`; none of them may open a socket. That promise is only as strong as the +//! dependency graph behind those three packages, so this gate walks the resolved graph recorded +//! in `Cargo.lock` and asks, for each of them and each crate that can open a socket, whether that +//! crate is reachable from it. The answer is compared with a reviewed baseline in +//! `xtask/baselines/network-crates.json`: a reachable pair the baseline does not list fails. +//! +//! A lockfile records the resolved dependencies of a package without their kind and without a +//! target: normal, build and development edges look the same, and a dependency only one platform +//! uses is listed beside the rest. The answer here is therefore a superset of what a normal-level, +//! per-target walk would give. It may name a crate that is only a build or a development +//! dependency, or one that only another platform uses; it does not miss one, so it errs toward +//! failing and never toward passing. `--update-baseline` rewrites the baseline; review that diff +//! line by line. +//! +//! The walk is in process, over the lockfile text: it starts no other program and reads no +//! network. This gate is local on purpose, not part of `cargo xtask ci`, which would be a +//! workflow change. + +use std::collections::{BTreeMap, BTreeSet, VecDeque}; +use std::path::Path; + +use anyhow::{Context, bail}; +use serde::{Deserialize, Serialize}; + +use crate::gates::root; + +/// The packages the batch path is built from. +const PACKAGES: &[&str] = &["pdfcraft-cli", "pdfcraft-automation", "pdfcraft-engine"]; + +/// Crates that can open a socket: HTTP clients, TLS stacks, async runtimes, socket wrappers and +/// libcurl bindings. +const NETWORK_CRATES: &[&str] = &["curl", "hyper", "isahc", "minreq", "native-tls", "reqwest", "socket2", "tokio", "ureq"]; + +/// The resolved dependency graph of the workspace, relative to the repository root. +const LOCKFILE: &str = "Cargo.lock"; + +/// The reviewed baseline, relative to the repository root. +const BASELINE: &str = "xtask/baselines/network-crates.json"; + +/// The most packages one walk will visit. A lock file has a few hundred packages; the cap stops a +/// damaged file from turning the walk into a long loop. +const MAX_VISITED: usize = 500_000; + +/// What the lock file says about one package and one network-capable crate. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +struct Row { + /// The package whose dependencies were walked. + #[serde(rename = "crate")] + package: String, + /// The network-capable crate asked about. + name: String, + /// Whether the package reaches the crate in the lock file graph. + reachable: bool, + /// The chain from the package to the crate, ` -> ` separated; empty when unreachable. + #[serde(default)] + path: String, +} + +/// The first word of a dependency entry: `"name"`, `"name 1.2.3"` and `"name 1.2.3 (source)"` +/// all name the same package. +fn entry_name(entry: &str) -> &str { + entry.split_whitespace().next().unwrap_or_default() +} + +/// The package a dependency line names, when the line is one. +fn dependency_name(line: &str) -> Option<&str> { + let quoted = line.strip_prefix('"')?; + let end = quoted.find('"')?; + let name = entry_name("ed[..end]); + (!name.is_empty()).then_some(name) +} + +/// The names in a `dependencies = ["a", "b"]` line, that is, the text between its brackets. +fn inline_names(value: &str) -> Vec<&str> { + value.split(',').filter_map(|item| dependency_name(item.trim())).collect() +} + +/// One edge of the graph: `package` depends on `dependency`. +fn add_edge(graph: &mut BTreeMap>, package: &str, dependency: &str) { + graph.entry(package.to_owned()).or_default().insert(dependency.to_owned()); +} + +/// The dependency graph recorded by a lock file, keyed by package name. +/// +/// Edges are by name: a lock file can hold two versions of one crate, and the walk only asks +/// whether a name can be reached at all. A file that names no package is an error rather than an +/// empty graph, so a format change fails loudly instead of reporting nothing reachable. +fn parse_lockfile(text: &str) -> anyhow::Result>> { + let mut graph: BTreeMap> = BTreeMap::new(); + let mut current: Option = None; + let mut in_list = false; + for line in text.lines() { + let line = line.trim(); + if line.is_empty() || line.starts_with('#') { + continue; + } + if in_list { + if line.starts_with(']') { + in_list = false; + } else if let Some(name) = dependency_name(line) + && let Some(package) = current.as_deref() + { + add_edge(&mut graph, package, name); + } + continue; + } + if line.starts_with("[[") { + current = None; + continue; + } + if let Some(name) = line.strip_prefix("name = \"").and_then(|rest| rest.strip_suffix('"')) { + graph.entry(name.to_owned()).or_default(); + current = Some(name.to_owned()); + continue; + } + if let Some(value) = line.strip_prefix("dependencies = [") { + match value.strip_suffix(']') { + Some(inner) => { + for name in inline_names(inner) { + if let Some(package) = current.as_deref() { + add_edge(&mut graph, package, name); + } + } + } + None => in_list = true, + } + } + } + if graph.is_empty() { + bail!("{LOCKFILE} names no package; the lock file format changed or the file is empty"); + } + Ok(graph) +} + +/// The shortest chain from `from` to `to` in the graph, both ends included. A walk past +/// [`MAX_VISITED`] is an error: it has no answer, and "unreachable" would pass the gate. +fn shortest_path(graph: &BTreeMap>, from: &str, to: &str) -> anyhow::Result>> { + let mut parents: BTreeMap = BTreeMap::new(); + let mut queue: VecDeque = VecDeque::from([from.to_owned()]); + let mut visited = 0usize; + while let Some(node) = queue.pop_front() { + visited = visited.saturating_add(1); + if visited > MAX_VISITED { + bail!("walking from {from} to {to} visited more than {MAX_VISITED} packages; is {LOCKFILE} damaged?"); + } + if node == to { + return Ok(Some(walk_back(&parents, node))); + } + let Some(next) = graph.get(&node) else { continue }; + for dependency in next { + if dependency != from && !parents.contains_key(dependency.as_str()) { + parents.insert(dependency.clone(), node.clone()); + queue.push_back(dependency.clone()); + } + } + } + Ok(None) +} + +/// The chain from the start of the walk to `node`, following the discovery parents. +fn walk_back(parents: &BTreeMap, node: String) -> Vec { + let mut chain = vec![node.clone()]; + let mut current = node; + while let Some(parent) = parents.get(¤t) { + chain.push(parent.clone()); + current = parent.clone(); + } + chain.reverse(); + chain +} + +/// One row per package and network-capable crate. +fn rows(graph: &BTreeMap>) -> anyhow::Result> { + let mut rows = Vec::with_capacity(PACKAGES.len().saturating_mul(NETWORK_CRATES.len())); + for package in PACKAGES { + for name in NETWORK_CRATES { + let path = shortest_path(graph, package, name)?; + let path = path.map(|chain| chain.join(" -> ")); + let reachable = path.is_some(); + let row = Row { package: (*package).to_owned(), name: (*name).to_owned(), reachable, path: path.unwrap_or_default() }; + rows.push(row); + } + } + Ok(rows) +} + +/// Write the reviewed baseline: the reachable pairs only, so a reader sees exactly the accepted +/// answers and nothing else. +fn write_baseline(path: &Path, rows: &[Row]) -> anyhow::Result<()> { + let reachable: Vec<&Row> = rows.iter().filter(|r| r.reachable).collect(); + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent).with_context(|| format!("creating {}", parent.display()))?; + } + let text = serde_json::to_string_pretty(&reachable)? + "\n"; + std::fs::write(path, text).with_context(|| format!("writing {}", path.display()))?; + println!("baseline updated: {} rows in {BASELINE}", reachable.len()); + Ok(()) +} + +fn read_baseline(path: &Path) -> anyhow::Result> { + let text = + std::fs::read_to_string(path).with_context(|| format!("{} is missing: run `cargo xtask netcheck --update-baseline`", path.display()))?; + serde_json::from_str(&text).with_context(|| format!("parsing {}", path.display())) +} + +/// The reachable pairs of a row list, for set comparison. +fn pairs(rows: &[Row]) -> BTreeSet<(&str, &str)> { + rows.iter().filter(|r| r.reachable).map(|r| (r.package.as_str(), r.name.as_str())).collect() +} + +/// Compare the current answers with the reviewed baseline: a reachable pair the baseline does not +/// list fails, and a pair that stopped being reachable or moved to another path is reported for +/// the next baseline update. +fn check_rows(rows: &[Row], reviewed: &[Row]) -> anyhow::Result<()> { + let listed = pairs(reviewed); + let found = pairs(rows); + for row in rows.iter().filter(|r| r.reachable) { + if let Some(before) = reviewed.iter().find(|b| b.reachable && b.package == row.package && b.name == row.name) + && before.path != row.path + { + println!("path changed for {} -> {}: {} (was {})", row.package, row.name, row.path, before.path); + } + } + let gone: Vec = listed.difference(&found).map(|(package, name)| format!("{package} -> {name}")).collect(); + if !gone.is_empty() { + println!("no longer reachable (run --update-baseline to record it): {}", gone.join("; ")); + } + let new: Vec = found.difference(&listed).map(|(package, name)| format!("{package} -> {name}")).collect(); + if new.is_empty() { + println!("OK: all {} reachable pairs are in the reviewed baseline", found.len()); + return Ok(()); + } + for pair in &new { + println!(" - {pair}"); + } + bail!("{} reachable network crate pair(s) are not in {BASELINE}; review them, then run `cargo xtask netcheck --update-baseline`", new.len()) +} + +pub fn run(args: &[String]) -> anyhow::Result<()> { + let update = args.iter().any(|a| a == "--update-baseline"); + let lockfile = root().join(LOCKFILE); + let text = std::fs::read_to_string(&lockfile).with_context(|| format!("reading {}", lockfile.display()))?; + let graph = parse_lockfile(&text)?; + if let Some(package) = PACKAGES.iter().find(|name| !graph.contains_key(**name)) { + bail!("{package} is not in {LOCKFILE}; is it still a workspace member?"); + } + let rows = rows(&graph)?; + let reachable = rows.iter().filter(|r| r.reachable).count(); + println!("netcheck: {} packages x {} network-capable crates; {reachable} reachable", PACKAGES.len(), NETWORK_CRATES.len()); + for row in rows.iter().filter(|r| r.reachable) { + println!(" {} -> {}: {}", row.package, row.name, row.path); + } + let path = root().join(BASELINE); + let reviewed = if update { + write_baseline(&path, &rows)?; + rows.clone() + } else { + read_baseline(&path)? + }; + check_rows(&rows, &reviewed) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// A lock file with a block list, an inline list, a versioned entry and a patch block. + const SAMPLE: &str = r#" +version = 4 + +[[package]] +name = "root" +version = "0.1.0" +dependencies = [ + "middle", + "serde 1.0.0", +] + +[[package]] +name = "middle" +version = "0.1.0" +source = "registry+https://example.invalid/index" +dependencies = [ + "leaf", +] + +[[package]] +name = "leaf" +version = "0.1.0" + +[[package]] +name = "empty" +version = "0.1.0" +dependencies = [] + +[[patch.unused]] +name = "spare" +version = "0.2.0" + +[[package]] +name = "tail" +version = "0.1.0" +dependencies = ["leaf"] +"#; + + #[test] + fn lockfile_dependency_lists_become_edges() { + let graph = parse_lockfile(SAMPLE).expect("the sample parses"); + assert_eq!(graph.get("root"), Some(&BTreeSet::from(["middle".to_string(), "serde".to_string()]))); + assert_eq!(graph.get("middle"), Some(&BTreeSet::from(["leaf".to_string()]))); + assert_eq!(graph.get("tail"), Some(&BTreeSet::from(["leaf".to_string()])), "an inline list is read too"); + assert_eq!(graph.get("empty"), Some(&BTreeSet::new())); + assert!(graph.contains_key("leaf"), "a package with no dependency list is still a node"); + assert!(graph.contains_key("spare"), "a patch block names a package too"); + } + + #[test] + fn shortest_path_names_the_chain_and_follows_dependencies_only() { + let graph = parse_lockfile(SAMPLE).expect("the sample parses"); + assert_eq!(shortest_path(&graph, "root", "leaf").unwrap(), Some(vec!["root".to_string(), "middle".to_string(), "leaf".to_string()])); + assert_eq!(shortest_path(&graph, "leaf", "root").unwrap(), None); + assert_eq!(shortest_path(&graph, "root", "nowhere").unwrap(), None); + assert_eq!(shortest_path(&graph, "root", "root").unwrap(), Some(vec!["root".to_string()])); + } + + #[test] + fn a_lockfile_without_packages_is_an_error() { + let error = parse_lockfile("version = 4\n").expect_err("an empty graph must not be read as an answer"); + assert!(error.to_string().contains("names no package"), "{error}"); + } + + #[test] + fn committed_baseline_accepts_the_reachable_pairs_of_the_lockfile_graph() { + let text = std::fs::read_to_string(root().join(LOCKFILE)).expect("Cargo.lock is committed"); + let graph = parse_lockfile(&text).expect("Cargo.lock parses"); + let found = rows(&graph).expect("the walk finishes"); + let reviewed = read_baseline(&root().join(BASELINE)).expect("the baseline is committed"); + // The gate's own rule: a pair that went away or moved is reported, not a failure. + check_rows(&found, &reviewed).expect("every reachable pair is in the baseline"); + } +}