* [PATCH 1/2] gitcredentials(7): clarify quoting of helper examples
2020-05-01 6:19 ` Jeff King
@ 2020-05-01 6:20 ` Jeff King
2020-05-01 7:19 ` Andreas Schwab
2020-05-01 6:23 ` [PATCH 2/2] gitcredentials(7): make shell-snippet example more realistic Jeff King
2020-05-01 14:40 ` [PATCH] doc: fix quoting bug in credential cache example douglas.fuller
2 siblings, 1 reply; 18+ messages in thread
From: Jeff King @ 2020-05-01 6:20 UTC (permalink / raw)
To: Junio C Hamano; +Cc: douglas.fuller, git
We give several helper config examples, but don't make clear that these
are raw values. It's up to the user to add the appropriate quoting to
put them into a config file (either by running with "git config" and
quoting against the shell, or by adding double-quotes as appropriate
within the git-config file).
Let's flesh them out as full config blocks, which makes the syntax more
clear (and makes it possible for people to just cut-and-paste them as a
starting point). I added double-quotes to any values larger than a
single word. That isn't strictly necessary in all cases, but it
sidesteps explaining the rules about exactly when you need to quote a
value.
The existing quotes can be converted to single-quotes in one instance,
and simply omitted in another for simplicity (unless you have odd
whitespace in your password, the shell whitespace-splitting behavior is
fine). I also swapped out backticks for our preferred $().
Reported-by: douglas.fuller@gmail.com
Signed-off-by: Jeff King <peff@peff.net>
---
Documentation/gitcredentials.txt | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/Documentation/gitcredentials.txt b/Documentation/gitcredentials.txt
index 1814d2d23c..8a20343dd7 100644
--- a/Documentation/gitcredentials.txt
+++ b/Documentation/gitcredentials.txt
@@ -216,20 +216,25 @@ Here are some example specifications:
----------------------------------------------------
# run "git credential-foo"
-foo
+[credential]
+helper = foo
# same as above, but pass an argument to the helper
-foo --bar=baz
+[credential]
+helper = "foo --bar=baz"
# the arguments are parsed by the shell, so use shell
# quoting if necessary
-foo --bar="whitespace arg"
+[credential]
+helper = "foo --bar='whitespace arg'"
# you can also use an absolute path, which will not use the git wrapper
-/path/to/my/helper --with-arguments
+[credential]
+helper = "/path/to/my/helper --with-arguments"
# or you can specify your own shell snippet
-!f() { echo "password=`cat $HOME/.secret`"; }; f
+[credential]
+helper = "!f() { echo password=$(cat $HOME/.secret); }; f"
----------------------------------------------------
Generally speaking, rule (3) above is the simplest for users to specify.
--
2.26.2.933.gdf62622942
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] gitcredentials(7): clarify quoting of helper examples
2020-05-01 6:20 ` [PATCH 1/2] gitcredentials(7): clarify quoting of helper examples Jeff King
@ 2020-05-01 7:19 ` Andreas Schwab
2020-05-01 7:31 ` Jeff King
0 siblings, 1 reply; 18+ messages in thread
From: Andreas Schwab @ 2020-05-01 7:19 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, douglas.fuller, git
On Mai 01 2020, Jeff King wrote:
> # or you can specify your own shell snippet
> -!f() { echo "password=`cat $HOME/.secret`"; }; f
> +[credential]
> +helper = "!f() { echo password=$(cat $HOME/.secret); }; f"
That now lacks a pair of quotes around the argument of echo.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] gitcredentials(7): clarify quoting of helper examples
2020-05-01 7:19 ` Andreas Schwab
@ 2020-05-01 7:31 ` Jeff King
0 siblings, 0 replies; 18+ messages in thread
From: Jeff King @ 2020-05-01 7:31 UTC (permalink / raw)
To: Andreas Schwab; +Cc: Junio C Hamano, douglas.fuller, git
On Fri, May 01, 2020 at 09:19:18AM +0200, Andreas Schwab wrote:
> On Mai 01 2020, Jeff King wrote:
>
> > # or you can specify your own shell snippet
> > -!f() { echo "password=`cat $HOME/.secret`"; }; f
> > +[credential]
> > +helper = "!f() { echo password=$(cat $HOME/.secret); }; f"
>
> That now lacks a pair of quotes around the argument of echo.
Right, as discussed in the commit message.
-Peff
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 2/2] gitcredentials(7): make shell-snippet example more realistic
2020-05-01 6:19 ` Jeff King
2020-05-01 6:20 ` [PATCH 1/2] gitcredentials(7): clarify quoting of helper examples Jeff King
@ 2020-05-01 6:23 ` Jeff King
2020-05-01 6:26 ` Junio C Hamano
2020-05-01 14:40 ` [PATCH] doc: fix quoting bug in credential cache example douglas.fuller
2 siblings, 1 reply; 18+ messages in thread
From: Jeff King @ 2020-05-01 6:23 UTC (permalink / raw)
To: Junio C Hamano; +Cc: douglas.fuller, git
There's an example of using your own bit of shell to act as a credential
helper, but it's not very realistic:
- It's stupid to hand out your secret password to _every_ host. In the
real world you'd use the config-matcher to limit it to a particular
host.
- We never provided a username. We can easily do that in another config
option (you can do it in the helper, too, but this is much more
readable).
- We were sending the secret even for store/erase operations. This
is OK because Git would just ignore it, but a real system would
probably be unlocking a password store, which you wouldn't want to do
more than necessary.
Signed-off-by: Jeff King <peff@peff.net>
---
This is in fact very close to what's in my own ~/.gitconfig, except that
I swap out "cat" for the "pass" tool.
Documentation/gitcredentials.txt | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/Documentation/gitcredentials.txt b/Documentation/gitcredentials.txt
index 8a20343dd7..63b20fc6a5 100644
--- a/Documentation/gitcredentials.txt
+++ b/Documentation/gitcredentials.txt
@@ -233,8 +233,9 @@ helper = "foo --bar='whitespace arg'"
helper = "/path/to/my/helper --with-arguments"
# or you can specify your own shell snippet
-[credential]
-helper = "!f() { echo password=$(cat $HOME/.secret); }; f"
+[credential "https://example.com"]
+username = your_user
+helper = "!f() { test $1 = get && echo password=$(cat $HOME/.secret); }; f"
----------------------------------------------------
Generally speaking, rule (3) above is the simplest for users to specify.
--
2.26.2.933.gdf62622942
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] gitcredentials(7): make shell-snippet example more realistic
2020-05-01 6:23 ` [PATCH 2/2] gitcredentials(7): make shell-snippet example more realistic Jeff King
@ 2020-05-01 6:26 ` Junio C Hamano
2020-05-01 6:32 ` Jeff King
0 siblings, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2020-05-01 6:26 UTC (permalink / raw)
To: Jeff King; +Cc: douglas.fuller, git
Jeff King <peff@peff.net> writes:
> There's an example of using your own bit of shell to act as a credential
> helper, but it's not very realistic:
>
> - It's stupid to hand out your secret password to _every_ host. In the
> real world you'd use the config-matcher to limit it to a particular
> host.
>
> - We never provided a username. We can easily do that in another config
> option (you can do it in the helper, too, but this is much more
> readable).
>
> - We were sending the secret even for store/erase operations. This
> is OK because Git would just ignore it, but a real system would
> probably be unlocking a password store, which you wouldn't want to do
> more than necessary.
All of them make sense, but I do not think we want to encourage that
loose style of passing unquoted argument to echo to lose embedded
$IFS spaces that is not a SP.
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> This is in fact very close to what's in my own ~/.gitconfig, except that
> I swap out "cat" for the "pass" tool.
>
> Documentation/gitcredentials.txt | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/gitcredentials.txt b/Documentation/gitcredentials.txt
> index 8a20343dd7..63b20fc6a5 100644
> --- a/Documentation/gitcredentials.txt
> +++ b/Documentation/gitcredentials.txt
> @@ -233,8 +233,9 @@ helper = "foo --bar='whitespace arg'"
> helper = "/path/to/my/helper --with-arguments"
>
> # or you can specify your own shell snippet
> -[credential]
> -helper = "!f() { echo password=$(cat $HOME/.secret); }; f"
> +[credential "https://example.com"]
> +username = your_user
> +helper = "!f() { test $1 = get && echo password=$(cat $HOME/.secret); }; f"
> ----------------------------------------------------
>
> Generally speaking, rule (3) above is the simplest for users to specify.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] gitcredentials(7): make shell-snippet example more realistic
2020-05-01 6:26 ` Junio C Hamano
@ 2020-05-01 6:32 ` Jeff King
2020-05-01 6:35 ` Jeff King
0 siblings, 1 reply; 18+ messages in thread
From: Jeff King @ 2020-05-01 6:32 UTC (permalink / raw)
To: Junio C Hamano; +Cc: douglas.fuller, git
On Thu, Apr 30, 2020 at 11:26:39PM -0700, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > There's an example of using your own bit of shell to act as a credential
> > helper, but it's not very realistic:
> >
> > - It's stupid to hand out your secret password to _every_ host. In the
> > real world you'd use the config-matcher to limit it to a particular
> > host.
> >
> > - We never provided a username. We can easily do that in another config
> > option (you can do it in the helper, too, but this is much more
> > readable).
> >
> > - We were sending the secret even for store/erase operations. This
> > is OK because Git would just ignore it, but a real system would
> > probably be unlocking a password store, which you wouldn't want to do
> > more than necessary.
>
> All of them make sense, but I do not think we want to encourage that
> loose style of passing unquoted argument to echo to lose embedded
> $IFS spaces that is not a SP.
You mean dropping the quotes in the first patch?
Doing:
echo "password=$(cat $HOME/.secret)"
already eats some trailing whitespace, though I guess if you have
newlines in your password you are beyond help anyway.
I can add back in the quoted \", though it does make the code slightly
harder to read.
-Peff
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] gitcredentials(7): make shell-snippet example more realistic
2020-05-01 6:32 ` Jeff King
@ 2020-05-01 6:35 ` Jeff King
2020-05-01 7:32 ` Jeff King
2020-05-01 15:11 ` [PATCH " Junio C Hamano
0 siblings, 2 replies; 18+ messages in thread
From: Jeff King @ 2020-05-01 6:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: douglas.fuller, git
On Fri, May 01, 2020 at 02:32:07AM -0400, Jeff King wrote:
> > All of them make sense, but I do not think we want to encourage that
> > loose style of passing unquoted argument to echo to lose embedded
> > $IFS spaces that is not a SP.
>
> You mean dropping the quotes in the first patch?
>
> Doing:
>
> echo "password=$(cat $HOME/.secret)"
>
> already eats some trailing whitespace, though I guess if you have
> newlines in your password you are beyond help anyway.
>
> I can add back in the quoted \", though it does make the code slightly
> harder to read.
Or did you mean passing $1 in the test call? It definitely isn't good
shell practice, but we know that we're getting a single-word action from
Git, per the protocol.
Fully quoting, it looks like this:
helper = "!f() { test \"$1\" = get && echo \"password=$(cat $HOME/.secret)\"; }; f"
which IMHO is getting a little hard to read. I think that's part of why
I gave such an unfinished example in the first place. :)
-Peff
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] gitcredentials(7): make shell-snippet example more realistic
2020-05-01 6:35 ` Jeff King
@ 2020-05-01 7:32 ` Jeff King
2020-05-01 7:33 ` [PATCH v2 1/2] gitcredentials(7): clarify quoting of helper examples Jeff King
2020-05-01 7:33 ` [PATCH v2 2/2] gitcredentials(7): make shell-snippet example more realistic Jeff King
2020-05-01 15:11 ` [PATCH " Junio C Hamano
1 sibling, 2 replies; 18+ messages in thread
From: Jeff King @ 2020-05-01 7:32 UTC (permalink / raw)
To: Junio C Hamano; +Cc: douglas.fuller, git
On Fri, May 01, 2020 at 02:35:13AM -0400, Jeff King wrote:
> On Fri, May 01, 2020 at 02:32:07AM -0400, Jeff King wrote:
>
> > > All of them make sense, but I do not think we want to encourage that
> > > loose style of passing unquoted argument to echo to lose embedded
> > > $IFS spaces that is not a SP.
> >
> > You mean dropping the quotes in the first patch?
> >
> > Doing:
> >
> > echo "password=$(cat $HOME/.secret)"
> >
> > already eats some trailing whitespace, though I guess if you have
> > newlines in your password you are beyond help anyway.
> >
> > I can add back in the quoted \", though it does make the code slightly
> > harder to read.
>
> Or did you mean passing $1 in the test call? It definitely isn't good
> shell practice, but we know that we're getting a single-word action from
> Git, per the protocol.
>
> Fully quoting, it looks like this:
>
> helper = "!f() { test \"$1\" = get && echo \"password=$(cat $HOME/.secret)\"; }; f"
>
> which IMHO is getting a little hard to read. I think that's part of why
> I gave such an unfinished example in the first place. :)
So I dunno. That's ugly, but I don't think is worth nitpicking over
more. So here it is with indentation and full-on quoting.
[1/2]: gitcredentials(7): clarify quoting of helper examples
[2/2]: gitcredentials(7): make shell-snippet example more realistic
Documentation/gitcredentials.txt | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
-Peff
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 1/2] gitcredentials(7): clarify quoting of helper examples
2020-05-01 7:32 ` Jeff King
@ 2020-05-01 7:33 ` Jeff King
2020-05-01 7:33 ` [PATCH v2 2/2] gitcredentials(7): make shell-snippet example more realistic Jeff King
1 sibling, 0 replies; 18+ messages in thread
From: Jeff King @ 2020-05-01 7:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: douglas.fuller, git
We give several helper config examples, but don't make clear that these
are raw values. It's up to the user to add the appropriate quoting to
put them into a config file (either by running with "git config" and
quoting against the shell, or by adding double-quotes as appropriate
within the git-config file).
Let's flesh them out as full config blocks, which makes the syntax more
clear (and makes it possible for people to just cut-and-paste them as a
starting point). I added double-quotes to any values larger than a
single word. That isn't strictly necessary in all cases, but it
sidesteps explaining the rules about exactly when you need to quote a
value.
The existing quotes can be converted to single-quotes in one instance,
and backslash-esccaped in the other. I also swapped out backticks for
our preferred $().
Reported-by: douglas.fuller@gmail.com
Signed-off-by: Jeff King <peff@peff.net>
---
Documentation/gitcredentials.txt | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/Documentation/gitcredentials.txt b/Documentation/gitcredentials.txt
index 1814d2d23c..8127dfcd2f 100644
--- a/Documentation/gitcredentials.txt
+++ b/Documentation/gitcredentials.txt
@@ -216,20 +216,25 @@ Here are some example specifications:
----------------------------------------------------
# run "git credential-foo"
-foo
+[credential]
+ helper = foo
# same as above, but pass an argument to the helper
-foo --bar=baz
+[credential]
+ helper = "foo --bar=baz"
# the arguments are parsed by the shell, so use shell
# quoting if necessary
-foo --bar="whitespace arg"
+[credential]
+ helper = "foo --bar='whitespace arg'"
# you can also use an absolute path, which will not use the git wrapper
-/path/to/my/helper --with-arguments
+[credential]
+ helper = "/path/to/my/helper --with-arguments"
# or you can specify your own shell snippet
-!f() { echo "password=`cat $HOME/.secret`"; }; f
+[credential]
+ helper = "!f() { echo \"password=$(cat $HOME/.secret)\"; }; f"
----------------------------------------------------
Generally speaking, rule (3) above is the simplest for users to specify.
--
2.26.2.933.gdf62622942
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 2/2] gitcredentials(7): make shell-snippet example more realistic
2020-05-01 7:32 ` Jeff King
2020-05-01 7:33 ` [PATCH v2 1/2] gitcredentials(7): clarify quoting of helper examples Jeff King
@ 2020-05-01 7:33 ` Jeff King
1 sibling, 0 replies; 18+ messages in thread
From: Jeff King @ 2020-05-01 7:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: douglas.fuller, git
There's an example of using your own bit of shell to act as a credential
helper, but it's not very realistic:
- It's stupid to hand out your secret password to _every_ host. In the
real world you'd use the config-matcher to limit it to a particular
host.
- We never provided a username. We can easily do that in another config
option (you can do it in the helper, too, but this is much more
readable).
- We were sending the secret even for store/erase operations. This
is OK because Git would just ignore it, but a real system would
probably be unlocking a password store, which you wouldn't want to do
more than necessary.
Signed-off-by: Jeff King <peff@peff.net>
---
Documentation/gitcredentials.txt | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/Documentation/gitcredentials.txt b/Documentation/gitcredentials.txt
index 8127dfcd2f..0d0f7149bd 100644
--- a/Documentation/gitcredentials.txt
+++ b/Documentation/gitcredentials.txt
@@ -233,8 +233,9 @@ Here are some example specifications:
helper = "/path/to/my/helper --with-arguments"
# or you can specify your own shell snippet
-[credential]
- helper = "!f() { echo \"password=$(cat $HOME/.secret)\"; }; f"
+[credential "https://example.com"]
+ username = your_user
+ helper = "!f() { test \"$1\" = get && echo \"password=$(cat $HOME/.secret)\"; }; f"
----------------------------------------------------
Generally speaking, rule (3) above is the simplest for users to specify.
--
2.26.2.933.gdf62622942
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] gitcredentials(7): make shell-snippet example more realistic
2020-05-01 6:35 ` Jeff King
2020-05-01 7:32 ` Jeff King
@ 2020-05-01 15:11 ` Junio C Hamano
1 sibling, 0 replies; 18+ messages in thread
From: Junio C Hamano @ 2020-05-01 15:11 UTC (permalink / raw)
To: Jeff King; +Cc: douglas.fuller, git
Jeff King <peff@peff.net> writes:
>> I can add back in the quoted \", though it does make the code slightly
>> harder to read.
>
> Or did you mean passing $1 in the test call? It definitely isn't good
> shell practice, but we know that we're getting a single-word action from
> Git, per the protocol.
I meant former. I actually am having a second thought.
I do not think this page should be the canonical place for config
and shell syntax lessons, but the value we have been using as an
example is a good candidate that an end user would encounter in the
real life and has need to do so.
Perhaps it deserves some comment next to it?
; For the specified site, use "your_user" as the username, and
; invoke the helper, which is a short shell script. Note that the
; value of the helper variable is enclosed in a double-quote pair,
; because it has a semicolon, which will cause the rest of the line
; discarded as a comment unless quoted. The shell script in turn
; needs to quote various pieces of it in double quotes, each of
; which needs to be escaped with a backslash.
[credential "https://example.com"]
username = your_user
helper = "!f() { test \"$1\" = get && echo \"password=$(cat $HOME/.secret)\"; }; f"
The more I think about it, the worse it becomes X-<
Do we expect that most of our users are comfortable editing
~/.gitconfig in their editor, or do they mostly work with the "git
config --global" command?
I have a feeling that my wishful-thinking answer, which is "former",
is not true in the real world. Which means they not only need to
quote for the parser of the configuration file, but they then need
to quote that for the shell X-<.
$ git config --global credential.https://example.com.helper \
'"!f() { test \"$1\" = get && echo \"password=$(cat $HOME/.secret)\"; }; f"'
I wonder if it helps the users to have something that guides them to
figure out how they do the above.
> Fully quoting, it looks like this:
>
> helper = "!f() { test \"$1\" = get && echo \"password=$(cat $HOME/.secret)\"; }; f"
>
> which IMHO is getting a little hard to read. I think that's part of why
> I gave such an unfinished example in the first place. :)
Yes, "these are values, go quote them as necessary" is certainly a
lot more attractive position to take. But apparently that wasn't
all that helpful.
Thanks.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] doc: fix quoting bug in credential cache example
2020-05-01 6:19 ` Jeff King
2020-05-01 6:20 ` [PATCH 1/2] gitcredentials(7): clarify quoting of helper examples Jeff King
2020-05-01 6:23 ` [PATCH 2/2] gitcredentials(7): make shell-snippet example more realistic Jeff King
@ 2020-05-01 14:40 ` douglas.fuller
2 siblings, 0 replies; 18+ messages in thread
From: douglas.fuller @ 2020-05-01 14:40 UTC (permalink / raw)
To: Jeff King, Junio C Hamano; +Cc: git
On Fri, 2020-05-01 at 02:19 -0400, Jeff King wrote:
> On Fri, May 01, 2020 at 01:57:38AM -0400, Jeff King wrote:
>
> > It may be easier to just use double-quotes consistently, even for
> > ones
> > that do not need it, to give readers one less thing to wonder
> > about.
>
> So here's a patch that does that. I also noticed a few other
> deficiencies in that final example, which are fixed in the second
> patch.
>
> I hope I'm not stealing Douglas's thunder. :)
My one-line patch definitely wasn't thunderous -- I didn't even know
how to write a comment in a config file. As posited upthread, I did
interpret the example as presenting config file syntax, so I got
tripped up by the semicolon there. This patch makes it more clear.
I use git (thanks!) without having looked at the code, so I figured I
was a good example of the target audience for this doc. Thanks for
making it more clear.
Cheers,
--Doug
> [1/2]: gitcredentials(7): clarify quoting of helper examples
> [2/2]: gitcredentials(7): make shell-snippet example more realistic
>
> Documentation/gitcredentials.txt | 16 +++++++++++-----
> 1 file changed, 11 insertions(+), 5 deletions(-)
>
> -Peff
^ permalink raw reply [flat|nested] 18+ messages in thread