mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-21 14:36:31 +00:00
fix(db): stop losing secrets and notes to the boot and migration paths
An unreadable or empty encryption-key file was treated the same as a missing one: regenerate and overwrite. That is the one case where regenerating is the worst possible move - every secret already at rest becomes undecryptable, permanently. Both now refuse to start and say which file to fix. The key-rotation script advertised re-encrypting all stored secrets while its hardcoded column lists had drifted: the AirTrail key, the per-user LLM key and the plugin OAuth tokens were never migrated, so they silently stopped decrypting after a rotation. The parity test now pins the full set against what the code actually encrypts, rather than two tables of it. The schema_version bump ran outside the migration's transaction, so a crash in between left the schema advanced and the version stale - the next boot replayed a non-idempotent step and exited, turning one crash into a boot loop. And the TICKETJSON receipt migration matched with LIKE, which SQLite treats case-insensitively, so a note somebody had typed starting 'ticketjson:' was cut up and its text dropped. Also: APP_VERSION was interpolated into migration SQL instead of bound, the capture-metadata backfill passed (photoId, userId) to a (userId, photoId) signature and its mock keyed on the wrong argument, TRUST_PROXY=0 was accepted and silently coerced to 1, demo-reset hardcoded the db path instead of honouring TREK_DB_FILE, and a demo instance with no DEMO_ADMIN_PASS set now says out loud that its admin password is the published default.
This commit is contained in:
@@ -108,7 +108,6 @@ The exemption list is enforced by the `no-restricted-syntax` ban on
|
||||
|
||||
- `numberOr` (`Number(x) || d`) treats `"0"` and negative-invalid values oddly:
|
||||
`PORT=0` silently becomes 3001, `TREK_PLUGIN_RPC_BURST=-5` stays -5.
|
||||
- `TRUST_PROXY=0` resolves to 1 hop (`parseInt || 1`).
|
||||
- APP_URL trailing-slash stripping differs per site (feeds strips one slash,
|
||||
notifications strips all).
|
||||
- `DEMO_ADMIN_EMAIL` defaults differ (demo-seed: admin@trek.app, demo-reset:
|
||||
|
||||
@@ -63,6 +63,9 @@ function resolveDefaultLanguage(raw: string | undefined): string {
|
||||
}
|
||||
|
||||
export function deriveHttp(raw: RawEnv) {
|
||||
// env.schema.ts accepts 0 as a valid hop count, so `|| 1` would quietly turn
|
||||
// "trust nothing" into "trust one hop" and let a forged X-Forwarded-For through.
|
||||
const trustProxyHops = Number.parseInt(raw.TRUST_PROXY ?? '', 10);
|
||||
return {
|
||||
allowedOriginsRaw: raw.ALLOWED_ORIGINS,
|
||||
/** globalMiddleware CORS variant: trim + drop empty entries. */
|
||||
@@ -70,7 +73,7 @@ export function deriveHttp(raw: RawEnv) {
|
||||
/** websocket variant: trim only — an empty entry stays (and can never match an Origin header). */
|
||||
wsOrigins: csvList(raw.ALLOWED_ORIGINS),
|
||||
trustProxyRaw: raw.TRUST_PROXY,
|
||||
trustProxy: Number.parseInt(raw.TRUST_PROXY ?? '') || 1,
|
||||
trustProxy: Number.isFinite(trustProxyHops) ? trustProxyHops : 1,
|
||||
forceHttps: parseBool(raw.FORCE_HTTPS) === true,
|
||||
hstsIncludeSubdomains: parseBool(raw.HSTS_INCLUDE_SUBDOMAINS) === true,
|
||||
/** COOKIE_SECURE is tri-state: an explicit falsy value disables secure cookies; anything else means auto-detect. */
|
||||
@@ -144,6 +147,8 @@ export function deriveDemo(raw: RawEnv) {
|
||||
/** Raw on purpose: demo-seed defaults to admin@trek.app, demo-reset to admin@nomad.app. */
|
||||
adminEmailRaw: raw.DEMO_ADMIN_EMAIL,
|
||||
adminPass: raw.DEMO_ADMIN_PASS || 'admin12345',
|
||||
/** False means the admin account is being created with the published default. */
|
||||
adminPassSet: !!raw.DEMO_ADMIN_PASS,
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
+21
-1
@@ -36,10 +36,30 @@ if (_encryptionKey) {
|
||||
// Try the dedicated key file first (covers all installs after first start).
|
||||
try {
|
||||
_encryptionKey = fs.readFileSync(encKeyFile, 'utf8').trim();
|
||||
} catch {
|
||||
} catch (readErr: unknown) {
|
||||
if ((readErr as NodeJS.ErrnoException)?.code !== 'ENOENT') {
|
||||
// The file is there, we just cannot read it (permissions, I/O error).
|
||||
// Continuing would resolve a different key and overwrite this one below,
|
||||
// which permanently orphans every stored secret. Refuse to start instead.
|
||||
console.error(
|
||||
'FATAL: could not read',
|
||||
encKeyFile,
|
||||
'-',
|
||||
readErr instanceof Error ? readErr.message : readErr,
|
||||
);
|
||||
console.error('Fix the file permissions or set ENCRYPTION_KEY explicitly.');
|
||||
process.exit(1);
|
||||
}
|
||||
// File not found — first start on an existing or fresh install.
|
||||
}
|
||||
|
||||
if (!_encryptionKey && fs.existsSync(encKeyFile)) {
|
||||
// Present but empty: a truncated write, a full disk, a broken restore.
|
||||
// Same reasoning as above. Regenerating here would silently replace the real key.
|
||||
console.error('FATAL:', encKeyFile, 'is empty. Restore it from backup or set ENCRYPTION_KEY.');
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
if (!_encryptionKey) {
|
||||
// One-time migration: existing install upgrading for the first time.
|
||||
// Use the JWT secret as the encryption key and immediately write it to
|
||||
|
||||
@@ -2721,7 +2721,9 @@ function runMigrations(db: Database.Database): void {
|
||||
`CREATE TABLE IF NOT EXISTS migrations (id integer PRIMARY KEY AUTOINCREMENT NOT NULL, timestamp bigint NOT NULL, name varchar NOT NULL);`,
|
||||
);
|
||||
db.exec(`INSERT INTO migrations (timestamp, name) VALUES (1777810195344, 'InitialSchema1777810195344');`);
|
||||
db.exec(`INSERT INTO app_settings (key, value) VALUES ('app_version', '${readEnv().app.appVersion || '3.0.14'}')`);
|
||||
db.prepare("INSERT INTO app_settings (key, value) VALUES ('app_version', ?)").run(
|
||||
readEnv().app.appVersion || '3.0.14',
|
||||
);
|
||||
},
|
||||
// trim leading/trailing whitespace from stored usernames and emails
|
||||
() => {
|
||||
@@ -2739,7 +2741,9 @@ function runMigrations(db: Database.Database): void {
|
||||
db.exec(`INSERT INTO schema_version_new (version) SELECT version FROM schema_version`);
|
||||
db.exec(`DROP TABLE schema_version`);
|
||||
db.exec(`ALTER TABLE schema_version_new RENAME TO schema_version`);
|
||||
db.exec(`UPDATE app_settings SET value = '${readEnv().app.appVersion || '3.0.15'}' WHERE key = 'app_version'`);
|
||||
db.prepare("UPDATE app_settings SET value = ? WHERE key = 'app_version'").run(
|
||||
readEnv().app.appVersion || '3.0.15',
|
||||
);
|
||||
},
|
||||
// Migration: OAuth 2.0 client_credentials grant — allow user-owned confidential
|
||||
// clients to skip the browser consent flow entirely and obtain tokens directly
|
||||
@@ -3847,13 +3851,15 @@ function runMigrations(db: Database.Database): void {
|
||||
// means an expense split by receipt can never carry a written note, and
|
||||
// saving an expense any other way wipes whatever was typed in the budget
|
||||
// table. The receipt moves to its own column and note becomes text again.
|
||||
// GLOB, not LIKE: LIKE is case-insensitive in SQLite, so a hand-written note
|
||||
// starting "ticketjson:" would be chopped up and its text dropped.
|
||||
() => {
|
||||
const hasTicket = db.prepare("SELECT 1 FROM pragma_table_info('budget_items') WHERE name = 'ticket_json'").get();
|
||||
if (!hasTicket) db.exec('ALTER TABLE budget_items ADD COLUMN ticket_json TEXT');
|
||||
db.exec(`
|
||||
UPDATE budget_items
|
||||
SET ticket_json = substr(note, 12), note = NULL
|
||||
WHERE note LIKE 'TICKETJSON:%'
|
||||
WHERE note GLOB 'TICKETJSON:*'
|
||||
`);
|
||||
},
|
||||
|
||||
@@ -4091,15 +4097,24 @@ function runMigrations(db: Database.Database): void {
|
||||
try {
|
||||
const migration = migrations[i];
|
||||
if (typeof migration === 'function') {
|
||||
db.transaction(migration)();
|
||||
// The version bump has to commit together with the migration. Bumped
|
||||
// afterwards, a crash in between leaves the schema advanced and the
|
||||
// version stale, and the next boot replays a step that is not
|
||||
// idempotent, exits 1, and turns one crash into a permanent boot loop.
|
||||
db.transaction(() => {
|
||||
migration();
|
||||
db.prepare('UPDATE schema_version SET version = ?').run(i + 1);
|
||||
})();
|
||||
} else {
|
||||
// raw steps run outside a transaction on purpose (see PRAGMA
|
||||
// foreign_keys above), so the bump stays a separate statement.
|
||||
migration.raw();
|
||||
db.prepare('UPDATE schema_version SET version = ?').run(i + 1);
|
||||
}
|
||||
} catch (err) {
|
||||
console.error(`[migrations] FATAL: Migration ${i + 1} failed, rolled back:`, err);
|
||||
process.exit(1);
|
||||
}
|
||||
db.prepare('UPDATE schema_version SET version = ?').run(i + 1);
|
||||
}
|
||||
console.log(`[DB] Migrations complete — schema version ${migrations.length}`);
|
||||
}
|
||||
|
||||
@@ -3,9 +3,15 @@ import path from 'path';
|
||||
import { readEnv } from '../app-config';
|
||||
|
||||
const dataDir = path.join(__dirname, '../../data');
|
||||
const dbPath = path.join(dataDir, 'travel.db');
|
||||
const baselinePath = path.join(dataDir, 'travel-baseline.db');
|
||||
|
||||
// Where the live DB actually is. database.ts honours TREK_DB_FILE, so hardcoding
|
||||
// data/travel.db here would copy the baseline over an unrelated file and leave
|
||||
// the database we just closed untouched. The open connection knows its own path.
|
||||
function liveDbPath(db: { name: string }): string {
|
||||
return db.name;
|
||||
}
|
||||
|
||||
function resetDemoUser(): void {
|
||||
if (!fs.existsSync(baselinePath)) {
|
||||
console.log('[Demo Reset] No baseline found, skipping. Admin must save baseline first.');
|
||||
@@ -13,6 +19,11 @@ function resetDemoUser(): void {
|
||||
}
|
||||
|
||||
const { db, closeDb, reinitialize } = require('../db/database');
|
||||
const dbPath = liveDbPath(db);
|
||||
if (dbPath === ':memory:') {
|
||||
console.log('[Demo Reset] In-memory database, nothing to restore.');
|
||||
return;
|
||||
}
|
||||
|
||||
// Save admin's current credentials and API keys (these should survive the reset)
|
||||
// NOTE: different default than demo-seed (admin@trek.app) — pinned legacy quirk.
|
||||
@@ -98,6 +109,11 @@ function resetDemoUser(): void {
|
||||
|
||||
function saveBaseline(): void {
|
||||
const { db } = require('../db/database');
|
||||
const dbPath = liveDbPath(db);
|
||||
if (dbPath === ':memory:') {
|
||||
console.log('[Demo] In-memory database, no baseline to save.');
|
||||
return;
|
||||
}
|
||||
|
||||
// Flush WAL so baseline file is self-contained
|
||||
try { db.exec('PRAGMA wal_checkpoint(TRUNCATE)'); } catch (e) {}
|
||||
|
||||
@@ -1,6 +1,9 @@
|
||||
import bcrypt from 'bcryptjs';
|
||||
import Database from 'better-sqlite3';
|
||||
import { readEnv } from '../app-config';
|
||||
// Static like in demo-reset.job.ts: the module top is inert, everything that
|
||||
// touches the database happens inside the functions.
|
||||
import { saveBaseline, hasBaseline } from './demo-reset';
|
||||
|
||||
function seedDemoData(db: Database.Database): { adminId: number; demoId: number } {
|
||||
const ADMIN_USER = readEnv().demo.adminUser;
|
||||
@@ -12,6 +15,15 @@ function seedDemoData(db: Database.Database): { adminId: number; demoId: number
|
||||
// Create admin user if not exists
|
||||
let admin = db.prepare('SELECT id FROM users WHERE email = ?').get(ADMIN_EMAIL) as { id: number } | undefined;
|
||||
if (!admin) {
|
||||
if (!readEnv().demo.adminPassSet) {
|
||||
// The default is published in the docs and in this file, so an operator who
|
||||
// never set DEMO_ADMIN_PASS is handing out an admin account. Say so loudly;
|
||||
// changing the default would lock out every existing demo instance.
|
||||
console.warn(
|
||||
`[Demo] SECURITY: DEMO_ADMIN_PASS is not set. The admin account ${ADMIN_EMAIL} is being created ` +
|
||||
'with the public default password. Set DEMO_ADMIN_PASS before exposing this instance.',
|
||||
);
|
||||
}
|
||||
const hash = bcrypt.hashSync(ADMIN_PASS, 10);
|
||||
const r = db.prepare('INSERT INTO users (username, email, password_hash, role) VALUES (?, ?, ?, ?)').run(ADMIN_USER, ADMIN_EMAIL, hash, 'admin');
|
||||
admin = { id: Number(r.lastInsertRowid) };
|
||||
@@ -46,7 +58,6 @@ function seedDemoData(db: Database.Database): { adminId: number; demoId: number
|
||||
seedExampleTrips(db, admin.id, demo.id);
|
||||
|
||||
// Auto-save baseline after first seed
|
||||
const { saveBaseline, hasBaseline } = require('./demo-reset');
|
||||
if (!hasBaseline()) {
|
||||
saveBaseline();
|
||||
}
|
||||
|
||||
@@ -48,7 +48,7 @@ export class PhotoCaptureBackfillService {
|
||||
continue;
|
||||
}
|
||||
|
||||
const info = await this.resolver.getPhotoInfo(id, userId);
|
||||
const info = await this.resolver.getPhotoInfo(userId, id);
|
||||
if (!info.success) continue;
|
||||
|
||||
this.photos.recordCaptureMetadata(id, {
|
||||
|
||||
@@ -1,4 +1,3 @@
|
||||
import { createRequire } from 'module';
|
||||
import semver from 'semver';
|
||||
import { readEnv } from '../app-config';
|
||||
import { db } from '../db/database.js';
|
||||
|
||||
Reference in New Issue
Block a user