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 @@ -65,3 +65,8 @@
**Vulnerability:** Path traversal in `media_shrinker.py` via unresolved `..` segments or symlink escapes before deriving conversion output paths.
**Learning:** `Path.relative_to()` is only a lexical containment check unless both the source and root have first been resolved into canonical absolute paths. Relative paths and symlinks can otherwise bypass root-boundary assumptions.
**Prevention:** Resolve both source and root once, reject sources outside the resolved root with a sanitized `MediaShrinkerError`, and derive `rel_source` from the resolved paths before planning outputs.

## 2024-10-13 - [DoS Risk via hmac.compare_digest with Non-ASCII Strings]
**Vulnerability:** Sending non-ASCII characters in an HTTP header compared via `hmac.compare_digest` raises an unhandled `TypeError` ("comparing strings with non-ASCII characters is not supported"). This allows a malicious user to crash the worker process or trigger 500 errors by repeatedly sending payloads with non-ASCII characters, resulting in a Denial of Service (DoS).
**Learning:** `hmac.compare_digest` safely compares standard ASCII strings, but strictly rejects Python strings containing non-ASCII characters.
**Prevention:** Always encode user-provided strings (e.g., `.encode('utf-8')`) before comparing them using `hmac.compare_digest` to ensure safe byte-level comparison and prevent exception-based DoS.
2 changes: 1 addition & 1 deletion saas_web.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ async def require_api_key(request: Request, call_next):
if configured_keys and not (request.method == "GET" and request.url.path == "/"):
provided_key = request.headers.get("x-api-key", "")
if not any(
hmac.compare_digest(provided_key, key) for key in configured_keys
hmac.compare_digest(provided_key.encode('utf-8'), key.encode('utf-8')) for key in configured_keys
):
return JSONResponse(
status_code=401,
Expand Down
13 changes: 13 additions & 0 deletions tests/test_saas_web.py
Original file line number Diff line number Diff line change
Expand Up @@ -1222,6 +1222,19 @@ def test_video_content_type_accepted_by_validator(self):
)
)

class HMACTest(unittest.TestCase):
def test_non_ascii_header_does_not_crash(self):
import os
from unittest.mock import patch
from fastapi.testclient import TestClient
Comment on lines +1225 to +1229

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

πŸ” Authentication test lacks optional-dependency guard

Without the web extra installed, HMACTest imports FastAPI instead of skipping like the other web tests. The stdlib-only test suite then fails during this test.

Devin Review


Was this helpful? React with πŸ‘ or πŸ‘Ž to provide feedback.

import saas_web

client = TestClient(saas_web.app)
with patch.dict(os.environ, {"CODEC_CARVER_API_KEYS": "secret"}):
# Pass header as raw bytes to prevent test client decoding errors
response = client.get("/jobs/missing", headers=[(b'x-api-key', 'ν•œκΈ€ν‚€'.encode('utf-8'))])
self.assertEqual(response.status_code, 401)


if __name__ == "__main__":
unittest.main()
Loading