Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions src/db/migrations.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ import { describe, it, expect } from 'vitest';
import { registry } from './migrations.js';

describe('migrations registry', () => {
it('has all 12 migrations registered', () => {
expect(registry.count()).toBe(12);
it('has all 13 migrations registered', () => {
expect(registry.count()).toBe(13);
});

it('first migration is v37 baseline', () => {
Expand All @@ -12,11 +12,11 @@ describe('migrations registry', () => {
expect(all[0].name).toContain('v37_baseline');
});

it('last migration is the auth schema alignment', () => {
it('last migration is the audit_log missing columns fix', () => {
const all = registry.getAll();
const last = all[all.length - 1];
expect(last.number).toBe(12);
expect(last.name).toContain('auth');
expect(last.number).toBe(13);
expect(last.name).toContain('audit_log');
});

it('migrations are sequentially numbered from 1 to 12', () => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Test Will Fail

This assertion is incorrect. The test expects migrations numbered 1 to 12, but with the addition of migration 013, we now have 13 migrations numbered 1 to 13.

Suggested change
it('migrations are sequentially numbered from 1 to 12', () => {
it('migrations are sequentially numbered from 1 to 13', () => {

Expand Down Expand Up @@ -46,7 +46,7 @@ describe('migrations registry', () => {
}
});

it('migrations 002-012 all have settingsKey', () => {
it('migrations 002-013 all have settingsKey', () => {
const all = registry.getAll();
for (let i = 1; i < all.length; i++) {
expect(all[i].settingsKey, `Migration ${all[i].number} should have settingsKey`).toBeTruthy();
Expand Down
17 changes: 16 additions & 1 deletion src/db/migrations.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* Migration Registry Barrel File
*
* Registers all 12 migrations in sequential order for use by the migration runner.
* Registers all 13 migrations in sequential order for use by the migration runner.
* Migration 001 is the v3.7 baseline (selfIdempotent — handles its own detection).
* Migrations 002-011 were originally 078-087 and retain their original settingsKeys
* for upgrade compatibility.
Expand All @@ -24,6 +24,7 @@ import { migration as fixCustomThemesColumnsMigration, runMigration085Postgres,
import { runMigration086Sqlite, runMigration086Postgres, runMigration086Mysql } from '../server/migrations/010_add_auto_distance_delete_log.js';
import { migration as fixMessageNodeNumBigintMigration, runMigration087Postgres, runMigration087Mysql } from '../server/migrations/011_fix_message_nodenum_bigint.js';
import { migration as authAlignMigration, runMigration012Postgres, runMigration012Mysql } from '../server/migrations/012_align_sqlite_auth_schema.js';
import { migration as auditLogColumnsMigration, runMigration013Postgres, runMigration013Mysql } from '../server/migrations/013_add_audit_log_missing_columns.js';

// ============================================================================
// Registry
Expand Down Expand Up @@ -154,3 +155,17 @@ registry.register({
postgres: (client) => runMigration012Postgres(client),
mysql: (pool) => runMigration012Mysql(pool),
});

// ---------------------------------------------------------------------------
// Migration 013: Add missing ip_address/user_agent columns to audit_log
// Pre-3.7 SQLite databases may lack these columns.
// ---------------------------------------------------------------------------

registry.register({
number: 13,
name: 'add_audit_log_missing_columns',
settingsKey: 'migration_013_add_audit_log_missing_columns',
sqlite: (db) => auditLogColumnsMigration.up(db),
postgres: (client) => runMigration013Postgres(client),
mysql: (pool) => runMigration013Mysql(pool),
});
101 changes: 101 additions & 0 deletions src/server/migrations/013_add_audit_log_missing_columns.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
/**
* Migration 013: Add missing columns to audit_log for pre-3.7 SQLite databases
*
* The Drizzle schema expects ip_address and user_agent columns on audit_log,
* but databases created before the v3.7 baseline may not have them.
* Migration 012 added username but missed these two.
*
* PostgreSQL/MySQL baselines already include these columns, so those are no-ops.
*/
import type { Database } from 'better-sqlite3';
import { logger } from '../../utils/logger.js';

// ============ SQLite ============

export const migration = {
up: (db: Database): void => {
logger.info('Running migration 013 (SQLite): Adding missing audit_log columns...');

// 1. Add ip_address to audit_log
try {
db.exec('ALTER TABLE audit_log ADD COLUMN ip_address TEXT');
logger.debug('Added ip_address column to audit_log');
} catch (e: any) {
if (e.message?.includes('duplicate column')) {
logger.debug('audit_log.ip_address already exists, skipping');
} else {
logger.warn('Could not add ip_address to audit_log:', e.message);
}
}

// 2. Add user_agent to audit_log
try {
db.exec('ALTER TABLE audit_log ADD COLUMN user_agent TEXT');
logger.debug('Added user_agent column to audit_log');
} catch (e: any) {
if (e.message?.includes('duplicate column')) {
logger.debug('audit_log.user_agent already exists, skipping');
} else {
logger.warn('Could not add user_agent to audit_log:', e.message);
}
}
Comment on lines +20 to +41
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Incomplete Schema Alignment

The migration only adds ip_address and user_agent, but looking at the v3.7 baseline (lines 253-266 in 001_v37_baseline.ts), the SQLite audit_log table also includes:

  • value_before TEXT
  • value_after TEXT

These columns are missing from the Drizzle schema as well. For complete schema alignment across all database versions, this migration should also add these missing columns:

Suggested change
try {
db.exec('ALTER TABLE audit_log ADD COLUMN ip_address TEXT');
logger.debug('Added ip_address column to audit_log');
} catch (e: any) {
if (e.message?.includes('duplicate column')) {
logger.debug('audit_log.ip_address already exists, skipping');
} else {
logger.warn('Could not add ip_address to audit_log:', e.message);
}
}
// 2. Add user_agent to audit_log
try {
db.exec('ALTER TABLE audit_log ADD COLUMN user_agent TEXT');
logger.debug('Added user_agent column to audit_log');
} catch (e: any) {
if (e.message?.includes('duplicate column')) {
logger.debug('audit_log.user_agent already exists, skipping');
} else {
logger.warn('Could not add user_agent to audit_log:', e.message);
}
}
// 1. Add ip_address to audit_log
try {
db.exec('ALTER TABLE audit_log ADD COLUMN ip_address TEXT');
logger.debug('Added ip_address column to audit_log');
} catch (e: any) {
if (e.message?.includes('duplicate column')) {
logger.debug('audit_log.ip_address already exists, skipping');
} else {
logger.warn('Could not add ip_address to audit_log:', e.message);
}
}
// 2. Add user_agent to audit_log
try {
db.exec('ALTER TABLE audit_log ADD COLUMN user_agent TEXT');
logger.debug('Added user_agent column to audit_log');
} catch (e: any) {
if (e.message?.includes('duplicate column')) {
logger.debug('audit_log.user_agent already exists, skipping');
} else {
logger.warn('Could not add user_agent to audit_log:', e.message);
}
}
// 3. Add value_before to audit_log (if not exists)
try {
db.exec('ALTER TABLE audit_log ADD COLUMN value_before TEXT');
logger.debug('Added value_before column to audit_log');
} catch (e: any) {
if (e.message?.includes('duplicate column')) {
logger.debug('audit_log.value_before already exists, skipping');
} else {
logger.warn('Could not add value_before to audit_log:', e.message);
}
}
// 4. Add value_after to audit_log (if not exists)
try {
db.exec('ALTER TABLE audit_log ADD COLUMN value_after TEXT');
logger.debug('Added value_after column to audit_log');
} catch (e: any) {
if (e.message?.includes('duplicate column')) {
logger.debug('audit_log.value_after already exists, skipping');
} else {
logger.warn('Could not add value_after to audit_log:', e.message);
}
}


logger.info('Migration 013 complete (SQLite): audit_log columns aligned');
},

down: (_db: Database): void => {
logger.debug('Migration 013 down: Not implemented (destructive column drops)');
}
};

// ============ PostgreSQL ============

export async function runMigration013Postgres(client: import('pg').PoolClient): Promise<void> {
logger.info('Running migration 013 (PostgreSQL): Ensuring audit_log columns exist...');

try {
await client.query('ALTER TABLE audit_log ADD COLUMN IF NOT EXISTS "ipAddress" TEXT');
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Critical Issue: Column Name Mismatch

The PostgreSQL migration uses "ipAddress" (camelCase with quotes), but the Drizzle schema defines it as ipAddress (camelCase without quotes). This inconsistency will cause schema validation failures.

Suggested change
await client.query('ALTER TABLE audit_log ADD COLUMN IF NOT EXISTS "ipAddress" TEXT');
await client.query('ALTER TABLE audit_log ADD COLUMN IF NOT EXISTS ipAddress TEXT');

The Drizzle schema in src/db/schema/auth.ts:109 expects ipAddress without quotes.

await client.query('ALTER TABLE audit_log ADD COLUMN IF NOT EXISTS "userAgent" TEXT');
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Critical Issue: Column Name Mismatch

Same issue as above - using quoted "userAgent" instead of unquoted userAgent to match the Drizzle schema.

Suggested change
await client.query('ALTER TABLE audit_log ADD COLUMN IF NOT EXISTS "userAgent" TEXT');
await client.query('ALTER TABLE audit_log ADD COLUMN IF NOT EXISTS userAgent TEXT');

logger.debug('Ensured ipAddress/userAgent exist on audit_log');
} catch (error: any) {
logger.error('Migration 013 (PostgreSQL) failed:', error.message);
throw error;
}

logger.info('Migration 013 complete (PostgreSQL): audit_log columns aligned');
}

// ============ MySQL ============

export async function runMigration013Mysql(pool: import('mysql2/promise').Pool): Promise<void> {
logger.info('Running migration 013 (MySQL): Ensuring audit_log columns exist...');

try {
const [ipRows] = await pool.query(`
SELECT COLUMN_NAME FROM information_schema.COLUMNS
WHERE TABLE_SCHEMA = DATABASE() AND TABLE_NAME = 'audit_log' AND COLUMN_NAME = 'ipAddress'
`);
if (!Array.isArray(ipRows) || ipRows.length === 0) {
await pool.query('ALTER TABLE audit_log ADD COLUMN ipAddress TEXT');
logger.debug('Added ipAddress to audit_log');
} else {
logger.debug('audit_log.ipAddress already exists, skipping');
}

const [uaRows] = await pool.query(`
SELECT COLUMN_NAME FROM information_schema.COLUMNS
WHERE TABLE_SCHEMA = DATABASE() AND TABLE_NAME = 'audit_log' AND COLUMN_NAME = 'userAgent'
`);
if (!Array.isArray(uaRows) || uaRows.length === 0) {
await pool.query('ALTER TABLE audit_log ADD COLUMN userAgent TEXT');
logger.debug('Added userAgent to audit_log');
} else {
logger.debug('audit_log.userAgent already exists, skipping');
}
Comment on lines +74 to +94
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Inconsistent Idempotency Approach

The MySQL implementation uses information_schema queries for idempotency, while PostgreSQL uses IF NOT EXISTS and SQLite uses try/catch. For consistency and performance, consider using MySQL's native IF NOT EXISTS syntax (available in MySQL 5.6+):

Suggested change
const [ipRows] = await pool.query(`
SELECT COLUMN_NAME FROM information_schema.COLUMNS
WHERE TABLE_SCHEMA = DATABASE() AND TABLE_NAME = 'audit_log' AND COLUMN_NAME = 'ipAddress'
`);
if (!Array.isArray(ipRows) || ipRows.length === 0) {
await pool.query('ALTER TABLE audit_log ADD COLUMN ipAddress TEXT');
logger.debug('Added ipAddress to audit_log');
} else {
logger.debug('audit_log.ipAddress already exists, skipping');
}
const [uaRows] = await pool.query(`
SELECT COLUMN_NAME FROM information_schema.COLUMNS
WHERE TABLE_SCHEMA = DATABASE() AND TABLE_NAME = 'audit_log' AND COLUMN_NAME = 'userAgent'
`);
if (!Array.isArray(uaRows) || uaRows.length === 0) {
await pool.query('ALTER TABLE audit_log ADD COLUMN userAgent TEXT');
logger.debug('Added userAgent to audit_log');
} else {
logger.debug('audit_log.userAgent already exists, skipping');
}
try {
await pool.query('ALTER TABLE audit_log ADD COLUMN IF NOT EXISTS ipAddress TEXT');
logger.debug('Ensured ipAddress exists on audit_log');
await pool.query('ALTER TABLE audit_log ADD COLUMN IF NOT EXISTS userAgent TEXT');
logger.debug('Ensured userAgent exists on audit_log');
} catch (error: any) {
logger.error('Migration 013 (MySQL) failed:', error.message);
throw error;
}

This approach is more efficient and matches the PostgreSQL pattern.

} catch (error: any) {
logger.error('Migration 013 (MySQL) failed:', error.message);
throw error;
}

logger.info('Migration 013 complete (MySQL): audit_log columns aligned');
}
Loading