Merge pull request #873 from cbcoutinho/fix/scanner-gate-enabled-apps

fix(vector): gate scanner app polls on per-user enabled apps

nextcloud_mcp_server/client/__init__.py

@@ -199,6 +199,46 @@ class NextcloudClient:
 
         return response.json()
 
+    async def get_enabled_apps(self) -> set[str]:
+        """Return the set of app ids enabled for the authenticated user.
+
+        Uses the per-user core navigation endpoint, which lists only apps the
+        current user can access (respecting group restrictions). The vector
+        scanner uses this to skip polling apps the user lacks, which would 404
+        and flood tenant logs. Preferred over ``/cloud/capabilities`` because
+        the News app advertises no capability and so never appears there.
+        """
+        response = await self._client.get(
+            "/ocs/v2.php/core/navigation/apps",
+            headers={"OCS-APIRequest": "true", "Accept": "application/json"},
+        )
+        response.raise_for_status()
+        data = response.json()
+        # ``X or {}``/``or []`` (not ``.get(k, default)``) so a present-but-null
+        # ``ocs``/``data`` (``{"ocs": null}``) coerces to empty instead of
+        # raising AttributeError on ``None.get``.
+        ocs = data.get("ocs") or {}
+        # A 200 carrying ``meta.status != "ok"`` is an OCS-level failure (auth /
+        # permission edge cases) that ``raise_for_status`` can't see. Raise so
+        # the scanner's ``_get_enabled_apps_or_none`` catches it and falls back
+        # to scanning every app, rather than silently gating all apps off for a
+        # cycle on an empty ``data``. Tolerate a missing/empty meta (our own
+        # mocks, and any envelope that omits it).
+        status = (ocs.get("meta") or {}).get("status")
+        if status and status != "ok":  # falsy (missing/None/"") tolerated
+            raise ValueError(f"OCS navigation returned status={status!r}")
+        entries = ocs.get("data") or []
+        enabled: set[str] = set()
+        for entry in entries:
+            # ``app`` is the canonical app id; ``id`` matches it for the apps we
+            # gate. Union both so an unexpected nav-entry shape never hides an
+            # enabled app — a false "disabled" would skip real indexing.
+            for key in ("app", "id"):
+                value = entry.get(key)
+                if value:
+                    enabled.add(value)
+        return enabled
+
     async def notes_search_notes(self, *, query: str):
         """Search notes using token-based matching with relevance ranking."""
         all_notes = self.notes.get_all_notes()

nextcloud_mcp_server/vector/scanner.py

@@ -247,6 +247,38 @@ async def scanner_task(
     logger.info("Scanner task stopped - stream closed")
 
 
+async def _get_enabled_apps_or_none(
+    nc_client: NextcloudClient, user_id: str, scan_id: int
+) -> set[str] | None:
+    """Enabled-app id set for gating, or ``None`` when detection fails.
+
+    ``None`` signals "couldn't determine" — callers must then scan every app
+    (the prior behaviour), so a transient navigation-endpoint failure never
+    silently halts indexing. The per-app 404 guards in ``scan_user_documents``
+    remain the safety net for that fallback path.
+    """
+    try:
+        return await nc_client.get_enabled_apps()
+    except Exception as e:
+        logger.warning(
+            "[SCAN-%s] Could not determine enabled apps for %s (%s); scanning all apps",
+            scan_id,
+            user_id,
+            e,
+        )
+        return None
+
+
+def _app_enabled(app_id: str, enabled_apps: set[str] | None) -> bool:
+    """Whether ``app_id`` should be scanned for the current user.
+
+    ``enabled_apps is None`` means detection failed — every app is treated as
+    enabled (the scan-all fallback) so a transient navigation-endpoint failure
+    never silently halts indexing.
+    """
+    return enabled_apps is None or app_id in enabled_apps
+
+
 async def scan_user_documents(
     user_id: str,
     send_stream: TaskProducer,
@@ -328,6 +360,11 @@ async def scan_user_documents(
 
             logger.debug("Found %s indexed documents in Qdrant", len(indexed_doc_ids))
 
+        # Determine which apps are enabled for this user so we skip polling
+        # apps they lack — those polls 404 and flood tenant logs. ``None`` means
+        # detection failed: fall back to scanning every app (prior behaviour).
+        enabled_apps = await _get_enabled_apps_or_none(nc_client, user_id, scan_id)
+
         # Notes (isolated so an uninstalled or disabled Notes app — whose API
         # returns 404 — cannot abort scanning of the other apps; this mirrors the
         # per-app try/except guards already wrapping files/news/deck below).
@@ -336,6 +373,7 @@ async def scan_user_documents(
         current_time = time.time()
         queued = 0
 
+        if _app_enabled("notes", enabled_apps):
             try:
                 queued += await scan_notes(
                     user_id=user_id,
@@ -359,6 +397,12 @@ async def scan_user_documents(
                     logger.warning("Failed to scan notes for %s: %s", user_id, e)
             except Exception as e:
                 logger.warning("Failed to scan notes for %s: %s", user_id, e)
+        else:
+            logger.debug(
+                "[SCAN-%s] Notes app not enabled for %s; skipping notes",
+                scan_id,
+                user_id,
+            )
 
         if initial_sync:
             logger.info("Sent %s documents for initial sync: %s", queued, user_id)
@@ -666,6 +710,7 @@ async def scan_user_documents(
 
         # Scan News items (starred + unread)
         news_queued = 0
+        if _app_enabled("news", enabled_apps):
             try:
                 news_queued = await scan_news_items(
                     user_id=user_id,
@@ -677,9 +722,16 @@ async def scan_user_documents(
                 queued += news_queued
             except Exception as e:
                 logger.warning("Failed to scan news items for %s: %s", user_id, e)
+        else:
+            logger.debug(
+                "[SCAN-%s] News app not enabled for %s; skipping news items",
+                scan_id,
+                user_id,
+            )
 
         # Scan Deck cards
         deck_queued = 0
+        if _app_enabled("deck", enabled_apps):
             try:
                 deck_queued = await scan_deck_cards(
                     user_id=user_id,
@@ -691,6 +743,12 @@ async def scan_user_documents(
                 queued += deck_queued
             except Exception as e:
                 logger.warning("Failed to scan deck cards for %s: %s", user_id, e)
+        else:
+            logger.debug(
+                "[SCAN-%s] Deck app not enabled for %s; skipping deck cards",
+                scan_id,
+                user_id,
+            )
 
         if queued > 0:
             logger.info(

tests/unit/client/test_nextcloud_client.py

@@ -7,7 +7,7 @@ folders) into a flat list of files.
 """
 
 from typing import Any
-from unittest.mock import AsyncMock
+from unittest.mock import AsyncMock, MagicMock
 
 import pytest
 
@@ -31,6 +31,103 @@ def _make_client() -> Any:
 pytestmark = pytest.mark.unit
 
 
+def _navigation_response(entries: list[dict]) -> MagicMock:
+    """Build a mocked OCS v2 ``core/navigation/apps`` response."""
+    response = MagicMock()
+    response.status_code = 200
+    response.raise_for_status = MagicMock()
+    response.json.return_value = {"ocs": {"meta": {}, "data": entries}}
+    return response
+
+
+class TestGetEnabledApps:
+    async def test_returns_app_ids_from_navigation(self):
+        client = _make_client()
+        client._client = AsyncMock()
+        client._client.get = AsyncMock(
+            return_value=_navigation_response(
+                [
+                    {"id": "files", "app": "files"},
+                    {"id": "notes", "app": "notes"},
+                    {"id": "deck", "app": "deck"},
+                    {"id": "news", "app": "news"},
+                ]
+            )
+        )
+
+        apps = await client.get_enabled_apps()
+
+        assert apps == {"files", "notes", "deck", "news"}
+        # Hits the per-user navigation endpoint, not capabilities.
+        assert (
+            client._client.get.await_args.args[0] == "/ocs/v2.php/core/navigation/apps"
+        )
+
+    async def test_unions_id_and_app_keys(self):
+        """When ``id`` and ``app`` differ, both are collected so an enabled
+        app is never hidden by an unexpected nav-entry id."""
+        client = _make_client()
+        client._client = AsyncMock()
+        client._client.get = AsyncMock(
+            return_value=_navigation_response([{"id": "files_sharing", "app": "files"}])
+        )
+
+        apps = await client.get_enabled_apps()
+
+        assert apps == {"files", "files_sharing"}
+
+    async def test_empty_navigation_returns_empty_set(self):
+        client = _make_client()
+        client._client = AsyncMock()
+        client._client.get = AsyncMock(return_value=_navigation_response([]))
+
+        assert await client.get_enabled_apps() == set()
+
+    async def test_skips_entries_missing_both_keys(self):
+        client = _make_client()
+        client._client = AsyncMock()
+        client._client.get = AsyncMock(
+            return_value=_navigation_response(
+                [{"name": "Logout", "href": "/logout"}, {"app": "notes"}]
+            )
+        )
+
+        assert await client.get_enabled_apps() == {"notes"}
+
+    @pytest.mark.parametrize("body", [{}, {"ocs": None}, {"ocs": {"data": None}}])
+    async def test_malformed_envelope_returns_empty_set(self, body):
+        """A missing/null ``ocs``/``data`` envelope yields an empty set rather
+        than raising. NOTE: an empty set does NOT trigger the
+        ``_get_enabled_apps_or_none`` scan-all fallback (that fires only on
+        exceptions) — all optional apps are gated off for this scan cycle, with
+        Files unaffected (unconditional) and the next cycle retrying normally."""
+        client = _make_client()
+        response = MagicMock()
+        response.raise_for_status = MagicMock()
+        response.json.return_value = body
+        client._client = AsyncMock()
+        client._client.get = AsyncMock(return_value=response)
+
+        assert await client.get_enabled_apps() == set()
+
+    async def test_ocs_failure_status_raises(self):
+        """A 200 with ``ocs.meta.status == "failure"`` raises so the scanner's
+        ``_get_enabled_apps_or_none`` falls back to scanning all apps, instead
+        of silently gating every app off on the empty ``data`` of a failure
+        envelope."""
+        client = _make_client()
+        response = MagicMock()
+        response.raise_for_status = MagicMock()
+        response.json.return_value = {
+            "ocs": {"meta": {"status": "failure", "statuscode": 997}, "data": None}
+        }
+        client._client = AsyncMock()
+        client._client.get = AsyncMock(return_value=response)
+
+        with pytest.raises(ValueError, match="failure"):
+            await client.get_enabled_apps()
+
+
 class TestNormaliseSearchResult:
     def test_adds_leading_slash_to_path(self):
         result = _normalise_search_result(

tests/unit/vector/test_scanner_app_gating.py

@@ -0,0 +1,75 @@
+"""Unit tests for the vector scanner's enabled-app gating helpers.
+
+``scan_user_documents`` skips polling apps the user doesn't have enabled (those
+polls 404 and flood tenant logs). ``_get_enabled_apps_or_none`` resolves the
+enabled-app set, returning ``None`` on any failure so the caller falls back to
+scanning every app (the prior behaviour) rather than silently halting indexing;
+``_app_enabled`` is the gate predicate applied per app.
+"""
+
+import logging
+from unittest.mock import AsyncMock
+
+import pytest
+from httpx import HTTPStatusError, Request, Response
+
+from nextcloud_mcp_server.vector.scanner import (
+    _app_enabled,
+    _get_enabled_apps_or_none,
+)
+
+pytestmark = pytest.mark.unit
+
+
+async def test_returns_enabled_set_on_success():
+    nc_client = AsyncMock()
+    nc_client.get_enabled_apps = AsyncMock(return_value={"files", "notes"})
+
+    result = await _get_enabled_apps_or_none(nc_client, "alice", scan_id=1234)
+
+    assert result == {"files", "notes"}
+
+
+async def test_returns_none_when_detection_raises(caplog):
+    nc_client = AsyncMock()
+    request = Request("GET", "https://nc.test/ocs/v2.php/core/navigation/apps")
+    nc_client.get_enabled_apps = AsyncMock(
+        side_effect=HTTPStatusError(
+            "boom", request=request, response=Response(503, request=request)
+        )
+    )
+
+    caplog.set_level(logging.WARNING, logger="nextcloud_mcp_server.vector.scanner")
+    result = await _get_enabled_apps_or_none(nc_client, "alice", scan_id=1234)
+
+    # None signals scan-all fallback; _app_enabled treats `None` as
+    # "every app enabled" so indexing never silently stops.
+    assert result is None
+    assert "scanning all apps" in caplog.text
+
+
+async def test_value_error_from_ocs_failure_returns_none():
+    """A ValueError (e.g. OCS meta.status=='failure' from get_enabled_apps)
+    routes through the scan-all fallback like any other exception."""
+    nc_client = AsyncMock()
+    nc_client.get_enabled_apps = AsyncMock(
+        side_effect=ValueError("OCS navigation returned status='failure'")
+    )
+
+    result = await _get_enabled_apps_or_none(nc_client, "alice", scan_id=1234)
+
+    assert result is None
+
+
+def test_none_set_enables_every_app():
+    """A None set means detection failed, so every app must be scanned."""
+    assert _app_enabled("news", None) is True
+    assert _app_enabled("deck", None) is True
+
+
+def test_concrete_set_gates_precisely():
+    """A resolved set scans only the apps it contains."""
+    enabled = {"notes", "files"}
+    assert _app_enabled("notes", enabled) is True
+    assert _app_enabled("news", enabled) is False
+    assert _app_enabled("deck", enabled) is False