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

From: Rasmus Villemoes <rv@rasmusvillemoes.dk>
Date: Oct 6, 2014, 09:28 UTC
Message-ID: <878uktwnqs.fsf@rasmusvillemoes.dk>
In-Reply-To: <CAPc5daV_txE9NrwvH5VWhXK+UmE7Avy8R2QaZaX0SsTC_+TU-A@mail.gmail.com>

Junio C Hamano <gitster@pobox.com> wrote:

> (by the way, we do not do dashes in names for configuration by
> convention)

OK. Actually, I now think I'd prefer a subsection [include "safe"], but I don't have any strong preferences regarding the names.

Show 6 quoted lines

> 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.

I agree that it would be most useful to interpret relative paths as being relative to the working tree. I'm not sure what would be gained by checking for ./ and ../ components, a symlink could easily be used to circumvent that.

> 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.

One might (ab)use the feature to only use some settings from a global file, e.g.

[include "safe"]
    whitelist = !foo.*
    path = ~/extra.gitconfig

But I'm fine with forbidding absolute paths until someone actually comes with such a use case.

Show 5 quoted lines

> Another design decision we would need to make is if it should be
> allowed for a safe-included file to use safe-include directive to
> include other files. Offhand I do not think of a reason we absolutely
> need to support it, but there may be an interesting workflow enabled
> if we did so. I dunno.

After one level of safe-include, any safe-include can also be done as a normal include (but one may need to spell the path differently if the two included files are not both at the top of the working tree). One could imagine a project supplying lots of defaults and splitting those into separate files, each included from a single project.gitconfig.

Anyway, my proposal allows nesting includes and safe-includes inside safe-includes; forbidding it would just be a matter of adding a safe_include_depth == 0 check in two places. (Then safe_include_depth probably could/should be renamed in_safe_include.) I think I have a slight preference to allowing nested includes, but if absolute paths are forbidden for safe-includes, they should also be forbidden for include-inside-safe-include.

Rasmus