Skip to content

Commit 68dd21a

Browse files
Dumbrisclaude
andcommitted
fix(security): FR-014 verdict purity on the report page — share the tier-driven verdict with the report payload
Codex review follow-up. The server-list summary (GetScanSummary) derived the verdict tier-driven/baseline-only (FR-014), but the report page badge still read the RAW threat-level ReportSummary — with deep scan on, a tierless Docker-scanner finding classified "dangerous" made the report say dangerous while the server list said clean, and the same finding counted as Warning on one surface and Dangerous on the other. - extract deriveBaselineVerdict() as the single source of truth, shared by GetScanSummary and AggregateReports so the two surfaces structurally cannot disagree - AggregatedReport gains Verdict + tier-driven FindingCounts (additive; raw Summary counts retained for transparency) - ScanReport.vue: status badge, threat tiles, Quarantine-button gate and the Approve-disable gate (hasUnresolvedCritical) now read the tier-driven verdict/finding_counts (raw summary only as a fallback for pre-Spec-077 payloads); the approve gate now mirrors the backend hard-tier-only isBlockingFinding instead of raw summary.critical - ServerDetail.vue: approve-confirmation count and the Security-tab summary strip prefer tier-driven finding_counts over the raw report summary, so a tierless deep-scan "dangerous" finding no longer triggers the "Dangerous Findings Detected" modal on a clean baseline - frontend/src/types/api.ts: SecurityScanSummary catches up with the wire contract (deep_scan, scanners_run/failed/total); SecurityScanReport gains verdict + finding_counts Tests: engine-level AggregateReports verdict matrix (tierless-dangerous → clean/warning-bucket, hard → dangerous, soft → warnings) and an end-to-end pin that GetScanReportByJobID.Verdict/FindingCounts equal GetScanSummary for the same scan data. Assumption documented (zero-interruption policy): ServerCard.vue was reported as sharing the raw-summary preference but already reads only the tier-driven security_scan.finding_counts — left unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 819bc59 commit 68dd21a

8 files changed

Lines changed: 317 additions & 73 deletions

File tree

frontend/src/types/api.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,15 @@ export interface SecurityScanSummary {
4242
risk_score: number
4343
status: SecurityScanStatus
4444
finding_counts?: SecurityScanFindingCounts
45+
// Scanner coverage for the primary (baseline) scan pass — informational only.
46+
// Spec 077 US3 (FR-008/FR-014): status derives SOLELY from baseline findings;
47+
// a failed Docker deep scanner never downgrades the verdict.
48+
scanners_run?: number
49+
scanners_failed?: number
50+
scanners_total?: number
51+
// Opt-in deep-scan layer status (Spec 077 US3), always emitted on a computed
52+
// summary (enabled=false when off). Informational — never influences status.
53+
deep_scan?: DeepScanDescriptor
4554
}
4655

4756
// Security scan finding (Spec 039)
@@ -74,7 +83,14 @@ export interface SecurityScanReport {
7483
status: SecurityScanStatus
7584
risk_score: number
7685
findings: SecurityScanFinding[]
77-
finding_counts: SecurityScanFindingCounts
86+
// Tier-driven, baseline-only verdict (Spec 077 FR-014): 'dangerous' only for
87+
// hard-tier baseline findings; tierless deep-scan/external findings never
88+
// move it. Verdict-bearing UI must read this, NOT summary (raw counts).
89+
verdict?: 'clean' | 'warnings' | 'dangerous'
90+
// Tier-driven buckets matching SecurityScanSummary.finding_counts (a tierless
91+
// 'dangerous' finding buckets as warning — informs, never gates).
92+
finding_counts?: SecurityScanFindingCounts
93+
// Raw threat-level/severity counts across ALL findings — transparency only.
7894
summary: SecurityScanReportSummary
7995
scanned_at: string
8096
duration_ms?: number

frontend/src/views/ScanReport.vue

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -96,25 +96,25 @@
9696
<div class="stats shadow bg-base-100">
9797
<div class="stat py-3 px-4">
9898
<div class="stat-title text-xs">Dangerous</div>
99-
<div class="stat-value text-lg text-error">{{ report.summary?.dangerous ?? 0 }}</div>
99+
<div class="stat-value text-lg text-error">{{ threatCounts.dangerous }}</div>
100100
</div>
101101
</div>
102102
<div class="stats shadow bg-base-100">
103103
<div class="stat py-3 px-4">
104104
<div class="stat-title text-xs">Warnings</div>
105-
<div class="stat-value text-lg text-warning">{{ report.summary?.warnings ?? 0 }}</div>
105+
<div class="stat-value text-lg text-warning">{{ threatCounts.warnings }}</div>
106106
</div>
107107
</div>
108108
<div class="stats shadow bg-base-100">
109109
<div class="stat py-3 px-4">
110110
<div class="stat-title text-xs">Info</div>
111-
<div class="stat-value text-lg text-info">{{ report.summary?.info_level ?? 0 }}</div>
111+
<div class="stat-value text-lg text-info">{{ threatCounts.info }}</div>
112112
</div>
113113
</div>
114114
<div class="stats shadow bg-base-100">
115115
<div class="stat py-3 px-4">
116116
<div class="stat-title text-xs">Total</div>
117-
<div class="stat-value text-lg">{{ report.summary?.total ?? 0 }}</div>
117+
<div class="stat-value text-lg">{{ threatCounts.total }}</div>
118118
</div>
119119
</div>
120120
</div>
@@ -555,7 +555,7 @@
555555
</div>
556556
<div class="flex gap-2">
557557
<button
558-
v-if="serverAdminState === 'enabled' && report.summary?.dangerous > 0"
558+
v-if="serverAdminState === 'enabled' && reportStatus === 'dangerous'"
559559
@click="quarantineServer"
560560
:disabled="actionLoading"
561561
class="btn btn-error btn-sm"
@@ -638,17 +638,41 @@ const scanContext = computed(() => {
638638
return report.value?.scan_context || null
639639
})
640640
641-
// Status display
641+
// Status display. Spec 077 FR-014 verdict purity: the badge shows the
642+
// tier-driven, baseline-only `verdict` computed server-side with the SAME
643+
// predicate as the server-list status, so this page can never say "dangerous"
644+
// while the server list says "clean" (a tierless deep-scan/external finding
645+
// never moves the verdict). Raw summary counts remain only as a fallback for
646+
// reports served by a core that predates the verdict field.
642647
const reportStatus = computed(() => {
643648
if (!report.value) return 'unknown'
644649
if (report.value.scan_complete === false) return 'incomplete'
645650
if (report.value.empty_scan) return 'empty'
646651
if (!report.value.findings || report.value.findings.length === 0) return 'clean'
652+
if (report.value.verdict) return report.value.verdict
647653
if (report.value.summary?.dangerous > 0) return 'dangerous'
648654
if (report.value.summary?.warnings > 0) return 'warnings'
649655
return 'clean'
650656
})
651657
658+
// Threat tiles use the tier-driven buckets (finding_counts) matching the
659+
// server list exactly (Spec 077 FR-014): a tierless deep-scan "dangerous"
660+
// finding counts as a warning on BOTH surfaces. Falls back to the raw
661+
// threat-level summary for pre-Spec-077 payloads.
662+
const threatCounts = computed(() => {
663+
const r = report.value
664+
const fc = r?.finding_counts
665+
if (fc) {
666+
return { dangerous: fc.dangerous ?? 0, warnings: fc.warning ?? 0, info: fc.info ?? 0, total: fc.total ?? 0 }
667+
}
668+
return {
669+
dangerous: r?.summary?.dangerous ?? 0,
670+
warnings: r?.summary?.warnings ?? 0,
671+
info: r?.summary?.info_level ?? 0,
672+
total: r?.summary?.total ?? 0,
673+
}
674+
})
675+
652676
const statusBadgeClass = computed(() => {
653677
switch (reportStatus.value) {
654678
case 'dangerous': return 'badge-error'
@@ -800,8 +824,15 @@ async function quarantineServer() {
800824
801825
// F-04: Go through the security-aware approval path instead of the legacy
802826
// unquarantine endpoint. hasUnresolvedCritical disables the primary Approve
803-
// button so the user must use Force Approve explicitly.
827+
// button so the user must use Force Approve explicitly. It mirrors the
828+
// backend approval gate (Spec 077 FR-021: hard-tier BASELINE findings only)
829+
// via the tier-driven finding_counts — a tierless deep-scan/external finding
830+
// or a non-blocking soft finding with "critical" severity must not lock the
831+
// Approve button when the backend would accept. Raw summary.critical is only
832+
// a fallback for payloads that predate finding_counts.
804833
const hasUnresolvedCritical = computed(() => {
834+
const fc = report.value?.finding_counts
835+
if (fc) return (fc.dangerous ?? 0) > 0
805836
return (report.value?.summary?.critical ?? 0) > 0
806837
})
807838

frontend/src/views/ServerDetail.vue

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1230,10 +1230,10 @@
12301230
<div class="text-xs text-base-content/50">Risk Score</div>
12311231
</div>
12321232
<div class="flex gap-4 text-sm">
1233-
<span v-if="scanReport.summary?.dangerous" class="text-error font-semibold">{{ scanReport.summary.dangerous }} dangerous</span>
1234-
<span v-if="scanReport.summary?.warnings" class="text-warning font-semibold">{{ scanReport.summary.warnings }} warnings</span>
1235-
<span v-if="scanReport.summary?.info_level" class="text-info">{{ scanReport.summary.info_level }} info</span>
1236-
<span v-if="scanReport.summary?.total === 0" class="text-success font-semibold">No findings</span>
1233+
<span v-if="scanThreatCounts.dangerous" class="text-error font-semibold">{{ scanThreatCounts.dangerous }} dangerous</span>
1234+
<span v-if="scanThreatCounts.warnings" class="text-warning font-semibold">{{ scanThreatCounts.warnings }} warnings</span>
1235+
<span v-if="scanThreatCounts.info" class="text-info">{{ scanThreatCounts.info }} info</span>
1236+
<span v-if="scanThreatCounts.total === 0" class="text-success font-semibold">No findings</span>
12371237
</div>
12381238
</div>
12391239

@@ -2401,19 +2401,40 @@ const approveDialogMode = ref<'no_scan' | 'critical'>('no_scan')
24012401
24022402
// Spec 077 FR-021: the approval gate blocks on baseline DANGEROUS findings only
24032403
// (hard-tier). Deep-scan findings inform but never gate. The server-side verdict
2404-
// is now tier-driven, so the modal mirrors it by counting `dangerous` (threat
2405-
// level) rather than `critical` (severity) — a soft finding can be "high"
2406-
// severity yet must not block approval.
2404+
// is tier-driven, so the modal mirrors it via the TIER-DRIVEN finding_counts —
2405+
// NOT the raw threat-level report summary, where a tierless deep-scan/external
2406+
// finding can read "dangerous" and would show the "Dangerous Findings Detected"
2407+
// dialog even though the backend gate (hard-tier only) would not block.
24072408
const dangerousFindingCount = computed(() => {
2408-
// Prefer the loaded scan report summary if available; otherwise fall back
2409-
// to finding_counts on the server's security_scan summary (if populated).
2409+
// Prefer the tier-driven counts on the loaded report, then the server's
2410+
// security_scan summary; the raw report summary is only a last-resort
2411+
// fallback for cores that predate report-level finding_counts.
24102412
const rep = scanReport.value as any
2411-
if (rep?.summary?.dangerous != null) return rep.summary.dangerous as number
2413+
if (rep?.finding_counts?.dangerous != null) return rep.finding_counts.dangerous as number
24122414
const scan = server.value?.security_scan as any
24132415
if (scan?.finding_counts?.dangerous != null) return scan.finding_counts.dangerous as number
2416+
if (rep?.summary?.dangerous != null) return rep.summary.dangerous as number
24142417
return 0
24152418
})
24162419
2420+
// Tier-driven counts for the Security-tab summary strip (Spec 077 FR-014):
2421+
// buckets findings exactly like the server list's finding_counts — a tierless
2422+
// deep-scan/external "dangerous" finding shows as a warning on both surfaces.
2423+
// Raw threat-level summary is only a fallback for pre-Spec-077 payloads.
2424+
const scanThreatCounts = computed(() => {
2425+
const rep = scanReport.value as any
2426+
const fc = rep?.finding_counts
2427+
if (fc) {
2428+
return { dangerous: fc.dangerous ?? 0, warnings: fc.warning ?? 0, info: fc.info ?? 0, total: fc.total ?? 0 }
2429+
}
2430+
return {
2431+
dangerous: rep?.summary?.dangerous ?? 0,
2432+
warnings: rep?.summary?.warnings ?? 0,
2433+
info: rep?.summary?.info_level ?? 0,
2434+
total: rep?.summary?.total ?? 0,
2435+
}
2436+
})
2437+
24172438
const hasCompletedScanForApprove = computed(() => {
24182439
if (scanReport.value) return true
24192440
return !!server.value?.security_scan?.last_scan_at

internal/security/scanner/engine.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -824,6 +824,16 @@ func AggregateReports(jobID, serverName string, reports []*ScanReport) *Aggregat
824824
agg.RiskScore = CalculateRiskScore(agg.Findings)
825825
agg.Summary = SummarizeFindings(agg.Findings)
826826

827+
// Spec 077 FR-014 verdict purity: the report-level verdict uses the SAME
828+
// tier-driven, baseline-only derivation as the server-list summary
829+
// (GetScanSummary via deriveBaselineVerdict), so the report page can never
830+
// disagree with the server verdict — a tierless deep-scan/external
831+
// "dangerous" finding never moves it. Summary above keeps the RAW
832+
// threat-level counts for transparency; verdict-bearing UI reads these.
833+
verdict, counts := deriveBaselineVerdict(agg.Findings)
834+
agg.Verdict = verdict
835+
agg.FindingCounts = &counts
836+
827837
// ScannersRun = number of successful reports
828838
agg.ScannersRun = len(reports)
829839
// ScanComplete = at least one scanner succeeded

internal/security/scanner/engine_test.go

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1315,3 +1315,80 @@ func TestDeepScanFailureLeavesBaselineVerdictUnchanged(t *testing.T) {
13151315
t.Errorf("disabled deep scan must not report failures, got %+v", baseline.DeepScan.ScannersFailed)
13161316
}
13171317
}
1318+
1319+
// TestAggregateReportsVerdictBaselineOnly locks Spec 077 FR-014 at the
1320+
// aggregated-report level (the payload the report PAGE renders): the
1321+
// report-level Verdict/FindingCounts must use the SAME tier-driven,
1322+
// baseline-only derivation as the server-list summary (GetScanSummary), so
1323+
// the report page badge can never say "dangerous" while the server list says
1324+
// "clean". Summary keeps the RAW threat-level counts for transparency; the
1325+
// verdict is what verdict-bearing UI must read.
1326+
func TestAggregateReportsVerdictBaselineOnly(t *testing.T) {
1327+
t.Run("tierless dangerous finding does not move the verdict", func(t *testing.T) {
1328+
// A deep-scan/external scanner finding carries no Tier. Rule id
1329+
// "tool-poisoning" makes ClassifyThreat assign threat_level=dangerous.
1330+
agg := AggregateReports("j1", "server-a", []*ScanReport{
1331+
{
1332+
ID: "r1", ScannerID: "trivy",
1333+
Findings: []ScanFinding{
1334+
{RuleID: "tool-poisoning", Severity: SeverityCritical, Title: "hidden instruction"},
1335+
},
1336+
},
1337+
})
1338+
if agg.Verdict != "clean" {
1339+
t.Errorf("verdict must derive solely from baseline findings (FR-014): expected 'clean', got %q", agg.Verdict)
1340+
}
1341+
if agg.FindingCounts == nil {
1342+
t.Fatal("expected FindingCounts on aggregated report")
1343+
}
1344+
if agg.FindingCounts.Dangerous != 0 {
1345+
t.Errorf("tierless findings must never count as dangerous, got %d", agg.FindingCounts.Dangerous)
1346+
}
1347+
if agg.FindingCounts.Warning != 1 {
1348+
t.Errorf("tierless dangerous finding must surface at warning prominence, got Warning=%d Info=%d",
1349+
agg.FindingCounts.Warning, agg.FindingCounts.Info)
1350+
}
1351+
// Raw threat-level counts are retained untouched for transparency.
1352+
if agg.Summary.Dangerous != 1 {
1353+
t.Errorf("raw Summary.Dangerous must keep the threat-level count, got %d", agg.Summary.Dangerous)
1354+
}
1355+
})
1356+
1357+
t.Run("hard-tier baseline finding yields dangerous", func(t *testing.T) {
1358+
agg := AggregateReports("j2", "server-a", []*ScanReport{
1359+
{
1360+
ID: "r1", ScannerID: inProcessTPAScannerID,
1361+
Findings: []ScanFinding{
1362+
{RuleID: "detect/phrase_injection", Tier: TierHard, ThreatLevel: ThreatLevelDangerous, ThreatType: "prompt_injection", Severity: SeverityCritical, Title: "injection"},
1363+
},
1364+
},
1365+
})
1366+
if agg.Verdict != "dangerous" {
1367+
t.Errorf("hard-tier baseline finding must yield 'dangerous', got %q", agg.Verdict)
1368+
}
1369+
if agg.FindingCounts == nil || agg.FindingCounts.Dangerous != 1 {
1370+
t.Errorf("expected FindingCounts.Dangerous=1, got %+v", agg.FindingCounts)
1371+
}
1372+
})
1373+
1374+
t.Run("soft-tier baseline finding yields warnings", func(t *testing.T) {
1375+
agg := AggregateReports("j3", "server-a", []*ScanReport{
1376+
{
1377+
ID: "r1", ScannerID: inProcessTPAScannerID,
1378+
Findings: []ScanFinding{
1379+
{RuleID: "detect/directive_imperative", Tier: TierSoft, ThreatLevel: ThreatLevelWarning, ThreatType: "tool_poisoning", Severity: SeverityMedium, Title: "directive"},
1380+
},
1381+
},
1382+
})
1383+
if agg.Verdict != "warnings" {
1384+
t.Errorf("soft-tier baseline finding must yield 'warnings', got %q", agg.Verdict)
1385+
}
1386+
})
1387+
1388+
t.Run("no findings yields clean", func(t *testing.T) {
1389+
agg := AggregateReports("j4", "server-a", []*ScanReport{{ID: "r1", ScannerID: "trivy"}})
1390+
if agg.Verdict != "clean" {
1391+
t.Errorf("expected 'clean' for empty findings, got %q", agg.Verdict)
1392+
}
1393+
})
1394+
}

0 commit comments

Comments
 (0)