basho / eleveldb

Erlang LevelDB API
263 stars 176 forks source link

c_src: do not tinker with user's ~/.gitconfig #279

Open ymorin-orange opened 9 months ago

ymorin-orange commented 9 months ago

In commit 7bb33e45b76c (take git action as suggested on an "unsafe directory"), git was told that the directory with the source tree was safe to use.

What safe.directory means, is that directories that are owned by another user are safe to use. This is meant to be used by teams that share a common git tree. In such a situation, git by default refuses to use that directory, and also refuses to run any hook from that directory; see "man git-config" for the details and rationale.

In the case of github workflows, the user that runs the git checkout is not the same as the one that actually runs the job (supposedly the checkout is done outside the container, while the job is done inside).

However, commit 7bb33e45b76c got it slightly wrong on three counts:

  1. the directory added to the safe list is hard-coded, which means it is most probably unfit for the local setup; furthermore, it is not robust in the face of Github changing the location where it exposes the git tree in workflow jobs;

  2. tweaking the git configuration is done from the Makefile, which means it changes the git config of the user running the build; it is very bad practice to do so, and may conflict with the security policy for that user;

  3. the modification is done unconditionally, which leads to an arbitrary number of entries added to the safe list: each time the build is attemped, the directory is added to the safe list, causing an ever-growing list of exactly the same value.

The correct solution is two fold:

  1. when building locally, users will (most probably) have cloned the repository, so they own the local clone, and thus do not need the safe entry; if they are using a shared repository, they already know what to do;

  2. in the Github workflow, add a step to effectively disable the safety check for the directory where the checkout was done, rather than hard-coding an arbitrary location.

ymorin-orange commented 6 months ago

Ping? Maybe @martinsumner?

martinsumner commented 6 months ago

@nsaadouni and the team at bet365 are now looking after all repos in the basho org