diff --git a/tests/tools/test_checkpoint_manager.py b/tests/tools/test_checkpoint_manager.py index ba9da6da1..a464afc06 100644 --- a/tests/tools/test_checkpoint_manager.py +++ b/tests/tools/test_checkpoint_manager.py @@ -587,3 +587,112 @@ class TestSecurity: result = mgr.restore(str(work_dir), target_hash, file_path="subdir/test.txt") assert result["success"] is True + + +# ========================================================================= +# GPG / global git config isolation +# ========================================================================= +# Regression tests for the bug where users with ``commit.gpgsign = true`` +# in their global git config got a pinentry popup (or a failed commit) +# every time the agent took a background snapshot. + +import os as _os + + +class TestGpgAndGlobalConfigIsolation: + def test_git_env_isolates_global_and_system_config(self, tmp_path): + """_git_env must null out GIT_CONFIG_GLOBAL / GIT_CONFIG_SYSTEM so the + shadow repo does not inherit user-level gpgsign, hooks, aliases, etc.""" + env = _git_env(tmp_path / "shadow", str(tmp_path)) + assert env["GIT_CONFIG_GLOBAL"] == _os.devnull + assert env["GIT_CONFIG_SYSTEM"] == _os.devnull + assert env["GIT_CONFIG_NOSYSTEM"] == "1" + + def test_init_sets_commit_gpgsign_false(self, work_dir, checkpoint_base, monkeypatch): + monkeypatch.setattr("tools.checkpoint_manager.CHECKPOINT_BASE", checkpoint_base) + shadow = _shadow_repo_path(str(work_dir)) + _init_shadow_repo(shadow, str(work_dir)) + # Inspect the shadow's own config directly — the settings must be + # written into the repo, not just inherited via env vars. + result = subprocess.run( + ["git", "config", "--file", str(shadow / "config"), "--get", "commit.gpgsign"], + capture_output=True, text=True, + ) + assert result.stdout.strip() == "false" + + def test_init_sets_tag_gpgsign_false(self, work_dir, checkpoint_base, monkeypatch): + monkeypatch.setattr("tools.checkpoint_manager.CHECKPOINT_BASE", checkpoint_base) + shadow = _shadow_repo_path(str(work_dir)) + _init_shadow_repo(shadow, str(work_dir)) + result = subprocess.run( + ["git", "config", "--file", str(shadow / "config"), "--get", "tag.gpgSign"], + capture_output=True, text=True, + ) + assert result.stdout.strip() == "false" + + def test_checkpoint_works_with_global_gpgsign_and_broken_gpg( + self, work_dir, checkpoint_base, monkeypatch, tmp_path + ): + """The real bug scenario: user has global commit.gpgsign=true but GPG + is broken or pinentry is unavailable. Before the fix, every snapshot + either failed or spawned a pinentry window. After the fix, snapshots + succeed without ever invoking GPG.""" + monkeypatch.setattr("tools.checkpoint_manager.CHECKPOINT_BASE", checkpoint_base) + + # Fake HOME with global gpgsign=true and a deliberately broken GPG + # binary. If isolation fails, the commit will try to exec this + # nonexistent path and the checkpoint will fail. + fake_home = tmp_path / "fake_home" + fake_home.mkdir() + (fake_home / ".gitconfig").write_text( + "[user]\n email = real@user.com\n name = Real User\n" + "[commit]\n gpgsign = true\n" + "[tag]\n gpgSign = true\n" + "[gpg]\n program = /nonexistent/fake-gpg-binary\n" + ) + monkeypatch.setenv("HOME", str(fake_home)) + monkeypatch.delenv("GPG_TTY", raising=False) + monkeypatch.delenv("DISPLAY", raising=False) # block GUI pinentry + + mgr = CheckpointManager(enabled=True) + assert mgr.ensure_checkpoint(str(work_dir), reason="with-global-gpgsign") is True + assert len(mgr.list_checkpoints(str(work_dir))) == 1 + + def test_checkpoint_works_on_prefix_shadow_without_local_gpgsign( + self, work_dir, checkpoint_base, monkeypatch, tmp_path + ): + """Users with shadow repos created before the fix will not have + commit.gpgsign=false in their shadow's own config. The inline + ``--no-gpg-sign`` flag on the commit call must cover them.""" + monkeypatch.setattr("tools.checkpoint_manager.CHECKPOINT_BASE", checkpoint_base) + + # Simulate a pre-fix shadow repo: init without commit.gpgsign=false + # in its own config. _init_shadow_repo now writes it, so we must + # manually remove it to mimic the pre-fix state. + shadow = _shadow_repo_path(str(work_dir)) + _init_shadow_repo(shadow, str(work_dir)) + subprocess.run( + ["git", "config", "--file", str(shadow / "config"), + "--unset", "commit.gpgsign"], + capture_output=True, text=True, check=False, + ) + subprocess.run( + ["git", "config", "--file", str(shadow / "config"), + "--unset", "tag.gpgSign"], + capture_output=True, text=True, check=False, + ) + + # And simulate hostile global config + fake_home = tmp_path / "fake_home" + fake_home.mkdir() + (fake_home / ".gitconfig").write_text( + "[commit]\n gpgsign = true\n" + "[gpg]\n program = /nonexistent/fake-gpg-binary\n" + ) + monkeypatch.setenv("HOME", str(fake_home)) + monkeypatch.delenv("GPG_TTY", raising=False) + monkeypatch.delenv("DISPLAY", raising=False) + + mgr = CheckpointManager(enabled=True) + assert mgr.ensure_checkpoint(str(work_dir), reason="prefix-shadow") is True + assert len(mgr.list_checkpoints(str(work_dir))) == 1 diff --git a/tools/checkpoint_manager.py b/tools/checkpoint_manager.py index 42900a643..277a23e44 100644 --- a/tools/checkpoint_manager.py +++ b/tools/checkpoint_manager.py @@ -126,7 +126,22 @@ def _shadow_repo_path(working_dir: str) -> Path: def _git_env(shadow_repo: Path, working_dir: str) -> dict: - """Build env dict that redirects git to the shadow repo.""" + """Build env dict that redirects git to the shadow repo. + + The shadow repo is internal Hermes infrastructure — it must NOT inherit + the user's global or system git config. User-level settings like + ``commit.gpgsign = true``, signing hooks, or credential helpers would + either break background snapshots or, worse, spawn interactive prompts + (pinentry GUI windows) mid-session every time a file is written. + + Isolation strategy: + * ``GIT_CONFIG_GLOBAL=`` — ignore ``~/.gitconfig`` (git 2.32+). + * ``GIT_CONFIG_SYSTEM=`` — ignore ``/etc/gitconfig`` (git 2.32+). + * ``GIT_CONFIG_NOSYSTEM=1`` — legacy belt-and-suspenders for older git. + + The shadow repo still has its own per-repo config (user.email, user.name, + commit.gpgsign=false) set in ``_init_shadow_repo``. + """ normalized_working_dir = _normalize_path(working_dir) env = os.environ.copy() env["GIT_DIR"] = str(shadow_repo) @@ -134,6 +149,13 @@ def _git_env(shadow_repo: Path, working_dir: str) -> dict: env.pop("GIT_INDEX_FILE", None) env.pop("GIT_NAMESPACE", None) env.pop("GIT_ALTERNATE_OBJECT_DIRECTORIES", None) + # Isolate the shadow repo from the user's global/system git config. + # Prevents commit.gpgsign, hooks, aliases, credential helpers, etc. from + # leaking into background snapshots. Uses os.devnull for cross-platform + # support (``/dev/null`` on POSIX, ``nul`` on Windows). + env["GIT_CONFIG_GLOBAL"] = os.devnull + env["GIT_CONFIG_SYSTEM"] = os.devnull + env["GIT_CONFIG_NOSYSTEM"] = "1" return env @@ -211,6 +233,13 @@ def _init_shadow_repo(shadow_repo: Path, working_dir: str) -> Optional[str]: _run_git(["config", "user.email", "hermes@local"], shadow_repo, working_dir) _run_git(["config", "user.name", "Hermes Checkpoint"], shadow_repo, working_dir) + # Explicitly disable commit/tag signing in the shadow repo. _git_env + # already isolates from the user's global config, but writing these into + # the shadow's own config is belt-and-suspenders — it guarantees the + # shadow repo is correct even if someone inspects or runs git against it + # directly (without the GIT_CONFIG_* env vars). + _run_git(["config", "commit.gpgsign", "false"], shadow_repo, working_dir) + _run_git(["config", "tag.gpgSign", "false"], shadow_repo, working_dir) info_dir = shadow_repo / "info" info_dir.mkdir(exist_ok=True) @@ -552,9 +581,11 @@ class CheckpointManager: logger.debug("Checkpoint skipped: no changes in %s", working_dir) return False - # Commit + # Commit. ``--no-gpg-sign`` inline covers shadow repos created before + # the commit.gpgsign=false config was added to _init_shadow_repo — so + # users with existing checkpoints never hit a GPG pinentry popup. ok, _, err = _run_git( - ["commit", "-m", reason, "--allow-empty-message"], + ["commit", "-m", reason, "--allow-empty-message", "--no-gpg-sign"], shadow, working_dir, timeout=_GIT_TIMEOUT * 2, ) if not ok: