From: D. Ben Knoble <ben.knoble@gmail.com>
Date: Mon, 13 Oct 2025 11:59:02 GMT
Subject: Re: [PATCH 5/5] setup: allow not marking self owned repos as safe in `ensure_safe_repository()`
Message-ID: <CALnO6CBLr2iL0r+ywM4Vjw0=J2DNFv9Nhhq_PHuxt4eK=Z95ww@mail.gmail.com>
In-Reply-To: <20251013094152.23597-6-git@lohmann.sh>

On Mon, Oct 13, 2025 at 5:43 AM Michael Lohmann <git@lohmann.sh> wrote:
>
> Git considers all repositories as safe, if they are either
>  - explicitly set in "safe.directory" config, or
>  - the user owns the repo
>
> Since a user could unzip a folder they downloaded from the internet and
> unknown to them, it is a repository with malicious hooks/config, an
> attacker could easily get code execution. Even a command line prompt
> would automatically trigger this if executing `git status` after
> entering the malicious directory.
>
> Allow not to automatically treat all repos owned by the user as safe.
> This can either be done by "--assume-unsafe", the environment variable
> "GIT_ASSUME_UNSAFE" or by setting the configuration "safe.assumeUnsafe"
> in a safe context (so not the repo config, as it should not be able to
> allow list itself).
>
> Signed-off-by: Michael Lohmann <git@lohmann.sh>
> ---
> Question in setup.c: is setting the environment variable inside of
> safe_directory_cb the best way to "communicate" this result?
> Alternatively one could add a new member to the struct, but I thought
> this was not the best either...
>
>
>  Documentation/config/safe.adoc    |  9 +++++++
>  Documentation/git.adoc            | 14 ++++++++++-
>  environment.h                     |  1 +
>  git.c                             |  6 ++++-
>  setup.c                           |  9 +++++++
>  t/t0036-allow-unsafe-directory.sh | 42 +++++++++++++++++++++++++++++++
>  6 files changed, 79 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/config/safe.adoc b/Documentation/config/safe.adoc
> index 2d45c98b12..2ac5d94762 100644
> --- a/Documentation/config/safe.adoc
> +++ b/Documentation/config/safe.adoc
> @@ -60,3 +60,12 @@ which id the original user has.
>  If that is not what you would prefer and want git to only trust
>  repositories that are owned by root instead, then you can remove
>  the `SUDO_UID` variable from root's environment before invoking git.
> +
> +safe.assumeUnsafe::
> +       Boolean to indicate that the ownership of a repository should not
> +       be taken into account when checking if the repository is safe. It
> +       will prevent against accidental arbitrariy code execution

s/arbitrariy/arbitrary. (fix typo + add period)

> ++
> +To temporarily allow git execution in case of an assumed unsafe repository,
> +run the command with `--allow-unsafe`. To permanently trust this path, add
> +it to the `safe.directory` config.
> diff --git a/Documentation/git.adoc b/Documentation/git.adoc
> index 7df51c38f9..162350f3db 100644
> --- a/Documentation/git.adoc
> +++ b/Documentation/git.adoc
> @@ -14,7 +14,7 @@ SYNOPSIS
>      [-p | --paginate | -P | --no-pager] [--no-replace-objects] [--no-lazy-fetch]
>      [--no-optional-locks] [--no-advice] [--bare] [--git-dir=<path>]
>      [--work-tree=<path>] [--namespace=<name>] [--config-env=<name>=<envvar>]
> -    [--allow-unsafe]
> +    [--allow-unsafe] [--assume-unsafe]
>      <command> [<args>]
>
>  DESCRIPTION
> @@ -238,6 +238,13 @@ If you just want to run git as if it was started in `<path>` then use
>         execution by hooks or configuration settings. Equivalent to setting
>         the environment variable `GIT_ALLOW_UNSAFE=1`.
>
> +--assume-unsafe::
> +       Prevent arbitrary code execution by hooks or configuration if not
> +       executed in a "safe.directory". With setting this, filesystem ownership
> +       of the repository in question no longer satisfies to mark it as safe.
> +       Equivalent to setting `GIT_ASSUME_UNSAFE=1`. This is overwritten if
> +       `--allow-unsafe` is passed as well.

Here and later, I think you mean "overridden" not "overwritten"

-- 
D. Ben Knoble