utilities: sanitize filenames and isolate temp operations

1 files changed, 105 insertions, 48 deletions

diff --git a/src/commands/utilities.rs b/src/commands/utilities.rs
index 79f4508..2179ce9 100644
--- a/src/commands/utilities.rs
+++ b/src/commands/utilities.rs
@@ -66,15 +66,47 @@ pub(crate) fn stem_of(filename: &str) -> &str {
     }
 }
 
-fn temp_path_for(name: &str) -> String {
-    format!("{}/{}", crate::config::TEMP_PATH, name)
+/// A safe basename: no directory components, no characters Discord/Copyparty
+/// filenames commonly abuse, and never `.`/`..`.
+fn safe_basename(name: &str) -> String {
+    // Treat both separators as directory boundaries: Discord filenames may
+    // come from Windows clients even though the bot runs on Linux.
+    let base = name.rsplit(['/', '\\']).next().unwrap_or(name);
+    let cleaned = sanitize_filename(base)
+        .trim_matches(|c| c == '.' || c == ' ')
+        .to_string();
+    if cleaned.is_empty() {
+        "file".to_string()
+    } else {
+        cleaned
+    }
 }
 
-/// Best-effort removal of temp files; ignores missing files.
-async fn cleanup(paths: &[&str]) {
-    for p in paths {
-        let _ = tokio::fs::remove_file(p).await;
-    }
+/// Create a unique per-operation directory and return
+/// `(directory, input_path, output_path)`. Concurrent commands can never
+/// overwrite or delete each other's files.
+async fn temp_op(
+    original: &str,
+    output_name: &str,
+) -> std::io::Result<(String, String, String)> {
+    let safe = safe_basename(original);
+    let ext = ext_of(&safe)
+        .map(|ext| format!(".{ext}"))
+        .unwrap_or_default();
+    let dir = format!(
+        "{}/op_{}",
+        crate::config::TEMP_PATH,
+        uuid::Uuid::new_v4()
+    );
+    tokio::fs::create_dir_all(&dir).await?;
+    let input_path = format!("{dir}/input{ext}");
+    let output_path = format!("{dir}/{}", safe_basename(output_name));
+    Ok((dir, input_path, output_path))
+}
+
+/// Best-effort removal of a per-operation temp directory.
+async fn cleanup_dir(dir: &str) {
+    let _ = tokio::fs::remove_dir_all(dir).await;
 }
 
 /// Resolve the input source — either an autocomplete-picked copyparty file or
@@ -234,35 +266,32 @@ enum DeliverKind {
 /// file, returning the encoded bytes and the output filename.
 ///
 /// Mirrors the per-attachment path of Python's `Compression` message command,
-/// including sequential `TEMP_PATH` reuse so identical attachment stems don't
-/// collide (each iteration writes, encodes, then `cleanup`s before the next).
+/// but each invocation gets its own temp directory, so concurrent calls cannot
+/// overwrite or delete each other's files.
 async fn compress_attachment_bytes(
     ft: FileType,
     original: &str,
     input_bytes: &[u8],
 ) -> Option<(Vec<u8>, String)> {
-    let stem = stem_of(original).to_string();
-    let out_ext = compress_output_ext(ft, original);
+    let safe_original = safe_basename(original);
+    let stem = stem_of(&safe_original).to_string();
+    let out_ext = compress_output_ext(ft, &safe_original);
     let out_filename = format!("compressed_{stem}.{out_ext}");
-    let input_path = temp_path_for(original);
-    let output_path = temp_path_for(&out_filename);
-    let cleanup_paths = [input_path.as_str(), output_path.as_str()];
+    let (dir, input_path, output_path) = temp_op(&safe_original, &out_filename).await.ok()?;
 
     if tokio::fs::write(&input_path, input_bytes).await.is_err() {
+        cleanup_dir(&dir).await;
         return None;
     }
-    let mut cmd = match build_compress_command(ft, &input_path, &output_path) {
-        Some(c) => c,
-        None => {
-            cleanup(&cleanup_paths).await;
-            return None;
-        }
+    let Some(mut cmd) = build_compress_command(ft, &input_path, &output_path) else {
+        cleanup_dir(&dir).await;
+        return None;
     };
     let res = match cmd.output().await {
         Ok(out) if out.status.success() => tokio::fs::read(&output_path).await.ok(),
         _ => None,
     };
-    cleanup(&cleanup_paths).await;
+    cleanup_dir(&dir).await;
     res.map(|b| (b, out_filename))
 }
 
@@ -270,17 +299,14 @@ async fn compress_attachment_bytes(
 /// Uses Vorbis instead of Opus because symphonia 0.6 lacks an Opus decoder.
 /// Returns the compressed .ogg bytes on success.
 pub(crate) async fn compress_audio_bytes(input_bytes: &[u8], original_name: &str) -> Option<Vec<u8>> {
-    let safe_name = sanitize_filename(original_name);
-    let safe_name = if safe_name.is_empty() { "audio".to_string() } else { safe_name };
+    let safe_name = safe_basename(original_name);
     let stem = stem_of(&safe_name).to_string();
     let out_filename = format!("compressed_{stem}.ogg");
-    let input_path = temp_path_for(&safe_name);
-    let output_path = temp_path_for(&out_filename);
-    let cleanup_paths = [input_path.as_str(), output_path.as_str()];
+    let (dir, input_path, output_path) = temp_op(&safe_name, &out_filename).await.ok()?;
 
-    let _ = tokio::fs::create_dir_all(crate::config::TEMP_PATH).await;
     if let Err(e) = tokio::fs::write(&input_path, input_bytes).await {
         tracing::warn!("compress_audio_bytes: failed to write input: {e}");
+        cleanup_dir(&dir).await;
         return None;
     }
     tracing::debug!(
@@ -324,7 +350,7 @@ pub(crate) async fn compress_audio_bytes(input_bytes: &[u8], original_name: &str
             None
         }
     };
-    cleanup(&cleanup_paths).await;
+    cleanup_dir(&dir).await;
     res
 }
 
@@ -608,16 +634,20 @@ pub async fn compress(
             None => return Ok(()),
         };
 
-    let ft = file_type(&original_filename);
-    let stem = stem_of(&original_filename).to_string();
-    let out_ext = compress_output_ext(ft, &original_filename);
+    let safe_original = safe_basename(&original_filename);
+    let ft = file_type(&safe_original);
+    let stem = stem_of(&safe_original).to_string();
+    let out_ext = compress_output_ext(ft, &safe_original);
     let out_filename = format!("compressed_{stem}.{out_ext}");
-    let input_path = temp_path_for(&original_filename);
-    let output_path = temp_path_for(&out_filename);
-    let cleanup_paths = [input_path.as_str(), output_path.as_str()];
+    let Some((dir, input_path, output_path)) = temp_op(&safe_original, &out_filename).await.ok()
+    else {
+        ctx.say(lang::L_UTIL_COMPRESS_FAILED).await?;
+        return Ok(());
+    };
 
     if let Err(e) = tokio::fs::write(&input_path, &input_bytes).await {
         tracing::warn!("compress: failed to write input: {e}");
+        cleanup_dir(&dir).await;
         ctx.say(lang::L_UTIL_COMPRESS_FAILED).await?;
         return Ok(());
     }
@@ -625,7 +655,7 @@ pub async fn compress(
     let result = match build_compress_command(ft, &input_path, &output_path) {
         Some(mut cmd) => cmd.output().await,
         None => {
-            cleanup(&cleanup_paths).await;
+            cleanup_dir(&dir).await;
             ctx.say(lang::L_UTIL_UNSUPPORTED_TYPE).await?;
             return Ok(());
         }
@@ -634,7 +664,7 @@ pub async fn compress(
     match result {
         Ok(out) if out.status.success() => match tokio::fs::read(&output_path).await {
             Ok(out_bytes) => {
-                cleanup(&cleanup_paths).await;
+                cleanup_dir(&dir).await;
                 deliver(
                     &ctx,
                     out_bytes,
@@ -646,12 +676,12 @@ pub async fn compress(
             }
             Err(e) => {
                 tracing::warn!("compress: output missing after encode: {e}");
-                cleanup(&cleanup_paths).await;
+                cleanup_dir(&dir).await;
                 ctx.say(lang::L_UTIL_COMPRESS_FAILED).await?;
             }
         },
         _ => {
-            cleanup(&cleanup_paths).await;
+            cleanup_dir(&dir).await;
             ctx.say(lang::L_UTIL_COMPRESS_FAILED).await?;
         }
     }
@@ -689,15 +719,19 @@ pub async fn convert(
         return Ok(());
     }
 
-    let ft = file_type(&original_filename);
-    let stem = stem_of(&original_filename).to_string();
+    let safe_original = safe_basename(&original_filename);
+    let ft = file_type(&safe_original);
+    let stem = stem_of(&safe_original).to_string();
     let out_filename = format!("{stem}.{fmt}");
-    let input_path = temp_path_for(&original_filename);
-    let output_path = temp_path_for(&out_filename);
-    let cleanup_paths = [input_path.as_str(), output_path.as_str()];
+    let Some((dir, input_path, output_path)) = temp_op(&safe_original, &out_filename).await.ok()
+    else {
+        ctx.say(lang::L_UTIL_CONVERT_FAILED).await?;
+        return Ok(());
+    };
 
     if let Err(e) = tokio::fs::write(&input_path, &input_bytes).await {
         tracing::warn!("convert: failed to write input: {e}");
+        cleanup_dir(&dir).await;
         ctx.say(lang::L_UTIL_CONVERT_FAILED).await?;
         return Ok(());
     }
@@ -714,7 +748,7 @@ pub async fn convert(
             c
         }
         FileType::Unknown => {
-            cleanup(&cleanup_paths).await;
+            cleanup_dir(&dir).await;
             ctx.say(lang::L_UTIL_UNSUPPORTED_TYPE).await?;
             return Ok(());
         }
@@ -723,7 +757,7 @@ pub async fn convert(
     match cmd.output().await {
         Ok(out) if out.status.success() => match tokio::fs::read(&output_path).await {
             Ok(out_bytes) => {
-                cleanup(&cleanup_paths).await;
+                cleanup_dir(&dir).await;
                 deliver(
                     &ctx,
                     out_bytes,
@@ -735,17 +769,17 @@ pub async fn convert(
             }
             Err(e) => {
                 tracing::warn!("convert: output missing after encode: {e}");
-                cleanup(&cleanup_paths).await;
+                cleanup_dir(&dir).await;
                 ctx.say(lang::L_UTIL_CONVERT_FAILED).await?;
             }
         },
         Ok(out) => {
             let stderr = String::from_utf8_lossy(&out.stderr);
-            cleanup(&cleanup_paths).await;
+            cleanup_dir(&dir).await;
             ctx.say(lang::fmt_util_convert_err(&stderr)).await?;
         }
         Err(e) => {
-            cleanup(&cleanup_paths).await;
+            cleanup_dir(&dir).await;
             ctx.say(lang::fmt_util_convert_err(&e.to_string())).await?;
         }
     }
@@ -1017,3 +1051,26 @@ pub fn commands() -> Vec<poise::Command<Data, Error>> {
         message_download(),
     ]
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn safe_basename_strips_directories() {
+        assert_eq!(safe_basename("/music/foo.mp3"), "foo.mp3");
+        assert_eq!(safe_basename("../../etc/passwd"), "passwd");
+        assert_eq!(
+            safe_basename(r"..\..\windows\system32\evil.exe"),
+            "evil.exe"
+        );
+        assert_eq!(safe_basename(r"C:\Users\x\song.mp3"), "song.mp3");
+    }
+
+    #[test]
+    fn safe_basename_never_returns_dot_entries() {
+        assert_eq!(safe_basename(".."), "file");
+        assert_eq!(safe_basename("."), "file");
+        assert_eq!(safe_basename(""), "file");
+    }
+}