Audit-hardening pass for secret and misconfiguration scanning (#977)

* fix(security): dedupe concurrent compose-stack scans Track stack scans in scanningImages keyed stack:<nodeId>:<stackName>. The /scan/stack route returns 409 when an in-flight scan exists, and the service-side check is the real correctness barrier (the route pre-check is a fast-path optimization that mirrors scanImage). The dedup key release lives in a try/finally so failed scans free the slot for retry. Why: scanComposeStack had no equivalent of scanImage's scanningImages guard, so two simultaneous calls for the same stack would both run trivy config, both insert a vulnerability_scans row, and double- process the result. * feat(security): acknowledge misconfig findings Adds a parallel acknowledgement system for Trivy misconfig findings that mirrors cve_suppressions: a new misconfig_acknowledgements table, read-time enrichment via the new misconfig-ack-filter utility, REST CRUD endpoints, fleet-sync replication from control to replicas, a Settings panel, and an Acknowledge button on the Misconfigs tab. Schema and behavior parity with cve_suppressions: - UNIQUE(rule_id, COALESCE(stack_pattern, '')) so fleet-wide acks collide as expected - blockIfReplica on every write - Audit-log entries name the scope (rule_id, stack_pattern) but never the reason text - replicated_from_control flag controls UI delete affordance and drives clearReplicatedRows on demote/reanchor - Validators reused: validateStackPatternForRedos for glob safety, sanitizeForLog for log fragments SARIF export emits an external/accepted suppression entry per acknowledged misconfig, matching the CVE pattern. Per-row Acknowledge dialog prefills stack_pattern with the scan's stack_context so the default scope is "rule + this stack only" and an operator must broaden explicitly. Tests: misconfig-ack-filter (15) and misconfig-ack-routes (23) including the duplicate-409 case for both pinned and fleet-wide acks. * fix(security): reap orphaned trivy tmp dirs at startup When the buildEnv path writes a per-scan DOCKER_CONFIG dir under os.tmpdir() and the process crashes before the finally block runs, the dir leaks. Mirrors GitSourceService.sweepStaleTempDirs: exported sweepStaleTrivyTempDirs is fire-and-forget at boot, removes prefix-matching dirs older than 1 hour, swallows permission/race failures, logs a single line if any were reaped. * perf(security): emit per-batch summary for scanAllNodeImages Adds one diag() line at the end of scanAllNodeImages summarising unique image count, scanned, skipped, failed, violation count, and elapsed time. Per-image diag inside scanImage stays useful for debugging individual scans; the summary gives operators a single fleet-level checkpoint when developer_mode is on. * perf(security): cap SARIF export at 5000 findings per type Replace the unbounded fetchAllPages walk on /scans/:id/sarif with a hard limit of 5000 findings per type. When any type trips the cap, emit run-level properties.truncated=true plus row_limit and per-type totals so downstream tooling can flag the export as partial. Console-warns for ops visibility. A scan with 50k vulns previously streamed every row into memory before serialising; the cap bounds memory and serialisation time at the cost of completeness on pathological scans. * docs(env): document TRIVY_BIN host-binary override The env var is honored by TrivyService.detectTrivy as a fallback when no managed install is present, but it was undocumented in .env.example. Adds the var with a comment explaining precedence (managed > TRIVY_BIN > PATH). * test(security): cover scanComposeStack failure modes Two new cases drive the existing try/catch through real failure paths: - Malformed Trivy stdout: row flips to status='failed' with the parser error preserved on `error`. - execFile rejection: row flips to status='failed' with a string error message. Pairs with the existing dedup tests so the failure path now also verifies the scan row state, not just the thrown exception. * test(e2e): security scanner + misconfig acknowledgement flow Seven Playwright tests covering the scanner UI and the new acknowledgement system end-to-end: - Trivy availability gate (skips suite when binary absent so CI without Trivy can opt out via E2E_SKIP_TRIVY=1) - Stack config scan completes and records misconfig findings - Concurrent stack scan returns 409 from the dedup gate - Misconfig ack POST creates and lists on Settings - Duplicate (rule_id, stack_pattern) returns 409 - Malformed rule_id (shell metacharacters) returns 400 - Misconfigs tab renders against a real stack scan Tests drive the API for behaviour assertions and the UI only for shell-rendering checks; the visual snapshot suite owns screenshots. * docs(features): add misconfig acknowledgement workflow and SARIF cap Refreshes vulnerability-scanning.mdx with: - Misconfig acknowledgements section covering the per-row dialog, Settings panel, scope/matching rules, and SARIF emission - Tier table row for the new feature - SARIF section note on the 5000 row-per-type cap and the properties.truncated marker for partial exports - Troubleshooting entries: SARIF cap, hidden Acknowledge button, findings resurfacing after delete, Trivy DB phone-home, and 409 on concurrent compose-stack scans * fix(ci): clear backend lint and CodeQL alerts - Remove the dead fetchAllPages helper in routes/security.ts. It lost its callers when the SARIF endpoint switched to direct paged reads for the truncation cap. ESLint flagged it as unused. - Switch the trivy-tmp-cleanup test helper to fs.mkdtempSync. Building paths under os.tmpdir() with predictable names tripped CodeQL's js/insecure-temporary-file rule (high severity), which warns about symlink-pre-creation attacks even in test code. mkdtempSync appends a process-random suffix and creates the dir atomically; the sencho-trivy- prefix is preserved so the production sweep still matches the test fixtures.
2026-08-18 06:23:18 +00:00 · 2026-05-07 19:23:11 -04:00
parent 4b1de35dda
commit 3b650523c1
20 changed files with 2376 additions and 162 deletions
@@ -93,6 +93,24 @@ function validateCveSuppressionRow(row: unknown): string | null {
  return null;
 }

+function validateMisconfigAcknowledgementRow(row: unknown): string | null {
+  if (!row || typeof row !== 'object') return 'row must be an object';
+  const r = row as Record<string, unknown>;
+  if (typeof r.rule_id !== 'string' || r.rule_id.length === 0 || r.rule_id.length > 200) return 'rule_id must be a non-empty string up to 200 chars';
+  if (r.stack_pattern !== null && typeof r.stack_pattern !== 'string') return 'stack_pattern must be a string or null';
+  if (typeof r.stack_pattern === 'string') {
+    if (r.stack_pattern.length > 300) return 'stack_pattern is too long';
+    const patternError = validateStackPatternForRedos(r.stack_pattern);
+    if (patternError) return patternError;
+  }
+  if (typeof r.reason !== 'string') return 'reason must be a string';
+  if (r.reason.length > 2000) return 'reason is too long';
+  if (typeof r.created_by !== 'string' || r.created_by.length > 200) return 'created_by must be a string';
+  if (typeof r.created_at !== 'number') return 'created_at must be a number';
+  if (r.expires_at !== null && typeof r.expires_at !== 'number') return 'expires_at must be a number or null';
+  return null;
+}
+
 interface FleetNodeOverview {
  id: number;
  name: string;
@@ -327,7 +345,11 @@ fleetRouter.get('/role', authMiddleware, (req: Request, res: Response): void =>
 fleetRouter.post('/sync/:resource', authMiddleware, (req: Request, res: Response): void => {
  if (!requireNodeProxy(req, res)) return;
  const resource = req.params.resource;
-  if (resource !== 'scan_policies' && resource !== 'cve_suppressions') {
+  if (
+    resource !== 'scan_policies'
+    && resource !== 'cve_suppressions'
+    && resource !== 'misconfig_acknowledgements'
+  ) {
    res.status(400).json({ error: `Unsupported sync resource: ${resource}` });
    return;
  }
@@ -353,7 +375,12 @@ fleetRouter.post('/sync/:resource', authMiddleware, (req: Request, res: Response
    res.status(413).json({ error: `Too many rows (max ${MAX_SYNC_ROWS})` });
    return;
  }
-  const validator = resource === 'scan_policies' ? validateScanPolicyRow : validateCveSuppressionRow;
+  const validator =
+    resource === 'scan_policies'
+      ? validateScanPolicyRow
+      : resource === 'cve_suppressions'
+        ? validateCveSuppressionRow
+        : validateMisconfigAcknowledgementRow;
  for (let i = 0; i < rows.length; i++) {
    const err = validator(rows[i]);
    if (err) {
@@ -9,6 +9,7 @@ import { FleetSyncService } from '../services/FleetSyncService';
 import { LicenseService } from '../services/LicenseService';
 import { validateImageRef } from '../utils/image-ref';
 import { applySuppressions } from '../utils/suppression-filter';
+import { applyMisconfigAcknowledgements } from '../utils/misconfig-ack-filter';
 import { generateSarif } from '../services/SarifExporter';
 import { sanitizeForLog } from '../utils/safeLog';
 import { getErrorMessage } from '../utils/errors';
@@ -18,6 +19,10 @@ import { validateStackPatternForRedos } from './fleet';
 import { FINDING_SEVERITIES, POLICY_SEVERITIES } from '../utils/severity';

 const CVE_ID_RE = /^(CVE-\d{4}-\d{4,}|GHSA-[\w-]{14,})$/;
+// Trivy emits misconfig rule ids in two shapes that Sencho persists verbatim:
+// short alpha-numeric codes (e.g. "DS002") and the AVD-prefixed long form
+// (e.g. "AVD-DS-0002"). Allow either, plus underscores for forward-compat.
+const MISCONFIG_RULE_RE = /^[A-Z0-9][A-Z0-9_-]{0,199}$/i;

 // Strip control characters and cap length so an operator-supplied pkg or image
 // pattern cannot inject a fake audit row by smuggling a newline plus a forged
@@ -41,9 +46,18 @@ function describeSuppressionScope(s: { cve_id: string; pkg_name: string | null;
  return pinned.length > 0 ? `${s.cve_id} (${pinned.join(', ')})` : s.cve_id;
 }

-function recordSuppressionAudit(
+// Misconfig ack scope summary mirrors the suppression variant. Reason is
+// elided on purpose; rule_id and stack_pattern are non-sensitive.
+function describeAckScope(a: { rule_id: string; stack_pattern: string | null }): string {
+  const pinned: string[] = [];
+  if (a.stack_pattern) pinned.push(`stack=${sanitiseScopeFragment(a.stack_pattern, 300)}`);
+  return pinned.length > 0 ? `${a.rule_id} (${pinned.join(', ')})` : a.rule_id;
+}
+
+function recordSecurityAudit(
  req: Request,
  res: Response,
+  prefix: 'cve_suppression' | 'misconfig_ack',
  action: 'create' | 'update' | 'delete',
  summary: string,
 ): void {
@@ -56,13 +70,31 @@ function recordSuppressionAudit(
      status_code: res.statusCode,
      node_id: null,
      ip_address: req.ip || 'unknown',
-      summary: `cve_suppression.${action}: ${summary}`,
+      summary: `${prefix}.${action}: ${summary}`,
    });
  } catch (err) {
-    console.warn('[Security] Suppression audit log write failed:', getErrorMessage(err, 'unknown'));
+    console.warn('[Security] Audit log write failed:', getErrorMessage(err, 'unknown'));
  }
 }

+function recordSuppressionAudit(
+  req: Request,
+  res: Response,
+  action: 'create' | 'update' | 'delete',
+  summary: string,
+): void {
+  recordSecurityAudit(req, res, 'cve_suppression', action, summary);
+}
+
+function recordAckAudit(
+  req: Request,
+  res: Response,
+  action: 'create' | 'update' | 'delete',
+  summary: string,
+): void {
+  recordSecurityAudit(req, res, 'misconfig_ack', action, summary);
+}
+
 function parseScannersInput(raw: unknown): readonly ('vuln' | 'secret')[] | undefined | null {
  if (raw === undefined || raw === null) return undefined;
  if (!Array.isArray(raw) || raw.length === 0) return null;
@@ -81,21 +113,6 @@ function shapeScanForResponse(scan: VulnerabilityScan): Omit<VulnerabilityScan,
  return { ...rest, policy_evaluation: parsePolicyEvaluation(policy_evaluation) };
 }

-function fetchAllPages<T>(
-  q: (opts: { limit?: number; offset?: number }) => { items: T[]; total: number },
-): T[] {
-  const pageSize = 1000;
-  const collected: T[] = [];
-  let offset = 0;
-  while (true) {
-    const page = q({ limit: pageSize, offset });
-    collected.push(...page.items);
-    if (collected.length >= page.total || page.items.length === 0) break;
-    offset += page.items.length;
-  }
-  return collected;
-}
-
 export const securityRouter = Router();

 securityRouter.get('/trivy-status', authMiddleware, (_req: Request, res: Response) => {
@@ -245,6 +262,9 @@ securityRouter.post('/scan/stack', authMiddleware, async (req: Request, res: Res
  if (!stackName || !/^[a-zA-Z0-9_-]+$/.test(stackName)) {
    res.status(400).json({ error: 'Invalid stack name' }); return;
  }
+  if (svc.isScanningStack(req.nodeId, stackName)) {
+    res.status(409).json({ error: 'Already scanning this stack' }); return;
+  }
  try {
    const scan = await svc.scanComposeStack(req.nodeId, stackName, 'manual');
    res.status(201).json(scan);
@@ -253,6 +273,9 @@ securityRouter.post('/scan/stack', authMiddleware, async (req: Request, res: Res
    if (message === 'Invalid stack path' || message.startsWith('No compose file found')) {
      res.status(404).json({ error: message }); return;
    }
+    if (message === 'Already scanning this stack') {
+      res.status(409).json({ error: message }); return;
+    }
    console.error('[Security] Stack config scan failed:', error);
    res.status(500).json({ error: message || 'Failed to scan stack' });
  }
@@ -372,7 +395,10 @@ securityRouter.get(
    }
    const limit = req.query.limit ? Number(req.query.limit) : undefined;
    const offset = req.query.offset ? Number(req.query.offset) : undefined;
-    res.json(db.getMisconfigFindings(scanId, { severity, limit, offset }));
+    const result = db.getMisconfigFindings(scanId, { severity, limit, offset });
+    const acks = db.getMisconfigAcknowledgements();
+    const enriched = applyMisconfigAcknowledgements(result.items, scan.stack_context, acks);
+    res.json({ ...result, items: enriched });
  },
 );

@@ -436,11 +462,42 @@ securityRouter.get(
      res.status(409).json({ error: 'Scan not complete' }); return;
    }
    try {
-      const details = fetchAllPages((opts) => db.getVulnerabilityDetails(scanId, opts));
-      const secrets = fetchAllPages((opts) => db.getSecretFindings(scanId, opts));
-      const misconfigs = fetchAllPages((opts) => db.getMisconfigFindings(scanId, opts));
-      const suppressed = applySuppressions(details, scan.image_ref, db.getCveSuppressions());
-      const sarif = generateSarif(scan, suppressed, secrets, misconfigs);
+      // Hard cap to bound memory and serialization on pathological scans.
+      // 5000 findings per type comfortably covers realistic scans; if any
+      // type trips the cap we surface `truncated` in the SARIF metadata so
+      // tooling can flag the export as partial.
+      const SARIF_ROW_LIMIT = 5000;
+      const detailsPage = db.getVulnerabilityDetails(scanId, { limit: SARIF_ROW_LIMIT });
+      const secretsPage = db.getSecretFindings(scanId, { limit: SARIF_ROW_LIMIT });
+      const misconfigsPage = db.getMisconfigFindings(scanId, { limit: SARIF_ROW_LIMIT });
+      const truncated =
+        detailsPage.total > SARIF_ROW_LIMIT
+        || secretsPage.total > SARIF_ROW_LIMIT
+        || misconfigsPage.total > SARIF_ROW_LIMIT;
+      if (truncated) {
+        console.warn(
+          `[Security] SARIF export truncated for scanId=${scanId}: `
+          + `vulns=${detailsPage.total}, secrets=${secretsPage.total}, misconfigs=${misconfigsPage.total}, cap=${SARIF_ROW_LIMIT}`,
+        );
+      }
+      const suppressed = applySuppressions(detailsPage.items, scan.image_ref, db.getCveSuppressions());
+      const acknowledged = applyMisconfigAcknowledgements(
+        misconfigsPage.items,
+        scan.stack_context,
+        db.getMisconfigAcknowledgements(),
+      );
+      const sarif = generateSarif(scan, suppressed, secretsPage.items, acknowledged);
+      if (truncated) {
+        sarif.runs[0].properties = {
+          truncated: true,
+          row_limit: SARIF_ROW_LIMIT,
+          totals: {
+            vulnerabilities: detailsPage.total,
+            secrets: secretsPage.total,
+            misconfigs: misconfigsPage.total,
+          },
+        };
+      }
      const safeName = scan.image_ref.replace(/[^a-zA-Z0-9._-]/g, '_') || `scan-${scanId}`;
      res.setHeader('Content-Type', 'application/sarif+json');
      res.setHeader('Content-Disposition', `attachment; filename="${safeName}.sarif.json"`);
@@ -682,6 +739,146 @@ securityRouter.delete('/suppressions/:id', authMiddleware, (req: Request, res: R
  );
 });

+// --- Misconfig Acknowledgements ---
+
+securityRouter.get('/misconfig-acks', authMiddleware, (req: Request, res: Response): void => {
+  const now = Date.now();
+  const rows = DatabaseService.getInstance().getMisconfigAcknowledgements().map((a) => ({
+    ...a,
+    active: a.expires_at === null || a.expires_at > now,
+  }));
+  res.json(rows);
+});
+
+securityRouter.post('/misconfig-acks', authMiddleware, (req: Request, res: Response): void => {
+  if (!requireAdmin(req, res)) return;
+  if (blockIfReplica(res, 'misconfig acknowledgements')) return;
+  const body = req.body ?? {};
+  const ruleId = typeof body.rule_id === 'string' ? body.rule_id.trim() : '';
+  if (!MISCONFIG_RULE_RE.test(ruleId)) {
+    res.status(400).json({ error: 'rule_id must be a non-empty alpha-numeric identifier (e.g. "DS002" or "AVD-DS-0002")' });
+    return;
+  }
+  const stackPatternRaw = body.stack_pattern == null || body.stack_pattern === ''
+    ? null
+    : String(body.stack_pattern).trim();
+  if (stackPatternRaw !== null) {
+    if (stackPatternRaw.length > 300) {
+      res.status(400).json({ error: 'stack_pattern is too long' }); return;
+    }
+    const patternError = validateStackPatternForRedos(stackPatternRaw);
+    if (patternError) {
+      res.status(400).json({ error: patternError }); return;
+    }
+  }
+  const reason = typeof body.reason === 'string' ? body.reason.trim() : '';
+  if (!reason) {
+    res.status(400).json({ error: 'reason is required' }); return;
+  }
+  if (reason.length > 2000) {
+    res.status(400).json({ error: 'reason is too long' }); return;
+  }
+  const expiresAt = body.expires_at == null ? null : Number(body.expires_at);
+  if (expiresAt !== null && !Number.isFinite(expiresAt)) {
+    res.status(400).json({ error: 'expires_at must be a timestamp or null' }); return;
+  }
+  try {
+    const ack = DatabaseService.getInstance().createMisconfigAcknowledgement({
+      rule_id: ruleId,
+      stack_pattern: stackPatternRaw,
+      reason,
+      created_by: req.user?.username || 'unknown',
+      created_at: Date.now(),
+      expires_at: expiresAt,
+      replicated_from_control: 0,
+    });
+    FleetSyncService.getInstance().pushResourceAsync('misconfig_acknowledgements');
+    res.status(201).json(ack);
+    recordAckAudit(req, res, 'create', describeAckScope(ack));
+  } catch (error) {
+    const message = (error as Error).message || '';
+    if (message.includes('UNIQUE')) {
+      res.status(409).json({ error: 'An acknowledgement already exists for this rule and stack pattern.' });
+      return;
+    }
+    console.error('[Security] Failed to create misconfig acknowledgement:', error);
+    res.status(500).json({ error: 'Failed to create acknowledgement' });
+  }
+});
+
+securityRouter.put('/misconfig-acks/:id', authMiddleware, (req: Request, res: Response): void => {
+  if (!requireAdmin(req, res)) return;
+  if (blockIfReplica(res, 'misconfig acknowledgements')) return;
+  const id = Number(req.params.id);
+  if (!Number.isFinite(id)) {
+    res.status(400).json({ error: 'Invalid acknowledgement id' }); return;
+  }
+  const body = req.body ?? {};
+  const updates: Partial<{ reason: string; stack_pattern: string | null; expires_at: number | null }> = {};
+  if (body.reason !== undefined) {
+    const reason = typeof body.reason === 'string' ? body.reason.trim() : '';
+    if (!reason) { res.status(400).json({ error: 'reason is required' }); return; }
+    if (reason.length > 2000) { res.status(400).json({ error: 'reason is too long' }); return; }
+    updates.reason = reason;
+  }
+  if (body.stack_pattern !== undefined) {
+    const pattern = body.stack_pattern == null || body.stack_pattern === ''
+      ? null
+      : String(body.stack_pattern).trim();
+    if (pattern !== null) {
+      if (pattern.length > 300) {
+        res.status(400).json({ error: 'stack_pattern is too long' }); return;
+      }
+      const patternError = validateStackPatternForRedos(pattern);
+      if (patternError) {
+        res.status(400).json({ error: patternError }); return;
+      }
+    }
+    updates.stack_pattern = pattern;
+  }
+  if (body.expires_at !== undefined) {
+    const expiresAt = body.expires_at == null ? null : Number(body.expires_at);
+    if (expiresAt !== null && !Number.isFinite(expiresAt)) {
+      res.status(400).json({ error: 'expires_at must be a timestamp or null' }); return;
+    }
+    updates.expires_at = expiresAt;
+  }
+  const ack = DatabaseService.getInstance().updateMisconfigAcknowledgement(id, updates);
+  if (!ack) {
+    res.status(404).json({ error: 'Acknowledgement not found' }); return;
+  }
+  FleetSyncService.getInstance().pushResourceAsync('misconfig_acknowledgements');
+  res.json(ack);
+  const changed = Object.keys(updates);
+  recordAckAudit(
+    req,
+    res,
+    'update',
+    `id=${id} ${describeAckScope(ack)} fields=[${changed.join(',')}]`,
+  );
+});
+
+securityRouter.delete('/misconfig-acks/:id', authMiddleware, (req: Request, res: Response): void => {
+  if (!requireAdmin(req, res)) return;
+  if (blockIfReplica(res, 'misconfig acknowledgements')) return;
+  const id = Number(req.params.id);
+  if (!Number.isFinite(id)) {
+    res.status(400).json({ error: 'Invalid acknowledgement id' }); return;
+  }
+  const db = DatabaseService.getInstance();
+  // Snapshot before delete so the audit summary names the rule rather than the bare id.
+  const existing = db.getMisconfigAcknowledgement(id);
+  db.deleteMisconfigAcknowledgement(id);
+  FleetSyncService.getInstance().pushResourceAsync('misconfig_acknowledgements');
+  res.json({ success: true });
+  recordAckAudit(
+    req,
+    res,
+    'delete',
+    existing ? `id=${id} ${describeAckScope(existing)}` : `id=${id} (not found)`,
+  );
+});
+
 securityRouter.get('/compare', authMiddleware, (req: Request, res: Response): void => {
  const scanId1 = Number(req.query.scanId1);
  const scanId2 = Number(req.query.scanId2);