Sign up
View stargazersSign up

fix(mcp): accept case-insensitive bearer authentication#23792

closed2
main
GitHub fork: main
opened13d agobySshethshlok
4 files changed
View on GitHubTwo-way sync enabledPR last imported 3d ago

Problem

The MCP endpoint rejects Authorization: bearer <valid-token> before validating the credential. Both the MCP middleware and the OAuth client's shared bearer parser require exactly Bearer . HTTP authentication schemes are case-insensitive under RFC 9110 section 11.1.

The middleware also reports missing_bearer_token for both an absent header and an unsupported scheme, making those failures indistinguishable in the logs.

Change

Make the two existing scheme checks case-insensitive. Both changes are needed: fixing the middleware alone would still leave OAuth client credentials rejected by EnvironmentAuth. Token contents remain case-sensitive, and existing credential validation and OAuth challenge behavior are preserved.

Classify rejected requests as a missing authorization header, unsupported scheme, empty bearer token, or unknown/expired token. Log only the fixed reason, without including the header or credential. Extend the existing HTTP transport and OAuth sign-in tests.

Scope and approval

Fixes https://github.com/pingdotgg/t3code/issues/23710

GitCafe: https://git.cafe/pingdotgg/t3code/issues/23710

This is a small, focused fix for the protocol mismatch and its misleading diagnostic in https://git.cafe/pingdotgg/t3code/issues/23710, under the obvious-bug exception. The issue has not received explicit maintainer approval. The patch updates the existing checks and tests without adding a parser abstraction, dependency, setting, or authentication workflow. The shared bearer parser also serves ordinary HTTP authentication, which receives the same case-insensitive scheme handling.

Verification

Maintainer triage independently reproduced the defect on current main (a46f80ee28) and applied this PR patch there: test-only changes produced the two expected failures; the full patch passed 42 focused tests, targeted lint, and the server typecheck. Real isolated server requests confirmed case-insensitive scheme handling and distinct missing-header/wrong-scheme reasons. Runtime transcript and verification details. Two independent reviewers found no concrete defects; shared authentication changes still need human approval.

On Linux x64, both focused regressions failed before the production changes:

text
1MCP HTTP transport: expected 200, received 401 for lowercase bearer.
2OAuth-issued MCP credential: ServerAuthMissingCredentialError for lowercase bearer.
3Test Files 2 failed (2)
4Tests 2 failed | 24 skipped (26)

Command used to reproduce and check those paths:

sh
1vp test run apps/server/src/mcp/McpHttpServer.test.ts apps/server/src/auth/McpOAuth.test.ts -t 'admits provider and OAuth|signs in with a pairing code'

After the fix, those two regressions pass. The HTTP fixture exercises MCP initialization and a tool call with provider and client credentials. Its asserted outcomes are:

text
1Bearer / bearer / bEaReR with valid credentials 200
2Missing header / Basic / BearerX / empty token 401
3Changed token case / invalid or expired token 401

The real OAuth sign-in test verifies a newly issued MCP token through the shared verifier with all three scheme variants. The HTTP test asserts the exact rejection logs, including that no credential value appears, and retains the client-versus-provider OAuth challenge checks.

Updated with upstream f58fad041621 at head abc940a458. The two targeted transport/OAuth regressions pass on the refreshed branch. vp run check:changed passes formatting, lint, both Knip checks, generated GraphQL checks, affected typechecks, and all 53 adjacent tests across four files. The unrelated untracked local issue-scout skill was excluded; no tracked PR files were excluded. Human authentication review and upstream CI authorization remain outstanding.

No UI changes, so screenshots are not applicable. The external ChatGPT connector and a deployed tunnel were not tested; this fixes the confirmed T3 parsing defect without claiming it explains every connector failure in the report. Full repository tests and builds remain for CI.

Written and verified with GPT-6.1 Sol through the Codex harness in T3 Code.

Checks

21 successful · 15 skipped
All checks passed
abc940a

Added 1 commit

  • fix(mcp): accept case-insensitive bearer authenticationf00806e
CcoderabbitaiBot13d ago
Synced from GitHub

Review in Change Stack →

Walkthrough

OAuth and MCP HTTP authentication now accept bearer schemes with different capitalization. MCP HTTP rejection logs distinguish missing headers, unsupported schemes, empty bearer tokens, and unknown or expired tokens.

Changes: Bearer authentication

Layer / File(s)Summary
OAuth bearer parsing
apps/server/src/auth/EnvironmentAuth.ts, apps/server/src/auth/McpOAuth.test.ts		The bearer token parser matches the scheme without regard to case. The OAuth test checks three capitalization variants.
MCP HTTP authentication
apps/server/src/mcp/McpHttpServer.ts, apps/server/src/mcp/McpHttpServer.test.ts		The MCP HTTP server matches bearer schemes without regard to case and distinguishes rejection reasons in its logs. Tests cover accepted credentials, rejected authorization values, and warning messages.

Priority: External image Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: juliusmarminge

Merge Risk: External image Minimal · up to abc94

Bearer schemes now accept different capitalization while preserving token contents. No merge-blocking risk remains identified.

Pre-merge checks: Passed 4
Check nameStatusExplanation
Title checkPassedThe title "fix(mcp): accept case-insensitive bearer authentication" matches the main change. The summary says both the MCP middleware and the shared OAuth bearer parser now accept the Bearer scheme ca…
Description checkPassedThe description follows the template sections: Problem, Change, Scope and approval, and Verification. It explains the RFC 9110 basis, the two parser fixes, and why both are needed. It links issue #237…
Linked Issues checkPassedThe changes satisfy the coding requirements in [#23710]. McpHttpServer matches the Bearer scheme case-insensitively and logs distinct reasons for a missing header, unsupported scheme, empty token,…
Out of Scope Changes checkPassedThe changes remain within [#23710]. Production changes modify the two required bearer checks and MCP rejection classification. Test changes verify the required behavior and preserve authentication beh…
Full details: Title check

Explanation

The title "fix(mcp): accept case-insensitive bearer authentication" matches the main change. The summary says both the MCP middleware and the shared OAuth bearer parser now accept the Bearer scheme case-insensitively. The title does not mention the secondary change to rejection logging, which the description covers, but that is expected. It follows the conventional commit style the template recommends.

Full details: Description check

Explanation

The description follows the template sections: Problem, Change, Scope and approval, and Verification. It explains the RFC 9110 basis, the two parser fixes, and why both are needed. It links issue #23710 and invokes the obvious-bug exception with reasons. The verification section gives concrete test commands, observed results, and items not checked, such as the external ChatGPT connector. It also leaves the template's HTML comments in place, which is acceptable. The claim that the obvious-bug exception applies is for maintainers to decide, but the author gives a reasoned basis for it.

Full details: Linked Issues check

Explanation

The changes satisfy the coding requirements in [#23710]. McpHttpServer matches the Bearer scheme case-insensitively and logs distinct reasons for a missing header, unsupported scheme, empty token, and unknown or expired token. The logs do not include the header or credential. EnvironmentAuth applies the same scheme handling. The HTTP and OAuth tests cover scheme variants, rejected credentials, token case sensitivity, and diagnostic safety.

Full details: Out of Scope Changes check

Explanation

The changes remain within [#23710]. Production changes modify the two required bearer checks and MCP rejection classification. Test changes verify the required behavior and preserve authentication behavior. No unrelated feature, dependency, setting, UI, or workflow changes are identified.

Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

MmacroscopeappBot13d ago
Synced from GitHub

Approvability

Verdict: Needs Human Approval

Macroscope's review found this PR requires human approval — This PR updates shared HTTP and MCP bearer-token authentication behavior and changes authentication failure diagnostics. Although the implementation is focused and well-covered by tests, changes to authentication code require human review.

You can add or adjust custom eligibility rules. Learn more.

Ffanaru12d ago
Synced from GitHub

Thanks for working on this fix. I can share some observations from a real-world integration.

Observed:

  • Setup: a ChatGPT custom MCP app (OAuth with dynamic client registration) connected to T3 Code Desktop 0.1.1.
  • Token issuance succeeded. All 8 POST requests to /mcp from the ChatGPT connector (User-Agent: Python/aiohttp) were rejected with HTTP 401 and the server-side reason missing_bearer_token, including the 5 sent after the token was issued (one of them less than a second later). Token verification never ran.
  • Control test through the public tunnel with a dummy value: Authorization: Bearer <dummy> reached verification (unknown_or_expired_token), while Authorization: bearer <dummy> returned missing_bearer_token.

Not confirmed:

  • Whether the connector omits the Authorization header or sends a different scheme or case.
  • I have not tested this PR, so I can't say it resolves this integration.

The more specific rejection reasons here should help tell these cases apart. Thanks again.

Added 1 commit

  • merge: update bearer authentication fix with mainabc940a
Sshethshlok5d ago
Synced from GitHub

@juliusmarminge This authentication fix is refreshed against main at abc940a458. All local changed-file checks pass, including 53 adjacent tests across four files. The targeted real HTTP transport and OAuth-token regressions also pass on the updated branch.

The tests cover valid credentials with Bearer/bearer/bEaReR schemes, rejection of Basic/BearerX/empty or invalid tokens, preservation of token case sensitivity, and credential-free rejection diagnostics. Runtime evidence and the unverified external ChatGPT/tunnel path are documented in the description.

Could you review the shared HTTP/MCP authentication change and authorize upstream CI? Macroscope explicitly requires human approval here. GitHub did not permit this account to submit a formal reviewer request.

CI update: the upstream CI aggregate Check now passes on abc940a458, including all server shards. The CI authorization request above is resolved; human authentication review is still requested. Passing CI.

Ttheo3d ago

Note

🤖 GPT-6.1 Terra [1m] responding on behalf of Theo

Closing as a duplicate of the earlier bearer parser fix, which handles mixed-case provider and OAuth schemes and distinguishes missing, unsupported, empty and unknown credentials. #23782 #23710