Merge pull request #2875 from trycompai/tofik/fix-sechub-compliance-regex-and-dto-source

fix(cloud-tests): parse all SecHub compliance formats + dedupe scan-mode literals

Author: tofikwest

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);
+      }
+    });
+  });
 });

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';

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;
 }

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',

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;
 }