From 43b512fee8f4964b6ad204c45188ae1da9564c48 Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Mon, 9 Feb 2026 21:04:03 +0000
Subject: [PATCH] refactor(security): improve sanitization logic and tests
 based on code review

- Change consecutive dots regex to simple replace for better precision
- Update tests to verify exact sanitized output
- Fix docstring syntax warning with raw string
- Add detailed comments explaining sanitization behavior
- All 43 tests pass (21 file upload + 22 filename utils)

Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>
---
 app/utils/filename_utils.py |  6 +++---
 tests/test_file_upload.py   | 20 +++++++++++++++-----
 2 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/app/utils/filename_utils.py b/app/utils/filename_utils.py
index a491613a..92751743 100644
--- a/app/utils/filename_utils.py
+++ b/app/utils/filename_utils.py
@@ -70,7 +70,7 @@ def get_unique_filename(original_path, check_exists_func=None):
 
 
 def sanitize_filename(filename):
-    """
+    r"""
     Sanitize a filename to ensure it's valid across different file systems
     and prevent path traversal attacks.
 
@@ -94,8 +94,8 @@ def sanitize_filename(filename):
     sanitized = re.sub(r"[^\w\-\. ]", "_", sanitized)
     
     # Remove or replace path traversal patterns
-    # Replace consecutive dots with a single underscore to prevent .. patterns
-    sanitized = re.sub(r"\.\.+", "_", sanitized)
+    # Replace specifically '..' to prevent path traversal while preserving single dots
+    sanitized = sanitized.replace("..", "_")
 
     # Replace multiple spaces/underscores with single ones
     sanitized = re.sub(r"__+", "_", sanitized)
diff --git a/tests/test_file_upload.py b/tests/test_file_upload.py
index 24feacb1..5687df29 100644
--- a/tests/test_file_upload.py
+++ b/tests/test_file_upload.py
@@ -296,11 +296,18 @@ class TestUploadSecurity:
         assert response.status_code == 200
         data = response.json()
         
-        # Backslashes should be removed or replaced
+        # On Unix systems, os.path.basename doesn't recognize Windows backslashes,
+        # so the full string goes to sanitize_filename which then:
+        # 1. Replaces backslashes with underscores
+        # 2. Replaces .. with underscores
+        # 3. Strips leading/trailing underscores
+        # Result: "windows_system32_config.pdf"
+        expected_filename = "windows_system32_config.pdf"
+        assert data["original_filename"] == expected_filename
+        # Verify no dangerous characters remain
         assert "\\" not in data["original_filename"]
         assert ".." not in data["original_filename"]
-        # Should contain sanitized version
-        assert "config.pdf" in data["original_filename"]
+        assert "/" not in data["original_filename"]
     
     def test_path_traversal_prevention_mixed_separators(self, client: TestClient, mock_celery_tasks):
         """Test handling of filenames with mixed path separators."""
@@ -315,11 +322,14 @@ class TestUploadSecurity:
         assert response.status_code == 200
         data = response.json()
         
-        # Should only keep the filename without any path components
+        # os.path.basename recognizes Unix / separators and extracts "file.pdf"
+        # sanitize_filename then ensures it's safe (already is in this case)
+        expected_filename = "file.pdf"
+        assert data["original_filename"] == expected_filename
+        # Verify no dangerous characters remain
         assert "/" not in data["original_filename"]
         assert "\\" not in data["original_filename"]
         assert ".." not in data["original_filename"]
-        assert "file.pdf" in data["original_filename"]
 
 
 @pytest.mark.integration