fix: reject ambiguous inline key-auth config that bypasses duplicate check

internal/webhook/v1/consumer_webhook.go

@@ -16,6 +16,7 @@
 package v1
 
 import (
+	"bytes"
 	"context"
 	"encoding/json"
 	"fmt"
@@ -227,15 +228,109 @@ func (v *ConsumerCustomValidator) extractCredentialKey(ctx context.Context, cons
 		return "", nil
 	}
 
-	var cfg struct {
-		Key string `json:"key"`
+	key, err := parseInlineKeyAuthKey(credential.Config.Raw)
+	if err != nil {
+		return "", fmt.Errorf("invalid key-auth credential config for Consumer %s/%s: %w",
+			consumer.Namespace, consumer.Name, err)
+	}
+	return key, nil
+}
+
+// parseInlineKeyAuthKey extracts the key-auth "key" from an inline credential
+// config the same way downstream cjson does: exact-case, string-valued,
+// last-wins. Ambiguous configs that Go's struct decoder would silently reject
+// while cjson still resolves to a live key (duplicate "key" members, or a
+// non-string "key") are returned as errors so they can't bypass the duplicate
+// check. Genuinely malformed JSON that cjson also rejects returns ("", nil) so
+// existing consumers with broken config are skipped, not suddenly denied.
+func parseInlineKeyAuthKey(raw []byte) (string, error) {
+	// cjson rejects malformed input (truncation, trailing data, multiple
+	// top-level values); mirror that by skipping anything that is not exactly
+	// one well-formed JSON value. Duplicate keys stay valid here and are caught
+	// by the token walk below.
+	if !json.Valid(raw) {
+		return "", nil
+	}
+
+	dec := json.NewDecoder(bytes.NewReader(raw))
+
+	// Top level must be an object, else there is no usable key.
+	tok, err := dec.Token()
+	if err != nil {
+		return "", nil
 	}
-	if err := json.Unmarshal(credential.Config.Raw, &cfg); err != nil {
-		// Malformed JSON is not a hard error: skip duplicate detection for this
-		// credential so existing consumers with bad config are not suddenly denied.
-		consumerLog.V(1).Info("skipping duplicate key-auth check: malformed credential config",
-			"consumer", consumer.Name, "error", err)
+	if delim, ok := tok.(json.Delim); !ok || delim != '{' {
 		return "", nil
 	}
-	return cfg.Key, nil
+
+	var (
+		key      string
+		keyCount int
+	)
+	for dec.More() {
+		nameTok, err := dec.Token()
+		if err != nil {
+			return "", nil
+		}
+		name, ok := nameTok.(string)
+		if !ok {
+			return "", nil
+		}
+		if name != "key" {
+			if err := skipJSONValue(dec); err != nil {
+				return "", nil
+			}
+			continue
+		}
+
+		keyCount++
+		valTok, err := dec.Token()
+		if err != nil {
+			return "", nil
+		}
+		switch val := valTok.(type) {
+		case string:
+			key = val
+		case nil:
+			// null key: no usable value, but still counts for dup detection.
+		default:
+			// number/bool/object/array: cjson would deliver a value here while
+			// Go's struct decoder errors and skips. Reject instead.
+			return "", fmt.Errorf("key-auth credential \"key\" must be a string")
+		}
+	}
+
+	if keyCount > 1 {
+		return "", fmt.Errorf("key-auth credential config has duplicate \"key\" members")
+	}
+	return key, nil
+}
+
+// skipJSONValue consumes a single JSON value (scalar or a whole object/array)
+// from the decoder so the token stream stays aligned.
+func skipJSONValue(dec *json.Decoder) error {
+	tok, err := dec.Token()
+	if err != nil {
+		return err
+	}
+	delim, ok := tok.(json.Delim)
+	if !ok || (delim != '{' && delim != '[') {
+		return nil
+	}
+	depth := 1
+	for depth > 0 {
+		t, err := dec.Token()
+		if err != nil {
+			return err
+		}
+		if d, ok := t.(json.Delim); ok {
+			switch d {
+			case '{', '[':
+				depth++
+			case '}', ']':
+				depth--
+			}
+		}
+	}
+	return nil
 }

internal/webhook/v1/consumer_webhook_test.go

@@ -192,3 +192,70 @@ func TestConsumerValidator_DenyDuplicateKeyAuthCredential(t *testing.T) {
 	require.Contains(t, err.Error(), `duplicate key-auth credential key "shared-key"`)
 	require.Contains(t, err.Error(), "default/existing")
 }
+
+// A duplicate-key inline config ({"key":123,"key":"K"}) is unreadable to Go's
+// struct decoder but resolves to "K" downstream via cjson. The webhook must
+// reject it instead of silently skipping the duplicate check.
+func TestConsumerValidator_DenyDuplicateKeyAuthCredential_ParserDivergence(t *testing.T) {
+	existing := &apisixv1alpha1.Consumer{
+		ObjectMeta: metav1.ObjectMeta{Name: "existing", Namespace: "default"},
+		Spec: apisixv1alpha1.ConsumerSpec{
+			GatewayRef: apisixv1alpha1.GatewayRef{Name: "test-gateway"},
+			Credentials: []apisixv1alpha1.Credential{{
+				Type:   "key-auth",
+				Config: apiextensionsv1.JSON{Raw: []byte(`{"key":"victims-key"}`)},
+			}},
+		},
+	}
+	consumer := &apisixv1alpha1.Consumer{
+		ObjectMeta: metav1.ObjectMeta{Name: "demo", Namespace: "default"},
+		Spec: apisixv1alpha1.ConsumerSpec{
+			GatewayRef: apisixv1alpha1.GatewayRef{Name: "test-gateway"},
+			Credentials: []apisixv1alpha1.Credential{{
+				Type:   "key-auth",
+				Config: apiextensionsv1.JSON{Raw: []byte(`{"key":123,"key":"victims-key"}`)},
+			}},
+		},
+	}
+
+	validator := buildConsumerValidator(t, existing)
+
+	_, err := validator.ValidateCreate(context.Background(), consumer)
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "invalid key-auth credential config")
+}
+
+func TestParseInlineKeyAuthKey(t *testing.T) {
+	tests := []struct {
+		name    string
+		raw     string
+		wantKey string
+		wantErr bool
+	}{
+		{name: "plain string key", raw: `{"key":"K"}`, wantKey: "K"},
+		{name: "extra fields ignored", raw: `{"key":"K","foo":{"a":1}}`, wantKey: "K"},
+		{name: "duplicate key members", raw: `{"key":"a","key":"b"}`, wantErr: true},
+		{name: "number then string (divergence PoC)", raw: `{"key":123,"key":"K"}`, wantErr: true},
+		{name: "non-string key", raw: `{"key":123}`, wantErr: true},
+		{name: "object key", raw: `{"key":{"nested":1}}`, wantErr: true},
+		{name: "null key skipped", raw: `{"key":null}`, wantKey: ""},
+		{name: "no key member", raw: `{"foo":"bar"}`, wantKey: ""},
+		{name: "exact-case only, Key ignored", raw: `{"Key":"K"}`, wantKey: ""},
+		{name: "malformed json skipped", raw: `{"key":`, wantKey: ""},
+		{name: "truncated object skipped", raw: `{"key":"K"`, wantKey: ""},
+		{name: "multiple top-level values skipped", raw: `{"key":"K"}{"key":"X"}`, wantKey: ""},
+		{name: "trailing garbage skipped", raw: `{"key":"K"} junk`, wantKey: ""},
+		{name: "non-object skipped", raw: `["key","K"]`, wantKey: ""},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			key, err := parseInlineKeyAuthKey([]byte(tt.raw))
+			if tt.wantErr {
+				require.Error(t, err)
+				return
+			}
+			require.NoError(t, err)
+			require.Equal(t, tt.wantKey, key)
+		})
+	}
+}