diff --git a/internal/httpapi/registry_add_test.go b/internal/httpapi/registry_add_test.go index 2f872267c..df4291a5f 100644 --- a/internal/httpapi/registry_add_test.go +++ b/internal/httpapi/registry_add_test.go @@ -54,3 +54,21 @@ func TestAddFromRegistry_SlashServerIDUnescaped(t *testing.T) { assert.Equal(t, "microsoft/markitdown", controller.gotServerID, "serverId must be percent-decoded before registry lookup") assert.Equal(t, "github-mcp", controller.gotRegistryID, "registry id must be percent-decoded before lookup") } + +// TestAddFromRegistry_NilConfigIsAnError pins the nil-tolerance of the success +// branch: a controller that returns no config and no error yields a JSON 500, +// never a recovered nil-pointer panic (which corrupts the heap on +// windows/amd64 under Go 1.26 — golang/go#81238 — and took the whole test +// binary down with it). +func TestAddFromRegistry_NilConfigIsAnError(t *testing.T) { + ctrl := &adminConfigController{ServerController: &MockServerController{}, apiKey: "admin-secret"} + srv := NewServer(ctrl, zaptest.NewLogger(t).Sugar(), nil) + + req := httptest.NewRequest(http.MethodPost, "/api/v1/registries/reg1/servers/srv1/add", nil) + req.Header.Set("X-API-Key", "admin-secret") + w := httptest.NewRecorder() + srv.ServeHTTP(w, req) + + assert.Equal(t, http.StatusInternalServerError, w.Code) + assert.Contains(t, w.Body.String(), "no server configuration") +} diff --git a/internal/httpapi/server.go b/internal/httpapi/server.go index 749651e0d..6f059693c 100644 --- a/internal/httpapi/server.go +++ b/internal/httpapi/server.go @@ -5836,6 +5836,17 @@ func (s *Server) handleAddFromRegistry(w http.ResponseWriter, r *http.Request) { s.writeRegistryAddError(w, r, status, rerr) return } + if cfg == nil { + // A controller that reports success without a server config (the + // test doubles do) used to be dereferenced here; chi's recoverer + // turned that into a bare 500 — and on windows/amd64 the recovered + // hardware fault corrupts the Go heap under Go 1.26 (golang/go#81238), + // so the httpapi test binary then died in a later GC. Same + // nil-tolerance as redactedRegistrySummary. + logger.Errorw("Add from registry returned no server config", "registry", registryID, "server", serverID) + s.writeError(w, r, http.StatusInternalServerError, "registry returned no server configuration") + return + } // Issue #1148, round 8: the MCP twin of this handler // (`upstream_servers add_from_registry`) has sourced this echo from the diff --git a/internal/security/scanner/service_test.go b/internal/security/scanner/service_test.go index dc4dd3926..1f730e627 100644 --- a/internal/security/scanner/service_test.go +++ b/internal/security/scanner/service_test.go @@ -2505,11 +2505,36 @@ func TestServiceStartScanDeepOnRunsSourceResolutionAndPass2(t *testing.T) { t.Errorf("deep scan on: Pass 2 (ResolveFullSource) must run, got %d call(s)", got) } - // Drain the engine so the background Pass-2 goroutine finishes before the - // test tears down its temp dirs (keeps -race teardown quiet). + // Drain Pass 2 before the test tears down its temp dirs. Waiting for the + // engine to go idle is not enough: the Pass-2 goroutine writes tools.json + // into workDir (exportToolDefinitions) AFTER ResolveFullSource returns and + // BEFORE it registers its job with the engine, so an idle engine can mean + // "Pass 1 cleared, Pass 2 not yet started" — and TempDir's RemoveAll then + // raced the write ("directory not empty", flaky on ubuntu CI). The Pass-2 + // job is saved to storage on every exit path (completed, or the failed + // placeholder), so that is the terminal signal. + waitForPass2Saved(t, store, "srv-on") waitForScanIdle(t, svc, "srv-on") } +// waitForPass2Saved polls storage until a Pass-2 job for server has reached a +// terminal status — the goroutine's last write, on every path startPass2 can +// take. +func waitForPass2Saved(t *testing.T, store *mockStorage, server string) { + t.Helper() + deadline := time.Now().Add(10 * time.Second) + for time.Now().Before(deadline) { + jobs, _ := store.ListScanJobs(server) + for _, j := range jobs { + if j.ScanPass == ScanPassSupplyChainAudit && j.Status != ScanJobStatusRunning && j.Status != ScanJobStatusPending { + return + } + } + time.Sleep(20 * time.Millisecond) + } + t.Fatalf("Pass 2 for %s did not reach a terminal status in time", server) +} + // TestApplySecurityConfigDefaultConfigGatesDeepScanOff locks the audit's FIX-1 // invariant against the real default config: config.DefaultConfig() never // initializes Config.Security (it stays nil), and the server wiring passes