Skip to content
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
115 changes: 71 additions & 44 deletions litellm/proxy/guardrails/guardrail_hooks/tool_permission.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,50 +63,7 @@ def __init__(
self.rules: List[ToolPermissionRule] = []
self._compiled_rule_patterns: Dict[str, Dict[str, re.Pattern]] = {}
self._compiled_rule_targets: Dict[str, Dict[str, Optional[re.Pattern]]] = {}
if rules:
for rule_item in rules:
if isinstance(rule_item, ToolPermissionRule):
rule = rule_item
else:
rule = ToolPermissionRule(**rule_item)
self.rules.append(rule)

compiled_target_patterns: Dict[str, Optional[re.Pattern]] = {
"tool_name": None,
"tool_type": None,
}
if rule.tool_name is not None:
try:
compiled_target_patterns["tool_name"] = re.compile(
rule.tool_name
)
except re.error as exc:
raise ValueError(
f"Invalid regex for tool_name in rule '{rule.id}': {exc}"
) from exc
if rule.tool_type is not None:
try:
compiled_target_patterns["tool_type"] = re.compile(
rule.tool_type
)
except re.error as exc:
raise ValueError(
f"Invalid regex for tool_type in rule '{rule.id}': {exc}"
) from exc
self._compiled_rule_targets[rule.id] = compiled_target_patterns

if rule.allowed_param_patterns:
compiled_patterns: Dict[str, re.Pattern] = {}
for path, pattern in rule.allowed_param_patterns.items():
try:
compiled_patterns[path] = re.compile(pattern)
except re.error as exc:
raise ValueError(
f"Invalid regex in allowed_param_patterns for rule '{rule.id}': {exc}"
) from exc

if compiled_patterns:
self._compiled_rule_patterns[rule.id] = compiled_patterns
self._recompile_rules(rules)
Comment on lines 63 to +66

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 The three explicit assignments on lines 63-65 are immediately overwritten by the _recompile_rules call that follows — that helper already resets all three attributes at its top. These lines are redundant.

Suggested change
self.rules: List[ToolPermissionRule] = []
self._compiled_rule_patterns: Dict[str, Dict[str, re.Pattern]] = {}
self._compiled_rule_targets: Dict[str, Dict[str, Optional[re.Pattern]]] = {}
if rules:
for rule_item in rules:
if isinstance(rule_item, ToolPermissionRule):
rule = rule_item
else:
rule = ToolPermissionRule(**rule_item)
self.rules.append(rule)
compiled_target_patterns: Dict[str, Optional[re.Pattern]] = {
"tool_name": None,
"tool_type": None,
}
if rule.tool_name is not None:
try:
compiled_target_patterns["tool_name"] = re.compile(
rule.tool_name
)
except re.error as exc:
raise ValueError(
f"Invalid regex for tool_name in rule '{rule.id}': {exc}"
) from exc
if rule.tool_type is not None:
try:
compiled_target_patterns["tool_type"] = re.compile(
rule.tool_type
)
except re.error as exc:
raise ValueError(
f"Invalid regex for tool_type in rule '{rule.id}': {exc}"
) from exc
self._compiled_rule_targets[rule.id] = compiled_target_patterns
if rule.allowed_param_patterns:
compiled_patterns: Dict[str, re.Pattern] = {}
for path, pattern in rule.allowed_param_patterns.items():
try:
compiled_patterns[path] = re.compile(pattern)
except re.error as exc:
raise ValueError(
f"Invalid regex in allowed_param_patterns for rule '{rule.id}': {exc}"
) from exc
if compiled_patterns:
self._compiled_rule_patterns[rule.id] = compiled_patterns
self._recompile_rules(rules)
self._recompile_rules(rules)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!


# Normalize to lowercase for case-insensitive handling
self.default_action = (
Expand Down Expand Up @@ -134,6 +91,76 @@ def get_config_model():

return ToolPermissionGuardrailConfigModel

def _recompile_rules(self, rules: Optional[List]) -> None:
"""Parse a raw rules list and rebuild compiled rule state."""
self.rules = []
self._compiled_rule_patterns = {}
self._compiled_rule_targets = {}
if not rules:
return
for rule_item in rules:
if isinstance(rule_item, ToolPermissionRule):
rule = rule_item
else:
rule = ToolPermissionRule(**rule_item)
self.rules.append(rule)

compiled_target_patterns: Dict[str, Optional[re.Pattern]] = {
"tool_name": None,
"tool_type": None,
}
if rule.tool_name is not None:
try:
compiled_target_patterns["tool_name"] = re.compile(rule.tool_name)
except re.error as exc:
raise ValueError(
f"Invalid regex for tool_name in rule '{rule.id}': {exc}"
) from exc
if rule.tool_type is not None:
try:
compiled_target_patterns["tool_type"] = re.compile(rule.tool_type)
except re.error as exc:
raise ValueError(
f"Invalid regex for tool_type in rule '{rule.id}': {exc}"
) from exc
self._compiled_rule_targets[rule.id] = compiled_target_patterns

if rule.allowed_param_patterns:
compiled_patterns: Dict[str, re.Pattern] = {}
for path, pattern in rule.allowed_param_patterns.items():
try:
compiled_patterns[path] = re.compile(pattern)
except re.error as exc:
raise ValueError(
f"Invalid regex in allowed_param_patterns for rule '{rule.id}': {exc}"
) from exc
if compiled_patterns:
self._compiled_rule_patterns[rule.id] = compiled_patterns

def update_in_memory_litellm_params(self, litellm_params: "LitellmParams") -> None:
"""Update guardrail params in memory and recompile rule state.

Without this override the base-class setattr loop updates self.rules but
leaves self._compiled_rule_targets and self._compiled_rule_patterns
pointing at the old state, so rule changes made through PUT /guardrails
are silently ignored until the process reinitializes the guardrail.
"""
super().update_in_memory_litellm_params(litellm_params)
if litellm_params.rules is not None:
self._recompile_rules(litellm_params.rules)
Comment on lines +148 to +150

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 self.rules left as None when litellm_params.rules is not provided

The base-class super().update_in_memory_litellm_params(litellm_params) iterates vars(litellm_params) and calls setattr(self, key, value) for every field — including rules, which defaults to None in ToolPermissionGuardrailConfigModel. When a PUT /guardrails call does not include rules, litellm_params.rules is None, so super() sets self.rules = None and then the guard if litellm_params.rules is not None: skips _recompile_rules. The next incoming request hits for rule in self.rules: in _check_tool_permission or _get_permission_for_tool_call and raises TypeError: 'NoneType' object is not iterable, crashing enforcement entirely. A minimal safeguard like if not isinstance(self.rules, list): self.rules = [] after the super call (or calling _recompile_rules unconditionally with litellm_params.rules or []) would prevent this.

if litellm_params.default_action is not None:
self.default_action = (
litellm_params.default_action.lower()
if isinstance(litellm_params.default_action, str)
else litellm_params.default_action
)
if litellm_params.on_disallowed_action is not None:
self.on_disallowed_action = (
litellm_params.on_disallowed_action.lower()
if isinstance(litellm_params.on_disallowed_action, str)
else litellm_params.on_disallowed_action
)

def _matches_regex(
self, pattern: Optional[re.Pattern], value: Optional[str]
) -> bool:
Expand Down
Loading