Skip to content

Commit 02d1317

Browse files
authored
Merge pull request #3136 from trycompai/main
[comp] Production Deploy
2 parents 7897da1 + e3a1495 commit 02d1317

10 files changed

Lines changed: 659 additions & 91 deletions

File tree

‎apps/api/src/cloud-security/cloud-security-query.service.ts‎

Lines changed: 10 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { getManifest } from '@trycompai/integration-platform';
44
import { sanitizeEvidence } from './evidence-sanitizer';
55
import { getLegacyFindings } from './cloud-security-query.legacy';
66
import { resolveCheckKey } from './check-definition.utils';
7+
import { loadActiveExceptionSet } from './finding-exceptions';
78
import type {
89
CloudFinding,
910
CloudProvider,
@@ -198,38 +199,21 @@ export class CloudSecurityQueryService {
198199
if (options.includeExceptions) return combined;
199200

200201
// Filter out findings under an active (non-revoked, non-expired)
201-
// FindingException. Looked up in one query keyed by org so the cost
202-
// stays constant regardless of finding count.
203-
const activeExceptionKeys = await this.loadActiveExceptionKeys(organizationId);
204-
if (activeExceptionKeys.size === 0) return combined;
202+
// FindingException, via the shared exception set so this view stays matched
203+
// with the task-check status/display (one source of truth).
204+
const exceptions = await loadActiveExceptionSet(organizationId);
205+
if (exceptions.size === 0) return combined;
205206

206207
return combined.filter((finding) => {
207208
if (!finding.checkKey || !finding.resourceId) return true;
208-
const key = `${finding.connectionId}::${finding.checkKey}::${finding.resourceId}`;
209-
return !activeExceptionKeys.has(key);
209+
return !exceptions.has(
210+
finding.connectionId,
211+
finding.checkKey,
212+
finding.resourceId,
213+
);
210214
});
211215
}
212216

213-
/**
214-
* Return the set of (connectionId, checkId, resourceId) tuples that have
215-
* an active exception in this org. One DB query per getFindings call.
216-
*/
217-
private async loadActiveExceptionKeys(
218-
organizationId: string,
219-
): Promise<Set<string>> {
220-
const active = await db.findingException.findMany({
221-
where: {
222-
organizationId,
223-
revokedAt: null,
224-
OR: [{ expiresAt: null }, { expiresAt: { gt: new Date() } }],
225-
},
226-
select: { connectionId: true, checkId: true, resourceId: true },
227-
});
228-
return new Set(
229-
active.map((e) => `${e.connectionId}::${e.checkId}::${e.resourceId}`),
230-
);
231-
}
232-
233217
private async getLatestRunsByConnection(
234218
connectionIds: string[],
235219
): Promise<Map<string, CloudProviderLatestRun>> {
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
jest.mock('@db', () => ({
2+
db: { findingException: { findMany: jest.fn() } },
3+
}));
4+
5+
import { db } from '@db';
6+
import {
7+
ActiveExceptionSet,
8+
loadActiveExceptionSet,
9+
} from './finding-exceptions';
10+
11+
const findMany = jest.mocked(db.findingException.findMany);
12+
13+
describe('ActiveExceptionSet', () => {
14+
it('matches by (connectionId, checkId, resourceId)', () => {
15+
const set = new ActiveExceptionSet([
16+
ActiveExceptionSet.key('c1', 'aws-s3-public-access', 'bucket-1'),
17+
]);
18+
expect(set.has('c1', 'aws-s3-public-access', 'bucket-1')).toBe(true);
19+
expect(set.has('c1', 'aws-s3-public-access', 'bucket-2')).toBe(false);
20+
expect(set.has('c2', 'aws-s3-public-access', 'bucket-1')).toBe(false);
21+
expect(set.size).toBe(1);
22+
});
23+
});
24+
25+
describe('loadActiveExceptionSet', () => {
26+
beforeEach(() => jest.clearAllMocks());
27+
28+
it('builds the set from active exceptions', async () => {
29+
findMany.mockResolvedValue([
30+
{
31+
connectionId: 'c1',
32+
checkId: 'aws-s3-public-access',
33+
resourceId: 'bucket-1',
34+
},
35+
] as never);
36+
37+
const set = await loadActiveExceptionSet('org_1');
38+
39+
expect(set.has('c1', 'aws-s3-public-access', 'bucket-1')).toBe(true);
40+
// Query only active exceptions (not revoked, not expired).
41+
expect(findMany).toHaveBeenCalledWith(
42+
expect.objectContaining({
43+
where: expect.objectContaining({
44+
organizationId: 'org_1',
45+
revokedAt: null,
46+
}),
47+
}),
48+
);
49+
});
50+
51+
it('fail-safe: returns an empty set if the lookup throws (suppress nothing)', async () => {
52+
findMany.mockRejectedValue(new Error('db down'));
53+
const set = await loadActiveExceptionSet('org_1');
54+
expect(set.size).toBe(0);
55+
expect(set.has('c1', 'aws-s3-public-access', 'bucket-1')).toBe(false);
56+
});
57+
});
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
import { db } from '@db';
2+
3+
/**
4+
* The set of findings — keyed by (connectionId, checkId, resourceId) — that
5+
* currently have an ACTIVE exception (not revoked, not expired) for an org.
6+
*
7+
* This is the SINGLE source of truth for "is this finding excepted?". Every
8+
* place that turns a check result into a pass/fail must go through here so an
9+
* exception is honored consistently:
10+
* - the Cloud Tests findings view (cloud-security-query.getFindings)
11+
* - the task-check display (task-integrations getTaskCheckRuns)
12+
* - the task-check run paths that set task status (manual run-check + the
13+
* scheduled Trigger task)
14+
*
15+
* Centralizing the key format + the active filter here is deliberate: it's the
16+
* easy-to-get-wrong part, and duplicating it risks honoring an exception in one
17+
* surface but not another.
18+
*/
19+
export class ActiveExceptionSet {
20+
private readonly keys: Set<string>;
21+
22+
constructor(keys: Iterable<string>) {
23+
this.keys = new Set(keys);
24+
}
25+
26+
/** Canonical key. The only place this format is defined. */
27+
static key(
28+
connectionId: string,
29+
checkId: string,
30+
resourceId: string,
31+
): string {
32+
return `${connectionId}::${checkId}::${resourceId}`;
33+
}
34+
35+
get size(): number {
36+
return this.keys.size;
37+
}
38+
39+
has(connectionId: string, checkId: string, resourceId: string): boolean {
40+
return this.keys.has(
41+
ActiveExceptionSet.key(connectionId, checkId, resourceId),
42+
);
43+
}
44+
}
45+
46+
/**
47+
* Load active finding exceptions for an org as an {@link ActiveExceptionSet}.
48+
*
49+
* Fail-safe: on any DB error this returns an EMPTY set (suppress nothing) so a
50+
* lookup failure can never hide a real finding — we would rather show a finding
51+
* the customer excepted than silently pass a genuine one.
52+
*/
53+
export async function loadActiveExceptionSet(
54+
organizationId: string,
55+
): Promise<ActiveExceptionSet> {
56+
try {
57+
const active = await db.findingException.findMany({
58+
where: {
59+
organizationId,
60+
revokedAt: null,
61+
OR: [{ expiresAt: null }, { expiresAt: { gt: new Date() } }],
62+
},
63+
select: { connectionId: true, checkId: true, resourceId: true },
64+
});
65+
return new ActiveExceptionSet(
66+
active.map((e) =>
67+
ActiveExceptionSet.key(e.connectionId, e.checkId, e.resourceId),
68+
),
69+
);
70+
} catch {
71+
return new ActiveExceptionSet([]);
72+
}
73+
}

0 commit comments

Comments
 (0)