🛡️ Sentinel: [CRITICAL/HIGH] Fix path validation vulnerability in SafeStaticFiles

.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())`.
+
+## 2024-05-24 - [Fix path validation vulnerability in SafeStaticFiles]
+**Vulnerability:** SafeStaticFiles didn't prevent access to hidden files and directories because it only checked if the file name started with ".env". It also did not handle None values or unexpected exceptions properly, violating fail-closed security principles.
+**Learning:** Checking for specific hidden files like ".env" leaves the application vulnerable to other hidden files or directories being accessed. Furthermore, path validation logic must handle `None` values and unexpected exceptions correctly to fail closed and ensure security.
+**Prevention:** In FastAPI apps and when serving static files, explicitly check `path is None` and wrap parsing logic in a `try...except` block, ensuring you raise an `HTTPException` on `Exception`. Check for any hidden file or directory access by verifying that no path parts start with a dot (`any(part.startswith(".") for part in p.parts)`).

src/audioformation/server/app.py

@@ -20,15 +20,24 @@ class SafeStaticFiles(StaticFiles):
     - 00_CONFIG directory (contains secrets/API keys)
     - .env files
     - .git directories
+    - Any hidden files or directories
     """
 
     async def get_response(self, path: str, scope) -> Response:
-        # Normalize path for check
-        p = Path(str(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"
-            )
+        if path is None:
+            raise HTTPException(status_code=400, detail="Invalid path")
+
+        try:
+            # Normalize path for check
+            p = Path(str(path).lower())
+            if "00_config" in p.parts or ".git" in p.parts or any(part.startswith(".") for part in p.parts):
+                raise HTTPException(
+                    status_code=403, detail="Access denied to sensitive resource"
+                )
+        except HTTPException:
+            raise
+        except Exception:
+            raise HTTPException(status_code=400, detail="Invalid path")
 
         return await super().get_response(path, scope)