fix(scan-eval): gate NetworkReq scanner skip on Docker-unavailable only (Codex finding)

The runnabilityReason function unconditionally skipped NetworkReq scanners
even when Docker was enabled. When the operator explicitly opts in via
--scanners + MCPPROXY_SCAN_EVAL_DOCKER=1, network-req scanners should be
runnable — the Docker-off case is already covered by the earlier return.

Adds TestSelectScanners_NetworkReq_RunnableWhenDockerEnabled to lock in
the new behavior.

Related #574

Author: Dumbris

cmd/scan-eval/scanners.go

@@ -56,8 +56,9 @@ func selectScanners(reg *scanner.Registry, csv string, dockerEnabled bool, looku
 // runnabilityReason returns "" when the scanner can run under the given
 // constraints, otherwise a human-readable reason it must be skipped. Used both
 // to partition in selectScanners and to explain the skip to the operator, so
-// the gating rules live in exactly one place (DRY). Order matters: Docker is
-// the cheapest gate, then secrets, then network.
+// the gating rules live in exactly one place (DRY). Order: Docker is the
+// cheapest gate, then secrets. Network-req scanners are NOT gated here — the
+// Docker gate above subsumes that (Docker-off → everything skipped).
 func runnabilityReason(p *scanner.ScannerPlugin, dockerEnabled bool, lookupEnv func(string) (string, bool)) string {
 	if !dockerEnabled {
 		return "Docker isolation disabled (set MCPPROXY_SCAN_EVAL_DOCKER=1 to enable)"
@@ -67,9 +68,12 @@ func runnabilityReason(p *scanner.ScannerPlugin, dockerEnabled bool, lookupEnv f
 			return fmt.Sprintf("missing required secret %s", req.Key)
 		}
 	}
-	if p.NetworkReq {
-		return "requires network access (unavailable in offline corpus evaluation)"
-	}
+	// Network-req scanners are NOT skipped here: when Docker is available the
+	// operator explicitly opted in via --scanners + MCPPROXY_SCAN_EVAL_DOCKER=1,
+	// and running the scanner (even offline — the runner enforces NetworkMode=none,
+	// Security-by-Default) is preferred over silently skipping it. The Docker gate
+	// above already covers the Docker-unavailable case (everything skipped), so
+	// reaching here means Docker IS enabled.
 	return ""
 }
 

cmd/scan-eval/scanners_test.go

@@ -17,9 +17,10 @@ import (
 )
 
 // testRegistry builds an isolated registry (temp dataDir → no user registry
-// bleed) seeded with two controlled custom scanners: one offline+no-secret and
-// one that requires a secret env var. Metadata-driven gating is exercised
-// against these so the tests do not couple to bundled-registry churn.
+// bleed) seeded with three controlled custom scanners: one offline+no-secret,
+// one that requires a secret env var, and one that requires network access.
+// Metadata-driven gating is exercised against these so the tests do not couple
+// to bundled-registry churn.
 func testRegistry(t *testing.T) *scanner.Registry {
 	t.Helper()
 	reg := scanner.NewRegistry(t.TempDir(), zap.NewNop())
@@ -38,7 +39,15 @@ func testRegistry(t *testing.T) *scanner.Registry {
 		Command:     []string{"scan"},
 		RequiredEnv: []scanner.EnvRequirement{{Key: "ENG_TEST_TOKEN", Label: "Token", Secret: true}},
 	}
-	for _, s := range []*scanner.ScannerPlugin{offline, secret} {
+	netReq := &scanner.ScannerPlugin{
+		ID:          "eng-netreq",
+		Name:        "Eng Network Required",
+		DockerImage: "example/eng-netreq:latest",
+		Inputs:      []string{"source"},
+		Command:     []string{"scan"},
+		NetworkReq:  true,
+	}
+	for _, s := range []*scanner.ScannerPlugin{offline, secret, netReq} {
 		if err := reg.Register(s); err != nil {
 			t.Fatalf("Register(%s): %v", s.ID, err)
 		}
@@ -100,6 +109,26 @@ func TestSelectScanners_DockerDisabled(t *testing.T) {
 	}
 }
 
+// TestSelectScanners_NetworkReq_RunnableWhenDockerEnabled — a scanner that
+// requires network access is NOT skipped when Docker is enabled. The operator
+// explicitly opted in via --scanners + MCPPROXY_SCAN_EVAL_DOCKER=1; running the
+// scanner (even offline — the runner enforces NetworkMode=none) is preferred
+// over silently skipping it. The Docker-unavailable case is covered by
+// TestSelectScanners_DockerDisabled (everything skipped).
+func TestSelectScanners_NetworkReq_RunnableWhenDockerEnabled(t *testing.T) {
+	reg := testRegistry(t)
+	run, skipped, err := selectScanners(reg, "eng-netreq", true, env(nil))
+	if err != nil {
+		t.Fatalf("selectScanners: %v", err)
+	}
+	if len(skipped) != 0 {
+		t.Errorf("skipped = %+v, want none (docker enabled, network-req scanner should be runnable)", skipped)
+	}
+	if len(run) != 1 || run[0].ID != "eng-netreq" {
+		t.Errorf("run = %v, want [eng-netreq]", ids(run))
+	}
+}
+
 // TestSelectScanners_Runnable — offline scanner with Docker enabled, and a
 // secret scanner with its secret present, both become runnable.
 func TestSelectScanners_Runnable(t *testing.T) {