Mark the repo as safe for Git

[abompard]

This relates to actions/checkout#760

Some tests need git to be functional (eg pre-commit checks) and since CVE-2022-24765 git checks directory ownership.

[hroncok]

Thanks, @abompard. Interesting stuff. I guess this should be OK, but I made an inline style-only suggestion.

[frenzymadness]

And it seems there is a problem when the example project is mounted as a volume inside the container.

[hroncok]

The example project has:

$ cat example_project/.git
gitdir: ../.git/modules/example_project

And that point to a nonexistent directory when mounted in the container. Apparently can be overridden by e.g. git --git-dir=~ config --global --add safe.directory $PWD

[hroncok]

Trying that.

[frenzymadness]

I hit the problem we are trying to fix here in one of my projects so I'm gonna test this PR and we can merge it then.

[frenzymadness]

This is a little bit tricky to reproduce. What I had to do locally is to add a user inside the running container, change ownership of the /src directory and then run the main entrypoint script. This way I'm able to reproduce the issue and also verify that the proposed fix does not solve it.

The problem is with the levels of git config files:

• --system is in /etc
• --global is in $HOME
• --local is in $PWD/.git

We cannot use --local because we might have that file mounted from outside of the container and we don't want to change it. We cannot use --global because the $HOME variable is not available in tox without explicit passenv=HOME and therefore git cannot find the ~/.gitconfig. So all we have and what seems to work is to use --system when we change the git config. That way the config file is created in /etc and exists only in the running container.

[frenzymadness]

I can theoretically test this in github actions if I:

• build and push the new container image to docker hub with some testing tag
• switch the action to the testing tag and push the change to some testing branch
• make my project use the testing branch of the github action

Do you think it's worth the effort?

[hroncok]

No.

[frenzymadness]

No.

And do you think this is ready to be merged?

[frenzymadness]

I can confirm that the fix solved the problem with pre-commit in one of my projects. Thanks @abompard for making it simpler for us by providing all the information.