Skip to content

fix(search): restore ContextMemory search for owners and shared principals - #30593

Merged
TeddyCr merged 4 commits into
mainfrom
pmbrull/private-memory-search-missing
Jul 28, 2026
Merged

fix(search): restore ContextMemory search for owners and shared principals#30593
TeddyCr merged 4 commits into
mainfrom
pmbrull/private-memory-search-missing

Conversation

@pmbrull

@pmbrull pmbrull commented Jul 28, 2026

Copy link
Copy Markdown
Member

Fixes #30596

Describe your changes:

I worked on restoring ContextMemory search for owners and shared principals because #29541 kept PRIVATE/SHARED memories out of the search index entirely, and GET /contextCenter/memories serves the ContextCenter listing from search whenever it is given a query, filter, sort or offset — which the UI always sends (ContextCenterMemoriesPage.tsx:178, ContextCenterDashboardPage.tsx:218). The result was that a user could not see or find their own private memories anywhere in the ContextCenter. This PR indexes every memory again and enforces shareConfig visibility per query instead, making the request builders fail closed so a search path that forgets to resolve the caller cannot leak.

Type of change:

  • Bug fix

High-level design:

Why not simply revert #29541. The index-time gate was the only thing preventing several unfiltered search paths from returning other users' memories. Reverting it alone would have reopened them, so the query-time model had to be completed first.

Enforcement now has one rule and one fallback. ContextMemorySearchVisibility (unchanged in behaviour) filters memories to visibility == Entity OR owner OR shared-with-subject. {Elastic,Open}SearchRequestBuilder.build() — the choke point every wrapped search funnels through — ANDs in a new fail-closed org-wide-only filter unless the caller marked the request resolved via contextMemoryVisibilityResolved(). applyContextMemoryVisibility marks it only when the subject is actually identifiable (isSubjectResolvable), so internal callers passing a null subject fall back to the restrictive default rather than searching unfiltered.

Paths that build raw requests bypass that choke point, so they call restrictToOrgWideMemories(...) explicitly: searchByField, searchBySourceUrl, both aggregation managers, the hierarchy rewrite (whose minimumShouldMatch(1) had demoted the filter to a should sibling), the entity-relationship traversals, and EsUtils/OsUtils lineage. getDocByID runs no query at all, so it checks ContextMemorySearchVisibility.isOrgWideReadable on the fetched document.

Vector/RAG deliberately stays restricted. New EntityRepository.isVectorEmbeddable (defaults to isSearchIndexable, overridden only by ContextMemoryRepository) keeps non-org-wide memories out of the vector index on both the live path and the reindex bulk sinks, and clearEntityEmbedding removes the stale entity-doc embedding on an Entity → Private flip that a partial merge would otherwise leave kNN-matchable. This is required rather than preferred: vector chunk documents carry neither visibility nor sharedWithIds, VectorSearchQueryBuilder has a closed filter allowlist, and VectorIndexService.search(...) takes no SubjectContext — there is nothing to filter on and nowhere to filter. Behaviour there is unchanged from #29541, so this is not a regression.

Rollout. Memories created while #29541 was in effect are absent from the index and are only written on their next update — a search reindex of contextMemory is needed to backfill them. ContextMemoryRepository.getReindexFilter() no longer restricts by visibility, so reader and counter stay balanced and the staged index still promotes. ListFilter.MEMORY_SEARCH_VISIBILITY_PARAM is removed as dead. No schema or data migration.

Tests:

Use cases covered

  • An owner finds their own PRIVATE memory through the search-backed /contextCenter/memories listing (q, sortBy and offset variants) and through global search.
  • A principal in sharedWith finds a SHARED memory the same way.
  • A non-admin never sees another user's PRIVATE/SHARED memory on any search path.
  • Flipping visibility keeps the document indexed and moves it in and out of each subject's result set.
  • A full reindex rebuilds memories of every visibility.

Unit tests

  • I added unit tests for the new/changed logic.
  • Added SearchRequestBuilderMemoryVisibilityTest (fail-closed default in both request builders; only an identified subject may mark a request resolved).
  • Updated ContextMemorySearchVisibilityTest (org-wide-only filter shape, should-only outer bool so minimum_should_match stays 1, isOrgWideReadable), ContextMemoryRepositoryTest, ListFilterTest, VectorEmbeddingHandlerTest, both *BulkSinkBehaviorTest.
  • mvn test -pl openmetadata-service6980 run, 0 failures. Note this is better than main: the same suite on origin/main fails 6 SearchRepositoryTest tests, because fix(search): enforce ContextMemory visibility at index time, not per-query #29541 made Entity.isSearchIndexable consult ENTITY_REPOSITORY_MAP while EntityCsvTest/RuleEvaluatorTest register never-torn-down mock repositories that answer false. Fixed here by stubbing the policy hooks on those mocks.

Backend integration tests

  • I added integration tests in openmetadata-integration-tests/.
  • ContextMemoryIT.restrictedMemoriesAreSearchableByOwnerAndSharedPrincipalOnly and ContextMemoryIT.memoryStaysInSearchIndexAcrossVisibilityFlips. createMinimalRequest/createRequest no longer force ENTITY, so BaseEntityIT's generic search-index tests now themselves cover a restricted memory reaching the index. These two require the Docker stack and were not run locally.

Ingestion integration tests

  • Not applicable (no ingestion changes).

Playwright (UI) tests

  • Not applicable (no UI changes).

Manual testing performed

Ran against a local stack (Postgres + OpenSearch) with an admin token and a second, non-admin principal, A/B against a server built from main:

  1. Created PRIVATE, SHARED, org-wide, non-admin-owned and shared-with-non-admin memories; confirmed all five reach context_memory_search_index, and that only the org-wide one carries an embedding field.
  2. /contextCenter/memories with sortBy / offset / q: admin sees all five; the non-admin sees exactly its own private, the one shared with it, and the org-wide one.
  3. Confirmed closed on this branch and leaking on main: GET /lineage/getLineage?fqn=<private-memory> returned the memory's answer text on main and is indistinguishable from "not found" here; getPlatformLineage?view=all, /search/fieldQuery (index=contextMemory and index=all), /search/get/{index}/doc/{id}, /search/aggregate and /search/entityTypeCounts likewise.
  4. Verified no collateral damage: /search/query and /search/fieldQuery against the user and tag indices return byte-identical results on both builds.
  5. Triggered a contextMemory-scoped reindex and confirmed PRIVATE/SHARED memories are backfilled and returned by the search-backed listing.

UI screen recording / screenshots:

Not applicable.

Checklist:

  • I have read the CONTRIBUTING document.

  • My PR title is Fixes <issue-number>: <short explanation>

  • My PR is linked to a GitHub issue via Fixes #<issue-number> above.

  • I have commented on my code, particularly in hard-to-understand areas.

  • For JSON Schema changes: I updated the migration scripts or explained why it is not needed.

  • For UI changes: I attached a screen recording and/or screenshots above.

  • I have added tests (unit / integration / Playwright as applicable) and listed them above.

  • I have added a test that covers the exact scenario we are fixing.

Greptile Summary

This PR restores keyword-search indexing and subject-aware retrieval of restricted ContextMemory documents.

  • Separates keyword-search indexing eligibility from vector-embedding eligibility.
  • Adds fail-closed ContextMemory visibility handling across Elasticsearch and OpenSearch request paths.
  • Updates live indexing, bulk reindexing, aggregation, lineage, direct-document retrieval, and visibility tests.

Confidence Score: 3/5

The PR is not yet safe to merge because hierarchy search still hides authorized PRIVATE and SHARED memories from owners and shared principals.

The hierarchy rewrite places the subject-aware query inside a should group and then applies an unconditional org-wide-only filter, so restricted memories remain excluded even when the caller owns them or is explicitly shared on them.

Files Needing Attention: openmetadata-service/src/main/java/org/openmetadata/service/search/elasticsearch/ElasticSearchSearchManager.java; openmetadata-service/src/main/java/org/openmetadata/service/search/opensearch/OpenSearchSearchManager.java

Important Files Changed

Filename Overview
openmetadata-service/src/main/java/org/openmetadata/service/search/elasticsearch/ElasticSearchSearchManager.java Applies subject-aware or fail-closed ContextMemory visibility across Elasticsearch search paths and hierarchy rewriting.
openmetadata-service/src/main/java/org/openmetadata/service/search/opensearch/OpenSearchSearchManager.java Mirrors the Elasticsearch ContextMemory visibility enforcement for OpenSearch.
openmetadata-service/src/main/java/org/openmetadata/service/search/security/ContextMemorySearchVisibility.java Centralizes subject-aware and org-wide-only ContextMemory query filters.
openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/ContextMemoryRepository.java Makes every memory keyword-searchable while limiting vector embeddings to org-wide memories.
openmetadata-service/src/main/java/org/openmetadata/service/Entity.java Adds repository-driven vector-embedding eligibility alongside search-index eligibility.

Reviews (3): Last reviewed commit: "fix(search): resolve subject for entityT..." | Re-trigger Greptile

#29541 kept PRIVATE/SHARED memories out of the search index, but
GET /contextCenter/memories serves the ContextCenter listing from
search whenever it is given a query, filter, sort or offset — which
the UI always sends. Owners stopped finding their own memories and
shared principals stopped finding the ones shared with them.

Index every memory again and enforce shareConfig visibility per
query instead. The request builders now fail closed: a search path
that never resolved the caller's subject gets org-wide memories
only, so a forgotten call site cannot leak. Paths that build raw
requests — field query, aggregations, lineage and entity-relationship
traversals, doc-by-id — apply the same restriction explicitly.

Restricted memories stay out of the vector index behind a new
isVectorEmbeddable hook. Vector chunk documents carry no visibility
fields and the vector query path takes no subject, so it cannot be
filtered per caller.

Also stubs the indexing policy on globally registered mock
repositories, which left SearchRepositoryTest failing on main.

Refs #29541

Co-Authored-By: Claude <noreply@anthropic.com>
@pmbrull
pmbrull requested a review from a team as a code owner July 28, 2026 14:19
Copilot AI review requested due to automatic review settings July 28, 2026 14:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

✅ PR checks passed

The linked issue has a description and all required Shipping project fields set. Thanks!

@github-actions github-actions Bot added Ingestion safe to test Add this label to run secure Github workflows on PRs labels Jul 28, 2026
@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

✅ Playwright Results — workflow succeeded

Validated commit 3c57cc4d8da8788f1c35afa8a51044e7c186bbbe in Playwright run 30377304577, attempt 1.

✅ 1385 passed · ❌ 0 failed · 🟡 5 flaky · ⏭️ 5 skipped · 🧰 0 lifecycle flaky

Performance

Blocking targets: ✅ met · Optimization targets: 🟡 in progress

Shard-job maxima below are not the full workflow wall time; the linked run includes build, fixture, planning, and reporting.

🕒 Full workflow signal wall (to summary) 54m 18s

⏱️ Max setup 3m 6s · max shard execution 16m 24s · max shard-job elapsed before upload 21m 14s · reporting 9s

🌐 180.91 requests/attempt · 2.13 app boots/UI scenario · 10.37% common-shard skew

Optimization targets still in progress:

  • Application boot ratio was 2.13 per UI scenario (3077 boots / 1442 scenarios; convergence target: at most 1).
Shard Passed Failed Flaky Skipped Lifecycle failed Lifecycle flaky
✅ Shard chromium-01 146 0 0 0 0 0
🟡 Shard chromium-02 133 0 2 0 0 0
✅ Shard chromium-03 134 0 0 0 0 0
✅ Shard chromium-04 130 0 0 0 0 0
✅ Shard chromium-05 141 0 0 0 0 0
🟡 Shard chromium-06 172 0 1 0 0 0
✅ Shard chromium-07 126 0 0 3 0 0
🟡 Shard chromium-08 237 0 2 0 0 0
✅ Shard data-asset-rules-01 61 0 0 0 0 0
✅ Shard domain-isolation-01 14 0 0 0 0 0
✅ Shard global-state-01 23 0 0 0 0 0
✅ Shard ingestion-01 2 0 0 0 0 0
✅ Shard reindex-01 28 0 0 0 0 0
✅ Shard search-01 11 0 0 0 0 0
✅ Shard search-rbac-01 27 0 0 2 0 0
🟡 5 flaky test(s) (passed on retry)
  • Pages/CustomProperties.spec.tsShould verify property name is visible for metric in right panel (shard chromium-02, 1 retry)
  • Pages/Entity.spec.tsDomain Propagation (shard chromium-02, 1 retry)
  • Pages/CustomProperties.spec.tsShould clear search and show all properties for apiCollection in right panel (shard chromium-06, 1 retry)
  • Features/ContextCenterMemories.spec.tsediting memory type persists after save (shard chromium-08, 1 retry)
  • Features/ContextCenterMemories.spec.tsadding a linked asset in edit mode shows entity badge on the row (shard chromium-08, 1 retry)

📦 Download artifacts

How to debug locally
# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip    # view trace

pmbrull and others added 2 commits July 28, 2026 17:27
#30528 marked these describe.fixme after the memory listing stopped
returning PRIVATE/SHARED entries. That was the #29541 regression this
branch fixes, not a test problem — the suites build Private, Shared
and Entity fixtures and assert what a dataConsumer sees.

CustomizeDetailPage stays fixme; it is unrelated to this change.

Refs #30528

Co-Authored-By: Claude <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 28, 2026 15:29
@pmbrull
pmbrull requested a review from a team as a code owner July 28, 2026 15:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

getEntityTypeCounts built through the request builder without ever
resolving the caller, so the fail-closed org-wide-only default always
applied — even for an admin or a memory's owner. The contextMemory
count then disagreed with the search-backed /contextCenter/memories
listing, which does show those callers their PRIVATE and SHARED
memories.

SearchResource already had the SubjectContext and used it only for
domains; thread it down to the aggregation managers and apply the
same per-subject filter the listing uses. The subject-less overload
stays and still fails closed, so an unidentifiable caller is
unaffected.

Co-Authored-By: Claude <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 28, 2026 16:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@sonarqubecloud

Copy link
Copy Markdown

@TeddyCr
TeddyCr merged commit d864c5b into main Jul 28, 2026
122 checks passed
@TeddyCr
TeddyCr deleted the pmbrull/private-memory-search-missing branch July 28, 2026 17:54
@gitar-bot

gitar-bot Bot commented Jul 28, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 1 resolved / 1 findings

Restores keyword-search indexing for restricted ContextMemory documents while enforcing subject visibility at query time, addressing the entityTypeCounts undercount finding. No issues found.

✅ 1 resolved
Edge Case: entityTypeCounts undercounts contextMemory for admins/owners

📄 openmetadata-service/src/main/java/org/openmetadata/service/search/elasticsearch/ElasticSearchAggregationManager.java:584-598 📄 openmetadata-service/src/main/java/org/openmetadata/service/search/opensearch/OpenSearchAggregationManager.java:475-489
Because getEntityTypeCounts builds through {Elastic,Open}SearchRequestBuilder.build() without ever calling contextMemoryVisibilityResolved(), the fail-closed org-wide-only filter is always ANDed in — even for an admin or a memory's owner. This is correct as a safety default (aggregations carry no SubjectContext), but it means the contextMemory count returned to the ContextCenter will only ever reflect ENTITY-visibility memories, while the search-backed /contextCenter/memories listing shows admins/owners their PRIVATE and SHARED memories too. The count badge will therefore disagree with the visible list. If exact per-subject counts matter for the UI, consider resolving the subject on this path (like the listing does) or documenting the intentional undercount; otherwise this is an acceptable fail-closed trade-off.

Options

Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar | Powered by Gitar — free for open source

pmbrull added a commit that referenced this pull request Jul 29, 2026
…ipals (#30593)

* fix(search): restore ContextMemory search for owners and shared users

#29541 kept PRIVATE/SHARED memories out of the search index, but
GET /contextCenter/memories serves the ContextCenter listing from
search whenever it is given a query, filter, sort or offset — which
the UI always sends. Owners stopped finding their own memories and
shared principals stopped finding the ones shared with them.

Index every memory again and enforce shareConfig visibility per
query instead. The request builders now fail closed: a search path
that never resolved the caller's subject gets org-wide memories
only, so a forgotten call site cannot leak. Paths that build raw
requests — field query, aggregations, lineage and entity-relationship
traversals, doc-by-id — apply the same restriction explicitly.

Restricted memories stay out of the vector index behind a new
isVectorEmbeddable hook. Vector chunk documents carry no visibility
fields and the vector query path takes no subject, so it cannot be
filtered per caller.

Also stubs the indexing policy on globally registered mock
repositories, which left SearchRepositoryTest failing on main.

Refs #29541

Co-Authored-By: Claude <noreply@anthropic.com>

* test(playwright): re-enable ContextCenter memory suites

#30528 marked these describe.fixme after the memory listing stopped
returning PRIVATE/SHARED entries. That was the #29541 regression this
branch fixes, not a test problem — the suites build Private, Shared
and Entity fixtures and assert what a dataConsumer sees.

CustomizeDetailPage stays fixme; it is unrelated to this change.

Refs #30528

Co-Authored-By: Claude <noreply@anthropic.com>

* fix(search): resolve subject for entityTypeCounts

getEntityTypeCounts built through the request builder without ever
resolving the caller, so the fail-closed org-wide-only default always
applied — even for an admin or a memory's owner. The contextMemory
count then disagreed with the search-backed /contextCenter/memories
listing, which does show those callers their PRIVATE and SHARED
memories.

SearchResource already had the SubjectContext and used it only for
domains; thread it down to the aggregation managers and apply the
same per-subject filter the listing uses. The subject-less overload
stays and still fails closed, so an unidentifiable caller is
unaffected.

Co-Authored-By: Claude <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Ingestion safe to test Add this label to run secure Github workflows on PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Restore Context Memory visibility issues

3 participants