I'm still not totally clear what's going on here. Some of the changes in 2.45.1 were judged overzealous and undone in 2.45.2. But don't know if that has anything to do with this. Anyway, git 2.45.2 is not yet in the Cygwin repositories, and I'm not sure it will be since it's not a security fix (so it might be skipped over, with some future version being packaged next).

This line tells us it will check the git-dir/repository-directory if there is no worktree, but it's certainly strange that awt/.git is a clone that is bare, it's an unusual name.

That specific fragment is:

structsafe_directory_datadata= {.path=worktree ?worktree :gitdir};

But that is only used for this:

/* * data.path is the "path" that identifies the repository and it is * constant regardless of what failed above. data.is_safe should be * initialized to false, and might be changed by the callback. */git_protected_config(safe_directory_cb,&data);returndata.is_safe;

However, that code is just for figuring out which path to look for as a value of thesafe.directory configuration variable. (It makes sense that you were looking at that: it's the part that is directly relevant to what I was saying about thesafe.directory variable in the PR description above! But I think the change may be subtler than that, and may not directly involve a change to that.)

In between the two is the ownership check, which does not referencedata:

if (!git_env_bool("GIT_TEST_ASSUME_DIFFERENT_OWNER",0)&&(!gitfile||is_path_owned_by_current_user(gitfile,report))&&(!worktree||is_path_owned_by_current_user(worktree,report))&&(!gitdir||is_path_owned_by_current_user(gitdir,report)))return1;

While none of this code has changed around the same time as the breakage that this PR fixed, nor recently, I do not know if thesafe.directory checking logic implemented throughgit_protected_config changed, though I would guess it has not--I think it's just doing a straightforward comparisong to values ofsafe.directory where it is defined in protected (not local or worktree) scopes. I also don't know if the way the effects of call that are used later changed.

More likely relevant, I think, is that I am also unsure if any of thegitfile,worktree, andgitdir parameters are passed as non-NULL values where they had beenNULL before.

So while it seems like maybe the need to add the.git directory explicitly is due to some oddity of Cygwin paths, I am not sure. Likewise, I cannot tell directly from this if any recent changes tosafe.directory-related checks in Git would have any implications for gitoxide.

I am wondering if some GitPython tests run commands in the.git directory of a non-bare repository.

At least with current versions of Git, this requires that the.git directory specifically be owned by the current user or listed as a value of thesafe.directory variable (in a protected scope), even if the working tree is a parent directory of that.git directory andgit will operate on the working tree itself due to it being listed explicitly insafe.directory:

ek@Kip:~/src$ git versiongit version 2.48.1ek@Kip:~/src$ git init ownership-experimentInitialized empty Git repository in /home/ek/src/ownership-experiment/.git/ek@Kip:~/src$ sudo chown -R ek2:ek2 ownership-experiment[sudo] password for ek:ek@Kip:~/src$ ls -ld ownership-experiment ownership-experiment/.gitdrwxrwxr-x 3 ek2 ek2 4096 Feb 20 15:45 ownership-experimentdrwxrwxr-x 6 ek2 ek2 4096 Feb 20 15:45 ownership-experiment/.gitek@Kip:~/src$ git -C ownership-experiment statusfatal: detected dubious ownership in repository at '/home/ek/src/ownership-experiment'To add an exception for this directory, call:        git config --global --add safe.directory /home/ek/src/ownership-experimentek@Kip:~/src[128]$ git config --global --add safe.directory /home/ek/src/ownership-experimentek@Kip:~/src$ git -C ownership-experiment statusOn branch mainNo commits yetnothing to commit (create/copy files and use "git add" to track)ek@Kip:~/src$ git -C ownership-experiment/.git statusfatal: detected dubious ownership in repository at '/home/ek/src/ownership-experiment/.git'To add an exception for this directory, call:        git config --global --add safe.directory /home/ek/src/ownership-experiment/.git

That was on GNU/Linux, but it is not specific to it (nor to systems with the somewhat rare relaxed 0002 umask value I have set there). It is also not specific to passing the directory to operate on using-C (which makes sense, since-C is implemented by actually changing directory).

That same experiment in Cygwin, where I create the repository with Cygwin git 2.45.1, then outside the Cygwin environment navigate to the working tree in Windows Explorer and recursively change its ownership to another user (which includes changing that of its.git subdirectory, since I do it recursively) has the same effect.

Could this behavior have been what changed? Could the working tree have been sufficient before, even when operating directly in the.git subdirectory?

Thanks for reviving this conversation.

I am entirely unsure how all this relates to GitPython, but know thatgitoxide has a completely different security model when it comes to that. It determines the ownership of configuration files so it can trust (or ignore) sections in the configuration (each section has 'trust' attached to it, based on file ownership).

I am entirely unsure how all this relates to GitPython

As far as I know, the only connection to GitPython is through its Cygwin CI workflow, which had to be changed to accommodate this. That is, I am not proposing that a bug in GitPython itself causes this or that anything in thegit/ directory should be changed related to it. Rather, I am curious if it is due to something specific to Cygwin. The Cygwin workflow breaks more often than some of the others, so if I can understand more of what is happening in it, then maybe that can be improved or at least handled more easily. To be clear, I donot think this has anything at all to do with the current breakage described in#2004.

but know thatgitoxide has a completely different security model when it comes to that. It determines the ownership of configuration files so it can trust (or ignore) sections in the configuration (each section has 'trust' attached to it, based on file ownership).

Thanks--this way of putting it clarifies a comment that puzzled me during something I'm working on in gitoxide!

Thanks for clarifying - indeed I was trying to understand if there was something actionable for GitPython and had/have no memory on how this works here. In that regard I am happy thatgitoxide won't have that problem at least as it's generally more open than Git is by default, ideally not any disadvantage.