Skip to content

fix(api-server): reject a non-ASCII bearer token with 401 instead of crashing - #65305

Closed
Drexuxux wants to merge 3 commits into
NousResearch:mainfrom
Drexuxux:fix/api-server-nonascii-bearer-crash
Closed

fix(api-server): reject a non-ASCII bearer token with 401 instead of crashing#65305
Drexuxux wants to merge 3 commits into
NousResearch:mainfrom
Drexuxux:fix/api-server-nonascii-bearer-crash

Conversation

@Drexuxux

Copy link
Copy Markdown
Contributor

What?

_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:

token = auth_header[7:].strip()
if hmac.compare_digest(token, self._api_key):   # both str
    return None

hmac.compare_digest raises TypeError: comparing strings with non-ASCII characters is not supported when given a str containing non-ASCII characters, and 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) raised 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 — a legitimate client with a mistyped key gets an inscrutable 500, and any request can force a 500 on the auth path.

Solution

Compare as bytes, matching web_server.py's dashboard-token check (hmac.compare_digest(auth.encode(), expected.encode())):

if hmac.compare_digest(token.encode(), self._api_key.encode()):
    return None

Encoding both sides keeps the timing-safe comparison and its semantics identical for valid keys, while a non-ASCII token now fails closed with a clean 401 instead of crashing.

How to test

  1. pytest tests/gateway/test_api_server.py::TestAuth -q
  2. Two new tests exercise _check_auth directly:
    • a Bearer token containing a non-ASCII byte must return a 401 response (and must not raise);
    • a non-ASCII configured key must still authenticate against its exact byte-for-byte value.
  3. Re-run against the pre-fix code to confirm both new tests raise TypeError there.

Test results

  • tests/gateway/test_api_server.py::TestAuth — 7 passed (5 existing + 2 new); existing valid/invalid/missing/malformed-header cases unchanged.
  • Pre-fix, both new tests fail with TypeError: comparing strings with non-ASCII characters is not supported at the _check_auth comparison; post-fix they return a clean 401 / authenticate correctly.
  • tests/gateway/test_api_server.py (full file) — 197 passed, 1 skipped, no new failures.

Drexuxux added 2 commits July 16, 2026 04:31
…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.
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery area/auth Authentication, OAuth, credential pools sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data labels Jul 16, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused API-server regression fix. The premise is verified on current main: gateway/platforms/api_server.py:1216-1220 extracts the raw Bearer header and compares two str values with hmac.compare_digest; the proposed byte comparison fixes that path and the added tests cover both rejection and valid Unicode-key behavior.

Problems

  • The same unsafe input pattern remains in the webhook authentication boundary. gateway/platforms/webhook.py:930, :935, :978, :1002, and :1056 pass request-header strings to hmac.compare_digest; _handle_webhook calls this validation directly at :535 before its normal 401 branch. This leaves sibling public signature paths outside the fix.

Suggested changes

  • Apply the byte-safe comparison to those webhook signature/token comparisons and add a regression test that malformed non-ASCII signature input is rejected without raising.

Automated hermes-sweeper review.

@teknium1 teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Jul 16, 2026
…ii-bearer-crash

# Conflicts:
#	scripts/release.py
teknium1 added a commit that referenced this pull request Jul 16, 2026
…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).
@teknium1

Copy link
Copy Markdown
Contributor

Merged via PR #65697 — your commits from this PR and #65307 were cherry-picked onto current main with your authorship preserved in git log (rebase-merge). We widened the same fix to the remaining sibling compare_digest sites (msgraph clientState, WhatsApp Cloud verify-token + signature, feishu/raft/line/sms plugin adapters, code_execution RPC token) in a follow-up commit so the whole bug class is closed. Thanks!

@teknium1 teknium1 closed this Jul 16, 2026
Gravezzz pushed a commit to Gravezzz/hermes-agent that referenced this pull request Jul 21, 2026
…g sites

Same bug class as the salvaged NousResearch#65305/NousResearch#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).
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
…g sites

Same bug class as the salvaged NousResearch#65305/NousResearch#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).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/auth Authentication, OAuth, credential pools comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants