From b5aa468853849744578117bba6f825abb77e9713 Mon Sep 17 00:00:00 2001 From: Pedro Del Moral Lopez Date: Sat, 30 May 2026 21:51:02 -0400 Subject: [PATCH] fix: validate applied migration checksums --- .changeset/validate-migration-checksums.md | 5 + .../migrations/src/runner/MigrationRunner.ts | 92 +++++++++++---- .../src/runner/tests/MigrationRunner.test.ts | 108 ++++++++++++++++-- 3 files changed, 173 insertions(+), 32 deletions(-) create mode 100644 .changeset/validate-migration-checksums.md diff --git a/.changeset/validate-migration-checksums.md b/.changeset/validate-migration-checksums.md new file mode 100644 index 00000000..37f223fd --- /dev/null +++ b/.changeset/validate-migration-checksums.md @@ -0,0 +1,5 @@ +--- +"@danceroutine/tango-migrations": patch +--- + +Validate stored migration checksums before treating applied migrations as unchanged. diff --git a/packages/migrations/src/runner/MigrationRunner.ts b/packages/migrations/src/runner/MigrationRunner.ts index 0f5a73c0..4db60e16 100644 --- a/packages/migrations/src/runner/MigrationRunner.ts +++ b/packages/migrations/src/runner/MigrationRunner.ts @@ -10,6 +10,7 @@ import { isError } from '@danceroutine/tango-core'; import { readdir } from 'node:fs/promises'; import { resolve } from 'node:path'; import { loadDefaultExport } from '../runtime/loadModule'; +import { webcrypto } from 'node:crypto'; const JOURNAL = '_tango_migrations'; @@ -21,6 +22,16 @@ interface DBClient { close(): Promise; } +interface AppliedMigrationRecord { + id: string; + checksum: string; +} + +interface MigrationChecksums { + current: string; + legacyOperations: string; +} + /** * Manages the lifecycle of database migrations: applying, planning, and tracking status. * @@ -84,7 +95,9 @@ export class MigrationRunner { if (toId && migration.id > toId) { break; } - if (applied.has(migration.id)) { + const storedChecksum = applied.get(migration.id); + if (storedChecksum !== undefined) { + await this.assertAppliedChecksum(migration, storedChecksum); continue; } @@ -126,10 +139,21 @@ export class MigrationRunner { const applied = await this.listApplied(); const migrations = await this.loadMigrations(); - return migrations.map((m) => ({ - id: m.id, - applied: applied.has(m.id), - })); + const statuses: { id: string; applied: boolean }[] = []; + + for (const migration of migrations) { + const storedChecksum = applied.get(migration.id); + if (storedChecksum !== undefined) { + await this.assertAppliedChecksum(migration, storedChecksum); + } + + statuses.push({ + id: migration.id, + applied: storedChecksum !== undefined, + }); + } + + return statuses; } private async ensureJournal(): Promise { @@ -149,10 +173,10 @@ export class MigrationRunner { await this.client.query(sql); } - private async listApplied(): Promise> { + private async listApplied(): Promise> { const table = this.dialect === InternalDialect.POSTGRES ? `"${JOURNAL}"` : JOURNAL; - const { rows } = await this.client.query<{ id: string }>(`SELECT id FROM ${table}`); - return new Set(rows.map((r) => r.id)); + const { rows } = await this.client.query(`SELECT id, checksum FROM ${table}`); + return new Map(rows.map((record) => [record.id, record.checksum])); } private async loadMigrations(): Promise { @@ -194,7 +218,7 @@ export class MigrationRunner { const preparedOps = this.compilerStrategy.prepareOperations(this.dialect, builder.ops); const sqls = preparedOps.flatMap((op) => this.compileOperation(op)); - const checksum = String(this.hashJSON(builder.ops)); + const checksum = await this.calculateBuilderChecksum(builder); const isOnline = (migration.mode ?? builder.getMode()) === 'online'; @@ -229,23 +253,47 @@ export class MigrationRunner { } } + private async assertAppliedChecksum(migration: Migration, storedChecksum: string): Promise { + const checksums = await this.calculateChecksums(migration); + + // Older Tango versions stored operation-only checksums. Keep those + // journals readable while writing data-aware checksums for new rows. + if (checksums.current !== storedChecksum && checksums.legacyOperations !== storedChecksum) { + throw new Error( + `Applied migration '${migration.id}' checksum mismatch. The migration file may have been modified after it was applied.` + ); + } + } + + private async calculateChecksums(migration: Migration): Promise { + const builder = new CollectingBuilder(); + await migration.up(builder); + + return { + current: await this.calculateBuilderChecksum(builder), + legacyOperations: await this.calculateLegacyOperationsChecksum(builder), + }; + } + + private async calculateBuilderChecksum(builder: CollectingBuilder): Promise { + return this.hashPayload({ + operations: builder.ops, + dataFns: builder.dataFns.map((fn) => fn.toString()), + }); + } + + private async calculateLegacyOperationsChecksum(builder: CollectingBuilder): Promise { + return this.hashPayload(builder.ops); + } + /** - * Compute a simple hash of the migration's operation list. + * Compute a SHA-256 digest of a migration checksum payload. * Stored alongside each applied migration in the journal table to detect * if a migration file has been modified after it was already applied. - * Uses a djb2-like hash over the JSON-serialized operations. */ - private hashJSON(x: unknown): number { - const s = JSON.stringify(x); - let h = 0; - for (let i = 0; i < s.length; i++) { - // oxlint-disable-next-line prefer-code-point - h = Math.imul(31, h) + s.charCodeAt(i); - // oxlint-disable-next-line prefer-math-trunc - h = h | 0; - } - // oxlint-disable-next-line unicorn/prefer-math-trunc - return h >>> 0; + private async hashPayload(payload: unknown): Promise { + const digest = await webcrypto.subtle.digest('SHA-256', Buffer.from(JSON.stringify(payload), 'utf8')); + return Buffer.from(digest).toString('hex'); } private compileOperation(op: MigrationOperation): SQL[] { diff --git a/packages/migrations/src/runner/tests/MigrationRunner.test.ts b/packages/migrations/src/runner/tests/MigrationRunner.test.ts index e94d6f07..e06003be 100644 --- a/packages/migrations/src/runner/tests/MigrationRunner.test.ts +++ b/packages/migrations/src/runner/tests/MigrationRunner.test.ts @@ -5,6 +5,7 @@ import { aDBClient } from '@danceroutine/tango-testing'; import { MigrationRunner } from '../MigrationRunner'; import { InternalDialect } from '../../domain/internal/InternalDialect'; import type { CompilerStrategy } from '../../strategies/CompilerStrategy'; +import { webcrypto } from 'node:crypto'; const DOMAIN_IMPORT = '../src/domain/index.ts'; @@ -20,7 +21,7 @@ function makeClient(queryImpl?: (sql: string, params?: readonly unknown[]) => Pr if (queryImpl) { return queryImpl(sql, params); } - if (sql.includes('SELECT id FROM')) { + if (sql.includes('SELECT id, checksum FROM')) { return { rows: [] }; } return { rows: [] }; @@ -36,6 +37,22 @@ function strategyReturning(sql: string): CompilerStrategy { } as unknown as CompilerStrategy; } +async function checksumForPayload(payload: unknown): Promise { + const digest = await webcrypto.subtle.digest('SHA-256', Buffer.from(JSON.stringify(payload), 'utf8')); + return Buffer.from(digest).toString('hex'); +} + +function tableDropChecksum(table: string): Promise { + return checksumForPayload([{ kind: 'table.drop', table }]); +} + +function tableDropMigrationChecksum(table: string, dataFns: string[] = []): Promise { + return checksumForPayload({ + operations: [{ kind: 'table.drop', table }], + dataFns, + }); +} + describe(MigrationRunner, () => { const tempDirs: string[] = []; @@ -52,7 +69,7 @@ describe(MigrationRunner, () => { tempDirs.push(dir); const client = makeClient(async (sql) => { - if (sql.includes('SELECT id FROM')) return { rows: [] }; + if (sql.includes('SELECT id, checksum FROM')) return { rows: [] }; return { rows: [] }; }); const runner = new MigrationRunner( @@ -83,7 +100,7 @@ describe(MigrationRunner, () => { tempDirs.push(dir); const client = makeClient(async (sql) => { - if (sql.includes('SELECT id FROM')) return { rows: [] }; + if (sql.includes('SELECT id, checksum FROM')) return { rows: [] }; if (sql === 'SELECT FAIL') throw new Error('boom'); return { rows: [] }; }); @@ -101,7 +118,7 @@ describe(MigrationRunner, () => { tempDirs.push(dir); const client = makeClient(async (sql) => { - if (sql.includes('SELECT id FROM')) return { rows: [] }; + if (sql.includes('SELECT id, checksum FROM')) return { rows: [] }; if (sql === 'SELECT FAIL SQLITE') throw new Error('sqlite-fail'); return { rows: [] }; }); @@ -125,7 +142,9 @@ describe(MigrationRunner, () => { tempDirs.push(dir); const client = makeClient(async (sql) => { - if (sql.includes('SELECT id FROM')) return { rows: [{ id: '001_one' }] }; + if (sql.includes('SELECT id, checksum FROM')) { + return { rows: [{ id: '001_one', checksum: await tableDropChecksum('a') }] }; + } return { rows: [] }; }); const runner = new MigrationRunner(client, InternalDialect.SQLITE, dir, strategyReturning('SELECT 1')); @@ -136,13 +155,62 @@ describe(MigrationRunner, () => { expect(inserts).toHaveLength(0); }); + it('detects a changed applied migration before skipping it', async () => { + const dir = await createTempMigrations({ + '001_changed.ts': `import { Migration } from '${DOMAIN_IMPORT}';\nexport default class MChanged extends Migration { id='001_changed'; up(m){ m.run({ kind: 'table.drop', table: 'accounts' }); } down(){} }`, + }); + tempDirs.push(dir); + + const client = makeClient(async (sql) => { + if (sql.includes('SELECT id, checksum FROM')) { + return { rows: [{ id: '001_changed', checksum: await tableDropChecksum('users') }] }; + } + return { rows: [] }; + }); + const runner = new MigrationRunner(client, InternalDialect.SQLITE, dir, strategyReturning('SELECT 1')); + + await expect(runner.apply()).rejects.toThrow("Applied migration '001_changed' checksum mismatch"); + const sqlCalls = vi.mocked(client.query).mock.calls as Array<[string, ...unknown[]]>; + const inserts = sqlCalls.filter((call) => String(call[0]).includes('INSERT INTO _tango_migrations')); + expect(inserts).toHaveLength(0); + }); + + it('detects a changed applied migration data step before skipping it', async () => { + const dir = await createTempMigrations({ + '001_changed_data.ts': `import { Migration } from '${DOMAIN_IMPORT}';\nexport default class MChangedData extends Migration { id='001_changed_data'; up(m){ m.run({ kind: 'table.drop', table: 'users' }); m.data(async (ctx)=>{ await ctx.query('SELECT changed'); }); } down(){} }`, + }); + tempDirs.push(dir); + + const client = makeClient(async (sql) => { + if (sql.includes('SELECT id, checksum FROM')) { + return { + rows: [ + { + id: '001_changed_data', + checksum: await tableDropMigrationChecksum('users', [ + "async (ctx) => { await ctx.query('SELECT original'); }", + ]), + }, + ], + }; + } + return { rows: [] }; + }); + const runner = new MigrationRunner(client, InternalDialect.SQLITE, dir, strategyReturning('SELECT 1')); + + await expect(runner.apply()).rejects.toThrow("Applied migration '001_changed_data' checksum mismatch"); + }); + it('generates plans, statuses, and validates invalid migration modules', async () => { const validDir = await createTempMigrations({ '001_plan.ts': `import { Migration } from '${DOMAIN_IMPORT}';\nexport default class MPlan extends Migration { id='001_plan'; up(m){ m.run({ kind: 'table.drop', table: 'users' }); m.data(async()=>{}); } down(){} }`, + '002_pending.ts': `import { Migration } from '${DOMAIN_IMPORT}';\nexport default class MPending extends Migration { id='002_pending'; up(m){ m.run({ kind: 'table.drop', table: 'posts' }); } down(){} }`, }); tempDirs.push(validDir); const client = makeClient(async (sql) => { - if (sql.includes('SELECT id FROM')) return { rows: [{ id: '001_plan' }] }; + if (sql.includes('SELECT id, checksum FROM')) { + return { rows: [{ id: '001_plan', checksum: await tableDropChecksum('users') }] }; + } return { rows: [] }; }); const runner = new MigrationRunner( @@ -152,7 +220,10 @@ describe(MigrationRunner, () => { strategyReturning('DROP TABLE users') ); await expect(runner.plan()).resolves.toContain('-- (data step present)'); - await expect(runner.status()).resolves.toEqual([{ id: '001_plan', applied: true }]); + await expect(runner.status()).resolves.toEqual([ + { id: '001_plan', applied: true }, + { id: '002_pending', applied: false }, + ]); const invalidDir = await createTempMigrations({ '001_invalid.js': 'export default { nope: true };' }); tempDirs.push(invalidDir); @@ -165,6 +236,23 @@ describe(MigrationRunner, () => { await expect(invalidRunner.plan()).rejects.toThrow('Invalid migration module'); }); + it('detects a changed applied migration while reporting status', async () => { + const dir = await createTempMigrations({ + '001_changed.ts': `import { Migration } from '${DOMAIN_IMPORT}';\nexport default class MChanged extends Migration { id='001_changed'; up(m){ m.run({ kind: 'table.drop', table: 'accounts' }); } down(){} }`, + }); + tempDirs.push(dir); + + const client = makeClient(async (sql) => { + if (sql.includes('SELECT id, checksum FROM')) { + return { rows: [{ id: '001_changed', checksum: await tableDropChecksum('users') }] }; + } + return { rows: [] }; + }); + const runner = new MigrationRunner(client, InternalDialect.SQLITE, dir, strategyReturning('SELECT 1')); + + await expect(runner.status()).rejects.toThrow("Applied migration '001_changed' checksum mismatch"); + }); + it('loads typescript migration modules directly', async () => { const dir = await createTempMigrations({ '001_ts.ts': `import { Migration } from '${DOMAIN_IMPORT}'; export default class MTs extends Migration { id='001_ts'; up(m){ m.run({ kind: 'table.drop', table: 'users' }); } down(){} }`, @@ -172,7 +260,7 @@ describe(MigrationRunner, () => { tempDirs.push(dir); const client = makeClient(async (sql) => { - if (sql.includes('SELECT id FROM')) return { rows: [] }; + if (sql.includes('SELECT id, checksum FROM')) return { rows: [] }; return { rows: [] }; }); const runner = new MigrationRunner(client, InternalDialect.SQLITE, dir, strategyReturning('SELECT 1')); @@ -187,7 +275,7 @@ describe(MigrationRunner, () => { tempDirs.push(dir); const client = makeClient(async (sql) => { - if (sql.includes('SELECT id FROM')) return { rows: [] }; + if (sql.includes('SELECT id, checksum FROM')) return { rows: [] }; return { rows: [] }; }); const runner = new MigrationRunner(client, InternalDialect.SQLITE, dir, strategyReturning('SELECT 1')); @@ -202,7 +290,7 @@ describe(MigrationRunner, () => { tempDirs.push(dir); const client = makeClient(async (sql) => { - if (sql.includes('SELECT id FROM')) return { rows: [] }; + if (sql.includes('SELECT id, checksum FROM')) return { rows: [] }; return { rows: [] }; }); const runner = new MigrationRunner(client, InternalDialect.SQLITE, dir, strategyReturning('SELECT 1'));