Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions scripts/install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -729,9 +729,12 @@ install_system_packages() {
return 0
fi
fi
elif [ -e /dev/tty ]; then
elif (: </dev/tty) 2>/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."
Expand Down Expand Up @@ -1330,7 +1333,12 @@ run_setup_wizard() {
# The setup wizard reads from /dev/tty, so it works even when the
# install script itself is piped (curl | bash). Only skip if no
# terminal is available at all (e.g. Docker build, CI).
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, so the wizard would proceed and
# then crash on `< /dev/tty` below.
if ! (: </dev/tty) 2>/dev/null; then
log_info "Setup wizard skipped (no terminal available). Run 'hermes setup' after install."
return 0
fi
Expand Down Expand Up @@ -1392,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/tty) 2>/dev/null; then
log_info "Gateway setup skipped (no terminal available). Run 'hermes gateway install' later."
return 0
fi
Expand Down
91 changes: 91 additions & 0 deletions tests/test_install_sh_setup_wizard_tty_probe.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
"""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 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 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

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_function_body(name: str) -> str:
"""Return the body of ``<name>()`` as a single string.

Anchored to ``<name>()`` and a top-of-line ``}`` so the helper keeps
working if neighbouring functions are renamed.
"""
text = INSTALL_SH.read_text()
match = re.search(
rf"^{re.escape(name)}\(\)\s*\{{\s*\n(?P<body>.*?)^\}}",
text,
re.MULTILINE | re.DOTALL,
)
assert match is not None, f"{name}() not found in scripts/install.sh"
return match["body"]


@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_function_body(fn_name)
# Cover ``[ -e /dev/tty ]``, ``[ -e "/dev/tty" ]``, ``test -e /dev/tty``
# and friends, with arbitrary surrounding whitespace.
pattern = re.compile(
r"""(
\[\s*-e\s+["']?/dev/tty["']?\s*\]
|
\btest\s+-e\s+["']?/dev/tty["']?
)""",
re.VERBOSE,
)
match = pattern.search(body)
assert match is None, (
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/tty) 2>/dev/null` or `exec 3</dev/tty`) so the skip "
"kicks in before the function tries to read from /dev/tty. "
"See #16746."
)


@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``/``if !``/``elif`` whose condition opens ``/dev/tty`` for
input counts: ``(: </dev/tty)``, ``exec 3</dev/tty``,
``{ exec 3</dev/tty; }``, etc. Asserting the higher-level invariant
rather than a specific spelling so equivalent refactors stay green.
"""
body = _extract_function_body(fn_name)
gate = re.compile(
r"^\s*(?:if|elif)\s+!?\s*[^\n]*<\s*/dev/tty[^\n]*;\s*then",
re.MULTILINE,
)
assert gate.search(body), (
f"{fn_name} must gate on an open-based probe of /dev/tty "
"(an `if`/`if !`/`elif` whose test redirects stdin from /dev/tty), "
"not a mere existence check. See #16746."
)
Loading