fix(security): PKCE verifier leak, OAuth refresh Content-Type, tool_choice mcp_ prefix#1757
Closed
0xbyt4 wants to merge 1 commit into
Closed
fix(security): PKCE verifier leak, OAuth refresh Content-Type, tool_choice mcp_ prefix#17570xbyt4 wants to merge 1 commit into
0xbyt4 wants to merge 1 commit into
Conversation
…hoice prefix 1. PKCE code_verifier was used as OAuth state parameter, leaking the PKCE secret in the authorization URL (browser history, proxy logs, Referer headers). Now uses a separate random value for state. 2. refresh_hermes_oauth_token sent application/json but RFC 6749 requires application/x-www-form-urlencoded for token endpoints. Matched to _refresh_oauth_token which already used the correct format. 3. When is_oauth=True, tool names get mcp_ prefix but tool_choice name did not, causing Anthropic API rejection (name mismatch). Now prefixes tool_choice name to match tool definitions.
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three bugs in
agent/anthropic_adapter.pyfound during security audit:PKCE code_verifier leaked via OAuth state parameter —
run_hermes_oauth_login()set"state": verifier, exposing the PKCE secret in the authorization URL (browser history, proxy logs, Referer headers). Now uses a separatesecrets.token_urlsafe(16)value.refresh_hermes_oauth_tokenused wrong Content-Type — sentapplication/jsonbut RFC 6749 requiresapplication/x-www-form-urlencodedfor token endpoints. The other refresh function (_refresh_oauth_token) already used the correct format.tool_choicename not mcp_-prefixed for OAuth — whenis_oauth=True, all tool names getmcp_prefix buttool_choicedid not, causing Anthropic API rejection (name mismatch between tools array and tool_choice).Test plan
TestPKCESecurityBugs::test_auth_url_does_not_contain_verifier— verifies verifier not in URLTestRefreshContentType::test_hermes_refresh_uses_form_urlencoded— verifies correct Content-TypeTestToolChoiceOAuthPrefix::test_specific_tool_choice_gets_mcp_prefix_with_oauth— verifies mcp_ prefix on tool_choice