Skip to content

Prevent OAuth access token from being logged (fastmcp v4) - #122

Closed
heymb wants to merge 2 commits into
googleads:mainfrom
heymb:prevent-oauth-access-token-being-logged
Closed

heymb wants to merge 2 commits into
googleads:mainfrom
heymb:prevent-oauth-access-token-being-logged

Conversation

@heymb

@heymb heymb commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

Security Bug

Root Cause

  • Prevent OAuth credentials from being logged #91 prevented logging the OAuth credentials by setting the httpx logger to WARNING so the access token in fastmcp's tokeninfo request URL would not be logged.
  • 32c804f upgraded fastmcp to 4.x, whose HTTP client is httpx2, so the logger name stopped matching and every authenticated request logged the user's Google access token again, e.g. INFO:httpx2:HTTP Request: GET https://oauth2.googleapis.com/tokeninfo?access_token=<token> "HTTP/1.1 200 OK"

Fix

  • Added failing test before the fix to demonstrate the bug
  • Updated the logger name to httpx2 to fix the bug

Possible Improvements

  • Stop gitignoring uv.lock since upgrading fastmcp switched httpx to httpx2, but it wasn't clear that the original logging fix was broken

@google-cla

google-cla Bot commented Sep 17, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@heymb heymb changed the title Prevent OAuth access token from being logged Prevent OAuth access token from being logged after fastmcp v4 upgrade Sep 17, 2026
@heymb heymb changed the title Prevent OAuth access token from being logged after fastmcp v4 upgrade Prevent OAuth access token from being logged (fastmcp v4) Sep 17, 2026
@Raibaz

Raibaz commented Sep 17, 2026

Copy link
Copy Markdown
Collaborator

@heymb thanks a lot for this! Would you please be able to accept the CLA so I can merge it?

@heymb

heymb commented Sep 19, 2026

Copy link
Copy Markdown
Contributor Author

@Raibaz You're welcome! Unfortunately, I didn't consider the CLA. I need to ask internally about it. Will keep you posted. Thanks

@Raibaz

Raibaz commented Sep 21, 2026

Copy link
Copy Markdown
Collaborator

I think this was superseded by #108.

@heymb

heymb commented Sep 21, 2026

Copy link
Copy Markdown
Contributor Author

@Raibaz Sounds good! If that's the case, feel free to close this.

P.S. FastMCP has a draft PR that would solve this upstream: PrefectHQ/fastmcp#5185

@Raibaz Raibaz closed this Sep 21, 2026
@heymb

heymb commented Sep 21, 2026

Copy link
Copy Markdown
Contributor Author

FYI the fastmcp PR above was merged! Once they release it, the dep here can be bumped to avoid this kind of bug altogether without any configuration here.

@heymb

heymb commented Sep 28, 2026

Copy link
Copy Markdown
Contributor Author

@Raibaz Opened a new PR since FastMCP handles this upstream now. CLA is signed :) #126

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.

2 participants