mbox series

[0/1] git-config --add allows values from stdin

Message ID 20190917133135.190145-1-git@zjvandeweg.nl (mailing list archive)
Headers show
Series git-config --add allows values from stdin | expand

Message

Zeger-Jan van de Weg Sept. 17, 2019, 1:31 p.m. UTC
When adding or updating configuration values using git-config, the
values could all be observed by different processes as these are passed
as arguments. In some environments all commands executed are also all
logged. When the value contains secrets, this is a side effect that
would be great to avoid. At GitLab we use Rugged/libgit2 to circumvent
this property[1].

The following patch allows a value to be set through stdin when the user
passes a `--stdin` flag.

[1]: https://gitlab.com/gitlab-org/gitaly/blob/8ab5bd595984678838f3f09a96798b149e68a939/ruby/lib/gitlab/git/http_auth.rb#L14-15

Zeger-Jan van de Weg (1):
  Git config allows value setting from stdin

 Documentation/git-config.txt |  5 ++++-
 builtin/config.c             | 23 +++++++++++++++++++++--
 t/t1300-config.sh            | 11 +++++++++++
 3 files changed, 36 insertions(+), 3 deletions(-)

--
2.23.0

Comments

Taylor Blau Sept. 22, 2019, 3:11 a.m. UTC | #1
Hi ZJ,

On Tue, Sep 17, 2019 at 03:31:34PM +0200, Zeger-Jan van de Weg wrote:
> When adding or updating configuration values using git-config, the
> values could all be observed by different processes as these are passed
> as arguments. In some environments all commands executed are also all
> logged. When the value contains secrets, this is a side effect that
> would be great to avoid. At GitLab we use Rugged/libgit2 to circumvent
> this property[1].
>
> The following patch allows a value to be set through stdin when the user
> passes a `--stdin` flag.

Interesting. I had thought some time ago about making an interactive
line-oriented 'mode' for using 'git-config(1)', which would allow
callers to add/delete/fetch multiple variables using only a single
process.

This would satisfy a more general use-case than yours: particularly my
idea was grown out of wanting to specify or read many configuration
entries at once when using a tool built around Git, such as Git LFS.

I had not considered tying '--stdin' to the '--add' (implicit or not)
mode of 'git-config(1)'. It is an interesting idea to be sure.

On the one hand, it lends itself to other modes, such as '--get'
combined with '--stdin', or '--unset' in the same fashion. One could
imagine that each of these would take either a key/value-pair (in the
case of '--add') or a set of key(s) (in the remaining cases). The most
desirable aspect is that this would allow for a clear path to this
series being picked up.

On the other hand, tying '--stdin' to a particular mode of using 'git
conifg' seems overly restrictive to me. If I am building a tool that
wants to fetch some values in the configuration, and then add/unset
others based on the results using only a single process, I don't think
that a mode-based '--stdin' flag gets the job done.

One happy medium that comes to mind is a new '--interactive' mode, which
implies '--stdin' and would allow the above use-case, e.g.:

  $ git config --interactive <<\EOF
  get core.myval
  set core.foo bar
  unset core.baz
  EOF

(An off-topic note is that it would be interesting to allow more
fanciful options than 'get', e.g., 'get' with a '--type' specifier, or
some such).

I'm not sure if anyone actually wants to use 'git-config(1)' in this
way, but I figured that I would at least share some things that I was
thinking about when initially considering this proposal.

> [1]: https://gitlab.com/gitlab-org/gitaly/blob/8ab5bd595984678838f3f09a96798b149e68a939/ruby/lib/gitlab/git/http_auth.rb#L14-15
>
> Zeger-Jan van de Weg (1):
>   Git config allows value setting from stdin
>
>  Documentation/git-config.txt |  5 ++++-
>  builtin/config.c             | 23 +++++++++++++++++++++--
>  t/t1300-config.sh            | 11 +++++++++++
>  3 files changed, 36 insertions(+), 3 deletions(-)
>
> --
> 2.23.0
>

Thanks,
Taylor
Phillip Wood Sept. 23, 2019, 9:46 a.m. UTC | #2
Hi Taylor and ZJ

On 22/09/2019 04:11, Taylor Blau wrote:
> Hi ZJ,
> 
> On Tue, Sep 17, 2019 at 03:31:34PM +0200, Zeger-Jan van de Weg wrote:
>> When adding or updating configuration values using git-config, the
>> values could all be observed by different processes as these are passed
>> as arguments. In some environments all commands executed are also all
>> logged. When the value contains secrets, this is a side effect that
>> would be great to avoid.

How much extra security does this actually add? - do the processes that 
can observe the command line arguments also have read access to the git 
config file?

  At GitLab we use Rugged/libgit2 to circumvent
>> this property[1].
>>
>> The following patch allows a value to be set through stdin when the user
>> passes a `--stdin` flag.
> 
> Interesting. I had thought some time ago about making an interactive
> line-oriented 'mode' for using 'git-config(1)', which would allow
> callers to add/delete/fetch multiple variables using only a single
> process.
> 
> This would satisfy a more general use-case than yours: particularly my
> idea was grown out of wanting to specify or read many configuration
> entries at once when using a tool built around Git, such as Git LFS.
> 
> I had not considered tying '--stdin' to the '--add' (implicit or not)
> mode of 'git-config(1)'. It is an interesting idea to be sure.
> 
> On the one hand, it lends itself to other modes, such as '--get'
> combined with '--stdin', or '--unset' in the same fashion. One could
> imagine that each of these would take either a key/value-pair (in the
> case of '--add') or a set of key(s) (in the remaining cases). The most
> desirable aspect is that this would allow for a clear path to this
> series being picked up.

It would be great to be able to --get multiple values and I can see 
people wanting to be able to --unset them as well.

> On the other hand, tying '--stdin' to a particular mode of using 'git
> conifg' seems overly restrictive to me. If I am building a tool that
> wants to fetch some values in the configuration, and then add/unset
> others based on the results using only a single process, I don't think
> that a mode-based '--stdin' flag gets the job done.

That's true but I don't know how common it is compared to a script 
wanting to read a bunch of config variables at startup (i.e. does it 
warrant the extra complexity)

Best Wishes

Phillip

> One happy medium that comes to mind is a new '--interactive' mode, which
> implies '--stdin' and would allow the above use-case, e.g.:
> 
>    $ git config --interactive <<\EOF
>    get core.myval
>    set core.foo bar
>    unset core.baz
>    EOF
> 
> (An off-topic note is that it would be interesting to allow more
> fanciful options than 'get', e.g., 'get' with a '--type' specifier, or
> some such).
> 
> I'm not sure if anyone actually wants to use 'git-config(1)' in this
> way, but I figured that I would at least share some things that I was
> thinking about when initially considering this proposal.
> 
>> [1]: https://gitlab.com/gitlab-org/gitaly/blob/8ab5bd595984678838f3f09a96798b149e68a939/ruby/lib/gitlab/git/http_auth.rb#L14-15
>>
>> Zeger-Jan van de Weg (1):
>>    Git config allows value setting from stdin
>>
>>   Documentation/git-config.txt |  5 ++++-
>>   builtin/config.c             | 23 +++++++++++++++++++++--
>>   t/t1300-config.sh            | 11 +++++++++++
>>   3 files changed, 36 insertions(+), 3 deletions(-)
>>
>> --
>> 2.23.0
>>
> 
> Thanks,
> Taylor
>
SZEDER Gábor Sept. 23, 2019, 11:45 a.m. UTC | #3
On Sat, Sep 21, 2019 at 11:11:28PM -0400, Taylor Blau wrote:
> Hi ZJ,
> 
> On Tue, Sep 17, 2019 at 03:31:34PM +0200, Zeger-Jan van de Weg wrote:
> > When adding or updating configuration values using git-config, the
> > values could all be observed by different processes as these are passed
> > as arguments. In some environments all commands executed are also all
> > logged. When the value contains secrets, this is a side effect that
> > would be great to avoid. At GitLab we use Rugged/libgit2 to circumvent
> > this property[1].
> >
> > The following patch allows a value to be set through stdin when the user
> > passes a `--stdin` flag.
> 
> Interesting. I had thought some time ago about making an interactive
> line-oriented 'mode' for using 'git-config(1)', which would allow
> callers to add/delete/fetch multiple variables using only a single
> process.
> 
> This would satisfy a more general use-case than yours: particularly my
> idea was grown out of wanting to specify or read many configuration
> entries at once when using a tool built around Git, such as Git LFS.

Getting multiple configuration variables from a single 'git config'
invocation is already supported fairly well, in the sense that it's
not at all difficult to parse the output of 'git config --list -z'
into a map/dict of the language of your choice.  Though, of course, it
doesn't offer the normalization and unit conversion of '--type'.

> I had not considered tying '--stdin' to the '--add' (implicit or not)
> mode of 'git-config(1)'. It is an interesting idea to be sure.
> 
> On the one hand, it lends itself to other modes, such as '--get'
> combined with '--stdin', or '--unset' in the same fashion. One could
> imagine that each of these would take either a key/value-pair (in the
> case of '--add') or a set of key(s) (in the remaining cases). The most
> desirable aspect is that this would allow for a clear path to this
> series being picked up.
> 
> On the other hand, tying '--stdin' to a particular mode of using 'git
> conifg' seems overly restrictive to me. If I am building a tool that
> wants to fetch some values in the configuration, and then add/unset
> others based on the results using only a single process, I don't think
> that a mode-based '--stdin' flag gets the job done.
> 
> One happy medium that comes to mind is a new '--interactive' mode, which
> implies '--stdin' and would allow the above use-case, e.g.:
> 
>   $ git config --interactive <<\EOF
>   get core.myval
>   set core.foo bar
>   unset core.baz
>   EOF

I think the option should still be called '--stdin' instead of
'--interactive'.  Consider the rather similar 'git update-ref
--stdin', which allows creating, updating, and deleting refs in one go
as well.

> (An off-topic note is that it would be interesting to allow more
> fanciful options than 'get', e.g., 'get' with a '--type' specifier, or
> some such).
> 
> I'm not sure if anyone actually wants to use 'git-config(1)' in this
> way, but I figured that I would at least share some things that I was
> thinking about when initially considering this proposal.

Once upon a time I had a script-generated repo with 10+k remotes, and
3 config variables per remote.  The resulting configuration file was
over 1MB, and creating it by forking a 'git config' to write those
configuration variables one at a time took over 18mins (IIRC).  I
ended up special casing the writing of the initial huge configuration
file with simple print() calls, which only took about a second or two.

So I would welcome a more general 'git config --stdin' that could
write configuration variables in bulk.