From e5afe57614407022c156d9a5a46fc6b975017f4d Mon Sep 17 00:00:00 2001 From: "Axl Ibiza, MBA" Date: Mon, 17 Aug 2026 18:23:45 -0500 Subject: [PATCH] fix(security): refuse file-tool writes to git-managed state under .git MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit write_file/patch (and the other mutating file tools) could silently rewrite git's own control files inside a normal repository's .git directory — HEAD, index, refs/, objects/, logs/, packed-refs, ORIG_HEAD, FETCH_HEAD, MERGE_HEAD, CHERRY_PICK_HEAD, REBASE_HEAD, COMMIT_EDITMSG, shallow, and info/* except exclude. A single misdirected write to .git/HEAD replaces the branch identity and turns a healthy checkout into an apparently empty one (same silent-corruption shape as the #78565 worktree-pointer guard, but inside the git dir). Add a git-state classifier to the shared write-deny path that refuses writes under any .git directory while keeping user-owned paths writable: .git/config, .git/hooks/*, .git/info/exclude, .git/description. Fixes #78793 --- agent/file_safety.py | 62 +++++++++++ contributors/emails/andrexibiza@gmail.com | 2 + tests/agent/test_file_safety_git_state.py | 122 ++++++++++++++++++++++ 3 files changed, 186 insertions(+) create mode 100644 contributors/emails/andrexibiza@gmail.com create mode 100644 tests/agent/test_file_safety_git_state.py diff --git a/agent/file_safety.py b/agent/file_safety.py index 7547000fa46b..c5319eecd633 100644 --- a/agent/file_safety.py +++ b/agent/file_safety.py @@ -129,6 +129,53 @@ def build_write_approval_paths(home: str) -> set[str]: } +def _classify_git_state_write(resolved: str) -> bool: + """Return True if ``resolved`` is a git-managed control path that the + generic file tools must not rewrite. + + Walks up the path components looking for a ``.git`` directory. If one is + found, the write is denied unless the target is one of the user-owned + paths inside a git dir that are safe (and legitimate) for the agent to + edit: + + * ``.git/config`` — repository configuration (user-owned) + * ``.git/hooks/`` — local hook scripts (user-owned) + * ``.git/info/exclude`` — per-repo ignore rules (user-owned) + * ``.git/description`` — repo description (user-owned) + + Everything else under a ``.git`` directory (HEAD, index, refs/, objects/, + logs/, packed-refs, ORIG_HEAD, FETCH_HEAD, MERGE_HEAD, CHERRY_PICK_HEAD, + REBASE_HEAD, COMMIT_EDITMSG, shallow, info/* other than exclude, ...) is + git-managed state that a misdirected write silently corrupts. + + Note: ``resolved`` must already be ``os.path.realpath()``-expanded so a + symlink that points into a git dir cannot bypass the check. + """ + parts = resolved.split(os.sep) + for i, part in enumerate(parts): + if part != ".git": + continue + # Found the git dir component. The target is anything after it. + rest = parts[i + 1:] + if not rest: + # Writing the `.git` dir itself is never legitimate. + return True + # The first segment inside `.git` decides allow vs deny. + head = rest[0] + if head in ("config", "description"): + return False + if head == "hooks": + # `.git/hooks/*` is user-owned; the hooks dir itself is fine. + return False + if head == "info": + # Only `.git/info/exclude` is user-owned; deny info/* else. + # Allow (return False) only for exactly ["info", "exclude"]. + return rest != ["info", "exclude"] + # Everything else under `.git/` is git-managed state. + return True + return False + + def _classify_write_denial(path: str) -> Optional[str]: """Return ``'credential'``, ``'safe_root'``, or ``None`` if writes are allowed.""" home = os.path.realpath(os.path.expanduser("~")) @@ -184,6 +231,21 @@ def _classify_write_denial(path: str) -> Optional[str]: except Exception: pass + # Git-managed state guard: never let the generic file tools rewrite + # git's own control files (HEAD, index, refs/, objects/, logs/, + # packed-refs, ORIG_HEAD, FETCH_HEAD, MERGE_HEAD, CHERRY_PICK_HEAD, + # REBASE_HEAD, COMMIT_EDITMSG, shallow, info/ except exclude, ...). + # A single misdirected write to `.git/HEAD` replaces the branch identity + # and turns a healthy checkout into an apparently empty one (same + # silent-corruption shape as the #78565 worktree-pointer guard). This + # closes the .git-directory case that the pointer-file guard cannot. + # + # User-owned paths inside a `.git` dir stay writable: `.git/config`, + # `.git/hooks/*`, `.git/info/exclude`, `.git/description`. + _git_state_denial = _classify_git_state_write(resolved) + if _git_state_denial: + return "credential" + safe_roots = get_safe_write_roots() if safe_roots: allowed = False diff --git a/contributors/emails/andrexibiza@gmail.com b/contributors/emails/andrexibiza@gmail.com new file mode 100644 index 000000000000..8969d151632e --- /dev/null +++ b/contributors/emails/andrexibiza@gmail.com @@ -0,0 +1,2 @@ +andrexibiza +# Axl Ibiza (andrexibiza) — canonical author identity diff --git a/tests/agent/test_file_safety_git_state.py b/tests/agent/test_file_safety_git_state.py new file mode 100644 index 000000000000..1ce46467472e --- /dev/null +++ b/tests/agent/test_file_safety_git_state.py @@ -0,0 +1,122 @@ +"""Tests for the git-managed state write guard in file_safety. + +Regression for https://github.com/NousResearch/hermes-agent/issues/78793 — +``write_file`` / ``patch`` (and the other mutating file tools) could silently +rewrite git-managed state inside a normal repository's ``.git`` directory +(``HEAD``, ``index``, ``refs/``, ``objects/``, ``logs/``, ``packed-refs``, +``ORIG_HEAD``, ...). A single misdirected write to ``.git/HEAD`` replaces the +branch identity and turns a healthy checkout into an apparently empty one — +the same silent-corruption shape as the #78565 worktree-pointer guard, but +inside the git directory itself. + +These tests verify that git-managed control paths are write-denied while the +user-owned paths inside a git dir (``config``, ``hooks/*``, +``info/exclude``, ``description``) remain writable, and that normal +non-git files are untouched. +""" + +from __future__ import annotations + +import os + +import pytest + +from agent import file_safety as fs + + +@pytest.fixture() +def git_repo(tmp_path): + """A minimal normal git-style repository layout under tmp_path.""" + repo = tmp_path / "repo" + git = repo / ".git" + (git / "refs" / "heads").mkdir(parents=True) + (git / "objects").mkdir(parents=True) + (git / "logs").mkdir(parents=True) + (git / "hooks").mkdir(parents=True) + (git / "info").mkdir(parents=True) + (repo / "src").mkdir(parents=True) + (git / "HEAD").write_text("ref: refs/heads/main\n", encoding="utf-8") + (repo / "src" / "main.py").write_text("print('hi')\n", encoding="utf-8") + return repo + + +# --- git-managed control paths must be denied ------------------------------ + +@pytest.mark.parametrize("rel", [ + ".git/HEAD", + ".git/index", + ".git/packed-refs", + ".git/ORIG_HEAD", + ".git/FETCH_HEAD", + ".git/MERGE_HEAD", + ".git/CHERRY_PICK_HEAD", + ".git/REBASE_HEAD", + ".git/COMMIT_EDITMSG", + ".git/shallow", + ".git/refs/heads/x", + ".git/refs/tags/v1", + ".git/objects/ab/cd1234", + ".git/logs/HEAD", + ".git/info/refs", + ".git/info/alternates", +]) +def test_git_managed_state_is_write_denied(git_repo, rel): + target = git_repo / rel + assert fs.is_write_denied(str(target)), f"{rel} should be write-denied" + + +def test_git_dir_itself_is_write_denied(git_repo): + assert fs.is_write_denied(str(git_repo / ".git")) + + +def test_git_managed_error_message(git_repo): + err = fs.get_write_denied_error(str(git_repo / ".git" / "HEAD")) + assert err is not None + assert "denied" in err + + +# --- user-owned paths inside .git must stay writable ----------------------- + +@pytest.mark.parametrize("rel", [ + ".git/config", + ".git/description", + ".git/hooks/pre-commit", + ".git/hooks/post-commit", + ".git/info/exclude", +]) +def test_user_owned_git_paths_stay_writable(git_repo, rel): + target = git_repo / rel + assert not fs.is_write_denied(str(target)), f"{rel} should remain writable" + + +# --- non-git files and unrelated .git dirs ------------------------------- + +def test_normal_file_not_blocked(git_repo): + assert not fs.is_write_denied(str(git_repo / "src" / "main.py")) + + +def test_parent_dir_not_blocked(git_repo): + assert not fs.is_write_denied(str(git_repo)) + + +def test_non_git_dotfile_named_gitfile_not_blocked(git_repo): + # A regular file literally named ".git" (a worktree pointer, not a dir) + # is the #78565 case handled elsewhere; here we ensure a plain sibling + # directory named "notgit" is not caught. + sibling = git_repo / "notgit" / "HEAD" + sibling.parent.mkdir(parents=True, exist_ok=True) + assert not fs.is_write_denied(str(sibling)) + + +def test_dotgit_in_unrelated_subpath_not_blocked(git_repo): + # `.git` as an intermediate dir inside a data folder (not a real git repo) + # is still refused because git reserves that name, but a normal nested + # dir that merely contains "git" in the name must be untouched. + normal = git_repo / "git-config-example" / "HEAD" + normal.parent.mkdir(parents=True, exist_ok=True) + assert not fs.is_write_denied(str(normal)) + + +def test_path_with_trailing_sep_is_git_dir(git_repo): + # Ensure the guard also fires for `/.git/` with a trailing sep. + assert fs.is_write_denied(str(git_repo / ".git" / ""))