diff --git a/api/telemetry.func b/api/telemetry.func index 1659aa8..8a51958 100644 --- a/api/telemetry.func +++ b/api/telemetry.func @@ -373,7 +373,8 @@ telemetry_new_attempt() { # does whiptail without a terminal. The user decided; the tool did not fail. _tm_dialog_exit() { [[ "$2" == 1 || "$2" == 255 ]] || return 1 - [[ "$1" =~ ^((local|export|declare)[[:space:]]+)?([A-Za-z_][A-Za-z0-9_]*=\$\()?whiptail[[:space:]] ]] + [[ "$1" =~ ^((local|export|declare)[[:space:]]+)?([A-Za-z_][A-Za-z0-9_]*=\$\()?whiptail[[:space:]] ]] && return 0 + [[ "$2" == 1 && "$1" =~ ^(read|select)[[:space:]] ]] } # EXIT trap set by init_tool_telemetry. is the one running when the @@ -384,7 +385,7 @@ _telemetry_report_exit() { local status="success" reason="" if [[ "$ec" -ne 0 ]]; then status="failed" - if _tm_dialog_exit "$cmd" "$ec"; then + if _tm_dialog_exit "${FAILED_COMMAND:-$cmd}" "$ec"; then status="aborted" reason="Dialog cancelled, or no terminal to show it" elif [[ -z "${FAILED_COMMAND:-}" && -n "$cmd" && ! "$cmd" =~ ^(exit|return)([[:space:]]|$) ]]; then diff --git a/core/core.func b/core/core.func index f693f54..fb6a394 100644 --- a/core/core.func +++ b/core/core.func @@ -1015,6 +1015,9 @@ _cs_reattach_output() { _cs_shield_hangup() { ( trap '' HUP + # apt restores the terminal it was given and exits 100 when it is gone. + [[ -t 0 ]] && exec >"$logfile" 2>&1 rc=$? fi + # A hangup during the call moved the output inside its redirection, which + # bash undid on return. + _cs_reattach_output # Restore error handling ONLY if it was active before this call. if $_restore_errexit; then set -Eeuo pipefail - trap 'error_handler' ERR + if [[ -n "$_err_trap" ]]; then + eval "$_err_trap" + elif declare -F error_handler >/dev/null; then + trap 'error_handler' ERR + fi fi if [[ $rc -ne 0 ]]; then @@ -2782,7 +2796,16 @@ arm64_notice() { # - Stops spinner safely and writes warning to active log # ------------------------------------------------------------------------------ on_hup_keepalive() { + if declare -F on_hangup >/dev/null; then + on_hangup + return + fi stop_spinner + # The next write to the dead terminal failed with EIO and ended the tool. + local log + log="$(get_active_logfile 2>/dev/null)" + exec >"${log:-/dev/null}" 2>&1 + export _CS_HANGUP=1 CS_ANIMATION_OK=0 _CS_HANGUP_LOG="${log:-/dev/null}" log_msg "[WARN] Received SIGHUP (terminal hangup). Continuing execution in background." } diff --git a/core/error_handler.func b/core/error_handler.func index 734790c..2430e6a 100644 --- a/core/error_handler.func +++ b/core/error_handler.func @@ -616,7 +616,8 @@ error_handler() { # Before any output, or the spinner overwrites it declare -f stop_spinner >/dev/null 2>&1 && { stop_spinner 2>/dev/null || true; } - printf "\e[?25h" + declare -f _cs_reattach_output >/dev/null 2>&1 && _cs_reattach_output + printf "\e[?25h" 2>/dev/null || true local explanation explanation="$(_eh_explanation "$exit_code" "$command")" @@ -691,7 +692,11 @@ _stop_container_if_installing() { # ------------------------------------------------------------------------------ on_exit() { local exit_code=$? - [[ "${_CS_HANGUP:-}" == "1" ]] && echo "[INFO] $(date '+%Y-%m-%d %H:%M:%S') on_exit with code ${exit_code} (pid $$)" + # A write to the dead terminal ended this trap before the host got its marker. + if [[ "${_CS_HANGUP:-}" == "1" ]]; then + declare -f _cs_reattach_output >/dev/null 2>&1 && _cs_reattach_output + echo "[INFO] $(date '+%Y-%m-%d %H:%M:%S') on_exit with code ${exit_code} (pid $$)" || true + fi # Before anything else: the timings are only useful if they survive a failure. declare -f dev_mode_timing_summary >/dev/null 2>&1 && dev_mode_timing_summary diff --git a/lib/db.func b/lib/db.func index e90ab68..8168547 100644 --- a/lib/db.func +++ b/lib/db.func @@ -1390,7 +1390,7 @@ setup_postgresql() { setup_deb822_repo \ "pgdg" \ - "https://www.postgresql.org/media/keys/ACCC4CF8.asc" \ + "https://apt.postgresql.org/pub/repos/apt/ACCC4CF8.asc" \ "https://apt.postgresql.org/pub/repos/apt" \ "$SUITE" \ "main" diff --git a/lib/system.func b/lib/system.func index 6e64f98..8f6a5ba 100644 --- a/lib/system.func +++ b/lib/system.func @@ -1954,7 +1954,7 @@ setup_deb822_repo() { # Import GPG key (auto-detect binary vs ASCII-armored format) local tmp_gpg tmp_gpg=$(mktemp) || return 252 - curl -fsSL "$gpg_url" -o "$tmp_gpg" || { + CURL_RETRIES=4 CURL_TIMEOUT=30 curl_with_retry "$gpg_url" "$tmp_gpg" || { msg_error "Failed to download GPG key for ${name} from: ${gpg_url}" msg_error "Hint: Check network connectivity. If behind a proxy or firewall, ensure HTTPS access to $(echo "$gpg_url" | grep -oE 'https?://[^/]+') is allowed." rm -f "$tmp_gpg" diff --git a/pve/backend.func b/pve/backend.func index b06ad33..cc53a90 100644 --- a/pve/backend.func +++ b/pve/backend.func @@ -202,8 +202,10 @@ _pve_ct_stopped_by() { # when someone stopped it (an abort), 199 when it stopped by itself. # ------------------------------------------------------------------------------ _pve_ct_gone_during_setup() { - local by - [[ "$(pct status "$CTID" 2>/dev/null)" == "status: running" ]] && return 1 + local by st + # A closed console killed pct status itself, and its empty answer read as "not running". + st="$(_cs_shield_hangup pct status "$CTID" 2>/dev/null)" || st="$(_cs_shield_hangup pct status "$CTID" 2>/dev/null)" + [[ "$st" == "status: running" ]] && return 1 by="$(_pve_ct_stopped_by)" if [[ -n "$by" ]]; then msg_warn "Container ${CTID} was stopped during its setup (${by})" @@ -1190,10 +1192,10 @@ EOF https://dl-cdn.alpinelinux.org/alpine/latest-stable/main https://dl-cdn.alpinelinux.org/alpine/latest-stable/community EOF' - pct exec "$CTID" -- ash -c 'set -a; [ -f /etc/profile.d/90-http-proxy.sh ] && . /etc/profile.d/90-http-proxy.sh; set +a; apk add bash newt curl openssh nano mc ncurses jq' >>"$BUILD_LOG" 2>&1 || { + _cs_shield_hangup pct exec "$CTID" -- ash -c 'set -a; [ -f /etc/profile.d/90-http-proxy.sh ] && . /etc/profile.d/90-http-proxy.sh; set +a; apk add bash newt curl openssh nano mc ncurses jq' >>"$BUILD_LOG" 2>&1 || { msg_warn "apk install failed (dl-cdn.alpinelinux.org), trying alternate mirrors..." local alpine_exit=0 - pct exec "$CTID" -- ash -c ' + _cs_shield_hangup pct exec "$CTID" -- ash -c ' set -a [ -f /etc/profile.d/90-http-proxy.sh ] && . /etc/profile.d/90-http-proxy.sh set +a @@ -1271,7 +1273,7 @@ EOF pct exec "$CTID" -- bash -c "echo -e 'nameserver 8.8.8.8\nnameserver 1.1.1.1' >/etc/resolv.conf" fi - pct exec "$CTID" -- bash -c "set -a; [ -f /etc/profile.d/90-http-proxy.sh ] && . /etc/profile.d/90-http-proxy.sh; set +a; ${_ct_env} apt-get update 2>&1 && apt-get install -y ${_base_pkgs} 2>&1" >>"$BUILD_LOG" 2>&1 || if _pve_ct_gone_during_setup; then + _cs_shield_hangup pct exec "$CTID" -- bash -c "set -a; [ -f /etc/profile.d/90-http-proxy.sh ] && . /etc/profile.d/90-http-proxy.sh; set +a; ${_ct_env} apt-get update 2>&1 && apt-get install -y ${_base_pkgs} 2>&1" >>"$BUILD_LOG" 2>&1 || if _pve_ct_gone_during_setup; then install_exit_code=$_CS_SETUP_GONE_RC else local failed_mirror @@ -1279,7 +1281,7 @@ EOF msg_warn "apt-get update failed (${failed_mirror})." msg_custom "ℹ️" "${YW}" "Probing alternate mirrors (this can take 1-2 minutes on network issues)" local mirror_exit=0 - pct exec "$CTID" -- env APT_BASE="$_base_pkgs" bash -c ' + _cs_shield_hangup pct exec "$CTID" -- env APT_BASE="$_base_pkgs" bash -c ' set -a [ -f /etc/profile.d/90-http-proxy.sh ] && . /etc/profile.d/90-http-proxy.sh set +a diff --git a/pve/vm-core.func b/pve/vm-core.func index c11172b..ad4dfec 100644 --- a/pve/vm-core.func +++ b/pve/vm-core.func @@ -1118,10 +1118,15 @@ vm_prompt_vmid() { while true; do if vm_dialog inputbox "VIRTUAL MACHINE ID" "Set Virtual Machine ID" 8 58 "$default_vmid" --cancel-button Exit-Script; then - VMID="$VM_DIALOG_RESULT" + VMID="${VM_DIALOG_RESULT//[[:space:]]/}" if [ -z "$VMID" ]; then VMID=$(get_valid_nextid) fi + if [[ ! "$VMID" =~ ^[1-9][0-9]{2,8}$ ]]; then + echo -e "${CROSS}${RD} ID $VMID is not a number from 100 to 999999999${CL}" + sleep 2 + continue + fi if pct status "$VMID" &>/dev/null || qm status "$VMID" &>/dev/null; then echo -e "${CROSS}${RD} ID $VMID is already in use${CL}" sleep 2 @@ -1581,15 +1586,23 @@ vm_prompt_ram() { vm_prompt_bridge() { local default_bridge="${1:-vmbr0}" - if vm_dialog inputbox "BRIDGE" "Set a Bridge" 8 58 "$default_bridge" --cancel-button Exit-Script; then - BRG="$VM_DIALOG_RESULT" - if [ -z "$BRG" ]; then - BRG="$default_bridge" + while true; do + if vm_dialog inputbox "BRIDGE" "Set a Bridge" 8 58 "$default_bridge" --cancel-button Exit-Script; then + BRG="${VM_DIALOG_RESULT//[[:space:]]/}" + if [ -z "$BRG" ]; then + BRG="$default_bridge" + fi + if ! ip -o link show "$BRG" &>/dev/null; then + echo -e "${CROSS}${RD} Bridge $BRG does not exist on this host${CL}" + sleep 2 + continue + fi + echo -e "${BRIDGE}${BOLD}${DGN}Bridge: ${BGN}$BRG${CL}" + break + else + exit_script fi - echo -e "${BRIDGE}${BOLD}${DGN}Bridge: ${BGN}$BRG${CL}" - else - exit_script - fi + done } vm_prompt_mac() { diff --git a/tests/console-hangup.sh b/tests/console-hangup.sh index 3fccfd5..ef69e18 100644 --- a/tests/console-hangup.sh +++ b/tests/console-hangup.sh @@ -6,7 +6,7 @@ set -uo pipefail CORE_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")/.." && pwd)" -eval "$(awk '/^(silent|_cs_shield_hangup|_cs_reattach_output)[(][)] [{]/,/^}/' "$CORE_DIR/core/core.func")" +eval "$(awk '/^(silent|_cs_shield_hangup|_cs_reattach_output|on_hup_keepalive)[(][)] [{]/,/^}/' "$CORE_DIR/core/core.func")" eval "$(awk '/^(on_hangup|on_exit|_eh_quiet_spinner)[(][)] [{]/,/^}/' "$CORE_DIR/core/error_handler.func")" eval "$(awk '/^(_pve_wait_for_detached_install|_pve_ct_stopped_by|_pve_ct_gone_during_setup)[(][)] [{]/,/^}/' "$CORE_DIR/pve/backend.func")" @@ -56,6 +56,26 @@ expect "later output lands in the log" 1 "$(grep -c 'after hangup' "$LOG")" expect "the hangup is noted in the log" 1 "$(grep -c 'The terminal closed' "$LOG")" expect "the run knows it is detached" 1 "$(grep -c 'flag=1' "$LOG")" +# A hangup during silent(): on_hangup moved the output inside the call's own +# redirection, bash undid it on return, and the next echo hit the dead terminal. +: >"$LOG" +out="$( (VERBOSE=no; trap on_hangup HUP; silent sh -c 'kill -HUP $PPID; sleep 0.3'; echo "after silent") 2>&1 )" +expect "output after the interrupted call stays off the terminal" "" "$out" +expect "and lands in the log" 1 "$(grep -c 'after silent' "$LOG")" +out="$(echo piped | _cs_shield_hangup cat)" +expect "a piped stdin still reaches the shielded command" "piped" "$out" + +# on_hup_keepalive(): tools without catch_errors kept writing to the dead +# terminal, and the next echo or read ended them. +: >"$LOG" +out="$( (unset -f on_hangup; log_msg() { echo "$*"; }; trap on_hup_keepalive HUP; kill -HUP $BASHPID; echo "after hangup"; read -r x || echo "read eof"; echo "flag=${_CS_HANGUP:-}") 2>&1 )" +expect "a tool survives the hangup without on_hangup" "" "$out" +expect "its output lands in the log" 1 "$(grep -c 'after hangup' "$LOG")" +expect "a prompt reads EOF instead of the dead terminal" 1 "$(grep -c 'read eof' "$LOG")" +expect "and the tool knows it is detached" 1 "$(grep -c 'flag=1' "$LOG")" +out="$( (trap on_hup_keepalive HUP; kill -HUP $BASHPID; echo "flag=${_CS_HANGUP:-}") 2>&1 )" +expect "with on_hangup loaded it delegates" 1 "$(grep -c 'The terminal closed' "$LOG")" + # on_exit(): a clean install leaves .done for the host; a failed one does not. INSTALL_LOG="$ROOT/.install-abc.log" export INSTALL_LOG diff --git a/tests/silent-err-trap.sh b/tests/silent-err-trap.sh new file mode 100644 index 0000000..e3d3023 --- /dev/null +++ b/tests/silent-err-trap.sh @@ -0,0 +1,43 @@ +#!/usr/bin/env bash +# silent() re-armed `trap error_handler ERR` after every call. A script run +# through pct exec has no error_handler, so add-tailscale-lxc and +# add-netbird-lxc ended with exit 127 instead of apt's error. +set -uo pipefail + +CORE_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")/.." && pwd)" +SILENT="$(awk '/^(silent|_cs_shield_hangup|_cs_reattach_output)[(][)] [{]/,/^}/' "$CORE_DIR/core/core.func")" + +TEST_DIR="$(mktemp -d)" +trap 'rm -rf -- "$TEST_DIR"' EXIT +export SILENT LOG="$TEST_DIR/install.log" +passed=0 failed=0 + +expect() { + if [[ "$2" == "$3" ]]; then + echo "PASS $1" + passed=$((passed + 1)) + else + echo "FAIL $1: expected <$2>, got <$3>" >&2 + failed=$((failed + 1)) + fi +} +run() { + bash -c 'eval "$SILENT"; get_active_logfile() { echo "$LOG"; }; '"$1" 2>&1 + echo "rc=$?" +} + +out="$(run 'set -e; silent true; f() { return 100; }; f')" +expect "without error_handler the failure keeps its own code" "rc=100" "$(tail -n1 <<<"$out")" +expect "and nothing calls a missing error_handler" 0 "$(grep -c 'error_handler' <<<"$out")" + +out="$(run 'set -eE; trap "echo MINE; exit 9" ERR; silent true; false')" +expect "the caller's own ERR trap survives" "MINE rc=9" "$(echo $out)" + +out="$(run 'error_handler() { echo HANDLED; exit 8; }; set -e; trap error_handler ERR; silent true; false')" +expect "error_handler is still armed when the trap is not visible" "HANDLED rc=8" "$(echo $out)" + +out="$(run 'set -e; silent sh -c "exit 3" || echo "caught $?"')" +expect "a handled failure stays handled" "caught 3 rc=0" "$(echo $out)" + +printf '%s passed, %s failed\n' "$passed" "$failed" +((failed == 0)) diff --git a/tests/telemetry-trace.sh b/tests/telemetry-trace.sh index 85a0a23..b00c384 100644 --- a/tests/telemetry-trace.sh +++ b/tests/telemetry-trace.sh @@ -173,6 +173,16 @@ reset _telemetry_report_exit 255 'whiptail --backtitle x --msgbox "About fstrim" 16 88' expect "Esc in a dialog is an abort" aborted "$(json_field "$SENT" status)" reset +_telemetry_report_exit 1 'read -rp "This will add Tailscale to an existing LXC Container ONLY. Proceed (y/n)? " yn' +expect "a prompt that reads no answer is an abort" aborted "$(json_field "$SENT" status)" +reset +FAILED_COMMAND='read -rp "Proceed (y/n)? " yn' +_telemetry_report_exit 1 'exit "$exit_code"' +expect "so is one an ERR trap ended with exit" aborted "$(json_field "$SENT" status)" +reset +_telemetry_report_exit 2 'read -r line <"$file"' +expect "a read that fails otherwise stays a failure" failed "$(json_field "$SENT" status)" +reset _telemetry_report_exit 255 'fstrim_output=$(pct fstrim 105 2>&1)' expect "any other command stays a failure" failed "$(json_field "$SENT" status)" expect "and is named" 'fstrim_output=$(pct fstrim 105 2>&1)' "$(json_field "$SENT" failed_command)" diff --git a/tests/vm-prompts.sh b/tests/vm-prompts.sh new file mode 100644 index 0000000..2a9c64a --- /dev/null +++ b/tests/vm-prompts.sh @@ -0,0 +1,54 @@ +#!/usr/bin/env bash +# A VM ID typed with a trailing space reached qm create split and qm disk import +# quoted ("got '100 '"), and a mistyped bridge only failed at qm create. +set -uo pipefail + +CORE_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")/.." && pwd)" +eval "$(awk '/^(vm_prompt_vmid|vm_prompt_bridge)[(][)] [{]/,/^}/' "$CORE_DIR/pve/vm-core.func")" + +passed=0 failed=0 +CROSS="" RD="" CL="" CONTAINERID="" BOLD="" DGN="" BGN="" BRIDGE="" +expect() { + if [[ "$2" == "$3" ]]; then + echo "PASS $1" + passed=$((passed + 1)) + else + echo "FAIL $1: expected <$2>, got <$3>" >&2 + failed=$((failed + 1)) + fi +} + +ANSWERS=() +vm_dialog() { + VM_DIALOG_RESULT="${ANSWERS[0]}" + ANSWERS=("${ANSWERS[@]:1}") +} +exit_script() { exit 1; } +sleep() { :; } +get_valid_nextid() { echo 105; } +pct() { return 1; } +qm() { [[ "$2" == 200 ]]; } +ip() { [[ "$4" == vmbr0 || "$4" == vmbr1 ]]; } + +ANSWERS=("100 ") +vm_prompt_vmid >/dev/null +expect "spaces around the ID are dropped" 100 "$VMID" + +ANSWERS=("abc" "12" "200" "201") +vm_prompt_vmid >/dev/null +expect "a non-number, a too small and a used ID are asked again" 201 "$VMID" + +ANSWERS=("") +vm_prompt_vmid >/dev/null +expect "an empty answer takes the next free ID" 105 "$VMID" + +ANSWERS=("vmbr080" " vmbr1 ") +vm_prompt_bridge vmbr0 >/dev/null +expect "a bridge that does not exist is asked again" vmbr1 "$BRG" + +ANSWERS=("") +vm_prompt_bridge vmbr0 >/dev/null +expect "an empty bridge takes the default" vmbr0 "$BRG" + +printf '%s passed, %s failed\n' "$passed" "$failed" +((failed == 0)) diff --git a/ui/advanced.func b/ui/advanced.func index a0ca7af..acdfd99 100644 --- a/ui/advanced.func +++ b/ui/advanced.func @@ -160,13 +160,19 @@ advanced_settings() { default_on="OFF" default_off="ON" } + # Users of apps that need a privileged container followed "recommended" and the install refused. + local unpriv_label="Unprivileged (recommended)" priv_label="Privileged" + [[ "${var_unprivileged:-1}" == "0" ]] && { + unpriv_label="Unprivileged" + priv_label="Privileged (app default)" + } if result=$(whiptail --backtitle "Proxmox VE Helper Scripts [Step $STEP/$MAX_STEP]" \ --title "CONTAINER TYPE" \ --ok-button "Next" --cancel-button "Exit" \ --radiolist "\nChoose container type:\n\nUse SPACE to select, ENTER to confirm." 14 58 2 \ - "1" "Unprivileged (recommended)" $default_on \ - "0" "Privileged" $default_off \ + "1" "$unpriv_label" $default_on \ + "0" "$priv_label" $default_off \ 3>&1 1>&2 2>&3); then [[ -n "$result" ]] && _ct_type="$result" ((STEP++))