fix(rate-limiting): dedupe a deployment's own repeated entry before counting declarations

A deployment declaring the identical concurrency_limits entry twice
appended its own id twice, inflating len(declaring_ids) past
total_deployments. That made is_chain_wide false for an entry every
deployment actually agreed on, and a non-chain-wide concurrency entry
is silently dropped entirely rather than degraded -- disabling
enforcement instead of just scoping it.
This commit is contained in:
Deepanshu 2026-08-17 14:00:59 -04:00
parent 93590434f4
commit 3b0f8287f4
2 changed files with 36 additions and 1 deletions

View file

@ -280,7 +280,14 @@ def _build_group_limits(deployments: Sequence[Mapping[str, object]], unit: _Limi
for entry in _entries_for_unit(deployment, unit):
signature = (entry.tag_id, entry.name, entry.limit, entry.period_seconds, entry.scope_by_key_hash)
ids_for_signature = declaring_ids_by_signature.setdefault(signature, []) # mutable-ok: see comment above
ids_for_signature.append(dep_id)
# One deployment declaring the identical entry twice (a config
# duplicate) must count once, or len(declaring_ids) inflates past
# total_deployments below, making is_chain_wide false for an
# entry every deployment actually agrees on -- for concurrency
# that silently drops the entry entirely (see the docstring
# above), disabling enforcement rather than degrading it.
if dep_id not in ids_for_signature:
ids_for_signature.append(dep_id) # mutable-ok: see comment above
representative_entry_by_signature.setdefault(signature, entry) # mutable-ok: see comment above
distinct_signature_count_by_name: Final[Mapping[tuple[str, str], int]] = MappingProxyType(

View file

@ -1510,6 +1510,34 @@ def test_build_limits_index_preserves_key_ttl_seconds_and_max_in_memory_cache_si
assert configured[0].entry.max_in_memory_cache_size == 500
def test_build_limits_index_treats_a_duplicated_entry_on_one_deployment_as_chain_wide():
"""
Regression test: a single deployment declaring the identical
concurrency_limits entry twice (a config duplicate) used to append that
deployment's id twice, inflating len(declaring_ids) past
total_deployments. That made is_chain_wide false even though every
deployment (there's only one) actually agreed on the entry, and for
concurrency a non-chain-wide entry is silently dropped entirely --
disabling enforcement rather than degrading it.
"""
deployment = _deployment(
"grp",
"dep-1",
{
"concurrency_limits": {
"limits": [
{"name": "inflight", "tag_id": "end_user_id", "limit": 5, "period_seconds": 300},
{"name": "inflight", "tag_id": "end_user_id", "limit": 5, "period_seconds": 300},
]
}
},
)
index = _build_limits_index([deployment])
configured = index.resolve("grp", team_id=None)
assert len(configured) == 1
assert configured[0].deployment_scope is None # chain-wide, not dropped
def test_build_limits_index_keeps_different_teams_same_alias_separate():
"""
`team_public_model_name` is only unique per team: Router itself lets two