From 72a878447fa2034e4200a93b447a9293e2f4aacb Mon Sep 17 00:00:00 2001 From: Jordan Harband Date: Fri, 24 Jul 2026 10:50:47 -0700 Subject: [PATCH] [Robustness] `nvm_install_binary_extract`: replace a broken version dir atomically MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A version directory left without a working `bin/node` - e.g. a partial or interrupted earlier install - wedged reinstallation: `nvm install` saw the version as not installed and re-extracted, but the per-entry `mv` refused to overwrite the leftover non-empty `bin/`, `lib/`, … subdirectories and left a half-updated tree behind (and, without `-b`, fell back to a from-source compile) Remove any pre-existing version directory and move the freshly extracted tree into place with a single rename, so a version is either fully installed or not present at all. The removal is safe: it runs only after the tarball has downloaded and extracted successfully into the cache. Fall back to the previous per-entry move when a single rename cannot cross filesystems. --- nvm.sh | 37 ++++++++--- ...ry_extract self-heals a broken version dir | 62 +++++++++++++++++++ 2 files changed, 89 insertions(+), 10 deletions(-) create mode 100755 test/fast/Unit tests/nvm_install_binary_extract self-heals a broken version dir diff --git a/nvm.sh b/nvm.sh index 37a13a50..d5d4c088 100755 --- a/nvm.sh +++ b/nvm.sh @@ -2377,22 +2377,39 @@ nvm_install_binary_extract() { if [ "${NVM_OS}" = 'win' ]; then VERSION_PATH="${VERSION_PATH}/bin" command unzip -q "${TARBALL}" -d "${TMPDIR}" || return 1 - # For non Windows system (including WSL running on Windows) - else - nvm_extract_tarball "${NVM_OS}" "${VERSION}" "${TARBALL}" "${TMPDIR}" - fi - - command mkdir -p "${VERSION_PATH}" || return 1 - - if [ "${NVM_OS}" = 'win' ]; then + # Replace any pre-existing (possibly broken or partial) install so the + # move below cannot collide with leftover files. Safe here: the archive + # has already downloaded and unzipped successfully into TMPDIR. + command rm -rf "${VERSION_PATH}" + command mkdir -p "${VERSION_PATH}" || return 1 command mv "${TMPDIR}/"*/* "${VERSION_PATH}/" || return 1 command chmod +x "${VERSION_PATH}"/node.exe || return 1 command chmod +x "${VERSION_PATH}"/npm || return 1 command chmod +x "${VERSION_PATH}"/npx 2>/dev/null - else - command mv "${TMPDIR}/"* "${VERSION_PATH}" || return 1 + command rm -rf "${TMPDIR}" + return 0 fi + # For non-Windows systems (including WSL running on Windows) + nvm_extract_tarball "${NVM_OS}" "${VERSION}" "${TARBALL}" "${TMPDIR}" || return 1 + + # Install atomically: replace any pre-existing version directory with a + # single rename, so a partial or broken tree is never observed as installed. + # A leftover directory without a working bin/node otherwise wedges the + # install - the per-entry `mv` refuses to overwrite the non-empty bin/, lib/, + # ... subdirectories and leaves a half-updated tree behind. Removing it first + # is safe: the tarball has already downloaded and extracted into TMPDIR. + command rm -rf "${VERSION_PATH}" || return 1 + command mkdir -p "$(dirname "${VERSION_PATH}")" || return 1 + if command mv "${TMPDIR}" "${VERSION_PATH}" 2>/dev/null; then + return 0 + fi + + # Fall back to a per-entry move when a single rename is not possible (e.g. + # TMPDIR and the versions directory are on different filesystems). + command rm -rf "${VERSION_PATH}" + command mkdir -p "${VERSION_PATH}" || return 1 + command mv "${TMPDIR}/"* "${VERSION_PATH}" || return 1 command rm -rf "${TMPDIR}" return 0 diff --git a/test/fast/Unit tests/nvm_install_binary_extract self-heals a broken version dir b/test/fast/Unit tests/nvm_install_binary_extract self-heals a broken version dir new file mode 100755 index 00000000..a353ec74 --- /dev/null +++ b/test/fast/Unit tests/nvm_install_binary_extract self-heals a broken version dir @@ -0,0 +1,62 @@ +#!/bin/sh + +cleanup() { + [ -n "${tmp_dir}" ] && [ -d "${tmp_dir}" ] && rm -rf "${tmp_dir}" + [ -n "${NVM_DIR}" ] && [ -d "${NVM_DIR}" ] && rm -rf "${NVM_DIR}" + unset -f die cleanup nvm_supports_xz + unset NVM_DIR tmp_dir version archi node_dir version_path +} + +die() { echo "$@" ; cleanup ; exit 1; } + +: nvm.sh +\. ../../../nvm.sh + +# Use gzip so the test does not depend on xz being installed. +nvm_supports_xz() { return 1; } + +set -ex + +type nvm_install_binary_extract > /dev/null 2>&1 || die 'nvm_install_binary_extract is not available' + +NVM_DIR="$(mktemp -d)" +tmp_dir="$(mktemp -d)" +[ -n "${NVM_DIR}" ] && [ -n "${tmp_dir}" ] || die 'Unable to create temporary folders' + +version='v14.15.4' +archi='linux-x64' +node_dir="${tmp_dir}/node-${version}-${archi}" + +# Build a fake binary tarball: a runnable node, an npm symlink, and a lib tree. +mkdir -p "${node_dir}/bin" "${node_dir}/lib/node_modules/npm/bin" || die 'setup mkdir failed' +printf '#!/bin/sh\necho "%s"\n' "${version}" > "${node_dir}/bin/node" +chmod +x "${node_dir}/bin/node" +echo 'npm-cli' > "${node_dir}/lib/node_modules/npm/bin/npm-cli.js" +( cd "${node_dir}/bin" && ln -s ../lib/node_modules/npm/bin/npm-cli.js npm ) +echo 'fresh' > "${node_dir}/lib/FRESHFILE" +( cd "${tmp_dir}" && tar -czf "${node_dir}.tar.gz" "node-${version}-${archi}" ) || die 'unable to create fake tarball' + +# Pre-create a BROKEN, non-empty version directory: npm and a stale lib file +# are present, but bin/node is missing - exactly the shape that wedges a +# per-entry move onto existing non-empty subdirectories. +version_path="${NVM_DIR}/versions/node/${version}" +mkdir -p "${version_path}/bin" "${version_path}/lib" || die 'unable to stage broken dir' +echo 'stale-npm' > "${version_path}/bin/npm" +echo 'stale' > "${version_path}/lib/STALEFILE" +[ -e "${version_path}/bin/node" ] && die 'precondition: broken dir should have no bin/node' + +# Extract over the broken directory. +nvm_install_binary_extract 'linux' "${version}" "$(expr "${version}" : '.\(.*\)')" "${node_dir}.tar.gz" "${tmp_dir}/files" || die 'nvm_install_binary_extract failed over a broken version dir' + +# bin/node is restored and runnable. +[ -x "${version_path}/bin/node" ] || die 'bin/node was not restored' +[ "$("${version_path}/bin/node")" = "${version}" ] || die 'restored bin/node has wrong contents' + +# The stale file is gone: the whole directory was replaced, not merged into. +[ ! -e "${version_path}/lib/STALEFILE" ] || die 'stale lib file survived: install was not atomic' +[ -e "${version_path}/lib/FRESHFILE" ] || die 'fresh lib content missing after install' + +# npm resolves to the freshly installed target. +[ -e "${version_path}/bin/npm" ] || die 'npm missing after install' + +cleanup