Preserve environment values containing spaces in the launcher carry list - #1468
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes an environment-carry bug in src/launcher.sh where passlisted variables whose values contain spaces were being word-split when invoking env -i, causing env to misinterpret parts of the value as the command to execute (e.g., NODE_OPTIONS with multiple flags). It also improves sensitive-environment redaction in verbose startup logging in src/entrypoint.sh.
Changes:
- Switch the environment “carry list” in
src/launcher.shfrom a single unquoted string to a bash array soNAME=VALUEpairs (including spaced values) are passed toenvas intact arguments. - Add an empty-array-safe array expansion for compatibility with
set -o nounsetbehavior on older bash versions. - Widen
CONTAINER_VERBOSEenv-dump redaction insrc/entrypoint.shto redact variables whose names contain sensitive keywords anywhere (PASSWORD/KEY/SECRET/TOKEN/SALT).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/launcher.sh | Builds the carry set as a bash array to preserve env var values containing spaces when calling env -i. |
| src/entrypoint.sh | Expands verbose env redaction to catch sensitive keywords anywhere in the variable name. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
felddy
left a comment
felddy
left a comment
There was a problem hiding this comment.
Looks good. Please rebase and we'll get this merged. Thanks for the contribution.
567c126 to
7c142d7
Compare
The carry list was expanded unquoted into 'env -i', so any passlisted
variable whose value contains a space breaks the exec command line.
Example: NODE_OPTIONS="--max-old-space-size=4096 --enable-source-maps"
splits into 'NODE_OPTIONS=--max-old-space-size=4096' plus a stray
'--enable-source-maps', which env treats as the program to run — exec
fails and the container crash-loops through backoff instead of starting
Foundry.
Build the carry set as a bash array instead; each NAME=VALUE reaches
env as one argument. The ${arr[@]+...} expansion guards the
empty-array case under 'set -o nounset' on bash <4.4.
Repro:
NODE_OPTIONS='--a --b' bash -c '<old carry loop>; env -i $LIST env'
-> env: '--b': No such file or directory
The redaction regex '(.*PASSWORD|KEY.*)=.*' misses variables where the sensitive token is not adjacent to '=': FOUNDRY_PASSWORD_SALT, and any *_SECRET/*_TOKEN names, are printed in clear in the CONTAINER_VERBOSE environment dump. Match the sensitive keyword anywhere in the variable name instead, and add SECRET/TOKEN/SALT to the list.
7c142d7 to
74efd75
Compare
|
Note for the record: I cherry-picked both commits onto the current |
🗣 Description
src/launcher.shbuilds the clean-environment carry list as asingle string and expands it unquoted into env -i. Any
passlisted variable whose value contains a space word-splits
at exec time.
This PR builds the carry set as a bash array instead, so each
NAME=VALUE reaches env as a single argument. The
${arr[@]+"${arr[@]}"} expansion guards the empty-array case
under set -o nounset on bash < 4.4. ENV_VAR_PASSLIST_REGEX is
unchanged.
A second, related commit widens the CONTAINER_VERBOSE
env-dump redaction in src/entrypoint.sh: the current pattern
(.PASSWORD|KEY.)=.* only matches the keyword adjacent to =,
so values like FOUNDRY_PASSWORD_SALT (and any
_SECRET/_TOKEN) print in clear. The pattern now matches
PASSWORD|KEY|SECRET|TOKEN|SALT anywhere in the variable name.
The commits are independent and cherry-pickable if you'd
prefer to take only one.
💭 Motivation and context
With e.g. NODE_OPTIONS='--max-old-space-size=4096
--enable-source-maps' (matches the ^NODE_.+$ passlist), env
receives --enable-source-maps as a standalone argument and
treats it as the program to execute, failing with env:
'--enable-source-maps': No such file or directory. The exec
never reaches Node and the container crash-loops through the
backoff handler.
Resolves #1467
🧪 Testing
ghcr.io/felddy/foundryvtt:release with the spaced
NODE_OPTIONS above (log output attached to Container crash-loops when a carried env var contains spaces (e.g. NODE_OPTIONS with two flags) #1467).
as a single intact variable, and the empty-carry edge case
under set -o nounset.
NGROK_AUTH_TOKEN, MY_SECRET, plus a non-sensitive HOME
control (only the control prints its value).
ENV_VAR_PASSLIST_REGEX, which is unchanged; shellcheck
reports no new findings on either file.
(same versions/flags) on both changed files: clean. Full
pytest suite: 36/36 pass on this branch.
✅ Pre-approval checklist
creep!