From 1625a104248d750f0b82d159497a09d99dcfadb7 Mon Sep 17 00:00:00 2001 From: roopesh Date: Mon, 2 Nov 2026 01:32:18 +0530 Subject: [PATCH] fix(server): refuse unwritable database in maintenance scripts (#2933) --- server/reset-admin.js | 22 +++- server/scripts/migrate-encryption.ts | 22 +++- .../db/maintenance-scripts-readonly.test.ts | 101 ++++++++++++++++++ wiki/Encryption-Key-Rotation.md | 4 +- wiki/Troubleshooting.md | 2 +- 5 files changed, 145 insertions(+), 6 deletions(-) create mode 100644 server/tests/unit/db/maintenance-scripts-readonly.test.ts diff --git a/server/reset-admin.js b/server/reset-admin.js index 3b62b7579..d5061a983 100644 --- a/server/reset-admin.js +++ b/server/reset-admin.js @@ -2,14 +2,15 @@ * Admin recovery — reset (or create) an admin account when you are locked out. * * Usage inside the container: - * docker exec -it trek node server/reset-admin.js - * docker exec -it -e RESET_ADMIN_EMAIL=me@example.com -e RESET_ADMIN_PASSWORD=secret trek node server/reset-admin.js + * docker exec -it -u node trek node server/reset-admin.js + * docker exec -it -u node -e RESET_ADMIN_EMAIL=me@example.com -e RESET_ADMIN_PASSWORD=secret trek node server/reset-admin.js * * Defaults to admin@trek.local with a generated password (printed below). The * account is flagged must_change_password, so you are prompted to set a new one * on first login. Honours TREK_DB_FILE, TREK_DB_JOURNAL_MODE and * TREK_DB_SYNCHRONOUS the same way the server does. */ +const fs = require('fs'); const path = require('path'); const crypto = require('crypto'); const Database = require('better-sqlite3'); @@ -45,6 +46,23 @@ if (wantedSync && synchronous !== wantedSync) { } const dbPath = process.env.TREK_DB_FILE || path.join(__dirname, 'data/travel.db'); +// SQLite silently opens an unwritable file read-only (#2933); db.readonly can't tell. +const notWritable = [dbPath, path.dirname(dbPath)].find((p) => { + if (!fs.existsSync(p)) return false; + try { + fs.accessSync(p, fs.constants.W_OK); + return false; + } catch { + return true; + } +}); +if (notWritable) { + console.error(`Cannot write to ${notWritable} as uid ${process.getuid?.() ?? '?'}.`); + console.error('Run this script as the user that owns the data directory. In the Docker image that is node:'); + console.error(' docker exec -it -u node trek node server/reset-admin.js'); + process.exit(1); +} + const db = new Database(dbPath); // Spelled out rather than left to better-sqlite3's default, so switching the // journal mode below waits for a busy server instead of failing on the spot. diff --git a/server/scripts/migrate-encryption.ts b/server/scripts/migrate-encryption.ts index d5724be9b..d5bc0b842 100644 --- a/server/scripts/migrate-encryption.ts +++ b/server/scripts/migrate-encryption.ts @@ -9,7 +9,7 @@ * node --import tsx scripts/migrate-encryption.ts * * Usage (Docker): - * docker exec -it trek node --import tsx scripts/migrate-encryption.ts + * docker exec -it -u node -w /app/server trek node --import tsx scripts/migrate-encryption.ts * * The script will prompt for the old and new keys interactively so they never * appear in shell history, process arguments, or log output. @@ -152,6 +152,23 @@ async function main() { console.log(`Database: ${dbPath}\n`); + // SQLite silently opens an unwritable file read-only (#2933); db.readonly can't tell. + const notWritable = [dbPath, path.dirname(dbPath)].find((p) => { + try { + fs.accessSync(p, fs.constants.W_OK); + return false; + } catch { + return true; + } + }); + if (notWritable) { + rl.close(); + console.error(`Cannot write to ${notWritable} as uid ${process.getuid?.() ?? '?'}.`); + console.error('Run this script as the user that owns the data directory. In the Docker image that is node:'); + console.error(' docker exec -it -u node -w /app/server trek node --import tsx scripts/migrate-encryption.ts'); + process.exit(1); + } + // Collect keys interactively const oldKey = await promptSecret('Old ENCRYPTION_KEY: '); const newKey = await promptSecret('New ENCRYPTION_KEY: '); @@ -193,7 +210,8 @@ async function main() { // copy is only the fallback for when it cannot (disk, lock). try { db.exec(`VACUUM INTO '${backupPath.replace(/'/g, "''")}'`); - } catch { + } catch (err) { + console.warn(`VACUUM INTO failed (${(err as Error).message}); falling back to checkpoint + copy.`); db.exec('PRAGMA wal_checkpoint(TRUNCATE)'); fs.copyFileSync(dbPath, backupPath); } diff --git a/server/tests/unit/db/maintenance-scripts-readonly.test.ts b/server/tests/unit/db/maintenance-scripts-readonly.test.ts new file mode 100644 index 000000000..ad238f868 --- /dev/null +++ b/server/tests/unit/db/maintenance-scripts-readonly.test.ts @@ -0,0 +1,101 @@ +/** + * reset-admin.js and migrate-encryption.ts refuse a database they cannot write + * (#2933). A read-only file stands in for the wrong user; root writes it + * regardless, so the cases skip under root. + */ +import Database from 'better-sqlite3'; +import { execFileSync } from 'child_process'; +import fs from 'fs'; +import os from 'os'; +import path from 'path'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; + +const SERVER_ROOT = path.resolve(__dirname, '../../..'); +const isRoot = process.getuid?.() === 0; + +let tmpDir: string; +let dbPath: string; + +beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'trek-readonly-')); + dbPath = path.join(tmpDir, 'travel.db'); + const db = new Database(dbPath); + db.exec(` + CREATE TABLE users ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + username TEXT UNIQUE, + email TEXT UNIQUE, + password_hash TEXT, + role TEXT, + must_change_password INTEGER DEFAULT 0, + password_version INTEGER NOT NULL DEFAULT 0 + ); + `); + db.close(); +}); + +afterEach(() => { + fs.chmodSync(tmpDir, 0o700); + fs.rmSync(tmpDir, { recursive: true, force: true }); +}); + +function run( + args: string[], + env: Record, + input = '', +): { status: number; stdout: string; stderr: string } { + try { + const stdout = execFileSync(process.execPath, args, { + cwd: SERVER_ROOT, + env: { ...process.env, ...env }, + input, + stdio: 'pipe', + encoding: 'utf8', + }); + return { status: 0, stdout, stderr: '' }; + } catch (err) { + const e = err as { status: number; stdout: string; stderr: string }; + return { status: e.status, stdout: e.stdout, stderr: e.stderr }; + } +} + +const resetAdmin = () => run(['reset-admin.js'], { TREK_DB_FILE: dbPath, RESET_ADMIN_PASSWORD: 'Recovery12345!' }); + +const migrateEncryption = () => + run(['--import', 'tsx', 'scripts/migrate-encryption.ts'], { DB_PATH: dbPath }, 'old-key\nnew-key\nyes\n'); + +describe.skipIf(isRoot)('maintenance scripts on a database they cannot write', () => { + it('READONLY-001: reset-admin.js refuses a read-only file and names the user to run as', () => { + fs.chmodSync(dbPath, 0o444); + + const r = resetAdmin(); + + expect(r.status).toBe(1); + expect(r.stderr).toContain(`Cannot write to ${dbPath}`); + expect(r.stderr).toContain('docker exec -it -u node trek node server/reset-admin.js'); + expect(r.stderr).not.toContain('SQLITE_READONLY'); + }, 60000); + + it('READONLY-002: reset-admin.js refuses a directory it cannot write', () => { + fs.chmodSync(tmpDir, 0o555); + + const r = resetAdmin(); + + expect(r.status).toBe(1); + expect(r.stderr).toContain(`Cannot write to ${tmpDir}`); + }, 60000); + + it('READONLY-003: migrate-encryption.ts refuses before asking for a key, and writes no backup', () => { + fs.chmodSync(dbPath, 0o444); + + const r = migrateEncryption(); + + expect(r.status).toBe(1); + expect(r.stdout).not.toContain('Old ENCRYPTION_KEY'); + expect(r.stderr).toContain(`Cannot write to ${dbPath}`); + expect(r.stderr).toContain( + 'docker exec -it -u node -w /app/server trek node --import tsx scripts/migrate-encryption.ts', + ); + expect(fs.readdirSync(tmpDir).filter((f) => f.includes('.backup-'))).toEqual([]); + }, 120000); +}); diff --git a/wiki/Encryption-Key-Rotation.md b/wiki/Encryption-Key-Rotation.md index 4692d5901..1aac8632f 100644 --- a/wiki/Encryption-Key-Rotation.md +++ b/wiki/Encryption-Key-Rotation.md @@ -61,9 +61,11 @@ Use `scripts/migrate-encryption.ts` to re-encrypt the stored secrets listed belo **Docker:** ```bash -docker exec -it -w /app/server trek node --import tsx scripts/migrate-encryption.ts +docker exec -it -u node -w /app/server trek node --import tsx scripts/migrate-encryption.ts ``` +Run it as `node`; a plain `docker exec` runs as root, which the shipped compose file (`cap_drop: ALL`) stops from writing `/app/data`. + **Host (run from the `server/` directory):** ```bash diff --git a/wiki/Troubleshooting.md b/wiki/Troubleshooting.md index 68c9e79d5..17cfa93ec 100644 --- a/wiki/Troubleshooting.md +++ b/wiki/Troubleshooting.md @@ -40,7 +40,7 @@ Log in with what it shows; you will be asked to set a new password. **Reset the admin without losing data** (locked-out, existing install): ```bash -docker exec -it trek node server/reset-admin.js +docker exec -it -u node trek node server/reset-admin.js ``` This resets (or creates) `admin@trek.local` and prints a generated password. Override with `-e RESET_ADMIN_EMAIL=you@example.com -e RESET_ADMIN_PASSWORD=yourpass`. You will be asked to change it on first login. Resetting an existing account also signs it out of every session it still had.