fix(security): reject non-ASCII auth/signature input with 401 instead of crashing (all compare_digest sites) - #65697
Merged
Merged
Conversation
…crashing _check_auth gates every OpenAI-compatible API server endpoint. It compared the client's raw bearer token against the configured key with hmac.compare_digest on two str values. compare_digest raises TypeError on a str containing non-ASCII characters, and the token comes straight from the Authorization header — so a request with a single non-ASCII byte in the key (a stray unicode char, a smart quote, a pasted BOM) crashed the handler with an unhandled TypeError. Every endpoint calls _check_auth without a try/except, so the framework turned that into a 500 Internal Server Error instead of the intended 401 Invalid API key. Compare as bytes, matching web_server.py's dashboard-token check (hmac.compare_digest(auth.encode(), expected.encode())). Encoding both sides keeps the timing-safe comparison and its semantics identical for valid keys while making a non-ASCII token fail closed with a clean 401. Adds regression tests: a non-ASCII bearer token returns 401 (no raise), and a non-ASCII configured key still authenticates against its exact value.
… the endpoint _validate_signature backs the public webhook receiver. It compared each attacker-supplied signature/token header (GitHub X-Hub-Signature-256, GitLab X-Gitlab-Token, generic X-Webhook-Signature / -V2, and the Svix v1 header) against a computed hex/base64 digest with hmac.compare_digest on two str values. compare_digest raises TypeError on a str containing non-ASCII characters, and the header is raw client input on an unauthenticated endpoint — so any internet client could POST a single non-ASCII byte in the signature header and raise out of the handler, returning a 500 instead of a clean 401. Fail-closed, but an on-demand crash of the request path. Route all five comparisons through a small _hmac_str_equal() helper that encodes both sides to UTF-8 bytes before the constant-time compare (compare_digest has no ASCII restriction on bytes). Semantics are unchanged for valid signatures; a hostile non-ASCII header now fails closed with a rejection instead of raising. Adds regression tests: non-ASCII GitHub/GitLab/generic/V2 signature headers return False (no raise), and a non-ASCII configured secret still matches its exact token value. Also maps drexux0@gmail.com in scripts/release.py AUTHOR_MAP.
…gression The fix routes the Svix v1 comparison through _hmac_str_equal too, but the existing non-ASCII tests only exercised the GitHub/GitLab/generic V1/V2 branches. Add a Svix case (valid svix-id + fresh svix-timestamp so it reaches the v1,<sig> compare) with a non-ASCII signature, which raised TypeError before the fix and now rejects cleanly.
…g sites Same bug class as the salvaged #65305/#65307: hmac.compare_digest (and secrets.compare_digest) raise TypeError when given a str containing non-ASCII characters, and these call sites feed it raw request input. Compare as UTF-8 bytes everywhere: - gateway/platforms/msgraph_webhook.py: clientState from request body - gateway/platforms/whatsapp_cloud.py: hub.verify_token query param + X-Hub-Signature-256 header (comment claimed 'works on str' — it doesn't for non-ASCII) - plugins/platforms/feishu: verification token + x-lark-signature - plugins/platforms/raft: bridge token header - plugins/platforms/line: X-Line-Signature - plugins/platforms/sms: X-Twilio-Signature - tools/code_execution_tool.py: sandbox RPC token (both loops) Regression tests for the two gateway-core sites (msgraph, whatsapp).
alt-glitch
added
type/bug
tonydwb
reviewed
Jul 16, 2026
tonydwb
left a comment
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Comment
Overview
- Security fix: reject non-ASCII auth/signature input with 401 instead of crashing
- 169 additions, 19 deletions
- Affects all
compare_digestsites
Assessment
- Good defensive security fix — non-ASCII input to HMAC comparison can cause type errors or unexpected behavior
- Using
.encode()on both sides beforecompare_digestis correct for Python 3 string handling - Covers multiple providers (Telegram, GitLab, etc.) — comprehensive
Note
- No input validation seen for the
secret.encode()call itself — ifself._api_keycould be None, this would crash. Verify this is prevented upstream.
Reviewed by Hermes Agent
This was referenced Jul 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A non-ASCII byte in any attacker-controlled auth/signature input now yields a clean 401/403/False instead of crashing the handler with a 500 — across the API server, the webhook adapter, and every sibling
compare_digestcall site that feeds it raw request input.Salvages #65305 and #65307 by @Drexuxux (3 commits cherry-picked, authorship preserved). Root cause:
hmac.compare_digest(andsecrets.compare_digest) raiseTypeErrorwhen given astrcontaining non-ASCII characters. Bearer tokens, signature headers, verify tokens, and clientState values are raw client input — several of these sit on public, unauthenticated endpoints.Changes
gateway/platforms/api_server.py: bearer-token check compares as bytes (@Drexuxux)gateway/platforms/webhook.py:_hmac_str_equal()helper; all 5 signature branches (GitHub, GitLab, generic V1/V2, Svix) routed through it (@Drexuxux)gateway/platforms/msgraph_webhook.py: clientState from request bodygateway/platforms/whatsapp_cloud.py:hub.verify_tokenquery param +X-Hub-Signature-256(the old comment claimed compare_digest "works on str" — not for non-ASCII)plugins/platforms/{feishu,raft,line,sms}: verification tokens + signature headerstools/code_execution_tool.py: sandbox RPC token (both server loops)Validation
Bearer ské-…to API serverské-input via real importsInfographic