🔒 Aisle Security Analysis

We found 2 potential security issue(s) in this PR:

1. 🟡 Main-thread blocking DoS risk from 1s SQLite busy_timeout on synchronous QMD read connection

Description

QmdMemoryManager opens its QMD index database using Node’s synchronous DatabaseSync (runs on the main thread) and now sets PRAGMA busy_timeout = 1000.

Impact:

• When another process/thread holds a write lock on the QMD index (e.g., the QMD updater process qmd update/qmd embed running concurrently), SQLite calls on this connection can block synchronously for up to 1 second per statement instead of failing fast.
• These synchronous calls occur on the latency-sensitive recall path:
  • QmdMemoryManager.search() resolves each result’s docid via resolveDocLocation() which calls ensureDb() and then executes synchronous db.prepare(...).all(...) queries.
  • The memory_search tool (used via the HTTP gateway /tools/invoke) calls manager.search(...) and then manager.status() (which calls readCounts() -> ensureDb() -> synchronous SQL).
• Increasing the busy timeout from 1ms to 1000ms therefore introduces an availability/DoS vector: an authenticated caller can repeatedly invoke memory_search during periods of index write activity to stall the Node event loop and degrade service responsiveness for other requests.

Vulnerable change:

this.db = new DatabaseSync(this.indexPath, { readOnly: true });
this.db.exec("PRAGMA busy_timeout = 1000");

Recommendation

Avoid long blocking waits on the main thread for the QMD read-only recall path.

Options:

1. Fail fast and fall back (restore a very small busy timeout) so the existing fallback manager can take over:

this.db = new DatabaseSync(this.indexPath, { readOnly: true });
this.db.exec("PRAGMA busy_timeout = 1"); // or 0

2. Move SQLite reads off the main thread (preferred if you want retry behavior):

• Use an async SQLite API / worker thread for docid->path lookups, or
• Isolate DB access into a worker so busy waits don’t block the event loop.

3. If retries are required, implement application-level retries with backoff and yielding (only feasible if DB access is not synchronous), rather than relying on busy_timeout.

2. 🟡 Node.js event-loop blocking DoS risk from SQLite DatabaseSync busy_timeout=5000 on request-path connection

Description

The builtin memory search/index uses node:sqlite's DatabaseSync (synchronous SQLite I/O on the Node.js main thread). This change sets a 5-second PRAGMA busy_timeout on every connection open, which can turn lock contention into multi-second event-loop stalls instead of a fast SQLITE_BUSY failure.

Concrete call chain to the affected connection:

• External request: POST /tools/invoke (gateway) → tool memory_search
• createMemorySearchTool.execute() → getMemorySearchManager() → MemoryIndexManager.get()
• new MemoryIndexManager(...) → this.db = this.openDatabase() → openDatabaseAtPath() sets busy_timeout = 5000
• Request path continues in MemoryIndexManager.search() which performs synchronous SQLite queries (db.prepare(...).all()/get())

Why this can be abused into a DoS:

• With busy_timeout=5000, any statement encountering a lock (from concurrent sync/reindex in-process or another process using the same DB file) may block up to 5 seconds.
• Because operations are synchronous (DatabaseSync), that wait occurs on the Node.js event loop, stalling unrelated requests and timers.
• MemoryIndexManager.search() can also trigger background sync (this.sync({ reason: "search" })) when dirty; sync performs many writes (DELETE/INSERT loops) and can increase lock contention.

Vulnerable code (changed):

const db = new DatabaseSync(dbPath, { allowExtension: this.settings.store.vector.enabled });
db.exec("PRAGMA busy_timeout = 5000");

Impact: a client that can trigger memory_search (or any path that uses this manager) during lock contention can cause repeated 5s event-loop stalls, amplifying into request queueing/thread starvation and service unavailability.

Recommendation

Avoid long busy_timeout waits on the request-path DatabaseSync connection.

Options (choose one):

1. Use separate connections: a read-only/search connection with a small timeout, and a background write/sync connection with longer timeout.

​// Request-path (search): fail fast to avoid event-loop stalls
const readDb = new DatabaseSync(dbPath, { readOnly: true, allowExtension: false });
readDb.exec("PRAGMA busy_timeout = 50");

​// Background sync/indexing: can tolerate waiting
const writeDb = new DatabaseSync(dbPath, { allowExtension: this.settings.store.vector.enabled });
writeDb.exec("PRAGMA busy_timeout = 5000");

2. Keep busy_timeout low (e.g., 50–250ms) for this manager and handle SQLITE_BUSY by returning an error or deferring work.

3. Move SQLite work off the main thread (switch to async DB access or a worker thread) so lock waits do not block the event loop.

Also consider enabling WAL mode (if compatible) to reduce reader/writer blocking and lock convoying.

Analyzed PR: #39183 at commit cc744d0

_{Last updated on: 2026-03-07T22:14:12Z}

Greptile Summary

This PR fixes a SQLITE_BUSY crash that occurred under concurrent process load by ensuring PRAGMA busy_timeout = 5000 is set on every SQLite connection open. Previously, write connections in manager-sync-ops.ts had no timeout at all (default 0 ms), and the read-only QMD connection in qmd-manager.ts had only a 1 ms timeout — both effectively causing immediate failures when another process held a lock.

Key changes:

• openDatabaseAtPath() in manager-sync-ops.ts now calls db.exec("PRAGMA busy_timeout = 5000") immediately after opening, covering all write opens (initial, temp-DB for vacuum/rebuild, and post-swap reopen).
• ensureDb() in qmd-manager.ts raises busy_timeout from 1 ms to 5000 ms on the read-only index connection.

One design consideration worth noting:
The original busy_timeout = 1 on the QMD read-only connection carried an explicit comment — "Keep QMD recall responsive when the updater holds a write lock" — signalling an intentional fail-fast strategy. Raising this to 5000 ms means QMD memory recall could stall for up to 5 seconds if an exclusive lock is held (e.g. during a VACUUM or when running without WAL journal mode). In WAL mode — strongly implied by the -wal/-shm file-suffix handling in moveIndexFiles/removeIndexFiles — readers and writers coexist without blocking, so the timeout would rarely fire in practice. However, no PRAGMA journal_mode = WAL is set explicitly in the connection-open code, so behaviour depends on whatever mode was last persisted to the database file. If WAL mode is reliably enabled elsewhere (e.g. by the qmd updater process), this change is safe. If it is not guaranteed, a more conservative value (e.g. 1000–2000 ms) for the read path might better preserve the original responsiveness goal.

Confidence Score: 4/5

• Safe to merge; the fix is correct and well-scoped, with a minor design trade-off on the QMD read-only path that is low-risk in WAL mode.
• Both changes correctly apply busy_timeout on every connection open, directly addressing the SQLITE_BUSY root cause. The write-path change (manager-sync-ops.ts) is straightforward and unambiguously correct. The QMD read-path change reverses a deliberate fail-fast design decision (1 ms → 5000 ms), which is acceptable but introduces the possibility of up to 5-second read stalls if an exclusive lock is held and WAL mode is not active. Since WAL mode appears to be in use based on the -wal/-shm file handling, the practical risk is low, warranting a 4 rather than a perfect 5.
• src/memory/qmd-manager.ts — confirm that the QMD SQLite database reliably uses WAL journal mode so the 5000 ms read timeout cannot cause user-visible recall latency spikes.

_{Last reviewed commit: 8b43255}

Landed on main in 3a2fdc513.

What we changed while landing:

• kept your core fix: apply PRAGMA busy_timeout on each connection open for both sync-store and QMD paths
• aligned QMD timeout to 1000ms (1s) instead of 1ms, with rationale in-code for synchronous read-path safety
• added regression tests asserting busy-timeout pragma values on both managers:
  • src/memory/manager.readonly-recovery.test.ts
  • src/memory/qmd-manager.test.ts
• added changelog entry in 2026.3.7 fixes

Source commits reviewed:

• 8b43255ea
• cc744d008

Thanks @MumuTW.