diff --git a/DECISIONS.md b/DECISIONS.md index f0bad4ab..6df50740 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -1200,3 +1200,4 @@ One line per decision: `[YYYY-MM-DD] : `. Appended by agents and [2026-08-24] Bokslutsbilagor pärm (Reko bilagor, PR 4) is generated from the sign-off rows, the trial balance through balansdagen and the attachment rows, never by recomputing each account's live status: the bilaga documents what was attested (numbers as they stood at sign-off, who, when, note) plus the files with their SHA-256, which is what a kvalitetskontroll reads. Whole period only (a bilaga is per balansdag), PDF-only export, written into every period folder of the full archive as JSON + PDF; an archive run has no acting user, so the checklist's readiness-derived items are left as stored there. [2026-08-25] A period klarmarkerad as closed in a previous system (closed_externally) no longer trips the trial balance's "closed without closing_entry_id" guard for statutory pre-closing balances: its closing verifikat never existed in these books, so the booked balances are the pre-closing balances and there is nothing to strip. The guard stays for periods our own engine closed, where a missing link is a real inconsistency. Found by Väla Redovisning: Klarmarkera + Årsredovisning = 500. [2026-08-24] fiscal_periods.previous_period_id is adjacency-only: findNextPeriod ignores a chained period that does not start the day after the current one, and SIE import only wires predecessor/successor links between date-adjacent periods (before: nearest period across any gap). A non-adjacent link is what sent a company's opening balances two years forward (feedback seq 249297); 40 such links exist on prod across 39 companies and are neutralized by the read-side guard, not repaired in this change. A gap in the chain means a missing räkenskapsår (BFL 3 kap), which reports should show as missing rather than bridge silently. +[2026-08-24] Pending-operation authorization refusals (401/403 from an executor) release the claim back to 'pending' instead of consuming the op as 'rejected': the refusal happens before any side-effect and reflects the credential, not the booking, so the same op must survive for an authorized approver (/pending UI or a scoped key). Every CommitResult now carries operation_status so agents stop inferring "consumed" from status 'failed'. Deterministic content errors (400) still consume the op: re-staging is the only fix for those. diff --git a/extensions/general/mcp-server/__tests__/payload-size.bench.test.ts b/extensions/general/mcp-server/__tests__/payload-size.bench.test.ts index 88d00786..cfbfeb55 100644 --- a/extensions/general/mcp-server/__tests__/payload-size.bench.test.ts +++ b/extensions/general/mcp-server/__tests__/payload-size.bench.test.ts @@ -198,9 +198,14 @@ describe('tools/list payload size guard', () => { // so strict clients stop failing successful unmatched uploads (seq // 261972). Prose trimmed to the floor first; headroom before the // change was ~19 tokens, so even the trimmed contract crossed. + // * 59.95K to 60K with operation_status on gnubok_approve_pending_operation + // (feedback seq 261545): a failed approve used to consume the op + // silently, and agents inferred "consumed" from status 'failed' both + // ways. The enum is the contract; the description is one clause; + // headroom before the change was ~15 tokens, so even that crossed. // Long-term answer to growth is leaning harder on gnubok_search_tools: if this // fires again, prefer trimming descriptions or making a tool opt-in via search // before bumping further. - expect(approxTokens).toBeLessThan(59_950) + expect(approxTokens).toBeLessThan(60_000) }) }) diff --git a/extensions/general/mcp-server/server.ts b/extensions/general/mcp-server/server.ts index 76a38346..9a92c21c 100644 --- a/extensions/general/mcp-server/server.ts +++ b/extensions/general/mcp-server/server.ts @@ -16843,6 +16843,7 @@ export const tools: McpTool[] = [ error: { type: 'string' }, error_code: { type: 'string' }, auto_rejected: { type: 'boolean' }, + operation_status: { type: 'string', enum: ['pending', 'committed', 'rejected', 'failed_partial'], description: 'pending = not consumed, re-approvable' }, }, required: ['status', 'operation_id'], }, @@ -16956,6 +16957,7 @@ export const tools: McpTool[] = [ ...(result.error ? { error: result.error } : {}), ...(result.code ? { error_code: result.code } : {}), ...(result.auto_rejected ? { auto_rejected: true } : {}), + ...(result.operation_status ? { operation_status: result.operation_status } : {}), } }, }, diff --git a/lib/pending-operations/__tests__/commit-authorization-recoverable.test.ts b/lib/pending-operations/__tests__/commit-authorization-recoverable.test.ts new file mode 100644 index 00000000..a974a172 --- /dev/null +++ b/lib/pending-operations/__tests__/commit-authorization-recoverable.test.ts @@ -0,0 +1,121 @@ +/** + * Authorization refusals must not consume a pending operation. + * + * Feedback seq 261545: an API-key approve of a bulk_book_transactions op hit + * BULK_BOOK_UNAUTHORIZED (the RPC saw auth.uid() = NULL on the service + * client), and the dispatcher landed the op as 'rejected'. It vanished from + * the /pending queue with nothing booked, and the user believed it had been + * approved. A 401/403 happens before any side-effect and says nothing about + * the op's content, so the claim is released back to 'pending' and the + * result says so explicitly (operation_status) instead of leaving agents to + * infer consumption from status 'failed'. + */ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { eventBus } from '@/lib/events/bus' +import { createQueuedMockSupabase } from '@/tests/helpers' +import type { PendingOperation } from '@/types' + +import { commitPendingOperation } from '../commit' + +function makeBulkBookOp(): PendingOperation { + return { + id: 'op-bulk-1', + user_id: 'user-1', + company_id: 'company-1', + operation_type: 'bulk_book_transactions', + status: 'pending', + title: 'Samlingsverifikation: 3 transaktioner 2026-07-22', + params: { + tx_ids: ['tx-1', 'tx-2', 'tx-3'], + existing_journal_entry_id: null, + new_entry: { description: 'Dagskassa', lines: [] }, + }, + preview_data: {}, + result_data: null, + actor_type: 'api_key', + actor_id: 'key-1', + actor_label: 'deepCFO', + risk_level: 'medium', + created_at: '2026-08-24T00:00:00Z', + resolved_at: null, + updated_at: '2026-08-24T00:00:00Z', + } as PendingOperation +} + +describe('commitPendingOperation: authorization refusal is recoverable', () => { + beforeEach(() => { + vi.clearAllMocks() + eventBus.clear() + }) + + it('releases the claim back to pending on BULK_BOOK_UNAUTHORIZED and reports operation_status', async () => { + const { supabase, enqueueMany, findCalls } = createQueuedMockSupabase() + enqueueMany([ + { data: { id: 'op-bulk-1' }, error: null }, // atomic claim pending -> committing + { data: { ok: false, code: 'BULK_BOOK_UNAUTHORIZED' }, error: null }, // RPC refusal + { data: null, error: null }, // release claim back to pending + ]) + + const result = await commitPendingOperation( + supabase as never, + 'user-1', + 'company-1', + makeBulkBookOp(), + ) + + expect(result.status).toBe('failed') + expect(result.http_status).toBe(403) + expect(result.code).toBe('BULK_BOOK_UNAUTHORIZED') + expect(result.operation_status).toBe('pending') + + const updates = findCalls('pending_operations', 'update') + expect(updates).toContainEqual([{ status: 'committing' }]) + expect(updates).toContainEqual([{ status: 'pending' }]) + expect(updates.some((args) => (args[0] as { status?: string }).status === 'rejected')).toBe(false) + }) + + it('passes the approving user as p_user_id so the service client is attributed', async () => { + const { supabase, enqueueMany } = createQueuedMockSupabase() + enqueueMany([ + { data: { id: 'op-bulk-1' }, error: null }, + { data: { ok: true, journal_entry_id: 'je-1', mode: 'create_new', linked_tx_count: 3 }, error: null }, + { data: null, error: null }, // finalize committed + ]) + + const result = await commitPendingOperation( + supabase as never, + 'user-1', + 'company-1', + makeBulkBookOp(), + ) + + expect(result.status).toBe('committed') + expect(result.operation_status).toBe('committed') + expect(supabase.rpc).toHaveBeenCalledTimes(1) + expect(supabase.rpc).toHaveBeenCalledWith( + 'bulk_book_transactions', + expect.objectContaining({ p_user_id: 'user-1', p_company_id: 'company-1' }), + ) + }) + + it('still consumes the op as rejected on a genuine input error (400)', async () => { + const { supabase, enqueueMany, findCalls } = createQueuedMockSupabase() + enqueueMany([ + { data: { id: 'op-bulk-1' }, error: null }, + { data: { ok: false, code: 'BULK_BOOK_INVALID_PAYLOAD' }, error: null }, + { data: null, error: null }, // rejected update + ]) + + const result = await commitPendingOperation( + supabase as never, + 'user-1', + 'company-1', + makeBulkBookOp(), + ) + + expect(result.status).toBe('failed') + expect(result.operation_status).toBe('rejected') + const updates = findCalls('pending_operations', 'update') + expect(updates.some((args) => (args[0] as { status?: string }).status === 'rejected')).toBe(true) + }) +}) diff --git a/lib/pending-operations/commit.ts b/lib/pending-operations/commit.ts index 98d0d2bc..b5cc1ead 100644 --- a/lib/pending-operations/commit.ts +++ b/lib/pending-operations/commit.ts @@ -208,6 +208,12 @@ export interface CommitResult { // callers that do not recognize the code fall back to `error`. code?: string account_numbers?: string[] + // Where the pending_operations row landed, independent of `status`: + // 'pending' means the op was NOT consumed and can be approved again + // (recoverable refusal: capability, chart accounts, Skatteverket, or an + // authorization failure that happened before any side-effect). Agents + // used to infer "consumed" from status 'failed' and were wrong both ways. + operation_status?: 'pending' | 'committed' | 'rejected' | 'failed_partial' } export interface CommitOptions { @@ -5874,6 +5880,7 @@ async function commitMatchBatchAllocate( async function commitBulkBookTransactions( supabase: SupabaseClient, + userId: string, companyId: string, params: Record ): Promise { @@ -5910,6 +5917,10 @@ async function commitBulkBookTransactions( p_existing_journal_entry_id: existingJeId, p_new_entry: newEntry, p_company_id: companyId, + // This path runs on the cookieless service client where auth.uid() is + // NULL; the RPC honors p_user_id only for service_role callers + // (migration 20260824170000), so the approving human is the actor. + p_user_id: userId, }) if (error) { // Sanitised log (A.8.11, CC7.2): only error code + message. @@ -5921,9 +5932,15 @@ async function commitBulkBookTransactions( } const result = data as { ok: boolean; code?: string; details?: unknown; journal_entry_id?: string; mode?: string; linked_tx_count?: number; docs_linked?: number } if (!result || !result.ok) { + const code = result?.code + const entry = code ? getErrorEntry(code) : undefined return { - error: result?.code || 'bulk_book_transactions failed', - status: 400, + error: code || 'bulk_book_transactions failed', + // Registry httpStatus so the dispatcher can tell an authorization + // refusal (403: nothing posted, op must stay pending) from bad input + // (400) or a vanished/already-booked tx (404/409: auto-reject). + status: entry?.httpStatus ?? 400, + ...(code ? { errorCode: code } : {}), data: result?.details as Record | undefined, } } @@ -6257,6 +6274,7 @@ async function commitPendingOperationInner( error: CAPABILITY_BLOCKED_MESSAGE_SV, http_status: 403, code: 'capability_blocked', + operation_status: 'pending', } } @@ -6472,7 +6490,7 @@ async function commitPendingOperationInner( result = await commitMatchBatchAllocate(supabase, userId, companyId, pendingOp.params) break case 'bulk_book_transactions': - result = await commitBulkBookTransactions(supabase, companyId, pendingOp.params) + result = await commitBulkBookTransactions(supabase, userId, companyId, pendingOp.params) break case 'bulk_book_inbox_items': result = await commitBulkBookInboxItems(supabase, userId, companyId, pendingOp.params) @@ -6528,6 +6546,7 @@ async function commitPendingOperationInner( http_status: 500, code: 'partial_commit', data: { posted_ids: err.postedIds }, + operation_status: 'failed_partial', } } // Accounts-not-in-chart is RECOVERABLE: the booking itself is valid; the @@ -6546,6 +6565,7 @@ async function commitPendingOperationInner( http_status: 400, code: ACCOUNTS_NOT_IN_CHART, account_numbers: err.accountNumbers, + operation_status: 'pending', } } // Recoverable Skatteverket failure (extension disabled, no connection, @@ -6562,6 +6582,7 @@ async function commitPendingOperationInner( error: err.message, http_status: err.httpStatus, code: err.code, + operation_status: 'pending', } } const isBkErr = isBookkeepingError(err) @@ -6581,6 +6602,7 @@ async function commitPendingOperationInner( status: 'failed', error: message, http_status: isBkErr ? 400 : 500, + operation_status: 'rejected', } } @@ -6613,6 +6635,27 @@ async function commitPendingOperationInner( http_status: result.status ?? 500, code: 'partial_commit', data: { posted_ids: partialPostedIds }, + operation_status: 'failed_partial', + } + } + // Authorization refusals (401/403) happen BEFORE any side-effect and say + // nothing about the op's content: the credential, not the booking, was + // wrong. Release the claim back to 'pending' so the op survives for a + // caller that IS authorized (the /pending UI, or a key with the scope), + // instead of vanishing as 'rejected'. Feedback seq 261545: three + // samlingsverifikat were consumed this way and the user believed they + // had been approved. + if (result.status === 401 || result.status === 403) { + await supabase + .from('pending_operations') + .update({ status: 'pending' }) + .eq('id', pendingOp.id) + return { + status: 'failed', + error: result.error, + http_status: result.status, + ...(result.errorCode ? { code: result.errorCode } : {}), + operation_status: 'pending', } } const isAutoReject = result.status === 404 || result.status === 409 @@ -6662,6 +6705,7 @@ async function commitPendingOperationInner( http_status: result.status, ...(result.errorCode ? { code: result.errorCode } : {}), ...(failureDetails ? { data: failureDetails } : {}), + operation_status: 'rejected', } } return { @@ -6670,6 +6714,7 @@ async function commitPendingOperationInner( http_status: result.status ?? 500, ...(result.errorCode ? { code: result.errorCode } : {}), ...(failureDetails ? { data: failureDetails } : {}), + operation_status: 'rejected', } } @@ -6706,5 +6751,6 @@ async function commitPendingOperationInner( return { status: 'committed', data: result.data, + operation_status: 'committed', } } diff --git a/supabase/migrations/20260824170000_bulk_book_transactions_service_actor.sql b/supabase/migrations/20260824170000_bulk_book_transactions_service_actor.sql new file mode 100644 index 00000000..56b3a10d --- /dev/null +++ b/supabase/migrations/20260824170000_bulk_book_transactions_service_actor.sql @@ -0,0 +1,424 @@ +-- bulk_book_transactions: honor an explicit actor for service-role callers. +-- +-- The pending-operations commit path runs on the cookieless service client +-- (createServiceClientNoCookies), where auth.uid() is NULL, so EVERY +-- MCP-approved samlingsverifikation returned BULK_BOOK_UNAUTHORIZED, and the +-- dispatcher then consumed the staged op as 'rejected': the user saw it +-- vanish from /pending and assumed it had been booked (gnubok_feedback +-- 2026-08-24, seq 261545, three op ids). The web /pending path only works +-- because it carries a cookie session. +-- +-- Fix: add p_user_id, gated exactly like match_batch_allocate +-- (20260817150000): honored only when auth.role() = 'service_role'; every +-- other caller resolves from its own auth.uid(), so an authenticated +-- PostgREST caller cannot impersonate. v_caller also stamps the journal +-- entry, so service-path commits are attributed to the approving human. +-- +-- The body is byte-for-byte the 20260726100000 definition (the latest: +-- currency guard) plus the header parameter and the actor-resolution +-- block. The 4-arg signature is dropped to avoid PostgREST overload +-- ambiguity (the 5th arg has a DEFAULT, so 4-arg call sites still resolve); +-- grants are re-asserted because DROP discards them. +-- +-- pg-test: tests/pg/bulk-book-transactions-service-actor.pg.test.ts + +DROP FUNCTION IF EXISTS public.bulk_book_transactions(uuid[], uuid, jsonb, uuid); + +CREATE OR REPLACE FUNCTION public.bulk_book_transactions( + p_tx_ids uuid[], + p_existing_journal_entry_id uuid, + p_new_entry jsonb, + p_company_id uuid, + p_user_id uuid DEFAULT NULL +) +RETURNS jsonb +LANGUAGE plpgsql +SECURITY DEFINER +SET search_path TO 'public' +AS $$ +DECLARE + v_tx RECORD; + v_tx_date date; + v_total_amount numeric := 0; + v_total_amount_abs numeric; + v_direction text; + v_tx_count int := 0; + + -- Currency homogeneity (BFL 4 kap 6 §). A separate "seen" flag rather + -- than a NULL check on v_currency: the first row's currency can itself + -- be NULL, and that must still pin the batch to SEK for the rest. + v_currency text; + v_currency_seen boolean := false; + + v_voucher RECORD; + v_voucher_bank_net numeric := 0; + + v_fiscal_period_id uuid; + v_period_is_closed boolean; + v_period_locked_at timestamptz; + + v_journal_entry_id uuid; + v_voucher_series text := 'A'; + v_voucher_number int; + v_entry_description text; + + v_line jsonb; + v_line_account text; + v_line_debit numeric; + v_line_credit numeric; + v_line_currency text; + v_line_dims jsonb; + v_lines_total_debit numeric := 0; + v_lines_total_credit numeric := 0; + v_lines_bank_net numeric := 0; + v_sort_order int := 0; + + v_docs_linked int := 0; + v_target_je uuid; + + v_invalid_accounts text[]; + + v_now timestamptz := now(); + v_caller uuid; +BEGIN + -- Actor resolution. p_user_id is an assertion by the caller, so it is + -- honored only for the service role (the pending-operations commit path + -- runs on createServiceClientNoCookies, where auth.uid() is NULL). Any + -- other caller is pinned to its own auth.uid(): an authenticated + -- PostgREST caller cannot impersonate another member. Mirrors + -- match_batch_allocate (20260817150000) and undo_sie_import. + IF auth.role() = 'service_role' THEN + v_caller := COALESCE(p_user_id, auth.uid()); + ELSE + v_caller := auth.uid(); + END IF; + IF v_caller IS NULL THEN + RETURN jsonb_build_object('ok', false, 'code', 'BULK_BOOK_UNAUTHORIZED'); + END IF; + IF NOT EXISTS ( + SELECT 1 FROM public.company_members + WHERE user_id = v_caller AND company_id = p_company_id + ) THEN + RETURN jsonb_build_object('ok', false, 'code', 'BULK_BOOK_UNAUTHORIZED'); + END IF; + + IF p_tx_ids IS NULL OR array_length(p_tx_ids, 1) IS NULL THEN + RETURN jsonb_build_object('ok', false, 'code', 'BULK_BOOK_NO_TXS'); + END IF; + + IF (p_existing_journal_entry_id IS NULL AND p_new_entry IS NULL) + OR (p_existing_journal_entry_id IS NOT NULL AND p_new_entry IS NOT NULL) THEN + RETURN jsonb_build_object('ok', false, 'code', 'BULK_BOOK_INVALID_PAYLOAD'); + END IF; + + FOR v_tx IN + SELECT * FROM public.transactions + WHERE id = ANY(p_tx_ids) AND company_id = p_company_id + ORDER BY id + FOR UPDATE + LOOP + v_tx_count := v_tx_count + 1; + IF v_tx.journal_entry_id IS NOT NULL THEN + RETURN jsonb_build_object('ok', false, 'code', 'BULK_BOOK_TX_ALREADY_BOOKED', + 'details', jsonb_build_object('tx_id', v_tx.id)); + END IF; + IF EXISTS ( + SELECT 1 FROM public.transaction_voucher_links tvl + WHERE tvl.transaction_id = v_tx.id + ) THEN + RETURN jsonb_build_object('ok', false, 'code', 'BULK_BOOK_TX_ALREADY_BOOKED', + 'details', jsonb_build_object('tx_id', v_tx.id, 'via', 'transaction_voucher_links')); + END IF; + IF v_tx.amount = 0 THEN + RETURN jsonb_build_object('ok', false, 'code', 'BULK_BOOK_TX_ZERO_AMOUNT', + 'details', jsonb_build_object('tx_id', v_tx.id)); + END IF; + + IF v_tx_date IS NULL THEN + v_tx_date := v_tx.date; + ELSIF v_tx_date <> v_tx.date THEN + RETURN jsonb_build_object('ok', false, 'code', 'BULK_BOOK_DATE_MISMATCH', + 'details', jsonb_build_object('first_date', v_tx_date, 'other_date', v_tx.date)); + END IF; + + -- Mixed currencies cannot be added into v_total_amount below. + IF NOT v_currency_seen THEN + v_currency := COALESCE(v_tx.currency, 'SEK'); + v_currency_seen := true; + ELSIF v_currency <> COALESCE(v_tx.currency, 'SEK') THEN + RETURN jsonb_build_object('ok', false, 'code', 'BULK_BOOK_MIXED_CURRENCY', + 'details', jsonb_build_object( + 'currencies', jsonb_build_array(v_currency, COALESCE(v_tx.currency, 'SEK')))); + END IF; + + IF v_direction IS NULL THEN + v_direction := CASE WHEN v_tx.amount > 0 THEN 'income' ELSE 'expense' END; + ELSIF (v_direction = 'income' AND v_tx.amount < 0) + OR (v_direction = 'expense' AND v_tx.amount > 0) THEN + RETURN jsonb_build_object('ok', false, 'code', 'BULK_BOOK_DIRECTION_MISMATCH', + 'details', jsonb_build_object('expected', v_direction, 'tx_id', v_tx.id)); + END IF; + + v_total_amount := v_total_amount + v_tx.amount; + END LOOP; + + IF v_tx_count = 0 THEN + RETURN jsonb_build_object('ok', false, 'code', 'BULK_BOOK_TXS_NOT_FOUND'); + END IF; + + IF v_tx_count <> COALESCE(array_length(p_tx_ids, 1), 0) THEN + RETURN jsonb_build_object('ok', false, 'code', 'BULK_BOOK_TXS_NOT_FOUND', + 'details', jsonb_build_object('expected', array_length(p_tx_ids, 1), 'found', v_tx_count)); + END IF; + + -- A homogeneous foreign batch is refused too. The debit/credit columns + -- written below are ALWAYS kronor and this function has no exchange rate: + -- letting a EUR selection through would post its foreign magnitudes as SEK + -- and every downstream reader (balansräkning, moms, SIE) would state an + -- amount matching no affärshändelse. Foreign transactions are booked one at + -- a time through the FX-aware flows instead. + IF COALESCE(v_currency, 'SEK') <> 'SEK' THEN + RETURN jsonb_build_object('ok', false, 'code', 'BULK_BOOK_FOREIGN_CURRENCY', + 'details', jsonb_build_object('currency', v_currency)); + END IF; + + v_total_amount_abs := ABS(v_total_amount); + + IF p_existing_journal_entry_id IS NOT NULL THEN + SELECT * INTO v_voucher FROM public.journal_entries + WHERE id = p_existing_journal_entry_id AND company_id = p_company_id + FOR UPDATE; + + IF NOT FOUND THEN + RETURN jsonb_build_object('ok', false, 'code', 'BULK_BOOK_JE_NOT_FOUND', + 'details', jsonb_build_object('journal_entry_id', p_existing_journal_entry_id)); + END IF; + + IF v_voucher.status <> 'posted' THEN + RETURN jsonb_build_object('ok', false, 'code', 'BULK_BOOK_JE_NOT_POSTED', + 'details', jsonb_build_object('status', v_voucher.status)); + END IF; + + SELECT COALESCE(SUM(debit_amount - credit_amount), 0) INTO v_voucher_bank_net + FROM public.journal_entry_lines + WHERE journal_entry_id = p_existing_journal_entry_id + AND length(account_number) = 4 + AND account_number BETWEEN '1900' AND '1999'; + + IF ABS(v_voucher_bank_net - v_total_amount) > 0.005 THEN + RETURN jsonb_build_object('ok', false, 'code', 'BULK_BOOK_AMOUNT_MISMATCH', + 'details', jsonb_build_object( + 'tx_sum', v_total_amount, 'voucher_bank_net', v_voucher_bank_net)); + END IF; + + FOR v_tx IN + SELECT * FROM public.transactions + WHERE id = ANY(p_tx_ids) AND company_id = p_company_id + ORDER BY id + LOOP + INSERT INTO public.transaction_voucher_links + (user_id, company_id, transaction_id, journal_entry_id, allocated_amount, role) + VALUES + (v_caller, p_company_id, v_tx.id, p_existing_journal_entry_id, v_tx.amount, 'bank_line'); + END LOOP; + + IF v_tx_count = 1 THEN + UPDATE public.transactions + SET journal_entry_id = p_existing_journal_entry_id, + reconciliation_method = 'manual', + is_business = TRUE, + updated_at = v_now + WHERE id = p_tx_ids[1]; + ELSE + UPDATE public.transactions + SET is_business = TRUE, updated_at = v_now + WHERE id = ANY(p_tx_ids); + END IF; + + v_target_je := p_existing_journal_entry_id; + v_voucher_series := v_voucher.voucher_series; + v_voucher_number := v_voucher.voucher_number; + + ELSE + v_entry_description := p_new_entry->>'description'; + IF v_entry_description IS NULL OR LENGTH(TRIM(v_entry_description)) = 0 THEN + RETURN jsonb_build_object('ok', false, 'code', 'BULK_BOOK_MISSING_DESCRIPTION'); + END IF; + + IF jsonb_typeof(p_new_entry->'lines') IS DISTINCT FROM 'array' + OR jsonb_array_length(p_new_entry->'lines') < 2 THEN + RETURN jsonb_build_object('ok', false, 'code', 'BULK_BOOK_NO_LINES'); + END IF; + + WITH submitted AS ( + SELECT DISTINCT value->>'account_number' AS acct + FROM jsonb_array_elements(p_new_entry->'lines') + ) + SELECT array_agg(s.acct ORDER BY s.acct) INTO v_invalid_accounts + FROM submitted s + WHERE NOT EXISTS ( + SELECT 1 FROM public.chart_of_accounts coa + WHERE coa.account_number = s.acct + AND coa.company_id = p_company_id + AND coa.is_active = true + ); + IF v_invalid_accounts IS NOT NULL AND array_length(v_invalid_accounts, 1) > 0 THEN + RETURN jsonb_build_object('ok', false, 'code', 'BULK_BOOK_INVALID_ACCOUNT', + 'details', jsonb_build_object('invalid_accounts', v_invalid_accounts)); + END IF; + + FOR v_line IN SELECT * FROM jsonb_array_elements(p_new_entry->'lines') + LOOP + v_line_account := v_line->>'account_number'; + v_line_debit := COALESCE((v_line->>'debit_amount')::numeric, 0); + v_line_credit := COALESCE((v_line->>'credit_amount')::numeric, 0); + IF v_line_debit < 0 OR v_line_credit < 0 THEN + RETURN jsonb_build_object('ok', false, 'code', 'BULK_BOOK_NEGATIVE_LINE', + 'details', jsonb_build_object('account', v_line_account)); + END IF; + IF v_line_debit > 0 AND v_line_credit > 0 THEN + RETURN jsonb_build_object('ok', false, 'code', 'BULK_BOOK_BOTH_SIDES_NONZERO', + 'details', jsonb_build_object('account', v_line_account)); + END IF; + IF v_line ? 'dimensions' + AND jsonb_typeof(v_line->'dimensions') IS DISTINCT FROM 'object' THEN + RETURN jsonb_build_object('ok', false, 'code', 'BULK_BOOK_INVALID_DIMENSIONS', + 'details', jsonb_build_object('account', v_line_account)); + END IF; + v_lines_total_debit := v_lines_total_debit + v_line_debit; + v_lines_total_credit := v_lines_total_credit + v_line_credit; + IF length(v_line_account) = 4 AND v_line_account BETWEEN '1900' AND '1999' THEN + v_lines_bank_net := v_lines_bank_net + v_line_debit - v_line_credit; + END IF; + END LOOP; + + IF ABS(v_lines_total_debit - v_lines_total_credit) > 0.005 THEN + RETURN jsonb_build_object('ok', false, 'code', 'BULK_BOOK_UNBALANCED', + 'details', jsonb_build_object( + 'debit_sum', v_lines_total_debit, 'credit_sum', v_lines_total_credit)); + END IF; + + IF ABS(v_lines_bank_net - v_total_amount) > 0.005 THEN + RETURN jsonb_build_object('ok', false, 'code', 'BULK_BOOK_AMOUNT_MISMATCH', + 'details', jsonb_build_object( + 'tx_sum', v_total_amount, + 'lines_bank_net', v_lines_bank_net)); + END IF; + + SELECT id, is_closed, locked_at INTO v_fiscal_period_id, v_period_is_closed, v_period_locked_at + FROM public.fiscal_periods + WHERE company_id = p_company_id AND v_tx_date BETWEEN period_start AND period_end + ORDER BY period_start DESC LIMIT 1; + + IF v_fiscal_period_id IS NULL THEN + RETURN jsonb_build_object('ok', false, 'code', 'BULK_BOOK_NO_FISCAL_PERIOD', + 'details', jsonb_build_object('tx_date', v_tx_date)); + END IF; + + IF v_period_is_closed OR v_period_locked_at IS NOT NULL THEN + RETURN jsonb_build_object('ok', false, 'code', 'BULK_BOOK_PERIOD_LOCKED', + 'details', jsonb_build_object('fiscal_period_id', v_fiscal_period_id)); + END IF; + + v_journal_entry_id := gen_random_uuid(); + + INSERT INTO public.journal_entries + (id, user_id, company_id, fiscal_period_id, voucher_number, voucher_series, + entry_date, description, source_type, status) + VALUES + (v_journal_entry_id, v_caller, p_company_id, v_fiscal_period_id, 0, v_voucher_series, + v_tx_date, v_entry_description, 'manual', 'draft'); + + v_sort_order := 0; + FOR v_line IN SELECT * FROM jsonb_array_elements(p_new_entry->'lines') + LOOP + v_line_account := v_line->>'account_number'; + v_line_debit := COALESCE((v_line->>'debit_amount')::numeric, 0); + v_line_credit := COALESCE((v_line->>'credit_amount')::numeric, 0); + v_line_currency := COALESCE(v_line->>'currency', 'SEK'); + + -- Bag normalization as before (DimensionsBagSchema parity). PR9: the + -- generated mirrors derive from the stored bag: no explicit columns. + SELECT COALESCE(jsonb_object_agg(d.key, btrim(d.value)), '{}'::jsonb) + INTO v_line_dims + FROM jsonb_each_text(COALESCE(v_line->'dimensions', '{}'::jsonb)) AS d + WHERE d.key ~ '^[1-9][0-9]*$' AND btrim(d.value) <> ''; + + INSERT INTO public.journal_entry_lines + (journal_entry_id, account_number, debit_amount, credit_amount, currency, + sort_order, line_description, dimensions) + VALUES + (v_journal_entry_id, v_line_account, v_line_debit, v_line_credit, v_line_currency, + COALESCE((v_line->>'sort_order')::int, v_sort_order), + v_line->>'line_description', + v_line_dims); + + v_sort_order := v_sort_order + 1; + END LOOP; + + SELECT voucher_number INTO v_voucher_number + FROM public.commit_journal_entry(p_company_id, v_journal_entry_id); + + FOR v_tx IN + SELECT * FROM public.transactions + WHERE id = ANY(p_tx_ids) AND company_id = p_company_id + ORDER BY id + LOOP + INSERT INTO public.transaction_voucher_links + (user_id, company_id, transaction_id, journal_entry_id, allocated_amount, role) + VALUES + (v_caller, p_company_id, v_tx.id, v_journal_entry_id, v_tx.amount, 'bank_line'); + END LOOP; + + IF v_tx_count = 1 THEN + UPDATE public.transactions + SET journal_entry_id = v_journal_entry_id, + is_business = TRUE, + updated_at = v_now + WHERE id = p_tx_ids[1]; + ELSE + UPDATE public.transactions + SET is_business = TRUE, updated_at = v_now + WHERE id = ANY(p_tx_ids); + END IF; + + v_target_je := v_journal_entry_id; + END IF; + + WITH linked AS ( + UPDATE public.document_attachments AS d + SET journal_entry_id = v_target_je, + updated_at = v_now + FROM public.transactions AS t + WHERE t.id = ANY(p_tx_ids) + AND t.company_id = p_company_id + AND t.document_id = d.id + AND d.company_id = p_company_id + AND d.journal_entry_id IS NULL + RETURNING d.id + ) + SELECT COUNT(*)::int INTO v_docs_linked FROM linked; + + RETURN jsonb_build_object( + 'ok', true, + 'mode', CASE WHEN p_existing_journal_entry_id IS NOT NULL THEN 'link_existing' ELSE 'create_new' END, + 'journal_entry_id', v_target_je, + 'voucher_series', v_voucher_series, + 'voucher_number', v_voucher_number, + 'linked_tx_count', v_tx_count, + 'tx_sum', v_total_amount, + 'docs_linked', v_docs_linked + ); +END; +$$; + +COMMENT ON FUNCTION public.bulk_book_transactions(uuid[], uuid, jsonb, uuid, uuid) IS + 'Bulk-book N SEK bank transactions sharing the same date into a single combined verifikat (samlingsverifikation per BFL 5 kap 6§). Mixed-currency selections are refused with BULK_BOOK_MIXED_CURRENCY (one redovisningsvaluta per BFL 4 kap 6§) and homogeneous non-SEK selections with BULK_BOOK_FOREIGN_CURRENCY (the ledger columns are always kronor and this RPC has no exchange rate). Dimensions PR9: lines write the dimensions bag only: cost_center/project are GENERATED columns derived from keys 1/6. p_user_id is honored only for service_role callers (pending-operations commit path).'; + +-- Default privileges hand new functions to anon as well (the Supabase +-- template grants EXECUTE ON FUNCTIONS to anon/authenticated/service_role), +-- so PUBLIC and anon are revoked explicitly, as match_batch_allocate does. +REVOKE ALL ON FUNCTION public.bulk_book_transactions(uuid[], uuid, jsonb, uuid, uuid) FROM PUBLIC, anon; +GRANT EXECUTE ON FUNCTION public.bulk_book_transactions(uuid[], uuid, jsonb, uuid, uuid) TO authenticated, service_role; + +NOTIFY pgrst, 'reload schema'; diff --git a/tests/pg/bulk-book-transactions-service-actor.pg.test.ts b/tests/pg/bulk-book-transactions-service-actor.pg.test.ts new file mode 100644 index 00000000..bab7bb48 --- /dev/null +++ b/tests/pg/bulk-book-transactions-service-actor.pg.test.ts @@ -0,0 +1,198 @@ +import { randomUUID } from 'node:crypto' +import { describe, expect, it } from 'vitest' +import { + insertAuthUser, + insertCompany, + insertCompanyMember, + insertFiscalPeriod, +} from '@/tests/pg/fixtures' +import { getPool, runAsServiceRole, withUserContext } from '@/tests/pg/setup' + +/** + * Covers 20260824170000_bulk_book_transactions_service_actor: + * - service_role caller + p_user_id of a member: the samlingsverifikat + * commits. This is the pending-operations commit path + * (createServiceClientNoCookies), which before the migration ALWAYS got + * BULK_BOOK_UNAUTHORIZED because auth.uid() is NULL on the service + * client, and the dispatcher then consumed the op (feedback seq 261545). + * - p_user_id is an assertion, honored ONLY under auth.role() = + * 'service_role': an authenticated non-member spoofing an owner's UUID + * and a caller with no JWT at all both stay BULK_BOOK_UNAUTHORIZED. + * - Grants: PUBLIC/anon revoked, authenticated + service_role kept; the + * old 4-arg signature is gone (the 5th arg has a DEFAULT, so 4-arg call + * sites still resolve). + */ + +async function insertTransaction(params: { + userId: string + companyId: string + amount: number +}): Promise { + const id = randomUUID() + await getPool().query( + `INSERT INTO public.transactions + (id, user_id, company_id, date, description, amount, currency, category) + VALUES ($1, $2, $3, '2026-06-05', 'Swish inbetalning', $4, 'SEK', 'uncategorized')`, + [id, params.userId, params.companyId, params.amount], + ) + return id +} + +async function seedTenant() { + const userId = await insertAuthUser() + const companyId = await insertCompany({ createdBy: userId }) + await insertCompanyMember({ companyId, userId, role: 'owner' }) + await insertFiscalPeriod({ + userId, + companyId, + periodStart: '2026-01-01', + periodEnd: '2026-12-31', + }) + // The RPC validates every line's account_number against the company's + // active chart; seed only the accounts the entry touches. + await getPool().query( + `INSERT INTO public.chart_of_accounts + (user_id, company_id, account_number, account_name, account_class, account_type, normal_balance, is_active) + SELECT $1, $2, n, name, cls, atype, nbal, true + FROM (VALUES + ('1930', 'Bankkonto', 1, 'asset', 'debit'), + ('2611', 'Utgående moms 25%', 2, 'liability', 'credit'), + ('3001', 'Försäljning 25% moms', 3, 'revenue', 'credit') + ) AS t(n, name, cls, atype, nbal)`, + [userId, companyId], + ) + const tx1 = await insertTransaction({ userId, companyId, amount: 250 }) + const tx2 = await insertTransaction({ userId, companyId, amount: 350 }) + return { userId, companyId, txIds: [tx1, tx2] } +} + +const NEW_ENTRY = { + description: 'Dagskassa Swish', + lines: [ + { account_number: '1930', debit_amount: 600, credit_amount: 0, currency: 'SEK', line_description: 'Inbetalningar Swish' }, + { account_number: '3001', debit_amount: 0, credit_amount: 480, currency: 'SEK', line_description: 'Försäljning' }, + { account_number: '2611', debit_amount: 0, credit_amount: 120, currency: 'SEK', line_description: 'Utgående moms 25%' }, + ], +} + +interface RpcResult { + ok: boolean + code?: string + journal_entry_id?: string + linked_tx_count?: number +} + +const CALL = `SELECT bulk_book_transactions($1::uuid[], $2, $3::jsonb, $4, $5) AS result` + +describe('bulk_book_transactions service actor', () => { + it('commits for a service_role caller with p_user_id of a member and attributes the verifikat to that user', async () => { + const { userId, companyId, txIds } = await seedTenant() + + const result = await runAsServiceRole(async (client) => { + const r = await client.query<{ result: RpcResult }>(CALL, [ + txIds, + null, + JSON.stringify(NEW_ENTRY), + companyId, + userId, + ]) + return r.rows[0]!.result + }) + + expect(result.ok).toBe(true) + expect(result.journal_entry_id).toBeTruthy() + expect(result.linked_tx_count).toBe(2) + + const je = await getPool().query<{ user_id: string; status: string }>( + `SELECT user_id, status FROM public.journal_entries WHERE id = $1`, + [result.journal_entry_id], + ) + expect(je.rows[0]!.user_id).toBe(userId) + expect(je.rows[0]!.status).toBe('posted') + }) + + it('still rejects a service_role caller that passes no p_user_id', async () => { + const { companyId, txIds } = await seedTenant() + + const result = await runAsServiceRole(async (client) => { + const r = await client.query<{ result: RpcResult }>(CALL, [ + txIds, + null, + JSON.stringify(NEW_ENTRY), + companyId, + null, + ]) + return r.rows[0]!.result + }) + + expect(result.ok).toBe(false) + expect(result.code).toBe('BULK_BOOK_UNAUTHORIZED') + }) + + it('ignores a spoofed p_user_id from an authenticated non-member', async () => { + const { userId, companyId, txIds } = await seedTenant() + const stranger = await insertAuthUser() + + const result = await withUserContext(stranger, async (client) => { + const r = await client.query<{ result: RpcResult }>(CALL, [ + txIds, + null, + JSON.stringify(NEW_ENTRY), + companyId, + userId, + ]) + return r.rows[0]!.result + }) + + expect(result.ok).toBe(false) + expect(result.code).toBe('BULK_BOOK_UNAUTHORIZED') + }) + + it('ignores p_user_id when there is no JWT context at all', async () => { + const { userId, companyId, txIds } = await seedTenant() + + const r = await getPool().query<{ result: RpcResult }>(CALL, [ + txIds, + null, + JSON.stringify(NEW_ENTRY), + companyId, + userId, + ]) + expect(r.rows[0]!.result.ok).toBe(false) + expect(r.rows[0]!.result.code).toBe('BULK_BOOK_UNAUTHORIZED') + }) + + it('keeps the 4-arg call shape working for authenticated members (web route)', async () => { + const { userId, companyId, txIds } = await seedTenant() + + const result = await withUserContext(userId, async (client) => { + const r = await client.query<{ result: RpcResult }>( + `SELECT bulk_book_transactions($1::uuid[], $2, $3::jsonb, $4) AS result`, + [txIds, null, JSON.stringify(NEW_ENTRY), companyId], + ) + return r.rows[0]!.result + }) + + expect(result.ok).toBe(true) + }) + + it('keeps least-privilege grants and drops the 4-arg overload', async () => { + const { rows } = await getPool().query<{ + anon_can: boolean + authenticated_can: boolean + service_role_can: boolean + overloads: string + }>( + `SELECT has_function_privilege('anon', 'public.bulk_book_transactions(uuid[],uuid,jsonb,uuid,uuid)', 'EXECUTE') AS anon_can, + has_function_privilege('authenticated', 'public.bulk_book_transactions(uuid[],uuid,jsonb,uuid,uuid)', 'EXECUTE') AS authenticated_can, + has_function_privilege('service_role', 'public.bulk_book_transactions(uuid[],uuid,jsonb,uuid,uuid)', 'EXECUTE') AS service_role_can, + (SELECT count(*) FROM pg_proc p + JOIN pg_namespace n ON n.oid = p.pronamespace + WHERE n.nspname = 'public' AND p.proname = 'bulk_book_transactions')::text AS overloads`, + ) + expect(rows[0]!.anon_can).toBe(false) + expect(rows[0]!.authenticated_can).toBe(true) + expect(rows[0]!.service_role_can).toBe(true) + expect(rows[0]!.overloads).toBe('1') + }) +})