fix: headless credential prompts refuse loudly, naming their variable #43

Merged

dan-claude-bot merged 1 commit from fix/silent-prompts into main

2026-07-19 13:00:48 +00:00

dan-claude-bot commented

2026-07-19 12:15:12 +00:00

(Migrated from github.com)

Fixes #42.

What

The interactive credential prompts — TS_AUTHKEY (bootstrap), RUNNER_TOKEN (runner install), RUNNER_REMOVE_TOKEN (runner remove), and both tokens in runner repoint — were bare read -rsp. With stdin not a tty (CI, box exec, any script), read exits non-zero, set -euo pipefail ends the run, and nothing is printed: the drill measured rig runner remove exiting 1 in total silence, and a guest bootstrap whose log just stops after system hostname already ….

How

Each prompt now:

refuses first when stdin is not a tty — die naming the exact variable that unblocks an unattended run (runner remove/repoint also name --local), matching how every other refusal in rig behaves (db.sh:152 already held this line);
guards the read itself with || die, so EOF at a real prompt (Ctrl-D) also dies with a last word instead of riding set -e into silence.

Tests

Three grep-proof checks (repo precedent: the login-path tag refusals) — one per named guard, since the prompts sit behind the root check the harness cannot cross.
One sweep check: no bare read -rsp may exist under commands/ (expects grep exit 1). This is the check that caught runner repoint's two prompts, which the issue had not counted — it fails on any future prompt added without the cure.

bash test/cli.sh: 325 passed, 0 failed · bash test/release.sh: 41 passed, 0 failed · shellcheck clean.

🤖 Generated with Claude Code

Fixes #42. ## What The interactive credential prompts — `TS_AUTHKEY` (`bootstrap`), `RUNNER_TOKEN` (`runner install`), `RUNNER_REMOVE_TOKEN` (`runner remove`), and both tokens in `runner repoint` — were bare `read -rsp`. With stdin not a tty (CI, `box exec`, any script), `read` exits non-zero, `set -euo pipefail` ends the run, and **nothing is printed**: the drill measured `rig runner remove` exiting 1 in total silence, and a guest bootstrap whose log just stops after `system hostname already …`. ## How Each prompt now: 1. refuses first when stdin is not a tty — `die` naming the exact variable that unblocks an unattended run (`runner remove`/`repoint` also name `--local`), matching how every other refusal in rig behaves (`db.sh:152` already held this line); 2. guards the `read` itself with `|| die`, so EOF at a real prompt (Ctrl-D) also dies with a last word instead of riding `set -e` into silence. ## Tests - Three grep-proof checks (repo precedent: the login-path tag refusals) — one per named guard, since the prompts sit behind the root check the harness cannot cross. - One sweep check: **no bare `read -rsp` may exist under `commands/`** (expects `grep` exit 1). This is the check that caught `runner repoint`'s two prompts, which the issue had not counted — it fails on any future prompt added without the cure. `bash test/cli.sh`: 325 passed, 0 failed · `bash test/release.sh`: 41 passed, 0 failed · shellcheck clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

danmt (Migrated from github.com) reviewed 2026-07-19 12:15:12 +00:00

claude-bot-andresmgsl (Migrated from github.com) reviewed 2026-07-19 12:39:11 +00:00

claude-bot-andresmgsl (Migrated from github.com) left a comment

✅ Reviewed — I agree with all of this, no concerns.

The two-part cure is right: refuse first with [ -t 0 ] || die naming the exact variable that unblocks an unattended run (and --local where it applies), then || die-guard the read so EOF at a real tty also gets a last word instead of riding set -e into silence. Every prompt gets the treatment, and the env-var override still skips the prompt so unattended runs with the var set work unchanged. The test design is the strong part — three per-guard grep checks (behind the root check the harness can't cross), plus the read -rsp[^|]*$ sweep that expects grep exit 1, which is exactly what caught runner repoint's two prompts the issue hadn't counted and will catch any future bare prompt added without the cure. Clean.

— automated review by claude-bot-andresmgsl · heavy-duty-review-bot

✅ **Reviewed — I agree with all of this, no concerns.** The two-part cure is right: refuse first with `[ -t 0 ] || die` naming the exact variable that unblocks an unattended run (and `--local` where it applies), then `|| die`-guard the `read` so EOF at a real tty also gets a last word instead of riding `set -e` into silence. Every prompt gets the treatment, and the env-var override still skips the prompt so unattended runs with the var set work unchanged. The test design is the strong part — three per-guard grep checks (behind the root check the harness can't cross), plus the `read -rsp[^|]*$` sweep that expects `grep` exit 1, which is exactly what caught `runner repoint`'s two prompts the issue hadn't counted and will catch any future bare prompt added without the cure. Clean. — _automated review by `claude-bot-andresmgsl` · heavy-duty-review-bot_

claude-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-19 12:44:22 +00:00

claude-bot-andresmgsl (Migrated from github.com) left a comment

Approve. The fix matches the drill-measured failure exactly, covers all five prompt sites (two more than #42 counted), the named variables are spelled correctly against what each script reads, refusals go to stderr with exit 1 via each script's die, and [ -t 0 ] is reliable here because bin/rig dispatches with exec so stdin reaches the command untouched. Both suites pass (325 + 41). Notes, none blocking:

All guards verified: bootstrap.sh:406, runner-install.sh:126, runner-remove.sh:77, runner-repoint.sh:134,144 — each names the variable it actually reads, remove/repoint also name --local, and every read carries the || die EOF guard.
Behavior change worth a line in the PR body: piping a token via stdin (printf '%s\n' "$TOK" | rig runner install) used to work and is now refused. Undocumented path and the error names the fix, so fine — but it's real.
test/cli.sh:483 sweep only matches the literal read -rsp and only under commands/; a future read -s -p or a prompt in bin/rig escapes it. Consider read .*-p or similar.
runner-repoint.sh has no per-guard grep test (only the sweep, which proves the || guard, not the tty refusal). A grep -c asserting 2 hits in repoint would close it.
Pre-existing, same class: bin/rig:234 read -r reply in uninstall_confirm still dies silently on Ctrl-D at a real tty under set -e (headless is guarded at 228). CHANGELOG's "now all of rig does" slightly overclaims — fair game for a follow-up.

— automated review by claude-bot-andresmgsl · heavy-duty-review-bot

**Approve.** The fix matches the drill-measured failure exactly, covers all five prompt sites (two more than #42 counted), the named variables are spelled correctly against what each script reads, refusals go to stderr with exit 1 via each script's `die`, and `[ -t 0 ]` is reliable here because `bin/rig` dispatches with `exec` so stdin reaches the command untouched. Both suites pass (325 + 41). Notes, none blocking: - All guards verified: `bootstrap.sh:406`, `runner-install.sh:126`, `runner-remove.sh:77`, `runner-repoint.sh:134,144` — each names the variable it actually reads, remove/repoint also name `--local`, and every `read` carries the `|| die` EOF guard. - Behavior change worth a line in the PR body: piping a token via stdin (`printf '%s\n' "$TOK" | rig runner install`) used to work and is now refused. Undocumented path and the error names the fix, so fine — but it's real. - `test/cli.sh:483` sweep only matches the literal `read -rsp` and only under `commands/`; a future `read -s -p` or a prompt in `bin/rig` escapes it. Consider `read .*-p` or similar. - `runner-repoint.sh` has no per-guard grep test (only the sweep, which proves the `||` guard, not the tty refusal). A `grep -c` asserting 2 hits in repoint would close it. - Pre-existing, same class: `bin/rig:234` `read -r reply` in `uninstall_confirm` still dies silently on Ctrl-D at a real tty under `set -e` (headless is guarded at 228). CHANGELOG's "now all of rig does" slightly overclaims — fair game for a follow-up. — _automated review by `claude-bot-andresmgsl` · heavy-duty-review-bot_

grok-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-19 12:49:37 +00:00

grok-bot-andresmgsl (Migrated from github.com) left a comment

Verdict: Approve — I agree with this as-is.

Why

Matches the drill-measured failure exactly: bare read -rsp under set -e with non-tty stdin exits 1 with no last word. The cure is two-part and consistent with db.sh: refuse first ([ -t 0 ] || die …) naming the variable that unblocks an unattended run, and || die on the read so EOF on a real tty is equally loud.
All five prompt sites are covered — TS_AUTHKEY (bootstrap), RUNNER_TOKEN (install + repoint), RUNNER_REMOVE_TOKEN (remove + repoint) — including the two the issue undercounted. Remove/repoint correctly also name --local.
The sweep check no bare read -rsp remains under commands/ is the load-bearing regression guard; per-command greps name the variables so a deleted guard cannot ship green.

Checked

CI green (check / db-integration / reconcile / scope).
Variable names in the die messages match what each script actually reads.

No blockers.

**Verdict: Approve** — I agree with this as-is. ### Why - Matches the drill-measured failure exactly: bare `read -rsp` under `set -e` with non-tty stdin exits 1 with **no last word**. The cure is two-part and consistent with `db.sh`: refuse first (`[ -t 0 ] || die …`) naming the variable that unblocks an unattended run, and `|| die` on the `read` so EOF on a real tty is equally loud. - All five prompt sites are covered — `TS_AUTHKEY` (bootstrap), `RUNNER_TOKEN` (install + repoint), `RUNNER_REMOVE_TOKEN` (remove + repoint) — including the two the issue undercounted. Remove/repoint correctly also name `--local`. - The sweep check `no bare read -rsp remains` under `commands/` is the load-bearing regression guard; per-command greps name the variables so a deleted guard cannot ship green. ### Checked - CI green (check / db-integration / reconcile / scope). - Variable names in the die messages match what each script actually reads. No blockers.

codex-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-19 12:49:53 +00:00

codex-bot-andresmgsl (Migrated from github.com) left a comment

Verdict: I agree with everything and have no additional feedback.

All credential prompt sites now reject non-tty input with the exact unattended variable, guard interactive EOF, and preserve environment-variable bypasses. The broader sweep also covers both repoint prompts, and the current check and integration runs are green.

Verdict: I agree with everything and have no additional feedback. All credential prompt sites now reject non-tty input with the exact unattended variable, guard interactive EOF, and preserve environment-variable bypasses. The broader sweep also covers both repoint prompts, and the current check and integration runs are green.

dan-claude-bot commented

2026-07-19 12:56:04 +00:00

(Migrated from github.com)

@danmt — handing this one to you. Final state:

What it fixes: rig#42 — every interactive credential prompt (TS_AUTHKEY in bootstrap, RUNNER_TOKEN in runner install, RUNNER_REMOVE_TOKEN in runner remove, both tokens in runner repoint) was a bare read -rsp. Headless (CI, box exec, any script), read fails, set -e ends the run, and nothing prints — the release drill measured runner remove exiting 1 in total silence and a guest bootstrap whose log just stops. Each prompt now refuses first on no-tty, naming the exact variable that unblocks an unattended run (and --local where it applies), and every read is || die-guarded so EOF at a real prompt also gets a last word.

Review history: one round — all three bots approved with no changes requested. The sweep test (no bare read -rsp under commands/) is what caught runner repoint's two prompts, which the issue had only counted as three.

Verification: test/cli.sh 325/0 (3 grep-proof guard checks + the sweep) · test/release.sh 41/0 · shellcheck clean. The silent-death shape itself was reproduced live in the drill before the fix.

No open questions from the round. Ready for your review.

🤖 Generated with Claude Code

@danmt — handing this one to you. Final state: **What it fixes:** rig#42 — every interactive credential prompt (`TS_AUTHKEY` in `bootstrap`, `RUNNER_TOKEN` in `runner install`, `RUNNER_REMOVE_TOKEN` in `runner remove`, both tokens in `runner repoint`) was a bare `read -rsp`. Headless (CI, `box exec`, any script), `read` fails, `set -e` ends the run, and **nothing prints** — the release drill measured `runner remove` exiting 1 in total silence and a guest bootstrap whose log just stops. Each prompt now refuses first on no-tty, naming the exact variable that unblocks an unattended run (and `--local` where it applies), and every `read` is `|| die`-guarded so EOF at a real prompt also gets a last word. **Review history:** one round — all three bots approved with no changes requested. The sweep test (`no bare read -rsp under commands/`) is what caught `runner repoint`'s two prompts, which the issue had only counted as three. **Verification:** `test/cli.sh` 325/0 (3 grep-proof guard checks + the sweep) · `test/release.sh` 41/0 · shellcheck clean. The silent-death shape itself was reproduced live in the drill before the fix. No open questions from the round. Ready for your review. 🤖 Generated with [Claude Code](https://claude.com/claude-code)