[Best Practices Request] clean/smudge configuration

[Best Practices Request] clean/smudge configuration

[Randall S. Becker]

Hi Git Team,

I'm trying to work out some best practices for managing clean/smudge filters
and hit a bump. The situation is that I have an environment where the
possible clean/smudge filter configuration can change over time and needs to
be versioned with the product being managed in a git repository. Part of the
configuration is no problem in .gitattributes, but the other bits are in
.git/config. I get that the runnable part of the filters need to be strictly
platform independent in principle, but I can abstract that part in this
situation.

The question: what is the best practice for versioning the parts of
clean/smudge filters that are in .git/config given that only some users in
my environment will be cloning the repository in question and that I really
can't put the entries in /etc/gitconfig or ~/.gitconfig because of potential
conflicts with other repositories that might also have clean/smudge
definitions.

Thanks,
Randall 

Re: [Best Practices Request] clean/smudge configuration

[Bryan Turner]

On Wed, May 9, 2018 at 3:09 PM Randall S. Becker <rsbecker@nexbridge.com>
wrote:

> The question: what is the best practice for versioning the parts of
> clean/smudge filters that are in .git/config given that only some users in
> my environment will be cloning the repository in question and that I
really
> can't put the entries in /etc/gitconfig or ~/.gitconfig because of
potential
> conflicts with other repositories that might also have clean/smudge
> definitions.

Depending on level of trust, one approach might be to use an [include] in
.git/config to include a file that's in the repository. Something like:

[include]
     path = ../path/to/config

That way that part of the configuration can be derived from the
repository's contents. If you checkout a revision where the file doesn't
exist, then Git just ignores the include.

You're trusting the repository's contents, though, at that point; whatever
is in that file will be included in your overall config and honored by Git.

Bryan

RE: [Best Practices Request] clean/smudge configuration

[Randall S. Becker]

On May 9, 2018 6:39 PM, Bryan Turner wrote
> 
> On Wed, May 9, 2018 at 3:09 PM Randall S. Becker
> <rsbecker@nexbridge.com>
> wrote:
> 
> > The question: what is the best practice for versioning the parts of
> > clean/smudge filters that are in .git/config given that only some
> > users in my environment will be cloning the repository in question and
> > that I
> really
> > can't put the entries in /etc/gitconfig or ~/.gitconfig because of
> potential
> > conflicts with other repositories that might also have clean/smudge
> > definitions.
> 
> Depending on level of trust, one approach might be to use an [include] in
> .git/config to include a file that's in the repository. Something like:
> 
> [include]
>      path = ../path/to/config

It's a possibility, but I don't like the implications. Files that are subject to the clean/smudge would need to be reprocessed manually. In the scenario:

1. A checkout is done, changing ../path/to/config.
2. The clean/smudge configuration changes in ../path/to/config, but the files impacted by it do not.
3. git does not look like it would not be aware of the change until after the checkout, which is too late.
4. The work tree is now inconsistent with the idempotency of the clean/smudge rules, basically because nothing happened. (not blaming git here, just timing).

As far as I understand, this is a bit of a chicken-and-egg problem because the clean/smudge config needs to be there before the checkout. Correct?

Cheers,
Randall

RE: [Best Practices Request] clean/smudge configuration

[Randall S. Becker]

On May 9, 2018 6:39 PM, Bryan Turner wrote:
> On Wed, May 9, 2018 at 3:09 PM Randall S. Becker
> <rsbecker@nexbridge.com>
> wrote:
> 
> > The question: what is the best practice for versioning the parts of
> > clean/smudge filters that are in .git/config given that only some
> > users in my environment will be cloning the repository in question and
> > that I
> really
> > can't put the entries in /etc/gitconfig or ~/.gitconfig because of
> potential
> > conflicts with other repositories that might also have clean/smudge
> > definitions.
> 
> Depending on level of trust, one approach might be to use an [include] in
> .git/config to include a file that's in the repository. Something like:
> 
> [include]
>      path = ../path/to/config
> 

What if we create a ../.gitconfig like ../.gitattributes, that is loaded
before .git/config? With loads of warnings in the documentation about what
*NOT* to put in here, any platform specifics and your own risk. The code in
config.c would look like the following, with obvious updates to
documentation and the test suite, so it's not fully baked yet. So far, I
don't have a solution to the chicken-and-egg problem, other than this.
However, if I'm barking up the wrong ballpark...

diff --git a/config.c b/config.c
index b0c20e6cb..75d5288ff 100644
--- a/config.c
+++ b/config.c
@@ -1555,11 +1555,15 @@ static int do_git_config_sequence(const struct
config_options *opts,
        char *xdg_config = xdg_config_home("config");
        char *user_config = expand_user_path("~/.gitconfig", 0);
        char *repo_config;
+       char *repo_config_versioned;

-       if (opts->commondir)
+       if (opts->commondir) {
                repo_config = mkpathdup("%s/config", opts->commondir);
-       else
+               repo_config_versioned = mkpathdup("%s/../.gitconfig",
opts->commondir);
+       } else {
                repo_config = NULL;
+               repo_config_versioned = NULL;
+       }

        current_parsing_scope = CONFIG_SCOPE_SYSTEM;
        if (git_config_system() && !access_or_die(git_etc_gitconfig(), R_OK,
0))
@@ -1574,6 +1578,8 @@ static int do_git_config_sequence(const struct
config_options *opts,
                ret += git_config_from_file(fn, user_config, data);

        current_parsing_scope = CONFIG_SCOPE_REPO;
+       if (repo_config_versioned && !access_or_die(repo_config_versioned,
R_OK, 0))
+               ret += git_config_from_file(fn, repo_config_versioned,
data);
        if (repo_config && !access_or_die(repo_config, R_OK, 0))
                ret += git_config_from_file(fn, repo_config, data);

@@ -1585,6 +1591,7 @@ static int do_git_config_sequence(const struct
config_options *opts,
        free(xdg_config);
        free(user_config);
        free(repo_config);
+       free(repo_config_versioned);
        return ret;
 }

Re: [Best Practices Request] clean/smudge configuration

[Junio C Hamano]

"Randall S. Becker" <rsbecker@nexbridge.com> writes:

> What if we create a ../.gitconfig like ../.gitattributes, that is loaded
> before .git/config?

You should not forget one of the two reasons why clean/smudge/diff
etc. drivers must be given with a step of redirection, split between
attributes and config.  "This path holds text from ancient macs" at
the logical level may be expressed in .gitattributes, and then "when
checking out text from ancient macs, process the blob data with this
program to turn it into a form suitable for working tree" is given
as a smudge filter program, that is (1) suitable for the platform
_you_ as the writer of the config file is using *AND* (2) something
you are confortable running on behalf of you.

We *deliberately* lack a mechanism to pay attention to in-tree
config that gets propagated to those who get them via "git clone",
exactly because otherwise your upstream may not just specify a
"smudge" program that your platform would be unable to run, but can
specify a "smudge" program that pretends to be a smudger, but is
actually a program that installs a keylogger and posts your login
password and bank details to this mailing list ;-)

So, no, I do not think in-tree configuration will fly.

RE: [Best Practices Request] clean/smudge configuration

[Randall S. Becker]

On May 10, 2018 10:27 PM, Junio C Hamano wrote:
> "Randall S. Becker" <rsbecker@nexbridge.com> writes:
> 
> > What if we create a ../.gitconfig like ../.gitattributes, that is
> > loaded before .git/config?
> 
> You should not forget one of the two reasons why clean/smudge/diff etc.
> drivers must be given with a step of redirection, split between attributes and
> config.  "This path holds text from ancient macs" at the logical level may be
> expressed in .gitattributes, and then "when checking out text from ancient
> macs, process the blob data with this program to turn it into a form suitable
> for working tree" is given as a smudge filter program, that is (1) suitable for
> the platform _you_ as the writer of the config file is using *AND* (2)
> something you are confortable running on behalf of you.
> 
> We *deliberately* lack a mechanism to pay attention to in-tree config that
> gets propagated to those who get them via "git clone", exactly because
> otherwise your upstream may not just specify a "smudge" program that your
> platform would be unable to run, but can specify a "smudge" program that
> pretends to be a smudger, but is actually a program that installs a keylogger
> and posts your login password and bank details to this mailing list ;-)
> 
> So, no, I do not think in-tree configuration will fly.

I agree, which is why I asked the original question instead of posting it as a formal patch. We would probably get a brand new CVE implementing the proposed proto-changes even if the original intent was internal trusted repositories only. That's why I asked this as a "Best Practices" type question, which I think I have a better idea on now. The actual situation is rather cool from a DevOps point of view, and whatever the ultimate solution is, might make for a nice presentation at some future conference 😉.

Cheers and thanks,
Randall