fix: quote environment and Postgres paths in Docker init - #1486
Open
lbellows wants to merge 1 commit into
Open
Conversation
Exporting /var/run/environment via unquoted command substitution broke values with spaces (database URLs, passwords) and could mis-parse filenames. printf without %s treated config paths as format strings. Quote Postgres data paths throughout init/upgrade and identifier-quote the superuser rename during pg_upgrade.
✅ Deploy Preview for viseron canceled.
|
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
It changes database upgrade/role-renaming logic in the pg_upgrade path without an end-to-end container upgrade run to validate behavior.
Pull request overview
This PR hardens the container init and Postgres startup/upgrade scripts against shell parsing pitfalls by correctly quoting environment exports and filesystem paths, preventing breakage when values contain spaces, glob characters, or % sequences.
Changes:
- Export
/var/run/environment/*variables safely by quoting filenames and assignment values. - Use
printf '%s'when writing environment values to avoid unintended format-string expansion. - Quote Postgres binary/data paths throughout init/upgrade, and identifier-quote the superuser rename during
pg_upgrade.
File summaries
| File | Description |
|---|---|
| rootfs/helpers/set_env.sh | Makes env-file export robust to spaces/globs and skips non-files. |
| rootfs/etc/services.d/postgres/run | Quotes config/log paths and Postgres command/data dir invocation. |
| rootfs/etc/cont-init.d/80-postgres | Quotes Postgres paths across init/upgrade and safely identifier-quotes role rename for pg_upgrade. |
| rootfs/etc/cont-init.d/40-set-env-vars | Writes env values with printf '%s' to avoid % being treated as a format string. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/var/run/environmentvia unquotedexport $(basename $file)=$(cat $file)broke values with spaces (database URLs, passwords) and could mis-parse filenames.printf $PATHtreated%in config paths as a format string.pg_upgrade.Test plan
bash -nonset_env.sh,40-set-env-vars,80-postgres, andpostgres/run|printf '%s'preserves a path containing%s; unquotedprintfdoes not"$in a role name is doubled for a Postgres identifier