From a5d15397853bdade8c026e187a95bacd996f319a Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Tue, 19 May 2026 17:03:00 -0400 Subject: [PATCH] fix(cloud-tests): parse all SecHub compliance formats + dedupe scan-mode literals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two issues cubic flagged on the production-deploy PR: P2 — formatSingleRequirement regex couldn't match 3 of 4 documented SecHub compliance formats. The character class `[A-Z][A-Z0-9 .]+?` excluded lowercase letters and hyphens, so NIST.800-53.r5, CIS AWS Foundations Benchmark, and AWS Foundational Security Best Practices all silently fell through to the raw-string fallback. Only PCI DSS (all uppercase + digits) matched. Effect: compliance chips for 3 of 4 standards rendered as one verbose label instead of split standard / version / control. Fix: 1. Widen the first capture group to `[A-Za-z][A-Za-z0-9 .\-]+?` so lowercase letters and hyphens are accepted. 2. Normalize `/` → ' ' before matching, so AWS FSBP's "v1.0.0/EC2.2" version-control separator works. 3. Make the version capture REQUIRED rather than optional — formats without an explicit version (NIST embeds "r5" in the standard name) cleanly fall through to the raw fallback. This avoids the awkward "unspecified" placeholder the prior code emitted. Verified against all 4 documented formats: NIST.800-53.r5 AC-2 → raw (no version separator) CIS AWS Foundations Benchmark v1.2.0 1.1 → cis 1.2.0 (1.1) PCI DSS v3.2.1 8.2.3 → pci dss 3.2.1 (8.2.3) AWS Foundational Security Best Practices v1.0.0/EC2.2 → aws fsbp 1.0.0 (EC2.2) P2 — update-scan-mode.dto.ts hardcoded the scan-mode literals ('comp_scanners', 'security_hub') in two places, drifting from the source-of-truth comment in aws-scan-mode.ts ("importers never spell the values themselves"). Added an exported const tuple AWS_SCAN_MODES, used it in the DTO for both @IsIn validator and Swagger enum. Adding a new mode now touches only one file. Tests: - 26 → 29 SecHub adapter tests (NIST verbatim, CIS / PCI / AWS FSBP each get a regression guard for the regex fix). - 6 → 9 aws-scan-mode tests (AWS_SCAN_MODES contents, default inclusion, round-trip consistency with resolveAwsScanMode). Identified by cubic. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/cloud-security/aws-scan-mode.spec.ts | 26 ++++++++++++++ apps/api/src/cloud-security/aws-scan-mode.ts | 10 ++++++ .../dto/update-scan-mode.dto.ts | 11 ++++-- .../aws/security-hub.adapter.spec.ts | 36 +++++++++++++++++-- .../providers/aws/security-hub.adapter.ts | 23 ++++++++---- 5 files changed, 93 insertions(+), 13 deletions(-) diff --git a/apps/api/src/cloud-security/aws-scan-mode.spec.ts b/apps/api/src/cloud-security/aws-scan-mode.spec.ts index 95701b206..90670baf5 100644 --- a/apps/api/src/cloud-security/aws-scan-mode.spec.ts +++ b/apps/api/src/cloud-security/aws-scan-mode.spec.ts @@ -1,4 +1,5 @@ import { + AWS_SCAN_MODES, DEFAULT_AWS_SCAN_MODE, isSecurityHubMode, resolveAwsScanMode, @@ -48,4 +49,29 @@ describe('aws-scan-mode', () => { expect(DEFAULT_AWS_SCAN_MODE).toBe('comp_scanners'); }); }); + + describe('AWS_SCAN_MODES', () => { + it('lists exactly the two known modes (source of truth for DTOs / validators)', () => { + // If a new mode is added, this test must change in lockstep with + // the AwsScanMode union — the DTO `@IsIn(AWS_SCAN_MODES)` will + // automatically pick up the new value, but reviewers should see + // this test fail as a sanity check that the list was updated. + expect([...AWS_SCAN_MODES]).toEqual(['comp_scanners', 'security_hub']); + }); + + it('contains the default mode', () => { + // Guards against future refactors that might remove the default + // from the list — would break validation for every existing + // connection that lacks an explicit mode. + expect([...AWS_SCAN_MODES]).toContain(DEFAULT_AWS_SCAN_MODE); + }); + + it('every entry passes resolveAwsScanMode round-trip (self-consistency)', () => { + // Catches drift if someone adds a mode to the array without + // updating resolveAwsScanMode. + for (const mode of AWS_SCAN_MODES) { + expect(resolveAwsScanMode(mode)).toBe(mode); + } + }); + }); }); diff --git a/apps/api/src/cloud-security/aws-scan-mode.ts b/apps/api/src/cloud-security/aws-scan-mode.ts index 390787872..3539c17a7 100644 --- a/apps/api/src/cloud-security/aws-scan-mode.ts +++ b/apps/api/src/cloud-security/aws-scan-mode.ts @@ -23,6 +23,16 @@ */ export type AwsScanMode = 'comp_scanners' | 'security_hub'; +/** Canonical list of valid scan modes. Exported so DTOs, validators, + * and tests reference ONE array instead of duplicating the string + * literals everywhere. If a new mode is added, only this file changes + * and all importers automatically pick it up — that's the + * "single source of truth" promise this module makes. */ +export const AWS_SCAN_MODES = [ + 'comp_scanners', + 'security_hub', +] as const satisfies readonly AwsScanMode[]; + /** Default behavior for AWS connections with no scan-mode set (including * every pre-feature connection that already exists in production). */ export const DEFAULT_AWS_SCAN_MODE: AwsScanMode = 'comp_scanners'; diff --git a/apps/api/src/cloud-security/dto/update-scan-mode.dto.ts b/apps/api/src/cloud-security/dto/update-scan-mode.dto.ts index fd92285ef..0b6239597 100644 --- a/apps/api/src/cloud-security/dto/update-scan-mode.dto.ts +++ b/apps/api/src/cloud-security/dto/update-scan-mode.dto.ts @@ -1,20 +1,25 @@ import { ApiProperty } from '@nestjs/swagger'; import { IsIn, IsString } from 'class-validator'; -import type { AwsScanMode } from '../aws-scan-mode'; +import { AWS_SCAN_MODES, type AwsScanMode } from '../aws-scan-mode'; /** * Request body for `PATCH /v1/cloud-security/connections/:id/scan-mode`. * * Only AWS connections accept this; the service layer validates the * connection is AWS before applying the change. + * + * The accepted values + Swagger enum both reference `AWS_SCAN_MODES` + * directly so this DTO can't drift from the source of truth — adding a + * new mode in `aws-scan-mode.ts` automatically widens what's accepted + * here. */ export class UpdateAwsScanModeDto { @ApiProperty({ description: 'Which scan engine to use for this AWS connection.', - enum: ['comp_scanners', 'security_hub'], + enum: AWS_SCAN_MODES, example: 'security_hub', }) @IsString() - @IsIn(['comp_scanners', 'security_hub']) + @IsIn([...AWS_SCAN_MODES]) mode!: AwsScanMode; } diff --git a/apps/api/src/cloud-security/providers/aws/security-hub.adapter.spec.ts b/apps/api/src/cloud-security/providers/aws/security-hub.adapter.spec.ts index e294b1d34..34512060e 100644 --- a/apps/api/src/cloud-security/providers/aws/security-hub.adapter.spec.ts +++ b/apps/api/src/cloud-security/providers/aws/security-hub.adapter.spec.ts @@ -88,12 +88,42 @@ describe('security-hub.adapter helpers', () => { expect(formatRelatedRequirements([])).toBe(''); }); - it('formats a NIST 800-53 requirement in parser-compatible form', () => { - expect(formatRelatedRequirements(['NIST.800-53.r5 AC-2'])).toMatch( - /nist.*AC-2/i, + it('emits NIST 800-53 verbatim when no explicit version separator is present', () => { + // NIST embeds the revision in the standard name ("NIST.800-53.r5"), + // so the structured `standard version (control)` regex doesn't + // match — fallback emits the raw string and the parser surfaces it + // as a single chip label. Better than fabricating a placeholder. + expect(formatRelatedRequirements(['NIST.800-53.r5 AC-2'])).toBe( + 'NIST.800-53.r5 AC-2', ); }); + it('formats CIS AWS Foundations Benchmark in parser-compatible form (regex must accept lowercase)', () => { + // Cubic P2 regression guard — the prior regex `[A-Z][A-Z0-9 .]+?` + // could not match "Foundations" / "Benchmark" because of the + // lowercase letters, so this format silently fell through. + const result = formatRelatedRequirements([ + 'CIS AWS Foundations Benchmark v1.2.0 1.1', + ]); + expect(result).toMatch(/^cis .*1\.2\.0 \(1\.1\)$/); + }); + + it('formats PCI DSS in parser-compatible form', () => { + const result = formatRelatedRequirements(['PCI DSS v3.2.1 8.2.3']); + expect(result).toBe('pci dss 3.2.1 (8.2.3)'); + }); + + it('formats AWS FSBP in parser-compatible form, handling the slash separator', () => { + // Cubic P2 regression guard — `/` between version and control is + // unique to AWS Foundational Security Best Practices. Without the + // pre-normalization, the regex `\s+` between version and control + // would never match. + const result = formatRelatedRequirements([ + 'AWS Foundational Security Best Practices v1.0.0/EC2.2', + ]); + expect(result).toMatch(/^aws fsbp 1\.0\.0 \(EC2\.2\)$/); + }); + it('joins multiple requirements with "; " so the parser splits them correctly', () => { const result = formatRelatedRequirements([ 'NIST.800-53.r5 AC-2', diff --git a/apps/api/src/cloud-security/providers/aws/security-hub.adapter.ts b/apps/api/src/cloud-security/providers/aws/security-hub.adapter.ts index 8036d3b4b..eca2bc256 100644 --- a/apps/api/src/cloud-security/providers/aws/security-hub.adapter.ts +++ b/apps/api/src/cloud-security/providers/aws/security-hub.adapter.ts @@ -292,20 +292,29 @@ function formatSingleRequirement(requirement: string): string { // "CIS AWS Foundations Benchmark v1.2.0 1.1" // "PCI DSS v3.2.1 8.2.3" // "AWS Foundational Security Best Practices v1.0.0/EC2.2" - // We try a few patterns then fall back to a sensible default so we - // surface SOMETHING rather than drop a real compliance mapping. + // + // AWS FSBP uses `/` between the version and the control id; normalize + // it to whitespace first so the same regex handles all four formats. + const normalized = cleaned.replace(/\//g, ' '); - const standardMatch = cleaned.match( - /^([A-Z][A-Z0-9 .]+?)(?:\s+v?([\d.]+(?:[a-z]\d*)?))?\s+([A-Za-z0-9.\-]+)$/, + // Match `STANDARD vVERSION CONTROL`. The first group must accept + // lowercase letters and hyphens — without them, 3 of the 4 documented + // formats (NIST, CIS, AWS FSBP) fall through to the raw fallback and + // never produce structured `standard version (control)` output for + // the compliance chips. + const standardMatch = normalized.match( + /^([A-Za-z][A-Za-z0-9 .\-]+?)\s+v?([\d.]+(?:[a-z]\d*)?)\s+([A-Za-z0-9.\-]+)$/, ); if (standardMatch) { const [, rawStandard, version, control] = standardMatch; const standard = normalizeStandardName(rawStandard); - const ver = version ?? 'unspecified'; - return `${standard} ${ver} (${control})`; + return `${standard} ${version} (${control})`; } - // Fallback — keep the raw string so we don't silently drop data. + // Fallback — keep the raw string for formats without an explicit + // version (e.g. NIST 800-53 where the revision is embedded in the + // standard name: "NIST.800-53.r5 AC-2"). The downstream parser + // surfaces it as a single chip label, which is still informative. return cleaned; }