From bdb6b3b0d7976b2f3c098edf13d8fda8a57c13ab Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Tue, 9 Jun 2026 16:21:22 -0400 Subject: [PATCH 1/4] fix(cloud-security): run task evidence checks against all connected accounts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A task's evidence check (e.g. AWS S3 — default encryption) only ran against the customer's FIRST connected account: the run-check endpoint ran the single referenced connectionId, and the task card showed only the latest single run. Customers with multiple AWS accounts saw results for just one of them. Manual Run now runs the check against EVERY active account of the provider (one IntegrationCheckRun per account, preserving reconciliation's per-connection findingKey diffing) and aggregates task status across all of them. The scheduler already ran every account per-connection, so it needs no run-level change — the same display fix surfaces all accounts for scheduled runs too. Display: /runs now guarantees the latest run per (connection, check) via a groupBy + completeness query (a flat row limit could bury an account behind a busier one's re-runs) and returns connectionId + a connection label. The task card groups runs per account and aggregates the header across accounts; single-account layout is unchanged. Covers all AWS check services (s3, cloudtrail, ec2, iam, kms, rds) since the change is account-level, not per-service. Extracted CheckRunItem/GroupedCheckRuns into check-run-history.tsx (the source file was 1610 lines) and added pure grouping helpers. DS migration of the moved legacy imports is intentionally out of scope (verbatim move; parent tree still uses them). Tests: per-account run loop + aggregate status + bad-account resilience (controller), latest-per-account completeness under a re-run burst (repository), account label branches (util), and run grouping/summary (app). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../task-integrations.controller.spec.ts | 379 ++++++++++++++++++ .../task-integrations.controller.ts | 364 ++++++++++------- .../repositories/check-run.repository.spec.ts | 164 ++++++++ .../repositories/check-run.repository.ts | 75 +++- .../repositories/connection.repository.ts | 22 + .../utils/connection-label.spec.ts | 79 ++++ .../utils/connection-label.ts | 33 ++ .../components/TaskIntegrationChecks.tsx | 319 +-------------- .../components/check-run-grouping.test.ts | 93 +++++ .../[taskId]/components/check-run-grouping.ts | 83 ++++ .../[taskId]/components/check-run-history.tsx | 347 ++++++++++++++++ .../[taskId]/hooks/useIntegrationChecks.ts | 4 + 12 files changed, 1514 insertions(+), 448 deletions(-) create mode 100644 apps/api/src/integration-platform/controllers/task-integrations.controller.spec.ts create mode 100644 apps/api/src/integration-platform/repositories/check-run.repository.spec.ts create mode 100644 apps/api/src/integration-platform/utils/connection-label.spec.ts create mode 100644 apps/api/src/integration-platform/utils/connection-label.ts create mode 100644 apps/app/src/app/(app)/[orgId]/tasks/[taskId]/components/check-run-grouping.test.ts create mode 100644 apps/app/src/app/(app)/[orgId]/tasks/[taskId]/components/check-run-grouping.ts create mode 100644 apps/app/src/app/(app)/[orgId]/tasks/[taskId]/components/check-run-history.tsx diff --git a/apps/api/src/integration-platform/controllers/task-integrations.controller.spec.ts b/apps/api/src/integration-platform/controllers/task-integrations.controller.spec.ts new file mode 100644 index 0000000000..6efed78f88 --- /dev/null +++ b/apps/api/src/integration-platform/controllers/task-integrations.controller.spec.ts @@ -0,0 +1,379 @@ +import { Test, TestingModule } from '@nestjs/testing'; +import { TaskIntegrationsController } from './task-integrations.controller'; +import { HybridAuthGuard } from '../../auth/hybrid-auth.guard'; +import { PermissionGuard } from '../../auth/permission.guard'; +import { ConnectionRepository } from '../repositories/connection.repository'; +import { ProviderRepository } from '../repositories/provider.repository'; +import { CheckRunRepository } from '../repositories/check-run.repository'; +import { CredentialVaultService } from '../services/credential-vault.service'; +import { OAuthCredentialsService } from '../services/oauth-credentials.service'; +import { TaskIntegrationChecksService } from '../services/task-integration-checks.service'; + +jest.mock('../../auth/auth.server', () => ({ + auth: { api: { getSession: jest.fn() } }, +})); + +jest.mock('@trycompai/auth', () => ({ + statement: { integration: ['create', 'read', 'update', 'delete'] }, + BUILT_IN_ROLE_PERMISSIONS: {}, +})); + +jest.mock('@db', () => ({ + db: { + task: { findUnique: jest.fn(), update: jest.fn() }, + }, +})); + +jest.mock('@trycompai/integration-platform', () => ({ + getManifest: jest.fn(), + getActiveManifests: jest.fn(), + runAllChecks: jest.fn(), +})); + +import { db } from '@db'; +import { getManifest, runAllChecks } from '@trycompai/integration-platform'; + +const mockedGetManifest = getManifest as jest.Mock; +const mockedRunAllChecks = runAllChecks as jest.Mock; +// Grab through the module reference to avoid the `unbound-method` lint rule. +const mockedTask = db.task as unknown as { + findUnique: jest.Mock; + update: jest.Mock; +}; +const mockTaskFindUnique = mockedTask.findUnique; +const mockTaskUpdate = mockedTask.update; + +const MANIFEST = { + id: 'aws', + name: 'Amazon Web Services', + auth: { type: 'custom' }, + checks: [ + { id: 'aws-s3-encryption', name: 'S3 — default encryption enabled' }, + ], +}; + +const VALID_CREDS = { + roleArn: 'arn:aws:iam::111111111111:role/x', + externalId: 'ext', + regions: ['us-east-1'], +}; + +function passingResult() { + return { + results: [ + { + checkId: 'aws-s3-encryption', + checkName: 'S3 — default encryption enabled', + status: 'success', + durationMs: 10, + error: undefined, + result: { + findings: [], + passingResults: [ + { + resourceType: 'aws-s3-bucket', + resourceId: 'b1', + title: 'ok', + description: 'ok', + }, + ], + summary: { totalChecked: 1 }, + logs: [], + }, + }, + ], + }; +} + +function failingResult() { + return { + results: [ + { + checkId: 'aws-s3-encryption', + checkName: 'S3 — default encryption enabled', + status: 'failed', + durationMs: 10, + error: undefined, + result: { + findings: [ + { + resourceType: 'aws-s3-bucket', + resourceId: 'b2', + title: 'no enc', + description: 'x', + severity: 'high', + remediation: 'fix', + }, + ], + passingResults: [], + summary: { totalChecked: 1 }, + logs: [], + }, + }, + ], + }; +} + +describe('TaskIntegrationsController', () => { + let controller: TaskIntegrationsController; + + const mockConnectionRepository = { + findById: jest.fn(), + findActiveByProviderAndOrg: jest.fn(), + }; + const mockProviderRepository = { findById: jest.fn() }; + const mockCheckRunRepository = { + create: jest.fn(), + complete: jest.fn(), + addResults: jest.fn(), + findLatestPerConnectionAndCheckByTask: jest.fn(), + }; + const mockCredentialVaultService = { getDecryptedCredentials: jest.fn() }; + const mockOAuthCredentialsService = { + getCredentials: jest.fn(), + checkAvailability: jest.fn(), + }; + const mockTaskIntegrationChecksService = { + disconnectCheckFromTask: jest.fn(), + reconnectCheckToTask: jest.fn(), + }; + const mockGuard = { canActivate: jest.fn().mockReturnValue(true) }; + + beforeEach(async () => { + const module: TestingModule = await Test.createTestingModule({ + controllers: [TaskIntegrationsController], + providers: [ + { provide: ConnectionRepository, useValue: mockConnectionRepository }, + { provide: ProviderRepository, useValue: mockProviderRepository }, + { provide: CheckRunRepository, useValue: mockCheckRunRepository }, + { + provide: CredentialVaultService, + useValue: mockCredentialVaultService, + }, + { + provide: OAuthCredentialsService, + useValue: mockOAuthCredentialsService, + }, + { + provide: TaskIntegrationChecksService, + useValue: mockTaskIntegrationChecksService, + }, + ], + }) + .overrideGuard(HybridAuthGuard) + .useValue(mockGuard) + .overrideGuard(PermissionGuard) + .useValue(mockGuard) + .compile(); + + controller = module.get(TaskIntegrationsController); + jest.clearAllMocks(); + + mockTaskFindUnique.mockResolvedValue({ + id: 'task_1', + organizationId: 'org_1', + status: 'todo', + frequency: null, + }); + mockProviderRepository.findById.mockResolvedValue({ + id: 'prov_aws', + slug: 'aws', + }); + mockedGetManifest.mockReturnValue(MANIFEST); + mockCheckRunRepository.create.mockImplementation(() => + Promise.resolve({ id: 'icr_x', startedAt: new Date() }), + ); + mockCheckRunRepository.complete.mockResolvedValue({}); + mockCheckRunRepository.addResults.mockResolvedValue({}); + mockCredentialVaultService.getDecryptedCredentials.mockResolvedValue( + VALID_CREDS, + ); + mockTaskUpdate.mockResolvedValue({}); + }); + + const body = { connectionId: 'conn_1', checkId: 'aws-s3-encryption' }; + + describe('runCheckForTask (all accounts)', () => { + it('runs the check for EVERY active account of the provider', async () => { + mockConnectionRepository.findById.mockResolvedValue({ + id: 'conn_1', + organizationId: 'org_1', + providerId: 'prov_aws', + }); + mockConnectionRepository.findActiveByProviderAndOrg.mockResolvedValue([ + { + id: 'conn_1', + organizationId: 'org_1', + providerId: 'prov_aws', + metadata: {}, + variables: {}, + }, + { + id: 'conn_2', + organizationId: 'org_1', + providerId: 'prov_aws', + metadata: {}, + variables: {}, + }, + ]); + mockedRunAllChecks.mockResolvedValue(passingResult()); + + const result = await controller.runCheckForTask('task_1', 'org_1', body); + + expect( + mockConnectionRepository.findActiveByProviderAndOrg, + ).toHaveBeenCalledWith('prov_aws', 'org_1'); + // One run per account. + expect(mockCheckRunRepository.create).toHaveBeenCalledTimes(2); + expect(mockedRunAllChecks).toHaveBeenCalledTimes(2); + expect(result.accountsRun).toBe(2); + expect(result.success).toBe(true); + expect(result.taskStatus).toBe('done'); + }); + + it('marks the task failed if ANY account has findings', async () => { + mockConnectionRepository.findById.mockResolvedValue({ + id: 'conn_1', + organizationId: 'org_1', + providerId: 'prov_aws', + }); + mockConnectionRepository.findActiveByProviderAndOrg.mockResolvedValue([ + { + id: 'conn_1', + organizationId: 'org_1', + providerId: 'prov_aws', + metadata: {}, + variables: {}, + }, + { + id: 'conn_2', + organizationId: 'org_1', + providerId: 'prov_aws', + metadata: {}, + variables: {}, + }, + ]); + mockedRunAllChecks + .mockResolvedValueOnce(passingResult()) + .mockResolvedValueOnce(failingResult()); + + const result = await controller.runCheckForTask('task_1', 'org_1', body); + + expect(result.accountsRun).toBe(2); + expect(result.totalFindings).toBe(1); + expect(result.taskStatus).toBe('failed'); + expect(mockTaskUpdate).toHaveBeenCalledWith( + expect.objectContaining({ + data: expect.objectContaining({ status: 'failed' }), + }), + ); + }); + + it('records a failed run for a bad account but keeps running the rest', async () => { + mockConnectionRepository.findById.mockResolvedValue({ + id: 'conn_1', + organizationId: 'org_1', + providerId: 'prov_aws', + }); + mockConnectionRepository.findActiveByProviderAndOrg.mockResolvedValue([ + { + id: 'conn_1', + organizationId: 'org_1', + providerId: 'prov_aws', + metadata: {}, + variables: {}, + }, + { + id: 'conn_2', + organizationId: 'org_1', + providerId: 'prov_aws', + metadata: {}, + variables: {}, + }, + ]); + // First account has no credentials → recorded failed; second runs fine. + mockCredentialVaultService.getDecryptedCredentials + .mockResolvedValueOnce({}) + .mockResolvedValueOnce(VALID_CREDS); + mockedRunAllChecks.mockResolvedValue(passingResult()); + + const result = await controller.runCheckForTask('task_1', 'org_1', body); + + expect(mockCheckRunRepository.create).toHaveBeenCalledTimes(2); + // Only the healthy account actually executed the check. + expect(mockedRunAllChecks).toHaveBeenCalledTimes(1); + // The bad account's run was completed as failed. + expect(mockCheckRunRepository.complete).toHaveBeenCalledWith( + 'icr_x', + expect.objectContaining({ status: 'failed' }), + ); + expect(result.accountsRun).toBe(2); + expect(result.success).toBe(true); + }); + }); + + describe('getTaskCheckRuns', () => { + it('labels each run with its account id + connection label', async () => { + mockCheckRunRepository.findLatestPerConnectionAndCheckByTask.mockResolvedValue( + [ + { + id: 'icr_1', + checkId: 'aws-s3-encryption', + checkName: 'S3', + status: 'success', + startedAt: new Date(), + completedAt: new Date(), + durationMs: 10, + totalChecked: 1, + passedCount: 1, + failedCount: 0, + errorMessage: null, + logs: [], + connectionId: 'conn_1', + createdAt: new Date(), + results: [], + connection: { + id: 'conn_1', + metadata: { connectionName: 'Production AWS' }, + provider: { slug: 'aws', name: 'AWS' }, + }, + }, + { + id: 'icr_2', + checkId: 'aws-s3-encryption', + checkName: 'S3', + status: 'success', + startedAt: new Date(), + completedAt: new Date(), + durationMs: 10, + totalChecked: 1, + passedCount: 1, + failedCount: 0, + errorMessage: null, + logs: [], + connectionId: 'conn_2', + createdAt: new Date(), + results: [], + connection: { + id: 'conn_2', + metadata: { roleArn: 'arn:aws:iam::222222222222:role/x' }, + provider: { slug: 'aws', name: 'AWS' }, + }, + }, + ], + ); + + const { runs } = await controller.getTaskCheckRuns('task_1'); + + expect(runs).toHaveLength(2); + expect(runs[0]).toMatchObject({ + connectionId: 'conn_1', + connectionLabel: 'Production AWS', + }); + expect(runs[1]).toMatchObject({ + connectionId: 'conn_2', + connectionLabel: 'AWS 222222222222', + }); + }); + }); +}); diff --git a/apps/api/src/integration-platform/controllers/task-integrations.controller.ts b/apps/api/src/integration-platform/controllers/task-integrations.controller.ts index 9d758739df..86e5b1aa6f 100644 --- a/apps/api/src/integration-platform/controllers/task-integrations.controller.ts +++ b/apps/api/src/integration-platform/controllers/task-integrations.controller.ts @@ -27,7 +27,6 @@ import { getActiveManifests, getManifest, runAllChecks, - type CheckRunResult, } from '@trycompai/integration-platform'; import { ConnectionRepository } from '../repositories/connection.repository'; import { ProviderRepository } from '../repositories/provider.repository'; @@ -38,8 +37,22 @@ import { TaskIntegrationChecksService } from '../services/task-integration-check import { getStringValue } from '../utils/credential-utils'; import { isCheckDisabledForTask } from '../utils/disabled-task-checks'; import { getProviderSummary } from '../utils/provider-summary'; +import { getConnectionLabel } from '../utils/connection-label'; import { db } from '@db'; -import type { Prisma } from '@db'; +import type { IntegrationConnection, Prisma } from '@db'; + +/** Manifest + check types derived from the integration-platform registry. */ +type IntegrationManifest = NonNullable>; +type IntegrationCheckDef = NonNullable[number]; + +/** Outcome of running one check against one connection (account). */ +interface ConnectionCheckOutcome { + connectionId: string; + checkRunId: string; + status: 'success' | 'failed' | 'error'; + findings: number; + passing: number; +} interface TaskIntegrationCheck { integrationId: string; @@ -291,8 +304,12 @@ export class TaskIntegrationsController { @Body() body: RunCheckForTaskDto, ): Promise<{ success: boolean; - result?: CheckRunResult; error?: string; + accountsRun?: number; + totalPassing?: number; + totalFindings?: number; + /** True when at least one account could not be checked (e.g. bad creds). */ + hadErrors?: boolean; checkRunId?: string; taskStatus?: string | null; }> { @@ -307,30 +324,23 @@ export class TaskIntegrationsController { throw new HttpException('Task not found', HttpStatus.NOT_FOUND); } - // Get connection - const connection = await this.connectionRepository.findById(connectionId); - if (!connection || connection.organizationId !== organizationId) { + // The UI references one connection, but a customer may have several + // accounts connected for the same provider (e.g. multiple AWS accounts). + // Use the referenced connection only to resolve the provider, then run the + // check against EVERY active account of that provider — matching the + // scheduler, which already runs each connection. This is why running once + // checked only the first account. + const referencedConnection = + await this.connectionRepository.findById(connectionId); + if ( + !referencedConnection || + referencedConnection.organizationId !== organizationId + ) { throw new HttpException('Connection not found', HttpStatus.NOT_FOUND); } - if (connection.status !== 'active') { - throw new HttpException( - 'Connection is not active', - HttpStatus.BAD_REQUEST, - ); - } - - // Reject runs for checks that have been disconnected from this task. - if (isCheckDisabledForTask(connection.metadata, taskId, checkId)) { - throw new HttpException( - 'This check is disconnected from the task. Reconnect it before running.', - HttpStatus.BAD_REQUEST, - ); - } - - // Get provider and manifest const provider = await this.providerRepository.findById( - connection.providerId, + referencedConnection.providerId, ); if (!provider) { throw new HttpException('Provider not found', HttpStatus.NOT_FOUND); @@ -341,72 +351,169 @@ export class TaskIntegrationsController { throw new HttpException('Manifest not found', HttpStatus.NOT_FOUND); } - // Get credentials - const credentials = - await this.credentialVaultService.getDecryptedCredentials(connectionId); - - // Validate credentials based on auth type - if (!credentials) { + const checkDef = manifest.checks?.find((c) => c.id === checkId); + if (!checkDef) { throw new HttpException( - 'No credentials found for connection', - HttpStatus.BAD_REQUEST, + `Check ${checkId} not found`, + HttpStatus.NOT_FOUND, ); } - // For OAuth, require access_token. For custom auth (like AWS), check for required fields - if (manifest.auth.type === 'oauth2' && !credentials.access_token) { - throw new HttpException( - 'No valid OAuth credentials found. Please reconnect.', - HttpStatus.BAD_REQUEST, + const activeConnections = + await this.connectionRepository.findActiveByProviderAndOrg( + provider.id, + organizationId, ); + // Never run zero accounts: if a status race leaves the active query empty, + // fall back to the referenced connection. + const connections = + activeConnections.length > 0 ? activeConnections : [referencedConnection]; + + let totalFindings = 0; + let totalPassing = 0; + let accountsRun = 0; + let hasExecutionError = false; + let lastCheckRunId: string | undefined; + + // Sequential so each per-account run commits as it completes — a slow or + // failing account still leaves the earlier accounts' results persisted. + for (const conn of connections) { + // Respect a check that was disconnected from this task for this account. + if (isCheckDisabledForTask(conn.metadata, taskId, checkId)) { + continue; + } + const outcome = await this.runCheckForConnection({ + connection: conn, + manifest, + checkDef, + taskId, + organizationId, + }); + accountsRun += 1; + totalFindings += outcome.findings; + totalPassing += outcome.passing; + if (outcome.status === 'error') hasExecutionError = true; + lastCheckRunId = outcome.checkRunId; } - // For custom auth, the credentials are the form field values directly - if ( - manifest.auth.type === 'custom' && - Object.keys(credentials).length === 0 - ) { + if (accountsRun === 0) { throw new HttpException( - 'No valid credentials found for custom integration', + 'This check is disconnected from the task. Reconnect it before running.', HttpStatus.BAD_REQUEST, ); } - const variables = - (connection.variables as Record< - string, - string | number | boolean | string[] | undefined - >) || {}; + // Aggregate task status across ALL accounts: any finding anywhere → failed; + // else any passing result → done; else leave unchanged. This replaces the + // old single-run update and removes the previous last-writer race. + const newStatus = + totalFindings > 0 ? 'failed' : totalPassing > 0 ? 'done' : null; + + if (newStatus) { + const isTransitioningToDone = + newStatus === 'done' && task.status !== 'done'; + + let reviewDate: Date | undefined; + if (isTransitioningToDone && task.frequency) { + reviewDate = new Date(); + switch (task.frequency) { + case 'monthly': + reviewDate.setMonth(reviewDate.getMonth() + 1); + break; + case 'quarterly': + reviewDate.setMonth(reviewDate.getMonth() + 3); + break; + case 'yearly': + reviewDate.setFullYear(reviewDate.getFullYear() + 1); + break; + } + } - // Find the check definition to get the name - const checkDef = manifest.checks?.find((c) => c.id === checkId); - if (!checkDef) { - throw new HttpException( - `Check ${checkId} not found`, - HttpStatus.NOT_FOUND, + await db.task.update({ + where: { id: taskId }, + data: { + status: newStatus, + ...(reviewDate ? { reviewDate } : {}), + }, + }); + this.logger.log( + `Updated task ${taskId} status to ${newStatus} across ${accountsRun} account(s)${reviewDate ? `, next review: ${reviewDate.toISOString()}` : ''}`, ); } - // Build token refresh callback for OAuth integrations that support it - let onTokenRefresh: (() => Promise) | undefined; - if (manifest.auth.type === 'oauth2') { - const oauthConfig = manifest.auth.config; + return { + success: true, + accountsRun, + totalPassing, + totalFindings, + hadErrors: hasExecutionError, + checkRunId: lastCheckRunId, + taskStatus: newStatus, + }; + } - // Only set up refresh callback if provider supports refresh tokens - const supportsRefresh = oauthConfig.supportsRefreshToken !== false; + /** + * Run one check against ONE connection (account) and persist the run + + * results. Resilient: any failure (missing credentials, execution error) is + * recorded on the check run and returned as an outcome rather than thrown, so + * a caller looping over multiple accounts is never aborted by one bad + * account. Does NOT update task status — the caller aggregates across + * accounts. + */ + private async runCheckForConnection(params: { + connection: IntegrationConnection; + manifest: IntegrationManifest; + checkDef: IntegrationCheckDef; + taskId: string; + organizationId: string; + }): Promise { + const { connection, manifest, checkDef, taskId, organizationId } = params; + const connectionId = connection.id; + + // Create the run up front so even an account that fails credential + // validation still produces a visible (failed) run row for that account. + const checkRun = await this.checkRunRepository.create({ + connectionId, + taskId, + checkId: checkDef.id, + checkName: checkDef.name, + }); - if (supportsRefresh) { - const oauthCredentials = - await this.oauthCredentialsService.getCredentials( - provider.slug, - organizationId, - ); + try { + const credentials = + await this.credentialVaultService.getDecryptedCredentials(connectionId); + + if ( + !credentials || + (manifest.auth.type === 'oauth2' && !credentials.access_token) || + (manifest.auth.type === 'custom' && + Object.keys(credentials).length === 0) + ) { + throw new Error( + 'No valid credentials found for this connection. Reconnect the integration.', + ); + } - if (oauthCredentials) { - onTokenRefresh = async () => { - return this.credentialVaultService.refreshOAuthTokens( - connectionId, - { + const variables = + (connection.variables as Record< + string, + string | number | boolean | string[] | undefined + >) || {}; + + // Build token refresh callback for OAuth integrations that support it. + let onTokenRefresh: (() => Promise) | undefined; + if (manifest.auth.type === 'oauth2') { + const oauthConfig = manifest.auth.config; + const supportsRefresh = oauthConfig.supportsRefreshToken !== false; + if (supportsRefresh) { + const oauthCredentials = + await this.oauthCredentialsService.getCredentials( + manifest.id, + organizationId, + ); + if (oauthCredentials) { + onTokenRefresh = async () => + this.credentialVaultService.refreshOAuthTokens(connectionId, { tokenUrl: oauthConfig.tokenUrl, refreshUrl: oauthConfig.refreshUrl, clientId: oauthCredentials.clientId, @@ -414,23 +521,11 @@ export class TaskIntegrationsController { clientAuthMethod: oauthConfig.clientAuthMethod, scope: oauthCredentials.scopes.join(' '), tokenParams: oauthConfig.tokenParams, - }, - ); - }; + }); + } } } - } - - // Create check run record - const checkRun = await this.checkRunRepository.create({ - connectionId, - taskId, - checkId, - checkName: checkDef.name, - }); - try { - // Run the specific check const accessToken = getStringValue(credentials.access_token); const result = await runAllChecks({ manifest, @@ -442,7 +537,7 @@ export class TaskIntegrationsController { variables, connectionId, organizationId, - checkId, // Only run this specific check + checkId: checkDef.id, // Only run this specific check onTokenRefresh, logger: { info: (msg, data) => this.logger.log(msg, data), @@ -452,7 +547,6 @@ export class TaskIntegrationsController { }); const checkResult = result.results[0]; - if (!checkResult) { await this.checkRunRepository.complete(checkRun.id, { status: 'failed', @@ -462,12 +556,16 @@ export class TaskIntegrationsController { failedCount: 0, errorMessage: 'Check not found in manifest', }); - return { success: false, error: 'Check not found' }; + return { + connectionId, + checkRunId: checkRun.id, + status: 'error', + findings: 0, + passing: 0, + }; } - // Store individual results const resultsToStore = [ - // Passing results ...checkResult.result.passingResults.map((r) => ({ checkRunId: checkRun.id, passed: true, @@ -477,7 +575,6 @@ export class TaskIntegrationsController { description: r.description, evidence: r.evidence as Prisma.InputJsonValue, })), - // Findings (failures) ...checkResult.result.findings.map((f) => ({ checkRunId: checkRun.id, passed: false, @@ -485,12 +582,7 @@ export class TaskIntegrationsController { resourceId: f.resourceId, title: f.title, description: f.description, - severity: f.severity as - | 'info' - | 'low' - | 'medium' - | 'high' - | 'critical', + severity: f.severity, remediation: f.remediation, evidence: f.evidence as Prisma.InputJsonValue, })), @@ -500,7 +592,6 @@ export class TaskIntegrationsController { await this.checkRunRepository.addResults(resultsToStore); } - // Complete the check run await this.checkRunRepository.complete(checkRun.id, { status: checkResult.status === 'error' ? 'failed' : checkResult.status, durationMs: checkResult.durationMs, @@ -517,70 +608,36 @@ export class TaskIntegrationsController { }); this.logger.log( - `Check ${checkId} for task ${taskId}: ${checkResult.status} - ${checkResult.result.findings.length} findings, ${checkResult.result.passingResults.length} passing`, + `Check ${checkDef.id} for task ${taskId} (connection ${connectionId}): ${checkResult.status} - ${checkResult.result.findings.length} findings, ${checkResult.result.passingResults.length} passing`, ); - // Update task status based on check results - const hasFindings = checkResult.result.findings.length > 0; - const hasPassing = checkResult.result.passingResults.length > 0; - const newStatus = hasFindings ? 'failed' : hasPassing ? 'done' : null; - - if (newStatus) { - // Only update review date if transitioning to done from a different status - const isTransitioningToDone = - newStatus === 'done' && task.status !== 'done'; - - // Calculate next review date based on frequency - let reviewDate: Date | undefined; - if (isTransitioningToDone && task.frequency) { - reviewDate = new Date(); - switch (task.frequency) { - case 'monthly': - reviewDate.setMonth(reviewDate.getMonth() + 1); - break; - case 'quarterly': - reviewDate.setMonth(reviewDate.getMonth() + 3); - break; - case 'yearly': - reviewDate.setFullYear(reviewDate.getFullYear() + 1); - break; - } - } - - await db.task.update({ - where: { id: taskId }, - data: { - status: newStatus, - ...(reviewDate ? { reviewDate } : {}), - }, - }); - this.logger.log( - `Updated task ${taskId} status to ${newStatus}${reviewDate ? `, next review: ${reviewDate.toISOString()}` : ''}`, - ); - } - return { - success: true, - result: checkResult, + connectionId, checkRunId: checkRun.id, - taskStatus: newStatus, + status: checkResult.status, + findings: checkResult.result.findings.length, + passing: checkResult.result.passingResults.length, }; } catch (error) { - // Mark run as failed await this.checkRunRepository.complete(checkRun.id, { status: 'failed', - durationMs: Date.now() - checkRun.startedAt!.getTime(), + durationMs: checkRun.startedAt + ? Date.now() - checkRun.startedAt.getTime() + : 0, totalChecked: 0, passedCount: 0, failedCount: 0, errorMessage: error instanceof Error ? error.message : String(error), }); - - this.logger.error(`Failed to run check: ${error}`); + this.logger.error( + `Failed to run check ${checkDef.id} for connection ${connectionId}: ${error}`, + ); return { - success: false, - error: error instanceof Error ? error.message : String(error), + connectionId, checkRunId: checkRun.id, + status: 'error', + findings: 0, + passing: 0, }; } } @@ -641,10 +698,15 @@ export class TaskIntegrationsController { @Param('taskId') taskId: string, @Query('limit') limit?: string, ) { - const runs = await this.checkRunRepository.findByTask( - taskId, - limit ? parseInt(limit, 10) : 10, - ); + // Latest run per (connection, check) is guaranteed present — so a customer + // with multiple accounts always sees every account, never just the most + // recently re-run one. `connectionId` + `connectionLabel` let the UI show + // which account each run belongs to. + const runs = + await this.checkRunRepository.findLatestPerConnectionAndCheckByTask( + taskId, + { historyPerGroup: limit ? parseInt(limit, 10) : 5 }, + ); return { runs: runs.map((run) => { @@ -663,6 +725,8 @@ export class TaskIntegrationsController { failedCount: run.failedCount, errorMessage: run.errorMessage, logs: run.logs, + connectionId: run.connectionId, + connectionLabel: getConnectionLabel(run.connection), provider: { slug: provider?.slug, name: provider?.name, diff --git a/apps/api/src/integration-platform/repositories/check-run.repository.spec.ts b/apps/api/src/integration-platform/repositories/check-run.repository.spec.ts new file mode 100644 index 0000000000..66c07c7509 --- /dev/null +++ b/apps/api/src/integration-platform/repositories/check-run.repository.spec.ts @@ -0,0 +1,164 @@ +jest.mock('@db', () => ({ + db: { + integrationCheckRun: { + groupBy: jest.fn(), + findMany: jest.fn(), + }, + }, +})); + +import { db } from '@db'; +import { CheckRunRepository } from './check-run.repository'; + +// Grab through the module reference to avoid the `unbound-method` lint rule +// that fires when extracting an instance method from an object literal. +const mockedCheckRun = db.integrationCheckRun as unknown as { + groupBy: jest.Mock; + findMany: jest.Mock; +}; +const mockGroupBy = mockedCheckRun.groupBy; +const mockFindMany = mockedCheckRun.findMany; + +function makeRun(opts: { + id: string; + connectionId: string; + checkId?: string; + createdAt: string; +}) { + return { + id: opts.id, + connectionId: opts.connectionId, + checkId: opts.checkId ?? 'aws-s3-encryption', + createdAt: new Date(opts.createdAt), + results: [], + connection: { + id: opts.connectionId, + metadata: {}, + provider: { slug: 'aws' }, + }, + }; +} + +describe('CheckRunRepository.findLatestPerConnectionAndCheckByTask', () => { + const repo = new CheckRunRepository(); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('returns nothing when the task has no runs', async () => { + mockGroupBy.mockResolvedValue([]); + const result = await repo.findLatestPerConnectionAndCheckByTask('task_1'); + expect(result).toEqual([]); + expect(mockFindMany).not.toHaveBeenCalled(); + }); + + it('GUARANTEES every account’s latest run even when one account dominates the recent window', async () => { + // 3 accounts (A, B, C) each ran the same check once; A was then re-run many + // times most recently. A flat "newest N rows" limit would bury B and C. + mockGroupBy.mockResolvedValue([ + { + connectionId: 'A', + checkId: 'aws-s3-encryption', + _max: { createdAt: new Date('2026-06-09T15:00:00Z') }, + }, + { + connectionId: 'B', + checkId: 'aws-s3-encryption', + _max: { createdAt: new Date('2026-06-01T09:00:00Z') }, + }, + { + connectionId: 'C', + checkId: 'aws-s3-encryption', + _max: { createdAt: new Date('2026-06-01T08:00:00Z') }, + }, + ]); + + const latestPerGroup = [ + makeRun({ + id: 'rA', + connectionId: 'A', + createdAt: '2026-06-09T15:00:00Z', + }), + makeRun({ + id: 'rB', + connectionId: 'B', + createdAt: '2026-06-01T09:00:00Z', + }), + makeRun({ + id: 'rC', + connectionId: 'C', + createdAt: '2026-06-01T08:00:00Z', + }), + ]; + // Recent window is dominated by account A's burst of re-runs. + const recentWindow = [ + makeRun({ + id: 'rA', + connectionId: 'A', + createdAt: '2026-06-09T15:00:00Z', + }), + makeRun({ + id: 'rA2', + connectionId: 'A', + createdAt: '2026-06-09T14:00:00Z', + }), + makeRun({ + id: 'rA3', + connectionId: 'A', + createdAt: '2026-06-09T13:00:00Z', + }), + makeRun({ + id: 'rA4', + connectionId: 'A', + createdAt: '2026-06-09T12:00:00Z', + }), + makeRun({ + id: 'rA5', + connectionId: 'A', + createdAt: '2026-06-09T11:00:00Z', + }), + ]; + + // First findMany = OR-of-tuples (latest per group); second = recent window. + mockFindMany.mockImplementation((args: { where?: { OR?: unknown } }) => + Promise.resolve(args.where?.OR ? latestPerGroup : recentWindow), + ); + + const result = await repo.findLatestPerConnectionAndCheckByTask('task_1'); + const connectionIds = new Set(result.map((r) => r.connectionId)); + + expect(connectionIds.has('A')).toBe(true); + expect(connectionIds.has('B')).toBe(true); + expect(connectionIds.has('C')).toBe(true); + // Newest-first ordering preserved. + expect(result[0].id).toBe('rA'); + // Deduped by id (rA appears in both the latest set and the recent window). + expect(result.filter((r) => r.id === 'rA')).toHaveLength(1); + }); + + it('excludes disconnected connections in both queries', async () => { + mockGroupBy.mockResolvedValue([ + { + connectionId: 'A', + checkId: 'c', + _max: { createdAt: new Date('2026-06-09T15:00:00Z') }, + }, + ]); + mockFindMany.mockResolvedValue([ + makeRun({ + id: 'rA', + connectionId: 'A', + createdAt: '2026-06-09T15:00:00Z', + }), + ]); + + await repo.findLatestPerConnectionAndCheckByTask('task_1'); + + for (const call of mockFindMany.mock.calls) { + expect(call[0].where.connection).toEqual({ + status: { not: 'disconnected' }, + }); + } + }); +}); diff --git a/apps/api/src/integration-platform/repositories/check-run.repository.ts b/apps/api/src/integration-platform/repositories/check-run.repository.ts index b5a535412c..681e607b8c 100644 --- a/apps/api/src/integration-platform/repositories/check-run.repository.ts +++ b/apps/api/src/integration-platform/repositories/check-run.repository.ts @@ -1,6 +1,6 @@ import { Injectable } from '@nestjs/common'; import { db } from '@db'; -import type { IntegrationRunStatus, Prisma } from '@db'; +import type { Prisma } from '@db'; export interface CreateCheckRunDto { connectionId: string; @@ -149,6 +149,79 @@ export class CheckRunRepository { }); } + /** + * Get check runs for a task, GUARANTEEING every (connection, check) group's + * most recent run is included (plus a bounded history tail). + * + * Why not just `findByTask` with a row limit: checks run once per connected + * account, so a customer with multiple AWS accounts has one run per account. + * A flat "newest N rows" limit can silently drop an account whose latest run + * predates a burst of re-runs on a busier account — that account then + * vanishes from the task UI. Here we first establish the per-group maxima via + * `groupBy`, fetch each group's latest run explicitly (completeness), then add + * a recent window for history. Used by the task UI so manual and scheduled + * runs both surface every account. + */ + async findLatestPerConnectionAndCheckByTask( + taskId: string, + { historyPerGroup = 5 }: { historyPerGroup?: number } = {}, + ) { + const include = { + results: true, + connection: { include: { provider: true } }, + } as const; + + const where = { + taskId, + connection: { status: { not: 'disconnected' } }, + } satisfies Prisma.IntegrationCheckRunWhereInput; + + // Every (connection, check) group that has runs for this task, with its + // most recent run timestamp. + const groups = await db.integrationCheckRun.groupBy({ + by: ['connectionId', 'checkId'], + where, + _max: { createdAt: true }, + }); + if (groups.length === 0) return []; + + // Completeness guarantee: explicitly fetch each group's latest run. + const tuples = groups.flatMap((g) => + g._max.createdAt + ? [ + { + connectionId: g.connectionId, + checkId: g.checkId, + createdAt: g._max.createdAt, + }, + ] + : [], + ); + const latest = tuples.length + ? await db.integrationCheckRun.findMany({ + where: { ...where, OR: tuples }, + include, + }) + : []; + + // History tail: a bounded recent window. Combined with the guaranteed + // latest set, every account shows its current result and recently-active + // accounts also show past runs. + const recent = await db.integrationCheckRun.findMany({ + where, + include, + orderBy: { createdAt: 'desc' }, + take: groups.length * historyPerGroup, + }); + + // Merge + dedupe by id, newest-first (preserves the /runs ordering contract). + const byId = new Map(); + for (const run of [...latest, ...recent]) byId.set(run.id, run); + return Array.from(byId.values()).sort( + (a, b) => b.createdAt.getTime() - a.createdAt.getTime(), + ); + } + /** * Get the latest check run for a specific check on a task */ diff --git a/apps/api/src/integration-platform/repositories/connection.repository.ts b/apps/api/src/integration-platform/repositories/connection.repository.ts index 260d202b43..c15fc5a798 100644 --- a/apps/api/src/integration-platform/repositories/connection.repository.ts +++ b/apps/api/src/integration-platform/repositories/connection.repository.ts @@ -88,6 +88,28 @@ export class ConnectionRepository { }); } + /** + * All active connections for a single provider in an org. Used to run a + * check against every connected account (e.g. each AWS account a customer + * has connected) rather than only the first one. + */ + async findActiveByProviderAndOrg( + providerId: string, + organizationId: string, + ): Promise { + return db.integrationConnection.findMany({ + where: { + providerId, + organizationId, + status: 'active', + }, + include: { + provider: true, + }, + orderBy: { createdAt: 'asc' }, + }); + } + async create(data: CreateConnectionDto): Promise { return db.integrationConnection.create({ data: { diff --git a/apps/api/src/integration-platform/utils/connection-label.spec.ts b/apps/api/src/integration-platform/utils/connection-label.spec.ts new file mode 100644 index 0000000000..08168fc605 --- /dev/null +++ b/apps/api/src/integration-platform/utils/connection-label.spec.ts @@ -0,0 +1,79 @@ +import { getConnectionLabel } from './connection-label'; + +describe('getConnectionLabel', () => { + it('prefers the customer-set connection name', () => { + expect( + getConnectionLabel({ + id: 'conn1234abcd', + metadata: { + connectionName: 'Production AWS', + accountId: '123456789012', + }, + }), + ).toBe('Production AWS'); + }); + + it('trims a padded connection name', () => { + expect( + getConnectionLabel({ + id: 'conn1234abcd', + metadata: { connectionName: ' Prod ' }, + }), + ).toBe('Prod'); + }); + + it('falls back to "AWS "', () => { + expect( + getConnectionLabel({ + id: 'conn1234abcd', + metadata: { accountId: '123456789012' }, + }), + ).toBe('AWS 123456789012'); + }); + + it('derives the account id from a commercial role ARN', () => { + expect( + getConnectionLabel({ + id: 'conn1234abcd', + metadata: { + roleArn: 'arn:aws:iam::953349023881:role/CompSecurityAudit', + }, + }), + ).toBe('AWS 953349023881'); + }); + + it('derives the account id from a GovCloud role ARN', () => { + expect( + getConnectionLabel({ + id: 'conn1234abcd', + metadata: { roleArn: 'arn:aws-us-gov:iam::633779453318:role/x' }, + }), + ).toBe('AWS 633779453318'); + }); + + it('falls back to an id slice when metadata is empty', () => { + // slice(4, 12) of 'conn1234abcd' → '1234abcd' + expect(getConnectionLabel({ id: 'conn1234abcd', metadata: {} })).toBe( + 'Account 1234abcd', + ); + }); + + it('handles missing or non-object metadata', () => { + expect(getConnectionLabel({ id: 'conn1234abcd' })).toBe('Account 1234abcd'); + expect(getConnectionLabel({ id: 'conn1234abcd', metadata: null })).toBe( + 'Account 1234abcd', + ); + expect( + getConnectionLabel({ id: 'conn1234abcd', metadata: 'not-an-object' }), + ).toBe('Account 1234abcd'); + }); + + it('ignores a blank connection name and falls through', () => { + expect( + getConnectionLabel({ + id: 'conn1234abcd', + metadata: { connectionName: ' ', accountId: '123456789012' }, + }), + ).toBe('AWS 123456789012'); + }); +}); diff --git a/apps/api/src/integration-platform/utils/connection-label.ts b/apps/api/src/integration-platform/utils/connection-label.ts new file mode 100644 index 0000000000..e8964e2d36 --- /dev/null +++ b/apps/api/src/integration-platform/utils/connection-label.ts @@ -0,0 +1,33 @@ +/** + * Human-readable label for an integration connection (one customer account). + * + * Mirrors the frontend `getConnectionDisplayLabel` + * (apps/app/.../integrations/[slug]/components/connection-display.ts) so the + * label is consistent with the account picker. Reads only `metadata` (an + * unencrypted Json column) — never credentials — so it is safe to compute + * server-side without decrypting anything. + * + * Precedence: customer-set connection name → `AWS ` → + * `AWS ` → `Account ` fallback. + */ +export function getConnectionLabel(connection: { + id: string; + metadata?: unknown; +}): string { + const meta = + connection.metadata && typeof connection.metadata === 'object' + ? (connection.metadata as Record) + : {}; + + if (typeof meta.connectionName === 'string' && meta.connectionName.trim()) { + return meta.connectionName.trim(); + } + if (typeof meta.accountId === 'string' && meta.accountId) { + return `AWS ${meta.accountId}`; + } + if (typeof meta.roleArn === 'string') { + const arnMatch = meta.roleArn.match(/arn:(?:aws|aws-us-gov):iam::(\d{12})/); + if (arnMatch) return `AWS ${arnMatch[1]}`; + } + return `Account ${connection.id.slice(4, 12)}`; +} diff --git a/apps/app/src/app/(app)/[orgId]/tasks/[taskId]/components/TaskIntegrationChecks.tsx b/apps/app/src/app/(app)/[orgId]/tasks/[taskId]/components/TaskIntegrationChecks.tsx index 2d7b7b19d7..21539b9968 100644 --- a/apps/app/src/app/(app)/[orgId]/tasks/[taskId]/components/TaskIntegrationChecks.tsx +++ b/apps/app/src/app/(app)/[orgId]/tasks/[taskId]/components/TaskIntegrationChecks.tsx @@ -45,7 +45,8 @@ import { useCallback, useEffect, useMemo, useRef, useState, type ChangeEvent } f import { toast } from 'sonner'; import type { StoredCheckRun, TaskIntegrationCheck } from '../hooks/useIntegrationChecks'; import { useIntegrationChecks } from '../hooks/useIntegrationChecks'; -import { EvidenceJsonView } from './EvidenceJsonView'; +import { summarizeLatestPerAccount } from './check-run-grouping'; +import { AccountRunGroups } from './check-run-history'; interface TaskIntegrationChecksProps { taskId: string; @@ -458,19 +459,18 @@ export function TaskIntegrationChecks({
{connectedChecks.map((check) => { const checkRuns = runsByCheck[check.checkId] || []; - const latestRun = checkRuns[0]; // Already sorted by createdAt desc + // A check runs once per connected account; summarize the latest + // run of EACH account so the header reflects all of them, not + // just the most recently run one. + const summary = summarizeLatestPerAccount(checkRuns); const isRunning = runningCheck === check.checkId; const isExpanded = expandedCheck === check.checkId; const needsConfig = check.needsConfiguration; const monitorName = getMonitorDisplayName(check); - // Determine status from latest run - const hasFailed = latestRun - ? latestRun.status === 'failed' || latestRun.failedCount > 0 - : false; - const hasSucceeded = latestRun - ? latestRun.status === 'success' && latestRun.failedCount === 0 - : false; + // Status across all accounts' latest runs. + const hasFailed = summary.hasFailed; + const hasSucceeded = summary.hasSucceeded; const dotColor = needsConfig ? 'bg-warning shadow-[0_0_8px_hsl(var(--warning)/0.4)]' @@ -480,8 +480,8 @@ export function TaskIntegrationChecks({ ? 'bg-primary shadow-[0_0_8px_rgba(0,77,64,0.4)]' : 'bg-muted-foreground'; - const lastRan = latestRun - ? formatDistanceToNow(new Date(latestRun.completedAt || latestRun.createdAt), { + const lastRan = summary.lastRunAt + ? formatDistanceToNow(new Date(summary.lastRunAt), { addSuffix: true, }) : null; @@ -555,14 +555,17 @@ export function TaskIntegrationChecks({ ) : lastRan ? (

Last ran {lastRan} - {latestRun && ( + + • {summary.passed} passed + {summary.failed > 0 && ( + + , {summary.failed} issues + + )} + + {summary.accountCount > 1 && ( - • {latestRun.passedCount} passed - {latestRun.failedCount > 0 && ( - - , {latestRun.failedCount} issues - - )} + · {summary.accountCount} accounts )}

@@ -704,9 +707,8 @@ export function TaskIntegrationChecks({ >
-
@@ -953,283 +955,6 @@ export function TaskIntegrationChecks({ ); } -// Group runs by date for display -function GroupedCheckRuns({ - runs, - maxRuns = 5, - organizationName, -}: { - runs: StoredCheckRun[]; - maxRuns?: number; - organizationName: string; -}) { - const [showAll, setShowAll] = useState(false); - - // Get the runs to display (limited or all) - const displayRuns = showAll ? runs : runs.slice(0, maxRuns); - const hasMore = runs.length > maxRuns; - - // Build grouped display from displayRuns - const displayGrouped = useMemo((): Record => { - const groups: Record = {}; - - displayRuns.forEach((run) => { - const date = new Date(run.createdAt).toLocaleDateString('en-US', { - month: 'short', - day: 'numeric', - year: 'numeric', - }); - if (!groups[date]) { - groups[date] = []; - } - groups[date].push(run); - }); - - return groups; - }, [displayRuns]); - - if (runs.length === 0) { - return

No runs yet

; - } - - let runIndex = 0; - - return ( -
- {Object.entries(displayGrouped).map(([date, dateRuns]) => ( -
-

- {date} -

-
- {dateRuns.map((run: StoredCheckRun) => { - const isLatest = runIndex === 0; - runIndex++; - return ( - - ); - })} -
-
- ))} - - {hasMore && ( - - )} -
- ); -} - -// Individual check run item with expandable details -function CheckRunItem({ - run, - isLatest, - organizationName, -}: { - run: StoredCheckRun; - isLatest: boolean; - organizationName: string; -}) { - const [expanded, setExpanded] = useState(isLatest); - - const timeAgo = formatDistanceToNow(new Date(run.createdAt), { addSuffix: true }); - const hasFailed = run.status === 'failed' || run.failedCount > 0; - const hasError = run.status === 'failed' && run.errorMessage; - - const findings = run.results.filter((r) => !r.passed); - const passing = run.results.filter((r) => r.passed); - - const statusColor = hasError ? 'text-destructive' : hasFailed ? 'text-warning' : 'text-primary'; - - const statusText = hasError ? 'Error' : hasFailed ? 'Issues Found' : 'Passed'; - - return ( -
- - -
-
-
- {/* Error */} - {run.errorMessage && ( -
-

{run.errorMessage}

-
- )} - - {/* Findings */} - {findings.length > 0 && ( -
- {findings.slice(0, 3).map((finding) => ( -
-
-

{finding.title}

- {finding.description && ( -

{finding.description}

- )} - {finding.remediation && ( -

{finding.remediation}

- )} -
- - {finding.resourceId} - - {finding.severity && ( - - {finding.severity} - - )} -
-
- {finding.evidence && Object.keys(finding.evidence).length > 0 && ( -
- - View Evidence - - -
- )} -
- ))} - {findings.length > 3 && ( -

- +{findings.length - 3} more issues -

- )} -
- )} - - {/* Passing Results - always show when there are passing results */} - {passing.length > 0 && ( -
- - ✓ {passing.length} passed - -
- {passing.slice(0, 3).map((result) => ( -
-
-

{result.title}

- {result.description && ( -

{result.description}

- )} - - {result.resourceId} - -
- {result.evidence && Object.keys(result.evidence).length > 0 && ( -
- - View Evidence - - -
- )} -
- ))} - {passing.length > 3 && ( -

- +{passing.length - 3} more passed -

- )} -
-
- )} - - {/* Logs */} - {run.logs && run.logs.length > 0 && ( -
- Logs - 
-                  {run.logs.map((log, i) => (
-                    

-                      
-                        [{new Date(log.timestamp).toLocaleTimeString()}]
-                      {' '}
-                      {log.message}
-                    

-                  ))}
-
-
- )} -
-
-
-
- ); -} - // Empty state when no integrations are connected - matches AutomationEmptyState style function IntegrationEmptyState({ disconnectedChecks, diff --git a/apps/app/src/app/(app)/[orgId]/tasks/[taskId]/components/check-run-grouping.test.ts b/apps/app/src/app/(app)/[orgId]/tasks/[taskId]/components/check-run-grouping.test.ts new file mode 100644 index 0000000000..1816b21cb2 --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/tasks/[taskId]/components/check-run-grouping.test.ts @@ -0,0 +1,93 @@ +import { describe, expect, it } from 'vitest'; +import type { StoredCheckRun } from '../hooks/useIntegrationChecks'; +import { + groupRunsByConnection, + summarizeLatestPerAccount, +} from './check-run-grouping'; + +const makeRun = ( + overrides: Partial & { + id: string; + connectionId: string; + }, +): StoredCheckRun => { + const createdAt = overrides.createdAt ?? '2026-06-09T00:00:00.000Z'; + return { + checkId: 'aws-s3-encryption', + checkName: 'S3 — default encryption enabled', + status: 'success', + startedAt: createdAt, + durationMs: 100, + totalChecked: 1, + passedCount: 0, + failedCount: 0, + connectionLabel: `AWS ${overrides.connectionId}`, + provider: { slug: 'aws', name: 'AWS' }, + results: [], + ...overrides, + // completedAt tracks createdAt (as in real runs) unless explicitly set. + completedAt: overrides.completedAt ?? createdAt, + createdAt, + }; +}; + +describe('groupRunsByConnection', () => { + it('groups runs by account, preserving order across and within accounts', () => { + const runs = [ + makeRun({ id: 'r1', connectionId: 'A', createdAt: '2026-06-09T12:00:00Z' }), + makeRun({ id: 'r2', connectionId: 'B', createdAt: '2026-06-09T11:00:00Z' }), + makeRun({ id: 'r3', connectionId: 'A', createdAt: '2026-06-09T10:00:00Z' }), + ]; + const groups = groupRunsByConnection(runs); + expect(groups.map((g) => g.connectionId)).toEqual(['A', 'B']); + expect(groups[0].runs.map((r) => r.id)).toEqual(['r1', 'r3']); + expect(groups[1].runs.map((r) => r.id)).toEqual(['r2']); + }); + + it('uses the connection label, falling back when absent', () => { + const groups = groupRunsByConnection([ + makeRun({ id: 'r1', connectionId: 'A', connectionLabel: 'Production AWS' }), + makeRun({ id: 'r2', connectionId: 'B', connectionLabel: '' }), + ]); + expect(groups[0].label).toBe('Production AWS'); + expect(groups[1].label).toBe('Account'); + }); +}); + +describe('summarizeLatestPerAccount', () => { + it('sums the latest run of each account', () => { + const runs = [ + // Account A: latest is newest (12:00) with 30 passed. + makeRun({ id: 'a1', connectionId: 'A', passedCount: 30, createdAt: '2026-06-09T12:00:00Z' }), + makeRun({ id: 'a0', connectionId: 'A', passedCount: 5, createdAt: '2026-06-09T08:00:00Z' }), + // Account B: latest has 2 findings. + makeRun({ + id: 'b1', + connectionId: 'B', + status: 'failed', + passedCount: 10, + failedCount: 2, + createdAt: '2026-06-09T11:00:00Z', + }), + ]; + const summary = summarizeLatestPerAccount(runs); + expect(summary.accountCount).toBe(2); + expect(summary.passed).toBe(40); // 30 (A latest) + 10 (B latest), NOT A's older 5 + expect(summary.failed).toBe(2); + expect(summary.hasFailed).toBe(true); + expect(summary.hasSucceeded).toBe(true); // A passed + expect(summary.lastRunAt).toBe('2026-06-09T12:00:00Z'); + }); + + it('is empty for no runs', () => { + const summary = summarizeLatestPerAccount([]); + expect(summary).toEqual({ + accountCount: 0, + passed: 0, + failed: 0, + lastRunAt: null, + hasFailed: false, + hasSucceeded: false, + }); + }); +}); diff --git a/apps/app/src/app/(app)/[orgId]/tasks/[taskId]/components/check-run-grouping.ts b/apps/app/src/app/(app)/[orgId]/tasks/[taskId]/components/check-run-grouping.ts new file mode 100644 index 0000000000..7ff52e945b --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/tasks/[taskId]/components/check-run-grouping.ts @@ -0,0 +1,83 @@ +import type { StoredCheckRun } from '../hooks/useIntegrationChecks'; + +export interface AccountRunGroup { + connectionId: string; + label: string; + /** This account's runs, newest-first. */ + runs: StoredCheckRun[]; +} + +/** + * Group check runs by the account (connection) they ran against. Checks run + * once per connected account, so this is how the UI shows every account's + * results when a customer has more than one (e.g. multiple AWS accounts). + * + * Input is expected newest-first; the output preserves that order both across + * accounts (ordered by each account's most recent run) and within each account. + */ +export function groupRunsByConnection( + runs: StoredCheckRun[], +): AccountRunGroup[] { + const groups = new Map(); + for (const run of runs) { + const existing = groups.get(run.connectionId); + if (existing) { + existing.runs.push(run); + } else { + groups.set(run.connectionId, { + connectionId: run.connectionId, + label: run.connectionLabel || 'Account', + runs: [run], + }); + } + } + return Array.from(groups.values()); +} + +export interface RunsSummary { + accountCount: number; + passed: number; + failed: number; + lastRunAt: string | null; + hasFailed: boolean; + hasSucceeded: boolean; +} + +/** + * Aggregate the latest run per account for the card header — so a multi-account + * check shows totals across all accounts, not just the most recently run one. + */ +export function summarizeLatestPerAccount( + runs: StoredCheckRun[], +): RunsSummary { + const groups = groupRunsByConnection(runs); + let passed = 0; + let failed = 0; + let hasFailed = false; + let hasSucceeded = false; + let lastRunAt: string | null = null; + + for (const group of groups) { + const latest = group.runs[0]; // newest-first + if (!latest) continue; + passed += latest.passedCount; + failed += latest.failedCount; + if (latest.status === 'failed' || latest.failedCount > 0) hasFailed = true; + if (latest.status === 'success' && latest.failedCount === 0) { + hasSucceeded = true; + } + const at = latest.completedAt || latest.createdAt; + if (at && (!lastRunAt || new Date(at) > new Date(lastRunAt))) { + lastRunAt = at; + } + } + + return { + accountCount: groups.length, + passed, + failed, + lastRunAt, + hasFailed, + hasSucceeded, + }; +} diff --git a/apps/app/src/app/(app)/[orgId]/tasks/[taskId]/components/check-run-history.tsx b/apps/app/src/app/(app)/[orgId]/tasks/[taskId]/components/check-run-history.tsx new file mode 100644 index 0000000000..fa98e045b8 --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/tasks/[taskId]/components/check-run-history.tsx @@ -0,0 +1,347 @@ +'use client'; + +import { cn } from '@/lib/utils'; +import { Badge } from '@trycompai/ui/badge'; +import { formatDistanceToNow } from 'date-fns'; +import { ChevronDown } from 'lucide-react'; +import { useMemo, useState } from 'react'; +import type { StoredCheckRun } from '../hooks/useIntegrationChecks'; +import { groupRunsByConnection } from './check-run-grouping'; +import { EvidenceJsonView } from './EvidenceJsonView'; + +/** + * Run history for a check, grouped by the account (connection) it ran against. + * + * With a single account this renders identically to the previous date-grouped + * history (no account header). With multiple accounts — e.g. a customer who + * connected several AWS accounts — each account gets its own labelled section + * showing that account's runs, so the one check surfaces results + logs for + * every account. Both manual and scheduled runs flow through here. + */ +export function AccountRunGroups({ + runs, + organizationName, +}: { + runs: StoredCheckRun[]; + organizationName: string; +}) { + const groups = useMemo(() => groupRunsByConnection(runs), [runs]); + + if (groups.length <= 1) { + return ; + } + + return ( +
+ {groups.map((group) => { + const latest = group.runs[0]; + const hasFailed = latest + ? latest.status === 'failed' || latest.failedCount > 0 + : false; + return ( +
+
+ + + {group.label} + + {latest && ( + + {latest.passedCount} passed + {latest.failedCount > 0 ? `, ${latest.failedCount} issues` : ''} + + )} +
+ +
+ ); + })} +
+ ); +} + +// Group runs by date for display +export function GroupedCheckRuns({ + runs, + maxRuns = 5, + organizationName, +}: { + runs: StoredCheckRun[]; + maxRuns?: number; + organizationName: string; +}) { + const [showAll, setShowAll] = useState(false); + + // Get the runs to display (limited or all) + const displayRuns = showAll ? runs : runs.slice(0, maxRuns); + const hasMore = runs.length > maxRuns; + + // Build grouped display from displayRuns + const displayGrouped = useMemo((): Record => { + const groups: Record = {}; + + displayRuns.forEach((run) => { + const date = new Date(run.createdAt).toLocaleDateString('en-US', { + month: 'short', + day: 'numeric', + year: 'numeric', + }); + if (!groups[date]) { + groups[date] = []; + } + groups[date].push(run); + }); + + return groups; + }, [displayRuns]); + + if (runs.length === 0) { + return

No runs yet

; + } + + let runIndex = 0; + + return ( +
+ {Object.entries(displayGrouped).map(([date, dateRuns]) => ( +
+

+ {date} +

+
+ {dateRuns.map((run: StoredCheckRun) => { + const isLatest = runIndex === 0; + runIndex++; + return ( + + ); + })} +
+
+ ))} + + {hasMore && ( + + )} +
+ ); +} + +// Individual check run item with expandable details +export function CheckRunItem({ + run, + isLatest, + organizationName, +}: { + run: StoredCheckRun; + isLatest: boolean; + organizationName: string; +}) { + const [expanded, setExpanded] = useState(isLatest); + + const timeAgo = formatDistanceToNow(new Date(run.createdAt), { addSuffix: true }); + const hasFailed = run.status === 'failed' || run.failedCount > 0; + const hasError = run.status === 'failed' && run.errorMessage; + + const findings = run.results.filter((r) => !r.passed); + const passing = run.results.filter((r) => r.passed); + + const statusColor = hasError ? 'text-destructive' : hasFailed ? 'text-warning' : 'text-primary'; + + const statusText = hasError ? 'Error' : hasFailed ? 'Issues Found' : 'Passed'; + + return ( +
+ + +
+
+
+ {/* Error */} + {run.errorMessage && ( +
+

{run.errorMessage}

+
+ )} + + {/* Findings */} + {findings.length > 0 && ( +
+ {findings.slice(0, 3).map((finding) => ( +
+
+

{finding.title}

+ {finding.description && ( +

{finding.description}

+ )} + {finding.remediation && ( +

{finding.remediation}

+ )} +
+ + {finding.resourceId} + + {finding.severity && ( + + {finding.severity} + + )} +
+
+ {finding.evidence && Object.keys(finding.evidence).length > 0 && ( +
+ + View Evidence + + +
+ )} +
+ ))} + {findings.length > 3 && ( +

+ +{findings.length - 3} more issues +

+ )} +
+ )} + + {/* Passing Results - always show when there are passing results */} + {passing.length > 0 && ( +
+ + ✓ {passing.length} passed + +
+ {passing.slice(0, 3).map((result) => ( +
+
+

{result.title}

+ {result.description && ( +

{result.description}

+ )} + + {result.resourceId} + +
+ {result.evidence && Object.keys(result.evidence).length > 0 && ( +
+ + View Evidence + + +
+ )} +
+ ))} + {passing.length > 3 && ( +

+ +{passing.length - 3} more passed +

+ )} +
+
+ )} + + {/* Logs */} + {run.logs && run.logs.length > 0 && ( +
+ Logs + 
+                  {run.logs.map((log, i) => (
+                    

+                      
+                        [{new Date(log.timestamp).toLocaleTimeString()}]
+                      {' '}
+                      {log.message}
+                    

+                  ))}
+
+
+ )} +
+
+
+
+ ); +} diff --git a/apps/app/src/app/(app)/[orgId]/tasks/[taskId]/hooks/useIntegrationChecks.ts b/apps/app/src/app/(app)/[orgId]/tasks/[taskId]/hooks/useIntegrationChecks.ts index 3c85c3c171..064c39b4d9 100644 --- a/apps/app/src/app/(app)/[orgId]/tasks/[taskId]/hooks/useIntegrationChecks.ts +++ b/apps/app/src/app/(app)/[orgId]/tasks/[taskId]/hooks/useIntegrationChecks.ts @@ -32,6 +32,10 @@ interface StoredCheckRun { passedCount: number; failedCount: number; errorMessage?: string; + /** The connection (account) this run belongs to — checks run once per account. */ + connectionId: string; + /** Human-readable account label (e.g. "AWS 123456789012" or a custom name). */ + connectionLabel: string; logs?: Array<{ level: string; message: string; From 18144fd7622b20a9db271b7357f3107d1ca080c7 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Tue, 9 Jun 2026 16:38:12 -0400 Subject: [PATCH 2/4] fix(people): respect "Send portal invite email" opt-out when adding users MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When adding a user via "+ Add User" on the People page, unchecking "Send portal invite email" did not suppress the email. The frontend and DTO correctly passed sendPortalEmail through, but addEmployeeWithoutInvite (taken for strictly-employee roles) still emailed the user in its else branch — sending an InviteEmail with a portal link instead of nothing. Now the portal invite email is sent only when requested. When opted out, the member is added silently. The caller also stops surfacing emailSent on an intentional skip, so the UI's "invite email could not be sent" warning no longer fires when the admin deliberately suppressed the email. Updates two existing specs that asserted the old always-send behavior and adds a regression test for the employee opt-out path. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/people/people-invite.service.spec.ts | 52 ++++++++++++++++++- apps/api/src/people/people-invite.service.ts | 35 ++++++------- 2 files changed, 67 insertions(+), 20 deletions(-) diff --git a/apps/api/src/people/people-invite.service.spec.ts b/apps/api/src/people/people-invite.service.spec.ts index 05e5cd77c8..c35c4bf780 100644 --- a/apps/api/src/people/people-invite.service.spec.ts +++ b/apps/api/src/people/people-invite.service.spec.ts @@ -309,7 +309,11 @@ describe('PeopleInviteService', () => { }); expect(results[0].success).toBe(true); - expect(results[0].emailSent).toBe(true); + // No portal invite was requested (sendPortalEmail omitted = false), so no + // email is sent and emailSent is not surfaced (the UI only warns on an + // actual send failure, never on an intentional skip). + expect(mockTriggerEmail).not.toHaveBeenCalled(); + expect(results[0].emailSent).toBeUndefined(); expect(mockDb.member.create).toHaveBeenCalledWith( expect.objectContaining({ data: expect.objectContaining({ @@ -437,9 +441,13 @@ describe('PeopleInviteService', () => { const results = await service.inviteMembers({ ...baseParams, - invites: [{ email: 'emp@example.com', roles: ['employee'] }], + invites: [ + { email: 'emp@example.com', roles: ['employee'], sendPortalEmail: true }, + ], }); + // A portal email was requested but the send failed — the member is still + // added and emailSent: false signals the UI to offer a resend. expect(results[0].success).toBe(true); expect(results[0].emailSent).toBe(false); }); @@ -545,6 +553,46 @@ describe('PeopleInviteService', () => { expect(mockInviteEmail).not.toHaveBeenCalled(); }); + // Regression: unchecking "Send portal invite email" when adding an + // employee via "+ Add User" must add the member WITHOUT emailing them. + // Previously the else-branch still sent an InviteEmail with a portal link. + it('employee only with portal UNchecked: adds member silently, sends no email', async () => { + (mockDb.organization.findUnique as jest.Mock).mockResolvedValue({ + name: 'Test Org', + }); + (mockDb.user.findFirst as jest.Mock).mockResolvedValue(null); + (mockDb.user.create as jest.Mock).mockResolvedValue({ + id: 'usr_emp', + email: 'emp@example.com', + }); + (mockDb.member.findFirst as jest.Mock).mockResolvedValue(null); + (mockDb.member.create as jest.Mock).mockResolvedValue({ id: 'mem_emp' }); + ( + mockDb.employeeTrainingVideoCompletion.createMany as jest.Mock + ).mockResolvedValue({ count: 5 }); + + const results = await service.inviteMembers({ + ...baseParams, + invites: [ + { + email: 'emp@example.com', + roles: ['employee'], + sendPortalEmail: false, + }, + ], + }); + + // Member is still created... + expect(results[0].success).toBe(true); + expect(mockDb.member.create).toHaveBeenCalled(); + // ...but NO email of any kind goes out when the portal invite is off. + expect(mockTriggerEmail).not.toHaveBeenCalled(); + expect(mockInvitePortalEmail).not.toHaveBeenCalled(); + expect(mockInviteEmail).not.toHaveBeenCalled(); + // And no false "could not be sent" warning leaks to the UI. + expect(results[0].emailSent).toBeUndefined(); + }); + it('admin only (no portal): sends app email without portal link', async () => { setupNewUserInvite(); diff --git a/apps/api/src/people/people-invite.service.ts b/apps/api/src/people/people-invite.service.ts index 094b91a1db..0ed1f2586a 100644 --- a/apps/api/src/people/people-invite.service.ts +++ b/apps/api/src/people/people-invite.service.ts @@ -106,7 +106,10 @@ export class PeopleInviteService { results.push({ email: invite.email, success: true, - emailSent: result.emailSent, + // Only surface email status when we actually attempted to send, so + // the UI's "invite email could not be sent" warning never fires for + // an intentional skip (portal invite unchecked). + ...(shouldSendPortalEmail ? { emailSent: result.emailSent } : {}), }); } else { await this.inviteWithCheck({ @@ -208,10 +211,12 @@ export class PeopleInviteService { await this.createTrainingVideoEntries(member.id, organizationId); } - // Send invite email (non-fatal) - let emailSent = true; - try { - if (sendPortalEmail) { + // Send the portal invite email only when requested (non-fatal). When the + // admin opts out ("Send portal invite email" unchecked) we add the member + // silently and send no email at all. + let emailSent = false; + if (sendPortalEmail) { + try { const inviteLink = this.buildPortalUrl(organizationId); await triggerEmail({ to: email, @@ -222,20 +227,14 @@ export class PeopleInviteService { email, }), }); - } else { - const inviteLink = this.buildPortalUrl(organizationId); - await triggerEmail({ - to: email, - subject: `You've been invited to join ${organization.name} on Comp AI`, - react: InviteEmail({ organizationName: organization.name, inviteLink }), - }); + emailSent = true; + } catch (emailErr) { + emailSent = false; + this.logger.error( + `Portal invite email failed after member was added: ${email}`, + emailErr instanceof Error ? emailErr.message : 'Unknown error', + ); } - } catch (emailErr) { - emailSent = false; - this.logger.error( - `Invite email failed after member was added: ${email}`, - emailErr instanceof Error ? emailErr.message : 'Unknown error', - ); } return { emailSent }; From c82f9fc3d4d130cb2a483bb52a314cbbcb297c29 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Tue, 9 Jun 2026 16:38:50 -0400 Subject: [PATCH 3/4] fix(cloud-security): validate run-history limit and reject inactive connections MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses two cubic review findings on PR #3067: - check-run.repository.ts: clamp historyPerGroup (which flows from the user-supplied ?limit= query param via parseInt → possibly NaN/negative/huge) to [1, 50], defaulting to 5, so it can never produce an invalid Prisma take (500) or an unbounded, expensive read. - task-integrations.controller.ts: restore the active-status guard on the referenced connection (dropped during the run-all-accounts refactor) so a manual run rejects an inactive connection up front; the empty-active fallback can now only ever contain a verified-active connection. Tests: historyPerGroup clamp (oversized → cap, NaN/negative → default) and inactive-connection rejection. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../task-integrations.controller.spec.ts | 20 +++++++++ .../task-integrations.controller.ts | 11 ++++- .../repositories/check-run.repository.spec.ts | 43 +++++++++++++++++++ .../repositories/check-run.repository.ts | 18 +++++++- 4 files changed, 89 insertions(+), 3 deletions(-) diff --git a/apps/api/src/integration-platform/controllers/task-integrations.controller.spec.ts b/apps/api/src/integration-platform/controllers/task-integrations.controller.spec.ts index 6efed78f88..5dde5c696e 100644 --- a/apps/api/src/integration-platform/controllers/task-integrations.controller.spec.ts +++ b/apps/api/src/integration-platform/controllers/task-integrations.controller.spec.ts @@ -199,6 +199,7 @@ describe('TaskIntegrationsController', () => { id: 'conn_1', organizationId: 'org_1', providerId: 'prov_aws', + status: 'active', }); mockConnectionRepository.findActiveByProviderAndOrg.mockResolvedValue([ { @@ -236,6 +237,7 @@ describe('TaskIntegrationsController', () => { id: 'conn_1', organizationId: 'org_1', providerId: 'prov_aws', + status: 'active', }); mockConnectionRepository.findActiveByProviderAndOrg.mockResolvedValue([ { @@ -274,6 +276,7 @@ describe('TaskIntegrationsController', () => { id: 'conn_1', organizationId: 'org_1', providerId: 'prov_aws', + status: 'active', }); mockConnectionRepository.findActiveByProviderAndOrg.mockResolvedValue([ { @@ -310,6 +313,23 @@ describe('TaskIntegrationsController', () => { expect(result.accountsRun).toBe(2); expect(result.success).toBe(true); }); + + it('rejects an inactive referenced connection (never runs a non-active account)', async () => { + mockConnectionRepository.findById.mockResolvedValue({ + id: 'conn_1', + organizationId: 'org_1', + providerId: 'prov_aws', + status: 'paused', + }); + + await expect( + controller.runCheckForTask('task_1', 'org_1', body), + ).rejects.toThrow('Connection is not active'); + expect( + mockConnectionRepository.findActiveByProviderAndOrg, + ).not.toHaveBeenCalled(); + expect(mockedRunAllChecks).not.toHaveBeenCalled(); + }); }); describe('getTaskCheckRuns', () => { diff --git a/apps/api/src/integration-platform/controllers/task-integrations.controller.ts b/apps/api/src/integration-platform/controllers/task-integrations.controller.ts index 86e5b1aa6f..c20c04bf64 100644 --- a/apps/api/src/integration-platform/controllers/task-integrations.controller.ts +++ b/apps/api/src/integration-platform/controllers/task-integrations.controller.ts @@ -339,6 +339,15 @@ export class TaskIntegrationsController { throw new HttpException('Connection not found', HttpStatus.NOT_FOUND); } + // Reject inactive connections up front. This also keeps the fallback below + // safe: it can only ever contain a connection we've verified is active. + if (referencedConnection.status !== 'active') { + throw new HttpException( + 'Connection is not active', + HttpStatus.BAD_REQUEST, + ); + } + const provider = await this.providerRepository.findById( referencedConnection.providerId, ); @@ -365,7 +374,7 @@ export class TaskIntegrationsController { organizationId, ); // Never run zero accounts: if a status race leaves the active query empty, - // fall back to the referenced connection. + // fall back to the referenced connection (verified active above). const connections = activeConnections.length > 0 ? activeConnections : [referencedConnection]; diff --git a/apps/api/src/integration-platform/repositories/check-run.repository.spec.ts b/apps/api/src/integration-platform/repositories/check-run.repository.spec.ts index 66c07c7509..a2f5eaf816 100644 --- a/apps/api/src/integration-platform/repositories/check-run.repository.spec.ts +++ b/apps/api/src/integration-platform/repositories/check-run.repository.spec.ts @@ -161,4 +161,47 @@ describe('CheckRunRepository.findLatestPerConnectionAndCheckByTask', () => { }); } }); + + it('clamps an oversized historyPerGroup to the cap (no unbounded read)', async () => { + mockGroupBy.mockResolvedValue([ + { + connectionId: 'A', + checkId: 'c', + _max: { createdAt: new Date('2026-06-09T15:00:00Z') }, + }, + ]); + mockFindMany.mockResolvedValue([]); + + await repo.findLatestPerConnectionAndCheckByTask('task_1', { + historyPerGroup: 100000, + }); + + // recent-window query = the findMany WITHOUT an OR clause. + const recentCall = mockFindMany.mock.calls.find((c) => !c[0].where.OR); + expect(recentCall?.[0].take).toBe(1 * 50); // groups.length(1) * MAX(50) + }); + + it('falls back to the default for an invalid historyPerGroup (NaN/negative)', async () => { + mockGroupBy.mockResolvedValue([ + { + connectionId: 'A', + checkId: 'c', + _max: { createdAt: new Date('2026-06-09T15:00:00Z') }, + }, + ]); + mockFindMany.mockResolvedValue([]); + + await repo.findLatestPerConnectionAndCheckByTask('task_1', { + historyPerGroup: Number.NaN, + }); + let recentCall = mockFindMany.mock.calls.find((c) => !c[0].where.OR); + expect(recentCall?.[0].take).toBe(1 * 5); // default 5 + + mockFindMany.mockClear(); + await repo.findLatestPerConnectionAndCheckByTask('task_1', { + historyPerGroup: -10, + }); + recentCall = mockFindMany.mock.calls.find((c) => !c[0].where.OR); + expect(recentCall?.[0].take).toBe(1 * 5); // default 5 + }); }); diff --git a/apps/api/src/integration-platform/repositories/check-run.repository.ts b/apps/api/src/integration-platform/repositories/check-run.repository.ts index 681e607b8c..b66fec86e4 100644 --- a/apps/api/src/integration-platform/repositories/check-run.repository.ts +++ b/apps/api/src/integration-platform/repositories/check-run.repository.ts @@ -2,6 +2,10 @@ import { Injectable } from '@nestjs/common'; import { db } from '@db'; import type { Prisma } from '@db'; +/** Default / hard cap for run-history depth per (connection, check) group. */ +const DEFAULT_HISTORY_PER_GROUP = 5; +const MAX_HISTORY_PER_GROUP = 50; + export interface CreateCheckRunDto { connectionId: string; taskId?: string; @@ -164,8 +168,18 @@ export class CheckRunRepository { */ async findLatestPerConnectionAndCheckByTask( taskId: string, - { historyPerGroup = 5 }: { historyPerGroup?: number } = {}, + { + historyPerGroup = DEFAULT_HISTORY_PER_GROUP, + }: { historyPerGroup?: number } = {}, ) { + // `historyPerGroup` can originate from a user-supplied `?limit=` query param + // (parsed with parseInt → possibly NaN/negative/huge). Clamp it so it never + // produces an invalid `take` (500) or an unbounded, expensive read. + const perGroup = + Number.isInteger(historyPerGroup) && historyPerGroup > 0 + ? Math.min(historyPerGroup, MAX_HISTORY_PER_GROUP) + : DEFAULT_HISTORY_PER_GROUP; + const include = { results: true, connection: { include: { provider: true } }, @@ -211,7 +225,7 @@ export class CheckRunRepository { where, include, orderBy: { createdAt: 'desc' }, - take: groups.length * historyPerGroup, + take: groups.length * perGroup, }); // Merge + dedupe by id, newest-first (preserves the /runs ordering contract). From 612d62403be69d11420cd767367cd0389d5f6c63 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Tue, 9 Jun 2026 16:55:27 -0400 Subject: [PATCH 4/4] fix(cloud-security): tenant-scope the task check-runs endpoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses a cubic finding on PR #3067 (pre-existing on main): GET :taskId/runs read check runs by taskId with no organization check, so an arbitrary taskId from another org would leak cross-tenant run data (account ids, connection labels, logs). Add @OrganizationId and validate the task belongs to the caller's org (404 otherwise) before returning runs — mirroring runCheckForTask / getChecksForTask. Test: rejects a task outside the caller's org and never queries runs. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../task-integrations.controller.spec.ts | 14 +++++++++++++- .../controllers/task-integrations.controller.ts | 12 ++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/apps/api/src/integration-platform/controllers/task-integrations.controller.spec.ts b/apps/api/src/integration-platform/controllers/task-integrations.controller.spec.ts index 5dde5c696e..93ea32103a 100644 --- a/apps/api/src/integration-platform/controllers/task-integrations.controller.spec.ts +++ b/apps/api/src/integration-platform/controllers/task-integrations.controller.spec.ts @@ -383,7 +383,7 @@ describe('TaskIntegrationsController', () => { ], ); - const { runs } = await controller.getTaskCheckRuns('task_1'); + const { runs } = await controller.getTaskCheckRuns('task_1', 'org_1'); expect(runs).toHaveLength(2); expect(runs[0]).toMatchObject({ @@ -395,5 +395,17 @@ describe('TaskIntegrationsController', () => { connectionLabel: 'AWS 222222222222', }); }); + + it('rejects a task that does not belong to the caller’s org (no cross-tenant leak)', async () => { + // Task lookup scoped to { id, organizationId } returns nothing. + mockTaskFindUnique.mockResolvedValue(null); + + await expect( + controller.getTaskCheckRuns('task_other_org', 'org_1'), + ).rejects.toThrow('Task not found'); + expect( + mockCheckRunRepository.findLatestPerConnectionAndCheckByTask, + ).not.toHaveBeenCalled(); + }); }); }); diff --git a/apps/api/src/integration-platform/controllers/task-integrations.controller.ts b/apps/api/src/integration-platform/controllers/task-integrations.controller.ts index c20c04bf64..64bf9ae378 100644 --- a/apps/api/src/integration-platform/controllers/task-integrations.controller.ts +++ b/apps/api/src/integration-platform/controllers/task-integrations.controller.ts @@ -705,8 +705,20 @@ export class TaskIntegrationsController { @RequirePermission('integration', 'read') async getTaskCheckRuns( @Param('taskId') taskId: string, + @OrganizationId() organizationId: string, @Query('limit') limit?: string, ) { + // Tenant scoping: confirm the task belongs to the caller's org before + // returning its check runs. The runs carry account ids, connection labels, + // and logs — without this an arbitrary taskId would leak cross-tenant data. + const task = await db.task.findUnique({ + where: { id: taskId, organizationId }, + select: { id: true }, + }); + if (!task) { + throw new HttpException('Task not found', HttpStatus.NOT_FOUND); + } + // Latest run per (connection, check) is guaranteed present — so a customer // with multiple accounts always sees every account, never just the most // recently re-run one. `connectionId` + `connectionLabel` let the UI show