refactor: split splinter rules

Author: psteinroe

crates/pgls_splinter/.sqlx/query-02927e584e85871ba6f84c58e8b5e4454b2c36eaf034657d5d2d95633fb85bdb.json

@@ -5,10 +5,24 @@ use std::path::Path;
 // Update this commit SHA to pull in a new version of splinter.sql
 const SPLINTER_COMMIT_SHA: &str = "27ea2ece65464213e466cd969cc61b6940d16219";
 
+// Rules that require Supabase-specific infrastructure (auth schema, anon/authenticated roles, pgrst.db_schemas)
+const SUPABASE_ONLY_RULES: &[&str] = &[
+    "auth_users_exposed",
+    "auth_rls_initplan",
+    "rls_disabled_in_public",
+    "security_definer_view",
+    "rls_references_user_metadata",
+    "materialized_view_in_api",
+    "foreign_table_in_api",
+    "insecure_queue_exposed_in_api",
+    "fkey_to_auth_unique",
+];
+
 fn main() {
     let out_dir = env::var("CARGO_MANIFEST_DIR").unwrap();
     let vendor_dir = Path::new(&out_dir).join("vendor");
-    let sql_file = vendor_dir.join("splinter.sql");
+    let generic_sql_file = vendor_dir.join("splinter_generic.sql");
+    let supabase_sql_file = vendor_dir.join("splinter_supabase.sql");
     let sha_file = vendor_dir.join("COMMIT_SHA.txt");
 
     // Create vendor directory if it doesn't exist
@@ -17,22 +31,23 @@ fn main() {
     }
 
     // Check if we need to download
-    let needs_download = if !sql_file.exists() || !sha_file.exists() {
-        true
-    } else {
-        // Check if stored SHA matches current constant
-        let stored_sha = fs::read_to_string(&sha_file)
-            .expect("Failed to read COMMIT_SHA.txt")
-            .trim()
-            .to_string();
-        stored_sha != SPLINTER_COMMIT_SHA
-    };
+    let needs_download =
+        if !generic_sql_file.exists() || !supabase_sql_file.exists() || !sha_file.exists() {
+            true
+        } else {
+            // Check if stored SHA matches current constant
+            let stored_sha = fs::read_to_string(&sha_file)
+                .expect("Failed to read COMMIT_SHA.txt")
+                .trim()
+                .to_string();
+            stored_sha != SPLINTER_COMMIT_SHA
+        };
 
     if needs_download {
         println!(
             "cargo:warning=Downloading splinter.sql from GitHub (commit: {SPLINTER_COMMIT_SHA})"
         );
-        download_and_process_sql(&sql_file);
+        download_and_process_sql(&generic_sql_file, &supabase_sql_file);
         fs::write(&sha_file, SPLINTER_COMMIT_SHA).expect("Failed to write COMMIT_SHA.txt");
     }
 
@@ -41,7 +56,7 @@ fn main() {
     println!("cargo:rerun-if-changed=vendor/COMMIT_SHA.txt");
 }
 
-fn download_and_process_sql(dest_path: &Path) {
+fn download_and_process_sql(generic_dest: &Path, supabase_dest: &Path) {
     let url = format!(
         "https://raw.githubusercontent.com/supabase/splinter/{SPLINTER_COMMIT_SHA}/splinter.sql"
     );
@@ -61,10 +76,16 @@ fn download_and_process_sql(dest_path: &Path) {
     // Add "!" suffix to column aliases for sqlx non-null checking
     processed_content = add_not_null_markers(&processed_content);
 
-    // Write to destination
-    fs::write(dest_path, processed_content).expect("Failed to write splinter.sql");
+    // Split into generic and Supabase-specific queries
+    let (generic_queries, supabase_queries) = split_queries(&processed_content);
 
-    println!("cargo:warning=Successfully downloaded and processed splinter.sql");
+    // Write to destination files
+    fs::write(generic_dest, generic_queries).expect("Failed to write splinter_generic.sql");
+    fs::write(supabase_dest, supabase_queries).expect("Failed to write splinter_supabase.sql");
+
+    println!(
+        "cargo:warning=Successfully downloaded and processed splinter.sql into generic and Supabase-specific files"
+    );
 }
 
 fn remove_set_search_path(content: &str) -> String {
@@ -107,3 +128,39 @@ fn add_not_null_markers(content: &str) -> String {
 
     result
 }
+
+fn split_queries(content: &str) -> (String, String) {
+    // Split the union all queries based on rule names
+    let queries: Vec<&str> = content.split("union all").collect();
+
+    let mut generic_queries = Vec::new();
+    let mut supabase_queries = Vec::new();
+
+    for query in queries {
+        // Extract the rule name from the query (it's the first 'name' field)
+        let is_supabase = SUPABASE_ONLY_RULES
+            .iter()
+            .any(|rule| query.contains(&format!("'{rule}' as \"name!\"")));
+
+        if is_supabase {
+            supabase_queries.push(query);
+        } else {
+            generic_queries.push(query);
+        }
+    }
+
+    // Join queries with "union all" and wrap in parentheses
+    let generic_sql = if generic_queries.is_empty() {
+        String::new()
+    } else {
+        generic_queries.join("union all\n")
+    };
+
+    let supabase_sql = if supabase_queries.is_empty() {
+        String::new()
+    } else {
+        supabase_queries.join("union all\n")
+    };
+
+    (generic_sql, supabase_sql)
+}

crates/pgls_splinter/.sqlx/query-425fc6118e76cea42cf256b3b0f11046dc8b77d84c94314a0ed0716e5803df69.json

@@ -1,4 +1,4 @@
-use pgls_diagnostics::{Category, Severity, category};
+use pgls_diagnostics::{category, Category, Severity};
 use serde_json::Value;
 
 use crate::{SplinterAdvices, SplinterDiagnostic, SplinterQueryResult};
@@ -27,13 +27,45 @@ impl From<SplinterQueryResult> for SplinterDiagnostic {
                 schema,
                 object_name,
                 object_type,
-                remediation_url: result.remediation,
+                remediation_url: build_remediation_url(&result.name),
                 additional_metadata,
             },
         }
     }
 }
 
+/// Build remediation URL from rule name
+/// Maps rule names to their Supabase linter documentation
+fn build_remediation_url(name: &str) -> String {
+    // Map rule names to their lint IDs
+    let lint_id = match name {
+        "unindexed_foreign_keys" => "0001_unindexed_foreign_keys",
+        "auth_users_exposed" => "0002_auth_users_exposed",
+        "auth_rls_initplan" => "0003_auth_rls_initplan",
+        "no_primary_key" => "0004_no_primary_key",
+        "unused_index" => "0005_unused_index",
+        "multiple_permissive_policies" => "0006_multiple_permissive_policies",
+        "policy_exists_rls_disabled" => "0007_policy_exists_rls_disabled",
+        "rls_enabled_no_policy" => "0008_rls_enabled_no_policy",
+        "duplicate_index" => "0009_duplicate_index",
+        "security_definer_view" => "0010_security_definer_view",
+        "function_search_path_mutable" => "0011_function_search_path_mutable",
+        "rls_disabled_in_public" => "0013_rls_disabled_in_public",
+        "extension_in_public" => "0014_extension_in_public",
+        "rls_references_user_metadata" => "0015_rls_references_user_metadata",
+        "materialized_view_in_api" => "0016_materialized_view_in_api",
+        "foreign_table_in_api" => "0017_foreign_table_in_api",
+        "unsupported_reg_types" => "unsupported_reg_types",
+        "insecure_queue_exposed_in_api" => "0019_insecure_queue_exposed_in_api",
+        "table_bloat" => "0020_table_bloat",
+        "fkey_to_auth_unique" => "0021_fkey_to_auth_unique",
+        "extension_versions_outdated" => "0022_extension_versions_outdated",
+        _ => return format!("https://supabase.com/docs/guides/database/database-linter"),
+    };
+
+    format!("https://supabase.com/docs/guides/database/database-linter?lint={lint_id}")
+}
+
 /// Parse severity level from the query result
 fn parse_severity(level: &str) -> Severity {
     match level {

crates/pgls_splinter/build.rs

@@ -12,7 +12,7 @@ pub struct SplinterParams<'a> {
     pub conn: &'a PgPool,
 }
 
-async fn check_required_roles(conn: &PgPool) -> Result<bool, sqlx::Error> {
+async fn check_supabase_roles(conn: &PgPool) -> Result<bool, sqlx::Error> {
     let required_roles = ["anon", "authenticated", "service_role"];
 
     let existing_roles: Vec<String> =
@@ -32,17 +32,19 @@ async fn check_required_roles(conn: &PgPool) -> Result<bool, sqlx::Error> {
 pub async fn run_splinter(
     params: SplinterParams<'_>,
 ) -> Result<Vec<SplinterDiagnostic>, sqlx::Error> {
-    // check if required supabase roles exist
-    // if they don't exist, return empty diagnostics since splinter is supabase-specific
-    // opened an issue to make it less supabase-specific: https://github.com/supabase/splinter/issues/135
-    let has_roles = check_required_roles(params.conn).await?;
-    if !has_roles {
-        return Ok(Vec::new());
-    }
+    let mut all_results = Vec::new();
+
+    let generic_results = query::load_generic_splinter_results(params.conn).await?;
+    all_results.extend(generic_results);
 
-    let results = query::load_splinter_results(params.conn).await?;
+    // Only run Supabase-specific rules if the required roles exist
+    let has_supabase_roles = check_supabase_roles(params.conn).await?;
+    if has_supabase_roles {
+        let supabase_results = query::load_supabase_splinter_results(params.conn).await?;
+        all_results.extend(supabase_results);
+    }
 
-    let diagnostics: Vec<SplinterDiagnostic> = results.into_iter().map(Into::into).collect();
+    let diagnostics: Vec<SplinterDiagnostic> = all_results.into_iter().map(Into::into).collect();
 
     Ok(diagnostics)
 }

crates/pgls_splinter/src/convert.rs

@@ -38,7 +38,9 @@ pub struct SplinterQueryResult {
     pub cache_key: String,
 }
 
-pub async fn load_splinter_results(pool: &PgPool) -> Result<Vec<SplinterQueryResult>, sqlx::Error> {
+pub async fn load_generic_splinter_results(
+    pool: &PgPool,
+) -> Result<Vec<SplinterQueryResult>, sqlx::Error> {
     let mut tx = pool.begin().await?;
 
     // this is done by the splinter.sql file normally, but we remove it so that sqlx can work with
@@ -47,7 +49,27 @@ pub async fn load_splinter_results(pool: &PgPool) -> Result<Vec<SplinterQueryRes
         .execute(&mut *tx)
         .await?;
 
-    let results = sqlx::query_file_as!(SplinterQueryResult, "vendor/splinter.sql")
+    let results = sqlx::query_file_as!(SplinterQueryResult, "vendor/splinter_generic.sql")
+        .fetch_all(&mut *tx)
+        .await?;
+
+    tx.commit().await?;
+
+    Ok(results)
+}
+
+pub async fn load_supabase_splinter_results(
+    pool: &PgPool,
+) -> Result<Vec<SplinterQueryResult>, sqlx::Error> {
+    let mut tx = pool.begin().await?;
+
+    // this is done by the splinter.sql file normally, but we remove it so that sqlx can work with
+    // the file properly.
+    sqlx::query("set local search_path = ''")
+        .execute(&mut *tx)
+        .await?;
+
+    let results = sqlx::query_file_as!(SplinterQueryResult, "vendor/splinter_supabase.sql")
         .fetch_all(&mut *tx)
         .await?;
 

crates/pgls_splinter/src/lib.rs

@@ -1,6 +1,6 @@
 use pgls_console::fmt::{Formatter, HTML};
 use pgls_diagnostics::{Diagnostic, LogCategory, Visit};
-use pgls_splinter::{SplinterParams, run_splinter};
+use pgls_splinter::{run_splinter, SplinterParams};
 use sqlx::PgPool;
 use std::fmt::Write;
 use std::io;
@@ -247,14 +247,31 @@ async fn multiple_issues(test_db: PgPool) {
 }
 
 #[sqlx::test(migrator = "pgls_test_utils::MIGRATIONS")]
-async fn missing_roles_returns_empty(test_db: PgPool) {
+async fn missing_roles_runs_generic_checks_only(test_db: PgPool) {
+    // Without Supabase roles, generic rules should still run
+    // but Supabase-specific rules should be skipped
     let diagnostics = run_splinter(SplinterParams { conn: &test_db })
         .await
-        .expect("Should not error when roles are missing");
+        .expect("Should not error when Supabase roles are missing");
 
     assert!(
         diagnostics.is_empty(),
-        "Expected empty diagnostics when Supabase roles are missing, but got {} diagnostics",
+        "Expected empty diagnostics for a clean database without Supabase roles, but got {} diagnostics",
         diagnostics.len()
     );
+
+    // Now create a table with a generic issue (no primary key)
+    sqlx::raw_sql("CREATE TABLE public.test_table (name text)")
+        .execute(&test_db)
+        .await
+        .expect("Failed to create test table");
+
+    let diagnostics_with_issue = run_splinter(SplinterParams { conn: &test_db })
+        .await
+        .expect("Should not error when checking for issues");
+
+    assert!(
+        !diagnostics_with_issue.is_empty(),
+        "Expected to detect generic issues (no primary key) even without Supabase roles"
+    );
 }

crates/pgls_splinter/src/query.rs

@@ -1 +1 @@
-27ea2ece65464213e466cd969cc61b6940d16219
+27ea2ece65464213e466cd969cc61b6940d16219