From b79cca9c2a49c2260a54e0f5d1094482846c9b67 Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Mon, 17 Aug 2026 12:53:37 +0200 Subject: [PATCH] fix(installer): a missed sign-in code costs one prompt, not a whole re-run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cli#517. `tracebloc login` failing was fatal to provision_client, so a device code that lapsed — the ordinary outcome when the human approving it is reading a phone — threw away every step the installer had already completed and asked for a full re-run. _device_sign_in wraps the call in the same shape the name prompt has used since the 2026-07-09 type-ahead report: one in-place retry, offered only when there is a live terminal to hand a fresh code to, and abandoned on a failed read (EOF / no live input) because re-prompting cannot fix that. A second failure is evidence of something other than a missed code, so it falls through to the same fatal error as before. The sign-in now runs with TRACEBLOC_INSTALLER=1 (a command prefix, not an export — no later CLI call inherits it), which makes the CLI drop its own "run `tracebloc login`" line. Under the installer that advice was actively wrong: a bare login leaves the client mint and the Helm install undone. The installer's "re-run the installer" is now the only next step printed. The /dev/tty openability probe moves into _login_tty_ok, unchanged, purely so the tests can force either branch without a controlling terminal. Co-Authored-By: Claude Opus 5 --- scripts/lib/provision.sh | 80 +++++++++++++++++++++++---- scripts/manifest.sha256 | 2 +- scripts/tests/provision.bats | 103 +++++++++++++++++++++++++++++++++++ 3 files changed, 172 insertions(+), 13 deletions(-) diff --git a/scripts/lib/provision.sh b/scripts/lib/provision.sh index bd734648..52d30ee2 100644 --- a/scripts/lib/provision.sh +++ b/scripts/lib/provision.sh @@ -47,6 +47,73 @@ _prompt_tty() { [[ -r /dev/tty && -w /dev/tty ]]; } # terminal. Overridable so tests can feed canned input on stdin (TB_TTY=/dev/stdin). : "${TB_TTY:=/dev/tty}" +# _login_tty_ok: is /dev/tty openable for the sign-in's redirection? Split out +# as its own function (rather than reusing _prompt_tty) purely so tests can force +# either branch: _prompt_tty gates the PROMPTS below, and several tests force it +# on while having no real /dev/tty to redirect a child process onto. +_login_tty_ok() { { : /dev/null; } + +# _run_device_login: one `tracebloc login`, given 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/tty 2>/dev/tty + else + TRACEBLOC_INSTALLER=1 tracebloc login /dev/tty 2>/dev/null || true + IFS= read -r _ <"$TB_TTY" || break + echo "" + done + error "Sign-in didn't complete — re-run the installer to try again." +} + # _detect_location_zone: best-effort ISO country code for where this machine # physically runs, derived from the system timezone via the OS's OWN zone.tab — # no network call (privacy-preserving for on-prem installs) and no embedded zone @@ -164,18 +231,7 @@ 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 "" - # 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 >"$ATTEMPTS_FILE" + [[ "$(grep -c . "$ATTEMPTS_FILE")" -le "$SIGN_IN_FAIL_FIRST" ]] && return 1 + return 0 + } +} + +_attempts() { grep -c . "$ATTEMPTS_FILE" 2>/dev/null || echo 0; } + +@test "_device_sign_in: a lapsed code is retried in place, not fatal" { + # THE cli#517 fix: before it, one missed ten-minute code threw away every step + # the installer had already completed. It must cost a single Enter instead. + _prompt_tty() { return 0; } # a live terminal is available + TB_TTY="$(mktemp)"; printf '\n' >"$TB_TTY" # the human presses Enter + _stub_sign_in_failing 1 + run _device_sign_in + [ "$status" -eq 0 ] || return 1 + [ "$(_attempts)" -eq 2 ] || return 1 + # The "Press Enter" line goes to /dev/tty (like every prompt here), so it is + # not in $output; the warn/hint that explain the pause do go through the log. + [[ "$output" == *"the code lapsed or wasn't approved"* ]] || return 1 + [[ "$output" == *"Nothing is lost"* ]] || return 1 +} + +@test "_device_sign_in: a second failure is fatal, and names the installer" { + # The retry is ONE retry. A second failure is evidence of something other than + # a missed code, and the advice must be the installer — not `tracebloc login`, + # which would leave the client mint and the Helm install undone. + _prompt_tty() { return 0; } + TB_TTY="$(mktemp)"; printf '\n\n\n' >"$TB_TTY" + _stub_sign_in_failing 99 + run _device_sign_in + [ "$status" -ne 0 ] || return 1 + [ "$(_attempts)" -eq 2 ] || return 1 + [[ "$output" == *"re-run the installer"* ]] || return 1 + [[ "$output" != *"tracebloc login"* ]] || return 1 + # Offered ONCE. The attempt count alone can't see a guard that lets the loop + # prompt after its final try — the user would be asked to press Enter for a + # code that is never fetched — so count the offer, not just the attempts. + [ "$(printf '%s\n' "$output" | grep -c 'Nothing is lost')" -eq 1 ] || return 1 +} + +@test "_device_sign_in: with no terminal there is no retry to offer" { + # Nobody to hand a fresh code to — re-prompting would just hang or spin. + # TB_TTY is deliberately READABLE here: with a dead one, dropping the + # _prompt_tty gate would stop on the failed read instead and the assertion + # below could not tell the gate from the EOF (the mutation ran green that way). + _prompt_tty() { return 1; } + TB_TTY="$(mktemp)"; printf '\n\n\n' >"$TB_TTY" + _stub_sign_in_failing 99 + run _device_sign_in + [ "$status" -ne 0 ] || return 1 + [ "$(_attempts)" -eq 1 ] || return 1 + [[ "$output" != *"Nothing is lost"* ]] || return 1 +} + +@test "_device_sign_in: an EOF on the retry prompt stops, it does not spin" { + # A failed read (rc != 0 — EOF, a non-PTY ssh session, a closed pipe) can't be + # fixed by asking again; the loop must fall straight through to the error. + _prompt_tty() { return 0; } + TB_TTY=/dev/null # read returns EOF immediately + _stub_sign_in_failing 99 + run _device_sign_in + [ "$status" -ne 0 ] || return 1 + [ "$(_attempts)" -eq 1 ] || return 1 + [[ "$output" == *"re-run the installer"* ]] || return 1 +} + +@test "_run_device_login: the CLI is told the installer is driving it" { + # TRACEBLOC_INSTALLER is what stops the CLI printing "run \`tracebloc login\`" + # — advice that is right for a hand-typed login and wrong under the installer. + # Pinned on the no-tty branch so it reads the same in CI and on a laptop. + _login_tty_ok() { return 1; } + local seen="$BATS_TEST_TMPDIR/env" + tracebloc() { printf '%s|%s\n' "${TRACEBLOC_INSTALLER:-unset}" "$*" >"$seen"; return 0; } + _run_device_login + [[ "$(cat "$seen")" == "1|login" ]] || return 1 +} + +@test "_run_device_login: TRACEBLOC_INSTALLER does not leak past the sign-in" { + # Set as a command prefix, not exported for the rest of the install: `client + # create` and every later CLI call must see the environment they always did. + _login_tty_ok() { return 1; } + tracebloc() { return 0; } + _run_device_login + [ -z "${TRACEBLOC_INSTALLER:-}" ] || return 1 +} + @test "provision_client: client create writing no credential file is fatal" { tracebloc() { return 0; } # login OK, create "succeeds" but writes nothing run provision_client