mirror of
https://github.com/meta-llama/llama-stack.git
synced 2025-12-03 09:53:45 +00:00
security: enforce Authorization rejection in remote MCP provider
Addresses reviewer concern about token isolation between services. The remote provider now rejects Authorization headers in mcp_headers to prevent accidentally passing inference tokens to MCP servers. This makes the remote provider consistent with the inline provider: - Both reject Authorization in headers dict - Both require dedicated authorization parameter - Prevents token leakage across service boundaries Related changes: - Added validation in get_headers_from_request() - Throws ValueError if Authorization found in mcp_headers - Added TODO for dedicated authorization field in provider_data
This commit is contained in:
parent
2b0423c337
commit
a842c90059
1 changed files with 23 additions and 7 deletions
|
|
@ -64,14 +64,22 @@ class ModelContextProtocolToolRuntimeImpl(ToolGroupsProtocolPrivate, ToolRuntime
|
||||||
authorization=authorization,
|
authorization=authorization,
|
||||||
)
|
)
|
||||||
|
|
||||||
async def get_headers_from_request(self, mcp_endpoint_uri: str) -> tuple[dict[str, str], str | None]:
|
async def get_headers_from_request(
|
||||||
|
self, mcp_endpoint_uri: str
|
||||||
|
) -> tuple[dict[str, str], str | None]:
|
||||||
"""
|
"""
|
||||||
Extract headers and authorization from request provider data.
|
Extract headers and authorization from request provider data.
|
||||||
|
|
||||||
|
For security, Authorization should not be passed via mcp_headers.
|
||||||
|
Instead, use a dedicated authorization field in the provider data.
|
||||||
|
|
||||||
Returns:
|
Returns:
|
||||||
Tuple of (headers_dict, authorization_token)
|
Tuple of (headers_dict, authorization_token)
|
||||||
- headers_dict: All headers except Authorization
|
- headers_dict: All headers except Authorization
|
||||||
- authorization_token: Token from Authorization header (with "Bearer " prefix removed), or None
|
- authorization_token: Token from Authorization header (with "Bearer " prefix removed), or None
|
||||||
|
|
||||||
|
Raises:
|
||||||
|
ValueError: If Authorization header is found in mcp_headers (security risk)
|
||||||
"""
|
"""
|
||||||
|
|
||||||
def canonicalize_uri(uri: str) -> str:
|
def canonicalize_uri(uri: str) -> str:
|
||||||
|
|
@ -85,12 +93,20 @@ class ModelContextProtocolToolRuntimeImpl(ToolGroupsProtocolPrivate, ToolRuntime
|
||||||
for uri, values in provider_data.mcp_headers.items():
|
for uri, values in provider_data.mcp_headers.items():
|
||||||
if canonicalize_uri(uri) != canonicalize_uri(mcp_endpoint_uri):
|
if canonicalize_uri(uri) != canonicalize_uri(mcp_endpoint_uri):
|
||||||
continue
|
continue
|
||||||
# Extract Authorization header separately for security
|
|
||||||
for key, value in values.items():
|
# Security check: reject Authorization header in mcp_headers
|
||||||
|
# This prevents accidentally passing inference tokens to MCP servers
|
||||||
|
for key in values.keys():
|
||||||
if key.lower() == "authorization":
|
if key.lower() == "authorization":
|
||||||
# Remove "Bearer " prefix if present
|
raise ValueError(
|
||||||
authorization = value.removeprefix("Bearer ").strip()
|
"Authorization header cannot be passed via 'mcp_headers'. "
|
||||||
else:
|
"Please use a dedicated authorization field in provider_data instead."
|
||||||
headers[key] = value
|
)
|
||||||
|
|
||||||
|
# Collect all headers (Authorization already rejected above)
|
||||||
|
headers.update(values)
|
||||||
|
|
||||||
|
# TODO: Extract authorization from a dedicated field in provider_data
|
||||||
|
# For now, authorization remains None until the API is updated
|
||||||
|
|
||||||
return headers, authorization
|
return headers, authorization
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue