Uh oh!
There was an error while loading. Please reload this page.
fix(installer): survive curl|bash — retry name prompt (customer-reported), read creds from terminal, guard pkg-index refresh - #326
Merged
Conversation
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 153c3c4. Configure here.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
LukasWodka added a commit
that referenced
this pull request
Jul 10, 2026
…, not just a missing one Review feedback on #326 (Asad + Bugbot): the credential reads route through $TB_TTY but had no EOF guard, unlike the provision.sh name read (which breaks on rc!=0). _tty_available only checks `-r`, so on a readable-but-dead-input tty (non-PTY ssh, an IDE terminal, a drained/queued tty — the same class this PR documents for provision.sh) it returns true, the actionable no-creds error is skipped, and the first `read <"$TB_TTY"` hits EOF and aborts under set -e — the exact opaque failure this PR set out to remove, left in place for creds. Factor the actionable env-var guidance into _no_interactive_creds_die and call it from BOTH the `! _tty_available` check AND a per-read `|| _no_interactive_creds_die` guard on all five prompts (the Use-previous read + the ID/password reads). A dead-input tty now fails fast with the same guidance as no-tty instead of aborting mid-read. Happy path (input present) is unchanged — the guard only fires on EOF; all existing cred tests (re-prompt, reuse-defaults, max-attempts) consume their fed input exactly and still pass. +1 regression test (readable /dev/stdin backed by /dev/null → EOF → actionable error, no helm). Regenerated scripts/manifest.sha256 (R8). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…esh flakes install_system_deps ran `spin_cmd "Updating package index…" $PM_UPDATE` unguarded — under set -e a transient mirror/network failure there aborted the whole install. Yet the per-package installs right below are already guarded (|| log), so a flaky refresh was MORE fatal than a failed install, which is backwards: a stale index usually still installs from cache. Guard it with || warn so we continue to the (guarded) installs, which surface a genuinely missing package with an actionable message. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…h doesn't abort The Step-5 credential prompt read from stdin. Under `curl … | bash` stdin is the piped script, not the terminal, so each `read` hit EOF and (under set -e) aborted the installer with an opaque failure the moment it reached the prompt — the dual-mode env-var path (TRACEBLOC_CLIENT_ID/PASSWORD) was the only way through, but nothing told the user that. Read prompts from TB_TTY (the controlling terminal, /dev/tty) instead, the same mechanism provision.sh already uses. When no terminal is available and no env creds were supplied, fail with an actionable message pointing at TRACEBLOC_CLIENT_ID/PASSWORD rather than the set -e abort. TB_TTY is overridable so the bats suite can feed canned input on stdin. Regenerated scripts/manifest.sha256 (R8 supply-chain: any scripts/ change must re-pin, alongside the setup-linux.sh guard in the previous commit). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…abort provisioning Customer-reported 2026-07-09 (external onboarding): the installer signed in, printed the name + location prompts, captured NEITHER, and died with 'A name for this client is required to provision it.' provision_client read the name with `IFS= read -r client_name </dev/tty || true` — a single shot whose status was swallowed by `|| true`, with no retry and no fallback (location silently defaults to the detected zone, which is why only the name failed hard). Any empty/failed read on the name → the fatal error. The most likely trigger is tty type-ahead: during the ~minute browser-approval wait the CLI reads nothing, so a stray newline queued in the terminal is consumed by the read as an empty name. (An adversarial pass ruled out the background-process-group and login-drains-the-tty theories; the reader is foreground and login never touches the tty — the empty read is an environmental dead/queued-input condition.) Fix: read the name in a bounded retry loop that RE-PROMPTS on an empty line (so a queued blank is skipped, not accepted) and BREAKS on a failed read (rc!=0 = EOF / no live input, which re-prompting can't fix) so the actionable 'set TRACEBLOC_CLIENT_NAME' error still fires. Reads route through TB_TTY (defaults to /dev/tty; overridable so the bats suite can feed stdin), matching the install-client-helm.sh credential reads in this branch; prompt WRITES stay on /dev/tty but are guarded so a test without a real terminal doesn't abort. The location reads adopt TB_TTY too (their empty->fallback behavior is unchanged). 2 new provision.bats tests: type-ahead blanks are re-prompted then the real name is captured; a dead-input tty (EOF) fails fast with the guidance. Regenerated scripts/manifest.sha256 (R8). NOTE: this is Failure 1 of the report; the existing install-client-helm.sh fix on this branch does NOT cover these provision.sh reads. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…, not just a missing one Review feedback on #326 (Asad + Bugbot): the credential reads route through $TB_TTY but had no EOF guard, unlike the provision.sh name read (which breaks on rc!=0). _tty_available only checks `-r`, so on a readable-but-dead-input tty (non-PTY ssh, an IDE terminal, a drained/queued tty — the same class this PR documents for provision.sh) it returns true, the actionable no-creds error is skipped, and the first `read <"$TB_TTY"` hits EOF and aborts under set -e — the exact opaque failure this PR set out to remove, left in place for creds. Factor the actionable env-var guidance into _no_interactive_creds_die and call it from BOTH the `! _tty_available` check AND a per-read `|| _no_interactive_creds_die` guard on all five prompts (the Use-previous read + the ID/password reads). A dead-input tty now fails fast with the same guidance as no-tty instead of aborting mid-read. Happy path (input present) is unchanged — the guard only fires on EOF; all existing cred tests (re-prompt, reuse-defaults, max-attempts) consume their fed input exactly and still pass. +1 regression test (readable /dev/stdin backed by /dev/null → EOF → actionable error, no helm). Regenerated scripts/manifest.sha256 (R8). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
LukasWodkaforce-pushed
the
fix/installer-robustness
branch
from
July 10, 2026 11:06
6770524 to
508a378Comparesaadqbal
approved these changes
Jul 10, 2026
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Jul 10, 2026
saadqbal added a commit
that referenced
this pull request
Jul 10, 2026
Publishes everything unreleased since v1.9.0: - #323 ingestor default tag → 0.6 (landed as 1.9.1, never released) - #325 stop leaking client password on curl's argv (CWE-214) - #326 survive curl|bash: retry name prompt, read creds from terminal, guard pkg-index refresh (customer-reported) Refs #328 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
saadqbal added a commit
that referenced
this pull request
Jul 10, 2026
… + curl|bash survival) (#327) * Merge pull request #325 from tracebloc/fix/cred-leak-curl-argv fix(installer): stop leaking the client password on curl's argv (CWE-214) * fix(installer): survive curl|bash — retry name prompt (customer-reported), read creds from terminal, guard pkg-index refresh (#326) * fix(installer): don't abort Linux install when the package-index refresh flakes install_system_deps ran `spin_cmd "Updating package index…" $PM_UPDATE` unguarded — under set -e a transient mirror/network failure there aborted the whole install. Yet the per-package installs right below are already guarded (|| log), so a flaky refresh was MORE fatal than a failed install, which is backwards: a stale index usually still installs from cache. Guard it with || warn so we continue to the (guarded) installs, which surface a genuinely missing package with an actionable message. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(installer): read credential prompts from the terminal so curl|bash doesn't abort The Step-5 credential prompt read from stdin. Under `curl … | bash` stdin is the piped script, not the terminal, so each `read` hit EOF and (under set -e) aborted the installer with an opaque failure the moment it reached the prompt — the dual-mode env-var path (TRACEBLOC_CLIENT_ID/PASSWORD) was the only way through, but nothing told the user that. Read prompts from TB_TTY (the controlling terminal, /dev/tty) instead, the same mechanism provision.sh already uses. When no terminal is available and no env creds were supplied, fail with an actionable message pointing at TRACEBLOC_CLIENT_ID/PASSWORD rather than the set -e abort. TB_TTY is overridable so the bats suite can feed canned input on stdin. Regenerated scripts/manifest.sha256 (R8 supply-chain: any scripts/ change must re-pin, alongside the setup-linux.sh guard in the previous commit). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(installer): retry the client-name prompt so tty type-ahead can't abort provisioning Customer-reported 2026-07-09 (external onboarding): the installer signed in, printed the name + location prompts, captured NEITHER, and died with 'A name for this client is required to provision it.' provision_client read the name with `IFS= read -r client_name </dev/tty || true` — a single shot whose status was swallowed by `|| true`, with no retry and no fallback (location silently defaults to the detected zone, which is why only the name failed hard). Any empty/failed read on the name → the fatal error. The most likely trigger is tty type-ahead: during the ~minute browser-approval wait the CLI reads nothing, so a stray newline queued in the terminal is consumed by the read as an empty name. (An adversarial pass ruled out the background-process-group and login-drains-the-tty theories; the reader is foreground and login never touches the tty — the empty read is an environmental dead/queued-input condition.) Fix: read the name in a bounded retry loop that RE-PROMPTS on an empty line (so a queued blank is skipped, not accepted) and BREAKS on a failed read (rc!=0 = EOF / no live input, which re-prompting can't fix) so the actionable 'set TRACEBLOC_CLIENT_NAME' error still fires. Reads route through TB_TTY (defaults to /dev/tty; overridable so the bats suite can feed stdin), matching the install-client-helm.sh credential reads in this branch; prompt WRITES stay on /dev/tty but are guarded so a test without a real terminal doesn't abort. The location reads adopt TB_TTY too (their empty->fallback behavior is unchanged). 2 new provision.bats tests: type-ahead blanks are re-prompted then the real name is captured; a dead-input tty (EOF) fails fast with the guidance. Regenerated scripts/manifest.sha256 (R8). NOTE: this is Failure 1 of the report; the existing install-client-helm.sh fix on this branch does NOT cover these provision.sh reads. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(installer): guard credential reads against a dead-input tty (EOF), not just a missing one Review feedback on #326 (Asad + Bugbot): the credential reads route through $TB_TTY but had no EOF guard, unlike the provision.sh name read (which breaks on rc!=0). _tty_available only checks `-r`, so on a readable-but-dead-input tty (non-PTY ssh, an IDE terminal, a drained/queued tty — the same class this PR documents for provision.sh) it returns true, the actionable no-creds error is skipped, and the first `read <"$TB_TTY"` hits EOF and aborts under set -e — the exact opaque failure this PR set out to remove, left in place for creds. Factor the actionable env-var guidance into _no_interactive_creds_die and call it from BOTH the `! _tty_available` check AND a per-read `|| _no_interactive_creds_die` guard on all five prompts (the Use-previous read + the ID/password reads). A dead-input tty now fails fast with the same guidance as no-tty instead of aborting mid-read. Happy path (input present) is unchanged — the guard only fires on EOF; all existing cred tests (re-prompt, reuse-defaults, max-attempts) consume their fed input exactly and still pass. +1 regression test (readable /dev/stdin backed by /dev/null → EOF → actionable error, no helm). Regenerated scripts/manifest.sha256 (R8). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * chore(chart): bump client 1.9.1 → 1.9.2 (version + appVersion) (#329) Publishes everything unreleased since v1.9.0: - #323 ingestor default tag → 0.6 (landed as 1.9.1, never released) - #325 stop leaking client password on curl's argv (CWE-214) - #326 survive curl|bash: retry name prompt, read creds from terminal, guard pkg-index refresh (customer-reported) Refs #328 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: lukasWuttke <54042461+LukasWodka@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Summary
Installer robustness for the
curl … | bashpath — three fixes that stop opaqueset -eaborts, including the customer-reported onboarding failure of 2026-07-09 (Moritz, external).1.
fix(installer)— retry the client-name prompt so tty type-ahead can't abort provisioning ⭐ customer-reportedprovision_clientread the client name withIFS= read -r client_name </dev/tty || true— one shot, status swallowed by|| true, no retry, no fallback (location silently defaults to the detected zone, which is why only the name failed hard). Any empty/failed read → the fatal "A name for this client is required to provision it." The customer saw both prompts print, neither captured, install aborted.Most likely trigger: tty type-ahead — during the ~minute browser-approval wait the CLI reads nothing, so a stray newline queued in the terminal is consumed by the read as an empty name. (An adversarial verification pass ruled out the background-process-group and "login drains the tty" theories: the reader is foreground and
tracebloc loginnever touches the tty — the empty read is an environmental dead/queued-input condition, not our process state.)Fix: read the name in a bounded retry loop that re-prompts on an empty line (a queued blank is skipped, not accepted) and breaks on a failed read (
rc≠0= EOF / no live input, which re-prompting can't fix) so the actionable setTRACEBLOC_CLIENT_NAMEerror still fires for genuinely non-interactive runs. Reads route throughTB_TTY(same seam as the credential reads below). +2 regression tests.2.
fix(installer)— read credential prompts from the terminal (TB_TTY)The Step-5 credential prompt read stdin, which under
curl … | bashis the piped script → EOF →set -eabort. Now reads fromTB_TTY(=/dev/tty), with an actionable error when there's no terminal and no env creds.3.
fix(installer)— don't abort on a flaky package-index refreshspin_cmd "…" $PM_UPDATEwas unguarded underset -e(a flaky mirror aborted the whole install, though the per-package installs below are already guarded). Now|| warn.Regenerated
scripts/manifest.sha256(R8 supply-chain) for all three.Test plan
bats scripts/tests/provision.bats— 18/18 green (incl. the 2 new type-ahead / dead-input tests).bats scripts/tests/install-client-helm.bats+setup-linux.bats— green for the touched paths._extract_yaml_value: single-quoted with '' escape,validate_config: valid config passes) are pre-existing ondevelopand macOS-bash-3.2-specific (untouched code) — verified by stashing my changes; Linux CI (standard-checks.ymlruns bats + shellcheck) is authoritative.Immediate customer unblock (independent of merge)
Pre-setting the name skips the prompt entirely (the read only runs
if [[ -z "$client_name" ]]).🤖 Generated with Claude Code
Note
Low Risk
Changes are confined to installer shell UX and error handling (no auth backend or cluster logic); behavior for env-supplied credentials and unattended installs is preserved, with new regression tests.
Overview
Hardens the bash installer for
curl … | bashand flaky Linux package mirrors so failures surface as clear guidance instead of opaqueset -eexits.Provisioning (
provision.sh) routes interactive reads throughTB_TTY(default/dev/tty) and retries the client-name prompt on empty lines (fixes customer-reported type-ahead after browser approval) while stopping on EOF so non-interactive runs still hit theTRACEBLOC_CLIENT_NAMEerror.Helm credential step (
install-client-helm.sh) adds the sameTB_TTYpattern,_tty_available/_no_interactive_creds_die, and per-read guards so missing or dead terminals fail withTRACEBLOC_CLIENT_ID/TRACEBLOC_CLIENT_PASSWORDinstructions instead of mid-readaborts; “use previous settings” only runs when a TTY exists.Linux deps (
setup-linux.sh) wraps$PM_UPDATEwith|| warnso a failed index refresh does not kill the whole install while per-package installs remain best-effort.scripts/manifest.sha256is updated for the touched libs; bats cover no-TTY, EOF-on-readable-TTY, and name type-ahead regressions.Reviewed by Cursor Bugbot for commit 508a378. Bugbot is set up for automated code reviews on this repo. Configure here.