On Tue, Jul 09, 2024 at 05:47:36PM -0700, Jeff Davis wrote: > On Tue, 2024-07-09 at 15:20 +0900, Michael Paquier wrote: > > On Sun, Jun 30, 2024 at 03:23:44PM -0700, Noah Misch wrote:
> > Hmm. Is RestrictSearchPath() something that we should advertise more > > strongly, thinking here about extensions that call NewGUCNestLevel()? > > That would be really easy to miss, and it could have bad > > consequences. > > I know that this is not something that's published in the release > > notes, but it looks like something sensible to have, though. > > The pattern also involves SetUserIdAndSecContext(). Perhaps we could > come up with a wrapper function to better encapsulate the general > pattern? Worth a look. usercontext.c has an existing wrapper for a superuser process switching to an untrusted user. It could become the home for another wrapper targeting MAINTAIN-relevant callers. > > > While "not necessary for security", ExecCreateTableAs() should do > > > it for the > > > same reason it calls NewGUCNestLevel(). > > > > +1. > > Do you have a suggestion about how that should be done? > > It's not trivial, because the both creates the table and populates it > in ExecutorRun. For table creation, we need to use the original > search_path, but we need to use the restricted search_path when > populating it. > > I could try to refactor it into two statements and execute them > separately, or I could try to rewrite the statement to use a fully- > qualified destination table before execution. Thoughts? Those sound fine. Also fine: just adding a comment on why creation namespace considerations led to not doing it there.
