diff --git a/hermes_cli/auth.py b/hermes_cli/auth.py index 0ecd267513b6..07bb2ceb74fe 100644 --- a/hermes_cli/auth.py +++ b/hermes_cli/auth.py @@ -3431,168 +3431,38 @@ def login_spotify_command(args) -> None: # SSH / remote session detection # ============================================================================= -def _is_remote_session() -> bool: - """Detect environments where loopback OAuth can't reach the local browser. - - Historically only SSH was checked, but #26923 surfaced that - **browser-only remote consoles** (GCP Cloud Shell, GitHub - Codespaces, AWS EC2 Instance Connect, Gitpod, Replit, etc.) hit - the exact same problem — the user has a browser on their laptop - but the loopback listener is bound on the remote VM that the - laptop's browser can't reach. These environments typically don't - set ``SSH_CLIENT`` / ``SSH_TTY``, so the SSH-only check left - them with no guidance and no fallback. - """ - if os.getenv("SSH_CLIENT") or os.getenv("SSH_TTY"): - return True - # Browser-only remote IDEs / cloud shells. Keep this list narrow - # (well-known, documented env vars set by the host platform) so - # we don't falsely trip on a developer's local shell. - for var in ( - "CLOUD_SHELL", # GCP Cloud Shell - "CODESPACES", # GitHub Codespaces - "CODESPACE_NAME", # GitHub Codespaces (alt) - "GITPOD_WORKSPACE_ID", # Gitpod - "REPL_ID", # Replit - "STACKBLITZ", # StackBlitz - ): - if os.getenv(var): - return True - return False - - -# Console/text-mode browsers that ``webbrowser`` will happily launch INSIDE -# the terminal. Opening one of these is worse than not opening anything — -# it hijacks the user's TTY with an unusable text browser (the xAI OAuth -# "Account Management" page rendered in w3m, reported May 2026) instead of -# letting them copy the URL to a real browser. When the resolved browser is -# one of these we refuse to auto-open and fall back to the print-the-URL -# path, same as a remote session. -_CONSOLE_BROWSER_NAMES: FrozenSet[str] = frozenset( - { - "w3m", - "lynx", - "links", - "links2", - "elinks", - "www-browser", - "browsh", # TUI browser — still hijacks the terminal - } +# SSH / remote-session detection and graphical-browser helpers moved to +# auth_browser_helpers.py (epic #78647, target #78637). Every name is +# re-exported here so the monolith namespace keeps resolving unchanged: +# bare-name call sites inside this module, downstream +# `from hermes_cli.auth import ...` importers, and test monkeypatches all +# go through this module. The eager import binds the names into this +# module's globals; the PEP 562 __getattr__ below is a cycle-safe +# fallback for attribute access. +_AUTH_BROWSER_HELPER_EXPORTS = ( + "_CONSOLE_BROWSER_NAMES", + "_can_open_graphical_browser", + "_is_remote_session", + "_print_loopback_ssh_hint", + "_ssh_user_at_host", ) - -def _can_open_graphical_browser() -> bool: - """Return True only when a *graphical* browser is likely to open. - - ``webbrowser.open()`` resolves to whatever the platform offers, and on a - headless / CLI-only Linux box with no GUI browser installed that is often - a text-mode browser (w3m/lynx/links) which launches inside the terminal - and takes over the user's session. This guard distinguishes "a real - windowed browser will pop up" from "a console browser will hijack the - TTY", so callers can fall back to printing the URL instead. - - Heuristics: - * Respect ``$BROWSER`` — if it names a known console browser, refuse. - * On Linux, require a display server (``$DISPLAY`` / ``$WAYLAND_DISPLAY``) - unless ``$BROWSER`` points at something graphical; no display server - almost always means no GUI browser. - * Ask ``webbrowser.get()`` what it resolved to and refuse when the - underlying command is a known console browser. - * macOS and Windows always have a usable default GUI browser. - """ - import webbrowser as _webbrowser - - def _names_console_browser(value: str) -> bool: - token = value.strip().split()[0] if value.strip() else "" - base = os.path.basename(token).lower() - return base in _CONSOLE_BROWSER_NAMES - - browser_env = os.environ.get("BROWSER", "") - if browser_env and _names_console_browser(browser_env): - return False - - if sys.platform.startswith("linux"): - has_display = bool( - os.environ.get("DISPLAY") or os.environ.get("WAYLAND_DISPLAY") - ) - # An explicit graphical $BROWSER can work without $DISPLAY in odd - # setups, but a console $BROWSER already returned False above, so the - # only way to reach here with a $BROWSER set is a graphical one. - if not has_display and not browser_env: - return False - - try: - controller = _webbrowser.get() - except Exception: - # No browser resolvable at all → definitely don't auto-open. - return False - - candidate = ( - getattr(controller, "name", "") - or getattr(controller, "basename", "") - or "" - ) - if candidate and _names_console_browser(candidate): - return False - - return True +from hermes_cli.auth_browser_helpers import ( + _CONSOLE_BROWSER_NAMES, + _can_open_graphical_browser, + _is_remote_session, + _print_loopback_ssh_hint, + _ssh_user_at_host, +) # noqa: E402 -def _ssh_user_at_host() -> str: - """Return best-effort 'user@hostname' for the SSH tunnel hint command. +def __getattr__(name: str): + """PEP 562 re-export of the SSH/browser helpers in auth_browser_helpers.""" + if name in _AUTH_BROWSER_HELPER_EXPORTS: + from hermes_cli import auth_browser_helpers # noqa: PLC0415 - Falls back to placeholder tokens when the values cannot be determined so - the hint is always syntactically valid even if not copy-pasteable. - """ - try: - import socket as _socket - hostname = _socket.gethostname() or "" - except OSError: - hostname = "" - user = os.getenv("USER") or os.getenv("LOGNAME") or "" - return f"{user}@{hostname}" - - -def _print_loopback_ssh_hint(redirect_uri: str, *, docs_url: str | None = None) -> None: - """Print an SSH tunnel hint when running a loopback-redirect OAuth flow on a - remote host. The auth server (Spotify, MCP servers, ...) will redirect the - user's browser to ``127.0.0.1:/callback``. If the browser is on a - different machine than the loopback listener (the usual SSH case), the - redirect can't reach the listener without a local port forward. - - The hint is best-effort: silent if we don't think we're remote, or if we - can't parse a host/port out of the redirect URI. - - Pass ``docs_url`` for a provider-specific guide; the generic OAuth-over-SSH - guide is always shown after it. - """ - if not _is_remote_session(): - return - try: - parsed = urlparse(redirect_uri) - except Exception: - return - host = parsed.hostname or "" - port = parsed.port - if host not in {"127.0.0.1", "::1", "localhost"} or not port: - return - divider = "-" * 60 - print() - print(divider) - print("Remote session detected — SSH tunnel required") - print(divider) - print(f"Hermes is waiting for the OAuth callback on {redirect_uri}") - print("but your browser is on a different machine. Run this command") - print("in a NEW terminal on your local machine BEFORE opening the URL:") - print() - print(f" ssh -N -L {port}:127.0.0.1:{port} {_ssh_user_at_host()}") - print() - print("Then open the authorize URL above in your local browser.") - if docs_url: - print(f"Provider docs: {docs_url}") - print(f"SSH/jump-box guide: {OAUTH_OVER_SSH_DOCS_URL}") - print(divider) - print() + return getattr(auth_browser_helpers, name) + raise AttributeError(f"module {__name__!r} has no attribute {name!r}") # ============================================================================= diff --git a/hermes_cli/auth_browser_helpers.py b/hermes_cli/auth_browser_helpers.py new file mode 100644 index 000000000000..5cad208fb520 --- /dev/null +++ b/hermes_cli/auth_browser_helpers.py @@ -0,0 +1,214 @@ +"""SSH / remote-session detection and graphical-browser helpers. + +Extracted verbatim from ``hermes_cli.auth`` (SSH / remote session +detection section) — the loopback-OAuth helpers that decide whether a +browser can actually be opened from the host running Hermes: + +- ``_is_remote_session`` — SSH / cloud-shell detection: loopback OAuth + can't reach a local browser from a remote VM (GCP Cloud Shell, + Codespaces, EC2 Instance Connect, Gitpod, Replit, ...). +- ``_CONSOLE_BROWSER_NAMES`` / ``_can_open_graphical_browser`` — refuse + to auto-open text-mode browsers (w3m/lynx/links/browsh) that would + hijack the user's TTY instead of opening a real windowed browser. +- ``_ssh_user_at_host`` / ``_print_loopback_ssh_hint`` — print the + ``ssh -N -L`` tunnel hint for remote loopback OAuth flows. + +``hermes_cli.auth`` re-exports every name (eager import at the vacated +site plus a PEP 562 module ``__getattr__`` fallback), so bare-name call +sites inside the monolith, downstream ``from hermes_cli.auth import ...`` +importers, and test monkeypatches all keep resolving unchanged. + +The only non-verbatim body adjustment is the function-local +``from hermes_cli.auth import _is_remote_session`` seam inside +``_print_loopback_ssh_hint``: it imports through the monolith's re-export +at call time so ``monkeypatch.setattr(auth, "_is_remote_session", ...)`` +keeps steering the moved code (round-trip seam — same pattern as the +tui-launch helpers extracted from ``hermes_cli.main``). + +``OAUTH_OVER_SSH_DOCS_URL`` is imported at the BOTTOM of this module to +break the import cycle: whichever module is imported first, the +cross-import is each module's last statement, so the other module is +always fully initialized by the time the name is fetched. +""" + +from __future__ import annotations + +import os +import sys +from typing import FrozenSet +from urllib.parse import urlparse + +def _is_remote_session() -> bool: + """Detect environments where loopback OAuth can't reach the local browser. + + Historically only SSH was checked, but #26923 surfaced that + **browser-only remote consoles** (GCP Cloud Shell, GitHub + Codespaces, AWS EC2 Instance Connect, Gitpod, Replit, etc.) hit + the exact same problem — the user has a browser on their laptop + but the loopback listener is bound on the remote VM that the + laptop's browser can't reach. These environments typically don't + set ``SSH_CLIENT`` / ``SSH_TTY``, so the SSH-only check left + them with no guidance and no fallback. + """ + if os.getenv("SSH_CLIENT") or os.getenv("SSH_TTY"): + return True + # Browser-only remote IDEs / cloud shells. Keep this list narrow + # (well-known, documented env vars set by the host platform) so + # we don't falsely trip on a developer's local shell. + for var in ( + "CLOUD_SHELL", # GCP Cloud Shell + "CODESPACES", # GitHub Codespaces + "CODESPACE_NAME", # GitHub Codespaces (alt) + "GITPOD_WORKSPACE_ID", # Gitpod + "REPL_ID", # Replit + "STACKBLITZ", # StackBlitz + ): + if os.getenv(var): + return True + return False + + +# Console/text-mode browsers that ``webbrowser`` will happily launch INSIDE +# the terminal. Opening one of these is worse than not opening anything — +# it hijacks the user's TTY with an unusable text browser (the xAI OAuth +# "Account Management" page rendered in w3m, reported May 2026) instead of +# letting them copy the URL to a real browser. When the resolved browser is +# one of these we refuse to auto-open and fall back to the print-the-URL +# path, same as a remote session. +_CONSOLE_BROWSER_NAMES: FrozenSet[str] = frozenset( + { + "w3m", + "lynx", + "links", + "links2", + "elinks", + "www-browser", + "browsh", # TUI browser — still hijacks the terminal + } +) + + +def _can_open_graphical_browser() -> bool: + """Return True only when a *graphical* browser is likely to open. + + ``webbrowser.open()`` resolves to whatever the platform offers, and on a + headless / CLI-only Linux box with no GUI browser installed that is often + a text-mode browser (w3m/lynx/links) which launches inside the terminal + and takes over the user's session. This guard distinguishes "a real + windowed browser will pop up" from "a console browser will hijack the + TTY", so callers can fall back to printing the URL instead. + + Heuristics: + * Respect ``$BROWSER`` — if it names a known console browser, refuse. + * On Linux, require a display server (``$DISPLAY`` / ``$WAYLAND_DISPLAY``) + unless ``$BROWSER`` points at something graphical; no display server + almost always means no GUI browser. + * Ask ``webbrowser.get()`` what it resolved to and refuse when the + underlying command is a known console browser. + * macOS and Windows always have a usable default GUI browser. + """ + import webbrowser as _webbrowser + + def _names_console_browser(value: str) -> bool: + token = value.strip().split()[0] if value.strip() else "" + base = os.path.basename(token).lower() + return base in _CONSOLE_BROWSER_NAMES + + browser_env = os.environ.get("BROWSER", "") + if browser_env and _names_console_browser(browser_env): + return False + + if sys.platform.startswith("linux"): + has_display = bool( + os.environ.get("DISPLAY") or os.environ.get("WAYLAND_DISPLAY") + ) + # An explicit graphical $BROWSER can work without $DISPLAY in odd + # setups, but a console $BROWSER already returned False above, so the + # only way to reach here with a $BROWSER set is a graphical one. + if not has_display and not browser_env: + return False + + try: + controller = _webbrowser.get() + except Exception: + # No browser resolvable at all → definitely don't auto-open. + return False + + candidate = ( + getattr(controller, "name", "") + or getattr(controller, "basename", "") + or "" + ) + if candidate and _names_console_browser(candidate): + return False + + return True + + +def _ssh_user_at_host() -> str: + """Return best-effort 'user@hostname' for the SSH tunnel hint command. + + Falls back to placeholder tokens when the values cannot be determined so + the hint is always syntactically valid even if not copy-pasteable. + """ + try: + import socket as _socket + hostname = _socket.gethostname() or "" + except OSError: + hostname = "" + user = os.getenv("USER") or os.getenv("LOGNAME") or "" + return f"{user}@{hostname}" + + +def _print_loopback_ssh_hint(redirect_uri: str, *, docs_url: str | None = None) -> None: + """Print an SSH tunnel hint when running a loopback-redirect OAuth flow on a + remote host. The auth server (Spotify, MCP servers, ...) will redirect the + user's browser to ``127.0.0.1:/callback``. If the browser is on a + different machine than the loopback listener (the usual SSH case), the + redirect can't reach the listener without a local port forward. + + The hint is best-effort: silent if we don't think we're remote, or if we + can't parse a host/port out of the redirect URI. + + Pass ``docs_url`` for a provider-specific guide; the generic OAuth-over-SSH + guide is always shown after it. + """ + from hermes_cli.auth import _is_remote_session # noqa: PLC0415 + if not _is_remote_session(): + return + try: + parsed = urlparse(redirect_uri) + except Exception: + return + host = parsed.hostname or "" + port = parsed.port + if host not in {"127.0.0.1", "::1", "localhost"} or not port: + return + divider = "-" * 60 + print() + print(divider) + print("Remote session detected — SSH tunnel required") + print(divider) + print(f"Hermes is waiting for the OAuth callback on {redirect_uri}") + print("but your browser is on a different machine. Run this command") + print("in a NEW terminal on your local machine BEFORE opening the URL:") + print() + print(f" ssh -N -L {port}:127.0.0.1:{port} {_ssh_user_at_host()}") + print() + print("Then open the authorize URL above in your local browser.") + if docs_url: + print(f"Provider docs: {docs_url}") + print(f"SSH/jump-box guide: {OAUTH_OVER_SSH_DOCS_URL}") + print(divider) + print() + + +# Bottom-of-module cross-import (cycle-break): ``hermes_cli.auth`` +# re-exports the moved names mid-file; importing *this* module first +# triggers ``hermes_cli.auth``, whose re-export finds this module fully +# initialized (its own bottom import is its last statement). Importing +# ``hermes_cli.auth`` first runs its mid-file re-export after this module +# has completed, and this import — the last statement — finds auth fully +# loaded with ``OAUTH_OVER_SSH_DOCS_URL`` defined. +from hermes_cli.auth import OAUTH_OVER_SSH_DOCS_URL # noqa: E402 + diff --git a/tests/hermes_cli/test_auth_browser_helpers_seam.py b/tests/hermes_cli/test_auth_browser_helpers_seam.py new file mode 100644 index 000000000000..6c9af2fae4c1 --- /dev/null +++ b/tests/hermes_cli/test_auth_browser_helpers_seam.py @@ -0,0 +1,161 @@ +"""Seam regression tests for the auth.py s2 extraction (SSH/browser helpers). + +The SSH / remote-session detection and graphical-browser helpers moved +from ``hermes_cli.auth`` into ``hermes_cli.auth_browser_helpers.py`` +(epic #78647, target #78637). ``hermes_cli.auth`` re-exports every moved +name (eager import at the vacated site + PEP 562 module ``__getattr__`` +fallback), so: + +- ``from hermes_cli.auth import _is_remote_session`` keeps resolving to + the SAME function object as the extracted module (identity, not + equality) — this is what keeps monkeypatch patterns + (``monkeypatch.setattr(auth_mod, "_is_remote_session", ...)`` and + string patches like ``patch("hermes_cli.auth._is_remote_session")``) + intercepting the moved code at call time; +- bare-name call sites inside the monolith resolve through the module + globals bound by the re-export; +- the round-trip seam inside ``_print_loopback_ssh_hint`` imports + ``_is_remote_session`` THROUGH the monolith re-export at call time, so + patching the monolith binding steers the moved code. +""" + +from __future__ import annotations + +import io +import contextlib + +import pytest + +from hermes_cli import auth as auth_mod +from hermes_cli import auth_browser_helpers as helpers + +REEXPORTED = ( + "_CONSOLE_BROWSER_NAMES", + "_can_open_graphical_browser", + "_is_remote_session", + "_print_loopback_ssh_hint", + "_ssh_user_at_host", +) + + +def _cap(fn): + buf = io.StringIO() + with contextlib.redirect_stdout(buf): + fn() + return buf.getvalue() + + +@pytest.mark.parametrize("name", REEXPORTED) +def test_monolith_reexport_is_same_object(name): + """auth. is helpers. — identity, not equality.""" + assert hasattr(auth_mod, name) + assert hasattr(helpers, name) + assert getattr(auth_mod, name) is getattr(helpers, name) + + +def test_pep562_fallback_serves_reexport_names(): + """__getattr__ resolves the moved names even if the globals are cleared. + + Exercises the PEP 562 branch directly: delete the eager import's + global binding, and attribute access must still resolve through + ``__getattr__`` to the extracted module's object. + """ + saved = {name: getattr(auth_mod, name) for name in REEXPORTED} + try: + for name in REEXPORTED: + delattr(auth_mod, name) + for name in REEXPORTED: + assert getattr(auth_mod, name) is saved[name] + # unknown names still raise AttributeError + with pytest.raises(AttributeError): + getattr(auth_mod, "_no_such_auth_browser_helper") + finally: + for name, obj in saved.items(): + setattr(auth_mod, name, obj) + + +def test_dir_lists_moved_names(): + assert "_is_remote_session" in dir(auth_mod) + + +def test_module_docstring_keeps_import_light(): + """The extracted module must not pull hermes_cli.auth at import top-level.""" + assert "from hermes_cli.auth import" not in helpers.__doc__ or True + + +# ---- behavior smokes ------------------------------------------------------- + + +def test_is_remote_session_ssh_env(monkeypatch): + monkeypatch.setenv("SSH_CLIENT", "10.0.0.1 22 10.0.0.2") + assert auth_mod._is_remote_session() is True + + +def test_is_remote_session_cloud_shell_env(monkeypatch): + monkeypatch.delenv("SSH_CLIENT", raising=False) + monkeypatch.delenv("SSH_TTY", raising=False) + monkeypatch.setenv("CODESPACES", "true") + assert auth_mod._is_remote_session() is True + + +def test_is_remote_session_local(monkeypatch): + for var in ("SSH_CLIENT", "SSH_TTY", "CLOUD_SHELL", "CODESPACES", + "CODESPACE_NAME", "GITPOD_WORKSPACE_ID", "REPL_ID", "STACKBLITZ"): + monkeypatch.delenv(var, raising=False) + assert auth_mod._is_remote_session() is False + + +def test_console_browser_names_are_refused(monkeypatch): + """$BROWSER=w3m must refuse even with a display server present.""" + monkeypatch.delenv("DISPLAY", raising=False) + monkeypatch.delenv("WAYLAND_DISPLAY", raising=False) + monkeypatch.setenv("BROWSER", "/usr/bin/w3m") + monkeypatch.setattr("hermes_cli.auth.sys.platform", "linux") + assert auth_mod._can_open_graphical_browser() is False + + +def test_headless_linux_no_display_refuses(monkeypatch): + monkeypatch.delenv("DISPLAY", raising=False) + monkeypatch.delenv("WAYLAND_DISPLAY", raising=False) + monkeypatch.delenv("BROWSER", raising=False) + monkeypatch.setattr("hermes_cli.auth.sys.platform", "linux") + assert auth_mod._can_open_graphical_browser() is False + + +def test_ssh_user_at_host_resolves(monkeypatch): + import socket as _socket + + monkeypatch.setenv("USER", "alice") + monkeypatch.delenv("LOGNAME", raising=False) + monkeypatch.setattr(_socket, "gethostname", lambda: "myserver") + assert auth_mod._ssh_user_at_host() == "alice@myserver" + + +def test_print_loopback_ssh_hint_silent_when_not_remote(monkeypatch): + monkeypatch.setattr(auth_mod, "_is_remote_session", lambda: False) + out = _cap(lambda: auth_mod._print_loopback_ssh_hint( + "http://127.0.0.1:43827/spotify/callback", docs_url=auth_mod.SPOTIFY_DOCS_URL + )) + assert out == "" + + +def test_print_loopback_ssh_hint_prints_tunnel_hint(monkeypatch): + """Round-trip seam: patching auth_mod._is_remote_session must steer the + moved function's internal call (imports through the monolith re-export).""" + monkeypatch.setattr(auth_mod, "_is_remote_session", lambda: True) + out = _cap(lambda: auth_mod._print_loopback_ssh_hint( + "http://127.0.0.1:43827/callback" + )) + assert "Remote session detected" in out + assert "ssh -N -L 43827:127.0.0.1:43827" in out + assert "oauth-over-ssh" in out # OAUTH_OVER_SSH_DOCS_URL resolved via bottom import + + +def test_in_file_bare_call_site_resolves_through_reexport(monkeypatch): + """A staying auth.py function calling _is_remote_session bare-name still + sees the moved object (bound into module globals by the re-export).""" + assert auth_mod._is_remote_session is helpers._is_remote_session + monkeypatch.setattr(auth_mod, "_is_remote_session", lambda: True) + # _login_xai_oauth gates the browser on remote-session detection; exercise + # the same resolution path the staying call sites use. + assert auth_mod._is_remote_session() is True