Date: Fri, 12 Jul 2024 16:11:49 -0700
From: Noah Misch <noah@leadboat.com>
To: Jeff Davis <pgsql@j-davis.com>
Cc: Michael Paquier <michael@paquier.xyz>,
	Nathan Bossart <nathandbossart@gmail.com>,
	pgsql-hackers@postgresql.org, Joe Conway <mail@joeconway.com>,
	Robert Haas <robertmhaas@gmail.com>
Subject: Re: MAINTAIN privilege -- what do we need to un-revert it?
Message-ID: <20240712231149.0d.nmisch@google.com>
References: <20240223213055.GA1805646@nathanxps13>
 <196d1c86d43a147e777bb74159774f255dbe73e3.camel@j-davis.com>
 <20240228165523.GA2435539@nathanxps13>
 <31ae1bf73e8a1dae3ee7ef2dd43a9bbda1e1ab86.camel@j-davis.com>
 <fe5d572b02d07d8e9940f9b518d900708e0b0166.camel@j-davis.com>
 <20240630222344.db.nmisch@google.com>
 <ZozWqg0XCkiPCnop@paquier.xyz>
 <23fac3b2a722373a52b92a12ee0f500821450f97.camel@j-davis.com>
 <20240711125207.98.nmisch@google.com>
 <d97b888293e7588164b602850969a4b7d9550cc5.camel@j-davis.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <d97b888293e7588164b602850969a4b7d9550cc5.camel@j-davis.com>
User-Agent: Mutt/2.2.12 (2023-09-09)
Archived-At: 
 <https://www.postgresql.org/message-id/20240712231149.0d.nmisch%40google.com>
Precedence: bulk

On Fri, Jul 12, 2024 at 02:50:52PM -0700, Jeff Davis wrote:
> On Thu, 2024-07-11 at 05:52 -0700, Noah Misch wrote:
> > > 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.
> 
> Attached. 0002 separates the CREATE MATERIALIZED VIEW ... WITH DATA
> into (effectively):
> 
>    CREATE MATERIALIZED VIEW ... WITH NO DATA;
>    REFRESH MATERIALIZED VIEW ...;
> 
> Using refresh also achieves the stated goal more directly: to (mostly)
> ensure that a subsequent REFRESH will succeed.
> 
> Note: the creation itself no longer executes in a security-restricted
> context, but I don't think that's a problem. The only reason it's using
> the security restricted context is so the following REFRESH will
> succeed, right?

Right, that's the only reason.

> @@ -346,13 +339,21 @@ ExecCreateTableAs(ParseState *pstate, CreateTableAsStmt *stmt,
>  		PopActiveSnapshot();
>  	}
>  
> -	if (is_matview)
> +	/*
> +	 * For materialized views, use REFRESH, which locks down
> +	 * security-restricted operations and restricts the search_path.
> +	 * Otherwise, one could create a materialized view not possible to
> +	 * refresh.
> +	 */
> +	if (do_refresh)
>  	{
> -		/* Roll back any GUC changes */
> -		AtEOXact_GUC(false, save_nestlevel);
> +		RefreshMatViewStmt *refresh = makeNode(RefreshMatViewStmt);
>  
> -		/* Restore userid and security context */
> -		SetUserIdAndSecContext(save_userid, save_sec_context);
> +		refresh->relation = into->rel;
> +		ExecRefreshMatView(refresh, pstate->p_sourcetext, NULL, qc);
> +
> +		if (qc)
> +			qc->commandTag = CMDTAG_SELECT;
>  	}

Since refresh->relation is a RangeVar, this departs from the standard against
repeated name lookups, from CVE-2014-0062 (commit 5f17304).