fix(039): wire SecurityService into HTTP server, E2E fixes

Critical fixes discovered during E2E testing:
- Wire scanner.Service as SecurityController in server setup
- Register security routes unconditionally (nil-guard in handlers)
- Use background context for scan goroutines (prevent request context cancellation)
- Sync registry from storage on startup (scanner state survives restart)
- Add source_dir parameter to scan API and service
- Fix Semgrep: mount source at /src (Docker image requirement), enable network
- Disable read-only for scanner containers (they need cache writes)
- Fix Trivy image name: aquasecurity/trivy (not aquasec/trivy)
- Add GetSecurityOverview alias on Service for interface compliance

Verified E2E:
- Semgrep scan found subprocess shell=True (HIGH) and eval() (MEDIUM)
- Full approve/reject/report CLI workflow tested against running server

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: claude

internal/httpapi/security_scanner.go

@@ -18,7 +18,7 @@ type SecurityController interface {
 	ConfigureScanner(ctx context.Context, id string, env map[string]string) error
 	GetScannerStatus(ctx context.Context, id string) (*scanner.ScannerPlugin, error)
 
-	StartScan(ctx context.Context, serverName string, dryRun bool, scannerIDs []string) (*scanner.ScanJob, error)
+	StartScan(ctx context.Context, serverName string, dryRun bool, scannerIDs []string, sourceDir string) (*scanner.ScanJob, error)
 	GetScanStatus(ctx context.Context, serverName string) (*scanner.ScanJob, error)
 	GetScanReport(ctx context.Context, serverName string) (*scanner.AggregatedReport, error)
 	CancelScan(ctx context.Context, serverName string) error
@@ -37,9 +37,21 @@ func (s *Server) SetSecurityController(ctrl SecurityController) {
 	s.securityController = ctrl
 }
 
+// requireSecurity returns true if the security controller is available, writing a 501 error if not.
+func (s *Server) requireSecurity(w http.ResponseWriter, r *http.Request) bool {
+	if s.securityController == nil {
+		s.writeError(w, r, http.StatusNotImplemented, "security scanner feature is not configured")
+		return false
+	}
+	return true
+}
+
 // --- Scanner management handlers ---
 
 func (s *Server) handleListScanners(w http.ResponseWriter, r *http.Request) {
+	if !s.requireSecurity(w, r) {
+		return
+	}
 	scanners, err := s.securityController.ListScanners(r.Context())
 	if err != nil {
 		s.writeError(w, r, http.StatusInternalServerError, err.Error())
@@ -49,6 +61,9 @@ func (s *Server) handleListScanners(w http.ResponseWriter, r *http.Request) {
 }
 
 func (s *Server) handleInstallScanner(w http.ResponseWriter, r *http.Request) {
+	if !s.requireSecurity(w, r) {
+		return
+	}
 	var req struct {
 		ID string `json:"id"`
 	}
@@ -69,6 +84,9 @@ func (s *Server) handleInstallScanner(w http.ResponseWriter, r *http.Request) {
 }
 
 func (s *Server) handleRemoveScanner(w http.ResponseWriter, r *http.Request) {
+	if !s.requireSecurity(w, r) {
+		return
+	}
 	id := chi.URLParam(r, "id")
 	if id == "" {
 		s.writeError(w, r, http.StatusBadRequest, "scanner id is required")
@@ -83,6 +101,9 @@ func (s *Server) handleRemoveScanner(w http.ResponseWriter, r *http.Request) {
 }
 
 func (s *Server) handleConfigureScanner(w http.ResponseWriter, r *http.Request) {
+	if !s.requireSecurity(w, r) {
+		return
+	}
 	id := chi.URLParam(r, "id")
 	if id == "" {
 		s.writeError(w, r, http.StatusBadRequest, "scanner id is required")
@@ -109,6 +130,9 @@ func (s *Server) handleConfigureScanner(w http.ResponseWriter, r *http.Request)
 }
 
 func (s *Server) handleGetScannerStatus(w http.ResponseWriter, r *http.Request) {
+	if !s.requireSecurity(w, r) {
+		return
+	}
 	id := chi.URLParam(r, "id")
 	if id == "" {
 		s.writeError(w, r, http.StatusBadRequest, "scanner id is required")
@@ -126,6 +150,9 @@ func (s *Server) handleGetScannerStatus(w http.ResponseWriter, r *http.Request)
 // --- Scan operation handlers ---
 
 func (s *Server) handleStartScan(w http.ResponseWriter, r *http.Request) {
+	if !s.requireSecurity(w, r) {
+		return
+	}
 	name := chi.URLParam(r, "id")
 	if name == "" {
 		s.writeError(w, r, http.StatusBadRequest, "server name is required")
@@ -135,11 +162,12 @@ func (s *Server) handleStartScan(w http.ResponseWriter, r *http.Request) {
 	var req struct {
 		DryRun     bool     `json:"dry_run"`
 		ScannerIDs []string `json:"scanner_ids"`
+		SourceDir  string   `json:"source_dir"`
 	}
 	// Body is optional for simple scans
 	_ = json.NewDecoder(r.Body).Decode(&req)
 
-	job, err := s.securityController.StartScan(r.Context(), name, req.DryRun, req.ScannerIDs)
+	job, err := s.securityController.StartScan(r.Context(), name, req.DryRun, req.ScannerIDs, req.SourceDir)
 	if err != nil {
 		s.writeError(w, r, http.StatusInternalServerError, err.Error())
 		return
@@ -148,6 +176,9 @@ func (s *Server) handleStartScan(w http.ResponseWriter, r *http.Request) {
 }
 
 func (s *Server) handleGetScanStatus(w http.ResponseWriter, r *http.Request) {
+	if !s.requireSecurity(w, r) {
+		return
+	}
 	name := chi.URLParam(r, "id")
 	if name == "" {
 		s.writeError(w, r, http.StatusBadRequest, "server name is required")
@@ -163,6 +194,9 @@ func (s *Server) handleGetScanStatus(w http.ResponseWriter, r *http.Request) {
 }
 
 func (s *Server) handleGetScanReport(w http.ResponseWriter, r *http.Request) {
+	if !s.requireSecurity(w, r) {
+		return
+	}
 	name := chi.URLParam(r, "id")
 	if name == "" {
 		s.writeError(w, r, http.StatusBadRequest, "server name is required")
@@ -178,6 +212,9 @@ func (s *Server) handleGetScanReport(w http.ResponseWriter, r *http.Request) {
 }
 
 func (s *Server) handleCancelScan(w http.ResponseWriter, r *http.Request) {
+	if !s.requireSecurity(w, r) {
+		return
+	}
 	name := chi.URLParam(r, "id")
 	if name == "" {
 		s.writeError(w, r, http.StatusBadRequest, "server name is required")
@@ -194,6 +231,9 @@ func (s *Server) handleCancelScan(w http.ResponseWriter, r *http.Request) {
 // --- Approval handlers ---
 
 func (s *Server) handleSecurityApprove(w http.ResponseWriter, r *http.Request) {
+	if !s.requireSecurity(w, r) {
+		return
+	}
 	name := chi.URLParam(r, "id")
 	if name == "" {
 		s.writeError(w, r, http.StatusBadRequest, "server name is required")
@@ -214,6 +254,9 @@ func (s *Server) handleSecurityApprove(w http.ResponseWriter, r *http.Request) {
 }
 
 func (s *Server) handleSecurityReject(w http.ResponseWriter, r *http.Request) {
+	if !s.requireSecurity(w, r) {
+		return
+	}
 	name := chi.URLParam(r, "id")
 	if name == "" {
 		s.writeError(w, r, http.StatusBadRequest, "server name is required")
@@ -228,6 +271,9 @@ func (s *Server) handleSecurityReject(w http.ResponseWriter, r *http.Request) {
 }
 
 func (s *Server) handleCheckIntegrity(w http.ResponseWriter, r *http.Request) {
+	if !s.requireSecurity(w, r) {
+		return
+	}
 	name := chi.URLParam(r, "id")
 	if name == "" {
 		s.writeError(w, r, http.StatusBadRequest, "server name is required")
@@ -245,6 +291,9 @@ func (s *Server) handleCheckIntegrity(w http.ResponseWriter, r *http.Request) {
 // --- Overview handler ---
 
 func (s *Server) handleSecurityOverview(w http.ResponseWriter, r *http.Request) {
+	if !s.requireSecurity(w, r) {
+		return
+	}
 	overview, err := s.securityController.GetSecurityOverview(r.Context())
 	if err != nil {
 		s.writeError(w, r, http.StatusInternalServerError, err.Error())

internal/httpapi/security_scanner_test.go

@@ -62,7 +62,7 @@ func (m *mockSecurityController) GetScannerStatus(_ context.Context, id string)
 	return nil, fmt.Errorf("scanner not found: %s", id)
 }
 
-func (m *mockSecurityController) StartScan(_ context.Context, serverName string, dryRun bool, scannerIDs []string) (*scanner.ScanJob, error) {
+func (m *mockSecurityController) StartScan(_ context.Context, serverName string, dryRun bool, scannerIDs []string, sourceDir string) (*scanner.ScanJob, error) {
 	if m.startScanErr != nil {
 		return nil, m.startScanErr
 	}
@@ -532,7 +532,7 @@ func TestSecurityHandlerOverview(t *testing.T) {
 	assert.Equal(t, 1, overview.FindingsBySeverity.Critical)
 }
 
-func TestSecurityRoutesNotRegisteredWithoutController(t *testing.T) {
+func TestSecurityRoutesReturnNotImplementedWithoutController(t *testing.T) {
 	logger := zap.NewNop().Sugar()
 	ctrl := &secTestController{}
 	srv := NewServer(ctrl, logger, nil)
@@ -542,6 +542,6 @@ func TestSecurityRoutesNotRegisteredWithoutController(t *testing.T) {
 	w := httptest.NewRecorder()
 	srv.ServeHTTP(w, req)
 
-	// Should be 404 since routes are not registered without security controller
-	assert.Equal(t, http.StatusNotFound, w.Code)
+	// Should be 501 since security controller is not configured
+	assert.Equal(t, http.StatusNotImplemented, w.Code)
 }

internal/security/scanner/docker.go

@@ -110,6 +110,8 @@ func (d *DockerRunner) RunScanner(ctx context.Context, cfg ScannerRunConfig) (st
 	// Mount source directory read-only
 	if cfg.SourceDir != "" {
 		args = append(args, "-v", cfg.SourceDir+":/scan/source:ro")
+		// Also mount at /src for scanners that expect it (e.g., Semgrep Docker image)
+		args = append(args, "-v", cfg.SourceDir+":/src:ro")
 	}
 
 	// Mount report directory writable

internal/security/scanner/engine.go

@@ -110,8 +110,9 @@ func (e *Engine) StartScan(ctx context.Context, req ScanRequest, callback ScanCa
 
 	callback.OnScanStarted(job)
 
-	// Run scanners in background
-	go e.executeScan(ctx, job, scanners, req, callback)
+	// Run scanners in background with detached context
+	// (the HTTP request context may be cancelled after the response is sent)
+	go e.executeScan(context.Background(), job, scanners, req, callback)
 
 	return job, nil
 }
@@ -288,7 +289,7 @@ func (e *Engine) runSingleScanner(ctx context.Context, s *ScannerPlugin, req Sca
 		ReportDir:     reportDir,
 		NetworkMode:   networkMode,
 		Timeout:       timeout,
-		ReadOnly:      true,
+		ReadOnly:      false, // Scanner containers need to write cache/temp files
 		MemoryLimit:   "512m",
 	}
 

internal/security/scanner/registry_bundled.go

@@ -53,9 +53,9 @@ var bundledScanners = []*ScannerPlugin{
 		OptionalEnv: []EnvRequirement{
 			{Key: "SEMGREP_APP_TOKEN", Label: "Semgrep Cloud Token", Secret: true},
 		},
-		Command:    []string{"semgrep", "scan", "--sarif", "--output", "/scan/report/results.sarif"},
-		Timeout:    "120s",
-		NetworkReq: false,
+		Command:    []string{"semgrep", "scan", "--sarif", "--output", "/scan/report/results.sarif", "/scan/source"},
+		Timeout:    "300s",
+		NetworkReq: true, // Downloads rules from registry
 	},
 	{
 		ID:          "trivy-mcp",
@@ -64,7 +64,7 @@ var bundledScanners = []*ScannerPlugin{
 		Description: "Comprehensive vulnerability scanner for filesystem, dependencies, and container images. Detects known CVEs and misconfigurations.",
 		License:     "Apache-2.0",
 		Homepage:    "https://trivy.dev",
-		DockerImage: "aquasec/trivy:latest",
+		DockerImage: "aquasecurity/trivy:latest",
 		Inputs:      []string{"source", "container_image"},
 		Outputs:     []string{"sarif"},
 		RequiredEnv: nil,

internal/security/scanner/service.go

@@ -66,21 +66,44 @@ type Service struct {
 // NewService creates a new SecurityService
 func NewService(storage Storage, registry *Registry, docker *DockerRunner, dataDir string, logger *zap.Logger) *Service {
 	engine := NewEngine(docker, registry, dataDir, logger)
-	return &Service{
+	svc := &Service{
 		storage:  storage,
 		engine:   engine,
 		registry: registry,
 		docker:   docker,
 		emitter:  &NoopEmitter{},
 		logger:   logger,
 	}
+	// Restore installed scanner state from storage (survives restart)
+	svc.syncRegistryFromStorage()
+	return svc
 }
 
 // SetEmitter sets the event emitter for the service
 func (s *Service) SetEmitter(emitter EventEmitter) {
 	s.emitter = emitter
 }
 
+// syncRegistryFromStorage updates the in-memory registry status from
+// persistent storage. This is needed after restart so the engine knows
+// which scanners are installed/configured.
+func (s *Service) syncRegistryFromStorage() {
+	installed, err := s.storage.ListScanners()
+	if err != nil || len(installed) == 0 {
+		return
+	}
+	for _, inst := range installed {
+		_ = s.registry.UpdateStatus(inst.ID, inst.Status)
+		// Also update configured env so the engine can pass it to containers
+		if inst.ConfiguredEnv != nil {
+			if reg, err := s.registry.Get(inst.ID); err == nil {
+				reg.ConfiguredEnv = inst.ConfiguredEnv
+			}
+		}
+	}
+	s.logger.Info("Synced scanner registry from storage", zap.Int("count", len(installed)))
+}
+
 // --- Scanner Management ---
 
 // ListScanners returns all scanners from registry merged with installed state from storage
@@ -282,11 +305,12 @@ func (a *scanCallbackAdapter) OnScanFailed(job *ScanJob, err error) {
 }
 
 // StartScan triggers a security scan for a server
-func (s *Service) StartScan(ctx context.Context, serverName string, dryRun bool, scannerIDs []string) (*ScanJob, error) {
+func (s *Service) StartScan(ctx context.Context, serverName string, dryRun bool, scannerIDs []string, sourceDir string) (*ScanJob, error) {
 	req := ScanRequest{
 		ServerName: serverName,
 		DryRun:     dryRun,
 		ScannerIDs: scannerIDs,
+		SourceDir:  sourceDir,
 	}
 
 	callback := &scanCallbackAdapter{service: s}
@@ -436,6 +460,12 @@ func (s *Service) CheckIntegrity(ctx context.Context, serverName string) (*Integ
 
 // --- Overview ---
 
+// GetSecurityOverview returns aggregated security statistics.
+// Satisfies the httpapi.SecurityController interface.
+func (s *Service) GetSecurityOverview(ctx context.Context) (*SecurityOverview, error) {
+	return s.GetOverview(ctx)
+}
+
 // GetOverview returns aggregated security statistics
 func (s *Service) GetOverview(ctx context.Context) (*SecurityOverview, error) {
 	overview := &SecurityOverview{}

internal/server/server.go

@@ -29,6 +29,7 @@ import (
 	"github.com/smart-mcp-proxy/mcpproxy-go/internal/observability"
 	"github.com/smart-mcp-proxy/mcpproxy-go/internal/runtime"
 	"github.com/smart-mcp-proxy/mcpproxy-go/internal/secret"
+	"github.com/smart-mcp-proxy/mcpproxy-go/internal/security/scanner"
 	"github.com/smart-mcp-proxy/mcpproxy-go/internal/storage"
 	"github.com/smart-mcp-proxy/mcpproxy-go/internal/tlslocal"
 	"github.com/smart-mcp-proxy/mcpproxy-go/internal/transport"
@@ -1647,6 +1648,18 @@ func (s *Server) startCustomHTTPServer(ctx context.Context, streamableServer *se
 		connectSvc := connect.NewService(cfg.Listen, cfg.APIKey)
 		httpAPIServer.SetConnectService(connectSvc)
 	}
+	// Wire security scanner service (Spec 039)
+	if sm := s.runtime.StorageManager(); sm != nil {
+		cfg := s.runtime.Config()
+		dataDir := ""
+		if cfg != nil {
+			dataDir = cfg.DataDir
+		}
+		secRegistry := scanner.NewRegistry(dataDir, s.logger)
+		secDocker := scanner.NewDockerRunner(s.logger)
+		secService := scanner.NewService(sm, secRegistry, secDocker, dataDir, s.logger)
+		httpAPIServer.SetSecurityController(secService)
+	}
 	// Wire teams multi-user OAuth (no-op in personal edition)
 	wireTeamsOAuth(s, httpAPIServer)
 	mux.Handle("/api/", httpAPIServer)