From 238a97af1c786ce68402db1f6685bb4eded9a3f1 Mon Sep 17 00:00:00 2001
From: Anas Khan <83116240+anxkhn@users.noreply.github.com>
Date: Mon, 6 Jul 2026 17:43:57 +0530
Subject: [PATCH] fix: guard interplex cache Store and Load against nil grpc
 conn (#1682)

grpc.NewClient returns a nil *ClientConn when it fails (for example on an
invalid connection string). Store and Load registered defer conn.Close()
before checking the returned error, so a client-creation failure triggered a
nil pointer dereference in the deferred Close instead of returning the error.

Move defer conn.Close() to after the error check in both methods. Add a
regression test that drives Store and Load with an invalid connection string
and asserts they return an error without panicking.

The deferred conn.Close() now discards its error with a plain _ = conn.Close()
rather than logging it: grpc's ClientConn.Close() only returns a non-nil error
on a double-close, which can't happen from a single call site here, so a
logging branch would be dead code that patch coverage can never satisfy.

Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
---
 pkg/cache/interplex_based.go      |  6 ++++--
 pkg/cache/interplex_based_test.go | 30 ++++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/pkg/cache/interplex_based.go b/pkg/cache/interplex_based.go
index 052c8d85..0251874d 100644
--- a/pkg/cache/interplex_based.go
+++ b/pkg/cache/interplex_based.go
@@ -44,10 +44,11 @@ func (c *InterplexCache) Store(key string, data string) error {
 	}
 
 	conn, err := grpc.NewClient(c.configuration.ConnectionString, grpc.WithInsecure(), grpc.WithBlock())
-	defer conn.Close()
 	if err != nil {
 		return err
 	}
+	// Best-effort cleanup: nothing meaningful to do with a close error here.
+	defer func() { _ = conn.Close() }()
 	serviceClient := rpc.NewCacheServiceClient(conn)
 	c.cacheServiceClient = serviceClient
 	req := schemav1.SetRequest{
@@ -67,10 +68,11 @@ func (c *InterplexCache) Load(key string) (string, error) {
 	}
 
 	conn, err := grpc.NewClient(c.configuration.ConnectionString, grpc.WithInsecure(), grpc.WithBlock())
-	defer conn.Close()
 	if err != nil {
 		return "", err
 	}
+	// Best-effort cleanup: nothing meaningful to do with a close error here.
+	defer func() { _ = conn.Close() }()
 	serviceClient := rpc.NewCacheServiceClient(conn)
 	c.cacheServiceClient = serviceClient
 	req := schemav1.GetRequest{
diff --git a/pkg/cache/interplex_based_test.go b/pkg/cache/interplex_based_test.go
index b653899e..e1b4c676 100644
--- a/pkg/cache/interplex_based_test.go
+++ b/pkg/cache/interplex_based_test.go
@@ -68,6 +68,36 @@ func TestInterplexCache(t *testing.T) {
 	})
 }
 
+// TestInterplexCacheClientError ensures Store and Load return the connection
+// error instead of panicking when grpc.NewClient fails. On error grpc.NewClient
+// returns a nil *ClientConn, so a deferred conn.Close() placed before the error
+// check dereferences nil.
+func TestInterplexCacheClientError(t *testing.T) {
+	// Store/Load overwrite the connection string with localhost:8084 when
+	// INTERPLEX_LOCAL_MODE is set, which would bypass grpc.NewClient. Pin it empty
+	// so the invalid connection string is exercised regardless of the environment.
+	t.Setenv("INTERPLEX_LOCAL_MODE", "")
+
+	// An invalid URL escape makes grpc.NewClient fail and return a nil conn.
+	cache := &InterplexCache{
+		configuration: InterplexCacheConfiguration{
+			ConnectionString: "dns://%zz",
+		},
+	}
+
+	t.Run("Store", func(t *testing.T) {
+		if err := cache.Store("key1", "value1"); err == nil {
+			t.Error("expected an error for an invalid connection string, got nil")
+		}
+	})
+
+	t.Run("Load", func(t *testing.T) {
+		if _, err := cache.Load("key1"); err == nil {
+			t.Error("expected an error for an invalid connection string, got nil")
+		}
+	})
+}
+
 type mockCacheService struct {
 	rpc.UnimplementedCacheServiceServer
 	data map[string]string