fix(auth): handle non-ASCII API keys without 500 responses - #326
fix(auth): handle non-ASCII API keys without 500 responses#326seonghobae wants to merge 3 commits 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. |
📝 WalkthroughWalkthroughAPI 키 비교 전에 설정값과 요청 헤더를 UTF-8 바이트로 변환합니다. 비ASCII API 키 요청에 대한 401 회귀 테스트와 ChangesAPI 키 인증
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tests/test_saas_web.py (1)
694-703: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNon-ASCII 설정 키도 검증하세요.
현재 테스트는 요청 키만
안녕으로 설정하고CODEC_CARVER_API_KEYS는 ASCII 값으로 설정합니다. 따라서key.encode("utf-8")경로와 Non-ASCII 설정 키의 정상 인증을 검증하지 않습니다. 설정 키와 요청 헤더가 모두안녕인 성공 케이스를 추가하고call_next호출도 확인하세요.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_saas_web.py` around lines 694 - 703, Update the API-key tests around require_api_key to add a successful case where both CODEC_CARVER_API_KEYS and the request header use the non-ASCII key “안녕”. Assert a successful response and verify mock_call_next is called, while preserving the existing invalid-key assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.jules/sentinel.md:
- Line 65: Update the 학습 note describing hmac.compare_digest to state that
ASCII-only str comparisons are supported, while comparisons involving non-ASCII
str values raise TypeError; do not claim that Unicode string comparisons are
categorically unsupported.
In `@tests/test_saas_web.py`:
- Line 694: MockRequest의 공유 mutable class attribute인 headers를 제거하고 __init__에서
인스턴스별 self.headers로 생성하세요. 기존 헤더 값은 유지하되 각 테스트 인스턴스가 독립적인 헤더 상태를 갖도록 수정해 Ruff
RUF012 경고를 해결하세요.
---
Nitpick comments:
In `@tests/test_saas_web.py`:
- Around line 694-703: Update the API-key tests around require_api_key to add a
successful case where both CODEC_CARVER_API_KEYS and the request header use the
non-ASCII key “안녕”. Assert a successful response and verify mock_call_next is
called, while preserving the existing invalid-key assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a6437a3c-1d61-453f-b2a3-63f213574415
📒 Files selected for processing (3)
.jules/sentinel.mdsaas_web.pytests/test_saas_web.py
seonghobae
changed the title
Understood. Acknowledging that this work is now obsolete and stopping work on this task. |
1 participant
Classification
This is an API-key input-validation and error-handling defect. Python
hmac.compare_digestaccepts ASCII-only strings but raisesTypeErrorfor non-ASCII string values; an unauthenticated request could therefore receive a request-scoped 500 response. The priorCRITICAL/service-wide DoS claim was not demonstrated and is intentionally removed.Changes
setuptoolsruntime lock from 82.0.1 to patched 83.0.0 for the repository's open Medium Dependabot alert, GHSA-h35f-9h28-mq5c / CVE-2026-59890.Verification
pytest -q tests/test_saas_web.py: 71 passedpytest -q: 464 passed, 4 subtests passedcompileall, and interrogate 100% passedpip install --dry-run --require-hashesresolved the refreshed lock, includingsetuptools==83.0.0c56bb97PR originally created by Jules for task 8779621121455543638 started by @seonghobae.