Automation: a confined batch can't take a step's params from a reference, and renders refuse folders with outward links (#2930)

* automation: confine batch steps whose params are a reference, and sequence render folders holding outward links

A confined engine.batch checked each step's file parameters before the batch
ran, but a step whose whole params was a `$N` reference became an earlier
step's result only at run time, so a file path in that result (for example a
manifest from essential.templateInfo) reached file.saveAs unchecked. Such
steps are now refused while file access is confined.

An image-sequence render writes numbered frame names that the check of its
output template never resolved, and each frame is created through any link
already at its name. A render output whose folder holds a link leading
outside the write roots (or one that cannot be resolved) is now refused when
it is queued and again when the queue renders.

* address review: check numbered render folders for outward links

* address review: fail closed when the numbered-folder scan cannot list its parent; unix-only symlink test

* address review: render-folder link scan fails closed on unreadable folders, entries and file types

crates/automation/src/confine.rs

@@ -75,6 +75,9 @@ const REFUSED: &[&str] = &[
     "library.preview",
 ];
 
+/// Most numbered folders `check_render_folder` inspects for one output; more are refused.
+const MAX_NUMBERED_FOLDERS: usize = 4096;
+
 /// Commands that read files named by these parameters.
 const READS: &[(&str, &[&str])] = &[
     ("file.open", &["path"]),
@@ -202,6 +205,80 @@ impl Confinement {
         if self.roots(access).any(|r| real.starts_with(r)) { Ok(real) } else { Err(denied("it is outside the automation roots")) }
     }
 
+    /// A render output's folder holds no symbolic link that leads outside the write roots. An
+    /// image sequence is written to numbered names (`out_[#####].png` → `out_00001.png`, and a
+    /// sequence output without `#` gets `_00001` too) that the check of its template never
+    /// resolved, and each frame file is created through whatever link already sits at its name,
+    /// truncating the file it points to. A link that can't be resolved counts as outside: the
+    /// render would create its target. When the frame number is in a folder name, the same goes for
+    /// each existing numbered folder and the folder it holds the frames in.
+    fn check_render_folder(&self, output: &str) -> Result<()> {
+        let real = self.check(output, Access::Write)?;
+        if let Some(dir) = real.parent() {
+            self.check_no_outward_links(dir, output)?;
+        }
+        // The frame number may sit in a folder name (`shots_[##]/f_[#####].png`): the folders the
+        // frames go to are the numbered ones that exist, which the template's own parent is not.
+        let (before, _, after) = effectcraft_engine::project::render_queue::sequence_parts(output);
+        let sep = ['/', '\\'];
+        let Some((dir_tail, rest)) = after.split_once(sep) else { return Ok(()) };
+        let (parent, prefix) = before.rsplit_once(sep).unwrap_or(("", &before));
+        // A parent that doesn't exist has no numbered folders; one that can't be listed might.
+        let scan = if parent.is_empty() { Path::new("/") } else { Path::new(parent) };
+        let entries = match std::fs::read_dir(scan) {
+            Ok(entries) => entries,
+            Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(()),
+            Err(e) => return Err(Error::BadArgs(format!("cannot render to `{output}`: its numbered folders in `{}` can't be checked: {e}", scan.display()))),
+        };
+        let mut seen = 0;
+        for e in entries {
+            let e =
+                e.map_err(|e| Error::BadArgs(format!("cannot render to `{output}`: its numbered folders in `{}` can't be checked: {e}", scan.display())))?;
+            let name = e.file_name().to_string_lossy().into_owned();
+            let Some(digits) = name.strip_prefix(prefix).and_then(|n| n.strip_suffix(dir_tail)) else { continue };
+            if !digits.strip_prefix('-').unwrap_or(digits).bytes().all(|b| b.is_ascii_digit()) || digits.is_empty() {
+                continue;
+            }
+            seen += 1;
+            if seen > MAX_NUMBERED_FOLDERS {
+                return Err(Error::BadArgs(format!("cannot render to `{output}`: more than {MAX_NUMBERED_FOLDERS} numbered folders already exist to check")));
+            }
+            // The numbered folder itself may be a link out; `check` resolves every existing part.
+            let frame_file = e.path().join(rest);
+            let frame_file = frame_file.to_string_lossy();
+            self.check(&frame_file, Access::Write)?;
+            if let Some(dir) = Path::new(&*frame_file).parent().and_then(resolve) {
+                self.check_no_outward_links(&dir, output)?;
+            }
+        }
+        Ok(())
+    }
+
+    /// `dir` holds no symbolic link that leads outside the write roots.
+    fn check_no_outward_links(&self, dir: &Path, output: &str) -> Result<()> {
+        // A folder that does not exist yet holds no link; one that can't be listed might.
+        let cannot = |e: std::io::Error| Error::BadArgs(format!("cannot render to `{output}`: its folder `{}` can't be checked for links: {e}", dir.display()));
+        let entries = match std::fs::read_dir(dir) {
+            Ok(entries) => entries,
+            Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(()),
+            Err(e) => return Err(cannot(e)),
+        };
+        for e in entries {
+            let e = e.map_err(cannot)?;
+            if !e.file_type().map_err(cannot)?.is_symlink() {
+                continue;
+            }
+            let link = e.path();
+            if !link.canonicalize().is_ok_and(|r| self.write.iter().any(|w| r.starts_with(w))) {
+                return Err(Error::BadArgs(format!(
+                    "cannot render to `{output}`: its folder holds `{}`, a link that leads outside the automation write roots, where a numbered frame could be written",
+                    link.display()
+                )));
+            }
+        }
+        Ok(())
+    }
+
     /// Check engine command `id` with `params` before it runs. Returns what must still be looked
     /// up and passed to [`Confinement::check_lookup`] for commands that write implicitly.
     pub fn check_command(&self, id: &str, params: &Value) -> Result<Vec<Lookup>> {
@@ -221,6 +298,12 @@ impl Confinement {
             for step in params.get("steps").and_then(Value::as_array).into_iter().flatten() {
                 let Some(inner) = step.get("command").or_else(|| step.get("id")).and_then(Value::as_str) else { continue };
                 let inner_params = step.get("params").cloned().unwrap_or(Value::Null);
+                // A whole `params` given as a `$N` reference becomes an earlier step's result
+                // only as the batch runs, so none of its file paths would have been checked
+                // (`essential.templateInfo` returns a manifest naming any file at all).
+                if !matches!(inner_params, Value::Null | Value::Object(_)) {
+                    return Err(Error::BadArgs(format!("`{inner}` in a batch: `params` must be an object while file access is confined")));
+                }
                 let named = keys_of(READS, inner).or_else(|| keys_of(WRITES, inner)).unwrap_or(&[]);
                 let mixed = keys_of(MIXED_WRITES, inner).unwrap_or(&[]);
                 // A step's `$N` reference is resolved only as the batch runs: never for a file.
@@ -258,6 +341,9 @@ impl Confinement {
                     Some(s) => {
                         gave_a_file |= v.is_string();
                         self.check(s, access)?;
+                        if QUEUES_AN_OUTPUT.contains(&id) || id.starts_with("renderQueue.") {
+                            self.check_render_folder(s)?;
+                        }
                     }
                     None => return Err(Error::BadArgs(format!("`{id}`: `{key}` must be a path"))),
                 }
@@ -321,11 +407,15 @@ impl Confinement {
                 let items = found.get("items").or(Some(found)).and_then(Value::as_array).cloned().unwrap_or_default();
                 for it in items.iter().filter(|it| it.get("render").and_then(Value::as_bool) != Some(false)) {
                     let extra = it.get("extraOutputs").and_then(Value::as_array).into_iter().flatten().map(|o| o.get("outputPath"));
-                    for p in std::iter::once(it.get("outputPath")).chain(extra).chain([it.get("logPath")]).flatten() {
+                    for p in std::iter::once(it.get("outputPath")).chain(extra).flatten() {
                         if let Some(p) = p.as_str() {
                             self.check(p, Access::Write)?;
+                            self.check_render_folder(p)?;
                         }
                     }
+                    if let Some(p) = it.get("logPath").and_then(Value::as_str) {
+                        self.check(p, Access::Write)?;
+                    }
                 }
                 Ok(())
             }
@@ -757,6 +847,112 @@ mod tests {
         ] {
             assert!(c.check_command("effect.plugins.install", &params).is_err(), "{params}");
         }
+    }
+
+    #[test]
+    fn a_batch_step_whose_params_are_a_reference_is_refused() {
+        let base = std::env::temp_dir().join(format!("ec-confine-batch-ref-{}", std::process::id()));
+        let _ = std::fs::remove_dir_all(&base);
+        let (read, write) = (base.join("in"), base.join("out"));
+        for d in [&read, &write] {
+            std::fs::create_dir_all(d).unwrap();
+        }
+        let c = Confinement::new(std::slice::from_ref(&read), std::slice::from_ref(&write)).unwrap();
+        let info = read.join("t.ectemplate").to_string_lossy().into_owned();
+        // Step 2's params are step 1's result, which can name any `path`.
+        let steps = serde_json::json!({"steps": [
+            {"command": "essential.templateInfo", "params": {"path": info}},
+            {"command": "file.saveAs", "params": "$1"},
+        ]});
+        let e = c.check_command("engine.batch", &steps).unwrap_err().to_string();
+        assert!(e.contains("must be an object"), "{e}");
+        for params in [serde_json::json!("$1.manifest"), serde_json::json!(["$1"]), serde_json::json!(7)] {
+            let steps = serde_json::json!({"steps": [{"command": "file.saveAs", "params": params}]});
+            assert!(c.check_command("engine.batch", &steps).is_err(), "{params}");
+        }
+        // Object params (or none) are checked as before.
+        let saved = write.join("p.ecproj").to_string_lossy().into_owned();
+        let steps = serde_json::json!({"steps": [{"command": "file.saveAs", "params": {"path": saved}}, {"command": "edit.undo"}]});
+        assert!(c.check_command("engine.batch", &steps).is_ok());
+        let _ = std::fs::remove_dir_all(&base);
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn a_render_folder_with_a_link_out_of_the_write_roots_is_refused() {
+        let base = std::env::temp_dir().join(format!("ec-confine-seq-link-{}", std::process::id()));
+        let _ = std::fs::remove_dir_all(&base);
+        let (read, write, outside) = (base.join("in"), base.join("out"), base.join("elsewhere"));
+        for d in [&read, &write, &outside] {
+            std::fs::create_dir_all(d).unwrap();
+        }
+        let c = Confinement::new(std::slice::from_ref(&read), std::slice::from_ref(&write)).unwrap();
+        let template = write.join("frames_[#####].png").to_string_lossy().into_owned();
+        let add = serde_json::json!({"output": template});
+        let queue = serde_json::json!({"items": [{"outputPath": template}]});
+        assert!(c.check_command("renderQueue.add", &add).is_ok());
+        assert!(c.check_lookup(&Lookup::RenderOutputs, &queue).is_ok());
+        // A link inside the write root is fine.
+        std::os::unix::fs::symlink(write.join("kept.png"), write.join("frames_00002.png")).unwrap();
+        std::fs::write(write.join("kept.png"), b"x").unwrap();
+        assert!(c.check_lookup(&Lookup::RenderOutputs, &queue).is_ok());
+        // Frame 1's name leads outside: rendering would truncate that file.
+        std::os::unix::fs::symlink(outside.join("victim"), write.join("frames_00001.png")).unwrap();
+        assert!(c.check_command("renderQueue.add", &add).is_err());
+        assert!(c.check_command("render.preRender", &add).is_err());
+        let e = c.check_lookup(&Lookup::RenderOutputs, &queue).unwrap_err().to_string();
+        assert!(e.contains("frames_00001.png"), "{e}");
+        let _ = std::fs::remove_dir_all(&base);
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn a_render_folder_that_cannot_be_listed_is_refused() {
+        use std::os::unix::fs::PermissionsExt;
+        let base = std::env::temp_dir().join(format!("ec-confine-seq-nolist-{}", std::process::id()));
+        let _ = std::fs::remove_dir_all(&base);
+        let write = base.join("out");
+        std::fs::create_dir_all(&write).unwrap();
+        let c = Confinement::new(&[], std::slice::from_ref(&write)).unwrap();
+        let template = write.join("frames_[#####].png").to_string_lossy().into_owned();
+        let add = serde_json::json!({"output": template});
+        // Writable and searchable but not listable: a link in it could not be seen.
+        std::fs::set_permissions(&write, std::fs::Permissions::from_mode(0o300)).unwrap();
+        // A user who can list it anyway (root) sees no difference to test.
+        if std::fs::read_dir(&write).is_err() {
+            assert!(c.check_command("renderQueue.add", &add).is_err());
+        }
+        std::fs::set_permissions(&write, std::fs::Permissions::from_mode(0o700)).unwrap();
+        assert!(c.check_command("renderQueue.add", &add).is_ok());
+        let _ = std::fs::remove_dir_all(&base);
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn numbered_render_folders_with_links_out_of_the_write_roots_are_refused() {
+        let base = std::env::temp_dir().join(format!("ec-confine-seq-dir-{}", std::process::id()));
+        let _ = std::fs::remove_dir_all(&base);
+        let (write, outside) = (base.join("out"), base.join("elsewhere"));
+        for d in [&write, &outside] {
+            std::fs::create_dir_all(d).unwrap();
+        }
+        let c = Confinement::new(&[], std::slice::from_ref(&write)).unwrap();
+        let template = write.join("shot_[###]/f_[#####].png").to_string_lossy().into_owned();
+        let add = serde_json::json!({"output": template});
+        assert!(c.check_command("renderQueue.add", &add).is_ok(), "no numbered folder exists yet");
+        // A numbered folder inside the roots holding a link out is refused.
+        std::fs::create_dir(write.join("shot_001")).unwrap();
+        assert!(c.check_command("renderQueue.add", &add).is_ok());
+        std::os::unix::fs::symlink(outside.join("victim"), write.join("shot_001/f_00001.png")).unwrap();
+        assert!(c.check_command("renderQueue.add", &add).is_err());
+        std::fs::remove_file(write.join("shot_001/f_00001.png")).unwrap();
+        // A numbered folder that is itself a link out is refused.
+        std::os::unix::fs::symlink(&outside, write.join("shot_002")).unwrap();
+        assert!(c.check_command("render.preRender", &add).is_err());
+        // Unrelated names are left alone.
+        std::fs::remove_file(write.join("shot_002")).unwrap();
+        std::os::unix::fs::symlink(&outside, write.join("shot_notes")).unwrap();
+        assert!(c.check_command("renderQueue.add", &add).is_ok());
         let _ = std::fs::remove_dir_all(&base);
     }
 }