On Thu, 9 Jun 2022 at 16:12, Peter Geoghegan <pg@bowt.ie> wrote:
>
> Presumably there is still significant value in detecting cases where
> some user-defined code provably does the wrong thing. Especially by
> targeting mistakes that experience has shown are relatively common.
> That's what the search_path case seems like to me.
By "relatively common" I think we're talking "nigh universal". Afaics
there are no warnings in the docs about worrying about search_path on
IMMUTABLE functions. There is for SECURITY DEFINER but I have to admit
I wasn't aware myself of all the gotchas described there.
For that matter.... the gotchas are a bit .... convoluted....
If you leave out pg_catalog from search_path that's fine but if you
leave out pg_temp that's a security disaster. If you put pg_catalog in
it better be first or else it might be ok or might be a security issue
but when you put pg_temp in it better be last or else it's
*definitely* a disaster. $user is in search_path by default and that's
fine for SECURITY DEFINER functions but it's a disaster for IMMUTABLE
functions...
I kind of feel like perhaps all the implicit stuff is unnecessary
baroque frills. We should just put pg_temp and pg_catalog into the
default postgresql.conf search_path and assume users will keep them
there. And I'm not sure why we put pg_temp *first* -- I mean it sort
of seems superficially sensible but it doesn't seem like there's any
real reason to name your temporary tables the same as your permanent
ones so why not just always add it last?
I've attached a very WIP patch that implements the checks I'm leaning
towards making (as elogs currently). They cause a ton of regression
failures so probably we need to think about how to reduce the pain for
users upgrading...
Perhaps we should automatically fix up the current search patch and
attach it to functions where necessary for users instead of just
whingeing at them....
