[PATCH 1/1] GitLab CI: set safe.directory exception for git outside of kas

[Felix Moessbauer]

In 70c98b32f we moved the git safedir handling from the container
entrypoint to the kas gitconfig setup, where already other CI related
aspects are handled. This is fine for git calls from inside the kas
environment, however it breaks git calls outside of kas, as there the
kas gitconfig is not in effect.

We fix it by a partial revert of the workaround in d9b6a7798. Instead of
completely disabling the safedirs security feature, we now only disable
it for the CI_PROJECT_DIR and only for the container user.

Fixes: d9b6a7798 ("Revert container: Disable git safe.directory ...")
Reported-by: Gokhan Cetin <gokhan...@siemens.com>
Signed-off-by: Felix Moessbauer <felix.mo...@siemens.com>
---
container-entrypoint | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/container-entrypoint b/container-entrypoint
index 927248db7..077e3d5c9 100755
--- a/container-entrypoint
+++ b/container-entrypoint
@@ -54,6 +54,12 @@ may also need to update the host distribution (e.g. Debian Jessie -> Stretch).
EOF
fi

+if [ -n "$CI_PROJECT_DIR" ]; then
+ # Work around for gitlab-runner not aligning checked out repo ownership
+ # with our builder user. We handle that internally in kas, but we
+ # need the exception here as well for git calls outside of kas.
+ git config --global safe.directory "$CI_PROJECT_DIR"
+fi
if [ -z "$USER_ID" ] || [ "$USER_ID" = 0 ]; then
# Not a kas-container call, or we shall run everything as root
GOSU=""
--
2.50.1

[Felix Moessbauer]

In 70c98b32f we moved the git safedir handling from the container
entrypoint to the kas gitconfig setup, where already other CI related
aspects are handled. This is fine for git calls from inside the kas
environment, however it breaks git calls outside of kas, as there the
kas gitconfig is not in effect.

We fix it by a partial revert of the workaround in d9b6a7798. Instead of
completely disabling the safedirs security feature, we now only disable

it for the CI_PROJECT_DIR and only for the container user. We further
now setup the kas gitconfig on GitLab CI in case a global config is
provided. This is needed, as the global config is prepared in the
entrypoint. The statement about "backwards compatibility" is dropped,
as we already append to the gitconfig and not overwrite it. It
was a leftover from times when the config was overwritten.

Fixes: d9b6a7798 ("Revert container: Disable git safe.directory ...")
Reported-by: Gokhan Cetin <gokhan...@siemens.com>

Tested-by: Gokhan Cetin <gokhan...@siemens.com>

Signed-off-by: Felix Moessbauer <felix.mo...@siemens.com>
---

Changes since v1:

- setup GitLab CI environment also if external .gitconfig is provided

container-entrypoint | 6 ++++++
docs/userguide/credentials.rst | 4 ++--
kas/libcmds.py | 3 +--
3 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/container-entrypoint b/container-entrypoint
index 927248db7..077e3d5c9 100755
--- a/container-entrypoint
+++ b/container-entrypoint
@@ -54,6 +54,12 @@ may also need to update the host distribution (e.g. Debian Jessie -> Stretch).
EOF
fi

+if [ -n "$CI_PROJECT_DIR" ]; then
+ # Work around for gitlab-runner not aligning checked out repo ownership
+ # with our builder user. We handle that internally in kas, but we
+ # need the exception here as well for git calls outside of kas.
+ git config --global safe.directory "$CI_PROJECT_DIR"
+fi
if [ -z "$USER_ID" ] || [ "$USER_ID" = 0 ]; then
# Not a kas-container call, or we shall run everything as root
GOSU=""

diff --git a/docs/userguide/credentials.rst b/docs/userguide/credentials.rst
index 94770bedf..720858349 100644
--- a/docs/userguide/credentials.rst
+++ b/docs/userguide/credentials.rst
@@ -62,8 +62,8 @@ for repos stored on the same server. Technically this is achieved by adding
`insteadof` entries to the ``.gitconfig`` file.

For backwards compatibility, the git rewrite rules are only added if
-``.gitconfig`` does not exist and no SSH configuration is provided (either
-via the kas ``SSH_`` variables or using ``.ssh/config``).
+no SSH configuration is provided (either via the kas ``SSH_`` variables or
+using ``.ssh/config``).

If the ``CI_REGISTRY``, ``CI_REGISTRY_USER`` and ``CI_JOB_TOKEN`` variables
are set, kas automatically creates a login file for the container
diff --git a/kas/libcmds.py b/kas/libcmds.py
index bf3d46bd6..e71c38788 100644
--- a/kas/libcmds.py
+++ b/kas/libcmds.py
@@ -340,8 +340,7 @@ class SetupHome(Command):
config['credential']['useHttpPath'] = \
os.environ.get('GIT_CREDENTIAL_USEHTTPPATH')

- if get_context().managed_env == ME.GITLAB_CI and \
- not gitconfig_host:
+ if get_context().managed_env == ME.GITLAB_CI:
ci_project_dir = self._path_from_env('CI_PROJECT_DIR')
if ci_project_dir:
logging.debug('Adding git safe.directory %s',
--
2.50.1

[Jan Kiszka]

We were shipping this behaviour not only since d9b6a7798 but actually
af6b9ae3 (4.4). Did we have the issue that the rewrites were not applied
when the should already before d9b6a7798? And why is it suddenly safe to
apply them even in the presence of a host gitconfig? I'm also thinking
of kas setups without our container.

Jan

> +no SSH configuration is provided (either via the kas ``SSH_`` variables or
> +using ``.ssh/config``).
>
> If the ``CI_REGISTRY``, ``CI_REGISTRY_USER`` and ``CI_JOB_TOKEN`` variables
> are set, kas automatically creates a login file for the container
> diff --git a/kas/libcmds.py b/kas/libcmds.py
> index bf3d46bd6..e71c38788 100644
> --- a/kas/libcmds.py
> +++ b/kas/libcmds.py
> @@ -340,8 +340,7 @@ class SetupHome(Command):
> config['credential']['useHttpPath'] = \
> os.environ.get('GIT_CREDENTIAL_USEHTTPPATH')
>
> - if get_context().managed_env == ME.GITLAB_CI and \
> - not gitconfig_host:
> + if get_context().managed_env == ME.GITLAB_CI:
> ci_project_dir = self._path_from_env('CI_PROJECT_DIR')
> if ci_project_dir:
> logging.debug('Adding git safe.directory %s',

--

Siemens AG, Foundational Technologies
Linux Expert Center

[MOESSBAUER, Felix]

I just checked the history of that code, but can't see a clear reason
why we required to have no .gitconfig. The merging definitely works.

Back in af6b9ae3, we used the system git config to inject the
git.safe.directory, and by that from kas perspective there was no
gitconfig provided. We now use the users gitconfig and by that need the
merging.

We could switch back to --system and keep the logic in kas as-is.

Felix

>
> Jan
>
> > +no SSH configuration is provided (either via the kas ``SSH_``
> > variables or
> > +using ``.ssh/config``).
> >  
> >  If the ``CI_REGISTRY``, ``CI_REGISTRY_USER`` and ``CI_JOB_TOKEN``
> > variables
> >  are set, kas automatically creates a login file for the container
> > diff --git a/kas/libcmds.py b/kas/libcmds.py
> > index bf3d46bd6..e71c38788 100644
> > --- a/kas/libcmds.py
> > +++ b/kas/libcmds.py
> > @@ -340,8 +340,7 @@ class SetupHome(Command):
> >                      config['credential']['useHttpPath'] = \
> >                         
> > os.environ.get('GIT_CREDENTIAL_USEHTTPPATH')
> >  
> > -            if get_context().managed_env == ME.GITLAB_CI and \
> > -                    not gitconfig_host:
> > +            if get_context().managed_env == ME.GITLAB_CI:
> >                  ci_project_dir =
> > self._path_from_env('CI_PROJECT_DIR')
> >                  if ci_project_dir:
> >                      logging.debug('Adding git safe.directory %s',

--
Siemens AG

Linux Expert Center
Friedrich-Ludwig-Bauer-Str. 3
85748 Garching, Germany

[Jan Kiszka]

At least we need two patches that argue clearly why the changes are
fine/needed.

Jan

[Felix Moessbauer]

In 70c98b32f we moved the git safedir handling from the container
entrypoint to the kas gitconfig setup, where already other CI related
aspects are handled. This is fine for git calls from inside the kas
environment, however it breaks git calls outside of kas, as there the
kas gitconfig is not in effect.

We fix it by a partial revert of the workaround in d9b6a7798. Instead of
completely disabling the safedirs security feature, we now only disable

it for the CI_PROJECT_DIR.

Fixes: d9b6a7798 ("Revert container: Disable git safe.directory ...")
Reported-by: Gokhan Cetin <gokhan...@siemens.com>

Signed-off-by: Felix Moessbauer <felix.mo...@siemens.com>
---

Changes since v2:

- revert changes of v1
- write git system config in entrypoint, so we can keep the logic inside kas
- further require that USER_ID is empty
- rebased onto next

Note: I successfully tested this on one of the failed pipelines from the
initial report.

Changes since v1:

- setup GitLab CI environment also if external .gitconfig is provided

container-entrypoint | 6 ++++++

1 file changed, 6 insertions(+)

diff --git a/container-entrypoint b/container-entrypoint
index b627a1d1b..8cf364540 100755
--- a/container-entrypoint
+++ b/container-entrypoint
@@ -59,6 +59,12 @@ if [ -n "$TERM" ]; then
infocmp "$TERM" > /dev/null 2>&1 || TERM=xterm
fi

+if [ -z "$USER_ID" ] && [ -n "$CI_PROJECT_DIR" ]; then

+ # Work around for gitlab-runner not aligning checked out repo ownership
+ # with our builder user. We handle that internally in kas, but we
+ # need the exception here as well for git calls outside of kas.

+ sudo git config --system safe.directory "$CI_PROJECT_DIR"

+fi
if [ -z "$USER_ID" ] || [ "$USER_ID" = 0 ]; then
# Not a kas-container call, or we shall run everything as root
GOSU=""

--
2.50.1

[Jan Kiszka]

Thanks, applied.