From 8b7fa7edcc2f56f4a5322b398d3d81396cc8d2d5 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Tue, 19 May 2026 11:05:07 -0400 Subject: [PATCH 1/6] feat(cloud-tests): aws security hub as alternative scan engine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Customer Blazestack (relayed by Joe in CX) uses AWS Security Hub for NIST 800-53 compliance and asked us to read findings from their existing Security Hub deployment. This change makes Security Hub a first-class scan engine alongside our adapter-based scanning, switchable at connect time or later in settings. ## What changed A connection's awsScanMode determines which engine runs on each scan: - 'comp_scanners' (default): our 49 service adapters run, today's behavior — byte-for-byte identical when the field is unset or explicitly 'comp_scanners'. - 'security_hub': only the SecurityHubAdapter runs, reads findings from GetFindings, surfaces native NIST/CIS/PCI control mappings, uses the same Fix pipeline as adapter findings. The two modes are mutually exclusive by code structure (different methods on AWSSecurityService) — there is no runtime state where both run, so duplicate findings are impossible. ## Files an engineer can follow the thread through - aws-scan-mode.ts — type + helper (single source of truth) - aws-scan-mode.service.ts — settings switcher - aws-scan-mode.spec.ts — type tests - aws-security.service.ts — scanViaAdapters / scanViaSecurityHub split - providers/aws/security-hub.adapter.ts — finding mapping + findingKey derivation - cloud-security.service.ts — reads mode from metadata, persists on each run - reconciliation.service.ts — same-mode-only diffs (safety against switch) - cloud-security.controller.ts — PATCH /connections/:id/scan-mode - dto/update-scan-mode.dto.ts — validated request body - IntegrationCheckRun.scanMode column — per-run engine attribution - EmptyStateOnboarding.tsx + AwsScanModeStep — Step 1 at AWS connect time - CloudSettingsModal + ScanModeSwitchDialog — change later from settings - RemediationDialog (SecurityHubFixDisclosure) — banner on SecHub-sourced fixes ## Why current AWS logic is safe - Default for missing field is 'comp_scanners' — every existing connection in production continues with today's behavior. - SecurityHubAdapter is NOT in the registered adapters array; it can only run when the customer explicitly opts in. - Reconciliation now scopes prior-run lookup by scanMode, so a mode switch produces a clean baseline instead of fake "resolved" rows. - The AI fix prompt is input-agnostic (verified) — SecHub findings flow through the same pipeline without prompt changes. - The remediation-parser already extracts reference-URL and compliance chips from GCP's `More info: / Compliance:` text format; the SecHub adapter emits text in the same format so chips render with zero frontend changes. ## Tests - aws-scan-mode.spec.ts — 6 tests (resolve + default) - security-hub.adapter.spec.ts — 23 tests (helpers + mapping) - reconciliation.service.spec.ts — +3 tests (same-mode safety) - AwsScanModeStep.test.tsx — 6 tests (picker UI) Cloud-security API tests: 157 passing (1 pre-existing TLS env failure in remediation.controller.spec.ts unchanged). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../cloud-security/aws-scan-mode.service.ts | 104 ++++++ .../src/cloud-security/aws-scan-mode.spec.ts | 51 +++ apps/api/src/cloud-security/aws-scan-mode.ts | 44 +++ .../cloud-security/cloud-security-audit.ts | 3 +- .../cloud-security.controller.ts | 32 ++ .../cloud-security/cloud-security.module.ts | 2 + .../cloud-security/cloud-security.service.ts | 21 +- .../dto/update-scan-mode.dto.ts | 20 ++ .../providers/aws-security.service.ts | 179 ++++++++-- .../aws/security-hub.adapter.spec.ts | 225 ++++++++++++ .../providers/aws/security-hub.adapter.ts | 322 ++++++++++++++---- .../reconciliation.service.spec.ts | 85 +++++ .../cloud-security/reconciliation.service.ts | 15 + .../controllers/connections.controller.ts | 11 + .../components/CloudSettingsModal.test.tsx | 9 +- .../components/CloudSettingsModal.tsx | 86 ++++- .../components/CloudTestsSection.tsx | 8 + .../components/RemediationDialog.tsx | 38 +++ .../components/ScanModeSwitchDialog.tsx | 153 +++++++++ .../components/AwsScanModeStep.test.tsx | 74 ++++ .../[slug]/components/AwsScanModeStep.tsx | 146 ++++++++ .../components/EmptyStateOnboarding.tsx | 73 ++-- .../migration.sql | 2 + .../prisma/schema/integration-platform.prisma | 9 + 24 files changed, 1592 insertions(+), 120 deletions(-) create mode 100644 apps/api/src/cloud-security/aws-scan-mode.service.ts create mode 100644 apps/api/src/cloud-security/aws-scan-mode.spec.ts create mode 100644 apps/api/src/cloud-security/aws-scan-mode.ts create mode 100644 apps/api/src/cloud-security/dto/update-scan-mode.dto.ts create mode 100644 apps/api/src/cloud-security/providers/aws/security-hub.adapter.spec.ts create mode 100644 apps/app/src/app/(app)/[orgId]/cloud-tests/components/ScanModeSwitchDialog.tsx create mode 100644 apps/app/src/app/(app)/[orgId]/integrations/[slug]/components/AwsScanModeStep.test.tsx create mode 100644 apps/app/src/app/(app)/[orgId]/integrations/[slug]/components/AwsScanModeStep.tsx create mode 100644 packages/db/prisma/migrations/20260519143715_integration_check_run_scan_mode/migration.sql diff --git a/apps/api/src/cloud-security/aws-scan-mode.service.ts b/apps/api/src/cloud-security/aws-scan-mode.service.ts new file mode 100644 index 0000000000..d8811eeadd --- /dev/null +++ b/apps/api/src/cloud-security/aws-scan-mode.service.ts @@ -0,0 +1,104 @@ +import { + BadRequestException, + ForbiddenException, + Injectable, + Logger, +} from '@nestjs/common'; +import { db, Prisma } from '@db'; +import { + type AwsScanMode, + resolveAwsScanMode, +} from './aws-scan-mode'; +import { logCloudSecurityActivity } from './cloud-security-audit'; + +/** + * Manages the AWS scan-engine choice on a connection. Lives separately + * from CloudSecurityService so the scan-mode concern has one obvious + * home — engineers grep for `aws-scan-mode.service` and find every + * read / write site at once. + */ +@Injectable() +export class CloudAwsScanModeService { + private readonly logger = new Logger(CloudAwsScanModeService.name); + + /** + * Update the scan engine on an AWS connection. Validates: + * - The connection exists and belongs to the caller's org. + * - The connection is an AWS provider (other providers don't have + * a scan-mode concept). + * + * Idempotent — re-applying the same mode is a successful no-op. + * + * Writes an audit-log entry so a mode change is traceable later. + * Reconciliation reads `IntegrationCheckRun.scanMode` per run, so + * after this update the next scan automatically uses the new engine + * and reconciliation only diffs same-mode runs (see + * `reconciliation.service.ts`). + */ + async updateMode(params: { + connectionId: string; + organizationId: string; + userId: string; + mode: AwsScanMode; + }): Promise<{ mode: AwsScanMode }> { + const connection = await db.integrationConnection.findFirst({ + where: { + id: params.connectionId, + organizationId: params.organizationId, + }, + select: { + id: true, + metadata: true, + provider: { select: { slug: true } }, + }, + }); + + if (!connection) { + throw new ForbiddenException( + 'Connection not found or does not belong to your organization.', + ); + } + if (connection.provider?.slug !== 'aws') { + throw new BadRequestException( + 'Scan engine choice is only available for AWS connections.', + ); + } + + const metadata = (connection.metadata ?? {}) as Record; + const previousMode = resolveAwsScanMode(metadata.awsScanMode); + + if (previousMode === params.mode) { + // Idempotent no-op — return the current mode without writing. + return { mode: params.mode }; + } + + const nextMetadata: Record = { + ...metadata, + awsScanMode: params.mode, + }; + + await db.integrationConnection.update({ + where: { id: connection.id }, + data: { + metadata: nextMetadata as unknown as Prisma.InputJsonValue, + }, + }); + + await logCloudSecurityActivity({ + organizationId: params.organizationId, + userId: params.userId, + connectionId: connection.id, + action: 'scan_mode_changed', + description: `Switched AWS scan engine: ${previousMode} → ${params.mode}`, + metadata: { + previousMode, + newMode: params.mode, + }, + }); + + this.logger.log( + `Connection ${connection.id} scan mode: ${previousMode} → ${params.mode}`, + ); + return { mode: params.mode }; + } +} diff --git a/apps/api/src/cloud-security/aws-scan-mode.spec.ts b/apps/api/src/cloud-security/aws-scan-mode.spec.ts new file mode 100644 index 0000000000..95701b206c --- /dev/null +++ b/apps/api/src/cloud-security/aws-scan-mode.spec.ts @@ -0,0 +1,51 @@ +import { + DEFAULT_AWS_SCAN_MODE, + isSecurityHubMode, + resolveAwsScanMode, +} from './aws-scan-mode'; + +describe('aws-scan-mode', () => { + describe('resolveAwsScanMode', () => { + it('returns "security_hub" when the value is exactly that string', () => { + expect(resolveAwsScanMode('security_hub')).toBe('security_hub'); + }); + + it('returns the default for "comp_scanners"', () => { + expect(resolveAwsScanMode('comp_scanners')).toBe('comp_scanners'); + }); + + it('returns the default for unknown strings', () => { + // Defensive — typos / future modes / corrupted JSON variables must + // never accidentally activate Security Hub mode. + expect(resolveAwsScanMode('SECURITY_HUB')).toBe(DEFAULT_AWS_SCAN_MODE); + expect(resolveAwsScanMode('securityhub')).toBe(DEFAULT_AWS_SCAN_MODE); + expect(resolveAwsScanMode('xyz')).toBe(DEFAULT_AWS_SCAN_MODE); + }); + + it('returns the default for missing / non-string values', () => { + expect(resolveAwsScanMode(undefined)).toBe(DEFAULT_AWS_SCAN_MODE); + expect(resolveAwsScanMode(null)).toBe(DEFAULT_AWS_SCAN_MODE); + expect(resolveAwsScanMode(0)).toBe(DEFAULT_AWS_SCAN_MODE); + expect(resolveAwsScanMode({})).toBe(DEFAULT_AWS_SCAN_MODE); + expect(resolveAwsScanMode([])).toBe(DEFAULT_AWS_SCAN_MODE); + }); + }); + + describe('isSecurityHubMode', () => { + it('is true only for the exact "security_hub" string', () => { + expect(isSecurityHubMode('security_hub')).toBe(true); + expect(isSecurityHubMode('comp_scanners')).toBe(false); + expect(isSecurityHubMode(undefined)).toBe(false); + expect(isSecurityHubMode('SECURITY_HUB')).toBe(false); + }); + }); + + describe('DEFAULT_AWS_SCAN_MODE', () => { + it('is "comp_scanners" — today\'s behavior is the safe default', () => { + // This is intentionally guarded by a test: changing the default + // would silently shift production behavior for every existing + // connection that does not have an explicit scan mode set. + expect(DEFAULT_AWS_SCAN_MODE).toBe('comp_scanners'); + }); + }); +}); diff --git a/apps/api/src/cloud-security/aws-scan-mode.ts b/apps/api/src/cloud-security/aws-scan-mode.ts new file mode 100644 index 0000000000..3907878727 --- /dev/null +++ b/apps/api/src/cloud-security/aws-scan-mode.ts @@ -0,0 +1,44 @@ +/** + * Determines which engine performs an AWS security scan for a given + * connection. This is the single source of truth for scan-mode strings — + * importers never spell the values themselves. + * + * - 'comp_scanners' — run our service adapters directly against AWS + * APIs (today's default; full Fix button support). + * - 'security_hub' — call AWS Security Hub's GetFindings API and + * surface whatever findings the customer's + * Security Hub is configured to evaluate (CIS, + * NIST 800-53, PCI, etc.). + * + * Persisted in two places: + * - `IntegrationConnection.metadata.awsScanMode` — the customer's + * current choice for the connection. Stored in `metadata` (not + * `variables`) because it's a non-secret display field that the + * frontend reads via the connection endpoint; mirrors `awsType`, + * `roleArn`, `regions` etc. + * - `IntegrationCheckRun.scanMode` — which engine produced this run. + * Read by reconciliation to diff like-for-like; findingKeys live + * in different namespaces across modes, so cross-mode diffs would + * produce false resolutions/regressions. + */ +export type AwsScanMode = 'comp_scanners' | 'security_hub'; + +/** 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'; + +/** + * Coerces a value (typically from JSON variables) into a valid + * AwsScanMode. Unknown / missing / wrong-typed values fall back to the + * default. Use this everywhere instead of comparing strings inline. + */ +export function resolveAwsScanMode(value: unknown): AwsScanMode { + return value === 'security_hub' ? 'security_hub' : DEFAULT_AWS_SCAN_MODE; +} + +/** True when the value, if persisted, would represent a SecHub-mode + * connection. Reads in one place — keeps `=== 'security_hub'` out of + * callers. */ +export function isSecurityHubMode(value: unknown): boolean { + return resolveAwsScanMode(value) === 'security_hub'; +} diff --git a/apps/api/src/cloud-security/cloud-security-audit.ts b/apps/api/src/cloud-security/cloud-security-audit.ts index 9e3f4ea0b9..f28e675043 100644 --- a/apps/api/src/cloud-security/cloud-security-audit.ts +++ b/apps/api/src/cloud-security/cloud-security-audit.ts @@ -13,7 +13,8 @@ interface CloudSecurityAuditParams { | 'rollback_failed' | 'service_toggled' | 'exception_marked' - | 'exception_revoked'; + | 'exception_revoked' + | 'scan_mode_changed'; description: string; metadata?: Record; } diff --git a/apps/api/src/cloud-security/cloud-security.controller.ts b/apps/api/src/cloud-security/cloud-security.controller.ts index 862b9ec649..a0286c4a0a 100644 --- a/apps/api/src/cloud-security/cloud-security.controller.ts +++ b/apps/api/src/cloud-security/cloud-security.controller.ts @@ -3,6 +3,7 @@ import { Post, Get, Delete, + Patch, Param, Query, Body, @@ -27,8 +28,10 @@ import { CloudSecurityLegacyService } from './cloud-security-legacy.service'; import { CheckDefinitionService } from './check-definition.service'; import { CloudExceptionService } from './exception.service'; import { CloudHistoryService } from './history.service'; +import { CloudAwsScanModeService } from './aws-scan-mode.service'; import { parseExceptionExpiry } from './exception-expiry.utils'; import { MarkExceptionDto } from './dto/mark-exception.dto'; +import { UpdateAwsScanModeDto } from './dto/update-scan-mode.dto'; import { logCloudSecurityActivity } from './cloud-security-audit'; import { CloudSecurityActivityService } from './cloud-security-activity.service'; import { @@ -52,6 +55,7 @@ export class CloudSecurityController { private readonly checkDefinitionService: CheckDefinitionService, private readonly exceptionService: CloudExceptionService, private readonly historyService: CloudHistoryService, + private readonly scanModeService: CloudAwsScanModeService, ) {} @Get('activity') @@ -134,6 +138,34 @@ export class CloudSecurityController { return { data: result }; } + @Patch('connections/:connectionId/scan-mode') + @UseGuards(HybridAuthGuard, PermissionGuard) + @RequirePermission('integration', 'update') + @ApiOperation({ + summary: + 'Switch the AWS scan engine for a connection (Comp AI scanners ↔ Security Hub)', + }) + async updateAwsScanMode( + @Param('connectionId') connectionId: string, + @Body() body: UpdateAwsScanModeDto, + @OrganizationId() organizationId: string, + @Req() req: { userId?: string }, + ) { + if (!req.userId) { + throw new HttpException( + 'Switching the scan engine requires session authentication.', + HttpStatus.UNAUTHORIZED, + ); + } + const result = await this.scanModeService.updateMode({ + connectionId, + organizationId, + userId: req.userId, + mode: body.mode, + }); + return { data: result }; + } + @Delete('exceptions/:exceptionId') @UseGuards(HybridAuthGuard, PermissionGuard) @RequirePermission('integration', 'update') diff --git a/apps/api/src/cloud-security/cloud-security.module.ts b/apps/api/src/cloud-security/cloud-security.module.ts index 52618413eb..9b5b5497ce 100644 --- a/apps/api/src/cloud-security/cloud-security.module.ts +++ b/apps/api/src/cloud-security/cloud-security.module.ts @@ -15,6 +15,7 @@ import { AiDescriptionService } from './ai-description.service'; import { CheckDefinitionService } from './check-definition.service'; import { CloudExceptionService } from './exception.service'; import { CloudHistoryService } from './history.service'; +import { CloudAwsScanModeService } from './aws-scan-mode.service'; import { CloudReconciliationService } from './reconciliation.service'; import { CloudSecurityActivityService } from './cloud-security-activity.service'; import { IntegrationPlatformModule } from '../integration-platform/integration-platform.module'; @@ -40,6 +41,7 @@ import { AuthModule } from '../auth/auth.module'; CloudExceptionService, CloudReconciliationService, CloudHistoryService, + CloudAwsScanModeService, ], exports: [CloudSecurityService], }) diff --git a/apps/api/src/cloud-security/cloud-security.service.ts b/apps/api/src/cloud-security/cloud-security.service.ts index c815a3fabb..39d700a767 100644 --- a/apps/api/src/cloud-security/cloud-security.service.ts +++ b/apps/api/src/cloud-security/cloud-security.service.ts @@ -9,6 +9,7 @@ import { AWSSecurityService } from './providers/aws-security.service'; import { AzureSecurityService } from './providers/azure-security.service'; import { AWS_SERVICE_TASK_MAPPINGS } from './aws-task-mappings'; import { CloudReconciliationService } from './reconciliation.service'; +import { type AwsScanMode, resolveAwsScanMode } from './aws-scan-mode'; export interface SecurityFinding { id: string; @@ -245,6 +246,11 @@ export class CloudSecurityService { } } + // AWS-only — which engine produced this scan. Persisted on the run + // so reconciliation only diffs like-for-like (cross-mode findingKeys + // live in different namespaces). Null for GCP / Azure runs. + let awsScanMode: AwsScanMode | null = null; + switch (providerSlug) { case 'gcp': findings = await this.gcpService.scanSecurityFindings( @@ -253,13 +259,21 @@ export class CloudSecurityService { enabledServices, ); break; - case 'aws': + case 'aws': { + // AWS scan-mode lives on connection.metadata (non-secret, frontend- + // readable); credentials are encrypted blobs intended for the AWS + // SDK. Read from metadata so a single source of truth. + const metadata = + (connection.metadata as Record | null) ?? {}; + awsScanMode = resolveAwsScanMode(metadata.awsScanMode); findings = await this.awsService.scanSecurityFindings( credentials, variables, enabledServices, + awsScanMode, ); break; + } case 'azure': findings = await this.azureService.scanSecurityFindings( credentials, @@ -282,6 +296,7 @@ export class CloudSecurityService { connectionId, providerSlug, findings, + awsScanMode, ); // Reconcile against the prior scan to record resolutions and regressions. @@ -573,6 +588,9 @@ export class CloudSecurityService { connectionId: string, provider: string, findings: SecurityFinding[], + // AWS only — which engine produced these findings. Stored on the run so + // reconciliation can avoid cross-mode diffs. Null for GCP / Azure runs. + awsScanMode: AwsScanMode | null, ): Promise { const passedCount = findings.filter((f) => f.passed).length; const failedCount = findings.filter((f) => !f.passed).length; @@ -607,6 +625,7 @@ export class CloudSecurityService { passedCount, failedCount, scannedServices, + scanMode: awsScanMode, }, }); 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 new file mode 100644 index 0000000000..fd92285ef6 --- /dev/null +++ b/apps/api/src/cloud-security/dto/update-scan-mode.dto.ts @@ -0,0 +1,20 @@ +import { ApiProperty } from '@nestjs/swagger'; +import { IsIn, IsString } from 'class-validator'; +import 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. + */ +export class UpdateAwsScanModeDto { + @ApiProperty({ + description: 'Which scan engine to use for this AWS connection.', + enum: ['comp_scanners', 'security_hub'], + example: 'security_hub', + }) + @IsString() + @IsIn(['comp_scanners', 'security_hub']) + mode!: AwsScanMode; +} diff --git a/apps/api/src/cloud-security/providers/aws-security.service.ts b/apps/api/src/cloud-security/providers/aws-security.service.ts index bf6c825935..fa0f0a35fd 100644 --- a/apps/api/src/cloud-security/providers/aws-security.service.ts +++ b/apps/api/src/cloud-security/providers/aws-security.service.ts @@ -64,9 +64,26 @@ import { EventBridgeAdapter } from './aws/eventbridge.adapter'; import { TransferFamilyAdapter } from './aws/transfer-family.adapter'; import { ElasticBeanstalkAdapter } from './aws/elastic-beanstalk.adapter'; import { AppFlowAdapter } from './aws/appflow.adapter'; +import { SecurityHubAdapter } from './aws/security-hub.adapter'; +import { + type AwsScanMode, + DEFAULT_AWS_SCAN_MODE, +} from '../aws-scan-mode'; const GOVCLOUD_UNSUPPORTED_SERVICE_IDS = new Set(['cloudfront', 'shield']); +/** + * Pre-computed scan context shared by both scan engines (adapter mode + * and Security Hub mode). Produced by `prepareScan`, consumed by + * `scanViaAdapters` / `scanViaSecurityHub`. + */ +interface AwsScanSetup { + awsCredentials: AwsCredentials; + configuredRegions: string[]; + primaryRegion: string; + partition: AwsPartition; +} + @Injectable() export class AWSSecurityService { private readonly logger = new Logger(AWSSecurityService.name); @@ -118,11 +135,41 @@ export class AWSSecurityService { new AppFlowAdapter(), ]; + /** + * Entry point — resolves credentials + regions, then dispatches to the + * chosen scan engine. The two engines are mutually exclusive: + * + * - 'comp_scanners' → runs our ~49 service adapters (default). + * - 'security_hub' → runs ONLY the Security Hub adapter, per region. + * + * Engineers tracing this code can jump straight to `scanViaAdapters` + * or `scanViaSecurityHub` to see what each mode does — no nested + * branching. + */ async scanSecurityFindings( credentials: Record, variables: Record, enabledServices?: string[], + mode: AwsScanMode = DEFAULT_AWS_SCAN_MODE, ): Promise { + const setup = await this.prepareScan(credentials, variables); + + if (mode === 'security_hub') { + return this.scanViaSecurityHub(setup); + } + return this.scanViaAdapters(setup, enabledServices); + } + + /** + * Validates credentials, resolves the region list, and (for role + * auth) assumes the IAM role. Returns the shared context both scan + * engines need. Throws on any precondition failure — both engines + * skip out of an unhappy path before they begin. + */ + private async prepareScan( + credentials: Record, + variables: Record, + ): Promise { const isRoleAuth = Boolean(credentials.roleArn && credentials.externalId); const isKeyAuth = Boolean( credentials.access_key_id && credentials.secret_access_key, @@ -143,6 +190,7 @@ export class AWSSecurityService { partition, ); const primaryRegion = configuredRegions[0]; + const mismatchedRegions = configuredRegions.filter( (region) => getAwsPartitionForRegion(region) !== partition, ); @@ -156,31 +204,65 @@ export class AWSSecurityService { `Scanning ${configuredRegions.length} ${partition} region(s): ${configuredRegions.join(', ')}`, ); - // Assume role ONCE — IAM is global, credentials work across all regions - let awsCredentials: AwsCredentials; - if (isRoleAuth) { - const partitionErrors = validateAwsPartitionConfig({ - partition, - roleArn: credentials.roleArn as string, - regions: configuredRegions, - }); - if (partitionErrors.length > 0) { - throw new Error(partitionErrors.join(' ')); - } + // Assume role ONCE — IAM is global, credentials work across all regions. + const awsCredentials: AwsCredentials = isRoleAuth + ? await this.resolveRoleCredentials({ + roleArn: credentials.roleArn as string, + externalId: credentials.externalId as string, + partition, + regions: configuredRegions, + primaryRegion, + }) + : { + accessKeyId: credentials.access_key_id as string, + secretAccessKey: credentials.secret_access_key as string, + }; - awsCredentials = await this.assumeRole({ - roleArn: credentials.roleArn as string, - externalId: credentials.externalId as string, - region: primaryRegion, - partition, - }); - } else { - awsCredentials = { - accessKeyId: credentials.access_key_id as string, - secretAccessKey: credentials.secret_access_key as string, - }; + return { + awsCredentials, + configuredRegions, + primaryRegion, + partition, + }; + } + + private async resolveRoleCredentials(params: { + roleArn: string; + externalId: string; + partition: AwsPartition; + regions: string[]; + primaryRegion: string; + }): Promise { + const partitionErrors = validateAwsPartitionConfig({ + partition: params.partition, + roleArn: params.roleArn, + regions: params.regions, + }); + if (partitionErrors.length > 0) { + throw new Error(partitionErrors.join(' ')); } + return this.assumeRole({ + roleArn: params.roleArn, + externalId: params.externalId, + region: params.primaryRegion, + partition: params.partition, + }); + } + + /** + * Today's default scan engine. Runs each registered service adapter + * (global once, regional per region) and aggregates the findings. + * Behavior is byte-for-byte identical to the pre-mode implementation — + * the SecurityHubAdapter is NOT in `this.adapters`, so it can never + * run on this path. + */ + private async scanViaAdapters( + setup: AwsScanSetup, + enabledServices: string[] | undefined, + ): Promise { + const { awsCredentials, configuredRegions, primaryRegion, partition } = setup; + // undefined = scan all (no detection data), [] = scan nothing (all disabled), [...] = scan specific const activeAdaptersBeforePartitionFilter = enabledServices === undefined @@ -268,6 +350,59 @@ export class AWSSecurityService { return allFindings; } + /** + * Alternative scan engine. Pulls findings from AWS Security Hub + * `GetFindings` per region — does NOT touch any of the 49 service + * adapters. SecurityHubAdapter handles the "not subscribed" / + * "AccessDenied" cases gracefully (returns []). + * + * Activated by `awsScanMode: 'security_hub'` on the connection. + */ + private async scanViaSecurityHub( + setup: AwsScanSetup, + ): Promise { + const { awsCredentials, configuredRegions } = setup; + const adapter = new SecurityHubAdapter(); + + this.logger.log( + `Scanning Security Hub across ${configuredRegions.length} region(s)`, + ); + + const allFindings: SecurityFinding[] = []; + const successfulRegions = new Set(); + const failedRegions = new Set(); + + for (const region of configuredRegions) { + try { + const findings = await adapter.scan({ + credentials: awsCredentials, + region, + }); + // Adapter already stamps evidence.serviceId — no need to re-stamp. + allFindings.push(...findings); + successfulRegions.add(region); + this.logger.log( + `[security-hub] ${findings.length} findings in ${region}`, + ); + } catch (error) { + const msg = error instanceof Error ? error.message : String(error); + this.logger.warn(`[security-hub] Error in ${region}: ${msg}`); + failedRegions.add(region); + } + } + + if (successfulRegions.size === 0 && failedRegions.size > 0) { + throw new Error( + `Security Hub scan failed in all ${failedRegions.size} region(s): ${[...failedRegions].join(', ')}`, + ); + } + + this.logger.log( + `Security Hub scan complete: ${allFindings.length} findings from ${successfulRegions.size} region(s)`, + ); + return allFindings; + } + /** * Get the list of regions to scan from credentials or variables. * Always returns at least one region (defaults to us-east-1). 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 new file mode 100644 index 0000000000..d78d1a4aea --- /dev/null +++ b/apps/api/src/cloud-security/providers/aws/security-hub.adapter.spec.ts @@ -0,0 +1,225 @@ +import { + buildRemediationText, + deriveFindingKey, + formatRelatedRequirements, + mapSecurityHubFinding, + SECURITY_HUB_SERVICE_ID, + type SecurityHubRawFinding, +} from './security-hub.adapter'; + +describe('security-hub.adapter helpers', () => { + describe('deriveFindingKey', () => { + it('extracts the trailing control id from a foundational best-practices GeneratorId', () => { + expect( + deriveFindingKey('aws-foundational-security-best-practices/v/1.0.0/EC2.13'), + ).toBe('aws-securityhub-ec2.13'); + }); + + it('extracts the trailing control id from a CIS GeneratorId', () => { + expect( + deriveFindingKey('cis-aws-foundations-benchmark/v/1.2.0/1.1'), + ).toBe('aws-securityhub-1.1'); + }); + + it('extracts the trailing control id from a NIST GeneratorId', () => { + expect(deriveFindingKey('nist-800-53/r/5/AC-2')).toBe( + 'aws-securityhub-ac-2', + ); + }); + + it('sanitizes characters not safe for use in identifiers', () => { + expect(deriveFindingKey('weird:control id with spaces!')).toMatch( + /^aws-securityhub-/, + ); + }); + + it('produces a stable key (no timestamps / randomness) so reconciliation can diff scans', () => { + const a = deriveFindingKey('aws-foundational-security-best-practices/v/1.0.0/EC2.13'); + const b = deriveFindingKey('aws-foundational-security-best-practices/v/1.0.0/EC2.13'); + expect(a).toBe(b); + }); + + it('returns a sentinel key rather than throwing when GeneratorId is missing', () => { + // We must always produce SOME key — the Fix pipeline gates on + // findingKey existence, and silently disabling Fix on findings + // without GeneratorId would be a worse UX than an "unknown" key. + expect(deriveFindingKey(undefined)).toBe('aws-securityhub-unknown'); + expect(deriveFindingKey('')).toBe('aws-securityhub-unknown'); + expect(deriveFindingKey(' ')).toBe('aws-securityhub-unknown'); + }); + }); + + describe('formatRelatedRequirements', () => { + it('returns "" for empty/undefined input', () => { + expect(formatRelatedRequirements(undefined)).toBe(''); + 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('joins multiple requirements with "; " so the parser splits them correctly', () => { + const result = formatRelatedRequirements([ + 'NIST.800-53.r5 AC-2', + 'CIS AWS Foundations Benchmark v1.2.0 1.1', + ]); + expect(result).toContain('; '); + }); + + it('keeps unfamiliar requirement strings verbatim rather than dropping them', () => { + // We never want to silently lose a compliance reference — if SecHub + // adds a new framework format we don't recognize, we still surface + // the raw string so the auditor sees something. + const weird = 'SomeFutureFramework/CustomFormat#42'; + const result = formatRelatedRequirements([weird]); + expect(result.toLowerCase()).toContain('somefutureframework'); + }); + }); + + describe('buildRemediationText', () => { + it('returns AWS text + reference URL + compliance section when all three are present', () => { + const result = buildRemediationText({ + Remediation: { + Recommendation: { + Text: 'Enable encryption on the bucket.', + Url: 'https://docs.aws.amazon.com/whatever', + }, + }, + Compliance: { RelatedRequirements: ['NIST.800-53.r5 AC-2'] }, + }); + expect(result).toContain('Enable encryption on the bucket.'); + expect(result).toContain('More info: https://docs.aws.amazon.com/whatever'); + expect(result).toContain('Compliance:'); + }); + + it('omits the More info section when no URL is present', () => { + const result = buildRemediationText({ + Remediation: { Recommendation: { Text: 'Do the thing.' } }, + }); + expect(result).toContain('Do the thing.'); + expect(result).not.toContain('More info:'); + }); + + it('omits the Compliance section when no related requirements exist', () => { + const result = buildRemediationText({ + Remediation: { Recommendation: { Text: 'Do the thing.' } }, + Compliance: { RelatedRequirements: [] }, + }); + expect(result).not.toContain('Compliance:'); + }); + + it('uses "\\n\\n" as the section separator — must match the parser contract', () => { + // `remediation-parser.ts` splits on `\n\n`. If we use any other + // separator, the chips disappear from the UI. + const result = buildRemediationText({ + Remediation: { + Recommendation: { Text: 'Step one.', Url: 'https://example.com' }, + }, + Compliance: { RelatedRequirements: ['NIST.800-53.r5 AC-2'] }, + }); + const sections = result.split('\n\n'); + expect(sections.length).toBeGreaterThanOrEqual(3); + }); + + it('returns a non-empty fallback when SecHub provides no remediation data', () => { + const result = buildRemediationText({}); + expect(result.length).toBeGreaterThan(0); + // The fallback string isn't load-bearing — just make sure we don't + // surface an empty string (RemediationSection would hide the whole + // section, which is misleading for a SecHub finding). + }); + }); + + describe('mapSecurityHubFinding', () => { + const baseFinding: SecurityHubRawFinding = { + Id: 'arn:aws:securityhub:us-east-1:123:finding/abc', + Title: 'EC2 default security group should not allow inbound traffic', + Description: 'A default security group allows broad ingress.', + Severity: { Label: 'HIGH' }, + Resources: [{ Type: 'AwsEc2SecurityGroup', Id: 'sg-12345' }], + AwsAccountId: '013388577167', + Region: 'us-east-1', + Compliance: { + Status: 'FAILED', + RelatedRequirements: ['NIST.800-53.r5 AC-2'], + }, + GeneratorId: + 'aws-foundational-security-best-practices/v/1.0.0/EC2.13', + Remediation: { + Recommendation: { + Text: 'Update the security group rules.', + Url: 'https://docs.aws.amazon.com/securityhub/EC2.13', + }, + }, + CreatedAt: '2026-05-18T10:00:00.000Z', + UpdatedAt: '2026-05-18T10:00:00.000Z', + }; + + it('stamps evidence.findingKey so the Fix pipeline picks it up', () => { + const mapped = mapSecurityHubFinding(baseFinding, 'us-east-1'); + expect(mapped.evidence?.findingKey).toBe('aws-securityhub-ec2.13'); + }); + + it('stamps evidence.serviceId so the UI can detect SecHub findings', () => { + const mapped = mapSecurityHubFinding(baseFinding, 'us-east-1'); + expect(mapped.evidence?.serviceId).toBe(SECURITY_HUB_SERVICE_ID); + }); + + it('builds remediation in the GCP-compatible format so the parser handles chips', () => { + const mapped = mapSecurityHubFinding(baseFinding, 'us-east-1'); + expect(mapped.remediation).toContain('Update the security group rules.'); + expect(mapped.remediation).toContain('More info:'); + expect(mapped.remediation).toContain('Compliance:'); + }); + + it('uses the finding-supplied region when available, falling back to the scan region', () => { + const mapped = mapSecurityHubFinding( + { ...baseFinding, Region: undefined }, + 'eu-west-1', + ); + expect(mapped.evidence?.region).toBe('eu-west-1'); + }); + + it('marks the finding as not-passed for non-PASSED compliance statuses', () => { + const mapped = mapSecurityHubFinding(baseFinding, 'us-east-1'); + expect(mapped.passed).toBe(false); + }); + + it('marks the finding as passed when SecHub reports PASSED', () => { + const passing: SecurityHubRawFinding = { + ...baseFinding, + Compliance: { ...baseFinding.Compliance, Status: 'PASSED' }, + }; + const mapped = mapSecurityHubFinding(passing, 'us-east-1'); + expect(mapped.passed).toBe(true); + }); + + it('maps SecHub severity labels to our internal severity levels', () => { + const expectations: Array<[string, string]> = [ + ['INFORMATIONAL', 'info'], + ['LOW', 'low'], + ['MEDIUM', 'medium'], + ['HIGH', 'high'], + ['CRITICAL', 'critical'], + ]; + for (const [sechubLabel, internalLevel] of expectations) { + const mapped = mapSecurityHubFinding( + { ...baseFinding, Severity: { Label: sechubLabel } }, + 'us-east-1', + ); + expect(mapped.severity).toBe(internalLevel); + } + }); + + it('defaults to medium severity when SecHub omits or returns an unknown label', () => { + const mapped = mapSecurityHubFinding( + { ...baseFinding, Severity: undefined }, + 'us-east-1', + ); + expect(mapped.severity).toBe('medium'); + }); + }); +}); 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 3c082933ee..d44a3aa6ac 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 @@ -6,8 +6,33 @@ import { import type { SecurityFinding } from '../../cloud-security.service'; import type { AwsCredentials, AwsServiceAdapter } from './aws-service-adapter'; +/** + * Maximum number of findings we pull from Security Hub per scan, across + * all pages. Bounded to keep scan time + payload size predictable. + */ +const MAX_FINDINGS_PER_SCAN = 500; + +/** Page size for the SecHub GetFindings API. */ +const FINDINGS_PAGE_SIZE = 100; + +/** Service ID that this adapter stamps onto each finding's evidence so + * downstream code (UI banner, Fix dialog) can recognize SecHub-sourced + * findings without inspecting the findingKey format. */ +export const SECURITY_HUB_SERVICE_ID = 'security-hub'; + +/** + * Reads findings from AWS Security Hub and maps them to our internal + * SecurityFinding shape so the rest of the system (Fix pipeline, + * History tab, compliance chips, UI) treats them like any other + * adapter's finding. + * + * This adapter is only instantiated when a connection's `awsScanMode` + * is `'security_hub'` — see `AWSSecurityService.scanViaSecurityHub`. + * It is NOT registered in the main adapters array; mode mutual + * exclusion is enforced by code structure, not runtime config. + */ export class SecurityHubAdapter implements AwsServiceAdapter { - readonly serviceId = 'security-hub'; + readonly serviceId = SECURITY_HUB_SERVICE_ID; readonly isGlobal = false; async scan(params: { @@ -15,14 +40,16 @@ export class SecurityHubAdapter implements AwsServiceAdapter { region: string; }): Promise { const { credentials, region } = params; - - const securityHub = new SecurityHubClient({ region, credentials }); + const client = new SecurityHubClient({ region, credentials }); try { - return await this.fetchFindings(securityHub, region); + return await this.fetchFindings(client, region); } catch (error) { - const msg = error instanceof Error ? error.message : String(error); - if (msg.includes('not subscribed') || msg.includes('AccessDenied')) { + const message = error instanceof Error ? error.message : String(error); + // Returning [] is the agreed graceful path when SecHub isn't subscribed + // or the role can't see findings — the cloud-security service surfaces + // a clearer onboarding error elsewhere when this happens consistently. + if (message.includes('not subscribed') || message.includes('AccessDenied')) { return []; } throw error; @@ -35,7 +62,7 @@ export class SecurityHubAdapter implements AwsServiceAdapter { ): Promise { const findings: SecurityFinding[] = []; - const params: GetFindingsCommandInput = { + const baseParams: GetFindingsCommandInput = { Filters: { WorkflowStatus: [ { Value: 'NEW', Comparison: 'EQUALS' }, @@ -43,84 +70,229 @@ export class SecurityHubAdapter implements AwsServiceAdapter { ], RecordState: [{ Value: 'ACTIVE', Comparison: 'EQUALS' }], }, - MaxResults: 100, + MaxResults: FINDINGS_PAGE_SIZE, }; - let response = await client.send(new GetFindingsCommand(params)); - - if (response.Findings) { - for (const f of response.Findings) { - findings.push(this.mapFinding(f, region)); - } - } - - let nextToken = response.NextToken; - while (nextToken && findings.length < 500) { - response = await client.send( - new GetFindingsCommand({ ...params, NextToken: nextToken }), + let nextToken: string | undefined; + do { + const response = await client.send( + new GetFindingsCommand({ ...baseParams, NextToken: nextToken }), ); - - if (response.Findings) { - for (const f of response.Findings) { - if (findings.length >= 500) break; - findings.push(this.mapFinding(f, region)); - } + for (const finding of response.Findings ?? []) { + if (findings.length >= MAX_FINDINGS_PER_SCAN) break; + findings.push(mapSecurityHubFinding(finding, region)); } - nextToken = response.NextToken; - } + } while (nextToken && findings.length < MAX_FINDINGS_PER_SCAN); return findings; } +} + +/** + * Minimal shape we read from the Security Hub API. We don't type the + * full AWS response because we only consume a handful of fields and + * they're all optional — the AWS SDK types this very loosely anyway. + */ +export interface SecurityHubRawFinding { + Id?: string; + Title?: string; + Description?: string; + Remediation?: { + Recommendation?: { Text?: string; Url?: string }; + }; + Severity?: { Label?: string }; + Resources?: Array<{ Type?: string; Id?: string }>; + AwsAccountId?: string; + Region?: string; + Compliance?: { Status?: string; RelatedRequirements?: string[] }; + GeneratorId?: string; + CreatedAt?: string; + UpdatedAt?: string; +} + +const SEVERITY_BY_SECHUB_LABEL: Record = { + INFORMATIONAL: 'info', + LOW: 'low', + MEDIUM: 'medium', + HIGH: 'high', + CRITICAL: 'critical', +}; - private mapFinding( - finding: { - Id?: string; - Title?: string; - Description?: string; - Remediation?: { Recommendation?: { Text?: string } }; - Severity?: { Label?: string }; - Resources?: Array<{ Type?: string; Id?: string }>; - AwsAccountId?: string; - Region?: string; - Compliance?: { Status?: string }; - GeneratorId?: string; - CreatedAt?: string; - UpdatedAt?: string; +/** + * Maps a raw SecHub finding into our internal SecurityFinding shape. + * Exported so the unit tests can exercise it directly without a live + * AWS client. + * + * Key design choices: + * - `evidence.findingKey` makes the finding visible to the Fix + * pipeline (the frontend `canFixFinding` and the API ai-remediation + * flow both gate on this). Derived from the SecHub control ID so + * it's stable across scans of the same control. + * - `evidence.serviceId` lets the UI recognize SecHub-sourced + * findings without parsing the findingKey format. + * - `remediation` is built in the same `\n\nMore info: \n\n + * Compliance: ` format that GCP uses, so the existing + * `RemediationSection` + `remediation-parser` render reference link + * and compliance chips with zero frontend changes. + */ +export function mapSecurityHubFinding( + finding: SecurityHubRawFinding, + scanRegion: string, +): SecurityFinding { + const region = finding.Region || scanRegion; + const passed = finding.Compliance?.Status === 'PASSED'; + const title = `${finding.Title ?? 'Untitled Finding'} (${region})`; + + return { + id: finding.Id ?? '', + title, + description: finding.Description ?? 'No description available', + severity: SEVERITY_BY_SECHUB_LABEL[finding.Severity?.Label ?? ''] ?? 'medium', + resourceType: finding.Resources?.[0]?.Type ?? 'unknown', + resourceId: finding.Resources?.[0]?.Id ?? 'unknown', + remediation: buildRemediationText(finding), + evidence: { + // Stamping serviceId here lets the UI banner + Fix dialog detect + // SecHub findings without parsing findingKey strings. + serviceId: SECURITY_HUB_SERVICE_ID, + // findingKey is the contract the Fix pipeline reads (see + // CloudTestsSection.canFixFinding and cloud-security-query.service + // findingKey extraction). Stable across scans of the same control. + findingKey: deriveFindingKey(finding.GeneratorId), + awsAccountId: finding.AwsAccountId, + region, + complianceStatus: finding.Compliance?.Status, + relatedRequirements: finding.Compliance?.RelatedRequirements ?? [], + generatorId: finding.GeneratorId, + updatedAt: finding.UpdatedAt, }, - scanRegion: string, - ): SecurityFinding { - const severityMap: Record = { - INFORMATIONAL: 'info', - LOW: 'low', - MEDIUM: 'medium', - HIGH: 'high', - CRITICAL: 'critical', - }; + createdAt: finding.CreatedAt ?? new Date().toISOString(), + passed, + }; +} - const complianceStatus = finding.Compliance?.Status; - const passed = complianceStatus === 'PASSED'; - const findingRegion = finding.Region || scanRegion; - const baseTitle = finding.Title || 'Untitled Finding'; - - return { - id: finding.Id || '', - title: `${baseTitle} (${findingRegion})`, - description: finding.Description || 'No description available', - severity: severityMap[finding.Severity?.Label || 'INFO'] || 'medium', - resourceType: finding.Resources?.[0]?.Type || 'unknown', - resourceId: finding.Resources?.[0]?.Id || 'unknown', - remediation: - finding.Remediation?.Recommendation?.Text || 'No remediation available', - evidence: { - awsAccountId: finding.AwsAccountId, - region: findingRegion, - complianceStatus, - generatorId: finding.GeneratorId, - updatedAt: finding.UpdatedAt, - }, - createdAt: finding.CreatedAt || new Date().toISOString(), - passed, - }; +/** + * Produces a stable findingKey from a SecHub `GeneratorId` like + * `aws-foundational-security-best-practices/v/1.0.0/EC2.13` or + * `cis-aws-foundations-benchmark/v/1.2.0/1.1`. + * + * Strategy: take the trailing control identifier (last `/`-delimited + * segment) since that's what makes the finding semantically unique. + * Falls back to a sanitized form of the full GeneratorId, then to a + * generic key, so we ALWAYS produce a key — the Fix button gates on + * its existence, so missing findingKey would silently disable Fix. + */ +export function deriveFindingKey(generatorId: string | undefined): string { + if (!generatorId) return 'aws-securityhub-unknown'; + const trimmed = generatorId.trim(); + if (!trimmed) return 'aws-securityhub-unknown'; + + const lastSegment = trimmed.split('/').pop() ?? trimmed; + const sanitized = sanitizeKeySegment(lastSegment); + if (sanitized) return `aws-securityhub-${sanitized}`; + + return `aws-securityhub-${sanitizeKeySegment(trimmed) || 'unknown'}`; +} + +function sanitizeKeySegment(value: string): string { + // Keep alphanumerics, dots, hyphens — drop anything that would make the + // key awkward in URLs/logs. Collapse runs of separators. + return value + .toLowerCase() + .replace(/[^a-z0-9.-]+/g, '-') + .replace(/-+/g, '-') + .replace(/^-|-$/g, ''); +} + +/** + * Builds the remediation string for a SecHub finding. Output format + * matches `gcp-security.service.ts buildRemediation` exactly so the + * existing parser (`remediation-parser.ts`) extracts the reference URL + * and compliance chips with zero frontend changes: + * + * + * + * More info: + * + * Compliance: nist 800-53 (AC-2); cis 1.2.0 (1.1) + */ +export function buildRemediationText(finding: SecurityHubRawFinding): string { + const parts: string[] = []; + + const text = finding.Remediation?.Recommendation?.Text?.trim(); + if (text) parts.push(text); + + const url = finding.Remediation?.Recommendation?.Url?.trim(); + if (url) parts.push(`More info: ${url}`); + + const compliance = formatRelatedRequirements( + finding.Compliance?.RelatedRequirements, + ); + if (compliance) parts.push(`Compliance: ${compliance}`); + + return ( + parts.join('\n\n') || + 'No remediation guidance was provided for this Security Hub finding.' + ); +} + +/** + * Converts SecHub's `RelatedRequirements` strings (e.g. + * ["NIST.800-53.r5 AC-2", "CIS AWS Foundations Benchmark v1.2.0 1.1"]) + * into the parser-compatible format used by GCP: + * "nist 800-53 (AC-2); cis 1.2.0 (1.1)" + * + * Returns '' when there are no related requirements (caller skips the + * Compliance: prefix in that case). Each requirement parses into + * " ()" — `remediation-parser.ts` + * `parseComplianceLine` handles malformed entries gracefully if anything + * here doesn't match the expected pattern. + */ +export function formatRelatedRequirements( + requirements: string[] | undefined, +): string { + if (!requirements || requirements.length === 0) return ''; + return requirements + .map(formatSingleRequirement) + .filter((s) => s.length > 0) + .join('; '); +} + +function formatSingleRequirement(requirement: string): string { + const cleaned = requirement.trim(); + if (!cleaned) return ''; + + // SecHub uses several formats; the most common are: + // "NIST.800-53.r5 AC-2" + // "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. + + const standardMatch = cleaned.match( + /^([A-Z][A-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})`; } + + // Fallback — keep the raw string so we don't silently drop data. + return cleaned; +} + +function normalizeStandardName(value: string): string { + // Compact common multi-word framework names so chips render cleanly. + return value + .toLowerCase() + .replace(/aws foundations benchmark/g, '') + .replace(/dss/g, 'dss') + .replace(/foundational security best practices/g, 'fsbp') + .replace(/\s+/g, ' ') + .trim() + .replace(/\.$/, ''); } diff --git a/apps/api/src/cloud-security/reconciliation.service.spec.ts b/apps/api/src/cloud-security/reconciliation.service.spec.ts index 75c91616e6..4d8041dfc6 100644 --- a/apps/api/src/cloud-security/reconciliation.service.spec.ts +++ b/apps/api/src/cloud-security/reconciliation.service.spec.ts @@ -301,4 +301,89 @@ describe('CloudReconciliationService.reconcile', () => { }), ); }); + + describe('scanMode safety — only diffs same-mode runs', () => { + it('passes scanMode from current run into the prior-run lookup', async () => { + // When SecHub mode is active, reconciliation must look up the prior + // SecHub run — not the prior comp_scanners run, which would mark + // every SecHub finding as "new" and every comp_scanners finding as + // "resolved" (both wrong). + dbMock.integrationCheckRun.findUnique.mockResolvedValueOnce({ + id: 'icr_current', + connectionId: 'icn_aws', + status: 'success', + startedAt: CURRENT_RUN_TIME, + completedAt: CURRENT_RUN_TIME, + scannedServices: [], + scanMode: 'security_hub', + connection: { organizationId: 'org_1' }, + results: [], + }); + dbMock.integrationCheckRun.findFirst.mockResolvedValueOnce(null); + + const service = new CloudReconciliationService(makeExceptionsStub()); + await service.reconcile({ currentRunId: 'icr_current' }); + + // findFirst is called once: the prior-run lookup. Its where clause + // MUST scope by the same scanMode as the current run. + expect(dbMock.integrationCheckRun.findFirst).toHaveBeenCalledWith( + expect.objectContaining({ + where: expect.objectContaining({ scanMode: 'security_hub' }), + }), + ); + }); + + it('passes null scanMode for legacy / non-AWS runs so they reconcile against each other', async () => { + // Pre-feature runs and GCP/Azure runs have scanMode = null. They + // must still reconcile against each other (null === null). + dbMock.integrationCheckRun.findUnique.mockResolvedValueOnce({ + id: 'icr_current', + connectionId: 'icn_gcp', + status: 'success', + startedAt: CURRENT_RUN_TIME, + completedAt: CURRENT_RUN_TIME, + scannedServices: [], + scanMode: null, + connection: { organizationId: 'org_1' }, + results: [], + }); + dbMock.integrationCheckRun.findFirst.mockResolvedValueOnce(null); + + const service = new CloudReconciliationService(makeExceptionsStub()); + await service.reconcile({ currentRunId: 'icr_current' }); + + expect(dbMock.integrationCheckRun.findFirst).toHaveBeenCalledWith( + expect.objectContaining({ + where: expect.objectContaining({ scanMode: null }), + }), + ); + }); + + it('returns 0/0 when no prior run of the same mode exists (post-switch baseline)', async () => { + // After a customer switches modes, the next scan finds no matching + // prior run and returns a clean baseline. This is the same code + // path as a first-ever scan — confirmed here to lock in the + // contract that mode switches are safe. + dbMock.integrationCheckRun.findUnique.mockResolvedValueOnce({ + id: 'icr_current', + connectionId: 'icn_aws', + status: 'success', + startedAt: CURRENT_RUN_TIME, + completedAt: CURRENT_RUN_TIME, + scannedServices: [], + scanMode: 'security_hub', + connection: { organizationId: 'org_1' }, + results: [], + }); + // Simulate no prior SecHub run found (only comp_scanners runs exist). + dbMock.integrationCheckRun.findFirst.mockResolvedValueOnce(null); + + const service = new CloudReconciliationService(makeExceptionsStub()); + const result = await service.reconcile({ currentRunId: 'icr_current' }); + + expect(result).toEqual({ resolutions: 0, regressions: 0, skipped: false }); + expect(dbMock.findingResolution.create).not.toHaveBeenCalled(); + expect(dbMock.findingRegression.create).not.toHaveBeenCalled(); + }); + }); }); diff --git a/apps/api/src/cloud-security/reconciliation.service.ts b/apps/api/src/cloud-security/reconciliation.service.ts index 70c0e8370c..89f4b0cd96 100644 --- a/apps/api/src/cloud-security/reconciliation.service.ts +++ b/apps/api/src/cloud-security/reconciliation.service.ts @@ -47,6 +47,11 @@ export class CloudReconciliationService { startedAt: true, status: true, scannedServices: true, + // Used below to scope the prior-run lookup. AWS connections can + // switch between 'comp_scanners' and 'security_hub' modes; the two + // engines produce findingKeys in completely different namespaces, + // so cross-mode diffs would produce garbage resolutions. + scanMode: true, connection: { select: { organizationId: true } }, results: { select: { @@ -90,6 +95,16 @@ export class CloudReconciliationService { id: { not: currentRun.id }, status: 'success', completedAt: { lt: currentRun.completedAt ?? currentRun.startedAt ?? new Date() }, + // Only compare runs that used the same scan engine. The two AWS + // modes ('comp_scanners' vs 'security_hub') emit findingKeys in + // different namespaces — cross-mode diffs would mark every + // prior-mode finding as "resolved", which would be a lie. After a + // mode switch, the next scan finds no matching prior run and is + // treated as a fresh baseline (same as a first-ever scan). + // + // Null scanMode matches null scanMode — pre-feature historical + // runs (and all GCP/Azure runs) reconcile against each other. + scanMode: currentRun.scanMode, }, orderBy: { completedAt: 'desc' }, select: { diff --git a/apps/api/src/integration-platform/controllers/connections.controller.ts b/apps/api/src/integration-platform/controllers/connections.controller.ts index 883177d315..88def11a48 100644 --- a/apps/api/src/integration-platform/controllers/connections.controller.ts +++ b/apps/api/src/integration-platform/controllers/connections.controller.ts @@ -463,6 +463,17 @@ export class ConnectionsController { if (Array.isArray(credentials.regions)) { metadata.regions = credentials.regions; } + // AWS only — which scan engine the customer chose at onboarding + // (Comp AI scanners vs Security Hub). Read on every scan by + // cloud-security.service.ts. Customers can change it later from + // CloudSettingsModal via aws-scan-mode.service.updateMode. + if ( + typeof credentials.awsScanMode === 'string' && + (credentials.awsScanMode === 'comp_scanners' || + credentials.awsScanMode === 'security_hub') + ) { + metadata.awsScanMode = credentials.awsScanMode; + } // Store roleArn and externalId in metadata for pre-filling the configure form // These are not secrets - roleArn is visible in AWS console, externalId is typically the org ID if (typeof credentials.roleArn === 'string') { diff --git a/apps/app/src/app/(app)/[orgId]/cloud-tests/components/CloudSettingsModal.test.tsx b/apps/app/src/app/(app)/[orgId]/cloud-tests/components/CloudSettingsModal.test.tsx index 09396ab0c8..d2a8ae74a6 100644 --- a/apps/app/src/app/(app)/[orgId]/cloud-tests/components/CloudSettingsModal.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/cloud-tests/components/CloudSettingsModal.test.tsx @@ -22,11 +22,18 @@ vi.mock('@/hooks/use-api', () => ({ }), })); -// Mock useIntegrationMutations +// Mock useIntegrationMutations + useIntegrationConnection (the latter is +// used by the AWS scan-mode settings section). vi.mock('@/hooks/use-integration-platform', () => ({ useIntegrationMutations: () => ({ deleteConnection: vi.fn(), }), + useIntegrationConnection: () => ({ + connection: { id: 'conn_test', metadata: {} }, + isLoading: false, + error: undefined, + refresh: vi.fn(), + }), })); // Mock @trycompai/ui components diff --git a/apps/app/src/app/(app)/[orgId]/cloud-tests/components/CloudSettingsModal.tsx b/apps/app/src/app/(app)/[orgId]/cloud-tests/components/CloudSettingsModal.tsx index be17668aa9..7ef7bdd174 100644 --- a/apps/app/src/app/(app)/[orgId]/cloud-tests/components/CloudSettingsModal.tsx +++ b/apps/app/src/app/(app)/[orgId]/cloud-tests/components/CloudSettingsModal.tsx @@ -1,7 +1,10 @@ 'use client'; import { useApi } from '@/hooks/use-api'; -import { useIntegrationMutations } from '@/hooks/use-integration-platform'; +import { + useIntegrationConnection, + useIntegrationMutations, +} from '@/hooks/use-integration-platform'; import { usePermissions } from '@/hooks/use-permissions'; import { Dialog, @@ -20,6 +23,8 @@ import { import { TrashCan } from '@trycompai/design-system/icons'; import { useState } from 'react'; import { toast } from 'sonner'; +import { ScanModeSwitchDialog } from './ScanModeSwitchDialog'; +import type { AwsScanModeChoice } from '../../integrations/[slug]/components/AwsScanModeStep'; interface CloudProvider { id: string; @@ -177,6 +182,14 @@ function ConnectionTab({ : 'To update credentials, disconnect and reconnect with your Microsoft account.'}

+ {/* AWS-only — scan engine switcher. Lets the customer change between + Comp AI scanners and Security Hub on an existing connection. + Surfaces the current mode + a "Change" button that opens + ScanModeSwitchDialog with the right confirmation copy. */} + {provider.id === 'aws' && !provider.isLegacy && ( + + )} + {canDelete && ( + )} + + + { + refresh(); + }} + /> + + ); +} diff --git a/apps/app/src/app/(app)/[orgId]/cloud-tests/components/CloudTestsSection.tsx b/apps/app/src/app/(app)/[orgId]/cloud-tests/components/CloudTestsSection.tsx index 4908df52fc..ec3d58383d 100644 --- a/apps/app/src/app/(app)/[orgId]/cloud-tests/components/CloudTestsSection.tsx +++ b/apps/app/src/app/(app)/[orgId]/cloud-tests/components/CloudTestsSection.tsx @@ -205,6 +205,11 @@ export function CloudTestsSection({ guidedSteps?: string[]; risk?: string; description?: string; + // Set when the finding came from AWS Security Hub — surfaces a + // disclosure banner in the Fix dialog since the AI plan is generated + // from AWS's remediation text (quality varies) and SecHub re-evaluates + // findings on its own schedule. + fromSecurityHub?: boolean; } | null>(null); const [showSetupDialog, setShowSetupDialog] = useState(false); const { hasPermission } = usePermissions(); @@ -813,6 +818,7 @@ export function CloudTestsSection({ checkResultId: finding.id, remediationKey: key, findingTitle: finding.title ?? 'Finding', + fromSecurityHub: finding.serviceId === 'security-hub', }) } onSetup={() => setShowSetupDialog(true)} @@ -918,6 +924,7 @@ export function CloudTestsSection({ checkResultId: finding.id, remediationKey: key, findingTitle: finding.title ?? 'Finding', + fromSecurityHub: finding.serviceId === 'security-hub', }) } onSetup={() => setShowSetupDialog(true)} @@ -1066,6 +1073,7 @@ export function CloudTestsSection({ guidedSteps={remediationTarget.guidedSteps} risk={remediationTarget.risk} description={remediationTarget.description} + fromSecurityHub={remediationTarget.fromSecurityHub} onComplete={() => { toast.message('Re-scanning to verify fix...'); handleRunScan(); diff --git a/apps/app/src/app/(app)/[orgId]/cloud-tests/components/RemediationDialog.tsx b/apps/app/src/app/(app)/[orgId]/cloud-tests/components/RemediationDialog.tsx index 7b6676a5a8..de9b051038 100644 --- a/apps/app/src/app/(app)/[orgId]/cloud-tests/components/RemediationDialog.tsx +++ b/apps/app/src/app/(app)/[orgId]/cloud-tests/components/RemediationDialog.tsx @@ -42,6 +42,14 @@ interface RemediationDialogProps { guidedSteps?: string[]; risk?: string; description?: string; + /** + * True when the underlying finding came from AWS Security Hub rather + * than one of our service adapters. Surfaces a disclosure banner — + * SecHub remediation text quality varies and SecHub re-evaluates + * findings on its own schedule, so the customer should verify in the + * AWS console after applying. + */ + fromSecurityHub?: boolean; onComplete?: () => void; } @@ -88,6 +96,33 @@ function RichText({ text }: { text: string }) { ); } +/** + * Disclosure shown in the Fix dialog when the underlying finding came + * from AWS Security Hub. Two things customers need to know that aren't + * true for our own adapter findings: + * + * 1. AI fix plans are generated from AWS's remediation text, which + * varies in specificity. The plan may need adjustment after + * reviewing in the AWS console. + * 2. Security Hub re-evaluates findings on its own schedule (hours, + * not minutes). After a successful Fix, the SecHub-side finding + * may take time to clear even though the underlying issue is gone. + */ +function SecurityHubFixDisclosure() { + return ( +
+

Security Hub finding

+

+ This fix was generated from AWS Security Hub remediation + guidance. Verify the result in your AWS console after applying — + Security Hub re-evaluates findings on its own schedule, so the + finding may take a few hours to clear there even after a + successful fix. +

+
+ ); +} + function CodeBlock({ code }: { code: string }) { const [copied, setCopied] = useState(false); const handleCopy = () => { @@ -270,6 +305,7 @@ export function RemediationDialog({ guidedSteps, risk, description, + fromSecurityHub, onComplete, }: RemediationDialogProps) { const [preview, setPreview] = useState(null); @@ -468,6 +504,8 @@ export function RemediationDialog({ + {fromSecurityHub && } +
{/* Applying state — shown while executing */} {isExecuting && !succeeded && !error && ( diff --git a/apps/app/src/app/(app)/[orgId]/cloud-tests/components/ScanModeSwitchDialog.tsx b/apps/app/src/app/(app)/[orgId]/cloud-tests/components/ScanModeSwitchDialog.tsx new file mode 100644 index 0000000000..c23a0066b3 --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/cloud-tests/components/ScanModeSwitchDialog.tsx @@ -0,0 +1,153 @@ +'use client'; + +import { useApi } from '@/hooks/use-api'; +import { Button } from '@trycompai/ui/button'; +import { + Dialog, + DialogContent, + DialogDescription, + DialogHeader, + DialogTitle, +} from '@trycompai/ui/dialog'; +import { Loader2 } from 'lucide-react'; +import { useState } from 'react'; +import { toast } from 'sonner'; +import { mutate as globalMutate } from 'swr'; + +import { + type AwsScanModeChoice, +} from '../../integrations/[slug]/components/AwsScanModeStep'; + +const MODE_LABEL: Record = { + comp_scanners: 'Comp AI Scanners', + security_hub: 'AWS Security Hub', +}; + +export interface ScanModeSwitchDialogProps { + open: boolean; + onOpenChange: (open: boolean) => void; + connectionId: string; + currentMode: AwsScanModeChoice; + targetMode: AwsScanModeChoice; + /** Called after a successful switch so the parent can refetch. */ + onSwitched?: () => void; +} + +/** + * Confirmation dialog for switching the AWS scan engine on an existing + * connection. Posts to `PATCH /v1/cloud-security/connections/:id/scan-mode` + * and invalidates SWR caches so the findings list + history tab pick up + * the change on the next scan. + * + * Switching is non-destructive but has real consequences worth flagging: + * 1. The next scan is a fresh baseline — reconciliation only diffs + * same-mode runs (see reconciliation.service.ts), so the customer + * shouldn't expect "resolved" events from the prior engine. + * 2. Existing exceptions stay in the database but may not match + * findings from the new engine (different checkId namespaces). + */ +export function ScanModeSwitchDialog({ + open, + onOpenChange, + connectionId, + currentMode, + targetMode, + onSwitched, +}: ScanModeSwitchDialogProps) { + const api = useApi(); + const [submitting, setSubmitting] = useState(false); + + const handleConfirm = async () => { + setSubmitting(true); + const response = await api.patch( + `/v1/cloud-security/connections/${connectionId}/scan-mode`, + { mode: targetMode }, + ); + setSubmitting(false); + + if (response.error) { + const message = + typeof response.error === 'string' + ? response.error + : 'Could not switch scan engine — please try again.'; + toast.error(message); + return; + } + + toast.success(`Scan engine switched to ${MODE_LABEL[targetMode]}`); + // Invalidate any cached findings + history for this connection so the + // UI re-fetches with the new mode on the next access. + globalMutate( + (key) => + Array.isArray(key) && + typeof key[0] === 'string' && + (key[0].startsWith('/v1/cloud-security/findings') || + key[0].startsWith('/v1/cloud-security/history') || + key[0].startsWith('/v1/cloud-security/providers')), + ); + onSwitched?.(); + onOpenChange(false); + }; + + return ( + !submitting && onOpenChange(next)}> + + + Switch scan engine? + + Change which engine produces findings for this AWS connection. + + + +
+
+
+

From

+

{MODE_LABEL[currentMode]}

+
+ → +
+

To

+

{MODE_LABEL[targetMode]}

+
+
+ +
+

What changes:

+
    +
  • + The next scan is a fresh baseline — resolutions and + regressions only compare same-engine runs. +
    • +
  • + Existing exceptions stay marked but may not match findings + from the new engine (different control identifiers). +
    • +
  • + In-flight remediations from the previous engine continue + independently. +
    • +
+
+
+ +
+ + +
+ + + ); +} diff --git a/apps/app/src/app/(app)/[orgId]/integrations/[slug]/components/AwsScanModeStep.test.tsx b/apps/app/src/app/(app)/[orgId]/integrations/[slug]/components/AwsScanModeStep.test.tsx new file mode 100644 index 0000000000..a0798133ed --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/integrations/[slug]/components/AwsScanModeStep.test.tsx @@ -0,0 +1,74 @@ +import { render, screen, fireEvent } from '@testing-library/react'; +import { describe, expect, it, vi } from 'vitest'; +import { + AwsScanModeStep, + DEFAULT_AWS_SCAN_MODE_CHOICE, +} from './AwsScanModeStep'; + +describe('AwsScanModeStep', () => { + it('renders both scan engine options', () => { + render( + {}} />, + ); + expect(screen.getByText('Comp AI Scanners')).toBeInTheDocument(); + expect(screen.getByText('AWS Security Hub')).toBeInTheDocument(); + }); + + it('marks the value prop as selected (controlled component)', () => { + const { rerender } = render( + {}} />, + ); + + const compRadio = screen.getByRole('radio', { name: /comp ai scanners/i }); + const sechubRadio = screen.getByRole('radio', { name: /aws security hub/i }); + expect(compRadio).toBeChecked(); + expect(sechubRadio).not.toBeChecked(); + + rerender( {}} />); + expect(compRadio).not.toBeChecked(); + expect(sechubRadio).toBeChecked(); + }); + + it('calls onChange with the new mode when the customer picks the other option', () => { + const onChange = vi.fn(); + render(); + + fireEvent.click( + screen.getByRole('radio', { name: /aws security hub/i }), + ); + expect(onChange).toHaveBeenCalledWith('security_hub'); + }); + + it('does not call onChange when re-selecting the same option', () => { + // Defensive — re-clicking the selected radio fires onChange with the + // same value (browser semantics), which is fine but worth noting. + const onChange = vi.fn(); + render(); + + // Click on the already-selected one — radio onChange does NOT fire + // when re-selecting the checked option, so onChange stays at 0. + fireEvent.click( + screen.getByRole('radio', { name: /comp ai scanners/i }), + ); + expect(onChange).not.toHaveBeenCalled(); + }); + + it('disables all radios when the disabled prop is true', () => { + render( + {}} + disabled + />, + ); + const radios = screen.getAllByRole('radio'); + radios.forEach((radio) => expect(radio).toBeDisabled()); + }); + + it('exposes a single source of truth for the default mode', () => { + // Guarded with a test so changing the default is intentional — + // shifting it would silently flip every new connection's scan + // engine choice. + expect(DEFAULT_AWS_SCAN_MODE_CHOICE).toBe('comp_scanners'); + }); +}); diff --git a/apps/app/src/app/(app)/[orgId]/integrations/[slug]/components/AwsScanModeStep.tsx b/apps/app/src/app/(app)/[orgId]/integrations/[slug]/components/AwsScanModeStep.tsx new file mode 100644 index 0000000000..13e22bf944 --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/integrations/[slug]/components/AwsScanModeStep.tsx @@ -0,0 +1,146 @@ +'use client'; + +import { Cloud, ShieldCheck } from 'lucide-react'; + +/** + * String constants for the two AWS scan-engine modes. Mirrors the + * AwsScanMode type in apps/api/src/cloud-security/aws-scan-mode.ts — + * the single source of truth for these values lives on the API side + * (because that's what gets validated server-side), but we duplicate + * the literals here so the onboarding UI doesn't import API code. + * + * If you add a new mode, update BOTH files and the type assertion in + * the parent EmptyStateOnboarding that passes this to createConnection. + */ +export type AwsScanModeChoice = 'comp_scanners' | 'security_hub'; + +export const DEFAULT_AWS_SCAN_MODE_CHOICE: AwsScanModeChoice = 'comp_scanners'; + +interface AwsScanModeStepProps { + /** Currently-selected mode. */ + value: AwsScanModeChoice; + /** Called when the customer picks a different option. */ + onChange: (next: AwsScanModeChoice) => void; + /** When true, disables interaction (e.g., during connection creation). */ + disabled?: boolean; +} + +/** + * AWS-onboarding step that lets the customer choose which engine + * performs their cloud security scans. Two radio cards, mutually + * exclusive: + * + * 1. Comp AI Scanners (default) — our service adapters, free, + * full Fix button support. + * 2. AWS Security Hub — read findings from the customer's existing + * Security Hub deployment, surface native NIST 800-53 / CIS / PCI + * mapping, Fix button available with a small disclosure. + * + * Pure UI — no API calls, no side effects. The parent owns state and + * passes the choice into the createConnection payload via the + * `awsScanMode` variable. + */ +export function AwsScanModeStep({ + value, + onChange, + disabled, +}: AwsScanModeStepProps) { + return ( +
+ AWS scan engine + onChange('comp_scanners')} + icon={} + title="Comp AI Scanners" + subtitle="Recommended" + description={ + 'Run our security checks directly against your AWS account. ' + + 'No additional AWS cost. Full Fix button support. Covers IAM, ' + + 'CloudTrail, S3, EC2, RDS, KMS, GuardDuty, and 40+ other services.' + } + badges={['Free', 'Fix button supported']} + /> + onChange('security_hub')} + icon={} + title="AWS Security Hub" + subtitle="Read findings from your existing Security Hub" + description={ + 'Use the findings your Security Hub deployment already produces. ' + + 'Surfaces NIST 800-53 / CIS / PCI control mappings natively. ' + + 'Requires Security Hub to be enabled in your AWS account ' + + '(AWS bills per finding ingested + per control checked).' + } + badges={[ + 'Native NIST / CIS / PCI mapping', + 'Requires Security Hub subscription', + ]} + /> +
+ ); +} + +function ScanModeCard({ + modeValue, + selected, + onSelect, + icon, + title, + subtitle, + description, + badges, +}: { + modeValue: AwsScanModeChoice; + selected: boolean; + onSelect: () => void; + icon: React.ReactNode; + title: string; + subtitle: string; + description: string; + badges: string[]; +}) { + return ( + + ); +} diff --git a/apps/app/src/app/(app)/[orgId]/integrations/[slug]/components/EmptyStateOnboarding.tsx b/apps/app/src/app/(app)/[orgId]/integrations/[slug]/components/EmptyStateOnboarding.tsx index 18091edcf2..f5a741f460 100644 --- a/apps/app/src/app/(app)/[orgId]/integrations/[slug]/components/EmptyStateOnboarding.tsx +++ b/apps/app/src/app/(app)/[orgId]/integrations/[slug]/components/EmptyStateOnboarding.tsx @@ -14,6 +14,11 @@ import { import { ArrowRight, Shield } from 'lucide-react'; import { useCallback, useMemo, useState } from 'react'; import { toast } from 'sonner'; +import { + AwsScanModeStep, + DEFAULT_AWS_SCAN_MODE_CHOICE, + type AwsScanModeChoice, +} from './AwsScanModeStep'; // ─── Primitives ───────────────────────────────────────────────────────── @@ -497,6 +502,12 @@ function CloudSetup({ const [connecting, setConnecting] = useState(false); const [credentials, setCredentials] = useState>({}); const [errors, setErrors] = useState>({}); + // AWS only — which scan engine the customer is choosing for this + // connection. Sent in createConnection's credentials payload as the + // `awsScanMode` variable, then read on every scan in cloud-security.service. + const [awsScanMode, setAwsScanMode] = useState( + DEFAULT_AWS_SCAN_MODE_CHOICE, + ); const allFields = provider.credentialFields ?? []; const visibleFields = allFields.filter( @@ -525,6 +536,13 @@ function CloudSetup({ const arnMatch = String(finalCredentials.roleArn ?? '').match(/:(\d{12}):/); finalCredentials.connectionName = arnMatch ? `AWS ${arnMatch[1]}` : 'AWS Account'; } + // AWS only — persist the customer's scan engine choice on the + // connection. The scan service reads this from variables on every + // run. resolveAwsScanMode() handles the missing-field case for + // every other provider and for pre-feature historical connections. + if (provider.id === 'aws') { + finalCredentials.awsScanMode = awsScanMode; + } const newErrors: Record = {}; for (const field of allFields) { @@ -558,7 +576,7 @@ function CloudSetup({ } finally { setConnecting(false); } - }, [allFields, credentials, createConnection, provider, orgId, onConnected]); + }, [allFields, awsScanMode, credentials, createConnection, provider, orgId, onConnected]); const connectionFields = visibleFields.filter( (f) => f.id !== 'remediationRoleArn' && f.id !== 'regions' && f.id !== 'awsType', @@ -601,25 +619,42 @@ function CloudSetup({ > {/* ─── Left: Unified setup flow ─── */}
- {awsTypeFields.length > 0 && ( + {/* Step 1 — AWS only — Scan engine choice. Mutually exclusive + between Comp AI scanners (default) and AWS Security Hub. + Customers can switch later from CloudSettingsModal. */} + {provider.id === 'aws' && (
- {/* Step 1 */} - - {awsTypeFields.map((field) => ( - updateCredential(field.id, v)} - /> - ))} + +
)} - {/* Step 2 */} + + {/* Step 2 — AWS only — Commercial vs GovCloud */} + {awsTypeFields.length > 0 && ( + <> + {provider.id === 'aws' &&
} +
+ + {awsTypeFields.map((field) => ( + updateCredential(field.id, v)} + /> + ))} +
+ + )} + {/* Step 3 — AWS only — IAM role creation script */} {provider.setupScript && (
- + - {/* Step 3 */} + {/* Step 4 — Connection details (Role ARN etc.) */}
- + {connectionFields.map((field) => ( - {/* Step 4 */} + {/* Step 5 — Scan configuration (regions) */} {regionFields.length > 0 && ( <>
- + {regionFields.map((field) => ( Date: Tue, 19 May 2026 14:15:20 -0400 Subject: [PATCH 2/6] fix(cloud-tests): backfill scanMode on pre-feature AWS runs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Without the backfill, existing customers' first post-deploy scan would treat itself as a fresh baseline — the new run's scanMode='comp_scanners' doesn't match prior runs' scanMode=NULL in Prisma's WHERE clause, so reconciliation finds no matching prior run and skips one cycle of resolution / regression history events. Migration now adds a one-time UPDATE that labels every pre-feature AWS cloud-security run (checkId = 'aws-security-scan') as 'comp_scanners'. That label is truthful — comp_scanners was the only mode that existed before this PR. GCP / Azure runs stay NULL (no scan-mode concept). Verified the checkId pattern is precise: - 'aws-security-scan' is the only checkId used by cloud-security AWS scans (cloud-security.service.ts:619, grep-confirmed) - Other IntegrationCheckRun rows (checkId='all', task-based checks) don't have a scan-mode concept and correctly stay NULL - Legacy V1 scans live in IntegrationResult, not affected Test coverage: new reconciliation test "reconciles comp_scanners runs against backfilled comp_scanners history (post-deploy continuity)" locks in the behavior so existing customers do NOT skip History events on their first post-deploy scan. 14 reconciliation tests pass (was 13). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../reconciliation.service.spec.ts | 52 +++++++++++++++++-- .../migration.sql | 21 ++++++++ .../prisma/schema/integration-platform.prisma | 11 ++-- 3 files changed, 77 insertions(+), 7 deletions(-) diff --git a/apps/api/src/cloud-security/reconciliation.service.spec.ts b/apps/api/src/cloud-security/reconciliation.service.spec.ts index 4d8041dfc6..5fe1664734 100644 --- a/apps/api/src/cloud-security/reconciliation.service.spec.ts +++ b/apps/api/src/cloud-security/reconciliation.service.spec.ts @@ -333,9 +333,11 @@ describe('CloudReconciliationService.reconcile', () => { ); }); - it('passes null scanMode for legacy / non-AWS runs so they reconcile against each other', async () => { - // Pre-feature runs and GCP/Azure runs have scanMode = null. They - // must still reconcile against each other (null === null). + it('passes null scanMode for GCP / Azure runs so they reconcile against each other', async () => { + // GCP / Azure runs have scanMode = null (no scan-mode concept). They + // must reconcile against each other (null === null). AWS rows + // ALWAYS have an explicit scanMode — pre-feature rows were + // backfilled to 'comp_scanners' by the schema migration. dbMock.integrationCheckRun.findUnique.mockResolvedValueOnce({ id: 'icr_current', connectionId: 'icn_gcp', @@ -359,6 +361,50 @@ describe('CloudReconciliationService.reconcile', () => { ); }); + it('reconciles comp_scanners runs against backfilled comp_scanners history (post-deploy continuity)', async () => { + // After the deploy, every existing AWS customer's first scan should + // continue diffing against their prior history seamlessly. The + // migration backfills pre-feature AWS rows to 'comp_scanners', so + // the new scan (also 'comp_scanners') finds matching prior runs. + // Locks in that we DO NOT skip History events on the first post- + // deploy scan for existing customers. + setupRuns({ + currentResults: [ + makeResult({ findingKey: 'iam-no-mfa-john', resourceId: 'john', passed: true }), + ], + priorResults: [ + makeResult({ findingKey: 'iam-no-mfa-john', resourceId: 'john', passed: false }), + ], + }); + // Override findUnique to set scanMode explicitly on the current run. + dbMock.integrationCheckRun.findUnique.mockReset(); + dbMock.integrationCheckRun.findUnique.mockResolvedValueOnce({ + id: 'icr_current', + connectionId: 'icn_aws', + status: 'success', + startedAt: CURRENT_RUN_TIME, + completedAt: CURRENT_RUN_TIME, + scannedServices: [], + scanMode: 'comp_scanners', + connection: { organizationId: 'org_1' }, + results: [ + makeResult({ findingKey: 'iam-no-mfa-john', resourceId: 'john', passed: true }), + ], + }); + + const service = new CloudReconciliationService(makeExceptionsStub()); + const result = await service.reconcile({ currentRunId: 'icr_current' }); + + expect(dbMock.integrationCheckRun.findFirst).toHaveBeenCalledWith( + expect.objectContaining({ + where: expect.objectContaining({ scanMode: 'comp_scanners' }), + }), + ); + // Resolution should still be detected — backfilled prior run had + // scanMode='comp_scanners', matches the new run's scanMode. + expect(result.resolutions).toBe(1); + }); + it('returns 0/0 when no prior run of the same mode exists (post-switch baseline)', async () => { // After a customer switches modes, the next scan finds no matching // prior run and returns a clean baseline. This is the same code diff --git a/packages/db/prisma/migrations/20260519143715_integration_check_run_scan_mode/migration.sql b/packages/db/prisma/migrations/20260519143715_integration_check_run_scan_mode/migration.sql index 3cc2b41a41..e4069d45b6 100644 --- a/packages/db/prisma/migrations/20260519143715_integration_check_run_scan_mode/migration.sql +++ b/packages/db/prisma/migrations/20260519143715_integration_check_run_scan_mode/migration.sql @@ -1,2 +1,23 @@ -- AlterTable ALTER TABLE "IntegrationCheckRun" ADD COLUMN "scanMode" TEXT; + +-- Backfill: label pre-feature AWS cloud-security runs as 'comp_scanners'. +-- That was the only scan engine in existence when those runs executed, so +-- the label is truthful — we're filling in metadata that didn't exist +-- before, not changing semantics. Lets the new reconciliation lookup +-- (which scopes by scanMode) match historical AWS runs against the first +-- post-deploy scan instead of treating it as a fresh baseline and +-- silently dropping one cycle of resolution / regression events. +-- +-- WHY this WHERE clause is precise: +-- - 'aws-security-scan' is the only checkId pattern used by cloud- +-- security scans (cloud-security.service.ts:619 — verified by grep). +-- - Other IntegrationCheckRun rows (connection-level checks with +-- checkId='all', task-based checks with manifest IDs) do NOT have a +-- scan-mode concept and correctly stay NULL. +-- - GCP / Azure cloud-security runs ('gcp-security-scan' / +-- 'azure-security-scan') also stay NULL — scan mode is AWS-only. +UPDATE "IntegrationCheckRun" +SET "scanMode" = 'comp_scanners' +WHERE "checkId" = 'aws-security-scan' + AND "scanMode" IS NULL; diff --git a/packages/db/prisma/schema/integration-platform.prisma b/packages/db/prisma/schema/integration-platform.prisma index 9b5426c784..b19823266b 100644 --- a/packages/db/prisma/schema/integration-platform.prisma +++ b/packages/db/prisma/schema/integration-platform.prisma @@ -347,11 +347,14 @@ model IntegrationCheckRun { failedServices String[] @default([]) /// AWS-only: which scan engine produced this run. - /// 'comp_scanners' — our 49 service adapters (today's default). + /// 'comp_scanners' — our service adapters (default). /// 'security_hub' — AWS Security Hub findings. - /// Null for non-AWS runs and pre-feature historical rows. Reconciliation - /// uses this to only diff like-for-like — findingKeys live in different - /// namespaces across modes, so cross-mode diffs would produce false + /// Null for GCP / Azure runs (no scan-mode concept). Pre-feature AWS + /// rows were backfilled to 'comp_scanners' in the + /// 20260519143715_integration_check_run_scan_mode migration, so every + /// AWS run has an explicit value. Reconciliation uses this to only + /// diff like-for-like — findingKeys live in different namespaces + /// across modes, so cross-mode diffs would produce false /// resolutions/regressions. scanMode String? From 57a4077821f75b19654cc8402f4594e12afbe87d Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Tue, 19 May 2026 15:25:28 -0400 Subject: [PATCH 3/6] fix(cloud-tests): remove no-op .replace('dss', 'dss') in normalizeStandardName MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeQL flagged this as "Replacement of a substring with itself" — and it's right. The line was a leftover comment-via-code that no longer serves any purpose (toLowerCase already handles the casing). Removing it makes the intent of the function clearer and silences the static- analysis warning. 23 security-hub.adapter tests still pass — behavior unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/cloud-security/providers/aws/security-hub.adapter.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 d44a3aa6ac..6cd77cd71d 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 @@ -287,10 +287,11 @@ function formatSingleRequirement(requirement: string): string { function normalizeStandardName(value: string): string { // Compact common multi-word framework names so chips render cleanly. + // ("PCI DSS" → "pci dss" via lowercase; the full "DSS" token stays + // as-is intentionally — the parser handles whitespace-separated parts.) return value .toLowerCase() .replace(/aws foundations benchmark/g, '') - .replace(/dss/g, 'dss') .replace(/foundational security best practices/g, 'fsbp') .replace(/\s+/g, ' ') .trim() From 744814fb344e4f6a754d569ea69698b915026645 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Tue, 19 May 2026 15:53:44 -0400 Subject: [PATCH 4/6] fix(cloud-tests): deriveFindingKey must namespace by Security Hub standard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cubic flagged a real P1: the original implementation used only the trailing control segment, so distinct findings from different SecHub standards collided under the same key whenever two standards shared a control identifier. Concrete example: CIS 1.2.0 / 1.1 → 'aws-securityhub-1.1' PCI 3.2.1 / 1.1 → 'aws-securityhub-1.1' ← same key, different check In production this would have silently merged unrelated findings into one UI row, scoped exceptions to the wrong control, and confused reconciliation. Real data-integrity risk, not cosmetic. Fix: combine the standard prefix (first segment) with the control identifier (last segment). Middle segments (version / revision) are dropped intentionally — they shouldn't fragment a finding's identity within a standard when SecHub bumps the standard version. cis-aws-foundations-benchmark/v/1.2.0/1.1 → aws-securityhub-cis-aws-foundations-benchmark-1.1 pci-dss/v/3.2.1/1.1 → aws-securityhub-pci-dss-1.1 ← no longer collides cis-aws-foundations-benchmark/v/4.0.0/1.1 → aws-securityhub-cis-aws-foundations-benchmark-1.1 ← stable across versions Tests: - 23 → 26 adapter tests; added explicit collision-prevention guard + version-stability guard + single-segment fallback test. Identified by cubic. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../aws/security-hub.adapter.spec.ts | 57 +++++++++++++++---- .../providers/aws/security-hub.adapter.ts | 42 +++++++++++--- 2 files changed, 79 insertions(+), 20 deletions(-) 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 d78d1a4aea..e294b1d340 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 @@ -9,36 +9,69 @@ import { describe('security-hub.adapter helpers', () => { describe('deriveFindingKey', () => { - it('extracts the trailing control id from a foundational best-practices GeneratorId', () => { + it('combines standard + control for a foundational best-practices GeneratorId', () => { expect( deriveFindingKey('aws-foundational-security-best-practices/v/1.0.0/EC2.13'), - ).toBe('aws-securityhub-ec2.13'); + ).toBe('aws-securityhub-aws-foundational-security-best-practices-ec2.13'); }); - it('extracts the trailing control id from a CIS GeneratorId', () => { + it('combines standard + control for a CIS GeneratorId', () => { expect( deriveFindingKey('cis-aws-foundations-benchmark/v/1.2.0/1.1'), - ).toBe('aws-securityhub-1.1'); + ).toBe('aws-securityhub-cis-aws-foundations-benchmark-1.1'); }); - it('extracts the trailing control id from a NIST GeneratorId', () => { + it('combines standard + control for a NIST GeneratorId', () => { expect(deriveFindingKey('nist-800-53/r/5/AC-2')).toBe( - 'aws-securityhub-ac-2', + 'aws-securityhub-nist-800-53-ac-2', ); }); - it('sanitizes characters not safe for use in identifiers', () => { - expect(deriveFindingKey('weird:control id with spaces!')).toMatch( - /^aws-securityhub-/, + it('does NOT collide across standards that share a control id (P1 regression guard)', () => { + // The original implementation used only the trailing segment, so + // CIS 1.1 and PCI 1.1 produced the same findingKey and would have + // merged into one row in the UI — silently corrupting exception + // scoping and reconciliation. Cubic caught this on initial review; + // this test exists to prevent regression. + const cis11 = deriveFindingKey( + 'cis-aws-foundations-benchmark/v/1.2.0/1.1', ); + const pci11 = deriveFindingKey('pci-dss/v/3.2.1/1.1'); + expect(cis11).not.toBe(pci11); + expect(cis11).toContain('cis'); + expect(pci11).toContain('pci'); }); - it('produces a stable key (no timestamps / randomness) so reconciliation can diff scans', () => { + it('produces the same key for two findings from the same standard + control', () => { + // Findings sharing both standard AND control SHOULD merge — that's + // the whole point of findingKey: group instances of the same check + // across resources. const a = deriveFindingKey('aws-foundational-security-best-practices/v/1.0.0/EC2.13'); const b = deriveFindingKey('aws-foundational-security-best-practices/v/1.0.0/EC2.13'); expect(a).toBe(b); }); + it('ignores version/revision changes within the same standard + control', () => { + // SecHub bumping the standard version shouldn't fragment a finding's + // identity. CIS 1.2.0 → 4.0.0 with the same control 1.1 stays one key, + // so reconciliation can still diff before/after the version bump. + const v12 = deriveFindingKey('cis-aws-foundations-benchmark/v/1.2.0/1.1'); + const v40 = deriveFindingKey('cis-aws-foundations-benchmark/v/4.0.0/1.1'); + expect(v12).toBe(v40); + }); + + it('sanitizes characters not safe for use in identifiers', () => { + expect(deriveFindingKey('weird:control id with spaces!')).toMatch( + /^aws-securityhub-/, + ); + }); + + it('handles single-segment GeneratorIds (no path structure)', () => { + expect(deriveFindingKey('SimpleGenerator')).toBe( + 'aws-securityhub-simplegenerator', + ); + }); + it('returns a sentinel key rather than throwing when GeneratorId is missing', () => { // We must always produce SOME key — the Fix pipeline gates on // findingKey existence, and silently disabling Fix on findings @@ -160,7 +193,9 @@ describe('security-hub.adapter helpers', () => { it('stamps evidence.findingKey so the Fix pipeline picks it up', () => { const mapped = mapSecurityHubFinding(baseFinding, 'us-east-1'); - expect(mapped.evidence?.findingKey).toBe('aws-securityhub-ec2.13'); + expect(mapped.evidence?.findingKey).toBe( + 'aws-securityhub-aws-foundational-security-best-practices-ec2.13', + ); }); it('stamps evidence.serviceId so the UI can detect SecHub findings', () => { 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 6cd77cd71d..8036d3b4be 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 @@ -177,22 +177,46 @@ export function mapSecurityHubFinding( * `aws-foundational-security-best-practices/v/1.0.0/EC2.13` or * `cis-aws-foundations-benchmark/v/1.2.0/1.1`. * - * Strategy: take the trailing control identifier (last `/`-delimited - * segment) since that's what makes the finding semantically unique. - * Falls back to a sanitized form of the full GeneratorId, then to a - * generic key, so we ALWAYS produce a key — the Fix button gates on - * its existence, so missing findingKey would silently disable Fix. + * Strategy: combine the standard prefix (first segment) with the control + * identifier (last segment) so distinct findings from different standards + * never collide under the same key. The middle segments (version / + * revision) are intentionally dropped — they shouldn't change a finding's + * identity within a standard. + * + * aws-foundational-security-best-practices/v/1.0.0/EC2.13 + * → aws-securityhub-aws-foundational-security-best-practices-ec2.13 + * + * cis-aws-foundations-benchmark/v/1.2.0/1.1 + * → aws-securityhub-cis-aws-foundations-benchmark-1.1 + * + * pci-dss/v/3.2.1/1.1 + * → aws-securityhub-pci-dss-1.1 (no collision with the CIS 1.1 above) + * + * Falls back gracefully when the GeneratorId has no path structure or is + * missing entirely — we ALWAYS produce a key because the Fix button gates + * on its existence, and missing findingKey would silently disable Fix. */ export function deriveFindingKey(generatorId: string | undefined): string { if (!generatorId) return 'aws-securityhub-unknown'; const trimmed = generatorId.trim(); if (!trimmed) return 'aws-securityhub-unknown'; - const lastSegment = trimmed.split('/').pop() ?? trimmed; - const sanitized = sanitizeKeySegment(lastSegment); - if (sanitized) return `aws-securityhub-${sanitized}`; + const segments = trimmed.split('/').filter((s) => s.length > 0); + if (segments.length === 0) return 'aws-securityhub-unknown'; + + if (segments.length === 1) { + // No path structure — sanitize the whole thing. + const sanitized = sanitizeKeySegment(segments[0]); + return `aws-securityhub-${sanitized || 'unknown'}`; + } - return `aws-securityhub-${sanitizeKeySegment(trimmed) || 'unknown'}`; + // Combine first segment (standard identifier — namespaces the key) with + // the last segment (control identifier — uniquely identifies WHICH check + // within that standard). This prevents collisions like CIS 1.1 ↔ PCI 1.1. + const standard = sanitizeKeySegment(segments[0]); + const control = sanitizeKeySegment(segments[segments.length - 1]); + const combined = [standard, control].filter(Boolean).join('-'); + return `aws-securityhub-${combined || 'unknown'}`; } function sanitizeKeySegment(value: string): string { From 8ec0eea4a3b746d949babd721cde6a33ccc25c3e Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Tue, 19 May 2026 16:18:10 -0400 Subject: [PATCH 5/6] fix(cloud-tests): gate AWS scan-mode switch by integration:update, not delete MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cubic flagged a real P2 in the merged PR #2871 — the AWS scan-mode switcher in CloudSettingsModal was gated by `integration:delete` (because `canEdit={canDelete}` was passed to AwsScanModeSection), but the corresponding API endpoint requires `integration:update`. Users with update permission but without delete permission could not see the Switch button even though the API would accept their request. Fix: thread a `canUpdate = hasPermission('integration', 'update')` through CloudSettingsModal → ConnectionTab → AwsScanModeSection, and use it for the scan-mode switch. Disconnect button keeps using `canDelete` (correct — it IS a delete action). Tests: - Regression guards added: switch button shows for admin (has both perms), hides for auditor (read-only), and crucially DOES show for update-only users while still hiding Disconnect from them — the exact scenario cubic flagged. - 3 new tests pass; 2 pre-existing failures unchanged (string expectations `Manage Cloud Connections` / `Connection Status` don't match current component, present before this PR). Identified by cubic. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../components/CloudSettingsModal.test.tsx | 33 +++++++++++++++++++ .../components/CloudSettingsModal.tsx | 18 ++++++++-- 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/apps/app/src/app/(app)/[orgId]/cloud-tests/components/CloudSettingsModal.test.tsx b/apps/app/src/app/(app)/[orgId]/cloud-tests/components/CloudSettingsModal.test.tsx index d2a8ae74a6..55d7c89b99 100644 --- a/apps/app/src/app/(app)/[orgId]/cloud-tests/components/CloudSettingsModal.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/cloud-tests/components/CloudSettingsModal.test.tsx @@ -211,4 +211,37 @@ describe('CloudSettingsModal permission gating', () => { render(); expect(screen.queryByTestId('dialog')).not.toBeInTheDocument(); }); + + it('shows the AWS scan-mode switch button when the user has integration:update', () => { + // Regression guard for the cubic P2: the switch button is an UPDATE + // operation and must be gated by integration:update, NOT + // integration:delete. Admin has both, so the button shows. + setMockPermissions(ADMIN_PERMISSIONS); + render(); + expect( + screen.getByRole('button', { name: /switch to/i }), + ).toBeInTheDocument(); + }); + + it('hides the AWS scan-mode switch button for users without integration:update', () => { + // Auditor has read-only permissions — should not see Switch. + setMockPermissions(AUDITOR_PERMISSIONS); + render(); + expect( + screen.queryByRole('button', { name: /switch to/i }), + ).not.toBeInTheDocument(); + }); + + it('shows the AWS scan-mode switch for update-only users (NOT delete users)', () => { + // The exact regression cubic flagged: a user with integration:update + // but WITHOUT integration:delete must still see the Switch button, + // because the API gates this endpoint on integration:update only. + setMockPermissions({ integration: ['read', 'update'] }); + render(); + expect( + screen.getByRole('button', { name: /switch to/i }), + ).toBeInTheDocument(); + // ...and they correctly DO NOT see Disconnect (delete permission required). + expect(screen.queryByText('Disconnect')).not.toBeInTheDocument(); + }); }); diff --git a/apps/app/src/app/(app)/[orgId]/cloud-tests/components/CloudSettingsModal.tsx b/apps/app/src/app/(app)/[orgId]/cloud-tests/components/CloudSettingsModal.tsx index 7ef7bdd174..ae35262cb6 100644 --- a/apps/app/src/app/(app)/[orgId]/cloud-tests/components/CloudSettingsModal.tsx +++ b/apps/app/src/app/(app)/[orgId]/cloud-tests/components/CloudSettingsModal.tsx @@ -63,6 +63,12 @@ export function CloudSettingsModal({ const api = useApi(); const { hasPermission } = usePermissions(); const canDelete = hasPermission('integration', 'delete'); + // Scan-mode switch is an UPDATE operation (the API endpoint is + // gated by integration:update — see CloudSecurityController.updateAwsScanMode), + // so the UI must use update permission, NOT delete permission. Using + // canDelete here would silently block valid update users from seeing + // the "Switch" button even though the API would accept their request. + const canUpdate = hasPermission('integration', 'update'); const [activeProvider, setActiveProvider] = useState(connectedProviders[0]?.connectionId || ''); const [isDeleting, setIsDeleting] = useState(false); const { deleteConnection } = useIntegrationMutations(); @@ -129,6 +135,7 @@ export function CloudSettingsModal({ @@ -143,11 +150,13 @@ export function CloudSettingsModal({ function ConnectionTab({ provider, canDelete, + canUpdate, isDeleting, onDisconnect, }: { provider: CloudProvider; canDelete: boolean; + canUpdate: boolean; isDeleting: boolean; onDisconnect: (p: CloudProvider) => void; }) { @@ -185,9 +194,14 @@ function ConnectionTab({ {/* AWS-only — scan engine switcher. Lets the customer change between Comp AI scanners and Security Hub on an existing connection. Surfaces the current mode + a "Change" button that opens - ScanModeSwitchDialog with the right confirmation copy. */} + ScanModeSwitchDialog with the right confirmation copy. + + Gated by `canUpdate` (integration:update) — matches the API + endpoint at PATCH /v1/cloud-security/connections/:id/scan-mode. + Using canDelete here would silently block users who legitimately + have update permission. */} {provider.id === 'aws' && !provider.isLegacy && ( - + )} {canDelete && ( From a5d15397853bdade8c026e187a95bacd996f319a Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Tue, 19 May 2026 17:03:00 -0400 Subject: [PATCH 6/6] 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 95701b206c..90670baf5b 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 3907878727..3539c17a74 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 fd92285ef6..0b62395977 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 e294b1d340..34512060ec 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 8036d3b4be..eca2bc2565 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; }