From 5f9e1539715e269f5ce42a24df4abf270ccfe148 Mon Sep 17 00:00:00 2001 From: "Bob.Hou" Date: Thu, 17 Sep 2026 17:47:25 -0400 Subject: [PATCH] fix(mcp): fall back when better-sqlite3 export is not callable (#13903) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * mcp/audit: fall back when better-sqlite3 export is not callable Dashboard MCP status polls reopen a failed native sqlite load every 30s because a minified TypeError ("a is not a function") was not treated as a native load failure and a failed open was not cached. Classify that shape, fall back to node:sqlite, cache the miss, and refuse to ship a Docker image without better_sqlite3.node. Signed-off-by: Minxi Hou * mcp/audit: force native better-sqlite3 compile in Docker better-sqlite3 13 ships a linux prebuild. Bare `node-gyp rebuild` then only TOUCHes stamp files and never writes build/Release/better_sqlite3.node, so the new test -f gate fails the image build. Pass --force_build=1, matching the package's own build-release script. Signed-off-by: Minxi Hou * db/core: keep native-load classification under the file-size cap The audit fallback added two TypeError fingerprints in core.ts and crossed the frozen 1788-line cap. Move the classifier into sqliteLoadError.ts and re-export it so existing importers stay stable. Signed-off-by: Minxi Hou * build/bootstrap: keep the encrypted-credentials probe narrow The native-load classifier was copied into scripts/build/bootstrap-env.mjs alongside the runtime one, but the two files consume its verdict in opposite directions. In src/lib/db/sqliteLoadError.ts a true verdict means "the driver is unusable, cascade to node:sqlite", so treating a non-callable export as a load failure is what we want. In the bootstrap the verdict feeds hasEncryptedCredentials, where true means "no encrypted credentials found" and clears the way to generate a fresh STORAGE_ENCRYPTION_KEY. With the TypeError patterns in the bootstrap copy, a binding that loads but exports something non-callable over a database full of enc:v1: rows reads as an empty database, and the operator silently loses access to every stored credential. Drop those two patterns from the bootstrap copy only, and note in both files why the pair is deliberately not identical. A corrupt binding still fails loudly there, now with the database path, the underlying message, and a rebuild hint, so the narrower classifier does not cost any diagnosability. Signed-off-by: Minxi Hou * fix(mcp): keep audit logging recoverable when the database is created later getDb() cached a null for the "storage.sqlite does not exist yet" branch, and closeAuditDb() returns before clearing a falsy cache — so an MCP server started before the app created the database stayed without audit logging for the whole process lifetime. Only a genuine driver-load failure is cached now; the not-found branch retries, which is how it recovers when the file appears. Covered by a new test that fails without the change. Also replace the fabricated minified TypeError text ("a is not a function") thrown by the loader with "better-sqlite3 export is not a function": the operator sees a diagnosable message and isNativeSqliteLoadError() still classifies it (it matches on "is not a function"). --------- Signed-off-by: Minxi Hou Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> --- Dockerfile | 4 +- .../fixes/mcp-audit-sqlite-fallback.md | 1 + open-sse/mcp-server/__tests__/audit.test.ts | 133 ++++++++++++++---- open-sse/mcp-server/audit.ts | 35 +++-- scripts/build/bootstrap-env.mjs | 15 +- src/lib/db/AGENTS.md | 6 +- src/lib/db/core.ts | 35 +---- src/lib/db/sqliteLoadError.ts | 44 ++++++ .../bootstrap-env-sqlite-classifier.test.mjs | 83 +++++++++++ .../dockerfile-better-sqlite3-native.test.mjs | 21 +++ tests/unit/db-core-native-error.test.ts | 10 ++ 11 files changed, 313 insertions(+), 74 deletions(-) create mode 100644 changelog.d/fixes/mcp-audit-sqlite-fallback.md create mode 100644 src/lib/db/sqliteLoadError.ts create mode 100644 tests/unit/build/bootstrap-env-sqlite-classifier.test.mjs create mode 100644 tests/unit/build/dockerfile-better-sqlite3-native.test.mjs diff --git a/Dockerfile b/Dockerfile index fe49083103..e42e9bcf22 100644 --- a/Dockerfile +++ b/Dockerfile @@ -106,7 +106,8 @@ RUN test -f package-lock.json \ RUN --mount=type=cache,id=s/92ca8a61-c1ba-421f-a389-d48ac7258c2d-npm-cache,target=/root/.npm \ npm ci --include=optional --no-audit --no-fund --legacy-peer-deps --ignore-scripts \ && (cd node_modules/better-sqlite3 \ - && node /usr/local/lib/node_modules/npm/node_modules/node-gyp/bin/node-gyp.js rebuild) \ + && node /usr/local/lib/node_modules/npm/node_modules/node-gyp/bin/node-gyp.js rebuild --force_build=1) \ + && test -f node_modules/better-sqlite3/build/Release/better_sqlite3.node \ && node -e "require('better-sqlite3')(':memory:').close()" \ && node -e "const wreq=require('wreq-js'); if(typeof wreq.createTransport!=='function') process.exit(1)" @@ -253,6 +254,7 @@ COPY --from=builder /app/.build/next/standalone ./ # Next.js tracing. bootstrap-env requires SQLite BEFORE the standalone server # starts, so guarantee the complete package independent of trace behaviour. COPY --from=builder /app/node_modules/better-sqlite3 ./node_modules/better-sqlite3 +RUN test -f /app/node_modules/better-sqlite3/build/Release/better_sqlite3.node # migrations land at /migrations via assembleStandalone; point the runtime at them. ENV OMNIROUTE_MIGRATIONS_DIR=/app/migrations diff --git a/changelog.d/fixes/mcp-audit-sqlite-fallback.md b/changelog.d/fixes/mcp-audit-sqlite-fallback.md new file mode 100644 index 0000000000..f7211f88ec --- /dev/null +++ b/changelog.d/fixes/mcp-audit-sqlite-fallback.md @@ -0,0 +1 @@ +- **fix(mcp):** MCP audit treats a non-callable better-sqlite3 export (`better-sqlite3 export is not a function`) as a native load failure, falls back to `node:sqlite`, and caches a failed driver load so dashboard polls stop reprinting (a database file that does not exist yet is never cached, so the connection recovers once the app creates it). Docker now refuses to ship without `better_sqlite3.node`. Native-load classification lives in `sqliteLoadError.ts` so `core.ts` stays under its frozen line cap. The build bootstrap keeps a deliberately narrower classifier: a corrupt binding there must not be read as "no encrypted credentials", or a fresh `STORAGE_ENCRYPTION_KEY` would be generated over an existing encrypted database. diff --git a/open-sse/mcp-server/__tests__/audit.test.ts b/open-sse/mcp-server/__tests__/audit.test.ts index 829acaf4af..4398d62047 100644 --- a/open-sse/mcp-server/__tests__/audit.test.ts +++ b/open-sse/mcp-server/__tests__/audit.test.ts @@ -44,34 +44,29 @@ describe("MCP audit shutdown", () => { vi.restoreAllMocks(); }); - it( - "checkpoints and closes the audit database during shutdown", - async () => { - const mockDb: MockAuditDb = { - prepare: vi.fn(() => createStatementMock()), - pragma: vi.fn(), - close: vi.fn(), - open: true, - }; + it("checkpoints and closes the audit database during shutdown", async () => { + const mockDb: MockAuditDb = { + prepare: vi.fn(() => createStatementMock()), + pragma: vi.fn(), + close: vi.fn(), + open: true, + }; - const audit = await import("../audit.ts"); - // Inject through the connection cache — the seam the module itself uses. - globalThis.__omnirouteMcpAuditDb = mockDb as unknown as typeof globalThis.__omnirouteMcpAuditDb; + const audit = await import("../audit.ts"); + // Inject through the connection cache — the seam the module itself uses. + globalThis.__omnirouteMcpAuditDb = mockDb as unknown as typeof globalThis.__omnirouteMcpAuditDb; - await audit.logToolCall("omniroute_get_health", { ok: true }, { ok: true }, 12, true); - expect(mockDb.prepare).toHaveBeenCalledTimes(1); + await audit.logToolCall("omniroute_get_health", { ok: true }, { ok: true }, 12, true); + expect(mockDb.prepare).toHaveBeenCalledTimes(1); - expect(audit.closeAuditDb()).toBe(true); - expect(mockDb.pragma).toHaveBeenCalledWith("wal_checkpoint(TRUNCATE)"); - expect(mockDb.close).toHaveBeenCalledTimes(1); - expect(audit.closeAuditDb()).toBe(false); - }, - // Explicit generous timeout (vitest default is 5000ms): under contended - // CI-runner load, vi.resetModules() + a fresh dynamic import + mocked DB - // calls can exceed the default budget though the behavior is correct - // (issue #6803). - 30000 - ); + expect(audit.closeAuditDb()).toBe(true); + expect(mockDb.pragma).toHaveBeenCalledWith("wal_checkpoint(TRUNCATE)"); + expect(mockDb.close).toHaveBeenCalledTimes(1); + expect(audit.closeAuditDb()).toBe(false); + }, // CI-runner load, vi.resetModules() + a fresh dynamic import + mocked DB // Explicit generous timeout (vitest default is 5000ms): under contended + // calls can exceed the default budget though the behavior is correct + // (issue #6803). + 30000); it("still closes the audit database when checkpoint fails", async () => { const mockDb: MockAuditDb = { @@ -140,4 +135,92 @@ describe("MCP audit shutdown", () => { audit.__setBetterSqliteLoaderForTests(null); } }); + + it("falls back to node:sqlite when better-sqlite3 export is not a function", async () => { + const [maj, min] = process.versions.node.split(".").map(Number); + if (maj < 22 || (maj === 22 && min < 5)) { + return; + } + + const mockNodeDb = { + prepare: vi.fn(() => createStatementMock()), + exec: vi.fn(), + close: vi.fn(), + }; + const DatabaseSync = vi.fn(function DatabaseSync() { + return mockNodeDb; + }); + vi.doMock("node:sqlite", () => ({ DatabaseSync })); + + const audit = await import("../audit.ts"); + // Webpack/standalone stub: require("better-sqlite3") returns a non-callable + // object, so the loader rejects it with "better-sqlite3 export is not a function" + // (the minified runtime form is "a is not a function"; both classify the same). + audit.__setBetterSqliteLoaderForTests(() => ({ default: { notAConstructor: true } })); + + try { + await audit.logToolCall("omniroute_get_health", { ok: true }, { ok: true }, 4, true); + expect(DatabaseSync).toHaveBeenCalledWith(dbFile); + expect(mockNodeDb.prepare).toHaveBeenCalled(); + } finally { + audit.closeAuditDb(); + audit.__setBetterSqliteLoaderForTests(null); + } + }); + + it("retries once the database file appears instead of caching the miss forever", async () => { + // An MCP server started before the app created ~/.omniroute/storage.sqlite must + // pick the database up on a later call. Caching the "not found" miss would leave + // that process without audit logging for its whole lifetime. + fs.rmSync(dbFile); + + const mockDb: MockAuditDb = { + prepare: vi.fn(() => createStatementMock()), + pragma: vi.fn(), + close: vi.fn(), + open: true, + }; + const audit = await import("../audit.ts"); + audit.__setBetterSqliteLoaderForTests( + () => + function Database() { + return mockDb; + } + ); + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + + try { + await audit.logToolCall("omniroute_get_health", { ok: true }, { ok: true }, 1, true); + expect(mockDb.prepare).not.toHaveBeenCalled(); + + fs.writeFileSync(dbFile, ""); + await audit.logToolCall("omniroute_get_health", { ok: true }, { ok: true }, 1, true); + expect(mockDb.prepare).toHaveBeenCalledTimes(1); + } finally { + errorSpy.mockRestore(); + audit.closeAuditDb(); + audit.__setBetterSqliteLoaderForTests(null); + } + }); + + it("caches a failed audit connection so dashboard polls do not reconnect", async () => { + const connectErr = new Error("permission denied"); + const audit = await import("../audit.ts"); + audit.__setBetterSqliteLoaderForTests(() => { + throw connectErr; + }); + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + + try { + await audit.queryAuditEntries({ limit: 1 }); + await audit.queryAuditEntries({ limit: 1 }); + const connectLogs = errorSpy.mock.calls.filter((args) => + String(args[0]).includes("Failed to connect to database") + ); + expect(connectLogs).toHaveLength(1); + } finally { + errorSpy.mockRestore(); + audit.__setBetterSqliteLoaderForTests(null); + } + }); }); diff --git a/open-sse/mcp-server/audit.ts b/open-sse/mcp-server/audit.ts index a658d3af7f..94bbefbaaf 100644 --- a/open-sse/mcp-server/audit.ts +++ b/open-sse/mcp-server/audit.ts @@ -184,11 +184,11 @@ function buildAuditFilterSql(filters: McpAuditQuery): { whereSql: string; params }; } -function getCachedAuditDb(): AuditDatabase | null { - return globalThis.__omnirouteMcpAuditDb ?? null; +function getCachedAuditDb(): AuditDatabase | null | undefined { + return globalThis.__omnirouteMcpAuditDb; } -function setCachedAuditDb(database: AuditDatabase | null): void { +function setCachedAuditDb(database: AuditDatabase | null | undefined): void { globalThis.__omnirouteMcpAuditDb = database; } @@ -226,10 +226,11 @@ async function openBetterSqliteAuditDb(dbPath: string): Promise { const _require = createRequire(import.meta.url); mod = _require("better-sqlite3"); } - const Database = ((mod as { default?: unknown })?.default || mod) as unknown as new ( - dbPath: string - ) => AuditDatabase; - return new Database(dbPath); + const Database = ((mod as { default?: unknown })?.default || mod) as unknown; + if (typeof Database !== "function") { + throw new TypeError("better-sqlite3 export is not a function"); + } + return new (Database as new (dbPath: string) => AuditDatabase)(dbPath); } function nodeSqliteFallbackAvailable(): boolean { @@ -244,7 +245,10 @@ async function openNodeSqliteAuditDb(dbPath: string): Promise { return createNodeSqliteAuditAdapter(new DatabaseSync(dbPath)); } -async function openFallbackAuditDb(dbPath: string, nativeMessage: string): Promise { +async function openFallbackAuditDb( + dbPath: string, + nativeMessage: string +): Promise { if (!nodeSqliteFallbackAvailable()) { console.error( `[MCP Audit] better-sqlite3 native binding unavailable and Node ${process.version} ` + @@ -283,10 +287,12 @@ async function openFallbackAuditDb(dbPath: string, nativeMessage: string): Promi */ async function getDb(): Promise { const cachedDb = getCachedAuditDb(); - if (cachedDb) return cachedDb; + // undefined = never tried / retryable; null = the driver itself failed to load. + // Only that second case is cached, so dashboard 30s polls do not reopen and + // reprint the same binding error. + if (cachedDb !== undefined) return cachedDb; try { - // Try importing the db module from the main app const { homedir } = await import("node:os"); const { join } = await import("node:path"); const { existsSync } = await import("node:fs"); @@ -296,6 +302,9 @@ async function getDb(): Promise { : join(homedir(), ".omniroute", "storage.sqlite"); if (!existsSync(dbPath)) { + // Do NOT cache this miss: an MCP server can start before the app creates + // storage.sqlite, and the file appearing is exactly how it recovers. A + // cached null would disable audit logging for the whole process lifetime. console.error(`[MCP Audit] Database not found at ${dbPath} — audit logging disabled`); return null; } @@ -308,6 +317,7 @@ async function getDb(): Promise { const nativeMessage = nativeErr instanceof Error ? nativeErr.message : String(nativeErr); if (!isNativeSqliteLoadError(nativeErr)) { console.error("[MCP Audit] Failed to connect to database:", nativeMessage); + setCachedAuditDb(null); return null; } const fallbackDb = await openFallbackAuditDb(dbPath, nativeMessage); @@ -317,6 +327,7 @@ async function getDb(): Promise { } catch (err: unknown) { const message = err instanceof Error ? err.message : String(err); console.error("[MCP Audit] Failed to connect to database:", message); + setCachedAuditDb(null); return null; } } @@ -325,7 +336,9 @@ export function closeAuditDb(): boolean { const database = getCachedAuditDb(); if (!database) return false; - setCachedAuditDb(null); + // Drop the cache to undefined (never tried), not null (tried and failed), + // so a later getDb() can open again after an intentional close. + setCachedAuditDb(undefined); try { try { diff --git a/scripts/build/bootstrap-env.mjs b/scripts/build/bootstrap-env.mjs index 5e02d283ae..4aa75b3373 100644 --- a/scripts/build/bootstrap-env.mjs +++ b/scripts/build/bootstrap-env.mjs @@ -67,6 +67,11 @@ function isNativeSqliteLoadError(error) { const message = error instanceof Error ? error.message : String(error); const code = error && typeof error === "object" && "code" in error ? error.code : undefined; + // Deliberately narrower than src/lib/db/sqliteLoadError.ts. There, a + // non-callable export means "fall back to another driver". Here, the only + // consumer treats a match as "no encrypted credentials exist", which lets + // STORAGE_ENCRYPTION_KEY be regenerated over a database that still holds + // enc:v1: rows. A generic TypeError must stay loud on this path. return ( message.includes("Module did not self-register") || message.includes("NODE_MODULE_VERSION") || @@ -78,6 +83,11 @@ function isNativeSqliteLoadError(error) { ); } +function isLikelyBrokenNativeBinding(error) { + const message = error instanceof Error ? error.message : String(error); + return message.includes("is not a function") || message.includes("is not a constructor"); +} + function hasEncryptedCredentials(dataDir) { const dbPath = join(dataDir, "storage.sqlite"); if (!existsSync(dbPath)) return false; @@ -133,7 +143,10 @@ function hasEncryptedCredentials(dataDir) { } const message = error instanceof Error ? error.message : String(error); - throw new Error(`Unable to inspect existing database at ${dbPath}: ${message}`); + const hint = isLikelyBrokenNativeBinding(error) + ? " The better-sqlite3 native binding loaded but did not expose a usable constructor; try `npm rebuild better-sqlite3`." + : ""; + throw new Error(`Unable to inspect existing database at ${dbPath}: ${message}${hint}`); } } diff --git a/src/lib/db/AGENTS.md b/src/lib/db/AGENTS.md index c133d0b90b..55c049f34e 100644 --- a/src/lib/db/AGENTS.md +++ b/src/lib/db/AGENTS.md @@ -2,13 +2,13 @@ **Purpose**: Domain-driven SQLite persistence. Each module owns a specific table set. Schema migrations are versioned and idempotent. No raw SQL in routes — all ops go through `src/lib/db/` modules. -Live count: `ls src/lib/db/*.ts | wc -l` (currently 117). Migrations: `ls src/lib/db/migrations/*.sql | wc -l` (currently 148). +Live count: `ls src/lib/db/*.ts | wc -l` (currently 131). Migrations: `ls src/lib/db/migrations/*.sql | wc -l` (currently 148). --- ## Core Infrastructure -- **`core.ts`** — `getDbInstance()` returns singleton `better-sqlite3` with WAL journaling. Exports `rowToCamel()` (snake_case → camelCase), `encryptConnectionFields()` for provider credentials at rest. `SCHEMA_SQL` defines **17 base tables** (verify: `grep -c "CREATE TABLE" src/lib/db/core.ts` minus 1 for `_omniroute_migrations`). +- **`core.ts`** — `getDbInstance()` returns singleton `better-sqlite3` with WAL journaling. Exports `rowToCamel()` (snake_case → camelCase), `encryptConnectionFields()` for provider credentials at rest. `SCHEMA_SQL` defines **17 base tables** (verify: `grep -c "CREATE TABLE" src/lib/db/core.ts` minus 1 for `_omniroute_migrations`). Native-load / missing-driver classification lives in `sqliteLoadError.ts` and is re-exported from here. - **`migrationRunner.ts`** — Applies versioned SQL files from `db/migrations/` inside transactions. Tracks applied migrations in `_omniroute_migrations`. Each migration is idempotent. - **`db/migrations/`** — 148 SQL files (`001_initial_schema.sql` → `153_radar_local_model_state.sql`; numbering has intentional gaps). Each runs in a transaction, never fails partially. - The old `localDb.ts` barrel has been removed — consumers must import from the owning named module below. @@ -44,7 +44,7 @@ Live count: `ls src/lib/db/*.ts | wc -l` (currently 117). Migrations: `ls src/li | `healthCheck.ts` | health ops | DB health monitoring | | `databaseSettings.ts` | database settings | DB-level configuration | -Full list: `ls src/lib/db/*.ts | wc -l` (115 files). Drift detection: `npm run check:docs-counts`. +Full list: `ls src/lib/db/*.ts | wc -l` (131 files). Drift detection: `npm run check:docs-counts`. ## Encryption & Security diff --git a/src/lib/db/core.ts b/src/lib/db/core.ts index bee1e42693..910883c30d 100644 --- a/src/lib/db/core.ts +++ b/src/lib/db/core.ts @@ -52,8 +52,10 @@ import { getWalMaintenanceState, logCheckpointOutcome, } from "./walMaintenance"; +import { isNativeSqliteLoadError, isSqliteDriverUnavailableError } from "./sqliteLoadError"; // Re-exported so existing call sites that pull these helpers off the core module keep working. export { toSnakeCase, toCamelCase, objToSnake, rowToCamel, cleanNulls } from "./caseMapping"; +export { isNativeSqliteLoadError, isSqliteDriverUnavailableError }; import { ensureProviderConnectionsColumns, ensureUsageHistoryAccountIndex, @@ -150,39 +152,6 @@ const CRITICAL_DB_TABLES: CriticalTableSpec[] = [ { table: "webhooks", maxRows: 5_000 }, ]; -export function isNativeSqliteLoadError(error: unknown): boolean { - const message = error instanceof Error ? error.message : String(error); - const code = getErrorCode(error); - return ( - message.includes("Module did not self-register") || - message.includes("NODE_MODULE_VERSION") || - message.includes("ERR_DLOPEN_FAILED") || - // bun and similar runtimes that skip the postinstall script never download - // the prebuilt *.node binary, so `bindings()` fails with this message - // before any DLOPEN even happens (#2358). - message.includes("Could not locate the bindings file") || - message.includes("Cannot find module 'better-sqlite3'") || - code === "ERR_DLOPEN_FAILED" || - code === "MODULE_NOT_FOUND" - ); -} - -export function isSqliteDriverUnavailableError(error: unknown): boolean { - const message = error instanceof Error ? error.message : String(error); - - return ( - message.includes("Nenhum driver SQLite disponível") || - message.includes("Chame ensureDbInitialized() no startup") || - message.includes("sql.js WASM ainda não foi pré-inicializado") - ); -} - -function getErrorCode(error: unknown): string | undefined { - if (!error || typeof error !== "object" || !("code" in error)) return undefined; - const code = (error as { code?: unknown }).code; - return typeof code === "string" ? code : undefined; -} - /** * Closes a probe/throwaway connection obtained from `openSqliteDatabase()` — * but ONLY when it is safe to do so. better-sqlite3/node:sqlite hand back an diff --git a/src/lib/db/sqliteLoadError.ts b/src/lib/db/sqliteLoadError.ts new file mode 100644 index 0000000000..c43b050955 --- /dev/null +++ b/src/lib/db/sqliteLoadError.ts @@ -0,0 +1,44 @@ +/** + * Classifies native better-sqlite3 load failures and the sql.js fallback + * "no driver" contract. Lives beside core.ts so the singleton module stays + * under the frozen file-size cap. + */ + +function getErrorCode(error: unknown): string | undefined { + if (!error || typeof error !== "object" || !("code" in error)) return undefined; + const code = (error as { code?: unknown }).code; + return typeof code === "string" ? code : undefined; +} + +export function isNativeSqliteLoadError(error: unknown): boolean { + const message = error instanceof Error ? error.message : String(error); + const code = getErrorCode(error); + return ( + message.includes("Module did not self-register") || + message.includes("NODE_MODULE_VERSION") || + message.includes("ERR_DLOPEN_FAILED") || + // bun and similar runtimes that skip the postinstall script never download + // the prebuilt *.node binary, so `bindings()` fails with this message + // before any DLOPEN even happens (#2358). + message.includes("Could not locate the bindings file") || + message.includes("Cannot find module 'better-sqlite3'") || + // Webpack/standalone can resolve better-sqlite3 to a stub or a non-callable + // export. `new (mod.default || mod)(path)` then throws TypeError + // " is not a function" / "X is not a constructor" instead of MODULE_NOT_FOUND + // — minified bundles shorten the name to a single letter ("a is not a function"). + message.includes("is not a function") || + message.includes("is not a constructor") || + code === "ERR_DLOPEN_FAILED" || + code === "MODULE_NOT_FOUND" + ); +} + +export function isSqliteDriverUnavailableError(error: unknown): boolean { + const message = error instanceof Error ? error.message : String(error); + + return ( + message.includes("Nenhum driver SQLite disponível") || + message.includes("Chame ensureDbInitialized() no startup") || + message.includes("sql.js WASM ainda não foi pré-inicializado") + ); +} diff --git a/tests/unit/build/bootstrap-env-sqlite-classifier.test.mjs b/tests/unit/build/bootstrap-env-sqlite-classifier.test.mjs new file mode 100644 index 0000000000..e0e6cd1a52 --- /dev/null +++ b/tests/unit/build/bootstrap-env-sqlite-classifier.test.mjs @@ -0,0 +1,83 @@ +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import test from "node:test"; + +// The two copies of isNativeSqliteLoadError are deliberately NOT identical. +// +// src/lib/db/sqliteLoadError.ts runs where a match means "this driver is +// unusable, cascade to the next one", so a non-callable better-sqlite3 export +// ("X is not a constructor") must match. +// +// scripts/build/bootstrap-env.mjs runs where the only consumer, +// hasEncryptedCredentials(), turns a match into `return false` — read by the +// caller as "no encrypted credentials exist" and used to mint a fresh +// STORAGE_ENCRYPTION_KEY. Matching a generic TypeError there would overwrite +// the key of a database that still holds enc:v1: rows, so that path stays loud. +// +// Sharing one widened classifier across both is what this test exists to catch. + +const GENERIC_TYPE_ERROR_PATTERNS = ['"is not a function"', '"is not a constructor"']; + +function readSource(relativePath) { + return readFileSync(new URL(relativePath, import.meta.url), "utf8"); +} + +function classifierBody(source) { + const start = source.indexOf("isNativeSqliteLoadError"); + assert.notEqual(start, -1, "isNativeSqliteLoadError not found"); + const end = source.indexOf("\n}", start); + assert.notEqual(end, -1, "could not delimit isNativeSqliteLoadError"); + return source.slice(start, end); +} + +test("the bootstrap classifier stays narrow so key generation cannot silently proceed", () => { + const body = classifierBody(readSource("../../../scripts/build/bootstrap-env.mjs")); + + for (const pattern of GENERIC_TYPE_ERROR_PATTERNS) { + assert.ok( + !body.includes(pattern), + `scripts/build/bootstrap-env.mjs must not classify ${pattern} as a native SQLite load failure: ` + + "hasEncryptedCredentials() would then report no encrypted credentials and bootstrapEnv " + + "would generate a new STORAGE_ENCRYPTION_KEY over encrypted rows." + ); + } + + // The narrow patterns it does need must still be there. + assert.ok(body.includes('"ERR_DLOPEN_FAILED"')); + assert.ok(body.includes("\"Cannot find module 'better-sqlite3'\"")); +}); + +test("the runtime classifier keeps matching a non-callable better-sqlite3 export", () => { + const body = classifierBody(readSource("../../../src/lib/db/sqliteLoadError.ts")); + + for (const pattern of GENERIC_TYPE_ERROR_PATTERNS) { + assert.ok( + body.includes(pattern), + `src/lib/db/sqliteLoadError.ts must classify ${pattern} so a non-callable export ` + + "cascades to the node:sqlite/sql.js fallback instead of disabling the driver." + ); + } +}); + +// Failing loudly is the point, but an operator staring at "Database is not a +// constructor" has no way to know a native rebuild is the remedy. The bootstrap +// wrapper appends that hint without widening what counts as a load failure. +test("a non-callable export still fails loudly, now with a rebuild hint", () => { + const source = readSource("../../../scripts/build/bootstrap-env.mjs"); + + assert.match( + source, + /function isLikelyBrokenNativeBinding\(error\)/, + "the hint predicate must exist and stay separate from isNativeSqliteLoadError" + ); + assert.match(source, /npm rebuild better-sqlite3/, "the remediation hint text must be present"); + + // The hint decorates the throw; it must never turn into a `return false`. + const hintIndex = source.indexOf("npm rebuild better-sqlite3"); + const tail = source.slice(hintIndex, hintIndex + 400); + assert.match( + tail, + /throw new Error\(`Unable to inspect existing database at \$\{dbPath\}: \$\{message\}\$\{hint\}`\)/, + "the broken-binding path must still throw, never report 'no encrypted credentials'" + ); +}); diff --git a/tests/unit/build/dockerfile-better-sqlite3-native.test.mjs b/tests/unit/build/dockerfile-better-sqlite3-native.test.mjs new file mode 100644 index 0000000000..fd3f28c4fa --- /dev/null +++ b/tests/unit/build/dockerfile-better-sqlite3-native.test.mjs @@ -0,0 +1,21 @@ +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import test from "node:test"; + +const dockerfile = readFileSync(new URL("../../../Dockerfile", import.meta.url), "utf8"); + +test("Node runner copies better-sqlite3 and refuses to ship without the native addon", () => { + assert.match(dockerfile, /COPY --from=builder \/app\/node_modules\/better-sqlite3/); + assert.match( + dockerfile, + /node-gyp\.js rebuild --force_build=1/ + ); + assert.match( + dockerfile, + /&& test -f node_modules\/better-sqlite3\/build\/Release\/better_sqlite3\.node/ + ); + assert.match( + dockerfile, + /^RUN test -f \/app\/node_modules\/better-sqlite3\/build\/Release\/better_sqlite3\.node$/m + ); +}); diff --git a/tests/unit/db-core-native-error.test.ts b/tests/unit/db-core-native-error.test.ts index 309e33b8a9..4d78adf0e0 100644 --- a/tests/unit/db-core-native-error.test.ts +++ b/tests/unit/db-core-native-error.test.ts @@ -45,6 +45,16 @@ test("isNativeSqliteLoadError detects MODULE_NOT_FOUND via error.code", () => { assert.equal(isNativeSqliteLoadError(err), true); }); +test("isNativeSqliteLoadError detects minified TypeError 'a is not a function'", () => { + const err = new TypeError("a is not a function"); + assert.equal(isNativeSqliteLoadError(err), true); +}); + +test("isNativeSqliteLoadError detects 'is not a constructor' from a non-function export", () => { + const err = new TypeError("b.default is not a constructor"); + assert.equal(isNativeSqliteLoadError(err), true); +}); + test("isNativeSqliteLoadError returns false for unrelated errors", () => { assert.equal(isNativeSqliteLoadError(new Error("SQLITE_BUSY: database is locked")), false); assert.equal(isNativeSqliteLoadError(new Error("ENOENT: no such file")), false);