core: load an app's own settings from its .vars file

.github/workflows/verify-split-files.yml

@@ -64,6 +64,7 @@ on:
       - "tests/gpg-key-reason.sh"
       - "tests/forge-branch-clean-install.sh"
       - "tests/alpine-clean-install.sh"
+      - "tests/app-vars-files.sh"
       - ".github/workflows/verify-split-files.yml"
   push:
     branches: [main]
@@ -119,6 +120,7 @@ on:
       - "tests/gpg-key-reason.sh"
       - "tests/forge-branch-clean-install.sh"
       - "tests/alpine-clean-install.sh"
+      - "tests/app-vars-files.sh"
       - ".github/workflows/verify-split-files.yml"
   workflow_dispatch:
 
@@ -245,6 +247,8 @@ jobs:
         run: bash tests/forge-branch-clean-install.sh
       - name: Clean install on Alpine tests
         run: bash tests/alpine-clean-install.sh
+      - name: App .vars file tests
+        run: bash tests/app-vars-files.sh
 
       - name: Prefetch lists cover every engine file
         run: |
@@ -317,6 +321,23 @@ jobs:
           check ui/build-ui.func  ui/API.txt     || fail=1
           check api/api.func      api/API.txt    || fail=1
           exit $fail
+      - name: Every var_* the engine reads is classified
+        # An app's own .vars file may set any var_* outside VAR_RESERVED, so each
+        # one the engine reads has to be placed: VAR_WHITELIST (validated),
+        # VAR_RESERVED (never from a file), VAR_PER_RUN (never saved from a run)
+        # or FILE_OK (harmless unvalidated).
+        run: |
+          set -euo pipefail
+          FILE_OK="var_docker_lxcfs var_extra_disk var_extra_disk_path var_profile var_ssh_import_glob"
+          LOCALS="var_key var_val"
+          listed=" $(sed -n '/^declare -ag VAR_\(WHITELIST\|RESERVED\|PER_RUN\)=(/,/^)/p' ui/defaults.func | grep -oE '\bvar_[a-z0-9_]+' | tr '\n' ' ') $FILE_OK $LOCALS "
+          fail=0
+          for v in $(grep -rhoE '\$\{?!?#?var_[a-z0-9_]+' $ENGINE_DIRS --include='*.func' | grep -oE 'var_[a-z0-9_]+' | sort -u); do
+            [[ "$listed" == *" $v "* ]] && continue
+            echo "::error file=ui/defaults.func::$v is read by the engine but in neither VAR_WHITELIST, VAR_RESERVED nor FILE_OK"
+            fail=1
+          done
+          exit $fail
 
       - name: Update mode prompt tests
         run: bash tests/update-mode-prompt.sh

docs/testing-builds.md

@@ -72,7 +72,7 @@ over.
 
 ## Two things left out on purpose
 
-**Not in `VAR_WHITELIST`.** `var_testurl` is not settable from a `.vars` file.
+**In `VAR_RESERVED`.** `var_testurl` is not settable from a `.vars` file.
 It describes the script, not the user's preferences, and making it configurable
 would mainly serve as a way to switch the request off.
 

docs/ui.md

@@ -32,6 +32,12 @@ Where settings come from and where they are kept: storage selection, the
 global and per-app `.vars` files, app defaults, and the diff shown when a
 saved default no longer matches what the script wants.
 
+`default.vars` takes the keys in `VAR_WHITELIST`, each one validated. An app's
+own `defaults/<app>.vars` also takes the app's own settings (any other `var_*`)
+as they are, since only the script knows what they mean. Keys in
+`VAR_RESERVED` are never read from a file. Saving app defaults keeps the app's
+own settings, but never takes the keys in `VAR_PER_RUN` from the current run.
+
 ## [`advanced.func`](../ui/advanced.func)
 
 `advanced_settings` — one function, the full interactive configuration path.

tests/app-vars-files.sh

@@ -0,0 +1,71 @@
+#!/usr/bin/env bash
+set -euo pipefail
+
+CORE_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")/.." && pwd)"
+DEFAULTS="$CORE_DIR/ui/defaults.func"
+eval "$(awk '/^declare -ag VAR_(WHITELIST|RESERVED|PER_RUN)=\(/,/^\)/' "$DEFAULTS")"
+for fn in _is_whitelisted_key _is_reserved_var_key load_vars_file _load_vars_file_to_map; do
+  eval "$(awk "/^${fn}\\(\\) \\{/,/^}/" "$DEFAULTS")"
+done
+
+WORK="$(mktemp -d)"
+trap 'rm -rf "$WORK"' EXIT
+WARNINGS="$WORK/warnings"
+msg_info() { :; }
+msg_ok() { :; }
+msg_warn() { echo "$*" >>"$WARNINGS"; }
+passed=0 failed=0
+
+check() {
+  if [[ "$3" == "$2" ]]; then
+    echo "PASS $1"
+    passed=$((passed + 1))
+  else
+    echo "FAIL $1: expected <$2>, got <$3>" >&2
+    failed=$((failed + 1))
+  fi
+}
+
+cat >"$WORK/test.vars" <<'EOF'
+var_cpu=4
+var_foo_port=8080
+var_foo_name="My App"
+var_ctid=150
+var_external_installer=yes
+var_Upper=1
+var_appversion=1.2.3
+EOF
+
+# $1: app_keys; prints the variables a load leaves set
+loaded() {
+  (
+    : >"$WARNINGS"
+    load_vars_file "$WORK/test.vars" no "" "$1"
+    printf '%s|' "${var_cpu:-}" "${var_foo_port:-}" "${var_foo_name:-}" "${var_ctid:-}" "${var_external_installer:-}" "${var_Upper:-}"
+  )
+}
+
+check "default.vars takes whitelisted keys only" "4||||||" "$(loaded no)"
+check "an app file also takes the app's own keys" "4|8080|My App||||" "$(loaded yes)"
+loaded yes >/dev/null
+check "each reserved key is reported" 2 "$(grep -c 'is ignored' "$WARNINGS")"
+check "a per-run key written into an app file by hand loads" 1.2.3 "$(load_vars_file "$WORK/test.vars" no "" yes && echo "${var_appversion:-}")"
+check "default.vars does not take a per-run key" "" "$(load_vars_file "$WORK/test.vars" no "" no && echo "${var_appversion:-}")"
+
+_load_vars_file_to_map "$WORK/test.vars"
+check "the map keeps the app's own keys" 8080 "${_VARS_IN[var_foo_port]:-}"
+check "the map drops reserved keys" "" "${_VARS_IN[var_ctid]:-}${_VARS_IN[var_external_installer]:-}"
+
+for key in "${VAR_WHITELIST[@]}" "${VAR_PER_RUN[@]}"; do
+  if _is_reserved_var_key "$key"; then
+    check "$key is in one list only" no yes
+  fi
+done
+for key in "${VAR_PER_RUN[@]}"; do
+  if _is_whitelisted_key "$key"; then
+    check "$key is in one list only" no yes
+  fi
+done
+
+printf '%s passed, %s failed\n' "$passed" "$failed"
+((failed == 0))

ui/API.txt

@@ -50,6 +50,7 @@ _eh_write_errinfo
 _get_current_ip
 _in_zone_tab
 _is_container_context
+_is_reserved_var_key
 _is_whitelisted_key
 _list_os_versions
 _list_templates

ui/defaults.func

@@ -404,37 +404,61 @@ base_settings() {
   resolve_os_version
 }
 
+# ------------------------------------------------------------------------------
+# var_* keys in .vars files
+#
+# VAR_WHITELIST: engine settings; load_vars_file validates each one.
+# VAR_RESERVED: never taken from a file, not even an app's own. Unique per
+#   container, declared by the script, read before any file is loaded, or a
+#   safety override that has to be asked for on the run that needs it.
+# VAR_PER_RUN: an app file may hold them, but saving app defaults never takes
+#   them from the current run; they describe that run, not the app.
+# Any other var_* is an app's own setting and loads only from its app file.
+# The frontend's .vars export mirrors VAR_WHITELIST and VAR_RESERVED.
+# ------------------------------------------------------------------------------
+declare -ag VAR_WHITELIST=(
+  var_apt_cacher var_apt_cacher_ip var_brg var_cmode var_cpu var_disk var_fuse var_github_token var_gpu var_http_no_proxy var_http_proxy var_inherit_host_ca var_keyctl
+  var_gateway var_hostname var_ipv6_method var_mac var_mknod var_mount_fs var_mtu
+  var_net var_nesting var_ns var_os var_post_install var_protection var_pw var_ram var_sdn_vnet var_searchdomain var_tags var_timezone var_tun var_unprivileged
+  var_verbose var_version var_vlan var_ssh var_ssh_authorized_key var_container_storage var_template_storage
+)
+declare -ag VAR_RESERVED=(
+  var_arm64 var_bridge var_ctid var_external_installer var_force_breaking_update var_ignore_breaking_changes
+  var_ignore_disable var_ignore_os_mismatch var_install var_ipv6_static var_testurl
+)
+declare -ag VAR_PER_RUN=(
+  var_appversion var_full_verbose var_unattended
+)
+
+_is_whitelisted_key() {
+  local k="$1" w
+  for w in "${VAR_WHITELIST[@]}"; do [[ "$k" == "$w" ]] && return 0; done
+  return 1
+}
+
+_is_reserved_var_key() {
+  local k="$1" w
+  for w in "${VAR_RESERVED[@]}"; do [[ "$k" == "$w" ]] && return 0; done
+  return 1
+}
+
 # ------------------------------------------------------------------------------
 # load_vars_file()
 #
 # - Safe parser for KEY=VALUE lines from vars files
 # - Used by default_var_settings and app defaults loading
-# - Only loads whitelisted var_* keys
+# - Loads whitelisted var_* keys; with app_keys=yes also the app's own ones
 # - Optional force parameter to override existing values (for app defaults)
 # - Optional protected list preserves genuinely user-exported var_* values
 # ------------------------------------------------------------------------------
 load_vars_file() {
   local file="$1"
   local force="${2:-no}"    # If "yes", override existing variables
   local protected="${3:-}"  # space-separated var_* keys the user genuinely exported before this file loaded; never overwritten
+  local app_keys="${4:-no}" # If "yes", also load var_* outside the whitelist (the app's own file)
   [ -f "$file" ] || return 0
   msg_info "Loading defaults from ${file}"
 
-  # Allowed var_* keys
-  local VAR_WHITELIST=(
-    var_apt_cacher var_apt_cacher_ip var_brg var_cmode var_cpu var_disk var_fuse var_github_token var_gpu var_http_no_proxy var_http_proxy var_inherit_host_ca var_keyctl
-    var_gateway var_hostname var_ipv6_method var_mac var_mknod var_mount_fs var_mtu
-    var_net var_nesting var_ns var_os var_post_install var_protection var_pw var_ram var_sdn_vnet var_searchdomain var_tags var_timezone var_tun var_unprivileged
-    var_verbose var_version var_vlan var_ssh var_ssh_authorized_key var_container_storage var_template_storage
-  )
-
-  # Whitelist check helper
-  _is_whitelisted() {
-    local k="$1" w
-    for w in "${VAR_WHITELIST[@]}"; do [ "$k" = "$w" ] && return 0; done
-    return 1
-  }
-
   # Protected check helper (genuinely user-exported vars, see $protected above)
   _is_protected() {
     local k="$1" p
@@ -452,7 +476,13 @@ load_vars_file() {
       local var_val="${BASH_REMATCH[2]}"
 
       [[ "$var_key" != var_* ]] && continue
-      _is_whitelisted "$var_key" || continue
+      if _is_reserved_var_key "$var_key"; then
+        msg_warn "${var_key} in ${file} is ignored: set it on the command line of the run that needs it"
+        continue
+      fi
+      if ! _is_whitelisted_key "$var_key"; then
+        [[ "$app_keys" == "yes" && "$var_key" =~ ^var_[a-z0-9_]+$ ]] || continue
+      fi
 
       # Strip inline comments (anything after unquoted #)
       # Only strip if not inside quotes
@@ -678,15 +708,6 @@ load_vars_file() {
 # - Calls base_settings "$VERBOSE" and echo_default
 # ------------------------------------------------------------------------------
 default_var_settings() {
-  # Allowed var_* keys (alphabetically sorted)
-  # Note: Removed var_ctid (can only exist once), var_ipv6_static (static IPs are unique)
-  local VAR_WHITELIST=(
-    var_apt_cacher var_apt_cacher_ip var_brg var_cmode var_cpu var_disk var_fuse var_github_token var_gpu var_http_no_proxy var_http_proxy var_inherit_host_ca var_keyctl
-    var_gateway var_hostname var_ipv6_method var_mac var_mknod var_mount_fs var_mtu
-    var_net var_nesting var_ns var_os var_post_install var_protection var_pw var_ram var_sdn_vnet var_searchdomain var_tags var_timezone var_tun var_unprivileged
-    var_verbose var_version var_vlan var_ssh var_ssh_authorized_key var_container_storage var_template_storage
-  )
-
   # Snapshot: environment variables (highest precedence)
   declare -A _HARD_ENV=()
   local _k
@@ -806,15 +827,6 @@ EOF
     # Silent creation - no output message
   }
 
-  # Whitelist check
-  local _is_whitelisted_key
-  _is_whitelisted_key() {
-    local k="$1"
-    local w
-    for w in "${VAR_WHITELIST[@]}"; do [ "$k" = "$w" ] && return 0; done
-    return 1
-  }
-
   # 1) Ensure file exists
   _ensure_default_vars
 
@@ -862,33 +874,6 @@ get_app_defaults_path() {
   echo "${base}/defaults/${n}.vars"
 }
 
-# ------------------------------------------------------------------------------
-# maybe_offer_save_app_defaults
-#
-# - Called after advanced_settings returned with fully chosen values.
-# - If no <nsapp>.vars exists, offers to persist current advanced settings
-#   into /usr/local/community-scripts/defaults/<nsapp>.vars
-# - Only writes whitelisted var_* keys.
-# - Extracts raw values from flags like ",gw=..." ",mtu=..." etc.
-# ------------------------------------------------------------------------------
-if ! declare -p VAR_WHITELIST >/dev/null 2>&1; then
-  # Note: Removed var_ctid (can only exist once), var_ipv6_static (static IPs are unique)
-  declare -ag VAR_WHITELIST=(
-    var_apt_cacher var_apt_cacher_ip var_brg var_cmode var_cpu var_disk var_fuse var_github_token var_gpu var_http_no_proxy var_http_proxy var_inherit_host_ca var_keyctl
-    var_gateway var_hostname var_ipv6_method var_mac var_mknod var_mount_fs var_mtu
-    var_net var_nesting var_ns var_os var_post_install var_protection var_pw var_ram var_sdn_vnet var_searchdomain var_tags var_timezone var_tun var_unprivileged
-    var_verbose var_version var_vlan var_ssh var_ssh_authorized_key var_container_storage var_template_storage
-  )
-fi
-
-# Global whitelist check function (used by _load_vars_file_to_map and others)
-_is_whitelisted_key() {
-  local k="$1"
-  local w
-  for w in "${VAR_WHITELIST[@]}"; do [ "$k" = "$w" ] && return 0; done
-  return 1
-}
-
 _sanitize_value() {
   # Disallow Command-Substitution / Shell-Meta
   case "$1" in
@@ -927,9 +912,7 @@ _load_vars_file_to_map() {
     val=$(printf "%s" "$line" | cut -d= -f2-)
     case "$key" in
     var_*)
-      if _is_whitelisted_key "$key"; then
-        _VARS_IN["$key"]="$val"
-      fi
+      _is_reserved_var_key "$key" || _VARS_IN["$key"]="$val"
       ;;
     esac
   done <"$file"
@@ -1105,6 +1088,30 @@ _build_current_app_vars_tmp() {
     [ -n "$_tpl_storage" ] && echo "var_template_storage=$(_sanitize_value "$_tpl_storage")"
     [ -n "$_ct_storage" ] && echo "var_container_storage=$(_sanitize_value "$_ct_storage")"
     [ -n "${var_post_install:-}" ] && echo "var_post_install=$(_sanitize_value "${var_post_install}")"
+
+    # The app's own settings are not in the dialog: keep what the file holds,
+    # and take what this run was started with.
+    local -A _own=()
+    local _k _f
+    _f="$(get_app_defaults_path)"
+    if [[ -f "$_f" ]]; then
+      _load_vars_file_to_map "$_f"
+      for _k in "${!_VARS_IN[@]}"; do
+        _is_whitelisted_key "$_k" || _own["$_k"]="${_VARS_IN[$_k]}"
+      done
+    fi
+    for _k in $(compgen -e var_); do
+      [[ "$_k" =~ ^var_[a-z0-9_]+$ && "${!_k}" != *$'\n'* ]] || continue
+      if _is_whitelisted_key "$_k" || _is_reserved_var_key "$_k" || [[ " ${VAR_PER_RUN[*]} " == *" $_k "* ]]; then continue; fi
+      _own["$_k"]="\"${!_k}\""
+    done
+    if ((${#_own[@]} > 0)); then
+      echo
+      echo "# ${APP}'s own settings"
+      while IFS= read -r _k; do
+        echo "${_k}=${_own[$_k]}"
+      done < <(printf '%s\n' "${!_own[@]}" | sort)
+    fi
   } >"$tmpf"
 
   echo "$tmpf"
@@ -1131,7 +1138,7 @@ maybe_offer_save_app_defaults() {
     if whiptail --backtitle "Proxmox VE Helper Scripts" \
       --yesno "Save these advanced settings as defaults for ${APP}?\n\nThis will create:\n${app_vars_path}" 12 72; then
       mkdir -p "$(dirname "$app_vars_path")"
-      install -m 0644 "$new_tmp" "$app_vars_path"
+      install -m 0600 "$new_tmp" "$app_vars_path"
       msg_ok "Saved app defaults: ${app_vars_path}"
     fi
     rm -f "$new_tmp" "$diff_tmp"
@@ -1165,7 +1172,7 @@ maybe_offer_save_app_defaults() {
 
     case "$sel" in
     "Update Defaults")
-      install -m 0644 "$new_tmp" "$app_vars_path"
+      install -m 0600 "$new_tmp" "$app_vars_path"
       msg_ok "Updated app defaults: ${app_vars_path}"
       break
       ;;

ui/menu.func

@@ -334,7 +334,7 @@ install_script() {
         header_info
         echo -e "${DEFAULT}${BOLD}${BL}Using App Defaults for ${APP} on node $PVEHOST_NAME${CL}"
         METHOD="appdefaults"
-        load_vars_file "$(get_app_defaults_path)" "yes" # Force override script defaults
+        load_vars_file "$(get_app_defaults_path)" "yes" "" "yes" # Force override script defaults, the app's own settings included
         base_settings
         echo_default
         defaults_target="$(get_app_defaults_path)"