Subject: [PATCH] credential: warn about git-credential-store [RFC]

From: "M Hickford via GitGitGadget" <[email-masked]>
Date: Fri, 31 Jan 2025 19:48:06 +0000

From: M Hickford <[email-masked]>

git-credential-store saves secrets unencrypted on disk.

Warn the user before they type their password, suggesting alternative
credential helpers.

An alternative could be to warn in "credential-store store". A
disadvantage is that the user wouldn't see the warning until after they
typed their password, which is less helpful. The warning would appear
again every time the user authenticated, which feels too frequently.

Signed-off-by: M Hickford <[email-masked]>
---
    credential: warn about git-credential-store [RFC]
    
    RFC for discussion. Some tests fail

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1856%2Fhickford%2Fstore-warn-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1856/hickford/store-warn-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1856

 credential.c                | 6 +++++-
 t/lib-credential.sh         | 2 ++
 t/t0302-credential-store.sh | 3 +++
 3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/credential.c b/credential.c
index 2594c0c4229..6e05bba7e2f 100644
--- a/credential.c
+++ b/credential.c
@@ -285,9 +285,13 @@ static int credential_getpass(struct repository *r, struct credential *c)
 	if (!c->username)
 		c->username = credential_ask_one("Username", c,
 						 PROMPT_ASKPASS|PROMPT_ECHO);
-	if (!c->password)
+	if (!c->password) {
+		if (c->helpers.nr >= 1 && starts_with(c->helpers.items[0].string, "store"))
+			warning("git-credential-store saves passwords unencrypted on disk. For alternatives, see gitcredentials(7).");
+
 		c->password = credential_ask_one("Password", c,
 						 PROMPT_ASKPASS);
+	}
 	trace2_region_leave("credential", "interactive", r);
 
 	return 0;
diff --git a/t/lib-credential.sh b/t/lib-credential.sh
index 58b9c740605..47483f09006 100644
--- a/t/lib-credential.sh
+++ b/t/lib-credential.sh
@@ -67,6 +67,8 @@ reject() {
 helper_test() {
 	HELPER=$1
 
+	# help wanted: expect warning "git-credential-store saves passwords
+	# unencrypted" when helper equals "store"
 	test_expect_success "helper ($HELPER) has no existing data" '
 		check fill $HELPER <<-\EOF
 		protocol=https
diff --git a/t/t0302-credential-store.sh b/t/t0302-credential-store.sh
index c1cd60edd01..349b5f0b084 100755
--- a/t/t0302-credential-store.sh
+++ b/t/t0302-credential-store.sh
@@ -133,6 +133,7 @@ invalid_credential_test() {
 		password=askpass-password
 		--
 		askpass: Username for '\''https://example.com'\'':
+		warning: git-credential-store saves passwords unencrypted on disk. For alternatives, see gitcredentials(7) or https://git-scm.com/doc/credential-helpers.
 		askpass: Password for '\''https://[email-masked]'\'':
 		--
 		EOF
@@ -155,6 +156,7 @@ test_expect_success 'get: credentials with DOS line endings are invalid' '
 	password=askpass-password
 	--
 	askpass: Username for '\''https://example.com'\'':
+	warning: git-credential-store saves passwords unencrypted on disk. For alternatives, see gitcredentials(7) or https://git-scm.com/doc/credential-helpers.
 	askpass: Password for '\''https://[email-masked]'\'':
 	--
 	EOF
@@ -186,6 +188,7 @@ test_expect_success 'get: credentials with DOS line endings are invalid if path
 	password=askpass-password
 	--
 	askpass: Username for '\''https://example.com/repo.git'\'':
+	warning: git-credential-store saves passwords unencrypted on disk. For alternatives, see gitcredentials(7) or https://git-scm.com/doc/credential-helpers.
 	askpass: Password for '\''https://[email-masked]/repo.git'\'':
 	--
 	EOF

base-commit: 4e746b1a31f9f0036032b6f94279cf16fb363203
-- 
gitgitgadget