diff --git a/crates/automation/src/rpc.rs b/crates/automation/src/rpc.rs index b98094eb5d..d85fae874c 100644 --- a/crates/automation/src/rpc.rs +++ b/crates/automation/src/rpc.rs @@ -771,6 +771,44 @@ mod tests { std::fs::remove_dir_all(base).unwrap(); } + /// A save or render never turns a file PhotoCraft doesn't write (source, settings, notes) + /// into image bytes, whatever `format` says; images and new files are written as before (#9439). + #[test] + fn workspace_writes_refuse_to_replace_a_file_that_is_not_an_image() { + let root = std::env::temp_dir().join(format!("pc-no-replace-foreign-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&root); + std::fs::create_dir_all(root.join("src")).unwrap(); + let foreign = [("src/main.rs", "fn main() {}\n"), ("Cargo.toml", "[package]\n"), (".env", "TOKEN=x\n"), ("Makefile", "all:\n")]; + for (name, text) in foreign { + std::fs::write(root.join(name), text).unwrap(); + } + std::fs::write(root.join("old.png"), b"not yet a png").unwrap(); + let mut h = Headless::with_workspace(crate::AuthorizedWorkspace::new(Some(&root), Some(&root)).unwrap()); + h.handle("doc.new", json!({"width": 8, "height": 8, "background": "white"})).unwrap(); + for (name, text) in foreign { + let saved = h.handle("doc.save", json!({"path": name, "format": "png"})).unwrap_err().to_string(); + assert!(saved.contains("already exists"), "doc.save over {name}: {saved}"); + let rendered = h.handle("doc.render", json!({"path": name})).unwrap_err().to_string(); + assert!(rendered.contains("already exists"), "doc.render over {name}: {rendered}"); + assert_eq!(std::fs::read_to_string(root.join(name)).unwrap(), text, "{name} is untouched"); + } + // An existing image is replaced, and a new file of any name is created. + h.handle("doc.save", json!({"path": "old.png"})).unwrap(); + assert!(std::fs::read(root.join("old.png")).unwrap().starts_with(b"\x89PNG")); + h.handle("doc.save", json!({"path": "fresh.txt", "format": "png"})).unwrap(); + assert!(std::fs::read(root.join("fresh.txt")).unwrap().starts_with(b"\x89PNG")); + h.handle("doc.render", json!({"path": "old.png"})).unwrap(); + std::fs::write(root.join("OLD.TIF"), b"x").unwrap(); + h.handle("doc.save", json!({"path": "OLD.TIF"})).unwrap(); + // An image saved under a name without an image extension is recognised by its bytes. + h.handle("doc.save", json!({"path": "out", "format": "png"})).unwrap(); + h.handle("doc.save", json!({"path": "out", "format": "png"})).unwrap(); + h.handle("doc.render", json!({"path": "fresh.txt"})).unwrap(); + // Release directory capabilities before cleanup on Windows. + drop(h); + std::fs::remove_dir_all(root).unwrap(); + } + #[test] fn doc_replace_smart_object_reads_through_the_read_root_and_never_outside_it() { let base = std::env::temp_dir().join(format!("pc-doc-replace-so-{}", std::process::id())); diff --git a/crates/automation/src/workspace.rs b/crates/automation/src/workspace.rs index e8566b439e..ba10120858 100644 --- a/crates/automation/src/workspace.rs +++ b/crates/automation/src/workspace.rs @@ -88,6 +88,15 @@ impl AuthorizedWorkspace { let relative = request_path(path, self.write.as_ref())?; let root = self.write.as_ref().ok_or_else(|| AutomationError::BadRequest(format!("{DENIED}: write authority is absent")))?; let leaf = relative.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default(); + // The bytes are an image or document whatever the name says (`format` picks them), so + // only a file of a kind PhotoCraft writes is replaced: never source, settings or notes + // that share the root (`src/main.rs`, `.env`; #9439). A new file may have any name, and + // an image saved under one (`out` with `format: png`) can be saved over again. + if root.dir.symlink_metadata(&relative).is_ok() && !written_kind(&leaf) && !holds_image(&root.dir, &relative) { + return Err(AutomationError::BadRequest(format!( + "automation write refused: `{path}` already exists and is not an image or document file PhotoCraft writes; save to a new name" + ))); + } let parent = relative.parent().map(Path::to_path_buf).unwrap_or_default(); let tmp = parent.join(photocraft_format::atomic::temp_name(&leaf)); let mut options = OpenOptions::new(); @@ -124,6 +133,23 @@ impl AuthorizedWorkspace { } } +/// Whether `name`'s extension is one of the formats PhotoCraft saves or exports. +fn written_kind(name: &str) -> bool { + let Some((_, ext)) = name.rsplit_once('.') else { return false }; + let ext = ext.to_ascii_lowercase(); + ["psd", "psb", "ora", "pdf", photocraft_format::EXTENSION].contains(&ext.as_str()) || photocraft_codecs::from_extension(&ext).is_some() +} + +/// Whether the file at `relative` starts like an image PhotoCraft reads (PSD/PSB or a codec's +/// magic). Containers that also hold other things (zip, PDF) are judged by name only. +fn holds_image(dir: &Dir, relative: &Path) -> bool { + let mut head = [0u8; 64]; + let Ok(mut file) = dir.open(relative) else { return false }; + let Ok(n) = file.read(&mut head) else { return false }; + let head = &head[..n]; + head.starts_with(b"8BPS") || photocraft_codecs::detect(head).is_some() +} + fn read_to_observed_length(reader: &mut impl Read, observed_length: u64) -> Result, AutomationError> { // Do not reserve from a potentially stale or attacker-controlled metadata length. Read in // chunks, and reject growth beyond the length observed on the opened file before appending. diff --git a/docs/mcp.md b/docs/mcp.md index 75cacd6e41..8846a3be1c 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -57,6 +57,15 @@ Paths are relative to the corresponding root, or absolute paths beneath it; with filesystem authority is absent. Bridge mode uses `--bridge` and `--control-token-file` as described in [development.md](development.md). UI tools require a running desktop app. +A client has the full rights of the roots it is given: it can read any file below the read +root, and `doc_save`, `doc_export` and the export tools can create files anywhere below the +write root. `doc_save` and `doc_export` replace an existing file only when its extension is an +image or document format PhotoCraft writes (PNG, JPEG, TIFF, PSD, PSB, .pcraft, OpenRaster, +PDF, …) or its bytes are an image; anything else (`src/main.rs`, `.env`) is refused and left +untouched. The multi-file exports (layers, data sets) never replace a file. An agent reads +text from documents (layer names, metadata) that may try to steer it, so prefer a dedicated +output folder as the write root over a whole project. + Use absolute root paths in MCP client configurations, since a relative root is resolved from the client's launch directory. In bridge mode, configure roots on the **desktop app**; root flags on `photocraft-cli mcp --bridge` are rejected. Explicit `--control-token` and @@ -89,7 +98,7 @@ protocol messages. For `serve`, explicit credential flags require `--port` (or Existing document/session/job/UI tools remain listed, including the documented `doc_render_preview` spelling. There are no hidden legacy aliases. Every tool has a title and all four annotation hints. Generic command/control tools conservatively advertise writes; -file saves may replace existing targets. Hints do not grant permission. +file saves may replace existing image and document files. Hints do not grant permission. After a failed batch, `completed` and per-step `results` report successful steps, not a rolled-back transaction. Inspect the document and its history with `doc_inspect`; commands