On Wed, Apr 27, 2022 at 01:49:32PM -0700, Junio C Hamano wrote:
> Junio C Hamano <gitster@xxxxxxxxx> writes:
>
> > If we had GIT_SAFE_DIRECTORIES that lists the safe directories (like
> > $PATH does), that would have been absolutely necessary to document
> > how it works, but GIT_CONFIG_* is merely an implementation detail of
> > how "git -c var=val" works and I am not sure if it is even a good
> > idea to hardcode how they happen to work like these tests. The only
> > thing the users should know is that GIT_CONFIG_{KEY,VALUE}_* are
> > used internally by the implementation and they should not muck with
> > it, no?
>
> I misremembered. GIT_CONFIG_COUNT and stuff are usable by end user
> scripts, but then ...
>
> > diff --git a/Documentation/config/safe.txt b/Documentation/config/safe.txt
> > index 6d764fe0cc..ae0e2e3bdb 100644
> > --- a/Documentation/config/safe.txt
> > +++ b/Documentation/config/safe.txt
> > @@ -13,8 +13,8 @@ override any such directories specified in the system config), add a
> > `safe.directory` entry with an empty value.
> > +
> > This config setting is only respected when specified in a system or global
> > -config, not when it is specified in a repository config or via the command
> > -line option `-c safe.directory=<path>`.
> > +config, not when it is specified in a repository config, via the command
> > +line option `-c safe.directory=<path>`, or in environment variables.
>
> ... this part must clarify what environment variables it is talking
> about.
>
> ... via the command line option `-c safe.directory=<path>`, or
> via the GIT_CONFIG_{KEY,VALUE} mechanism.
Well, the proposed phrasing covers all environment variables that
affect the configuration.
It doesn't feel right to explicitly list all those variables,
including GIT_CONFIG_PARAMETERS, because that vairable is such an
implementation detail that it isn't even mentioned elsewhere in the
documentation at all. OTOH, listing only some of the ignored
variables but omitting others doesn't feel quite right either, hence
the general "in environment variables".
> or something, perhaps. I actually do think it is a useful addition
> to have GIT_SAFE_DIRECTORIES environment variable that should NOT be
> ignored.
Is it safe to have such an environment variable?
So, it's quite clear why 'safe.directory' is ignored in the repository
config: it can be under control of someone else, and thus a malicious
repositoy could trivially safe-list itself. However, it's unclear (to
me) why 'git -c safe.directory=...' is ignored: 8959555cee
(setup_git_directory(): add an owner check for the top-level
directory, 2022-03-02) only states that it's ignored, but doesn't
justify why. Now, let's suppose that there was a compelling reason to
do so, e.g. in some way that I don't readily see it can be spoofed and
thus could be abused by an attacker to safe-list a malicious
repository. If that were indeed the case, then couldn't
'GIT_SAFE_DIRECTORIES=/path/... git cmd' could be spoofed and abused
just as easily?
Or to approach it from the opposite direction: if such a
GIT_SAFE_DIRECTORIES environment variable is safe, then why should we
ignore 'safe.directory' in the command line, or in the environment
for that matter?
