Merge pull request #1786 from MarekSpodBiedry/issue-1696

Refuse to package over the source document or its linked files (#1696)

crates/engine/src/cmd/package.rs

@@ -343,6 +343,7 @@ fn package(s: &mut Session, p: &Value) -> Result<Value> {
     let warnings = std::mem::take(&mut collector.warnings);
     let fonts = collector.fonts;
     let links = collector.copies.len();
+    let copied_sources: Vec<String> = collector.copies.keys().cloned().collect();
 
     // The report.
     if let Some(info) = info {
@@ -357,6 +358,9 @@ fn package(s: &mut Session, p: &Value) -> Result<Value> {
     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<Value> {
     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<PathBuf> = 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.

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");

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
 `<document> 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: "<name>.zip", dataBase64}`, entries under `<name>/`).
 
 `document.info {selectionOnly?, category?, format?: "text"}` adds `sections` (`[{id, title, rows: [[label, value]]}]`: