fix(install): read prompt from /dev/tty when stdin is piped#42
Merged
Conversation
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]
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]
Contributor
Author
Review-feedback responseRan 4 internal reviewers (code-first, testing, skeptical, big-picture) and Codex CLI against the initial commit. Summary of how each finding was handled in the follow-up commit Addressed
Deferred (worth doing, separate PR scope)
Manual verificationRe-ran the pty harness over five scenarios (all pass):
Companion PR against [AI-assisted - Claude] |
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Problem
The Linux install script advertised on the quickstart page is invoked via
curl -fsSL https://freenet.org/install.sh | sh. When piped, stdin is thecurl output (script bytes), not the user's terminal, so
read -r responseat the service-install prompt silently consumes nothing and the prompt
ignores keyboard input.
Reported by Ivvor on Matrix:
Approach
A previous fix (
eff317ae, Dec 2025) wroteread -r response < /dev/tty,but PRs #36 / #37 ("resync install.sh + uninstall.sh with freenet-core")
overwrote the file with the freenet-core copy and dropped the override.
This change reapplies the fix and hardens it:
readdirectly (coverssh install.sh).read < /dev/tty(coverscurl | sh).aborting under
set -eu(covers CI /setsid).{ true </dev/tty; } 2>/dev/nullis used instead of[ -r /dev/tty ],because the
-raccess check can succeed when the process has nocontrolling tty and the subsequent open fails with ENXIO.
Testing
Smoke-tested with a small harness that extracts the prompt block:
curl | shunder a real pty (script -c): stdin is piped,/dev/tty present. Script reads
yfrom /dev/tty and proceeds withservice install. ✅
setsid ... </dev/null(no controlling tty): script prints"Non-interactive shell detected; skipping service installation prompt"
and continues without erroring. ✅
sh -nparses cleanly.Follow-up
freenet-core/scripts/install.shcarries the identical bug (the two filesare bit-identical). A matching PR will land there next so the next resync
doesn't regress this file again.
[AI-assisted - Claude]