From 3fb9d02f36f34d2518421b0cf68d1e3f8a0e3e11 Mon Sep 17 00:00:00 2001
From: CanbiZ <47820557+MickLesk@users.noreply.github.com>
Date: Mon, 8 Dec 2025 14:20:05 +0100
Subject: [PATCH] fix: always show SSH access dialog in advanced settings
 (#9765)

- SSH access dialog is now always displayed regardless of password or SSH keys
- Added step indicator to SSH settings dialogs for consistency
- configure_ssh_settings() now accepts optional step_info parameter
- Updated documentation for SSH configuration functions

Fixes #9753
---
 .../BUILD_FUNC_FUNCTIONS_REFERENCE.md         | 176 ++++++++++++++----
 misc/build.func                               |  31 +--
 2 files changed, 154 insertions(+), 53 deletions(-)

diff --git a/docs/misc/build.func/BUILD_FUNC_FUNCTIONS_REFERENCE.md b/docs/misc/build.func/BUILD_FUNC_FUNCTIONS_REFERENCE.md
index a8128d4e91..5782ac28b3 100644
--- a/docs/misc/build.func/BUILD_FUNC_FUNCTIONS_REFERENCE.md
+++ b/docs/misc/build.func/BUILD_FUNC_FUNCTIONS_REFERENCE.md
@@ -9,30 +9,35 @@ This document provides a comprehensive reference of all functions in `build.func
 ### Initialization Functions
 
 #### `start()`
+
 **Purpose**: Main entry point when build.func is sourced or executed
 **Parameters**: None
 **Returns**: None
 **Side Effects**:
+
 - Detects execution context (Proxmox host vs container)
 - Captures hard environment variables
 - Sets CT_TYPE based on context
 - Routes to appropriate workflow (install_script or update_script)
-**Dependencies**: None
-**Environment Variables Used**: `CT_TYPE`, `APP`, `CTID`
+  **Dependencies**: None
+  **Environment Variables Used**: `CT_TYPE`, `APP`, `CTID`
 
 #### `variables()`
+
 **Purpose**: Load and resolve all configuration variables using precedence chain
 **Parameters**: None
 **Returns**: None
 **Side Effects**:
+
 - Loads app-specific .vars file
 - Loads global default.vars file
 - Applies variable precedence chain
 - Sets all configuration variables
-**Dependencies**: `base_settings()`
-**Environment Variables Used**: All configuration variables
+  **Dependencies**: `base_settings()`
+  **Environment Variables Used**: All configuration variables
 
 #### `base_settings()`
+
 **Purpose**: Set built-in default values for all configuration variables
 **Parameters**: None
 **Returns**: None
@@ -43,28 +48,33 @@ This document provides a comprehensive reference of all functions in `build.func
 ### UI and Menu Functions
 
 #### `install_script()`
+
 **Purpose**: Main installation workflow coordinator
 **Parameters**: None
 **Returns**: None
 **Side Effects**:
+
 - Displays installation mode selection menu
 - Coordinates the entire installation process
 - Handles user interaction and validation
-**Dependencies**: `variables()`, `build_container()`, `default_var_settings()`
-**Environment Variables Used**: `APP`, `CTID`, `var_hostname`
+  **Dependencies**: `variables()`, `build_container()`, `default_var_settings()`
+  **Environment Variables Used**: `APP`, `CTID`, `var_hostname`
 
 #### `advanced_settings()`
+
 **Purpose**: Provide advanced configuration options via whiptail menus
 **Parameters**: None
 **Returns**: None
 **Side Effects**:
+
 - Displays whiptail menus for configuration
 - Updates configuration variables based on user input
 - Validates user selections
-**Dependencies**: `select_storage()`, `detect_gpu_devices()`
-**Environment Variables Used**: All configuration variables
+  **Dependencies**: `select_storage()`, `detect_gpu_devices()`
+  **Environment Variables Used**: All configuration variables
 
 #### `settings_menu()`
+
 **Purpose**: Display and handle settings configuration menu
 **Parameters**: None
 **Returns**: None
@@ -75,58 +85,68 @@ This document provides a comprehensive reference of all functions in `build.func
 ### Storage Functions
 
 #### `select_storage()`
+
 **Purpose**: Handle storage selection for templates and containers
 **Parameters**: None
 **Returns**: None
 **Side Effects**:
+
 - Resolves storage preselection
 - Prompts user for storage selection if needed
 - Validates storage availability
 - Sets var_template_storage and var_container_storage
-**Dependencies**: `resolve_storage_preselect()`, `choose_and_set_storage_for_file()`
-**Environment Variables Used**: `var_template_storage`, `var_container_storage`, `TEMPLATE_STORAGE`, `CONTAINER_STORAGE`
+  **Dependencies**: `resolve_storage_preselect()`, `choose_and_set_storage_for_file()`
+  **Environment Variables Used**: `var_template_storage`, `var_container_storage`, `TEMPLATE_STORAGE`, `CONTAINER_STORAGE`
 
 #### `resolve_storage_preselect()`
+
 **Purpose**: Resolve preselected storage options
 **Parameters**:
+
 - `storage_type`: Type of storage (template or container)
-**Returns**: Storage name if valid, empty if invalid
-**Side Effects**: Validates storage availability
-**Dependencies**: None
-**Environment Variables Used**: `var_template_storage`, `var_container_storage`
+  **Returns**: Storage name if valid, empty if invalid
+  **Side Effects**: Validates storage availability
+  **Dependencies**: None
+  **Environment Variables Used**: `var_template_storage`, `var_container_storage`
 
 #### `choose_and_set_storage_for_file()`
+
 **Purpose**: Interactive storage selection via whiptail
 **Parameters**:
+
 - `storage_type`: Type of storage (template or container)
 - `content_type`: Content type (vztmpl or rootdir)
-**Returns**: None
-**Side Effects**:
+  **Returns**: None
+  **Side Effects**:
 - Displays whiptail menu
 - Updates storage variables
 - Validates selection
-**Dependencies**: None
-**Environment Variables Used**: `var_template_storage`, `var_container_storage`
+  **Dependencies**: None
+  **Environment Variables Used**: `var_template_storage`, `var_container_storage`
 
 ### Container Creation Functions
 
 #### `build_container()`
+
 **Purpose**: Validate settings and prepare container creation
 **Parameters**: None
 **Returns**: None
 **Side Effects**:
+
 - Validates all configuration
 - Checks for conflicts
 - Prepares container configuration
 - Calls create_lxc_container()
-**Dependencies**: `create_lxc_container()`
-**Environment Variables Used**: All configuration variables
+  **Dependencies**: `create_lxc_container()`
+  **Environment Variables Used**: All configuration variables
 
 #### `create_lxc_container()`
+
 **Purpose**: Create the actual LXC container
 **Parameters**: None
 **Returns**: None
 **Side Effects**:
+
 - Creates LXC container with basic configuration
 - Configures network settings
 - Sets up storage and mount points
@@ -134,108 +154,176 @@ This document provides a comprehensive reference of all functions in `build.func
 - Sets resource limits
 - Configures startup options
 - Starts container
-**Dependencies**: `configure_gpu_passthrough()`, `fix_gpu_gids()`
-**Environment Variables Used**: All configuration variables
+  **Dependencies**: `configure_gpu_passthrough()`, `fix_gpu_gids()`
+  **Environment Variables Used**: All configuration variables
 
 ### GPU and Hardware Functions
 
 #### `detect_gpu_devices()`
+
 **Purpose**: Detect available GPU hardware on the system
 **Parameters**: None
 **Returns**: None
 **Side Effects**:
+
 - Scans for Intel, AMD, and NVIDIA GPUs
 - Updates var_gpu_type and var_gpu_devices
 - Determines GPU capabilities
-**Dependencies**: None
-**Environment Variables Used**: `var_gpu_type`, `var_gpu_devices`, `GPU_APPS`
+  **Dependencies**: None
+  **Environment Variables Used**: `var_gpu_type`, `var_gpu_devices`, `GPU_APPS`
 
 #### `configure_gpu_passthrough()`
+
 **Purpose**: Configure GPU passthrough for the container
 **Parameters**: None
 **Returns**: None
 **Side Effects**:
+
 - Adds GPU device entries to container config
 - Configures proper device permissions
 - Sets up device mapping
 - Updates /etc/pve/lxc/<ctid>.conf
-**Dependencies**: `detect_gpu_devices()`
-**Environment Variables Used**: `var_gpu`, `var_gpu_type`, `var_gpu_devices`, `CTID`
+  **Dependencies**: `detect_gpu_devices()`
+  **Environment Variables Used**: `var_gpu`, `var_gpu_type`, `var_gpu_devices`, `CTID`
 
 #### `fix_gpu_gids()`
+
 **Purpose**: Fix GPU group IDs after container creation
 **Parameters**: None
 **Returns**: None
 **Side Effects**:
+
 - Updates GPU group IDs in container
 - Ensures proper GPU access permissions
 - Configures video and render groups
-**Dependencies**: `configure_gpu_passthrough()`
-**Environment Variables Used**: `CTID`, `var_gpu_type`
+  **Dependencies**: `configure_gpu_passthrough()`
+  **Environment Variables Used**: `CTID`, `var_gpu_type`
+
+### SSH Configuration Functions
+
+#### `configure_ssh_settings()`
+
+**Purpose**: Interactive SSH key and access configuration wizard
+**Parameters**:
+
+- `step_info` (optional): Step indicator string (e.g., "Step 17/19") for consistent dialog headers
+  **Returns**: None
+  **Side Effects**:
+- Creates temporary file for SSH keys
+- Discovers and presents available SSH keys from host
+- Allows manual key entry or folder/glob scanning
+- Sets `SSH` variable to "yes" or "no" based on user selection
+- Sets `SSH_AUTHORIZED_KEY` if manual key provided
+- Populates `SSH_KEYS_FILE` with selected keys
+  **Dependencies**: `ssh_discover_default_files()`, `ssh_build_choices_from_files()`
+  **Environment Variables Used**: `SSH`, `SSH_AUTHORIZED_KEY`, `SSH_KEYS_FILE`
+
+**SSH Key Source Options**:
+
+1. `found` - Select from auto-detected host keys
+2. `manual` - Paste a single public key
+3. `folder` - Scan custom folder or glob pattern
+4. `none` - No SSH keys
+
+**Note**: The "Enable root SSH access?" dialog is always shown, regardless of whether SSH keys or password are configured. This ensures users can always enable SSH access even with automatic login.
+
+#### `ssh_discover_default_files()`
+
+**Purpose**: Discover SSH public key files on the host system
+**Parameters**: None
+**Returns**: Array of discovered key file paths
+**Side Effects**: Scans common SSH key locations
+**Dependencies**: None
+**Environment Variables Used**: `var_ssh_import_glob`
+
+#### `ssh_build_choices_from_files()`
+
+**Purpose**: Build whiptail checklist choices from SSH key files
+**Parameters**:
+
+- Array of file paths to process
+  **Returns**: None
+  **Side Effects**:
+- Sets `CHOICES` array for whiptail checklist
+- Sets `COUNT` variable with number of keys found
+- Creates `MAPFILE` for key tag to content mapping
+  **Dependencies**: None
+  **Environment Variables Used**: `CHOICES`, `COUNT`, `MAPFILE`
 
 ### Settings Persistence Functions
 
 #### `default_var_settings()`
+
 **Purpose**: Offer to save current settings as defaults
 **Parameters**: None
 **Returns**: None
 **Side Effects**:
+
 - Prompts user to save settings
 - Saves to default.vars file
 - Saves to app-specific .vars file
-**Dependencies**: `maybe_offer_save_app_defaults()`
-**Environment Variables Used**: All configuration variables
+  **Dependencies**: `maybe_offer_save_app_defaults()`
+  **Environment Variables Used**: All configuration variables
 
 #### `maybe_offer_save_app_defaults()`
+
 **Purpose**: Offer to save app-specific defaults
 **Parameters**: None
 **Returns**: None
 **Side Effects**:
+
 - Prompts user to save app-specific settings
 - Saves to app.vars file
 - Updates app-specific configuration
-**Dependencies**: None
-**Environment Variables Used**: `APP`, `SAVE_APP_DEFAULTS`
+  **Dependencies**: None
+  **Environment Variables Used**: `APP`, `SAVE_APP_DEFAULTS`
 
 ### Utility Functions
 
 #### `validate_settings()`
+
 **Purpose**: Validate all configuration settings
 **Parameters**: None
 **Returns**: 0 if valid, 1 if invalid
 **Side Effects**:
+
 - Checks for configuration conflicts
 - Validates resource limits
 - Validates network configuration
 - Validates storage configuration
-**Dependencies**: None
-**Environment Variables Used**: All configuration variables
+  **Dependencies**: None
+  **Environment Variables Used**: All configuration variables
 
 #### `check_conflicts()`
+
 **Purpose**: Check for configuration conflicts
 **Parameters**: None
 **Returns**: 0 if no conflicts, 1 if conflicts found
 **Side Effects**:
+
 - Checks for conflicting settings
 - Validates resource allocation
 - Checks network configuration
-**Dependencies**: None
-**Environment Variables Used**: All configuration variables
+  **Dependencies**: None
+  **Environment Variables Used**: All configuration variables
 
 #### `cleanup_on_error()`
+
 **Purpose**: Clean up resources on error
 **Parameters**: None
 **Returns**: None
 **Side Effects**:
+
 - Removes partially created containers
 - Cleans up temporary files
 - Resets configuration
-**Dependencies**: None
-**Environment Variables Used**: `CTID`
+  **Dependencies**: None
+  **Environment Variables Used**: `CTID`
 
 ## Function Call Flow
 
 ### Main Installation Flow
+
 ```
 start()
 ├── variables()
@@ -259,6 +347,7 @@ start()
 ```
 
 ### Error Handling Flow
+
 ```
 Error Detection
 ├── validate_settings()
@@ -271,24 +360,29 @@ Error Detection
 ## Function Dependencies
 
 ### Core Dependencies
+
 - `start()` → `install_script()` → `build_container()` → `create_lxc_container()`
 - `variables()` → `base_settings()`
 - `advanced_settings()` → `select_storage()` → `detect_gpu_devices()`
 
 ### Storage Dependencies
+
 - `select_storage()` → `resolve_storage_preselect()`
 - `select_storage()` → `choose_and_set_storage_for_file()`
 
 ### GPU Dependencies
+
 - `configure_gpu_passthrough()` → `detect_gpu_devices()`
 - `fix_gpu_gids()` → `configure_gpu_passthrough()`
 
 ### Settings Dependencies
+
 - `default_var_settings()` → `maybe_offer_save_app_defaults()`
 
 ## Function Usage Examples
 
 ### Basic Container Creation
+
 ```bash
 # Set required variables
 export APP="plex"
@@ -304,6 +398,7 @@ start()  # Entry point
 ```
 
 ### Advanced Configuration
+
 ```bash
 # Set advanced variables
 export var_os="debian"
@@ -319,6 +414,7 @@ advanced_settings()  # Interactive configuration
 ```
 
 ### GPU Passthrough
+
 ```bash
 # Enable GPU passthrough
 export GPU_APPS="plex"
@@ -331,6 +427,7 @@ fix_gpu_gids()  # Fix permissions
 ```
 
 ### Settings Persistence
+
 ```bash
 # Save settings as defaults
 export SAVE_DEFAULTS="true"
@@ -344,15 +441,18 @@ maybe_offer_save_app_defaults()  # Save app defaults
 ## Function Error Handling
 
 ### Validation Functions
+
 - `validate_settings()`: Returns 0 for valid, 1 for invalid
 - `check_conflicts()`: Returns 0 for no conflicts, 1 for conflicts
 
 ### Error Recovery
+
 - `cleanup_on_error()`: Cleans up on any error
 - Error codes are propagated up the call stack
 - Critical errors cause script termination
 
 ### Error Types
+
 1. **Configuration Errors**: Invalid settings or conflicts
 2. **Resource Errors**: Insufficient resources or conflicts
 3. **Network Errors**: Invalid network configuration
diff --git a/misc/build.func b/misc/build.func
index 2b7bf0d898..ce024eeac0 100644
--- a/misc/build.func
+++ b/misc/build.func
@@ -1492,7 +1492,7 @@ advanced_settings() {
     # STEP 17: SSH Settings
     # ═══════════════════════════════════════════════════════════════════════════
     17)
-      configure_ssh_settings
+      configure_ssh_settings "Step $STEP/$MAX_STEP"
       # configure_ssh_settings handles its own flow, always advance
       ((STEP++))
       ;;
@@ -2114,6 +2114,10 @@ ssh_discover_default_files() {
 }
 
 configure_ssh_settings() {
+  local step_info="${1:-}"
+  local backtitle="Proxmox VE Helper Scripts"
+  [[ -n "$step_info" ]] && backtitle="Proxmox VE Helper Scripts [${step_info}]"
+
   SSH_KEYS_FILE="$(mktemp)"
   : >"$SSH_KEYS_FILE"
 
@@ -2123,14 +2127,14 @@ configure_ssh_settings() {
 
   local ssh_key_mode
   if [[ "$default_key_count" -gt 0 ]]; then
-    ssh_key_mode=$(whiptail --backtitle "Proxmox VE Helper Scripts" --title "SSH KEY SOURCE" --menu \
+    ssh_key_mode=$(whiptail --backtitle "$backtitle" --title "SSH KEY SOURCE" --menu \
       "Provision SSH keys for root:" 14 72 4 \
       "found" "Select from detected keys (${default_key_count})" \
       "manual" "Paste a single public key" \
       "folder" "Scan another folder (path or glob)" \
       "none" "No keys" 3>&1 1>&2 2>&3) || exit_script
   else
-    ssh_key_mode=$(whiptail --backtitle "Proxmox VE Helper Scripts" --title "SSH KEY SOURCE" --menu \
+    ssh_key_mode=$(whiptail --backtitle "$backtitle" --title "SSH KEY SOURCE" --menu \
       "No host keys detected; choose manual/none:" 12 72 2 \
       "manual" "Paste a single public key" \
       "none" "No keys" 3>&1 1>&2 2>&3) || exit_script
@@ -2139,7 +2143,7 @@ configure_ssh_settings() {
   case "$ssh_key_mode" in
   found)
     local selection
-    selection=$(whiptail --backtitle "Proxmox VE Helper Scripts" --title "SELECT HOST KEYS" \
+    selection=$(whiptail --backtitle "$backtitle" --title "SELECT HOST KEYS" \
       --checklist "Select one or more keys to import:" 20 140 10 "${CHOICES[@]}" 3>&1 1>&2 2>&3) || exit_script
     for tag in $selection; do
       tag="${tag%\"}"
@@ -2150,13 +2154,13 @@ configure_ssh_settings() {
     done
     ;;
   manual)
-    SSH_AUTHORIZED_KEY="$(whiptail --backtitle "Proxmox VE Helper Scripts" \
+    SSH_AUTHORIZED_KEY="$(whiptail --backtitle "$backtitle" \
       --inputbox "Paste one SSH public key line (ssh-ed25519/ssh-rsa/...)" 10 72 --title "SSH Public Key" 3>&1 1>&2 2>&3)"
     [[ -n "$SSH_AUTHORIZED_KEY" ]] && printf '%s\n' "$SSH_AUTHORIZED_KEY" >>"$SSH_KEYS_FILE"
     ;;
   folder)
     local glob_path
-    glob_path=$(whiptail --backtitle "Proxmox VE Helper Scripts" \
+    glob_path=$(whiptail --backtitle "$backtitle" \
       --inputbox "Enter a folder or glob to scan (e.g. /root/.ssh/*.pub)" 10 72 --title "Scan Folder/Glob" 3>&1 1>&2 2>&3)
     if [[ -n "$glob_path" ]]; then
       shopt -s nullglob
@@ -2166,7 +2170,7 @@ configure_ssh_settings() {
         ssh_build_choices_from_files "${_scan_files[@]}"
         if [[ "$COUNT" -gt 0 ]]; then
           local folder_selection
-          folder_selection=$(whiptail --backtitle "Proxmox VE Helper Scripts" --title "SELECT FOLDER KEYS" \
+          folder_selection=$(whiptail --backtitle "$backtitle" --title "SELECT FOLDER KEYS" \
             --checklist "Select key(s) to import:" 20 78 10 "${CHOICES[@]}" 3>&1 1>&2 2>&3) || exit_script
           for tag in $folder_selection; do
             tag="${tag%\"}"
@@ -2176,10 +2180,10 @@ configure_ssh_settings() {
             [[ -n "$line" ]] && printf '%s\n' "$line" >>"$SSH_KEYS_FILE"
           done
         else
-          whiptail --backtitle "Proxmox VE Helper Scripts" --msgbox "No keys found in: $glob_path" 8 60
+          whiptail --backtitle "$backtitle" --msgbox "No keys found in: $glob_path" 8 60
         fi
       else
-        whiptail --backtitle "Proxmox VE Helper Scripts" --msgbox "Path/glob returned no files." 8 60
+        whiptail --backtitle "$backtitle" --msgbox "Path/glob returned no files." 8 60
       fi
     fi
     ;;
@@ -2193,12 +2197,9 @@ configure_ssh_settings() {
     printf '\n' >>"$SSH_KEYS_FILE"
   fi
 
-  if [[ -s "$SSH_KEYS_FILE" || "$PW" == -password* ]]; then
-    if (whiptail --backtitle "Proxmox VE Helper Scripts" --defaultno --title "SSH ACCESS" --yesno "Enable root SSH access?" 10 58); then
-      SSH="yes"
-    else
-      SSH="no"
-    fi
+  # Always show SSH access dialog - user should be able to enable SSH even without keys
+  if (whiptail --backtitle "$backtitle" --defaultno --title "SSH ACCESS" --yesno "Enable root SSH access?" 10 58); then
+    SSH="yes"
   else
     SSH="no"
   fi