[RFC] bitbake.conf: mark all directories as safe for git to read

Recent git releases containing [1] have an ownership check when opening
repositories, and refuse to open a repository if it is owned by a
different user.

This breaks any use of git in do_install, as that is executed by the
(fake) root user. Whilst not common, this does happen.

Setting the git configuration safe.directories=* disables this check, so
that git is usable in fakeroot tasks.  This can be set globally via the
internal environment variable GIT_CONFIG_PARAMETERS, we can't use
GIT_CONFIG_*_KEY/VALUE as that isn't present in all the releases which
have the ownership check.

We already set GIT_CEILING_DIRECTORIES to ensure that git doesn't
recurse up out of the work directory, so this isn't a security issue.

[1] https://github.com/git/git/commit/8959555cee7ec045958f9b6dd62e541affb7e7d9

Signed-off-by: Ross Burton <ross.burton@arm.com>
---
 meta/conf/bitbake.conf | 8 ++++++++
 1 file changed, 8 insertions(+)

I propose we merge this fix and revert the git intercept changes that are current in master/master-next.

Whilst a global export is quite a brute force solution, it’s also surgically precise in what it does.  Improvements to how git works in devshell and other context can be done after release, not when we’re trying to get a release out of the door.

Ross

> On 26 Apr 2022, at 13:07, Ross Burton via lists.openembedded.org <ross.burton=arm.com@lists.openembedded.org> wrote:
>
> Recent git releases containing [1] have an ownership check when opening
> repositories, and refuse to open a repository if it is owned by a
> different user.
>
> This breaks any use of git in do_install, as that is executed by the
> (fake) root user. Whilst not common, this does happen.
>
> Setting the git configuration safe.directories=* disables this check, so
> that git is usable in fakeroot tasks.  This can be set globally via the
> internal environment variable GIT_CONFIG_PARAMETERS, we can't use
> GIT_CONFIG_*_KEY/VALUE as that isn't present in all the releases which
> have the ownership check.
>
> We already set GIT_CEILING_DIRECTORIES to ensure that git doesn't
> recurse up out of the work directory, so this isn't a security issue.
>
> [1] https://github.com/git/git/commit/8959555cee7ec045958f9b6dd62e541affb7e7d9
>
> Signed-off-by: Ross Burton <ross.burton@arm.com>
> ---
> meta/conf/bitbake.conf | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf
> index 0e939aca4f..1deba8d910 100644
> --- a/meta/conf/bitbake.conf
> +++ b/meta/conf/bitbake.conf
> @@ -776,10 +776,18 @@ export PKG_CONFIG_DISABLE_UNINSTALLED = "yes"
> export PKG_CONFIG_SYSTEM_LIBRARY_PATH = "${base_libdir}:${libdir}"
> export PKG_CONFIG_SYSTEM_INCLUDE_PATH = "${includedir}"
>
> +# Git configuration
> +
> # Don't allow git to chdir up past WORKDIR so that it doesn't detect the OE
> # repository when building a recipe
> export GIT_CEILING_DIRECTORIES = "${WORKDIR}"
>
> +# Treat all directories are safe, as during fakeroot tasks git will run as
> +# root so recent git releases (eg 2.30.3) will refuse to work on repositories. See
> +# https://github.com/git/git/commit/8959555cee7ec045958f9b6dd62e541affb7e7d9 for
> +# further details.
> +export GIT_CONFIG_PARAMETERS="'safe.directory=*'"
> +
> ###
> ### Config file processing
> ###
> --
> 2.25.1
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#164868): https://lists.openembedded.org/g/openembedded-core/message/164868
> Mute This Topic: https://lists.openembedded.org/mt/90706188/6875888
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [ross.burton@arm.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

On Tue, 2022-04-26 at 12:42 +0000, Ross Burton wrote:
> I propose we merge this fix and revert the git intercept changes that are current in master/master-next.
> 
> Whilst a global export is quite a brute force solution, it’s also surgically
> precise in what it does.  Improvements to how git works in devshell and other
> context can be done after release, not when we’re trying to get a release out
> of the door.

I'm not convinced that git is ever going to work well under "fake" root
permissions as people have complained it won't see their local git config, e.g.
for user/email or potentially other credential information.

It is unusual for people to run git under fakeroot tasks but some builds do,
e.g. meson.

The export is more precise but also misses the improvements under devshell that
the intercept offers. I can see arguments for retaining the intercept and just
reverting my master patch to make it global.

The downside to the export is that it will "bloat" all shell task siginfo files,
we really want to be cleaning up the global environment, not adding to it :(. It
leaves differences in behaviour for git under fakeroot.

I'm quite torn on this. I think we have the intercept working in master-next
FWIW. The changes are going to need backporting to all active releases though
which there is an easier case for the export for.

Cheers,

Richard