🛡️ Sentinel: [MEDIUM] Fix path handling 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())`.
+
+## 2025-02-24 - Unhandled Exceptions Leaking Information and DoS
+**Vulnerability:** Several endpoints in `routes.py` and the `validate_path_within` helper used broad `try...except Exception` blocks that caught standard library exceptions (`ValueError`, `TypeError`, `OSError`) and returned an HTTP 500 without logging the exception via `logger.exception(..., exc_info=True)`. Additionally, the `validate_path_within` helper didn't handle null inputs explicitly.
+**Learning:** Broad exception handling that returns HTTP errors without logging masks the underlying system failures (like disk full or invalid paths), complicating debugging. Path utilities must handle `None` gracefully and limit caught exceptions to expected standard library errors.
+**Prevention:** Always log caught exceptions using `logger.error(..., exc_info=True)` before returning a generic 500 error to the user, and ensure utility functions handle `NoneType` to fail safely and gracefully without crashing the server.

src/audioformation/server/routes.py

@@ -208,15 +208,25 @@
         target_dir = project_path / "05_MUSIC" / "generated"
     else:
         raise HTTPException(status_code=400, detail="Invalid category")
-    target_dir.mkdir(parents=True, exist_ok=True)
 
-    # Secure filename handling
-    safe_name = os.path.basename(file.filename)
-    # Additional strict regex guard
-    if not re.fullmatch(r"^[A-Za-z0-9_.-]+$", safe_name):
-        raise HTTPException(status_code=400, detail="Invalid filename characters")
+    try:
+        target_dir.mkdir(parents=True, exist_ok=True)
+    except Exception as e:
+        logger.error(f"Failed to create upload directory: {e}", exc_info=True)
+        raise HTTPException(status_code=500, detail="Upload failed")
 
-    dest = target_dir / safe_name
+    try:
+        # Secure filename handling
+        safe_name = os.path.basename(file.filename)
+        # Additional strict regex guard
+        if not re.fullmatch(r"^[A-Za-z0-9_.-]+$", safe_name):
+            raise HTTPException(status_code=400, detail="Invalid filename characters")
+
+        dest = target_dir / safe_name
+    except HTTPException:
+        raise
+    except Exception:
+        raise HTTPException(status_code=400, detail="Invalid path")
 
     # Check bounds using fixed validator
     if not validate_path_within(dest, target_dir):
@@ -229,7 +239,8 @@
             "path": str(dest.relative_to(project_path)).replace("\\", "/"),
             "filename": safe_name,
         }
-    except Exception:
+    except Exception as e:
+        logger.error(f"Failed to create upload directory: {e}", exc_info=True)
         raise HTTPException(status_code=500, detail="Upload failed")
 
 
@@ -253,21 +264,34 @@
     if request.reference_audio:
         # CODEQL FIX: Validate reference_audio is strictly within project
         # We do NOT call .resolve() here as it's a sink for unvalidated user input.
-        possible_ref = project_path / request.reference_audio
+        try:
+            possible_ref = project_path / request.reference_audio
+        except Exception:
+            raise HTTPException(status_code=400, detail="Invalid reference audio path")
+
         if not validate_path_within(possible_ref, project_path):
             raise HTTPException(status_code=400, detail="Invalid reference audio path")
 
         # Now it is safe to resolve if we need the absolute path
-        ref_path = possible_ref.resolve()
+        try:
+            ref_path = possible_ref.resolve()
+        except Exception:
+            raise HTTPException(status_code=400, detail="Invalid reference audio path")
+
         if not ref_path.exists():
             raise HTTPException(status_code=400, detail="Reference audio not found")
 
     # Create temp file for output
-    with tempfile.NamedTemporaryFile(suffix=".wav", delete=False) as tmp:
-        output_path = Path(tmp.name)
+    try:
+        with tempfile.NamedTemporaryFile(suffix=".wav", delete=False) as tmp:
+            output_path = Path(tmp.name)
+
+        # Validate temp path is within system temp directory
+        temp_root = Path(tempfile.gettempdir())
+    except Exception as e:
+        logger.error(f"Failed to create temp path: {e}", exc_info=True)
+        raise HTTPException(status_code=500, detail="Invalid temp path")
 
-    # Validate temp path is within system temp directory
-    temp_root = Path(tempfile.gettempdir())
     if not validate_path_within(output_path, temp_root):
         raise HTTPException(status_code=500, detail="Invalid temp path")
 

src/audioformation/utils/security.py

@@ -67,6 +67,9 @@ def validate_path_within(path: Path, root: Path) -> bool:
     Ensure `path` is within `root`, resolving all symlinks.
     This prevents path traversal and symlink bypass attacks.
     """
+    if path is None or root is None:
+        return False
+
     try:
         # Resolve to absolute paths first
         abs_path = os.path.abspath(str(path))
@@ -81,7 +84,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, OSError, AttributeError):
         return False