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 <noreply@anthropic.com>

---------

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 <noreply@anthropic.com>

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<u8> {
+    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}");
+        }
+    }
+}

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;

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"
+  }
+]

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",

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(&quoted[..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<String, BTreeSet<String>>, 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<BTreeMap<String, BTreeSet<String>>> {
+    let mut graph: BTreeMap<String, BTreeSet<String>> = BTreeMap::new();
+    let mut current: Option<String> = 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<String, BTreeSet<String>>, from: &str, to: &str) -> anyhow::Result<Option<Vec<String>>> {
+    let mut parents: BTreeMap<String, String> = BTreeMap::new();
+    let mut queue: VecDeque<String> = 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<String, String>, node: String) -> Vec<String> {
+    let mut chain = vec![node.clone()];
+    let mut current = node;
+    while let Some(parent) = parents.get(&current) {
+        chain.push(parent.clone());
+        current = parent.clone();
+    }
+    chain.reverse();
+    chain
+}
+
+/// One row per package and network-capable crate.
+fn rows(graph: &BTreeMap<String, BTreeSet<String>>) -> anyhow::Result<Vec<Row>> {
+    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<Vec<Row>> {
+    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<String> = 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<String> = 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");
+    }
+}