GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — fix(index): let git check the repository before running a commit hook by Keerthana-64 · Pull Request #2306 · gitpython-developers/GitPython

Conversation

@Keerthana-64

@Keerthana-64 Keerthana-64 commented Oct 23, 2026

Copy link
Copy Markdown
Contributor

Repro: in a repository git refuses as "dubious ownership" (simulated with GIT_TEST_ASSUME_DIFFERENT_OWNER=1), repo.index.commit("x") runs .git/hooks/pre-commit and only then fails in git hash-object, while git commit refuses without running any hook.
Cause: run_commit_hook finds and executes the hook without calling git, and IndexFile.commit runs pre-commit and commit-msg before its first git command, so git's safe.directory check (CVE-2022-24765) comes after code from the repository has already run.
Fix: run git rev-parse --git-dir before executing an existing hook so git applies its own ownership policy. Repositories git accepts behave as before, and the new test in test/test_index.py fails before the change and passes after.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@Keerthana-64
fix(index): let git check the repository before running a commit hook
cdfb428
`run_commit_hook` finds a hook in `.git/hooks` or `core.hooksPath`, read
through GitPython's own config parser, and executes it directly. No git
command runs before that, so git's check that the repository is safe to
use (the ownership and `safe.directory` check added for CVE-2022-24765)
has not happened when code from the repository is executed.

`IndexFile.commit` runs the `pre-commit` and `commit-msg` hooks before
it calls git at all. In a repository owned by another user, where
`git commit` stops with "detected dubious ownership" without running any
hook, `repo.index.commit()` ran the repository's `pre-commit` hook and
only then failed in `git hash-object`. That is the situation git's check
exists for: a `.git` directory planted by another user in a shared or
parent directory (`Repo(path, search_parent_directories=True)` finds it
the same way git would).

Before executing a hook that exists, `run_commit_hook` now runs
`git rev-parse --git-dir` in the repository, so git applies its own
policy, including `safe.directory` and the exemptions git makes. When
git refuses, the `GitCommandError` it raises is the one the commit
already ended with, only now before the hook runs. When git accepts the
repository, hooks run as before, and nothing changes when there is no
hook. Asking git, rather than reimplementing the ownership rules, keeps
the result identical to `git commit` for every git version, including
versions older than 2.35.2 that make no check.

Adds `test_pre_commit_hook_not_run_in_repository_git_refuses` to
`test/test_index.py`, which uses git's `GIT_TEST_ASSUME_DIFFERENT_OWNER`
to simulate another owner; it fails before this change and passes
after. `test_run_commit_hook` and
`test_run_commit_hook_outside_worktree_on_windows` mock `Git.execute`
(the latter because it fakes the Windows `PATH`), so they now allow this
one call while still rejecting any git command during hook lookup.

`ruff check`, `ruff format --check` and `mypy` pass. In a full clone
prepared with `init-tests-after-clone.sh`, the pytest suite passes
except `test_diff_with_staged_file`, `test_index_mutation`,
`test_blame_content_with_carriage_returns`, `test_empty_repo`,
`test_rebasing` and the two `test_actor_get_uid_laziness_*` tests,
which fail the same way without this change on this machine.
Byron pushed a commit that referenced this pull request Oct 24, 2026
@codex
fix(index): check repository ownership before commit hooks
75a9276
Port PR #2306's regression against direct `git commit` and `git hook`
invocations. Before this fix, `Repo`'s automatic `GIT_DIR` binding made
Git treat the repository as explicitly selected: `IndexFile.commit()`
ran its pre-commit hook even when ordinary Git rejected dubious
ownership. The regression fails on both backends without the fix.

Retain the original discovery location and run `git rev-parse --git-dir`
before executing any commit hook, removing only the automatically bound
environment values. Remembering discovery supports external
`core.worktree` locations while matching `safe.directory` on ordinary,
bare, linked and separate-directory repositories. Explicit environment
or command-line repository selections retain Git's behavior, including
relative paths and consumption of one-shot options after failures.
Validate NUL arguments before starting either process.

The fixture tests verify rejection without hook side effects, Git's
safe-directory exemptions, and explicit repository-selection behavior.
Update the native-backend coverage test and GIX-23 documentation to
require two CLI processes per valid hook request, including missing
hooks. No independent ownership policy or native hook implementation
is introduced.

Validation: before the final option-preservation refinements, the full
index, repository and backend modules passed (CLI: 236 tests, 106 skips;
Gix: 338 tests, 4 skips; each with 19 passing subtests). Final focused
hook coverage passes with CLI (12 tests, 2 skips) and Gix (14 tests),
each with 9 passing subtests. A separate review independently reran the
ownership and repository-selection regressions on both backends.
`ruff check`, targeted `mypy`, fixture `bash -n` and `git diff --check`
pass. Windows and Linux were not run locally.
Byron pushed a commit that referenced this pull request Oct 24, 2026
@codex @Byron
fix(index): check repository ownership before commit hooks
0709904
Port PR #2306's regression against direct `git commit` and `git hook`
invocations. Before this fix, `Repo`'s automatic `GIT_DIR` binding made
Git treat the repository as explicitly selected: `IndexFile.commit()`
ran its pre-commit hook even when ordinary Git rejected dubious
ownership. The regression fails on both backends without the fix.

Retain the original discovery location and run `git rev-parse --git-dir`
before executing any commit hook, removing only the automatically bound
environment values. Remembering discovery supports external
`core.worktree` locations while matching `safe.directory` on ordinary,
bare, linked and separate-directory repositories. Explicit environment
or command-line repository selections retain Git's behavior, including
relative paths and consumption of one-shot options after failures.
Validate NUL arguments before starting either process.

The fixture tests verify rejection without hook side effects, Git's
safe-directory exemptions, and explicit repository-selection behavior.
Update the native-backend coverage test and GIX-23 documentation to
require two CLI processes per valid hook request, including missing
hooks. No independent ownership policy or native hook implementation
is introduced.

Validation: before the final option-preservation refinements, the full
index, repository and backend modules passed (CLI: 236 tests, 106 skips;
Gix: 338 tests, 4 skips; each with 19 passing subtests). Final focused
hook coverage passes with CLI (12 tests, 2 skips) and Gix (14 tests),
each with 9 passing subtests. A separate review independently reran the
ownership and repository-selection regressions on both backends.
`ruff check`, targeted `mypy`, fixture `bash -n` and `git diff --check`
pass. Windows and Linux were not run locally.
@Byron

Byron commented Oct 24, 2026

Copy link
Copy Markdown
Member

Codex, operating on behalf of Byron: This PR has been superseded by #2274. Its regression coverage has been integrated there, with assertions checked against Git’s behavior and fixes included where needed.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Byron Byron closed this Oct 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@Keerthana-64 @Byron
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.