From fe137346dff2ebbed47ee20f687798cc0d815f87 Mon Sep 17 00:00:00 2001 From: Jakob Wennberg <149234542+jakobwennberg@users.noreply.github.com> Date: Thu, 16 Apr 2026 15:46:27 +0200 Subject: [PATCH] fix: prevent silent data truncation in report generators (#257) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: prevent silent data truncation in report generators Supabase PostgREST silently truncates queries at 1000 rows. Several report generators used bare .select() without fetchAllRows(), causing incomplete financial data — a BFL compliance violation. Wrapped 10 queries across 7 files with fetchAllRows(): - monthly-breakdown: journal_entry_lines (easily >1000/period) - ar-ledger: unpaid invoices - supplier-ledger: unpaid supplier invoices - salary-journal: salary_run_employees (+ optimized with !inner join) - vacation-liability: employees + salary_run_employees - full-archive-export: document_attachments + journal_entry IDs - ingest.ts: supplier invoices for auto-matching Co-Authored-By: Claude Opus 4.6 (1M context) * fix: address Greptile review — try/catch and client-side safety checks - Wrap full-archive-export document fetch in try/catch to match the VAT section pattern — a failed document query should not prevent the rest of the archive from being generated. - Restore client-side year/status safety checks in salary-journal and vacation-liability as defense-in-depth against PostgREST !inner filter regressions, per BFL lönejournal and BFNAR 2016:10 compliance. Co-Authored-By: Claude Opus 4.6 (1M context) --------- Co-authored-by: Claude Opus 4.6 (1M context) --- lib/reports/__tests__/ar-ledger.test.ts | 2 +- .../__tests__/monthly-breakdown.test.ts | 136 +++++++++--------- lib/reports/__tests__/supplier-ledger.test.ts | 2 +- lib/reports/ar-ledger.ts | 20 ++- lib/reports/full-archive-export.ts | 45 ++++-- lib/reports/monthly-breakdown.ts | 44 +++--- lib/reports/salary-journal.ts | 39 ++--- lib/reports/supplier-ledger.ts | 20 ++- lib/reports/vacation-liability.ts | 58 ++++---- lib/transactions/ingest.ts | 18 +-- 10 files changed, 217 insertions(+), 167 deletions(-) diff --git a/lib/reports/__tests__/ar-ledger.test.ts b/lib/reports/__tests__/ar-ledger.test.ts index d74ffc1c..6bab7855 100644 --- a/lib/reports/__tests__/ar-ledger.test.ts +++ b/lib/reports/__tests__/ar-ledger.test.ts @@ -9,7 +9,7 @@ let results: Array<{ data?: unknown; error?: unknown }> function makeBuilder() { const b: Record = {} - for (const m of ['select', 'eq', 'in']) { + for (const m of ['select', 'eq', 'in', 'range']) { b[m] = vi.fn().mockReturnValue(b) } b.single = vi.fn().mockImplementation(async () => results[resultIdx++] ?? { data: null, error: null }) diff --git a/lib/reports/__tests__/monthly-breakdown.test.ts b/lib/reports/__tests__/monthly-breakdown.test.ts index e4973f9a..83e54c91 100644 --- a/lib/reports/__tests__/monthly-breakdown.test.ts +++ b/lib/reports/__tests__/monthly-breakdown.test.ts @@ -54,11 +54,13 @@ describe('generateMonthlyBreakdown', () => { select: () => ({ eq: () => ({ eq: () => ({ - eq: () => - Promise.resolve({ - data: [], - error: null, - }), + eq: () => ({ + range: () => + Promise.resolve({ + data: [], + error: null, + }), + }), }), }), }), @@ -96,36 +98,38 @@ describe('generateMonthlyBreakdown', () => { select: () => ({ eq: () => ({ eq: () => ({ - eq: () => - Promise.resolve({ - data: [ - { - account_number: '3001', - debit_amount: 0, - credit_amount: 10000, - journal_entry: { entry_date: '2024-01-15', status: 'posted', user_id: 'user-1', fiscal_period_id: 'period-1' }, - }, - { - account_number: '5010', - debit_amount: 3000, - credit_amount: 0, - journal_entry: { entry_date: '2024-01-20', status: 'posted', user_id: 'user-1', fiscal_period_id: 'period-1' }, - }, - { - account_number: '3001', - debit_amount: 0, - credit_amount: 5000, - journal_entry: { entry_date: '2024-02-10', status: 'posted', user_id: 'user-1', fiscal_period_id: 'period-1' }, - }, - { - account_number: '6200', - debit_amount: 1500, - credit_amount: 0, - journal_entry: { entry_date: '2024-02-15', status: 'posted', user_id: 'user-1', fiscal_period_id: 'period-1' }, - }, - ], - error: null, - }), + eq: () => ({ + range: () => + Promise.resolve({ + data: [ + { + account_number: '3001', + debit_amount: 0, + credit_amount: 10000, + journal_entry: { entry_date: '2024-01-15', status: 'posted', user_id: 'user-1', fiscal_period_id: 'period-1' }, + }, + { + account_number: '5010', + debit_amount: 3000, + credit_amount: 0, + journal_entry: { entry_date: '2024-01-20', status: 'posted', user_id: 'user-1', fiscal_period_id: 'period-1' }, + }, + { + account_number: '3001', + debit_amount: 0, + credit_amount: 5000, + journal_entry: { entry_date: '2024-02-10', status: 'posted', user_id: 'user-1', fiscal_period_id: 'period-1' }, + }, + { + account_number: '6200', + debit_amount: 1500, + credit_amount: 0, + journal_entry: { entry_date: '2024-02-15', status: 'posted', user_id: 'user-1', fiscal_period_id: 'period-1' }, + }, + ], + error: null, + }), + }), }), }), }), @@ -175,36 +179,38 @@ describe('generateMonthlyBreakdown', () => { select: () => ({ eq: () => ({ eq: () => ({ - eq: () => - Promise.resolve({ - data: [ - { - account_number: '1930', - debit_amount: 10000, - credit_amount: 0, - journal_entry: { entry_date: '2024-01-15', status: 'posted', user_id: 'user-1', fiscal_period_id: 'period-1' }, - }, - { - account_number: '2611', - debit_amount: 0, - credit_amount: 2500, - journal_entry: { entry_date: '2024-01-15', status: 'posted', user_id: 'user-1', fiscal_period_id: 'period-1' }, - }, - { - account_number: '8400', - debit_amount: 500, - credit_amount: 0, - journal_entry: { entry_date: '2024-01-20', status: 'posted', user_id: 'user-1', fiscal_period_id: 'period-1' }, - }, - { - account_number: '8300', - debit_amount: 0, - credit_amount: 200, - journal_entry: { entry_date: '2024-01-25', status: 'posted', user_id: 'user-1', fiscal_period_id: 'period-1' }, - }, - ], - error: null, - }), + eq: () => ({ + range: () => + Promise.resolve({ + data: [ + { + account_number: '1930', + debit_amount: 10000, + credit_amount: 0, + journal_entry: { entry_date: '2024-01-15', status: 'posted', user_id: 'user-1', fiscal_period_id: 'period-1' }, + }, + { + account_number: '2611', + debit_amount: 0, + credit_amount: 2500, + journal_entry: { entry_date: '2024-01-15', status: 'posted', user_id: 'user-1', fiscal_period_id: 'period-1' }, + }, + { + account_number: '8400', + debit_amount: 500, + credit_amount: 0, + journal_entry: { entry_date: '2024-01-20', status: 'posted', user_id: 'user-1', fiscal_period_id: 'period-1' }, + }, + { + account_number: '8300', + debit_amount: 0, + credit_amount: 200, + journal_entry: { entry_date: '2024-01-25', status: 'posted', user_id: 'user-1', fiscal_period_id: 'period-1' }, + }, + ], + error: null, + }), + }), }), }), }), diff --git a/lib/reports/__tests__/supplier-ledger.test.ts b/lib/reports/__tests__/supplier-ledger.test.ts index 14af4946..26faf623 100644 --- a/lib/reports/__tests__/supplier-ledger.test.ts +++ b/lib/reports/__tests__/supplier-ledger.test.ts @@ -9,7 +9,7 @@ let results: Array<{ data?: unknown; error?: unknown }> function makeBuilder() { const b: Record = {} - for (const m of ['select', 'eq', 'in']) { + for (const m of ['select', 'eq', 'in', 'range']) { b[m] = vi.fn().mockReturnValue(b) } b.single = vi.fn().mockImplementation(async () => results[resultIdx++] ?? { data: null, error: null }) diff --git a/lib/reports/ar-ledger.ts b/lib/reports/ar-ledger.ts index 529065ea..96800e24 100644 --- a/lib/reports/ar-ledger.ts +++ b/lib/reports/ar-ledger.ts @@ -1,4 +1,5 @@ import type { SupabaseClient } from '@supabase/supabase-js' +import { fetchAllRows } from '@/lib/supabase/fetch-all' export interface ARInvoiceDetail { invoice_id: string @@ -44,13 +45,18 @@ export async function generateARLedger( const refDate = asOfDate ? new Date(asOfDate) : new Date() // Fetch all unpaid/sent/overdue invoices with customer info - const { data: invoices, error } = await supabase - .from('invoices') - .select('*, customer:customers(id, name)') - .eq('company_id', companyId) - .in('status', ['sent', 'overdue']) - - if (error || !invoices) { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + let invoices: any[] + try { + invoices = await fetchAllRows(({ from, to }) => + supabase + .from('invoices') + .select('*, customer:customers(id, name)') + .eq('company_id', companyId) + .in('status', ['sent', 'overdue']) + .range(from, to) + ) + } catch { return { entries: [], total_outstanding: 0, diff --git a/lib/reports/full-archive-export.ts b/lib/reports/full-archive-export.ts index 9026c85f..3f6a6c3c 100644 --- a/lib/reports/full-archive-export.ts +++ b/lib/reports/full-archive-export.ts @@ -8,6 +8,7 @@ import { generateGeneralLedger } from './general-ledger' import { generateJournalRegister } from './journal-register' import { calculateVatDeclaration } from './vat-declaration' import { getAuditLog } from '@/lib/core/audit/audit-service' +import { fetchAllRows } from '@/lib/supabase/fetch-all' import type { AuditLogEntry } from '@/types' export interface FullArchiveOptions { @@ -117,22 +118,35 @@ export async function generateFullArchive( const manifest: DocumentManifestEntry[] = [] // Fetch document attachments linked to journal entries in this period - const { data: documents } = await supabase - .from('document_attachments') - .select('id, file_name, storage_path, journal_entry_id, sha256_hash, version, digitization_date, upload_source, mime_type, file_size_bytes') - .eq('company_id', companyId) - .not('journal_entry_id', 'is', null) - - if (documents && documents.length > 0) { - // Filter to entries in this period - const { data: periodEntryIds } = await supabase - .from('journal_entries') - .select('id') + // Wrapped in try/catch to match VAT section — a failed document fetch + // should not prevent the rest of the archive from being generated. + try { + const documents = await fetchAllRows<{ + id: string; file_name: string; storage_path: string; journal_entry_id: string | null + sha256_hash: string; version: number; digitization_date: string | null + upload_source: string | null; mime_type: string | null; file_size_bytes: number | null + }>(({ from, to }) => + supabase + .from('document_attachments') + .select('id, file_name, storage_path, journal_entry_id, sha256_hash, version, digitization_date, upload_source, mime_type, file_size_bytes') .eq('company_id', companyId) - .eq('fiscal_period_id', period_id) - .in('status', ['posted', 'reversed']) + .not('journal_entry_id', 'is', null) + .range(from, to) + ) - const periodEntryIdSet = new Set((periodEntryIds || []).map((e: { id: string }) => e.id)) + if (documents.length > 0) { + // Filter to entries in this period + const periodEntryIds = await fetchAllRows<{ id: string }>(({ from, to }) => + supabase + .from('journal_entries') + .select('id') + .eq('company_id', companyId) + .eq('fiscal_period_id', period_id) + .in('status', ['posted', 'reversed']) + .range(from, to) + ) + + const periodEntryIdSet = new Set(periodEntryIds.map((e) => e.id)) const periodDocuments = documents.filter( (d: { journal_entry_id: string | null }) => d.journal_entry_id && periodEntryIdSet.has(d.journal_entry_id) ) @@ -182,6 +196,9 @@ export async function generateFullArchive( } } } + } catch { + // Document fetch failed — archive will still contain reports and audit trail + } dokument.file('manifest.json', JSON.stringify(manifest, null, 2)) } diff --git a/lib/reports/monthly-breakdown.ts b/lib/reports/monthly-breakdown.ts index cbdc41d4..832a93cd 100644 --- a/lib/reports/monthly-breakdown.ts +++ b/lib/reports/monthly-breakdown.ts @@ -1,4 +1,5 @@ import type { SupabaseClient } from '@supabase/supabase-js' +import { fetchAllRows } from '@/lib/supabase/fetch-all' export interface MonthlyBreakdownMonth { label: string @@ -42,24 +43,29 @@ export async function generateMonthlyBreakdown( } // Get all posted journal entry lines for this period with their entry dates - const { data: lines, error: linesError } = await supabase - .from('journal_entry_lines') - .select(` - account_number, - debit_amount, - credit_amount, - journal_entry:journal_entries!inner( - entry_date, - status, - company_id, - fiscal_period_id - ) - `) - .eq('journal_entries.fiscal_period_id', fiscalPeriodId) - .eq('journal_entries.company_id', companyId) - .eq('journal_entries.status', 'posted') - - if (linesError || !lines) { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + let lines: any[] + try { + lines = await fetchAllRows(({ from, to }) => + supabase + .from('journal_entry_lines') + .select(` + account_number, + debit_amount, + credit_amount, + journal_entry:journal_entries!inner( + entry_date, + status, + company_id, + fiscal_period_id + ) + `) + .eq('journal_entries.fiscal_period_id', fiscalPeriodId) + .eq('journal_entries.company_id', companyId) + .eq('journal_entries.status', 'posted') + .range(from, to) + ) + } catch { return { months: [] } } @@ -81,7 +87,7 @@ export async function generateMonthlyBreakdown( } for (const line of lines) { - const entry = line.journal_entry as unknown as { + const entry = line.journal_entry as { entry_date: string status: string company_id: string diff --git a/lib/reports/salary-journal.ts b/lib/reports/salary-journal.ts index 242c327a..9660a8a7 100644 --- a/lib/reports/salary-journal.ts +++ b/lib/reports/salary-journal.ts @@ -1,4 +1,5 @@ import type { SupabaseClient } from '@supabase/supabase-js' +import { fetchAllRows } from '@/lib/supabase/fetch-all' /** * Lönejournal — Monthly/annual per-employee salary register. @@ -57,29 +58,29 @@ export async function generateSalaryJournal( monthFrom?: number, monthTo?: number ): Promise { - const query = supabase - .from('salary_run_employees') - .select(` - *, - employee:employees(id, first_name, last_name, personnummer_last4, employment_type), - salary_run:salary_runs(period_year, period_month, payment_date, status) - `) - .eq('company_id', companyId) + // Use !inner join to filter server-side by year and status, avoiding + // fetching all salary_run_employees across all years. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const data: any[] = await fetchAllRows(({ from, to }) => + supabase + .from('salary_run_employees') + .select(` + *, + employee:employees(id, first_name, last_name, personnummer_last4, employment_type), + salary_run:salary_runs!inner(period_year, period_month, payment_date, status) + `) + .eq('company_id', companyId) + .eq('salary_runs.period_year', year) + .eq('salary_runs.status', 'booked') + .order('created_at') + .range(from, to) + ) - // We need to filter by the salary_run's period_year, which requires a join filter - // Supabase doesn't support filtering on joined columns directly in .eq(), - // so we fetch all and filter client-side for the year - const { data, error } = await query.order('created_at') - - if (error) { - throw new Error(`Failed to generate salary journal: ${error.message}`) - } - - const rows: SalaryJournalRow[] = (data || []) + const rows: SalaryJournalRow[] = data .filter(sre => { const run = sre.salary_run as { period_year: number; period_month: number; status: string } | null if (!run || run.period_year !== year) return false - if (run.status !== 'booked') return false // Only booked runs for BFL-compliant lönejournal + if (run.status !== 'booked') return false if (monthFrom && run.period_month < monthFrom) return false if (monthTo && run.period_month > monthTo) return false return true diff --git a/lib/reports/supplier-ledger.ts b/lib/reports/supplier-ledger.ts index afd32ed9..392bc3dc 100644 --- a/lib/reports/supplier-ledger.ts +++ b/lib/reports/supplier-ledger.ts @@ -1,4 +1,5 @@ import type { SupabaseClient } from '@supabase/supabase-js' +import { fetchAllRows } from '@/lib/supabase/fetch-all' export interface SupplierLedgerEntry { supplier_id: string @@ -30,13 +31,18 @@ export async function generateSupplierLedger( const refDate = asOfDate ? new Date(asOfDate) : new Date() // Fetch all unpaid/partially_paid supplier invoices - const { data: invoices, error } = await supabase - .from('supplier_invoices') - .select('*, supplier:suppliers(id, name)') - .eq('company_id', companyId) - .in('status', ['registered', 'approved', 'partially_paid', 'overdue']) - - if (error || !invoices) { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + let invoices: any[] + try { + invoices = await fetchAllRows(({ from, to }) => + supabase + .from('supplier_invoices') + .select('*, supplier:suppliers(id, name)') + .eq('company_id', companyId) + .in('status', ['registered', 'approved', 'partially_paid', 'overdue']) + .range(from, to) + ) + } catch { return { entries: [], total_outstanding: 0, diff --git a/lib/reports/vacation-liability.ts b/lib/reports/vacation-liability.ts index 0b32ae9b..893895a2 100644 --- a/lib/reports/vacation-liability.ts +++ b/lib/reports/vacation-liability.ts @@ -1,4 +1,5 @@ import type { SupabaseClient } from '@supabase/supabase-js' +import { fetchAllRows } from '@/lib/supabase/fetch-all' /** * Semesterlöneskuld — Vacation liability report per BFNAR 2016:10. @@ -50,32 +51,37 @@ export async function generateVacationLiability( const r = (x: number) => Math.round(x * 100) / 100 // Load active employees - const { data: employees, error: empError } = await supabase - .from('employees') - .select('id, first_name, last_name, personnummer_last4, vacation_rule, vacation_days_per_year, vacation_days_saved') - .eq('company_id', companyId) - .eq('is_active', true) - .order('last_name') + const employees = await fetchAllRows(({ from, to }) => + supabase + .from('employees') + .select('id, first_name, last_name, personnummer_last4, vacation_rule, vacation_days_per_year, vacation_days_saved') + .eq('company_id', companyId) + .eq('is_active', true) + .order('last_name') + .range(from, to) + ) - if (empError) throw new Error(`Failed to load employees: ${empError.message}`) + // Load salary run employees for booked runs this year (server-side filtered via !inner join) + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const bookedForYear: any[] = await fetchAllRows(({ from, to }) => + supabase + .from('salary_run_employees') + .select(` + employee_id, + vacation_accrual, + vacation_accrual_avgifter, + avgifter_rate, + vacation_days_taken, + salary_run:salary_runs!inner(period_year, status) + `) + .eq('company_id', companyId) + .eq('salary_runs.period_year', year) + .eq('salary_runs.status', 'booked') + .range(from, to) + ) - // Load all salary run employees for booked runs this year - const { data: runEmployees, error: sreError } = await supabase - .from('salary_run_employees') - .select(` - employee_id, - vacation_accrual, - vacation_accrual_avgifter, - avgifter_rate, - vacation_days_taken, - salary_run:salary_runs!inner(period_year, status) - `) - .eq('company_id', companyId) - - if (sreError) throw new Error(`Failed to load salary run data: ${sreError.message}`) - - // Filter to booked runs for the year - const bookedForYear = (runEmployees || []).filter(sre => { + // Client-side safety check: ensure server-side !inner filter was applied + const verifiedBookedForYear = bookedForYear.filter(sre => { const run = sre.salary_run as unknown as { period_year: number; status: string } | null return run && run.period_year === year && run.status === 'booked' }) @@ -88,7 +94,7 @@ export async function generateVacationLiability( lastRate: number }>() - for (const sre of bookedForYear) { + for (const sre of verifiedBookedForYear) { const current = accrualsByEmployee.get(sre.employee_id) || { totalAccrual: 0, totalAvgifter: 0, totalDaysTaken: 0, lastRate: 0.3142, } @@ -99,7 +105,7 @@ export async function generateVacationLiability( accrualsByEmployee.set(sre.employee_id, current) } - const rows: VacationLiabilityRow[] = (employees || []).map(emp => { + const rows: VacationLiabilityRow[] = employees.map(emp => { const accruals = accrualsByEmployee.get(emp.id) const accruedAmount = r(accruals?.totalAccrual || 0) const accruedAvgifter = r(accruals?.totalAvgifter || 0) diff --git a/lib/transactions/ingest.ts b/lib/transactions/ingest.ts index e2ea92e9..a939893e 100644 --- a/lib/transactions/ingest.ts +++ b/lib/transactions/ingest.ts @@ -7,6 +7,7 @@ import { findSupplierInvoiceMatch } from '@/lib/invoices/supplier-invoice-matchi import { tryReconcileTransaction, fetchUnlinkedGLLines } from '@/lib/reconciliation/bank-reconciliation' import { fetchMultipleRates } from '@/lib/currency/riksbanken' import { logMatchEvent } from '@/lib/invoices/match-log' +import { fetchAllRows } from '@/lib/supabase/fetch-all' import type { UnlinkedGLLine } from '@/lib/reconciliation/bank-reconciliation' import type { Transaction, RawTransaction, IngestResult, IngestOptions, SupplierInvoice, Currency, ExchangeRate } from '@/types' @@ -135,14 +136,15 @@ export async function ingestTransactions( // Pre-fetch unpaid supplier invoices for expense matching (non-critical) try { - const { data } = await supabase - .from('supplier_invoices') - .select('*, supplier:suppliers(*)') - .eq('company_id', companyId) - .in('status', ['registered', 'approved']) - .gt('remaining_amount', 0) - - if (data) unpaidSupplierInvoices = data as SupplierInvoice[] + unpaidSupplierInvoices = await fetchAllRows(({ from, to }) => + supabase + .from('supplier_invoices') + .select('*, supplier:suppliers(*)') + .eq('company_id', companyId) + .in('status', ['registered', 'approved']) + .gt('remaining_amount', 0) + .range(from, to) + ) } catch { // Non-critical — supplier invoice matching will be skipped }