Fix semantic search MCP tool leaking embedding vectors into results by borisno2 · Pull Request #1553 · OpenSaasAU/stack

borisno2 · 2026-09-13T21:37:31Z

Summary

The semantic_search_<list> MCP tool built its result by spreading the whole matched row ({ ...match.item, _similarity: match.score }), which included every embedding field's stored vector — 1,536 floats per result on the default OpenAI provider — plus that field's metadata sibling.
This burned context on every search, cost tokens, and could leak a column that a field-level access.read rule was meant to restrict.
Fixed by stripping all of this list's embedding-typed fields (not just the one searched) out of each match's row before spreading it into the result.
Checked the RAG package's other search paths (semanticSearch, findSimilar in runtime/search.ts) — neither spreads a row itself; they return SearchResult<TRow>[] and leave projection to the caller, so no equivalent fix was needed there.

Test plan

Added a regression test (plugin.test.ts) with a nearest() double returning a row carrying two embedding fields, asserting the tool's result keeps the plain fields and _similarity but drops both embedding columns and their metadata.
Verified the new test fails without the fix (by temporarily reverting plugin.ts) and passes with it.
pnpm build in packages/core, packages/ui, packages/rag
pnpm test in packages/rag — 258/258 passing
pnpm lint, pnpm manypkg fix, pnpm format at the repo root
Added a changeset (.changeset/thirty-otters-search.md, patch on @opensaas/stack-rag)

Closes #1297

🤖 Generated with Claude Code

https://claude.ai/code/session_01NZ2pqay76RJhiaV76BUi9a

Generated by Claude Code

The tool's result mapping spread the full row, including every embedding field's vector and metadata, into each search result — a real cost in context tokens and a leak of a column that may carry its own read access restriction. Strip embedding-typed fields out of each match before spreading. Closes #1297 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NZ2pqay76RJhiaV76BUi9a

changeset-bot · 2026-09-13T21:37:35Z

🦋 Changeset detected

Latest commit: 0c7a219

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 9 packages

Name	Type
@opensaas/stack-rag	Patch
@opensaas/stack-cli	Patch
@opensaas/stack-auth	Patch
@opensaas/stack-core	Patch
@opensaas/stack-storage-s3	Patch
@opensaas/stack-storage-vercel	Patch
@opensaas/stack-storage	Patch
@opensaas/stack-tiptap	Patch
@opensaas/stack-ui	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

chatgpt-codex-connector · 2026-09-13T21:37:35Z

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

vercel · 2026-09-13T21:37:37Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
stack-docs	Ready	Preview	Sep 14, 2026 6:16am UTC

borisno2 · 2026-09-13T21:39:35Z

The test check is failing on pnpm check:adr-duplicates, not on anything in this diff:

Duplicate ADR numbers found in docs/adr/:

  0070:
    - 0070-the-auth-plugins-id-strategy-is-config-not-a-hardcoded-pin.md
    - 0070-the-generated-clients-identity-is-a-process-wide-registry-entry.md

Both files already exist at main's current tip (2c000b1, this PR's own base commit) — that's #1550 and #1548 landing back-to-back and each claiming ADR number 0070. This PR touches nothing under docs/adr/, so it inherits the failure rather than causing it, and it isn't red on a check this PR's diff can fix.

No fix for it exists yet on main, so I'm not pushing one into this PR (would just widen it past #1297). The fix is a one-line rename: 0070-the-generated-clients-identity-is-a-process-wide-registry-entry.md → 0071-the-generated-clients-identity-is-a-process-wide-registry-entry.md (that ADR is the later of the two by commit time), with its own duplicate-ADR-number cross-references updated if any point at "0070".

Generated by Claude Code

borisno2 · 2026-09-13T21:40:41Z

Code Review Summary

Reviewed the diff in this PR (branch claude/jolly-turing-rpw4yi → main), fixing issue #1297: packages/rag/src/config/plugin.ts (the semantic_search_<list> MCP tool result construction) and the new regression test in packages/rag/src/config/plugin.test.ts.

What I checked:

The fix correctly omit()s embedding-typed fields (including companion embedding fields injected by searchable() via extendList, not just directly-declared embedding() fields) from match.item before it is spread into each search result.
embeddingFieldNames is computed once per list at plugin-init time, not per request — no per-row overhead added.
Confirmed match.item has already passed through core's Field Visibility inside nearest(), so this strip is an additive, MCP-context-specific projection rather than papering over a missing access-control layer — the right layer for this fix.
No existing helper duplicated (the admin UI's redactEmbeddingForClient serves a different, partial-redaction purpose and isn't a fit here).
A changeset is included per repo convention; no any/type casts introduced.
Ran tsc --noEmit and the full plugin.test.ts suite (52/52 passing), including the new regression test.

Verdict: Approve — ready to merge as-is. No correctness bugs or reuse/simplification/efficiency issues found at medium review effort.

Generated by Claude Code

github-actions · 2026-09-14T06:29:14Z