Re: Wrong security context for deferred triggers?

Laurenz Albe <laurenz.albe@cybertec.at>

From: Laurenz Albe <laurenz.albe@cybertec.at>

To: Pavel Stehule <pavel.stehule@gmail.com>

Cc: pgsql-hackers@lists.postgresql.org

Date: 2024-10-18T13:24:29Z

Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

Doc: improve description of which role runs a trigger.
- c37be39a74b2 18.0 landed
Change role names used in trigger test.
- 4b05ebf0957b 18.0 landed
Ensure that AFTER triggers run as the instigating user.
- 01463e1cccd3 18.0 landed
Reverse the search order in afterTriggerAddEvent().
- 7921927bbb9d 18.0 landed

Attachments

v2-0001-Make-AFTER-triggers-run-with-the-correct-user.patch (text/x-patch) patch v2-0001

On Fri, 2024-10-18 at 11:32 +0200, Pavel Stehule wrote:
> pá 18. 10. 2024 v 10:20 odesílatel Laurenz Albe <laurenz.albe@cybertec.at> napsal:
> > On Fri, 2024-10-18 at 07:47 +0200, Pavel Stehule wrote:
> > > Without deeper checks I don't like using GetUserNameFromId for checking the validity of a role.
> > > 
> > > Maybe it is better to use own read of syscache or wrap SetUserIdAndSecContext to do this check.
> > 
> > I agree; it was just the simplest way I could make it happen.  It is ugly to allocate and
> > return the user name, since we don't really need it.
> > 
> > I could write a dedicated function to check the existence of a user.
> 
> +1

The check is so simple that I didn't write a dedicated function.
Instead, I put the catalog search into the code directly.

> > 
> > The trigger queue exists only in memory, and PostgreSQL tracks dependencies
> > only between persisted objects.  Do you think that I should add a sentence
> > like that to the comment?
> 
> yes, please. I think so it is not too intuitive. Inside a context of this patch
> it is ok, but without knowledge of this context, can be strange, why some role used
> for trigger can be invalid, although the transaction is not fully finished yet.

I tried to improve the patch along these lines.

Attached is a new version.

Thanks for the review!

Yours,
Laurenz Albe