fix(retrieval): enforce workspace file scoping through to Milvus search (#706) - #744
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (20)
🚧 Files skipped from review as they are similar to previous changes (17)
📝 WalkthroughWalkthroughWorkspace search resolves authorized partitions and file IDs, propagates parameterized restrictions through retrieval expansion, and enforces them in vector-store searches and neighboring-chunk hydration. Empty scopes fail closed, with API, chat, storage, and integration test coverage. ChangesWorkspace-scoped retrieval
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant SearchRoute
participant WorkspaceService
participant RetrievalService
participant VectorStoreSearcher
participant MilvusVectorStore
Client->>SearchRoute: workspace-scoped search
SearchRoute->>WorkspaceService: resolve_scope(workspace_id, partitions)
WorkspaceService-->>SearchRoute: partition and file_ids
SearchRoute->>RetrievalService: search(filter_params)
RetrievalService->>VectorStoreSearcher: search with filter_params
VectorStoreSearcher->>MilvusVectorStore: search scoped filters
MilvusVectorStore-->>VectorStoreSearcher: matching chunks
VectorStoreSearcher-->>Client: scoped results
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Ahmath-Gadji
force-pushed
the
fix/706-workspace-filter-params-dropped
branch
from
July 21, 2026 12:52
7023a25 to
816c418
Compare
aditykris
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
openrag/services/storage/vector_store_searcher.py (1)
76-77: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winHarden
filter_paramsmerge against reserved-key collisions. In bothsearch()andmulti_query_search(),filters.update(filter_params)runs afterfilters["partition"](and optionallyfilters["expr"]) are set, so afilter_paramsdict that ever contained a"partition"or"expr"key would silently override the partition-scoping/raw-filter clause — the exact tenant-isolation boundary this PR hardens. No current caller passes those keys (only"file_id"is ever used), so this isn't exploitable today, but a small guard (e.g. reject/ignore reserved keys infilter_params, or assert they're absent) would protect the invariant against future misuse.
openrag/services/storage/vector_store_searcher.py#L76-L77: guard againstfilter_paramscontaining"partition"/"expr"beforefilters.update(filter_params)insearch().openrag/services/storage/vector_store_searcher.py#L107-L108: apply the same guard inmulti_query_search().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@openrag/services/storage/vector_store_searcher.py` around lines 76 - 77, The filter_params merge can overwrite reserved partition-scoping or raw-filter clauses. In openrag/services/storage/vector_store_searcher.py lines 76-77 within search(), guard or reject filter_params keys named "partition" and "expr" before updating filters; apply the identical protection at lines 107-108 within multi_query_search(), preserving the internally constructed filters.openrag/api/routers/user/search.py (1)
167-174: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate workspace-scope resolution logic. Both handlers repeat the same "resolve_scope → 404 on None → build filter_params" block; extracting a small helper (e.g.
_resolve_workspace_filter_params(workspaces, workspace, allowed_partitions)returning(partitions, filter_params)or raising) would keep the two call sites in sync as the scoping rule evolves.
openrag/api/routers/user/search.py#L167-L174: extract the resolve_scope/404/filter_params block fromsearch_multiple_partitionsinto a shared helper.openrag/api/routers/user/search.py#L256-L259: reuse the same helper insearch_one_partitioninstead of re-implementing the pattern.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@openrag/api/routers/user/search.py` around lines 167 - 174, The workspace-scope resolution and filter construction are duplicated across both search handlers. In openrag/api/routers/user/search.py lines 167-174, extract the resolve_scope, 404 handling, partition narrowing, and filter_params construction into a shared helper; in lines 256-259, replace the duplicate logic in search_one_partition with that helper, preserving the existing return values and error behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@openrag/api/routers/user/search.py`:
- Around line 167-174: The workspace-scope resolution and filter construction
are duplicated across both search handlers. In
openrag/api/routers/user/search.py lines 167-174, extract the resolve_scope, 404
handling, partition narrowing, and filter_params construction into a shared
helper; in lines 256-259, replace the duplicate logic in search_one_partition
with that helper, preserving the existing return values and error behavior.
In `@openrag/services/storage/vector_store_searcher.py`:
- Around line 76-77: The filter_params merge can overwrite reserved
partition-scoping or raw-filter clauses. In
openrag/services/storage/vector_store_searcher.py lines 76-77 within search(),
guard or reject filter_params keys named "partition" and "expr" before updating
filters; apply the identical protection at lines 107-108 within
multi_query_search(), preserving the internally constructed filters.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b7d8d5cc-ffb7-4ed7-9b6f-545a9ceffc78
📒 Files selected for processing (20)
openrag/api/routers/user/search.pyopenrag/core/models/workspace.pyopenrag/core/retrieval/pipeline.pyopenrag/core/retrieval/retriever.pyopenrag/core/retrieval/searcher.pyopenrag/core/utils/exceptions.pyopenrag/services/orchestrators/mcp_service.pyopenrag/services/orchestrators/query_service.pyopenrag/services/orchestrators/retrieval_service.pyopenrag/services/orchestrators/workspace_service.pyopenrag/services/storage/milvus_store.pyopenrag/services/storage/vector_store_searcher.pytests/integration/repos/test_workspace_scoping_e2e.pytests/unit/api/routers/user/test_search.pytests/unit/core/retrieval/test_pipeline.pytests/unit/services/orchestrators/test_mcp_service.pytests/unit/services/orchestrators/test_query_service.pytests/unit/services/orchestrators/test_workspace_service.pytests/unit/services/storage/test_milvus_store.pytests/unit/services/storage/test_vector_store_searcher.py
Collaborator
Author
|
@CodeRabbit review |
✅ Action performedReview finished.
|
Collaborator
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
aditykris
force-pushed
the
fix/706-workspace-filter-params-dropped
branch
from
July 21, 2026 14:30
816c418 to
ea44c8a
Compare
hedhoud
approved these changes
Jul 22, 2026
hedhoud
left a comment
hedhoud
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed the current workspace scoping changes. The search and expansion paths are covered and look good.
hedhoud
self-requested a review
July 22, 2026 07:56
aditykris
force-pushed
the
fix/706-workspace-filter-params-dropped
branch
from
July 22, 2026 08:07
ea44c8a to
996adde
Compare
This was referenced Jul 22, 2026
This was referenced Jul 24, 2026
This was referenced Jul 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #706 — workspace-scoped search and chat validated the workspace but never actually restricted results to it.
filter_paramswas threaded through every layer of retrieval but silently dropped atVectorStoreSearcher, the one place that builds the Milvus query — so a "workspace search" quietly ran against the whole partition. Root cause and fix details are in the issue.WorkspaceService.resolve_scope) that resolvesworkspace_id → (partition, file_ids)via Postgres and fails closed on missing/inaccessible workspacesVectorStoreSearcher.search/multi_query_searchto actually mergefilter_paramsinto the Milvus query (dense + both legs of hybrid)openrag-all+ workspace can't match a same-named file in another partitionWorkspaceNotFoundError(404 for REST search, structured error for both streaming and non-streaming chat)filter_paramsdidn't work; both now bind through it properlyfalseliteral, which Milvus 2.6 rejects outright — changed to1 == 0Test plan
uv run pytest tests/unit/— 1872 passeduv run pytest tests/integration/repos/against a real Postgres + Milvus stack — all workspace/Milvus-related suites pass (2 pre-existing, unrelated failures intest_partition_repo.py)test_workspace_scoping_e2e.py): two partitions, a file sharing the samefile_idacross both, a workspace scoped to one — confirms cross-partition search returns only the in-scope file, and that removing the file returns zero results rather than falling back to the full partitionuv run ruff check/uv run ruff format --check— cleanSummary by CodeRabbit
file_idusing parameterized scoping (no hard-coded filter strings), while preserving any caller filter.