fix: patch critical auth and RLS security findings from final review

2026-07-10 06:01:36 +08:00
parent 152a6602d6
commit 28957f1e0f
2 changed files with 131 additions and 0 deletions
@@ -0,0 +1,113 @@
+-- supabase/migrations/20260709000009_security_fixes.sql
+-- Security fixes from IMS Phase 0 final code review
+
+-- =============================================================================
+-- 1. CRITICAL: Drop and recreate users_update_own with WITH CHECK to prevent
+--    role self-escalation (any user setting their own role to admin)
+-- =============================================================================
+DROP POLICY IF EXISTS "users_update_own" ON users;
+
+CREATE POLICY "users_update_own" ON users
+  FOR UPDATE
+  USING (id = auth.uid())
+  WITH CHECK (
+    id = auth.uid()
+    AND role = (SELECT role FROM public.users WHERE id = auth.uid())
+  );
+
+-- =============================================================================
+-- 2. IMPORTANT: SECURITY DEFINER audit writer function — app code calls this
+--    instead of direct INSERT on audit_log
+-- =============================================================================
+CREATE OR REPLACE FUNCTION public.write_audit_log(
+  p_table_name TEXT,
+  p_record_id  UUID,
+  p_action     TEXT,
+  p_old_value  JSONB DEFAULT NULL,
+  p_new_value  JSONB DEFAULT NULL
+)
+RETURNS void
+LANGUAGE plpgsql
+SECURITY DEFINER
+SET search_path = public
+AS $$
+BEGIN
+  INSERT INTO public.audit_log (table_name, record_id, action, changed_by, old_value, new_value)
+  VALUES (p_table_name, p_record_id, p_action, auth.uid(), p_old_value, p_new_value);
+END;
+$$;
+
+-- =============================================================================
+-- 3. IMPORTANT: notifications_log INSERT policy for elevated roles
+-- =============================================================================
+CREATE POLICY "notifications_insert_elevated" ON notifications_log
+  FOR INSERT
+  WITH CHECK (auth_user_role() IN ('hse', 'admin', 'supervisor'));
+
+-- =============================================================================
+-- 4. IMPORTANT: Recreate generate_incident_reference trigger function with
+--    SECURITY DEFINER and SET search_path to prevent search_path injection
+-- =============================================================================
+CREATE OR REPLACE FUNCTION public.generate_incident_reference()
+RETURNS TRIGGER
+LANGUAGE plpgsql
+SECURITY DEFINER
+SET search_path = public
+AS $$
+DECLARE
+  v_site_code TEXT;
+  v_month     TEXT;
+  v_seq       INT;
+BEGIN
+  SELECT UPPER(REGEXP_REPLACE(SUBSTRING(name, 1, 6), '[^A-Za-z0-9]', '', 'g'))
+  INTO v_site_code
+  FROM sites
+  WHERE id = NEW.site_id;
+
+  v_month := TO_CHAR(NEW.reported_at, 'YYYYMM');
+
+  SELECT COUNT(*) + 1
+  INTO v_seq
+  FROM incidents
+  WHERE site_id = NEW.site_id
+    AND TO_CHAR(reported_at, 'YYYYMM') = v_month;
+
+  NEW.reference_no := COALESCE(v_site_code, 'UNK') || '-' || v_month || '-' || LPAD(v_seq::TEXT, 4, '0');
+  RETURN NEW;
+END;
+$$;
+
+-- =============================================================================
+-- 5. IMPORTANT: Recreate helper functions with SET search_path to prevent
+--    search_path hijacking attacks
+-- =============================================================================
+CREATE OR REPLACE FUNCTION public.auth_user_role()
+RETURNS user_role
+LANGUAGE sql
+SECURITY DEFINER
+STABLE
+SET search_path = public
+AS $$ SELECT role FROM public.users WHERE id = auth.uid() $$;
+
+CREATE OR REPLACE FUNCTION public.auth_user_site_id()
+RETURNS UUID
+LANGUAGE sql
+SECURITY DEFINER
+STABLE
+SET search_path = public
+AS $$ SELECT site_id FROM public.users WHERE id = auth.uid() $$;
+
+CREATE OR REPLACE FUNCTION public.auth_user_department()
+RETURNS TEXT
+LANGUAGE sql
+SECURITY DEFINER
+STABLE
+SET search_path = public
+AS $$ SELECT department FROM public.users WHERE id = auth.uid() $$;
+
+-- =============================================================================
+-- 6. IMPORTANT: Add UNIQUE constraint to dosh_reports to prevent duplicate
+--    form submissions for the same incident
+-- =============================================================================
+ALTER TABLE dosh_reports
+  ADD CONSTRAINT dosh_reports_incident_form_uq UNIQUE (incident_id, form_type);