Skip to content

Commit bf8b2d6

Browse files
authored
feat(scanner): compute and render per-scanner duration_ms (#667)
The scan report carried per-scanner started_at/completed_at but no duration, and the CLI rendered no per-scanner timing. Add a derived duration_ms to each scanner status and surface it in the CLI: - ScannerJobStatus gains a duration_ms field and a Duration() helper; the engine recomputes it once both timestamps are known (the live scan path sets start and completion in separate updates). - security status grows a DURATION column. - security report renders a per-scanner "Scanner timing" block. - CLI reads duration_ms, falling back to started_at/completed_at for reports produced before the field existed. Related to MCP-2402
1 parent 35d2f35 commit bf8b2d6

6 files changed

Lines changed: 317 additions & 29 deletions

File tree

cmd/mcpproxy/security_cmd.go

Lines changed: 106 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1254,6 +1254,104 @@ func getMapFloat(m map[string]interface{}, key string) float64 {
12541254
return 0
12551255
}
12561256

1257+
// scannerDurationMs returns a scanner_statuses entry's wall-clock duration in
1258+
// milliseconds. It prefers the explicit duration_ms field and falls back to
1259+
// computing it from started_at/completed_at so reports produced before
1260+
// duration_ms was recorded still render a timing value.
1261+
func scannerDurationMs(ss map[string]interface{}) float64 {
1262+
if ms := getMapFloat(ss, "duration_ms"); ms > 0 {
1263+
return ms
1264+
}
1265+
start := getMapString(ss, "started_at")
1266+
end := getMapString(ss, "completed_at")
1267+
if start == "" || end == "" {
1268+
return 0
1269+
}
1270+
st, err1 := time.Parse(time.RFC3339Nano, start)
1271+
et, err2 := time.Parse(time.RFC3339Nano, end)
1272+
if err1 != nil || err2 != nil || et.Before(st) {
1273+
return 0
1274+
}
1275+
return float64(et.Sub(st).Milliseconds())
1276+
}
1277+
1278+
// formatScannerDurationMs renders a per-scanner duration for human-readable
1279+
// output: sub-second values in milliseconds, larger values as a compact "X.Ys",
1280+
// and missing/zero timing as a dash.
1281+
func formatScannerDurationMs(ms float64) string {
1282+
if ms <= 0 {
1283+
return "-"
1284+
}
1285+
if ms < 1000 {
1286+
return fmt.Sprintf("%dms", int(ms))
1287+
}
1288+
return fmt.Sprintf("%.1fs", ms/1000)
1289+
}
1290+
1291+
// printScannerStatusTable renders the per-scanner execution table including a
1292+
// DURATION column. Nothing is printed when there are no scanner statuses.
1293+
func printScannerStatusTable(scannerStatuses []interface{}) {
1294+
if len(scannerStatuses) == 0 {
1295+
return
1296+
}
1297+
fmt.Println()
1298+
fmt.Printf(" %-20s %-12s %-10s %-8s %s\n", "SCANNER", "STATUS", "DURATION", "FINDINGS", "ERROR")
1299+
fmt.Printf(" %s\n", strings.Repeat("-", 75))
1300+
for _, s := range scannerStatuses {
1301+
ss, ok := s.(map[string]interface{})
1302+
if !ok {
1303+
continue
1304+
}
1305+
scannerID := getMapString(ss, "scanner_id")
1306+
ssStatus := getMapString(ss, "status")
1307+
findings := "0"
1308+
if fc, ok := ss["findings_count"].(float64); ok {
1309+
findings = fmt.Sprintf("%d", int(fc))
1310+
}
1311+
ssErr := getMapString(ss, "error")
1312+
if len(ssErr) > 25 {
1313+
ssErr = ssErr[:22] + "..."
1314+
}
1315+
dur := formatScannerDurationMs(scannerDurationMs(ss))
1316+
fmt.Printf(" %-20s %-12s %-10s %-8s %s\n", scannerID, ssStatus, dur, findings, ssErr)
1317+
}
1318+
}
1319+
1320+
// printScannerTimings renders a compact per-scanner wall-clock timing block
1321+
// from a scan report's scanner_statuses. Nothing is printed when timing data
1322+
// is absent.
1323+
func printScannerTimings(report map[string]interface{}) {
1324+
statuses, ok := report["scanner_statuses"].([]interface{})
1325+
if !ok || len(statuses) == 0 {
1326+
return
1327+
}
1328+
type timingRow struct{ id, status, dur string }
1329+
rows := make([]timingRow, 0, len(statuses))
1330+
for _, s := range statuses {
1331+
ss, ok := s.(map[string]interface{})
1332+
if !ok {
1333+
continue
1334+
}
1335+
id := getMapString(ss, "scanner_id")
1336+
if id == "" {
1337+
continue
1338+
}
1339+
rows = append(rows, timingRow{
1340+
id: id,
1341+
status: getMapString(ss, "status"),
1342+
dur: formatScannerDurationMs(scannerDurationMs(ss)),
1343+
})
1344+
}
1345+
if len(rows) == 0 {
1346+
return
1347+
}
1348+
fmt.Println()
1349+
fmt.Println("Scanner timing:")
1350+
for _, r := range rows {
1351+
fmt.Printf(" %-20s %-12s %s\n", r.id, r.status, r.dur)
1352+
}
1353+
}
1354+
12571355
func runSecurityStatus(_ *cobra.Command, args []string) error {
12581356
client, _, err := newSecurityCLIClient()
12591357
if err != nil {
@@ -1310,26 +1408,9 @@ func runSecurityStatus(_ *cobra.Command, args []string) error {
13101408
fmt.Printf(" Error: %s\n", errMsg)
13111409
}
13121410

1313-
// Per-scanner statuses
1314-
if scannerStatuses, ok := status["scanner_statuses"].([]interface{}); ok && len(scannerStatuses) > 0 {
1315-
fmt.Println()
1316-
fmt.Printf(" %-20s %-12s %-8s %s\n", "SCANNER", "STATUS", "FINDINGS", "ERROR")
1317-
fmt.Printf(" %s\n", strings.Repeat("-", 65))
1318-
for _, s := range scannerStatuses {
1319-
if ss, ok := s.(map[string]interface{}); ok {
1320-
scannerID := getMapString(ss, "scanner_id")
1321-
ssStatus := getMapString(ss, "status")
1322-
findings := "0"
1323-
if fc, ok := ss["findings_count"].(float64); ok {
1324-
findings = fmt.Sprintf("%d", int(fc))
1325-
}
1326-
ssErr := getMapString(ss, "error")
1327-
if len(ssErr) > 25 {
1328-
ssErr = ssErr[:22] + "..."
1329-
}
1330-
fmt.Printf(" %-20s %-12s %-8s %s\n", scannerID, ssStatus, findings, ssErr)
1331-
}
1332-
}
1411+
// Per-scanner statuses (includes a DURATION column)
1412+
if scannerStatuses, ok := status["scanner_statuses"].([]interface{}); ok {
1413+
printScannerStatusTable(scannerStatuses)
13331414
}
13341415

13351416
return nil
@@ -1768,6 +1849,11 @@ func printReportTable(serverName string, report map[string]interface{}, failedSc
17681849
fmt.Println(line)
17691850
}
17701851

1852+
// Per-scanner wall-clock timing, so users can see which scanner dominated
1853+
// the scan time. Sourced from scanner_statuses; silently skipped when the
1854+
// report carries no per-scanner status data.
1855+
printScannerTimings(report)
1856+
17711857
// Scan context: show the user what was actually scanned. Without this the
17721858
// terse "0 findings" / "1 finding" output gives no signal as to whether
17731859
// the scan looked at real source code (docker_extract), a working_dir

cmd/mcpproxy/security_cmd_test.go

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -329,3 +329,101 @@ func TestPrintReportTableRendersFailedScannerReasons(t *testing.T) {
329329
}
330330
}
331331
}
332+
333+
// TestFormatScannerDurationMs verifies the per-scanner duration formatter:
334+
// sub-second values render in milliseconds, second-and-up values render as a
335+
// compact "X.Ys", and missing/zero timing renders as a dash rather than "0ms".
336+
func TestFormatScannerDurationMs(t *testing.T) {
337+
cases := []struct {
338+
ms float64
339+
want string
340+
}{
341+
{0, "-"},
342+
{-5, "-"},
343+
{850, "850ms"},
344+
{999, "999ms"},
345+
{1000, "1.0s"},
346+
{1500, "1.5s"},
347+
{12340, "12.3s"},
348+
}
349+
for _, c := range cases {
350+
if got := formatScannerDurationMs(c.ms); got != c.want {
351+
t.Errorf("formatScannerDurationMs(%v) = %q, want %q", c.ms, got, c.want)
352+
}
353+
}
354+
}
355+
356+
// TestScannerDurationMs verifies that the explicit duration_ms field is
357+
// preferred, with a fallback to computing the duration from the
358+
// started_at/completed_at timestamps for reports produced before duration_ms
359+
// was recorded.
360+
func TestScannerDurationMs(t *testing.T) {
361+
// Explicit duration_ms wins.
362+
if got := scannerDurationMs(map[string]interface{}{"duration_ms": float64(1200)}); got != 1200 {
363+
t.Errorf("expected 1200 from explicit duration_ms, got %v", got)
364+
}
365+
// Fallback: compute from timestamps when duration_ms is absent.
366+
got := scannerDurationMs(map[string]interface{}{
367+
"started_at": "2026-06-14T10:00:00Z",
368+
"completed_at": "2026-06-14T10:00:02Z",
369+
})
370+
if got != 2000 {
371+
t.Errorf("expected 2000 from timestamp fallback, got %v", got)
372+
}
373+
// No timing data → 0.
374+
if got := scannerDurationMs(map[string]interface{}{"scanner_id": "x"}); got != 0 {
375+
t.Errorf("expected 0 with no timing, got %v", got)
376+
}
377+
}
378+
379+
// TestPrintScannerStatusTableRendersDuration verifies the per-scanner status
380+
// table gained a DURATION column populated from each scanner's timing.
381+
func TestPrintScannerStatusTableRendersDuration(t *testing.T) {
382+
out := captureStdout(t, func() {
383+
printScannerStatusTable([]interface{}{
384+
map[string]interface{}{
385+
"scanner_id": "mcp-scan",
386+
"status": "completed",
387+
"findings_count": float64(2),
388+
"duration_ms": float64(1500),
389+
},
390+
map[string]interface{}{
391+
"scanner_id": "trivy-mcp",
392+
"status": "completed",
393+
"started_at": "2026-06-14T10:00:00Z",
394+
"completed_at": "2026-06-14T10:00:03Z",
395+
},
396+
})
397+
})
398+
for _, w := range []string{"DURATION", "mcp-scan", "1.5s", "trivy-mcp", "3.0s"} {
399+
if !strings.Contains(out, w) {
400+
t.Errorf("status table missing %q\n--- output ---\n%s", w, out)
401+
}
402+
}
403+
}
404+
405+
// TestPrintReportTableRendersScannerTimings verifies the scan report renders a
406+
// per-scanner timing block sourced from scanner_statuses.
407+
func TestPrintReportTableRendersScannerTimings(t *testing.T) {
408+
report := map[string]interface{}{
409+
"job_id": "scan-foo-1",
410+
"risk_score": float64(0),
411+
"scanned_at": "2026-04-26T17:39:05Z",
412+
"findings": []interface{}{},
413+
"scanner_statuses": []interface{}{
414+
map[string]interface{}{
415+
"scanner_id": "mcp-scan",
416+
"status": "completed",
417+
"duration_ms": float64(1200),
418+
},
419+
},
420+
}
421+
out := captureStdout(t, func() {
422+
_ = printReportTable("foo", report, nil)
423+
})
424+
for _, w := range []string{"Scanner timing:", "mcp-scan", "1.2s"} {
425+
if !strings.Contains(out, w) {
426+
t.Errorf("report missing %q\n--- output ---\n%s", w, out)
427+
}
428+
}
429+
}

docs/cli/security-commands.md

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,11 @@ Risk Score: 0/100
297297
Scanned: 2026-04-10 10:08:19
298298
Scanners: 6 run, 1 failed (ramparts) of 7
299299
300+
Scanner timing:
301+
mcp-scan completed 1.2s
302+
trivy-mcp completed 12.3s
303+
ramparts failed -
304+
300305
WARNING: Scan coverage incomplete: 1 of 7 scanners did not run
301306
302307
=== Security Scan (Pass 1) ===
@@ -421,17 +426,19 @@ Scan Status: everything
421426
Started: 2026-04-10 08:41:17
422427
Finished: 2026-04-10 08:42:09
423428

424-
SCANNER STATUS FINDINGS ERROR
425-
-----------------------------------------------------------------
426-
cisco-mcp-scanner completed 0
427-
mcp-ai-scanner completed 0
428-
mcp-scan failed 0 scanner mcp-scan produ...
429-
nova-proximity completed 0
430-
ramparts failed 0 scanner ramparts produ...
431-
semgrep-mcp completed 0
432-
trivy-mcp completed 0
429+
SCANNER STATUS DURATION FINDINGS ERROR
430+
---------------------------------------------------------------------------
431+
cisco-mcp-scanner completed 1.2s 0
432+
mcp-ai-scanner completed 3.4s 0
433+
mcp-scan failed 850ms 0 scanner mcp-scan produ...
434+
nova-proximity completed 2.1s 0
435+
ramparts failed 120ms 0 scanner ramparts produ...
436+
semgrep-mcp completed 5.7s 0
437+
trivy-mcp completed 12.3s 0
433438
```
434439

440+
The `DURATION` column is each scanner's wall-clock execution time, computed from its `started_at`/`completed_at` timestamps. It renders `-` when timing is unavailable (e.g. a scanner that never started).
441+
435442
:::tip Use status for diagnostics
436443
If `security report` shows "0 findings" but you think a scanner should have flagged something, open `status` — failed scanners appear here with their truncated stderr. The full stderr is available via `security status <server> -o json`.
437444
:::
@@ -469,6 +476,11 @@ Risk Score: 0/100
469476
Scanned: 2026-04-10 10:08:19
470477
Scanners: 6 run, 1 failed (ramparts) of 7
471478
479+
Scanner timing:
480+
mcp-scan completed 1.2s
481+
trivy-mcp completed 12.3s
482+
ramparts failed -
483+
472484
WARNING: Scan coverage incomplete: 1 of 7 scanners did not run
473485
474486
=== Security Scan (Pass 1) ===
@@ -488,6 +500,7 @@ The `Scanners: X run, Y failed (names) of Z` line surfaces per-scanner failures
488500
- `summary` — severity counts (`critical`, `high`, `medium`, `low`, `info`, `dangerous`, `warnings`, `info_level`, `total`)
489501
- `findings` — normalized findings across all scanners
490502
- `reports` — per-scanner raw results (also includes SARIF when `?include_sarif=true` is passed to the REST endpoint)
503+
- `scanner_statuses` — per-scanner execution records, each with `scanner_id`, `status`, `started_at`, `completed_at`, `duration_ms` (wall-clock execution time in milliseconds), `findings_count`, and `error`
491504
- `scan_context` — source method, source path, scanned file list
492505
- `scanners_run`, `scanners_failed`, `scanners_total`
493506
- `pass1_complete`, `pass2_complete`, `pass2_running`

internal/security/scanner/engine.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -675,6 +675,10 @@ func (e *Engine) updateScannerStatus(job *ScanJob, scannerID, status string, sta
675675
}
676676
job.ScannerStatuses[i].Error = errMsg
677677
job.ScannerStatuses[i].FindingsCount = findingsCount
678+
// Recompute duration once both timestamps are known. The live scan
679+
// path sets StartedAt and CompletedAt in separate calls, so this
680+
// derives from the stored StartedAt when CompletedAt arrives.
681+
job.ScannerStatuses[i].DurationMs = job.ScannerStatuses[i].Duration().Milliseconds()
678682
return
679683
}
680684
}
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
package scanner
2+
3+
import (
4+
"testing"
5+
"time"
6+
7+
"go.uber.org/zap"
8+
)
9+
10+
// TestScannerJobStatusDuration verifies per-scanner wall-clock computation,
11+
// including the degenerate cases (still running, never started, clock skew)
12+
// where we must report 0 rather than a negative or bogus value.
13+
func TestScannerJobStatusDuration(t *testing.T) {
14+
start := time.Date(2026, 6, 14, 10, 0, 0, 0, time.UTC)
15+
tests := []struct {
16+
name string
17+
status ScannerJobStatus
18+
wantMs int64
19+
}{
20+
{
21+
name: "normal duration",
22+
status: ScannerJobStatus{StartedAt: start, CompletedAt: start.Add(1500 * time.Millisecond)},
23+
wantMs: 1500,
24+
},
25+
{
26+
name: "never started",
27+
status: ScannerJobStatus{CompletedAt: start},
28+
wantMs: 0,
29+
},
30+
{
31+
name: "still running",
32+
status: ScannerJobStatus{StartedAt: start},
33+
wantMs: 0,
34+
},
35+
{
36+
name: "completed before started (clock skew)",
37+
status: ScannerJobStatus{StartedAt: start, CompletedAt: start.Add(-time.Second)},
38+
wantMs: 0,
39+
},
40+
}
41+
for _, tt := range tests {
42+
t.Run(tt.name, func(t *testing.T) {
43+
if got := tt.status.Duration().Milliseconds(); got != tt.wantMs {
44+
t.Errorf("Duration() = %d ms, want %d ms", got, tt.wantMs)
45+
}
46+
})
47+
}
48+
}
49+
50+
// TestUpdateScannerStatusComputesDurationMs guards that DurationMs is populated
51+
// as soon as both timestamps are known. The live scan path sets StartedAt and
52+
// CompletedAt in two separate updateScannerStatus calls, so the duration must
53+
// be recomputed from the stored StartedAt when CompletedAt arrives.
54+
func TestUpdateScannerStatusComputesDurationMs(t *testing.T) {
55+
engine := NewEngine(nil, nil, t.TempDir(), zap.NewNop())
56+
job := &ScanJob{
57+
ID: "job-1",
58+
ScannerStatuses: []ScannerJobStatus{
59+
{ScannerID: "scanner-a", Status: ScanJobStatusPending},
60+
},
61+
}
62+
start := time.Now()
63+
64+
// Running: only StartedAt is set; CompletedAt stays zero → no duration yet.
65+
engine.updateScannerStatus(job, "scanner-a", ScanJobStatusRunning, start, time.Time{}, "", 0)
66+
if got := job.ScannerStatuses[0].DurationMs; got != 0 {
67+
t.Fatalf("DurationMs = %d while running, want 0", got)
68+
}
69+
70+
// Completed: CompletedAt set; duration computed from the stored StartedAt.
71+
engine.updateScannerStatus(job, "scanner-a", ScanJobStatusCompleted, time.Time{}, start.Add(2*time.Second), "", 3)
72+
if got := job.ScannerStatuses[0].DurationMs; got != 2000 {
73+
t.Errorf("DurationMs = %d, want 2000", got)
74+
}
75+
}

0 commit comments

Comments
 (0)