mirror of
https://github.com/tiennm99/litellm.git
synced 2026-08-24 10:27:13 +00:00
fix(guardrails): address PR #25575 review feedback + sync kill switch with opt-out list
Renames the new per-guardrail opt-out field from `disabled_global_guardrails` to `opted_out_global_guardrails` to eliminate the one-character collision with the legacy `disable_global_guardrails` boolean kill switch. Adds a type guard on the new gate so a misnamed bool can't crash the guardrail check. Filters duplicates out of the team-edit guardrail display for legacy teams that have a global name persisted in `metadata.guardrails` from before this PR. Drops the unused `isGuardrailsLoading` and `guardrailsError` destructures left in AddModelForm after the hook refactor. Adds Python tests for the new gate behavior (root, litellm_metadata, metadata, non-matching name, empty list, malformed bool value, opt-in coexistence) and extends useGuardrails.test.ts to exercise the global / optional partition logic that the rebuilt hook performs in its `select` transform. Wires the legacy kill switch and the new opt-out list together in the team edit form so they can never fall out of sync: - Toggling the kill switch reactively updates the Guardrails Select via `onValuesChange` — switch on strips all globals from the selection, switch off re-adds them. Existing opt-in extras are preserved either way. - When the switch is on, global options in the Select are individually disabled (greyed out) so the user can still manage opt-in guardrails but cannot accidentally re-enable a global the kill switch is bypassing. - The save handler writes both fields together: `disable_global_guardrails` reflects the switch, and `opted_out_global_guardrails` is set to either every global (when the switch is on) or the user's explicit opt-outs. - `effectiveGuardrails` for the form's initialValues honors the kill switch on legacy teams so the form opens in a state that matches what the runtime gate is actually doing — fixes the visual lie where chips appeared active while the switch was bypassing them. The backend gate already reads the list as the primary path with the bool as a fallback, so untouched legacy teams keep working until they get edited, at which point they migrate naturally.
This commit is contained in:
@@ -266,14 +266,16 @@ class CustomGuardrail(CustomLogger):
|
||||
return metadata["disable_global_guardrails"]
|
||||
return False
|
||||
|
||||
def get_disabled_global_guardrails_from_metadata(self, data: dict) -> List[str]:
|
||||
def get_opted_out_global_guardrails_from_metadata(self, data: dict) -> List[str]:
|
||||
"""
|
||||
Returns the list of global guardrail names the team/key has opted out of.
|
||||
"""
|
||||
if "disabled_global_guardrails" in data:
|
||||
return data["disabled_global_guardrails"] or []
|
||||
if "opted_out_global_guardrails" in data:
|
||||
value = data["opted_out_global_guardrails"]
|
||||
return value if isinstance(value, list) else []
|
||||
metadata = data.get("litellm_metadata") or data.get("metadata", {})
|
||||
return metadata.get("disabled_global_guardrails") or []
|
||||
value = metadata.get("opted_out_global_guardrails")
|
||||
return value if isinstance(value, list) else []
|
||||
|
||||
def _is_valid_response_type(self, result: Any) -> bool:
|
||||
"""
|
||||
@@ -415,7 +417,7 @@ class CustomGuardrail(CustomLogger):
|
||||
"""
|
||||
requested_guardrails = self.get_guardrail_from_metadata(data)
|
||||
disable_global_guardrail = self.get_disable_global_guardrail(data)
|
||||
disabled_global_guardrails = self.get_disabled_global_guardrails_from_metadata(data)
|
||||
opted_out_global_guardrails = self.get_opted_out_global_guardrails_from_metadata(data)
|
||||
verbose_logger.debug(
|
||||
"inside should_run_guardrail for guardrail=%s event_type= %s guardrail_supported_event_hooks= %s requested_guardrails= %s self.default_on= %s",
|
||||
self.guardrail_name,
|
||||
@@ -424,7 +426,7 @@ class CustomGuardrail(CustomLogger):
|
||||
requested_guardrails,
|
||||
self.default_on,
|
||||
)
|
||||
if self.default_on is True and self.guardrail_name in disabled_global_guardrails:
|
||||
if self.default_on is True and self.guardrail_name in opted_out_global_guardrails:
|
||||
return False
|
||||
|
||||
if self.default_on is True and disable_global_guardrail is not True:
|
||||
|
||||
@@ -1103,11 +1103,11 @@ async def add_litellm_data_to_request( # noqa: PLR0915
|
||||
data[_metadata_variable_name]["disable_global_guardrails"] = team_metadata[
|
||||
"disable_global_guardrails"
|
||||
]
|
||||
if "disabled_global_guardrails" in team_metadata and isinstance(
|
||||
team_metadata["disabled_global_guardrails"], list
|
||||
if "opted_out_global_guardrails" in team_metadata and isinstance(
|
||||
team_metadata["opted_out_global_guardrails"], list
|
||||
):
|
||||
data[_metadata_variable_name]["disabled_global_guardrails"] = team_metadata[
|
||||
"disabled_global_guardrails"
|
||||
data[_metadata_variable_name]["opted_out_global_guardrails"] = team_metadata[
|
||||
"opted_out_global_guardrails"
|
||||
]
|
||||
if "spend_logs_metadata" in team_metadata and isinstance(
|
||||
team_metadata["spend_logs_metadata"], dict
|
||||
|
||||
@@ -245,6 +245,121 @@ class TestCustomGuardrailShouldRunGuardrail:
|
||||
result is True
|
||||
), "Global guardrail should still run when disable_global_guardrail=False"
|
||||
|
||||
def test_should_run_guardrail_with_opted_out_global_guardrails(self):
|
||||
"""Test the per-guardrail opt-out list for global (default_on=True) guardrails"""
|
||||
from litellm.types.guardrails import GuardrailEventHooks
|
||||
|
||||
custom_guardrail = CustomGuardrail(
|
||||
guardrail_name="global_guardrail",
|
||||
default_on=True,
|
||||
event_hook=GuardrailEventHooks.pre_call,
|
||||
)
|
||||
|
||||
# Test 1: guardrail in the opt-out list at root level → skipped
|
||||
data_root = {
|
||||
"model": "gpt-3.5-turbo",
|
||||
"messages": [{"role": "user", "content": "test"}],
|
||||
"opted_out_global_guardrails": ["global_guardrail"],
|
||||
}
|
||||
assert (
|
||||
custom_guardrail.should_run_guardrail(
|
||||
data=data_root, event_type=GuardrailEventHooks.pre_call
|
||||
)
|
||||
is False
|
||||
)
|
||||
|
||||
# Test 2: guardrail in the opt-out list inside litellm_metadata → skipped
|
||||
data_litellm = {
|
||||
"model": "gpt-3.5-turbo",
|
||||
"messages": [{"role": "user", "content": "test"}],
|
||||
"litellm_metadata": {"opted_out_global_guardrails": ["global_guardrail"]},
|
||||
}
|
||||
assert (
|
||||
custom_guardrail.should_run_guardrail(
|
||||
data=data_litellm, event_type=GuardrailEventHooks.pre_call
|
||||
)
|
||||
is False
|
||||
)
|
||||
|
||||
# Test 3: guardrail in the opt-out list inside metadata → skipped
|
||||
data_metadata = {
|
||||
"model": "gpt-3.5-turbo",
|
||||
"messages": [{"role": "user", "content": "test"}],
|
||||
"metadata": {"opted_out_global_guardrails": ["global_guardrail"]},
|
||||
}
|
||||
assert (
|
||||
custom_guardrail.should_run_guardrail(
|
||||
data=data_metadata, event_type=GuardrailEventHooks.pre_call
|
||||
)
|
||||
is False
|
||||
)
|
||||
|
||||
# Test 4: a different guardrail in the opt-out list → still runs
|
||||
data_other = {
|
||||
"model": "gpt-3.5-turbo",
|
||||
"messages": [{"role": "user", "content": "test"}],
|
||||
"metadata": {"opted_out_global_guardrails": ["some_other_guardrail"]},
|
||||
}
|
||||
assert (
|
||||
custom_guardrail.should_run_guardrail(
|
||||
data=data_other, event_type=GuardrailEventHooks.pre_call
|
||||
)
|
||||
is True
|
||||
)
|
||||
|
||||
# Test 5: empty opt-out list → still runs
|
||||
data_empty = {
|
||||
"model": "gpt-3.5-turbo",
|
||||
"messages": [{"role": "user", "content": "test"}],
|
||||
"metadata": {"opted_out_global_guardrails": []},
|
||||
}
|
||||
assert (
|
||||
custom_guardrail.should_run_guardrail(
|
||||
data=data_empty, event_type=GuardrailEventHooks.pre_call
|
||||
)
|
||||
is True
|
||||
)
|
||||
|
||||
# Test 6: malformed value (bool instead of list) → safely ignored, guardrail runs
|
||||
data_malformed = {
|
||||
"model": "gpt-3.5-turbo",
|
||||
"messages": [{"role": "user", "content": "test"}],
|
||||
"metadata": {"opted_out_global_guardrails": True},
|
||||
}
|
||||
assert (
|
||||
custom_guardrail.should_run_guardrail(
|
||||
data=data_malformed, event_type=GuardrailEventHooks.pre_call
|
||||
)
|
||||
is True
|
||||
)
|
||||
|
||||
def test_should_run_guardrail_opt_out_does_not_affect_non_global(self):
|
||||
"""Opt-out list only matters for default_on=True guardrails"""
|
||||
from litellm.types.guardrails import GuardrailEventHooks
|
||||
|
||||
non_global = CustomGuardrail(
|
||||
guardrail_name="opt_in_guardrail",
|
||||
default_on=False,
|
||||
event_hook=GuardrailEventHooks.pre_call,
|
||||
)
|
||||
|
||||
# An opt-in guardrail named in opted_out_global_guardrails is still controlled
|
||||
# by the explicit `guardrails` request list, not by the global opt-out list.
|
||||
data = {
|
||||
"model": "gpt-3.5-turbo",
|
||||
"messages": [{"role": "user", "content": "test"}],
|
||||
"metadata": {
|
||||
"opted_out_global_guardrails": ["opt_in_guardrail"],
|
||||
"guardrails": ["opt_in_guardrail"],
|
||||
},
|
||||
}
|
||||
assert (
|
||||
non_global.should_run_guardrail(
|
||||
data=data, event_type=GuardrailEventHooks.pre_call
|
||||
)
|
||||
is True
|
||||
)
|
||||
|
||||
|
||||
class TestApplyGuardrailCheck:
|
||||
def test_apply_guardrail_check_only_on_direct_implementation(self):
|
||||
|
||||
@@ -271,4 +271,29 @@ describe("useGuardrails", () => {
|
||||
expect(names).toContain("custom-guardrail-1");
|
||||
expect(names).toContain("custom-guardrail-2");
|
||||
});
|
||||
|
||||
it("should partition guardrails into global and optional sets based on default_on", async () => {
|
||||
const mockMixedResponse = {
|
||||
guardrails: [
|
||||
{ guardrail_name: "global-guard-a", litellm_params: { default_on: true } },
|
||||
{ guardrail_name: "global-guard-b", litellm_params: { default_on: true } },
|
||||
{ guardrail_name: "optional-guard-a", litellm_params: { default_on: false } },
|
||||
{ guardrail_name: "optional-guard-b" },
|
||||
],
|
||||
};
|
||||
(getGuardrailsList as any).mockResolvedValue(mockMixedResponse);
|
||||
|
||||
const { result } = renderHook(() => useGuardrails(), { wrapper });
|
||||
|
||||
await waitFor(() => {
|
||||
expect(result.current.isSuccess).toBe(true);
|
||||
});
|
||||
|
||||
expect(result.current.data?.globalGuardrailNames).toEqual(
|
||||
new Set(["global-guard-a", "global-guard-b"]),
|
||||
);
|
||||
expect(result.current.data?.optionalGuardrailNames).toEqual(
|
||||
new Set(["optional-guard-a", "optional-guard-b"]),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -63,7 +63,7 @@ const AddModelForm: React.FC<AddModelFormProps> = ({
|
||||
isLoading: isProviderMetadataLoading,
|
||||
error: providerMetadataError,
|
||||
} = useProviderFields();
|
||||
const { data: guardrailsData, isLoading: isGuardrailsLoading, error: guardrailsError } = useGuardrails();
|
||||
const { data: guardrailsData } = useGuardrails();
|
||||
const guardrailsList = guardrailsData?.guardrails.map((g) => g.guardrail_name);
|
||||
const { data: tagsList, isLoading: isTagsLoading, error: tagsError } = useTags();
|
||||
|
||||
|
||||
@@ -203,6 +203,7 @@ const TeamInfoView: React.FC<TeamInfoProps> = ({
|
||||
// Models currently selected in the team edit form, used to scope the per-model
|
||||
// rate limit dropdown to models this team actually has access to.
|
||||
const selectedModelsInForm = Form.useWatch("models", form) as string[] | undefined;
|
||||
const killSwitchOn = Form.useWatch("disable_global_guardrails", form) as boolean | undefined;
|
||||
const availableRateLimitModels = useMemo(() => {
|
||||
const selected = selectedModelsInForm ?? teamData?.team_info?.models ?? [];
|
||||
if (selected.includes("all-proxy-models") || selected.includes("all-team-models")) {
|
||||
@@ -480,6 +481,13 @@ const TeamInfoView: React.FC<TeamInfoProps> = ({
|
||||
}
|
||||
}
|
||||
|
||||
const killSwitchOnAtSave = values.disable_global_guardrails === true;
|
||||
const optedOutGlobalGuardrails = killSwitchOnAtSave
|
||||
? Array.from(globalGuardrailNames)
|
||||
: Array.from(globalGuardrailNames).filter(
|
||||
(n) => !(values.guardrails || []).includes(n),
|
||||
);
|
||||
|
||||
const updateData: any = {
|
||||
team_id: teamId,
|
||||
team_alias: values.team_alias,
|
||||
@@ -494,11 +502,9 @@ const TeamInfoView: React.FC<TeamInfoProps> = ({
|
||||
metadata: {
|
||||
...parsedMetadata,
|
||||
guardrails: (values.guardrails || []).filter((n: string) => !globalGuardrailNames.has(n)),
|
||||
disabled_global_guardrails: Array.from(globalGuardrailNames).filter(
|
||||
(n) => !(values.guardrails || []).includes(n),
|
||||
),
|
||||
opted_out_global_guardrails: optedOutGlobalGuardrails,
|
||||
...(values.logging_settings?.length > 0 ? { logging: values.logging_settings } : {}),
|
||||
disable_global_guardrails: values.disable_global_guardrails || false,
|
||||
disable_global_guardrails: killSwitchOnAtSave,
|
||||
soft_budget_alerting_emails:
|
||||
typeof values.soft_budget_alerting_emails === "string"
|
||||
? values.soft_budget_alerting_emails
|
||||
@@ -602,11 +608,17 @@ const TeamInfoView: React.FC<TeamInfoProps> = ({
|
||||
|
||||
const { team_info: info } = teamData;
|
||||
|
||||
const optedOutGlobals = new Set<string>(info.metadata?.disabled_global_guardrails || []);
|
||||
const effectiveGuardrails = [
|
||||
...Array.from(globalGuardrailNames).filter((n) => !optedOutGlobals.has(n)),
|
||||
...(info.metadata?.guardrails || []),
|
||||
];
|
||||
const initialKillSwitchOn = info.metadata?.disable_global_guardrails === true;
|
||||
const optedOutGlobals = new Set<string>(info.metadata?.opted_out_global_guardrails || []);
|
||||
const nonGlobalOptIns = (info.metadata?.guardrails || []).filter(
|
||||
(n: string) => !globalGuardrailNames.has(n),
|
||||
);
|
||||
const effectiveGuardrails = initialKillSwitchOn
|
||||
? nonGlobalOptIns
|
||||
: [
|
||||
...Array.from(globalGuardrailNames).filter((n) => !optedOutGlobals.has(n)),
|
||||
...nonGlobalOptIns,
|
||||
];
|
||||
|
||||
const preventTagMouseDown = (e: React.MouseEvent) => {
|
||||
e.preventDefault();
|
||||
@@ -861,6 +873,17 @@ const TeamInfoView: React.FC<TeamInfoProps> = ({
|
||||
<Form
|
||||
form={form}
|
||||
onFinish={handleTeamUpdate}
|
||||
onValuesChange={(changedValues) => {
|
||||
if ("disable_global_guardrails" in changedValues) {
|
||||
const checked = changedValues.disable_global_guardrails === true;
|
||||
const current = (form.getFieldValue("guardrails") || []) as string[];
|
||||
const nonGlobals = current.filter((n) => !globalGuardrailNames.has(n));
|
||||
form.setFieldValue(
|
||||
"guardrails",
|
||||
checked ? nonGlobals : [...Array.from(globalGuardrailNames), ...nonGlobals],
|
||||
);
|
||||
}
|
||||
}}
|
||||
initialValues={{
|
||||
...info,
|
||||
team_alias: info.team_alias,
|
||||
@@ -1136,6 +1159,7 @@ const TeamInfoView: React.FC<TeamInfoProps> = ({
|
||||
key={g.guardrail_name}
|
||||
value={g.guardrail_name}
|
||||
label={g.guardrail_name}
|
||||
disabled={killSwitchOn}
|
||||
>
|
||||
{g.guardrail_name}
|
||||
</Select.Option>
|
||||
|
||||
Reference in New Issue
Block a user