Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tests fail due to security vulnerability fix in git 2.38.1 #1544

Closed
Lightborne opened this issue Jan 21, 2023 · 5 comments · Fixed by #1648
Closed

Tests fail due to security vulnerability fix in git 2.38.1 #1544

Lightborne opened this issue Jan 21, 2023 · 5 comments · Fixed by #1648

Comments

@Lightborne
Copy link
Contributor

Lightborne commented Jan 21, 2023

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.

@Lightborne
Copy link
Contributor Author

Pull request that fixes this is here:

#1546

@Byron
Copy link
Member

Byron commented Jan 22, 2023

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.

@Lightborne
Copy link
Contributor Author

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?

@Byron
Copy link
Member

Byron commented Jan 23, 2023

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?

@Lightborne
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

2 participants