Re: [RFC/PATCH 1/2] config: Add safe-include directive

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 3, 2014, 05:27 UTC
Message-ID: <CAPc5daV_txE9NrwvH5VWhXK+UmE7Avy8R2QaZaX0SsTC_+TU-A@mail.gmail.com>
In-Reply-To: <1412300254-11281-2-git-send-email-rv@rasmusvillemoes.dk>

On Thu, Oct 2, 2014 at 6:37 PM, Rasmus Villemoes <rv@rasmusvillemoes.dk> wrote:

Show 16 quoted lines

> This adds a variant of the include directive, where only certain
> config variables in the included files are honoured. The set of
> honoured variables consists of those the user has mentioned in a
> safe-include.whitelist directive, along with a small set of git.git
> blessed ones.
>
> This can, for example, be used by a project to supply a set of
> suggested configuration variables, such as "diff.renames = true". The
> project would provide these in e.g project.gitconfig, and the user then
> has to explicitly opt-in by putting
>
> [safe-include]
>     path = ../project.gitconfig
>
> into .git/config, possibly preceding the path directive with a
> whitelist directive.

Good thinking to protect against accidental inclusion by older versions of Git.

Even though I did allude to ../project.gitconfig in the original message, I think there should probably be an explicit syntax to name a path that is relative to the root level of the working tree. People do funky things using $GIT_DIR and $GIT_WORK_TREE to break the ".. relative to the config file is the root level of the working tree" assumption, and also a repository can have a regular file ".git" that points at the real location of the directory that has "config" in it, in which case its parent directory is very unlikely to be the root level of the working tree.

That syntax _could_ be just a relative path (e.g. project.gitconfig names the file with that name at the top-level of the working tree), and if we are to do so, we should forbid any relative path that escapes from the working tree (e.g. ../project.gitconfig is forbidden, but down/down/../../.gitconfig could be OK as it is the same as .gitconfig). For that matter, anything with /./ and /../ in it can safely be forbidden without losing functionality.

The reason why I think it is sufficient to take a relative path as relative to the working tree is primarily because I do not see a reason why we would want to do a safer inclusion of anything inside $GIT_DIR (which would be the natural interpretation if the relatigve path is taken as relative to the including config file, in the same way as how the regular include is processed). But I could be missing some other useful use cases.

And we can allow absolute path, e.g. /etc/gitconfig, of course, but I'd prefer to at least initially forbid an absolute path, due to the same worries I have against the "unset some variables defined in /etc/gitconfig" topic we discussed earlier today in a separate thread.