From ebcebdf41af9cbc96acba21a41ddf12fffae65d3 Mon Sep 17 00:00:00 2001 From: UNITRONIX <36471318+UNITRONIX@users.noreply.github.com> Date: Sat, 26 Sep 2026 02:27:32 +0200 Subject: [PATCH] fix(installer): bound native Go build and preserve dirty clones Refs #371 Add stage-specific watchdogs for module graph validation and Go builds, and make native source branch recovery non-destructive. Add regression coverage for timeout propagation, dirty clones, runtime data preservation, and clone failure recovery. Tests: Git Bash bash -n; scripts/test-native-installer.sh; installer-focused npm tests (4 suites, 33 tests); IDE lints. Thanks: INSOLVE (Honorary); Marco Jakobs (@jacotec); MyNameisStitch (@MyNameisStitch); Redspin (@playerumpknow) --- .github/workflows/installer-ci.yml | 4 + betterdesk.sh | 151 ++++++++++++++++++++++++++--- install.sh | 106 ++++++++++++++++++-- scripts/test-native-installer.sh | 128 ++++++++++++++++++++++++ 4 files changed, 365 insertions(+), 24 deletions(-) create mode 100644 scripts/test-native-installer.sh diff --git a/.github/workflows/installer-ci.yml b/.github/workflows/installer-ci.yml index 8c71bd3f..a6803c91 100644 --- a/.github/workflows/installer-ci.yml +++ b/.github/workflows/installer-ci.yml @@ -63,6 +63,7 @@ jobs: install.sh \ betterdesk.sh \ betterdesk-docker.sh \ + scripts/test-native-installer.sh \ scripts/test-betterdesk-node-install.sh \ scripts/test-postgresql-hba.sh) - name: Run ShellCheck @@ -71,10 +72,13 @@ jobs: install.sh \ betterdesk.sh \ betterdesk-docker.sh \ + scripts/test-native-installer.sh \ scripts/test-betterdesk-node-install.sh \ scripts/test-postgresql-hba.sh - name: Test native Node.js installer recovery run: bash scripts/test-betterdesk-node-install.sh + - name: Test native installer watchdog and clone recovery + run: bash scripts/test-native-installer.sh - name: Test PostgreSQL pg_hba.conf setup run: bash scripts/test-postgresql-hba.sh diff --git a/betterdesk.sh b/betterdesk.sh index 9836ea21..679e4c25 100644 --- a/betterdesk.sh +++ b/betterdesk.sh @@ -191,6 +191,13 @@ GO_MIN_VERSION="1.26.6" GO_DOWNLOAD_VERSION="1.26.6" # Maximum time allowed for the first module download on a native install. GO_MODULE_DOWNLOAD_TIMEOUT="${GO_MODULE_DOWNLOAD_TIMEOUT:-600}" +# Maximum time allowed to validate the downloaded module graph. +GO_MODULE_GRAPH_TIMEOUT="${GO_MODULE_GRAPH_TIMEOUT:-120}" +# Maximum time allowed to compile the Go server. +GO_BUILD_TIMEOUT="${GO_BUILD_TIMEOUT:-1800}" +# Heartbeat/deadline settings for bounded Go commands. +GO_COMMAND_HEARTBEAT_INTERVAL="${GO_COMMAND_HEARTBEAT_INTERVAL:-15}" +GO_COMMAND_KILL_AFTER="${GO_COMMAND_KILL_AFTER:-15}" # Short HTTPS probe before go mod download (fail fast on blocked proxy/DNS). GO_MODULE_PREFLIGHT_TIMEOUT="${GO_MODULE_PREFLIGHT_TIMEOUT:-20}" # Set when preflight detects IPv4 OK but IPv6 broken (common on GCP VMs). @@ -3106,7 +3113,94 @@ _go_kill_process_group() { fi } -# Run go mod download with a hard bash process-group deadline (primary guard). +# Run a Go command with a hard deadline and visible progress. +_run_go_command_bounded() { + local label="$1" + local deadline="$2" + shift 2 + local command_pid="" + local command_pgid="" + local heartbeat_pid="" + local elapsed=0 + local status=0 + local heartbeat_interval="${GO_COMMAND_HEARTBEAT_INTERVAL}" + local kill_after="${GO_COMMAND_KILL_AFTER}" + + case "$heartbeat_interval" in + ''|*[!0-9]*) heartbeat_interval=15 ;; + esac + case "$kill_after" in + ''|*[!0-9]*) kill_after=15 ;; + esac + + if command -v setsid &> /dev/null; then + setsid "$@" & + else + "$@" & + fi + command_pid=$! + + if command -v ps &> /dev/null; then + command_pgid=$(ps -o pgid= -p "$command_pid" 2>/dev/null | tr -d ' ' || true) + case "$command_pgid" in + ''|*[!0-9]*) command_pgid="" ;; + esac + fi + + print_info "${label} started (PID ${command_pid}, deadline ${deadline}s)" + + ( + heartbeat_elapsed=0 + while kill -0 "$command_pid" 2>/dev/null; do + sleep "$heartbeat_interval" + if ! kill -0 "$command_pid" 2>/dev/null; then + break + fi + heartbeat_elapsed=$((heartbeat_elapsed + heartbeat_interval)) + printf 'ℹ still %s... %ss elapsed (watchdog active)\n' \ + "$label" "$heartbeat_elapsed" >&2 + done + ) & + heartbeat_pid=$! + + while kill -0 "$command_pid" 2>/dev/null; do + if [ "$elapsed" -ge "$deadline" ]; then + print_error "${label} exceeded ${deadline}s" + if [ -n "$command_pgid" ] && [ "$command_pgid" -gt 1 ] 2>/dev/null; then + _go_kill_process_group "$command_pgid" "$label" + else + kill -TERM "$command_pid" 2>/dev/null || true + sleep "$kill_after" + if kill -0 "$command_pid" 2>/dev/null; then + kill -KILL "$command_pid" 2>/dev/null || true + fi + fi + status=124 + break + fi + sleep 1 + elapsed=$((elapsed + 1)) + done + + if [ "$status" -ne 124 ]; then + if wait "$command_pid"; then + status=0 + else + status=$? + fi + else + wait "$command_pid" 2>/dev/null || true + fi + + if [ -n "$heartbeat_pid" ]; then + kill "$heartbeat_pid" 2>/dev/null || true + wait "$heartbeat_pid" 2>/dev/null || true + fi + + return "$status" +} + +# Run go mod download with a hard bash deadline (primary guard). # GNU timeout alone was insufficient on some cloud VMs (#371). _run_go_mod_download_bounded() { local deadline="${GO_MODULE_DOWNLOAD_TIMEOUT}" @@ -3175,14 +3269,6 @@ _run_go_mod_download_bounded() { wait "$heartbeat_pid" 2>/dev/null || true fi - # Keep IPv6 disabled until we confirm the module graph is readable. - if [ "$status" -eq 0 ]; then - if ! go list -m all >/dev/null 2>&1; then - print_error "go mod download reported success but 'go list -m all' failed" - status=1 - fi - fi - _go_ipv4_force_end return "$status" } @@ -3230,8 +3316,8 @@ compile_go_server() { return 1 fi - # Download dependencies. Keep this visible and hard-bounded (bash PGID watchdog). - print_info "Downloading Go modules (timeout: ${GO_MODULE_DOWNLOAD_TIMEOUT}s, kill-after: 15s, setsid watchdog)..." + # Download dependencies. Keep this visible and hard-bounded (bash watchdog). + print_info "Downloading Go modules (timeout: ${GO_MODULE_DOWNLOAD_TIMEOUT}s, kill-after: 15s, process watchdog)..." local module_download_status=0 # Keep the command in the if condition so `set -e` does not abort before # we can capture the actual watchdog/download exit status. Do not use `!` @@ -3252,9 +3338,43 @@ compile_go_server() { print_error "On GCP/cloud VMs with broken IPv6, confirm IPv4 works: curl -4 -I https://proxy.golang.org/" return 1 fi + + print_info "Go module download completed; validating module graph (timeout: ${GO_MODULE_GRAPH_TIMEOUT}s)..." + local module_graph_status=0 + if _run_go_command_bounded "validating Go module graph" "$GO_MODULE_GRAPH_TIMEOUT" go list -m all; then + module_graph_status=0 + else + module_graph_status=$? + fi + if [ "$module_graph_status" -ne 0 ]; then + if [ "$module_graph_status" -eq 124 ] || [ "$module_graph_status" -eq 137 ]; then + print_error "Go module graph validation timed out after ${GO_MODULE_GRAPH_TIMEOUT}s" + else + print_error "Go module graph validation failed (exit code ${module_graph_status})" + fi + print_error "The module cache may be incomplete; retry the native installation after checking GOPROXY and disk space" + return 1 + fi + print_success "Go module graph validated" # Build with optimizations - CGO_ENABLED=0 go build -ldflags="-s -w -X main.Version=${VERSION}" -o "$output_name" . + print_info "Building BetterDesk server (timeout: ${GO_BUILD_TIMEOUT}s)..." + local build_status=0 + if _run_go_command_bounded "building Go server" "$GO_BUILD_TIMEOUT" \ + env CGO_ENABLED=0 "$go_bin" build \ + -ldflags="-s -w -X main.Version=${VERSION}" -o "$output_name" .; then + build_status=0 + else + build_status=$? + fi + if [ "$build_status" -ne 0 ]; then + if [ "$build_status" -eq 124 ] || [ "$build_status" -eq 137 ]; then + print_error "Go server build timed out after ${GO_BUILD_TIMEOUT}s" + else + print_error "Go server build failed (exit code ${build_status})" + fi + return 1 + fi if [ -f "$output_name" ]; then chmod +x "$output_name" @@ -8369,5 +8489,8 @@ main() { done } -# Run -main "$@" +# Run only when executed directly; this also lets installer tests source the +# bounded command helpers without starting the interactive manager. +if [[ "${BASH_SOURCE[0]}" == "$0" ]]; then + main "$@" +fi diff --git a/install.sh b/install.sh index 2eb24593..1109cf8e 100755 --- a/install.sh +++ b/install.sh @@ -550,10 +550,70 @@ uninstall_docker_mode() { # ── Native install ──────────────────────────────────────────────────────────── +native_backup_source() { + local source_dir="$1" + local backup_dir="${source_dir}.backup.$(date +%Y%m%d%H%M%S).$$" + + while [ -e "$backup_dir" ]; do + backup_dir="${source_dir}.backup.$(date +%Y%m%d%H%M%S).${RANDOM}" + done + if ! mv "$source_dir" "$backup_dir"; then + print_error "Could not preserve existing native source at ${source_dir}" + return 1 + fi + printf '%s\n' "$backup_dir" +} + +native_clone_source() { + local repo_dir="$1" + local backup_dir="${2:-}" + local staging_parent + local staging_repo + + mkdir -p "$INSTALL_DIR" + staging_parent=$(mktemp -d "${INSTALL_DIR}/.source-clone.XXXXXX") + staging_repo="${staging_parent}/source" + + if ! git clone --depth 1 --branch "$BETTERDESK_BRANCH" \ + "https://github.com/${BETTERDESK_REPO}.git" "$staging_repo"; then + rm -rf "$staging_parent" + if [ -n "$backup_dir" ] && [ ! -e "$repo_dir" ]; then + mv "$backup_dir" "$repo_dir" 2>/dev/null || true + fi + return 1 + fi + + # The native update path keeps local Go runtime data beside the source. + # Carry it forward without replacing anything created by the fresh clone. + if [ -n "$backup_dir" ] \ + && [ -d "$backup_dir/betterdesk-server/data" ] \ + && [ ! -e "$staging_repo/betterdesk-server/data" ]; then + if ! cp -a "$backup_dir/betterdesk-server/data" "$staging_repo/betterdesk-server/"; then + rm -rf "$staging_parent" + if [ -n "$backup_dir" ]; then + mv "$backup_dir" "$repo_dir" 2>/dev/null || true + fi + return 1 + fi + fi + + if ! mv "$staging_repo" "$repo_dir"; then + rm -rf "$staging_parent" + if [ -n "$backup_dir" ]; then + mv "$backup_dir" "$repo_dir" 2>/dev/null || true + fi + return 1 + fi + rm -rf "$staging_parent" + return 0 +} + install_native_mode() { local repo_dir="${INSTALL_DIR}/source" local relay local commit_sha + local source_backup="" + local checkout_error="" log "BetterDesk native installer v${VERSION}" log "Native install branch: ${BETTERDESK_BRANCH} (override with --branch)" @@ -565,18 +625,42 @@ install_native_mode() { log "Updating existing clone in ${repo_dir} to ${BETTERDESK_BRANCH}..." git -C "$repo_dir" remote set-url origin "https://github.com/${BETTERDESK_REPO}.git" || true git -C "$repo_dir" fetch --depth 1 origin "$BETTERDESK_BRANCH" - # Shallow clones previously pinned to another branch need a hard reset. - if ! git -C "$repo_dir" checkout -B "$BETTERDESK_BRANCH" "FETCH_HEAD"; then - warn "Could not switch existing clone to ${BETTERDESK_BRANCH}; recloning..." - rm -rf "$repo_dir" - git clone --depth 1 --branch "$BETTERDESK_BRANCH" \ - "https://github.com/${BETTERDESK_REPO}.git" "$repo_dir" + # Shallow clones previously pinned to another branch need a branch switch. + checkout_error=$(mktemp) + if git -C "$repo_dir" checkout -B "$BETTERDESK_BRANCH" "FETCH_HEAD" 2>"$checkout_error"; then + rm -f "$checkout_error" + else + warn "Could not switch existing clone to ${BETTERDESK_BRANCH}." + if [ -n "$(git -C "$repo_dir" status --short 2>/dev/null || true)" ]; then + warn "The existing clone contains local changes; preserving it before recloning." + cat "$checkout_error" >&2 || true + else + warn "Git checkout failed; preserving the existing clone before recloning." + cat "$checkout_error" >&2 || true + fi + rm -f "$checkout_error" + if ! source_backup=$(native_backup_source "$repo_dir"); then + die "Cannot preserve ${repo_dir}; refusing to remove the existing native source" + fi + if ! native_clone_source "$repo_dir" "$source_backup"; then + die "Could not clone ${BETTERDESK_REPO}@${BETTERDESK_BRANCH}; original source preserved at ${source_backup}" + fi + warn "Previous native source preserved at ${source_backup}" fi else log "Cloning ${BETTERDESK_REPO} (${BETTERDESK_BRANCH})..." - rm -rf "$repo_dir" - git clone --depth 1 --branch "$BETTERDESK_BRANCH" \ - "https://github.com/${BETTERDESK_REPO}.git" "$repo_dir" + if [ -e "$repo_dir" ]; then + if ! source_backup=$(native_backup_source "$repo_dir"); then + die "Cannot preserve ${repo_dir}; refusing to remove the existing native source" + fi + warn "Existing non-Git source preserved at ${source_backup}" + fi + if ! native_clone_source "$repo_dir" "$source_backup"; then + die "Could not clone ${BETTERDESK_REPO}@${BETTERDESK_BRANCH}; original source preserved at ${source_backup:-none}" + fi + if [ -n "$source_backup" ]; then + warn "Previous native source preserved at ${source_backup}" + fi fi commit_sha=$(git -C "$repo_dir" rev-parse --short HEAD 2>/dev/null || echo "unknown") @@ -699,4 +783,6 @@ main() { esac } -main "$@" +if [[ "${BASH_SOURCE[0]}" == "$0" ]]; then + main "$@" +fi diff --git a/scripts/test-native-installer.sh b/scripts/test-native-installer.sh new file mode 100644 index 00000000..d0270cf7 --- /dev/null +++ b/scripts/test-native-installer.sh @@ -0,0 +1,128 @@ +#!/usr/bin/env bash +set -euo pipefail + +SCRIPT_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" + +fail() { + printf 'FAIL: %s\n' "$*" >&2 + exit 1 +} + +test_bounded_go_commands() ( + export GO_COMMAND_HEARTBEAT_INTERVAL=1 + export GO_COMMAND_KILL_AFTER=1 + set -- + # shellcheck disable=SC1091 + source "$SCRIPT_ROOT/betterdesk.sh" + + if ! _run_go_command_bounded "successful test command" 5 sh -c 'exit 0'; then + fail "successful bounded command returned non-zero" + fi + + if _run_go_command_bounded "failed test command" 5 sh -c 'exit 7'; then + fail "failed bounded command returned success" + else + status=$? + fi + [ "$status" -eq 7 ] || fail "expected exit code 7, got ${status}" + + if _run_go_command_bounded "timed test command" 1 sh -c 'sleep 10'; then + fail "timed bounded command returned success" + else + status=$? + fi + [ "$status" -eq 124 ] || fail "expected timeout code 124, got ${status}" +) + +test_dirty_native_clone() ( + local temp_dir fake_bin install_dir repo_dir + temp_dir=$(mktemp -d) + trap 'rm -rf "$temp_dir"' EXIT + fake_bin="$temp_dir/bin" + install_dir="$temp_dir/install" + repo_dir="$install_dir/source" + mkdir -p "$fake_bin" "$repo_dir/.git" "$repo_dir/betterdesk-server/data" + printf 'local edit\n' > "$repo_dir/betterdesk.sh" + printf 'runtime state\n' > "$repo_dir/betterdesk-server/data/state.txt" + + cat > "$fake_bin/git" <<'FAKE_GIT' +#!/usr/bin/env bash +set -euo pipefail + +if [ "${1:-}" = "-C" ]; then + shift 2 + case "${1:-}" in + remote|fetch) + exit 0 + ;; + status) + printf ' M betterdesk.sh\n' + exit 0 + ;; + checkout) + printf 'error: local changes would be overwritten\n' >&2 + exit 1 + ;; + rev-parse) + printf 'abc123\n' + exit 0 + ;; + esac +fi + +if [ "${1:-}" = "clone" ]; then + if [ "${FAKE_GIT_FAIL_CLONE:-0}" = "1" ]; then + printf 'simulated clone failure\n' >&2 + exit 1 + fi + destination="${!#}" + mkdir -p "$destination/.git" "$destination/betterdesk-server" + printf '#!/usr/bin/env bash\nexit 0\n' > "$destination/betterdesk.sh" + exit 0 +fi + +printf 'unexpected fake git invocation: %s\n' "$*" >&2 +exit 1 +FAKE_GIT + chmod +x "$fake_bin/git" + + export PATH="$fake_bin:$PATH" + export INSTALL_DIR="$install_dir" + export RELAY_SERVERS="127.0.0.1:21117" + export BETTERDESK_REPO="example/BetterDesk" + export BETTERDESK_BRANCH="dev" + set -- + # shellcheck disable=SC1091 + source "$SCRIPT_ROOT/install.sh" + + install_native_mode + + [ -d "$repo_dir/.git" ] || fail "fresh clone was not installed" + [ -f "$repo_dir/betterdesk-server/data/state.txt" ] \ + || fail "runtime Go data was not preserved" + compgen -G "$install_dir/source.backup.*" > /dev/null \ + || fail "dirty source backup was not preserved" + backup_dir="" + for path in "$install_dir"/source.backup.*; do + if [ -e "$path" ]; then + backup_dir="$path" + break + fi + done + [ -f "$backup_dir/betterdesk.sh" ] || fail "original dirty source was not backed up" + + if ( + export FAKE_GIT_FAIL_CLONE=1 + set -- + # shellcheck disable=SC1091 + source "$SCRIPT_ROOT/install.sh" + install_native_mode + ); then + fail "failed clone returned success" + fi + [ -d "$repo_dir/.git" ] || fail "original source was not restored after clone failure" +) + +test_bounded_go_commands +test_dirty_native_clone +printf 'PASS: native installer regression tests\n'