fix(mcp): run pre_call_tool_check on OpenAPI/local-registry path (VERIA-7)

[stuxf]

Summary

`execute_mcp_tool` dispatches in two ways:

• Managed MCP servers → `_handle_managed_mcp_tool` → `MCPServerManager.pre_call_tool_check`, which enforces allowed/banned tool lists, key/team `object_permission` tool grants, and parameter validation.
• OpenAPI-backed tools → `global_mcp_tool_registry.get_tool` → `_handle_local_mcp_tool` directly. No pre-call check.

A caller could therefore invoke any registered OpenAPI tool regardless of their key/team permissions, including administrative or destructive operations on the upstream API.

This patch runs `pre_call_tool_check` before the local-registry dispatch whenever the resolved server is set (the same condition used to surface server context to the managed path) and honors any guardrail-modified arguments the hook returns. Errors raised by the hook propagate up before `_handle_local_mcp_tool` runs.

Behavior changes

• A caller without the `object_permission.allowed_tools` grant for an OpenAPI tool now gets `403` instead of executing the tool.
• Banned tools (`disallowed_tools` on the server config) are also blocked on the local path.
• Parameter validation (`allowed_params`) and pre-call guardrail hooks fire for OpenAPI tools too.

Test plan

• `uv run pytest tests/test_litellm/proxy/_experimental/mcp_server/test_openapi_tool_auth.py -q` — 2 pass (hook fires + hook-blocked tool not executed)
• `uv run black --check` on touched files

Type

🐛 Bug Fix
✅ Test

[greptile-apps]

Greptile Summary

This PR closes an auth-bypass on the OpenAPI/local-registry MCP tool path by inserting pre_call_tool_check before _handle_local_mcp_tool runs, mirroring the existing check on the managed-server path. Both previously-noted gaps (proxy_logging_obj null-crash and unguarded None-server dispatch) are addressed in the new code. The remaining P2 items are a backwards-compatibility concern for the hard 503 when server resolution fails and a fragile test assertion on proxy_logging_obj initialisation state.

Confidence Score: 5/5

Safe to merge; no P0 or P1 issues found — only P2 suggestions.

The core security fix is correctly implemented: pre_call_tool_check fires before local dispatch, proxy_logging_obj is sourced from the canonical module, and the mcp_server=None path is hard-blocked. Both previous review thread concerns are resolved. Remaining findings are P2 (backwards-compat flag suggestion and a test assertion fragility), neither of which blocks correctness.

No files require special attention beyond the P2 notes above.

Important Files Changed

Filename	Overview
litellm/proxy/_experimental/mcp_server/server.py	Adds pre_call_tool_check before local-registry dispatch; correctly guards against mcp_server is None with a 503, imports proxy_logging_obj from the canonical module, and propagates guardrail-modified arguments. The 503 hard-failure for any tool without a server mapping is a backwards-incompatible change with no feature flag.
tests/test_litellm/proxy/_experimental/mcp_server/test_openapi_tool_auth.py	New mock-only test file with 3 tests covering hook-fires, hook-blocked, and server-not-resolvable paths; the PR description lists "2 pass" but the file contains 3 tests. The proxy_logging_obj is not None assertion in test 1 is tied to module-level initialisation state.

_{Reviews (2): Last reviewed commit: "fix(mcp): use canonical proxy_logging_ob..." | Re-trigger Greptile}

[codecov]

Codecov Report

❌ Patch coverage is 40.00000% with 6 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
litellm/proxy/_experimental/mcp_server/server.py	40.00%	6 Missing ⚠️

📢 Thoughts on this report? Let us know!

[stuxf]

Greptile P1 + P2 fixed in 8ee599a:

• P1 (None proxy_logging_obj crash): the call now sources proxy_logging_obj from litellm.proxy.proxy_server the same way _handle_managed_mcp_tool does. Test test_openapi_local_tool_runs_pre_call_tool_check now asserts proxy_logging_obj is not None at the call site.

• P2 (mcp_server-is-None bypass): if the tool resolves locally but no server can be found, we now fail with 503 instead of dispatching unguarded. New test test_openapi_local_tool_denied_when_server_not_resolvable covers the deny branch. OpenAPI-backed tools are always registered with a server context, so an unresolvable server is a startup-race or orphaned-entry configuration fault, not a normal path.

@greptileai

[greptile-apps]

Both fixes are solid. The implementation is correct:

P1 — The from litellm.proxy.proxy_server import proxy_logging_obj import at call-time captures the live module-level object, matching how _handle_managed_mcp_tool sources it. The assertion pre_call_kwargs["proxy_logging_obj"] is not None in test_openapi_local_tool_runs_pre_call_tool_check is a meaningful runtime guard.

P2 — The 503 before the pre_call_tool_check call is the right place to fail. If it were after, you'd still have a potential gap if pre_call_tool_check itself tried to use mcp_server. The reasoning in the comment ("always tied to a server → unresolvable means startup race or orphaned entry") is sound, and 503 (Service Unavailable) is the right status code for a transient infrastructure fault vs. 403 for a policy denial.

The three tests give good coverage of the three branches: normal dispatch, policy deny, and infrastructure deny. LGTM.

[stuxf]

Bumping for re-review. @greptileai please re-trigger.