chore: resync install.sh + uninstall.sh with freenet-core post-review#37
Merged
Conversation
PR #36 merged an early snapshot of these two scripts. After that PR opened, freenet-core#3906 picked up several review fixes that never made it into the freenet/web copy: - uninstall.sh: Linux paths lowercased to match what `directories` actually creates (`~/.local/share/freenet` etc.), HOME="" and HOME="/root" safety guards added, absolute /usr/bin/id used so a PATH-shimmed id can't bypass the root check, FREENET_INSTALL_DIR now honored, em-dashes replaced with hyphens, docstring corrected to drop the "current exe's dir" claim that was never implemented. - install.sh: a trailing em-dash in the post-install sudo warning replaced with an ASCII hyphen for house-style consistency. This PR is a straight copy from `freenet-core/scripts/` at commit 1b7c8855 so the two sources stay in sync. [AI-assisted - Claude]
sanity
added a commit
that referenced
this pull request
Apr 30, 2026
* 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]
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
#36 merged an early snapshot of
static/install.shandstatic/uninstall.sh. After #36 opened, freenet-core#3906 picked up several review fixes (from code-first, skeptical, and codex) that never made it into the freenet/web copies — socurl https://freenet.org/uninstall.sh | shis currently serving the pre-review version with known bugs.What's missed in the current live copy
uninstall.sh:
~/.local/share/Freenet).directoriescrate on Linux lowercases the application name, so the actual paths are lowercase. Purge silently removed nothing.rm -rf "${HOME}/..."with emptyHOMEturned into an absolute-from-root path. HOME=/rootalso deleted the wrong user's data.$(id -u)via PATH could be spoofed by a user-writable~/.local/bin/id.FREENET_INSTALL_DIRhonored by install.sh was ignored by uninstall.sh.install.sh:
Approach
Straight copy from
freenet-core/scripts/uninstall.shandinstall.shat commit 1b7c8855 (freenet-core#3906 head after all review fixes). No freenet/web-specific changes on top.Testing
sh -nclean on both files.scripts/test-uninstall-sh.shin freenet-core#3906 (13 smoke-test cases, all passing).Related
[AI-assisted - Claude]