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())`.

## 2025-02-23 - Information Disclosure in Ingest Route Exception Handling
**Vulnerability:** In `src/audioformation/server/routes.py`, the `/projects/{project_id}/ingest` endpoint was returning internal exception details directly in the `HTTPException` detail field upon file upload failure (`raise HTTPException(status_code=500, detail=f"Upload failed: {e}")`). This could expose sensitive internal paths, library versions, or system state to a client.
**Learning:** Returning unhandled exception strings (like `str(e)`) in HTTP responses is a common source of information disclosure. Error handlers must 'fail closed' and sanitize external error messages while logging the actual cause internally.
**Prevention:** Always use internal logging (`logger.error` or `logger.exception`) to capture the full exception, and return a generic, safe error message to the client (e.g., `detail="Upload failed"`).
3 changes: 2 additions & 1 deletion src/audioformation/server/routes.py
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: {e}")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

When handling exceptions that result in a 500 Internal Server Error, it is more idiomatic and useful to use logger.exception(). This method automatically captures the full stack trace, which is essential for debugging the root cause of the failure. Additionally, using logger.exception("Upload failed") is preferred over f-strings for the log message to allow for cleaner logging and better integration with logging aggregators.

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

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

background_tasks.add_task(
_run_with_status,
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 (ValueError, RuntimeError, OSError, TypeError, AttributeError):
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