fix(upgrade): quote MOSHCODE_HOME and installer URL in the self-upgrade shell#45
Merged
Merged
Conversation
…de shell
selfSpec() embedded both values raw in a `sh -c` command line. An apostrophe
in the install path (e.g. /Users/o'brien/moshcode) broke `moshcode upgrade`
with a shell syntax error, and a hostile path could break out of the quotes
and inject extra shell into the update command. POSIX single-quote escaping
(' -> '\'') fixes both; selfSpec is exported with injectable home/url so the
quoting is unit-tested end to end through a real sh.
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
2 participants
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.
Bug
selfSpec()insrc/upgrade.mjsinterpolatedMOSHCODE_HOMEand the installer URL raw inside ansh -ccommand line, wrapped in single quotes:`export MOSHCODE_HOME='${MOSHCODE_HOME}'; curl -fsSL ${SELF_URL} | sh -s -- update`Two consequences:
'(e.g./Users/o'brien/moshcode),moshcode upgradedies with a shell syntax error:'…'; <cmd>; #breaks out of the quotes and runs extra shell inside the self-update command.Repro
Fix
shQ()— POSIX-safe single-quoting: wrap in'…', escape every'as'\''.MOSHCODE_HOMEand the installer URL.selfSpec(home, url)with injectable params so it is testable.Tests
New tests in
test/upgrade.test.mjsrun the actual generated command with a stubbedcurlon PATH:'; touch /tmp/pwned; #) cannot create its marker file.Both fail against the original code and pass with the fix. Full suite: 160/160.