From 28957f1e0ffb5df0bc6f6dbbeecf9dfda8c91a3d Mon Sep 17 00:00:00 2001
From: weeihan <whyap79@gmail.com>
Date: Fri, 10 Jul 2026 06:01:36 +0800
Subject: [PATCH] fix: patch critical auth and RLS security findings from final
 review

---
 middleware.ts                                 |  18 +++
 .../20260709000009_security_fixes.sql         | 113 ++++++++++++++++++
 2 files changed, 131 insertions(+)
 create mode 100644 supabase/migrations/20260709000009_security_fixes.sql

diff --git a/middleware.ts b/middleware.ts
index 0dcd857..5992fb0 100644
--- a/middleware.ts
+++ b/middleware.ts
@@ -51,6 +51,24 @@ export async function middleware(request: NextRequest) {
     }
   }
 
+  // Role-based route enforcement for protected paths
+  if (user && !isPublicRoute && pathname !== '/') {
+    const { data: profile } = await supabase
+      .from('users')
+      .select('role')
+      .eq('id', user.id)
+      .single()
+
+    const role = profile?.role
+    if (isValidRole(role)) {
+      const allowedPrefix = ROLE_HOME[role as UserRole]
+      // Block access to routes that don't belong to this role
+      if (!pathname.startsWith(allowedPrefix)) {
+        return NextResponse.redirect(new URL(allowedPrefix, request.url))
+      }
+    }
+  }
+
   return supabaseResponse
 }
 
diff --git a/supabase/migrations/20260709000009_security_fixes.sql b/supabase/migrations/20260709000009_security_fixes.sql
new file mode 100644
index 0000000..9e280d6
--- /dev/null
+++ b/supabase/migrations/20260709000009_security_fixes.sql
@@ -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);