RE: [Best Practices Request] clean/smudge configuration - Avoiding the chicken and egg

2018-05-12 Thread Randall S. Becker
On May 11, 2018 3:26 PM, I wrote:
> On May 10, 2018 10:27 PM, Junio C Hamano wrote:
> > "Randall S. Becker"  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 .

Here's where I ended up, from a solution standpoint:

0. Make sure the git scripts you use are always trusted using your favourite 
technique
1. Wrap the clone in a such a script to do the next two steps to avoid the 
usual problems of forgetting things
2. The clone script should use "git -c name=value clone repo" for all 
clean/smudge values needed that would otherwise be in .git/config if we had one 
which we don't
3. Have the script create/update .git/config using "git config --local name 
value" with all of the same clean/smudge values for subsequent operations.

>From there, it seems that the contents of the smudged files are always 
>correct, assuming the filter works of course. It was the use of -c that makes 
>this work.

Sound about right?

Cheers,
Randall



RE: [Best Practices Request] clean/smudge configuration

2018-05-11 Thread Randall S. Becker


On May 10, 2018 10:27 PM, Junio C Hamano wrote:
> "Randall S. Becker"  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



Re: [Best Practices Request] clean/smudge configuration

2018-05-10 Thread Junio C Hamano
"Randall S. Becker"  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

2018-05-10 Thread 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
> 
> 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

2018-05-09 Thread 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
> 
> 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

2018-05-09 Thread Bryan Turner
On Wed, May 9, 2018 at 3:09 PM Randall S. Becker 
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


[Best Practices Request] clean/smudge configuration

2018-05-09 Thread 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