diff --git a/install.d/interactive.sh b/install.d/interactive.sh index 595d805..0afb11a 100644 --- a/install.d/interactive.sh +++ b/install.d/interactive.sh @@ -109,7 +109,7 @@ EOF esac echo "" - read -p "Choose engine [1-5] (default: $DEFAULT_CHOICE): " ENGINE_CHOICE + read_prompt -p "Choose engine [1-5] (default: $DEFAULT_CHOICE): " ENGINE_CHOICE ENGINE_CHOICE=${ENGINE_CHOICE:-$DEFAULT_CHOICE} case "$ENGINE_CHOICE" in @@ -224,7 +224,7 @@ EOF local DEFAULT_BACKEND="2" fi - read -p "Choose backend [1-2] (default: $DEFAULT_BACKEND): " BACKEND_CHOICE + read_prompt -p "Choose backend [1-2] (default: $DEFAULT_BACKEND): " BACKEND_CHOICE BACKEND_CHOICE=${BACKEND_CHOICE:-$DEFAULT_BACKEND} if [[ "$BACKEND_CHOICE" == "1" ]]; then @@ -250,7 +250,7 @@ EOF echo " • Faster Whisper: faster-whisper-server --host 0.0.0.0 --port 8080" echo " • Any OpenAI-compatible speech API" echo "" - read -p "Enter remote server URL (or leave blank to set later): " REMOTE_API_URL_INPUT + read_prompt -p "Enter remote server URL (or leave blank to set later): " REMOTE_API_URL_INPUT if [ -n "$REMOTE_API_URL_INPUT" ]; then REMOTE_API_URL="$REMOTE_API_URL_INPUT" REMOTE_DISPLAY="$REMOTE_API_URL" @@ -276,7 +276,7 @@ EOF echo " • Models download automatically on first use" echo "" - read -p "Download models now? [1-2] (default: 1): " MODELS_CHOICE + read_prompt -p "Download models now? [1-2] (default: 1): " MODELS_CHOICE MODELS_CHOICE=${MODELS_CHOICE:-1} if [[ "$MODELS_CHOICE" == "2" ]]; then @@ -307,6 +307,6 @@ EOF echo " Models: $MODELS_DISPLAY" echo " Install Location: ${INSTALL_DIR:-\$HOME/.local/share/vocalinux}" echo "" - read -p "Press Enter to continue with installation, or Ctrl+C to cancel..." + read_prompt -p "Press Enter to continue with installation, or Ctrl+C to cancel..." echo "" } diff --git a/install.d/system_dependencies.sh b/install.d/system_dependencies.sh index c6b5a39..6d25f85 100644 --- a/install.d/system_dependencies.sh +++ b/install.d/system_dependencies.sh @@ -584,7 +584,7 @@ install_system_dependencies() { print_info " https://github.com/VocaHQ/vocalinux/wiki" print_info "" if [[ "$NON_INTERACTIVE" != "yes" ]]; then - read -p "Continue anyway? (y/n) " -n 1 -r + read_prompt -p "Continue anyway? (y/n) " -n 1 -r echo if [[ ! $REPLY =~ ^[Yy]$ ]]; then exit "$EXIT_USER_ABORT" diff --git a/install.sh b/install.sh index e24009a..ed9468b 100755 --- a/install.sh +++ b/install.sh @@ -42,6 +42,19 @@ command_exists() { command -v "$1" >/dev/null 2>&1 } +# Read one interactive answer. When the installer itself is piped to bash +# (`curl ... | bash`), stdin still carries the script text to bash, so prompts +# must pull from the controlling terminal explicitly. A global +# `exec < /dev/tty` would close the pipe mid-script instead: bash loses the +# rest of the installer and the download dies with "curl: (23)". +read_prompt() { + if [ -t 0 ]; then + read "$@" + else + read "$@" < /dev/tty + fi +} + # The installer must build its venv from the *system* Python: distro PyGObject # (python3-gi / python3-gobject) is compiled for that interpreter only. When the # script starts inside an activated virtualenv — a shell left in uv's .venv @@ -221,7 +234,7 @@ check_running_processes() { if [[ "$NON_INTERACTIVE" == "yes" ]]; then print_info "Non-interactive mode: stopping Vocalinux automatically..." else - read -p "Vocalinux must be stopped before installation. Kill running process(es)? (Y/n) " -n 1 -r + read_prompt -p "Vocalinux must be stopped before installation. Kill running process(es)? (Y/n) " -n 1 -r echo if [[ $REPLY =~ ^[Nn]$ ]]; then print_error "Cannot proceed with installation while Vocalinux is running." @@ -285,24 +298,22 @@ WHISPERCPP_BACKEND="" WHISPERCPP_ALREADY_INSTALLED="false" REMOTE_API_URL="" -# Detect if running non-interactively (e.g., via curl | bash) -# If stdin is a pipe but /dev/tty exists, redirect stdin so user input works normally. -# If no terminal is available at all (headless/CI), fall back to automatic mode. -if [ ! -t 0 ]; then - if [ -e /dev/tty ] && [ -r /dev/tty ]; then - if { true < /dev/tty; } 2>/dev/null; then - exec < /dev/tty - INTERACTIVE_MODE="ask" - else - AUTO_MODE="yes" - INTERACTIVE_MODE="no" - NON_INTERACTIVE="yes" - fi - else - AUTO_MODE="yes" - INTERACTIVE_MODE="no" - NON_INTERACTIVE="yes" - fi +# Detect if running non-interactively (e.g., via curl | bash). +# stdin keeps carrying the script text to bash, so it must stay on the pipe: +# redirecting fd 0 here closes the pipe mid-download (curl error 23) and makes +# bash read the rest of the installer from the terminal as commands. Prompts +# reach the user through /dev/tty instead (see read_prompt). With no terminal +# at all (headless/CI), fall back to automatic mode. +TERMINAL_PROMPTS="no" +if [ -t 0 ]; then + TERMINAL_PROMPTS="yes" +elif [ -e /dev/tty ] && [ -r /dev/tty ] && { true < /dev/tty; } 2>/dev/null; then + TERMINAL_PROMPTS="yes" + INTERACTIVE_MODE="ask" +else + AUTO_MODE="yes" + INTERACTIVE_MODE="no" + NON_INTERACTIVE="yes" fi while [[ $# -gt 0 ]]; do @@ -500,6 +511,15 @@ handoff_to_tagged_installer() { export VOCALINUX_REMOTE_INSTALL=yes export TMPDIR="$(dirname "$VOCALINUX_TMP_DIR")" rmdir "$VOCALINUX_TMP_DIR" + # A bootstrap read from a pipe (curl | bash) leaves its unread tail on + # stdin: bash hands the pipe to the tagged installer, which reads its + # commands from a file instead, so nothing drains it. A producer still + # streaming (the download is bigger than the pipe buffer) stays blocked + # for the whole install and reports curl error 23 when this fd closes. + # Drain to EOF first; a tty has no EOF to wait for and is left alone. + if [ ! -t 0 ]; then + cat >/dev/null 2>&1 || true + fi exec bash "$tagged_installer" "${INSTALLER_ARGS[@]}" "--venv-dir=$remote_venv" fi @@ -1257,7 +1277,7 @@ case "$DISTRO_FAMILY" in if [[ "$NON_INTERACTIVE" == "yes" ]]; then print_info "Non-interactive mode: continuing anyway..." else - read -p "Do you want to continue anyway? (y/n) " -n 1 -r + read_prompt -p "Do you want to continue anyway? (y/n) " -n 1 -r echo if [[ ! $REPLY =~ ^[Yy]$ ]]; then exit "$EXIT_USER_ABORT" @@ -1274,7 +1294,7 @@ if [[ "$INTERACTIVE_MODE" == "ask" ]]; then echo " 1. Interactive (recommended) - guided setup with recommendations" echo " 2. Automatic - quick install with defaults (whisper.cpp)" echo "" - read -p "Choose mode [1-2] (default: 1): " MODE_CHOICE + read_prompt -p "Choose mode [1-2] (default: 1): " MODE_CHOICE MODE_CHOICE=${MODE_CHOICE:-1} if [[ "$MODE_CHOICE" == "2" ]]; then @@ -1291,7 +1311,7 @@ fi # Run interactive installation if selected if [[ "$INTERACTIVE_MODE" == "yes" ]]; then # Check if we have a TTY (required for interactive mode) - if [ ! -t 0 ]; then + if [[ "$TERMINAL_PROMPTS" != "yes" ]]; then print_error "Interactive mode requires a terminal (TTY)." print_error "Download and run the installer directly from a terminal:" print_error " curl -fsSL https://raw.githubusercontent.com/VocaHQ/vocalinux/main/install.sh -o /tmp/vl.sh && bash /tmp/vl.sh" @@ -1467,7 +1487,7 @@ setup_virtual_environment() { source "$VENV_DIR/bin/activate" || { print_error "Failed to activate virtual environment"; exit "$EXIT_MISSING_DEPS"; } return 0 else - read -p "Do you want to recreate it? (y/n) " -n 1 -r + read_prompt -p "Do you want to recreate it? (y/n) " -n 1 -r echo if [[ $REPLY =~ ^[Yy]$ ]]; then print_info "Removing existing virtual environment..." @@ -1819,7 +1839,7 @@ should_rebuild_whispercpp() { return 0 fi - read -p "Rebuild/reinstall pywhispercpp? This can take several minutes. (y/N) " -n 1 -r + read_prompt -p "Rebuild/reinstall pywhispercpp? This can take several minutes. (y/N) " -n 1 -r echo if [[ $REPLY =~ ^[Yy]$ ]]; then return 0 diff --git a/scripts/install-remote-test.sh b/scripts/install-remote-test.sh index 9ed4195..d399f94 100755 --- a/scripts/install-remote-test.sh +++ b/scripts/install-remote-test.sh @@ -20,10 +20,11 @@ # guardrail allows the bootstrap to come from main, and this commit's copy is # what a merged PR would serve anyway. # -# Flags the installer sees: --auto (no TTY), --skip-models (a model is 40-75 MB +# Flags the installer sees: --auto, --skip-models (a model is 40-75 MB # per run and the download path is checksum-verified elsewhere), --tag (the # selection under test). Not --skip-system-deps: the per-distro package -# installation is the point. +# installation is the point. Run 1 is headless; run 2 pipes the bootstrap in +# under a `script` pty so the real `curl | bash` fd-0 shape stays covered. # # Fail closed: after the install, the clone's tree must equal HEAD's tree — # the #701 guardrail that installer, sourced modules and requirement exports @@ -146,6 +147,49 @@ run_remote_install() { su - "$INSTALL_USER" -c "cd '$INSTALL_HOME' && VOCALINUX_REPO_URL='file://$MIRROR' bash '$BOOTSTRAP' --auto --skip-models '--tag=$GATE_TAG'" } +# The user's literal `curl | bash`: installer code reaches bash on a pipe while +# a controlling terminal exists. `script` allocates that pty — a saved file on +# stdin or a headless pipe takes the headless --auto branch instead, which is +# exactly how the fd-0 redirect regression in #933 slipped through. bsdutils / +# util-linux `script` is in the base package set of every gated distro. +command -v script >/dev/null 2>&1 || fail "script(1) not found; needed to give the piped run a terminal" +# +# The producer half matters too: a `cat` of the whole file finishes inside the +# pipe buffer long before bash reaches the tagged-installer handoff, so it can +# never be killed by a consumer that abandons the pipe the way a still- +# streaming curl can (the error 23 the #933 fix still had to drain for). The +# helper below trickles the bootstrap so it is mid-write at the handoff, and +# pipefail makes its exit status count — a consumer that closes the pipe early +# kills it with SIGPIPE and fails the run instead of hiding behind bash's own +# exit status. +PIPED_FEED=/tmp/vl-piped-feed.sh +cat >"$PIPED_FEED" <<'EOF' +#!/usr/bin/env bash +# Usage: vl-piped-feed.sh BOOTSTRAP [installer args...] +set -o pipefail +bootstrap=$1; shift +# ~16 KiB/s: still writing (and soon blocked on the full pipe buffer) when the +# bootstrap reaches the tagged-installer handoff, the way curl is mid-download. +drip() { + local file=$1 chunk rc=0 + split -b 8192 "$file" /tmp/vl-feed- + for chunk in /tmp/vl-feed-*; do + # A cat killed by SIGPIPE means the consumer closed the pipe early — the + # download failure this arm exists to catch. Keep the status through the + # cleanup so pipefail above sees it. + cat "$chunk" || { rc=$?; break; } + sleep 0.5 + done + rm -f /tmp/vl-feed-* + return "$rc" +} +drip "$bootstrap" | bash -s -- "$@" +EOF +chmod 0755 "$PIPED_FEED" +run_remote_install_piped() { + su - "$INSTALL_USER" -c "cd '$INSTALL_HOME' && VOCALINUX_REPO_URL='file://$MIRROR' script -qec \"'$PIPED_FEED' '$BOOTSTRAP' --auto --skip-models '--tag=$GATE_TAG'\" /dev/null" +} + assert_remote_install() { echo "== Assert: the clone is exactly the selected tag ==" [ -d "$CLONE_DIR/.git" ] || fail "remote install left no clone at $CLONE_DIR" @@ -256,9 +300,11 @@ run_remote_install || fail "the remote install exited non-zero on a fresh clone" echo " install took $(( $(date +%s) - START ))s" assert_remote_install -echo "== Run 2: re-run over the existing clone (fetch + reset) ==" +echo "== Run 2: re-run over the existing clone (fetch + reset), piped under a terminal ==" # The second bootstrap sees $CLONE_DIR/.git and updates it instead of cloning: -# the arm a user re-running curl|bash takes on every release after the first. +# the arm a user re-running curl|bash takes on every release after the first — +# served here literally through `cat | bash` on a pty so the terminal-backed +# pipe keeps regression coverage (#933). # Move the clone off the tag and drop its local tag ref first: a run that only # had to notice the right commit was already checked out would pass even if # the fetch and reset were both no-ops. @@ -273,7 +319,7 @@ su - "$INSTALL_USER" -c "git -C '$CLONE_DIR' reset --hard '$AWAY_SHA' \ || fail "the clone did not move off $GATE_TAG; the re-run proves nothing" [ -z "$(git -C "$CLONE_DIR" tag -l "$GATE_TAG")" ] \ || fail "the clone kept refs/tags/$GATE_TAG; the re-run proves nothing" -run_remote_install || fail "the remote install exited non-zero over the existing clone" +run_remote_install_piped || fail "the terminal-backed piped install exited non-zero over the existing clone" grep -l "Updating existing clone" "$INSTALL_HOME"/.local/state/vocalinux/install-*.log >/dev/null 2>&1 \ || fail "no install log shows 'Updating existing clone'; the fetch arm did not run" assert_remote_install diff --git a/tests/test_cross_distro_compatibility.py b/tests/test_cross_distro_compatibility.py index b1efb3a..82b7b90 100644 --- a/tests/test_cross_distro_compatibility.py +++ b/tests/test_cross_distro_compatibility.py @@ -198,7 +198,14 @@ class TestCrossDistroCompatibility: def test_noninteractive_tty_fallback_does_not_abort(self, install_sh_content): """Test that missing controlling TTY falls back to non-interactive mode.""" assert "{ true < /dev/tty; } 2>/dev/null" in install_sh_content - assert "exec < /dev/tty" in install_sh_content + # stdin must stay on the pipe while bash reads the script from it; + # prompts reach the terminal through read_prompt instead (#933). + code = "\n".join( + line for line in install_sh_content.splitlines() if not line.lstrip().startswith("#") + ) + assert "exec < /dev/tty" not in code + assert "exec 0<" not in code + assert "read_prompt" in install_sh_content assert 'NON_INTERACTIVE="yes"' in install_sh_content def test_ci_workflow_uses_dynamic_detection(self, workflow_content): diff --git a/tests/test_installer_tty_prompts.py b/tests/test_installer_tty_prompts.py new file mode 100644 index 0000000..e97b148 --- /dev/null +++ b/tests/test_installer_tty_prompts.py @@ -0,0 +1,156 @@ +"""Regression guards for the curl|bash terminal-prompt path (issue #933). + +`curl ... | bash` leaves bash reading the installer itself from stdin, so the +script must never redirect fd 0 away from the pipe: the old `exec < /dev/tty` +closed the pipe mid-download (curl error 23) and made bash read the rest of +the installer from the terminal as commands. Interactive prompts now pull +from /dev/tty per-read through `read_prompt`, and `TERMINAL_PROMPTS` records +whether a controlling terminal exists instead of testing `-t 0` on stdin. + +The behavioral tests run the real install.sh under a pseudo-terminal exactly +the way a curl user runs it — script text on a pipe, /dev/tty on a terminal. +A saved file or a headless pipe exercises different branches and proves +nothing about this one. +""" + +import os +import re +import shlex +import shutil +import subprocess +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parents[1] +INSTALLER = REPO_ROOT / "install.sh" +MODULE_DIR = REPO_ROOT / "install.d" + +#: A `read -p` (optionally `read_prompt -p`) anywhere in the installer tree. +PROMPT_LINE = re.compile(r"^\s*(read|read_prompt)\s+-p\s") + +SCRIPT = shutil.which("script") +needs_pty = pytest.mark.skipif( + SCRIPT is None, + reason="script(1) is needed to give a piped installer a controlling terminal", +) + + +def _installer_sources() -> dict[Path, str]: + sources = {INSTALLER: INSTALLER.read_text(encoding="utf-8")} + for module in MODULE_DIR.glob("*.sh"): + sources[module] = module.read_text(encoding="utf-8") + return sources + + +def _pty_env(tmp_path: Path) -> dict: + """Isolate the installer's log and scratch writes inside the test tmp dir.""" + env = os.environ.copy() + env["HOME"] = str(tmp_path) + env["XDG_STATE_HOME"] = str(tmp_path / ".local" / "state") + env["TMPDIR"] = str(tmp_path) + return env + + +def test_stdin_is_never_redirected_off_the_pipe() -> None: + """No fd-0 redirect may run while bash still reads the script from stdin.""" + for path, source in _installer_sources().items(): + for line in source.splitlines(): + stripped = line.strip() + if stripped.startswith("#"): + continue + assert ( + "exec <" not in stripped + ), f"{path.name}: `{stripped}` steals stdin from the piped script" + assert "exec 0<" not in stripped + + +def test_every_interactive_read_uses_read_prompt() -> None: + """All `read -p` sites go through read_prompt so answers come from /dev/tty.""" + source = INSTALLER.read_text(encoding="utf-8") + helper = re.search(r"read_prompt\(\)\s*\{(.*?)\}", source, re.S) + assert helper is not None, "install.sh does not define read_prompt" + assert "< /dev/tty" in helper.group(1), "read_prompt does not read /dev/tty" + + for path, source in _installer_sources().items(): + bare = [line for line in source.splitlines() if PROMPT_LINE.match(line)] + leftovers = [line.strip() for line in bare if "read_prompt" not in line] + assert not leftovers, f"{path.name} prompts straight off stdin: {leftovers}" + + +def test_terminal_probe_selects_ask_or_auto() -> None: + """A reachable /dev/tty asks for a mode; no terminal falls back to --auto.""" + source = INSTALLER.read_text(encoding="utf-8") + assert "{ true < /dev/tty; } 2>/dev/null" in source + assert 'INTERACTIVE_MODE="ask"' in source + assert 'NON_INTERACTIVE="yes"' in source + # Interactive mode is gated on a reachable terminal, not on a tty stdin: + # under curl|bash stdin stays the pipe even while prompts work. + assert '"$TERMINAL_PROMPTS" != "yes"' in source + + +@needs_pty +@pytest.mark.timeout(60) +def test_piped_installer_reaches_past_the_stdin_probe(tmp_path: Path) -> None: + """`cat install.sh | bash -s -- --help` under a pty prints usage. + + On the broken version bash never got past the early stdin redirect: it + read the rest of the script from the terminal instead of the pipe, so no + installer output appeared at all. + """ + transcript = tmp_path / "help.typescript" + inner = f"cat {shlex.quote(str(INSTALLER))} | bash -s -- --help" + subprocess.run( + [SCRIPT, "-ec", f"bash -c {shlex.quote(inner)}", str(transcript)], + stdin=subprocess.DEVNULL, + capture_output=True, + timeout=45, + check=False, + ) + output = transcript.read_text(encoding="utf-8", errors="replace") + assert "Vocalinux Installer" in output + assert "Usage:" in output + + +@needs_pty +@pytest.mark.timeout(90) +def test_piped_installer_prompts_answer_on_the_terminal(tmp_path: Path) -> None: + """Prompts read /dev/tty while bash keeps reading the script from the pipe. + + Feed "1" (interactive) early — pty input queues until the mode prompt + reads it — then Ctrl+C once the engine menu is up. Reaching "Choose + engine" proves the tty answer was accepted while the script still flowed + through the pipe. --tag skips the release-tag lookup so a slow GitHub + cannot eat the window before the first prompt appears. + """ + transcript = tmp_path / "interactive.typescript" + feeder = tmp_path / "feed-input.sh" + feeder.write_text( + "#!/bin/sh\n" + # Queued until the "Choose mode" prompt reads /dev/tty. + "sleep 1; printf '1\\n'\n" + # Interrupt only once the engine menu is on screen: a blind delay + # loses to a slow runner and Ctrl+C's a healthy installer mid-prompt. + "for _ in $(seq 1 60); do\n" + f" grep -q 'Choose engine' {shlex.quote(str(transcript))} 2>/dev/null && break\n" + " sleep 1\n" + "done\n" + "printf '\\003'\n", + encoding="utf-8", + ) + feeder.chmod(0o755) + subprocess.run( + f"{shlex.quote(str(feeder))} | {shlex.quote(str(SCRIPT))} -ec " + + shlex.quote(f"cat {shlex.quote(str(INSTALLER))} | bash -s -- --tag=v0.0.0-test") + + f" {shlex.quote(str(transcript))}", + shell=True, + stdin=subprocess.DEVNULL, + capture_output=True, + timeout=75, + check=False, + cwd=REPO_ROOT, + env=_pty_env(tmp_path), + ) + output = transcript.read_text(encoding="utf-8", errors="replace") + assert "Choose mode" in output + assert "Choose engine" in output