From 418961267c4a5d8f4066bbb71c52ef757847f234 Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Sun, 14 Jun 2026 17:15:34 +0300 Subject: [PATCH] fix(security): enable in-process tpa-descriptions scanner without Docker (MCP-2396) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The built-in, Docker-less tpa-descriptions scanner has an empty EffectiveImage(), so the enable path (InstallScanner) fell through to the Docker pull flow and left it stuck in 'error'/'pulling'. resolveScanners then prefail-skipped every scan with an unactionable 'reconfigure it from the Security page' notice — the flagship Tool-Poisoning-Attack description scan never ran via the pipeline. - InstallScanner: short-circuit InProcess scanners to 'installed' synchronously, skipping the Docker image-availability/pull path entirely. - syncRegistryFromStorage: self-heal in-process scanners an older build persisted in a Docker state (error/pulling) back to 'installed' at startup, so existing broken installs recover without a manual re-toggle. Adds TestServiceInstallInProcessScanner (enable path) and TestServiceHealsInProcessScannerStuckInError (startup heal). Related MCP-2396 --- internal/security/scanner/service.go | 32 ++++++ internal/security/scanner/service_test.go | 127 ++++++++++++++++++++++ 2 files changed, 159 insertions(+) diff --git a/internal/security/scanner/service.go b/internal/security/scanner/service.go index 9bbf9673f..1c1abf7e6 100644 --- a/internal/security/scanner/service.go +++ b/internal/security/scanner/service.go @@ -192,6 +192,20 @@ func (s *Service) syncRegistryFromStorage() { return } for _, inst := range installed { + // Heal in-process scanners (e.g. tpa-descriptions) that an older build + // wrongly persisted in a Docker state (error/pulling/available): they + // have no image, are always runnable, and must be "installed" so the + // engine runs them instead of prefail-skipping every scan (MCP-2396). + if reg, err := s.registry.Get(inst.ID); err == nil && reg.InProcess && + inst.Status != ScannerStatusInstalled && inst.Status != ScannerStatusConfigured { + healed := targetStatusAfterPull(inst) + inst.Status = healed + inst.ErrorMsg = "" + _ = s.storage.SaveScanner(inst) + s.logger.Info("Healed in-process scanner stuck in Docker state", + zap.String("scanner", inst.ID), zap.String("status", healed)) + } + _ = s.registry.UpdateStatus(inst.ID, inst.Status) // Also update configured env so the engine can pass it to containers if inst.ConfiguredEnv != nil { @@ -298,6 +312,24 @@ func (s *Service) InstallScanner(ctx context.Context, id string) error { } } + // In-process scanners (e.g. tpa-descriptions) run in Go with no Docker + // image to pull, so there is nothing to install. Mark them enabled + // synchronously and skip the Docker image-availability path entirely — + // otherwise the empty EffectiveImage() falls through to the pull path and + // the scanner gets stuck in "error"/"pulling", prefail-skipping every scan + // (MCP-2396). + if scanner.InProcess { + scanner.Status = targetStatusAfterPull(scanner) + scanner.InstalledAt = time.Now() + scanner.ErrorMsg = "" + if err := s.storage.SaveScanner(scanner); err != nil { + return fmt.Errorf("failed to save scanner: %w", err) + } + _ = s.registry.UpdateStatus(id, scanner.Status) + s.emit().EmitSecurityScannerChanged(id, scanner.Status, "") + return nil + } + image := scanner.EffectiveImage() // Fast path: image already present → no need to pull at all. diff --git a/internal/security/scanner/service_test.go b/internal/security/scanner/service_test.go index 3d3afdb48..6d5466926 100644 --- a/internal/security/scanner/service_test.go +++ b/internal/security/scanner/service_test.go @@ -471,6 +471,133 @@ func TestServiceListScannersMerge(t *testing.T) { } } +// TestServiceInstallInProcessScanner verifies that enabling an in-process +// (Docker-less) scanner like tpa-descriptions lands it in the "installed" +// state synchronously without ever touching Docker. Before MCP-2396 the +// enable path always assumed a Docker image, so an empty-image in-process +// scanner fell through to the pull path and got stuck in "error" with an +// empty error message — prefail-skipping every scan with an unactionable +// "reconfigure it from the Security page" notice. +func TestServiceInstallInProcessScanner(t *testing.T) { + svc, store, emitter := newTestService(t) + + // Register a bundled in-process scanner mirroring tpa-descriptions: no + // Docker image, seeds as "installed". + svc.registry.scanners["tpa-descriptions"] = &ScannerPlugin{ + ID: "tpa-descriptions", + Name: "TPA Descriptions", + InProcess: true, + Inputs: []string{"mcp_connection"}, + Outputs: []string{"sarif"}, + Status: ScannerStatusInstalled, + } + + if err := svc.InstallScanner(context.Background(), "tpa-descriptions"); err != nil { + t.Fatalf("InstallScanner(tpa-descriptions) returned error: %v", err) + } + + got, err := svc.GetScanner(context.Background(), "tpa-descriptions") + if err != nil { + t.Fatalf("GetScanner failed: %v", err) + } + if got.Status != ScannerStatusInstalled { + t.Fatalf("expected status %q, got %q (err=%q)", ScannerStatusInstalled, got.Status, got.ErrorMsg) + } + if got.ErrorMsg != "" { + t.Errorf("expected empty ErrorMsg after enabling in-process scanner, got %q", got.ErrorMsg) + } + + // Must be persisted as installed so a restart keeps it enabled. + persisted, err := store.GetScanner("tpa-descriptions") + if err != nil { + t.Fatalf("expected in-process scanner persisted to storage: %v", err) + } + if persisted.Status != ScannerStatusInstalled { + t.Errorf("expected persisted status 'installed', got %q", persisted.Status) + } + + // A scanner_changed event announcing "installed" (not "error"/"pulling") + // must have been emitted so the UI reflects the enabled state. + var sawInstalled bool + for _, ev := range emitter.events { + if ev.eventType != "scanner_changed" { + continue + } + if ev.data["scanner_id"] == "tpa-descriptions" { + if ev.data["status"] != ScannerStatusInstalled { + t.Errorf("expected scanner_changed status 'installed', got %v (err=%v)", ev.data["status"], ev.data["error"]) + } + sawInstalled = true + } + } + if !sawInstalled { + t.Errorf("expected a scanner_changed event for tpa-descriptions, got %+v", emitter.events) + } +} + +// TestServiceHealsInProcessScannerStuckInError verifies that a service +// constructed with an in-process scanner whose persisted state is "error" +// (the bad state an older build left behind) self-heals to "installed" at +// startup, so the engine runs it instead of prefail-skipping it (MCP-2396). +func TestServiceHealsInProcessScannerStuckInError(t *testing.T) { + logger := zap.NewNop() + store := newMockStorage() + // Pre-seed storage with a stale error record, as an older buggy enable + // path would have left it. + _ = store.SaveScanner(&ScannerPlugin{ + ID: "tpa-descriptions", + Name: "TPA Descriptions", + InProcess: true, + Status: ScannerStatusError, + ErrorMsg: "", + }) + + registry := &Registry{ + scanners: map[string]*ScannerPlugin{ + "tpa-descriptions": { + ID: "tpa-descriptions", + Name: "TPA Descriptions", + InProcess: true, + Inputs: []string{"mcp_connection"}, + Outputs: []string{"sarif"}, + Status: ScannerStatusInstalled, + }, + }, + logger: logger, + } + + svc := NewService(store, registry, NewDockerRunner(logger), t.TempDir(), logger) + + // Registry should now report the in-process scanner as installed. + reg, err := registry.Get("tpa-descriptions") + if err != nil { + t.Fatalf("registry.Get failed: %v", err) + } + if reg.Status != ScannerStatusInstalled { + t.Errorf("expected registry status 'installed' after heal, got %q", reg.Status) + } + + // And the heal should be persisted so it survives subsequent restarts. + persisted, err := store.GetScanner("tpa-descriptions") + if err != nil { + t.Fatalf("GetScanner failed: %v", err) + } + if persisted.Status != ScannerStatusInstalled { + t.Errorf("expected persisted status 'installed' after heal, got %q", persisted.Status) + } + + // ListScanners (registry+storage merge) must surface it as installed. + list, err := svc.ListScanners(context.Background()) + if err != nil { + t.Fatalf("ListScanners failed: %v", err) + } + for _, sc := range list { + if sc.ID == "tpa-descriptions" && sc.Status != ScannerStatusInstalled { + t.Errorf("expected merged status 'installed', got %q", sc.Status) + } + } +} + func TestServiceConfigureScanner(t *testing.T) { svc, store, _ := newTestService(t)