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-15).

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 `safe.directory=*`.  This change
is currently included in Git versions 2.30.4, 2.31.3, 2.32.2, 2.33.3,
2.34.3 and 2.35.3 only, so not and option for the git version of
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.

Furthermore, we need to have proper author information available.
Otherwise it might be failing with `Author identity unknown [...] Please
tell me who you are` and fail with exit code 128 as well, as has been
observed in our Github's Debian packaging pipeline for sid.

While at it, let's unify our git configuration, by using the following
settings for all the 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 16, 2022
1 parent a7f0d41 commit 249842f
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 9 deletions.
26 changes: 22 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,14 @@ 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))

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

# 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 +167,10 @@ 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)
# ensure we have valid user information
git.config("--local", "user.email", "pytest@example.com")
git.config("--local", "user.name", "pytest")


def process_conf(env, cfg, git):
base = Path(cfg.get("ngcpcfg", "NGCPCTL_MAIN"))
Expand Down Expand Up @@ -199,15 +211,21 @@ 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")
# 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 +314,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 249842f

Please sign in to comment.