search

supabase / splinter

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

The lint test for function paths has a misleading description #84

Closed khera closed 3 months ago

khera commented 3 months ago

Improve documentation

Link

Add a link to the page which needs improvement (if relevant)

The description in lints/0011_function_search_path_mutable.sql and also https://supabase.com/docs/guides/database/database-linter?lint=0011_function_search_path_mutable

Describe the problem

Is the documentation missing? Or is it confusing? Why is it confusing?

The description claims that the path is mutable on the condition that it is not set to an empty string. This is not true. If you set the path to search_path = 'public' the path is no longer mutable and it is safe.

Describe the improvement

A clear and concise description of the improvement.

One of two:

  1. Change the test to see if the path is set to any value
  2. Change the description of the linter test to clearly state that the test checks only for empty search path.

The detailed description implies that your expectation is the second one, but the text is not entirely consistent on this point.

Additional context

Add any other context or screenshots that help clarify your question.

olirice commented 3 months ago

You're absolutely right that to avoid the security risk its only necessary to specify ANY value search path.

Forcing it to an empty string is admittedly an opinionated stance. Fully qualified names are always the the safest wrt changes in user search path and to pause/restore. Since schemas are only ever 1 level deep its not too much of an eyesore and it lets us eliminate a bug class.

Its an opinion we may consider walking back in the future, but for now we'd prefer to enforce the solution that yields the most reliable and debuggable projects

khera commented 3 months ago

You should change the description to match that opinion, so as to reduce confusion. It took me a while to figure out why my path did not meet the described condition.

Thanks for replying.