[Fix] Agent endpoint and routing permission checks#25922
[Fix] Agent endpoint and routing permission checks#25922yuneng-berri merged 8 commits intolitellm_yj_apr17from
Conversation
…headers and add tests - Move protected-headers set to module level as a frozenset - Add x-api-key, x-goog-api-key to protected set (provider credential headers) - Block x-amz- prefix to cover AWS SigV4 signing headers - Normalize forwarded header names to lowercase on write - Log at debug level when a protected header is skipped - Add unit test covering protected-header drop and non-protected forwarding
[Fix] Restrict x-pass- header forwarding for credential and protocol headers
[Fix] Tighten api_key value check in credential validation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR adds per-agent ACL enforcement to two previously unguarded paths: Confidence Score: 5/5Safe to merge — the permission checks are correctly implemented, admin bypass is consistent, and all remaining considerations are pre-existing or P2. No P0 or P1 findings. Both new ACL check sites correctly call is_agent_allowed for non-admins, return 403 on denial, and allow all agents when the allowed list is empty (unrestricted keys). The route_request signature change is backwards-compatible (Optional with default None). The async conversion of route_a2a_agent_request is consistent with the surrounding coroutine-return pattern used throughout route_request. No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/proxy/agent_endpoints/a2a_routing.py | Converted to async and added is_agent_allowed check before injecting api_base; admin shortcut correctly guards the None case; pre-existing fail-open on DB error is noted in prior threads. |
| litellm/proxy/agent_endpoints/endpoints.py | get_agent_by_id now enforces ACL for non-admins after the feature-flag check; remaining changes are Black reformats of multi-line assignments. |
| litellm/proxy/common_request_processing.py | Sole functional change: threads user_api_key_dict through to route_request; all other diffs are Black formatting. |
| litellm/proxy/route_llm_request.py | Adds optional user_api_key_dict param to route_request and forwards it to the now-awaited route_a2a_agent_request; backwards-compatible change. |
Sequence Diagram
sequenceDiagram
participant Client
participant Auth as user_api_key_auth
participant EP as endpoints.py get_agent_by_id
participant CR as common_request_processing
participant RR as route_request
participant A2A as route_a2a_agent_request
participant APH as AgentRequestHandler is_agent_allowed
participant LiteLLM
Client->>Auth: GET /v1/agents/{id} or POST /v1/chat/completions
Auth-->>EP: user_api_key_dict
alt GET /v1/agents/{id}
EP->>EP: check_feature_access_for_user
EP->>EP: is_admin check
alt not admin
EP->>APH: is_agent_allowed(agent_id, user_api_key_dict)
APH-->>EP: True / False
EP->>EP: raise 403 if not allowed
end
EP-->>Client: AgentResponse
else POST /v1/chat/completions with a2a/ model
CR->>RR: route_request(..., user_api_key_dict)
RR->>RR: _is_a2a_agent_model check
RR->>A2A: await route_a2a_agent_request(data, route_type, user_api_key_dict)
A2A->>A2A: is_admin check
alt not admin
A2A->>APH: is_agent_allowed(agent_id, user_api_key_dict)
APH-->>A2A: True / False
A2A->>A2A: raise 403 if not allowed
end
A2A->>LiteLLM: litellm.route_type with api_base injected
LiteLLM-->>Client: completion response
end
Reviews (2): Last reviewed commit: "fix: add explicit admin bypass to agent ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| is_allowed = await AgentRequestHandler.is_agent_allowed( | ||
| agent_id=agent_id, user_api_key_auth=user_api_key_dict | ||
| ) | ||
| if not is_allowed: | ||
| raise HTTPException( | ||
| status_code=403, | ||
| detail=f"Agent '{agent_id}' is not allowed for your key/team. Contact proxy admin for access.", | ||
| ) |
There was a problem hiding this comment.
get_agents (the list endpoint) has an explicit admin shortcut that skips the ACL entirely. get_agent_by_id calls is_agent_allowed for all users, relying on the implicit behaviour that admins have no object_permission so get_allowed_agents returns [] → allow all. In practice both paths produce the same result, but if an admin key were ever created with a non-empty object_permission.agents, get_agent_by_id would 403 them while GET /v1/agents would still return the full list. Consider adding the same explicit guard used in the list endpoint for consistency:
if (
user_api_key_dict.user_role == LitellmUserRoles.PROXY_ADMIN
or user_api_key_dict.user_role == LitellmUserRoles.PROXY_ADMIN.value
):
pass # admin always allowed
else:
is_allowed = await AgentRequestHandler.is_agent_allowed(...)
if not is_allowed:
raise HTTPException(status_code=403, ...)| # Verify the caller is permitted to use this agent | ||
| is_allowed = await AgentRequestHandler.is_agent_allowed( | ||
| agent_id=agent.agent_id, | ||
| user_api_key_auth=user_api_key_dict, | ||
| ) | ||
| if not is_allowed: | ||
| raise HTTPException( | ||
| status_code=403, | ||
| detail=f"Agent '{agent_name}' is not allowed for your key/team. Contact proxy admin for access.", | ||
| ) |
There was a problem hiding this comment.
AgentRequestHandler.get_allowed_agents swallows all exceptions and returns [], which is_agent_allowed treats as "no restrictions → allow all". If the DB or cache is unavailable while fetching team permissions, every A2A request silently passes the ACL check. This is pre-existing behaviour in the permission handler and is now active on the critical completion path. Consider whether a DB-error case should deny rather than allow, or at least emit a metric to make the silent pass-through observable.
yuneng-berri
temporarily deployed
to
integration-postgres
GitHub Actions
Inactive
yuneng-berri
temporarily deployed
to
integration-postgres
GitHub Actions
Inactive
yuneng-berri
temporarily deployed
to
integration-postgres
GitHub Actions
Inactive
yuneng-berri
had a problem deploying
to
integration-postgres
GitHub Actions
Error
yuneng-berri
changed the base branch from
litellm_internal_staging
to
litellm_yj_apr17
1 participant
Summary
GET /v1/agents/{agent_id}endpoint verified feature access but did not enforce per-agent ACLs. Any authenticated user with the agents feature enabled could fetch the full configuration of any agent.route_a2a_agent_requesthelper, used when routinga2a/<name>model strings through/v1/chat/completions, had no permission check before forwarding to the backend.Changes
get_agent_by_idnow callsAgentRequestHandler.is_agent_allowedafter the feature-flag check, returning 403 if the caller's key/team does not include the requested agent in its allowed list.route_a2a_agent_requestis now async and accepts an optionaluser_api_key_dict. It callsAgentRequestHandler.is_agent_allowedbefore injectingapi_baseand forwarding the request.route_requestinroute_llm_request.pynow accepts and passes throughuser_api_key_dict, andbase_process_llm_requestforwards it from the handler. Both changes follow the pattern already used ininvoke_agent_a2aandget_agent_card.Testing
GET /v1/agents/{id}returns 403 for a key whoseobject_permission.agentsdoes not include the requested agent (was 200 before).static_headers.object_permission) still get 200 on any agent.uv run pytest tests/test_litellm/proxy/agent_endpoints/ tests/test_litellm/proxy/test_route_a2a_models.py -v— 65 passed.Type
🐛 Bug Fix
✅ Test