Hi, On Thu, Jul 16, 2026 at 8:32 PM Amit Langote <[email protected]> wrote: > On Mon, Jul 6, 2026 at 6:05 AM Noah Misch <[email protected]> wrote: > > I reviewed the ri_Fast* family of commits. This thread covers $SUBJECT and > > some other findings. Feel free to fork more threads as needed. > > ==== ri_CheckPermissions() does not cover hooks / sepgsql > > > > > The ri_CheckPermissions() function performs schema USAGE and table > > > SELECT checks, matching what the SPI path gets implicitly through > > > the executor's permission checks. > > > > It doesn't call ExecutorCheckPerms_hook or object_access_hook (via > > e.g. InvokeFunctionExecuteHook), so sepgsql doesn't get control. That might > > be okay if called out in the sepgsql documentation. > > You're right that the fast path doesn't reach ExecutorCheckPerms_hook > or the object access hooks, so sepgsql doesn't get control where it > would on the SPI path. Let me think through what restoring that would > mean, because I'm not sure it's the right goal. > > On the SPI path these hooks fired as a consequence of the check > running through the executor. For the per-row validation path in > particular, that meant a hook invocation per row checked, so a foreign > key validation over a large table would have produced an audit record > per row. I don't think that was ever an intended sepgsql behavior; it > seems more like a side effect of the execution path. Reproducing it > deliberately on the fast path doesn't seem necessary IMHO. > > Invoking the hooks would also mean synthesizing an RTEPermissionInfo > list outside any planned query, which is the kind of executor > scaffolding the fast path is trying to avoid. > > Given that, my inclination is to leave it as is rather than wire up > the hooks. I'm also unsure a sepgsql doc note is the right place, > since it might read as a limitation we intend to close rather than an > implementation detail. But I'd rather get your read before deciding. > If you or anyone else thinks the bypass is worth addressing or noting > somewhere, I'm happy to work out how.
On thinking about this more, I think it was wrong to say that RI checks going through ExecutorCheckPerms_hook is an accidental detail. These checks do access relations, and anyone who relies on the hook to track every relation access, for example, won't see these, because the fast path doesn't call it and so diverges from the SPI path there. In light of the various recent fixes whose point was to bring the fast path's behavior in line with the SPI path's, I'd like to propose changing it to call the hook as well. Patch attached. I'll add an open item. -- Thanks, Amit Langote
0001-Route-RI-fast-path-permission-check-through-ExecChec.patch
Description: Binary data
