fix: audit batch — pagination truncation, MFA/dead-code cleanup, mark-paid fail-closed (#841)

* fix(reports): paginate 8 more report/ledger queries (1000-row truncation) Raw .select() without fetchAllRows() silently caps at PostgREST's 1000-row limit, producing wrong statutory output for high-volume companies. Following #806 (trial-balance/VAT), wrap the remaining offenders in fetchAllRows + a stable .order('id') + dedupeBy: - ink2-engine / ne-engine: INK2 & NE-bilaga tax declarations under-counted - ar-reconciliation (1510/1513), supplier-reconciliation (2440): phantom "Ej avstämd" gaps - full-archive-export: 7-year DR archive (added a unique total order so rows are not silently skipped/duplicated across pages) - avgifter-basis, currency-revaluation, vat-declaration Adds a regression guard test asserting >1000 ledger lines are summed, not truncated at 1000. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(api): close extension-dispatcher MFA gap, scope /api/events to API key, sweep dead code Security/correctness: - ext/[...path] dispatcher now uses requireAuth() instead of inline supabase.auth.getUser(), enforcing MFA (AAL2) on hosted across the whole enabled-extension surface (banking sync, document upload/booking, supplier invoices, migration). Ratchets antipatterns-baseline raw-route-auth 168->165. - /api/events now filters by the API key's bound company_id instead of the user's active company (was a cross-company read with a scoped key). - enable-banking OAuth callback calls ensureInitialized() at module load so the PSD2 consent audit event (ASVS V16 / GDPR Art.30) isn't dropped on a cold-start instance. Dead-code sweep (all confirmed zero importers): - delete lib/tax/calculator.ts, lib/salary/engangsskatt.ts (+test), lib/email/resend.ts, lib/salary/salary-transaction-matcher.ts, lib/webhooks/diff.ts, lib/salary/effective-values.ts, lib/bookkeeping/template-prompt.ts - trim unused lib/vat/eu-countries.ts helpers (keep EU_COUNTRIES) - remove dead getAutomaticStatus() and the abandoned Activepieces CSP entry Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(invoices): fail closed when a payment journal entry doesn't post Three mark-paid paths (legacy route, v1 API, agent commit) diverged on the "mark paid but the JE failed" case — two would flip the invoice to paid (or leave an orphaned posted voucher) with no booking, silently diverging the GL from the AR/AP sub-ledger. Unify on fail-closed: - legacy + v1 + agent commitMarkInvoicePaid: never mark paid without a posted voucher; on a null/failed JE return INVOICE_PAID_BOOK_FAILED before any state mutation (v1 mirrors the match-invoice strict mode). - agent path: add the .in('status',[...]).select('id') CAS guard and cancel the orphaned voucher (cancelOrphanedPaymentEntry) on a lost race or update error, matching the web route. - legacy route: cancel the orphan on a non-race update error too (was only handled on the race branch). - supplier mark-paid: stop swallowing a failed supplier_invoice_payments insert — that row drives the reversal amount in payment-sync; roll back the status flip and cancel the voucher instead. - pending-ops orchestrator: error-check the terminal 'committed' write so an op stranded in 'committing' (the expire sweep only targets 'pending') is at least logged loudly. Adds a guard test for the legacy fail-closed path. Full unit suite green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(ci): unblock core build + address compliance-review findings - avgifter-basis.ts: fix the core-build TypeScript error — PostgREST's type-level select parser models the salary_run embed as an array, which wasn't assignable to the object-typed generic. Type it `unknown` (rows are read via an explicit cast), making it robust across postgrest-js versions. - /api/events: add a non-null companyId guard before the event_log query (defense-in-depth for the API-key-bound scope) — addresses ASVS V8.2.1 / ISO A.5.15. - supplier mark-paid: add a CAS guard (.eq('status', newStatus)) to the payment-insert-failure rollback so a concurrent settlement can't be clobbered — addresses ASVS V2.3. - dispatcher: add an AAL2 regression test asserting a non-MFA session is rejected (403) and the extension handler never runs — addresses the GDPR Art.32 review ask for the single extension chokepoint. Verified deletions are safe: effective-values.ts was a dead duplicate — the live AGI/payslip path inlines the same `?? override` coalescing (generate-declaration.ts), so AGI correctness is unaffected. next build: exit 0. Full unit suite: 6147 passing. ESLint clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-30 14:34:23 +02:00
parent b800dcd403
commit f8504f3bd0
32 changed files with 522 additions and 870 deletions
@@ -23,6 +23,11 @@ export async function GET(request: Request) {
  // Dual auth: API key or session
  let userId: string
  let supabase: SupabaseClient
+  // When authenticated via an API key, the key is BOUND to a specific company.
+  // Honor that binding (least privilege) rather than resolving the user's
+  // active company — otherwise a key scoped to company A would leak company B's
+  // events whenever the user's active_company_id happened to point elsewhere.
+  let keyCompanyId: string | null = null

  const token = extractBearerToken(request)
  if (token?.startsWith('gnubok_sk_')) {
@@ -31,6 +36,7 @@ export async function GET(request: Request) {
      return NextResponse.json({ error: authResult.error }, { status: authResult.status })
    }
    userId = authResult.userId
+    keyCompanyId = authResult.companyId
    supabase = createServiceClientNoCookies()
  } else {
    supabase = await createClient()
@@ -41,7 +47,14 @@ export async function GET(request: Request) {
    userId = user.id
  }

-  const companyId = await requireCompanyId(supabase, userId)
+  // Session auth resolves the active company; API-key auth uses the key's bound company.
+  const companyId = keyCompanyId ?? await requireCompanyId(supabase, userId)
+  // Defense in depth: never run the event_log query with an empty/undefined
+  // scope. requireCompanyId throws when there is no company, but guard the
+  // key-bound path too so a malformed binding can't widen the query scope.
+  if (!companyId) {
+    return NextResponse.json({ error: 'Forbidden' }, { status: 403 })
+  }

  // Validate query params
  const result = validateQuery(request, EventsQuerySchema)
@@ -1,10 +1,18 @@
 import { createServiceClient } from '@/lib/supabase/server'
 import { NextResponse } from 'next/server'
+import { ensureInitialized } from '@/lib/init'
 import { createSession, type AccountInfo } from '@/extensions/general/enable-banking/lib/api-client'
 import type { StoredAccount } from '@/extensions/general/enable-banking/types'
 import { eventBus } from '@/lib/events/bus'
 import { upsertFromPsd2 } from '@/lib/cash-accounts/service'

+// This route emits bank_connection.consent_granted / .cash_account_mirror_failed
+// (ASVS V16 / GDPR Art.30 audit events). ensureInitialized() must run at module
+// load so registerEventLogHandler() has subscribed before the first emit() —
+// otherwise the audit row is silently dropped on a cold instance where this
+// redirect route is the first event-emitting code path to execute.
+ensureInitialized()
+
 // Suggested BAS account per currency. Mirrors the AccountPickerDialog defaults
 // (SEK→1930, EUR→1932, USD→1933, GBP→1934). The user can re-map in the picker
 // after this callback redirects them.
@@ -27,11 +27,19 @@ vi.mock('@/lib/extensions/context-factory', () => ({
  }),
 }))

+// Default to "MFA not enforced" so existing tests authenticate normally;
+// the AAL2-gate regression test below flips this on.
+vi.mock('@/lib/auth/mfa', () => ({
+  shouldEnforceMfa: vi.fn(() => false),
+}))
+
 import { createClient } from '@/lib/supabase/server'
+import { shouldEnforceMfa } from '@/lib/auth/mfa'
 import { extensionRegistry } from '@/lib/extensions/registry'
 import { GET, POST } from '../route'

 const mockCreateClient = vi.mocked(createClient)
+const mockShouldEnforceMfa = vi.mocked(shouldEnforceMfa)

 function createPathParams(path: string[]) {
  return { params: Promise.resolve({ path }) }
@@ -40,6 +48,9 @@ function createPathParams(path: string[]) {
 describe('Extension Catch-All Route', () => {
  beforeEach(() => {
    vi.clearAllMocks()
+    // clearAllMocks doesn't reset implementations — re-assert the default so the
+    // AAL2 test's mockReturnValue(true) can't leak into later cases.
+    mockShouldEnforceMfa.mockReturnValue(false)
    extensionRegistry.clear()
  })

@@ -88,6 +99,40 @@ describe('Extension Catch-All Route', () => {
    expect(status).toBe(401)
  })

+  it('blocks a session that has not completed MFA (AAL2) and never dispatches the handler', async () => {
+    // Regression for the audit fix: the dispatcher is the single chokepoint for
+    // the whole extension surface, so an AAL1 (single-factor) session on hosted
+    // must be rejected before any extension handler runs.
+    const handler = vi.fn()
+    extensionRegistry.register({
+      id: 'test-ext',
+      name: 'Test',
+      version: '1.0.0',
+      apiRoutes: [{ method: 'GET', path: '/data', handler }],
+    })
+
+    const { supabase } = createQueuedMockSupabase()
+    supabase.auth.getUser.mockResolvedValue({
+      data: { user: { id: 'user-1', app_metadata: {} } },
+      error: null,
+    })
+    // MFA is required for this user, but only AAL1 has been reached.
+    mockShouldEnforceMfa.mockReturnValue(true)
+    ;(supabase.auth as unknown as { mfa: unknown }).mfa = {
+      getAuthenticatorAssuranceLevel: vi
+        .fn()
+        .mockResolvedValue({ data: { currentLevel: 'aal1', nextLevel: 'aal2' } }),
+    }
+    mockCreateClient.mockResolvedValue(supabase as never)
+
+    const request = createMockRequest('/api/extensions/ext/test-ext/data')
+    const response = await GET(request, createPathParams(['test-ext', 'data']))
+    const { status } = await parseJsonResponse(response)
+
+    expect(status).toBe(403)
+    expect(handler).not.toHaveBeenCalled()
+  })
+
  it('returns 404 for unmatched method/path', async () => {
    extensionRegistry.register({
      id: 'test-ext',
@@ -1,4 +1,4 @@
-import { createClient } from '@/lib/supabase/server'
+import { requireAuth } from '@/lib/auth/require-auth'
 import { NextResponse } from 'next/server'
 import { ensureInitialized } from '@/lib/init'
 import { extensionRegistry } from '@/lib/extensions/registry'
@@ -232,16 +232,16 @@ async function handleRequest(
    return decorateResponse(response, requestId)
  }

-  // Auth check
-  const supabase = await createClient()
-  const { data: { user } } = await supabase.auth.getUser()
-
-  if (!user) {
-    return decorateResponse(
-      NextResponse.json({ error: 'Unauthorized' }, { status: 401 }),
-      requestId,
-    )
+  // Auth check — requireAuth() enforces MFA (AAL2) on hosted, which the previous
+  // inline supabase.auth.getUser() did not. This dispatcher is the single
+  // chokepoint for the entire enabled-extension surface (banking sync, document
+  // upload/booking, supplier-invoice flows, migration), so enforcing MFA here
+  // closes the gap across all of them at once.
+  const auth = await requireAuth()
+  if (auth.error) {
+    return decorateResponse(auth.error, requestId)
  }
+  const { user, supabase } = auth

  // If path params were extracted, create a new Request with them as search params
  let handlerRequest = request
@@ -159,6 +159,32 @@ describe('POST /api/invoices/[id]/mark-paid', () => {
    )
  })

+  it('refuses to mark paid (INVOICE_PAID_BOOK_FAILED) when no payment journal entry is produced', async () => {
+    const customer = makeCustomer()
+    const invoice = makeInvoice({ id: 'inv-1', status: 'sent', total: 12500, customer })
+
+    // Fetch invoice
+    enqueue({ data: invoice, error: null })
+    // Duplicate-payment guard: two ILIKE probes — no candidates
+    enqueue({ data: [], error: null })
+    enqueue({ data: [], error: null })
+    // Company settings
+    enqueue({ data: { accounting_method: 'accrual', entity_type: 'enskild_firma' }, error: null })
+    // Deliberately NO status-update enqueued: the route must fail closed BEFORE
+    // touching the invoice row when nothing was booked.
+
+    // Helper returns null without throwing (e.g. a closed/locked fiscal period).
+    mockCreateInvoicePaymentJournalEntry.mockResolvedValue(null)
+
+    const request = createMockRequest('/api/invoices/inv-1/mark-paid', { method: 'POST' })
+    const response = await POST(request, createMockRouteParams({ id: 'inv-1' }))
+    const { body } = await parseJsonResponse<{ error: { code: string } }>(response)
+
+    expect(mockCreateInvoicePaymentJournalEntry).toHaveBeenCalled()
+    // No silent "paid with no journal entry" — GL must not diverge from the AR ledger.
+    expect(body.error.code).toBe('INVOICE_PAID_BOOK_FAILED')
+  })
+
  it('marks overdue invoice as paid with cash method', async () => {
    const customer = makeCustomer()
    const invoice = makeInvoice({
@@ -6,6 +6,7 @@ import {
 import { createJournalEntry, findFiscalPeriod } from '@/lib/bookkeeping/engine'
 import { resolveInvoicePaymentSourceType } from '@/lib/bookkeeping/propose-payment-lines'
 import { isBookkeepingError } from '@/lib/bookkeeping/errors'
+import { cancelOrphanedPaymentEntry } from '@/lib/bookkeeping/cancel-orphaned-entry'
 import { MarkInvoicePaidSchema } from '@/lib/api/schemas'
 import { ensureInitialized } from '@/lib/init'
 import { withRouteContext } from '@/lib/api/with-route-context'
@@ -204,6 +205,20 @@ export const POST = withRouteContext(
          details: { reason: err instanceof Error ? err.message : 'unknown' },
        })
      }
+
+      // Fail closed: a real invoice must produce a payment voucher. If a helper
+      // returned null without throwing (e.g. a closed/locked fiscal period),
+      // refuse to mark the invoice paid — flipping status with no journal entry
+      // orphans the receivable and diverges the GL from the sub-ledger.
+      if (!journalEntryId) {
+        opLog.error('mark-paid produced no journal entry; refusing to mark paid', undefined, {
+          invoiceId: id,
+        })
+        return errorResponseFromCode('INVOICE_PAID_BOOK_FAILED', opLog, {
+          requestId,
+          details: { reason: 'no_journal_entry_created' },
+        })
+      }
    }

    // CAS guard: only update if status is still in a payable state.
@@ -221,34 +236,31 @@ export const POST = withRouteContext(

    if (updateError) {
      opLog.error('failed to update invoice status', updateError)
+      // The payment voucher already posted but the invoice row did not flip to
+      // paid; cancel the orphan so the GL doesn't diverge from the sub-ledger.
+      if (journalEntryId) {
+        await cancelOrphanedPaymentEntry(
+          supabase,
+          companyId!,
+          user.id,
+          journalEntryId,
+          'Automatiskt makulerad: fakturauppdatering misslyckades efter bokförd betalning',
+        )
+      }
      return errorResponse(updateError, opLog, { requestId })
    }

    if (!updateResult || updateResult.length === 0) {
-      // Status changed between read and write — cancel the orphaned JE and
-      // document the voucher gap before reporting back.
+      // Status changed between read and write (concurrent settle) — cancel the
+      // orphaned payment voucher and document the voucher gap before reporting.
      if (journalEntryId) {
-        const { data: orphan } = await supabase
-          .from('journal_entries')
-          .select('fiscal_period_id, voucher_series, voucher_number')
-          .eq('id', journalEntryId)
-          .single()
-
-        await supabase
-          .from('journal_entries')
-          .update({ status: 'cancelled' })
-          .eq('id', journalEntryId)
-
-        if (orphan) {
-          await supabase.from('voucher_gap_explanations').insert({
-            company_id: companyId,
-            fiscal_period_id: orphan.fiscal_period_id,
-            voucher_series: orphan.voucher_series || 'A',
-            gap_number: orphan.voucher_number,
-            explanation: 'Automatiskt makulerad: dubblettbokning förhindrad av samtidighetsskydd',
-            created_by: user.id,
-          })
-        }
+        await cancelOrphanedPaymentEntry(
+          supabase,
+          companyId!,
+          user.id,
+          journalEntryId,
+          'Automatiskt makulerad: dubblettbokning förhindrad av samtidighetsskydd',
+        )
      }
      return errorResponseFromCode('INVOICE_PAID_RACE', opLog, { requestId })
    }
@@ -207,6 +207,17 @@ export const POST = withRouteContext(
      })
    }

+    // Fail closed: every supplier payment must post a voucher. If a helper
+    // returned null without throwing (e.g. a closed/locked fiscal period), do
+    // NOT flip the invoice — that would diverge the GL from the AP sub-ledger.
+    if (!journalEntryId) {
+      opLog.error('supplier mark-paid produced no journal entry; refusing to mark paid', undefined)
+      return errorResponseFromCode('SI_PAID_FAILED', opLog, {
+        requestId,
+        details: { reason: 'no_journal_entry_created' },
+      })
+    }
+
    const newRemaining = Math.round((invoice.remaining_amount - paymentAmount) * 100) / 100
    const newPaidAmount = Math.round((invoice.paid_amount + paymentAmount) * 100) / 100
    const isFullyPaid = newRemaining <= 0
@@ -228,24 +239,35 @@ export const POST = withRouteContext(

    if (updateError) {
      opLog.error('supplier invoice update failed', updateError)
+      // The payment voucher already posted but the invoice row did not flip;
+      // cancel the orphan so the GL doesn't diverge from the AP sub-ledger.
+      await cancelOrphanedPaymentEntry(
+        supabase, companyId!, user.id, journalEntryId,
+        'Automatiskt makulerad: fakturauppdatering misslyckades efter bokförd betalning',
+      )
      return errorResponse(updateError, opLog, { requestId })
    }

    if (!updateResult || updateResult.length === 0) {
      // CAS guard: another request paid the invoice between our read and write.
      // Cancel the orphaned JE and document the voucher gap.
-      if (journalEntryId) {
-        await cancelOrphanedPaymentEntry(
-          supabase, companyId!, user.id, journalEntryId,
-          'Automatiskt makulerad: dubblettbokning förhindrad av samtidighetsskydd',
-        )
-      }
+      await cancelOrphanedPaymentEntry(
+        supabase, companyId!, user.id, journalEntryId,
+        'Automatiskt makulerad: dubblettbokning förhindrad av samtidighetsskydd',
+      )
      return errorResponseFromCode('SI_PAID_ALREADY', opLog, {
        requestId,
        details: { reason: 'race' },
      })
    }

+    // Record the payment row. payment-sync.ts derives the reversal/recalc amount
+    // from this row (falling back to the FULL paid_amount when the row is
+    // missing), so a missing row would silently desync a later reversal of a
+    // PARTIAL payment. The status flip above already succeeded, so on insert
+    // failure roll the invoice back to its pre-payment state and cancel the
+    // voucher rather than leave it 'paid' with no payment record (the previous
+    // code swallowed this error and left the sub-ledger desynced).
    const { error: paymentError } = await supabase
      .from('supplier_invoice_payments')
      .insert({
@@ -261,7 +283,30 @@ export const POST = withRouteContext(
      })

    if (paymentError) {
-      opLog.warn('failed to record supplier_invoice_payments row', paymentError)
+      opLog.error('failed to record supplier_invoice_payments row — rolling back', paymentError)
+      await supabase
+        .from('supplier_invoices')
+        .update({
+          status: invoice.status,
+          remaining_amount: invoice.remaining_amount,
+          paid_amount: invoice.paid_amount,
+          paid_at: invoice.paid_at ?? null,
+          payment_journal_entry_id:
+            (invoice as { payment_journal_entry_id?: string | null }).payment_journal_entry_id ?? null,
+        })
+        .eq('id', id)
+        .eq('company_id', companyId)
+        // CAS: only undo OUR flip. If a concurrent request already transitioned
+        // the row away from newStatus, don't clobber that legitimate state.
+        .eq('status', newStatus)
+      await cancelOrphanedPaymentEntry(
+        supabase, companyId!, user.id, journalEntryId,
+        'Automatiskt makulerad: betalningspost kunde inte registreras',
+      )
+      return errorResponseFromCode('SI_PAID_FAILED', opLog, {
+        requestId,
+        details: { reason: 'payment_record_insert_failed' },
+      })
    }

    // Under kontantmetoden the cash payment entry is the ONLY booking of the
@@ -38,6 +38,8 @@ import {
  createInvoicePaymentJournalEntry,
 } from '@/lib/bookkeeping/invoice-entries'
 import { createJournalEntry, findFiscalPeriod } from '@/lib/bookkeeping/engine'
+import { AccountsNotInChartError, isBookkeepingError } from '@/lib/bookkeeping/errors'
+import { getErrorMessage } from '@/lib/errors/get-error-message'
 import { eventBus } from '@/lib/events'
 import { findDuplicatePaymentCandidatesForInvoice } from '@/lib/invoices/duplicate-payment-candidates'
 import type { CreateJournalEntryInput, EntityType, Invoice } from '@/types'
@@ -402,21 +404,36 @@ export const POST = withApiV1<{ params: Promise<{ companyId: string; id: string
        }

        if (!journalEntryId) {
-          warnings.push({
-            code: 'JOURNAL_ENTRY_NOT_POSTED',
-            message:
-              'Payment journal entry was not created (likely no open fiscal period). Verify the period and book manually if required.',
+          // Fail closed: a real invoice must produce a posted payment voucher.
+          // A null here (e.g. no open fiscal period) means nothing was booked,
+          // so flipping the invoice to paid/partially_paid would diverge the GL
+          // from the AR sub-ledger. Abort BEFORE the invoice update below —
+          // mirrors the v1 match-invoice strict mode.
+          ctx.log.error('mark-paid: no payment journal entry produced — aborting before state mutation', undefined, {
+            invoiceId,
+            companyId: ctx.companyId,
+          })
+          return v1ErrorResponseFromCode('INVOICE_PAID_BOOK_FAILED', ctx.log, {
+            requestId: ctx.requestId,
+            details: { reason: 'no_journal_entry_created' },
          })
        }
      } catch (err) {
-        ctx.log.error('mark-paid: journal entry creation failed', err as Error, {
+        if (err instanceof AccountsNotInChartError) {
+          return v1ErrorResponse(err, ctx.log, { requestId: ctx.requestId })
+        }
+        ctx.log.error('mark-paid: payment JE creation failed — aborting before state mutation', err as Error, {
          invoiceId,
          companyId: ctx.companyId,
        })
-        warnings.push({
-          code: 'JOURNAL_ENTRY_NOT_POSTED',
-          message:
-            'Payment was recorded but the journal entry posting failed. Check the engine logs; reconcile before period close.',
+        const message = isBookkeepingError(err)
+          ? getErrorMessage(err, { context: 'invoice' })
+          : err instanceof Error
+            ? err.message
+            : 'Unknown error'
+        return v1ErrorResponseFromCode('INVOICE_PAID_BOOK_FAILED', ctx.log, {
+          requestId: ctx.requestId,
+          details: { reason: message },
        })
      }
    }