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-05-24 - hmac.compare_digest Non-ASCII DoS
**Vulnerability:** `hmac.compare_digest()` crashes with a `TypeError` when comparing strings containing non-ASCII characters. Since `x-api-key` is user input, this causes an unhandled exception and a Denial of Service.
**Learning:** Python's `hmac.compare_digest` does not support non-ASCII string comparisons.
**Prevention:** Always encode user-controlled input and secrets to bytes (`.encode('utf-8')`) before using `hmac.compare_digest`.
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
Comment on lines 113 to +118

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📝 Info: API keys still sourced from env, not KV

get_configured_api_keys reads CODEC_CARVER_API_KEYS from os.environ at saas_web.py, which AGENTS.md flags as a known deviation to migrate to the credential registry. This PR does not introduce or change that read; it only fixes the byte comparison.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

):
return JSONResponse(
status_code=401,
Expand Down
23 changes: 23 additions & 0 deletions tests/test_saas_web.py
Original file line number Diff line number Diff line change
Expand Up @@ -707,6 +707,29 @@ def test_missing_header_rejected_when_keys_configured(self):
self.assertEqual(response.json(), {"error": "Invalid or missing API key"})
self.assertNotIn("secret-key", response.text)

@patch("saas_web.JSONResponse")
def test_non_ascii_key_does_not_crash(self, mock_json_response):
# httpx TestClient forbids non-ascii headers entirely; test middleware directly.
from starlette.requests import Request
import asyncio

with patch.dict(os.environ, {"CODEC_CARVER_API_KEYS": "secret-key"}):
scope = {
"type": "http",
"method": "POST",
"path": "/shrink",
"headers": [(b"x-api-key", "에스".encode("utf-8"))],
}
request = Request(scope)

async def mock_call_next(req):
return "SUCCESS"

mock_json_response.return_value = "REJECTED"

result = asyncio.run(saas_web.require_api_key(request, mock_call_next))
self.assertEqual(result, "REJECTED")

def test_wrong_key_rejected(self):
with patch.dict(os.environ, {"CODEC_CARVER_API_KEYS": "secret-key"}):
response = self._post_shrink(headers={"X-API-Key": "wrong-key"})
Expand Down
Loading