| Message ID | 20200501062316.GB25603@coredump.intra.peff.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] gitcredentials(7): clarify quoting of helper examples | expand |
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.
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
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
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
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.
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.
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(-)