From 6f5b471fe0d1c3c62437d4c296f1e0e66a17f5a9 Mon Sep 17 00:00:00 2001
From: Nathan Wallace <nathan@turbot.com>
Date: Sat, 15 Nov 2025 19:11:10 -0500
Subject: [PATCH] Nil pool validation in newRefreshConnectionState closes #4778
 (#4825)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* Add test demonstrating bug #4778 - nil pool causes panic

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* Fix #4778: Add nil pool validation in newRefreshConnectionState

Add nil check immediately after retrieving pool from pluginManager
to prevent panic when pool is nil. This addresses the issue where
a nil pool would cause a segmentation fault when passed to
db_local.SetUserSearchPath().

The fix returns a descriptive error instead of panicking, allowing
calling code to handle the error gracefully.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>
---
 pkg/connection/refresh_connections_state.go   |  3 +
 .../refresh_connections_state_test.go         | 64 +++++++++++++++++++
 2 files changed, 67 insertions(+)

diff --git a/pkg/connection/refresh_connections_state.go b/pkg/connection/refresh_connections_state.go
index 0de9e4fea..2779d89bd 100644
--- a/pkg/connection/refresh_connections_state.go
+++ b/pkg/connection/refresh_connections_state.go
@@ -63,6 +63,9 @@ func newRefreshConnectionState(ctx context.Context, pluginManager pluginManager,
 	defer log.Println("[DEBUG] newRefreshConnectionState end")
 
 	pool := pluginManager.Pool()
+	if pool == nil {
+		return nil, sperr.New("plugin manager returned nil pool")
+	}
 	// set user search path first
 	log.Printf("[INFO] setting up search path")
 	searchPath, err := db_local.SetUserSearchPath(ctx, pool)
diff --git a/pkg/connection/refresh_connections_state_test.go b/pkg/connection/refresh_connections_state_test.go
index 00ad8efcb..977a64ec7 100644
--- a/pkg/connection/refresh_connections_state_test.go
+++ b/pkg/connection/refresh_connections_state_test.go
@@ -8,6 +8,11 @@ import (
 	"testing"
 	"time"
 
+	"github.com/jackc/pgx/v5/pgxpool"
+	"github.com/turbot/pipe-fittings/v2/error_helpers"
+	"github.com/turbot/pipe-fittings/v2/plugin"
+	"github.com/turbot/steampipe-plugin-sdk/v5/grpc/proto"
+	"github.com/turbot/steampipe/v2/pkg/pluginmanager_service/grpc/shared"
 	"github.com/turbot/steampipe/v2/pkg/steampipeconfig"
 )
 
@@ -479,3 +484,62 @@ func TestConnectionError(t *testing.T) {
 		t.Error("Error not preserved")
 	}
 }
+
+// mockPluginManager is a mock implementation of pluginManager interface for testing
+type mockPluginManager struct {
+	shared.PluginManager
+	pool *pgxpool.Pool
+}
+
+func (m *mockPluginManager) Pool() *pgxpool.Pool {
+	return m.pool
+}
+
+// Implement other required methods from pluginManager interface
+func (m *mockPluginManager) OnConnectionConfigChanged(context.Context, ConnectionConfigMap, map[string]*plugin.Plugin) {
+}
+
+func (m *mockPluginManager) GetConnectionConfig() ConnectionConfigMap {
+	return nil
+}
+
+func (m *mockPluginManager) HandlePluginLimiterChanges(PluginLimiterMap) error {
+	return nil
+}
+
+func (m *mockPluginManager) ShouldFetchRateLimiterDefs() bool {
+	return false
+}
+
+func (m *mockPluginManager) LoadPluginRateLimiters(map[string]string) (PluginLimiterMap, error) {
+	return nil, nil
+}
+
+func (m *mockPluginManager) SendPostgresSchemaNotification(context.Context) error {
+	return nil
+}
+
+func (m *mockPluginManager) SendPostgresErrorsAndWarningsNotification(context.Context, error_helpers.ErrorAndWarnings) {
+}
+
+func (m *mockPluginManager) UpdatePluginColumnsTable(context.Context, map[string]*proto.Schema, []string) error {
+	return nil
+}
+
+// TestNewRefreshConnectionState_NilPool tests that newRefreshConnectionState handles nil pool gracefully
+// This test demonstrates issue #4778 - nil pool from pluginManager causes panic
+func TestNewRefreshConnectionState_NilPool(t *testing.T) {
+	ctx := context.Background()
+
+	// Create a mock plugin manager that returns nil pool
+	mockPM := &mockPluginManager{
+		pool: nil,
+	}
+
+	// This should not panic - should return an error instead
+	_, err := newRefreshConnectionState(ctx, mockPM, []string{})
+
+	if err == nil {
+		t.Error("Expected error when pool is nil, got nil")
+	}
+}