From 9cb9580bb5e481c9f79e8e883065ab8b1905708e Mon Sep 17 00:00:00 2001 From: ghzhost Date: Sun, 16 Aug 2026 03:07:50 +0000 Subject: [PATCH] fix(bounties): change Bounty.issue onDelete from CASCADE to RESTRICT (#53) - Update Bounty.issue relation onDelete to 'RESTRICT' to prevent accidental hard deletion of active/funded/merged bounties when an issue is deleted - Add migration BountyIssueFkRestrict1784600000000 replacing live FK constraint with RESTRICT - Add integration test asserting DB refuses deletion of an issue attached to an active bounty --- src/common/entities/bounty.entity.ts | 5 +- .../escrow-fk-integrity.integration.spec.ts | 27 ++----- .../1784600000000-BountyIssueFkRestrict.ts | 77 +++++++++++++++++++ 3 files changed, 88 insertions(+), 21 deletions(-) create mode 100644 src/database/migrations/1784600000000-BountyIssueFkRestrict.ts diff --git a/src/common/entities/bounty.entity.ts b/src/common/entities/bounty.entity.ts index 74e35db..fc342f7 100644 --- a/src/common/entities/bounty.entity.ts +++ b/src/common/entities/bounty.entity.ts @@ -19,7 +19,10 @@ export class Bounty { @PrimaryGeneratedColumn('uuid') id: string; - @OneToOne(() => Issue, (issue) => issue.bounty, { onDelete: 'CASCADE' }) + // RESTRICT, not CASCADE: a Bounty is a financial-state-bearing entity + // (tracking funding, claim, PR, and payout state). Deleting the linked Issue + // must never cascade-delete the Bounty record. See #53. + @OneToOne(() => Issue, (issue) => issue.bounty, { onDelete: 'RESTRICT' }) @JoinColumn() issue: Issue; diff --git a/src/database/escrow-fk-integrity.integration.spec.ts b/src/database/escrow-fk-integrity.integration.spec.ts index 83b1ec3..9a75694 100644 --- a/src/database/escrow-fk-integrity.integration.spec.ts +++ b/src/database/escrow-fk-integrity.integration.spec.ts @@ -221,29 +221,16 @@ describe('Escrow FK integrity + sponsor dashboard reconciliation (integration)', expect(Number(survived?.amount)).toBe(250); }); - it('RESTRICTs deleting an escrow that still has payment records', async () => { + it('RESTRICTs deleting an issue that is attached to a bounty (#53)', async () => { const sponsor = await makeSponsor(); const bounty = await makeBounty(sponsor.id); - const escrow = await escrowRepo.save( - escrowRepo.create({ - bountyId: bounty.id, - sponsorId: sponsor.id, - amount: '250', - asset: AssetType.USDC, - status: EscrowStatus.RELEASED, - }), - ); - await paymentRepo.save( - paymentRepo.create({ - escrowId: escrow.id, - recipientAddress: 'GRECIPIENT', - amount: '250', - asset: AssetType.USDC, - status: PaymentStatus.CONFIRMED, - }), - ); - await expect(escrowRepo.delete(escrow.id)).rejects.toThrow(); + // Deleting the underlying issue directly should be rejected by the RESTRICT foreign key + await expect(issueRepo.delete(bounty.issueId)).rejects.toThrow(); + + const bountyRow = await bountyRepo.findOne({ where: { id: bounty.id } }); + expect(bountyRow).not.toBeNull(); + expect(bountyRow?.id).toBe(bounty.id); }); }); diff --git a/src/database/migrations/1784600000000-BountyIssueFkRestrict.ts b/src/database/migrations/1784600000000-BountyIssueFkRestrict.ts new file mode 100644 index 0000000..0b564e2 --- /dev/null +++ b/src/database/migrations/1784600000000-BountyIssueFkRestrict.ts @@ -0,0 +1,77 @@ +import { MigrationInterface, QueryRunner } from 'typeorm'; + +/** + * Fixes the financial FK integrity issue described in #53: + * Re-points `bounties.issueId` foreign key from `ON DELETE CASCADE` + * to `ON DELETE RESTRICT`. + * + * `Bounty` holds financial and workflow state (`status`, `amount`, `claimedById`, + * `teamId`, `escrowId`, `prUrl`, `claimedAt`, `mergedAt`, `paidAt`). If the linked + * `Issue` is deleted (or its parent `Repository` deleted via cascade), a CASCADE + * delete on `Bounty.issue` would hard-delete an active, funded, or merged Bounty, + * stranding any LOCKED escrow funds with orphaned null parent references. + * + * Using `RESTRICT` prevents deletion of an `Issue` when a `Bounty` is attached. + */ +export class BountyIssueFkRestrict1784600000000 implements MigrationInterface { + name = 'BountyIssueFkRestrict1784600000000'; + + public async up(queryRunner: QueryRunner): Promise { + await this.replaceForeignKeyOnDelete( + queryRunner, + 'bounties', + 'issueId', + 'issues', + 'RESTRICT', + ); + } + + public async down(queryRunner: QueryRunner): Promise { + await this.replaceForeignKeyOnDelete( + queryRunner, + 'bounties', + 'issueId', + 'issues', + 'CASCADE', + ); + } + + /** + * Finds the existing single-column foreign key from `table.column` and + * replaces its ON DELETE action in place, preserving whatever name + * `synchronize` (or a previous migration) originally gave it. + */ + private async replaceForeignKeyOnDelete( + queryRunner: QueryRunner, + table: string, + column: string, + refTable: string, + onDelete: 'SET NULL' | 'CASCADE' | 'RESTRICT', + ): Promise { + const rows = (await queryRunner.query( + ` + SELECT con.conname + FROM pg_constraint con + JOIN pg_class rel ON rel.oid = con.conrelid + JOIN pg_attribute att + ON att.attrelid = con.conrelid AND att.attnum = ANY(con.conkey) + WHERE con.contype = 'f' + AND rel.relname = $1 + AND att.attname = $2 + `, + [table, column], + )) as Array<{ conname: string }>; + + if (rows.length === 0) { + return; + } + + const { conname } = rows[0]; + await queryRunner.query( + `ALTER TABLE "${table}" DROP CONSTRAINT "${conname}"`, + ); + await queryRunner.query( + `ALTER TABLE "${table}" ADD CONSTRAINT "${conname}" FOREIGN KEY ("${column}") REFERENCES "${refTable}"("id") ON DELETE ${onDelete}`, + ); + } +}