diff --git a/FUTURE.md b/FUTURE.md index 70649b4..9b44daf 100644 --- a/FUTURE.md +++ b/FUTURE.md @@ -2,8 +2,8 @@ **This document tracks potential future enhancements for Bill Tracker.** -**Last Updated:** 2026-05-09 -**Current Version:** v0.21.0 +**Last Updated:** 2026-05-10 +**Current Version:** v0.20.2 ## How to Use This Document @@ -33,25 +33,7 @@ Items are grouped under their priority section heading (`## πŸ”΄ CRITICAL`, `## ### πŸ”΄ CRITICAL -### No Transaction Wrapping for Migrations -**Priority:** CRITICAL -**Status:** PENDING -**Added:** 2026-05-09 by Neo -**Description:** -Migrations are not atomic. If a migration fails partway through, database is left in inconsistent state with no rollback. - -**Rationale:** -- Multi-statement migrations (ALTER TABLE + UPDATE + CREATE INDEX) not wrapped in transactions -- If step 2 fails, step 1 already committed -- No recovery mechanism for partially-applied migrations -- Risk: corrupt schema state that's hard to debug - -**Implementation Notes:** -- Wrap each migration in BEGIN/COMMIT/ROLLBACK -- Error handling must ROLLBACK on any failure -- Log transaction state for debugging -- Test with intentional failures to verify rollback diff --git a/HISTORY.md b/HISTORY.md index eee5519..226fce0 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -1,5 +1,14 @@ # Bill Tracker β€” Changelog +## v0.20.2 + +### Added +- **Transaction wrapping for database migrations** β€” All migrations (versioned, legacy, and unversioned) now run within BEGIN/COMMIT transactions with ROLLBACK on failure, ensuring atomic schema changes +- **PRAGMA foreign_keys safety** β€” v0.40 migration uses try/finally to guarantee FK checks are always re-enabled, even on failure + +### Fixed +- **Hudson audit fix** β€” v0.40 migration now restores foreign_keys = ON in a finally block, preventing FK checks from being left disabled if migration fails + ## v0.20.1 ### Added diff --git a/client/lib/version.js b/client/lib/version.js index 8fbf311..96047fc 100644 --- a/client/lib/version.js +++ b/client/lib/version.js @@ -1,14 +1,10 @@ -export const APP_VERSION = '0.20.1'; +export const APP_VERSION = '0.20.2'; export const APP_NAME = 'BillTracker'; export const RELEASE_NOTES = { - version: '0.20.1', + version: '0.20.2', date: '2026-05-09', highlights: [ - { icon: 'πŸš€', title: 'Code splitting', desc: 'Lazy loading for faster initial page load.' }, - { icon: 'πŸ—ΊοΈ', title: 'Admin Dashboard', desc: 'New admin-only dashboard with roadmap and activity log.' }, - { icon: '🧹', title: 'Session token cleanup', desc: 'Expired sessions auto-purged on startup, daily, and on login.' }, - { icon: 'πŸ”‘', title: 'Admin password reset', desc: 'INIT_ADMIN_PASS now resets existing admin passwords on legacy DBs.' }, - { icon: 'πŸͺŸ', title: 'React Error Boundaries', desc: 'App no longer crashes to white screen on errors.' }, + { icon: 'πŸ“¦', title: 'Transaction Wrapping', desc: 'All database migrations now run within transactions for data integrity.' }, ], }; \ No newline at end of file diff --git a/db/database.js b/db/database.js index 9b0029d..b82f761 100644 --- a/db/database.js +++ b/db/database.js @@ -629,11 +629,16 @@ function reconcileLegacyMigrations() { // Migration changes are NOT present - run the migration to apply them try { console.log(`[migration] Running legacy migration ${migration.version}: ${migration.description}`); + // Wrap legacy migration in transaction + db.exec('BEGIN'); + console.log(`[migration] Transaction BEGIN for legacy ${migration.version}`); migration.run(); recordMigration(migration.version, migration.description); - console.log(`[migration] Applied legacy migration ${migration.version}: ${migration.description}`); + db.exec('COMMIT'); + console.log(`[migration] Transaction COMMIT for legacy ${migration.version}`); } catch (err) { - console.error(`[migration-error] Failed to apply legacy migration ${migration.version}: ${err.message}`); + db.exec('ROLLBACK'); + console.error(`[migration-error] Failed to apply legacy migration ${migration.version}: ${err.message}. Rolled back.`); throw err; } } @@ -980,55 +985,102 @@ function runMigrations() { // ── users: notification columns ─────────────────────────────────────────── // This migration needs to run first since it's not versioned in the schema - const userCols = db.prepare('PRAGMA table_info(users)').all().map(c => c.name); - const newUserCols = [ - ['active', 'INTEGER NOT NULL DEFAULT 1'], - ['is_default_admin', 'INTEGER NOT NULL DEFAULT 0'], - ['notification_email', 'TEXT'], - ['notifications_enabled', 'INTEGER NOT NULL DEFAULT 0'], - ['notify_3d', 'INTEGER NOT NULL DEFAULT 1'], - ['notify_1d', 'INTEGER NOT NULL DEFAULT 1'], - ['notify_due', 'INTEGER NOT NULL DEFAULT 1'], - ['notify_overdue', 'INTEGER NOT NULL DEFAULT 1'], - ]; - for (const [col, def] of newUserCols) { - if (!userCols.includes(col)) { - // Security FIX (2026-05-08): Validate column name and definition to prevent SQL injection - if (!isValidColumnName(col) || !isValidSqlDefinition(def)) { - throw new Error(`Invalid migration: column '${col}' not in whitelist`); + try { + db.exec('BEGIN'); + console.log('[migration] Transaction BEGIN for unversioned user notification columns'); + const userCols = db.prepare('PRAGMA table_info(users)').all().map(c => c.name); + const newUserCols = [ + ['active', 'INTEGER NOT NULL DEFAULT 1'], + ['is_default_admin', 'INTEGER NOT NULL DEFAULT 0'], + ['notification_email', 'TEXT'], + ['notifications_enabled', 'INTEGER NOT NULL DEFAULT 0'], + ['notify_3d', 'INTEGER NOT NULL DEFAULT 1'], + ['notify_1d', 'INTEGER NOT NULL DEFAULT 1'], + ['notify_due', 'INTEGER NOT NULL DEFAULT 1'], + ['notify_overdue', 'INTEGER NOT NULL DEFAULT 1'], + ]; + for (const [col, def] of newUserCols) { + if (!userCols.includes(col)) { + // Security FIX (2026-05-08): Validate column name and definition to prevent SQL injection + if (!isValidColumnName(col) || !isValidSqlDefinition(def)) { + throw new Error(`Invalid migration: column '${col}' not in whitelist`); + } + db.exec(`ALTER TABLE users ADD COLUMN ${col} ${def}`); } - db.exec(`ALTER TABLE users ADD COLUMN ${col} ${def}`); } - } - const defaultAdminName = process.env.INIT_ADMIN_USER || 'admin'; - db.prepare(` - UPDATE users - SET is_default_admin = 1 - WHERE role = 'admin' - AND username = ? - AND NOT EXISTS (SELECT 1 FROM users WHERE is_default_admin = 1) - `).run(defaultAdminName); - db.exec(` - UPDATE users - SET is_default_admin = 1 - WHERE id = ( - SELECT id FROM users + const defaultAdminName = process.env.INIT_ADMIN_USER || 'admin'; + db.prepare(` + UPDATE users + SET is_default_admin = 1 WHERE role = 'admin' - ORDER BY id - LIMIT 1 - ) - AND NOT EXISTS (SELECT 1 FROM users WHERE is_default_admin = 1) - `); + AND username = ? + AND NOT EXISTS (SELECT 1 FROM users WHERE is_default_admin = 1) + `).run(defaultAdminName); + db.exec(` + UPDATE users + SET is_default_admin = 1 + WHERE id = ( + SELECT id FROM users + WHERE role = 'admin' + ORDER BY id + LIMIT 1 + ) + AND NOT EXISTS (SELECT 1 FROM users WHERE is_default_admin = 1) + `); + db.exec('COMMIT'); + console.log('[migration] Transaction COMMIT for unversioned user notification columns'); + } catch (err) { + db.exec('ROLLBACK'); + console.error(`[migration-error] Failed to apply unversioned user notification columns: ${err.message}. Rolled back.`); + throw err; + } // Process all versioned migrations for (const migration of migrations) { if (!hasMigrationBeenApplied(migration.version)) { console.log(`[migration] Applying ${migration.version}: ${migration.description}`); try { - migration.run(); - recordMigration(migration.version, migration.description); + // Special handling for v0.40 migration which uses PRAGMA statements + if (migration.version === 'v0.40') { + // PRAGMA foreign_keys cannot run inside a transaction, so we + // disable FK checks before BEGIN and re-enable in a finally block + // to guarantee FK is always restored even on failure. + const billCols = db.prepare('PRAGMA table_info(bills)').all().map(c => c.name); + const categoryCols = db.prepare('PRAGMA table_info(categories)').all().map(c => c.name); + const needsForeignKeyOff = !billCols.includes('user_id') || !categoryCols.includes('user_id'); + + if (needsForeignKeyOff) { + db.exec('PRAGMA foreign_keys = OFF'); + } + try { + db.exec('BEGIN'); + console.log(`[migration] Transaction BEGIN for ${migration.version}`); + migration.run(); + recordMigration(migration.version, migration.description); + db.exec('COMMIT'); + console.log(`[migration] Transaction COMMIT for ${migration.version}`); + } catch (innerErr) { + db.exec('ROLLBACK'); + console.error(`[migration-error] Failed to apply ${migration.version}: ${innerErr.message}. Rolled back.`); + throw innerErr; + } finally { + // Always restore FK checks β€” even on failure path + if (needsForeignKeyOff) { + db.exec('PRAGMA foreign_keys = ON'); + } + } + } else { + // Standard transaction wrapping for other migrations + db.exec('BEGIN'); + console.log(`[migration] Transaction BEGIN for ${migration.version}`); + migration.run(); + recordMigration(migration.version, migration.description); + db.exec('COMMIT'); + console.log(`[migration] Transaction COMMIT for ${migration.version}`); + } } catch (err) { - console.error(`[migration-error] Failed to apply ${migration.version}: ${err.message}`); + db.exec('ROLLBACK'); + console.error(`[migration-error] Failed to apply ${migration.version}: ${err.message}. Rolled back.`); throw err; } } else { diff --git a/package.json b/package.json index 65d2a19..e58e857 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "bill-tracker", - "version": "0.20.1", + "version": "0.20.2", "description": "Monthly bill tracking system", "main": "server.js", "scripts": {