Skip to content

Enforce per-object ACLs on semantic search results - #1345

Merged
tomchop merged 2 commits into
mainfrom
fix/semantic-search-rbac
Aug 13, 2026
Merged

Enforce per-object ACLs on semantic search results#1345
tomchop merged 2 commits into
mainfrom
fix/semantic-search-rbac

Conversation

@tomchop

@tomchop tomchop commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • POST /search/semantic fetched candidate objects straight from Arango by id (cls.get(meta["id"])) with zero user/ACL check -- any authenticated caller got back every nearest-neighbor match regardless of what they actually have READ permission on. Harmless while nothing called this endpoint from the product; not fine once an agent (with a restricted service account) starts using it as a tool.
  • Filtering has to happen per-candidate, not as an endpoint-level guard: a @global_permission-style decorator only checks a blanket "can this user read anything" flag, not whether this specific object is one they can see. Reuses the same primitive as permission_on_target/permission_on_ids: user.has_permissions(extended_id, Permission.READ) (one ACL-graph traversal per candidate), same not RBAC_ENABLED or user.admin bypass as everywhere else.
  • Since ACL filtering happens after the similarity search, overfetches from Chroma (3x) when enforcement is active so dropped candidates don't shrink the result count below what was requested. This is a best-effort cushion, not a guarantee -- documented in the code and in the memory notes for this workstream.

Test plan

  • Existing tests/core_tests/chromadb_test.py suite converted from calling semantic_search() directly to going through an authenticated TestClient, since the endpoint now reads httpreq.state.user.
  • New test_semantic_search_respects_acls: two entities, ACL granted on only one, confirms the ungranted one never appears in results.
  • New test_semantic_search_admin_bypasses_acls: confirms an admin still sees everything regardless of ACLs (matches every other RBAC bypass in the codebase).
  • Full unittest suite (schemas, apiv2, core_tests) run locally -- no regressions (10 pre-existing failures are unrelated /opt/yeti/... permission issues in this sandbox).
  • ruff check/format --check and ty check clean on touched files.

Companion docker-compose.yaml change (persistent volume for the chromadb path, previously wiped on every container recreate) is in yeti-docker, not this repo.

/search/semantic fetched candidate objects straight from Arango by id
with no user/ACL check at all, so any authenticated caller could get back
objects they have no READ permission on -- ChromaDB's nearest-neighbor
lookup has no concept of ACLs, so filtering has to happen per-candidate
at fetch time, the same way permission_on_target/permission_on_ids do
for other endpoints (user.has_permissions(extended_id, Permission.READ)).

Since ACL filtering happens after the similarity search, overfetch from
Chroma when RBAC is enforced so dropped candidates don't shrink the
result count below what was asked for -- not a guarantee, just a
best-effort cushion; a user with narrow-enough visibility can still get
fewer results than requested.
ChromaDB's PersistentClient caches its underlying connection per Python
process (SharedSystemClient._identifier_to_system, a process-wide dict):
the first PersistentClient(path=...) call in a process gets cached and
reused for that process's lifetime, so a long-running process never sees
writes made by a *different* process against the same on-disk files.
The scheduled indexer runs in the celery worker; /search/semantic is
served by the API process -- without clearing the cache, the API process
would keep answering from whatever snapshot it had cached at its own
startup, oblivious to anything indexed afterwards, until it restarts.

Verified against a live index: querying from an already-running process
missed a document written seconds earlier by a separate process, and
clear_system_cache() fixed it with no measurable added latency.

Also surfaces each result's similarity as an explicit `semantic_score`
(1 - cosine distance, higher is better) instead of leaving relevance
purely implicit in list order.
@tomchop
tomchop merged commit fad11bc into main Aug 13, 2026
5 of 7 checks passed
@tomchop
tomchop deleted the fix/semantic-search-rbac branch August 13, 2026 21:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant