mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-24 00:52:24 +00:00
fix(policy_engine): ignore inapplicable non-default attachments when selecting defaults
A non-default attachment whose policy is missing or whose condition does not match the request no longer suppresses default attachments. The impact preview marks default counts as an upper bound Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
parent
7fc114c24f
commit
ef55eb6bdf
9 changed files with 94 additions and 10 deletions
|
|
@ -3216,7 +3216,9 @@ def _match_and_track_policies(
|
|||
attachment_registry: Final = (
|
||||
attachment_registry_override if attachment_registry_override is not None else get_attachment_registry()
|
||||
)
|
||||
matches_with_reasons: Final = attachment_registry.get_attached_policies_with_reasons(context)
|
||||
matches_with_reasons: Final = attachment_registry.get_attached_policies_with_reasons(
|
||||
context, PolicyMatcher.policy_applies(context, policies_override)
|
||||
)
|
||||
matching_policy_names: Final = [m["policy_name"] for m in matches_with_reasons]
|
||||
policy_reasons: Final = {m["policy_name"]: m["matched_via"] for m in matches_with_reasons}
|
||||
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ Attachments define WHERE policies apply, separate from the policy definitions.
|
|||
This allows the same policy to be attached to multiple scopes.
|
||||
"""
|
||||
|
||||
from collections.abc import Callable
|
||||
from datetime import datetime, timezone
|
||||
from types import MappingProxyType
|
||||
from typing import TYPE_CHECKING, Any, Final, TypedDict
|
||||
|
|
@ -122,24 +123,34 @@ class AttachmentRegistry:
|
|||
default=attachment_data.get("default", False),
|
||||
)
|
||||
|
||||
def get_attached_policies(self, context: PolicyMatchContext) -> list[str]:
|
||||
def get_attached_policies(
|
||||
self,
|
||||
context: PolicyMatchContext,
|
||||
policy_applies: Callable[[str], bool] | None = None,
|
||||
) -> list[str]:
|
||||
"""
|
||||
Get list of policy names attached to the given context.
|
||||
|
||||
Args:
|
||||
context: The request context to match against
|
||||
policy_applies: Optional predicate; attachments whose policy does not apply are ignored
|
||||
|
||||
Returns:
|
||||
List of policy names that are attached to matching scopes
|
||||
"""
|
||||
return [r["policy_name"] for r in self.get_attached_policies_with_reasons(context)]
|
||||
return [r["policy_name"] for r in self.get_attached_policies_with_reasons(context, policy_applies)]
|
||||
|
||||
def get_attached_policies_with_reasons(self, context: PolicyMatchContext) -> list[PolicyAttachmentMatch]:
|
||||
def get_attached_policies_with_reasons(
|
||||
self,
|
||||
context: PolicyMatchContext,
|
||||
policy_applies: Callable[[str], bool] | None = None,
|
||||
) -> list[PolicyAttachmentMatch]:
|
||||
"""
|
||||
Get list of policy names and match reasons for the given context.
|
||||
|
||||
Returns a list of dicts with 'policy_name' and 'matched_via' keys.
|
||||
The 'matched_via' describes which dimension caused the match.
|
||||
Attachments whose policy fails `policy_applies` are dropped before defaults are considered.
|
||||
"""
|
||||
from litellm.proxy.policy_engine.policy_matcher import PolicyMatcher
|
||||
|
||||
|
|
@ -147,6 +158,7 @@ class AttachmentRegistry:
|
|||
attachment
|
||||
for attachment in self._attachments
|
||||
if PolicyMatcher.scope_matches(scope=attachment.to_policy_scope(), context=context)
|
||||
and (policy_applies is None or policy_applies(attachment.policy))
|
||||
)
|
||||
non_default: Final = tuple(attachment for attachment in in_scope if not attachment.default)
|
||||
matching_attachments: Final = sorted(
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ apply to a given request based on team alias, key alias, and model.
|
|||
Policies are matched via policy_attachments which define WHERE each policy applies.
|
||||
"""
|
||||
|
||||
from collections.abc import Callable
|
||||
from typing import Final
|
||||
|
||||
from litellm._logging import verbose_proxy_logger
|
||||
|
|
@ -130,6 +131,20 @@ class PolicyMatcher:
|
|||
"""
|
||||
return PolicyMatcher.get_matching_policies(context=context)
|
||||
|
||||
@staticmethod
|
||||
def policy_applies(
|
||||
context: PolicyMatchContext,
|
||||
policies: dict[str, Policy] | None = None,
|
||||
) -> Callable[[str], bool]:
|
||||
"""Predicate telling whether a policy exists and its condition matches the context."""
|
||||
return lambda policy_name: bool(
|
||||
PolicyMatcher.get_policies_with_matching_conditions(
|
||||
policy_names=[policy_name], # mutable-ok: the matcher takes a list
|
||||
context=context,
|
||||
policies=policies,
|
||||
)
|
||||
)
|
||||
|
||||
@staticmethod
|
||||
def get_policies_with_matching_conditions(
|
||||
policy_names: list[str],
|
||||
|
|
|
|||
|
|
@ -265,7 +265,9 @@ async def resolve_policies_for_context(
|
|||
)
|
||||
|
||||
# Get matching policies with reasons
|
||||
match_results: Final = get_attachment_registry().get_attached_policies_with_reasons(context=context)
|
||||
match_results: Final = get_attachment_registry().get_attached_policies_with_reasons(
|
||||
context=context, policy_applies=PolicyMatcher.policy_applies(context)
|
||||
)
|
||||
|
||||
if not match_results:
|
||||
return PolicyResolveResponse(
|
||||
|
|
|
|||
|
|
@ -84,7 +84,9 @@ def _retrieval_context(
|
|||
|
||||
|
||||
def _post_call_pipelines_for_context(context: PolicyMatchContext) -> tuple[PolicyPipelines, Mapping[str, str]]:
|
||||
matches: Final = get_attachment_registry().get_attached_policies_with_reasons(context)
|
||||
matches: Final = get_attachment_registry().get_attached_policies_with_reasons(
|
||||
context, PolicyMatcher.policy_applies(context)
|
||||
)
|
||||
if not matches:
|
||||
return (), MappingProxyType({})
|
||||
applied_policy_names: Final = PolicyMatcher.get_policies_with_matching_conditions(
|
||||
|
|
|
|||
|
|
@ -14,7 +14,8 @@ from litellm.proxy.policy_engine.attachment_registry import (
|
|||
AttachmentRegistry,
|
||||
get_attachment_registry,
|
||||
)
|
||||
from litellm.types.proxy.policy_engine import PolicyMatchContext
|
||||
from litellm.proxy.policy_engine.policy_matcher import PolicyMatcher
|
||||
from litellm.types.proxy.policy_engine import Policy, PolicyCondition, PolicyGuardrails, PolicyMatchContext
|
||||
|
||||
|
||||
class TestGetAttachedPolicies:
|
||||
|
|
@ -561,6 +562,38 @@ class TestDefaultAttachments:
|
|||
|
||||
assert results == [{"policy_name": "guardrail-y", "matched_via": "default:scope:*"}]
|
||||
|
||||
def test_inapplicable_opt_in_policy_does_not_suppress_default(self):
|
||||
context = PolicyMatchContext(team_alias="t", key_alias="k", model="gpt-5.2", tags=["opt-in"])
|
||||
policies = {
|
||||
"guardrail-y": Policy(guardrails=PolicyGuardrails(add=["y"])),
|
||||
"guardrail-x": Policy(guardrails=PolicyGuardrails(add=["x"]), condition=PolicyCondition(model="claude.*")),
|
||||
}
|
||||
|
||||
results = self._registry().get_attached_policies_with_reasons(
|
||||
context, PolicyMatcher.policy_applies(context, policies)
|
||||
)
|
||||
|
||||
assert results == [{"policy_name": "guardrail-y", "matched_via": "default:scope:*"}]
|
||||
|
||||
def test_attachment_to_missing_policy_does_not_suppress_default(self):
|
||||
context = PolicyMatchContext(team_alias="t", key_alias="k", model="gpt-5.2", tags=["opt-in"])
|
||||
policies = {"guardrail-y": Policy(guardrails=PolicyGuardrails(add=["y"]))}
|
||||
|
||||
assert self._registry().get_attached_policies(context, PolicyMatcher.policy_applies(context, policies)) == [
|
||||
"guardrail-y"
|
||||
]
|
||||
|
||||
def test_applicable_opt_in_policy_still_wins_with_predicate(self):
|
||||
context = PolicyMatchContext(team_alias="t", key_alias="k", model="gpt-5.2", tags=["opt-in"])
|
||||
policies = {
|
||||
"guardrail-y": Policy(guardrails=PolicyGuardrails(add=["y"])),
|
||||
"guardrail-x": Policy(guardrails=PolicyGuardrails(add=["x"]), condition=PolicyCondition(model="gpt.*")),
|
||||
}
|
||||
|
||||
assert self._registry().get_attached_policies(context, PolicyMatcher.policy_applies(context, policies)) == [
|
||||
"guardrail-x"
|
||||
]
|
||||
|
||||
def test_default_defaults_to_false_when_omitted(self):
|
||||
registry = AttachmentRegistry()
|
||||
registry.load_attachments([{"policy": "p"}])
|
||||
|
|
|
|||
|
|
@ -473,7 +473,7 @@ const AddAttachmentForm: React.FC<AddAttachmentFormProps> = ({
|
|||
</FormField>
|
||||
</FieldGroup>
|
||||
|
||||
{impactResult && <ImpactPreviewAlert impactResult={impactResult} />}
|
||||
{impactResult && <ImpactPreviewAlert impactResult={impactResult} isDefault={form.watch("default")} />}
|
||||
|
||||
<div className="flex justify-end space-x-2 mt-4">
|
||||
<Button type="button" variant="secondary" onClick={handleClose}>
|
||||
|
|
|
|||
|
|
@ -69,6 +69,17 @@ describe("ImpactPreviewAlert", () => {
|
|||
expect(screen.getByText(/1 key\b/i)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("should present the counts as an upper bound for a default attachment", () => {
|
||||
renderWithProviders(<ImpactPreviewAlert impactResult={specificImpact} isDefault />);
|
||||
expect(screen.getByText(/would affect up to/i)).toBeInTheDocument();
|
||||
expect(screen.getByText(/no non-default attachment matches/i)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("should not qualify the counts for a non-default attachment", () => {
|
||||
renderWithProviders(<ImpactPreviewAlert impactResult={specificImpact} />);
|
||||
expect(screen.queryByText(/up to/i)).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("should not show a key section when there are no sample keys", () => {
|
||||
const noKeys = { affected_keys_count: 0, affected_teams_count: 2, sample_keys: [], sample_teams: ["t1", "t2"] };
|
||||
renderWithProviders(<ImpactPreviewAlert impactResult={noKeys} />);
|
||||
|
|
|
|||
|
|
@ -12,6 +12,7 @@ interface ImpactResult {
|
|||
|
||||
interface ImpactPreviewAlertProps {
|
||||
impactResult: ImpactResult;
|
||||
isDefault?: boolean;
|
||||
}
|
||||
|
||||
interface SampleListProps {
|
||||
|
|
@ -32,8 +33,9 @@ const SampleList: React.FC<SampleListProps> = ({ label, samples, totalCount }) =
|
|||
</div>
|
||||
);
|
||||
|
||||
const ImpactPreviewAlert: React.FC<ImpactPreviewAlertProps> = ({ impactResult }) => {
|
||||
const ImpactPreviewAlert: React.FC<ImpactPreviewAlertProps> = ({ impactResult, isDefault = false }) => {
|
||||
const isGlobal = impactResult.affected_keys_count === -1;
|
||||
const qualifier = isDefault ? "up to " : "";
|
||||
|
||||
return (
|
||||
<Alert className="mb-4">
|
||||
|
|
@ -47,7 +49,7 @@ const ImpactPreviewAlert: React.FC<ImpactPreviewAlertProps> = ({ impactResult })
|
|||
) : (
|
||||
<div>
|
||||
<span>
|
||||
This attachment would affect{" "}
|
||||
This attachment would affect {qualifier}
|
||||
<strong>
|
||||
{impactResult.affected_keys_count} key{impactResult.affected_keys_count !== 1 ? "s" : ""}
|
||||
</strong>{" "}
|
||||
|
|
@ -57,6 +59,11 @@ const ImpactPreviewAlert: React.FC<ImpactPreviewAlertProps> = ({ impactResult })
|
|||
</strong>
|
||||
.
|
||||
</span>
|
||||
{isDefault && (
|
||||
<div className="text-xs text-muted-foreground">
|
||||
Default attachments only apply to requests no non-default attachment matches, so fewer may be affected.
|
||||
</div>
|
||||
)}
|
||||
{impactResult.sample_keys.length > 0 && (
|
||||
<SampleList
|
||||
label="Keys"
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue