Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .jules/sentinel.md
Original file line number Diff line number Diff line change
Expand Up @@ -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-27 - Fail Closed When Handling `None` paths and Hidden Files in StaticFiles
**Vulnerability:** The custom `SafeStaticFiles` implementation used for static file serving was explicitly checking for specific sensitive files/directories (like `.env`, `.git`) but not generally blocking *all* hidden files or resolving when a file name might implicitly have a dot prefix. In addition, when path resolution handlers like `validate_path_within` received `None` (for example, if a path component was missing or malformed), they might throw unexpected `TypeError` or `AttributeError` exceptions instead of properly failing closed and raising `False` or an HTTP 400.
**Learning:** Hardcoding a blocklist of hidden files (e.g. `.env`, `.git`) is a losing battle because it does not cover other sensitive hidden artifacts (e.g. `.htpasswd`, `.DS_Store`, `.ssh`). Additionally, failure to catch type-level exceptions (`TypeError`, `AttributeError`) on `Path` object construction or resolution from unexpected external inputs (like `None` or integers instead of strings) can bypass standard path traversal or allow-list checks.
**Prevention:** Always use `any(part.startswith('.') for part in p.parts)` in `pathlib` parts to block all hidden files comprehensively. Ensure that path validation and security check functions (like `validate_path_within`) explicitly handle all unexpected data types (catching `TypeError`, `AttributeError`) to fail securely (`return False`) rather than crashing or failing open.
5 changes: 4 additions & 1 deletion src/audioformation/server/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,12 @@ class SafeStaticFiles(StaticFiles):
"""

async def get_response(self, path: str, scope) -> Response:
if path is None:
raise HTTPException(status_code=400, detail="Path cannot be None")

# 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:
if "00_config" in p.parts or any(part.startswith(".") for part in p.parts):
raise HTTPException(
status_code=403, detail="Access denied to sensitive resource"
)
Comment on lines 30 to 34
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-medium medium

If the path parameter contains null bytes (e.g., foo\x00bar), instantiating Path(str(path).lower()) will raise a ValueError: embedded null byte. Since this exception is not caught within get_response, it will result in an unhandled 500 Internal Server Error (or application crash) rather than a clean client-side error.

To ensure consistent fail-closed behavior and prevent potential Denial of Service (DoS) or crash vectors, wrap the path initialization and checks in a try...except ValueError block and raise a 400 Bad Request or 403 Forbidden exception.

Suggested change
p = Path(str(path).lower())
if "00_config" in p.parts or p.name.startswith(".env") or ".git" in p.parts:
if "00_config" in p.parts or any(part.startswith(".") for part in p.parts):
raise HTTPException(
status_code=403, detail="Access denied to sensitive resource"
)
try:
p = Path(str(path).lower())
if "00_config" in p.parts or any(part.startswith(".") for part in p.parts):
raise HTTPException(
status_code=403, detail="Access denied to sensitive resource"
)
except ValueError:
raise HTTPException(
status_code=400, detail="Invalid path format"
)

Expand Down
2 changes: 1 addition & 1 deletion src/audioformation/utils/security.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down
4 changes: 2 additions & 2 deletions uv.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading