Skip to content

fix(mcp): ignore OAuth credentials without access tokens - #7590

Open
CorgiBoyG wants to merge 1 commit into
google:mainfrom
CorgiBoyG:fix/mcp-empty-oauth-header
Open

CorgiBoyG wants to merge 1 commit into
google:mainfrom
CorgiBoyG:fix/mcp-empty-oauth-header

Conversation

@CorgiBoyG

Copy link
Copy Markdown
Contributor

Link to Issue or Description of Change

Problem:

After #7550 made explicit auth headers authoritative, McpTool._get_headers() can still emit the literal header Authorization: Bearer None when an OAuth2 credential object exists without an access token. That invalid auth header then overrides a valid header_provider value during invocation, while discovery correctly keeps the provider header.

Copybara imported the earlier #7550 patch as f82aa222 before its final revision added this guard and regression coverage.

Solution:

Only construct an OAuth2 Authorization header when access_token is non-empty. Otherwise, return no OAuth auth header and allow the existing provider header to remain effective.

The change is deliberately limited to the OAuth2 guard and one regression test at the MCP session-creation boundary.

Testing Plan

Unit Tests:

  • I have added or updated unit tests for my change.
  • All relevant unit tests pass locally.

The new test fails on current main with:

Actual: create_session(headers={'Authorization': 'Bearer None'})

After the fix:

pytest -q -p no:cacheprovider tests/unittests/tools/mcp_tool
441 passed

pytest -q -p no:cacheprovider tests/unittests/tools tests/unittests/auth
3463 passed, 1 skipped

Also verified:

pyink --check: passed
ruff check: passed
git diff --check: passed

The full repository test suite was not run.

Manual End-to-End (E2E) Tests:

Not run. The regression test asserts the final headers passed to create_session().

Checklist

  • I have read the CONTRIBUTING.md document.
  • I have performed a self-review of my own code.
  • The changed condition is self-explanatory; no additional code comment is required.
  • I have added a test that proves the fix is effective.
  • Relevant MCP and auth unit tests pass locally.
  • I have manually tested my changes end-to-end.
  • No downstream dependency changes are required.

Additional context

The later credential-manager fix in 7fca46f1 rejects an unredeemed authorization-code credential, but it does not cover an empty-token credential without an auth code or one returned by a custom auth provider. Guarding header construction remains necessary at this boundary.

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.

MCP invocation can replace a valid provider header with Bearer None

2 participants