diff --git a/nextcloud_mcp_server/search/access_filter.py b/nextcloud_mcp_server/search/access_filter.py index 7f75f326..5fcccf79 100644 --- a/nextcloud_mcp_server/search/access_filter.py +++ b/nextcloud_mcp_server/search/access_filter.py @@ -44,6 +44,12 @@ from nextcloud_mcp_server.vector.placeholder import get_placeholder_filter logger = logging.getLogger(__name__) +# Upper bound on the number of folder filters a single search may apply. Caps +# the width of the ``Filter(should=[...])`` OR-clause built from path_prefixes +# so no caller can degrade query latency with a huge folder list. Mirrored by +# the ``Field(max_length=...)`` on the MCP tool and the Astrolabe PHP cap. +MAX_PATH_PREFIXES = 20 + # Short-lived per-user cache for the OCS shares lookup, which otherwise runs on # every search/viz request. Trades up to this many seconds of share-visibility # staleness (a freshly-granted share is searchable a little late) for avoiding @@ -195,7 +201,7 @@ def normalize_path_prefixes( path_prefixes: Iterable[str] | None = None, ) -> list[str]: """Merge the legacy single ``path_prefix`` and list ``path_prefixes`` into - one clean, de-duplicated list of folder filters. + one clean, de-duplicated, bounded list of folder filters. Blank/whitespace entries are dropped (an empty UI field must mean "no filter", not "match everything"), surrounding whitespace is stripped, and @@ -203,13 +209,21 @@ def normalize_path_prefixes( the pre-ADR-027-Phase-2 single-value contract working while callers migrate to the multi-folder list. + The result is capped at ``MAX_PATH_PREFIXES`` so no caller — the MCP tool, + the REST/viz endpoints, or a misbehaving client — can build an unbounded + ``Filter(should=[...])`` OR-clause that would widen every Qdrant query. + This is the single server-side enforcement point (the MCP tool also + declares ``Field(max_length=...)`` for an earlier, clearer validation + error, and the Astrolabe PHP controller caps before forwarding). + Args: path_prefix: Legacy single folder filter (deprecated; folded into the returned list). path_prefixes: Zero or more folder filters. Returns: - Ordered, de-duplicated list of non-empty folder filters (possibly empty). + Ordered, de-duplicated list of non-empty folder filters, capped at + ``MAX_PATH_PREFIXES`` (possibly empty). """ raw: list[str] = [] if path_prefix: @@ -227,7 +241,7 @@ def normalize_path_prefixes( if stripped and stripped not in seen: seen.add(stripped) cleaned.append(stripped) - return cleaned + return cleaned[:MAX_PATH_PREFIXES] def build_base_filter_conditions( diff --git a/nextcloud_mcp_server/search/algorithms.py b/nextcloud_mcp_server/search/algorithms.py index ad0546a7..7833b3e8 100644 --- a/nextcloud_mcp_server/search/algorithms.py +++ b/nextcloud_mcp_server/search/algorithms.py @@ -2,6 +2,7 @@ import logging from abc import ABC, abstractmethod +from collections.abc import Iterable from dataclasses import dataclass from typing import Any, Protocol, runtime_checkable @@ -300,7 +301,7 @@ class SearchAlgorithm(ABC): modified_after: int | None = None, modified_before: int | None = None, path_prefix: str | None = None, - path_prefixes: list[str] | None = None, + path_prefixes: Iterable[str] | None = None, **kwargs: Any, ) -> list[SearchResult]: """Execute search with the given parameters. diff --git a/nextcloud_mcp_server/search/bm25_hybrid.py b/nextcloud_mcp_server/search/bm25_hybrid.py index 4a5b0597..fc1ce41e 100644 --- a/nextcloud_mcp_server/search/bm25_hybrid.py +++ b/nextcloud_mcp_server/search/bm25_hybrid.py @@ -1,6 +1,7 @@ """BM25 hybrid search algorithm using Qdrant native RRF fusion.""" import logging +from collections.abc import Iterable from typing import Any from qdrant_client import models @@ -75,7 +76,7 @@ class BM25HybridSearchAlgorithm(SearchAlgorithm): modified_after: int | None = None, modified_before: int | None = None, path_prefix: str | None = None, - path_prefixes: list[str] | None = None, + path_prefixes: Iterable[str] | None = None, **kwargs: Any, ) -> list[SearchResult]: """ diff --git a/nextcloud_mcp_server/search/semantic.py b/nextcloud_mcp_server/search/semantic.py index e0085b2a..ff516688 100644 --- a/nextcloud_mcp_server/search/semantic.py +++ b/nextcloud_mcp_server/search/semantic.py @@ -1,6 +1,7 @@ """Semantic search algorithm using vector similarity (Qdrant).""" import logging +from collections.abc import Iterable from typing import Any from qdrant_client.models import FieldCondition, Filter, MatchAny @@ -55,7 +56,7 @@ class SemanticSearchAlgorithm(SearchAlgorithm): modified_after: int | None = None, modified_before: int | None = None, path_prefix: str | None = None, - path_prefixes: list[str] | None = None, + path_prefixes: Iterable[str] | None = None, **kwargs: Any, ) -> list[SearchResult]: """Execute semantic search using vector similarity. diff --git a/nextcloud_mcp_server/server/semantic.py b/nextcloud_mcp_server/server/semantic.py index 371f96cb..6104740c 100644 --- a/nextcloud_mcp_server/server/semantic.py +++ b/nextcloud_mcp_server/server/semantic.py @@ -33,6 +33,7 @@ from nextcloud_mcp_server.observability.metrics import ( instrument_tool, ) from nextcloud_mcp_server.search.access_filter import ( + MAX_PATH_PREFIXES, list_accessible_owners, normalize_path_prefixes, ) @@ -102,14 +103,14 @@ def configure_semantic_tools(mcp: FastMCP): path_prefixes: Annotated[ list[str] | None, Field( - max_length=20, + max_length=MAX_PATH_PREFIXES, description=( "Restrict to files under any of these folders/paths " "(e.g. ['/Projects/Reports', '/Shared/Specs']). Folders are " "OR-ed together. Matches the file_path of indexed files " "only, so setting it implicitly limits results to files. " - "Capped at 20 folders to bound the OR-filter width. " - "None or empty = no path filter." + f"Capped at {MAX_PATH_PREFIXES} folders to bound the " + "OR-filter width. None or empty = no path filter." ), ), ] = None, diff --git a/tests/unit/search/test_access_filter.py b/tests/unit/search/test_access_filter.py index 6c9faa4c..830501c3 100644 --- a/tests/unit/search/test_access_filter.py +++ b/tests/unit/search/test_access_filter.py @@ -9,6 +9,7 @@ from qdrant_client.models import FieldCondition, Filter, MatchText, Range from nextcloud_mcp_server.search import access_filter from nextcloud_mcp_server.search.access_filter import ( + MAX_PATH_PREFIXES, build_base_filter_conditions, build_ownership_filter, clear_accessible_owners_cache, @@ -377,3 +378,12 @@ class TestNormalizePathPrefixes: " /Projects ", ["/Archive", "/Projects", " ", "/Specs"] ) assert result == ["/Projects", "/Archive", "/Specs"] + + @pytest.mark.unit + def test_caps_at_max_path_prefixes(self) -> None: + # A huge list is truncated to MAX_PATH_PREFIXES so no caller can build + # an unbounded OR-clause; the first N (order-preserving) survive. + folders = [f"/dir{i}" for i in range(MAX_PATH_PREFIXES + 30)] + result = normalize_path_prefixes(None, folders) + assert len(result) == MAX_PATH_PREFIXES + assert result == folders[:MAX_PATH_PREFIXES]