Horton

Re: Possibility to disable `ALTER SYSTEM`

Robert Haas <robertmhaas@gmail.com>

View thread
From: Robert Haas <robertmhaas@gmail.com>
To: Bruce Momjian <bruce@momjian.us>
Cc: Jelte Fennema-Nio <postgres@jeltef.nl>, Andrew Dunstan <andrew@dunslane.net>, Tom Lane <tgl@sss.pgh.pa.us>, Heikki Linnakangas <hlinnaka@iki.fi>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>, Daniel Gustafsson <daniel@yesql.se>, Joel Jacobson <joel@compiler.org>, Gabriele Bartolini <gabriele.bartolini@enterprisedb.com>, Magnus Hagander <magnus.hagander@redpill-linpro.com>, Maciek Sakrejda <m.sakrejda@gmail.com>
Date: 2024-03-27T15:10:31Z
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 →

  1. Add allow_alter_system GUC.

  2. Rename COMPAT_OPTIONS_CLIENT to COMPAT_OPTIONS_OTHER.

  3. Remove support for version-0 calling conventions.

On Wed, Mar 27, 2024 at 11:01 AM Bruce Momjian <bruce@momjian.us> wrote:
> Uh, the above is clearly wrong.  I think you mean "off" on the second line.

Woops. When the name changed from externally_managed_configuration to
allow_alter_system, the sense of it was reversed, and I guess Jelte
missed flipping the documentation references around. I likely would
have made the same mistake, but it's easily fixed.

> > +
> > +       <para>
> > +        Note that this setting cannot be regarded as a security feature. It
> > +        only disables the <literal>ALTER SYSTEM</literal> command. It does not
> > +        prevent a superuser from changing the configuration remotely using
>
> Why "remotely"?

This wording was suggested upthread. I think the point here is that if
the superuser is logging in from the local machine, it's obvious that
they can do whatever they want. The point is to emphasize that a
superuser without a local login can, too.

> "its"?

Yeah, that seems like an extra word.

> > +        some outside mechanism. In such environments, using <command>ALTER
> > +        SYSTEM</command> to make configuration changes might appear to work,
> > +        but then may be discarded at some point in the future when that outside
>
> "might"

This does not seem like a mistake to me. I'm not sure why you think it is.

> > +        mechanism updates the configuration. Setting this parameter to
> > +        <literal>on</literal> can help to avoid such mistakes.
> > +       </para>
>
> "off"

This is another case that needs to be fixed now that the sense of the
GUC is reversed. (We'd better make sure the code has the test the
right way around, too.)

> Is this really a patch we think we can push into PG 17. I am having my
> doubts.

If the worst thing that happens in PG 17 is that we push a patch that
needs a few documentation corrections, we're going to be doing
fabulously well.

-- 
Robert Haas
EDB: http://www.enterprisedb.com