mirror of
https://github.com/Studio-Saelix/sencho.git
synced 2026-08-08 18:05:10 +00:00
fix: keep the scan banner consistent with the deploy gate after suppression changes (#1485)
The policy verdict shown on the scan-detail banner was the snapshot computed once at scan time. When the deploy gate is set to honor suppressions, the gate re-reads current suppressions on every deploy, but the stored verdict never changed: after creating a matching suppression the banner kept claiming a violation the gate would now pass, and after deleting or expiring a suppression the banner kept reporting "ok" while the gate would block. Recompute the banner verdict against current suppressions when serving the scan detail, so it agrees with the gate across the create, update, delete, and expire lifecycle. The recompute runs only when honor-suppressions is enabled (otherwise suppressions affect neither the gate nor the verdict, and the stored snapshot is returned unchanged), is read-only, and falls back to the stored snapshot if it fails so an informational banner can never fail the scan-detail request.
This commit is contained in:
Anso
@@ -0,0 +1,160 @@
|
||||
/**
|
||||
* 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.
|
||||
*/
|
||||
import { describe, it, expect, beforeAll, afterAll, beforeEach, vi } from 'vitest';
|
||||
import request from 'supertest';
|
||||
import { setupTestDb, cleanupTestDb, loginAsTestAdmin } from './helpers/setupTestDb';
|
||||
import type { VulnerabilityScan } from '../services/DatabaseService';
|
||||
|
||||
let tmpDir: string;
|
||||
let app: import('express').Express;
|
||||
let DatabaseService: typeof import('../services/DatabaseService').DatabaseService;
|
||||
let parsePolicyEvaluation: typeof import('../services/DatabaseService').parsePolicyEvaluation;
|
||||
let FleetSyncService: typeof import('../services/FleetSyncService').FleetSyncService;
|
||||
let adminCookie: string;
|
||||
|
||||
const KEV_CVE = 'CVE-2026-8000';
|
||||
|
||||
beforeAll(async () => {
|
||||
tmpDir = await setupTestDb();
|
||||
({ DatabaseService, parsePolicyEvaluation } = await import('../services/DatabaseService'));
|
||||
({ FleetSyncService } = await import('../services/FleetSyncService'));
|
||||
|
||||
const { LicenseService } = await import('../services/LicenseService');
|
||||
vi.spyOn(LicenseService.getInstance(), 'getTier').mockReturnValue('paid');
|
||||
|
||||
({ app } = await import('../index'));
|
||||
adminCookie = await loginAsTestAdmin(app);
|
||||
});
|
||||
|
||||
afterAll(() => cleanupTestDb(tmpDir));
|
||||
|
||||
/**
|
||||
* Seed a node-1 scan of a KEV finding under a block-on-KEV policy, then store
|
||||
* the verdict computed with no suppressions in place (violated). This mirrors
|
||||
* finishScan persisting a snapshot at scan time.
|
||||
*/
|
||||
function seedViolatingScan(): number {
|
||||
const db = DatabaseService.getInstance();
|
||||
db.createScanPolicy({
|
||||
name: 'kev', node_id: null, node_identity: '', stack_pattern: 'web',
|
||||
max_severity: 'HIGH', block_on_deploy: 1, enabled: 1,
|
||||
block_on_severity: 0, block_on_kev: 1, block_on_fixable: 0, replicated_from_control: 0,
|
||||
});
|
||||
db.replaceKev([{ cve_id: KEV_CVE, date_added: '2026-01-01' }], Date.now());
|
||||
const scanId = db.createVulnerabilityScan({
|
||||
node_id: 1, image_ref: 'nginx:1.14', image_digest: null, scanned_at: Date.now(),
|
||||
total_vulnerabilities: 1, critical_count: 1, high_count: 0, medium_count: 0, low_count: 0,
|
||||
unknown_count: 0, fixable_count: 0, secret_count: 0, misconfig_count: 0, scanners_used: 'vuln',
|
||||
highest_severity: 'CRITICAL', os_info: null, trivy_version: '0.50.0', scan_duration_ms: null,
|
||||
triggered_by: 'manual', status: 'completed', error: null, stack_context: 'web',
|
||||
});
|
||||
db.insertVulnerabilityDetails(scanId, [{
|
||||
vulnerability_id: KEV_CVE, pkg_name: 'openssl', installed_version: '1.0', fixed_version: null,
|
||||
severity: 'CRITICAL', title: null, description: null, primary_url: null,
|
||||
}]);
|
||||
const scan = db.getVulnerabilityScan(scanId) as VulnerabilityScan;
|
||||
db.setScanPolicyEvaluation(scanId, db.evaluateScanAgainstPolicies(1, scan, FleetSyncService.getSelfIdentity()));
|
||||
return scanId;
|
||||
}
|
||||
|
||||
function suppress(expiresAt: number | null): number {
|
||||
return DatabaseService.getInstance().createCveSuppression({
|
||||
cve_id: KEV_CVE, pkg_name: null, image_pattern: null, reason: 'accepted', created_by: 'admin',
|
||||
created_at: Date.now(), expires_at: expiresAt, replicated_from_control: 0, status: 'accepted',
|
||||
}).id;
|
||||
}
|
||||
|
||||
async function bannerViolated(scanId: number): Promise<boolean | undefined> {
|
||||
const res = await request(app).get(`/api/security/scans/${scanId}`).set('Cookie', adminCookie);
|
||||
expect(res.status).toBe(200);
|
||||
return res.body.policy_evaluation?.violated;
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
const db = DatabaseService.getInstance();
|
||||
db.getScanPolicies().forEach((p) => db.deleteScanPolicy(p.id));
|
||||
db.getDb().prepare('DELETE FROM cve_suppressions').run();
|
||||
db.getDb().prepare('DELETE FROM vulnerability_scans').run();
|
||||
db.updateGlobalSetting('deploy_block_honor_suppressions', '1');
|
||||
});
|
||||
|
||||
describe('GET /api/security/scans/:scanId banner vs deploy gate', () => {
|
||||
it('shows the stored violation when honor-suppressions is on and nothing is suppressed', async () => {
|
||||
const scanId = seedViolatingScan();
|
||||
expect(await bannerViolated(scanId)).toBe(true);
|
||||
});
|
||||
|
||||
it('clears the banner after a matching suppression is created (the gate would now pass)', async () => {
|
||||
const scanId = seedViolatingScan();
|
||||
suppress(null);
|
||||
expect(await bannerViolated(scanId)).toBe(false);
|
||||
// The stored snapshot is untouched: the banner is recomputed at read time.
|
||||
const stored = parsePolicyEvaluation(
|
||||
DatabaseService.getInstance().getVulnerabilityScan(scanId)?.policy_evaluation,
|
||||
);
|
||||
expect(stored?.violated).toBe(true);
|
||||
});
|
||||
|
||||
it('restores the banner after the suppression is deleted (the gate would block again)', async () => {
|
||||
const scanId = seedViolatingScan();
|
||||
const id = suppress(null);
|
||||
expect(await bannerViolated(scanId)).toBe(false);
|
||||
DatabaseService.getInstance().deleteCveSuppression(id);
|
||||
expect(await bannerViolated(scanId)).toBe(true);
|
||||
});
|
||||
|
||||
it('restores the banner after a suppression is edited to no longer match the finding', async () => {
|
||||
const scanId = seedViolatingScan();
|
||||
const id = suppress(null);
|
||||
expect(await bannerViolated(scanId)).toBe(false);
|
||||
// Narrow the suppression onto a different image so it stops covering this scan.
|
||||
DatabaseService.getInstance().updateCveSuppression(id, { image_pattern: 'other:*' });
|
||||
expect(await bannerViolated(scanId)).toBe(true);
|
||||
});
|
||||
|
||||
it('ignores an expired suppression so the banner matches the gate', async () => {
|
||||
const scanId = seedViolatingScan();
|
||||
suppress(Date.now() - 1000); // already expired
|
||||
expect(await bannerViolated(scanId)).toBe(true);
|
||||
});
|
||||
|
||||
it('clears a stale violation when no policy matches at read time (e.g. the policy was disabled)', async () => {
|
||||
const scanId = seedViolatingScan();
|
||||
// Disable the policy after the snapshot was stored: getMatchingPolicy no longer
|
||||
// returns it, so the recompute yields no verdict and the banner must clear.
|
||||
const db = DatabaseService.getInstance();
|
||||
db.getScanPolicies().forEach((p) => db.updateScanPolicy(p.id, { enabled: 0 }));
|
||||
expect(await bannerViolated(scanId)).toBeFalsy();
|
||||
});
|
||||
|
||||
it('falls back to the stored snapshot (HTTP 200) when the recompute throws', async () => {
|
||||
const scanId = seedViolatingScan();
|
||||
suppress(null); // recompute would clear it, but the recompute is made to throw
|
||||
const spy = vi
|
||||
.spyOn(DatabaseService.getInstance(), 'evaluateScanAgainstPolicies')
|
||||
.mockImplementationOnce(() => { throw new Error('boom'); });
|
||||
try {
|
||||
expect(await bannerViolated(scanId)).toBe(true); // stored snapshot, not a 500
|
||||
} finally {
|
||||
spy.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
it('uses the stored snapshot verbatim when honor-suppressions is off', 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.
|
||||
expect(await bannerViolated(scanId)).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -114,11 +114,49 @@ function parseScannersInput(raw: unknown): readonly ('vuln' | 'secret')[] | unde
|
||||
return Array.from(out) as readonly ('vuln' | 'secret')[];
|
||||
}
|
||||
|
||||
function shapeScanForResponse(scan: VulnerabilityScan): Omit<VulnerabilityScan, 'policy_evaluation'> & {
|
||||
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'> & {
|
||||
policy_evaluation: ReturnType<typeof parsePolicyEvaluation>;
|
||||
} {
|
||||
const { policy_evaluation, ...rest } = scan;
|
||||
return { ...rest, policy_evaluation: parsePolicyEvaluation(policy_evaluation) };
|
||||
return {
|
||||
...rest,
|
||||
policy_evaluation: evaluationOverride !== undefined ? evaluationOverride : 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.
|
||||
*/
|
||||
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) {
|
||||
// The banner is informational and the deploy gate is authoritative, so a
|
||||
// recompute failure must degrade to the stored snapshot rather than 500 the
|
||||
// scan detail. The recompute is synchronous DB reads that should not throw,
|
||||
// so surface it at error level if it ever does.
|
||||
console.error('[Security] banner re-evaluation failed for scanId=%s node=%s:', scan.id, scan.node_id, getErrorMessage(err, 'unknown'));
|
||||
return parsePolicyEvaluation(scan.policy_evaluation);
|
||||
}
|
||||
}
|
||||
|
||||
// A completed scan whose latest run is older than this is considered "stale" in
|
||||
@@ -548,7 +586,7 @@ securityRouter.get('/scans', authMiddleware, (req: Request, res: Response) => {
|
||||
limit,
|
||||
offset,
|
||||
});
|
||||
res.json({ ...result, items: result.items.map(shapeScanForResponse) });
|
||||
res.json({ ...result, items: result.items.map((scan) => shapeScanForResponse(scan)) });
|
||||
} catch (error) {
|
||||
console.error('[Security] Failed to list scans:', error);
|
||||
res.status(500).json({ error: 'Failed to list scans' });
|
||||
@@ -574,7 +612,7 @@ securityRouter.get('/scans/:scanId', authMiddleware, (req: Request, res: Respons
|
||||
}).filter(Boolean),
|
||||
);
|
||||
const publiclyExposed = exposedMap.get(scan.image_ref) ?? null;
|
||||
res.json({ ...shapeScanForResponse(scan), publicly_exposed: publiclyExposed });
|
||||
res.json({ ...shapeScanForResponse(scan, resolveBannerEvaluation(db, scan)), publicly_exposed: publiclyExposed });
|
||||
});
|
||||
|
||||
securityRouter.get(
|
||||
|
||||
Reference in New Issue
Block a user