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. 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" ;;