diff --git a/app/api/bookkeeping/journal-entries/[id]/route.ts b/app/api/bookkeeping/journal-entries/[id]/route.ts index 4bdbea14..a716c1de 100644 --- a/app/api/bookkeeping/journal-entries/[id]/route.ts +++ b/app/api/bookkeeping/journal-entries/[id]/route.ts @@ -5,6 +5,9 @@ import { requireWritePermission } from '@/lib/auth/require-write' import { ensureInitialized } from '@/lib/init' import { eventBus } from '@/lib/events/bus' import { getErrorMessage } from '@/lib/errors/get-error-message' +import { createLogger } from '@/lib/logger' + +const logger = createLogger('journal-entries') ensureInitialized() @@ -59,6 +62,7 @@ export async function DELETE( }) if (error) { + logger.error('delete_last_voucher failed', { entryId: id, error }) return NextResponse.json( { error: getErrorMessage(error, { context: 'journal_entry', statusCode: 400 }) }, { status: 400 } diff --git a/lib/bookkeeping/__tests__/delete-last-voucher.pg.test.ts b/lib/bookkeeping/__tests__/delete-last-voucher.pg.test.ts new file mode 100644 index 00000000..4004c21c --- /dev/null +++ b/lib/bookkeeping/__tests__/delete-last-voucher.pg.test.ts @@ -0,0 +1,149 @@ +import { randomUUID } from 'node:crypto' +import { describe, expect, it } from 'vitest' +import { getPool, withUserContext } from '@/tests/pg/setup' +import { + insertBalancedLines, + insertDraftJournalEntry, + seedCompany, +} from '@/tests/pg/fixtures' + +// Set up a posted journal entry with balanced lines, going through draft so +// the line-immutability trigger is happy. Returns the entry id. +async function insertPostedEntryWithLines(params: { + userId: string + companyId: string + fiscalPeriodId: string + voucherNumber: number + reversesId?: string + sourceType?: string +}): Promise { + const id = randomUUID() + await getPool().query( + `INSERT INTO public.journal_entries + (id, user_id, company_id, fiscal_period_id, voucher_number, voucher_series, + entry_date, description, source_type, status, reverses_id) + VALUES ($1, $2, $3, $4, $5, 'A', '2026-06-01', 'Test entry', $6, 'draft', $7)`, + [ + id, + params.userId, + params.companyId, + params.fiscalPeriodId, + params.voucherNumber, + params.sourceType ?? 'manual', + params.reversesId ?? null, + ], + ) + await insertBalancedLines(id) + await getPool().query( + `UPDATE public.journal_entries SET status = 'posted' WHERE id = $1`, + [id], + ) + return id +} + +describe('delete_last_voucher.pg — RPC + immutability trigger interaction', () => { + it('deletes the last posted voucher in a series', async () => { + const { userId, companyId, fiscalPeriodId } = await seedCompany() + const entryId = await insertPostedEntryWithLines({ + userId, companyId, fiscalPeriodId, voucherNumber: 1, + }) + + await withUserContext(userId, async (client) => { + await client.query( + `SELECT public.delete_last_voucher($1::uuid, $2::uuid)`, + [companyId, entryId], + ) + // Verify inside the txn — withUserContext rolls back on exit, so an + // outer pool query would see the row again. + const after = await client.query( + `SELECT 1 FROM public.journal_entries WHERE id = $1`, + [entryId], + ) + expect(after.rowCount).toBe(0) + }) + }) + + it('flips original from reversed back to posted when its storno is deleted', async () => { + const { userId, companyId, fiscalPeriodId } = await seedCompany() + + const originalId = await insertPostedEntryWithLines({ + userId, companyId, fiscalPeriodId, voucherNumber: 1, + }) + + // Storno: insert with reverses_id already set so the immutability trigger + // never sees an UPDATE that adds it after the fact. + const stornoId = await insertPostedEntryWithLines({ + userId, companyId, fiscalPeriodId, voucherNumber: 2, + sourceType: 'storno', reversesId: originalId, + }) + + // Mark original as reversed — posted → reversed is allowed by the state + // machine as long as no other fields change. + await getPool().query( + `UPDATE public.journal_entries SET status = 'reversed', reversed_by_id = $1 WHERE id = $2`, + [stornoId, originalId], + ) + + await withUserContext(userId, async (client) => { + await client.query( + `SELECT public.delete_last_voucher($1::uuid, $2::uuid)`, + [companyId, stornoId], + ) + const restored = await client.query<{ status: string; reversed_by_id: string | null }>( + `SELECT status, reversed_by_id FROM public.journal_entries WHERE id = $1`, + [originalId], + ) + expect(restored.rows[0]!.status).toBe('posted') + expect(restored.rows[0]!.reversed_by_id).toBeNull() + }) + }) + + it('blocks direct DELETE on a posted entry without the bypass flag', async () => { + const { userId, companyId, fiscalPeriodId } = await seedCompany() + const entryId = await insertPostedEntryWithLines({ + userId, companyId, fiscalPeriodId, voucherNumber: 1, + }) + + await expect( + getPool().query(`DELETE FROM public.journal_entries WHERE id = $1`, [entryId]), + ).rejects.toThrow(/Cannot delete journal entries/i) + }) + + it('blocks UPDATE of arbitrary fields on a posted entry even when bypass flag is set', async () => { + const { userId, companyId, fiscalPeriodId } = await seedCompany() + const entryId = await insertPostedEntryWithLines({ + userId, companyId, fiscalPeriodId, voucherNumber: 1, + }) + + const client = await getPool().connect() + try { + await client.query(`SELECT set_config('gnubok.allow_delete', 'true', true)`) + await expect( + client.query( + `UPDATE public.journal_entries SET description = 'tampered' WHERE id = $1`, + [entryId], + ), + ).rejects.toThrow(/Cannot modify a posted journal entry/i) + } finally { + client.release() + } + }) + + it('blocks reversed → posted UPDATE without the bypass flag', async () => { + const { userId, companyId, fiscalPeriodId } = await seedCompany() + const entryId = await insertPostedEntryWithLines({ + userId, companyId, fiscalPeriodId, voucherNumber: 1, + }) + await getPool().query( + `UPDATE public.journal_entries SET status = 'reversed' WHERE id = $1`, + [entryId], + ) + + await expect( + getPool().query( + `UPDATE public.journal_entries SET status = 'posted' WHERE id = $1`, + [entryId], + ), + ).rejects.toThrow(/Cannot modify a reversed journal entry/i) + }) +}) diff --git a/supabase/migrations/20260428160000_fix_journal_entry_immutability_delete_bypass.sql b/supabase/migrations/20260428160000_fix_journal_entry_immutability_delete_bypass.sql new file mode 100644 index 00000000..6d72f436 --- /dev/null +++ b/supabase/migrations/20260428160000_fix_journal_entry_immutability_delete_bypass.sql @@ -0,0 +1,68 @@ +-- Fix: enforce_journal_entry_immutability blocks DELETE and the +-- reversed → posted "un-reversal" UPDATE that delete_last_voucher RPC needs +-- when removing a storno entry. The RPC sets gnubok.allow_delete, but the +-- trigger blocks both operations unconditionally. Result: the RPC fails, +-- even though BFNAR 2013:2 explicitly permits deletion of the last voucher +-- in a series. +-- +-- enforce_journal_entry_line_immutability already respects this flag; +-- this migration brings the entries trigger in line — but narrowly. +-- +-- Bypass scope: +-- * DELETE: allowed when gnubok.allow_delete='true'. delete_last_voucher +-- enforces all legal constraints (last-in-series, no references, period +-- not locked, owner/admin) before setting the flag. +-- * UPDATE: allowed only for the specific reversed → posted transition +-- used to "un-reverse" the original entry when its storno is being +-- deleted. All other UPDATE paths still go through the normal state +-- machine — defense-in-depth on posted-entry mutation is preserved. + +CREATE OR REPLACE FUNCTION public.enforce_journal_entry_immutability() +RETURNS trigger +LANGUAGE plpgsql +AS $function$ +BEGIN + IF TG_OP = 'DELETE' THEN + IF current_setting('gnubok.allow_delete', true) = 'true' THEN + RETURN OLD; + END IF; + RAISE EXCEPTION 'Cannot delete journal entries (id: %, status: %). Use cancelled status instead.', + OLD.id, OLD.status; + END IF; + + IF OLD.status = 'draft' AND NEW.status IN ('draft', 'posted', 'cancelled') THEN + RETURN NEW; + END IF; + + IF OLD.status = 'posted' AND NEW.status IN ('reversed', 'cancelled') THEN + IF NEW.status = 'reversed' THEN + IF NEW.description != OLD.description OR NEW.entry_date != OLD.entry_date + OR NEW.fiscal_period_id != OLD.fiscal_period_id + OR NEW.voucher_number != OLD.voucher_number + OR NEW.commit_method IS DISTINCT FROM OLD.commit_method + OR NEW.rubric_version IS DISTINCT FROM OLD.rubric_version + OR NEW.source_voucher_series IS DISTINCT FROM OLD.source_voucher_series + OR NEW.source_voucher_number IS DISTINCT FROM OLD.source_voucher_number THEN + RAISE EXCEPTION 'Cannot modify fields of a posted entry during reversal (id: %)', OLD.id; + END IF; + END IF; + RETURN NEW; + END IF; + + -- Narrow un-reversal path: when delete_last_voucher removes a storno entry, + -- it flips the original from 'reversed' back to 'posted'. No other fields + -- may change, and the bypass flag must be set. + IF OLD.status = 'reversed' AND NEW.status = 'posted' + AND current_setting('gnubok.allow_delete', true) = 'true' THEN + IF NEW.description != OLD.description OR NEW.entry_date != OLD.entry_date + OR NEW.fiscal_period_id != OLD.fiscal_period_id + OR NEW.voucher_number != OLD.voucher_number THEN + RAISE EXCEPTION 'Cannot modify fields during un-reversal (id: %)', OLD.id; + END IF; + RETURN NEW; + END IF; + + RAISE EXCEPTION 'Cannot modify a % journal entry (id: %). Committed entries are immutable per Bokforingslagen.', + OLD.status, OLD.id; +END; +$function$;