Conversation
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
…ndardName 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) <noreply@anthropic.com>
…mp into tofik/aws-security-hub-mode
…ndard
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) <noreply@anthropic.com>
feat(cloud-tests): AWS Security Hub as alternative scan engine
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 24 files
Confidence score: 3/5
- There is a concrete permission regression risk in
apps/app/src/app/(app)/[orgId]/cloud-tests/components/CloudSettingsModal.tsx: the AWS scan-mode switch is gated byintegration:deleteinstead ofintegration:update. - Because this is severity 6/10 with high confidence (9/10) and directly affects authorized users’ ability to change scan mode, it carries real user-facing impact if merged as-is.
- This looks isolated to UI authorization logic, so the PR may still be close, but it should be corrected before merge to avoid blocking expected update workflows.
- Pay close attention to
apps/app/src/app/(app)/[orgId]/cloud-tests/components/CloudSettingsModal.tsx- permission check mismatch can prevent valid users from switching scan mode.
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
…t delete 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) <noreply@anthropic.com>
…n-gate fix(cloud-tests): gate AWS scan-mode switch by integration:update, not delete
Contributor
|
@cubic-dev-ai review it |
Contributor
@tofikwest I have started the AI code review. It will take a few minutes to complete. |
Contributor
There was a problem hiding this comment.
2 issues found across 24 files
Confidence score: 3/5
- There is a concrete regression risk in
apps/api/src/cloud-security/providers/aws/security-hub.adapter.ts: the current first capture group excludes lowercase letters and hyphens, so several documented Security Hub control formats (NIST, CIS, AWS FSBP) may fail to parse and fall back to raw strings. - The DTO issue in
apps/api/src/cloud-security/dto/update-scan-mode.dto.tsis lower severity, but duplicating scan-mode literals against the stated source-of-truth guidance increases drift and future maintenance risk. - Given one medium-severity, high-confidence parsing bug with likely user-visible impact, this is not fully low-risk yet, though it looks straightforward to address.
- Pay close attention to
apps/api/src/cloud-security/providers/aws/security-hub.adapter.ts,apps/api/src/cloud-security/dto/update-scan-mode.dto.ts- fix regex coverage for documented formats and align scan-mode values with the canonical export.
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
…ode literals
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) <noreply@anthropic.com>
5 tasks
…egex-and-dto-source fix(cloud-tests): parse all SecHub compliance formats + dedupe scan-mode literals
Contributor
|
@cubic-dev-ai review it |
Contributor
@tofikwest I have started the AI code review. It will take a few minutes to complete. |
Contributor
|
🎉 This PR is included in version 3.59.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is an automated pull request to release the candidate branch into production, which will trigger a deployment.
It was created by the [Production PR] action.
Summary by cubic
Adds AWS Security Hub as an alternative scan engine for AWS connections. Users can choose the engine at connect time and switch later; existing connections keep today’s behavior by default. Also fixes compliance chip parsing for Security Hub findings and unifies scan‑mode constants.
New Features
metadata.awsScanMode, persists per-runscanModeonIntegrationCheckRun; reconciliation only compares runs with the same mode.PATCH /v1/cloud-security/connections/:connectionId/scan-modeto switch modes; emitsscan_mode_changedaudit event.GetFindingswith caps, stable findingKey (standard+control), remediation text with “More info” and “Compliance” sections, stampsserviceId: security-hub, and handles not-subscribed/AccessDenied gracefully.scanModecolumn; backfills pre-feature AWS runs (checkId='aws-security-scan') to'comp_scanners'.Bug Fixes
AWS_SCAN_MODESto prevent drift.integration:update(notdelete), so update-only users can switch modes.Written for commit 6059fcc. Summary will update on new commits. Review in cubic