Merge branch 'kn/osxkeychain-idempotent-store-fix'

An earlier check added to osx keychain credential helper to avoid storing the credential itself supplied was overeager and rejected credential material supplied by other helper backends that it would have wanted to store, which has been corrected. * kn/osxkeychain-idempotent-store-fix: osxkeychain: avoid incorrectly skipping store operation
author: Junio C Hamano <gitster@pobox.com> 2025-11-24 15:46:41 -0800
committer: Junio C Hamano <gitster@pobox.com> 2025-11-24 15:46:41 -0800
commit: 861312b51d761b7a787e3a5d71ab2edf266ad297 (patch)
tree: 344a6a365fdc83f1d9b30fe315a41ddac8b38981
parent: aa934e09502b222cee362cda42982cabfc550ebb (diff)
parent: 4580bcd2354aab9369164d936f7ccaa21fc98c98 (diff)
download: git-861312b51d761b7a787e3a5d71ab2edf266ad297.tar.gz
3 files changed, 132 insertions, 30 deletions
diff --git a/contrib/credential/osxkeychain/Makefile b/contrib/credential/osxkeychain/Makefile
index 9680717abe..c68445b82d 100644
--- a/contrib/credential/osxkeychain/Makefile
+++ b/contrib/credential/osxkeychain/Makefile
@@ -1,21 +1,55 @@
 # The default target of this Makefile is...
 all:: git-credential-osxkeychain
 
+include ../../../config.mak.uname
 -include ../../../config.mak.autogen
 -include ../../../config.mak
 
+ifdef ZLIB_NG
+	BASIC_CFLAGS += -DHAVE_ZLIB_NG
+        ifdef ZLIB_NG_PATH
+		BASIC_CFLAGS += -I$(ZLIB_NG_PATH)/include
+		EXTLIBS += $(call libpath_template,$(ZLIB_NG_PATH)/$(lib))
+        endif
+	EXTLIBS += -lz-ng
+else
+        ifdef ZLIB_PATH
+		BASIC_CFLAGS += -I$(ZLIB_PATH)/include
+		EXTLIBS += $(call libpath_template,$(ZLIB_PATH)/$(lib))
+        endif
+	EXTLIBS += -lz
+endif
+ifndef NO_ICONV
+        ifdef NEEDS_LIBICONV
+                ifdef ICONVDIR
+			BASIC_CFLAGS += -I$(ICONVDIR)/include
+			ICONV_LINK = $(call libpath_template,$(ICONVDIR)/$(lib))
+                else
+			ICONV_LINK =
+                endif
+                ifdef NEEDS_LIBINTL_BEFORE_LIBICONV
+			ICONV_LINK += -lintl
+                endif
+		EXTLIBS += $(ICONV_LINK) -liconv
+        endif
+endif
+ifndef LIBC_CONTAINS_LIBINTL
+	EXTLIBS += -lintl
+endif
+
 prefix ?= /usr/local
 gitexecdir ?= $(prefix)/libexec/git-core
 
 CC ?= gcc
-CFLAGS ?= -g -O2 -Wall
+CFLAGS ?= -g -O2 -Wall -I../../.. $(BASIC_CFLAGS)
+LDFLAGS ?= $(BASIC_LDFLAGS) $(EXTLIBS)
 INSTALL ?= install
 RM ?= rm -f
 
 %.o: %.c
 	$(CC) $(CFLAGS) $(CPPFLAGS) -o $@ -c $<
 
-git-credential-osxkeychain: git-credential-osxkeychain.o
+git-credential-osxkeychain: git-credential-osxkeychain.o ../../../libgit.a
 	$(CC) $(CFLAGS) -o $@ $^ $(LDFLAGS) \
 		-framework Security -framework CoreFoundation
 
@@ -23,6 +57,9 @@ install: git-credential-osxkeychain
 	$(INSTALL) -d -m 755 $(DESTDIR)$(gitexecdir)
 	$(INSTALL) -m 755 $< $(DESTDIR)$(gitexecdir)
 
+../../../libgit.a:
+	cd ../../..; make libgit.a
+
 clean:
 	$(RM) git-credential-osxkeychain git-credential-osxkeychain.o
 
diff --git a/contrib/credential/osxkeychain/git-credential-osxkeychain.c b/contrib/credential/osxkeychain/git-credential-osxkeychain.c
index 611c9798b3..b180267034 100644
--- a/contrib/credential/osxkeychain/git-credential-osxkeychain.c
+++ b/contrib/credential/osxkeychain/git-credential-osxkeychain.c
@@ -2,6 +2,9 @@
 #include <string.h>
 #include <stdlib.h>
 #include <Security/Security.h>
+#include "git-compat-util.h"
+#include "strbuf.h"
+#include "wrapper.h"
 
 #define ENCODING kCFStringEncodingUTF8
 static CFStringRef protocol; /* Stores constant strings - not memory managed */
@@ -12,7 +15,7 @@ static CFStringRef username;
 static CFDataRef password;
 static CFDataRef password_expiry_utc;
 static CFDataRef oauth_refresh_token;
-static int state_seen;
+static char *state_seen;
 
 static void clear_credential(void)
 {
@@ -48,27 +51,6 @@ static void clear_credential(void)
 
 #define STRING_WITH_LENGTH(s) s, sizeof(s) - 1
 
-__attribute__((format (printf, 1, 2), __noreturn__))
-static void die(const char *err, ...)
-{
-	char msg[4096];
-	va_list params;
-	va_start(params, err);
-	vsnprintf(msg, sizeof(msg), err, params);
-	fprintf(stderr, "%s\n", msg);
-	va_end(params);
-	clear_credential();
-	exit(1);
-}
-
-static void *xmalloc(size_t len)
-{
-	void *ret = malloc(len);
-	if (!ret)
-		die("Out of memory");
-	return ret;
-}
-
 static CFDictionaryRef create_dictionary(CFAllocatorRef allocator, ...)
 {
 	va_list args;
@@ -112,6 +94,66 @@ static void write_item(const char *what, const char *buf, size_t len)
 	putchar('\n');
 }
 
+static void write_item_strbuf(struct strbuf *sb, const char *what, const char *buf, int n)
+{
+	char s[32];
+
+	xsnprintf(s, sizeof(s), "__%s=", what);
+	strbuf_add(sb, s, strlen(s));
+	strbuf_add(sb, buf, n);
+}
+
+static void write_item_strbuf_cfstring(struct strbuf *sb, const char *what, CFStringRef ref)
+{
+	char *buf;
+	int len;
+
+	if (!ref)
+		return;
+	len = CFStringGetMaximumSizeForEncoding(CFStringGetLength(ref), ENCODING) + 1;
+	buf = xmalloc(len);
+	if (CFStringGetCString(ref, buf, len, ENCODING))
+		write_item_strbuf(sb, what, buf, strlen(buf));
+	free(buf);
+}
+
+static void write_item_strbuf_cfnumber(struct strbuf *sb, const char *what, CFNumberRef ref)
+{
+	short n;
+	char buf[32];
+
+	if (!ref)
+		return;
+	if (!CFNumberGetValue(ref, kCFNumberShortType, &n))
+		return;
+	xsnprintf(buf, sizeof(buf), "%d", n);
+	write_item_strbuf(sb, what, buf, strlen(buf));
+}
+
+static void write_item_strbuf_cfdata(struct strbuf *sb, const char *what, CFDataRef ref)
+{
+	char *buf;
+	int len;
+
+	if (!ref)
+		return;
+	buf = (char *)CFDataGetBytePtr(ref);
+	if (!buf || strlen(buf) == 0)
+		return;
+	len = CFDataGetLength(ref);
+	write_item_strbuf(sb, what, buf, len);
+}
+
+static void encode_state_seen(struct strbuf *sb)
+{
+	strbuf_add(sb, "osxkeychain:seen=", strlen("osxkeychain:seen="));
+	write_item_strbuf_cfstring(sb, "host", host);
+	write_item_strbuf_cfnumber(sb, "port", port);
+	write_item_strbuf_cfstring(sb, "path", path);
+	write_item_strbuf_cfstring(sb, "username", username);
+	write_item_strbuf_cfdata(sb, "password", password);
+}
+
 static void find_username_in_item(CFDictionaryRef item)
 {
 	CFStringRef account_ref;
@@ -124,6 +166,7 @@ static void find_username_in_item(CFDictionaryRef item)
 		write_item("username", "", 0);
 		return;
 	}
+	username = CFStringCreateCopy(kCFAllocatorDefault, account_ref);
 
 	username_buf = (char *)CFStringGetCStringPtr(account_ref, ENCODING);
 	if (username_buf)
@@ -163,6 +206,7 @@ static OSStatus find_internet_password(void)
 	}
 
 	data = CFDictionaryGetValue(item, kSecValueData);
+	password = CFDataCreateCopy(kCFAllocatorDefault, data);
 
 	write_item("password",
 		   (const char *)CFDataGetBytePtr(data),
@@ -173,7 +217,14 @@ static OSStatus find_internet_password(void)
 	CFRelease(item);
 
 	write_item("capability[]", "state", strlen("state"));
-	write_item("state[]", "osxkeychain:seen=1", strlen("osxkeychain:seen=1"));
+	{
+		struct strbuf sb;
+
+		strbuf_init(&sb, 1024);
+		encode_state_seen(&sb);
+		write_item("state[]", sb.buf, strlen(sb.buf));
+		strbuf_release(&sb);
+	}
 
 out:
 	CFRelease(attrs);
@@ -288,13 +339,22 @@ static OSStatus add_internet_password(void)
 	CFDictionaryRef attrs;
 	OSStatus result;
 
-	if (state_seen)
-		return errSecSuccess;
-
 	/* Only store complete credentials */
 	if (!protocol || !host || !username || !password)
 		return -1;
 
+	if (state_seen) {
+		struct strbuf sb;
+
+		strbuf_init(&sb, 1024);
+		encode_state_seen(&sb);
+		if (!strcmp(state_seen, sb.buf)) {
+			strbuf_release(&sb);
+			return errSecSuccess;
+		}
+		strbuf_release(&sb);
+	}
+
 	data = CFDataCreateMutableCopy(kCFAllocatorDefault, 0, password);
 	if (password_expiry_utc) {
 		CFDataAppendBytes(data,
@@ -403,8 +463,9 @@ static void read_credential(void)
 							   (UInt8 *)v,
 							   strlen(v));
 		else if (!strcmp(buf, "state[]")) {
-			if (!strcmp(v, "osxkeychain:seen=1"))
-				state_seen = 1;
+			int len = strlen("osxkeychain:seen=");
+			if (!strncmp(v, "osxkeychain:seen=", len))
+				state_seen = xstrdup(v);
 		}
 		/*
 		 * Ignore other lines; we don't know what they mean, but
@@ -443,5 +504,8 @@ int main(int argc, const char **argv)
 
 	clear_credential();
 
+	if (state_seen)
+		free(state_seen);
+
 	return 0;
 }
diff --git a/contrib/credential/osxkeychain/meson.build b/contrib/credential/osxkeychain/meson.build
index 3c7677f736..ec91d0c14b 100644
--- a/contrib/credential/osxkeychain/meson.build
+++ b/contrib/credential/osxkeychain/meson.build
@@ -1,6 +1,7 @@
 executable('git-credential-osxkeychain',
   sources: 'git-credential-osxkeychain.c',
   dependencies: [
+    libgit,
     dependency('CoreFoundation'),
     dependency('Security'),
   ],
author	Junio C Hamano <gitster@pobox.com>	2025-11-24 15:46:41 -0800
committer	Junio C Hamano <gitster@pobox.com>	2025-11-24 15:46:41 -0800
commit	861312b51d761b7a787e3a5d71ab2edf266ad297 (patch)
tree	344a6a365fdc83f1d9b30fe315a41ddac8b38981
parent	aa934e09502b222cee362cda42982cabfc550ebb (diff)
parent	4580bcd2354aab9369164d936f7ccaa21fc98c98 (diff)
download	git-861312b51d761b7a787e3a5d71ab2edf266ad297.tar.gz