From 201d646e68b8d8937e33cb089c3d5ef7fe490d0d Mon Sep 17 00:00:00 2001 From: Daniel Padrino Date: Sat, 1 Aug 2026 22:43:15 -0300 Subject: [PATCH 1/5] feat(gs): grant staff clearance to the Debug account MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The staff-clearance rule (#4395 -> #4572) requires a non-empty user_data.verifiedName on top of the role for every elevated RoleGuard, including the Debug gate on POST /gs/debug. The backfill in #4574 covered two accounts; user data 403938 was not among them and still answers every elevated endpoint with STAFF_KYC_REQUIRED. Adds a PRD-only, idempotent, audited migration in the shape of #4574. The personal name is read from the deployment variable STAFF_VERIFIED_NAME_403938 and never appears in this repository. The closing assertion checks the clearance predicate itself — the BlankChars-aware BTRIM that StaffKycClearanceService uses — instead of equality with the supplied value. Should an identity-verified path have written a different, correct name in the meantime, that account is cleared and the deploy must not fail over the spelling. --- ...35000000-BackfillDebugStaffVerifiedName.js | 82 +++++++++++++++++++ 1 file changed, 82 insertions(+) create mode 100644 migration/1785635000000-BackfillDebugStaffVerifiedName.js diff --git a/migration/1785635000000-BackfillDebugStaffVerifiedName.js b/migration/1785635000000-BackfillDebugStaffVerifiedName.js new file mode 100644 index 0000000000..cc39c8a940 --- /dev/null +++ b/migration/1785635000000-BackfillDebugStaffVerifiedName.js @@ -0,0 +1,82 @@ +/** + * @typedef {import('typeorm').MigrationInterface} MigrationInterface + * @typedef {import('typeorm').QueryRunner} QueryRunner + */ + +// Same character set as `BlankChars` in StaffKycClearanceService — every character +// `String.prototype.trim()` strips. Postgres' bare `TRIM(x)` removes ASCII space only, so a name of a +// single tab or a non-breaking space would pass a `TRIM(x) <> ''` test while the clearance query still +// rejects it. Duplicated rather than imported: migrations are plain JS executed by TypeORM and cannot +// pull in application sources. +const BLANK_CHARS = + '\u0009\u000a\u000b\u000c\u000d\u0020\u00a0\u1680\u2000\u2001\u2002\u2003\u2004\u2005\u2006\u2007' + + '\u2008\u2009\u200a\u2028\u2029\u202f\u205f\u3000\ufeff'; + +/** + * PRD-only backfill for the remaining staff account gated out by the staff-clearance rule (#4395 → + * #4572). The earlier backfill (#4574) covered two accounts; user data 403938 (the Debug account) was + * not among them and still fails every elevated endpoint with STAFF_KYC_REQUIRED. + * + * No plaintext personal name lives in this file: the value is read from the deployment variable + * STAFF_VERIFIED_NAME_403938, which is mandatory on PRD so TypeORM cannot record a partial/no-op + * migration when it is missing. The update is idempotent (only touches a still-null verifiedName) and + * coupled to a durable before/after audit entry. Guarded to prd; a no-op elsewhere. + * + * The closing assertion checks the clearance predicate itself rather than equality with the supplied + * name: should an identity-verified path have written a different (correct) name in the meantime, that + * account is cleared and the migration must not fail the deploy over the spelling. + * @class @implements {MigrationInterface} + */ +module.exports = class BackfillDebugStaffVerifiedName1785635000000 { + name = 'BackfillDebugStaffVerifiedName1785635000000'; + + async up(queryRunner) { + if (process.env.ENVIRONMENT !== 'prd') return; + + const verifiedName = process.env.STAFF_VERIFIED_NAME_403938?.trim(); + if (!verifiedName) throw new Error('STAFF_VERIFIED_NAME_403938 is required for the PRD staff-name backfill'); + + // Array.of avoids looking like MSSQL bracket quoting to the repository's migration syntax guard. + await queryRunner.query( + `WITH "affected" AS ( + SELECT "id", "verifiedName" AS "previousVerifiedName" + FROM "user_data" + WHERE "id" = 403938 AND "verifiedName" IS NULL + FOR UPDATE + ), + "audit" AS ( + INSERT INTO "log" ("created", "updated", "system", "subsystem", "severity", "message") + SELECT now(), now(), 'User', 'StaffVerifiedNameBackfill', 'Info', + json_agg(json_build_object( + 'userDataId', "id", + 'previousVerifiedName', "previousVerifiedName", + 'nextVerifiedName', $1::varchar + ) ORDER BY "id")::text + FROM "affected" + HAVING count(*) > 0 + RETURNING 1 + ) + UPDATE "user_data" ud + SET "verifiedName" = $1::varchar, "updated" = now() + FROM "affected" a + WHERE ud."id" = a."id" AND EXISTS (SELECT 1 FROM "audit")`, + Array.of(verifiedName), + ); + + const rows = await queryRunner.query( + `SELECT count(*)::int AS "clearedCount" + FROM "user_data" + WHERE "id" = 403938 AND BTRIM("verifiedName", $1::varchar) <> ''`, + Array.of(BLANK_CHARS), + ); + + if (Number(rows.at(0)?.clearedCount) !== 1) { + throw new Error('PRD staff-name backfill did not reach the required state for user data 403938'); + } + } + + async down() { + // No-op: a granted clearance is not auto-revoked here; removal requires a separate reviewed, + // audited revocation so an unrelated rollback cannot silently erase an identity grant. + } +}; From d30f0549c18dde9d0a0167f550e99f2dc3e4f911 Mon Sep 17 00:00:00 2001 From: Daniel Padrino Date: Sat, 1 Aug 2026 22:59:53 -0300 Subject: [PATCH 2/5] fix(gs): make the backfill precondition the exact negation of its assertion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The update repaired only a NULL verifiedName while the closing assertion demanded a non-blank one, so a present-but-blank name (a lone tab, a non-breaking space) was a state the migration refused to repair and then refused to accept. Because migrationsTransactionMode defaults to 'all', that throw rolls back the whole release's migration batch and fails DataSource.initialize() — a PRD crash-loop, not a failed tool. The precondition now uses the same BTRIM predicate as the assertion, so the two are complementary by construction and the blank case becomes a repair. Keeping a divergent existing name is no longer silent either: the audit entry records the outcome, so the deployed state cannot differ from the reviewed one without a trace. A re-run after a successful backfill stays a true no-op. Adds the migration spec the two preceding data migrations ship, including the blank-name cases that were the defect, so the verification runs in CI on the postgres:16 service instead of living in a throwaway container. --- ...35000000-BackfillDebugStaffVerifiedName.js | 37 +- ...ebug-staff-verified-name.migration.spec.ts | 321 ++++++++++++++++++ 2 files changed, 349 insertions(+), 9 deletions(-) create mode 100644 src/subdomains/generic/user/models/user/__tests__/backfill-debug-staff-verified-name.migration.spec.ts diff --git a/migration/1785635000000-BackfillDebugStaffVerifiedName.js b/migration/1785635000000-BackfillDebugStaffVerifiedName.js index cc39c8a940..66d29daf7a 100644 --- a/migration/1785635000000-BackfillDebugStaffVerifiedName.js +++ b/migration/1785635000000-BackfillDebugStaffVerifiedName.js @@ -24,7 +24,9 @@ const BLANK_CHARS = * * The closing assertion checks the clearance predicate itself rather than equality with the supplied * name: should an identity-verified path have written a different (correct) name in the meantime, that - * account is cleared and the migration must not fail the deploy over the spelling. + * account is cleared and the migration must not fail the deploy over the spelling. That divergence is + * not silent — it is recorded as its own audit entry, so the deployed state never differs from the + * reviewed one without a trace. * @class @implements {MigrationInterface} */ module.exports = class BackfillDebugStaffVerifiedName1785635000000 { @@ -36,31 +38,48 @@ module.exports = class BackfillDebugStaffVerifiedName1785635000000 { const verifiedName = process.env.STAFF_VERIFIED_NAME_403938?.trim(); if (!verifiedName) throw new Error('STAFF_VERIFIED_NAME_403938 is required for the PRD staff-name backfill'); + // `needsBackfill` is the exact negation of the closing assertion below. The two must stay + // complementary: a precondition of `verifiedName IS NULL` against a non-blank postcondition would + // leave a present-but-blank name (a lone tab, a non-breaking space) as a state the migration + // refuses to repair and then refuses to accept — and because `migrationsTransactionMode` defaults + // to 'all', that throw rolls back the whole release's batch and takes the boot down with it. + // + // `noteworthy` is what gets audited: the repair itself, or the deliberate decision to keep a + // divergent name that an identity-verified path wrote in the meantime. A re-run after a successful + // backfill is neither, so it stays a true no-op instead of appending an audit row every time. // Array.of avoids looking like MSSQL bracket quoting to the repository's migration syntax guard. await queryRunner.query( - `WITH "affected" AS ( - SELECT "id", "verifiedName" AS "previousVerifiedName" + `WITH "target" AS ( + SELECT "id", + "verifiedName" AS "previousVerifiedName", + BTRIM(COALESCE("verifiedName", ''), $2::varchar) = '' AS "needsBackfill" FROM "user_data" - WHERE "id" = 403938 AND "verifiedName" IS NULL + WHERE "id" = 403938 FOR UPDATE ), + "noteworthy" AS ( + SELECT "id", "previousVerifiedName", "needsBackfill" + FROM "target" + WHERE "needsBackfill" OR "previousVerifiedName" IS DISTINCT FROM $1::varchar + ), "audit" AS ( INSERT INTO "log" ("created", "updated", "system", "subsystem", "severity", "message") SELECT now(), now(), 'User', 'StaffVerifiedNameBackfill', 'Info', json_agg(json_build_object( 'userDataId', "id", 'previousVerifiedName', "previousVerifiedName", - 'nextVerifiedName', $1::varchar + 'nextVerifiedName', CASE WHEN "needsBackfill" THEN $1::varchar ELSE "previousVerifiedName" END, + 'action', CASE WHEN "needsBackfill" THEN 'backfilled' ELSE 'keptExistingName' END ) ORDER BY "id")::text - FROM "affected" + FROM "noteworthy" HAVING count(*) > 0 RETURNING 1 ) UPDATE "user_data" ud SET "verifiedName" = $1::varchar, "updated" = now() - FROM "affected" a - WHERE ud."id" = a."id" AND EXISTS (SELECT 1 FROM "audit")`, - Array.of(verifiedName), + FROM "target" t + WHERE ud."id" = t."id" AND t."needsBackfill" AND EXISTS (SELECT 1 FROM "audit")`, + Array.of(verifiedName, BLANK_CHARS), ); const rows = await queryRunner.query( diff --git a/src/subdomains/generic/user/models/user/__tests__/backfill-debug-staff-verified-name.migration.spec.ts b/src/subdomains/generic/user/models/user/__tests__/backfill-debug-staff-verified-name.migration.spec.ts new file mode 100644 index 0000000000..35fde035e5 --- /dev/null +++ b/src/subdomains/generic/user/models/user/__tests__/backfill-debug-staff-verified-name.migration.spec.ts @@ -0,0 +1,321 @@ +import { DataSource, QueryRunner } from 'typeorm'; + +const PG_URL = process.env.MIGRATION_TEST_PG; +const describeDb = PG_URL ? describe : describe.skip; +const SCHEMA = 'backfill_debug_staff_verified_name_spec'; +const STAFF_NAME_ENV = 'STAFF_VERIFIED_NAME_403938'; +const ACCOUNT_ID = 403938; +const OTHER_ACCOUNT_ID = 111222; + +let BackfillDebugStaffVerifiedName: new () => { + up(queryRunner: QueryRunner): Promise; + down(): Promise; +}; + +function setEnv(name: string, value: string | undefined): void { + if (value === undefined) delete process.env[name]; + else process.env[name] = value; +} + +describe('BackfillDebugStaffVerifiedName migration (SQL content)', () => { + const originalEnvironment = process.env.ENVIRONMENT; + const originalStaffName = process.env[STAFF_NAME_ENV]; + + beforeAll(() => { + // The migration is intentionally a plain CommonJS module, matching TypeORM's runtime loader. + // eslint-disable-next-line @typescript-eslint/no-require-imports + BackfillDebugStaffVerifiedName = require('../../../../../../../migration/1785635000000-BackfillDebugStaffVerifiedName'); + }); + + afterEach(() => { + setEnv('ENVIRONMENT', originalEnvironment); + setEnv(STAFF_NAME_ENV, originalStaffName); + }); + + it.each([['dev'], ['loc'], ['staging'], [undefined]])( + 'up() issues no queries when ENVIRONMENT is %s', + async (environment) => { + setEnv('ENVIRONMENT', environment); + setEnv(STAFF_NAME_ENV, undefined); + const queryRunner = { query: jest.fn(async (_sql: string) => []) }; + + await new BackfillDebugStaffVerifiedName().up(queryRunner as unknown as QueryRunner); + + expect(queryRunner.query).not.toHaveBeenCalled(); + }, + ); + + it.each([[undefined], [''], [' ']])( + 'fails before issuing SQL when the PRD deployment variable is %p', + async (staffName) => { + process.env.ENVIRONMENT = 'prd'; + setEnv(STAFF_NAME_ENV, staffName); + const queryRunner = { query: jest.fn(async (_sql: string) => []) }; + + await expect(new BackfillDebugStaffVerifiedName().up(queryRunner as unknown as QueryRunner)).rejects.toThrow( + `${STAFF_NAME_ENV} is required`, + ); + expect(queryRunner.query).not.toHaveBeenCalled(); + }, + ); + + it('issues one parameterized, audited update on PRD and never inlines the name', async () => { + process.env.ENVIRONMENT = 'prd'; + process.env[STAFF_NAME_ENV] = ' Test Staff Name '; + const queryRunner = { + query: jest + .fn() + .mockResolvedValueOnce([]) + .mockResolvedValueOnce([{ clearedCount: 1 }]), + }; + + await new BackfillDebugStaffVerifiedName().up(queryRunner as unknown as QueryRunner); + + expect(queryRunner.query).toHaveBeenCalledTimes(2); + const [sql, parameters] = queryRunner.query.mock.calls[0]; + expect(parameters[0]).toBe('Test Staff Name'); + expect(sql).toContain('INSERT INTO "log"'); + expect(sql).toContain("'StaffVerifiedNameBackfill'"); + expect(sql).toContain("'previousVerifiedName'"); + expect(sql).toContain("'nextVerifiedName'"); + expect(sql).toContain('FOR UPDATE'); + expect(sql).toContain('EXISTS (SELECT 1 FROM "audit")'); + expect(sql).toContain('SET "verifiedName" = $1::varchar, "updated" = now()'); + expect(sql).toContain(String(ACCOUNT_ID)); + expect(sql).not.toContain('Test Staff Name'); + + // The precondition must be the exact negation of the postcondition — otherwise a present-but-blank + // name is a state the update refuses to produce and the assertion refuses to accept, and the deploy + // dies on a row the migration itself could have repaired. + expect(sql).toContain("BTRIM(COALESCE(\"verifiedName\", ''), $2::varchar) = ''"); + expect(sql).toContain("'action', CASE WHEN \"needsBackfill\" THEN 'backfilled' ELSE 'keptExistingName' END"); + + const [postconditionSql, postconditionParameters] = queryRunner.query.mock.calls[1]; + expect(postconditionSql).toContain('AS "clearedCount"'); + expect(postconditionSql).toContain('BTRIM("verifiedName", $1::varchar)'); + // The postcondition asserts the clearance predicate, not equality with the supplied name. + expect(postconditionSql).not.toContain('"verifiedName" = $'); + expect(postconditionParameters[0]).toBe(parameters[1]); + }); + + it('rejects when the account does not reach the cleared state', async () => { + process.env.ENVIRONMENT = 'prd'; + process.env[STAFF_NAME_ENV] = 'Test Staff Name'; + const queryRunner = { + query: jest + .fn() + .mockResolvedValueOnce([]) + .mockResolvedValueOnce([{ clearedCount: 0 }]), + }; + + await expect(new BackfillDebugStaffVerifiedName().up(queryRunner as unknown as QueryRunner)).rejects.toThrow( + `did not reach the required state for user data ${ACCOUNT_ID}`, + ); + }); + + it('down() deliberately performs no rollback', async () => { + const migration = new BackfillDebugStaffVerifiedName(); + + expect(migration.down).toHaveLength(0); + await expect(migration.down()).resolves.toBeUndefined(); + }); +}); + +describeDb('BackfillDebugStaffVerifiedName migration (real Postgres)', () => { + const originalEnvironment = process.env.ENVIRONMENT; + const originalStaffName = process.env[STAFF_NAME_ENV]; + let dataSource: DataSource; + let queryRunner: QueryRunner; + + beforeAll(async () => { + // The migration is intentionally a plain CommonJS module, matching TypeORM's runtime loader. + // eslint-disable-next-line @typescript-eslint/no-require-imports + BackfillDebugStaffVerifiedName = require('../../../../../../../migration/1785635000000-BackfillDebugStaffVerifiedName'); + dataSource = new DataSource({ type: 'postgres', url: PG_URL }); + await dataSource.initialize(); + }); + + beforeEach(async () => { + process.env.ENVIRONMENT = 'prd'; + process.env[STAFF_NAME_ENV] = 'Test Staff Name'; + queryRunner = dataSource.createQueryRunner(); + await queryRunner.connect(); + await queryRunner.query(`DROP SCHEMA IF EXISTS "${SCHEMA}" CASCADE`); + await queryRunner.query(`CREATE SCHEMA "${SCHEMA}"`); + await queryRunner.query(`SET search_path TO "${SCHEMA}"`); + await queryRunner.query(` + CREATE TABLE "user_data" ( + "id" integer PRIMARY KEY, + "updated" TIMESTAMP NOT NULL DEFAULT now(), + "verifiedName" varchar(256) + ) + `); + await queryRunner.query(` + CREATE TABLE "log" ( + "id" SERIAL PRIMARY KEY, + "created" TIMESTAMP NOT NULL DEFAULT now(), + "updated" TIMESTAMP NOT NULL DEFAULT now(), + "system" varchar(256) NOT NULL, + "subsystem" varchar(256) NOT NULL, + "severity" varchar(256) NOT NULL, + "message" text NOT NULL + ) + `); + }); + + afterEach(async () => { + setEnv('ENVIRONMENT', originalEnvironment); + setEnv(STAFF_NAME_ENV, originalStaffName); + if (queryRunner.isTransactionActive) await queryRunner.rollbackTransaction(); + await queryRunner.query(`SET search_path TO public`); + await queryRunner.query(`DROP SCHEMA IF EXISTS "${SCHEMA}" CASCADE`); + await queryRunner.release(); + }); + + afterAll(async () => { + if (dataSource?.isInitialized) await dataSource.destroy(); + }); + + async function insertAccounts(targetName: string | null = null, otherName: string | null = null): Promise { + await queryRunner.query( + `INSERT INTO "user_data" ("id", "updated", "verifiedName") + VALUES (${ACCOUNT_ID}, TIMESTAMP '2000-01-01', $1), (${OTHER_ACCOUNT_ID}, TIMESTAMP '2000-01-01', $2)`, + [targetName, otherName], + ); + } + + async function readAccounts(): Promise<{ id: number; verifiedName: string | null }[]> { + return queryRunner.query(`SELECT "id", "verifiedName" FROM "user_data" ORDER BY "id"`); + } + + it('backfills the target, updates its timestamp, and records one before/after audit row', async () => { + await insertAccounts(); + + await new BackfillDebugStaffVerifiedName().up(queryRunner); + + const users = (await queryRunner.query( + `SELECT "id", "verifiedName", "updated" > TIMESTAMP '2000-01-01' AS "wasUpdated" + FROM "user_data" ORDER BY "id"`, + )) as { id: number; verifiedName: string | null; wasUpdated: boolean }[]; + expect(users).toEqual([ + { id: OTHER_ACCOUNT_ID, verifiedName: null, wasUpdated: false }, + { id: ACCOUNT_ID, verifiedName: 'Test Staff Name', wasUpdated: true }, + ]); + + const logs = (await queryRunner.query( + `SELECT "message" FROM "log" WHERE "system" = 'User' AND "subsystem" = 'StaffVerifiedNameBackfill'`, + )) as { message: string }[]; + expect(logs).toHaveLength(1); + expect(JSON.parse(logs[0].message)).toEqual([ + { + userDataId: ACCOUNT_ID, + previousVerifiedName: null, + nextVerifiedName: 'Test Staff Name', + action: 'backfilled', + }, + ]); + }); + + it('is idempotent and does not append another audit row on a second run', async () => { + await insertAccounts(); + const migration = new BackfillDebugStaffVerifiedName(); + + await migration.up(queryRunner); + await migration.up(queryRunner); + + const logCount = (await queryRunner.query(`SELECT count(*)::int AS "count" FROM "log"`)) as { count: number }[]; + expect(logCount[0].count).toBe(1); + expect(await readAccounts()).toEqual([ + { id: OTHER_ACCOUNT_ID, verifiedName: null }, + { id: ACCOUNT_ID, verifiedName: 'Test Staff Name' }, + ]); + }); + + // A blank name clears no account — the gate's predicate is BTRIM-based, not IS NOT NULL. Repairing it + // is the whole point of widening the precondition: a name of a single tab would otherwise be a row the + // migration refuses to fix and then refuses to accept, taking the boot down with it. + it.each([['\t'], [' '], ['\u00a0'], ['\ufeff']])('repairs a blank verifiedName (%j)', async (blank) => { + await insertAccounts(blank); + + await new BackfillDebugStaffVerifiedName().up(queryRunner); + + expect(await readAccounts()).toEqual([ + { id: OTHER_ACCOUNT_ID, verifiedName: null }, + { id: ACCOUNT_ID, verifiedName: 'Test Staff Name' }, + ]); + const logs = (await queryRunner.query(`SELECT "message" FROM "log"`)) as { message: string }[]; + expect(JSON.parse(logs[0].message)).toEqual([ + { + userDataId: ACCOUNT_ID, + previousVerifiedName: blank, + nextVerifiedName: 'Test Staff Name', + action: 'backfilled', + }, + ]); + }); + + // The deliberate deviation from #4574: an identity-verified path may have written a different, valid + // name. That account is cleared, so the migration must leave it alone and must NOT fail the deploy — + // but the divergence between the reviewed value and the deployed one must not be silent. + it('leaves an existing verified name untouched, records the divergence, and still succeeds', async () => { + await insertAccounts('Existing Verified Name'); + + await expect(new BackfillDebugStaffVerifiedName().up(queryRunner)).resolves.toBeUndefined(); + + expect(await readAccounts()).toEqual([ + { id: OTHER_ACCOUNT_ID, verifiedName: null }, + { id: ACCOUNT_ID, verifiedName: 'Existing Verified Name' }, + ]); + const logs = (await queryRunner.query(`SELECT "message" FROM "log"`)) as { message: string }[]; + expect(logs).toHaveLength(1); + expect(JSON.parse(logs[0].message)).toEqual([ + { + userDataId: ACCOUNT_ID, + previousVerifiedName: 'Existing Verified Name', + nextVerifiedName: 'Existing Verified Name', + action: 'keptExistingName', + }, + ]); + }); + + it('rejects when the target row is absent and relies on the migration transaction to roll back', async () => { + await queryRunner.query(`INSERT INTO "user_data" ("id") VALUES (${OTHER_ACCOUNT_ID})`); + await queryRunner.startTransaction(); + + await expect(new BackfillDebugStaffVerifiedName().up(queryRunner)).rejects.toThrow( + `did not reach the required state for user data ${ACCOUNT_ID}`, + ); + await queryRunner.rollbackTransaction(); + + const logs = (await queryRunner.query(`SELECT "message" FROM "log"`)) as { message: string }[]; + expect(logs).toHaveLength(0); + }); + + it('changes nothing when a trigger suppresses the audit insert', async () => { + await insertAccounts(); + await queryRunner.query(` + CREATE FUNCTION suppress_log_insert() RETURNS trigger AS $fn$ + BEGIN + RETURN NULL; + END; + $fn$ LANGUAGE plpgsql + `); + await queryRunner.query(` + CREATE TRIGGER suppress_log_insert_trigger + BEFORE INSERT ON "log" + FOR EACH ROW + EXECUTE FUNCTION suppress_log_insert() + `); + + await expect(new BackfillDebugStaffVerifiedName().up(queryRunner)).rejects.toThrow( + `did not reach the required state for user data ${ACCOUNT_ID}`, + ); + + expect(await readAccounts()).toEqual([ + { id: OTHER_ACCOUNT_ID, verifiedName: null }, + { id: ACCOUNT_ID, verifiedName: null }, + ]); + const logCount = (await queryRunner.query(`SELECT count(*)::int AS "count" FROM "log"`)) as { count: number }[]; + expect(logCount[0].count).toBe(0); + }); +}); From 53f5dc3eeb810fdbc9b58e5cd796728a579e350e Mon Sep 17 00:00:00 2001 From: Daniel Padrino Date: Sat, 1 Aug 2026 23:18:09 -0300 Subject: [PATCH 3/5] test(gs): pin the assertion's account scope and the duplicated BlankChars MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The spec left the non-target account's verifiedName NULL in every database case, so `count(*) = 1` held whether or not the closing assertion was scoped to the account. Dropping `"id" = 403938 AND` from the postcondition passed all 20 tests while in production it would count every cleared account and throw. The fixture now gives that account a cleared name, which kills the mutant with 10 failures. The blank-name cases covered four hand-picked characters, so the migration's copy of BlankChars could lose any of the other 21 code points unnoticed. The set is now derived from the runtime — every character String.prototype.trim() strips, which is what BlankChars is defined as — and the migration must repair a name built from all of them. Also drops a vacuous assertion: the log count was read after the rollback, where it holds regardless of what the migration wrote. docs/staff-kyc-clearance.md still prescribed the `verifiedName IS NULL` precondition this branch removed, so the next author following it would reproduce the boot-fatal gap. The recipe now states the exact-negation requirement, and step 2 no longer says the value is applied "only when present" when PRD in fact throws without it. --- docs/staff-kyc-clearance.md | 18 +++++--- ...ebug-staff-verified-name.migration.spec.ts | 45 +++++++++++++++---- 2 files changed, 50 insertions(+), 13 deletions(-) diff --git a/docs/staff-kyc-clearance.md b/docs/staff-kyc-clearance.md index cb437b9a52..f4972f4933 100644 --- a/docs/staff-kyc-clearance.md +++ b/docs/staff-kyc-clearance.md @@ -32,12 +32,20 @@ be reviewed and reproducible like any other schema/data change. 1. Open a PR against the api repo that adds a new migration under `migration/` (never modify an existing one — `api-migration-check.yaml` blocks that). The migration runs a guarded, idempotent - `UPDATE user_data SET "verifiedName" = … WHERE id = … AND "verifiedName" IS NULL`. + `UPDATE user_data SET "verifiedName" = … WHERE id = … AND BTRIM(COALESCE("verifiedName", ''), ) = ''`. + The precondition must be the **exact negation** of the migration's closing assertion, and both must + use the same `BlankChars` set as `StaffKycClearanceService`. A narrower `"verifiedName" IS NULL` + precondition against a non-blank assertion leaves a present-but-blank name (a lone tab, a + non-breaking space) as a state the migration refuses to repair and then refuses to accept — and since + `migrationsTransactionMode` defaults to `all`, that throw rolls back the whole release's migration + batch and crash-loops the boot. 2. **A real person's name is PII and must not be hard-coded in this public repo.** The migration - reads the value from a deployment secret (e.g. `process.env.STAFF_VERIFIED_NAME_`), set in the - production config, and applies it only when present. A non-personal service designation (for a - machine account that cannot complete a personal identification) is not PII and may appear inline. - The concrete name↔account mapping is recorded in the private operations repo, not here. + reads the value from a deployment variable (e.g. `process.env.STAFF_VERIFIED_NAME_`), set in the + production config. On PRD the variable is **mandatory**: the migration throws when it is absent, + rather than silently recording a no-op — so the value has to be live in the production environment + before the migration reaches `main`. A non-personal service designation (for a machine account that + cannot complete a personal identification) is not PII and may appear inline. The concrete + name↔account mapping is recorded in the private operations repo, not here. 3. Merge the PR through the normal review, then release `develop → main`. The production deploy runs pending migrations automatically when `SQL_MIGRATE=true` (see `migrationsRun` in `src/config/config.ts`) — no shell access to the database is involved. diff --git a/src/subdomains/generic/user/models/user/__tests__/backfill-debug-staff-verified-name.migration.spec.ts b/src/subdomains/generic/user/models/user/__tests__/backfill-debug-staff-verified-name.migration.spec.ts index 35fde035e5..89854673c3 100644 --- a/src/subdomains/generic/user/models/user/__tests__/backfill-debug-staff-verified-name.migration.spec.ts +++ b/src/subdomains/generic/user/models/user/__tests__/backfill-debug-staff-verified-name.migration.spec.ts @@ -6,6 +6,7 @@ const SCHEMA = 'backfill_debug_staff_verified_name_spec'; const STAFF_NAME_ENV = 'STAFF_VERIFIED_NAME_403938'; const ACCOUNT_ID = 403938; const OTHER_ACCOUNT_ID = 111222; +const OTHER_ACCOUNT_NAME = 'Other Cleared Staff'; let BackfillDebugStaffVerifiedName: new () => { up(queryRunner: QueryRunner): Promise; @@ -176,7 +177,12 @@ describeDb('BackfillDebugStaffVerifiedName migration (real Postgres)', () => { if (dataSource?.isInitialized) await dataSource.destroy(); }); - async function insertAccounts(targetName: string | null = null, otherName: string | null = null): Promise { + // The non-target account carries a cleared name on purpose: an unscoped postcondition would then + // count it too, so `count(*) = 1` only holds while the assertion stays pinned to ACCOUNT_ID. + async function insertAccounts( + targetName: string | null = null, + otherName: string | null = OTHER_ACCOUNT_NAME, + ): Promise { await queryRunner.query( `INSERT INTO "user_data" ("id", "updated", "verifiedName") VALUES (${ACCOUNT_ID}, TIMESTAMP '2000-01-01', $1), (${OTHER_ACCOUNT_ID}, TIMESTAMP '2000-01-01', $2)`, @@ -198,7 +204,7 @@ describeDb('BackfillDebugStaffVerifiedName migration (real Postgres)', () => { FROM "user_data" ORDER BY "id"`, )) as { id: number; verifiedName: string | null; wasUpdated: boolean }[]; expect(users).toEqual([ - { id: OTHER_ACCOUNT_ID, verifiedName: null, wasUpdated: false }, + { id: OTHER_ACCOUNT_ID, verifiedName: OTHER_ACCOUNT_NAME, wasUpdated: false }, { id: ACCOUNT_ID, verifiedName: 'Test Staff Name', wasUpdated: true }, ]); @@ -226,7 +232,25 @@ describeDb('BackfillDebugStaffVerifiedName migration (real Postgres)', () => { const logCount = (await queryRunner.query(`SELECT count(*)::int AS "count" FROM "log"`)) as { count: number }[]; expect(logCount[0].count).toBe(1); expect(await readAccounts()).toEqual([ - { id: OTHER_ACCOUNT_ID, verifiedName: null }, + { id: OTHER_ACCOUNT_ID, verifiedName: OTHER_ACCOUNT_NAME }, + { id: ACCOUNT_ID, verifiedName: 'Test Staff Name' }, + ]); + }); + + // `BlankChars` is defined as every character `String.prototype.trim()` strips, so derive that set from + // the runtime instead of restating it, and assert the migration's duplicated copy repairs a name built + // from all of them at once. A copy that lost a code point — the drift the migration's own comment warns + // about — would leave such a name unrepaired and then fail its own assertion. + it('repairs a name built from every character trim() strips, pinning the duplicated BlankChars', async () => { + const blankChars = Array.from({ length: 0x10000 }, (_, code) => String.fromCharCode(code)).filter( + (char) => char.trim() === '', + ); + await insertAccounts(blankChars.join('')); + + await new BackfillDebugStaffVerifiedName().up(queryRunner); + + expect(await readAccounts()).toEqual([ + { id: OTHER_ACCOUNT_ID, verifiedName: OTHER_ACCOUNT_NAME }, { id: ACCOUNT_ID, verifiedName: 'Test Staff Name' }, ]); }); @@ -240,7 +264,7 @@ describeDb('BackfillDebugStaffVerifiedName migration (real Postgres)', () => { await new BackfillDebugStaffVerifiedName().up(queryRunner); expect(await readAccounts()).toEqual([ - { id: OTHER_ACCOUNT_ID, verifiedName: null }, + { id: OTHER_ACCOUNT_ID, verifiedName: OTHER_ACCOUNT_NAME }, { id: ACCOUNT_ID, verifiedName: 'Test Staff Name' }, ]); const logs = (await queryRunner.query(`SELECT "message" FROM "log"`)) as { message: string }[]; @@ -263,7 +287,7 @@ describeDb('BackfillDebugStaffVerifiedName migration (real Postgres)', () => { await expect(new BackfillDebugStaffVerifiedName().up(queryRunner)).resolves.toBeUndefined(); expect(await readAccounts()).toEqual([ - { id: OTHER_ACCOUNT_ID, verifiedName: null }, + { id: OTHER_ACCOUNT_ID, verifiedName: OTHER_ACCOUNT_NAME }, { id: ACCOUNT_ID, verifiedName: 'Existing Verified Name' }, ]); const logs = (await queryRunner.query(`SELECT "message" FROM "log"`)) as { message: string }[]; @@ -279,16 +303,21 @@ describeDb('BackfillDebugStaffVerifiedName migration (real Postgres)', () => { }); it('rejects when the target row is absent and relies on the migration transaction to roll back', async () => { - await queryRunner.query(`INSERT INTO "user_data" ("id") VALUES (${OTHER_ACCOUNT_ID})`); + await queryRunner.query(`INSERT INTO "user_data" ("id", "verifiedName") VALUES (${OTHER_ACCOUNT_ID}, $1)`, [ + OTHER_ACCOUNT_NAME, + ]); await queryRunner.startTransaction(); await expect(new BackfillDebugStaffVerifiedName().up(queryRunner)).rejects.toThrow( `did not reach the required state for user data ${ACCOUNT_ID}`, ); - await queryRunner.rollbackTransaction(); + // Read INSIDE the transaction: after the rollback every write is gone regardless, so the same + // assertion afterwards would hold even if the migration had written an audit row. const logs = (await queryRunner.query(`SELECT "message" FROM "log"`)) as { message: string }[]; expect(logs).toHaveLength(0); + + await queryRunner.rollbackTransaction(); }); it('changes nothing when a trigger suppresses the audit insert', async () => { @@ -312,7 +341,7 @@ describeDb('BackfillDebugStaffVerifiedName migration (real Postgres)', () => { ); expect(await readAccounts()).toEqual([ - { id: OTHER_ACCOUNT_ID, verifiedName: null }, + { id: OTHER_ACCOUNT_ID, verifiedName: OTHER_ACCOUNT_NAME }, { id: ACCOUNT_ID, verifiedName: null }, ]); const logCount = (await queryRunner.query(`SELECT count(*)::int AS "count" FROM "log"`)) as { count: number }[]; From e517bb612dc122f55479f375d30093e6e264c286 Mon Sep 17 00:00:00 2001 From: Daniel Padrino Date: Sat, 1 Aug 2026 23:32:00 -0300 Subject: [PATCH 4/5] docs(gs): correct the JSDoc precondition and state the assertion the recipe requires MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The migration's JSDoc still said the update "only touches a still-null verifiedName", carried over from the migration this one is modelled on and false since the precondition became the negation of the closing assertion. This has to be fixed before the merge: api-migration-check.yaml allows comment-only edits to an existing migration by stripping whole lines starting with `//`, and JSDoc lines start with `*`, so afterwards the sentence could never be corrected. The runbook demanded the precondition be the exact negation of the closing assertion without saying what that assertion must be. The only precedent in the repo asserts equality with the supplied name, so an author following both would write an equality assertion — which throws when an identity-verified path wrote a different but valid name, and under the 'all' transaction mode takes the release's whole migration batch and the boot with it. The recipe now states the clearance-predicate assertion explicitly, and names `develop` rather than `main` as the last point where the deploy order can still be arranged. Two spec corrections: the BlankChars pin claimed a drifted copy would fail the migration's own assertion, when in fact the postcondition shares the drifted constant and reports success — the drift is silent, which is precisely why the test asserts the repaired state. And a test name still promised a post-rollback assertion that no longer exists. Adds the sanity floor on the derived character set that the sibling spec already carries. --- docs/staff-kyc-clearance.md | 28 +++++++++++++------ ...35000000-BackfillDebugStaffVerifiedName.js | 4 +-- ...ebug-staff-verified-name.migration.spec.ts | 7 +++-- 3 files changed, 26 insertions(+), 13 deletions(-) diff --git a/docs/staff-kyc-clearance.md b/docs/staff-kyc-clearance.md index f4972f4933..2e0e4d8d53 100644 --- a/docs/staff-kyc-clearance.md +++ b/docs/staff-kyc-clearance.md @@ -33,19 +33,29 @@ be reviewed and reproducible like any other schema/data change. 1. Open a PR against the api repo that adds a new migration under `migration/` (never modify an existing one — `api-migration-check.yaml` blocks that). The migration runs a guarded, idempotent `UPDATE user_data SET "verifiedName" = … WHERE id = … AND BTRIM(COALESCE("verifiedName", ''), ) = ''`. - The precondition must be the **exact negation** of the migration's closing assertion, and both must - use the same `BlankChars` set as `StaffKycClearanceService`. A narrower `"verifiedName" IS NULL` - precondition against a non-blank assertion leaves a present-but-blank name (a lone tab, a - non-breaking space) as a state the migration refuses to repair and then refuses to accept — and since - `migrationsTransactionMode` defaults to `all`, that throw rolls back the whole release's migration - batch and crash-loops the boot. + The closing assertion must check the **clearance predicate itself** — that + `… WHERE id = … AND BTRIM("verifiedName", ) <> ''` yields exactly one row — and never + equality with the supplied name. An equality assertion (as in the first such migration) throws when + an identity-verified path wrote a different but perfectly valid name in the meantime, failing the + deploy over a spelling difference. + + The precondition must then be the **exact negation** of that assertion, using the same `BlankChars` + set as `StaffKycClearanceService`. A narrower `"verifiedName" IS NULL` precondition leaves a + present-but-blank name (a lone tab, a non-breaking space) as a state the migration refuses to repair + and then refuses to accept. + + Both mistakes are boot-fatal, not merely wrong: `migrationsTransactionMode` defaults to `all`, so the + throw rolls back the whole release's migration batch and fails `DataSource.initialize()`. + 2. **A real person's name is PII and must not be hard-coded in this public repo.** The migration reads the value from a deployment variable (e.g. `process.env.STAFF_VERIFIED_NAME_`), set in the production config. On PRD the variable is **mandatory**: the migration throws when it is absent, rather than silently recording a no-op — so the value has to be live in the production environment - before the migration reaches `main`. A non-personal service designation (for a machine account that - cannot complete a personal identification) is not PII and may appear inline. The concrete - name↔account mapping is recorded in the private operations repo, not here. + before the migration is merged to `develop`. `auto-release-pr.yaml` opens the `develop` → `main` + release PR on every `develop` push, so `develop` is the last point at which the order can still be + arranged. A non-personal service designation (for a machine account that cannot complete a personal + identification) is not PII and may appear inline. The concrete name↔account mapping is recorded in + the private operations repo, not here. 3. Merge the PR through the normal review, then release `develop → main`. The production deploy runs pending migrations automatically when `SQL_MIGRATE=true` (see `migrationsRun` in `src/config/config.ts`) — no shell access to the database is involved. diff --git a/migration/1785635000000-BackfillDebugStaffVerifiedName.js b/migration/1785635000000-BackfillDebugStaffVerifiedName.js index 66d29daf7a..f06e21c438 100644 --- a/migration/1785635000000-BackfillDebugStaffVerifiedName.js +++ b/migration/1785635000000-BackfillDebugStaffVerifiedName.js @@ -19,8 +19,8 @@ const BLANK_CHARS = * * No plaintext personal name lives in this file: the value is read from the deployment variable * STAFF_VERIFIED_NAME_403938, which is mandatory on PRD so TypeORM cannot record a partial/no-op - * migration when it is missing. The update is idempotent (only touches a still-null verifiedName) and - * coupled to a durable before/after audit entry. Guarded to prd; a no-op elsewhere. + * migration when it is missing. The update is idempotent (only touches a null-or-blank verifiedName) + * and coupled to a durable before/after audit entry. Guarded to prd; a no-op elsewhere. * * The closing assertion checks the clearance predicate itself rather than equality with the supplied * name: should an identity-verified path have written a different (correct) name in the meantime, that diff --git a/src/subdomains/generic/user/models/user/__tests__/backfill-debug-staff-verified-name.migration.spec.ts b/src/subdomains/generic/user/models/user/__tests__/backfill-debug-staff-verified-name.migration.spec.ts index 89854673c3..fa45b21d43 100644 --- a/src/subdomains/generic/user/models/user/__tests__/backfill-debug-staff-verified-name.migration.spec.ts +++ b/src/subdomains/generic/user/models/user/__tests__/backfill-debug-staff-verified-name.migration.spec.ts @@ -240,11 +240,14 @@ describeDb('BackfillDebugStaffVerifiedName migration (real Postgres)', () => { // `BlankChars` is defined as every character `String.prototype.trim()` strips, so derive that set from // the runtime instead of restating it, and assert the migration's duplicated copy repairs a name built // from all of them at once. A copy that lost a code point — the drift the migration's own comment warns - // about — would leave such a name unrepaired and then fail its own assertion. + // about — would leave such a name unrepaired and still report success, because the postcondition + // shares the drifted constant and reads the residual character as non-blank. The migration cannot + // self-detect this; that is why the test asserts the repaired state rather than a rejection. it('repairs a name built from every character trim() strips, pinning the duplicated BlankChars', async () => { const blankChars = Array.from({ length: 0x10000 }, (_, code) => String.fromCharCode(code)).filter( (char) => char.trim() === '', ); + expect(blankChars.length).toBeGreaterThan(20); // sanity: the derivation actually found them await insertAccounts(blankChars.join('')); await new BackfillDebugStaffVerifiedName().up(queryRunner); @@ -302,7 +305,7 @@ describeDb('BackfillDebugStaffVerifiedName migration (real Postgres)', () => { ]); }); - it('rejects when the target row is absent and relies on the migration transaction to roll back', async () => { + it('rejects when the target row is absent, without writing an audit row first', async () => { await queryRunner.query(`INSERT INTO "user_data" ("id", "verifiedName") VALUES (${OTHER_ACCOUNT_ID}, $1)`, [ OTHER_ACCOUNT_NAME, ]); From 51cfd7f84c571a02b69c227232f31ae8f03a195c Mon Sep 17 00:00:00 2001 From: Daniel Padrino Date: Sat, 1 Aug 2026 23:45:15 -0300 Subject: [PATCH 5/5] docs(gs): make the recipe's negation NULL-total and require the audit coupling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Taken literally, "the exact negation of that assertion" produced `BTRIM("verifiedName", ) = ''` — without the COALESCE that the printed precondition one paragraph earlier does carry. That is not the negation over NULL: a NULL name yields NULL rather than true, so the ordinary un-backfilled account is neither repaired nor accepted, which is the same boot-fatal shape the paragraph exists to prevent. Verified as a mutant against the shipped SQL: three tests fail with the postcondition error. The instruction now spells out both forms and says which is wrong. The ban on equality assertions was justified with only one of its two failure modes, and that one is unreachable once the negation rule is applied — inviting the conclusion that the ban is redundant. Both branches are now stated: against a blankness precondition an equality assertion throws over a spelling difference, against its own negation it silently overwrites an identity-verified name. The recipe also never mentioned the audit coupling, though both shipped backfills gate the write on EXISTS (SELECT 1 FROM "audit") and CONTRIBUTING treats unaudited mutation of a PII column as blocking. An author following only the runbook produced an unaudited migration. Corrects the release-PR mechanism too: auto-release-pr.yaml checks for an open PR first and creates one only when none exists, so it keeps a release PR open continuously rather than opening one per push. The conclusion — develop is the last point where the deploy order can be arranged — is unchanged. --- docs/staff-kyc-clearance.md | 37 +++++++++++++------ ...ebug-staff-verified-name.migration.spec.ts | 2 +- 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/docs/staff-kyc-clearance.md b/docs/staff-kyc-clearance.md index 2e0e4d8d53..6e8cad6fca 100644 --- a/docs/staff-kyc-clearance.md +++ b/docs/staff-kyc-clearance.md @@ -35,25 +35,38 @@ be reviewed and reproducible like any other schema/data change. `UPDATE user_data SET "verifiedName" = … WHERE id = … AND BTRIM(COALESCE("verifiedName", ''), ) = ''`. The closing assertion must check the **clearance predicate itself** — that `… WHERE id = … AND BTRIM("verifiedName", ) <> ''` yields exactly one row — and never - equality with the supplied name. An equality assertion (as in the first such migration) throws when - an identity-verified path wrote a different but perfectly valid name in the meantime, failing the - deploy over a spelling difference. + equality with the supplied name. The goal state is a cleared account, not a particular spelling, and + an equality assertion fails whichever precondition it is paired with: against a blankness + precondition (the shape of the first such migration) it throws when an identity-verified path wrote a + different but perfectly valid name in the meantime; against its own negation it does the opposite and + silently overwrites that name. - The precondition must then be the **exact negation** of that assertion, using the same `BlankChars` - set as `StaffKycClearanceService`. A narrower `"verifiedName" IS NULL` precondition leaves a - present-but-blank name (a lone tab, a non-breaking space) as a state the migration refuses to repair - and then refuses to accept. + The precondition must then be the **exact negation** of that assertion **including the NULL case** — + `BTRIM(COALESCE("verifiedName", ''), ) = ''`, not + `BTRIM("verifiedName", ) = ''`. Without the `COALESCE`, NULL yields NULL rather than + true, so the ordinary un-backfilled account is neither repaired nor accepted. Use the same + `BlankChars` set as `StaffKycClearanceService`. A narrower `"verifiedName" IS NULL` precondition has + the mirror-image flaw: it leaves a present-but-blank name (a lone tab, a non-breaking space) as a + state the migration refuses to repair and then refuses to accept. - Both mistakes are boot-fatal, not merely wrong: `migrationsTransactionMode` defaults to `all`, so the - throw rolls back the whole release's migration batch and fails `DataSource.initialize()`. + The `UPDATE` must be coupled to a durable before/after audit row — a `log` insert (`system` `'User'`, + `subsystem` `'StaffVerifiedNameBackfill'`, `severity` `'Info'`) in the same statement via a + data-modifying CTE, with the update conditioned on `EXISTS (SELECT 1 FROM "audit")` so the column + cannot change unaudited. `verifiedName` is PII; CONTRIBUTING treats unaudited mutation of it as + blocking. + + Every one of these mistakes is boot-fatal rather than merely wrong: `migrationsTransactionMode` + defaults to `all`, so the throw rolls back the whole release's migration batch and fails + `DataSource.initialize()`. 2. **A real person's name is PII and must not be hard-coded in this public repo.** The migration reads the value from a deployment variable (e.g. `process.env.STAFF_VERIFIED_NAME_`), set in the production config. On PRD the variable is **mandatory**: the migration throws when it is absent, rather than silently recording a no-op — so the value has to be live in the production environment - before the migration is merged to `develop`. `auto-release-pr.yaml` opens the `develop` → `main` - release PR on every `develop` push, so `develop` is the last point at which the order can still be - arranged. A non-personal service designation (for a machine account that cannot complete a personal + before the migration is merged to `develop`. `auto-release-pr.yaml` keeps a `develop` → `main` + release PR open continuously — every `develop` push either opens one or lands on the one already + open — so `develop` is the last point at which the order can still be arranged. A non-personal + service designation (for a machine account that cannot complete a personal identification) is not PII and may appear inline. The concrete name↔account mapping is recorded in the private operations repo, not here. 3. Merge the PR through the normal review, then release `develop → main`. The production deploy runs diff --git a/src/subdomains/generic/user/models/user/__tests__/backfill-debug-staff-verified-name.migration.spec.ts b/src/subdomains/generic/user/models/user/__tests__/backfill-debug-staff-verified-name.migration.spec.ts index fa45b21d43..19b419f74a 100644 --- a/src/subdomains/generic/user/models/user/__tests__/backfill-debug-staff-verified-name.migration.spec.ts +++ b/src/subdomains/generic/user/models/user/__tests__/backfill-debug-staff-verified-name.migration.spec.ts @@ -86,7 +86,7 @@ describe('BackfillDebugStaffVerifiedName migration (SQL content)', () => { expect(sql).not.toContain('Test Staff Name'); // The precondition must be the exact negation of the postcondition — otherwise a present-but-blank - // name is a state the update refuses to produce and the assertion refuses to accept, and the deploy + // name is a state the update refuses to repair and the assertion refuses to accept, and the deploy // dies on a row the migration itself could have repaired. expect(sql).toContain("BTRIM(COALESCE(\"verifiedName\", ''), $2::varchar) = ''"); expect(sql).toContain("'action', CASE WHEN \"needsBackfill\" THEN 'backfilled' ELSE 'keptExistingName' END");