feat(mcp)!: oauth2_flow read verbatim from DB rows and required in config; inference reduced to the request-time backstop by tin-berri · Pull Request #32292 · BerriAI/litellm

tin-berri · 2026-07-06T22:58:59Z

Relevant issues

Fourth and last of the oauth2_flow persistence sequence, stacked on #32290 (startup backfill), after #32283 (DCR stamp) and #32288 (create-time stamps). With every DB write site stamping the flow and the backfill healing legacy rows at boot, the DB read path no longer needs to guess

Behavior

Reads: the DB build takes the oauth2_flow column verbatim, with no field-shape guessing and no discovery dependence; the only surviving inference is the request-time security backstop (it keeps an unstamped M2M-shaped row from forwarding caller Authorization upstream) and it logs a warning whenever it fires. Config: auth_type oauth2 in config.yaml without an explicit oauth2_flow fails proxy startup with an error naming both valid values and what each means. Net effect: classification is asserted everywhere, the config guess is deleted outright, and the last runtime guess is telemetry-gated for deletion

Linear ticket

N/A

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

I have added meaningful tests
My PR passes all CI/CD checks (e.g., lint, format, unit tests)
My PR's scope is as isolated as possible; it only solves 1 specific problem
I have requested a Greptile review by commenting @greptileai and received a Confidence Score of at least 4/5 before requesting a maintainer review

Delays in PR merge?

If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).

Screenshots / Proof of Fix

Reproduction on a live proxy backed by Postgres (master key sk-1234), continuing the #32290 runbook

Seed a legacy-shaped row (M2M shape, null flow) exactly as in feat(mcp): startup backfill stamping oauth2_flow on legacy null rows #32290 step 1, then start the proxy. The backfill stamps it client_credentials at boot and the registry build reads the column; M2M egress behaves as before
Simulate a backfill gap to see the backstop fire: null the flow again while the proxy is running (no restart)

psql $DATABASE_URL -c "UPDATE \"LiteLLM_MCPServerTable\" SET oauth2_flow = NULL WHERE server_name = 'legacy_m2m';"
curl -s -X POST http://localhost:4000/v1/mcp/server/reload -H "Authorization: Bearer sk-1234" > /dev/null
curl -s http://localhost:4000/mcp/tools -H "x-litellm-api-key: sk-1234" > /dev/null

The tools call still refuses to forward the caller's Authorization upstream (the P1 property, held by the request-time backstop) and litellm.log now shows the fire-rate warning: MCP server ... has no persisted oauth2_flow but matches the client_credentials shape; using the inferred flow for this request. The startup backfill stamps this row at the next proxy boot. A restart stamps the row and the warning stops

Live run (stack tip)

A config declaring auth_type oauth2 without oauth2_flow fails startup on port 4001:

ValueError: Invalid config for MCP server 'bad_oauth': auth_type oauth2 requires an explicit oauth2_flow (got None). Set oauth2_flow: client_credentials for machine-to-machine servers (the proxy mints a shared token at token_url using client_id/client_secret, no user interaction) or oauth2_flow: authorization_code for interactive servers (per-user tokens via browser sign-in, including delegate_auth_to_upstream).
ERROR:    Application startup failed. Exiting.

curl to port 4001 afterwards: connection refused. After the backfill healed the DB rows, the registry build reads the column verbatim (GET list shows authorization_code and client_credentials for the two seeded rows)

Type

🧹 Refactoring

Changes

build_mcp_server_from_table read oauth2_flow through the field-shape inference, which cannot tell a DCR-registered interactive server (client creds + token_url, no persisted authorization_url) from an M2M server unless endpoint discovery succeeds first. With the write-side stamps and the startup backfill in place, the DB build now reads the column verbatim via _explicit_oauth2_flow: unknown or null values resolve to None, which needs_user_oauth_token already treats as interactive, so an unstamped row degrades to the safe per-user default instead of guessing M2M

Field-shape inference survives in exactly two places, each with a reason. config.yaml-loaded servers keep it at load time: they are rebuilt from the config on every boot, so there is no row to backfill and load-time resolution is effectively their write-time stamp (declaring oauth2_flow in config wins, and is the documented recommendation going forward). And the request-time backstop in _get_allowed_mcp_servers stays: it is the security property from the P1 fix that keeps a not-yet-backfilled M2M row blocking caller Authorization forwarding, and deleting it in the same release that introduces its replacement would turn any backfill failure into a header-forwarding regression. The backstop now logs a warning whenever it actually changes a value, which is the fire-rate signal: once deployments have booted past the backfill and the warning stays silent, removing the backstop is a trivial follow-up

Config-defined oauth2 servers now assert their flow instead of the proxy guessing it: auth_type oauth2 without an explicit oauth2_flow fails proxy startup with a config validation error that names both values and what each means (oauth2_flow: client_credentials for machine-to-machine, where the proxy mints a shared token at token_url using client_id/client_secret; oauth2_flow: authorization_code for interactive, per-user tokens via browser sign-in, including delegate_auth_to_upstream). This deletes config-level shape inference entirely; the credential shape is genuinely ambiguous, so the config is the right place to state the answer. This is breaking for config oauth2 servers that omitted the field: the fix is one line in the server block, and the error text says exactly which value to pick

Regression tests pin that the DB build does not infer M2M from the credential shape (null flow + M2M shape resolves to None with needs_user_oauth_token true) and reads an explicit column value verbatim. The existing P1 regression test continues to cover the backstop

Note

High Risk
Changes authentication bypass rules, anonymous server exposure, and upstream token forwarding for legacy unstamped OAuth2 MCP rows—security-sensitive behavior with a breaking config requirement.

Overview
MCP OAuth2 classification stops inferring oauth2_flow from credential shape when building servers from the DB: the column is read verbatim (null stays null, so those rows default to interactive/per-user behavior). Config auth_type: oauth2 without an explicit oauth2_flow now fails startup with guidance for client_credentials vs authorization_code.

A shared request-time layer (effective_oauth2_flow / resolve_oauth2_flow_for_request) remains for security only: unstamped rows that still look M2M are treated as client_credentials so anonymous delegate bypass, anonymous allowlists, tool listing, and tool calls fail closed and do not forward caller Authorization to M2M upstreams; inference logs a warning and does not promise self-heal without an explicit stamp.

Breaking: OAuth2 MCP servers in config.yaml must declare oauth2_flow.

^{Reviewed by Cursor Bugbot for commit a4d8d73. Bugbot is set up for automated code reviews on this repo. Configure here.}

tin-berri · 2026-07-06T22:59:08Z

@greptileai

greptile-apps · 2026-07-06T23:02:18Z

Greptile Summary

This PR completes the four-part oauth2_flow persistence sequence by making DB and config builds read the flow column verbatim rather than guessing from credential shape, and enforcing that config-defined oauth2 servers must declare the flow explicitly at startup.

DB build now calls _explicit_oauth2_flow (null/unknown → None, treated as interactive) instead of shape-inferring _resolve_oauth2_flow; the sole surviving inference is the request-time security backstop (resolve_oauth2_flow_for_request/effective_oauth2_flow), shared across the anonymous-delegate allowlist, the auth gate, and both the listing and tool-call egress paths.
Config validation raises ValueError at startup for any auth_type: oauth2 server that omits or misspells oauth2_flow, deleting the config-level shape guess entirely; the error text names both valid values and what each means.
Tests pin the verbatim-read contract, the config validator, and the tool-call gap closure introduced in this PR (unstamped M2M-shape row reaching execute_mcp_tool with the resolved flow).

Confidence Score: 5/5

Safe to merge; the auth gate and egress paths all route through a single resolution helper that fails closed for unstamped M2M-shape rows.

All three security-sensitive call sites (anonymous-delegate allowlist, auth gate, egress object copy) now go through effective_oauth2_flow / resolve_oauth2_flow_for_request. The DB verbatim-read and config validation changes are well-tested with regression tests covering null flow, both valid explicit values, the tool-call path, and the delegate bypass boundary cases. The two findings are a misleading inline comment and a config-breaking change already called out in the PR description.

mcp_server_manager.py — contains the breaking config validation and the misleading _resolve_oauth2_flow comment worth reviewing before merge.

Important Files Changed

Filename	Overview
litellm/proxy/_experimental/mcp_server/mcp_server_manager.py	Core change: adds `_explicit_oauth2_flow` (verbatim read), `effective_oauth2_flow` (column-first with shape fallback), and `resolve_oauth2_flow_for_request` (egress copy + warning); replaces shape-inferring `_resolve_oauth2_flow` calls in both config and DB build paths; adds startup config validation that hard-fails for oauth2 servers missing `oauth2_flow`. One misleading comment in `_resolve_oauth2_flow`.
litellm/proxy/_experimental/mcp_server/auth/user_api_key_auth_mcp.py	Anonymous-delegate auth gate now calls `MCPServerManager.effective_oauth2_flow(server)` instead of reading `server.has_client_credentials` directly, so unstamped M2M-shape rows still fail closed. Logic is correct and covers the PKCE bypass case.
litellm/proxy/_experimental/mcp_server/server.py	Both the listing path and the tool-call path now call `resolve_oauth2_flow_for_request` before appending to `allowed_mcp_servers`, closing the previously uncovered tool-call gap.
tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py	New test classes cover config validation (missing/invalid/valid flows), verbatim DB column reads (null, client_credentials, authorization_code), and effective-flow / resolve-for-request helpers including the warning-text assertions.
tests/test_litellm/proxy/_experimental/mcp_server/auth/test_user_api_key_auth_mcp.py	Adds tests for unstamped M2M-shape (bypass blocked), pure PKCE (bypass still allowed), and the anonymous-delegate allowlist excluding unstamped M2M. Two existing tests patched with `oauth2_flow: authorization_code` to pass the new config validator.
tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server.py	Adds regression test pinning that the tool-call path applies the oauth2_flow backstop; verifies the resolved server passed to `execute_mcp_tool` has `oauth2_flow=client_credentials` for a legacy null-flow M2M-shape row.

_{Reviews (4): Last reviewed commit: "fix(mcp): centralize the request-time oa..." | Re-trigger Greptile}

codecov · 2026-07-06T23:05:05Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

…s config-only plus a logged backstop With every DB write site stamping oauth2_flow (#32283, #32288) and the startup backfill healing legacy null rows, the DB build no longer needs to re-derive the flow from field shape. build_mcp_server_from_table now reads the column verbatim via _explicit_oauth2_flow: unknown or null values resolve to None, which needs_user_oauth_token already treats as interactive, so an unstamped row degrades to the safe default instead of guessing M2M from a shape that a DCR-registered interactive server shares whenever discovery is down Field-shape inference survives in exactly two places. config.yaml-loaded servers keep it at load time: they are rebuilt from the config on every boot, so there is no row to backfill and load-time resolution is their write-time stamp. And the request-time backstop in _get_allowed_mcp_servers keeps a not-yet-backfilled M2M row blocking caller Authorization forwarding (the P1 property); it now logs a warning whenever it actually fires, which is the fire-rate signal for deleting it once deployments have booted past the backfill Regression tests pin that the DB build does not infer M2M from the credential shape and reads an explicit column value verbatim Fourth step of the oauth2_flow persistence sequence, stacked on the backfill

…er than declared A config.yaml oauth2 server whose credential shape decides client_credentials without an explicit oauth2_flow now logs a warning at load pointing the admin at the explicit declaration. First rung of the deprecation ladder: the docs make oauth2_flow the recommended path, the warning surfaces configs still relying on inference, and a future breaking release can turn it into a config validation error, at which point config-level shape inference dies entirely. Interactive omissions stay silent since the default matches inference there and nothing load-bearing is being guessed

…rvers A config.yaml server with auth_type oauth2 must now declare its flow; the load raises a config validation error naming both values and what each means: oauth2_flow: client_credentials for machine-to-machine (the proxy mints a shared token at token_url using client_id/client_secret) or oauth2_flow: authorization_code for interactive (per-user tokens via browser sign-in, including delegate_auth_to_upstream) This replaces the load-time shape inference for config servers entirely. The credential shape is genuinely ambiguous (a DCR-registered interactive server carries client creds + token_url with no authorization_url, identical to M2M), so the config asserts the answer instead of the proxy guessing it. With this, field-shape inference survives in exactly one place: the request-time security backstop, which is telemetry-gated for deletion BREAKING CHANGE: config-defined oauth2 MCP servers without oauth2_flow fail proxy startup with the error above. Add the one line to the server block; the error text says exactly which value to pick

…ent_credentials Raised by review on the PR

veria-ai · 2026-07-07T02:03:07Z

PR overview

All previously flagged issues have been addressed. No open security concerns remain on this pull request.

Security review

No open security issues remain on this pull request.

Fixed/addressed: 1 · PR risk: 0/10

…M-shaped servers Reading oauth2_flow verbatim (this PR) changed has_client_credentials from True to False for a legacy null-flow row that still carries the M2M credential shape. That value is what the anonymous upstream-delegate gate checks before skipping LiteLLM auth entirely, so an M2M-shaped delegate server that was never stamped would newly pass the gate: an unauthenticated caller could get it selected and then list/read upstream data using the client credentials the request-time backstop re-infers, running as LiteLLM's service account. This reopens the hole the gate's existing 'never delegate for M2M' guard was written to close The gate now resolves the flow (column first, shape fallback) instead of reading the bare column, mirroring the request-time backstop in _get_allowed_mcp_servers: both fail closed on the ambiguous M2M shape and are removed together once no null rows remain. A pure-PKCE delegate server (no stored credentials) resolves to a non-M2M flow and keeps its bypass, so the common delegate case is unaffected Tests: an unstamped M2M-shaped delegate server is denied the bypass (mutation-checked against the bare-column regression), and a pure-PKCE delegate server still bypasses Raised by review on the PR

tin-berri · 2026-07-07T02:29:39Z

bugbot run

tin-berri · 2026-07-07T02:29:46Z

@greptileai rereview

…ry security site Reading oauth2_flow verbatim made has_client_credentials unreliable for legacy null rows, and the backstop that compensates was applied at only one reader. Review found three more consequences of that per-site approach: - the anonymous-delegate allowlist in get_allowed_mcp_servers read the bare column, so an unstamped M2M-shape delegate server was surfaced to anonymous callers (High) - call_mcp_tool resolved allowed ids into MCPServer objects without the backstop, so a null-flow M2M-shape row kept has_client_credentials false on tool execution during a backfill gap, though the listing path was covered (High) - the request-time warning claimed the startup backfill would stamp the row next boot, but the backfill deliberately leaves the ambiguous M2M shape unstamped (Low) Rather than patch each site, introduce two helpers on MCPServerManager that are the single choke point for request-time resolution: effective_oauth2_flow(server) for the enum/boolean decisions (allowlist filter, anonymous-delegate gate) and resolve_oauth2_flow_for_request(server) for the egress object copy (listing and tool call). Both fail closed on the M2M shape and leave stamped rows and pure-PKCE rows untouched. The gate now shares effective_oauth2_flow instead of its inline resolution, and the corrected warning lives once inside resolve_oauth2_flow_for_request, so deleting the whole transitional layer later is a single-site change. Tests: helper unit coverage (stamped verbatim, null M2M-shape resolves, pure-PKCE stays None, stamped/pure-PKCE return the same object, corrected warning text), the anonymous allowlist excludes an unstamped M2M-shape delegate server, and the call path resolves the flow like the listing path. The two security-integration tests are mutation-checked against the bare-column regression. Raised by review on the PR

tin-berri · 2026-07-07T03:04:53Z

@greptileai rereview

tin-berri · 2026-07-07T03:04:56Z

bugbot run

cursor

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit a4d8d73. Configure here.}

mateo-berri

The two findings are a misleading inline comment and a config-breaking change already called out in the PR description.

Are either of these legit? I'm assuming the latter you're fixing in the next PR?

tin-berri · 2026-07-07T05:51:13Z

@greptileai what are the two misleading inline comments you are talking about?

greptile-apps · 2026-07-07T05:53:27Z

To clarify: my summary mentioned two findings, not two misleading inline comments. They are:

A misleading inline comment — in the updated _resolve_oauth2_flow docstring in mcp_server_manager.py. The new docstring says:

"Not called directly by security sites; they go through effective_oauth2_flow (boolean/enum decisions) or resolve_oauth2_flow_for_request..."

This is technically accurate but could mislead a reader into thinking the function has no security relevance. It is the shape-inference engine that both security helpers delegate to, so someone reading it in isolation might incorrectly conclude it's safe to modify or remove without security implications. The comment would be clearer as something like "Always accessed through effective_oauth2_flow or resolve_oauth2_flow_for_request; do not call from security-sensitive sites directly."
A config-breaking change — the ValueError raised at startup for any auth_type: oauth2 server that omits oauth2_flow. This is an intentional breaking change already documented in the PR description, not a bug. I flagged it as a pre-merge note because it will break existing config-only deployments that relied on shape inference, even though the error message is clear and actionable.

Both are in mcp_server_manager.py. The inline review comment I left on the test file was a separate, already-resolved suggestion to add authorization_code verbatim-read coverage (which you added in 9f341b1d8e).

mateo-berri

LGTM

For future context,

We're introducing a breaking change because we currently rely on whether client_id, client_credentials, & token_url exists as a metric for whether we are in M2M vs Interactive (PKCE), but thats not reliable. When we have DCR (Dynamic Client Registration), the server will have the same fields The breaking change forces users to specify auth_flow=M2M / Interactive (PKCE)
The other comment by Greptile is trivial and addressed

…ers (#505) auth_type: oauth2 servers in config.yaml now require an explicit oauth2_flow (client_credentials or authorization_code); the proxy raises at startup when it is missing and no longer infers the flow from field shape for config entries (BerriAI/litellm#32292). Add the field to every oauth2 config example, update the mcp_oauth config reference row from optional-with-inference to required, and map each flow to its value in the flow comparison table. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

This comment was marked as outdated.

Sign in to view

greptile-apps Bot reviewed Jul 6, 2026

View reviewed changes

Comment thread tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py

tin-berri changed the title ~~refactor(mcp): read oauth2_flow verbatim from DB rows; inference stays config-only plus a logged backstop~~ feat(mcp)!: oauth2_flow read verbatim from DB rows and required in config; inference reduced to the request-time backstop Jul 6, 2026

tin-berri force-pushed the litellm_mcp_oauth2_flow_backfill branch from 13f4365 to e39acdf Compare July 7, 2026 00:16

tin-berri force-pushed the litellm_mcp_oauth2_flow_inference_removal branch from a3e248e to 9f341b1 Compare July 7, 2026 00:18

tin-berri mentioned this pull request Jul 7, 2026

feat(MCP/UI): add OAuth flow selector on the MCP edit page #32298

Merged

4 tasks

tin-berri force-pushed the litellm_mcp_oauth2_flow_backfill branch from e39acdf to 882419e Compare July 7, 2026 00:57

tin-berri force-pushed the litellm_mcp_oauth2_flow_inference_removal branch 2 times, most recently from f84a6f5 to 993c161 Compare July 7, 2026 01:18

Base automatically changed from litellm_mcp_oauth2_flow_backfill to litellm_internal_staging July 7, 2026 01:42

tin-berri added 4 commits July 6, 2026 18:43

test(mcp): pin the verbatim read for authorization_code alongside cli…

4ac467b

…ent_credentials Raised by review on the PR

tin-berri force-pushed the litellm_mcp_oauth2_flow_inference_removal branch from 993c161 to 4ac467b Compare July 7, 2026 01:43

veria-ai Bot reviewed Jul 7, 2026

View reviewed changes

Comment thread litellm/proxy/_experimental/mcp_server/mcp_server_manager.py

cursor Bot reviewed Jul 7, 2026

View reviewed changes

Comment thread litellm/proxy/_experimental/mcp_server/mcp_server_manager.py

Comment thread litellm/proxy/_experimental/mcp_server/server.py Outdated

Comment thread litellm/proxy/_experimental/mcp_server/server.py Outdated

cursor Bot reviewed Jul 7, 2026

View reviewed changes

mateo-berri requested changes Jul 7, 2026

View reviewed changes

mateo-berri approved these changes Jul 7, 2026

View reviewed changes

tin-berri merged commit 733c019 into litellm_internal_staging Jul 7, 2026
126 checks passed

tin-berri deleted the litellm_mcp_oauth2_flow_inference_removal branch July 7, 2026 16:55

tin-berri mentioned this pull request Jul 8, 2026

docs(mcp-oauth): document oauth2_flow as required for oauth2 MCP servers BerriAI/litellm-docs#505

Merged

Mattchewone mentioned this pull request Jul 20, 2026

chore: sync upstream v1.93.0 bitovi/litellm#14

Merged

Uh oh!

Uh oh!

Conversation

tin-berri commented Jul 6, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Relevant issues

Behavior

Linear ticket

Pre-Submission checklist

Delays in PR merge?

Screenshots / Proof of Fix

Live run (stack tip)

Type

Changes

Uh oh!

tin-berri commented Jul 6, 2026

Uh oh!

greptile-apps Bot commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Uh oh!

This comment was marked as outdated.

Uh oh!

codecov Bot commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

veria-ai Bot commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR overview

Security review

Uh oh!

tin-berri commented Jul 7, 2026

Uh oh!

tin-berri commented Jul 7, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tin-berri commented Jul 7, 2026

Uh oh!

tin-berri commented Jul 7, 2026

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

mateo-berri left a comment

Choose a reason for hiding this comment

Uh oh!

tin-berri commented Jul 7, 2026

Uh oh!

greptile-apps Bot commented Jul 7, 2026

Uh oh!

mateo-berri left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

tin-berri commented Jul 6, 2026 •

edited by cursor Bot

Loading

greptile-apps Bot commented Jul 6, 2026 •

edited

Loading

codecov Bot commented Jul 6, 2026 •

edited

Loading

veria-ai Bot commented Jul 7, 2026 •

edited

Loading