From b7fb5fa4bf2ed577d4122781b22ba8ea13eef2e3 Mon Sep 17 00:00:00 2001 From: Asad Iqbal Date: Tue, 14 Jul 2026 21:04:53 +0500 Subject: [PATCH 1/3] fix(installer): bound the services progress-bar kubectl calls with --request-timeout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bugbot (Medium): _download_services_progress claims to be bounded/non-blocking, but its two `kubectl get pods` calls omitted --request-timeout. The TB_PULL_TIMEOUT deadline is only checked BETWEEN iterations, so a wedged/ unreachable API makes kubectl block indefinitely — freezing step e's progress bar before the authoritative readiness gate in step f ever runs. Add --request-timeout (default 5s, overridable via TB_PROGRESS_KUBECTL_TIMEOUT) to both calls, mirroring assess.sh's bounded probe — a call that can't reach the API now returns quickly, the loop re-checks the deadline, and step e degrades to the honest "still downloading in the background" line instead of hanging. Manifest regenerated. Co-Authored-By: Claude Opus 4.8 --- scripts/lib/install-client-helm.sh | 10 ++++++++-- scripts/manifest.sha256 | 2 +- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/scripts/lib/install-client-helm.sh b/scripts/lib/install-client-helm.sh index 95ebc80c..28951af4 100644 --- a/scripts/lib/install-client-helm.sh +++ b/scripts/lib/install-client-helm.sh @@ -351,10 +351,16 @@ _download_services_progress() { has kubectl || return 0 [[ -n "$ns" ]] || return 0 + # Every kubectl call is bounded with --request-timeout so a wedged/unreachable + # API can never make the poll BLOCK — the between-iteration deadline check below + # only fires if kubectl actually returns, so an unbounded call would hang step e + # forever despite TB_PULL_TIMEOUT. Overridable; mirrors assess.sh's bounded probe. + local kube_timeout="${TB_PROGRESS_KUBECTL_TIMEOUT:-5s}" + # Establish the total container count once the pods are scheduled (bounded). local total=0 tries=0 while (( tries < 15 )); do - total="$(kubectl get pods -n "$ns" \ + total="$(kubectl get pods -n "$ns" --request-timeout="$kube_timeout" \ -o jsonpath='{range .items[*].spec.containers[*]}{"x"}{end}' 2>/dev/null \ | tr -cd 'x' | wc -c | tr -d ' ')" || total=0 [[ "$total" =~ ^[0-9]+$ ]] || total=0 @@ -367,7 +373,7 @@ _download_services_progress() { deadline=$(( $(date +%s) + ${TB_PULL_TIMEOUT:-300} )) tput civis 2>/dev/null || true while :; do - pulled="$(kubectl get pods -n "$ns" \ + pulled="$(kubectl get pods -n "$ns" --request-timeout="$kube_timeout" \ -o jsonpath='{range .items[*].status.containerStatuses[*]}{.imageID}{"\n"}{end}' 2>/dev/null \ | grep -c '.')" || pulled=0 [[ "$pulled" =~ ^[0-9]+$ ]] || pulled=0 diff --git a/scripts/manifest.sha256 b/scripts/manifest.sha256 index 15368d82..a9d14fcc 100644 --- a/scripts/manifest.sha256 +++ b/scripts/manifest.sha256 @@ -8,7 +8,7 @@ fd8d37d698c242ab081095625aa430bb91971db646e35393a02bbadd41325898 scripts/lib/gp 238f9bc0e344376c3883bad9e980db250799cacc9e7ce3bfc7ef37c5b5881244 scripts/lib/setup-linux.sh ba01f93f3e15b67eb438314551cee87c9172611c9c630ab8b4b207324efc897d scripts/lib/cluster.sh 5de1afac8b1062a7f8d0691b46ad39ccdc2287e0e422b53c48628ecfd47d852e scripts/lib/gpu-plugins.sh -edf31e901bb77e8ec8da221817ebfb9a7369685661104b38640b20f08414ac11 scripts/lib/install-client-helm.sh +46eda5179b331a6246094fac42a12717d6edbabdfdbd0f4e7a617fd9569a0368 scripts/lib/install-client-helm.sh f365c563bbbc85cb2d6864c372dc6998544afa520ef208aaeba2e065b18b74ee scripts/lib/install-cli.sh ff39119445da8373352e1caf1037529242de3de103af54770bd891abbd7638db scripts/lib/provision.sh e2ea63d844e6649f1d3aaae9fd4733845a1a39df37d68abbaeda00330f9e1c7e scripts/lib/assess.sh From f31ef26ac4d01524f8b507d92afc9a513cda141c Mon Sep 17 00:00:00 2001 From: Asad Iqbal Date: Tue, 14 Jul 2026 21:40:29 +0500 Subject: [PATCH 2/3] =?UTF-8?q?fix(installer):=20proactive=20hardening=20s?= =?UTF-8?q?weep=20=E2=80=94=20bounded=20probes,=20tty=20handoffs,=20fail-c?= =?UTF-8?q?losed=20guards?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A comprehensive audit of the installer (bash + PowerShell) for the failure classes Bugbot has been surfacing one at a time on #345, fixing the genuine instances in one pass. Bounded probes (no more indefinite hangs against a wedged/unreachable API): - cluster.sh: `kubectl cluster-info` in the _wait_for_api gate (the 60s cap was only re-checked between iterations — a single call could block forever). - summary.sh: cluster-info / get nodes / get pods / logs on the summary + not-ready diagnostic paths. - diagnose.sh: every kubectl call in the support-bundle path (`set +e` stops aborts but NOT hangs); helm calls now run only behind a bounded cluster-info probe (helm has no --request-timeout). - gpu-plugins.sh: `kubectl get nodes` in the verify_gpu poll loop. - common.sh download_with_progress: HEAD probe (-m) + the backgrounded curl (--connect-timeout + --speed-limit/--speed-time stall abort — it was monitored only by `kill -0`, no deadline, no kill). - install.sh: every bootstrap fetch (--connect-timeout/--max-time; retry already present, so a stall becomes retriable). - install-cli.sh: the CLI-installer download (a stall now falls to "install later" instead of hanging the step). curl|bash tty handoffs (stdin is the install pipe; stdout/stderr are the tee): - provision.sh: `tracebloc login` (the credential-mint device flow) now gets the real terminal on all three streams when openable, else no reboot. - setup-macos.sh: the Docker-arch replace prompt read the pipe (empty answer => meaningless confirm); now reads /dev/tty. Fail-closed guards (a failed check must not read as "safe to proceed"): - install-client-helm.sh detect_installed_client now reports INSTALLED_CLIENT_UNKNOWN=1 when `helm list` FAILS (vs genuinely no releases), and the one-client guard refuses rather than risk overwriting a client it couldn't enumerate. +bats coverage. - install-k8s.ps1 one-client guard: same fix at the `helm list` level (non-zero exit OR non-JSON now fails closed, matching the per-release fix from #350). +Pester coverage. set -e footgun: - common.sh _chart_version: trailing `|| true` so a no-match `grep` (no client release) can't abort callers that assign it under `set -e`. PowerShell 5.1 portability: - install-k8s.ps1: the WSL-path build used a scriptblock `-replace` (PS 6.1+); under Windows PowerShell 5.1 (the bootstrap target) the drive letter wasn't lowercased -> malformed path -> 180s NCT-install timeout. Now -match/$Matches. Deliberately NOT changed (documented): - k3d `cluster create --wait` has no --timeout in EITHER bash or PowerShell — at parity, and the known --wait hang cause (proxy misroute) is already mitigated by the proxy config. Not changing critical-path create behavior speculatively. - The dataset-mount check's inspect-failure no-op is a documented, tested design choice (cluster.bats) — left as-is. - Long tail of Linux-GPU-only driver-download curls (gpu-nvidia/gpu-amd, setup-linux) and misc PowerShell -TimeoutSec: LOW severity, retry-wrapped; left for a follow-up to keep this reviewable. All bash suites green (assess 33, install-client-helm 54, cluster 27, provision, summary, install-cli, bootstrap, preflight); shellcheck --severity=error clean; install-k8s.ps1 parses cleanly; manifest regenerated. Two pre-existing local-env bats failures (_extract_yaml_value '' escape; validate_config) are unrelated and also fail on develop. Co-Authored-By: Claude Opus 4.8 --- scripts/install-k8s.ps1 | 45 ++++++++++++++++++++----- scripts/install.sh | 18 +++++----- scripts/lib/cluster.sh | 6 +++- scripts/lib/common.sh | 18 ++++++++-- scripts/lib/diagnose.sh | 32 ++++++++++++------ scripts/lib/gpu-nvidia.sh | 8 ++++- scripts/lib/gpu-plugins.sh | 4 ++- scripts/lib/install-cli.sh | 5 ++- scripts/lib/install-client-helm.sh | 46 ++++++++++++++++++++------ scripts/lib/provision.sh | 13 +++++++- scripts/lib/setup-macos.sh | 8 +++-- scripts/lib/summary.sh | 12 ++++--- scripts/manifest.sha256 | 22 ++++++------ scripts/tests/install-client-helm.bats | 20 +++++++++++ scripts/tests/install-k8s.Tests.ps1 | 33 ++++++++++++++++++ 15 files changed, 225 insertions(+), 65 deletions(-) diff --git a/scripts/install-k8s.ps1 b/scripts/install-k8s.ps1 index de6620cb..989f58a6 100644 --- a/scripts/install-k8s.ps1 +++ b/scripts/install-k8s.ps1 @@ -563,7 +563,18 @@ echo "NCT installed successfully." $scriptPath = [System.IO.Path]::Combine($env:TEMP, "install-nct-$(Get-Random -Maximum 999999).sh") [System.IO.File]::WriteAllText($scriptPath, $nctScript.Replace("`r`n", "`n")) - $wslPath = "/mnt/" + ($scriptPath -replace '\\','/' -replace '^([A-Za-z]):/', { $_.Groups[1].Value.ToLower() + '/' }) + # Build the WSL path WITHOUT a scriptblock -replace: scriptblock substitution in + # the -replace operator is PowerShell 6.1+, but the bootstrap (install.ps1) runs + # this via powershell.exe (Windows PowerShell 5.1, per #Requires -Version 5.1), + # where the scriptblock is coerced to its literal text and the drive letter is + # NOT lowercased -> a malformed $wslPath and a 180s NCT-install timeout. -match + # / $Matches is 5.1-safe. + $fwd = $scriptPath -replace '\\','/' + if ($fwd -match '^([A-Za-z]):/(.*)$') { + $wslPath = "/mnt/" + $Matches[1].ToLower() + '/' + $Matches[2] + } else { + $wslPath = "/mnt/" + $fwd + } $nctInstallJob = Start-Job -ScriptBlock { param($d, $p) @@ -1176,8 +1187,16 @@ function Install-ClientHelm { # fails OPEN and lets a re-install silently overwrite an existing client we # simply couldn't identify. Record it and fail CLOSED below (#200 follow-up). $unreadableNs = "" + # $listUnknown: `helm list` itself failed or returned non-JSON, so we couldn't + # even ENUMERATE releases. Same fail-open risk one level up from $unreadableNs — + # skipping the guard here would let a re-install overwrite a different client. + $listUnknown = $false $listJson = (helm list -A -o json 2>$null) | Out-String - if ($LASTEXITCODE -eq 0 -and $listJson.Trim()) { + if ($LASTEXITCODE -ne 0) { + # helm list failed (wedged/unreachable API, kubeconfig glitch) -> unknown. + # (helm returns 0 with an empty `[]` when there are genuinely no releases.) + $listUnknown = $true + } elseif ($listJson.Trim()) { try { foreach ($rel in ($listJson | ConvertFrom-Json)) { if ($rel.chart -and $rel.chart.StartsWith("client-")) { @@ -1202,17 +1221,25 @@ function Install-ClientHelm { if ($id) { $existingId = $id; $existingNs = $rel.namespace; break } } } - } catch { } + } catch { + # helm list returned non-JSON/garbage -> can't trust the enumeration. + $listUnknown = $true + } } - # Fail closed: a client release exists here but we couldn't read its clientId, - # and no OTHER release gave us a definitive id. Refuse rather than overwrite an + # Fail closed when we couldn't identify a client we can see ($unreadableNs) OR + # couldn't enumerate at all ($listUnknown). Refuse rather than overwrite an # unknown client -- the operator must resolve it explicitly. - if (-not $existingId -and $unreadableNs) { + if (-not $existingId -and ($unreadableNs -or $listUnknown)) { Write-Host "" - Warn "A tracebloc client release is installed here (namespace '$unreadableNs') but its configuration could not be read." + if ($listUnknown) { + Warn "Couldn't determine which tracebloc client (if any) is already installed here -- helm could not enumerate releases." + } else { + Warn "A tracebloc client release is installed here (namespace '$unreadableNs') but its configuration could not be read." + } Hint "tracebloc runs one client per machine, so the installer will not overwrite" - Hint "a client it cannot identify. Inspect or remove it, then re-run:" - Hint " helm get values -A (see what is installed)" + Hint "a client it cannot see (usually the cluster API is briefly unreachable). Check and re-run:" + Hint " kubectl cluster-info (is the API reachable?)" + Hint " helm get values -A (see what is installed)" Hint " k3d cluster delete $CLUSTER_NAME (wipes this client + its local data)" Write-Host "" Err "Refusing to replace an unidentifiable existing client." diff --git a/scripts/install.sh b/scripts/install.sh index e3ae64cf..a462cecc 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -277,8 +277,10 @@ download_with_retry() { for attempt in 1 2 3; do # --tlsv1.2 floor; honor any proxy / custom-CA env the corporate-proxy # segment relies on (#172/#722) — curl picks up HTTPS_PROXY/NO_PROXY/ - # CURL_CA_BUNDLE from the environment automatically. - if curl -fsSL --tlsv1.2 "$url" -o "$dest"; then return 0; fi + # CURL_CA_BUNDLE from the environment automatically. --connect-timeout/--max-time + # (added to every fetch in this file) turn a stalled endpoint into a retriable + # failure instead of hanging the "1. Downloading" phase forever. + if curl -fsSL --tlsv1.2 --connect-timeout 30 --max-time 300 "$url" -o "$dest"; then return 0; fi if [[ $attempt -ge $max_attempts ]]; then echo "[ERROR] Failed to download $url after $max_attempts attempts." exit 1 @@ -372,11 +374,11 @@ download_manifest() { # Authoritative source: the signed release asset. Fall back to the in-repo # copy in the tag tree only under the unverified dev opt-in (a branch checkout # has no release assets). - if curl -fsSL --tlsv1.2 "$REPO_REL/manifest.sha256" -o "$dest" 2>/dev/null; then + if curl -fsSL --tlsv1.2 --connect-timeout 30 --max-time 300 "$REPO_REL/manifest.sha256" -o "$dest" 2>/dev/null; then return 0 fi if [[ "$ALLOW_UNVERIFIED" == "1" ]]; then - curl -fsSL --tlsv1.2 "$REPO_RAW/scripts/manifest.sha256" -o "$dest" 2>/dev/null + curl -fsSL --tlsv1.2 --connect-timeout 30 --max-time 300 "$REPO_RAW/scripts/manifest.sha256" -o "$dest" 2>/dev/null return $? fi return 1 @@ -406,8 +408,8 @@ verify_manifest_signature() { exit 1 fi - if ! curl -fsSL --tlsv1.2 "$REPO_REL/manifest.sha256.sig" -o "$sig" 2>/dev/null \ - || ! curl -fsSL --tlsv1.2 "$REPO_REL/manifest.sha256.cert" -o "$cert" 2>/dev/null; then + if ! curl -fsSL --tlsv1.2 --connect-timeout 30 --max-time 300 "$REPO_REL/manifest.sha256.sig" -o "$sig" 2>/dev/null \ + || ! curl -fsSL --tlsv1.2 --connect-timeout 30 --max-time 300 "$REPO_REL/manifest.sha256.cert" -o "$cert" 2>/dev/null; then if [[ "$ALLOW_UNVERIFIED" == "1" ]]; then echo "[WARN] manifest signature/cert not published for ref '$REF' — not verified (TRACEBLOC_ALLOW_UNVERIFIED=1)." >&2 return 0 @@ -462,8 +464,8 @@ ensure_cosign() { local sums="$TMPDIR/cosign_checksums.txt" echo " · Fetching the signature-verification tool (cosign)…" - curl -fsSL --tlsv1.2 "$base/$asset" -o "$bin" 2>/dev/null || return 1 - curl -fsSL --tlsv1.2 "$base/cosign_checksums.txt" -o "$sums" 2>/dev/null || return 1 + curl -fsSL --tlsv1.2 --connect-timeout 30 --max-time 300 "$base/$asset" -o "$bin" 2>/dev/null || return 1 + curl -fsSL --tlsv1.2 --connect-timeout 30 --max-time 300 "$base/cosign_checksums.txt" -o "$sums" 2>/dev/null || return 1 local want got want="$(grep " ${asset}\$" "$sums" | awk '{print $1}' | head -1)" diff --git a/scripts/lib/cluster.sh b/scripts/lib/cluster.sh index b9108deb..e675c298 100755 --- a/scripts/lib/cluster.sh +++ b/scripts/lib/cluster.sh @@ -408,7 +408,11 @@ _wait_for_api() { local frames=('⠋' '⠙' '⠹' '⠸' '⠼' '⠴' '⠦' '⠧' '⠇' '⠏') local f=0 for attempt in $(seq 1 $max); do - if kubectl cluster-info &>/dev/null 2>&1; then + # --request-timeout bounds the call itself: the 60s cap here is only re-checked + # BETWEEN iterations, so an unbounded cluster-info against an API that accepts + # the TCP connection but never responds (corporate-proxy intercept of + # localhost, half-booted apiserver) would hang this gate forever. + if kubectl cluster-info --request-timeout=5s &>/dev/null 2>&1; then printf "\r\033[K" tput cnorm 2>/dev/null || true success "Secure environment ready" diff --git a/scripts/lib/common.sh b/scripts/lib/common.sh index d5e2bab3..19950ea5 100755 --- a/scripts/lib/common.sh +++ b/scripts/lib/common.sh @@ -48,7 +48,11 @@ has() { command -v "$1" &>/dev/null; } _chart_version() { local ns="${1:-${TB_NAMESPACE:-tracebloc}}" has helm || return 0 - helm list -n "$ns" 2>/dev/null | grep -oE 'client-[0-9][^[:space:]]*' | head -1 | sed 's/^client-//' + # Trailing `|| true`: when no client-* release exists, `grep` exits 1 and, under + # `set -o pipefail`, the pipeline (this function's last command) returns 1 — + # which would abort callers that assign it under `set -e` (e.g. diagnose.sh). + # The version (or empty) has already been emitted to stdout regardless. + helm list -n "$ns" 2>/dev/null | grep -oE 'client-[0-9][^[:space:]]*' | head -1 | sed 's/^client-//' || true } # The client's core workload Deployments in namespace $1 — the set whose @@ -176,7 +180,9 @@ download_with_progress() { local url="$1" dest="$2" label="$3" local total_bytes - total_bytes=$(curl -fsSLI "$url" 2>/dev/null \ + # -m bounds the HEAD probe so a stalled server can't hang it (it's not + # retry-wrapped and its failure just means "no total" -> indeterminate bar). + total_bytes=$(curl -fsSLI -m 15 "$url" 2>/dev/null \ | awk 'tolower($0) ~ /content-length/ {gsub(/[^0-9]/,"",$2); print $2}' \ | tail -1) @@ -192,7 +198,13 @@ download_with_progress() { local logfile="${LOG_FILE:-/tmp/tracebloc-spin.log}" rm -f "$dest" - curl -fSL -o "$dest" "$url" >> "$logfile" 2>&1 & + # --connect-timeout bounds the dial; --speed-limit/--speed-time abort a STALLED + # transfer (<1 KB/s for 60s) without capping a legitimately slow-but-progressing + # large download. Without these the backgrounded curl is monitored only by + # `kill -0` (no deadline, no kill), so a slow-loris / mid-stream stall would + # hang the progress loop forever. + curl -fSL --connect-timeout 30 --speed-limit 1024 --speed-time 60 \ + -o "$dest" "$url" >> "$logfile" 2>&1 & local curl_pid=$! local bar_width=30 diff --git a/scripts/lib/diagnose.sh b/scripts/lib/diagnose.sh index f64acee5..eaf0d84a 100644 --- a/scripts/lib/diagnose.sh +++ b/scripts/lib/diagnose.sh @@ -51,9 +51,14 @@ run_diagnose() { # Namespace discovery — TB_NAMESPACE isn't set on a standalone diagnose run, # so find the namespace of the jobs-manager pod (falls back to "default"). + # Every kubectl call below carries --request-timeout: run_diagnose runs `set +e` + # so a non-zero exit is harmless, but that does NOT bound an indefinite BLOCK — + # and --diagnose is exactly the "API may be wedged" path, where an unbounded + # call would freeze the bundle this function exists to produce. + local kt="--request-timeout=5s" ns="${TB_NAMESPACE:-}" if [[ -z "$ns" ]] && has kubectl; then - ns="$(kubectl get pods -A 2>/dev/null | awk '/-jobs-manager/{print $1; exit}')" + ns="$(kubectl get pods -A $kt 2>/dev/null | awk '/-jobs-manager/{print $1; exit}')" fi [[ -z "$ns" ]] && ns="default" @@ -106,28 +111,31 @@ run_diagnose() { # ── kubectl overview + per-pod detail ── if has kubectl; then { - echo "## nodes"; kubectl get nodes -o wide 2>&1 - echo; echo "## pods (all namespaces)"; kubectl get pods -A -o wide 2>&1 - echo; echo "## workloads"; kubectl get deploy,ds,sts -A 2>&1 - echo; echo "## recent events"; kubectl get events -A --sort-by=.lastTimestamp 2>&1 | tail -120 + echo "## nodes"; kubectl get nodes -o wide $kt 2>&1 + echo; echo "## pods (all namespaces)"; kubectl get pods -A -o wide $kt 2>&1 + echo; echo "## workloads"; kubectl get deploy,ds,sts -A $kt 2>&1 + echo; echo "## recent events"; kubectl get events -A --sort-by=.lastTimestamp $kt 2>&1 | tail -120 } > "$d/02-kubectl.txt" 2>&1 { echo "## describe of non-Running pods in namespace '$ns'" - for p in $(kubectl get pods -n "$ns" --no-headers 2>/dev/null | awk '$3!="Running" && $3!="Completed"{print $1}'); do - echo; echo "### $p"; kubectl describe pod -n "$ns" "$p" 2>&1 + for p in $(kubectl get pods -n "$ns" --no-headers $kt 2>/dev/null | awk '$3!="Running" && $3!="Completed"{print $1}'); do + echo; echo "### $p"; kubectl describe pod -n "$ns" "$p" $kt 2>&1 done } > "$d/03-describe.txt" 2>&1 # workload logs (current + previous) local w for w in mysql-client "${ns}-jobs-manager" "${ns}-requests-proxy"; do - kubectl logs -n "$ns" "deploy/$w" --all-containers --tail=500 > "$d/logs/${w}.log" 2>&1 - kubectl logs -n "$ns" "deploy/$w" --all-containers --previous --tail=500 > "$d/logs/${w}.previous.log" 2>&1 + kubectl logs -n "$ns" "deploy/$w" --all-containers --tail=500 $kt > "$d/logs/${w}.log" 2>&1 + kubectl logs -n "$ns" "deploy/$w" --all-containers --previous --tail=500 $kt > "$d/logs/${w}.previous.log" 2>&1 done - kubectl logs -n "$ns" "daemonset/tracebloc-resource-monitor" --tail=300 > "$d/logs/resource-monitor.log" 2>&1 + kubectl logs -n "$ns" "daemonset/tracebloc-resource-monitor" --tail=300 $kt > "$d/logs/resource-monitor.log" 2>&1 fi # ── helm (redacted afterwards) ── - if has helm; then + # helm has no --request-timeout; it talks to the same API. Only run it when a + # BOUNDED probe confirms the API is reachable, so a wedged API can't hang the + # bundle here (the kubectl output above already captured the degraded state). + if has helm && { ! has kubectl || kubectl cluster-info --request-timeout=5s >/dev/null 2>&1; }; then # NOTE: deliberately NOT collecting `helm get manifest` — it renders the # Secret objects with base64-encoded credentials (CLIENT_PASSWORD, # .dockerconfigjson), which the text redaction can't see. `helm get values` @@ -136,6 +144,8 @@ run_diagnose() { echo "## helm list -A"; helm list -A 2>&1 echo; echo "## helm get values $ns"; helm get values "$ns" -n "$ns" 2>&1 } > "$d/04-helm.txt" 2>&1 + elif has helm; then + echo "## helm skipped — API unreachable (bounded cluster-info probe failed)" > "$d/04-helm.txt" 2>&1 fi # ── install artifacts (copied, redacted afterwards) ── diff --git a/scripts/lib/gpu-nvidia.sh b/scripts/lib/gpu-nvidia.sh index 029a9a45..2f521925 100755 --- a/scripts/lib/gpu-nvidia.sh +++ b/scripts/lib/gpu-nvidia.sh @@ -54,7 +54,13 @@ install_nvidia_drivers() { log "TRACEBLOC_SKIP_REBOOT_PROMPT set — skipping reboot prompt." exit 2 fi - read -r -p " Reboot now? [y/N]: " _choice + # Read the terminal directly: the main install path is `curl … | bash`, where + # this shell's stdin is the (EOF) install pipe — a bare `read` there returns + # non-zero and, with `set -e` active in this code path, would ABORT the whole + # installer right after a successful driver install. No tty (unattended) => treat + # as "no reboot" (same as TRACEBLOC_SKIP_REBOOT_PROMPT). + local _choice="" + if [[ -r /dev/tty ]]; then read -r -p " Reboot now? [y/N]: " _choice /dev/null \ + # --request-timeout bounds the call: the 18×5s cap is only re-checked between + # iterations, so an unbounded get-nodes against a wedged API would hang here. + RAW=$(kubectl get nodes -o json --request-timeout=5s 2>/dev/null \ | grep -o '"[^"]*gpu[^"]*"\s*:\s*"[^"]*"' \ | sed 's/"//g; s/\s*:\s*/=/g' | head -5 \ 2>/dev/null || echo "") diff --git a/scripts/lib/install-cli.sh b/scripts/lib/install-cli.sh index c6c17a02..33f13404 100644 --- a/scripts/lib/install-cli.sh +++ b/scripts/lib/install-cli.sh @@ -163,7 +163,10 @@ install_tracebloc_cli() { # 1) Download the released installer. A failure here is a download problem, # distinct from an install problem below. - if ! curl -fsSL "$CURL_SECURE" "$TRACEBLOC_CLI_INSTALL_URL" -o "$installer" 2>>"${LOG_FILE:-/dev/null}"; then + # --connect-timeout/--max-time so a stalled CDN turns into a clean "install later" + # failure below instead of hanging the CLI-install step (this call isn't retry- + # wrapped, and a hang is not a failure the graceful fallback would otherwise catch). + if ! curl -fsSL "$CURL_SECURE" --connect-timeout 30 --max-time 120 "$TRACEBLOC_CLI_INSTALL_URL" -o "$installer" 2>>"${LOG_FILE:-/dev/null}"; then warn "Couldn't download the tracebloc CLI installer — your client is set up fine." hint "Install it later: curl -fsSL ${TRACEBLOC_CLI_INSTALL_URL} | sh" rm -f "$installer" diff --git a/scripts/lib/install-client-helm.sh b/scripts/lib/install-client-helm.sh index 28951af4..dd6591ec 100644 --- a/scripts/lib/install-client-helm.sh +++ b/scripts/lib/install-client-helm.sh @@ -57,25 +57,36 @@ _extract_yaml_value() { # the first two whitespace-free columns and the CHART column matches # `client-`, the same jq-free parse _chart_version uses. Shared by the # pre-provision ownership pre-flight (#303) and the Helm-step one-client guard so -# the two can never disagree on "what already runs here". Always returns 0 — a -# missing helm / unreadable values just yields the empty (no-client) result. +# the two can never disagree on "what already runs here". Always returns 0. A +# missing helm just yields the empty (no-client) result — but a helm/API FAILURE +# is reported as INSTALLED_CLIENT_UNKNOWN=1 (not "no client"), so guards can fail +# CLOSED instead of silently overwriting a client they couldn't see. detect_installed_client() { - INSTALLED_CLIENT_ID=""; INSTALLED_CLIENT_NS="" - local _gvf _rel _ns _id - # A mktemp failure is an environment error, NOT proof of "no client here" — - # treating it as such would silently skip the one-client / #303 ownership - # guards. Fall back to a path in a dir we own (same reasoning as the create - # step: never a predictable world-writable /tmp path under sudo). Only give up - # (empty result) when there is genuinely nowhere to write. + INSTALLED_CLIENT_ID=""; INSTALLED_CLIENT_NS=""; INSTALLED_CLIENT_UNKNOWN=0 + # No helm => nothing helm-installed here; a genuine (documented) "no client". + has helm || return 0 + local _gvf _rel _ns _id _list + # A mktemp failure is an environment error, NOT proof of "no client here" — flag + # UNKNOWN so the guards fail closed rather than skip. Fall back to a path in a + # dir we own (never a predictable world-writable /tmp path under sudo) before + # giving up. _gvf="$(mktemp 2>/dev/null)" || _gvf="${HOST_DATA_DIR:+${HOST_DATA_DIR}/.tb-detect-values.$$}" - [[ -n "$_gvf" ]] || return 0 + [[ -n "$_gvf" ]] || { INSTALLED_CLIENT_UNKNOWN=1; return 0; } + # Capture `helm list`'s exit code: a FAILED enumeration (wedged/unreachable API, + # kubeconfig glitch) must NOT read as "no client here" — that fails OPEN and lets + # a re-install silently overwrite an existing client. `helm list` returns 0 with + # empty output when there are genuinely no releases, so only a non-zero exit is + # "unknown". + if ! _list="$(helm list -A 2>/dev/null)"; then + INSTALLED_CLIENT_UNKNOWN=1; rm -f "$_gvf"; return 0 + fi while read -r _rel _ns; do [[ -z "$_rel" ]] && continue if helm get values "$_rel" -n "$_ns" > "$_gvf" 2>/dev/null; then _id="$(_extract_yaml_value "$_gvf" clientId)" [[ -n "$_id" ]] && { INSTALLED_CLIENT_ID="$_id"; INSTALLED_CLIENT_NS="$_ns"; break; } fi - done < <(helm list -A 2>/dev/null | awk '/[[:space:]]client-[0-9]/ { print $1, $2 }') + done < <(printf '%s\n' "$_list" | awk '/[[:space:]]client-[0-9]/ { print $1, $2 }') rm -f "$_gvf" return 0 } @@ -559,6 +570,19 @@ install_client_helm() { local existing_id="" existing_ns="" detect_installed_client existing_id="$INSTALLED_CLIENT_ID"; existing_ns="$INSTALLED_CLIENT_NS" + # Fail CLOSED when we couldn't enumerate what's here (API/helm failure): refuse + # rather than risk overwriting a client the guard simply couldn't see. + if [[ "${INSTALLED_CLIENT_UNKNOWN:-0}" == 1 ]]; then + echo "" + warn "Couldn't determine which tracebloc client (if any) is already installed here." + hint "tracebloc runs one client per machine, so the installer won't risk overwriting" + hint "an existing client it can't see — usually the cluster API is briefly unreachable." + hint "Check it and re-run:" + hint " kubectl cluster-info" + hint " helm list -A" + echo "" + error "Refusing to install without verifying what's already on this machine." + fi if [[ -n "$existing_id" && "$existing_id" != "$TB_CLIENT_ID" ]]; then echo "" warn "This machine already runs the tracebloc client '${existing_id}' (namespace '${existing_ns}')." diff --git a/scripts/lib/provision.sh b/scripts/lib/provision.sh index f5911368..e724e711 100644 --- a/scripts/lib/provision.sh +++ b/scripts/lib/provision.sh @@ -149,7 +149,18 @@ provision_client() { echo -e " Sign in to approve this machine — open the link in your browser" echo -e " (on this or any device) and enter the code:" echo "" - tracebloc login || error "Sign-in didn't complete — re-run the installer to try again." + # Give the interactive device-flow sign-in the user's REAL terminal. This runs + # after setup_log_file (`exec > >(tee …) 2>&1`), so the shell's stdout/stderr are + # a pipe and — under `curl … | bash` — stdin is the install pipe; a bare + # `tracebloc login` would then have no tty on any stream and can misrender or + # fail (same class assess.sh's hand-off handles). Redirect all three to /dev/tty + # when openable, else /dev/null; then + tracebloc login /dev/tty 2>/dev/tty || error "Sign-in didn't complete — re-run the installer to try again." + else + tracebloc login empty => proceed, + # preserving the opt-in prompt's prior non-interactive behavior. + local reply="" + if [[ -r /dev/tty ]]; then read -r -p " Replace wrong-architecture Docker with native version? [Y/n] " reply > "${LOG_FILE:-/dev/null}" 2>&1 || true - kubectl get nodes -o wide >> "${LOG_FILE:-/dev/null}" 2>&1 || true - kubectl get pods -n "${TB_NAMESPACE:-default}" -o wide >> "${LOG_FILE:-/dev/null}" 2>&1 || true + # --request-timeout so a wedged API can't hang the final summary/diagnostics + # (|| true only swallows the exit code, not an indefinite block). + kubectl cluster-info --request-timeout=5s >> "${LOG_FILE:-/dev/null}" 2>&1 || true + kubectl get nodes -o wide --request-timeout=5s >> "${LOG_FILE:-/dev/null}" 2>&1 || true + kubectl get pods -n "${TB_NAMESPACE:-default}" -o wide --request-timeout=5s >> "${LOG_FILE:-/dev/null}" 2>&1 || true log "--- End Cluster Status ---" } @@ -60,11 +62,11 @@ _diagnose_not_ready() { local ns="$1" pods jm_logs # Wrong credentials: jobs-manager authenticates to the backend on startup and # crash-loops when rejected — surfaced as an auth error in its logs. - jm_logs="$(kubectl logs -n "$ns" "deployment/${ns}-jobs-manager" --all-containers --tail=50 2>/dev/null || true)" + jm_logs="$(kubectl logs -n "$ns" "deployment/${ns}-jobs-manager" --all-containers --tail=50 --request-timeout=5s 2>/dev/null || true)" if printf '%s' "$jm_logs" | grep -qiE 'authentication failed|unable to log in'; then printf 'bad_creds'; return fi - pods="$(kubectl get pods -n "$ns" 2>/dev/null || true)" + pods="$(kubectl get pods -n "$ns" --request-timeout=5s 2>/dev/null || true)" if printf '%s' "$pods" | grep -qiE 'ImagePullBackOff|ErrImagePull|InvalidImageName'; then printf 'image_pull'; return fi diff --git a/scripts/manifest.sha256 b/scripts/manifest.sha256 index a9d14fcc..54440446 100644 --- a/scripts/manifest.sha256 +++ b/scripts/manifest.sha256 @@ -1,17 +1,17 @@ 22402403189204bf02fba2de1882ab93acf31351033bb23ac645599249dbbdb2 scripts/install-k8s.sh -269a1f124ac77004f55db43664873617c1a4e3c4bb29389725addf66aafa9cb0 scripts/lib/common.sh +8bfb9f5888fcf206c7b10e68c17174bfb7c4076c22c1daaf2be323d29bbc6b19 scripts/lib/common.sh f96dba6897312851c47be33cc97ece65036b263b885cb54472979e122f2015c8 scripts/lib/preflight.sh 19be2771df0e1a41b4fa9678e1cf6a77492304f66f73cf705a2ae42b1dac2ba3 scripts/lib/detect-gpu.sh -ff17020ab0fe30bde430a54b33150084bb52c9543eabce8897706df94f0c48f5 scripts/lib/gpu-nvidia.sh +b1e54d00c2aa6afc7142dcf5668cdb7f054d5700a5f21c3d6fba3ac248b3c81f scripts/lib/gpu-nvidia.sh fd8d37d698c242ab081095625aa430bb91971db646e35393a02bbadd41325898 scripts/lib/gpu-amd.sh -265fa0575c17fff46d7e424a1b1c2664493cc833e0ff51464e4cf472d9c954fa scripts/lib/setup-macos.sh +b9a1f3be69c0ea5fe78555e7da101b45e485960795a9881126361e36d6c740af scripts/lib/setup-macos.sh 238f9bc0e344376c3883bad9e980db250799cacc9e7ce3bfc7ef37c5b5881244 scripts/lib/setup-linux.sh -ba01f93f3e15b67eb438314551cee87c9172611c9c630ab8b4b207324efc897d scripts/lib/cluster.sh -5de1afac8b1062a7f8d0691b46ad39ccdc2287e0e422b53c48628ecfd47d852e scripts/lib/gpu-plugins.sh -46eda5179b331a6246094fac42a12717d6edbabdfdbd0f4e7a617fd9569a0368 scripts/lib/install-client-helm.sh -f365c563bbbc85cb2d6864c372dc6998544afa520ef208aaeba2e065b18b74ee scripts/lib/install-cli.sh -ff39119445da8373352e1caf1037529242de3de103af54770bd891abbd7638db scripts/lib/provision.sh +41001b69be6bd1bb56aa64c0550b04af2ef0a407107befe3c44cd41e95f2af92 scripts/lib/cluster.sh +1790633a102040537707ba868ca350bee6a6d78dbbd8858bd51d2121d79f5a35 scripts/lib/gpu-plugins.sh +5457f2c07d7b3c92a978f1d6fbd51220663df74c35e3ccd5fa6f82e4f190bcc1 scripts/lib/install-client-helm.sh +7d180a17a6308a6039c6067bd7429be7af39c2af9cd504ee40d0092deb530151 scripts/lib/install-cli.sh +dc4bf176752f319ec2d1934fd13c440c2aa1698a5b4c9454050d50a6e0459789 scripts/lib/provision.sh e2ea63d844e6649f1d3aaae9fd4733845a1a39df37d68abbaeda00330f9e1c7e scripts/lib/assess.sh -9227862f1f86aff1985783babdec47cb92adb4619eaf32a1927c145b8111f06f scripts/lib/summary.sh -f22b3f57722feaf3a05a8527988c4404c27b772dd8ba8d6d4c6fe08077081a4a scripts/lib/diagnose.sh -8dfc1ae6ee432cbca71dd6cd316fba527b3f5edcba18310b8937a4e6fd2c38de scripts/install-k8s.ps1 +dfdaedb3cc8cfd3b5778efe388202dc14874b5a7d8cc081296c20db7c62a4542 scripts/lib/summary.sh +153ebc0d46157cfb5b9d6eeed2c534337f6c780a5dce6d1410daa47fb7d8f220 scripts/lib/diagnose.sh +7dea0ed20365007f29de918aa945a289ca0a15c76e1c86a45e059ae08a8f71a6 scripts/install-k8s.ps1 diff --git a/scripts/tests/install-client-helm.bats b/scripts/tests/install-client-helm.bats index 4e786d5c..6b184d4c 100644 --- a/scripts/tests/install-client-helm.bats +++ b/scripts/tests/install-client-helm.bats @@ -477,6 +477,26 @@ setup() { [[ "$output" != *"helm upgrade"* ]] } +@test "install_client_helm: helm list failure -> fails CLOSED (refuses, no upgrade)" { + HOST_DATA_DIR="$BATS_TEST_TMPDIR/data"; mkdir -p "$HOST_DATA_DIR" + _ensure_tracebloc_dirs() { :; } + _ensure_release_dirs() { :; } + _ensure_helm_runnable() { :; } + # `helm list` errors (wedged/unreachable API): detect_installed_client can't + # enumerate, so the guard must REFUSE rather than read empty as "no client here" + # and silently overwrite whatever is installed. + helm() { + if [ "$1" = list ]; then return 1; fi # enumeration fails + record "helm $*"; return 0 + } + verify_credentials() { printf valid; } + run install_client_helm <<< $'newclient\nmypw' + [ "$status" -ne 0 ] + [[ "$output" == *"Couldn't determine which tracebloc client"* ]] + run mock_calls + [[ "$output" != *"helm upgrade"* ]] +} + @test "install_client_helm: same client re-run is allowed (upgrade in place)" { HOST_DATA_DIR="$BATS_TEST_TMPDIR/data"; mkdir -p "$HOST_DATA_DIR" _ensure_tracebloc_dirs() { :; } diff --git a/scripts/tests/install-k8s.Tests.ps1 b/scripts/tests/install-k8s.Tests.ps1 index 0bac4cd3..b2f05328 100644 --- a/scripts/tests/install-k8s.Tests.ps1 +++ b/scripts/tests/install-k8s.Tests.ps1 @@ -423,6 +423,39 @@ Describe "Install-ClientHelm" { { Install-ClientHelm } | Should -Throw Should -Not -Invoke helm -ParameterFilter { $args -contains "upgrade" } } + It "fails CLOSED when 'helm list' itself errors (can't enumerate -> no silent overwrite)" { + # A failed enumeration must not read as "no client here" — that fails OPEN. + $HOST_DATA_DIR = "$TestDrive/d5-listfail" + Mock Err { throw "err" } + Mock Read-Host { + param([string]$Prompt, [switch]$AsSecureString) + if ($Prompt -match 'password') { return (ConvertTo-SecureString "pw" -AsPlainText -Force) } + return "newclient" + } + Mock Test-Credentials { "valid" } + Mock helm { + if ($args -contains "list") { $global:LASTEXITCODE = 1; return } # helm list failed + $global:LASTEXITCODE = 0 + } + { Install-ClientHelm } | Should -Throw + Should -Not -Invoke helm -ParameterFilter { $args -contains "upgrade" } + } + It "fails CLOSED when 'helm list' returns non-JSON garbage" { + $HOST_DATA_DIR = "$TestDrive/d5-listgarbage" + Mock Err { throw "err" } + Mock Read-Host { + param([string]$Prompt, [switch]$AsSecureString) + if ($Prompt -match 'password') { return (ConvertTo-SecureString "pw" -AsPlainText -Force) } + return "newclient" + } + Mock Test-Credentials { "valid" } + Mock helm { + if ($args -contains "list") { 'this is not json'; $global:LASTEXITCODE = 0; return } # rc 0 but garbage + $global:LASTEXITCODE = 0 + } + { Install-ClientHelm } | Should -Throw + Should -Not -Invoke helm -ParameterFilter { $args -contains "upgrade" } + } It "values without a clientId key do not trip the guard" { $HOST_DATA_DIR = "$TestDrive/d5-nokey" Mock Read-Host { From 4e6640e1c60149a3304e2823d1f24dcde58a4a17 Mon Sep 17 00:00:00 2001 From: Asad Iqbal Date: Tue, 14 Jul 2026 21:45:54 +0500 Subject: [PATCH 3/3] fix(installer): fail closed on unknown helm state in the #303 pre-provision guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bugbot (High) on the sweep: the sweep taught detect_installed_client to signal INSTALLED_CLIENT_UNKNOWN=1 on a failed `helm list` and wired it into the Helm-step one-client guard — but provision_client's #303 pre-flight still only checked INSTALLED_CLIENT_NS. A failed enumeration leaves both globals empty, so provisioning continued to `client create` and could register a dashboard client the later Helm guard then refuses to install — the exact orphan the pre-flight exists to prevent. Fail closed on INSTALLED_CLIENT_UNKNOWN right after detect_installed_client, before any mint — same signal the Helm-step guard keys on. +bats coverage (unknown state refuses before `client create`, no orphan). Co-Authored-By: Claude Opus 4.8 --- scripts/lib/provision.sh | 15 +++++++++++++++ scripts/manifest.sha256 | 2 +- scripts/tests/provision.bats | 18 ++++++++++++++++++ 3 files changed, 34 insertions(+), 1 deletion(-) diff --git a/scripts/lib/provision.sh b/scripts/lib/provision.sh index e724e711..87666534 100644 --- a/scripts/lib/provision.sh +++ b/scripts/lib/provision.sh @@ -178,6 +178,21 @@ provision_client() { # shared probe being present (a stale bootstrap may not have sourced it). if declare -F detect_installed_client >/dev/null 2>&1; then detect_installed_client + # Fail CLOSED when we couldn't enumerate what's here (helm/API failure): the + # NS check below can't distinguish "no client" from "couldn't tell", so minting + # now could strand a SECOND client (the exact orphan this pre-flight prevents). + # Same signal the Helm-step one-client guard keys on. + if [[ "${INSTALLED_CLIENT_UNKNOWN:-0}" == 1 ]]; then + echo "" + warn "Couldn't determine whether a tracebloc client is already installed here." + hint "tracebloc runs one client per machine. Registering a new client now could strand" + hint "a second one if an existing client just couldn't be seen — usually the cluster API" + hint "is briefly unreachable. Check it and re-run:" + hint " kubectl cluster-info" + hint " helm list -A" + echo "" + error "Refusing to provision without verifying what's already on this machine." + fi if [[ -n "$INSTALLED_CLIENT_NS" ]]; then local _own_rc=0 _account_owns_namespace "$INSTALLED_CLIENT_NS" || _own_rc=$? diff --git a/scripts/manifest.sha256 b/scripts/manifest.sha256 index 54440446..0c80922c 100644 --- a/scripts/manifest.sha256 +++ b/scripts/manifest.sha256 @@ -10,7 +10,7 @@ b9a1f3be69c0ea5fe78555e7da101b45e485960795a9881126361e36d6c740af scripts/lib/se 1790633a102040537707ba868ca350bee6a6d78dbbd8858bd51d2121d79f5a35 scripts/lib/gpu-plugins.sh 5457f2c07d7b3c92a978f1d6fbd51220663df74c35e3ccd5fa6f82e4f190bcc1 scripts/lib/install-client-helm.sh 7d180a17a6308a6039c6067bd7429be7af39c2af9cd504ee40d0092deb530151 scripts/lib/install-cli.sh -dc4bf176752f319ec2d1934fd13c440c2aa1698a5b4c9454050d50a6e0459789 scripts/lib/provision.sh +ea0d8a0e8e460ea06b90bfce4f436c635bc0a8e215d6c46e4299d4ae75511928 scripts/lib/provision.sh e2ea63d844e6649f1d3aaae9fd4733845a1a39df37d68abbaeda00330f9e1c7e scripts/lib/assess.sh dfdaedb3cc8cfd3b5778efe388202dc14874b5a7d8cc081296c20db7c62a4542 scripts/lib/summary.sh 153ebc0d46157cfb5b9d6eeed2c534337f6c780a5dce6d1410daa47fb7d8f220 scripts/lib/diagnose.sh diff --git a/scripts/tests/provision.bats b/scripts/tests/provision.bats index 99283a04..17def3d3 100644 --- a/scripts/tests/provision.bats +++ b/scripts/tests/provision.bats @@ -183,6 +183,24 @@ _stub_tracebloc() { [ ! -s "$CREATE_ARGS_FILE" ] # create was never called — no orphan minted } +@test "provision_client: unknown helm state (list failed) refuses BEFORE minting — no orphan (#303)" { + # detect_installed_client couldn't enumerate (helm/API failure): both globals + # empty but INSTALLED_CLIENT_UNKNOWN=1. Minting now could strand a second client, + # so provision_client must refuse before `client create` ever runs. + detect_installed_client() { INSTALLED_CLIENT_ID=""; INSTALLED_CLIENT_NS=""; INSTALLED_CLIENT_UNKNOWN=1; } + tracebloc() { + [[ "$*" == *--help ]] && return 0 + [ "$1" = "login" ] && return 0 + local f="" prev=""; for a in "$@"; do [ "$prev" = "--credential-file" ] && f="$a"; prev="$a"; done + [ -n "$f" ] && printf '%s\n' "$*" >>"${CREATE_ARGS_FILE}" + return 0 + } + run provision_client + [ "$status" -ne 0 ] + [[ "$output" == *"Couldn't determine whether a tracebloc client is already installed"* ]] + [ ! -s "$CREATE_ARGS_FILE" ] # create never ran — no orphan minted +} + @test "provision_client: same-account re-run with a local client still provisions (adopt path intact) (#303)" { # The account owns the local client's namespace → create proceeds and adopts. detect_installed_client() { INSTALLED_CLIENT_ID="uuid-x"; INSTALLED_CLIENT_NS="my-ns"; }