From 58708c7066c1bf2abecb0d32fb76cf8ac8c0a917 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Tue, 28 Jul 2026 17:16:43 -0700 Subject: [PATCH] fix(git): never block internal git calls on credential prompts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Port from openai/codex#34540 / #34612 ("detach non-interactive subprocesses from stdin"): internal git invocations that run with nobody attached — MCP catalog installs, plugin install/update, profile distribution staging, worktree base fetches, and the desktop review pane's git/gh backend — could hang on a credential prompt when a remote is private, misconfigured, or requires auth. git prompts on the inherited terminal (or via Git Credential Manager on Windows), so the operation silently waits until its timeout, or forever at sites without one (mcp_catalog clones have no timeout at all and inherit the parent terminal). - Add noninteractive_git_env() to hermes_cli/_subprocess_compat.py: GIT_TERMINAL_PROMPT=0 + GCM_INTERACTIVE=Never on a copy of the environment; GIT_ASKPASS/SSH_ASKPASS deliberately preserved so working non-interactive auth still succeeds. - Wire it + stdin=DEVNULL into: mcp_catalog._do_git_install (clone/ checkout), plugins_cmd (clone + pull), profile_distribution._git_clone, web_git._git/_gh (gh also gets GH_PROMPT_DISABLED=1), and cli.py's worktree base fetch helper. - Tests: env contract, a real-git E2E against a local 401 Basic-auth HTTP server proving fail-fast ("terminal prompts disabled") instead of a hang, and per-call-site plumbing assertions. Sabotage-verified: removing the env from web_git._git fails the site test. --- cli.py | 4 + hermes_cli/_subprocess_compat.py | 49 ++++- hermes_cli/mcp_catalog.py | 20 +- hermes_cli/plugins_cmd.py | 5 + hermes_cli/profile_distribution.py | 3 + hermes_cli/web_git.py | 20 +- tests/hermes_cli/test_noninteractive_git.py | 218 ++++++++++++++++++++ 7 files changed, 314 insertions(+), 5 deletions(-) create mode 100644 tests/hermes_cli/test_noninteractive_git.py 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)