Reject path separators in temp file/dir affixes (#14)

## What

`TempFile`/`TempDir` builders compose the name as
`{prefix}{random}{suffix}`. `create_with_affixes` passed that straight
to `dir.join(...)` with no validation, so a path separator in
`prefix`/`suffix` could make the name escape the target directory:

- `Path::join` treats an absolute fragment as a full replacement of
`dir` — prefix `/tmp/abs_` created a file **outside** the target dir.
- `../` is a parent traversal — escaped one level up into the parent.

Both were confirmed by failing tests before the fix.

## Fix

- Add `affix_is_safe` (in `lib.rs`): rejects any affix containing a path
separator via `std::path::is_separator` (`/` on all platforms, `\` on
Windows).
- Check it in both `create_with_affixes` before touching the filesystem.
File path returns `Error::InvalidFile`; dir path returns
`Error::InvalidDirectory`. No public API change.
- Matches the `tempfile` crate, which likewise forbids separators in
affixes.

Severity is low (affixes are programmer-supplied, not typically end-user
input), but it's an undocumented footgun worth closing.

## Tests

- `tests/adversarial_affixes.rs`:
- separator / `../` traversal / absolute affixes rejected for both file
and dir
- traversal containment verified by scanning the parent dir (nothing
leaks up)
- embedded NUL and a 5000-char prefix surface as a clean `Err` (no
panic, no infinite retry loop)
  - ordinary affixes still work and stay inside the target dir
- `src/random_name.rs`: 10k-iteration uniqueness + prefix guard for the
name generator, plus an `affix_is_safe` truth table

## Review order

1. `src/lib.rs` — the `affix_is_safe` helper + rationale.
2. `src/tempfile.rs` / `src/tempdir.rs` — the two checkpoints (identical
shape).
3. `tests/adversarial_affixes.rs` — the behavior being locked in.

## Verification

`cargo test`, `cargo test --features uuid`, `cargo clippy --all-features
--all-targets`, `cargo fmt --check` all clean.

## Note

Considered a `proptest` over generated affixes but skipped it: the
name-uniqueness property has no input domain to generate/shrink over,
and random affixes hitting the real filesystem is just slow fuzzing.
Explicit adversarial cases pin the risk better. Easy to add later if
generated-input coverage is wanted.

Author: sunsided

src/errors.rs

@@ -7,6 +7,13 @@ pub enum Error {
     InvalidDirectory,
     /// An invalid or missing file was specified.
     InvalidFile,
+    /// A name affix (prefix or suffix) contained a path separator.
+    ///
+    /// Affixes are composed into a single path component
+    /// (`{prefix}{random}{suffix}`); a separator would let the name escape the
+    /// target directory, so such affixes are rejected before any filesystem
+    /// access. The target directory itself is unaffected (and still valid).
+    InvalidAffix,
     /// An I/O error occurred.
     Io(std::io::Error),
 }
@@ -16,6 +23,9 @@ impl Display for Error {
         match self {
             Self::InvalidDirectory => write!(f, "An invalid directory was specified"),
             Self::InvalidFile => write!(f, "An invalid file name was specified"),
+            Self::InvalidAffix => {
+                write!(f, "A name prefix or suffix contained a path separator")
+            }
             Self::Io(e) => Display::fmt(e, f),
         }
     }

src/lib.rs

@@ -65,6 +65,20 @@ pub(crate) async fn path_is_file(path: &Path) -> bool {
         .unwrap_or(false)
 }
 
+/// Returns `true` if `affix` is safe to splice into a file name.
+///
+/// `prefix`/`suffix` are composed into a single path component
+/// (`{prefix}{random}{suffix}`). A path separator in either would let the
+/// composed name escape the target directory once joined onto it: `Path::join`
+/// replaces the base entirely when the fragment is absolute, and a fragment
+/// containing `..` resolves to a parent directory when the OS interprets the
+/// resulting path. We therefore reject any affix containing a separator
+/// ([`std::path::is_separator`], which is platform-aware: `/` everywhere, plus
+/// `\` on Windows) rather than letting it reach the filesystem.
+pub(crate) fn affix_is_safe(affix: &str) -> bool {
+    !affix.chars().any(std::path::is_separator)
+}
+
 /// Determines the ownership of a temporary file or directory.
 #[derive(Debug, Eq, PartialEq, Copy, Clone)]
 pub enum Ownership {

src/random_name.rs

@@ -61,4 +61,51 @@ mod tests {
         assert!(second.as_str().starts_with("test"));
         assert_ne!(first.as_str(), second.as_str());
     }
+
+    /// Property guard for the name generator: across many rapid, same-prefix
+    /// calls every name must be distinct (no collisions from the monotonic
+    /// counter or entropy mixing) and carry the requested prefix. Guards against
+    /// regressions in the counter / entropy / format wiring.
+    #[test]
+    fn names_are_unique_and_prefixed_over_many_iterations() {
+        use std::collections::HashSet;
+
+        const ITERATIONS: usize = 10_000;
+        let mut seen = HashSet::with_capacity(ITERATIONS);
+        for _ in 0..ITERATIONS {
+            let name = RandomName::new("px_");
+            assert!(
+                name.as_str().starts_with("px_"),
+                "missing prefix: {}",
+                name.as_str()
+            );
+            assert!(
+                seen.insert(name.as_str().to_string()),
+                "duplicate name generated: {}",
+                name.as_str()
+            );
+        }
+    }
+
+    /// Affixes containing a path separator must be flagged unsafe so they can be
+    /// rejected before reaching the filesystem (see `crate::affix_is_safe`).
+    #[test]
+    fn separator_bearing_affixes_are_unsafe() {
+        assert!(crate::affix_is_safe("ok_"));
+        assert!(crate::affix_is_safe(".log"));
+        assert!(crate::affix_is_safe("")); // empty is fine
+        assert!(crate::affix_is_safe("..")); // no separator -> still a plain name fragment
+
+        assert!(!crate::affix_is_safe("../"));
+        assert!(!crate::affix_is_safe("a/b"));
+        assert!(!crate::affix_is_safe("/etc/passwd"));
+
+        // Backslash is a path separator only on Windows. `is_separator` is
+        // platform-aware, so the expectation differs by target: rejected on
+        // Windows, an ordinary filename character elsewhere.
+        #[cfg(windows)]
+        assert!(!crate::affix_is_safe("a\\b"));
+        #[cfg(not(windows))]
+        assert!(crate::affix_is_safe("a\\b"));
+    }
 }

src/tempdir.rs

@@ -449,6 +449,12 @@ impl TempDir {
         if !crate::path_is_dir(root).await {
             return Err(Error::InvalidDirectory);
         }
+        // Affixes are name fragments, not paths: a separator would let the
+        // composed name escape `root` (`../` traversal, or an absolute prefix
+        // replacing it via `Path::join`).
+        if !crate::affix_is_safe(prefix) || !crate::affix_is_safe(suffix) {
+            return Err(Error::InvalidAffix);
+        }
         let mut last_err = None;
         for _ in 0..MAX_NAME_ATTEMPTS {
             let name = format!("{prefix}{}{suffix}", Self::random_core_name());

src/tempfile.rs

@@ -517,6 +517,12 @@ impl TempFile {
         if !crate::path_is_dir(dir).await {
             return Err(Error::InvalidDirectory);
         }
+        // Affixes are filename fragments, not paths: a separator would let the
+        // composed name escape `dir` (`../` traversal, or an absolute prefix
+        // replacing it via `Path::join`).
+        if !crate::affix_is_safe(prefix) || !crate::affix_is_safe(suffix) {
+            return Err(Error::InvalidAffix);
+        }
         let mut last_err = None;
         for _ in 0..MAX_NAME_ATTEMPTS {
             let name = format!("{prefix}{}{suffix}", Self::random_core_name());

tests/adversarial_affixes.rs

@@ -0,0 +1,140 @@
+//! Adversarial `prefix` / `suffix` handling.
+//!
+//! Affixes are composed into a single path component
+//! (`{prefix}{random}{suffix}`). These tests pin down the behavior for hostile
+//! affixes:
+//!
+//! * Path separators / traversal must be **rejected** so a name can never escape
+//!   the target directory (`../` traversal, or an absolute prefix that would
+//!   otherwise replace the target via `Path::join`).
+//! * Names the OS itself rejects (an embedded NUL, an over-long component) must
+//!   surface as a clean `Err`, never a panic or an infinite retry loop.
+//! * Ordinary affixes keep working and stay inside the target directory.
+
+use async_tempfile::{TempDir, TempFile};
+
+// --- path separators / traversal are rejected -------------------------------
+
+#[tokio::test]
+async fn file_prefix_with_path_separator_is_rejected() {
+    let dir = TempDir::new().await.unwrap();
+    let result = TempFile::builder()
+        .prefix("sub/evil_")
+        .dir(dir.dir_path().clone())
+        .create()
+        .await;
+    assert!(result.is_err(), "separator in prefix must be rejected");
+}
+
+#[tokio::test]
+async fn file_suffix_with_path_separator_is_rejected() {
+    let dir = TempDir::new().await.unwrap();
+    let result = TempFile::builder()
+        .suffix("/etc/passwd")
+        .dir(dir.dir_path().clone())
+        .create()
+        .await;
+    assert!(result.is_err(), "separator in suffix must be rejected");
+}
+
+#[tokio::test]
+async fn file_traversal_prefix_cannot_escape_target_dir() {
+    // A nested target dir so an escape would be observable: `../` would land the
+    // file in the parent (`root`) instead of `target`.
+    let root = TempDir::new().await.unwrap();
+    let target = TempDir::new_in(root.dir_path().as_path()).await.unwrap();
+
+    let result = TempFile::builder()
+        .prefix("../")
+        .dir(target.dir_path().clone())
+        .create()
+        .await;
+
+    assert!(result.is_err(), "`../` traversal must be rejected");
+
+    // Nothing leaked one level up into `root`.
+    let mut entries = tokio::fs::read_dir(root.dir_path()).await.unwrap();
+    while let Some(entry) = entries.next_entry().await.unwrap() {
+        assert_eq!(
+            entry.path(),
+            *target.dir_path(),
+            "unexpected entry escaped into the parent dir: {:?}",
+            entry.path()
+        );
+    }
+}
+
+#[tokio::test]
+async fn file_absolute_prefix_is_rejected() {
+    let dir = TempDir::new().await.unwrap();
+    let result = TempFile::builder()
+        .prefix("/tmp/abs_")
+        .dir(dir.dir_path().clone())
+        .create()
+        .await;
+    assert!(
+        result.is_err(),
+        "absolute (separator-bearing) prefix must be rejected"
+    );
+}
+
+#[tokio::test]
+async fn dir_prefix_with_path_separator_is_rejected() {
+    let root = TempDir::new().await.unwrap();
+    let result = TempDir::builder()
+        .prefix("../escaped_")
+        .dir(root.dir_path().clone())
+        .create()
+        .await;
+    assert!(result.is_err(), "separator in dir prefix must be rejected");
+}
+
+// --- OS-rejected names surface as a clean `Err`, never a panic --------------
+
+#[tokio::test]
+async fn file_nul_byte_in_suffix_errors_without_panic() {
+    let dir = TempDir::new().await.unwrap();
+    let result = TempFile::builder()
+        .suffix("\0bad")
+        .dir(dir.dir_path().clone())
+        .create()
+        .await;
+    assert!(result.is_err(), "embedded NUL must error, not panic");
+}
+
+#[tokio::test]
+async fn file_overlong_prefix_errors_without_panic() {
+    let dir = TempDir::new().await.unwrap();
+    let result = TempFile::builder()
+        .prefix("a".repeat(5000))
+        .dir(dir.dir_path().clone())
+        .create()
+        .await;
+    assert!(result.is_err(), "over-long name must error, not panic");
+}
+
+// --- ordinary affixes keep working and stay contained -----------------------
+
+#[tokio::test]
+async fn ordinary_affixes_still_work_and_stay_contained() {
+    let dir = TempDir::new().await.unwrap();
+    let file = TempFile::builder()
+        .prefix("ok_")
+        .suffix(".log")
+        .dir(dir.dir_path().clone())
+        .create()
+        .await
+        .unwrap();
+
+    let name = file
+        .file_path()
+        .file_name()
+        .unwrap()
+        .to_string_lossy()
+        .into_owned();
+    assert!(name.starts_with("ok_"), "name was {name}");
+    assert!(name.ends_with(".log"), "name was {name}");
+
+    // The file lives directly inside the requested directory.
+    assert_eq!(file.file_path().parent().unwrap(), dir.dir_path());
+}