🛡️ Sentinel: [HIGH] Fix DoS AttributeError and enforce fail-closed access in SafeStaticFiles

.jules/sentinel.md

@@ -1,4 +1,8 @@
 ## 2025-02-21 - Path Traversal in Mix Endpoint API Parameter
 **Vulnerability:** The `/projects/{project_id}/mix` API endpoint in `src/audioformation/server/routes.py` accepted a `music` parameter (meant to specify a filename within the `05_MUSIC/generated` directory) but directly passed it to `mix_project` without sanitization. This allowed directory traversal payloads like `../../../etc/passwd` to be used for background music resolution.
 **Learning:** Even internal API inputs that map strictly to filenames inside an expected directory must be sanitized. A simple check for file existence (`if not bg_music_path.exists():`) is insufficient as it confirms existence but allows looking outside the bounded directory.
-**Prevention:** Always use established sanitization helpers (like `sanitize_filename`) or bound checks (like `validate_path_within`) for any user-supplied string that forms part of a filesystem path. Ensure bypass parameters like `FORCE_NO_MUSIC` are handled before and mutually exclusively from sanitization.
+**Prevention:** Always use established sanitization helpers (like `sanitize_filename`) or bound checks (like `validate_path_within`) for any user-supplied string that forms part of a filesystem path. Ensure bypass parameters like `FORCE_NO_MUSIC` are handled before and mutually exclusively from sanitization.
+## 2024-05-24 - [DoS / Security Check Bypass in SafeStaticFiles]
+**Vulnerability:** The `SafeStaticFiles` middleware crashed with an `AttributeError` when constructing paths for access control checks because `pathlib.Path` objects lack a `.lower()` method. By calling `.lower()` before instantiating `Path()`, the crash was avoided. However, the initial fix implementation created a "fail-open" scenario where malformed requests could bypass security checks entirely.
+**Learning:** Security validations and access controls must always "fail closed". If an unexpected error or bad input occurs during validation, the system should actively deny access (e.g., raise HTTP 400 or 403) rather than passing silently.
+**Prevention:** Always verify that exception handlers surrounding security-critical logic explicitly raise errors rather than suppressing them with `pass`.

src/audioformation/server/app.py

@@ -24,10 +24,16 @@ class SafeStaticFiles(StaticFiles):
 
     async def get_response(self, path: str, scope) -> Response:
         # Normalize path for check
-        p = Path(path).lower()
-        if "00_config" in p.parts or p.name.startswith(".env") or ".git" in p.parts:
+        try:
+            p = Path(path.lower())
+            if "00_config" in p.parts or p.name.startswith(".env") or ".git" in p.parts:
+                raise HTTPException(
+                    status_code=403, detail="Access denied to sensitive resource"
+                )
+        except (TypeError, ValueError, AttributeError):
+            # Fail closed: deny access if path parsing fails
             raise HTTPException(
-                status_code=403, detail="Access denied to sensitive resource"
+                status_code=400, detail="Invalid path"
             )
 
         return await super().get_response(path, scope)