supabase / splinter

Supabase Postgres Linter: Performance and Security Advisors
https://supabase.github.io/splinter/
83 stars 7 forks source link

Feature request: configurable additional functions for `0003 auth rls initplan` rule #79

Closed fnimick closed 4 months ago

fnimick commented 4 months ago

Please note that I didn't use the link to open a discussion in the supabase org since this is a feature request specific to splinter itself. Feel free to close and direct me there instead if this is not appropriate.

The 0003 auth rls initplan rule is great for catching performance issues - but if you have a stable function that you define yourself that uses an auth function, in a schema other than auth, it would be great for that to also be included in these checks.

Perhaps the function name could be required to have a specific prefix, or maybe there could be configuration exposed to allow this to work?

An example function:

create or replace function public.jwt_is_organization_member(organization_id uuid)
returns boolean language plpgsql stable
set search_path = ''
as $function$
DECLARE retval bool;
BEGIN 
  IF session_user = 'authenticator' OR session_user = 'rls_user' THEN
    if public.jwt_is_expired() then return false;
    end if;
    select coalesce(
        auth.jwt()->'app_metadata'->'organizations' ? organization_id::text,
        false
      ) into retval;
    return retval;
  ELSE -- not a user session, probably being called from a trigger or something
    return true;
  END IF;
END;
$function$;

This function should also only ever be called with (select ...) in RLS policies, and it would be very nice to be able to configure splinter to alert on that.

olirice commented 4 months ago

Our ability to introspect function definitions to understand their intent through static analysis is limited. If you're confident that you've implemented your security correctly in a re-usable function, you can safely ignore the lint, but we'd like to have exactly one recommended method for lints / the supabase style of development rather than introduce configurability.

Its worth noting that simple RLS policies with:

can inline on queries, which tends to make them the most performant. The example you provided would be a barrier to that optimization

fnimick commented 4 months ago

I should clarify, there currently is no linter error no matter how this function is used. I'm suggesting adding the ability to mark additional functions as "should be called using (select ...), in addition to functions in the auth namespace.

olirice commented 4 months ago

thanks, I did understand that. At this point we're trying to avoid configurability to encourage some norms across supabase projects