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-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.
20 changes: 14 additions & 6 deletions src/audioformation/server/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Comment on lines +32 to +35
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-high high

The check p.name.startswith(".") only validates the final component of the path. This means that while a hidden file like .env is blocked, a file inside a hidden directory (e.g., .ssh/id_rsa) would be allowed because the filename id_rsa does not start with a dot.

To ensure all hidden resources are blocked, you should check if any part of the path starts with a dot. This also makes the explicit ".git" in p.parts check redundant as it is covered by the more general hidden path check.

Suggested change
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"
)
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 HTTPException:
raise
except Exception:
raise HTTPException(status_code=400, detail="Malformed path")

return await super().get_response(path, scope)

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