Fix RLS security: switch to service_role client and enable RLS policies

migrations/README.md

@@ -0,0 +1,70 @@
+# Row Level Security (RLS) Implementation
+
+## Overview
+This migration enables RLS on the `public.trees` table to secure the TreeTrack application against unauthorized database access via the public anon key.
+
+## Security Model
+
+### Before (Insecure)
+- RLS disabled on `public.trees`
+- Backend used `anon_key` for all database operations
+- Anyone with the anon key could access all tree data directly
+
+### After (Secure)
+- RLS enabled with deny-all policy for anon role
+- Backend uses `service_role` key (bypasses RLS)
+- Authorization enforced by FastAPI layer via `AuthManager`
+- Frontend never accesses Supabase directly (all via `/api/*` endpoints)
+
+## Architecture
+
+```
+Frontend (JS) 
+    ↓ (Bearer token auth)
+FastAPI Endpoints (/api/*)
+    ↓ (validates JWT via AuthManager)
+SupabaseDatabase (service_role client)
+    ↓ (bypasses RLS)
+Supabase Postgres (RLS enabled, anon blocked)
+```
+
+## Migration Steps
+
+1. **Execute SQL migration**
+   - Open Supabase Dashboard → SQL Editor
+   - Run `migrations/enable_rls.sql`
+   - Verify output shows `rowsecurity = true`
+
+2. **Code changes** (already applied)
+   - `supabase_database.py` now uses `get_service_client()`
+   - Removed unnecessary comments throughout module
+
+3. **Test the application**
+   ```bash
+   # Ensure service_role key is set
+   $env:SUPABASE_SERVICE_ROLE_KEY="your-service-role-key"
+   
+   # Start the app
+   uv run uvicorn app:app --reload
+   ```
+
+4. **Verify security**
+   - Test CRUD operations through web interface
+   - Confirm all operations work normally
+   - Verify anon key cannot access data directly (use Supabase API client to test)
+
+## Important Notes
+
+- **Service role key must remain server-side only** - never expose in frontend code or logs
+- Frontend authentication handled by custom JWT system (not Supabase Auth)
+- All database authorization checks done by FastAPI before database access
+- RLS policy blocks anon role as defense-in-depth measure
+
+## Rollback
+
+If issues occur, disable RLS temporarily:
+```sql
+ALTER TABLE public.trees DISABLE ROW LEVEL SECURITY;
+```
+
+Then investigate and fix before re-enabling.

migrations/enable_rls.sql

@@ -0,0 +1,25 @@
+-- Enable Row Level Security on public.trees table
+-- Execute this in Supabase SQL Editor
+
+-- Step 1: Enable RLS on the trees table
+ALTER TABLE public.trees ENABLE ROW LEVEL SECURITY;
+
+-- Step 2: Drop any existing policies (idempotent)
+DROP POLICY IF EXISTS "Deny all access to anon role" ON public.trees;
+
+-- Step 3: Create policy to deny all access via anon key
+-- This ensures the anon key cannot be used to access tree data
+-- All operations must go through the FastAPI backend using service_role
+CREATE POLICY "Deny all access to anon role" 
+ON public.trees 
+FOR ALL 
+TO anon 
+USING (false);
+
+-- Step 4: Verify RLS is enabled
+SELECT tablename, rowsecurity 
+FROM pg_tables 
+WHERE schemaname = 'public' 
+AND tablename = 'trees';
+
+-- Expected result: rowsecurity = true

static/index.html

@@ -947,7 +947,7 @@
         // Force refresh if we detect cached version
         (function() {
             const currentVersion = '5.1.1';
-            const timestamp = '1761514190'; // Cache-busting bump
+            const timestamp = '1762279460'; // Cache-busting bump
             const lastVersion = sessionStorage.getItem('treetrack_version');
             const lastTimestamp = sessionStorage.getItem('treetrack_timestamp');
             
@@ -1193,7 +1193,7 @@
         </div>
     </div>
 
-    <script type="module" src="/static/js/tree-track-app.js?v=5.1.1&t=1761514190"></script>
+    <script type="module" src="/static/js/tree-track-app.js?v=5.1.1&t=1762279460"></script>
     
     <script>
         // Idle-time prefetch of map assets to speed up first navigation

static/map.html

@@ -968,7 +968,7 @@
         // Force refresh if we detect cached version
         (function() {
             const currentVersion = '5.1.1';
-const timestamp = '1761514190'; // Current timestamp for cache busting
+const timestamp = '1762279460'; // Current timestamp for cache busting
             const lastVersion = sessionStorage.getItem('treetrack_version');
             const lastTimestamp = sessionStorage.getItem('treetrack_timestamp');
             
@@ -1137,7 +1137,7 @@ const timestamp = '1761514190'; // Current timestamp for cache busting
     <script src="https://unpkg.com/leaflet@1.9.4/dist/leaflet.js"></script>
     <!-- Leaflet MarkerCluster JS for performance and grouping -->
     <script src="https://unpkg.com/leaflet.markercluster@1.4.1/dist/leaflet.markercluster.js"></script>
-<script src="/static/map.js?v=5.1.1&t=1761514190"></script>
+<script src="/static/map.js?v=5.1.1&t=1762279460"></script>
     <script>
         console.log('🗺️ Map script loaded successfully');
     </script>

static/sw.js

@@ -1,5 +1,5 @@
 // TreeTrack Service Worker - PWA and Offline Support
-const VERSION = 1761514190; // Cache busting bump - force clients to fetch new static assets and header image change
+const VERSION = 1762279460; // Cache busting bump - force clients to fetch new static assets and header image change
 const CACHE_NAME = `treetrack-v${VERSION}`;
 const STATIC_CACHE = `static-v${VERSION}`;
 const API_CACHE = `api-v${VERSION}`;

supabase_database.py

@@ -1,42 +1,36 @@
 """
 Supabase database implementation for TreeTrack
-Standalone implementation for Supabase Postgres operations
 """
 
 import json
 import logging
 from typing import Dict, List, Optional, Any
-from supabase_client import get_supabase_client, get_service_client
+from supabase_client import get_service_client
 
 logger = logging.getLogger(__name__)
 
 
 class SupabaseDatabase:
-    """Supabase implementation of DatabaseInterface"""
+    """Uses service_role client to bypass RLS. Authorization handled by FastAPI."""
     
     def __init__(self):
         try:
-            self.client = get_supabase_client()
+            self.client = get_service_client()
             self.connected = True
-            logger.info("SupabaseDatabase initialized successfully")
+            logger.info("SupabaseDatabase initialized with service_role client")
         except ValueError as e:
             logger.warning(f"Supabase not configured: {e}")
-            logger.warning("Database operations will be disabled until Supabase is properly configured")
             self.client = None
             self.connected = False
     
     def _check_connection(self):
-        """Check if database is connected, raise error if not"""
         if not self.connected or not self.client:
             raise RuntimeError("Database not connected. Please configure Supabase credentials.")
     
     def initialize_database(self) -> bool:
-        """Initialize database tables and indexes (already done via SQL)"""
         try:
-            # Verify trees table
             self.client.table('trees').select("id").limit(1).execute()
-            logger.info("Trees table verified in Supabase")
-            # Ensure telemetry table exists
+            logger.info("Trees table verified")
             self._ensure_telemetry_table()
             return True
         except Exception as e:
@@ -44,15 +38,12 @@ class SupabaseDatabase:
             return False
     
     def test_connection(self) -> bool:
-        """Test Supabase connection with enhanced debugging"""
         from supabase_client import test_supabase_connection
         return test_supabase_connection()
     
     async def create_tree(self, tree_data: Dict[str, Any]) -> Dict[str, Any]:
-        """Create a new tree record"""
         self._check_connection()
         try:
-            # Supabase handles JSON fields automatically, no need to stringify
             result = self.client.table('trees').insert(tree_data).execute()
             
             if result.data:
@@ -68,27 +59,19 @@ class SupabaseDatabase:
     
     async def get_trees(self, limit: int = 100, offset: int = 0, 
                   species: str = None, health_status: str = None) -> List[Dict[str, Any]]:
-        """Get trees with pagination and optional filters"""
         self._check_connection()
         try:
             query = self.client.table('trees').select("*")
             
-            # Apply filters
             if species:
                 query = query.ilike('scientific_name', f'%{species}%')
             
-            if health_status:
-                # Note: health_status field doesn't exist in our schema
-                # This is for future compatibility
-                pass
-            
-            # Apply pagination and ordering
             result = query.order('updated_at', desc=True) \
                          .order('created_at', desc=True) \
                          .range(offset, offset + limit - 1) \
                          .execute()
             
-            logger.info(f"Retrieved {len(result.data)} trees from Supabase")
+            logger.info(f"Retrieved {len(result.data)} trees")
             return result.data
             
         except Exception as e:
@@ -96,7 +79,6 @@ class SupabaseDatabase:
             raise
     
     async def get_tree(self, tree_id: int) -> Optional[Dict[str, Any]]:
-        """Get a specific tree by ID"""
         self._check_connection()
         try:
             result = self.client.table('trees') \
@@ -113,10 +95,8 @@ class SupabaseDatabase:
             raise
     
     async def update_tree(self, tree_id: int, tree_data: Dict[str, Any]) -> Dict[str, Any]:
-        """Update a tree record"""
         self._check_connection()
         try:
-            # Remove id from update data if present
             update_data = {k: v for k, v in tree_data.items() if k != 'id'}
             
             result = self.client.table('trees') \
@@ -136,7 +116,6 @@ class SupabaseDatabase:
             raise
     
     async def delete_tree(self, tree_id: int) -> bool:
-        """Delete a tree record"""
         self._check_connection()
         try:
             result = self.client.table('trees') \
@@ -152,7 +131,6 @@ class SupabaseDatabase:
             raise
     
     def get_tree_count(self) -> int:
-        """Get total number of trees"""
         try:
             result = self.client.table('trees') \
                                .select("id", count="exact") \
@@ -165,44 +143,25 @@ class SupabaseDatabase:
             return 0
     
     def get_species_distribution(self, limit: int = 20) -> List[Dict[str, Any]]:
-        """Get trees by species distribution using database aggregation"""
         try:
-            # Try to use Supabase RPC for aggregation first
             try:
-                # This would require a stored procedure in Supabase:
-                # CREATE OR REPLACE FUNCTION get_species_distribution(record_limit INTEGER DEFAULT 20)
-                # RETURNS TABLE(species TEXT, count BIGINT) AS $$
-                # BEGIN
-                #   RETURN QUERY
-                #   SELECT scientific_name, COUNT(*) as count_val
-                #   FROM trees
-                #   WHERE scientific_name IS NOT NULL AND scientific_name != ''
-                #   GROUP BY scientific_name
-                #   ORDER BY count_val DESC
-                #   LIMIT record_limit;
-                # END;
-                # $$ LANGUAGE plpgsql;
-                
                 result = self.client.rpc('get_species_distribution', {'record_limit': limit}).execute()
                 if result.data:
                     return [{"species": row["species"], "count": row["count"]} for row in result.data]
             except Exception as rpc_error:
-                logger.info(f"RPC function not available, falling back to Python aggregation: {rpc_error}")
+                logger.info(f"RPC not available, using Python aggregation")
             
-            # Fallback: Fetch only what we need and aggregate in Python
             result = self.client.table('trees') \
                                .select("scientific_name") \
                                .not_.is_('scientific_name', 'null') \
                                .neq('scientific_name', '') \
                                .execute()
             
-            # Group by species in Python (not optimal but works)
             species_count = {}
             for tree in result.data:
                 species = tree.get('scientific_name', 'Unknown')
                 species_count[species] = species_count.get(species, 0) + 1
             
-            # Convert to list and sort
             distribution = [
                 {"species": species, "count": count}
                 for species, count in species_count.items()
@@ -216,10 +175,7 @@ class SupabaseDatabase:
             return []
     
     def get_health_distribution(self) -> List[Dict[str, Any]]:
-        """Get trees by health status distribution"""
         try:
-            # Health status field doesn't exist in our current schema
-            # Return empty for compatibility
             return []
             
         except Exception as e:
@@ -227,9 +183,7 @@ class SupabaseDatabase:
             return []
     
     def get_average_measurements(self) -> Dict[str, float]:
-        """Get average height and diameter"""
         try:
-            # Note: we have 'width' field, not 'diameter'
             result = self.client.table('trees') \
                                .select("height, width") \
                                .not_.is_('height', 'null') \
@@ -247,7 +201,7 @@ class SupabaseDatabase:
             
             return {
                 "average_height": round(avg_height, 2),
-                "average_diameter": round(avg_width, 2)  # Using width as diameter
+                "average_diameter": round(avg_width, 2)
             }
             
         except Exception as e:
@@ -255,30 +209,20 @@ class SupabaseDatabase:
             return {"average_height": 0, "average_diameter": 0}
     
     def backup_database(self) -> bool:
-        """Backup database (not needed for Supabase - automatically backed up)"""
-        logger.info("Supabase automatically backs up data - no manual backup needed")
         return True
     
     def restore_database(self) -> bool:
-        """Restore database (not needed for Supabase)"""
-        logger.info("Supabase data is persistent - no restore needed")
         return True
 
-    # Telemetry support
     def _ensure_telemetry_table(self) -> None:
-        """Ensure telemetry_events table exists via SQL RPC if available."""
         try:
-            # Attempt to select; if it fails, try to create via RPC or ignore
             self.client.table('telemetry_events').select('id').limit(1).execute()
-            logger.info("Telemetry table verified in Supabase")
+            logger.info("Telemetry table verified")
         except Exception as e:
             logger.info(f"telemetry_events table not accessible: {e}")
-            # As we may not have a generic SQL RPC, we rely on manual creation documented.
-            # We'll create a fallback table via a stored RPC if present; otherwise, first insert will fail gracefully.
             pass
 
     def log_telemetry(self, event: Dict[str, Any]) -> bool:
-        """Insert a telemetry event into Supabase telemetry_events table."""
         try:
             payload = {
                 'event_type': event.get('event_type'),
@@ -297,7 +241,6 @@ class SupabaseDatabase:
             return False
 
     def get_recent_telemetry(self, limit: int = 100) -> List[Dict[str, Any]]:
-        """Fetch recent telemetry events from Supabase."""
         try:
             result = self.client.table('telemetry_events') \
                 .select('*') \

version.json

@@ -1,4 +1,4 @@
 {
   "version": "5.1.1",
-  "timestamp": 1761514190
+  "timestamp": 1762279460
 }