Skip to content

fix(proxy): read guardrail config from admin metadata, fix tag routing consistency - #25832

Closed
stuxf wants to merge 4 commits into
BerriAI:litellm_yj_apr15from
stuxf:fix/metadata-security-controls
Closed

fix(proxy): read guardrail config from admin metadata, fix tag routing consistency#25832
stuxf wants to merge 4 commits into
BerriAI:litellm_yj_apr15from
stuxf:fix/metadata-security-controls

Conversation

@stuxf

@stuxf stuxf commented Apr 16, 2026

Copy link
Copy Markdown
Collaborator

Relevant issues

Fixes inconsistencies in guardrail configuration resolution and tag-based routing/budget enforcement.

Pre-Submission checklist

  • I have Added testing in the tests/test_litellm/ directory, Adding at least 1 test is a hard requirement - see details
  • My PR passes all unit tests on make test-unit
  • 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

Type

🐛 Bug Fix
🧹 Refactoring

Changes

1. Read guardrail control flags from admin metadata

get_disable_global_guardrail() and get_opted_out_global_guardrails_from_metadata() in CustomGuardrail now read from user_api_key_metadata (admin-configured key/team metadata populated by the proxy) instead of scanning the top-level request body and user-supplied metadata. Extracted _get_admin_metadata() helper to deduplicate the lookup pattern.

Also strips _pipeline_managed_guardrails from user-supplied metadata at the proxy boundary since it is internal pipeline state.

2. Fix tag routing strict check

_match_deployment() in tag_based_routing.py previously required bool(request_tags) for the strict tag check to fire. This meant requests with no tags could bypass the strict policy and fall through to regex matching. Removed the bool(request_tags) condition so strict tag enforcement applies regardless of whether the request includes tags.

3. Fix tag budget metadata key resolution

budget_limiter.py used a hardcoded default "metadata" key when calling _get_tags_from_request_kwargs(), while the tag router dynamically resolves between "metadata" and "litellm_metadata". This inconsistency meant tags could be extracted differently by routing vs budget enforcement. Now passes metadata_variable_name from get_metadata_variable_name_from_kwargs() at all three call sites. Also hoisted tag resolution above the deployment loop since it's loop-invariant.

stuxf added 2 commits April 16, 2026 02:24
…g consistency

Read guardrail control flags (disable_global_guardrails, opted_out_global_guardrails)
from admin-configured key metadata instead of the request body. This ensures
callers cannot override admin security policies.

Fix tag-based routing to enforce strict tag checks regardless of whether the
request includes tags. Fix budget limiter to use the same dynamic metadata
key resolution as the tag router for consistent tag extraction.
…olution

Extract _get_admin_metadata() in CustomGuardrail to deduplicate metadata
lookup. Hoist tag resolution above the deployment loop in budget limiter.
Update stale comment in tag routing.
@vercel

vercel Bot commented Apr 16, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
litellm Ready Ready Preview, Comment Apr 16, 2026 2:48am

Request Review

@codecov

codecov Bot commented Apr 16, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 54.54545% with 5 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
litellm/router_strategy/budget_limiter.py 50.00% 5 Missing ⚠️

📢 Thoughts on this report? Let us know!

@greptile-apps

greptile-apps Bot commented Apr 16, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes three related proxy correctness issues: guardrail bypass via user-supplied metadata, regex routing bypassing strict tag policy, and inconsistent metadata key resolution in tag budget enforcement.

  • The guardrail change correctly reads disable_global_guardrails / opted_out_global_guardrails exclusively from admin-controlled user_api_key_metadata and user_api_key_team_metadata, closing the user-bypass vector.
  • Removing bool(request_tags) from strict_tag_check_failed is a deliberate security fix but is backward-incompatible: deployments with both tags and tag_regex under match_any=False will now reject tagless requests that previously matched via regex (per project policy, this warrants a feature flag or deprecation notice).
  • Removing turn_off_message_logging from _supported_callback_params is similarly backward-incompatible — existing users who set this per-request will have it silently ignored with no warning.

Confidence Score: 4/5

Safe to merge with awareness of two deliberate backward-incompatible changes that may affect existing operator configurations.

All three core fixes are logically correct and well-tested for the guardrail path. Two P2 findings reflect deliberate backward-incompatible behavioral changes (strict tag routing and per-request turn_off_message_logging) that violate the project's policy of requiring feature flags for such changes — operators upgrading could silently lose routing coverage or message-logging control without any error or deprecation notice.

litellm/router_strategy/tag_based_routing.py (routing behavior change), litellm/litellm_core_utils/initialize_dynamic_callback_params.py (silent removal of per-request turn_off_message_logging)

Important Files Changed

Filename Overview
litellm/integrations/custom_guardrail.py Adds _get_admin_metadata() helper that correctly merges user_api_key_team_metadata and user_api_key_metadata; refactors get_disable_global_guardrail and get_opted_out_global_guardrails_from_metadata to read only from admin-controlled metadata, closing the user-bypass vector.
litellm/router_strategy/tag_based_routing.py Removes bool(request_tags) from strict_tag_check_failed; fixes the security bypass but is a backward-incompatible change for deployments configured with both tags and tag_regex under match_any=False.
litellm/router_strategy/budget_limiter.py Fixes metadata variable name resolution for tag extraction (using get_metadata_variable_name_from_kwargs) and hoists loop-invariant tag resolution outside the deployment loop; no new tests for the changed behavior.
litellm/litellm_core_utils/initialize_dynamic_callback_params.py Removes turn_off_message_logging from _supported_callback_params, making it admin-only; silently ignores existing users who pass this in request bodies without a deprecation warning.
litellm/proxy/litellm_pre_call_utils.py Adds stripping of _pipeline_managed_guardrails from user-supplied metadata at proxy boundary; remaining changes are formatting/style only.
tests/test_litellm/integrations/test_custom_guardrail.py Tests updated to reflect new behavior: user-injected guardrail bypass flags are now expected to be ignored; admin-configured flags via user_api_key_metadata are verified to work.
tests/test_litellm/litellm_core_utils/test_initialize_dynamic_callback_params.py Adds explicit test confirming turn_off_message_logging cannot be set from request kwargs; updates the existing non-string test to remove the removed parameter.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Incoming Request] --> B[add_litellm_data_to_request]
    B --> B1[Strip _pipeline_managed_guardrails\nfrom user metadata]
    B1 --> C[Populate admin metadata\nuser_api_key_metadata\nuser_api_key_team_metadata]
    C --> D{Tag routing enabled?}
    D -- Yes --> E[get_deployments_for_tag]
    E --> F[_match_deployment]
    F --> G{Exact tag match?}
    G -- Yes --> H[Include deployment]
    G -- No --> I{strict_tag_check_failed?\nnot match_any AND bool deployment_tags}
    I -- True --> J[Block regex path]
    I -- False --> K{Regex match?}
    K -- Yes --> H
    K -- No --> L[Exclude deployment]
    D -- No --> M[All deployments eligible]
    H --> N[RouterBudgetLimiting]
    M --> N
    N --> O[_get_tags_from_request_kwargs\nwith correct metadata_variable_name]
    O --> P[Check tag budget in cache]
    P --> Q{should_run_guardrail?}
    Q --> R[_get_admin_metadata\nmerge team_meta + key_meta]
    R --> S{disable_global_guardrails\nor opted_out?}
    S -- Yes --> T[Skip guardrail]
    S -- No --> U[Run guardrail]
Loading

Reviews (2): Last reviewed commit: "test: update dynamic callback params tes..." | Re-trigger Greptile

Comment thread litellm/integrations/custom_guardrail.py Outdated
…ging from dynamic params

Include user_api_key_team_metadata alongside user_api_key_metadata in
_get_admin_metadata() so team-level guardrail settings are respected.
Key-level settings take precedence over team-level.

Remove turn_off_message_logging from _supported_callback_params so it
cannot be set via request metadata. Admin controls logging globally
or via key/team configuration.

Update tests to verify user-injected guardrail flags are ignored while
admin-configured flags are respected.
…g removal

Verify turn_off_message_logging is no longer extracted from request
kwargs since it is now admin-only.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants