threads / discuss / 48449

[Best Practices Request] clean/smudge configuration

Subject: [Best Practices Request] clean/smudge configuration

## tl;dr

6 messages between May 9, 2018 and May 11, 2018.

replies: 5people: 3as markdown or json

Randall S. Becker· May 9, 2018, 22:08 UTC · lore
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

Bryan Turner· May 9, 2018, 22:39 UTC · re: Randall S. Becker · lore

Re: [Best Practices Request] clean/smudge configuration

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
Randall S. Becker· May 9, 2018, 22:57 UTC · re: Bryan Turner · lore

RE: [Best Practices Request] clean/smudge configuration

On May 9, 2018 6:39 PM, Bryan Turner wrote
Show 20 quoted lines
> 
> 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

Randall S. Becker· May 10, 2018, 19:59 UTC · re: Bryan Turner · lore

RE: [Best Practices Request] clean/smudge configuration

On May 9, 2018 6:39 PM, Bryan Turner wrote:
Show 20 quoted lines
> 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;
 }
Junio C Hamano· May 11, 2018, 02:27 UTC · re: Randall S. Becker · lore

Re: [Best Practices Request] clean/smudge configuration

"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.
Randall S. Becker· May 11, 2018, 19:25 UTC · re: Junio C Hamano · lore

RE: [Best Practices Request] clean/smudge configuration

On May 10, 2018 10:27 PM, Junio C Hamano wrote:
Show 22 quoted lines
> "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

← back to recent threads