From caa039fdd820fa0e9e6a18b2e17f718cf5663694 Mon Sep 17 00:00:00 2001 From: Byron Date: Tue, 3 Nov 2026 05:49:55 +0100 Subject: [PATCH 1/2] fix(submodule): bind writes to the validated checkout Rubber-stamp, it's certainly good to have this test as it seems to be a real issue compared to Git, trusting everything Astra discovered. In any case, it seems like GIX-14 should be fixed sooner than later. The requested audit against Git's submodule path validation found that `Submodule.module()` checked the checkout path, then let `Repo` select a different working tree from `core.worktree`. A symlink in that configured path could redirect `update()` writes outside the checkout. Unforced `remove(configuration=False)` could delete the external working tree. Pass the checked checkout as `GIT_WORK_TREE` when opening the shared module handle. This keeps updates, removal and recursive traversal on the validated path without making repository queries rewrite configuration. Existing Git metadata symlinks remain supported. Git reference `6de20f6092dcf9bdb1c8efe03db4b70c82b423dd` validates checkout components in `submodule.c:validate_submodule_path()` and repairs configured worktrees before update in `builtin/submodule--helper.c:ensure_core_worktree()`. Git 2.54.0 preserves external targets in the new update/deinit comparisons. GitPython previously overwrote or deleted them. The regressions cover leaf and intermediate symlinks, and pass on both CLI and Gix backends after the fix. Record the independently observed GixPython 0.3.0 configured-worktree canonicalization difference under the still-open `GIX-14` ledger entry. The scoped submodule guard does not claim to fix native discovery upstream. Assisted-by: GPT 6.0 Astra Co-authored-by: GPT 6.0 Astra --- doc/gix-backend.md | 12 +++++++++++ git/objects/submodule/base.py | 3 ++- test/test_submodule.py | 30 ++++++++++++++++++++++++++++ tests/fixtures/submodule-symlinks.sh | 19 ++++++++++++++++++ 4 files changed, 63 insertions(+), 1 deletion(-) diff --git a/doc/gix-backend.md b/doc/gix-backend.md index 49a6ae42d..2d9834987 100644 --- a/doc/gix-backend.md +++ b/doc/gix-backend.md @@ -968,6 +968,18 @@ identify the historical fixture where needed. | GIX-33 | Bug (open); missing exact-name mode | With Git 2.54.0 and GixPython 0.3.0, `exact-refs.sh sha1 files symbolic` makes `refs/tags/REBASE_HEAD` a symbolic ref to another tag while the root `REBASE_HEAD` is absent. `Repository.try_find_reference("REBASE_HEAD")` returns that tag; native `_symbolic_ref` formerly returned its target, whereas `git symbolic-ref --quiet --no-recurse -- REBASE_HEAD` exits 1. GitPython's root-reference object read consequently returned a commit through Gix and raised `ValueError` through CLI. SHA-256 reproduces the same mismatch. | Expose an exact-name lookup mode, including root references, whose absence and error behavior matches Git. `_symbolic_ref()` compares the native reference's name with the requested bytes and selects CLI for a lookalike. `test_missing_root_reference_does_not_follow_symbolic_lookalike_tag` verifies parity and forced CLI fallback. This adapter guard does not close the upstream compatibility bug. | | GIX-34 | Bug (open) | With Git 2.54.0 and GixPython 0.3.0, an author, committer or tagger line with fields after its time zone, such as `A 1112591163 +0200 1700000000 -0500`, decodes through `Commit.author()`/`committer()` and `Tag.tagger()` with the correct seconds but an offset of 0. Git reads only the first date and time zone after the last `>` (`split_ident_line`) and reports `1112591163 +0200` through `git show --format=%ad --date=raw` and `%(taggerdate:raw)`. `git fsck` reports `badTimezone` for such objects, but the default `transfer.fsckObjects=false` accepts them. The `trailing-date` read-only `commit.sh` fixture in `test_deserialization_reads_date_after_email` reproduces both object kinds. | Report the time zone Git reads for identity lines with trailing fields, or reject them consistently. `object_metadata()` selects CLI when the decoded signature does not reproduce the end of its raw header line. This adapter guard does not close the upstream compatibility bug. | +GIX-14 also covers configured worktree symlink resolution. On macOS with +GixPython 0.3.0, CPython 3.12.14 and Git 2.54.0, set a submodule's +`core.worktree` to `parent/redirect`, where `redirect` links to `outside`. +`Repository.workdir()` retains `parent/redirect`; Git's +`rev-parse --show-toplevel` returns `outside`. A trailing `/worktree` reproduces +the same difference for an intermediate symlink. Opening the canonical gitfile +or reopening the canonical native Git directory does not resolve it. +`test_submodule_writes_use_checked_worktree` exercises both layouts: submodule +handles now pass the validated checkout as `GIT_WORK_TREE` to prevent redirected +writes in either backend. General native discovery still needs Git-compatible +configured-worktree resolution; this guard does not close GIX-14. + The PR #2306 regression exposed an adapter ownership-check bypass in both backends: `Repo`'s automatic `GIT_DIR` made Git 2.54.0 treat hook execution as explicitly selecting the repository, so it ran a hook where an ordinary diff --git a/git/objects/submodule/base.py b/git/objects/submodule/base.py index 408ed71d0..0c24dbc87 100644 --- a/git/objects/submodule/base.py +++ b/git/objects/submodule/base.py @@ -1727,7 +1727,8 @@ def module(self) -> "Repo": module_checkout_abspath = self.abspath self._module_abspath(self.repo, self.path, self.name) try: - repo = git.Repo(module_checkout_abspath) + # Keep core.worktree from redirecting writes past the validated checkout. + repo = git.Repo(module_checkout_abspath, _env={"GIT_WORK_TREE": os.fspath(module_checkout_abspath)}) if repo != self.repo: return repo # END handle repo uninitialized diff --git a/test/test_submodule.py b/test/test_submodule.py index 0ed276e4a..c2620e9d9 100644 --- a/test/test_submodule.py +++ b/test/test_submodule.py @@ -949,6 +949,36 @@ def test_submodule_rejects_checkout_and_gitmodules_symlinks(configured_submodule assert (root / "moved").is_dir() +@pytest.mark.parametrize("implementation", ["git", "gitpython"]) +@pytest.mark.parametrize("operation", ["update", "remove"]) +@pytest.mark.parametrize("kind", ["leaf", "intermediate"]) +def test_submodule_writes_use_checked_worktree(configured_submodule, implementation, operation, kind): + """A configured worktree must not redirect writes past the checked checkout.""" + sm = configured_submodule + root = Path(sm.repo.working_tree_dir) + outside = root.parent / "outside" + if kind == "intermediate": + outside /= "worktree" + before = {path.name: path.read_bytes() for path in outside.iterdir()} + if implementation == "git": + if operation == "update": + sm.repo.git.submodule("update", "--force", "--no-fetch", "--", sm.path) + else: + sm.repo.git.submodule("deinit", "--force", "--", sm.path) + elif operation == "update": + sm.update(force=True, no_fetch=True) + else: + sm.remove(configuration=False) + + assert outside.is_dir() + assert {path.name: path.read_bytes() for path in outside.iterdir()} == before + assert (root / "redirect").is_symlink() + if operation == "update": + assert (root / "module/file").read_text() == "updated" + else: + assert not (root / "module/file").exists() + + def test_add_closes_checkout_processes(movable_submodule, monkeypatch): """Adding a submodule must not leave a child process holding its checkout open.""" # Native reads have no persistent process; keep exercising the CLI fallback. diff --git a/tests/fixtures/submodule-symlinks.sh b/tests/fixtures/submodule-symlinks.sh index 8ee0e25f2..3b2b5edd8 100644 --- a/tests/fixtures/submodule-symlinks.sh +++ b/tests/fixtures/submodule-symlinks.sh @@ -10,6 +10,25 @@ relative_target=${args[6]} modules="$root/parent/.git/modules" target="$root/outside" case "$scenario" in + test_submodule_writes_use_checked_worktree) + if test "${args[5]}" = update; then + printf updated > parent/module/file + git -C parent/module add file + git -C parent/module commit -qm 'Advance submodule' + git -C parent add module + git -C parent commit -qm 'Record updated submodule' + git -C parent/module reset -q --hard HEAD^ + fi + mkdir "$target" + ln -s "$target" parent/redirect + worktree="$root/parent/redirect" + if test "$kind" = intermediate; then + mkdir "$target/worktree" + worktree="$worktree/worktree" + fi + printf content > "$worktree/file" + git -C parent/module config core.worktree "$worktree" + ;; test_move_leaf_symlink_compatibility) case "$kind" in empty|nonempty) mkdir "$target" ;; From 8439407277ad503d356c564a69c33741b6fdc219 Mon Sep 17 00:00:00 2001 From: Codex Date: Tue, 3 Nov 2026 08:08:09 +0100 Subject: [PATCH 2/2] docs(security): inherit Git's threat model explicitly `SECURITY.md` described supported versions and vulnerability reporting without defining which repository inputs are trusted. This left the distinction between untrusted repository content and locally supplied administration data unclear. Document that both GitPython backends inherit Git's threat model directly. Summarize the trust placed in local configuration and hooks, the limits of ownership checks and filesystem confinement, and Git's guidance for obtaining a clean copy of an untrusted repository. Link to Git's documentation on GitHub and keep violations of its intended security boundaries reportable, including bugs that also affect Git. Checked the summary against the local Git reference at `6de20f6092dcf9bdb1c8efe03db4b70c82b423dd`. `git diff --check` passes. No runtime tests were run because this change only updates documentation. Assisted-by: GPT 6.0 Astra Co-authored-by: GPT 6.0 Astra --- SECURITY.md | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/SECURITY.md b/SECURITY.md index 0aea34845..e8e777aa0 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -9,6 +9,37 @@ Only the latest version of GitPython can receive security updates. If a vulnerab | 3.x.x | :white_check_mark: | | < 3.0 | :x: | +## Threat Model + +GitPython inherits its threat model directly from Git. The trust boundaries in +[Git's security documentation](https://github.com/git/git/blob/master/Documentation/git.adoc#security) +apply to both the CLI and Gix backends: + +- **Repository content may be untrusted.** Cloning an untrusted repository and + inspecting its history are supported operations. A clone creates local + repository metadata without copying the source repository's configuration or + hooks. Tracked files, including `.gitmodules`, remain untrusted input. +- **Local repository metadata must be trusted.** Running commands in an + attacker-supplied `.git` directory, bare repository, or surrounding worktree is + unsafe: configuration and hooks can execute commands. This includes repositories + received in archives or copied directories. Owning the extracted files or listing + a repository in `safe.directory` does not make its contents trustworthy. +- **Repository paths are not a sandbox.** Git supports metadata and working trees + in [separate locations](https://github.com/git/git/blob/master/Documentation/git-clone.adoc), + including metadata reached through gitfiles or symlinks. + GitPython does not promise that all filesystem access stays within a checkout + or `.git` directory. Paths supplied by untrusted repository content must still + receive Git's intended protections. + +For an untrusted repository directory, Git recommends obtaining a clean copy with +`git clone --no-local` before running other commands. Serving the source through +`git-upload-pack` still has an attack surface; Git recommends using an +unprivileged user for additional isolation. See +[Git's `git-upload-pack` security guidance](https://github.com/git/git/blob/master/Documentation/git-upload-pack.adoc#security). + +Violations of these trust boundaries or bypasses of Git's intended protections +remain reportable, including cases where Git itself is affected. + ## Reporting a Vulnerability Please report private portions of a vulnerability to . Doing so helps to receive updates and collaborate on the matter, without disclosing it publicly right away.