diff --git a/server/reset-admin.js b/server/reset-admin.js index da41ecca6..cf0b3835a 100644 --- a/server/reset-admin.js +++ b/server/reset-admin.js @@ -50,6 +50,10 @@ const db = new Database(dbPath); // journal mode below waits for a busy server instead of failing on the spot. // Five seconds, same as src/db/database.ts. db.exec('PRAGMA busy_timeout = 5000'); +// Temp files in memory, like every connection src/db/connection.ts opens: a +// container with a read-only root and no tmpfs on /tmp has nowhere to put them +// (#2518). +db.exec('PRAGMA temp_store = MEMORY'); db.exec(`PRAGMA journal_mode = ${journalMode}`); db.exec(`PRAGMA synchronous = ${synchronous}`); diff --git a/server/scripts/migrate-encryption.ts b/server/scripts/migrate-encryption.ts index 5ce11fe7b..36324ae25 100644 --- a/server/scripts/migrate-encryption.ts +++ b/server/scripts/migrate-encryption.ts @@ -184,6 +184,10 @@ async function main() { const db = new Database(dbPath); db.pragma('busy_timeout = 5000'); + // Temp files in memory, like every connection src/db/connection.ts opens: a + // container with a read-only root and no tmpfs on /tmp has nowhere to put + // them, and the rotation below runs as one long transaction (#2518). + db.pragma('temp_store = MEMORY'); // The rollback copy has to be a consistent snapshot, not a plain file copy: // this script is documented as running against a live server, so in WAL mode diff --git a/server/src/db/connection.ts b/server/src/db/connection.ts new file mode 100644 index 000000000..b2923add2 --- /dev/null +++ b/server/src/db/connection.ts @@ -0,0 +1,27 @@ +import Database from 'better-sqlite3'; + +/** + * Opens a SQLite connection. Every handle the server opens comes from here (the + * main database, each plugin's own database, a backup being checked before a + * restore), so what has to hold for all of them is set in one place. + * + * Temporary storage stays in memory. Statement journals, sort runs and transient + * indices move to a temp file once they outgrow a small in-memory buffer, and + * SQLite looks for a directory to hold that file in SQLITE_TMPDIR, TMPDIR, + * /var/tmp, /usr/tmp, /tmp and the working directory. A container with a + * read-only root filesystem and no tmpfs on /tmp has none of them writable, so + * the first statement that needs a temp file fails with SQLITE_IOERR_GETTEMPPATH. + * Deleting a trip is where that showed: the cascade keeps a statement journal + * larger than the 64 KiB SQLite holds before it spills to disk (#2518). + * + * The data directory is the only place TREK can count on being able to write, + * and SQLite has no supported per-connection way to put temp files there, so + * they stay in RAM. The shipped compose file already had them there, its /tmp + * being a tmpfs. VACUUM INTO, the only VACUUM TREK runs, writes straight into + * its target file and builds no temporary database. + */ +export function openDatabase(filename: string, options?: Database.Options): Database.Database { + const db = new Database(filename, options); + db.pragma('temp_store = MEMORY'); + return db; +} diff --git a/server/src/db/database.ts b/server/src/db/database.ts index 6270d5ad4..dbab97d67 100644 --- a/server/src/db/database.ts +++ b/server/src/db/database.ts @@ -1,7 +1,8 @@ -import Database from 'better-sqlite3'; +import type Database from 'better-sqlite3'; import path from 'path'; import fs from 'fs'; import { readEnv } from '../app-config'; +import { openDatabase } from './connection'; import { applyDurabilityPragmas } from './durability'; import { createTables } from './schema'; import { runMigrations } from './migrations'; @@ -39,7 +40,7 @@ function initDb(): void { _db = null; } - _db = new Database(dbPath); + _db = openDatabase(dbPath); // Ahead of the journal switch now: changing journal_mode needs an exclusive // lock, which a sibling process (reset-admin, the rotation script) may hold. _db.exec('PRAGMA busy_timeout = 5000'); diff --git a/server/src/nest/backup/backup.impl.ts b/server/src/nest/backup/backup.impl.ts index 8d623af4a..509be1695 100644 --- a/server/src/nest/backup/backup.impl.ts +++ b/server/src/nest/backup/backup.impl.ts @@ -4,7 +4,8 @@ import path from 'path'; import { pipeline } from 'node:stream/promises'; import { readEnv } from '../../app-config'; import fs from 'fs'; -import Database from 'better-sqlite3'; +import type Database from 'better-sqlite3'; +import { openDatabase } from '../../db/connection'; import { db, closeDb, reinitialize } from '../../db/database'; import { VALID_INTERVALS } from './auto-backup.settings'; import { invalidatePermissionsCache } from '../permissions/permissions-cache'; @@ -436,7 +437,7 @@ export async function restoreFromZip(storage: StorageService, zipPath: string): let uploadedDb: InstanceType | null = null; try { - uploadedDb = new Database(extractedDb, { readonly: true }); + uploadedDb = openDatabase(extractedDb, { readonly: true }); const integrityResult = uploadedDb.prepare('PRAGMA integrity_check').get() as { integrity_check: string }; if (integrityResult.integrity_check !== 'ok') { diff --git a/server/src/nest/plugins/host/plugin-data.service.ts b/server/src/nest/plugins/host/plugin-data.service.ts index ae798e466..a31c6c159 100644 --- a/server/src/nest/plugins/host/plugin-data.service.ts +++ b/server/src/nest/plugins/host/plugin-data.service.ts @@ -1,7 +1,7 @@ import fs from 'node:fs'; import path from 'node:path'; -import Database from 'better-sqlite3'; import type { Database as Db } from 'better-sqlite3'; +import { openDatabase } from '../../../db/connection'; import { pluginDataDir, pluginDbFile, pluginsDataRoot } from '../paths'; /** @@ -63,7 +63,7 @@ export class PluginDataDb { constructor(pluginId: string) { this.pluginId = pluginId; fs.mkdirSync(pluginDataDir(pluginId), { recursive: true }); - this.db = new Database(pluginDbFile(pluginId)); + this.db = openDatabase(pluginDbFile(pluginId)); this.db.pragma('journal_mode = WAL'); this.db.pragma('foreign_keys = ON'); openDbs.add(this); diff --git a/server/tests/unit/db/connection.test.ts b/server/tests/unit/db/connection.test.ts new file mode 100644 index 000000000..3c81b93b5 --- /dev/null +++ b/server/tests/unit/db/connection.test.ts @@ -0,0 +1,207 @@ +import fs from 'fs'; +import os from 'os'; +import path from 'path'; +import Database from 'better-sqlite3'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; + +import { openDatabase } from '../../../src/db/connection'; +import { createTables } from '../../../src/db/schema'; +import { runMigrations } from '../../../src/db/migrations'; +import { DatabaseService } from '../../../src/nest/database/database.service'; +import { TripsService } from '../../../src/nest/trips/trips.service'; +import { + createDayAssignment, + createDayNote, + createPackingItem, + createPlace, + createTrip, + createUser, +} from '../../helpers/factories'; + +// #2518: a container with a read-only root filesystem and no tmpfs on /tmp +// gives SQLite nowhere to put a temp file, so deleting a trip failed with +// SQLITE_IOERR_GETTEMPPATH. Every connection TREK opens keeps its temp storage +// in memory now, and these tests pin that down. + +const SERVER_ROOT = path.resolve(__dirname, '../../..'); +const MEMORY = 2; // PRAGMA temp_store: 0 default, 1 file, 2 memory + +let tmpDir: string; +let schemaFile: string; + +beforeAll(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'trek-connection-')); + // Real schema, built once and copied per test: the delete below has to + // cascade through the same tables and foreign keys a live install has. + schemaFile = path.join(tmpDir, 'schema.db'); + const db = openDatabase(schemaFile); + db.exec('PRAGMA journal_mode = WAL'); + db.exec('PRAGMA foreign_keys = ON'); + createTables(db); + runMigrations(db); + db.exec('PRAGMA wal_checkpoint(TRUNCATE)'); + db.close(); +}); + +afterAll(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); +}); + +function freshCopy(name: string): string { + const file = path.join(tmpDir, `${name}.db`); + fs.copyFileSync(schemaFile, file); + return file; +} + +describe('openDatabase', () => { + it('keeps temp storage in memory on a read-write handle', () => { + const db = openDatabase(freshCopy('rw')); + try { + expect(db.pragma('temp_store', { simple: true })).toBe(MEMORY); + } finally { + db.close(); + } + }); + + it('keeps temp storage in memory on a read-only handle', () => { + const db = openDatabase(freshCopy('ro'), { readonly: true }); + try { + expect(db.readonly).toBe(true); + expect(db.pragma('temp_store', { simple: true })).toBe(MEMORY); + } finally { + db.close(); + } + }); + + it('is what the server opens its main database with', async () => { + const { db } = await import('../../../src/db/database'); + expect(db.pragma('temp_store', { simple: true })).toBe(MEMORY); + }); +}); + +// SQLite names every temp file etilqs_. Linux unlinks the file the +// moment it is opened, so it only shows among the process's open descriptors; +// Windows keeps it in the temp directory until it is closed. macOS offers +// neither view, which is why the test below does not run there. +const canSeeTempFiles = process.platform === 'linux' || process.platform === 'win32'; + +function sqliteTempFiles(): string[] { + if (process.platform === 'linux') { + return fs + .readdirSync('/proc/self/fd') + .flatMap((fd) => { + try { + return [fs.readlinkSync(`/proc/self/fd/${fd}`)]; + } catch { + return []; + } + }) + .filter((target) => target.includes('etilqs_')); + } + return fs.readdirSync(os.tmpdir()).filter((name) => name.startsWith('etilqs_')); +} + +/** + * Builds a trip the size of a real one on `db`, deletes it through + * TripsService.remove and reports which SQLite temp files were open while the + * delete ran. A trigger calls back into the test after the cascade, still + * inside the statement, which is when a spilled statement journal is open. + */ +function deleteTripAndWatchTempFiles(db: Database.Database): { + tempFiles: string[]; + tripLeft: number; + placesLeft: number; +} { + db.exec('PRAGMA journal_mode = WAL'); + db.exec('PRAGMA foreign_keys = ON'); + + const { user } = createUser(db); + const trip = createTrip(db, user.id, { + title: 'Niederlande Hoofddorp', + start_date: '2026-06-01', + end_date: '2026-06-14', + }); + const days = db.prepare('SELECT id FROM days WHERE trip_id = ? ORDER BY day_number').all(trip.id) as { id: number }[]; + for (let i = 0; i < 25; i++) { + const place = createPlace(db, trip.id, { name: `Ort ${i}`, description: 'Grachten und Tulpen. '.repeat(10) }); + createDayAssignment(db, days[i % days.length].id, place.id); + } + days.forEach((day) => createDayNote(db, day.id, trip.id)); + for (let i = 0; i < 20; i++) createPackingItem(db, trip.id, { name: `Teil ${i}` }); + + const before = new Set(sqliteTempFiles()); + const seen = new Set(); + db.function('trek_watch_temp_files', () => { + for (const file of sqliteTempFiles()) if (!before.has(file)) seen.add(file); + return null; + }); + db.exec('CREATE TRIGGER trek_watch_trip_delete AFTER DELETE ON trips BEGIN SELECT trek_watch_temp_files(); END'); + + const none = undefined as never; + const trips = new TripsService(new DatabaseService(db), none, none, none, none, none, none, none, none, none); + trips.remove(trip.id, user.id, 'user'); + + return { + tempFiles: [...seen], + tripLeft: (db.prepare('SELECT COUNT(*) AS n FROM trips WHERE id = ?').get(trip.id) as { n: number }).n, + placesLeft: (db.prepare('SELECT COUNT(*) AS n FROM places WHERE trip_id = ?').get(trip.id) as { n: number }).n, + }; +} + +describe.runIf(canSeeTempFiles)('deleting a trip with content (#2518)', () => { + it('needs a temp file on a connection left at the SQLite default', () => { + // The control: proves the watcher sees temp files, and that this delete is + // one SQLite cannot finish without one where no temp directory is writable. + const db = new Database(freshCopy('default')); + try { + const result = deleteTripAndWatchTempFiles(db); + expect(result.tempFiles.length).toBeGreaterThan(0); + expect(result.tripLeft).toBe(0); + } finally { + db.close(); + } + }); + + it('never touches a temp file on a connection TREK opens', () => { + const db = openDatabase(freshCopy('trek')); + try { + const result = deleteTripAndWatchTempFiles(db); + expect(result.tempFiles).toEqual([]); + expect(result.tripLeft).toBe(0); + expect(result.placesLeft).toBe(0); + } finally { + db.close(); + } + }); +}); + +describe('every SQLite connection goes through openDatabase', () => { + function sourceFiles(dir: string): string[] { + return fs.readdirSync(dir, { withFileTypes: true }).flatMap((entry) => { + const full = path.join(dir, entry.name); + if (entry.isDirectory()) return sourceFiles(full); + return entry.name.endsWith('.ts') ? [full] : []; + }); + } + + it('leaves no other place in src/ constructing a better-sqlite3 handle', () => { + const offenders = sourceFiles(path.join(SERVER_ROOT, 'src')).filter((file) => { + if (file === path.join(SERVER_ROOT, 'src', 'db', 'connection.ts')) return false; + const source = fs.readFileSync(file, 'utf8'); + // A value import (not `import type`) under whatever local name, then `new` on it. + const imported = /import\s+(?!type\b)(\w+)[^;]*?from\s+['"]better-sqlite3['"]/.exec(source)?.[1]; + const required = /(\w+)\s*=\s*require\(\s*['"]better-sqlite3['"]\s*\)/.exec(source)?.[1]; + return [imported, required].some((name) => name && new RegExp(`new\\s+${name}\\s*\\(`).test(source)); + }); + expect(offenders.map((file) => path.relative(SERVER_ROOT, file))).toEqual([]); + }); + + it('is mirrored by the standalone scripts that cannot import it', () => { + // reset-admin.js and the key rotation run in the container without src/, + // so they set the pragma themselves. + for (const script of ['reset-admin.js', 'scripts/migrate-encryption.ts']) { + const source = fs.readFileSync(path.join(SERVER_ROOT, script), 'utf8'); + expect(source, script).toMatch(/PRAGMA temp_store = MEMORY|pragma\('temp_store = MEMORY'\)/); + } + }); +}); diff --git a/server/tests/unit/nest/backup.impl.test.ts b/server/tests/unit/nest/backup.impl.test.ts index ee9e182c0..5bbd528b3 100644 --- a/server/tests/unit/nest/backup.impl.test.ts +++ b/server/tests/unit/nest/backup.impl.test.ts @@ -856,6 +856,7 @@ describe('BACKUP-061 restoreFromZip extraction', () => { .mockReturnValueOnce({ all: vi.fn().mockReturnValue([{ name: 'users' }, { name: 'trips' }, { name: 'trip_members' }, { name: 'places' }, { name: 'days' }]), }), + pragma: vi.fn(), close: vi.fn(), }; DatabaseMock.mockImplementation(function () { @@ -1059,6 +1060,7 @@ describe('BACKUP-042 restoreFromZip — integrity check fails', () => { get: vi.fn().mockReturnValue({ integrity_check: 'corruption' }), all: vi.fn(), }), + pragma: vi.fn(), close: vi.fn(), }; DatabaseMock.mockImplementation(function () { @@ -1095,6 +1097,7 @@ describe('BACKUP-043 restoreFromZip — missing required table', () => { .mockReturnValueOnce({ all: vi.fn().mockReturnValue([{ name: 'users' }, { name: 'trips' }]), }), + pragma: vi.fn(), close: vi.fn(), }; DatabaseMock.mockImplementation(function () { @@ -1156,6 +1159,7 @@ describe('BACKUP-045 restoreFromZip — full success path (no uploads)', () => { { name: 'days' }, ]), }), + pragma: vi.fn(), close: vi.fn(), }; DatabaseMock.mockImplementation(function () { @@ -1166,7 +1170,7 @@ describe('BACKUP-045 restoreFromZip — full success path (no uploads)', () => { it('BACKUP-045a — returns { success: true } on full success', async () => { setupSuccessfulExtraction(); - setupAllTablesPresent(); + const uploaded = setupAllTablesPresent(); fsMock.existsSync.mockImplementation((p: string) => { if (String(p).endsWith('travel.db')) return true; @@ -1180,6 +1184,10 @@ describe('BACKUP-045 restoreFromZip — full success path (no uploads)', () => { const result = await restoreFromZip(stubStorage(), '/data/tmp/upload.zip'); expect(result).toEqual({ success: true }); + // The uploaded database is checked through the shared opener, read-only and + // with its temp files in memory like every other handle (#2518). + expect(DatabaseMock).toHaveBeenCalledWith(expect.stringContaining('travel.db'), { readonly: true }); + expect(uploaded.pragma).toHaveBeenCalledWith('temp_store = MEMORY'); }); it('BACKUP-045b — closeDb is called before file copy operations', async () => { @@ -1325,6 +1333,7 @@ describe('BACKUP-046 restoreFromZip — uploads rehydration through storage', () { name: 'days' }, ]), }), + pragma: vi.fn(), close: vi.fn(), }; DatabaseMock.mockImplementation(function () {