fix(storage): fail closed on an empty partition scope (cross-partition leak)

[andyne13]

Closes #759.

Problem

MilvusVectorStore._build_filter_expr failed open on an empty partition scope: {"partition": []} dropped the partition clause and returned an empty expression, so the search ran unfiltered across every partition — a cross-tenant read. A scope that resolves to no accessible partition (e.g. a user with zero memberships hitting openrag-all) got other tenants' documents back, where the pre-refactor line failed closed (partition in [] matched nothing). The fail-open was even enshrined in a test.

Fix

An empty (non-wildcard) partition list/tuple now short-circuits to _MATCH_NOTHING_EXPR (1 == 0) — fail closed — and dominates any co-filter (early return before expr/other keys are read). The explicit "all" wildcard remains the only way to run an intentionally unscoped search.

• One shared chokepoint: _build_filter_expr is the sole filter builder for all read/delete paths (dense + hybrid search, scalar query, delete), so the guarantee holds everywhere. Verified the hybrid path applies the expr to both the dense and BM25 sparse legs.
• None/missing partition is deliberately left unscoped: the search routers already 403 an empty scope upstream (get_partition_name, search.py), and the partition-less path is required by the authz-gated global-_id lookup in ConversionService.get_chunk (extractText/download). Closing it there would break that flow with no isolation benefit.
• No regression: the "everything" wildcard is ["all"], never []; no caller passes an empty partition list meaning all. delete_by_filter({"partition": []}) now safely no-ops instead of erroring.

Tests

Rewrote the test that had enshrined the fail-open into a revert-prove (test_empty_partition_list_matches_nothing asserts 1 == 0; reverting the fix yields "" and fails). Added empty-tuple and two co-filter-dominance cases (expr and file_id).

• Revert-prove confirmed (3 tests fail on the reverted code).
• uv run pytest tests/unit/ green; full test_milvus_store.py (78) green; ruff check/format + layer-import-guard clean.

Scope / follow-ups (separate, noted in #759)

• Defense-in-depth: reject an empty user_partitions scope even earlier at the resolution layer.
• Unrelated but real: vector_store_searcher.search/multi_query_search accept but never read filter_params, so Workspace scoping is inert: filter_params is threaded through retrieval and dropped at the vector store #706 workspace file-scoping doesn't reach Milvus (within-partition over-retrieval, not this cross-partition leak).

Summary by CodeRabbit

• Bug Fixes
  • Empty partition filters now safely return no results instead of unintentionally querying across all partitions.
  • This behavior is enforced even when additional search conditions are provided.
• Documentation
  • Clarified how wildcard and explicitly specified partitions are handled.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 75f56590-4131-48fe-b1b4-35178bc800bc

📥 Commits

Reviewing files that changed from the base of the PR and between 1d66b77 and bd12e9d.

📒 Files selected for processing (2)

• openrag/services/storage/milvus_store.py
• tests/unit/services/storage/test_milvus_store.py

---

📝 Walkthrough

Walkthrough

_build_filter_expr now fails closed for empty partition scopes by returning "1 == 0". Documentation clarifies wildcard handling, and unit tests cover lists, tuples, and additional filter predicates.

Changes

Partition filter enforcement

Layer / File(s)	Summary
Partition filter contract and logic openrag/services/storage/milvus_store.py	Documents wildcard and empty-partition behavior, rejects wildcard mixing, and returns the match-nothing expression for empty lists or tuples.
Empty-scope regression coverage tests/unit/services/storage/test_milvus_store.py	Verifies empty lists and tuples produce "1 == 0" and override raw expr and file_id filters.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• linagora/openrag#466: Hardens Milvus filter precedence around partition constraints.
• linagora/openrag#744: Updates Milvus filter construction for fail-closed empty scopes.

Suggested labels: bug, fix

Suggested reviewers: aditykris, enjoybacon7

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main fix: failing closed on empty partition scopes to prevent cross-partition leaks.
Linked Issues check	✅ Passed	The code and tests match #759 by making empty partition lists/tuples return 1 == 0 and dominate other filters.
Out of Scope Changes check	✅ Passed	The changes stay focused on the partition leak fix and its regression tests, with no unrelated scope apparent.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/partition-empty-scope-fail-open

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[hedhoud]

LGTM. The empty partition scope now fails closed, and the focused regression coverage looks good.