mirror of
https://github.com/Studio-Saelix/sencho.git
synced 2026-08-07 09:24:09 +00:00
fix: name matched risk inputs in policy scan banner and alerts (#1473)
The pre-deploy gate names the inputs that matched a scan policy (a known-exploited CVE, a fixable Critical/High, or a severity threshold), but the informational post-scan surfaces still framed every violation as a severity ceiling. The scan detail banner read "blocks severities at or above X, highest severity is Y" and the scheduled-scan alert read "<severity> exceeds <maxSeverity>", which is wrong for a KEV- or fixable-only policy that never gated on severity. Persist the matched reasons on the policy evaluation, carry them on the scheduled-scan violation, and render them on the banner so every policy surface names the input that actually matched. Evaluations persisted before this change carry no reasons: the parser defaults the field to an empty array and the banner falls back to a plain violation notice.
This commit is contained in:
@@ -10,10 +10,11 @@ import type { VulnerabilityScan, VulnerabilityDetail } from '../services/Databas
|
||||
|
||||
let tmpDir: string;
|
||||
let DatabaseService: typeof import('../services/DatabaseService').DatabaseService;
|
||||
let parsePolicyEvaluation: typeof import('../services/DatabaseService').parsePolicyEvaluation;
|
||||
|
||||
beforeAll(async () => {
|
||||
tmpDir = await setupTestDb();
|
||||
({ DatabaseService } = await import('../services/DatabaseService'));
|
||||
({ DatabaseService, parsePolicyEvaluation } = await import('../services/DatabaseService'));
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
@@ -126,4 +127,39 @@ describe('evaluateScanAgainstPolicies risk inputs', () => {
|
||||
const cleanScan = seedScan('web', 'CRITICAL', [detail({ vulnerability_id: 'CVE-2026-9200', severity: 'CRITICAL' })]);
|
||||
expect(db.evaluateScanAgainstPolicies(1, cleanScan, '')!.violated).toBe(false);
|
||||
});
|
||||
|
||||
it('records the matched inputs as reasons so the banner can name them', () => {
|
||||
const db = DatabaseService.getInstance();
|
||||
db.createScanPolicy({ ...basePolicy, stack_pattern: 'web', block_on_severity: 0, block_on_kev: 1, block_on_fixable: 1 });
|
||||
db.replaceKev([{ cve_id: 'CVE-2026-9100', date_added: '2026-01-01' }], Date.now());
|
||||
const scan = seedScan('web', 'CRITICAL', [
|
||||
detail({ vulnerability_id: 'CVE-2026-9100', severity: 'CRITICAL', fixed_version: '2.0' }),
|
||||
]);
|
||||
expect(db.evaluateScanAgainstPolicies(1, scan, '')!.reasons).toEqual(['kev', 'fixable']);
|
||||
});
|
||||
|
||||
it('records ["severity"] for a severity-only violation and [] when within limits', () => {
|
||||
const db = DatabaseService.getInstance();
|
||||
db.createScanPolicy({ ...basePolicy, stack_pattern: 'web', max_severity: 'HIGH' });
|
||||
expect(db.evaluateScanAgainstPolicies(1, seedScan('web', 'CRITICAL', []), '')!.reasons).toEqual(['severity']);
|
||||
expect(db.evaluateScanAgainstPolicies(1, seedScan('web', 'LOW', []), '')!.reasons).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('parsePolicyEvaluation', () => {
|
||||
const base = { policyId: 1, policyName: 'p', maxSeverity: 'HIGH', violated: true };
|
||||
|
||||
it('defaults reasons to [] for rows persisted before reason tracking', () => {
|
||||
expect(parsePolicyEvaluation(JSON.stringify(base))).toMatchObject({ violated: true, reasons: [] });
|
||||
});
|
||||
|
||||
it('drops reason values outside the known set', () => {
|
||||
const tampered = JSON.stringify({ ...base, reasons: ['kev', 'banana', 'fixable'] });
|
||||
expect(parsePolicyEvaluation(tampered)!.reasons).toEqual(['kev', 'fixable']);
|
||||
});
|
||||
|
||||
it('returns null for null or malformed input', () => {
|
||||
expect(parsePolicyEvaluation(null)).toBeNull();
|
||||
expect(parsePolicyEvaluation('not json')).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -118,6 +118,7 @@ function summaryWith(violations: Array<{
|
||||
maxSeverity: string;
|
||||
severity: string;
|
||||
scanId: number;
|
||||
reasons: Array<'severity' | 'kev' | 'fixable'>;
|
||||
}>) {
|
||||
return {
|
||||
scanned: violations.length,
|
||||
@@ -142,8 +143,9 @@ describe('SchedulerService - scheduled scan policy alerts', () => {
|
||||
policyId: 1,
|
||||
policyName: 'prod-high-gate',
|
||||
maxSeverity: 'HIGH',
|
||||
severity: 'CRITICAL',
|
||||
severity: 'MEDIUM',
|
||||
scanId: 42,
|
||||
reasons: ['kev'],
|
||||
},
|
||||
{
|
||||
imageRef: 'redis:6',
|
||||
@@ -152,6 +154,7 @@ describe('SchedulerService - scheduled scan policy alerts', () => {
|
||||
maxSeverity: 'HIGH',
|
||||
severity: 'HIGH',
|
||||
scanId: 43,
|
||||
reasons: ['severity'],
|
||||
},
|
||||
]));
|
||||
|
||||
@@ -160,11 +163,15 @@ describe('SchedulerService - scheduled scan policy alerts', () => {
|
||||
|
||||
const warningCalls = mockDispatchAlert.mock.calls.filter((c) => c[0] === 'warning');
|
||||
expect(warningCalls).toHaveLength(2);
|
||||
// The first violation matched KEV, so the alert names the known-exploited
|
||||
// input and never the severity ceiling it did not enforce.
|
||||
expect(warningCalls[0][2]).toContain('prod-high-gate');
|
||||
expect(warningCalls[0][2]).toContain('nginx:1.14');
|
||||
expect(warningCalls[0][2]).toContain('CRITICAL');
|
||||
expect(warningCalls[0][2]).toContain('HIGH');
|
||||
expect(warningCalls[0][2]).toContain('known-exploited CVE (KEV)');
|
||||
expect(warningCalls[0][2]).not.toContain('HIGH');
|
||||
// The second matched the severity threshold.
|
||||
expect(warningCalls[1][2]).toContain('redis:6');
|
||||
expect(warningCalls[1][2]).toContain('severity threshold');
|
||||
});
|
||||
|
||||
it('does not dispatch any policy alert when no violations occur', async () => {
|
||||
@@ -194,6 +201,7 @@ describe('SchedulerService - scheduled scan policy alerts', () => {
|
||||
maxSeverity: 'CRITICAL',
|
||||
severity: 'CRITICAL',
|
||||
scanId: 44,
|
||||
reasons: ['severity'],
|
||||
},
|
||||
]));
|
||||
|
||||
|
||||
@@ -3,7 +3,7 @@ import path from 'path';
|
||||
import fs from 'fs';
|
||||
import { CryptoService } from './CryptoService';
|
||||
import { isSeverityAtLeast } from '../utils/severity';
|
||||
import { evaluatePolicyRisk, policyInputs } from '../utils/policy-risk';
|
||||
import { evaluatePolicyRisk, policyInputs, type PolicyBlockReason } from '../utils/policy-risk';
|
||||
import type { AuditStatsInput } from './AuditAnomalyService';
|
||||
import { EXPOSURE_INTENTS, type ExposureIntent } from './network/types';
|
||||
import type { BackendScheduledAction } from './scheduledActionRegistry';
|
||||
@@ -601,6 +601,10 @@ export interface PolicyEvaluation {
|
||||
policyId: number;
|
||||
policyName: string;
|
||||
maxSeverity: VulnSeverity;
|
||||
// Inputs that actually matched (severity / kev / fixable). Empty when not
|
||||
// violated. Lets the scan banner name the reason instead of always citing a
|
||||
// severity threshold the policy may not have gated on.
|
||||
reasons: PolicyBlockReason[];
|
||||
violated: boolean;
|
||||
evaluatedAt: number;
|
||||
}
|
||||
@@ -640,6 +644,8 @@ export interface VulnerabilityScan {
|
||||
// a clean vuln read can never be a secret-only or config scan in disguise.
|
||||
export const VULN_BEARING_SCANNER_SETS = ['vuln', 'vuln,secret'] as const;
|
||||
|
||||
const VALID_BLOCK_REASONS: ReadonlySet<PolicyBlockReason> = new Set(['severity', 'kev', 'fixable']);
|
||||
|
||||
export function parsePolicyEvaluation(raw: string | null | undefined): PolicyEvaluation | null {
|
||||
if (!raw) return null;
|
||||
try {
|
||||
@@ -647,6 +653,12 @@ export function parsePolicyEvaluation(raw: string | null | undefined): PolicyEva
|
||||
if (typeof parsed.policyId !== 'number' || typeof parsed.policyName !== 'string') {
|
||||
return null;
|
||||
}
|
||||
// Rows persisted before reason tracking lack `reasons`; default to empty
|
||||
// and keep only the known inputs so a stray stored value cannot reach the
|
||||
// banner or alert text.
|
||||
parsed.reasons = Array.isArray(parsed.reasons)
|
||||
? parsed.reasons.filter((r) => VALID_BLOCK_REASONS.has(r))
|
||||
: [];
|
||||
return parsed;
|
||||
} catch {
|
||||
return null;
|
||||
@@ -5527,24 +5539,25 @@ export class DatabaseService {
|
||||
const policy = this.getMatchingPolicy(nodeId, scan.stack_context, selfIdentity);
|
||||
if (!policy) return null;
|
||||
const inputs = policyInputs(policy);
|
||||
let violated: boolean;
|
||||
let reasons: PolicyBlockReason[];
|
||||
if (!inputs.blockOnKev && !inputs.blockOnFixable) {
|
||||
// Severity-only banner from the stored aggregate: no per-finding read.
|
||||
violated = inputs.blockOnSeverity && isSeverityAtLeast(scan.highest_severity, policy.max_severity);
|
||||
const severityHit = inputs.blockOnSeverity && isSeverityAtLeast(scan.highest_severity, policy.max_severity);
|
||||
reasons = severityHit ? ['severity'] : [];
|
||||
} else {
|
||||
// Best-effort banner: scores the raw findings without honoring
|
||||
// suppressions or the truncation fail-closed rule. The pre-deploy gate
|
||||
// is authoritative; this only drives the informational scan banner.
|
||||
const findings = this.getAllVulnerabilityDetails(scan.id);
|
||||
const intel = inputs.blockOnKev ? this.getCveIntel(findings.map((f) => f.vulnerability_id)) : null;
|
||||
const outcome = evaluatePolicyRisk(findings, (cveId) => intel?.get(cveId)?.kev === true, inputs);
|
||||
violated = outcome.reasons.length > 0;
|
||||
reasons = evaluatePolicyRisk(findings, (cveId) => intel?.get(cveId)?.kev === true, inputs).reasons;
|
||||
}
|
||||
return {
|
||||
policyId: policy.id,
|
||||
policyName: policy.name,
|
||||
maxSeverity: policy.max_severity,
|
||||
violated,
|
||||
reasons,
|
||||
violated: reasons.length > 0,
|
||||
evaluatedAt: Date.now(),
|
||||
};
|
||||
}
|
||||
|
||||
@@ -954,7 +954,7 @@ export class SchedulerService {
|
||||
NotificationService.getInstance().dispatchAlert(
|
||||
'warning',
|
||||
'scan_finding',
|
||||
`Policy "${v.policyName}" violated by ${v.imageRef}: ${v.severity} exceeds ${v.maxSeverity}`,
|
||||
`Policy "${v.policyName}" violated by ${v.imageRef}: matched ${summarizeBlockReasons([v])}`,
|
||||
{ actor: 'system:scheduler' },
|
||||
);
|
||||
}
|
||||
|
||||
@@ -9,6 +9,7 @@ import {
|
||||
VulnSeverity,
|
||||
VulnScanTrigger,
|
||||
VulnerabilityScan,
|
||||
parsePolicyEvaluation,
|
||||
} from './DatabaseService';
|
||||
import { FileSystemService } from './FileSystemService';
|
||||
import { RegistryService } from './RegistryService';
|
||||
@@ -19,6 +20,7 @@ import { FleetSyncService } from './FleetSyncService';
|
||||
import { getErrorMessage } from '../utils/errors';
|
||||
import { isDebugEnabled } from '../utils/debug';
|
||||
import { SEVERITY_ORDER } from '../utils/severity';
|
||||
import type { PolicyBlockReason } from '../utils/policy-risk';
|
||||
|
||||
const execFileAsync = promisify(execFile);
|
||||
|
||||
@@ -138,6 +140,9 @@ export interface ScanAllNodeImagesViolation {
|
||||
severity: VulnSeverity;
|
||||
policyName: string;
|
||||
maxSeverity: VulnSeverity;
|
||||
// Inputs that matched (severity / kev / fixable), so the scheduled-scan
|
||||
// alert names the reason rather than always citing a severity threshold.
|
||||
reasons: PolicyBlockReason[];
|
||||
}
|
||||
|
||||
export interface ScanAllNodeImagesResult {
|
||||
@@ -1189,24 +1194,20 @@ class TrivyService {
|
||||
};
|
||||
|
||||
const collectViolation = (row: VulnerabilityScan | null): void => {
|
||||
if (!row || !row.policy_evaluation) return;
|
||||
try {
|
||||
const parsed = JSON.parse(row.policy_evaluation) as {
|
||||
violated: boolean;
|
||||
policyName: string;
|
||||
maxSeverity: VulnSeverity;
|
||||
};
|
||||
if (parsed.violated) {
|
||||
violations.push({
|
||||
imageRef: row.image_ref,
|
||||
scanId: row.id,
|
||||
severity: row.highest_severity ?? 'UNKNOWN',
|
||||
policyName: parsed.policyName,
|
||||
maxSeverity: parsed.maxSeverity,
|
||||
});
|
||||
}
|
||||
} catch {
|
||||
// Ignore malformed evaluation JSON; presence is informational.
|
||||
if (!row) return;
|
||||
// Shared parser tolerates malformed JSON (returns null) and normalizes
|
||||
// reasons, so the scheduled-scan alert names the same validated inputs
|
||||
// as the banner.
|
||||
const parsed = parsePolicyEvaluation(row.policy_evaluation);
|
||||
if (parsed?.violated) {
|
||||
violations.push({
|
||||
imageRef: row.image_ref,
|
||||
scanId: row.id,
|
||||
severity: row.highest_severity ?? 'UNKNOWN',
|
||||
policyName: parsed.policyName,
|
||||
maxSeverity: parsed.maxSeverity,
|
||||
reasons: parsed.reasons,
|
||||
});
|
||||
}
|
||||
};
|
||||
|
||||
|
||||
@@ -47,6 +47,7 @@ import { cn } from '@/lib/utils';
|
||||
import { cveUrl } from '@/lib/cveUrl';
|
||||
import { SEVERITY_ROW_TINT } from '@/lib/severityStyles';
|
||||
import { formatTimeAgo } from '@/lib/relativeTime';
|
||||
import { formatPolicyReasons } from '@/lib/policyReasons';
|
||||
import type {
|
||||
VulnerabilityScan,
|
||||
VulnerabilityDetail,
|
||||
@@ -629,16 +630,13 @@ export function VulnerabilityScanSheet({
|
||||
Policy violation
|
||||
</div>
|
||||
<div className="text-sm text-stat-value mt-0.5">
|
||||
<span className="font-mono">{scan.policy_evaluation.policyName}</span> blocks
|
||||
severities at or above{' '}
|
||||
<span className="font-mono tabular-nums">
|
||||
{scan.policy_evaluation.maxSeverity}
|
||||
</span>
|
||||
. This scan's highest severity is{' '}
|
||||
<span className="font-mono tabular-nums">
|
||||
{scan.highest_severity ?? 'UNKNOWN'}
|
||||
</span>
|
||||
.
|
||||
This scan violates{' '}
|
||||
<span className="font-mono">{scan.policy_evaluation.policyName}</span>
|
||||
{scan.policy_evaluation.reasons.length > 0 ? (
|
||||
<>: matched {formatPolicyReasons(scan.policy_evaluation.reasons, scan.policy_evaluation.maxSeverity)}.</>
|
||||
) : (
|
||||
<>.</>
|
||||
)}
|
||||
</div>
|
||||
</div>
|
||||
)}
|
||||
|
||||
@@ -0,0 +1,22 @@
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { formatPolicyReasons } from './policyReasons';
|
||||
|
||||
describe('formatPolicyReasons', () => {
|
||||
it('names a KEV match without citing a severity ceiling', () => {
|
||||
expect(formatPolicyReasons(['kev'], 'HIGH')).toBe('a known-exploited CVE (KEV)');
|
||||
});
|
||||
|
||||
it('names the configured ceiling for a severity match', () => {
|
||||
expect(formatPolicyReasons(['severity'], 'CRITICAL')).toBe('severity at or above CRITICAL');
|
||||
});
|
||||
|
||||
it('joins every matched reason in order', () => {
|
||||
expect(formatPolicyReasons(['severity', 'kev', 'fixable'], 'HIGH')).toBe(
|
||||
'severity at or above HIGH, a known-exploited CVE (KEV), a fixable Critical/High',
|
||||
);
|
||||
});
|
||||
|
||||
it('returns an empty string when no reason was recorded', () => {
|
||||
expect(formatPolicyReasons([], 'HIGH')).toBe('');
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,27 @@
|
||||
import type { PolicyBlockReason, VulnSeverity } from '@/types/security';
|
||||
|
||||
/**
|
||||
* Human-readable description of the policy inputs that matched, for the scan
|
||||
* detail banner. Echoes the deploy-gate dialog's phrasing (PolicyBlockDialog)
|
||||
* so the banner and the block dialog read consistently, and names the
|
||||
* configured severity ceiling so the severity input stays specific. Returns an
|
||||
* empty string when no reason was recorded (evaluations persisted before
|
||||
* reason tracking).
|
||||
*/
|
||||
export function formatPolicyReasons(
|
||||
reasons: PolicyBlockReason[],
|
||||
maxSeverity: VulnSeverity,
|
||||
): string {
|
||||
return reasons
|
||||
.map((reason) => {
|
||||
switch (reason) {
|
||||
case 'severity':
|
||||
return `severity at or above ${maxSeverity}`;
|
||||
case 'kev':
|
||||
return 'a known-exploited CVE (KEV)';
|
||||
case 'fixable':
|
||||
return 'a fixable Critical/High';
|
||||
}
|
||||
})
|
||||
.join(', ');
|
||||
}
|
||||
@@ -4,10 +4,16 @@ export type VulnSeverity = 'CRITICAL' | 'HIGH' | 'MEDIUM' | 'LOW' | 'UNKNOWN';
|
||||
export type VulnScanStatus = 'in_progress' | 'completed' | 'failed';
|
||||
export type VulnScanTrigger = 'manual' | 'scheduled' | 'deploy' | 'deploy-preflight';
|
||||
|
||||
export type PolicyBlockReason = 'severity' | 'kev' | 'fixable';
|
||||
|
||||
export interface ScanPolicyEvaluation {
|
||||
policyId: number;
|
||||
policyName: string;
|
||||
maxSeverity: VulnSeverity;
|
||||
// Inputs that matched (severity / kev / fixable). Empty for evaluations
|
||||
// persisted before reason tracking; the banner falls back to a plain
|
||||
// violation notice in that case.
|
||||
reasons: PolicyBlockReason[];
|
||||
violated: boolean;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user