π‘οΈ Sentinel: [HIGH] hmac.compare_digest DoS μ·¨μ½μ μμ - #519
π‘οΈ Sentinel: [HIGH] hmac.compare_digest DoS μ·¨μ½μ μμ #519seonghobae wants to merge 1 commit into
Conversation
|
π Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a π emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
Warning Review limit reachedNext included review available in 46 minutes. View limit detailsLimit details: Youβve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: βοΈ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: π Files selected for processing (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| @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"}) |
There was a problem hiding this comment.
π 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.
Was this helpful? React with π or π to provide feedback.
| 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 |
There was a problem hiding this comment.
π¨ 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.
Was this helpful? React with π or π to provide feedback.
1 participant
Verified succession
μ΄ generated branchμ μ ν¨ν μꡬμ¬νμ non-ASCII raw API-key μ λ ₯μ΄ μΈμ¦ λ―Έλ€μ¨μ΄μμ μ²λ¦¬λμ§ μμ μμΈλ₯Ό λ§λ€μ§ μκ³ 401λ‘ κ±°μ λμ΄μΌ νλ€λ κ²μ λλ€. Canonical #520 exact
9ee46273ad9e7023af23cf88e19e6ee808840334κ° μ΄ κ³μ½μ μμ μΉκ³νλ©΄μ λ κ°ν raw-ASGI/Unicode/duplicate-header κ²½κ³λ₯Ό μ 곡ν©λλ€.#519 exact
7d90f2fbcecf118aa7c1f17f45c89e8cc0437891μ production deltaλrequest.headers.get()μΌλ‘ μ»μ Starlette λ¬Έμμ΄ viewλ₯Ό.encode('utf-8')ν λ€ λΉκ΅νλ λ°©μμ λλ€. κ·Έ λ°©μμ attacker μ λ ₯μTypeErrorλ νΌνμ§λ§ raw UTF-8 configured Unicode credentialμ Latin-1 λ¬Έμμ΄ viewλ₯Ό κ±°μ³ μ¬μΈμ½λ©νμ¬ λ€λ₯Έ byte sequenceλ‘ λ§λ€ μ μμ΅λλ€. #520μrequest.scope['headers']μμ raw bytesλ₯Ό μ§μ μ½μ΄ exactly-oneX-API-Keyλ₯Ό μꡬνκ³ configured UTF-8 bytesμ λΉκ΅νλ―λ‘ μ΄ νκ·λ₯Ό λ§λ€μ§ μμ΅λλ€.#519μ async regressionμ΄ μꡬνλ non-ASCII mismatchβ401μ #520μ
tests/test_saas_web.pyμtests/test_api_key_unicode_contract.pyκ° μ μ§νλ©°, #520μ valid Unicode credential successμ duplicate-header rejectionκΉμ§ μΆκ°λ‘ κ²μ¦ν©λλ€. #519μ.julesblanket "νμ stringμ UTF-8 encode" κ·μΉμ raw HTTP credential κ²½κ³μμλ λΆμ νν΄ λ³λ μ ν¨ deltaκ° μλλλ€.#520μ protected
main@90717c6e9954bf3b7a351137995ebe89975e46c2λ₯Ό non-forceλ‘ μ±νν exact9ee46273...,behind_by=0μ΄λ©° νμ¬ hosted checksλ μ headμμ queued/pendingμ λλ€. λ°λΌμ #519μ λͺ¨λ μ ν¨ behavior/test contractλ stronger canonical successorμ μΉκ³λκ³ , μ½ν source/doc deltaλ μλμ μΌλ‘ μΉκ³νμ§ μμ΅λλ€.