Skip to content

🛡️ Sentinel: [MEDIUM] Fix Exception Details Leakage in API response #70

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
socialawy-dev wants to merge 1 commit into from
Closed

🛡️ Sentinel: [MEDIUM] Fix Exception Details Leakage in API response #70

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 5 additions & 1 deletion .jules/sentinel.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.
**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-21 - Exception Details Leakage in Ingest Endpoint
**Vulnerability:** The `/projects/{project_id}/ingest` API endpoint in `src/audioformation/server/routes.py` returned the raw exception string in the `detail` field of the HTTP 500 response (`Upload failed: {e}`).
**Learning:** Exposing internal exception details (like file system errors, internal state, or stack traces) to the client can give attackers insight into the internal workings of the application. Error handling should log the details server-side and return generic error messages to the client.
**Prevention:** Always log the exception object using a logger (e.g., `logger.error()`) internally, and return a sanitized, generic error message (e.g., `detail="Upload failed"`) to the client in the `HTTPException`.
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Apr 24, 2026

Choose a reason for hiding this comment

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

medium

The prevention advice should recommend using logger.exception() instead of logger.error() when handling exceptions. logger.exception() is specifically designed for use inside except blocks to capture the full traceback, which is essential for internal troubleshooting while still allowing the API to return a generic error message to the client.

3 changes: 2 additions & 1 deletion src/audioformation/server/routes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,8 @@ async def ingest_files(
shutil.copyfileobj(file.file, buffer)
except Exception as e:
shutil.rmtree(tmp_dir, ignore_errors=True)
raise HTTPException(status_code=500, detail=f"Upload failed: {e}")
logger.error(f"Upload failed for project {project_id}: {e}")
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Apr 24, 2026

Choose a reason for hiding this comment

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

medium

When catching an exception to log it, it is best practice to use logger.exception(). This method automatically captures the full stack trace, which is much more helpful for debugging than just the exception string. Using logger.error() with an f-string only logs the message and the exception's string representation, losing the traceback context.

Suggested change
logger.error(f"Upload failed for project {project_id}: {e}")
logger.exception(f"Upload failed for project {project_id}")

raise HTTPException(status_code=500, detail="Upload failed")

background_tasks.add_task(
_run_with_status,
Expand Down
Loading