From 8b26ecafb6e8dda8d08ca237310537e7ae162944 Mon Sep 17 00:00:00 2001 From: shamoon <4887959+shamoon@users.noreply.github.com> Date: Thu, 15 Oct 2026 22:42:34 -0700 Subject: [PATCH] Fix: prevent ownerless docs leaking others in to LLM prompts --- src/documents/permissions.py | 22 ++++ src/documents/tests/test_workflows.py | 44 ++++++++ src/paperless_ai/ai_classifier.py | 32 ++++-- src/paperless_ai/matching.py | 4 +- src/paperless_ai/taxonomy.py | 4 +- src/paperless_ai/tests/test_ai_classifier.py | 103 +++++++++++++++++-- src/paperless_ai/tests/test_matching.py | 28 ++--- src/paperless_ai/tests/test_taxonomy.py | 32 ++---- 8 files changed, 203 insertions(+), 66 deletions(-) diff --git a/src/documents/permissions.py b/src/documents/permissions.py index 553e3ddf2..3f7329dcb 100644 --- a/src/documents/permissions.py +++ b/src/documents/permissions.py @@ -511,6 +511,28 @@ def restrict_queryset_to_visible( return queryset.filter(pk__in=permitted_object_ids(user, queryset.model, perm)) +def user_is_active_superuser(user: User | None) -> bool: + """ + True only for an active, authenticated superuser. + """ + return user is not None and user_is_unrestricted(user) + + +def restrict_queryset_to_owner_visible( + queryset: QuerySet[ModelT], + user: User | None, + perm: str, +) -> QuerySet[ModelT]: + """ + Like ``restrict_queryset_to_visible``, but ``user=None`` sees only unowned + rows (``permitted_object_ids(None, ...)`` semantics) instead of every row. + See ``user_is_active_superuser``. + """ + if user_is_active_superuser(user): + return queryset + return queryset.filter(pk__in=permitted_object_ids(user, queryset.model, perm)) + + def permitted_document_ids( user: User | None, *, diff --git a/src/documents/tests/test_workflows.py b/src/documents/tests/test_workflows.py index 3bd9e0775..7d7b5c080 100644 --- a/src/documents/tests/test_workflows.py +++ b/src/documents/tests/test_workflows.py @@ -6176,6 +6176,50 @@ class TestApplyAISuggestionsWorkflowAction( self.assertEqual(self.doc.tags.count(), 0) + def test_unowned_document_does_not_get_private_objects_by_id(self) -> None: + """ + GIVEN: + - An unowned document + - AI suggestions selecting another user's private tag and + correspondent by ID, plus an unowned tag + WHEN: + - The suggestions are applied + THEN: + - Only the unowned tag is assigned; an unowned document must not + be treated as unrestricted + """ + self.doc.owner = None + self.doc.save(update_fields=["owner"]) + other = User.objects.create(username="someone-else") + private_tag = Tag.objects.create(name="Private Tag", owner=other) + public_tag = Tag.objects.create(name="Public Tag", owner=None) + private_correspondent = Correspondent.objects.create( + name="Private Correspondent", + owner=other, + ) + action = self.make_action( + ai_suggestion_fields=[ + WorkflowAction.AISuggestionField.TAGS, + WorkflowAction.AISuggestionField.CORRESPONDENT, + ], + ) + suggestions: ClassificationSuggestions = { + **SUGGESTIONS, + "tags": { + "existing_ids": [private_tag.pk, public_tag.pk], + "new_names": [], + }, + "correspondents": { + "existing_ids": [private_correspondent.pk], + "new_names": [], + }, + } + + self.apply(action, suggestions) + + self.assertEqual(list(self.doc.tags.all()), [public_tag]) + self.assertIsNone(self.doc.correspondent) + def test_unparsable_dates_are_skipped(self) -> None: """ GIVEN: diff --git a/src/paperless_ai/ai_classifier.py b/src/paperless_ai/ai_classifier.py index 391edca30..b07beada7 100644 --- a/src/paperless_ai/ai_classifier.py +++ b/src/paperless_ai/ai_classifier.py @@ -5,7 +5,7 @@ from django.contrib.auth.models import User from documents.models import Document from documents.permissions import permitted_document_ids -from documents.permissions import user_is_unrestricted +from documents.permissions import user_is_active_superuser from documents.versioning import annotate_effective_content from paperless.config import AIConfig from paperless_ai.base_model import ClassificationSuggestions @@ -50,16 +50,17 @@ def _fulltext_similar_documents( similarity - cruder, but far better than no candidates at all. more_like_this_ids returns only a ranked ID list, no scores, so weight is synthesized from rank (descending from top_k) rather than claiming a - similarity magnitude that doesn't exist. An unrestricted user (none, or an - active superuser - see user_is_unrestricted) is normalized to ``None`` - before calling, since the backend's permission filter has no superuser - short-circuit of its own. Results are re-checked with + similarity magnitude that doesn't exist. An active superuser is + normalized to ``None`` (no filtering) before calling, since the backend's + permission filter has no superuser short-circuit of its own. ``user=None`` + means the document is unowned, so results are limited to unowned documents + rather than the whole library. Results are re-checked with permitted_document_ids() since Tantivy's indexed permission fields lag the DB via async reindexing and judge a version by its own owner. """ from documents.search import get_backend - unrestricted = user_is_unrestricted(user) + unrestricted = user_is_active_superuser(user) search_user = None if unrestricted else user backend = get_backend() similar_ids = backend.more_like_this_ids( @@ -187,13 +188,16 @@ def get_taxonomy_context( candidates/context rather than propagating the exception - neither a vector-store outage nor a search-index issue should block classification, only its context-assisted enrichment. + + ``user=None`` means "acting for an unowned document" (workflows act as the + document owner), NOT "no restriction": only unowned documents and taxonomy + are considered, so another user's private documents never reach the prompt. """ ai_config = AIConfig() try: if ai_config.llm_embedding_backend: - # None means "no restriction" to retrieve_similar_nodes. An - # unrestricted user (no user at all, or an active superuser -- see - # user_is_unrestricted) can see every document, so skip + # None means "no restriction" to retrieve_similar_nodes. An active + # superuser can see every document, so skip # materializing every visible pk into a Python list and passing it # through as an IN filter: for a large library that is a wasted # quadratic scan in the vector store at best, and past ~32,763 @@ -204,7 +208,7 @@ def get_taxonomy_context( # which documents are considered -- only how we get there. visible_document_ids = ( None - if user_is_unrestricted(user) + if user_is_active_superuser(user) else list(permitted_document_ids(user)) ) nodes = retrieve_similar_nodes( @@ -226,8 +230,14 @@ def get_taxonomy_context( # similar_documents is already ordered by descending weight; don't lose it. similar_document_ids = [s["document_id"] for s in similar_documents] + similar_queryset = Document.objects.all() + if not user_is_active_superuser(user): + # Already scoped by retrieval but re-checked here before being used in the prompt + similar_queryset = similar_queryset.filter( + id__in=permitted_document_ids(user), + ) similar_documents_by_id = annotate_effective_content( - Document.objects.all(), + similar_queryset, ).in_bulk(similar_document_ids) similar_docs = [ similar_documents_by_id[document_id] diff --git a/src/paperless_ai/matching.py b/src/paperless_ai/matching.py index b2e0ec955..a5ed40f37 100644 --- a/src/paperless_ai/matching.py +++ b/src/paperless_ai/matching.py @@ -12,7 +12,7 @@ from documents.models import DocumentType from documents.models import StoragePath from documents.models import Tag from documents.permissions import permitted_object_ids -from documents.permissions import restrict_queryset_to_visible +from documents.permissions import restrict_queryset_to_owner_visible MATCH_THRESHOLD = 0.8 @@ -34,7 +34,7 @@ def _resolve_visible_ids( """ if not ids: return [] - queryset = restrict_queryset_to_visible( + queryset = restrict_queryset_to_owner_visible( model.objects.filter(pk__in=ids), user, perm, diff --git a/src/paperless_ai/taxonomy.py b/src/paperless_ai/taxonomy.py index 4e3f39271..be2edfab8 100644 --- a/src/paperless_ai/taxonomy.py +++ b/src/paperless_ai/taxonomy.py @@ -13,7 +13,7 @@ from documents.models import Document from documents.models import DocumentType from documents.models import StoragePath from documents.models import Tag -from documents.permissions import restrict_queryset_to_visible +from documents.permissions import restrict_queryset_to_owner_visible from paperless_ai.prompts.context import TaxonomyBlockPromptContext from paperless_ai.prompts.render import render_prompt @@ -97,7 +97,7 @@ def _visible_ranked_candidates( """ if not weighted_ids: return [] - visible_queryset = restrict_queryset_to_visible( + visible_queryset = restrict_queryset_to_owner_visible( model.objects.filter(pk__in=weighted_ids), user, perm, diff --git a/src/paperless_ai/tests/test_ai_classifier.py b/src/paperless_ai/tests/test_ai_classifier.py index ccc2cd08a..7c5bf525f 100644 --- a/src/paperless_ai/tests/test_ai_classifier.py +++ b/src/paperless_ai/tests/test_ai_classifier.py @@ -613,32 +613,83 @@ class TestGetTaxonomyContextVisibility: @pytest.mark.django_db @override_settings(LLM_EMBEDDING_BACKEND="huggingface") - def test_skips_permission_lookup_when_no_user( + def test_restricts_to_unowned_documents_when_no_user( self, mocker: pytest_mock.MockerFixture, ) -> None: """ GIVEN: - - No user is supplied + - No user is supplied (a workflow acting for an unowned document) + - Another user's private document WHEN: - get_taxonomy_context() is called THEN: - - Permission lookup is skipped and no document_ids restriction is - passed to retrieve_similar_nodes() + - Only unowned documents are passed to retrieve_similar_nodes(), + never the whole library """ - document = DocumentFactory.create(content="Some content") + other_owner = UserFactory.create() + document = DocumentFactory.create(content="Some content", owner=None) + unowned = DocumentFactory.create(content="Public", owner=None) + private = DocumentFactory.create(content="Private", owner=other_owner) mock_retrieve = mocker.patch( "paperless_ai.ai_classifier.retrieve_similar_nodes", return_value=[], ) - mock_permitted = mocker.patch( - "paperless_ai.ai_classifier.permitted_document_ids", - ) get_taxonomy_context(document, None) - mock_permitted.assert_not_called() - assert mock_retrieve.call_args.kwargs["document_ids"] is None + visible = mock_retrieve.call_args.kwargs["document_ids"] + assert visible is not None + assert document.pk in visible + assert unowned.pk in visible + assert private.pk not in visible + + @pytest.mark.django_db + @override_settings(LLM_EMBEDDING_BACKEND="huggingface") + def test_private_document_content_never_reaches_context_when_no_user( + self, + mocker: pytest_mock.MockerFixture, + ) -> None: + """ + GIVEN: + - No user is supplied (a workflow acting for an unowned document) + - Retrieval returns another user's private document anyway (e.g. + a stale vector store) + WHEN: + - get_taxonomy_context() is called + THEN: + - The private document's title and content are not in the RAG + context, while an unowned neighbour's are + """ + other_owner = UserFactory.create() + document = DocumentFactory.create(content="Some content", owner=None) + unowned = DocumentFactory.create( + title="Public neighbour", + content="public text", + owner=None, + ) + private = DocumentFactory.create( + title="Secret neighbour", + content="secret text", + owner=other_owner, + ) + mocker.patch( + "paperless_ai.ai_classifier.retrieve_similar_nodes", + return_value=[], + ) + mocker.patch( + "paperless_ai.ai_classifier._node_document_weights", + return_value=[ + SimilarDocument(document_id=private.pk, weight=0.9), + SimilarDocument(document_id=unowned.pk, weight=0.5), + ], + ) + + _candidates, context = get_taxonomy_context(document, None) + + assert "Public neighbour" in context + assert "Secret neighbour" not in context + assert "secret text" not in context @pytest.mark.django_db @override_settings(LLM_EMBEDDING_BACKEND="huggingface") @@ -668,7 +719,9 @@ class TestGetTaxonomyContextVisibility: get_taxonomy_context(document, user) - mock_permitted.assert_called_once_with(user) + # Once for retrieval, once to re-check the documents whose text goes + # into the prompt + assert {c.args for c in mock_permitted.call_args_list} == {(user,)} assert mock_retrieve.call_args.kwargs["document_ids"] == [1, 2, 3] @pytest.mark.django_db @@ -877,6 +930,34 @@ class TestFulltextSimilarDocuments: assert [s["document_id"] for s in result] == [other.pk] + def test_no_user_only_returns_unowned_documents( + self, + fulltext_backend: TantivyBackend, + ) -> None: + """ + GIVEN: + - An unowned source document, an unowned similar document and a + similar document privately owned by another user + WHEN: + - _fulltext_similar_documents() is called with no user (a workflow + acting for an unowned document) + THEN: + - Only the unowned similar document is returned + """ + other_owner = UserFactory.create() + source = DocumentFactory.create(content="shared content phrase", owner=None) + unowned = DocumentFactory.create(content="shared content phrase", owner=None) + private = DocumentFactory.create( + content="shared content phrase", + owner=other_owner, + ) + for doc in (source, unowned, private): + fulltext_backend.add_or_update(doc) + + result = _fulltext_similar_documents(source, user=None, top_k=5) + + assert [s["document_id"] for s in result] == [unowned.pk] + def test_excludes_stale_permitted_document_for_regular_user( self, fulltext_backend: TantivyBackend, diff --git a/src/paperless_ai/tests/test_matching.py b/src/paperless_ai/tests/test_matching.py index d98745922..effd938ed 100644 --- a/src/paperless_ai/tests/test_matching.py +++ b/src/paperless_ai/tests/test_matching.py @@ -154,27 +154,19 @@ class TestResolveTagIds: user = UserFactory.create() assert resolve_tag_ids([], user) == [] - def test_user_none_means_unrestricted_not_owner_isnull( - self, - mocker: pytest_mock.MockerFixture, - ) -> None: - """GIVEN a tag owned by another user and user=None - WHEN resolving the tag's id - THEN the tag is returned unfiltered and permitted_object_ids is never - called - user=None means "no restriction", not the narrower - "only unowned rows" meaning permitted_object_ids(None, ...) has. - Same convention as build_taxonomy_candidates's own call site. + def test_user_none_only_resolves_unowned_tags(self) -> None: + """GIVEN a tag owned by another user, an unowned tag and user=None + (a workflow acting for an unowned document) + WHEN resolving both tags' ids + THEN only the unowned tag is returned - user=None must not expose + another user's private taxonomy. """ - tag = TagFactory.create(name="Owned") - owner = UserFactory.create() - tag.owner = owner - tag.save() - spy = mocker.patch("documents.permissions.permitted_object_ids") + owned = TagFactory.create(name="Owned", owner=UserFactory.create()) + unowned = TagFactory.create(name="Unowned", owner=None) - result = resolve_tag_ids([tag.pk], None) + result = resolve_tag_ids([owned.pk, unowned.pk], None) - assert result == [tag] - spy.assert_not_called() + assert result == [unowned] @pytest.mark.django_db diff --git a/src/paperless_ai/tests/test_taxonomy.py b/src/paperless_ai/tests/test_taxonomy.py index eb225a1fe..710eeda18 100644 --- a/src/paperless_ai/tests/test_taxonomy.py +++ b/src/paperless_ai/tests/test_taxonomy.py @@ -271,39 +271,27 @@ class TestBuildTaxonomyCandidates: assert result["tags"] == [] - def test_user_none_means_unrestricted_not_owner_isnull( - self, - mocker: pytest_mock.MockerFixture, - ) -> None: + def test_user_none_only_includes_unowned_taxonomy(self) -> None: """ GIVEN: - - An owned tag (owner is not None) - - user=None (system/superuser/no-auth classification) + - A similar document with a tag owned by another user and an + unowned tag + - user=None (a workflow acting for an unowned document) WHEN: - build_taxonomy_candidates() is called THEN: - - The tag is included (no permission filtering occurs) - - permitted_object_ids() is never called + - Only the unowned tag is a candidate - user=None must not expose + another user's private taxonomy """ - # user=None means "no restriction" throughout ai_classifier.py (the - # same superuser/no-user fast path get_taxonomy_context uses). - # permitted_object_ids(None, ...) itself means something - # different ("only unowned rows") - it must not be called at all - # when user is None, or an owned tag like this one would be wrongly - # dropped for every unauthenticated/system-triggered classification. - tag = TagFactory.create(name="Owned") - owner = UserFactory.create() - tag.owner = owner - tag.save() + owned = TagFactory.create(name="Owned", owner=UserFactory.create()) + unowned = TagFactory.create(name="Unowned", owner=None) document = DocumentFactory.create() - document.tags.add(tag) + document.tags.add(owned, unowned) similar_documents = [make_similar(document.pk, 0.5)] - spy = mocker.patch("documents.permissions.permitted_object_ids") result = build_taxonomy_candidates(similar_documents, user=None) - assert result["tags"][0]["name"] == "Owned" - spy.assert_not_called() + assert [c["name"] for c in result["tags"]] == ["Unowned"] class TestFormatTaxonomyForPrompt: