diff --git a/apps/api/src/frameworks/frameworks-people-score.helper.ts b/apps/api/src/frameworks/frameworks-people-score.helper.ts index ac0b116717..7f0a0940a6 100644 --- a/apps/api/src/frameworks/frameworks-people-score.helper.ts +++ b/apps/api/src/frameworks/frameworks-people-score.helper.ts @@ -158,6 +158,10 @@ async function getMembersWithInstalledDevices({ where: { organizationId, memberId: { in: memberIds }, + // Integration-imported devices are inventory records, not proof that + // the member installed the device agent — exclude them so they don't + // falsely satisfy the device-agent step in the people score. + source: { not: 'integration' }, }, select: { memberId: true }, distinct: ['memberId'], diff --git a/apps/api/src/integration-platform/controllers/internal-integration-debug.controller.ts b/apps/api/src/integration-platform/controllers/internal-integration-debug.controller.ts index c9ddb7ad6d..43c48dd47e 100644 --- a/apps/api/src/integration-platform/controllers/internal-integration-debug.controller.ts +++ b/apps/api/src/integration-platform/controllers/internal-integration-debug.controller.ts @@ -157,6 +157,23 @@ export class InternalIntegrationDebugController { }); } + /** + * Persist the REAL result for a check (success/failed — never held) — called by + * the self-heal agent when it verdicts a held check as a genuine fail (a + * customer-side error or a real compliance finding) so the customer sees the red. + */ + @Post('connections/:connectionId/reveal') + async revealConnectionCheck( + @Param('connectionId') connectionId: string, + @Body() body: RerunCheckBody, + ) { + return this.debugService.revealAndPersistCheck({ + connectionId, + checkId: body.checkId, + taskId: body.taskId, + }); + } + /** * Read recently captured OAuth callback errors (recorded by the frontend on a * failed connect). Use this to diagnose "the integration won't connect" for 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 c3458f0ece..19acb78c37 100644 --- a/apps/api/src/integration-platform/controllers/task-integrations.controller.ts +++ b/apps/api/src/integration-platform/controllers/task-integrations.controller.ts @@ -43,9 +43,7 @@ import { countEffectiveFailures, decideTaskStatus, decideRunStatus, - splitFailuresByDisposition, - failureSignalsFromEvidence, - type ClassifiableFailure, + type FailingFinding, } from '../utils/task-check-evaluation'; import { capEvidence, @@ -66,9 +64,9 @@ interface ConnectionCheckOutcome { status: 'success' | 'failed' | 'error'; findings: number; passing: number; - /** This account's failing findings (+ redacted error signals) for exception - * filtering and self-heal classification. */ - failures: ClassifiableFailure[]; + /** This account's failing findings (identity only) for exception filtering. + * comp does not classify — the self-heal agent decides our-bug vs real fail. */ + failures: FailingFinding[]; } interface TaskIntegrationCheck { @@ -411,7 +409,10 @@ export class TaskIntegrationsController { let lastCheckRunId: string | undefined; // Failing findings across all accounts (keyed like an exception) so task // status can exclude explicitly-excepted ones below. - const failingFindings: ClassifiableFailure[] = []; + const failingFindings: FailingFinding[] = []; + // Checks HELD ('inconclusive') across accounts this run — including error-only + // runs with no findings — so a held check keeps the task pending, not 'done'. + let heldRunCount = 0; // Sequential so each per-account run commits as it completes — a slow or // failing account still leaves the earlier accounts' results persisted. @@ -432,6 +433,9 @@ export class TaskIntegrationsController { totalFindings += outcome.findings; totalPassing += outcome.passing; if (outcome.status === 'error') hasExecutionError = true; + // For dynamic, any non-success account run was held (pending) — count it so + // an error-only account (no findings) still keeps the task pending. + if (isDynamic && outcome.status !== 'success') heldRunCount++; failingFindings.push(...outcome.failures); lastCheckRunId = outcome.checkRunId; } @@ -449,29 +453,23 @@ export class TaskIntegrationsController { // rule, via the shared helpers). Any real (non-excepted) finding → failed; // else any passing result → done; else leave unchanged. const exceptions = await loadActiveExceptionSet(organizationId); - // For DYNAMIC integrations, hold our-side/transient failures as inconclusive - // (same rule as the scheduled path): they must not fail the task. Static/AWS - // behavior is unchanged. Safe degradation: a finding with no readable error - // signal classifies as a real failure, exactly as today. - const statusFailures = isDynamic - ? splitFailuresByDisposition(failingFindings).effective - : failingFindings; - const heldCount = failingFindings.length - statusFailures.length; + // For DYNAMIC integrations EVERY failure is held (pending) — comp never + // classifies; the self-heal agent decides our-bug vs real fail. So none fail + // the task here. Static/AWS behavior is unchanged (no holding). + const statusFailures = isDynamic ? [] : failingFindings; + // heldCount = checks HELD this run (incl. error-only, no-findings) so any held + // check keeps the task pending instead of slipping to 'done'. + const heldCount = heldRunCount; if (heldCount > 0) { this.logger.log( - `Held ${heldCount} our-side/transient finding(s) as inconclusive for task ${taskId} (manual run; not shown as failed)`, + `Held ${heldCount} check(s) as inconclusive (pending) for task ${taskId} (manual run) — not failed, not done`, ); } - const effectiveFailures = countEffectiveFailures( - statusFailures, - exceptions, - ); - // Held findings are indeterminate — exclude them from the finding count so an - // all-held run doesn't flip the task to "done" either. + const effectiveFailures = countEffectiveFailures(statusFailures, exceptions); const newStatus = decideTaskStatus( effectiveFailures, totalPassing, - totalFindings - heldCount, + totalFindings, heldCount, ); @@ -674,16 +672,13 @@ export class TaskIntegrationsController { connectionId, checkId: checkDef.id, resourceId: f.resourceId, - // Redacted error signals for self-heal classification (dynamic only). - ...failureSignalsFromEvidence(f.evidence, checkResult.status), })); - // Per-account run status (shared rule): a dynamic run that failed only for - // our-side/transient reasons is held as 'inconclusive' (customer never sees - // it; the agent fixes it). Static integrations keep success/failed. + // Per-account run status (shared rule): a DYNAMIC run that didn't succeed is + // held as 'inconclusive' (pending, hidden) and handed to the self-heal agent + // — comp never classifies. Static integrations keep success/failed. const runStatus = decideRunStatus({ resultStatus: checkResult.status, - failures, isDynamic, }); diff --git a/apps/api/src/integration-platform/services/check-failure-classifier.spec.ts b/apps/api/src/integration-platform/services/check-failure-classifier.spec.ts deleted file mode 100644 index 43915e4a9c..0000000000 --- a/apps/api/src/integration-platform/services/check-failure-classifier.spec.ts +++ /dev/null @@ -1,80 +0,0 @@ -import { classifyCheckFailure } from './check-failure-classifier'; - -describe('classifyCheckFailure', () => { - it('treats a runtime exception as our-side', () => { - const r = classifyCheckFailure({ threw: true, errorText: 'x is not a function' }); - expect(r.class).toBe('our_side'); - expect(r.customerActionable).toBe(false); - }); - - it('treats a failure with no execution signal as a real compliance finding', () => { - const r = classifyCheckFailure({}); - expect(r.class).toBe('compliance'); - }); - - it.each([500, 502, 503, 408, 429])('treats HTTP %i as transient', (status) => { - expect(classifyCheckFailure({ httpStatus: status }).class).toBe('transient'); - }); - - it.each([ - 'request timed out', - 'fetch failed', - 'ECONNRESET', - 'service unavailable', - ])('treats network text "%s" as transient', (errorText) => { - expect(classifyCheckFailure({ errorText }).class).toBe('transient'); - }); - - it.each([401, 403])('treats HTTP %i as customer-side + actionable', (status) => { - const r = classifyCheckFailure({ httpStatus: status }); - expect(r.class).toBe('customer_side'); - expect(r.customerActionable).toBe(true); - }); - - it.each([ - 'Invalid API key', - 'token has expired', - 'your key was revoked', - 'organization is not entitled for api access', - 'please upgrade your plan', - ])('treats creds/plan text "%s" as customer-side', (errorText) => { - expect(classifyCheckFailure({ errorText }).class).toBe('customer_side'); - }); - - it.each([ - 'not allowed to perform actions outside the project this key is scoped to', - 'org_id is required', - 'not allowed for organization API keys', - 'this endpoint is deprecated', - 'res.text is not a function', - ])('treats our-bug text "%s" as our-side', (errorText) => { - expect(classifyCheckFailure({ errorText }).class).toBe('our_side'); - }); - - it.each([404, 400, 405, 422])('treats HTTP %i as our-side', (status) => { - expect(classifyCheckFailure({ httpStatus: status }).class).toBe('our_side'); - }); - - it('never blames the customer for an ambiguous execution failure (defaults our-side)', () => { - const r = classifyCheckFailure({ errorText: 'something weird happened' }); - expect(r.class).toBe('our_side'); - expect(r.customerActionable).toBe(false); - }); - - it('treats a customer-looking error as our-side when the whole fleet is failing', () => { - const r = classifyCheckFailure({ - httpStatus: 403, - fleet: { passing: 0, failing: 5 }, - }); - expect(r.class).toBe('our_side'); - }); - - it('keeps a 401 as customer-side when other connections are passing', () => { - const r = classifyCheckFailure({ - httpStatus: 401, - fleet: { passing: 9, failing: 1 }, - }); - expect(r.class).toBe('customer_side'); - expect(r.customerActionable).toBe(true); - }); -}); diff --git a/apps/api/src/integration-platform/services/check-failure-classifier.ts b/apps/api/src/integration-platform/services/check-failure-classifier.ts deleted file mode 100644 index 3dd86b5fd2..0000000000 --- a/apps/api/src/integration-platform/services/check-failure-classifier.ts +++ /dev/null @@ -1,138 +0,0 @@ -/** - * Classifies WHY a dynamic-integration check failed, so the self-heal layer can - * decide what the customer sees: - * - * - 'compliance' → the check evaluated fine and returned a real finding - * (customer is genuinely non-compliant). SHOW as a fail. - * - 'transient' → timeout / 5xx / rate-limit. RETRY, show nothing. - * - 'customer_side' → their credentials / plan / config. Show "action needed". - * - 'our_side' → our bug / a vendor change we don't handle yet. HIDE the - * red, hand to the agent / open a ticket. - * - * Guiding rule (the product guarantee): NEVER blame the customer without positive - * proof. Any ambiguous EXECUTION failure defaults to 'our_side', and a customer- - * looking error that is actually failing fleet-wide is treated as 'our_side'. - * - * Pure function — no I/O, no secrets. `errorText` MUST be pre-redacted by the caller. - */ - -export type FailureClass = - | 'compliance' - | 'transient' - | 'customer_side' - | 'our_side'; - -export interface ClassifyInput { - /** HTTP status the failure carried, if any (e.g. 401, 404, 503). */ - httpStatus?: number | null; - /** Error message / response-body snippet. MUST already be redacted of secrets. */ - errorText?: string | null; - /** True if the runtime itself threw (a code error), not a vendor response. */ - threw?: boolean; - /** - * Optional fleet signal for the SAME check across the provider's other active - * connections. Used only to avoid blaming a single customer for a fleet-wide - * (i.e. our-side) breakage. - */ - fleet?: { passing: number; failing: number } | null; -} - -export interface ClassifyResult { - class: FailureClass; - reason: string; - /** True only for a confirmed customer_side result (safe to ask them to act). */ - customerActionable: boolean; -} - -const TRANSIENT_RE = - /(timeout|timed out|etimedout|econnreset|econnrefused|socket hang up|network error|fetch failed|eai_again|temporarily unavailable|service unavailable|try again)/i; - -const CUSTOMER_RE = - /(invalid api key|invalid token|invalid credentials|unauthorized|authentication failed|access denied|token (has )?expired|expired token|revoked|not entitled|api access is not|no access to the api|upgrade your plan|plan does ?n.?t include|plan does not include|insufficient (permission|scope|privileges))/i; - -const OUR_SIDE_RE = - /(scoped to|org_id is required|not allowed for organization|deprecated|no longer supported|is not a function|cannot read propert|undefined is not|unexpected (token|response|status|end of)|method not allowed|missing_header|malformed)/i; - -function make( - cls: FailureClass, - reason: string, - customerActionable = false, -): ClassifyResult { - return { class: cls, reason, customerActionable }; -} - -export function classifyCheckFailure(input: ClassifyInput): ClassifyResult { - const status = input.httpStatus ?? null; - const text = input.errorText ?? ''; - const fleet = input.fleet ?? null; - // "Failing for many, passing for none" = almost certainly our bug, not one - // customer's problem. - const fleetWide = !!fleet && fleet.failing >= 2 && fleet.passing === 0; - - // 1. A runtime exception in our own code is unambiguously our-side. - if (input.threw) { - return make('our_side', 'Runtime error while executing the check.'); - } - - const hasHttpError = status != null && status >= 400; - const hasErrorText = text.trim().length > 0; - - // 2. No execution-failure signal at all → the check evaluated and returned a - // genuine finding. Show it (this is the product working correctly). - if (!hasHttpError && !hasErrorText) { - return make('compliance', 'Check evaluated and returned a finding.'); - } - - // 3. Transient — retry, show nothing. - if ( - status === 408 || - status === 425 || - status === 429 || - (status != null && status >= 500) || - TRANSIENT_RE.test(text) - ) { - return make('transient', 'Transient network/availability error.'); - } - - // 4. Customer-side — requires a positive signal (hard 401/403 or an explicit - // creds/plan message). Even then, if the whole fleet is failing it is ours. - const customerSignal = status === 401 || status === 403 || CUSTOMER_RE.test(text); - if (customerSignal) { - if (fleetWide) { - return make( - 'our_side', - 'Auth/plan-looking error, but the check is failing fleet-wide → treat as our-side.', - ); - } - return make( - 'customer_side', - 'Authentication/authorization/plan issue specific to this connection.', - true, - ); - } - - // 5. Known our-side shapes: 4xx on endpoints we should handle, deprecated - // endpoints, unhandled response shapes, etc. - if ( - status === 404 || - status === 400 || - status === 405 || - status === 422 || - OUR_SIDE_RE.test(text) - ) { - return make( - 'our_side', - 'Endpoint/response our check does not handle (our bug or a vendor change).', - ); - } - - // 6. Fleet tiebreaker + conservative default: never blame the customer without - // proof — any remaining ambiguous execution failure is treated as our-side. - if (fleetWide) { - return make('our_side', 'Ambiguous error failing fleet-wide → our-side.'); - } - return make( - 'our_side', - 'Ambiguous execution failure → defaulting to our-side (never blame the customer without proof).', - ); -} diff --git a/apps/api/src/integration-platform/services/internal-integration-debug.service.ts b/apps/api/src/integration-platform/services/internal-integration-debug.service.ts index 37efd8156b..ee72cf982e 100644 --- a/apps/api/src/integration-platform/services/internal-integration-debug.service.ts +++ b/apps/api/src/integration-platform/services/internal-integration-debug.service.ts @@ -6,10 +6,7 @@ import { type RunAllChecksResult, } from './connection-check-runner.service'; import { CheckRunRepository } from '../repositories/check-run.repository'; -import { - decideRunStatus, - failureSignalsFromEvidence, -} from '../utils/task-check-evaluation'; +import { decideRunStatus } from '../utils/task-check-evaluation'; /** * Read-only/diagnostic toolkit for dynamic integrations, used by internal @@ -454,6 +451,29 @@ export class InternalIntegrationDebugService { return { runs, total: runs.length }; } + /** + * A persisted run may only be associated with a task in the SAME org as the + * connection. The agent always passes the held run's own taskId, but validate + * it so a wrong/forged internal call can't write into another tenant's task + * history or flip its status. No-op when no taskId is supplied. + */ + private async assertTaskBelongsToOrg( + taskId: string | null | undefined, + organizationId: string, + connectionId: string, + ): Promise { + if (!taskId) return; + const task = await db.task.findUnique({ + where: { id: taskId }, + select: { organizationId: true }, + }); + if (!task || task.organizationId !== organizationId) { + throw new NotFoundException( + `Task ${taskId} does not belong to connection ${connectionId}'s organization`, + ); + } + } + /** * Re-run ONE check for ONE connection AND PERSIST a fresh run. Used by the * self-heal agent right after it applies a fix: re-running every affected @@ -477,6 +497,7 @@ export class InternalIntegrationDebugService { if (!connection) { throw new NotFoundException(`Connection ${connectionId} not found`); } + await this.assertTaskBelongsToOrg(taskId, connection.organizationId, connectionId); // Execute on the real runtime in-process (runChecks never persists). const result = await this.runner.runChecks({ @@ -496,15 +517,8 @@ export class InternalIntegrationDebugService { select: { id: true }, })); - const failures = checkResult.result.findings.map((f) => ({ - connectionId, - checkId, - resourceId: f.resourceId, - ...failureSignalsFromEvidence(f.evidence, checkResult.status), - })); const status = decideRunStatus({ resultStatus: checkResult.status, - failures, isDynamic, }); @@ -564,4 +578,115 @@ export class InternalIntegrationDebugService { ); return { status }; } + + /** + * Re-run ONE check and PERSIST the REAL result — used when the self-heal agent + * verdicts a held check as a GENUINE fail (the customer's creds/config are + * wrong, OR a real compliance finding). Unlike rerunAndPersistCheck (which + * applies the dynamic hold rule and may re-hold as 'inconclusive'), this writes + * the TRUE status: 'success' if it now passes, 'failed' (with the real findings + * shown, failedCount > 0) otherwise — so the customer sees the red instead of a + * silent "pending". It never holds and never disables. + */ + async revealAndPersistCheck(params: { + connectionId: string; + checkId: string; + taskId?: string | null; + }): Promise<{ status: 'success' | 'failed' }> { + const { connectionId, checkId, taskId } = params; + const connection = await db.integrationConnection.findUnique({ + where: { id: connectionId }, + select: { organizationId: true }, + }); + if (!connection) { + throw new NotFoundException(`Connection ${connectionId} not found`); + } + await this.assertTaskBelongsToOrg(taskId, connection.organizationId, connectionId); + + const result = await this.runner.runChecks({ + connectionId, + organizationId: connection.organizationId, + checkId, + }); + const checkResult = result.results[0]; + if (!checkResult) { + throw new NotFoundException( + `Check ${checkId} produced no result for connection ${connectionId}`, + ); + } + + // The REAL status — never held. A genuine fail shows red to the customer. + const status: 'success' | 'failed' = + checkResult.status === 'success' ? 'success' : 'failed'; + + const checkRun = await this.checkRunRepository.create({ + connectionId, + taskId: taskId ?? undefined, + checkId, + checkName: checkResult.checkName, + }); + const resultsToStore = [ + ...checkResult.result.passingResults.map((r) => ({ + checkRunId: checkRun.id, + passed: true, + resourceType: r.resourceType, + resourceId: r.resourceId, + title: r.title, + description: r.description, + evidence: r.evidence + ? (JSON.parse(JSON.stringify(r.evidence)) as Prisma.InputJsonValue) + : undefined, + })), + ...checkResult.result.findings.map((f) => ({ + checkRunId: checkRun.id, + passed: false, + resourceType: f.resourceType, + resourceId: f.resourceId, + title: f.title, + description: f.description, + severity: f.severity, + remediation: f.remediation, + evidence: f.evidence as Prisma.InputJsonValue, + })), + ]; + if (resultsToStore.length > 0) { + await this.checkRunRepository.addResults(resultsToStore); + } + await this.checkRunRepository.complete(checkRun.id, { + status, + durationMs: checkResult.durationMs, + totalChecked: + checkResult.result.summary?.totalChecked ?? + checkResult.result.passingResults.length + + checkResult.result.findings.length, + passedCount: checkResult.result.passingResults.length, + // REAL fail → show the findings (NOT hidden like a held 'inconclusive' run). + failedCount: checkResult.result.findings.length, + errorMessage: checkResult.error, + logs: JSON.parse( + JSON.stringify(checkResult.result.logs), + ) as Prisma.InputJsonValue, + }); + + // Sync the TASK: a genuine fail must show on the task itself, not just in run + // history — otherwise the task stays green while the check failed, the exact + // false-success the reveal flow exists to prevent. Mirrors the run paths' + // decideTaskStatus → 'failed' write. A reveal that now PASSES does NOT force + // 'done' here: the task spans other checks and is recomputed on the next + // scheduled run (forcing 'done' could hide another still-failing/held check). + if (taskId && status === 'failed') { + // Only flip ACTIVE workflow statuses → failed. Never resurrect a human-set + // not_relevant (dismissed) or in_review (under review) task — or rewrite an + // already-failed one — from a self-heal reveal. + await db.task.updateMany({ + where: { id: taskId, status: { in: ['todo', 'in_progress', 'done'] } }, + data: { status: 'failed' }, + }); + } + + this.logger.log( + `Self-heal reveal: connection ${connectionId}, check ${checkId} -> ${status} (real result shown)`, + ); + return { status }; + } } diff --git a/apps/api/src/integration-platform/utils/task-check-evaluation.runstatus.spec.ts b/apps/api/src/integration-platform/utils/task-check-evaluation.runstatus.spec.ts index ee88b850db..3ba479e557 100644 --- a/apps/api/src/integration-platform/utils/task-check-evaluation.runstatus.spec.ts +++ b/apps/api/src/integration-platform/utils/task-check-evaluation.runstatus.spec.ts @@ -1,105 +1,32 @@ -import { - decideRunStatus, - type ClassifiableFailure, -} from './task-check-evaluation'; - -const fail = ( - over: Partial = {}, -): ClassifiableFailure => ({ - connectionId: 'c', - checkId: 'k', - resourceId: 'r', - ...over, -}); +import { decideRunStatus } from './task-check-evaluation'; describe('decideRunStatus', () => { - // Static / AWS / GCP / Azure — isDynamic=false → NEVER held; identical to the - // historical mapping (error → failed, else raw status). + // Static / AWS / GCP / Azure — isDynamic=false → never held; historical mapping + // (error → failed, else raw status). describe('non-dynamic (never held)', () => { it('success → success', () => { - expect( - decideRunStatus({ - resultStatus: 'success', - failures: [], - isDynamic: false, - }), - ).toBe('success'); + expect(decideRunStatus({ resultStatus: 'success', isDynamic: false })).toBe('success'); }); - it('failed (even an our-side-looking 404) → failed', () => { - expect( - decideRunStatus({ - resultStatus: 'failed', - failures: [fail({ httpStatus: 404 })], - isDynamic: false, - }), - ).toBe('failed'); + it('failed → failed', () => { + expect(decideRunStatus({ resultStatus: 'failed', isDynamic: false })).toBe('failed'); }); it('execution error → failed', () => { - expect( - decideRunStatus({ - resultStatus: 'error', - failures: [fail({ threw: true })], - isDynamic: false, - }), - ).toBe('failed'); + expect(decideRunStatus({ resultStatus: 'error', isDynamic: false })).toBe('failed'); }); }); - // Dynamic — our-side/transient held as inconclusive; real failures shown. - describe('dynamic', () => { + // Dynamic — comp does NO classification: EVERY non-success is held as + // 'inconclusive' ("pending") and handed to the self-heal agent, the only + // decider of our-bug (fix) vs real fail (show). No error-code logic at all. + describe('dynamic (everything non-success → pending)', () => { it('success → success', () => { - expect( - decideRunStatus({ - resultStatus: 'success', - failures: [], - isDynamic: true, - }), - ).toBe('success'); - }); - it('execution error → inconclusive (held)', () => { - expect( - decideRunStatus({ - resultStatus: 'error', - failures: [], - isDynamic: true, - }), - ).toBe('inconclusive'); - }); - it('all failures our-side (404) → inconclusive (held)', () => { - expect( - decideRunStatus({ - resultStatus: 'failed', - failures: [fail({ httpStatus: 404 })], - isDynamic: true, - }), - ).toBe('inconclusive'); - }); - it('genuine compliance finding (no error signal) → failed (shown)', () => { - expect( - decideRunStatus({ - resultStatus: 'failed', - failures: [fail()], - isDynamic: true, - }), - ).toBe('failed'); + expect(decideRunStatus({ resultStatus: 'success', isDynamic: true })).toBe('success'); }); - it('customer-side (401) → failed (shown — action needed)', () => { - expect( - decideRunStatus({ - resultStatus: 'failed', - failures: [fail({ httpStatus: 401 })], - isDynamic: true, - }), - ).toBe('failed'); + it('failed (a finding, a customer error, anything) → inconclusive (pending)', () => { + expect(decideRunStatus({ resultStatus: 'failed', isDynamic: true })).toBe('inconclusive'); }); - it('mixed held + real → failed (any effective failure surfaces)', () => { - expect( - decideRunStatus({ - resultStatus: 'failed', - failures: [fail({ httpStatus: 404 }), fail({ resourceId: 'r2' })], - isDynamic: true, - }), - ).toBe('failed'); + it('execution error → inconclusive (pending)', () => { + expect(decideRunStatus({ resultStatus: 'error', isDynamic: true })).toBe('inconclusive'); }); }); }); diff --git a/apps/api/src/integration-platform/utils/task-check-evaluation.signals.spec.ts b/apps/api/src/integration-platform/utils/task-check-evaluation.signals.spec.ts deleted file mode 100644 index c829c45840..0000000000 --- a/apps/api/src/integration-platform/utils/task-check-evaluation.signals.spec.ts +++ /dev/null @@ -1,82 +0,0 @@ -import { failureSignalsFromEvidence } from './task-check-evaluation'; - -describe('failureSignalsFromEvidence', () => { - it('parses http status from evidence.error like "http_404"', () => { - const s = failureSignalsFromEvidence({ - error: 'http_404', - message: 'not found', - }); - expect(s.httpStatus).toBe(404); - expect(s.errorText).toContain('not found'); - expect(s.threw).toBe(false); - }); - - it('falls back to numeric evidence.status', () => { - expect(failureSignalsFromEvidence({ status: 503 }).httpStatus).toBe(503); - }); - - it('parses spaced/colon HTTP forms so 401/403 are not missed', () => { - // These would otherwise default to our_side (held) instead of customer_side. - expect( - failureSignalsFromEvidence({ message: 'HTTP 401 Unauthorized' }) - .httpStatus, - ).toBe(401); - expect( - failureSignalsFromEvidence({ error: 'HTTP: 403 Forbidden' }).httpStatus, - ).toBe(403); - expect( - failureSignalsFromEvidence({ message: 'HTTP-429 rate limited' }) - .httpStatus, - ).toBe(429); - // A URL must NOT be mistaken for a status. - expect( - failureSignalsFromEvidence({ - message: 'fetch https://x.com/v404/p failed', - }).httpStatus, - ).toBeNull(); - }); - - it('does NOT let an empty message mask the error text', () => { - // `msgStr ?? errStr` kept '' (not null) — an empty message hid the error. - const s = failureSignalsFromEvidence({ - error: 'http_500 upstream boom', - message: '', - }); - expect(s.errorText).toContain('boom'); - expect(s.httpStatus).toBe(500); - }); - - it('parses a STRING evidence.status (e.g. "401", "403 Forbidden")', () => { - expect(failureSignalsFromEvidence({ status: '401' }).httpStatus).toBe(401); - expect( - failureSignalsFromEvidence({ status: '403 Forbidden' }).httpStatus, - ).toBe(403); - expect( - failureSignalsFromEvidence({ status: 'weird' }).httpStatus, - ).toBeNull(); - // Code must be at the START — don't grab an unrelated 3-digit run. - expect( - failureSignalsFromEvidence({ status: 'build 200 ok' }).httpStatus, - ).toBeNull(); - }); - - it('marks threw when the result status is "error"', () => { - expect(failureSignalsFromEvidence({}, 'error').threw).toBe(true); - }); - - it('prefers message over error for errorText and redacts it', () => { - const s = failureSignalsFromEvidence({ - error: 'http_401', - message: 'auth failed for Bearer sk-supersecrettokenvalue123456', - }); - expect(s.errorText).not.toContain('sk-supersecrettokenvalue123456'); - expect(s.errorText).toContain('auth failed'); - }); - - it('returns empty signals for an evidence-less finding (treated as compliance)', () => { - const s = failureSignalsFromEvidence(undefined); - expect(s.httpStatus).toBeNull(); - expect(s.errorText).toBeNull(); - expect(s.threw).toBe(false); - }); -}); diff --git a/apps/api/src/integration-platform/utils/task-check-evaluation.split.spec.ts b/apps/api/src/integration-platform/utils/task-check-evaluation.split.spec.ts deleted file mode 100644 index 2b13d89068..0000000000 --- a/apps/api/src/integration-platform/utils/task-check-evaluation.split.spec.ts +++ /dev/null @@ -1,62 +0,0 @@ -import { - splitFailuresByDisposition, - ClassifiableFailure, -} from './task-check-evaluation'; - -const f = (over: Partial): ClassifiableFailure => ({ - connectionId: 'icn_1', - checkId: 'chk', - resourceId: 'res', - ...over, -}); - -describe('splitFailuresByDisposition', () => { - it('keeps a genuine compliance finding (no error signal) as effective', () => { - const { effective, held } = splitFailuresByDisposition([f({})]); - expect(effective).toHaveLength(1); - expect(held).toHaveLength(0); - }); - - it('holds our-side failures (404 / unhandled endpoint)', () => { - const { effective, held } = splitFailuresByDisposition([ - f({ httpStatus: 404 }), - f({ errorText: 'not allowed ... scoped to' }), - ]); - expect(effective).toHaveLength(0); - expect(held).toHaveLength(2); - }); - - it('holds transient failures (5xx / timeout)', () => { - const { held } = splitFailuresByDisposition([ - f({ httpStatus: 503 }), - f({ errorText: 'request timed out' }), - ]); - expect(held).toHaveLength(2); - }); - - it('shows proven customer-side failures (401)', () => { - const { effective, held } = splitFailuresByDisposition([f({ httpStatus: 401 })]); - expect(effective).toHaveLength(1); - expect(held).toHaveLength(0); - }); - - it('splits a mixed batch correctly', () => { - const { effective, held } = splitFailuresByDisposition([ - f({}), // compliance -> effective - f({ httpStatus: 401 }), // customer -> effective - f({ httpStatus: 404 }), // our-side -> held - f({ errorText: 'fetch failed' }), // transient -> held - ]); - expect(effective).toHaveLength(2); - expect(held).toHaveLength(2); - }); - - it('holds a customer-looking failure when the whole fleet is failing', () => { - const { effective, held } = splitFailuresByDisposition( - [f({ httpStatus: 403 })], - { passing: 0, failing: 6 }, - ); - expect(effective).toHaveLength(0); - expect(held).toHaveLength(1); - }); -}); diff --git a/apps/api/src/integration-platform/utils/task-check-evaluation.ts b/apps/api/src/integration-platform/utils/task-check-evaluation.ts index b593418ef6..0cc504b54f 100644 --- a/apps/api/src/integration-platform/utils/task-check-evaluation.ts +++ b/apps/api/src/integration-platform/utils/task-check-evaluation.ts @@ -1,6 +1,4 @@ import { ActiveExceptionSet } from '../../cloud-security/finding-exceptions'; -import { classifyCheckFailure } from '../services/check-failure-classifier'; -import { redactSecrets } from './redact-secrets'; /** A failing finding, identified the same way an exception is keyed. */ export interface FailingFinding { @@ -19,9 +17,7 @@ export function countEffectiveFailures( exceptions: ActiveExceptionSet, ): number { if (exceptions.size === 0) return failing.length; - return failing.filter( - (f) => !exceptions.has(f.connectionId, f.checkId, f.resourceId), - ).length; + return failing.filter((f) => !exceptions.has(f.connectionId, f.checkId, f.resourceId)).length; } /** @@ -30,13 +26,14 @@ export function countEffectiveFailures( * - 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) + * - else leave unchanged (nothing was evaluated, e.g. an all-errored run). * - * `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. + * For DYNAMIC integrations, EVERY failure is held as 'inconclusive' (pending) and + * counted in `heldCount`, never in `effectiveFailures` — so a held check never + * fails the task (the self-heal agent is the only decider of our-bug vs real + * fail). When the agent reveals a genuine fail, that run persists 'failed' and a + * later evaluation fails the task; when it fixes, the run passes and the task goes + * done. heldCount is always 0 for non-dynamic (static/AWS/GCP/Azure). */ export function decideTaskStatus( effectiveFailures: number, @@ -45,148 +42,26 @@ export function decideTaskStatus( heldCount = 0, ): 'failed' | 'done' | null { if (effectiveFailures > 0) return 'failed'; - // Held (our-side/transient) failures are UNRESOLVED — the self-heal agent is - // still fixing them. Never declare the task done while any check is held, even - // if other checks passed; that would hide an unresolved failure behind a green - // task. Leave it unchanged (indeterminate) until the held checks actually pass - // — the agent's re-run then produces a clean pass and a later run goes done. - // heldCount is always 0 for non-dynamic (static/AWS/GCP/Azure), so unchanged. + // Any held (pending) check is UNRESOLVED — never declare the task done while one + // is pending; that would hide an unresolved failure behind a green task. if (heldCount > 0) return null; if (totalPassing > 0 || totalFindings > 0) return 'done'; return null; } -/** A failing finding plus the signals needed to classify WHY it failed. */ -export interface ClassifiableFailure extends FailingFinding { - /** HTTP status the failure carried, if any. */ - httpStatus?: number | null; - /** Error text from the finding's evidence. MUST be pre-redacted of secrets. */ - errorText?: string | null; - /** True if the runtime threw rather than the vendor returning an error. */ - threw?: boolean; -} - -export interface FailureDisposition { - /** Genuine failures — fail the task + show (compliance findings + proven customer-side). */ - effective: ClassifiableFailure[]; - /** Held failures — our-side bug / transient. Task NOT failed; surfaced as inconclusive. */ - held: ClassifiableFailure[]; -} - /** - * Split failing findings into those that should fail the task (real compliance - * findings + proven customer-side issues) vs those to HOLD as inconclusive - * (our-side bug / transient), so a customer never sees a red for our problem. - * - * For DYNAMIC integrations only — the caller gates this; static/AWS checks keep - * their existing behavior. The classifier is conservative (never blames the - * customer without proof), so ambiguous failures are held, not shown. - * - * `fleet` (optional) is the same check's pass/fail counts across the provider's - * other active connections — a fleet-wide failure is held even if it looks - * customer-like. - */ -export function splitFailuresByDisposition( - failing: ClassifiableFailure[], - fleet?: { passing: number; failing: number } | null, -): FailureDisposition { - const effective: ClassifiableFailure[] = []; - const held: ClassifiableFailure[] = []; - for (const f of failing) { - const { class: cls } = classifyCheckFailure({ - httpStatus: f.httpStatus, - errorText: f.errorText, - threw: f.threw, - fleet, - }); - if (cls === 'our_side' || cls === 'transient') { - held.push(f); - } else { - // 'compliance' + 'customer_side' → show the customer. - effective.push(f); - } - } - return { effective, held }; -} - -/** - * Decide the per-run status stored on an IntegrationCheckRun. Canonical rule, - * shared by ALL run paths (scheduled, manual, and the agent re-run) so a held - * run is classified identically everywhere: - * - base: an execution 'error' → 'failed'; otherwise the raw success/failed. - * - DYNAMIC only: an execution error, or a run whose failures are ALL held - * (our-side/transient → no effective failures), becomes 'inconclusive' — - * the self-heal queue, hidden from the customer. Static/AWS keep the base. - * - * `failures` carries the per-finding signals (from {@link failureSignalsFromEvidence}). + * Decide the per-run status stored on an IntegrationCheckRun. Shared by ALL run + * paths (scheduled, manual, agent re-run). comp does NO classification: for a + * DYNAMIC integration every non-success — a finding, a customer/transport error, + * or a thrown execution error — is held as 'inconclusive' ("pending", hidden from + * the customer) and handed to the self-heal agent, the ONLY decider of our-bug vs + * real fail. Static/AWS/GCP/Azure (isDynamic = false) keep the plain mapping. */ export function decideRunStatus(params: { resultStatus: string; - failures: ClassifiableFailure[]; isDynamic: boolean; - fleet?: { passing: number; failing: number } | null; }): 'success' | 'failed' | 'inconclusive' { - const { resultStatus, failures, isDynamic, fleet } = params; - let runStatus: 'success' | 'failed' | 'inconclusive' = - resultStatus === 'success' ? 'success' : 'failed'; - if (isDynamic) { - if (resultStatus === 'error') { - runStatus = 'inconclusive'; - } else if ( - failures.length > 0 && - splitFailuresByDisposition(failures, fleet).effective.length === 0 - ) { - runStatus = 'inconclusive'; - } - } - return runStatus; -} - -/** - * Extract classification signals from a failing finding's evidence. `errorText` - * is REDACTED of secrets/PII before it leaves this function. - * - * Defensive about the heterogeneous evidence shapes checks emit: if no signal is - * found, returns empty signals → the failure is treated as a genuine compliance - * finding (today's behavior), so a check is never wrongly held. `resultStatus` - * of 'error' means the check threw → an execution failure. - */ -export function failureSignalsFromEvidence( - evidence: Record | null | undefined, - resultStatus?: string, -): { httpStatus: number | null; errorText: string | null; threw: boolean } { - const ev = evidence ?? {}; - const errStr = typeof ev.error === 'string' ? ev.error : null; - const msgStr = typeof ev.message === 'string' ? ev.message : null; - - let httpStatus: number | null = null; - // Search BOTH error and message — the status often lives in the human message - // ('HTTP 401 Unauthorized'), not the error code. Tolerate space/colon/_/- - // separators so 'http_404', 'HTTP 401', 'HTTP: 403', 'HTTP-429' all parse; - // otherwise a customer-actionable 401/403 would be missed and default to - // our_side (held) instead of customer_side (shown). Won't match a URL. - const m = `${errStr ?? ''} ${msgStr ?? ''}`.match(/\bhttp[\s:_-]*(\d{3})\b/i); - if (m) httpStatus = Number(m[1]); - // evidence.status may be a number (404) OR a status-code string ('404', - // '401 Unauthorized'). For strings the code must be at the START (anchored) so - // we don't grab an unrelated 3-digit run from arbitrary text (e.g. 'build 200'). - if (httpStatus == null && ev.status != null) { - const n = - typeof ev.status === 'number' - ? ev.status - : Number( - String(ev.status) - .trim() - .match(/^(\d{3})\b/)?.[1], - ); - if (Number.isFinite(n) && n >= 100 && n < 600) httpStatus = n; - } - - // Use the message ONLY when it has content; an empty-string message must not - // mask the error text (?? keeps '' because it's not null/undefined). - const rawText = msgStr && msgStr.trim() ? msgStr : (errStr ?? ''); - const errorText = rawText ? redactSecrets(rawText) : null; - const threw = resultStatus === 'error'; - - return { httpStatus, errorText, threw }; + const { resultStatus, isDynamic } = params; + if (resultStatus === 'success') return 'success'; + return isDynamic ? 'inconclusive' : 'failed'; } diff --git a/apps/api/src/isms/documents/objectives.spec.ts b/apps/api/src/isms/documents/objectives.spec.ts index 9babd6ec37..74405b818d 100644 --- a/apps/api/src/isms/documents/objectives.spec.ts +++ b/apps/api/src/isms/documents/objectives.spec.ts @@ -131,7 +131,7 @@ describe('buildObjectivesSections', () => { 'Operate controls and pass the audit', 'Audit outcome', 'Annual', - 'on_track', + 'On track', ], ]); }); @@ -155,7 +155,33 @@ describe('buildObjectivesSections', () => { }; const sections = buildObjectivesSections(input); expect(sections[0].table?.rows).toEqual([ - ['Reduce phishing click rate', '< 3%', '—', '—', '—', 'on_track'], + ['Reduce phishing click rate', '< 3%', '—', '—', '—', 'On track'], ]); }); + + it('renders human-readable status labels, not raw enum values', () => { + const makeInput = (status: string): DocumentExportInput => ({ + contextIssues: [], + interestedParties: [], + requirements: [], + objectives: [ + { + objective: 'o', + target: null, + cadence: null, + status, + plan: null, + measurementMethod: null, + }, + ], + narrative: null, + }); + const statusCell = (status: string) => + buildObjectivesSections(makeInput(status))[0].table?.rows[0].at(-1); + + expect(statusCell('not_started')).toBe('Not started'); + expect(statusCell('on_track')).toBe('On track'); + expect(statusCell('at_risk')).toBe('At risk'); + expect(statusCell('met')).toBe('Met'); + }); }); diff --git a/apps/api/src/isms/documents/objectives.ts b/apps/api/src/isms/documents/objectives.ts index 5e8939eda2..6d8f8fe6f3 100644 --- a/apps/api/src/isms/documents/objectives.ts +++ b/apps/api/src/isms/documents/objectives.ts @@ -89,6 +89,25 @@ export function deriveObjectives(data: IsmsPlatformData): DerivedObjective[] { return rows.map((row, index) => ({ ...row, position: index })); } +/** + * Human-readable labels for the objective status enum, mirroring the UI + * (apps/app/.../components/objectives-status.ts) so the export and the screen + * read identically. Unknown values fall back to a humanized form of the raw key. + */ +const STATUS_LABELS: Record = { + not_started: 'Not started', + on_track: 'On track', + at_risk: 'At risk', + met: 'Met', +}; + +function formatObjectiveStatus(status: string): string { + const known = STATUS_LABELS[status]; + if (known) return known; + const cleaned = status.replace(/[_-]+/g, ' ').trim(); + return cleaned ? cleaned.charAt(0).toUpperCase() + cleaned.slice(1) : status; +} + export function buildObjectivesSections( input: DocumentExportInput, ): IsmsExportSection[] { @@ -111,7 +130,7 @@ export function buildObjectivesSections( objective.plan ?? '—', objective.measurementMethod ?? '—', objective.cadence ?? '—', - objective.status, + formatObjectiveStatus(objective.status), ]), }, }, diff --git a/apps/api/src/isms/documents/requirements.spec.ts b/apps/api/src/isms/documents/requirements.spec.ts index 7fa1f89a61..1ac105db0d 100644 --- a/apps/api/src/isms/documents/requirements.spec.ts +++ b/apps/api/src/isms/documents/requirements.spec.ts @@ -32,7 +32,10 @@ describe('deriveRequirements', () => { const rows = deriveRequirements({ parties, data }); expect(rows).toHaveLength(2); expect(rows[0].interestedPartyId).toBe('ip_1'); - expect(rows[0].derivedFrom).toBe('party:ip_1'); + // Provenance carries the party NAME (rendered verbatim in the source badge), + // never the raw record id — the link itself lives in interestedPartyId. + expect(rows[0].derivedFrom).toBe('party:Customers'); + expect(rows[0].derivedFrom).not.toContain('ip_1'); expect(rows.every((r) => r.source === 'derived')).toBe(true); expect(rows.every((r) => r.requirement.length > 0)).toBe(true); expect(rows.every((r) => r.treatment.length > 0)).toBe(true); diff --git a/apps/api/src/isms/documents/requirements.ts b/apps/api/src/isms/documents/requirements.ts index dce2ee33d4..3e23c415d1 100644 --- a/apps/api/src/isms/documents/requirements.ts +++ b/apps/api/src/isms/documents/requirements.ts @@ -100,9 +100,9 @@ export function deriveRequirements({ requirement: mapped.requirement, treatment: mapped.treatment, source: 'derived', - derivedFrom: party.interestedPartyId - ? `party:${party.interestedPartyId}` - : `party:${party.name}`, + // Provenance carries the human party name (shown verbatim in the source + // badge); the actual link is the separate interestedPartyId field below. + derivedFrom: `party:${party.name}`, position: index, interestedPartyId: party.interestedPartyId, }; diff --git a/apps/api/src/isms/registers/register-registry.spec.ts b/apps/api/src/isms/registers/register-registry.spec.ts index ce00c0ff46..9455b3931a 100644 --- a/apps/api/src/isms/registers/register-registry.spec.ts +++ b/apps/api/src/isms/registers/register-registry.spec.ts @@ -158,6 +158,32 @@ describe('createRegisterRegistry', () => { }); }); + it('update accepts a null interestedPartyId (clearing the link)', async () => { + await registry.requirements.update({ + rowId: 'req_1', + organizationId: 'org_1', + data: { interestedPartyId: null, requirement: 'r2' }, + }); + expect(requirements.update).toHaveBeenCalledWith({ + requirementId: 'req_1', + organizationId: 'org_1', + dto: { interestedPartyId: null, requirement: 'r2' }, + }); + }); + + it('create accepts a null interestedPartyId', async () => { + await registry.requirements.create({ + documentId: 'doc_1', + organizationId: 'org_1', + data: { partyName: 'C', requirement: 'r', treatment: 't', interestedPartyId: null }, + }); + expect(requirements.create).toHaveBeenCalledWith({ + documentId: 'doc_1', + organizationId: 'org_1', + dto: { partyName: 'C', requirement: 'r', treatment: 't', interestedPartyId: null }, + }); + }); + it('remove dispatches with requirementId', async () => { await registry.requirements.remove({ rowId: 'req_1', diff --git a/apps/api/src/isms/registers/register-registry.ts b/apps/api/src/isms/registers/register-registry.ts index 7af3da2eb2..d331ef4786 100644 --- a/apps/api/src/isms/registers/register-registry.ts +++ b/apps/api/src/isms/registers/register-registry.ts @@ -45,14 +45,18 @@ const schemas = { position, }), requirementCreate: z.object({ - interestedPartyId: z.string().optional(), + // Nullish: the UI sends null for an unlinked requirement, and the service + // treats null/empty as "no linked party" (see IsmsRequirementService). + interestedPartyId: z.string().nullish(), partyName: z.string(), requirement: z.string(), treatment: z.string(), position, }), requirementUpdate: z.object({ - interestedPartyId: z.string().optional(), + // Nullish, not optional: undefined = leave the link as-is, null = clear it. + // The service relies on this three-state contract. + interestedPartyId: z.string().nullish(), partyName: z.string().optional(), requirement: z.string().optional(), treatment: z.string().optional(), diff --git a/apps/api/src/security-penetration-tests/pentest-credits.controller.spec.ts b/apps/api/src/security-penetration-tests/pentest-credits.controller.spec.ts new file mode 100644 index 0000000000..dffae98fcb --- /dev/null +++ b/apps/api/src/security-penetration-tests/pentest-credits.controller.spec.ts @@ -0,0 +1,69 @@ +// The controller's runtime import chain reaches `@db`, which instantiates +// a Prisma client at module load. These tests never touch the database. +jest.mock('@db', () => ({ db: {} })); + +jest.mock('../auth/hybrid-auth.guard', () => ({ + HybridAuthGuard: class { + canActivate() { + return true; + } + }, +})); + +// The real PermissionGuard transitively imports better-auth's ESM bundle, +// which Jest cannot parse. PERMISSIONS_KEY keeps its real value so +// @RequirePermission metadata stays intact for any metadata assertions. +jest.mock('../auth/permission.guard', () => ({ + PermissionGuard: class { + canActivate() { + return true; + } + }, + PERMISSIONS_KEY: 'required_permissions', +})); + +import { PentestCreditsController } from './pentest-credits.controller'; +import type { PentestCreditsService } from './pentest-credits.service'; + +describe('PentestCreditsController', () => { + const grantInitialTrialMock = jest.fn(); + const getStatusMock = jest.fn(); + + const serviceMock: jest.Mocked = { + grantInitialTrial: grantInitialTrialMock, + getStatus: getStatusMock, + } as unknown as jest.Mocked; + + const controller = new PentestCreditsController(serviceMock); + + const status = { + balance: 1, + totalGranted: 1, + totalConsumed: 0, + lastGrantSource: 'trial', + }; + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('returns the credit status for the organization', async () => { + getStatusMock.mockResolvedValueOnce(status); + + const response = await controller.getStatus('org_123'); + + expect(getStatusMock).toHaveBeenCalledWith('org_123'); + expect(response).toEqual(status); + }); + + it('grants the initial credit then returns the resulting status', async () => { + grantInitialTrialMock.mockResolvedValueOnce(undefined); + getStatusMock.mockResolvedValueOnce(status); + + const response = await controller.grantInitial('org_123'); + + expect(grantInitialTrialMock).toHaveBeenCalledWith('org_123'); + expect(getStatusMock).toHaveBeenCalledWith('org_123'); + expect(response).toEqual(status); + }); +}); diff --git a/apps/api/src/security-penetration-tests/pentest-credits.controller.ts b/apps/api/src/security-penetration-tests/pentest-credits.controller.ts index 1067b1b9c0..f9981feb32 100644 --- a/apps/api/src/security-penetration-tests/pentest-credits.controller.ts +++ b/apps/api/src/security-penetration-tests/pentest-credits.controller.ts @@ -1,4 +1,4 @@ -import { Controller, Get, UseGuards } from '@nestjs/common'; +import { Controller, Get, Post, UseGuards } from '@nestjs/common'; import { ApiHeader, ApiOperation, @@ -41,4 +41,23 @@ export class PentestCreditsController { async getStatus(@OrganizationId() organizationId: string) { return this.credits.getStatus(organizationId); } + + /** + * Grant the org its single free pentest credit. Called once during org + * creation (server-side, owner session) so a new org can run a first + * penetration test without entering a card. Idempotent per org — calling + * it again is a safe no-op that returns the unchanged balance. + */ + @Post('grant-initial') + @RequirePermission('pentest', 'create') + @ApiOperation({ + summary: 'Grant the new-org free pentest credit', + description: + 'Idempotently grants the organization a single free pentest credit so the first penetration test can run without a paid plan. Returns the resulting credit status.', + }) + @ApiResponse({ status: 201, description: 'Credits status after grant' }) + async grantInitial(@OrganizationId() organizationId: string) { + await this.credits.grantInitialTrial(organizationId); + return this.credits.getStatus(organizationId); + } } diff --git a/apps/api/src/security-penetration-tests/pentest-credits.service.spec.ts b/apps/api/src/security-penetration-tests/pentest-credits.service.spec.ts index 63710af0f6..b94da03525 100644 --- a/apps/api/src/security-penetration-tests/pentest-credits.service.spec.ts +++ b/apps/api/src/security-penetration-tests/pentest-credits.service.spec.ts @@ -1,5 +1,6 @@ import { HttpException, HttpStatus } from '@nestjs/common'; import { db } from '@db'; +import type { BillingCreditsService } from '../billing/billing-credits.service'; import { PentestCreditsService } from './pentest-credits.service'; jest.mock('@db', () => ({ @@ -12,6 +13,9 @@ jest.mock('@db', () => ({ update: jest.fn(), updateMany: jest.fn(), }, + organization: { + findUnique: jest.fn(), + }, member: { findFirst: jest.fn(), }, @@ -28,6 +32,9 @@ type MockDb = { update: jest.Mock; updateMany: jest.Mock; }; + organization: { + findUnique: jest.Mock; + }; member: { findFirst: jest.Mock; }; @@ -103,6 +110,67 @@ describe('PentestCreditsService', () => { }); }); + describe('grantInitialTrial', () => { + it('grants one pentest credit to the billing-credits wallet with a stable per-org key', async () => { + mockedDb.organization.findUnique.mockResolvedValue({ + onboardingCompleted: false, + }); + const billingCredits = { grant: jest.fn().mockResolvedValue({}) }; + const svc = new PentestCreditsService( + billingCredits as unknown as BillingCreditsService, + ); + + await svc.grantInitialTrial('org_new'); + + expect(billingCredits.grant).toHaveBeenCalledWith( + expect.objectContaining({ + organizationId: 'org_new', + productKey: 'pentest', + quantity: 1, + source: 'trial', + idempotencyKey: 'initial-pentest-trial:org_new', + }), + ); + // Must NOT touch the legacy pentest_credits wallet — the run-creation + // fallback reads billing_credit_balances, so a legacy-table grant would + // be invisible to scan creation. + expect(mockedDb.pentestCredits.upsert).not.toHaveBeenCalled(); + }); + + it('does not grant to an already-onboarded (existing) org', async () => { + mockedDb.organization.findUnique.mockResolvedValue({ + onboardingCompleted: true, + }); + const billingCredits = { grant: jest.fn().mockResolvedValue({}) }; + const svc = new PentestCreditsService( + billingCredits as unknown as BillingCreditsService, + ); + + await svc.grantInitialTrial('org_existing'); + + expect(billingCredits.grant).not.toHaveBeenCalled(); + }); + + it('does not grant when the org does not exist', async () => { + mockedDb.organization.findUnique.mockResolvedValue(null); + const billingCredits = { grant: jest.fn().mockResolvedValue({}) }; + const svc = new PentestCreditsService( + billingCredits as unknown as BillingCreditsService, + ); + + await svc.grantInitialTrial('org_missing'); + + expect(billingCredits.grant).not.toHaveBeenCalled(); + }); + + it('is a no-op when no billing-credits backend is wired', async () => { + // `service` (from beforeEach) is built without a BillingCreditsService. + await expect(service.grantInitialTrial('org_x')).resolves.toBeUndefined(); + expect(mockedDb.organization.findUnique).not.toHaveBeenCalled(); + expect(mockedDb.pentestCredits.upsert).not.toHaveBeenCalled(); + }); + }); + describe('debitOrThrow', () => { it('atomically decrements when balance > 0 and returns new status', async () => { mockedDb.pentestCredits.updateMany.mockResolvedValue({ count: 1 }); diff --git a/apps/api/src/security-penetration-tests/pentest-credits.service.ts b/apps/api/src/security-penetration-tests/pentest-credits.service.ts index 4e89994752..ae84005b20 100644 --- a/apps/api/src/security-penetration-tests/pentest-credits.service.ts +++ b/apps/api/src/security-penetration-tests/pentest-credits.service.ts @@ -98,15 +98,70 @@ export class PentestCreditsService { }; } - // Initial trial credits are granted via Prisma's nested-create on - // `Organization` in the org-create server actions - // (apps/app/src/app/(app)/setup/actions/create-organization*.ts) so - // the credit row is inserted in the same transaction as the - // organization itself — no chance of an org existing without its - // trial credit. Existing orgs were backfilled by migration - // `20260427000000_pentest_credits`. There is intentionally no - // service-side `grantInitialTrial` method; if a manual re-grant is - // ever needed, it goes through the platform-admin grant flow. + /** + * Grant a brand-new org its single free pentest credit so the owner can + * run a first penetration test WITHOUT entering a card. Once that credit + * is spent, running another pentest requires explicitly choosing a paid + * plan — an active decision, not a surprise auto-charge. This replaces the + * earlier Stripe free-TRIAL flow (which collected a card up front and + * auto-converted to a charge, driving refund requests). + * + * Writes to the billing-credits wallet (`billing_credit_balances`) — the + * SAME store the run-creation fallback reads + * (BillingEntitlementsService.tryConsumeCreditFallback -> + * BillingCreditsService.tryConsumeForProduct). Granting to the legacy + * `pentest_credits` table would be invisible to scan creation. + * + * Idempotent per org via a stable idempotency key, so org-create retries + * (and the minimal action's reuse-existing-org branch) can call this more + * than once without ever granting a second credit. + * + * Eligibility: only granted while the org is still in the creation / + * onboarding phase (`onboardingCompleted = false`). An established org has + * `onboardingCompleted = true` (the flag only ever flips false -> true, set + * by complete-onboarding), so this prevents any `pentest:create` holder from + * minting a free credit for an existing org outside the new-org flow. + * + * No-op when no billing-credits backend is wired (the @Optional dep is + * absent, e.g. legacy unit tests) — there is no authoritative wallet to + * grant against in that case. + */ + async grantInitialTrial(organizationId: string): Promise { + if (!this.billingCredits) { + this.logger.warn( + `[Credits] grantInitialTrial skipped: billing credits unavailable org=${organizationId}`, + ); + return; + } + // Eligibility gate — only brand-new orgs (still onboarding) qualify. + const org = await db.organization.findUnique({ + where: { id: organizationId }, + select: { onboardingCompleted: true }, + }); + if (!org) { + this.logger.warn( + `[Credits] grantInitialTrial skipped: org not found org=${organizationId}`, + ); + return; + } + if (org.onboardingCompleted) { + this.logger.warn( + `[Credits] grantInitialTrial skipped: org not eligible (already onboarded) org=${organizationId}`, + ); + return; + } + await this.billingCredits.grant({ + organizationId, + productKey: 'pentest', + quantity: this.initialTrialAmount, + source: 'trial', + note: 'Free pentest credit for new organization', + idempotencyKey: `initial-pentest-trial:${organizationId}`, + }); + this.logger.log( + `[Credits] initial trial grant org=${organizationId} amount=+${this.initialTrialAmount} source=trial`, + ); + } /** * Add credits from any source. Used for v2 Stripe webhooks (subscription 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 f78ff5a176..4c841febb6 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 @@ -17,9 +17,7 @@ import { countEffectiveFailures, decideTaskStatus, decideRunStatus, - splitFailuresByDisposition, - failureSignalsFromEvidence, - type ClassifiableFailure, + type FailingFinding, } from '../../integration-platform/utils/task-check-evaluation'; /** @@ -225,8 +223,13 @@ export const runTaskIntegrationChecks = task({ let hasExecutionErrors = false; // Failing findings (keyed like an exception) so task status can exclude // explicitly-excepted ones below. Carries redacted error signals so the - // self-heal layer can classify our-side failures (dynamic integrations). - const failingFindings: ClassifiableFailure[] = []; + // identity only — comp never classifies; the self-heal agent reads the stored + // findings/evidence and decides our-bug vs real fail itself. + const failingFindings: FailingFinding[] = []; + // Count of checks HELD ('inconclusive') this run — including error-only runs + // that produced no findings — so a held/errored check keeps the task pending + // (not 'done') until the self-heal agent resolves it. + let heldRunCount = 0; // Run only the checks that apply to this task try { @@ -304,7 +307,6 @@ export const runTaskIntegrationChecks = task({ // never shows the customer a red), static/AWS → 'failed'. status: decideRunStatus({ resultStatus: 'error', - failures: [], isDynamic, }), startedAt: new Date(), @@ -319,6 +321,9 @@ export const runTaskIntegrationChecks = task({ logger.error( `Server-run failed for check ${checkId} on task ${taskId}: ${message}`, ); + // A dynamic transport error was just held as 'inconclusive' — count it + // so an error-only failure (no findings) still keeps the task pending. + if (isDynamic) heldRunCount++; continue; } @@ -329,15 +334,13 @@ export const runTaskIntegrationChecks = task({ // exception) so task status can exclude explicitly-excepted ones. totalFindings += checkResult.result.findings.length; totalPassing += checkResult.result.passingResults.length; - // Build this check's failing findings (with redacted signals) once — - // reused for the task-level decision and the per-check run status. + // Build this check's failing findings — identity only. comp does NOT + // classify; the self-heal agent reads the stored findings/evidence and + // decides our-bug vs real fail itself. const checkFailures = checkResult.result.findings.map((f) => ({ connectionId, checkId: checkResult.checkId, resourceId: f.resourceId, - // Redacted error signals so the self-heal layer can classify - // our-side/transient failures and hold them as inconclusive. - ...failureSignalsFromEvidence(f.evidence, checkResult.status), })); failingFindings.push(...checkFailures); if (checkResult.status === 'error') { @@ -350,9 +353,11 @@ export const runTaskIntegrationChecks = task({ // base success/failed mapping. const runStatus = decideRunStatus({ resultStatus: checkResult.status, - failures: checkFailures, isDynamic, }); + // Any held check (a finding, a customer error, or an execution error) + // keeps the task pending — count it so it can't slip to 'done'. + if (runStatus === 'inconclusive') heldRunCount++; // Store check run const checkRun = await db.integrationCheckRun.create({ @@ -448,27 +453,22 @@ export const runTaskIntegrationChecks = task({ // For DYNAMIC integrations, hold our-side/transient failures as // inconclusive: they must not fail the task or send a "task failed" email — // the self-heal layer investigates/fixes them. Static/AWS behavior is - // unchanged. Safe degradation: a finding with no readable error signal - // classifies as a real (compliance) failure, exactly as today. - const statusFailures = isDynamic - ? splitFailuresByDisposition(failingFindings).effective - : failingFindings; - const heldCount = failingFindings.length - statusFailures.length; + // DYNAMIC: every failure is held (pending) — comp never classifies, the + // agent decides. Static/AWS: unchanged (no holding). + const statusFailures = isDynamic ? [] : failingFindings; + // heldCount = checks HELD this run (incl. error-only runs with NO findings), + // so any held check keeps the task pending instead of slipping to 'done'. + const heldCount = heldRunCount; if (heldCount > 0) { logger.info( - `Held ${heldCount} our-side/transient finding(s) as inconclusive for task ${taskId} (not shown as failed)`, + `Held ${heldCount} check(s) as inconclusive (pending) for task ${taskId} — not failed, not done`, ); } - const effectiveFailures = countEffectiveFailures( - statusFailures, - exceptions, - ); - // Held findings are indeterminate (like an all-errored run) — they must not - // flip the task to "done" either, so exclude them from the finding count. + const effectiveFailures = countEffectiveFailures(statusFailures, exceptions); const newStatus = decideTaskStatus( effectiveFailures, totalPassing, - totalFindings - heldCount, + totalFindings, heldCount, ); diff --git a/apps/app/src/actions/organization/lib/grant-initial-pentest-credit.test.ts b/apps/app/src/actions/organization/lib/grant-initial-pentest-credit.test.ts new file mode 100644 index 0000000000..43687c2704 --- /dev/null +++ b/apps/app/src/actions/organization/lib/grant-initial-pentest-credit.test.ts @@ -0,0 +1,41 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { serverApi } from '@/lib/api-server'; +import { grantInitialPentestCredit } from './grant-initial-pentest-credit'; + +vi.mock('@/lib/api-server', () => ({ + serverApi: { post: vi.fn() }, +})); + +describe('grantInitialPentestCredit', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('POSTs to the grant-initial pentest credit endpoint', async () => { + vi.mocked(serverApi.post).mockResolvedValue({ + data: { balance: 1 }, + status: 201, + }); + + await grantInitialPentestCredit(); + + expect(serverApi.post).toHaveBeenCalledWith( + '/v1/pentest-credits/grant-initial', + ); + }); + + it('never throws when the API returns an error (non-fatal)', async () => { + const consoleError = vi + .spyOn(console, 'error') + .mockImplementation(() => {}); + vi.mocked(serverApi.post).mockResolvedValue({ + error: 'HTTP 500', + status: 500, + }); + + await expect(grantInitialPentestCredit()).resolves.toBeUndefined(); + expect(consoleError).toHaveBeenCalled(); + + consoleError.mockRestore(); + }); +}); diff --git a/apps/app/src/actions/organization/lib/grant-initial-pentest-credit.ts b/apps/app/src/actions/organization/lib/grant-initial-pentest-credit.ts new file mode 100644 index 0000000000..e8a6cf2560 --- /dev/null +++ b/apps/app/src/actions/organization/lib/grant-initial-pentest-credit.ts @@ -0,0 +1,28 @@ +import { serverApi } from '@/lib/api-server'; + +/** + * Grant a newly-created organization its single free pentest credit via the + * API so the owner can run a first penetration test without entering a card. + * Once that credit is spent, running another pentest is an explicit, opt-in + * purchase — which avoids the surprise auto-charges the Stripe free-trial flow + * was generating refund requests over. + * + * Non-fatal: org creation must never fail because of this call, so errors are + * logged and swallowed (mirroring the adjacent trust-portal bootstrap). + * Idempotent server-side (stable per-org key), so being called more than once + * for the same org — e.g. from the minimal action's reuse-existing-org branch + * or a retry — never grants a second credit. + * + * Relies on the caller having already set the new org as the active + * organization; the API resolves the org from the session, so no orgId is + * passed here. + */ +export async function grantInitialPentestCredit(): Promise { + const response = await serverApi.post('/v1/pentest-credits/grant-initial'); + if (response.error) { + console.error( + 'Non-critical: failed to grant initial pentest credit:', + response.error, + ); + } +} diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsDocumentShell.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsDocumentShell.tsx index f87101e1de..419474124c 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsDocumentShell.tsx +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsDocumentShell.tsx @@ -145,7 +145,7 @@ export function IsmsDocumentShell({ description={description} status={document?.status ?? null} isStale={isStale} - backHref={`/${organizationId}/documents`} + backHref={`/${organizationId}/documents?tab=iso-27001`} actions={ <> {canManage && ( diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/RequirementsClient.test.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/RequirementsClient.test.tsx index 0189616c61..123d855946 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/components/RequirementsClient.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/RequirementsClient.test.tsx @@ -143,6 +143,16 @@ describe('RequirementsClient', () => { expect(screen.getAllByText('Manual').length).toBeGreaterThan(0); }); + it('back link returns to the ISO 27001 documents tab', () => { + setMockPermissions(ADMIN_PERMISSIONS); + render(); + + expect(screen.getByRole('link', { name: 'ISMS' })).toHaveAttribute( + 'href', + '/org-1/documents?tab=iso-27001', + ); + }); + it('allows editing (shows mutating controls) for a user with evidence:update', () => { setMockPermissions(ADMIN_PERMISSIONS); render(); diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/RequirementsForm.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/RequirementsForm.tsx index 829a63955c..d107a3ad11 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/components/RequirementsForm.tsx +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/RequirementsForm.tsx @@ -48,31 +48,18 @@ function RequirementsFields({ return (
-
- - ( - - )} - /> - {errors.partyName?.message} - - - ( - - )} - /> - -
+ {/* No "Linked party ID" input: the party link is system-managed and only + set for rows derived from the Interested Parties Register. */} + + ( + + )} + /> + {errors.partyName?.message} +
{ ); expect(onSave).not.toHaveBeenCalled(); }); + + it('does not surface a raw "Linked party ID" input', () => { + render( + , + ); + + fireEvent.click(screen.getByLabelText('Edit requirement')); + + expect(screen.queryByLabelText('Requirement party ID')).not.toBeInTheDocument(); + expect(screen.queryByText('Linked party ID (optional)')).not.toBeInTheDocument(); + }); + + it('carries the system-managed party link through on save without exposing it', async () => { + const onSave = vi.fn().mockResolvedValue(undefined); + render( + , + ); + + fireEvent.click(screen.getByLabelText('Edit requirement')); + fireEvent.change(screen.getByLabelText('Requirement description'), { + target: { value: 'Updated requirement text' }, + }); + fireEvent.click(screen.getByText('Save')); + + await waitFor(() => expect(onSave).toHaveBeenCalled()); + expect(onSave).toHaveBeenCalledWith( + expect.objectContaining({ + interestedPartyId: 'isms_ip_abc123', + requirement: 'Updated requirement text', + }), + ); + }); }); diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/RequirementsRow.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/RequirementsRow.tsx index f7fcecc813..955ef55496 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/components/RequirementsRow.tsx +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/RequirementsRow.tsx @@ -116,33 +116,22 @@ export function RequirementsRow({ requirement, canEdit, onSave, onDelete }: Requ headerEnd={actions} > -
- - - ( - - )} - /> - {errors.partyName?.message} - - - + {/* interestedPartyId is a system-managed link (set when the row is + derived from an Interested Parties Register entry). It rides along + in the form's default values and is carried through on save, but is + never surfaced as a raw-id input. */} + + ( - + )} /> - -
+ {errors.partyName?.message} +
+ (
+ {backHref ? ISMS : null}

{`${clause} ${title}`}

{actions}
diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/shared/IsmsSourceBadge.test.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/shared/IsmsSourceBadge.test.tsx new file mode 100644 index 0000000000..32af7bc0c5 --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/shared/IsmsSourceBadge.test.tsx @@ -0,0 +1,42 @@ +import { render, screen } from '@testing-library/react'; +import { describe, expect, it, vi } from 'vitest'; +import { ismsDesignSystemMock } from '../__test-helpers__/dsMocks'; + +vi.mock('@trycompai/design-system', () => ismsDesignSystemMock()); + +import { IsmsSourceBadge } from './IsmsSourceBadge'; + +describe('IsmsSourceBadge', () => { + it('shows the party name for a party-derived row, never the raw record id', () => { + render(); + + expect(screen.getByText('Employees / workforce')).toBeInTheDocument(); + // The raw "party:" provenance prefix must never leak into the label. + expect(screen.queryByText(/^party:/i)).not.toBeInTheDocument(); + expect(screen.queryByText(/^Party /)).not.toBeInTheDocument(); + }); + + it('falls back to "Interested party" when the party name is blank', () => { + render(); + + expect(screen.getByText('Interested party')).toBeInTheDocument(); + }); + + it('shows the framework name for framework provenance', () => { + render(); + + expect(screen.getByText('ISO 27001')).toBeInTheDocument(); + }); + + it('maps known provenance keys to friendly labels', () => { + render(); + + expect(screen.getByText('Vendor register')).toBeInTheDocument(); + }); + + it('labels manual rows "Manual"', () => { + render(); + + expect(screen.getByText('Manual')).toBeInTheDocument(); + }); +}); diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/shared/IsmsSourceBadge.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/shared/IsmsSourceBadge.tsx index b17b6bf895..95fa3cc75b 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/components/shared/IsmsSourceBadge.tsx +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/shared/IsmsSourceBadge.tsx @@ -26,6 +26,12 @@ function humanizeProvenance(derivedFrom?: string | null): string { const name = derivedFrom.slice('framework:'.length).trim(); return name || 'Framework'; } + // Requirement rows are derived per interested party; the suffix is the party + // name, shown verbatim (never the raw record id). + if (derivedFrom.startsWith('party:')) { + const name = derivedFrom.slice('party:'.length).trim(); + return name || 'Interested party'; + } if (derivedFrom.startsWith('wizard:')) return 'Setup wizard'; const mapped = PROVENANCE_LABELS[derivedFrom]; if (mapped) return mapped; diff --git a/apps/app/src/app/(app)/[orgId]/people/[employeeId]/components/EmployeeDevice.tsx b/apps/app/src/app/(app)/[orgId]/people/[employeeId]/components/EmployeeDevice.tsx index 2e0bc5e7d0..1435c10fb0 100644 --- a/apps/app/src/app/(app)/[orgId]/people/[employeeId]/components/EmployeeDevice.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/[employeeId]/components/EmployeeDevice.tsx @@ -49,6 +49,27 @@ function staleTooltipCopy(daysSinceLastCheckIn: number | null): string { } function DeviceComplianceBadge({ device }: { device: DeviceWithChecks }) { + if (device.source === 'integration') { + const provider = device.integrationProvider?.name ?? 'an integration'; + return ( +
+ Not tracked + + + event.stopPropagation()} + > + + + + {`This device was imported from ${provider}. CompAI doesn't collect compliance checks for imported devices — install the CompAI agent to track its security posture.`} + + + +
+ ); + } if (device.complianceStatus === 'stale') { return (
@@ -115,8 +136,13 @@ export function EmployeeDevice({
{CHECK_FIELDS.map(({ key, dbKey, label }) => { + const isIntegration = memberDevice.source === 'integration'; const isFleetUnsupported = memberDevice.source === 'fleet' && key !== 'diskEncryptionEnabled'; + const isUntracked = isIntegration || isFleetUnsupported; + const untrackedCopy = isIntegration + ? 'Not collected for imported devices' + : 'Not tracked by Fleet'; const isStale = memberDevice.complianceStatus === 'stale'; const passed = memberDevice[key]; const details = memberDevice.checkDetails?.[dbKey]; @@ -124,19 +150,19 @@ export function EmployeeDevice({
{label} - {!isFleetUnsupported && !isStale && details?.message && ( + {!isUntracked && !isStale && details?.message && (

{details.message}

)} - {isFleetUnsupported && ( -

Not tracked by Fleet

+ {isUntracked && ( +

{untrackedCopy}

)} - {!isFleetUnsupported && !isStale && details?.exception && ( + {!isUntracked && !isStale && details?.exception && (

{details.exception}

)}
- {isFleetUnsupported ? ( + {isUntracked ? ( N/A ) : isStale ? ( @@ -153,7 +179,8 @@ export function EmployeeDevice({
{memberDevice.lastCheckIn && (

- Last check-in: {new Date(memberDevice.lastCheckIn).toLocaleString()} + {memberDevice.source === 'integration' ? 'Last synced' : 'Last check-in'}:{' '} + {new Date(memberDevice.lastCheckIn).toLocaleString()}

)} diff --git a/apps/app/src/app/(app)/[orgId]/people/[employeeId]/page.tsx b/apps/app/src/app/(app)/[orgId]/people/[employeeId]/page.tsx index 80960daa74..f43221e2dd 100644 --- a/apps/app/src/app/(app)/[orgId]/people/[employeeId]/page.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/[employeeId]/page.tsx @@ -293,7 +293,7 @@ const getMemberDevice = async ( memberId: string, organizationId: string, ): Promise => { - const device = await db.device.findFirst({ + const devices = await db.device.findMany({ where: { memberId, organizationId }, include: { member: { @@ -310,10 +310,41 @@ const getMemberDevice = async ( orderBy: { installedAt: 'desc' }, }); - if (!device) { + if (devices.length === 0) { return null; } + // An agent device carries real compliance data; an integration import does + // not. Prefer the richest source so the detail page never shows an imported + // device as a failing agent device. Order of richness: agent > fleet > integration. + const device = + devices.find((d) => d.source === 'agent') ?? + devices.find((d) => d.source === 'fleet') ?? + devices[0]; + + const source: DeviceWithChecks['source'] = + device.source === 'integration' + ? 'integration' + : device.source === 'fleet' + ? 'fleet' + : 'device_agent'; + + // Resolve the provider (name/slug) so an imported device shows its provenance + // instead of being mislabeled as an agent device. + let integrationProvider: DeviceWithChecks['integrationProvider']; + if (source === 'integration' && device.integrationConnectionId) { + const connection = await db.integrationConnection.findFirst({ + where: { id: device.integrationConnectionId, organizationId }, + select: { provider: { select: { slug: true, name: true } } }, + }); + if (connection?.provider) { + integrationProvider = { + slug: connection.provider.slug, + name: connection.provider.name, + }; + } + } + const complianceStatus = getDeviceComplianceStatus({ isCompliant: device.isCompliant, lastCheckIn: device.lastCheckIn, @@ -336,14 +367,18 @@ const getMemberDevice = async ( lastCheckIn: device.lastCheckIn?.toISOString() ?? null, agentVersion: device.agentVersion, installedAt: device.installedAt.toISOString(), + memberId: device.memberId, user: { name: device.member.user.name, email: device.member.user.email, }, - source: 'device_agent' as const, + source, + ...(integrationProvider ? { integrationProvider } : {}), complianceStatus, daysSinceLastCheckIn: daysSinceCheckIn(device.lastCheckIn), hasActiveAgentSession: - !!device.agentSession && device.agentSession.expiresAt.getTime() > Date.now(), + source === 'device_agent' && + !!device.agentSession && + device.agentSession.expiresAt.getTime() > Date.now(), }; }; diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/TeamMembersClient.tsx b/apps/app/src/app/(app)/[orgId]/people/all/components/TeamMembersClient.tsx index c481f278b1..bb4d9772d9 100644 --- a/apps/app/src/app/(app)/[orgId]/people/all/components/TeamMembersClient.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/all/components/TeamMembersClient.tsx @@ -53,6 +53,11 @@ import type { MemberWithUser, TaskCompletion, TeamMembersData } from './TeamMemb import type { EmployeeSyncConnectionsData } from '../data/queries'; import { useEmployeeSync } from '../hooks/useEmployeeSync'; +// Sentinel value for the "Don't auto-sync" item in the sync-source dropdown. +// Radix Select items can't have an empty value, so disabling is modeled as a +// distinct option rather than the absence of a selection. +const NO_SYNC_VALUE = '__no_sync__'; + interface TeamMembersClientProps { data: TeamMembersData; organizationId: string; @@ -119,6 +124,7 @@ export function TeamMembersClient({ selectedProvider, isSyncing, syncEmployees, + setSyncProvider, hasAnyConnection, getProviderName, getProviderLogo, @@ -127,6 +133,7 @@ export function TeamMembersClient({ const lastSyncAt = employeeSyncData.lastSyncAt; const nextSyncAt = employeeSyncData.nextSyncAt; + const [isDisablingSync, setIsDisablingSync] = useState(false); const handleEmployeeSync = async ( provider: string, @@ -137,6 +144,21 @@ export function TeamMembersClient({ } }; + // Turn off the daily auto-sync without disconnecting the integration (which + // would also stop its compliance checks). Clears the org's sync provider so + // the scheduled job skips it; already-imported people are left untouched. + // Sets a busy flag (mirroring isSyncing) so the dropdown is locked while the + // request is in flight, preventing an overlapping provider change from racing. + const handleDisableSync = async () => { + if (!selectedProvider || isDisablingSync) return; + setIsDisablingSync(true); + try { + await setSyncProvider(null); + } finally { + setIsDisablingSync(false); + } + }; + const allItems = buildDisplayItems(data); const hasOffboardFilter = !!(offboardFrom || offboardTo); const effectiveStatusFilter = hasOffboardFilter && !statusFilter ? 'all' : statusFilter; @@ -358,13 +380,15 @@ export function TeamMembersClient({
diff --git a/apps/app/src/app/(app)/[orgId]/people/all/components/compute-device-status-map.ts b/apps/app/src/app/(app)/[orgId]/people/all/components/compute-device-status-map.ts index a78a5282b4..ac0af2ad67 100644 --- a/apps/app/src/app/(app)/[orgId]/people/all/components/compute-device-status-map.ts +++ b/apps/app/src/app/(app)/[orgId]/people/all/components/compute-device-status-map.ts @@ -35,6 +35,10 @@ export function computeDeviceStatusMap({ const agentRollup = new Map(); for (const d of agentDevices) { + // Integration-imported devices carry no compliance data, so they must not + // set a member's status (it would falsely read non-compliant/stale) nor + // suppress the richer Fleet fallback below. Only true agent devices count. + if (d.source !== 'device_agent') continue; if (!d.memberId || !complianceSet.has(d.memberId)) continue; const prev = agentRollup.get(d.memberId); diff --git a/apps/app/src/app/(app)/[orgId]/people/all/hooks/useEmployeeSync.test.ts b/apps/app/src/app/(app)/[orgId]/people/all/hooks/useEmployeeSync.test.ts new file mode 100644 index 0000000000..06ff41770e --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/people/all/hooks/useEmployeeSync.test.ts @@ -0,0 +1,75 @@ +import { act, renderHook } from '@testing-library/react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import type { EmployeeSyncConnectionsData } from '../data/queries'; + +vi.mock('@/lib/api-client', () => ({ + apiClient: { post: vi.fn().mockResolvedValue({ data: { success: true } }) }, +})); + +vi.mock('sonner', () => ({ + toast: { success: vi.fn(), error: vi.fn(), info: vi.fn(), warning: vi.fn() }, +})); + +import { apiClient } from '@/lib/api-client'; +import { toast } from 'sonner'; +import { useEmployeeSync } from './useEmployeeSync'; + +const initialData: EmployeeSyncConnectionsData = { + googleWorkspaceConnectionId: null, + ripplingConnectionId: null, + jumpcloudConnectionId: null, + selectedProvider: 'entra-id', + lastSyncAt: null, + nextSyncAt: null, + availableProviders: [ + { + slug: 'entra-id', + name: 'Microsoft Entra ID', + logoUrl: '', + connected: true, + connectionId: 'conn_1', + lastSyncAt: null, + nextSyncAt: null, + }, + ], +}; + +const ENDPOINT = + '/v1/integrations/sync/employee-sync-provider?organizationId=org_1'; + +describe('useEmployeeSync.setSyncProvider', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('turns off auto-sync by POSTing provider: null and confirms to the user', async () => { + const { result } = renderHook(() => + useEmployeeSync({ organizationId: 'org_1', initialData }), + ); + + await act(async () => { + await result.current.setSyncProvider(null); + }); + + expect(apiClient.post).toHaveBeenCalledWith(ENDPOINT, { provider: null }); + expect(toast.success).toHaveBeenCalledWith('Employee auto-sync turned off'); + }); + + it('selects a provider by POSTing its slug', async () => { + const { result } = renderHook(() => + useEmployeeSync({ organizationId: 'org_1', initialData }), + ); + + await act(async () => { + await result.current.setSyncProvider('entra-id'); + }); + + expect(apiClient.post).toHaveBeenCalledWith(ENDPOINT, { + provider: 'entra-id', + }); + expect(toast.success).toHaveBeenCalledWith( + 'Microsoft Entra ID set as your employee sync provider', + ); + }); +}); diff --git a/apps/app/src/app/(app)/[orgId]/people/all/hooks/useEmployeeSync.ts b/apps/app/src/app/(app)/[orgId]/people/all/hooks/useEmployeeSync.ts index d7fa5e5922..e835e9141f 100644 --- a/apps/app/src/app/(app)/[orgId]/people/all/hooks/useEmployeeSync.ts +++ b/apps/app/src/app/(app)/[orgId]/people/all/hooks/useEmployeeSync.ts @@ -97,6 +97,8 @@ export const useEmployeeSync = ({ ? PROVIDER_CONFIG[provider as BuiltInSyncProvider].name : (availableProviders.find((p) => p.slug === provider)?.name ?? provider); toast.success(`${name} set as your employee sync provider`); + } else { + toast.success('Employee auto-sync turned off'); } } catch (error) { toast.error('Failed to set sync provider'); diff --git a/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceAgentDevicesList.test.tsx b/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceAgentDevicesList.test.tsx index 020736e64b..704fe6ef39 100644 --- a/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceAgentDevicesList.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceAgentDevicesList.test.tsx @@ -1,4 +1,4 @@ -import { render, screen, fireEvent } from '@testing-library/react'; +import { render, screen, fireEvent, within } from '@testing-library/react'; import { describe, expect, it, vi, beforeEach } from 'vitest'; import type { DeviceWithChecks } from '../types'; @@ -183,3 +183,112 @@ describe('DeviceAgentDevicesList', () => { ).not.toBeInTheDocument(); }); }); + +function makeIntegrationDevice( + overrides: Partial = {}, +): DeviceWithChecks { + return makeDevice({ + id: 'dev_int', + name: 'Imported Mac', + // Imported devices carry no real compliance data — defaults are all false. + isCompliant: false, + diskEncryptionEnabled: false, + antivirusEnabled: false, + passwordPolicySet: false, + screenLockEnabled: false, + agentVersion: null, + hasActiveAgentSession: false, + // Even though lastCheckIn (= last sync) is fresh, the server still derives + // non_compliant; the UI must override this to "Not tracked" by source. + complianceStatus: 'non_compliant', + source: 'integration', + integrationProvider: { slug: 'kandji', name: 'Kandji' }, + ...overrides, + }); +} + +describe('DeviceAgentDevicesList — integration-imported devices', () => { + it('labels the device with its integration provider in the Source column', () => { + render(); + expect(screen.getByText('Kandji')).toBeInTheDocument(); + }); + + it('shows "Not tracked" compliance instead of a false "No"', () => { + render(); + expect(screen.getByText('Not tracked')).toBeInTheDocument(); + expect(screen.queryByText('No')).not.toBeInTheDocument(); + expect(screen.queryByText('Yes')).not.toBeInTheDocument(); + }); + + it('renders the "not tracked" explainer tooltip trigger', () => { + render(); + expect( + screen.getByRole('button', { name: /Why is compliance not tracked\?/i }), + ).toBeInTheDocument(); + }); + + it('does not present an imported device as Online or Offline', () => { + render(); + // Imported devices get no live-status dot at all (a spacer, no title). + expect(screen.queryByTitle('Online')).not.toBeInTheDocument(); + expect(screen.queryByTitle('Offline')).not.toBeInTheDocument(); + }); + + it('renders a source filter only when more than one source is present', () => { + const { rerender } = render( + , + ); + expect(screen.queryByLabelText('Filter by source')).not.toBeInTheDocument(); + + rerender( + , + ); + expect(screen.getByLabelText('Filter by source')).toBeInTheDocument(); + }); + + it('filters the table to a single source when selected', () => { + render( + , + ); + expect(screen.getByText('Agent Mac')).toBeInTheDocument(); + expect(screen.getByText('Imported Mac')).toBeInTheDocument(); + + fireEvent.change(screen.getByLabelText('Filter by source'), { + target: { value: 'integration:kandji' }, + }); + expect(screen.queryByText('Agent Mac')).not.toBeInTheDocument(); + expect(screen.getByText('Imported Mac')).toBeInTheDocument(); + }); + + it('keeps distinct providers separate in the filter even with the same display name', () => { + render( + , + ); + const select = screen.getByLabelText('Filter by source'); + // "All sources" + two distinct providers (not merged into one "MDM"). + expect(within(select).getAllByRole('option')).toHaveLength(3); + fireEvent.change(select, { target: { value: 'integration:intune' } }); + expect(screen.queryByText('Device X')).not.toBeInTheDocument(); + expect(screen.getByText('Device Y')).toBeInTheDocument(); + }); +}); diff --git a/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceAgentDevicesList.tsx b/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceAgentDevicesList.tsx index 63822fd0a7..4b47758dac 100644 --- a/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceAgentDevicesList.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceAgentDevicesList.tsx @@ -1,12 +1,7 @@ 'use client'; import { - Badge, Button, - DropdownMenu, - DropdownMenuContent, - DropdownMenuItem, - DropdownMenuTrigger, Empty, EmptyDescription, EmptyHeader, @@ -17,21 +12,11 @@ import { Stack, Table, TableBody, - TableCell, TableHead, TableHeader, TableRow, - Text, } from '@trycompai/design-system'; -import { - Download, - Information, - OverflowMenuVertical, - Search, - TrashCan, -} from '@trycompai/design-system/icons'; -import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from '@trycompai/ui/tooltip'; -import Link from 'next/link'; +import { Download, Search } from '@trycompai/design-system/icons'; import { useParams } from 'next/navigation'; import { useMemo, useState } from 'react'; import { toast } from 'sonner'; @@ -44,6 +29,8 @@ import { devicesCsvFilename, downloadDevicesCsv, } from '../lib/devices-csv'; +import { DeviceTableRow } from './DeviceListCells'; +import { sourceKey, sourceLabel } from '../lib/device-source'; import { DeviceDetails } from './DeviceDetails'; import { RemoveDeviceAlert } from '../../all/components/RemoveDeviceAlert'; @@ -51,129 +38,6 @@ export interface DeviceAgentDevicesListProps { devices: DeviceWithChecks[]; } -const CHECK_FIELDS = [ - { key: 'diskEncryptionEnabled' as const, label: 'Disk Encryption' }, - { key: 'antivirusEnabled' as const, label: 'Antivirus' }, - { key: 'passwordPolicySet' as const, label: 'Password Policy' }, - { key: 'screenLockEnabled' as const, label: 'Screen Lock' }, -]; - -const PLATFORM_LABELS: Record = { - macos: 'macOS', - windows: 'Windows', - linux: 'Linux', -}; - -function formatTimeAgo(dateString: string | null): string { - if (!dateString) return 'Never'; - const date = new Date(dateString); - const now = new Date(); - const diffMs = now.getTime() - date.getTime(); - const diffHours = Math.floor(diffMs / (1000 * 60 * 60)); - - if (diffHours < 1) return 'Just now'; - if (diffHours < 24) return `${diffHours}h ago`; - const diffDays = Math.floor(diffHours / 24); - return `${diffDays}d ago`; -} - -/** Device is considered online if it checked in within the last 2 hours */ -function isDeviceOnline(lastCheckIn: string | null): boolean { - if (!lastCheckIn) return false; - const diffMs = Date.now() - new Date(lastCheckIn).getTime(); - return diffMs < 2 * 60 * 60 * 1000; -} - -function staleLabel(daysSinceLastCheckIn: number | null): string { - return daysSinceLastCheckIn === null ? 'Stale' : `Stale (${daysSinceLastCheckIn}d)`; -} - -function staleTooltipCopy(daysSinceLastCheckIn: number | null): string { - return daysSinceLastCheckIn === null - ? "This device was registered but hasn't sent a compliance check yet. If it's not new, the agent may not be running or the device may be offline." - : "This device hasn't reported to CompAI in over 7 days, so we can't verify its current compliance. It may be offline, the agent may need to be updated, or the device may no longer be in use. Check with the employee."; -} - -function UserNameCell({ device, orgId }: { device: DeviceWithChecks; orgId: string }) { - const memberId = device.memberId; - - if (!memberId) { - return ( -
- {device.user.name} - {device.user.email} -
- ); - } - - return ( -
- e.stopPropagation()} - > - {device.user.name} - - {device.user.email} -
- ); -} - -function CompliantBadge({ device }: { device: DeviceWithChecks }) { - if (device.complianceStatus === 'stale') { - return ( -
- {staleLabel(device.daysSinceLastCheckIn)} - - - - - - - {staleTooltipCopy(device.daysSinceLastCheckIn)} - - - -
- ); - } - if (device.complianceStatus === 'compliant') { - return Yes; - } - return No; -} - -function CheckBadges({ device }: { device: DeviceWithChecks }) { - if (device.complianceStatus === 'stale') { - return ( -
- {CHECK_FIELDS.map(({ key, label }) => ( - - — - - ))} -
- ); - } - return ( -
- {CHECK_FIELDS.map(({ key, label }) => ( - - {label} - - ))} -
- ); -} - export const DeviceAgentDevicesList = ({ devices, }: DeviceAgentDevicesListProps) => { @@ -185,22 +49,37 @@ export const DeviceAgentDevicesList = ({ const [selectedDevice, setSelectedDevice] = useState(null); const [actionDevice, setActionDevice] = useState(null); const [searchQuery, setSearchQuery] = useState(''); + const [sourceFilter, setSourceFilter] = useState('all'); const [page, setPage] = useState(1); const [perPage, setPerPage] = useState(50); const [isRemoveDeviceAlertOpen, setIsRemoveDeviceAlertOpen] = useState(false); const [isRemovingDevice, setIsRemovingDevice] = useState(false); + // Distinct sources present, keyed by a stable id (so two providers that share + // a display name don't collapse into one option) with a label for display. + const sourceOptions = useMemo(() => { + const byKey = new Map(); + for (const d of devices) byKey.set(sourceKey(d), sourceLabel(d)); + return Array.from(byKey, ([key, label]) => ({ key, label })).sort((a, b) => + a.label.localeCompare(b.label), + ); + }, [devices]); + const filteredDevices = useMemo(() => { - if (!searchQuery) return devices; const query = searchQuery.toLowerCase(); - return devices.filter( - (device) => + return devices.filter((device) => { + if (sourceFilter !== 'all' && sourceKey(device) !== sourceFilter) { + return false; + } + if (!query) return true; + return ( device.name.toLowerCase().includes(query) || device.user.name.toLowerCase().includes(query) || device.user.email.toLowerCase().includes(query) || - device.platform.toLowerCase().includes(query), - ); - }, [devices, searchQuery]); + device.platform.toLowerCase().includes(query) + ); + }); + }, [devices, searchQuery, sourceFilter]); const pageCount = Math.max(1, Math.ceil(filteredDevices.length / perPage)); const paginatedDevices = useMemo(() => { @@ -251,20 +130,40 @@ export const DeviceAgentDevicesList = ({ return (
-
- - - - - +
+ + + + + { + setSearchQuery(e.target.value); + setPage(1); + }} + /> + +
+ {(sourceOptions.length > 1 || sourceFilter !== 'all') && ( + + )}
+ + {copy} + + + ); +} + +export function SourceBadge({ device }: { device: DeviceWithChecks }) { + return {sourceLabel(device)}; +} + +export function UserNameCell({ + device, + orgId, +}: { + device: DeviceWithChecks; + orgId: string; +}) { + const memberId = device.memberId; + + if (!memberId) { + return ( +
+ {device.user.name} + {device.user.email} +
+ ); + } + + return ( +
+ e.stopPropagation()} + > + {device.user.name} + + {device.user.email} +
+ ); +} + +/** + * Compliance badge for an integration-imported device. Shared with the details + * panel so the "Not tracked" copy and provider fallback stay consistent. + */ +export function NotTrackedBadge({ device }: { device: DeviceWithChecks }) { + return ( +
+ Not tracked + +
+ ); +} + +export function CompliantBadge({ device }: { device: DeviceWithChecks }) { + // Integration-imported devices are inventory records, not compliance records — + // CompAI never ran security checks on them, so showing "No" (red) would be a + // false negative. Present them as untracked instead. + if (!isComplianceTracked(device)) { + return ; + } + + if (device.complianceStatus === 'stale') { + return ( +
+ {staleLabel(device.daysSinceLastCheckIn)} + +
+ ); + } + if (device.complianceStatus === 'compliant') { + return Yes; + } + return No; +} + +export function CheckBadges({ device }: { device: DeviceWithChecks }) { + if (!isComplianceTracked(device) || device.complianceStatus === 'stale') { + const reason = !isComplianceTracked(device) + ? 'not collected for imported devices' + : 'unknown (device is stale)'; + return ( +
+ {CHECK_FIELDS.map(({ key, label }) => ( + + — + + ))} +
+ ); + } + return ( +
+ {CHECK_FIELDS.map(({ key, label }) => ( + + {label} + + ))} +
+ ); +} + +export interface DeviceTableRowProps { + device: DeviceWithChecks; + orgId: string; + canRemoveDevice: boolean; + onSelect: (device: DeviceWithChecks) => void; + onRequestRemove: (device: DeviceWithChecks) => void; +} + +export function DeviceTableRow({ + device, + orgId, + canRemoveDevice, + onSelect, + onRequestRemove, +}: DeviceTableRowProps) { + const isAgent = device.source === 'device_agent'; + const showOnline = isAgent && isDeviceOnline(device.lastCheckIn); + return ( + onSelect(device)} style={{ cursor: 'pointer' }}> + +
+ {/* Live status only applies to agent devices; imported/fleet rows get a + spacer so they aren't visually labeled as an offline agent. */} + {isAgent ? ( + + ) : ( +
+
+ + + + +
+ {PLATFORM_LABELS[device.platform] ?? device.platform} + + {device.osVersion} + +
+
+ + + + + {formatTimeAgo(device.lastCheckIn)} + + + + + + + + +
+ + e.stopPropagation()} + className="inline-flex h-8 w-8 items-center justify-center rounded-md text-muted-foreground transition-colors hover:bg-muted hover:text-foreground" + > + + + + { + e.stopPropagation(); + onRequestRemove(device); + }} + variant="destructive" + > + + Remove Device + + + +
+
+
+ ); +} diff --git a/apps/app/src/app/(app)/[orgId]/people/devices/components/DevicesTabContent.tsx b/apps/app/src/app/(app)/[orgId]/people/devices/components/DevicesTabContent.tsx index 45b0818592..06eff5ddab 100644 --- a/apps/app/src/app/(app)/[orgId]/people/devices/components/DevicesTabContent.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/devices/components/DevicesTabContent.tsx @@ -25,11 +25,16 @@ export function DevicesTabContent({ isCurrentUserOwner }: DevicesTabContentProps error: fleetError, } = useFleetHosts(); - // Filter out Fleet hosts for members who already have device-agent devices. - // Device agent takes priority over Fleet. + // Filter out Fleet hosts for members who already have a device-agent device. + // Device agent takes priority over Fleet. Integration-imported devices are + // inventory-only (no compliance data) and must NOT suppress a richer Fleet + // host, so only true agent devices count toward de-duplication. const filteredFleetDevices = useMemo(() => { const memberIdsWithAgent = new Set( - agentDevices.map((d) => d.memberId).filter(Boolean), + agentDevices + .filter((d) => d.source === 'device_agent') + .map((d) => d.memberId) + .filter(Boolean), ); return fleetHosts.filter( (host) => !host.member_id || !memberIdsWithAgent.has(host.member_id), diff --git a/apps/app/src/app/(app)/[orgId]/people/devices/lib/device-source.ts b/apps/app/src/app/(app)/[orgId]/people/devices/lib/device-source.ts new file mode 100644 index 0000000000..5658cae312 --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/people/devices/lib/device-source.ts @@ -0,0 +1,83 @@ +import type { DeviceWithChecks } from '../types'; + +/** + * Human label for where a device came from. Shared by the devices table, the + * details panel, and the CSV export so they can't drift on source labels. + */ +export function sourceLabel(device: DeviceWithChecks): string { + if (device.source === 'integration') { + return device.integrationProvider?.name ?? 'Integration'; + } + if (device.source === 'fleet') return 'Fleet'; + return 'Comp Agent'; +} + +/** + * Stable identifier for a device's source — used to key the source filter so two + * distinct providers that happen to share a display name don't collapse into one + * option. (sourceLabel is for display only.) + */ +export function sourceKey(device: DeviceWithChecks): string { + if (device.source === 'integration') { + return `integration:${device.integrationProvider?.slug ?? 'unknown'}`; + } + return device.source; // 'device_agent' | 'fleet' +} + +/** + * True for devices whose compliance posture CompAI actually collects. Only + * integration-imported devices are inventory-only ("Not tracked"); agent and + * Fleet devices both report real compliance. + */ +export function isComplianceTracked(device: DeviceWithChecks): boolean { + return device.source !== 'integration'; +} + +// --------------------------------------------------------------------------- +// Shared device presentation helpers (used by both the list and details views +// so copy/labels/thresholds can't drift between them). +// --------------------------------------------------------------------------- + +export const PLATFORM_LABELS: Record = { + macos: 'macOS', + windows: 'Windows', + linux: 'Linux', +}; + +export const CHECK_FIELDS = [ + { key: 'diskEncryptionEnabled' as const, dbKey: 'disk_encryption', label: 'Disk Encryption' }, + { key: 'antivirusEnabled' as const, dbKey: 'antivirus', label: 'Antivirus' }, + { key: 'passwordPolicySet' as const, dbKey: 'password_policy', label: 'Password Policy' }, + { key: 'screenLockEnabled' as const, dbKey: 'screen_lock', label: 'Screen Lock' }, +]; + +export function formatTimeAgo(dateString: string | null): string { + if (!dateString) return 'Never'; + const diffMs = Date.now() - new Date(dateString).getTime(); + const diffHours = Math.floor(diffMs / (1000 * 60 * 60)); + if (diffHours < 1) return 'Just now'; + if (diffHours < 24) return `${diffHours}h ago`; + return `${Math.floor(diffHours / 24)}d ago`; +} + +/** Device is considered online if it checked in within the last 2 hours. */ +export function isDeviceOnline(lastCheckIn: string | null): boolean { + if (!lastCheckIn) return false; + return Date.now() - new Date(lastCheckIn).getTime() < 2 * 60 * 60 * 1000; +} + +export function staleLabel(daysSinceLastCheckIn: number | null): string { + return daysSinceLastCheckIn === null ? 'Stale' : `Stale (${daysSinceLastCheckIn}d)`; +} + +export function staleTooltipCopy(daysSinceLastCheckIn: number | null): string { + return daysSinceLastCheckIn === null + ? "This device was registered but hasn't sent a compliance check yet. If it's not new, the agent may not be running or the device may be offline." + : "This device hasn't reported to CompAI in over 7 days, so we can't verify its current compliance. It may be offline, the agent may need to be updated, or the device may no longer be in use. Check with the employee."; +} + +/** Tooltip copy for the "Not tracked" badge on integration-imported devices. */ +export function notTrackedTooltipCopy(device: DeviceWithChecks): string { + const provider = device.integrationProvider?.name ?? 'an integration'; + return `This device was imported from ${provider}. CompAI doesn't collect compliance checks for imported devices — install the CompAI agent to track its security posture.`; +} diff --git a/apps/app/src/app/(app)/[orgId]/people/devices/lib/devices-csv.test.ts b/apps/app/src/app/(app)/[orgId]/people/devices/lib/devices-csv.test.ts index 9069e85ae3..d0432a606f 100644 --- a/apps/app/src/app/(app)/[orgId]/people/devices/lib/devices-csv.test.ts +++ b/apps/app/src/app/(app)/[orgId]/people/devices/lib/devices-csv.test.ts @@ -72,6 +72,7 @@ describe('buildDevicesCsv', () => { 'yes', 'yes', 'yes', + 'Comp Agent', ].join(','), ); }); @@ -90,7 +91,43 @@ describe('buildDevicesCsv', () => { const cells = row.split(','); expect(cells).toContain('stale'); expect(cells[7]).toBe('51'); // days since sync - expect(cells.slice(-4)).toEqual(['no', 'no', 'yes', 'yes']); + expect(cells.slice(9, 13)).toEqual(['no', 'no', 'yes', 'yes']); // the four checks + }); + + it('exports imported devices as not_tracked / n/a with the provider as source', () => { + const csv = buildDevicesCsv([ + makeDevice({ + source: 'integration', + integrationProvider: { slug: 'kandji', name: 'Kandji' }, + // Imported devices carry only defaults — must NOT export as non_compliant/no. + isCompliant: false, + complianceStatus: 'non_compliant', + diskEncryptionEnabled: false, + antivirusEnabled: false, + passwordPolicySet: false, + screenLockEnabled: false, + agentVersion: null, + }), + ]); + const cells = stripBom(csv).slice(0, -2).split('\r\n')[1].split(','); + expect(cells[8]).toBe('not_tracked'); // status + expect(cells.slice(9, 13)).toEqual(['n/a', 'n/a', 'n/a', 'n/a']); + expect(cells[13]).toBe('Kandji'); // source + }); + + it('neutralizes formula injection coming from an integration provider name', () => { + const csv = buildDevicesCsv([ + makeDevice({ + source: 'integration', + integrationProvider: { slug: 'evil', name: '=HYPERLINK("http://evil","x")' }, + }), + ]); + const body = stripBom(csv); + // The provider name lands in the trailing Source cell; the leading "=" must be + // apostrophe-prefixed INSIDE the quotes so spreadsheets treat it as text. + expect(body).toContain('"\'=HYPERLINK(""http://evil"",""x"")"'); + // And it must never appear as a bare, executable formula cell. + expect(body).not.toContain(',=HYPERLINK'); }); it('represents never-synced devices with empty last-check-in and empty days', () => { diff --git a/apps/app/src/app/(app)/[orgId]/people/devices/lib/devices-csv.ts b/apps/app/src/app/(app)/[orgId]/people/devices/lib/devices-csv.ts index 3aa656bf6d..638ab9aaf9 100644 --- a/apps/app/src/app/(app)/[orgId]/people/devices/lib/devices-csv.ts +++ b/apps/app/src/app/(app)/[orgId]/people/devices/lib/devices-csv.ts @@ -1,4 +1,5 @@ import type { DeviceWithChecks } from '../types'; +import { isComplianceTracked, sourceLabel } from './device-source'; export const DEVICES_CSV_HEADER = [ 'Device Name', @@ -14,6 +15,7 @@ export const DEVICES_CSV_HEADER = [ 'Antivirus', 'Password Policy', 'Screen Lock', + 'Source', ].join(','); const FORMULA_TRIGGER = /^[=+\-@\t\r]/; @@ -35,8 +37,13 @@ function yesNo(value: boolean): 'yes' | 'no' { } export function buildDevicesCsv(devices: DeviceWithChecks[]): string { - const rows = devices.map((d) => - [ + const rows = devices.map((d) => { + // Integration-imported devices are inventory-only; agent + Fleet carry real + // compliance. Use the shared helper so the CSV matches the rest of the UI. + const tracked = isComplianceTracked(d); + const status = tracked ? d.complianceStatus : 'not_tracked'; + const check = (value: boolean) => (tracked ? yesNo(value) : 'n/a'); + return [ escapeCell(d.name), escapeCell(d.user.name), escapeCell(d.user.email), @@ -45,13 +52,14 @@ export function buildDevicesCsv(devices: DeviceWithChecks[]): string { escapeCell(d.agentVersion ?? ''), escapeCell(d.lastCheckIn ?? ''), escapeCell(d.daysSinceLastCheckIn ?? ''), - escapeCell(d.complianceStatus), - escapeCell(yesNo(d.diskEncryptionEnabled)), - escapeCell(yesNo(d.antivirusEnabled)), - escapeCell(yesNo(d.passwordPolicySet)), - escapeCell(yesNo(d.screenLockEnabled)), - ].join(','), - ); + escapeCell(status), + escapeCell(check(d.diskEncryptionEnabled)), + escapeCell(check(d.antivirusEnabled)), + escapeCell(check(d.passwordPolicySet)), + escapeCell(check(d.screenLockEnabled)), + escapeCell(sourceLabel(d)), + ].join(','); + }); // RFC 4180: records separated by CRLF; trailing CRLF after the final record. // Prepend a UTF-8 BOM so Excel correctly detects UTF-8 encoding for non-ASCII data. return '\uFEFF' + [DEVICES_CSV_HEADER, ...rows].join('\r\n') + '\r\n'; diff --git a/apps/app/src/app/(app)/[orgId]/people/devices/types/index.ts b/apps/app/src/app/(app)/[orgId]/people/devices/types/index.ts index 149a5573f0..3ebb9834f8 100644 --- a/apps/app/src/app/(app)/[orgId]/people/devices/types/index.ts +++ b/apps/app/src/app/(app)/[orgId]/people/devices/types/index.ts @@ -34,7 +34,18 @@ export interface DeviceWithChecks { email: string; }; /** Indicates which system reported this device */ - source: 'device_agent' | 'fleet'; + source: 'device_agent' | 'fleet' | 'integration'; + /** + * Set only when `source === 'integration'`: the provider that imported this + * device, so the UI can label its provenance instead of mislabeling it as an + * agent device. `logoUrl` is optional — the DB provider row has no logo, so it + * may be filled later from the manifest registry. + */ + integrationProvider?: { + slug: string; + name: string; + logoUrl?: string; + }; /** Derived on the server; 'stale' = no check-in for >= 7 days. */ complianceStatus: DeviceComplianceStatus; /** Whole days since last check-in, or null when never synced. */ diff --git a/apps/app/src/app/(app)/setup/actions/create-organization-minimal.ts b/apps/app/src/app/(app)/setup/actions/create-organization-minimal.ts index 5177c55ed3..665fd5eb53 100644 --- a/apps/app/src/app/(app)/setup/actions/create-organization-minimal.ts +++ b/apps/app/src/app/(app)/setup/actions/create-organization-minimal.ts @@ -1,5 +1,6 @@ 'use server'; +import { grantInitialPentestCredit } from '@/actions/organization/lib/grant-initial-pentest-credit'; import { initializeOrganization } from '@/actions/organization/lib/initialize-organization'; import { authActionClientWithoutOrg } from '@/actions/safe-action'; import { env } from '@/env.mjs'; @@ -112,6 +113,9 @@ export const createOrganizationMinimal = authActionClientWithoutOrg console.error('Non-critical: failed to publish trust portal:', trustPortalResponse.error); } + // Ensure the reused org has its free pentest credit (idempotent). + await grantInitialPentestCredit(); + return { success: true, organizationId: existingOrg.id, @@ -213,6 +217,10 @@ export const createOrganizationMinimal = authActionClientWithoutOrg console.error('Non-critical: failed to publish trust portal:', trustPortalResponse.error); } + // Grant the new org its one free pentest credit so the owner can run a + // first penetration test without entering a card (non-fatal, idempotent). + await grantInitialPentestCredit(); + // Revalidate paths (non-critical, don't let failures kill the flow) try { const headersList = await headers(); diff --git a/apps/app/src/app/(app)/setup/actions/create-organization.ts b/apps/app/src/app/(app)/setup/actions/create-organization.ts index 0214448536..31d03a2813 100644 --- a/apps/app/src/app/(app)/setup/actions/create-organization.ts +++ b/apps/app/src/app/(app)/setup/actions/create-organization.ts @@ -1,5 +1,6 @@ 'use server'; +import { grantInitialPentestCredit } from '@/actions/organization/lib/grant-initial-pentest-credit'; import { initializeOrganization } from '@/actions/organization/lib/initialize-organization'; import { authActionClientWithoutOrg } from '@/actions/safe-action'; import { serverApi } from '@/lib/api-server'; @@ -128,6 +129,10 @@ export const createOrganization = authActionClientWithoutOrg console.error('Non-critical: failed to publish trust portal:', trustPortalResponse.error); } + // Grant the new org its one free pentest credit so the owner can run a + // first penetration test without entering a card (non-fatal, idempotent). + await grantInitialPentestCredit(); + const userOrgs = await db.member.findMany({ where: { userId: session.user.id, diff --git a/apps/app/src/app/api/people/agent-devices/route.test.ts b/apps/app/src/app/api/people/agent-devices/route.test.ts index d61d763aeb..cc1b65d736 100644 --- a/apps/app/src/app/api/people/agent-devices/route.test.ts +++ b/apps/app/src/app/api/people/agent-devices/route.test.ts @@ -1,15 +1,8 @@ import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; +import { NextResponse } from 'next/server'; -vi.mock('@/utils/auth', () => ({ - auth: { - api: { - getSession: vi.fn(), - }, - }, -})); - -vi.mock('next/headers', () => ({ - headers: vi.fn(async () => new Headers()), +vi.mock('@/lib/permissions.server', () => ({ + requireApiPermission: vi.fn(), })); vi.mock('@db/server', () => ({ @@ -17,14 +10,17 @@ vi.mock('@db/server', () => ({ device: { findMany: vi.fn(), }, + integrationConnection: { + findMany: vi.fn(async () => []), + }, }, })); -import { auth } from '@/utils/auth'; +import { requireApiPermission } from '@/lib/permissions.server'; import { db } from '@db/server'; import { GET } from './route'; -const mockedGetSession = vi.mocked(auth.api.getSession); +const mockedRequire = vi.mocked(requireApiPermission); const mockedFindMany = vi.mocked( (db as unknown as { device: { findMany: ReturnType } }).device.findMany, ); @@ -32,6 +28,18 @@ const mockedFindMany = vi.mocked( // Freeze "now" so day math is deterministic. const FIXED_NOW = new Date('2026-04-17T12:00:00.000Z'); +function req() { + return new Request('http://test/api/people/agent-devices'); +} + +function grant() { + mockedRequire.mockResolvedValue({ + organizationId: 'org_1', + userId: 'u_1', + permissions: {}, + } as unknown as Awaited>); +} + beforeAll(() => { vi.useFakeTimers(); vi.setSystemTime(FIXED_NOW); @@ -43,9 +51,7 @@ afterAll(() => { beforeEach(() => { vi.clearAllMocks(); - mockedGetSession.mockResolvedValue({ - session: { activeOrganizationId: 'org_1' }, - } as unknown as Awaited>); + grant(); }); function deviceRow(overrides: Partial> = {}) { @@ -73,15 +79,18 @@ function deviceRow(overrides: Partial> = {}) { } describe('GET /api/people/agent-devices', () => { - it('returns 401 when no organization is active', async () => { - mockedGetSession.mockResolvedValue({ session: {} } as never); - const res = await GET(); - expect(res.status).toBe(401); + it('forwards the 401/403 response when the RBAC guard denies access', async () => { + mockedRequire.mockResolvedValue( + NextResponse.json({ error: 'Forbidden' }, { status: 403 }), + ); + const res = await GET(req()); + expect(res.status).toBe(403); + expect(mockedFindMany).not.toHaveBeenCalled(); }); it('marks a fresh + isCompliant device as compliant', async () => { mockedFindMany.mockResolvedValue([deviceRow({ lastCheckIn: new Date(FIXED_NOW) })]); - const res = await GET(); + const res = await GET(req()); const body = await res.json(); expect(body.data[0].complianceStatus).toBe('compliant'); expect(body.data[0].daysSinceLastCheckIn).toBe(0); @@ -95,7 +104,7 @@ describe('GET /api/people/agent-devices', () => { lastCheckIn: new Date(FIXED_NOW), }), ]); - const res = await GET(); + const res = await GET(req()); const body = await res.json(); expect(body.data[0].complianceStatus).toBe('non_compliant'); }); @@ -103,7 +112,7 @@ describe('GET /api/people/agent-devices', () => { it('marks a device with lastCheckIn >= 7 days ago as stale', async () => { const eightDaysAgo = new Date(FIXED_NOW.getTime() - 8 * 24 * 60 * 60 * 1000); mockedFindMany.mockResolvedValue([deviceRow({ lastCheckIn: eightDaysAgo })]); - const res = await GET(); + const res = await GET(req()); const body = await res.json(); expect(body.data[0].complianceStatus).toBe('stale'); expect(body.data[0].daysSinceLastCheckIn).toBe(8); @@ -111,7 +120,7 @@ describe('GET /api/people/agent-devices', () => { it('marks a device with null lastCheckIn as stale', async () => { mockedFindMany.mockResolvedValue([deviceRow({ lastCheckIn: null })]); - const res = await GET(); + const res = await GET(req()); const body = await res.json(); expect(body.data[0].complianceStatus).toBe('stale'); expect(body.data[0].daysSinceLastCheckIn).toBeNull(); @@ -127,7 +136,7 @@ describe('GET /api/people/agent-devices', () => { deviceRow({ id: 'dev_expired', agentSession: { expiresAt: past } }), ]); - const res = await GET(); + const res = await GET(req()); const body = await res.json(); const byId = Object.fromEntries( body.data.map((d: { id: string }) => [d.id, d]), diff --git a/apps/app/src/app/api/people/agent-devices/route.ts b/apps/app/src/app/api/people/agent-devices/route.ts index 286459fa76..c44fa85ae4 100644 --- a/apps/app/src/app/api/people/agent-devices/route.ts +++ b/apps/app/src/app/api/people/agent-devices/route.ts @@ -1,20 +1,27 @@ -import { auth } from '@/utils/auth'; import { db } from '@db/server'; -import { headers } from 'next/headers'; import { NextResponse } from 'next/server'; +import { requireApiPermission } from '@/lib/permissions.server'; import { daysSinceCheckIn, getDeviceComplianceStatus, } from '@trycompai/utils/devices'; import type { CheckDetails, DeviceWithChecks } from '@/app/(app)/[orgId]/people/devices/types'; -export async function GET() { - const session = await auth.api.getSession({ headers: await headers() }); - const organizationId = session?.session.activeOrganizationId; +/** Maps the DB `DeviceSource` enum to the frontend source discriminant. */ +function mapSource(source: string): DeviceWithChecks['source'] { + if (source === 'integration') return 'integration'; + if (source === 'fleet') return 'fleet'; + return 'device_agent'; +} - if (!organizationId) { - return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }); - } +export async function GET(req: Request) { + // Enforce the same RBAC as the People area (route permission 'people' = + // member:read). The session-only check was insufficient — this route returns + // org device + integration-provider data and can be called directly by any + // active-org session, so gate it explicitly. + const ctx = await requireApiPermission(req, 'member', 'read'); + if (ctx instanceof NextResponse) return ctx; + const { organizationId } = ctx; const devices = await db.device.findMany({ where: { @@ -35,40 +42,98 @@ export async function GET() { orderBy: { installedAt: 'desc' }, }); - const data: DeviceWithChecks[] = devices.map((device) => { - const complianceStatus = getDeviceComplianceStatus({ - isCompliant: device.isCompliant, - lastCheckIn: device.lastCheckIn, - }); - return { - id: device.id, - name: device.name, - hostname: device.hostname, - platform: device.platform as 'macos' | 'windows' | 'linux', - osVersion: device.osVersion, - serialNumber: device.serialNumber, - hardwareModel: device.hardwareModel, - isCompliant: device.isCompliant, - diskEncryptionEnabled: device.diskEncryptionEnabled, - antivirusEnabled: device.antivirusEnabled, - passwordPolicySet: device.passwordPolicySet, - screenLockEnabled: device.screenLockEnabled, - checkDetails: (device.checkDetails as CheckDetails) ?? null, - lastCheckIn: device.lastCheckIn?.toISOString() ?? null, - agentVersion: device.agentVersion, - installedAt: device.installedAt.toISOString(), - memberId: device.memberId, - user: { - name: device.member.user.name, - email: device.member.user.email, + // Resolve provider name/slug for integration-sourced devices in one batched + // query, so each imported device shows its real provenance (e.g. "Kandji") + // instead of being mislabeled as an agent device. The DB provider row has no + // logo, so logoUrl is intentionally left undefined here. + const connectionIds = Array.from( + new Set( + devices + .filter((d) => d.source === 'integration' && d.integrationConnectionId) + .map((d) => d.integrationConnectionId as string), + ), + ); + + const providerByConnectionId = new Map(); + if (connectionIds.length > 0) { + const connections = await db.integrationConnection.findMany({ + where: { id: { in: connectionIds }, organizationId }, + select: { + id: true, + provider: { select: { slug: true, name: true } }, }, - source: 'device_agent' as const, - complianceStatus, - daysSinceLastCheckIn: daysSinceCheckIn(device.lastCheckIn), - hasActiveAgentSession: - !!device.agentSession && device.agentSession.expiresAt.getTime() > Date.now(), - }; - }); + }); + for (const conn of connections) { + if (conn.provider) { + providerByConnectionId.set(conn.id, { + slug: conn.provider.slug, + name: conn.provider.name, + }); + } + } + } + + // An agent device is always richer than an integration import of the same + // physical machine (it carries live compliance). The backend already refuses + // to overwrite an agent serial, but guard the read path too: if a serial is + // owned by an agent device, drop any integration row that shares it. + const agentSerials = new Set( + devices + .filter((d) => d.source !== 'integration' && d.serialNumber) + .map((d) => d.serialNumber as string), + ); + + const data: DeviceWithChecks[] = devices + .filter( + (device) => + !( + device.source === 'integration' && + device.serialNumber && + agentSerials.has(device.serialNumber) + ), + ) + .map((device) => { + const source = mapSource(device.source); + const complianceStatus = getDeviceComplianceStatus({ + isCompliant: device.isCompliant, + lastCheckIn: device.lastCheckIn, + }); + const provider = + source === 'integration' && device.integrationConnectionId + ? providerByConnectionId.get(device.integrationConnectionId) + : undefined; + return { + id: device.id, + name: device.name, + hostname: device.hostname, + platform: device.platform as 'macos' | 'windows' | 'linux', + osVersion: device.osVersion, + serialNumber: device.serialNumber, + hardwareModel: device.hardwareModel, + isCompliant: device.isCompliant, + diskEncryptionEnabled: device.diskEncryptionEnabled, + antivirusEnabled: device.antivirusEnabled, + passwordPolicySet: device.passwordPolicySet, + screenLockEnabled: device.screenLockEnabled, + checkDetails: (device.checkDetails as CheckDetails) ?? null, + lastCheckIn: device.lastCheckIn?.toISOString() ?? null, + agentVersion: device.agentVersion, + installedAt: device.installedAt.toISOString(), + memberId: device.memberId, + user: { + name: device.member.user.name, + email: device.member.user.email, + }, + source, + ...(provider ? { integrationProvider: provider } : {}), + complianceStatus, + daysSinceLastCheckIn: daysSinceCheckIn(device.lastCheckIn), + hasActiveAgentSession: + source === 'device_agent' && + !!device.agentSession && + device.agentSession.expiresAt.getTime() > Date.now(), + }; + }); return NextResponse.json({ data }); } diff --git a/apps/app/src/app/api/people/fleet-hosts/route.ts b/apps/app/src/app/api/people/fleet-hosts/route.ts index 4d65c52961..4046f585bb 100644 --- a/apps/app/src/app/api/people/fleet-hosts/route.ts +++ b/apps/app/src/app/api/people/fleet-hosts/route.ts @@ -1,19 +1,17 @@ -import { auth } from '@/utils/auth'; import { getFleetInstance } from '@/lib/fleet'; import { db } from '@db/server'; -import { headers } from 'next/headers'; import { NextResponse } from 'next/server'; +import { requireApiPermission } from '@/lib/permissions.server'; import type { Host } from '@/app/(app)/[orgId]/people/devices/types'; const MDM_POLICY_ID = -9999; -export async function GET() { - const session = await auth.api.getSession({ headers: await headers() }); - const organizationId = session?.session.activeOrganizationId; - - if (!organizationId) { - return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }); - } +export async function GET(req: Request) { + // Same RBAC as the People area (member:read) — this returns org device data + // and must not be reachable by an active-org session lacking people access. + const ctx = await requireApiPermission(req, 'member', 'read'); + if (ctx instanceof NextResponse) return ctx; + const { organizationId } = ctx; const fleet = await getFleetInstance();