fix(db): check owner of provisional game in admin_resolve_report, harden report tests (KAT-05 2c review) Refs #2
All checks were successful
CI / flutter (pull_request) Successful in 1m18s
CI / web-build (pull_request) Successful in 57s
CI / supabase (pull_request) Successful in 9s
CI / supabase-tests (pull_request) Successful in 33s
CI / e2e (pull_request) Successful in 2m52s

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
cschlaefke 2026-11-03 21:23:13 +01:00
parent e665a71f13
commit 119267bb26
4 changed files with 45 additions and 17 deletions

View file

@ -453,9 +453,11 @@ where r.id = '7c1d2c3d-0000-4000-8000-000000000001';
| `report_status_invalid` | Der Status ist nicht `approved`, `merged` oder `rejected` | | `report_status_invalid` | Der Status ist nicht `approved`, `merged` oder `rejected` |
| `report_target_required` | `approved` und `merged` brauchen ein Ziel | | `report_target_required` | `approved` und `merged` brauchen ein Ziel |
| `report_target_not_listed` | Das Ziel ist kein sichtbares Katalogspiel (privat, ausgeblendet, zusammengeführt oder unbekannt); das endgültige Ziel wählen oder zuerst wieder einblenden (siehe 4) | | `report_target_not_listed` | Das Ziel ist kein sichtbares Katalogspiel (privat, ausgeblendet, zusammengeführt oder unbekannt); das endgültige Ziel wählen oder zuerst wieder einblenden (siehe 4) |
| `report_provisional_not_private` | Der vorläufige Eintrag der Meldung ist kein privates Spiel (Datenfehler); nicht umhängen, sondern die Meldung prüfen | | `report_provisional_not_private` | Der vorläufige Eintrag der Meldung ist kein privates Spiel **des Meldenden** (Datenfehler, z. B. ein Katalogspiel oder das Spiel eines anderen Nutzers); nicht umhängen, sondern die Meldung prüfen |
Ein Aufruf über die App-Schnittstelle (als `anon` oder `authenticated`, auch mit Admin-Claims) scheitert mit `42501`; die Funktion ist nur für die Datenbankrolle ausführbar (E10). Meldungen anlegen, ändern oder löschen können Nutzer in dieser Etappe nicht (das Anlegen kommt mit KAT-04); sie lesen nur ihre eigenen. Das Löschen eines Zielspiels, auf das eine Meldung verweist, schlägt mit `23503` fehl (gewollt: ausblenden statt löschen). **Falsch gesetzte `visibility` oder `owner_id`** lassen sich nicht mit `update` korrigieren (Trigger `games_visibility_immutable`, Fehler `visibility_immutable`). Korrigieren geht nur durch Löschen und Neuanlegen des Spiels oder, bewusst und nur als Tabelleneigentümer in einer Transaktion, mit `alter table public.games disable trigger games_visibility_immutable;` … `enable trigger …`. Für Clients: Der Fehler `P0001` kommt über PostgREST als HTTP 400 an (Hinweis für die Fehlerabbildung in #3 und #13).
Ein Aufruf über die App-Schnittstelle (als `anon` oder `authenticated`, auch mit Admin-Claims) scheitert mit `42501`; die Funktion ist nur für die Datenbankrolle ausführbar (E10). Hinweis: Die Rolle `service_role` hat (Supabase-Standard) volle Tabellenrechte und umgeht RLS; die Zugriffsgrenzen in Krit. 9 gelten für die API-Rollen `anon` und `authenticated`, den Funktionen ist `service_role` ausdrücklich entzogen. Meldungen anlegen, ändern oder löschen können Nutzer in dieser Etappe nicht (das Anlegen kommt mit KAT-04); sie lesen nur ihre eigenen. Das Löschen eines Zielspiels, auf das eine Meldung verweist, schlägt mit `23503` fehl (gewollt: ausblenden statt löschen).
## 7. Admin-Rolle in der App ## 7. Admin-Rolle in der App

View file

@ -723,7 +723,7 @@ Umgesetzt: B5 (Migration `20261103193928_catalog_merge.sql`), B6 (pgTAP `07_cata
### 2026-11-03 – Umsetzung PR 2c (backend-dev) ### 2026-11-03 – Umsetzung PR 2c (backend-dev)
Umgesetzt: B8 (Migration `20261103195151_game_reports.sql`), B9 (pgTAP `08_game_reports.test.sql`, 153 Tests; `07` und `00` angepasst), B10 (Runbook Abschnitt 6). Abweichungen und Ergänzungen, jeweils zur Prüfung im PR: Umgesetzt: B8 (Migration `20261103195151_game_reports.sql`), B9 (pgTAP `08_game_reports.test.sql`, 157 Tests; `07` und `00` angepasst), B10 (Runbook Abschnitt 6). Abweichungen und Ergänzungen, jeweils zur Prüfung im PR:
- **Branch:** `feature/2c-katalog-meldungen` statt `feature/2-katalog-meldungen` (Vorgabe des Auftrags), gestapelt auf `feature/2b-katalog-zusammenfuehren`. - **Branch:** `feature/2c-katalog-meldungen` statt `feature/2-katalog-meldungen` (Vorgabe des Auftrags), gestapelt auf `feature/2b-katalog-zusammenfuehren`.
- **Zusätzliche Indizes** auf beide Fremdschlüssel nach `games` (Regel: Index auf jeden Fremdschlüssel). `provisional_game_id` ist **eindeutig** (`where provisional_game_id is not null`, 1:1: ein vorläufiger Eintrag gehört zu höchstens einer Meldung), `target_game_id` ein einfacher Index. - **Zusätzliche Indizes** auf beide Fremdschlüssel nach `games` (Regel: Index auf jeden Fremdschlüssel). `provisional_game_id` ist **eindeutig** (`where provisional_game_id is not null`, 1:1: ein vorläufiger Eintrag gehört zu höchstens einer Meldung), `target_game_id` ein einfacher Index.
@ -734,7 +734,8 @@ Umgesetzt: B8 (Migration `20261103195151_game_reports.sql`), B9 (pgTAP `08_game_
- **Ziel wird auch bei `rejected` geprüft** (`report_target_not_listed`), wenn eines angegeben ist. Reihenfolge der Prüfungen wie in 3.4; `p_report = null` ist `report_not_found`, `p_status = null` ist `report_status_invalid`. - **Ziel wird auch bei `rejected` geprüft** (`report_target_not_listed`), wenn eines angegeben ist. Reihenfolge der Prüfungen wie in 3.4; `p_report = null` ist `report_not_found`, `p_status = null` ist `report_status_invalid`.
- **Trigger `games_visibility_immutable`** läuft vor der RLS-Prüfung: Ein Nutzer, der `visibility` ändert, bekommt `P0001 visibility_immutable` statt `42501` (API-Änderung für Clients; bestehende Tests in `02` bleiben grün, weil dort `owner_id` ohne Spaltenrecht vorher mit 42501 scheitert). Es gibt weiterhin keine generierten Datenbanktypen im Repo. - **Trigger `games_visibility_immutable`** läuft vor der RLS-Prüfung: Ein Nutzer, der `visibility` ändert, bekommt `P0001 visibility_immutable` statt `42501` (API-Änderung für Clients; bestehende Tests in `02` bleiben grün, weil dort `owner_id` ohne Spaltenrecht vorher mit 42501 scheitert). Es gibt weiterhin keine generierten Datenbanktypen im Repo.
- **RLS:** zwei Lese-Policies (eigene Zeilen; `is_admin()` für R4). Bewusst kein Test mit `is_admin()` als Zugang in R1, aber `08` dokumentiert, dass `app_metadata.role = 'admin'` lesen, nicht schreiben kann, und `user_metadata` nichts öffnet. - **RLS:** zwei Lese-Policies (eigene Zeilen; `is_admin()` für R4). Bewusst kein Test mit `is_admin()` als Zugang in R1, aber `08` dokumentiert, dass `app_metadata.role = 'admin'` lesen, nicht schreiben kann, und `user_metadata` nichts öffnet.
- **Tests stabil gegenüber #13:** `08` prüft Schreibrechte je Spalte (`has_column_privilege` auf `status`, `target_game_id`, `resolved_at`, `origin`, `owner_id`, `bgg_id`, `created_at`), „Delete verboten“ und „fremde Meldung nicht sichtbar/änderbar“, nicht „kein Insert/Update auf der Tabelle“. - **Tests und #13:** `08` prüft Schreibrechte je Spalte: beim Anlegen sind `status`, `target_game_id`, `resolved_at`, `origin`, `created_at`, `updated_at` nicht setzbar (nicht pauschal „kein Insert“), ein Update-Recht gibt es auf keine Spalte außer (ab #13) `result_seen_at`. Dazu „Delete verboten“ und „fremde Meldung nicht sichtbar/änderbar“. Einzelne Insert-/Update-Assertions im Abschnitt „Lesen, RLS“ (z. B. Anna legt eine Meldung für Ben an) gehen von fehlenden Insert-Rechten aus; 13a muss diese anpassen, `08` ändert sich mit #13 ohnehin.
- **Review 2c:** `admin_resolve_report` prüft zusätzlich, dass der vorläufige Eintrag existiert und dem Meldenden gehört (`report_provisional_not_private`, deckt Katalogspiel, fremdes Spiel und fehlende Zeile ab); Test mit fremdem Spiel in `08`. Hinweise: `service_role` hat Supabase-Standardrechte auf Tabellen (Krit. 9 gilt für `anon`/`authenticated`); `P0001` → HTTP 400 in PostgREST; **der Fremdschlüssel-Wächter existiert in `07` und `08`: #3 (`game_alt_titles.game_id`) und jede weitere Tabelle mit Verweis auf `games` muss beide anpassen**. Optionaler Merkpunkt (nicht gebaut): ein Check-Trigger, dass `target_game_id` auf ein Katalogspiel zeigt (analog `games_merged_into_catalog`); heute stellt das nur `admin_resolve_report` sicher.
- **Bewusst nicht gebaut (offene Entscheidung Chris, Spec #13):** `game_reports.result_seen_at` und `editions_cleared`. #13 ergänzt bei Freigabe das Spaltenrecht `update (result_seen_at)` und ggf. `editions_cleared` sowie Insert-Recht und -Policy; die Tests in `08` sind darauf vorbereitet. - **Bewusst nicht gebaut (offene Entscheidung Chris, Spec #13):** `game_reports.result_seen_at` und `editions_cleared`. #13 ergänzt bei Freigabe das Spaltenrecht `update (result_seen_at)` und ggf. `editions_cleared` sowie Insert-Recht und -Policy; die Tests in `08` sind darauf vorbereitet.
- **Fremdschlüssel-Wächter:** erweiterte Liste in `07` und in `08` (`game_reports.target_game_id` behandelt, `game_reports.provisional_game_id` bewusst ausgelassen). `00_schema.test.sql`: 21 Tabellen. - **Fremdschlüssel-Wächter:** erweiterte Liste in `07` und in `08` (`game_reports.target_game_id` behandelt, `game_reports.provisional_game_id` bewusst ausgelassen). `00_schema.test.sql`: 21 Tabellen.
- **Runbook Abschnitt 6:** Alle SQL-Bausteine laufen wörtlich in `08` (einmalige Prüfung per Skript, keine CI-Prüfung). Der Trockenlauf-Baustein aus 2b gilt entsprechend (`begin; …; rollback;` in einem Lauf). - **Runbook Abschnitt 6:** Alle SQL-Bausteine laufen wörtlich in `08` (einmalige Prüfung per Skript, keine CI-Prüfung). Der Trockenlauf-Baustein aus 2b gilt entsprechend (`begin; …; rollback;` in einem Lauf).

View file

@ -133,9 +133,11 @@ begin
if p_status in ('approved', 'merged') and rep.provisional_game_id is not null then if p_status in ('approved', 'merged') and rep.provisional_game_id is not null then
select * into prov from public.games g where g.id = rep.provisional_game_id; select * into prov from public.games g where g.id = rep.provisional_game_id;
-- Zusatzprüfung (nicht in der Spec): Nur ein privater Eintrag darf als vorläufiger Eintrag -- Zusatzprüfung (nicht in der Spec): Nur ein privater Eintrag des Meldenden darf umgehängt
-- umgehängt werden; ein Katalogspiel würde sonst über den Variantenfall "Katalog" kopiert. -- werden. Ein Katalogspiel würde sonst über den Variantenfall "Katalog" kopiert, und ein
if prov.visibility <> 'private' then -- fremdes privates Spiel (Datenfehler im Studio) würde Daten eines anderen Nutzers umhängen.
-- "not found" deckt eine fehlende Zeile ab (null <> 'private' wäre sonst nicht wahr).
if not found or prov.visibility <> 'private' or prov.owner_id is distinct from rep.owner_id then
raise exception 'report_provisional_not_private' using errcode = 'P0001'; raise exception 'report_provisional_not_private' using errcode = 'P0001';
end if; end if;
-- privat -> Katalog: nichts wird kopiert (RB1, Klärung N15) -- privat -> Katalog: nichts wird kopiert (RB1, Klärung N15)

View file

@ -5,7 +5,7 @@
-- Läuft auch mit eingespieltem Startbestand: eigene Fixtures mit festen IDs (Präfix 7c1d/f8), gezählt wird nur darüber. -- Läuft auch mit eingespieltem Startbestand: eigene Fixtures mit festen IDs (Präfix 7c1d/f8), gezählt wird nur darüber.
-- Regel dieser Datei: Jede Verweigerung steht neben einem Positivfall, sonst würde "alles verweigern" bestehen. -- Regel dieser Datei: Jede Verweigerung steht neben einem Positivfall, sonst würde "alles verweigern" bestehen.
begin; begin;
select plan(153); select plan(157);
insert into auth.users (id, email, raw_user_meta_data) values insert into auth.users (id, email, raw_user_meta_data) values
('11111111-1111-1111-1111-111111111111', 'anna@example.com', '{"age_confirmed":"true","terms_accepted":"true","terms_version":"test","display_name":"Anna"}'), ('11111111-1111-1111-1111-111111111111', 'anna@example.com', '{"age_confirmed":"true","terms_accepted":"true","terms_version":"test","display_name":"Anna"}'),
@ -19,12 +19,17 @@ select ok((select relrowsecurity from pg_class where oid = 'public.game_reports'
'Sicherheit Kriterium 1: game_reports hat Row Level Security'); 'Sicherheit Kriterium 1: game_reports hat Row Level Security');
select ok(has_table_privilege('authenticated', 'public.game_reports', 'select'), select ok(has_table_privilege('authenticated', 'public.game_reports', 'select'),
'KAT-05 Kriterium 5: authenticated darf game_reports lesen (eigene Zeilen, Policy)'); 'KAT-05 Kriterium 5: authenticated darf game_reports lesen (eigene Zeilen, Policy)');
-- Formuliert stabil gegenüber späteren Spaltenrechten (#13 ergänzt Insert-Recht und update (result_seen_at)): -- Formuliert stabil gegenüber späteren Spaltenrechten (#13 ergänzt Insert-Recht, u. a. auf owner_id und
-- geprüft wird je Spalte, nicht "kein Insert/Update auf der Tabelle". -- provisional_game_id, und update (result_seen_at)): geprüft wird je Spalte.
select is_empty($$select c, p from unnest(array['status', 'target_game_id', 'resolved_at', 'origin', 'owner_id', 'bgg_id', 'created_at']) c, -- Insert: nur Spalten, die der Nutzer nie setzen darf (Ergebnis und Herkunft der Meldung).
unnest(array['insert', 'update']) p select is_empty($$select c from unnest(array['status', 'target_game_id', 'resolved_at', 'origin', 'created_at', 'updated_at']) c
where has_column_privilege('authenticated', 'public.game_reports', c, p)$$, where has_column_privilege('authenticated', 'public.game_reports', c, 'insert')$$,
'KAT-05 Kriterium 9: authenticated hat kein Schreibrecht auf status, target_game_id, resolved_at, origin, owner_id, bgg_id, created_at'); 'KAT-05 Kriterium 9: authenticated kann beim Anlegen status, target_game_id, resolved_at, origin, created_at, updated_at nicht setzen');
-- Update: auf keine Spalte außer (ab #13) result_seen_at
select is_empty($$select a.attname from pg_attribute a
where a.attrelid = 'public.game_reports'::regclass and a.attnum > 0 and not a.attisdropped and a.attname <> 'result_seen_at'
and has_column_privilege('authenticated', 'public.game_reports', a.attname, 'update')$$,
'KAT-05 Kriterium 9: authenticated hat auf keine Spalte von game_reports ein Update-Recht (ausgenommen ab #13 result_seen_at)');
select is_empty($$select c, p from (select attname as c from pg_attribute where attrelid = 'public.game_reports'::regclass and attnum > 0 and not attisdropped) a, select is_empty($$select c, p from (select attname as c from pg_attribute where attrelid = 'public.game_reports'::regclass and attnum > 0 and not attisdropped) a,
unnest(array['insert', 'update']) p unnest(array['insert', 'update']) p
where has_column_privilege('anon', 'public.game_reports', c, p)$$, where has_column_privilege('anon', 'public.game_reports', c, p)$$,
@ -348,7 +353,7 @@ insert into public.games (id, title, visibility, owner_id) values
create temp table rb_before as create temp table rb_before as
select (select count(*) from public.game_reports where status <> 'open') as erledigt, select (select count(*) from public.game_reports where status <> 'open') as erledigt,
(select count(*) from public.games where hidden_at is not null) as ausgeblendet, (select count(*) from public.games where hidden_at is not null) as ausgeblendet,
(select string_agg(id::text || target_game_id::text || status, ',' order by id) from public.game_reports where id::text like '7c1d2c3d%') as meldungen; (select string_agg(id::text || coalesce(target_game_id::text, '-') || status, ',' order by id) from public.game_reports where id::text like '7c1d2c3d%') as meldungen;
select throws_ok($$select public.admin_resolve_report(gen_random_uuid(), 'rejected')$$, 'P0001', 'report_not_found', select throws_ok($$select public.admin_resolve_report(gen_random_uuid(), 'rejected')$$, 'P0001', 'report_not_found',
'KAT-05 Kriterium 5: unbekannte Meldung (report_not_found)'); 'KAT-05 Kriterium 5: unbekannte Meldung (report_not_found)');
@ -379,6 +384,24 @@ select throws_ok($$select public.admin_resolve_report('7c1d2c3d-0000-4000-8000-0
select throws_ok($$select public.admin_resolve_report('7c1d2c3d-0000-4000-8000-00000000000b', 'rejected', 'f8000000-0000-0000-0000-000000000d01')$$, select throws_ok($$select public.admin_resolve_report('7c1d2c3d-0000-4000-8000-00000000000b', 'rejected', 'f8000000-0000-0000-0000-000000000d01')$$,
'P0001', 'report_target_not_listed', 'KAT-05 Kriterium 5: auch bei rejected wird das Ziel geprüft'); 'P0001', 'report_target_not_listed', 'KAT-05 Kriterium 5: auch bei rejected wird das Ziel geprüft');
-- Meldung MIT vorläufigem Eintrag und ungültigem Ziel: die Zielprüfung der Funktion greift zuerst
-- (sonst käme repoint_target_not_listed aus der internen Funktion)
select throws_ok($$select public.admin_resolve_report('7c1d2c3d-0000-4000-8000-000000000001', 'approved', 'f8000000-0000-0000-0000-000000000d01')$$,
'P0001', 'report_target_not_listed', 'KAT-05 Kriterium 5: Meldung mit vorläufigem Eintrag und ausgeblendetem Ziel (report_target_not_listed)');
-- Datenfehler: der vorläufige Eintrag gehört einem anderen Nutzer (Zusatzprüfung gegenüber der Spec)
insert into public.games (id, title, visibility, owner_id) values
('f8000000-0000-0000-0000-000000000e11', 'Annas fremdes Spiel', 'private', '11111111-1111-1111-1111-111111111111');
insert into public.collection_items (id, owner_id, game_id, list) values
('f8000000-0000-0000-0000-000000000e12', '11111111-1111-1111-1111-111111111111', 'f8000000-0000-0000-0000-000000000e11', 'owned');
insert into public.game_reports (id, owner_id, title, provisional_game_id) values
('f8000000-0000-0000-0000-000000000e02', '22222222-2222-2222-2222-222222222222', 'Bens Meldung auf Annas Spiel', 'f8000000-0000-0000-0000-000000000e11');
select throws_ok($$select public.admin_resolve_report('f8000000-0000-0000-0000-000000000e02', 'merged', '7c1d2c3d-0000-4000-8000-000000000004')$$,
'P0001', 'report_provisional_not_private', 'Zusatzprüfung: ein privates Spiel eines anderen Nutzers wird nicht umgehängt');
select is((select game_id::text || ':' || (select hidden_at is null and merged_into_id is null from public.games where id = 'f8000000-0000-0000-0000-000000000e11')::text
from public.collection_items where id = 'f8000000-0000-0000-0000-000000000e12'),
'f8000000-0000-0000-0000-000000000e11:true', 'Annas Daten und ihr Spiel bleiben unverändert');
-- Datenfehler: vorläufiger Eintrag ist ein Katalogspiel (Zusatzprüfung gegenüber der Spec) -- Datenfehler: vorläufiger Eintrag ist ein Katalogspiel (Zusatzprüfung gegenüber der Spec)
insert into public.game_reports (id, owner_id, title, provisional_game_id) values insert into public.game_reports (id, owner_id, title, provisional_game_id) values
('f8000000-0000-0000-0000-000000000e01', '22222222-2222-2222-2222-222222222222', 'Vorläufig ist Katalog', 'f8000000-0000-0000-0000-00000000000e'); ('f8000000-0000-0000-0000-000000000e01', '22222222-2222-2222-2222-222222222222', 'Vorläufig ist Katalog', 'f8000000-0000-0000-0000-00000000000e');
@ -391,7 +414,7 @@ select is((select hidden_at is null and merged_into_id is null from public.games
select is((select row(erledigt, ausgeblendet, meldungen)::text from rb_before), select is((select row(erledigt, ausgeblendet, meldungen)::text from rb_before),
(select row((select count(*) from public.game_reports where status <> 'open'), (select row((select count(*) from public.game_reports where status <> 'open'),
(select count(*) from public.games where hidden_at is not null), (select count(*) from public.games where hidden_at is not null),
(select string_agg(id::text || target_game_id::text || status, ',' order by id) from public.game_reports where id::text like '7c1d2c3d%'))::text), (select string_agg(id::text || coalesce(target_game_id::text, '-') || status, ',' order by id) from public.game_reports where id::text like '7c1d2c3d%'))::text),
'Nach allen Fehlerfällen ist nichts verändert (Meldungen, Ausblendungen)'); 'Nach allen Fehlerfällen ist nichts verändert (Meldungen, Ausblendungen)');
-- --------------------------------------------------------------------------------------------- -- ---------------------------------------------------------------------------------------------
@ -507,7 +530,7 @@ select is((select count(*) from public.catalog_games where id = '7c1d2c3d-0000-4
'Das neue Katalogspiel erscheint in catalog_games'); 'Das neue Katalogspiel erscheint in catalog_games');
select is((select count(*) from public.catalog_games where id = '7c1d2c3d-0000-4000-8000-000000000003'), 0::bigint, select is((select count(*) from public.catalog_games where id = '7c1d2c3d-0000-4000-8000-000000000003'), 0::bigint,
'Der vorläufige Eintrag erscheint nie in catalog_games'); 'Der vorläufige Eintrag erscheint nie in catalog_games');
select is((select resolved_at is not null and updated_at >= created_at from public.game_reports where id = '7c1d2c3d-0000-4000-8000-000000000001'), true, select is((select resolved_at is not null and updated_at > '2026-01-01' from public.game_reports where id = '7c1d2c3d-0000-4000-8000-000000000001'), true,
'resolved_at ist gesetzt'); 'resolved_at ist gesetzt');
-- Verweise -- Verweise