From ebd70336b232e35032205106e1730f057d1e9dde Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 19 Oct 2026 01:27:33 +0000 Subject: [PATCH 01/15] feat(migrate): add ColumnMigration and move the engine's opt-in rules into it One module now owns the column-migration rules both directions follow: which twin a column migration fills, whether it is the one in progress, and the forced-pass obligation from ADR 0001. The engine asks it for the pass mode and the resume point instead of taking `trackSkipped`, `resumeFromError` and `canResume` from each caller. - `ColumnMigration.load()` returns `{ phase, twin, obligations, blockedBy }`; phase changes go through `confirmDualWrites({ waiver? })`, `recordDropGenerated()` and `recordSourceRemoved()`. The rules are a pure function of the event history (`deriveColumnMigration`, `planPassFor`), covered by table tests. - Every engine event records the pass mode. An open obligation forces the run, and the result says why (`forcedBy`). A reverse pass over a twin that already held values is owed a forced pass unless a waiver is recorded. - Forward runs now resume after an `error` event, count their total from the resume cursor, and finish an interrupted `--force` pass as a forced pass. - `error` events store `errorName` plus details each direction marks as safe, never `err.message`. - A run is refused while the other direction's column migration of the pair is open, or when its twin contradicts the recorded one. - `forceObligationOpen` and `ChunkedRunEvents` are removed (unreleased). Refs #1217 Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01Scub1tpuegHHCTXrgn7SHf --- .../__tests__/backfill.integration.test.ts | 136 +++- .../src/__tests__/column-migration.test.ts | 423 +++++++++++ .../src/__tests__/decrypt.integration.test.ts | 245 ++++-- .../packages/migrate/src/backfill.ts | 57 +- .../packages/migrate/src/column-migration.ts | 700 ++++++++++++++++++ .../typescript/packages/migrate/src/cursor.ts | 5 +- .../packages/migrate/src/decrypt.ts | 158 ++-- .../typescript/packages/migrate/src/engine.ts | 190 +++-- .../typescript/packages/migrate/src/index.ts | 23 +- .../typescript/packages/migrate/src/state.ts | 3 +- 10 files changed, 1656 insertions(+), 284 deletions(-) create mode 100644 languages/typescript/packages/migrate/src/__tests__/column-migration.test.ts create mode 100644 languages/typescript/packages/migrate/src/column-migration.ts diff --git a/languages/typescript/packages/migrate/src/__tests__/backfill.integration.test.ts b/languages/typescript/packages/migrate/src/__tests__/backfill.integration.test.ts index c94e8db2b..d96a36f29 100644 --- a/languages/typescript/packages/migrate/src/__tests__/backfill.integration.test.ts +++ b/languages/typescript/packages/migrate/src/__tests__/backfill.integration.test.ts @@ -21,8 +21,9 @@ import 'dotenv/config' import pg from 'pg' import { afterAll, afterEach, beforeAll, describe, expect, it } from 'vitest' import { type EncryptionClientLike, runBackfill } from '../backfill.js' +import { ColumnMigrationError } from '../column-migration.js' import { installMigrationsSchema } from '../install.js' -import { latestByColumn, progress } from '../state.js' +import { appendEvent, latestByColumn, progress } from '../state.js' const PG_URL = process.env.PG_TEST_URL @@ -289,9 +290,10 @@ describe.skipIf(!runIntegration)('runBackfill (integration)', () => { try { const state = await progress(readDb, 'migrate_test.users', 'email') expect(state?.event).toBe('error') - expect((state?.details as { message?: string } | null)?.message).toMatch( - /ZeroKMS exploded/, - ) + // The error's name, never its message: a client or database message + // can carry row data. + expect(state?.details).toMatchObject({ errorName: 'Error' }) + expect(JSON.stringify(state?.details)).not.toContain('ZeroKMS exploded') } finally { readDb.release() } @@ -428,4 +430,130 @@ describe.skipIf(!runIntegration)('runBackfill (integration)', () => { readDb.release() } }) + + /** One forward run over `migrate_test.users` with the stub client. */ + async function forward( + overrides: Partial[0]> = {}, + ) { + const db = await pool.connect() + try { + return await runBackfill({ + db, + encryptionClient: stubClient, + tableSchema: { tableName: 'users', build: () => ({}) }, + tableName: 'migrate_test.users', + schemaColumnKey: 'email', + plaintextColumn: 'email', + encryptedColumn: 'email_encrypted', + pkColumn: 'id', + chunkSize: 50, + ...overrides, + }) + } finally { + db.release() + } + } + + async function staleCount(): Promise { + const result = await pool.query<{ n: string }>( + `SELECT count(*)::text AS n FROM migrate_test.users WHERE email_encrypted->>'c' = 'stale'`, + ) + return Number(result.rows[0]?.n ?? 0) + } + + it('a resumed run counts its total once, so progress ends at 100%', async () => { + await seed(300) + const controller = new AbortController() + await forward({ + signal: controller.signal, + onProgress: (p) => { + if (p.rowsProcessed >= 100) controller.abort() + }, + }) + const resumed = await forward() + expect(resumed.resumed).toBe(true) + expect(resumed.rowsTotal).toBe(300) + expect(resumed.rowsProcessed).toBe(300) + }) + + it('resumes after a failed run from its last committed cursor', async () => { + await seed(150) + let calls = 0 + const failSecond: EncryptionClientLike = { + bulkEncryptModels(input, table) { + calls += 1 + return calls === 2 + ? Promise.resolve({ failure: { message: 'boom' } }) + : stubClient.bulkEncryptModels(input, table) + }, + } + await expect(forward({ encryptionClient: failSecond })).rejects.toThrow() + const fixed = await forward() + expect(fixed.resumed).toBe(true) + expect(fixed.rowsProcessed).toBe(150) + expect(await countEncrypted()).toBe(150) + }) + + it('a plain run finishes an interrupted --force pass as a forced pass', async () => { + await seed(300) + await pool.query( + `UPDATE migrate_test.users SET email_encrypted = '{"v":2,"c":"stale"}'::jsonb`, + ) + const controller = new AbortController() + await forward({ + force: true, + signal: controller.signal, + onProgress: (p) => { + if (p.rowsProcessed >= 100) controller.abort() + }, + }) + + // Unforced, this run would find nothing pending and leave rows 101-300 + // stale. The open obligation makes it forced, and it resumes the pass. + const finished = await forward() + expect(finished).toMatchObject({ + resumed: true, + completed: true, + force: true, + forcedBy: 'forced-pass', + }) + expect(await staleCount()).toBe(0) + + // The completed forced pass met the obligation. + expect(await forward()).toMatchObject({ force: false, forcedBy: null }) + }) + + it('records the pass mode on every event', async () => { + await seed(120) + await forward({ force: true }) + const { rows } = await pool.query<{ event: string; force: unknown }>( + `SELECT event, details->'force' AS force FROM cipherstash.cs_migrations ORDER BY id`, + ) + expect(rows.map((r) => r.event)).toEqual([ + 'backfill_started', + 'backfill_checkpoint', + 'backfill_checkpoint', + 'backfill_checkpoint', + 'backfilled', + ]) + expect(rows.every((r) => r.force === true)).toBe(true) + }) + + it('refuses to start while a reverse column migration of the pair is open', async () => { + await seed(10) + const db = await pool.connect() + try { + await appendEvent(db, { + tableName: 'migrate_test.users', + columnName: 'email_encrypted', + event: 'decrypt_started', + phase: 'decrypting', + details: { plaintextColumn: 'email' }, + }) + } finally { + db.release() + } + await expect(forward()).rejects.toBeInstanceOf(ColumnMigrationError) + expect(await countEncrypted()).toBe(0) + }) }) diff --git a/languages/typescript/packages/migrate/src/__tests__/column-migration.test.ts b/languages/typescript/packages/migrate/src/__tests__/column-migration.test.ts new file mode 100644 index 000000000..78d9433fc --- /dev/null +++ b/languages/typescript/packages/migrate/src/__tests__/column-migration.test.ts @@ -0,0 +1,423 @@ +/** + * Table tests for the column-migration rules: event history in, the column + * migration's `{ phase, obligations }` out. No database — the histories are + * the `cs_migrations` rows `ColumnMigration.load` would read. + */ + +import { describe, expect, it } from 'vitest' +import { + type ColumnMigrationState, + deriveColumnMigration, + openCounterpart, + planPassFor, +} from '../column-migration.js' +import type { + MigrationEvent, + MigrationPhase, + MigrationStateRow, +} from '../state.js' + +let nextId = 1 +function ev( + event: MigrationEvent, + phase: MigrationPhase, + details: Record | null = null, + extra: Partial = {}, +): MigrationStateRow { + return { + id: String(nextId++), + tableName: 'users', + columnName: 'email_encrypted', + event, + phase, + cursorValue: null, + rowsProcessed: null, + rowsTotal: null, + details, + createdAt: new Date(0), + ...extra, + } +} + +const twin = { plaintextColumn: 'email' } +const forced = { ...twin, force: true } +const unforced = { ...twin, force: false } + +// Reverse events, named for the table below. +const confirm = () => + ev('decrypt_dual_writing', 'decrypt-dual-writing', { ...twin }) +const confirmOverValues = () => + ev('decrypt_dual_writing', 'decrypt-dual-writing', { + ...twin, + existingTwinValues: true, + }) +const confirmWaived = () => + ev('decrypt_dual_writing', 'decrypt-dual-writing', { + ...twin, + existingTwinValues: true, + waiver: 'plaintext-current', + }) +const started = (details: Record) => + ev('decrypt_started', 'decrypting', details) +const checkpoint = (details: Record, cursor = '100') => + ev('decrypt_checkpoint', 'decrypting', details, { cursorValue: cursor }) +const failed = (details: Record, cursor: string | null) => + ev('error', 'decrypting', details, { cursorValue: cursor }) +const completed = (details: Record) => + ev('decrypted', 'decrypted', details) +const dropGenerated = () => + ev('encrypted_dropped', 'encrypted-dropped', { ...twin }) + +type Expected = Pick & { + obligation: null | { reason: string; waivable: boolean } +} + +const REVERSE_CASES: Array<{ + name: string + history: () => MigrationStateRow[] + expected: Expected +}> = [ + { + name: 'no history: no column migration', + history: () => [], + expected: { phase: null, open: false, obligation: null }, + }, + { + name: 'dual-writes confirmed over an empty twin: nothing owed', + history: () => [confirm()], + expected: { phase: 'decrypt-dual-writing', open: true, obligation: null }, + }, + { + name: 'opened up front: confirmed over a twin that already held values', + history: () => [confirmOverValues()], + expected: { + phase: 'decrypt-dual-writing', + open: true, + obligation: { reason: 'existing-values', waivable: true }, + }, + }, + { + name: 'closed by a waiver recorded at confirmation', + history: () => [confirmWaived()], + expected: { phase: 'decrypt-dual-writing', open: true, obligation: null }, + }, + { + name: 'closed by a waiver recorded on a later confirmation, before any pass', + history: () => [confirmOverValues(), confirmWaived()], + expected: { phase: 'decrypt-dual-writing', open: true, obligation: null }, + }, + { + name: 'opened by a forced pass that started', + history: () => [confirm(), started(forced)], + expected: { + phase: 'decrypting', + open: true, + obligation: { reason: 'forced-pass', waivable: false }, + }, + }, + { + name: 'opened by a forced pass that failed before its first commit', + history: () => [confirm(), started(forced), failed(forced, null)], + expected: { + phase: 'decrypting', + open: true, + obligation: { reason: 'forced-pass', waivable: false }, + }, + }, + { + name: 'a forced pass started over an up-front obligation is no longer waivable', + history: () => [confirmOverValues(), started(forced)], + expected: { + phase: 'decrypting', + open: true, + obligation: { reason: 'forced-pass', waivable: false }, + }, + }, + { + name: 'closed by a completed forced pass', + history: () => [ + confirmOverValues(), + started(forced), + checkpoint(forced), + completed(forced), + ], + expected: { phase: 'decrypted', open: true, obligation: null }, + }, + { + name: 'an unforced completion does not close it', + history: () => [confirm(), started(forced), completed(unforced)], + expected: { + phase: 'decrypted', + open: true, + obligation: { reason: 'forced-pass', waivable: false }, + }, + }, + { + name: 'unforced passes neither open nor close anything', + history: () => [confirm(), started(unforced), completed(unforced)], + expected: { phase: 'decrypted', open: true, obligation: null }, + }, + { + name: 'a pass run with no dual-write confirmation is still scoped', + history: () => [started(forced), checkpoint(forced)], + expected: { + phase: 'decrypting', + open: true, + obligation: { reason: 'forced-pass', waivable: false }, + }, + }, + { + name: 'an obligation left open by an earlier column migration does not carry over', + history: () => [confirmOverValues(), started(forced), confirm()], + expected: { phase: 'decrypt-dual-writing', open: true, obligation: null }, + }, + { + name: 'a generated drop is the current column migration, but no longer open', + history: () => [ + confirm(), + started(unforced), + completed(unforced), + dropGenerated(), + ], + expected: { phase: 'encrypted-dropped', open: false, obligation: null }, + }, + { + name: 'forward events under the same key are not reverse history', + history: () => [ + ev('backfill_checkpoint', 'backfilling', null, { cursorValue: '40' }), + ], + expected: { phase: null, open: false, obligation: null }, + }, +] + +describe('deriveColumnMigration (reverse)', () => { + it.each(REVERSE_CASES)('$name', ({ history, expected }) => { + const state = deriveColumnMigration({ + direction: 'reverse', + history: history(), + }) + expect(state.phase).toBe(expected.phase) + expect(state.open).toBe(expected.open) + const [obligation] = state.obligations + expect( + obligation === undefined + ? null + : { reason: obligation.reason, waivable: obligation.waivable }, + ).toEqual(expected.obligation) + expect(state.obligations.length).toBeLessThanOrEqual(1) + }) + + it('records the twin the latest reverse event names', () => { + const state = deriveColumnMigration({ + direction: 'reverse', + history: [confirm(), started(unforced)], + }) + expect(state.recordedTwin).toBe('email') + }) + + it('a generated drop superseded by a later forward column migration of the twin is history', () => { + const history = [confirm(), completed(unforced), dropGenerated()] + const forwardOfTwin = [ + ev('dual_writing', 'dual-writing', null, { columnName: 'email' }), + ] + const state = deriveColumnMigration({ + direction: 'reverse', + history, + recordedTwinHistory: forwardOfTwin, + }) + expect(state.phase).toBeNull() + expect(state.open).toBe(false) + expect(state.recordedTwin).toBeUndefined() + // …while forward events from BEFORE the drop leave it current. + const before = [ + ev('dropped', 'dropped', null, { columnName: 'email', id: '0' }), + ] + expect( + deriveColumnMigration({ + direction: 'reverse', + history, + recordedTwinHistory: before, + }).phase, + ).toBe('encrypted-dropped') + }) +}) + +describe('deriveColumnMigration (forward)', () => { + const fwd = ( + event: MigrationEvent, + phase: MigrationPhase, + details: Record | null = null, + ) => ev(event, phase, details, { columnName: 'email' }) + + it('a forced pass interrupted by an older release (force only on started) stays owed', () => { + const state = deriveColumnMigration({ + direction: 'forward', + history: [ + fwd('dual_writing', 'dual-writing'), + fwd('backfill_started', 'backfilling', { force: true }), + fwd('backfill_checkpoint', 'backfilling', { chunkIndex: 0 }), + ], + }) + expect(state.obligations.map((o) => o.reason)).toEqual(['forced-pass']) + }) + + it('a dropped forward column migration is finished', () => { + const state = deriveColumnMigration({ + direction: 'forward', + history: [fwd('dual_writing', 'dual-writing'), fwd('dropped', 'dropped')], + }) + expect(state.phase).toBe('dropped') + expect(state.open).toBe(false) + }) + + it('ignores reverse events', () => { + const state = deriveColumnMigration({ + direction: 'forward', + history: [confirm()], + }) + expect(state.phase).toBeNull() + }) +}) + +describe('openCounterpart', () => { + it('refuses a forward-source twin: a forward column migration not yet dropped', () => { + const blocking = openCounterpart({ + direction: 'reverse', + twin: 'email', + twinHistory: [ + ev('dual_writing', 'dual-writing', null, { columnName: 'email' }), + ev('backfilled', 'backfilled', null, { columnName: 'email' }), + ], + }) + expect(blocking).toEqual({ + direction: 'forward', + column: 'email', + phase: 'backfilled', + }) + }) + + it('a dropped forward column migration does not block the reverse one', () => { + expect( + openCounterpart({ + direction: 'reverse', + twin: 'email', + twinHistory: [ev('dropped', 'dropped', null, { columnName: 'email' })], + }), + ).toBeNull() + }) + + it('an open reverse column migration blocks a forward one on the same pair', () => { + expect( + openCounterpart({ + direction: 'forward', + twin: 'email_encrypted', + twinHistory: [started(unforced)], + }), + ).toEqual({ + direction: 'reverse', + column: 'email_encrypted', + phase: 'decrypting', + }) + }) + + it('a generated reverse drop does not block a forward one', () => { + expect( + openCounterpart({ + direction: 'forward', + twin: 'email_encrypted', + twinHistory: [completed(unforced), dropGenerated()], + }), + ).toBeNull() + }) +}) + +describe('planPassFor', () => { + const state = (history: MigrationStateRow[]) => + deriveColumnMigration({ direction: 'reverse', history }) + + it('an open obligation forces a run the caller did not force, and says why', () => { + const plan = planPassFor(state([confirm(), started(forced)]), { + force: false, + }) + expect(plan).toMatchObject({ force: true, forcedBy: 'forced-pass' }) + }) + + it('an up-front obligation forces the first run', () => { + expect( + planPassFor(state([confirmOverValues()]), { force: false }), + ).toMatchObject({ force: true, forcedBy: 'existing-values' }) + }) + + it('a caller-forced run says so', () => { + expect(planPassFor(state([confirm()]), { force: true })).toMatchObject({ + force: true, + forcedBy: 'caller', + }) + }) + + it('nothing owed and nothing asked: unforced', () => { + expect(planPassFor(state([confirm()]), { force: false })).toMatchObject({ + force: false, + forcedBy: null, + resumeFrom: null, + }) + }) + + /** The history, and its last event (ids are assigned in call order). */ + const ending = (...rows: MigrationStateRow[]) => + [rows, rows[rows.length - 1]] as const + + it('resumes a checkpoint of the same mode', () => { + const [history, last] = ending( + confirm(), + started(unforced), + checkpoint(unforced, '200'), + ) + expect(planPassFor(state(history), { force: false }).resumeFrom).toBe(last) + }) + + it('resumes an interrupted forced pass even when the caller did not force', () => { + const [history, last] = ending( + confirm(), + started(forced), + checkpoint(forced, '200'), + ) + const plan = planPassFor(state(history), { force: false }) + expect(plan.force).toBe(true) + expect(plan.resumeFrom).toBe(last) + }) + + it('resumes after a failed run from its committed cursor', () => { + const [history, last] = ending( + confirm(), + started(unforced), + failed(unforced, '300'), + ) + expect(planPassFor(state(history), { force: false }).resumeFrom).toBe(last) + }) + + it('a failure before the first commit has no cursor to resume from', () => { + expect( + planPassFor( + state([confirm(), started(unforced), failed(unforced, null)]), + { force: false }, + ).resumeFrom, + ).toBeNull() + }) + + it('a forced run never resumes an unforced pass', () => { + expect( + planPassFor(state([confirm(), started(unforced), checkpoint(unforced)]), { + force: true, + }).resumeFrom, + ).toBeNull() + }) + + it('a completed pass is not resumed', () => { + expect( + planPassFor(state([confirm(), started(unforced), completed(unforced)]), { + force: false, + }).resumeFrom, + ).toBeNull() + }) +}) diff --git a/languages/typescript/packages/migrate/src/__tests__/decrypt.integration.test.ts b/languages/typescript/packages/migrate/src/__tests__/decrypt.integration.test.ts index 4e11c5de9..71503921e 100644 --- a/languages/typescript/packages/migrate/src/__tests__/decrypt.integration.test.ts +++ b/languages/typescript/packages/migrate/src/__tests__/decrypt.integration.test.ts @@ -21,11 +21,11 @@ import 'dotenv/config' import pg from 'pg' import { afterAll, afterEach, beforeAll, describe, expect, it } from 'vitest' +import { ColumnMigration, ColumnMigrationError } from '../column-migration.js' import { columnHasValues, countUndecrypted } from '../cursor.js' import { type BulkDecryptItem, type DecryptionClientLike, - forceObligationOpen, MAX_DECRYPT_CHUNK_SIZE, PlaintextWriteError, runDecryptBackfill, @@ -334,32 +334,175 @@ describe.skipIf(!runIntegration)('runDecryptBackfill (integration)', () => { expect(rows[0]?.n).toBe('0') }) - it('a stale twin acknowledged with --force at dual-write confirmation is overwritten by a plain run', async () => { + /** The reverse column migration of `email_encrypted`, freshly loaded. */ + async function migration(): Promise { + const db = await pool.connect() + try { + return await ColumnMigration.load(db, { + table: TABLE, + column: 'email_encrypted', + direction: 'reverse', + twin: 'email', + manifest: null, + }) + } finally { + db.release() + } + } + + /** Confirm dual-writes on a connection held for the call. */ + async function confirm(waiver?: 'plaintext-current') { + const db = await pool.connect() + try { + const loaded = await ColumnMigration.load(db, { + table: TABLE, + column: 'email_encrypted', + direction: 'reverse', + twin: 'email', + manifest: null, + }) + return await loaded.confirmDualWrites({ waiver }) + } finally { + db.release() + } + } + + async function staleCount(): Promise { + const { rows } = await pool.query<{ n: string }>( + `SELECT count(*)::text AS n FROM ${TABLE} WHERE email = 'stale@example.com'`, + ) + return rows[0]?.n + } + + it('dual-writes confirmed over a twin that already holds values: a plain run overwrites it', async () => { await seed(300) await pool.query(`UPDATE ${TABLE} SET email = 'stale@example.com'`) - // What `stash decrypt backfill --force` records before its first run. + const confirmed = await confirm() + expect(confirmed.obligations).toMatchObject([ + { reason: 'existing-values', waivable: true }, + ]) + + const result = await run() + expect(result).toMatchObject({ force: true, forcedBy: 'existing-values' }) + expect(await staleCount()).toBe('0') + expect((await migration()).obligations).toEqual([]) + }) + + it('a pass with no column migration on record treats a twin with values as owed', async () => { + // An embedded worker that never confirmed dual-writes. + await seed(50) + await pool.query(`UPDATE ${TABLE} SET email = 'stale@example.com'`) + expect(await run()).toMatchObject({ + force: true, + forcedBy: 'existing-values', + }) + expect(await staleCount()).toBe('0') + }) + + it('a waiver recorded at confirmation keeps the existing values', async () => { + await seed(50) + await pool.query( + `UPDATE ${TABLE} SET email = 'current@example.com' WHERE id <= 10`, + ) + const confirmed = await confirm('plaintext-current') + expect(confirmed.obligations).toEqual([]) + + expect(await run()).toMatchObject({ force: false, forcedBy: null }) + const { rows } = await pool.query<{ n: string }>( + `SELECT count(*)::text AS n FROM ${TABLE} WHERE email = 'current@example.com'`, + ) + expect(rows[0]?.n).toBe('10') + }) + + it('a waiver given later still closes an up-front obligation no pass has started on', async () => { + await seed(20) + await pool.query(`UPDATE ${TABLE} SET email = 'stale@example.com'`) + expect((await confirm()).obligations).toHaveLength(1) + expect((await confirm('plaintext-current')).obligations).toEqual([]) + }) + + it('refuses a waiver once a forced pass has started', async () => { + await seed(20) + await pool.query(`UPDATE ${TABLE} SET email = 'stale@example.com'`) + await confirm() + const controller = new AbortController() + controller.abort() + await run({ signal: controller.signal }) + + const error = await confirm('plaintext-current').catch((e: unknown) => e) + expect(error).toBeInstanceOf(ColumnMigrationError) + expect((error as ColumnMigrationError).code).toBe('waiver-refused') + expect((await migration()).obligations).toMatchObject([ + { reason: 'forced-pass', waivable: false }, + ]) + }) + + it('refuses a reverse column migration while the forward one of the twin is open', async () => { + await seed(10) const db = await pool.connect() try { await appendEvent(db, { tableName: TABLE, - columnName: 'email_encrypted', - event: 'decrypt_dual_writing', - phase: 'decrypt-dual-writing', - details: { - plaintextColumn: 'email', - existingPlaintext: 'prepopulated', - acknowledgedWith: 'force', - }, + columnName: 'email', + event: 'backfilled', + phase: 'backfilled', }) } finally { db.release() } + expect((await migration()).blockedBy).toEqual({ + direction: 'forward', + column: 'email', + phase: 'backfilled', + }) + await expect(confirm()).rejects.toMatchObject({ code: 'counterpart-open' }) + await expect(run()).rejects.toMatchObject({ code: 'counterpart-open' }) + expect(await plaintextCount()).toBe(0) + }) + + it('refuses a run into a different plaintext column than the one recorded', async () => { + await seed(10) + await confirm() + await pool.query(`ALTER TABLE ${TABLE} ADD COLUMN other text`) + await expect(run({ plaintextColumn: 'other' })).rejects.toMatchObject({ + code: 'twin-mismatch', + }) + }) + + it('generates the drop only once the backfill completed', async () => { + await seed(10) + await confirm() + const db = await pool.connect() + try { + const before = await ColumnMigration.load(db, { + table: TABLE, + column: 'email_encrypted', + direction: 'reverse', + manifest: null, + }) + await expect(before.recordDropGenerated()).rejects.toMatchObject({ + code: 'wrong-phase', + }) + } finally { + db.release() + } await run() - const { rows } = await pool.query<{ n: string }>( - `SELECT count(*)::text AS n FROM ${TABLE} WHERE email = 'stale@example.com'`, - ) - expect(rows[0]?.n).toBe('0') + const db2 = await pool.connect() + try { + const done = await ColumnMigration.load(db2, { + table: TABLE, + column: 'email_encrypted', + direction: 'reverse', + manifest: null, + }) + const dropped = await done.recordDropGenerated({ migrationFile: 'x.sql' }) + expect(dropped.phase).toBe('encrypted-dropped') + expect(dropped.open).toBe(false) + expect(dropped.twin).toEqual({ column: 'email', via: 'recorded' }) + } finally { + db2.release() + } }) it('a completed forced pass meets the obligation: later plain runs are unforced again', async () => { @@ -448,6 +591,8 @@ describe.skipIf(!runIntegration)('runDecryptBackfill (integration)', () => { await pool.query( `UPDATE ${TABLE} SET email = 'stale@example.com' WHERE id <= 50`, ) + // Kept, so the first pass is unforced. + await confirm('plaintext-current') const controller = new AbortController() await run({ @@ -499,6 +644,12 @@ describe.skipIf(!runIntegration)('runDecryptBackfill (integration)', () => { expect(JSON.stringify(rows)).not.toContain('secret-not-a-number') const state = await progress(db, TABLE, 'email_encrypted') expect(state?.event).toBe('error') + expect(state?.details).toMatchObject({ + errorName: 'PlaintextWriteError', + sqlState: '22P02', + plaintextSqlType: 'integer', + }) + expect(state?.details).not.toHaveProperty('message') } finally { db.release() } @@ -629,6 +780,8 @@ describe.skipIf(!runIntegration)('runDecryptBackfill (integration)', () => { it('never overwrites plaintext already written by a dual-writing app, unless forced', async () => { await seed(10) + // Dual-writes are live, so the app writes the plaintext column itself. + await confirm() await pool.query( `UPDATE ${TABLE} SET email = 'newer@example.com' WHERE id = 3`, ) @@ -706,68 +859,6 @@ describe.skipIf(!runIntegration)('runDecryptBackfill (integration)', () => { }) }) - // F8: behaviour-preservation pin for forceObligationOpen. The obligation is - // scoped to the latest decrypt_dual_writing; a forced pass left open in an - // earlier lifecycle does not carry into the next one. - it('a forced pass left open in an earlier lifecycle does not force the next one', async () => { - await seed(5) - const db = await pool.connect() - try { - const base = { tableName: TABLE, columnName: 'email_encrypted' } - // Lifecycle 1: acknowledged with force, a forced pass starts and never - // completes. - await appendEvent(db, { - ...base, - event: 'decrypt_dual_writing', - phase: 'decrypt-dual-writing', - details: { plaintextColumn: 'email', acknowledgedWith: 'force' }, - }) - await appendEvent(db, { - ...base, - event: 'decrypt_started', - phase: 'decrypting', - details: { plaintextColumn: 'email', force: true }, - }) - expect(await forceObligationOpen(db, TABLE, 'email_encrypted')).toBe(true) - // Lifecycle 2: a fresh dual-write confirmation over an empty twin. - await appendEvent(db, { - ...base, - event: 'decrypt_dual_writing', - phase: 'decrypt-dual-writing', - details: { plaintextColumn: 'email' }, - }) - expect(await forceObligationOpen(db, TABLE, 'email_encrypted')).toBe( - false, - ) - // An unforced completed event does not close a forced obligation. - await appendEvent(db, { - ...base, - event: 'decrypt_started', - phase: 'decrypting', - details: { plaintextColumn: 'email', force: true }, - }) - await appendEvent(db, { - ...base, - event: 'decrypted', - phase: 'decrypted', - details: { plaintextColumn: 'email' }, - }) - expect(await forceObligationOpen(db, TABLE, 'email_encrypted')).toBe(true) - // A forced completed event does. - await appendEvent(db, { - ...base, - event: 'decrypted', - phase: 'decrypted', - details: { plaintextColumn: 'email', force: true }, - }) - expect(await forceObligationOpen(db, TABLE, 'email_encrypted')).toBe( - false, - ) - } finally { - db.release() - } - }) - it('ignores a forward checkpoint recorded under the same key — no cross-direction resume', async () => { await seed(50) const db = await pool.connect() diff --git a/languages/typescript/packages/migrate/src/backfill.ts b/languages/typescript/packages/migrate/src/backfill.ts index 09c46dd93..02e73d6f6 100644 --- a/languages/typescript/packages/migrate/src/backfill.ts +++ b/languages/typescript/packages/migrate/src/backfill.ts @@ -1,11 +1,12 @@ import { isEncryptedPayload } from '@cipherstash/stack' import type { ClientBase, PoolClient } from 'pg' +import type { ForcedBy } from './column-migration.js' import { countUnencrypted, fetchUnencryptedPage, qualifyTable, } from './cursor.js' -import { type ChunkedRunEvents, runChunkedMigration } from './engine.js' +import { runChunkedMigration } from './engine.js' import { quoteIdent } from './sql.js' // Loose structural types — keep this library decoupled from @cipherstash/stack @@ -212,9 +213,12 @@ export interface BackfillOptions { * Not destructive (overwriting a correctly-encrypted value with a * fresh encryption of the same plaintext is wasted work, not data * loss), but expensive: every row is re-encrypted even if it didn't - * need to be. The CLI also appends `details: { force: true }` to the - * `backfill_started` event so audit queries can spot the recovery - * runs in `cs_migrations` history. + * need to be. Every event of the run records `details.force`, so audit + * queries can spot the recovery runs in `cs_migrations` history. + * + * Once a forced pass has started it is owed: until one completes, every + * run is forced whatever this says, and {@link BackfillResult.forcedBy} + * says why. An interrupted forced pass is therefore always finished as one. */ force?: boolean } @@ -225,8 +229,9 @@ export interface BackfillOptions { export interface BackfillResult { /** * `true` if the run began from a previously-recorded checkpoint rather - * than starting fresh. Determined by the most recent event for this - * column being a `backfill_checkpoint`. + * than starting fresh: the most recent event for this column is a + * `backfill_checkpoint`, or an `error` carrying the last committed cursor, + * of a pass in the same mode. */ resumed: boolean /** @@ -245,6 +250,13 @@ export interface BackfillResult { * paused and should be resumed by re-invoking with the same options. */ completed: boolean + /** Whether this run was a forced pass. */ + force: boolean + /** + * Why: `caller` (`force` was passed), `forced-pass` (an earlier forced pass + * had not completed, so this one finishes it), or `null` when unforced. + */ + forcedBy: ForcedBy } /** @@ -297,20 +309,28 @@ export async function runBackfill( ): Promise { const chunkSize = options.chunkSize ?? 1000 const { db, tableName, pkColumn, plaintextColumn, encryptedColumn } = options - const force = options.force ?? false const coerce = options.transformPlaintext ?? ((v: unknown) => v) const result = await runChunkedMigration({ db, tableName, stateColumn: plaintextColumn, + twinColumn: encryptedColumn, + direction: 'forward', chunkSize, signal: options.signal, - events: FORWARD_EVENTS, - startedDetails: force ? { force: true } : {}, - countPending: () => - countUnencrypted(db, tableName, plaintextColumn, encryptedColumn, force), - fetchPage: (after, limit) => + force: options.force, + baseDetails: { encryptedColumn }, + countPending: (after, { force }) => + countUnencrypted( + db, + tableName, + plaintextColumn, + encryptedColumn, + force, + after === null ? null : { pkColumn, pk: after }, + ), + fetchPage: (after, limit, { force }) => fetchUnencryptedPage(db, { tableName, pkColumn, @@ -320,7 +340,7 @@ export async function runBackfill( limit, force, }), - prepareChunk: async (rows) => { + prepareChunk: async (rows, { force }) => { const models = rows.map((row) => ({ __pk: row.pk, [options.schemaColumnKey]: coerce(row.plaintext), @@ -390,18 +410,11 @@ export async function runBackfill( rowsProcessed: result.rowsProcessed, rowsTotal: result.rowsTotal, completed: result.completed, + force: result.force, + forcedBy: result.forcedBy, } } -/** The forward lifecycle's `cs_migrations` vocabulary. */ -const FORWARD_EVENTS = { - started: 'backfill_started', - checkpoint: 'backfill_checkpoint', - completed: 'backfilled', - runningPhase: 'backfilling', - completedPhase: 'backfilled', -} as const satisfies ChunkedRunEvents - interface WriteChunkOptions { tableName: string pkColumn: string diff --git a/languages/typescript/packages/migrate/src/column-migration.ts b/languages/typescript/packages/migrate/src/column-migration.ts new file mode 100644 index 000000000..97269ef82 --- /dev/null +++ b/languages/typescript/packages/migrate/src/column-migration.ts @@ -0,0 +1,700 @@ +import type { ClientBase } from 'pg' +import { columnHasValues } from './cursor.js' +import { type Manifest, readManifest } from './manifest.js' +import { + appendEvent, + isReverseMigrationPhase, + type MigrationEvent, + type MigrationPhase, + type MigrationStateRow, + rowToState, +} from './state.js' +import { columnExists } from './version.js' + +/** + * Which way a column migration moves data: `forward` (plaintext to + * encrypted) or `reverse` (encrypted to plaintext). See `CONTEXT.md`. + */ +export type MigrationDirection = 'forward' | 'reverse' + +/** + * The `cs_migrations` vocabulary one direction writes. Both directions run + * the same rules over different event names, so their histories stay apart + * and neither resumes from the other's cursor. + */ +export interface DirectionVocabulary { + /** Starts the column migration: dual-writes are confirmed deployed. */ + dualWriting: MigrationEvent + dualWritingPhase: MigrationPhase + started: MigrationEvent + /** The event a resume looks for. Its `cursorValue` is the restart point. */ + checkpoint: MigrationEvent + completed: MigrationEvent + /** Phase recorded on `started`, every `checkpoint`, and any `error`. */ + runningPhase: MigrationPhase + completedPhase: MigrationPhase + /** Recorded when the drop of the source column is generated. */ + dropped: MigrationEvent + droppedPhase: MigrationPhase + /** The `details` key every event uses to name the twin, when it does. */ + twinKey: string +} + +export const DIRECTION_VOCABULARY: Readonly< + Record +> = { + forward: { + dualWriting: 'dual_writing', + dualWritingPhase: 'dual-writing', + started: 'backfill_started', + checkpoint: 'backfill_checkpoint', + completed: 'backfilled', + runningPhase: 'backfilling', + completedPhase: 'backfilled', + dropped: 'dropped', + droppedPhase: 'dropped', + twinKey: 'encryptedColumn', + }, + reverse: { + dualWriting: 'decrypt_dual_writing', + dualWritingPhase: 'decrypt-dual-writing', + started: 'decrypt_started', + checkpoint: 'decrypt_checkpoint', + completed: 'decrypted', + runningPhase: 'decrypting', + completedPhase: 'decrypted', + dropped: 'encrypted_dropped', + droppedPhase: 'encrypted-dropped', + twinKey: 'plaintextColumn', + }, +} + +const OPPOSITE: Record = { + forward: 'reverse', + reverse: 'forward', +} + +/** + * An open obligation to complete a forced pass. While one is open, every run + * is a forced pass, whatever the caller passes. See ADR 0001. + * + * - `existing-values`: a reverse column migration began on a twin that + * already held values. A {@link Waiver} can close it until a forced pass + * starts. + * - `forced-pass`: a forced pass started. Only a completed forced pass closes + * it. + */ +export interface Obligation { + kind: 'forced-pass' + reason: 'existing-values' | 'forced-pass' + /** Id of the event that opened it, or `null` when no event records it yet. */ + openedAt: string | null + /** Whether a waiver may still close it. */ + waivable: boolean +} + +/** + * The operator's recorded assertion that a twin's existing values are already + * current: the application has written that column on every insert and + * update. Reverse only. + */ +export type Waiver = 'plaintext-current' + +/** Why a run is a forced pass, or `null` for an unforced one. */ +export type ForcedBy = 'caller' | Obligation['reason'] | null + +/** The other direction's column migration on the same pair, while open. */ +export interface OpenCounterpart { + direction: MigrationDirection + /** The column its events are keyed by: this column migration's twin. */ + column: string + phase: MigrationPhase +} + +/** What the event history says about one column migration. */ +export interface ColumnMigrationState { + direction: MigrationDirection + /** + * Latest event of this direction under this column, whether or not it + * belongs to the column migration in progress. + */ + latest: MigrationStateRow | null + /** + * Phase of the column migration the column is in now, or `null` when there + * is none (no history, or only a finished column migration's). + */ + phase: MigrationPhase | null + /** `phase` is set and the drop has not been generated. */ + open: boolean + /** The twin the current column migration's events name, if they do. */ + recordedTwin: string | undefined + /** At most one: there is only one kind of obligation. */ + obligations: readonly Obligation[] +} + +/** + * The column migration rules, as a pure function of the event history. + * + * `history` is the column's events in `id` order (any subset that contains + * every non-checkpoint event and the latest checkpoint). Events of the other + * direction are ignored. `recordedTwinHistory` is the events under the twin + * the latest event names: for reverse, a forward column migration of the twin + * recorded after a generated drop means the drop was applied and the column + * was encrypted again, so the reverse history is finished. + * + * Obligations are scoped to the column migration in progress: the events + * since its latest dual-write confirmation (all of them when a pass ran + * without one). + */ +export function deriveColumnMigration(input: { + direction: MigrationDirection + history: readonly MigrationStateRow[] + recordedTwinHistory?: readonly MigrationStateRow[] +}): ColumnMigrationState { + const { direction } = input + const vocab = DIRECTION_VOCABULARY[direction] + const events = ofDirection(direction, input.history) + const latest = events.at(-1) ?? null + + const none: ColumnMigrationState = { + direction, + latest, + phase: null, + open: false, + recordedTwin: undefined, + obligations: [], + } + if (latest === null) return none + + const recordedTwin = twinNamedBy(vocab, latest) + if (latest.phase === vocab.droppedPhase && direction === 'reverse') { + const superseded = ofDirection( + OPPOSITE[direction], + input.recordedTwinHistory ?? [], + ).some((row) => BigInt(row.id) > BigInt(latest.id)) + if (superseded) return none + } + + let scopeStart = 0 + for (const [i, e] of events.entries()) { + if (e.event === vocab.dualWriting) scopeStart = i + } + const scope = events.slice(scopeStart) + + let obligation: Obligation | null = null + for (const e of scope) { + if (e.event === vocab.dualWriting) { + obligation = + e.details?.existingTwinValues === true && + e.details?.waiver === undefined + ? { + kind: 'forced-pass', + reason: 'existing-values', + openedAt: e.id, + waivable: true, + } + : null + } else if (e.event === vocab.completed) { + if (isForced(e)) obligation = null + } else if (e.phase === vocab.runningPhase && isForced(e)) { + if (obligation?.reason !== 'forced-pass') { + obligation = { + kind: 'forced-pass', + reason: 'forced-pass', + openedAt: e.id, + waivable: false, + } + } + } + } + + return { + direction, + latest, + phase: latest.phase, + open: latest.phase !== vocab.droppedPhase, + recordedTwin, + obligations: obligation === null ? [] : [obligation], + } +} + +/** + * The other direction's column migration on this pair, when it is open: at + * most one column migration is open per column pair. `twinHistory` is the + * events keyed by the twin, which is where the other direction records them. + */ +export function openCounterpart(input: { + direction: MigrationDirection + twin: string + twinHistory: readonly MigrationStateRow[] +}): OpenCounterpart | null { + const direction = OPPOSITE[input.direction] + const latest = ofDirection(direction, input.twinHistory).at(-1) + if (latest === undefined) return null + if (latest.phase === DIRECTION_VOCABULARY[direction].droppedPhase) return null + return { direction, column: input.twin, phase: latest.phase } +} + +/** How the next run will go: its mode, why, and where it resumes. */ +export interface PassPlan { + force: boolean + forcedBy: ForcedBy + /** + * The event the run resumes from (a checkpoint, or an error carrying the + * last committed cursor), or `null` for a fresh pass from the first row. + */ + resumeFrom: MigrationStateRow | null +} + +/** + * Plan the next run of a column migration. An open obligation forces it. A + * run resumes only a pass of the same mode: a forced run never resumes an + * unforced pass, whose rows before the cursor were never overwritten. + * + * `twinHasValues` covers a reverse pass started with no column migration on + * record (an embedded worker that never confirmed dual-writes): the twin + * already holding values opens the up-front obligation there and then. + */ +export function planPassFor( + state: ColumnMigrationState, + opts: { force: boolean; twinHasValues?: boolean }, +): PassPlan { + const vocab = DIRECTION_VOCABULARY[state.direction] + let forcedBy: ForcedBy = opts.force ? 'caller' : null + if (forcedBy === null) { + const owed = state.obligations[0] + if (owed !== undefined) forcedBy = owed.reason + else if ( + state.direction === 'reverse' && + state.phase === null && + opts.twinHasValues === true + ) { + forcedBy = 'existing-values' + } + } + const force = forcedBy !== null + + const last = state.phase === null ? null : state.latest + const resumable = + last !== null && + (last.event === vocab.checkpoint || + (last.event === 'error' && + last.phase === vocab.runningPhase && + last.cursorValue !== null)) && + isForced(last) === force + return { force, forcedBy, resumeFrom: resumable ? last : null } +} + +/** + * Thrown when a column migration refuses a step. The message carries table, + * column and phase names only — never a value — so callers may print it. + */ +export class ColumnMigrationError extends Error { + constructor( + readonly code: + | 'counterpart-open' + | 'twin-mismatch' + | 'waiver-refused' + | 'wrong-phase' + | 'obligation-open', + message: string, + ) { + super(message) + this.name = 'ColumnMigrationError' + } +} + +/** How a column migration's twin was decided. */ +export type TwinSource = + | 'flag' + | 'recorded' + | 'manifest' + | 'convention' + | 'fallback' + +export interface LoadColumnMigrationOptions { + /** Physical table; `schema.table` supported. */ + table: string + /** The SOURCE column: the one this direction migrates away from. */ + column: string + direction: MigrationDirection + /** + * The twin the caller names. Refused when it contradicts the twin the + * column migration in progress recorded. + */ + twin?: string + /** + * The forward manifest, for the twin's name when none is given or + * recorded. `undefined` reads `.cipherstash/migrations.json` from the + * current directory; `null` skips it. + */ + manifest?: Manifest | null +} + +/** + * One direction applied to one column, read from `cipherstash.cs_migrations`. + * The single owner of the column-migration rules: which twin it fills + * (recorded, then manifest, then derived), whether it is the one in + * progress, which obligations are open, and which phase changes are allowed. + * Commands and the backfill engine ask it instead of reading event names. + * + * A loaded value is a snapshot. Each method that records an event returns a + * freshly loaded one. + */ +export class ColumnMigration implements ColumnMigrationState { + readonly direction: MigrationDirection + readonly latest: MigrationStateRow | null + readonly phase: MigrationPhase | null + readonly open: boolean + readonly recordedTwin: string | undefined + readonly obligations: readonly Obligation[] + + private constructor( + private readonly db: ClientBase, + readonly table: string, + readonly column: string, + state: ColumnMigrationState, + /** The twin this column migration fills (reverse) or writes (forward). */ + readonly twin: { column: string; via: TwinSource }, + /** The other direction's column migration on this pair, when open. */ + readonly blockedBy: OpenCounterpart | null, + private readonly manifest: Manifest | null | undefined, + ) { + this.direction = state.direction + this.latest = state.latest + this.phase = state.phase + this.open = state.open + this.recordedTwin = state.recordedTwin + this.obligations = state.obligations + } + + static async load( + db: ClientBase, + opts: LoadColumnMigrationOptions, + ): Promise { + const { table, column, direction } = opts + const history = await readHistory(db, table, column) + const named = latestTwin(direction, history) + const namedHistory = + named === undefined ? [] : await readHistory(db, table, named) + const state = deriveColumnMigration({ + direction, + history, + recordedTwinHistory: namedHistory, + }) + + const recorded = state.recordedTwin + let twin: { column: string; via: TwinSource } + if (opts.twin !== undefined) { + if (recorded !== undefined && recorded !== opts.twin) { + throw new ColumnMigrationError( + 'twin-mismatch', + `${table}.${column} is already being migrated with "${recorded}" (recorded in cipherstash.cs_migrations), not "${opts.twin}". Refusing to switch columns in the middle of a column migration: the coverage check before ${column} is dropped would then verify the wrong column. Continue with "${recorded}".`, + ) + } + twin = { column: opts.twin, via: 'flag' } + } else if (recorded !== undefined) { + twin = { column: recorded, via: 'recorded' } + } else { + const manifest = + opts.manifest === undefined ? await readManifest() : opts.manifest + const fromManifest = manifestTwin(direction, manifest, table, column) + twin = + fromManifest !== undefined + ? { column: fromManifest, via: 'manifest' } + : deriveTwinName(direction, column) + } + if (twin.column === column) { + throw new ColumnMigrationError( + 'twin-mismatch', + `The twin of ${table}.${column} cannot be ${column} itself.`, + ) + } + + const twinHistory = + twin.column === named + ? namedHistory + : await readHistory(db, table, twin.column) + const blockedBy = openCounterpart({ + direction, + twin: twin.column, + twinHistory, + }) + return new ColumnMigration( + db, + table, + column, + state, + twin, + blockedBy, + opts.manifest, + ) + } + + /** + * Whether the twin already holds values a reverse column migration did not + * write: there is no column migration in progress, and the twin exists + * with at least one non-NULL value. Opens the up-front obligation. Always + * `false` forward. + */ + async existingTwinValues(): Promise { + if (this.direction !== 'reverse' || this.phase !== null) return false + if (!(await columnExists(this.db, this.table, this.twin.column))) { + return false + } + return columnHasValues(this.db, this.table, this.twin.column) + } + + /** How the next run will go; read-only. See {@link planPassFor}. */ + async planPass(opts: { force: boolean }): Promise { + const twinHasValues = + !opts.force && this.obligations.length === 0 + ? await this.existingTwinValues() + : false + return planPassFor(this, { force: opts.force, twinHasValues }) + } + + /** + * The message for {@link blockedBy}, or `null`. Names and phases only. + */ + counterpartMessage(): string | null { + const other = this.blockedBy + if (other === null) return null + const verb = other.direction === 'forward' ? 'encryption' : 'decryption' + const mine = this.direction === 'forward' ? 'encryption' : 'decryption' + return `${this.table}.${this.twin.column} has a ${other.direction} ${verb} with ${this.column} that is still open at '${other.phase}' in cipherstash.cs_migrations. At most one column migration may be open per pair of columns, so a ${this.direction} ${mine} cannot start until it is dropped or abandoned.` + } + + /** + * Start the column migration: the application writes both columns on + * every insert and update. Records the dual-write confirmation, naming the + * twin. Over a reverse twin that already holds values, this opens the + * up-front obligation unless `waiver` is given. + * + * On a column migration already in progress this records nothing, except + * that a `waiver` closes an up-front obligation no forced pass has started + * on. A waiver given once a forced pass has started is refused. + */ + async confirmDualWrites( + opts: { waiver?: Waiver } = {}, + ): Promise { + const vocab = DIRECTION_VOCABULARY[this.direction] + const counterpart = this.counterpartMessage() + if (counterpart !== null) { + throw new ColumnMigrationError('counterpart-open', counterpart) + } + if (opts.waiver !== undefined && this.direction !== 'reverse') { + throw new ColumnMigrationError( + 'waiver-refused', + `A waiver applies to a reverse column migration only; ${this.table}.${this.column} is ${this.direction}.`, + ) + } + + let existingTwinValues: boolean + if (this.phase !== null) { + const owed = this.obligations[0] + if (opts.waiver === undefined || owed === undefined) return this + if (!owed.waivable) { + throw new ColumnMigrationError( + 'waiver-refused', + `A forced pass over ${this.table}.${this.twin.column} has already started, so it may hold a mix of old and overwritten values. A waiver cannot close that obligation: re-run the backfill to complete the forced pass.`, + ) + } + existingTwinValues = true + } else { + existingTwinValues = await this.existingTwinValues() + } + + await appendEvent(this.db, { + tableName: this.table, + columnName: this.column, + event: vocab.dualWriting, + phase: vocab.dualWritingPhase, + details: { + [vocab.twinKey]: this.twin.column, + ...(existingTwinValues ? { existingTwinValues: true } : {}), + ...(existingTwinValues && opts.waiver !== undefined + ? { waiver: opts.waiver } + : {}), + }, + }) + return this.reload() + } + + /** + * Record that the drop of the source column was generated. Allowed only + * once the backfill completed with no obligation open. + */ + async recordDropGenerated( + details: Record = {}, + ): Promise { + const vocab = DIRECTION_VOCABULARY[this.direction] + if (this.phase !== vocab.completedPhase) { + throw new ColumnMigrationError( + 'wrong-phase', + `${this.table}.${this.column} is in phase '${this.phase ?? '—'}', so its drop cannot be generated. It must be '${vocab.completedPhase}'.`, + ) + } + const owed = this.obligations[0] + if (owed !== undefined) { + throw new ColumnMigrationError( + 'obligation-open', + `${this.table}.${this.column} still owes a forced pass (${owed.reason}), so its drop cannot be generated. Re-run the backfill to complete it.`, + ) + } + await appendEvent(this.db, { + tableName: this.table, + columnName: this.column, + event: vocab.dropped, + phase: vocab.droppedPhase, + details: { [vocab.twinKey]: this.twin.column, ...details }, + }) + return this.reload() + } + + /** + * Record that the source column is gone though no drop was generated: it + * was removed outside stash. Closes the column migration so the other + * direction may start on the pair. Allowed only while it is open. + */ + async recordSourceRemoved( + details: Record = {}, + ): Promise { + const vocab = DIRECTION_VOCABULARY[this.direction] + if (!this.open) { + throw new ColumnMigrationError( + 'wrong-phase', + `${this.table}.${this.column} has no open column migration to close.`, + ) + } + await appendEvent(this.db, { + tableName: this.table, + columnName: this.column, + event: vocab.dropped, + phase: vocab.droppedPhase, + details: { observedAbsent: true, ...details }, + }) + return this.reload() + } + + private reload(): Promise { + return ColumnMigration.load(this.db, { + table: this.table, + column: this.column, + direction: this.direction, + twin: this.twin.via === 'flag' ? this.twin.column : undefined, + manifest: this.manifest, + }) + } +} + +/** + * The twin's name when nothing better is known. Reverse: the + * `_encrypted` convention stripped, else `_plaintext`, so a + * derived name never collides with the column itself. Forward: + * `_encrypted`. + */ +export function deriveTwinName( + direction: MigrationDirection, + column: string, +): { column: string; via: 'convention' | 'fallback' } { + if (direction === 'forward') { + return { column: `${column}_encrypted`, via: 'convention' } + } + const suffix = '_encrypted' + return column.endsWith(suffix) && column.length > suffix.length + ? { column: column.slice(0, -suffix.length), via: 'convention' } + : { column: `${column}_plaintext`, via: 'fallback' } +} + +/** + * The forward manifest's twin for this column. Reverse: the entry whose + * recorded `encryptedColumn` is it (or, for entries written before that field + * existed, whose `_encrypted` convention names it), giving the + * original plaintext name. Forward: the entry's `encryptedColumn`. + */ +export function manifestTwin( + direction: MigrationDirection, + manifest: Manifest | null, + table: string, + column: string, +): string | undefined { + const entries = manifest?.tables[table] ?? [] + if (direction === 'forward') { + return entries.find((e) => e.column === column)?.encryptedColumn + } + const entry = + entries.find((e) => e.encryptedColumn === column) ?? + entries.find( + (e) => + e.encryptedColumn === undefined && `${e.column}_encrypted` === column, + ) + return entry?.column +} + +function ofDirection( + direction: MigrationDirection, + rows: readonly MigrationStateRow[], +): MigrationStateRow[] { + const reverse = direction === 'reverse' + return rows + .filter((row) => isReverseMigrationPhase(row.phase) === reverse) + .sort((a, b) => (BigInt(a.id) < BigInt(b.id) ? -1 : 1)) +} + +function isForced(row: MigrationStateRow): boolean { + return row.details?.force === true +} + +function twinNamedBy( + vocab: DirectionVocabulary, + row: MigrationStateRow, +): string | undefined { + const value = row.details?.[vocab.twinKey] + return typeof value === 'string' && value.length > 0 ? value : undefined +} + +function latestTwin( + direction: MigrationDirection, + history: readonly MigrationStateRow[], +): string | undefined { + const latest = ofDirection(direction, history).at(-1) + return latest && twinNamedBy(DIRECTION_VOCABULARY[direction], latest) +} + +/** + * The events the rules read for one column: every event except checkpoints, + * plus the latest checkpoint of each direction (the only one a resume can + * use). A pass writes one checkpoint per chunk, so this keeps the read small + * on a large table. A missing `cs_migrations` table reads as no history. + */ +async function readHistory( + db: ClientBase, + table: string, + column: string, +): Promise { + const checkpoints = [ + DIRECTION_VOCABULARY.forward.checkpoint, + DIRECTION_VOCABULARY.reverse.checkpoint, + ] + try { + const { rows } = await db.query( + `SELECT id, table_name, column_name, event, phase, cursor_value, rows_processed, rows_total, details, created_at + FROM cipherstash.cs_migrations + WHERE table_name = $1 AND column_name = $2 + AND (event <> ALL($3::text[]) + OR id IN (SELECT max(id) FROM cipherstash.cs_migrations + WHERE table_name = $1 AND column_name = $2 + AND event = ANY($3::text[]) + GROUP BY event)) + ORDER BY id`, + [table, column, checkpoints], + ) + return rows.map(rowToState) + } catch (error) { + // 42P01 undefined_table, 3F000 invalid_schema_name: not installed yet. + const code = (error as { code?: unknown } | null)?.code + if (code === '42P01' || code === '3F000') return [] + throw error + } +} diff --git a/languages/typescript/packages/migrate/src/cursor.ts b/languages/typescript/packages/migrate/src/cursor.ts index 1045f4cb8..c8bbc4697 100644 --- a/languages/typescript/packages/migrate/src/cursor.ts +++ b/languages/typescript/packages/migrate/src/cursor.ts @@ -198,7 +198,8 @@ function pendingPredicate( * * When `force` is true, drops the `encrypted IS NULL` guard — used by * `--force` to count every plaintext row, including ones that already - * have a (potentially stale) ciphertext. + * have a (potentially stale) ciphertext. `after` counts only rows past a + * resume cursor. */ export async function countUnencrypted( client: ClientBase, @@ -206,6 +207,7 @@ export async function countUnencrypted( plaintextColumn: string, encryptedColumn: string, force = false, + after: KeysetBound | null = null, ): Promise { return countPending( client, @@ -213,6 +215,7 @@ export async function countUnencrypted( plaintextColumn, encryptedColumn, force, + after, ) } diff --git a/languages/typescript/packages/migrate/src/decrypt.ts b/languages/typescript/packages/migrate/src/decrypt.ts index 06b85af3e..b2c882971 100644 --- a/languages/typescript/packages/migrate/src/decrypt.ts +++ b/languages/typescript/packages/migrate/src/decrypt.ts @@ -1,15 +1,12 @@ import { isEncryptedPayload } from '@cipherstash/stack' import type { ClientBase, PoolClient } from 'pg' +import type { ForcedBy } from './column-migration.js' import { countUndecrypted, fetchUndecryptedPage, qualifyTable, } from './cursor.js' -import { - type ChunkedRunEvents, - type ChunkedRunProgress, - runChunkedMigration, -} from './engine.js' +import { type ChunkedRunProgress, runChunkedMigration } from './engine.js' import { quoteIdent } from './sql.js' /** @@ -102,9 +99,9 @@ export class PlaintextWriteError extends Error { /** * The largest chunk one write can carry. Each decrypted row binds TWO * parameters in the chunk's `UPDATE … FROM (VALUES ($1, $2), …)` (the primary - * key, and the value packed with the ciphertext it was decrypted from), and Postgres's wire protocol caps a statement at 65535 - * parameters, so 32767 rows is the most that can ever be written in one - * statement. A larger chunk would fail only after its ZeroKMS request. + * key, and the value packed with the ciphertext it was decrypted from), and + * Postgres's wire protocol caps a statement at 65535 parameters, so 32767 + * rows is the most that can ever be written in one statement. A larger chunk would fail only after its ZeroKMS request. */ export const MAX_DECRYPT_CHUNK_SIZE = Math.floor(65535 / 2) @@ -149,6 +146,10 @@ export interface DecryptBackfillOptions { * over whatever the plaintext column holds, so any row the application * wrote to the plaintext column alone (a newer value, or a value cleared on * purpose) gets the older ciphertext's value back. + * + * The column migration can force a run this does not: while a forced pass + * is owed (see {@link ColumnMigration}), every run is one, and + * {@link DecryptBackfillResult.forcedBy} says why. */ force?: boolean /** @@ -198,81 +199,20 @@ export interface DecryptBackfillResult { */ nullPlaintextPks: string[] completed: boolean -} - -/** - * Whether this column migration owes a forced pass, and so every run must be - * one whatever the caller passes. It is owed from the moment a forced pass - * starts, or from the `decrypt_dual_writing` event when a twin that already - * held values was acknowledged with `force` (`details.acknowledgedWith`), and - * stops being owed when a forced pass completes. Reads at most two events: - * the latest `decrypt_dual_writing`, which starts the column migration, and - * the latest forced pass or completion event after it (after the first event - * when there is no `decrypt_dual_writing`). Unforced events neither open nor - * close the obligation, so they are not read. - */ -export async function forceObligationOpen( - db: ClientBase, - tableName: string, - encryptedColumn: string, -): Promise { - // `scope` is the latest `decrypt_dual_writing` (absent for a pass run - // without one, when every event counts). `latest` is the newest forced - // event after it that opens or closes the obligation. - const { rows } = await db.query<{ - acknowledged_with: string | null - latest_event: string | null - }>( - `WITH scope AS ( - SELECT id, details->>'acknowledgedWith' AS acknowledged_with - FROM cipherstash.cs_migrations - WHERE table_name = $1 AND column_name = $2 - AND event = 'decrypt_dual_writing' - ORDER BY id DESC - LIMIT 1 - ), latest AS ( - SELECT event - FROM cipherstash.cs_migrations - WHERE table_name = $1 AND column_name = $2 - AND id > COALESCE((SELECT id FROM scope), 0) - AND details->>'force' = 'true' - AND (event = $3 OR phase = $4) - ORDER BY id DESC - LIMIT 1 - ) - SELECT (SELECT acknowledged_with FROM scope) AS acknowledged_with, - (SELECT event FROM latest) AS latest_event`, - [ - tableName, - encryptedColumn, - REVERSE_EVENTS.completed, - REVERSE_EVENTS.runningPhase, - ], - ) - const row = rows[0] - if (row === undefined) return false - // The latest forced event decides: a completed forced pass closes the - // obligation, a started, checkpointed or failed one opens it. - if (row.latest_event !== null) { - return row.latest_event !== REVERSE_EVENTS.completed - } - // No forced event yet. A twin that already held values and was - // acknowledged with `force` owes a forced pass before any has run. - return row.acknowledged_with === 'force' + /** Whether this run was a forced pass. */ + force: boolean + /** + * Why: `caller` (`force` was passed), `forced-pass` (an earlier forced pass + * had not completed), `existing-values` (the plaintext column held values + * before the column migration began, with no waiver recorded), or `null` + * when unforced. + */ + forcedBy: ForcedBy } /** How many `NULL_PLAINTEXT` primary keys a result carries. */ export const NULL_PLAINTEXT_SAMPLE_SIZE = 20 -/** The reverse lifecycle's `cs_migrations` vocabulary. */ -const REVERSE_EVENTS = { - started: 'decrypt_started', - checkpoint: 'decrypt_checkpoint', - completed: 'decrypted', - runningPhase: 'decrypting', - completedPhase: 'decrypted', -} as const satisfies ChunkedRunEvents - /** * Run a chunked, resumable, idempotent backfill of ciphertext → plaintext — * the reverse of {@link runBackfill}, on the same engine. @@ -305,13 +245,19 @@ const REVERSE_EVENTS = { * `details.plaintextColumn` on every event, so later commands recover the * pairing from state alone. * - * **Resume.** A run resumes from its last checkpoint, and also from an - * `error` event, which records the last committed cursor; a failed run does - * not throw away the progress before it. A forced pass, once started, opens - * an obligation ({@link forceObligationOpen}): until a forced pass completes, - * every run is forced whether or not `force` is passed, so an interrupted - * forced pass is always finished as one. A `force` run over an unforced pass - * starts from the first row. + * **Resume and obligations.** A run resumes from its last checkpoint, and + * also from an `error` event, which records the last committed cursor; a + * failed run does not throw away the progress before it. Whether the run is + * forced is decided by the column migration ({@link ColumnMigration}), not + * only by `force`: a forced pass, once started, is owed until one completes, + * and so is one over a plaintext column that held values before the column + * migration began (unless a waiver was recorded with + * {@link ColumnMigration.confirmDualWrites}). A `force` run over an unforced + * pass starts from the first row. + * + * **Refused** while a forward column migration of `plaintextColumn` into + * this column is still open, and when `plaintextColumn` is not the column the + * column migration in progress recorded. * * **Write errors.** A failed chunk write throws {@link PlaintextWriteError}, * never the database's own message, which can contain the decrypted value. @@ -337,9 +283,6 @@ export async function runDecryptBackfill( plaintextColumn, plaintextSqlType, } = options - const force = - (options.force ?? false) || - (await forceObligationOpen(db, tableName, encryptedColumn)) const serialize = plaintextSerializer(plaintextSqlType) const skippedByCode: Record = {} const nullPlaintextPks: string[] = [] @@ -348,17 +291,14 @@ export async function runDecryptBackfill( db, tableName, stateColumn: encryptedColumn, + twinColumn: plaintextColumn, + direction: 'reverse', chunkSize, signal: options.signal, - events: REVERSE_EVENTS, - trackSkipped: true, - resumeFromError: true, - // `force` on every event, so a resume knows which mode the pass ran in. - baseDetails: force ? { plaintextColumn, force: true } : { plaintextColumn }, - // A forced run never resumes an unforced pass: the rows before its - // cursor were never overwritten. It starts over from the first row. - canResume: (last) => (last.details?.force === true) === force, - countPending: (after) => + force: options.force, + baseDetails: { plaintextColumn }, + errorDetails: safeErrorDetails, + countPending: (after, { force }) => countUndecrypted( db, tableName, @@ -367,7 +307,7 @@ export async function runDecryptBackfill( force, after === null ? null : { pkColumn, pk: after }, ), - fetchPage: (after, limit) => + fetchPage: (after, limit, { force }) => fetchUndecryptedPage(db, { tableName, pkColumn, @@ -377,7 +317,7 @@ export async function runDecryptBackfill( limit, force, }), - prepareChunk: async (rows, { chunkIndex }) => { + prepareChunk: async (rows, { chunkIndex, force }) => { const payloads = rows.map((row) => ({ id: row.pk, data: asPayload(row.ciphertext), @@ -490,7 +430,27 @@ export async function runDecryptBackfill( skippedByCode, nullPlaintextPks, completed: result.completed, + force: result.force, + forcedBy: result.forcedBy, + } +} + +/** + * What the `error` event may store about this direction's own failures: + * names, codes and counts, never a value or a database message. + */ +function safeErrorDetails(error: unknown): Record | undefined { + if (error instanceof PlaintextWriteError) { + return { + sqlState: error.sqlState ?? null, + constraint: error.constraint ?? null, + plaintextSqlType: error.plaintextSqlType, + } + } + if (error instanceof UndecryptableBatchError) { + return { batchRows: error.rows, batchCodes: { ...error.codes } } } + return undefined } /** diff --git a/languages/typescript/packages/migrate/src/engine.ts b/languages/typescript/packages/migrate/src/engine.ts index bf4434536..2ca7d8c9a 100644 --- a/languages/typescript/packages/migrate/src/engine.ts +++ b/languages/typescript/packages/migrate/src/engine.ts @@ -1,28 +1,13 @@ import type { ClientBase, PoolClient } from 'pg' -import type { KeysetPage } from './cursor.js' import { - appendEvent, - type MigrationEvent, - type MigrationPhase, - type MigrationStateRow, - progress, -} from './state.js' - -/** - * The `cs_migrations` vocabulary one direction of the lifecycle writes. The - * forward backfill and the reverse (decrypt) backfill run the same engine and - * differ here, so their histories stay distinguishable and neither resumes - * from the other's cursor. - */ -export interface ChunkedRunEvents { - started: MigrationEvent - /** The event a resume looks for. Its `cursorValue` is the restart point. */ - checkpoint: MigrationEvent - completed: MigrationEvent - /** Phase recorded on `started`, every `checkpoint`, and any `error`. */ - runningPhase: MigrationPhase - completedPhase: MigrationPhase -} + ColumnMigration, + ColumnMigrationError, + DIRECTION_VOCABULARY, + type ForcedBy, + type MigrationDirection, +} from './column-migration.js' +import type { KeysetPage } from './cursor.js' +import { appendEvent } from './state.js' /** * A chunk whose network work (encrypt or decrypt) is done and whose database @@ -33,8 +18,7 @@ export interface PreparedChunk { write(db: ClientBase): Promise /** * Rows in this chunk that were visited but deliberately not written — for - * the reverse direction, values that could not be decrypted. Counted only - * when {@link ChunkedRunOptions.trackSkipped} is set. + * the reverse direction, values that could not be decrypted. Default `0`. */ skipped?: number /** Extra fields merged into this chunk's checkpoint event `details`. */ @@ -47,6 +31,14 @@ export interface ChunkContext { chunkIndex: number /** True when this run resumed from a recorded checkpoint. */ resumed: boolean + /** Whether this run is a forced pass (see {@link ChunkedRunResult.force}). */ + force: boolean +} + +/** The pass mode handed to the page and count callbacks. */ +export interface PassMode { + /** Overwrite values already in the target instead of skipping them. */ + force: boolean } /** Progress snapshot, emitted after every committed chunk. */ @@ -56,18 +48,28 @@ export interface ChunkedRunProgress { lastPk: string | null chunkSize: number chunkIndex: number - /** Cumulative skipped rows; always `0` unless skipping is tracked. */ + /** Cumulative skipped rows, including a resumed pass's. */ rowsSkipped: number } export interface ChunkedRunOptions { db: PoolClient tableName: string - /** The `column_name` this run's events are recorded under. */ + /** + * The SOURCE column, which this run's events are recorded under: the + * plaintext column forward, the encrypted column reverse. + */ stateColumn: string + /** The TARGET column this run fills: the column migration's twin. */ + twinColumn: string + direction: MigrationDirection chunkSize: number signal?: AbortSignal - events: ChunkedRunEvents + /** + * Ask for a forced pass. An open obligation on the column migration forces + * the run whatever this says; {@link ChunkedRunResult.forcedBy} says why. + */ + force?: boolean /** * Rows still to process, for `rowsTotal`. Evaluated once, at start, after * the resume point is known: `after` is the cursor the run restarts from @@ -75,9 +77,13 @@ export interface ChunkedRunOptions { * already visited and are in the resumed `rowsProcessed`, including any the * previous run skipped, which still match the pending predicate. */ - countPending(after: string | null): Promise + countPending(after: string | null, mode: PassMode): Promise /** The next keyset page after `after` (exclusive). */ - fetchPage(after: string | null, limit: number): Promise> + fetchPage( + after: string | null, + limit: number, + mode: PassMode, + ): Promise> /** * Do the chunk's network work BEFORE the transaction opens, so a slow * ZeroKMS round trip never holds row locks. Throwing aborts the run (an @@ -94,25 +100,12 @@ export interface ChunkedRunOptions { */ baseDetails?: Record /** - * Maintain a cumulative `rowsSkipped` (restored from the resumed checkpoint) - * and record it on checkpoint/completed events. Off for the forward - * backfill, whose event details are pinned by existing history readers. + * Fields of a failure that are safe to store in the `error` event: names, + * codes and counts, never a value. The event never stores the error's + * message, which a database or client may fill with row data; it stores + * `err.name` and whatever this returns. */ - trackSkipped?: boolean - /** - * Also resume from this direction's `error` event, which records the last - * COMMITTED cursor and counters. Without it an error hides the previous - * checkpoint and the next run starts a fresh pass from the first primary - * key. Off for the forward backfill, whose resume behaviour is pinned. - */ - resumeFromError?: boolean - /** - * Called with the event a run is about to resume from. Return `false` to - * start a fresh pass instead; throw to refuse the run before anything is - * recorded. Lets a direction refuse to resume a pass that ran in a - * different mode (the reverse backfill's `force`). - */ - canResume?: (last: MigrationStateRow) => boolean + errorDetails?: (error: unknown) => Record | undefined onProgress?: (progress: ChunkedRunProgress) => void } @@ -122,6 +115,15 @@ export interface ChunkedRunResult { rowsTotal: number completed: boolean rowsSkipped: number + /** Whether this run was a forced pass. */ + force: boolean + /** + * Why it was: the caller asked (`caller`), or the column migration owed a + * forced pass (`forced-pass`: an earlier forced pass has not completed; + * `existing-values`: the reverse twin held values before the column + * migration began). `null` for an unforced run. + */ + forcedBy: ForcedBy } /** @@ -134,12 +136,17 @@ export interface ChunkedRunResult { * the `target IS NULL` guard in every page query makes a lost cursor * harmless: a re-run simply re-selects whatever is still pending. * - * Resume: when the column's latest event is this direction's checkpoint, the - * run restarts from its cursor and carries its counters forward. With - * `resumeFromError`, an `error` event recorded in this direction's running - * phase resumes the same way: it carries the last committed cursor. Any other - * latest event (a completed run, the other direction's history) starts a - * fresh pass from the beginning, which the idempotency guard keeps cheap. + * Mode and resume come from the column migration ({@link ColumnMigration}): + * an open obligation forces the run, and the run resumes from this + * direction's latest checkpoint, or from an `error` event carrying the last + * committed cursor, when that pass ran in the same mode. Anything else starts + * a fresh pass from the beginning, which the idempotency guard keeps cheap. + * Every event records the mode (`details.force`), which is what lets a later + * run tell a forced pass from an unforced one. + * + * Refused, before anything is recorded, while the other direction's column + * migration on the same pair is open, or when `twinColumn` contradicts the + * twin the column migration in progress recorded. * * Abort: `signal` is checked between chunks; the in-flight chunk always * finishes and checkpoints first. @@ -147,27 +154,32 @@ export interface ChunkedRunResult { export async function runChunkedMigration( options: ChunkedRunOptions, ): Promise { - const { db, tableName, stateColumn, chunkSize, events } = options - const base = options.baseDetails ?? {} - const trackSkipped = options.trackSkipped ?? false + const { db, tableName, stateColumn, chunkSize, direction } = options + const events = DIRECTION_VOCABULARY[direction] + + const migration = await ColumnMigration.load(db, { + table: tableName, + column: stateColumn, + direction, + twin: options.twinColumn, + manifest: null, + }) + const counterpart = migration.counterpartMessage() + if (counterpart !== null) { + throw new ColumnMigrationError('counterpart-open', counterpart) + } + const plan = await migration.planPass({ force: options.force ?? false }) + const { force, forcedBy } = plan + const mode: PassMode = { force } + const base = { ...options.baseDetails, force } - const last = await progress(db, tableName, stateColumn) - const resumable = - last !== null && - (last.event === events.checkpoint || - (options.resumeFromError === true && - last.event === 'error' && - last.phase === events.runningPhase && - last.cursorValue !== null)) - const resumedFrom = - resumable && (options.canResume?.(last) ?? true) ? last : null + const resumedFrom = plan.resumeFrom const resumeCursor = resumedFrom ? resumedFrom.cursorValue : null const resumed = resumeCursor !== null const priorProcessed = resumedFrom ? (resumedFrom.rowsProcessed ?? 0) : 0 - const priorSkipped = - trackSkipped && resumedFrom ? readSkipped(resumedFrom.details) : 0 + const priorSkipped = resumedFrom ? readSkipped(resumedFrom.details) : 0 - const pending = await options.countPending(resumeCursor) + const pending = await options.countPending(resumeCursor, mode) const rowsTotal = priorProcessed + pending await appendEvent(db, { @@ -180,6 +192,7 @@ export async function runChunkedMigration( rowsTotal, details: { ...base, + ...(forcedBy !== null ? { forcedBy } : {}), chunkSize, resumed, ...options.startedDetails, @@ -191,13 +204,12 @@ export async function runChunkedMigration( let rowsSkipped = priorSkipped let chunkIndex = 0 let completed = false - const skippedDetails = () => (trackSkipped ? { rowsSkipped } : {}) try { while (true) { if (options.signal?.aborted) break - const page = await options.fetchPage(cursor, chunkSize) + const page = await options.fetchPage(cursor, chunkSize, mode) if (page.rows.length === 0) { completed = true break @@ -206,8 +218,9 @@ export async function runChunkedMigration( const prepared = await options.prepareChunk(page.rows, { chunkIndex, resumed, + force, }) - const chunkSkipped = trackSkipped ? (prepared.skipped ?? 0) : 0 + const chunkSkipped = prepared.skipped ?? 0 // The counters and cursor advance only once the chunk has COMMITTED: // an `error` event records them, and a resumed run trusts them. @@ -228,7 +241,8 @@ export async function runChunkedMigration( ...base, chunkIndex, chunkRows: page.rows.length, - ...(trackSkipped ? { chunkSkipped, rowsSkipped: nextSkipped } : {}), + chunkSkipped, + rowsSkipped: nextSkipped, ...prepared.checkpointDetails, }, }) @@ -261,7 +275,7 @@ export async function runChunkedMigration( cursorValue: cursor, rowsProcessed, rowsTotal, - details: { ...base, chunkCount: chunkIndex, ...skippedDetails() }, + details: { ...base, chunkCount: chunkIndex, rowsSkipped }, }) } } catch (err) { @@ -275,15 +289,35 @@ export async function runChunkedMigration( rowsTotal, details: { ...base, - message: err instanceof Error ? err.message : String(err), + ...options.errorDetails?.(err), + errorName: safeErrorName(err), chunkIndex, - ...skippedDetails(), + rowsSkipped, }, }) throw err } - return { resumed, rowsProcessed, rowsTotal, completed, rowsSkipped } + return { + resumed, + rowsProcessed, + rowsTotal, + completed, + rowsSkipped, + force, + forcedBy, + } +} + +/** + * The error's `name` when it looks like a class name, else `Error`. Never its + * message: a database or client error can carry row data there. + */ +function safeErrorName(error: unknown): string { + const name = error instanceof Error ? error.name : undefined + return name !== undefined && /^[A-Za-z_$][\w$.]{0,63}$/.test(name) + ? name + : 'Error' } /** Cumulative skipped count from a checkpoint's details; `0` when absent. */ diff --git a/languages/typescript/packages/migrate/src/index.ts b/languages/typescript/packages/migrate/src/index.ts index 702d80f8b..7af29d545 100644 --- a/languages/typescript/packages/migrate/src/index.ts +++ b/languages/typescript/packages/migrate/src/index.ts @@ -37,6 +37,26 @@ export { type BackfillResult, runBackfill, } from './backfill.js' +export { + ColumnMigration, + ColumnMigrationError, + type ColumnMigrationState, + DIRECTION_VOCABULARY, + type DirectionVocabulary, + deriveColumnMigration, + deriveTwinName, + type ForcedBy, + type LoadColumnMigrationOptions, + type MigrationDirection, + manifestTwin, + type Obligation, + type OpenCounterpart, + openCounterpart, + type PassPlan, + planPassFor, + type TwinSource, + type Waiver, +} from './column-migration.js' export { columnHasValues, countEncrypted, @@ -57,7 +77,6 @@ export { type DecryptBackfillProgress, type DecryptBackfillResult, type DecryptionClientLike, - forceObligationOpen, formatCodes, MAX_DECRYPT_CHUNK_SIZE, NULL_PLAINTEXT_SAMPLE_SIZE, @@ -68,10 +87,10 @@ export { } from './decrypt.js' export { type ChunkContext, - type ChunkedRunEvents, type ChunkedRunOptions, type ChunkedRunProgress, type ChunkedRunResult, + type PassMode, type PreparedChunk, runChunkedMigration, } from './engine.js' diff --git a/languages/typescript/packages/migrate/src/state.ts b/languages/typescript/packages/migrate/src/state.ts index 08d84c981..84989a16a 100644 --- a/languages/typescript/packages/migrate/src/state.ts +++ b/languages/typescript/packages/migrate/src/state.ts @@ -247,7 +247,8 @@ export async function progress( return rowToState(result.rows[0]) } -function rowToState(row: { +/** Decode one `cs_migrations` row. Internal to this package. */ +export function rowToState(row: { id: string | number table_name: string column_name: string From e0ba9d0b4b621becdf4551573f4bfd9b2958aef1 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 19 Oct 2026 01:44:55 +0000 Subject: [PATCH 02/15] refactor(cli): resolve every decrypt command through ColumnMigration decrypt plan, status, backfill and drop now ask @cipherstash/migrate's ColumnMigration for the twin, phase, obligations and open counterpart instead of each deriving them. A live-Postgres scenario runs the four commands end to end and checks they name the same twin; it replaces the mocked status unit test. The backfill help is held to the constants the command enforces, and states that the waiver is refused once a forced pass has started. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01Scub1tpuegHHCTXrgn7SHf --- .../packages/cli/src/cli/registry.ts | 10 +- .../decrypt/__tests__/backfill-help.test.ts | 29 ++ .../__tests__/decrypt-backfill.test.ts | 187 ++++----- .../__tests__/decrypt-drop-plan.test.ts | 168 ++++----- .../__tests__/decrypt-lifecycle.live.test.ts | 357 ++++++++++++++++++ .../decrypt/__tests__/decrypt-status.test.ts | 174 --------- .../decrypt/__tests__/fake-migration.ts | 112 ++++++ .../decrypt/__tests__/resolve-pair.test.ts | 298 +++++++-------- .../cli/src/commands/decrypt/backfill.ts | 130 ++++--- .../packages/cli/src/commands/decrypt/drop.ts | 33 +- .../src/commands/decrypt/lib/resolve-pair.ts | 291 ++++---------- .../packages/cli/src/commands/decrypt/plan.ts | 57 ++- .../cli/src/commands/decrypt/status.ts | 209 +++++----- .../cli/src/commands/encrypt/status.ts | 19 +- .../src/__tests__/column-migration.test.ts | 148 ++++++++ .../packages/migrate/src/column-migration.ts | 41 ++ .../typescript/packages/migrate/src/index.ts | 2 + 17 files changed, 1289 insertions(+), 976 deletions(-) create mode 100644 languages/typescript/packages/cli/src/commands/decrypt/__tests__/backfill-help.test.ts create mode 100644 languages/typescript/packages/cli/src/commands/decrypt/__tests__/decrypt-lifecycle.live.test.ts delete mode 100644 languages/typescript/packages/cli/src/commands/decrypt/__tests__/decrypt-status.test.ts create mode 100644 languages/typescript/packages/cli/src/commands/decrypt/__tests__/fake-migration.ts diff --git a/languages/typescript/packages/cli/src/cli/registry.ts b/languages/typescript/packages/cli/src/cli/registry.ts index 1e44415e3..2ee742c1b 100644 --- a/languages/typescript/packages/cli/src/cli/registry.ts +++ b/languages/typescript/packages/cli/src/cli/registry.ts @@ -699,9 +699,11 @@ export const registry: CommandGroup[] = [ 'and never dropped: finish or abandon that migration first. The first', 'run also refuses a plaintext column that already holds values: the', 'backfill only fills NULLs, so stale values would survive. Pass --force', - 'to overwrite them from the ciphertext (every later run is then forced', - 'until a forced pass completes), or --confirm-plaintext-current to keep', - 'them. Keep writing both columns until the drop is applied.', + 'to overwrite them from the ciphertext, or --confirm-plaintext-current', + 'to keep them. A forced pass, once started, is owed: every later run is', + 'forced until one completes, and the waiver is refused from then on.', + '`decrypt status` flags a column that owes one (forced-pass-owed).', + 'Keep writing both columns until the drop is applied.', ].join('\n'), examples: [ 'decrypt backfill --table users --column email_encrypted', @@ -739,7 +741,7 @@ export const registry: CommandGroup[] = [ { name: '--confirm-plaintext-current', description: - 'First run only: keep the values a plaintext column already holds, asserting they are current.', + 'Keep the values a plaintext column already holds, asserting they are current. Refused once a forced pass has started.', }, { name: '--continue-on-undecryptable', diff --git a/languages/typescript/packages/cli/src/commands/decrypt/__tests__/backfill-help.test.ts b/languages/typescript/packages/cli/src/commands/decrypt/__tests__/backfill-help.test.ts new file mode 100644 index 000000000..ef972fe4b --- /dev/null +++ b/languages/typescript/packages/cli/src/commands/decrypt/__tests__/backfill-help.test.ts @@ -0,0 +1,29 @@ +/** + * The command registry is plain data with no imports, so `decrypt backfill`'s + * help writes its flag names and limits as literals. These tests hold them to + * the constants the command enforces, so the help cannot drift from them. + */ + +import { MAX_DECRYPT_CHUNK_SIZE } from '@cipherstash/migrate' +import { describe, expect, it } from 'vitest' +import { renderCommandHelp } from '@/cli/help.js' +import { DEFAULT_DECRYPT_CHUNK_SIZE } from '../backfill.js' +import { TWIN_HAZARD_FLAGS } from '../lib/resolve-pair.js' + +const help = renderCommandHelp('decrypt backfill', 'npx stash') ?? '' + +describe('decrypt backfill help', () => { + it('states the chunk-size bound and default the command enforces', () => { + expect(help).toContain(`from 1 to ${MAX_DECRYPT_CHUNK_SIZE}`) + expect(help).toMatch( + new RegExp(`--chunk-size [\\s\\S]*${DEFAULT_DECRYPT_CHUNK_SIZE}`), + ) + }) + + it.each(Object.values(TWIN_HAZARD_FLAGS))( + 'offers %s, a way past a twin that holds values, as a flag', + (flag) => { + expect(help).toContain(` ${flag}`) + }, + ) +}) diff --git a/languages/typescript/packages/cli/src/commands/decrypt/__tests__/decrypt-backfill.test.ts b/languages/typescript/packages/cli/src/commands/decrypt/__tests__/decrypt-backfill.test.ts index aad7cde96..de52a58c6 100644 --- a/languages/typescript/packages/cli/src/commands/decrypt/__tests__/decrypt-backfill.test.ts +++ b/languages/typescript/packages/cli/src/commands/decrypt/__tests__/decrypt-backfill.test.ts @@ -12,7 +12,13 @@ import { UndecryptableBatchError, } from '@cipherstash/migrate' import { beforeEach, describe, expect, it, vi } from 'vitest' -import type { DecryptPair, TwinHazard } from '../lib/resolve-pair.js' +import type { DecryptPair } from '../lib/resolve-pair.js' +import { + type FakeMigration, + fakeMigration, + inFlight, + reverseEvent, +} from './fake-migration.js' const releaseMock = vi.hoisted(() => vi.fn()) vi.mock('pg', () => ({ @@ -26,7 +32,6 @@ vi.mock('pg', () => ({ const migrateMocks = vi.hoisted(() => ({ installMigrationsSchema: vi.fn(async () => {}), - appendEvent: vi.fn(async () => {}), runDecryptBackfill: vi.fn( async (): Promise => ({ resumed: false, @@ -37,6 +42,8 @@ const migrateMocks = vi.hoisted(() => ({ skippedByCode: {}, nullPlaintextPks: [], completed: true, + force: false, + forcedBy: null, }), ), })) @@ -46,13 +53,9 @@ vi.mock('@cipherstash/migrate', async (importOriginal) => ({ })) const resolveMock = vi.hoisted(() => vi.fn()) -const hazardMock = vi.hoisted(() => - vi.fn(async (): Promise => null), -) vi.mock('../lib/resolve-pair.js', async (importOriginal) => ({ ...(await importOriginal()), resolveDecryptPair: resolveMock, - assessExistingTwin: hazardMock, })) const decryptionClient = vi.hoisted(() => ({ bulkDecrypt: vi.fn() })) @@ -91,7 +94,11 @@ vi.mock('@/commands/init/utils.js', () => ({ import * as p from '@clack/prompts' import { decryptBackfillCommand, skippedReport } from '../backfill.js' -function pair(overrides: Partial = {}): DecryptPair { +function pair( + overrides: Partial> & { + migration?: FakeMigration + } = {}, +): DecryptPair & { migration: FakeMigration } { return { table: 'users', encryptedColumn: 'email_encrypted', @@ -100,27 +107,14 @@ function pair(overrides: Partial = {}): DecryptPair { plaintextColumn: 'email', via: 'convention', plaintextSqlType: 'text', - state: null, - // A recorded reverse event means a lifecycle in progress, unless a test - // says otherwise. - lifecycleActive: overrides.state !== undefined && overrides.state !== null, - forwardState: null, + migration: fakeMigration(), ...overrides, } } -const reverseInFlight = { - id: '1', - tableName: 'users', - columnName: 'email_encrypted', - event: 'decrypt_checkpoint' as const, - phase: 'decrypting' as const, - cursorValue: '5', - rowsProcessed: 5, - rowsTotal: 10, - details: { plaintextColumn: 'email' }, - createdAt: new Date(0), -} +/** A pair whose reverse column migration is mid-pass. */ +const midPass = (overrides: Parameters[0] = {}) => + pair({ migration: fakeMigration({ history: inFlight() }), ...overrides }) function spyExit() { return vi @@ -131,17 +125,13 @@ function spyExit() { describe('decrypt backfill', () => { beforeEach(() => { vi.clearAllMocks() - resolveMock.mockResolvedValue(pair({ state: reverseInFlight })) - hazardMock.mockResolvedValue(null) + resolveMock.mockResolvedValue(midPass()) vi.mocked(p.confirm).mockResolvedValue(true) }) it('decrypts into the twin with its declared type, 500 rows per ZeroKMS request by default', async () => { resolveMock.mockResolvedValue( - pair({ - state: reverseInFlight, - plaintextSqlType: 'character varying(64)', - }), + midPass({ plaintextSqlType: 'character varying(64)' }), ) await decryptBackfillCommand({ table: 'users', column: 'email_encrypted' }) @@ -176,19 +166,21 @@ describe('decrypt backfill', () => { }) it('non-interactive first run: refuses without --confirm-dual-writes-deployed', async () => { - resolveMock.mockResolvedValue(pair({ state: null })) + const fresh = pair() + resolveMock.mockResolvedValue(fresh) await decryptBackfillCommand({ table: 'users', column: 'email_encrypted' }) expect(p.log.error).toHaveBeenCalledWith( expect.stringContaining('--confirm-dual-writes-deployed'), ) - expect(migrateMocks.appendEvent).not.toHaveBeenCalled() + expect(fresh.migration.confirmDualWrites).not.toHaveBeenCalled() expect(migrateMocks.runDecryptBackfill).not.toHaveBeenCalled() }) - it('records the reverse dual-write gate, with the pairing, on first acceptance', async () => { - resolveMock.mockResolvedValue(pair({ state: null })) + it('records the reverse dual-write gate on first acceptance', async () => { + const fresh = pair() + resolveMock.mockResolvedValue(fresh) await decryptBackfillCommand({ table: 'users', @@ -196,13 +188,12 @@ describe('decrypt backfill', () => { confirmDualWritesDeployed: true, }) - expect(migrateMocks.appendEvent).toHaveBeenCalledWith(expect.anything(), { - tableName: 'users', - columnName: 'email_encrypted', - event: 'decrypt_dual_writing', - phase: 'decrypt-dual-writing', - details: { plaintextColumn: 'email' }, + expect(fresh.migration.confirmDualWrites).toHaveBeenCalledWith({ + waiver: undefined, }) + expect(p.log.success).toHaveBeenCalledWith( + "users.email_encrypted marked as 'decrypt-dual-writing' in cs_migrations.", + ) expect(migrateMocks.runDecryptBackfill).toHaveBeenCalled() }) @@ -216,6 +207,8 @@ describe('decrypt backfill', () => { skippedByCode: { UNKNOWN: 3 }, nullPlaintextPks: [], completed: true, + force: false, + forcedBy: null, }) await decryptBackfillCommand({ table: 'users', column: 'email_encrypted' }) @@ -268,18 +261,16 @@ describe('decrypt backfill', () => { }) it('refuses a twin whose forward migration was never dropped, even with --force, recording nothing', async () => { - resolveMock.mockResolvedValue( - pair({ - state: null, - forwardState: { - ...reverseInFlight, - columnName: 'email', - event: 'backfilled', + const blocked = pair({ + migration: fakeMigration({ + blockedBy: { + direction: 'forward', + column: 'email', phase: 'backfilled', - details: null, }, }), - ) + }) + resolveMock.mockResolvedValue(blocked) const exitSpy = spyExit() await decryptBackfillCommand({ @@ -292,21 +283,21 @@ describe('decrypt backfill', () => { expect(p.log.error).toHaveBeenCalledWith( expect.stringContaining("still open at 'backfilled'"), ) - expect(migrateMocks.appendEvent).not.toHaveBeenCalled() + expect(blocked.migration.confirmDualWrites).not.toHaveBeenCalled() expect(migrateMocks.runDecryptBackfill).not.toHaveBeenCalled() expect(exitSpy).toHaveBeenCalledWith(1) exitSpy.mockRestore() }) describe('a plaintext twin that already holds values (first run)', () => { - const prepopulated: TwinHazard = { kind: 'prepopulated' } + let held: ReturnType beforeEach(() => { - resolveMock.mockResolvedValue(pair({ state: null })) + held = pair({ migration: fakeMigration({ twinHasValues: true }) }) + resolveMock.mockResolvedValue(held) }) it('refuses it, recording nothing and decrypting nothing', async () => { - hazardMock.mockResolvedValue(prepopulated) const exitSpy = spyExit() await decryptBackfillCommand({ @@ -319,15 +310,13 @@ describe('decrypt backfill', () => { expect(message).toContain('users.email already holds values') expect(message).toContain('--force') expect(message).toContain('--confirm-plaintext-current') - expect(migrateMocks.appendEvent).not.toHaveBeenCalled() + expect(held.migration.confirmDualWrites).not.toHaveBeenCalled() expect(migrateMocks.runDecryptBackfill).not.toHaveBeenCalled() expect(exitSpy).toHaveBeenCalledWith(1) exitSpy.mockRestore() }) - it('--force overwrites it from the ciphertext, and the gate records the choice', async () => { - hazardMock.mockResolvedValue(prepopulated) - + it('--force overwrites it from the ciphertext, recording no waiver', async () => { await decryptBackfillCommand({ table: 'users', column: 'email_encrypted', @@ -335,25 +324,15 @@ describe('decrypt backfill', () => { force: true, }) - expect(migrateMocks.appendEvent).toHaveBeenCalledWith( - expect.anything(), - expect.objectContaining({ - event: 'decrypt_dual_writing', - details: { - plaintextColumn: 'email', - existingPlaintext: 'prepopulated', - acknowledgedWith: 'force', - }, - }), - ) + expect(held.migration.confirmDualWrites).toHaveBeenCalledWith({ + waiver: undefined, + }) expect(migrateMocks.runDecryptBackfill).toHaveBeenCalledWith( expect.objectContaining({ force: true }), ) }) - it('--confirm-plaintext-current keeps it and fills only the NULLs', async () => { - hazardMock.mockResolvedValue(prepopulated) - + it('--confirm-plaintext-current keeps it, recorded as the waiver', async () => { await decryptBackfillCommand({ table: 'users', column: 'email_encrypted', @@ -364,22 +343,15 @@ describe('decrypt backfill', () => { expect(p.log.warn).toHaveBeenCalledWith( expect.stringContaining('Proceeding under --confirm-plaintext-current'), ) - expect(migrateMocks.appendEvent).toHaveBeenCalledWith( - expect.anything(), - expect.objectContaining({ - details: expect.objectContaining({ - existingPlaintext: 'prepopulated', - acknowledgedWith: 'confirm-plaintext-current', - }), - }), - ) + expect(held.migration.confirmDualWrites).toHaveBeenCalledWith({ + waiver: 'plaintext-current', + }) expect(migrateMocks.runDecryptBackfill).toHaveBeenCalledWith( expect.not.objectContaining({ force: true }), ) }) it('checks the twin before the --force prompt, so cancelling it records nothing', async () => { - hazardMock.mockResolvedValue(prepopulated) vi.mocked(p.confirm).mockResolvedValueOnce(false) await decryptBackfillCommand({ @@ -390,9 +362,9 @@ describe('decrypt backfill', () => { }) // Had the gate's event been recorded first, the next run would treat - // the twin's values as this lifecycle's own and skip the check. - expect(hazardMock).toHaveBeenCalled() - expect(migrateMocks.appendEvent).not.toHaveBeenCalled() + // the twin's values as this column migration's own. + expect(held.migration.existingTwinValues).toHaveBeenCalled() + expect(held.migration.confirmDualWrites).not.toHaveBeenCalled() expect(migrateMocks.runDecryptBackfill).not.toHaveBeenCalled() }) @@ -414,20 +386,51 @@ describe('decrypt backfill', () => { }) }) - it('a finished earlier lifecycle does not pass the dual-write gate', async () => { + it('says why a run it did not ask to force is a forced pass', async () => { resolveMock.mockResolvedValue( pair({ - state: { - ...reverseInFlight, - event: 'encrypted_dropped', - phase: 'encrypted-dropped', - }, - lifecycleActive: false, + migration: fakeMigration({ + history: [ + reverseEvent('decrypt_dual_writing', 'decrypt-dual-writing'), + reverseEvent('decrypt_started', 'decrypting', { + details: { plaintextColumn: 'email', force: true }, + }), + ], + }), }), ) await decryptBackfillCommand({ table: 'users', column: 'email_encrypted' }) + expect(p.log.warn).toHaveBeenCalledWith( + expect.stringContaining( + 'This run is a forced pass: a forced pass over users.email started earlier', + ), + ) + }) + + it('a waiver on a column migration in progress goes to the column migration, which may refuse it', async () => { + const running = midPass() + resolveMock.mockResolvedValue(running) + + await decryptBackfillCommand({ + table: 'users', + column: 'email_encrypted', + confirmPlaintextCurrent: true, + }) + + expect(running.migration.confirmDualWrites).toHaveBeenCalledWith({ + waiver: 'plaintext-current', + }) + }) + + it('a finished earlier column migration does not pass the dual-write gate', async () => { + // A drop generated, applied, and the column encrypted again: the forward + // events of the twin after the drop make the reverse history finished. + resolveMock.mockResolvedValue(pair()) + + await decryptBackfillCommand({ table: 'users', column: 'email_encrypted' }) + expect(p.log.error).toHaveBeenCalledWith( expect.stringContaining('--confirm-dual-writes-deployed'), ) @@ -486,6 +489,8 @@ describe('decrypt backfill', () => { skippedByCode: { UNKNOWN: 1, NULL_PLAINTEXT: 2 }, nullPlaintextPks: ['4', '9'], completed: true, + force: false, + forcedBy: null, }) await decryptBackfillCommand({ table: 'users', column: 'email_encrypted' }) diff --git a/languages/typescript/packages/cli/src/commands/decrypt/__tests__/decrypt-drop-plan.test.ts b/languages/typescript/packages/cli/src/commands/decrypt/__tests__/decrypt-drop-plan.test.ts index 9ba1accf5..fcf015484 100644 --- a/languages/typescript/packages/cli/src/commands/decrypt/__tests__/decrypt-drop-plan.test.ts +++ b/languages/typescript/packages/cli/src/commands/decrypt/__tests__/decrypt-drop-plan.test.ts @@ -9,9 +9,16 @@ * `encrypt-v3.test.ts` pins `encrypt drop`. */ -import type { ColumnIndexInfo, MigrationStateRow } from '@cipherstash/migrate' +import type { ColumnIndexInfo, OpenCounterpart } from '@cipherstash/migrate' import { beforeEach, describe, expect, it, vi } from 'vitest' import type { DecryptPair } from '../lib/resolve-pair.js' +import { + decrypted, + type FakeMigration, + fakeMigration, + inFlight, + reverseEvent, +} from './fake-migration.js' const queryMock = vi.hoisted(() => vi.fn(async (_sql: string): Promise<{ rows: unknown[] }> => ({ rows: [] })), @@ -30,7 +37,6 @@ const migrateMocks = vi.hoisted(() => ({ countUndecrypted: vi.fn(async () => 0), listColumnIndexes: vi.fn(async (): Promise => []), countEqlColumnsInDatabase: vi.fn(async () => 1), - appendEvent: vi.fn(async () => {}), removeManifestColumn: vi.fn(async () => true), columnHasValues: vi.fn(async () => false), readManifest: vi.fn(async (): Promise => null), @@ -38,9 +44,12 @@ const migrateMocks = vi.hoisted(() => ({ { column: 'email_encrypted', domain: 'eql_v3_text_search', version: 3 }, ]), })) +// The forward column migration `decrypt plan` closes for a removed source. +const forwardLoad = vi.hoisted(() => vi.fn()) vi.mock('@cipherstash/migrate', async (importOriginal) => ({ ...(await importOriginal()), ...migrateMocks, + ColumnMigration: { load: forwardLoad }, })) const resolveMock = vi.hoisted(() => vi.fn()) @@ -101,7 +110,11 @@ import { BackfillConfigError } from '@/commands/encrypt/backfill.js' import { decryptDropCommand } from '../drop.js' import { buildAddPlaintextSql, decryptPlanCommand } from '../plan.js' -function pair(overrides: Partial = {}): DecryptPair { +function pair( + overrides: Partial> & { + migration?: FakeMigration + } = {}, +): DecryptPair & { migration: FakeMigration } { return { table: 'users', encryptedColumn: 'email_encrypted', @@ -110,44 +123,19 @@ function pair(overrides: Partial = {}): DecryptPair { plaintextColumn: 'email', via: 'recorded', plaintextSqlType: 'text', - state: decrypted('email'), - // A recorded reverse event means a lifecycle in progress, unless a test - // says otherwise. - lifecycleActive: 'state' in overrides ? overrides.state !== null : true, - forwardState: null, + migration: fakeMigration({ history: decrypted() }), ...overrides, } } -/** A forward event recorded under the plaintext twin's name. */ -function forward(phase: MigrationStateRow['phase']): MigrationStateRow { - return { - id: '3', - tableName: 'users', - columnName: 'email', - event: phase === 'backfilled' ? 'backfilled' : 'dual_writing', - phase, - cursorValue: null, - rowsProcessed: null, - rowsTotal: null, - details: null, - createdAt: new Date(0), - } -} +/** A pair with no reverse column migration on record. */ +const fresh = (overrides: Parameters[0] = {}) => + pair({ migration: fakeMigration(), ...overrides }) -function decrypted(plaintextColumn?: string): MigrationStateRow { - return { - id: '9', - tableName: 'users', - columnName: 'email_encrypted', - event: 'decrypted', - phase: 'decrypted', - cursorValue: '100', - rowsProcessed: 100, - rowsTotal: 100, - details: plaintextColumn ? { plaintextColumn, rowsSkipped: 0 } : {}, - createdAt: new Date(0), - } +const forwardOpen: OpenCounterpart = { + direction: 'forward', + column: 'email', + phase: 'backfilled', } function spyExit() { @@ -169,6 +157,8 @@ describe('decrypt drop', () => { }) it('verifies coverage against the RECORDED twin and drops the encrypted column', async () => { + const dropping = pair() + resolveMock.mockResolvedValue(dropping) await decryptDropCommand({ table: 'users', column: 'email_encrypted' }) expect(migrateMocks.countUndecrypted).toHaveBeenCalledWith( @@ -187,15 +177,8 @@ describe('decrypt drop', () => { ) // The plaintext twin is never touched. expect(sql).not.toMatch(/DROP COLUMN "email"'/) - expect(migrateMocks.appendEvent).toHaveBeenCalledWith( - expect.anything(), - expect.objectContaining({ - tableName: 'users', - columnName: 'email_encrypted', - event: 'encrypted_dropped', - phase: 'encrypted-dropped', - details: expect.objectContaining({ plaintextColumn: 'email' }), - }), + expect(dropping.migration.recordDropGenerated).toHaveBeenCalledWith( + expect.objectContaining({ migrationFile: expect.any(String) }), ) }) @@ -242,14 +225,13 @@ describe('decrypt drop', () => { ), ) expect(writeFileMock).not.toHaveBeenCalled() - expect(migrateMocks.appendEvent).not.toHaveBeenCalled() expect(exitSpy).toHaveBeenCalledWith(1) exitSpy.mockRestore() }) it("requires the reverse phase 'decrypted'", async () => { resolveMock.mockResolvedValue( - pair({ state: { ...decrypted('email'), phase: 'decrypting' } }), + pair({ migration: fakeMigration({ history: inFlight() }) }), ) const exitSpy = spyExit() @@ -265,7 +247,13 @@ describe('decrypt drop', () => { }) it('refuses without a RECORDED pairing, even when a twin name could be guessed', async () => { - resolveMock.mockResolvedValue(pair({ state: decrypted(undefined) })) + resolveMock.mockResolvedValue( + pair({ + migration: fakeMigration({ + history: [reverseEvent('decrypted', 'decrypted', { details: {} })], + }), + }), + ) const exitSpy = spyExit() await decryptDropCommand({ table: 'users', column: 'email_encrypted' }) @@ -371,7 +359,13 @@ describe('decrypt drop', () => { table: 'tenant.weird"table', encryptedColumn: "o'brien_encrypted", plaintextColumn: 'o"brien', - state: decrypted('o"brien'), + migration: fakeMigration({ + history: [ + reverseEvent('decrypted', 'decrypted', { + details: { plaintextColumn: 'o"brien' }, + }), + ], + }), }), ) @@ -399,7 +393,7 @@ describe('decrypt plan', () => { detectMocks.detectPrismaNext.mockReturnValue(false) migrateMocks.columnHasValues.mockResolvedValue(false) resolveMock.mockResolvedValue( - pair({ plaintextSqlType: null, state: null, via: 'convention' }), + fresh({ plaintextSqlType: null, via: 'convention' }), ) }) @@ -458,7 +452,7 @@ describe('decrypt plan', () => { }) it('adds nothing for an empty twin that already exists', async () => { - resolveMock.mockResolvedValue(pair({ state: null })) + resolveMock.mockResolvedValue(fresh()) await decryptPlanCommand({ table: 'users' }) @@ -470,8 +464,9 @@ describe('decrypt plan', () => { }) it('warns, with the backfill refusal text, when an existing twin already holds values', async () => { - resolveMock.mockResolvedValue(pair({ state: null })) - migrateMocks.columnHasValues.mockResolvedValueOnce(true) + resolveMock.mockResolvedValue( + fresh({ migration: fakeMigration({ twinHasValues: true }) }), + ) await decryptPlanCommand({ table: 'users' }) @@ -490,9 +485,13 @@ describe('decrypt plan', () => { it('warns that the backfill will refuse a twin whose forward migration was never dropped', async () => { resolveMock.mockResolvedValue( - pair({ state: null, forwardState: forward('backfilled') }), + fresh({ + migration: fakeMigration({ + blockedBy: forwardOpen, + twinHasValues: true, + }), + }), ) - migrateMocks.columnHasValues.mockResolvedValueOnce(true) await decryptPlanCommand({ table: 'users' }) @@ -504,22 +503,28 @@ describe('decrypt plan', () => { it('closes an open forward migration whose source column was removed outside stash', async () => { resolveMock.mockResolvedValue( - pair({ + fresh({ plaintextSqlType: null, - state: null, via: 'convention', - forwardState: forward('backfilled'), + migration: fakeMigration({ blockedBy: forwardOpen }), }), ) + const recordSourceRemoved = vi.fn(async () => ({})) + forwardLoad.mockResolvedValue({ recordSourceRemoved }) await decryptPlanCommand({ table: 'users' }) - expect(migrateMocks.appendEvent).toHaveBeenCalledWith(expect.anything(), { - tableName: 'users', - columnName: 'email', - event: 'dropped', - phase: 'dropped', - details: { observedAbsent: true, recordedBy: 'stash decrypt plan' }, + expect(forwardLoad).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ + table: 'users', + column: 'email', + direction: 'forward', + twin: 'email_encrypted', + }), + ) + expect(recordSourceRemoved).toHaveBeenCalledWith({ + recordedBy: 'stash decrypt plan', }) expect(p.log.info).toHaveBeenCalledWith( expect.stringContaining("still open at 'backfilled'"), @@ -531,49 +536,40 @@ describe('decrypt plan', () => { it('--dry-run records nothing for a removed forward source', async () => { resolveMock.mockResolvedValue( - pair({ + fresh({ plaintextSqlType: null, - state: null, via: 'convention', - forwardState: forward('backfilled'), + migration: fakeMigration({ blockedBy: forwardOpen }), }), ) await decryptPlanCommand({ table: 'users', dryRun: true }) - expect(migrateMocks.appendEvent).not.toHaveBeenCalled() + expect(forwardLoad).not.toHaveBeenCalled() expect(p.log.info).toHaveBeenCalledWith( expect.stringContaining('would record'), ) }) it('records nothing for a missing twin with no open forward migration', async () => { - for (const forwardState of [null, forward('dropped')]) { - vi.clearAllMocks() - resolveMock.mockResolvedValue( - pair({ - plaintextSqlType: null, - state: null, - via: 'convention', - forwardState, - }), - ) + resolveMock.mockResolvedValue( + fresh({ plaintextSqlType: null, via: 'convention' }), + ) - await decryptPlanCommand({ table: 'users' }) + await decryptPlanCommand({ table: 'users' }) - expect(migrateMocks.appendEvent).not.toHaveBeenCalled() - } + expect(forwardLoad).not.toHaveBeenCalled() }) - it('does not second-guess a twin the reverse lifecycle in progress filled', async () => { + it('does not second-guess a twin the reverse column migration in progress filled', async () => { resolveMock.mockResolvedValue( - pair({ state: { ...decrypted('email'), phase: 'decrypting' } }), + pair({ + migration: fakeMigration({ history: inFlight(), twinHasValues: true }), + }), ) - migrateMocks.columnHasValues.mockResolvedValue(true) await decryptPlanCommand({ table: 'users' }) - expect(migrateMocks.columnHasValues).not.toHaveBeenCalled() expect(p.log.warn).not.toHaveBeenCalled() }) @@ -596,7 +592,7 @@ describe('decrypt plan', () => { }) resolveMock .mockResolvedValueOnce( - pair({ plaintextSqlType: null, state: null, via: 'convention' }), + fresh({ plaintextSqlType: null, via: 'convention' }), ) .mockRejectedValueOnce( new BackfillConfigError('users.note is itself an EQL v3 column'), diff --git a/languages/typescript/packages/cli/src/commands/decrypt/__tests__/decrypt-lifecycle.live.test.ts b/languages/typescript/packages/cli/src/commands/decrypt/__tests__/decrypt-lifecycle.live.test.ts new file mode 100644 index 000000000..7d1672d47 --- /dev/null +++ b/languages/typescript/packages/cli/src/commands/decrypt/__tests__/decrypt-lifecycle.live.test.ts @@ -0,0 +1,357 @@ +/** + * The reverse lifecycle through the real commands against live Postgres: + * `decrypt plan`, `status`, `backfill` and `drop` each resolve the plaintext + * twin through `ColumnMigration`, and must all name the same one. They once + * each derived it, and disagreed on edge cases the unit tests' mocks could + * not show. + * + * Only the edges are stubbed: the prompts (answered yes), the stash config + * (this database), and the encryption client (a `bulkDecrypt` that decodes + * deterministic markers, so no CipherStash credentials are needed). The + * catalog reads, `cipherstash.cs_migrations`, the manifest file and the + * generated SQL are all real. + * + * Gated on STASH_TEST_DATABASE_URL so the default `pnpm test` stays green + * without a database. Locally: + * + * docker compose -f local/docker-compose.postgres.yml up -d --wait + * export STASH_TEST_DATABASE_URL=postgres://cipherstash:password@localhost:55432/cipherstash + * + * Needs no EQL install: the suite's own schema declares `jsonb` domains with + * the EQL v3 names, which is all the commands look at. It touches only its own + * schema and the `cs_migrations` rows for its own table. + */ + +import fs from 'node:fs' +import os from 'node:os' +import path from 'node:path' +import { + afterAll, + afterEach, + beforeAll, + beforeEach, + describe, + expect, + it, + vi, +} from 'vitest' + +const DATABASE_URL = process.env.STASH_TEST_DATABASE_URL +const describeLive = DATABASE_URL ? describe : describe.skip + +const SCHEMA = 'stash_decrypt_lifecycle' +const TABLE = 'lifecycle_users' + +const { SCOPED_URL, output, record } = vi.hoisted(() => { + const url = process.env.STASH_TEST_DATABASE_URL + /** Everything the commands printed, in order. */ + const output: string[] = [] + return { + /** The connection every command makes: this suite's schema first. */ + SCOPED_URL: url + ? `${url}${url.includes('?') ? '&' : '?'}options=${encodeURIComponent('-c search_path=stash_decrypt_lifecycle')}` + : '', + output, + record: (...args: unknown[]) => { + output.push(args.map(String).join(' ')) + }, + } +}) + +vi.mock('@clack/prompts', () => ({ + intro: vi.fn(record), + outro: vi.fn(record), + note: vi.fn((body: string, title?: string) => record(title ?? '', body)), + log: { + info: vi.fn(record), + warn: vi.fn(record), + error: vi.fn(record), + success: vi.fn(record), + step: vi.fn(record), + }, + confirm: vi.fn(async () => true), + isCancel: vi.fn(() => false), +})) +vi.mock('@/config/index.js', () => ({ + loadStashConfig: vi.fn(async () => ({ databaseUrl: SCOPED_URL })), +})) +vi.mock('@/commands/encrypt/context.js', () => ({ + // Decodes {@link envelope}. Only the client is used by `decrypt backfill`. + loadEncryptionContext: vi.fn(async () => ({ + client: { + bulkDecrypt: async ( + payloads: Array<{ id?: string; data: { c: string } | null }>, + ) => ({ + data: payloads.map((payload) => ({ + id: payload.id, + data: JSON.parse(payload.data?.c ?? 'null').plaintext, + })), + }), + }, + })), + requireTable: vi.fn(), +})) + +import { appendEvent, installMigrationsSchema } from '@cipherstash/migrate' +import type pg from 'pg' +import { decryptBackfillCommand } from '../backfill.js' +import { decryptDropCommand } from '../drop.js' +import { decryptPlanCommand } from '../plan.js' +import { decryptStatusCommand } from '../status.js' + +/** A v3-shaped marker the stub client "decrypts" back to `plaintext`. */ +function envelope(plaintext: string) { + return { + v: 3, + i: { t: TABLE, c: 'email_encrypted' }, + c: JSON.stringify({ plaintext }), + } +} + +/** The status table's data rows, from the latest `decrypt status`. */ +async function statusRows(): Promise { + output.length = 0 + await decryptStatusCommand() + const table = output.find((line) => + line.startsWith('Reverse migration status'), + ) + expect(table, output.join('\n')).toBeDefined() + return (table ?? '').split('\n').slice(1) +} + +function rowFor(rows: string[], column: string): string { + const row = rows.find((r) => r.startsWith(`${TABLE} ${column} `)) + expect(row, rows.join('\n')).toBeDefined() + return row ?? '' +} + +describeLive('decrypt lifecycle — live Postgres', () => { + let client: pg.Client + let cwd: string + let originalCwd: string + + beforeAll(async () => { + const { default: pgModule } = await import('pg') + client = new pgModule.Client({ connectionString: SCOPED_URL }) + await client.connect() + originalCwd = process.cwd() + }) + + afterAll(async () => { + await client.query(`DROP SCHEMA IF EXISTS ${SCHEMA} CASCADE`) + await client + .query('DELETE FROM cipherstash.cs_migrations WHERE table_name = $1', [ + TABLE, + ]) + .catch(() => undefined) + await client.end().catch(() => undefined) + }) + + beforeEach(async () => { + vi.spyOn(process, 'exit').mockImplementation((code) => { + throw new Error(`process.exit(${code}):\n${output.join('\n')}`) + }) + await client.query(`DROP SCHEMA IF EXISTS ${SCHEMA} CASCADE`) + await client.query(`CREATE SCHEMA ${SCHEMA}`) + for (const domain of [ + 'eql_v3_text_search', + 'eql_v3_text', + 'eql_v3_text_eq', + ]) { + await client.query(`CREATE DOMAIN ${SCHEMA}.${domain} AS jsonb`) + } + await client + .query('DELETE FROM cipherstash.cs_migrations WHERE table_name = $1', [ + TABLE, + ]) + .catch(() => undefined) + + cwd = fs.mkdtempSync(path.join(os.tmpdir(), 'stash-decrypt-lifecycle-')) + process.chdir(cwd) + output.length = 0 + }) + + afterEach(() => { + process.chdir(originalCwd) + fs.rmSync(cwd, { recursive: true, force: true }) + vi.restoreAllMocks() + }) + + it('plan, status, backfill and drop all reverse into the twin the manifest names', async () => { + await client.query(` + CREATE TABLE ${TABLE} ( + id int PRIMARY KEY, + email_encrypted eql_v3_text_search + ) + `) + for (let id = 1; id <= 5; id++) { + await client.query( + `INSERT INTO ${TABLE} (id, email_encrypted) VALUES ($1, $2)`, + [id, JSON.stringify(envelope(`user-${id}@example.com`))], + ) + } + // The forward manifest names the original column `contact`, not the + // `email` the `_encrypted` convention would propose. + fs.mkdirSync(path.join(cwd, '.cipherstash')) + fs.writeFileSync( + path.join(cwd, '.cipherstash', 'migrations.json'), + JSON.stringify({ + version: 1, + tables: { + [TABLE]: [{ column: 'contact', encryptedColumn: 'email_encrypted' }], + }, + }), + ) + + // Before anything: status proposes the twin plan will add. + expect(rowFor(await statusRows(), 'email_encrypted')).toContain( + 'contact (text, to add)', + ) + + // plan writes the ADD COLUMN; apply it as the user would. + await decryptPlanCommand({ table: TABLE, column: 'email_encrypted' }) + const [planFile] = fs.readdirSync(path.join(cwd, 'drizzle')) + expect(planFile).toBeDefined() + const planSql = fs.readFileSync( + path.join(cwd, 'drizzle', planFile), + 'utf-8', + ) + expect(planSql).toContain('ADD COLUMN IF NOT EXISTS "contact" text') + await client.query(planSql) + fs.rmSync(path.join(cwd, 'drizzle'), { recursive: true }) + + let row = rowFor(await statusRows(), 'email_encrypted') + expect(row).toMatch(/email_encrypted\s+eql_v3_text_search\s+contact\s/) + expect(row).not.toContain('twin-has-values') + + await decryptBackfillCommand({ + table: TABLE, + column: 'email_encrypted', + confirmDualWritesDeployed: true, + }) + const { rows: filled } = await client.query<{ n: string }>( + `SELECT count(*) AS n FROM ${TABLE} WHERE contact = 'user-' || id || '@example.com'`, + ) + expect(filled[0].n).toBe('5') + + row = rowFor(await statusRows(), 'email_encrypted') + expect(row).toMatch(/contact\s+decrypted\s+\(backfill complete\)/) + expect(row).not.toContain('forced-pass-owed') + + output.length = 0 + await decryptPlanCommand() + expect(output.join('\n')).toContain( + `${TABLE}.email_encrypted → contact: decrypted`, + ) + + output.length = 0 + await decryptDropCommand({ table: TABLE, column: 'email_encrypted' }) + const [dropFile] = fs.readdirSync(path.join(cwd, 'drizzle')) + const dropSql = fs.readFileSync( + path.join(cwd, 'drizzle', dropFile), + 'utf-8', + ) + expect(dropSql).toContain('"contact"') + await client.query(dropSql) + const { rows: left } = await client.query<{ column_name: string }>( + `SELECT column_name FROM information_schema.columns + WHERE table_schema = $1 AND table_name = $2 ORDER BY column_name`, + [SCHEMA, TABLE], + ) + expect(left.map((r) => r.column_name)).toEqual(['contact', 'id']) + + // The encrypted column is gone; status still shows the finished migration. + row = rowFor(await statusRows(), 'email_encrypted') + expect(row).toMatch(/contact\s+encrypted-dropped\s+\(dropped\)/) + expect(row).not.toContain('encrypted-col-missing') + }) + + it('status names the fallback twin for a column called _encrypted', async () => { + await client.query(` + CREATE TABLE ${TABLE} ( + id int PRIMARY KEY, + _encrypted eql_v3_text, + email_encrypted eql_v3_text_search + ) + `) + + const rows = await statusRows() + + expect(rowFor(rows, '_encrypted')).toContain( + '_encrypted_plaintext (text, to add)', + ) + expect(rowFor(rows, 'email_encrypted')).toContain('email (text, to add)') + }) + + it('status flags a twin that is itself an EQL column, which plan and backfill refuse', async () => { + await client.query(` + CREATE TABLE ${TABLE} ( + id int PRIMARY KEY, + email_encrypted eql_v3_text_search, + email eql_v3_text_eq + ) + `) + + const row = rowFor(await statusRows(), 'email_encrypted') + + expect(row).toContain('plaintext-col-is-eql') + expect(row).not.toContain('plaintext-col-missing') + }) + + it('status shows a column whose earlier reverse migration was superseded as starting over', async () => { + await client.query(` + CREATE TABLE ${TABLE} ( + id int PRIMARY KEY, + email_encrypted eql_v3_text_search + ) + `) + await installMigrationsSchema(client) + // Reversed into old_email once, then old_email was encrypted again: the + // reverse drop is history, and the forward event after it supersedes it. + await appendEvent(client, { + tableName: TABLE, + columnName: 'email_encrypted', + event: 'encrypted_dropped', + phase: 'encrypted-dropped', + details: { plaintextColumn: 'old_email' }, + }) + await appendEvent(client, { + tableName: TABLE, + columnName: 'old_email', + event: 'dual_writing', + phase: 'dual-writing', + details: { encryptedColumn: 'email_encrypted' }, + }) + + const row = rowFor(await statusRows(), 'email_encrypted') + + expect(row).not.toContain('encrypted-dropped') + expect(row).not.toContain('old_email') + expect(row).toMatch(/email_encrypted\s+eql_v3_text_search\s+email\s/) + }) + + it('status flags a twin that already holds values, which backfill refuses without a choice', async () => { + await client.query(` + CREATE TABLE ${TABLE} ( + id int PRIMARY KEY, + email_encrypted eql_v3_text_search, + email text + ) + `) + await client.query( + `INSERT INTO ${TABLE} VALUES (1, $1, 'stale@example.com')`, + [JSON.stringify(envelope('user-1@example.com'))], + ) + + expect(rowFor(await statusRows(), 'email_encrypted')).toContain( + 'twin-has-values', + ) + await expect( + decryptBackfillCommand({ + table: TABLE, + column: 'email_encrypted', + confirmDualWritesDeployed: true, + }), + ).rejects.toThrow(/--force[\s\S]*--confirm-plaintext-current/) + }) +}) diff --git a/languages/typescript/packages/cli/src/commands/decrypt/__tests__/decrypt-status.test.ts b/languages/typescript/packages/cli/src/commands/decrypt/__tests__/decrypt-status.test.ts deleted file mode 100644 index 956a7933b..000000000 --- a/languages/typescript/packages/cli/src/commands/decrypt/__tests__/decrypt-status.test.ts +++ /dev/null @@ -1,174 +0,0 @@ -/** - * `stash decrypt status` must name the same plaintext twin `decrypt plan` and - * `decrypt backfill` would, and flag a twin those commands would refuse. It - * once derived the name itself and disagreed with them on an edge case. - */ - -import type { MigrationStateRow } from '@cipherstash/migrate' -import { beforeEach, describe, expect, it, vi } from 'vitest' - -vi.mock('pg', () => ({ - default: { - Client: class { - connect = vi.fn(async () => {}) - end = vi.fn(async () => {}) - query = vi.fn(async () => ({ rows: [] })) - }, - }, -})) - -const readers = vi.hoisted(() => ({ - latestByColumnSafe: vi.fn( - async (): Promise> => new Map(), - ), - fetchPhysicalColumns: vi.fn( - async (): Promise>> => new Map(), - ), -})) -vi.mock('@/commands/encrypt/lib/db-readers.js', () => readers) - -vi.mock('@cipherstash/migrate', async (importOriginal) => ({ - ...(await importOriginal()), - readManifest: vi.fn(async () => null), -})) -vi.mock('@clack/prompts', () => ({ - intro: vi.fn(), - outro: vi.fn(), - note: vi.fn(), - log: { info: vi.fn(), error: vi.fn(), warn: vi.fn(), success: vi.fn() }, -})) -vi.mock('@/config/index.js', () => ({ - loadStashConfig: vi.fn(async () => ({ databaseUrl: 'postgres://test' })), -})) -vi.mock('@/commands/encrypt/context.js', () => ({ - loadEncryptionContext: vi.fn(), - requireTable: vi.fn(), -})) -vi.mock('@/commands/init/utils.js', () => ({ - detectPackageManager: vi.fn(() => 'npm'), - runnerCommand: vi.fn((_pm: string, ref: string) => `npx ${ref}`), -})) - -import * as p from '@clack/prompts' -import { decryptStatusCommand } from '../status.js' - -function event( - overrides: Partial & - Pick, -): MigrationStateRow { - return { - tableName: 'users', - cursorValue: null, - rowsProcessed: null, - rowsTotal: null, - details: null, - createdAt: new Date(0), - ...overrides, - } -} - -/** The rendered status table, one string per data row. */ -async function statusRows(): Promise { - await decryptStatusCommand() - const table = - vi - .mocked(p.note) - .mock.calls.find( - ([, title]) => title === 'Reverse migration status', - )?.[0] ?? '' - return table.split('\n').slice(1) -} - -describe('decrypt status', () => { - beforeEach(() => { - vi.clearAllMocks() - readers.latestByColumnSafe.mockResolvedValue(new Map()) - }) - - it('derives the twin name with the same rule as plan, even for a column named _encrypted', async () => { - readers.fetchPhysicalColumns.mockResolvedValue( - new Map([ - [ - 'users', - new Map([ - ['_encrypted', 'eql_v3_text'], - ['email_encrypted', 'eql_v3_text_search'], - ]), - ], - ]), - ) - - const rows = await statusRows() - - expect(rows.find((row) => row.includes('_encrypted '))).toContain( - '_encrypted_plaintext (text, to add)', - ) - expect(rows.find((row) => row.includes('email_encrypted'))).toContain( - 'email (text, to add)', - ) - }) - - it('flags a derived twin that is itself an EQL column, which plan and backfill refuse', async () => { - readers.fetchPhysicalColumns.mockResolvedValue( - new Map([ - [ - 'users', - new Map([ - ['email_encrypted', 'eql_v3_text_search'], - ['email', 'eql_v3_text_eq'], - ]), - ], - ]), - ) - - const rows = await statusRows() - - const row = rows.find((r) => r.startsWith('users email_encrypted')) ?? '' - expect(row).toContain('plaintext-col-is-eql') - expect(row).not.toContain('plaintext-col-missing') - }) - - it('shows a column whose earlier reverse lifecycle finished as starting over', async () => { - readers.fetchPhysicalColumns.mockResolvedValue( - new Map([ - [ - 'users', - new Map([ - ['email_encrypted', 'eql_v3_text_search'], - ['email', null], - ]), - ], - ]), - ) - readers.latestByColumnSafe.mockResolvedValue( - new Map([ - [ - 'users.email_encrypted', - event({ - id: '10', - columnName: 'email_encrypted', - event: 'encrypted_dropped', - phase: 'encrypted-dropped', - details: { plaintextColumn: 'old_email' }, - }), - ], - [ - 'users.old_email', - event({ - id: '12', - columnName: 'old_email', - event: 'dual_writing', - phase: 'dual-writing', - }), - ], - ]), - ) - - const rows = await statusRows() - - const row = rows.find((r) => r.startsWith('users email_encrypted')) ?? '' - expect(row).not.toContain('encrypted-dropped') - expect(row).not.toContain('old_email') - expect(row).toMatch(/email_encrypted\s+eql_v3_text_search\s+email\s/) - }) -}) diff --git a/languages/typescript/packages/cli/src/commands/decrypt/__tests__/fake-migration.ts b/languages/typescript/packages/cli/src/commands/decrypt/__tests__/fake-migration.ts new file mode 100644 index 000000000..67f67d5ec --- /dev/null +++ b/languages/typescript/packages/cli/src/commands/decrypt/__tests__/fake-migration.ts @@ -0,0 +1,112 @@ +/** + * A stand-in for `@cipherstash/migrate`'s `ColumnMigration`, for the command + * tests that mock the database away. Its state comes from the real rules + * (`deriveColumnMigration`, `planPassFor`) over the history given, so a test + * describes the event history, not the answers; only the methods that write + * are spies. The rules themselves are covered in `@cipherstash/migrate`, and + * the commands against real Postgres in `decrypt-lifecycle.live.test.ts`. + */ + +import { + type ColumnMigration, + deriveColumnMigration, + type MigrationEvent, + type MigrationPhase, + type MigrationStateRow, + type OpenCounterpart, + planPassFor, +} from '@cipherstash/migrate' +import { vi } from 'vitest' + +let nextId = 1 + +/** One reverse event under `users.email_encrypted`, naming the twin. */ +export function reverseEvent( + event: MigrationEvent, + phase: MigrationPhase, + extra: Partial = {}, +): MigrationStateRow { + return { + id: String(nextId++), + tableName: 'users', + columnName: 'email_encrypted', + event, + phase, + cursorValue: null, + rowsProcessed: null, + rowsTotal: null, + details: { plaintextColumn: 'email' }, + createdAt: new Date(0), + ...extra, + } +} + +/** A reverse column migration mid-pass. */ +export const inFlight = (): MigrationStateRow[] => [ + reverseEvent('decrypt_dual_writing', 'decrypt-dual-writing'), + reverseEvent('decrypt_checkpoint', 'decrypting', { + cursorValue: '5', + rowsProcessed: 5, + rowsTotal: 10, + }), +] + +/** A reverse column migration whose backfill completed. */ +export const decrypted = (): MigrationStateRow[] => [ + reverseEvent('decrypt_dual_writing', 'decrypt-dual-writing'), + reverseEvent('decrypted', 'decrypted', { rowsProcessed: 10, rowsTotal: 10 }), +] + +export type FakeMigration = ColumnMigration & { + confirmDualWrites: ReturnType + recordDropGenerated: ReturnType + recordSourceRemoved: ReturnType +} + +export function fakeMigration( + opts: { + history?: MigrationStateRow[] + twin?: string + blockedBy?: OpenCounterpart | null + twinHasValues?: boolean + } = {}, +): FakeMigration { + const state = deriveColumnMigration({ + direction: 'reverse', + history: opts.history ?? [], + }) + const twinHasValues = opts.twinHasValues ?? false + const fake = { + ...state, + table: 'users', + column: 'email_encrypted', + twin: { column: opts.twin ?? 'email', via: 'convention' as const }, + blockedBy: opts.blockedBy ?? null, + existingTwinValues: vi.fn( + async () => state.phase === null && twinHasValues, + ), + planPass: vi.fn(async ({ force }: { force: boolean }) => + planPassFor(state, { + force, + twinHasValues: state.phase === null && twinHasValues, + }), + ), + counterpartMessage: vi.fn(() => null), + confirmDualWrites: vi.fn(), + recordDropGenerated: vi.fn(), + recordSourceRemoved: vi.fn(), + } + fake.confirmDualWrites.mockImplementation(async () => { + const confirmed = fakeMigration({ + ...opts, + history: [ + ...(opts.history ?? []), + reverseEvent('decrypt_dual_writing', 'decrypt-dual-writing'), + ], + }) + return confirmed + }) + fake.recordDropGenerated.mockImplementation(async () => fake) + fake.recordSourceRemoved.mockImplementation(async () => fake) + return fake as unknown as FakeMigration +} diff --git a/languages/typescript/packages/cli/src/commands/decrypt/__tests__/resolve-pair.test.ts b/languages/typescript/packages/cli/src/commands/decrypt/__tests__/resolve-pair.test.ts index 88d78c157..ed08bbe49 100644 --- a/languages/typescript/packages/cli/src/commands/decrypt/__tests__/resolve-pair.test.ts +++ b/languages/typescript/packages/cli/src/commands/decrypt/__tests__/resolve-pair.test.ts @@ -4,31 +4,23 @@ * The pairing is what every later step trusts: the backfill fills that column, * and `decrypt drop`'s coverage gate verifies it before destroying the * ciphertext. A pairing that silently drifted — a flag naming a different - * column mid-lifecycle — would let that gate pass against the wrong column. + * column mid-migration — would let that gate pass against the wrong column. + * + * The catalog probes are replaced; the `cs_migrations` history is served by a + * fake client, so the twin rules run for real in `ColumnMigration`. */ -import type { - EncryptedColumnInfo, - MigrationStateRow, -} from '@cipherstash/migrate' -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import type { EncryptedColumnInfo } from '@cipherstash/migrate' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -// Only the catalog/state probes are replaced; the domain mapping and phase -// predicates stay real so a rename on either side fails loudly. const probes = vi.hoisted(() => ({ listEncryptedColumns: vi.fn(async (): Promise => []), columnExists: vi.fn(async () => true), detectColumnEqlVersion: vi.fn(async (): Promise<2 | 3 | null> => null), columnSqlType: vi.fn(async (): Promise => null), - columnHasValues: vi.fn(async () => false), - progress: vi.fn( - async ( - _client: unknown, - _table: string, - _column: string, - ): Promise | null> => null, - ), - readManifest: vi.fn(async (): Promise => null), })) vi.mock('@cipherstash/migrate', async (importOriginal) => ({ ...(await importOriginal()), @@ -42,29 +34,58 @@ vi.mock('@/commands/encrypt/context.js', () => ({ import type pg from 'pg' import { BackfillConfigError } from '@/commands/encrypt/backfill.js' import { - assessExistingTwin, type DecryptPair, - derivePlaintextName, openForwardMigrationMessage, + PlaintextColumnIsEqlError, resolveDecryptPair, twinHazardMessage, } from '../lib/resolve-pair.js' +import { fakeMigration } from './fake-migration.js' -const db = {} as pg.ClientBase const EMAIL_ENC: EncryptedColumnInfo = { column: 'email_encrypted', domain: 'eql_v3_text_search', version: 3, } -function reverseState(plaintextColumn: string): Partial { - return { - event: 'decrypt_checkpoint', - phase: 'decrypting', - details: { plaintextColumn }, - } +/** A raw `cs_migrations` row, as the driver returns it. */ +interface RawEvent { + id: number + column: string + event: string + phase: string + details?: Record } +let events: RawEvent[] = [] +/** Answers `ColumnMigration`'s history read from {@link events}. */ +const db = { + query: vi.fn(async (_sql: string, params: unknown[]) => ({ + rows: events + .filter((e) => e.column === params[1]) + .map((e) => ({ + id: String(e.id), + table_name: 'users', + column_name: e.column, + event: e.event, + phase: e.phase, + cursor_value: null, + rows_processed: null, + rows_total: null, + details: e.details ?? null, + created_at: new Date(0), + })), + })), +} as unknown as pg.ClientBase + +const recorded = (id: number, plaintextColumn: string): RawEvent => ({ + id, + column: 'email_encrypted', + event: 'decrypt_checkpoint', + phase: 'decrypting', + details: { plaintextColumn }, +}) + async function failure(promise: Promise): Promise { const error = await promise.catch((e: unknown) => e) expect(error).toBeInstanceOf(BackfillConfigError) @@ -72,76 +93,89 @@ async function failure(promise: Promise): Promise { } describe('resolveDecryptPair', () => { + const cwd = process.cwd() + let dir: string + beforeEach(() => { vi.clearAllMocks() + events = [] + dir = mkdtempSync(join(tmpdir(), 'resolve-pair-')) + process.chdir(dir) probes.listEncryptedColumns.mockResolvedValue([EMAIL_ENC]) probes.detectColumnEqlVersion.mockResolvedValue(null) probes.columnSqlType.mockResolvedValue(null) - probes.columnHasValues.mockResolvedValue(false) - probes.progress.mockResolvedValue(null) - probes.readManifest.mockResolvedValue(null) }) - /** `progress` answers per column: reverse history under the encrypted - * column, forward history under the plaintext name. */ - function history(byColumn: Record>) { - probes.progress.mockImplementation( - async (_client, _table, column) => byColumn[column] ?? null, - ) - } + afterEach(() => { + process.chdir(cwd) + rmSync(dir, { recursive: true, force: true }) + }) it('reads the forward history recorded under the twin’s name', async () => { - history({ email: { id: '3', event: 'backfilled', phase: 'backfilled' } }) + events = [ + { id: 3, column: 'email', event: 'backfilled', phase: 'backfilled' }, + ] const pair = await resolveDecryptPair(db, { table: 'users', encryptedColumn: 'email_encrypted', }) - expect(pair.forwardState?.phase).toBe('backfilled') - expect(pair.lifecycleActive).toBe(false) + expect(pair.migration.blockedBy).toEqual({ + direction: 'forward', + column: 'email', + phase: 'backfilled', + }) + expect(pair.migration.phase).toBeNull() }) - it('a drop generated in THIS lifecycle (not yet applied, or aborted) keeps it active', async () => { - history({ - email_encrypted: { - id: '10', + it('a drop generated in THIS column migration (not yet applied, or aborted) keeps it current', async () => { + events = [ + { + id: 10, + column: 'email_encrypted', event: 'encrypted_dropped', phase: 'encrypted-dropped', details: { plaintextColumn: 'contact_email' }, }, // Older than the drop: the forward history that preceded the reversal. - contact_email: { id: '4', event: 'backfilled', phase: 'backfilled' }, - }) + { id: 4, column: 'contact_email', event: 'dropped', phase: 'dropped' }, + ] const pair = await resolveDecryptPair(db, { table: 'users', encryptedColumn: 'email_encrypted', }) - expect(pair.lifecycleActive).toBe(true) + expect(pair.migration.phase).toBe('encrypted-dropped') expect(pair).toMatchObject({ plaintextColumn: 'contact_email', via: 'recorded', }) }) - it('a lifecycle that finished before the column was encrypted again neither binds nor counts as active', async () => { - history({ - email_encrypted: { - id: '10', + it('a column migration that finished before the column was encrypted again neither binds nor counts', async () => { + events = [ + { + id: 10, + column: 'email_encrypted', event: 'encrypted_dropped', phase: 'encrypted-dropped', details: { plaintextColumn: 'contact_email' }, }, // A forward rollout AFTER the drop re-created the encrypted column. - contact_email: { id: '12', event: 'dual_writing', phase: 'dual-writing' }, - }) + { + id: 12, + column: 'contact_email', + event: 'dual_writing', + phase: 'dual-writing', + }, + ] const pair = await resolveDecryptPair(db, { table: 'users', encryptedColumn: 'email_encrypted', plaintextColumn: 'email', }) - expect(pair.lifecycleActive).toBe(false) + expect(pair.migration.phase).toBeNull() expect(pair).toMatchObject({ plaintextColumn: 'email', via: 'flag' }) }) @@ -155,14 +189,24 @@ describe('resolveDecryptPair', () => { }) it('prefers the original name the forward manifest recorded', async () => { - probes.readManifest.mockResolvedValue({ - version: 1, - tables: { - users: [ - { column: 'email_address', encryptedColumn: 'email_encrypted' }, - ], - }, - }) + mkdirSync(join(dir, '.cipherstash')) + writeFileSync( + join(dir, '.cipherstash', 'migrations.json'), + JSON.stringify({ + version: 1, + tables: { + users: [ + { + column: 'email_address', + castAs: 'text', + indexes: [], + targetPhase: 'dropped', + encryptedColumn: 'email_encrypted', + }, + ], + }, + }), + ) const pair = await resolveDecryptPair(db, { table: 'users', @@ -198,7 +242,7 @@ describe('resolveDecryptPair', () => { }) it('reuses the pairing an earlier reverse run recorded', async () => { - probes.progress.mockResolvedValue(reverseState('contact_email')) + events = [recorded(1, 'contact_email')] const pair = await resolveDecryptPair(db, { table: 'users', @@ -211,7 +255,7 @@ describe('resolveDecryptPair', () => { }) it('refuses a --plaintext-column that contradicts the recorded pairing', async () => { - probes.progress.mockResolvedValue(reverseState('contact_email')) + events = [recorded(1, 'contact_email')] const error = await failure( resolveDecryptPair(db, { @@ -221,23 +265,28 @@ describe('resolveDecryptPair', () => { }), ) expect(error.message).toContain( - 'already being reversed into "contact_email"', + 'already being migrated with "contact_email"', ) expect(error.message).toContain('coverage check') + expect(error.message).toContain('Drop --plaintext-column') }) it('ignores forward history under the same key — only reverse events pair', async () => { - probes.progress.mockResolvedValue({ - event: 'backfilled', - phase: 'backfilled', - details: { plaintextColumn: 'not-a-reverse-record' }, - }) + events = [ + { + id: 1, + column: 'email_encrypted', + event: 'backfilled', + phase: 'backfilled', + details: { plaintextColumn: 'not-a-reverse-record' }, + }, + ] const pair = await resolveDecryptPair(db, { table: 'users', encryptedColumn: 'email_encrypted', }) - expect(pair.state).toBeNull() + expect(pair.migration.latest).toBeNull() expect(pair.via).toBe('convention') }) @@ -271,6 +320,7 @@ describe('resolveDecryptPair', () => { }), ) expect(error.message).toContain('is itself an EQL v3 column') + expect(error).toBeInstanceOf(PlaintextColumnIsEqlError) }) it('reports the twin’s declared type once it exists', async () => { @@ -284,29 +334,7 @@ describe('resolveDecryptPair', () => { }) }) -describe('derivePlaintextName', () => { - it('strips the _encrypted convention', () => { - expect(derivePlaintextName('email_encrypted')).toEqual({ - name: 'email', - via: 'convention', - }) - }) - - it('never derives an empty name from a column called exactly _encrypted', () => { - expect(derivePlaintextName('_encrypted')).toEqual({ - name: '_encrypted_plaintext', - via: 'fallback', - }) - expect(derivePlaintextName('secret')).toEqual({ - name: 'secret_plaintext', - via: 'fallback', - }) - }) -}) - -describe('assessExistingTwin', () => { - const db = {} as pg.ClientBase - +describe('openForwardMigrationMessage and twinHazardMessage', () => { function twin(overrides: Partial = {}): DecryptPair { return { table: 'users', @@ -316,60 +344,11 @@ describe('assessExistingTwin', () => { plaintextColumn: 'email', via: 'manifest', plaintextSqlType: 'text', - state: null, - lifecycleActive: false, - forwardState: null, + migration: fakeMigration(), ...overrides, } } - function forwardAt(phase: MigrationStateRow['phase']): MigrationStateRow { - return { - id: '3', - tableName: 'users', - columnName: 'email', - event: 'backfilled', - phase, - cursorValue: null, - rowsProcessed: null, - rowsTotal: null, - details: null, - createdAt: new Date(0), - } - } - - beforeEach(() => { - vi.clearAllMocks() - probes.columnHasValues.mockResolvedValue(false) - }) - - it('trusts a twin the reverse lifecycle in progress filled, without scanning it', async () => { - probes.columnHasValues.mockResolvedValue(true) - - expect(await assessExistingTwin(db, twin({ lifecycleActive: true }))).toBe( - null, - ) - expect(probes.columnHasValues).not.toHaveBeenCalled() - }) - - it('has nothing to check before the twin exists', async () => { - expect( - await assessExistingTwin(db, twin({ plaintextSqlType: null })), - ).toBeNull() - }) - - it('accepts an empty twin with no forward history', async () => { - expect(await assessExistingTwin(db, twin())).toBeNull() - }) - - it('flags a twin that already holds values', async () => { - probes.columnHasValues.mockResolvedValue(true) - - expect(await assessExistingTwin(db, twin())).toEqual({ - kind: 'prepopulated', - }) - }) - it.each([ 'schema-added', 'dual-writing', @@ -378,38 +357,19 @@ describe('assessExistingTwin', () => { ] as const)( 'refuses the source column of a forward migration still open at %s', (phase) => { - expect( - openForwardMigrationMessage(twin({ forwardState: forwardAt(phase) })), - ).toContain(`still open at '${phase}'`) + const migration = fakeMigration({ + blockedBy: { direction: 'forward', column: 'email', phase }, + }) + expect(openForwardMigrationMessage(twin({ migration }))).toContain( + `still open at '${phase}'`, + ) }, ) - it("does not refuse once the forward migration reached 'dropped'", () => { - expect( - openForwardMigrationMessage(twin({ forwardState: forwardAt('dropped') })), - ).toBeNull() + it('does not refuse when no forward column migration is open', () => { expect(openForwardMigrationMessage(twin())).toBeNull() }) - it("a forward lifecycle at 'dropped' is no longer the source, but its values still count", async () => { - expect( - await assessExistingTwin( - db, - twin({ forwardState: forwardAt('dropped') }), - ), - ).toBeNull() - - // `dropped` is recorded when the drop is generated; if it was never - // applied the old column is still there, full. - probes.columnHasValues.mockResolvedValue(true) - expect( - await assessExistingTwin( - db, - twin({ forwardState: forwardAt('dropped') }), - ), - ).toEqual({ kind: 'prepopulated' }) - }) - it('explains both ways forward, with names only', () => { const message = twinHazardMessage(twin()) expect(message).toContain('users.email already holds values') diff --git a/languages/typescript/packages/cli/src/commands/decrypt/backfill.ts b/languages/typescript/packages/cli/src/commands/decrypt/backfill.ts index f8c01a225..6d3634a7d 100644 --- a/languages/typescript/packages/cli/src/commands/decrypt/backfill.ts +++ b/languages/typescript/packages/cli/src/commands/decrypt/backfill.ts @@ -1,6 +1,8 @@ import { - appendEvent, + type ColumnMigration, + ColumnMigrationError, type DecryptBackfillResult, + type ForcedBy, formatCodes, installMigrationsSchema, PlaintextWriteError, @@ -18,11 +20,9 @@ import { detectPackageManager, runnerCommand } from '@/commands/init/utils.js' import { loadStashConfig } from '@/config/index.js' import { buildPgClientConfig, explainTlsError } from '@/db/client.js' import { - assessExistingTwin, type DecryptPair, openForwardMigrationMessage, resolveDecryptPair, - type TwinHazard, twinHazardMessage, } from './lib/resolve-pair.js' @@ -73,8 +73,9 @@ export interface DecryptBackfillCommandOptions { force?: boolean /** * Assert that a plaintext twin which already holds values before the first - * reverse run is current, and keep those values. Without it (or `force`) - * the first run refuses such a twin; see {@link TwinHazard}. + * reverse run is current, and keep those values: recorded as the column + * migration's waiver. Without it (or `force`) the first run refuses such a + * twin. Refused once a forced pass has started. */ confirmPlaintextCurrent?: boolean /** @@ -100,12 +101,12 @@ export interface DecryptBackfillCommandOptions { * skipped, counted, left NULL, and reported. There is deliberately no bypass; * see the lock-context limitation in the `stash-cli` docs. * - * On the first run, a twin that already holds values (or is the source of a - * forward encryption that was never dropped) is refused unless the operator - * passes `--force` (overwrite it from the ciphertext) or - * `--confirm-plaintext-current` (keep it). The backfill only fills NULLs, so - * trusting stale values silently would let `decrypt drop` destroy the only - * current copy. + * A twin that is the source of a forward encryption never dropped is refused + * whatever the flags. On the first run, a twin that already holds values is + * refused unless the operator passes `--force` (overwrite it from the + * ciphertext, an obligation `@cipherstash/migrate` then enforces on every + * run until a forced pass completes) or `--confirm-plaintext-current` (keep + * it, recorded as a waiver). The rules themselves live in `ColumnMigration`. */ export async function decryptBackfillCommand( options: DecryptBackfillCommandOptions, @@ -171,9 +172,9 @@ export async function decryptBackfillCommand( if (openForward !== null) throw new BackfillConfigError(openForward) // Before anything is recorded: once the dual-write gate writes its event, - // later runs treat the twin's values as this lifecycle's own. - const hazard = await assessExistingTwin(db, pair) - if (hazard !== null) { + // later runs treat the twin's values as this column migration's own. + const twinHasValues = await pair.migration.existingTwinValues() + if (twinHasValues) { if (!options.force && !options.confirmPlaintextCurrent) { throw new BackfillConfigError(twinHazardMessage(pair)) } @@ -202,22 +203,20 @@ export async function decryptBackfillCommand( } } - const okToProceed = await ensureReverseDualWritesDeployed(db, { + const migration = await ensureReverseDualWritesDeployed({ pair, confirmFlag: options.confirmDualWritesDeployed === true, - acknowledgedHazard: - hazard === null - ? undefined - : { - hazard, - via: options.force ? 'force' : 'confirm-plaintext-current', - }, + waive: options.confirmPlaintextCurrent === true && !options.force, }) - if (!okToProceed) { + if (migration === null) { p.outro('Aborted.') return } + const plan = await migration.planPass({ force: options.force === true }) + const why = forcedReason(plan.forcedBy, options.table, pair) + if (why !== null) p.log.warn(why) + p.log.info( `Decrypting ${options.table}.${pair.encryptedColumn} → ${pair.plaintextColumn} (pk: ${pkColumn}, chunk: ${chunkSize} rows = one ZeroKMS request).`, ) @@ -278,6 +277,7 @@ export async function decryptBackfillCommand( } catch (error) { if ( error instanceof BackfillConfigError || + error instanceof ColumnMigrationError || error instanceof UndecryptableBatchError || error instanceof PlaintextWriteError ) { @@ -319,26 +319,30 @@ function requirePlaintextTwin(pair: DecryptPair): string { /** * The reverse dual-write gate — the mirror of `encrypt backfill`'s. Rows the * application writes DURING the backfill must land in both columns, or they - * end up encrypted-only and the drop coverage check later refuses. Records a - * `decrypt_dual_writing` event on first acceptance so re-runs skip the prompt. + * end up encrypted-only and the drop coverage check later refuses. On first + * acceptance `ColumnMigration.confirmDualWrites` records it, naming the twin, + * so re-runs skip the prompt; over a twin that already holds values that + * opens the forced-pass obligation, unless `waive` records the waiver. + * + * Only the reverse column migration IN PROGRESS passes the gate: an + * `encrypted-dropped` event from an earlier one (the column was dropped and + * later encrypted again) asks again. On one in progress, `waive` still + * records a waiver, or is refused once a forced pass has started. * - * Only the reverse lifecycle IN PROGRESS passes the gate: an - * `encrypted-dropped` event from an earlier lifecycle (the column was dropped - * and later encrypted again) asks again. + * Returns the column migration as it now stands, or `null` when declined. */ -async function ensureReverseDualWritesDeployed( - db: pg.PoolClient, - opts: { - pair: DecryptPair - confirmFlag: boolean - acknowledgedHazard?: { - hazard: TwinHazard - via: 'force' | 'confirm-plaintext-current' - } - }, -): Promise { +async function ensureReverseDualWritesDeployed(opts: { + pair: DecryptPair + confirmFlag: boolean + waive: boolean +}): Promise { const { pair } = opts - if (pair.lifecycleActive) return true + const waiver = opts.waive ? ('plaintext-current' as const) : undefined + if (pair.migration.phase !== null) { + return waiver === undefined + ? pair.migration + : pair.migration.confirmDualWrites({ waiver }) + } const isInteractive = Boolean(process.stdin.isTTY) && process.env.CI !== 'true' @@ -356,34 +360,36 @@ async function ensureReverseDualWritesDeployed( message: `Has the dual-write code been deployed for ${target} → ${pair.plaintextColumn}?`, initialValue: false, }) - if (p.isCancel(ok) || !ok) return false + if (p.isCancel(ok) || !ok) return null } else { p.log.error( `${target} has no recorded reverse \`dual-writing\` transition. Re-run with --confirm-dual-writes-deployed once your application is writing to both columns.`, ) - return false + return null } - await appendEvent(db, { - tableName: pair.table, - columnName: pair.encryptedColumn, - event: 'decrypt_dual_writing', - phase: 'decrypt-dual-writing', - details: { - plaintextColumn: pair.plaintextColumn, - // A twin that held values before this lifecycle: what was found and - // which flag the operator chose. `acknowledgedWith: 'force'` opens the - // forced-pass obligation `runDecryptBackfill` enforces on every run. - ...(opts.acknowledgedHazard - ? { - existingPlaintext: opts.acknowledgedHazard.hazard.kind, - acknowledgedWith: opts.acknowledgedHazard.via, - } - : {}), - }, - }) - p.log.success(`${target} marked as 'decrypt-dual-writing' in cs_migrations.`) - return true + const confirmed = await pair.migration.confirmDualWrites({ waiver }) + p.log.success(`${target} marked as '${confirmed.phase}' in cs_migrations.`) + return confirmed +} + +/** + * Why a run is a forced pass the operator did not ask for, or `null`. The + * column migration forces it; the operator should know before it overwrites. + */ +export function forcedReason( + forcedBy: ForcedBy, + table: string, + pair: Pick, +): string | null { + switch (forcedBy) { + case 'forced-pass': + return `This run is a forced pass: a forced pass over ${table}.${pair.plaintextColumn} started earlier and has not completed, so ${pair.plaintextColumn} may hold a mix of overwritten and stale values. Every run overwrites it from ${pair.encryptedColumn} until one completes.` + case 'existing-values': + return `This run is a forced pass: ${table}.${pair.plaintextColumn} held values before this reverse migration began and no waiver (--confirm-plaintext-current) is recorded, so it overwrites them from ${pair.encryptedColumn}.` + default: + return null + } } /** diff --git a/languages/typescript/packages/cli/src/commands/decrypt/drop.ts b/languages/typescript/packages/cli/src/commands/decrypt/drop.ts index cf53b95c8..e450fbf7d 100644 --- a/languages/typescript/packages/cli/src/commands/decrypt/drop.ts +++ b/languages/typescript/packages/cli/src/commands/decrypt/drop.ts @@ -1,9 +1,9 @@ import { readUninstallSql } from '@cipherstash/eql/sql' import { - appendEvent, type ColumnIndexInfo, countEqlColumnsInDatabase, countUndecrypted, + DIRECTION_VOCABULARY, EQL_COLUMN_PREDICATE_SQL, listColumnIndexes, qualifyTable, @@ -19,10 +19,7 @@ import { import { detectPackageManager, runnerCommand } from '@/commands/init/utils.js' import { loadStashConfig } from '@/config/index.js' import { createPgClient } from '@/db/client.js' -import { - recordedPlaintextColumn, - resolveDecryptPair, -} from './lib/resolve-pair.js' +import { resolveDecryptPair } from './lib/resolve-pair.js' /** * Options accepted by `stash decrypt drop`. Generates — never applies — a @@ -93,10 +90,11 @@ export async function decryptDropCommand(options: DecryptDropOptions) { plaintextColumn: options.plaintextColumn, }) - const requiredPhase = 'decrypted' - if (pair.state?.phase !== requiredPhase) { + const { migration } = pair + const requiredPhase = DIRECTION_VOCABULARY.reverse.completedPhase + if (migration.phase !== requiredPhase || migration.obligations.length > 0) { p.log.error( - `Cannot generate the drop migration: ${options.table}.${options.column} is in reverse phase '${pair.state?.phase ?? '—'}'. Must be '${requiredPhase}' — run \`stash decrypt backfill --table ${options.table} --column ${options.column}\` to completion, switch the application's reads to the plaintext column, then drop.`, + `Cannot generate the drop migration: ${options.table}.${options.column} is in reverse phase '${migration.phase ?? '—'}'${migration.obligations.length > 0 ? ' and still owes a forced pass' : ''}. Must be '${requiredPhase}' — run \`stash decrypt backfill --table ${options.table} --column ${options.column}\` to completion, switch the application's reads to the plaintext column, then drop.`, ) exitCode = 1 return @@ -105,7 +103,7 @@ export async function decryptDropCommand(options: DecryptDropOptions) { // `resolveDecryptPair` already refused a flag that contradicts the // record; here the record itself is required, because it is the only // positive evidence of which column the backfill filled. - const plaintextColumn = recordedPlaintextColumn(pair.state) + const plaintextColumn = migration.recordedTwin if (plaintextColumn === undefined) { p.log.error( `${options.table}.${options.column} reached '${requiredPhase}' but no plaintext column is recorded for it, so this command cannot verify which column holds the data. Re-run \`stash decrypt backfill --table ${options.table} --column ${options.column} --plaintext-column \` (it records the pairing; already-decrypted rows are not rewritten), then drop.`, @@ -177,18 +175,11 @@ export async function decryptDropCommand(options: DecryptDropOptions) { migrationsDir: options.migrationsDir, }) - await appendEvent(client, { - tableName: options.table, - columnName: options.column, - event: 'encrypted_dropped', - phase: 'encrypted-dropped', - details: { - plaintextColumn, - migrationFile: filePath, - drizzleScaffolded, - droppedIndexes: indexes.map((index) => `${index.schema}.${index.name}`), - uninstallEql: uninstallStatements.length > 0, - }, + await migration.recordDropGenerated({ + migrationFile: filePath, + drizzleScaffolded, + droppedIndexes: indexes.map((index) => `${index.schema}.${index.name}`), + uninstallEql: uninstallStatements.length > 0, }) // The forward manifest entry stays: the CLI owns `.cipherstash/`, and diff --git a/languages/typescript/packages/cli/src/commands/decrypt/lib/resolve-pair.ts b/languages/typescript/packages/cli/src/commands/decrypt/lib/resolve-pair.ts index 1657df59f..ad480ba19 100644 --- a/languages/typescript/packages/cli/src/commands/decrypt/lib/resolve-pair.ts +++ b/languages/typescript/packages/cli/src/commands/decrypt/lib/resolve-pair.ts @@ -1,30 +1,23 @@ import { + ColumnMigration, + ColumnMigrationError, columnExists, - columnHasValues, columnSqlType, detectColumnEqlVersion, - isReverseMigrationPhase, listEncryptedColumns, - type MigrationStateRow, type PlaintextTypeMapping, plaintextTypeForEqlDomain, - readManifest, + type TwinSource, } from '@cipherstash/migrate' import type pg from 'pg' import { BackfillConfigError } from '@/commands/encrypt/backfill.js' -import { progressSafe } from '@/commands/encrypt/lib/db-readers.js' /** * How the plaintext twin's name was decided. Destructive steps trust only the * first two: `flag` is the user saying so, `recorded` is the pairing the * reverse backfill actually ran against. The rest are proposals. */ -export type PlaintextResolution = - | 'flag' - | 'recorded' - | 'manifest' - | 'convention' - | 'fallback' +export type PlaintextResolution = TwinSource /** One encrypted column and the plaintext twin it reverses into. */ export interface DecryptPair { @@ -47,125 +40,35 @@ export interface DecryptPair { */ plaintextSqlType: string | null /** - * Latest REVERSE event for this encrypted column, if any. It can belong to - * an earlier, finished lifecycle; {@link lifecycleActive} says whether it - * is the one in progress. + * The reverse column migration of this encrypted column, from + * `@cipherstash/migrate`: its phase, the obligations it owes, and the + * forward column migration that blocks it, if any. Every rule about the + * pair is asked of it, not re-derived here. */ - state: MigrationStateRow | null - /** Whether {@link state} belongs to the reverse lifecycle in progress. */ - lifecycleActive: boolean - /** - * Latest FORWARD event recorded under the plaintext twin's name: the - * forward lifecycle that encrypted it, when there was one. Forward events - * are keyed by the plaintext column, so this is where they live. - */ - forwardState: MigrationStateRow | null -} - -/** - * The reverse lifecycle's state for one encrypted column: its latest event, - * but only when that event belongs to the reverse lifecycle. Reverse events - * are keyed by the encrypted column precisely so forward history never shows - * up here, but a hand-written or legacy row under the same key must not be - * mistaken for reverse progress either. - */ -export async function reverseState( - client: pg.ClientBase, - table: string, - encryptedColumn: string, -): Promise { - // `cs_migrations` not installed yet reads as no reverse history, which is - // a normal state for `plan`/`status` on a fresh project. - const last = await progressSafe(client, table, encryptedColumn) - return last && isReverseMigrationPhase(last.phase) ? last : null + migration: ColumnMigration } /** - * The forward lifecycle's state for a plaintext column: its latest event, but - * only when that event is a forward one. `null` when there is no history or - * no `cs_migrations` table yet. + * Thrown when the twin's name belongs to an EQL column, which cannot hold + * plaintext. A {@link BackfillConfigError}, so commands print it verbatim; + * `decrypt status` tells it apart to flag the row instead. */ -export async function forwardState( - client: pg.ClientBase, - table: string, - plaintextColumn: string, -): Promise { - const last = await progressSafe(client, table, plaintextColumn) - return last && !isReverseMigrationPhase(last.phase) ? last : null -} - -/** - * Whether the latest reverse event belongs to the lifecycle in progress, as - * opposed to an earlier one that finished. - * - * `decrypt-dual-writing`, `decrypting` and `decrypted` are always current. - * `encrypted-dropped` is recorded when the drop migration is GENERATED, and - * the encrypted column still exists (it is being resolved), so it means one - * of two things: the migration has not been applied, or its apply-time check - * aborted. That is still the same lifecycle. Or the column was dropped and - * later encrypted again: then the forward lifecycle that re-encrypted it has - * recorded events under the twin's name AFTER the drop. Event ids are - * monotonic, so the order tells them apart. - */ -export function isActiveReverseLifecycle( - state: MigrationStateRow | null, - forwardOfRecordedTwin: MigrationStateRow | null, -): boolean { - if (state === null) return false - if (state.phase !== 'encrypted-dropped') return true - return ( - forwardOfRecordedTwin === null || - BigInt(forwardOfRecordedTwin.id) < BigInt(state.id) - ) -} - -/** - * The plaintext name derived from the encrypted column's own name when - * nothing better is known: the `_encrypted` convention stripped, else - * `_plaintext`, so a derived name never collides with the - * encrypted column itself. Shared by every command that shows a twin name. - */ -export function derivePlaintextName(encryptedColumn: string): { - name: string - via: 'convention' | 'fallback' -} { - const suffix = '_encrypted' - return encryptedColumn.endsWith(suffix) && - encryptedColumn.length > suffix.length - ? { name: encryptedColumn.slice(0, -suffix.length), via: 'convention' } - : { name: `${encryptedColumn}_plaintext`, via: 'fallback' } -} - -/** `details.plaintextColumn` from a reverse event, when it recorded one. */ -export function recordedPlaintextColumn( - state: MigrationStateRow | null, -): string | undefined { - const value = state?.details?.plaintextColumn - return typeof value === 'string' && value.length > 0 ? value : undefined -} +export class PlaintextColumnIsEqlError extends BackfillConfigError {} /** * Resolve an encrypted column and the plaintext twin it reverses into. * * The SOURCE must be an EQL v3 column — named explicitly, never guessed — - * because the reverse lifecycle starts from what exists. The TARGET's name is - * decided in this order: - * - * 1. `--plaintext-column`, when given; - * 2. the pairing the reverse lifecycle IN PROGRESS recorded in - * `cs_migrations` — a flag that disagrees with it is refused rather than - * silently switching which column the lifecycle fills (and which column - * `decrypt drop`'s coverage gate then checks). A pairing from an earlier, - * finished lifecycle is not binding (see {@link isActiveReverseLifecycle}); - * 3. the forward manifest's entry whose `encryptedColumn` is this column — - * the original plaintext name, which is what most reversals want back; - * 4. the `_encrypted` convention, stripped; - * 5. `_plaintext`, so a name that cannot be derived never - * collides with the encrypted column itself. + * because the reverse column migration starts from what exists. The TARGET + * is decided by `ColumnMigration.load` in `@cipherstash/migrate`: + * `--plaintext-column` when given (refused when it contradicts the pairing + * the column migration in progress recorded), else that recorded pairing, + * else the forward manifest's original name, else the `_encrypted` + * convention stripped, else `_plaintext`. * * Throws {@link BackfillConfigError} (printed verbatim by the commands) for a - * source that is missing or not EQL v3, and for a target that is itself an - * EQL column. + * source that is missing or not EQL v3, for a contradicting flag, and for a + * target that is itself an EQL column ({@link PlaintextColumnIsEqlError}). */ export async function resolveDecryptPair( client: pg.ClientBase, @@ -180,55 +83,40 @@ export async function resolveDecryptPair( const candidates = await listEncryptedColumns(client, table) const source = candidates.find((c) => c.column === encryptedColumn) if (!source) { + const others = + candidates.length > 0 + ? ` EQL v3 columns on ${table}: ${candidates.map((c) => c.column).join(', ')}.` + : '' if (!(await columnExists(client, table, encryptedColumn))) { throw new BackfillConfigError( - `Column ${encryptedColumn} does not exist on ${table}. Pass the ENCRYPTED column with --column (the EQL v3 column you want to move back to plaintext).${candidates.length > 0 ? ` EQL v3 columns on ${table}: ${candidates.map((c) => c.column).join(', ')}.` : ''}`, + `Column ${encryptedColumn} does not exist on ${table}. Pass the ENCRYPTED column with --column (the EQL v3 column you want to move back to plaintext).${others}`, ) } throw new BackfillConfigError( - `${table}.${encryptedColumn} is not an EQL v3 column, so there is nothing to decrypt. Pass the ENCRYPTED column with --column.${candidates.length > 0 ? ` EQL v3 columns on ${table}: ${candidates.map((c) => c.column).join(', ')}.` : ''}`, + `${table}.${encryptedColumn} is not an EQL v3 column, so there is nothing to decrypt. Pass the ENCRYPTED column with --column.${others}`, ) } - const state = await reverseState(client, table, encryptedColumn) - const recordedTwin = recordedPlaintextColumn(state) - const recordedForward = - recordedTwin === undefined - ? null - : await forwardState(client, table, recordedTwin) - const lifecycleActive = isActiveReverseLifecycle(state, recordedForward) - const recorded = lifecycleActive ? recordedTwin : undefined - - let plaintextColumn: string - let via: PlaintextResolution - if (opts.plaintextColumn !== undefined) { - if (recorded !== undefined && recorded !== opts.plaintextColumn) { - throw new BackfillConfigError( - `${table}.${encryptedColumn} is already being reversed into "${recorded}" (recorded in cipherstash.cs_migrations), but --plaintext-column is "${opts.plaintextColumn}". Refusing to switch targets mid-lifecycle: the coverage check before the encrypted column is dropped would then verify the wrong column. Drop the flag to continue with "${recorded}".`, - ) - } - plaintextColumn = opts.plaintextColumn - via = 'flag' - } else if (recorded !== undefined) { - plaintextColumn = recorded - via = 'recorded' - } else { - const fromManifest = await manifestPlaintextFor(table, encryptedColumn) - if (fromManifest) { - plaintextColumn = fromManifest - via = 'manifest' - } else { - ;({ name: plaintextColumn, via } = derivePlaintextName(encryptedColumn)) - } - } - - if (plaintextColumn === encryptedColumn) { + let migration: ColumnMigration + try { + migration = await ColumnMigration.load(client, { + table, + column: encryptedColumn, + direction: 'reverse', + twin: opts.plaintextColumn, + }) + } catch (error) { + if (!(error instanceof ColumnMigrationError)) throw error throw new BackfillConfigError( - `The plaintext column cannot be the encrypted column itself (${encryptedColumn}). Pass --plaintext-column .`, + opts.plaintextColumn === undefined + ? `${error.message} Pass --plaintext-column .` + : `${error.message} Drop --plaintext-column to continue with the recorded column.`, ) } + const plaintextColumn = migration.twin.column + if ((await detectColumnEqlVersion(client, table, plaintextColumn)) !== null) { - throw new BackfillConfigError( + throw new PlaintextColumnIsEqlError( `${table}.${plaintextColumn} is itself an EQL v3 column, so it cannot hold plaintext. Pass --plaintext-column to choose another target.`, ) } @@ -239,66 +127,34 @@ export async function resolveDecryptPair( domain: source.domain, mapping: plaintextTypeForEqlDomain(source.domain), plaintextColumn, - via, + via: migration.twin.via, plaintextSqlType: await columnSqlType(client, table, plaintextColumn), - state, - lifecycleActive, - forwardState: - plaintextColumn === recordedTwin - ? recordedForward - : await forwardState(client, table, plaintextColumn), + migration, } } /** - * Why a plaintext twin that already exists cannot be trusted as-is when a - * reverse lifecycle starts: it already holds values (`prepopulated`), from - * wherever. The reverse backfill only fills NULLs, so it would keep those - * values, record `decrypted`, and let `decrypt drop` destroy the only current - * copy, in the encrypted column. (A twin that is the source of a forward - * migration still open is not a hazard to acknowledge but a refusal: - * {@link openForwardMigrationMessage}.) - */ -export type TwinHazard = { kind: 'prepopulated' } - -/** - * Check a twin before the FIRST reverse run trusts it. Returns `null` when - * there is nothing to check (the twin does not exist yet, or a reverse - * lifecycle is already in progress, so its values came from this lifecycle's - * own backfill and dual-writes) or nothing wrong. - */ -export async function assessExistingTwin( - client: pg.ClientBase, - pair: DecryptPair, -): Promise { - if (pair.lifecycleActive || pair.plaintextSqlType === null) return null - const hasValues = await columnHasValues( - client, - pair.table, - pair.plaintextColumn, - ) - return hasValues ? { kind: 'prepopulated' } : null -} - -/** - * Why no reverse migration may start on this pair yet, or `null`: the twin is - * the source column of a forward migration into the encrypted column that was - * never dropped. At most one column migration is open per column pair, so the - * forward one must be finished (`stash encrypt drop`) or abandoned first. - * Names and phases only, never a value. + * Why no reverse column migration may start on this pair yet, or `null`: the + * twin is the source column of a forward column migration into the encrypted + * column that was never dropped. At most one column migration is open per + * column pair (`ColumnMigration.blockedBy`), so the forward one must be + * finished (`stash encrypt drop`) or abandoned first. Names and phases only, + * never a value. */ export function openForwardMigrationMessage( - pair: Pick< - DecryptPair, - 'table' | 'encryptedColumn' | 'plaintextColumn' | 'forwardState' - >, + pair: Pick & { + migration: Pick + }, ): string | null { - const forward = pair.forwardState - if (forward === null || forward.phase === 'dropped') return null + const forward = pair.migration.blockedBy + if (forward === null) return null return `${pair.table}.${pair.plaintextColumn} is the source of a forward encryption into ${pair.encryptedColumn} that is still open at '${forward.phase}' in cipherstash.cs_migrations. A reverse migration cannot start while it is open. Finish it with \`stash encrypt drop\` first, or, to abandon encryption instead, stop writing ${pair.encryptedColumn}, switch reads back to ${pair.plaintextColumn} and drop ${pair.encryptedColumn}: while dual-writes are live, ${pair.plaintextColumn} is still current and there is nothing to decrypt.` } -/** The two ways past a {@link TwinHazard}, as `decrypt backfill` flags. */ +/** + * The two ways past a twin that already holds values, as `decrypt backfill` + * flags: a forced pass, or a waiver. + */ export const TWIN_HAZARD_FLAGS = { overwrite: '--force', keep: '--confirm-plaintext-current', @@ -306,7 +162,7 @@ export const TWIN_HAZARD_FLAGS = { /** * The refusal (in `decrypt backfill`) or warning (in `decrypt plan`) for a - * {@link TwinHazard}. Names and phases only, never a value. + * twin that already holds values. Names and phases only, never a value. */ export function twinHazardMessage( pair: Pick, @@ -314,31 +170,10 @@ export function twinHazardMessage( const twin = `${pair.table}.${pair.plaintextColumn}` const source = `${pair.table}.${pair.encryptedColumn}` return [ - `${twin} already holds values, and no reverse migration is in progress for ${source}. Nothing records where they came from or whether they are current.`, - `The reverse backfill only fills rows where ${pair.plaintextColumn} is NULL, so it would keep those values, mark the column decrypted, and \`stash decrypt drop\` would then drop ${pair.encryptedColumn}: the only current copy.`, + `${twin} already holds values, and no reverse column migration is in progress for ${source}. Nothing records where they came from or whether they are current.`, + `Kept as they are, stale values would survive the backfill, which fills only rows where ${pair.plaintextColumn} is NULL, and \`stash decrypt drop\` would then drop ${pair.encryptedColumn}: the only current copy.`, 'Choose one, once the application writes BOTH columns on every insert and update:', ` ${TWIN_HAZARD_FLAGS.overwrite} overwrite every row of ${pair.plaintextColumn} with the decrypted ciphertext (the safe choice when in doubt).`, ` ${TWIN_HAZARD_FLAGS.keep} keep the existing values. Only when the application has written ${pair.plaintextColumn} on every insert and update for the whole time ${pair.encryptedColumn} existed.`, ].join('\n') } - -/** - * The forward manifest's plaintext name for this encrypted column: the entry - * whose recorded `encryptedColumn` is it, or — for entries written before that - * field existed — whose `_encrypted` convention names it. - */ -export async function manifestPlaintextFor( - table: string, - encryptedColumn: string, -): Promise { - const manifest = await readManifest() - const entries = manifest?.tables[table] ?? [] - const entry = - entries.find((e) => e.encryptedColumn === encryptedColumn) ?? - entries.find( - (e) => - e.encryptedColumn === undefined && - `${e.column}_encrypted` === encryptedColumn, - ) - return entry?.column -} diff --git a/languages/typescript/packages/cli/src/commands/decrypt/plan.ts b/languages/typescript/packages/cli/src/commands/decrypt/plan.ts index 3ef9cc43f..f3bd3203f 100644 --- a/languages/typescript/packages/cli/src/commands/decrypt/plan.ts +++ b/languages/typescript/packages/cli/src/commands/decrypt/plan.ts @@ -1,6 +1,7 @@ import { - appendEvent, + ColumnMigration, classifyEqlDomain, + DIRECTION_VOCABULARY, listEncryptedColumns, type PlaintextSqlType, qualifyTable, @@ -19,12 +20,10 @@ import { loadStashConfig } from '@/config/index.js' import { createPgClient } from '@/db/client.js' import { drizzleColumnLine, drizzleImports } from './lib/orm-hints.js' import { - assessExistingTwin, type DecryptPair, openForwardMigrationMessage, resolveDecryptPair, TWIN_HAZARD_FLAGS, - type TwinHazard, twinHazardMessage, } from './lib/resolve-pair.js' @@ -75,7 +74,7 @@ export interface DecryptPlanOptions { * A twin that already exists is checked the way `decrypt backfill`'s first * run checks it: when it is the source of a forward encryption that was never * dropped ({@link openForwardMigrationMessage}), or holds values - * ({@link assessExistingTwin}), plan warns with the same text the backfill + * (`ColumnMigration.existingTwinValues`), plan warns with the same text the backfill * refuses with. Plan writes nothing for such a twin, so it warns rather than * fails. */ @@ -131,7 +130,7 @@ export async function decryptPlanCommand(options: DecryptPlanOptions = {}) { } const existing = pairs.filter((pair) => pair.plaintextSqlType !== null) - const hazards = new Map() + const hazards = new Set() for (const pair of existing) { const openForward = openForwardMigrationMessage(pair) if (openForward !== null) { @@ -140,14 +139,13 @@ export async function decryptPlanCommand(options: DecryptPlanOptions = {}) { ) continue } - const hazard = await assessExistingTwin(client, pair) - if (hazard === null) { + if (!(await pair.migration.existingTwinValues())) { p.log.info( `${options.table}.${pair.plaintextColumn} already exists (${pair.plaintextSqlType}) — nothing to add for ${pair.encryptedColumn}.`, ) continue } - hazards.set(pair, hazard) + hazards.add(pair) p.log.warn( `${options.table}.${pair.plaintextColumn} already exists (${pair.plaintextSqlType}), but it cannot be trusted as the plaintext twin of ${pair.encryptedColumn} as it stands. \`stash decrypt backfill\` will refuse it until you choose:\n${twinHazardMessage(pair)}`, ) @@ -165,21 +163,24 @@ export async function decryptPlanCommand(options: DecryptPlanOptions = {}) { // dropped, which `decrypt backfill` refuses. So the forward lifecycle is // closed here, recorded as observed rather than as a drop stash generated. for (const pair of toAdd) { - const openForward = openForwardMigrationMessage(pair) - if (openForward === null || pair.forwardState === null) continue - const what = `${options.table}.${pair.plaintextColumn} does not exist, but the forward encryption into ${pair.encryptedColumn} is still open at '${pair.forwardState.phase}' in cipherstash.cs_migrations: the column was removed outside stash.` + const forward = pair.migration.blockedBy + if (forward === null) continue + const what = `${options.table}.${pair.plaintextColumn} does not exist, but the forward encryption into ${pair.encryptedColumn} is still open at '${forward.phase}' in cipherstash.cs_migrations: the column was removed outside stash.` if (options.dryRun) { p.log.info( `${what} Without --dry-run, plan would record that forward migration as dropped so \`stash decrypt backfill\` can start.`, ) continue } - await appendEvent(client, { - tableName: options.table, - columnName: pair.plaintextColumn, - event: 'dropped', - phase: 'dropped', - details: { observedAbsent: true, recordedBy: 'stash decrypt plan' }, + const forwardMigration = await ColumnMigration.load(client, { + table: options.table, + column: pair.plaintextColumn, + direction: 'forward', + twin: pair.encryptedColumn, + manifest: null, + }) + await forwardMigration.recordSourceRemoved({ + recordedBy: 'stash decrypt plan', }) p.log.info(`${what} Recorded it as dropped.`) } @@ -313,7 +314,7 @@ function describeVia(pair: DecryptPair): string { function nextSteps( table: string, pairs: readonly DecryptPair[], - hazards: ReadonlyMap = new Map(), + hazards: ReadonlySet = new Set(), ): string { const runner = (command: string) => runnerCommand(detectPackageManager(), command) @@ -369,15 +370,16 @@ async function renderOverview( continue } const target = `${pair.plaintextColumn} ${pair.plaintextSqlType ?? pair.mapping?.sqlType ?? '(unknown type)'}` - // An `encrypted-dropped` from an earlier, finished lifecycle is history, + // `phase` is null for a finished, earlier column migration: history, // not where this column stands now. - const phase = pair.lifecycleActive ? pair.state?.phase : undefined + const { migration } = pair + const phase = migration.phase ?? undefined - if (phase === 'encrypted-dropped') { + if (migration.dropGenerated) { lines.push( ` ✓ ${key}: encrypted-dropped (apply the generated drop migration)`, ) - } else if (phase === 'decrypted') { + } else if (phase === DIRECTION_VOCABULARY.reverse.completedPhase) { lines.push( ` → ${key} → ${pair.plaintextColumn}: decrypted → switch reads to ${pair.plaintextColumn}, then ${runner(`stash decrypt drop --table ${table} --column ${column}`)}`, ) @@ -385,17 +387,14 @@ async function renderOverview( lines.push( ` → ${key} → ${pair.plaintextColumn}: ${phase} → ${runner(`stash decrypt backfill --table ${table} --column ${column}`)}`, ) - } else if ( - pair.forwardState !== null && - pair.forwardState.phase !== 'dropped' - ) { + } else if (migration.blockedBy !== null) { lines.push( - ` ! ${key} → ${target}: forward encryption still open at '${pair.forwardState.phase}' — finish or abandon it first, see ${runner(`stash decrypt plan --table ${table} --column ${column}`)}`, + ` ! ${key} → ${target}: forward encryption still open at '${migration.blockedBy.phase}' — finish or abandon it first, see ${runner(`stash decrypt plan --table ${table} --column ${column}`)}`, ) } else if (pair.plaintextSqlType !== null) { - const hazard = await assessExistingTwin(client, pair) + const hasValues = await migration.existingTwinValues() lines.push( - hazard === null + !hasValues ? ` → ${key} → ${target}: twin exists → ${runner(`stash decrypt backfill --table ${table} --column ${column}`)}` : ` ! ${key} → ${target}: twin already holds values; the backfill needs ${TWIN_HAZARD_FLAGS.overwrite} or ${TWIN_HAZARD_FLAGS.keep} — see ${runner(`stash decrypt plan --table ${table} --column ${column}`)}`, ) diff --git a/languages/typescript/packages/cli/src/commands/decrypt/status.ts b/languages/typescript/packages/cli/src/commands/decrypt/status.ts index 74ce3985e..efac456e7 100644 --- a/languages/typescript/packages/cli/src/commands/decrypt/status.ts +++ b/languages/typescript/packages/cli/src/commands/decrypt/status.ts @@ -1,10 +1,11 @@ import { + ColumnMigration, + type ColumnMigrationState, classifyEqlDomain, - isReverseMigrationPhase, - type MigrationStateRow, - plaintextTypeForEqlDomain, + passProgress, } from '@cipherstash/migrate' import * as p from '@clack/prompts' +import { BackfillConfigError } from '@/commands/encrypt/backfill.js' import { fetchPhysicalColumns, latestByColumnSafe, @@ -14,10 +15,10 @@ import { detectPackageManager, runnerCommand } from '@/commands/init/utils.js' import { loadStashConfig } from '@/config/index.js' import { createPgClient } from '@/db/client.js' import { - derivePlaintextName, - isActiveReverseLifecycle, - manifestPlaintextFor, - recordedPlaintextColumn, + type DecryptPair, + openForwardMigrationMessage, + PlaintextColumnIsEqlError, + resolveDecryptPair, } from './lib/resolve-pair.js' interface Row { @@ -45,21 +46,18 @@ const STATUS_HEADERS: Row = { /** * CLI handler for `stash decrypt status` — the reverse twin of * `stash encrypt status`. One row per EQL v3 column in the current schema, - * plus any column whose reverse lifecycle is on record but whose encrypted - * column is already gone, merging: + * plus any column whose reverse column migration is on record but whose + * encrypted column is already gone. * - * - `information_schema.columns` — which columns are encrypted, their domain, - * and whether the plaintext twin exists yet; - * - the latest REVERSE event per encrypted column in `cs_migrations` — phase, - * progress, and the count of values the backfill could not decrypt. + * Each row comes from the same `resolveDecryptPair` (and so the same + * `ColumnMigration`) that `decrypt plan`, `backfill` and `drop` use, so the + * commands never show a different twin, phase or warning for one column. A + * twin that is itself an EQL column, which `plan` and `backfill` refuse, is + * flagged `plaintext-col-is-eql`; one they would refuse or warn about is + * flagged `forward-open` or `twin-has-values`. * - * The twin name comes from the same rules `decrypt plan` and `decrypt - * backfill` use (recorded pairing of the lifecycle in progress, then the - * manifest, then {@link derivePlaintextName}), so the three commands never - * show different names for one column. A twin that is itself an EQL column, - * which `plan` and `backfill` refuse, is flagged `plaintext-col-is-eql`. - * - * Read-only, and runs no per-row coverage queries (as `encrypt status`). + * Read-only. Like `encrypt status` it runs no coverage counts; the only + * per-row probe is whether an existing twin holds any value (`EXISTS`). */ export async function decryptStatusCommand() { p.intro(runnerCommand(detectPackageManager(), 'stash decrypt status')) @@ -82,58 +80,33 @@ export async function decryptStatusCommand() { for (const [table, columns] of physicalCols) { for (const [column, domain] of columns) { if (domain === null || classifyEqlDomain(domain) !== 3) continue - const key = `${table}.${column}` - seen.add(key) - const latest = reverseOnly(stateMap.get(`${table}.${column}`)) - const recordedTwin = recordedPlaintextColumn(latest) - const active = isActiveReverseLifecycle( - latest, - recordedTwin === undefined - ? null - : forwardOnly(stateMap.get(`${table}.${recordedTwin}`)), - ) - // A finished lifecycle's events are history: this column starts over. - const state = active ? latest : null - const plaintext = - (active ? recordedTwin : undefined) ?? - (await manifestPlaintextFor(table, column)) ?? - derivePlaintextName(column).name - const twinDomain = columns.get(plaintext) - rows.push( - renderRow({ - table, - column, - domain, - plaintext, - state, - plaintextExists: columns.has(plaintext), - plaintextIsEql: - twinDomain !== undefined && - twinDomain !== null && - classifyEqlDomain(twinDomain) !== null, - mappedType: plaintextTypeForEqlDomain(domain)?.sqlType, - }), - ) + seen.add(`${table}.${column}`) + rows.push(await currentRow(client, table, column, domain)) } } - // Reverse lifecycles whose encrypted column is no longer visible — most - // often `encrypted-dropped` after the migration was applied. - for (const [key, row] of stateMap) { + // Reverse column migrations whose encrypted column is no longer visible + // — most often a generated drop that was applied. + for (const key of stateMap.keys()) { if (seen.has(key)) continue - const state = reverseOnly(row) - if (!state) continue const lastDot = key.lastIndexOf('.') const table = key.slice(0, lastDot) const column = key.slice(lastDot + 1) - const plaintext = recordedPlaintextColumn(state) ?? '—' + const migration = await ColumnMigration.load(client, { + table, + column, + direction: 'reverse', + manifest: null, + }) + if (migration.latest === null) continue + const plaintext = migration.recordedTwin ?? '—' rows.push( renderRow({ table, column, domain: '—', plaintext, - state, + migration, plaintextExists: physicalCols.get(table)?.has(plaintext) ?? false, encryptedGone: true, }), @@ -161,16 +134,44 @@ export async function decryptStatusCommand() { if (exitCode) process.exit(exitCode) } -function reverseOnly( - row: MigrationStateRow | undefined, -): MigrationStateRow | null { - return row && isReverseMigrationPhase(row.phase) ? row : null -} - -function forwardOnly( - row: MigrationStateRow | undefined, -): MigrationStateRow | null { - return row && !isReverseMigrationPhase(row.phase) ? row : null +/** The row for an EQL v3 column that exists, from its resolved pair. */ +async function currentRow( + client: ReturnType, + table: string, + column: string, + domain: string, +): Promise { + let pair: DecryptPair + try { + pair = await resolveDecryptPair(client, { table, encryptedColumn: column }) + } catch (error) { + if (!(error instanceof BackfillConfigError)) throw error + const migration = await ColumnMigration.load(client, { + table, + column, + direction: 'reverse', + }) + return renderRow({ + table, + column, + domain, + plaintext: migration.twin.column, + migration, + plaintextExists: true, + plaintextIsEql: error instanceof PlaintextColumnIsEqlError, + }) + } + return renderRow({ + table, + column, + domain, + plaintext: pair.plaintextColumn, + migration: pair.migration, + plaintextExists: pair.plaintextSqlType !== null, + mappedType: pair.mapping?.sqlType, + forwardOpen: openForwardMigrationMessage(pair) !== null, + twinHasValues: await pair.migration.existingTwinValues(), + }) } function renderRow(input: { @@ -178,7 +179,7 @@ function renderRow(input: { column: string domain: string plaintext: string - state: MigrationStateRow | null + migration: ColumnMigrationState plaintextExists: boolean /** The twin's name is taken by an EQL column, so it cannot hold plaintext. */ plaintextIsEql?: boolean @@ -186,22 +187,30 @@ function renderRow(input: { mappedType?: string /** The encrypted column is no longer in the catalog. */ encryptedGone?: boolean + /** The twin is the source of a forward encryption not yet dropped. */ + forwardOpen?: boolean + /** The twin already holds values no reverse migration wrote. */ + twinHasValues?: boolean }): Row { - const { state } = input + const { migration } = input const flags: string[] = [] if (input.plaintextIsEql) { flags.push('plaintext-col-is-eql') } else if (!input.plaintextExists && input.plaintext !== '—') { flags.push('plaintext-col-missing') } - if (input.encryptedGone && state?.phase !== 'encrypted-dropped') { + if (input.encryptedGone && !migration.dropGenerated) { flags.push('encrypted-col-missing') } - if (!input.encryptedGone && input.mappedType === undefined) { + if (!input.encryptedGone && !input.plaintextIsEql && !input.mappedType) { flags.push('unmapped-domain') } + if (input.forwardOpen) flags.push('forward-open') + if (input.twinHasValues) flags.push('twin-has-values') + if (migration.obligations.length > 0) flags.push('forced-pass-owed') - const skipped = skippedCount(state) + const progress = passProgress(migration) + const skipped = progress?.rowsSkipped ?? null const plaintext = input.plaintextExists || input.plaintext === '—' ? input.plaintext @@ -212,46 +221,28 @@ function renderRow(input: { column: input.column, domain: input.domain, plaintext, - phase: state?.phase ?? '—', - progress: formatProgress(state), + phase: migration.phase ?? '—', + progress: formatProgress(migration), skipped: skipped === null ? '—' : skipped.toLocaleString(), flags: flags.join(', '), } } -/** Cumulative undecryptable count recorded on the latest reverse event. */ -function skippedCount(state: MigrationStateRow | null): number | null { - const value = state?.details?.rowsSkipped - return typeof value === 'number' && Number.isFinite(value) ? value : null -} - /** - * Phase-aware PROGRESS framing, matching `encrypt status`: - * - * decrypt-dual-writing — gate passed, no backfill yet. - * decrypting — rowsProcessed/rowsTotal (an `error` event keeps - * this phase, so a failed run shows where it stopped). - * decrypted — complete; not a degenerate `0/0` ratio. - * encrypted-dropped — drop migration generated. + * PROGRESS framing, matching `encrypt status`: awaiting backfill once + * dual-writes are confirmed, `rowsProcessed/rowsTotal` while a pass runs (a + * failed run shows where it stopped), complete, then dropped. */ -function formatProgress(state: MigrationStateRow | null): string { - switch (state?.phase) { - case 'decrypt-dual-writing': - return '(awaiting backfill)' - case 'decrypting': { - if (state.rowsTotal === null) return '—' - const processed = state.rowsProcessed ?? 0 - const pct = - state.rowsTotal > 0 - ? Math.floor((processed / state.rowsTotal) * 100) - : 100 - return `${processed}/${state.rowsTotal} (${pct}%)${state.event === 'error' ? ' (last run failed)' : ''}` - } - case 'decrypted': - return '(backfill complete)' - case 'encrypted-dropped': - return '(dropped)' - default: - return '—' - } +function formatProgress(migration: ColumnMigrationState): string { + if (migration.phase === null) return '—' + if (migration.dropGenerated) return '(dropped)' + const progress = passProgress(migration) + if (progress === null) return '(awaiting backfill)' + if (progress.completed) return '(backfill complete)' + if (progress.rowsTotal === null) return '—' + const pct = + progress.rowsTotal > 0 + ? Math.floor((progress.rowsProcessed / progress.rowsTotal) * 100) + : 100 + return `${progress.rowsProcessed}/${progress.rowsTotal} (${pct}%)${progress.failed ? ' (last run failed)' : ''}` } diff --git a/languages/typescript/packages/cli/src/commands/encrypt/status.ts b/languages/typescript/packages/cli/src/commands/encrypt/status.ts index b53d813a8..b9218d25b 100644 --- a/languages/typescript/packages/cli/src/commands/encrypt/status.ts +++ b/languages/typescript/packages/cli/src/commands/encrypt/status.ts @@ -1,6 +1,9 @@ import { + type ColumnKey, classifyEqlDomain, + deriveColumnMigration, type MigrationPhase, + type MigrationStateRow, readManifest, } from '@cipherstash/migrate' import * as p from '@clack/prompts' @@ -15,6 +18,15 @@ import { } from './lib/db-readers.js' import { formatTable } from './lib/table-format.js' +/** The latest event under `key`, as the history `deriveColumnMigration` reads. */ +function latestOf( + stateMap: ReadonlyMap, + key: ColumnKey, +): MigrationStateRow[] { + const row = stateMap.get(key) + return row === undefined ? [] : [row] +} + interface Row { table: string column: string @@ -77,9 +89,10 @@ export async function statusCommand() { // `stash decrypt drop` records this against the ENCRYPTED // column. The manifest entry stays, and `reversed` is its final // state: the CLI owns `.cipherstash/`. - reversed: - stateMap.get(`${tableName}.${encryptedName}`)?.phase === - 'encrypted-dropped', + reversed: deriveColumnMigration({ + direction: 'reverse', + history: latestOf(stateMap, `${tableName}.${encryptedName}`), + }).dropGenerated, }), ) } diff --git a/languages/typescript/packages/migrate/src/__tests__/column-migration.test.ts b/languages/typescript/packages/migrate/src/__tests__/column-migration.test.ts index 78d9433fc..b009301b0 100644 --- a/languages/typescript/packages/migrate/src/__tests__/column-migration.test.ts +++ b/languages/typescript/packages/migrate/src/__tests__/column-migration.test.ts @@ -8,9 +8,13 @@ import { describe, expect, it } from 'vitest' import { type ColumnMigrationState, deriveColumnMigration, + deriveTwinName, + manifestTwin, openCounterpart, + passProgress, planPassFor, } from '../column-migration.js' +import type { Manifest } from '../manifest.js' import type { MigrationEvent, MigrationPhase, @@ -421,3 +425,147 @@ describe('planPassFor', () => { ).toBeNull() }) }) + +describe('passProgress', () => { + const progressOf = (history: MigrationStateRow[]) => + passProgress(deriveColumnMigration({ direction: 'reverse', history })) + + it.each([ + { name: 'nothing on record', history: () => [], expected: null }, + { + name: 'awaiting the first run', + history: () => [confirm()], + expected: null, + }, + { + name: 'mid-pass, with the undecryptable rows skipped so far', + history: () => [ + confirm(), + ev( + 'decrypt_checkpoint', + 'decrypting', + { ...unforced, rowsSkipped: 2 }, + { rowsProcessed: 40, rowsTotal: 100 }, + ), + ], + expected: { + rowsProcessed: 40, + rowsTotal: 100, + rowsSkipped: 2, + failed: false, + completed: false, + }, + }, + { + name: 'a failed run shows where it stopped', + history: () => [ + confirm(), + ev('error', 'decrypting', unforced, { + rowsProcessed: 10, + rowsTotal: 100, + }), + ], + expected: { + rowsProcessed: 10, + rowsTotal: 100, + rowsSkipped: null, + failed: true, + completed: false, + }, + }, + { + name: 'complete', + history: () => [ + confirm(), + ev('decrypted', 'decrypted', unforced, { + rowsProcessed: 100, + rowsTotal: 100, + }), + ], + expected: { + rowsProcessed: 100, + rowsTotal: 100, + rowsSkipped: null, + failed: false, + completed: true, + }, + }, + { + name: 'a generated drop is past the pass', + history: () => [confirm(), completed(unforced), dropGenerated()], + expected: null, + }, + ])('$name', ({ history, expected }) => { + expect(progressOf(history())).toEqual(expected) + }) +}) + +describe('deriveTwinName', () => { + it.each([ + { + direction: 'reverse', + column: 'email_encrypted', + twin: 'email', + via: 'convention', + }, + { + direction: 'reverse', + column: '_encrypted', + twin: '_encrypted_plaintext', + via: 'fallback', + }, + { + direction: 'reverse', + column: 'email_ct', + twin: 'email_ct_plaintext', + via: 'fallback', + }, + { + direction: 'forward', + column: 'email', + twin: 'email_encrypted', + via: 'convention', + }, + ] as const)( + '$direction $column → $twin', + ({ direction, column, twin, via }) => { + expect(deriveTwinName(direction, column)).toEqual({ column: twin, via }) + }, + ) +}) + +describe('manifestTwin', () => { + const entry = (column: string, encryptedColumn?: string) => ({ + column, + encryptedColumn, + castAs: 'text' as const, + indexes: [], + targetPhase: 'cut-over' as const, + }) + const manifest: Manifest = { + version: 1, + tables: { + users: [entry('contact', 'email_encrypted'), entry('phone')], + }, + } + + it.each([ + { direction: 'reverse', column: 'email_encrypted', twin: 'contact' }, + // An entry from before `encryptedColumn` was recorded: the convention. + { direction: 'reverse', column: 'phone_encrypted', twin: 'phone' }, + { direction: 'reverse', column: 'contact_encrypted', twin: undefined }, + { direction: 'forward', column: 'contact', twin: 'email_encrypted' }, + { direction: 'forward', column: 'phone', twin: undefined }, + ] as const)('$direction $column → $twin', ({ direction, column, twin }) => { + expect(manifestTwin(direction, manifest, 'users', column)).toBe(twin) + }) + + it('no manifest, or no entry for the table, names nothing', () => { + expect( + manifestTwin('reverse', null, 'users', 'email_encrypted'), + ).toBeUndefined() + expect( + manifestTwin('reverse', manifest, 'orders', 'email_encrypted'), + ).toBeUndefined() + }) +}) diff --git a/languages/typescript/packages/migrate/src/column-migration.ts b/languages/typescript/packages/migrate/src/column-migration.ts index 97269ef82..4eae7f797 100644 --- a/languages/typescript/packages/migrate/src/column-migration.ts +++ b/languages/typescript/packages/migrate/src/column-migration.ts @@ -126,6 +126,8 @@ export interface ColumnMigrationState { phase: MigrationPhase | null /** `phase` is set and the drop has not been generated. */ open: boolean + /** The drop of the source column has been generated (`phase` is the last). */ + dropGenerated: boolean /** The twin the current column migration's events name, if they do. */ recordedTwin: string | undefined /** At most one: there is only one kind of obligation. */ @@ -161,6 +163,7 @@ export function deriveColumnMigration(input: { latest, phase: null, open: false, + dropGenerated: false, recordedTwin: undefined, obligations: [], } @@ -213,6 +216,7 @@ export function deriveColumnMigration(input: { latest, phase: latest.phase, open: latest.phase !== vocab.droppedPhase, + dropGenerated: latest.phase === vocab.droppedPhase, recordedTwin, obligations: obligation === null ? [] : [obligation], } @@ -235,6 +239,41 @@ export function openCounterpart(input: { return { direction, column: input.twin, phase: latest.phase } } +/** Where the latest backfill pass stands, from its latest event. */ +export interface PassProgress { + rowsProcessed: number + /** `null` before a pass has counted its rows. */ + rowsTotal: number | null + /** Cumulative rows the pass visited but could not write. */ + rowsSkipped: number | null + /** The latest run failed (an `error` event); a re-run resumes it. */ + failed: boolean + /** The pass completed. */ + completed: boolean +} + +/** + * The latest pass's progress, or `null` when no pass has run in the column + * migration in progress. + */ +export function passProgress(state: ColumnMigrationState): PassProgress | null { + const { latest } = state + if (state.phase === null || latest === null) return null + const vocab = DIRECTION_VOCABULARY[state.direction] + if (latest.phase !== vocab.runningPhase && latest.event !== vocab.completed) { + return null + } + const skipped = latest.details?.rowsSkipped + return { + rowsProcessed: latest.rowsProcessed ?? 0, + rowsTotal: latest.rowsTotal, + rowsSkipped: + typeof skipped === 'number' && Number.isFinite(skipped) ? skipped : null, + failed: latest.event === 'error', + completed: latest.event === vocab.completed, + } +} + /** How the next run will go: its mode, why, and where it resumes. */ export interface PassPlan { force: boolean @@ -346,6 +385,7 @@ export class ColumnMigration implements ColumnMigrationState { readonly latest: MigrationStateRow | null readonly phase: MigrationPhase | null readonly open: boolean + readonly dropGenerated: boolean readonly recordedTwin: string | undefined readonly obligations: readonly Obligation[] @@ -364,6 +404,7 @@ export class ColumnMigration implements ColumnMigrationState { this.latest = state.latest this.phase = state.phase this.open = state.open + this.dropGenerated = state.dropGenerated this.recordedTwin = state.recordedTwin this.obligations = state.obligations } diff --git a/languages/typescript/packages/migrate/src/index.ts b/languages/typescript/packages/migrate/src/index.ts index 7af29d545..ddda340b7 100644 --- a/languages/typescript/packages/migrate/src/index.ts +++ b/languages/typescript/packages/migrate/src/index.ts @@ -53,6 +53,8 @@ export { type OpenCounterpart, openCounterpart, type PassPlan, + type PassProgress, + passProgress, planPassFor, type TwinSource, type Waiver, From bde5b7390afd1d3ba4e1ce33a1d82588e1c596d0 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 19 Oct 2026 01:45:37 +0000 Subject: [PATCH 03/15] docs(skills): describe the forced-pass and waiver rules ColumnMigration enforces Adds the changeset for the ColumnMigration API and the engine changes. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01Scub1tpuegHHCTXrgn7SHf --- .changeset/column-migration-api.md | 10 ++++++++++ skills/stash-cli/SKILL.md | 10 +++++----- 2 files changed, 15 insertions(+), 5 deletions(-) create mode 100644 .changeset/column-migration-api.md diff --git a/.changeset/column-migration-api.md b/.changeset/column-migration-api.md new file mode 100644 index 000000000..85dc7aec4 --- /dev/null +++ b/.changeset/column-migration-api.md @@ -0,0 +1,10 @@ +--- +'@cipherstash/migrate': minor +'stash': patch +--- + +`@cipherstash/migrate` adds `ColumnMigration`, one place for the rules of a column migration in either direction: which twin column it pairs with (the flag, the recorded pairing, the manifest, then the naming convention), its phase, whether a forced pass is owed, and which open migration in the other direction blocks it. `ColumnMigration.load()` reads it from `cipherstash.cs_migrations`; `confirmDualWrites()`, `recordDropGenerated()` and `recordSourceRemoved()` record events, refusing those the rules forbid with a `ColumnMigrationError`. The pure rules are exported as `deriveColumnMigration`, `openCounterpart`, `planPassFor`, `passProgress`, `deriveTwinName` and `manifestTwin`. + +`runBackfill` and `runDecryptBackfill` now apply the rules themselves rather than leaving them to the caller. Every event records whether its pass was forced. An interrupted forced pass is finished as one by the next run, with or without `force`, and the result says why in `forcedBy`. A forward backfill resumes after a failed run from its last committed position, counts its total once when it resumes a forced pass, and refuses to start while a reverse migration of the same columns is open. A reverse backfill over a twin that already holds values is forced unless the caller confirmed dual-writes with the `plaintext-current` waiver. Error events store the error's class (`errorName`) and safe details instead of its message, which could carry row data. + +`stash decrypt plan`, `status`, `backfill` and `drop` resolve every column through `ColumnMigration`, so they always agree on the twin and the phase. `decrypt status` flags `forward-open`, `twin-has-values` and `forced-pass-owed`. `decrypt backfill --confirm-plaintext-current` is accepted on a later run as long as no forced pass has started, and refused once one has. diff --git a/skills/stash-cli/SKILL.md b/skills/stash-cli/SKILL.md index db86efccd..19c7bcfe7 100644 --- a/skills/stash-cli/SKILL.md +++ b/skills/stash-cli/SKILL.md @@ -613,7 +613,7 @@ stash encrypt backfill --table users --column email stash encrypt backfill --table users --column email --chunk-size 5000 ``` -Chunked, resumable, idempotent. Walks the table in keyset-pagination order, encrypts each chunk via `bulkEncryptModels`, and writes one `UPDATE ... FROM (VALUES ...)` per chunk in a transaction that also checkpoints to `cs_migrations`. SIGINT/SIGTERM finishes the current chunk and exits cleanly; re-running resumes. The ` IS NOT NULL AND _encrypted IS NULL` guard makes concurrent runners and re-runs converge. +Chunked, resumable, idempotent. Walks the table in keyset-pagination order, encrypts each chunk via `bulkEncryptModels`, and writes one `UPDATE ... FROM (VALUES ...)` per chunk in a transaction that also checkpoints to `cs_migrations`. SIGINT/SIGTERM finishes the current chunk and exits cleanly; re-running resumes, from the last checkpoint or, after a failed run, from the last committed position. A failure is recorded in `cs_migrations` by error class and safe details only, never the error message, which can carry row data. The ` IS NOT NULL AND _encrypted IS NULL` guard makes concurrent runners and re-runs converge. Backfill requires a `public.eql_v3_*` target column, records version 3 and the `dropped` target phase in `.cipherstash/migrations.json`, then prints the next steps: switch the application to the encrypted column by name and run `stash encrypt drop`. A missing, plaintext, or legacy v2 target is rejected before encryption begins. @@ -631,7 +631,7 @@ Backfill requires a `public.eql_v3_*` target column, records version 3 and the ` | `--encrypted-column ` | Override the `_encrypted` target name | | `--schema-column-key ` | Override the schema lookup key | | `--confirm-dual-writes-deployed` | Non-interactive equivalent of the prompt | -| `--force` | Re-encrypt every plaintext row, including ones with existing ciphertext. Recovery path for drift. Expensive, not destructive. Flagged in the audit trail. | +| `--force` | Re-encrypt every plaintext row, including ones with existing ciphertext. Recovery path for drift. Expensive, not destructive. Flagged in the audit trail. An interrupted `--force` run is finished as one: the next run is forced, with or without the flag, until a forced pass completes. | #### `encrypt drop` @@ -655,7 +655,7 @@ The reverse lifecycle: moving encrypted columns back to plaintext, for leaving C #### `decrypt status` / `decrypt plan` -`status` lists every EQL v3 column in the current schema plus any reverse lifecycle on record, with the plaintext twin, phase, progress, and the count of values the backfill could not decrypt (`UNDECRYPTABLE`). A twin name already taken by an EQL column is flagged `plaintext-col-is-eql`. Read-only. +`status` lists every EQL v3 column in the current schema plus any reverse lifecycle on record, with the plaintext twin, phase, progress, and the count of values the backfill could not decrypt (`UNDECRYPTABLE`). Its `FLAGS` column names what the other commands would refuse or warn about, from the same rules: `plaintext-col-is-eql` (the twin name is taken by an EQL column), `forward-open` (the twin is the source of a forward encryption not yet dropped), `twin-has-values` (the twin holds values and no reverse migration has started), `forced-pass-owed` (every backfill run is forced until one completes, see below). Read-only. `plan` with no flags is the read-only overview: each encrypted column, its proposed twin and type, and the next command. With `--table` (optionally `--column`, `--plaintext-column`) it generates the change that adds the twins, **nullable**: @@ -684,7 +684,7 @@ Same engine as `encrypt backfill`: keyset pagination, one transaction per chunk - `--force` overwrites every row of the twin with the decrypted ciphertext. Use this when in doubt. - `--confirm-plaintext-current` keeps the existing values and fills only the NULLs. Use it only when the application has written the plaintext column on every insert and update for the whole time the encrypted column existed. -The choice is recorded in `cs_migrations`. Later runs of the same reverse migration do not ask again: the twin's values are then its own. Choosing `--force` makes a forced pass owed from that moment, and so does starting any `--force` run: every later run is forced, with or without `--force`, until one completes, so an interrupted `--force` run is always finished as one, even if it failed before writing anything. A `--force` run after an interrupted plain one starts again from the first row. +The choice is recorded in `cs_migrations`. Later runs of the same reverse migration do not ask again: the twin's values are then its own. Choosing `--force` makes a forced pass owed from that moment, and so does starting any `--force` run: every later run is forced, with or without `--force`, until one completes, so an interrupted `--force` run is always finished as one, even if it failed before writing anything. The run says when it is forced for this reason. `--confirm-plaintext-current` is accepted on a later run too, as long as no forced pass has started; once one has, it is refused, because rows already overwritten cannot be un-overwritten. A `--force` run after an interrupted plain one starts again from the first row. **Lock contexts — known limitation, no bypass.** A value encrypted with an end-user lock context can only be decrypted as that user (see `stash-auth`). With workspace credentials it fails *on its own row* (`bulkDecrypt` reports per row), so the backfill **skips it, counts it, leaves the twin NULL** (a `--force` pass sets a skipped row's twin to NULL, so a stale value is never left behind), and reports the count and error codes; the count is also recorded in `cs_migrations` and shown by `decrypt status`. Decrypt those rows in your application on each user's behalf (writing the plaintext column), then re-run without `--force`. Do this only after the last forced pass has completed: a forced pass (including a plain run that finishes an interrupted one) sets the twin back to NULL on every row it cannot decrypt, an application-written value included. `decrypt drop` refuses while any remain. @@ -702,7 +702,7 @@ The choice is recorded in `cs_migrations`. Later runs of the same reverse migrat | `--pk-column ` | Override primary-key detection | | `--confirm-dual-writes-deployed` | Non-interactive equivalent of the prompt: the app writes the encrypted column **and** its plaintext twin on every insert and update | | `--force` | Re-decrypt rows whose plaintext is already set and overwrite it: drift recovery, or replacing a twin that held values before the first run. Safe only while dual-writes are live: any row the application wrote to the plaintext column alone (a newer value, or one cleared on purpose) gets the older ciphertext's value back. | -| `--confirm-plaintext-current` | First run only: keep the values a twin already holds, asserting they are current (see above). On any other run it has no effect, and the backfill says so | +| `--confirm-plaintext-current` | Keep the values a twin already holds, asserting they are current (see above). Refused once a forced pass has started | | `--continue-on-undecryptable` | Do not stop when the first batch decrypts nothing | #### `decrypt drop` From e464c66f7f0dc2b577d3a7e53aaf012d3e909093 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 19 Oct 2026 01:47:43 +0000 Subject: [PATCH 04/15] refactor(cli): split decrypt status into row builders decryptStatusCommand and renderRow now sit under Biome's cognitive complexity limit of 15, from 20 and 16. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01Scub1tpuegHHCTXrgn7SHf --- .../cli/src/commands/decrypt/status.ts | 146 +++++++++++------- 1 file changed, 89 insertions(+), 57 deletions(-) diff --git a/languages/typescript/packages/cli/src/commands/decrypt/status.ts b/languages/typescript/packages/cli/src/commands/decrypt/status.ts index efac456e7..a61108515 100644 --- a/languages/typescript/packages/cli/src/commands/decrypt/status.ts +++ b/languages/typescript/packages/cli/src/commands/decrypt/status.ts @@ -74,44 +74,11 @@ export async function decryptStatusCommand() { fetchPhysicalColumns(client), ]) - const rows: Row[] = [] - const seen = new Set() - - for (const [table, columns] of physicalCols) { - for (const [column, domain] of columns) { - if (domain === null || classifyEqlDomain(domain) !== 3) continue - seen.add(`${table}.${column}`) - rows.push(await currentRow(client, table, column, domain)) - } - } - - // Reverse column migrations whose encrypted column is no longer visible - // — most often a generated drop that was applied. - for (const key of stateMap.keys()) { - if (seen.has(key)) continue - const lastDot = key.lastIndexOf('.') - const table = key.slice(0, lastDot) - const column = key.slice(lastDot + 1) - const migration = await ColumnMigration.load(client, { - table, - column, - direction: 'reverse', - manifest: null, - }) - if (migration.latest === null) continue - const plaintext = migration.recordedTwin ?? '—' - rows.push( - renderRow({ - table, - column, - domain: '—', - plaintext, - migration, - plaintextExists: physicalCols.get(table)?.has(plaintext) ?? false, - encryptedGone: true, - }), - ) - } + const current = await currentRows(client, physicalCols) + const rows = [ + ...current.rows, + ...(await goneRows(client, stateMap, current.seen, physicalCols)), + ] if (rows.length === 0) { p.log.info( @@ -134,6 +101,64 @@ export async function decryptStatusCommand() { if (exitCode) process.exit(exitCode) } +type PhysicalColumns = Awaited> + +/** One row per EQL v3 column in the current schema, and the keys shown. */ +async function currentRows( + client: ReturnType, + physicalCols: PhysicalColumns, +): Promise<{ rows: Row[]; seen: Set }> { + const rows: Row[] = [] + const seen = new Set() + for (const [table, columns] of physicalCols) { + for (const [column, domain] of columns) { + if (domain === null || classifyEqlDomain(domain) !== 3) continue + seen.add(`${table}.${column}`) + rows.push(await currentRow(client, table, column, domain)) + } + } + return { rows, seen } +} + +/** + * Reverse column migrations whose encrypted column is no longer visible — + * most often a generated drop that was applied. + */ +async function goneRows( + client: ReturnType, + stateMap: ReadonlyMap, + seen: ReadonlySet, + physicalCols: PhysicalColumns, +): Promise { + const rows: Row[] = [] + for (const key of stateMap.keys()) { + if (seen.has(key)) continue + const lastDot = key.lastIndexOf('.') + const table = key.slice(0, lastDot) + const column = key.slice(lastDot + 1) + const migration = await ColumnMigration.load(client, { + table, + column, + direction: 'reverse', + manifest: null, + }) + if (migration.latest === null) continue + const plaintext = migration.recordedTwin ?? '—' + rows.push( + renderRow({ + table, + column, + domain: '—', + plaintext, + migration, + plaintextExists: physicalCols.get(table)?.has(plaintext) ?? false, + encryptedGone: true, + }), + ) + } + return rows +} + /** The row for an EQL v3 column that exists, from its resolved pair. */ async function currentRow( client: ReturnType, @@ -174,7 +199,7 @@ async function currentRow( }) } -function renderRow(input: { +interface RowInput { table: string column: string domain: string @@ -191,24 +216,10 @@ function renderRow(input: { forwardOpen?: boolean /** The twin already holds values no reverse migration wrote. */ twinHasValues?: boolean -}): Row { - const { migration } = input - const flags: string[] = [] - if (input.plaintextIsEql) { - flags.push('plaintext-col-is-eql') - } else if (!input.plaintextExists && input.plaintext !== '—') { - flags.push('plaintext-col-missing') - } - if (input.encryptedGone && !migration.dropGenerated) { - flags.push('encrypted-col-missing') - } - if (!input.encryptedGone && !input.plaintextIsEql && !input.mappedType) { - flags.push('unmapped-domain') - } - if (input.forwardOpen) flags.push('forward-open') - if (input.twinHasValues) flags.push('twin-has-values') - if (migration.obligations.length > 0) flags.push('forced-pass-owed') +} +function renderRow(input: RowInput): Row { + const { migration } = input const progress = passProgress(migration) const skipped = progress?.rowsSkipped ?? null const plaintext = @@ -224,10 +235,31 @@ function renderRow(input: { phase: migration.phase ?? '—', progress: formatProgress(migration), skipped: skipped === null ? '—' : skipped.toLocaleString(), - flags: flags.join(', '), + flags: rowFlags(input).join(', '), } } +/** The FLAGS column: what the other commands would refuse or warn about. */ +function rowFlags(input: RowInput): string[] { + const { migration } = input + const flags: string[] = [] + if (input.plaintextIsEql) { + flags.push('plaintext-col-is-eql') + } else if (!input.plaintextExists && input.plaintext !== '—') { + flags.push('plaintext-col-missing') + } + if (input.encryptedGone && !migration.dropGenerated) { + flags.push('encrypted-col-missing') + } + if (!input.encryptedGone && !input.plaintextIsEql && !input.mappedType) { + flags.push('unmapped-domain') + } + if (input.forwardOpen) flags.push('forward-open') + if (input.twinHasValues) flags.push('twin-has-values') + if (migration.obligations.length > 0) flags.push('forced-pass-owed') + return flags +} + /** * PROGRESS framing, matching `encrypt status`: awaiting backfill once * dual-writes are confirmed, `rowsProcessed/rowsTotal` while a pass runs (a From 7928e22015ff1485561c1dfa3795be1fff635f47 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 19 Oct 2026 01:49:19 +0000 Subject: [PATCH 05/15] chore: address review of the ColumnMigration work Drops the engine's unused startedDetails option, routes quest's reverse phases through isReverseMigrationPhase, names the forced-pass ADR by its path, and describes the engine's rules in the stash-encryption library section. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01Scub1tpuegHHCTXrgn7SHf --- .../packages/cli/src/commands/status/quest.ts | 16 +++++++++------- .../packages/migrate/src/column-migration.ts | 4 +++- .../typescript/packages/migrate/src/decrypt.ts | 3 ++- .../typescript/packages/migrate/src/engine.ts | 3 --- skills/stash-encryption/SKILL.md | 2 +- 5 files changed, 15 insertions(+), 13 deletions(-) diff --git a/languages/typescript/packages/cli/src/commands/status/quest.ts b/languages/typescript/packages/cli/src/commands/status/quest.ts index af40ac419..ed049312d 100644 --- a/languages/typescript/packages/cli/src/commands/status/quest.ts +++ b/languages/typescript/packages/cli/src/commands/status/quest.ts @@ -1,4 +1,7 @@ -import type { MigrationPhase } from '@cipherstash/migrate' +import { + isReverseMigrationPhase, + type MigrationPhase, +} from '@cipherstash/migrate' /** Status of one objective inside a quest. */ export type ObjectiveStatus = 'done' | 'active' | 'locked' @@ -215,14 +218,13 @@ function computeDoneMigrate(obs: ColumnObservation): number { // Phase progression dominates when we have it. The v3 ladder is one rung // shorter (no cut-over), so its terminal phases map one lower. + // Reverse (`stash decrypt`) phases are keyed by the encrypted column and + // never reach this forward lookup; if one does, the forward ladder is done. + if (obs.phase !== undefined && isReverseMigrationPhase(obs.phase)) { + return isV3 ? 4 : 5 + } switch (obs.phase) { case 'dropped': - // Reverse (`stash decrypt`) phases are keyed by the encrypted column and - // never reach this forward lookup; if one does, the forward ladder is done. - case 'decrypt-dual-writing': - case 'decrypting': - case 'decrypted': - case 'encrypted-dropped': return isV3 ? 4 : 5 case 'cut-over': return 4 diff --git a/languages/typescript/packages/migrate/src/column-migration.ts b/languages/typescript/packages/migrate/src/column-migration.ts index 4eae7f797..08558a9dc 100644 --- a/languages/typescript/packages/migrate/src/column-migration.ts +++ b/languages/typescript/packages/migrate/src/column-migration.ts @@ -76,7 +76,9 @@ const OPPOSITE: Record = { /** * An open obligation to complete a forced pass. While one is open, every run - * is a forced pass, whatever the caller passes. See ADR 0001. + * is a forced pass, whatever the caller passes. See this package's ADR 0001, + * "A forced pass is an obligation" + * (`docs/adr/0001-forced-pass-is-an-obligation.md`). * * - `existing-values`: a reverse column migration began on a twin that * already held values. A {@link Waiver} can close it until a forced pass diff --git a/languages/typescript/packages/migrate/src/decrypt.ts b/languages/typescript/packages/migrate/src/decrypt.ts index b2c882971..65157fbaa 100644 --- a/languages/typescript/packages/migrate/src/decrypt.ts +++ b/languages/typescript/packages/migrate/src/decrypt.ts @@ -101,7 +101,8 @@ export class PlaintextWriteError extends Error { * parameters in the chunk's `UPDATE … FROM (VALUES ($1, $2), …)` (the primary * key, and the value packed with the ciphertext it was decrypted from), and * Postgres's wire protocol caps a statement at 65535 parameters, so 32767 - * rows is the most that can ever be written in one statement. A larger chunk would fail only after its ZeroKMS request. + * rows is the most that can ever be written in one statement. A larger chunk + * would fail only after its ZeroKMS request. */ export const MAX_DECRYPT_CHUNK_SIZE = Math.floor(65535 / 2) diff --git a/languages/typescript/packages/migrate/src/engine.ts b/languages/typescript/packages/migrate/src/engine.ts index 2ca7d8c9a..3dc69849c 100644 --- a/languages/typescript/packages/migrate/src/engine.ts +++ b/languages/typescript/packages/migrate/src/engine.ts @@ -91,8 +91,6 @@ export interface ChunkedRunOptions { * for this chunk and the cursor where it was. */ prepareChunk(rows: Row[], context: ChunkContext): Promise - /** Extra fields merged into the `started` event's `details`. */ - startedDetails?: Record /** * Fields merged into EVERY event's `details` (started, checkpoint, * completed, error) — e.g. the reverse run records which plaintext column @@ -195,7 +193,6 @@ export async function runChunkedMigration( ...(forcedBy !== null ? { forcedBy } : {}), chunkSize, resumed, - ...options.startedDetails, }, }) diff --git a/skills/stash-encryption/SKILL.md b/skills/stash-encryption/SKILL.md index cf4ae37a5..936584ad7 100644 --- a/skills/stash-encryption/SKILL.md +++ b/skills/stash-encryption/SKILL.md @@ -1050,7 +1050,7 @@ await runBackfill({ }) ``` -Useful when the backfill needs to run in a worker, on a schedule, or alongside an existing job runner. `runBackfill` is version-agnostic — for an EQL v3 column pass an `Encryption` client built from a v3 schema set and it writes v3 envelopes straight into the concrete `eql_v3_*` domain column. +Useful when the backfill needs to run in a worker, on a schedule, or alongside an existing job runner. The engine applies the same rules as the CLI, whoever calls it: it throws a `ColumnMigrationError` while a reverse migration of the same columns is open; a run that follows an interrupted `force: true` run is forced too, until a forced pass completes (the result's `forcedBy` says why); a failed run resumes from its last committed position; and an `error` event records the error's class (`errorName`) and safe details, never its message. `ColumnMigration.load()` from `@cipherstash/migrate` answers the same questions (phase, twin, owed forced pass) before a run. `runBackfill` is version-agnostic — for an EQL v3 column pass an `Encryption` client built from a v3 schema set and it writes v3 envelopes straight into the concrete `eql_v3_*` domain column. ### Invariants the rollout preserves From f22c8c0df97882bc6e298a1a32e4315665741660 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 19 Oct 2026 01:58:14 +0000 Subject: [PATCH 06/15] test(cli): leave no cipherstash schema behind from the decrypt lifecycle suite The suite's backfill creates cipherstash.cs_migrations. CI connects as the role cipherstash, whose default search_path starts with "$user", so the leftover schema received the unqualified operators a later suite's EQL install created, and verify.live and upgrade-encrypted-indexes.live then failed with EqlReinstallRefusalError. Drop the schema afterwards when the suite created it. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01Scub1tpuegHHCTXrgn7SHf --- .../__tests__/decrypt-lifecycle.live.test.ts | 24 +++++++++++++++---- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/languages/typescript/packages/cli/src/commands/decrypt/__tests__/decrypt-lifecycle.live.test.ts b/languages/typescript/packages/cli/src/commands/decrypt/__tests__/decrypt-lifecycle.live.test.ts index 7d1672d47..d86fa235c 100644 --- a/languages/typescript/packages/cli/src/commands/decrypt/__tests__/decrypt-lifecycle.live.test.ts +++ b/languages/typescript/packages/cli/src/commands/decrypt/__tests__/decrypt-lifecycle.live.test.ts @@ -129,21 +129,35 @@ describeLive('decrypt lifecycle — live Postgres', () => { let client: pg.Client let cwd: string let originalCwd: string + /** Whether `cipherstash` existed before this suite created it. */ + let trackingSchemaExisted = true beforeAll(async () => { const { default: pgModule } = await import('pg') client = new pgModule.Client({ connectionString: SCOPED_URL }) await client.connect() originalCwd = process.cwd() + const { rows } = await client.query( + `SELECT 1 FROM pg_namespace WHERE nspname = 'cipherstash'`, + ) + trackingSchemaExisted = rows.length > 0 }) afterAll(async () => { await client.query(`DROP SCHEMA IF EXISTS ${SCHEMA} CASCADE`) - await client - .query('DELETE FROM cipherstash.cs_migrations WHERE table_name = $1', [ - TABLE, - ]) - .catch(() => undefined) + if (trackingSchemaExisted) { + await client + .query('DELETE FROM cipherstash.cs_migrations WHERE table_name = $1', [ + TABLE, + ]) + .catch(() => undefined) + } else { + // Leave the database as found. A role named `cipherstash` has + // `"$user"` first on its default search_path, so a `cipherstash` schema + // left behind would receive the unqualified objects a later suite's EQL + // install creates, and the installer would then refuse to reinstall. + await client.query('DROP SCHEMA IF EXISTS cipherstash CASCADE') + } await client.end().catch(() => undefined) }) From 2c489a18950148e8345913982b26ee0d6e13b51b Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 19 Oct 2026 03:28:15 +0000 Subject: [PATCH 07/15] fix(migrate): read obligations from earlier event shapes and record the twin in the engine - A decrypt_dual_writing event recorded as acknowledgedWith: 'force' by the earlier decrypt CLI opens the up-front forced-pass obligation, as existingTwinValues does now. - A completion event without details.force, as the earlier forward backfill wrote, takes the mode of the pass started before it, so a completed legacy forced pass is not owed again. - runChunkedMigration records the twin on every event itself, so a direct caller that passes no baseDetails cannot resume a paused run into a different twin. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01Scub1tpuegHHCTXrgn7SHf --- .../__tests__/backfill.integration.test.ts | 54 +++++++++++++++++ .../src/__tests__/column-migration.test.ts | 58 +++++++++++++++++++ .../packages/migrate/src/column-migration.ts | 47 +++++++++++---- .../typescript/packages/migrate/src/engine.ts | 8 ++- 4 files changed, 155 insertions(+), 12 deletions(-) diff --git a/languages/typescript/packages/migrate/src/__tests__/backfill.integration.test.ts b/languages/typescript/packages/migrate/src/__tests__/backfill.integration.test.ts index d96a36f29..b44c4ab02 100644 --- a/languages/typescript/packages/migrate/src/__tests__/backfill.integration.test.ts +++ b/languages/typescript/packages/migrate/src/__tests__/backfill.integration.test.ts @@ -22,6 +22,7 @@ import pg from 'pg' import { afterAll, afterEach, beforeAll, describe, expect, it } from 'vitest' import { type EncryptionClientLike, runBackfill } from '../backfill.js' import { ColumnMigrationError } from '../column-migration.js' +import { runChunkedMigration } from '../engine.js' import { installMigrationsSchema } from '../install.js' import { appendEvent, latestByColumn, progress } from '../state.js' @@ -556,4 +557,57 @@ describe.skipIf(!runIntegration)('runBackfill (integration)', () => { await expect(forward()).rejects.toBeInstanceOf(ColumnMigrationError) expect(await countEncrypted()).toBe(0) }) + it('the engine records the twin itself, so a paused run is not resumed into another', async () => { + await seed(4) + const run = async (twinColumn: string, signal?: AbortSignal) => { + const db = await pool.connect() + try { + return await runChunkedMigration<{ pk: string }>({ + db, + tableName: 'migrate_test.users', + stateColumn: 'email', + twinColumn, + direction: 'forward', + chunkSize: 2, + signal, + countPending: async (after) => + Number( + ( + await db.query<{ n: string }>( + 'SELECT count(*)::text AS n FROM migrate_test.users WHERE id > $1', + [after ?? 0], + ) + ).rows[0]?.n, + ), + fetchPage: async (after, limit) => { + const { rows } = await db.query<{ pk: string }>( + 'SELECT id::text AS pk FROM migrate_test.users WHERE id > $1 ORDER BY id LIMIT $2', + [after ?? 0, limit], + ) + return { rows, lastPk: rows.at(-1)?.pk ?? null } + }, + prepareChunk: async () => ({ write: async () => {} }), + }) + } finally { + db.release() + } + } + + // Pause after the first chunk, with no baseDetails naming the twin. + const controller = new AbortController() + controller.abort() + const paused = await run('email_encrypted', controller.signal) + expect(paused.completed).toBe(false) + const { rows: recorded } = await pool.query<{ details: unknown }>( + `SELECT details FROM cipherstash.cs_migrations + WHERE column_name = 'email' ORDER BY id DESC LIMIT 1`, + ) + expect(recorded[0]?.details).toMatchObject({ + encryptedColumn: 'email_encrypted', + }) + + await expect(run('email_ciphertext')).rejects.toMatchObject({ + code: 'twin-mismatch', + }) + }) }) diff --git a/languages/typescript/packages/migrate/src/__tests__/column-migration.test.ts b/languages/typescript/packages/migrate/src/__tests__/column-migration.test.ts index b009301b0..f4120773c 100644 --- a/languages/typescript/packages/migrate/src/__tests__/column-migration.test.ts +++ b/languages/typescript/packages/migrate/src/__tests__/column-migration.test.ts @@ -246,6 +246,38 @@ describe('deriveColumnMigration (reverse)', () => { }) }) +describe('deriveColumnMigration (reverse, events from an earlier release)', () => { + it('a twin acknowledged with --force at confirm owes a forced pass', () => { + const state = deriveColumnMigration({ + direction: 'reverse', + history: [ + ev('decrypt_dual_writing', 'decrypt-dual-writing', { + ...twin, + existingPlaintext: 'prepopulated', + acknowledgedWith: 'force', + }), + ], + }) + expect(state.obligations).toMatchObject([ + { reason: 'existing-values', waivable: true }, + ]) + }) + + it('a twin acknowledged as current at confirm owes nothing', () => { + const state = deriveColumnMigration({ + direction: 'reverse', + history: [ + ev('decrypt_dual_writing', 'decrypt-dual-writing', { + ...twin, + existingPlaintext: 'prepopulated', + acknowledgedWith: 'confirm-plaintext-current', + }), + ], + }) + expect(state.obligations).toEqual([]) + }) +}) + describe('deriveColumnMigration (forward)', () => { const fwd = ( event: MigrationEvent, @@ -265,6 +297,32 @@ describe('deriveColumnMigration (forward)', () => { expect(state.obligations.map((o) => o.reason)).toEqual(['forced-pass']) }) + it('a forced pass an older release completed (force only on started) is not owed again', () => { + const state = deriveColumnMigration({ + direction: 'forward', + history: [ + fwd('dual_writing', 'dual-writing'), + fwd('backfill_started', 'backfilling', { force: true }), + fwd('backfill_checkpoint', 'backfilling', { chunkIndex: 0 }), + fwd('backfilled', 'backfilled', {}), + ], + }) + expect(state.obligations).toEqual([]) + }) + + it('an older unforced pass completing does not close a forced pass started after it', () => { + const state = deriveColumnMigration({ + direction: 'forward', + history: [ + fwd('dual_writing', 'dual-writing'), + fwd('backfill_started', 'backfilling', { force: true }), + fwd('backfill_started', 'backfilling', {}), + fwd('backfilled', 'backfilled', {}), + ], + }) + expect(state.obligations.map((o) => o.reason)).toEqual(['forced-pass']) + }) + it('a dropped forward column migration is finished', () => { const state = deriveColumnMigration({ direction: 'forward', diff --git a/languages/typescript/packages/migrate/src/column-migration.ts b/languages/typescript/packages/migrate/src/column-migration.ts index 08558a9dc..db7c5f2da 100644 --- a/languages/typescript/packages/migrate/src/column-migration.ts +++ b/languages/typescript/packages/migrate/src/column-migration.ts @@ -187,20 +187,22 @@ export function deriveColumnMigration(input: { const scope = events.slice(scopeStart) let obligation: Obligation | null = null + // The mode of the latest pass started, for completion events written + // before every event recorded its own (see `passForced`). + let startedForced = false for (const e of scope) { + if (e.event === vocab.started) startedForced = isForced(e) if (e.event === vocab.dualWriting) { - obligation = - e.details?.existingTwinValues === true && - e.details?.waiver === undefined - ? { - kind: 'forced-pass', - reason: 'existing-values', - openedAt: e.id, - waivable: true, - } - : null + obligation = owesUpFront(e) + ? { + kind: 'forced-pass', + reason: 'existing-values', + openedAt: e.id, + waivable: true, + } + : null } else if (e.event === vocab.completed) { - if (isForced(e)) obligation = null + if (passForced(e, startedForced)) obligation = null } else if (e.phase === vocab.runningPhase && isForced(e)) { if (obligation?.reason !== 'forced-pass') { obligation = { @@ -689,6 +691,29 @@ function isForced(row: MigrationStateRow): boolean { return row.details?.force === true } +/** + * Whether a completion event closes a forced pass. An earlier release + * recorded `force` on the started event only, so a completion without the + * key takes the mode of the pass started before it. + */ +function passForced(row: MigrationStateRow, startedForced: boolean): boolean { + if (row.details !== null && 'force' in row.details) return isForced(row) + return startedForced +} + +/** + * Whether a dual-write confirmation opens the up-front obligation: the twin + * already held values and no waiver was given. An earlier release recorded + * the same choice as `acknowledgedWith: 'force'`. + */ +function owesUpFront(row: MigrationStateRow): boolean { + if (row.details?.acknowledgedWith === 'force') return true + return ( + row.details?.existingTwinValues === true && + row.details?.waiver === undefined + ) +} + function twinNamedBy( vocab: DirectionVocabulary, row: MigrationStateRow, diff --git a/languages/typescript/packages/migrate/src/engine.ts b/languages/typescript/packages/migrate/src/engine.ts index 3dc69849c..0cfb42f36 100644 --- a/languages/typescript/packages/migrate/src/engine.ts +++ b/languages/typescript/packages/migrate/src/engine.ts @@ -169,7 +169,13 @@ export async function runChunkedMigration( const plan = await migration.planPass({ force: options.force ?? false }) const { force, forcedBy } = plan const mode: PassMode = { force } - const base = { ...options.baseDetails, force } + // The pairing is recorded on every event whatever the caller passes, so a + // later run with a different twin is refused rather than resumed. + const base = { + ...options.baseDetails, + [events.twinKey]: options.twinColumn, + force, + } const resumedFrom = plan.resumeFrom const resumeCursor = resumedFrom ? resumedFrom.cursorValue : null From c705194c0950ad7f2e134cac47d161940ecc1aa6 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 01:01:15 +0000 Subject: [PATCH 08/15] fix(cli): reconcile ColumnMigration with main's decrypt fixes --confirm-plaintext-current is a no-op only when no obligation is open, so main's warning no longer fires on a later run where the waiver closes one. The help text and the stash-cli skill say the waiver is refused while a started forced pass is owed. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01Pkrkj6eBHR934hMcSGKSHg --- .../typescript/packages/cli/src/cli/registry.ts | 4 ++-- .../decrypt/__tests__/decrypt-backfill.test.ts | 17 ----------------- .../cli/src/commands/decrypt/backfill.ts | 7 +++++-- skills/stash-cli/SKILL.md | 4 ++-- 4 files changed, 9 insertions(+), 23 deletions(-) diff --git a/languages/typescript/packages/cli/src/cli/registry.ts b/languages/typescript/packages/cli/src/cli/registry.ts index 2ee742c1b..e052aa712 100644 --- a/languages/typescript/packages/cli/src/cli/registry.ts +++ b/languages/typescript/packages/cli/src/cli/registry.ts @@ -701,7 +701,7 @@ export const registry: CommandGroup[] = [ 'backfill only fills NULLs, so stale values would survive. Pass --force', 'to overwrite them from the ciphertext, or --confirm-plaintext-current', 'to keep them. A forced pass, once started, is owed: every later run is', - 'forced until one completes, and the waiver is refused from then on.', + 'forced until one completes, and the waiver is refused while it is owed.', '`decrypt status` flags a column that owes one (forced-pass-owed).', 'Keep writing both columns until the drop is applied.', ].join('\n'), @@ -741,7 +741,7 @@ export const registry: CommandGroup[] = [ { name: '--confirm-plaintext-current', description: - 'Keep the values a plaintext column already holds, asserting they are current. Refused once a forced pass has started.', + 'Keep the values a plaintext column already holds, asserting they are current. Refused while a started forced pass is owed.', }, { name: '--continue-on-undecryptable', diff --git a/languages/typescript/packages/cli/src/commands/decrypt/__tests__/decrypt-backfill.test.ts b/languages/typescript/packages/cli/src/commands/decrypt/__tests__/decrypt-backfill.test.ts index de52a58c6..8eac27450 100644 --- a/languages/typescript/packages/cli/src/commands/decrypt/__tests__/decrypt-backfill.test.ts +++ b/languages/typescript/packages/cli/src/commands/decrypt/__tests__/decrypt-backfill.test.ts @@ -367,23 +367,6 @@ describe('decrypt backfill', () => { expect(held.migration.confirmDualWrites).not.toHaveBeenCalled() expect(migrateMocks.runDecryptBackfill).not.toHaveBeenCalled() }) - - it('says --confirm-plaintext-current has no effect when there is nothing to confirm', async () => { - // A later run of a lifecycle in progress, or an empty twin. - hazardMock.mockResolvedValue(null) - - await decryptBackfillCommand({ - table: 'users', - column: 'email_encrypted', - confirmDualWritesDeployed: true, - confirmPlaintextCurrent: true, - }) - - expect(p.log.warn).toHaveBeenCalledWith( - expect.stringContaining('--confirm-plaintext-current has no effect'), - ) - expect(migrateMocks.runDecryptBackfill).toHaveBeenCalled() - }) }) it('says why a run it did not ask to force is a forced pass', async () => { diff --git a/languages/typescript/packages/cli/src/commands/decrypt/backfill.ts b/languages/typescript/packages/cli/src/commands/decrypt/backfill.ts index 6d3634a7d..fe6be8d29 100644 --- a/languages/typescript/packages/cli/src/commands/decrypt/backfill.ts +++ b/languages/typescript/packages/cli/src/commands/decrypt/backfill.ts @@ -183,9 +183,12 @@ export async function decryptBackfillCommand( `Proceeding under --confirm-plaintext-current: keeping the values already in ${options.table}.${pair.plaintextColumn} and decrypting only the rows where it is NULL. You are asserting those values are current.`, ) } - } else if (options.confirmPlaintextCurrent) { + } else if ( + options.confirmPlaintextCurrent && + pair.migration.obligations.length === 0 + ) { p.log.warn( - `--confirm-plaintext-current has no effect here: it applies only to the first run of a reverse migration whose plaintext column ${options.table}.${pair.plaintextColumn} already holds values, and this run has nothing to confirm.`, + `--confirm-plaintext-current has no effect here: it applies only while a reverse migration owes a forced pass because its plaintext column ${options.table}.${pair.plaintextColumn} already held values, and this run has nothing to confirm.`, ) } diff --git a/skills/stash-cli/SKILL.md b/skills/stash-cli/SKILL.md index 19c7bcfe7..77336726a 100644 --- a/skills/stash-cli/SKILL.md +++ b/skills/stash-cli/SKILL.md @@ -684,7 +684,7 @@ Same engine as `encrypt backfill`: keyset pagination, one transaction per chunk - `--force` overwrites every row of the twin with the decrypted ciphertext. Use this when in doubt. - `--confirm-plaintext-current` keeps the existing values and fills only the NULLs. Use it only when the application has written the plaintext column on every insert and update for the whole time the encrypted column existed. -The choice is recorded in `cs_migrations`. Later runs of the same reverse migration do not ask again: the twin's values are then its own. Choosing `--force` makes a forced pass owed from that moment, and so does starting any `--force` run: every later run is forced, with or without `--force`, until one completes, so an interrupted `--force` run is always finished as one, even if it failed before writing anything. The run says when it is forced for this reason. `--confirm-plaintext-current` is accepted on a later run too, as long as no forced pass has started; once one has, it is refused, because rows already overwritten cannot be un-overwritten. A `--force` run after an interrupted plain one starts again from the first row. +The choice is recorded in `cs_migrations`. Later runs of the same reverse migration do not ask again: the twin's values are then its own. Choosing `--force` makes a forced pass owed from that moment, and so does starting any `--force` run: every later run is forced, with or without `--force`, until one completes, so an interrupted `--force` run is always finished as one, even if it failed before writing anything. The run says when it is forced for this reason. `--confirm-plaintext-current` is accepted on a later run too, as long as no forced pass has started; while a started one is owed, it is refused, because rows already overwritten cannot be un-overwritten. Once that pass completes there is nothing left to confirm, and the waiver has no effect. A `--force` run after an interrupted plain one starts again from the first row. **Lock contexts — known limitation, no bypass.** A value encrypted with an end-user lock context can only be decrypted as that user (see `stash-auth`). With workspace credentials it fails *on its own row* (`bulkDecrypt` reports per row), so the backfill **skips it, counts it, leaves the twin NULL** (a `--force` pass sets a skipped row's twin to NULL, so a stale value is never left behind), and reports the count and error codes; the count is also recorded in `cs_migrations` and shown by `decrypt status`. Decrypt those rows in your application on each user's behalf (writing the plaintext column), then re-run without `--force`. Do this only after the last forced pass has completed: a forced pass (including a plain run that finishes an interrupted one) sets the twin back to NULL on every row it cannot decrypt, an application-written value included. `decrypt drop` refuses while any remain. @@ -702,7 +702,7 @@ The choice is recorded in `cs_migrations`. Later runs of the same reverse migrat | `--pk-column ` | Override primary-key detection | | `--confirm-dual-writes-deployed` | Non-interactive equivalent of the prompt: the app writes the encrypted column **and** its plaintext twin on every insert and update | | `--force` | Re-decrypt rows whose plaintext is already set and overwrite it: drift recovery, or replacing a twin that held values before the first run. Safe only while dual-writes are live: any row the application wrote to the plaintext column alone (a newer value, or one cleared on purpose) gets the older ciphertext's value back. | -| `--confirm-plaintext-current` | Keep the values a twin already holds, asserting they are current (see above). Refused once a forced pass has started | +| `--confirm-plaintext-current` | Keep the values a twin already holds, asserting they are current (see above). Refused while a started forced pass is owed. On a run with nothing to confirm it has no effect, and the backfill says so | | `--continue-on-undecryptable` | Do not stop when the first batch decrypts nothing | #### `decrypt drop` From fb838a3f1a237820578df75d6e02b0ac858658f2 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 00:58:55 +0000 Subject: [PATCH 09/15] fix(migrate,cli): address review of the column migration API - A forward column migration dropped and then reversed is superseded, like the reverse case, so encrypting the column again records a new dual_writing event instead of reading as 'dropped'. - `decrypt status` leaves out a finished reversal (drop applied, column migrated back) instead of flagging it encrypted-col-missing. - Live tests make forward-open and forced-pass-owed appear, and check the waiver is refused while a started forced pass is owed. - The backfill tests' fake ColumnMigration applies the real waiver rules, and the superseded-history test uses a superseded history. - resolve-pair suggests the recorded column only on a real mismatch. - The engine twin test pauses after a committed chunk and resumes it. - Help text, skills and changeset: the waiver is refused while a started forced pass is owed; runBackfill throws twin-mismatch. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01Scub1tpuegHHCTXrgn7SHf --- .changeset/column-migration-api.md | 2 +- .../__tests__/decrypt-backfill.test.ts | 88 ++++++++++++- .../__tests__/decrypt-lifecycle.live.test.ts | 118 ++++++++++++++++++ .../decrypt/__tests__/fake-migration.ts | 47 +++++-- .../decrypt/__tests__/resolve-pair.test.ts | 13 ++ .../src/commands/decrypt/lib/resolve-pair.ts | 14 ++- .../cli/src/commands/decrypt/status.ts | 3 +- .../__tests__/backfill.integration.test.ts | 29 +++-- .../src/__tests__/column-migration.test.ts | 24 ++++ .../src/__tests__/decrypt.integration.test.ts | 47 +++++++ .../packages/migrate/src/column-migration.ts | 9 +- skills/stash-encryption/SKILL.md | 2 +- 12 files changed, 361 insertions(+), 35 deletions(-) diff --git a/.changeset/column-migration-api.md b/.changeset/column-migration-api.md index 85dc7aec4..23dbcdcb5 100644 --- a/.changeset/column-migration-api.md +++ b/.changeset/column-migration-api.md @@ -5,6 +5,6 @@ `@cipherstash/migrate` adds `ColumnMigration`, one place for the rules of a column migration in either direction: which twin column it pairs with (the flag, the recorded pairing, the manifest, then the naming convention), its phase, whether a forced pass is owed, and which open migration in the other direction blocks it. `ColumnMigration.load()` reads it from `cipherstash.cs_migrations`; `confirmDualWrites()`, `recordDropGenerated()` and `recordSourceRemoved()` record events, refusing those the rules forbid with a `ColumnMigrationError`. The pure rules are exported as `deriveColumnMigration`, `openCounterpart`, `planPassFor`, `passProgress`, `deriveTwinName` and `manifestTwin`. -`runBackfill` and `runDecryptBackfill` now apply the rules themselves rather than leaving them to the caller. Every event records whether its pass was forced. An interrupted forced pass is finished as one by the next run, with or without `force`, and the result says why in `forcedBy`. A forward backfill resumes after a failed run from its last committed position, counts its total once when it resumes a forced pass, and refuses to start while a reverse migration of the same columns is open. A reverse backfill over a twin that already holds values is forced unless the caller confirmed dual-writes with the `plaintext-current` waiver. Error events store the error's class (`errorName`) and safe details instead of its message, which could carry row data. +`runBackfill` and `runDecryptBackfill` now apply the rules themselves rather than leaving them to the caller. Every event records whether its pass was forced. An interrupted forced pass is finished as one by the next run, with or without `force`, and the result says why in `forcedBy`. A forward backfill resumes after a failed run from its last committed position, counts its total once when it resumes a forced pass, and refuses to start while a reverse migration of the same columns is open. `runBackfill` now throws a `ColumnMigrationError` with code `twin-mismatch` when `encryptedColumn` differs from the encrypted column already recorded for that column, rather than continuing into a different one. A reverse backfill over a twin that already holds values is forced unless the caller confirmed dual-writes with the `plaintext-current` waiver. Error events store the error's class (`errorName`) and safe details instead of its message, which could carry row data. `stash decrypt plan`, `status`, `backfill` and `drop` resolve every column through `ColumnMigration`, so they always agree on the twin and the phase. `decrypt status` flags `forward-open`, `twin-has-values` and `forced-pass-owed`. `decrypt backfill --confirm-plaintext-current` is accepted on a later run as long as no forced pass has started, and refused once one has. diff --git a/languages/typescript/packages/cli/src/commands/decrypt/__tests__/decrypt-backfill.test.ts b/languages/typescript/packages/cli/src/commands/decrypt/__tests__/decrypt-backfill.test.ts index 8eac27450..9425bfda5 100644 --- a/languages/typescript/packages/cli/src/commands/decrypt/__tests__/decrypt-backfill.test.ts +++ b/languages/typescript/packages/cli/src/commands/decrypt/__tests__/decrypt-backfill.test.ts @@ -367,6 +367,23 @@ describe('decrypt backfill', () => { expect(held.migration.confirmDualWrites).not.toHaveBeenCalled() expect(migrateMocks.runDecryptBackfill).not.toHaveBeenCalled() }) + + it('says --confirm-plaintext-current has no effect when there is nothing to confirm', async () => { + // An empty twin: nothing for the waiver to keep. + resolveMock.mockResolvedValue(pair()) + + await decryptBackfillCommand({ + table: 'users', + column: 'email_encrypted', + confirmDualWritesDeployed: true, + confirmPlaintextCurrent: true, + }) + + expect(p.log.warn).toHaveBeenCalledWith( + expect.stringContaining('--confirm-plaintext-current has no effect'), + ) + expect(migrateMocks.runDecryptBackfill).toHaveBeenCalled() + }) }) it('says why a run it did not ask to force is a forced pass', async () => { @@ -392,9 +409,17 @@ describe('decrypt backfill', () => { ) }) - it('a waiver on a column migration in progress goes to the column migration, which may refuse it', async () => { - const running = midPass() - resolveMock.mockResolvedValue(running) + it('a later waiver closes an up-front obligation no forced pass has started on', async () => { + const owed = pair({ + migration: fakeMigration({ + history: [ + reverseEvent('decrypt_dual_writing', 'decrypt-dual-writing', { + details: { plaintextColumn: 'email', existingTwinValues: true }, + }), + ], + }), + }) + resolveMock.mockResolvedValue(owed) await decryptBackfillCommand({ table: 'users', @@ -402,15 +427,68 @@ describe('decrypt backfill', () => { confirmPlaintextCurrent: true, }) - expect(running.migration.confirmDualWrites).toHaveBeenCalledWith({ + expect(owed.migration.confirmDualWrites).toHaveBeenCalledWith({ waiver: 'plaintext-current', }) + expect(p.log.warn).not.toHaveBeenCalledWith( + expect.stringContaining('--confirm-plaintext-current has no effect'), + ) + expect(migrateMocks.runDecryptBackfill).toHaveBeenCalledWith( + expect.not.objectContaining({ force: true }), + ) + }) + + it('a waiver once a forced pass has started is refused, and nothing runs', async () => { + const exitSpy = spyExit() + resolveMock.mockResolvedValue( + pair({ + migration: fakeMigration({ + history: [ + reverseEvent('decrypt_dual_writing', 'decrypt-dual-writing'), + reverseEvent('decrypt_started', 'decrypting', { + details: { plaintextColumn: 'email', force: true }, + }), + ], + }), + }), + ) + + await decryptBackfillCommand({ + table: 'users', + column: 'email_encrypted', + confirmPlaintextCurrent: true, + }) + + expect(p.log.error).toHaveBeenCalledWith( + expect.stringContaining('has already started'), + ) + expect(migrateMocks.runDecryptBackfill).not.toHaveBeenCalled() + expect(exitSpy).toHaveBeenCalledWith(1) + exitSpy.mockRestore() }) it('a finished earlier column migration does not pass the dual-write gate', async () => { // A drop generated, applied, and the column encrypted again: the forward // events of the twin after the drop make the reverse history finished. - resolveMock.mockResolvedValue(pair()) + const finished = [ + reverseEvent('decrypt_dual_writing', 'decrypt-dual-writing'), + reverseEvent('decrypted', 'decrypted'), + reverseEvent('encrypted_dropped', 'encrypted-dropped'), + ] + const encryptedAgain = [ + reverseEvent('dual_writing', 'dual-writing', { + columnName: 'email', + details: { encryptedColumn: 'email_encrypted' }, + }), + ] + resolveMock.mockResolvedValue( + pair({ + migration: fakeMigration({ + history: finished, + recordedTwinHistory: encryptedAgain, + }), + }), + ) await decryptBackfillCommand({ table: 'users', column: 'email_encrypted' }) diff --git a/languages/typescript/packages/cli/src/commands/decrypt/__tests__/decrypt-lifecycle.live.test.ts b/languages/typescript/packages/cli/src/commands/decrypt/__tests__/decrypt-lifecycle.live.test.ts index d86fa235c..b4ca64c13 100644 --- a/languages/typescript/packages/cli/src/commands/decrypt/__tests__/decrypt-lifecycle.live.test.ts +++ b/languages/typescript/packages/cli/src/commands/decrypt/__tests__/decrypt-lifecycle.live.test.ts @@ -368,4 +368,122 @@ describeLive('decrypt lifecycle — live Postgres', () => { }), ).rejects.toThrow(/--force[\s\S]*--confirm-plaintext-current/) }) + it('status flags a twin a forward encryption is still open on, which backfill refuses', async () => { + await client.query(` + CREATE TABLE ${TABLE} ( + id int PRIMARY KEY, + email_encrypted eql_v3_text_search, + email text + ) + `) + await installMigrationsSchema(client) + await appendEvent(client, { + tableName: TABLE, + columnName: 'email', + event: 'dual_writing', + phase: 'dual-writing', + details: { encryptedColumn: 'email_encrypted' }, + }) + + expect(rowFor(await statusRows(), 'email_encrypted')).toContain( + 'forward-open', + ) + await expect( + decryptBackfillCommand({ + table: TABLE, + column: 'email_encrypted', + confirmDualWritesDeployed: true, + }), + ).rejects.toThrow(/forward encryption/) + }) + + it('status flags a started forced pass as owed, refuses the waiver, and clears once a run finishes it', async () => { + await client.query(` + CREATE TABLE ${TABLE} ( + id int PRIMARY KEY, + email_encrypted eql_v3_text_search, + email text + ) + `) + for (let id = 1; id <= 3; id++) { + await client.query( + `INSERT INTO ${TABLE} VALUES ($1, $2, 'stale@example.com')`, + [id, JSON.stringify(envelope(`user-${id}@example.com`))], + ) + } + await installMigrationsSchema(client) + // A `--force` run over a twin that held values, interrupted after it + // started: the forced pass is owed. + await appendEvent(client, { + tableName: TABLE, + columnName: 'email_encrypted', + event: 'decrypt_dual_writing', + phase: 'decrypt-dual-writing', + details: { plaintextColumn: 'email', existingTwinValues: true }, + }) + await appendEvent(client, { + tableName: TABLE, + columnName: 'email_encrypted', + event: 'decrypt_started', + phase: 'decrypting', + details: { plaintextColumn: 'email', force: true }, + }) + + expect(rowFor(await statusRows(), 'email_encrypted')).toContain( + 'forced-pass-owed', + ) + await expect( + decryptBackfillCommand({ + table: TABLE, + column: 'email_encrypted', + confirmPlaintextCurrent: true, + }), + ).rejects.toThrow(/forced pass .* has already started/) + + // A plain run finishes it as a forced pass, overwriting the stale values. + await decryptBackfillCommand({ table: TABLE, column: 'email_encrypted' }) + const { rows: stale } = await client.query<{ n: string }>( + `SELECT count(*) AS n FROM ${TABLE} WHERE email = 'stale@example.com'`, + ) + expect(stale[0].n).toBe('0') + expect(rowFor(await statusRows(), 'email_encrypted')).not.toContain( + 'forced-pass-owed', + ) + }) + + it('status leaves out a finished reversal whose plaintext column was encrypted again under a new name', async () => { + // email_encrypted was decrypted into email and dropped; email was then + // encrypted again into email_v2. + await client.query(` + CREATE TABLE ${TABLE} ( + id int PRIMARY KEY, + email text, + email_v2 eql_v3_text_search + ) + `) + await installMigrationsSchema(client) + await appendEvent(client, { + tableName: TABLE, + columnName: 'email_encrypted', + event: 'encrypted_dropped', + phase: 'encrypted-dropped', + details: { plaintextColumn: 'email' }, + }) + await appendEvent(client, { + tableName: TABLE, + columnName: 'email', + event: 'dual_writing', + phase: 'dual-writing', + details: { encryptedColumn: 'email_v2' }, + }) + + const rows = await statusRows() + + expect( + rows.find((r) => r.startsWith(`${TABLE} email_encrypted `)), + rows.join('\n'), + ).toBeUndefined() + expect(rows.join('\n')).not.toContain('encrypted-col-missing') + expect(rowFor(rows, 'email_v2')).toBeDefined() + }) }) diff --git a/languages/typescript/packages/cli/src/commands/decrypt/__tests__/fake-migration.ts b/languages/typescript/packages/cli/src/commands/decrypt/__tests__/fake-migration.ts index 67f67d5ec..53d120d1a 100644 --- a/languages/typescript/packages/cli/src/commands/decrypt/__tests__/fake-migration.ts +++ b/languages/typescript/packages/cli/src/commands/decrypt/__tests__/fake-migration.ts @@ -9,6 +9,7 @@ import { type ColumnMigration, + ColumnMigrationError, deriveColumnMigration, type MigrationEvent, type MigrationPhase, @@ -66,6 +67,8 @@ export type FakeMigration = ColumnMigration & { export function fakeMigration( opts: { history?: MigrationStateRow[] + /** Events under the recorded twin, which can make `history` finished. */ + recordedTwinHistory?: MigrationStateRow[] twin?: string blockedBy?: OpenCounterpart | null twinHasValues?: boolean @@ -74,6 +77,7 @@ export function fakeMigration( const state = deriveColumnMigration({ direction: 'reverse', history: opts.history ?? [], + recordedTwinHistory: opts.recordedTwinHistory, }) const twinHasValues = opts.twinHasValues ?? false const fake = { @@ -96,16 +100,39 @@ export function fakeMigration( recordDropGenerated: vi.fn(), recordSourceRemoved: vi.fn(), } - fake.confirmDualWrites.mockImplementation(async () => { - const confirmed = fakeMigration({ - ...opts, - history: [ - ...(opts.history ?? []), - reverseEvent('decrypt_dual_writing', 'decrypt-dual-writing'), - ], - }) - return confirmed - }) + // The real rules: in progress, record nothing unless a waiver closes a + // waivable obligation, and refuse one that is not. + fake.confirmDualWrites.mockImplementation( + async ({ waiver }: { waiver?: 'plaintext-current' } = {}) => { + let existingTwinValues: boolean + if (state.phase !== null) { + const owed = state.obligations[0] + if (waiver === undefined || owed === undefined) return fake + if (!owed.waivable) { + throw new ColumnMigrationError( + 'waiver-refused', + 'A forced pass over users.email has already started.', + ) + } + existingTwinValues = true + } else { + existingTwinValues = twinHasValues + } + return fakeMigration({ + ...opts, + history: [ + ...(opts.history ?? []), + reverseEvent('decrypt_dual_writing', 'decrypt-dual-writing', { + details: { + plaintextColumn: 'email', + ...(existingTwinValues ? { existingTwinValues: true } : {}), + ...(existingTwinValues && waiver !== undefined ? { waiver } : {}), + }, + }), + ], + }) + }, + ) fake.recordDropGenerated.mockImplementation(async () => fake) fake.recordSourceRemoved.mockImplementation(async () => fake) return fake as unknown as FakeMigration diff --git a/languages/typescript/packages/cli/src/commands/decrypt/__tests__/resolve-pair.test.ts b/languages/typescript/packages/cli/src/commands/decrypt/__tests__/resolve-pair.test.ts index ed08bbe49..bcb4be208 100644 --- a/languages/typescript/packages/cli/src/commands/decrypt/__tests__/resolve-pair.test.ts +++ b/languages/typescript/packages/cli/src/commands/decrypt/__tests__/resolve-pair.test.ts @@ -271,6 +271,19 @@ describe('resolveDecryptPair', () => { expect(error.message).toContain('Drop --plaintext-column') }) + it('does not suggest the recorded column when nothing is recorded', async () => { + const error = await failure( + resolveDecryptPair(db, { + table: 'users', + encryptedColumn: 'email_encrypted', + plaintextColumn: 'email_encrypted', + }), + ) + expect(error.message).toContain('cannot be email_encrypted itself') + expect(error.message).not.toContain('recorded column') + expect(error.message).toContain('Pass a different --plaintext-column') + }) + it('ignores forward history under the same key — only reverse events pair', async () => { events = [ { diff --git a/languages/typescript/packages/cli/src/commands/decrypt/lib/resolve-pair.ts b/languages/typescript/packages/cli/src/commands/decrypt/lib/resolve-pair.ts index ad480ba19..9492579f2 100644 --- a/languages/typescript/packages/cli/src/commands/decrypt/lib/resolve-pair.ts +++ b/languages/typescript/packages/cli/src/commands/decrypt/lib/resolve-pair.ts @@ -107,11 +107,15 @@ export async function resolveDecryptPair( }) } catch (error) { if (!(error instanceof ColumnMigrationError)) throw error - throw new BackfillConfigError( - opts.plaintextColumn === undefined - ? `${error.message} Pass --plaintext-column .` - : `${error.message} Drop --plaintext-column to continue with the recorded column.`, - ) + let hint: string + if (opts.plaintextColumn === undefined) { + hint = 'Pass --plaintext-column .' + } else if (opts.plaintextColumn === encryptedColumn) { + hint = 'Pass a different --plaintext-column.' + } else { + hint = 'Drop --plaintext-column to continue with the recorded column.' + } + throw new BackfillConfigError(`${error.message} ${hint}`) } const plaintextColumn = migration.twin.column diff --git a/languages/typescript/packages/cli/src/commands/decrypt/status.ts b/languages/typescript/packages/cli/src/commands/decrypt/status.ts index a61108515..5e03123f9 100644 --- a/languages/typescript/packages/cli/src/commands/decrypt/status.ts +++ b/languages/typescript/packages/cli/src/commands/decrypt/status.ts @@ -142,7 +142,8 @@ async function goneRows( direction: 'reverse', manifest: null, }) - if (migration.latest === null) continue + // No history, or finished: a drop applied and the pair migrated back. + if (migration.phase === null) continue const plaintext = migration.recordedTwin ?? '—' rows.push( renderRow({ diff --git a/languages/typescript/packages/migrate/src/__tests__/backfill.integration.test.ts b/languages/typescript/packages/migrate/src/__tests__/backfill.integration.test.ts index b44c4ab02..59aa0a5ed 100644 --- a/languages/typescript/packages/migrate/src/__tests__/backfill.integration.test.ts +++ b/languages/typescript/packages/migrate/src/__tests__/backfill.integration.test.ts @@ -559,8 +559,10 @@ describe.skipIf(!runIntegration)('runBackfill (integration)', () => { }) it('the engine records the twin itself, so a paused run is not resumed into another', async () => { await seed(4) - const run = async (twinColumn: string, signal?: AbortSignal) => { + const firstAfter: Array = [] + const run = async (twinColumn: string, controller?: AbortController) => { const db = await pool.connect() + let fetched = false try { return await runChunkedMigration<{ pk: string }>({ db, @@ -569,7 +571,9 @@ describe.skipIf(!runIntegration)('runBackfill (integration)', () => { twinColumn, direction: 'forward', chunkSize: 2, - signal, + signal: controller?.signal, + // Pause once the first chunk has committed. + onProgress: () => controller?.abort(), countPending: async (after) => Number( ( @@ -580,6 +584,8 @@ describe.skipIf(!runIntegration)('runBackfill (integration)', () => { ).rows[0]?.n, ), fetchPage: async (after, limit) => { + if (!fetched) firstAfter.push(after) + fetched = true const { rows } = await db.query<{ pk: string }>( 'SELECT id::text AS pk FROM migrate_test.users WHERE id > $1 ORDER BY id LIMIT $2', [after ?? 0, limit], @@ -593,21 +599,28 @@ describe.skipIf(!runIntegration)('runBackfill (integration)', () => { } } - // Pause after the first chunk, with no baseDetails naming the twin. - const controller = new AbortController() - controller.abort() - const paused = await run('email_encrypted', controller.signal) + // Paused after its first chunk, with no baseDetails naming the twin. + const paused = await run('email_encrypted', new AbortController()) expect(paused.completed).toBe(false) - const { rows: recorded } = await pool.query<{ details: unknown }>( - `SELECT details FROM cipherstash.cs_migrations + const { rows: recorded } = await pool.query<{ + cursor_value: string | null + details: unknown + }>( + `SELECT cursor_value, details FROM cipherstash.cs_migrations WHERE column_name = 'email' ORDER BY id DESC LIMIT 1`, ) + expect(recorded[0]?.cursor_value).toBe('2') expect(recorded[0]?.details).toMatchObject({ encryptedColumn: 'email_encrypted', }) + // Another twin is refused before it can resume the paused cursor… await expect(run('email_ciphertext')).rejects.toMatchObject({ code: 'twin-mismatch', }) + // …while the recorded one resumes after it. + const resumed = await run('email_encrypted') + expect(resumed.completed).toBe(true) + expect(firstAfter).toEqual([null, '2']) }) }) diff --git a/languages/typescript/packages/migrate/src/__tests__/column-migration.test.ts b/languages/typescript/packages/migrate/src/__tests__/column-migration.test.ts index f4120773c..85826c899 100644 --- a/languages/typescript/packages/migrate/src/__tests__/column-migration.test.ts +++ b/languages/typescript/packages/migrate/src/__tests__/column-migration.test.ts @@ -244,6 +244,30 @@ describe('deriveColumnMigration (reverse)', () => { }).phase, ).toBe('encrypted-dropped') }) + + it('a forward drop superseded by a later reverse column migration of its twin is history', () => { + // email encrypted into email_encrypted and dropped, then decrypted back + // into email and email_encrypted dropped: encrypting email again starts + // a new column migration, not the dropped one. + const forwardTwin = { encryptedColumn: 'email_encrypted' } + const history = [ + ev('dual_writing', 'dual-writing', forwardTwin, { columnName: 'email' }), + ev('backfilled', 'backfilled', forwardTwin, { columnName: 'email' }), + ev('dropped', 'dropped', forwardTwin, { columnName: 'email' }), + ] + const reverseOfTwin = [confirm(), completed(unforced), dropGenerated()] + const state = deriveColumnMigration({ + direction: 'forward', + history, + recordedTwinHistory: reverseOfTwin, + }) + expect(state.phase).toBeNull() + expect(state.open).toBe(false) + expect(state.recordedTwin).toBeUndefined() + expect(deriveColumnMigration({ direction: 'forward', history }).phase).toBe( + 'dropped', + ) + }) }) describe('deriveColumnMigration (reverse, events from an earlier release)', () => { diff --git a/languages/typescript/packages/migrate/src/__tests__/decrypt.integration.test.ts b/languages/typescript/packages/migrate/src/__tests__/decrypt.integration.test.ts index 71503921e..37d89f5b6 100644 --- a/languages/typescript/packages/migrate/src/__tests__/decrypt.integration.test.ts +++ b/languages/typescript/packages/migrate/src/__tests__/decrypt.integration.test.ts @@ -461,6 +461,53 @@ describe.skipIf(!runIntegration)('runDecryptBackfill (integration)', () => { expect(await plaintextCount()).toBe(0) }) + it('encrypting a column again after a full round trip starts a new forward column migration', async () => { + await seed(10) + const db = await pool.connect() + try { + // email encrypted into email_encrypted and dropped, then decrypted back + // into email and email_encrypted's drop generated. + const forwardTwin = { encryptedColumn: 'email_encrypted' } + for (const [event, phase] of [ + ['dual_writing', 'dual-writing'], + ['backfilled', 'backfilled'], + ['dropped', 'dropped'], + ] as const) { + await appendEvent(db, { + tableName: TABLE, + columnName: 'email', + event, + phase, + details: forwardTwin, + }) + } + await confirm() + await run() + const reverse = await ColumnMigration.load(db, { + table: TABLE, + column: 'email_encrypted', + direction: 'reverse', + manifest: null, + }) + await reverse.recordDropGenerated({ migrationFile: 'x.sql' }) + + const forward = await ColumnMigration.load(db, { + table: TABLE, + column: 'email', + direction: 'forward', + twin: 'email_encrypted_v2', + manifest: null, + }) + expect(forward.phase).toBeNull() + const confirmed = await forward.confirmDualWrites() + expect(confirmed.phase).toBe('dual-writing') + expect(confirmed.latest?.event).toBe('dual_writing') + expect(confirmed.twin.column).toBe('email_encrypted_v2') + } finally { + db.release() + } + }) + it('refuses a run into a different plaintext column than the one recorded', async () => { await seed(10) await confirm() diff --git a/languages/typescript/packages/migrate/src/column-migration.ts b/languages/typescript/packages/migrate/src/column-migration.ts index db7c5f2da..1a8fa8c33 100644 --- a/languages/typescript/packages/migrate/src/column-migration.ts +++ b/languages/typescript/packages/migrate/src/column-migration.ts @@ -142,9 +142,10 @@ export interface ColumnMigrationState { * `history` is the column's events in `id` order (any subset that contains * every non-checkpoint event and the latest checkpoint). Events of the other * direction are ignored. `recordedTwinHistory` is the events under the twin - * the latest event names: for reverse, a forward column migration of the twin - * recorded after a generated drop means the drop was applied and the column - * was encrypted again, so the reverse history is finished. + * the latest event names: a column migration of the twin in the other + * direction, recorded after a drop, means the drop was applied and the pair + * was migrated back (for reverse, the column encrypted again; forward, the + * column decrypted again), so this history is finished. * * Obligations are scoped to the column migration in progress: the events * since its latest dual-write confirmation (all of them when a pass ran @@ -172,7 +173,7 @@ export function deriveColumnMigration(input: { if (latest === null) return none const recordedTwin = twinNamedBy(vocab, latest) - if (latest.phase === vocab.droppedPhase && direction === 'reverse') { + if (latest.phase === vocab.droppedPhase) { const superseded = ofDirection( OPPOSITE[direction], input.recordedTwinHistory ?? [], diff --git a/skills/stash-encryption/SKILL.md b/skills/stash-encryption/SKILL.md index 936584ad7..3daf57211 100644 --- a/skills/stash-encryption/SKILL.md +++ b/skills/stash-encryption/SKILL.md @@ -1050,7 +1050,7 @@ await runBackfill({ }) ``` -Useful when the backfill needs to run in a worker, on a schedule, or alongside an existing job runner. The engine applies the same rules as the CLI, whoever calls it: it throws a `ColumnMigrationError` while a reverse migration of the same columns is open; a run that follows an interrupted `force: true` run is forced too, until a forced pass completes (the result's `forcedBy` says why); a failed run resumes from its last committed position; and an `error` event records the error's class (`errorName`) and safe details, never its message. `ColumnMigration.load()` from `@cipherstash/migrate` answers the same questions (phase, twin, owed forced pass) before a run. `runBackfill` is version-agnostic — for an EQL v3 column pass an `Encryption` client built from a v3 schema set and it writes v3 envelopes straight into the concrete `eql_v3_*` domain column. +Useful when the backfill needs to run in a worker, on a schedule, or alongside an existing job runner. The engine applies the same rules as the CLI, whoever calls it: it throws a `ColumnMigrationError` while a reverse migration of the same columns is open; a run that follows an interrupted `force: true` run is forced too, until a forced pass completes (the result's `forcedBy` says why); a failed run resumes from its last committed position; and an `error` event records the error's class (`errorName`) and safe details, never its message. it throws a `ColumnMigrationError` with code `twin-mismatch` when `encryptedColumn` differs from the encrypted column recorded for that column. `ColumnMigration.load()` from `@cipherstash/migrate` answers the same questions (phase, twin, owed forced pass) before a run. A column whose migration was dropped and then migrated back the other way starts a new column migration. `runBackfill` is version-agnostic — for an EQL v3 column pass an `Encryption` client built from a v3 schema set and it writes v3 envelopes straight into the concrete `eql_v3_*` domain column. ### Invariants the rollout preserves From 6a7230eec97ced53dd1f8c6e081b0f58c86edf21 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 01:08:11 +0000 Subject: [PATCH 10/15] fix(migrate): name the twin on an observed-absent drop recordSourceRemoved wrote its dropped event without the twin, so a forward migration closed that way could not be superseded by the reverse migration of its twin, and re-encrypting the column later skipped its dual-write confirmation. The stash-cli skill no longer says concurrent encrypt backfill runners converge: nothing serializes them. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01Scub1tpuegHHCTXrgn7SHf --- .../src/__tests__/decrypt.integration.test.ts | 48 +++++++++++++++++++ .../packages/migrate/src/column-migration.ts | 6 ++- skills/stash-cli/SKILL.md | 2 +- 3 files changed, 54 insertions(+), 2 deletions(-) diff --git a/languages/typescript/packages/migrate/src/__tests__/decrypt.integration.test.ts b/languages/typescript/packages/migrate/src/__tests__/decrypt.integration.test.ts index 37d89f5b6..154a6bb31 100644 --- a/languages/typescript/packages/migrate/src/__tests__/decrypt.integration.test.ts +++ b/languages/typescript/packages/migrate/src/__tests__/decrypt.integration.test.ts @@ -508,6 +508,54 @@ describe.skipIf(!runIntegration)('runDecryptBackfill (integration)', () => { } }) + it('a forward migration closed as observed absent is superseded by the reverse one too', async () => { + await seed(10) + const db = await pool.connect() + try { + // email's forward migration was open when its drop happened outside + // stash; `decrypt plan` records that before the reverse one starts. + await appendEvent(db, { + tableName: TABLE, + columnName: 'email', + event: 'dual_writing', + phase: 'dual-writing', + details: { encryptedColumn: 'email_encrypted' }, + }) + const open = await ColumnMigration.load(db, { + table: TABLE, + column: 'email', + direction: 'forward', + manifest: null, + }) + const closed = await open.recordSourceRemoved() + expect(closed.latest?.details).toMatchObject({ + encryptedColumn: 'email_encrypted', + observedAbsent: true, + }) + + await confirm() + await run() + const reverse = await ColumnMigration.load(db, { + table: TABLE, + column: 'email_encrypted', + direction: 'reverse', + manifest: null, + }) + await reverse.recordDropGenerated({ migrationFile: 'x.sql' }) + + const again = await ColumnMigration.load(db, { + table: TABLE, + column: 'email', + direction: 'forward', + twin: 'email_encrypted_v2', + manifest: null, + }) + expect(again.phase).toBeNull() + } finally { + db.release() + } + }) + it('refuses a run into a different plaintext column than the one recorded', async () => { await seed(10) await confirm() diff --git a/languages/typescript/packages/migrate/src/column-migration.ts b/languages/typescript/packages/migrate/src/column-migration.ts index 1a8fa8c33..46923d034 100644 --- a/languages/typescript/packages/migrate/src/column-migration.ts +++ b/languages/typescript/packages/migrate/src/column-migration.ts @@ -618,7 +618,11 @@ export class ColumnMigration implements ColumnMigrationState { columnName: this.column, event: vocab.dropped, phase: vocab.droppedPhase, - details: { observedAbsent: true, ...details }, + details: { + [vocab.twinKey]: this.twin.column, + observedAbsent: true, + ...details, + }, }) return this.reload() } diff --git a/skills/stash-cli/SKILL.md b/skills/stash-cli/SKILL.md index 77336726a..f45c156bf 100644 --- a/skills/stash-cli/SKILL.md +++ b/skills/stash-cli/SKILL.md @@ -613,7 +613,7 @@ stash encrypt backfill --table users --column email stash encrypt backfill --table users --column email --chunk-size 5000 ``` -Chunked, resumable, idempotent. Walks the table in keyset-pagination order, encrypts each chunk via `bulkEncryptModels`, and writes one `UPDATE ... FROM (VALUES ...)` per chunk in a transaction that also checkpoints to `cs_migrations`. SIGINT/SIGTERM finishes the current chunk and exits cleanly; re-running resumes, from the last checkpoint or, after a failed run, from the last committed position. A failure is recorded in `cs_migrations` by error class and safe details only, never the error message, which can carry row data. The ` IS NOT NULL AND _encrypted IS NULL` guard makes concurrent runners and re-runs converge. +Chunked, resumable, idempotent. Walks the table in keyset-pagination order, encrypts each chunk via `bulkEncryptModels`, and writes one `UPDATE ... FROM (VALUES ...)` per chunk in a transaction that also checkpoints to `cs_migrations`. SIGINT/SIGTERM finishes the current chunk and exits cleanly; re-running resumes, from the last checkpoint or, after a failed run, from the last committed position. A failure is recorded in `cs_migrations` by error class and safe details only, never the error message, which can carry row data. The ` IS NOT NULL AND _encrypted IS NULL` guard makes re-runs converge. Run one backfill per column at a time: nothing serializes two runners, and the slower one's events can leave `cs_migrations` reporting stale progress. Backfill requires a `public.eql_v3_*` target column, records version 3 and the `dropped` target phase in `.cipherstash/migrations.json`, then prints the next steps: switch the application to the encrypted column by name and run `stash encrypt drop`. A missing, plaintext, or legacy v2 target is rejected before encryption begins. From b26a0091d10f82492b41550418a57ab650647880 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 05:23:12 +0000 Subject: [PATCH 11/15] fix(migrate): keep the engine's resume fields over caller details A caller's checkpointDetails or errorDetails was spread after the engine's twin, force and rowsSkipped, so it could overwrite the fields a resume reads. The engine's fields now go last. A cs_migrations row whose jsonb details is a scalar no longer throws in passForced. Adds the tests the review asked for: recordDropGenerated and decrypt drop refusing while a forced pass is owed, the forward waiver refusal, and ColumnMigration.load reading a missing schema as no history while rethrowing any other error. Also makes DirectionVocabulary readonly, makes forcedReason exhaustive, and corrects the PlaintextWriteError comment. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01Scub1tpuegHHCTXrgn7SHf --- .../__tests__/decrypt-backfill.test.ts | 21 +++++- .../__tests__/decrypt-drop-plan.test.ts | 30 +++++++++ .../cli/src/commands/decrypt/backfill.ts | 7 +- .../__tests__/backfill.integration.test.ts | 52 +++++++++++++++ .../src/__tests__/column-migration.test.ts | 53 ++++++++++++++- .../src/__tests__/decrypt.integration.test.ts | 66 +++++++++++++++++++ .../packages/migrate/src/column-migration.ts | 25 +++---- .../packages/migrate/src/decrypt.ts | 6 +- .../typescript/packages/migrate/src/engine.ts | 10 +-- 9 files changed, 249 insertions(+), 21 deletions(-) diff --git a/languages/typescript/packages/cli/src/commands/decrypt/__tests__/decrypt-backfill.test.ts b/languages/typescript/packages/cli/src/commands/decrypt/__tests__/decrypt-backfill.test.ts index 9425bfda5..5447390b5 100644 --- a/languages/typescript/packages/cli/src/commands/decrypt/__tests__/decrypt-backfill.test.ts +++ b/languages/typescript/packages/cli/src/commands/decrypt/__tests__/decrypt-backfill.test.ts @@ -92,7 +92,11 @@ vi.mock('@/commands/init/utils.js', () => ({ })) import * as p from '@clack/prompts' -import { decryptBackfillCommand, skippedReport } from '../backfill.js' +import { + decryptBackfillCommand, + forcedReason, + skippedReport, +} from '../backfill.js' function pair( overrides: Partial> & { @@ -594,3 +598,18 @@ describe('skippedReport', () => { expect(report).not.toContain('lock context') }) }) + +describe('forcedReason', () => { + const names = { encryptedColumn: 'email_encrypted', plaintextColumn: 'email' } + + it('explains a pass forced by values the twin held before the reversal', () => { + const reason = forcedReason('existing-values', 'users', names) + expect(reason).toContain('users.email held values') + expect(reason).toContain('--confirm-plaintext-current') + }) + + it('says nothing for a pass the caller forced, or an unforced one', () => { + expect(forcedReason('caller', 'users', names)).toBeNull() + expect(forcedReason(null, 'users', names)).toBeNull() + }) +}) diff --git a/languages/typescript/packages/cli/src/commands/decrypt/__tests__/decrypt-drop-plan.test.ts b/languages/typescript/packages/cli/src/commands/decrypt/__tests__/decrypt-drop-plan.test.ts index fcf015484..8fb38ed5e 100644 --- a/languages/typescript/packages/cli/src/commands/decrypt/__tests__/decrypt-drop-plan.test.ts +++ b/languages/typescript/packages/cli/src/commands/decrypt/__tests__/decrypt-drop-plan.test.ts @@ -246,6 +246,36 @@ describe('decrypt drop', () => { exitSpy.mockRestore() }) + it("refuses at 'decrypted' while a forced pass is still owed", async () => { + const twin = { plaintextColumn: 'email' } + const owing = pair({ + migration: fakeMigration({ + history: [ + reverseEvent('decrypt_dual_writing', 'decrypt-dual-writing'), + reverseEvent('decrypt_started', 'decrypting', { + details: { ...twin, force: true }, + }), + reverseEvent('decrypted', 'decrypted', { + details: { ...twin, force: false }, + }), + ], + }), + }) + expect(owing.migration.phase).toBe('decrypted') + resolveMock.mockResolvedValue(owing) + const exitSpy = spyExit() + + await decryptDropCommand({ table: 'users', column: 'email_encrypted' }) + + expect(p.log.error).toHaveBeenCalledWith( + expect.stringContaining('still owes a forced pass'), + ) + expect(writeFileMock).not.toHaveBeenCalled() + expect(owing.migration.recordDropGenerated).not.toHaveBeenCalled() + expect(exitSpy).toHaveBeenCalledWith(1) + exitSpy.mockRestore() + }) + it('refuses without a RECORDED pairing, even when a twin name could be guessed', async () => { resolveMock.mockResolvedValue( pair({ diff --git a/languages/typescript/packages/cli/src/commands/decrypt/backfill.ts b/languages/typescript/packages/cli/src/commands/decrypt/backfill.ts index fe6be8d29..2336c6a21 100644 --- a/languages/typescript/packages/cli/src/commands/decrypt/backfill.ts +++ b/languages/typescript/packages/cli/src/commands/decrypt/backfill.ts @@ -390,8 +390,13 @@ export function forcedReason( return `This run is a forced pass: a forced pass over ${table}.${pair.plaintextColumn} started earlier and has not completed, so ${pair.plaintextColumn} may hold a mix of overwritten and stale values. Every run overwrites it from ${pair.encryptedColumn} until one completes.` case 'existing-values': return `This run is a forced pass: ${table}.${pair.plaintextColumn} held values before this reverse migration began and no waiver (--confirm-plaintext-current) is recorded, so it overwrites them from ${pair.encryptedColumn}.` - default: + case 'caller': + case null: return null + default: { + const unhandled: never = forcedBy + return unhandled + } } } diff --git a/languages/typescript/packages/migrate/src/__tests__/backfill.integration.test.ts b/languages/typescript/packages/migrate/src/__tests__/backfill.integration.test.ts index 59aa0a5ed..cabce68cf 100644 --- a/languages/typescript/packages/migrate/src/__tests__/backfill.integration.test.ts +++ b/languages/typescript/packages/migrate/src/__tests__/backfill.integration.test.ts @@ -623,4 +623,56 @@ describe.skipIf(!runIntegration)('runBackfill (integration)', () => { expect(resumed.completed).toBe(true) expect(firstAfter).toEqual([null, '2']) }) + + it("a caller's checkpoint and error details cannot overwrite the fields a resume reads", async () => { + await seed(4) + const spoof = { encryptedColumn: 'spoofed', force: true, rowsSkipped: 99 } + const db = await pool.connect() + let chunk = 0 + try { + await expect( + runChunkedMigration<{ pk: string }>({ + db, + tableName: 'migrate_test.users', + stateColumn: 'email', + twinColumn: 'email_encrypted', + direction: 'forward', + chunkSize: 2, + errorDetails: () => spoof, + countPending: async () => 4, + fetchPage: async (after, limit) => { + const { rows } = await db.query<{ pk: string }>( + 'SELECT id::text AS pk FROM migrate_test.users WHERE id > $1 ORDER BY id LIMIT $2', + [after ?? 0, limit], + ) + return { rows, lastPk: rows.at(-1)?.pk ?? null } + }, + prepareChunk: async () => { + chunk += 1 + if (chunk === 2) throw new Error('second chunk fails') + return { checkpointDetails: spoof, write: async () => {} } + }, + }), + ).rejects.toThrow('second chunk fails') + } finally { + db.release() + } + + const { rows } = await pool.query<{ + event: string + details: Record + }>( + `SELECT event, details FROM cipherstash.cs_migrations + WHERE column_name = 'email' AND event IN ('backfill_checkpoint', 'error') + ORDER BY id`, + ) + expect(rows.map((r) => r.event)).toEqual(['backfill_checkpoint', 'error']) + for (const { details } of rows) { + expect(details).toMatchObject({ + encryptedColumn: 'email_encrypted', + force: false, + rowsSkipped: 0, + }) + } + }) }) diff --git a/languages/typescript/packages/migrate/src/__tests__/column-migration.test.ts b/languages/typescript/packages/migrate/src/__tests__/column-migration.test.ts index 85826c899..137c1dad1 100644 --- a/languages/typescript/packages/migrate/src/__tests__/column-migration.test.ts +++ b/languages/typescript/packages/migrate/src/__tests__/column-migration.test.ts @@ -4,8 +4,10 @@ * the `cs_migrations` rows `ColumnMigration.load` would read. */ -import { describe, expect, it } from 'vitest' +import type { ClientBase } from 'pg' +import { describe, expect, it, vi } from 'vitest' import { + ColumnMigration, type ColumnMigrationState, deriveColumnMigration, deriveTwinName, @@ -363,6 +365,55 @@ describe('deriveColumnMigration (forward)', () => { }) expect(state.phase).toBeNull() }) + + it('a completion whose details are a JSON scalar takes the mode of its pass', () => { + // `details` is jsonb, so a hand-written row can hold a string. + const scalar = 'x' as unknown as Record + const state = deriveColumnMigration({ + direction: 'forward', + history: [ + fwd('dual_writing', 'dual-writing'), + fwd('backfill_started', 'backfilling', { force: true }), + fwd('backfilled', 'backfilled', scalar), + ], + }) + expect(state.phase).toBe('backfilled') + expect(state.obligations).toEqual([]) + }) +}) + +describe('ColumnMigration.load without cs_migrations', () => { + const failingDb = (error: unknown) => + ({ query: vi.fn().mockRejectedValue(error) }) as unknown as ClientBase + + it.each([ + ['42P01', 'undefined_table'], + ['3F000', 'invalid_schema_name'], + ])('reads %s (%s) as no history', async (code) => { + const migration = await ColumnMigration.load( + failingDb(Object.assign(new Error('not installed'), { code })), + { + table: 'users', + column: 'email_encrypted', + direction: 'reverse', + twin: 'email', + }, + ) + expect(migration.phase).toBeNull() + expect(migration.obligations).toEqual([]) + }) + + it('rethrows any other error', async () => { + const lost = new Error('connection lost') + await expect( + ColumnMigration.load(failingDb(lost), { + table: 'users', + column: 'email_encrypted', + direction: 'reverse', + twin: 'email', + }), + ).rejects.toBe(lost) + }) }) describe('openCounterpart', () => { diff --git a/languages/typescript/packages/migrate/src/__tests__/decrypt.integration.test.ts b/languages/typescript/packages/migrate/src/__tests__/decrypt.integration.test.ts index 154a6bb31..bfa4f1159 100644 --- a/languages/typescript/packages/migrate/src/__tests__/decrypt.integration.test.ts +++ b/languages/typescript/packages/migrate/src/__tests__/decrypt.integration.test.ts @@ -600,6 +600,72 @@ describe.skipIf(!runIntegration)('runDecryptBackfill (integration)', () => { } }) + it('refuses to generate the drop while a forced pass is owed', async () => { + await seed(10) + const db = await pool.connect() + try { + // A forced pass started, then an unforced one completed: still owed. + for (const [event, phase, force] of [ + ['decrypt_dual_writing', 'decrypt-dual-writing', undefined], + ['decrypt_started', 'decrypting', true], + ['decrypted', 'decrypted', false], + ] as const) { + await appendEvent(db, { + tableName: TABLE, + columnName: 'email_encrypted', + event, + phase, + details: { + plaintextColumn: 'email', + ...(force === undefined ? {} : { force }), + }, + }) + } + const owing = await ColumnMigration.load(db, { + table: TABLE, + column: 'email_encrypted', + direction: 'reverse', + manifest: null, + }) + await expect(owing.recordDropGenerated()).rejects.toMatchObject({ + code: 'obligation-open', + }) + const after = await ColumnMigration.load(db, { + table: TABLE, + column: 'email_encrypted', + direction: 'reverse', + manifest: null, + }) + expect(after.phase).toBe('decrypted') + } finally { + db.release() + } + }) + + it('refuses a waiver on a forward column migration, recording nothing', async () => { + await seed(10) + const db = await pool.connect() + try { + const forward = await ColumnMigration.load(db, { + table: TABLE, + column: 'email', + direction: 'forward', + twin: 'email_encrypted', + manifest: null, + }) + await expect( + forward.confirmDualWrites({ waiver: 'plaintext-current' }), + ).rejects.toMatchObject({ code: 'waiver-refused' }) + const { rows } = await db.query( + 'SELECT 1 FROM cipherstash.cs_migrations WHERE column_name = $1', + ['email'], + ) + expect(rows).toEqual([]) + } finally { + db.release() + } + }) + it('a completed forced pass meets the obligation: later plain runs are unforced again', async () => { await seed(300) await pool.query(`UPDATE ${TABLE} SET email = 'stale@example.com'`) diff --git a/languages/typescript/packages/migrate/src/column-migration.ts b/languages/typescript/packages/migrate/src/column-migration.ts index 46923d034..016753c60 100644 --- a/languages/typescript/packages/migrate/src/column-migration.ts +++ b/languages/typescript/packages/migrate/src/column-migration.ts @@ -24,20 +24,20 @@ export type MigrationDirection = 'forward' | 'reverse' */ export interface DirectionVocabulary { /** Starts the column migration: dual-writes are confirmed deployed. */ - dualWriting: MigrationEvent - dualWritingPhase: MigrationPhase - started: MigrationEvent + readonly dualWriting: MigrationEvent + readonly dualWritingPhase: MigrationPhase + readonly started: MigrationEvent /** The event a resume looks for. Its `cursorValue` is the restart point. */ - checkpoint: MigrationEvent - completed: MigrationEvent + readonly checkpoint: MigrationEvent + readonly completed: MigrationEvent /** Phase recorded on `started`, every `checkpoint`, and any `error`. */ - runningPhase: MigrationPhase - completedPhase: MigrationPhase + readonly runningPhase: MigrationPhase + readonly completedPhase: MigrationPhase /** Recorded when the drop of the source column is generated. */ - dropped: MigrationEvent - droppedPhase: MigrationPhase + readonly dropped: MigrationEvent + readonly droppedPhase: MigrationPhase /** The `details` key every event uses to name the twin, when it does. */ - twinKey: string + readonly twinKey: string } export const DIRECTION_VOCABULARY: Readonly< @@ -702,7 +702,10 @@ function isForced(row: MigrationStateRow): boolean { * key takes the mode of the pass started before it. */ function passForced(row: MigrationStateRow, startedForced: boolean): boolean { - if (row.details !== null && 'force' in row.details) return isForced(row) + // `details` is jsonb: a hand-written scalar must not throw on `in`. + if (typeof row.details === 'object' && row.details !== null) { + if ('force' in row.details) return isForced(row) + } return startedForced } diff --git a/languages/typescript/packages/migrate/src/decrypt.ts b/languages/typescript/packages/migrate/src/decrypt.ts index 65157fbaa..4a9e8f1ce 100644 --- a/languages/typescript/packages/migrate/src/decrypt.ts +++ b/languages/typescript/packages/migrate/src/decrypt.ts @@ -76,9 +76,9 @@ export class UndecryptableBatchError extends Error { * * Postgres puts the offending INPUT in the text of a cast error * (`invalid input syntax for type integer: "…"`), and on this path the input - * is decrypted plaintext. The engine persists an error's message into the - * `cs_migrations` `error` event, so the database's message must never reach - * it. This error carries only the SQLSTATE, the constraint name when there is + * is decrypted plaintext. The engine's `error` event stores only the error's + * name and the safe details this direction supplies, never a message, and + * this error must not carry the database's message either. It carries only the SQLSTATE, the constraint name when there is * one, and the column and type names; the original error is deliberately not * kept as `cause`. */ diff --git a/languages/typescript/packages/migrate/src/engine.ts b/languages/typescript/packages/migrate/src/engine.ts index 0cfb42f36..4dbdcd21b 100644 --- a/languages/typescript/packages/migrate/src/engine.ts +++ b/languages/typescript/packages/migrate/src/engine.ts @@ -169,8 +169,9 @@ export async function runChunkedMigration( const plan = await migration.planPass({ force: options.force ?? false }) const { force, forcedBy } = plan const mode: PassMode = { force } - // The pairing is recorded on every event whatever the caller passes, so a - // later run with a different twin is refused rather than resumed. + // The pairing and mode are recorded on every event whatever the caller + // passes, so a later run with a different twin is refused rather than + // resumed. Caller details are spread before them on every event. const base = { ...options.baseDetails, [events.twinKey]: options.twinColumn, @@ -240,13 +241,14 @@ export async function runChunkedMigration( cursorValue: page.lastPk, rowsProcessed: nextProcessed, rowsTotal, + // The caller's fields go first: a resume reads the engine's. details: { + ...prepared.checkpointDetails, ...base, chunkIndex, chunkRows: page.rows.length, chunkSkipped, rowsSkipped: nextSkipped, - ...prepared.checkpointDetails, }, }) await db.query('COMMIT') @@ -291,8 +293,8 @@ export async function runChunkedMigration( rowsProcessed, rowsTotal, details: { - ...base, ...options.errorDetails?.(err), + ...base, errorName: safeErrorName(err), chunkIndex, rowsSkipped, From bb0c01a8e65c975d5428d92c194c5ce14c94f5f4 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 05:23:56 +0000 Subject: [PATCH 12/15] docs(changeset): one sentence per line Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01Scub1tpuegHHCTXrgn7SHf --- .changeset/column-migration-api.md | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/.changeset/column-migration-api.md b/.changeset/column-migration-api.md index 23dbcdcb5..6a2198ce4 100644 --- a/.changeset/column-migration-api.md +++ b/.changeset/column-migration-api.md @@ -3,8 +3,18 @@ 'stash': patch --- -`@cipherstash/migrate` adds `ColumnMigration`, one place for the rules of a column migration in either direction: which twin column it pairs with (the flag, the recorded pairing, the manifest, then the naming convention), its phase, whether a forced pass is owed, and which open migration in the other direction blocks it. `ColumnMigration.load()` reads it from `cipherstash.cs_migrations`; `confirmDualWrites()`, `recordDropGenerated()` and `recordSourceRemoved()` record events, refusing those the rules forbid with a `ColumnMigrationError`. The pure rules are exported as `deriveColumnMigration`, `openCounterpart`, `planPassFor`, `passProgress`, `deriveTwinName` and `manifestTwin`. +`@cipherstash/migrate` adds `ColumnMigration`, one place for the rules of a column migration in either direction: which twin column it pairs with (the flag, the recorded pairing, the manifest, then the naming convention), its phase, whether a forced pass is owed, and which open migration in the other direction blocks it. +`ColumnMigration.load()` reads it from `cipherstash.cs_migrations`; `confirmDualWrites()`, `recordDropGenerated()` and `recordSourceRemoved()` record events, refusing those the rules forbid with a `ColumnMigrationError`. +The pure rules are exported as `deriveColumnMigration`, `openCounterpart`, `planPassFor`, `passProgress`, `deriveTwinName` and `manifestTwin`. -`runBackfill` and `runDecryptBackfill` now apply the rules themselves rather than leaving them to the caller. Every event records whether its pass was forced. An interrupted forced pass is finished as one by the next run, with or without `force`, and the result says why in `forcedBy`. A forward backfill resumes after a failed run from its last committed position, counts its total once when it resumes a forced pass, and refuses to start while a reverse migration of the same columns is open. `runBackfill` now throws a `ColumnMigrationError` with code `twin-mismatch` when `encryptedColumn` differs from the encrypted column already recorded for that column, rather than continuing into a different one. A reverse backfill over a twin that already holds values is forced unless the caller confirmed dual-writes with the `plaintext-current` waiver. Error events store the error's class (`errorName`) and safe details instead of its message, which could carry row data. +`runBackfill` and `runDecryptBackfill` now apply the rules themselves rather than leaving them to the caller. +Every event records whether its pass was forced. +An interrupted forced pass is finished as one by the next run, with or without `force`, and the result says why in `forcedBy`. +A forward backfill resumes after a failed run from its last committed position, counts its total once when it resumes a forced pass, and refuses to start while a reverse migration of the same columns is open. +`runBackfill` now throws a `ColumnMigrationError` with code `twin-mismatch` when `encryptedColumn` differs from the encrypted column already recorded for that column, rather than continuing into a different one. +A reverse backfill over a twin that already holds values is forced unless the caller confirmed dual-writes with the `plaintext-current` waiver. +Error events store the error's class (`errorName`) and safe details instead of its message, which could carry row data. -`stash decrypt plan`, `status`, `backfill` and `drop` resolve every column through `ColumnMigration`, so they always agree on the twin and the phase. `decrypt status` flags `forward-open`, `twin-has-values` and `forced-pass-owed`. `decrypt backfill --confirm-plaintext-current` is accepted on a later run as long as no forced pass has started, and refused once one has. +`stash decrypt plan`, `status`, `backfill` and `drop` resolve every column through `ColumnMigration`, so they always agree on the twin and the phase. +`decrypt status` flags `forward-open`, `twin-has-values` and `forced-pass-owed`. +`decrypt backfill --confirm-plaintext-current` is accepted on a later run as long as no forced pass has started, and refused once one has. From 97d60b79c45845120943237d6a3bf60396093388 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 05:46:57 +0000 Subject: [PATCH 13/15] fix(migrate): make Obligation and PassPlan discriminated unions waivable now follows from reason, and force from forcedBy, so the types no longer admit states the rules never produce. Type-level tests checked by tsc. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01Pkrkj6eBHR934hMcSGKSHg --- .../src/__tests__/column-migration.test.ts | 59 ++++++++++++++++++- .../packages/migrate/src/column-migration.ts | 32 ++++++---- 2 files changed, 80 insertions(+), 11 deletions(-) diff --git a/languages/typescript/packages/migrate/src/__tests__/column-migration.test.ts b/languages/typescript/packages/migrate/src/__tests__/column-migration.test.ts index 137c1dad1..4a6d3fb9f 100644 --- a/languages/typescript/packages/migrate/src/__tests__/column-migration.test.ts +++ b/languages/typescript/packages/migrate/src/__tests__/column-migration.test.ts @@ -5,14 +5,16 @@ */ import type { ClientBase } from 'pg' -import { describe, expect, it, vi } from 'vitest' +import { describe, expect, expectTypeOf, it, vi } from 'vitest' import { ColumnMigration, type ColumnMigrationState, deriveColumnMigration, deriveTwinName, manifestTwin, + type Obligation, openCounterpart, + type PassPlan, passProgress, planPassFor, } from '../column-migration.js' @@ -270,6 +272,25 @@ describe('deriveColumnMigration (reverse)', () => { 'dropped', ) }) + + it('reverse events of the twin from BEFORE a forward drop leave the drop current', () => { + // An earlier round trip: email_encrypted decrypted back and dropped, then + // email encrypted into email_encrypted again and dropped. + const earlierReverse = [confirm(), completed(unforced), dropGenerated()] + const forwardTwin = { encryptedColumn: 'email_encrypted' } + const history = [ + ev('dual_writing', 'dual-writing', forwardTwin, { columnName: 'email' }), + ev('backfilled', 'backfilled', forwardTwin, { columnName: 'email' }), + ev('dropped', 'dropped', forwardTwin, { columnName: 'email' }), + ] + const state = deriveColumnMigration({ + direction: 'forward', + history, + recordedTwinHistory: earlierReverse, + }) + expect(state.phase).toBe('dropped') + expect(state.recordedTwin).toBe('email_encrypted') + }) }) describe('deriveColumnMigration (reverse, events from an earlier release)', () => { @@ -702,3 +723,39 @@ describe('manifestTwin', () => { ).toBeUndefined() }) }) + +describe('the obligation and pass plan types', () => { + it('admit only the states the rules produce', () => { + // Checked by `tsc`: each assignment marked below would compile if the + // fields were independent. + const neverWaivable = { reason: 'forced-pass', waivable: true } as const + const alwaysWaivable = { + reason: 'existing-values', + waivable: false, + } as const + const base = { kind: 'forced-pass', openedAt: null } as const + // @ts-expect-error a forced-pass obligation is never waivable + const a: Obligation = { ...base, ...neverWaivable } + // @ts-expect-error an existing-values obligation is always waivable + const b: Obligation = { ...base, ...alwaysWaivable } + const unforced = { force: false, forcedBy: 'caller' } as const + const forced = { force: true, forcedBy: null } as const + // @ts-expect-error an unforced plan has no reason + const c: PassPlan = { ...unforced, resumeFrom: null } + // @ts-expect-error a forced plan always has one + const d: PassPlan = { ...forced, resumeFrom: null } + expect([a, b, c, d]).toHaveLength(4) + + const plan = planPassFor( + deriveColumnMigration({ direction: 'reverse', history: [] }), + { force: false }, + ) + if (plan.force) { + expectTypeOf(plan.forcedBy).toEqualTypeOf< + 'caller' | Obligation['reason'] + >() + } else { + expectTypeOf(plan.forcedBy).toEqualTypeOf() + } + }) +}) diff --git a/languages/typescript/packages/migrate/src/column-migration.ts b/languages/typescript/packages/migrate/src/column-migration.ts index 016753c60..425234444 100644 --- a/languages/typescript/packages/migrate/src/column-migration.ts +++ b/languages/typescript/packages/migrate/src/column-migration.ts @@ -86,14 +86,22 @@ const OPPOSITE: Record = { * - `forced-pass`: a forced pass started. Only a completed forced pass closes * it. */ -export interface Obligation { +export type Obligation = { kind: 'forced-pass' - reason: 'existing-values' | 'forced-pass' /** Id of the event that opened it, or `null` when no event records it yet. */ openedAt: string | null - /** Whether a waiver may still close it. */ - waivable: boolean -} +} & ( + | { + reason: 'existing-values' + /** Whether a waiver may still close it: always, for this reason. */ + waivable: true + } + | { + reason: 'forced-pass' + /** Whether a waiver may still close it: never, for this reason. */ + waivable: false + } +) /** * The operator's recorded assertion that a twin's existing values are already @@ -280,15 +288,16 @@ export function passProgress(state: ColumnMigrationState): PassProgress | null { } /** How the next run will go: its mode, why, and where it resumes. */ -export interface PassPlan { - force: boolean - forcedBy: ForcedBy +export type PassPlan = { /** * The event the run resumes from (a checkpoint, or an error carrying the * last committed cursor), or `null` for a fresh pass from the first row. */ resumeFrom: MigrationStateRow | null -} +} & ( + | { force: false; forcedBy: null } + | { force: true; forcedBy: NonNullable } +) /** * Plan the next run of a column migration. An open obligation forces it. A @@ -326,7 +335,10 @@ export function planPassFor( last.phase === vocab.runningPhase && last.cursorValue !== null)) && isForced(last) === force - return { force, forcedBy, resumeFrom: resumable ? last : null } + const resumeFrom = resumable ? last : null + return forcedBy === null + ? { force: false, forcedBy, resumeFrom } + : { force: true, forcedBy, resumeFrom } } /** From 1d9fee665364c278a8589bca1cf4731fef311261 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 05:46:57 +0000 Subject: [PATCH 14/15] test(migrate): cover the error-name fallback, batch details, closing a closed migration - safeErrorName records 'Error' for a non-Error throw and for a name that is not an identifier, never the value. - An UndecryptableBatchError event stores batchRows and batchCodes only. - recordSourceRemoved refuses wrong-phase with no open migration, and on a second close, recording nothing. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01Pkrkj6eBHR934hMcSGKSHg --- .../__tests__/backfill.integration.test.ts | 39 ++++++++++++++ .../src/__tests__/decrypt.integration.test.ts | 51 +++++++++++++++++++ 2 files changed, 90 insertions(+) diff --git a/languages/typescript/packages/migrate/src/__tests__/backfill.integration.test.ts b/languages/typescript/packages/migrate/src/__tests__/backfill.integration.test.ts index cabce68cf..e7590d886 100644 --- a/languages/typescript/packages/migrate/src/__tests__/backfill.integration.test.ts +++ b/languages/typescript/packages/migrate/src/__tests__/backfill.integration.test.ts @@ -624,6 +624,45 @@ describe.skipIf(!runIntegration)('runBackfill (integration)', () => { expect(firstAfter).toEqual([null, '2']) }) + it.each([ + ['a thrown value that is not an Error', () => 'row 7: alice@example.com'], + [ + 'an Error whose name is not an identifier', + () => + Object.assign(new Error('boom'), { name: 'alice@example.com failed' }), + ], + ])('records %s as errorName "Error"', async (_label, thrown) => { + await seed(2) + const db = await pool.connect() + try { + await expect( + runChunkedMigration<{ pk: string }>({ + db, + tableName: 'migrate_test.users', + stateColumn: 'email', + twinColumn: 'email_encrypted', + direction: 'forward', + chunkSize: 2, + countPending: async () => 2, + fetchPage: async () => ({ rows: [{ pk: '1' }], lastPk: '1' }), + prepareChunk: async () => { + throw thrown() + }, + }), + ).rejects.toBeDefined() + } finally { + db.release() + } + + const { rows } = await pool.query<{ details: Record }>( + `SELECT details FROM cipherstash.cs_migrations + WHERE column_name = 'email' AND event = 'error'`, + ) + expect(rows).toHaveLength(1) + expect(rows[0]?.details.errorName).toBe('Error') + expect(JSON.stringify(rows[0]?.details)).not.toContain('alice@example.com') + }) + it("a caller's checkpoint and error details cannot overwrite the fields a resume reads", async () => { await seed(4) const spoof = { encryptedColumn: 'spoofed', force: true, rowsSkipped: 99 } diff --git a/languages/typescript/packages/migrate/src/__tests__/decrypt.integration.test.ts b/languages/typescript/packages/migrate/src/__tests__/decrypt.integration.test.ts index bfa4f1159..f90d15217 100644 --- a/languages/typescript/packages/migrate/src/__tests__/decrypt.integration.test.ts +++ b/languages/typescript/packages/migrate/src/__tests__/decrypt.integration.test.ts @@ -508,6 +508,49 @@ describe.skipIf(!runIntegration)('runDecryptBackfill (integration)', () => { } }) + it('refuses to close a column migration that is not open, recording nothing', async () => { + await seed(10) + const db = await pool.connect() + try { + const none = await ColumnMigration.load(db, { + table: TABLE, + column: 'email', + direction: 'forward', + twin: 'email_encrypted', + manifest: null, + }) + await expect(none.recordSourceRemoved()).rejects.toMatchObject({ + code: 'wrong-phase', + }) + + // Closed once, it cannot be closed again. + await appendEvent(db, { + tableName: TABLE, + columnName: 'email', + event: 'dual_writing', + phase: 'dual-writing', + details: { encryptedColumn: 'email_encrypted' }, + }) + const open = await ColumnMigration.load(db, { + table: TABLE, + column: 'email', + direction: 'forward', + manifest: null, + }) + const closed = await open.recordSourceRemoved() + await expect(closed.recordSourceRemoved()).rejects.toMatchObject({ + code: 'wrong-phase', + }) + const { rows } = await db.query<{ event: string }>( + 'SELECT event FROM cipherstash.cs_migrations WHERE column_name = $1 ORDER BY id', + ['email'], + ) + expect(rows.map((r) => r.event)).toEqual(['dual_writing', 'dropped']) + } finally { + db.release() + } + }) + it('a forward migration closed as observed absent is superseded by the reverse one too', async () => { await seed(10) const db = await pool.connect() @@ -916,6 +959,14 @@ describe.skipIf(!runIntegration)('runDecryptBackfill (integration)', () => { expect(state?.event).toBe('error') expect(state?.cursorValue).toBeNull() expect(state?.details).toMatchObject({ plaintextColumn: 'email' }) + // The refused batch is recorded by counts and codes: no value, no + // message. + expect(state?.details).toMatchObject({ + errorName: 'UndecryptableBatchError', + batchRows: 30, + batchCodes: { UNKNOWN: 30 }, + }) + expect(state?.details).not.toHaveProperty('message') } finally { db.release() } From d2c232ebbeed7b6412f32c7c690ea17239dfe28e Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 05:46:57 +0000 Subject: [PATCH 15/15] fix(cli): skip forward-only columns in decrypt status before loading them decrypt status loaded a reverse ColumnMigration (two cs_migrations queries) for every tracked column, forward-only ones included. It now skips a key whose latest event is not a reverse phase, as the base code did. Also: - Live tests for an open reversal whose encrypted column is gone (encrypted-col-missing) and for the decrypt plan overview lines: a generated drop, an open forward migration, a twin that holds values. - fake-migration.ts types its spies from ColumnMigration, so a signature change fails the CLI typecheck. - The changeset says the waiver is refused while a started forced pass is owed, matching the help text and skill. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01Pkrkj6eBHR934hMcSGKSHg --- .changeset/column-migration-api.md | 2 +- .../__tests__/decrypt-lifecycle.live.test.ts | 104 ++++++++++++++++- .../decrypt/__tests__/fake-migration.ts | 106 ++++++++++-------- .../cli/src/commands/decrypt/status.ts | 10 +- 4 files changed, 171 insertions(+), 51 deletions(-) diff --git a/.changeset/column-migration-api.md b/.changeset/column-migration-api.md index 6a2198ce4..4ca556384 100644 --- a/.changeset/column-migration-api.md +++ b/.changeset/column-migration-api.md @@ -17,4 +17,4 @@ Error events store the error's class (`errorName`) and safe details instead of i `stash decrypt plan`, `status`, `backfill` and `drop` resolve every column through `ColumnMigration`, so they always agree on the twin and the phase. `decrypt status` flags `forward-open`, `twin-has-values` and `forced-pass-owed`. -`decrypt backfill --confirm-plaintext-current` is accepted on a later run as long as no forced pass has started, and refused once one has. +`decrypt backfill --confirm-plaintext-current` is accepted on a later run as long as no forced pass has started, and refused while a started forced pass is owed. diff --git a/languages/typescript/packages/cli/src/commands/decrypt/__tests__/decrypt-lifecycle.live.test.ts b/languages/typescript/packages/cli/src/commands/decrypt/__tests__/decrypt-lifecycle.live.test.ts index b4ca64c13..27125ad3e 100644 --- a/languages/typescript/packages/cli/src/commands/decrypt/__tests__/decrypt-lifecycle.live.test.ts +++ b/languages/typescript/packages/cli/src/commands/decrypt/__tests__/decrypt-lifecycle.live.test.ts @@ -92,7 +92,11 @@ vi.mock('@/commands/encrypt/context.js', () => ({ requireTable: vi.fn(), })) -import { appendEvent, installMigrationsSchema } from '@cipherstash/migrate' +import { + appendEvent, + ColumnMigration, + installMigrationsSchema, +} from '@cipherstash/migrate' import type pg from 'pg' import { decryptBackfillCommand } from '../backfill.js' import { decryptDropCommand } from '../drop.js' @@ -486,4 +490,102 @@ describeLive('decrypt lifecycle — live Postgres', () => { expect(rows.join('\n')).not.toContain('encrypted-col-missing') expect(rowFor(rows, 'email_v2')).toBeDefined() }) + + it('status flags an open reversal whose encrypted column is gone as encrypted-col-missing', async () => { + // The backfill completed, then email_encrypted was dropped outside stash + // before `decrypt drop` generated anything. + await client.query(`CREATE TABLE ${TABLE} (id int PRIMARY KEY, email text)`) + await installMigrationsSchema(client) + for (const [event, phase] of [ + ['decrypt_dual_writing', 'decrypt-dual-writing'], + ['decrypted', 'decrypted'], + ] as const) { + await appendEvent(client, { + tableName: TABLE, + columnName: 'email_encrypted', + event, + phase, + details: { plaintextColumn: 'email', force: false }, + }) + } + + const row = rowFor(await statusRows(), 'email_encrypted') + + expect(row).toMatch(/email_encrypted\s+—\s+email\s+decrypted/) + expect(row).toContain('encrypted-col-missing') + }) + + it('status does not load a reverse migration for a column with only forward history', async () => { + await client.query(`CREATE TABLE ${TABLE} (id int PRIMARY KEY, email text)`) + await installMigrationsSchema(client) + await appendEvent(client, { + tableName: TABLE, + columnName: 'email', + event: 'dual_writing', + phase: 'dual-writing', + details: { encryptedColumn: 'email_encrypted' }, + }) + const load = vi.spyOn(ColumnMigration, 'load') + + output.length = 0 + await decryptStatusCommand() + + expect(load).not.toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ table: TABLE, column: 'email' }), + ) + expect(output.join('\n')).not.toContain(`${TABLE} email `) + }) + + it('the plan overview names a generated drop, an open forward migration and a twin that holds values', async () => { + await client.query(` + CREATE TABLE ${TABLE} ( + id int PRIMARY KEY, + a_encrypted eql_v3_text_search, + a text, + b_encrypted eql_v3_text_search, + b text, + c_encrypted eql_v3_text_search, + c text + ) + `) + await client.query( + `INSERT INTO ${TABLE} (id, c) VALUES (1, 'stale@example.com')`, + ) + await installMigrationsSchema(client) + for (const [event, phase] of [ + ['decrypt_dual_writing', 'decrypt-dual-writing'], + ['decrypted', 'decrypted'], + ['encrypted_dropped', 'encrypted-dropped'], + ] as const) { + await appendEvent(client, { + tableName: TABLE, + columnName: 'a_encrypted', + event, + phase, + details: { plaintextColumn: 'a', force: false }, + }) + } + await appendEvent(client, { + tableName: TABLE, + columnName: 'b', + event: 'dual_writing', + phase: 'dual-writing', + details: { encryptedColumn: 'b_encrypted' }, + }) + + output.length = 0 + await decryptPlanCommand() + const overview = output.join('\n') + + expect(overview).toContain( + `✓ ${TABLE}.a_encrypted: encrypted-dropped (apply the generated drop migration)`, + ) + expect(overview).toContain( + `! ${TABLE}.b_encrypted → b text: forward encryption still open at 'dual-writing'`, + ) + expect(overview).toContain( + `! ${TABLE}.c_encrypted → c text: twin already holds values; the backfill needs --force or --confirm-plaintext-current`, + ) + }) }) diff --git a/languages/typescript/packages/cli/src/commands/decrypt/__tests__/fake-migration.ts b/languages/typescript/packages/cli/src/commands/decrypt/__tests__/fake-migration.ts index 53d120d1a..1fb5289f7 100644 --- a/languages/typescript/packages/cli/src/commands/decrypt/__tests__/fake-migration.ts +++ b/languages/typescript/packages/cli/src/commands/decrypt/__tests__/fake-migration.ts @@ -17,7 +17,7 @@ import { type OpenCounterpart, planPassFor, } from '@cipherstash/migrate' -import { vi } from 'vitest' +import { type Mock, vi } from 'vitest' let nextId = 1 @@ -58,10 +58,17 @@ export const decrypted = (): MigrationStateRow[] => [ reverseEvent('decrypted', 'decrypted', { rowsProcessed: 10, rowsTotal: 10 }), ] +/** + * The spies are typed from `ColumnMigration` itself, so a change to a real + * method's signature fails these tests' typecheck instead of passing them. + */ export type FakeMigration = ColumnMigration & { - confirmDualWrites: ReturnType - recordDropGenerated: ReturnType - recordSourceRemoved: ReturnType + existingTwinValues: Mock + planPass: Mock + counterpartMessage: Mock + confirmDualWrites: Mock + recordDropGenerated: Mock + recordSourceRemoved: Mock } export function fakeMigration( @@ -80,60 +87,65 @@ export function fakeMigration( recordedTwinHistory: opts.recordedTwinHistory, }) const twinHasValues = opts.twinHasValues ?? false - const fake = { - ...state, - table: 'users', - column: 'email_encrypted', - twin: { column: opts.twin ?? 'email', via: 'convention' as const }, - blockedBy: opts.blockedBy ?? null, - existingTwinValues: vi.fn( + const spies = { + existingTwinValues: vi.fn( async () => state.phase === null && twinHasValues, ), - planPass: vi.fn(async ({ force }: { force: boolean }) => + planPass: vi.fn(async ({ force }) => planPassFor(state, { force, twinHasValues: state.phase === null && twinHasValues, }), ), - counterpartMessage: vi.fn(() => null), - confirmDualWrites: vi.fn(), - recordDropGenerated: vi.fn(), - recordSourceRemoved: vi.fn(), + counterpartMessage: vi.fn( + () => null, + ), + confirmDualWrites: vi.fn(), + recordDropGenerated: vi.fn(), + recordSourceRemoved: vi.fn(), } + // `ColumnMigration` has private members, so no object literal is one; the + // spies above are what carry its method types. + const fake = { + ...state, + table: 'users', + column: 'email_encrypted', + twin: { column: opts.twin ?? 'email', via: 'convention' as const }, + blockedBy: opts.blockedBy ?? null, + ...spies, + } as unknown as FakeMigration // The real rules: in progress, record nothing unless a waiver closes a // waivable obligation, and refuse one that is not. - fake.confirmDualWrites.mockImplementation( - async ({ waiver }: { waiver?: 'plaintext-current' } = {}) => { - let existingTwinValues: boolean - if (state.phase !== null) { - const owed = state.obligations[0] - if (waiver === undefined || owed === undefined) return fake - if (!owed.waivable) { - throw new ColumnMigrationError( - 'waiver-refused', - 'A forced pass over users.email has already started.', - ) - } - existingTwinValues = true - } else { - existingTwinValues = twinHasValues + fake.confirmDualWrites.mockImplementation(async ({ waiver } = {}) => { + let existingTwinValues: boolean + if (state.phase !== null) { + const owed = state.obligations[0] + if (waiver === undefined || owed === undefined) return fake + if (!owed.waivable) { + throw new ColumnMigrationError( + 'waiver-refused', + 'A forced pass over users.email has already started.', + ) } - return fakeMigration({ - ...opts, - history: [ - ...(opts.history ?? []), - reverseEvent('decrypt_dual_writing', 'decrypt-dual-writing', { - details: { - plaintextColumn: 'email', - ...(existingTwinValues ? { existingTwinValues: true } : {}), - ...(existingTwinValues && waiver !== undefined ? { waiver } : {}), - }, - }), - ], - }) - }, - ) + existingTwinValues = true + } else { + existingTwinValues = twinHasValues + } + return fakeMigration({ + ...opts, + history: [ + ...(opts.history ?? []), + reverseEvent('decrypt_dual_writing', 'decrypt-dual-writing', { + details: { + plaintextColumn: 'email', + ...(existingTwinValues ? { existingTwinValues: true } : {}), + ...(existingTwinValues && waiver !== undefined ? { waiver } : {}), + }, + }), + ], + }) + }) fake.recordDropGenerated.mockImplementation(async () => fake) fake.recordSourceRemoved.mockImplementation(async () => fake) - return fake as unknown as FakeMigration + return fake } diff --git a/languages/typescript/packages/cli/src/commands/decrypt/status.ts b/languages/typescript/packages/cli/src/commands/decrypt/status.ts index 5e03123f9..31200c221 100644 --- a/languages/typescript/packages/cli/src/commands/decrypt/status.ts +++ b/languages/typescript/packages/cli/src/commands/decrypt/status.ts @@ -2,6 +2,8 @@ import { ColumnMigration, type ColumnMigrationState, classifyEqlDomain, + isReverseMigrationPhase, + type MigrationStateRow, passProgress, } from '@cipherstash/migrate' import * as p from '@clack/prompts' @@ -126,13 +128,17 @@ async function currentRows( */ async function goneRows( client: ReturnType, - stateMap: ReadonlyMap, + stateMap: ReadonlyMap, seen: ReadonlySet, physicalCols: PhysicalColumns, ): Promise { const rows: Row[] = [] - for (const key of stateMap.keys()) { + for (const [key, latest] of stateMap) { if (seen.has(key)) continue + // Reverse events are keyed by the encrypted column, so a key whose latest + // event is forward is a plaintext column, not a reversal: skip it before + // reading its history. + if (!isReverseMigrationPhase(latest.phase)) continue const lastDot = key.lastIndexOf('.') const table = key.slice(0, lastDot) const column = key.slice(lastDot + 1)