fix: always reconcile the scan banner with the current policy verdict (#1488)

The scan-detail banner only recomputed its verdict when honor-suppressions was
enabled; otherwise it returned the snapshot stored at scan time. The deploy gate
always re-evaluates current policies, so with honor-suppressions off a policy
lifecycle change drifted the banner from the gate: disabling (or editing) an
enabled policy left the banner claiming a violation the gate would now pass, and
tightening a passing policy left the banner reporting a pass the gate would block.

Recompute the banner verdict unconditionally so it agrees with the gate across
the full policy and suppression lifecycle, regardless of the honor-suppressions
setting. The recompute already reads that setting itself (so a raw or
suppression-filtered verdict is chosen correctly), returns no verdict when no
policy matches (clearing the banner to match a passing gate), stays read-only,
and still falls back to the stored snapshot if it throws.
This commit is contained in:
Anso
2026-06-26 23:35:57 -04:00
committed by GitHub
parent 3386c63152
commit 3343630171
2 changed files with 66 additions and 32 deletions
@@ -1,14 +1,13 @@
/**
* GET /api/security/scans/:scanId banner consistency with the deploy gate.
*
* The verdict stored on a scan at scan time is a one-time snapshot. Once the
* deploy gate is set to honor suppressions, the gate re-reads current
* suppressions on every deploy while the stored verdict does not, so creating,
* deleting, or expiring a suppression used to leave the scan-detail banner
* disagreeing with what the gate would actually do. The detail route now
* recomputes the banner verdict against current suppressions; these tests pin
* that the banner tracks the gate across the suppression lifecycle, and that the
* stored snapshot is still used verbatim when honor-suppressions is off.
* The verdict stored on a scan at scan time is a one-time snapshot, while the
* deploy gate always re-evaluates current policies and suppressions. The detail
* route therefore recomputes the banner verdict on every read so it tracks the
* gate across the full lifecycle: creating, deleting, editing, or expiring a
* suppression, and enabling, disabling, or tightening a policy, with
* honor-suppressions both on and off. These tests pin that agreement; the stored
* snapshot is used only as a fallback if the recompute throws.
*/
import { describe, it, expect, beforeAll, afterAll, beforeEach, vi } from 'vitest';
import request from 'supertest';
@@ -150,11 +149,51 @@ describe('GET /api/security/scans/:scanId banner vs deploy gate', () => {
}
});
it('uses the stored snapshot verbatim when honor-suppressions is off', async () => {
it('recomputes the raw verdict when honor-suppressions is off, ignoring suppressions like the gate', async () => {
const scanId = seedViolatingScan();
suppress(null);
DatabaseService.getInstance().updateGlobalSetting('deploy_block_honor_suppressions', '0');
// Setting off: the gate ignores suppressions too, so the raw stored verdict stands.
// Honor off: the gate ignores suppressions, so the recomputed banner still
// flags the KEV (matching the raw gate), rather than honoring the suppression.
expect(await bannerViolated(scanId)).toBe(true);
});
it('clears the banner when the policy is disabled, even with honor-suppressions off', async () => {
const scanId = seedViolatingScan();
const db = DatabaseService.getInstance();
db.updateGlobalSetting('deploy_block_honor_suppressions', '0');
// The stored snapshot says violated, but a disabled policy makes the gate pass.
// The banner must recompute (to no verdict), not echo the stale snapshot.
db.getScanPolicies().forEach((p) => db.updateScanPolicy(p.id, { enabled: 0 }));
expect(await bannerViolated(scanId)).toBeFalsy();
// The stored snapshot is unchanged, proving the banner was recomputed at read time.
expect(parsePolicyEvaluation(db.getVulnerabilityScan(scanId)?.policy_evaluation)?.violated).toBe(true);
// Re-enabling the policy brings the violation back, matching the gate again.
db.getScanPolicies().forEach((p) => db.updateScanPolicy(p.id, { enabled: 1 }));
expect(await bannerViolated(scanId)).toBe(true);
});
it('shows a violation after a passing policy is tightened to block, with honor-suppressions off', async () => {
const db = DatabaseService.getInstance();
db.updateGlobalSetting('deploy_block_honor_suppressions', '0');
const policy = db.createScanPolicy({
name: 'sev', node_id: null, node_identity: '', stack_pattern: 'web',
max_severity: 'CRITICAL', block_on_deploy: 1, enabled: 1,
block_on_severity: 1, block_on_kev: 0, block_on_fixable: 0, replicated_from_control: 0,
});
const scanId = db.createVulnerabilityScan({
node_id: 1, image_ref: 'web:1', image_digest: null, scanned_at: Date.now(),
total_vulnerabilities: 1, critical_count: 0, high_count: 1, medium_count: 0, low_count: 0,
unknown_count: 0, fixable_count: 0, secret_count: 0, misconfig_count: 0, scanners_used: 'vuln',
highest_severity: 'HIGH', os_info: null, trivy_version: '0.50.0', scan_duration_ms: null,
triggered_by: 'manual', status: 'completed', error: null, stack_context: 'web',
});
const scan = db.getVulnerabilityScan(scanId) as VulnerabilityScan;
db.setScanPolicyEvaluation(scanId, db.evaluateScanAgainstPolicies(1, scan, FleetSyncService.getSelfIdentity()));
// HIGH is within a CRITICAL threshold, so the stored verdict passes.
expect(await bannerViolated(scanId)).toBeFalsy();
// Tightening the threshold to HIGH makes the gate block; the banner must follow.
db.updateScanPolicy(policy.id, { max_severity: 'HIGH' });
expect(await bannerViolated(scanId)).toBe(true);
});
});
+17 -22
View File
@@ -114,39 +114,28 @@ function parseScannersInput(raw: unknown): readonly ('vuln' | 'secret')[] | unde
return Array.from(out) as readonly ('vuln' | 'secret')[];
}
function shapeScanForResponse(
scan: VulnerabilityScan,
// The scan-detail endpoint passes a verdict recomputed against current
// suppressions (resolveBannerEvaluation) so the banner matches the deploy
// gate; other callers omit it and the snapshot stored at scan time is used.
evaluationOverride?: ReturnType<typeof parsePolicyEvaluation>,
): Omit<VulnerabilityScan, 'policy_evaluation'> & {
function shapeScanForResponse(scan: VulnerabilityScan): Omit<VulnerabilityScan, 'policy_evaluation'> & {
policy_evaluation: ReturnType<typeof parsePolicyEvaluation>;
} {
const { policy_evaluation, ...rest } = scan;
return {
...rest,
policy_evaluation: evaluationOverride !== undefined ? evaluationOverride : parsePolicyEvaluation(policy_evaluation),
};
return { ...rest, policy_evaluation: parsePolicyEvaluation(policy_evaluation) };
}
/**
* The policy verdict the scan-detail banner shows. The verdict stored at scan
* time drifts from the deploy gate once the gate is set to honor suppressions,
* because the gate always re-reads current suppressions while the stored value
* is a one-time snapshot: creating, editing, deleting, or expiring a suppression
* leaves the banner claiming a violation the gate would now pass, or the reverse.
* Recompute against current suppressions so the banner agrees with the gate.
* With honor-suppressions off, suppressions affect neither the gate nor the
* verdict, so the stored snapshot is authoritative and returned without a reread.
* time is a one-time snapshot, while the deploy gate always re-evaluates current
* policies and suppressions: enabling, disabling, or editing a policy (and
* creating, editing, deleting, or expiring a suppression) leaves the stored
* snapshot claiming a violation the gate would now pass, or the reverse. Always
* recompute so the banner agrees with the gate across the full policy and
* suppression lifecycle. evaluateScanAgainstPolicies reads the honor-suppressions
* setting itself, so this is correct whether or not suppressions are honored, and
* returns null when no policy matches (the banner clears, matching a passing gate).
*/
function resolveBannerEvaluation(
db: DatabaseService,
scan: VulnerabilityScan,
): ReturnType<typeof parsePolicyEvaluation> {
if (db.getGlobalSettings()['deploy_block_honor_suppressions'] !== '1') {
return parsePolicyEvaluation(scan.policy_evaluation);
}
try {
return db.evaluateScanAgainstPolicies(scan.node_id, scan, FleetSyncService.getSelfIdentity());
} catch (err) {
@@ -612,7 +601,13 @@ securityRouter.get('/scans/:scanId', authMiddleware, (req: Request, res: Respons
}).filter(Boolean),
);
const publiclyExposed = exposedMap.get(scan.image_ref) ?? null;
res.json({ ...shapeScanForResponse(scan, resolveBannerEvaluation(db, scan)), publicly_exposed: publiclyExposed });
// The banner verdict is recomputed against current policies/suppressions so it
// matches the deploy gate; the list endpoint keeps the stored snapshot.
res.json({
...shapeScanForResponse(scan),
policy_evaluation: resolveBannerEvaluation(db, scan),
publicly_exposed: publiclyExposed,
});
});
securityRouter.get(