fix: don't let LOCALSTACK_PATH clobber emulator PATH#378
Open
gtsiolis wants to merge 2 commits into
Open
Conversation
…lator env (DEVX-984) The emulator image's docker-entrypoint.sh strips the LOCALSTACK_ prefix from environment variables and re-exports the remainder, so a host shell var like LOCALSTACK_PATH forwarded by filterHostEnv became PATH inside the emulator and killed startup with 'mkdir: command not found'. Drop LOCALSTACK_* vars whose stripped name collides with a critical runtime variable (PATH, HOME, LD_PRELOAD, LD_LIBRARY_PATH, PYTHONPATH, PYTHONHOME) and emit a warning so the drop is visible to the user.
gtsiolis
added
semver: patch
docs: skip
gtsiolis
changed the title
gtsiolis
changed the title
…ocklist (DEVX-984) Folds in the two improvements from #379: a multi-line value is split by the entrypoint's line-oriented env pipeline and can inject rogue exports (e.g. an embedded 'LOCALSTACK_PATH=' line blanks PATH), and IFS/SHELL/ ENV/BASH_ENV join the reserved set (the entrypoint sources the stripped exports mid-script, so IFS corrupts its word splitting). Unlike #379, both drops surface a warning instead of happening silently. Co-Authored-By: Claude <noreply@anthropic.com>
gtsiolis
force-pushed
the
devx-984-lstk-exits-unexpectedly-with-mkdir-command-not-found
branch
from
393cc72 to
695098d
Compare
Member
Author
|
I'd appreciate a check from @anisaoshafi before merging. 🏓 |
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
1 participant
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.
Motivation
Reported in DEVX-984 (Slack, patrick.lafond):
lstk startdied with/usr/local/bin/docker-entrypoint.sh: line 22: mkdir: command not foundon any config, tag, and lstk version, but only on one machine. Root cause since confirmed by the reporter: aLOCALSTACK_PATHvar exported in his zsh config (pointing agents at a localstack source checkout);env -u LOCALSTACK_PATH lstkstarts cleanly.Why it breaks: since #161 lstk forwards every host
LOCALSTACK_*env var into the emulator. The emulator image'sdocker-entrypoint.shstrips theLOCALSTACK_prefix and re-exports the remainder (onlyLOCALSTACK_HOST/LOCALSTACK_HOSTNAMEare excluded), soLOCALSTACK_PATH=/home/user/repos/localstackbecomesexport PATH=/home/user/repos/localstackinside the emulator. The entrypoint itself is started by absolute path, so the first command that needsPATHismkdir— exactly the reported failure — andset -eo pipefailkills startup.Changes
filterHostEnvdropsLOCALSTACK_*vars whose prefix-stripped name collides with a critical runtime variable:PATH,HOME,SHELL,IFS,ENV,BASH_ENV,LD_PRELOAD,LD_LIBRARY_PATH,PYTHONPATH,PYTHONHOME(criticalContainerVar). Matching is on the exact stripped name (LOCALSTACK_PATHFINDERstill forwards);LOCALSTACK_HOSTNAMEstill forwards since the entrypoint excludes it from stripping.filterHostEnvalso drops forwarded values containing a newline/CR: the entrypoint re-exports vars through a line-orientedenv | sedpipeline, so an embedded line likeLOCALSTACK_PATH=injects a rogueexport PATH=that blanksPATH— a second, independent way to hit the same crash (adopted from fix: don't forward host LOCALSTACK_* env that clobbers container PATH #379).droppedHostEnv{name, overrides}andStartemits a warning for each, with the reason (…it would override PATH inside the emulator/…its value contains line breaks), so neither drop is silent. The long-standingLOCALSTACK_AUTH_TOKENdrop stays silent as before.Testing
TestFilterHostEnvunit test covering both drop reasons and the reported drop metadata (fails before the fix).TestStartCommandPassesCIAndLocalStackEnvVarsintegration test to start withLOCALSTACK_PATHset: before the fix the emulator dies exactly as reported; after it, start succeeds, the warning is printed, and the var is absent from the container env. Needs Docker +LOCALSTACK_AUTH_TOKEN, so it runs in CI.Supersedes #379 (its multiline guard and wider reserved set are folded in here, with warnings added on top).
A follow-up worth filing separately: excluding
PATHand friends from prefix-stripping in the image entrypoint itself, which would also protect compose users who exportLOCALSTACK_PATHwithout lstk involved.Fixes DEVX-984