diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index a43a30bb..ea2a45af 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -50,10 +50,10 @@ jobs: - nextcloud_version: "31" nextcloud_image: "docker.io/library/nextcloud:31.0.14@sha256:07ec73cc816e58d6f45a162cd53ef886462c29271a23fc68d0124cec276e3767" - nextcloud_version: "32" - nextcloud_image: "docker.io/library/nextcloud:32.0.9@sha256:a6faf7f884036fc754fbeef0a88177463df39f11ce699fb19edc4c2b7e481a47" + nextcloud_image: "docker.io/library/nextcloud:32.0.10@sha256:611669115cccef3f96aa8eb47bd07c4d57452d894ebcfc1d81f5e8ce368e7d2d" # Disabled until all upstream apps support NC 33 # - nextcloud_version: "33" - # nextcloud_image: "docker.io/library/nextcloud:33.0.3@sha256:90a730e9a3dd290d9626ca64740c4d2fa901b4f231fe4ab6eb0dcf300dbc7271" + # nextcloud_image: "docker.io/library/nextcloud:33.0.4@sha256:caa40b8beaf0057ac213d8dfc515c36ce64f7a8f0825b6a287e6f7cf2f4a095d" # Mode-specific properties - mode: single-user diff --git a/CHANGELOG.md b/CHANGELOG.md index e0cf11ca..5cf98715 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,18 @@ All notable changes to the Nextcloud MCP Server will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [PEP 440](https://peps.python.org/pep-0440/). +## v0.90.2 (2026-05-30) + +### Fix + +- **vector-sync**: isolate per-app scans so a disabled Notes app can't abort sync + +## v0.90.1 (2026-05-30) + +### Fix + +- **api**: validate app password against Nextcloud using loginName, not UID + ## v0.90.0 (2026-05-29) ### Feat diff --git a/docker-compose.yml b/docker-compose.yml index 82fc3088..49ca90d7 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -23,7 +23,7 @@ services: restart: always app: - image: ${NEXTCLOUD_IMAGE:-docker.io/library/nextcloud:32.0.9@sha256:a6faf7f884036fc754fbeef0a88177463df39f11ce699fb19edc4c2b7e481a47} + image: ${NEXTCLOUD_IMAGE:-docker.io/library/nextcloud:32.0.10@sha256:611669115cccef3f96aa8eb47bd07c4d57452d894ebcfc1d81f5e8ce368e7d2d} restart: always ports: - 127.0.0.1:8080:80 @@ -36,7 +36,7 @@ services: # Mount OIDC development directory outside /var/www/html to avoid rsync conflicts # The post-installation hook will register /opt/apps as an additional app directory #- ./third_party:/opt/apps:ro - - ./third_party/astrolabe:/opt/apps/astrolabe:ro + #- ./third_party/astrolabe:/opt/apps/astrolabe:ro #- ./third_party/oidc:/opt/apps/oidc:ro environment: - NEXTCLOUD_TRUSTED_DOMAINS=app diff --git a/nextcloud_mcp_server/api/passwords.py b/nextcloud_mcp_server/api/passwords.py index 212355b9..19ae1bdf 100644 --- a/nextcloud_mcp_server/api/passwords.py +++ b/nextcloud_mcp_server/api/passwords.py @@ -238,6 +238,24 @@ async def provision_app_password(request: Request) -> JSONResponse: status_code=400, ) + # Parse optional scopes and the Nextcloud loginName from the request body + # up front. Nextcloud authenticates app passwords against the *loginName*, + # which can differ from the UID — e.g. OIDC-provisioned users whose UID is + # their display name (UID "Chris Coutinho", loginName "chris@coutinho.io"). + # Use the loginName for the BasicAuth validation below, falling back to the + # path user_id for legacy callers that don't send one (where UID == + # loginName). + scopes = None + nc_username = None + try: + body = await request.json() + scopes = body.get("scopes") # list[str] | None + nc_username = body.get("username") # Nextcloud loginName + except Exception: + pass # No JSON body = legacy call without scopes / loginName + + login_name = nc_username or username + # Get Nextcloud host from settings settings = get_settings() nextcloud_host = settings.nextcloud_host @@ -249,7 +267,10 @@ async def provision_app_password(request: Request) -> JSONResponse: status_code=500, ) - # Validate app password against Nextcloud + # Validate app password against Nextcloud. BasicAuth places the user-id + # literally in the header (RFC 7617 — no URL-encoding) and Nextcloud keys + # app-password auth on the loginName, so authenticate as the loginName, not + # the UID. try: async with nextcloud_httpx_client( timeout=NEXTCLOUD_VALIDATION_TIMEOUT @@ -258,7 +279,7 @@ async def provision_app_password(request: Request) -> JSONResponse: test_url = f"{nextcloud_host}/ocs/v1.php/cloud/user" response = await client.get( test_url, - auth=(username, app_password), + auth=(login_name, app_password), params={"format": "json"}, headers={"OCS-APIRequest": "true"}, ) @@ -274,10 +295,11 @@ async def provision_app_password(request: Request) -> JSONResponse: status_code=401, ) - # Verify the user ID from response matches + # Verify the authenticated account maps to the path user_id (UID): + # the loginName must resolve to the UID claimed in the URL path. data = response.json() ocs_user_id = data.get("ocs", {}).get("data", {}).get("id") - if ocs_user_id != username: + if ocs_user_id != path_user_id: logger.warning("User ID mismatch in OCS response") _record_rate_limit_attempt(path_user_id, success=False) return JSONResponse( @@ -292,16 +314,6 @@ async def provision_app_password(request: Request) -> JSONResponse: status_code=500, ) - # Parse optional scopes and username from request body - scopes = None - nc_username = None - try: - body = await request.json() - scopes = body.get("scopes") # list[str] | None - nc_username = body.get("username") # Nextcloud loginName - except Exception: - pass # No JSON body = legacy call without scopes - # Store the validated app password try: storage = await _get_app_password_storage(request) diff --git a/nextcloud_mcp_server/vector/scanner.py b/nextcloud_mcp_server/vector/scanner.py index 94ad168c..14e2158c 100644 --- a/nextcloud_mcp_server/vector/scanner.py +++ b/nextcloud_mcp_server/vector/scanner.py @@ -13,6 +13,7 @@ from typing import cast import anyio from anyio.abc import TaskStatus +from httpx import HTTPStatusError from qdrant_client import AsyncQdrantClient from qdrant_client.models import FieldCondition, Filter, MatchValue, Record @@ -323,165 +324,42 @@ async def scan_user_documents( logger.debug("Found %s indexed documents in Qdrant", len(indexed_doc_ids)) - # Stream notes from Nextcloud and process immediately - note_count = 0 + # 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). + settings = get_settings() + grace_period = settings.vector_sync_scan_interval * 1.5 + current_time = time.time() queued = 0 - nextcloud_doc_ids = set() - async for note in nc_client.notes.get_all_notes(prune_before=prune_before): - note_count += 1 - doc_id = str(note["id"]) - nextcloud_doc_ids.add(doc_id) - modified_at = note.get("modified", 0) - - if initial_sync: - # Send everything on first sync - write placeholder first - await write_placeholder_point( - doc_id=doc_id, - doc_type="note", - user_id=user_id, - modified_at=modified_at, - etag=note.get("etag", ""), + try: + queued += await scan_notes( + user_id=user_id, + send_stream=send_stream, + nc_client=nc_client, + initial_sync=initial_sync, + scan_id=scan_id, + prune_before=prune_before, + indexed_doc_ids=indexed_doc_ids, + grace_period=grace_period, + current_time=current_time, + ) + except HTTPStatusError as e: + if e.response.status_code == 404: + logger.info( + "[SCAN-%s] Notes app unavailable for %s (HTTP 404); skipping notes", + scan_id, + user_id, ) - await send_stream.send( - DocumentTask( - user_id=user_id, - doc_id=doc_id, - doc_type="note", - operation="index", - modified_at=modified_at, - etag=note.get("etag"), - ) - ) - queued += 1 else: - # Incremental sync: check if document exists and compare modified_at - # If document reappeared, remove from potentially_deleted - doc_key = (user_id, doc_id) - if doc_key in _potentially_deleted: - logger.debug( - "Document %s reappeared, removing from deletion grace period", - doc_id, - ) - del _potentially_deleted[doc_key] - - # Query Qdrant for existing entry (placeholder or real) - existing_metadata = await query_document_metadata( - doc_id=doc_id, doc_type="note", user_id=user_id - ) - - # Send if never indexed or modified since last index - # Compare against stored modified_at (not indexed_at!) - needs_indexing = False - if existing_metadata is None: - # Never seen before - needs_indexing = True - elif existing_metadata.get("modified_at", 0) < modified_at: - # Document modified since last indexing - needs_indexing = True - elif existing_metadata.get("is_placeholder", False): - # Placeholder exists - check if it's stale (processing may have failed) - # Only requeue if placeholder is older than 5x scan interval - # (Large PDFs can take 3-4 minutes to process) - queued_at = existing_metadata.get("queued_at", 0) - placeholder_age = time.time() - queued_at - stale_threshold = get_settings().vector_sync_scan_interval * 5 - if placeholder_age > stale_threshold: - logger.debug( - "Found stale placeholder for note %s (age=%ss), requeuing", - doc_id, - format(placeholder_age, ".1f"), - ) - needs_indexing = True - else: - logger.debug( - "Skipping note %s with recent placeholder (age=%ss < %ss)", - doc_id, - format(placeholder_age, ".1f"), - format(stale_threshold, ".1f"), - ) - - if needs_indexing: - # Write placeholder before queuing - await write_placeholder_point( - doc_id=doc_id, - doc_type="note", - user_id=user_id, - modified_at=modified_at, - etag=note.get("etag", ""), - ) - await send_stream.send( - DocumentTask( - user_id=user_id, - doc_id=doc_id, - doc_type="note", - operation="index", - modified_at=modified_at, - etag=note.get("etag"), - ) - ) - queued += 1 - - # Log and record metrics after streaming - logger.info("[SCAN-%s] Found %s notes for %s", scan_id, note_count, user_id) - record_vector_sync_scan(note_count) + 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) if initial_sync: logger.info("Sent %s documents for initial sync: %s", queued, user_id) return - # Check for deleted documents (in Qdrant but not in Nextcloud) - # Use grace period: only delete after 2 consecutive scans confirm absence - settings = get_settings() - grace_period = ( - settings.vector_sync_scan_interval * 1.5 - ) # Allow 1.5 scan intervals - current_time = time.time() - - for doc_id in indexed_doc_ids: - if doc_id not in nextcloud_doc_ids: - doc_key = (user_id, doc_id) - - if doc_key in _potentially_deleted: - # Already marked as potentially deleted, check if grace period elapsed - first_missing_time = _potentially_deleted[doc_key] - time_missing = current_time - first_missing_time - - if time_missing >= grace_period: - # Grace period elapsed, send for deletion - logger.info( - "Document %s missing for %ss (>%ss grace period), sending deletion", - doc_id, - format(time_missing, ".1f"), - format(grace_period, ".1f"), - ) - await send_stream.send( - DocumentTask( - user_id=user_id, - doc_id=doc_id, - doc_type="note", - operation="delete", - modified_at=0, - ) - ) - queued += 1 - # Remove from tracking after sending deletion - del _potentially_deleted[doc_key] - else: - logger.debug( - "Document %s still missing (%ss/%ss grace period)", - doc_id, - format(time_missing, ".1f"), - format(grace_period, ".1f"), - ) - else: - # First time missing, add to grace period tracking - logger.debug( - "Document %s missing for first time, starting grace period", - doc_id, - ) - _potentially_deleted[doc_key] = current_time - # Scan tagged PDF files (after notes) # Get indexed file IDs from Qdrant (for deletion tracking) indexed_file_ids = set() @@ -751,6 +629,181 @@ async def scan_user_documents( logger.debug("No changes detected for %s", user_id) +async def scan_notes( + user_id: str, + send_stream: TaskProducer, + nc_client: NextcloudClient, + initial_sync: bool, + scan_id: int, + prune_before: int | None, + indexed_doc_ids: set[str], + grace_period: float, + current_time: float, +) -> int: + """Scan a user's Notes and queue changed notes for indexing. + + Extracted into its own function (like scan_news_items / scan_deck_cards) so a + failure here -- e.g. the Notes API returning 404 because the app is not + installed -- propagates to the caller's per-app guard instead of aborting the + whole user scan. The deletion-tracking pass runs only after the Notes fetch + succeeds, so a failed fetch never mass-deletes a user's indexed notes. + + Returns: + Number of notes queued for processing (index + delete operations). + """ + # Stream notes from Nextcloud and process immediately + note_count = 0 + queued = 0 + nextcloud_doc_ids: set[str] = set() + + async for note in nc_client.notes.get_all_notes(prune_before=prune_before): + note_count += 1 + doc_id = str(note["id"]) + nextcloud_doc_ids.add(doc_id) + modified_at = note.get("modified", 0) + + if initial_sync: + # Send everything on first sync - write placeholder first + await write_placeholder_point( + doc_id=doc_id, + doc_type="note", + user_id=user_id, + modified_at=modified_at, + etag=note.get("etag", ""), + ) + await send_stream.send( + DocumentTask( + user_id=user_id, + doc_id=doc_id, + doc_type="note", + operation="index", + modified_at=modified_at, + ) + ) + queued += 1 + else: + # Incremental sync: check if document exists and compare modified_at + # If document reappeared, remove from potentially_deleted + doc_key = (user_id, doc_id) + if doc_key in _potentially_deleted: + logger.debug( + "Document %s reappeared, removing from deletion grace period", + doc_id, + ) + del _potentially_deleted[doc_key] + + # Query Qdrant for existing entry (placeholder or real) + existing_metadata = await query_document_metadata( + doc_id=doc_id, doc_type="note", user_id=user_id + ) + + # Send if never indexed or modified since last index + # Compare against stored modified_at (not indexed_at!) + needs_indexing = False + if existing_metadata is None: + # Never seen before + needs_indexing = True + elif existing_metadata.get("modified_at", 0) < modified_at: + # Document modified since last indexing + needs_indexing = True + elif existing_metadata.get("is_placeholder", False): + # Placeholder exists - check if it's stale (processing may have failed) + # Only requeue if placeholder is older than 5x scan interval + # (Large PDFs can take 3-4 minutes to process) + queued_at = existing_metadata.get("queued_at", 0) + placeholder_age = time.time() - queued_at + stale_threshold = get_settings().vector_sync_scan_interval * 5 + if placeholder_age > stale_threshold: + logger.debug( + "Found stale placeholder for note %s (age=%ss), requeuing", + doc_id, + format(placeholder_age, ".1f"), + ) + needs_indexing = True + else: + logger.debug( + "Skipping note %s with recent placeholder (age=%ss < %ss)", + doc_id, + format(placeholder_age, ".1f"), + format(stale_threshold, ".1f"), + ) + + if needs_indexing: + # Write placeholder before queuing + await write_placeholder_point( + doc_id=doc_id, + doc_type="note", + user_id=user_id, + modified_at=modified_at, + etag=note.get("etag", ""), + ) + await send_stream.send( + DocumentTask( + user_id=user_id, + doc_id=doc_id, + doc_type="note", + operation="index", + modified_at=modified_at, + ) + ) + queued += 1 + + # Log and record metrics after streaming + logger.info("[SCAN-%s] Found %s notes for %s", scan_id, note_count, user_id) + record_vector_sync_scan(note_count) + + if initial_sync: + return queued + + # Check for deleted documents (in Qdrant but not in Nextcloud) + # Use grace period: only delete after 2 consecutive scans confirm absence + for doc_id in indexed_doc_ids: + if doc_id not in nextcloud_doc_ids: + doc_key = (user_id, doc_id) + + if doc_key in _potentially_deleted: + # Already marked as potentially deleted, check if grace period elapsed + first_missing_time = _potentially_deleted[doc_key] + time_missing = current_time - first_missing_time + + if time_missing >= grace_period: + # Grace period elapsed, send for deletion + logger.info( + "Document %s missing for %ss (>%ss grace period), sending deletion", + doc_id, + format(time_missing, ".1f"), + format(grace_period, ".1f"), + ) + await send_stream.send( + DocumentTask( + user_id=user_id, + doc_id=doc_id, + doc_type="note", + operation="delete", + modified_at=0, + ) + ) + queued += 1 + # Remove from tracking after sending deletion + del _potentially_deleted[doc_key] + else: + logger.debug( + "Document %s still missing (%ss/%ss grace period)", + doc_id, + format(time_missing, ".1f"), + format(grace_period, ".1f"), + ) + else: + # First time missing, add to grace period tracking + logger.debug( + "Document %s missing for first time, starting grace period", + doc_id, + ) + _potentially_deleted[doc_key] = current_time + + return queued + + async def scan_news_items( user_id: str, send_stream: TaskProducer, diff --git a/pyproject.toml b/pyproject.toml index 5b9fea30..4295545e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "nextcloud-mcp-server" -version = "0.90.0" +version = "0.90.2" description = "Model Context Protocol (MCP) server for Nextcloud integration - enables AI assistants to interact with Nextcloud data" authors = [ {name = "Chris Coutinho", email = "chris@coutinho.io"} diff --git a/tests/unit/test_management_app_password_endpoints.py b/tests/unit/test_management_app_password_endpoints.py index 2f0fd0f3..0a6fcb55 100644 --- a/tests/unit/test_management_app_password_endpoints.py +++ b/tests/unit/test_management_app_password_endpoints.py @@ -248,6 +248,64 @@ async def test_provision_app_password_success(temp_storage, mocker): stored_password = await temp_storage.get_app_password("testuser") assert stored_password == "aaaaa-bbbbb-ccccc-ddddd-eeeee" + # Legacy callers send no loginName in the body → the OCS validation falls + # back to authenticating as the UID (here UID == loginName). + _, get_kwargs = mock_client.get.call_args + assert get_kwargs["auth"] == ("testuser", "aaaaa-bbbbb-ccccc-ddddd-eeeee") + + +async def test_provision_app_password_uses_loginname_not_uid(temp_storage, mocker): + """Regression: when the Nextcloud UID differs from the loginName (e.g. + OIDC-provisioned users whose UID is their display name — UID + "Chris Coutinho", loginName "chris@coutinho.io"), the OCS BasicAuth + validation must authenticate as the loginName from the request body, not + the UID. Authenticating as the UID is rejected by Nextcloud with HTTP 401. + """ + mocker.patch( + "nextcloud_mcp_server.api.passwords.get_settings", + return_value=MagicMock( + nextcloud_host="http://localhost:8080", + nextcloud_verify_ssl=True, + nextcloud_ca_bundle=None, + ), + ) + + # OCS validation succeeds and reports the UID as the account id. + mock_response = MagicMock() + mock_response.status_code = 200 + mock_response.json.return_value = {"ocs": {"data": {"id": "Chris Coutinho"}}} + + mock_client = AsyncMock() + mock_client.get = AsyncMock(return_value=mock_response) + mock_client.__aenter__ = AsyncMock(return_value=mock_client) + mock_client.__aexit__ = AsyncMock() + mocker.patch( + "nextcloud_mcp_server.api.passwords.nextcloud_httpx_client", + return_value=mock_client, + ) + + app = create_test_app(temp_storage) + client = TestClient(app) + + pw = "aaaaa-bbbbb-ccccc-ddddd-eeeee" + # A literal space in the path is encoded by the client and decoded back to + # the UID; the BasicAuth username matches that UID. + response = client.post( + "/api/v1/users/Chris Coutinho/app-password", + headers={"Authorization": create_basic_auth_header("Chris Coutinho", pw)}, + json={"username": "chris@coutinho.io"}, + ) + + assert response.status_code == 200 + assert response.json()["success"] is True + + # The OCS BasicAuth used the loginName from the body, not the UID. + _, get_kwargs = mock_client.get.call_args + assert get_kwargs["auth"] == ("chris@coutinho.io", pw) + + # Stored under the UID (the identity key). + assert await temp_storage.get_app_password("Chris Coutinho") == pw + async def test_provision_app_password_nextcloud_validation_fails(mocker): """Test that failed Nextcloud validation returns 401.""" diff --git a/third_party/astrolabe b/third_party/astrolabe index c2dec999..8bf678f8 160000 --- a/third_party/astrolabe +++ b/third_party/astrolabe @@ -1 +1 @@ -Subproject commit c2dec999063e15e652321b0867457fef6e0f8e5c +Subproject commit 8bf678f85fa4ef605539804edf7081c6e7d2f6e9 diff --git a/uv.lock b/uv.lock index df24f4a8..f153f36d 100644 --- a/uv.lock +++ b/uv.lock @@ -2180,7 +2180,7 @@ wheels = [ [[package]] name = "nextcloud-mcp-server" -version = "0.90.0" +version = "0.90.2" source = { editable = "." } dependencies = [ { name = "aiosqlite" },