From 35cc685e90e42bce8ce30a049c13bd8985dd6bf9 Mon Sep 17 00:00:00 2001
From: Hamza <mail.hamzanaveed@gmail.com>
Date: Mon, 19 Oct 2026 00:52:40 +0500
Subject: [PATCH] cli: an unconfined write still follows a symbolic link

Routing the CLI's writes through `file_access` changed what happens when the
output path is a symbolic link. `file_access::write` stages the bytes and
renames them into place, and a rename replaces the link with a regular file,
where `std::fs::write` followed it and wrote to its target. A render to a link
reported success while the target kept its old contents.

Unconfined writes go back to `std::fs::write`. Confined ones keep the staged
write, which is right there: `Roots::check` resolves the path first and has
already refused a link leading out of the root, so the write lands on the
resolved target inside it.
---
 apps/figcraft-cli/src/files.rs | 15 ++++++++++++---
 apps/figcraft-cli/tests/cli.rs | 28 ++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/apps/figcraft-cli/src/files.rs b/apps/figcraft-cli/src/files.rs
index 9dc357b1..46215599 100644
--- a/apps/figcraft-cli/src/files.rs
+++ b/apps/figcraft-cli/src/files.rs
@@ -29,13 +29,22 @@ fn image_format(e: &str) -> Option<ExportFormat> {
     }
 }
 
-/// Writes through [`figcraft_engine::file_access`], so a run given automation roots can only
-/// write inside a write root (#833). Unconfined it writes as before.
+/// Writes through [`figcraft_engine::file_access`] when automation roots are in force, so a
+/// confined run can only write inside a write root (#833).
+///
+/// Unconfined it writes with `std::fs::write`, as it always did. `file_access::write` stages the
+/// bytes and renames them into place, which **replaces** a symbolic link at `path` instead of
+/// writing through it to its target -- a change of behaviour for anyone whose output path is a
+/// link, and nothing to do with confinement. Inside a root the staged write is the right one:
+/// `Roots::check` has already refused a link that leads out of the root.
 fn write(path: &str, bytes: &[u8]) -> CliResult<()> {
     if let Some(dir) = Path::new(path).parent().filter(|d| !d.as_os_str().is_empty()) {
         figcraft_engine::file_access::create_dir_all(dir).map_err(|e| CliError::Failed(e.to_string()))?;
     }
-    figcraft_engine::file_access::write(path, bytes).map_err(|e| CliError::Failed(e.to_string()))
+    if figcraft_engine::file_access::current().is_some() {
+        return figcraft_engine::file_access::write(path, bytes).map_err(|e| CliError::Failed(e.to_string()));
+    }
+    std::fs::write(path, bytes).map_err(|e| CliError::Failed(format!("{path}: {e}")))
 }
 
 fn page_named(doc: &Document, name: Option<&str>) -> CliResult<NodeId> {
diff --git a/apps/figcraft-cli/tests/cli.rs b/apps/figcraft-cli/tests/cli.rs
index 399e57bb..eb42b8d8 100644
--- a/apps/figcraft-cli/tests/cli.rs
+++ b/apps/figcraft-cli/tests/cli.rs
@@ -239,3 +239,31 @@ fn file_subcommands_are_confined_to_automation_roots() {
     assert!(escaped.is_file());
     let _ = std::fs::remove_dir_all(&d);
 }
+
+/// An output path that is a symbolic link is written through, to its target, as `std::fs::write`
+/// always did. Routing the CLI's writes through `file_access` for #833 would otherwise have
+/// replaced the link with a regular file, because that write stages the bytes and renames them
+/// into place.
+#[cfg(unix)]
+#[test]
+fn an_unconfined_write_follows_a_symbolic_link() {
+    let d = tmp("symlink-out");
+    let _ = std::fs::remove_dir_all(&d);
+    std::fs::create_dir_all(d.join("real")).unwrap();
+    let p = |rel: &str| d.join(rel).to_string_lossy().into_owned();
+    std::fs::write(d.join("in.json"), br#"[{"command":"shape.rectangle","params":{"x":0,"y":0,"width":40,"height":20}}]"#).unwrap();
+    let made = bin().args(["run", &p("in.json"), "--out", &p("doc.figcraft")]).output().unwrap();
+    assert!(made.status.success(), "{}", String::from_utf8_lossy(&made.stderr));
+
+    let target = d.join("real/target.png");
+    std::fs::write(&target, b"placeholder").unwrap();
+    let link = d.join("link.png");
+    std::os::unix::fs::symlink(&target, &link).unwrap();
+
+    let out = bin().args(["render", &p("doc.figcraft"), "-o", &link.to_string_lossy()]).output().unwrap();
+    assert!(out.status.success(), "{}", String::from_utf8_lossy(&out.stderr));
+    assert!(std::fs::symlink_metadata(&link).unwrap().file_type().is_symlink(), "the link is still a link");
+    let written = std::fs::read(&target).unwrap();
+    assert!(written.starts_with(b"\x89PNG"), "the target holds the render, not the placeholder");
+    let _ = std::fs::remove_dir_all(&d);
+}