From a5bef10740aa72765c98d1df7fc8c833381befbf Mon Sep 17 00:00:00 2001
From: Peter Steinberger <steipete@gmail.com>
Date: Mon, 16 Feb 2026 05:20:35 +0100
Subject: [PATCH] fix(contacts): hard-validate custom and sort output

---
 internal/cmd/contacts_crud.go                 |  1 -
 internal/cmd/contacts_helpers_test.go         | 38 ++++++++++++++
 .../cmd/execute_more_text_coverage_test.go    | 52 ++++++++++++++++++-
 3 files changed, 89 insertions(+), 2 deletions(-)

diff --git a/internal/cmd/contacts_crud.go b/internal/cmd/contacts_crud.go
index 2c19d34..3ed7ca5 100644
--- a/internal/cmd/contacts_crud.go
+++ b/internal/cmd/contacts_crud.go
@@ -426,7 +426,6 @@ func (c *ContactsUpdateCmd) Run(ctx context.Context, kctx *kong.Context, flags *
 		userDefined, parseErr := parseCustomUserDefined(c.Custom, true)
 		if parseErr != nil {
 			return usage(parseErr.Error())
-		}
 		existing.UserDefined = userDefined
 		updateFields = append(updateFields, "userDefined")
 	}
diff --git a/internal/cmd/contacts_helpers_test.go b/internal/cmd/contacts_helpers_test.go
index 8471e0c..4fae881 100644
--- a/internal/cmd/contacts_helpers_test.go
+++ b/internal/cmd/contacts_helpers_test.go
@@ -95,3 +95,41 @@ func TestPrimaryBirthday_EdgeCases(t *testing.T) {
 		t.Fatalf("unexpected: %q", got)
 	}
 }
+
+func TestParseCustomUserDefined_InvalidInput(t *testing.T) {
+	if _, _, err := parseCustomUserDefined([]string{"bad"}, true); err == nil {
+		t.Fatalf("expected error for missing '='")
+	}
+	if _, _, err := parseCustomUserDefined([]string{"=value"}, true); err == nil {
+		t.Fatalf("expected error for empty key")
+	}
+	if _, _, err := parseCustomUserDefined([]string{""}, false); err == nil {
+		t.Fatalf("expected error for empty custom value")
+	}
+}
+
+func TestParseCustomUserDefined_ValidInput(t *testing.T) {
+	fields, clear, err := parseCustomUserDefined([]string{"team=devops", " repo = gog"}, false)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if clear {
+		t.Fatalf("did not expect clear")
+	}
+	if len(fields) != 2 || fields[0].Key != "team" || fields[0].Value != "devops" || fields[1].Key != "repo" || fields[1].Value != "gog" {
+		t.Fatalf("unexpected fields: %#v", fields)
+	}
+}
+
+func TestParseCustomUserDefined_ClearAll(t *testing.T) {
+	fields, clear, err := parseCustomUserDefined([]string{""}, true)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if !clear {
+		t.Fatalf("expected clear")
+	}
+	if len(fields) != 0 {
+		t.Fatalf("expected empty fields, got %v", len(fields))
+	}
+}
diff --git a/internal/cmd/execute_more_text_coverage_test.go b/internal/cmd/execute_more_text_coverage_test.go
index 8bb91e0..059e7be 100644
--- a/internal/cmd/execute_more_text_coverage_test.go
+++ b/internal/cmd/execute_more_text_coverage_test.go
@@ -68,7 +68,7 @@ func TestExecute_ContactsGet_ByResource_Text(t *testing.T) {
 	t.Cleanup(func() { newPeopleContactsService = origNew })
 
 	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		if !(strings.Contains(r.URL.Path, "/people/c1") && r.Method == http.MethodGet) {
+		if !(strings.Contains(r.URL.Path, "/people/c1") && r.Method == http.MethodGet && !strings.Contains(r.URL.Path, ":")) {
 			http.NotFound(w, r)
 			return
 		}
@@ -106,6 +106,56 @@ func TestExecute_ContactsGet_ByResource_Text(t *testing.T) {
 	}
 }
 
+func TestExecute_ContactsGet_CustomFieldsSorted_Text(t *testing.T) {
+	origNew := newPeopleContactsService
+	t.Cleanup(func() { newPeopleContactsService = origNew })
+
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if !(strings.Contains(r.URL.Path, "/people/c1") && r.Method == http.MethodGet) {
+			http.NotFound(w, r)
+			return
+		}
+		w.Header().Set("Content-Type", "application/json")
+		_ = json.NewEncoder(w).Encode(map[string]any{
+			"resourceName": "people/c1",
+			"userDefined": []map[string]any{
+				{"key": "zzz", "value": "3"},
+				{"key": "aaa", "value": "1"},
+				{"key": "mmm", "value": "2"},
+			},
+		})
+	}))
+	defer srv.Close()
+
+	svc, err := people.NewService(context.Background(),
+		option.WithoutAuthentication(),
+		option.WithHTTPClient(srv.Client()),
+		option.WithEndpoint(srv.URL+"/"),
+	)
+	if err != nil {
+		t.Fatalf("NewService: %v", err)
+	}
+	newPeopleContactsService = func(context.Context, string) (*people.Service, error) { return svc, nil }
+
+	out := captureStdout(t, func() {
+		_ = captureStderr(t, func() {
+			if err := Execute([]string{"--account", "a@b.com", "contacts", "get", "people/c1"}); err != nil {
+				t.Fatalf("Execute: %v", err)
+			}
+		})
+	})
+
+	a := strings.Index(out, "custom:aaa\t1")
+	b := strings.Index(out, "custom:mmm\t2")
+	c := strings.Index(out, "custom:zzz\t3")
+	if a < 0 || b < 0 || c < 0 {
+		t.Fatalf("missing custom fields: %q", out)
+	}
+	if !(a < b && b < c) {
+		t.Fatalf("custom fields not sorted: %q", out)
+	}
+}
+
 func TestExecute_CalendarFreeBusy_Text(t *testing.T) {
 	origNew := newCalendarService
 	t.Cleanup(func() { newCalendarService = origNew })