From d58888e2d2bdb7ff51e373f0cb4aa2cd51ec1db6 Mon Sep 17 00:00:00 2001 From: Nathan Wallace Date: Sun, 16 Nov 2025 00:32:47 +0800 Subject: [PATCH] Nil pointer dereference in OnConnectionConfigChanged closes #4784 (#4829) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add test demonstrating bug #4784 - OnConnectionConfigChanged panics with nil pool 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude * Fix #4784: Add nil pool check in handlePluginInstanceChanges 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --------- Co-authored-by: Claude --- .../plugin_manager_plugin_instance.go | 6 ++++ .../plugin_manager_test.go | 33 +++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/pkg/pluginmanager_service/plugin_manager_plugin_instance.go b/pkg/pluginmanager_service/plugin_manager_plugin_instance.go index 4df1972a4..0ca519fbd 100644 --- a/pkg/pluginmanager_service/plugin_manager_plugin_instance.go +++ b/pkg/pluginmanager_service/plugin_manager_plugin_instance.go @@ -27,6 +27,12 @@ func (m *PluginManager) handlePluginInstanceChanges(ctx context.Context, newPlug // update connectionConfigMap m.plugins = newPlugins + // if pool is nil, we're in a test environment or the plugin manager hasn't been fully initialized + // in this case, we can't repopulate the plugin table, so just return early + if m.pool == nil { + return nil + } + // repopulate the plugin table conn, err := m.pool.Acquire(ctx) if err != nil { diff --git a/pkg/pluginmanager_service/plugin_manager_test.go b/pkg/pluginmanager_service/plugin_manager_test.go index d1575a025..5e1876415 100644 --- a/pkg/pluginmanager_service/plugin_manager_test.go +++ b/pkg/pluginmanager_service/plugin_manager_test.go @@ -1,6 +1,7 @@ package pluginmanager_service import ( + "context" "fmt" "runtime" "sync" @@ -714,3 +715,35 @@ func TestPluginManager_StressConcurrentMapAccess(t *testing.T) { close(stopCh) wg.Wait() } + +// Test 24: OnConnectionConfigChanged with Nil Pool (Bug #4784) + +// TestPluginManager_OnConnectionConfigChanged_EmptyToNonEmpty tests the scenario where +// a PluginManager with no pool (e.g., in a testing environment) receives a configuration change. +// This test demonstrates bug #4784 - a nil pointer panic when m.pool is nil. +func TestPluginManager_OnConnectionConfigChanged_EmptyToNonEmpty(t *testing.T) { + // Create a minimal PluginManager without pool initialization + // This simulates a testing scenario or edge case where the pool might not be initialized + m := &PluginManager{ + plugins: make(map[string]*plugin.Plugin), + // Note: pool is intentionally nil to demonstrate the bug + } + + // Create a new plugin map with one plugin + newPlugins := map[string]*plugin.Plugin{ + "aws": { + Plugin: "hub.steampipe.io/plugins/turbot/aws@latest", + Instance: "aws", + }, + } + + ctx := context.Background() + + // This should panic with nil pointer dereference when trying to use m.pool + err := m.handlePluginInstanceChanges(ctx, newPlugins) + + // If we get here without panic, the fix is working + if err != nil { + t.Logf("Expected error when pool is nil: %v", err) + } +}