Merge pull request #8 from supabase/or/pre-commit

Add pre-commit hooks for consistency

.github/workflows/pre-commit_hooks.yaml

@@ -0,0 +1,25 @@
+name: pre-commit hooks
+
+on: [push]
+
+jobs:
+  build:
+    runs-on: ubuntu-latest
+
+    steps:
+    - name: checkout
+      uses: actions/checkout@v2
+
+    - name: set up python 3.9
+      uses: actions/setup-python@v1
+      with:
+        python-version: 3.9
+
+    - name: install pre-commit
+      run: |
+        python -m pip install --upgrade pip
+        pip install pre-commit
+
+    - name: run static analysis
+      run: |
+        pre-commit run --all-files

.pre-commit-config.yaml

@@ -0,0 +1,38 @@
+repos:
+- repo: https://github.com/pre-commit/pre-commit-hooks
+  rev: v3.3.0
+  hooks:
+  - id: trailing-whitespace
+    exclude: '^test/expected'
+  - id: check-added-large-files
+  - id: check-yaml
+  - id: mixed-line-ending
+    args: ['--fix=lf']
+
+- repo: https://github.com/Lucas-C/pre-commit-hooks
+  rev: v1.5.5
+  hooks:
+    - id: remove-tabs
+      args: [--whitespaces-count, '4']  # defaults to: 4
+
+- repo: https://github.com/Lucas-C/pre-commit-hooks
+  rev: v1.5.5
+  hooks:
+    - id: remove-tabs
+      args: [--whitespaces-count, '4']  # defaults to: 4
+
+repos:
+-   repo: https://github.com/psf/black
+    rev: 24.2.0
+    hooks:
+    - id: black
+      language_version: python3.9
+
+- repo: local
+  hooks:
+  - id: compile-script
+    name: Compile SQL Files
+    entry: python bin/compile.py
+    language: system
+    always_run: true
+    pass_filenames: false

README.md

@@ -6,7 +6,11 @@ This project maintains a set of lints for Supabase projects. It uses SQL queries
 
 Currently `splinter` is intended to house the SQL for the lints but it not opinionated about how they will be executed against user projects. See [Project Linting RFC](https://www.notion.so/supabase/Project-Lints-f34e7b24bb5846c188c8096ad10eb045) for options under consideration.
 
-## Interface
+## Usage
+
+If you are only interested in linting a project, a single query containing the latest version of all lints is availble in splinter.sql in the repo root.
+
+## Lint Interface
 
 Each lint creates a view that returns a common interface. The interface is:
 
@@ -16,7 +20,7 @@ Each lint creates a view that returns a common interface. The interface is:
 - description (text) not null -- This is a description of the lint and why its an issue
 - detail (text) not null -- A text description of the issue that includes references to the specific table/column/constraint/whatever that fails the lint
 - remediation (text) optional -- The SQL to resolve the issue
-- metadata (jsonb) optional -- Any additional information that 
+- metadata (jsonb) optional -- Any additional information that
 - cache_key (text) not null -- A short, uniquely identifiable string that users can add to an exclusion list to avoid repeatedly seeing the same lint failures. It should identify the releavnt table/column/constraint. The string should be prefixed with the lint name. For example a lint named "unindexed_foreign_key" might have a unique key "unindexed_foreign_key_public_user_created_by_id"
 
 
@@ -61,7 +65,7 @@ Detects indexes that have never been used to service a query.
 
 Detects if multiple permissive policies are present on a table for the same `role` and `action` (e.g. insert).
 
-- Level: WARN 
+- Level: WARN
 - Facing: EXTERNAL
 
 ## Requirements
@@ -79,7 +83,7 @@ cd splinter
 
 All lints must have positive and negative tests.
 
-To run the test suite, 
+To run the test suite,
 
 Run test
 ```sh

bin/compile.py

@@ -0,0 +1,41 @@
+import sys
+from pathlib import Path
+
+
+def join_sql_files(directory_path) -> str:
+    # Convert the directory path to a Path object
+    directory = Path(directory_path)
+
+    # Initialize an empty string to store the concatenated SQL
+    queries = []
+
+    # Check if directory exists
+    if not directory.exists():
+        raise Exception(f"The directory {directory} does not exist.")
+
+    # Loop through each file in the specified directory
+    for file_path in sorted(directory.glob("*.sql")):
+        # Open and read the SQL file
+        with file_path.open("r") as file:
+            # Read the file's content and add it to the concatenated SQL string
+            contents = file.read()
+            lines = contents.strip().split("\n")
+
+            # Query without semicolons and "create view" statement
+            adjusted_contents = "\n".join(
+                [line.replace(";", "") for line in lines if "create view" not in line]
+            )
+            # wrap each query in "(" ")" so queries with CTEs can be unioned
+            # without syntax errors
+            adjusted_contents = f"({adjusted_contents})"
+
+            queries.append(adjusted_contents)
+    return "\nunion all\n".join(queries)
+
+
+# Example usage
+directory_path = "./lints"
+concatenated_sql = join_sql_files(directory_path)
+
+with open("splinter.sql", "w") as f:
+    f.write(concatenated_sql)

lints/0001_unindexed_foreign_keys.sql

@@ -1,36 +1,36 @@
 create view lint."0001_unindexed_foreign_keys" as
 
 with foreign_keys as (
-	select
-		cl.oid::regclass as table_,
-		ct.conname as fkey_name,
-		ct.conkey col_attnums
-	from
-		pg_constraint ct
-		join pg_class cl -- fkey owning table
-			on ct.conrelid = cl.oid
-		left join pg_depend d
-			on d.objid = cl.oid
-			and d.deptype = 'e'
-	where
-		ct.contype = 'f' -- foreign key constraints
-		and d.objid is null -- exclude tables that are dependencies of extensions
-		and cl.relnamespace::regnamespace::text not in (
-			'pg_catalog', 'information_schema', 'auth', 'storage', 'vault', 'extensions'
-		)
+    select
+        cl.oid::regclass as table_,
+        ct.conname as fkey_name,
+        ct.conkey col_attnums
+    from
+        pg_constraint ct
+        join pg_class cl -- fkey owning table
+            on ct.conrelid = cl.oid
+        left join pg_depend d
+            on d.objid = cl.oid
+            and d.deptype = 'e'
+    where
+        ct.contype = 'f' -- foreign key constraints
+        and d.objid is null -- exclude tables that are dependencies of extensions
+        and cl.relnamespace::regnamespace::text not in (
+            'pg_catalog', 'information_schema', 'auth', 'storage', 'vault', 'extensions'
+        )
 ),
 index_ as (
-	select
-		indrelid::regclass as table_,
-		indexrelid::regclass as index_,
-		string_to_array(indkey::text, ' ')::smallint[] as col_attnums
-	from
-		pg_index
-	where
-		indisvalid
+    select
+        indrelid::regclass as table_,
+        indexrelid::regclass as index_,
+        string_to_array(indkey::text, ' ')::smallint[] as col_attnums
+    from
+        pg_index
+    where
+        indisvalid
 )
 select
-    '0001_unindexed_foreign_keys' as name,
+    'unindexed_foreign_keys' as name,
     'INFO' as level,
     'EXTERNAL' as facing,
     'Identifies foreign key constraints without a covering index, which can impact database performance.' as description,

lints/0002_auth_users_exposed.sql

@@ -17,23 +17,23 @@ select
     ) as metadata,
     format('auth_users_exposed_%s', c.relname) as cache_key
 from
-	pg_depend d
-	join pg_rewrite r
-		on r.oid = d.objid
-	join pg_class c
-		on c.oid = r.ev_class
-	join pg_namespace n
-		on n.oid = c.relnamespace
+    pg_depend d
+    join pg_rewrite r
+        on r.oid = d.objid
+    join pg_class c
+        on c.oid = r.ev_class
+    join pg_namespace n
+        on n.oid = c.relnamespace
 where
-	d.refobjid = 'auth.users'::regclass
-	and d.deptype = 'n'
-	and c.relkind in ('v', 'm') -- v for view, m for materialized view
-	and n.nspname = 'public'
-	and (
-	  pg_catalog.has_table_privilege('anon', c.oid, 'SELECT')
-	  or pg_catalog.has_table_privilege('authenticated', c.oid, 'SELECT')
-	)
-	-- Exclude self
-	and c.relname <> '0002_auth_users_exposed'
+    d.refobjid = 'auth.users'::regclass
+    and d.deptype = 'n'
+    and c.relkind in ('v', 'm') -- v for view, m for materialized view
+    and n.nspname = 'public'
+    and (
+      pg_catalog.has_table_privilege('anon', c.oid, 'SELECT')
+      or pg_catalog.has_table_privilege('authenticated', c.oid, 'SELECT')
+    )
+    -- Exclude self
+    and c.relname <> '0002_auth_users_exposed'
 group by
-	c.relname, c.oid;
+    c.relname, c.oid;

lints/0003_auth_rls_initplan.sql

@@ -3,9 +3,9 @@ Usage of auth.uid(), auth.role() ... are common in RLS policies.
 
 A naive policy like
 
-	create policy "rls_test_select" on test_table
-	to authenticated
-	using ( auth.uid() = user_id );
+    create policy "rls_test_select" on test_table
+    to authenticated
+    using ( auth.uid() = user_id );
 
 will re-evaluate the auth.uid() function for every row. That can result in 100s of times slower performance
 https://supabase.com/docs/guides/database/postgres/row-level-security#call-functions-with-select
@@ -15,74 +15,74 @@ be executed exactly 1 time per query
 
 For example:
 
-	create policy "rls_test_select" on test_table
-	to authenticated
-	using ( (select auth.uid()) = user_id );
+    create policy "rls_test_select" on test_table
+    to authenticated
+    using ( (select auth.uid()) = user_id );
 
 NOTE:
-	This lint requires search_path = '' or 'auth' not in search_path;
-	because qual and with_check are dependent on search_path to determine if function calls include the "auth" schema
+    This lint requires search_path = '' or 'auth' not in search_path;
+    because qual and with_check are dependent on search_path to determine if function calls include the "auth" schema
 */
 
 
 create view lint."0003_auth_rls_initplan" as
 
 with policies as (
-	select
-		polrelid::regclass table_,
+    select
+        polrelid::regclass table_,
         pc.relrowsecurity is_rls_active,
-		polname as policy_name,
-		polpermissive as is_permissive, -- if not, then restrictive
-		(select array_agg(r::regrole) from unnest(polroles) as x(r)) as roles,
-		case polcmd
-			when 'r' then 'SELECT'
-			when 'a' then 'INSERT'
-			when 'w' then 'UPDATE'
-			when 'd' then 'DELETE'
-			when '*' then 'ALL'
-		end as command,
-		qual,
-		with_check
-	from
-		pg_policy pa
-		join pg_class pc
-			on pa.polrelid = pc.oid
-		join pg_namespace nsp
-			on pc.relnamespace = nsp.oid
-		join pg_policies pb
-			on pc.relname = pb.tablename
-			and nsp.nspname = pb.schemaname
-			and pa.polname = pb.policyname
+        polname as policy_name,
+        polpermissive as is_permissive, -- if not, then restrictive
+        (select array_agg(r::regrole) from unnest(polroles) as x(r)) as roles,
+        case polcmd
+            when 'r' then 'SELECT'
+            when 'a' then 'INSERT'
+            when 'w' then 'UPDATE'
+            when 'd' then 'DELETE'
+            when '*' then 'ALL'
+        end as command,
+        qual,
+        with_check
+    from
+        pg_policy pa
+        join pg_class pc
+            on pa.polrelid = pc.oid
+        join pg_namespace nsp
+            on pc.relnamespace = nsp.oid
+        join pg_policies pb
+            on pc.relname = pb.tablename
+            and nsp.nspname = pb.schemaname
+            and pa.polname = pb.policyname
 )
 select
-	'auth_rls_initplan' as name,
-	'WARN' as level,
-	'EXTERNAL' as facing,
-	'Detects if calls to auth.<function>() in RLS policies are being unnecessarily re-evaluated for each row' as description,
-	format(
-		'Table "%s" has a row level security policy "%s" that re-evaluates an auth.<function>() for each row. This produces suboptimal query performance at scale. Resolve the issue by replacing "auth.<function>()" with "(select auth.<function>())". See https://supabase.com/docs/guides/database/postgres/row-level-security#call-functions-with-select for more.',
-		table_,
-		policy_name
-	) as detail,
-	null as remediation,
-	null as metadata,
-	format('auth_rls_init_plan_%s_%s', table_, policy_name) as cache_key
+    'auth_rls_initplan' as name,
+    'WARN' as level,
+    'EXTERNAL' as facing,
+    'Detects if calls to auth.<function>() in RLS policies are being unnecessarily re-evaluated for each row' as description,
+    format(
+        'Table "%s" has a row level security policy "%s" that re-evaluates an auth.<function>() for each row. This produces suboptimal query performance at scale. Resolve the issue by replacing "auth.<function>()" with "(select auth.<function>())". See https://supabase.com/docs/guides/database/postgres/row-level-security#call-functions-with-select for more.',
+        table_,
+        policy_name
+    ) as detail,
+    null as remediation,
+    null as metadata,
+    format('auth_rls_init_plan_%s_%s', table_, policy_name) as cache_key
 from
     policies
 where
     is_rls_active
     and (
-		(
-			-- Example: auth.uid()
-			qual  ~ '(auth)\.(uid|jwt|role|email)\(\)'
-			-- Example: select auth.uid()
-			and lower(qual) !~ 'select\s+(auth)\.(uid|jwt|role|email)\(\)'
-		)
+        (
+            -- Example: auth.uid()
+            qual  ~ '(auth)\.(uid|jwt|role|email)\(\)'
+            -- Example: select auth.uid()
+            and lower(qual) !~ 'select\s+(auth)\.(uid|jwt|role|email)\(\)'
+        )
         or
-		(
-			-- Example: auth.uid()
-			with_check  ~ '(auth)\.(uid|jwt|role|email)\(\)'
-			-- Example: select auth.uid()
-			and lower(with_check) !~ 'select\s+(auth)\.(uid|jwt|role|email)\(\)'
-		)
+        (
+            -- Example: auth.uid()
+            with_check  ~ '(auth)\.(uid|jwt|role|email)\(\)'
+            -- Example: select auth.uid()
+            and lower(with_check) !~ 'select\s+(auth)\.(uid|jwt|role|email)\(\)'
+        )
     );