From 5d7e2a001dc2cdfe00ed68b2fafd3a6bfc3bb83b Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Wed, 10 Jun 2026 16:25:01 -0400 Subject: [PATCH 01/18] fix(integration-platform): surface real read errors in cloudtrail/kms/iam/ec2/rds checks (CS-533) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ports the toReadFailure error-visibility pattern from the merged S3 fix (PR #3075) to the remaining AWS checks, so 'could not verify' findings stop asserting unverified permission claims: - new read-failure.ts module (shared.ts was over the file-size cap): toReadFailure gains a regionDisabled class (OptInRequired/AuthFailure from opted-out regions -> 'remove the region' advice instead of a useless 're-run'), plus combineReadFailures for aggregate findings and remediationForReadFailure as the single source of gated wording - cloudtrail: GetTrailStatus/DescribeTrails failures carry readError in evidence with gated remediation; the factually wrong 'No CloudTrail configured' title (trails homed outside scanned regions are invisible) becomes 'No CloudTrail trail found' with scanned regions in evidence - kms: per-key rotation finding carries readError; aggregate finding lists per-region errors and sample per-key errors - iam: a non-NoSuchEntity password-policy read error no longer aborts the whole check (it was rethrowing, suppressing the independent root-MFA/root-access-key findings) — emits 'Could not verify IAM password policy' and continues - ec2/rds: failedRegions evidence enriched to {region, error} pairs; rds dedupes per region preferring denied failures - maxAttempts: 5 on all clients (parity with the S3 fix) Pass/fail verdicts are unchanged except iam continuing instead of aborting. Evidence is display-only (verified: no programmatic consumers). Co-Authored-By: Claude Fable 5 --- .../aws/checks/__tests__/aws-checks.test.ts | 104 +++++++++++++++++- .../src/manifests/aws/checks/cloudtrail.ts | 86 +++++++++++---- .../src/manifests/aws/checks/ec2.ts | 47 +++++--- .../src/manifests/aws/checks/iam.ts | 52 +++++++-- .../src/manifests/aws/checks/kms.ts | 94 +++++++++++----- .../src/manifests/aws/checks/rds.ts | 64 ++++++++--- .../src/manifests/aws/checks/read-failure.ts | 78 +++++++++++++ .../src/manifests/aws/checks/s3.ts | 20 ++-- .../src/manifests/aws/checks/shared.ts | 39 ++----- 9 files changed, 451 insertions(+), 133 deletions(-) create mode 100644 packages/integration-platform/src/manifests/aws/checks/read-failure.ts diff --git a/packages/integration-platform/src/manifests/aws/checks/__tests__/aws-checks.test.ts b/packages/integration-platform/src/manifests/aws/checks/__tests__/aws-checks.test.ts index 02ad8ef7e8..97bc76ac06 100644 --- a/packages/integration-platform/src/manifests/aws/checks/__tests__/aws-checks.test.ts +++ b/packages/integration-platform/src/manifests/aws/checks/__tests__/aws-checks.test.ts @@ -20,6 +20,8 @@ import { awsAccountIdFromCtx, emitOutcomes, resolveAwsCredentialInputs, + combineReadFailures, + remediationForReadFailure, toReadFailure, type CheckOutcome, } from '../shared'; @@ -454,7 +456,7 @@ describe('toReadFailure — read-error classification', () => { it('classifies AccessDenied by error name', () => { const err = new Error('Access Denied'); err.name = 'AccessDenied'; - expect(toReadFailure(err)).toEqual({ error: 'AccessDenied: Access Denied', denied: true }); + expect(toReadFailure(err)).toEqual({ error: 'AccessDenied: Access Denied', denied: true, regionDisabled: false }); }); it('classifies 403 by http status even with a generic name', () => { @@ -468,11 +470,44 @@ describe('toReadFailure — read-error classification', () => { it('treats network/timeout errors as not denied', () => { const err = new Error('socket hang up'); err.name = 'TimeoutError'; - expect(toReadFailure(err)).toEqual({ error: 'TimeoutError: socket hang up', denied: false }); + expect(toReadFailure(err)).toEqual({ error: 'TimeoutError: socket hang up', denied: false, regionDisabled: false }); }); it('stringifies non-Error throwables', () => { - expect(toReadFailure('boom')).toEqual({ error: 'boom', denied: false }); + expect(toReadFailure('boom')).toEqual({ error: 'boom', denied: false, regionDisabled: false }); + }); + + it('classifies opted-out region errors as regionDisabled, not transient', () => { + const optIn = new Error('region is not opted in'); + optIn.name = 'OptInRequired'; + expect(toReadFailure(optIn)).toMatchObject({ denied: false, regionDisabled: true }); + + const authFailure = new Error('AWS was not able to validate the provided access credentials'); + authFailure.name = 'AuthFailure'; + expect(toReadFailure(authFailure)).toMatchObject({ denied: false, regionDisabled: true }); + }); +}); + +describe('combineReadFailures / remediationForReadFailure', () => { + const denied = { error: 'AccessDenied: nope', denied: true }; + const transient = { error: 'TimeoutError: hang', denied: false }; + const disabled = { error: 'OptInRequired: not opted in', denied: false, regionDisabled: true }; + + it('combine: any denied wins; regionDisabled only when ALL are', () => { + expect(combineReadFailures([])).toBeUndefined(); + expect(combineReadFailures([transient, denied])!.denied).toBe(true); + expect(combineReadFailures([disabled, disabled])).toMatchObject({ regionDisabled: true, denied: false }); + // mixed disabled + transient must NOT advise removing a region + expect(combineReadFailures([disabled, transient])!.regionDisabled).toBe(false); + }); + + it('remediation: grant hint only for denied (or missing detail); region message for disabled; re-run otherwise', () => { + const grant = 'Grant x:Read to the role.'; + expect(remediationForReadFailure(undefined, grant)).toBe(grant); + expect(remediationForReadFailure(denied, grant)).toBe(grant); + expect(remediationForReadFailure(disabled, grant)).toMatch(/disabled or not opted in/i); + expect(remediationForReadFailure(transient, grant)).toMatch(/re-run/i); + expect(remediationForReadFailure(transient, grant)).not.toContain(grant); }); }); @@ -604,6 +639,38 @@ describe('AWS KMS rotation evaluator', () => { }); }); +describe('KMS rotation read-failure gating', () => { + it('transient rotation-status failure surfaces readError and avoids the permission claim', () => { + const out = evaluateKmsRotation([ + { + keyId: 'k1', region: 'us-east-1', rotationEligible: true, rotationStatusKnown: false, rotationEnabled: false, + rotationReadFailure: { error: 'ThrottlingException: Rate exceeded', denied: false }, + }, + ]); + expect(out[0]!.kind).toBe('fail'); + expect(out[0]!.evidence).toMatchObject({ readError: 'ThrottlingException: Rate exceeded' }); + expect(out[0]!.remediation).not.toContain('Grant kms:GetKeyRotationStatus'); + expect(out[0]!.remediation).toMatch(/re-run/i); + }); + + it('denied rotation-status failure keeps the grant remediation', () => { + const out = evaluateKmsRotation([ + { + keyId: 'k1', region: 'us-east-1', rotationEligible: true, rotationStatusKnown: false, rotationEnabled: false, + rotationReadFailure: { error: 'AccessDeniedException: no kms:GetKeyRotationStatus', denied: true }, + }, + ]); + expect(out[0]!.remediation).toContain('Grant kms:GetKeyRotationStatus'); + }); + + it('without failure detail the legacy permission hint is kept', () => { + const out = evaluateKmsRotation([ + { keyId: 'k1', region: 'us-east-1', rotationEligible: true, rotationStatusKnown: false, rotationEnabled: false }, + ]); + expect(out[0]!.remediation).toContain('Grant kms:GetKeyRotationStatus'); + }); +}); + describe('AWS CloudTrail evaluator', () => { it('passes when a multi-region trail with validation is actively logging', () => { const out = evaluateCloudTrail([ @@ -645,6 +712,30 @@ describe('AWS CloudTrail evaluator', () => { expect(out[0]!.kind).toBe('fail'); expect(out[0]!.title).toMatch(/Could not verify/); }); + + it('status-read failure: transient error surfaces readError and does not claim a missing permission', () => { + const out = evaluateCloudTrail([ + { + name: 't1', multiRegion: true, logValidation: true, logging: false, loggingKnown: false, + statusReadFailure: { error: 'TimeoutError: socket hang up', denied: false }, + }, + ]); + expect(out[0]!.evidence).toMatchObject({ readError: 'TimeoutError: socket hang up' }); + expect(out[0]!.description).toContain('TimeoutError: socket hang up'); + expect(out[0]!.remediation).not.toContain('Grant cloudtrail:GetTrailStatus'); + expect(out[0]!.remediation).toMatch(/re-run/i); + }); + + it('status-read failure: AccessDenied keeps the grant-permission remediation', () => { + const out = evaluateCloudTrail([ + { + name: 't1', multiRegion: true, logValidation: true, logging: false, loggingKnown: false, + statusReadFailure: { error: 'AccessDeniedException: nope', denied: true }, + }, + ]); + expect(out[0]!.remediation).toContain('Grant cloudtrail:GetTrailStatus'); + expect(out[0]!.evidence).toMatchObject({ readError: 'AccessDeniedException: nope' }); + }); }); describe('IAM/CloudTrail outcomes carry evidence (so the UI shows "View Evidence")', () => { @@ -682,10 +773,11 @@ describe('IAM/CloudTrail outcomes carry evidence (so the UI shows "View Evidence ).toBe(true); }); - it('the "No CloudTrail configured" outcome has evidence', () => { - const out = evaluateCloudTrail([]); + it('the "No CloudTrail trail found" outcome has evidence incl. scanned regions', () => { + const out = evaluateCloudTrail([], { scannedRegions: ['us-east-1', 'eu-west-1'] }); expect(out).toHaveLength(1); - expect(out[0]!.title).toMatch(/No CloudTrail configured/); + expect(out[0]!.title).toMatch(/No CloudTrail trail found/); + expect(out[0]!.evidence).toMatchObject({ trailsFound: 0, scannedRegions: ['us-east-1', 'eu-west-1'] }); expect(hasEvidence(out[0]!)).toBe(true); }); diff --git a/packages/integration-platform/src/manifests/aws/checks/cloudtrail.ts b/packages/integration-platform/src/manifests/aws/checks/cloudtrail.ts index 436832d583..b3fb2ee424 100644 --- a/packages/integration-platform/src/manifests/aws/checks/cloudtrail.ts +++ b/packages/integration-platform/src/manifests/aws/checks/cloudtrail.ts @@ -6,7 +6,15 @@ import { } from '@aws-sdk/client-cloudtrail'; import { TASK_TEMPLATES } from '../../../task-mappings'; import type { CheckContext, IntegrationCheck } from '../../../types'; -import { resolveAwsSessionOrFail, type CheckOutcome, emitOutcomes } from './shared'; +import { + combineReadFailures, + remediationForReadFailure, + resolveAwsSessionOrFail, + toReadFailure, + type CheckOutcome, + type ReadFailure, + emitOutcomes, +} from './shared'; export interface TrailInfo { name: string; @@ -20,9 +28,19 @@ export interface TrailInfo { * be read, this is set to false so it is NOT misreported as logging=false. */ loggingKnown?: boolean; + /** set when GetTrailStatus failed — the real error, surfaced in evidence */ + statusReadFailure?: ReadFailure; +} + +export interface CloudTrailEvalOptions { + /** regions DescribeTrails ran in — shown when no trail is found */ + scannedRegions?: string[]; } -export function evaluateCloudTrail(trails: TrailInfo[]): CheckOutcome[] { +export function evaluateCloudTrail( + trails: TrailInfo[], + opts?: CloudTrailEvalOptions, +): CheckOutcome[] { const good = trails.find( (t) => t.multiRegion && t.logValidation && t.logging && t.loggingKnown !== false, ); @@ -47,31 +65,44 @@ export function evaluateCloudTrail(trails: TrailInfo[]): CheckOutcome[] { (t) => t.multiRegion && t.logValidation && t.loggingKnown === false, ); if (unverifiableCandidate) { + const failure = unverifiableCandidate.statusReadFailure; return [ { kind: 'fail', title: 'Could not verify CloudTrail logging status', - description: `Trail "${unverifiableCandidate.name}" is multi-region with log file validation, but its logging status (GetTrailStatus) could not be read, so active logging is unverified.`, + description: `Trail "${unverifiableCandidate.name}" is multi-region with log file validation, but its logging status (GetTrailStatus) could not be read${failure ? ` (${failure.error})` : ''}, so active logging is unverified.`, resourceType: 'aws-cloudtrail', resourceId: unverifiableCandidate.name, severity: 'medium', - remediation: + remediation: remediationForReadFailure( + failure, 'Grant cloudtrail:GetTrailStatus to the integration role so logging status can be verified, then re-run the check.', - evidence: { trail: unverifiableCandidate.name }, + ), + evidence: { + trail: unverifiableCandidate.name, + ...(failure ? { readError: failure.error } : {}), + }, }, ]; } if (trails.length === 0) { + const scanned = opts?.scannedRegions ?? []; return [ { kind: 'fail', - title: 'No CloudTrail configured', - description: 'No CloudTrail trail is configured for the account.', + title: 'No CloudTrail trail found', + // A multi-region trail shadows into every region, so it would be + // visible in any scanned region — only single-region trails homed + // outside the scanned regions (non-compliant anyway) are invisible. + description: `No CloudTrail trail was found${scanned.length > 0 ? ` in the scanned regions (${scanned.join(', ')})` : ''}. A compliant multi-region trail would be visible in any scanned region.`, resourceType: 'aws-cloudtrail', resourceId: 'account', severity: 'high', remediation: 'Create a multi-region CloudTrail trail with log file validation enabled.', - evidence: { trailsFound: 0 }, + evidence: { + trailsFound: 0, + ...(scanned.length > 0 ? { scannedRegions: scanned } : {}), + }, }, ]; } @@ -118,12 +149,15 @@ export const cloudTrailEnabledCheck: IntegrationCheck = { // dedupe by TrailARN before evaluating. const seenArns = new Set(); const trails: TrailInfo[] = []; - const failedRegions: string[] = []; + const regionFailures: Array<{ region: string; failure: ReadFailure }> = []; for (const region of session.regions) { const ct = new CloudTrailClient({ region, credentials: session.credentials, + // Reads are idempotent; extra attempts ride out transient network or + // throttling failures during the scheduled-run herd. + maxAttempts: 5, }); let trailList: DescribeTrailsCommandOutput['trailList']; @@ -131,10 +165,9 @@ export const cloudTrailEnabledCheck: IntegrationCheck = { const resp = await ct.send(new DescribeTrailsCommand({})); trailList = resp.trailList; } catch (err) { - failedRegions.push(region); - ctx.log( - `CloudTrail: could not list trails in ${region}: ${err instanceof Error ? err.message : String(err)}`, - ); + const failure = toReadFailure(err); + regionFailures.push({ region, failure }); + ctx.log(`CloudTrail: could not list trails in ${region}: ${failure.error}`); continue; } @@ -149,6 +182,7 @@ export const cloudTrailEnabledCheck: IntegrationCheck = { // Track whether the logging status was actually read so a failed // GetTrailStatus is not misreported as logging=false. let loggingKnown = true; + let statusReadFailure: ReadFailure | undefined; // Logging status only matters for otherwise-compliant trails. if (multiRegion && logValidation && t.TrailARN) { // Query GetTrailStatus against the trail's home region. A multi-region @@ -163,6 +197,7 @@ export const cloudTrailEnabledCheck: IntegrationCheck = { : new CloudTrailClient({ region: homeRegion, credentials: session.credentials, + maxAttempts: 5, }); try { const status = await statusClient.send( @@ -171,8 +206,9 @@ export const cloudTrailEnabledCheck: IntegrationCheck = { logging = status.IsLogging === true; } catch (err) { loggingKnown = false; + statusReadFailure = toReadFailure(err); ctx.log( - `CloudTrail: could not read logging status for ${t.Name ?? t.TrailARN}: ${err instanceof Error ? err.message : String(err)}`, + `CloudTrail: could not read logging status for ${t.Name ?? t.TrailARN}: ${statusReadFailure.error}`, ); } } @@ -182,6 +218,7 @@ export const cloudTrailEnabledCheck: IntegrationCheck = { logValidation, logging, loggingKnown, + statusReadFailure, }); } } @@ -189,20 +226,31 @@ export const cloudTrailEnabledCheck: IntegrationCheck = { // If we found no trails AND at least one region's DescribeTrails failed, we // can't conclude "No CloudTrail configured" (that would be a false high on a // permissions/transient error) — report it as unverified instead. - if (trails.length === 0 && failedRegions.length > 0) { + if (trails.length === 0 && regionFailures.length > 0) { + const combined = combineReadFailures(regionFailures.map((r) => r.failure)); ctx.fail({ title: 'Could not verify CloudTrail configuration', - description: `CloudTrail trails could not be listed in: ${failedRegions.join(', ')}, so trail configuration is unverified.`, + description: `CloudTrail trails could not be listed in: ${regionFailures.map((r) => r.region).join(', ')}, so trail configuration is unverified.`, resourceType: 'aws-cloudtrail', resourceId: 'account', severity: 'medium', - remediation: + remediation: remediationForReadFailure( + combined, 'Grant cloudtrail:DescribeTrails to the integration role in all enabled regions, then re-run the check.', - evidence: { failedRegions }, + ), + evidence: { + failedRegions: regionFailures.map((r) => ({ + region: r.region, + error: r.failure.error, + })), + }, }); return; } - emitOutcomes(ctx, evaluateCloudTrail(trails)); + emitOutcomes( + ctx, + evaluateCloudTrail(trails, { scannedRegions: session.regions }), + ); }, }; diff --git a/packages/integration-platform/src/manifests/aws/checks/ec2.ts b/packages/integration-platform/src/manifests/aws/checks/ec2.ts index 10368f9182..994363b9d1 100644 --- a/packages/integration-platform/src/manifests/aws/checks/ec2.ts +++ b/packages/integration-platform/src/manifests/aws/checks/ec2.ts @@ -1,7 +1,15 @@ import { DescribeSecurityGroupsCommand, EC2Client } from '@aws-sdk/client-ec2'; import { TASK_TEMPLATES } from '../../../task-mappings'; import type { CheckContext, FindingSeverity, IntegrationCheck } from '../../../types'; -import { resolveAwsSessionOrFail, type CheckOutcome, emitOutcomes } from './shared'; +import { + combineReadFailures, + remediationForReadFailure, + resolveAwsSessionOrFail, + toReadFailure, + type CheckOutcome, + type ReadFailure, + emitOutcomes, +} from './shared'; export interface SgPermission { ipProtocol: string; @@ -95,12 +103,18 @@ export const ec2SecurityGroupsCheck: IntegrationCheck = { return; } const sgs: SgInfo[] = []; - const failedRegions: string[] = []; + const regionFailures: Array<{ region: string; failure: ReadFailure }> = []; for (const region of session.regions) { // Isolate per-region failures (opted-out/disabled regions, throttling) // so one region's error doesn't abort scanning of the others. try { - const ec2 = new EC2Client({ region, credentials: session.credentials }); + const ec2 = new EC2Client({ + region, + credentials: session.credentials, + // Reads are idempotent; extra attempts ride out transient network or + // throttling failures during the scheduled-run herd. + maxAttempts: 5, + }); let token: string | undefined; do { const resp = await ec2.send( @@ -125,24 +139,31 @@ export const ec2SecurityGroupsCheck: IntegrationCheck = { token = resp.NextToken; } while (token); } catch (err) { - failedRegions.push(region); - ctx.log( - `EC2: could not list security groups in ${region}: ${err instanceof Error ? err.message : String(err)}`, - ); + const failure = toReadFailure(err); + regionFailures.push({ region, failure }); + ctx.log(`EC2: could not list security groups in ${region}: ${failure.error}`); } } // A region we couldn't read is unverified — surface it instead of letting a // total/partial read failure end as a silent clean run (no findings). - if (failedRegions.length > 0) { + if (regionFailures.length > 0) { + const regions = regionFailures.map((r) => r.region); ctx.fail({ title: 'Could not verify security groups in some regions', - description: `Security groups could not be listed in: ${failedRegions.join(', ')}. Internet exposure in those regions is unverified.`, + description: `Security groups could not be listed in: ${regions.join(', ')}. Internet exposure in those regions is unverified.`, resourceType: 'aws-security-group', - resourceId: `regions:${failedRegions.join(',')}`, + resourceId: `regions:${regions.join(',')}`, severity: 'medium', - remediation: - 'Ensure the integration role can call ec2:DescribeSecurityGroups in all enabled regions, then re-run the check.', - evidence: { failedRegions }, + remediation: remediationForReadFailure( + combineReadFailures(regionFailures.map((r) => r.failure)), + 'Grant ec2:DescribeSecurityGroups to the integration role in all enabled regions, then re-run the check.', + ), + evidence: { + failedRegions: regionFailures.map((r) => ({ + region: r.region, + error: r.failure.error, + })), + }, }); } if (sgs.length === 0) return; diff --git a/packages/integration-platform/src/manifests/aws/checks/iam.ts b/packages/integration-platform/src/manifests/aws/checks/iam.ts index 813727f457..c10c674764 100644 --- a/packages/integration-platform/src/manifests/aws/checks/iam.ts +++ b/packages/integration-platform/src/manifests/aws/checks/iam.ts @@ -5,7 +5,14 @@ import { } from '@aws-sdk/client-iam'; import { TASK_TEMPLATES } from '../../../task-mappings'; import type { CheckContext, IntegrationCheck } from '../../../types'; -import { resolveAwsSessionOrFail, type CheckOutcome, emitOutcomes } from './shared'; +import { + remediationForReadFailure, + resolveAwsSessionOrFail, + toReadFailure, + type CheckOutcome, + type ReadFailure, + emitOutcomes, +} from './shared'; export interface IamAccountData { /** null = no password policy configured */ @@ -153,22 +160,48 @@ export const iamAccountSecurityCheck: IntegrationCheck = { const iam = new IAMClient({ region: session.regions[0], credentials: session.credentials, + // Reads are idempotent; extra attempts ride out transient network or + // throttling failures during the scheduled-run herd. + maxAttempts: 5, }); let passwordPolicy: IamAccountData['passwordPolicy'] = null; + let policyReadFailure: ReadFailure | undefined; try { const pp = await iam.send(new GetAccountPasswordPolicyCommand({})); passwordPolicy = pp.PasswordPolicy ?? null; } catch (err) { // No password policy set surfaces as NoSuchEntity(Exception); treat as - // null (a finding). Anything else (e.g. AccessDenied) propagates. - if (!(err instanceof Error && /NoSuchEntity/i.test(err.name))) throw err; + // null (a genuine finding). Anything else (AccessDenied, throttling) is + // indeterminate: do NOT rethrow (that would abort the whole check and + // suppress the independent root-MFA/root-access-key findings below), and + // do NOT evaluate null as "no policy" (a false finding) — surface + // "could not verify" instead. + if (!(err instanceof Error && /NoSuchEntity/i.test(err.name))) { + policyReadFailure = toReadFailure(err); + ctx.log(`IAM: could not read password policy: ${policyReadFailure.error}`); + } } // Password policy and account summary are independent — emit the // password-policy findings now so they aren't lost if the summary read // fails below. - emitOutcomes(ctx, evaluatePasswordPolicy(passwordPolicy)); + if (policyReadFailure) { + ctx.fail({ + title: 'Could not verify IAM password policy', + description: `The IAM account password policy could not be read (${policyReadFailure.error}), so password-policy strength is unverified.`, + resourceType: 'aws-account', + resourceId: 'account', + severity: 'medium', + remediation: remediationForReadFailure( + policyReadFailure, + 'Grant iam:GetAccountPasswordPolicy to the integration role, then re-run the check.', + ), + evidence: { readError: policyReadFailure.error }, + }); + } else { + emitOutcomes(ctx, evaluatePasswordPolicy(passwordPolicy)); + } try { const summaryResp = await iam.send(new GetAccountSummaryCommand({})); @@ -178,16 +211,19 @@ export const iamAccountSecurityCheck: IntegrationCheck = { // The account summary drives the root-MFA / root-access-key findings — if // it can't be read, surface "could not verify" rather than aborting the // check with a bare error (or omitting those critical findings). + const failure = toReadFailure(err); + ctx.log(`IAM: could not read account summary: ${failure.error}`); ctx.fail({ title: 'Could not verify IAM account summary', - description: - 'The IAM account summary (root MFA, root access keys) could not be read, so root-account security is unverified.', + description: `The IAM account summary (root MFA, root access keys) could not be read (${failure.error}), so root-account security is unverified.`, resourceType: 'aws-account', resourceId: 'account', severity: 'medium', - remediation: + remediation: remediationForReadFailure( + failure, 'Grant iam:GetAccountSummary to the integration role, then re-run the check.', - evidence: { error: err instanceof Error ? err.message : String(err) }, + ), + evidence: { readError: failure.error }, }); } }, diff --git a/packages/integration-platform/src/manifests/aws/checks/kms.ts b/packages/integration-platform/src/manifests/aws/checks/kms.ts index bfb38f5014..b5bdd06ac1 100644 --- a/packages/integration-platform/src/manifests/aws/checks/kms.ts +++ b/packages/integration-platform/src/manifests/aws/checks/kms.ts @@ -7,9 +7,13 @@ import { import { TASK_TEMPLATES } from '../../../task-mappings'; import type { CheckContext, IntegrationCheck } from '../../../types'; import { + combineReadFailures, + remediationForReadFailure, resolveAwsSessionOrFail, + toReadFailure, type AwsSession, type CheckOutcome, + type ReadFailure, emitOutcomes, } from './shared'; @@ -25,6 +29,8 @@ export interface KmsKeyInfo { /** false when GetKeyRotationStatus couldn't be read → emit no finding. */ rotationStatusKnown: boolean; rotationEnabled: boolean; + /** set when GetKeyRotationStatus failed — the real error, surfaced in evidence */ + rotationReadFailure?: ReadFailure; } /** @@ -37,16 +43,24 @@ export function evaluateKmsRotation(keys: KmsKeyInfo[]): CheckOutcome[] { .filter((k) => k.rotationEligible) .map((k): CheckOutcome => { if (!k.rotationStatusKnown) { + const failure = k.rotationReadFailure; return { kind: 'fail', title: `Could not verify KMS key rotation: ${k.keyId}`, - description: `Rotation status for customer-managed KMS key "${k.keyId}" (${k.region}) could not be read, so rotation is unverified.`, + description: `Rotation status for customer-managed KMS key "${k.keyId}" (${k.region}) could not be read${failure ? ` (${failure.error})` : ''}, so rotation is unverified.`, resourceType: 'aws-kms-key', resourceId: k.keyId, severity: 'medium', - remediation: + remediation: remediationForReadFailure( + failure, 'Grant kms:GetKeyRotationStatus to the integration role so rotation can be verified, then re-run.', - evidence: { keyId: k.keyId, region: k.region, rotationStatusKnown: false }, + ), + evidence: { + keyId: k.keyId, + region: k.region, + rotationStatusKnown: false, + ...(failure ? { readError: failure.error } : {}), + }, }; } return k.rotationEnabled @@ -73,8 +87,8 @@ export function evaluateKmsRotation(keys: KmsKeyInfo[]): CheckOutcome[] { interface KmsKeyScan { keys: KmsKeyInfo[]; - /** Keys whose DescribeKey failed — eligibility couldn't be classified. */ - unreadableKeyIds: string[]; + /** Keys (or "region:" markers) whose read failed, with the real error. */ + unreadable: Array<{ id: string; failure: ReadFailure }>; } async function listKmsKeys( @@ -82,9 +96,15 @@ async function listKmsKeys( session: AwsSession, ): Promise { const out: KmsKeyInfo[] = []; - const unreadableKeyIds: string[] = []; + const unreadable: Array<{ id: string; failure: ReadFailure }> = []; for (const region of session.regions) { - const kms = new KMSClient({ region, credentials: session.credentials }); + const kms = new KMSClient({ + region, + credentials: session.credentials, + // Reads are idempotent; extra attempts ride out transient network or + // throttling failures during the scheduled-run herd. + maxAttempts: 5, + }); let marker: string | undefined; try { do { @@ -99,10 +119,9 @@ async function listKmsKeys( // Can't classify this key's eligibility — record it as unreadable so // an all-unreadable account isn't reported as a clean run (a denied // kms:DescribeKey would otherwise leave zero eligible keys silently). - unreadableKeyIds.push(keyId); - ctx.log( - `KMS: could not describe key ${keyId} in ${region}: ${err instanceof Error ? err.message : String(err)}`, - ); + const failure = toReadFailure(err); + unreadable.push({ id: keyId, failure }); + ctx.log(`KMS: could not describe key ${keyId} in ${region}: ${failure.error}`); continue; } // Only symmetric, enabled, AWS-managed-material, encrypt/decrypt @@ -116,32 +135,40 @@ async function listKmsKeys( let rotationEnabled = false; let rotationStatusKnown = false; + let rotationReadFailure: ReadFailure | undefined; if (rotationEligible) { try { const rot = await kms.send(new GetKeyRotationStatusCommand({ KeyId: keyId })); rotationEnabled = rot.KeyRotationEnabled === true; rotationStatusKnown = true; } catch (err) { + rotationReadFailure = toReadFailure(err); ctx.log( - `KMS: could not read rotation status for ${keyId} in ${region}: ${err instanceof Error ? err.message : String(err)}`, + `KMS: could not read rotation status for ${keyId} in ${region}: ${rotationReadFailure.error}`, ); rotationStatusKnown = false; } } - out.push({ keyId, region, rotationEligible, rotationStatusKnown, rotationEnabled }); + out.push({ + keyId, + region, + rotationEligible, + rotationStatusKnown, + rotationEnabled, + rotationReadFailure, + }); } marker = resp.NextMarker; } while (marker); } catch (err) { // ListKeys failed for this region — record a region marker so run() // surfaces "could not verify" instead of aborting / silently skipping it. - unreadableKeyIds.push(`region:${region}`); - ctx.log( - `KMS: could not list keys in ${region}: ${err instanceof Error ? err.message : String(err)}`, - ); + const failure = toReadFailure(err); + unreadable.push({ id: `region:${region}`, failure }); + ctx.log(`KMS: could not list keys in ${region}: ${failure.error}`); } } - return { keys: out, unreadableKeyIds }; + return { keys: out, unreadable }; } export const kmsKeyRotationCheck: IntegrationCheck = { @@ -156,23 +183,23 @@ export const kmsKeyRotationCheck: IntegrationCheck = { ctx.log('AWS KMS check: connection not configured — skipping'); return; } - const { keys, unreadableKeyIds } = await listKmsKeys(ctx, session); + const { keys, unreadable } = await listKmsKeys(ctx, session); // Keys/regions that couldn't be read can't be classified — surface them so // an all-unreadable account (e.g. kms:ListKeys or kms:DescribeKey denied) // isn't recorded as a clean run with no findings. Region markers // ("region:") are ListKeys failures; the rest are DescribeKey failures. - if (unreadableKeyIds.length > 0) { - const failedRegions = unreadableKeyIds - .filter((k) => k.startsWith('region:')) - .map((k) => k.slice('region:'.length)); - const failedKeyCount = unreadableKeyIds.length - failedRegions.length; + if (unreadable.length > 0) { + const failedRegions = unreadable + .filter((u) => u.id.startsWith('region:')) + .map((u) => ({ region: u.id.slice('region:'.length), error: u.failure.error })); + const failedKeys = unreadable.filter((u) => !u.id.startsWith('region:')); const parts: string[] = []; if (failedRegions.length > 0) { - parts.push(`keys could not be listed in ${failedRegions.length} region(s) (${failedRegions.join(', ')})`); + parts.push(`keys could not be listed in ${failedRegions.length} region(s) (${failedRegions.map((r) => r.region).join(', ')})`); } - if (failedKeyCount > 0) { - parts.push(`metadata could not be read for ${failedKeyCount} key(s)`); + if (failedKeys.length > 0) { + parts.push(`metadata could not be read for ${failedKeys.length} key(s)`); } ctx.fail({ title: 'Could not verify KMS keys', @@ -180,9 +207,18 @@ export const kmsKeyRotationCheck: IntegrationCheck = { resourceType: 'aws-kms-key', resourceId: 'account', severity: 'medium', - remediation: + remediation: remediationForReadFailure( + combineReadFailures(unreadable.map((u) => u.failure)), 'Grant kms:ListKeys, kms:DescribeKey, and kms:GetKeyRotationStatus to the integration role in all enabled regions, then re-run the check.', - evidence: { failedRegions, failedKeyCount }, + ), + evidence: { + failedRegions, + failedKeyCount: failedKeys.length, + // first few per-key errors so the cause is visible without log digging + keyReadErrors: failedKeys + .slice(0, 5) + .map((u) => ({ keyId: u.id, error: u.failure.error })), + }, }); } diff --git a/packages/integration-platform/src/manifests/aws/checks/rds.ts b/packages/integration-platform/src/manifests/aws/checks/rds.ts index b4ca418632..3d95bc43d3 100644 --- a/packages/integration-platform/src/manifests/aws/checks/rds.ts +++ b/packages/integration-platform/src/manifests/aws/checks/rds.ts @@ -6,9 +6,13 @@ import { import { TASK_TEMPLATES } from '../../../task-mappings'; import type { CheckContext, IntegrationCheck } from '../../../types'; import { + combineReadFailures, + remediationForReadFailure, resolveAwsSessionOrFail, + toReadFailure, type AwsSession, type CheckOutcome, + type ReadFailure, emitOutcomes, } from './shared'; @@ -140,7 +144,7 @@ export function evaluateRdsClusterBackups(clusters: RdsClusterInfo[]): CheckOutc interface RegionScan { items: T[]; /** Regions whose listing call failed — their resources are unverified. */ - failedRegions: string[]; + failedRegions: Array<{ region: string; failure: ReadFailure }>; } async function listRdsInstances( @@ -148,11 +152,17 @@ async function listRdsInstances( ctx: CheckContext, ): Promise> { const items: RdsInstanceInfo[] = []; - const failedRegions: string[] = []; + const failedRegions: Array<{ region: string; failure: ReadFailure }> = []; for (const region of session.regions) { // Isolate per-region failures so one bad region doesn't abort the rest. try { - const rds = new RDSClient({ region, credentials: session.credentials }); + const rds = new RDSClient({ + region, + credentials: session.credentials, + // Reads are idempotent; extra attempts ride out transient network or + // throttling failures during the scheduled-run herd. + maxAttempts: 5, + }); let marker: string | undefined; do { const resp = await rds.send(new DescribeDBInstancesCommand({ Marker: marker })); @@ -168,10 +178,9 @@ async function listRdsInstances( marker = resp.Marker; } while (marker); } catch (err) { - failedRegions.push(region); - ctx.log( - `RDS: could not list DB instances in ${region}: ${err instanceof Error ? err.message : String(err)}`, - ); + const failure = toReadFailure(err); + failedRegions.push({ region, failure }); + ctx.log(`RDS: could not list DB instances in ${region}: ${failure.error}`); } } return { items, failedRegions }; @@ -182,10 +191,14 @@ async function listRdsClusters( ctx: CheckContext, ): Promise> { const items: RdsClusterInfo[] = []; - const failedRegions: string[] = []; + const failedRegions: Array<{ region: string; failure: ReadFailure }> = []; for (const region of session.regions) { try { - const rds = new RDSClient({ region, credentials: session.credentials }); + const rds = new RDSClient({ + region, + credentials: session.credentials, + maxAttempts: 5, + }); let marker: string | undefined; do { const resp = await rds.send(new DescribeDBClustersCommand({ Marker: marker })); @@ -201,10 +214,9 @@ async function listRdsClusters( marker = resp.Marker; } while (marker); } catch (err) { - failedRegions.push(region); - ctx.log( - `RDS: could not list DB clusters in ${region}: ${err instanceof Error ? err.message : String(err)}`, - ); + const failure = toReadFailure(err); + failedRegions.push({ region, failure }); + ctx.log(`RDS: could not list DB clusters in ${region}: ${failure.error}`); } } return { items, failedRegions }; @@ -216,20 +228,36 @@ async function listRdsClusters( */ function failUnverifiedRegions( ctx: CheckContext, - failedRegions: string[], + failedRegions: Array<{ region: string; failure: ReadFailure }>, what: string, ): void { if (failedRegions.length === 0) return; - const regions = [...new Set(failedRegions)]; + // One entry per region; a denied failure wins over a transient one so the + // grant hint isn't masked when instances and clusters failed differently. + const byRegion = new Map(); + for (const f of failedRegions) { + const existing = byRegion.get(f.region); + if (!existing || (f.failure.denied && !existing.denied)) { + byRegion.set(f.region, f.failure); + } + } + const regions = [...byRegion.keys()]; ctx.fail({ title: `Could not verify RDS ${what} in some regions`, description: `RDS resources could not be listed in: ${regions.join(', ')}, so ${what} in those regions is unverified.`, resourceType: 'aws-rds', resourceId: `regions:${regions.join(',')}`, severity: 'medium', - remediation: - 'Ensure the integration role can describe RDS instances and clusters in all enabled regions, then re-run the check.', - evidence: { failedRegions: regions }, + remediation: remediationForReadFailure( + combineReadFailures([...byRegion.values()]), + 'Grant rds:DescribeDBInstances and rds:DescribeDBClusters to the integration role in all enabled regions, then re-run the check.', + ), + evidence: { + failedRegions: [...byRegion.entries()].map(([region, failure]) => ({ + region, + error: failure.error, + })), + }, }); } diff --git a/packages/integration-platform/src/manifests/aws/checks/read-failure.ts b/packages/integration-platform/src/manifests/aws/checks/read-failure.ts new file mode 100644 index 0000000000..70af629b0e --- /dev/null +++ b/packages/integration-platform/src/manifests/aws/checks/read-failure.ts @@ -0,0 +1,78 @@ +/** Why a per-resource read failed: the real error plus its classification. */ +export interface ReadFailure { + /** "ErrorName: message" — preserved in finding evidence so the failure is diagnosable. */ + error: string; + /** true for authorization failures (403/AccessDenied); false for transient/network errors. */ + denied: boolean; + /** true when the region is disabled / not opted in — permanent, re-running won't help. */ + regionDisabled?: boolean; +} + +export const TRANSIENT_READ_REMEDIATION = + 'The read failed with the error shown in the evidence — not a missing permission. Re-run the check; if it keeps failing, contact support.'; + +export const REGION_DISABLED_REMEDIATION = + "The failing region(s) appear to be disabled or not opted in for this AWS account (see the error in the evidence) — remove them from the connection's regions or enable them in AWS, then re-run the check."; + +/** + * Classify a thrown read error so an "unverified" finding can tell a + * permissions problem ("grant X to the role") apart from a transient one + * ("re-run") or a disabled region ("remove the region") — asserting a missing + * permission for what was actually a transient failure sends customers on a + * wild-goose IAM audit. + */ +export function toReadFailure(err: unknown): ReadFailure { + const error = + err instanceof Error + ? `${err.name}: ${err.message}`.slice(0, 300) + : String(err).slice(0, 300); + const status = (err as { $metadata?: { httpStatusCode?: number } } | null) + ?.$metadata?.httpStatusCode; + const denied = + status === 403 || + (err instanceof Error && + /AccessDenied|UnauthorizedOperation|Forbidden|NotAuthorized/i.test( + err.name, + )); + // OptInRequired / AuthFailure are what opted-out or disabled regions throw — + // a permanent condition for this connection, not something a re-run fixes. + const regionDisabled = + !denied && + err instanceof Error && + /OptInRequired|AuthFailure/i.test(err.name); + return { error, denied, regionDisabled }; +} + +/** + * Collapse per-region/per-resource failures into one classification for an + * aggregate finding: denied wins (the grant hint is actionable), and + * region-disabled applies only when EVERY failure is one — on mixed causes the + * transient wording is used so nobody removes a healthy region. + */ +export function combineReadFailures( + failures: ReadFailure[], +): ReadFailure | undefined { + if (failures.length === 0) return undefined; + return { + error: failures + .map((f) => f.error) + .join('; ') + .slice(0, 600), + denied: failures.some((f) => f.denied), + regionDisabled: failures.every((f) => f.regionDisabled === true), + }; +} + +/** + * Remediation for an unverified-read finding: the specific grant hint only + * when the error really was an authorization failure (or when no failure + * detail exists — legacy paths); otherwise advice matching the actual cause. + */ +export function remediationForReadFailure( + failure: ReadFailure | undefined, + grantRemediation: string, +): string { + if (!failure || failure.denied) return grantRemediation; + if (failure.regionDisabled) return REGION_DISABLED_REMEDIATION; + return TRANSIENT_READ_REMEDIATION; +} diff --git a/packages/integration-platform/src/manifests/aws/checks/s3.ts b/packages/integration-platform/src/manifests/aws/checks/s3.ts index dc91453ac3..2d9570f674 100644 --- a/packages/integration-platform/src/manifests/aws/checks/s3.ts +++ b/packages/integration-platform/src/manifests/aws/checks/s3.ts @@ -12,6 +12,7 @@ import { } from './s3-buckets'; import { awsAccountIdFromCtx, + remediationForReadFailure, resolveAwsSessionOrFail, type CheckOutcome, emitOutcomes, @@ -19,9 +20,6 @@ import { export type { BpaFlags, S3BucketInfo } from './s3-buckets'; -const TRANSIENT_READ_REMEDIATION = - 'The read failed with the error shown in the evidence — not a missing permission. Re-run the check; if it keeps failing, contact support.'; - const FLAG_KEYS: Array = [ 'blockPublicAcls', 'ignorePublicAcls', @@ -42,7 +40,6 @@ export function evaluateS3Encryption(buckets: S3BucketInfo[]): CheckOutcome[] { // account pass with no findings). Only claim a missing permission when // the error actually was one — otherwise surface the real error. const failure = b.encryptionReadFailure; - const transient = failure !== undefined && !failure.denied; return { kind: 'fail', title: `Could not verify encryption: ${b.name}`, @@ -52,9 +49,10 @@ export function evaluateS3Encryption(buckets: S3BucketInfo[]): CheckOutcome[] { resourceType: 'aws-s3-bucket', resourceId: b.name, severity: 'medium', - remediation: transient - ? TRANSIENT_READ_REMEDIATION - : 'Grant s3:GetEncryptionConfiguration to the integration role so default encryption can be verified, then re-run.', + remediation: remediationForReadFailure( + failure, + 'Grant s3:GetEncryptionConfiguration to the integration role so default encryption can be verified, then re-run.', + ), evidence: { bucket: b.name, encryptionDetermined: false, @@ -91,7 +89,6 @@ export function evaluateS3PublicAccess( return buckets.map((b): CheckOutcome => { if (!b.publicAccessDetermined) { const failure = b.publicAccessReadFailure; - const transient = failure !== undefined && !failure.denied; return { kind: 'fail', title: `Could not verify public access: ${b.name}`, @@ -101,9 +98,10 @@ export function evaluateS3PublicAccess( resourceType: 'aws-s3-bucket', resourceId: b.name, severity: 'medium', - remediation: transient - ? TRANSIENT_READ_REMEDIATION - : 'Grant s3:GetBucketPublicAccessBlock to the integration role so public-access settings can be verified, then re-run.', + remediation: remediationForReadFailure( + failure, + 'Grant s3:GetBucketPublicAccessBlock to the integration role so public-access settings can be verified, then re-run.', + ), evidence: { bucket: b.name, publicAccessDetermined: false, diff --git a/packages/integration-platform/src/manifests/aws/checks/shared.ts b/packages/integration-platform/src/manifests/aws/checks/shared.ts index d9a810983c..2d014a2cec 100644 --- a/packages/integration-platform/src/manifests/aws/checks/shared.ts +++ b/packages/integration-platform/src/manifests/aws/checks/shared.ts @@ -243,35 +243,16 @@ export async function resolveAwsSessionOrFail( } } -/** Why a per-resource read failed: the real error plus whether it was an authorization failure. */ -export interface ReadFailure { - /** "ErrorName: message" — preserved in finding evidence so the failure is diagnosable. */ - error: string; - /** true for authorization failures (403/AccessDenied); false for transient/network errors. */ - denied: boolean; -} - -/** - * Classify a thrown read error so an "unverified" finding can tell a - * permissions problem ("grant X to the role") apart from a transient one - * ("re-run") — asserting a missing permission for what was actually a - * transient failure sends customers on a wild-goose IAM audit. - */ -export function toReadFailure(err: unknown): ReadFailure { - const error = - err instanceof Error - ? `${err.name}: ${err.message}`.slice(0, 300) - : String(err).slice(0, 300); - const status = (err as { $metadata?: { httpStatusCode?: number } } | null) - ?.$metadata?.httpStatusCode; - const denied = - status === 403 || - (err instanceof Error && - /AccessDenied|UnauthorizedOperation|Forbidden|NotAuthorized/i.test( - err.name, - )); - return { error, denied }; -} +// Read-failure classification lives in read-failure.ts; re-exported here so +// existing imports from './shared' keep working. +export { + combineReadFailures, + remediationForReadFailure, + toReadFailure, + REGION_DISABLED_REMEDIATION, + TRANSIENT_READ_REMEDIATION, + type ReadFailure, +} from './read-failure'; /** A provider-agnostic pass/fail outcome produced by a pure evaluator. */ export interface CheckOutcome { From e9ed93df7740d23279ca461550a6c5cbe2187351 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Wed, 10 Jun 2026 16:58:01 -0400 Subject: [PATCH 02/18] feat(trust-portal): display custom frameworks on the trust portal Lets orgs show org-authored custom frameworks on the public Trust Portal with the same toggle/status/certificate UX as native frameworks. - DB: new TrustCustomFramework join table (org + customFramework, enabled + status); TrustResource.framework made nullable with an optional customFrameworkId + exactly-one CHECK (mirrors the FrameworkInstance/CustomRequirement dual-parent pattern). - API: TrustCustomFrameworkService (admin list/update + public display); compliance-resource upload/list/signed-url extended to accept a custom framework via an XOR helper; gated NDA download + public display endpoint on trust-access. - App: Custom Frameworks section in the Trust Portal Frameworks tab; extracted the ComplianceFramework row into its own file and reused it. Companion PR in comp-private renders these on the public portal (reads over HTTP, no @trycompai/db coupling). Co-Authored-By: Claude Fable 5 --- .../dto/compliance-resource.dto.ts | 37 +- .../dto/trust-custom-framework.dto.ts | 43 ++ .../trust-portal/trust-access.controller.ts | 55 +- .../trust-portal/trust-access.service.spec.ts | 1 + .../src/trust-portal/trust-access.service.ts | 101 +++- .../trust-custom-framework.service.spec.ts | 208 ++++++++ .../trust-custom-framework.service.ts | 177 +++++++ .../trust-portal.controller.spec.ts | 15 +- .../trust-portal/trust-portal.controller.ts | 35 +- .../src/trust-portal/trust-portal.module.ts | 4 +- .../src/trust-portal/trust-portal.service.ts | 153 +++++- apps/app/src/app/(app)/[orgId]/trust/page.tsx | 51 +- .../components/ComplianceFramework.tsx | 478 ++++++++++++++++++ .../CustomFrameworksSection.test.tsx | 87 ++++ .../components/CustomFrameworksSection.tsx | 160 ++++++ .../components/TrustPortalSwitch.test.tsx | 1 + .../components/TrustPortalSwitch.tsx | 476 +---------------- .../src/hooks/use-trust-portal-settings.ts | 96 +++- .../migration.sql | 55 ++ .../db/prisma/schema/custom-framework.prisma | 4 + packages/db/prisma/schema/organization.prisma | 1 + packages/db/prisma/schema/trust.prisma | 60 ++- 22 files changed, 1744 insertions(+), 554 deletions(-) create mode 100644 apps/api/src/trust-portal/dto/trust-custom-framework.dto.ts create mode 100644 apps/api/src/trust-portal/trust-custom-framework.service.spec.ts create mode 100644 apps/api/src/trust-portal/trust-custom-framework.service.ts create mode 100644 apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/ComplianceFramework.tsx create mode 100644 apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/CustomFrameworksSection.test.tsx create mode 100644 apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/CustomFrameworksSection.tsx create mode 100644 packages/db/prisma/migrations/20260610120000_trust_custom_frameworks/migration.sql diff --git a/apps/api/src/trust-portal/dto/compliance-resource.dto.ts b/apps/api/src/trust-portal/dto/compliance-resource.dto.ts index 23a8c301c1..c31a51ad20 100644 --- a/apps/api/src/trust-portal/dto/compliance-resource.dto.ts +++ b/apps/api/src/trust-portal/dto/compliance-resource.dto.ts @@ -1,5 +1,5 @@ -import { ApiProperty } from '@nestjs/swagger'; -import { IsEnum, IsString } from 'class-validator'; +import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger'; +import { IsEnum, IsOptional, IsString } from 'class-validator'; import { TrustFramework } from '@db'; export class ComplianceResourceBaseDto { @@ -10,13 +10,26 @@ export class ComplianceResourceBaseDto { @IsString() organizationId!: string; - @ApiProperty({ - description: 'Compliance framework identifier', + // A compliance certificate targets EITHER a native framework OR a custom + // framework. Exactly one of `framework` / `customFrameworkId` must be set; + // the service enforces this (assertExactlyOneFrameworkRef). + @ApiPropertyOptional({ + description: 'Native compliance framework identifier', enum: TrustFramework, example: TrustFramework.iso_27001, }) + @IsOptional() @IsEnum(TrustFramework) - framework!: TrustFramework; + framework?: TrustFramework; + + @ApiPropertyOptional({ + description: + 'Org-authored custom framework ID (alternative to `framework`)', + example: 'cfrm_6914cd0e16e4c7dccbb54426', + }) + @IsOptional() + @IsString() + customFrameworkId?: string; } export class UploadComplianceResourceDto extends ComplianceResourceBaseDto { @@ -44,8 +57,18 @@ export class UploadComplianceResourceDto extends ComplianceResourceBaseDto { export class ComplianceResourceSignedUrlDto extends ComplianceResourceBaseDto {} export class ComplianceResourceResponseDto { - @ApiProperty({ enum: TrustFramework }) - framework!: TrustFramework; + @ApiPropertyOptional({ + enum: TrustFramework, + description: 'Set for native-framework certificates; null for custom ones', + nullable: true, + }) + framework!: TrustFramework | null; + + @ApiPropertyOptional({ + description: 'Set for custom-framework certificates; null for native ones', + nullable: true, + }) + customFrameworkId!: string | null; @ApiProperty() fileName!: string; diff --git a/apps/api/src/trust-portal/dto/trust-custom-framework.dto.ts b/apps/api/src/trust-portal/dto/trust-custom-framework.dto.ts new file mode 100644 index 0000000000..38f792c8b0 --- /dev/null +++ b/apps/api/src/trust-portal/dto/trust-custom-framework.dto.ts @@ -0,0 +1,43 @@ +import { z } from 'zod'; + +/** + * Update the public Trust Portal selection for a single org-authored custom + * framework. Mirrors the enabled + status that native frameworks store as + * columns on `Trust`. At least one of `enabled` / `status` must be provided. + */ +export const UpdateTrustCustomFrameworkSchema = z + .object({ + customFrameworkId: z.string().min(1), + enabled: z.boolean().optional(), + status: z.enum(['started', 'in_progress', 'compliant']).optional(), + }) + .refine((data) => data.enabled !== undefined || data.status !== undefined, { + message: 'At least one of `enabled` or `status` must be provided', + }); + +export type UpdateTrustCustomFrameworkDto = z.infer< + typeof UpdateTrustCustomFrameworkSchema +>; + +/** A custom framework plus its Trust Portal selection state (admin view). */ +export interface TrustCustomFrameworkAdminItem { + customFrameworkId: string; + name: string; + description: string; + /** Whether the framework is shown on the public portal. */ + enabled: boolean; + /** Displayed status; defaults to 'started' when never configured. */ + status: 'started' | 'in_progress' | 'compliant'; + /** Whether a compliance certificate PDF has been uploaded. */ + hasCertificate: boolean; + certificateFileName: string | null; +} + +/** A custom framework as shown on the public portal. */ +export interface TrustCustomFrameworkPublicItem { + id: string; + name: string; + description: string; + status: 'started' | 'in_progress' | 'compliant'; + hasCertificate: boolean; +} diff --git a/apps/api/src/trust-portal/trust-access.controller.ts b/apps/api/src/trust-portal/trust-access.controller.ts index 1845bb01fd..05dc1358cf 100644 --- a/apps/api/src/trust-portal/trust-access.controller.ts +++ b/apps/api/src/trust-portal/trust-access.controller.ts @@ -456,10 +456,7 @@ export class TrustAccessController { @Param('token') token: string, @Param('policyId') policyId: string, ) { - return this.trustAccessService.downloadPolicyByAccessToken( - token, - policyId, - ); + return this.trustAccessService.downloadPolicyByAccessToken(token, policyId); } @Get('access/:token/policies/download-all-zip') @@ -558,6 +555,37 @@ export class TrustAccessController { ); } + @Get('access/:token/compliance-resources/custom/:customFrameworkId') + @HttpCode(HttpStatus.OK) + @ApiOperation({ + summary: + 'Download a custom-framework compliance certificate by access token', + description: + 'Get a signed URL to download a specific custom-framework certificate file', + }) + @ApiParam({ + name: 'customFrameworkId', + description: 'Org-authored custom framework ID', + example: 'cfrm_abc123', + }) + @ApiResponse({ + status: HttpStatus.OK, + description: 'Signed URL for the custom-framework certificate returned', + }) + @ApiResponse({ + status: HttpStatus.NOT_FOUND, + description: 'Certificate not found', + }) + async getCustomComplianceResourceUrlByAccessToken( + @Param('token') token: string, + @Param('customFrameworkId') customFrameworkId: string, + ) { + return this.trustAccessService.getCustomComplianceResourceUrlByAccessToken( + token, + customFrameworkId, + ); + } + @Get('access/:token/compliance-resources/:framework') @HttpCode(HttpStatus.OK) @ApiOperation({ @@ -719,4 +747,23 @@ export class TrustAccessController { async getPublicVendors(@Param('friendlyUrl') friendlyUrl: string) { return this.trustAccessService.getPublicVendors(friendlyUrl); } + + @Get(':friendlyUrl/custom-frameworks') + @HttpCode(HttpStatus.OK) + @ApiOperation({ + summary: 'Get org-authored custom frameworks shown on a trust portal', + description: + 'Retrieve the list of custom frameworks the org has chosen to display on its public trust portal.', + }) + @ApiParam({ + name: 'friendlyUrl', + description: 'Trust Portal friendly URL or Organization ID', + }) + @ApiResponse({ + status: HttpStatus.OK, + description: 'Custom frameworks retrieved successfully', + }) + async getPublicCustomFrameworks(@Param('friendlyUrl') friendlyUrl: string) { + return this.trustAccessService.getPublicCustomFrameworks(friendlyUrl); + } } diff --git a/apps/api/src/trust-portal/trust-access.service.spec.ts b/apps/api/src/trust-portal/trust-access.service.spec.ts index 19e80f3c43..b67329857f 100644 --- a/apps/api/src/trust-portal/trust-access.service.spec.ts +++ b/apps/api/src/trust-portal/trust-access.service.spec.ts @@ -70,6 +70,7 @@ describe('TrustAccessService favicon branding', () => { {} as any, {} as any, {} as any, + {} as any, ); beforeEach(() => { diff --git a/apps/api/src/trust-portal/trust-access.service.ts b/apps/api/src/trust-portal/trust-access.service.ts index 407014849c..657f67b430 100644 --- a/apps/api/src/trust-portal/trust-access.service.ts +++ b/apps/api/src/trust-portal/trust-access.service.ts @@ -17,6 +17,8 @@ import { TrustEmailService } from './email.service'; import { NdaPdfService } from './nda-pdf.service'; import { AttachmentsService } from '../attachments/attachments.service'; import { PolicyPdfRendererService } from './policy-pdf-renderer.service'; +import { TrustCustomFrameworkService } from './trust-custom-framework.service'; +import type { TrustCustomFrameworkPublicItem } from './dto/trust-custom-framework.dto'; import { GetObjectCommand, PutObjectCommand } from '@aws-sdk/client-s3'; import { APP_AWS_ORG_ASSETS_BUCKET, s3Client, getSignedUrl } from '../app/s3'; import { Prisma, TrustFramework } from '@db'; @@ -234,6 +236,7 @@ export class TrustAccessService { private readonly emailService: TrustEmailService, private readonly attachmentsService: AttachmentsService, private readonly pdfRendererService: PolicyPdfRendererService, + private readonly trustCustomFrameworkService: TrustCustomFrameworkService, ) { if ( !process.env.TRUST_APP_URL && @@ -1661,6 +1664,8 @@ export class TrustAccessService { }, select: { framework: true, + customFrameworkId: true, + customFramework: { select: { name: true } }, fileName: true, fileSize: true, updatedAt: true, @@ -1670,9 +1675,13 @@ export class TrustAccessService { }, }); - // Return all resources - the download endpoint will auto-enable frameworks as needed + // Return all resources - the download endpoint will auto-enable frameworks as needed. + // Custom-framework certificates carry customFrameworkId + the framework name so the + // gated access page can label and download them (native ones keep `framework`). return complianceResources.map((resource) => ({ framework: resource.framework, + customFrameworkId: resource.customFrameworkId, + customFrameworkName: resource.customFramework?.name ?? null, fileName: resource.fileName, fileSize: resource.fileSize, updatedAt: resource.updatedAt.toISOString(), @@ -2036,6 +2045,96 @@ export class TrustAccessService { }; } + /** + * Gated download of a custom-framework certificate (token + NDA required). + * Mirrors getComplianceResourceUrlByAccessToken but keyed by customFrameworkId. + * No framework auto-enable: custom display visibility is governed separately by + * TrustCustomFramework.enabled. + */ + async getCustomComplianceResourceUrlByAccessToken( + token: string, + customFrameworkId: string, + ) { + const grant = await this.validateAccessToken(token); + + if (!s3Client || !APP_AWS_ORG_ASSETS_BUCKET) { + throw new InternalServerErrorException( + 'Organization assets bucket is not configured', + ); + } + + const record = await db.trustResource.findUnique({ + where: { + organizationId_customFrameworkId: { + organizationId: grant.accessRequest.organizationId, + customFrameworkId, + }, + }, + }); + + if (!record) { + throw new NotFoundException( + 'No certificate uploaded for this custom framework', + ); + } + + const getCommand = new GetObjectCommand({ + Bucket: APP_AWS_ORG_ASSETS_BUCKET, + Key: record.s3Key, + }); + + const response = await s3Client.send(getCommand); + const chunks: Uint8Array[] = []; + + if (!response.Body) { + throw new InternalServerErrorException('No file data received from S3'); + } + + for await (const chunk of response.Body as Readable) { + chunks.push(chunk); + } + + const originalPdfBuffer = Buffer.concat(chunks); + + const docId = `compliance-${grant.id}-custom-${customFrameworkId}-${Date.now()}`; + const watermarked = await this.ndaPdfService.watermarkExistingPdf( + originalPdfBuffer, + { + name: grant.accessRequest.name, + email: grant.subjectEmail, + docId, + watermarkText: 'Comp AI', + }, + ); + + const key = await this.attachmentsService.uploadToS3( + watermarked, + `compliance-custom-${customFrameworkId}-grant-${grant.id}-${Date.now()}.pdf`, + 'application/pdf', + grant.accessRequest.organizationId, + 'trust_compliance_downloads', + `${grant.id}`, + ); + + const downloadUrl = + await this.attachmentsService.getPresignedDownloadUrl(key); + + return { + signedUrl: downloadUrl, + fileName: record.fileName, + fileSize: watermarked.length, + }; + } + + /** Custom frameworks shown on the public portal (delegates to the dedicated service). */ + async getPublicCustomFrameworks( + friendlyUrl: string, + ): Promise { + return this.trustCustomFrameworkService.getPublicCustomFrameworks( + friendlyUrl, + ); + } + async downloadAllPoliciesByAccessToken(token: string) { const grant = await this.validateAccessToken(token); diff --git a/apps/api/src/trust-portal/trust-custom-framework.service.spec.ts b/apps/api/src/trust-portal/trust-custom-framework.service.spec.ts new file mode 100644 index 0000000000..3c0e243c74 --- /dev/null +++ b/apps/api/src/trust-portal/trust-custom-framework.service.spec.ts @@ -0,0 +1,208 @@ +import { NotFoundException } from '@nestjs/common'; +import { db } from '@db'; +import { TrustCustomFrameworkService } from './trust-custom-framework.service'; + +jest.mock('@db', () => ({ + db: { + customFramework: { + findMany: jest.fn(), + findFirst: jest.fn(), + }, + trustCustomFramework: { + findMany: jest.fn(), + upsert: jest.fn(), + }, + trustResource: { + findMany: jest.fn(), + }, + trust: { + findUnique: jest.fn(), + }, + }, +})); + +const mockDb = db as unknown as { + customFramework: { findMany: jest.Mock; findFirst: jest.Mock }; + trustCustomFramework: { findMany: jest.Mock; upsert: jest.Mock }; + trustResource: { findMany: jest.Mock }; + trust: { findUnique: jest.Mock }; +}; + +describe('TrustCustomFrameworkService', () => { + let service: TrustCustomFrameworkService; + + beforeEach(() => { + jest.clearAllMocks(); + service = new TrustCustomFrameworkService(); + }); + + describe('listForOrg', () => { + it('merges custom frameworks with selection state and certificates', async () => { + mockDb.customFramework.findMany.mockResolvedValue([ + { id: 'cfrm_a', name: 'Acme Std', description: 'Internal' }, + { id: 'cfrm_b', name: 'HR Base', description: 'HR' }, + ]); + mockDb.trustCustomFramework.findMany.mockResolvedValue([ + { customFrameworkId: 'cfrm_a', enabled: true, status: 'compliant' }, + ]); + mockDb.trustResource.findMany.mockResolvedValue([ + { customFrameworkId: 'cfrm_a', fileName: 'acme.pdf' }, + ]); + + const result = await service.listForOrg('org_1'); + + expect(result).toEqual([ + { + customFrameworkId: 'cfrm_a', + name: 'Acme Std', + description: 'Internal', + enabled: true, + status: 'compliant', + hasCertificate: true, + certificateFileName: 'acme.pdf', + }, + { + // Never configured for the portal -> disabled / started / no cert. + customFrameworkId: 'cfrm_b', + name: 'HR Base', + description: 'HR', + enabled: false, + status: 'started', + hasCertificate: false, + certificateFileName: null, + }, + ]); + expect(mockDb.customFramework.findMany).toHaveBeenCalledWith({ + where: { organizationId: 'org_1' }, + select: { id: true, name: true, description: true }, + orderBy: { name: 'asc' }, + }); + }); + + it('returns an empty array when the org has no custom frameworks', async () => { + mockDb.customFramework.findMany.mockResolvedValue([]); + mockDb.trustCustomFramework.findMany.mockResolvedValue([]); + mockDb.trustResource.findMany.mockResolvedValue([]); + + await expect(service.listForOrg('org_1')).resolves.toEqual([]); + }); + }); + + describe('updateSelection', () => { + it('throws NotFound when the custom framework is not in the org', async () => { + mockDb.customFramework.findFirst.mockResolvedValue(null); + + await expect( + service.updateSelection('org_1', { + customFrameworkId: 'cfrm_x', + enabled: true, + }), + ).rejects.toBeInstanceOf(NotFoundException); + expect(mockDb.trustCustomFramework.upsert).not.toHaveBeenCalled(); + }); + + it('upserts only the provided fields', async () => { + mockDb.customFramework.findFirst.mockResolvedValue({ id: 'cfrm_a' }); + mockDb.trustCustomFramework.upsert.mockResolvedValue({}); + + await service.updateSelection('org_1', { + customFrameworkId: 'cfrm_a', + status: 'in_progress', + }); + + expect(mockDb.trustCustomFramework.upsert).toHaveBeenCalledWith({ + where: { + organizationId_customFrameworkId: { + organizationId: 'org_1', + customFrameworkId: 'cfrm_a', + }, + }, + create: { + organizationId: 'org_1', + customFrameworkId: 'cfrm_a', + status: 'in_progress', + }, + update: { status: 'in_progress' }, + }); + }); + + it('scopes the tenant check by organizationId', async () => { + mockDb.customFramework.findFirst.mockResolvedValue({ id: 'cfrm_a' }); + mockDb.trustCustomFramework.upsert.mockResolvedValue({}); + + await service.updateSelection('org_1', { + customFrameworkId: 'cfrm_a', + enabled: false, + }); + + expect(mockDb.customFramework.findFirst).toHaveBeenCalledWith({ + where: { id: 'cfrm_a', organizationId: 'org_1' }, + select: { id: true }, + }); + }); + }); + + describe('getPublicCustomFrameworks', () => { + it('returns [] when no trust portal resolves for the friendly URL', async () => { + mockDb.trust.findUnique.mockResolvedValue(null); + + await expect( + service.getPublicCustomFrameworks('unknown'), + ).resolves.toEqual([]); + expect(mockDb.trustCustomFramework.findMany).not.toHaveBeenCalled(); + }); + + it('returns only enabled frameworks with certificate flags', async () => { + mockDb.trust.findUnique.mockResolvedValue({ organizationId: 'org_1' }); + mockDb.trustCustomFramework.findMany.mockResolvedValue([ + { + status: 'compliant', + customFramework: { id: 'cfrm_a', name: 'Acme Std', description: 'x' }, + }, + ]); + mockDb.trustResource.findMany.mockResolvedValue([ + { customFrameworkId: 'cfrm_a' }, + ]); + + const result = await service.getPublicCustomFrameworks('acme'); + + expect(result).toEqual([ + { + id: 'cfrm_a', + name: 'Acme Std', + description: 'x', + status: 'compliant', + hasCertificate: true, + }, + ]); + expect(mockDb.trustCustomFramework.findMany).toHaveBeenCalledWith({ + where: { organizationId: 'org_1', enabled: true }, + select: { + status: true, + customFramework: { + select: { id: true, name: true, description: true }, + }, + }, + orderBy: { customFramework: { name: 'asc' } }, + }); + }); + + it('falls back to resolving the route id as an organizationId', async () => { + mockDb.trust.findUnique + .mockResolvedValueOnce(null) // friendlyUrl miss + .mockResolvedValueOnce({ organizationId: 'org_1' }); // org id hit + mockDb.trustCustomFramework.findMany.mockResolvedValue([]); + + await service.getPublicCustomFrameworks('org_1'); + + expect(mockDb.trust.findUnique).toHaveBeenNthCalledWith(1, { + where: { friendlyUrl: 'org_1' }, + select: { organizationId: true }, + }); + expect(mockDb.trust.findUnique).toHaveBeenNthCalledWith(2, { + where: { organizationId: 'org_1' }, + select: { organizationId: true }, + }); + }); + }); +}); diff --git a/apps/api/src/trust-portal/trust-custom-framework.service.ts b/apps/api/src/trust-portal/trust-custom-framework.service.ts new file mode 100644 index 0000000000..daeaaa2f47 --- /dev/null +++ b/apps/api/src/trust-portal/trust-custom-framework.service.ts @@ -0,0 +1,177 @@ +import { Injectable, Logger, NotFoundException } from '@nestjs/common'; +import { db } from '@db'; +import type { + TrustCustomFrameworkAdminItem, + TrustCustomFrameworkPublicItem, + UpdateTrustCustomFrameworkDto, +} from './dto/trust-custom-framework.dto'; + +/** + * Manages which org-authored custom frameworks are displayed on the public + * Trust Portal. Native frameworks store their enabled/status as columns on + * `Trust`; custom frameworks are dynamic per-org rows, so their portal selection + * lives in the `TrustCustomFramework` join table. + * + * Certificate upload/download for custom frameworks reuses the shared PDF/S3 + * code paths in TrustPortalService / TrustAccessService (keyed by + * `customFrameworkId` instead of the TrustFramework enum). + */ +@Injectable() +export class TrustCustomFrameworkService { + private readonly logger = new Logger(TrustCustomFrameworkService.name); + + /** + * List every custom framework the org owns, joined with its Trust Portal + * selection state and whether a certificate has been uploaded. Frameworks the + * org never configured for the portal come back as disabled / 'started'. + */ + async listForOrg( + organizationId: string, + ): Promise { + const [customFrameworks, selections, certificates] = await Promise.all([ + db.customFramework.findMany({ + where: { organizationId }, + select: { id: true, name: true, description: true }, + orderBy: { name: 'asc' }, + }), + db.trustCustomFramework.findMany({ + where: { organizationId }, + select: { customFrameworkId: true, enabled: true, status: true }, + }), + db.trustResource.findMany({ + where: { organizationId, customFrameworkId: { not: null } }, + select: { customFrameworkId: true, fileName: true }, + }), + ]); + + const selectionByFramework = new Map( + selections.map((s) => [s.customFrameworkId, s]), + ); + const certificateByFramework = new Map( + certificates + .filter((c) => c.customFrameworkId) + .map((c) => [c.customFrameworkId as string, c.fileName]), + ); + + return customFrameworks.map((framework) => { + const selection = selectionByFramework.get(framework.id); + const certificateFileName = + certificateByFramework.get(framework.id) ?? null; + return { + customFrameworkId: framework.id, + name: framework.name, + description: framework.description, + enabled: selection?.enabled ?? false, + status: selection?.status ?? 'started', + hasCertificate: certificateFileName !== null, + certificateFileName, + }; + }); + } + + /** + * Upsert the portal selection (enabled / status) for one custom framework. + * Only the provided fields are changed; on first write the schema defaults + * (enabled=true, status='started') fill the rest. + */ + async updateSelection( + organizationId: string, + dto: UpdateTrustCustomFrameworkDto, + ): Promise<{ success: true }> { + const { customFrameworkId, enabled, status } = dto; + + // Tenant check: the custom framework must belong to this org. Also satisfies + // the composite FK (customFrameworkId, organizationId) -> CustomFramework. + const customFramework = await db.customFramework.findFirst({ + where: { id: customFrameworkId, organizationId }, + select: { id: true }, + }); + + if (!customFramework) { + throw new NotFoundException('Custom framework not found'); + } + + const changes = { + ...(enabled !== undefined ? { enabled } : {}), + ...(status !== undefined ? { status } : {}), + }; + + await db.trustCustomFramework.upsert({ + where: { + organizationId_customFrameworkId: { organizationId, customFrameworkId }, + }, + create: { organizationId, customFrameworkId, ...changes }, + update: changes, + }); + + this.logger.log( + `Updated trust portal selection for custom framework ${customFrameworkId} (org ${organizationId})`, + ); + + return { success: true }; + } + + /** + * Custom frameworks shown on the public portal for a given friendly URL. + * Only enabled selections are returned. Resolves the org by friendlyUrl, + * falling back to treating the route id as the organizationId (matches the + * other public trust-access endpoints). + */ + async getPublicCustomFrameworks( + friendlyUrl: string, + ): Promise { + const organizationId = await this.resolveOrganizationId(friendlyUrl); + if (!organizationId) { + return []; + } + + const selections = await db.trustCustomFramework.findMany({ + where: { organizationId, enabled: true }, + select: { + status: true, + customFramework: { + select: { id: true, name: true, description: true }, + }, + }, + orderBy: { customFramework: { name: 'asc' } }, + }); + + if (selections.length === 0) { + return []; + } + + const certificates = await db.trustResource.findMany({ + where: { organizationId, customFrameworkId: { not: null } }, + select: { customFrameworkId: true }, + }); + const withCertificate = new Set( + certificates.map((c) => c.customFrameworkId), + ); + + return selections.map((selection) => ({ + id: selection.customFramework.id, + name: selection.customFramework.name, + description: selection.customFramework.description, + status: selection.status, + hasCertificate: withCertificate.has(selection.customFramework.id), + })); + } + + private async resolveOrganizationId( + friendlyUrl: string, + ): Promise { + const byFriendlyUrl = await db.trust.findUnique({ + where: { friendlyUrl }, + select: { organizationId: true }, + }); + if (byFriendlyUrl) { + return byFriendlyUrl.organizationId; + } + + const byOrgId = await db.trust.findUnique({ + where: { organizationId: friendlyUrl }, + select: { organizationId: true }, + }); + return byOrgId?.organizationId ?? null; + } +} diff --git a/apps/api/src/trust-portal/trust-portal.controller.spec.ts b/apps/api/src/trust-portal/trust-portal.controller.spec.ts index 142fc1bc61..2bdbefaa21 100644 --- a/apps/api/src/trust-portal/trust-portal.controller.spec.ts +++ b/apps/api/src/trust-portal/trust-portal.controller.spec.ts @@ -2,6 +2,7 @@ import { Test, TestingModule } from '@nestjs/testing'; import { BadRequestException } from '@nestjs/common'; import { TrustPortalController } from './trust-portal.controller'; import { TrustPortalService } from './trust-portal.service'; +import { TrustCustomFrameworkService } from './trust-custom-framework.service'; import { HybridAuthGuard } from '../auth/hybrid-auth.guard'; import { PermissionGuard } from '../auth/permission.guard'; import type { AuthContext as AuthContextType } from '../auth/types'; @@ -51,6 +52,12 @@ describe('TrustPortalController', () => { getAllVendorsWithSync: jest.fn(), }; + const mockCustomFrameworkService = { + listForOrg: jest.fn(), + updateSelection: jest.fn(), + getPublicCustomFrameworks: jest.fn(), + }; + const mockGuard = { canActivate: jest.fn().mockReturnValue(true) }; const orgId = 'org_test123'; @@ -71,7 +78,13 @@ describe('TrustPortalController', () => { beforeEach(async () => { const module: TestingModule = await Test.createTestingModule({ controllers: [TrustPortalController], - providers: [{ provide: TrustPortalService, useValue: mockService }], + providers: [ + { provide: TrustPortalService, useValue: mockService }, + { + provide: TrustCustomFrameworkService, + useValue: mockCustomFrameworkService, + }, + ], }) .overrideGuard(HybridAuthGuard) .useValue(mockGuard) diff --git a/apps/api/src/trust-portal/trust-portal.controller.ts b/apps/api/src/trust-portal/trust-portal.controller.ts index 30f67b916d..83c47e68ed 100644 --- a/apps/api/src/trust-portal/trust-portal.controller.ts +++ b/apps/api/src/trust-portal/trust-portal.controller.ts @@ -58,7 +58,9 @@ import type { UpdateTrustOverviewDto } from './dto/update-trust-overview.dto'; import { UpdateTrustOverviewSchema } from './dto/update-trust-overview.dto'; import type { UpdateVendorTrustSettingsDto } from './dto/trust-vendor.dto'; import { UpdateVendorTrustSettingsSchema } from './dto/trust-vendor.dto'; +import { UpdateTrustCustomFrameworkSchema } from './dto/trust-custom-framework.dto'; import { TrustPortalService } from './trust-portal.service'; +import { TrustCustomFrameworkService } from './trust-custom-framework.service'; class ListComplianceResourcesDto { @ApiProperty({ @@ -74,7 +76,10 @@ class ListComplianceResourcesDto { @UseGuards(HybridAuthGuard, PermissionGuard) @ApiSecurity('apikey') export class TrustPortalController { - constructor(private readonly trustPortalService: TrustPortalService) {} + constructor( + private readonly trustPortalService: TrustPortalService, + private readonly trustCustomFrameworkService: TrustCustomFrameworkService, + ) {} @Get('settings') @HttpCode(HttpStatus.OK) @@ -391,6 +396,34 @@ export class TrustPortalController { return this.trustPortalService.updateFrameworks(organizationId, body); } + @Get('custom-frameworks') + @HttpCode(HttpStatus.OK) + @RequirePermission('trust', 'read') + @ApiOperation({ + summary: + 'List org-authored custom frameworks with their trust portal selection', + }) + async listCustomFrameworks(@OrganizationId() organizationId: string) { + return this.trustCustomFrameworkService.listForOrg(organizationId); + } + + @Put('custom-frameworks') + @RequirePermission('trust', 'update') + @ApiOperation({ + summary: + 'Enable/disable a custom framework on the trust portal and set its status', + }) + async updateCustomFramework( + @OrganizationId() organizationId: string, + @Body() body: unknown, + ) { + const dto = UpdateTrustCustomFrameworkSchema.parse(body); + return this.trustCustomFrameworkService.updateSelection( + organizationId, + dto, + ); + } + @Post('overview') @HttpCode(HttpStatus.OK) @RequirePermission('trust', 'update') diff --git a/apps/api/src/trust-portal/trust-portal.module.ts b/apps/api/src/trust-portal/trust-portal.module.ts index a9764f9bcf..bc783eb4ec 100644 --- a/apps/api/src/trust-portal/trust-portal.module.ts +++ b/apps/api/src/trust-portal/trust-portal.module.ts @@ -8,17 +8,19 @@ import { TrustAccessController } from './trust-access.controller'; import { TrustAccessService } from './trust-access.service'; import { TrustPortalController } from './trust-portal.controller'; import { TrustPortalService } from './trust-portal.service'; +import { TrustCustomFrameworkService } from './trust-custom-framework.service'; @Module({ imports: [AuthModule, AttachmentsModule], controllers: [TrustPortalController, TrustAccessController], providers: [ TrustPortalService, + TrustCustomFrameworkService, TrustAccessService, NdaPdfService, TrustEmailService, PolicyPdfRendererService, ], - exports: [TrustPortalService, TrustAccessService], + exports: [TrustPortalService, TrustCustomFrameworkService, TrustAccessService], }) export class TrustPortalModule {} diff --git a/apps/api/src/trust-portal/trust-portal.service.ts b/apps/api/src/trust-portal/trust-portal.service.ts index e89cb6b6da..616323a7ae 100644 --- a/apps/api/src/trust-portal/trust-portal.service.ts +++ b/apps/api/src/trust-portal/trust-portal.service.ts @@ -308,22 +308,34 @@ export class TrustPortalService { dto: UploadComplianceResourceDto, ): Promise { this.ensureS3Availability(); - await this.assertFrameworkIsCompliant(dto.organizationId, dto.framework); + const target = this.assertExactlyOneFrameworkRef( + dto.framework, + dto.customFrameworkId, + ); + + let slug: string; + if (target.kind === 'native') { + await this.assertFrameworkIsCompliant( + dto.organizationId, + target.framework, + ); + slug = TrustPortalService.FRAMEWORK_CONFIG[target.framework].slug; + } else { + await this.assertCustomFrameworkIsCompliant( + dto.organizationId, + target.customFrameworkId, + ); + slug = `custom-${target.customFrameworkId}`; + } const { fileBuffer, sanitizedFileName } = this.preparePdfPayload(dto); - const slug = TrustPortalService.FRAMEWORK_CONFIG[dto.framework].slug; const timestamp = Date.now(); const s3Prefix = `${dto.organizationId}/resources/${slug}`; const s3Key = `${s3Prefix}/${timestamp}-${sanitizedFileName}`; - const existingResource = await db.trustResource.findUnique({ - where: { - organizationId_framework: { - organizationId: dto.organizationId, - framework: dto.framework, - }, - }, - }); + const where = this.buildResourceWhere(dto.organizationId, target); + + const existingResource = await db.trustResource.findUnique({ where }); if (existingResource) { await this.safeDeleteObject(existingResource.s3Key); @@ -343,13 +355,13 @@ export class TrustPortalService { await s3Client!.send(putCommand); + const frameworkRef = + target.kind === 'native' + ? { framework: target.framework } + : { customFrameworkId: target.customFrameworkId }; + const record = await db.trustResource.upsert({ - where: { - organizationId_framework: { - organizationId: dto.organizationId, - framework: dto.framework, - }, - }, + where, update: { s3Key, fileName: dto.fileName, @@ -357,7 +369,7 @@ export class TrustPortalService { }, create: { organizationId: dto.organizationId, - framework: dto.framework, + ...frameworkRef, s3Key, fileName: dto.fileName, fileSize: fileBuffer.length, @@ -366,6 +378,7 @@ export class TrustPortalService { return { framework: record.framework, + customFrameworkId: record.customFrameworkId, fileName: record.fileName, fileSize: record.fileSize, updatedAt: record.updatedAt.toISOString(), @@ -386,6 +399,7 @@ export class TrustPortalService { return records.map((record) => ({ framework: record.framework, + customFrameworkId: record.customFrameworkId, fileName: record.fileName, fileSize: record.fileSize, updatedAt: record.updatedAt.toISOString(), @@ -396,19 +410,20 @@ export class TrustPortalService { dto: ComplianceResourceSignedUrlDto, ): Promise { this.ensureS3Availability(); + const target = this.assertExactlyOneFrameworkRef( + dto.framework, + dto.customFrameworkId, + ); const record = await db.trustResource.findUnique({ - where: { - organizationId_framework: { - organizationId: dto.organizationId, - framework: dto.framework, - }, - }, + where: this.buildResourceWhere(dto.organizationId, target), }); if (!record) { throw new NotFoundException( - `No certificate uploaded for framework ${dto.framework}`, + target.kind === 'native' + ? `No certificate uploaded for framework ${target.framework}` + : 'No certificate uploaded for this custom framework', ); } @@ -1210,6 +1225,96 @@ export class TrustPortalService { }; } + /** + * A compliance certificate targets EITHER a native framework OR a custom + * framework — never both, never neither. Validates the DTO and returns a + * discriminated target the cert methods branch on. + */ + private assertExactlyOneFrameworkRef( + framework: TrustFramework | undefined, + customFrameworkId: string | undefined, + ): + | { kind: 'native'; framework: TrustFramework } + | { kind: 'custom'; customFrameworkId: string } { + const hasNative = framework !== undefined && framework !== null; + const hasCustom = Boolean(customFrameworkId); + if (hasNative === hasCustom) { + throw new BadRequestException( + 'Provide exactly one of `framework` or `customFrameworkId`', + ); + } + return hasNative + ? { kind: 'native', framework: framework as TrustFramework } + : { kind: 'custom', customFrameworkId: customFrameworkId as string }; + } + + private buildResourceWhere( + organizationId: string, + target: + | { kind: 'native'; framework: TrustFramework } + | { kind: 'custom'; customFrameworkId: string }, + ): Prisma.TrustResourceWhereUniqueInput { + return target.kind === 'native' + ? { + organizationId_framework: { + organizationId, + framework: target.framework, + }, + } + : { + organizationId_customFrameworkId: { + organizationId, + customFrameworkId: target.customFrameworkId, + }, + }; + } + + /** + * Custom-framework analog of assertFrameworkIsCompliant: the framework must be + * selected for the portal and marked compliant before a certificate can be + * uploaded. Mirrors native by auto-enabling display on upload. + */ + private async assertCustomFrameworkIsCompliant( + organizationId: string, + customFrameworkId: string, + ): Promise { + const customFramework = await db.customFramework.findFirst({ + where: { id: customFrameworkId, organizationId }, + select: { id: true }, + }); + + if (!customFramework) { + throw new BadRequestException( + 'Custom framework not found for organization', + ); + } + + const selection = await db.trustCustomFramework.findUnique({ + where: { + organizationId_customFrameworkId: { organizationId, customFrameworkId }, + }, + select: { status: true, enabled: true }, + }); + + if (!selection || selection.status !== 'compliant') { + throw new BadRequestException( + 'Custom framework must be marked as compliant before uploading a certificate', + ); + } + + if (!selection.enabled) { + await db.trustCustomFramework.update({ + where: { + organizationId_customFrameworkId: { + organizationId, + customFrameworkId, + }, + }, + data: { enabled: true }, + }); + } + } + private async assertFrameworkIsCompliant( organizationId: string, framework: TrustFramework, diff --git a/apps/app/src/app/(app)/[orgId]/trust/page.tsx b/apps/app/src/app/(app)/[orgId]/trust/page.tsx index 2dac763620..aba9167cc1 100644 --- a/apps/app/src/app/(app)/[orgId]/trust/page.tsx +++ b/apps/app/src/app/(app)/[orgId]/trust/page.tsx @@ -6,36 +6,36 @@ import Link from 'next/link'; import { TrustPortalGettingStarted } from './components/TrustPortalGettingStarted'; import { TrustPortalSwitch } from './portal-settings/components/TrustPortalSwitch'; -export default async function TrustPage({ - params, -}: { - params: Promise<{ orgId: string }>; -}) { +export default async function TrustPage({ params }: { params: Promise<{ orgId: string }> }) { const { orgId } = await params; - const [settingsRes, customLinksRes, vendorsRes, certificatesRes, documentsRes] = - await Promise.all([ - serverApi.get('/v1/trust-portal/settings'), - serverApi.get('/v1/trust-portal/custom-links?organizationId=' + orgId), - serverApi.get('/v1/trust-portal/vendors?all=true'), - serverApi.post('/v1/trust-portal/compliance-resources/list', { - organizationId: orgId, - }), - serverApi.post('/v1/trust-portal/documents/list', { - organizationId: orgId, - }), - ]); + const [ + settingsRes, + customLinksRes, + vendorsRes, + certificatesRes, + documentsRes, + customFrameworksRes, + ] = await Promise.all([ + serverApi.get('/v1/trust-portal/settings'), + serverApi.get('/v1/trust-portal/custom-links?organizationId=' + orgId), + serverApi.get('/v1/trust-portal/vendors?all=true'), + serverApi.post('/v1/trust-portal/compliance-resources/list', { + organizationId: orgId, + }), + serverApi.post('/v1/trust-portal/documents/list', { + organizationId: orgId, + }), + serverApi.get('/v1/trust-portal/custom-frameworks'), + ]); const settings = settingsRes.data as any; const isTrustConfigured = settings?.isConfigured ?? true; - const customLinks = Array.isArray(customLinksRes.data) - ? customLinksRes.data - : []; + const customLinks = Array.isArray(customLinksRes.data) ? customLinksRes.data : []; const vendors = Array.isArray(vendorsRes.data) ? vendorsRes.data : []; - const certificateResources = Array.isArray(certificatesRes.data) - ? certificatesRes.data - : []; + const certificateResources = Array.isArray(certificatesRes.data) ? certificatesRes.data : []; const documents = Array.isArray(documentsRes.data) ? documentsRes.data : []; + const customFrameworks = Array.isArray(customFrameworksRes.data) ? customFrameworksRes.data : []; // Map compliance resources to fileName props const API_FRAMEWORK_TO_PROP: Record = { @@ -69,9 +69,7 @@ export default async function TrustPage({ }; for (const resource of certificateResources) { - const propKey = resource?.framework - ? API_FRAMEWORK_TO_PROP[resource.framework] - : undefined; + const propKey = resource?.framework ? API_FRAMEWORK_TO_PROP[resource.framework] : undefined; if (propKey) { certificateFiles[propKey] = resource.fileName ?? null; } @@ -158,6 +156,7 @@ export default async function TrustPage({ }} customLinks={customLinks} vendors={vendors} + customFrameworks={customFrameworks} faviconUrl={settings?.faviconUrl ?? null} /> diff --git a/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/ComplianceFramework.tsx b/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/ComplianceFramework.tsx new file mode 100644 index 0000000000..b57cb88349 --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/ComplianceFramework.tsx @@ -0,0 +1,478 @@ +'use client'; + +import { + Button, + Card, + CardContent, + CardDescription, + CardHeader, + CardTitle, + Select, + SelectContent, + SelectItem, + SelectTrigger, + Switch, + Tooltip, + TooltipContent, + TooltipTrigger, +} from '@trycompai/design-system'; +import { CertificateCheck, Download, Upload, View } from '@trycompai/design-system/icons'; +import { useRef, useState } from 'react'; +import { toast } from 'sonner'; +import { + CCPA, + CCPAInProgress, + GDPR, + GDPRInProgress, + HIPAA, + HIPAAInProgress, + ISO27001, + ISO27001InProgress, + ISO42001, + ISO42001InProgress, + ISO9001, + ISO9001InProgress, + NEN7510, + NEN7510InProgress, + PCIDSS, + PCIDSSInProgress, + PIPEDA, + PIPEDAInProgress, + SOC2Type1, + SOC2Type1InProgress, + SOC2Type2, + SOC2Type2InProgress, + SOC3, + SOC3InProgress, +} from './logos'; + +export function ComplianceFrameworkLogo({ + title, + status, + enabled, +}: { + title: string; + status: string; + enabled: boolean; +}) { + const isInProgress = status === 'in_progress'; + let LogoComponent: React.ElementType | null = null; + + if (title === 'ISO 27001') { + LogoComponent = enabled && isInProgress ? ISO27001InProgress : ISO27001; + } else if (title === 'ISO 42001') { + LogoComponent = enabled && isInProgress ? ISO42001InProgress : ISO42001; + } else if (title === 'GDPR') { + LogoComponent = enabled && isInProgress ? GDPRInProgress : GDPR; + } else if (title === 'HIPAA') { + LogoComponent = enabled && isInProgress ? HIPAAInProgress : HIPAA; + } else if (title === 'SOC 2 Type 1') { + LogoComponent = enabled && isInProgress ? SOC2Type1InProgress : SOC2Type1; + } else if (title === 'SOC 2 Type 2') { + LogoComponent = enabled && isInProgress ? SOC2Type2InProgress : SOC2Type2; + } else if (title === 'CCPA') { + LogoComponent = enabled && isInProgress ? CCPAInProgress : CCPA; + } else if (title === 'PCI DSS') { + LogoComponent = enabled && isInProgress ? PCIDSSInProgress : PCIDSS; + } else if (title === 'PIPEDA') { + LogoComponent = enabled && isInProgress ? PIPEDAInProgress : PIPEDA; + } else if (title === 'NEN 7510') { + LogoComponent = enabled && isInProgress ? NEN7510InProgress : NEN7510; + } else if (title === 'ISO 9001') { + LogoComponent = enabled && isInProgress ? ISO9001InProgress : ISO9001; + } else if (title === 'SOC 3') { + LogoComponent = enabled && isInProgress ? SOC3InProgress : SOC3; + } else { + LogoComponent = null; + } + + if (LogoComponent) { + return ( +
+ +
+ ); + } + + // Custom frameworks have no built-in SVG logo — fall back to an initials + // avatar (same idea as the main Frameworks page). + return ( +
+ {getFrameworkInitials(title)} +
+ ); +} + +function getFrameworkInitials(title: string): string { + const words = title.trim().split(/\s+/).filter(Boolean); + if (words.length === 0) return '?'; + if (words.length === 1) return words[0].slice(0, 2); + return (words[0][0] + words[1][0]).slice(0, 2); +} + +// Extracted component for compliance frameworks to reduce repetition and improve readability. +// Exported so the custom-frameworks section can reuse the exact same row UX. +export function ComplianceFramework({ + title, + description, + isEnabled: isEnabledProp, + status: statusProp, + onStatusChange, + onToggle, + fileName, + onFileUpload, + onFilePreview, + frameworkKey, + orgId, + disabled, +}: { + title: string; + description: string; + isEnabled: boolean; + status: string; + onStatusChange: (value: string) => Promise; + onToggle: (checked: boolean) => Promise; + fileName?: string | null; + onFileUpload?: (file: File, frameworkKey: string) => Promise; + onFilePreview?: (frameworkKey: string) => Promise; + frameworkKey: string; + orgId: string; + disabled?: boolean; +}) { + const [isEnabled, setIsEnabled] = useState(isEnabledProp); + const [status, setStatus] = useState(statusProp); + const [isUploading, setIsUploading] = useState(false); + const [isDragging, setIsDragging] = useState(false); + const fileInputRef = useRef(null); + const dragCounterRef = useRef(0); + + const processFile = async (file: File) => { + if (file.type !== 'application/pdf' && !file.name.toLowerCase().endsWith('.pdf')) { + toast.error('Please upload a PDF file'); + return; + } + + const MAX_FILE_SIZE = 100 * 1024 * 1024; + if (file.size > MAX_FILE_SIZE) { + toast.error('File size must be less than 100MB'); + return; + } + + if (onFileUpload) { + setIsUploading(true); + try { + await onFileUpload(file, frameworkKey); + toast.success('Certificate uploaded successfully'); + if (fileInputRef.current) { + fileInputRef.current.value = ''; + } + } catch (error) { + const message = error instanceof Error ? error.message : 'Failed to upload certificate'; + toast.error(message); + console.error('File upload error:', error); + } finally { + setIsUploading(false); + } + } + }; + + const handleDragEnter = (e: React.DragEvent) => { + e.preventDefault(); + e.stopPropagation(); + dragCounterRef.current++; + if (e.dataTransfer.items && e.dataTransfer.items.length > 0) { + setIsDragging(true); + } + }; + + const handleDragLeave = (e: React.DragEvent) => { + e.preventDefault(); + e.stopPropagation(); + dragCounterRef.current--; + if (dragCounterRef.current === 0) { + setIsDragging(false); + } + }; + + const handleDragOver = (e: React.DragEvent) => { + e.preventDefault(); + e.stopPropagation(); + }; + + const handleDrop = async (e: React.DragEvent) => { + e.preventDefault(); + e.stopPropagation(); + setIsDragging(false); + dragCounterRef.current = 0; + + const files = e.dataTransfer.files; + if (files && files.length > 0) { + await processFile(files[0]); + } + }; + + return ( + <> + + +
+
+ +
+
+ {title} +
+ {description} +
+
+
+ +
+ +
+
+ {isEnabled ? ( + + ) : ( +
+ Disabled +
+ )} +
+
+ { + setIsEnabled(checked); + try { + await onToggle(checked); + } catch { + setIsEnabled(!checked); + } + }} + /> +
+
+ + {/* File Upload Section - Only show when status is "compliant" */} + {isEnabled && status === 'compliant' && ( +
+ { + const file = e.target.files?.[0]; + if (file) { + await processFile(file); + } + }} + disabled={isUploading || disabled} + /> + + {/* Section Header */} +

Compliance Certificate

+ + {/* Certificate Content */} + {fileName ? ( + /* File Uploaded State */ +
+
+
+ +
+
+

{fileName}

+

Certificate uploaded

+
+ {onFilePreview && ( + + { + try { + await onFilePreview(frameworkKey); + } catch (error) { + const message = + error instanceof Error + ? error.message + : 'Failed to preview certificate'; + toast.error(message); + } + }} + className="text-xs font-medium text-primary hover:text-primary/80 hover:underline transition-colors flex items-center gap-1" + /> + } + > + + View + + Open certificate in new tab + + )} +
+ + {/* Action Bar */} +
+ + fileInputRef.current?.click()} + disabled={isUploading || disabled} + iconLeft={} + /> + } + > + Replace + + Replace current certificate (PDF) + + + {onFilePreview && ( + + { + try { + await onFilePreview(frameworkKey); + } catch (error) { + const message = + error instanceof Error + ? error.message + : 'Failed to download certificate'; + toast.error(message); + } + }} + iconLeft={} + /> + } + > + Download + + Download certificate + + )} +
+
+ ) : ( + /* Empty State - Drop zone matching uploaded state height (122px) */ +
!isUploading && !disabled && fileInputRef.current?.click()} + className={` + relative rounded-lg bg-muted/40 border border-border/50 p-4 cursor-pointer + h-[122px] flex items-center + transition-all duration-200 ease-in-out + ${isDragging ? 'border-primary bg-primary/5' : ''} + ${isUploading ? 'opacity-50 cursor-not-allowed' : ''} + `} + > +
+
+ +
+
+

+ {isDragging ? 'Drop your certificate here' : 'Drag & drop certificate'} +

+

+ or click to browse • PDF only, max 100MB +

+
+
+ + {isUploading && ( +
+
+
+ Uploading... +
+
+ )} +
+ )} +
+ )} + + + + ); +} diff --git a/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/CustomFrameworksSection.test.tsx b/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/CustomFrameworksSection.test.tsx new file mode 100644 index 0000000000..be292ebb54 --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/CustomFrameworksSection.test.tsx @@ -0,0 +1,87 @@ +import type { TrustCustomFrameworkItem } from '@/hooks/use-trust-portal-settings'; +import { render, screen } from '@testing-library/react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { CustomFrameworksSection } from './CustomFrameworksSection'; + +const updateCustomFramework = vi.fn(); +const uploadCustomComplianceResource = vi.fn(); +const getCustomComplianceResourceUrl = vi.fn(); + +vi.mock('@/hooks/use-trust-portal-settings', () => ({ + useTrustPortalSettings: () => ({ + updateCustomFramework, + uploadCustomComplianceResource, + getCustomComplianceResourceUrl, + }), +})); + +vi.mock('sonner', () => ({ + toast: { success: vi.fn(), error: vi.fn() }, +})); + +const frameworks: TrustCustomFrameworkItem[] = [ + { + customFrameworkId: 'cfrm_a', + name: 'Acme Internal Standard', + description: 'Our internal security standard', + enabled: true, + status: 'compliant', + hasCertificate: false, + certificateFileName: null, + }, + { + customFrameworkId: 'cfrm_b', + name: 'HR Security Base', + description: 'HR controls', + enabled: false, + status: 'started', + hasCertificate: false, + certificateFileName: null, + }, +]; + +describe('CustomFrameworksSection', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('renders a card per custom framework', () => { + render( + , + ); + + expect(screen.getByText('Acme Internal Standard')).toBeInTheDocument(); + expect(screen.getByText('HR Security Base')).toBeInTheDocument(); + }); + + it('reflects each framework status (enabled shows status, disabled shows Disabled)', () => { + render( + , + ); + + // Enabled + compliant framework shows its status; disabled one shows "Disabled". + expect(screen.getByText('Compliant')).toBeInTheDocument(); + expect(screen.getByText('Disabled')).toBeInTheDocument(); + }); + + it('shows an empty state when the org has no custom frameworks', () => { + render(); + + expect(screen.getByText('No custom frameworks yet.')).toBeInTheDocument(); + }); + + it('disables the toggle for read-only users', () => { + render( + , + ); + + // The design-system Switch (Base UI) marks disabled via aria-disabled. + const switches = screen.getAllByRole('switch'); + expect(switches.length).toBeGreaterThan(0); + switches.forEach((toggle) => expect(toggle).toHaveAttribute('aria-disabled', 'true')); + }); +}); diff --git a/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/CustomFrameworksSection.tsx b/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/CustomFrameworksSection.tsx new file mode 100644 index 0000000000..56631efbb9 --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/CustomFrameworksSection.tsx @@ -0,0 +1,160 @@ +'use client'; + +import { + TrustCustomFrameworkItem, + useTrustPortalSettings, +} from '@/hooks/use-trust-portal-settings'; +import { useState } from 'react'; +import { toast } from 'sonner'; +import { ComplianceFramework } from './ComplianceFramework'; + +type FrameworkStatus = 'started' | 'in_progress' | 'compliant'; + +async function convertFileToBase64(file: File): Promise { + const buffer = await file.arrayBuffer(); + let binary = ''; + const bytes = new Uint8Array(buffer); + const chunkSize = 0x8000; + for (let i = 0; i < bytes.length; i += chunkSize) { + binary += String.fromCharCode(...bytes.subarray(i, i + chunkSize)); + } + return btoa(binary); +} + +/** + * Lets an org choose which of its custom frameworks appear on the public Trust + * Portal, with the same toggle / status / certificate UX as native frameworks + * (reuses the ComplianceFramework row). Custom frameworks render with an + * initials avatar instead of a built-in SVG logo. + */ +export function CustomFrameworksSection({ + orgId, + canUpdate, + initialCustomFrameworks, +}: { + orgId: string; + canUpdate: boolean; + initialCustomFrameworks: TrustCustomFrameworkItem[]; +}) { + const { updateCustomFramework, uploadCustomComplianceResource, getCustomComplianceResourceUrl } = + useTrustPortalSettings(); + + const [frameworks, setFrameworks] = useState(initialCustomFrameworks); + + // Nothing to show until the org authors a custom framework — keep the section + // out of the way rather than rendering an empty grid. + if (frameworks.length === 0) { + return ( +
+
+

Custom Frameworks

+

+ Frameworks you create under Frameworks → Add Custom Framework will appear here, ready to + publish on your Trust Portal. +

+
+
+ No custom frameworks yet. +
+
+ ); + } + + const updateFileName = (customFrameworkId: string, fileName: string) => { + setFrameworks((prev) => + prev.map((framework) => + framework.customFrameworkId === customFrameworkId + ? { ...framework, certificateFileName: fileName, hasCertificate: true } + : framework, + ), + ); + }; + + const handleToggle = async (customFrameworkId: string, checked: boolean) => { + await updateCustomFramework({ customFrameworkId, enabled: checked }); + setFrameworks((prev) => + prev.map((framework) => + framework.customFrameworkId === customFrameworkId + ? { ...framework, enabled: checked } + : framework, + ), + ); + }; + + const handleStatusChange = async (customFrameworkId: string, status: FrameworkStatus) => { + await updateCustomFramework({ customFrameworkId, status }); + setFrameworks((prev) => + prev.map((framework) => + framework.customFrameworkId === customFrameworkId ? { ...framework, status } : framework, + ), + ); + }; + + const handleFileUpload = async (file: File, customFrameworkId: string) => { + const fileData = await convertFileToBase64(file); + const payload = await uploadCustomComplianceResource( + orgId, + customFrameworkId, + file.name, + file.type || 'application/pdf', + fileData, + ); + updateFileName(customFrameworkId, payload.fileName); + }; + + const handleFilePreview = async (customFrameworkId: string) => { + const payload = await getCustomComplianceResourceUrl(orgId, customFrameworkId); + window.open(payload.signedUrl, '_blank', 'noopener,noreferrer'); + }; + + return ( +
+
+

Custom Frameworks

+

+ Display your organization's custom frameworks alongside the standard ones. +

+
+
+ {frameworks.map((framework) => ( + { + const status = value as FrameworkStatus; + try { + await handleStatusChange(framework.customFrameworkId, status); + toast.success(`${framework.name} status updated`); + } catch (error) { + toast.error(`Failed to update ${framework.name} status`, { + description: error instanceof Error ? error.message : undefined, + }); + throw error; + } + }} + onToggle={async (checked) => { + try { + await handleToggle(framework.customFrameworkId, checked); + toast.success(`${framework.name} updated`); + } catch (error) { + toast.error(`Failed to update ${framework.name}`, { + description: error instanceof Error ? error.message : undefined, + }); + throw error; + } + }} + onFileUpload={handleFileUpload} + onFilePreview={handleFilePreview} + /> + ))} +
+
+ ); +} diff --git a/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/TrustPortalSwitch.test.tsx b/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/TrustPortalSwitch.test.tsx index 90c2e71e89..90dd66695b 100644 --- a/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/TrustPortalSwitch.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/TrustPortalSwitch.test.tsx @@ -244,6 +244,7 @@ describe('TrustPortalSwitch permission gating', () => { }, customLinks: [], vendors: [], + customFrameworks: [], }; beforeEach(() => { diff --git a/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/TrustPortalSwitch.tsx b/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/TrustPortalSwitch.tsx index dff2d249a3..da3d045ada 100644 --- a/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/TrustPortalSwitch.tsx +++ b/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/TrustPortalSwitch.tsx @@ -1,61 +1,18 @@ 'use client'; import { usePermissions } from '@/hooks/use-permissions'; +import type { TrustCustomFrameworkItem } from '@/hooks/use-trust-portal-settings'; import { useTrustPortalSettings } from '@/hooks/use-trust-portal-settings'; import { useDebounce } from '@/hooks/useDebounce'; import { zodResolver } from '@hookform/resolvers/zod'; -import { - Button, - Card, - CardContent, - CardDescription, - CardHeader, - CardTitle, - Select, - SelectContent, - SelectItem, - SelectTrigger, - Switch, - Tabs, - TabsContent, - TabsList, - TabsTrigger, - Tooltip, - TooltipContent, - TooltipTrigger, -} from '@trycompai/design-system'; -import { CertificateCheck, Download, Upload, View } from '@trycompai/design-system/icons'; +import { Tabs, TabsContent, TabsList, TabsTrigger } from '@trycompai/design-system'; import { Form } from '@trycompai/ui/form'; import { useCallback, useEffect, useRef, useState } from 'react'; import { useForm } from 'react-hook-form'; import { toast } from 'sonner'; import { z } from 'zod'; -import { - CCPA, - CCPAInProgress, - GDPR, - GDPRInProgress, - HIPAA, - HIPAAInProgress, - ISO27001, - ISO27001InProgress, - ISO42001, - ISO42001InProgress, - ISO9001, - ISO9001InProgress, - NEN7510, - NEN7510InProgress, - PCIDSS, - PCIDSSInProgress, - PIPEDA, - PIPEDAInProgress, - SOC2Type1, - SOC2Type1InProgress, - SOC2Type2, - SOC2Type2InProgress, - SOC3, - SOC3InProgress, -} from './logos'; +import { ComplianceFramework } from './ComplianceFramework'; +import { CustomFrameworksSection } from './CustomFrameworksSection'; import { TrustPortalAdditionalDocumentsSection, type TrustPortalDocument, @@ -211,6 +168,7 @@ export function TrustPortalSwitch({ overview, customLinks, vendors, + customFrameworks, faviconUrl, }: { enabled: boolean; @@ -261,6 +219,7 @@ export function TrustPortalSwitch({ overview: TrustOverviewData; customLinks: TrustCustomLink[]; vendors: TrustVendor[]; + customFrameworks: TrustCustomFrameworkItem[]; faviconUrl?: string | null; }) { const { hasPermission } = usePermissions(); @@ -1008,6 +967,12 @@ export function TrustPortalSwitch({ disabled={!canUpdate} />
+ +
@@ -1065,420 +1030,3 @@ export function TrustPortalSwitch({ ); } - -function ComplianceFrameworkLogo({ - title, - status, - enabled, -}: { - title: string; - status: string; - enabled: boolean; -}) { - const isInProgress = status === 'in_progress'; - let LogoComponent: React.ElementType | null = null; - - if (title === 'ISO 27001') { - LogoComponent = enabled && isInProgress ? ISO27001InProgress : ISO27001; - } else if (title === 'ISO 42001') { - LogoComponent = enabled && isInProgress ? ISO42001InProgress : ISO42001; - } else if (title === 'GDPR') { - LogoComponent = enabled && isInProgress ? GDPRInProgress : GDPR; - } else if (title === 'HIPAA') { - LogoComponent = enabled && isInProgress ? HIPAAInProgress : HIPAA; - } else if (title === 'SOC 2 Type 1') { - LogoComponent = enabled && isInProgress ? SOC2Type1InProgress : SOC2Type1; - } else if (title === 'SOC 2 Type 2') { - LogoComponent = enabled && isInProgress ? SOC2Type2InProgress : SOC2Type2; - } else if (title === 'CCPA') { - LogoComponent = enabled && isInProgress ? CCPAInProgress : CCPA; - } else if (title === 'PCI DSS') { - LogoComponent = enabled && isInProgress ? PCIDSSInProgress : PCIDSS; - } else if (title === 'PIPEDA') { - LogoComponent = enabled && isInProgress ? PIPEDAInProgress : PIPEDA; - } else if (title === 'NEN 7510') { - LogoComponent = enabled && isInProgress ? NEN7510InProgress : NEN7510; - } else if (title === 'ISO 9001') { - LogoComponent = enabled && isInProgress ? ISO9001InProgress : ISO9001; - } else if (title === 'SOC 3') { - LogoComponent = enabled && isInProgress ? SOC3InProgress : SOC3; - } else { - LogoComponent = null; - } - - if (LogoComponent) { - return ( -
- -
- ); - } - - return null; -} - -// Extracted component for compliance frameworks to reduce repetition and improve readability -function ComplianceFramework({ - title, - description, - isEnabled: isEnabledProp, - status: statusProp, - onStatusChange, - onToggle, - fileName, - onFileUpload, - onFilePreview, - frameworkKey, - orgId, - disabled, -}: { - title: string; - description: string; - isEnabled: boolean; - status: string; - onStatusChange: (value: string) => Promise; - onToggle: (checked: boolean) => Promise; - fileName?: string | null; - onFileUpload?: (file: File, frameworkKey: string) => Promise; - onFilePreview?: (frameworkKey: string) => Promise; - frameworkKey: string; - orgId: string; - disabled?: boolean; -}) { - const [isEnabled, setIsEnabled] = useState(isEnabledProp); - const [status, setStatus] = useState(statusProp); - const [isUploading, setIsUploading] = useState(false); - const [isDragging, setIsDragging] = useState(false); - const fileInputRef = useRef(null); - const dragCounterRef = useRef(0); - - const processFile = async (file: File) => { - if (file.type !== 'application/pdf' && !file.name.toLowerCase().endsWith('.pdf')) { - toast.error('Please upload a PDF file'); - return; - } - - const MAX_FILE_SIZE = 100 * 1024 * 1024; - if (file.size > MAX_FILE_SIZE) { - toast.error('File size must be less than 100MB'); - return; - } - - if (onFileUpload) { - setIsUploading(true); - try { - await onFileUpload(file, frameworkKey); - toast.success('Certificate uploaded successfully'); - if (fileInputRef.current) { - fileInputRef.current.value = ''; - } - } catch (error) { - const message = error instanceof Error ? error.message : 'Failed to upload certificate'; - toast.error(message); - console.error('File upload error:', error); - } finally { - setIsUploading(false); - } - } - }; - - const handleDragEnter = (e: React.DragEvent) => { - e.preventDefault(); - e.stopPropagation(); - dragCounterRef.current++; - if (e.dataTransfer.items && e.dataTransfer.items.length > 0) { - setIsDragging(true); - } - }; - - const handleDragLeave = (e: React.DragEvent) => { - e.preventDefault(); - e.stopPropagation(); - dragCounterRef.current--; - if (dragCounterRef.current === 0) { - setIsDragging(false); - } - }; - - const handleDragOver = (e: React.DragEvent) => { - e.preventDefault(); - e.stopPropagation(); - }; - - const handleDrop = async (e: React.DragEvent) => { - e.preventDefault(); - e.stopPropagation(); - setIsDragging(false); - dragCounterRef.current = 0; - - const files = e.dataTransfer.files; - if (files && files.length > 0) { - await processFile(files[0]); - } - }; - - return ( - <> - - -
-
- -
-
- {title} -
- {description} -
-
-
- -
- -
-
- {isEnabled ? ( - - ) : ( -
- Disabled -
- )} -
-
- { - setIsEnabled(checked); - try { - await onToggle(checked); - } catch { - setIsEnabled(!checked); - } - }} - /> -
-
- - {/* File Upload Section - Only show when status is "compliant" */} - {isEnabled && status === 'compliant' && ( -
- { - const file = e.target.files?.[0]; - if (file) { - await processFile(file); - } - }} - disabled={isUploading || disabled} - /> - - {/* Section Header */} -

Compliance Certificate

- - {/* Certificate Content */} - {fileName ? ( - /* File Uploaded State */ -
-
-
- -
-
-

{fileName}

-

Certificate uploaded

-
- {onFilePreview && ( - - { - try { - await onFilePreview(frameworkKey); - } catch (error) { - const message = - error instanceof Error - ? error.message - : 'Failed to preview certificate'; - toast.error(message); - } - }} - className="text-xs font-medium text-primary hover:text-primary/80 hover:underline transition-colors flex items-center gap-1" - /> - } - > - - View - - Open certificate in new tab - - )} -
- - {/* Action Bar */} -
- - fileInputRef.current?.click()} - disabled={isUploading || disabled} - iconLeft={} - /> - } - > - Replace - - Replace current certificate (PDF) - - - {onFilePreview && ( - - { - try { - await onFilePreview(frameworkKey); - } catch (error) { - const message = - error instanceof Error - ? error.message - : 'Failed to download certificate'; - toast.error(message); - } - }} - iconLeft={} - /> - } - > - Download - - Download certificate - - )} -
-
- ) : ( - /* Empty State - Drop zone matching uploaded state height (122px) */ -
!isUploading && !disabled && fileInputRef.current?.click()} - className={` - relative rounded-lg bg-muted/40 border border-border/50 p-4 cursor-pointer - h-[122px] flex items-center - transition-all duration-200 ease-in-out - ${isDragging ? 'border-primary bg-primary/5' : ''} - ${isUploading ? 'opacity-50 cursor-not-allowed' : ''} - `} - > -
-
- -
-
-

- {isDragging ? 'Drop your certificate here' : 'Drag & drop certificate'} -

-

- or click to browse • PDF only, max 100MB -

-
-
- - {isUploading && ( -
-
-
- Uploading... -
-
- )} -
- )} -
- )} - - - - ); -} diff --git a/apps/app/src/hooks/use-trust-portal-settings.ts b/apps/app/src/hooks/use-trust-portal-settings.ts index 7f53cf978e..d6b9f618e3 100644 --- a/apps/app/src/hooks/use-trust-portal-settings.ts +++ b/apps/app/src/hooks/use-trust-portal-settings.ts @@ -14,7 +14,8 @@ interface FrameworkSettingsData { } interface ComplianceResourceResponse { - framework: string; + framework: string | null; + customFrameworkId?: string | null; fileName: string; fileSize: number; updatedAt: string; @@ -42,6 +43,22 @@ interface VendorTrustSettingsData { showOnTrustPortal: boolean; } +export interface TrustCustomFrameworkItem { + customFrameworkId: string; + name: string; + description: string; + enabled: boolean; + status: 'started' | 'in_progress' | 'compliant'; + hasCertificate: boolean; + certificateFileName: string | null; +} + +interface UpdateCustomFrameworkData { + customFrameworkId: string; + enabled?: boolean; + status?: 'started' | 'in_progress' | 'compliant'; +} + export function useTrustPortalSettings() { const api = useApi(); @@ -56,10 +73,7 @@ export function useTrustPortalSettings() { const updateFrameworkSettings = useCallback( async (data: FrameworkSettingsData) => { - const response = await api.put( - '/v1/trust-portal/settings/frameworks', - data, - ); + const response = await api.put('/v1/trust-portal/settings/frameworks', data); if (response.error) throw new Error(response.error); return response.data; }, @@ -98,6 +112,55 @@ export function useTrustPortalSettings() { [api], ); + const listCustomFrameworks = useCallback(async () => { + const response = await api.get( + '/v1/trust-portal/custom-frameworks', + ); + if (response.error) throw new Error(response.error); + return Array.isArray(response.data) ? response.data : []; + }, [api]); + + const updateCustomFramework = useCallback( + async (data: UpdateCustomFrameworkData) => { + const response = await api.put('/v1/trust-portal/custom-frameworks', data); + if (response.error) throw new Error(response.error); + return response.data; + }, + [api], + ); + + const uploadCustomComplianceResource = useCallback( + async ( + organizationId: string, + customFrameworkId: string, + fileName: string, + fileType: string, + fileData: string, + ) => { + const response = await api.post( + '/v1/trust-portal/compliance-resources/upload', + { organizationId, customFrameworkId, fileName, fileType, fileData }, + ); + if (response.error) throw new Error(response.error); + if (!response.data) throw new Error('Unexpected API response'); + return response.data; + }, + [api], + ); + + const getCustomComplianceResourceUrl = useCallback( + async (organizationId: string, customFrameworkId: string) => { + const response = await api.post( + '/v1/trust-portal/compliance-resources/signed-url', + { organizationId, customFrameworkId }, + ); + if (response.error) throw new Error(response.error); + if (!response.data?.signedUrl) throw new Error('Preview link unavailable'); + return response.data; + }, + [api], + ); + const saveOverview = useCallback( async (data: OverviewData) => { const response = await api.post('/v1/trust-portal/overview', data); @@ -109,10 +172,7 @@ export function useTrustPortalSettings() { const updateVendorTrustSettings = useCallback( async (vendorId: string, data: VendorTrustSettingsData) => { - const response = await api.post( - `/v1/trust-portal/vendors/${vendorId}/trust-settings`, - data, - ); + const response = await api.post(`/v1/trust-portal/vendors/${vendorId}/trust-settings`, data); if (response.error) throw new Error(response.error); return response.data; }, @@ -121,10 +181,7 @@ export function useTrustPortalSettings() { const updateAllowedDomains = useCallback( async (domains: string[]) => { - const response = await api.put( - '/v1/trust-portal/settings/allowed-domains', - { domains }, - ); + const response = await api.put('/v1/trust-portal/settings/allowed-domains', { domains }); if (response.error) throw new Error(response.error); return response.data; }, @@ -133,10 +190,11 @@ export function useTrustPortalSettings() { const uploadFavicon = useCallback( async (fileName: string, fileType: string, fileData: string) => { - const response = await api.post( - '/v1/trust-portal/favicon', - { fileName, fileType, fileData }, - ); + const response = await api.post('/v1/trust-portal/favicon', { + fileName, + fileType, + fileData, + }); if (response.error) throw new Error(response.error); return response.data; }, @@ -184,6 +242,10 @@ export function useTrustPortalSettings() { updateFrameworkSettings, uploadComplianceResource, getComplianceResourceUrl, + listCustomFrameworks, + updateCustomFramework, + uploadCustomComplianceResource, + getCustomComplianceResourceUrl, saveOverview, updateVendorTrustSettings, updateAllowedDomains, diff --git a/packages/db/prisma/migrations/20260610120000_trust_custom_frameworks/migration.sql b/packages/db/prisma/migrations/20260610120000_trust_custom_frameworks/migration.sql new file mode 100644 index 0000000000..f0a6421f11 --- /dev/null +++ b/packages/db/prisma/migrations/20260610120000_trust_custom_frameworks/migration.sql @@ -0,0 +1,55 @@ +-- Custom frameworks on the Trust Portal. +-- +-- A TrustResource (compliance certificate PDF) can now belong to EITHER a native +-- framework (TrustFramework enum) OR an org-authored CustomFramework. New table +-- TrustCustomFramework records which custom frameworks an org displays on its +-- public portal, mirroring the per-framework enabled/status columns that native +-- frameworks store on `Trust`. + +-- AlterTable: TrustResource — framework becomes optional; add custom framework link. +ALTER TABLE "TrustResource" ALTER COLUMN "framework" DROP NOT NULL; +ALTER TABLE "TrustResource" ADD COLUMN "customFrameworkId" TEXT; + +-- CreateTable +CREATE TABLE "TrustCustomFramework" ( + "id" TEXT NOT NULL DEFAULT generate_prefixed_cuid('tcf'::text), + "organizationId" TEXT NOT NULL, + "customFrameworkId" TEXT NOT NULL, + "enabled" BOOLEAN NOT NULL DEFAULT true, + "status" "FrameworkStatus" NOT NULL DEFAULT 'started', + "createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, + "updatedAt" TIMESTAMP(3) NOT NULL, + + CONSTRAINT "TrustCustomFramework_pkey" PRIMARY KEY ("id") +); + +-- CreateIndex +CREATE UNIQUE INDEX "TrustResource_organizationId_customFrameworkId_key" ON "TrustResource"("organizationId", "customFrameworkId"); + +-- CreateIndex +CREATE INDEX "TrustResource_customFrameworkId_idx" ON "TrustResource"("customFrameworkId"); + +-- CreateIndex +CREATE INDEX "TrustCustomFramework_organizationId_idx" ON "TrustCustomFramework"("organizationId"); + +-- CreateIndex +CREATE INDEX "TrustCustomFramework_customFrameworkId_idx" ON "TrustCustomFramework"("customFrameworkId"); + +-- CreateIndex +CREATE UNIQUE INDEX "TrustCustomFramework_organizationId_customFrameworkId_key" ON "TrustCustomFramework"("organizationId", "customFrameworkId"); + +-- AddForeignKey +ALTER TABLE "TrustResource" ADD CONSTRAINT "TrustResource_customFrameworkId_organizationId_fkey" FOREIGN KEY ("customFrameworkId", "organizationId") REFERENCES "CustomFramework"("id", "organizationId") ON DELETE CASCADE ON UPDATE CASCADE; + +-- AddForeignKey +ALTER TABLE "TrustCustomFramework" ADD CONSTRAINT "TrustCustomFramework_organizationId_fkey" FOREIGN KEY ("organizationId") REFERENCES "Organization"("id") ON DELETE CASCADE ON UPDATE CASCADE; + +-- AddForeignKey +ALTER TABLE "TrustCustomFramework" ADD CONSTRAINT "TrustCustomFramework_customFrameworkId_organizationId_fkey" FOREIGN KEY ("customFrameworkId", "organizationId") REFERENCES "CustomFramework"("id", "organizationId") ON DELETE CASCADE ON UPDATE CASCADE; + +-- CHECK constraint: a TrustResource belongs to exactly one of a native framework +-- or a custom framework. Existing rows (framework set, customFrameworkId NULL) +-- satisfy this, so the constraint is safe to add in place. +ALTER TABLE "TrustResource" + ADD CONSTRAINT "TrustResource_one_framework_check" + CHECK (("framework" IS NOT NULL)::int + ("customFrameworkId" IS NOT NULL)::int = 1); diff --git a/packages/db/prisma/schema/custom-framework.prisma b/packages/db/prisma/schema/custom-framework.prisma index fbd7241a7e..f3a0388a9b 100644 --- a/packages/db/prisma/schema/custom-framework.prisma +++ b/packages/db/prisma/schema/custom-framework.prisma @@ -10,6 +10,10 @@ model CustomFramework { requirements CustomRequirement[] instances FrameworkInstance[] + // Trust Portal display selection + per-framework certificate uploads. + trustSelections TrustCustomFramework[] + trustResources TrustResource[] + createdAt DateTime @default(now()) updatedAt DateTime @default(now()) @updatedAt diff --git a/packages/db/prisma/schema/organization.prisma b/packages/db/prisma/schema/organization.prisma index 079cf79052..67c0aa37c3 100644 --- a/packages/db/prisma/schema/organization.prisma +++ b/packages/db/prisma/schema/organization.prisma @@ -55,6 +55,7 @@ model Organization { trustDocuments TrustDocument[] trustResources TrustResource[] @relation("OrganizationTrustResources") trustCustomLinks TrustCustomLink[] + trustCustomFrameworks TrustCustomFramework[] knowledgeBaseDocuments KnowledgeBaseDocument[] questionnaires Questionnaire[] securityQuestionnaireManualAnswers SecurityQuestionnaireManualAnswer[] diff --git a/packages/db/prisma/schema/trust.prisma b/packages/db/prisma/schema/trust.prisma index 0f34261861..56ab096fe7 100644 --- a/packages/db/prisma/schema/trust.prisma +++ b/packages/db/prisma/schema/trust.prisma @@ -83,18 +83,62 @@ enum TrustFramework { } model TrustResource { - id String @id @default(dbgenerated("generate_prefixed_cuid('tcr'::text)")) + id String @id @default(dbgenerated("generate_prefixed_cuid('tcr'::text)")) organizationId String - organization Organization @relation("OrganizationTrustResources", fields: [organizationId], references: [id], onDelete: Cascade) - framework TrustFramework - s3Key String - fileName String - fileSize Int - createdAt DateTime @default(now()) - updatedAt DateTime @updatedAt + organization Organization @relation("OrganizationTrustResources", fields: [organizationId], references: [id], onDelete: Cascade) + + // A trust resource (compliance certificate PDF) belongs to EITHER a native + // framework (TrustFramework enum) OR an org-authored custom framework. Exactly + // one of `framework` / `customFrameworkId` is set — enforced by a DB CHECK + // constraint (TrustResource_one_framework_check). + framework TrustFramework? + + customFrameworkId String? + // Composite FK onto (id, organizationId) so a custom-framework certificate can + // only reference a CustomFramework in its own org. Enforced at the DB level. + customFramework CustomFramework? @relation(fields: [customFrameworkId, organizationId], references: [id, organizationId], onDelete: Cascade) + + s3Key String + fileName String + fileSize Int + createdAt DateTime @default(now()) + updatedAt DateTime @updatedAt + // Postgres treats NULLs as distinct in unique indexes, so the inactive parent + // column doesn't collide across rows (native rows have customFrameworkId NULL, + // custom rows have framework NULL). @@unique([organizationId, framework]) + @@unique([organizationId, customFrameworkId]) + @@index([organizationId]) + @@index([customFrameworkId]) +} + +/// Links an org-authored CustomFramework to the public Trust Portal so it can be +/// displayed alongside native frameworks. Native frameworks store their +/// enabled/status as columns on `Trust`; custom frameworks are dynamic per-org +/// rows, so their portal selection lives here instead. +model TrustCustomFramework { + id String @id @default(dbgenerated("generate_prefixed_cuid('tcf'::text)")) + + organizationId String + organization Organization @relation(fields: [organizationId], references: [id], onDelete: Cascade) + + customFrameworkId String + // Composite FK onto (id, organizationId) so a selection can only reference a + // CustomFramework in its own org. Enforced at the DB level. + customFramework CustomFramework @relation(fields: [customFrameworkId, organizationId], references: [id, organizationId], onDelete: Cascade) + + // Whether the framework is shown on the public portal, and its displayed + // status — mirrors the per-framework enabled + *_status columns on `Trust`. + enabled Boolean @default(true) + status FrameworkStatus @default(started) + + createdAt DateTime @default(now()) + updatedAt DateTime @updatedAt + + @@unique([organizationId, customFrameworkId]) @@index([organizationId]) + @@index([customFrameworkId]) } model TrustAccessRequest { From 91135b2adb80851c309e3023c8beee69063e7be4 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Wed, 10 Jun 2026 17:03:38 -0400 Subject: [PATCH 03/18] fix(integration-platform): surface real read errors in azure/gcp checks (CS-534 part 1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ports the read-failure error-visibility pattern to the Azure and GCP checks, closing the fully-swallowed error sites found in the CS-532 audit — '.catch(() => null)' paths that discarded the real error and asserted a specific missing permission the code never verified: - new src/manifests/http-read-failure.ts: toHttpReadFailure classifies ctx.fetch/ctx.post errors (.status 401/403 or PERMISSION_DENIED/ AuthorizationFailed/Forbidden in the message = denied; else transient) - azure sql (firewall rules + auditing), monitor (alerts + diagnostic settings), mysql/postgresql flexible (server configs), entra-id (role definition resolution), shared armListAllOrFail: real error now lands in finding evidence + logs; 'Grant X' remediation only when the error actually was an authorization failure - gcp iam-primitive-roles (bare 'catch { return null }' sites now capture and surface the error), storage/cloud-sql/vpc project-level catches: remediation gated on the error class - fixes the factually wrong storage getIamPolicy role hint: legacyBucketReader/Viewer do not contain storage.buckets.getIamPolicy; the hint now names roles/iam.securityReviewer Pass/fail verdicts and control flow are unchanged — only remediation/ description/evidence/log text. ctx.fetch already retries 429/5xx, so no retry changes were needed. 187 tests pass (12 new). Co-Authored-By: Claude Fable 5 --- .../__tests__/http-read-failure.test.ts | 36 +++++++++++ .../checks/__tests__/azure-checks.test.ts | 64 ++++++++++++++++++- .../src/manifests/azure/checks/entra-id.ts | 21 +++++- .../src/manifests/azure/checks/monitor.ts | 37 ++++++++--- .../manifests/azure/checks/mysql-flexible.ts | 33 ++++++++-- .../azure/checks/postgresql-flexible.ts | 28 ++++++-- .../src/manifests/azure/checks/shared.ts | 10 ++- .../src/manifests/azure/checks/sql.ts | 42 +++++++++--- .../gcp/checks/__tests__/gcp-checks.test.ts | 64 ++++++++++++++++++- .../manifests/gcp/checks/cloud-sql-backups.ts | 13 +++- .../src/manifests/gcp/checks/cloud-sql-ssl.ts | 13 +++- .../gcp/checks/iam-primitive-roles.ts | 36 ++++++++--- .../gcp/checks/storage-public-access.ts | 26 ++++++-- .../gcp/checks/vpc-open-firewalls.ts | 13 +++- .../src/manifests/http-read-failure.ts | 30 +++++++++ 15 files changed, 402 insertions(+), 64 deletions(-) create mode 100644 packages/integration-platform/src/manifests/__tests__/http-read-failure.test.ts create mode 100644 packages/integration-platform/src/manifests/http-read-failure.ts diff --git a/packages/integration-platform/src/manifests/__tests__/http-read-failure.test.ts b/packages/integration-platform/src/manifests/__tests__/http-read-failure.test.ts new file mode 100644 index 0000000000..f7fe676273 --- /dev/null +++ b/packages/integration-platform/src/manifests/__tests__/http-read-failure.test.ts @@ -0,0 +1,36 @@ +import { describe, expect, it } from 'bun:test'; +import { toHttpReadFailure } from '../http-read-failure'; + +const httpError = (status: number, message: string) => { + const err = new Error(message); + (err as Error & { status: number }).status = status; + return err; +}; + +describe('toHttpReadFailure — ctx.fetch error classification', () => { + it('classifies 403 and 401 as denied', () => { + expect(toHttpReadFailure(httpError(403, 'HTTP 403: Forbidden')).denied).toBe(true); + expect(toHttpReadFailure(httpError(401, 'HTTP 401: Unauthorized')).denied).toBe(true); + }); + + it('classifies provider permission phrases as denied even without a status', () => { + expect(toHttpReadFailure(new Error('PERMISSION_DENIED: missing role')).denied).toBe(true); + expect(toHttpReadFailure(new Error('AuthorizationFailed: no access')).denied).toBe(true); + }); + + it('treats 5xx/429/network errors as transient (not denied)', () => { + expect(toHttpReadFailure(httpError(500, 'HTTP 500: Internal Server Error')).denied).toBe(false); + expect(toHttpReadFailure(httpError(429, 'HTTP 429: Too Many Requests')).denied).toBe(false); + expect(toHttpReadFailure(new Error('socket hang up')).denied).toBe(false); + }); + + it('preserves the error message and never sets regionDisabled', () => { + const f = toHttpReadFailure(httpError(503, 'HTTP 503: Service Unavailable - upstream')); + expect(f.error).toBe('HTTP 503: Service Unavailable - upstream'); + expect(f.regionDisabled).toBe(false); + }); + + it('stringifies non-Error throwables', () => { + expect(toHttpReadFailure('boom')).toMatchObject({ error: 'boom', denied: false }); + }); +}); diff --git a/packages/integration-platform/src/manifests/azure/checks/__tests__/azure-checks.test.ts b/packages/integration-platform/src/manifests/azure/checks/__tests__/azure-checks.test.ts index d71656d4c6..d80ea6ac32 100644 --- a/packages/integration-platform/src/manifests/azure/checks/__tests__/azure-checks.test.ts +++ b/packages/integration-platform/src/manifests/azure/checks/__tests__/azure-checks.test.ts @@ -27,7 +27,12 @@ import { interface Captured { passed: string[]; - failed: Array<{ title: string; severity: string }>; + failed: Array<{ + title: string; + severity: string; + remediation?: string; + evidence?: Record; + }>; } async function run( @@ -48,7 +53,13 @@ async function run( warn: () => {}, error: () => {}, pass: (r) => passed.push(r.title), - fail: (r) => failed.push({ title: r.title, severity: r.severity }), + fail: (r) => + failed.push({ + title: r.title, + severity: r.severity, + remediation: r.remediation, + evidence: r.evidence, + }), fetch: (async (url: string): Promise => fetchFn(url) as T) as CheckContext['fetch'], post: (async () => ({})) as CheckContext['post'], put: (async () => ({})) as CheckContext['put'], @@ -610,3 +621,52 @@ describe('Azure PostgreSQL Flexible Server TLS check', () => { expect(failed).toHaveLength(0); }); }); + +describe('Azure read-failure remediation gating', () => { + const httpError = (status: number, message: string) => { + const err = new Error(message); + (err as Error & { status: number }).status = status; + return err; + }; + const SERVER = { + id: '/subscriptions/sub-1/resourceGroups/rg/providers/Microsoft.Sql/servers/s1', + name: 's1', + properties: {}, + }; + + it('sql auditing: transient read says re-run; denied keeps the grant hint', async () => { + const transient = await run(sqlAuditingCheck, (url: string) => { + if (url.includes('/providers/Microsoft.Sql/servers?')) return { value: [SERVER] }; + if (url.includes('/auditingSettings/')) throw httpError(500, 'HTTP 500: Internal Server Error'); + return {}; + }); + const f = transient.failed.find((x) => x.title.includes('Could not read SQL auditing settings')); + expect(f).toBeDefined(); + expect(f!.remediation).toMatch(/re-run/i); + expect(f!.remediation).not.toContain('auditingSettings/read'); + expect(f!.evidence).toMatchObject({ readError: 'HTTP 500: Internal Server Error' }); + + const denied = await run(sqlAuditingCheck, (url: string) => { + if (url.includes('/providers/Microsoft.Sql/servers?')) return { value: [SERVER] }; + if (url.includes('/auditingSettings/')) throw httpError(403, 'HTTP 403: Forbidden - AuthorizationFailed'); + return {}; + }); + const fd = denied.failed.find((x) => x.title.includes('Could not read SQL auditing settings')); + expect(fd).toBeDefined(); + expect(fd!.remediation).toContain('Microsoft.Sql/servers/auditingSettings/read'); + expect(fd!.evidence).toMatchObject({ readError: 'HTTP 403: Forbidden - AuthorizationFailed' }); + }); + + it('monitor: unreadable alerts carry readError and a gated (transient) remediation', async () => { + const out = await run(monitorLoggingAlertingCheck, (url: string) => { + if (url.includes('activityLogAlerts')) throw httpError(500, 'HTTP 500: boom'); + if (url.includes('diagnosticSettings')) return { value: [] }; + return {}; + }); + const f = out.failed.find((x) => x.title === 'Could not read activity log alerts'); + expect(f).toBeDefined(); + expect(f!.remediation).toMatch(/re-run/i); + expect(f!.remediation).not.toContain('Monitoring Reader'); + expect(f!.evidence).toMatchObject({ readError: 'HTTP 500: boom' }); + }); +}); diff --git a/packages/integration-platform/src/manifests/azure/checks/entra-id.ts b/packages/integration-platform/src/manifests/azure/checks/entra-id.ts index 7af63eef6e..f65aa2fbe4 100644 --- a/packages/integration-platform/src/manifests/azure/checks/entra-id.ts +++ b/packages/integration-platform/src/manifests/azure/checks/entra-id.ts @@ -1,5 +1,11 @@ import { TASK_TEMPLATES } from '../../../task-mappings'; import type { CheckContext, IntegrationCheck } from '../../../types'; +import { + combineReadFailures, + remediationForReadFailure, + toHttpReadFailure, + type ReadFailure, +} from '../../http-read-failure'; import { ARM_BASE, armListAllOrFail, resolveAzureSubscriptionId } from './shared'; interface RoleAssignment { @@ -92,6 +98,7 @@ export const rbacLeastPrivilegeCheck: IntegrationCheck = { // Resolve any missing definition directly so privileged principals aren't // undercounted. Cache by id to avoid refetching shared definitions. const resolvedDefs = new Map(); + const resolveFailures: ReadFailure[] = []; const resolveDef = async ( roleDefinitionId: string, ): Promise => { @@ -107,9 +114,11 @@ export const rbacLeastPrivilegeCheck: IntegrationCheck = { } return null; } catch (err) { + const failure = toHttpReadFailure(err); + resolveFailures.push(failure); ctx.warn('Failed to resolve Azure role definition for assignment', { roleDefinitionId, - error: err instanceof Error ? err.message : String(err), + error: failure.error, }); return null; } @@ -138,9 +147,15 @@ export const rbacLeastPrivilegeCheck: IntegrationCheck = { resourceType: 'azure-subscription', resourceId: sub, severity: 'medium', - remediation: + remediation: remediationForReadFailure( + combineReadFailures(resolveFailures), 'Ensure the integration principal has read access to all role definitions in scope (including management-group and resource-group scopes), then re-run the check.', - evidence: { unresolvedAssignments }, + ), + evidence: { + unresolvedAssignments, + // first few real errors so the cause is visible without log digging + readErrors: resolveFailures.slice(0, 3).map((f) => f.error), + }, }); } diff --git a/packages/integration-platform/src/manifests/azure/checks/monitor.ts b/packages/integration-platform/src/manifests/azure/checks/monitor.ts index 515a6a113c..b368afb8eb 100644 --- a/packages/integration-platform/src/manifests/azure/checks/monitor.ts +++ b/packages/integration-platform/src/manifests/azure/checks/monitor.ts @@ -1,5 +1,10 @@ import { TASK_TEMPLATES } from '../../../task-mappings'; import type { CheckContext, IntegrationCheck } from '../../../types'; +import { + remediationForReadFailure, + toHttpReadFailure, + type ReadFailure, +} from '../../http-read-failure'; import { ARM_BASE, armListAll, resolveAzureSubscriptionId } from './shared'; interface ActivityLogAlert { @@ -38,10 +43,15 @@ export const monitorLoggingAlertingCheck: IntegrationCheck = { if (!sub) return; let evaluated = false; + let alertsReadFailure: ReadFailure | undefined; const alerts = await armListAll( ctx, `${ARM_BASE}/subscriptions/${sub}/providers/Microsoft.Insights/activityLogAlerts?api-version=2020-10-01`, - ).catch(() => null); + ).catch((err) => { + alertsReadFailure = toHttpReadFailure(err); + ctx.log(`Azure Monitor: activity log alerts read failed — ${alertsReadFailure.error}`); + return null; + }); if (alerts !== null) { evaluated = true; const ops = new Set(); @@ -80,22 +90,28 @@ export const monitorLoggingAlertingCheck: IntegrationCheck = { // shared Monitoring task on incomplete evaluation. ctx.fail({ title: 'Could not read activity log alerts', - description: - 'Activity log alert coverage could not be read, so alerting was not verified.', + description: `Activity log alert coverage could not be read${alertsReadFailure ? ` (${alertsReadFailure.error})` : ''}, so alerting was not verified.`, resourceType: 'azure-subscription', resourceId: sub, severity: 'medium', - remediation: + remediation: remediationForReadFailure( + alertsReadFailure, 'Grant Monitoring Reader (or Reader) so activity log alerts can be evaluated.', - evidence: {}, + ), + evidence: alertsReadFailure ? { readError: alertsReadFailure.error } : {}, }); } + let diagReadFailure: ReadFailure | undefined; const diag = await ctx .fetch<{ value?: DiagnosticSetting[] }>( `${ARM_BASE}/subscriptions/${sub}/providers/Microsoft.Insights/diagnosticSettings?api-version=2021-05-01-preview`, ) - .catch(() => null); + .catch((err) => { + diagReadFailure = toHttpReadFailure(err); + ctx.log(`Azure Monitor: diagnostic settings read failed — ${diagReadFailure.error}`); + return null; + }); if (diag !== null) { evaluated = true; const settings = diag.value ?? []; @@ -149,14 +165,15 @@ export const monitorLoggingAlertingCheck: IntegrationCheck = { // pass the shared Monitoring task on incomplete evaluation. ctx.fail({ title: 'Could not read diagnostic settings', - description: - 'Subscription diagnostic settings could not be read, so log export was not verified.', + description: `Subscription diagnostic settings could not be read${diagReadFailure ? ` (${diagReadFailure.error})` : ''}, so log export was not verified.`, resourceType: 'azure-subscription', resourceId: sub, severity: 'medium', - remediation: + remediation: remediationForReadFailure( + diagReadFailure, 'Grant Monitoring Reader (or Reader) so diagnostic settings can be evaluated.', - evidence: {}, + ), + evidence: diagReadFailure ? { readError: diagReadFailure.error } : {}, }); } diff --git a/packages/integration-platform/src/manifests/azure/checks/mysql-flexible.ts b/packages/integration-platform/src/manifests/azure/checks/mysql-flexible.ts index d077f83e16..2fb7af9b18 100644 --- a/packages/integration-platform/src/manifests/azure/checks/mysql-flexible.ts +++ b/packages/integration-platform/src/manifests/azure/checks/mysql-flexible.ts @@ -1,5 +1,11 @@ import { TASK_TEMPLATES } from '../../../task-mappings'; import type { CheckContext, IntegrationCheck } from '../../../types'; +import { + combineReadFailures, + remediationForReadFailure, + toHttpReadFailure, + type ReadFailure, +} from '../../http-read-failure'; import { ARM_BASE, armListAllOrFail, resolveAzureSubscriptionId } from './shared'; // Pinned stable api-version for Azure Database for MySQL Flexible Server. @@ -79,12 +85,18 @@ async function readConfigValue( ctx: CheckContext, serverId: string, name: string, + onReadError?: (failure: ReadFailure) => void, ): Promise { const res = await ctx .fetch( `${ARM_BASE}${serverId}/configurations/${name}?api-version=${MYSQL_API_VERSION}`, ) - .catch(() => null); + .catch((err) => { + const failure = toHttpReadFailure(err); + ctx.log(`MySQL ${serverId}: could not read ${name} — ${failure.error}`); + onReadError?.(failure); + return null; + }); const value = res?.properties?.value; return typeof value === 'string' ? value : null; } @@ -110,25 +122,36 @@ export const mysqlFlexibleTlsCheck: IntegrationCheck = { if (!servers) return; if (servers.length === 0) return; for (const s of servers) { + const readFailures: ReadFailure[] = []; + const collect = (failure: ReadFailure) => readFailures.push(failure); const requireSecureTransport = await readConfigValue( ctx, s.id, 'require_secure_transport', + collect, ); - const tlsVersion = await readConfigValue(ctx, s.id, 'tls_version'); + const tlsVersion = await readConfigValue(ctx, s.id, 'tls_version', collect); if (requireSecureTransport === null || tlsVersion === null) { // Couldn't read the TLS parameters — fail explicitly so the TLS task // isn't falsely satisfied by other servers/checks that read cleanly. + const combined = combineReadFailures(readFailures); ctx.fail({ title: `Could not verify MySQL TLS settings: ${s.name}`, - description: `Unable to read the TLS server parameters for MySQL flexible server "${s.name}", so TLS enforcement cannot be verified.`, + description: `Unable to read the TLS server parameters for MySQL flexible server "${s.name}"${combined ? ` (${combined.error})` : ''}, so TLS enforcement cannot be verified.`, resourceType: 'azure-mysql-flexible-server', resourceId: s.id, severity: 'medium', - remediation: + remediation: remediationForReadFailure( + combined, 'Grant read access to server configurations (Microsoft.DBforMySQL/flexibleServers/configurations/read), then re-run the check.', - evidence: { server: s.name, requireSecureTransport, tlsVersion }, + ), + evidence: { + server: s.name, + requireSecureTransport, + tlsVersion, + ...(combined ? { readError: combined.error } : {}), + }, }); continue; } diff --git a/packages/integration-platform/src/manifests/azure/checks/postgresql-flexible.ts b/packages/integration-platform/src/manifests/azure/checks/postgresql-flexible.ts index 8377f5511a..321a64068f 100644 --- a/packages/integration-platform/src/manifests/azure/checks/postgresql-flexible.ts +++ b/packages/integration-platform/src/manifests/azure/checks/postgresql-flexible.ts @@ -1,5 +1,11 @@ import { TASK_TEMPLATES } from '../../../task-mappings'; import type { CheckContext, IntegrationCheck } from '../../../types'; +import { + combineReadFailures, + remediationForReadFailure, + toHttpReadFailure, + type ReadFailure, +} from '../../http-read-failure'; import { ARM_BASE, armListAllOrFail, resolveAzureSubscriptionId } from './shared'; // Pinned stable api-version for Azure Database for PostgreSQL Flexible Server. @@ -84,15 +90,17 @@ async function readConfig( ctx: CheckContext, serverId: string, name: string, -): Promise<{ ok: true; value: string } | { ok: false }> { +): Promise<{ ok: true; value: string } | { ok: false; failure: ReadFailure }> { try { const res = await ctx.fetch( `${ARM_BASE}${serverId}/configurations/${name}?api-version=${POSTGRES_API_VERSION}`, ); const value = res?.properties?.value; return { ok: true, value: typeof value === 'string' ? value : '' }; - } catch { - return { ok: false }; + } catch (err) { + const failure = toHttpReadFailure(err); + ctx.log(`PostgreSQL ${serverId}: could not read ${name} — ${failure.error}`); + return { ok: false, failure }; } } @@ -126,15 +134,23 @@ export const postgresqlFlexibleTlsCheck: IntegrationCheck = { // back as an empty string on a SUCCESSFUL response, which evaluatePgTls // treats as a compliant TLS 1.2 floor; that is distinct from a failed read.) if (!requireSecure.ok || !sslMin.ok) { + const combined = combineReadFailures( + [requireSecure, sslMin].flatMap((r) => (r.ok ? [] : [r.failure])), + ); ctx.fail({ title: `Could not verify PostgreSQL TLS settings: ${s.name}`, - description: `Unable to read the TLS server parameters for PostgreSQL flexible server "${s.name}", so TLS enforcement cannot be verified.`, + description: `Unable to read the TLS server parameters for PostgreSQL flexible server "${s.name}"${combined ? ` (${combined.error})` : ''}, so TLS enforcement cannot be verified.`, resourceType: 'azure-postgresql-flexible-server', resourceId: s.id, severity: 'medium', - remediation: + remediation: remediationForReadFailure( + combined, 'Grant read access to server configurations (Microsoft.DBforPostgreSQL/flexibleServers/configurations/read), then re-run the check.', - evidence: { server: s.name }, + ), + evidence: { + server: s.name, + ...(combined ? { readError: combined.error } : {}), + }, }); continue; } diff --git a/packages/integration-platform/src/manifests/azure/checks/shared.ts b/packages/integration-platform/src/manifests/azure/checks/shared.ts index d309d7db2a..9f39743710 100644 --- a/packages/integration-platform/src/manifests/azure/checks/shared.ts +++ b/packages/integration-platform/src/manifests/azure/checks/shared.ts @@ -1,4 +1,5 @@ import type { CheckContext } from '../../../types'; +import { remediationForReadFailure, toHttpReadFailure } from '../../http-read-failure'; const ARM = 'https://management.azure.com'; @@ -79,15 +80,18 @@ export async function armListAllOrFail( try { return await armListAll(ctx, url); } catch (err) { + const failure = toHttpReadFailure(err); ctx.fail({ title: `Could not verify ${opts.what}`, - description: `${opts.what} could not be listed from Azure, so this check is unverified.`, + description: `${opts.what} could not be listed from Azure (${failure.error}), so this check is unverified.`, resourceType: opts.resourceType, resourceId: opts.subscriptionId, severity: 'medium', - remediation: + remediation: remediationForReadFailure( + failure, 'Ensure the connection has Reader access to the subscription, then re-run the check.', - evidence: { error: err instanceof Error ? err.message : String(err) }, + ), + evidence: { readError: failure.error }, }); return null; } diff --git a/packages/integration-platform/src/manifests/azure/checks/sql.ts b/packages/integration-platform/src/manifests/azure/checks/sql.ts index 035c3d3a96..8e6e06edfe 100644 --- a/packages/integration-platform/src/manifests/azure/checks/sql.ts +++ b/packages/integration-platform/src/manifests/azure/checks/sql.ts @@ -1,5 +1,10 @@ import { TASK_TEMPLATES } from '../../../task-mappings'; import type { CheckContext, IntegrationCheck } from '../../../types'; +import { + remediationForReadFailure, + toHttpReadFailure, + type ReadFailure, +} from '../../http-read-failure'; import { ARM_BASE, armListAll, armListAllOrFail, resolveAzureSubscriptionId } from './shared'; interface SqlServer { @@ -93,10 +98,15 @@ export const sqlPublicAccessCheck: IntegrationCheck = { } // null = firewall read failed → do NOT treat as "no wide-open rules". + let rulesReadFailure: ReadFailure | undefined; const rules = await armListAll( ctx, `${ARM_BASE}${s.id}/firewallRules?api-version=2023-05-01-preview`, - ).catch(() => null); + ).catch((err) => { + rulesReadFailure = toHttpReadFailure(err); + ctx.log(`SQL ${s.name}: firewall rules read failed — ${rulesReadFailure.error}`); + return null; + }); if (rules) { const wideOpen = rules.find( @@ -146,13 +156,19 @@ export const sqlPublicAccessCheck: IntegrationCheck = { // satisfied by other servers passing. ctx.fail({ title: `Could not read SQL firewall rules: ${s.name}`, - description: `Unable to read firewall rules for SQL Server "${s.name}", so wide-open access cannot be ruled out.`, + description: `Unable to read firewall rules for SQL Server "${s.name}"${rulesReadFailure ? ` (${rulesReadFailure.error})` : ''}, so wide-open access cannot be ruled out.`, resourceType: 'azure-sql-server', resourceId: s.id, severity: 'medium', - remediation: + remediation: remediationForReadFailure( + rulesReadFailure, 'Grant read access to SQL firewall rules (Microsoft.Sql/servers/firewallRules/read) so public access can be verified.', - evidence: { server: s.name, publicNetworkAccess: s.properties?.publicNetworkAccess ?? null }, + ), + evidence: { + server: s.name, + publicNetworkAccess: s.properties?.publicNetworkAccess ?? null, + ...(rulesReadFailure ? { readError: rulesReadFailure.error } : {}), + }, }); } else { ctx.pass({ @@ -189,23 +205,33 @@ export const sqlAuditingCheck: IntegrationCheck = { if (!servers) return; if (servers.length === 0) return; for (const s of servers) { + let auditingReadFailure: ReadFailure | undefined; const auditing = await ctx .fetch( `${ARM_BASE}${s.id}/auditingSettings/default?api-version=2021-11-01`, ) - .catch(() => null); + .catch((err) => { + auditingReadFailure = toHttpReadFailure(err); + ctx.log(`SQL ${s.name}: auditing settings read failed — ${auditingReadFailure.error}`); + return null; + }); if (auditing === null) { // Couldn't read auditing settings — fail explicitly so the Monitoring // task isn't falsely passed by other servers that read successfully. ctx.fail({ title: `Could not read SQL auditing settings: ${s.name}`, - description: `Unable to read auditing settings for SQL Server "${s.name}", so auditing state cannot be verified.`, + description: `Unable to read auditing settings for SQL Server "${s.name}"${auditingReadFailure ? ` (${auditingReadFailure.error})` : ''}, so auditing state cannot be verified.`, resourceType: 'azure-sql-server', resourceId: s.id, severity: 'medium', - remediation: + remediation: remediationForReadFailure( + auditingReadFailure, 'Grant read access to SQL auditing settings (Microsoft.Sql/servers/auditingSettings/read) so auditing can be verified.', - evidence: { server: s.name }, + ), + evidence: { + server: s.name, + ...(auditingReadFailure ? { readError: auditingReadFailure.error } : {}), + }, }); continue; } diff --git a/packages/integration-platform/src/manifests/gcp/checks/__tests__/gcp-checks.test.ts b/packages/integration-platform/src/manifests/gcp/checks/__tests__/gcp-checks.test.ts index 56801f4182..22d8ac639c 100644 --- a/packages/integration-platform/src/manifests/gcp/checks/__tests__/gcp-checks.test.ts +++ b/packages/integration-platform/src/manifests/gcp/checks/__tests__/gcp-checks.test.ts @@ -12,7 +12,13 @@ import { vpcOpenFirewallsCheck } from '../vpc-open-firewalls'; interface Captured { passed: Array<{ resourceId: string; title: string }>; - failed: Array<{ resourceId: string; title: string; severity: string }>; + failed: Array<{ + resourceId: string; + title: string; + severity: string; + remediation?: string; + evidence?: Record; + }>; } async function runCheck( @@ -42,6 +48,8 @@ async function runCheck( resourceId: r.resourceId, title: r.title, severity: r.severity, + remediation: r.remediation, + evidence: r.evidence, }), fetch: (async (url: string): Promise => (opts.fetch ? opts.fetch(url) : {}) as T) as CheckContext['fetch'], @@ -62,6 +70,60 @@ async function runCheck( return { passed, failed }; } +describe('GCP read-failure remediation gating', () => { + const httpError = (status: number, message: string) => { + const err = new Error(message); + (err as Error & { status: number }).status = status; + return err; + }; + + it('iam: a 403 policy read keeps the grant remediation and carries the error', async () => { + const out = await runCheck(iamPrimitiveRolesCheck, { + post: () => { + throw httpError(403, 'HTTP 403: Forbidden - PERMISSION_DENIED'); + }, + }); + expect(out.failed).toHaveLength(1); + expect(out.failed[0]!.title).toMatch(/Could not verify IAM primitive roles/); + expect(out.failed[0]!.remediation).toContain('resourcemanager.projects.getIamPolicy'); + expect(out.failed[0]!.evidence).toMatchObject({ + error: 'HTTP 403: Forbidden - PERMISSION_DENIED', + }); + }); + + it('iam: a transient 500 policy read says re-run instead of claiming a missing permission', async () => { + const out = await runCheck(iamPrimitiveRolesCheck, { + post: () => { + throw httpError(500, 'HTTP 500: Internal Server Error'); + }, + }); + expect(out.failed).toHaveLength(1); + expect(out.failed[0]!.remediation).not.toContain('resourcemanager.projects.getIamPolicy'); + expect(out.failed[0]!.remediation).toMatch(/re-run/i); + expect(out.failed[0]!.evidence).toMatchObject({ + error: 'HTTP 500: Internal Server Error', + }); + }); + + it('storage: a transient bucket-list failure says re-run; a denied one keeps the grant hint', async () => { + const transient = await runCheck(storagePublicAccessCheck, { + fetch: () => { + throw httpError(503, 'HTTP 503: Service Unavailable'); + }, + }); + expect(transient.failed[0]!.title).toMatch(/Could not verify Cloud Storage/); + expect(transient.failed[0]!.remediation).toMatch(/re-run/i); + expect(transient.failed[0]!.remediation).not.toContain('storage.buckets.list'); + + const denied = await runCheck(storagePublicAccessCheck, { + fetch: () => { + throw httpError(403, 'HTTP 403: Forbidden'); + }, + }); + expect(denied.failed[0]!.remediation).toContain('storage.buckets.list'); + }); +}); + describe('GCP IAM primitive roles check', () => { it('fails on roles/owner binding (high)', async () => { const { passed, failed } = await runCheck(iamPrimitiveRolesCheck, { diff --git a/packages/integration-platform/src/manifests/gcp/checks/cloud-sql-backups.ts b/packages/integration-platform/src/manifests/gcp/checks/cloud-sql-backups.ts index 2e6ce89b8b..b3de68f40f 100644 --- a/packages/integration-platform/src/manifests/gcp/checks/cloud-sql-backups.ts +++ b/packages/integration-platform/src/manifests/gcp/checks/cloud-sql-backups.ts @@ -1,5 +1,9 @@ import { TASK_TEMPLATES } from '../../../task-mappings'; import type { CheckContext, IntegrationCheck } from '../../../types'; +import { + remediationForReadFailure, + toHttpReadFailure, +} from '../../http-read-failure'; import { gcpListItems, resolveGcpProjectIds } from './shared'; interface SqlInstance { @@ -74,15 +78,18 @@ export const cloudSqlBackupsCheck: IntegrationCheck = { } catch (err) { // Unverified project → emit a finding, not a warn-and-skip, so an // all-projects-failed run doesn't leave the task stale (silent pass). + const failure = toHttpReadFailure(err); ctx.fail({ title: `Could not verify Cloud SQL backups: ${projectId}`, - description: `Cloud SQL instances for project "${projectId}" could not be listed, so backup configuration is unverified.`, + description: `Cloud SQL instances for project "${projectId}" could not be listed (${failure.error}), so backup configuration is unverified.`, resourceType: 'gcp-project', resourceId: projectId, severity: 'medium', - remediation: + remediation: remediationForReadFailure( + failure, 'Grant cloudsql.instances.list (e.g. roles/cloudsql.viewer) to the connection for this project, then re-run.', - evidence: { projectId, error: err instanceof Error ? err.message : String(err) }, + ), + evidence: { projectId, error: failure.error }, }); continue; } diff --git a/packages/integration-platform/src/manifests/gcp/checks/cloud-sql-ssl.ts b/packages/integration-platform/src/manifests/gcp/checks/cloud-sql-ssl.ts index a195d1dd1b..3e1c3dbbe1 100644 --- a/packages/integration-platform/src/manifests/gcp/checks/cloud-sql-ssl.ts +++ b/packages/integration-platform/src/manifests/gcp/checks/cloud-sql-ssl.ts @@ -1,5 +1,9 @@ import { TASK_TEMPLATES } from '../../../task-mappings'; import type { CheckContext, IntegrationCheck } from '../../../types'; +import { + remediationForReadFailure, + toHttpReadFailure, +} from '../../http-read-failure'; import { gcpListItems, resolveGcpProjectIds } from './shared'; interface SqlInstance { @@ -84,15 +88,18 @@ export const cloudSqlSslCheck: IntegrationCheck = { } catch (err) { // Unverified project → emit a finding, not a warn-and-skip, so an // all-projects-failed run doesn't leave the task stale (silent pass). + const failure = toHttpReadFailure(err); ctx.fail({ title: `Could not verify Cloud SQL SSL: ${projectId}`, - description: `Cloud SQL instances for project "${projectId}" could not be listed, so SSL/TLS enforcement is unverified.`, + description: `Cloud SQL instances for project "${projectId}" could not be listed (${failure.error}), so SSL/TLS enforcement is unverified.`, resourceType: 'gcp-project', resourceId: projectId, severity: 'medium', - remediation: + remediation: remediationForReadFailure( + failure, 'Grant cloudsql.instances.list (e.g. roles/cloudsql.viewer) to the connection for this project, then re-run.', - evidence: { projectId, error: err instanceof Error ? err.message : String(err) }, + ), + evidence: { projectId, error: failure.error }, }); continue; } diff --git a/packages/integration-platform/src/manifests/gcp/checks/iam-primitive-roles.ts b/packages/integration-platform/src/manifests/gcp/checks/iam-primitive-roles.ts index 5a65da5ff7..acea3bd703 100644 --- a/packages/integration-platform/src/manifests/gcp/checks/iam-primitive-roles.ts +++ b/packages/integration-platform/src/manifests/gcp/checks/iam-primitive-roles.ts @@ -1,5 +1,10 @@ import { TASK_TEMPLATES } from '../../../task-mappings'; import type { CheckContext, FindingSeverity, IntegrationCheck } from '../../../types'; +import { + remediationForReadFailure, + toHttpReadFailure, + type ReadFailure, +} from '../../http-read-failure'; import { resolveGcpProjectIds } from './shared'; /** Primitive roles grant broad, non-least-privilege access. */ @@ -17,6 +22,7 @@ interface IamBinding { async function getBindings( ctx: CheckContext, resourcePath: string, + onReadError?: (failure: ReadFailure) => void, ): Promise { try { const policy = await ctx.post<{ bindings?: IamBinding[] }>( @@ -24,7 +30,10 @@ async function getBindings( { options: { requestedPolicyVersion: 3 } }, ); return policy.bindings ?? []; - } catch { + } catch (err) { + const failure = toHttpReadFailure(err); + ctx.log(`GCP IAM: could not read policy for ${resourcePath} — ${failure.error}`); + onReadError?.(failure); return null; } } @@ -39,21 +48,21 @@ async function getBindings( function failUnverifiedProject( ctx: CheckContext, projectId: string, - error?: unknown, + failure?: ReadFailure, ): void { ctx.fail({ title: `Could not verify IAM primitive roles: ${projectId}`, - description: `IAM policy for project "${projectId}" could not be read, so primitive-role usage is unverified.`, + description: `IAM policy for project "${projectId}" could not be read${failure ? ` (${failure.error})` : ''}, so primitive-role usage is unverified.`, resourceType: 'gcp-project', resourceId: projectId, severity: 'medium', - remediation: + remediation: remediationForReadFailure( + failure, 'Grant resourcemanager.projects.getIamPolicy (e.g. roles/iam.securityReviewer) to the connection for this project, then re-run.', + ), evidence: { projectId, - ...(error !== undefined - ? { error: error instanceof Error ? error.message : String(error) } - : {}), + ...(failure ? { error: failure.error } : {}), }, }); } @@ -82,14 +91,18 @@ export const iamPrimitiveRolesCheck: IntegrationCheck = { for (const projectId of projectIds) { try { + let projectReadFailure: ReadFailure | undefined; const projectBindings = await getBindings( ctx, `v3/projects/${encodeURIComponent(projectId)}`, + (failure) => { + projectReadFailure = failure; + }, ); if (projectBindings === null) { // Couldn't read the project's own IAM policy (getBindings swallowed // the throw → null). Fail closed rather than silently skipping. - failUnverifiedProject(ctx, projectId); + failUnverifiedProject(ctx, projectId, projectReadFailure); continue; } @@ -120,8 +133,11 @@ export const iamPrimitiveRolesCheck: IntegrationCheck = { } scopes.push({ label: `${type} ${id}`, bindings }); } - } catch { + } catch (err) { hierarchyFullyEvaluated = false; + ctx.log( + `GCP IAM: could not resolve ancestry for "${projectId}" — ${toHttpReadFailure(err).error}`, + ); } let violations = 0; @@ -171,7 +187,7 @@ export const iamPrimitiveRolesCheck: IntegrationCheck = { // One project's API error must not abort the whole check — but it is // unverified, so emit a finding rather than warn-and-skip (an // all-projects-failed run would otherwise leave the task stale). - failUnverifiedProject(ctx, projectId, error); + failUnverifiedProject(ctx, projectId, toHttpReadFailure(error)); continue; } } diff --git a/packages/integration-platform/src/manifests/gcp/checks/storage-public-access.ts b/packages/integration-platform/src/manifests/gcp/checks/storage-public-access.ts index eefd6b6685..a6dc3374b0 100644 --- a/packages/integration-platform/src/manifests/gcp/checks/storage-public-access.ts +++ b/packages/integration-platform/src/manifests/gcp/checks/storage-public-access.ts @@ -1,5 +1,9 @@ import { TASK_TEMPLATES } from '../../../task-mappings'; import type { CheckContext, IntegrationCheck } from '../../../types'; +import { + remediationForReadFailure, + toHttpReadFailure, +} from '../../http-read-failure'; import { gcpListItems, resolveGcpProjectIds } from './shared'; interface Bucket { @@ -54,17 +58,20 @@ export const storagePublicAccessCheck: IntegrationCheck = { } catch (err) { // Unverified project → emit a finding, not a warn-and-skip, so an // all-projects-failed run doesn't leave the task stale (silent pass). + const failure = toHttpReadFailure(err); ctx.fail({ title: `Could not verify Cloud Storage: ${projectId}`, - description: `Buckets for project "${projectId}" could not be listed, so public access is unverified.`, + description: `Buckets for project "${projectId}" could not be listed (${failure.error}), so public access is unverified.`, resourceType: 'gcp-project', resourceId: projectId, severity: 'medium', - remediation: + remediation: remediationForReadFailure( + failure, 'Grant storage.buckets.list (e.g. roles/storage.legacyBucketReader or Viewer) to the connection for this project, then re-run.', + ), evidence: { projectId, - error: err instanceof Error ? err.message : String(err), + error: failure.error, }, }); } @@ -103,18 +110,23 @@ async function evaluateBucket( ); } catch (err) { // Couldn't read the policy → unverified, never a silent pass. + const failure = toHttpReadFailure(err); ctx.fail({ title: `Could not verify public access: ${bucket.name}`, - description: `Bucket "${bucket.name}" IAM policy could not be read, so public access is unverified.`, + description: `Bucket "${bucket.name}" IAM policy could not be read (${failure.error}), so public access is unverified.`, resourceType: 'gcp-storage-bucket', resourceId, severity: 'medium', - remediation: - 'Grant storage.buckets.getIamPolicy (e.g. roles/storage.legacyBucketReader or Viewer) to the connection, then re-run.', + // legacyBucketReader/Viewer do NOT contain storage.buckets.getIamPolicy — + // iam.securityReviewer is the documented read-only role that does. + remediation: remediationForReadFailure( + failure, + 'Grant storage.buckets.getIamPolicy (e.g. roles/iam.securityReviewer) to the connection, then re-run.', + ), evidence: { projectId, bucket: bucket.name, - error: err instanceof Error ? err.message : String(err), + error: failure.error, }, }); return; diff --git a/packages/integration-platform/src/manifests/gcp/checks/vpc-open-firewalls.ts b/packages/integration-platform/src/manifests/gcp/checks/vpc-open-firewalls.ts index 70d4e8440d..fe9560fbdb 100644 --- a/packages/integration-platform/src/manifests/gcp/checks/vpc-open-firewalls.ts +++ b/packages/integration-platform/src/manifests/gcp/checks/vpc-open-firewalls.ts @@ -1,5 +1,9 @@ import { TASK_TEMPLATES } from '../../../task-mappings'; import type { CheckContext, FindingSeverity, IntegrationCheck } from '../../../types'; +import { + remediationForReadFailure, + toHttpReadFailure, +} from '../../http-read-failure'; import { gcpListItems, portsCover, resolveGcpProjectIds } from './shared'; interface FirewallRule { @@ -113,17 +117,20 @@ export const vpcOpenFirewallsCheck: IntegrationCheck = { // A read failure for this project is unverified — emit a finding rather // than warn-and-skip, otherwise an all-projects-failed run emits no // outcomes and leaves the mapped task stale (a silent clean run). + const failure = toHttpReadFailure(err); ctx.fail({ title: `Could not verify VPC firewall rules: ${projectId}`, - description: `Firewall rules for project "${projectId}" could not be read, so internet exposure is unverified.`, + description: `Firewall rules for project "${projectId}" could not be read (${failure.error}), so internet exposure is unverified.`, resourceType: 'gcp-project', resourceId: projectId, severity: 'medium', - remediation: + remediation: remediationForReadFailure( + failure, 'Grant compute.firewalls.list (e.g. roles/compute.viewer) to the connection for this project, then re-run.', + ), evidence: { projectId, - error: err instanceof Error ? err.message : String(err), + error: failure.error, }, }); } diff --git a/packages/integration-platform/src/manifests/http-read-failure.ts b/packages/integration-platform/src/manifests/http-read-failure.ts new file mode 100644 index 0000000000..7294ca2acb --- /dev/null +++ b/packages/integration-platform/src/manifests/http-read-failure.ts @@ -0,0 +1,30 @@ +// HTTP flavor of the AWS read-failure classifier (the generic ReadFailure +// machinery lives in aws/checks/read-failure.ts where it shipped first). +// Used by the Azure and GCP checks, whose ctx.fetch/ctx.post errors are plain +// Errors carrying a `.status` number and an "HTTP : ..." message. +import { + combineReadFailures, + remediationForReadFailure, + type ReadFailure, +} from './aws/checks/read-failure'; + +export { combineReadFailures, remediationForReadFailure, type ReadFailure }; + +/** + * Classify a thrown ctx.fetch/ctx.post error so an "unverified" finding can + * tell a permissions problem ("grant X") apart from a transient one + * ("re-run") — asserting a missing permission for what was actually a + * transient failure sends customers on a wild-goose permissions audit. + * GCP returns 403 PERMISSION_DENIED; ARM returns 403 AuthorizationFailed. + */ +export function toHttpReadFailure(err: unknown): ReadFailure { + const error = + err instanceof Error ? err.message.slice(0, 300) : String(err).slice(0, 300); + const status = (err as { status?: number } | null)?.status; + const denied = + status === 401 || + status === 403 || + (err instanceof Error && + /PERMISSION_DENIED|AuthorizationFailed|Forbidden/i.test(err.message)); + return { error, denied, regionDisabled: false }; +} From ec5ba440965293de95de03b6ba104ca4d70e46fc Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Wed, 10 Jun 2026 17:05:51 -0400 Subject: [PATCH 04/18] fix(trust-portal): resync custom-framework state + mark response fields nullable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses cubic review: - CustomFrameworksSection: resync local state from initialCustomFrameworks via useEffect so refetched server data isn't ignored (mirrors the certificate-file resync in TrustPortalSwitch). - ComplianceResourceResponseDto: framework/customFrameworkId are always present but nullable, so use @ApiProperty({ nullable: true }) instead of @ApiPropertyOptional — accurate OpenAPI contract (null, not undefined). Co-Authored-By: Claude Fable 5 --- .../src/trust-portal/dto/compliance-resource.dto.ts | 6 ++++-- .../components/CustomFrameworksSection.tsx | 10 +++++++++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/apps/api/src/trust-portal/dto/compliance-resource.dto.ts b/apps/api/src/trust-portal/dto/compliance-resource.dto.ts index c31a51ad20..e8a606c555 100644 --- a/apps/api/src/trust-portal/dto/compliance-resource.dto.ts +++ b/apps/api/src/trust-portal/dto/compliance-resource.dto.ts @@ -57,14 +57,16 @@ export class UploadComplianceResourceDto extends ComplianceResourceBaseDto { export class ComplianceResourceSignedUrlDto extends ComplianceResourceBaseDto {} export class ComplianceResourceResponseDto { - @ApiPropertyOptional({ + // Always present in the response (one of the two is null), so these are + // required-but-nullable — not optional. + @ApiProperty({ enum: TrustFramework, description: 'Set for native-framework certificates; null for custom ones', nullable: true, }) framework!: TrustFramework | null; - @ApiPropertyOptional({ + @ApiProperty({ description: 'Set for custom-framework certificates; null for native ones', nullable: true, }) diff --git a/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/CustomFrameworksSection.tsx b/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/CustomFrameworksSection.tsx index 56631efbb9..e2a6d90bc9 100644 --- a/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/CustomFrameworksSection.tsx +++ b/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/CustomFrameworksSection.tsx @@ -4,7 +4,7 @@ import { TrustCustomFrameworkItem, useTrustPortalSettings, } from '@/hooks/use-trust-portal-settings'; -import { useState } from 'react'; +import { useEffect, useState } from 'react'; import { toast } from 'sonner'; import { ComplianceFramework } from './ComplianceFramework'; @@ -41,6 +41,14 @@ export function CustomFrameworksSection({ const [frameworks, setFrameworks] = useState(initialCustomFrameworks); + // Resync from the server when it refetches (e.g. after a new custom framework + // is created). The prop reference is stable across client-only re-renders, so + // this won't clobber in-session optimistic edits — it only fires on fresh + // server data. Mirrors the certificate-file resync in TrustPortalSwitch. + useEffect(() => { + setFrameworks(initialCustomFrameworks); + }, [initialCustomFrameworks]); + // Nothing to show until the org authors a custom framework — keep the section // out of the way rather than rendering an empty grid. if (frameworks.length === 0) { From 9912b9a54a220c33c62fdb8ebac453958a2dbdba Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Wed, 10 Jun 2026 17:39:01 -0400 Subject: [PATCH 05/18] feat(integration-platform): scan all enabled Azure subscriptions (CS-534 part 2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Azure evidence checks previously scanned only the FIRST Enabled subscription — silently skipping the rest in multi-subscription tenants (a compliance product reporting partial coverage as full coverage). - new subscription_ids multi-select variable with a subscription picker (mirrors the GCP project_ids precedent); explicit selection wins - resolveAzureSubscriptionIds: selected ids > ALL Enabled subscriptions > legacy subscription_id only when subscriptions cannot be listed (that variable is auto-saved by Cloud Tests detection — a cache, not a scope choice; Cloud Tests itself is untouched and keeps using it) - all 13 azure checks now loop subscriptions via extracted per- subscription helpers (bodies byte-identical; internal returns correctly skip only that subscription) - fan-out bounded at 50 subscriptions with a loud warning, and the entra-id role-definition cache is shared across subscriptions - scope failures now emit an explicit 'Could not verify … scope' finding on BOTH providers (azure subscriptions + gcp projects) instead of silently skipping every check - the 'Add another account' button is hidden for oauth2 providers: the OAuth callback reuses the existing connection, so a second connect silently merged into the first (gated in IntegrationProviderHero — the live component) Single-subscription tenants emit byte-identical findings (verified per check). 192 tests pass (5 new). Co-Authored-By: Claude Fable 5 --- .../components/IntegrationProviderHero.tsx | 29 +++-- .../checks/__tests__/azure-checks.test.ts | 93 +++++++++++++++ .../src/manifests/azure/checks/entra-id.ts | 36 ++++-- .../src/manifests/azure/checks/key-vault.ts | 52 +++++---- .../src/manifests/azure/checks/monitor.ts | 27 +++-- .../manifests/azure/checks/mysql-flexible.ts | 27 +++-- .../src/manifests/azure/checks/network.ts | 27 +++-- .../azure/checks/postgresql-flexible.ts | 27 +++-- .../src/manifests/azure/checks/shared.ts | 109 ++++++++++++++---- .../src/manifests/azure/checks/sql.ts | 73 +++++++----- .../src/manifests/azure/checks/storage.ts | 77 ++++++++----- .../src/manifests/azure/index.ts | 29 +++++ .../gcp/checks/__tests__/gcp-checks.test.ts | 17 +++ .../src/manifests/gcp/checks/shared.ts | 23 +++- 14 files changed, 473 insertions(+), 173 deletions(-) diff --git a/apps/app/src/app/(app)/[orgId]/integrations/[slug]/components/IntegrationProviderHero.tsx b/apps/app/src/app/(app)/[orgId]/integrations/[slug]/components/IntegrationProviderHero.tsx index bab3fe8218..a87adfa40d 100644 --- a/apps/app/src/app/(app)/[orgId]/integrations/[slug]/components/IntegrationProviderHero.tsx +++ b/apps/app/src/app/(app)/[orgId]/integrations/[slug]/components/IntegrationProviderHero.tsx @@ -138,17 +138,24 @@ export function IntegrationProviderHero({ />
)} -
- -
+ {/* The OAuth callback reuses the org's existing + connection, so a second OAuth connect silently + merges into the first — only offer "Add" where + the connect flow can actually create another. */} + {provider.supportsMultipleConnections && + provider.authType !== 'oauth2' && ( +
+ +
+ )}
diff --git a/packages/integration-platform/src/manifests/azure/checks/__tests__/azure-checks.test.ts b/packages/integration-platform/src/manifests/azure/checks/__tests__/azure-checks.test.ts index d80ea6ac32..1e2a3d330c 100644 --- a/packages/integration-platform/src/manifests/azure/checks/__tests__/azure-checks.test.ts +++ b/packages/integration-platform/src/manifests/azure/checks/__tests__/azure-checks.test.ts @@ -670,3 +670,96 @@ describe('Azure read-failure remediation gating', () => { expect(f!.evidence).toMatchObject({ readError: 'HTTP 500: boom' }); }); }); + +describe('Azure multi-subscription scanning', () => { + const SUBS = { + value: [ + { subscriptionId: 'sub-a', state: 'Enabled', displayName: 'A' }, + { subscriptionId: 'sub-b', state: 'Enabled', displayName: 'B' }, + { subscriptionId: 'sub-old', state: 'Disabled', displayName: 'Old' }, + ], + }; + const serverIn = (sub: string) => ({ + value: [ + { + id: `/subscriptions/${sub}/resourceGroups/rg/providers/Microsoft.Sql/servers/s-${sub}`, + name: `s-${sub}`, + properties: { minimalTlsVersion: '1.2' }, + }, + ], + }); + + it('scans every Enabled subscription when none are selected (Disabled excluded)', async () => { + const seen: string[] = []; + const out = await run( + sqlTlsCheck, + (url: string) => { + if (url.includes('/subscriptions?api-version')) return SUBS; + const m = url.match(/subscriptions\/(sub-\w+)\/providers\/Microsoft.Sql\/servers\?/); + if (m) { + seen.push(m[1]!); + return serverIn(m[1]!); + } + return {}; + }, + {}, + ); + expect(seen).toEqual(['sub-a', 'sub-b']); + expect(out.passed).toHaveLength(2); + expect(out.failed).toHaveLength(0); + }); + + it('scopes to the selected subscription_ids when set', async () => { + const seen: string[] = []; + const out = await run( + sqlTlsCheck, + (url: string) => { + const m = url.match(/subscriptions\/(sub-\w+)\/providers\/Microsoft.Sql\/servers\?/); + if (m) { + seen.push(m[1]!); + return serverIn(m[1]!); + } + return {}; + }, + { subscription_ids: ['sub-b'] }, + ); + expect(seen).toEqual(['sub-b']); + expect(out.passed).toHaveLength(1); + }); + + it('falls back to the legacy subscription_id when subscriptions cannot be listed', async () => { + const seen: string[] = []; + const out = await run( + sqlTlsCheck, + (url: string) => { + if (url.includes('/subscriptions?api-version')) { + throw new Error('HTTP 403: Forbidden'); + } + const m = url.match(/subscriptions\/(sub-\w+)\/providers\/Microsoft.Sql\/servers\?/); + if (m) { + seen.push(m[1]!); + return serverIn(m[1]!); + } + return {}; + }, + { subscription_id: 'sub-legacy' }, + ); + expect(seen).toEqual(['sub-legacy']); + expect(out.passed).toHaveLength(1); + expect(out.failed).toHaveLength(0); + }); + + it('emits an explicit scope finding when nothing is visible and no legacy value exists', async () => { + const out = await run( + sqlTlsCheck, + (url: string) => { + if (url.includes('/subscriptions?api-version')) return { value: [] }; + return {}; + }, + {}, + ); + expect(out.passed).toHaveLength(0); + expect(out.failed).toHaveLength(1); + expect(out.failed[0]!.title).toMatch(/Could not verify Azure subscription scope/); + }); +}); diff --git a/packages/integration-platform/src/manifests/azure/checks/entra-id.ts b/packages/integration-platform/src/manifests/azure/checks/entra-id.ts index f65aa2fbe4..1231b30662 100644 --- a/packages/integration-platform/src/manifests/azure/checks/entra-id.ts +++ b/packages/integration-platform/src/manifests/azure/checks/entra-id.ts @@ -6,7 +6,7 @@ import { toHttpReadFailure, type ReadFailure, } from '../../http-read-failure'; -import { ARM_BASE, armListAllOrFail, resolveAzureSubscriptionId } from './shared'; +import { ARM_BASE, armListAllOrFail, resolveAzureSubscriptionIds } from './shared'; interface RoleAssignment { properties: { roleDefinitionId: string; principalId: string; principalType: string }; @@ -66,16 +66,14 @@ function defIsPrivileged(def: RoleDefinition): boolean { * Role-based Access Controls. Flags excessive privileged assignments, wildcard * custom roles, and service principals holding privileged roles. */ -export const rbacLeastPrivilegeCheck: IntegrationCheck = { - id: 'azure-rbac-least-privilege', - name: 'Azure RBAC — least privilege', - description: - 'Flags excessive privileged role assignments, custom roles with wildcard permissions, and service principals with privileged roles.', - service: 'entra-id', - taskMapping: TASK_TEMPLATES.rolebasedAccessControls, - run: async (ctx: CheckContext) => { - const sub = await resolveAzureSubscriptionId(ctx); - if (!sub) return; +async function runRbacLeastPrivilegeForSubscription( + ctx: CheckContext, + sub: string, + // Shared across subscriptions: management-group/resource-group role + // definitions are tenant-level, so re-fetching them per subscription only + // burns budget. + resolvedDefs: Map, +): Promise { const [assignments, definitions] = await Promise.all([ armListAllOrFail( @@ -97,7 +95,6 @@ export const rbacLeastPrivilegeCheck: IntegrationCheck = { // resource group, which won't appear in the subscription-scope list above. // Resolve any missing definition directly so privileged principals aren't // undercounted. Cache by id to avoid refetching shared definitions. - const resolvedDefs = new Map(); const resolveFailures: ReadFailure[] = []; const resolveDef = async ( roleDefinitionId: string, @@ -251,5 +248,20 @@ export const rbacLeastPrivilegeCheck: IntegrationCheck = { }, }); } +} + +export const rbacLeastPrivilegeCheck: IntegrationCheck = { + id: 'azure-rbac-least-privilege', + name: 'Azure RBAC — least privilege', + description: + 'Flags excessive privileged role assignments, custom roles with wildcard permissions, and service principals with privileged roles.', + service: 'entra-id', + taskMapping: TASK_TEMPLATES.rolebasedAccessControls, + run: async (ctx: CheckContext) => { + const subs = await resolveAzureSubscriptionIds(ctx); + const resolvedDefs = new Map(); + for (const sub of subs) { + await runRbacLeastPrivilegeForSubscription(ctx, sub, resolvedDefs); + } }, }; diff --git a/packages/integration-platform/src/manifests/azure/checks/key-vault.ts b/packages/integration-platform/src/manifests/azure/checks/key-vault.ts index 4015c0c995..dd30c2ccc2 100644 --- a/packages/integration-platform/src/manifests/azure/checks/key-vault.ts +++ b/packages/integration-platform/src/manifests/azure/checks/key-vault.ts @@ -1,6 +1,6 @@ import { TASK_TEMPLATES } from '../../../task-mappings'; import type { CheckContext, FindingSeverity, IntegrationCheck } from '../../../types'; -import { ARM_BASE, armListAllOrFail, resolveAzureSubscriptionId } from './shared'; +import { ARM_BASE, armListAllOrFail, resolveAzureSubscriptionIds } from './shared'; interface KeyVault { id: string; @@ -26,16 +26,7 @@ async function listVaults( } /** Soft delete + purge protection + no public access on Key Vaults → Secure Secrets. */ -export const keyVaultProtectionCheck: IntegrationCheck = { - id: 'azure-key-vault-protection', - name: 'Key Vault — soft delete, purge protection, no public access', - description: - 'Verify Key Vaults enable soft delete and purge protection and restrict public network access.', - service: 'key-vault', - taskMapping: TASK_TEMPLATES.secureSecrets, - run: async (ctx: CheckContext) => { - const sub = await resolveAzureSubscriptionId(ctx); - if (!sub) return; +async function runKeyVaultProtectionForSubscription(ctx: CheckContext, sub: string): Promise { const vaults = await listVaults(ctx, sub); if (!vaults) return; if (vaults.length === 0) return; @@ -87,20 +78,25 @@ export const keyVaultProtectionCheck: IntegrationCheck = { }); } } - }, -}; +} -/** Azure RBAC authorization (not legacy access policies) on Key Vaults → Role-based Access Controls. */ -export const keyVaultRbacCheck: IntegrationCheck = { - id: 'azure-key-vault-rbac', - name: 'Key Vault — RBAC authorization', +export const keyVaultProtectionCheck: IntegrationCheck = { + id: 'azure-key-vault-protection', + name: 'Key Vault — soft delete, purge protection, no public access', description: - 'Verify Key Vaults use Azure RBAC instead of legacy vault access policies.', + 'Verify Key Vaults enable soft delete and purge protection and restrict public network access.', service: 'key-vault', - taskMapping: TASK_TEMPLATES.rolebasedAccessControls, + taskMapping: TASK_TEMPLATES.secureSecrets, run: async (ctx: CheckContext) => { - const sub = await resolveAzureSubscriptionId(ctx); - if (!sub) return; + const subs = await resolveAzureSubscriptionIds(ctx); + for (const sub of subs) { + await runKeyVaultProtectionForSubscription(ctx, sub); + } + }, +}; + +/** Azure RBAC authorization (not legacy access policies) on Key Vaults → Role-based Access Controls. */ +async function runKeyVaultRbacForSubscription(ctx: CheckContext, sub: string): Promise { const vaults = await listVaults(ctx, sub); if (!vaults) return; if (vaults.length === 0) return; @@ -129,5 +125,19 @@ export const keyVaultRbacCheck: IntegrationCheck = { }); } } +} + +export const keyVaultRbacCheck: IntegrationCheck = { + id: 'azure-key-vault-rbac', + name: 'Key Vault — RBAC authorization', + description: + 'Verify Key Vaults use Azure RBAC instead of legacy vault access policies.', + service: 'key-vault', + taskMapping: TASK_TEMPLATES.rolebasedAccessControls, + run: async (ctx: CheckContext) => { + const subs = await resolveAzureSubscriptionIds(ctx); + for (const sub of subs) { + await runKeyVaultRbacForSubscription(ctx, sub); + } }, }; diff --git a/packages/integration-platform/src/manifests/azure/checks/monitor.ts b/packages/integration-platform/src/manifests/azure/checks/monitor.ts index b368afb8eb..141cdf1951 100644 --- a/packages/integration-platform/src/manifests/azure/checks/monitor.ts +++ b/packages/integration-platform/src/manifests/azure/checks/monitor.ts @@ -5,7 +5,7 @@ import { toHttpReadFailure, type ReadFailure, } from '../../http-read-failure'; -import { ARM_BASE, armListAll, resolveAzureSubscriptionId } from './shared'; +import { ARM_BASE, armListAll, resolveAzureSubscriptionIds } from './shared'; interface ActivityLogAlert { properties?: { @@ -31,16 +31,7 @@ const RECOMMENDED_ALERTS = [ ]; /** Activity log alerts for critical ops + subscription log export → Monitoring & Alerting. */ -export const monitorLoggingAlertingCheck: IntegrationCheck = { - id: 'azure-monitor-logging-alerting', - name: 'Azure Monitor — alerts and log export', - description: - 'Verify activity log alerts exist for critical operations and subscription logs are exported.', - service: 'monitor', - taskMapping: TASK_TEMPLATES.monitoringAlerting, - run: async (ctx: CheckContext) => { - const sub = await resolveAzureSubscriptionId(ctx); - if (!sub) return; +async function runMonitorLoggingAlertingForSubscription(ctx: CheckContext, sub: string): Promise { let evaluated = false; let alertsReadFailure: ReadFailure | undefined; @@ -180,5 +171,19 @@ export const monitorLoggingAlertingCheck: IntegrationCheck = { if (!evaluated) { ctx.log('Azure monitor check: could not read monitor data — skipping'); } +} + +export const monitorLoggingAlertingCheck: IntegrationCheck = { + id: 'azure-monitor-logging-alerting', + name: 'Azure Monitor — alerts and log export', + description: + 'Verify activity log alerts exist for critical operations and subscription logs are exported.', + service: 'monitor', + taskMapping: TASK_TEMPLATES.monitoringAlerting, + run: async (ctx: CheckContext) => { + const subs = await resolveAzureSubscriptionIds(ctx); + for (const sub of subs) { + await runMonitorLoggingAlertingForSubscription(ctx, sub); + } }, }; diff --git a/packages/integration-platform/src/manifests/azure/checks/mysql-flexible.ts b/packages/integration-platform/src/manifests/azure/checks/mysql-flexible.ts index 2fb7af9b18..591c152ff3 100644 --- a/packages/integration-platform/src/manifests/azure/checks/mysql-flexible.ts +++ b/packages/integration-platform/src/manifests/azure/checks/mysql-flexible.ts @@ -6,7 +6,7 @@ import { toHttpReadFailure, type ReadFailure, } from '../../http-read-failure'; -import { ARM_BASE, armListAllOrFail, resolveAzureSubscriptionId } from './shared'; +import { ARM_BASE, armListAllOrFail, resolveAzureSubscriptionIds } from './shared'; // Pinned stable api-version for Azure Database for MySQL Flexible Server. const MYSQL_API_VERSION = '2023-12-30'; @@ -108,16 +108,7 @@ async function readConfigValue( * Flexible Server resource type (Microsoft.DBforMySQL/flexibleServers), whose * TLS enforcement lives in server parameters rather than a top-level property. */ -export const mysqlFlexibleTlsCheck: IntegrationCheck = { - id: 'azure-mysql-flexible-tls', - name: 'Database for MySQL — TLS 1.2 enforced', - description: - 'Verify Azure Database for MySQL Flexible Servers require secure transport and a minimum TLS version of 1.2.', - service: 'mysql-flexible', - taskMapping: TASK_TEMPLATES.tlsHttps, - run: async (ctx: CheckContext) => { - const sub = await resolveAzureSubscriptionId(ctx); - if (!sub) return; +async function runMysqlFlexibleTlsForSubscription(ctx: CheckContext, sub: string): Promise { const servers = await listMySqlFlexibleServers(ctx, sub); if (!servers) return; if (servers.length === 0) return; @@ -181,5 +172,19 @@ export const mysqlFlexibleTlsCheck: IntegrationCheck = { }); } } +} + +export const mysqlFlexibleTlsCheck: IntegrationCheck = { + id: 'azure-mysql-flexible-tls', + name: 'Database for MySQL — TLS 1.2 enforced', + description: + 'Verify Azure Database for MySQL Flexible Servers require secure transport and a minimum TLS version of 1.2.', + service: 'mysql-flexible', + taskMapping: TASK_TEMPLATES.tlsHttps, + run: async (ctx: CheckContext) => { + const subs = await resolveAzureSubscriptionIds(ctx); + for (const sub of subs) { + await runMysqlFlexibleTlsForSubscription(ctx, sub); + } }, }; diff --git a/packages/integration-platform/src/manifests/azure/checks/network.ts b/packages/integration-platform/src/manifests/azure/checks/network.ts index b84f62129d..79967d92b6 100644 --- a/packages/integration-platform/src/manifests/azure/checks/network.ts +++ b/packages/integration-platform/src/manifests/azure/checks/network.ts @@ -1,6 +1,6 @@ import { TASK_TEMPLATES } from '../../../task-mappings'; import type { CheckContext, FindingSeverity, IntegrationCheck } from '../../../types'; -import { ARM_BASE, armListAllOrFail, resolveAzureSubscriptionId } from './shared'; +import { ARM_BASE, armListAllOrFail, resolveAzureSubscriptionIds } from './shared'; interface SecurityRule { name: string; @@ -71,16 +71,7 @@ function rulePorts(r: SecurityRule): string[] { } /** NSG inbound rules open to the internet on sensitive ports → Production Firewall / no public access. */ -export const nsgNoOpenPortsCheck: IntegrationCheck = { - id: 'azure-nsg-no-open-ports', - name: 'Network — no NSG ports open to the internet', - description: - 'Flags NSG inbound rules that allow SSH, RDP, database ports, or all ports from the internet.', - service: 'network-watcher', - taskMapping: TASK_TEMPLATES.productionFirewallNopublicaccessControls, - run: async (ctx: CheckContext) => { - const sub = await resolveAzureSubscriptionId(ctx); - if (!sub) return; +async function runNsgNoOpenPortsForSubscription(ctx: CheckContext, sub: string): Promise { const nsgs = await armListAllOrFail( ctx, `${ARM_BASE}/subscriptions/${sub}/providers/Microsoft.Network/networkSecurityGroups?api-version=2023-11-01`, @@ -149,5 +140,19 @@ export const nsgNoOpenPortsCheck: IntegrationCheck = { }); } } +} + +export const nsgNoOpenPortsCheck: IntegrationCheck = { + id: 'azure-nsg-no-open-ports', + name: 'Network — no NSG ports open to the internet', + description: + 'Flags NSG inbound rules that allow SSH, RDP, database ports, or all ports from the internet.', + service: 'network-watcher', + taskMapping: TASK_TEMPLATES.productionFirewallNopublicaccessControls, + run: async (ctx: CheckContext) => { + const subs = await resolveAzureSubscriptionIds(ctx); + for (const sub of subs) { + await runNsgNoOpenPortsForSubscription(ctx, sub); + } }, }; diff --git a/packages/integration-platform/src/manifests/azure/checks/postgresql-flexible.ts b/packages/integration-platform/src/manifests/azure/checks/postgresql-flexible.ts index 321a64068f..d398983e54 100644 --- a/packages/integration-platform/src/manifests/azure/checks/postgresql-flexible.ts +++ b/packages/integration-platform/src/manifests/azure/checks/postgresql-flexible.ts @@ -6,7 +6,7 @@ import { toHttpReadFailure, type ReadFailure, } from '../../http-read-failure'; -import { ARM_BASE, armListAllOrFail, resolveAzureSubscriptionId } from './shared'; +import { ARM_BASE, armListAllOrFail, resolveAzureSubscriptionIds } from './shared'; // Pinned stable api-version for Azure Database for PostgreSQL Flexible Server. // NOTE: PostgreSQL is a SEPARATE resource provider from MySQL with its own @@ -112,16 +112,7 @@ async function readConfig( * customer running only PostgreSQL Flexible Server gets 0 servers found by the * Azure SQL check → "0 passed" for the TLS task (the reported bug class). */ -export const postgresqlFlexibleTlsCheck: IntegrationCheck = { - id: 'azure-postgresql-flexible-tls', - name: 'Database for PostgreSQL — TLS 1.2 enforced', - description: - 'Verify Azure Database for PostgreSQL Flexible Servers require secure transport and a minimum TLS version of 1.2.', - service: 'postgresql-flexible', - taskMapping: TASK_TEMPLATES.tlsHttps, - run: async (ctx: CheckContext) => { - const sub = await resolveAzureSubscriptionId(ctx); - if (!sub) return; +async function runPostgresqlFlexibleTlsForSubscription(ctx: CheckContext, sub: string): Promise { const servers = await listPgFlexibleServers(ctx, sub); if (!servers) return; if (servers.length === 0) return; @@ -182,5 +173,19 @@ export const postgresqlFlexibleTlsCheck: IntegrationCheck = { }); } } +} + +export const postgresqlFlexibleTlsCheck: IntegrationCheck = { + id: 'azure-postgresql-flexible-tls', + name: 'Database for PostgreSQL — TLS 1.2 enforced', + description: + 'Verify Azure Database for PostgreSQL Flexible Servers require secure transport and a minimum TLS version of 1.2.', + service: 'postgresql-flexible', + taskMapping: TASK_TEMPLATES.tlsHttps, + run: async (ctx: CheckContext) => { + const subs = await resolveAzureSubscriptionIds(ctx); + for (const sub of subs) { + await runPostgresqlFlexibleTlsForSubscription(ctx, sub); + } }, }; diff --git a/packages/integration-platform/src/manifests/azure/checks/shared.ts b/packages/integration-platform/src/manifests/azure/checks/shared.ts index 9f39743710..cfe6933dca 100644 --- a/packages/integration-platform/src/manifests/azure/checks/shared.ts +++ b/packages/integration-platform/src/manifests/azure/checks/shared.ts @@ -3,35 +3,104 @@ import { remediationForReadFailure, toHttpReadFailure } from '../../http-read-fa const ARM = 'https://management.azure.com'; +/** Fan-out bound for auto-discovered subscriptions (13 checks × N subs). */ +const MAX_SUBSCRIPTIONS = 50; + +/** The legacy single-subscription variable, auto-saved by the Cloud Tests + * setup. It is a detection cache, not a deliberate scope choice, so the + * evidence checks only use it when subscriptions cannot be listed. */ +function legacySubscriptionId(ctx: CheckContext): string | null { + const configured = ctx.variables.subscription_id; + return typeof configured === 'string' && configured.trim().length > 0 + ? configured.trim() + : null; +} + /** - * Resolve the Azure subscription to scan: the user-set `subscription_id` - * variable, else the first enabled subscription the token can see. Returns - * null when none — the check should then no-op (no false pass). + * Resolve the Azure subscriptions a check should evaluate: the user-selected + * `subscription_ids` variable if present, otherwise ALL Enabled subscriptions + * the token can see (mirrors how GCP scans all active projects). Previously + * only the first Enabled subscription was scanned — silently skipping the + * rest in multi-subscription tenants. + * + * Returns [] only after emitting an explicit "could not verify" finding, so + * a scope failure never leaves the mapped tasks silently stale. */ -export async function resolveAzureSubscriptionId( +export async function resolveAzureSubscriptionIds( ctx: CheckContext, -): Promise { - const configured = ctx.variables.subscription_id; - if (typeof configured === 'string' && configured.trim().length > 0) { - return configured.trim(); +): Promise { + const selected = ctx.variables.subscription_ids; + if (Array.isArray(selected)) { + const cleaned = selected + .filter((s): s is string => typeof s === 'string') + .map((s) => s.trim()) + .filter((s) => s.length > 0); + if (cleaned.length > 0) return cleaned; } + + const legacy = legacySubscriptionId(ctx); try { const data = await ctx.fetch<{ value?: Array<{ subscriptionId: string; state?: string }>; }>(`${ARM}/subscriptions?api-version=2020-01-01`); - const subs = data.value ?? []; - // Only auto-select an Enabled subscription. Falling back to the first - // subscription regardless of state could pick a Disabled/PastDue one whose - // API calls fail; returning null instead makes the check no-op cleanly (the - // user can set subscription_id explicitly). - const active = subs.find((s) => s.state === 'Enabled'); - return active?.subscriptionId ?? null; + // Only Enabled subscriptions — a Disabled/PastDue one would fail every + // API call and drown the run in false "could not verify" findings. + const enabled = (data.value ?? []) + .filter((s) => s.state === 'Enabled') + .map((s) => s.subscriptionId); + if (enabled.length > 0) { + if (legacy && (enabled.length > 1 || enabled[0] !== legacy)) { + ctx.log( + `Azure: scanning all ${enabled.length} enabled subscription(s); set subscription_ids to scope explicitly (legacy subscription_id is no longer used for scoping)`, + ); + } + // Bound the fan-out so a giant tenant can't blow the run budget — + // loudly, so capped coverage is never mistaken for full coverage. + if (enabled.length > MAX_SUBSCRIPTIONS) { + ctx.warn( + `Azure: ${enabled.length} enabled subscriptions found; scanning the first ${MAX_SUBSCRIPTIONS} — set subscription_ids to scope explicitly`, + { enabled: enabled.length, scanned: MAX_SUBSCRIPTIONS }, + ); + return enabled.slice(0, MAX_SUBSCRIPTIONS); + } + return enabled; + } + // Nothing visible via the list call — the token may lack list permission + // at root scope while still having Reader on one subscription. + if (legacy) return [legacy]; + ctx.fail({ + title: 'Could not verify Azure subscription scope', + description: + 'No enabled Azure subscription is visible to the connection, so nothing could be scanned.', + resourceType: 'azure-subscription', + resourceId: 'unknown', + severity: 'medium', + remediation: + 'Grant the connection Reader access to at least one subscription (or select subscriptions in the integration settings), then re-run the check.', + evidence: { enabledSubscriptionsVisible: 0 }, + }); + return []; } catch (err) { - ctx.warn( - 'Failed to auto-detect Azure subscription; set subscription_id manually', - { error: err instanceof Error ? err.message : String(err) }, - ); - return null; + if (legacy) { + ctx.log( + `Azure: could not list subscriptions (${err instanceof Error ? err.message : String(err)}); falling back to the detected subscription_id`, + ); + return [legacy]; + } + const failure = toHttpReadFailure(err); + ctx.fail({ + title: 'Could not verify Azure subscription scope', + description: `Azure subscriptions could not be listed (${failure.error}), so nothing could be scanned.`, + resourceType: 'azure-subscription', + resourceId: 'unknown', + severity: 'medium', + remediation: remediationForReadFailure( + failure, + 'Grant the connection Reader access to the subscription(s), then re-run the check.', + ), + evidence: { readError: failure.error }, + }); + return []; } } diff --git a/packages/integration-platform/src/manifests/azure/checks/sql.ts b/packages/integration-platform/src/manifests/azure/checks/sql.ts index 8e6e06edfe..57d0934a18 100644 --- a/packages/integration-platform/src/manifests/azure/checks/sql.ts +++ b/packages/integration-platform/src/manifests/azure/checks/sql.ts @@ -5,7 +5,7 @@ import { toHttpReadFailure, type ReadFailure, } from '../../http-read-failure'; -import { ARM_BASE, armListAll, armListAllOrFail, resolveAzureSubscriptionId } from './shared'; +import { ARM_BASE, armListAll, armListAllOrFail, resolveAzureSubscriptionIds } from './shared'; interface SqlServer { id: string; @@ -32,15 +32,7 @@ async function listSqlServers( } /** SQL Server minimum TLS 1.2 → TLS / HTTPS. */ -export const sqlTlsCheck: IntegrationCheck = { - id: 'azure-sql-tls', - name: 'SQL Database — TLS 1.2 enforced', - description: 'Verify SQL Servers require a minimum TLS version of 1.2.', - service: 'sql-database', - taskMapping: TASK_TEMPLATES.tlsHttps, - run: async (ctx: CheckContext) => { - const sub = await resolveAzureSubscriptionId(ctx); - if (!sub) return; +async function runSqlTlsForSubscription(ctx: CheckContext, sub: string): Promise { const servers = await listSqlServers(ctx, sub); if (!servers) return; if (servers.length === 0) return; @@ -68,20 +60,24 @@ export const sqlTlsCheck: IntegrationCheck = { }); } } +} + +export const sqlTlsCheck: IntegrationCheck = { + id: 'azure-sql-tls', + name: 'SQL Database — TLS 1.2 enforced', + description: 'Verify SQL Servers require a minimum TLS version of 1.2.', + service: 'sql-database', + taskMapping: TASK_TEMPLATES.tlsHttps, + run: async (ctx: CheckContext) => { + const subs = await resolveAzureSubscriptionIds(ctx); + for (const sub of subs) { + await runSqlTlsForSubscription(ctx, sub); + } }, }; /** SQL Server no public network / wide-open firewall → Production Firewall / no public access. */ -export const sqlPublicAccessCheck: IntegrationCheck = { - id: 'azure-sql-no-public-access', - name: 'SQL Database — no public access', - description: - 'Verify SQL Servers disable public network access and have no wide-open firewall rules.', - service: 'sql-database', - taskMapping: TASK_TEMPLATES.productionFirewallNopublicaccessControls, - run: async (ctx: CheckContext) => { - const sub = await resolveAzureSubscriptionId(ctx); - if (!sub) return; +async function runSqlPublicAccessForSubscription(ctx: CheckContext, sub: string): Promise { const servers = await listSqlServers(ctx, sub); if (!servers) return; if (servers.length === 0) return; @@ -184,6 +180,20 @@ export const sqlPublicAccessCheck: IntegrationCheck = { }); } } +} + +export const sqlPublicAccessCheck: IntegrationCheck = { + id: 'azure-sql-no-public-access', + name: 'SQL Database — no public access', + description: + 'Verify SQL Servers disable public network access and have no wide-open firewall rules.', + service: 'sql-database', + taskMapping: TASK_TEMPLATES.productionFirewallNopublicaccessControls, + run: async (ctx: CheckContext) => { + const subs = await resolveAzureSubscriptionIds(ctx); + for (const sub of subs) { + await runSqlPublicAccessForSubscription(ctx, sub); + } }, }; @@ -192,15 +202,7 @@ interface AuditingSetting { } /** SQL Server auditing enabled → Monitoring & Alerting. */ -export const sqlAuditingCheck: IntegrationCheck = { - id: 'azure-sql-auditing', - name: 'SQL Database — auditing enabled', - description: 'Verify SQL Servers have auditing enabled to track database operations.', - service: 'sql-database', - taskMapping: TASK_TEMPLATES.monitoringAlerting, - run: async (ctx: CheckContext) => { - const sub = await resolveAzureSubscriptionId(ctx); - if (!sub) return; +async function runSqlAuditingForSubscription(ctx: CheckContext, sub: string): Promise { const servers = await listSqlServers(ctx, sub); if (!servers) return; if (servers.length === 0) return; @@ -255,5 +257,18 @@ export const sqlAuditingCheck: IntegrationCheck = { }); } } +} + +export const sqlAuditingCheck: IntegrationCheck = { + id: 'azure-sql-auditing', + name: 'SQL Database — auditing enabled', + description: 'Verify SQL Servers have auditing enabled to track database operations.', + service: 'sql-database', + taskMapping: TASK_TEMPLATES.monitoringAlerting, + run: async (ctx: CheckContext) => { + const subs = await resolveAzureSubscriptionIds(ctx); + for (const sub of subs) { + await runSqlAuditingForSubscription(ctx, sub); + } }, }; diff --git a/packages/integration-platform/src/manifests/azure/checks/storage.ts b/packages/integration-platform/src/manifests/azure/checks/storage.ts index 82c26ac29a..04f0ad83cf 100644 --- a/packages/integration-platform/src/manifests/azure/checks/storage.ts +++ b/packages/integration-platform/src/manifests/azure/checks/storage.ts @@ -1,6 +1,6 @@ import { TASK_TEMPLATES } from '../../../task-mappings'; import type { CheckContext, IntegrationCheck } from '../../../types'; -import { ARM_BASE, armListAllOrFail, resolveAzureSubscriptionId } from './shared'; +import { ARM_BASE, armListAllOrFail, resolveAzureSubscriptionIds } from './shared'; interface StorageAccount { id: string; @@ -36,16 +36,7 @@ async function listStorageAccounts( } /** HTTPS-only + minimum TLS 1.2 on storage accounts → TLS / HTTPS. */ -export const storageHttpsTlsCheck: IntegrationCheck = { - id: 'azure-storage-https-tls', - name: 'Storage — HTTPS and TLS 1.2 enforced', - description: - 'Verify storage accounts enforce HTTPS-only traffic and a minimum TLS version of 1.2.', - service: 'storage-account', - taskMapping: TASK_TEMPLATES.tlsHttps, - run: async (ctx: CheckContext) => { - const sub = await resolveAzureSubscriptionId(ctx); - if (!sub) return; +async function runStorageHttpsTlsForSubscription(ctx: CheckContext, sub: string): Promise { const accounts = await listStorageAccounts(ctx, sub); if (!accounts) return; if (accounts.length === 0) return; @@ -85,20 +76,25 @@ export const storageHttpsTlsCheck: IntegrationCheck = { }); } } - }, -}; +} -/** No public blob/network access on storage accounts → Production Firewall / no public access. */ -export const storagePublicAccessCheck: IntegrationCheck = { - id: 'azure-storage-no-public-access', - name: 'Storage — no public access', +export const storageHttpsTlsCheck: IntegrationCheck = { + id: 'azure-storage-https-tls', + name: 'Storage — HTTPS and TLS 1.2 enforced', description: - 'Verify storage accounts disable anonymous blob access and public network access.', + 'Verify storage accounts enforce HTTPS-only traffic and a minimum TLS version of 1.2.', service: 'storage-account', - taskMapping: TASK_TEMPLATES.productionFirewallNopublicaccessControls, + taskMapping: TASK_TEMPLATES.tlsHttps, run: async (ctx: CheckContext) => { - const sub = await resolveAzureSubscriptionId(ctx); - if (!sub) return; + const subs = await resolveAzureSubscriptionIds(ctx); + for (const sub of subs) { + await runStorageHttpsTlsForSubscription(ctx, sub); + } + }, +}; + +/** No public blob/network access on storage accounts → Production Firewall / no public access. */ +async function runStoragePublicAccessForSubscription(ctx: CheckContext, sub: string): Promise { const accounts = await listStorageAccounts(ctx, sub); if (!accounts) return; if (accounts.length === 0) return; @@ -146,20 +142,25 @@ export const storagePublicAccessCheck: IntegrationCheck = { }); } } - }, -}; +} -/** Service-side encryption enabled on storage accounts → Encryption at Rest. */ -export const storageEncryptionCheck: IntegrationCheck = { - id: 'azure-storage-encryption-at-rest', - name: 'Storage — encryption at rest enabled', +export const storagePublicAccessCheck: IntegrationCheck = { + id: 'azure-storage-no-public-access', + name: 'Storage — no public access', description: - 'Verify storage accounts have blob and file service encryption enabled.', + 'Verify storage accounts disable anonymous blob access and public network access.', service: 'storage-account', - taskMapping: TASK_TEMPLATES.encryptionAtRest, + taskMapping: TASK_TEMPLATES.productionFirewallNopublicaccessControls, run: async (ctx: CheckContext) => { - const sub = await resolveAzureSubscriptionId(ctx); - if (!sub) return; + const subs = await resolveAzureSubscriptionIds(ctx); + for (const sub of subs) { + await runStoragePublicAccessForSubscription(ctx, sub); + } + }, +}; + +/** Service-side encryption enabled on storage accounts → Encryption at Rest. */ +async function runStorageEncryptionForSubscription(ctx: CheckContext, sub: string): Promise { const accounts = await listStorageAccounts(ctx, sub); if (!accounts) return; if (accounts.length === 0) return; @@ -187,5 +188,19 @@ export const storageEncryptionCheck: IntegrationCheck = { }); } } +} + +export const storageEncryptionCheck: IntegrationCheck = { + id: 'azure-storage-encryption-at-rest', + name: 'Storage — encryption at rest enabled', + description: + 'Verify storage accounts have blob and file service encryption enabled.', + service: 'storage-account', + taskMapping: TASK_TEMPLATES.encryptionAtRest, + run: async (ctx: CheckContext) => { + const subs = await resolveAzureSubscriptionIds(ctx); + for (const sub of subs) { + await runStorageEncryptionForSubscription(ctx, sub); + } }, }; diff --git a/packages/integration-platform/src/manifests/azure/index.ts b/packages/integration-platform/src/manifests/azure/index.ts index ac8cc144c1..6655bfe558 100644 --- a/packages/integration-platform/src/manifests/azure/index.ts +++ b/packages/integration-platform/src/manifests/azure/index.ts @@ -97,6 +97,35 @@ Our integration only makes read-only API calls for security scanning.`, variables: [ { + id: 'subscription_ids', + label: 'Azure Subscriptions', + type: 'multi-select', + required: false, + helpText: + 'Select which subscriptions to scan. Leave empty to scan all enabled subscriptions.', + fetchOptions: async (ctx) => { + const data = await ctx.fetch<{ + value?: Array<{ + subscriptionId: string; + displayName?: string; + state?: string; + }>; + }>('https://management.azure.com/subscriptions?api-version=2020-01-01'); + return (data.value ?? []) + .filter((s) => s.state === 'Enabled') + .sort((a, b) => (a.displayName ?? '').localeCompare(b.displayName ?? '')) + .map((s) => ({ + value: s.subscriptionId, + label: s.displayName + ? `${s.displayName} (${s.subscriptionId})` + : s.subscriptionId, + })); + }, + }, + { + // Kept for the Cloud Tests product, which auto-detects and reads this + // value on its own path. The evidence checks scope via subscription_ids + // and only fall back to this when subscriptions cannot be listed. id: 'subscription_id', label: 'Azure Subscription ID', type: 'text', diff --git a/packages/integration-platform/src/manifests/gcp/checks/__tests__/gcp-checks.test.ts b/packages/integration-platform/src/manifests/gcp/checks/__tests__/gcp-checks.test.ts index 22d8ac639c..30b8f2b99f 100644 --- a/packages/integration-platform/src/manifests/gcp/checks/__tests__/gcp-checks.test.ts +++ b/packages/integration-platform/src/manifests/gcp/checks/__tests__/gcp-checks.test.ts @@ -124,6 +124,23 @@ describe('GCP read-failure remediation gating', () => { }); }); +describe('GCP project scope failure', () => { + it('emits an explicit scope finding when project discovery fails', async () => { + const err = new Error('HTTP 500: boom'); + (err as Error & { status: number }).status = 500; + const out = await runCheck(vpcOpenFirewallsCheck, { + variables: {}, + fetch: () => { + throw err; + }, + }); + expect(out.failed).toHaveLength(1); + expect(out.failed[0]!.title).toMatch(/Could not verify GCP project scope/); + expect(out.failed[0]!.remediation).toMatch(/re-run/i); + expect(out.failed[0]!.evidence).toMatchObject({ readError: 'HTTP 500: boom' }); + }); +}); + describe('GCP IAM primitive roles check', () => { it('fails on roles/owner binding (high)', async () => { const { passed, failed } = await runCheck(iamPrimitiveRolesCheck, { diff --git a/packages/integration-platform/src/manifests/gcp/checks/shared.ts b/packages/integration-platform/src/manifests/gcp/checks/shared.ts index 3dd0416c6c..9a6cef22ca 100644 --- a/packages/integration-platform/src/manifests/gcp/checks/shared.ts +++ b/packages/integration-platform/src/manifests/gcp/checks/shared.ts @@ -1,11 +1,12 @@ import type { CheckContext } from '../../../types'; +import { remediationForReadFailure, toHttpReadFailure } from '../../http-read-failure'; /** * Resolve which GCP projects a check should evaluate: the user-selected * `project_ids` variable if present, otherwise a bounded best-effort - * detection of active projects. Returns [] when none can be resolved — the - * check should then no-op (emit neither pass nor fail) rather than produce a - * false pass. + * detection of active projects. Returns [] when none can be resolved; a + * discovery FAILURE emits an explicit "could not verify" finding first, so + * the mapped task never goes silently stale. */ export async function resolveGcpProjectIds(ctx: CheckContext): Promise { const selected = ctx.variables.project_ids; @@ -54,8 +55,20 @@ export async function resolveGcpProjectIds(ctx: CheckContext): Promise } return projectIds; } catch (err) { - ctx.warn('GCP project auto-discovery failed; checks will be skipped', { - error: err instanceof Error ? err.message : String(err), + // Surface the scope failure as an explicit finding — a silent [] would + // leave every mapped task stale with no signal anything went wrong. + const failure = toHttpReadFailure(err); + ctx.fail({ + title: 'Could not verify GCP project scope', + description: `GCP projects could not be listed (${failure.error}), so nothing could be scanned.`, + resourceType: 'gcp-project', + resourceId: 'unknown', + severity: 'medium', + remediation: remediationForReadFailure( + failure, + 'Grant resourcemanager.projects.get to the connection (or select projects in the integration settings), then re-run the check.', + ), + evidence: { readError: failure.error }, }); return []; } From 1493c5c50eb48d057bfba411bd52eed8f229e7ce Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Wed, 10 Jun 2026 17:44:02 -0400 Subject: [PATCH 06/18] fix(integration-platform): stamp account attribution on account-level AWS findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cubic flagged on PR #3086 that the new iam.ts password-policy finding bypasses the shared attribution path — in a multi-account org the merged task panel shows it with no way to tell which AWS account it belongs to. The same gap existed in every account-level direct ctx.fail finding: could-not-assume-role (shared.ts), the S3/EC2/RDS/ KMS/CloudTrail aggregate could-not-verify findings, and both iam.ts findings. Route all 9 sites through emitOutcomes, which stamps awsAccountId + awsConnectionName into evidence and appends the account label to the description (same as every per-resource finding since PR #3065). Co-Authored-By: Claude Fable 5 --- .../aws/checks/__tests__/aws-checks.test.ts | 29 ++++++++++ .../src/manifests/aws/checks/cloudtrail.ts | 35 ++++++------ .../src/manifests/aws/checks/ec2.ts | 35 ++++++------ .../src/manifests/aws/checks/iam.ts | 54 ++++++++++--------- .../src/manifests/aws/checks/kms.ts | 39 +++++++------- .../src/manifests/aws/checks/rds.ts | 35 ++++++------ .../src/manifests/aws/checks/s3.ts | 50 +++++++++-------- .../src/manifests/aws/checks/shared.ts | 25 +++++---- 8 files changed, 179 insertions(+), 123 deletions(-) diff --git a/packages/integration-platform/src/manifests/aws/checks/__tests__/aws-checks.test.ts b/packages/integration-platform/src/manifests/aws/checks/__tests__/aws-checks.test.ts index 97bc76ac06..1286692797 100644 --- a/packages/integration-platform/src/manifests/aws/checks/__tests__/aws-checks.test.ts +++ b/packages/integration-platform/src/manifests/aws/checks/__tests__/aws-checks.test.ts @@ -20,6 +20,7 @@ import { awsAccountIdFromCtx, emitOutcomes, resolveAwsCredentialInputs, + resolveAwsSessionOrFail, combineReadFailures, remediationForReadFailure, toReadFailure, @@ -886,3 +887,31 @@ describe('emitOutcomes — attributes findings to the AWS account', () => { ); }); }); + +describe('account-level findings carry AWS account attribution (cubic finding on CS-533)', () => { + it('resolveAwsSessionOrFail failures are stamped with the account from the role ARN', async () => { + const failed: Array<{ description: string; evidence?: Record }> = []; + const ctx = { + credentials: { + roleArn: 'arn:aws:iam::123456789012:role/x', + externalId: 'eid', + regions: ['us-east-1'], + connectionName: 'Prod AWS', + __resolvedSessionError: 'assume failed for test', + }, + fail: (r: { description: string; evidence?: Record }) => failed.push(r), + pass: () => {}, + log: () => {}, + } as unknown as Parameters[0]; + + const session = await resolveAwsSessionOrFail(ctx); + expect(session).toBeNull(); + expect(failed).toHaveLength(1); + expect(failed[0]!.evidence).toMatchObject({ + awsAccountId: '123456789012', + awsConnectionName: 'Prod AWS', + error: 'assume failed for test', + }); + expect(failed[0]!.description).toContain('AWS account 123456789012'); + }); +}); diff --git a/packages/integration-platform/src/manifests/aws/checks/cloudtrail.ts b/packages/integration-platform/src/manifests/aws/checks/cloudtrail.ts index b3fb2ee424..678c354001 100644 --- a/packages/integration-platform/src/manifests/aws/checks/cloudtrail.ts +++ b/packages/integration-platform/src/manifests/aws/checks/cloudtrail.ts @@ -228,23 +228,26 @@ export const cloudTrailEnabledCheck: IntegrationCheck = { // permissions/transient error) — report it as unverified instead. if (trails.length === 0 && regionFailures.length > 0) { const combined = combineReadFailures(regionFailures.map((r) => r.failure)); - ctx.fail({ - title: 'Could not verify CloudTrail configuration', - description: `CloudTrail trails could not be listed in: ${regionFailures.map((r) => r.region).join(', ')}, so trail configuration is unverified.`, - resourceType: 'aws-cloudtrail', - resourceId: 'account', - severity: 'medium', - remediation: remediationForReadFailure( - combined, - 'Grant cloudtrail:DescribeTrails to the integration role in all enabled regions, then re-run the check.', - ), - evidence: { - failedRegions: regionFailures.map((r) => ({ - region: r.region, - error: r.failure.error, - })), + emitOutcomes(ctx, [ + { + kind: 'fail', + title: 'Could not verify CloudTrail configuration', + description: `CloudTrail trails could not be listed in: ${regionFailures.map((r) => r.region).join(', ')}, so trail configuration is unverified.`, + resourceType: 'aws-cloudtrail', + resourceId: 'account', + severity: 'medium', + remediation: remediationForReadFailure( + combined, + 'Grant cloudtrail:DescribeTrails to the integration role in all enabled regions, then re-run the check.', + ), + evidence: { + failedRegions: regionFailures.map((r) => ({ + region: r.region, + error: r.failure.error, + })), + }, }, - }); + ]); return; } diff --git a/packages/integration-platform/src/manifests/aws/checks/ec2.ts b/packages/integration-platform/src/manifests/aws/checks/ec2.ts index 994363b9d1..f27745c6e4 100644 --- a/packages/integration-platform/src/manifests/aws/checks/ec2.ts +++ b/packages/integration-platform/src/manifests/aws/checks/ec2.ts @@ -148,23 +148,26 @@ export const ec2SecurityGroupsCheck: IntegrationCheck = { // total/partial read failure end as a silent clean run (no findings). if (regionFailures.length > 0) { const regions = regionFailures.map((r) => r.region); - ctx.fail({ - title: 'Could not verify security groups in some regions', - description: `Security groups could not be listed in: ${regions.join(', ')}. Internet exposure in those regions is unverified.`, - resourceType: 'aws-security-group', - resourceId: `regions:${regions.join(',')}`, - severity: 'medium', - remediation: remediationForReadFailure( - combineReadFailures(regionFailures.map((r) => r.failure)), - 'Grant ec2:DescribeSecurityGroups to the integration role in all enabled regions, then re-run the check.', - ), - evidence: { - failedRegions: regionFailures.map((r) => ({ - region: r.region, - error: r.failure.error, - })), + emitOutcomes(ctx, [ + { + kind: 'fail', + title: 'Could not verify security groups in some regions', + description: `Security groups could not be listed in: ${regions.join(', ')}. Internet exposure in those regions is unverified.`, + resourceType: 'aws-security-group', + resourceId: `regions:${regions.join(',')}`, + severity: 'medium', + remediation: remediationForReadFailure( + combineReadFailures(regionFailures.map((r) => r.failure)), + 'Grant ec2:DescribeSecurityGroups to the integration role in all enabled regions, then re-run the check.', + ), + evidence: { + failedRegions: regionFailures.map((r) => ({ + region: r.region, + error: r.failure.error, + })), + }, }, - }); + ]); } if (sgs.length === 0) return; emitOutcomes(ctx, evaluateSecurityGroups(sgs)); diff --git a/packages/integration-platform/src/manifests/aws/checks/iam.ts b/packages/integration-platform/src/manifests/aws/checks/iam.ts index c10c674764..1fe0431707 100644 --- a/packages/integration-platform/src/manifests/aws/checks/iam.ts +++ b/packages/integration-platform/src/manifests/aws/checks/iam.ts @@ -187,18 +187,21 @@ export const iamAccountSecurityCheck: IntegrationCheck = { // password-policy findings now so they aren't lost if the summary read // fails below. if (policyReadFailure) { - ctx.fail({ - title: 'Could not verify IAM password policy', - description: `The IAM account password policy could not be read (${policyReadFailure.error}), so password-policy strength is unverified.`, - resourceType: 'aws-account', - resourceId: 'account', - severity: 'medium', - remediation: remediationForReadFailure( - policyReadFailure, - 'Grant iam:GetAccountPasswordPolicy to the integration role, then re-run the check.', - ), - evidence: { readError: policyReadFailure.error }, - }); + emitOutcomes(ctx, [ + { + kind: 'fail', + title: 'Could not verify IAM password policy', + description: `The IAM account password policy could not be read (${policyReadFailure.error}), so password-policy strength is unverified.`, + resourceType: 'aws-account', + resourceId: 'account', + severity: 'medium', + remediation: remediationForReadFailure( + policyReadFailure, + 'Grant iam:GetAccountPasswordPolicy to the integration role, then re-run the check.', + ), + evidence: { readError: policyReadFailure.error }, + }, + ]); } else { emitOutcomes(ctx, evaluatePasswordPolicy(passwordPolicy)); } @@ -213,18 +216,21 @@ export const iamAccountSecurityCheck: IntegrationCheck = { // check with a bare error (or omitting those critical findings). const failure = toReadFailure(err); ctx.log(`IAM: could not read account summary: ${failure.error}`); - ctx.fail({ - title: 'Could not verify IAM account summary', - description: `The IAM account summary (root MFA, root access keys) could not be read (${failure.error}), so root-account security is unverified.`, - resourceType: 'aws-account', - resourceId: 'account', - severity: 'medium', - remediation: remediationForReadFailure( - failure, - 'Grant iam:GetAccountSummary to the integration role, then re-run the check.', - ), - evidence: { readError: failure.error }, - }); + emitOutcomes(ctx, [ + { + kind: 'fail', + title: 'Could not verify IAM account summary', + description: `The IAM account summary (root MFA, root access keys) could not be read (${failure.error}), so root-account security is unverified.`, + resourceType: 'aws-account', + resourceId: 'account', + severity: 'medium', + remediation: remediationForReadFailure( + failure, + 'Grant iam:GetAccountSummary to the integration role, then re-run the check.', + ), + evidence: { readError: failure.error }, + }, + ]); } }, }; diff --git a/packages/integration-platform/src/manifests/aws/checks/kms.ts b/packages/integration-platform/src/manifests/aws/checks/kms.ts index b5bdd06ac1..5407126b1d 100644 --- a/packages/integration-platform/src/manifests/aws/checks/kms.ts +++ b/packages/integration-platform/src/manifests/aws/checks/kms.ts @@ -201,25 +201,28 @@ export const kmsKeyRotationCheck: IntegrationCheck = { if (failedKeys.length > 0) { parts.push(`metadata could not be read for ${failedKeys.length} key(s)`); } - ctx.fail({ - title: 'Could not verify KMS keys', - description: `${parts.join('; ')} — rotation eligibility/status is unverified.`, - resourceType: 'aws-kms-key', - resourceId: 'account', - severity: 'medium', - remediation: remediationForReadFailure( - combineReadFailures(unreadable.map((u) => u.failure)), - 'Grant kms:ListKeys, kms:DescribeKey, and kms:GetKeyRotationStatus to the integration role in all enabled regions, then re-run the check.', - ), - evidence: { - failedRegions, - failedKeyCount: failedKeys.length, - // first few per-key errors so the cause is visible without log digging - keyReadErrors: failedKeys - .slice(0, 5) - .map((u) => ({ keyId: u.id, error: u.failure.error })), + emitOutcomes(ctx, [ + { + kind: 'fail', + title: 'Could not verify KMS keys', + description: `${parts.join('; ')} — rotation eligibility/status is unverified.`, + resourceType: 'aws-kms-key', + resourceId: 'account', + severity: 'medium', + remediation: remediationForReadFailure( + combineReadFailures(unreadable.map((u) => u.failure)), + 'Grant kms:ListKeys, kms:DescribeKey, and kms:GetKeyRotationStatus to the integration role in all enabled regions, then re-run the check.', + ), + evidence: { + failedRegions, + failedKeyCount: failedKeys.length, + // first few per-key errors so the cause is visible without log digging + keyReadErrors: failedKeys + .slice(0, 5) + .map((u) => ({ keyId: u.id, error: u.failure.error })), + }, }, - }); + ]); } // Rotation-eligible keys each produce an outcome (incl. could-not-verify for diff --git a/packages/integration-platform/src/manifests/aws/checks/rds.ts b/packages/integration-platform/src/manifests/aws/checks/rds.ts index 3d95bc43d3..37659e28b8 100644 --- a/packages/integration-platform/src/manifests/aws/checks/rds.ts +++ b/packages/integration-platform/src/manifests/aws/checks/rds.ts @@ -242,23 +242,26 @@ function failUnverifiedRegions( } } const regions = [...byRegion.keys()]; - ctx.fail({ - title: `Could not verify RDS ${what} in some regions`, - description: `RDS resources could not be listed in: ${regions.join(', ')}, so ${what} in those regions is unverified.`, - resourceType: 'aws-rds', - resourceId: `regions:${regions.join(',')}`, - severity: 'medium', - remediation: remediationForReadFailure( - combineReadFailures([...byRegion.values()]), - 'Grant rds:DescribeDBInstances and rds:DescribeDBClusters to the integration role in all enabled regions, then re-run the check.', - ), - evidence: { - failedRegions: [...byRegion.entries()].map(([region, failure]) => ({ - region, - error: failure.error, - })), + emitOutcomes(ctx, [ + { + kind: 'fail', + title: `Could not verify RDS ${what} in some regions`, + description: `RDS resources could not be listed in: ${regions.join(', ')}, so ${what} in those regions is unverified.`, + resourceType: 'aws-rds', + resourceId: `regions:${regions.join(',')}`, + severity: 'medium', + remediation: remediationForReadFailure( + combineReadFailures([...byRegion.values()]), + 'Grant rds:DescribeDBInstances and rds:DescribeDBClusters to the integration role in all enabled regions, then re-run the check.', + ), + evidence: { + failedRegions: [...byRegion.entries()].map(([region, failure]) => ({ + region, + error: failure.error, + })), + }, }, - }); + ]); } export const rdsEncryptionCheck: IntegrationCheck = { diff --git a/packages/integration-platform/src/manifests/aws/checks/s3.ts b/packages/integration-platform/src/manifests/aws/checks/s3.ts index 2d9570f674..57c96b9fe0 100644 --- a/packages/integration-platform/src/manifests/aws/checks/s3.ts +++ b/packages/integration-platform/src/manifests/aws/checks/s3.ts @@ -153,17 +153,20 @@ export const s3EncryptionCheck: IntegrationCheck = { clientForRegion, }); } catch (err) { - ctx.fail({ - title: 'Could not verify S3 encryption', - description: - 'S3 buckets could not be listed, so default encryption could not be verified.', - resourceType: 'aws-account', - resourceId: 'account', - severity: 'medium', - remediation: - 'Grant s3:ListAllMyBuckets (and s3:GetEncryptionConfiguration) to the integration role, then re-run the check.', - evidence: { error: err instanceof Error ? err.message : String(err) }, - }); + emitOutcomes(ctx, [ + { + kind: 'fail', + title: 'Could not verify S3 encryption', + description: + 'S3 buckets could not be listed, so default encryption could not be verified.', + resourceType: 'aws-account', + resourceId: 'account', + severity: 'medium', + remediation: + 'Grant s3:ListAllMyBuckets (and s3:GetEncryptionConfiguration) to the integration role, then re-run the check.', + evidence: { error: err instanceof Error ? err.message : String(err) }, + }, + ]); return; } if (buckets.length === 0) return; @@ -222,17 +225,20 @@ export const s3PublicAccessCheck: IntegrationCheck = { clientForRegion, }); } catch (err) { - ctx.fail({ - title: 'Could not verify S3 public access', - description: - 'S3 buckets could not be listed, so Block Public Access could not be verified.', - resourceType: 'aws-account', - resourceId: 'account', - severity: 'medium', - remediation: - 'Grant s3:ListAllMyBuckets (and s3:GetBucketPublicAccessBlock) to the integration role, then re-run the check.', - evidence: { error: err instanceof Error ? err.message : String(err) }, - }); + emitOutcomes(ctx, [ + { + kind: 'fail', + title: 'Could not verify S3 public access', + description: + 'S3 buckets could not be listed, so Block Public Access could not be verified.', + resourceType: 'aws-account', + resourceId: 'account', + severity: 'medium', + remediation: + 'Grant s3:ListAllMyBuckets (and s3:GetBucketPublicAccessBlock) to the integration role, then re-run the check.', + evidence: { error: err instanceof Error ? err.message : String(err) }, + }, + ]); return; } if (buckets.length === 0) return; diff --git a/packages/integration-platform/src/manifests/aws/checks/shared.ts b/packages/integration-platform/src/manifests/aws/checks/shared.ts index 2d014a2cec..a72a584ac6 100644 --- a/packages/integration-platform/src/manifests/aws/checks/shared.ts +++ b/packages/integration-platform/src/manifests/aws/checks/shared.ts @@ -228,17 +228,20 @@ export async function resolveAwsSessionOrFail( try { return await assumeAwsSession(ctx); } catch (err) { - ctx.fail({ - title: 'Could not assume AWS role', - description: - 'The cross-account IAM role could not be assumed, so this check could not be verified.', - resourceType: 'aws-account', - resourceId: 'account', - severity: 'medium', - remediation: - 'Verify the role ARN and external ID are correct and the role trust policy allows Comp to assume it, then re-run the check.', - evidence: { error: err instanceof Error ? err.message : String(err) }, - }); + emitOutcomes(ctx, [ + { + kind: 'fail', + title: 'Could not assume AWS role', + description: + 'The cross-account IAM role could not be assumed, so this check could not be verified.', + resourceType: 'aws-account', + resourceId: 'account', + severity: 'medium', + remediation: + 'Verify the role ARN and external ID are correct and the role trust policy allows Comp to assume it, then re-run the check.', + evidence: { error: err instanceof Error ? err.message : String(err) }, + }, + ]); return null; } } From 62fd6927be813c3592075ca801b77ae9f3f61d9b Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Wed, 10 Jun 2026 17:54:31 -0400 Subject: [PATCH 07/18] fix(integration-platform): isolate per-subscription wildcard scan + graceful subscription picker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses both cubic findings on this PR: - entra-id: the cross-subscription resolvedDefs cache leaked into each subscription's wildcard scan via allDefs — an MG wildcard role referenced only by subscription A was re-reported in every other subscription's scan. The shared cache now only dedupes fetches; the wildcard scan sees solely the definitions referenced by THIS subscription's assignments (subResolvedDefs). Test proves one finding + one fetch across two subscriptions. - azure manifest fetchOptions: a failed subscriptions list now returns an empty picker instead of throwing (matches the GCP project_ids precedent); scanning is unaffected either way. Co-Authored-By: Claude Fable 5 --- .../checks/__tests__/azure-checks.test.ts | 61 +++++++++++++++++++ .../src/manifests/azure/checks/entra-id.ts | 18 ++++-- .../src/manifests/azure/index.ts | 38 +++++++----- 3 files changed, 97 insertions(+), 20 deletions(-) diff --git a/packages/integration-platform/src/manifests/azure/checks/__tests__/azure-checks.test.ts b/packages/integration-platform/src/manifests/azure/checks/__tests__/azure-checks.test.ts index 1e2a3d330c..de03f189d3 100644 --- a/packages/integration-platform/src/manifests/azure/checks/__tests__/azure-checks.test.ts +++ b/packages/integration-platform/src/manifests/azure/checks/__tests__/azure-checks.test.ts @@ -4,6 +4,7 @@ import type { CheckVariableValues, IntegrationCheck, } from '../../../../types'; +import { azureManifest } from '../../index'; import { rbacLeastPrivilegeCheck } from '../entra-id'; import { keyVaultProtectionCheck, keyVaultRbacCheck } from '../key-vault'; import { monitorLoggingAlertingCheck } from '../monitor'; @@ -763,3 +764,63 @@ describe('Azure multi-subscription scanning', () => { expect(out.failed[0]!.title).toMatch(/Could not verify Azure subscription scope/); }); }); + +describe('entra-id multi-subscription wildcard isolation (cubic finding on #3090)', () => { + it('an MG wildcard role referenced only by one subscription is reported exactly once', async () => { + const MG_DEF_ID = '/providers/Microsoft.Management/managementGroups/mg1/providers/Microsoft.Authorization/roleDefinitions/wild'; + let mgDefFetches = 0; + const { failed } = await run( + rbacLeastPrivilegeCheck, + (url: string) => { + if (url.includes('/subscriptions?api-version')) { + return { + value: [ + { subscriptionId: 'sub-a', state: 'Enabled' }, + { subscriptionId: 'sub-b', state: 'Enabled' }, + ], + }; + } + if (url.startsWith(MG_DEF_ID)) { + mgDefFetches++; + return { + id: MG_DEF_ID, + properties: { + roleName: 'MG Wildcard', + type: 'CustomRole', + permissions: [{ actions: ['*'], dataActions: [] }], + }, + }; + } + if (url.includes('roleDefinitions')) { + return { value: [{ id: 'reader', properties: { roleName: 'Reader', type: 'BuiltInRole', permissions: [] } }] }; + } + if (url.includes('roleAssignments')) { + // only sub-a has an assignment referencing the MG wildcard role + return url.includes('sub-a') + ? { value: [{ properties: { roleDefinitionId: MG_DEF_ID, principalId: 'p1', principalType: 'User' } }] } + : { value: [] }; + } + return { value: [] }; + }, + {}, + ); + const wildcardFindings = failed.filter((f) => f.title.match(/[Ww]ildcard/)); + expect(wildcardFindings).toHaveLength(1); + // the shared cache still prevents refetching across subscriptions + expect(mgDefFetches).toBe(1); + }); +}); + +describe('azure subscription picker fetchOptions', () => { + it('returns [] instead of throwing when subscriptions cannot be listed', async () => { + const variable = azureManifest.variables?.find((v) => v.id === 'subscription_ids'); + expect(variable?.fetchOptions).toBeDefined(); + const ctx = { + fetch: async () => { + throw new Error('HTTP 403: Forbidden'); + }, + } as unknown as Parameters>[0]; + const options = await variable!.fetchOptions!(ctx); + expect(options).toEqual([]); + }); +}); diff --git a/packages/integration-platform/src/manifests/azure/checks/entra-id.ts b/packages/integration-platform/src/manifests/azure/checks/entra-id.ts index 1231b30662..f72c0cbf87 100644 --- a/packages/integration-platform/src/manifests/azure/checks/entra-id.ts +++ b/packages/integration-platform/src/manifests/azure/checks/entra-id.ts @@ -94,19 +94,29 @@ async function runRbacLeastPrivilegeForSubscription( // Assignments can reference role definitions scoped to a management group or // resource group, which won't appear in the subscription-scope list above. // Resolve any missing definition directly so privileged principals aren't - // undercounted. Cache by id to avoid refetching shared definitions. + // undercounted. The shared cross-subscription cache (resolvedDefs) only + // avoids refetching; the wildcard scan below must see ONLY definitions + // referenced by THIS subscription's assignments (subResolvedDefs), or a + // wildcard role from another subscription's loop would be re-reported here. const resolveFailures: ReadFailure[] = []; + const subResolvedDefs = new Map(); const resolveDef = async ( roleDefinitionId: string, ): Promise => { - const cached = defMap.get(roleDefinitionId) ?? resolvedDefs.get(roleDefinitionId); - if (cached) return cached; + const own = defMap.get(roleDefinitionId); + if (own) return own; + const shared = resolvedDefs.get(roleDefinitionId); + if (shared) { + subResolvedDefs.set(roleDefinitionId, shared); + return shared; + } try { const def = await ctx.fetch( `${roleDefinitionId}?api-version=2022-04-01`, ); if (def?.properties) { resolvedDefs.set(roleDefinitionId, def); + subResolvedDefs.set(roleDefinitionId, def); return def; } return null; @@ -203,7 +213,7 @@ async function runRbacLeastPrivilegeForSubscription( const allDefs = new Map( definitions.map((d) => [d.id, d]), ); - for (const [id, def] of resolvedDefs) allDefs.set(id, def); + for (const [id, def] of subResolvedDefs) allDefs.set(id, def); const wildcardRoles = [...allDefs.values()].filter( (d) => diff --git a/packages/integration-platform/src/manifests/azure/index.ts b/packages/integration-platform/src/manifests/azure/index.ts index 6655bfe558..d64260fbc7 100644 --- a/packages/integration-platform/src/manifests/azure/index.ts +++ b/packages/integration-platform/src/manifests/azure/index.ts @@ -104,22 +104,28 @@ Our integration only makes read-only API calls for security scanning.`, helpText: 'Select which subscriptions to scan. Leave empty to scan all enabled subscriptions.', fetchOptions: async (ctx) => { - const data = await ctx.fetch<{ - value?: Array<{ - subscriptionId: string; - displayName?: string; - state?: string; - }>; - }>('https://management.azure.com/subscriptions?api-version=2020-01-01'); - return (data.value ?? []) - .filter((s) => s.state === 'Enabled') - .sort((a, b) => (a.displayName ?? '').localeCompare(b.displayName ?? '')) - .map((s) => ({ - value: s.subscriptionId, - label: s.displayName - ? `${s.displayName} (${s.subscriptionId})` - : s.subscriptionId, - })); + try { + const data = await ctx.fetch<{ + value?: Array<{ + subscriptionId: string; + displayName?: string; + state?: string; + }>; + }>('https://management.azure.com/subscriptions?api-version=2020-01-01'); + return (data.value ?? []) + .filter((s) => s.state === 'Enabled') + .sort((a, b) => (a.displayName ?? '').localeCompare(b.displayName ?? '')) + .map((s) => ({ + value: s.subscriptionId, + label: s.displayName + ? `${s.displayName} (${s.subscriptionId})` + : s.subscriptionId, + })); + } catch { + // Graceful empty picker (matches the GCP project_ids precedent) — + // the user can still rely on scan-all or the legacy subscription_id. + return []; + } }, }, { From c1bce471fad128a1100f4fb142938ed27e343165 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Wed, 10 Jun 2026 18:11:21 -0400 Subject: [PATCH 08/18] fix(integration-platform): make multi-subscription scanning strictly opt-in MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Product decision: a deploy must never silently expand an existing customer's scan scope. The resolver now scans multiple subscriptions ONLY when the customer explicitly selects them in the subscription_ids picker (selecting all = explicit scan-everything). Without a selection, behavior is byte-identical to before the picker existed: the saved subscription_id, else the first Enabled subscription — with a log line pointing at the picker when more subscriptions are visible. Scope-failure findings, the picker, the Add-button fix, and the wildcard-scan isolation are unchanged. Co-Authored-By: Claude Fable 5 --- .../checks/__tests__/azure-checks.test.ts | 37 +++++++----- .../src/manifests/azure/checks/shared.ts | 58 +++++++++---------- .../src/manifests/azure/index.ts | 2 +- 3 files changed, 52 insertions(+), 45 deletions(-) diff --git a/packages/integration-platform/src/manifests/azure/checks/__tests__/azure-checks.test.ts b/packages/integration-platform/src/manifests/azure/checks/__tests__/azure-checks.test.ts index de03f189d3..dc3836c996 100644 --- a/packages/integration-platform/src/manifests/azure/checks/__tests__/azure-checks.test.ts +++ b/packages/integration-platform/src/manifests/azure/checks/__tests__/azure-checks.test.ts @@ -690,7 +690,7 @@ describe('Azure multi-subscription scanning', () => { ], }); - it('scans every Enabled subscription when none are selected (Disabled excluded)', async () => { + it('without a selection, keeps the pre-picker single-subscription behavior (first Enabled)', async () => { const seen: string[] = []; const out = await run( sqlTlsCheck, @@ -705,9 +705,28 @@ describe('Azure multi-subscription scanning', () => { }, {}, ); + // scope expansion is strictly opt-in: only the first Enabled sub is scanned + expect(seen).toEqual(['sub-a']); + expect(out.passed).toHaveLength(1); + expect(out.failed).toHaveLength(0); + }); + + it('scans multiple subscriptions ONLY when explicitly selected', async () => { + const seen: string[] = []; + const out = await run( + sqlTlsCheck, + (url: string) => { + const m = url.match(/subscriptions\/(sub-\w+)\/providers\/Microsoft.Sql\/servers\?/); + if (m) { + seen.push(m[1]!); + return serverIn(m[1]!); + } + return {}; + }, + { subscription_ids: ['sub-a', 'sub-b'] }, + ); expect(seen).toEqual(['sub-a', 'sub-b']); expect(out.passed).toHaveLength(2); - expect(out.failed).toHaveLength(0); }); it('scopes to the selected subscription_ids when set', async () => { @@ -728,13 +747,13 @@ describe('Azure multi-subscription scanning', () => { expect(out.passed).toHaveLength(1); }); - it('falls back to the legacy subscription_id when subscriptions cannot be listed', async () => { + it('a saved subscription_id keeps exactly its previous scope (no list call needed)', async () => { const seen: string[] = []; const out = await run( sqlTlsCheck, (url: string) => { if (url.includes('/subscriptions?api-version')) { - throw new Error('HTTP 403: Forbidden'); + throw new Error('must not list subscriptions when legacy value is set'); } const m = url.match(/subscriptions\/(sub-\w+)\/providers\/Microsoft.Sql\/servers\?/); if (m) { @@ -772,14 +791,6 @@ describe('entra-id multi-subscription wildcard isolation (cubic finding on #3090 const { failed } = await run( rbacLeastPrivilegeCheck, (url: string) => { - if (url.includes('/subscriptions?api-version')) { - return { - value: [ - { subscriptionId: 'sub-a', state: 'Enabled' }, - { subscriptionId: 'sub-b', state: 'Enabled' }, - ], - }; - } if (url.startsWith(MG_DEF_ID)) { mgDefFetches++; return { @@ -802,7 +813,7 @@ describe('entra-id multi-subscription wildcard isolation (cubic finding on #3090 } return { value: [] }; }, - {}, + { subscription_ids: ['sub-a', 'sub-b'] }, ); const wildcardFindings = failed.filter((f) => f.title.match(/[Ww]ildcard/)); expect(wildcardFindings).toHaveLength(1); diff --git a/packages/integration-platform/src/manifests/azure/checks/shared.ts b/packages/integration-platform/src/manifests/azure/checks/shared.ts index cfe6933dca..02fd2542c8 100644 --- a/packages/integration-platform/src/manifests/azure/checks/shared.ts +++ b/packages/integration-platform/src/manifests/azure/checks/shared.ts @@ -6,9 +6,9 @@ const ARM = 'https://management.azure.com'; /** Fan-out bound for auto-discovered subscriptions (13 checks × N subs). */ const MAX_SUBSCRIPTIONS = 50; -/** The legacy single-subscription variable, auto-saved by the Cloud Tests - * setup. It is a detection cache, not a deliberate scope choice, so the - * evidence checks only use it when subscriptions cannot be listed. */ +/** The legacy single-subscription variable (auto-saved by Cloud Tests + * detection or typed manually). Kept as the no-selection default so existing + * connections keep their exact pre-picker scan scope. */ function legacySubscriptionId(ctx: CheckContext): string | null { const configured = ctx.variables.subscription_id; return typeof configured === 'string' && configured.trim().length > 0 @@ -17,14 +17,16 @@ function legacySubscriptionId(ctx: CheckContext): string | null { } /** - * Resolve the Azure subscriptions a check should evaluate: the user-selected - * `subscription_ids` variable if present, otherwise ALL Enabled subscriptions - * the token can see (mirrors how GCP scans all active projects). Previously - * only the first Enabled subscription was scanned — silently skipping the - * rest in multi-subscription tenants. + * Resolve the Azure subscriptions a check should evaluate. * - * Returns [] only after emitting an explicit "could not verify" finding, so - * a scope failure never leaves the mapped tasks silently stale. + * Scanning MORE than one subscription is strictly opt-in: customers select + * subscriptions (one, several, or all) via the `subscription_ids` variable. + * Without a selection the behavior is identical to before the picker existed — + * the saved `subscription_id`, else the first Enabled subscription — so a + * deploy never silently expands an existing customer's scan scope. + * + * Returns [] only after emitting an explicit "could not verify" finding, so a + * scope failure never leaves the mapped tasks silently stale. */ export async function resolveAzureSubscriptionIds( ctx: CheckContext, @@ -35,10 +37,22 @@ export async function resolveAzureSubscriptionIds( .filter((s): s is string => typeof s === 'string') .map((s) => s.trim()) .filter((s) => s.length > 0); + if (cleaned.length > MAX_SUBSCRIPTIONS) { + // Bound the fan-out (13 checks x N subs) — loudly, so capped coverage + // is never mistaken for full coverage. + ctx.warn( + `Azure: ${cleaned.length} subscriptions selected; scanning the first ${MAX_SUBSCRIPTIONS}`, + { selected: cleaned.length, scanned: MAX_SUBSCRIPTIONS }, + ); + return cleaned.slice(0, MAX_SUBSCRIPTIONS); + } if (cleaned.length > 0) return cleaned; } + // No explicit selection: preserve the pre-picker behavior exactly. const legacy = legacySubscriptionId(ctx); + if (legacy) return [legacy]; + try { const data = await ctx.fetch<{ value?: Array<{ subscriptionId: string; state?: string }>; @@ -49,25 +63,13 @@ export async function resolveAzureSubscriptionIds( .filter((s) => s.state === 'Enabled') .map((s) => s.subscriptionId); if (enabled.length > 0) { - if (legacy && (enabled.length > 1 || enabled[0] !== legacy)) { + if (enabled.length > 1) { ctx.log( - `Azure: scanning all ${enabled.length} enabled subscription(s); set subscription_ids to scope explicitly (legacy subscription_id is no longer used for scoping)`, + `Azure: ${enabled.length} enabled subscriptions visible but none selected — scanning "${enabled[0]}" only. Select subscriptions in the integration settings to scan more.`, ); } - // Bound the fan-out so a giant tenant can't blow the run budget — - // loudly, so capped coverage is never mistaken for full coverage. - if (enabled.length > MAX_SUBSCRIPTIONS) { - ctx.warn( - `Azure: ${enabled.length} enabled subscriptions found; scanning the first ${MAX_SUBSCRIPTIONS} — set subscription_ids to scope explicitly`, - { enabled: enabled.length, scanned: MAX_SUBSCRIPTIONS }, - ); - return enabled.slice(0, MAX_SUBSCRIPTIONS); - } - return enabled; + return [enabled[0]!]; } - // Nothing visible via the list call — the token may lack list permission - // at root scope while still having Reader on one subscription. - if (legacy) return [legacy]; ctx.fail({ title: 'Could not verify Azure subscription scope', description: @@ -81,12 +83,6 @@ export async function resolveAzureSubscriptionIds( }); return []; } catch (err) { - if (legacy) { - ctx.log( - `Azure: could not list subscriptions (${err instanceof Error ? err.message : String(err)}); falling back to the detected subscription_id`, - ); - return [legacy]; - } const failure = toHttpReadFailure(err); ctx.fail({ title: 'Could not verify Azure subscription scope', diff --git a/packages/integration-platform/src/manifests/azure/index.ts b/packages/integration-platform/src/manifests/azure/index.ts index d64260fbc7..309058c679 100644 --- a/packages/integration-platform/src/manifests/azure/index.ts +++ b/packages/integration-platform/src/manifests/azure/index.ts @@ -102,7 +102,7 @@ Our integration only makes read-only API calls for security scanning.`, type: 'multi-select', required: false, helpText: - 'Select which subscriptions to scan. Leave empty to scan all enabled subscriptions.', + 'Select which subscriptions to scan (select all to scan everything). Leave empty to keep scanning the single auto-detected subscription.', fetchOptions: async (ctx) => { try { const data = await ctx.fetch<{ From e6dda8c14ba88536e8336b3a343c70c9f8e939c5 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Wed, 10 Jun 2026 18:14:41 -0400 Subject: [PATCH 09/18] fix(integration-platform): surface the subscription scan cap as an explicit finding Cubic flagged that capping at 50 subscriptions left the gap visible only in run logs. Scanning less than the customer explicitly selected must never be silent: the cap now emits a 'selection exceeds the scan limit' finding listing the unscanned subscription ids. Only reachable via an explicit >50 selection (no-selection default scans one). Co-Authored-By: Claude Fable 5 --- .../checks/__tests__/azure-checks.test.ts | 27 +++++++++++++++++++ .../src/manifests/azure/checks/shared.ts | 23 +++++++++++----- 2 files changed, 44 insertions(+), 6 deletions(-) diff --git a/packages/integration-platform/src/manifests/azure/checks/__tests__/azure-checks.test.ts b/packages/integration-platform/src/manifests/azure/checks/__tests__/azure-checks.test.ts index dc3836c996..0e7122e99f 100644 --- a/packages/integration-platform/src/manifests/azure/checks/__tests__/azure-checks.test.ts +++ b/packages/integration-platform/src/manifests/azure/checks/__tests__/azure-checks.test.ts @@ -784,6 +784,33 @@ describe('Azure multi-subscription scanning', () => { }); }); +describe('Azure subscription cap', () => { + it('selecting more than the limit scans the first 50 AND emits an explicit coverage finding', async () => { + const selected = Array.from({ length: 53 }, (_, i) => `sub-${i}`); + const seen = new Set(); + const out = await run( + sqlTlsCheck, + (url: string) => { + const m = url.match(/subscriptions\/(sub-\d+)\/providers\/Microsoft.Sql\/servers\?/); + if (m) { + seen.add(m[1]!); + return { value: [] }; + } + return {}; + }, + { subscription_ids: selected }, + ); + expect(seen.size).toBe(50); + const capFinding = out.failed.find((f) => f.title.includes('exceeds the scan limit')); + expect(capFinding).toBeDefined(); + expect(capFinding!.evidence).toMatchObject({ + selected: 53, + scanned: 50, + unscannedSubscriptionIds: ['sub-50', 'sub-51', 'sub-52'], + }); + }); +}); + describe('entra-id multi-subscription wildcard isolation (cubic finding on #3090)', () => { it('an MG wildcard role referenced only by one subscription is reported exactly once', async () => { const MG_DEF_ID = '/providers/Microsoft.Management/managementGroups/mg1/providers/Microsoft.Authorization/roleDefinitions/wild'; diff --git a/packages/integration-platform/src/manifests/azure/checks/shared.ts b/packages/integration-platform/src/manifests/azure/checks/shared.ts index 02fd2542c8..97bc02d2e0 100644 --- a/packages/integration-platform/src/manifests/azure/checks/shared.ts +++ b/packages/integration-platform/src/manifests/azure/checks/shared.ts @@ -38,12 +38,23 @@ export async function resolveAzureSubscriptionIds( .map((s) => s.trim()) .filter((s) => s.length > 0); if (cleaned.length > MAX_SUBSCRIPTIONS) { - // Bound the fan-out (13 checks x N subs) — loudly, so capped coverage - // is never mistaken for full coverage. - ctx.warn( - `Azure: ${cleaned.length} subscriptions selected; scanning the first ${MAX_SUBSCRIPTIONS}`, - { selected: cleaned.length, scanned: MAX_SUBSCRIPTIONS }, - ); + // Bound the fan-out (13 checks x N subs) to protect the run budget — + // and surface the gap as a FINDING: scanning less than the customer + // selected must never hide in a run log. + const unscanned = cleaned.slice(MAX_SUBSCRIPTIONS); + ctx.fail({ + title: `Subscription selection exceeds the scan limit (${cleaned.length} selected, ${MAX_SUBSCRIPTIONS} scanned)`, + description: `${unscanned.length} selected subscription(s) were not scanned because runs are limited to ${MAX_SUBSCRIPTIONS} subscriptions. Resources in the unscanned subscriptions are unverified.`, + resourceType: 'azure-subscription', + resourceId: 'subscription-scope', + severity: 'medium', + remediation: `Reduce the selection to at most ${MAX_SUBSCRIPTIONS} subscriptions, or contact support to raise the limit for this connection.`, + evidence: { + selected: cleaned.length, + scanned: MAX_SUBSCRIPTIONS, + unscannedSubscriptionIds: unscanned, + }, + }); return cleaned.slice(0, MAX_SUBSCRIPTIONS); } if (cleaned.length > 0) return cleaned; From 9f3013854bc2a86bffc35c57ad2fb9c570c117b0 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Wed, 10 Jun 2026 18:47:04 -0400 Subject: [PATCH 10/18] fix: resolve cubic findings from the production deploy review (#3087) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five findings from the main->release deploy review, all verified real: - read-failure.ts: OptInRequired arrives as HTTP 403, so the status check classified disabled regions as permission failures and pointed users at IAM instead of 'remove the region'. Region classification now wins over the status check. - s3.ts: the two account-level ListBuckets failures were the last AWS findings still asserting an unconditional IAM grant for transient errors — now gated via remediationForReadFailure with readError evidence, consistent with every other check. - azure picker: fetchOptions ignored ARM nextLink pagination, so large tenants could only select first-page subscriptions. Now follows nextLink (bounded, ARM-host-guarded). - ComplianceFramework.tsx: enabled/status state initialized from props but never re-synced — rows went stale after SWR revalidation. Now syncs on prop change while keeping optimistic updates. - trust-portal.controller.ts: PUT /custom-frameworks parses a body but had no @ApiBody, leaving the OpenAPI spec (and the generated MCP server) without the body schema. 199 integration-platform tests pass (4 new: OptInRequired-with-403 classification, picker pagination, picker host guard). Co-Authored-By: Claude Fable 5 --- .../trust-portal/trust-portal.controller.ts | 15 ++++++++ .../components/ComplianceFramework.tsx | 12 ++++++- .../aws/checks/__tests__/aws-checks.test.ts | 8 +++++ .../src/manifests/aws/checks/read-failure.ts | 21 ++++++------ .../src/manifests/aws/checks/s3.ts | 21 +++++++----- .../checks/__tests__/azure-checks.test.ts | 34 +++++++++++++++++++ .../src/manifests/azure/index.ts | 27 ++++++++++++--- 7 files changed, 115 insertions(+), 23 deletions(-) diff --git a/apps/api/src/trust-portal/trust-portal.controller.ts b/apps/api/src/trust-portal/trust-portal.controller.ts index 83c47e68ed..cb50f8a266 100644 --- a/apps/api/src/trust-portal/trust-portal.controller.ts +++ b/apps/api/src/trust-portal/trust-portal.controller.ts @@ -413,6 +413,21 @@ export class TrustPortalController { summary: 'Enable/disable a custom framework on the trust portal and set its status', }) + @ApiBody({ + description: 'At least one of `enabled` or `status` must be provided.', + schema: { + type: 'object', + required: ['customFrameworkId'], + properties: { + customFrameworkId: { type: 'string', minLength: 1 }, + enabled: { type: 'boolean' }, + status: { + type: 'string', + enum: ['started', 'in_progress', 'compliant'], + }, + }, + }, + }) async updateCustomFramework( @OrganizationId() organizationId: string, @Body() body: unknown, diff --git a/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/ComplianceFramework.tsx b/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/ComplianceFramework.tsx index b57cb88349..c76088acab 100644 --- a/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/ComplianceFramework.tsx +++ b/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/ComplianceFramework.tsx @@ -17,7 +17,7 @@ import { TooltipTrigger, } from '@trycompai/design-system'; import { CertificateCheck, Download, Upload, View } from '@trycompai/design-system/icons'; -import { useRef, useState } from 'react'; +import { useEffect, useRef, useState } from 'react'; import { toast } from 'sonner'; import { CCPA, @@ -141,6 +141,16 @@ export function ComplianceFramework({ }) { const [isEnabled, setIsEnabled] = useState(isEnabledProp); const [status, setStatus] = useState(statusProp); + + // State is optimistic-first, but must follow the parent's data when it + // refreshes (SWR revalidation, another tab/user) — otherwise the row shows + // stale enabled/status values forever. + useEffect(() => { + setIsEnabled(isEnabledProp); + }, [isEnabledProp]); + useEffect(() => { + setStatus(statusProp); + }, [statusProp]); const [isUploading, setIsUploading] = useState(false); const [isDragging, setIsDragging] = useState(false); const fileInputRef = useRef(null); diff --git a/packages/integration-platform/src/manifests/aws/checks/__tests__/aws-checks.test.ts b/packages/integration-platform/src/manifests/aws/checks/__tests__/aws-checks.test.ts index 1286692797..190da69c8b 100644 --- a/packages/integration-platform/src/manifests/aws/checks/__tests__/aws-checks.test.ts +++ b/packages/integration-platform/src/manifests/aws/checks/__tests__/aws-checks.test.ts @@ -483,6 +483,14 @@ describe('toReadFailure — read-error classification', () => { optIn.name = 'OptInRequired'; expect(toReadFailure(optIn)).toMatchObject({ denied: false, regionDisabled: true }); + // OptInRequired arrives as HTTP 403 in the real SDK — the status check + // must NOT win over the region classification (cubic finding on #3087) + const optIn403 = Object.assign(new Error('not subscribed to this region'), { + name: 'OptInRequired', + $metadata: { httpStatusCode: 403 }, + }); + expect(toReadFailure(optIn403)).toMatchObject({ denied: false, regionDisabled: true }); + const authFailure = new Error('AWS was not able to validate the provided access credentials'); authFailure.name = 'AuthFailure'; expect(toReadFailure(authFailure)).toMatchObject({ denied: false, regionDisabled: true }); diff --git a/packages/integration-platform/src/manifests/aws/checks/read-failure.ts b/packages/integration-platform/src/manifests/aws/checks/read-failure.ts index 70af629b0e..6217fe05ae 100644 --- a/packages/integration-platform/src/manifests/aws/checks/read-failure.ts +++ b/packages/integration-platform/src/manifests/aws/checks/read-failure.ts @@ -28,18 +28,19 @@ export function toReadFailure(err: unknown): ReadFailure { : String(err).slice(0, 300); const status = (err as { $metadata?: { httpStatusCode?: number } } | null) ?.$metadata?.httpStatusCode; - const denied = - status === 403 || - (err instanceof Error && - /AccessDenied|UnauthorizedOperation|Forbidden|NotAuthorized/i.test( - err.name, - )); // OptInRequired / AuthFailure are what opted-out or disabled regions throw — - // a permanent condition for this connection, not something a re-run fixes. + // a permanent condition for this connection, not something a re-run or an + // IAM grant fixes. Classified BEFORE denied: OptInRequired arrives as HTTP + // 403, so the status check alone would mislabel it as a permission gap. const regionDisabled = - !denied && - err instanceof Error && - /OptInRequired|AuthFailure/i.test(err.name); + err instanceof Error && /OptInRequired|AuthFailure/i.test(err.name); + const denied = + !regionDisabled && + (status === 403 || + (err instanceof Error && + /AccessDenied|UnauthorizedOperation|Forbidden|NotAuthorized/i.test( + err.name, + ))); return { error, denied, regionDisabled }; } diff --git a/packages/integration-platform/src/manifests/aws/checks/s3.ts b/packages/integration-platform/src/manifests/aws/checks/s3.ts index 57c96b9fe0..548b7a362f 100644 --- a/packages/integration-platform/src/manifests/aws/checks/s3.ts +++ b/packages/integration-platform/src/manifests/aws/checks/s3.ts @@ -14,6 +14,7 @@ import { awsAccountIdFromCtx, remediationForReadFailure, resolveAwsSessionOrFail, + toReadFailure, type CheckOutcome, emitOutcomes, } from './shared'; @@ -153,18 +154,20 @@ export const s3EncryptionCheck: IntegrationCheck = { clientForRegion, }); } catch (err) { + const failure = toReadFailure(err); emitOutcomes(ctx, [ { kind: 'fail', title: 'Could not verify S3 encryption', - description: - 'S3 buckets could not be listed, so default encryption could not be verified.', + description: `S3 buckets could not be listed (${failure.error}), so default encryption could not be verified.`, resourceType: 'aws-account', resourceId: 'account', severity: 'medium', - remediation: + remediation: remediationForReadFailure( + failure, 'Grant s3:ListAllMyBuckets (and s3:GetEncryptionConfiguration) to the integration role, then re-run the check.', - evidence: { error: err instanceof Error ? err.message : String(err) }, + ), + evidence: { readError: failure.error }, }, ]); return; @@ -225,18 +228,20 @@ export const s3PublicAccessCheck: IntegrationCheck = { clientForRegion, }); } catch (err) { + const failure = toReadFailure(err); emitOutcomes(ctx, [ { kind: 'fail', title: 'Could not verify S3 public access', - description: - 'S3 buckets could not be listed, so Block Public Access could not be verified.', + description: `S3 buckets could not be listed (${failure.error}), so Block Public Access could not be verified.`, resourceType: 'aws-account', resourceId: 'account', severity: 'medium', - remediation: + remediation: remediationForReadFailure( + failure, 'Grant s3:ListAllMyBuckets (and s3:GetBucketPublicAccessBlock) to the integration role, then re-run the check.', - evidence: { error: err instanceof Error ? err.message : String(err) }, + ), + evidence: { readError: failure.error }, }, ]); return; diff --git a/packages/integration-platform/src/manifests/azure/checks/__tests__/azure-checks.test.ts b/packages/integration-platform/src/manifests/azure/checks/__tests__/azure-checks.test.ts index 0e7122e99f..6a3c2fc6c3 100644 --- a/packages/integration-platform/src/manifests/azure/checks/__tests__/azure-checks.test.ts +++ b/packages/integration-platform/src/manifests/azure/checks/__tests__/azure-checks.test.ts @@ -850,6 +850,40 @@ describe('entra-id multi-subscription wildcard isolation (cubic finding on #3090 }); describe('azure subscription picker fetchOptions', () => { + it('follows nextLink so every subscription page is selectable', async () => { + const variable = azureManifest.variables?.find((v) => v.id === 'subscription_ids'); + const ctx = { + fetch: async (url: string) => { + if (url.includes('skiptoken')) { + return { value: [{ subscriptionId: 'sub-2', displayName: 'B', state: 'Enabled' }] }; + } + return { + value: [{ subscriptionId: 'sub-1', displayName: 'A', state: 'Enabled' }], + nextLink: 'https://management.azure.com/subscriptions?api-version=2020-01-01&skiptoken=x', + }; + }, + } as unknown as Parameters>[0]; + const options = await variable!.fetchOptions!(ctx); + expect(options.map((o) => o.value)).toEqual(['sub-1', 'sub-2']); + }); + + it('does not follow a nextLink that leaves the ARM host', async () => { + const variable = azureManifest.variables?.find((v) => v.id === 'subscription_ids'); + const fetched: string[] = []; + const ctx = { + fetch: async (url: string) => { + fetched.push(url); + return { + value: [{ subscriptionId: 'sub-1', displayName: 'A', state: 'Enabled' }], + nextLink: 'https://evil.example.com/subscriptions', + }; + }, + } as unknown as Parameters>[0]; + const options = await variable!.fetchOptions!(ctx); + expect(fetched).toHaveLength(1); + expect(options.map((o) => o.value)).toEqual(['sub-1']); + }); + it('returns [] instead of throwing when subscriptions cannot be listed', async () => { const variable = azureManifest.variables?.find((v) => v.id === 'subscription_ids'); expect(variable?.fetchOptions).toBeDefined(); diff --git a/packages/integration-platform/src/manifests/azure/index.ts b/packages/integration-platform/src/manifests/azure/index.ts index 309058c679..57e41328fa 100644 --- a/packages/integration-platform/src/manifests/azure/index.ts +++ b/packages/integration-platform/src/manifests/azure/index.ts @@ -105,14 +105,33 @@ Our integration only makes read-only API calls for security scanning.`, 'Select which subscriptions to scan (select all to scan everything). Leave empty to keep scanning the single auto-detected subscription.', fetchOptions: async (ctx) => { try { - const data = await ctx.fetch<{ + type SubsPage = { value?: Array<{ subscriptionId: string; displayName?: string; state?: string; }>; - }>('https://management.azure.com/subscriptions?api-version=2020-01-01'); - return (data.value ?? []) + nextLink?: string; + }; + const subs: NonNullable = []; + // ARM paginates via nextLink — follow it (bounded) so large tenants + // can see and select every subscription, not just the first page. + let url: string | undefined = + 'https://management.azure.com/subscriptions?api-version=2020-01-01'; + let pages = 0; + while (url && pages < 10) { + const data: SubsPage = await ctx.fetch(url); + subs.push(...(data.value ?? [])); + // only follow nextLink on the ARM host, so the bearer token can't + // be sent elsewhere + url = + data.nextLink && + data.nextLink.startsWith('https://management.azure.com/') + ? data.nextLink + : undefined; + pages++; + } + return subs .filter((s) => s.state === 'Enabled') .sort((a, b) => (a.displayName ?? '').localeCompare(b.displayName ?? '')) .map((s) => ({ @@ -123,7 +142,7 @@ Our integration only makes read-only API calls for security scanning.`, })); } catch { // Graceful empty picker (matches the GCP project_ids precedent) — - // the user can still rely on scan-all or the legacy subscription_id. + // the user can still rely on the saved subscription_id default. return []; } }, From c39011fe803c01855558361b8b658fb028545cb1 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Wed, 10 Jun 2026 18:54:17 -0400 Subject: [PATCH 11/18] fix: guard optimistic state sync and align picker page cap (cubic on #3095) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - ComplianceFramework: single sync effect, skipped while a mutation is in flight — a concurrent SWR revalidation can no longer clobber an optimistic toggle/status value before the request settles - azure subscription picker: page cap raised 10 -> 50 to match the armListAll precedent (the cap is a malformed-nextLink loop guard, not a coverage budget; 50 pages covers thousands of subscriptions) Co-Authored-By: Claude Fable 5 --- .../components/ComplianceFramework.tsx | 16 ++++++++++++---- .../src/manifests/azure/index.ts | 7 ++++--- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/ComplianceFramework.tsx b/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/ComplianceFramework.tsx index c76088acab..a30d3384ca 100644 --- a/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/ComplianceFramework.tsx +++ b/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/ComplianceFramework.tsx @@ -144,13 +144,15 @@ export function ComplianceFramework({ // State is optimistic-first, but must follow the parent's data when it // refreshes (SWR revalidation, another tab/user) — otherwise the row shows - // stale enabled/status values forever. + // stale enabled/status values forever. Single guarded sync: skipped while a + // mutation is in flight so a concurrent revalidation can't clobber the + // optimistic value before the request settles. + const mutationInFlightRef = useRef(false); useEffect(() => { + if (mutationInFlightRef.current) return; setIsEnabled(isEnabledProp); - }, [isEnabledProp]); - useEffect(() => { setStatus(statusProp); - }, [statusProp]); + }, [isEnabledProp, statusProp]); const [isUploading, setIsUploading] = useState(false); const [isDragging, setIsDragging] = useState(false); const fileInputRef = useRef(null); @@ -250,10 +252,13 @@ export function ComplianceFramework({ if (!value) return; const prev = status; setStatus(value); + mutationInFlightRef.current = true; try { await onStatusChange(value); } catch { setStatus(prev); + } finally { + mutationInFlightRef.current = false; } }} > @@ -302,10 +307,13 @@ export function ComplianceFramework({ checked={isEnabled} onCheckedChange={async (checked) => { setIsEnabled(checked); + mutationInFlightRef.current = true; try { await onToggle(checked); } catch { setIsEnabled(!checked); + } finally { + mutationInFlightRef.current = false; } }} /> diff --git a/packages/integration-platform/src/manifests/azure/index.ts b/packages/integration-platform/src/manifests/azure/index.ts index 57e41328fa..f8eb4255e7 100644 --- a/packages/integration-platform/src/manifests/azure/index.ts +++ b/packages/integration-platform/src/manifests/azure/index.ts @@ -114,12 +114,13 @@ Our integration only makes read-only API calls for security scanning.`, nextLink?: string; }; const subs: NonNullable = []; - // ARM paginates via nextLink — follow it (bounded) so large tenants - // can see and select every subscription, not just the first page. + // ARM paginates via nextLink — follow it so large tenants can see + // and select every subscription. The page cap matches armListAll's + // (a loop guard against malformed nextLink chains, not a budget). let url: string | undefined = 'https://management.azure.com/subscriptions?api-version=2020-01-01'; let pages = 0; - while (url && pages < 10) { + while (url && pages < 50) { const data: SubsPage = await ctx.fetch(url); subs.push(...(data.value ?? [])); // only follow nextLink on the ARM host, so the bearer token can't From 7d51e2cce240395bccf2e42a19843a6486fc9713 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Wed, 10 Jun 2026 19:07:53 -0400 Subject: [PATCH 12/18] fix(trust): reset the certificate file input on every selection, not only success MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cubic flagged on the deploy review that a failed upload left the file input value set — browsers skip onChange when the same file is re-selected, so retrying the same file dead-ended. The validation rejections (non-PDF, oversized) had the same problem. The input now resets in onChange itself, covering success, failure, and rejection in one place. Co-Authored-By: Claude Fable 5 --- .../portal-settings/components/ComplianceFramework.tsx | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/ComplianceFramework.tsx b/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/ComplianceFramework.tsx index a30d3384ca..3237a35244 100644 --- a/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/ComplianceFramework.tsx +++ b/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/ComplianceFramework.tsx @@ -175,9 +175,6 @@ export function ComplianceFramework({ try { await onFileUpload(file, frameworkKey); toast.success('Certificate uploaded successfully'); - if (fileInputRef.current) { - fileInputRef.current.value = ''; - } } catch (error) { const message = error instanceof Error ? error.message : 'Failed to upload certificate'; toast.error(message); @@ -330,6 +327,10 @@ export function ComplianceFramework({ className="hidden" onChange={async (e) => { const file = e.target.files?.[0]; + // Always reset the input: browsers skip onChange when the + // same file is re-selected, which would block retrying after + // a failed upload or a rejected (non-PDF/too-large) file. + e.target.value = ''; if (file) { await processFile(file); } From 3e3ed70d59af17a3189065804f33e4ef20aaee67 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Wed, 10 Jun 2026 19:17:33 -0400 Subject: [PATCH 13/18] feat(trust-portal): add per-email NDA-bypass allowlist Adds Trust.allowedEmails alongside the existing allowedDomains so admins can grant a specific individual trust portal access without re-signing an NDA (e.g. one already signed outside Comp), without bypassing the NDA for that contact's entire domain. - schema: Trust.allowedEmails String[] + migration - api: approveRequest skips the NDA when the requester's email OR domain is allow-listed (admin approval still required); audit log records bypassReason; PUT /v1/trust-portal/settings/allowed-emails - app: AllowedEmailsManager card (design-system) in Trust settings - tests: service bypass branches, controller endpoint, component tests Allowlist is evaluated only at admin approval; no change to the visitor portal or submission flow. comp-only. Co-Authored-By: Claude Fable 5 --- .../trust-portal/trust-access.service.spec.ts | 137 ++++++++++++ .../src/trust-portal/trust-access.service.ts | 40 +++- .../trust-portal.controller.spec.ts | 21 ++ .../trust-portal/trust-portal.controller.ts | 13 ++ .../src/trust-portal/trust-portal.service.ts | 15 ++ .../components/AllowedEmailsManager.test.tsx | 107 +++++++++ .../components/AllowedEmailsManager.tsx | 206 ++++++++++++++++++ .../components/TrustSettingsClient.test.tsx | 3 + .../components/TrustSettingsClient.tsx | 6 + .../app/(app)/[orgId]/trust/settings/page.tsx | 2 + .../src/hooks/use-trust-portal-settings.ts | 10 + .../migration.sql | 2 + packages/db/prisma/schema/trust.prisma | 3 + 13 files changed, 558 insertions(+), 7 deletions(-) create mode 100644 apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/AllowedEmailsManager.test.tsx create mode 100644 apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/AllowedEmailsManager.tsx create mode 100644 packages/db/prisma/migrations/20260610225432_add_trust_allowed_emails/migration.sql diff --git a/apps/api/src/trust-portal/trust-access.service.spec.ts b/apps/api/src/trust-portal/trust-access.service.spec.ts index b67329857f..87fb26a09d 100644 --- a/apps/api/src/trust-portal/trust-access.service.spec.ts +++ b/apps/api/src/trust-portal/trust-access.service.spec.ts @@ -15,6 +15,13 @@ jest.mock('@db', () => ({ trustAccessGrant: { findUnique: jest.fn(), }, + trustAccessRequest: { + findFirst: jest.fn(), + }, + member: { + findFirst: jest.fn(), + }, + $transaction: jest.fn(), }, Prisma: { PrismaClientKnownRequestError: class PrismaClientKnownRequestError extends Error { @@ -56,6 +63,13 @@ const mockDb = db as unknown as { trustAccessGrant: { findUnique: jest.Mock; }; + trustAccessRequest: { + findFirst: jest.Mock; + }; + member: { + findFirst: jest.Mock; + }; + $transaction: jest.Mock; }; const mockGetSignedUrl = getSignedUrl as jest.MockedFunction< @@ -173,3 +187,126 @@ describe('TrustAccessService favicon branding', () => { expect(result.portalUrl).toContain('/acme-security'); }); }); + +describe('TrustAccessService approveRequest NDA bypass', () => { + const emailService = { + sendAccessGrantedEmail: jest.fn(), + sendNdaSigningEmail: jest.fn(), + }; + const service = new TrustAccessService( + {} as any, + emailService as any, + {} as any, + {} as any, + {} as any, + ); + const buildPortalAccessUrlSpy = jest.spyOn( + service as any, + 'buildPortalAccessUrl', + ); + + const baseRequest = { + id: 'tar_1', + status: 'under_review', + email: 'chang.liu@client.com', + name: 'Chang Liu', + requestedDurationDays: 30, + organization: { name: 'Acme Security' }, + }; + + let txMock: { + trustAccessRequest: { update: jest.Mock }; + trustAccessGrant: { create: jest.Mock }; + trustNDAAgreement: { create: jest.Mock }; + auditLog: { create: jest.Mock }; + }; + + beforeEach(() => { + jest.clearAllMocks(); + txMock = { + trustAccessRequest: { + update: jest + .fn() + .mockResolvedValue({ id: 'tar_1', status: 'approved' }), + }, + trustAccessGrant: { + create: jest + .fn() + .mockResolvedValue({ id: 'tag_1', expiresAt: new Date() }), + }, + trustNDAAgreement: { + create: jest + .fn() + .mockResolvedValue({ id: 'tna_1', signToken: 'sign-token' }), + }, + auditLog: { create: jest.fn().mockResolvedValue({}) }, + }; + mockDb.trustAccessRequest.findFirst.mockResolvedValue(baseRequest); + mockDb.member.findFirst.mockResolvedValue({ id: 'mem_1', userId: 'usr_1' }); + mockDb.$transaction.mockImplementation( + (cb: (tx: typeof txMock) => Promise) => cb(txMock), + ); + buildPortalAccessUrlSpy.mockResolvedValue( + 'https://portal.example.com/access/token', + ); + }); + + it('bypasses NDA when the exact email is allow-listed', async () => { + mockDb.trust.findUnique.mockResolvedValue({ + allowedDomains: [], + allowedEmails: ['chang.liu@client.com'], + }); + + const result = await service.approveRequest('org_1', 'tar_1', {}, 'mem_1'); + + expect(txMock.trustAccessGrant.create).toHaveBeenCalledTimes(1); + expect(txMock.trustNDAAgreement.create).not.toHaveBeenCalled(); + expect(emailService.sendAccessGrantedEmail).toHaveBeenCalledTimes(1); + expect(emailService.sendNdaSigningEmail).not.toHaveBeenCalled(); + expect(txMock.auditLog.create).toHaveBeenCalledWith( + expect.objectContaining({ + data: expect.objectContaining({ + data: expect.objectContaining({ + ndaBypassed: true, + bypassReason: 'allowed email', + }), + }), + }), + ); + expect(result.message).toBe('Access granted'); + }); + + it('bypasses NDA via domain match and records the domain reason', async () => { + mockDb.trust.findUnique.mockResolvedValue({ + allowedDomains: ['client.com'], + allowedEmails: [], + }); + + await service.approveRequest('org_1', 'tar_1', {}, 'mem_1'); + + expect(txMock.trustAccessGrant.create).toHaveBeenCalledTimes(1); + expect(emailService.sendAccessGrantedEmail).toHaveBeenCalledTimes(1); + expect(txMock.auditLog.create).toHaveBeenCalledWith( + expect.objectContaining({ + data: expect.objectContaining({ + data: expect.objectContaining({ bypassReason: 'allowed domain' }), + }), + }), + ); + }); + + it('requires NDA signing when neither email nor domain is allow-listed', async () => { + mockDb.trust.findUnique.mockResolvedValue({ + allowedDomains: ['other.com'], + allowedEmails: ['someone@else.com'], + }); + + const result = await service.approveRequest('org_1', 'tar_1', {}, 'mem_1'); + + expect(txMock.trustNDAAgreement.create).toHaveBeenCalledTimes(1); + expect(txMock.trustAccessGrant.create).not.toHaveBeenCalled(); + expect(emailService.sendNdaSigningEmail).toHaveBeenCalledTimes(1); + expect(emailService.sendAccessGrantedEmail).not.toHaveBeenCalled(); + expect(result.message).toBe('NDA signing email sent'); + }); +}); diff --git a/apps/api/src/trust-portal/trust-access.service.ts b/apps/api/src/trust-portal/trust-access.service.ts index 657f67b430..7f9f3dd1dd 100644 --- a/apps/api/src/trust-portal/trust-access.service.ts +++ b/apps/api/src/trust-portal/trust-access.service.ts @@ -533,6 +533,24 @@ export class TrustAccessService { ); } + /** + * Check if the email address is in the allow list (bypasses NDA requirement) + */ + private isEmailInAllowList(email: string, allowedEmails: string[]): boolean { + if (!allowedEmails || allowedEmails.length === 0) { + return false; + } + + const normalizedEmail = email.toLowerCase().trim(); + if (!normalizedEmail) { + return false; + } + + return allowedEmails.some( + (allowed) => allowed.toLowerCase().trim() === normalizedEmail, + ); + } + async approveRequest( organizationId: string, requestId: string, @@ -573,25 +591,30 @@ export class TrustAccessService { throw new BadRequestException('Invalid member ID'); } - // Check if email domain is in the allow list + // Check if the email or its domain is in the allow list const trust = await db.trust.findUnique({ where: { organizationId }, - select: { allowedDomains: true }, + select: { allowedDomains: true, allowedEmails: true }, }); const isAllowedDomain = this.isDomainInAllowList( request.email, trust?.allowedDomains ?? [], ); + const isAllowedEmail = this.isEmailInAllowList( + request.email, + trust?.allowedEmails ?? [], + ); - // If domain is in allow list, skip NDA and grant access directly - if (isAllowedDomain) { + // If the email or domain is in the allow list, skip NDA and grant access directly + if (isAllowedDomain || isAllowedEmail) { return this.approveWithoutNda({ organizationId, requestId, request, member, durationDays, + bypassReason: isAllowedEmail ? 'allowed email' : 'allowed domain', }); } @@ -657,7 +680,7 @@ export class TrustAccessService { } /** - * Approve request without NDA for allowed domains - grants immediate access + * Approve request without NDA for allowlisted domains/emails - grants immediate access */ private async approveWithoutNda({ organizationId, @@ -665,6 +688,7 @@ export class TrustAccessService { request, member, durationDays, + bypassReason, }: { organizationId: string; requestId: string; @@ -675,6 +699,7 @@ export class TrustAccessService { }; member: { id: string; userId: string }; durationDays: number; + bypassReason: 'allowed domain' | 'allowed email'; }) { const expiresAt = new Date(); expiresAt.setDate(expiresAt.getDate() + durationDays); @@ -712,12 +737,13 @@ export class TrustAccessService { memberId: member.id, entityType: 'trust', entityId: requestId, - description: `Access request approved for ${request.email} (allowed domain - NDA bypassed)`, + description: `Access request approved for ${request.email} (${bypassReason} - NDA bypassed)`, data: { requestId, grantId: grant.id, durationDays, ndaBypassed: true, + bypassReason, }, }, }); @@ -742,7 +768,7 @@ export class TrustAccessService { return { request: result.request, grant: result.grant, - message: 'Access granted', // NDA bypassed for allowed domain + message: 'Access granted', // NDA bypassed for allowlisted domain/email }; } diff --git a/apps/api/src/trust-portal/trust-portal.controller.spec.ts b/apps/api/src/trust-portal/trust-portal.controller.spec.ts index 2bdbefaa21..5dbd0bc1da 100644 --- a/apps/api/src/trust-portal/trust-portal.controller.spec.ts +++ b/apps/api/src/trust-portal/trust-portal.controller.spec.ts @@ -39,6 +39,7 @@ describe('TrustPortalController', () => { checkDnsRecords: jest.fn(), updateFaqs: jest.fn(), updateAllowedDomains: jest.fn(), + updateAllowedEmails: jest.fn(), updateFrameworks: jest.fn(), updateOverview: jest.fn(), getOverview: jest.fn(), @@ -431,6 +432,26 @@ describe('TrustPortalController', () => { }); }); + describe('updateAllowedEmails', () => { + it('should call service.updateAllowedEmails with organizationId and emails', async () => { + const emails = ['person@example.com', 'other@test.com']; + mockService.updateAllowedEmails.mockResolvedValue({ success: true }); + + const result = await controller.updateAllowedEmails(orgId, { emails }); + + expect(result).toEqual({ success: true }); + expect(service.updateAllowedEmails).toHaveBeenCalledWith(orgId, emails); + }); + + it('should default to empty array when emails is undefined', async () => { + mockService.updateAllowedEmails.mockResolvedValue({ success: true }); + + await controller.updateAllowedEmails(orgId, {} as any); + + expect(service.updateAllowedEmails).toHaveBeenCalledWith(orgId, []); + }); + }); + describe('updateFrameworks', () => { it('should call service.updateFrameworks with organizationId and body', async () => { const body = { SOC2: true, ISO27001: false }; diff --git a/apps/api/src/trust-portal/trust-portal.controller.ts b/apps/api/src/trust-portal/trust-portal.controller.ts index 83c47e68ed..50288ca8bd 100644 --- a/apps/api/src/trust-portal/trust-portal.controller.ts +++ b/apps/api/src/trust-portal/trust-portal.controller.ts @@ -386,6 +386,19 @@ export class TrustPortalController { ); } + @Put('settings/allowed-emails') + @RequirePermission('trust', 'update') + @ApiOperation({ summary: 'Update allowed emails for the trust portal' }) + async updateAllowedEmails( + @OrganizationId() organizationId: string, + @Body() body: { emails: string[] }, + ) { + return this.trustPortalService.updateAllowedEmails( + organizationId, + body.emails ?? [], + ); + } + @Put('settings/frameworks') @RequirePermission('trust', 'update') @ApiOperation({ summary: 'Update trust portal framework settings' }) diff --git a/apps/api/src/trust-portal/trust-portal.service.ts b/apps/api/src/trust-portal/trust-portal.service.ts index 616323a7ae..f58099f8a1 100644 --- a/apps/api/src/trust-portal/trust-portal.service.ts +++ b/apps/api/src/trust-portal/trust-portal.service.ts @@ -630,6 +630,20 @@ export class TrustPortalService { return { success: true }; } + async updateAllowedEmails(organizationId: string, emails: string[]) { + const normalizedEmails = [ + ...new Set(emails.map((e) => e.toLowerCase().trim())), + ]; + + await db.trust.upsert({ + where: { organizationId }, + update: { allowedEmails: normalizedEmails }, + create: { organizationId, allowedEmails: normalizedEmails }, + }); + + return { success: true }; + } + async updateFrameworks( organizationId: string, frameworks: Record, @@ -1682,6 +1696,7 @@ export class TrustPortalService { vercelVerification: trust.vercelVerification ?? null, contactEmail: trust.contactEmail ?? null, allowedDomains: trust.allowedDomains ?? [], + allowedEmails: trust.allowedEmails ?? [], // Framework flags soc2type1: trust.soc2type1 ?? false, soc2type2: trust.soc2type2 || trust.soc2 || false, diff --git a/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/AllowedEmailsManager.test.tsx b/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/AllowedEmailsManager.test.tsx new file mode 100644 index 0000000000..30be76f87e --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/AllowedEmailsManager.test.tsx @@ -0,0 +1,107 @@ +import { + ADMIN_PERMISSIONS, + AUDITOR_PERMISSIONS, + mockHasPermission, + setMockPermissions, +} from '@/test-utils/mocks/permissions'; +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const { updateAllowedEmails } = vi.hoisted(() => ({ + updateAllowedEmails: vi.fn(), +})); + +vi.mock('@/hooks/use-permissions', () => ({ + usePermissions: () => ({ + permissions: {}, + hasPermission: mockHasPermission, + }), +})); + +vi.mock('@/hooks/use-trust-portal-settings', () => ({ + useTrustPortalSettings: () => ({ updateAllowedEmails }), +})); + +vi.mock('sonner', () => ({ + toast: { success: vi.fn(), error: vi.fn() }, +})); + +import { toast } from 'sonner'; +import { AllowedEmailsManager } from './AllowedEmailsManager'; + +const PLACEHOLDER = 'person@example.com'; + +describe('AllowedEmailsManager', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('renders the card title and description', () => { + setMockPermissions(ADMIN_PERMISSIONS); + render(); + + expect(screen.getByText('NDA Bypass - Allowed Emails')).toBeInTheDocument(); + expect( + screen.getByText( + 'Individual email addresses that bypass NDA signing for trust portal access', + ), + ).toBeInTheDocument(); + }); + + it('renders existing emails as chips', () => { + setMockPermissions(ADMIN_PERMISSIONS); + render( + , + ); + + expect(screen.getByText('chang@client.com')).toBeInTheDocument(); + }); + + it('enables the input for users with trust:update permission', () => { + setMockPermissions(ADMIN_PERMISSIONS); + render(); + + expect(screen.getByPlaceholderText(PLACEHOLDER)).not.toBeDisabled(); + }); + + it('disables the input for read-only users', () => { + setMockPermissions(AUDITOR_PERMISSIONS); + render(); + + expect(screen.getByPlaceholderText(PLACEHOLDER)).toBeDisabled(); + }); + + it('saves a valid email, normalizing case and whitespace', async () => { + setMockPermissions(ADMIN_PERMISSIONS); + render(); + + fireEvent.change(screen.getByPlaceholderText(PLACEHOLDER), { + target: { value: ' Chang.Liu@Client.com ' }, + }); + fireEvent.keyDown(screen.getByPlaceholderText(PLACEHOLDER), { + key: 'Enter', + }); + + await waitFor(() => + expect(updateAllowedEmails).toHaveBeenCalledWith([ + 'chang.liu@client.com', + ]), + ); + expect(toast.success).toHaveBeenCalledWith('Allowed emails updated'); + }); + + it('rejects an invalid email and does not save', () => { + setMockPermissions(ADMIN_PERMISSIONS); + render(); + + fireEvent.change(screen.getByPlaceholderText(PLACEHOLDER), { + target: { value: 'not-an-email' }, + }); + fireEvent.keyDown(screen.getByPlaceholderText(PLACEHOLDER), { + key: 'Enter', + }); + + expect(screen.getByText(/invalid email format/i)).toBeInTheDocument(); + expect(updateAllowedEmails).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/AllowedEmailsManager.tsx b/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/AllowedEmailsManager.tsx new file mode 100644 index 0000000000..ba6ba5fc2b --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/AllowedEmailsManager.tsx @@ -0,0 +1,206 @@ +'use client'; + +import { usePermissions } from '@/hooks/use-permissions'; +import { useTrustPortalSettings } from '@/hooks/use-trust-portal-settings'; +import { + AlertDialog, + AlertDialogAction, + AlertDialogCancel, + AlertDialogContent, + AlertDialogDescription, + AlertDialogFooter, + AlertDialogHeader, + AlertDialogTitle, + Badge, + Button, + Card, + CardContent, + CardDescription, + CardHeader, + CardTitle, + Input, + Tooltip, + TooltipContent, + TooltipProvider, + TooltipTrigger, +} from '@trycompai/design-system'; +import { Add, Close, Information } from '@trycompai/design-system/icons'; +import { useState } from 'react'; +import { toast } from 'sonner'; + +interface AllowedEmailsManagerProps { + initialEmails: string[]; + orgId: string; +} + +const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; + +export function AllowedEmailsManager({ initialEmails }: AllowedEmailsManagerProps) { + const { hasPermission } = usePermissions(); + const canUpdate = hasPermission('trust', 'update'); + const { updateAllowedEmails } = useTrustPortalSettings(); + const [emails, setEmails] = useState(initialEmails); + const [lastSavedEmails, setLastSavedEmails] = useState(initialEmails); + const [newEmail, setNewEmail] = useState(''); + const [error, setError] = useState(null); + const [emailToDelete, setEmailToDelete] = useState(null); + const [isUpdating, setIsUpdating] = useState(false); + + const saveEmails = async (updatedEmails: string[]) => { + setIsUpdating(true); + try { + await updateAllowedEmails(updatedEmails); + toast.success('Allowed emails updated'); + setLastSavedEmails(updatedEmails); + } catch { + toast.error('Failed to update allowed emails'); + setEmails(lastSavedEmails); + } finally { + setIsUpdating(false); + } + }; + + const handleAddEmail = () => { + setError(null); + const normalized = newEmail.toLowerCase().trim(); + + if (!normalized) { + setError('Please enter an email address'); + return; + } + + if (!emailRegex.test(normalized)) { + setError('Invalid email format (e.g., person@example.com)'); + return; + } + + if (emails.includes(normalized)) { + setError('Email already in list'); + return; + } + + const updatedEmails = [...emails, normalized]; + setEmails(updatedEmails); + setNewEmail(''); + saveEmails(updatedEmails); + }; + + const handleRemoveEmail = (emailToRemove: string) => { + const updatedEmails = emails.filter((email) => email !== emailToRemove); + setEmails(updatedEmails); + saveEmails(updatedEmails); + setEmailToDelete(null); + }; + + const handleConfirmDelete = () => { + if (emailToDelete) { + handleRemoveEmail(emailToDelete); + } + }; + + const handleKeyDown = (event: React.KeyboardEvent) => { + if (event.key === 'Enter') { + event.preventDefault(); + handleAddEmail(); + } + }; + + return ( + + +
+ NDA Bypass - Allowed Emails + + + + + + +

+ Individuals with these exact email addresses receive direct + access to the trust portal without signing an NDA when their + request is approved. Use this when an NDA has already been + signed outside Comp. +

+ + + +
+ + Individual email addresses that bypass NDA signing for trust portal access + + + +
+
+
+ { + setNewEmail(event.target.value); + setError(null); + }} + onKeyDown={handleKeyDown} + disabled={isUpdating || !canUpdate} + /> + {error &&

{error}

} +
+ +
+ + {emails.length > 0 && ( +
+ {emails.map((email) => ( + + + {email} + + + + ))} +
+ )} +
+ + + !open && setEmailToDelete(null)} + > + + + Remove Email + + Are you sure you want to remove {emailToDelete} from + the allowed emails list? This person will need to sign an NDA when + requesting access. + + + + Cancel + Remove + + + + + ); +} diff --git a/apps/app/src/app/(app)/[orgId]/trust/settings/components/TrustSettingsClient.test.tsx b/apps/app/src/app/(app)/[orgId]/trust/settings/components/TrustSettingsClient.test.tsx index cc29f18381..6d520b0750 100644 --- a/apps/app/src/app/(app)/[orgId]/trust/settings/components/TrustSettingsClient.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/trust/settings/components/TrustSettingsClient.test.tsx @@ -20,6 +20,7 @@ vi.mock('@/hooks/use-trust-portal-settings', () => ({ submitCustomDomain: vi.fn(), checkDns: vi.fn(), updateAllowedDomains: vi.fn(), + updateAllowedEmails: vi.fn(), }), })); @@ -68,6 +69,7 @@ describe('TrustSettingsClient permission gating', () => { isVercelDomain: false, vercelVerification: null, allowedDomains: ['example.com'], + allowedEmails: ['vip@partner.com'], }; beforeEach(() => { @@ -81,6 +83,7 @@ describe('TrustSettingsClient permission gating', () => { expect(screen.getByText('Contact Information')).toBeInTheDocument(); expect(screen.getByText('Configure Custom Domain')).toBeInTheDocument(); expect(screen.getByText('NDA Bypass - Allowed Domains')).toBeInTheDocument(); + expect(screen.getByText('NDA Bypass - Allowed Emails')).toBeInTheDocument(); }); it('enables contact email input when user has trust:update permission', () => { diff --git a/apps/app/src/app/(app)/[orgId]/trust/settings/components/TrustSettingsClient.tsx b/apps/app/src/app/(app)/[orgId]/trust/settings/components/TrustSettingsClient.tsx index 048313d801..214c928437 100644 --- a/apps/app/src/app/(app)/[orgId]/trust/settings/components/TrustSettingsClient.tsx +++ b/apps/app/src/app/(app)/[orgId]/trust/settings/components/TrustSettingsClient.tsx @@ -12,6 +12,7 @@ import { useForm } from 'react-hook-form'; import { toast } from 'sonner'; import { z } from 'zod'; import { AllowedDomainsManager } from '../../portal-settings/components/AllowedDomainsManager'; +import { AllowedEmailsManager } from '../../portal-settings/components/AllowedEmailsManager'; import { TrustPortalDomain } from '../../portal-settings/components/TrustPortalDomain'; const trustSettingsSchema = z.object({ @@ -26,6 +27,7 @@ interface TrustSettingsClientProps { isVercelDomain: boolean; vercelVerification: string | null; allowedDomains: string[]; + allowedEmails: string[]; } export function TrustSettingsClient({ @@ -36,6 +38,7 @@ export function TrustSettingsClient({ isVercelDomain, vercelVerification, allowedDomains, + allowedEmails, }: TrustSettingsClientProps) { const { hasPermission } = usePermissions(); const canUpdate = hasPermission('trust', 'update'); @@ -163,6 +166,9 @@ export function TrustSettingsClient({ {/* Allowed Domains */} + + {/* Allowed Emails */} + ); } diff --git a/apps/app/src/app/(app)/[orgId]/trust/settings/page.tsx b/apps/app/src/app/(app)/[orgId]/trust/settings/page.tsx index 02580a9c0f..f5aed28019 100644 --- a/apps/app/src/app/(app)/[orgId]/trust/settings/page.tsx +++ b/apps/app/src/app/(app)/[orgId]/trust/settings/page.tsx @@ -10,6 +10,7 @@ interface TrustPortalSettings { isVercelDomain: boolean; vercelVerification: string | null; allowedDomains: string[]; + allowedEmails: string[]; } export default async function TrustSettingsPage({ @@ -35,6 +36,7 @@ export default async function TrustSettingsPage({ isVercelDomain={trustPortal?.isVercelDomain ?? false} vercelVerification={trustPortal?.vercelVerification ?? null} allowedDomains={trustPortal?.allowedDomains ?? []} + allowedEmails={trustPortal?.allowedEmails ?? []} /> ); diff --git a/apps/app/src/hooks/use-trust-portal-settings.ts b/apps/app/src/hooks/use-trust-portal-settings.ts index d6b9f618e3..b82951ec1c 100644 --- a/apps/app/src/hooks/use-trust-portal-settings.ts +++ b/apps/app/src/hooks/use-trust-portal-settings.ts @@ -188,6 +188,15 @@ export function useTrustPortalSettings() { [api], ); + const updateAllowedEmails = useCallback( + async (emails: string[]) => { + const response = await api.put('/v1/trust-portal/settings/allowed-emails', { emails }); + if (response.error) throw new Error(response.error); + return response.data; + }, + [api], + ); + const uploadFavicon = useCallback( async (fileName: string, fileType: string, fileData: string) => { const response = await api.post('/v1/trust-portal/favicon', { @@ -249,6 +258,7 @@ export function useTrustPortalSettings() { saveOverview, updateVendorTrustSettings, updateAllowedDomains, + updateAllowedEmails, uploadFavicon, removeFavicon, submitCustomDomain, diff --git a/packages/db/prisma/migrations/20260610225432_add_trust_allowed_emails/migration.sql b/packages/db/prisma/migrations/20260610225432_add_trust_allowed_emails/migration.sql new file mode 100644 index 0000000000..059ef380bc --- /dev/null +++ b/packages/db/prisma/migrations/20260610225432_add_trust_allowed_emails/migration.sql @@ -0,0 +1,2 @@ +-- AlterTable +ALTER TABLE "Trust" ADD COLUMN "allowedEmails" TEXT[] DEFAULT ARRAY[]::TEXT[]; diff --git a/packages/db/prisma/schema/trust.prisma b/packages/db/prisma/schema/trust.prisma index 56ab096fe7..850a00ae72 100644 --- a/packages/db/prisma/schema/trust.prisma +++ b/packages/db/prisma/schema/trust.prisma @@ -12,6 +12,9 @@ model Trust { /// Domains that bypass NDA signing when requesting trust portal access allowedDomains String[] @default([]) + /// Individual email addresses that bypass NDA signing when requesting trust portal access + allowedEmails String[] @default([]) + email String? privacyPolicy String? soc2 Boolean @default(false) From c2b91222c3f0c3dbce37433809426a37ef65657f Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Wed, 10 Jun 2026 19:20:03 -0400 Subject: [PATCH 14/18] fix(trust): resolve 4 cubic findings from the production deploy review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - ComplianceFramework: replace the shared boolean in-flight guard with a counter so overlapping toggle + status mutations both keep prop resync gated until the last request settles (no mid-flight clobber) - CustomFrameworksSection: add a 100MB size guard before base64-encoding the upload (defense-in-depth at the API-call boundary; also avoids building a ~133MB string — the row flow already validates size first) - trust-portal.controller: encode the 'at least one of enabled/status' rule in the @ApiBody schema via anyOf so the OpenAPI/MCP contract matches the runtime zod .refine() - trust-access.service: extract the duplicated S3-download → watermark → re-upload → signed-URL pipeline shared by native and custom framework certificate downloads into watermarkAndSignTrustResource (behavior identical; removes drift risk between the two paths) Co-Authored-By: Claude Fable 5 --- .../src/trust-portal/trust-access.service.ts | 182 +++++++++--------- .../trust-portal/trust-portal.controller.ts | 3 + .../components/ComplianceFramework.tsx | 16 +- .../components/CustomFrameworksSection.tsx | 8 + 4 files changed, 108 insertions(+), 101 deletions(-) diff --git a/apps/api/src/trust-portal/trust-access.service.ts b/apps/api/src/trust-portal/trust-access.service.ts index 657f67b430..2a50484e67 100644 --- a/apps/api/src/trust-portal/trust-access.service.ts +++ b/apps/api/src/trust-portal/trust-access.service.ts @@ -1916,6 +1916,74 @@ export class TrustAccessService { return { faqs: Array.isArray(faqs) ? faqs : null }; } + /** + * Shared certificate-download pipeline for trust-portal resources (native + * and custom frameworks): fetch the stored PDF from S3, watermark it for the + * requesting grant, re-upload the watermarked copy, and return a signed URL. + * The two callers differ only in the trustResource lookup and the doc/file + * naming, which they pass in. + */ + private async watermarkAndSignTrustResource(params: { + s3Key: string; + fileName: string; + recipientName: string; + recipientEmail: string; + organizationId: string; + grantId: string; + docId: string; + downloadFileName: string; + }): Promise<{ signedUrl: string; fileName: string; fileSize: number }> { + if (!s3Client || !APP_AWS_ORG_ASSETS_BUCKET) { + throw new InternalServerErrorException( + 'Organization assets bucket is not configured', + ); + } + + const response = await s3Client.send( + new GetObjectCommand({ + Bucket: APP_AWS_ORG_ASSETS_BUCKET, + Key: params.s3Key, + }), + ); + if (!response.Body) { + throw new InternalServerErrorException('No file data received from S3'); + } + + const chunks: Uint8Array[] = []; + for await (const chunk of response.Body as Readable) { + chunks.push(chunk); + } + const originalPdfBuffer = Buffer.concat(chunks); + + const watermarked = await this.ndaPdfService.watermarkExistingPdf( + originalPdfBuffer, + { + name: params.recipientName, + email: params.recipientEmail, + docId: params.docId, + watermarkText: 'Comp AI', + }, + ); + + const key = await this.attachmentsService.uploadToS3( + watermarked, + params.downloadFileName, + 'application/pdf', + params.organizationId, + 'trust_compliance_downloads', + params.grantId, + ); + + const downloadUrl = + await this.attachmentsService.getPresignedDownloadUrl(key); + + return { + signedUrl: downloadUrl, + fileName: params.fileName, + fileSize: watermarked.length, + }; + } + async getComplianceResourceUrlByAccessToken( token: string, framework: TrustFramework, @@ -1993,56 +2061,17 @@ export class TrustAccessService { }); } - // Download the original PDF from S3 - const getCommand = new GetObjectCommand({ - Bucket: APP_AWS_ORG_ASSETS_BUCKET, - Key: record.s3Key, - }); - - const response = await s3Client.send(getCommand); - const chunks: Uint8Array[] = []; - - if (!response.Body) { - throw new InternalServerErrorException('No file data received from S3'); - } - - for await (const chunk of response.Body as any) { - chunks.push(chunk); - } - - const originalPdfBuffer = Buffer.concat(chunks); - - // Watermark the PDF - const docId = `compliance-${grant.id}-${framework}-${Date.now()}`; - const watermarked = await this.ndaPdfService.watermarkExistingPdf( - originalPdfBuffer, - { - name: grant.accessRequest.name, - email: grant.subjectEmail, - docId, - watermarkText: 'Comp AI', - }, - ); - - // Upload watermarked PDF to S3 - const key = await this.attachmentsService.uploadToS3( - watermarked, - `compliance-${framework}-grant-${grant.id}-${Date.now()}.pdf`, - 'application/pdf', - grant.accessRequest.organizationId, - 'trust_compliance_downloads', - `${grant.id}`, - ); - - // Generate signed URL for the watermarked PDF - const downloadUrl = - await this.attachmentsService.getPresignedDownloadUrl(key); - - return { - signedUrl: downloadUrl, + // Download → watermark → re-upload → signed URL (shared pipeline). + return this.watermarkAndSignTrustResource({ + s3Key: record.s3Key, fileName: record.fileName, - fileSize: watermarked.length, - }; + recipientName: grant.accessRequest.name, + recipientEmail: grant.subjectEmail, + organizationId: grant.accessRequest.organizationId, + grantId: `${grant.id}`, + docId: `compliance-${grant.id}-${framework}-${Date.now()}`, + downloadFileName: `compliance-${framework}-grant-${grant.id}-${Date.now()}.pdf`, + }); } /** @@ -2078,52 +2107,17 @@ export class TrustAccessService { ); } - const getCommand = new GetObjectCommand({ - Bucket: APP_AWS_ORG_ASSETS_BUCKET, - Key: record.s3Key, - }); - - const response = await s3Client.send(getCommand); - const chunks: Uint8Array[] = []; - - if (!response.Body) { - throw new InternalServerErrorException('No file data received from S3'); - } - - for await (const chunk of response.Body as Readable) { - chunks.push(chunk); - } - - const originalPdfBuffer = Buffer.concat(chunks); - - const docId = `compliance-${grant.id}-custom-${customFrameworkId}-${Date.now()}`; - const watermarked = await this.ndaPdfService.watermarkExistingPdf( - originalPdfBuffer, - { - name: grant.accessRequest.name, - email: grant.subjectEmail, - docId, - watermarkText: 'Comp AI', - }, - ); - - const key = await this.attachmentsService.uploadToS3( - watermarked, - `compliance-custom-${customFrameworkId}-grant-${grant.id}-${Date.now()}.pdf`, - 'application/pdf', - grant.accessRequest.organizationId, - 'trust_compliance_downloads', - `${grant.id}`, - ); - - const downloadUrl = - await this.attachmentsService.getPresignedDownloadUrl(key); - - return { - signedUrl: downloadUrl, + // Download → watermark → re-upload → signed URL (shared pipeline). + return this.watermarkAndSignTrustResource({ + s3Key: record.s3Key, fileName: record.fileName, - fileSize: watermarked.length, - }; + recipientName: grant.accessRequest.name, + recipientEmail: grant.subjectEmail, + organizationId: grant.accessRequest.organizationId, + grantId: `${grant.id}`, + docId: `compliance-${grant.id}-custom-${customFrameworkId}-${Date.now()}`, + downloadFileName: `compliance-custom-${customFrameworkId}-grant-${grant.id}-${Date.now()}.pdf`, + }); } /** Custom frameworks shown on the public portal (delegates to the dedicated service). */ diff --git a/apps/api/src/trust-portal/trust-portal.controller.ts b/apps/api/src/trust-portal/trust-portal.controller.ts index cb50f8a266..9a3215726e 100644 --- a/apps/api/src/trust-portal/trust-portal.controller.ts +++ b/apps/api/src/trust-portal/trust-portal.controller.ts @@ -418,6 +418,9 @@ export class TrustPortalController { schema: { type: 'object', required: ['customFrameworkId'], + // Mirrors UpdateTrustCustomFrameworkSchema's .refine(): the id is + // required and at least one mutable field must be present. + anyOf: [{ required: ['enabled'] }, { required: ['status'] }], properties: { customFrameworkId: { type: 'string', minLength: 1 }, enabled: { type: 'boolean' }, diff --git a/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/ComplianceFramework.tsx b/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/ComplianceFramework.tsx index 3237a35244..b84c2efef2 100644 --- a/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/ComplianceFramework.tsx +++ b/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/ComplianceFramework.tsx @@ -146,10 +146,12 @@ export function ComplianceFramework({ // refreshes (SWR revalidation, another tab/user) — otherwise the row shows // stale enabled/status values forever. Single guarded sync: skipped while a // mutation is in flight so a concurrent revalidation can't clobber the - // optimistic value before the request settles. - const mutationInFlightRef = useRef(false); + // optimistic value before the request settles. A counter (not a boolean) + // so overlapping toggle + status mutations both keep resync gated until the + // last one settles. + const inFlightCountRef = useRef(0); useEffect(() => { - if (mutationInFlightRef.current) return; + if (inFlightCountRef.current > 0) return; setIsEnabled(isEnabledProp); setStatus(statusProp); }, [isEnabledProp, statusProp]); @@ -249,13 +251,13 @@ export function ComplianceFramework({ if (!value) return; const prev = status; setStatus(value); - mutationInFlightRef.current = true; + inFlightCountRef.current += 1; try { await onStatusChange(value); } catch { setStatus(prev); } finally { - mutationInFlightRef.current = false; + inFlightCountRef.current -= 1; } }} > @@ -304,13 +306,13 @@ export function ComplianceFramework({ checked={isEnabled} onCheckedChange={async (checked) => { setIsEnabled(checked); - mutationInFlightRef.current = true; + inFlightCountRef.current += 1; try { await onToggle(checked); } catch { setIsEnabled(!checked); } finally { - mutationInFlightRef.current = false; + inFlightCountRef.current -= 1; } }} /> diff --git a/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/CustomFrameworksSection.tsx b/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/CustomFrameworksSection.tsx index e2a6d90bc9..6540877704 100644 --- a/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/CustomFrameworksSection.tsx +++ b/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/CustomFrameworksSection.tsx @@ -99,6 +99,14 @@ export function CustomFrameworksSection({ }; const handleFileUpload = async (file: File, customFrameworkId: string) => { + // Defense-in-depth size cap at the API-call boundary: the raw file is + // limited to 100MB (server decodes and re-checks), and base64 inflates it + // ~33% — guarding here avoids building a ~133MB string and surfaces a clean + // error if this handler is ever invoked outside the validated row flow. + const MAX_FILE_SIZE = 100 * 1024 * 1024; + if (file.size > MAX_FILE_SIZE) { + throw new Error('File size must be less than 100MB'); + } const fileData = await convertFileToBase64(file); const payload = await uploadCustomComplianceResource( orgId, From 06ed9bd3da20245e22ac57531f19518b132f83b3 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Wed, 10 Jun 2026 19:32:24 -0400 Subject: [PATCH 15/18] fix(trust-portal): validate allowed-emails body with a DTO Replace the inline @Body type on the allowed-emails endpoint with UpdateAllowedEmailsDto (class-validator + @ApiProperty) and @ApiBody so the ValidationPipe rejects malformed input and the OpenAPI/MCP schema is generatable. Flagged by cubic on PR 3097. Co-Authored-By: Claude Fable 5 --- .../trust-portal/dto/update-allowed-emails.dto.ts | 14 ++++++++++++++ .../src/trust-portal/trust-portal.controller.ts | 4 +++- 2 files changed, 17 insertions(+), 1 deletion(-) create mode 100644 apps/api/src/trust-portal/dto/update-allowed-emails.dto.ts diff --git a/apps/api/src/trust-portal/dto/update-allowed-emails.dto.ts b/apps/api/src/trust-portal/dto/update-allowed-emails.dto.ts new file mode 100644 index 0000000000..8de6c4f1ea --- /dev/null +++ b/apps/api/src/trust-portal/dto/update-allowed-emails.dto.ts @@ -0,0 +1,14 @@ +import { ApiProperty } from '@nestjs/swagger'; +import { IsArray, IsEmail } from 'class-validator'; + +export class UpdateAllowedEmailsDto { + @ApiProperty({ + description: + 'Email addresses that bypass NDA signing for trust portal access. Replaces the full list; send an empty array to clear it.', + type: [String], + example: ['person@example.com'], + }) + @IsArray() + @IsEmail({}, { each: true }) + emails: string[]; +} diff --git a/apps/api/src/trust-portal/trust-portal.controller.ts b/apps/api/src/trust-portal/trust-portal.controller.ts index 50288ca8bd..26d4647e94 100644 --- a/apps/api/src/trust-portal/trust-portal.controller.ts +++ b/apps/api/src/trust-portal/trust-portal.controller.ts @@ -56,6 +56,7 @@ import { } from './dto/trust-custom-link.dto'; import type { UpdateTrustOverviewDto } from './dto/update-trust-overview.dto'; import { UpdateTrustOverviewSchema } from './dto/update-trust-overview.dto'; +import { UpdateAllowedEmailsDto } from './dto/update-allowed-emails.dto'; import type { UpdateVendorTrustSettingsDto } from './dto/trust-vendor.dto'; import { UpdateVendorTrustSettingsSchema } from './dto/trust-vendor.dto'; import { UpdateTrustCustomFrameworkSchema } from './dto/trust-custom-framework.dto'; @@ -389,9 +390,10 @@ export class TrustPortalController { @Put('settings/allowed-emails') @RequirePermission('trust', 'update') @ApiOperation({ summary: 'Update allowed emails for the trust portal' }) + @ApiBody({ type: UpdateAllowedEmailsDto }) async updateAllowedEmails( @OrganizationId() organizationId: string, - @Body() body: { emails: string[] }, + @Body() body: UpdateAllowedEmailsDto, ) { return this.trustPortalService.updateAllowedEmails( organizationId, From 086bf7c4f6396f8f4a13281fd2678ed72e4686e0 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Wed, 10 Jun 2026 19:42:27 -0400 Subject: [PATCH 16/18] fix(trust): gate certificate drag-and-drop behind the read-only permission MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cubic P1: drag-and-drop bypassed the disabled gate the click path already had, so read-only users (canUpdate=false → disabled) could upload certificates by dropping a file. - ComplianceFramework: handleDrop and handleDragEnter now early-return when disabled || isUploading (mirrors the gated click path), so read-only users get neither the drop affordance nor the upload - CustomFrameworksSection: pass onFileUpload only when canUpdate (defense-in-depth; processFile already no-ops without the callback) - new ComplianceFramework.test.tsx: asserts a dropped PDF uploads when editable and does NOT upload when disabled Co-Authored-By: Claude Fable 5 --- .../components/ComplianceFramework.test.tsx | 48 +++++++++++++++++++ .../components/ComplianceFramework.tsx | 7 +++ .../components/CustomFrameworksSection.tsx | 2 +- 3 files changed, 56 insertions(+), 1 deletion(-) create mode 100644 apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/ComplianceFramework.test.tsx diff --git a/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/ComplianceFramework.test.tsx b/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/ComplianceFramework.test.tsx new file mode 100644 index 0000000000..0f0ed3b8c9 --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/ComplianceFramework.test.tsx @@ -0,0 +1,48 @@ +import { fireEvent, render, screen } from '@testing-library/react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { ComplianceFramework } from './ComplianceFramework'; + +vi.mock('sonner', () => ({ + toast: { success: vi.fn(), error: vi.fn() }, +})); + +function renderRow(overrides: { disabled?: boolean } = {}) { + const onFileUpload = vi.fn().mockResolvedValue(undefined); + render( + , + ); + return { onFileUpload }; +} + +function dropPdf() { + const dropZone = screen.getByText(/Drag & drop certificate/i); + const file = new File(['%PDF-1.4'], 'cert.pdf', { type: 'application/pdf' }); + fireEvent.drop(dropZone, { dataTransfer: { files: [file] } }); +} + +describe('ComplianceFramework drag-and-drop permission gate', () => { + beforeEach(() => vi.clearAllMocks()); + + it('uploads a dropped certificate when editable', async () => { + const { onFileUpload } = renderRow({ disabled: false }); + dropPdf(); + expect(onFileUpload).toHaveBeenCalledTimes(1); + }); + + it('does NOT upload a dropped certificate for read-only users (disabled)', () => { + const { onFileUpload } = renderRow({ disabled: true }); + dropPdf(); + expect(onFileUpload).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/ComplianceFramework.tsx b/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/ComplianceFramework.tsx index b84c2efef2..71b96ad685 100644 --- a/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/ComplianceFramework.tsx +++ b/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/ComplianceFramework.tsx @@ -190,6 +190,9 @@ export function ComplianceFramework({ const handleDragEnter = (e: React.DragEvent) => { e.preventDefault(); e.stopPropagation(); + // Read-only / mid-upload: no drag affordance, no drop (mirrors the + // disabled click path). + if (disabled || isUploading) return; dragCounterRef.current++; if (e.dataTransfer.items && e.dataTransfer.items.length > 0) { setIsDragging(true); @@ -216,6 +219,10 @@ export function ComplianceFramework({ setIsDragging(false); dragCounterRef.current = 0; + // Gate uploads the same way the click path is gated — drag-and-drop must + // not bypass the read-only / uploading state. + if (disabled || isUploading) return; + const files = e.dataTransfer.files; if (files && files.length > 0) { await processFile(files[0]); diff --git a/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/CustomFrameworksSection.tsx b/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/CustomFrameworksSection.tsx index 6540877704..9a55668802 100644 --- a/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/CustomFrameworksSection.tsx +++ b/apps/app/src/app/(app)/[orgId]/trust/portal-settings/components/CustomFrameworksSection.tsx @@ -166,7 +166,7 @@ export function CustomFrameworksSection({ throw error; } }} - onFileUpload={handleFileUpload} + onFileUpload={canUpdate ? handleFileUpload : undefined} onFilePreview={handleFilePreview} /> ))} From bb3074948e8d6de94d45fc6eb2533bae0086fae0 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Wed, 10 Jun 2026 19:52:55 -0400 Subject: [PATCH 17/18] fix(trust): return 400 not 500 on malformed PUT /custom-frameworks body MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Full-feature audit follow-up: the endpoint validated the body with an inline UpdateTrustCustomFrameworkSchema.parse(), which throws a raw ZodError. No global ZodError filter exists, so malformed input surfaced as HTTP 500 — contradicting the @ApiBody/MCP contract that says the body is validated. Switched to the existing ZodValidationPipe so it returns 400. Scoped to this feature's endpoint; the same pre-existing pattern on sibling endpoints is noted as a separate follow-up (a global @Catch(ZodError) filter would fix them all). Co-Authored-By: Claude Fable 5 --- .../api/src/trust-portal/trust-portal.controller.ts | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/apps/api/src/trust-portal/trust-portal.controller.ts b/apps/api/src/trust-portal/trust-portal.controller.ts index 9a3215726e..efb27ed68e 100644 --- a/apps/api/src/trust-portal/trust-portal.controller.ts +++ b/apps/api/src/trust-portal/trust-portal.controller.ts @@ -58,7 +58,11 @@ import type { UpdateTrustOverviewDto } from './dto/update-trust-overview.dto'; import { UpdateTrustOverviewSchema } from './dto/update-trust-overview.dto'; import type { UpdateVendorTrustSettingsDto } from './dto/trust-vendor.dto'; import { UpdateVendorTrustSettingsSchema } from './dto/trust-vendor.dto'; -import { UpdateTrustCustomFrameworkSchema } from './dto/trust-custom-framework.dto'; +import { + UpdateTrustCustomFrameworkSchema, + type UpdateTrustCustomFrameworkDto, +} from './dto/trust-custom-framework.dto'; +import { ZodValidationPipe } from '../common/pipes/zod-validation.pipe'; import { TrustPortalService } from './trust-portal.service'; import { TrustCustomFrameworkService } from './trust-custom-framework.service'; @@ -433,9 +437,12 @@ export class TrustPortalController { }) async updateCustomFramework( @OrganizationId() organizationId: string, - @Body() body: unknown, + // Validate via the pipe so a malformed body returns 400 (matching the + // @ApiBody/MCP contract) instead of an inline .parse() throwing a raw + // ZodError that surfaces as a 500. + @Body(new ZodValidationPipe(UpdateTrustCustomFrameworkSchema)) + dto: UpdateTrustCustomFrameworkDto, ) { - const dto = UpdateTrustCustomFrameworkSchema.parse(body); return this.trustCustomFrameworkService.updateSelection( organizationId, dto, From 6c35625d6dd38cd77b44322cf1c21ccedc3c37c5 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Wed, 10 Jun 2026 20:11:08 -0400 Subject: [PATCH 18/18] fix(integrations): gate the Add-account CTA on integration:create RBAC The 'Add' connection CTA in the integration hero was gated only by provider metadata (supportsMultipleConnections + non-oauth2), not by permission, so a user without integration:create was still offered the create-connection flow. Add the same hasPermission('integration', 'create') check the rest of the integrations UI uses (PlatformIntegrations, ServiceDetailView), threaded from ProviderDetailView into the hero. Server-side connect endpoints already enforce the permission; this closes the UI authorization-surface gap. Co-Authored-By: Claude Fable 5 --- .../[slug]/components/IntegrationProviderHero.tsx | 8 +++++++- .../integrations/[slug]/components/ProviderDetailView.tsx | 4 ++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/apps/app/src/app/(app)/[orgId]/integrations/[slug]/components/IntegrationProviderHero.tsx b/apps/app/src/app/(app)/[orgId]/integrations/[slug]/components/IntegrationProviderHero.tsx index a87adfa40d..9d93536ed8 100644 --- a/apps/app/src/app/(app)/[orgId]/integrations/[slug]/components/IntegrationProviderHero.tsx +++ b/apps/app/src/app/(app)/[orgId]/integrations/[slug]/components/IntegrationProviderHero.tsx @@ -10,6 +10,7 @@ import { getConnectionDisplayLabel } from './connection-display'; type HeroProps = { provider: IntegrationProvider; isConnected: boolean; + canCreateConnection: boolean; activeConnections: ConnectionListItem[]; selectedConnection: ConnectionListItem | null; onSelectConnection: (id: string) => void; @@ -20,6 +21,7 @@ type HeroProps = { export function IntegrationProviderHero({ provider, isConnected, + canCreateConnection, activeConnections, selectedConnection, onSelectConnection, @@ -142,7 +144,11 @@ export function IntegrationProviderHero({ connection, so a second OAuth connect silently merges into the first — only offer "Add" where the connect flow can actually create another. */} - {provider.supportsMultipleConnections && + {/* Adding a connection is a create action — gate on + RBAC, not just provider metadata, so users without + integration:create aren't offered the flow. */} + {canCreateConnection && + provider.supportsMultipleConnections && provider.authType !== 'oauth2' && (