diff --git a/apps/api/src/cloud-security/cloud-security-query.service.ts b/apps/api/src/cloud-security/cloud-security-query.service.ts index 1828b34c38..0fe5241f5d 100644 --- a/apps/api/src/cloud-security/cloud-security-query.service.ts +++ b/apps/api/src/cloud-security/cloud-security-query.service.ts @@ -4,6 +4,7 @@ import { getManifest } from '@trycompai/integration-platform'; import { sanitizeEvidence } from './evidence-sanitizer'; import { getLegacyFindings } from './cloud-security-query.legacy'; import { resolveCheckKey } from './check-definition.utils'; +import { loadActiveExceptionSet } from './finding-exceptions'; import type { CloudFinding, CloudProvider, @@ -198,38 +199,21 @@ export class CloudSecurityQueryService { if (options.includeExceptions) return combined; // Filter out findings under an active (non-revoked, non-expired) - // FindingException. Looked up in one query keyed by org so the cost - // stays constant regardless of finding count. - const activeExceptionKeys = await this.loadActiveExceptionKeys(organizationId); - if (activeExceptionKeys.size === 0) return combined; + // FindingException, via the shared exception set so this view stays matched + // with the task-check status/display (one source of truth). + const exceptions = await loadActiveExceptionSet(organizationId); + if (exceptions.size === 0) return combined; return combined.filter((finding) => { if (!finding.checkKey || !finding.resourceId) return true; - const key = `${finding.connectionId}::${finding.checkKey}::${finding.resourceId}`; - return !activeExceptionKeys.has(key); + return !exceptions.has( + finding.connectionId, + finding.checkKey, + finding.resourceId, + ); }); } - /** - * Return the set of (connectionId, checkId, resourceId) tuples that have - * an active exception in this org. One DB query per getFindings call. - */ - private async loadActiveExceptionKeys( - organizationId: string, - ): Promise> { - const active = await db.findingException.findMany({ - where: { - organizationId, - revokedAt: null, - OR: [{ expiresAt: null }, { expiresAt: { gt: new Date() } }], - }, - select: { connectionId: true, checkId: true, resourceId: true }, - }); - return new Set( - active.map((e) => `${e.connectionId}::${e.checkId}::${e.resourceId}`), - ); - } - private async getLatestRunsByConnection( connectionIds: string[], ): Promise> { diff --git a/apps/api/src/cloud-security/finding-exceptions.spec.ts b/apps/api/src/cloud-security/finding-exceptions.spec.ts new file mode 100644 index 0000000000..36e2b1507a --- /dev/null +++ b/apps/api/src/cloud-security/finding-exceptions.spec.ts @@ -0,0 +1,57 @@ +jest.mock('@db', () => ({ + db: { findingException: { findMany: jest.fn() } }, +})); + +import { db } from '@db'; +import { + ActiveExceptionSet, + loadActiveExceptionSet, +} from './finding-exceptions'; + +const findMany = jest.mocked(db.findingException.findMany); + +describe('ActiveExceptionSet', () => { + it('matches by (connectionId, checkId, resourceId)', () => { + const set = new ActiveExceptionSet([ + ActiveExceptionSet.key('c1', 'aws-s3-public-access', 'bucket-1'), + ]); + expect(set.has('c1', 'aws-s3-public-access', 'bucket-1')).toBe(true); + expect(set.has('c1', 'aws-s3-public-access', 'bucket-2')).toBe(false); + expect(set.has('c2', 'aws-s3-public-access', 'bucket-1')).toBe(false); + expect(set.size).toBe(1); + }); +}); + +describe('loadActiveExceptionSet', () => { + beforeEach(() => jest.clearAllMocks()); + + it('builds the set from active exceptions', async () => { + findMany.mockResolvedValue([ + { + connectionId: 'c1', + checkId: 'aws-s3-public-access', + resourceId: 'bucket-1', + }, + ] as never); + + const set = await loadActiveExceptionSet('org_1'); + + expect(set.has('c1', 'aws-s3-public-access', 'bucket-1')).toBe(true); + // Query only active exceptions (not revoked, not expired). + expect(findMany).toHaveBeenCalledWith( + expect.objectContaining({ + where: expect.objectContaining({ + organizationId: 'org_1', + revokedAt: null, + }), + }), + ); + }); + + it('fail-safe: returns an empty set if the lookup throws (suppress nothing)', async () => { + findMany.mockRejectedValue(new Error('db down')); + const set = await loadActiveExceptionSet('org_1'); + expect(set.size).toBe(0); + expect(set.has('c1', 'aws-s3-public-access', 'bucket-1')).toBe(false); + }); +}); diff --git a/apps/api/src/cloud-security/finding-exceptions.ts b/apps/api/src/cloud-security/finding-exceptions.ts new file mode 100644 index 0000000000..a63158188a --- /dev/null +++ b/apps/api/src/cloud-security/finding-exceptions.ts @@ -0,0 +1,73 @@ +import { db } from '@db'; + +/** + * The set of findings — keyed by (connectionId, checkId, resourceId) — that + * currently have an ACTIVE exception (not revoked, not expired) for an org. + * + * This is the SINGLE source of truth for "is this finding excepted?". Every + * place that turns a check result into a pass/fail must go through here so an + * exception is honored consistently: + * - the Cloud Tests findings view (cloud-security-query.getFindings) + * - the task-check display (task-integrations getTaskCheckRuns) + * - the task-check run paths that set task status (manual run-check + the + * scheduled Trigger task) + * + * Centralizing the key format + the active filter here is deliberate: it's the + * easy-to-get-wrong part, and duplicating it risks honoring an exception in one + * surface but not another. + */ +export class ActiveExceptionSet { + private readonly keys: Set; + + constructor(keys: Iterable) { + this.keys = new Set(keys); + } + + /** Canonical key. The only place this format is defined. */ + static key( + connectionId: string, + checkId: string, + resourceId: string, + ): string { + return `${connectionId}::${checkId}::${resourceId}`; + } + + get size(): number { + return this.keys.size; + } + + has(connectionId: string, checkId: string, resourceId: string): boolean { + return this.keys.has( + ActiveExceptionSet.key(connectionId, checkId, resourceId), + ); + } +} + +/** + * Load active finding exceptions for an org as an {@link ActiveExceptionSet}. + * + * Fail-safe: on any DB error this returns an EMPTY set (suppress nothing) so a + * lookup failure can never hide a real finding — we would rather show a finding + * the customer excepted than silently pass a genuine one. + */ +export async function loadActiveExceptionSet( + organizationId: string, +): Promise { + try { + const active = await db.findingException.findMany({ + where: { + organizationId, + revokedAt: null, + OR: [{ expiresAt: null }, { expiresAt: { gt: new Date() } }], + }, + select: { connectionId: true, checkId: true, resourceId: true }, + }); + return new ActiveExceptionSet( + active.map((e) => + ActiveExceptionSet.key(e.connectionId, e.checkId, e.resourceId), + ), + ); + } catch { + return new ActiveExceptionSet([]); + } +} 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 93ea32103a..ccb2f529ac 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 @@ -21,6 +21,7 @@ jest.mock('@trycompai/auth', () => ({ jest.mock('@db', () => ({ db: { task: { findUnique: jest.fn(), update: jest.fn() }, + findingException: { findMany: jest.fn() }, }, })); @@ -42,6 +43,9 @@ const mockedTask = db.task as unknown as { }; const mockTaskFindUnique = mockedTask.findUnique; const mockTaskUpdate = mockedTask.update; +const mockFindingExceptionFindMany = ( + db as unknown as { findingException: { findMany: jest.Mock } } +).findingException.findMany; const MANIFEST = { id: 'aws', @@ -114,6 +118,44 @@ function failingResult() { }; } +// One passing bucket (b1) + one failing finding (b2) — mirrors the real +// customer case (e.g. 18 pass, 1 fails) used to test exception handling. +function mixedResult() { + 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: [ + { + resourceType: 'aws-s3-bucket', + resourceId: 'b1', + title: 'ok', + description: 'ok', + }, + ], + summary: { totalChecked: 2 }, + logs: [], + }, + }, + ], + }; +} + describe('TaskIntegrationsController', () => { let controller: TaskIntegrationsController; @@ -180,6 +222,8 @@ describe('TaskIntegrationsController', () => { slug: 'aws', }); mockedGetManifest.mockReturnValue(MANIFEST); + // Default: no active exceptions (existing tests behave as before). + mockFindingExceptionFindMany.mockResolvedValue([]); mockCheckRunRepository.create.mockImplementation(() => Promise.resolve({ id: 'icr_x', startedAt: new Date() }), ); @@ -271,6 +315,83 @@ describe('TaskIntegrationsController', () => { ); }); + it('does NOT fail the task when the only finding is excepted (goes done)', async () => { + mockConnectionRepository.findById.mockResolvedValue({ + id: 'conn_1', + organizationId: 'org_1', + providerId: 'prov_aws', + status: 'active', + }); + mockConnectionRepository.findActiveByProviderAndOrg.mockResolvedValue([ + { + id: 'conn_1', + organizationId: 'org_1', + providerId: 'prov_aws', + metadata: {}, + variables: {}, + }, + ]); + // 1 passing (b1) + 1 finding (b2); b2 is under an active exception. + mockedRunAllChecks.mockResolvedValue(mixedResult()); + mockFindingExceptionFindMany.mockResolvedValue([ + { + connectionId: 'conn_1', + checkId: 'aws-s3-encryption', + resourceId: 'b2', + }, + ]); + + const result = await controller.runCheckForTask('task_1', 'org_1', body); + + // The raw finding is still counted/persisted, but it must not fail the + // task — matching the Cloud Tests view + the scheduled run. + expect(result.totalFindings).toBe(1); + expect(result.taskStatus).toBe('done'); + expect(mockTaskUpdate).toHaveBeenCalledWith( + expect.objectContaining({ + data: expect.objectContaining({ status: 'done' }), + }), + ); + }); + + it('goes done when the only finding is excepted and there are NO passing results', async () => { + mockConnectionRepository.findById.mockResolvedValue({ + id: 'conn_1', + organizationId: 'org_1', + providerId: 'prov_aws', + status: 'active', + }); + mockConnectionRepository.findActiveByProviderAndOrg.mockResolvedValue([ + { + id: 'conn_1', + organizationId: 'org_1', + providerId: 'prov_aws', + metadata: {}, + variables: {}, + }, + ]); + // Only a failing finding (b2), zero passing results; b2 is excepted. + mockedRunAllChecks.mockResolvedValue(failingResult()); + mockFindingExceptionFindMany.mockResolvedValue([ + { + connectionId: 'conn_1', + checkId: 'aws-s3-encryption', + resourceId: 'b2', + }, + ]); + + const result = await controller.runCheckForTask('task_1', 'org_1', body); + + // No effective failures + no passing — must still transition to done, not + // stay stuck in the prior status. + expect(result.taskStatus).toBe('done'); + expect(mockTaskUpdate).toHaveBeenCalledWith( + expect.objectContaining({ + data: expect.objectContaining({ status: 'done' }), + }), + ); + }); + it('records a failed run for a bad account but keeps running the rest', async () => { mockConnectionRepository.findById.mockResolvedValue({ id: 'conn_1', @@ -396,6 +517,99 @@ describe('TaskIntegrationsController', () => { }); }); + it('marks excepted results and drops them from the failed count + status', async () => { + mockCheckRunRepository.findLatestPerConnectionAndCheckByTask.mockResolvedValue( + [ + { + id: 'icr_1', + checkId: 'aws-s3-public-access', + checkName: 'S3 public access', + status: 'failed', + startedAt: new Date(), + completedAt: new Date(), + durationMs: 10, + totalChecked: 1, + passedCount: 0, + failedCount: 1, + errorMessage: null, + logs: [], + connectionId: 'conn_1', + createdAt: new Date(), + results: [ + { + id: 'res_1', + passed: false, + resourceType: 'aws-s3-bucket', + resourceId: 'reports-bucket', + title: 'Public access not fully blocked: reports-bucket', + description: 'x', + severity: 'high', + remediation: 'fix', + evidence: {}, + collectedAt: new Date(), + }, + ], + connection: { + id: 'conn_1', + metadata: { connectionName: 'PRIMER' }, + provider: { slug: 'aws', name: 'AWS' }, + }, + }, + ], + ); + mockFindingExceptionFindMany.mockResolvedValue([ + { + connectionId: 'conn_1', + checkId: 'aws-s3-public-access', + resourceId: 'reports-bucket', + }, + ]); + + const { runs } = await controller.getTaskCheckRuns('task_1', 'org_1'); + + expect(runs[0].failedCount).toBe(0); + expect(runs[0].exceptedCount).toBe(1); + expect(runs[0].status).toBe('success'); + expect(runs[0].results[0].excepted).toBe(true); + }); + + it('keeps an execution-error run as failed (no findings, not excepted)', async () => { + // A failed run with zero findings is an execution error, not an + // all-excepted run — it must NOT be rewritten to success. + mockCheckRunRepository.findLatestPerConnectionAndCheckByTask.mockResolvedValue( + [ + { + id: 'icr_err', + checkId: 'aws-s3-public-access', + checkName: 'S3 public access', + status: 'failed', + startedAt: new Date(), + completedAt: new Date(), + durationMs: 10, + totalChecked: 0, + passedCount: 0, + failedCount: 0, + errorMessage: 'Could not assume AWS role', + logs: [], + connectionId: 'conn_1', + createdAt: new Date(), + results: [], + connection: { + id: 'conn_1', + metadata: { connectionName: 'PRIMER' }, + provider: { slug: 'aws', name: 'AWS' }, + }, + }, + ], + ); + mockFindingExceptionFindMany.mockResolvedValue([]); + + const { runs } = await controller.getTaskCheckRuns('task_1', 'org_1'); + + expect(runs[0].status).toBe('failed'); + expect(runs[0].exceptedCount).toBe(0); + }); + 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); 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 64bf9ae378..253dbf75ec 100644 --- a/apps/api/src/integration-platform/controllers/task-integrations.controller.ts +++ b/apps/api/src/integration-platform/controllers/task-integrations.controller.ts @@ -38,6 +38,11 @@ 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 { loadActiveExceptionSet } from '../../cloud-security/finding-exceptions'; +import { + countEffectiveFailures, + decideTaskStatus, +} from '../utils/task-check-evaluation'; import { db } from '@db'; import type { IntegrationConnection, Prisma } from '@db'; @@ -52,6 +57,8 @@ interface ConnectionCheckOutcome { status: 'success' | 'failed' | 'error'; findings: number; passing: number; + /** resourceIds of this account's failing findings, for exception filtering. */ + failingResourceIds: string[]; } interface TaskIntegrationCheck { @@ -383,6 +390,13 @@ export class TaskIntegrationsController { let accountsRun = 0; let hasExecutionError = false; let lastCheckRunId: string | undefined; + // Failing findings across all accounts (keyed like an exception) so task + // status can exclude explicitly-excepted ones below. + const failingFindings: Array<{ + connectionId: string; + checkId: string; + resourceId: string; + }> = []; // Sequential so each per-account run commits as it completes — a slow or // failing account still leaves the earlier accounts' results persisted. @@ -402,6 +416,9 @@ export class TaskIntegrationsController { totalFindings += outcome.findings; totalPassing += outcome.passing; if (outcome.status === 'error') hasExecutionError = true; + for (const resourceId of outcome.failingResourceIds) { + failingFindings.push({ connectionId: conn.id, checkId, resourceId }); + } lastCheckRunId = outcome.checkRunId; } @@ -412,11 +429,21 @@ export class TaskIntegrationsController { ); } - // 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; + // Aggregate task status across ALL accounts, HONORING active finding + // exceptions so an explicitly-excepted finding doesn't fail the task — + // matched with the scheduled run and the Cloud Tests findings view (one + // rule, via the shared helpers). Any real (non-excepted) finding → failed; + // else any passing result → done; else leave unchanged. + const exceptions = await loadActiveExceptionSet(organizationId); + const effectiveFailures = countEffectiveFailures( + failingFindings, + exceptions, + ); + const newStatus = decideTaskStatus( + effectiveFailures, + totalPassing, + totalFindings, + ); if (newStatus) { const isTransitioningToDone = @@ -571,6 +598,7 @@ export class TaskIntegrationsController { status: 'error', findings: 0, passing: 0, + failingResourceIds: [], }; } @@ -626,6 +654,9 @@ export class TaskIntegrationsController { status: checkResult.status, findings: checkResult.result.findings.length, passing: checkResult.result.passingResults.length, + failingResourceIds: checkResult.result.findings.map( + (f) => f.resourceId, + ), }; } catch (error) { await this.checkRunRepository.complete(checkRun.id, { @@ -647,6 +678,7 @@ export class TaskIntegrationsController { status: 'error', findings: 0, passing: 0, + failingResourceIds: [], }; } } @@ -729,21 +761,56 @@ export class TaskIntegrationsController { { historyPerGroup: limit ? parseInt(limit, 10) : 5 }, ); + // Honor active finding exceptions in what the UI shows: a failing result + // under an active exception is surfaced as `excepted` and excluded from the + // run's failed count/status — matched with task status + the Cloud Tests + // findings view (one rule, via the shared exception set). Raw rows are left + // untouched in the DB; this only affects the response. + const exceptions = await loadActiveExceptionSet(organizationId); + return { runs: runs.map((run) => { const provider = getProviderSummary(run.connection); + const results = run.results.map((r) => ({ + id: r.id, + passed: r.passed, + resourceType: r.resourceType, + resourceId: r.resourceId, + title: r.title, + description: r.description, + severity: r.severity, + remediation: r.remediation, + evidence: r.evidence, + collectedAt: r.collectedAt, + excepted: + !r.passed && + exceptions.has(run.connectionId, run.checkId, r.resourceId), + })); + + const exceptedCount = results.filter((r) => r.excepted).length; + const effectiveFailed = Math.max(0, run.failedCount - exceptedCount); + // Only downgrade failed → success when the failures were actually + // EXCEPTED. A failed run with no findings (e.g. an execution error, + // which is persisted as failed with failedCount 0) must stay failed so + // real runtime errors aren't hidden. + const displayStatus = + run.status === 'failed' && effectiveFailed === 0 && exceptedCount > 0 + ? 'success' + : run.status; + return { id: run.id, checkId: run.checkId, checkName: run.checkName, - status: run.status, + status: displayStatus, startedAt: run.startedAt, completedAt: run.completedAt, durationMs: run.durationMs, totalChecked: run.totalChecked, passedCount: run.passedCount, - failedCount: run.failedCount, + failedCount: effectiveFailed, + exceptedCount, errorMessage: run.errorMessage, logs: run.logs, connectionId: run.connectionId, @@ -752,18 +819,7 @@ export class TaskIntegrationsController { slug: provider?.slug, name: provider?.name, }, - results: run.results.map((r) => ({ - id: r.id, - passed: r.passed, - resourceType: r.resourceType, - resourceId: r.resourceId, - title: r.title, - description: r.description, - severity: r.severity, - remediation: r.remediation, - evidence: r.evidence, - collectedAt: r.collectedAt, - })), + results, createdAt: run.createdAt, }; }), diff --git a/apps/api/src/integration-platform/utils/task-check-evaluation.spec.ts b/apps/api/src/integration-platform/utils/task-check-evaluation.spec.ts new file mode 100644 index 0000000000..cb8381ebde --- /dev/null +++ b/apps/api/src/integration-platform/utils/task-check-evaluation.spec.ts @@ -0,0 +1,94 @@ +// ActiveExceptionSet's module imports `db`; mock it (this suite only uses the +// pure class, not the DB-backed loader). +jest.mock('@db', () => ({ db: {} })); + +import { ActiveExceptionSet } from '../../cloud-security/finding-exceptions'; +import { + countEffectiveFailures, + decideTaskStatus, +} from './task-check-evaluation'; + +describe('task-check-evaluation', () => { + const failing = [ + { connectionId: 'c1', checkId: 'chk', resourceId: 'r1' }, + { connectionId: 'c1', checkId: 'chk', resourceId: 'r2' }, + ]; + + describe('countEffectiveFailures', () => { + it('counts every failure when there are no exceptions', () => { + expect(countEffectiveFailures(failing, new ActiveExceptionSet([]))).toBe( + 2, + ); + }); + + it('excludes a failure that is excepted', () => { + const ex = new ActiveExceptionSet([ + ActiveExceptionSet.key('c1', 'chk', 'r2'), + ]); + expect(countEffectiveFailures(failing, ex)).toBe(1); + }); + + it('is zero when every failure is excepted', () => { + const ex = new ActiveExceptionSet([ + ActiveExceptionSet.key('c1', 'chk', 'r1'), + ActiveExceptionSet.key('c1', 'chk', 'r2'), + ]); + expect(countEffectiveFailures(failing, ex)).toBe(0); + }); + + it('does not match an exception for a different connection/check', () => { + const ex = new ActiveExceptionSet([ + ActiveExceptionSet.key('OTHER', 'chk', 'r1'), + ActiveExceptionSet.key('c1', 'OTHER', 'r2'), + ]); + expect(countEffectiveFailures(failing, ex)).toBe(2); + }); + + it('is provider-agnostic — works for AWS, GCP and Azure findings', () => { + const mixed = [ + { + connectionId: 'aws_c', + checkId: 'aws-s3-public-access', + resourceId: 'bucket', + }, + { + connectionId: 'gcp_c', + checkId: 'gcp-storage-no-public-access', + resourceId: 'gs-bucket', + }, + { + connectionId: 'az_c', + checkId: 'azure-storage-secure-transfer', + resourceId: 'sa1', + }, + ]; + // Except only the GCP finding. + const ex = new ActiveExceptionSet([ + ActiveExceptionSet.key( + 'gcp_c', + 'gcp-storage-no-public-access', + 'gs-bucket', + ), + ]); + expect(countEffectiveFailures(mixed, ex)).toBe(2); + }); + }); + + describe('decideTaskStatus', () => { + // (effectiveFailures, totalPassing, totalFindings) + it('failed when there is a real (non-excepted) failure', () => { + expect(decideTaskStatus(1, 5, 1)).toBe('failed'); + }); + it('done when no failures and something passed', () => { + expect(decideTaskStatus(0, 5, 0)).toBe('done'); + }); + it('done when the only finding is excepted and there are NO passing results', () => { + // All findings excepted (effectiveFailures 0) with 0 passing must still + // transition to done, not stay stuck in the prior status. + expect(decideTaskStatus(0, 0, 1)).toBe('done'); + }); + it('null (leave unchanged) when the check evaluated nothing (e.g. all errored)', () => { + expect(decideTaskStatus(0, 0, 0)).toBeNull(); + }); + }); +}); diff --git a/apps/api/src/integration-platform/utils/task-check-evaluation.ts b/apps/api/src/integration-platform/utils/task-check-evaluation.ts new file mode 100644 index 0000000000..6263ad9e4a --- /dev/null +++ b/apps/api/src/integration-platform/utils/task-check-evaluation.ts @@ -0,0 +1,47 @@ +import { ActiveExceptionSet } from '../../cloud-security/finding-exceptions'; + +/** A failing finding, identified the same way an exception is keyed. */ +export interface FailingFinding { + connectionId: string; + checkId: string; + resourceId: string; +} + +/** + * Count failing findings that are NOT under an active exception. Shared by the + * manual run-check and the scheduled Trigger task so both decide task status + * identically — an explicitly-excepted finding must not fail the task in either. + */ +export function countEffectiveFailures( + failing: FailingFinding[], + exceptions: ActiveExceptionSet, +): number { + if (exceptions.size === 0) return failing.length; + return failing.filter( + (f) => !exceptions.has(f.connectionId, f.checkId, f.resourceId), + ).length; +} + +/** + * Decide a task's status from its check results. Canonical rule, shared by the + * manual and scheduled paths: + * - any real (non-excepted) failure → failed + * - else if the check evaluated any resource (a passing result OR a finding, + * including the case where every finding is excepted) → done + * - else leave unchanged (nothing was evaluated, e.g. an all-errored run — + * indeterminate, not a violation; it retries next tick) + * + * `effectiveFailures` is the non-excepted failure count from + * {@link countEffectiveFailures}; `totalFindings` is the RAW finding count so an + * all-excepted run (effectiveFailures 0, no passing results) still transitions + * to done instead of getting stuck in its prior status. + */ +export function decideTaskStatus( + effectiveFailures: number, + totalPassing: number, + totalFindings: number, +): 'failed' | 'done' | null { + if (effectiveFailures > 0) return 'failed'; + if (totalPassing > 0 || totalFindings > 0) return 'done'; + return null; +} diff --git a/apps/api/src/trigger/integration-platform/run-task-integration-checks.ts b/apps/api/src/trigger/integration-platform/run-task-integration-checks.ts index ad7c5d0722..8145f00b4e 100644 --- a/apps/api/src/trigger/integration-platform/run-task-integration-checks.ts +++ b/apps/api/src/trigger/integration-platform/run-task-integration-checks.ts @@ -14,6 +14,12 @@ import { runChecksOnServer, type RunAllChecksResult, } from './run-checks-on-server'; +import { loadActiveExceptionSet } from '../../cloud-security/finding-exceptions'; +import { + countEffectiveFailures, + decideTaskStatus, + type FailingFinding, +} from '../../integration-platform/utils/task-check-evaluation'; /** * Send email notifications for task status change @@ -333,8 +339,10 @@ export const runTaskIntegrationChecks = task({ // Track overall results across all checks for this task let totalFindings = 0; let totalPassing = 0; - let hasFailedChecks = false; let hasExecutionErrors = false; + // Failing findings (keyed like an exception) so task status can exclude + // explicitly-excepted ones below. + const failingFindings: FailingFinding[] = []; // Run only the checks that apply to this task try { @@ -391,7 +399,8 @@ export const runTaskIntegrationChecks = task({ const message = error instanceof Error ? error.message : String(error); const checkDef = manifest.checks?.find((c) => c.id === checkId); - hasFailedChecks = true; + // A transport blip is indeterminate, not a finding: it gates + // integrationLastRunAt (retry next tick) but must not fail the task. hasExecutionErrors = true; await db.integrationCheckRun.create({ data: { @@ -418,11 +427,16 @@ export const runTaskIntegrationChecks = task({ const checkResult = result.results[0]; if (!checkResult) continue; - // Accumulate results + // Accumulate results. Record each failing finding (keyed like an + // exception) so task status can exclude explicitly-excepted ones. totalFindings += checkResult.result.findings.length; totalPassing += checkResult.result.passingResults.length; - if (checkResult.status === 'failed' || checkResult.status === 'error') { - hasFailedChecks = true; + for (const f of checkResult.result.findings) { + failingFindings.push({ + connectionId, + checkId: checkResult.checkId, + resourceId: f.resourceId, + }); } if (checkResult.status === 'error') { hasExecutionErrors = true; @@ -508,10 +522,23 @@ export const runTaskIntegrationChecks = task({ }); } - // Update task status based on check results - // If any findings or check failures, mark as failed - // If all checks pass with no findings, mark as done (only if not already done) - if (totalFindings > 0 || hasFailedChecks) { + // Decide task status from the run, HONORING active finding exceptions so + // an explicitly-excepted finding does not fail the task — matched with the + // manual run-check path and the Cloud Tests findings view (one rule, via + // the shared helpers). Execution errors don't drive status here; they gate + // integrationLastRunAt above so the next tick retries. + const exceptions = await loadActiveExceptionSet(organizationId); + const effectiveFailures = countEffectiveFailures( + failingFindings, + exceptions, + ); + const newStatus = decideTaskStatus( + effectiveFailures, + totalPassing, + totalFindings, + ); + + if (newStatus === 'failed') { // Get current status before updating const taskBeforeUpdate = await db.task.findUnique({ where: { id: taskId }, @@ -524,7 +551,7 @@ export const runTaskIntegrationChecks = task({ data: { status: 'failed' }, }); logger.info( - `Task ${taskId} marked as failed due to ${totalFindings} findings${hasFailedChecks ? ' and failed checks' : ''}`, + `Task ${taskId} marked as failed due to ${effectiveFailures} finding(s)`, ); // Only send email notifications if status actually changed @@ -541,7 +568,7 @@ export const runTaskIntegrationChecks = task({ `Skipping notification: task ${taskId} was already in failed status`, ); } - } else if (totalPassing > 0) { + } else if (newStatus === 'done') { // Only update to done if not already done const currentTask = await db.task.findUnique({ where: { id: taskId }, @@ -584,12 +611,7 @@ export const runTaskIntegrationChecks = task({ checksRun: effectiveCheckIds.length, totalPassing, totalFindings, - taskStatus: - totalFindings > 0 || hasFailedChecks - ? 'failed' - : totalPassing > 0 - ? 'done' - : null, + taskStatus: newStatus, }; } catch (error) { logger.error(`Failed to run checks for task ${taskId}`, { 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 index fa98e045b8..3e96910182 100644 --- 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 @@ -35,9 +35,7 @@ export function AccountRunGroups({
{groups.map((group) => { const latest = group.runs[0]; - const hasFailed = latest - ? latest.status === 'failed' || latest.failedCount > 0 - : false; + const hasFailed = latest ? latest.status === 'failed' || latest.failedCount > 0 : false; return (
@@ -47,9 +45,7 @@ export function AccountRunGroups({ hasFailed ? 'bg-destructive' : 'bg-primary', )} /> - - {group.label} - + {group.label} {latest && ( {latest.passedCount} passed @@ -57,11 +53,7 @@ export function AccountRunGroups({ )}
- +
); })} @@ -162,7 +154,10 @@ export function CheckRunItem({ const hasFailed = run.status === 'failed' || run.failedCount > 0; const hasError = run.status === 'failed' && run.errorMessage; - const findings = run.results.filter((r) => !r.passed); + // Excepted findings are surfaced separately (not as red issues) so the row + // matches the run's status, which the API already computes excluding them. + const findings = run.results.filter((r) => !r.passed && !r.excepted); + const excepted = run.results.filter((r) => r.excepted); const passing = run.results.filter((r) => r.passed); const statusColor = hasError ? 'text-destructive' : hasFailed ? 'text-warning' : 'text-primary'; @@ -276,6 +271,33 @@ export function CheckRunItem({
)} + {/* Excepted - failing findings the customer marked as an exception. + Shown muted (not an issue) so it's clear the exception applied. */} + {excepted.length > 0 && ( +
+ {excepted.slice(0, 3).map((finding) => ( +
+
+

{finding.title}

+ + Exception + +
+
+ + {finding.resourceId} + +
+
+ ))} + {excepted.length > 3 && ( +

+ +{excepted.length - 3} more excepted +

+ )} +
+ )} + {/* Passing Results - always show when there are passing results */} {passing.length > 0 && (
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 064c39b4d9..f0e7f9f827 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 @@ -30,7 +30,10 @@ interface StoredCheckRun { durationMs: number; totalChecked: number; passedCount: number; + /** Effective failures — excludes findings under an active exception. */ failedCount: number; + /** Failing findings suppressed by an active exception. */ + exceptedCount?: number; errorMessage?: string; /** The connection (account) this run belongs to — checks run once per account. */ connectionId: string; @@ -57,11 +60,13 @@ interface StoredCheckRun { remediation?: string; evidence?: Record; collectedAt: string; + /** True when this failing result is suppressed by an active exception. */ + excepted?: boolean; }>; createdAt: string; } -export type { TaskIntegrationCheck, StoredCheckRun }; +export type { StoredCheckRun, TaskIntegrationCheck }; export const integrationChecksKey = (taskId: string, orgId: string) => ['/v1/integrations/tasks/checks', taskId, orgId] as const; @@ -145,20 +150,17 @@ export function useIntegrationChecks({ taskId, orgId }: UseIntegrationChecksOpti * stays connected — only the (task, check) pair is affected. Applies an * optimistic update to the SWR cache and revalidates in the background. */ - const disconnectCheckFromTask = async ( - connectionId: string, - checkId: string, - ): Promise => { + const disconnectCheckFromTask = async (connectionId: string, checkId: string): Promise => { await mutateChecks( async (current) => { const response = await api.post<{ success: boolean; disabled: true; error?: string; - }>( - `/v1/integrations/tasks/${taskId}/checks/disconnect?organizationId=${orgId}`, - { connectionId, checkId }, - ); + }>(`/v1/integrations/tasks/${taskId}/checks/disconnect?organizationId=${orgId}`, { + connectionId, + checkId, + }); if (response.error || !response.data?.success) { throw new Error(response.error || 'Failed to disconnect check'); @@ -186,20 +188,17 @@ export function useIntegrationChecks({ taskId, orgId }: UseIntegrationChecksOpti /** * Re-enable a previously disconnected check for the current task. */ - const reconnectCheckToTask = async ( - connectionId: string, - checkId: string, - ): Promise => { + const reconnectCheckToTask = async (connectionId: string, checkId: string): Promise => { await mutateChecks( async (current) => { const response = await api.post<{ success: boolean; disabled: false; error?: string; - }>( - `/v1/integrations/tasks/${taskId}/checks/reconnect?organizationId=${orgId}`, - { connectionId, checkId }, - ); + }>(`/v1/integrations/tasks/${taskId}/checks/reconnect?organizationId=${orgId}`, { + connectionId, + checkId, + }); if (response.error || !response.data?.success) { throw new Error(response.error || 'Failed to reconnect check');