From ce3af4d17eecc5428087a154206f002900f7a267 Mon Sep 17 00:00:00 2001 From: Mattsson <111893710+mattssonn@users.noreply.github.com> Date: Wed, 6 May 2026 14:08:38 +0200 Subject: [PATCH] Fix/multiple domain issue (#401) * feat: enhance invoice management and immutability checks - Update InvoiceDetailPage to prevent deletion of drafts with assigned invoice numbers, providing user feedback. - Modify the invoice conversion API to ensure invoice number allocation occurs only after successful item insertion and proforma cancellation. - Implement structured error responses for invoice deletion, ensuring only drafts without assigned numbers can be deleted. - Add comprehensive tests for invoice deletion and conversion scenarios, including edge cases for draft invoices. - Introduce immutability checks in the document management system to prevent unauthorized changes to linked documents. - Create SQL migration to enforce document metadata immutability, ensuring compliance with accounting regulations. * fix(invoice): prevent invoice number consumption on PDF render failure * feat: add document journal entry immutability enforcement for delete_last_voucher RPC * fix(invoice): implement rollback for orphan invoices on proforma cancel failure * fix(document): extend immutability trigger to protect journal entry links --- app/(dashboard)/invoices/[id]/page.tsx | 44 +-- app/api/invoices/[id]/__tests__/route.test.ts | 126 +++++++++ .../[id]/convert/__tests__/route.test.ts | 225 +++++++++++++++ app/api/invoices/[id]/convert/route.ts | 54 ++-- app/api/invoices/[id]/route.ts | 30 +- .../[id]/send/__tests__/route.test.ts | 27 ++ app/api/invoices/[id]/send/route.ts | 41 ++- .../__tests__/delete-last-voucher.pg.test.ts | 44 +++ lib/bookkeeping/__tests__/vat-entries.test.ts | 266 ++++++++++++++++++ .../document-immutability.pg.test.ts | 233 +++++++++++++++ lib/errors/structured-errors.ts | 25 ++ ...ment_journal_entry_immutability_bypass.sql | 41 +++ ...06150000_protect_document_journal_link.sql | 64 +++++ 13 files changed, 1159 insertions(+), 61 deletions(-) create mode 100644 app/api/invoices/[id]/__tests__/route.test.ts create mode 100644 app/api/invoices/[id]/convert/__tests__/route.test.ts create mode 100644 lib/bookkeeping/__tests__/vat-entries.test.ts create mode 100644 lib/core/documents/__tests__/document-immutability.pg.test.ts create mode 100644 supabase/migrations/20260506140000_document_journal_entry_immutability_bypass.sql create mode 100644 supabase/migrations/20260506150000_protect_document_journal_link.sql diff --git a/app/(dashboard)/invoices/[id]/page.tsx b/app/(dashboard)/invoices/[id]/page.tsx index 633647f7..d56b5443 100644 --- a/app/(dashboard)/invoices/[id]/page.tsx +++ b/app/(dashboard)/invoices/[id]/page.tsx @@ -904,15 +904,24 @@ export default function InvoiceDetailPage({ params }: { params: Promise<{ id: st

)} - + {invoice.invoice_number ? ( +
+ +

+ Utkastet har redan tilldelats löpnummer {invoice.invoice_number} och kan inte tas bort. Försök skicka fakturan igen — om sändningen lyckas behövs inget annat steg. +

+
+ ) : ( + + )} )} {(invoice.status === 'sent' || invoice.status === 'overdue') && isRealInvoice && ( @@ -947,22 +956,17 @@ export default function InvoiceDetailPage({ params }: { params: Promise<{ id: st - {/* Delete confirmation dialog */} + {/* Delete confirmation dialog. Only reachable when invoice_number is null; + numbered drafts surface an inline retry-send notice instead. */} Ta bort fakturautkast - Är du säker på att du vill ta bort {invoice.invoice_number ? `utkast ${invoice.invoice_number}` : 'utkastet'}? Detta kan inte ångras. - {invoice.invoice_number ? ( - - Löpnummer {invoice.invoice_number} är redan reserverat och kommer att bli ett permanent hopp i fakturaserien. - - ) : ( - - Inget löpnummer har tilldelats — fakturaserien påverkas inte. - - )} + Är du säker på att du vill ta bort utkastet? Detta kan inte ångras. + + Inget löpnummer har tilldelats — fakturaserien påverkas inte. + diff --git a/app/api/invoices/[id]/__tests__/route.test.ts b/app/api/invoices/[id]/__tests__/route.test.ts new file mode 100644 index 00000000..ec110ce2 --- /dev/null +++ b/app/api/invoices/[id]/__tests__/route.test.ts @@ -0,0 +1,126 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { + createMockRequest, + createMockRouteParams, + parseJsonResponse, + createQueuedMockSupabase, +} from '@/tests/helpers' + +const { supabase: mockSupabase, enqueue, reset } = createQueuedMockSupabase() +vi.mock('@/lib/supabase/server', () => ({ + createClient: () => Promise.resolve(mockSupabase), +})) + +vi.mock('@/lib/company/context', () => ({ + requireCompanyId: vi.fn().mockResolvedValue('company-1'), +})) + +vi.mock('@/lib/auth/require-write', () => ({ + requireWritePermission: vi.fn().mockResolvedValue({ ok: true }), +})) + +import { DELETE } from '../route' + +describe('DELETE /api/invoices/[id]', () => { + const mockUser = { id: 'user-1', email: 'test@test.se' } + + beforeEach(() => { + vi.clearAllMocks() + reset() + mockSupabase.auth.getUser.mockResolvedValue({ data: { user: mockUser } }) + }) + + it('returns 401 when not authenticated', async () => { + mockSupabase.auth.getUser.mockResolvedValue({ data: { user: null } }) + + const response = await DELETE( + createMockRequest('/api/invoices/inv-1', { method: 'DELETE' }), + createMockRouteParams({ id: 'inv-1' }) + ) + const { status, body } = await parseJsonResponse<{ error: string }>(response) + + expect(status).toBe(401) + expect(body.error).toBe('Unauthorized') + }) + + it('returns 404 when invoice not found', async () => { + enqueue({ data: null, error: { message: 'not found' } }) + + const response = await DELETE( + createMockRequest('/api/invoices/inv-1', { method: 'DELETE' }), + createMockRouteParams({ id: 'inv-1' }) + ) + const { status } = await parseJsonResponse(response) + + expect(status).toBe(404) + }) + + it('rejects deletion of a non-draft invoice with INVOICE_DELETE_NOT_DRAFT', async () => { + enqueue({ + data: { id: 'inv-1', status: 'sent', invoice_number: 'F-2026099', user_id: 'user-1' }, + error: null, + }) + + const response = await DELETE( + createMockRequest('/api/invoices/inv-1', { method: 'DELETE' }), + createMockRouteParams({ id: 'inv-1' }) + ) + const { status, body } = await parseJsonResponse<{ error: { code: string } }>(response) + + expect(status).toBe(400) + expect(body.error.code).toBe('INVOICE_DELETE_NOT_DRAFT') + }) + + it('rejects deletion of a draft that already has an invoice_number', async () => { + enqueue({ + data: { id: 'inv-1', status: 'draft', invoice_number: 'F-2026001', user_id: 'user-1' }, + error: null, + }) + + const response = await DELETE( + createMockRequest('/api/invoices/inv-1', { method: 'DELETE' }), + createMockRouteParams({ id: 'inv-1' }) + ) + const { status, body } = await parseJsonResponse<{ + error: { code: string; details?: { invoice_number?: string } } + }>(response) + + expect(status).toBe(400) + expect(body.error.code).toBe('INVOICE_DELETE_NUMBERED') + expect(body.error.details?.invoice_number).toBe('F-2026001') + }) + + it('deletes a draft with no invoice_number', async () => { + enqueue({ + data: { id: 'inv-1', status: 'draft', invoice_number: null, user_id: 'user-1' }, + error: null, + }) + enqueue({ data: null, error: null }) + enqueue({ data: null, error: null }) + + const response = await DELETE( + createMockRequest('/api/invoices/inv-1', { method: 'DELETE' }), + createMockRouteParams({ id: 'inv-1' }) + ) + const { status, body } = await parseJsonResponse<{ data: { deleted: boolean } }>(response) + + expect(status).toBe(200) + expect(body.data.deleted).toBe(true) + }) + + it('returns 500 when items delete fails', async () => { + enqueue({ + data: { id: 'inv-1', status: 'draft', invoice_number: null, user_id: 'user-1' }, + error: null, + }) + enqueue({ data: null, error: { message: 'items delete failed' } }) + + const response = await DELETE( + createMockRequest('/api/invoices/inv-1', { method: 'DELETE' }), + createMockRouteParams({ id: 'inv-1' }) + ) + const { status } = await parseJsonResponse(response) + + expect(status).toBe(500) + }) +}) diff --git a/app/api/invoices/[id]/convert/__tests__/route.test.ts b/app/api/invoices/[id]/convert/__tests__/route.test.ts new file mode 100644 index 00000000..9a2275ef --- /dev/null +++ b/app/api/invoices/[id]/convert/__tests__/route.test.ts @@ -0,0 +1,225 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { + createMockRequest, + createMockRouteParams, + parseJsonResponse, + createQueuedMockSupabase, +} from '@/tests/helpers' +import { eventBus } from '@/lib/events' + +const { supabase: mockSupabase, enqueue, reset } = createQueuedMockSupabase() +vi.mock('@/lib/supabase/server', () => ({ + createClient: () => Promise.resolve(mockSupabase), +})) + +vi.mock('@/lib/init', () => ({ + ensureInitialized: vi.fn(), +})) + +vi.mock('@/lib/company/context', () => ({ + requireCompanyId: vi.fn().mockResolvedValue('company-1'), +})) + +vi.mock('@/lib/auth/require-write', () => ({ + requireWritePermission: vi.fn().mockResolvedValue({ ok: true }), +})) + +import { POST } from '../route' + +describe('POST /api/invoices/[id]/convert', () => { + const mockUser = { id: 'user-1', email: 'test@test.se' } + + const baseProforma = { + id: 'pf-1', + document_type: 'proforma', + status: 'draft', + customer_id: 'customer-1', + due_date: '2026-06-15', + currency: 'SEK', + exchange_rate: null, + exchange_rate_date: null, + subtotal: 10000, + subtotal_sek: null, + vat_amount: 2500, + vat_amount_sek: null, + total: 12500, + total_sek: null, + vat_treatment: 'standard_25', + vat_rate: 25, + moms_ruta: '10', + reverse_charge_text: null, + your_reference: null, + our_reference: null, + notes: null, + items: [ + { + sort_order: 0, + description: 'Konsultation', + quantity: 10, + unit: 'tim', + unit_price: 1000, + line_total: 10000, + }, + ], + } + + beforeEach(() => { + vi.clearAllMocks() + reset() + eventBus.clear() + mockSupabase.auth.getUser.mockResolvedValue({ data: { user: mockUser } }) + }) + + it('returns 401 when not authenticated', async () => { + mockSupabase.auth.getUser.mockResolvedValue({ data: { user: null } }) + + const response = await POST( + createMockRequest('/api/invoices/pf-1/convert', { method: 'POST' }), + createMockRouteParams({ id: 'pf-1' }) + ) + const { status } = await parseJsonResponse(response) + + expect(status).toBe(401) + }) + + it('returns 404 when proforma not found', async () => { + enqueue({ data: null, error: { message: 'not found' } }) + + const response = await POST( + createMockRequest('/api/invoices/pf-1/convert', { method: 'POST' }), + createMockRouteParams({ id: 'pf-1' }) + ) + const { status } = await parseJsonResponse(response) + + expect(status).toBe(404) + }) + + it('returns 400 when invoice is not a proforma', async () => { + enqueue({ data: { ...baseProforma, document_type: 'invoice' }, error: null }) + + const response = await POST( + createMockRequest('/api/invoices/pf-1/convert', { method: 'POST' }), + createMockRouteParams({ id: 'pf-1' }) + ) + const { status } = await parseJsonResponse(response) + + expect(status).toBe(400) + }) + + it('does NOT advance the F-series counter when items insert fails', async () => { + // 1. fetch proforma + enqueue({ data: baseProforma, error: null }) + // 2. insert real invoice with null number — succeeds + enqueue({ + data: { id: 'inv-1', invoice_number: null, document_type: 'invoice' }, + error: null, + }) + // 3. insert items — FAILS + enqueue({ data: null, error: { message: 'items insert failed' } }) + // 4. rollback delete of orphan row — succeeds + enqueue({ data: null, error: null }) + + const response = await POST( + createMockRequest('/api/invoices/pf-1/convert', { method: 'POST' }), + createMockRouteParams({ id: 'pf-1' }) + ) + const { status } = await parseJsonResponse(response) + + expect(status).toBe(500) + // Critical: counter must not have been touched. + expect(mockSupabase.rpc).not.toHaveBeenCalled() + }) + + it('rolls back the orphan invoice when proforma cancel fails', async () => { + // 1. fetch proforma + enqueue({ data: baseProforma, error: null }) + // 2. insert real invoice + enqueue({ + data: { id: 'inv-1', invoice_number: null, document_type: 'invoice' }, + error: null, + }) + // 3. insert items + enqueue({ data: null, error: null }) + // 4. cancel proforma — FAILS + enqueue({ data: null, error: { message: 'cancel failed' } }) + // 5. rollback delete of orphan invoice + enqueue({ data: null, error: null }) + + const response = await POST( + createMockRequest('/api/invoices/pf-1/convert', { method: 'POST' }), + createMockRouteParams({ id: 'pf-1' }) + ) + const { status, body } = await parseJsonResponse<{ error: string }>(response) + + expect(status).toBe(500) + expect(body.error).toContain('cancel failed') + // Counter must not have been touched and orphan invoice must have been + // deleted (5 enqueued calls all consumed). + expect(mockSupabase.rpc).not.toHaveBeenCalled() + }) + + it('rolls back invoice + un-cancels proforma when number allocation fails', async () => { + // 1. fetch proforma + enqueue({ data: baseProforma, error: null }) + // 2. insert real invoice + enqueue({ + data: { id: 'inv-1', invoice_number: null, document_type: 'invoice' }, + error: null, + }) + // 3. insert items + enqueue({ data: null, error: null }) + // 4. cancel proforma — succeeds + enqueue({ data: null, error: null }) + // 5. ensureInvoiceNumber → rpc THROWS + enqueue({ data: null, error: { message: 'number allocation failed' } }) + // 6. un-cancel proforma (restore previous status) + enqueue({ data: null, error: null }) + // 7. delete orphan invoice + enqueue({ data: null, error: null }) + + const response = await POST( + createMockRequest('/api/invoices/pf-1/convert', { method: 'POST' }), + createMockRouteParams({ id: 'pf-1' }) + ) + const { status } = await parseJsonResponse(response) + + expect(status).toBe(500) + }) + + it('allocates the F-number after items + proforma cancel succeed', async () => { + // 1. fetch proforma + enqueue({ data: baseProforma, error: null }) + // 2. insert real invoice + enqueue({ + data: { id: 'inv-1', invoice_number: null, document_type: 'invoice' }, + error: null, + }) + // 3. insert items + enqueue({ data: null, error: null }) + // 4. cancel proforma + enqueue({ data: null, error: null }) + // 5. ensureInvoiceNumber → rpc returns the assigned F-number + enqueue({ data: 'F-2026005', error: null }) + // 6. fetch complete invoice + enqueue({ + data: { id: 'inv-1', invoice_number: 'F-2026005', items: [] }, + error: null, + }) + + const response = await POST( + createMockRequest('/api/invoices/pf-1/convert', { method: 'POST' }), + createMockRouteParams({ id: 'pf-1' }) + ) + const { status, body } = await parseJsonResponse<{ data: { invoice_number: string } }>(response) + + expect(status).toBe(200) + expect(body.data.invoice_number).toBe('F-2026005') + expect(mockSupabase.rpc).toHaveBeenCalledWith( + 'generate_invoice_number', + expect.objectContaining({ + p_company_id: 'company-1', + p_invoice_id: 'inv-1', + }) + ) + }) +}) diff --git a/app/api/invoices/[id]/convert/route.ts b/app/api/invoices/[id]/convert/route.ts index b2304ad0..de737280 100644 --- a/app/api/invoices/[id]/convert/route.ts +++ b/app/api/invoices/[id]/convert/route.ts @@ -14,6 +14,11 @@ ensureInitialized() * * Converts a proforma invoice to a real invoice. * Copies all data, generates a real invoice number, and marks the proforma as cancelled. + * + * Ordering note: ensureInvoiceNumber() is the LAST side effect. The F-series + * counter only advances after items are inserted and the proforma is marked + * cancelled — so a partial failure in any earlier step rolls back the orphan + * row without leaking a number. */ export async function POST( request: Request, @@ -33,7 +38,6 @@ export async function POST( const companyId = await requireCompanyId(supabase, user.id) - // Fetch proforma with items const { data: proforma, error: proformaError } = await supabase .from('invoices') .select('*, items:invoice_items(*)') @@ -59,9 +63,6 @@ export async function POST( ) } - // Create the real invoice with invoice_number=null; assign atomically below. - // generate_invoice_number now requires the target row to exist so it can lock - // it (FOR UPDATE) and persist the number in the same transaction. const { data: invoice, error: invoiceError } = await supabase .from('invoices') .insert({ @@ -97,21 +98,6 @@ export async function POST( return NextResponse.json({ error: invoiceError.message }, { status: 500 }) } - // Now that the row exists, allocate the F-series number. Mutates invoice - // in place so the response includes the assigned number. - try { - await ensureInvoiceNumber(supabase, companyId, invoice as Invoice) - } catch (err) { - // Roll back the partially-created invoice so the company counter is the - // only side effect to clean up (manually in worst case). - await supabase.from('invoices').delete().eq('id', invoice.id) - return NextResponse.json( - { error: err instanceof Error ? err.message : 'Failed to assign invoice number' }, - { status: 500 } - ) - } - - // Copy invoice items const items = (proforma.items || []).map((item: { sort_order: number; description: string; quantity: number; unit: string; unit_price: number; line_total: number }) => ({ invoice_id: invoice.id, sort_order: item.sort_order, @@ -133,13 +119,37 @@ export async function POST( } } - // Mark proforma as cancelled - await supabase + // Cancel the proforma. If this fails, the new (still unnumbered) invoice + // is an orphan — delete it so the user can retry without ending up with + // two active invoices for the same proforma. invoice_items cascade. + const previousProformaStatus = proforma.status + const { error: cancelError } = await supabase .from('invoices') .update({ status: 'cancelled' }) .eq('id', id) - // Fetch complete invoice + if (cancelError) { + await supabase.from('invoices').delete().eq('id', invoice.id) + return NextResponse.json({ error: cancelError.message }, { status: 500 }) + } + + // Allocate the F-series number last. If allocation fails, restore the + // proforma's previous status and delete the orphan invoice. The F-counter + // is unaffected because generate_invoice_number only commits on success. + try { + await ensureInvoiceNumber(supabase, companyId, invoice as Invoice) + } catch (err) { + await supabase + .from('invoices') + .update({ status: previousProformaStatus }) + .eq('id', id) + await supabase.from('invoices').delete().eq('id', invoice.id) + return NextResponse.json( + { error: err instanceof Error ? err.message : 'Failed to assign invoice number' }, + { status: 500 } + ) + } + const { data: completeInvoice } = await supabase .from('invoices') .select('*, customer:customers(*), items:invoice_items(*)') diff --git a/app/api/invoices/[id]/route.ts b/app/api/invoices/[id]/route.ts index d518e360..a5064509 100644 --- a/app/api/invoices/[id]/route.ts +++ b/app/api/invoices/[id]/route.ts @@ -2,13 +2,24 @@ import { createClient } from '@/lib/supabase/server' import { NextResponse } from 'next/server' import { requireCompanyId } from '@/lib/company/context' import { requireWritePermission } from '@/lib/auth/require-write' +import { errorResponseFromCode } from '@/lib/errors/get-structured-error' +import { createLogger } from '@/lib/logger' + +const log = createLogger('api.invoices.delete') /** * DELETE /api/invoices/[id] * * Permanently deletes a draft invoice and its items. - * Only invoices with status 'draft' can be deleted — committed invoices - * are immutable per BFL and must be reversed via credit note instead. + * + * Two preconditions: + * 1. status === 'draft' — committed invoices are immutable per BFL and + * must be reversed via credit note. + * 2. invoice_number IS NULL — a draft that already holds an F-series + * number is a side effect of an interrupted send/convert/mark-sent. + * Destroying it would orphan the number and create a permanent gap + * in the verifications series. Refuse and let the user retry the + * send instead (ensureInvoiceNumber is idempotent). */ export async function DELETE( request: Request, @@ -28,10 +39,9 @@ export async function DELETE( const companyId = await requireCompanyId(supabase, user.id) - // Fetch invoice to verify ownership and status const { data: invoice, error: fetchError } = await supabase .from('invoices') - .select('id, status, user_id') + .select('id, status, invoice_number, user_id') .eq('id', id) .eq('company_id', companyId) .single() @@ -41,13 +51,15 @@ export async function DELETE( } if (invoice.status !== 'draft') { - return NextResponse.json( - { error: 'Endast utkast kan tas bort. Bokförda fakturor måste krediteras istället.' }, - { status: 400 } - ) + return errorResponseFromCode('INVOICE_DELETE_NOT_DRAFT', log) + } + + if (invoice.invoice_number !== null) { + return errorResponseFromCode('INVOICE_DELETE_NUMBERED', log, { + details: { invoice_number: invoice.invoice_number }, + }) } - // Delete items first (FK constraint), then the invoice const { error: itemsError } = await supabase .from('invoice_items') .delete() diff --git a/app/api/invoices/[id]/send/__tests__/route.test.ts b/app/api/invoices/[id]/send/__tests__/route.test.ts index 4eca859c..4d085875 100644 --- a/app/api/invoices/[id]/send/__tests__/route.test.ts +++ b/app/api/invoices/[id]/send/__tests__/route.test.ts @@ -303,6 +303,33 @@ describe('POST /api/invoices/[id]/send', () => { expect(mockSupabase.rpc).not.toHaveBeenCalledWith('generate_invoice_number', expect.anything()) }) + it('does NOT consume an invoice number when PDF render fails (preflight)', async () => { + const draftWithoutNumber = makeInvoice({ + id: 'inv-1', + status: 'draft', + invoice_number: null, + customer, + items: invoice.items, + }) + + enqueue({ data: draftWithoutNumber, error: null }) + enqueue({ data: company, error: null }) + + // First render call (the preflight) throws. + mockRenderToBuffer.mockRejectedValueOnce(new Error('PDF render exploded')) + + const request = createMockRequest('/api/invoices/inv-1/send', { method: 'POST' }) + const response = await POST(request, createMockRouteParams({ id: 'inv-1' })) + const { status, body } = await parseJsonResponse<{ error: string }>(response) + + expect(status).toBe(500) + expect((body.error as unknown as { code: string }).code).toBe('INVOICE_SEND_PDF_RENDER_FAILED') + // Critical: counter must not have advanced. + expect(mockSupabase.rpc).not.toHaveBeenCalledWith('generate_invoice_number', expect.anything()) + // Email must not have been attempted. + expect(mockSendEmail).not.toHaveBeenCalled() + }) + it('returns 500 when email sending fails', async () => { enqueue({ data: invoice, error: null }) enqueue({ data: company, error: null }) diff --git a/app/api/invoices/[id]/send/route.ts b/app/api/invoices/[id]/send/route.ts index 18403dcb..9186e0f1 100644 --- a/app/api/invoices/[id]/send/route.ts +++ b/app/api/invoices/[id]/send/route.ts @@ -63,15 +63,6 @@ export const POST = withRouteContext( return errorResponseFromCode('INVOICE_SEND_COMPANY_SETTINGS_MISSING', opLog, { requestId }) } - // Eagerly assign the invoice number — drafts get one only at send time so - // discarded drafts never consume a number. - try { - await ensureInvoiceNumber(supabase, companyId!, invoice as Invoice) - } catch (err) { - opLog.error('failed to assign invoice number on send', err as Error) - return errorResponseFromCode('INVOICE_SEND_NUMBER_ASSIGN_FAILED', opLog, { requestId }) - } - const items = (invoice.items as InvoiceItem[]).sort((a, b) => a.sort_order - b.sort_order) let originalInvoiceNumber: string | undefined @@ -88,7 +79,37 @@ export const POST = withRouteContext( } } - // Generate PDF — non-fatal failures here become PARTIAL after send. + // Preflight render: validate the PDF pipeline BEFORE consuming an F-series + // number. If the row is already numbered (retry path), skip — we'd just + // render twice for no gain. + const isFreshAllocation = !invoice.invoice_number + if (isFreshAllocation) { + try { + await renderToBuffer( + InvoicePDF({ + invoice: { ...(invoice as Invoice), invoice_number: 'F-PREVIEW' }, + customer, + items, + company: company as CompanySettings, + originalInvoiceNumber, + }), + ) + } catch (err) { + opLog.error('preflight PDF render failed before invoice number assignment', err as Error) + return errorResponseFromCode('INVOICE_SEND_PDF_RENDER_FAILED', opLog, { requestId }) + } + } + + // Allocate the F-series number. Idempotent — retries reuse the same number. + try { + await ensureInvoiceNumber(supabase, companyId!, invoice as Invoice) + } catch (err) { + opLog.error('failed to assign invoice number on send', err as Error) + return errorResponseFromCode('INVOICE_SEND_NUMBER_ASSIGN_FAILED', opLog, { requestId }) + } + + // Final render with the assigned number — this is the buffer attached to + // the email and later archived as underlag. const pdfBuffer = await renderToBuffer( InvoicePDF({ invoice: invoice as Invoice, diff --git a/lib/bookkeeping/__tests__/delete-last-voucher.pg.test.ts b/lib/bookkeeping/__tests__/delete-last-voucher.pg.test.ts index 4004c21c..02d1c6d9 100644 --- a/lib/bookkeeping/__tests__/delete-last-voucher.pg.test.ts +++ b/lib/bookkeeping/__tests__/delete-last-voucher.pg.test.ts @@ -129,6 +129,50 @@ describe('delete_last_voucher.pg — RPC + immutability trigger interaction', () } }) + it('clears journal_entry_id on attached documents and deletes the voucher', async () => { + // Regression for the document-immutability triggers ignoring the + // gnubok.allow_delete bypass. delete_last_voucher unlinks documents + // (UPDATE document_attachments SET journal_entry_id = NULL) before + // deleting the entry; if the trigger refused the unlink the whole RPC + // would fail and the entry would remain. + const { userId, companyId, fiscalPeriodId } = await seedCompany() + const entryId = await insertPostedEntryWithLines({ + userId, companyId, fiscalPeriodId, voucherNumber: 1, + }) + const docId = randomUUID() + await getPool().query( + `INSERT INTO public.document_attachments + (id, user_id, company_id, storage_path, file_name, file_size_bytes, + mime_type, sha256_hash, journal_entry_id) + VALUES ($1, $2, $3, $4, 'underlag.pdf', 1024, 'application/pdf', $5, $6)`, + [ + docId, + userId, + companyId, + `documents/${userId}/${docId}.pdf`, + 'a'.repeat(64), + entryId, + ], + ) + + await withUserContext(userId, async (client) => { + await client.query( + `SELECT public.delete_last_voucher($1::uuid, $2::uuid)`, + [companyId, entryId], + ) + const entryAfter = await client.query( + `SELECT 1 FROM public.journal_entries WHERE id = $1`, + [entryId], + ) + expect(entryAfter.rowCount).toBe(0) + const docAfter = await client.query<{ journal_entry_id: string | null }>( + `SELECT journal_entry_id FROM public.document_attachments WHERE id = $1`, + [docId], + ) + expect(docAfter.rows[0]!.journal_entry_id).toBeNull() + }) + }) + it('blocks reversed → posted UPDATE without the bypass flag', async () => { const { userId, companyId, fiscalPeriodId } = await seedCompany() const entryId = await insertPostedEntryWithLines({ diff --git a/lib/bookkeeping/__tests__/vat-entries.test.ts b/lib/bookkeeping/__tests__/vat-entries.test.ts new file mode 100644 index 00000000..43562424 --- /dev/null +++ b/lib/bookkeeping/__tests__/vat-entries.test.ts @@ -0,0 +1,266 @@ +import { describe, it, expect } from 'vitest' +import { + getVatRate, + generateSalesVatLines, + generateReverseChargeLines, + generateInputVatLine, + extractNetAmount, + extractVatAmount, +} from '../vat-entries' + +describe('getVatRate', () => { + it('returns 0.25 for standard_25', () => { + expect(getVatRate('standard_25')).toBe(0.25) + }) + + it('returns 0.12 for reduced_12', () => { + expect(getVatRate('reduced_12')).toBe(0.12) + }) + + it('returns 0.06 for reduced_6', () => { + expect(getVatRate('reduced_6')).toBe(0.06) + }) + + it('returns 0 for reverse_charge', () => { + expect(getVatRate('reverse_charge')).toBe(0) + }) + + it('returns 0 for export', () => { + expect(getVatRate('export')).toBe(0) + }) + + it('returns 0 for exempt', () => { + expect(getVatRate('exempt')).toBe(0) + }) +}) + +describe('generateSalesVatLines', () => { + it('credits 2611 (Utgående moms 25%) at standard rate', () => { + const lines = generateSalesVatLines({ + vatTreatment: 'standard_25', + baseAmount: 1000, + direction: 'sales', + }) + expect(lines).toHaveLength(1) + expect(lines[0].account_number).toBe('2611') + expect(lines[0].debit_amount).toBe(0) + expect(lines[0].credit_amount).toBe(250) + }) + + it('credits 2621 (Utgående moms 12%) at reduced rate', () => { + const lines = generateSalesVatLines({ + vatTreatment: 'reduced_12', + baseAmount: 1000, + direction: 'sales', + }) + expect(lines).toHaveLength(1) + expect(lines[0].account_number).toBe('2621') + expect(lines[0].credit_amount).toBe(120) + }) + + it('credits 2631 (Utgående moms 6%) at reduced rate', () => { + const lines = generateSalesVatLines({ + vatTreatment: 'reduced_6', + baseAmount: 1000, + direction: 'sales', + }) + expect(lines).toHaveLength(1) + expect(lines[0].account_number).toBe('2631') + expect(lines[0].credit_amount).toBe(60) + }) + + it('returns empty array for reverse_charge (no domestic VAT line)', () => { + expect( + generateSalesVatLines({ + vatTreatment: 'reverse_charge', + baseAmount: 1000, + direction: 'sales', + }) + ).toEqual([]) + }) + + it('returns empty array for export', () => { + expect( + generateSalesVatLines({ + vatTreatment: 'export', + baseAmount: 1000, + direction: 'sales', + }) + ).toEqual([]) + }) + + it('returns empty array for exempt', () => { + expect( + generateSalesVatLines({ + vatTreatment: 'exempt', + baseAmount: 1000, + direction: 'sales', + }) + ).toEqual([]) + }) + + it('rounds VAT to 2 decimals (333.33 * 0.25 = 83.3325 → 83.33)', () => { + const lines = generateSalesVatLines({ + vatTreatment: 'standard_25', + baseAmount: 333.33, + direction: 'sales', + }) + expect(lines[0].credit_amount).toBe(83.33) + }) +}) + +describe('generateReverseChargeLines — EU/non-EU (isDomestic=false)', () => { + it('debits 2645 and credits 2614 at 25%', () => { + const lines = generateReverseChargeLines(1000, 0.25, false) + expect(lines).toHaveLength(2) + expect(lines[0].account_number).toBe('2645') + expect(lines[0].debit_amount).toBe(250) + expect(lines[0].credit_amount).toBe(0) + expect(lines[1].account_number).toBe('2614') + expect(lines[1].debit_amount).toBe(0) + expect(lines[1].credit_amount).toBe(250) + }) + + it('debits 2645 and credits 2624 at 12%', () => { + const lines = generateReverseChargeLines(1000, 0.12, false) + expect(lines[0].account_number).toBe('2645') + expect(lines[0].debit_amount).toBe(120) + expect(lines[1].account_number).toBe('2624') + expect(lines[1].credit_amount).toBe(120) + }) + + it('debits 2645 and credits 2634 at 6%', () => { + const lines = generateReverseChargeLines(1000, 0.06, false) + expect(lines[0].account_number).toBe('2645') + expect(lines[0].debit_amount).toBe(60) + expect(lines[1].account_number).toBe('2634') + expect(lines[1].credit_amount).toBe(60) + }) +}) + +describe('generateReverseChargeLines — domestic (isDomestic=true, ML 16 kap)', () => { + it('debits 2647 (not 2645) and credits 2614 at 25%', () => { + const lines = generateReverseChargeLines(1000, 0.25, true) + expect(lines).toHaveLength(2) + expect(lines[0].account_number).toBe('2647') + expect(lines[0].debit_amount).toBe(250) + expect(lines[1].account_number).toBe('2614') + expect(lines[1].credit_amount).toBe(250) + }) + + it('debits 2647 and credits 2624 at 12%', () => { + const lines = generateReverseChargeLines(1000, 0.12, true) + expect(lines[0].account_number).toBe('2647') + expect(lines[0].debit_amount).toBe(120) + expect(lines[1].account_number).toBe('2624') + expect(lines[1].credit_amount).toBe(120) + }) + + it('debits 2647 and credits 2634 at 6%', () => { + const lines = generateReverseChargeLines(1000, 0.06, true) + expect(lines[0].account_number).toBe('2647') + expect(lines[0].debit_amount).toBe(60) + expect(lines[1].account_number).toBe('2634') + expect(lines[1].credit_amount).toBe(60) + }) +}) + +describe('generateReverseChargeLines — defaults & invariants', () => { + it('defaults to vatRate=0.25 and isDomestic=false when omitted', () => { + const lines = generateReverseChargeLines(1000) + expect(lines[0].account_number).toBe('2645') + expect(lines[1].account_number).toBe('2614') + expect(lines[0].debit_amount).toBe(250) + expect(lines[1].credit_amount).toBe(250) + }) + + it('keeps debit-credit pair balanced for every rate × isDomestic combination', () => { + for (const rate of [0.25, 0.12, 0.06]) { + for (const isDomestic of [true, false]) { + const lines = generateReverseChargeLines(1000, rate, isDomestic) + expect(lines[0].debit_amount).toBe(lines[1].credit_amount) + expect(lines[0].credit_amount).toBe(0) + expect(lines[1].debit_amount).toBe(0) + } + } + }) +}) + +describe('generateInputVatLine', () => { + it('debits 2641 with VAT extracted from gross at 25% (1250 → 250)', () => { + const line = generateInputVatLine(1250, 0.25) + expect(line).not.toBeNull() + expect(line!.account_number).toBe('2641') + expect(line!.debit_amount).toBe(250) + expect(line!.credit_amount).toBe(0) + }) + + it('debits 2641 at 12% (1120 → 120)', () => { + const line = generateInputVatLine(1120, 0.12) + expect(line!.account_number).toBe('2641') + expect(line!.debit_amount).toBe(120) + }) + + it('debits 2641 at 6% (1060 → 60)', () => { + const line = generateInputVatLine(1060, 0.06) + expect(line!.account_number).toBe('2641') + expect(line!.debit_amount).toBe(60) + }) + + it('returns null at zero rate (export/exempt/reverse_charge purchases)', () => { + expect(generateInputVatLine(1000, 0)).toBeNull() + }) + + it('defaults to vatRate=0.25 when omitted', () => { + const line = generateInputVatLine(1250) + expect(line!.debit_amount).toBe(250) + }) +}) + +describe('extractNetAmount', () => { + it('extracts 1000 net from 1250 gross at 25%', () => { + expect(extractNetAmount(1250, 0.25)).toBe(1000) + }) + + it('extracts 1000 net from 1120 gross at 12%', () => { + expect(extractNetAmount(1120, 0.12)).toBe(1000) + }) + + it('extracts 1000 net from 1060 gross at 6%', () => { + expect(extractNetAmount(1060, 0.06)).toBe(1000) + }) + + it('returns total unchanged at zero rate', () => { + expect(extractNetAmount(1000, 0)).toBe(1000) + }) +}) + +describe('extractVatAmount', () => { + it('extracts 250 VAT from 1250 gross at 25%', () => { + expect(extractVatAmount(1250, 0.25)).toBe(250) + }) + + it('extracts 120 VAT from 1120 gross at 12%', () => { + expect(extractVatAmount(1120, 0.12)).toBe(120) + }) + + it('extracts 60 VAT from 1060 gross at 6%', () => { + expect(extractVatAmount(1060, 0.06)).toBe(60) + }) + + it('returns 0 at zero rate', () => { + expect(extractVatAmount(1000, 0)).toBe(0) + }) +}) + +describe('extractNetAmount + extractVatAmount round-trip', () => { + it.each([ + [1250, 0.25], + [1120, 0.12], + [1060, 0.06], + ])('reconstructs total %s from net + vat at rate %s', (total, rate) => { + const net = extractNetAmount(total, rate) + const vat = extractVatAmount(total, rate) + expect(net + vat).toBe(total) + }) +}) diff --git a/lib/core/documents/__tests__/document-immutability.pg.test.ts b/lib/core/documents/__tests__/document-immutability.pg.test.ts new file mode 100644 index 00000000..280294f6 --- /dev/null +++ b/lib/core/documents/__tests__/document-immutability.pg.test.ts @@ -0,0 +1,233 @@ +import { randomUUID } from 'node:crypto' +import { describe, expect, it } from 'vitest' +import { getPool } from '@/tests/pg/setup' +import { + insertBalancedLines, + insertDraftJournalEntry, + seedCompany, +} from '@/tests/pg/fixtures' + +// BFL retention is enforced by two independent triggers on document_attachments: +// * enforce_document_journal_entry_immutability (20260506130000) — blocks +// any change to journal_entry_id once it has been set, regardless of the +// linked entry's status. Honors gnubok.allow_delete (20260506140000). +// * enforce_document_metadata_immutability (extended in 20260506120000) — +// blocks metadata changes and journal_entry_line_id changes when the +// linked entry is posted/reversed. Also honors gnubok.allow_delete. +// +// Both wordings — "BFL 5 kap" (entry trigger) and "BFL 7 kap" (metadata +// trigger) — are accepted; which one fires first depends on what column the +// UPDATE touches. +const BFL_RETENTION_ERROR = /BFL [57] kap/i + +async function insertDocument(params: { + userId: string + companyId: string + journalEntryId: string | null + journalEntryLineId?: string | null +}): Promise { + const id = randomUUID() + await getPool().query( + `INSERT INTO public.document_attachments + (id, user_id, company_id, storage_path, file_name, file_size_bytes, + mime_type, sha256_hash, journal_entry_id, journal_entry_line_id) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10)`, + [ + id, + params.userId, + params.companyId, + `documents/${params.userId}/${id}.pdf`, + 'underlag.pdf', + 1024, + 'application/pdf', + 'a'.repeat(64), + params.journalEntryId, + params.journalEntryLineId ?? null, + ], + ) + return id +} + +// Insert a draft, balance it, and walk through the legal state-machine +// transitions to land on the requested status. enforce_journal_entry_immutability +// only allows draft→posted and posted→reversed, so the path matters. +async function insertEntryAtStatus(params: { + userId: string + companyId: string + fiscalPeriodId: string + voucherNumber: number + status?: 'posted' | 'reversed' +}): Promise { + const entryId = await insertDraftJournalEntry({ + userId: params.userId, + companyId: params.companyId, + fiscalPeriodId: params.fiscalPeriodId, + voucherNumber: params.voucherNumber, + }) + await insertBalancedLines(entryId) + await getPool().query( + `UPDATE public.journal_entries SET status = 'posted' WHERE id = $1`, + [entryId], + ) + if (params.status === 'reversed') { + await getPool().query( + `UPDATE public.journal_entries SET status = 'reversed' WHERE id = $1`, + [entryId], + ) + } + return entryId +} + +describe('document-immutability.pg — BFL retention bypass guards', () => { + it('rejects unlinking (journal_entry_id → NULL) on a doc linked to a posted entry', async () => { + const { userId, companyId, fiscalPeriodId } = await seedCompany() + const entryId = await insertEntryAtStatus({ + userId, companyId, fiscalPeriodId, voucherNumber: 1, + }) + const docId = await insertDocument({ userId, companyId, journalEntryId: entryId }) + + await expect( + getPool().query( + `UPDATE public.document_attachments SET journal_entry_id = NULL WHERE id = $1`, + [docId], + ), + ).rejects.toThrow(BFL_RETENTION_ERROR) + }) + + it('rejects unlinking on a doc linked to a reversed entry', async () => { + const { userId, companyId, fiscalPeriodId } = await seedCompany() + const entryId = await insertEntryAtStatus({ + userId, companyId, fiscalPeriodId, voucherNumber: 1, status: 'reversed', + }) + const docId = await insertDocument({ userId, companyId, journalEntryId: entryId }) + + await expect( + getPool().query( + `UPDATE public.document_attachments SET journal_entry_id = NULL WHERE id = $1`, + [docId], + ), + ).rejects.toThrow(BFL_RETENTION_ERROR) + }) + + it('rejects unlinking on a doc linked to a draft entry — link is durable from first set', async () => { + // The entry-level trigger does not consult journal_entries.status; once + // journal_entry_id is set on a document, it cannot be cleared. This is + // stricter than the original branch design and matches main's intent + // that the verifikation→underlag link be durable from first set. + const { userId, companyId, fiscalPeriodId } = await seedCompany() + const draftId = await insertDraftJournalEntry({ + userId, companyId, fiscalPeriodId, voucherNumber: 0, + }) + const docId = await insertDocument({ userId, companyId, journalEntryId: draftId }) + + await expect( + getPool().query( + `UPDATE public.document_attachments SET journal_entry_id = NULL WHERE id = $1`, + [docId], + ), + ).rejects.toThrow(BFL_RETENTION_ERROR) + }) + + it('rejects re-pointing journal_entry_id to a different posted entry', async () => { + const { userId, companyId, fiscalPeriodId } = await seedCompany() + const entryA = await insertEntryAtStatus({ + userId, companyId, fiscalPeriodId, voucherNumber: 1, + }) + const entryB = await insertEntryAtStatus({ + userId, companyId, fiscalPeriodId, voucherNumber: 2, + }) + const docId = await insertDocument({ userId, companyId, journalEntryId: entryA }) + + await expect( + getPool().query( + `UPDATE public.document_attachments SET journal_entry_id = $1 WHERE id = $2`, + [entryB, docId], + ), + ).rejects.toThrow(BFL_RETENTION_ERROR) + }) + + it('allows first-time linking (NULL → UUID) — legitimate linkToJournalEntry path', async () => { + const { userId, companyId, fiscalPeriodId } = await seedCompany() + const entryId = await insertEntryAtStatus({ + userId, companyId, fiscalPeriodId, voucherNumber: 1, + }) + const docId = await insertDocument({ userId, companyId, journalEntryId: null }) + + await getPool().query( + `UPDATE public.document_attachments SET journal_entry_id = $1 WHERE id = $2`, + [entryId, docId], + ) + const after = await getPool().query<{ journal_entry_id: string | null }>( + `SELECT journal_entry_id FROM public.document_attachments WHERE id = $1`, + [docId], + ) + expect(after.rows[0]!.journal_entry_id).toBe(entryId) + }) + + it('rejects unlinking journal_entry_line_id on a doc linked to a posted entry', async () => { + const { userId, companyId, fiscalPeriodId } = await seedCompany() + const entryId = await insertEntryAtStatus({ + userId, companyId, fiscalPeriodId, voucherNumber: 1, + }) + const lineRow = await getPool().query<{ id: string }>( + `SELECT id FROM public.journal_entry_lines WHERE journal_entry_id = $1 LIMIT 1`, + [entryId], + ) + const lineId = lineRow.rows[0]!.id + const docId = await insertDocument({ + userId, companyId, journalEntryId: entryId, journalEntryLineId: lineId, + }) + + await expect( + getPool().query( + `UPDATE public.document_attachments SET journal_entry_line_id = NULL WHERE id = $1`, + [docId], + ), + ).rejects.toThrow(BFL_RETENTION_ERROR) + }) + + it('end-to-end: unlink-then-delete attack is blocked at the unlink step', async () => { + const { userId, companyId, fiscalPeriodId } = await seedCompany() + const entryId = await insertEntryAtStatus({ + userId, companyId, fiscalPeriodId, voucherNumber: 1, + }) + const docId = await insertDocument({ userId, companyId, journalEntryId: entryId }) + + await expect( + getPool().query( + `UPDATE public.document_attachments SET journal_entry_id = NULL WHERE id = $1`, + [docId], + ), + ).rejects.toThrow(BFL_RETENTION_ERROR) + + await expect( + getPool().query(`DELETE FROM public.document_attachments WHERE id = $1`, [docId]), + ).rejects.toThrow(/Bokföringslagen/i) + }) + + it('respects gnubok.allow_delete bypass — delete_last_voucher RPC keeps working', async () => { + const { userId, companyId, fiscalPeriodId } = await seedCompany() + const entryId = await insertEntryAtStatus({ + userId, companyId, fiscalPeriodId, voucherNumber: 1, + }) + const docId = await insertDocument({ userId, companyId, journalEntryId: entryId }) + + const client = await getPool().connect() + try { + await client.query('BEGIN') + await client.query(`SELECT set_config('gnubok.allow_delete', 'true', true)`) + await client.query( + `UPDATE public.document_attachments SET journal_entry_id = NULL WHERE id = $1`, + [docId], + ) + const after = await client.query<{ journal_entry_id: string | null }>( + `SELECT journal_entry_id FROM public.document_attachments WHERE id = $1`, + [docId], + ) + expect(after.rows[0]!.journal_entry_id).toBeNull() + await client.query('ROLLBACK') + } finally { + client.release() + } + }) +}) diff --git a/lib/errors/structured-errors.ts b/lib/errors/structured-errors.ts index 41590933..0c1662ec 100644 --- a/lib/errors/structured-errors.ts +++ b/lib/errors/structured-errors.ts @@ -433,6 +433,12 @@ const INVOICE: Record = { message_sv: 'E-postleverantören kunde inte skicka meddelandet.', message_en: 'The email provider could not deliver the message.', }, + INVOICE_SEND_PDF_RENDER_FAILED: { + httpStatus: 500, + message_sv: + 'Fakturans PDF kunde inte skapas. Kontrollera fakturarader och kunduppgifter och försök igen.', + message_en: 'Failed to render invoice PDF before send; no invoice number was consumed.', + }, INVOICE_SEND_PARTIAL: { httpStatus: 200, message_sv: @@ -469,6 +475,25 @@ const INVOICE: Record = { message_sv: 'Kunde inte bokföra betalningen.', message_en: 'Failed to create payment journal entry.', }, + INVOICE_DELETE_NOT_DRAFT: { + httpStatus: 400, + message_sv: 'Endast utkast kan tas bort. Bokförda fakturor måste krediteras istället.', + message_en: 'Only draft invoices can be deleted; non-drafts must be credited.', + remediation: { + description: 'Issue a credit note instead of deleting a posted invoice.', + }, + }, + INVOICE_DELETE_NUMBERED: { + httpStatus: 400, + message_sv: + 'Det här utkastet har redan tilldelats ett löpnummer och kan inte tas bort. Försök skicka det igen — om sändningen lyckas behövs inget annat steg.', + message_en: + 'Draft already has an invoice number assigned; refusing to delete to preserve the number sequence. Retry the send — assignment is idempotent.', + remediation: { + description: + 'Retry sending the invoice; ensureInvoiceNumber is idempotent so no new number will be consumed. If sending is no longer desired, contact support to clean up the orphan number.', + }, + }, } const SUPPLIER_INVOICE: Record = { diff --git a/supabase/migrations/20260506140000_document_journal_entry_immutability_bypass.sql b/supabase/migrations/20260506140000_document_journal_entry_immutability_bypass.sql new file mode 100644 index 00000000..35c1f80b --- /dev/null +++ b/supabase/migrations/20260506140000_document_journal_entry_immutability_bypass.sql @@ -0,0 +1,41 @@ +-- enforce_document_journal_entry_immutability (added in 20260506130000) +-- blocks any change to a non-null journal_entry_id once set. That is +-- correct for end-user paths but breaks delete_last_voucher: the RPC +-- legitimately needs to clear journal_entry_id before deleting a posted +-- voucher in a series (BFNAR 2013:2 permits removing the last voucher). +-- +-- Bring this trigger in line with enforce_journal_entry_immutability, +-- which already honors a transaction-local gnubok.allow_delete=true bypass. +-- Only delete_last_voucher sets that flag, and only after enforcing all +-- legal constraints (last-in-series, no references, period not locked, +-- owner/admin). Outside the RPC the flag is unset, so end-user UPDATE +-- paths remain blocked. + +CREATE OR REPLACE FUNCTION public.enforce_document_journal_entry_immutability() +RETURNS TRIGGER +LANGUAGE plpgsql +AS $$ +BEGIN + IF current_setting('gnubok.allow_delete', true) = 'true' THEN + RETURN NEW; + END IF; + + IF NEW.journal_entry_id IS NOT DISTINCT FROM OLD.journal_entry_id THEN + RETURN NEW; + END IF; + + IF OLD.journal_entry_id IS NULL THEN + RETURN NEW; + END IF; + + IF NEW.journal_entry_id IS NULL OR NEW.journal_entry_id <> OLD.journal_entry_id THEN + RAISE EXCEPTION + 'BFL_DOCUMENT_IMMUTABILITY: cannot clear or change journal_entry_id on document % once set (BFL 5 kap 6 §). Reverse the journal entry first.', + OLD.id; + END IF; + + RETURN NEW; +END; +$$; + +NOTIFY pgrst, 'reload schema'; diff --git a/supabase/migrations/20260506150000_protect_document_journal_link.sql b/supabase/migrations/20260506150000_protect_document_journal_link.sql new file mode 100644 index 00000000..8a4d88e4 --- /dev/null +++ b/supabase/migrations/20260506150000_protect_document_journal_link.sql @@ -0,0 +1,64 @@ +-- Closes BFL 7 kap 2§ retention bypass on document_attachments. +-- +-- Problem: enforce_document_metadata_immutability protected file identity +-- columns but not journal_entry_id / journal_entry_line_id. block_document_deletion +-- only fires when OLD.journal_entry_id IS NOT NULL. An attacker could therefore +-- run UPDATE document_attachments SET journal_entry_id = NULL WHERE id = X, +-- followed by DELETE FROM document_attachments WHERE id = X, severing +-- räkenskapsinformation from the audit trail in violation of BFL 7 kap 2§. +-- +-- Fix: extend the existing immutability trigger to also reject changes to +-- journal_entry_id and journal_entry_line_id when the document is currently +-- linked to a posted or reversed journal entry. Honour the existing +-- gnubok.allow_delete transaction-local bypass that delete_last_voucher uses +-- when intentionally tearing down a voucher. + +CREATE OR REPLACE FUNCTION public.enforce_document_metadata_immutability() + RETURNS trigger + LANGUAGE plpgsql + SECURITY DEFINER + SET search_path TO 'public' +AS $function$ +DECLARE + v_entry_status text; +BEGIN + IF current_setting('gnubok.allow_delete', true) = 'true' THEN + RETURN NEW; + END IF; + + IF OLD.journal_entry_id IS NULL THEN + RETURN NEW; + END IF; + + SELECT status INTO v_entry_status + FROM public.journal_entries + WHERE id = OLD.journal_entry_id; + + IF v_entry_status IS NULL OR v_entry_status NOT IN ('posted', 'reversed') THEN + RETURN NEW; + END IF; + + IF NEW.file_name IS DISTINCT FROM OLD.file_name + OR NEW.storage_path IS DISTINCT FROM OLD.storage_path + OR NEW.file_size_bytes IS DISTINCT FROM OLD.file_size_bytes + OR NEW.mime_type IS DISTINCT FROM OLD.mime_type + OR NEW.sha256_hash IS DISTINCT FROM OLD.sha256_hash + OR NEW.upload_source IS DISTINCT FROM OLD.upload_source + OR NEW.digitization_date IS DISTINCT FROM OLD.digitization_date + OR NEW.uploaded_by IS DISTINCT FROM OLD.uploaded_by + OR NEW.version IS DISTINCT FROM OLD.version + OR NEW.original_id IS DISTINCT FROM OLD.original_id + OR NEW.is_current_version IS DISTINCT FROM OLD.is_current_version + OR NEW.journal_entry_id IS DISTINCT FROM OLD.journal_entry_id + OR NEW.journal_entry_line_id IS DISTINCT FROM OLD.journal_entry_line_id + THEN + INSERT INTO public.audit_log (user_id, company_id, action, table_name, record_id, description) + VALUES (OLD.user_id, OLD.company_id, 'SECURITY_EVENT', 'document_attachments', OLD.id, + 'Blocked metadata or link modification of document linked to ' || v_entry_status || ' entry ' || OLD.journal_entry_id); + + RAISE EXCEPTION 'Cannot modify metadata or journal entry link of document linked to a % journal entry (BFL 7 kap)', v_entry_status; + END IF; + + RETURN NEW; +END; +$function$;