diff --git a/nextcloud_mcp_server/api/vector_sync.py b/nextcloud_mcp_server/api/vector_sync.py index 58a1144e..f70cc00b 100644 --- a/nextcloud_mcp_server/api/vector_sync.py +++ b/nextcloud_mcp_server/api/vector_sync.py @@ -32,6 +32,11 @@ from ..http import nextcloud_httpx_client logger = logging.getLogger(__name__) +# Upper bound on doc_types per purge request. There are only a handful of real +# indexed types; this caps a hostile/buggy caller's fan-out of count+delete +# calls without constraining legitimate use. +_MAX_PURGE_DOC_TYPES = 64 + async def purge_doc_types_route(request: Request) -> JSONResponse: """POST /api/v1/vector-sync/purge — delete indexed vectors by doc type. @@ -83,6 +88,16 @@ async def purge_doc_types_route(request: Request) -> JSONResponse: doc_types = [d for d in raw if d] if not doc_types: return JSONResponse({"purged": {}}) + # Bound the batch: there are only a handful of real indexed types, so a huge + # list is abuse — cap it rather than fan out unbounded count+delete calls. + if len(doc_types) > _MAX_PURGE_DOC_TYPES: + return JSONResponse( + { + "error": "Bad request", + "message": f"doc_types exceeds the maximum of {_MAX_PURGE_DOC_TYPES}", + }, + status_code=400, + ) try: username, app_password = await get_basic_auth_for_user(user_id) diff --git a/nextcloud_mcp_server/server/semantic.py b/nextcloud_mcp_server/server/semantic.py index eec7bd41..07225aae 100644 --- a/nextcloud_mcp_server/server/semantic.py +++ b/nextcloud_mcp_server/server/semantic.py @@ -57,6 +57,23 @@ logger = logging.getLogger(__name__) _USAGE_METADATA_MAX_DOC_TYPES = 16 +def _consent_narrowed_doc_types( + doc_types: list[str] | None, allowed: frozenset[str] +) -> list[str]: + """Apply the admin allow-set to a requested ``doc_types`` filter. + + Caller has already established ``allowed is not None`` (a concrete allow-set; + ``None`` means "no restriction" and is handled by skipping this call). When + no explicit ``doc_types`` are requested, restrict to the full allow-set; + otherwise intersect (preserving the caller's order). An empty result means + nothing the caller asked for is admin-approved — the caller short-circuits + to an empty response rather than falling through to an all-types search. + """ + if doc_types is None: + return sorted(allowed) + return [dt for dt in doc_types if dt in allowed] + + async def record_search_usage( *, enabled: bool, @@ -309,10 +326,7 @@ def configure_semantic_tools(mcp: FastMCP): # means the admin disabled every source. allowed = await allowed_doc_types(client, username) if allowed is not None: - if doc_types is None: - doc_types = sorted(allowed) - else: - doc_types = [dt for dt in doc_types if dt in allowed] + doc_types = _consent_narrowed_doc_types(doc_types, allowed) if not doc_types: logger.info( "Semantic search short-circuited for user %s: no requested " diff --git a/nextcloud_mcp_server/vector/scanner.py b/nextcloud_mcp_server/vector/scanner.py index 537ab94e..f55c0d1b 100644 --- a/nextcloud_mcp_server/vector/scanner.py +++ b/nextcloud_mcp_server/vector/scanner.py @@ -282,9 +282,16 @@ def _app_enabled(app_id: str, enabled_apps: set[str] | None) -> bool: # Text doc types whose deletion-tracking lives *inside* their scan_* function, # so skipping that function (when admin-disabled) leaves indexed points with no -# grace-period backstop. ``file`` is intentionally excluded: its scan path -# empties discovery and lets the existing reconcile loop purge on disable. -_TEXT_BACKSTOP_DOC_TYPES: tuple[str, ...] = ("note", "news_item", "deck_card") +# grace-period backstop. Derived from INDEXED_DOC_TYPES so a newly-indexed type +# automatically gets the backstop. ``file`` is excluded: its scan path empties +# discovery and lets the existing reconcile loop purge on disable. +_TEXT_BACKSTOP_DOC_TYPES: tuple[str, ...] = tuple(sorted(INDEXED_DOC_TYPES - {"file"})) + +# Per-process record of (user_id, doc_type) whose consent backstop deletes have +# already been enqueued, so a *standing* admin-disable doesn't re-flood the +# processor with idempotent deletes on every scan tick. An entry is cleared once +# the type is allowed again, so a later re-disable re-triggers the backstop. +_consent_backstop_done: set[tuple[str, str]] = set() async def _enqueue_deletes_for_disabled_types( @@ -303,7 +310,18 @@ async def _enqueue_deletes_for_disabled_types( if allowed is None: return 0 - disabled = [dt for dt in _TEXT_BACKSTOP_DOC_TYPES if dt not in allowed] + # Re-enabled types: clear their one-shot marker so a later re-disable + # re-triggers the backstop. + for doc_type in _TEXT_BACKSTOP_DOC_TYPES: + if doc_type in allowed: + _consent_backstop_done.discard((user_id, doc_type)) + + # Disabled types not yet backstopped this episode. + disabled = [ + dt + for dt in _TEXT_BACKSTOP_DOC_TYPES + if dt not in allowed and (user_id, dt) not in _consent_backstop_done + ] if not disabled: return 0 @@ -346,6 +364,9 @@ async def _enqueue_deletes_for_disabled_types( ) ) queued += 1 + # Mark this (user, doc_type) backstopped for the current disable episode + # so subsequent scans don't re-enqueue the same idempotent deletes. + _consent_backstop_done.add((user_id, doc_type)) return queued diff --git a/tests/unit/server/test_semantic_consent_narrowing.py b/tests/unit/server/test_semantic_consent_narrowing.py new file mode 100644 index 00000000..534f84ba --- /dev/null +++ b/tests/unit/server/test_semantic_consent_narrowing.py @@ -0,0 +1,37 @@ +"""Unit tests for the search-side consent narrowing in nc_semantic_search. + +The narrowing logic (intersect requested doc_types with the admin allow-set, +or restrict to the allow-set when none requested) is extracted into +``_consent_narrowed_doc_types`` so it can be tested without exercising the full +search path. ``allowed is None`` (no restriction / fail-open) is handled by the +caller skipping this helper entirely. +""" + +from __future__ import annotations + +from nextcloud_mcp_server.server.semantic import _consent_narrowed_doc_types + + +def test_none_request_restricts_to_allow_set(): + # No explicit doc_types -> search exactly the allowed set (sorted). + assert _consent_narrowed_doc_types(None, frozenset({"file", "note"})) == [ + "file", + "note", + ] + + +def test_request_intersected_with_allow_set_preserves_order(): + result = _consent_narrowed_doc_types( + ["deck_card", "note", "file"], frozenset({"note", "file"}) + ) + assert result == ["note", "file"] + + +def test_disjoint_request_yields_empty(): + # Caller short-circuits to an empty response on []. + assert _consent_narrowed_doc_types(["deck_card"], frozenset({"note"})) == [] + + +def test_empty_allow_set_blocks_all(): + assert _consent_narrowed_doc_types(None, frozenset()) == [] + assert _consent_narrowed_doc_types(["note"], frozenset()) == [] diff --git a/tests/unit/test_vector_sync_purge_route.py b/tests/unit/test_vector_sync_purge_route.py index df4e0bb4..5b4e92aa 100644 --- a/tests/unit/test_vector_sync_purge_route.py +++ b/tests/unit/test_vector_sync_purge_route.py @@ -151,6 +151,20 @@ def test_bad_request_when_body_not_object(mocker): purge.assert_not_called() +def test_bad_request_when_too_many_doc_types(mocker): + _patch_token(mocker) + purge = _patch_purge(mocker) + + client = TestClient(_build_app()) + resp = client.post( + "/api/v1/vector-sync/purge", + json={"doc_types": [f"t{i}" for i in range(65)]}, + ) + + assert resp.status_code == 400 + purge.assert_not_called() + + def test_provisioning_required_returns_428(mocker): _patch_token(mocker, "admin") mocker.patch( diff --git a/tests/unit/vector/test_scanner_consent_backstop.py b/tests/unit/vector/test_scanner_consent_backstop.py index 2a71be30..2ae1e127 100644 --- a/tests/unit/vector/test_scanner_consent_backstop.py +++ b/tests/unit/vector/test_scanner_consent_backstop.py @@ -12,11 +12,21 @@ from types import SimpleNamespace from typing import cast from unittest.mock import AsyncMock +import pytest + from nextcloud_mcp_server.vector import scanner as scanner_module from nextcloud_mcp_server.vector.queue.ports import TaskProducer from nextcloud_mcp_server.vector.scanner import _enqueue_deletes_for_disabled_types +@pytest.fixture(autouse=True) +def _clear_backstop_state(): + """The one-shot guard is module-level; reset it between tests.""" + scanner_module._consent_backstop_done.clear() + yield + scanner_module._consent_backstop_done.clear() + + def _producer(send: AsyncMock) -> TaskProducer: """A minimal stand-in for the TaskProducer protocol (only ``send`` is used).""" return cast(TaskProducer, SimpleNamespace(send=send)) @@ -79,3 +89,42 @@ async def test_noop_when_all_text_types_allowed(monkeypatch): ) assert queued == 0 send.assert_not_called() + + +async def test_one_shot_does_not_reflood_on_subsequent_scans(monkeypatch): + _patch_qdrant(monkeypatch, {"note": ["n1", "n2"]}) + send = AsyncMock() + allowed = frozenset({"file"}) # note disabled + + first = await _enqueue_deletes_for_disabled_types( + "alice", _producer(send), allowed, 1 + ) + second = await _enqueue_deletes_for_disabled_types( + "alice", _producer(send), allowed, 2 + ) + + assert first == 2 + # Standing disable: the next scan must not re-enqueue the same deletes. + assert second == 0 + + +async def test_re_enable_then_disable_retriggers_backstop(monkeypatch): + _patch_qdrant(monkeypatch, {"note": ["n1"]}) + send = AsyncMock() + disabled = frozenset({"file"}) + enabled = frozenset({"file", "note"}) + + assert ( + await _enqueue_deletes_for_disabled_types("alice", _producer(send), disabled, 1) + == 1 + ) + # Re-enabled: clears the one-shot marker. + assert ( + await _enqueue_deletes_for_disabled_types("alice", _producer(send), enabled, 2) + == 0 + ) + # Disabled again: backstop fires once more. + assert ( + await _enqueue_deletes_for_disabled_types("alice", _producer(send), disabled, 3) + == 1 + )