Re: Disabling credential helper?
Jeff King p...@peff.net writes: 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. Yeah, it is roughly equivalent to the 'ASKPASS=true' approach, and probably is a good enough solution, I would think. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Disabling credential helper?
On Tue, Dec 02, 2014 at 08:21:48PM -0500, Jeff King wrote: 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]. Yes, I did notice that. I tried /bin/false at first, and was a bit surprised it wasn't effective. 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 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. 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. 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 signature.asc Description: Digital signature
Re: Disabling credential helper?
On Tue, Dec 02, 2014 at 08:36:07PM -0500, Jeff King wrote:
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.
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):
diff --git a/credential.c b/credential.c
index 1886ea5..9e84575 100644
--- a/credential.c
+++ b/credential.c
@@ -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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Disabling credential helper?
Jeff King p...@peff.net writes: 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. 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. 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). 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). If we can accept new syntax and new config code, something like: git -c '!credential.helper' fetch ... is probably workable. 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. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Disabling credential helper?
(+peff) Hi, brian m. carlson wrote: 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? 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 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Disabling credential helper?
On Tue, Dec 02, 2014 at 04:59:53PM -0800, Jonathan Nieder wrote: brian m. carlson wrote: 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? 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. 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). As long as you have no credential helpers configured, your GIT_ASKPASS based approach should work fine. 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). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Disabling credential helper?
Jeff King wrote: On Tue, Dec 02, 2014 at 04:59:53PM -0800, Jonathan Nieder wrote: As long as you have no credential helpers configured, your GIT_ASKPASS based approach should work fine. 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. 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 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Disabling credential helper?
On Tue, Dec 02, 2014 at 05:29:50PM -0800, Jonathan Nieder wrote: Jeff King wrote: On Tue, Dec 02, 2014 at 04:59:53PM -0800, Jonathan Nieder wrote: As long as you have no credential helpers configured, your GIT_ASKPASS based approach should work fine. 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. 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? 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 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
