diff --git a/cli.py b/cli.py index f4223f2e930a..18dbe94e02d5 100644 --- a/cli.py +++ b/cli.py @@ -1475,10 +1475,14 @@ def _resolve_worktree_base(repo_root: str) -> tuple: """ import subprocess + from hermes_cli._subprocess_compat import noninteractive_git_env + def _git(args, timeout=20): return subprocess.run( ["git", *args], capture_output=True, text=True, encoding="utf-8", errors="replace", timeout=timeout, cwd=repo_root, + stdin=subprocess.DEVNULL, + env=noninteractive_git_env(), ) # 1. Current branch's upstream, if it tracks one. diff --git a/hermes_cli/_subprocess_compat.py b/hermes_cli/_subprocess_compat.py index 339adf87dafa..b6b64066608c 100644 --- a/hermes_cli/_subprocess_compat.py +++ b/hermes_cli/_subprocess_compat.py @@ -28,10 +28,11 @@ guarantee. from __future__ import annotations +import os import shutil import subprocess import sys -from typing import Sequence +from typing import Mapping, Sequence __all__ = [ "IS_WINDOWS", @@ -42,6 +43,7 @@ __all__ = [ "windows_hide_flags", "windows_detach_popen_kwargs", "bounded_git_probe", + "noninteractive_git_env", ] @@ -297,6 +299,51 @@ def windows_detach_popen_kwargs() -> dict: return {"start_new_session": True} +# ----------------------------------------------------------------------------- +# Non-interactive git environment (credential-prompt hang guard) +# ----------------------------------------------------------------------------- + + +def noninteractive_git_env( + base: "Mapping[str, str] | None" = None, +) -> dict[str, str]: + """Environment for *internal* git invocations that must never prompt. + + Hermes shells out to git from many non-interactive contexts — MCP catalog + installs, plugin install/update, profile distribution staging, worktree + base fetches, desktop review-pane fetch/push. When the remote is private, + misconfigured, or requires auth, git's default behavior is to prompt on + the inherited terminal (or via an askpass helper), which silently hangs + the operation until its timeout — or forever at call sites without one. + Ported from openai/codex#34540 / #34612 ("detach non-interactive + subprocesses from stdin"): a background tool invocation must fail fast + with a readable error, not wait for input nobody can type. + + Returns a copy of ``base`` (default ``os.environ``) with: + + * ``GIT_TERMINAL_PROMPT=0`` — git fails with "terminal prompts disabled" + instead of prompting for credentials. + * ``GCM_INTERACTIVE=Never`` — Git Credential Manager (the default + credential helper on Windows installs) never pops its own dialog. + + ``GIT_ASKPASS`` / ``SSH_ASKPASS`` are deliberately left alone: when the + user has a *working* askpass helper or ssh-agent configured, auth should + still succeed non-interactively. The env only disables paths that block + on a human. + + Pair with ``stdin=subprocess.DEVNULL`` so git (and any credential helper + it spawns) also can't read the parent's inherited stdin. + + This is for internal plumbing calls only — the agent-facing terminal tool + has its own policy layer and user-visible PTY, where prompting can be + legitimate. + """ + env = dict(base if base is not None else os.environ) + env["GIT_TERMINAL_PROMPT"] = "0" + env["GCM_INTERACTIVE"] = "Never" + return env + + # ----------------------------------------------------------------------------- # Bounded, fail-open git probing (Windows post-kill deadlock guard) # ----------------------------------------------------------------------------- diff --git a/hermes_cli/mcp_catalog.py b/hermes_cli/mcp_catalog.py index f2a5303abe94..6f8c9b30c648 100644 --- a/hermes_cli/mcp_catalog.py +++ b/hermes_cli/mcp_catalog.py @@ -37,6 +37,7 @@ from typing import Any, Dict, List, Optional import yaml from hermes_constants import get_hermes_home, get_optional_mcps_dir +from hermes_cli._subprocess_compat import noninteractive_git_env from hermes_cli.colors import Colors, color from hermes_cli.config import ( load_config, @@ -412,9 +413,16 @@ def _do_git_install(entry: CatalogEntry) -> Path: # SHA ref before we fall back to full-clone-then-checkout). is_sha_ref = bool(re.fullmatch(r"[0-9a-f]{7,40}", install.ref)) + # Never let an install hang on a credential prompt: catalog installs run + # from CLI commands and dashboard flows where nobody can answer git's + # username/password prompt (private repo, bad remote, auth required). + _git_env = noninteractive_git_env() + if not is_sha_ref: proc = subprocess.run( [git, "clone", "--depth", "1", "--branch", install.ref, install.url, str(dest)], + stdin=subprocess.DEVNULL, + env=_git_env, ) if proc.returncode == 0: pass @@ -426,10 +434,18 @@ def _do_git_install(entry: CatalogEntry) -> Path: is_sha_ref = True # treat the same as a SHA ref from here if is_sha_ref: - proc = subprocess.run([git, "clone", install.url, str(dest)]) + proc = subprocess.run( + [git, "clone", install.url, str(dest)], + stdin=subprocess.DEVNULL, + env=_git_env, + ) if proc.returncode != 0: raise CatalogError(f"git clone failed for {install.url}") - proc = subprocess.run([git, "-C", str(dest), "checkout", install.ref]) + proc = subprocess.run( + [git, "-C", str(dest), "checkout", install.ref], + stdin=subprocess.DEVNULL, + env=_git_env, + ) if proc.returncode != 0: raise CatalogError(f"git checkout {install.ref} failed") diff --git a/hermes_cli/plugins_cmd.py b/hermes_cli/plugins_cmd.py index 8bc1edd9d05e..04b66380d55b 100644 --- a/hermes_cli/plugins_cmd.py +++ b/hermes_cli/plugins_cmd.py @@ -21,6 +21,7 @@ from pathlib import Path from typing import Any, Optional from hermes_constants import get_hermes_home +from hermes_cli._subprocess_compat import noninteractive_git_env from hermes_cli.config import cfg_get from hermes_cli.secret_prompt import masked_secret_prompt @@ -474,6 +475,8 @@ def _install_plugin_core(identifier: str, *, force: bool) -> tuple[Path, dict, s capture_output=True, text=True, encoding='utf-8', errors='replace', timeout=60, + stdin=subprocess.DEVNULL, + env=noninteractive_git_env(), ) except FileNotFoundError as e: raise PluginOperationError( @@ -2005,6 +2008,8 @@ def _git_pull_plugin_dir(target: Path) -> tuple[bool, str]: text=True, encoding='utf-8', errors='replace', timeout=60, cwd=str(target), + stdin=subprocess.DEVNULL, + env=noninteractive_git_env(), ) except FileNotFoundError: return False, "git is not installed or not in PATH." diff --git a/hermes_cli/profile_distribution.py b/hermes_cli/profile_distribution.py index c981015d4b03..55a6be17f0c8 100644 --- a/hermes_cli/profile_distribution.py +++ b/hermes_cli/profile_distribution.py @@ -71,6 +71,7 @@ from pathlib import Path from typing import Any, Dict, List, Optional, Tuple from agent.skill_utils import is_excluded_skill_path +from hermes_cli._subprocess_compat import noninteractive_git_env # --------------------------------------------------------------------------- @@ -381,6 +382,8 @@ def _git_clone(url: str, dest: Path) -> None: ["git", "clone", "--depth", "1", url, str(dest)], check=True, capture_output=True, + stdin=subprocess.DEVNULL, + env=noninteractive_git_env(), ) except FileNotFoundError as exc: raise DistributionError("git is required for git-URL installs") from exc diff --git a/hermes_cli/web_git.py b/hermes_cli/web_git.py index 0a00c0246cb9..92389adda660 100644 --- a/hermes_cli/web_git.py +++ b/hermes_cli/web_git.py @@ -19,6 +19,8 @@ import shutil import subprocess from pathlib import Path +from hermes_cli._subprocess_compat import noninteractive_git_env + _GIT_TIMEOUT = 30 _GH_TIMEOUT = 30 _MAX_BUFFER = 32 * 1024 * 1024 @@ -31,7 +33,13 @@ _TRUNK_BRANCHES = ("main", "master") def _git(cwd: str, args: list[str], *, timeout: int = _GIT_TIMEOUT) -> tuple[int, str, str]: """Run ``git`` in ``cwd``. Returns (returncode, stdout, stderr); never raises - on a non-zero exit (callers decide what an error means).""" + on a non-zero exit (callers decide what an error means). + + Runs non-interactively (stdin nulled, ``GIT_TERMINAL_PROMPT=0``): these + calls serve authenticated REST requests from the dashboard/desktop, so a + credential prompt from ``fetch``/``push``/``pull`` could never be answered + — it would just hang the request until the timeout. Failing fast surfaces + the real auth error in the toast instead.""" try: proc = subprocess.run( ["git", *args], @@ -39,6 +47,8 @@ def _git(cwd: str, args: list[str], *, timeout: int = _GIT_TIMEOUT) -> tuple[int capture_output=True, text=True, encoding='utf-8', errors='replace', timeout=timeout, + stdin=subprocess.DEVNULL, + env=noninteractive_git_env(), ) except (OSError, subprocess.SubprocessError): return 1, "", "git invocation failed" @@ -419,9 +429,15 @@ def review_commit_context(cwd: str) -> dict: def _gh(cwd: str, args: list[str]) -> tuple[bool, str]: if not shutil.which("gh"): return False, "" + # Same non-interactive contract as _git: these serve REST requests, so gh + # must fail fast instead of prompting (GH_PROMPT_DISABLED is gh's own + # documented kill-switch for interactive prompts). + env = noninteractive_git_env() + env["GH_PROMPT_DISABLED"] = "1" try: proc = subprocess.run( - ["gh", *args], cwd=cwd, capture_output=True, text=True, encoding='utf-8', errors='replace', timeout=_GH_TIMEOUT + ["gh", *args], cwd=cwd, capture_output=True, text=True, encoding='utf-8', errors='replace', timeout=_GH_TIMEOUT, + stdin=subprocess.DEVNULL, env=env, ) except (OSError, subprocess.SubprocessError): return False, "" diff --git a/tests/hermes_cli/test_noninteractive_git.py b/tests/hermes_cli/test_noninteractive_git.py new file mode 100644 index 000000000000..cabc388987b8 --- /dev/null +++ b/tests/hermes_cli/test_noninteractive_git.py @@ -0,0 +1,218 @@ +"""Non-interactive internal git invocations (port of openai/codex#34540/#34612). + +Internal git plumbing (MCP catalog installs, plugin install/update, profile +distribution staging, worktree base fetches, desktop review-pane git/gh) must +never block on a credential prompt: nobody is attached to answer it, so a +prompt is an indefinite hang (or a dead wait until the timeout). + +Two layers of coverage: + +1. Unit contract on :func:`hermes_cli._subprocess_compat.noninteractive_git_env`. +2. A real-git E2E proving the env actually disables the prompt: a local HTTP + server answers 401 with a Basic challenge; ``git clone`` against it with + the hardened env fails *fast* with "terminal prompts disabled" instead of + waiting for a username. +3. Plumbing tests asserting each internal call site passes ``stdin=DEVNULL`` + and the hardened env to subprocess. +""" + +from __future__ import annotations + +import http.server +import os +import shutil +import subprocess +import threading +import time +from pathlib import Path + +import pytest + +from hermes_cli._subprocess_compat import noninteractive_git_env + + +# --------------------------------------------------------------------------- +# 1. Env helper contract +# --------------------------------------------------------------------------- + + +class TestNoninteractiveGitEnv: + def test_sets_prompt_kill_switches(self): + env = noninteractive_git_env({}) + assert env["GIT_TERMINAL_PROMPT"] == "0" + assert env["GCM_INTERACTIVE"] == "Never" + + def test_defaults_to_process_environ_copy(self, monkeypatch): + monkeypatch.setenv("HERMES_TEST_SENTINEL", "xyz") + env = noninteractive_git_env() + assert env["HERMES_TEST_SENTINEL"] == "xyz" + assert env["GIT_TERMINAL_PROMPT"] == "0" + # Never mutates the live process environment. + assert os.environ.get("GIT_TERMINAL_PROMPT") != "0" or True + assert "GCM_INTERACTIVE" not in os.environ or os.environ["GCM_INTERACTIVE"] == env["GCM_INTERACTIVE"] + + def test_does_not_mutate_base_mapping(self): + base = {"PATH": "/usr/bin"} + env = noninteractive_git_env(base) + assert "GIT_TERMINAL_PROMPT" not in base + assert env is not base + + def test_preserves_working_askpass(self): + """A configured askpass helper is a *working* non-interactive auth + path — the env must not strip it.""" + base = {"GIT_ASKPASS": "/usr/local/bin/my-askpass", "SSH_ASKPASS": "/x"} + env = noninteractive_git_env(base) + assert env["GIT_ASKPASS"] == "/usr/local/bin/my-askpass" + assert env["SSH_ASKPASS"] == "/x" + + def test_overrides_explicit_prompt_enable(self): + env = noninteractive_git_env({"GIT_TERMINAL_PROMPT": "1"}) + assert env["GIT_TERMINAL_PROMPT"] == "0" + + +# --------------------------------------------------------------------------- +# 2. Real-git E2E: 401 remote fails fast instead of prompting +# --------------------------------------------------------------------------- + + +class _BasicAuthChallenge(http.server.BaseHTTPRequestHandler): + def _challenge(self): + self.send_response(401) + self.send_header("WWW-Authenticate", 'Basic realm="hermes-test"') + self.send_header("Content-Length", "0") + self.end_headers() + + do_GET = _challenge + do_POST = _challenge + + def log_message(self, *args): # silence test output + pass + + +@pytest.mark.skipif(shutil.which("git") is None, reason="git not installed") +def test_git_clone_against_auth_remote_fails_fast(tmp_path: Path): + server = http.server.HTTPServer(("127.0.0.1", 0), _BasicAuthChallenge) + port = server.server_address[1] + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + try: + t0 = time.monotonic() + proc = subprocess.run( + ["git", "clone", f"http://127.0.0.1:{port}/private.git", + str(tmp_path / "dest")], + capture_output=True, + text=True, + timeout=30, + stdin=subprocess.DEVNULL, + env=noninteractive_git_env(), + ) + elapsed = time.monotonic() - t0 + assert proc.returncode != 0 + # With GIT_TERMINAL_PROMPT=0 git refuses to ask for a username + # instead of blocking on a prompt. + stderr = proc.stderr.lower() + assert ( + "terminal prompts disabled" in stderr + or "authentication failed" in stderr + ), f"unexpected git error: {proc.stderr!r}" + # Fail-fast, not a hang-until-timeout. + assert elapsed < 20 + finally: + server.shutdown() + server.server_close() + + +# --------------------------------------------------------------------------- +# 3. Call-site plumbing: internal git callers pass stdin=DEVNULL + env +# --------------------------------------------------------------------------- + + +def _capture_run(monkeypatch, module, **result_kwargs): + """Monkeypatch ``module.subprocess.run`` recording every call's kwargs.""" + calls: list[dict] = [] + + class _Result: + returncode = result_kwargs.get("returncode", 0) + stdout = result_kwargs.get("stdout", "") + stderr = result_kwargs.get("stderr", "") + + def fake_run(argv, **kwargs): + calls.append({"argv": list(argv), **kwargs}) + return _Result() + + monkeypatch.setattr(module.subprocess, "run", fake_run) + return calls + + +def _assert_noninteractive(call: dict): + assert call.get("stdin") is subprocess.DEVNULL, call["argv"] + env = call.get("env") + assert env is not None and env.get("GIT_TERMINAL_PROMPT") == "0", call["argv"] + + +def test_web_git_runs_noninteractively(monkeypatch, tmp_path): + from hermes_cli import web_git + + calls = _capture_run(monkeypatch, web_git) + web_git._git(str(tmp_path), ["fetch", "origin", "main"]) + assert calls + _assert_noninteractive(calls[0]) + + +def test_web_gh_runs_noninteractively(monkeypatch, tmp_path): + from hermes_cli import web_git + + monkeypatch.setattr(web_git.shutil, "which", lambda name: "/usr/bin/gh") + calls = _capture_run(monkeypatch, web_git) + web_git._gh(str(tmp_path), ["auth", "status"]) + assert calls + _assert_noninteractive(calls[0]) + assert calls[0]["env"].get("GH_PROMPT_DISABLED") == "1" + + +def test_plugin_git_pull_runs_noninteractively(monkeypatch, tmp_path): + from hermes_cli import plugins_cmd + + monkeypatch.setattr(plugins_cmd, "_resolve_git_executable", lambda: "git") + calls = _capture_run(monkeypatch, plugins_cmd) + plugins_cmd._git_pull_plugin_dir(tmp_path) + assert calls + _assert_noninteractive(calls[0]) + + +def test_profile_distribution_clone_runs_noninteractively(monkeypatch, tmp_path): + from hermes_cli import profile_distribution + + calls = _capture_run(monkeypatch, profile_distribution) + profile_distribution._git_clone( + "https://github.com/example/repo", tmp_path / "dest" + ) + assert calls + _assert_noninteractive(calls[0]) + + +def test_mcp_catalog_git_install_runs_noninteractively(monkeypatch, tmp_path): + from hermes_cli import mcp_catalog + + calls = _capture_run(monkeypatch, mcp_catalog) + monkeypatch.setattr(mcp_catalog.shutil, "which", lambda name: "/usr/bin/git") + monkeypatch.setattr(mcp_catalog, "_install_root", lambda: tmp_path) + + entry = mcp_catalog.CatalogEntry( + name="test-mcp", + description="", + source="official", + transport=mcp_catalog.TransportSpec(type="stdio", command="python"), + auth=mcp_catalog.AuthSpec(type="none"), + install=mcp_catalog.InstallSpec( + type="git", + url="https://github.com/example/mcp.git", + ref="main", + bootstrap=[], + ), + ) + mcp_catalog._do_git_install(entry) + assert calls + for call in calls: + if call["argv"][0].endswith("git") or "git" in call["argv"][0]: + _assert_noninteractive(call)