Skip to content

Commit

Permalink
TT#171400 Adjust git repos for new safe.directory behavior
Browse files Browse the repository at this point in the history
In more recent versions, Git upstream does an owner check for the
top-level directory (see git upstream commit 8959555ce), also see
https://github.blog/2022-04-12-git-security-vulnerability-announced/

This change is included in git versions >=2.30.3, >=2.31.2, >=2.34.2,
>=2.35.2 + >=2.36.0-rc2, and therefore also affects the Git package
v2.35.2-1 as present in current Debian/unstable (as of 2022-04-16).

Now due to this behavioral change, our unit tests fail with e.g.:

| err        = ('fatal: unsafe repository '
|  "('/tmp/pytest-of-root/pytest-0/test_status_build0/ngcpctl-pytest-base/ngcp-config' "
|  'is owned by someone else)\n'
|  'To add an exception for this directory, call:\n'
|  '\n'
|  '\tgit config --global --add safe.directory '
|  '/tmp/pytest-of-root/pytest-0/test_status_build0/ngcpctl-pytest-base/ngcp-config\n')
| ex         = 128

We're creating many temporary git repositories. Therefore, adding every
single repository via `git config --global --add safe.directory` as
suggested in git's error message isn't really a viable option for us.

Git upstream also recognized this, and as of git rev 0f85c4a30 it's
possible to opt-out of this check via `git config --global --add
safe.directory *`. This change is currently included only in Git
versions 2.30.4, 2.31.3, 2.32.2, 2.33.3, 2.34.3 and 2.35.3, so not
available in Debian/unstable, yet.

But nevertheless, it's not really an ideal option for us, as we don't
want to mess with $HOME/.gitconfig ever, as this might not always be
some random directory inside a testing container, but pointing to an
actual user configuration.

The underlying reason, why this issue showed up in our Github actions is
caused by the fact, that the checkout of the artifacts is running as
user (also see actions/checkout#47):

| uid=1001(runner) gid=121(docker) groups=121(docker),4(adm),101(systemd-journal)

But the docker containers are executed with root permissions in the
following steps. To properly handle this, we set the permissions of the
git repository to $UID/$GID of the executing user.

Even more tricky and worth being aware of, certain git actions might
fail due to permission issues, without telling you directly:

| root@8d1e4156f6d8:/tmp# mkdir testrepo/
| root@8d1e4156f6d8:/tmp# cd testrepo/
| root@8d1e4156f6d8:/tmp/testrepo# git init
| Initialized empty Git repository in /tmp/testrepo/.git/
| root@8d1e4156f6d8:/tmp/testrepo# chown testbuild .
| root@8d1e4156f6d8:/tmp/testrepo# git config --local user.email pytest@example.com
| fatal: --local can only be used inside a git repository
| root@8d1e4156f6d8:/tmp/testrepo# echo $?
| 128
| root@8d1e4156f6d8:/tmp/testrepo# chown root .
| root@8d1e4156f6d8:/tmp/testrepo# git config --local user.email pytest@example.com
| root@8d1e4156f6d8:/tmp/testrepo# echo $?
| 0

While at it, let's unify our git configuration, by using the following
settings for all our user configuration:

| git config --local user.email pytest@example.com
| git config --local user.name pytest

Change-Id: Icad0ea4c3daf22f17481f23b27fa17750bd623da
  • Loading branch information
mika committed Apr 19, 2022
1 parent a7f0d41 commit 82abf22
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 9 deletions.
22 changes: 18 additions & 4 deletions t/fixtures/programs.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import pytest
from os import getuid
from os import chown, getuid, getgid
from pathlib import Path
import subprocess
import sys
Expand Down Expand Up @@ -147,6 +147,10 @@ def process_pool(env, cfg, git):
src = cfg.get("ngcpcfg", key_base)
copy_tree(src, src, dst_pool)
cfg.set("ngcpcfg", key_base, str(dst_pool))

# required for git versions >=2.35.2
chown(ngcpctl_dir, getuid(), getgid())

ex, out, err = git.add("templates")
assert ex == 0
# print("{}\nstdout:\n{}stderr:{}\n".format("git add", out, err))
Expand All @@ -159,6 +163,8 @@ def process_pool(env, cfg, git):
dir_path = Path(outdir).joinpath(dir[1:])
print("create empty git repository at {}".format(dir_path))
gitrepo.extract_archive(str(EMPTY_GIT), dir_path)
# required for git versions >=2.35.2
chown(dir_path, getuid(), getgid())

def process_conf(env, cfg, git):
base = Path(cfg.get("ngcpcfg", "NGCPCTL_MAIN"))
Expand Down Expand Up @@ -199,15 +205,23 @@ def prepare_conf(env={}):
config.set("ngcpcfg", key, str(testenv[key]))

with gitrepo.in_folder(ngcpctl_dir) as git:
git.config("user.email", "fake@pytest.fake")
git.config("user.name", "pytest")
# required for git versions >=2.35.2
chown(git.root, getuid(), getgid())

# ensure we have valid user information
git.config("--local", "user.email", "pytest@example.com")
git.config("--local", "user.name", "pytest")

process_conf(testenv, config, git)
# generate NGCPCFG with config values
testenv["NGCPCFG"] = gen_cfg(config, ngcpctl_dir)
git.add(testenv["NGCPCFG"])
# ex, out, err = git.diff("HEAD")
# print("{}\nstdout:\n{}stderr:{}\n".format("git diff", out, err))
ex, out, err = git.commit("-a", "-m", "prepare_conf done")

# for debugging underlying problems like safe.directory situation
#print("debug: git show: {}\n".format(subprocess.getoutput("git show")))
print("{}\nstdout:\n{}stderr:{}\n".format("git commit", out, err))
assert ex == 0
return testenv, config["ngcpcfg"]
Expand Down Expand Up @@ -296,4 +310,4 @@ def run(helper, *args, env={}):
)
return result

return run
return run
14 changes: 11 additions & 3 deletions t/test_git.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,14 @@ def test_add_file_to_default_repo(cli, gitrepo):
src = "default-git-repository.tar.gz"

with gitrepo.from_archive(src) as git:
# configure git user
git.config("--local", "user.email", "me@example.com")
git.config("--local", "user.name", "pytest robot")
# required for git versions >=2.35.2
os.chown(git.root, os.getuid(), os.getgid())

# ensure we have valid user information
git.config("--local", "user.email", "pytest@example.com")
git.config("--local", "user.name", "pytest")

# debug information, visible only in case of errors
print("Using git {}".format(git.version))

# git status
Expand Down Expand Up @@ -39,6 +44,9 @@ def test_add_file_to_default_repo(cli, gitrepo):
def test_status_output(cli, gitrepo):
# we mock an existing repository by loading it from the default archive
with gitrepo.from_archive(gitrepo.default) as git:
# required for git versions >=2.35.2
os.chown(git.root, os.getuid(), os.getgid())

# now we work with "existing" repository with path given in git.root
with gitrepo.in_folder(git.root) as git:
ex, out, err = git.status()
Expand Down
5 changes: 3 additions & 2 deletions t/test_ngcpcfg_apply_instances.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,9 @@ def test_apply_instances_changes(ngcpcfg, ngcpcfgcli, tmpdir, gitrepo):
]
# put expected output of build_instances
with gitrepo.in_folder(base_git) as git:
git.config("user.email", "fake@pytest.fake")
git.config("user.name", "pytest")
# ensure we have valid user information
git.config("--local", "user.email", "pytest@example.com")
git.config("--local", "user.name", "pytest")
for file, dir in orig:
dst_path = base_path.joinpath(dir)
dst_path.mkdir(parents=True, exist_ok=True)
Expand Down

0 comments on commit 82abf22

Please sign in to comment.