From efc751c3d836b69b9613a8fd6e0f34fe45c1c65d Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Mon, 18 May 2026 21:29:37 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[security?= =?UTF-8?q?=20improvement]=20Prevent=20unhandled=20exceptions=20in=20SafeS?= =?UTF-8?q?taticFiles?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Modified `SafeStaticFiles` middleware in `src/audioformation/server/app.py`: - Added check for `None` path to avoid TypeError. - Wrapped path parsing logic in a `try...except` block to ensure exceptions are securely caught and converted to 400 Bad Request `HTTPException`. - Enhanced the restricted file check to reject any hidden files (starting with `.`) instead of just `.env`. Co-authored-by: socialawy <24765060+socialawy@users.noreply.github.com> --- .jules/sentinel.md | 5 +++++ src/audioformation/server/app.py | 20 ++++++++++++++------ uv.lock | 4 ++-- 3 files changed, 21 insertions(+), 8 deletions(-) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index c1caaeb..a8376c8 100644 --- a/.jules/sentinel.md +++ b/.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-18 - Prevent Unhandled Exceptions in Static File Routing +**Vulnerability:** The custom `SafeStaticFiles` middleware in `app.py` passed unsanitized `path` variables directly to `Path(str(path))`. If the underlying HTTP framework passed `None`, this would cause an unhandled `TypeError`, resulting in a 500 Internal Server Error, potentially disrupting service or leaking stack traces if debug mode is on. It also only checked for `.env` specifically, missing other sensitive hidden files. +**Learning:** Security middleware must validate all inputs robustly, including type safety (e.g., checking for `None`). Exception handling in security gates must "fail closed" by explicitly catching and converting parsing errors into safe 400 Bad Request responses. +**Prevention:** Always validate `None` types and wrap path/string manipulation inside security routing components with `try...except` blocks that fail gracefully. Broaden specific file checks (like `.env`) to generic pattern checks (like `.startswith(".")`) for better coverage. diff --git a/src/audioformation/server/app.py b/src/audioformation/server/app.py index 54243ef..1a8497f 100644 --- a/src/audioformation/server/app.py +++ b/src/audioformation/server/app.py @@ -23,12 +23,20 @@ class SafeStaticFiles(StaticFiles): """ 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 provided") + + try: + # Normalize path for check + p = Path(str(path).lower()) + if "00_config" in p.parts or p.name.startswith(".") or ".git" 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="Malformed path") return await super().get_response(path, scope) diff --git a/uv.lock b/uv.lock index e34eac4..7970295 100644 --- a/uv.lock +++ b/uv.lock @@ -270,7 +270,7 @@ requires-dist = [ { name = "jsonschema", specifier = ">=4.21,<5" }, { name = "midiutil", marker = "extra == 'midi'", specifier = ">=1.2,<2" }, { name = "mutagen", marker = "extra == 'm4b'", specifier = ">=1.47,<2" }, - { name = "mypy", marker = "extra == 'dev'", specifier = ">=1.0,<2" }, + { name = "mypy", marker = "extra == 'dev'", specifier = ">=1.0,<3" }, { name = "numpy", specifier = ">=1.26,<3" }, { name = "pre-commit", marker = "extra == 'dev'", specifier = ">=3.6,<5" }, { name = "pydub", specifier = ">=0.25,<1" }, @@ -280,7 +280,7 @@ requires-dist = [ { name = "pytest-cov", marker = "extra == 'dev'", specifier = ">=4.0,<8" }, { name = "python-dotenv", marker = "extra == 'cloud'", specifier = ">=1.0,<2" }, { name = "python-multipart", marker = "extra == 'server'", specifier = ">=0.0.27,<1" }, - { name = "ruff", marker = "extra == 'dev'", specifier = ">=0.1.9" }, + { name = "ruff", marker = "extra == 'dev'", specifier = ">=0.15.1" }, { name = "silero-vad", marker = "extra == 'vad'", specifier = ">=6.0,<7" }, { name = "soundfile", specifier = ">=0.12,<1" }, { name = "transformers", specifier = ">=4.44,<6" },