Commit 9a23f46
authored
fix(install): read prompt from /dev/tty when stdin is piped (#42)
* fix(install): read prompt from /dev/tty when stdin is piped
When the install script is run via the documented `curl ... | sh`
pattern, stdin is the pipe carrying script bytes, so `read -r response`
cannot capture keystrokes and the service-install prompt silently
ignores keyboard input (reported by Ivvor on Matrix).
A previous fix (eff317ae, Dec 2025) addressed this with `< /dev/tty`,
but PRs #36 / #37 resynced this file from freenet-core and dropped the
override. Reapply the fix and harden it for environments without a
controlling terminal.
The new logic:
- If stdin is a TTY, read normally (covers `sh install.sh`).
- Else, probe /dev/tty by actually opening it (`{ true </dev/tty; }`)
rather than relying on `[ -r /dev/tty ]`, which can return true even
when the open later fails. If /dev/tty is openable, read from it
(covers `curl | sh`).
- Otherwise, skip the prompt rather than aborting under `set -eu`
(covers truly non-interactive shells, e.g. CI or `setsid`).
Verified via a pty harness: `curl | sh` simulation under `script(1)`
reads "y" from /dev/tty and proceeds; `setsid ... </dev/null` skips
the prompt and prints follow-up instructions.
Note: freenet-core/scripts/install.sh has the identical bug; a matching
fix will follow there to keep the resync in sync.
[AI-assisted - Claude]
* fix(install): address review feedback - $0 detection, EOF guard
Five-perspective review on PR #42 surfaced three issues with the
initial fix:
- Codex P2: discriminating only on `[ -t 0 ]` regresses
`printf 'y' | sh install.sh` automation patterns. When stdin is
piped but the script ran from a file, the user's answer is on
stdin, not /dev/tty.
- Skeptical #1: `read -r response` returning EOF (Ctrl-D, closed
stdin) under `set -eu` aborts the script.
- Skeptical #10: comment used an em-dash, which Ian's writing-style
rule rejects.
Redesigned the dispatch around `$0`: when sh runs a script file
(file-execution), $0 is the script path; when sh reads its own
source from stdin (`curl | sh`), $0 is the shell name. The case
arm tests for known shell names and only redirects to /dev/tty in
that branch. The file-execution arm reads from stdin as before, so
piped automation still works.
Both `read` calls now have `|| response=""` so EOF leaves the
default-N case path intact rather than aborting.
Also (lower priority feedback applied):
- Skipped path now mentions FREENET_NO_SERVICE=1 as the documented
way to bypass the prompt non-interactively (Skeptical #2/#3).
- Removed the redundant "Non-interactive shell detected" info line
(Skeptical #7); the existing `*)` case arm already prints
"Skipping service installation" plus remediation guidance.
- Added a NOTE block at the top of the file pointing future
resync agents at the freenet-core sibling and explaining why
the prompt block must not be flattened back to a plain `read`
(Big-picture #5).
Re-tested via the same pty harness across five scenarios:
1. `curl | sh` under pty: reads 'y' from /dev/tty, installs.
2. `sh install.sh` interactive: reads from stdin.
3. `printf 'y' | sh install.sh`: reads from piped stdin (regression
test for Codex's concern).
4. `setsid ... </dev/null`: skips, prints FREENET_NO_SERVICE hint.
5. `sh install.sh </dev/null`: handles EOF without aborting.
All five pass.
[AI-assisted - Claude]1 parent c034ae6 commit 9a23f46
1 file changed
Lines changed: 47 additions & 3 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
9 | 9 | | |
10 | 10 | | |
11 | 11 | | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
12 | 20 | | |
13 | 21 | | |
14 | 22 | | |
| |||
409 | 417 | | |
410 | 418 | | |
411 | 419 | | |
412 | | - | |
| 420 | + | |
| 421 | + | |
| 422 | + | |
| 423 | + | |
| 424 | + | |
| 425 | + | |
| 426 | + | |
| 427 | + | |
| 428 | + | |
| 429 | + | |
| 430 | + | |
| 431 | + | |
| 432 | + | |
| 433 | + | |
| 434 | + | |
| 435 | + | |
| 436 | + | |
| 437 | + | |
| 438 | + | |
| 439 | + | |
| 440 | + | |
| 441 | + | |
413 | 442 | | |
414 | 443 | | |
415 | | - | |
416 | | - | |
| 444 | + | |
| 445 | + | |
| 446 | + | |
| 447 | + | |
| 448 | + | |
| 449 | + | |
| 450 | + | |
| 451 | + | |
| 452 | + | |
| 453 | + | |
| 454 | + | |
| 455 | + | |
| 456 | + | |
| 457 | + | |
| 458 | + | |
417 | 459 | | |
418 | 460 | | |
419 | 461 | | |
| |||
427 | 469 | | |
428 | 470 | | |
429 | 471 | | |
| 472 | + | |
| 473 | + | |
430 | 474 | | |
431 | 475 | | |
432 | 476 | | |
| |||
0 commit comments