mirror of
https://github.com/paperless-ngx/paperless-ngx.git
synced 2026-10-16 13:07:14 +00:00
Fix: prevent ownerless docs leaking others in to LLM prompts
This commit is contained in:
8 files changed
+203
-66
No files matched your search
@@ -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,
|
||||
*,
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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]
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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:
|
||||
|
||||
Reference in new issue
Block a user