Skip to content

Commit 5889ac8

Browse files
authored
fix(security): enable in-process tpa-descriptions scanner without Docker (MCP-2396) (#662)
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
1 parent 61d78da commit 5889ac8

2 files changed

Lines changed: 159 additions & 0 deletions

File tree

internal/security/scanner/service.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,20 @@ func (s *Service) syncRegistryFromStorage() {
207207
return
208208
}
209209
for _, inst := range installed {
210+
// Heal in-process scanners (e.g. tpa-descriptions) that an older build
211+
// wrongly persisted in a Docker state (error/pulling/available): they
212+
// have no image, are always runnable, and must be "installed" so the
213+
// engine runs them instead of prefail-skipping every scan (MCP-2396).
214+
if reg, err := s.registry.Get(inst.ID); err == nil && reg.InProcess &&
215+
inst.Status != ScannerStatusInstalled && inst.Status != ScannerStatusConfigured {
216+
healed := targetStatusAfterPull(inst)
217+
inst.Status = healed
218+
inst.ErrorMsg = ""
219+
_ = s.storage.SaveScanner(inst)
220+
s.logger.Info("Healed in-process scanner stuck in Docker state",
221+
zap.String("scanner", inst.ID), zap.String("status", healed))
222+
}
223+
210224
_ = s.registry.UpdateStatus(inst.ID, inst.Status)
211225
// Also update configured env so the engine can pass it to containers
212226
if inst.ConfiguredEnv != nil {
@@ -313,6 +327,24 @@ func (s *Service) InstallScanner(ctx context.Context, id string) error {
313327
}
314328
}
315329

330+
// In-process scanners (e.g. tpa-descriptions) run in Go with no Docker
331+
// image to pull, so there is nothing to install. Mark them enabled
332+
// synchronously and skip the Docker image-availability path entirely —
333+
// otherwise the empty EffectiveImage() falls through to the pull path and
334+
// the scanner gets stuck in "error"/"pulling", prefail-skipping every scan
335+
// (MCP-2396).
336+
if scanner.InProcess {
337+
scanner.Status = targetStatusAfterPull(scanner)
338+
scanner.InstalledAt = time.Now()
339+
scanner.ErrorMsg = ""
340+
if err := s.storage.SaveScanner(scanner); err != nil {
341+
return fmt.Errorf("failed to save scanner: %w", err)
342+
}
343+
_ = s.registry.UpdateStatus(id, scanner.Status)
344+
s.emit().EmitSecurityScannerChanged(id, scanner.Status, "")
345+
return nil
346+
}
347+
316348
image := scanner.EffectiveImage()
317349

318350
// Fast path: image already present → no need to pull at all.

internal/security/scanner/service_test.go

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -471,6 +471,133 @@ func TestServiceListScannersMerge(t *testing.T) {
471471
}
472472
}
473473

474+
// TestServiceInstallInProcessScanner verifies that enabling an in-process
475+
// (Docker-less) scanner like tpa-descriptions lands it in the "installed"
476+
// state synchronously without ever touching Docker. Before MCP-2396 the
477+
// enable path always assumed a Docker image, so an empty-image in-process
478+
// scanner fell through to the pull path and got stuck in "error" with an
479+
// empty error message — prefail-skipping every scan with an unactionable
480+
// "reconfigure it from the Security page" notice.
481+
func TestServiceInstallInProcessScanner(t *testing.T) {
482+
svc, store, emitter := newTestService(t)
483+
484+
// Register a bundled in-process scanner mirroring tpa-descriptions: no
485+
// Docker image, seeds as "installed".
486+
svc.registry.scanners["tpa-descriptions"] = &ScannerPlugin{
487+
ID: "tpa-descriptions",
488+
Name: "TPA Descriptions",
489+
InProcess: true,
490+
Inputs: []string{"mcp_connection"},
491+
Outputs: []string{"sarif"},
492+
Status: ScannerStatusInstalled,
493+
}
494+
495+
if err := svc.InstallScanner(context.Background(), "tpa-descriptions"); err != nil {
496+
t.Fatalf("InstallScanner(tpa-descriptions) returned error: %v", err)
497+
}
498+
499+
got, err := svc.GetScanner(context.Background(), "tpa-descriptions")
500+
if err != nil {
501+
t.Fatalf("GetScanner failed: %v", err)
502+
}
503+
if got.Status != ScannerStatusInstalled {
504+
t.Fatalf("expected status %q, got %q (err=%q)", ScannerStatusInstalled, got.Status, got.ErrorMsg)
505+
}
506+
if got.ErrorMsg != "" {
507+
t.Errorf("expected empty ErrorMsg after enabling in-process scanner, got %q", got.ErrorMsg)
508+
}
509+
510+
// Must be persisted as installed so a restart keeps it enabled.
511+
persisted, err := store.GetScanner("tpa-descriptions")
512+
if err != nil {
513+
t.Fatalf("expected in-process scanner persisted to storage: %v", err)
514+
}
515+
if persisted.Status != ScannerStatusInstalled {
516+
t.Errorf("expected persisted status 'installed', got %q", persisted.Status)
517+
}
518+
519+
// A scanner_changed event announcing "installed" (not "error"/"pulling")
520+
// must have been emitted so the UI reflects the enabled state.
521+
var sawInstalled bool
522+
for _, ev := range emitter.events {
523+
if ev.eventType != "scanner_changed" {
524+
continue
525+
}
526+
if ev.data["scanner_id"] == "tpa-descriptions" {
527+
if ev.data["status"] != ScannerStatusInstalled {
528+
t.Errorf("expected scanner_changed status 'installed', got %v (err=%v)", ev.data["status"], ev.data["error"])
529+
}
530+
sawInstalled = true
531+
}
532+
}
533+
if !sawInstalled {
534+
t.Errorf("expected a scanner_changed event for tpa-descriptions, got %+v", emitter.events)
535+
}
536+
}
537+
538+
// TestServiceHealsInProcessScannerStuckInError verifies that a service
539+
// constructed with an in-process scanner whose persisted state is "error"
540+
// (the bad state an older build left behind) self-heals to "installed" at
541+
// startup, so the engine runs it instead of prefail-skipping it (MCP-2396).
542+
func TestServiceHealsInProcessScannerStuckInError(t *testing.T) {
543+
logger := zap.NewNop()
544+
store := newMockStorage()
545+
// Pre-seed storage with a stale error record, as an older buggy enable
546+
// path would have left it.
547+
_ = store.SaveScanner(&ScannerPlugin{
548+
ID: "tpa-descriptions",
549+
Name: "TPA Descriptions",
550+
InProcess: true,
551+
Status: ScannerStatusError,
552+
ErrorMsg: "",
553+
})
554+
555+
registry := &Registry{
556+
scanners: map[string]*ScannerPlugin{
557+
"tpa-descriptions": {
558+
ID: "tpa-descriptions",
559+
Name: "TPA Descriptions",
560+
InProcess: true,
561+
Inputs: []string{"mcp_connection"},
562+
Outputs: []string{"sarif"},
563+
Status: ScannerStatusInstalled,
564+
},
565+
},
566+
logger: logger,
567+
}
568+
569+
svc := NewService(store, registry, NewDockerRunner(logger), t.TempDir(), logger)
570+
571+
// Registry should now report the in-process scanner as installed.
572+
reg, err := registry.Get("tpa-descriptions")
573+
if err != nil {
574+
t.Fatalf("registry.Get failed: %v", err)
575+
}
576+
if reg.Status != ScannerStatusInstalled {
577+
t.Errorf("expected registry status 'installed' after heal, got %q", reg.Status)
578+
}
579+
580+
// And the heal should be persisted so it survives subsequent restarts.
581+
persisted, err := store.GetScanner("tpa-descriptions")
582+
if err != nil {
583+
t.Fatalf("GetScanner failed: %v", err)
584+
}
585+
if persisted.Status != ScannerStatusInstalled {
586+
t.Errorf("expected persisted status 'installed' after heal, got %q", persisted.Status)
587+
}
588+
589+
// ListScanners (registry+storage merge) must surface it as installed.
590+
list, err := svc.ListScanners(context.Background())
591+
if err != nil {
592+
t.Fatalf("ListScanners failed: %v", err)
593+
}
594+
for _, sc := range list {
595+
if sc.ID == "tpa-descriptions" && sc.Status != ScannerStatusInstalled {
596+
t.Errorf("expected merged status 'installed', got %q", sc.Status)
597+
}
598+
}
599+
}
600+
474601
func TestServiceConfigureScanner(t *testing.T) {
475602
svc, store, _ := newTestService(t)
476603

0 commit comments

Comments
 (0)