🛡️ Sentinel: [CRITICAL/HIGH] Fix path validation unhandled exceptions

.jules/sentinel.md

@@ -6,3 +6,13 @@
 **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 - Unhandled Exceptions in Path Handling
+**Vulnerability:** Core path validation routines (`validate_path_within`) and security middleware (`SafeStaticFiles`) failed to account for `None` inputs or unexpected types, leading to unhandled exceptions (`AttributeError`, `TypeError`). If an unhandled exception occurred during security validation, it could result in denial of service (500 errors) or, depending on upstream error handling, failing open to expose sensitive files.
+**Learning:** Security controls that rely on type-specific methods (like `pathlib.Path.resolve` or `str.lower()`) must strictly validate input types (e.g., `is not None`) and wrap operations in comprehensive `try...except` blocks that catch `TypeError` and `AttributeError` in addition to filesystem exceptions.
+**Prevention:** When building security gates, always assume malformed or null inputs. Explicitly check for `None` and ensure the exception handler catches all potential casting/method errors to fail closed securely.
+
+## 2025-02-23 - Catching Unhandled Exceptions in Path Validation
+**Vulnerability:** The core path validation routine `validate_path_within` could fail open if an upstream component supplied a malformed input (like `None`), which caused unexpected unhandled exceptions (`AttributeError`, `TypeError`) before the default `except` block.
+**Learning:** Type enforcement failures should be caught gracefully in security boundaries. When interacting with built-in path libraries, include `TypeError` and `AttributeError` in the exception block. Adding an arbitrary try/catch on safe built-ins (like `str.lower()`) constitutes "security theater".
+**Prevention:** Catch specific exceptions (`TypeError`, `AttributeError`) within the target library calls rather than wrapping large chunks of business logic in a generic `Exception` catch, which might accidentally mask explicitly raised HTTP errors like 403 Forbidden.

src/audioformation/server/app.py

@@ -23,8 +23,12 @@ class SafeStaticFiles(StaticFiles):
     """
 
     async def get_response(self, path: str, scope) -> Response:
+        if path is None:
+            raise HTTPException(status_code=400, detail="Invalid path")
+
         # 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"

src/audioformation/utils/security.py

@@ -81,7 +81,7 @@ def validate_path_within(path: Path, root: Path) -> bool:
             return resolved_path.is_relative_to(resolved_root)
 
         return False
-    except (ValueError, RuntimeError, OSError):
+    except (TypeError, ValueError, RuntimeError, AttributeError, OSError):
         return False
 
 

tests/test_security.py

@@ -127,6 +127,12 @@ def test_symlink_traversal_rejected(self, tmp_path: Path) -> None:
         # because it resolves outside the root.
         assert validate_path_within(traversal_path, sandbox) is False
 
+    def test_validate_path_within_none(self, tmp_path: Path) -> None:
+        assert validate_path_within(None, tmp_path) is False
+
+    def test_validate_path_within_none_root(self, tmp_path: Path) -> None:
+        assert validate_path_within(tmp_path, None) is False
+
 
 class TestRedactApiKeys:
     """Tests for API key redaction in logging."""