diff --git a/supabase/migrations/20260727130000_om_cleanup_and_positions.sql b/supabase/migrations/20260727130000_om_cleanup_and_positions.sql index 27398bf..496d196 100644 --- a/supabase/migrations/20260727130000_om_cleanup_and_positions.sql +++ b/supabase/migrations/20260727130000_om_cleanup_and_positions.sql @@ -32,8 +32,14 @@ drop table if exists reorg_scenarios; -- Planstelle, und eine offene Stelle ist schlicht eine unbesetzte. Anlegen -- und Schliessen sind deshalb Operationen auf om_positions. +-- set search_path bei jeder Funktion: siehe die folgende Migration, dort steht +-- warum. Kurz: heute sind das INVOKER-Funktionen und der Pfad ist harmlos, +-- aber sobald eine davon einmal SECURITY DEFINER wird, wäre er es nicht mehr — +-- und daran denkt dann niemand. create or replace function next_position_number() -returns text language sql stable as $$ +returns text language sql stable +set search_path = public, pg_temp +as $$ select '6' || lpad((coalesce(max(substring(position_number from 2)::bigint), 0) + 1)::text, 7, '0') from om_positions where position_number ~ '^6[0-9]{7}$'; @@ -43,7 +49,9 @@ comment on function next_position_number() is 'Nächste freie Planstellennummer im Nummernkreis 6xxxxxxx.'; create or replace function create_position(payload jsonb) -returns uuid language plpgsql as $$ +returns uuid language plpgsql +set search_path = public, pg_temp +as $$ declare v_org_unit_id uuid := (payload->>'org_unit_id')::uuid; v_job_title text := nullif(trim(payload->>'job_title'), ''); @@ -95,7 +103,9 @@ end; $$; create or replace function delete_position(payload jsonb) -returns void language plpgsql as $$ +returns void language plpgsql +set search_path = public, pg_temp +as $$ declare v_position_id uuid := (payload->>'position_id')::uuid; v_label text; diff --git a/supabase/migrations/20260727140000_pin_function_search_path.sql b/supabase/migrations/20260727140000_pin_function_search_path.sql new file mode 100644 index 0000000..41016f5 --- /dev/null +++ b/supabase/migrations/20260727140000_pin_function_search_path.sql @@ -0,0 +1,77 @@ +-- search_path für alle eigenen Funktionen festnageln. +-- +-- Der Supabase-Linter meldet 34 Funktionen mit „Function Search Path Mutable". +-- Nachgezählt sind das alles SECURITY-INVOKER-Funktionen; die vier +-- SECURITY-DEFINER-Funktionen (is_hr_user, current_hr_user_id, +-- apply_due_pending_changes, fn_track_employee_assignment) setzen den Pfad +-- längst. Deshalb steht im Advisor auch 0 errors. +-- +-- Warum das trotzdem behoben wird: +-- +-- Der Angriff braucht SECURITY DEFINER. Wer in einem Schema, das im +-- search_path früher liegt, eine eigene Tabelle `employees` anlegt, bringt +-- eine unqualifiziert schreibende Funktion dazu, auf die untergeschobene +-- zuzugreifen — mit den Rechten der Eigentümerin der Funktion. Bei INVOKER +-- läuft alles mit den Rechten der aufrufenden Person, es gibt also nichts zu +-- gewinnen, und die RLS-Policies greifen unverändert. +-- +-- Zur Lücke wird die Warnung erst, wenn eine dieser Funktionen später auf +-- SECURITY DEFINER umgestellt wird, etwa weil eine Mutation an RLS vorbei +-- schreiben muss. In dem Moment denkt niemand mehr an den search_path. +-- Einmal festnageln räumt die Falle weg und ändert kein Verhalten. +-- +-- `pg_temp` steht ausdrücklich am Ende: ohne die Angabe durchsucht Postgres +-- das temporäre Schema *zuerst*, und dort darf jede Sitzung anlegen, was sie +-- will. + +do $$ +declare + v_func record; + v_count int := 0; +begin + for v_func in + select p.oid::regprocedure as signature + from pg_proc p + join pg_namespace n on n.oid = p.pronamespace + where n.nspname = 'public' + -- Nur Funktionen, keine Prozeduren oder Aggregate. + and p.prokind = 'f' + -- Erweiterungen gehören uns nicht: pg_trgm legt show_trgm und show_limit + -- in public ab. Daran zu drehen bricht bei der nächsten Aktualisierung + -- der Erweiterung oder wird stillschweigend zurückgesetzt. + and not exists ( + select 1 from pg_depend d where d.objid = p.oid and d.deptype = 'e' + ) + -- Bereits gesetzte nicht anfassen: die vier DEFINER-Funktionen stehen + -- auf `search_path = public` und sollen so bleiben. + and not exists ( + select 1 from unnest(coalesce(p.proconfig, '{}')) c where c like 'search_path=%' + ) + loop + execute format('alter function %s set search_path = public, pg_temp', v_func.signature); + v_count := v_count + 1; + end loop; + + raise notice 'search_path festgenagelt für % Funktion(en)', v_count; +end; +$$; + +-- Gegenprobe: danach darf in public keine eigene Funktion ohne search_path +-- mehr stehen. Schlägt das an, hat die Schleife oben etwas übersehen — besser +-- hier, als es im Advisor stehen zu lassen. +do $$ +declare v_offen int; +begin + select count(*) into v_offen + from pg_proc p + join pg_namespace n on n.oid = p.pronamespace + where n.nspname = 'public' + and p.prokind = 'f' + and not exists (select 1 from pg_depend d where d.objid = p.oid and d.deptype = 'e') + and not exists (select 1 from unnest(coalesce(p.proconfig, '{}')) c where c like 'search_path=%'); + + if v_offen > 0 then + raise exception 'Es stehen noch % Funktion(en) ohne search_path in public.', v_offen; + end if; +end; +$$; diff --git a/supabase/migrations/20260727150000_revoke_definer_execute.sql b/supabase/migrations/20260727150000_revoke_definer_execute.sql new file mode 100644 index 0000000..408fbdd --- /dev/null +++ b/supabase/migrations/20260727150000_revoke_definer_execute.sql @@ -0,0 +1,72 @@ +-- Ausführungsrechte auf den SECURITY-DEFINER-Funktionen zurechtrücken. +-- +-- Der Advisor meldet alle vier als „Public Can Execute" und „Signed-In Users +-- Can Execute". Das ist nicht bei allen vieren dasselbe Problem — nachgemessen +-- mit dem anon-Schlüssel gegen die laufende Datenbank: +-- +-- anon.rpc(is_hr_user) -> false +-- anon.rpc(current_hr_user_id) -> null +-- anon.rpc(apply_due_pending_changes) -> 0 ← das ist der Befund +-- +-- Nur der dritte ist einer. + +-- ── Bleibt offen, und zwar mit Absicht ─────────────────────────── +-- +-- is_hr_user() und current_hr_user_id() werden *aus den RLS-Policies heraus* +-- aufgerufen. Ein Policy-Ausdruck wird mit den Rechten der abfragenden Rolle +-- ausgewertet; ohne EXECUTE für anon und authenticated scheitert damit jede +-- Abfrage auf jeder Tabelle mit „permission denied for function". Der Entzug +-- würde die Anwendung vollständig lahmlegen. +-- +-- Preisgegeben wird dabei nichts: beide nehmen keine Argumente und beantworten +-- ausschliesslich eine Frage über die aufrufende Person selbst. Wer nicht +-- angemeldet ist, bekommt false beziehungsweise null — siehe Messung oben. + +-- ── Wird entzogen ──────────────────────────────────────────────── +-- +-- apply_due_pending_changes() wendet vorgemerkte Versetzungen, Beförderungen +-- und Abwesenheiten an, sobald ihr Datum erreicht ist. Es ist SECURITY +-- DEFINER, umgeht also RLS, und war bis hierher ohne Anmeldung aufrufbar — der +-- anon-Schlüssel steht im ausgelieferten Browser-Bündel. +-- +-- Der Schaden wäre begrenzt, weil nur ohnehin fällige Änderungen angewandt +-- werden. Aber es ist ein Schreibpfad, den Fremde auslösen können, und er +-- macht das Geheimnis der Cron-Route (app/api/cron/apply-pending-changes) +-- wirkungslos. +-- +-- Diese Route ist der einzige Aufrufer und benutzt createAdminClient(), also +-- die service_role — der Entzug für anon und authenticated bricht sie nicht. +revoke execute on function apply_due_pending_changes() from anon, authenticated; + +-- rls_auto_enable() stammt nicht aus diesen Migrationen und wird von der +-- Anwendung nirgends aufgerufen. Was sie tut, ist von hier aus nicht +-- feststellbar; eine Funktion, die RLS umschaltet und ohne Anmeldung +-- aufrufbar ist, wäre allerdings ernst. Der Entzug ist risikolos, weil kein +-- Aufrufer existiert — und falls doch jemand sie braucht, meldet er sich mit +-- einer klaren Fehlermeldung statt still etwas zu verstellen. +do $$ +begin + if exists ( + select 1 from pg_proc p + join pg_namespace n on n.oid = p.pronamespace + where n.nspname = 'public' and p.proname = 'rls_auto_enable' + ) then + execute 'revoke execute on function public.rls_auto_enable() from anon, authenticated'; + end if; +end; +$$; + +-- ── Was bewusst *nicht* passiert ───────────────────────────────── +-- +-- „Extension in Public" (pg_trgm) bleibt stehen. Die Erweiterung trägt die +-- Operatorklasse gin_trgm_ops, auf der zwei GIN-Indizes auf employees liegen +-- (20260714120400_performance_indexes.sql). Ein Schemawechsel müsste die +-- Indizes und jeden search_path mitziehen, der sie erreichen soll — gerade +-- jetzt, wo jede Funktion auf `public, pg_temp` festgenagelt ist. Das ist +-- Aufwand und Risiko für einen Hinweis, der keine Rechteausweitung beschreibt, +-- sondern eine Konvention. +-- +-- „Leaked Password Protection Disabled" ist gegenstandslos: die +-- Passwort-Anmeldung ist abgeschaltet. Eine Anmeldung mit E-Mail und Passwort +-- gegen die API antwortet mit `email_provider_disabled` (422). Es gibt kein +-- Passwort, dessen Kompromittierung geprüft werden könnte.