From 82abf22d10f8e2f16f86cf2a4da41aa3aaf9da4b Mon Sep 17 00:00:00 2001 From: Michael Prokop Date: Sat, 16 Apr 2022 03:48:02 +0200 Subject: [PATCH] TT#171400 Adjust git repos for new safe.directory behavior 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 https://github.com/actions/checkout/issues/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 --- t/fixtures/programs.py | 22 ++++++++++++++++++---- t/test_git.py | 14 +++++++++++--- t/test_ngcpcfg_apply_instances.py | 5 +++-- 3 files changed, 32 insertions(+), 9 deletions(-) diff --git a/t/fixtures/programs.py b/t/fixtures/programs.py index 22511487..78a7b260 100644 --- a/t/fixtures/programs.py +++ b/t/fixtures/programs.py @@ -1,5 +1,5 @@ import pytest -from os import getuid +from os import chown, getuid, getgid from pathlib import Path import subprocess import sys @@ -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)) @@ -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")) @@ -199,8 +205,13 @@ 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) @@ -208,6 +219,9 @@ def prepare_conf(env={}): # 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"] @@ -296,4 +310,4 @@ def run(helper, *args, env={}): ) return result - return run \ No newline at end of file + return run diff --git a/t/test_git.py b/t/test_git.py index 293b90d1..2e17c84e 100644 --- a/t/test_git.py +++ b/t/test_git.py @@ -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 @@ -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() diff --git a/t/test_ngcpcfg_apply_instances.py b/t/test_ngcpcfg_apply_instances.py index a3ad5132..e7e4c9d3 100644 --- a/t/test_ngcpcfg_apply_instances.py +++ b/t/test_ngcpcfg_apply_instances.py @@ -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)