fix: add missing audit_log columns for pre-3.7 SQLite databases#2361
fix: add missing audit_log columns for pre-3.7 SQLite databases#2361
Conversation
….7 databases Pre-3.7 SQLite databases may lack ip_address and user_agent columns on audit_log. Migration 012 added username but missed these two, causing "table audit_log has no column named user_agent" errors. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Code Review SummarySeverity: HIGH - This migration contains several critical issues that could cause data corruption and schema inconsistencies. 🔴 Critical Issues Found
🟡 Implementation Issues
✅ Positive Aspects
Detailed FindingsSee inline comments for specific technical details and fix suggestions. The most critical issue is the column name mismatch that will break the Drizzle ORM when switching between database backends. Recommendations
This PR should not be merged until the critical column naming issues are resolved. |
| 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'); |
There was a problem hiding this comment.
🔴 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.
| 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.
|
|
||
| try { | ||
| 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 "userAgent" TEXT'); |
There was a problem hiding this comment.
🔴 Critical Issue: Column Name Mismatch
Same issue as above - using quoted "userAgent" instead of unquoted userAgent to match the Drizzle schema.
| 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'); |
| 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); | ||
| } | ||
| } |
There was a problem hiding this comment.
🟡 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 TEXTvalue_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:
| 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); | |
| } | |
| } |
| 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'); | ||
| } |
There was a problem hiding this comment.
🟡 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+):
| 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.
| expect(last.name).toContain('audit_log'); | ||
| }); | ||
|
|
||
| it('migrations are sequentially numbered from 1 to 12', () => { |
There was a problem hiding this comment.
🔴 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.
| it('migrations are sequentially numbered from 1 to 12', () => { | |
| it('migrations are sequentially numbered from 1 to 13', () => { |
Summary
Fixes
SqliteError: table audit_log has no column named user_agenterrors on pre-3.7 SQLite databases.Root cause: Migration 012 added
usernametoaudit_logbut missedip_addressanduser_agent. The v3.7 baseline creates these columns for fresh installs, but databases that predate the baseline never got them. Drizzle ORM then fails when trying to insert audit log entries.Fix: Migration 013 adds
ip_addressanduser_agentcolumns toaudit_log(idempotent — skips if already present). All three backends covered (SQLite, PostgreSQL, MySQL).Files Changed
src/server/migrations/013_add_audit_log_missing_columns.tssrc/db/migrations.tssrc/db/migrations.test.tsTest plan
🤖 Generated with Claude Code