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
4 changes: 4 additions & 0 deletions .jules/sentinel.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,3 +65,7 @@
**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-09-02 - hmac.compare_digest DoS Vulnerability
**Vulnerability:** `hmac.compare_digest` throws a `TypeError` when comparing strings with non-ASCII characters, which could lead to an unhandled exception and Denial of Service (DoS) when used with user-provided HTTP headers like API keys.
**Learning:** Python's `hmac.compare_digest` does not support non-ASCII string comparisons.
**Prevention:** Always encode strings to bytes (e.g., `.encode('utf-8')`) before passing them to `hmac.compare_digest`, especially when the input is user-controlled.
3 changes: 2 additions & 1 deletion saas_web.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,9 @@ async def require_api_key(request: Request, call_next):
configured_keys = get_configured_api_keys()
if configured_keys and not (request.method == "GET" and request.url.path == "/"):
provided_key = request.headers.get("x-api-key", "")
provided_key_bytes = provided_key.encode("utf-8")
if not any(
hmac.compare_digest(provided_key, key) for key in configured_keys
hmac.compare_digest(provided_key_bytes, key.encode("utf-8")) for key in configured_keys

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

๐ŸŸจ Runtime keys bypass credential registry

require_api_key still consumes environment-backed secrets through get_configured_api_keys. Runtime API keys bypass the mandatory encrypted credential registry and its managed lifecycle.

Devin Review

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

):
return JSONResponse(
status_code=401,
Expand Down
25 changes: 25 additions & 0 deletions tests/test_saas_web.py
Original file line number Diff line number Diff line change
Expand Up @@ -1225,3 +1225,28 @@ def test_video_content_type_accepted_by_validator(self):

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


@unittest.skipUnless(
_HAS_FASTAPI, "fastapi not installed (optional integration dependency)"
)
class TestApiKeyAuthAsync(unittest.IsolatedAsyncioTestCase):
async def test_non_ascii_key_rejected_safely(self):
from starlette.requests import Request
import saas_web
import json

async def mock_call_next(request):
return "SUCCESS"

with patch.dict(os.environ, {"CODEC_CARVER_API_KEYS": "secret-key"}):
scope = {
"type": "http",
"method": "POST",
"path": "/shrink",
"headers": [(b"x-api-key", "non-ascii-รฉ".encode("utf-8"))],
}
request = Request(scope)
response = await saas_web.require_api_key(request, mock_call_next)
self.assertEqual(response.status_code, 401)
self.assertEqual(json.loads(response.body), {"error": "Invalid or missing API key"})
Comment on lines +1230 to +1252

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

๐Ÿ” Direct execution skips regression test

Placing the class after unittest.main() excludes it when this module runs directly. The non-ASCII regression then receives no coverage.

Devin Review

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

Loading