From 52790c360074579b4c2c4090954feb407dff8eae Mon Sep 17 00:00:00 2001 From: fullstackjam Date: Fri, 17 Jul 2026 20:56:38 +0800 Subject: [PATCH] =?UTF-8?q?fix(install.sh):=20never=20prompt=20on=20stdin?= =?UTF-8?q?=20=E2=80=94=20it's=20the=20script=20under=20curl|bash?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `curl -fsSL openboot.dev/install.sh | bash` is the documented install command, and there stdin is the pipe carrying the script — not the user's keyboard. Both prompts read from it anyway. The consequence was not cosmetic. The already-installed branch asked "Reinstall? (y/N)" and defaulted to No; via curl|bash the `read` consumed the script's own next bytes as the answer, which never matched ^[Yy]$. So it printed "Using existing installation", exec'd the old binary, and every existing user stayed on their old version no matter how many times they ran the installer. Releases went out that nobody could receive. The symptom — `openboot version` reporting yesterday's build right after a successful-looking install — pointed nowhere near the cause. (It also ate a byte of the script, so the following line ran mangled.) Changes: - prompts go through a new ask_tty helper: reads /dev/tty, and falls back to an explicit default when there is no terminal (CI, piped shells). - the already-installed branch no longer asks. Someone running the installer wants the current release; "reinstall?" defaulting to No was hostile even when the read worked. - `brew update` before upgrading. Homebrew only refreshes a tap every HOMEBREW_AUTO_UPDATE_SECS (24h default), so a release published inside that window is invisible and `upgrade` reports success while leaving the old binary in place — the second, independent way to get a stale install. This is not theoretical either: the report that prompted this landed 23.5h after the previous release. - the resolved version is always printed. Running the installer and silently getting yesterday's binary is exactly what this path failed at; printing it makes that impossible to miss. An archtest pins the rule so a bare `read` can't come back. It is a static check because the behavioural gap is wider than one script: curl-bash-smoke is gated `if: github.event_name != 'pull_request'` and only drives the mock-server path, so scripts/install.sh's brew branch has no test at all. Noted in HARNESS.md as the next control to add. --- docs/HARNESS.md | 2 + internal/archtest/installsh_test.go | 72 +++++++++++++++++++++++++++++ scripts/install.sh | 54 ++++++++++++++++------ 3 files changed, 115 insertions(+), 13 deletions(-) create mode 100644 internal/archtest/installsh_test.go diff --git a/docs/HARNESS.md b/docs/HARNESS.md index 1bf392a..00d240e 100644 --- a/docs/HARNESS.md +++ b/docs/HARNESS.md @@ -47,6 +47,7 @@ Three regulation categories: | Arch. | `no-os-getenv-home` | L1 | `internal/archtest/envhome_test.go` | | Arch. | `dryrun` — destructive ops must check `DryRun` | L1 | `internal/archtest/dryrun_test.go` | | Arch. | `no-raw-fmtprint` — UI output via `ui.*` helpers, not raw `fmt.Print*` | L1 | `internal/archtest/fmtprint_test.go` | +| Arch. | `install.sh` must not prompt on stdin — under `curl \| bash` stdin is the script | L1 | `internal/archtest/installsh_test.go` | | Behav. | L1 unit + integration + contract (faked runners *and* real brew/git/npm in temp dirs) | pre-push, CI | `make test-unit` | | Behav. | L2 contract schema (against openboot-contract repo) | CI | `.github/workflows/test.yml` `contract` job | | Behav. | L3 e2e binary | release | `make test-e2e` | @@ -74,6 +75,7 @@ When you observe a recurring issue, decide where to encode the fix: | "Agent doesn't know about preset X." | Update `internal/config/data/presets.yaml`. Source of truth, not docs. | | "Agent introduced a new lint failure that golangci-lint should have caught." | Enable the relevant linter in `.golangci.yml`. | | "Agent broke a behaviour that has no test." | Write the test at the right tier — L1 covers both faked-runner units in `internal//` and real-subprocess integration in `test/integration/`. | +| "A shipped release reached nobody: `install.sh`'s already-installed branch prompted on stdin, which under `curl \| bash` is the script itself, so it always took the don't-upgrade default." | Already handled by the `installsh` archtest. The wider lesson the sensor does *not* cover: `curl-bash-smoke` is gated `if: github.event_name != 'pull_request'` and only exercises the mock-server path, so the real `scripts/install.sh` brew branch has no behavioural test. Upgrade-over-existing-install is the case to add. | | "Agent missed a CLAUDE.md rule we keep restating." | Make it a hard or soft archtest rule (a docs rule that doesn't fail is a docs rule that drifts). | | "Agent did something safe but suboptimal." | Add to CLAUDE.md "Project-specific conventions" and consider whether it's encodable. | | "Agent guessed at an API contract." | Update `openboot-contract` repo + fixtures; CI already runs schema validation. | diff --git a/internal/archtest/installsh_test.go b/internal/archtest/installsh_test.go new file mode 100644 index 0000000..8261027 --- /dev/null +++ b/internal/archtest/installsh_test.go @@ -0,0 +1,72 @@ +package archtest + +import ( + "os" + "path/filepath" + "regexp" + "strings" + "testing" +) + +// bareReadRe matches a `read` prompt that takes no explicit input redirect. +// `read ... 0 { + t.Errorf("install.sh prompts on stdin, which under `curl | bash` is the script itself:\n %s\n\n"+ + "Fix: ask via the ask_tty helper (reads /dev/tty, falls back to a default with no terminal).", + strings.Join(bad, "\n ")) + } +} + +func itoa(n int) string { + if n == 0 { + return "0" + } + var b []byte + for n > 0 { + b = append([]byte{byte('0' + n%10)}, b...) + n /= 10 + } + return string(b) +} diff --git a/scripts/install.sh b/scripts/install.sh index 3ae119a..30b4b5b 100644 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -7,6 +7,27 @@ BINARY_NAME="openboot" TAP_NAME="openbootdotdev/tap" DRY_RUN="${OPENBOOT_DRY_RUN:-false}" +# This script's headline use is `curl -fsSL openboot.dev/install.sh | bash`, +# where stdin is the pipe carrying the script itself — NOT the user's keyboard. +# A bare `read` there consumes the script's own next bytes as the answer, which +# both mangles the script and silently takes a branch the user never chose. +# Every prompt must therefore read from the controlling terminal, and must have +# a safe answer for when there isn't one (piped into a CI job, no tty). +has_tty() { [[ -e /dev/tty ]] && (: >/dev/tty) 2>/dev/null; } + +# ask_tty [read-opts...] — echoes the reply, or +# when no terminal is available to ask. +ask_tty() { + local prompt="$1" default="$2"; shift 2 + if ! has_tty; then + echo "$default" + return + fi + local reply="" + read -r "$@" -p "$prompt" reply /dev/null; then return 0 @@ -18,7 +39,7 @@ install_xcode_clt() { echo "Xcode Command Line Tools need to be installed." echo "A dialog will appear - please click 'Install' and enter your password." echo "" - read -p "Press Enter to launch installer..." -r + ask_tty "Press Enter to launch installer..." "" >/dev/null echo "" xcode-select --install 2>/dev/null || true @@ -161,29 +182,36 @@ main() { fi if brew list openboot &>/dev/null 2>&1; then - echo "OpenBoot is already installed via Homebrew." + echo "OpenBoot is already installed — updating..." echo "" - read -p "Reinstall? (y/N) " -n 1 -r - echo - - if [[ $REPLY =~ ^[Yy]$ ]]; then - echo "Reinstalling OpenBoot..." + + # Refresh the tap before upgrading. Homebrew only auto-updates it every + # HOMEBREW_AUTO_UPDATE_SECS (24h by default), so a release published + # since the last refresh is invisible to `upgrade`, which then reports + # success while leaving the old binary in place. + brew update >/dev/null 2>&1 || true + if ! brew upgrade ${TAP_NAME}/openboot 2>/dev/null; then brew reinstall ${TAP_NAME}/openboot - echo "" - echo "✓ OpenBoot reinstalled!" - else - echo "Using existing installation." fi + + echo "" + echo "✓ OpenBoot updated!" else echo "Installing OpenBoot via Homebrew..." echo "" - + brew install ${TAP_NAME}/openboot - + echo "" echo "✓ OpenBoot installed!" fi + # Always state the version we ended up on. Running the installer and + # silently getting yesterday's binary is the failure this whole path is + # guarding against; printing it makes that impossible to miss. + hash -r 2>/dev/null || true + echo " $(openboot version 2>/dev/null || echo 'version unavailable')" + echo "" echo "Starting OpenBoot setup..." echo ""