Skip to content

Python: fix(devui): compare bearer tokens as bytes so non-ASCII input cannot raise - #9714

Open
lisry-1 wants to merge 3 commits into
microsoft:mainfrom
lisry-1:fix/devui-nonascii-token
Open

lisry-1 wants to merge 3 commits into
microsoft:mainfrom
lisry-1:fix/devui-nonascii-token

Conversation

@lisry-1

@lisry-1 lisry-1 commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

Motivation & Context

DevUI's bearer-token middleware compares the presented token with the configured one using secrets.compare_digest(token, expected_token). compare_digest accepts only ASCII str values — anything else raises TypeError ("comparing strings with non-ASCII characters is not supported"). Nothing catches it, so:

  1. A request whose Authorization: Bearer … value holds a non-ASCII byte answers HTTP 500 instead of HTTP 401.
  2. If DEVUI_AUTH_TOKEN itself contains non-ASCII characters, the request raises the same way, so that configured token can never authenticate — the second point in the issue.

The failure is fail-closed (no bypass), but it turns an authentication failure into an internal error, and it makes a non-ASCII DEVUI_AUTH_TOKEN unusable.

Reported in #9713.

Description & Review Guide

  • What are the major changes?

    python/packages/devui/agent_framework_devui/_server.py:

    • New DevServer._token_matches(presented, expected) static helper replaces the direct secrets.compare_digest call in auth_middleware. It compares bytes, which compare_digest accepts without restriction.
    • The presented value is re-encoded with latin-1. Headers travel as bytes and Starlette hands them back decoded as latin-1, so this recovers exactly what the client put on the wire.
    • The expected value (a constructor argument, or DEVUI_AUTH_TOKEN from the environment) is text, so UTF-8 is its wire form.
    • A presented value that cannot be re-encoded at all returns False rather than raising, so no input can turn into a 500.

    python/packages/devui/tests/devui/test_server.py:

    • Four-case parametrized test over (configured, presented): ASCII/ASCII → 200, ASCII/non-ASCII → 401, non-ASCII/non-ASCII → 200, non-ASCII/ASCII → 401. The header is sent as raw bytes because httpx refuses to encode a non-ASCII header value itself; raise_server_exceptions=False turns the old crash into an observable 500.

    ASCII tokens are unaffected: latin-1 and UTF-8 agree on ASCII, so the comparison is bit-for-bit what it was.

  • What is the impact of these changes?

    A wrong or malformed bearer token is now consistently 401 invalid_token instead of 500. A DEVUI_AUTH_TOKEN containing non-ASCII characters becomes usable when the client sends the same characters as UTF-8. No behavior change for ASCII tokens, which is every token DevUI generates itself (secrets.token_urlsafe).

  • What do you want reviewers to focus on?

    The encoding choice. There is no standard for a non-ASCII bearer token (RFC 6750 restricts b64token to ASCII), so I had to pick one: wire bytes for the presented value, UTF-8 for the configured one, which is what makes case 2 of the issue work. If you would rather treat any non-ASCII token as simply invalid, the alternative is a one-line try/except TypeError: return False around the existing call — that fixes the 500 but leaves a non-ASCII DEVUI_AUTH_TOKEN unusable. Happy to switch.

Related Issue

Fixes #9713

This is submitted before maintainer triage (the issue carries needs-maintainer-triage). To be explicit about why I consider it under the trivial-change bar: the issue states the desired outcome ("invalid or malformed bearer tokens should be rejected with HTTP 401 rather than causing an internal server error"), the fix is contained to the comparison, and no ASCII behavior changes. If you would rather this waited for triage, say so and I will hold.

Contribution Checklist

  • The code builds clean without any errors/warnings (ruff check and ruff format --check pass)
  • New tests were added for the change
  • Regression verified: all three non-ASCII cases fail without the fix (assert 500 == 401 / assert 500 == 200)
  • No unrelated changes included

AI Assistance

  • An AI coding assistant helped locate the defect, draft the fix, and write the tests.
  • I understand and reviewed every change in this pull request.
  • The reproduction and all tests were run locally; the outputs quoted below are from those runs.

Sending Authorization as raw bytes (httpx refuses to encode a non-ASCII header value itself), before the fix:

expected=b's3cret'      presented=b's3cret'      -> 200
expected=b's3cret'      presented=b's\xc3\xa9cret' -> 500
expected=b's\xc3\xa9cret' presented=b's\xc3\xa9cret' -> 500
expected=b's\xc3\xa9cret' presented=b's3cret'      -> 500

After the fix:

expected=b's3cret'      presented=b's3cret'      -> 200
expected=b's3cret'      presented=b's\xc3\xa9cret' -> 401
expected=b's\xc3\xa9cret' presented=b's\xc3\xa9cret' -> 200
expected=b's\xc3\xa9cret' presented=b's3cret'      -> 401

Test runs: python/packages/devui/tests/devui/test_server.py 58 passed; the wider python/packages/devui/tests/devui/ suite 219 passed / 4 skipped (test_ui_memory_regression.py needs websockets, which is not installed here).

… cannot raise

secrets.compare_digest accepts only ASCII str values and raises TypeError when
either side holds a non-ASCII character. Uncaught, a merely wrong token answers
500 instead of 401, and a non-ASCII DEVUI_AUTH_TOKEN never matches even when the
client presents it correctly.

Compare bytes instead: headers travel as bytes and Starlette hands them back
decoded as latin-1, while the configured token is text, so UTF-8 is its wire
form. ASCII tokens compare exactly as before.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Unicode whitespace trimming still corrupts some valid UTF-8 token endings, causing correct credentials to receive HTTP 401.

1 open issue
What changed in this PR

This PR addresses #9713 by updating Python DevUI bearer-token authentication to compare bytes, avoiding non-ASCII string comparison errors.

Changes:

  • Adds a token-comparison helper that recovers header bytes using Latin-1 and encodes configured tokens as UTF-8.
  • Adds four regression cases covering matching and mismatched ASCII and non-ASCII tokens.
File Description
python/​packages/​devui/​tests/​devui/​test_server.py Adds authentication regression tests using raw-byte headers.
python/​packages/​devui/​agent_framework_devui/​_server.py Introduces byte-based token comparison and integrates it into authentication middleware.

Review effort: Balanced


token = auth_header.replace("Bearer ", "", 1).strip()
if not secrets.compare_digest(token, expected_token):
if not self._token_matches(token, expected_token):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lisry-1 Please take a look at the inline test-typing finding in addition to the existing authentication feedback.

# Sent as raw bytes: httpx refuses to encode a non-ASCII header value itself.
response = client.get(
"/v1/entities",
headers={"Authorization": b"Bearer " + presented_token.encode("utf-8")},

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lisry-1 This header mapping mixes a string key with a bytes value. HTTPX accepts string/string or bytes/bytes mappings, so the request works at runtime but the current test fails Pyright and Pyrefly and blocks the merge gatekeeper. Please use a bytes key—{b"Authorization": b"Bearer " + presented_token.encode("utf-8")}—to preserve the raw token bytes without violating the declared header type.

A bare .strip() on the presented token also removed non-ASCII whitespace
such as NBSP (U+00A0), which is a valid byte in a UTF-8 token (the second
byte of 'à' is 0xA0). A correct non-ASCII token was therefore truncated
before the byte comparison and rejected with 401.

Trim only space/tab, and switch the test header mapping to a bytes key so
the bytes/bytes mapping stays type-consistent for Pyright/Pyrefly. Add
regression cases for a token ending in 'à'.
@lisry-1

lisry-1 commented Oct 29, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review — both findings are addressed in the latest commit.

  1. Unicode whitespace stripping — replaced the bare .strip() with .strip(" \t") so only HTTP optional-whitespace is trimmed. A token ending in à (UTF-8 0xC3 0xA0, second byte is NBSP) is no longer truncated before the byte comparison. Added two regression cases (secretà matches / secretà vs secreté rejected).

  2. Test header typing — the header mapping now uses a bytes key ({b"Authorization": b"Bearer " + ...}) so the mapping is bytes/bytes and passes Pyright/Pyrefly without violating the declared header type.

Let me know if there's anything else to adjust.

except UnicodeEncodeError:
# Not something a header could have carried; treat it as a wrong token.
return False
return secrets.compare_digest(presented_bytes, expected.encode("utf-8"))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lisry-1, this UTF-8/latin-1 split makes a correctly configured non-ASCII token unusable through the bundled browser client. The frontend passes the token directly to Fetch, whose header string uses ByteString semantics; for example, sécret is sent with byte 0xE9, while this comparison expects UTF-8 bytes 0xC3 0xA9. The correct token therefore returns 401 and the UI clears it. Please make the frontend and server use one explicit wire encoding, and add a regression that exercises the actual bundled client path rather than supplying UTF-8 bytes directly to the middleware.

The bundled browser client sends Authorization via fetch(), which
encodes a header value with ByteString (latin-1) semantics. A token like
sécret therefore travels as the single byte 0xE9, while the server
compared the expected token's UTF-8 bytes (0xC3 0xA9). The correct token
was answered 401 and the UI cleared it.

Encode both the presented and the expected token as latin-1 so the
browser client and the server share one wire encoding. A token outside
latin-1 cannot be expressed by the browser header encoding and is treated
as a mismatch rather than raising. ASCII tokens are unchanged.

Update the regression to exercise the browser path by sending latin-1
bytes instead of UTF-8 bytes.
@lisry-1
lisry-1 temporarily deployed to github-app-auth October 30, 2026 13:31 — with GitHub Actions Inactive
@lisry-1

lisry-1 commented Oct 30, 2026

Copy link
Copy Markdown
Contributor Author

Eduard van Valkenburg (@eavanvalkenburg) thanks for the precise repro — you are right that the UTF-8/latin-1 split made a correctly configured non-ASCII token unusable through the bundled browser client.

I have pushed a fix that makes the browser client and the server share one wire encoding (latin-1):

  • _token_matches now encodes both the presented and the expected token as latin-1. fetch() encodes header values with ByteString (latin-1) semantics, and Starlette decodes headers as latin-1, so sécret now travels as the single byte 0xE9 on both sides and matches.
  • A token outside latin-1 cannot be expressed by the browser header encoding, so it is treated as a mismatch instead of raising (no 500).
  • ASCII tokens are unchanged.

The regression now sends latin-1 bytes (the browser path) rather than UTF-8 bytes, and the sécret case (U+00E9) plus the à (U+00E0) NBSP-trailing case both pass. Verified the byte flow with a minimal comparison script before committing.

This branch was previously deployed

1 inactive deployment
github-app-auth — 9d5b3adb Deployed Oct 30, 2026 by lisry-1 via add_label #26675
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

python Usage: [Issues, PRs], Target: Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python: [Bug]: DevUI bearer-token authentication returns 500 for non-ASCII tokens

3 participants