From f79cd5325874e89dbf3ac6c623c5e5940d2bed90 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Thu, 16 Apr 2026 22:13:01 +0000 Subject: [PATCH] fix: path validation logic error causing `AttributeError` Fixed `SafeStaticFiles` middleware in `app.py` where `p = Path(path).lower()` caused an `AttributeError` because `Path` objects don't have a `.lower()` method. This would lead to 500 errors or bypassed checks instead of returning a proper 403 Forbidden for restricted directories. Changed to lowercasing the string *before* converting to `Path`: `p = Path(path.lower())`. Also documented this finding in `.jules/sentinel.md`. Co-authored-by: socialawy <24765060+socialawy@users.noreply.github.com> --- .jules/sentinel.md | 7 ++++++- src/audioformation/server/app.py | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 9bd8528..aa72b27 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -1,4 +1,9 @@ ## 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. \ No newline at end of file +**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 - [Path Validation Bypass via AttributeError] +**Vulnerability:** In `src/audioformation/server/app.py`, the `SafeStaticFiles` middleware attempted to normalize paths for security checks using `p = Path(path).lower()`. However, `Path` objects don't have a `.lower()` method, causing an `AttributeError` when a path is processed. This completely bypasses the security check because `SafeStaticFiles` could raise an unhandled exception or fail to block access to sensitive files depending on how the error is handled upstream. +**Learning:** Security validations that crash or raise exceptions before completing their checks can lead to open failure modes or denial of service. The path normalization must occur on the string *before* conversion to a `Path` object to successfully block restricted directories like `00_config`. +**Prevention:** Always verify string manipulations are applied to string objects and validate that security checks do not crash and fail open. Convert path strings to lower case before instantiating `Path` objects (e.g. `Path(path.lower())`) to avoid `AttributeError`. diff --git a/src/audioformation/server/app.py b/src/audioformation/server/app.py index 9334beb..5cf513b 100644 --- a/src/audioformation/server/app.py +++ b/src/audioformation/server/app.py @@ -24,7 +24,7 @@ class SafeStaticFiles(StaticFiles): async def get_response(self, path: str, scope) -> Response: # Normalize path for check - p = Path(path).lower() + 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"