Skip to content

🛡️ Sentinel: [MEDIUM] 비 ASCII API 키로 인한 500 서버 에러 해결 - #505

Closed
seonghobae wants to merge 4 commits into
mainfrom
sentinel/fix-hmac-unicode-7639675748107847099
Closed

🛡️ Sentinel: [MEDIUM] 비 ASCII API 키로 인한 500 서버 에러 해결#505
seonghobae wants to merge 4 commits into
mainfrom
sentinel/fix-hmac-unicode-7639675748107847099

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Verified succession

Exact predecessor 90a894da969ce726ab068e0e6926743bef92b325의 유효 delta는 non-ASCII mismatch→401 behavior/test, 그리고 Unreleased CHANGELOG에 auth exception 수리를 기록하는 traceability입니다. Canonical #520 exact cf730d007543ee828b7b8e77c9473288924047d4가 이를 완전 승계했습니다. tests/test_saas_web.py와 raw-ASGI Unicode contract가 invalid non-ASCII rejection을 실행하고, CHANGELOG.md는 raw header bytes·configured UTF-8 key·duplicate-header rejection까지 실제 current implementation에 맞게 더 정확히 기록합니다.

#505의 decoded request.headers.get(...).encode('utf-8') source와 blanket .jules encoding 지침은 configured Unicode credential을 Starlette/Latin-1 string round-trip에서 훼손할 수 있어 별도 유효 delta가 아닙니다. Canonical .jules/sentinel.md도 raw ASGI credential boundary와 exactly-one header 정책으로 교정했습니다. #520은 protected main@90717c6e9954bf3b7a351137995ebe89975e46c2 대비 behind_by=0인 non-force descendant이며 exact-head hosted checks를 새로 검증 중입니다.

@google-labs-jules

Copy link
Copy Markdown

👋 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 @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@coderabbitai

coderabbitai Bot commented Aug 30, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 43 minutes.

View limit details

Limit 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.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d15416db-288c-422b-8a69-4a19ccc2c9e5

📥 Commits

Reviewing files that changed from the base of the PR and between a8e4956 and 90a894d.

📒 Files selected for processing (4)
  • .jules/sentinel.md
  • CHANGELOG.md
  • saas_web.py
  • tests/test_saas_web.py

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Devin Review found 2 potential issues.

Devin Review

Comment thread saas_web.py

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔍 Credential registry migration remains incomplete

get_configured_api_keys still reads runtime secrets directly from the environment. Repository policy requires one-time environment bootstrap into a credential registry, then registry-only request handling.

Devin Review

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

Comment thread saas_web.py
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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Non-ASCII configured keys always fail

When a configured key contains non-ASCII characters, encode("utf-8") re-encodes Starlette's Latin-1 header text rather than the received bytes. Matching clients always receive 401.

Suggested change
hmac.compare_digest(provided_key.encode("utf-8"), key.encode("utf-8")) for key in configured_keys
hmac.compare_digest(provided_key.encode("latin-1"), key.encode("utf-8")) for key in configured_keys
Devin Review

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

@seonghobae seonghobae closed this Sep 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant