feat(search): ACL-aware vector filter via Nextcloud Shares lookup
The vector index has always been strictly per-user: every Qdrant payload
carries a `user_id` and the search filter is `user_id == querying_user`.
A file Alice indexed cannot be discovered by Bob even if she has shared
it with him — Bob would have to re-index it under his own user_id to
make it searchable, which means duplicate index entries for every share
recipient.
Switch to ownership-with-ACL-expansion:
- New `nextcloud_mcp_server.search.access_filter` module:
- `list_accessible_owners(sharing_client, user_id)` calls the OCS
Sharing API (`shared_with_me=true`) and returns
`{user_id} ∪ {uid_owner of each share}`. Fails open to `[user_id]`
so a misbehaving Sharing API doesn't black-hole search.
- `build_ownership_filter(user_id, accessible_owners)` returns a
Qdrant `Filter` whose `should` branch matches either the new
`owner_id IN accessible_owners` field or the legacy `user_id` field.
The legacy branch keeps points indexed before this change reachable
without a migration backfill.
- Indexer payload (`vector/processor.py`) now writes `owner_id` alongside
`user_id`. `DocumentTask` gains an optional `owner_id` field; today the
scanner always runs as the owner so the processor falls back to
`user_id`, but the field is plumbed so a future shared-with-me crawler
can set the true owner without reshaping the payload contract.
- `SemanticSearchAlgorithm.search` and `BM25HybridSearchAlgorithm.search`
accept `accessible_owners` via kwargs and use the new ownership filter.
Default behaviour with no kwarg is unchanged (self-only).
- Both user-facing callers — the MCP tool path (`server/semantic.py`) and
the visualization Starlette route (`auth/viz_routes.py`) — compute
`accessible_owners` from the authenticated Nextcloud client before
invoking the search algorithm. Eviction, scanner deletion, placeholder,
and chunk-context paths intentionally keep the legacy `user_id`
semantics (those are "operations on a specific user's records", not
cross-user reads).
- 10 new unit tests in `tests/unit/search/test_access_filter.py` cover
self-only default, owner expansion, dedup, fallback fields, OCS
failure, and the legacy `should`-branch shape.
Pairs with cbcoutinho/astrolabe#89 — together they let an Astrolabe user
find content owners have shared with them without going through any
re-authorization flow or re-indexing.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.7
parent
e2ad8220d5
commit
37db82613d
@@ -33,6 +33,7 @@ from nextcloud_mcp_server.search import (
|
||||
BM25HybridSearchAlgorithm,
|
||||
SemanticSearchAlgorithm,
|
||||
)
|
||||
from nextcloud_mcp_server.search.access_filter import list_accessible_owners
|
||||
from nextcloud_mcp_server.search.context import (
|
||||
get_chunk_bbox_and_page_from_qdrant,
|
||||
get_chunk_with_context,
|
||||
@@ -158,7 +159,7 @@ async def vector_visualization_search(request: Request) -> JSONResponse:
|
||||
with trace_operation("vector_viz.get_auth_client"):
|
||||
auth_client_ctx = await _get_authenticated_client_for_userinfo(request)
|
||||
|
||||
async with auth_client_ctx as nc_client: # noqa: F841
|
||||
async with auth_client_ctx as nc_client:
|
||||
# Create search algorithm (no client needed - verification removed)
|
||||
if algorithm == "semantic":
|
||||
search_algo = SemanticSearchAlgorithm(score_threshold=score_threshold)
|
||||
@@ -172,6 +173,13 @@ async def vector_visualization_search(request: Request) -> JSONResponse:
|
||||
status_code=400,
|
||||
)
|
||||
|
||||
# Expand the caller to every owner whose content they have
|
||||
# read access to — same logic as the MCP tool path. See
|
||||
# nextcloud_mcp_server.search.access_filter.
|
||||
accessible_owners = await list_accessible_owners(
|
||||
nc_client.sharing, username
|
||||
)
|
||||
|
||||
# Execute search (supports cross-app when doc_types=None)
|
||||
# Get unverified results with buffer for filtering
|
||||
search_start = time.perf_counter()
|
||||
@@ -192,6 +200,7 @@ async def vector_visualization_search(request: Request) -> JSONResponse:
|
||||
limit=limit * 2, # Buffer for verification filtering
|
||||
doc_type=None, # Search all types
|
||||
score_threshold=score_threshold,
|
||||
accessible_owners=accessible_owners,
|
||||
)
|
||||
all_results.extend(unverified_results)
|
||||
else:
|
||||
@@ -211,6 +220,7 @@ async def vector_visualization_search(request: Request) -> JSONResponse:
|
||||
limit=limit * 2, # Buffer for verification filtering
|
||||
doc_type=doc_type,
|
||||
score_threshold=score_threshold,
|
||||
accessible_owners=accessible_owners,
|
||||
)
|
||||
all_results.extend(unverified_results)
|
||||
# Sort by score before verification
|
||||
|
||||
@@ -0,0 +1,98 @@
|
||||
"""ACL-aware ownership filter for semantic / BM25 search.
|
||||
|
||||
The vector store payload carries an ``owner_id`` field — the UID of the user
|
||||
who owns the underlying Nextcloud document. At query time, a user should
|
||||
be able to find every document whose owner has shared it (directly or via
|
||||
group / link) with them, without re-indexing.
|
||||
|
||||
This module turns "who can user X read?" into a Qdrant filter:
|
||||
``owner_id IN accessible_owners`` where ``accessible_owners`` is
|
||||
``{X} ∪ {owners of files / objects shared with X}``.
|
||||
|
||||
A second OR-branch matches the legacy ``user_id`` field so points indexed
|
||||
before this change (which carry only ``user_id``) continue to be findable
|
||||
by their original indexer. New points carry both fields.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import logging
|
||||
from typing import Any, Protocol
|
||||
|
||||
from qdrant_client.models import FieldCondition, Filter, MatchAny, MatchValue
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
class _SharingClientProtocol(Protocol):
|
||||
"""Subset of SharingClient that this module actually uses."""
|
||||
|
||||
async def list_shares(
|
||||
self, path: str | None = None, shared_with_me: bool = False
|
||||
) -> list[dict[str, Any]]: ...
|
||||
|
||||
|
||||
async def list_accessible_owners(
|
||||
sharing_client: _SharingClientProtocol,
|
||||
user_id: str,
|
||||
) -> list[str]:
|
||||
"""Return every owner UID whose content `user_id` should be able to search.
|
||||
|
||||
The set is ``{user_id} ∪ {uid_owner of each share with shared_with_me=True}``.
|
||||
Duplicates are removed; ordering is not significant (Qdrant ``MatchAny``
|
||||
treats the list as a set).
|
||||
|
||||
Sharing API failures are non-fatal — we degrade to ``[user_id]`` and log
|
||||
so a hiccup in OCS doesn't black-hole the user's own search.
|
||||
"""
|
||||
owners: set[str] = {user_id}
|
||||
try:
|
||||
shares = await sharing_client.list_shares(shared_with_me=True)
|
||||
except Exception as exc: # noqa: BLE001 — degrade gracefully
|
||||
logger.warning(
|
||||
"Sharing API unavailable; falling back to self-only owner filter "
|
||||
"for user %s (%s)",
|
||||
user_id,
|
||||
exc,
|
||||
)
|
||||
return [user_id]
|
||||
|
||||
for share in shares:
|
||||
# OCS returns the share owner under `uid_owner` (the file owner,
|
||||
# not the share recipient). Some Nextcloud versions also surface
|
||||
# `owner` as a fallback display field — we tolerate both.
|
||||
owner = share.get("uid_owner") or share.get("owner")
|
||||
if isinstance(owner, str) and owner:
|
||||
owners.add(owner)
|
||||
|
||||
logger.debug("Accessible owners for user %s: %d entries", user_id, len(owners))
|
||||
return list(owners)
|
||||
|
||||
|
||||
def build_ownership_filter(
|
||||
user_id: str, accessible_owners: list[str] | None = None
|
||||
) -> Filter:
|
||||
"""Build the Qdrant ``Filter`` constraining a search to readable points.
|
||||
|
||||
Matches points whose ``owner_id`` is in ``accessible_owners`` OR whose
|
||||
legacy ``user_id`` equals ``user_id``. The legacy branch keeps points
|
||||
indexed before this change reachable until they're re-indexed.
|
||||
|
||||
Args:
|
||||
user_id: Querying user (used for the legacy ``user_id`` fallback
|
||||
and as the only-self default when ``accessible_owners`` is None).
|
||||
accessible_owners: Pre-computed list of owner UIDs the user has
|
||||
access to. When None, defaults to ``[user_id]`` (no shares
|
||||
expansion — used by callers that genuinely want self-only
|
||||
scope such as eviction sweeps).
|
||||
|
||||
Returns:
|
||||
A Qdrant ``Filter`` ready to be nested under a parent ``must`` clause.
|
||||
"""
|
||||
owners = accessible_owners if accessible_owners is not None else [user_id]
|
||||
return Filter(
|
||||
should=[
|
||||
FieldCondition(key="owner_id", match=MatchAny(any=owners)),
|
||||
FieldCondition(key="user_id", match=MatchValue(value=user_id)),
|
||||
]
|
||||
)
|
||||
@@ -10,6 +10,7 @@ from nextcloud_mcp_server.config import get_settings
|
||||
from nextcloud_mcp_server.embedding import get_bm25_service, get_embedding_service
|
||||
from nextcloud_mcp_server.observability.metrics import record_qdrant_operation
|
||||
from nextcloud_mcp_server.observability.tracing import trace_operation
|
||||
from nextcloud_mcp_server.search.access_filter import build_ownership_filter
|
||||
from nextcloud_mcp_server.search.algorithms import (
|
||||
SearchAlgorithm,
|
||||
SearchResult,
|
||||
@@ -98,6 +99,7 @@ class BM25HybridSearchAlgorithm(SearchAlgorithm):
|
||||
"""
|
||||
settings = get_settings()
|
||||
score_threshold = kwargs.get("score_threshold", self.score_threshold)
|
||||
accessible_owners: list[str] | None = kwargs.get("accessible_owners")
|
||||
|
||||
logger.info(
|
||||
"BM25 hybrid search: query='%s', user=%s, limit=%s, score_threshold=%s, doc_type=%s, fusion=%s",
|
||||
@@ -131,10 +133,7 @@ class BM25HybridSearchAlgorithm(SearchAlgorithm):
|
||||
# Build Qdrant filter
|
||||
filter_conditions = [
|
||||
get_placeholder_filter(), # Always exclude placeholders from user-facing queries
|
||||
FieldCondition(
|
||||
key="user_id",
|
||||
match=MatchValue(value=user_id),
|
||||
),
|
||||
build_ownership_filter(user_id, accessible_owners),
|
||||
]
|
||||
|
||||
# Add doc_type filter if specified
|
||||
|
||||
@@ -8,6 +8,7 @@ from qdrant_client.models import FieldCondition, Filter, MatchValue
|
||||
from nextcloud_mcp_server.config import get_settings
|
||||
from nextcloud_mcp_server.embedding import get_embedding_service
|
||||
from nextcloud_mcp_server.observability.metrics import record_qdrant_operation
|
||||
from nextcloud_mcp_server.search.access_filter import build_ownership_filter
|
||||
from nextcloud_mcp_server.search.algorithms import (
|
||||
SearchAlgorithm,
|
||||
SearchResult,
|
||||
@@ -65,7 +66,11 @@ class SemanticSearchAlgorithm(SearchAlgorithm):
|
||||
user_id: User ID for filtering
|
||||
limit: Maximum results to return
|
||||
doc_type: Optional document type filter
|
||||
**kwargs: Additional parameters (score_threshold override)
|
||||
**kwargs:
|
||||
- score_threshold (float): override the instance default
|
||||
- accessible_owners (list[str]): owner UIDs the user can read
|
||||
(self + share senders). Pre-computed by the caller from the
|
||||
OCS Sharing API. Defaults to ``[user_id]`` when omitted.
|
||||
|
||||
Returns:
|
||||
List of unverified SearchResult objects ranked by similarity score
|
||||
@@ -75,6 +80,7 @@ class SemanticSearchAlgorithm(SearchAlgorithm):
|
||||
"""
|
||||
settings = get_settings()
|
||||
score_threshold = kwargs.get("score_threshold", self.score_threshold)
|
||||
accessible_owners: list[str] | None = kwargs.get("accessible_owners")
|
||||
|
||||
logger.info(
|
||||
"Semantic search: query='%s', user=%s, limit=%s, score_threshold=%s, doc_type=%s",
|
||||
@@ -97,10 +103,7 @@ class SemanticSearchAlgorithm(SearchAlgorithm):
|
||||
# Build Qdrant filter
|
||||
filter_conditions = [
|
||||
get_placeholder_filter(), # Always exclude placeholders from user-facing queries
|
||||
FieldCondition(
|
||||
key="user_id",
|
||||
match=MatchValue(value=user_id),
|
||||
),
|
||||
build_ownership_filter(user_id, accessible_owners),
|
||||
]
|
||||
|
||||
# Add doc_type filter if specified
|
||||
|
||||
@@ -30,6 +30,7 @@ from nextcloud_mcp_server.models.semantic import (
|
||||
from nextcloud_mcp_server.observability.metrics import (
|
||||
instrument_tool,
|
||||
)
|
||||
from nextcloud_mcp_server.search.access_filter import list_accessible_owners
|
||||
from nextcloud_mcp_server.search.bm25_hybrid import BM25HybridSearchAlgorithm
|
||||
from nextcloud_mcp_server.search.context import get_chunk_with_context
|
||||
from nextcloud_mcp_server.search.verification import verify_search_results
|
||||
@@ -121,6 +122,12 @@ def configure_semantic_tools(mcp: FastMCP):
|
||||
)
|
||||
)
|
||||
|
||||
# Expand the caller's identity to every owner whose content they
|
||||
# have read access to via Nextcloud shares. Lets a user find files
|
||||
# owners have shared with them without having to re-index those
|
||||
# files under their own user_id.
|
||||
accessible_owners = await list_accessible_owners(client.sharing, username)
|
||||
|
||||
try:
|
||||
# Create BM25 hybrid search algorithm with specified fusion
|
||||
search_algo = BM25HybridSearchAlgorithm(
|
||||
@@ -153,6 +160,7 @@ def configure_semantic_tools(mcp: FastMCP):
|
||||
limit=limit * 2,
|
||||
doc_type=None, # Signal to search all types
|
||||
score_threshold=score_threshold,
|
||||
accessible_owners=accessible_owners,
|
||||
)
|
||||
all_results.extend(unverified_results)
|
||||
else:
|
||||
@@ -177,6 +185,7 @@ def configure_semantic_tools(mcp: FastMCP):
|
||||
limit=limit * 2,
|
||||
doc_type=dtype,
|
||||
score_threshold=score_threshold,
|
||||
accessible_owners=accessible_owners,
|
||||
)
|
||||
all_results.extend(unverified_results)
|
||||
|
||||
|
||||
@@ -706,6 +706,16 @@ async def _index_document(
|
||||
},
|
||||
payload={
|
||||
"user_id": doc_task.user_id,
|
||||
# owner_id is the UID of the file's owner — what
|
||||
# search-time ACL expansion filters on. Today the scanner
|
||||
# always runs as the file's owner (per-user crawl, only
|
||||
# surfaces files the user owns or that fall under their
|
||||
# WebDAV root), so owner_id == user_id is correct for
|
||||
# every doc type indexed here. The fields are kept
|
||||
# separate so a future indexer change that lets a user
|
||||
# crawl shared-with-them content can set owner_id to the
|
||||
# true owner without losing the "who indexed this" trail.
|
||||
"owner_id": doc_task.owner_id or doc_task.user_id,
|
||||
"doc_id": doc_task.doc_id,
|
||||
"doc_type": doc_task.doc_type,
|
||||
"is_placeholder": False, # Real indexed document (not placeholder)
|
||||
|
||||
@@ -108,6 +108,12 @@ class DocumentTask:
|
||||
metadata: dict[str, int | str] | None = (
|
||||
None # Additional metadata (e.g., board_id/stack_id for deck_card)
|
||||
)
|
||||
# UID of the true owner of the indexed object, used by the search-time
|
||||
# ACL filter. None today (scanner always runs as the owner, so the
|
||||
# processor falls back to user_id), but settable so a future
|
||||
# shared-with-me crawl can pass through the actual owner without
|
||||
# reshaping the payload contract.
|
||||
owner_id: str | None = None
|
||||
|
||||
|
||||
# Track documents potentially deleted (grace period before actual deletion)
|
||||
|
||||
@@ -0,0 +1,118 @@
|
||||
"""Tests for nextcloud_mcp_server.search.access_filter."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from unittest.mock import AsyncMock
|
||||
|
||||
import pytest
|
||||
|
||||
from nextcloud_mcp_server.search.access_filter import (
|
||||
build_ownership_filter,
|
||||
list_accessible_owners,
|
||||
)
|
||||
|
||||
|
||||
class TestListAccessibleOwners:
|
||||
@pytest.mark.unit
|
||||
async def test_includes_self_even_with_no_shares(self) -> None:
|
||||
sharing = AsyncMock()
|
||||
sharing.list_shares.return_value = []
|
||||
|
||||
owners = await list_accessible_owners(sharing, "alice")
|
||||
assert owners == ["alice"]
|
||||
|
||||
@pytest.mark.unit
|
||||
async def test_collects_uid_owner_from_shares(self) -> None:
|
||||
sharing = AsyncMock()
|
||||
sharing.list_shares.return_value = [
|
||||
{"uid_owner": "bob", "share_with": "alice"},
|
||||
{"uid_owner": "carol", "share_with": "alice"},
|
||||
]
|
||||
|
||||
owners = await list_accessible_owners(sharing, "alice")
|
||||
assert set(owners) == {"alice", "bob", "carol"}
|
||||
|
||||
@pytest.mark.unit
|
||||
async def test_deduplicates_repeated_owners(self) -> None:
|
||||
sharing = AsyncMock()
|
||||
sharing.list_shares.return_value = [
|
||||
{"uid_owner": "bob"},
|
||||
{"uid_owner": "bob"}, # same owner shares many files
|
||||
{"uid_owner": "bob"},
|
||||
]
|
||||
|
||||
owners = await list_accessible_owners(sharing, "alice")
|
||||
assert sorted(owners) == ["alice", "bob"]
|
||||
|
||||
@pytest.mark.unit
|
||||
async def test_falls_back_to_owner_field_when_uid_owner_missing(self) -> None:
|
||||
# Some Nextcloud versions surface `owner` instead of `uid_owner`
|
||||
# on the shared-with-me response.
|
||||
sharing = AsyncMock()
|
||||
sharing.list_shares.return_value = [{"owner": "bob"}]
|
||||
|
||||
owners = await list_accessible_owners(sharing, "alice")
|
||||
assert sorted(owners) == ["alice", "bob"]
|
||||
|
||||
@pytest.mark.unit
|
||||
async def test_ignores_share_with_no_owner_field(self) -> None:
|
||||
sharing = AsyncMock()
|
||||
sharing.list_shares.return_value = [
|
||||
{"id": 42}, # malformed share entry
|
||||
{"uid_owner": "bob"},
|
||||
{"uid_owner": 12345}, # non-string owner — skip
|
||||
]
|
||||
|
||||
owners = await list_accessible_owners(sharing, "alice")
|
||||
assert sorted(owners) == ["alice", "bob"]
|
||||
|
||||
@pytest.mark.unit
|
||||
async def test_degrades_to_self_on_sharing_api_failure(self) -> None:
|
||||
sharing = AsyncMock()
|
||||
sharing.list_shares.side_effect = RuntimeError("OCS down")
|
||||
|
||||
owners = await list_accessible_owners(sharing, "alice")
|
||||
# Fail-open to "self only" rather than blowing up search.
|
||||
assert owners == ["alice"]
|
||||
|
||||
@pytest.mark.unit
|
||||
async def test_calls_shared_with_me(self) -> None:
|
||||
sharing = AsyncMock()
|
||||
sharing.list_shares.return_value = []
|
||||
|
||||
await list_accessible_owners(sharing, "alice")
|
||||
sharing.list_shares.assert_awaited_once_with(shared_with_me=True)
|
||||
|
||||
|
||||
class TestBuildOwnershipFilter:
|
||||
def test_defaults_to_self_only_when_owners_omitted(self) -> None:
|
||||
flt = build_ownership_filter("alice")
|
||||
|
||||
assert flt.should is not None
|
||||
assert len(flt.should) == 2 # owner_id branch + legacy user_id branch
|
||||
owner_branch, user_branch = flt.should
|
||||
assert owner_branch.key == "owner_id"
|
||||
assert owner_branch.match.any == ["alice"]
|
||||
assert user_branch.key == "user_id"
|
||||
assert user_branch.match.value == "alice"
|
||||
|
||||
def test_expands_owner_branch_with_accessible_owners(self) -> None:
|
||||
flt = build_ownership_filter("alice", ["alice", "bob", "carol"])
|
||||
|
||||
owner_branch, user_branch = flt.should
|
||||
# Owner branch reflects the expanded set.
|
||||
assert set(owner_branch.match.any) == {"alice", "bob", "carol"}
|
||||
# Legacy user_id branch keeps the original user — that's the only
|
||||
# legacy match path, so it must NOT widen to other owners.
|
||||
assert user_branch.match.value == "alice"
|
||||
|
||||
def test_explicit_empty_list_still_keeps_legacy_branch(self) -> None:
|
||||
# Edge case: caller passed an explicit empty list. We shouldn't
|
||||
# silently re-default to [user_id] in the owner branch, but the
|
||||
# legacy branch is still the safety net so the user can find their
|
||||
# own content from before the migration.
|
||||
flt = build_ownership_filter("alice", [])
|
||||
|
||||
owner_branch, user_branch = flt.should
|
||||
assert owner_branch.match.any == []
|
||||
assert user_branch.match.value == "alice"
|
||||
Reference in New Issue
Block a user