From 581317d05c7c27848c439f57e8bdb6e4bef2511d Mon Sep 17 00:00:00 2001 From: Jordan Harband Date: Sat, 14 Mar 2026 14:41:11 -0700 Subject: [PATCH] [Fix] `install.sh`: fix POSIX compliance, printf format strings, and profile detection - Use `=` instead of `==` for string comparison (POSIX compliance) - Use `printf '%b'` instead of variable as format string (prevents `%` characters in paths from being interpreted as format specifiers) - Fix `TRIED_PROFILE` to reference `PROFILE` instead of `NVM_PROFILE` which is known to be empty at that point Bugs introduced in https://github.com/nvm-sh/nvm/commit/a24ff3e605d72ce9480b2fad2d4c93f0093fa306, https://github.com/nvm-sh/nvm/commit/b6f1c156da0a998442c1c0b638082390118085ce, and https://github.com/nvm-sh/nvm/commit/a461a0fffcd9163726f4d652ca974d97a1097356 (PR https://github.com/nvm-sh/nvm/pull/1605). --- install.sh | 12 ++-- .../nvm_do_install_printf_format | 70 +++++++++++++++++++ .../nvm_do_install_tried_profile | 68 ++++++++++++++++++ 3 files changed, 144 insertions(+), 6 deletions(-) create mode 100755 test/install_script/nvm_do_install_printf_format create mode 100755 test/install_script/nvm_do_install_tried_profile diff --git a/install.sh b/install.sh index 04d75ce..4aea547 100755 --- a/install.sh +++ b/install.sh @@ -216,7 +216,7 @@ nvm_install_node() { local CURRENT_NVM_NODE CURRENT_NVM_NODE="$(nvm_version current)" - if [ "$(nvm_version "$NODE_VERSION_LOCAL")" == "$CURRENT_NVM_NODE" ]; then + if [ "$(nvm_version "$NODE_VERSION_LOCAL")" = "$CURRENT_NVM_NODE" ]; then nvm_echo "=> Node.js version $NODE_VERSION_LOCAL has been successfully installed" else nvm_echo >&2 "Failed to install Node.js $NODE_VERSION_LOCAL" @@ -434,13 +434,13 @@ nvm_do_install() { elif [ -z "${NVM_PROFILE-}" ] ; then local TRIED_PROFILE if [ -n "${PROFILE}" ]; then - TRIED_PROFILE="${NVM_PROFILE} (as defined in \$PROFILE), " + TRIED_PROFILE="${PROFILE} (as defined in \$PROFILE), " fi nvm_echo "=> Profile not found. Tried ${TRIED_PROFILE-}~/.bashrc, ~/.bash_profile, ~/.zprofile, ~/.zshrc, and ~/.profile." nvm_echo "=> Create one of them and run this script again" nvm_echo " OR" nvm_echo "=> Append the following lines to the correct file yourself:" - command printf "${SOURCE_STR}" + command printf '%b' "${SOURCE_STR}" nvm_echo else if nvm_profile_is_bash_or_zsh "${NVM_PROFILE-}"; then @@ -448,14 +448,14 @@ nvm_do_install() { fi if ! command grep -qc '/nvm.sh' "$NVM_PROFILE"; then nvm_echo "=> Appending nvm source string to $NVM_PROFILE" - command printf "${SOURCE_STR}" >> "$NVM_PROFILE" + command printf '%b' "${SOURCE_STR}" >> "$NVM_PROFILE" else nvm_echo "=> nvm source string already in ${NVM_PROFILE}" fi # shellcheck disable=SC2016 if ${BASH_OR_ZSH} && ! command grep -qc '$NVM_DIR/bash_completion' "$NVM_PROFILE"; then nvm_echo "=> Appending bash_completion source string to $NVM_PROFILE" - command printf "$COMPLETION_STR" >> "$NVM_PROFILE" + command printf '%b' "$COMPLETION_STR" >> "$NVM_PROFILE" else nvm_echo "=> bash_completion source string already in ${NVM_PROFILE}" fi @@ -476,7 +476,7 @@ nvm_do_install() { nvm_reset nvm_echo "=> Close and reopen your terminal to start using nvm or run the following to use it now:" - command printf "${SOURCE_STR}" + command printf '%b' "${SOURCE_STR}" if ${BASH_OR_ZSH} ; then command printf "${COMPLETION_STR}" fi diff --git a/test/install_script/nvm_do_install_printf_format b/test/install_script/nvm_do_install_printf_format new file mode 100755 index 0000000..8961e55 --- /dev/null +++ b/test/install_script/nvm_do_install_printf_format @@ -0,0 +1,70 @@ +#!/bin/sh + +die () { echo "$@" ; cleanup ; exit 1; } + +cleanup() { + unset -f install_nvm_from_git install_nvm_as_script nvm_detect_profile nvm_has nvm_install_dir + unset -f nvm_check_global_modules nvm_install_node + unset -f setup cleanup die + unset NVM_ENV METHOD PROFILE + rm -rf "${TMPDIR_FOR_TEST-}" 2>/dev/null +} + +setup() { + NVM_ENV=testing \. ../../install.sh + + # Mock installation functions to do nothing + install_nvm_from_git() { :; } + install_nvm_as_script() { :; } + nvm_check_global_modules() { :; } + nvm_install_node() { :; } + + # Mock nvm_has to return true for git + nvm_has() { + case "$1" in + git) return 0 ;; + xcode-select) return 1 ;; + *) return 1 ;; + esac + } +} + +setup + +# +# Test: printf calls in nvm_do_install should not interpret % in paths +# When NVM_DIR contains printf format specifiers like %s or %d, +# the output should contain them literally, not interpret them. +# + +TMPDIR_FOR_TEST="$(mktemp -d)" +PERCENT_DIR="${TMPDIR_FOR_TEST}/nvm_%s_test" +mkdir -p "${PERCENT_DIR}" + +# Copy nvm.sh to the temp dir so sourcing succeeds +cp ../../nvm.sh "${PERCENT_DIR}/nvm.sh" + +# Mock nvm_install_dir to return our percent-containing path +nvm_install_dir() { + nvm_echo "${PERCENT_DIR}" +} + +# Mock nvm_detect_profile to return empty so we hit the "Profile not found" branch +# which calls: command printf '%b' "${SOURCE_STR}" +nvm_detect_profile() { + echo "" +} + +OUTPUT="$(PROFILE='' METHOD='' NVM_DIR="${PERCENT_DIR}" nvm_do_install 2>&1)" + +# The SOURCE_STR should contain the %s from the path literally +if ! echo "${OUTPUT}" | grep -q '%s'; then + die "printf should not have consumed the %s in the path. Output: ${OUTPUT}" +fi + +# Also verify via the "Profile not found" branch that SOURCE_STR was printed correctly +if ! echo "${OUTPUT}" | grep -q 'nvm_%s_test'; then + die "Expected nvm_%s_test in output but got: ${OUTPUT}" +fi + +cleanup diff --git a/test/install_script/nvm_do_install_tried_profile b/test/install_script/nvm_do_install_tried_profile new file mode 100755 index 0000000..ee0ed62 --- /dev/null +++ b/test/install_script/nvm_do_install_tried_profile @@ -0,0 +1,68 @@ +#!/bin/sh + +die () { echo "$@" ; cleanup ; exit 1; } + +cleanup() { + unset -f install_nvm_from_git install_nvm_as_script nvm_detect_profile nvm_has + unset -f nvm_check_global_modules nvm_install_node + unset -f setup cleanup die + unset NVM_ENV METHOD PROFILE +} + +setup() { + NVM_ENV=testing \. ../../install.sh + + # Mock installation functions to do nothing + install_nvm_from_git() { :; } + install_nvm_as_script() { :; } + nvm_check_global_modules() { :; } + nvm_install_node() { :; } + + # Mock nvm_has to return true for git + nvm_has() { + case "$1" in + git) return 0 ;; + xcode-select) return 1 ;; + *) return 1 ;; + esac + } + + # Mock nvm_detect_profile to return empty (profile not found) + nvm_detect_profile() { + echo "" + } +} + +setup + +# +# Test: When PROFILE is set to a non-/dev/null value but nvm_detect_profile +# returns empty, the "Profile not found" message should include the PROFILE +# value in the TRIED_PROFILE string. +# +# Before the fix, TRIED_PROFILE was set to "${NVM_PROFILE} (as defined in $PROFILE)" +# but NVM_PROFILE is known to be empty at that point, so the message would say +# " (as defined in $PROFILE)" instead of "/some/path (as defined in $PROFILE)". +# + +OUTPUT="$(PROFILE='/my/custom/profile' METHOD='' NVM_DIR="$(mktemp -d)" nvm_do_install 2>&1)" + +# The output should mention /my/custom/profile in the "Tried" line +if ! echo "${OUTPUT}" | grep -q '/my/custom/profile (as defined in \$PROFILE)'; then + # Check without the escaped dollar sign too + if ! echo "${OUTPUT}" | grep -q '/my/custom/profile'; then + die "Expected TRIED_PROFILE to contain '/my/custom/profile', got: ${OUTPUT}" + fi +fi + +# Verify the "Profile not found" message appears +if ! echo "${OUTPUT}" | grep -q 'Profile not found'; then + die "Expected 'Profile not found' message, got: ${OUTPUT}" +fi + +# The message should NOT start with " (as defined" (which would indicate empty TRIED_PROFILE prefix) +if echo "${OUTPUT}" | grep -q 'Tried (as defined'; then + die "TRIED_PROFILE appears to have an empty prefix (NVM_PROFILE instead of PROFILE), got: ${OUTPUT}" +fi + +cleanup