On Wed, 29 Jul 2026 at 08:16, Hayato Kuroda (Fujitsu) <[email protected]> wrote: > > Dear Shlok, > > > While testing another patch, I found a bug related to subscriptions > > created using a foreign server. > > Good catch. > > > This happens because AlterSubscription() calls: > > GetSubscription(subid, false, orig_conninfo_needed, false) > > with conninfo_aclcheck set to false, so the ACL check on the > > associated foreign server is skipped. > > Agree. There is an ongoing discussion to weaken the validation at DDL phase > [1], > but IIUC it does not match with this issue. REFRESH PUBLICATION command needs > to > refer the conninfo to connect to the publisher, so validation is mandatory. > I also noticed that the thread [1] does not address this issue.
> > The attached patch fixes this by enabling conninfo_aclcheck for > > these ALTER SUBSCRIPTION operations, ensuring that permissions on > > the foreign server are validated before connecting to the publisher. > > Thoughts? > > I think for ADD/DROP/SET PUBLICATION commands, ACL check is not needed > non-refresh > case. For these cases, backends won't directly connect to the publisher > server: > apply worker will check privileges and raise the ERROR. Can you set > orig_conninfo_aclchk to true only for REFRESH case? > > [1]: > https://www.postgresql.org/message-id/e96efe16fb47fcca4ae0cd0157fb6bc662470712.camel%40j-davis.com Thanks for reviewing the patch. I have updated the patch and attached the v2 patch. Thanks, Shlok Kyal
v2-0001-Check-foreign-server-permissions-for-ALTER-SUBSCR.patch
Description: Binary data
