diff --git a/apps/api/src/trigger/policies/update-policy-helpers.spec.ts b/apps/api/src/trigger/policies/update-policy-helpers.spec.ts index f7ddc36ae0..e2068c34ea 100644 --- a/apps/api/src/trigger/policies/update-policy-helpers.spec.ts +++ b/apps/api/src/trigger/policies/update-policy-helpers.spec.ts @@ -31,6 +31,18 @@ jest.mock('@trigger.dev/sdk', () => ({ logger: { info: jest.fn(), warn: jest.fn(), error: jest.fn() }, })); +// Detached-PDF cleanup imports the S3 SDK dynamically; the mock intercepts it +// so tests can assert deletions without touching AWS. +const s3Send = jest.fn(); +jest.mock('@aws-sdk/client-s3', () => ({ + S3Client: class { + send = s3Send; + }, + DeleteObjectCommand: class { + constructor(public readonly input: { Bucket: string; Key: string }) {} + }, +})); + jest.mock('ai', () => ({ generateObject: jest.fn(), NoObjectGeneratedError: { isInstance: jest.fn(() => false) }, @@ -239,6 +251,7 @@ describe('updatePolicyInDatabase (draft policy regeneration)', () => { ]; let txPolicyUpdate: jest.Mock; + let txPolicyFind: jest.Mock; let txVersionUpdate: jest.Mock; let txVersionCreate: jest.Mock; let txVersionDeleteMany: jest.Mock; @@ -254,6 +267,7 @@ describe('updatePolicyInDatabase (draft policy regeneration)', () => { currentVersionId: 'pv_1', displayFormat: 'PDF', pdfUrl: 'org_1/policies/pol_1/uploaded.pdf', + currentVersion: { pdfUrl: 'org_1/policies/pol_1/v1.pdf' }, content: [ { type: 'paragraph', content: [{ type: 'text', text: 'Stale draft' }] }, ], @@ -264,6 +278,11 @@ describe('updatePolicyInDatabase (draft policy regeneration)', () => { }); txPolicyUpdate = jest.fn(); + // The in-transaction (row-locked) re-read that captures the detached keys. + txPolicyFind = jest.fn().mockResolvedValue({ + pdfUrl: 'org_1/policies/pol_1/uploaded.pdf', + currentVersion: { pdfUrl: 'org_1/policies/pol_1/v1.pdf' }, + }); txVersionUpdate = jest.fn(); txVersionCreate = jest.fn(() => ({ id: 'pv_2' })); txVersionDeleteMany = jest.fn(); @@ -271,7 +290,8 @@ describe('updatePolicyInDatabase (draft policy regeneration)', () => { (db.$transaction as jest.Mock).mockImplementation( async (cb: (tx: unknown) => Promise) => cb({ - policy: { update: txPolicyUpdate }, + $executeRaw: jest.fn(), + policy: { update: txPolicyUpdate, findUniqueOrThrow: txPolicyFind }, policyVersion: { update: txVersionUpdate, create: txVersionCreate, @@ -327,4 +347,91 @@ describe('updatePolicyInDatabase (draft policy regeneration)', () => { const versionUpdate = txVersionUpdate.mock.calls[0][0]; expect(versionUpdate.data.pdfUrl).toBeNull(); }); + + describe('detached-PDF S3 cleanup', () => { + const ENV_KEYS = [ + 'APP_AWS_BUCKET_NAME', + 'APP_AWS_ACCESS_KEY_ID', + 'APP_AWS_SECRET_ACCESS_KEY', + ] as const; + const ORIGINAL_ENV = Object.fromEntries( + ENV_KEYS.map((key) => [key, process.env[key]]), + ); + + const configureS3Env = () => { + process.env.APP_AWS_BUCKET_NAME = 'test-bucket'; + process.env.APP_AWS_ACCESS_KEY_ID = 'test-key'; + process.env.APP_AWS_SECRET_ACCESS_KEY = 'test-secret'; + }; + + afterEach(() => { + for (const key of ENV_KEYS) { + const original = ORIGINAL_ENV[key]; + if (original === undefined) delete process.env[key]; + else process.env[key] = original; + } + }); + + it('deletes the PDF keys captured inside the locked transaction', async () => { + configureS3Env(); + + await updatePolicyInDatabase('pol_1', REGEN_CONTENT, 'mem_regen'); + + // The keys come from the in-transaction re-read (not the outer snapshot), + // so a PDF uploaded concurrently before the row lock is still covered. + expect(txPolicyFind).toHaveBeenCalledTimes(1); + + const deletedKeys = s3Send.mock.calls.map( + (call) => (call[0] as { input: { Bucket: string; Key: string } }).input, + ); + expect(deletedKeys).toEqual([ + { Bucket: 'test-bucket', Key: 'org_1/policies/pol_1/uploaded.pdf' }, + { Bucket: 'test-bucket', Key: 'org_1/policies/pol_1/v1.pdf' }, + ]); + }); + + it('deduplicates when the policy and its version share one PDF key', async () => { + configureS3Env(); + txPolicyFind.mockResolvedValue({ + pdfUrl: 'org_1/policies/pol_1/shared.pdf', + currentVersion: { pdfUrl: 'org_1/policies/pol_1/shared.pdf' }, + }); + + await updatePolicyInDatabase('pol_1', REGEN_CONTENT, 'mem_regen'); + + expect(s3Send).toHaveBeenCalledTimes(1); + }); + + it('never fails the regeneration when an S3 delete fails (logged for cleanup)', async () => { + configureS3Env(); + s3Send.mockRejectedValueOnce(new Error('AccessDenied')); + + await expect( + updatePolicyInDatabase('pol_1', REGEN_CONTENT, 'mem_regen'), + ).resolves.toBeUndefined(); + // Both keys are still attempted; the failure is logged, not thrown. + expect(s3Send).toHaveBeenCalledTimes(2); + }); + + it('skips deletion (with a warning) when the S3 configuration is missing', async () => { + configureS3Env(); + delete process.env.APP_AWS_BUCKET_NAME; + + await updatePolicyInDatabase('pol_1', REGEN_CONTENT, 'mem_regen'); + + expect(s3Send).not.toHaveBeenCalled(); + }); + + it('does not touch S3 for an editor-mode draft with no PDFs', async () => { + configureS3Env(); + txPolicyFind.mockResolvedValue({ + pdfUrl: null, + currentVersion: { pdfUrl: null }, + }); + + await updatePolicyInDatabase('pol_1', REGEN_CONTENT, 'mem_regen'); + + expect(s3Send).not.toHaveBeenCalled(); + }); + }); }); diff --git a/apps/api/src/trigger/policies/update-policy-helpers.ts b/apps/api/src/trigger/policies/update-policy-helpers.ts index 3cff8397c4..fb42f31380 100644 --- a/apps/api/src/trigger/policies/update-policy-helpers.ts +++ b/apps/api/src/trigger/policies/update-policy-helpers.ts @@ -1,3 +1,4 @@ +import { DeleteObjectCommand, S3Client } from '@aws-sdk/client-s3'; import { db, Prisma, PolicyStatus } from '@db'; import type { FrameworkEditorFramework, @@ -71,6 +72,43 @@ export async function fetchOrganizationAndPolicy( // on the [policyId, version] key. const POLICY_VERSION_CREATE_RETRIES = 3; +/** + * Best-effort removal of the PDF objects a draft regeneration detached + * (pdfUrl stores the raw S3 key — same contract as the policies controller). + * Runs AFTER the database transaction commits so a failed delete can never + * roll back the content update; failures are logged with their keys so the + * objects can be cleaned up later instead of orphaning silently. + */ +async function deleteDetachedPdfObjects(keys: string[]): Promise { + if (keys.length === 0) return; + // Same APP_AWS_* configuration as the other trigger tasks (evidence export) + // — but non-throwing: cleanup is best-effort and must never fail the + // regeneration, so missing configuration is logged and skipped. + const bucketName = process.env.APP_AWS_BUCKET_NAME; + const accessKeyId = process.env.APP_AWS_ACCESS_KEY_ID; + const secretAccessKey = process.env.APP_AWS_SECRET_ACCESS_KEY; + if (!bucketName || !accessKeyId || !secretAccessKey) { + logger.warn( + `APP_AWS_* S3 configuration missing; skipped deleting detached policy PDFs: ${keys.join(', ')}`, + ); + return; + } + const s3 = new S3Client({ + region: process.env.APP_AWS_REGION || 'us-east-1', + credentials: { accessKeyId, secretAccessKey }, + ...(process.env.APP_AWS_ENDPOINT + ? { endpoint: process.env.APP_AWS_ENDPOINT, forcePathStyle: true } + : {}), + }); + for (const key of keys) { + try { + await s3.send(new DeleteObjectCommand({ Bucket: bucketName, Key: key })); + } catch (error) { + logger.warn(`Failed to delete detached policy PDF ${key}: ${error}`); + } + } +} + export async function updatePolicyInDatabase( policyId: string, content: Record[], @@ -103,7 +141,18 @@ export async function updatePolicyInDatabase( // currentVersion.pdfUrl ?? policy.pdfUrl) keep serving the old uploaded // document instead of the regenerated content (CS-766). if (policy.status === PolicyStatus.draft) { - await db.$transaction(async (tx) => { + // The uploaded-PDF keys this regeneration detaches, captured INSIDE the + // transaction under a row lock: a concurrent PDF upload (a plain UPDATE + // on the policy row) blocks on the lock, so the captured keys are + // exactly the values the updates below clear — nothing committed in + // between can slip an untracked orphan past the cleanup. Deleted from + // S3 only after the transaction commits. + const detachedPdfKeys = await db.$transaction(async (tx) => { + await tx.$executeRaw`SELECT id FROM "Policy" WHERE id = ${policyId} FOR UPDATE`; + const current = await tx.policy.findUniqueOrThrow({ + where: { id: policyId }, + select: { pdfUrl: true, currentVersion: { select: { pdfUrl: true } } }, + }); if (policy.currentVersionId) { await tx.policyVersion.update({ where: { id: policy.currentVersionId }, @@ -123,7 +172,15 @@ export async function updatePolicyInDatabase( displayFormat: 'EDITOR', }, }); + return [ + ...new Set( + [current.pdfUrl, current.currentVersion?.pdfUrl].filter( + (key): key is string => !!key, + ), + ), + ]; }); + await deleteDetachedPdfObjects(detachedPdfKeys); return; }