[1/3] credential: add new interactive config option

Commit Message

Derrick Stolee Sept. 20, 2024, midnight UTC

From: Derrick Stolee <[e-mail]>

When scripts or background maintenance wish to perform HTTP(S) requests,
there is a risk that our stored credentials might be invalid. At the
moment, this causes the credential helper to ping the user and block the
process. Even if the credential helper does not ping the user, Git falls
back to the 'askpass' method, which includes a direct ping to the user
via the terminal.

Even setting the 'core.askPass' config as something like 'echo' will
causes Git to fallback to a terminal prompt. It uses
git_terminal_prompt(), which finds the terminal from the environment and
ignores whether stdin has been redirected. This can also block the
process awaiting input.

Create a new config option to prevent user interaction, favoring a
failure to a blocked process.

The chosen name, 'credential.interactive', is taken from the config
option used by Git Credential Manager to already avoid user
interactivity, so there is already one credential helper that integrates
with this option. However, older versions of Git Credential Manager also
accepted other string values, including 'auto', 'never', and 'always'.
The modern use is to use a boolean value, but we should still be
careful that some users could have these non-booleans. Further, we
should respect 'never' the same as 'false'. This is respected by the
implementation and test, but not mentioned in the documentation.

The implementation for the Git interactions takes place within
credential_getpass(). The method prototype is modified to return an
'int' instead of 'void'. This allows us to detect that no attempt was
made to fill the given credential, changing the single caller slightly.

Also, a new trace2 region is added around the interactive portion of the
credential request. This provides a way to measure the amount of time
spent in that region for commands that _are_ interactive. It also makes
a conventient way to test that the config option works with
'test_region'.

Signed-off-by: Derrick Stolee <[e-mail]>
---
 Documentation/config/credential.txt |  8 ++++++++
 credential.c                        | 30 ++++++++++++++++++++++++++---
 t/t5551-http-fetch-smart.sh         | 22 +++++++++++++++++++++
 3 files changed, 57 insertions(+), 3 deletions(-)

Comments

Junio C Hamano Sept. 20, 2024, 10:07 p.m. UTC | #1

"Derrick Stolee via GitGitGadget" <[e-mail]> writes:

> @@ -501,8 +525,8 @@ void credential_fill(struct credential *c, int all_capabilities)
>  			    c->helpers.items[i].string);
>  	}
>  
> -	credential_getpass(c);
> -	if (!c->username && !c->password && !c->credential)
> +	if (credential_getpass(c) ||
> +	    (!c->username && !c->password && !c->credential))
>  		die("unable to get password from user");
>  }

This is a fallback mode after credential helpers have failed to fill
and return.  Unless these helpers pay attention to the "interactive"
configuration, they may still get stuck.  So it would be #leftoverbits
to update each credential helpers to do the right thing.

The sample credential-store backend does not have to be updated I
guess ;-)

> diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh
> index 7b5ab0eae16..ceb3336a5c4 100755
> --- a/t/t5551-http-fetch-smart.sh
> +++ b/t/t5551-http-fetch-smart.sh
> @@ -186,6 +186,28 @@ test_expect_success 'clone from password-protected repository' '
>  	test_cmp expect actual
>  '
>  
> +test_expect_success 'credential.interactive=false skips askpass' '
> +	set_askpass bogus nonsense &&
> +	(
> +		GIT_TRACE2_EVENT="$(pwd)/interactive-true" &&
> +		export GIT_TRACE2_EVENT &&
> +		test_must_fail git clone --bare "$HTTPD_URL/auth/smart/repo.git" interactive-true-dir &&
> +		test_region credential interactive interactive-true &&
> +
> +		GIT_TRACE2_EVENT="$(pwd)/interactive-false" &&
> +		export GIT_TRACE2_EVENT &&
> +		test_must_fail git -c credential.interactive=false \
> +			clone --bare "$HTTPD_URL/auth/smart/repo.git" interactive-false-dir &&
> +		test_region ! credential interactive interactive-false &&
> +
> +		GIT_TRACE2_EVENT="$(pwd)/interactive-never" &&
> +		export GIT_TRACE2_EVENT &&
> +		test_must_fail git -c credential.interactive=never \
> +			clone --bare "$HTTPD_URL/auth/smart/repo.git" interactive-never-dir &&
> +		test_region ! credential interactive interactive-never
> +	)
> +'
> +
>  test_expect_success 'clone from auth-only-for-push repository' '
>  	echo two >expect &&
>  	set_askpass wrong &&

Patch

diff --git a/Documentation/config/credential.txt b/Documentation/config/credential.txt
index 0221c3e620d..470482ff4c2 100644
--- a/Documentation/config/credential.txt
+++ b/Documentation/config/credential.txt
@@ -9,6 +9,14 @@  credential.helper::
 Note that multiple helpers may be defined. See linkgit:gitcredentials[7]
 for details and examples.
 
+credential.interactive::
+	By default, Git and any configured credential helpers will ask for
+	user input when new credentials are required. Many of these helpers
+	will succeed based on stored credentials if those credentials are
+	still valid. To avoid the possibility of user interactivity from
+	Git, set `credential.interactive=false`. Some credential helpers
+	respect this option as well.
+
 credential.useHttpPath::
 	When acquiring credentials, consider the "path" component of an http
 	or https URL to be important. Defaults to false. See
diff --git a/credential.c b/credential.c
index ee46351ce01..6dea3859ece 100644
--- a/credential.c
+++ b/credential.c
@@ -13,6 +13,8 @@ 
 #include "strbuf.h"
 #include "urlmatch.h"
 #include "git-compat-util.h"
+#include "trace2.h"
+#include "repository.h"
 
 void credential_init(struct credential *c)
 {
@@ -251,14 +253,36 @@  static char *credential_ask_one(const char *what, struct credential *c,
 	return xstrdup(r);
 }
 
-static void credential_getpass(struct credential *c)
+static int credential_getpass(struct credential *c)
 {
+	int interactive;
+	char *value;
+	if (!git_config_get_maybe_bool("credential.interactive", &interactive) &&
+	    !interactive) {
+		trace2_data_intmax("credential", the_repository,
+				   "interactive/skipped", 1);
+		return -1;
+	}
+	if (!git_config_get_string("credential.interactive", &value)) {
+		int same = !strcmp(value, "never");
+		free(value);
+		if (same) {
+			trace2_data_intmax("credential", the_repository,
+					   "interactive/skipped", 1);
+			return -1;
+		}
+	}
+
+	trace2_region_enter("credential", "interactive", the_repository);
 	if (!c->username)
 		c->username = credential_ask_one("Username", c,
 						 PROMPT_ASKPASS|PROMPT_ECHO);
 	if (!c->password)
 		c->password = credential_ask_one("Password", c,
 						 PROMPT_ASKPASS);
+	trace2_region_leave("credential", "interactive", the_repository);
+
+	return 0;
 }
 
 int credential_has_capability(const struct credential_capability *capa,
@@ -501,8 +525,8 @@  void credential_fill(struct credential *c, int all_capabilities)
 			    c->helpers.items[i].string);
 	}
 
-	credential_getpass(c);
-	if (!c->username && !c->password && !c->credential)
+	if (credential_getpass(c) ||
+	    (!c->username && !c->password && !c->credential))
 		die("unable to get password from user");
 }
 
diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh
index 7b5ab0eae16..ceb3336a5c4 100755
--- a/t/t5551-http-fetch-smart.sh
+++ b/t/t5551-http-fetch-smart.sh
@@ -186,6 +186,28 @@  test_expect_success 'clone from password-protected repository' '
 	test_cmp expect actual
 '
 
+test_expect_success 'credential.interactive=false skips askpass' '
+	set_askpass bogus nonsense &&
+	(
+		GIT_TRACE2_EVENT="$(pwd)/interactive-true" &&
+		export GIT_TRACE2_EVENT &&
+		test_must_fail git clone --bare "$HTTPD_URL/auth/smart/repo.git" interactive-true-dir &&
+		test_region credential interactive interactive-true &&
+
+		GIT_TRACE2_EVENT="$(pwd)/interactive-false" &&
+		export GIT_TRACE2_EVENT &&
+		test_must_fail git -c credential.interactive=false \
+			clone --bare "$HTTPD_URL/auth/smart/repo.git" interactive-false-dir &&
+		test_region ! credential interactive interactive-false &&
+
+		GIT_TRACE2_EVENT="$(pwd)/interactive-never" &&
+		export GIT_TRACE2_EVENT &&
+		test_must_fail git -c credential.interactive=never \
+			clone --bare "$HTTPD_URL/auth/smart/repo.git" interactive-never-dir &&
+		test_region ! credential interactive interactive-never
+	)
+'
+
 test_expect_success 'clone from auth-only-for-push repository' '
 	echo two >expect &&
 	set_askpass wrong &&