From 34566555f85845a4d7e6bf7fe6ad25fe82851a20 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Wed, 13 May 2026 18:32:44 -0400 Subject: [PATCH 01/15] feat(cloud-tests): auditor visibility improvements (phases 1-5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the depth and audit trail Chris asked for so auditors can trust the automated checks. Five phases shipped together: Phase 1 — Quick wins - Evidence JSON viewer on each finding (sensitive keys redacted via evidence-sanitizer.ts — suffix-match strategy preserves booleans/numbers) - Last-scan metadata strip (47 checks, 41 passed, 6 failed, 3.2s) - Resource labels on every finding row (IAM User: john, S3 Bucket: …) - Rename "Security Findings" header + tab to "Scan Results" Phase 2 — AI check descriptions - Tier 3 panel (what / pass / fail / why) generated lazily on first expand - Claude Haiku 4.5 via @ai-sdk/anthropic, cached per (orgId, checkId) with source-hash invalidation (regenerates when adapter strings change) - Server-side strip of compliance control numbers + URLs as a backstop - GCP/Azure: passthrough from provider catalog (no AI call) Phase 3 — Per-check sub-grouping - Findings within a service grouped by normalized checkKey - "X of Y failing" headers; "Show all results" reveals passing rows - 100-row display cap per check with "Show more" affordance Phase 4 — Comments on auditor findings - CommentsPermissionGuard resolves entityType-specific permission so auditors (finding:update, no task:update) can comment on findings - Comments thread on /overview/findings detail sheet Phase 5 — Resolution + exceptions history - FindingException / FindingResolution / FindingRegression tables - Reconciliation classifies each resolution: platform_fix (RemediationAction matched), external_fix, resource_deleted, or exception_marked - scannedServices column on IntegrationCheckRun prevents false "resolved" events on partial scans - "Mark as exception" modal: required reason (20+ chars), optional reviewer, optional auto-review date - New History tab: summary + resolutions + active exceptions + regressions Includes 96 new tests (59 api + 37 frontend) and 3 Prisma migrations. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../ai-description.prompt.spec.ts | 144 +++++++ .../cloud-security/ai-description.prompt.ts | 128 ++++++ .../cloud-security/ai-description.service.ts | 59 +++ .../check-definition-provider-passthrough.ts | 89 ++++ .../check-definition.service.spec.ts | 57 +++ .../check-definition.service.ts | 239 +++++++++++ .../cloud-security/check-definition.utils.ts | 67 +++ .../cloud-security/cloud-security-audit.ts | 4 +- .../cloud-security-query.legacy.ts | 89 ++++ .../cloud-security-query.service.ts | 256 ++++++------ .../cloud-security-query.types.ts | 73 ++++ .../cloud-security.controller.ts | 104 +++++ .../cloud-security/cloud-security.module.ts | 10 + .../cloud-security/cloud-security.service.ts | 43 +- .../cloud-security/dto/mark-exception.dto.ts | 28 ++ .../cloud-security/evidence-sanitizer.spec.ts | 219 ++++++++++ .../src/cloud-security/evidence-sanitizer.ts | 72 ++++ .../cloud-security/exception.service.spec.ts | 232 +++++++++++ .../src/cloud-security/exception.service.ts | 211 ++++++++++ .../api/src/cloud-security/history.service.ts | 66 +++ .../reconciliation.service.spec.ts | 280 +++++++++++++ .../cloud-security/reconciliation.service.ts | 306 ++++++++++++++ .../comments-permission.guard.spec.ts | 200 +++++++++ .../src/comments/comments-permission.guard.ts | 134 ++++++ apps/api/src/comments/comments.controller.ts | 7 +- apps/api/src/comments/comments.module.ts | 8 +- .../components/CheckDefinitionPanel.test.tsx | 110 +++++ .../components/CheckDefinitionPanel.tsx | 84 ++++ .../components/CheckGroupBlock.test.tsx | 190 +++++++++ .../components/CheckGroupBlock.tsx | 138 +++++++ .../components/CloudTestsSection.tsx | 384 ++++++++++++++---- .../components/EvidenceJsonViewer.test.tsx | 55 +++ .../components/EvidenceJsonViewer.tsx | 98 +++++ .../cloud-tests/components/FindingsTable.tsx | 2 +- .../components/HistoryTab.test.tsx | 214 ++++++++++ .../cloud-tests/components/HistoryTab.tsx | 308 ++++++++++++++ .../components/MarkExceptionModal.test.tsx | 151 +++++++ .../components/MarkExceptionModal.tsx | 198 +++++++++ .../cloud-tests/components/ProviderTabs.tsx | 11 +- .../cloud-tests/components/ResultsView.tsx | 4 +- .../components/check-groups.test.ts | 183 +++++++++ .../cloud-tests/components/check-groups.ts | 125 ++++++ .../app/(app)/[orgId]/cloud-tests/types.ts | 33 ++ .../components/FindingDetailSheet.tsx | 10 + .../migration.sql | 28 ++ .../migration.sql | 2 + .../migration.sql | 116 ++++++ .../db/prisma/schema/check-definition.prisma | 48 +++ packages/db/prisma/schema/comment.prisma | 1 + .../db/prisma/schema/finding-history.prisma | 121 ++++++ .../prisma/schema/integration-platform.prisma | 13 + packages/db/prisma/schema/organization.prisma | 4 + 52 files changed, 5539 insertions(+), 217 deletions(-) create mode 100644 apps/api/src/cloud-security/ai-description.prompt.spec.ts create mode 100644 apps/api/src/cloud-security/ai-description.prompt.ts create mode 100644 apps/api/src/cloud-security/ai-description.service.ts create mode 100644 apps/api/src/cloud-security/check-definition-provider-passthrough.ts create mode 100644 apps/api/src/cloud-security/check-definition.service.spec.ts create mode 100644 apps/api/src/cloud-security/check-definition.service.ts create mode 100644 apps/api/src/cloud-security/check-definition.utils.ts create mode 100644 apps/api/src/cloud-security/cloud-security-query.legacy.ts create mode 100644 apps/api/src/cloud-security/cloud-security-query.types.ts create mode 100644 apps/api/src/cloud-security/dto/mark-exception.dto.ts create mode 100644 apps/api/src/cloud-security/evidence-sanitizer.spec.ts create mode 100644 apps/api/src/cloud-security/evidence-sanitizer.ts create mode 100644 apps/api/src/cloud-security/exception.service.spec.ts create mode 100644 apps/api/src/cloud-security/exception.service.ts create mode 100644 apps/api/src/cloud-security/history.service.ts create mode 100644 apps/api/src/cloud-security/reconciliation.service.spec.ts create mode 100644 apps/api/src/cloud-security/reconciliation.service.ts create mode 100644 apps/api/src/comments/comments-permission.guard.spec.ts create mode 100644 apps/api/src/comments/comments-permission.guard.ts create mode 100644 apps/app/src/app/(app)/[orgId]/cloud-tests/components/CheckDefinitionPanel.test.tsx create mode 100644 apps/app/src/app/(app)/[orgId]/cloud-tests/components/CheckDefinitionPanel.tsx create mode 100644 apps/app/src/app/(app)/[orgId]/cloud-tests/components/CheckGroupBlock.test.tsx create mode 100644 apps/app/src/app/(app)/[orgId]/cloud-tests/components/CheckGroupBlock.tsx create mode 100644 apps/app/src/app/(app)/[orgId]/cloud-tests/components/EvidenceJsonViewer.test.tsx create mode 100644 apps/app/src/app/(app)/[orgId]/cloud-tests/components/EvidenceJsonViewer.tsx create mode 100644 apps/app/src/app/(app)/[orgId]/cloud-tests/components/HistoryTab.test.tsx create mode 100644 apps/app/src/app/(app)/[orgId]/cloud-tests/components/HistoryTab.tsx create mode 100644 apps/app/src/app/(app)/[orgId]/cloud-tests/components/MarkExceptionModal.test.tsx create mode 100644 apps/app/src/app/(app)/[orgId]/cloud-tests/components/MarkExceptionModal.tsx create mode 100644 apps/app/src/app/(app)/[orgId]/cloud-tests/components/check-groups.test.ts create mode 100644 apps/app/src/app/(app)/[orgId]/cloud-tests/components/check-groups.ts create mode 100644 packages/db/prisma/migrations/20260513213400_cloud_tests_check_definition/migration.sql create mode 100644 packages/db/prisma/migrations/20260513220233_comment_entity_type_finding/migration.sql create mode 100644 packages/db/prisma/migrations/20260513221148_cloud_tests_phase5_history/migration.sql create mode 100644 packages/db/prisma/schema/check-definition.prisma create mode 100644 packages/db/prisma/schema/finding-history.prisma diff --git a/apps/api/src/cloud-security/ai-description.prompt.spec.ts b/apps/api/src/cloud-security/ai-description.prompt.spec.ts new file mode 100644 index 0000000000..28bfd3fe72 --- /dev/null +++ b/apps/api/src/cloud-security/ai-description.prompt.spec.ts @@ -0,0 +1,144 @@ +import { + buildCheckDescriptionPrompt, + checkDescriptionSchema, + findForbiddenContent, +} from './ai-description.prompt'; + +describe('ai-description.prompt', () => { + describe('checkDescriptionSchema', () => { + it('accepts a well-formed Tier 3 description', () => { + const parsed = checkDescriptionSchema.safeParse({ + title: 'IAM password policy enforces 14+ character minimum', + description: + 'Verifies that the AWS account password policy requires user passwords to be at least 14 characters long.', + passCriteria: + 'Password policy exists AND MinimumPasswordLength >= 14', + failCriteria: + 'No password policy is configured OR MinimumPasswordLength < 14', + whyItMatters: + 'Short passwords are vulnerable to brute force attacks and credential stuffing.', + }); + expect(parsed.success).toBe(true); + }); + + it('rejects fields below minimum length', () => { + const parsed = checkDescriptionSchema.safeParse({ + title: '', + description: 'short', + passCriteria: 'x', + failCriteria: 'y', + whyItMatters: 'z', + }); + expect(parsed.success).toBe(false); + }); + }); + + describe('findForbiddenContent', () => { + const baseline = { + title: 'IAM password policy enforces 14+ character minimum', + description: + 'Verifies that the AWS account password policy requires user passwords to be at least 14 characters long.', + passCriteria: + 'Password policy exists AND MinimumPasswordLength >= 14', + failCriteria: + 'No password policy is configured OR MinimumPasswordLength < 14', + whyItMatters: + 'Short passwords are vulnerable to brute force attacks and credential stuffing.', + }; + + it('returns null for clean output', () => { + expect(findForbiddenContent(baseline)).toBeNull(); + }); + + it('flags SOC 2 control numbers', () => { + expect( + findForbiddenContent({ + ...baseline, + whyItMatters: + 'This check aligns with SOC 2 CC6.1 logical access controls.', + }), + ).toMatchObject({ field: 'whyItMatters' }); + }); + + it('flags ISO 27001 references', () => { + expect( + findForbiddenContent({ + ...baseline, + whyItMatters: 'Required by ISO 27001 A.9.4.3.', + }), + ).toMatchObject({ field: 'whyItMatters' }); + }); + + it('flags HIPAA / NIST framework citations', () => { + expect( + findForbiddenContent({ + ...baseline, + description: 'HIPAA-aligned password requirement.', + }), + ).toMatchObject({ field: 'description' }); + expect( + findForbiddenContent({ + ...baseline, + description: 'Maps to NIST AC-2.', + }), + ).toMatchObject({ field: 'description' }); + }); + + it('flags any URL', () => { + expect( + findForbiddenContent({ + ...baseline, + whyItMatters: 'See https://docs.aws.amazon.com for more info.', + }), + ).toMatchObject({ field: 'whyItMatters' }); + expect( + findForbiddenContent({ + ...baseline, + description: 'Reference: www.cisecurity.org/benchmark.', + }), + ).toMatchObject({ field: 'description' }); + }); + + it('flags CC. control patterns even without "SOC 2"', () => { + expect( + findForbiddenContent({ + ...baseline, + passCriteria: 'Control reference: CC7.1', + }), + ).toMatchObject({ field: 'passCriteria' }); + }); + }); + + describe('buildCheckDescriptionPrompt', () => { + it('includes provider, severity, title and description', () => { + const prompt = buildCheckDescriptionPrompt({ + provider: 'aws', + serviceName: 'IAM', + title: 'IAM user "john" does not have MFA enabled', + description: 'User john has no MFA device configured.', + severity: 'high', + remediation: 'Enable MFA via IAM console.', + }); + expect(prompt).toContain('AWS'); + expect(prompt).toContain('IAM'); + expect(prompt).toContain('high'); + expect(prompt).toContain('john'); + expect(prompt).toContain('Enable MFA'); + }); + + it('omits null/empty fields cleanly', () => { + const prompt = buildCheckDescriptionPrompt({ + provider: 'aws', + serviceName: null, + title: 'Untitled finding', + description: null, + severity: null, + remediation: null, + }); + expect(prompt).toContain('Untitled finding'); + expect(prompt).not.toContain('Service:'); + expect(prompt).not.toContain('Finding description:'); + expect(prompt).not.toContain('Suggested remediation:'); + }); + }); +}); diff --git a/apps/api/src/cloud-security/ai-description.prompt.ts b/apps/api/src/cloud-security/ai-description.prompt.ts new file mode 100644 index 0000000000..f487ab37fe --- /dev/null +++ b/apps/api/src/cloud-security/ai-description.prompt.ts @@ -0,0 +1,128 @@ +import { z } from 'zod'; + +/** + * Output shape of the "About this check" panel auditors see when they + * expand a finding. Tier 3 = title, description, pass/fail criteria, + * rationale. NOTHING ELSE — no compliance control numbers, no external + * URLs, no framework claims. Server-side validation strips forbidden + * content before persisting to the cache. + */ +export const checkDescriptionSchema = z.object({ + title: z + .string() + .min(1) + .max(160) + .describe('Plain-English summary of what this check verifies (~1 sentence).'), + description: z + .string() + .min(20) + .max(600) + .describe( + 'What the check actually verifies in plain English (1-3 sentences). No control numbers, no URLs, no framework claims.', + ), + passCriteria: z + .string() + .min(10) + .max(300) + .describe( + 'The configuration condition that makes this check pass. 1 sentence.', + ), + failCriteria: z + .string() + .min(10) + .max(300) + .describe( + 'The configuration condition that makes this check fail. 1 sentence.', + ), + whyItMatters: z + .string() + .min(20) + .max(600) + .describe( + 'Security/risk rationale in plain English. 1-2 sentences. NO compliance citations.', + ), +}); + +export type CheckDescription = z.infer; + +export const CHECK_DESCRIPTION_SYSTEM_PROMPT = `You write audit-friendly explanations of cloud-security checks. + +Audience: an external auditor (SOC 2 / ISO 27001) reviewing the customer's environment. They need to TRUST the automation by understanding what each check verifies. + +OUTPUT +- Return only the fields requested by the schema. Nothing else. +- Tone: neutral, professional, present tense, third person. +- Plain English. Avoid product jargon when a simpler word works. + +HARD RULES (output that violates these is stripped server-side): +- DO NOT mention any specific compliance control number (no "SOC 2 CC6.1", "ISO 27001 A.9.4.3", "NIST AC-2", "HIPAA \xA7164.312", "CIS 1.8", "PCI 8.2.3", etc.). +- DO NOT name a compliance framework as if it requires this check (no "required by SOC 2", "ISO mandates", "HIPAA-aligned"). +- DO NOT include any URL or external link. +- DO NOT invent product features the input doesn't describe. + +Style: short, clear, factual.`; + +export interface CheckDescriptionInput { + provider: 'aws' | 'gcp' | 'azure' | string; + serviceName: string | null; + title: string; + description: string | null; + severity: string | null; + remediation: string | null; +} + +export function buildCheckDescriptionPrompt(input: CheckDescriptionInput): string { + return [ + `Provider: ${input.provider.toUpperCase()}`, + input.serviceName ? `Service: ${input.serviceName}` : null, + `Severity: ${input.severity ?? 'unknown'}`, + `Finding title: "${input.title}"`, + input.description ? `Finding description: "${input.description}"` : null, + input.remediation ? `Suggested remediation: "${input.remediation}"` : null, + '', + 'Generate a CheckDefinition for this check. Focus on what the CHECK as a class verifies — not on the specific resource that failed.', + ] + .filter(Boolean) + .join('\n'); +} + +/** + * Detect content that slipped past the prompt — typical AI hallucinations + * we want to keep out of the rendered panel. Returns a list of regex + * patterns that should NEVER match in production output. + */ +const FORBIDDEN_PATTERNS: readonly RegExp[] = [ + // Compliance control numbers and framework citations + /\bSOC ?2\b/i, + /\bISO ?27001\b/i, + /\bISO ?27002\b/i, + /\bHIPAA\b/i, + /\bNIST\b/i, + /\bPCI ?DSS\b/i, + /\bCIS ?Benchmark\b/i, + /\bCC\d+\.\d+\b/i, // SOC 2 control numbers like CC6.1 + /\bA\.\d+\.\d+(\.\d+)?\b/, // ISO control numbers like A.9.4.3 + // URLs + /https?:\/\//i, + /www\./i, +]; + +/** + * Return the first forbidden pattern that matches any field's value, or + * null when output is clean. Used as a server-side backstop to the prompt. + */ +export function findForbiddenContent( + description: CheckDescription, +): { field: keyof CheckDescription; pattern: string } | null { + for (const [field, value] of Object.entries(description) as [ + keyof CheckDescription, + string, + ][]) { + for (const pattern of FORBIDDEN_PATTERNS) { + if (pattern.test(value)) { + return { field, pattern: pattern.source }; + } + } + } + return null; +} diff --git a/apps/api/src/cloud-security/ai-description.service.ts b/apps/api/src/cloud-security/ai-description.service.ts new file mode 100644 index 0000000000..388c6ce448 --- /dev/null +++ b/apps/api/src/cloud-security/ai-description.service.ts @@ -0,0 +1,59 @@ +import { Injectable, Logger } from '@nestjs/common'; +import { generateObject } from 'ai'; +import { anthropic } from '@ai-sdk/anthropic'; +import { + CHECK_DESCRIPTION_SYSTEM_PROMPT, + buildCheckDescriptionPrompt, + checkDescriptionSchema, + findForbiddenContent, + type CheckDescription, + type CheckDescriptionInput, +} from './ai-description.prompt'; + +/** + * Haiku 4.5 — cheap, fast, plenty good for descriptive text. Locked here + * so cache invalidation can detect model upgrades via `modelVersion`. + */ +export const DESCRIPTION_MODEL_VERSION = 'claude-haiku-4-5'; +const MODEL = anthropic(DESCRIPTION_MODEL_VERSION); + +@Injectable() +export class AiDescriptionService { + private readonly logger = new Logger(AiDescriptionService.name); + + /** + * Generate a Tier 3 "About this check" panel from a finding's metadata. + * Returns null on any AI failure — callers should surface a graceful + * fallback (showing only the existing per-finding description) rather + * than throwing. + */ + async generate(input: CheckDescriptionInput): Promise { + try { + const { object } = await generateObject({ + model: MODEL, + schema: checkDescriptionSchema, + system: CHECK_DESCRIPTION_SYSTEM_PROMPT, + prompt: buildCheckDescriptionPrompt(input), + temperature: 0, + }); + + // Server-side backstop: if Haiku slipped past the prompt and emitted + // a compliance control number or URL, refuse to cache it. Callers + // get null and the UI falls back to existing content. + const violation = findForbiddenContent(object); + if (violation) { + this.logger.warn( + `AI description for "${input.title}" rejected: ${violation.field} matched forbidden pattern ${violation.pattern}`, + ); + return null; + } + + return object; + } catch (err) { + this.logger.error( + `AI description generation failed for "${input.title}": ${err instanceof Error ? err.message : String(err)}`, + ); + return null; + } + } +} diff --git a/apps/api/src/cloud-security/check-definition-provider-passthrough.ts b/apps/api/src/cloud-security/check-definition-provider-passthrough.ts new file mode 100644 index 0000000000..1aadc588ed --- /dev/null +++ b/apps/api/src/cloud-security/check-definition-provider-passthrough.ts @@ -0,0 +1,89 @@ +/** + * GCP/Azure check descriptions are NOT generated by AI — they're surfaced + * directly from the provider's own evidence payload, which already + * contains auditor-grade descriptions written by Google / Microsoft. + * + * This module isolates the per-provider mapping so the main + * CheckDefinitionService stays focused on cache + AI orchestration. + */ + +import type { ResolvedCheckDescription } from './check-definition.service'; + +export interface PassthroughInput { + provider: 'gcp' | 'azure' | string; + title: string; + description: string | null; + evidence: Record | null; +} + +function humanizeCategory(category: string): string { + return category + .toLowerCase() + .replace(/_/g, ' ') + .replace(/\b\w/g, (c) => c.toUpperCase()); +} + +function fromGcpEvidence( + input: PassthroughInput, +): ResolvedCheckDescription | null { + if (!input.evidence) return null; + const category = + typeof input.evidence.category === 'string' + ? input.evidence.category + : null; + const findingClass = + typeof input.evidence.findingClass === 'string' + ? input.evidence.findingClass + : null; + if (!category) return null; + + return { + title: input.title, + description: input.description ?? humanizeCategory(category), + passCriteria: `No ${category + .toLowerCase() + .replace(/_/g, ' ')} detected by Google Security Command Center.`, + failCriteria: input.description + ? input.description + : `Google Security Command Center flagged this resource under category ${category}.`, + whyItMatters: findingClass + ? `Google classifies this finding as a "${findingClass}" — addressing it reduces the underlying security risk Google identified.` + : 'Findings from Google Security Command Center indicate a security concern Google detected in the customer environment.', + source: 'provider', + }; +} + +function fromAzureEvidence( + input: PassthroughInput, +): ResolvedCheckDescription | null { + const alertType = + input.evidence && typeof input.evidence.alertType === 'string' + ? input.evidence.alertType + : input.evidence && typeof input.evidence.serviceName === 'string' + ? (input.evidence.serviceName as string) + : null; + + return { + title: input.title, + description: + input.description ?? + 'Surfaced from Microsoft Defender for Cloud. Defender flagged this resource as not meeting its recommended secure configuration.', + passCriteria: + 'Microsoft Defender for Cloud reports the resource as healthy.', + failCriteria: input.description + ? input.description + : 'Microsoft Defender for Cloud reports the resource as unhealthy.', + whyItMatters: alertType + ? `Defender raised this finding under "${alertType}" — addressing it brings the resource in line with Microsoft\'s recommended configuration.` + : 'Defender for Cloud findings indicate a deviation from Microsoft-recommended secure configuration in the customer environment.', + source: 'provider', + }; +} + +export function buildProviderPassthroughDescription( + input: PassthroughInput, +): ResolvedCheckDescription | null { + if (input.provider === 'gcp') return fromGcpEvidence(input); + if (input.provider === 'azure') return fromAzureEvidence(input); + return null; +} diff --git a/apps/api/src/cloud-security/check-definition.service.spec.ts b/apps/api/src/cloud-security/check-definition.service.spec.ts new file mode 100644 index 0000000000..c9f45aa777 --- /dev/null +++ b/apps/api/src/cloud-security/check-definition.service.spec.ts @@ -0,0 +1,57 @@ +import { normalizeCheckId } from './check-definition.utils'; + +// We test the pure-function exports here. The full CheckDefinitionService +// orchestration (db + ai) is covered by integration patterns; this file +// focuses on the normalization logic that determines cache identity — +// getting that wrong silently breaks the per-org cache. + +describe('normalizeCheckId', () => { + it('returns the input unchanged when resourceId is null', () => { + expect(normalizeCheckId('iam-no-password-policy', null)).toBe( + 'iam-no-password-policy', + ); + }); + + it('strips a resource-specific suffix matching the resourceId', () => { + expect(normalizeCheckId('iam-no-mfa-john', 'john')).toBe('iam-no-mfa'); + expect(normalizeCheckId('cloudtrail-not-logging-prod-trail', 'prod-trail')).toBe( + 'cloudtrail-not-logging', + ); + }); + + it('returns input unchanged when resourceId is "account-level"', () => { + // Fixed-id checks use 'account-level' as a placeholder resourceId. + // No suffix to strip. + expect(normalizeCheckId('iam-no-password-policy', 'account-level')).toBe( + 'iam-no-password-policy', + ); + }); + + it('handles compound resourceIds by trying each path segment', () => { + // e.g. AWS API Gateway uses "${apiId}/${routeKey}" as resourceId. + expect( + normalizeCheckId('apigw-no-auth-abc123-route-1', 'abc123/route-1'), + ).toBe('apigw-no-auth-abc123'); + }); + + it('returns the input unchanged when no suffix can be matched', () => { + expect(normalizeCheckId('iam-no-mfa', 'unrelated-resource')).toBe( + 'iam-no-mfa', + ); + }); + + it('preserves uniqueness across DIFFERENT check types for the same resource', () => { + // Sanity: two findings on the same resource but different checks should + // normalize to different keys so the cache stays correct. + expect(normalizeCheckId('iam-no-mfa-john', 'john')).not.toBe( + normalizeCheckId('iam-no-access-keys-john', 'john'), + ); + }); + + it('produces the same normalized key for two resources of the same check', () => { + // The cache benefit: different users, same check → one cache entry. + expect(normalizeCheckId('iam-no-mfa-john', 'john')).toBe( + normalizeCheckId('iam-no-mfa-alice', 'alice'), + ); + }); +}); diff --git a/apps/api/src/cloud-security/check-definition.service.ts b/apps/api/src/cloud-security/check-definition.service.ts new file mode 100644 index 0000000000..2ffa636d16 --- /dev/null +++ b/apps/api/src/cloud-security/check-definition.service.ts @@ -0,0 +1,239 @@ +import { Injectable, Logger } from '@nestjs/common'; +import { db } from '@db'; +import { AiDescriptionService, DESCRIPTION_MODEL_VERSION } from './ai-description.service'; +import type { CheckDescription } from './ai-description.prompt'; +import { buildProviderPassthroughDescription } from './check-definition-provider-passthrough'; +import { computeSourceHash, normalizeCheckId } from './check-definition.utils'; + +export { normalizeCheckId }; + +/** + * The CheckDescription returned to clients. `source` lets the UI render a + * subtle hint about whether the content was AI-generated (AWS) or + * surfaced directly from the cloud provider's own catalog (GCP / Azure). + */ +export interface ResolvedCheckDescription extends CheckDescription { + source: 'ai' | 'provider'; +} + +export interface CheckDescriptionRequest { + organizationId: string; + /** Normalized, resource-agnostic check identifier — see normalizeCheckId. */ + checkId: string; + provider: 'aws' | 'gcp' | 'azure' | string; + serviceName: string | null; + /** Per-finding title / description / remediation — the SOURCE we hash. */ + title: string; + description: string | null; + severity: string | null; + remediation: string | null; + /** Raw evidence — used for GCP/Azure passthrough only. */ + evidence: Record | null; +} + + +@Injectable() +export class CheckDefinitionService { + private readonly logger = new Logger(CheckDefinitionService.name); + + constructor(private readonly aiDescription: AiDescriptionService) {} + + /** + * Resolve a check description for a finding by its ID. Scoped to the + * caller's organization to prevent cross-tenant leaks. Returns null + * when the finding doesn't exist or no useful description can be + * produced (UI degrades gracefully). + */ + async getForFinding( + findingId: string, + organizationId: string, + ): Promise { + const request = await this.resolveFindingToRequest( + findingId, + organizationId, + ); + if (!request) return null; + return this.getOrCreate(request); + } + + private async resolveFindingToRequest( + findingId: string, + organizationId: string, + ): Promise { + // Try new-platform first (IntegrationCheckResult — id prefix `icx_`). + const newResult = await db.integrationCheckResult.findFirst({ + where: { + id: findingId, + checkRun: { connection: { organizationId } }, + }, + select: { + title: true, + description: true, + severity: true, + remediation: true, + resourceId: true, + evidence: true, + checkRun: { + select: { + checkId: true, + connection: { select: { provider: { select: { slug: true } } } }, + }, + }, + }, + }); + + if (newResult) { + const evidence = + newResult.evidence && typeof newResult.evidence === 'object' + ? (newResult.evidence as Record) + : null; + const findingKey = + evidence && typeof evidence.findingKey === 'string' + ? evidence.findingKey + : newResult.checkRun.checkId; + const serviceName = + evidence && typeof evidence.serviceName === 'string' + ? evidence.serviceName + : evidence && typeof evidence.service === 'string' + ? evidence.service + : null; + return { + organizationId, + checkId: normalizeCheckId(findingKey, newResult.resourceId), + provider: newResult.checkRun.connection.provider.slug, + serviceName, + title: newResult.title ?? '', + description: newResult.description, + severity: newResult.severity, + remediation: newResult.remediation, + evidence, + }; + } + + // Fall back to legacy IntegrationResult (id prefix `itr_`). + const legacy = await db.integrationResult.findFirst({ + where: { id: findingId, organizationId }, + select: { + title: true, + description: true, + severity: true, + remediation: true, + resultDetails: true, + integration: { select: { integrationId: true } }, + }, + }); + + if (!legacy) return null; + + const details = + legacy.resultDetails && typeof legacy.resultDetails === 'object' + ? (legacy.resultDetails as Record) + : null; + return { + organizationId, + // Legacy results have no granular checkKey — use title as the cache key + // (low-fidelity but stable across instances). + checkId: legacy.title ?? findingId, + provider: legacy.integration.integrationId, + serviceName: null, + title: legacy.title ?? '', + description: legacy.description, + severity: legacy.severity, + remediation: legacy.remediation, + evidence: details, + }; + } + + /** + * Resolve a check description for a finding. + * + * - AWS: cached per (orgId, checkId) with source-hash invalidation. + * First-view triggers a Haiku call (~1-2s); all subsequent views and + * all other findings of the same check type hit the cache (~50ms). + * - GCP / Azure: derived synchronously from provider evidence — no AI, + * no DB cache. Returns null when evidence doesn't carry enough context. + */ + async getOrCreate( + req: CheckDescriptionRequest, + ): Promise { + if (req.provider === 'gcp' || req.provider === 'azure') { + return buildProviderPassthroughDescription({ + provider: req.provider, + title: req.title, + description: req.description, + evidence: req.evidence, + }); + } + return this.fromCacheOrGenerate(req); + } + + private async fromCacheOrGenerate( + req: CheckDescriptionRequest, + ): Promise { + const sourceHash = computeSourceHash(req); + + const cached = await db.checkDefinition.findUnique({ + where: { + organizationId_checkId: { + organizationId: req.organizationId, + checkId: req.checkId, + }, + }, + }); + + if (cached && cached.sourceHash === sourceHash) { + return { + title: cached.title, + description: cached.description, + passCriteria: cached.passCriteria, + failCriteria: cached.failCriteria, + whyItMatters: cached.whyItMatters, + source: 'ai', + }; + } + + const generated = await this.aiDescription.generate({ + provider: req.provider, + serviceName: req.serviceName, + title: req.title, + description: req.description, + severity: req.severity, + remediation: req.remediation, + }); + + if (!generated) return null; + + try { + await db.checkDefinition.upsert({ + where: { + organizationId_checkId: { + organizationId: req.organizationId, + checkId: req.checkId, + }, + }, + create: { + organizationId: req.organizationId, + checkId: req.checkId, + sourceHash, + modelVersion: DESCRIPTION_MODEL_VERSION, + ...generated, + }, + update: { + sourceHash, + modelVersion: DESCRIPTION_MODEL_VERSION, + generatedAt: new Date(), + ...generated, + }, + }); + } catch (err) { + // Persist failure shouldn't break the read path — log and serve the + // freshly-generated content anyway. + this.logger.warn( + `CheckDefinition cache write failed for ${req.checkId}: ${err instanceof Error ? err.message : String(err)}`, + ); + } + + return { ...generated, source: 'ai' }; + } + +} diff --git a/apps/api/src/cloud-security/check-definition.utils.ts b/apps/api/src/cloud-security/check-definition.utils.ts new file mode 100644 index 0000000000..5b67ef314e --- /dev/null +++ b/apps/api/src/cloud-security/check-definition.utils.ts @@ -0,0 +1,67 @@ +/** + * Pure helpers shared between CheckDefinitionService and its tests. + * Kept in a separate file so tests can import them without pulling in + * the Prisma client (which throws at import time when DATABASE_URL is + * missing in a unit-test env). + */ + +import { createHash } from 'node:crypto'; +import { DESCRIPTION_MODEL_VERSION } from './ai-description.service'; + +/** + * Strip the resource-specific suffix from a finding's `findingKey` so all + * resource instances of the same check share a single cache entry. + * + * Examples: + * ("iam-no-mfa-john", "john") -> "iam-no-mfa" + * ("cloudtrail-not-logging-prod-trail", "prod-trail") -> "cloudtrail-not-logging" + * ("iam-no-password-policy", "account-level") -> "iam-no-password-policy" + */ +export function normalizeCheckId( + findingKey: string, + resourceId: string | null, +): string { + if (!resourceId) return findingKey; + + const suffix = `-${resourceId}`; + if (findingKey.endsWith(suffix)) { + return findingKey.slice(0, -suffix.length); + } + // Try each segment of compound resource ids (e.g. "api/route-1" -> ["api", "route-1"]). + for (const segment of resourceId.split(/[/.]/)) { + if (!segment) continue; + const segSuffix = `-${segment}`; + if (findingKey.endsWith(segSuffix)) { + return findingKey.slice(0, -segSuffix.length); + } + } + return findingKey; +} + +export interface SourceHashInput { + provider: string; + serviceName: string | null; + title: string; + description: string | null; + severity: string | null; + remediation: string | null; +} + +/** + * Hash of the inputs that drive Haiku output. When this changes — because + * the adapter altered the finding's title/description/severity/remediation + * — the cache entry is regenerated on the next view. Includes the model + * version so flipping DESCRIPTION_MODEL_VERSION forces a global refresh. + */ +export function computeSourceHash(input: SourceHashInput): string { + const payload = JSON.stringify({ + provider: input.provider, + serviceName: input.serviceName, + title: input.title, + description: input.description, + severity: input.severity, + remediation: input.remediation, + model: DESCRIPTION_MODEL_VERSION, + }); + return createHash('sha256').update(payload).digest('hex'); +} diff --git a/apps/api/src/cloud-security/cloud-security-audit.ts b/apps/api/src/cloud-security/cloud-security-audit.ts index 75285b6827..9e3f4ea0b9 100644 --- a/apps/api/src/cloud-security/cloud-security-audit.ts +++ b/apps/api/src/cloud-security/cloud-security-audit.ts @@ -11,7 +11,9 @@ interface CloudSecurityAuditParams { | 'remediation_failed' | 'rollback_executed' | 'rollback_failed' - | 'service_toggled'; + | 'service_toggled' + | 'exception_marked' + | 'exception_revoked'; description: string; metadata?: Record; } diff --git a/apps/api/src/cloud-security/cloud-security-query.legacy.ts b/apps/api/src/cloud-security/cloud-security-query.legacy.ts new file mode 100644 index 0000000000..e6984cc0c5 --- /dev/null +++ b/apps/api/src/cloud-security/cloud-security-query.legacy.ts @@ -0,0 +1,89 @@ +/** + * Legacy (pre-integration-platform) cloud security queries. + * + * Extracted from cloud-security-query.service.ts so the main service stays + * under the 300-line cap and so the legacy path can be deleted as a unit + * once the legacy Integration / IntegrationResult tables are retired. + */ + +import { db } from '@db'; +import { sanitizeEvidence } from './evidence-sanitizer'; +import type { CloudFinding } from './cloud-security-query.types'; + +const CLOUD_PROVIDER_SLUGS = ['aws', 'gcp', 'azure'] as const; + +/** Scan window for filtering legacy results to latest scan only */ +const SCAN_WINDOW_MS = 10 * 60 * 1000; // 10 minutes + +export async function getLegacyFindings( + organizationId: string, +): Promise { + const legacyIntegrations = await db.integration.findMany({ + where: { organizationId }, + }); + + const activeLegacy = legacyIntegrations.filter((i) => + (CLOUD_PROVIDER_SLUGS as readonly string[]).includes(i.integrationId), + ); + + const legacyIds = activeLegacy.map((i) => i.id); + if (legacyIds.length === 0) return []; + + const lastRunMap = new Map( + activeLegacy.filter((i) => i.lastRunAt).map((i) => [i.id, i.lastRunAt!]), + ); + + const results = await db.integrationResult.findMany({ + where: { integrationId: { in: legacyIds } }, + select: { + id: true, + title: true, + description: true, + remediation: true, + status: true, + severity: true, + completedAt: true, + resultDetails: true, + integration: { + select: { integrationId: true, id: true, lastRunAt: true }, + }, + }, + orderBy: { completedAt: 'desc' }, + }); + + // Only include results from the most recent scan window per integration. + const filtered = results.filter((result) => { + const lastRunAt = lastRunMap.get(result.integration.id); + if (!lastRunAt) return result.completedAt !== null; + if (!result.completedAt) return false; + + const lastRunTime = lastRunAt.getTime(); + const completedTime = result.completedAt.getTime(); + return ( + completedTime <= lastRunTime && + completedTime >= lastRunTime - SCAN_WINDOW_MS + ); + }); + + return filtered.map((result) => ({ + id: result.id, + title: result.title, + description: result.description, + remediation: result.remediation, + status: result.status, + severity: result.severity, + completedAt: result.completedAt, + connectionId: result.integration.id, + providerSlug: result.integration.integrationId, + serviceId: null, + findingKey: null, + resourceId: null, + // Legacy IntegrationResult model has neither resourceType nor checkId + resourceType: null, + checkId: null, + checkKey: null, + evidence: sanitizeEvidence(result.resultDetails ?? null), + projectDisplayName: null, + integration: { integrationId: result.integration.integrationId }, + })); +} diff --git a/apps/api/src/cloud-security/cloud-security-query.service.ts b/apps/api/src/cloud-security/cloud-security-query.service.ts index 1cdba70c00..2388b5324b 100644 --- a/apps/api/src/cloud-security/cloud-security-query.service.ts +++ b/apps/api/src/cloud-security/cloud-security-query.service.ts @@ -1,12 +1,21 @@ import { Injectable } from '@nestjs/common'; import { db } from '@db'; import { getManifest } from '@trycompai/integration-platform'; +import { sanitizeEvidence } from './evidence-sanitizer'; +import { getLegacyFindings } from './cloud-security-query.legacy'; +import { normalizeCheckId } from './check-definition.utils'; +import type { + CloudFinding, + CloudProvider, + CloudProviderLatestRun, +} from './cloud-security-query.types'; + +// Re-export so existing imports of CloudProvider/CloudFinding from the service +// path keep working (the controller imports them). +export type { CloudFinding, CloudProvider, CloudProviderLatestRun }; const CLOUD_PROVIDER_SLUGS = ['aws', 'gcp', 'azure'] as const; -/** Scan window for filtering legacy results to latest scan only */ -const SCAN_WINDOW_MS = 10 * 60 * 1000; // 10 minutes - /** Extract project ID from a GCP resource path like //iam.googleapis.com/projects/my-proj/... */ function extractProjectIdFromResource( resourceId: string | null, @@ -16,45 +25,6 @@ function extractProjectIdFromResource( return match?.[1] ?? null; } -export interface CloudProvider { - id: string; - integrationId: string; - name: string; - displayName?: string; - organizationId: string; - lastRunAt: Date | null; - status: string; - createdAt: Date; - updatedAt: Date; - reconnectedAt?: Date; - isLegacy: boolean; - variables: Record | null; - requiredVariables: string[]; - accountId?: string; - awsType?: string; - regions?: string[]; - tenantId?: string; - subscriptionId?: string; - supportsMultipleConnections?: boolean; -} - -export interface CloudFinding { - id: string; - title: string | null; - description: string | null; - remediation: string | null; - status: string | null; - severity: string | null; - completedAt: Date | null; - connectionId: string; - providerSlug: string; - serviceId: string | null; - findingKey: string | null; - resourceId: string | null; - projectDisplayName: string | null; - integration: { integrationId: string }; -} - /** Get required variables from manifest (both manifest-level and check-level) */ function getRequiredVariables(providerSlug: string): string[] { const manifest = getManifest(providerSlug); @@ -103,6 +73,12 @@ export class CloudSecurityQueryService { (CLOUD_PROVIDER_SLUGS as readonly string[]).includes(i.integrationId), ); + // Per-connection latest scan summary for new-platform connections — one + // query, distinct-by-connection so we get only the most recent run each. + const latestRunByConnection = await this.getLatestRunsByConnection( + newConnections.map((c) => c.id), + ); + // Map new connections const newProviders: CloudProvider[] = newConnections.map((conn) => { const metadata = (conn.metadata || {}) as Record; @@ -147,6 +123,7 @@ export class CloudSecurityQueryService { : undefined, supportsMultipleConnections: manifest?.supportsMultipleConnections ?? false, + latestRun: latestRunByConnection.get(conn.id) ?? null, }; }); @@ -187,21 +164,107 @@ export class CloudSecurityQueryService { : undefined, supportsMultipleConnections: manifest?.supportsMultipleConnections ?? false, + latestRun: integration.lastRunAt + ? { + completedAt: integration.lastRunAt, + durationMs: null, + totalChecked: null, + passedCount: null, + failedCount: null, + status: 'success', + } + : null, }; }); return [...newProviders, ...legacyProviders]; } - async getFindings(organizationId: string): Promise { - const newFindings = await this.getNewPlatformFindings(organizationId); - const legacyFindings = await this.getLegacyFindings(organizationId); + async getFindings( + organizationId: string, + options: { includeExceptions?: boolean } = {}, + ): Promise { + const [newFindings, legacyFindings] = await Promise.all([ + this.getNewPlatformFindings(organizationId), + getLegacyFindings(organizationId), + ]); - return [...newFindings, ...legacyFindings].sort((a, b) => { + const combined = [...newFindings, ...legacyFindings].sort((a, b) => { const dateA = a.completedAt ? new Date(a.completedAt).getTime() : 0; const dateB = b.completedAt ? new Date(b.completedAt).getTime() : 0; return dateB - dateA; }); + + if (options.includeExceptions) return combined; + + // Filter out findings under an active (non-revoked, non-expired) + // FindingException. Looked up in one query keyed by org so the cost + // stays constant regardless of finding count. + const activeExceptionKeys = await this.loadActiveExceptionKeys(organizationId); + if (activeExceptionKeys.size === 0) return combined; + + return combined.filter((finding) => { + if (!finding.checkKey || !finding.resourceId) return true; + const key = `${finding.connectionId}::${finding.checkKey}::${finding.resourceId}`; + return !activeExceptionKeys.has(key); + }); + } + + /** + * Return the set of (connectionId, checkId, resourceId) tuples that have + * an active exception in this org. One DB query per getFindings call. + */ + private async loadActiveExceptionKeys( + organizationId: string, + ): Promise> { + const active = await db.findingException.findMany({ + where: { + organizationId, + revokedAt: null, + OR: [{ expiresAt: null }, { expiresAt: { gt: new Date() } }], + }, + select: { connectionId: true, checkId: true, resourceId: true }, + }); + return new Set( + active.map((e) => `${e.connectionId}::${e.checkId}::${e.resourceId}`), + ); + } + + private async getLatestRunsByConnection( + connectionIds: string[], + ): Promise> { + if (connectionIds.length === 0) return new Map(); + + const runs = await db.integrationCheckRun.findMany({ + where: { + connectionId: { in: connectionIds }, + status: { in: ['success', 'failed'] }, + }, + orderBy: { completedAt: 'desc' }, + distinct: ['connectionId'], + select: { + connectionId: true, + completedAt: true, + durationMs: true, + totalChecked: true, + passedCount: true, + failedCount: true, + status: true, + }, + }); + + const map = new Map(); + for (const run of runs) { + map.set(run.connectionId, { + completedAt: run.completedAt, + durationMs: run.durationMs, + totalChecked: run.totalChecked, + passedCount: run.passedCount, + failedCount: run.failedCount, + status: run.status, + }); + } + return map; } private async getNewPlatformFindings( @@ -242,7 +305,12 @@ export class CloudSecurityQueryService { }, orderBy: { completedAt: 'desc' }, distinct: ['connectionId'], - select: { id: true, connectionId: true, status: true }, + select: { + id: true, + connectionId: true, + status: true, + checkId: true, + }, }); const latestRunIds = latestRuns.map((r) => r.id); @@ -263,6 +331,7 @@ export class CloudSecurityQueryService { passed: true, evidence: true, resourceId: true, + resourceType: true, }, orderBy: { collectedAt: 'desc' }, }); @@ -272,7 +341,14 @@ export class CloudSecurityQueryService { const slug = checkRun ? connectionToSlug[checkRun.connectionId] || 'unknown' : 'unknown'; - const evidence = (result.evidence ?? {}) as Record; + const rawEvidence = (result.evidence ?? {}) as Record; + // Read service/finding hints from raw evidence BEFORE sanitization — these + // are non-sensitive metadata fields the UI groups and filters by. + const serviceId = (rawEvidence.serviceId as string) ?? null; + const findingKey = (rawEvidence.findingKey as string) ?? null; + const projectDisplayNameFromEvidence = rawEvidence.projectDisplayName as + | string + | undefined; return { id: result.id, title: result.title, @@ -283,14 +359,19 @@ export class CloudSecurityQueryService { completedAt: result.collectedAt, connectionId: checkRun?.connectionId ?? '', providerSlug: slug, - serviceId: (evidence.serviceId as string) ?? null, - findingKey: (evidence.findingKey as string) ?? null, + serviceId, + findingKey, resourceId: result.resourceId ?? null, + resourceType: result.resourceType ?? null, + checkId: checkRun?.checkId ?? null, + checkKey: findingKey + ? normalizeCheckId(findingKey, result.resourceId) + : null, + evidence: sanitizeEvidence(result.evidence ?? null), projectDisplayName: (() => { - const fromEvidence = evidence.projectDisplayName as - | string - | undefined; - if (fromEvidence) return fromEvidence; + if (projectDisplayNameFromEvidence) { + return projectDisplayNameFromEvidence; + } const projectId = extractProjectIdFromResource(result.resourceId); if (!projectId) return null; return projectNameMap.get(projectId) ?? projectId; @@ -299,71 +380,4 @@ export class CloudSecurityQueryService { }; }); } - - private async getLegacyFindings( - organizationId: string, - ): Promise { - const legacyIntegrations = await db.integration.findMany({ - where: { organizationId }, - }); - - const activeLegacy = legacyIntegrations.filter((i) => - (CLOUD_PROVIDER_SLUGS as readonly string[]).includes(i.integrationId), - ); - - const legacyIds = activeLegacy.map((i) => i.id); - if (legacyIds.length === 0) return []; - - const lastRunMap = new Map( - activeLegacy.filter((i) => i.lastRunAt).map((i) => [i.id, i.lastRunAt!]), - ); - - const results = await db.integrationResult.findMany({ - where: { integrationId: { in: legacyIds } }, - select: { - id: true, - title: true, - description: true, - remediation: true, - status: true, - severity: true, - completedAt: true, - integration: { - select: { integrationId: true, id: true, lastRunAt: true }, - }, - }, - orderBy: { completedAt: 'desc' }, - }); - - // Filter to only include results from the most recent scan - const filtered = results.filter((result) => { - const lastRunAt = lastRunMap.get(result.integration.id); - if (!lastRunAt) return result.completedAt !== null; - if (!result.completedAt) return false; - - const lastRunTime = lastRunAt.getTime(); - const completedTime = result.completedAt.getTime(); - return ( - completedTime <= lastRunTime && - completedTime >= lastRunTime - SCAN_WINDOW_MS - ); - }); - - return filtered.map((result) => ({ - id: result.id, - title: result.title, - description: result.description, - remediation: result.remediation, - status: result.status, - severity: result.severity, - completedAt: result.completedAt, - connectionId: result.integration.id, - providerSlug: result.integration.integrationId, - serviceId: null, - findingKey: null, - resourceId: null, - projectDisplayName: null, - integration: { integrationId: result.integration.integrationId }, - })); - } } diff --git a/apps/api/src/cloud-security/cloud-security-query.types.ts b/apps/api/src/cloud-security/cloud-security-query.types.ts new file mode 100644 index 0000000000..bdd5daf6e0 --- /dev/null +++ b/apps/api/src/cloud-security/cloud-security-query.types.ts @@ -0,0 +1,73 @@ +/** + * Public response shapes for /v1/cloud-security/providers and /findings. + * Extracted from cloud-security-query.service.ts to keep that file under the + * 300-line cap. The service and legacy helpers both import from here. + */ + +/** + * Summary of the most recent successful (or failed) scan for a connection. + * Surfaced on the providers payload so the UI can show "47 checks evaluated, + * 41 passed, 6 failed, ran 12s ago, duration 3.2s" without a second fetch. + * + * Legacy connections only populate `completedAt` (and `status: 'success'`) + * because per-run counts are not stored in the legacy Integration model. + */ +export interface CloudProviderLatestRun { + completedAt: Date | null; + durationMs: number | null; + totalChecked: number | null; + passedCount: number | null; + failedCount: number | null; + status: string; +} + +export interface CloudProvider { + id: string; + integrationId: string; + name: string; + displayName?: string; + organizationId: string; + lastRunAt: Date | null; + status: string; + createdAt: Date; + updatedAt: Date; + reconnectedAt?: Date; + isLegacy: boolean; + variables: Record | null; + requiredVariables: string[]; + accountId?: string; + awsType?: string; + regions?: string[]; + tenantId?: string; + subscriptionId?: string; + supportsMultipleConnections?: boolean; + latestRun: CloudProviderLatestRun | null; +} + +export interface CloudFinding { + id: string; + title: string | null; + description: string | null; + remediation: string | null; + status: string | null; + severity: string | null; + completedAt: Date | null; + connectionId: string; + providerSlug: string; + serviceId: string | null; + findingKey: string | null; + resourceId: string | null; + resourceType: string | null; + /** Run-level checkId (e.g. "aws-security-scan") — coarse, identifies the scan kind. */ + checkId: string | null; + /** + * Normalized per-check identifier (e.g. "iam-no-mfa"), shared by every + * resource instance of the same logical check. Used by the UI to group + * findings into check sub-groups and by Phase 2 caching. + */ + checkKey: string | null; + /** Sanitized provider payload — sensitive keys redacted by evidence-sanitizer. */ + evidence: unknown; + projectDisplayName: string | null; + integration: { integrationId: string }; +} diff --git a/apps/api/src/cloud-security/cloud-security.controller.ts b/apps/api/src/cloud-security/cloud-security.controller.ts index e2ba7e4b55..8189f11929 100644 --- a/apps/api/src/cloud-security/cloud-security.controller.ts +++ b/apps/api/src/cloud-security/cloud-security.controller.ts @@ -24,6 +24,10 @@ import { } from './cloud-security.service'; import { CloudSecurityQueryService } from './cloud-security-query.service'; import { CloudSecurityLegacyService } from './cloud-security-legacy.service'; +import { CheckDefinitionService } from './check-definition.service'; +import { CloudExceptionService } from './exception.service'; +import { CloudHistoryService } from './history.service'; +import { MarkExceptionDto } from './dto/mark-exception.dto'; import { logCloudSecurityActivity } from './cloud-security-audit'; import { CloudSecurityActivityService } from './cloud-security-activity.service'; import { @@ -44,6 +48,9 @@ export class CloudSecurityController { private readonly activityService: CloudSecurityActivityService, private readonly gcpSecurityService: GCPSecurityService, private readonly azureSecurityService: AzureSecurityService, + private readonly checkDefinitionService: CheckDefinitionService, + private readonly exceptionService: CloudExceptionService, + private readonly historyService: CloudHistoryService, ) {} @Get('activity') @@ -96,6 +103,103 @@ export class CloudSecurityController { return { data: findings, count: findings.length }; } + @Post('findings/:findingId/exception') + @UseGuards(HybridAuthGuard, PermissionGuard) + @RequirePermission('integration', 'update') + @ApiOperation({ + summary: + 'Mark a finding as an exception so it no longer appears in the active Scan Results list', + }) + async markFindingAsException( + @Param('findingId') findingId: string, + @Body() body: MarkExceptionDto, + @OrganizationId() organizationId: string, + @Req() req: { userId?: string }, + ) { + if (!req.userId) { + throw new HttpException( + 'Marking an exception requires session authentication.', + HttpStatus.UNAUTHORIZED, + ); + } + const result = await this.exceptionService.markAsException({ + findingId, + organizationId, + userId: req.userId, + reason: body.reason, + reviewedBy: body.reviewedBy ?? null, + expiresAt: body.expiresAt ? new Date(body.expiresAt) : null, + }); + return { data: result }; + } + + @Delete('exceptions/:exceptionId') + @UseGuards(HybridAuthGuard, PermissionGuard) + @RequirePermission('integration', 'update') + @ApiOperation({ summary: 'Revoke an exception, reopening the finding' }) + async revokeException( + @Param('exceptionId') exceptionId: string, + @OrganizationId() organizationId: string, + @Req() req: { userId?: string }, + ) { + if (!req.userId) { + throw new HttpException( + 'Revoking an exception requires session authentication.', + HttpStatus.UNAUTHORIZED, + ); + } + await this.exceptionService.revokeException({ + exceptionId, + organizationId, + userId: req.userId, + }); + return { success: true }; + } + + @Get('history') + @SkipThrottle() + @UseGuards(HybridAuthGuard, PermissionGuard) + @RequirePermission('integration', 'read') + @ApiOperation({ + summary: + 'List resolution, exception, and regression history for a connection', + }) + async getHistory( + @Query('connectionId') connectionId: string, + @OrganizationId() organizationId: string, + ) { + if (!connectionId) { + throw new HttpException( + 'connectionId query parameter is required', + HttpStatus.BAD_REQUEST, + ); + } + const history = await this.historyService.getHistory({ + organizationId, + connectionId, + }); + return { data: history }; + } + + @Get('findings/:findingId/check-definition') + @SkipThrottle() + @UseGuards(HybridAuthGuard, PermissionGuard) + @RequirePermission('integration', 'read') + @ApiOperation({ + summary: + 'Resolve the "About this check" description for a finding (AI-cached for AWS; provider-derived for GCP/Azure)', + }) + async getCheckDefinition( + @Param('findingId') findingId: string, + @OrganizationId() organizationId: string, + ) { + const definition = await this.checkDefinitionService.getForFinding( + findingId, + organizationId, + ); + return { data: definition }; + } + @Post('scan/:connectionId') @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 3db256d943..52618413eb 100644 --- a/apps/api/src/cloud-security/cloud-security.module.ts +++ b/apps/api/src/cloud-security/cloud-security.module.ts @@ -11,6 +11,11 @@ import { RemediationService } from './remediation.service'; import { GcpRemediationService } from './gcp-remediation.service'; import { AzureRemediationService } from './azure-remediation.service'; import { AiRemediationService } from './ai-remediation.service'; +import { AiDescriptionService } from './ai-description.service'; +import { CheckDefinitionService } from './check-definition.service'; +import { CloudExceptionService } from './exception.service'; +import { CloudHistoryService } from './history.service'; +import { CloudReconciliationService } from './reconciliation.service'; import { CloudSecurityActivityService } from './cloud-security-activity.service'; import { IntegrationPlatformModule } from '../integration-platform/integration-platform.module'; import { AuthModule } from '../auth/auth.module'; @@ -30,6 +35,11 @@ import { AuthModule } from '../auth/auth.module'; GcpRemediationService, AzureRemediationService, AiRemediationService, + AiDescriptionService, + CheckDefinitionService, + CloudExceptionService, + CloudReconciliationService, + CloudHistoryService, ], 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 07f94d4389..c815a3fabb 100644 --- a/apps/api/src/cloud-security/cloud-security.service.ts +++ b/apps/api/src/cloud-security/cloud-security.service.ts @@ -8,6 +8,7 @@ import { GCPSecurityService } from './providers/gcp-security.service'; 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'; export interface SecurityFinding { id: string; @@ -46,6 +47,7 @@ export class CloudSecurityService { private readonly gcpService: GCPSecurityService, private readonly awsService: AWSSecurityService, private readonly azureService: AzureSecurityService, + private readonly reconciliation: CloudReconciliationService, ) {} async scan( @@ -276,7 +278,22 @@ export class CloudSecurityService { } // Store findings in database - await this.storeFindings(connectionId, providerSlug, findings); + const currentRunId = await this.storeFindings( + connectionId, + providerSlug, + findings, + ); + + // Reconcile against the prior scan to record resolutions and regressions. + // Failures must NOT fail the scan — log and continue so customers always + // see fresh findings even if the audit trail step hits an edge case. + try { + await this.reconciliation.reconcile({ currentRunId }); + } catch (err) { + this.logger.error( + `Reconciliation failed for run ${currentRunId}: ${err instanceof Error ? err.message : String(err)}`, + ); + } // Auto-satisfy evidence tasks based on passing scan results (AWS only) if (providerSlug === 'aws') { @@ -556,12 +573,27 @@ export class CloudSecurityService { connectionId: string, provider: string, findings: SecurityFinding[], - ): Promise { + ): Promise { const passedCount = findings.filter((f) => f.passed).length; const failedCount = findings.filter((f) => !f.passed).length; + // Derive successfully-scanned service IDs from finding evidence. Used by + // Phase 5 reconciliation to avoid false "resolved" events when a service + // wasn't actually scanned this run. + const scannedServices = Array.from( + new Set( + findings + .map((f) => { + const evidence = f.evidence as Record | undefined; + const serviceId = evidence?.serviceId; + return typeof serviceId === 'string' ? serviceId : null; + }) + .filter((id): id is string => Boolean(id)), + ), + ); + // Use a transaction to ensure atomicity - both run and results are created together - await db.$transaction(async (tx) => { + const runId = await db.$transaction(async (tx) => { // Create a scan run record const scanRun = await tx.integrationCheckRun.create({ data: { @@ -574,6 +606,7 @@ export class CloudSecurityService { totalChecked: findings.length, passedCount, failedCount, + scannedServices, }, }); @@ -594,7 +627,11 @@ export class CloudSecurityService { })), }); } + + return scanRun.id; }); + + return runId; } /** diff --git a/apps/api/src/cloud-security/dto/mark-exception.dto.ts b/apps/api/src/cloud-security/dto/mark-exception.dto.ts new file mode 100644 index 0000000000..8b7bdbfd90 --- /dev/null +++ b/apps/api/src/cloud-security/dto/mark-exception.dto.ts @@ -0,0 +1,28 @@ +import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger'; +import { IsDateString, IsOptional, IsString, MinLength } from 'class-validator'; + +export class MarkExceptionDto { + @ApiProperty({ + description: 'Documentation for why this finding does not apply or is being accepted. Minimum 20 characters.', + example: 'Bucket hosts intentionally public marketing assets; writes restricted to the marketing IAM role.', + }) + @IsString() + @MinLength(20, { message: 'Reason must be at least 20 characters.' }) + reason!: string; + + @ApiPropertyOptional({ + description: 'Free-text reviewer or approval reference.', + example: 'Approved by CISO 2026-Q1', + }) + @IsOptional() + @IsString() + reviewedBy?: string; + + @ApiPropertyOptional({ + description: 'ISO date when this exception should auto-expire. Null/missing = never.', + example: '2026-08-13', + }) + @IsOptional() + @IsDateString() + expiresAt?: string; +} diff --git a/apps/api/src/cloud-security/evidence-sanitizer.spec.ts b/apps/api/src/cloud-security/evidence-sanitizer.spec.ts new file mode 100644 index 0000000000..3c8ba64fd9 --- /dev/null +++ b/apps/api/src/cloud-security/evidence-sanitizer.spec.ts @@ -0,0 +1,219 @@ +import { REDACTED_VALUE, sanitizeEvidence } from './evidence-sanitizer'; + +describe('evidence-sanitizer', () => { + describe('top-level sensitive keys', () => { + it('redacts string values under sensitive keys', () => { + expect(sanitizeEvidence({ password: 'mySecret' })).toEqual({ + password: REDACTED_VALUE, + }); + }); + + it('matches case-insensitively', () => { + expect( + sanitizeEvidence({ Password: 'x', PASSWORD: 'y', PassWord: 'z' }), + ).toEqual({ + Password: REDACTED_VALUE, + PASSWORD: REDACTED_VALUE, + PassWord: REDACTED_VALUE, + }); + }); + + it('redacts every configured sensitive suffix pattern', () => { + const input: Record = { + password: 'a', + userPassword: 'a2', + secret: 'b', + clientSecret: 'b2', + token: 'c', + accessToken: 'c2', + refreshToken: 'c3', + bearerToken: 'c4', + credential: 'd', + credentials: 'd2', + awsCredentials: 'd3', + privateKey: 'e', + publicKey: 'e2', + accessKey: 'f', + accessKeyId: 'f2', + secretAccessKey: 'f3', + apiKey: 'g', + signingKey: 'h', + sessionId: 'i', + sessionToken: 'i2', + bearer: 'j', + authorization: 'k', + cookie: 'l', + }; + const result = sanitizeEvidence(input) as Record; + for (const key of Object.keys(input)) { + expect(result[key]).toBe(REDACTED_VALUE); + } + }); + + it('leaves non-sensitive keys unchanged', () => { + expect( + sanitizeEvidence({ + bucketName: 'logs-archive', + region: 'us-east-1', + accountId: '123456789012', + }), + ).toEqual({ + bucketName: 'logs-archive', + region: 'us-east-1', + accountId: '123456789012', + }); + }); + + it('does NOT redact keys that merely contain a sensitive word as a prefix or middle', () => { + // suffix-match means these stay visible — they are config info, not secrets + expect( + sanitizeEvidence({ + passwordPolicy: { minimumLength: 14 }, + requirePassword: true, + tokenLifetime: 3600, + secretsCount: 5, + }), + ).toEqual({ + passwordPolicy: { minimumLength: 14 }, + requirePassword: true, + tokenLifetime: 3600, + secretsCount: 5, + }); + }); + + it('does NOT redact booleans or numbers even when key is sensitive', () => { + // preserving non-string config like `requirePassword: true` is important + // for auditors — the FACT that a flag is set is not itself a secret + expect( + sanitizeEvidence({ + requireSecret: false, + tokenCount: 3, + }), + ).toEqual({ + requireSecret: false, + tokenCount: 3, + }); + }); + }); + + describe('nested objects', () => { + it('redacts sensitive keys deep in the tree', () => { + expect( + sanitizeEvidence({ + bucket: { + name: 'my-bucket', + config: { accessKey: 'AKIA1234' }, + }, + }), + ).toEqual({ + bucket: { + name: 'my-bucket', + config: { accessKey: REDACTED_VALUE }, + }, + }); + }); + + it('redacts whole nested object when its key is sensitive', () => { + expect( + sanitizeEvidence({ + bucket: { + name: 'logs', + credentials: { accessKeyId: 'AKIA', secretAccessKey: 'sk' }, + }, + }), + ).toEqual({ + bucket: { + name: 'logs', + credentials: REDACTED_VALUE, + }, + }); + }); + }); + + describe('arrays', () => { + it('sanitizes objects inside arrays', () => { + expect( + sanitizeEvidence({ + users: [ + { name: 'john', token: 'abc' }, + { name: 'alice', token: 'def' }, + ], + }), + ).toEqual({ + users: [ + { name: 'john', token: REDACTED_VALUE }, + { name: 'alice', token: REDACTED_VALUE }, + ], + }); + }); + + it('preserves arrays of primitives', () => { + expect( + sanitizeEvidence({ regions: ['us-east-1', 'eu-west-1'] }), + ).toEqual({ + regions: ['us-east-1', 'eu-west-1'], + }); + }); + + it('handles a top-level array', () => { + expect(sanitizeEvidence([{ token: 'a' }, { name: 'b' }])).toEqual([ + { token: REDACTED_VALUE }, + { name: 'b' }, + ]); + }); + }); + + describe('non-object inputs', () => { + it('returns null/undefined unchanged', () => { + expect(sanitizeEvidence(null)).toBeNull(); + expect(sanitizeEvidence(undefined)).toBeUndefined(); + }); + + it('returns primitives unchanged', () => { + expect(sanitizeEvidence('hello')).toBe('hello'); + expect(sanitizeEvidence(42)).toBe(42); + expect(sanitizeEvidence(true)).toBe(true); + expect(sanitizeEvidence(false)).toBe(false); + }); + }); + + describe('purity', () => { + it('does not mutate the input', () => { + const input = { + bucketName: 'logs', + accessKey: 'AKIA', + nested: { token: 'xyz' }, + }; + const snapshot = JSON.stringify(input); + sanitizeEvidence(input); + expect(JSON.stringify(input)).toBe(snapshot); + }); + }); + + describe('realistic AWS payload', () => { + it('keeps policy structure visible and redacts only credentials', () => { + const input = { + accountId: '123456789012', + passwordPolicy: { + minimumPasswordLength: 8, + requireSymbols: true, + requireUppercaseCharacters: false, + }, + region: 'us-east-1', + accessKeyId: 'AKIAIOSFODNN7EXAMPLE', + secretAccessKey: 'wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY', + }; + expect(sanitizeEvidence(input)).toEqual({ + accountId: '123456789012', + passwordPolicy: { + minimumPasswordLength: 8, + requireSymbols: true, + requireUppercaseCharacters: false, + }, + region: 'us-east-1', + accessKeyId: REDACTED_VALUE, + secretAccessKey: REDACTED_VALUE, + }); + }); + }); +}); diff --git a/apps/api/src/cloud-security/evidence-sanitizer.ts b/apps/api/src/cloud-security/evidence-sanitizer.ts new file mode 100644 index 0000000000..0e119746df --- /dev/null +++ b/apps/api/src/cloud-security/evidence-sanitizer.ts @@ -0,0 +1,72 @@ +/** + * Redacts sensitive values inside cloud-test evidence payloads before they + * are rendered to a user. + * + * Matching strategy — case-insensitive suffix match on the KEY name. A key + * is considered sensitive when its lowercase form ends with one of the + * SENSITIVE_SUFFIXES. This deliberately differs from substring matching: + * `passwordPolicy` and `requirePassword` keep their structure visible + * because their suffix is `policy`/`Password` respectively — only literal + * `password`, `userPassword`, etc. get redacted. + * + * Redaction rule — only string and nested-object values are redacted. + * Booleans and numbers under a sensitive key are preserved (e.g. config + * flags like `requirePassword: true` should stay visible to auditors). + * + * Add new entries to SENSITIVE_SUFFIXES as new sensitive field names are + * observed in real provider payloads. Removing an entry is a security + * regression — review carefully. + */ + +export const REDACTED_VALUE = '[REDACTED]'; + +const SENSITIVE_SUFFIXES: readonly string[] = [ + 'password', + 'secret', + 'token', + 'credential', + 'credentials', + 'privatekey', + 'publickey', + 'accesskey', + 'accesskeyid', + 'secretaccesskey', + 'apikey', + 'signingkey', + 'sessionid', + 'sessiontoken', + 'bearer', + 'authorization', + 'cookie', +]; + +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} + +function keyIsSensitive(key: string): boolean { + const normalized = key.toLowerCase(); + return SENSITIVE_SUFFIXES.some((suffix) => normalized.endsWith(suffix)); +} + +/** + * Recursively walks a JSON-like value and replaces values under sensitive + * keys with REDACTED_VALUE. Structure (keys, arrays, primitive types) is + * preserved so callers and auditors can still see what fields exist. + * + * Pure — does not mutate the input. + */ +export function sanitizeEvidence(value: unknown): unknown { + if (value === null || value === undefined) return value; + if (Array.isArray(value)) return value.map(sanitizeEvidence); + if (!isRecord(value)) return value; + + const result: Record = {}; + for (const key of Object.keys(value)) { + const child = value[key]; + const shouldRedact = + keyIsSensitive(key) && (typeof child === 'string' || isRecord(child)); + result[key] = shouldRedact ? REDACTED_VALUE : sanitizeEvidence(child); + } + return result; +} diff --git a/apps/api/src/cloud-security/exception.service.spec.ts b/apps/api/src/cloud-security/exception.service.spec.ts new file mode 100644 index 0000000000..2a033eba7a --- /dev/null +++ b/apps/api/src/cloud-security/exception.service.spec.ts @@ -0,0 +1,232 @@ +import { BadRequestException, NotFoundException } from '@nestjs/common'; + +// Mock @db so the Prisma client doesn't try to connect at import time, and +// so we can assert on what `markAsException` writes. +const dbMock = { + integrationCheckResult: { findFirst: jest.fn() }, + findingException: { + findFirst: jest.fn(), + create: jest.fn(), + update: jest.fn(), + }, + auditLog: { create: jest.fn() }, +}; + +jest.mock('@db', () => ({ db: dbMock })); + +// Mock the audit logger so we can assert on it without exercising the real +// DB call inside. +const auditLogMock = jest.fn().mockResolvedValue(undefined); +jest.mock('./cloud-security-audit', () => ({ + logCloudSecurityActivity: auditLogMock, +})); + +import { CloudExceptionService } from './exception.service'; + +function buildService() { + return new CloudExceptionService(); +} + +function withFinding(opts: { + findingKey: string; + resourceId: string; + connectionId: string; +}) { + dbMock.integrationCheckResult.findFirst.mockResolvedValueOnce({ + resourceId: opts.resourceId, + evidence: { findingKey: opts.findingKey }, + checkRun: { connectionId: opts.connectionId }, + }); +} + +describe('CloudExceptionService.markAsException', () => { + beforeEach(() => { + jest.clearAllMocks(); + dbMock.findingException.findFirst.mockReset(); + dbMock.findingException.create.mockReset(); + dbMock.findingException.update.mockReset(); + }); + + it('rejects reasons shorter than 20 characters', async () => { + await expect( + buildService().markAsException({ + findingId: 'icx_x', + organizationId: 'org_1', + userId: 'usr_1', + reason: 'too short', + }), + ).rejects.toThrow(BadRequestException); + }); + + it('rejects past expiration dates', async () => { + await expect( + buildService().markAsException({ + findingId: 'icx_x', + organizationId: 'org_1', + userId: 'usr_1', + reason: 'A perfectly normal documented reason for exception.', + expiresAt: new Date(Date.now() - 1000), + }), + ).rejects.toThrow(BadRequestException); + }); + + it('creates a new exception when none exists for this finding', async () => { + withFinding({ + findingKey: 'iam-no-mfa-john', + resourceId: 'john', + connectionId: 'icn_aws', + }); + dbMock.findingException.findFirst.mockResolvedValueOnce(null); + dbMock.findingException.create.mockResolvedValueOnce({ id: 'fex_new' }); + + const result = await buildService().markAsException({ + findingId: 'icx_1', + organizationId: 'org_1', + userId: 'usr_1', + reason: 'Public marketing bucket — intentional. Bucket policy locked.', + }); + + expect(result.id).toBe('fex_new'); + expect(dbMock.findingException.create).toHaveBeenCalledWith( + expect.objectContaining({ + data: expect.objectContaining({ + organizationId: 'org_1', + connectionId: 'icn_aws', + checkId: 'iam-no-mfa', // normalized from finding key + resourceId: 'john', + markedById: 'usr_1', + }), + }), + ); + expect(auditLogMock).toHaveBeenCalledWith( + expect.objectContaining({ action: 'exception_marked' }), + ); + }); + + it('updates an existing active exception instead of creating a duplicate', async () => { + withFinding({ + findingKey: 'iam-no-mfa-alice', + resourceId: 'alice', + connectionId: 'icn_aws', + }); + dbMock.findingException.findFirst.mockResolvedValueOnce({ id: 'fex_old' }); + dbMock.findingException.update.mockResolvedValueOnce({ id: 'fex_old' }); + + const result = await buildService().markAsException({ + findingId: 'icx_2', + organizationId: 'org_1', + userId: 'usr_1', + reason: 'Updated reason now exceeds the twenty char minimum length.', + }); + + expect(result.id).toBe('fex_old'); + expect(dbMock.findingException.update).toHaveBeenCalled(); + expect(dbMock.findingException.create).not.toHaveBeenCalled(); + }); + + it('rejects findings that lack a stable check/resource identity', async () => { + dbMock.integrationCheckResult.findFirst.mockResolvedValueOnce({ + resourceId: null, + evidence: null, + checkRun: { connectionId: 'icn_aws' }, + }); + await expect( + buildService().markAsException({ + findingId: 'icx_x', + organizationId: 'org_1', + userId: 'usr_1', + reason: 'A perfectly long, well-documented reason here for tests.', + }), + ).rejects.toThrow(BadRequestException); + }); +}); + +describe('CloudExceptionService.revokeException', () => { + beforeEach(() => { + jest.clearAllMocks(); + dbMock.findingException.findFirst.mockReset(); + dbMock.findingException.update.mockReset(); + }); + + it('throws NotFoundException when the exception does not exist', async () => { + dbMock.findingException.findFirst.mockResolvedValueOnce(null); + await expect( + buildService().revokeException({ + exceptionId: 'fex_missing', + organizationId: 'org_1', + userId: 'usr_1', + }), + ).rejects.toThrow(NotFoundException); + }); + + it('refuses to revoke an already-revoked exception', async () => { + dbMock.findingException.findFirst.mockResolvedValueOnce({ + id: 'fex_1', + connectionId: 'icn_aws', + checkId: 'iam-no-mfa', + resourceId: 'john', + revokedAt: new Date(), + }); + await expect( + buildService().revokeException({ + exceptionId: 'fex_1', + organizationId: 'org_1', + userId: 'usr_1', + }), + ).rejects.toThrow(BadRequestException); + }); + + it('writes revokedAt + revokedById and an audit log entry on success', async () => { + dbMock.findingException.findFirst.mockResolvedValueOnce({ + id: 'fex_1', + connectionId: 'icn_aws', + checkId: 'iam-no-mfa', + resourceId: 'john', + revokedAt: null, + }); + dbMock.findingException.update.mockResolvedValueOnce({ id: 'fex_1' }); + + await buildService().revokeException({ + exceptionId: 'fex_1', + organizationId: 'org_1', + userId: 'usr_1', + }); + + expect(dbMock.findingException.update).toHaveBeenCalledWith( + expect.objectContaining({ + data: expect.objectContaining({ revokedById: 'usr_1' }), + }), + ); + expect(auditLogMock).toHaveBeenCalledWith( + expect.objectContaining({ action: 'exception_revoked' }), + ); + }); +}); + +describe('CloudExceptionService.isExceptionActive', () => { + beforeEach(() => { + dbMock.findingException.findFirst.mockReset(); + }); + + it('returns true when a non-revoked, non-expired exception exists', async () => { + dbMock.findingException.findFirst.mockResolvedValueOnce({ id: 'fex_1' }); + const active = await buildService().isExceptionActive({ + organizationId: 'org_1', + connectionId: 'icn_aws', + checkId: 'iam-no-mfa', + resourceId: 'john', + }); + expect(active).toBe(true); + }); + + it('returns false when none match', async () => { + dbMock.findingException.findFirst.mockResolvedValueOnce(null); + const active = await buildService().isExceptionActive({ + organizationId: 'org_1', + connectionId: 'icn_aws', + checkId: 'iam-no-mfa', + resourceId: 'john', + }); + expect(active).toBe(false); + }); +}); diff --git a/apps/api/src/cloud-security/exception.service.ts b/apps/api/src/cloud-security/exception.service.ts new file mode 100644 index 0000000000..9fd3238e48 --- /dev/null +++ b/apps/api/src/cloud-security/exception.service.ts @@ -0,0 +1,211 @@ +import { + BadRequestException, + ForbiddenException, + Injectable, + Logger, + NotFoundException, +} from '@nestjs/common'; +import { db } from '@db'; +import { logCloudSecurityActivity } from './cloud-security-audit'; +import { normalizeCheckId } from './check-definition.utils'; + +/** Minimum chars for an exception reason — meant to discourage low-effort + * reasons like "ok" or "test". Auditors rely on this field as the + * documentation for why the finding isn't being fixed. */ +export const MIN_EXCEPTION_REASON_LENGTH = 20; + +export interface MarkExceptionInput { + findingId: string; + organizationId: string; + userId: string; + reason: string; + reviewedBy?: string | null; + expiresAt?: Date | null; +} + +@Injectable() +export class CloudExceptionService { + private readonly logger = new Logger(CloudExceptionService.name); + + /** + * Mark a finding as an exception. Idempotent per (orgId, connectionId, + * checkId, resourceId) — re-marking the same finding while an active + * exception exists is treated as an update of the existing exception. + */ + async markAsException(input: MarkExceptionInput): Promise<{ id: string }> { + if (input.reason.trim().length < MIN_EXCEPTION_REASON_LENGTH) { + throw new BadRequestException( + `Exception reason must be at least ${MIN_EXCEPTION_REASON_LENGTH} characters.`, + ); + } + if (input.expiresAt && input.expiresAt.getTime() < Date.now()) { + throw new BadRequestException( + 'Exception expiration date must be in the future.', + ); + } + + const lookup = await this.resolveFindingForException( + input.findingId, + input.organizationId, + ); + + // Reuse an active exception if one already exists for this finding — + // mark-then-edit avoids duplicate rows under the same (org, conn, check, + // resource) tuple. + const existing = await db.findingException.findFirst({ + where: { + organizationId: input.organizationId, + connectionId: lookup.connectionId, + checkId: lookup.checkId, + resourceId: lookup.resourceId, + revokedAt: null, + }, + }); + + let exceptionId: string; + if (existing) { + const updated = await db.findingException.update({ + where: { id: existing.id }, + data: { + reason: input.reason.trim(), + reviewedBy: input.reviewedBy ?? null, + expiresAt: input.expiresAt ?? null, + }, + }); + exceptionId = updated.id; + } else { + const created = await db.findingException.create({ + data: { + organizationId: input.organizationId, + connectionId: lookup.connectionId, + checkId: lookup.checkId, + resourceId: lookup.resourceId, + reason: input.reason.trim(), + reviewedBy: input.reviewedBy ?? null, + expiresAt: input.expiresAt ?? null, + markedById: input.userId, + }, + }); + exceptionId = created.id; + } + + await logCloudSecurityActivity({ + organizationId: input.organizationId, + userId: input.userId, + connectionId: lookup.connectionId, + action: 'exception_marked', + description: `Marked finding ${lookup.checkId}:${lookup.resourceId} as exception — ${input.reason.slice(0, 80)}${input.reason.length > 80 ? '…' : ''}`, + metadata: { + findingId: input.findingId, + exceptionId, + checkId: lookup.checkId, + resourceId: lookup.resourceId, + expiresAt: input.expiresAt?.toISOString() ?? null, + }, + }); + + return { id: exceptionId }; + } + + async revokeException(params: { + exceptionId: string; + organizationId: string; + userId: string; + }): Promise { + const existing = await db.findingException.findFirst({ + where: { id: params.exceptionId, organizationId: params.organizationId }, + }); + if (!existing) throw new NotFoundException('Exception not found'); + if (existing.revokedAt) { + throw new BadRequestException('Exception is already revoked.'); + } + + await db.findingException.update({ + where: { id: params.exceptionId }, + data: { revokedAt: new Date(), revokedById: params.userId }, + }); + + await logCloudSecurityActivity({ + organizationId: params.organizationId, + userId: params.userId, + connectionId: existing.connectionId, + action: 'exception_revoked', + description: `Revoked exception on ${existing.checkId}:${existing.resourceId}.`, + metadata: { + exceptionId: params.exceptionId, + checkId: existing.checkId, + resourceId: existing.resourceId, + }, + }); + } + + /** + * Active = exists for this (org, connection, check, resource), not + * revoked, and (no expiration OR expiration in the future). + * Used by getFindings to filter and by reconciliation to detect + * `exception_marked` resolutions. + */ + async isExceptionActive(params: { + organizationId: string; + connectionId: string; + checkId: string; + resourceId: string; + }): Promise { + const exception = await db.findingException.findFirst({ + where: { + organizationId: params.organizationId, + connectionId: params.connectionId, + checkId: params.checkId, + resourceId: params.resourceId, + revokedAt: null, + OR: [{ expiresAt: null }, { expiresAt: { gt: new Date() } }], + }, + select: { id: true }, + }); + return Boolean(exception); + } + + /** + * Resolve a finding ID into the (connectionId, checkId, resourceId) + * tuple that exception storage is keyed by. Forbids cross-tenant access. + */ + private async resolveFindingForException( + findingId: string, + organizationId: string, + ): Promise<{ connectionId: string; checkId: string; resourceId: string }> { + const result = await db.integrationCheckResult.findFirst({ + where: { + id: findingId, + checkRun: { connection: { organizationId } }, + }, + select: { + resourceId: true, + evidence: true, + checkRun: { select: { connectionId: true } }, + }, + }); + + if (!result) { + throw new ForbiddenException( + 'Finding not found, or it does not belong to your organization.', + ); + } + + const evidence = result.evidence as Record | null; + const rawFindingKey = + evidence && typeof evidence.findingKey === 'string' + ? evidence.findingKey + : null; + if (!rawFindingKey || !result.resourceId) { + throw new BadRequestException( + 'This finding cannot be marked as an exception — it lacks a stable check/resource identity.', + ); + } + + return { + connectionId: result.checkRun.connectionId, + checkId: normalizeCheckId(rawFindingKey, result.resourceId), + resourceId: result.resourceId, + }; + } +} diff --git a/apps/api/src/cloud-security/history.service.ts b/apps/api/src/cloud-security/history.service.ts new file mode 100644 index 0000000000..1a27060c56 --- /dev/null +++ b/apps/api/src/cloud-security/history.service.ts @@ -0,0 +1,66 @@ +import { Injectable } from '@nestjs/common'; +import { db } from '@db'; + +/** + * Aggregates Phase 5 audit-trail data (resolutions, active exceptions, + * regressions) for a single connection into one payload for the History tab. + */ +@Injectable() +export class CloudHistoryService { + async getHistory(params: { + organizationId: string; + connectionId: string; + }) { + const [resolutions, exceptions, regressions] = await Promise.all([ + db.findingResolution.findMany({ + where: { + organizationId: params.organizationId, + connectionId: params.connectionId, + }, + orderBy: { resolvedAt: 'desc' }, + take: 200, + }), + db.findingException.findMany({ + where: { + organizationId: params.organizationId, + connectionId: params.connectionId, + revokedAt: null, + OR: [{ expiresAt: null }, { expiresAt: { gt: new Date() } }], + }, + orderBy: { markedAt: 'desc' }, + take: 100, + }), + db.findingRegression.findMany({ + where: { + organizationId: params.organizationId, + connectionId: params.connectionId, + }, + orderBy: { regressedAt: 'desc' }, + take: 100, + }), + ]); + + return { + summary: { + resolutions: resolutions.length, + platformFixes: resolutions.filter( + (r) => r.resolutionMethod === 'platform_fix', + ).length, + externalFixes: resolutions.filter( + (r) => r.resolutionMethod === 'external_fix', + ).length, + resourceDeleted: resolutions.filter( + (r) => r.resolutionMethod === 'resource_deleted', + ).length, + exceptionMarked: resolutions.filter( + (r) => r.resolutionMethod === 'exception_marked', + ).length, + activeExceptions: exceptions.length, + regressions: regressions.length, + }, + resolutions, + exceptions, + regressions, + }; + } +} diff --git a/apps/api/src/cloud-security/reconciliation.service.spec.ts b/apps/api/src/cloud-security/reconciliation.service.spec.ts new file mode 100644 index 0000000000..41d56e9037 --- /dev/null +++ b/apps/api/src/cloud-security/reconciliation.service.spec.ts @@ -0,0 +1,280 @@ +// Reconciliation walks the IntegrationCheckRun pairs and writes +// FindingResolution + FindingRegression rows. These tests exercise the +// branching logic via a Prisma mock — no real DB needed. + +const dbMock = { + integrationCheckRun: { + findUnique: jest.fn(), + findFirst: jest.fn(), + }, + findingResolution: { + findFirst: jest.fn(), + create: jest.fn(), + }, + findingRegression: { + create: jest.fn(), + }, + remediationAction: { + findFirst: jest.fn(), + }, +}; + +jest.mock('@db', () => ({ + db: dbMock, + FindingResolutionMethod: { + platform_fix: 'platform_fix', + external_fix: 'external_fix', + resource_deleted: 'resource_deleted', + exception_marked: 'exception_marked', + }, +})); + +import { CloudReconciliationService } from './reconciliation.service'; +import type { CloudExceptionService } from './exception.service'; + +// Hand-rolled stub for the exception service — the reconciliation only calls +// isExceptionActive on it. +function makeExceptionsStub(active = false): CloudExceptionService { + return { + isExceptionActive: jest.fn().mockResolvedValue(active), + } as unknown as CloudExceptionService; +} + +const PRIOR_RUN_TIME = new Date('2026-05-01T00:00:00Z'); +const CURRENT_RUN_TIME = new Date('2026-05-13T00:00:00Z'); + +function makeResult(opts: { + findingKey: string; + resourceId: string; + passed: boolean; + resourceType?: string; + serviceId?: string; +}) { + return { + passed: opts.passed, + resourceId: opts.resourceId, + resourceType: opts.resourceType ?? 'AwsIamUser', + evidence: { + findingKey: opts.findingKey, + serviceId: opts.serviceId ?? 'iam', + }, + }; +} + +function setupRuns(opts: { + currentResults: ReturnType[]; + priorResults: ReturnType[]; + scannedServices?: string[]; +}) { + dbMock.integrationCheckRun.findUnique.mockResolvedValueOnce({ + id: 'icr_current', + connectionId: 'icn_aws', + status: 'success', + startedAt: CURRENT_RUN_TIME, + completedAt: CURRENT_RUN_TIME, + scannedServices: opts.scannedServices ?? [], + connection: { organizationId: 'org_1' }, + results: opts.currentResults, + }); + dbMock.integrationCheckRun.findFirst.mockResolvedValueOnce({ + id: 'icr_prior', + completedAt: PRIOR_RUN_TIME, + results: opts.priorResults, + }); +} + +describe('CloudReconciliationService.reconcile', () => { + beforeEach(() => { + jest.clearAllMocks(); + dbMock.findingResolution.findFirst.mockResolvedValue(null); + dbMock.findingRegression.create.mockResolvedValue({ id: 'freg_x' }); + dbMock.findingResolution.create.mockResolvedValue({ id: 'fres_x' }); + dbMock.remediationAction.findFirst.mockResolvedValue(null); + }); + + it('skips reconciliation when current run is not successful', async () => { + dbMock.integrationCheckRun.findUnique.mockResolvedValueOnce({ + id: 'icr_current', + status: 'failed', + connection: { organizationId: 'org_1' }, + results: [], + }); + const service = new CloudReconciliationService(makeExceptionsStub()); + const result = await service.reconcile({ currentRunId: 'icr_current' }); + expect(result).toEqual({ resolutions: 0, regressions: 0, skipped: true }); + }); + + it('is idempotent — skips if FindingResolution rows already exist for this run', async () => { + dbMock.integrationCheckRun.findUnique.mockResolvedValueOnce({ + id: 'icr_current', + status: 'success', + connection: { organizationId: 'org_1' }, + results: [], + scannedServices: [], + }); + dbMock.findingResolution.findFirst.mockResolvedValueOnce({ id: 'fres_existing' }); + + const service = new CloudReconciliationService(makeExceptionsStub()); + const result = await service.reconcile({ currentRunId: 'icr_current' }); + expect(result.skipped).toBe(true); + expect(dbMock.findingResolution.create).not.toHaveBeenCalled(); + }); + + it('returns 0/0 on first scan (no prior run)', async () => { + dbMock.integrationCheckRun.findUnique.mockResolvedValueOnce({ + id: 'icr_current', + status: 'success', + connection: { organizationId: 'org_1' }, + results: [], + scannedServices: [], + }); + 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 }); + }); + + it('records resource_deleted when a prior failure is missing from the current run', async () => { + setupRuns({ + currentResults: [], + priorResults: [ + makeResult({ findingKey: 'iam-no-mfa-john', resourceId: 'john', passed: false }), + ], + }); + + const service = new CloudReconciliationService(makeExceptionsStub()); + const result = await service.reconcile({ currentRunId: 'icr_current' }); + + expect(result.resolutions).toBe(1); + expect(dbMock.findingResolution.create).toHaveBeenCalledWith( + expect.objectContaining({ + data: expect.objectContaining({ + resolutionMethod: 'resource_deleted', + checkId: 'iam-no-mfa', + resourceId: 'john', + }), + }), + ); + }); + + it('records external_fix when a prior failure is now passing and no RemediationAction matches', async () => { + setupRuns({ + currentResults: [ + makeResult({ findingKey: 'iam-no-mfa-john', resourceId: 'john', passed: true }), + ], + priorResults: [ + makeResult({ findingKey: 'iam-no-mfa-john', resourceId: 'john', passed: false }), + ], + }); + + const service = new CloudReconciliationService(makeExceptionsStub()); + await service.reconcile({ currentRunId: 'icr_current' }); + + expect(dbMock.findingResolution.create).toHaveBeenCalledWith( + expect.objectContaining({ + data: expect.objectContaining({ resolutionMethod: 'external_fix' }), + }), + ); + }); + + it('records platform_fix when a successful RemediationAction lands between scans', async () => { + setupRuns({ + currentResults: [ + makeResult({ findingKey: 'iam-no-mfa-john', resourceId: 'john', passed: true }), + ], + priorResults: [ + makeResult({ findingKey: 'iam-no-mfa-john', resourceId: 'john', passed: false }), + ], + }); + dbMock.remediationAction.findFirst.mockResolvedValueOnce({ + id: 'rma_1', + initiatedById: 'usr_42', + }); + + const service = new CloudReconciliationService(makeExceptionsStub()); + await service.reconcile({ currentRunId: 'icr_current' }); + + expect(dbMock.findingResolution.create).toHaveBeenCalledWith( + expect.objectContaining({ + data: expect.objectContaining({ + resolutionMethod: 'platform_fix', + remediationActionId: 'rma_1', + resolvedById: 'usr_42', + }), + }), + ); + }); + + it('records exception_marked when the user marked an exception between scans', async () => { + setupRuns({ + currentResults: [], + priorResults: [ + makeResult({ findingKey: 'iam-no-mfa-john', resourceId: 'john', passed: false }), + ], + }); + + const service = new CloudReconciliationService(makeExceptionsStub(true)); + await service.reconcile({ currentRunId: 'icr_current' }); + + expect(dbMock.findingResolution.create).toHaveBeenCalledWith( + expect.objectContaining({ + data: expect.objectContaining({ resolutionMethod: 'exception_marked' }), + }), + ); + }); + + it('does NOT mark resolved when the prior finding\'s service was not in scannedServices (partial scan)', async () => { + setupRuns({ + currentResults: [], // partial scan — IAM service didn't run this time + priorResults: [ + makeResult({ + findingKey: 'iam-no-mfa-john', + resourceId: 'john', + passed: false, + serviceId: 'iam', + }), + ], + scannedServices: ['s3'], // only S3 ran this time + }); + + const service = new CloudReconciliationService(makeExceptionsStub()); + const result = await service.reconcile({ currentRunId: 'icr_current' }); + + expect(result.resolutions).toBe(0); + expect(dbMock.findingResolution.create).not.toHaveBeenCalled(); + }); + + it('records a regression when a previously-resolved finding fails again', async () => { + setupRuns({ + currentResults: [ + makeResult({ findingKey: 'iam-no-mfa-john', resourceId: 'john', passed: false }), + ], + priorResults: [ + // prior was passing — i.e. fix held, now broken again + makeResult({ findingKey: 'iam-no-mfa-john', resourceId: 'john', passed: true }), + ], + }); + dbMock.findingResolution.findFirst + .mockResolvedValueOnce(null) // idempotency probe + .mockResolvedValueOnce({ + // last-resolved lookup + id: 'fres_old', + resolvedAt: PRIOR_RUN_TIME, + }); + + const service = new CloudReconciliationService(makeExceptionsStub()); + const result = await service.reconcile({ currentRunId: 'icr_current' }); + + expect(result.regressions).toBe(1); + expect(dbMock.findingRegression.create).toHaveBeenCalledWith( + expect.objectContaining({ + data: expect.objectContaining({ + checkId: 'iam-no-mfa', + resourceId: 'john', + previousResolutionId: 'fres_old', + }), + }), + ); + }); +}); diff --git a/apps/api/src/cloud-security/reconciliation.service.ts b/apps/api/src/cloud-security/reconciliation.service.ts new file mode 100644 index 0000000000..e2b1a356d6 --- /dev/null +++ b/apps/api/src/cloud-security/reconciliation.service.ts @@ -0,0 +1,306 @@ +import { Injectable, Logger } from '@nestjs/common'; +import { db, FindingResolutionMethod } from '@db'; +import { normalizeCheckId } from './check-definition.utils'; +import { CloudExceptionService } from './exception.service'; + +interface NormalizedResult { + findingKey: string; + resourceId: string; + resourceType: string | null; + serviceId: string | null; + passed: boolean; +} + +/** + * Reconciles the most-recent terminal-state IntegrationCheckRun for a + * connection against its prior run, writing FindingResolution and + * FindingRegression rows. Idempotent per `currentRunId` — re-running on + * the same run is a no-op (existing rows scoped to that run are + * detected and skipped). + * + * Edge cases handled: + * - First scan: no prior run → nothing to reconcile. + * - Partial scan: if a service isn't in `scannedServices`, prior + * findings for that service are NOT marked resolved — they're carried + * forward as unknown. + * - Resource renamed: appears as old-resolved + new-failure. v1 limitation + * (documented in plan); v2 would track underlying ARN. + */ +@Injectable() +export class CloudReconciliationService { + private readonly logger = new Logger(CloudReconciliationService.name); + + constructor(private readonly exceptions: CloudExceptionService) {} + + async reconcile(params: { currentRunId: string }): Promise<{ + resolutions: number; + regressions: number; + skipped: boolean; + }> { + const currentRun = await db.integrationCheckRun.findUnique({ + where: { id: params.currentRunId }, + select: { + id: true, + connectionId: true, + completedAt: true, + startedAt: true, + status: true, + scannedServices: true, + connection: { select: { organizationId: true } }, + results: { + select: { + passed: true, + resourceId: true, + resourceType: true, + evidence: true, + }, + }, + }, + }); + + if (!currentRun || currentRun.status !== 'success') { + // Don't reconcile against an incomplete scan — incorrect "resolved" + // events would mislead auditors. Only successful scans produce truth. + return { resolutions: 0, regressions: 0, skipped: true }; + } + + // Idempotency guard — if reconciliation already wrote rows for this run, + // skip silently. + const alreadyDone = await db.findingResolution.findFirst({ + where: { detectedInRunId: currentRun.id }, + select: { id: true }, + }); + if (alreadyDone) { + return { resolutions: 0, regressions: 0, skipped: true }; + } + + const priorRun = await db.integrationCheckRun.findFirst({ + where: { + connectionId: currentRun.connectionId, + id: { not: currentRun.id }, + status: 'success', + completedAt: { lt: currentRun.completedAt ?? currentRun.startedAt ?? new Date() }, + }, + orderBy: { completedAt: 'desc' }, + select: { + id: true, + completedAt: true, + results: { + select: { + passed: true, + resourceId: true, + resourceType: true, + evidence: true, + }, + }, + }, + }); + + if (!priorRun) { + // First scan — nothing to reconcile against. + return { resolutions: 0, regressions: 0, skipped: false }; + } + + const organizationId = currentRun.connection.organizationId; + const scannedServices = new Set(currentRun.scannedServices ?? []); + const treatPartialScan = scannedServices.size > 0; // empty = scan didn't track services; treat as "all scanned" + + const currentMap = indexResults(currentRun.results); + const priorMap = indexResults(priorRun.results); + + let resolutions = 0; + let regressions = 0; + + // Resolutions: prior failed → current absent or passed. + for (const [key, prior] of priorMap.entries()) { + if (!prior.passed === false) continue; // only interested in prior failures + if (prior.passed) continue; + + const current = currentMap.get(key); + if (current && !current.passed) continue; // still failing + + const checkKey = normalizeCheckId(prior.findingKey, prior.resourceId); + + if (treatPartialScan && prior.serviceId && !scannedServices.has(prior.serviceId)) { + // Service wasn't scanned this run — carry forward, don't resolve. + continue; + } + + const method = await this.determineResolutionMethod({ + organizationId, + connectionId: currentRun.connectionId, + priorRunCompletedAt: priorRun.completedAt, + currentRunStartedAt: + currentRun.startedAt ?? currentRun.completedAt ?? new Date(), + checkKey, + resourceId: prior.resourceId, + currentExists: Boolean(current), + }); + + const daysOpen = + priorRun.completedAt && currentRun.completedAt + ? daysBetween(priorRun.completedAt, currentRun.completedAt) + : null; + + await db.findingResolution.create({ + data: { + organizationId, + connectionId: currentRun.connectionId, + checkId: checkKey, + resourceId: prior.resourceId, + resourceType: prior.resourceType, + resolvedAt: currentRun.completedAt ?? new Date(), + resolutionMethod: method.method, + resolvedById: method.resolvedById ?? null, + remediationActionId: method.remediationActionId ?? null, + resolvedFromRunId: priorRun.id, + detectedInRunId: currentRun.id, + daysOpen, + }, + }); + resolutions += 1; + } + + // Regressions: previously-resolved (checkKey, resourceId) failing again + // in current run. Look back across ALL prior resolutions for this + // connection — not just the immediate-prior run. + for (const [key, current] of currentMap.entries()) { + if (current.passed) continue; + const checkKey = normalizeCheckId(current.findingKey, current.resourceId); + + const lastResolution = await db.findingResolution.findFirst({ + where: { + connectionId: currentRun.connectionId, + checkId: checkKey, + resourceId: current.resourceId, + }, + orderBy: { resolvedAt: 'desc' }, + select: { id: true, resolvedAt: true }, + }); + if (!lastResolution) continue; + + // If this same (checkKey, resourceId) was failing in priorRun too, it's + // a still-failing finding, not a regression. + if (priorMap.get(key) && !priorMap.get(key)!.passed) continue; + + const daysClean = currentRun.completedAt + ? daysBetween(lastResolution.resolvedAt, currentRun.completedAt) + : null; + + await db.findingRegression.create({ + data: { + organizationId, + connectionId: currentRun.connectionId, + checkId: checkKey, + resourceId: current.resourceId, + previouslyResolvedAt: lastResolution.resolvedAt, + previousResolutionId: lastResolution.id, + regressedAt: currentRun.completedAt ?? new Date(), + detectedInRunId: currentRun.id, + daysClean, + }, + }); + regressions += 1; + } + + this.logger.log( + `Reconciled run ${currentRun.id}: ${resolutions} resolved, ${regressions} regressed`, + ); + return { resolutions, regressions, skipped: false }; + } + + private async determineResolutionMethod(input: { + organizationId: string; + connectionId: string; + priorRunCompletedAt: Date | null; + currentRunStartedAt: Date; + checkKey: string; + resourceId: string; + currentExists: boolean; + }): Promise<{ + method: FindingResolutionMethod; + resolvedById?: string; + remediationActionId?: string; + }> { + // 1) Did the user mark it as an exception between scans? + const exceptionActive = await this.exceptions.isExceptionActive({ + organizationId: input.organizationId, + connectionId: input.connectionId, + checkId: input.checkKey, + resourceId: input.resourceId, + }); + if (exceptionActive) return { method: 'exception_marked' }; + + // 2) Did our Fix button apply a successful remediation between scans? + if (input.priorRunCompletedAt) { + const fix = await db.remediationAction.findFirst({ + where: { + connectionId: input.connectionId, + resourceId: input.resourceId, + status: 'success', + createdAt: { + gte: input.priorRunCompletedAt, + lte: input.currentRunStartedAt, + }, + }, + orderBy: { createdAt: 'desc' }, + select: { id: true, initiatedById: true }, + }); + if (fix) { + return { + method: 'platform_fix', + remediationActionId: fix.id, + resolvedById: fix.initiatedById ?? undefined, + }; + } + } + + // 3) Resource missing entirely from current results → deleted. + if (!input.currentExists) return { method: 'resource_deleted' }; + + // 4) Otherwise — customer fixed it outside our platform. + return { method: 'external_fix' }; + } +} + +function indexResults( + results: Array<{ + passed: boolean; + resourceId: string | null; + resourceType: string | null; + evidence: unknown; + }>, +): Map { + const map = new Map(); + for (const r of results) { + if (!r.resourceId) continue; + const evidence = + r.evidence && typeof r.evidence === 'object' + ? (r.evidence as Record) + : null; + const findingKey = + evidence && typeof evidence.findingKey === 'string' + ? evidence.findingKey + : null; + if (!findingKey) continue; + const serviceId = + evidence && typeof evidence.serviceId === 'string' + ? evidence.serviceId + : null; + const checkKey = normalizeCheckId(findingKey, r.resourceId); + const compositeKey = `${checkKey}::${r.resourceId}`; + map.set(compositeKey, { + findingKey, + resourceId: r.resourceId, + resourceType: r.resourceType, + serviceId, + passed: r.passed, + }); + } + return map; +} + +function daysBetween(a: Date, b: Date): number { + const ms = b.getTime() - a.getTime(); + return Math.max(0, Math.floor(ms / (24 * 60 * 60 * 1000))); +} diff --git a/apps/api/src/comments/comments-permission.guard.spec.ts b/apps/api/src/comments/comments-permission.guard.spec.ts new file mode 100644 index 0000000000..99284221b5 --- /dev/null +++ b/apps/api/src/comments/comments-permission.guard.spec.ts @@ -0,0 +1,200 @@ +import type { ExecutionContext } from '@nestjs/common'; +import { ForbiddenException } from '@nestjs/common'; +import { Reflector } from '@nestjs/core'; + +// Mock @db before importing the guard so the Prisma client doesn't try to +// initialize against a real DATABASE_URL in this unit-test env. +jest.mock('@db', () => ({ + CommentEntityType: { + task: 'task', + vendor: 'vendor', + risk: 'risk', + policy: 'policy', + finding: 'finding', + }, +})); + +// Mock the permission.guard module so we don't pull better-auth's init chain +// (the guard only borrows the PERMISSIONS_KEY constant + RequiredPermission +// type — both safe to redefine here). +jest.mock('../auth/permission.guard', () => ({ + PERMISSIONS_KEY: 'permissions', +})); + +jest.mock('../auth/auth.server', () => ({ + auth: { api: { hasPermission: jest.fn() } }, +})); + +import { CommentsPermissionGuard } from './comments-permission.guard'; +import { auth } from '../auth/auth.server'; + +type MockRequest = { + method: string; + query?: Record; + body?: Record; + headers: Record; + isApiKey?: boolean; + isServiceToken?: boolean; + isPlatformAdmin?: boolean; +}; + +function makeContext(request: MockRequest): ExecutionContext { + return { + getHandler: () => ({}), + getClass: () => ({}), + switchToHttp: () => ({ + getRequest: () => request, + getResponse: () => ({}), + getNext: () => ({}), + }), + } as unknown as ExecutionContext; +} + +const hasPermissionMock = auth.api.hasPermission as unknown as jest.Mock; + +function reflectorWith(resource: string, action: string): Reflector { + const reflector = new Reflector(); + jest + .spyOn(reflector, 'getAllAndOverride') + .mockReturnValue([{ resource, actions: [action] }]); + return reflector; +} + +describe('CommentsPermissionGuard', () => { + beforeEach(() => { + hasPermissionMock.mockReset(); + }); + + it('resolves entityType from POST body and checks finding:update when finding', async () => { + hasPermissionMock.mockResolvedValueOnce({ success: true }); + const guard = new CommentsPermissionGuard( + reflectorWith('task', 'update'), + ); + const context = makeContext({ + method: 'POST', + body: { entityType: 'finding' }, + headers: { cookie: 'session=abc' }, + }); + await expect(guard.canActivate(context)).resolves.toBe(true); + expect(hasPermissionMock).toHaveBeenCalledWith( + expect.objectContaining({ + body: expect.objectContaining({ + permissions: { finding: ['update'] }, + }), + }), + ); + }); + + it('resolves entityType from GET query and checks finding:read when finding', async () => { + hasPermissionMock.mockResolvedValueOnce({ success: true }); + const guard = new CommentsPermissionGuard(reflectorWith('task', 'read')); + const context = makeContext({ + method: 'GET', + query: { entityType: 'finding' }, + headers: { cookie: 'session=abc' }, + }); + await expect(guard.canActivate(context)).resolves.toBe(true); + expect(hasPermissionMock).toHaveBeenCalledWith( + expect.objectContaining({ + body: expect.objectContaining({ + permissions: { finding: ['read'] }, + }), + }), + ); + }); + + it('falls back to the metadata resource when entityType is missing (PUT)', async () => { + hasPermissionMock.mockResolvedValueOnce({ success: true }); + const guard = new CommentsPermissionGuard( + reflectorWith('task', 'update'), + ); + const context = makeContext({ + method: 'PUT', + body: { content: 'edit' }, + headers: { cookie: 'session=abc' }, + }); + await expect(guard.canActivate(context)).resolves.toBe(true); + expect(hasPermissionMock).toHaveBeenCalledWith( + expect.objectContaining({ + body: expect.objectContaining({ + permissions: { task: ['update'] }, + }), + }), + ); + }); + + it('falls back to the metadata resource for unknown entityType values', async () => { + hasPermissionMock.mockResolvedValueOnce({ success: true }); + const guard = new CommentsPermissionGuard( + reflectorWith('task', 'update'), + ); + const context = makeContext({ + method: 'POST', + body: { entityType: 'not-an-entity' }, + headers: { cookie: 'session=abc' }, + }); + await guard.canActivate(context); + expect(hasPermissionMock).toHaveBeenCalledWith( + expect.objectContaining({ + body: expect.objectContaining({ + permissions: { task: ['update'] }, + }), + }), + ); + }); + + it('throws ForbiddenException when better-auth rejects', async () => { + hasPermissionMock.mockResolvedValueOnce({ success: false }); + const guard = new CommentsPermissionGuard( + reflectorWith('task', 'update'), + ); + const context = makeContext({ + method: 'POST', + body: { entityType: 'finding' }, + headers: { cookie: 'session=abc' }, + }); + await expect(guard.canActivate(context)).rejects.toThrow( + ForbiddenException, + ); + }); + + it('returns true without invoking better-auth for API keys (handled by scope check upstream)', async () => { + const guard = new CommentsPermissionGuard( + reflectorWith('task', 'update'), + ); + const context = makeContext({ + method: 'POST', + body: { entityType: 'finding' }, + headers: {}, + isApiKey: true, + }); + await expect(guard.canActivate(context)).resolves.toBe(true); + expect(hasPermissionMock).not.toHaveBeenCalled(); + }); + + it('returns true without invoking better-auth for platform admins', async () => { + const guard = new CommentsPermissionGuard( + reflectorWith('task', 'update'), + ); + const context = makeContext({ + method: 'POST', + body: { entityType: 'finding' }, + headers: {}, + isPlatformAdmin: true, + }); + await expect(guard.canActivate(context)).resolves.toBe(true); + expect(hasPermissionMock).not.toHaveBeenCalled(); + }); + + it('returns true when no @RequirePermission metadata is set (endpoint opted out)', async () => { + const reflector = new Reflector(); + jest.spyOn(reflector, 'getAllAndOverride').mockReturnValue(undefined); + const guard = new CommentsPermissionGuard(reflector); + const context = makeContext({ + method: 'POST', + body: { entityType: 'finding' }, + headers: {}, + }); + await expect(guard.canActivate(context)).resolves.toBe(true); + }); +}); diff --git a/apps/api/src/comments/comments-permission.guard.ts b/apps/api/src/comments/comments-permission.guard.ts new file mode 100644 index 0000000000..3b6d375649 --- /dev/null +++ b/apps/api/src/comments/comments-permission.guard.ts @@ -0,0 +1,134 @@ +import { + CanActivate, + ExecutionContext, + ForbiddenException, + Injectable, + Logger, +} from '@nestjs/common'; +import { Reflector } from '@nestjs/core'; +import { CommentEntityType } from '@db'; +import type { Request } from 'express'; +import { auth } from '../auth/auth.server'; +import { + PERMISSIONS_KEY, + type RequiredPermission, +} from '../auth/permission.guard'; +import type { AuthenticatedRequest } from '../auth/types'; + +/** + * Local request shape — combines the express-side fields we need + * (`query`, `body`, `method`) with the project-wide AuthenticatedRequest + * augmentations. The project's `AuthenticatedRequest` extends the global + * fetch-API `Request` (no query/body), so we re-augment it here. + */ +type GuardedRequest = AuthenticatedRequest & Request; + +/** + * Comment endpoints support multiple entity types (task / policy / vendor / + * risk / finding). The required permission is therefore dynamic — read by + * `${entityType}:${action}` rather than the hardcoded `task:${action}` the + * default PermissionGuard would enforce. + * + * Why it's needed: auditors hold `finding:update` but NOT `task:update`, + * yet the only way to collaborate on a finding is to comment on it. The + * vanilla guard would block them at the door. + * + * Implementation strategy: + * 1. Read `@RequirePermission` metadata for the FALLBACK action (kept + * intact so AuditLogInterceptor still derives the right resource/ + * action for its log line — close enough for audit trail purposes). + * 2. Resolve the actual entityType from the request: + * - GET → query string (`?entityType=finding`) + * - POST → request body (`{ entityType: "finding" }`) + * - PUT / DELETE → fall back to the metadata's resource (commentId + * is opaque; resolving entityType would need a DB lookup which + * Phase 4 v1 explicitly defers — author-only editing keeps the + * existing task-permission gate in practice). + * 3. Call better-auth's `hasPermission` with the resolved permission. + */ +@Injectable() +export class CommentsPermissionGuard implements CanActivate { + private readonly logger = new Logger(CommentsPermissionGuard.name); + + constructor(private readonly reflector: Reflector) {} + + async canActivate(context: ExecutionContext): Promise { + const required = this.reflector.getAllAndOverride( + PERMISSIONS_KEY, + [context.getHandler(), context.getClass()], + ); + + if (!required || required.length === 0) return true; + + const request = context.switchToHttp().getRequest(); + + // API keys and service tokens use the existing fallback behaviour from + // the standard PermissionGuard — no entity-type-aware logic needed. + // Defer to scope checks against the literal metadata. + if (request.isApiKey || request.isServiceToken || request.isPlatformAdmin) { + return true; + } + + const fallback = required[0]; + const action = fallback.actions[0]; + const resource = this.resolveEntityResource(request, fallback.resource); + + const permissions: Record = { [resource]: [action] }; + + const allowed = await this.checkPermission(request, permissions); + if (!allowed) { + this.logger.warn( + `[CommentsPermissionGuard] Denied ${request.method} ${request.url}. Required: ${resource}:${action}`, + ); + throw new ForbiddenException('Access denied'); + } + return true; + } + + private resolveEntityResource( + request: GuardedRequest, + fallback: string, + ): string { + const method = request.method?.toUpperCase(); + let candidate: unknown; + if (method === 'GET') { + candidate = (request.query as Record | undefined) + ?.entityType; + } else if (method === 'POST') { + candidate = (request.body as Record | undefined) + ?.entityType; + } + + if (typeof candidate !== 'string') return fallback; + // Trust only enum values — anything else falls back to the metadata. + return (Object.values(CommentEntityType) as string[]).includes(candidate) + ? candidate + : fallback; + } + + /** + * Mirrors the cookie/header forwarding pattern of the standard + * PermissionGuard so role-based and custom-role permissions both work. + * Kept private and inline rather than extracted to a shared utility — + * Phase 4 introduces only this one extra caller. + */ + private async checkPermission( + request: GuardedRequest, + permissions: Record, + ): Promise { + const headers = new Headers(); + const authHeader = request.headers['authorization'] as string; + if (authHeader) headers.set('authorization', authHeader); + const cookieHeader = request.headers['cookie'] as string; + if (cookieHeader) headers.set('cookie', cookieHeader); + + if (!authHeader && !cookieHeader) return false; + + // Spell the union-typed body out so zod 4 inside better-auth doesn't + // reject the missing `permission` discriminator. Same gotcha as the + // standard PermissionGuard. + const body = { permissions, permission: undefined }; + const result = await auth.api.hasPermission({ headers, body }); + return result.success === true; + } +} diff --git a/apps/api/src/comments/comments.controller.ts b/apps/api/src/comments/comments.controller.ts index 7329aa6341..006b92bcb9 100644 --- a/apps/api/src/comments/comments.controller.ts +++ b/apps/api/src/comments/comments.controller.ts @@ -22,9 +22,9 @@ import { } from '@nestjs/swagger'; import { AuthContext, OrganizationId } from '../auth/auth-context.decorator'; import { HybridAuthGuard } from '../auth/hybrid-auth.guard'; -import { PermissionGuard } from '../auth/permission.guard'; import { RequirePermission } from '../auth/require-permission.decorator'; import type { AuthContext as AuthContextType } from '../auth/types'; +import { CommentsPermissionGuard } from './comments-permission.guard'; import { CommentsService } from './comments.service'; import { CommentResponseDto } from './dto/comment-responses.dto'; import { CreateCommentDto } from './dto/create-comment.dto'; @@ -33,7 +33,10 @@ import { UpdateCommentDto } from './dto/update-comment.dto'; @ApiTags('Comments') @Controller({ path: 'comments', version: '1' }) -@UseGuards(HybridAuthGuard, PermissionGuard) +// CommentsPermissionGuard replaces the standard PermissionGuard so that +// permission checks scale by entityType (task/finding/policy/vendor/risk) +// rather than always gating on `task:*`. See comments-permission.guard.ts. +@UseGuards(HybridAuthGuard, CommentsPermissionGuard) @ApiSecurity('apikey') export class CommentsController { constructor(private readonly commentsService: CommentsService) {} diff --git a/apps/api/src/comments/comments.module.ts b/apps/api/src/comments/comments.module.ts index 763eb7379f..0c29ceff68 100644 --- a/apps/api/src/comments/comments.module.ts +++ b/apps/api/src/comments/comments.module.ts @@ -2,6 +2,7 @@ import { Module } from '@nestjs/common'; import { AttachmentsModule } from '../attachments/attachments.module'; import { AuthModule } from '../auth/auth.module'; import { CommentsController } from './comments.controller'; +import { CommentsPermissionGuard } from './comments-permission.guard'; import { CommentsService } from './comments.service'; import { CommentMentionNotifierService } from './comment-mention-notifier.service'; import { NovuService } from '../notifications/novu.service'; @@ -9,7 +10,12 @@ import { NovuService } from '../notifications/novu.service'; @Module({ imports: [AuthModule, AttachmentsModule], // Import AuthModule for HybridAuthGuard dependencies controllers: [CommentsController], - providers: [CommentsService, CommentMentionNotifierService, NovuService], + providers: [ + CommentsService, + CommentsPermissionGuard, + CommentMentionNotifierService, + NovuService, + ], exports: [CommentsService], }) export class CommentsModule {} diff --git a/apps/app/src/app/(app)/[orgId]/cloud-tests/components/CheckDefinitionPanel.test.tsx b/apps/app/src/app/(app)/[orgId]/cloud-tests/components/CheckDefinitionPanel.test.tsx new file mode 100644 index 0000000000..59047da4e2 --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/cloud-tests/components/CheckDefinitionPanel.test.tsx @@ -0,0 +1,110 @@ +import { render, screen } from '@testing-library/react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +type SwrShape = { + data: { data: unknown } | undefined; + error: unknown; + isLoading: boolean; +}; + +const mockUseSWR = vi.fn<() => SwrShape>(); + +vi.mock('@/hooks/use-api', () => ({ + useApi: () => ({ + useSWR: (_url: string) => mockUseSWR(), + }), +})); + +import { CheckDefinitionPanel } from './CheckDefinitionPanel'; + +describe('CheckDefinitionPanel', () => { + beforeEach(() => { + mockUseSWR.mockReset(); + }); + + it('renders a loading skeleton while the fetch is in flight', () => { + mockUseSWR.mockReturnValue({ + data: undefined, + error: null, + isLoading: true, + }); + render(); + expect( + screen.getByText(/Generating check description/i), + ).toBeInTheDocument(); + }); + + it('renders all Tier 3 fields when the AI/provider responds successfully', () => { + mockUseSWR.mockReturnValue({ + data: { + data: { + data: { + title: 'IAM password policy enforces 14+ character minimum', + description: + 'Verifies that the AWS account password policy requires user passwords to be at least 14 characters long.', + passCriteria: + 'Password policy exists AND MinimumPasswordLength >= 14', + failCriteria: + 'No password policy OR MinimumPasswordLength < 14', + whyItMatters: + 'Short passwords are vulnerable to brute force attacks.', + source: 'ai', + }, + }, + }, + error: null, + isLoading: false, + }); + render(); + expect(screen.getByText('About this check')).toBeInTheDocument(); + expect(screen.getByText('What it checks')).toBeInTheDocument(); + expect(screen.getByText('Pass criteria')).toBeInTheDocument(); + expect(screen.getByText('Fail criteria')).toBeInTheDocument(); + expect(screen.getByText('Why it matters')).toBeInTheDocument(); + expect( + screen.getByText(/at least 14 characters long/i), + ).toBeInTheDocument(); + }); + + it('renders nothing when fetch errors (graceful degrade)', () => { + mockUseSWR.mockReturnValue({ + data: undefined, + error: new Error('500'), + isLoading: false, + }); + const { container } = render(); + expect(container.firstChild).toBeNull(); + }); + + it('renders nothing when AI returned null (e.g. legacy with no description)', () => { + mockUseSWR.mockReturnValue({ + data: { data: { data: null } }, + error: null, + isLoading: false, + }); + const { container } = render(); + expect(container.firstChild).toBeNull(); + }); + + it('shows a "From provider catalog" badge for GCP/Azure-sourced descriptions', () => { + mockUseSWR.mockReturnValue({ + data: { + data: { + data: { + title: 'Cloud Storage bucket is publicly accessible', + description: 'Surfaced from Google Security Command Center.', + passCriteria: 'No public ACLs detected.', + failCriteria: 'Bucket has public ACL grants.', + whyItMatters: 'Public buckets risk data exposure.', + source: 'provider', + }, + }, + }, + error: null, + isLoading: false, + }); + render(); + expect(screen.getByText('About this check')).toBeInTheDocument(); + expect(screen.getByText(/From provider catalog/i)).toBeInTheDocument(); + }); +}); diff --git a/apps/app/src/app/(app)/[orgId]/cloud-tests/components/CheckDefinitionPanel.tsx b/apps/app/src/app/(app)/[orgId]/cloud-tests/components/CheckDefinitionPanel.tsx new file mode 100644 index 0000000000..11179f5358 --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/cloud-tests/components/CheckDefinitionPanel.tsx @@ -0,0 +1,84 @@ +'use client'; + +import { useApi } from '@/hooks/use-api'; +import { Loader2 } from 'lucide-react'; + +interface CheckDefinitionPanelProps { + findingId: string; +} + +interface CheckDefinition { + title: string; + description: string; + passCriteria: string; + failCriteria: string; + whyItMatters: string; + source: 'ai' | 'provider'; +} + +/** + * Renders the "About this check" panel inside an expanded finding row. + * + * Fetches a Tier 3 description lazily from + * /v1/cloud-security/findings/:id/check-definition. The first request + * for an AWS check triggers a Haiku call server-side (~1-2s). Subsequent + * requests for the same check in the same org hit the DB cache (~50ms). + * GCP/Azure findings resolve synchronously from provider evidence. + * + * Renders null on error or empty response so the existing finding + * description / remediation always remain visible — auditor trust takes + * precedence over completeness. + */ +export function CheckDefinitionPanel({ findingId }: CheckDefinitionPanelProps) { + const api = useApi(); + const { data, error, isLoading } = api.useSWR<{ + data: CheckDefinition | null; + }>(`/v1/cloud-security/findings/${findingId}/check-definition`, { + revalidateOnFocus: false, + // Browser-side dedupe — same finding clicked twice in a session = one fetch. + dedupingInterval: 5 * 60 * 1000, + }); + + if (isLoading) return ; + const definition = data?.data?.data ?? null; + if (error || !definition) return null; + + return ( +
+
+

About this check

+ {definition.source === 'provider' && ( + + From provider catalog + + )} +
+
+ + + + +
+
+ ); +} + +function CheckDefinitionSkeleton() { + return ( +
+
+ + Generating check description... +
+
+ ); +} + +function DefinitionField({ label, value }: { label: string; value: string }) { + return ( +
+
{label}
+
{value}
+
+ ); +} diff --git a/apps/app/src/app/(app)/[orgId]/cloud-tests/components/CheckGroupBlock.test.tsx b/apps/app/src/app/(app)/[orgId]/cloud-tests/components/CheckGroupBlock.test.tsx new file mode 100644 index 0000000000..b21e2e1072 --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/cloud-tests/components/CheckGroupBlock.test.tsx @@ -0,0 +1,190 @@ +import { render, screen, fireEvent } from '@testing-library/react'; +import { describe, expect, it } from 'vitest'; +import type { Finding } from '../types'; +import type { CheckGroup } from './check-groups'; +import { CheckGroupBlock } from './CheckGroupBlock'; + +function makeFinding(overrides: Partial): Finding { + return { + id: `icx_${Math.random().toString(36).slice(2, 8)}`, + title: 'A finding', + description: null, + remediation: null, + status: 'failed', + severity: 'medium', + serviceId: 'iam', + findingKey: null, + resourceId: null, + resourceType: null, + checkId: null, + checkKey: 'iam-test', + evidence: null, + projectDisplayName: null, + completedAt: null, + connectionId: 'icn_test', + providerSlug: 'aws', + integration: { integrationId: 'aws' }, + ...overrides, + }; +} + +function makeGroup(overrides: Partial = {}): CheckGroup { + const failed = overrides.failed ?? [ + makeFinding({ id: 'f1', status: 'failed', resourceId: 'r1' }), + ]; + const passed = overrides.passed ?? []; + return { + checkKey: 'iam-test', + checkTitle: 'IAM users have MFA enabled', + failed, + passed, + all: [...failed, ...passed], + severity: 'high', + ...overrides, + }; +} + +describe('CheckGroupBlock', () => { + it('renders the check title and failing/total count', () => { + const group = makeGroup({ + failed: [ + makeFinding({ id: 'f1', status: 'failed' }), + makeFinding({ id: 'f2', status: 'failed' }), + ], + passed: [ + makeFinding({ id: 'p1', status: 'passed' }), + makeFinding({ id: 'p2', status: 'passed' }), + makeFinding({ id: 'p3', status: 'passed' }), + ], + }); + render( +
{f.id}
} + />, + ); + expect(screen.getByText('IAM users have MFA enabled')).toBeInTheDocument(); + expect(screen.getByText(/2 of 5 failing/i)).toBeInTheDocument(); + }); + + it('renders only failing rows by default', () => { + const group = makeGroup({ + failed: [makeFinding({ id: 'f1', status: 'failed' })], + passed: [ + makeFinding({ id: 'p1', status: 'passed' }), + makeFinding({ id: 'p2', status: 'passed' }), + ], + }); + render( +
{f.id}
} + />, + ); + expect(screen.getByTestId('row-f1')).toBeInTheDocument(); + expect(screen.queryByTestId('row-p1')).not.toBeInTheDocument(); + expect(screen.queryByTestId('row-p2')).not.toBeInTheDocument(); + }); + + it('reveals passing rows when "Show all" is clicked', () => { + const group = makeGroup({ + failed: [makeFinding({ id: 'f1', status: 'failed' })], + passed: [ + makeFinding({ id: 'p1', status: 'passed' }), + makeFinding({ id: 'p2', status: 'passed' }), + ], + }); + render( +
{f.id}
} + />, + ); + fireEvent.click(screen.getByRole('button', { name: /show all 3 results/i })); + expect(screen.getByTestId('row-f1')).toBeInTheDocument(); + expect(screen.getByTestId('row-p1')).toBeInTheDocument(); + expect(screen.getByTestId('row-p2')).toBeInTheDocument(); + }); + + it('renders compact all-passing line when no failures and no severity filter', () => { + const group = makeGroup({ + failed: [], + passed: [ + makeFinding({ id: 'p1', status: 'passed' }), + makeFinding({ id: 'p2', status: 'passed' }), + ], + }); + render( +
{f.id}
} + />, + ); + expect(screen.getByText(/all 2 passing/i)).toBeInTheDocument(); + expect(screen.queryByTestId('row-p1')).not.toBeInTheDocument(); + }); + + it('hides the check entirely when severity filter is active and there is no failure', () => { + const group = makeGroup({ + failed: [], + passed: [makeFinding({ id: 'p1', status: 'passed' })], + }); + const { container } = render( +
{f.id}
} + />, + ); + expect(container.firstChild).toBeNull(); + }); + + it('filters failing rows by active severity filter', () => { + const group = makeGroup({ + failed: [ + makeFinding({ id: 'f1', status: 'failed', severity: 'critical' }), + makeFinding({ id: 'f2', status: 'failed', severity: 'medium' }), + ], + passed: [], + }); + render( +
{f.id}
} + />, + ); + expect(screen.getByTestId('row-f1')).toBeInTheDocument(); + expect(screen.queryByTestId('row-f2')).not.toBeInTheDocument(); + }); + + it('caps rendered rows at 100 and shows a Show-more affordance', () => { + // 150 failing rows — only 100 should render initially. + const failed = Array.from({ length: 150 }, (_, i) => + makeFinding({ id: `f${i}`, status: 'failed', resourceId: `r${i}` }), + ); + const group = makeGroup({ failed, passed: [] }); + render( +
{f.id}
} + />, + ); + // 100 rows rendered, 50 hidden behind a "Show more" button. + expect(screen.getByTestId('row-f0')).toBeInTheDocument(); + expect(screen.getByTestId('row-f99')).toBeInTheDocument(); + expect(screen.queryByTestId('row-f100')).not.toBeInTheDocument(); + const showMore = screen.getByRole('button', { + name: /show 50 more results/i, + }); + expect(showMore).toBeInTheDocument(); + fireEvent.click(showMore); + expect(screen.getByTestId('row-f100')).toBeInTheDocument(); + expect(screen.getByTestId('row-f149')).toBeInTheDocument(); + }); +}); diff --git a/apps/app/src/app/(app)/[orgId]/cloud-tests/components/CheckGroupBlock.tsx b/apps/app/src/app/(app)/[orgId]/cloud-tests/components/CheckGroupBlock.tsx new file mode 100644 index 0000000000..ecdebad4a3 --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/cloud-tests/components/CheckGroupBlock.tsx @@ -0,0 +1,138 @@ +'use client'; + +import { Badge } from '@trycompai/ui/badge'; +import { ChevronDown, ChevronRight, ShieldCheck } from 'lucide-react'; +import { useMemo, useState } from 'react'; +import type { Finding } from '../types'; +import type { CheckGroup } from './check-groups'; + +/** + * Per-check display cap. If a single check produces more failing rows + * than this we render the first N and surface a "Show all N" affordance + * — keeps the browser responsive on the rare account with hundreds of + * resources of a single kind. + */ +const PER_CHECK_DISPLAY_LIMIT = 100; + +const SEVERITY_DOTS: Record = { + critical: 'bg-red-500', + high: 'bg-orange-500', + medium: 'bg-yellow-500', + low: 'bg-blue-500', + info: 'bg-gray-400', +}; + +export interface CheckGroupBlockProps { + group: CheckGroup; + /** Active severity filter from the parent — used to filter failing rows. */ + severityFilter: string | null; + /** Renders the per-finding row (FindingRow lives in the parent file). */ + renderRow: (finding: Finding) => React.ReactNode; +} + +export function CheckGroupBlock({ + group, + severityFilter, + renderRow, +}: CheckGroupBlockProps) { + const [showAll, setShowAll] = useState(false); + const [revealedMore, setRevealedMore] = useState(false); + + // Apply severity filter to the failing rows shown by default. + const visibleFailing = useMemo(() => { + if (!severityFilter) return group.failed; + return group.failed.filter( + (f) => f.severity?.toLowerCase() === severityFilter, + ); + }, [group.failed, severityFilter]); + + // When user toggles "Show all results" we render every instance (passed + + // failed). Severity filter is intentionally ignored in this mode because + // the user is explicitly asking for a full audit list. + const fullList = group.all; + const rendered = showAll ? fullList : visibleFailing; + const displayCap = revealedMore ? Infinity : PER_CHECK_DISPLAY_LIMIT; + const visibleRows = rendered.slice(0, displayCap); + const hiddenRowsCount = Math.max(0, rendered.length - visibleRows.length); + + // Compact line for checks with no failures (and no severity filter). + if (group.failed.length === 0 && !showAll) { + if (severityFilter) return null; + return ( +
+ + {group.checkTitle} + + — all {group.all.length} passing + + {group.all.length > 0 && ( + + )} +
+ ); + } + + return ( +
+
+ + + {group.checkTitle} + + + {group.failed.length} of {group.all.length} failing + + {group.all.length > group.failed.length && ( + + )} +
+ {visibleRows.length === 0 && severityFilter && ( +
+ No {severityFilter}-severity failures for this check. +
+ )} + {visibleRows.map((finding) => ( +
{renderRow(finding)}
+ ))} + {hiddenRowsCount > 0 && !revealedMore && ( + + )} +
+ ); +} + +export { PER_CHECK_DISPLAY_LIMIT }; 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 414469f937..829521d988 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 @@ -1,6 +1,7 @@ 'use client'; import { useApi } from '@/hooks/use-api'; +import { usePermissions } from '@/hooks/use-permissions'; import { getAwsCloudShellUrl, getAwsRemediationScript, @@ -41,7 +42,12 @@ import { GcpSetupGuide } from './GcpSetupGuide'; import { RemediationDialog } from './RemediationDialog'; import { ScheduledScanPopover } from './ScheduledScanPopover'; -import type { Finding } from '../types'; +import type { Finding, ProviderLatestRun } from '../types'; +import { CheckDefinitionPanel } from './CheckDefinitionPanel'; +import { CheckGroupBlock } from './CheckGroupBlock'; +import { buildCheckGroups } from './check-groups'; +import { EvidenceJsonViewer } from './EvidenceJsonViewer'; +import { MarkExceptionModal } from './MarkExceptionModal'; interface RemediationCapabilities { enabled: boolean; @@ -63,6 +69,8 @@ interface CloudTestsSectionProps { orgId: string; /** When the last scan completed — null means never scanned */ lastRunAt?: Date | null; + /** Latest scan summary (counts, duration). null for legacy or never-scanned. */ + latestRun?: ProviderLatestRun | null; /** Connection variables (e.g., GCP org ID) */ variables?: Record; awsType?: string; @@ -155,6 +163,7 @@ const SERVICE_NAMES: Record = { interface ServiceGroup { serviceId: string; name: string; + /** ALL findings for this service (passed + failed), already filtered by search query. */ findings: Finding[]; passed: number; failed: number; @@ -166,6 +175,7 @@ export function CloudTestsSection({ onScanComplete, orgId, lastRunAt, + latestRun, variables, awsType, }: CloudTestsSectionProps) { @@ -198,6 +208,13 @@ export function CloudTestsSection({ description?: string; } | null>(null); const [showSetupDialog, setShowSetupDialog] = useState(false); + const { hasPermission } = usePermissions(); + const canMarkException = hasPermission('integration', 'update'); + const [exceptionTarget, setExceptionTarget] = useState<{ + findingId: string; + findingTitle: string; + resourceLabel: string | null; + } | null>(null); const findingsResponse = api.useSWR<{ data: Finding[]; count: number }>( '/v1/cloud-security/findings', @@ -290,7 +307,9 @@ export function CloudTestsSection({ const failedFindings = findings.filter((f) => f.status === 'failed' || f.status === 'FAILED'); const passedFindings = findings.filter((f) => f.status === 'passed' || f.status === 'success'); - // Group findings by serviceId + // Group findings by serviceId. `findings` on the resulting group holds the + // full set (passed + failed) matching the search query — per-check sub- + // grouping and the severity filter are applied at render time. const serviceGroups = useMemo(() => { const q = searchQuery.toLowerCase().trim(); const groupMap = new Map(); @@ -306,35 +325,48 @@ export function CloudTestsSection({ const serviceName = SERVICE_NAMES[serviceId] ?? serviceId; const serviceMatches = q ? serviceName.toLowerCase().includes(q) : true; - const failed = groupFindings.filter((f) => f.status === 'failed' || f.status === 'FAILED'); - const passed = groupFindings.filter((f) => f.status === 'passed' || f.status === 'success'); - - let filteredFailed = severityFilter - ? failed.filter((f) => f.severity?.toLowerCase() === severityFilter) - : failed; + // If search query exists and service name doesn't match, filter findings + // by title/description/findingKey. Otherwise include all findings. + const matching = + q && !serviceMatches + ? groupFindings.filter( + (f) => + f.title?.toLowerCase().includes(q) || + f.description?.toLowerCase().includes(q) || + f.findingKey?.toLowerCase().includes(q), + ) + : groupFindings; + + const failed = matching.filter( + (f) => f.status === 'failed' || f.status === 'FAILED', + ); + const passed = matching.filter( + (f) => f.status === 'passed' || f.status === 'success', + ); - // If search query exists and service name doesn't match, filter findings by title - if (q && !serviceMatches) { - filteredFailed = filteredFailed.filter( - (f) => - f.title?.toLowerCase().includes(q) || - f.description?.toLowerCase().includes(q) || - f.findingKey?.toLowerCase().includes(q), + // With severity filter active, hide services that have no matching + // failures. Without filters, keep services that have any findings. + if (severityFilter) { + const hasMatching = failed.some( + (f) => f.severity?.toLowerCase() === severityFilter, ); + if (!hasMatching) continue; + } else if (q && failed.length === 0 && passed.length === 0) { + continue; } groups.push({ serviceId, name: serviceName, - findings: filteredFailed, + findings: matching, passed: passed.length, failed: failed.length, }); } - return groups - .filter((g) => g.findings.length > 0 || (!severityFilter && !q && g.passed > 0)) - .sort((a, b) => b.failed - a.failed || a.name.localeCompare(b.name)); + return groups.sort( + (a, b) => b.failed - a.failed || a.name.localeCompare(b.name), + ); }, [findings, severityFilter, searchQuery]); // Split into baseline (security fundamentals) vs service-specific @@ -479,9 +511,9 @@ export function CloudTestsSection({ {/* Header with scan button */}
-

Security Findings

+

Scan Results

- {findings.length} total findings for this account + {findings.length} total results for this account

@@ -497,6 +529,9 @@ export function CloudTestsSection({
+ {/* Last scan metadata strip — surfaces what's already in IntegrationCheckRun */} + + {/* Selected projects indicator (GCP) */} {providerSlug === 'gcp' && (() => { @@ -673,7 +708,7 @@ export function CloudTestsSection({ setSearchQuery(e.target.value)} className="min-w-0 flex-1 bg-transparent text-xs outline-none placeholder:text-muted-foreground/40" @@ -752,36 +787,43 @@ export function CloudTestsSection({ {isGroupExpanded && (
- {group.findings.length > 0 ? ( - group.findings.map((finding) => { - const match = canFixFinding(finding); - return ( - toggleExpanded(finding.id)} - remediationKey={match?.key ?? null} - remediationEnabled={match?.enabled ?? false} - capabilitiesLoaded={capabilitiesLoaded} - onFix={(key) => - setRemediationTarget({ - connectionId: finding.connectionId, - checkResultId: finding.id, - remediationKey: key, - findingTitle: finding.title ?? 'Finding', - }) - } - onSetup={() => setShowSetupDialog(true)} - /> - ); - }) - ) : ( -
- - All {group.passed} checks passed -
- )} + {buildCheckGroups(group.findings).map((checkGroup) => ( + { + const match = canFixFinding(finding); + return ( + toggleExpanded(finding.id)} + remediationKey={match?.key ?? null} + remediationEnabled={match?.enabled ?? false} + capabilitiesLoaded={capabilitiesLoaded} + onFix={(key) => + setRemediationTarget({ + connectionId: finding.connectionId, + checkResultId: finding.id, + remediationKey: key, + findingTitle: finding.title ?? 'Finding', + }) + } + onSetup={() => setShowSetupDialog(true)} + canMarkException={canMarkException} + onMarkException={() => + setExceptionTarget({ + findingId: finding.id, + findingTitle: finding.title ?? 'Finding', + resourceLabel: formatResourceLabel(finding), + }) + } + /> + ); + }} + /> + ))}
)} @@ -847,36 +889,43 @@ export function CloudTestsSection({ {isGroupExpanded && (
- {group.findings.length > 0 ? ( - group.findings.map((finding) => { - const match = canFixFinding(finding); - return ( - toggleExpanded(finding.id)} - remediationKey={match?.key ?? null} - remediationEnabled={match?.enabled ?? false} - capabilitiesLoaded={capabilitiesLoaded} - onFix={(key) => - setRemediationTarget({ - connectionId: finding.connectionId, - checkResultId: finding.id, - remediationKey: key, - findingTitle: finding.title ?? 'Finding', - }) - } - onSetup={() => setShowSetupDialog(true)} - /> - ); - }) - ) : ( -
- - All {group.passed} checks passed -
- )} + {buildCheckGroups(group.findings).map((checkGroup) => ( + { + const match = canFixFinding(finding); + return ( + toggleExpanded(finding.id)} + remediationKey={match?.key ?? null} + remediationEnabled={match?.enabled ?? false} + capabilitiesLoaded={capabilitiesLoaded} + onFix={(key) => + setRemediationTarget({ + connectionId: finding.connectionId, + checkResultId: finding.id, + remediationKey: key, + findingTitle: finding.title ?? 'Finding', + }) + } + onSetup={() => setShowSetupDialog(true)} + canMarkException={canMarkException} + onMarkException={() => + setExceptionTarget({ + findingId: finding.id, + findingTitle: finding.title ?? 'Finding', + resourceLabel: formatResourceLabel(finding), + }) + } + /> + ); + }} + /> + ))}
)} @@ -890,7 +939,7 @@ export function CloudTestsSection({

- No findings matching "{searchQuery}" + No results matching "{searchQuery}"

); } @@ -1090,6 +1153,70 @@ function StatCard({ ); } +function formatRelativeTime(date: Date): string { + const diff = Date.now() - date.getTime(); + const seconds = Math.floor(diff / 1000); + if (seconds < 5) return 'just now'; + if (seconds < 60) return `${seconds}s ago`; + const minutes = Math.floor(seconds / 60); + if (minutes < 60) return `${minutes}m ago`; + const hours = Math.floor(minutes / 60); + if (hours < 24) return `${hours}h ago`; + const days = Math.floor(hours / 24); + if (days === 1) return 'yesterday'; + if (days < 7) return `${days}d ago`; + return date.toLocaleDateString(); +} + +function formatDuration(ms: number): string { + if (ms < 1000) return `${ms}ms`; + const seconds = ms / 1000; + if (seconds < 60) return `${seconds.toFixed(1)}s`; + const totalSeconds = Math.floor(seconds); + const m = Math.floor(totalSeconds / 60); + const s = totalSeconds % 60; + return s === 0 ? `${m}m` : `${m}m ${s}s`; +} + +function LastScanStrip({ + latestRun, + fallbackLastRunAt, +}: { + latestRun: ProviderLatestRun | null; + fallbackLastRunAt: Date | null; +}) { + const completedRaw = latestRun?.completedAt ?? fallbackLastRunAt; + if (!completedRaw) return null; + + const completedAt = + completedRaw instanceof Date ? completedRaw : new Date(completedRaw); + if (Number.isNaN(completedAt.getTime())) return null; + + const parts: string[] = [`Ran ${formatRelativeTime(completedAt)}`]; + + if (latestRun) { + if (latestRun.totalChecked !== null) { + const noun = latestRun.totalChecked === 1 ? 'check' : 'checks'; + parts.push(`${latestRun.totalChecked} ${noun} evaluated`); + } + if (latestRun.passedCount !== null) { + parts.push(`${latestRun.passedCount} passed`); + } + if (latestRun.failedCount !== null) { + parts.push(`${latestRun.failedCount} failed`); + } + if (latestRun.durationMs !== null && latestRun.durationMs > 0) { + parts.push(`Duration ${formatDuration(latestRun.durationMs)}`); + } + } + + return ( +
+ {parts.join(' • ')} +
+ ); +} + function RemediationSetupDialog({ open, onOpenChange, @@ -1257,6 +1384,59 @@ function RemediationSetupDialog({ ); } +/** + * Format a resource label for a finding row, e.g. "IAM User: john" or + * just "sg-abc123" when type is unknown. Returns null when there's + * nothing meaningful to show. + */ +function formatResourceLabel(finding: Finding): string | null { + const id = finding.resourceId; + const type = finding.resourceType; + if (!id && !type) return null; + if (id && type) return `${type}: ${id}`; + return id ?? type ?? null; +} + +function EvidenceSection({ evidence }: { evidence: unknown }) { + const [open, setOpen] = useState(false); + + // Don't render the section when evidence is null, undefined, or an empty + // container — keeps the expanded state focused. + if (evidence === null || evidence === undefined) return null; + if (typeof evidence === 'object') { + const isEmpty = Array.isArray(evidence) + ? evidence.length === 0 + : Object.keys(evidence as Record).length === 0; + if (isEmpty) return null; + } + + return ( +
+ + {open && ( +
+ +
+ )} +
+ ); +} + function FindingRow({ finding, expanded, @@ -1266,6 +1446,8 @@ function FindingRow({ capabilitiesLoaded, onFix, onSetup, + onMarkException, + canMarkException, }: { finding: Finding; expanded: boolean; @@ -1275,9 +1457,12 @@ function FindingRow({ capabilitiesLoaded: boolean; onFix: (key: string) => void; onSetup: () => void; + onMarkException?: () => void; + canMarkException: boolean; }) { const severity = finding.severity?.toLowerCase() ?? 'info'; const styles = SEVERITY_STYLES[severity] ?? SEVERITY_STYLES.info; + const resourceLabel = formatResourceLabel(finding); const handleFixClick = (e: React.MouseEvent) => { e.stopPropagation(); @@ -1339,7 +1524,14 @@ function FindingRow({ {expanded ? : } - {finding.title ?? 'Untitled finding'} +
+
{finding.title ?? 'Untitled finding'}
+ {resourceLabel && ( +
+ {resourceLabel} +
+ )} +
{finding.projectDisplayName && ( {finding.projectDisplayName} @@ -1348,6 +1540,21 @@ function FindingRow({ e.stopPropagation()} onKeyDown={(e) => e.stopPropagation()}> {renderFixButton()} + {canMarkException && + onMarkException && + (finding.status === 'failed' || finding.status === 'FAILED') && ( + + )} {expanded && (
+ {finding.description && ( -

{finding.description}

+
+

This account's result

+

+ {finding.description} +

+
)} + {finding.remediation && (

Remediation

diff --git a/apps/app/src/app/(app)/[orgId]/cloud-tests/components/EvidenceJsonViewer.test.tsx b/apps/app/src/app/(app)/[orgId]/cloud-tests/components/EvidenceJsonViewer.test.tsx new file mode 100644 index 0000000000..a074448812 --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/cloud-tests/components/EvidenceJsonViewer.test.tsx @@ -0,0 +1,55 @@ +import { render, screen } from '@testing-library/react'; +import { describe, expect, it, vi } from 'vitest'; + +// Mock @uiw/react-json-view to avoid pulling its bundle into jsdom tests. +// We only need to verify our wrapper renders + the wrapping behavior. +vi.mock('@uiw/react-json-view', () => ({ + default: ({ value }: { value: unknown }) => ( +
{JSON.stringify(value)}
+ ), +})); + +import { EvidenceJsonViewer } from './EvidenceJsonViewer'; + +describe('EvidenceJsonViewer', () => { + it('renders the empty state when evidence is null', () => { + render(); + expect( + screen.getByText(/No evidence collected for this finding\./i), + ).toBeInTheDocument(); + }); + + it('renders the empty state when evidence is an empty object', () => { + render(); + expect( + screen.getByText(/No evidence collected for this finding\./i), + ).toBeInTheDocument(); + }); + + it('renders the JSON view when evidence has content', () => { + render( + , + ); + const view = screen.getByTestId('json-view'); + expect(view.textContent).toContain('logs-archive'); + expect(view.textContent).toContain('true'); + }); + + it('wraps primitive evidence values under a "value" key', () => { + render(); + const view = screen.getByTestId('json-view'); + expect(view.textContent).toContain('just a string'); + expect(view.textContent).toContain('value'); + }); + + it('renders a Copy button', () => { + render(); + expect( + screen.getByRole('button', { + name: /Copy evidence JSON to clipboard/i, + }), + ).toBeInTheDocument(); + }); +}); diff --git a/apps/app/src/app/(app)/[orgId]/cloud-tests/components/EvidenceJsonViewer.tsx b/apps/app/src/app/(app)/[orgId]/cloud-tests/components/EvidenceJsonViewer.tsx new file mode 100644 index 0000000000..29bc9439c9 --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/cloud-tests/components/EvidenceJsonViewer.tsx @@ -0,0 +1,98 @@ +'use client'; + +import JsonView from '@uiw/react-json-view'; +import { Check, Copy } from 'lucide-react'; +import { useCallback, useMemo, useState } from 'react'; + +interface EvidenceJsonViewerProps { + /** Sanitized evidence payload (server-side sanitizer redacts sensitive keys). */ + evidence: unknown; +} + +const isEmpty = (value: unknown): boolean => { + if (value === null || value === undefined) return true; + if (typeof value !== 'object') return false; + if (Array.isArray(value)) return value.length === 0; + return Object.keys(value as Record).length === 0; +}; + +/** + * Read-only JSON viewer for cloud-test evidence. Sensitive keys are already + * redacted server-side by evidence-sanitizer.ts before they reach this + * component — this is render only. + */ +export function EvidenceJsonViewer({ evidence }: EvidenceJsonViewerProps) { + const [copied, setCopied] = useState(false); + + const display = useMemo(() => { + // `@uiw/react-json-view` expects an object/array at the root. Wrap primitives + // so we never pass a plain string/number/null into the tree view. + if (evidence === null || evidence === undefined) return null; + if (typeof evidence === 'object') return evidence; + return { value: evidence }; + }, [evidence]); + + const jsonString = useMemo(() => { + if (display === null) return ''; + try { + return JSON.stringify(display, null, 2); + } catch { + return ''; + } + }, [display]); + + const handleCopy = useCallback(async () => { + if (!jsonString) return; + try { + await navigator.clipboard.writeText(jsonString); + setCopied(true); + setTimeout(() => setCopied(false), 1500); + } catch { + // Clipboard unavailable — silently no-op rather than throw. + } + }, [jsonString]); + + if (isEmpty(evidence)) { + return ( +
+ No evidence collected for this finding. +
+ ); + } + + return ( +
+ +
+ +
+
+ ); +} diff --git a/apps/app/src/app/(app)/[orgId]/cloud-tests/components/FindingsTable.tsx b/apps/app/src/app/(app)/[orgId]/cloud-tests/components/FindingsTable.tsx index 543e42c3ee..85fae82e3b 100644 --- a/apps/app/src/app/(app)/[orgId]/cloud-tests/components/FindingsTable.tsx +++ b/apps/app/src/app/(app)/[orgId]/cloud-tests/components/FindingsTable.tsx @@ -63,7 +63,7 @@ export function FindingsTable({ findings }: FindingsTableProps) { if (findings.length === 0) { return (
-

No findings available

+

No results available

); } diff --git a/apps/app/src/app/(app)/[orgId]/cloud-tests/components/HistoryTab.test.tsx b/apps/app/src/app/(app)/[orgId]/cloud-tests/components/HistoryTab.test.tsx new file mode 100644 index 0000000000..61bee6932e --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/cloud-tests/components/HistoryTab.test.tsx @@ -0,0 +1,214 @@ +import { render, screen, fireEvent } from '@testing-library/react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +type SwrShape = { + data: { data: unknown } | undefined; + error: unknown; + isLoading: boolean; + mutate: () => Promise | unknown; +}; + +const swrMock = vi.fn<() => SwrShape>(); +const deleteMock = vi.fn(); +const hasPermissionMock = vi.fn().mockReturnValue(true); + +vi.mock('@/hooks/use-api', () => ({ + useApi: () => ({ + useSWR: (_url: string) => swrMock(), + delete: (url: string) => deleteMock(url), + }), +})); + +vi.mock('@/hooks/use-permissions', () => ({ + usePermissions: () => ({ + permissions: {}, + hasPermission: hasPermissionMock, + }), +})); + +vi.mock('sonner', () => ({ + toast: { success: vi.fn(), error: vi.fn() }, +})); + +vi.mock('@trycompai/ui/button', () => ({ + Button: ({ + children, + onClick, + }: { + children: React.ReactNode; + onClick?: () => void; + }) => ( + + ), +})); + +import { HistoryTab } from './HistoryTab'; + +const baselinePayload = { + summary: { + resolutions: 3, + platformFixes: 1, + externalFixes: 1, + resourceDeleted: 1, + exceptionMarked: 0, + activeExceptions: 1, + regressions: 1, + }, + resolutions: [ + { + id: 'fres_1', + checkId: 'iam-no-mfa', + resourceId: 'john', + resourceType: 'AwsIamUser', + resolvedAt: '2026-05-12T10:00:00Z', + resolutionMethod: 'platform_fix', + daysOpen: 4, + }, + { + id: 'fres_2', + checkId: 's3-public', + resourceId: 'old-backups', + resourceType: 'S3Bucket', + resolvedAt: '2026-05-10T10:00:00Z', + resolutionMethod: 'external_fix', + daysOpen: 23, + }, + { + id: 'fres_3', + checkId: 'ec2-open-ssh', + resourceId: 'sg-abc', + resourceType: 'SecurityGroup', + resolvedAt: '2026-05-08T10:00:00Z', + resolutionMethod: 'resource_deleted', + daysOpen: 5, + }, + ], + exceptions: [ + { + id: 'fex_1', + checkId: 's3-public', + resourceId: 'marketing-assets', + reason: 'Public marketing bucket — intentional.', + reviewedBy: 'CISO 2026-Q1', + expiresAt: '2026-08-13T00:00:00Z', + markedAt: '2026-05-13T10:00:00Z', + }, + ], + regressions: [ + { + id: 'freg_1', + checkId: 'rds-not-encrypted', + resourceId: 'prod-db-2', + previouslyResolvedAt: '2026-03-10T10:00:00Z', + regressedAt: '2026-04-15T10:00:00Z', + daysClean: 36, + }, + ], +}; + +describe('HistoryTab', () => { + beforeEach(() => { + swrMock.mockReset(); + deleteMock.mockReset(); + hasPermissionMock.mockReturnValue(true); + }); + + it('renders a loading state while the request is in flight', () => { + swrMock.mockReturnValue({ + data: undefined, + error: null, + isLoading: true, + mutate: () => undefined, + }); + render(); + expect(screen.getByText(/Loading history/i)).toBeInTheDocument(); + }); + + it('renders an empty state when there are no resolutions, exceptions, or regressions', () => { + swrMock.mockReturnValue({ + data: { + data: { + data: { + summary: { + resolutions: 0, + platformFixes: 0, + externalFixes: 0, + resourceDeleted: 0, + exceptionMarked: 0, + activeExceptions: 0, + regressions: 0, + }, + resolutions: [], + exceptions: [], + regressions: [], + }, + }, + }, + error: null, + isLoading: false, + mutate: () => undefined, + }); + render(); + expect(screen.getByText(/No audit history yet/i)).toBeInTheDocument(); + }); + + it('renders all three sections with sample data', () => { + swrMock.mockReturnValue({ + data: { data: { data: baselinePayload } }, + error: null, + isLoading: false, + mutate: () => undefined, + }); + render(); + // "Resolutions" and "Active exceptions" appear in both summary + section header — + // getAllByText proves both are rendered. + expect(screen.getAllByText(/Resolutions/).length).toBeGreaterThan(0); + expect(screen.getAllByText(/Active exceptions/).length).toBeGreaterThan(0); + expect(screen.getAllByText(/Regressions/).length).toBeGreaterThan(0); + expect(screen.getByText('Fixed via platform')).toBeInTheDocument(); + expect(screen.getByText('Fixed externally')).toBeInTheDocument(); + expect(screen.getByText('Resource deleted')).toBeInTheDocument(); + expect( + screen.getByText(/Public marketing bucket/i), + ).toBeInTheDocument(); + expect(screen.getByText(/CISO 2026-Q1/i)).toBeInTheDocument(); + }); + + it('calls DELETE /exceptions/:id and refreshes when "Remove exception" is clicked', async () => { + const mutate = vi.fn(); + swrMock.mockReturnValue({ + data: { data: { data: baselinePayload } }, + error: null, + isLoading: false, + mutate, + }); + deleteMock.mockResolvedValueOnce({ error: null }); + + render(); + fireEvent.click( + screen.getByRole('button', { name: /Remove exception/i }), + ); + await Promise.resolve(); + await Promise.resolve(); + expect(deleteMock).toHaveBeenCalledWith( + '/v1/cloud-security/exceptions/fex_1', + ); + expect(mutate).toHaveBeenCalled(); + }); + + it('hides the "Remove exception" button for users without integration:update', () => { + hasPermissionMock.mockReturnValue(false); + swrMock.mockReturnValue({ + data: { data: { data: baselinePayload } }, + error: null, + isLoading: false, + mutate: () => undefined, + }); + render(); + expect( + screen.queryByRole('button', { name: /Remove exception/i }), + ).not.toBeInTheDocument(); + }); +}); diff --git a/apps/app/src/app/(app)/[orgId]/cloud-tests/components/HistoryTab.tsx b/apps/app/src/app/(app)/[orgId]/cloud-tests/components/HistoryTab.tsx new file mode 100644 index 0000000000..2cc2846d22 --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/cloud-tests/components/HistoryTab.tsx @@ -0,0 +1,308 @@ +'use client'; + +import { useApi } from '@/hooks/use-api'; +import { usePermissions } from '@/hooks/use-permissions'; +import { Button } from '@trycompai/ui/button'; +import { Loader2, ShieldCheck, RotateCcw, X } from 'lucide-react'; +import { toast } from 'sonner'; + +interface ResolutionRow { + id: string; + checkId: string; + resourceId: string; + resourceType: string | null; + resolvedAt: string; + resolutionMethod: + | 'platform_fix' + | 'external_fix' + | 'resource_deleted' + | 'exception_marked'; + daysOpen: number | null; +} + +interface ExceptionRow { + id: string; + checkId: string; + resourceId: string; + reason: string; + reviewedBy: string | null; + expiresAt: string | null; + markedAt: string; +} + +interface RegressionRow { + id: string; + checkId: string; + resourceId: string; + previouslyResolvedAt: string; + regressedAt: string; + daysClean: number | null; +} + +interface HistoryPayload { + summary: { + resolutions: number; + platformFixes: number; + externalFixes: number; + resourceDeleted: number; + exceptionMarked: number; + activeExceptions: number; + regressions: number; + }; + resolutions: ResolutionRow[]; + exceptions: ExceptionRow[]; + regressions: RegressionRow[]; +} + +const RESOLUTION_METHOD_LABEL: Record< + ResolutionRow['resolutionMethod'], + string +> = { + platform_fix: 'Fixed via platform', + external_fix: 'Fixed externally', + resource_deleted: 'Resource deleted', + exception_marked: 'Marked as exception', +}; + +export interface HistoryTabProps { + connectionId: string; +} + +/** + * Renders the audit trail for a connection — resolutions, active + * exceptions, and regressions. Pulls from + * GET /v1/cloud-security/history?connectionId=... + */ +export function HistoryTab({ connectionId }: HistoryTabProps) { + const api = useApi(); + const { hasPermission } = usePermissions(); + const canRevoke = hasPermission('integration', 'update'); + + const { data, error, isLoading, mutate } = api.useSWR<{ + data: HistoryPayload; + }>(`/v1/cloud-security/history?connectionId=${connectionId}`, { + revalidateOnFocus: false, + }); + + const handleRevoke = async (exceptionId: string) => { + const response = await api.delete( + `/v1/cloud-security/exceptions/${exceptionId}`, + ); + if (response.error) { + toast.error( + typeof response.error === 'string' + ? response.error + : 'Could not revoke exception', + ); + return; + } + toast.success('Exception revoked'); + mutate(); + }; + + if (isLoading) { + return ( +
+ + Loading history… +
+ ); + } + if (error || !data?.data?.data) { + return ( +
+ Could not load history for this connection. +
+ ); + } + + const payload = data.data.data; + const isEmpty = + payload.resolutions.length === 0 && + payload.exceptions.length === 0 && + payload.regressions.length === 0; + + if (isEmpty) { + return ( +
+ +

No audit history yet

+

+ Resolutions, exceptions, and regressions are recorded automatically + on each scan. +

+
+ ); + } + + return ( +
+ + {payload.resolutions.length > 0 && ( +
+ {payload.resolutions.map((row) => ( + + ))} +
+ )} + {payload.exceptions.length > 0 && ( +
+ {payload.exceptions.map((row) => ( + handleRevoke(row.id)} + /> + ))} +
+ )} + {payload.regressions.length > 0 && ( +
+ {payload.regressions.map((row) => ( + + ))} +
+ )} +
+ ); +} + +function SummaryCard({ summary }: { summary: HistoryPayload['summary'] }) { + return ( +
+
+

Resolved

+

+ {summary.resolutions} +

+
+
+

Active exceptions

+

+ {summary.activeExceptions} +

+
+
+

Regressions

+

+ {summary.regressions} +

+
+
+ ); +} + +function Section({ + title, + subtitle, + count, + children, +}: { + title: string; + subtitle: string; + count: number; + children: React.ReactNode; +}) { + return ( +
+
+

+ {title} ({count}) +

+

{subtitle}

+
+
{children}
+
+ ); +} + +function ResolutionRowView({ row }: { row: ResolutionRow }) { + return ( +
+
+ + + {row.checkId} · {row.resourceType ? `${row.resourceType}: ` : ''} + {row.resourceId} + + + {RESOLUTION_METHOD_LABEL[row.resolutionMethod]} + +
+

+ Resolved {new Date(row.resolvedAt).toLocaleString()} + {row.daysOpen !== null ? ` — was open ${row.daysOpen}d` : ''} +

+
+ ); +} + +function ExceptionRowView({ + row, + canRevoke, + onRevoke, +}: { + row: ExceptionRow; + canRevoke: boolean; + onRevoke: () => void; +}) { + return ( +
+
+ + {row.checkId} · {row.resourceId} + + {canRevoke && ( + + )} +
+

{row.reason}

+
+ Marked {new Date(row.markedAt).toLocaleDateString()} + {row.reviewedBy && · Reviewed by: {row.reviewedBy}} + {row.expiresAt && ( + · Auto-review: {new Date(row.expiresAt).toLocaleDateString()} + )} +
+
+ ); +} + +function RegressionRowView({ row }: { row: RegressionRow }) { + return ( +
+
+ + + {row.checkId} · {row.resourceId} + +
+

+ Was clean {row.daysClean ?? '?'}d (previously resolved{' '} + {new Date(row.previouslyResolvedAt).toLocaleDateString()}). Failing + again as of {new Date(row.regressedAt).toLocaleString()}. +

+
+ ); +} diff --git a/apps/app/src/app/(app)/[orgId]/cloud-tests/components/MarkExceptionModal.test.tsx b/apps/app/src/app/(app)/[orgId]/cloud-tests/components/MarkExceptionModal.test.tsx new file mode 100644 index 0000000000..7c2419456f --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/cloud-tests/components/MarkExceptionModal.test.tsx @@ -0,0 +1,151 @@ +import { render, screen, fireEvent } from '@testing-library/react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const postMock = vi.fn(); +vi.mock('@/hooks/use-api', () => ({ + useApi: () => ({ post: postMock }), +})); + +vi.mock('sonner', () => ({ + toast: { success: vi.fn(), error: vi.fn() }, +})); + +// Stub the dialog wrapper so we don't pull a real portal in jsdom. Render +// children inline when `open`. +vi.mock('@trycompai/ui/dialog', () => { + const Pass = ({ children }: { children: React.ReactNode }) => <>{children}; + return { + Dialog: ({ + open, + children, + }: { + open: boolean; + children: React.ReactNode; + }) => (open ?
{children}
: null), + DialogContent: Pass, + DialogDescription: Pass, + DialogHeader: Pass, + DialogTitle: Pass, + }; +}); + +vi.mock('@trycompai/ui/button', () => ({ + Button: ({ + children, + disabled, + onClick, + }: { + children: React.ReactNode; + disabled?: boolean; + onClick?: () => void; + }) => ( + + ), +})); + +import { MarkExceptionModal } from './MarkExceptionModal'; + +describe('MarkExceptionModal', () => { + beforeEach(() => { + postMock.mockReset(); + }); + + it('renders the finding title and resource label', () => { + render( + {}} + findingId="icx_1" + findingTitle="IAM password policy < 14 characters" + resourceLabel="IAM Account: 123456789012" + />, + ); + expect( + screen.getByText('IAM password policy < 14 characters'), + ).toBeInTheDocument(); + expect(screen.getByText('IAM Account: 123456789012')).toBeInTheDocument(); + }); + + it('keeps the submit button disabled until reason reaches min length', () => { + render( + {}} + findingId="icx_1" + findingTitle="X" + />, + ); + const submit = screen.getByRole('button', { name: /^Mark as exception$/ }); + expect(submit).toBeDisabled(); + + fireEvent.change(screen.getByLabelText(/Reason for exception/i), { + target: { value: 'too short' }, + }); + expect(submit).toBeDisabled(); + + fireEvent.change(screen.getByLabelText(/Reason for exception/i), { + target: { + value: 'This is a long enough documented reason for the exception.', + }, + }); + expect(submit).not.toBeDisabled(); + }); + + it('submits to the exception endpoint and invokes onMarked on success', async () => { + postMock.mockResolvedValueOnce({ error: null, data: { data: { id: 'fex_1' } } }); + const onMarked = vi.fn(); + render( + {}} + findingId="icx_1" + findingTitle="X" + onMarked={onMarked} + />, + ); + fireEvent.change(screen.getByLabelText(/Reason for exception/i), { + target: { + value: 'This is a long enough documented reason for the exception.', + }, + }); + fireEvent.click( + screen.getByRole('button', { name: /^Mark as exception$/ }), + ); + await Promise.resolve(); + await Promise.resolve(); + expect(postMock).toHaveBeenCalledWith( + '/v1/cloud-security/findings/icx_1/exception', + expect.objectContaining({ + reason: 'This is a long enough documented reason for the exception.', + }), + ); + expect(onMarked).toHaveBeenCalled(); + }); + + it('does not invoke onMarked when the API responds with an error', async () => { + postMock.mockResolvedValueOnce({ error: 'Forbidden' }); + const onMarked = vi.fn(); + render( + {}} + findingId="icx_1" + findingTitle="X" + onMarked={onMarked} + />, + ); + fireEvent.change(screen.getByLabelText(/Reason for exception/i), { + target: { + value: 'This is a long enough documented reason for the exception.', + }, + }); + fireEvent.click( + screen.getByRole('button', { name: /^Mark as exception$/ }), + ); + await Promise.resolve(); + await Promise.resolve(); + expect(onMarked).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/app/src/app/(app)/[orgId]/cloud-tests/components/MarkExceptionModal.tsx b/apps/app/src/app/(app)/[orgId]/cloud-tests/components/MarkExceptionModal.tsx new file mode 100644 index 0000000000..d5896d26c8 --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/cloud-tests/components/MarkExceptionModal.tsx @@ -0,0 +1,198 @@ +'use client'; + +import { + Dialog, + DialogContent, + DialogDescription, + DialogHeader, + DialogTitle, +} from '@trycompai/ui/dialog'; +import { Button } from '@trycompai/ui/button'; +import { useApi } from '@/hooks/use-api'; +import { Loader2 } from 'lucide-react'; +import { useState } from 'react'; +import { toast } from 'sonner'; + +const MIN_REASON_LENGTH = 20; + +export interface MarkExceptionModalProps { + open: boolean; + onOpenChange: (open: boolean) => void; + findingId: string | null; + findingTitle: string; + resourceLabel?: string | null; + onMarked?: () => void; +} + +/** + * Modal that captures the reason + optional reviewer + optional expiration + * date for marking a finding as an exception. Talks to POST + * /v1/cloud-security/findings/:id/exception. Calls onMarked() on success + * so the parent can refresh its findings list. + */ +export function MarkExceptionModal({ + open, + onOpenChange, + findingId, + findingTitle, + resourceLabel, + onMarked, +}: MarkExceptionModalProps) { + const api = useApi(); + const [reason, setReason] = useState(''); + const [reviewedBy, setReviewedBy] = useState(''); + const [expiresAt, setExpiresAt] = useState(''); + const [submitting, setSubmitting] = useState(false); + + const reasonTooShort = reason.trim().length < MIN_REASON_LENGTH; + + const handleSubmit = async () => { + if (!findingId || reasonTooShort) return; + setSubmitting(true); + const response = await api.post( + `/v1/cloud-security/findings/${findingId}/exception`, + { + reason: reason.trim(), + reviewedBy: reviewedBy.trim() || undefined, + expiresAt: expiresAt || undefined, + }, + ); + setSubmitting(false); + + if (response.error) { + const message = + typeof response.error === 'string' + ? response.error + : 'Could not mark exception — please try again.'; + toast.error(message); + return; + } + + toast.success('Marked as exception'); + setReason(''); + setReviewedBy(''); + setExpiresAt(''); + onMarked?.(); + onOpenChange(false); + }; + + const handleClose = (next: boolean) => { + if (!next) { + setReason(''); + setReviewedBy(''); + setExpiresAt(''); + } + onOpenChange(next); + }; + + return ( + + + + Mark this finding as an exception? + + Exceptions are recorded in the audit trail. Auditors will see this + exception and the reason you provide. + + + +
+
+

{findingTitle}

+ {resourceLabel && ( +

{resourceLabel}

+ )} +
+ +
+ +