Unset bootstrap credentials before exec-ing the server

[mkindahl]

POSTGRES_PASSWORD (and related vars) are only needed during initdb and the temporary-server initialisation phase. After that they serve no purpose, but remain in the process environment for the entire lifetime of the container, where any loaded C extension can read them via environ.

Unsetting them immediately before the final exec ensures the running PostgreSQL server process starts with a clean environment.

[tianon]

This should move up inside the final fi above (so it lives inside the if [ "$1" = 'postgres' ] && ! _pg_want_help "$@"; then block) so that it's only unset for postgres itself. I think I'd also prefer to do something like unset "${!POSTGRES_@}" so it catches anything else too (PostgreSQL itself doesn't use this naming scheme, to my knowledge).

Suggested change

	fi
	unset POSTGRES_PASSWORD POSTGRES_USER POSTGRES_DB POSTGRES_INITDB_ARGS
	unset "${!POSTGRES_@}"
	fi

It also needs to be applied across all the templated files (./apply-templates.sh).

[mkindahl]

This should move up inside the final fi above (so it lives inside the if [ "$1" = 'postgres' ] && ! _pg_want_help "$@"; then block) so that it's only unset for postgres itself. I think I'd also prefer to do something like unset "${!POSTGRES_@}" so it catches anything else too (PostgreSQL itself doesn't use this naming scheme, to my knowledge).

I could not find any usage of that pattern (for environment variables) in the Postgres source code. Most of the getenv() calls have hard-coded names not matching this, or are in libpq or in binaries (and does seem to use that either, but these are variables).

It also needs to be applied across all the templated files (./apply-templates.sh).

Done.

[tianon]

This is technically a breaking change, but maybe it's OK?
(COPY ... FROM PROGRAM '...')

[mkindahl]

This is technically a breaking change, but maybe it's OK? (COPY ... FROM PROGRAM '...')

Not sure what you're referring to here. Where would that SQL be added?

[tianon]

I mean that end users can run SQL commands that read these environment variables from the server's environment via running explicit server-side programs with COPY ... FROM PROGRAM 'echo $POSTGRES_DB' for example, so technically a breaking change, but I don't know whether that's really all that common or worth worrying about in any way.

I also don't know if that's the only way these might have been historically exposed to end-users or PostgreSQL itself (environment variables in config, etc); just trying to watch out for Hyrum's Law because it tends to bite us every time we harden something like this. 😂

[yosifkit]

@tianon, there could be extensions that read the environment as well (like @mkindahl wrote about on their blog or the envvar extension). Even a non-superuser can read %ENV via PL/Perl and os.getenv or os.environ from PL/python.

So, this seems good to me, but I agree that we will likely have someone affected.

[mkindahl]

I mean that end users can run SQL commands that read these environment variables from the server's environment via running explicit server-side programs with COPY ... FROM PROGRAM 'echo $POSTGRES_DB' for example, so technically a breaking change, but I don't know whether that's really all that common or worth worrying about in any way.

Ah, I see. 😄 Yes, that scenario would break, but only for reading these variables.

I also don't know if that's the only way these might have been historically exposed to end-users or PostgreSQL itself (environment variables in config, etc); just trying to watch out for Hyrum's Law because it tends to bite us every time we harden something like this. 😂

Yeah, that is always a risk. 😄

[mkindahl]

@tianon, there could be extensions that read the environment as well (like @mkindahl wrote about on their blog or the envvar extension). Even a non-superuser can read %ENV via PL/Perl and os.getenv or os.environ from PL/python.

So, this seems good to me, but I agree that we will likely have someone affected.

Thank you. Who normally hits the merge button? Submitter or reviewers? It is different everywhere.