Skip to content

Commit c0bee9a

Browse files
tofikwesttofikwest
authored
Merge pull request #3389 from trycompai/main
[comp] Production Deploy
2 parents 2e2a345 + d52f0b6 commit c0bee9a

10 files changed

Lines changed: 407 additions & 113 deletions

File tree

‎apps/api/src/policies/dto/create-policy.dto.spec.ts‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
import { ValidationPipe } from '@nestjs/common';
2+
import { CreatePolicyDto } from './create-policy.dto';
3+
import { UpdatePolicyDto } from './update-policy.dto';
4+
5+
/**
6+
* Regression test for the MCP/public-API policy content serialization bug.
7+
*
8+
* The global ValidationPipe in main.ts runs with `transform: true` and
9+
* `transformOptions: { enableImplicitConversion: true }`. class-transformer
10+
* then coerces each TipTap node object toward the reflected `Array` design-type
11+
* of `content: unknown[]`, turning `[{...}, {...}]` into `[[], []]` — silently
12+
* blanking the policy body on create/update. These tests exercise the exact
13+
* pipe configuration and assert the structured content survives intact.
14+
*/
15+
const pipe = new ValidationPipe({
16+
whitelist: true,
17+
forbidNonWhitelisted: true,
18+
transform: true,
19+
transformOptions: { enableImplicitConversion: true },
20+
});
21+
22+
const TIPTAP_NODES = [
23+
{
24+
type: 'heading',
25+
attrs: { level: 2, textAlign: null },
26+
content: [{ type: 'text', text: 'Purpose' }],
27+
},
28+
{
29+
type: 'paragraph',
30+
attrs: { textAlign: null },
31+
content: [
32+
{
33+
type: 'text',
34+
text: 'Verify workforce integrity and grant the right access.',
35+
marks: [{ type: 'bold' }],
36+
},
37+
],
38+
},
39+
];
40+
41+
describe('Policy DTO content serialization (ValidationPipe)', () => {
42+
it('preserves structured TipTap content on CreatePolicyDto', async () => {
43+
const result = await pipe.transform(
44+
{ name: 'Access Control Policy', content: TIPTAP_NODES },
45+
{ type: 'body', metatype: CreatePolicyDto },
46+
);
47+
48+
expect(result.content).toEqual(TIPTAP_NODES);
49+
// Guard against the exact regression: nodes must not collapse to `[]`.
50+
expect(result.content[0]).toMatchObject({ type: 'heading' });
51+
expect(result.content).not.toEqual([[], []]);
52+
});
53+
54+
it('preserves structured TipTap content on UpdatePolicyDto', async () => {
55+
const result = await pipe.transform(
56+
{ content: TIPTAP_NODES },
57+
{ type: 'body', metatype: UpdatePolicyDto },
58+
);
59+
60+
expect(result.content).toEqual(TIPTAP_NODES);
61+
expect(result.content[1]).toMatchObject({ type: 'paragraph' });
62+
expect(result.content).not.toEqual([[], []]);
63+
});
64+
65+
it('leaves content untouched when omitted on update', async () => {
66+
const result = await pipe.transform(
67+
{ name: 'Renamed only' },
68+
{ type: 'body', metatype: UpdatePolicyDto },
69+
);
70+
71+
expect(result.content).toBeUndefined();
72+
});
73+
});

‎apps/api/src/policies/dto/create-policy.dto.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,12 @@ export class CreatePolicyDto {
9090
items: { type: 'object', additionalProperties: true },
9191
})
9292
@IsArray()
93-
@Transform(({ value }) => value)
93+
// Return the raw source value. Under the global ValidationPipe's implicit
94+
// conversion, class-transformer coerces each TipTap node toward the reflected
95+
// Array design-type of `content`, mangling `[{...}, {...}]` into `[[], []]`.
96+
// The transform runs after that coercion, so `value` is already mangled —
97+
// `obj.content` is the untouched original. Do not revert this to `value`.
98+
@Transform(({ obj }) => obj.content)
9499
content: unknown[];
95100

96101
@ApiProperty({

‎apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceAgentDevicesList.test.tsx‎

Lines changed: 45 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -248,63 +248,95 @@ describe('DeviceAgentDevicesList — integration-imported devices', () => {
248248
expect(screen.getByTitle('Offline')).toBeInTheDocument();
249249
});
250250

251-
it('shows the SOURCE-reported verdict and named checks when the provider reports them', () => {
251+
it('shows Yes only when all four canonical checks pass (extras are display-only)', () => {
252252
render(
253253
<DeviceAgentDevicesList
254254
devices={[
255255
makeIntegrationDevice({
256256
integrationProvider: { slug: 'intune', name: 'Microsoft Intune' },
257257
sourceCompliance: {
258-
isCompliant: true,
259258
checks: [
260259
{ id: 'disk_encryption', label: 'Disk Encryption', passed: true },
260+
{ id: 'antivirus', label: 'Antivirus', passed: true },
261+
{ id: 'password_policy', label: 'Password Policy', passed: true },
262+
{ id: 'screen_lock', label: 'Screen Lock', passed: true },
263+
// Non-canonical: shown as a badge but never affects the verdict.
261264
{ id: 'firewall', label: 'Firewall', passed: false },
262265
],
263266
},
264267
}),
265268
]}
266269
/>,
267270
);
268-
// Verdict shown instead of "Not tracked", with provider attribution.
269271
expect(screen.getByText('Yes')).toBeInTheDocument();
270-
expect(screen.queryByText('Not tracked')).not.toBeInTheDocument();
271-
expect(
272-
screen.getByRole('button', { name: /Where does this compliance status come from\?/i }),
273-
).toBeInTheDocument();
274-
// Provider-named check badges, pass and fail.
272+
expect(screen.queryByText(/Unverified/)).not.toBeInTheDocument();
273+
// Provider-named check badges, incl. the informational extra.
275274
expect(screen.getByText('Disk Encryption')).toBeInTheDocument();
276275
expect(screen.getByText('Firewall')).toBeInTheDocument();
277276
});
278277

279-
it('shows "No" when the source reports non-compliant', () => {
278+
it('shows "No" when a canonical check fails, even if the vendor claims compliant (showcase bug)', () => {
280279
render(
281280
<DeviceAgentDevicesList
282281
devices={[
283282
makeIntegrationDevice({
284-
sourceCompliance: { isCompliant: false },
283+
sourceCompliance: {
284+
// Intune with no policies configured calls everything compliant —
285+
// CompAI's framework standard (the four checks) must overrule it.
286+
isCompliant: true,
287+
checks: [
288+
{ id: 'disk_encryption', label: 'Disk Encryption', passed: false },
289+
],
290+
},
285291
}),
286292
]}
287293
/>,
288294
);
289295
expect(screen.getByText('No')).toBeInTheDocument();
290-
expect(screen.queryByText('Not tracked')).not.toBeInTheDocument();
296+
expect(screen.queryByText('Yes')).not.toBeInTheDocument();
297+
expect(screen.queryByText(/Unverified/)).not.toBeInTheDocument();
298+
});
299+
300+
it('shows "Unverified (n/4)" when only some canonical checks are reported and none fail', () => {
301+
render(
302+
<DeviceAgentDevicesList
303+
devices={[
304+
makeIntegrationDevice({
305+
sourceCompliance: {
306+
checks: [
307+
{ id: 'disk_encryption', label: 'Disk Encryption', passed: true },
308+
{ id: 'antivirus', label: 'Antivirus', passed: true },
309+
],
310+
},
311+
}),
312+
]}
313+
/>,
314+
);
315+
expect(screen.getByText('Unverified (2/4)')).toBeInTheDocument();
316+
expect(
317+
screen.getByRole('button', { name: /Why is compliance unverified\?/i }),
318+
).toBeInTheDocument();
319+
expect(screen.queryByText('Yes')).not.toBeInTheDocument();
320+
expect(screen.queryByText('No')).not.toBeInTheDocument();
291321
});
292322

293-
it('keeps "Not tracked" when the source reports checks but no verdict', () => {
323+
it('keeps "Not tracked" when the source reports no canonical checks at all', () => {
294324
render(
295325
<DeviceAgentDevicesList
296326
devices={[
297327
makeIntegrationDevice({
298328
sourceCompliance: {
329+
// A vendor verdict alone is NOT evidence against our standard.
330+
isCompliant: true,
299331
checks: [{ id: 'os_up_to_date', label: 'OS up to date', passed: true }],
300332
},
301333
}),
302334
]}
303335
/>,
304336
);
305-
// Checks render, but with no overall verdict the compliance column stays honest.
306337
expect(screen.getByText('OS up to date')).toBeInTheDocument();
307338
expect(screen.getByText('Not tracked')).toBeInTheDocument();
339+
expect(screen.queryByText('Yes')).not.toBeInTheDocument();
308340
});
309341

310342
it('renders a source filter only when more than one source is present', () => {

‎apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceComplianceChart.test.tsx‎

Lines changed: 38 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -220,11 +220,11 @@ describe('DeviceComplianceChart', () => {
220220
expect(screen.getByText('Compliant (1)')).toBeInTheDocument();
221221
});
222222

223-
it('does not show a Not Tracked legend entry when every device has a verdict', () => {
223+
it('does not show an Unverified legend entry when every device has a verdict', () => {
224224
render(
225225
<DeviceComplianceChart fleetDevices={[]} agentDevices={[makeAgentDevice()]} />,
226226
);
227-
expect(screen.queryByText(/Not Tracked/)).not.toBeInTheDocument();
227+
expect(screen.queryByText(/Unverified/)).not.toBeInTheDocument();
228228
});
229229
});
230230

@@ -244,32 +244,49 @@ function makeIntegrationDevice(
244244
}
245245

246246
describe('DeviceComplianceChart — integration-imported devices', () => {
247-
it('counts an imported device with a source verdict of compliant as Compliant', () => {
247+
it('counts an imported device with all four canonical checks passing as Compliant', () => {
248248
render(
249249
<DeviceComplianceChart
250250
fleetDevices={[]}
251251
agentDevices={[
252252
makeAgentDevice({ isCompliant: false, complianceStatus: 'non_compliant' }),
253-
makeIntegrationDevice({ sourceCompliance: { isCompliant: true } }),
253+
makeIntegrationDevice({
254+
sourceCompliance: {
255+
checks: [
256+
{ id: 'disk_encryption', label: 'Disk Encryption', passed: true },
257+
{ id: 'antivirus', label: 'Antivirus', passed: true },
258+
{ id: 'password_policy', label: 'Password Policy', passed: true },
259+
{ id: 'screen_lock', label: 'Screen Lock', passed: true },
260+
],
261+
},
262+
}),
254263
]}
255264
/>,
256265
);
257266
expect(screen.getByText('Compliant (1)')).toBeInTheDocument();
258267
expect(screen.getByText('Non-Compliant (1)')).toBeInTheDocument();
259-
expect(screen.queryByText(/Not Tracked/)).not.toBeInTheDocument();
268+
expect(screen.queryByText(/Unverified/)).not.toBeInTheDocument();
260269
});
261270

262-
it('counts an imported device with a source verdict of non-compliant as Non-Compliant', () => {
271+
it('counts an imported device with a failed canonical check as Non-Compliant (vendor verdict ignored)', () => {
263272
render(
264273
<DeviceComplianceChart
265274
fleetDevices={[]}
266-
agentDevices={[makeIntegrationDevice({ sourceCompliance: { isCompliant: false } })]}
275+
agentDevices={[
276+
makeIntegrationDevice({
277+
sourceCompliance: {
278+
// Vendor says compliant — CompAI's canonical standard overrules.
279+
isCompliant: true,
280+
checks: [{ id: 'disk_encryption', label: 'Disk Encryption', passed: false }],
281+
},
282+
}),
283+
]}
267284
/>,
268285
);
269286
expect(screen.getByText('Non-Compliant (1)')).toBeInTheDocument();
270287
});
271288

272-
it('puts an imported device with no source verdict into a Not Tracked segment', () => {
289+
it('puts an imported device with partial/no canonical checks into an Unverified segment', () => {
273290
render(
274291
<DeviceComplianceChart
275292
fleetDevices={[]}
@@ -281,14 +298,25 @@ describe('DeviceComplianceChart — integration-imported devices', () => {
281298
);
282299
expect(screen.getByText('Compliant (1)')).toBeInTheDocument();
283300
expect(screen.getByText('Non-Compliant (0)')).toBeInTheDocument();
284-
expect(screen.getByText('Not Tracked (1)')).toBeInTheDocument();
301+
expect(screen.getByText('Unverified (1)')).toBeInTheDocument();
285302
});
286303

287304
it('renders the chart (not the empty state) for an org with ONLY imported devices', () => {
288305
render(
289306
<DeviceComplianceChart
290307
fleetDevices={[]}
291-
agentDevices={[makeIntegrationDevice({ sourceCompliance: { isCompliant: true } })]}
308+
agentDevices={[
309+
makeIntegrationDevice({
310+
sourceCompliance: {
311+
checks: [
312+
{ id: 'disk_encryption', label: 'Disk Encryption', passed: true },
313+
{ id: 'antivirus', label: 'Antivirus', passed: true },
314+
{ id: 'password_policy', label: 'Password Policy', passed: true },
315+
{ id: 'screen_lock', label: 'Screen Lock', passed: true },
316+
],
317+
},
318+
}),
319+
]}
292320
/>,
293321
);
294322
expect(screen.queryByText(/No device data available/)).not.toBeInTheDocument();

‎apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceComplianceChart.tsx‎

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
import * as React from 'react';
1111
import { Cell, Label, Pie, PieChart } from 'recharts';
1212
import type { DeviceWithChecks, Host } from '../types';
13+
import { computeSourceComplianceVerdict } from '../lib/device-source';
1314

1415
interface DeviceComplianceChartProps {
1516
fleetDevices: Host[];
@@ -23,14 +24,15 @@ interface DeviceComplianceChartProps {
2324
const CHART_COLORS = {
2425
compliant: 'var(--primary)', // brand green
2526
nonCompliant: 'var(--destructive)', // red
26-
notTracked: 'var(--muted-foreground)', // gray — no compliance data available
27+
unverified: 'var(--muted-foreground)', // gray — canonical checks not (fully) reported
2728
};
2829

2930
export function DeviceComplianceChart({ fleetDevices, agentDevices }: DeviceComplianceChartProps) {
3031
// ALL devices count — the chart total must match the table so the page never
31-
// looks self-contradictory. Imported devices use the SOURCE's verdict when
32-
// it reports one; imported devices with no verdict land in a gray
33-
// "Not tracked" segment rather than being fabricated into a verdict.
32+
// looks self-contradictory. Imported devices are judged by CompAI's OWN
33+
// standard (the four canonical checks, computed from source-reported data);
34+
// partially/un-reported devices land in a gray "Unverified" segment rather
35+
// than being fabricated into a verdict.
3436
const devices = [...(agentDevices ?? []), ...(fleetDevices ?? [])];
3537

3638
const { pieDisplayData, legendDisplayData } = React.useMemo(() => {
@@ -39,15 +41,18 @@ export function DeviceComplianceChart({ fleetDevices, agentDevices }: DeviceComp
3941
}
4042
let compliantCount = 0;
4143
let nonCompliantCount = 0;
42-
let notTrackedCount = 0;
44+
let unverifiedCount = 0;
4345

4446
for (const device of agentDevices ?? []) {
4547
if (device.source === 'integration') {
46-
// Source-reported verdict (e.g. Intune complianceState), if any.
47-
const verdict = device.sourceCompliance?.isCompliant;
48-
if (verdict === true) compliantCount++;
49-
else if (verdict === false) nonCompliantCount++;
50-
else notTrackedCount++;
48+
// CompAI's verdict computed from the canonical source-reported checks
49+
// (vendor's own verdict is informational-only and never counted).
50+
const verdict = computeSourceComplianceVerdict(device);
51+
if (verdict?.kind === 'compliant') compliantCount++;
52+
else if (verdict?.kind === 'non_compliant') nonCompliantCount++;
53+
// Both partial ('unverified') and zero-data ('not_tracked') devices
54+
// land in the gray segment: neither has a defensible verdict.
55+
else unverifiedCount++;
5156
continue;
5257
}
5358
// Device-agent devices. Stale devices (no check-in for >= 7 days)
@@ -81,17 +86,17 @@ export function DeviceComplianceChart({ fleetDevices, agentDevices }: DeviceComp
8186
fill: CHART_COLORS.nonCompliant,
8287
},
8388
{
84-
name: 'Not Tracked',
85-
value: notTrackedCount,
86-
fill: CHART_COLORS.notTracked,
89+
name: 'Unverified',
90+
value: unverifiedCount,
91+
fill: CHART_COLORS.unverified,
8792
},
8893
];
8994
return {
9095
pieDisplayData: allItems.filter((item) => item.value > 0),
91-
// "Not Tracked" only appears in the legend when it exists — orgs with no
92-
// verdict-less imported devices keep the familiar two-entry legend.
96+
// "Unverified" only appears in the legend when it exists — orgs with no
97+
// partially-verified imported devices keep the familiar two-entry legend.
9398
legendDisplayData: allItems.filter(
94-
(item) => item.name !== 'Not Tracked' || item.value > 0,
99+
(item) => item.name !== 'Unverified' || item.value > 0,
95100
),
96101
};
97102
}, [agentDevices, fleetDevices]);

0 commit comments

Comments
 (0)