fix: reject empty tag_id on TagRateLimitScope and TagRateLimitEntry

Greptile flagged this as P1 on the stacked #39902: both tag
identifiers accepted an empty string, and downstream identity lookup
builds a key off tag_id, so an empty value would silently search for
a bare colon prefix and never match instead of erroring at config
load time. Rejects it at construction, matching the existing
non-empty guard on TagRateLimitScope.values. Verified the new tests
fail without the fix and pass with it.
This commit is contained in:
Deepanshu 2026-09-08 16:49:09 -04:00
parent cf4b80b24a
commit 1dfe1d968c
2 changed files with 26 additions and 0 deletions

View file

@ -169,6 +169,12 @@ class TagRateLimitScope(BaseModel):
model_config = ConfigDict(frozen=True)
@model_validator(mode="after")
def _validate_tag_id(self) -> "TagRateLimitScope":
if not self.tag_id:
raise ValueError("tag_id must be a non-empty string")
return self
@model_validator(mode="after")
def _validate_values(self) -> "TagRateLimitScope":
if not self.values:
@ -200,6 +206,12 @@ class TagRateLimitEntry(BaseModel):
model_config = ConfigDict(protected_namespaces=())
@model_validator(mode="after")
def _validate_tag_id(self) -> "TagRateLimitEntry":
if not self.tag_id:
raise ValueError("tag_id must be a non-empty string")
return self
@model_validator(mode="after")
def _validate_limit(self) -> "TagRateLimitEntry":
# NaN compares False against every ordering operator, silently defeating whichever check gates this limit

View file

@ -238,6 +238,13 @@ def test_apply_to_models_sorted_and_deduped():
assert entry.apply_to_models == ("claude", "gpt-4o")
def test_scope_tag_id_empty_string_rejected():
"""An empty tag_id makes identity lookup search for a bare `:` prefix, silently
never matching instead of erroring at config load time (Greptile P1 on #39902)."""
with pytest.raises(ValueError, match="tag_id must be a non-empty string"):
TagRateLimitScope(tag_id="", values=("1032",))
def test_scope_values_empty_list_rejected():
with pytest.raises(ValueError, match="values must be a non-empty list"):
TagRateLimitScope(tag_id="company_id", values=())
@ -254,6 +261,13 @@ def test_scope_is_frozen():
scope.tag_id = "other_tag"
def test_entry_tag_id_empty_string_rejected():
"""Same silent-no-match failure mode as TagRateLimitScope.tag_id (Greptile P1 on
#39902); the default is non-empty, but an explicit override could still be empty."""
with pytest.raises(ValueError, match="tag_id must be a non-empty string"):
TagRateLimitEntry(name="daily", tag_id="", limit=10, period_seconds=60)
def test_entry_accepts_enabled_for_and_disabled_for_scopes():
entry = TagRateLimitEntry(
name="daily",