🛡️ Sentinel: [HIGH] Fix path traversal vulnerability in validate_path_within

.jules/sentinel.md

@@ -6,3 +6,8 @@
 **Vulnerability:** The custom `SafeStaticFiles` middleware in `src/audioformation/server/app.py` intended to block access to sensitive directories (like `00_CONFIG` and `.git`). However, it used `p = Path(path).lower()`, which raises an `AttributeError` because `pathlib.Path` objects lack a `.lower()` method. This effectively broke static file serving entirely (causing 500 errors) and represented a malformed security check. If such errors were ever 'swallowed' without raising an HTTP exception, it could result in 'failing open' and allowing access to sensitive files.
 **Learning:** Security checks that rely on path manipulation or normalization must be carefully tested for runtime exceptions. An unhandled exception in a security gate can either block legitimate traffic (Denial of Service) or, if caught improperly elsewhere, fail open. Always normalize the string representation of paths before converting them to `Path` objects.
 **Prevention:** Thoroughly test security middleware endpoints for both valid and invalid access attempts. Ensure that path string normalizations like `.lower()` are applied directly to the string before instantiating `Path(str(path).lower())`.
+
+## 2025-02-23 - Path Traversal Bypass in `validate_path_within` via String Manipulation
+**Vulnerability:** The `validate_path_within` helper in `src/audioformation/utils/security.py` used string-based checking (`os.path.abspath`) before actually enforcing relative-to bounds on resolved paths. This is problematic because string comparisons can be bypassed or misinterpret symlink chains if they're used to shortcut path resolution.
+**Learning:** Never rely on string-based path checking (like `startswith`) for validating that a path falls inside a directory. Symlinks, case insensitivity, and null bytes make string analysis inherently brittle for filesystem validation.
+**Prevention:** Always use `Path.resolve().is_relative_to()` as the only source of truth for bounds checking. Furthermore, `Path.resolve()` operations should catch a broad range of exceptions, including `TypeError` and `AttributeError`, to gracefully handle edge cases like `None` inputs or maliciously crafted paths that fail during resolution.

src/audioformation/utils/security.py

@@ -68,20 +68,14 @@
     This prevents path traversal and symlink bypass attacks.
     """
     try:
-        # Resolve to absolute paths first
-        abs_path = os.path.abspath(str(path))
-        abs_root = os.path.abspath(str(root))
-
-        # On Windows, abspath can have different casing for the drive letter.
-        # We normalize to lowercase for the preliminary string check.
-        if abs_path.lower().startswith(abs_root.lower()):
-            # String check passed, now do the rigorous resolution check
-            resolved_root = root.resolve()
-            resolved_path = path.resolve()
-            return resolved_path.is_relative_to(resolved_root)
+        if path is None or root is None:
+            return False
 
-        return False
-    except (ValueError, RuntimeError, OSError):
+        resolved_root = root.resolve()
+        resolved_path = path.resolve()
+
+        return resolved_path.is_relative_to(resolved_root)
+    except (TypeError, ValueError, RuntimeError, AttributeError, OSError):
         return False