Fix substring matches in WWW-Authenticate parsing#3041
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a correctness issue in extract_field_from_www_auth where an auth-param name could be matched as a substring of a different param name (e.g., scope incorrectly matching inside error_scope), and adds regression coverage to prevent reintroducing the bug.
Changes:
- Anchor auth-param name matching to the start of the header or a parameter separator to avoid substring shadowing.
- Escape
field_namein the regex to ensure literal matching. - Add regression test cases covering both “shadowing” (real param present but a decoy exists) and “false positive” (only decoy param exists).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/mcp/client/auth/utils.py |
Updates extract_field_from_www_auth regex to require a real auth-param name boundary and escape the field name. |
tests/client/test_auth.py |
Adds regression cases ensuring substring-shadowing and substring-only params no longer match. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
1 issue found across 2 files
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
There was a problem hiding this comment.
2 issues found across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
|
Followed up on two additional parser edge cases. This now keeps the first Bearer challenge when multiple Bearer challenges appear in one WWW-Authenticate header, and it accepts auth-params written with optional whitespace around = (for example |
Fixes #3009.
extract_field_from_www_authcurrently matchesfield_nameas a substring of another auth-param name, so fields likescopecan be shadowed byerror_scopeandresource_metadatacan be shadowed byx_resource_metadata.This change anchors the auth-param name to the start of the header or a parameter separator, and adds regressions for both the shadowing and false-positive cases.
Tested with:
PYTHONPATH=src:src/mcp-types python -m pytest -q tests/client/test_auth.py -k "extract_field_from_www_auth_valid_cases or extract_field_from_www_auth_invalid_cases"PYTHONPATH=src:src/mcp-types python -m pytest -q tests/client/test_scope_bug_1630.pyPYTHONPATH=src:src/mcp-types python -m pytest -q tests/client/test_auth.py