diff --git a/crates/engine/src/cmd/package.rs b/crates/engine/src/cmd/package.rs index ed87362b..ecbba8e4 100644 --- a/crates/engine/src/cmd/package.rs +++ b/crates/engine/src/cmd/package.rs @@ -343,6 +343,7 @@ fn package(s: &mut Session, p: &Value) -> Result { let warnings = std::mem::take(&mut collector.warnings); let fonts = collector.fonts; let links = collector.copies.len(); + let copied_sources: Vec = collector.copies.keys().cloned().collect(); // The report. if let Some(info) = info { @@ -357,6 +358,9 @@ fn package(s: &mut Session, p: &Value) -> Result { let mut out = json!({ "files": files, "links": links, "fonts": fonts, "missingLinks": missing, "skippedFonts": skipped, "warnings": warnings }); match root { Some(root) => { + // Never replace the open document or a file it links to: the folder may be the document's own. + let sources: Vec<&str> = std::iter::once(doc_path.as_str()).chain(copied_sources.iter().map(String::as_str)).collect(); + check_no_overwrite(&root, &entries, &sources)?; for (rel, bytes) in &entries { let path = root.join(rel); if let Some(dir) = path.parent() { @@ -377,6 +381,21 @@ fn package(s: &mut Session, p: &Value) -> Result { Ok(out) } +/// An error when writing any of `entries` (paths relative to `root`) would replace one of the +/// `sources` (compared by their real paths, so aliases and `..` don't hide a collision). +fn check_no_overwrite(root: &Path, entries: &[Entry], sources: &[&str]) -> Result<()> { + let real: Vec = sources.iter().filter_map(|s| std::fs::canonicalize(s).ok()).collect(); + for (rel, _) in entries { + let path = root.join(rel); + if let Ok(dest) = std::fs::canonicalize(&path) + && real.contains(&dest) + { + return Err(bad(C, format!("`{}` would be overwritten by the package: choose a different folder or name", path.display()))); + } + } + Ok(()) +} + // ---------- zip archives ---------- /// The MS-DOS time and date a zip entry records: now (UTC), else 1980-01-01. diff --git a/crates/engine/src/tests_package.rs b/crates/engine/src/tests_package.rs index a4c899c3..f6b5b6c8 100644 --- a/crates/engine/src/tests_package.rs +++ b/crates/engine/src/tests_package.rs @@ -143,6 +143,28 @@ fn options_leave_out_links_fonts_and_the_report() { assert!(s.execute("file.package", &json!({"name": "../up"})).is_err()); } +#[test] +fn a_package_never_replaces_its_source_or_its_links() { + let dir = Folder::new("package-overwrite"); + let mut s = poster(&dir); + let doc = dir.file("poster.vectorcraft"); + let (saved, red) = (std::fs::read(&doc).unwrap(), std::fs::read(dir.file("art/red.png")).unwrap()); + let parent = dir.0.parent().unwrap(); + let name = dir.0.file_name().unwrap().to_string_lossy().into_owned(); + // The document's own folder, named directly and through `..`. + for folder in [parent.to_path_buf(), dir.0.join("art").join("..").join("..")] { + let e = s.execute("file.package", &json!({"folder": folder, "name": name})).unwrap_err().to_string(); + assert!(e.contains("would be overwritten"), "{e}"); + } + // The folder of its linked files, with the links beside the packaged document. + let e = s.execute("file.package", &json!({"folder": dir.0, "name": "art", "linksFolder": false})).unwrap_err().to_string(); + assert!(e.contains("red.png"), "{e}"); + // Nothing was written. + assert_eq!(std::fs::read(&doc).unwrap(), saved); + assert_eq!(std::fs::read(dir.file("art/red.png")).unwrap(), red); + assert!(!dir.0.join("art/poster.vectorcraft").exists() && !dir.0.join("poster Report.txt").exists()); +} + #[test] fn a_missing_link_is_reported_not_copied() { let dir = Folder::new("package-missing"); diff --git a/docs/mcp.md b/docs/mcp.md index f7f16a2b..61ae0b42 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -2302,7 +2302,9 @@ files in `Links/` (relinked: the packaged document points at the copies; the ope type uses in `Fonts/` (fonts whose licence doesn't allow embedding are listed in `skippedFonts` instead) and ` Report.txt`. A placed `.vectorcraft` document is packaged with its own linked files and fonts, relinked to the copies (each file copied once); one that can't be read is copied as it is, and a cycle of placed documents or one -past the nesting limit stops there, each with a line in `warnings` and the report. Every option defaults to true. Without `folder` (the web, or an agent that wants the bytes) the +past the nesting limit stops there, each with a line in `warnings` and the report. A `folder/name` where a packaged file +would replace the document or one of its linked files (the document's own folder, say) is an error, and nothing is +written. Every option defaults to true. Without `folder` (the web, or an agent that wants the bytes) the result carries the same files as a zip (`{name: ".zip", dataBase64}`, entries under `/`). `document.info {selectionOnly?, category?, format?: "text"}` adds `sections` (`[{id, title, rows: [[label, value]]}]`: