v0.20.2: Transaction wrapping for database migrations
- All migrations (versioned, legacy, unversioned) now run within BEGIN/COMMIT with ROLLBACK on failure - v0.40 migration uses try/finally to guarantee PRAGMA foreign_keys is always re-enabled, even on error paths - Clear transaction boundary logging (BEGIN/COMMIT/ROLLBACK) - Hudson security audit: 6/7 PASS, FK fix applied for v0.40 edge case
This commit is contained in:
null
parent
04a0ecbb80
commit
d34316844e
22
FUTURE.md
22
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
|
||||
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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.' },
|
||||
],
|
||||
};
|
||||
134
db/database.js
134
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 {
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
{
|
||||
"name": "bill-tracker",
|
||||
"version": "0.20.1",
|
||||
"version": "0.20.2",
|
||||
"description": "Monthly bill tracking system",
|
||||
"main": "server.js",
|
||||
"scripts": {
|
||||
|
|
|
|||
Loading…
Reference in New Issue