Re: [PATCH v3 4/5] setup: allow temporary bypass of `ensure_safe_repository()` checks

[Junio C Hamano <gitster@pobox.com>]

Michael Lohmann <git@lohmann.sh> writes:

> So far, the only option to allow executing git in what it considers to
> be an "unsafe context" is to set this repository as "safe.directory". If
> a user only wants to temporarily execute one command, they would need to
> set the path as safe, execute the command and then remove the path
> again. Forgetting to do the latter would make the user vulnerable if
> this repo was changed afterwards in a malicious way.

If you want to do a one-shot thing, wouldn't ...

	$ cd $there
	$ GIT_DIR=$(pwd)/.git GIT_WORK_TREE=$(pwd) git ...

... be more or less the standard practice?  If you are at the top
level of the working tree (which is why the above example uses
$(pwd)/.git for GIT_DIR), you do not even have to specify
GIT_WORK_TREE and get away with

	$ GIT_DIR=.git git ...

In other words, the above argument does not sound like a very strong
justification.

> +--allow-unsafe::
> +	Temporarily trust the repository regardless of "safe.directory"
> +	configuration or ownership, potentially resulting in arbitrary code
> +	execution by hooks or configuration settings.

As the only justification for this new feature to exist that was
explained in the proposed log message was "one shot execution", this
command line option does look justifiable.  Even though with the
current system, you do not have to muck with configuration files and
only have to set the GIT_DIR environment variable, passing this
command line option that does not take a value may still be slightly
easier.

> + Equivalent to setting
> +	the environment variable `GIT_ALLOW_UNSAFE=1`.

But such an enviornment variable is not justified.  Setting an
engironment variable would last until you unset it, and it implies
that it is no longer a single shot use case that this new feature
targets.

> +`GIT_ALLOW_UNSAFE`::
> +	This Boolean environment variable can be set to true to skip the
> +	safety checks of "safe.directory" configuration and if the user
> +	owns the repository before potentially executing arbitrary code
> +	from hooks or config.

Please don't add this.  It has the same "Forgetting to unset the
environment variable will make the user vulnerable" downside as
temporarily editing your configuration file.

Not convinced why this feature must exist, at least not yet.