Skip to main content

max / audiofiles

Fix pre-launch audit backlog (AF-3..AF-10) Forge chop (AF-5): clamp EqualDivisions n to [1, total_frames] and use a u64 intermediate so an adversarial divisor count can neither overflow nor iterate unboundedly. Forge temp dirs (AF-4): unique per-run subdir (pid + counter) instead of a shared fixed path, with cleanup after import. Store (AF-6): cap original_name to 255 bytes on a UTF-8 boundary at both import sites before it reaches the DB and sync payload. Rhai (AF-3): wall-clock watchdog via on_progress that aborts a run past a 5s deadline, armed per-eval with an RAII guard in every hook runner (correct under the shared, reused engine). UI: gate the forge Chop button on a current preview so the slice count is always known before committing (AF-9); inner ScrollArea in the shared tool_window so panels don't clip on short screens (AF-8); log the cause when the device-profile registry falls back to empty (AF-7); replace user-facing Unicode arrows with "to" in pricing and rename-tag status (AF-10). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-06-12 21:10 UTC
Commit: ac167fed8e47e01dffefeec32dc4e38fd660de5f
Parent: 39d9f63
10 files changed, +191 insertions, -12 deletions
M crates/audiofiles-browser/src/backend/direct.rs +6 -1
@@ -93,7 +93,12 @@ impl DirectBackend {
93 93 fingerprint_index: Mutex::new(None),
94 94 similarity_index: Mutex::new(None),
95 95 #[cfg(feature = "device-profiles")]
96 - plugin_registry: audiofiles_rhai::create_registry().unwrap_or_else(|_| {
96 + plugin_registry: audiofiles_rhai::create_registry().unwrap_or_else(|e| {
97 + // Embedded (include_str!) profiles are compile-checked, so this
98 + // should never trip — but if it does, the conform/M8/MPC wedge
99 + // would silently show "No device profiles". Log so the empty
100 + // registry is diagnosable instead of mysterious.
101 + tracing::error!("Failed to load device-profile registry, falling back to empty: {e}");
97 102 audiofiles_rhai::registry::PluginRegistry::new()
98 103 }),
99 104 }
M crates/audiofiles-browser/src/state/library.rs +1 -1
@@ -168,7 +168,7 @@ impl BrowserState {
168 168 // isn't filtering on a tag that no longer exists.
169 169 self.search_filter.required_tags.retain(|t| t != old_tag);
170 170 self.apply_search();
171 - self.status = format!("Renamed tag: {old_tag} → {new_tag} ({count} sample{})",
171 + self.status = format!("Renamed tag: {old_tag} to {new_tag} ({count} sample{})",
172 172 if count == 1 { "" } else { "s" });
173 173 }
174 174 Err(e) => self.status = format!("Rename failed: {e}"),
M crates/audiofiles-browser/src/ui/forge_panel.rs +12 -3
@@ -165,15 +165,24 @@ fn draw_chop_section(ui: &mut egui::Ui, state: &mut BrowserState) {
165 165 if ui.add_enabled(!disabled, egui::Button::new("Preview slices")).clicked() {
166 166 state.forge_preview_slices();
167 167 }
168 + // AF-9: require a current preview before chopping so the user always
169 + // commits to a known slice count instead of an unknown number. Changing
170 + // any chop parameter clears the marks, which re-gates this button.
168 171 let slice_count = state.forge.slice_marks.len().saturating_sub(1);
169 - let chop_label = if slice_count > 0 {
170 - format!("Chop into {slice_count} slices")
172 + let has_preview = slice_count > 0;
173 + let chop_label = if has_preview {
174 + let noun = if slice_count == 1 { "slice" } else { "slices" };
175 + format!("Chop into {slice_count} {noun}")
171 176 } else {
172 177 "Chop".to_string()
173 178 };
174 - if ui.add_enabled(!disabled, egui::Button::new(chop_label)).clicked() {
179 + let chop = ui.add_enabled(!disabled && has_preview, egui::Button::new(chop_label));
180 + if chop.clicked() {
175 181 state.forge_apply_chop();
176 182 }
183 + if !has_preview {
184 + chop.on_hover_text("Preview the slices first to see how many will be created.");
185 + }
177 186 });
178 187 ui.label(
179 188 egui::RichText::new("Slices are written into a new folder beside this sample.")
M crates/audiofiles-browser/src/ui/sync_panel.rs +1 -1
@@ -713,7 +713,7 @@ fn draw_cap_picker(
713 713 BillingInterval::Annual => "year",
714 714 };
715 715 ui.label(format!(
716 - "{} → {}/{}",
716 + "{} to {}/{}",
717 717 format_cap(cap_bytes),
718 718 format_cents(price_cents),
719 719 interval_word,
M crates/audiofiles-browser/src/ui/widgets.rs +7 -1
@@ -103,7 +103,13 @@ pub fn tool_window<R>(
103 103 .collapsible(true)
104 104 .default_width(default_width)
105 105 .min_width(min_width)
106 - .show(ctx, |ui| add_contents(ui))
106 + // Inner scroll so tool surfaces taller than the window (or a short
107 + // screen) stay fully reachable instead of clipping their lower sections.
108 + .show(ctx, |ui| {
109 + egui::ScrollArea::vertical()
110 + .show(ui, |ui| add_contents(ui))
111 + .inner
112 + })
107 113 .and_then(|r| r.inner)
108 114 }
109 115
M crates/audiofiles-core/src/forge/chop.rs +34 -2
@@ -68,8 +68,15 @@ pub fn compute_slices(
68 68 starts
69 69 }
70 70 ChopMethod::EqualDivisions(n) => {
71 - let n = (*n).max(1);
72 - (0..n).map(|i| i * total_frames / n).collect()
71 + // Clamp to [1, total_frames]: more divisions than frames can't yield
72 + // more than `total_frames` distinct boundaries (extras dedup away),
73 + // and the cap bounds the iteration for adversarial n. `compute_slices`
74 + // is public and accepts arbitrary n. The u64 intermediate then keeps
75 + // `i * total_frames` from overflowing usize on 32-bit / multi-GB bufs.
76 + let n = (*n).clamp(1, total_frames);
77 + (0..n)
78 + .map(|i| ((i as u64 * total_frames as u64) / n as u64) as usize)
79 + .collect()
73 80 }
74 81 ChopMethod::BpmGrid {
75 82 bpm,
@@ -291,6 +298,31 @@ mod tests {
291 298 }
292 299
293 300 #[test]
301 + fn equal_divisions_non_multiple_has_trailing_slice() {
302 + // 10 mono frames into 3: 0,3,6 -> [0,3),[3,6),[6,10). Last slice carries
303 + // the remainder; tiling still covers the whole buffer with no gap.
304 + let samples = vec![0.0f32; 10];
305 + let slices = compute_slices(&samples, 1, 44100, &ChopMethod::EqualDivisions(3)).unwrap();
306 + assert_eq!(slices.len(), 3);
307 + assert_eq!(slices[0], Slice { start_frame: 0, end_frame: 3 });
308 + assert_eq!(slices[1], Slice { start_frame: 3, end_frame: 6 });
309 + assert_eq!(slices[2], Slice { start_frame: 6, end_frame: 10 });
310 + }
311 +
312 + #[test]
313 + fn equal_divisions_huge_n_is_clamped_not_pathological() {
314 + // AF-5: an adversarial n must neither overflow `i * total_frames` (u64
315 + // intermediate) nor iterate billions of times (n is clamped to
316 + // total_frames). 1000-frame buffer -> at most 1000 one-frame slices,
317 + // returning promptly.
318 + let samples = vec![0.0f32; 1000];
319 + let slices = compute_slices(&samples, 1, 44100, &ChopMethod::EqualDivisions(usize::MAX)).unwrap();
320 + assert_eq!(slices.len(), 1000);
321 + assert_eq!(slices.first().unwrap().start_frame, 0);
322 + assert_eq!(slices.last().unwrap().end_frame, 1000);
323 + }
324 +
325 + #[test]
294 326 fn bpm_grid_step_matches_tempo() {
295 327 // 120 BPM at 48k = 0.5s/beat = 24000 frames/beat.
296 328 let sample_rate = 48000;
M crates/audiofiles-core/src/forge/runner.rs +24 -1
@@ -127,6 +127,7 @@ pub fn import_chop(
127 127 let dir_name = format!("{}_slices", staging.stem);
128 128 let dir_node = vfs::create_directory(db, vfs_id, parent_id, &dir_name)?;
129 129
130 + let run_dir = staging.slices.first().and_then(|(_, p)| p.parent().map(Path::to_path_buf));
130 131 let mut written = 0usize;
131 132 for (slice_name, temp_path) in &staging.slices {
132 133 let import = store.import(temp_path, db);
@@ -135,6 +136,7 @@ pub fn import_chop(
135 136 vfs::create_sample_link(db, vfs_id, Some(dir_node), slice_name, &hash)?;
136 137 written += 1;
137 138 }
139 + cleanup_run_dir(run_dir.as_deref());
138 140
139 141 Ok(ChopResult { dir_node, slice_count: written })
140 142 }
@@ -212,10 +214,12 @@ pub fn import_conform(
212 214 parent_id: Option<NodeId>,
213 215 staging: ConformStaging,
214 216 ) -> Result<ConformResult, CoreError> {
217 + let run_dir = staging.temp_path.parent().map(Path::to_path_buf);
215 218 let import = store.import(&staging.temp_path, db);
216 219 let _ = std::fs::remove_file(&staging.temp_path);
217 220 let hash = import?;
218 221 vfs::create_sample_link(db, vfs_id, parent_id, &staging.out_name, &hash)?;
222 + cleanup_run_dir(run_dir.as_deref());
219 223 Ok(ConformResult { hash, overshoot: staging.overshoot })
220 224 }
221 225
@@ -268,12 +272,31 @@ fn format_rate_tag(rate: u32) -> String {
268 272 }
269 273
270 274 /// Per-process temp dir for forge intermediates; created on demand.
275 + /// Monotonic per-process counter distinguishing forge runs in this process;
276 + /// combined with the pid it uniquifies the temp run dir without an extra dep.
277 + static FORGE_RUN_SEQ: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0);
278 +
271 279 fn forge_temp_dir() -> Result<PathBuf, CoreError> {
272 - let dir = std::env::temp_dir().join("audiofiles_forge");
280 + let parent = std::env::temp_dir().join("audiofiles_forge");
281 + std::fs::create_dir_all(&parent).map_err(|e| io_err(&parent, e))?;
282 + // Unique per-run subdir. Forge temp filenames are derived from the source
283 + // stem, so two concurrent or retried runs on identically-named sources would
284 + // otherwise collide on the shared path. pid separates processes; the counter
285 + // separates runs within one. The import stage removes the dir once empty.
286 + let seq = FORGE_RUN_SEQ.fetch_add(1, std::sync::atomic::Ordering::Relaxed);
287 + let dir = parent.join(format!("run_{}_{}", std::process::id(), seq));
273 288 std::fs::create_dir_all(&dir).map_err(|e| io_err(&dir, e))?;
274 289 Ok(dir)
275 290 }
276 291
292 + /// Best-effort removal of a now-empty forge run dir after its temp files have
293 + /// been imported and deleted. Silently ignores a non-empty/missing dir.
294 + fn cleanup_run_dir(dir: Option<&Path>) {
295 + if let Some(dir) = dir {
296 + let _ = std::fs::remove_dir(dir);
297 + }
298 + }
299 +
277 300 #[cfg(test)]
278 301 mod tests {
279 302 use super::*;
M crates/audiofiles-core/src/store.rs +34 -2
@@ -157,7 +157,7 @@ impl SampleStore {
157 157 Err(rusqlite::Error::QueryReturnedNoRows) => crate::util::get_extension(path),
158 158 Err(e) => return Err(CoreError::Db(e)),
159 159 };
160 - let original_name = crate::util::get_filename(path, "unknown");
160 + let original_name = clamp_original_name(crate::util::get_filename(path, "unknown"));
161 161
162 162 // Probe duration from file headers (cheap, no full decode)
163 163 let duration = probe_duration(path);
@@ -351,6 +351,22 @@ pub fn sample_original_name(db: &Database, hash: &str) -> Result<String> {
351 351 query_sample_field(db, hash, "original_name")
352 352 }
353 353
354 + /// Clamp an imported file's display name before it enters the DB and the sync
355 + /// payload. The name is taken verbatim from disk, so bound its length (on a
356 + /// UTF-8 char boundary) to a filesystem-scale limit. SQL binds it as a
357 + /// parameter, so this is storage hygiene, not an injection guard.
358 + fn clamp_original_name(name: String) -> String {
359 + const MAX_BYTES: usize = 255;
360 + if name.len() <= MAX_BYTES {
361 + return name;
362 + }
363 + let mut end = MAX_BYTES;
364 + while end > 0 && !name.is_char_boundary(end) {
365 + end -= 1;
366 + }
367 + name[..end].to_string()
368 + }
369 +
354 370 // --- Loose-files mode ---
355 371
356 372 /// Look up the source_path for a sample (loose-files mode imports only).
@@ -648,7 +664,7 @@ impl SampleStore {
648 664 let hash = format!("{:x}", hasher.finalize());
649 665
650 666 let ext = crate::util::get_extension(path);
651 - let original_name = crate::util::get_filename(path, "unknown");
667 + let original_name = clamp_original_name(crate::util::get_filename(path, "unknown"));
652 668
653 669 // Probe duration from file headers (cheap, no full decode)
654 670 let duration = probe_duration(path);
@@ -694,6 +710,22 @@ mod tests {
694 710 }
695 711
696 712 #[test]
713 + fn clamp_original_name_caps_length_on_char_boundary() {
714 + // Short names pass through untouched.
715 + assert_eq!(clamp_original_name("kick.wav".to_string()), "kick.wav");
716 +
717 + // Over-long ASCII is capped to 255 bytes.
718 + let long = "a".repeat(1000);
719 + assert_eq!(clamp_original_name(long).len(), 255);
720 +
721 + // Multi-byte chars at the cap don't split mid-codepoint (valid UTF-8).
722 + let multibyte = "é".repeat(200); // 400 bytes
723 + let clamped = clamp_original_name(multibyte);
724 + assert!(clamped.len() <= 255);
725 + assert!(std::str::from_utf8(clamped.as_bytes()).is_ok());
726 + }
727 +
728 + #[test]
697 729 fn import_creates_file_and_row() {
698 730 let (dir, db, store) = setup();
699 731 let src = create_test_file(&dir, "kick.wav", b"fake audio data");
M crates/audiofiles-rhai/src/engine.rs +68
@@ -1,10 +1,44 @@
1 1 //! Rhai engine setup: sandboxed engine with registered types and host API.
2 2
3 + use std::cell::Cell;
4 + use std::time::{Duration, Instant};
5 +
3 6 use rhai::Engine;
4 7
5 8 use crate::host_api;
6 9 use crate::types::{self, RhaiExportContext, RhaiSampleInfo};
7 10
11 + /// Wall-clock ceiling for a single script run. `set_max_operations` already
12 + /// bounds interpreter ops, but this also caps total elapsed time so a script
13 + /// that spends a long time across many operations can't hang the rename/import
14 + /// thread.
15 + const SCRIPT_WALL_CLOCK_LIMIT: Duration = Duration::from_secs(5);
16 +
17 + thread_local! {
18 + // Deadline for the script currently running on this thread, if any. The
19 + // engine is shared and reused across hook calls, so timing is armed per-run
20 + // (not at engine creation) via [`arm_script_deadline`].
21 + static SCRIPT_DEADLINE: Cell<Option<Instant>> = const { Cell::new(None) };
22 + }
23 +
24 + /// RAII guard that disarms the per-thread script deadline on drop.
25 + #[must_use = "the deadline is only active while the guard is alive"]
26 + pub struct ScriptDeadlineGuard(());
27 +
28 + impl Drop for ScriptDeadlineGuard {
29 + fn drop(&mut self) {
30 + SCRIPT_DEADLINE.with(|d| d.set(None));
31 + }
32 + }
33 +
34 + /// Arm the wall-clock deadline for the next script eval on this thread. Hold the
35 + /// returned guard for the duration of the eval; the engine's `on_progress` hook
36 + /// aborts evaluation once the deadline passes.
37 + pub fn arm_script_deadline() -> ScriptDeadlineGuard {
38 + SCRIPT_DEADLINE.with(|d| d.set(Some(Instant::now() + SCRIPT_WALL_CLOCK_LIMIT)));
39 + ScriptDeadlineGuard(())
40 + }
41 +
8 42 /// Create a sandboxed Rhai engine with all custom types and host functions registered.
9 43 pub fn create_engine() -> Engine {
10 44 let mut engine = Engine::new();
@@ -17,6 +51,16 @@ pub fn create_engine() -> Engine {
17 51 engine.set_max_map_size(100);
18 52 engine.set_max_expr_depths(64, 32);
19 53
54 + // Wall-clock watchdog: abort once the per-run deadline (if armed) passes.
55 + // Fires between operations, so it bounds total script time on top of the
56 + // operation-count cap. No-op when no deadline is armed.
57 + engine.on_progress(|_ops| {
58 + let expired = SCRIPT_DEADLINE
59 + .with(|d| d.get())
60 + .is_some_and(|deadline| Instant::now() >= deadline);
61 + expired.then(|| "script exceeded its wall-clock time limit".into())
62 + });
63 +
20 64 // Suppress print/debug output from scripts
21 65 engine.on_print(|_| {});
22 66 engine.on_debug(|_, _, _| {});
@@ -57,6 +101,30 @@ mod tests {
57 101 }
58 102
59 103 #[test]
104 + fn engine_aborts_on_wall_clock_deadline() {
105 + // AF-3: arm an already-expired deadline so the first on_progress check
106 + // trips. The error must be ErrorTerminated (the wall-clock watchdog),
107 + // distinguishing it from the operation-count limit (ErrorTooManyOperations).
108 + let engine = create_engine();
109 + SCRIPT_DEADLINE.with(|d| d.set(Some(Instant::now() - Duration::from_secs(1))));
110 + let err = engine.eval::<()>("let x = 0; loop { x += 1; }").unwrap_err();
111 + SCRIPT_DEADLINE.with(|d| d.set(None));
112 + assert!(
113 + matches!(*err, rhai::EvalAltResult::ErrorTerminated(..)),
114 + "expected wall-clock termination, got {err:?}"
115 + );
116 + }
117 +
118 + #[test]
119 + fn arm_script_deadline_guard_disarms_on_drop() {
120 + {
121 + let _guard = arm_script_deadline();
122 + assert!(SCRIPT_DEADLINE.with(|d| d.get()).is_some(), "armed while guard alive");
123 + }
124 + assert!(SCRIPT_DEADLINE.with(|d| d.get()).is_none(), "disarmed after guard drop");
125 + }
126 +
127 + #[test]
60 128 fn engine_sample_info_getters_work() {
61 129 let engine = create_engine();
62 130 let mut scope = rhai::Scope::new();
M crates/audiofiles-rhai/src/hooks.rs +4
@@ -45,6 +45,7 @@ pub fn run_validate_sample(
45 45 let mut scope = Scope::new();
46 46 scope.push("info", info);
47 47
48 + let _deadline = crate::engine::arm_script_deadline();
48 49 engine
49 50 .eval_ast_with_scope::<bool>(&mut scope, ast)
50 51 .map_err(|e| PluginError::ScriptRuntime(e.to_string()))
@@ -62,6 +63,7 @@ pub fn run_transform_filename(
62 63 scope.push("name", name);
63 64 scope.push("ctx", ctx);
64 65
66 + let _deadline = crate::engine::arm_script_deadline();
65 67 engine
66 68 .eval_ast_with_scope::<String>(&mut scope, ast)
67 69 .map_err(|e| PluginError::ScriptRuntime(e.to_string()))
@@ -77,6 +79,7 @@ pub fn run_pre_export(
77 79 let mut scope = Scope::new();
78 80 scope.push("ctx", ctx);
79 81
82 + let _deadline = crate::engine::arm_script_deadline();
80 83 let _ = engine
81 84 .eval_ast_with_scope::<rhai::Dynamic>(&mut scope, ast)
82 85 .map_err(|e| PluginError::ScriptRuntime(e.to_string()))?;
@@ -93,6 +96,7 @@ pub fn run_post_export(
93 96 let mut scope = Scope::new();
94 97 scope.push("ctx", ctx);
95 98
99 + let _deadline = crate::engine::arm_script_deadline();
96 100 let _ = engine
97 101 .eval_ast_with_scope::<rhai::Dynamic>(&mut scope, ast)
98 102 .map_err(|e| PluginError::ScriptRuntime(e.to_string()))?;