Fall back to the caller-configured scope when the server advertises none - #3172
Open
maxisbey wants to merge 3 commits into
Open
Fall back to the caller-configured scope when the server advertises none#3172maxisbey wants to merge 3 commits into
maxisbey wants to merge 3 commits into
Claude / Claude Code Review
completed
Jul 25, 2026 in 23m 13s
Code review found 2 potential issues
Found 3 candidates, confirmed 2. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 0 |
| 🟡 Nit | 2 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🟡 Nit | src/mcp/client/auth/oauth2.py:278-284 |
configured_scope snapshot polluted when OAuthClientMetadata is reused across providers |
| 🟡 Nit | src/mcp/client/auth/utils.py:113-126 |
Behavior change not documented in docs/migration.md |
Annotations
Check warning on line 284 in src/mcp/client/auth/oauth2.py
claude / Claude Code Review
configured_scope snapshot polluted when OAuthClientMetadata is reused across providers
The `configured_scope=client_metadata.scope` snapshot can capture a previously *discovered* scope rather than the caller-configured one: the provider stores the caller's `OAuthClientMetadata` by reference and Step 3 mutates `client_metadata.scope` in place, so a second provider constructed from the same metadata object after the first's 401 flow snapshots server A's discovered scopes and the new tier-4 sends them to server B when B advertises nothing — the same stale-scope leak this snapshot was
Check warning on line 126 in src/mcp/client/auth/utils.py
claude / Claude Code Review
Behavior change not documented in docs/migration.md
This PR changes wire behavior — a caller-configured `scope` that was previously dropped when the server advertises no scopes is now sent on `/token` and `/authorize` — but adds no `docs/migration.md` entry, which AGENTS.md requires for behavior changes. Consider adding a short note alongside the existing scope-behavior entries (the `scopes=` → `scope=` rename at ~line 1978, or the SEP-2207/SEP-2350 sections), since strict authorization servers that reject un-allowlisted scopes may now fail with
Loading