security(engine): confine stored media filesystem access (#2869)

* security(engine): confine stored media filesystem access

* relink: index symlinked files; skip only symlinked folders

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Brandon Thomas <brandon@storyteller.ai>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Earl Co
2026-10-23 10:53:29 -04:00
committed by GitHub
co-authored by Claude Opus 5.5 Brandon Thomas
parent 875eec90e9
commit 7f8c9921a1
3 changed files with 57 additions and 19 deletions
+19 -13
View File
@@ -137,7 +137,10 @@ pub fn run(s: &mut Session, p: &Value) -> Result<Value> {
for id in ids {
let Some(m) = proj.item(id).and_then(|i| i.as_media()).cloned() else { continue };
let MediaRef::File { path } = &m.media else { continue };
let size = s.services.file_size(path).unwrap_or(0);
// Project files carry stored paths; use only the media pool's resolved, policy-approved
// pathname for filesystem I/O. Keep the stored path in the project itself.
let source_path = s.media.follow(path).map_err(|why| EngineError::Other(format!("{path} is not opened: {why}")))?;
let size = s.services.file_size(&source_path).unwrap_or(0);
original_bytes += size;
let file = Path::new(path);
let stem = file.file_stem().map(|n| n.to_string_lossy().into_owned()).unwrap_or_else(|| format!("media-{}", id.0));
@@ -158,20 +161,23 @@ pub fn run(s: &mut Session, p: &Value) -> Result<Value> {
};
taken.push(out.clone());
let proxy = match (&m.proxy, include_proxies) {
(Some(MediaRef::File { path: pp }), true) if s.services.file_size(pp).is_ok() => {
let pf = Path::new(pp);
let ps = pf.file_stem().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default();
let pe = pf.extension().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default();
let o = unique_path(&dest_dir.join("Proxies"), &ps, "", &pe, &taken).to_string_lossy().into_owned();
taken.push(o.clone());
let b = s.services.file_size(pp).unwrap_or(0);
original_bytes += b;
result_bytes += b;
Some((pp.clone(), o))
}
(Some(MediaRef::File { path: pp }), true) => match s.media.follow(pp) {
Ok(resolved_proxy) if s.services.file_size(&resolved_proxy).is_ok() => {
let pf = Path::new(&resolved_proxy);
let ps = pf.file_stem().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default();
let pe = pf.extension().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default();
let o = unique_path(&dest_dir.join("Proxies"), &ps, "", &pe, &taken).to_string_lossy().into_owned();
taken.push(o.clone());
let b = s.services.file_size(&resolved_proxy).unwrap_or(0);
original_bytes += b;
result_bytes += b;
Some((resolved_proxy, o))
}
_ => None,
},
_ => None,
};
work.push(Work { item: id, src_path: path.clone(), out, range, proxy });
work.push(Work { item: id, src_path: source_path, out, range, proxy });
}
let previews = s.path.as_deref().map(crate::previews::dir_for_project).filter(|d| include_previews && d.is_dir());
if let Some(d) = &previews {
+14 -5
View File
@@ -489,9 +489,14 @@ pub fn index_folder(root: &Path, limit: usize) -> BTreeMap<String, Vec<PathBuf>>
if name.starts_with('.') {
continue;
}
if p.is_dir() {
let Ok(file_type) = e.file_type() else { continue };
// A relink search must stay within the folder explicitly chosen by the user. In
// particular, following a directory symlink can make a small loop defeat the file
// budget and walk an unrelated tree. Symlinked files are still candidates.
let is_file = if file_type.is_symlink() { std::fs::metadata(&p).is_ok_and(|m| m.is_file()) } else { file_type.is_file() };
if file_type.is_dir() {
queue.push_back(p);
} else {
} else if is_file {
out.entry(name.to_ascii_lowercase()).or_default().push(p);
n += 1;
if n >= limit {
@@ -564,7 +569,11 @@ pub fn make_offline(s: &mut Session, p: &Value) -> Result<Value> {
let mut deleted = Vec::new();
if delete {
for (_, path) in &files {
if std::fs::remove_file(path).is_ok() {
// Stored paths in an opened document are not authority to delete arbitrary files.
// Delete only the resolved path that the media pool permits opening.
if let Ok(path) = s.media.follow(path)
&& std::fs::remove_file(&path).is_ok()
{
deleted.push(path.clone());
}
}
@@ -583,14 +592,14 @@ fn status_json(s: &Session, id: ItemId) -> Option<Value> {
let status = match &path {
None => "generated",
Some(_) if m.offline => "offline",
Some(p) if s.services.file_size(p).is_err() => "missing",
Some(p) if s.media.follow(p).ok().is_none_or(|file| s.services.file_size(&file).is_err()) => "missing",
Some(_) => match s.media.offline_status(id) {
Some(o) if o.reason == filmcraft_render::offline::OfflineReason::Unreadable => "unreadable",
_ => "online",
},
};
let proxy = match &m.proxy {
Some(MediaRef::File { path }) => json!({"path": path, "online": s.services.file_size(path).is_ok()}),
Some(MediaRef::File { path }) => json!({"path": path, "online": s.media.follow(path).is_ok_and(|file| s.services.file_size(&file).is_ok())}),
_ => Value::Null,
};
Some(json!({
+24 -1
View File
@@ -9,7 +9,7 @@ use serde_json::json;
use crate::Session;
use crate::media_test_util::{frame_rgba, make_movie, psnr, session_with, tmp_dir};
use crate::relink::{apply_remap, derive_remap};
use crate::relink::{apply_remap, derive_remap, index_folder};
const W: u32 = 160;
const H: u32 = 90;
@@ -163,6 +163,29 @@ fn make_offline_shows_the_slate_until_relinked() {
let _ = std::fs::remove_dir_all(&root);
}
#[cfg(unix)]
#[test]
fn relink_search_does_not_follow_directory_symlinks() {
use std::os::unix::fs::symlink;
let root = tmp_dir("relink-no-symlink-walk");
let inside = root.join("inside");
let outside = root.join("outside");
std::fs::create_dir_all(&inside).unwrap();
std::fs::create_dir_all(&outside).unwrap();
std::fs::write(inside.join("kept.mov"), b"kept").unwrap();
std::fs::write(outside.join("outside.mov"), b"outside").unwrap();
symlink(&outside, inside.join("loop")).unwrap();
symlink(outside.join("outside.mov"), inside.join("linked.mov")).unwrap();
let index = index_folder(&inside, 20_000);
assert!(index.contains_key("kept.mov"));
assert!(!index.contains_key("outside.mov"));
// a symlinked file is still media the user can relink to
assert!(index.contains_key("linked.mov"));
let _ = std::fs::remove_dir_all(&root);
}
// Regression test for issue #110: after `file.import` re-imports a file at a path the project
// already listed as offline, `s.offline.missing` must drop that item so the slate and the
// "Media missing" badge clear without reopening the project. Before the fix (the patch in