Description

Lightborne

opened on Jan 21, 2023

Issue body actions

Hello,
Due to a change made in Git to address a security vulnerability, some tests are failing.

See here for a description of the change:

https://github.blog/2022-10-18-git-security-vulnerabilities-announced/#cve-2022-39253

These are the failing tests:

• test_list_only_valid_submodules
• test_git_submodules_and_add_sm_with_new_commit

The fail signature is the same in both cases:

cmdline: git submodule add /[redacted]/GitPython/git/ext/gitdb/gitdb/ext/smmap module
stderr: 'Cloning into '/tmp/test_list_only_valid_submoduleshv3nprno/parent/module'...
fatal: transport 'file' not allowed
fatal: clone of '/[redacted]/GitPython/git/ext/gitdb/gitdb/ext/smmap' into submodule path '/tmp/test_list_only_valid_submoduleshv3nprno/parent/module' failed'

Here is a blog post discussing this issue affecting others:

https://vielmetti.typepad.com/logbook/2022/10/git-security-fixes-lead-to-fatal-transport-file-not-allowed-error-in-ci-systems-cve-2022-39253.html

I have fixed this locally by changing the submodule add command in each test from:

repo.git.submodule("add", self._small_repo_url(), "module")

to

repo.git.submodule("add", Git.polish_url("https://github.com/gitpython-developers/smmap.git"), "module")

If this is an acceptable fix I can provide it in a pull request.

Activity

1. Lightborne commented on Jan 21, 2023

   

   Lightborne

   on Jan 21, 2023

   ContributorAuthor

   More actions

   Pull request that fixes this is here:

   #1546
2. Byron commented on Jan 22, 2023

   

   Byron

   on Jan 22, 2023

   Member

   More actions

   I wonder why the fix applied on CI isn't feasible for you.

   I'd like to avoid cloning from a remote repository, and prefer a local fix. Maybe the git configuration change that is currently applied on CI can be enforced before running any GitPython test, using some sort of global initialization? Git configuration can be affected through environment variables for example, which would certainly work here.
3. Byron

   added

   waiting for feedback

   on Jan 22, 2023
4. Lightborne commented on Jan 23, 2023

   

   Lightborne

   on Jan 23, 2023

   ContributorAuthor

   More actions

   I think you made the right call declining my PR - it was a hacky fix.

   Maybe the git configuration change that is currently applied on CI can be enforced before running any GitPython test, using some sort of global initialization?

   Would this solution modify a user's git configuration without them knowing? I don't know how I feel about that.

   If it's possible to probe a user's global configuration, can it be used as a condition to run the test or not?
5. Byron commented on Jan 23, 2023

   

   Byron

   on Jan 23, 2023

   Member

   More actions

   Would this solution modify a user's git configuration without them knowing? I don't know how I feel about that.

   Indeed, that's not what I am proposing. It's possible to set git configuration using environment variables, they could be set for the entire test process and alleviate the need for the caller to configure anything.

   Do you think that could be done as initialization for pytest?
6. Lightborne commented on Jan 23, 2023

   

   Lightborne

   on Jan 23, 2023

   ContributorAuthor

   More actions

   Ah! I see what you mean now. That would be a better solution for sure. I will see if I can get that to work on my end.

   Reacted by Sebastian Thiel
7. Byron

   added

   acknowledged

   and removed

   waiting for feedback

   on Jan 24, 2023
8. Byron

   closed this as completedin #1648on Sep 7, 2023
9. Byron

   added this to the v3.1.35 - Bugfixes milestone on Sep 7, 2023

Issue actions