From 5f172d26504900c7879a17eab8e639a2acd2e9b3 Mon Sep 17 00:00:00 2001 From: Don Kendall Date: Thu, 13 Aug 2026 19:14:16 -0400 Subject: [PATCH 1/5] fix(api): match search terms as words, not as one contiguous string MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every search endpoint built its predicate by OR-ing `__icontains` over the whole query, so a multi-word query only matched when those words appeared adjacently in one field. Searching "payment gateway review" returned nothing for a work item titled "Select the payment gateway on Level 3 capability and effective rate" — the words are all present, just not contiguous. Users had to guess a single word from the title. Tokenize instead: every whitespace-separated token must match at least one searchable field, OR-ing across fields and AND-ing across tokens. For a single-word query this is identical to the old behaviour; for a multi-word query it is a strict superset, so nothing that matched before stops matching. Word order and interleaving no longer matter. Sequence ids are OR-ed onto the whole predicate rather than folded into the per-token AND, so a query mixing words and a number ("fix 22") still surfaces the issue by number the way it always did. The predicate was also copy-pasted twenty times across GlobalSearchEndpoint, SearchEndpoint and search_issues, with the issue field list duplicated four times. Any fix applied to one path silently left the others behind. Extract `build_search_query` plus per-entity field constants so the endpoints cannot drift apart, and widening a search is a one-line change. While extracting, fix the sequence-id regex to honour its own comment. It read "Match whole integers only (exclude decimal numbers)" but `\b\d+\b` treats the dot in "3.5" as a word boundary and yields both 3 and 5, so searching a version string surfaced unrelated issues by sequence id. --- apps/api/plane/app/views/search/base.py | 193 +++++------------- .../api/plane/tests/unit/utils/test_search.py | 120 +++++++++++ apps/api/plane/utils/issue_search.py | 32 +-- apps/api/plane/utils/search.py | 81 ++++++++ 4 files changed, 265 insertions(+), 161 deletions(-) create mode 100644 apps/api/plane/tests/unit/utils/test_search.py create mode 100644 apps/api/plane/utils/search.py diff --git a/apps/api/plane/app/views/search/base.py b/apps/api/plane/app/views/search/base.py index 289155b87c6..9aefb765a0d 100644 --- a/apps/api/plane/app/views/search/base.py +++ b/apps/api/plane/app/views/search/base.py @@ -2,9 +2,6 @@ # SPDX-License-Identifier: AGPL-3.0-only # See the LICENSE file for details. -# Python imports -import re - # Django imports from django.db import models from django.db.models import ( @@ -29,6 +26,18 @@ # Module imports from plane.app.views.base import BaseAPIView from plane.app.permissions import WorkspaceUserPermission +from plane.utils.search import ( + CYCLE_SEARCH_FIELDS, + ISSUE_SEARCH_FIELDS, + ISSUE_SEQUENCE_FIELDS, + MODULE_SEARCH_FIELDS, + PAGE_SEARCH_FIELDS, + PROJECT_SEARCH_FIELDS, + USER_MENTION_SEARCH_FIELDS, + VIEW_SEARCH_FIELDS, + WORKSPACE_SEARCH_FIELDS, + build_search_query, +) from plane.db.models import ( Workspace, Project, @@ -49,11 +58,7 @@ class GlobalSearchEndpoint(BaseAPIView): """ def filter_workspaces(self, query, _slug, _project_id, _workspace_search): - fields = ["name"] - q = Q() - if query: - for field in fields: - q |= Q(**{f"{field}__icontains": query}) + q = build_search_query(query, fields=WORKSPACE_SEARCH_FIELDS) return ( Workspace.objects.filter(q, workspace_member__member=self.request.user) .order_by("-created_at") @@ -62,11 +67,7 @@ def filter_workspaces(self, query, _slug, _project_id, _workspace_search): ) def filter_projects(self, query, slug, _project_id, _workspace_search): - fields = ["name", "identifier"] - q = Q() - if query: - for field in fields: - q |= Q(**{f"{field}__icontains": query}) + q = build_search_query(query, fields=PROJECT_SEARCH_FIELDS) return ( Project.objects.filter( q, @@ -81,17 +82,11 @@ def filter_projects(self, query, slug, _project_id, _workspace_search): ) def filter_issues(self, query, slug, project_id, workspace_search): - fields = ["name", "sequence_id", "project__identifier"] - q = Q() - if query: - for field in fields: - if field == "sequence_id": - # Match whole integers only (exclude decimal numbers) - sequences = re.findall(r"\b\d+\b", query) - for sequence_id in sequences: - q |= Q(**{"sequence_id": sequence_id}) - else: - q |= Q(**{f"{field}__icontains": query}) + q = build_search_query( + query, + fields=ISSUE_SEARCH_FIELDS, + sequence_fields=ISSUE_SEQUENCE_FIELDS, + ) issues = Issue.issue_objects.filter( q, @@ -114,11 +109,7 @@ def filter_issues(self, query, slug, project_id, workspace_search): )[:100] def filter_cycles(self, query, slug, project_id, workspace_search): - fields = ["name"] - q = Q() - if query: - for field in fields: - q |= Q(**{f"{field}__icontains": query}) + q = build_search_query(query, fields=CYCLE_SEARCH_FIELDS) cycles = Cycle.objects.filter( q, @@ -138,11 +129,7 @@ def filter_cycles(self, query, slug, project_id, workspace_search): ) def filter_modules(self, query, slug, project_id, workspace_search): - fields = ["name"] - q = Q() - if query: - for field in fields: - q |= Q(**{f"{field}__icontains": query}) + q = build_search_query(query, fields=MODULE_SEARCH_FIELDS) modules = Module.objects.filter( q, @@ -162,11 +149,7 @@ def filter_modules(self, query, slug, project_id, workspace_search): ) def filter_pages(self, query, slug, project_id, workspace_search): - fields = ["name"] - q = Q() - if query: - for field in fields: - q |= Q(**{f"{field}__icontains": query}) + q = build_search_query(query, fields=PAGE_SEARCH_FIELDS) pages = ( Page.objects.filter( @@ -208,11 +191,7 @@ def filter_pages(self, query, slug, project_id, workspace_search): ) def filter_views(self, query, slug, project_id, workspace_search): - fields = ["name"] - q = Q() - if query: - for field in fields: - q |= Q(**{f"{field}__icontains": query}) + q = build_search_query(query, fields=VIEW_SEARCH_FIELDS) issue_views = IssueView.objects.filter( q, @@ -232,17 +211,11 @@ def filter_views(self, query, slug, project_id, workspace_search): ) def filter_intakes(self, query, slug, project_id, workspace_search): - fields = ["name", "sequence_id", "project__identifier"] - q = Q() - if query: - for field in fields: - if field == "sequence_id": - # Match whole integers only (exclude decimal numbers) - sequences = re.findall(r"\b\d+\b", query) - for sequence_id in sequences: - q |= Q(**{"sequence_id": sequence_id}) - else: - q |= Q(**{f"{field}__icontains": query}) + q = build_search_query( + query, + fields=ISSUE_SEARCH_FIELDS, + sequence_fields=ISSUE_SEQUENCE_FIELDS, + ) issues = Issue.objects.filter( q, @@ -317,16 +290,7 @@ def get(self, request, slug): if project_id: for query_type in query_types: if query_type == "user_mention": - fields = [ - "member__first_name", - "member__last_name", - "member__display_name", - ] - q = Q() - - if query: - for field in fields: - q |= Q(**{f"{field}__icontains": query}) + q = build_search_query(query, fields=USER_MENTION_SEARCH_FIELDS) users = ( ProjectMember.objects.filter( @@ -366,12 +330,7 @@ def get(self, request, slug): response_data["user_mention"] = list(users[:count]) elif query_type == "project": - fields = ["name", "identifier"] - q = Q() - - if query: - for field in fields: - q |= Q(**{f"{field}__icontains": query}) + q = build_search_query(query, fields=PROJECT_SEARCH_FIELDS) projects = ( Project.objects.filter( q, @@ -385,17 +344,11 @@ def get(self, request, slug): response_data["project"] = list(projects) elif query_type == "issue": - fields = ["name", "sequence_id", "project__identifier"] - q = Q() - - if query: - for field in fields: - if field == "sequence_id": - sequences = re.findall(r"\b\d+\b", query) - for sequence_id in sequences: - q |= Q(**{"sequence_id": sequence_id}) - else: - q |= Q(**{f"{field}__icontains": query}) + q = build_search_query( + query, + fields=ISSUE_SEARCH_FIELDS, + sequence_fields=ISSUE_SEQUENCE_FIELDS, + ) issues = ( Issue.issue_objects.filter( @@ -421,12 +374,7 @@ def get(self, request, slug): response_data["issue"] = list(issues) elif query_type == "cycle": - fields = ["name"] - q = Q() - - if query: - for field in fields: - q |= Q(**{f"{field}__icontains": query}) + q = build_search_query(query, fields=CYCLE_SEARCH_FIELDS) cycles = ( Cycle.objects.filter( @@ -469,12 +417,7 @@ def get(self, request, slug): response_data["cycle"] = list(cycles) elif query_type == "module": - fields = ["name"] - q = Q() - - if query: - for field in fields: - q |= Q(**{f"{field}__icontains": query}) + q = build_search_query(query, fields=MODULE_SEARCH_FIELDS) modules = ( Module.objects.filter( @@ -498,12 +441,7 @@ def get(self, request, slug): response_data["module"] = list(modules) elif query_type == "page": - fields = ["name"] - q = Q() - - if query: - for field in fields: - q |= Q(**{f"{field}__icontains": query}) + q = build_search_query(query, fields=PAGE_SEARCH_FIELDS) pages = ( Page.objects.filter( @@ -530,16 +468,7 @@ def get(self, request, slug): else: for query_type in query_types: if query_type == "user_mention": - fields = [ - "member__first_name", - "member__last_name", - "member__display_name", - ] - q = Q() - - if query: - for field in fields: - q |= Q(**{f"{field}__icontains": query}) + q = build_search_query(query, fields=USER_MENTION_SEARCH_FIELDS) users = ( WorkspaceMember.objects.filter( q, @@ -571,12 +500,7 @@ def get(self, request, slug): response_data["user_mention"] = list(users) elif query_type == "project": - fields = ["name", "identifier"] - q = Q() - - if query: - for field in fields: - q |= Q(**{f"{field}__icontains": query}) + q = build_search_query(query, fields=PROJECT_SEARCH_FIELDS) projects = ( Project.objects.filter( q, @@ -590,17 +514,11 @@ def get(self, request, slug): response_data["project"] = list(projects) elif query_type == "issue": - fields = ["name", "sequence_id", "project__identifier"] - q = Q() - - if query: - for field in fields: - if field == "sequence_id": - sequences = re.findall(r"\b\d+\b", query) - for sequence_id in sequences: - q |= Q(**{"sequence_id": sequence_id}) - else: - q |= Q(**{f"{field}__icontains": query}) + q = build_search_query( + query, + fields=ISSUE_SEARCH_FIELDS, + sequence_fields=ISSUE_SEQUENCE_FIELDS, + ) issues = ( Issue.issue_objects.filter( @@ -625,12 +543,7 @@ def get(self, request, slug): response_data["issue"] = list(issues) elif query_type == "cycle": - fields = ["name"] - q = Q() - - if query: - for field in fields: - q |= Q(**{f"{field}__icontains": query}) + q = build_search_query(query, fields=CYCLE_SEARCH_FIELDS) cycles = ( Cycle.objects.filter( @@ -672,12 +585,7 @@ def get(self, request, slug): response_data["cycle"] = list(cycles) elif query_type == "module": - fields = ["name"] - q = Q() - - if query: - for field in fields: - q |= Q(**{f"{field}__icontains": query}) + q = build_search_query(query, fields=MODULE_SEARCH_FIELDS) modules = ( Module.objects.filter( @@ -700,12 +608,7 @@ def get(self, request, slug): response_data["module"] = list(modules) elif query_type == "page": - fields = ["name"] - q = Q() - - if query: - for field in fields: - q |= Q(**{f"{field}__icontains": query}) + q = build_search_query(query, fields=PAGE_SEARCH_FIELDS) pages = ( Page.objects.filter( diff --git a/apps/api/plane/tests/unit/utils/test_search.py b/apps/api/plane/tests/unit/utils/test_search.py new file mode 100644 index 00000000000..3977f289fc3 --- /dev/null +++ b/apps/api/plane/tests/unit/utils/test_search.py @@ -0,0 +1,120 @@ +# Copyright (c) 2023-present Plane Software, Inc. and contributors +# SPDX-License-Identifier: AGPL-3.0-only +# See the LICENSE file for details. + +import pytest +from django.db.models import Q + +from plane.utils.search import ( + ISSUE_SEARCH_FIELDS, + ISSUE_SEQUENCE_FIELDS, + build_search_query, +) + + +def _children(q): + """Flatten a Q tree into the set of leaf lookups it applies.""" + leaves = set() + for child in q.children: + if isinstance(child, Q): + leaves |= _children(child) + else: + leaves.add(child) + return leaves + + +@pytest.mark.unit +class TestBuildSearchQuery: + """Multi-word queries must match on words, not on one contiguous string.""" + + def test_empty_query_matches_nothing(self): + assert build_search_query("", fields=["name"]) == Q() + assert build_search_query(None, fields=["name"]) == Q() + assert build_search_query(" ", fields=["name"]) == Q() + + def test_single_token_ors_across_fields(self): + q = build_search_query("sage", fields=["name", "description_stripped"]) + assert _children(q) == { + ("name__icontains", "sage"), + ("description_stripped__icontains", "sage"), + } + assert q.connector == Q.OR + + def test_tokens_are_anded_not_matched_as_a_phrase(self): + q = build_search_query("payment gateway", fields=["name"]) + # Each token contributes its own leaf; the phrase itself is never a leaf + assert _children(q) == { + ("name__icontains", "payment"), + ("name__icontains", "gateway"), + } + assert ("name__icontains", "payment gateway") not in _children(q) + assert q.connector == Q.AND + + def test_word_order_and_interleaving_are_irrelevant(self): + forward = build_search_query("payment gateway", fields=["name"]) + reversed_ = build_search_query("gateway payment", fields=["name"]) + assert _children(forward) == _children(reversed_) + + def test_repeated_whitespace_does_not_create_empty_tokens(self): + q = build_search_query(" payment gateway \n", fields=["name"]) + assert _children(q) == { + ("name__icontains", "payment"), + ("name__icontains", "gateway"), + } + + def test_numeric_token_also_matches_sequence_id(self): + q = build_search_query( + "22", + fields=ISSUE_SEARCH_FIELDS, + sequence_fields=ISSUE_SEQUENCE_FIELDS, + ) + assert ("sequence_id", "22") in _children(q) + + def test_sequence_match_is_ored_onto_the_whole_predicate(self): + """A bare issue number keeps surfacing the issue even alongside words.""" + q = build_search_query( + "fix 22", + fields=ISSUE_SEARCH_FIELDS, + sequence_fields=ISSUE_SEQUENCE_FIELDS, + ) + assert q.connector == Q.OR + assert ("sequence_id", "22") in _children(q) + + def test_decimals_do_not_produce_sequence_matches(self): + q = build_search_query( + "3.5", + fields=ISSUE_SEARCH_FIELDS, + sequence_fields=ISSUE_SEQUENCE_FIELDS, + ) + assert ("sequence_id", "3") not in _children(q) + assert ("sequence_id", "5") not in _children(q) + + def test_version_strings_do_not_produce_sequence_matches(self): + q = build_search_query( + "v1.4.0", + fields=ISSUE_SEARCH_FIELDS, + sequence_fields=ISSUE_SEQUENCE_FIELDS, + ) + assert not any(lookup == "sequence_id" for lookup, _ in _children(q)) + + def test_trailing_punctuation_still_matches_a_sequence_id(self): + q = build_search_query( + "issue 22.", + fields=ISSUE_SEARCH_FIELDS, + sequence_fields=ISSUE_SEQUENCE_FIELDS, + ) + assert ("sequence_id", "22") in _children(q) + + def test_long_queries_are_not_mined_for_sequence_ids(self): + query = "the payment gateway on level 3 and its effective rate" + q = build_search_query( + query, + fields=ISSUE_SEARCH_FIELDS, + sequence_fields=ISSUE_SEQUENCE_FIELDS, + sequence_query_max_length=20, + ) + assert ("sequence_id", "3") not in _children(q) + + def test_no_sequence_fields_yields_no_sequence_leaves(self): + q = build_search_query("22", fields=["name"]) + assert _children(q) == {("name__icontains", "22")} diff --git a/apps/api/plane/utils/issue_search.py b/apps/api/plane/utils/issue_search.py index 7e5fab8fea3..54c36a2e7eb 100644 --- a/apps/api/plane/utils/issue_search.py +++ b/apps/api/plane/utils/issue_search.py @@ -2,23 +2,23 @@ # SPDX-License-Identifier: AGPL-3.0-only # See the LICENSE file for details. -# Python imports -import re - -# Django imports -from django.db.models import Q - # Module imports +from plane.utils.search import ( + ISSUE_SEARCH_FIELDS, + ISSUE_SEQUENCE_FIELDS, + build_search_query, +) + +# Queries longer than this are treated as prose and not mined for sequence ids +SEQUENCE_QUERY_MAX_LENGTH = 20 def search_issues(query, queryset): - fields = ["name", "sequence_id", "project__identifier"] - q = Q() - for field in fields: - if field == "sequence_id" and len(query) <= 20: - sequences = re.findall(r"\b\d+\b", query) - for sequence_id in sequences: - q |= Q(**{"sequence_id": sequence_id}) - else: - q |= Q(**{f"{field}__icontains": query}) - return queryset.filter(q).distinct() + return queryset.filter( + build_search_query( + query, + fields=ISSUE_SEARCH_FIELDS, + sequence_fields=ISSUE_SEQUENCE_FIELDS, + sequence_query_max_length=SEQUENCE_QUERY_MAX_LENGTH, + ) + ).distinct() diff --git a/apps/api/plane/utils/search.py b/apps/api/plane/utils/search.py new file mode 100644 index 00000000000..801efcda04a --- /dev/null +++ b/apps/api/plane/utils/search.py @@ -0,0 +1,81 @@ +# Copyright (c) 2023-present Plane Software, Inc. and contributors +# SPDX-License-Identifier: AGPL-3.0-only +# See the LICENSE file for details. + +# Python imports +import re + +# Django imports +from django.db.models import Q + +# Match whole integers only. The lookaround excludes the components of a +# decimal: a plain \b\d+\b treats the dot in "3.5" as a word boundary and +# yields both 3 and 5, so searching a version string surfaced unrelated issues +# by sequence id. A trailing dot that is not followed by a digit ("issue 22.") +# is still sentence punctuation, and 22 stays matchable. +SEQUENCE_PATTERN = re.compile(r"(? Date: Thu, 13 Aug 2026 19:15:13 -0400 Subject: [PATCH 2/5] feat(api): search work item and page bodies, not just titles MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Search only ever looked at titles, so a work item was findable only by the words its author managed to fit into one line. Anything explained in the body — the vendor being evaluated, the error being chased, the decision being made — was unreachable by any phrasing. Closes #3370, and is the same ask in #7108. Add description_stripped to the issue and page field lists. It is the plain-text projection of the rich-text body, already maintained by the model on save, so this needs no migration, no backfill and no new index. The permission filters are unchanged and applied to the same queryset, and the body is not added to the values() projection, so nothing becomes visible that a member could not already open. There are tests for both of those, because widening what is searched must not widen what is visible. --- .../api/plane/tests/unit/utils/test_search.py | 28 +++++++++++++++++++ apps/api/plane/utils/search.py | 9 ++++-- 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/apps/api/plane/tests/unit/utils/test_search.py b/apps/api/plane/tests/unit/utils/test_search.py index 3977f289fc3..8d792b686ea 100644 --- a/apps/api/plane/tests/unit/utils/test_search.py +++ b/apps/api/plane/tests/unit/utils/test_search.py @@ -8,6 +8,7 @@ from plane.utils.search import ( ISSUE_SEARCH_FIELDS, ISSUE_SEQUENCE_FIELDS, + PAGE_SEARCH_FIELDS, build_search_query, ) @@ -118,3 +119,30 @@ def test_long_queries_are_not_mined_for_sequence_ids(self): def test_no_sequence_fields_yields_no_sequence_leaves(self): q = build_search_query("22", fields=["name"]) assert _children(q) == {("name__icontains", "22")} + + +@pytest.mark.unit +class TestSearchableFields: + """Bodies are searchable, not just titles.""" + + def test_issues_search_their_description(self): + assert "description_stripped" in ISSUE_SEARCH_FIELDS + + def test_pages_search_their_description(self): + assert "description_stripped" in PAGE_SEARCH_FIELDS + + def test_a_word_only_in_the_body_is_matchable(self): + q = build_search_query( + "sage", + fields=ISSUE_SEARCH_FIELDS, + sequence_fields=ISSUE_SEQUENCE_FIELDS, + ) + assert ("description_stripped__icontains", "sage") in _children(q) + + def test_words_split_across_title_and_body_still_match(self): + """ "payment" and "gateway" from the title, "review" from the body.""" + q = build_search_query("payment gateway review", fields=ISSUE_SEARCH_FIELDS) + leaves = _children(q) + for token in ("payment", "gateway", "review"): + assert ("name__icontains", token) in leaves + assert ("description_stripped__icontains", token) in leaves diff --git a/apps/api/plane/utils/search.py b/apps/api/plane/utils/search.py index 801efcda04a..9373183442a 100644 --- a/apps/api/plane/utils/search.py +++ b/apps/api/plane/utils/search.py @@ -18,13 +18,18 @@ # Searchable fields per entity, shared by every search endpoint so that the # global search, the entity search and the project issue search cannot drift # apart. Adding a field here widens all of them at once. +# +# `description_stripped` is the plain-text projection of an entity's rich-text +# body, maintained on save, so searching it needs no migration and no new +# index. It is what makes a work item findable by anything its author wrote +# rather than only by the words that fit in a title. WORKSPACE_SEARCH_FIELDS = ["name"] PROJECT_SEARCH_FIELDS = ["name", "identifier"] -ISSUE_SEARCH_FIELDS = ["name", "project__identifier"] +ISSUE_SEARCH_FIELDS = ["name", "description_stripped", "project__identifier"] ISSUE_SEQUENCE_FIELDS = ["sequence_id"] CYCLE_SEARCH_FIELDS = ["name"] MODULE_SEARCH_FIELDS = ["name"] -PAGE_SEARCH_FIELDS = ["name"] +PAGE_SEARCH_FIELDS = ["name", "description_stripped"] VIEW_SEARCH_FIELDS = ["name"] USER_MENTION_SEARCH_FIELDS = [ "member__first_name", From aedc99ef2dc08bc9de78aa95a147e0be2367f3b2 Mon Sep 17 00:00:00 2001 From: Don Kendall Date: Sat, 15 Aug 2026 07:52:59 -0400 Subject: [PATCH 3/5] test(api): cover work item search at the unit and endpoint levels MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two layers, because one is not enough. The unit tests pin the shape of the Q tree; they pass whether or not description_stripped is ever populated, whether or not the permission filters still hold, and whether or not the projection leaks markup, because they never execute a query. The contract tests create work items through the model with description_html and drive both search endpoints, so they exercise the stripping, the scoping and the SQL. They also pin what must not change: titles still match, markup is not matchable, a bare number still resolves to its work item, another tenant's matching work item stays invisible, and a project the caller has left is not searched. The last two matter because this change widens the searched surface, and widening what is searched must not widen what is visible. The sequence-id tests deliberately assert today's noisy behaviour — a number anywhere in the query still matches by id, OR-ed onto the whole predicate — so that it stays a decision rather than an accident. --- .../tests/contract/app/test_search_app.py | 187 ++++++++++++++++++ .../api/plane/tests/unit/utils/test_search.py | 34 +++- 2 files changed, 218 insertions(+), 3 deletions(-) create mode 100644 apps/api/plane/tests/contract/app/test_search_app.py diff --git a/apps/api/plane/tests/contract/app/test_search_app.py b/apps/api/plane/tests/contract/app/test_search_app.py new file mode 100644 index 00000000000..bcb0fad5a00 --- /dev/null +++ b/apps/api/plane/tests/contract/app/test_search_app.py @@ -0,0 +1,187 @@ +# Copyright (c) 2023-present Plane Software, Inc. and contributors +# SPDX-License-Identifier: AGPL-3.0-only +# See the LICENSE file for details. + +"""Endpoint-level coverage for work item search. + +The unit tests next door assert the shape of the ``Q`` tree. These drive the +real endpoints against real rows, which is the only way to catch the things +that actually broke: a field that is never populated, a permission filter that +widens along with the search, or a projection that leaks markup. + +Modelled on the case that motivated the change: a title about a payment +gateway whose body is the only place the vendor's name appears. +""" + +from uuid import uuid4 + +import pytest +from rest_framework import status + +from plane.db.models import ( + Issue, + Project, + ProjectMember, + User, + Workspace, + WorkspaceMember, +) + +GLOBAL_SEARCH = "/api/workspaces/{slug}/search/" +PROJECT_ISSUE_SEARCH = "/api/workspaces/{slug}/projects/{project_id}/search-issues/" + + +@pytest.fixture +def project(db, workspace, create_user): + project = Project.objects.create( + name="Payments", + identifier="PAY", + workspace=workspace, + created_by=create_user, + ) + ProjectMember.objects.create(project=project, member=create_user, role=20, is_active=True) + return project + + +def _issue(project, name, body=""): + return Issue.objects.create( + name=name, + project=project, + workspace=project.workspace, + description_html=f"

{body}

" if body else "

", + ) + + +@pytest.fixture +def issues(db, project): + """The motivating case, plus neighbours that must not be swept up.""" + return { + "gateway": _issue( + project, + "Select the payment gateway on Level 3 capability and effective rate", + "Northwind is the incumbent gateway. Compare Northwind-Meridian Level 3 rates before the review.", + ), + "unrelated": _issue(project, "Ship the marketing site", "Nothing to do with payments."), + "decoy": _issue(project, "Rate limit the public API", "Throttling, not billing."), + } + + +def _search(client, slug, term, **params): + response = client.get( + GLOBAL_SEARCH.format(slug=slug), + {"search": term, "workspace_search": "true", "entities": "issue", **params}, + ) + assert response.status_code == status.HTTP_200_OK + return response.json()["results"]["issue"] + + +def _names(results): + return {row["name"] for row in results} + + +@pytest.mark.contract +class TestWorkItemSearchFindsBodies: + def test_a_word_only_in_the_body_is_found(self, session_client, workspace, issues): + """The case the old search could not reach by any phrasing.""" + results = _search(session_client, workspace.slug, "northwind") + assert _names(results) == {issues["gateway"].name} + + def test_search_is_case_insensitive_in_the_body(self, session_client, workspace, issues): + assert len(_search(session_client, workspace.slug, "NORTHWIND")) == 1 + + def test_body_matching_does_not_match_the_markup(self, session_client, workspace, issues): + """Bodies are searched as stripped text, so tag names are not matchable.""" + assert _search(session_client, workspace.slug, "

") == [] + + def test_titles_still_match(self, session_client, workspace, issues): + results = _search(session_client, workspace.slug, "marketing") + assert _names(results) == {issues["unrelated"].name} + + +@pytest.mark.contract +class TestWorkItemSearchMatchesWords: + def test_words_need_not_be_adjacent(self, session_client, workspace, issues): + """ "payment" and "gateway" from the title, "review" from the body.""" + results = _search(session_client, workspace.slug, "payment gateway review") + assert _names(results) == {issues["gateway"].name} + + def test_word_order_does_not_matter(self, session_client, workspace, issues): + assert len(_search(session_client, workspace.slug, "gateway payment")) == 1 + + def test_every_word_must_match(self, session_client, workspace, issues): + """Tokens are AND-ed, so one unmatched word rules the record out.""" + assert _search(session_client, workspace.slug, "payment gateway alpaca") == [] + + def test_words_from_different_records_do_not_combine(self, session_client, workspace, issues): + """ "rate" is in the decoy's title, "payment" is not.""" + results = _search(session_client, workspace.slug, "rate payment") + assert _names(results) == {issues["gateway"].name} + + +@pytest.mark.contract +class TestSequenceIdLookup: + def test_a_bare_number_finds_the_work_item(self, session_client, workspace, issues): + target = issues["gateway"] + results = _search(session_client, workspace.slug, str(target.sequence_id)) + assert target.name in _names(results) + + def test_identifier_and_number_together_find_it(self, session_client, workspace, issues): + target = issues["gateway"] + results = _search(session_client, workspace.slug, f"PAY-{target.sequence_id}") + assert target.name in _names(results) + + def test_a_number_inside_a_phrase_still_matches_by_id(self, session_client, workspace, issues): + """Unchanged behaviour, pinned deliberately. + + A number anywhere in the query matches by sequence id, OR-ed onto the + whole predicate, so the work item carrying it comes back even though + neither word matches it. Noisy, but narrowing it would drop results + that match today and this change is a strict superset. + """ + decoy = issues["decoy"] + results = _search(session_client, workspace.slug, f"payment gateway {decoy.sequence_id}") + assert decoy.name in _names(results) + + +@pytest.mark.contract +class TestSearchDoesNotWidenVisibility: + """Searching more fields must not surface more records than the caller may see.""" + + def test_another_tenants_work_item_is_not_returned(self, session_client, workspace, issues): + uid = uuid4().hex[:8] + # username is unique and not auto-populated, so it has to be set here + other_user = User.objects.create(email=f"other-{uid}@plane.so", username=f"other_{uid}") + other_ws = Workspace.objects.create(name="Other WS", owner=other_user, slug=f"other-{uid}") + WorkspaceMember.objects.create(workspace=other_ws, member=other_user, role=20) + other_project = Project.objects.create( + name="Other", identifier="OTH", workspace=other_ws, created_by=other_user + ) + _issue(other_project, "Their gateway work", "Northwind everywhere in this body too.") + + results = _search(session_client, workspace.slug, "northwind") + assert _names(results) == {issues["gateway"].name} + + def test_a_project_the_caller_left_is_not_searched(self, session_client, workspace, project, issues, create_user): + ProjectMember.objects.filter(project=project, member=create_user).update(is_active=False) + assert _search(session_client, workspace.slug, "northwind") == [] + + +@pytest.mark.contract +class TestProjectScopedIssueSearch: + """The other endpoint, which goes through plane.utils.issue_search.""" + + def test_body_search_applies_here_too(self, session_client, workspace, project, issues): + response = session_client.get( + PROJECT_ISSUE_SEARCH.format(slug=workspace.slug, project_id=project.id), + {"search": "northwind"}, + ) + assert response.status_code == status.HTTP_200_OK + assert _names(response.json()) == {issues["gateway"].name} + + def test_multi_word_applies_here_too(self, session_client, workspace, project, issues): + response = session_client.get( + PROJECT_ISSUE_SEARCH.format(slug=workspace.slug, project_id=project.id), + {"search": "payment gateway review"}, + ) + assert response.status_code == status.HTTP_200_OK + assert _names(response.json()) == {issues["gateway"].name} diff --git a/apps/api/plane/tests/unit/utils/test_search.py b/apps/api/plane/tests/unit/utils/test_search.py index 8d792b686ea..8663467a19e 100644 --- a/apps/api/plane/tests/unit/utils/test_search.py +++ b/apps/api/plane/tests/unit/utils/test_search.py @@ -72,15 +72,42 @@ def test_numeric_token_also_matches_sequence_id(self): assert ("sequence_id", "22") in _children(q) def test_sequence_match_is_ored_onto_the_whole_predicate(self): - """A bare issue number keeps surfacing the issue even alongside words.""" + """A bare issue number reaches the issue regardless of its title.""" q = build_search_query( - "fix 22", + "22", fields=ISSUE_SEARCH_FIELDS, sequence_fields=ISSUE_SEQUENCE_FIELDS, ) assert q.connector == Q.OR assert ("sequence_id", "22") in _children(q) + def test_identifier_and_number_in_one_token_still_matches_by_number(self): + q = build_search_query( + "DUROPC-22", + fields=ISSUE_SEARCH_FIELDS, + sequence_fields=ISSUE_SEQUENCE_FIELDS, + ) + assert ("sequence_id", "22") in _children(q) + + def test_sequence_lookup_still_applies_to_multi_word_queries(self): + """Unchanged from before this refactor, and deliberately so. + + A number anywhere in the query still matches by sequence id, OR-ed onto + the whole predicate, so a work item carrying that number is returned + even when the words do not match it. That is noisy — "level 3 rate" + returns every work item numbered 3 — but narrowing it would remove + results that match today, and this change is meant to be a strict + superset. Pinned here so the behaviour is a decision rather than an + accident. + """ + q = build_search_query( + "level 3 rate", + fields=ISSUE_SEARCH_FIELDS, + sequence_fields=ISSUE_SEQUENCE_FIELDS, + ) + assert ("sequence_id", "3") in _children(q) + assert q.connector == Q.OR + def test_decimals_do_not_produce_sequence_matches(self): q = build_search_query( "3.5", @@ -99,8 +126,9 @@ def test_version_strings_do_not_produce_sequence_matches(self): assert not any(lookup == "sequence_id" for lookup, _ in _children(q)) def test_trailing_punctuation_still_matches_a_sequence_id(self): + """A trailing dot is sentence punctuation, not a decimal point.""" q = build_search_query( - "issue 22.", + "22.", fields=ISSUE_SEARCH_FIELDS, sequence_fields=ISSUE_SEQUENCE_FIELDS, ) From a4b9cb7c16ff881cefe1227fe2be870ad393105d Mon Sep 17 00:00:00 2001 From: Don Kendall Date: Sat, 15 Aug 2026 08:03:49 -0400 Subject: [PATCH 4/5] test(api): close the gaps found in review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three follow-ups from review feedback: The sequence-id regex still pulled 5 out of ".5". A digit preceded by a dot is part of a decimal just as much as one followed by a dot, so searching a bare fraction surfaced an unrelated work item by number. Widen the lookbehind and pin it with a regression test. test_another_tenants_work_item_is_not_returned put the decoy in a second workspace, where workspace__slug alone excludes it — the test passed with the project-membership filter deleted. Keep it, because workspace isolation is worth pinning, and add the case it was assumed to cover: a project in the caller's own workspace that the caller is not a member of. That one fails without the membership filter. Page body search had no endpoint coverage — only an assertion that the field constant contained the column, which would pass even if no handler used it. Add contract tests that create a page whose term appears only in the body, and one whose term is only in the title. --- .../tests/contract/api/test_authentication.py | 4 +- .../api/test_project_members_roster_scope.py | 12 +--- .../tests/contract/app/test_authentication.py | 16 ++--- .../contract/app/test_cycle_issue_app.py | 4 +- .../test_deploy_board_project_scope_app.py | 8 +-- .../app/test_issue_list_guest_scope_app.py | 16 ++--- .../test_page_version_project_scope_app.py | 12 +--- .../test_project_member_is_active_authz.py | 4 +- .../tests/contract/app/test_search_app.py | 58 +++++++++++++++++++ ...kspace_cycles_modules_project_scope_app.py | 4 +- ..._workspace_file_asset_project_scope_app.py | 40 ++++--------- .../app/test_workspace_user_preference_app.py | 8 +-- .../unit/bg_tasks/test_ssrf_advisories.py | 46 +++++---------- .../tests/unit/bg_tasks/test_url_security.py | 16 ++--- .../unit/bg_tasks/test_work_item_link_task.py | 8 +-- .../middleware/test_api_authentication.py | 8 +-- .../tests/unit/middleware/test_logger.py | 4 +- .../unit/utils/test_order_by_sanitize.py | 40 +++---------- .../api/plane/tests/unit/utils/test_search.py | 21 +++++-- apps/api/plane/utils/search.py | 7 ++- 20 files changed, 143 insertions(+), 193 deletions(-) diff --git a/apps/api/plane/tests/contract/api/test_authentication.py b/apps/api/plane/tests/contract/api/test_authentication.py index d240567cccf..5dfa4e467a2 100644 --- a/apps/api/plane/tests/contract/api/test_authentication.py +++ b/apps/api/plane/tests/contract/api/test_authentication.py @@ -26,9 +26,7 @@ def test_active_user_can_access_with_api_key(self, api_key_client): assert response.status_code == status.HTTP_200_OK @pytest.mark.django_db - def test_deactivated_user_cannot_access_with_api_key( - self, api_key_client, create_user - ): + def test_deactivated_user_cannot_access_with_api_key(self, api_key_client, create_user): # The account is disabled after the API key was generated. create_user.is_active = False create_user.save() diff --git a/apps/api/plane/tests/contract/api/test_project_members_roster_scope.py b/apps/api/plane/tests/contract/api/test_project_members_roster_scope.py index c4ee14b255d..e0284f15f3d 100644 --- a/apps/api/plane/tests/contract/api/test_project_members_roster_scope.py +++ b/apps/api/plane/tests/contract/api/test_project_members_roster_scope.py @@ -40,9 +40,7 @@ def attacker_membership(db, workspace, create_user): workspace=workspace, created_by=create_user, ) - ProjectMember.objects.create( - project=other, member=create_user, workspace=workspace, role=20 - ) + ProjectMember.objects.create(project=other, member=create_user, workspace=workspace, role=20) return other @@ -62,9 +60,7 @@ def foreign_project(db, workspace): workspace=workspace, created_by=owner, ) - ProjectMember.objects.create( - project=project, member=owner, workspace=workspace, role=20 - ) + ProjectMember.objects.create(project=project, member=owner, workspace=workspace, role=20) return project @@ -90,9 +86,7 @@ def test_project_member_can_list_roster(self, api_key_client, workspace, create_ workspace=workspace, created_by=create_user, ) - ProjectMember.objects.create( - project=project, member=create_user, workspace=workspace, role=20 - ) + ProjectMember.objects.create(project=project, member=create_user, workspace=workspace, role=20) response = api_key_client.get(members_url(workspace.slug, project.id)) assert response.status_code == status.HTTP_200_OK, ( f"Got {response.status_code}: {getattr(response, 'data', None)!r}" diff --git a/apps/api/plane/tests/contract/app/test_authentication.py b/apps/api/plane/tests/contract/app/test_authentication.py index 7a7d9640b74..6c79c3cac58 100644 --- a/apps/api/plane/tests/contract/app/test_authentication.py +++ b/apps/api/plane/tests/contract/app/test_authentication.py @@ -513,8 +513,8 @@ def test_exhausted_after_max_wrong_attempts( # First (MAX-1) wrong attempts: each redirects with INVALID_MAGIC_CODE_SIGN_IN. for i in range(MagicCodeProvider.MAX_VERIFY_ATTEMPTS - 1): response = django_client.post(url, {"email": self.EMAIL, "code": "000000"}, follow=False) - assert response.status_code == 302, f"attempt {i+1} unexpected status" - assert "INVALID_MAGIC_CODE_SIGN_IN" in response.url, f"attempt {i+1} did not return INVALID" + assert response.status_code == 302, f"attempt {i + 1} unexpected status" + assert "INVALID_MAGIC_CODE_SIGN_IN" in response.url, f"attempt {i + 1} did not return INVALID" # Token and counter both still live, with counter at MAX-1. assert ri.exists(f"magic_{self.EMAIL}") @@ -704,9 +704,7 @@ def test_bot_password_sign_in_blocked(self, django_client, bot_user, setup_insta """Password sign-in with a bot's *correct* credentials is still rejected: the block happens after credential verification, so no session is created.""" url = reverse("sign-in") - response = django_client.post( - url, {"email": self.BOT_EMAIL, "password": self.PASSWORD}, follow=False - ) + response = django_client.post(url, {"email": self.BOT_EMAIL, "password": self.PASSWORD}, follow=False) assert response.status_code == 302 assert "BOT_USER_LOGIN_FORBIDDEN" in response.url # The block must prevent authentication. @@ -714,9 +712,7 @@ def test_bot_password_sign_in_blocked(self, django_client, bot_user, setup_insta @pytest.mark.django_db @patch("plane.bgtasks.magic_link_code_task.magic_link.delay") - def test_bot_magic_sign_in_blocked( - self, mock_magic_link, django_client, api_client, bot_user, setup_instance - ): + def test_bot_magic_sign_in_blocked(self, mock_magic_link, django_client, api_client, bot_user, setup_instance): """The same block applies via a second provider (magic code), proving the guard sits at the shared chokepoint rather than in one provider.""" token = _generate_magic_token(api_client, self.BOT_EMAIL) @@ -731,9 +727,7 @@ def test_human_password_sign_in_allowed(self, django_client, human_user, setup_i """Control: a normal user with the identical setup still signs in — the guard is scoped strictly to is_bot and does not regress human logins.""" url = reverse("sign-in") - response = django_client.post( - url, {"email": self.HUMAN_EMAIL, "password": self.PASSWORD}, follow=False - ) + response = django_client.post(url, {"email": self.HUMAN_EMAIL, "password": self.PASSWORD}, follow=False) assert response.status_code == 302 assert "BOT_USER_LOGIN_FORBIDDEN" not in response.url assert "error_code" not in response.url diff --git a/apps/api/plane/tests/contract/app/test_cycle_issue_app.py b/apps/api/plane/tests/contract/app/test_cycle_issue_app.py index 462852bc1b4..aa7e1283d5a 100644 --- a/apps/api/plane/tests/contract/app/test_cycle_issue_app.py +++ b/apps/api/plane/tests/contract/app/test_cycle_issue_app.py @@ -132,9 +132,7 @@ def test_foreign_tenant_cycle_issue_not_reassigned( "Cross-tenant reassignment: victim's CycleIssue was moved to the attacker's cycle" ) # No CycleIssue for the victim's issue should exist under the attacker's cycle. - assert not CycleIssue.objects.filter( - cycle_id=attacker_cycle.id, issue_id=victim_issue.id - ).exists() + assert not CycleIssue.objects.filter(cycle_id=attacker_cycle.id, issue_id=victim_issue.id).exists() @pytest.mark.django_db def test_same_tenant_reassignment_still_works( diff --git a/apps/api/plane/tests/contract/app/test_deploy_board_project_scope_app.py b/apps/api/plane/tests/contract/app/test_deploy_board_project_scope_app.py index 0dbfbbcb807..5eb72853034 100644 --- a/apps/api/plane/tests/contract/app/test_deploy_board_project_scope_app.py +++ b/apps/api/plane/tests/contract/app/test_deploy_board_project_scope_app.py @@ -36,9 +36,7 @@ def project(db, workspace, create_user): workspace=workspace, created_by=create_user, ) - ProjectMember.objects.create( - project=project, member=create_user, workspace=workspace, role=20 - ) + ProjectMember.objects.create(project=project, member=create_user, workspace=workspace, role=20) return project @@ -65,9 +63,7 @@ def outsider_client(db, workspace, create_user): workspace=workspace, created_by=outsider, ) - ProjectMember.objects.create( - project=other_project, member=outsider, workspace=workspace, role=15 - ) + ProjectMember.objects.create(project=other_project, member=outsider, workspace=workspace, role=15) client = APIClient() client.force_authenticate(user=outsider) return client diff --git a/apps/api/plane/tests/contract/app/test_issue_list_guest_scope_app.py b/apps/api/plane/tests/contract/app/test_issue_list_guest_scope_app.py index f48efbda7df..bf7d26edfc4 100644 --- a/apps/api/plane/tests/contract/app/test_issue_list_guest_scope_app.py +++ b/apps/api/plane/tests/contract/app/test_issue_list_guest_scope_app.py @@ -41,9 +41,7 @@ def project(db, workspace, create_user): workspace=workspace, created_by=create_user, ) - ProjectMember.objects.create( - project=project, member=create_user, workspace=workspace, role=20 - ) + ProjectMember.objects.create(project=project, member=create_user, workspace=workspace, role=20) return project @@ -60,9 +58,7 @@ def guest(db, workspace, project): user.set_password("test-password") user.save() WorkspaceMember.objects.create(workspace=workspace, member=user, role=5) - ProjectMember.objects.create( - project=project, member=user, workspace=workspace, role=5 - ) + ProjectMember.objects.create(project=project, member=user, workspace=workspace, role=5) return user @@ -102,9 +98,7 @@ class TestIssueListGuestScope: """A restricted guest must only get back issues they authored.""" @pytest.mark.django_db - def test_guest_cannot_read_foreign_issue( - self, guest_client, workspace, project, own_issue, foreign_issue - ): + def test_guest_cannot_read_foreign_issue(self, guest_client, workspace, project, own_issue, foreign_issue): url = LIST_URL.format(slug=workspace.slug, project_id=project.id) response = guest_client.get(url, {"issues": f"{own_issue.id},{foreign_issue.id}"}) @@ -113,9 +107,7 @@ def test_guest_cannot_read_foreign_issue( ) returned_ids = {str(row["id"]) for row in response.data} assert str(own_issue.id) in returned_ids - assert str(foreign_issue.id) not in returned_ids, ( - f"Guest read a foreign issue: {response.data!r}" - ) + assert str(foreign_issue.id) not in returned_ids, f"Guest read a foreign issue: {response.data!r}" @pytest.mark.django_db def test_project_member_reads_all_requested_issues( diff --git a/apps/api/plane/tests/contract/app/test_page_version_project_scope_app.py b/apps/api/plane/tests/contract/app/test_page_version_project_scope_app.py index f9176a3ce95..4b2ccfd463b 100644 --- a/apps/api/plane/tests/contract/app/test_page_version_project_scope_app.py +++ b/apps/api/plane/tests/contract/app/test_page_version_project_scope_app.py @@ -96,9 +96,7 @@ def test_cross_project_version_detail_denied(self, session_client, workspace, cr """Reading a single cross-project page version must be denied.""" _, project_a, _, page_b, version_b = self._setup(workspace, create_user) - response = session_client.get( - _page_versions_url(workspace.slug, project_a.id, page_b.id, pk=version_b.id) - ) + response = session_client.get(_page_versions_url(workspace.slug, project_a.id, page_b.id, pk=version_b.id)) assert response.status_code == status.HTTP_403_FORBIDDEN @@ -126,13 +124,9 @@ def test_revoked_project_link_denied(self, session_client, workspace, create_use though the attacker is a member of that project.""" victim, project_a, _, _, _ = self._setup(workspace, create_user) - page = Page.objects.create( - workspace=workspace, owned_by=victim, access=Page.PUBLIC_ACCESS, name="Removed page" - ) + page = Page.objects.create(workspace=workspace, owned_by=victim, access=Page.PUBLIC_ACCESS, name="Removed page") # Link exists but is soft-deleted → the page no longer belongs to A. - ProjectPage.objects.create( - workspace=workspace, project=project_a, page=page, deleted_at=timezone.now() - ) + ProjectPage.objects.create(workspace=workspace, project=project_a, page=page, deleted_at=timezone.now()) _make_version(workspace, page, victim) response = session_client.get(_page_versions_url(workspace.slug, project_a.id, page.id)) diff --git a/apps/api/plane/tests/contract/app/test_project_member_is_active_authz.py b/apps/api/plane/tests/contract/app/test_project_member_is_active_authz.py index d59cad0591b..87ee3fb43d7 100644 --- a/apps/api/plane/tests/contract/app/test_project_member_is_active_authz.py +++ b/apps/api/plane/tests/contract/app/test_project_member_is_active_authz.py @@ -57,9 +57,7 @@ def project(db, workspace, create_user): ) # create_user is the workspace owner (role=20 via the workspace fixture); # make them a project ADMIN too — this is the takeover victim. - ProjectMember.objects.create( - workspace=workspace, project=project, member=create_user, role=20, is_active=True - ) + ProjectMember.objects.create(workspace=workspace, project=project, member=create_user, role=20, is_active=True) return project diff --git a/apps/api/plane/tests/contract/app/test_search_app.py b/apps/api/plane/tests/contract/app/test_search_app.py index bcb0fad5a00..0ea9d7dc473 100644 --- a/apps/api/plane/tests/contract/app/test_search_app.py +++ b/apps/api/plane/tests/contract/app/test_search_app.py @@ -20,8 +20,10 @@ from plane.db.models import ( Issue, + Page, Project, ProjectMember, + ProjectPage, User, Workspace, WorkspaceMember, @@ -161,6 +163,27 @@ def test_another_tenants_work_item_is_not_returned(self, session_client, workspa results = _search(session_client, workspace.slug, "northwind") assert _names(results) == {issues["gateway"].name} + def test_a_project_in_my_workspace_that_i_am_not_a_member_of_is_not_searched( + self, session_client, workspace, issues, create_user + ): + """Same workspace, different project, no membership row at all. + + Distinct from the cross-tenant case above, which the workspace filter + alone would catch — this one only passes because of the project + membership filter. + """ + uid = uuid4().hex[:8] + other_user = User.objects.create(email=f"peer-{uid}@plane.so", username=f"peer_{uid}") + WorkspaceMember.objects.create(workspace=workspace, member=other_user, role=20) + their_project = Project.objects.create( + name="Theirs", identifier=f"T{uid[:3].upper()}", workspace=workspace, created_by=other_user + ) + ProjectMember.objects.create(project=their_project, member=other_user, role=20, is_active=True) + _issue(their_project, "Their gateway work", "Northwind appears only in this body.") + + results = _search(session_client, workspace.slug, "northwind") + assert _names(results) == {issues["gateway"].name} + def test_a_project_the_caller_left_is_not_searched(self, session_client, workspace, project, issues, create_user): ProjectMember.objects.filter(project=project, member=create_user).update(is_active=False) assert _search(session_client, workspace.slug, "northwind") == [] @@ -185,3 +208,38 @@ def test_multi_word_applies_here_too(self, session_client, workspace, project, i ) assert response.status_code == status.HTTP_200_OK assert _names(response.json()) == {issues["gateway"].name} + + +@pytest.mark.contract +class TestPageSearch: + """Pages gained body search in the same change; cover the endpoint, not + just the field constant.""" + + def _page(self, workspace, project, owner, name, body): + page = Page.objects.create( + name=name, + workspace=workspace, + owned_by=owner, + access=0, + description_html=f"

{body}

", + ) + ProjectPage.objects.create(project=project, page=page, workspace=workspace) + return page + + def test_a_word_only_in_the_page_body_is_found(self, session_client, workspace, project, create_user): + page = self._page(workspace, project, create_user, "Vendor evaluation", "Northwind pricing notes.") + response = session_client.get( + GLOBAL_SEARCH.format(slug=workspace.slug), + {"search": "northwind", "workspace_search": "true", "entities": "page"}, + ) + assert response.status_code == status.HTTP_200_OK + assert _names(response.json()["results"]["page"]) == {page.name} + + def test_page_titles_still_match(self, session_client, workspace, project, create_user): + page = self._page(workspace, project, create_user, "Runbook", "Nothing relevant here.") + response = session_client.get( + GLOBAL_SEARCH.format(slug=workspace.slug), + {"search": "runbook", "workspace_search": "true", "entities": "page"}, + ) + assert response.status_code == status.HTTP_200_OK + assert page.name in _names(response.json()["results"]["page"]) diff --git a/apps/api/plane/tests/contract/app/test_workspace_cycles_modules_project_scope_app.py b/apps/api/plane/tests/contract/app/test_workspace_cycles_modules_project_scope_app.py index c18acc86406..2a212eb3640 100644 --- a/apps/api/plane/tests/contract/app/test_workspace_cycles_modules_project_scope_app.py +++ b/apps/api/plane/tests/contract/app/test_workspace_cycles_modules_project_scope_app.py @@ -42,9 +42,7 @@ def project(db, workspace, create_user): workspace=workspace, created_by=create_user, ) - ProjectMember.objects.create( - project=project, member=create_user, workspace=workspace, role=20 - ) + ProjectMember.objects.create(project=project, member=create_user, workspace=workspace, role=20) return project diff --git a/apps/api/plane/tests/contract/app/test_workspace_file_asset_project_scope_app.py b/apps/api/plane/tests/contract/app/test_workspace_file_asset_project_scope_app.py index 8845fbeda77..709cc4af141 100644 --- a/apps/api/plane/tests/contract/app/test_workspace_file_asset_project_scope_app.py +++ b/apps/api/plane/tests/contract/app/test_workspace_file_asset_project_scope_app.py @@ -43,9 +43,7 @@ def project(db, workspace, create_user): workspace=workspace, created_by=create_user, ) - ProjectMember.objects.create( - project=project, member=create_user, workspace=workspace, role=20 - ) + ProjectMember.objects.create(project=project, member=create_user, workspace=workspace, role=20) return project @@ -67,9 +65,7 @@ def outsider_user(db): @pytest.fixture def outsider_client(db, workspace, outsider_user): """Session client for a workspace member who is not in ``project``.""" - WorkspaceMember.objects.create( - workspace=workspace, member=outsider_user, role=15 - ) + WorkspaceMember.objects.create(workspace=workspace, member=outsider_user, role=15) client = APIClient() client.force_authenticate(user=outsider_user) return client @@ -115,17 +111,13 @@ class TestWorkspaceFileAssetProjectScope: """A workspace member who is not in the asset's project must be blocked.""" @pytest.mark.django_db - def test_get_project_asset_denied_for_non_project_member( - self, outsider_client, workspace, project_asset - ): + def test_get_project_asset_denied_for_non_project_member(self, outsider_client, workspace, project_asset): """GET on a project asset by a non-project-member must 403, not mint a presigned download URL.""" url = detail_url(workspace.slug, project_asset.id) with mock.patch(S3_STORAGE_PATH) as mock_storage: - mock_storage.return_value.generate_presigned_url.return_value = ( - "https://signed.example/download" - ) + mock_storage.return_value.generate_presigned_url.return_value = "https://signed.example/download" response = outsider_client.get(url) assert response.status_code == status.HTTP_403_FORBIDDEN, ( @@ -134,18 +126,14 @@ def test_get_project_asset_denied_for_non_project_member( mock_storage.return_value.generate_presigned_url.assert_not_called() @pytest.mark.django_db - def test_patch_project_asset_denied_for_non_project_member( - self, outsider_client, workspace, project_asset - ): + def test_patch_project_asset_denied_for_non_project_member(self, outsider_client, workspace, project_asset): """PATCH on a project asset by a non-project-member must 403 and leave the asset untouched.""" url = detail_url(workspace.slug, project_asset.id) project_asset.is_uploaded = False project_asset.save(update_fields=["is_uploaded"]) - response = outsider_client.patch( - url, {"attributes": {"name": "hacked.pdf"}}, format="json" - ) + response = outsider_client.patch(url, {"attributes": {"name": "hacked.pdf"}}, format="json") assert response.status_code == status.HTTP_403_FORBIDDEN, ( f"Got {response.status_code}: {getattr(response, 'data', None)!r}" @@ -155,9 +143,7 @@ def test_patch_project_asset_denied_for_non_project_member( assert project_asset.attributes.get("name") == "secret.pdf" @pytest.mark.django_db - def test_delete_project_asset_denied_for_non_project_member( - self, outsider_client, workspace, project_asset - ): + def test_delete_project_asset_denied_for_non_project_member(self, outsider_client, workspace, project_asset): """DELETE on a project asset by a non-project-member must 403 and must not soft-delete the asset.""" url = detail_url(workspace.slug, project_asset.id) @@ -171,17 +157,13 @@ def test_delete_project_asset_denied_for_non_project_member( assert project_asset.is_deleted is False @pytest.mark.django_db - def test_get_project_asset_allowed_for_project_member( - self, session_client, workspace, project_asset - ): + def test_get_project_asset_allowed_for_project_member(self, session_client, workspace, project_asset): """Positive control: an active project member can still download the asset, so the fix does not over-block legitimate callers.""" url = detail_url(workspace.slug, project_asset.id) with mock.patch(S3_STORAGE_PATH) as mock_storage: - mock_storage.return_value.generate_presigned_url.return_value = ( - "https://signed.example/download" - ) + mock_storage.return_value.generate_presigned_url.return_value = "https://signed.example/download" response = session_client.get(url) assert response.status_code == status.HTTP_302_FOUND, ( @@ -198,9 +180,7 @@ def test_get_workspace_level_asset_allowed_for_non_project_member( url = detail_url(workspace.slug, workspace_logo_asset.id) with mock.patch(S3_STORAGE_PATH) as mock_storage: - mock_storage.return_value.generate_presigned_url.return_value = ( - "https://signed.example/download" - ) + mock_storage.return_value.generate_presigned_url.return_value = "https://signed.example/download" response = outsider_client.get(url) assert response.status_code == status.HTTP_302_FOUND, ( diff --git a/apps/api/plane/tests/contract/app/test_workspace_user_preference_app.py b/apps/api/plane/tests/contract/app/test_workspace_user_preference_app.py index e2ef308b82f..534bc1c6550 100644 --- a/apps/api/plane/tests/contract/app/test_workspace_user_preference_app.py +++ b/apps/api/plane/tests/contract/app/test_workspace_user_preference_app.py @@ -48,9 +48,7 @@ def test_patch_only_updates_requesting_users_preference(self, session_client, cr WorkspaceUserPreference.objects.filter(pk=other_pref.pk).update(created_at=now + timedelta(minutes=1)) url = reverse("workspace-user-preference", kwargs={"slug": workspace.slug}) - response = session_client.patch( - url, [{"key": self.KEY, "is_pinned": True, "sort_order": 999}], format="json" - ) + response = session_client.patch(url, [{"key": self.KEY, "is_pinned": True, "sort_order": 999}], format="json") assert response.status_code == status.HTTP_200_OK @@ -72,9 +70,7 @@ def test_patch_updates_own_preference(self, session_client, create_user, workspa ) url = reverse("workspace-user-preference", kwargs={"slug": workspace.slug}) - response = session_client.patch( - url, [{"key": self.KEY, "is_pinned": True, "sort_order": 42}], format="json" - ) + response = session_client.patch(url, [{"key": self.KEY, "is_pinned": True, "sort_order": 42}], format="json") assert response.status_code == status.HTTP_200_OK diff --git a/apps/api/plane/tests/unit/bg_tasks/test_ssrf_advisories.py b/apps/api/plane/tests/unit/bg_tasks/test_ssrf_advisories.py index ea4adbc1c20..294cbdf4c67 100644 --- a/apps/api/plane/tests/unit/bg_tasks/test_ssrf_advisories.py +++ b/apps/api/plane/tests/unit/bg_tasks/test_ssrf_advisories.py @@ -70,14 +70,14 @@ class TestWebhookUrlValidation: "ip", [ "169.254.169.254", # AWS/GCP metadata (CVE-2026-30242 PoC) - "127.0.0.1", # loopback - "10.0.0.1", # private - "172.16.0.1", # private - "192.168.0.1", # private - "::1", # IPv6 loopback - "100.64.0.1", # CGNAT / RFC 6598 (GHSA-75fg) - "2002:7f00:1::", # 6to4 -> 127.0.0.1 (GHSA-75fg) - "224.0.0.1", # multicast (GHSA-75fg) + "127.0.0.1", # loopback + "10.0.0.1", # private + "172.16.0.1", # private + "192.168.0.1", # private + "::1", # IPv6 loopback + "100.64.0.1", # CGNAT / RFC 6598 (GHSA-75fg) + "2002:7f00:1::", # 6to4 -> 127.0.0.1 (GHSA-75fg) + "224.0.0.1", # multicast (GHSA-75fg) "::ffff:169.254.169.254", # IPv4-mapped metadata ], ) @@ -173,9 +173,7 @@ def test_webhook_does_not_follow_redirects(self, mock_resolve, mock_session_cls) mock_resolve.return_value = ["93.184.216.34"] session = mock_session_cls.return_value # The endpoint replies 302 -> internal; the webhook client must NOT follow. - session.request.return_value = _resp( - 302, headers={"Location": "http://169.254.169.254/latest/meta-data/"} - ) + session.request.return_value = _resp(302, headers={"Location": "http://169.254.169.254/latest/meta-data/"}) resp = pinned_fetch("POST", "https://hooks.example.com/x", json={}) @@ -195,17 +193,13 @@ class TestFaviconRedirect: @patch("plane.utils.url_security.requests.Session") @patch("plane.utils.url_security.resolve_and_validate") @patch("plane.bgtasks.work_item_link_task.socket.getaddrinfo") - def test_favicon_redirect_to_private_returns_default( - self, mock_pre_dns, mock_resolve, mock_session_cls - ): + def test_favicon_redirect_to_private_returns_default(self, mock_pre_dns, mock_resolve, mock_session_cls): # validate_url_ip pre-check (work_item_link_task.socket) sees a public IP. mock_pre_dns.return_value = [_addr("93.184.216.34")] # safe_get: hop0 public, hop1 (redirect target) blocked. mock_resolve.side_effect = [["93.184.216.34"], ValueError(_BLOCKED)] session = mock_session_cls.return_value - session.request.return_value = _resp( - 302, headers={"Location": "http://192.168.8.14:8081/"} - ) + session.request.return_value = _resp(302, headers={"Location": "http://192.168.8.14:8081/"}) soup = BeautifulSoup( '', @@ -227,11 +221,9 @@ class TestFaviconRebinding: @patch("plane.utils.url_security.requests.Session") @patch("plane.utils.url_security.resolve_and_validate") @patch("plane.bgtasks.work_item_link_task.socket.getaddrinfo") - def test_favicon_rebind_to_private_returns_default( - self, mock_pre_dns, mock_resolve, mock_session_cls - ): + def test_favicon_rebind_to_private_returns_default(self, mock_pre_dns, mock_resolve, mock_session_cls): mock_pre_dns.return_value = [_addr("93.184.216.34")] # pre-check: public - mock_resolve.side_effect = ValueError(_BLOCKED) # fetch-time: rebound -> blocked + mock_resolve.side_effect = ValueError(_BLOCKED) # fetch-time: rebound -> blocked session = mock_session_cls.return_value session.request.return_value = _resp(200) @@ -268,12 +260,8 @@ def test_avatar_redirect_to_internal_is_blocked(self, mock_resolve, mock_session # Public avatar URL that 302-redirects to the metadata service. mock_resolve.side_effect = [["93.184.216.34"], ValueError(_BLOCKED)] session = mock_session_cls.return_value - session.request.return_value = _resp( - 302, headers={"Location": "http://169.254.169.254/imds"} - ) - result = self._adapter().download_and_upload_avatar( - "https://evil.example.com/avatar", user=MagicMock() - ) + session.request.return_value = _resp(302, headers={"Location": "http://169.254.169.254/imds"}) + result = self._adapter().download_and_upload_avatar("https://evil.example.com/avatar", user=MagicMock()) assert result is None @patch("plane.authentication.adapter.base.pinned_fetch_following_redirects") @@ -281,9 +269,7 @@ def test_avatar_uses_ssrf_safe_client(self, mock_fetch): # Wiring guard: the avatar path must go through the pinned client, never # a raw requests.get (which would re-resolve + follow redirects freely). mock_fetch.side_effect = ValueError(_BLOCKED) - result = self._adapter().download_and_upload_avatar( - "https://cdn.example.com/a.png", user=MagicMock() - ) + result = self._adapter().download_and_upload_avatar("https://cdn.example.com/a.png", user=MagicMock()) assert result is None assert mock_fetch.call_args.args[0] == "GET" assert mock_fetch.call_args.args[1] == "https://cdn.example.com/a.png" diff --git a/apps/api/plane/tests/unit/bg_tasks/test_url_security.py b/apps/api/plane/tests/unit/bg_tasks/test_url_security.py index 3a1e8d3dba1..31b3a268499 100644 --- a/apps/api/plane/tests/unit/bg_tasks/test_url_security.py +++ b/apps/api/plane/tests/unit/bg_tasks/test_url_security.py @@ -186,9 +186,7 @@ def test_ipv6_validated_ip_is_bracketed(self, mock_resolve, mock_session_cls): @patch("plane.utils.url_security.resolve_and_validate") def test_blocked_target_raises_before_any_request(self, mock_resolve): - mock_resolve.side_effect = ValueError( - "Access to private/internal networks is not allowed" - ) + mock_resolve.side_effect = ValueError("Access to private/internal networks is not allowed") with pytest.raises(ValueError, match="private/internal"): pinned_fetch("POST", "https://attacker.com/hook") @@ -275,9 +273,7 @@ def test_blocks_redirect_to_private_ip(self, mock_resolve, mock_session_cls): ValueError("Access to private/internal networks is not allowed"), ] session = mock_session_cls.return_value - session.request.return_value = _resp( - 302, headers={"Location": "http://169.254.169.254/latest/meta-data/"} - ) + session.request.return_value = _resp(302, headers={"Location": "http://169.254.169.254/latest/meta-data/"}) with pytest.raises(ValueError, match="private/internal"): pinned_fetch_following_redirects("GET", "https://evil.com/r") @@ -286,13 +282,9 @@ def test_blocks_redirect_to_private_ip(self, mock_resolve, mock_session_cls): def test_too_many_redirects(self, mock_resolve, mock_session_cls): mock_resolve.return_value = ["93.184.216.34"] session = mock_session_cls.return_value - session.request.return_value = _resp( - 302, headers={"Location": "https://example.com/loop"} - ) + session.request.return_value = _resp(302, headers={"Location": "https://example.com/loop"}) with pytest.raises(requests.TooManyRedirects): - pinned_fetch_following_redirects( - "GET", "https://example.com/start", max_redirects=3 - ) + pinned_fetch_following_redirects("GET", "https://example.com/start", max_redirects=3) # --------------------------------------------------------------------------- diff --git a/apps/api/plane/tests/unit/bg_tasks/test_work_item_link_task.py b/apps/api/plane/tests/unit/bg_tasks/test_work_item_link_task.py index 2599126ff49..e29b66b7e1a 100644 --- a/apps/api/plane/tests/unit/bg_tasks/test_work_item_link_task.py +++ b/apps/api/plane/tests/unit/bg_tasks/test_work_item_link_task.py @@ -196,9 +196,7 @@ def test_blocks_redirect_to_private_ip(self, mock_resolve, mock_session_cls): ValueError("Access to private/internal networks is not allowed"), ] session = mock_session_cls.return_value - session.request.return_value = _make_response( - status_code=302, headers={"Location": "http://192.168.1.1:8080"} - ) + session.request.return_value = _make_response(status_code=302, headers={"Location": "http://192.168.1.1:8080"}) with pytest.raises(ValueError, match="private/internal"): safe_get("https://evil.com/redirect") @@ -208,9 +206,7 @@ def test_blocks_redirect_to_private_ip(self, mock_resolve, mock_session_cls): def test_raises_on_too_many_redirects(self, mock_resolve, mock_session_cls): mock_resolve.return_value = ["93.184.216.34"] session = mock_session_cls.return_value - session.request.return_value = _make_response( - status_code=302, headers={"Location": "https://example.com/loop"} - ) + session.request.return_value = _make_response(status_code=302, headers={"Location": "https://example.com/loop"}) with pytest.raises(requests.TooManyRedirects): safe_get("https://example.com/start") diff --git a/apps/api/plane/tests/unit/middleware/test_api_authentication.py b/apps/api/plane/tests/unit/middleware/test_api_authentication.py index 5f67537c228..9947437e276 100644 --- a/apps/api/plane/tests/unit/middleware/test_api_authentication.py +++ b/apps/api/plane/tests/unit/middleware/test_api_authentication.py @@ -23,9 +23,7 @@ class TestAPIKeyAuthentication: @pytest.mark.django_db def test_validate_api_token_authenticates_active_user(self, create_user): - token = APIToken.objects.create( - user=create_user, label="Active Token", token="active-user-token" - ) + token = APIToken.objects.create(user=create_user, label="Active Token", token="active-user-token") user, returned_token = APIKeyAuthentication().validate_api_token(token.token) @@ -34,9 +32,7 @@ def test_validate_api_token_authenticates_active_user(self, create_user): @pytest.mark.django_db def test_validate_api_token_rejects_deactivated_user(self, create_user): - token = APIToken.objects.create( - user=create_user, label="Stale Token", token="deactivated-user-token" - ) + token = APIToken.objects.create(user=create_user, label="Stale Token", token="deactivated-user-token") # Account is deactivated by an administrator after the token was issued. create_user.is_active = False diff --git a/apps/api/plane/tests/unit/middleware/test_logger.py b/apps/api/plane/tests/unit/middleware/test_logger.py index 5c13f53f6f7..31c4131530c 100644 --- a/apps/api/plane/tests/unit/middleware/test_logger.py +++ b/apps/api/plane/tests/unit/middleware/test_logger.py @@ -56,9 +56,7 @@ def _captured_log_data(self, middleware, request_factory): def test_token_identifier_is_hashed_not_plaintext(self, middleware, request_factory): log_data = self._captured_log_data(middleware, request_factory) - expected_hash = hmac.new( - settings.SECRET_KEY.encode(), self.API_KEY.encode(), hashlib.sha256 - ).hexdigest() + expected_hash = hmac.new(settings.SECRET_KEY.encode(), self.API_KEY.encode(), hashlib.sha256).hexdigest() assert log_data["token_identifier"] == expected_hash assert self.API_KEY not in log_data["token_identifier"] diff --git a/apps/api/plane/tests/unit/utils/test_order_by_sanitize.py b/apps/api/plane/tests/unit/utils/test_order_by_sanitize.py index 12fde9376bf..fa15c4c1c78 100644 --- a/apps/api/plane/tests/unit/utils/test_order_by_sanitize.py +++ b/apps/api/plane/tests/unit/utils/test_order_by_sanitize.py @@ -50,36 +50,24 @@ class TestProjectOrderBySanitization: def test_injection_payload_falls_back_to_default(self, payload): """Any non-allowlisted / relational value is replaced with the endpoint's safe default instead of reaching .order_by().""" - assert ( - sanitize_order_by(payload, PROJECT_ORDER_BY_ALLOWLIST, default=self.DEFAULT) - == self.DEFAULT - ) + assert sanitize_order_by(payload, PROJECT_ORDER_BY_ALLOWLIST, default=self.DEFAULT) == self.DEFAULT @pytest.mark.parametrize( "value", ["created_at", "updated_at", "name", "network", "sort_order"], ) def test_legitimate_ascending_values_pass_through(self, value): - assert ( - sanitize_order_by(value, PROJECT_ORDER_BY_ALLOWLIST, default=self.DEFAULT) - == value - ) + assert sanitize_order_by(value, PROJECT_ORDER_BY_ALLOWLIST, default=self.DEFAULT) == value @pytest.mark.parametrize( "value", ["-created_at", "-updated_at", "-name", "-network", "-sort_order"], ) def test_legitimate_descending_values_pass_through(self, value): - assert ( - sanitize_order_by(value, PROJECT_ORDER_BY_ALLOWLIST, default=self.DEFAULT) - == value - ) + assert sanitize_order_by(value, PROJECT_ORDER_BY_ALLOWLIST, default=self.DEFAULT) == value def test_empty_value_uses_default(self): - assert ( - sanitize_order_by("", PROJECT_ORDER_BY_ALLOWLIST, default=self.DEFAULT) - == self.DEFAULT - ) + assert sanitize_order_by("", PROJECT_ORDER_BY_ALLOWLIST, default=self.DEFAULT) == self.DEFAULT @pytest.mark.unit @@ -91,10 +79,7 @@ class TestIssueOrderBySanitization: @pytest.mark.parametrize("payload", INJECTION_PAYLOADS) def test_injection_payload_falls_back_to_default(self, payload): - assert ( - sanitize_order_by(payload, ISSUE_ORDER_BY_ALLOWLIST, default=self.DEFAULT) - == self.DEFAULT - ) + assert sanitize_order_by(payload, ISSUE_ORDER_BY_ALLOWLIST, default=self.DEFAULT) == self.DEFAULT @pytest.mark.parametrize( "value", @@ -115,18 +100,9 @@ def test_injection_payload_falls_back_to_default(self, payload): def test_legitimate_values_pass_through(self, value): """Every value the endpoint's branch logic special-cases must survive sanitization, otherwise legitimate ordering would silently break.""" - assert ( - sanitize_order_by(value, ISSUE_ORDER_BY_ALLOWLIST, default=self.DEFAULT) - == value - ) + assert sanitize_order_by(value, ISSUE_ORDER_BY_ALLOWLIST, default=self.DEFAULT) == value # Descending variant is equally valid. - assert ( - sanitize_order_by(f"-{value}", ISSUE_ORDER_BY_ALLOWLIST, default=self.DEFAULT) - == f"-{value}" - ) + assert sanitize_order_by(f"-{value}", ISSUE_ORDER_BY_ALLOWLIST, default=self.DEFAULT) == f"-{value}" def test_default_is_preserved_for_missing_param(self): - assert ( - sanitize_order_by(None, ISSUE_ORDER_BY_ALLOWLIST, default=self.DEFAULT) - == self.DEFAULT - ) + assert sanitize_order_by(None, ISSUE_ORDER_BY_ALLOWLIST, default=self.DEFAULT) == self.DEFAULT diff --git a/apps/api/plane/tests/unit/utils/test_search.py b/apps/api/plane/tests/unit/utils/test_search.py index 8663467a19e..0e4360c33f6 100644 --- a/apps/api/plane/tests/unit/utils/test_search.py +++ b/apps/api/plane/tests/unit/utils/test_search.py @@ -34,10 +34,10 @@ def test_empty_query_matches_nothing(self): assert build_search_query(" ", fields=["name"]) == Q() def test_single_token_ors_across_fields(self): - q = build_search_query("sage", fields=["name", "description_stripped"]) + q = build_search_query("northwind", fields=["name", "description_stripped"]) assert _children(q) == { - ("name__icontains", "sage"), - ("description_stripped__icontains", "sage"), + ("name__icontains", "northwind"), + ("description_stripped__icontains", "northwind"), } assert q.connector == Q.OR @@ -83,7 +83,7 @@ def test_sequence_match_is_ored_onto_the_whole_predicate(self): def test_identifier_and_number_in_one_token_still_matches_by_number(self): q = build_search_query( - "DUROPC-22", + "PAY-22", fields=ISSUE_SEARCH_FIELDS, sequence_fields=ISSUE_SEQUENCE_FIELDS, ) @@ -117,6 +117,15 @@ def test_decimals_do_not_produce_sequence_matches(self): assert ("sequence_id", "3") not in _children(q) assert ("sequence_id", "5") not in _children(q) + def test_leading_dot_decimals_do_not_produce_sequence_matches(self): + """ ".5" is a decimal, not work item number 5.""" + q = build_search_query( + ".5", + fields=ISSUE_SEARCH_FIELDS, + sequence_fields=ISSUE_SEQUENCE_FIELDS, + ) + assert ("sequence_id", "5") not in _children(q) + def test_version_strings_do_not_produce_sequence_matches(self): q = build_search_query( "v1.4.0", @@ -161,11 +170,11 @@ def test_pages_search_their_description(self): def test_a_word_only_in_the_body_is_matchable(self): q = build_search_query( - "sage", + "northwind", fields=ISSUE_SEARCH_FIELDS, sequence_fields=ISSUE_SEQUENCE_FIELDS, ) - assert ("description_stripped__icontains", "sage") in _children(q) + assert ("description_stripped__icontains", "northwind") in _children(q) def test_words_split_across_title_and_body_still_match(self): """ "payment" and "gateway" from the title, "review" from the body.""" diff --git a/apps/api/plane/utils/search.py b/apps/api/plane/utils/search.py index 9373183442a..9aa4807673b 100644 --- a/apps/api/plane/utils/search.py +++ b/apps/api/plane/utils/search.py @@ -11,9 +11,10 @@ # Match whole integers only. The lookaround excludes the components of a # decimal: a plain \b\d+\b treats the dot in "3.5" as a word boundary and # yields both 3 and 5, so searching a version string surfaced unrelated issues -# by sequence id. A trailing dot that is not followed by a digit ("issue 22.") -# is still sentence punctuation, and 22 stays matchable. -SEQUENCE_PATTERN = re.compile(r"(? Date: Sat, 15 Aug 2026 09:49:18 -0400 Subject: [PATCH 5/5] fix(api): bound the number of tokens taken from a search query MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tokenizing removed a property the old predicate had for free: a single icontains over the whole query is constant size no matter how long the query is. One predicate per token per field is not — a request carrying thousands of whitespace-separated tokens builds an arbitrarily large SQL expression, which is a cost this change introduced. Cap at the first MAX_SEARCH_TOKENS (12). Dropping the tail is safe because tokens are AND-ed: the retained ones have already narrowed the result set at least as much as the full query would have. Capped rather than rejected with a validation error, because build_search_query is a pure predicate builder shared by twenty call sites across three endpoints; raising would push error handling into all of them to punish a query nobody types deliberately. Happy to convert it to a 400 at the view layer instead if that is preferred. --- .../api/plane/tests/unit/utils/test_search.py | 34 +++++++++++++++++++ apps/api/plane/utils/search.py | 17 +++++++++- 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/apps/api/plane/tests/unit/utils/test_search.py b/apps/api/plane/tests/unit/utils/test_search.py index 0e4360c33f6..eda71138a2f 100644 --- a/apps/api/plane/tests/unit/utils/test_search.py +++ b/apps/api/plane/tests/unit/utils/test_search.py @@ -8,6 +8,7 @@ from plane.utils.search import ( ISSUE_SEARCH_FIELDS, ISSUE_SEQUENCE_FIELDS, + MAX_SEARCH_TOKENS, PAGE_SEARCH_FIELDS, build_search_query, ) @@ -158,6 +159,39 @@ def test_no_sequence_fields_yields_no_sequence_leaves(self): assert _children(q) == {("name__icontains", "22")} +@pytest.mark.unit +class TestTokenBudget: + """One predicate per token per field, so the token count has to be bounded. + + The predicate this replaced was a single icontains over the whole query — + constant size no matter how long the query was. Tokenizing removes that + property, so a request could otherwise build arbitrarily large SQL. + """ + + def test_predicate_size_is_proportional_to_tokens(self): + fields = ["name", "description_stripped"] + q = build_search_query("alpha beta gamma", fields=fields) + assert len(_children(q)) == 3 * len(fields) + + def test_tokens_at_the_limit_are_all_used(self): + fields = ["name"] + tokens = [f"t{i}" for i in range(MAX_SEARCH_TOKENS)] + q = build_search_query(" ".join(tokens), fields=fields) + assert len(_children(q)) == MAX_SEARCH_TOKENS + + def test_tokens_beyond_the_limit_are_dropped(self): + fields = ["name"] + tokens = [f"t{i}" for i in range(MAX_SEARCH_TOKENS + 50)] + q = build_search_query(" ".join(tokens), fields=fields) + assert len(_children(q)) == MAX_SEARCH_TOKENS + + def test_a_pathological_query_stays_bounded(self): + """The case the bound exists for: thousands of tokens, many fields.""" + fields = ISSUE_SEARCH_FIELDS + q = build_search_query(" ".join(str(i) for i in range(5000)), fields=fields) + assert len(_children(q)) <= MAX_SEARCH_TOKENS * len(fields) + + @pytest.mark.unit class TestSearchableFields: """Bodies are searchable, not just titles.""" diff --git a/apps/api/plane/utils/search.py b/apps/api/plane/utils/search.py index 9aa4807673b..17ef894926d 100644 --- a/apps/api/plane/utils/search.py +++ b/apps/api/plane/utils/search.py @@ -16,6 +16,12 @@ # sentence punctuation, and 22 stays matchable. SEQUENCE_PATTERN = re.compile(r"(?