mirror of
https://github.com/meta-llama/llama-stack.git
synced 2025-12-03 09:53:45 +00:00
rejecting headers that include Authorization in the header and pointing them to the authorization param.
This commit is contained in:
parent
411b18a90f
commit
18aff1abaa
2 changed files with 22 additions and 20 deletions
|
|
@ -27,30 +27,33 @@ from llama_stack.providers.utils.tools.ttl_dict import TTLDict
|
|||
|
||||
logger = get_logger(__name__, category="tools")
|
||||
|
||||
|
||||
def prepare_mcp_headers(base_headers: dict[str, str] | None, authorization: str | None) -> dict[str, str]:
|
||||
"""Prepare headers for MCP requests with authorization handling.
|
||||
"""
|
||||
Prepare headers for MCP requests with authorization support.
|
||||
|
||||
Args:
|
||||
base_headers: Base headers to use (e.g., from mcp_tool.headers)
|
||||
authorization: OAuth access token (just the token, not "Bearer <token>")
|
||||
base_headers: Base headers dictionary (can be None)
|
||||
authorization: OAuth access token (without "Bearer " prefix)
|
||||
|
||||
Returns:
|
||||
Final headers dict with Authorization header if authorization is provided
|
||||
Headers dictionary with Authorization header if token provided
|
||||
|
||||
Raises:
|
||||
ValueError: If both base_headers contains Authorization and authorization parameter is provided
|
||||
ValueError: If Authorization header is specified in the headers dict (security risk)
|
||||
"""
|
||||
headers = dict(base_headers or {})
|
||||
|
||||
# Security check: reject any Authorization header in the headers dict
|
||||
# Users must use the authorization parameter instead to avoid security risks
|
||||
existing_keys_lower = {k.lower() for k in headers.keys()}
|
||||
if "authorization" in existing_keys_lower:
|
||||
raise ValueError(
|
||||
"For security reasons, Authorization header cannot be passed via 'headers'. "
|
||||
"Please use the 'authorization' parameter instead."
|
||||
)
|
||||
|
||||
# Add Authorization header if token provided
|
||||
if authorization:
|
||||
# Check if Authorization header already exists (case-insensitive check)
|
||||
existing_keys_lower = {k.lower() for k in headers.keys()}
|
||||
if "authorization" in existing_keys_lower:
|
||||
raise ValueError(
|
||||
"Cannot specify Authorization in both 'headers' and 'authorization' fields. "
|
||||
"Please use only the 'authorization' field."
|
||||
)
|
||||
# OAuth access token - add "Bearer " prefix
|
||||
headers["Authorization"] = f"Bearer {authorization}"
|
||||
|
||||
|
|
|
|||
|
|
@ -91,8 +91,8 @@ def test_mcp_authorization_different_token(compat_client, text_model_id):
|
|||
assert response.output[1].error is None
|
||||
|
||||
|
||||
def test_mcp_authorization_error_when_both_provided(compat_client, text_model_id):
|
||||
"""Test that providing both headers['Authorization'] and authorization field raises an error."""
|
||||
def test_mcp_authorization_error_when_header_provided(compat_client, text_model_id):
|
||||
"""Test that providing Authorization in headers raises a security error."""
|
||||
if not isinstance(compat_client, LlamaStackAsLibraryClient):
|
||||
pytest.skip("in-process MCP server is only supported in library client")
|
||||
|
||||
|
|
@ -102,18 +102,17 @@ def test_mcp_authorization_error_when_both_provided(compat_client, text_model_id
|
|||
[
|
||||
{
|
||||
"type": "mcp",
|
||||
"server_label": "both-auth-mcp",
|
||||
"server_label": "header-auth-mcp",
|
||||
"server_url": "<FILLED_BY_TEST_RUNNER>",
|
||||
"headers": {"Authorization": f"Bearer {test_token}"},
|
||||
"authorization": "should-cause-error", # This should trigger an error
|
||||
"headers": {"Authorization": f"Bearer {test_token}"}, # Security risk - should be rejected
|
||||
}
|
||||
],
|
||||
mcp_server_info,
|
||||
)
|
||||
|
||||
# Create response - should raise ValueError
|
||||
# Create response - should raise ValueError for security reasons
|
||||
with pytest.raises(
|
||||
ValueError, match="Cannot specify Authorization in both 'headers' and 'authorization' fields"
|
||||
ValueError, match="For security reasons, Authorization header cannot be passed via 'headers'"
|
||||
):
|
||||
compat_client.responses.create(
|
||||
model=text_model_id,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue