Re: RI fastpath misses checking EXECUTE on functions

Hi Matheus,

On Thu, Sep 24, 2026 at 11:45 PM Matheus Alcantara
<matheusssilv97(at)gmail(dot)com> wrote:
>
> On 24/09/26 00:42, Amit Langote wrote:
> > Hi,
> >
> > It was pointed out to me offlist by Nikolay Samokhvalov that the RI
> > fast path (ri_FastPathCheck) doesn't check EXECUTE privilege on the
> > functions it invokes on the FK values. There are two such functions:
> > the equality operator's function, which goes into the ScanKey the
> > index AM evaluates, and the implicit cast function, which is used when
> > the FK value must be converted to the PK column's type. In the SPI
> > path's case, because the operator appears in the generated query's
> > WHERE clause and the cast as $1::pktype, ExecutorStart checked EXECUTE
> > on both as part of initializing the query, as the referenced table's
> > owner. So while I added ri_CheckPermissions() to parallel the
> > executor's ExecCheckPermissions(), I missed the non-relation
> > permission checks that occur as part of expression initialization.
> >
> > The attached patch adds ri_CheckFunctionPermissions(), which does
> > object_aclcheck(ProcedureRelationId, ..., ACL_EXECUTE) plus
> > InvokeFunctionExecuteHook() for each of the two functions, and calls
> > it once the fast-path metadata is populated. That puts the checks at
> > the same point as the existing relation permission check.
> >
> > Regression tests are included for both functions. They reach the fast
> > path through per-row validation of a new constraint, and repeat each
> > case against a partitioned referenced table, which takes the SPI path,
> > to establish that the two behave identically.
> >
> > I'll add an open item. I would like to get this committed by EOW if
> > there are no comments.
>
> I take a look on the patch, it looks good to me. Just a minor
> comment:

Thanks for looking.

> ri_CheckFunctionPermissions(riinfo, fpmeta) passes both when fpmeta
> == riinfo->fpmeta. I'm wondering if we could just pass riinfo?

That's just for consistency with build_index_scankeys(); it isn't
needed, so I don't feel strongly either way.

> IIUC this patch only fix the case for FastPath without batching right?
> Since batching is still on master, I'm wondering if we could also fix
> it. See attached patch (v2-0001 is your v1-0001).

I've left the batch code alone because I intend to revert it from
master too sometime next week. Thanks for the patch, though.

I have attached a new version where I polished
ri_CheckFunctionPermissions()'s comment and the commit message. I
would like to commit it tomorrow if there are no more comments.

--
Thanks, Amit Langote