Reject overpayment on all invoice-match paths (audit C3) (#647)

* fix(invoices): reject overpayment on all invoice-match paths (audit C3) The paid/remaining math was copy-pasted across three sites; the dashboard match-invoice route guarded against overpayment but the v1 public API route and the agent/MCP commitMatchTransactionInvoice had drifted WITHOUT it — silently accepting payment > remaining (recording paid_amount > total, over-crediting AR; cleanup needs storno, not edit). - New lib/invoices/apply-invoice-payment.ts planInvoicePayment(): single source of the paid/remaining/status math + overpayment guard, via canonical roundOre (@/lib/money, guard rail #9). FX-agnostic — caller passes the invoice-currency amount. - All three sites delegate; the guard runs BEFORE journal-entry creation so a rejected match never burns a voucher number. Dashboard behaviour unchanged (faithful extraction — its existing overpayment test still passes, the equivalence anchor). v1 returns MATCH_AMOUNT_EXCEEDS_REMAINING; commit returns the same registry message at 400. - Removes 7 hand-rolled Math.round(x*100)/100 sites; antipattern guard ratchets 668 -> 661. - Unit tests for the helper (overpayment rejection, half-öre tolerance, remaining_amount fallback). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * review: run overpayment guard before the storno (PR #647) greptile: in commit.ts and the v1 route the conflicting-JE storno ran BEFORE the new guard, so a rejected overpayment would still reverse the transaction's prior JE and null its journal_entry_id — a side effect on a rejected match. Move planInvoicePayment above the storno so a rejection leaves the transaction fully untouched. (The dashboard route's pre-existing storno-before-guard ordering is FX-entangled and unchanged here; noted as a follow-up.) 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-03 17:00:22 +02:00
parent 0b86901a2b
commit 5777f51940
6 changed files with 194 additions and 36 deletions
@@ -10,6 +10,7 @@ import { errorResponse, errorResponseFromCode } from '@/lib/errors/get-structure
 import { validateBody } from '@/lib/api/validate'
 import { MatchInvoiceSchema } from '@/lib/api/schemas'
 import { logMatchEvent } from '@/lib/invoices/match-log'
+import { planInvoicePayment } from '@/lib/invoices/apply-invoice-payment'
 import { detectDuplicatePaymentVoucher } from '@/lib/invoices/duplicate-payment-detection'
 import { eventBus } from '@/lib/events/bus'
 import { ensureInitialized } from '@/lib/init'
@@ -296,28 +297,17 @@ export const POST = withRouteContext(
      ? fx.paidInInvoiceCurrency
      : transaction.amount

-    const currentRemaining = invoice.remaining_amount ?? (invoice.total - (invoice.paid_amount || 0))
-
-    // Overshoot guard: the single-tx match endpoint always books tx.amount in
-    // full against the invoice. If tx > remaining the legacy code path would
-    // push invoice.paid_amount past invoice.total — silently. Reject and
-    // point the user at the split-payment flow which can allocate the excess
-    // across additional invoices.
-    if (paidAmountInInvoiceCurrency > currentRemaining + 0.005) {
+    // Overshoot guard + paid/remaining math — shared with the v1 and agent
+    // (commit) paths via planInvoicePayment so they cannot drift again. Runs
+    // before any JE is created, so a doomed match never burns a voucher number.
+    const payment = planInvoicePayment(invoice, paidAmountInInvoiceCurrency)
+    if (!payment.ok) {
      return errorResponseFromCode('MATCH_AMOUNT_EXCEEDS_REMAINING', txLog, {
        requestId,
-        details: {
-          transaction_amount: paidAmountInInvoiceCurrency,
-          remaining_amount: Math.round(currentRemaining * 100) / 100,
-          excess: Math.round((paidAmountInInvoiceCurrency - currentRemaining) * 100) / 100,
-        },
+        details: payment.details,
      })
    }
-
-    const newPaidAmount = Math.round(((invoice.paid_amount || 0) + paidAmountInInvoiceCurrency) * 100) / 100
-    const newRemaining = Math.max(0, Math.round((currentRemaining - paidAmountInInvoiceCurrency) * 100) / 100)
-    const isFullyPaid = newRemaining <= 0
-    const newStatus = isFullyPaid ? 'paid' : 'partially_paid'
+    const { newPaidAmount, newRemaining, isFullyPaid, newStatus } = payment.plan

    const { data: settings } = await supabase
      .from('company_settings')
@@ -30,6 +30,7 @@ import { reverseEntry, createJournalEntry, findFiscalPeriod } from '@/lib/bookke
 import { AccountsNotInChartError, isBookkeepingError } from '@/lib/bookkeeping/errors'
 import { getErrorMessage } from '@/lib/errors/get-error-message'
 import { logMatchEvent } from '@/lib/invoices/match-log'
+import { planInvoicePayment } from '@/lib/invoices/apply-invoice-payment'
 import { detectDuplicatePaymentVoucher } from '@/lib/invoices/duplicate-payment-detection'
 import { eventBus } from '@/lib/events/bus'
 import type { EntityType, Invoice, Transaction } from '@/types'
@@ -265,6 +266,22 @@ export const POST = withApiV1<{ params: Promise<{ companyId: string; id: string
      })
    }

+    const paidAmount = transaction.amount
+
+    // Overshoot guard + paid/remaining math — shared with the dashboard and
+    // agent (commit) paths via planInvoicePayment. Without this, the public API
+    // silently overpaid an invoice (recording paid_amount > total, over-crediting
+    // AR). Runs BEFORE the storno + strict-mode JE creation, so a rejected match
+    // touches no state.
+    const payment = planInvoicePayment(invoice, paidAmount)
+    if (!payment.ok) {
+      return v1ErrorResponseFromCode('MATCH_AMOUNT_EXCEEDS_REMAINING', txLog, {
+        requestId: ctx.requestId,
+        details: payment.details,
+      })
+    }
+    const { newPaidAmount, newRemaining, isFullyPaid, newStatus } = payment.plan
+
    if (transaction.journal_entry_id) {
      try {
        await reverseEntry(ctx.supabase, ctx.companyId!, ctx.userId, transaction.journal_entry_id)
@@ -288,17 +305,6 @@ export const POST = withApiV1<{ params: Promise<{ companyId: string; id: string
    }

    const now = new Date().toISOString()
-    const paidAmount = transaction.amount
-    const newPaidAmount =
-      Math.round(((invoice.paid_amount || 0) + paidAmount) * 100) / 100
-    const currentRemaining =
-      invoice.remaining_amount ?? invoice.total - (invoice.paid_amount || 0)
-    const newRemaining = Math.max(
-      0,
-      Math.round((currentRemaining - paidAmount) * 100) / 100,
-    )
-    const isFullyPaid = newRemaining <= 0
-    const newStatus = isFullyPaid ? 'paid' : 'partially_paid'

    const { data: settings } = await ctx.supabase
      .from('company_settings')
@@ -0,0 +1,67 @@
+import { describe, it, expect } from 'vitest'
+import {
+  planInvoicePayment,
+  PAYMENT_OVERSHOOT_TOLERANCE,
+} from '@/lib/invoices/apply-invoice-payment'
+
+describe('planInvoicePayment', () => {
+  it('marks fully paid on an exact payment', () => {
+    const r = planInvoicePayment({ total: 1000, paid_amount: 0, remaining_amount: 1000 }, 1000)
+    expect(r.ok).toBe(true)
+    if (r.ok) {
+      expect(r.plan).toEqual({
+        newPaidAmount: 1000,
+        newRemaining: 0,
+        isFullyPaid: true,
+        newStatus: 'paid',
+      })
+    }
+  })
+
+  it('marks partially paid on a partial payment', () => {
+    const r = planInvoicePayment({ total: 1000, paid_amount: 0, remaining_amount: 1000 }, 400)
+    expect(r.ok).toBe(true)
+    if (r.ok) {
+      expect(r.plan.newStatus).toBe('partially_paid')
+      expect(r.plan.newPaidAmount).toBe(400)
+      expect(r.plan.newRemaining).toBe(600)
+      expect(r.plan.isFullyPaid).toBe(false)
+    }
+  })
+
+  it('accumulates onto an existing paid_amount', () => {
+    const r = planInvoicePayment({ total: 1000, paid_amount: 600, remaining_amount: 400 }, 400)
+    expect(r.ok).toBe(true)
+    if (r.ok) {
+      expect(r.plan.newPaidAmount).toBe(1000)
+      expect(r.plan.isFullyPaid).toBe(true)
+    }
+  })
+
+  it('REJECTS overpayment (the bug: agent/v1 paths used to swallow it)', () => {
+    const r = planInvoicePayment({ total: 1000, paid_amount: 0, remaining_amount: 1000 }, 1500)
+    expect(r.ok).toBe(false)
+    if (!r.ok) {
+      expect(r.code).toBe('MATCH_AMOUNT_EXCEEDS_REMAINING')
+      expect(r.details).toEqual({
+        transaction_amount: 1500,
+        remaining_amount: 1000,
+        excess: 500,
+      })
+    }
+  })
+
+  it('accepts a sub-öre overshoot (float drift) but rejects a real one-öre over', () => {
+    expect(planInvoicePayment({ total: 1000, remaining_amount: 1000 }, 1000.004).ok).toBe(true)
+    expect(planInvoicePayment({ total: 1000, remaining_amount: 1000 }, 1000.01).ok).toBe(false)
+  })
+
+  it('falls back to total - paid_amount when remaining_amount is absent', () => {
+    expect(planInvoicePayment({ total: 1000, paid_amount: 300 }, 700).ok).toBe(true)
+    expect(planInvoicePayment({ total: 1000, paid_amount: 300 }, 701).ok).toBe(false)
+  })
+
+  it('overshoot tolerance is half an öre', () => {
+    expect(PAYMENT_OVERSHOOT_TOLERANCE).toBe(0.005)
+  })
+})
@@ -0,0 +1,83 @@
+/**
+ * Single source of truth for applying a payment amount to a customer invoice.
+ *
+ * Computes the new paid/remaining/status and — critically — REJECTS overpayment
+ * before the caller creates any journal entry, so a doomed match never burns a
+ * voucher number.
+ *
+ * Background: this math was copy-pasted across three sites — the dashboard
+ * match-invoice route (which had the overpayment guard), the v1 public API
+ * route, and `commitMatchTransactionInvoice` (the agent/MCP path). The latter
+ * two had drifted WITHOUT the guard, so they silently swallowed overpayment via
+ * `Math.max(0, …)` — recording e.g. 1500 paid on a 1000 invoice and
+ * over-crediting accounts receivable. Centralizing the math + guard here closes
+ * that drift; all three sites delegate to `planInvoicePayment`.
+ *
+ * FX: `paymentAmountInInvoiceCurrency` MUST already be in the invoice's
+ * currency. The caller owns any conversion (cross-currency settlement lives in
+ * the dashboard route), keeping this helper FX-agnostic.
+ *
+ * Extracted from the proven dashboard route with the same half-öre overshoot
+ * tolerance. Rounding goes through the canonical `roundOre` (@/lib/money) per
+ * guard rail #9 — identical to the route's previous `Math.round(x*100)/100`
+ * except on exact-half-öre amounts, where `roundOre` rounds correctly.
+ */
+import { roundOre, ORE_TOLERANCE } from '@/lib/money'
+
+/** Half an öre — anything over the remaining by more than this is a real overpayment. */
+export const PAYMENT_OVERSHOOT_TOLERANCE = ORE_TOLERANCE
+
+export interface InvoicePaymentTotals {
+  total: number
+  paid_amount?: number | null
+  remaining_amount?: number | null
+}
+
+export interface InvoicePaymentPlan {
+  newPaidAmount: number
+  newRemaining: number
+  isFullyPaid: boolean
+  newStatus: 'paid' | 'partially_paid'
+}
+
+export type PlanInvoicePaymentResult =
+  | { ok: true; plan: InvoicePaymentPlan }
+  | {
+      ok: false
+      code: 'MATCH_AMOUNT_EXCEEDS_REMAINING'
+      details: { transaction_amount: number; remaining_amount: number; excess: number }
+    }
+
+export function planInvoicePayment(
+  invoice: InvoicePaymentTotals,
+  paymentAmountInInvoiceCurrency: number,
+): PlanInvoicePaymentResult {
+  const currentRemaining =
+    invoice.remaining_amount ?? invoice.total - (invoice.paid_amount || 0)
+
+  if (paymentAmountInInvoiceCurrency > currentRemaining + PAYMENT_OVERSHOOT_TOLERANCE) {
+    return {
+      ok: false,
+      code: 'MATCH_AMOUNT_EXCEEDS_REMAINING',
+      details: {
+        transaction_amount: paymentAmountInInvoiceCurrency,
+        remaining_amount: roundOre(currentRemaining),
+        excess: roundOre(paymentAmountInInvoiceCurrency - currentRemaining),
+      },
+    }
+  }
+
+  const newPaidAmount = roundOre((invoice.paid_amount || 0) + paymentAmountInInvoiceCurrency)
+  const newRemaining = Math.max(0, roundOre(currentRemaining - paymentAmountInInvoiceCurrency))
+  const isFullyPaid = newRemaining <= 0
+
+  return {
+    ok: true,
+    plan: {
+      newPaidAmount,
+      newRemaining,
+      isFullyPaid,
+      newStatus: isFullyPaid ? 'paid' : 'partially_paid',
+    },
+  }
+}
@@ -40,6 +40,7 @@ import {
  createSupplierInvoiceRegistrationEntry,
 } from '@/lib/bookkeeping/supplier-invoice-entries'
 import { linkInvoiceToVoucher } from '@/lib/invoices/voucher-matching'
+import { planInvoicePayment } from '@/lib/invoices/apply-invoice-payment'
 import { linkSupplierInvoiceToVoucher } from '@/lib/invoices/supplier-voucher-matching'
 import { linkTransactionToJournalEntry } from '@/lib/transactions/link-journal-entry'
 import { getErrorEntry } from '@/lib/errors/structured-errors'
@@ -899,18 +900,29 @@ async function commitMatchTransactionInvoice(
    return { error: 'Invoice is not in a matchable state', status: 409 }
  }

+  // Overshoot guard + paid/remaining math — shared with the dashboard and v1
+  // routes via planInvoicePayment. This agent/MCP path previously had NO guard,
+  // so a 1500 payment on a 1000 invoice was silently accepted (paid_amount >
+  // total, AR over-credited). Runs BEFORE the storno + JE below, so a rejected
+  // match leaves the transaction untouched and never burns a voucher number.
+  const paidAmount = transaction.amount
+  const payment = planInvoicePayment(invoice, paidAmount)
+  if (!payment.ok) {
+    return {
+      error:
+        getErrorEntry('MATCH_AMOUNT_EXCEEDS_REMAINING')?.message_sv ??
+        'Transaktionsbeloppet är större än fakturans återstående belopp.',
+      status: 400,
+    }
+  }
+  const { newPaidAmount, newRemaining, isFullyPaid, newStatus } = payment.plan
+
  if (transaction.journal_entry_id) {
    await reverseEntry(supabase, companyId, userId, transaction.journal_entry_id)
    await supabase.from('transactions').update({ journal_entry_id: null }).eq('id', transactionId)
  }

  const now = new Date().toISOString()
-  const paidAmount = transaction.amount
-  const newPaidAmount = Math.round(((invoice.paid_amount || 0) + paidAmount) * 100) / 100
-  const currentRemaining = invoice.remaining_amount ?? (invoice.total - (invoice.paid_amount || 0))
-  const newRemaining = Math.max(0, Math.round((currentRemaining - paidAmount) * 100) / 100)
-  const isFullyPaid = newRemaining <= 0
-  const newStatus = isFullyPaid ? 'paid' : 'partially_paid'

  const { data: settings } = await supabase
    .from('company_settings').select('accounting_method, entity_type').eq('company_id', companyId).single()
@@ -177,6 +177,6 @@
    ]
  },
  "naiveOreRound": {
-    "count": 668
+    "count": 661
  }
 }