From 2febfaa0eb1a07f70b071376d7afa95c6945f53e Mon Sep 17 00:00:00 2001 From: Zach Vorhies Date: Sun, 13 Sep 2026 01:10:07 -0700 Subject: [PATCH] feat(fs): own temporary-directory lifecycle behind the facade Refs #182. Validate bounded prefixes, create owner-only Unix directories, retain explicit cleanup and ownership transfer semantics. --- Cargo.toml | 2 +- docs/temporary-directories.md | 32 +++++++++ src/platform/fs.rs | 5 ++ src/platform/fs/temporary.rs | 96 +++++++++++++++++++++++++ tests/temporary_directory.rs | 128 ++++++++++++++++++++++++++++++++++ 5 files changed, 262 insertions(+), 1 deletion(-) create mode 100644 docs/temporary-directories.md create mode 100644 src/platform/fs/temporary.rs create mode 100644 tests/temporary_directory.rs diff --git a/Cargo.toml b/Cargo.toml index a5b9f434..9739b257 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -43,7 +43,7 @@ secure-random = ["dep:getrandom"] # matching genuinely need a maintained backend -- there is no std equivalent, # and the on-disk correctness cost of a hand-rolled reflink ioctl is high -- # so those three stay private dependencies behind this facade. -fs = ["dep:dirs", "dep:globset", "dep:jwalk", "dep:reflink-copy"] +fs = ["dep:dirs", "dep:globset", "dep:jwalk", "dep:reflink-copy", "dep:tempfile"] # Filesystem-change-notification primitive: watcher construction and event # classification only, not debouncing, ignore-lists, or cache-invalidation # policy. Linux and macOS privately adapt the exact-pinned `notify` release; diff --git a/docs/temporary-directories.md b/docs/temporary-directories.md new file mode 100644 index 00000000..d2185a51 --- /dev/null +++ b/docs/temporary-directories.md @@ -0,0 +1,32 @@ +# Owned temporary directories + +The `fs` feature provides `platform::fs::TemporaryDirectory`, backed privately +by the already pinned temporary-file implementation. It does not expose the +backend guard or builder. `new()` selects OS temporary storage; +`in_directory(parent, prefix)` places staging under a trusted existing parent +so callers can subsequently rename on the same filesystem. + +Prefixes are limited to 128 UTF-8 bytes and exclude control characters, +separators, dot/parent components and Windows filename/drive metacharacters. +The generated suffix has 16 characters. The pinned backend limits randomized +creation to 65536 collision attempts and never overwrites a live directory. +Unix directories request owner-only 0700 permissions at creation (subject to +umask); Windows uses the backend's inherited access-control behavior. + +Paths become absolute at creation, so changing working directories does not +redirect cleanup. Drop performs best-effort recursive removal. `close()` reports +errors; it does not retry automatically after a failure. Save the path first if +explicit recovery is needed. `persist()` transfers the path and all cleanup +responsibility to the caller; cache validation, replacement and publication +remain product policy. + +Creation and cleanup are synchronous native filesystem operations, without a +wall-clock or cancellation guarantee. Avoid dropping large trees on async worker +threads. This is not a path-security sandbox: parent replacement, external temp +cleaners and hostile changes to owned paths can violate lifetime assumptions. +Close open child handles before Windows cleanup. Directory names are collision +avoidance, not authorization tokens. + +Issue #182 tracks native checks, application migration, release and exact +published adoption. Generic lifetime/permission/prefix tests live upstream; +FastLED must retain its cache-publication and download-policy integration tests. diff --git a/src/platform/fs.rs b/src/platform/fs.rs index 6aea593f..c60d1286 100644 --- a/src/platform/fs.rs +++ b/src/platform/fs.rs @@ -18,6 +18,11 @@ //! a maintained backend privately, because there is no std equivalent and a //! hand-rolled reflink ioctl is not something to get wrong silently. +#[cfg(feature = "fs")] +mod temporary; +#[cfg(feature = "fs")] +pub use temporary::{TemporaryDirectory, MAX_TEMP_PREFIX_BYTES}; + /// A descriptor the caller already owns and has asked us to write to. /// /// Deliberately opaque. Callers hold host-specific things -- a `RawFd` on diff --git a/src/platform/fs/temporary.rs b/src/platform/fs/temporary.rs new file mode 100644 index 00000000..5f1d10d1 --- /dev/null +++ b/src/platform/fs/temporary.rs @@ -0,0 +1,96 @@ +//! Owned native temporary directories; cache publication is caller policy. + +use std::{ + io, + path::{Path, PathBuf}, +}; + +/// Maximum UTF-8 prefix length, excluding the 16-character generated suffix. +pub const MAX_TEMP_PREFIX_BYTES: usize = 128; + +/// A newly created, exclusively owned directory with best-effort recursive +/// cleanup on drop. Creation uses the existing private temporary-file backend, +/// with at most 65536 collision attempts and a bounded generated name. +/// +/// Native creation/cleanup are synchronous and have no wall-clock or +/// cancellation guarantee. Do not drop a large populated directory on a +/// latency-sensitive async thread. Use [`Self::close`] to observe cleanup +/// errors, or [`Self::persist`] to transfer responsibility explicitly. +/// +/// The selected parent must be trusted. This is not a filesystem sandbox: +/// external cleaners, renamed/replaced paths and hostile parent-directory +/// mutations can invalidate ownership assumptions. Temporary names are not +/// security tokens. Close child file handles before cleanup on Windows. +#[derive(Debug)] +pub struct TemporaryDirectory { + inner: tempfile::TempDir, +} + +impl TemporaryDirectory { + /// Create under the operating system's temporary directory. + /// + /// # Errors + /// Reports native creation failures. No existing directory is overwritten. + pub fn new() -> io::Result { + Self::in_directory(&std::env::temp_dir(), "kernal-") + } + + /// Create directly under an existing caller-selected parent, preserving + /// same-filesystem staging. Relative parents become absolute at creation. + /// The prefix is UTF-8, at most 128 bytes, and cannot contain separators, + /// NUL or Windows filename/drive metacharacters. Empty prefixes are allowed. + /// + /// # Errors + /// Invalid prefixes fail before any filesystem operation. Native errors, + /// including a missing/non-directory parent, are returned unchanged. + pub fn in_directory(parent: &Path, prefix: &str) -> io::Result { + if prefix.len() > MAX_TEMP_PREFIX_BYTES + || matches!(prefix, "." | "..") + || prefix + .chars() + .any(|ch| ch.is_control() || "\\/<>:\"|?*".contains(ch)) + { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + "invalid temporary-directory prefix", + )); + } + let mut builder = tempfile::Builder::new(); + builder.prefix(prefix).rand_bytes(16); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + builder.permissions(std::fs::Permissions::from_mode(0o700)); + } + let inner = builder.tempdir_in(parent)?; + Ok(Self { inner }) + } + + /// The absolute owned directory path. Do not replace or rename it while + /// this guard owns cleanup; transfer ownership first when publishing. + pub fn path(&self) -> &Path { + self.inner.path() + } + + /// Transfer the existing path to the caller, disabling automatic cleanup. + #[must_use] + pub fn persist(self) -> PathBuf { + self.inner.keep() + } + + /// Recursively remove the owned directory, reporting native errors. + /// On failure, remaining contents are not retried automatically; save + /// `path().to_path_buf()` first if explicit recovery is needed. + /// + /// # Errors + /// Reports filesystem removal errors, including Windows sharing violations. + pub fn close(self) -> io::Result<()> { + self.inner.close() + } +} + +impl AsRef for TemporaryDirectory { + fn as_ref(&self) -> &Path { + self.path() + } +} diff --git a/tests/temporary_directory.rs b/tests/temporary_directory.rs new file mode 100644 index 00000000..5549c886 --- /dev/null +++ b/tests/temporary_directory.rs @@ -0,0 +1,128 @@ +#![cfg(feature = "fs")] + +use kernal_api::platform::fs::TemporaryDirectory; + +// Issue #182: generic ownership belongs in the kernel, not cache policy. +#[test] +fn owned_directory_cleanup_and_transfer() { + let parent = TemporaryDirectory::new().unwrap(); + let child = TemporaryDirectory::in_directory(parent.path(), "stage-").unwrap(); + let child_path = child.path().to_path_buf(); + assert_eq!(child_path.parent(), Some(parent.path())); + std::fs::write(child_path.join("payload"), b"data").unwrap(); + drop(child); + assert!(!child_path.exists()); + + let retained = TemporaryDirectory::in_directory(parent.path(), "keep-").unwrap(); + let retained_path = retained.persist(); + assert!(retained_path.is_dir()); + parent.close().unwrap(); + assert!(!retained_path.exists()); +} + +#[test] +fn invalid_prefixes_create_nothing() { + let parent = TemporaryDirectory::new().unwrap(); + for prefix in [ + "../escape", + "/absolute", + "a/b", + "a\\b", + "..", + ".", + "bad\0name", + "C:", + ] { + assert_eq!( + TemporaryDirectory::in_directory(parent.path(), prefix) + .unwrap_err() + .kind(), + std::io::ErrorKind::InvalidInput + ); + } + assert!(TemporaryDirectory::in_directory(parent.path(), &"x".repeat(129)).is_err()); + assert_eq!(std::fs::read_dir(parent.path()).unwrap().count(), 0); +} + +#[test] +fn creation_never_reuses_a_live_directory_and_missing_parent_fails() { + let parent = TemporaryDirectory::new().unwrap(); + let first = TemporaryDirectory::in_directory(parent.path(), "stage-").unwrap(); + let second = TemporaryDirectory::in_directory(parent.path(), "stage-").unwrap(); + assert_ne!(first.path(), second.path()); + assert!(first.path().is_absolute()); + assert_eq!( + TemporaryDirectory::in_directory(&parent.path().join("missing"), "stage-") + .unwrap_err() + .kind(), + std::io::ErrorKind::NotFound + ); +} + +#[test] +fn relative_parent_cleanup_survives_working_directory_change() { + const PROBE: &str = "KERNAL_TEMP_CWD_PROBE"; + if std::env::var_os(PROBE).is_some() { + let owned = TemporaryDirectory::in_directory(std::path::Path::new("."), "cwd-").unwrap(); + let path = owned.path().to_path_buf(); + assert!(path.is_absolute()); + std::env::set_current_dir("..").unwrap(); + owned.close().unwrap(); + assert!(!path.exists()); + return; + } + // Isolate process-global cwd changes from concurrently running tests. + let parent = TemporaryDirectory::new().unwrap(); + let status = std::process::Command::new(std::env::current_exe().unwrap()) + .args([ + "--exact", + "relative_parent_cleanup_survives_working_directory_change", + ]) + .env(PROBE, "1") + .current_dir(parent.path()) + .status() + .unwrap(); + assert!(status.success()); + assert_eq!(std::fs::read_dir(parent.path()).unwrap().count(), 0); +} + +#[cfg(unix)] +#[test] +fn private_permissions_and_cleanup_do_not_follow_child_symlinks() { + use std::os::unix::{fs::symlink, fs::PermissionsExt}; + let target = TemporaryDirectory::new().unwrap(); + std::fs::write(target.path().join("keep"), b"keep").unwrap(); + let owned = TemporaryDirectory::new().unwrap(); + assert_eq!( + std::fs::metadata(owned.path()) + .unwrap() + .permissions() + .mode() + & 0o777, + 0o700 + ); + symlink(target.path(), owned.path().join("link")).unwrap(); + owned.close().unwrap(); + assert_eq!(std::fs::read(target.path().join("keep")).unwrap(), b"keep"); +} + +#[cfg(windows)] +#[test] +fn explicit_cleanup_reports_an_exclusively_open_file() { + use std::os::windows::fs::OpenOptionsExt; + let owned = TemporaryDirectory::new().unwrap(); + let path = owned.path().to_path_buf(); + let file = std::fs::OpenOptions::new() + .create_new(true) + .write(true) + .share_mode(0) + .open(path.join("held")) + .unwrap(); + let result = owned.close(); + drop(file); + // Recover our exact test-owned directory after releasing the Windows handle. + if path.exists() { + std::fs::remove_dir_all(&path).unwrap(); + } + assert!(result.is_err()); +}