mirror of
https://github.com/storytold/figcraft.git
synced 2026-10-24 10:27:00 -06:00
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.
This commit is contained in:
@@ -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> {
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user