Fix privs in Node, regressions from #199 (#241)

Co-authored-by: Samruddhi Khandale <skhandale@microsoft.com>

5 files changed, 89 insertions, 52 deletions

diff --git a/src/node/devcontainer-feature.json b/src/node/devcontainer-feature.json
index 43ba0fa..cdc2fec 100644
--- a/src/node/devcontainer-feature.json
+++ b/src/node/devcontainer-feature.json
@@ -1,6 +1,6 @@
 {
     "id": "node",
-    "version": "1.0.8",
+    "version": "1.1.0",
     "name": "Node.js (via nvm) and yarn",
     "documentationURL": "https://github.com/devcontainers/features/tree/main/src/node",
     "description": "Installs Node.js, nvm, yarn, and needed dependencies.",
@@ -27,6 +27,11 @@
             "type": "string",
             "default": "/usr/local/share/nvm",
             "description": "The path where NVM will be installed."
+        },
+        "nvmVersion": {
+            "type": "string",
+            "default": "0.39.2",
+            "description": "Version of NVM to install."
         }
     },
     "customizations": {
diff --git a/src/node/install.sh b/src/node/install.sh
index 2bdd21a..cc5e1dc 100755
--- a/src/node/install.sh
+++ b/src/node/install.sh
@@ -1,13 +1,14 @@
 #!/bin/bash
-#-------------------------------------------------------------------------------------------------------------
+#-------------------------------------------------------------------------------------------------------------------------
 # Copyright (c) Microsoft Corporation. All rights reserved.
-# Licensed under the MIT License. See https://go.microsoft.com/fwlink/?linkid=2090316 for license information.
-#-------------------------------------------------------------------------------------------------------------
+# Licensed under the MIT License. See https://github.com/devcontainers/features/blob/main/LICENSE for license information.
+#-------------------------------------------------------------------------------------------------------------------------
 #
-# Docs: https://github.com/microsoft/vscode-dev-containers/blob/main/script-library/docs/node.md
-# Maintainer: The VS Code and Codespaces Teams
+# Docs: https://github.com/devcontainers/features/tree/main/src/node
+# Maintainer: The Dev Container spec maintainers
 
 export NODE_VERSION=${VERSION:-"lts"}
+export NVM_VERSION="${NVMVERSION:-"0.39.2"}"
 export NVM_DIR=${NVMINSTALLPATH:-"/usr/local/share/nvm"}
 INSTALL_TOOLS_FOR_NODE_GYP="${NODEGYPDEPENDENCIES:-true}"
 
@@ -18,8 +19,6 @@ ADDITIONAL_VERSIONS=${ADDITIONALVERSIONS:-""}
 USERNAME=${USERNAME:-"automatic"}
 UPDATE_RC=${UPDATE_RC:-"true"}
 
-export NVM_VERSION="0.38.0"
-
 set -e
 
 # Clean up
@@ -105,67 +104,73 @@ elif [ "${NODE_VERSION}" = "latest" ]; then
     export NODE_VERSION="node"
 fi
 
+# Install snipppet that we will run as the user
+nvm_install_snippet="$(cat << EOF
+set -e
+umask 0002
+# Do not update profile - we'll do this manually
+export PROFILE=/dev/null
+curl -so- https://raw.githubusercontent.com/nvm-sh/nvm/v${NVM_VERSION}/install.sh | bash 
+source ${NVM_DIR}/nvm.sh
+if [ "${NODE_VERSION}" != "" ]; then
+    nvm alias default ${NODE_VERSION}
+fi
+EOF
+)"
+
+# Snippet that should be added into rc / profiles
+nvm_rc_snippet="$(cat << EOF
+export NVM_DIR="${NVM_DIR}"
+[ -s "\$NVM_DIR/nvm.sh" ] && . "\$NVM_DIR/nvm.sh"
+[ -s "\$NVM_DIR/bash_completion" ] && . "\$NVM_DIR/bash_completion"
+EOF
+)"
+
 # Create a symlink to the installed version for use in Dockerfile PATH statements
 export NVM_SYMLINK_CURRENT=true
 
-# Install the specified node version if NVM directory already exists, then exit
-if [ -d "${NVM_DIR}" ]; then
-    echo "NVM already installed."
-    if [ "${NODE_VERSION}" != "" ]; then
-       su ${USERNAME} -c ". $NVM_DIR/nvm.sh && nvm install ${NODE_VERSION} && nvm clear-cache"
-    fi
-    # Clean up
-    rm -rf /var/lib/apt/lists/*
-    exit 0
-fi
-
-# Create nvm group, nvm dir, and set sticky bit
+# Create nvm group to the user's UID or GID to change while still allowing access to nvm
 if ! cat /etc/group | grep -e "^nvm:" > /dev/null 2>&1; then
     groupadd -r nvm
 fi
-umask 0002
 usermod -a -G nvm ${USERNAME}
-mkdir -p ${NVM_DIR}
-chown "${USERNAME}:nvm" ${NVM_DIR}
-chmod -R g+r+w ${NVM_DIR}
-su ${USERNAME} -c "$(cat << EOF
-    set -e
-    umask 0002
-    # Do not update profile - we'll do this manually
-    export PROFILE=/dev/null
-    curl -so- https://raw.githubusercontent.com/nvm-sh/nvm/v${NVM_VERSION}/install.sh | bash 
-    source ${NVM_DIR}/nvm.sh
+
+# Install nvm (which also installs NODE_VERSION), otherwise
+# use nvm to install the specified node version. Always use
+# umask 0002 so both the owner so that everything is u+rw,g+rw
+umask 0002
+if [ ! -d "${NVM_DIR}" ]; then
+    # Create nvm dir, and set sticky bit
+    mkdir -p ${NVM_DIR}
+    chown "${USERNAME}:nvm" ${NVM_DIR}
+    chmod g+rws ${NVM_DIR}
+    su ${USERNAME} -c "${nvm_install_snippet}" 2>&1
+    # Update rc files
+    if [ "${UPDATE_RC}" = "true" ]; then
+        updaterc "${nvm_rc_snippet}"
+    fi
+else
+    echo "NVM already installed."
     if [ "${NODE_VERSION}" != "" ]; then
-        nvm alias default ${NODE_VERSION}
+        su ${USERNAME} -c "umask 0002 && . $NVM_DIR/nvm.sh && nvm install ${NODE_VERSION}"
     fi
-    nvm clear-cache
-EOF
-)" 2>&1
-# Update rc files
-if [ "${UPDATE_RC}" = "true" ]; then
-updaterc "$(cat <<EOF
-export NVM_DIR="${NVM_DIR}"
-[ -s "\$NVM_DIR/nvm.sh" ] && . "\$NVM_DIR/nvm.sh"
-[ -s "\$NVM_DIR/bash_completion" ] && . "\$NVM_DIR/bash_completion"
-EOF
-)"
 fi
 
-# Additional node versions to be installed but not be set as default.
+# Additional node versions to be installed but not be set as 
+# default we can assume the nvm is the group owner of the nvm
+# directory and the sticky bit on directories so any installed
+# files will have will have the correct ownership (nvm)
 if [ ! -z "${ADDITIONAL_VERSIONS}" ]; then
-
     OLDIFS=$IFS
     IFS=","
         read -a additional_versions <<< "$ADDITIONAL_VERSIONS"
         for ver in "${additional_versions[@]}"; do
-            su ${USERNAME} -c ". $NVM_DIR/nvm.sh && nvm install ${ver}"
-            su ${USERNAME} -c ". $NVM_DIR/nvm.sh && nvm clear-cache"
-            # Reset the NODE_VERSION as the default version on the path.
+            su ${USERNAME} -c "umask 0002 && . $NVM_DIR/nvm.sh && nvm install ${ver}"
         done
 
         # Ensure $NODE_VERSION is on the $PATH
         if [ "${NODE_VERSION}" != "" ]; then
-                su ${USERNAME} -c ". $NVM_DIR/nvm.sh && nvm use default"
+                su ${USERNAME} -c "umask 0002 && . $NVM_DIR/nvm.sh && nvm use default"
         fi
     IFS=$OLDIFS
 fi
@@ -192,9 +197,14 @@ if [ "${INSTALL_TOOLS_FOR_NODE_GYP}" = "true" ]; then
     fi
 fi
 
-find "${NVM_DIR}" -type d -print0 | xargs -n 1 -0 chmod g+s
 
 # Clean up
+su ${USERNAME} -c "umask 0002 && . $NVM_DIR/nvm.sh && nvm clear-cache"
 rm -rf /var/lib/apt/lists/*
 
+# Ensure privs are correct for installed node versions. Unfortunately the
+# way nvm installs node versions pulls privs from the tar which does not
+# have group write set. We need this when the gid/uid is updated.
+chmod -R g+rw "${NVM_DIR}/versions"
+
 echo "Done!"
diff --git a/test/node/install_additional_node.sh b/test/node/install_additional_node.sh
index 83dae86..1a1abca 100644
--- a/test/node/install_additional_node.sh
+++ b/test/node/install_additional_node.sh
@@ -5,7 +5,7 @@ set -e
 # Optional: Import test library
 source dev-container-features-test-lib
 
-# 'latest' is some version of node 18 for a while.
+# 'lts' is some version of node 18 for a while.
 check "version_on_path"  node -v | grep 18
 
 check "v18_installed" ls -1 /usr/local/share/nvm/versions/node | grep 18
diff --git a/test/node/non_root_user.sh b/test/node/non_root_user.sh
new file mode 100755
index 0000000..6b6804d
--- /dev/null
+++ b/test/node/non_root_user.sh
@@ -0,0 +1,13 @@
+#!/bin/bash
+
+set -e
+
+# Optional: Import test library
+source dev-container-features-test-lib
+
+# Definition specific tests
+check "version" node  --version
+check "nvm" bash -c ". /usr/local/share/nvm/nvm.sh && nvm install 10"
+
+# Report result
+reportResults
\ No newline at end of file
diff --git a/test/node/scenarios.json b/test/node/scenarios.json
index 845c892..8f93d82 100644
--- a/test/node/scenarios.json
+++ b/test/node/scenarios.json
@@ -3,9 +3,18 @@
         "image": "debian:11",
         "features": {
             "node": {
-                "version": "latest",
+                "version": "lts",
                 "additionalVersions": "v17.9.1,v14.19.3"
             }
         }
+    },
+    "non_root_user": {
+        "image": "mcr.microsoft.com/devcontainers/base",
+        "remoteUser": "vscode",
+        "features": {
+            "node": {
+                "version": "latest"
+            }
+        }
     }
 }
\ No newline at end of file