Skip to content

Commit 9572916

Browse files
committed
fix(store): comprehensive audit fixes — search, validation, types, perf
- Fix hybrid search early return: operator-only text in hybrid mode now falls back to vector search instead of returning empty - Fix bumpUsage not incrementing version (broke optimistic locking) - Fix likeEscape not escaping backslash (LIKE ESCAPE '\' vulnerability) - Fix double now() calls producing inconsistent timestamps in bulkMove, bulkPriority, bumpUsage - Add IN-clause chunking (SQL_CHUNK_SIZE=900) to prevent hitting SQLite SQLITE_MAX_VARIABLE_NUMBER limit in batch operations - Add LIMIT 10000 to listEdges to prevent unbounded queries - Add migration v002: composite index on attachments(project_id, graph, entity_id) - Add evictProject() to Store for cache invalidation after project deletion - Refactor KnowledgeStore.list() from positional args to NoteListOptions object - Wire FileNode.language/mimeType through updateFile via FileUpdateOptions - Add order field to EpicCreate/EpicPatch + reorder() method - Remove dead TagRecord type - Add assertEmbeddingDim() validation on all embedding inserts - Optimize setTags: reuse prepared statements instead of re-preparing per tag - Remove unused ROW_NUMBER() from vec0 search query
1 parent c0c9938 commit 9572916

20 files changed

Lines changed: 207 additions & 114 deletions

File tree

src/store/sqlite/lib/bigint.ts

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,34 @@ export function now(): bigint {
88
return BigInt(Date.now());
99
}
1010

11-
/** Escape LIKE special characters (%, _) for safe use in SQL LIKE patterns */
11+
/** Escape LIKE special characters (\, %, _) for safe use in SQL LIKE patterns with ESCAPE '\' */
1212
export function likeEscape(text: string): string {
13-
return text.replace(/[%_]/g, '\\$&');
13+
return text.replace(/\\/g, '\\\\').replace(/[%_]/g, '\\$&');
14+
}
15+
16+
import { EMBEDDING_DIM } from '../migrations/v001';
17+
18+
/** Assert that an embedding has the expected dimensionality */
19+
export function assertEmbeddingDim(embedding: number[]): void {
20+
if (embedding.length !== EMBEDDING_DIM) {
21+
throw new Error(`Embedding dimension mismatch: expected ${EMBEDDING_DIM}, got ${embedding.length}`);
22+
}
23+
}
24+
25+
/**
26+
* Max items per SQL IN-clause to stay within SQLite's SQLITE_MAX_VARIABLE_NUMBER.
27+
* Reserves a few slots for other query params.
28+
*/
29+
export const SQL_CHUNK_SIZE = 900;
30+
31+
/** Split an array into chunks of at most `size` elements */
32+
export function chunk<T>(arr: T[], size: number = SQL_CHUNK_SIZE): T[][] {
33+
if (arr.length <= size) return [arr];
34+
const result: T[][] = [];
35+
for (let i = 0; i < arr.length; i += size) {
36+
result.push(arr.slice(i, i + size));
37+
}
38+
return result;
1439
}
1540

1641
/** Safely parse JSON with a fallback for corrupted data */

src/store/sqlite/lib/edge-helper.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ export class EdgeHelper {
3636

3737
const where = conditions.length > 0 ? `WHERE ${conditions.join(' AND ')}` : '';
3838
const rows = this.db.prepare(
39-
`SELECT from_graph, from_id, to_graph, to_id, kind FROM edges ${where}`
39+
`SELECT from_graph, from_id, to_graph, to_id, kind FROM edges ${where} LIMIT 10000`
4040
).all(...params) as Array<Record<string, unknown>>;
4141

4242
return rows.map(r => ({

src/store/sqlite/lib/entity-helpers.ts

Lines changed: 37 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import Database from 'better-sqlite3';
22
import type { AttachmentMeta, Edge, GraphName } from '../../types';
3-
import { num } from './bigint';
3+
import { num, chunk } from './bigint';
44

55
/**
66
* Shared helpers for user-managed stores (knowledge, tasks, skills).
@@ -32,13 +32,15 @@ export class EntityHelpers {
3232
`).run(this.projectId, ...ids);
3333
}
3434

35-
// Insert new tags
35+
// Insert new tags — use INSERT OR IGNORE + SELECT per tag
36+
const insertTag = this.db.prepare('INSERT OR IGNORE INTO tags (project_id, name) VALUES (?, ?)');
37+
const selectTag = this.db.prepare('SELECT id FROM tags WHERE project_id = ? AND name = ?');
38+
const insertEdge = this.db.prepare(`INSERT INTO edges (project_id, from_graph, from_id, to_graph, to_id, kind) VALUES (?, 'tags', ?, ?, ?, 'tagged')`);
3639
for (const tag of tags) {
37-
this.db.prepare('INSERT OR IGNORE INTO tags (project_id, name) VALUES (?, ?)').run(this.projectId, tag);
38-
const row = this.db.prepare('SELECT id FROM tags WHERE project_id = ? AND name = ?').get(this.projectId, tag) as { id: bigint } | undefined;
40+
insertTag.run(this.projectId, tag);
41+
const row = selectTag.get(this.projectId, tag) as { id: bigint } | undefined;
3942
if (!row) throw new Error(`Failed to resolve tag: ${tag}`);
40-
this.db.prepare(`INSERT INTO edges (project_id, from_graph, from_id, to_graph, to_id, kind) VALUES (?, 'tags', ?, ?, ?, 'tagged')`)
41-
.run(this.projectId, num(row.id), graph, entityId);
43+
insertEdge.run(this.projectId, num(row.id), graph, entityId);
4244
}
4345
}
4446

@@ -58,20 +60,22 @@ export class EntityHelpers {
5860
if (entityIds.length === 0) return result;
5961
for (const id of entityIds) result.set(id, []);
6062

61-
const ph = entityIds.map(() => '?').join(',');
62-
const rows = this.db.prepare(`
63-
SELECT e.to_id AS entity_id, t.name
64-
FROM edges e
65-
JOIN tags t ON t.id = e.from_id AND t.project_id = e.project_id
66-
WHERE e.project_id = ? AND e.from_graph = 'tags' AND e.to_graph = ? AND e.kind = 'tagged'
67-
AND e.to_id IN (${ph})
68-
ORDER BY t.name
69-
`).all(this.projectId, graph, ...entityIds) as Array<{ entity_id: bigint; name: string }>;
70-
71-
for (const r of rows) {
72-
const id = num(r.entity_id);
73-
const arr = result.get(id);
74-
if (arr) arr.push(r.name);
63+
for (const batch of chunk(entityIds)) {
64+
const ph = batch.map(() => '?').join(',');
65+
const rows = this.db.prepare(`
66+
SELECT e.to_id AS entity_id, t.name
67+
FROM edges e
68+
JOIN tags t ON t.id = e.from_id AND t.project_id = e.project_id
69+
WHERE e.project_id = ? AND e.from_graph = 'tags' AND e.to_graph = ? AND e.kind = 'tagged'
70+
AND e.to_id IN (${ph})
71+
ORDER BY t.name
72+
`).all(this.projectId, graph, ...batch) as Array<{ entity_id: bigint; name: string }>;
73+
74+
for (const r of rows) {
75+
const id = num(r.entity_id);
76+
const arr = result.get(id);
77+
if (arr) arr.push(r.name);
78+
}
7579
}
7680
return result;
7781
}
@@ -92,17 +96,19 @@ export class EntityHelpers {
9296
if (entityIds.length === 0) return result;
9397
for (const id of entityIds) result.set(id, []);
9498

95-
const ph = entityIds.map(() => '?').join(',');
96-
const rows = this.db.prepare(`
97-
SELECT entity_id, filename, mime_type, size, url, added_at FROM attachments
98-
WHERE project_id = ? AND graph = ? AND entity_id IN (${ph})
99-
ORDER BY added_at
100-
`).all(this.projectId, graph, ...entityIds) as Array<Record<string, unknown>>;
101-
102-
for (const r of rows) {
103-
const id = num(r.entity_id as bigint);
104-
const arr = result.get(id);
105-
if (arr) arr.push(this.toAttachmentMeta(r));
99+
for (const batch of chunk(entityIds)) {
100+
const ph = batch.map(() => '?').join(',');
101+
const rows = this.db.prepare(`
102+
SELECT entity_id, filename, mime_type, size, url, added_at FROM attachments
103+
WHERE project_id = ? AND graph = ? AND entity_id IN (${ph})
104+
ORDER BY added_at
105+
`).all(this.projectId, graph, ...batch) as Array<Record<string, unknown>>;
106+
107+
for (const r of rows) {
108+
const id = num(r.entity_id as bigint);
109+
const arr = result.get(id);
110+
if (arr) arr.push(this.toAttachmentMeta(r));
111+
}
106112
}
107113
return result;
108114
}

src/store/sqlite/lib/search.ts

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -80,16 +80,18 @@ export function hybridSearch(
8080
// FTS5 keyword search
8181
if (mode !== 'vector' && query.text) {
8282
const escaped = ftsEscape(query.text);
83-
if (!escaped) return [];
84-
const rows = db.prepare(`
85-
SELECT p.${config.parentIdColumn} AS id, ROW_NUMBER() OVER (ORDER BY rank) AS rn
86-
FROM ${config.ftsTable} fts
87-
JOIN ${config.parentTable} p ON p.${config.parentIdColumn} = fts.rowid AND p.project_id = ? ${extraJoin}
88-
WHERE ${config.ftsTable} MATCH ?
89-
LIMIT ?
90-
`).all(projectId, escaped, topK) as Array<{ id: bigint; rn: bigint }>;
91-
92-
ftsRanked = rows.map(r => ({ id: num(r.id), rn: num(r.rn) }));
83+
if (!escaped && mode === 'keyword') return [];
84+
if (escaped) {
85+
const rows = db.prepare(`
86+
SELECT p.${config.parentIdColumn} AS id, ROW_NUMBER() OVER (ORDER BY rank) AS rn
87+
FROM ${config.ftsTable} fts
88+
JOIN ${config.parentTable} p ON p.${config.parentIdColumn} = fts.rowid AND p.project_id = ? ${extraJoin}
89+
WHERE ${config.ftsTable} MATCH ?
90+
LIMIT ?
91+
`).all(projectId, escaped, topK) as Array<{ id: bigint; rn: bigint }>;
92+
93+
ftsRanked = rows.map(r => ({ id: num(r.id), rn: num(r.rn) }));
94+
}
9395
}
9496

9597
// vec0 vector search
@@ -99,11 +101,11 @@ export function hybridSearch(
99101
const vecK = topK * 3;
100102

101103
const rows = db.prepare(`
102-
SELECT v.rowid AS id, v.distance, ROW_NUMBER() OVER (ORDER BY v.distance) AS rn
104+
SELECT v.rowid AS id, v.distance
103105
FROM ${config.vecTable} v
104106
JOIN ${config.parentTable} p ON p.${config.parentIdColumn} = v.rowid AND p.project_id = ? ${extraJoin}
105107
WHERE v.embedding MATCH ? AND v.k = ?
106-
`).all(projectId, embeddingBuf, vecK) as Array<{ id: bigint; distance: number; rn: bigint }>;
108+
`).all(projectId, embeddingBuf, vecK) as Array<{ id: bigint; distance: number }>;
107109

108110
vecRanked = rows.slice(0, topK).map((r, i) => ({ id: num(r.id), rn: i + 1 }));
109111
}

src/store/sqlite/migrations/v001.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import type { Migration } from '../lib/migrate';
22

3-
const EMBEDDING_DIM = 384;
3+
export const EMBEDDING_DIM = 384;
44

55
export const v001: Migration = {
66
version: 1,
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
import type { Migration } from '../lib/migrate';
2+
3+
export const v002: Migration = {
4+
version: 2,
5+
sql: `
6+
-- Composite index for attachment lookups (project_id, graph, entity_id)
7+
CREATE INDEX IF NOT EXISTS idx_attachments_entity ON attachments(project_id, graph, entity_id);
8+
`,
9+
};

src/store/sqlite/store.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,12 @@ import { runMigrations } from './lib/migrate';
1414
import { MetaHelper } from './lib/meta';
1515
import { EdgeHelper } from './lib/edge-helper';
1616
import { v001 } from './migrations/v001';
17+
import { v002 } from './migrations/v002';
1718
import { SqliteTeamStore } from './stores/team';
1819
import { SqliteProjectsStore } from './stores/projects';
1920
import { SqliteProjectScopedStore } from './stores/project-scoped';
2021

21-
const ALL_MIGRATIONS = [v001];
22+
const ALL_MIGRATIONS = [v001, v002];
2223

2324
export class SqliteStore implements Store {
2425
private db: Database.Database | null = null;
@@ -76,6 +77,11 @@ export class SqliteStore implements Store {
7677
return scoped;
7778
}
7879

80+
/** Evict a project from the scoped store cache (call after project deletion) */
81+
evictProject(projectId: number): void {
82+
this.scopedCache.delete(projectId);
83+
}
84+
7985
// --- Edges ---
8086

8187
createEdge(projectId: number, edge: Edge): void {

src/store/sqlite/stores/epics.ts

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import type {
1616
import { VersionConflictError } from '../../types';
1717
import { MetaHelper } from '../lib/meta';
1818
import { EntityHelpers } from '../lib/entity-helpers';
19-
import { num, now, likeEscape } from '../lib/bigint';
19+
import { num, now, likeEscape, chunk, assertEmbeddingDim } from '../lib/bigint';
2020
import { hybridSearch, SearchConfig } from '../lib/search';
2121

2222
const GRAPH = 'epics';
@@ -81,20 +81,22 @@ export class SqliteEpicsStore implements EpicsStore {
8181
if (epicIds.length === 0) return result;
8282
for (const id of epicIds) result.set(id, { total: 0, done: 0 });
8383

84-
const ph = epicIds.map(() => '?').join(',');
85-
const rows = this.db.prepare(`
86-
SELECT e.from_id AS epic_id,
87-
COALESCE(SUM(CASE WHEN t.status != 'cancelled' THEN 1 ELSE 0 END), 0) AS total,
88-
COALESCE(SUM(CASE WHEN t.status = 'done' THEN 1 ELSE 0 END), 0) AS done
89-
FROM edges e
90-
JOIN tasks t ON t.id = e.to_id AND t.project_id = e.project_id
91-
WHERE e.project_id = ? AND e.from_graph = 'epics' AND e.to_graph = 'tasks' AND e.kind = 'belongs_to'
92-
AND e.from_id IN (${ph})
93-
GROUP BY e.from_id
94-
`).all(this.projectId, ...epicIds) as Array<{ epic_id: bigint; total: bigint; done: bigint }>;
95-
96-
for (const r of rows) {
97-
result.set(num(r.epic_id), { total: num(r.total), done: num(r.done) });
84+
for (const batch of chunk(epicIds)) {
85+
const ph = batch.map(() => '?').join(',');
86+
const rows = this.db.prepare(`
87+
SELECT e.from_id AS epic_id,
88+
COALESCE(SUM(CASE WHEN t.status != 'cancelled' THEN 1 ELSE 0 END), 0) AS total,
89+
COALESCE(SUM(CASE WHEN t.status = 'done' THEN 1 ELSE 0 END), 0) AS done
90+
FROM edges e
91+
JOIN tasks t ON t.id = e.to_id AND t.project_id = e.project_id
92+
WHERE e.project_id = ? AND e.from_graph = 'epics' AND e.to_graph = 'tasks' AND e.kind = 'belongs_to'
93+
AND e.from_id IN (${ph})
94+
GROUP BY e.from_id
95+
`).all(this.projectId, ...batch) as Array<{ epic_id: bigint; total: bigint; done: bigint }>;
96+
97+
for (const r of rows) {
98+
result.set(num(r.epic_id), { total: num(r.total), done: num(r.done) });
99+
}
98100
}
99101
return result;
100102
}
@@ -104,14 +106,16 @@ export class SqliteEpicsStore implements EpicsStore {
104106
// =========================================================================
105107

106108
create(data: EpicCreate, embedding: number[]): EpicRecord {
109+
assertEmbeddingDim(embedding);
107110
const slug = randomUUID();
108111
const ts = now();
109112
const authorId = data.authorId ?? null;
110113

114+
const order = data.order ?? this.nextOrder();
111115
const result = this.db.prepare(`
112116
INSERT INTO epics (project_id, slug, title, description, status, priority, "order", version, created_by_id, updated_by_id, created_at, updated_at)
113117
VALUES (?, ?, ?, ?, ?, ?, ?, 1, ?, ?, ?, ?)
114-
`).run(this.projectId, slug, data.title, data.description ?? '', data.status ?? 'open', data.priority ?? 'medium', this.nextOrder(), authorId, authorId, ts, ts);
118+
`).run(this.projectId, slug, data.title, data.description ?? '', data.status ?? 'open', data.priority ?? 'medium', order, authorId, authorId, ts, ts);
115119
const id = result.lastInsertRowid;
116120

117121
this.db.prepare('INSERT INTO epics_vec (rowid, embedding) VALUES (?, ?)').run(BigInt(id as number | bigint), Buffer.from(new Float32Array(embedding).buffer));
@@ -138,6 +142,7 @@ export class SqliteEpicsStore implements EpicsStore {
138142
if (patch.description !== undefined) set('description', patch.description);
139143
if (patch.status !== undefined) set('status', patch.status);
140144
if (patch.priority !== undefined) set('priority', patch.priority);
145+
if (patch.order !== undefined) set('"order"', patch.order);
141146

142147
set('version', num(row.version as bigint) + 1);
143148
set('updated_by_id', authorId ?? null);
@@ -147,6 +152,7 @@ export class SqliteEpicsStore implements EpicsStore {
147152
this.db.prepare(`UPDATE epics SET ${fields.join(', ')} WHERE id = ? AND project_id = ?`).run(...params);
148153

149154
if (embedding) {
155+
assertEmbeddingDim(embedding);
150156
this.db.prepare('DELETE FROM epics_vec WHERE rowid = ?').run(BigInt(epicId));
151157
this.db.prepare('INSERT INTO epics_vec (rowid, embedding) VALUES (?, ?)').run(BigInt(epicId), Buffer.from(new Float32Array(embedding).buffer));
152158
}
@@ -215,6 +221,10 @@ export class SqliteEpicsStore implements EpicsStore {
215221
return row.m ? num(row.m) + ORDER_GAP : ORDER_GAP;
216222
}
217223

224+
reorder(epicId: number, order: number, authorId?: number): EpicRecord {
225+
return this.update(epicId, { order }, null, authorId);
226+
}
227+
218228
// =========================================================================
219229
// Link / Unlink tasks
220230
// =========================================================================

src/store/sqlite/stores/files.ts

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,12 @@ import type {
44
FilesStore,
55
FileNode,
66
FileListOptions,
7+
FileUpdateOptions,
78
SearchQuery,
89
SearchResult,
910
} from '../../types';
1011
import { MetaHelper } from '../lib/meta';
11-
import { num, likeEscape } from '../lib/bigint';
12+
import { num, likeEscape, assertEmbeddingDim } from '../lib/bigint';
1213

1314
const GRAPH = 'files';
1415

@@ -68,11 +69,14 @@ export class SqliteFilesStore implements FilesStore {
6869
// updateFile
6970
// =========================================================================
7071

71-
updateFile(filePath: string, size: number, mtime: number, embedding: number[]): void {
72+
updateFile(filePath: string, size: number, mtime: number, embedding: number[], opts?: FileUpdateOptions): void {
73+
assertEmbeddingDim(embedding);
7274
const run = this.db.transaction(() => {
7375
const fileName = path.basename(filePath);
7476
const directory = path.dirname(filePath);
7577
const extension = path.extname(filePath);
78+
const language = opts?.language ?? null;
79+
const mimeType = opts?.mimeType ?? null;
7680

7781
// Ensure parent directory exists
7882
if (directory && directory !== '.') {
@@ -90,9 +94,9 @@ export class SqliteFilesStore implements FilesStore {
9094
fileId = num(existing.id);
9195
// Update existing
9296
this.db.prepare(`
93-
UPDATE files SET file_name = ?, directory = ?, extension = ?, size = ?, mtime = ?
97+
UPDATE files SET file_name = ?, directory = ?, extension = ?, language = ?, mime_type = ?, size = ?, mtime = ?
9498
WHERE id = ? AND project_id = ?
95-
`).run(fileName, directory, extension, size, mtime, fileId, this.projectId);
99+
`).run(fileName, directory, extension, language, mimeType, size, mtime, fileId, this.projectId);
96100

97101
// Update vec0 (DELETE + INSERT pattern)
98102
this.db.prepare('DELETE FROM files_vec WHERE rowid = ?').run(BigInt(fileId));
@@ -101,9 +105,9 @@ export class SqliteFilesStore implements FilesStore {
101105
} else {
102106
// Insert new file
103107
const result = this.db.prepare(`
104-
INSERT INTO files (project_id, kind, file_path, file_name, directory, extension, size, mtime)
105-
VALUES (?, 'file', ?, ?, ?, ?, ?, ?)
106-
`).run(this.projectId, filePath, fileName, directory, extension, size, mtime);
108+
INSERT INTO files (project_id, kind, file_path, file_name, directory, extension, language, mime_type, size, mtime)
109+
VALUES (?, 'file', ?, ?, ?, ?, ?, ?, ?, ?)
110+
`).run(this.projectId, filePath, fileName, directory, extension, language, mimeType, size, mtime);
107111
fileId = num(result.lastInsertRowid as bigint);
108112

109113
// Insert vec0

0 commit comments

Comments
 (0)