From 04d46d3534ef6f9c531b231de243f7463ed5efdb Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Tue, 12 May 2026 22:00:33 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[CRITICAL]?= =?UTF-8?q?=20Fix=20broken=20security=20check=20in=20SafeStaticFiles?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: socialawy <24765060+socialawy@users.noreply.github.com> --- .jules/sentinel.md | 6 +++++- src/audioformation/server/app.py | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 9bd8528..c1caaeb 100644 --- a/.jules/sentinel.md +++ b/.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. \ 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. +## 2025-02-23 - Broken Security Check in SafeStaticFiles +**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())`. diff --git a/src/audioformation/server/app.py b/src/audioformation/server/app.py index 9334beb..54243ef 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(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"