feat: secure memory queries and implement pgvector semantic search#3
Open
ApoorvSaxena0109 wants to merge 1 commit into
Open
feat: secure memory queries and implement pgvector semantic search#3ApoorvSaxena0109 wants to merge 1 commit into
ApoorvSaxena0109 wants to merge 1 commit into
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates the MemoryStore query building to use parameterized SQL (reducing SQL injection risk) and expands the semantic search tests to cover MemoryStore CRUD and search behavior.
Changes:
- Reworked
getMemories,searchMemories, andsemanticSearchto use$nplaceholders withsql.unsafe(query, values)instead of interpolating user input into SQL. - Added hybrid semantic+keyword search behavior to
searchMemorieswith embedding-generation fallback. - Expanded test coverage with a mock DB helper and CRUD/search tests for MemoryStore.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/memory/semantic_search.test.ts | Adds DB mocking + MemoryStore CRUD/search tests, including parameterization assertions and hybrid search coverage. |
| src/memory/store.ts | Converts dynamic SQL conditions to placeholder-based parameterization and implements hybrid semantic search/fallback logic. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+243
to
+246
| const rows = await this.sql.unsafe( | ||
| `SELECT * FROM memories WHERE ${whereClause} ORDER BY ${sortColumn} ${order} LIMIT ${limit} OFFSET ${offset}`, | ||
| values | ||
| ) as MemoryRow[]; |
Comment on lines
688
to
696
| SELECT | ||
| *, | ||
| 1 - (embedding <=> ${JSON.stringify(queryEmbedding)}::vector) as similarity | ||
| 1 - (embedding <=> $${paramIdx}::vector) as similarity | ||
| FROM memories | ||
| WHERE ${this.sql.unsafe(whereClause)} | ||
| AND 1 - (embedding <=> ${JSON.stringify(queryEmbedding)}::vector) >= ${minSimilarity} | ||
| WHERE ${vectorWhere} | ||
| AND 1 - (embedding <=> $${paramIdx}::vector) >= ${minSimilarity} | ||
| ORDER BY similarity DESC | ||
| LIMIT ${limit} | ||
| `; |
| const whereClause = conditions.join(' AND '); | ||
| const sortColumn = sortBy === 'createdAt' ? 'created_at' : | ||
| sortBy === 'importance' ? "metadata->>'importance'" : 'created_at'; | ||
| sortBy === 'importance' ? "(metadata->>'importance')::int" : 'created_at'; |
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
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.
Pull Request
Description
This PR secures the database query interface for ClawKeeper's agent memory store and implements true hybrid semantic search utilizing pgvector to fulfill the inline
TODOinsidesearchMemories().getMemories(),searchMemories(), andsemanticSearch()with strictly bound parameterized placeholders ($1,$2, etc.), protecting the multi-tenant database layer against SQL Injection.embedding_serviceinto the primarysearchMemories()handler. If querytextis provided, we perform a vector similarity search (using corrected parameter-specific::vectorcasting) and a keyword fallback search, merging and ranking results based on relevance/similarity scores.Fixes # (No associated issue, open-source repository maturity task)
Type of Change
Changes Made
src/memory/store.ts:getMemories,searchMemories, andsemanticSearchby using parameterized values instead of template string interpolation.searchMemorieswhen search text is specified, supporting sorting byrelevance(similarity),importance, andcreatedAt.(embedding <=> $param)::vectorto(embedding <=> $param::vector).tests/memory/semantic_search.test.ts:MemoryStoreCRUD operations using a mocked database client.Testing
Describe the testing you performed:
Test details:
bun run testlocally. All 139 tests passed successfully (including all 7 newMemoryStoreoperations tests).tsc --noEmit(0 compilation/type errors).eslint(0 warnings or errors).Checklist
Security Considerations
Screenshots (if applicable)
N/A (Backend logic change)
Additional Notes
sql.unsafe()with variable parameters arrays, ensuring complete safety from SQL injection vectors.1 - (embedding <=> $param::vector)) are correctly cast to avoid runtime syntax exceptions.Reviewers
@Alexi5000