From 858ffe18ef0a8027ab7af08b6997d08cdfea83f2 Mon Sep 17 00:00:00 2001 From: darylmcd Date: Sat, 29 Aug 2026 11:58:15 -0500 Subject: [PATCH] fix(sqlite): resolve TOML `database` paths instead of rewriting them buildDSNFromSource encoded SQLite DSNs as `sqlite:///${database}` unconditionally. SQLiteDSNParser reads the path positionally, so that encoding only round-trips for Windows drive letters and `:memory:`: /var/lib/x.db -> sqlite:////var/lib/x.db -> var/lib/x.db (absolute -> relative) data/x.db -> sqlite:///data/x.db -> /data/x.db (relative -> absolute) The relative case fails loudly with ERR_SQLITE_ERROR 14. The absolute case fails silently: the server starts and serves queries against a different file than the config names. Resolve `database` at load time against the directory holding the config file rather than process.cwd(), so a config selects the same database regardless of where the server was launched from. Normalise separators to forward slashes so the parser's `C:/` drive-letter branch matches, and encode the DSN as `sqlite://` plus a leading-slash path so the value survives the round-trip on both platforms. This also repairs `~` expansion on Windows, which produced `sqlite:///C:\Users\...` and decoded back to `/C:\Users\...`. The existing DSN assertion encoded the old behaviour, so it is updated; the new round-trip tests assert the path the connector actually opens rather than the intermediate DSN string, which is what let this through. Fixes #411 Co-Authored-By: Claude Opus 5 --- src/config/__tests__/toml-loader.test.ts | 92 +++++++++++++++++++++++- src/config/toml-loader.ts | 55 +++++++++++++- 2 files changed, 141 insertions(+), 6 deletions(-) diff --git a/src/config/__tests__/toml-loader.test.ts b/src/config/__tests__/toml-loader.test.ts index 179345c2..2261b06d 100644 --- a/src/config/__tests__/toml-loader.test.ts +++ b/src/config/__tests__/toml-loader.test.ts @@ -1,6 +1,7 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import { loadTomlConfig, buildDSNFromSource, interpolateEnvVars } from '../toml-loader.js'; import type { SourceConfig } from '../../types/config.js'; +import { SQLiteConnector } from '../../connectors/sqlite/index.js'; import fs from 'fs'; import path from 'path'; import os from 'os'; @@ -97,6 +98,60 @@ dsn = "sqlite:///path/to/database.db" expect(result?.sources[0].user).toBeUndefined(); }); + it('should resolve a relative sqlite database against the config file directory', () => { + // Config lives in a subdirectory while the process runs from tempDir, so + // resolving against the config file and resolving against process.cwd() + // produce different answers and the test can tell them apart. + const confDir = path.join(tempDir, 'conf'); + fs.mkdirSync(path.join(confDir, 'data'), { recursive: true }); + const configPath = path.join(confDir, 'dbhub.toml'); + fs.writeFileSync( + configPath, + ` +[[sources]] +id = "rel" +type = "sqlite" +database = "data/app.db" +` + ); + process.argv = ['node', 'test', '--config', configPath]; + + const result = loadTomlConfig(); + + expect(result?.sources[0].database).toBe( + path.join(confDir, 'data', 'app.db').replace(/\\/g, '/') + ); + }); + + it('should leave an absolute sqlite database path unchanged', () => { + const absolute = path.join(tempDir, 'elsewhere', 'app.db').replace(/\\/g, '/'); + const tomlContent = ` +[[sources]] +id = "abs" +type = "sqlite" +database = "${absolute}" +`; + fs.writeFileSync(path.join(tempDir, 'dbhub.toml'), tomlContent); + + const result = loadTomlConfig(); + + expect(result?.sources[0].database).toBe(absolute); + }); + + it('should pass the :memory: sentinel through untouched', () => { + const tomlContent = ` +[[sources]] +id = "mem" +type = "sqlite" +database = ":memory:" +`; + fs.writeFileSync(path.join(tempDir, 'dbhub.toml'), tomlContent); + + const result = loadTomlConfig(); + + expect(result?.sources[0].database).toBe(':memory:'); + }); + it('should reject identity fields that conflict with the DSN', () => { // A DSN already encodes the connection identity; setting a field to a // different value is silently ignored at connection time, so it must error. @@ -267,8 +322,12 @@ database = "~/databases/test.db" const result = loadTomlConfig(); + // Separators are normalised to forward slashes. On Windows path.join + // yields backslashes, and `sqlite:///C:\Users\...` does not match the + // drive-letter branch of SQLiteDSNParser, so the expanded path came back + // out as `/C:\Users\...` and could never be opened. expect(result?.sources[0].database).toBe( - path.join(os.homedir(), 'databases', 'test.db') + path.join(os.homedir(), 'databases', 'test.db').replace(/\\/g, '/') ); }); @@ -1675,7 +1734,34 @@ collation = "utf8mb4_0900_ai_ci" const dsn = buildDSNFromSource(source); - expect(dsn).toBe('sqlite:////path/to/database.db'); + expect(dsn).toBe('sqlite:///path/to/database.db'); + }); + + // The DSN is only an intermediate encoding; what matters is the path the + // connector ends up opening. Asserting the DSN string on its own let an + // encoding that rewrote absolute paths into relative ones pass unnoticed. + describe('SQLite path round-trip through SQLiteDSNParser', () => { + const roundTrip = async (database: string): Promise => { + const dsn = buildDSNFromSource({ id: 'test', type: 'sqlite', database }); + const { dbPath } = await new SQLiteConnector().dsnParser.parse(dsn); + return dbPath; + }; + + it('preserves a POSIX absolute path', async () => { + await expect(roundTrip('/var/lib/app/data.db')).resolves.toBe('/var/lib/app/data.db'); + }); + + it('preserves a Windows drive-letter path', async () => { + await expect(roundTrip('C:/Data/app/data.db')).resolves.toBe('C:/Data/app/data.db'); + }); + + it('preserves a path containing spaces', async () => { + await expect(roundTrip('/var/lib/my app/data.db')).resolves.toBe('/var/lib/my app/data.db'); + }); + + it('preserves the :memory: sentinel', async () => { + await expect(roundTrip(':memory:')).resolves.toBe(':memory:'); + }); }); it('should encode special characters in credentials', () => { @@ -1876,7 +1962,7 @@ database = "~/databases/local.db" type: 'sqlite', }); expect(result?.sources[2].database).toBe( - path.join(os.homedir(), 'databases', 'local.db') + path.join(os.homedir(), 'databases', 'local.db').replace(/\\/g, '/') ); }); diff --git a/src/config/toml-loader.ts b/src/config/toml-loader.ts index 28f3315a..6ea86caa 100644 --- a/src/config/toml-loader.ts +++ b/src/config/toml-loader.ts @@ -705,6 +705,55 @@ function validateSourceConfig(source: SourceConfig, configPath: string): void { } } +/** + * Resolve a SQLite source's `database` field to a concrete absolute path. + * + * A relative path is resolved against the directory holding the config file + * rather than process.cwd(), so a config names the same database no matter + * where the server was launched from. + * + * Separators are normalised to forward slashes: path.resolve yields backslashes + * on Windows, and the drive-letter branch of SQLiteDSNParser only matches + * `C:/...`, so a backslash path would survive the DSN round-trip with a spurious + * leading slash. + * + * `:memory:` is a sentinel rather than a path and is passed through untouched. + */ +function resolveSqliteDatabasePath(database: string, configPath: string): string { + if (database === ":memory:") { + return database; + } + + const expanded = expandHomeDir(database); + const absolute = path.isAbsolute(expanded) + ? expanded + : path.resolve(path.dirname(configPath), expanded); + + return absolute.replace(/\\/g, "/"); +} + +/** + * Encode a SQLite database path as a DSN that SQLiteDSNParser decodes back to + * the same path. + * + * The parser reads the path positionally, so the slash count is load-bearing. + * `sqlite://` followed by a leading-slash path yields pathname `/`, which + * the parser returns unchanged for POSIX paths and de-prefixes for `C:/` drive + * letters. Prefixing three slashes unconditionally instead turns + * `/var/lib/x.db` into `sqlite:////var/lib/x.db`, whose pathname starts with + * `//` and loses its leading slash on the way back out — silently converting an + * absolute path into a relative one. + * + * `database` is expected to already be resolved (see resolveSqliteDatabasePath). + */ +function buildSqliteDSN(database: string): string { + if (database === ":memory:") { + return "sqlite:///:memory:"; + } + + return `sqlite://${database.startsWith("/") ? database : `/${database}`}`; +} + /** * Process source configurations (expand paths, populate fields from DSN) */ @@ -725,9 +774,9 @@ function processSourceConfigs( processed.sslrootcert = expandHomeDir(processed.sslrootcert); } - // Expand ~ in SQLite database path (if relative) + // Expand ~ and resolve a relative SQLite database path if (processed.type === "sqlite" && processed.database) { - processed.database = expandHomeDir(processed.database); + processed.database = resolveSqliteDatabasePath(processed.database, configPath); } // Expand ~ in DSN for SQLite @@ -917,7 +966,7 @@ export function buildDSNFromSource(source: SourceConfig): string { `Source '${source.id}': 'database' field is required for SQLite` ); } - return `sqlite:///${source.database}`; + return buildSqliteDSN(source.database); } // For other databases, require host, user, database