{"thread":{"id":"38096","subject":"Disabling credential helper?","startedAt":"2014-12-03T00:03:10Z","lastAt":"2014-12-05T17:37:22Z","messageCount":17,"participants":["brian m. carlson","Jonathan Nieder","Jeff King","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"252899","messageId":"20141203000310.GE90134@vauxhall.crustytoothpaste.net","threadId":"38096","inReplyTo":null,"subject":"Disabling credential helper?","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2014-12-03T00:03:10Z","receivedAt":"2014-12-03T00:03:10Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"At $DAYJOB, we have a Git server[0] that supports the smart HTTP\nprotocol.  That server can return a 401 if the repository is private or\ndoesn't exist.\n\nWe have several scripts, some of which run interactively, some not, that\nwe simply want to fail if git fetch gets a non-2xx code.  Unfortunately,\ngit is very insistent about trying to use the default credential helper\nto prompt for a username and password in this case, even opening\n/dev/tty.\n\nWe've used GIT_ASKPASS=/bin/echo, which seems to solve the problem,\nalthough it's ugly and I'm concerned it might break in the future.  Is\nthere a better way to do this?  I didn't see one in the documentation or\ncode when I looked.\n\n[0] An Atlassian Stash instance.\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\n+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187\n"},{"id":"252905","messageId":"20141203005953.GB6527@google.com","threadId":"38096","inReplyTo":"20141203000310.GE90134@vauxhall.crustytoothpaste.net","subject":"Re: Disabling credential helper?","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-12-03T00:59:53Z","receivedAt":"2014-12-03T00:59:53Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"(+peff)\nHi,\n\nbrian m. carlson wrote:\n\n> We've used GIT_ASKPASS=/bin/echo, which seems to solve the problem,\n> although it's ugly and I'm concerned it might break in the future.  Is\n> there a better way to do this?\n\nThat's a good question.  Before falling back to the askpass based\nprompt, Git tries each credential helper matching the URL in turn, and\nthere doesn't seem to be an option to override that behavior and disable\ncredential helpers.\n\nAs long as you have no credential helpers configured, your GIT_ASKPASS\nbased approach should work fine.\n\nBut once you have helpers configured, you're potentially in trouble.\nI'm wondering if we ought to provide an --no-credential-helpers option\nto help with this.  (Or to go further and provide a way to unset\nconfiguration items --- e.g., '-c \"credential.*=unset\"'.)\n\nThoughts?\nJonathan\n"},{"id":"252907","messageId":"20141203012148.GB29427@peff.net","threadId":"38096","inReplyTo":"20141203005953.GB6527@google.com","subject":"Re: Disabling credential helper?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-12-03T01:21:48Z","receivedAt":"2014-12-03T01:21:48Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Dec 02, 2014 at 04:59:53PM -0800, Jonathan Nieder wrote:\n\n> brian m. carlson wrote:\n> \n> > We've used GIT_ASKPASS=/bin/echo, which seems to solve the problem,\n> > although it's ugly and I'm concerned it might break in the future.  Is\n> > there a better way to do this?\n> \n> That's a good question.  Before falling back to the askpass based\n> prompt, Git tries each credential helper matching the URL in turn, and\n> there doesn't seem to be an option to override that behavior and disable\n> credential helpers.\n\nI think this has nothing at all to do with credential helpers. Git tries\nthe helpers, of which there are none, and falls back to askpass and\nprompting on the terminal. There is no way to design a helper to say \"I\ntried and failed; do not bother prompting on the terminal\". Git only\nsees that no helper provided an answer and uses its internal methods.\n\nI did at one point consider making the terminal prompt a credential\nhelper (since it is, after all, no different from git's perspective;\nit's just a mechanism for \"somehow\" coming up with a username/password\npair).  But people generally thought that was unnecessarily complicated\n(and it certainly is for the common cases).\n\n> As long as you have no credential helpers configured, your GIT_ASKPASS\n> based approach should work fine.\n\nYeah, it's fine (as is GIT_ASKPASS=true). You could also provide a\ncredential helper that gives you an empty username and password. But in\nboth cases, I think that git will then feed the empty password to the\nserver again, resulting in an extra useless round-trip. You probably\ninstead want to say \"stop now, git, there is nothing else to be done\".\n\nWe could teach the credential-helper code to do that (e.g., a helper\nreturns \"stop=true\" and we respect that). But I think you can do it\nreasonably well today by making the input process fail. Sadly setting\nGIT_ASKPASS to \"false\" just makes git complain and then try harder[1].\nBut you can dissociate git from the terminal, like:\n\n  $ setsid -w git ls-remote https://github.com/private/repo\n  fatal: could not read Username for 'https://github.com': No such device or address\n\nThat might have other fallouts if you use process groups for anything. I\nhave no problem with either an option to disable the terminal prompting,\nor teaching the credential-helper interface to allow helpers to stop\nlookup, either of which would be cleaner.\n\n-Peff\n\n[1] Courtesy of 84d7273 (prompt: fall back to terminal if askpass fails,\n2012-02-03).\n"},{"id":"252908","messageId":"20141203012950.GC6527@google.com","threadId":"38096","inReplyTo":"20141203012148.GB29427@peff.net","subject":"Re: Disabling credential helper?","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-12-03T01:29:50Z","receivedAt":"2014-12-03T01:29:50Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jeff King wrote:\n> On Tue, Dec 02, 2014 at 04:59:53PM -0800, Jonathan Nieder wrote:\n\n>> As long as you have no credential helpers configured, your GIT_ASKPASS\n>> based approach should work fine.\n>\n> Yeah, it's fine (as is GIT_ASKPASS=true). You could also provide a\n> credential helper that gives you an empty username and password. But in\n> both cases, I think that git will then feed the empty password to the\n> server again, resulting in an extra useless round-trip. You probably\n> instead want to say \"stop now, git, there is nothing else to be done\".\n>\n> We could teach the credential-helper code to do that (e.g., a helper\n> returns \"stop=true\" and we respect that). But I think you can do it\n> reasonably well today by making the input process fail.\n\nHow can my scripts defend against a credential helper that I didn't\nset up that e.g. pops up a GUI window to ask for a password?\n\nThanks,\nJonathan\n"},{"id":"252909","messageId":"20141203013607.GA30037@peff.net","threadId":"38096","inReplyTo":"20141203012950.GC6527@google.com","subject":"Re: Disabling credential helper?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-12-03T01:36:07Z","receivedAt":"2014-12-03T01:36:07Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Dec 02, 2014 at 05:29:50PM -0800, Jonathan Nieder wrote:\n\n> Jeff King wrote:\n> > On Tue, Dec 02, 2014 at 04:59:53PM -0800, Jonathan Nieder wrote:\n> \n> >> As long as you have no credential helpers configured, your GIT_ASKPASS\n> >> based approach should work fine.\n> >\n> > Yeah, it's fine (as is GIT_ASKPASS=true). You could also provide a\n> > credential helper that gives you an empty username and password. But in\n> > both cases, I think that git will then feed the empty password to the\n> > server again, resulting in an extra useless round-trip. You probably\n> > instead want to say \"stop now, git, there is nothing else to be done\".\n> >\n> > We could teach the credential-helper code to do that (e.g., a helper\n> > returns \"stop=true\" and we respect that). But I think you can do it\n> > reasonably well today by making the input process fail.\n> \n> How can my scripts defend against a credential helper that I didn't\n> set up that e.g. pops up a GUI window to ask for a password?\n\nMaybe I am misunderstanding the original situation, but I did not think\nthat was the problem. I thought the situation was one where the\nenvironment was controlled, but Git still would not do what was wanted\n(if you did have such a renegade helper, setting GIT_ASKPASS certainly\nwould not help, as it is the fallback).\n\nBut to answer your question: you can't currently. I would be happy to\nhave a config syntax that means \"reset this multi-value config option\nlist to nothing\", but it does not yet exist. It would be useful for more\nthan just credential-helper config.\n\n-Peff\n"},{"id":"252959","messageId":"xmqqbnnkk64s.fsf@gitster.dls.corp.google.com","threadId":"38096","inReplyTo":"20141203012148.GB29427@peff.net","subject":"Re: Disabling credential helper?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-12-03T17:14:27Z","receivedAt":"2014-12-03T17:14:27Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Yeah, it's fine (as is GIT_ASKPASS=true). You could also provide a\n> credential helper that gives you an empty username and password. But in\n> both cases, I think that git will then feed the empty password to the\n> server again, resulting in an extra useless round-trip.\n> ...\n> You probably\n> instead want to say \"stop now, git, there is nothing else to be done\".\n> ...\n> We could teach the credential-helper code to do that (e.g., a helper\n> returns \"stop=true\" and we respect that). But I think you can do it\n> reasonably well today by making the input process fail.\n\nYeah, it is roughly equivalent to the 'ASKPASS=true' approach, and\nprobably is a good enough solution, I would think.\n"},{"id":"253012","messageId":"20141204004231.GA200195@vauxhall.crustytoothpaste.net","threadId":"38096","inReplyTo":"20141203012148.GB29427@peff.net","subject":"Re: Disabling credential helper?","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2014-12-04T00:42:31Z","receivedAt":"2014-12-04T00:42:31Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Tue, Dec 02, 2014 at 08:21:48PM -0500, Jeff King wrote:\n> We could teach the credential-helper code to do that (e.g., a helper\n> returns \"stop=true\" and we respect that). But I think you can do it\n> reasonably well today by making the input process fail. Sadly setting\n> GIT_ASKPASS to \"false\" just makes git complain and then try harder[1].\n\nYes, I did notice that.  I tried /bin/false at first, and was a bit\nsurprised it wasn't effective.\n\n> But you can dissociate git from the terminal, like:\n> \n>   $ setsid -w git ls-remote https://github.com/private/repo\n>   fatal: could not read Username for 'https://github.com': No such device or address\n\nI think this is a bit heavy-handed for my needs.  At work, we develop on\nheadless VMs, so we use SSH for pushing since we can forward the agent.\nAt home, I use Kerberos, so the prompt generally indicates I need to run\nkinit.\n\nIn neither case do I actually want to enter a password, so the\nenvironment variable will work fine, I think, since it sounds like it's\nat least semi-supported and it works well in scripts and in\nconfiguration files.\n\nAlso, having to patch the Perl git wrappers to use setsid would be more\ninconvenience than it's worth.\n\n> That might have other fallouts if you use process groups for anything. I\n> have no problem with either an option to disable the terminal prompting,\n> or teaching the credential-helper interface to allow helpers to stop\n> lookup, either of which would be cleaner.\n\nI'll probably submit a patch to disable the terminal prompting this\nweekend.\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\n+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187\n"},{"id":"253014","messageId":"20141204013306.GA9406@peff.net","threadId":"38096","inReplyTo":"20141203013607.GA30037@peff.net","subject":"Re: Disabling credential helper?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-12-04T01:33:06Z","receivedAt":"2014-12-04T01:33:06Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Dec 02, 2014 at 08:36:07PM -0500, Jeff King wrote:\n\n> But to answer your question: you can't currently. I would be happy to\n> have a config syntax that means \"reset this multi-value config option\n> list to nothing\", but it does not yet exist. It would be useful for more\n> than just credential-helper config.\n\nThis is something I've wanted for a while, so I took another peek at it.\nIt turns out to be rather complicated.\n\nThis is the initial patch I came up with (don't bother reading it too\nclosely):\n\ndiff --git a/credential.c b/credential.c\nindex 1886ea5..9e84575 100644\n--- a/credential.c\n+++ b/credential.c\n@@ -43,9 +43,6 @@ static int credential_config_callback(const char *var, const char *value,\n \tif (!skip_prefix(var, \"credential.\", &key))\n \t\treturn 0;\n \n-\tif (!value)\n-\t\treturn config_error_nonbool(var);\n-\n \tdot = strrchr(key, '.');\n \tif (dot) {\n \t\tstruct credential want = CREDENTIAL_INIT;\n@@ -63,8 +60,13 @@ static int credential_config_callback(const char *var, const char *value,\n \t\tkey = dot + 1;\n \t}\n \n-\tif (!strcmp(key, \"helper\"))\n-\t\tstring_list_append(&c->helpers, value);\n+\tif (!strcmp(key, \"helper\")) {\n+\t\tif (value)\n+\t\t\tstring_list_append(&c->helpers, value);\n+\t\telse\n+\t\t\tstring_list_clear(&c->helpers, 0);\n+\t} else if (!value)\n+\t\treturn config_error_nonbool(var);\n \telse if (!strcmp(key, \"username\")) {\n \t\tif (!c->username)\n \t\t\tc->username = xstrdup(value);\n\n\nIt allows you to do:\n\n  git -c credential.helper [-c credential.helper=foo] fetch ...\n\nThe first one \"resets\" the list to empty, and then you can further\nappend to the list after that. There are a bunch of shortcomings,\nthough:\n\n  1. I chose the value-less boolean as a token to reset the list (since\n     it is otherwise an unmeaningful error). The example above shows its\n     use with \"-c\", but you could also do:\n\n       [credential]\n       helper\n       helper = foo\n\n     in a config file itself.  This is probably rather unintuitive.\n\n     But whatever syntax is chosen would need to have some mechanism for\n     specifying it in the config file itself, as well as in the\n     command-line (and therefore in the semi-public\n     GIT_CONFIG_PARAMETERS environment variable which is passed around).\n     So this is at least backwards-compatible, \"just works\" with\n     existing config code, and won't stomp on any existing working\n     values.\n\n     If we can accept stomping on an unlikely-used token, something\n     like:\n\n       git -c credential.helper=RESET fetch ...\n\n     is more sensible (and we can argue about the exact token used).\n     If we can accept new syntax and new config code, something like:\n\n       git -c '!credential.helper' fetch ...\n\n     is probably workable.\n\n  2. Notice how we didn't touch the config code at all here. That's\n     because it doesn't know anything about whether a config item is a\n     multi-valued list, or that the string list exists. It is up to each\n     individual consumer of the config callbacks to implement this\n     list-clearing mechanism (and obviously we should make them all\n     agree on the token used). So we'd probably want to make similar\n     changes everywhere that multi-values are used (which, to be fair,\n     really isn't that many, I don't think).\n\n  3. Running `git config credential.helper` doesn't know about this\n     mechanism either. You can either get the \"last one wins\" behavior,\n     or you can use --get-all to get all instances. We'd probably have\n     to teach it a new `--get-list` that recognized the magic reset\n     token.\n\nNone of those problems is insurmountable. It just takes a little more\ncode than what I wrote above. But for credential helpers specifically,\nit's a bit more challenging. Because we're _not_ constructing just a\nlist of just credential.helper here. We're constructing a list of all\nmatching credential.*.helper config keys. So in something like:\n\n  [credential \"http://example.com\"]\n  helper = foo\n  [credential]\n  helper = bar\n\nif you run \"git fetch http://example.com\", you'll get both helpers, in\nsequence.\n\nWhat should adding:\n\n  [credential]\n  helper = RESET\n\ndo on top of that?  Intuitively, I'd say that it would only touch the\ncredential.helper variables. But that's not what the user probably\nwants. They probably want to reset _any_ helpers that have matched. And\nthat's actually what the code I posted above would do. So that's good, I\nguess. But I wonder if this approach is introducing more confusion than\nit is worth.\n\nHaving a separate --no-respect-credential-helpers is conceptually much\nsimpler. But it also does not allow you to reset the list and add in a\nnew helper.\n\n-Peff\n"},{"id":"253018","messageId":"20141204034206.GA1493@peff.net","threadId":"38096","inReplyTo":"20141204004231.GA200195@vauxhall.crustytoothpaste.net","subject":"[PATCH 0/2] disabling terminal prompts","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-12-04T03:42:07Z","receivedAt":"2014-12-04T03:42:07Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Dec 04, 2014 at 12:42:31AM +0000, brian m. carlson wrote:\n\n> I'll probably submit a patch to disable the terminal prompting this\n> weekend.\n\nToo late. You got me thinking about it, so I wrote the following series.\n\n  [1/2]: credential: let helpers tell us to quit\n  [2/2]: prompt: respect GIT_TERMINAL_PROMPT to disable terminal prompts\n\nTechnically the second one is redundant with the first. You could just\ndefine a credential helper which bails before we get to the prompts. But\nfor simple cases, I think that:\n\n  GIT_TERMINAL_PROMPT=0 git ...\n\nis a lot more intuitive than:\n\n  git -c credential.helper='!f() { echo quit=1; }; f' ...\n\nand is also easier to set at the top of a non-interactive script. The\nfirst is not redundant with the second, because it is a lot more\nflexible. E.g., you can abort only after seeing that we match a specific\nURL, or after running any arbitrary code you like (e.g., checking out\nsome characteristics of the tty and deciding it is not a good idea to\naccess it).\n\nThere is one thing this _doesn't_ handle, which is that of preventing\nexisting configured credential-helpers from running (even a helper you\ndefine with \"git -c\" runs after the others). But I think that is a\nseparate issue.\n\n-Peff\n"},{"id":"253020","messageId":"20141204034647.GA21492@peff.net","threadId":"38096","inReplyTo":"20141204034206.GA1493@peff.net","subject":"[PATCH 1/2] credential: let helpers tell us to quit","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-12-04T03:46:48Z","receivedAt":"2014-12-04T03:46:48Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When we are trying to fill a credential, we loop over the\nset of defined credential-helpers, then fall back to running\naskpass, and then finally prompt on the terminal. Helpers\nwhich cannot find a credential are free to tell us nothing,\nbut they cannot currently ask us to stop prompting.\n\nThis patch lets them provide a \"quit\" attribute, which asks\nus to stop the process entirely (avoiding running more\nhelpers, as well as the askpass/terminal prompt).\n\nThis has a few possible uses:\n\n  1. A helper which prompts the user itself (e.g., in a\n     dialog) can provide a \"cancel\" button to the user to\n     stop further prompts.\n\n  2. Some helpers may know that prompting cannot possibly\n     work. For example, if their role is to broker a ticket\n     from an external auth system and that auth system\n     cannot be contacted, there is no point in continuing\n     (we need a ticket to authenticate, and the user cannot\n     provide one by typing it in).\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n+cc spearce, as I recall that you may do something similar to the second\nscenario (and this would prevent annoying useless prompts when the\nhelper fails).\n\n Documentation/technical/api-credentials.txt | 5 ++++-\n credential.c                                | 5 +++++\n credential.h                                | 1 +\n t/t0300-credentials.sh                      | 9 +++++++++\n 4 files changed, 19 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/technical/api-credentials.txt b/Documentation/technical/api-credentials.txt\nindex c1b42a4..e44426d 100644\n--- a/Documentation/technical/api-credentials.txt\n+++ b/Documentation/technical/api-credentials.txt\n@@ -248,7 +248,10 @@ FORMAT` in linkgit:git-credential[7] for a detailed specification).\n For a `get` operation, the helper should produce a list of attributes\n on stdout in the same format. A helper is free to produce a subset, or\n even no values at all if it has nothing useful to provide. Any provided\n-attributes will overwrite those already known about by Git.\n+attributes will overwrite those already known about by Git.  If a helper\n+outputs a `quit` attribute with a value of `true` or `1`, no further\n+helpers will be consulted, nor will the user be prompted (if no\n+credential has been provided, the operation will then fail).\n \n For a `store` or `erase` operation, the helper's output is ignored.\n If it fails to perform the requested operation, it may complain to\ndiff --git a/credential.c b/credential.c\nindex 1886ea5..b146ad8 100644\n--- a/credential.c\n+++ b/credential.c\n@@ -173,6 +173,8 @@ int credential_read(struct credential *c, FILE *fp)\n \t\t\tc->path = xstrdup(value);\n \t\t} else if (!strcmp(key, \"url\")) {\n \t\t\tcredential_from_url(c, value);\n+\t\t} else if (!strcmp(key, \"quit\")) {\n+\t\t\tc->quit = !!git_config_bool(\"quit\", value);\n \t\t}\n \t\t/*\n \t\t * Ignore other lines; we don't know what they mean, but\n@@ -274,6 +276,9 @@ void credential_fill(struct credential *c)\n \t\tcredential_do(c, c->helpers.items[i].string, \"get\");\n \t\tif (c->username && c->password)\n \t\t\treturn;\n+\t\tif (c->quit)\n+\t\t\tdie(\"credential helper '%s' told us to quit\",\n+\t\t\t    c->helpers.items[i].string);\n \t}\n \n \tcredential_getpass(c);\ndiff --git a/credential.h b/credential.h\nindex 0c3e85e..6b0cd16 100644\n--- a/credential.h\n+++ b/credential.h\n@@ -7,6 +7,7 @@ struct credential {\n \tstruct string_list helpers;\n \tunsigned approved:1,\n \t\t configured:1,\n+\t\t quit:1,\n \t\t use_http_path:1;\n \n \tchar *username;\ndiff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh\nindex 57ea5a1..d7ef44b 100755\n--- a/t/t0300-credentials.sh\n+++ b/t/t0300-credentials.sh\n@@ -289,4 +289,13 @@ test_expect_success 'http paths can be part of context' '\n \tEOF\n '\n \n+test_expect_success 'helpers can abort the process' '\n+\ttest_must_fail git \\\n+\t\t-c credential.helper=\"!f() { echo quit=1; }; f\" \\\n+\t\t-c credential.helper=\"verbatim foo bar\" \\\n+\t\tcredential fill >stdout &&\n+\t>expect &&\n+\ttest_cmp expect stdout\n+'\n+\n test_done\n-- \n2.2.0.390.gf60752d\n"},{"id":"253021","messageId":"20141204035228.GB21492@peff.net","threadId":"38096","inReplyTo":"20141204034206.GA1493@peff.net","subject":"[PATCH 2/2] prompt: respect GIT_TERMINAL_PROMPT to disable terminal prompts","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-12-04T03:52:29Z","receivedAt":"2014-12-04T03:52:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"If you run git as part of an automated system, you might\nprefer git to die rather than try to issue a prompt on the\nterminal (because there would be nobody to see it and\nrespond, and the process would hang forever).\n\nThis usually works out of the box because getpass() (and our\nmore featureful replacements) will fail when there is no\ntty, but this does not cover all cases. For example, a batch\nsystem run via ssh might have a tty, even when the user does\nnot expect it.\n\nLet's provide an environment variable the user can set to\navoid even trying to touch the tty at all.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nI also considered adding a \"git --[no-]terminal-prompt\"\noption, but it seemed like overkill to me. I imagine the\ncommon use would be to just set this at the top of a script.\nBut it would be trivial to add if we want to.\n\nNo tests, because our lib-terminal.sh infrastructure is not\nup to the challenge of fooling getpass(). Besides not\nhandling input at all, git will actually open /dev/tty\nitself. So we would have to open a pty and then set it as\nthe controlling terminal for the sub-process.  It's probably\npossible, but I'm not sure the portability headaches are\nworth testing such a trivial feature.\n\n Documentation/git.txt |  4 ++++\n prompt.c              | 16 ++++++++++++----\n 2 files changed, 16 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git.txt b/Documentation/git.txt\nindex afb48d3..54f7a1c 100644\n--- a/Documentation/git.txt\n+++ b/Documentation/git.txt\n@@ -906,6 +906,10 @@ for further details.\n \tand read the password from its STDOUT. See also the 'core.askpass'\n \toption in linkgit:git-config[1].\n \n+'GIT_TERMINAL_PROMPT'::\n+\tIf this environment variable is set to `0`, git will not prompt\n+\ton the terminal (e.g., when asking for HTTP authentication).\n+\n 'GIT_CONFIG_NOSYSTEM'::\n \tWhether to skip reading settings from the system-wide\n \t`$(prefix)/etc/gitconfig` file.  This environment variable can\ndiff --git a/prompt.c b/prompt.c\nindex e5b4938..8181eeb 100644\n--- a/prompt.c\n+++ b/prompt.c\n@@ -57,11 +57,19 @@ char *git_prompt(const char *prompt, int flags)\n \t\t\tr = do_askpass(askpass, prompt);\n \t}\n \n-\tif (!r)\n-\t\tr = git_terminal_prompt(prompt, flags & PROMPT_ECHO);\n \tif (!r) {\n-\t\t/* prompts already contain \": \" at the end */\n-\t\tdie(\"could not read %s%s\", prompt, strerror(errno));\n+\t\tconst char *err;\n+\n+\t\tif (git_env_bool(\"GIT_TERMINAL_PROMPT\", 1)) {\n+\t\t\tr = git_terminal_prompt(prompt, flags & PROMPT_ECHO);\n+\t\t\terr = strerror(errno);\n+\t\t} else {\n+\t\t\terr = \"terminal prompts disabled\";\n+\t\t}\n+\t\tif (!r) {\n+\t\t\t/* prompts already contain \": \" at the end */\n+\t\t\tdie(\"could not read %s%s\", prompt, err);\n+\t\t}\n \t}\n \treturn r;\n }\n-- \n2.2.0.390.gf60752d\n"},{"id":"253023","messageId":"xmqq7fy8uev9.fsf@gitster.dls.corp.google.com","threadId":"38096","inReplyTo":"20141204013306.GA9406@peff.net","subject":"Re: Disabling credential helper?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-12-04T06:07:54Z","receivedAt":"2014-12-04T06:07:54Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n>   1. I chose the value-less boolean as a token to reset the list (since\n>      it is otherwise an unmeaningful error). The example above shows its\n>      use with \"-c\", but you could also do:\n>\n>        [credential]\n>        helper\n>        helper = foo\n>\n>      in a config file itself.  This is probably rather unintuitive.\n\nFor this one, and I suspect all the \"multi-valued\" ones, I think it\nactually is the most sensible syntax (another possiblity is to give\nan empty string, assuming that all multi-valued variables we care\nabout take non-empty string or numeric values), as I do not see a\nuseful/valid use case for wanting to define boolean multi-valued\nvariable.\n\n>      If we can accept stomping on an unlikely-used token, something\n>      like:\n>\n>        git -c credential.helper=RESET fetch ...\n>\n>      is more sensible (and we can argue about the exact token used).\n\nThis unfortunately is unlikely to fly well if we are shooting for a\ngeneric mechanism that is applicable for multi-valued ones in\ngeneral (your comment 2. below is very much relevant and true).\n\n>      If we can accept new syntax and new config code, something like:\n>\n>        git -c '!credential.helper' fetch ...\n>\n>      is probably workable.\n\nI think I suggested exactly this syntax (and \"[credential] !helper\"\nin the config file) when this was brought up the last time, but it\nwas shot down because that would make the resulting configuration\nfile unparsable (not just ignored) by existing versions of Git.\n\nBut perhaps it is a good thing to break existing parser when \"clear\nthe variable settings seen so far\" is used.  It would not do us very\ngood to allow existing implementations to ignore it and continue as\nif all other entries (and special token like RESET) matter will\nsilently give users incorrect result.\n"},{"id":"253071","messageId":"xmqqy4qntgs6.fsf@gitster.dls.corp.google.com","threadId":"38096","inReplyTo":"20141204035228.GB21492@peff.net","subject":"Re: [PATCH 2/2] prompt: respect GIT_TERMINAL_PROMPT to disable terminal prompts","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-12-04T18:24:09Z","receivedAt":"2014-12-04T18:24:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> diff --git a/prompt.c b/prompt.c\n> index e5b4938..8181eeb 100644\n> --- a/prompt.c\n> +++ b/prompt.c\n> @@ -57,11 +57,19 @@ char *git_prompt(const char *prompt, int flags)\n>  \t\t\tr = do_askpass(askpass, prompt);\n>  \t}\n>  \n> -\tif (!r)\n> -\t\tr = git_terminal_prompt(prompt, flags & PROMPT_ECHO);\n>  \tif (!r) {\n> -\t\t/* prompts already contain \": \" at the end */\n> -\t\tdie(\"could not read %s%s\", prompt, strerror(errno));\n> +\t\tconst char *err;\n> +\n> +\t\tif (git_env_bool(\"GIT_TERMINAL_PROMPT\", 1)) {\n> +\t\t\tr = git_terminal_prompt(prompt, flags & PROMPT_ECHO);\n> +\t\t\terr = strerror(errno);\n> +\t\t} else {\n> +\t\t\terr = \"terminal prompts disabled\";\n> +\t\t}\n> +\t\tif (!r) {\n> +\t\t\t/* prompts already contain \": \" at the end */\n> +\t\t\tdie(\"could not read %s%s\", prompt, err);\n> +\t\t}\n>  \t}\n>  \treturn r;\n>  }\n\nI wish this covered a lot more than just this part from an\nend-user's point of view, but this is definitely one of the most\nimportant code paths the mechanism should cover.\n\nThanks, will queue.\n"},{"id":"253095","messageId":"20141204210149.GB19953@peff.net","threadId":"38096","inReplyTo":"xmqqy4qntgs6.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 2/2] prompt: respect GIT_TERMINAL_PROMPT to disable terminal prompts","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-12-04T21:01:49Z","receivedAt":"2014-12-04T21:01:49Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Dec 04, 2014 at 10:24:09AM -0800, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > diff --git a/prompt.c b/prompt.c\n> > index e5b4938..8181eeb 100644\n> > --- a/prompt.c\n> > +++ b/prompt.c\n> > @@ -57,11 +57,19 @@ char *git_prompt(const char *prompt, int flags)\n> >  \t\t\tr = do_askpass(askpass, prompt);\n> >  \t}\n> >  \n> > -\tif (!r)\n> > -\t\tr = git_terminal_prompt(prompt, flags & PROMPT_ECHO);\n> >  \tif (!r) {\n> > -\t\t/* prompts already contain \": \" at the end */\n> > -\t\tdie(\"could not read %s%s\", prompt, strerror(errno));\n> > +\t\tconst char *err;\n> > +\n> > +\t\tif (git_env_bool(\"GIT_TERMINAL_PROMPT\", 1)) {\n> > +\t\t\tr = git_terminal_prompt(prompt, flags & PROMPT_ECHO);\n> > +\t\t\terr = strerror(errno);\n> > +\t\t} else {\n> > +\t\t\terr = \"terminal prompts disabled\";\n> > +\t\t}\n> > +\t\tif (!r) {\n> > +\t\t\t/* prompts already contain \": \" at the end */\n> > +\t\t\tdie(\"could not read %s%s\", prompt, err);\n> > +\t\t}\n> >  \t}\n> >  \treturn r;\n> >  }\n> \n> I wish this covered a lot more than just this part from an\n> end-user's point of view, but this is definitely one of the most\n> important code paths the mechanism should cover.\n\nWhich parts do you mean? Stuff like \"git add -i\"?  I agree it might be\nnice to turn that off, but it is a little bit of a different beast, in\nthat it reads from stdin. The git_prompt code is unique in accessing\n/dev/tty directly, which makes it hard to shut off.\n\nI don't know of any other code in git that  (and it is used in\nmany spots due to calls into the credential_fill code).\n\nI suspect there's similar code in git-svn that comes from the svn\nlibrary, and it might be nice to cover that, too.\n\nAnyway, I'm happy to give other prompts the same treatment, but I think\nwe can wait and add them as they are noticed.\n\n-Peff\n"},{"id":"253100","messageId":"xmqqbnnjrtfi.fsf@gitster.dls.corp.google.com","threadId":"38096","inReplyTo":"20141204210149.GB19953@peff.net","subject":"Re: [PATCH 2/2] prompt: respect GIT_TERMINAL_PROMPT to disable terminal prompts","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-12-04T21:33:53Z","receivedAt":"2014-12-04T21:33:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Thu, Dec 04, 2014 at 10:24:09AM -0800, Junio C Hamano wrote:\n>\n>> I wish this covered a lot more than just this part from an\n>> end-user's point of view, but this is definitely one of the most\n>> important code paths the mechanism should cover.\n>\n> Which parts do you mean? Stuff like \"git add -i\"?\n\nNo, more like \"tag -s\" that eventually leads to somebody prompting\nfor the passphrase to unlock your GPG key---and from an end user's\npoint of view, that somebody is Git.\n\nOf course, from _our_ point of view, that somebody is not us.  We do\nnot have direct control, certainly from this codepath.\n"},{"id":"253155","messageId":"20141205091049.GB32112@peff.net","threadId":"38096","inReplyTo":"xmqqbnnjrtfi.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 2/2] prompt: respect GIT_TERMINAL_PROMPT to disable terminal prompts","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-12-05T09:10:49Z","receivedAt":"2014-12-05T09:10:49Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Dec 04, 2014 at 01:33:53PM -0800, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > On Thu, Dec 04, 2014 at 10:24:09AM -0800, Junio C Hamano wrote:\n> >\n> >> I wish this covered a lot more than just this part from an\n> >> end-user's point of view, but this is definitely one of the most\n> >> important code paths the mechanism should cover.\n> >\n> > Which parts do you mean? Stuff like \"git add -i\"?\n> \n> No, more like \"tag -s\" that eventually leads to somebody prompting\n> for the passphrase to unlock your GPG key---and from an end user's\n> point of view, that somebody is Git.\n\nAh, yeah, I definitely agree that GIT_TERMINAL_PROMPT should work there,\ntoo.\n\n> Of course, from _our_ point of view, that somebody is not us.  We do\n> not have direct control, certainly from this codepath.\n\nRight, but in theory we can provoke gpg to do what we want when we spawn\nit. However, having had zero luck in convincing it to stop asking me for\na passphrase recently in another thread, I do not know what magic\ncommand line option is required. :(\n\nI think it would be OK to merge the git handling of GIT_TERMINAL_PROMPT\n(i.e., the patch I sent), and somebody who runs into the issue with gpg\nand can figure out how to tame it can scratch their own itch later. I\nhate leaving things half-implemented or inconsistent, but I also don't\nknow how to make gpg do what we want. And doing a partial solution\nseems better to me than holding the credential.c half hostage.\n\n-Peff\n"},{"id":"253170","messageId":"xmqqk326q9pp.fsf@gitster.dls.corp.google.com","threadId":"38096","inReplyTo":"20141205091049.GB32112@peff.net","subject":"Re: [PATCH 2/2] prompt: respect GIT_TERMINAL_PROMPT to disable terminal prompts","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-12-05T17:37:22Z","receivedAt":"2014-12-05T17:37:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Thu, Dec 04, 2014 at 01:33:53PM -0800, Junio C Hamano wrote:\n>\n>> Of course, from _our_ point of view, that somebody is not us.  We do\n>> not have direct control, certainly from this codepath.\n\nBelated typofix - s/from /not &/.\n\n>\n> Right, but in theory we can provoke gpg to do what we want when we spawn\n> it. However, having had zero luck in convincing it to stop asking me for\n> a passphrase recently in another thread, I do not know what magic\n> command line option is required. :(\n>\n> I think it would be OK to merge the git handling of GIT_TERMINAL_PROMPT\n> (i.e., the patch I sent), and somebody who runs into the issue with gpg\n> and can figure out how to tame it can scratch their own itch later. I\n> hate leaving things half-implemented or inconsistent, but I also don't\n> know how to make gpg do what we want. And doing a partial solution\n> seems better to me than holding the credential.c half hostage.\n\nOh, no question about that.  You are making things better by\nadvancing one step at a time.\n\nQueued and will be moving to 'next' shortly.\n"}]}