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

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

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

ymorin-orange
Copy link

In commit 7bb33e4 (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 7bb33e4 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.

In commit 7bb33e4 (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 7bb33e4 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.

Signed-off-by: Yann E. MORIN <yann.morin@orange.com>
@ymorin-orange
Copy link
Author

Ping? Maybe @martinsumner?

@martinsumner
Copy link
Contributor

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

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

Successfully merging this pull request may close these issues.

None yet

2 participants