From 3d8be2c617e07db0150471353ee85fd6dec32fd4 Mon Sep 17 00:00:00 2001 From: teknium1 Date: Tue, 28 Apr 2026 06:43:53 -0700 Subject: [PATCH] fix(install): widen /dev/tty open-probe to sibling gates (#16746) The contributor's PR (#16750) scoped the fix to run_setup_wizard() and explicitly punted the two sibling sites. Both have the identical [ -e /dev/tty ] pattern followed by a < /dev/tty redirect and crash in Docker the same way: - scripts/install.sh:732 install_system_packages() -- apt sudo prompt fallback. sudo ... < /dev/tty dies with the same ENXIO. - scripts/install.sh:1395 maybe_start_gateway() -- gateway-install gate, same function path as the wizard reproducer. Fix both with the same (: /dev/null probe, and parametrize the regression test over all three gated functions so any future regression is caught regardless of which site breaks. --- scripts/install.sh | 10 ++- .../test_install_sh_setup_wizard_tty_probe.py | 66 ++++++++++++------- 2 files changed, 49 insertions(+), 27 deletions(-) diff --git a/scripts/install.sh b/scripts/install.sh index 53d8e081a8..21aa122a8f 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -729,9 +729,12 @@ install_system_packages() { return 0 fi fi - elif [ -e /dev/tty ]; then + elif (: /dev/null; then # Non-interactive (e.g. curl | bash) but a terminal is available. # Read the prompt from /dev/tty (same approach the setup wizard uses). + # Probe by actually opening /dev/tty: a bare existence test passes + # in Docker builds where the device node is in the mount namespace + # but opening fails with ENXIO. See #16746. echo "" log_info "sudo is needed ONLY to install optional system packages (${pkgs[*]}) via your package manager." log_info "Hermes Agent itself does not require or retain root access." @@ -1397,7 +1400,10 @@ maybe_start_gateway() { fi fi - if ! [ -e /dev/tty ]; then + # Probe by actually opening /dev/tty: a bare existence test passes + # in Docker builds where the device node is in the mount namespace + # but opening fails with ENXIO. See #16746. + if ! (: /dev/null; then log_info "Gateway setup skipped (no terminal available). Run 'hermes gateway install' later." return 0 fi diff --git a/tests/test_install_sh_setup_wizard_tty_probe.py b/tests/test_install_sh_setup_wizard_tty_probe.py index 0f2a15fc58..a9f8a26e75 100644 --- a/tests/test_install_sh_setup_wizard_tty_probe.py +++ b/tests/test_install_sh_setup_wizard_tty_probe.py @@ -1,11 +1,16 @@ -"""Regression for #16746: setup-wizard tty gate must actually open /dev/tty. +"""Regression for #16746: install.sh /dev/tty gates must actually open /dev/tty. In a Docker build, ``/dev/tty`` exists as a device node (so a bare ``-e`` existence test returns true) but opening it fails with ``ENXIO: No such -device or address``. Under the old gate the wizard proceeded past the "no +device or address``. Under the old gates the script proceeded past the "no terminal available" skip and then crashed on the ``< /dev/tty`` redirect a -few lines later, aborting the entire image build. The fix replaces the -existence check with an open-based probe so the skip kicks in correctly. +few lines later, aborting the entire image build. The fix replaces every +existence-based check that guards a subsequent ``< /dev/tty`` redirect with +an open-based probe so the skip kicks in correctly. + +This module covers all three affected functions: ``run_setup_wizard()`` +(the reproducer in #16746), ``install_system_packages()`` (the apt sudo +prompt fallback), and ``maybe_start_gateway()`` (the gateway-install gate). """ from __future__ import annotations @@ -13,29 +18,36 @@ from __future__ import annotations import re from pathlib import Path +import pytest + REPO_ROOT = Path(__file__).resolve().parent.parent INSTALL_SH = REPO_ROOT / "scripts" / "install.sh" +# Every function in scripts/install.sh that previously gated on a bare +# ``[ -e /dev/tty ]`` check before redirecting stdin from ``/dev/tty``. +GATED_FUNCTIONS = ("run_setup_wizard", "install_system_packages", "maybe_start_gateway") -def _extract_run_setup_wizard() -> str: - """Return the body of ``run_setup_wizard()`` as a single string. - Anchored to ``run_setup_wizard()`` and a top-of-line ``}`` so the helper - keeps working if neighbouring functions are renamed. +def _extract_function_body(name: str) -> str: + """Return the body of ``()`` as a single string. + + Anchored to ``()`` and a top-of-line ``}`` so the helper keeps + working if neighbouring functions are renamed. """ text = INSTALL_SH.read_text() match = re.search( - r"^run_setup_wizard\(\)\s*\{\s*\n(?P.*?)^\}", + rf"^{re.escape(name)}\(\)\s*\{{\s*\n(?P.*?)^\}}", text, re.MULTILINE | re.DOTALL, ) - assert match is not None, "run_setup_wizard() not found in scripts/install.sh" + assert match is not None, f"{name}() not found in scripts/install.sh" return match["body"] -def test_run_setup_wizard_does_not_use_existence_only_tty_check() -> None: +@pytest.mark.parametrize("fn_name", GATED_FUNCTIONS) +def test_tty_gate_does_not_use_existence_only_check(fn_name: str) -> None: """The bare ``-e`` test is the bug — no spelling of it should remain.""" - body = _extract_run_setup_wizard() + body = _extract_function_body(fn_name) # Cover ``[ -e /dev/tty ]``, ``[ -e "/dev/tty" ]``, ``test -e /dev/tty`` # and friends, with arbitrary surrounding whitespace. pattern = re.compile( @@ -48,28 +60,32 @@ def test_run_setup_wizard_does_not_use_existence_only_tty_check() -> None: ) match = pattern.search(body) assert match is None, ( - "run_setup_wizard contains an existence-only check on /dev/tty " + f"{fn_name} contains an existence-only check on /dev/tty " f"({match.group(0)!r}). Bare `-e` tests pass in Docker builds " "where the device node is in the mount namespace but cannot be " "opened (ENXIO). Use an open-based probe (e.g. " "`(: /dev/null` or `exec 3 None: +@pytest.mark.parametrize("fn_name", GATED_FUNCTIONS) +def test_tty_gate_uses_open_based_probe(fn_name: str) -> None: """The gate must actually attempt to open ``/dev/tty``. - Any ``if !`` (or ``if``) whose condition opens ``/dev/tty`` for input - counts: ``(: