Fix code review issues: validate _REMOTE_USER, fix common-utils none handling, remove dead code

Co-authored-by: abdurriq <[email protected]>

Author: Copilot

src/_common/common-setup.sh

@@ -37,8 +37,17 @@ determine_user_from_input() {
 
         # First, check if _REMOTE_USER is set and is not root
         if [ -n "${_REMOTE_USER:-}" ] && [ "${_REMOTE_USER}" != "root" ]; then
-            resolved_username="${_REMOTE_USER}"
-        else
+            # Verify the user exists before using it
+            if id -u "${_REMOTE_USER}" > /dev/null 2>&1; then
+                resolved_username="${_REMOTE_USER}"
+            else
+                # _REMOTE_USER doesn't exist, fall through to normal detection
+                resolved_username=""
+            fi
+        fi
+        
+        # If we didn't resolve via _REMOTE_USER, try to find a non-root user
+        if [ -z "${resolved_username}" ]; then
             # Try to find a non-root user from a list of common usernames
             # The list includes: devcontainer, vscode, node, codespace, and the user with UID 1000
             local possible_users=("devcontainer" "vscode" "node" "codespace" "$(awk -v val=1000 -F ":" '$3==val{print $1}' /etc/passwd 2>/dev/null || echo '')")

src/common-utils/main.sh

@@ -395,13 +395,16 @@ esac
 SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
 source "${SCRIPT_DIR}/../_common/common-setup.sh"
 
-# If in automatic mode, determine if a user already exists, if not use vscode (which will be created)
-USERNAME=$(determine_user_from_input "${USERNAME}" "vscode")
-
-# Handle the special "none" case for common-utils
-if [ "${USERNAME}" = "none" ]; then
+# Handle the special "none" case for common-utils before user determination
+# The "none" case sets USER_UID and USER_GID to 0
+ORIGINAL_USERNAME="${USERNAME}"
+if [ "${ORIGINAL_USERNAME}" = "none" ]; then
+    USERNAME="root"
     USER_UID=0
     USER_GID=0
+else
+    # If in automatic mode, determine if a user already exists, if not use vscode (which will be created)
+    USERNAME=$(determine_user_from_input "${USERNAME}" "vscode")
 fi
 # Create or update a non-root user to match UID/GID.
 group_name="${USERNAME}"

src/java/wrapper.sh

@@ -23,10 +23,6 @@ if [ "${is_jdk_8}" = "true" ]; then
     ./install.sh "${ADDITIONAL_JAVA_VERSION}" "${SDKMAN_DIR}" "${USERNAME}" "${UPDATE_RC}"
     jdk_11_folder="$(ls --format=single-column ${SDKMAN_DIR}/candidates/java | grep -oE -m 1 '11\..+')"
     ln -s "${SDKMAN_DIR}/candidates/java/${jdk_11_folder}" /extension-java-home
-
-    # Source common helper functions to determine the appropriate non-root user
-    source "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/../_common/common-setup.sh"
-    username=$(determine_user_from_input "${USERNAME}" "root")
 else
     ln -s ${SDKMAN_DIR}/candidates/java/current /extension-java-home
 fi

test/_common/test-common-setup.sh

@@ -113,21 +113,26 @@ test_remote_user_root() {
 
 # Test 10: Finding vscode user if it exists
 test_find_vscode_user() {
-    # Check if vscode user exists
-    if id -u vscode > /dev/null 2>&1; then
+    # Check if vscode user exists and no higher priority users exist
+    if id -u vscode > /dev/null 2>&1 && \
+       ! id -u devcontainer > /dev/null 2>&1; then
+        # Unset _REMOTE_USER to ensure it doesn't interfere
+        unset _REMOTE_USER
         local result=$(determine_user_from_input "automatic")
         # Should find vscode (it's second in priority after devcontainer)
         run_test "Finds vscode user in automatic mode" "vscode" "${result}"
     else
-        # Skip this test if vscode user doesn't exist
-        run_test "Finds vscode user in automatic mode (SKIPPED - user doesn't exist)" "SKIP" "SKIP"
+        # Skip this test if vscode user doesn't exist or higher priority user exists
+        run_test "Finds vscode user in automatic mode (SKIPPED - conditions not met)" "SKIP" "SKIP"
     fi
 }
 
 # Test 11: Finding devcontainer user (highest priority)
 test_find_devcontainer_user() {
     # Check if devcontainer user exists
     if id -u devcontainer > /dev/null 2>&1; then
+        # Unset _REMOTE_USER to ensure it doesn't interfere
+        unset _REMOTE_USER
         local result=$(determine_user_from_input "automatic")
         # Should find devcontainer (highest priority)
         run_test "Finds devcontainer user (highest priority)" "devcontainer" "${result}"
@@ -165,6 +170,18 @@ test_empty_input() {
     run_test "Empty input treated as automatic and finds UID 1000 or uses fallback" "${expected}" "${result}"
 }
 
+# Test 14: _REMOTE_USER set to non-existent user should use fallback
+test_remote_user_nonexistent() {
+    export _REMOTE_USER="nonexistentuser99999"
+    local result=$(determine_user_from_input "automatic" "mydefault")
+    unset _REMOTE_USER
+    
+    # Should fall through to normal detection - find UID 1000 or use fallback
+    local uid_1000_user=$(awk -v val=1000 -F ":" '$3==val{print $1}' /etc/passwd 2>/dev/null || echo '')
+    local expected="${uid_1000_user:-mydefault}"
+    run_test "_REMOTE_USER set to non-existent user falls back to detection" "${expected}" "${result}"
+}
+
 # Run all tests
 echo "Running tests for common-setup.sh..."
 echo "======================================"
@@ -183,6 +200,7 @@ test_find_vscode_user
 test_find_devcontainer_user
 test_find_uid_1000
 test_empty_input
+test_remote_user_nonexistent
 
 # Print summary
 echo ""