Skip to content

Commit c93fe46

Browse files
authored
Merge pull request #3282 from trycompai/main
[comp] Production Deploy
2 parents e23110b + 44caba8 commit c93fe46

17 files changed

Lines changed: 640 additions & 167 deletions

File tree

‎apps/api/src/cloud-security/finding-exceptions.spec.ts‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,30 @@ describe('ActiveExceptionSet', () => {
2020
expect(set.has('c2', 'aws-s3-public-access', 'bucket-1')).toBe(false);
2121
expect(set.size).toBe(1);
2222
});
23+
24+
it('exposes excepted resourceIds grouped by (connectionId, checkId)', () => {
25+
const set = new ActiveExceptionSet([
26+
ActiveExceptionSet.key('c1', 'check-a', 'r1'),
27+
ActiveExceptionSet.key('c1', 'check-a', 'r2'),
28+
ActiveExceptionSet.key('c1', 'check-b', 'r3'),
29+
]);
30+
expect(new Set(set.exceptedResourceIds('c1', 'check-a'))).toEqual(
31+
new Set(['r1', 'r2']),
32+
);
33+
expect(set.exceptedResourceIds('c1', 'check-b')).toEqual(['r3']);
34+
// Nothing excepted for this pair → empty (callers skip the count query).
35+
expect(set.exceptedResourceIds('c1', 'check-c')).toEqual([]);
36+
expect(set.exceptedResourceIds('c2', 'check-a')).toEqual([]);
37+
});
38+
39+
it('reconstructs resourceIds that themselves contain the "::" delimiter', () => {
40+
const resourceId = 'arn:aws:s3:::my::weird::bucket';
41+
const set = new ActiveExceptionSet([
42+
ActiveExceptionSet.key('c1', 'check-a', resourceId),
43+
]);
44+
expect(set.has('c1', 'check-a', resourceId)).toBe(true);
45+
expect(set.exceptedResourceIds('c1', 'check-a')).toEqual([resourceId]);
46+
});
2347
});
2448

2549
describe('loadActiveExceptionSet', () => {

‎apps/api/src/cloud-security/finding-exceptions.ts‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,31 @@ import { db } from '@db';
1818
*/
1919
export class ActiveExceptionSet {
2020
private readonly keys: Set<string>;
21+
/**
22+
* Excepted resourceIds grouped by `connectionId::checkId`. Lets a caller
23+
* count a run's excepted failures with a targeted query instead of loading
24+
* every result row into memory.
25+
*/
26+
private readonly resourceIdsByConnCheck: Map<string, Set<string>>;
2127

2228
constructor(keys: Iterable<string>) {
2329
this.keys = new Set(keys);
30+
this.resourceIdsByConnCheck = new Map();
31+
for (const key of this.keys) {
32+
// key = `${connectionId}::${checkId}::${resourceId}`. A resourceId can
33+
// itself contain "::", so take the first two segments and rejoin the
34+
// rest — reconstructing exactly the resourceId used to build the key.
35+
const parts = key.split('::');
36+
if (parts.length < 3) continue;
37+
const groupKey = `${parts[0]}::${parts[1]}`;
38+
const resourceId = parts.slice(2).join('::');
39+
let ids = this.resourceIdsByConnCheck.get(groupKey);
40+
if (!ids) {
41+
ids = new Set();
42+
this.resourceIdsByConnCheck.set(groupKey, ids);
43+
}
44+
ids.add(resourceId);
45+
}
2446
}
2547

2648
/** Canonical key. The only place this format is defined. */
@@ -41,6 +63,18 @@ export class ActiveExceptionSet {
4163
ActiveExceptionSet.key(connectionId, checkId, resourceId),
4264
);
4365
}
66+
67+
/**
68+
* The excepted resourceIds for a (connection, check) pair. Empty when nothing
69+
* is excepted for that pair — callers use this to skip the count query
70+
* entirely (the common case: no exceptions).
71+
*/
72+
exceptedResourceIds(connectionId: string, checkId: string): string[] {
73+
const ids = this.resourceIdsByConnCheck.get(
74+
`${connectionId}::${checkId}`,
75+
);
76+
return ids ? Array.from(ids) : [];
77+
}
4478
}
4579

4680
/**

‎apps/api/src/integration-platform/controllers/task-integrations.controller.spec.ts‎

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,7 @@ describe('TaskIntegrationsController', () => {
169169
complete: jest.fn(),
170170
addResults: jest.fn(),
171171
findLatestPerConnectionAndCheckByTask: jest.fn(),
172+
countExceptedFailures: jest.fn(),
172173
};
173174
const mockCredentialVaultService = { getDecryptedCredentials: jest.fn() };
174175
const mockOAuthCredentialsService = {
@@ -229,6 +230,9 @@ describe('TaskIntegrationsController', () => {
229230
);
230231
mockCheckRunRepository.complete.mockResolvedValue({});
231232
mockCheckRunRepository.addResults.mockResolvedValue({});
233+
// Default: nothing excepted (no count query needed). Exception tests
234+
// override this to the exact excepted-failure count for the run.
235+
mockCheckRunRepository.countExceptedFailures.mockResolvedValue(0);
232236
mockCredentialVaultService.getDecryptedCredentials.mockResolvedValue(
233237
VALID_CREDS,
234238
);
@@ -564,13 +568,23 @@ describe('TaskIntegrationsController', () => {
564568
resourceId: 'reports-bucket',
565569
},
566570
]);
571+
// The excepted-failure count is now computed via a targeted query (the
572+
// full result set is no longer loaded). reports-bucket is the one
573+
// excepted failing result for this run.
574+
mockCheckRunRepository.countExceptedFailures.mockResolvedValue(1);
567575

568576
const { runs } = await controller.getTaskCheckRuns('task_1', 'org_1');
569577

570578
expect(runs[0].failedCount).toBe(0);
571579
expect(runs[0].exceptedCount).toBe(1);
572580
expect(runs[0].status).toBe('success');
573581
expect(runs[0].results[0].excepted).toBe(true);
582+
// Exact count is computed via the targeted query, scoped to this run's
583+
// excepted resourceIds (not by loading + filtering every result).
584+
expect(mockCheckRunRepository.countExceptedFailures).toHaveBeenCalledWith(
585+
'icr_1',
586+
['reports-bucket'],
587+
);
574588
});
575589

576590
it('keeps an execution-error run as failed (no findings, not excepted)', async () => {
@@ -621,5 +635,108 @@ describe('TaskIntegrationsController', () => {
621635
mockCheckRunRepository.findLatestPerConnectionAndCheckByTask,
622636
).not.toHaveBeenCalled();
623637
});
638+
639+
it('bounds a run with a huge result set + logs so the payload stays small (CS-588)', async () => {
640+
// Defense-in-depth response cap: even if a run somehow carries a large
641+
// result/log set, the serialized response is bounded (results per
642+
// category, evidence size, log count) while the run's summary counts stay
643+
// accurate. The PRIMARY fix — never LOADING all result rows from the DB —
644+
// lives in CheckRunRepository.findLatestPerConnectionAndCheckByTask and is
645+
// covered in check-run.repository.spec.ts.
646+
const HUGE = 5000;
647+
const results = [
648+
// First finding carries an oversized evidence blob.
649+
{
650+
id: 'icx_finding_0',
651+
passed: false,
652+
resourceType: 'firebase-user',
653+
resourceId: 'user_0',
654+
title: 'finding 0',
655+
description: 'd',
656+
severity: 'high',
657+
remediation: 'fix',
658+
evidence: { blob: 'x'.repeat(30_000) },
659+
collectedAt: new Date(),
660+
},
661+
...Array.from({ length: HUGE - 1 }, (_, i) => ({
662+
id: `icx_finding_${i + 1}`,
663+
passed: false,
664+
resourceType: 'firebase-user',
665+
resourceId: `user_f_${i + 1}`,
666+
title: 'finding',
667+
description: 'd',
668+
severity: 'high',
669+
remediation: 'fix',
670+
evidence: { ok: true },
671+
collectedAt: new Date(),
672+
})),
673+
...Array.from({ length: HUGE }, (_, i) => ({
674+
id: `icx_pass_${i}`,
675+
passed: true,
676+
resourceType: 'firebase-user',
677+
resourceId: `user_p_${i}`,
678+
title: 'passing',
679+
description: 'd',
680+
evidence: { ok: true },
681+
collectedAt: new Date(),
682+
})),
683+
];
684+
const logs = Array.from({ length: HUGE }, (_, i) => ({
685+
level: 'info',
686+
message: `log ${i}`,
687+
timestamp: new Date().toISOString(),
688+
}));
689+
690+
mockCheckRunRepository.findLatestPerConnectionAndCheckByTask.mockResolvedValue(
691+
[
692+
{
693+
id: 'icr_huge',
694+
checkId: 'firebase-employee-access',
695+
checkName: 'Employee Access',
696+
status: 'failed',
697+
startedAt: new Date(),
698+
completedAt: new Date(),
699+
durationMs: 10,
700+
totalChecked: HUGE * 2,
701+
passedCount: HUGE,
702+
failedCount: HUGE,
703+
errorMessage: null,
704+
logs,
705+
connectionId: 'conn_1',
706+
createdAt: new Date(),
707+
results,
708+
connection: {
709+
id: 'conn_1',
710+
metadata: { connectionName: 'Firebase' },
711+
provider: { slug: 'firebase', name: 'Firebase' },
712+
},
713+
},
714+
],
715+
);
716+
717+
const { runs } = await controller.getTaskCheckRuns('task_1', 'org_1');
718+
719+
// Result detail is bounded (a few findings + a few passing), NOT 10000.
720+
expect(runs[0].results.length).toBeLessThanOrEqual(15);
721+
expect(runs[0].results.length).toBeLessThan(results.length);
722+
// Logs are bounded too.
723+
expect(Array.isArray(runs[0].logs)).toBe(true);
724+
if (Array.isArray(runs[0].logs)) {
725+
expect(runs[0].logs.length).toBeLessThanOrEqual(100);
726+
}
727+
// Summary counts remain authoritative (computed from the full set).
728+
expect(runs[0].passedCount).toBe(HUGE);
729+
expect(runs[0].failedCount).toBe(HUGE);
730+
expect(runs[0].exceptedCount).toBe(0);
731+
// The oversized evidence blob is replaced with a compact placeholder.
732+
const shippedFinding = runs[0].results.find(
733+
(r) => r.id === 'icx_finding_0',
734+
);
735+
expect(shippedFinding).toBeDefined();
736+
expect(shippedFinding?.evidence).toMatchObject({ truncated: true });
737+
// Normal small evidence is left intact.
738+
const shippedPass = runs[0].results.find((r) => r.passed);
739+
expect(shippedPass?.evidence).toEqual({ ok: true });
740+
});
624741
});
625742
});

‎apps/api/src/integration-platform/controllers/task-integrations.controller.ts‎

Lines changed: 36 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,11 @@ import {
4343
countEffectiveFailures,
4444
decideTaskStatus,
4545
} from '../utils/task-check-evaluation';
46+
import {
47+
capEvidence,
48+
capLogs,
49+
capResultsForList,
50+
} from '../utils/run-history-limits';
4651
import { db } from '@db';
4752
import type { IntegrationConnection, Prisma } from '@db';
4853

@@ -768,11 +773,31 @@ export class TaskIntegrationsController {
768773
// untouched in the DB; this only affects the response.
769774
const exceptions = await loadActiveExceptionSet(organizationId);
770775

771-
return {
772-
runs: runs.map((run) => {
776+
const mappedRuns = await Promise.all(
777+
runs.map(async (run) => {
773778
const provider = getProviderSummary(run.connection);
774779

775-
const results = run.results.map((r) => ({
780+
// `run.results` is a BOUNDED, findings-first sample — the repo caps how
781+
// many rows it loads per run (a check can produce tens of thousands, so
782+
// loading them all hangs/OOMs the request). The effective failure count
783+
// is therefore computed EXACTLY via a targeted count query over the
784+
// full set, NOT by filtering this sample. The query is skipped when
785+
// this (connection, check) has no exceptions — the common case.
786+
const exceptedResourceIds = exceptions.exceptedResourceIds(
787+
run.connectionId,
788+
run.checkId,
789+
);
790+
const exceptedCount =
791+
await this.checkRunRepository.countExceptedFailures(
792+
run.id,
793+
exceptedResourceIds,
794+
);
795+
796+
// Tag each sampled result with whether it's excepted (for display);
797+
// authoritative totals come from the run's summary columns +
798+
// exceptedCount above. Cap evidence so one oversized blob can't bloat
799+
// the payload that the browser must parse + render.
800+
const sample = run.results.map((r) => ({
776801
id: r.id,
777802
passed: r.passed,
778803
resourceType: r.resourceType,
@@ -787,8 +812,11 @@ export class TaskIntegrationsController {
787812
!r.passed &&
788813
exceptions.has(run.connectionId, run.checkId, r.resourceId),
789814
}));
815+
const results = capResultsForList(sample).map((r) => ({
816+
...r,
817+
evidence: capEvidence(r.evidence),
818+
}));
790819

791-
const exceptedCount = results.filter((r) => r.excepted).length;
792820
const effectiveFailed = Math.max(0, run.failedCount - exceptedCount);
793821
// Only downgrade failed → success when the failures were actually
794822
// EXCEPTED. A failed run with no findings (e.g. an execution error,
@@ -812,7 +840,7 @@ export class TaskIntegrationsController {
812840
failedCount: effectiveFailed,
813841
exceptedCount,
814842
errorMessage: run.errorMessage,
815-
logs: run.logs,
843+
logs: capLogs(run.logs),
816844
connectionId: run.connectionId,
817845
connectionLabel: getConnectionLabel(run.connection),
818846
provider: {
@@ -823,6 +851,8 @@ export class TaskIntegrationsController {
823851
createdAt: run.createdAt,
824852
};
825853
}),
826-
};
854+
);
855+
856+
return { runs: mappedRuns };
827857
}
828858
}

0 commit comments

Comments
 (0)