From 627109b5bd2fe6877672199d3b4c413aaac111df Mon Sep 17 00:00:00 2001
From: Mattsson <111893710+mattssonn@users.noreply.github.com>
Date: Wed, 27 May 2026 17:07:27 +0200
Subject: [PATCH] Mcp/entry bug (#580)

* feat: restore document version supersession on posted entries

* refactor: update VAT handling logic to remove dependency on seller registration status

* feat: update VAT handling to respect seller registration status across invoice processing

* feat: add no-op migration placeholder for document supersession
---
 app/(dashboard)/invoices/page.tsx             |  8 +-
 extensions/general/mcp-server/server.ts       |  4 +-
 lib/invoices/__tests__/vat-rules.test.ts      | 39 ++++----
 lib/invoices/pdf-template.tsx                 |  7 +-
 lib/invoices/vat-rules.ts                     |  7 +-
 lib/pending-operations/commit.ts              |  4 +-
 ...0527122059_allow_document_supersession.sql | 22 +++++
 ...260527160000_fix_supersede_audit_actor.sql | 88 +++++++++++++++++++
 8 files changed, 146 insertions(+), 33 deletions(-)
 create mode 100644 supabase/migrations/20260527122059_allow_document_supersession.sql
 create mode 100644 supabase/migrations/20260527160000_fix_supersede_audit_actor.sql

diff --git a/app/(dashboard)/invoices/page.tsx b/app/(dashboard)/invoices/page.tsx
index 91367891..29a4b259 100644
--- a/app/(dashboard)/invoices/page.tsx
+++ b/app/(dashboard)/invoices/page.tsx
@@ -23,7 +23,7 @@ import { formatCurrency, formatDate } from '@/lib/utils'
 import { cn } from '@/lib/utils'
 import { invoiceNumberDisplay } from '@/lib/invoices/display'
 import { getDisplayTotal } from '@/lib/invoices/rounding'
-import { Plus, Search, Receipt, Lock, Repeat, FileText } from 'lucide-react'
+import { Plus, Search, Receipt, Lock, Repeat } from 'lucide-react'
 import { EmptyInvoices } from '@/components/ui/empty-state'
 import { useCompany } from '@/contexts/CompanyContext'
 import { useCanWrite } from '@/lib/hooks/use-can-write'
@@ -151,12 +151,6 @@ export default function InvoicesPage() {
         title={t('title')}
         action={
           <div className="flex gap-2">
-            <Link href="/invoices/quotes">
-              <Button variant="secondary">
-                <FileText className="mr-2 h-4 w-4" />
-                Offerter
-              </Button>
-            </Link>
             <Link href="/invoices/recurring">
               <Button variant="secondary">
                 <Repeat className="mr-2 h-4 w-4" />
diff --git a/extensions/general/mcp-server/server.ts b/extensions/general/mcp-server/server.ts
index 28917313..c50d185a 100644
--- a/extensions/general/mcp-server/server.ts
+++ b/extensions/general/mcp-server/server.ts
@@ -6070,7 +6070,7 @@ export const tools: McpTool[] = [
         .from('journal_entries')
         .select(
           'id, status, entry_date, description, voucher_number, voucher_series, fiscal_period_id, ' +
-          'fiscal_periods!inner(name, is_closed, locked_at), lines:journal_entry_lines(account_number, debit_amount, credit_amount, line_description)'
+          'fiscal_periods!journal_entries_fiscal_period_id_fkey!inner(name, is_closed, locked_at), lines:journal_entry_lines(account_number, debit_amount, credit_amount, line_description)'
         )
         .eq('id', entryId)
         .eq('company_id', companyId)
@@ -6201,7 +6201,7 @@ export const tools: McpTool[] = [
         .from('journal_entries')
         .select(
           'id, status, entry_date, description, voucher_number, voucher_series, fiscal_period_id, ' +
-          'fiscal_periods!inner(name, is_closed, locked_at), lines:journal_entry_lines(account_number, debit_amount, credit_amount, line_description)'
+          'fiscal_periods!journal_entries_fiscal_period_id_fkey!inner(name, is_closed, locked_at), lines:journal_entry_lines(account_number, debit_amount, credit_amount, line_description)'
         )
         .eq('id', entryId)
         .eq('company_id', companyId)
diff --git a/lib/invoices/__tests__/vat-rules.test.ts b/lib/invoices/__tests__/vat-rules.test.ts
index 88b31c0c..36644e52 100644
--- a/lib/invoices/__tests__/vat-rules.test.ts
+++ b/lib/invoices/__tests__/vat-rules.test.ts
@@ -67,11 +67,13 @@ describe('getAvailableVatRates', () => {
     expect(rates).toHaveLength(4)
   })
 
-  it('collapses to single 0% exempt option when seller is NOT VAT-registered', () => {
-    // ML 1 kap. 1§ — a non-skattskyldig seller may not charge VAT, so the
-    // picker must offer 0% only, regardless of customer type.
-    for (const ct of ['individual', 'swedish_business', 'eu_business', 'non_eu_business'] as const) {
-      const rates = getAvailableVatRates(ct, true, false)
+  it('collapses to 0%/exempt for non-registered sellers regardless of customer type', () => {
+    // ML 1 kap. 1§ — only a skattskyldig may charge output VAT. The picker
+    // must never offer non-zero rates when vat_registered=false; the API
+    // route (route.ts) and preview-pdf route enforce the same gate so a
+    // client bypassing the UI also gets rejected.
+    for (const customerType of ['individual', 'swedish_business', 'eu_business', 'non_eu_business'] as const) {
+      const rates = getAvailableVatRates(customerType, false, false)
       expect(rates).toHaveLength(1)
       expect(rates[0]).toEqual({
         rate: 0,
@@ -81,10 +83,10 @@ describe('getAvailableVatRates', () => {
     }
   })
 
-  it('defaults vatRegistered to true (current behavior preserved)', () => {
-    // Existing callers omit the third arg — they must still see the full
-    // rate set for Swedish customers.
-    const rates = getAvailableVatRates('swedish_business')
+  it('defaults vatRegistered to true (legitimate VAT path)', () => {
+    // Explicit default-arg pin so a future signature change doesn't silently
+    // flip the default and strip VAT from invoices for registered sellers.
+    const rates = getAvailableVatRates('swedish_business', false)
     expect(rates).toHaveLength(4)
   })
 })
@@ -177,11 +179,12 @@ describe('getVatRules', () => {
     })
   })
 
-  it('short-circuits to exempt/0/empty momsRuta when seller is NOT VAT-registered', () => {
-    // ML 1 kap. 1§ — no output VAT, no momsdeklaration row, regardless of
-    // customer type. Verified for all four customer types.
-    for (const ct of ['individual', 'swedish_business', 'eu_business', 'non_eu_business'] as const) {
-      const rules = getVatRules(ct, true, false)
+  it('short-circuits to exempt / rate 0 / empty momsRuta for non-registered sellers', () => {
+    // ML 1 kap. 1§ — non-skattskyldig cannot charge output VAT. momsRuta is
+    // intentionally empty so a downstream momsdeklaration generator never
+    // mis-files a non-registered seller's revenue into ruta 05.
+    for (const customerType of ['individual', 'swedish_business', 'eu_business', 'non_eu_business'] as const) {
+      const rules = getVatRules(customerType, false, false)
       expect(rules).toEqual({
         treatment: 'exempt',
         rate: 0,
@@ -190,12 +193,12 @@ describe('getVatRules', () => {
     }
   })
 
-  it('defaults vatRegistered to true (current behavior preserved)', () => {
-    // Existing callers omit the third arg — they must still see standard_25
-    // for Swedish customers.
-    const rules = getVatRules('swedish_business')
+  it('defaults vatRegistered to true (legitimate VAT path)', () => {
+    // Explicit default-arg pin (see corresponding getAvailableVatRates test).
+    const rules = getVatRules('swedish_business', false)
     expect(rules.rate).toBe(25)
     expect(rules.treatment).toBe('standard_25')
+    expect(rules.momsRuta).toBe('05')
   })
 })
 
diff --git a/lib/invoices/pdf-template.tsx b/lib/invoices/pdf-template.tsx
index b5fb9565..03e072a9 100644
--- a/lib/invoices/pdf-template.tsx
+++ b/lib/invoices/pdf-template.tsx
@@ -873,7 +873,7 @@ export function InvoicePDF({ invoice, customer, items, company, originalInvoiceN
               // charge output VAT, so a "Moms 0%" line would imply VAT
               // accounting that doesn't exist. The notice block below the
               // payment section explains the absence of VAT.
-              !(company.vat_registered === false && invoice.vat_amount === 0) && (
+              company.vat_registered !== false && (
                 <View style={styles.totalRow}>
                   <Text style={styles.totalLabel}>{L.vatRow(invoice.vat_rate ?? (vatByRate.size === 1 ? (vatByRate.keys().next().value ?? 0) : 0))}</Text>
                   <Text style={styles.totalValue}>{formatCurrency(invoice.vat_amount, invoice.currency, lang)}</Text>
@@ -1069,7 +1069,10 @@ export function InvoicePDF({ invoice, customer, items, company, originalInvoiceN
             don't apply, and a single dedicated notice is clearer for the
             customer than reusing the exempt notice (which implies the sale
             specifically is exempt while the seller is otherwise within the
-            VAT system). */}
+            VAT system). Server-side enforcement (api/invoices/route.ts +
+            preview-pdf/route.ts) coerces vat_amount to 0 for non-registered
+            sellers, so this branch always lines up with what's in the
+            totals block. */}
         {company.vat_registered === false ? (
           <View style={styles.reverseChargeBox}>
             <Text style={styles.reverseChargeText}>{L.notVatRegisteredNotice}</Text>
diff --git a/lib/invoices/vat-rules.ts b/lib/invoices/vat-rules.ts
index 97531fb3..7a3d44f9 100644
--- a/lib/invoices/vat-rules.ts
+++ b/lib/invoices/vat-rules.ts
@@ -15,7 +15,9 @@ export interface VatRateOption {
  * When the seller is not VAT-registered (`vatRegistered=false`), every customer
  * type collapses to a single 0% / exempt option. ML 1 kap. 1§ — only a
  * skattskyldig person may charge VAT, so the picker must never offer non-zero
- * rates in that mode.
+ * rates in that mode. ML 16 kap. 23 § (faktureringsmoms) imposes liability for
+ * VAT erroneously stated on a document, but does NOT grant the right to charge
+ * it — so we block at source rather than allow + warn.
  */
 export function getAvailableVatRates(
   customerType: CustomerType,
@@ -82,7 +84,8 @@ export interface VatRule {
  * When the seller is not VAT-registered (`vatRegistered=false`), the rules
  * short-circuit to `{ treatment: 'exempt', rate: 0, momsRuta: '' }` regardless
  * of customer type — ML 1 kap. 1§ bars a non-skattskyldig from charging output
- * VAT. `momsRuta` is empty so the invoice doesn't claim a momsdeklaration row.
+ * VAT. `momsRuta` is empty so a downstream momsdeklaration generator never
+ * mis-files a non-registered seller's "sales" into ruta 05.
  */
 export function getVatRules(
   customerType: CustomerType,
diff --git a/lib/pending-operations/commit.ts b/lib/pending-operations/commit.ts
index f34279f1..1c690a91 100644
--- a/lib/pending-operations/commit.ts
+++ b/lib/pending-operations/commit.ts
@@ -2198,7 +2198,7 @@ async function commitCorrectEntry(
   // commit-time gate matches the staging-time signal.
   const { data: original, error: origErr } = await supabase
     .from('journal_entries')
-    .select('id, status, entry_date, fiscal_period_id, fiscal_periods!inner(is_closed, locked_at)')
+    .select('id, status, entry_date, fiscal_period_id, fiscal_periods!journal_entries_fiscal_period_id_fkey!inner(is_closed, locked_at)')
     .eq('id', entryId)
     .eq('company_id', companyId)
     .maybeSingle()
@@ -2281,7 +2281,7 @@ async function commitReverseEntry(
   // via resolvePeriodStatusForDate, matching the staging-time signal.
   const { data: original, error: origErr } = await supabase
     .from('journal_entries')
-    .select('id, status, entry_date, fiscal_period_id, fiscal_periods!inner(is_closed, locked_at)')
+    .select('id, status, entry_date, fiscal_period_id, fiscal_periods!journal_entries_fiscal_period_id_fkey!inner(is_closed, locked_at)')
     .eq('id', entryId)
     .eq('company_id', companyId)
     .maybeSingle()
diff --git a/supabase/migrations/20260527122059_allow_document_supersession.sql b/supabase/migrations/20260527122059_allow_document_supersession.sql
new file mode 100644
index 00000000..86e970fe
--- /dev/null
+++ b/supabase/migrations/20260527122059_allow_document_supersession.sql
@@ -0,0 +1,22 @@
+-- No-op placeholder.
+--
+-- Background: this version was originally created on the mcp/entry-bug branch
+-- and was applied to the Supabase preview branch before review. Reviewers
+-- flagged it as a duplicate of 20260527130000_allow_document_supersession.sql
+-- (already in main, byte-identical CREATE OR REPLACE bodies). Because 122059
+-- < 130000, both files ran but the 130000 file always wrote the final state
+-- — the 122059 run was wasted work, not a correctness problem.
+--
+-- We can't delete this file because Supabase's schema_migrations table on the
+-- preview branch already tracks 20260527122059 as applied; removing the file
+-- triggers "Remote migration versions not found in local migrations directory"
+-- on the next `supabase db push`. Repairing the migration record would also
+-- work but is heavier than keeping the file as a documented no-op.
+--
+-- The actual trigger + RPC bodies live in:
+--   20260527130000_allow_document_supersession.sql  (canonical definitions)
+--   20260527160000_fix_supersede_audit_actor.sql    (actor_id attribution fix)
+--
+-- This file intentionally contains no DDL.
+
+SELECT 1;
diff --git a/supabase/migrations/20260527160000_fix_supersede_audit_actor.sql b/supabase/migrations/20260527160000_fix_supersede_audit_actor.sql
new file mode 100644
index 00000000..49675259
--- /dev/null
+++ b/supabase/migrations/20260527160000_fix_supersede_audit_actor.sql
@@ -0,0 +1,88 @@
+-- Fix actor attribution on enforce_document_metadata_immutability blocked-write audits.
+--
+-- The trigger introduced in 20260527130000 records SECURITY_EVENT rows when a
+-- caller attempts a forbidden mutation on a posted document. Both inserts
+-- assigned the existing OLD.user_id (the document's owner) to audit_log.user_id
+-- and left actor_id NULL. That means the trail names the victim — not the
+-- attacker — as the actor for any blocked tampering attempt, which is the
+-- opposite of what an incident reviewer needs.
+--
+-- Fix: capture auth.uid() into a local at the top of the trigger and write it
+-- to actor_id on every SECURITY_EVENT insert. We keep user_id = OLD.user_id
+-- (document owner) so the affected resource is still discoverable by owner,
+-- while actor_id identifies the session that triggered the blocked write.
+-- auth.uid() can be NULL in service-role / cron contexts, so the column is
+-- nullable already and a NULL actor_id correctly signals "non-user origin".
+
+CREATE OR REPLACE FUNCTION public.enforce_document_metadata_immutability()
+ RETURNS trigger
+ LANGUAGE plpgsql
+ SECURITY DEFINER
+ SET search_path TO 'public'
+AS $function$
+DECLARE
+  v_entry_status text;
+  v_allow_supersede boolean;
+  v_actor uuid := auth.uid();
+BEGIN
+  IF current_setting('gnubok.allow_delete', true) = 'true' THEN
+    RETURN NEW;
+  END IF;
+
+  v_allow_supersede := current_setting('gnubok.allow_supersede', true) = 'true';
+
+  IF OLD.journal_entry_id IS NULL THEN
+    RETURN NEW;
+  END IF;
+
+  SELECT status INTO v_entry_status
+  FROM public.journal_entries
+  WHERE id = OLD.journal_entry_id;
+
+  IF v_entry_status IS NULL OR v_entry_status NOT IN ('posted', 'reversed') THEN
+    RETURN NEW;
+  END IF;
+
+  -- Even with allow_supersede, every field other than is_current_version and
+  -- superseded_by_id remains immutable. The bypass is intentionally narrow
+  -- so that a session that obtains the GUC (e.g. via SQL injection) cannot
+  -- mutate journal_entry_id, sha256_hash, storage_path, or any other field
+  -- the BFL 7 kap audit trail depends on.
+  IF NEW.file_name              IS DISTINCT FROM OLD.file_name
+     OR NEW.storage_path        IS DISTINCT FROM OLD.storage_path
+     OR NEW.file_size_bytes     IS DISTINCT FROM OLD.file_size_bytes
+     OR NEW.mime_type           IS DISTINCT FROM OLD.mime_type
+     OR NEW.sha256_hash         IS DISTINCT FROM OLD.sha256_hash
+     OR NEW.upload_source       IS DISTINCT FROM OLD.upload_source
+     OR NEW.digitization_date   IS DISTINCT FROM OLD.digitization_date
+     OR NEW.uploaded_by         IS DISTINCT FROM OLD.uploaded_by
+     OR NEW.version             IS DISTINCT FROM OLD.version
+     OR NEW.original_id         IS DISTINCT FROM OLD.original_id
+     OR NEW.journal_entry_id    IS DISTINCT FROM OLD.journal_entry_id
+     OR NEW.journal_entry_line_id IS DISTINCT FROM OLD.journal_entry_line_id
+  THEN
+    INSERT INTO public.audit_log (user_id, company_id, action, table_name, record_id, actor_id, description)
+    VALUES (OLD.user_id, OLD.company_id, 'SECURITY_EVENT', 'document_attachments', OLD.id, v_actor,
+      'Blocked metadata or link modification of document linked to ' || v_entry_status || ' entry ' || OLD.journal_entry_id);
+
+    RAISE EXCEPTION 'Cannot modify metadata or journal entry link of document linked to a % journal entry (BFL 7 kap)', v_entry_status;
+  END IF;
+
+  -- is_current_version and superseded_by_id may only be changed under the
+  -- supersede GUC. Without it, those flips are also blocked.
+  IF NOT v_allow_supersede
+     AND (NEW.is_current_version IS DISTINCT FROM OLD.is_current_version
+          OR NEW.superseded_by_id IS DISTINCT FROM OLD.superseded_by_id)
+  THEN
+    INSERT INTO public.audit_log (user_id, company_id, action, table_name, record_id, actor_id, description)
+    VALUES (OLD.user_id, OLD.company_id, 'SECURITY_EVENT', 'document_attachments', OLD.id, v_actor,
+      'Blocked is_current_version/superseded_by_id flip without supersede GUC on document linked to ' || v_entry_status || ' entry ' || OLD.journal_entry_id);
+
+    RAISE EXCEPTION 'Cannot modify is_current_version of document linked to a % journal entry without supersede GUC (BFL 7 kap)', v_entry_status;
+  END IF;
+
+  RETURN NEW;
+END;
+$function$;
+
+NOTIFY pgrst, 'reload schema';