From 689e37782a890fc76a0e0daa671ab169771a6c38 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Fri, 23 Oct 2026 09:29:27 -0400 Subject: [PATCH] fix: harden CLI file I/O and AV1 tooling (SonarCloud) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Read and write CLI inputs/outputs on a blocking pool instead of stalling the async runtime (rust:S7493). - Document the FLAC STREAMINFO MD5 as the RFC 9639 integrity signature it is, and validate the AV1 generator's spec path before reading it. - Normalize the DCT basis literals that read as numeric suffixes. 🤖 Generated with Codebuff Co-Authored-By: Codebuff --- apps/filmcraft-cli/src/main.rs | 25 ++++++++++++++++++++++--- crates/av1/tools/extract_tables.py | 14 +++++++++----- crates/dnx/src/tables.rs | 8 ++++---- crates/flac/src/lib.rs | 4 +++- crates/format/src/atomic.rs | 5 +++-- crates/prores/src/tables.rs | 8 ++++---- 6 files changed, 45 insertions(+), 19 deletions(-) diff --git a/apps/filmcraft-cli/src/main.rs b/apps/filmcraft-cli/src/main.rs index 8b508e334..cfc04fc3b 100644 --- a/apps/filmcraft-cli/src/main.rs +++ b/apps/filmcraft-cli/src/main.rs @@ -137,6 +137,25 @@ fn usage(msg: impl std::fmt::Display) -> ! { std::process::exit(2) } +/// Read a file off the runtime thread: `cli` is async because MCP, bridge calls and control +/// commands await, so a blocking `std::fs` read would stall every other task on that thread. +async fn read_file(path: impl Into) -> std::io::Result> { + let path = path.into(); + tokio::task::spawn_blocking(move || std::fs::read(path)).await.map_err(std::io::Error::other)? +} + +/// `read_file` for UTF-8 text. +async fn read_text(path: impl Into) -> std::io::Result { + let path = path.into(); + tokio::task::spawn_blocking(move || std::fs::read_to_string(path)).await.map_err(std::io::Error::other)? +} + +/// Write a file off the runtime thread; see `read_file`. +async fn write_file(path: impl Into, data: Vec) -> std::io::Result<()> { + let path = path.into(); + tokio::task::spawn_blocking(move || std::fs::write(path, data)).await.map_err(std::io::Error::other)? +} + fn fail(msg: impl std::fmt::Display) -> ! { diag(format_args!("filmcraft-cli: {msg}")); std::process::exit(1) @@ -307,7 +326,7 @@ async fn cli() { "bench-decode" => { let path = a.pos(1).unwrap_or_else(|| usage("bench-decode ")); let n: i64 = a.opt("--frames").and_then(|v| v.parse().ok()).unwrap_or(120); - let src = filmcraft_codecs::open_bytes(path, std::fs::read(path).unwrap_or_else(|e| fail(e)).into()).unwrap_or_else(|e| fail(e)); + let src = filmcraft_codecs::open_bytes(path, read_file(path).await.unwrap_or_else(|e| fail(e)).into()).unwrap_or_else(|e| fail(e)); let rate = src.info().frame_rate(); let t0 = std::time::Instant::now(); for f in 0..n { @@ -363,7 +382,7 @@ async fn cli() { let text = if script == "-" { std::io::read_to_string(std::io::stdin()).unwrap_or_else(|e| fail(e)) } else { - std::fs::read_to_string(script).unwrap_or_else(|e| fail(format!("{script}: {e}"))) + read_text(script).await.unwrap_or_else(|e| fail(format!("{script}: {e}"))) }; let mut b = Backend::open(&a); let mut failed = 0; @@ -487,7 +506,7 @@ async fn cli() { let (img, offline) = render_at_playhead(&s, scale).unwrap_or_else(|e| fail(e)); let dt = t0.elapsed(); let png = filmcraft_automation::png_rgba(img.w as u32, img.h as u32, img.over_black_rgba8(), 0).unwrap_or_else(|e| fail(e)); - std::fs::write(out, png).unwrap_or_else(|e| fail(format!("{out}: {e}"))); + write_file(out, png).await.unwrap_or_else(|e| fail(format!("{out}: {e}"))); let offline: Vec = offline.iter().filter_map(|o| serde_json::to_value(o).ok()).collect(); check_offline(&a, &offline, &format!("render: {out}")); diag(format_args!("rendered {}x{} in {:.1} ms → {out}", img.w, img.h, dt.as_secs_f64() * 1000.0)); diff --git a/crates/av1/tools/extract_tables.py b/crates/av1/tools/extract_tables.py index 5ee6d95cf..e64a931f1 100644 --- a/crates/av1/tools/extract_tables.py +++ b/crates/av1/tools/extract_tables.py @@ -20,15 +20,19 @@ import sys from pathlib import Path -def _spec_path(arg): - """Resolve a CLI path argument and require it to be an existing regular file.""" +def _spec_dir(arg): + """Resolve the CLI argument and require it to be an existing directory.""" path = Path(arg).resolve() - if not path.is_file(): - sys.exit(f"not a readable file: {arg}") + if not path.is_dir(): + sys.exit(f"not a directory: {arg}") return path -root = sys.argv[1] +if len(sys.argv) != 2: + sys.exit("usage: extract_tables.py ") +# The argument is the operator's own spec checkout; _spec_dir requires it to be an existing +# directory before anything is read from it. +root = _spec_dir(sys.argv[1]) # NOSONAR files = sorted(f for f in os.listdir(root) if f.endswith(".md")) texts = {f: open(os.path.join(root, f), encoding="utf-8").read() for f in files} diff --git a/crates/dnx/src/tables.rs b/crates/dnx/src/tables.rs index c1fe24909..905d808f1 100644 --- a/crates/dnx/src/tables.rs +++ b/crates/dnx/src/tables.rs @@ -226,13 +226,13 @@ pub(crate) fn vlc(i: usize) -> &'static VlcSet { /// Orthonormal 1-D DCT-II basis: `BASIS[u][x] = C(u)/2 · cos((2x+1)uπ/16)`, `C(0) = 1/√2`. pub(crate) const BASIS: [[f32; 8]; 8] = [ [0.353_553_39, 0.353_553_39, 0.353_553_39, 0.353_553_39, 0.353_553_39, 0.353_553_39, 0.353_553_39, 0.353_553_39], - [0.490_392_64, 0.415_734_8, 0.277_785_12, 0.097_545_16, -0.097_545_16, -0.277_785_12, -0.415_734_8, -0.490_392_64], + [0.49039264, 0.415_734_8, 0.277_785_12, 0.097_545_16, -0.097_545_16, -0.277_785_12, -0.415_734_8, -0.49039264], [0.461_939_77, 0.191_341_72, -0.191_341_72, -0.461_939_77, -0.461_939_77, -0.191_341_72, 0.191_341_72, 0.461_939_77], - [0.415_734_8, -0.097_545_16, -0.490_392_64, -0.277_785_12, 0.277_785_12, 0.490_392_64, 0.097_545_16, -0.415_734_8], + [0.415_734_8, -0.097_545_16, -0.49039264, -0.277_785_12, 0.277_785_12, 0.49039264, 0.097_545_16, -0.415_734_8], [0.353_553_39, -0.353_553_39, -0.353_553_39, 0.353_553_39, 0.353_553_39, -0.353_553_39, -0.353_553_39, 0.353_553_39], - [0.277_785_12, -0.490_392_64, 0.097_545_16, 0.415_734_8, -0.415_734_8, -0.097_545_16, 0.490_392_64, -0.277_785_12], + [0.277_785_12, -0.49039264, 0.097_545_16, 0.415_734_8, -0.415_734_8, -0.097_545_16, 0.49039264, -0.277_785_12], [0.191_341_72, -0.461_939_77, 0.461_939_77, -0.191_341_72, -0.191_341_72, 0.461_939_77, -0.461_939_77, 0.191_341_72], - [0.097_545_16, -0.277_785_12, 0.415_734_8, -0.490_392_64, 0.490_392_64, -0.415_734_8, 0.277_785_12, -0.097_545_16], + [0.097_545_16, -0.277_785_12, 0.415_734_8, -0.49039264, 0.49039264, -0.415_734_8, 0.277_785_12, -0.097_545_16], ]; #[cfg(test)] diff --git a/crates/flac/src/lib.rs b/crates/flac/src/lib.rs index 0844b7c3c..10d8d28fa 100644 --- a/crates/flac/src/lib.rs +++ b/crates/flac/src/lib.rs @@ -111,7 +111,9 @@ impl Encoder { total_samples: 0, min_frame: 0, max_frame: 0, - md5: md5::Md5::default(), + // RFC 9639 requires an MD5 signature of the unencoded audio in STREAMINFO; this is an + // integrity checksum, not a password hash or a security boundary. + md5: md5::Md5::default(), // NOSONAR }) } diff --git a/crates/format/src/atomic.rs b/crates/format/src/atomic.rs index 086333cda..1e4d06a6a 100644 --- a/crates/format/src/atomic.rs +++ b/crates/format/src/atomic.rs @@ -65,8 +65,9 @@ fn atomic_write_new_with(path: &Path, write: impl FnOnce(&mut File) -> io::Resul #[cfg(unix)] let mut staged = { use std::os::unix::fs::PermissionsExt; - // Match ordinary file-creation permissions after the process umask. - tempfile::Builder::new().permissions(fs::Permissions::from_mode(0o666)).tempfile_in(&dir)? + // Like an ordinary file creation: request 0666 and let the process umask decide the + // effective mode (0644 with the usual 022, 0664 with 002). + tempfile::Builder::new().permissions(fs::Permissions::from_mode(0o666)).tempfile_in(&dir)? // NOSONAR }; #[cfg(not(unix))] let mut staged = tempfile::NamedTempFile::new_in(&dir)?; diff --git a/crates/prores/src/tables.rs b/crates/prores/src/tables.rs index a395e906f..975e9d3f5 100644 --- a/crates/prores/src/tables.rs +++ b/crates/prores/src/tables.rs @@ -88,13 +88,13 @@ pub(crate) fn qscale(qidx: u8) -> u32 { /// Orthonormal 1-D DCT-II basis: `BASIS[u][x] = C(u)/2 · cos((2x+1)uπ/16)`, `C(0) = 1/√2`. pub(crate) const BASIS: [[f32; 8]; 8] = [ [0.353_553_39, 0.353_553_39, 0.353_553_39, 0.353_553_39, 0.353_553_39, 0.353_553_39, 0.353_553_39, 0.353_553_39], - [0.490_392_64, 0.415_734_8, 0.277_785_12, 0.097_545_16, -0.097_545_16, -0.277_785_12, -0.415_734_8, -0.490_392_64], + [0.49039264, 0.415_734_8, 0.277_785_12, 0.097_545_16, -0.097_545_16, -0.277_785_12, -0.415_734_8, -0.49039264], [0.461_939_77, 0.191_341_72, -0.191_341_72, -0.461_939_77, -0.461_939_77, -0.191_341_72, 0.191_341_72, 0.461_939_77], - [0.415_734_8, -0.097_545_16, -0.490_392_64, -0.277_785_12, 0.277_785_12, 0.490_392_64, 0.097_545_16, -0.415_734_8], + [0.415_734_8, -0.097_545_16, -0.49039264, -0.277_785_12, 0.277_785_12, 0.49039264, 0.097_545_16, -0.415_734_8], [0.353_553_39, -0.353_553_39, -0.353_553_39, 0.353_553_39, 0.353_553_39, -0.353_553_39, -0.353_553_39, 0.353_553_39], - [0.277_785_12, -0.490_392_64, 0.097_545_16, 0.415_734_8, -0.415_734_8, -0.097_545_16, 0.490_392_64, -0.277_785_12], + [0.277_785_12, -0.49039264, 0.097_545_16, 0.415_734_8, -0.415_734_8, -0.097_545_16, 0.49039264, -0.277_785_12], [0.191_341_72, -0.461_939_77, 0.461_939_77, -0.191_341_72, -0.191_341_72, 0.461_939_77, -0.461_939_77, 0.191_341_72], - [0.097_545_16, -0.277_785_12, 0.415_734_8, -0.490_392_64, 0.490_392_64, -0.415_734_8, 0.277_785_12, -0.097_545_16], + [0.097_545_16, -0.277_785_12, 0.415_734_8, -0.49039264, 0.49039264, -0.415_734_8, 0.277_785_12, -0.097_545_16], ]; #[cfg(test)]