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
2026-05-27 17:07:27 +02:00
parent 8979f6eda3
commit 627109b5bd
8 changed files with 146 additions and 33 deletions
@@ -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;
@@ -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';