Disabling credential helper?

At $DAYJOB, we have a Git server[0] that supports the smart HTTP protocol. That server can return a 401 if the repository is private or doesn't exist.

We have several scripts, some of which run interactively, some not, that we simply want to fail if git fetch gets a non-2xx code. Unfortunately, git is very insistent about trying to use the default credential helper to prompt for a username and password in this case, even opening /dev/tty.

We've used GIT_ASKPASS=/bin/echo, which seems to solve the problem, although it's ugly and I'm concerned it might break in the future. Is there a better way to do this? I didn't see one in the documentation or code when I looked.

[0] An Atlassian Stash instance.

brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187

(+peff)
Hi,

That's a good question. Before falling back to the askpass based prompt, Git tries each credential helper matching the URL in turn, and there doesn't seem to be an option to override that behavior and disable credential helpers.

As long as you have no credential helpers configured, your GIT_ASKPASS based approach should work fine.

But once you have helpers configured, you're potentially in trouble. I'm wondering if we ought to provide an --no-credential-helpers option to help with this. (Or to go further and provide a way to unset configuration items --- e.g., '-c "credential.*=unset"'.)

Thoughts?
Jonathan

I think this has nothing at all to do with credential helpers. Git tries the helpers, of which there are none, and falls back to askpass and prompting on the terminal. There is no way to design a helper to say "I tried and failed; do not bother prompting on the terminal". Git only sees that no helper provided an answer and uses its internal methods.

I did at one point consider making the terminal prompt a credential helper (since it is, after all, no different from git's perspective; it's just a mechanism for "somehow" coming up with a username/password pair). But people generally thought that was unnecessarily complicated (and it certainly is for the common cases).

Yeah, it's fine (as is GIT_ASKPASS=true). You could also provide a credential helper that gives you an empty username and password. But in both cases, I think that git will then feed the empty password to the server again, resulting in an extra useless round-trip. You probably instead want to say "stop now, git, there is nothing else to be done".

We could teach the credential-helper code to do that (e.g., a helper returns "stop=true" and we respect that). But I think you can do it reasonably well today by making the input process fail. Sadly setting GIT_ASKPASS to "false" just makes git complain and then try harder[1]. But you can dissociate git from the terminal, like:

  $ setsid -w git ls-remote https://github.com/private/repo
  fatal: could not read Username for 'https://github.com': No such device or address

That might have other fallouts if you use process groups for anything. I have no problem with either an option to disable the terminal prompting, or teaching the credential-helper interface to allow helpers to stop lookup, either of which would be cleaner.

-Peff

[1] Courtesy of 84d7273 (prompt: fall back to terminal if askpass fails, 2012-02-03).

How can my scripts defend against a credential helper that I didn't set up that e.g. pops up a GUI window to ask for a password?

Thanks,
Jonathan

Maybe I am misunderstanding the original situation, but I did not think that was the problem. I thought the situation was one where the environment was controlled, but Git still would not do what was wanted (if you did have such a renegade helper, setting GIT_ASKPASS certainly would not help, as it is the fallback).

But to answer your question: you can't currently. I would be happy to have a config syntax that means "reset this multi-value config option list to nothing", but it does not yet exist. It would be useful for more than just credential-helper config.

-Peff

This is something I've wanted for a while, so I took another peek at it. It turns out to be rather complicated.

This is the initial patch I came up with (don't bother reading it too closely):

index 1886ea5..9e84575 100644
@@ -43,9 +43,6 @@ static int credential_config_callback(const char *var, const char *value,
 	if (!skip_prefix(var, "credential.", &key))
 		return 0;
 
-	if (!value)
-		return config_error_nonbool(var);
-
 	dot = strrchr(key, '.');
 	if (dot) {
 		struct credential want = CREDENTIAL_INIT;

@@ -63,8 +60,13 @@ static int credential_config_callback(const char *var, const char *value,
 		key = dot + 1;
 	}
 
-	if (!strcmp(key, "helper"))
-		string_list_append(&c->helpers, value);
+	if (!strcmp(key, "helper")) {
+		if (value)
+			string_list_append(&c->helpers, value);
+		else
+			string_list_clear(&c->helpers, 0);
+	} else if (!value)
+		return config_error_nonbool(var);
 	else if (!strcmp(key, "username")) {
 		if (!c->username)
 			c->username = xstrdup(value);

It allows you to do:

git -c credential.helper [-c credential.helper=foo] fetch ...

The first one "resets" the list to empty, and then you can further append to the list after that. There are a bunch of shortcomings, though:

1. I chose the value-less boolean as a token to reset the list (since it is otherwise an unmeaningful error). The example above shows its use with "-c", but you could also do:

[credential]
helper
helper = foo

in a config file itself. This is probably rather unintuitive.

But whatever syntax is chosen would need to have some mechanism for specifying it in the config file itself, as well as in the command-line (and therefore in the semi-public GIT_CONFIG_PARAMETERS environment variable which is passed around). So this is at least backwards-compatible, "just works" with existing config code, and won't stomp on any existing working values.

If we can accept stomping on an unlikely-used token, something like:

git -c credential.helper=RESET fetch ...

is more sensible (and we can argue about the exact token used). If we can accept new syntax and new config code, something like:

git -c '!credential.helper' fetch ...

is probably workable.

2. Notice how we didn't touch the config code at all here. That's because it doesn't know anything about whether a config item is a multi-valued list, or that the string list exists. It is up to each individual consumer of the config callbacks to implement this list-clearing mechanism (and obviously we should make them all agree on the token used). So we'd probably want to make similar changes everywhere that multi-values are used (which, to be fair, really isn't that many, I don't think).

3. Running git config credential.helper doesn't know about this mechanism either. You can either get the "last one wins" behavior, or you can use --get-all to get all instances. We'd probably have to teach it a new --get-list that recognized the magic reset token.

None of those problems is insurmountable. It just takes a little more code than what I wrote above. But for credential helpers specifically, it's a bit more challenging. Because we're _not_ constructing just a list of just credential.helper here. We're constructing a list of all matching credential.*.helper config keys. So in something like:

[credential "http://example.com"]
helper = foo
[credential]
helper = bar

if you run "git fetch http://example.com", you'll get both helpers, in sequence.

What should adding:

[credential]
helper = RESET

do on top of that? Intuitively, I'd say that it would only touch the credential.helper variables. But that's not what the user probably wants. They probably want to reset _any_ helpers that have matched. And that's actually what the code I posted above would do. So that's good, I guess. But I wonder if this approach is introducing more confusion than it is worth.

Having a separate --no-respect-credential-helpers is conceptually much simpler. But it also does not allow you to reset the list and add in a new helper.

-Peff

For this one, and I suspect all the "multi-valued" ones, I think it actually is the most sensible syntax (another possiblity is to give an empty string, assuming that all multi-valued variables we care about take non-empty string or numeric values), as I do not see a useful/valid use case for wanting to define boolean multi-valued variable.

This unfortunately is unlikely to fly well if we are shooting for a generic mechanism that is applicable for multi-valued ones in general (your comment 2. below is very much relevant and true).

I think I suggested exactly this syntax (and "[credential] !helper" in the config file) when this was brought up the last time, but it was shot down because that would make the resulting configuration file unparsable (not just ignored) by existing versions of Git.

But perhaps it is a good thing to break existing parser when "clear the variable settings seen so far" is used. It would not do us very good to allow existing implementations to ignore it and continue as if all other entries (and special token like RESET) matter will silently give users incorrect result.

Yes, I did notice that. I tried /bin/false at first, and was a bit surprised it wasn't effective.

I think this is a bit heavy-handed for my needs. At work, we develop on headless VMs, so we use SSH for pushing since we can forward the agent. At home, I use Kerberos, so the prompt generally indicates I need to run kinit.

In neither case do I actually want to enter a password, so the environment variable will work fine, I think, since it sounds like it's at least semi-supported and it works well in scripts and in configuration files.

Also, having to patch the Perl git wrappers to use setsid would be more inconvenience than it's worth.

I'll probably submit a patch to disable the terminal prompting this weekend.

brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187

Too late. You got me thinking about it, so I wrote the following series.

[1/2]: credential: let helpers tell us to quit
[2/2]: prompt: respect GIT_TERMINAL_PROMPT to disable terminal prompts

Technically the second one is redundant with the first. You could just define a credential helper which bails before we get to the prompts. But for simple cases, I think that:

GIT_TERMINAL_PROMPT=0 git ...

is a lot more intuitive than:

git -c credential.helper='!f() { echo quit=1; }; f' ...

and is also easier to set at the top of a non-interactive script. The first is not redundant with the second, because it is a lot more flexible. E.g., you can abort only after seeing that we match a specific URL, or after running any arbitrary code you like (e.g., checking out some characteristics of the tty and deciding it is not a good idea to access it).

There is one thing this _doesn't_ handle, which is that of preventing existing configured credential-helpers from running (even a helper you define with "git -c" runs after the others). But I think that is a separate issue.

-Peff

When we are trying to fill a credential, we loop over the set of defined credential-helpers, then fall back to running askpass, and then finally prompt on the terminal. Helpers which cannot find a credential are free to tell us nothing, but they cannot currently ask us to stop prompting.

This patch lets them provide a "quit" attribute, which asks us to stop the process entirely (avoiding running more helpers, as well as the askpass/terminal prompt).

This has a few possible uses:

1. A helper which prompts the user itself (e.g., in a dialog) can provide a "cancel" button to the user to stop further prompts.

2. Some helpers may know that prompting cannot possibly work. For example, if their role is to broker a ticket from an external auth system and that auth system cannot be contacted, there is no point in continuing (we need a ticket to authenticate, and the user cannot provide one by typing it in).

---
+cc spearce, as I recall that you may do something similar to the second scenario (and this would prevent annoying useless prompts when the helper fails).

 Documentation/technical/api-credentials.txt | 5 ++++-
 credential.c                                | 5 +++++
 credential.h                                | 1 +
 t/t0300-credentials.sh                      | 9 +++++++++
 4 files changed, 19 insertions(+), 1 deletion(-)

index c1b42a4..e44426d 100644
@@ -248,7 +248,10 @@ FORMAT` in linkgit:git-credential[7] for a detailed specification).
 For a `get` operation, the helper should produce a list of attributes
 on stdout in the same format. A helper is free to produce a subset, or
 even no values at all if it has nothing useful to provide. Any provided
-attributes will overwrite those already known about by Git.
+attributes will overwrite those already known about by Git.  If a helper
+outputs a `quit` attribute with a value of `true` or `1`, no further
+helpers will be consulted, nor will the user be prompted (if no
+credential has been provided, the operation will then fail).
 
 For a `store` or `erase` operation, the helper's output is ignored.
 If it fails to perform the requested operation, it may complain to

index 1886ea5..b146ad8 100644
@@ -173,6 +173,8 @@ int credential_read(struct credential *c, FILE *fp)
 			c->path = xstrdup(value);
 		} else if (!strcmp(key, "url")) {
 			credential_from_url(c, value);
+		} else if (!strcmp(key, "quit")) {
+			c->quit = !!git_config_bool("quit", value);
 		}
 		/*
 		 * Ignore other lines; we don't know what they mean, but

@@ -274,6 +276,9 @@ void credential_fill(struct credential *c)
 		credential_do(c, c->helpers.items[i].string, "get");
 		if (c->username && c->password)
 			return;
+		if (c->quit)
+			die("credential helper '%s' told us to quit",
+			    c->helpers.items[i].string);
 	}
 
 	credential_getpass(c);

index 0c3e85e..6b0cd16 100644
@@ -7,6 +7,7 @@ struct credential {
 	struct string_list helpers;
 	unsigned approved:1,
 		 configured:1,
+		 quit:1,
 		 use_http_path:1;
 
 	char *username;

index 57ea5a1..d7ef44b 100755
@@ -289,4 +289,13 @@ test_expect_success 'http paths can be part of context' '
 	EOF
 '
 
+test_expect_success 'helpers can abort the process' '
+	test_must_fail git \
+		-c credential.helper="!f() { echo quit=1; }; f" \
+		-c credential.helper="verbatim foo bar" \
+		credential fill >stdout &&
+	>expect &&
+	test_cmp expect stdout
+'
+
 test_done

2.2.0.390.gf60752d

If you run git as part of an automated system, you might prefer git to die rather than try to issue a prompt on the terminal (because there would be nobody to see it and respond, and the process would hang forever).

This usually works out of the box because getpass() (and our more featureful replacements) will fail when there is no tty, but this does not cover all cases. For example, a batch system run via ssh might have a tty, even when the user does not expect it.

Let's provide an environment variable the user can set to avoid even trying to touch the tty at all.

---
I also considered adding a "git --[no-]terminal-prompt" option, but it seemed like overkill to me. I imagine the common use would be to just set this at the top of a script. But it would be trivial to add if we want to.

No tests, because our lib-terminal.sh infrastructure is not up to the challenge of fooling getpass(). Besides not handling input at all, git will actually open /dev/tty itself. So we would have to open a pty and then set it as the controlling terminal for the sub-process. It's probably possible, but I'm not sure the portability headaches are worth testing such a trivial feature.

 Documentation/git.txt |  4 ++++
 prompt.c              | 16 ++++++++++++----
 2 files changed, 16 insertions(+), 4 deletions(-)

index afb48d3..54f7a1c 100644
@@ -906,6 +906,10 @@ for further details.
 	and read the password from its STDOUT. See also the 'core.askpass'
 	option in linkgit:git-config[1].
 
+'GIT_TERMINAL_PROMPT'::
+	If this environment variable is set to `0`, git will not prompt
+	on the terminal (e.g., when asking for HTTP authentication).
+
 'GIT_CONFIG_NOSYSTEM'::
 	Whether to skip reading settings from the system-wide
 	`$(prefix)/etc/gitconfig` file.  This environment variable can

index e5b4938..8181eeb 100644
@@ -57,11 +57,19 @@ char *git_prompt(const char *prompt, int flags)
 			r = do_askpass(askpass, prompt);
 	}
 
-	if (!r)
-		r = git_terminal_prompt(prompt, flags & PROMPT_ECHO);
 	if (!r) {
-		/* prompts already contain ": " at the end */
-		die("could not read %s%s", prompt, strerror(errno));
+		const char *err;
+
+		if (git_env_bool("GIT_TERMINAL_PROMPT", 1)) {
+			r = git_terminal_prompt(prompt, flags & PROMPT_ECHO);
+			err = strerror(errno);
+		} else {
+			err = "terminal prompts disabled";
+		}
+		if (!r) {
+			/* prompts already contain ": " at the end */
+			die("could not read %s%s", prompt, err);
+		}
 	}
 	return r;
 }

2.2.0.390.gf60752d

Which parts do you mean? Stuff like "git add -i"? I agree it might be nice to turn that off, but it is a little bit of a different beast, in that it reads from stdin. The git_prompt code is unique in accessing /dev/tty directly, which makes it hard to shut off.

I don't know of any other code in git that (and it is used in many spots due to calls into the credential_fill code).

I suspect there's similar code in git-svn that comes from the svn library, and it might be nice to cover that, too.

Anyway, I'm happy to give other prompts the same treatment, but I think we can wait and add them as they are noticed.

-Peff

No, more like "tag -s" that eventually leads to somebody prompting for the passphrase to unlock your GPG key---and from an end user's point of view, that somebody is Git.

Of course, from _our_ point of view, that somebody is not us. We do not have direct control, certainly from this codepath.

Ah, yeah, I definitely agree that GIT_TERMINAL_PROMPT should work there, too.

Right, but in theory we can provoke gpg to do what we want when we spawn it. However, having had zero luck in convincing it to stop asking me for a passphrase recently in another thread, I do not know what magic command line option is required. :(

I think it would be OK to merge the git handling of GIT_TERMINAL_PROMPT (i.e., the patch I sent), and somebody who runs into the issue with gpg and can figure out how to tame it can scratch their own itch later. I hate leaving things half-implemented or inconsistent, but I also don't know how to make gpg do what we want. And doing a partial solution seems better to me than holding the credential.c half hostage.

-Peff

Belated typofix - s/from /not &/.

Oh, no question about that. You are making things better by advancing one step at a time.

Queued and will be moving to 'next' shortly.

Yeah, it is roughly equivalent to the 'ASKPASS=true' approach, and probably is a good enough solution, I would think.