Skip to content
Closed
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
7 changes: 6 additions & 1 deletion .jules/sentinel.md
Original file line number Diff line number Diff line change
@@ -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.
**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`.
2 changes: 1 addition & 1 deletion src/audioformation/server/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Comment on lines +27 to 28
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

The fix correctly addresses the AttributeError by ensuring .lower() is called on the string before Path instantiation. However, the security check can be further hardened:

  1. Backslash Normalization: On Linux systems, backslashes in the path might not be treated as separators by pathlib.Path, potentially bypassing the parts check if a client sends Windows-style paths (e.g., 00_config\\secrets.json). Normalizing separators to / ensures consistent behavior across platforms.
  2. Robust .env Check: The current check p.name.startswith(".env") only validates the final component of the path. If a directory is named .env.secrets, files inside it (e.g., .env.secrets/config) would not be blocked. Checking all path components for the .env prefix is more secure.
Suggested change
p = Path(path.lower())
if "00_config" in p.parts or p.name.startswith(".env") or ".git" in p.parts:
p = Path(path.lower().replace("\\", "/"))
if any(part == "00_config" or part == ".git" or part.startswith(".env") for part in p.parts):

raise HTTPException(
status_code=403, detail="Access denied to sensitive resource"
Expand Down
Loading