From cca4da6e0070e9c9ccdaec484650c6bd07d2bee5 Mon Sep 17 00:00:00 2001 From: Simon Pinfold Date: Wed, 5 Aug 2026 13:36:37 -0700 Subject: [PATCH] Cap tag-filter lists at 100 entries, all spellings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding: unbounded tag lists fan out into one correlated EXISTS per tag on both page and count statements. Cap each list at 100 normalized entries with 400 INVALID_TAG_FILTER naming the parameter. Applies to the legacy spellings as well — a deliberate, decided exception to the old-names-behave-identically rule, since a cap only on new names would leave the same fan-out reachable through the aliases. --- app/assets/api/routes.py | 25 ++++++++++++++++++++++ tests-unit/assets_test/test_list_filter.py | 22 +++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/app/assets/api/routes.py b/app/assets/api/routes.py index 63921cd1e..2acd1d6c0 100644 --- a/app/assets/api/routes.py +++ b/app/assets/api/routes.py @@ -125,6 +125,13 @@ class InvalidTagFilterError(Exception): self.details = details +# Applies to every tag-filter list, legacy spellings included — unlike the +# combination validation below, which only runs when a new-name parameter is +# present. Deliberate exception to legacy byte-identity (decision 2026-08-05): +# an unbounded list fans out into one EXISTS per tag on page and count queries. +MAX_TAG_FILTER_TAGS = 100 + + def _resolve_tag_filters( q: schemas_in.ListAssetsQuery | schemas_in.TagsRefineQuery, ) -> tuple[list[str], list[str], list[str]]: @@ -146,6 +153,24 @@ def _resolve_tag_filters( tags_any = normalize_tags(q.tags_any) tags_none = normalize_tags(q.tags_none) + for param_name, values in ( + ("include_tags", include_tags), + ("exclude_tags", exclude_tags), + ("tags_all", tags_all), + ("tags_any", tags_any), + ("tags_none", tags_none), + ): + if len(values) > MAX_TAG_FILTER_TAGS: + raise InvalidTagFilterError( + f"'{param_name}' lists {len(values)} tags; the maximum is " + f"{MAX_TAG_FILTER_TAGS}.", + { + "parameter": param_name, + "count": len(values), + "max": MAX_TAG_FILTER_TAGS, + }, + ) + if not (tags_all or tags_any or tags_none): return include_tags, [], exclude_tags diff --git a/tests-unit/assets_test/test_list_filter.py b/tests-unit/assets_test/test_list_filter.py index c5cb50b9c..c1245491e 100644 --- a/tests-unit/assets_test/test_list_filter.py +++ b/tests-unit/assets_test/test_list_filter.py @@ -719,6 +719,28 @@ def test_list_assets_tag_values_case_sensitive(http, api_base, asset_factory, ma assert names_for({"tags_all": f"unit-tests,{scope},{upper}", "tags_none": lower}) == {a["name"]} +def test_tag_list_cap_applies_to_all_spellings(http, api_base): + """The 100-tag cap covers legacy spellings too — the one deliberate + exception to legacy byte-identity (decision 2026-08-05).""" + big = ",".join(f"cap-{i}" for i in range(101)) + for param in ("tags_any", "include_tags"): + r = http.get(api_base + "/api/assets", params={param: big}, timeout=120) + body = r.json() + assert r.status_code == 400, body + assert body["error"]["code"] == "INVALID_TAG_FILTER" + assert body["error"]["details"]["parameter"] == param + assert body["error"]["details"]["max"] == 100 + + exact = ",".join(f"cap-{i}" for i in range(100)) + r = http.get(api_base + "/api/assets", params={"tags_any": exact}, timeout=120) + assert r.status_code == 200, r.json() + + # The cap counts normalized (deduped) tags, not raw CSV items. + dups = ",".join("cap-dup" for _ in range(150)) + r = http.get(api_base + "/api/assets", params={"tags_any": dups}, timeout=120) + assert r.status_code == 200, r.json() + + def test_resolve_tag_filters_no_deprecation_warning(): """The deprecated-field warning is for API clients; the server's own remap shim must not fire it on every request."""