Skip to main content

max / audiofiles

Give a forge run a temp dir nobody else can own The scratch path was $TMPDIR/audiofiles_forge, a fixed name in a world-writable directory, adopted with create_dir_all whether or not we made it. On astra it has belonged to the pom user since 2026-08-06, so every forge run as max failed PermissionDenied and eight tests across core and browser failed with it. tempfile creates with O_EXCL and a random suffix, so the directory cannot be pre-created, cannot be a symlink elsewhere, and does not collide after pid reuse.
Author: Max Johnson <me@maxj.phd> · 2026-08-12 17:15 UTC
Signed with PGP, not checked
Commit: 2e4f23b2a61247ecc25cce0ec7dfcdf379edcf08
Parent: 2dd9ee6
3 files changed, +37 insertions, -18 deletions
M Cargo.lock +1 -1
@@ -7338,7 +7338,7 @@
7338 7338
7339 7339 [[patch.unused]]
7340 7340 name = "docengine"
7341 - version = "0.5.0"
7341 + version = "0.7.0"
7342 7342
7343 7343 [[patch.unused]]
7344 7344 name = "kberg"
@@ -26,8 +26,8 @@
26 26 rayon = { workspace = true }
27 27 tagtree = { workspace = true }
28 28 synckit-config = { git = "https://makenot.work/git/max/synckit.git" }
29 -
30 - [dev-dependencies]
29 + # Runtime, not just tests, since 2026-08-12: forge's scratch directory has to be
30 + # created unguessably and exclusively. See `forge::runner::forge_temp_dir`.
31 31 tempfile = "3.25.0"
32 32
33 33 [lints]
@@ -398,22 +398,41 @@
398 398 }
399 399 }
400 400
401 - /// Per-process temp dir for forge intermediates; created on demand.
402 - /// Monotonic per-process counter distinguishing forge runs in this process;
403 - /// combined with the pid it uniquifies the temp run dir without an extra dep.
404 - static FORGE_RUN_SEQ: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0);
405 -
401 + /// A scratch directory for one forge run, created fresh and owned by us.
402 + ///
403 + /// Forge temp filenames are derived from the source stem, so two concurrent or
404 + /// retried runs on identically-named sources collide unless each gets a
405 + /// directory of its own. The import stage removes it once empty.
406 + ///
407 + /// # Why this is not a subdirectory of a shared parent
408 + ///
409 + /// It was `$TMPDIR/audiofiles_forge/run_<pid>_<seq>` until 2026-08-12, which
410 + /// uniquified the leaf and left the parent a fixed, predictable name in a
411 + /// world-writable directory, adopted with `create_dir_all` whether or not we
412 + /// made it. Whoever creates that path first owns it for every other user on the
413 + /// machine. On astra it was `pom`, mode 775, since 2026-08-06, so every forge
414 + /// run as `max` failed with `PermissionDenied` on the run dir underneath it, and
415 + /// eight tests across audiofiles-core and audiofiles-browser had been failing
416 + /// for it. A desktop app on a single-user laptop never sees this; a shared box
417 + /// sees it immediately, and it is a denial of service anybody with a shell can
418 + /// arrange deliberately.
419 + ///
420 + /// `tempfile` rather than a pid and a counter: it creates with `O_EXCL` and a
421 + /// random suffix, so the directory cannot be pre-created by somebody else, cannot
422 + /// be a symlink pointing somewhere we did not choose, and does not collide after
423 + /// pid reuse across a reboot.
424 + ///
425 + /// The `TempDir` guard is deliberately released rather than held. Cleanup here
426 + /// is `cleanup_run_dir`'s job after the import stage has taken what it wants,
427 + /// and a guard dropped at the end of this function would delete the directory
428 + /// before a single file was written into it.
406 429 fn forge_temp_dir() -> Result<PathBuf, CoreError> {
407 - let parent = std::env::temp_dir().join("audiofiles_forge");
408 - std::fs::create_dir_all(&parent).map_err(|e| io_err(&parent, e))?;
409 - // Unique per-run subdir. Forge temp filenames are derived from the source
410 - // stem, so two concurrent or retried runs on identically-named sources would
411 - // otherwise collide on the shared path. pid separates processes; the counter
412 - // separates runs within one. The import stage removes the dir once empty.
413 - let seq = FORGE_RUN_SEQ.fetch_add(1, std::sync::atomic::Ordering::Relaxed);
414 - let dir = parent.join(format!("run_{}_{}", std::process::id(), seq));
415 - std::fs::create_dir_all(&dir).map_err(|e| io_err(&dir, e))?;
416 - Ok(dir)
430 + let parent = std::env::temp_dir();
431 + tempfile::Builder::new()
432 + .prefix("audiofiles_forge_")
433 + .tempdir()
434 + .map(tempfile::TempDir::keep)
435 + .map_err(|e| io_err(&parent, e))
417 436 }
418 437
419 438 /// Best-effort removal of a now-empty forge run dir after its temp files have