Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 28 additions & 1 deletion apps/api/src/cloud-security/finding-exceptions.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,23 +44,50 @@ describe('ActiveExceptionSet', () => {
expect(set.has('c1', 'check-a', resourceId)).toBe(true);
expect(set.exceptedResourceIds('c1', 'check-a')).toEqual([resourceId]);
});

it('exposes exception metadata via infoFor when built with it', () => {
const key = ActiveExceptionSet.key('c1', 'check-a', 'r1');
const set = new ActiveExceptionSet(
[key],
new Map([[key, { id: 'fex_1', reason: 'Intentional public bucket' }]]),
);
expect(set.infoFor('c1', 'check-a', 'r1')).toEqual({
id: 'fex_1',
reason: 'Intentional public bucket',
});
expect(set.infoFor('c1', 'check-a', 'r2')).toBeNull();
});

it('infoFor returns null on a set built from bare keys (has() still true)', () => {
const set = new ActiveExceptionSet([
ActiveExceptionSet.key('c1', 'check-a', 'r1'),
]);
expect(set.has('c1', 'check-a', 'r1')).toBe(true);
expect(set.infoFor('c1', 'check-a', 'r1')).toBeNull();
});
});

describe('loadActiveExceptionSet', () => {
beforeEach(() => jest.clearAllMocks());

it('builds the set from active exceptions', async () => {
it('builds the set from active exceptions, carrying id + reason', async () => {
findMany.mockResolvedValue([
{
id: 'fex_1',
connectionId: 'c1',
checkId: 'aws-s3-public-access',
resourceId: 'bucket-1',
reason: 'Bucket only hosts a static website redirect.',
},
] as never);

const set = await loadActiveExceptionSet('org_1');

expect(set.has('c1', 'aws-s3-public-access', 'bucket-1')).toBe(true);
expect(set.infoFor('c1', 'aws-s3-public-access', 'bucket-1')).toEqual({
id: 'fex_1',
reason: 'Bucket only hosts a static website redirect.',
});
// Query only active exceptions (not revoked, not expired).
expect(findMany).toHaveBeenCalledWith(
expect.objectContaining({
Expand Down
55 changes: 49 additions & 6 deletions apps/api/src/cloud-security/finding-exceptions.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,15 @@
import { db } from '@db';

/**
* Metadata about one active exception. Display surfaces use it to show the
* documented reason next to a suppressed finding and to offer revoke (which
* needs the row id) without a second query.
*/
export interface ActiveExceptionInfo {
id: string;
reason: string;
}

/**
* The set of findings — keyed by (connectionId, checkId, resourceId) — that
* currently have an ACTIVE exception (not revoked, not expired) for an org.
Expand All @@ -24,9 +34,16 @@ export class ActiveExceptionSet {
* every result row into memory.
*/
private readonly resourceIdsByConnCheck: Map<string, Set<string>>;
/** Exception metadata per canonical key. Optional — evaluation-only callers
* build the set from bare keys and never ask for it. */
private readonly infoByKey: ReadonlyMap<string, ActiveExceptionInfo>;

constructor(keys: Iterable<string>) {
constructor(
keys: Iterable<string>,
infoByKey?: ReadonlyMap<string, ActiveExceptionInfo>,
) {
this.keys = new Set(keys);
this.infoByKey = infoByKey ?? new Map();
this.resourceIdsByConnCheck = new Map();
for (const key of this.keys) {
// key = `${connectionId}::${checkId}::${resourceId}`. A resourceId can
Expand Down Expand Up @@ -75,6 +92,23 @@ export class ActiveExceptionSet {
);
return ids ? Array.from(ids) : [];
}

/**
* Metadata (row id + reason) of the active exception covering this finding,
* or null when none. Null also for sets built without metadata (bare keys) —
* `has()` stays the authority on whether a finding is excepted.
*/
infoFor(
connectionId: string,
checkId: string,
resourceId: string,
): ActiveExceptionInfo | null {
return (
this.infoByKey.get(
ActiveExceptionSet.key(connectionId, checkId, resourceId),
) ?? null
);
}
}

/**
Expand All @@ -94,13 +128,22 @@ export async function loadActiveExceptionSet(
revokedAt: null,
OR: [{ expiresAt: null }, { expiresAt: { gt: new Date() } }],
},
select: { connectionId: true, checkId: true, resourceId: true },
select: {
id: true,
connectionId: true,
checkId: true,
resourceId: true,
reason: true,
},
});
return new ActiveExceptionSet(
active.map((e) =>
const infoByKey = new Map<string, ActiveExceptionInfo>();
for (const e of active) {
infoByKey.set(
ActiveExceptionSet.key(e.connectionId, e.checkId, e.resourceId),
),
);
{ id: e.id, reason: e.reason },
);
}
return new ActiveExceptionSet(infoByKey.keys(), infoByKey);
} catch {
return new ActiveExceptionSet([]);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -811,9 +811,11 @@ describe('TaskIntegrationsController', () => {
);
mockFindingExceptionFindMany.mockResolvedValue([
{
id: 'fex_1',
connectionId: 'conn_1',
checkId: 'aws-s3-public-access',
resourceId: 'reports-bucket',
reason: 'Bucket intentionally public: static website redirect only.',
},
]);
// The excepted-failure count is now computed via a targeted query (the
Expand All @@ -827,6 +829,11 @@ describe('TaskIntegrationsController', () => {
expect(runs[0].exceptedCount).toBe(1);
expect(runs[0].status).toBe('success');
expect(runs[0].results[0].excepted).toBe(true);
// Excepted rows carry the exception's id (for revoke) and its reason.
expect(runs[0].results[0].exceptionId).toBe('fex_1');
expect(runs[0].results[0].exceptionReason).toBe(
'Bucket intentionally public: static website redirect only.',
);
// Exact count is computed via the targeted query, scoped to this run's
// excepted resourceIds (not by loading + filtering every result).
expect(mockCheckRunRepository.countExceptedFailures).toHaveBeenCalledWith(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -857,23 +857,33 @@ export class TaskIntegrationsController {

// Tag each sampled result with whether it's excepted (for display);
// authoritative totals come from the run's summary columns +
// exceptedCount above. Cap evidence so one oversized blob can't bloat
// the payload that the browser must parse + render.
const sample = run.results.map((r) => ({
id: r.id,
passed: r.passed,
resourceType: r.resourceType,
resourceId: r.resourceId,
title: r.title,
description: r.description,
severity: r.severity,
remediation: r.remediation,
evidence: r.evidence,
collectedAt: r.collectedAt,
excepted:
// exceptedCount above. Excepted rows also carry the exception's id
// (so the UI can offer revoke) and its documented reason. Cap evidence
// so one oversized blob can't bloat the payload that the browser must
// parse + render.
const sample = run.results.map((r) => {
const excepted =
!r.passed &&
exceptions.has(run.connectionId, run.checkId, r.resourceId),
}));
exceptions.has(run.connectionId, run.checkId, r.resourceId);
const exceptionInfo = excepted
? exceptions.infoFor(run.connectionId, run.checkId, r.resourceId)
: null;
return {
id: r.id,
passed: r.passed,
resourceType: r.resourceType,
resourceId: r.resourceId,
title: r.title,
description: r.description,
severity: r.severity,
remediation: r.remediation,
evidence: r.evidence,
collectedAt: r.collectedAt,
excepted,
exceptionId: exceptionInfo?.id,
exceptionReason: exceptionInfo?.reason,
};
});
const results = capResultsForList(sample).map((r) => ({
...r,
evidence: capEvidence(r.evidence),
Expand Down
73 changes: 65 additions & 8 deletions apps/api/src/soa/soa.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -596,7 +596,11 @@ describe('SOAService', () => {
expect(result).toEqual(generated);
});

it('forces Not Applicable on physical-security (7.x) controls for a fully remote org', async () => {
it('keeps a fully remote org\'s saved answer on physical-security (7.x) controls so the export matches the editable SoA', async () => {
// Regression (CS-749): a fully remote org can mark a 7.x control
// Applicable, and the export must reflect the saved answer instead of
// force-locking it to Not Applicable — the org can move to a physical
// office at any time.
const generated = {
fileBuffer: Buffer.from('pdf'),
mimeType: 'application/pdf',
Expand Down Expand Up @@ -643,8 +647,8 @@ describe('SOAService', () => {
},
approver: null,
answers: [
// A stale justification persisted before the org went remote — must
// NOT leak into the forced Not Applicable output.
// This org edited the 7.x control back to Applicable — the export must
// honour it, not force Not Applicable.
{
questionId: 'q-phys',
answer: 'We maintain physical access controls at our office',
Expand All @@ -660,19 +664,72 @@ describe('SOAService', () => {
const phys = passedQuestions.find((q) => q.id === 'q-phys');
const other = passedQuestions.find((q) => q.id === 'q-other');

// Fully remote + 7.x → forced Not Applicable with the remote justification,
// overriding the stale persisted answer.
expect(phys?.columnMapping.isApplicable).toBe(false);
// Fully remote + 7.x, but a saved answer exists → the saved answer wins.
expect(phys?.columnMapping.isApplicable).toBe(true);
expect(phys?.columnMapping.justification).toBe(
'This control is not applicable as our organization operates fully remotely.',
'We maintain physical access controls at our office',
);
expect(phys?.answer).toBe(
'This control is not applicable as our organization operates fully remotely.',
'We maintain physical access controls at our office',
);
// Non-physical controls are unaffected and use the org's own answer.
expect(other?.columnMapping.isApplicable).toBe(true);
expect(other?.columnMapping.justification).toBe('Our own 5.1 justification');
});

it('defaults an unanswered physical-security (7.x) control to Not Applicable for a fully remote org', async () => {
const generated = {
fileBuffer: Buffer.from('pdf'),
mimeType: 'application/pdf',
filename: 'statement-of-applicability-iso-27001-v1.pdf',
};
mockGenerateSOAExportFile.mockReturnValue(generated);
mockCheckIfFullyRemote.mockResolvedValue(true);
(mockDb.sOADocument.findFirst as jest.Mock).mockResolvedValue({
id: 'doc-1',
organizationId: 'org-1',
preparedBy: 'Comp AI',
answeredQuestions: 0,
totalQuestions: 1,
approvedAt: null,
declinedAt: null,
status: 'draft',
version: 1,
framework: { name: 'ISO 27001' },
configuration: {
questions: [
{
id: 'q-phys',
text: 'Physical entry',
columnMapping: {
closure: '7.2',
title: 'Physical entry',
control_objective: 'obj',
isApplicable: true,
justification: 'another org shared-config text',
},
},
],
},
approver: null,
// No saved answer for the 7.x control yet.
answers: [],
});

await service.exportDocument(dto);

const passedQuestions = mockGenerateSOAExportFile.mock.calls[0][0];
const phys = passedQuestions.find((q) => q.id === 'q-phys');

// No saved answer → the remote default fills it in.
expect(phys?.columnMapping.isApplicable).toBe(false);
expect(phys?.columnMapping.justification).toBe(
'This control is not applicable as our organization operates fully remotely.',
);
expect(phys?.answer).toBe(
'This control is not applicable as our organization operates fully remotely.',
);
});
});

describe('saveAnswer', () => {
Expand Down
19 changes: 11 additions & 8 deletions apps/api/src/soa/soa.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -529,8 +529,8 @@ export class SOAService {
document.answers.map((answer) => [answer.questionId, answer]),
);

// Enforced rule, applied identically on screen and in this export: a fully
// remote org's physical-security (7.x) controls are Not Applicable.
// A fully remote org's physical-security (7.x) controls default to Not
// Applicable, applied identically on screen and in this export.
const isFullyRemote = await checkIfFullyRemote(
dto.organizationId,
this.storageLogger,
Expand All @@ -539,11 +539,14 @@ export class SOAService {
const exportQuestions: SOAExportQuestion[] = questions.map((question) => {
const answer = answersByQuestionId.get(question.id);
const closure = question.columnMapping?.closure ?? null;
const forceNotApplicable =
isFullyRemote && isPhysicalSecurityControl(closure ?? '');
// Forced-remote controls always use the remote rationale, never a stale
// persisted justification that could contradict the Not Applicable status.
const justification = forceNotApplicable
// The remote default only fills controls the org has never answered — a
// saved answer (including an edited one) always wins, so remote physical
// controls stay editable and the export matches the on-screen SoA.
const useRemoteDefault =
answer === undefined &&
isFullyRemote &&
isPhysicalSecurityControl(closure ?? '');
const justification = useRemoteDefault
? FULLY_REMOTE_JUSTIFICATION
: (answer?.answer ?? null);
return {
Expand All @@ -553,7 +556,7 @@ export class SOAService {
closure,
title: question.columnMapping?.title ?? null,
control_objective: question.columnMapping?.control_objective ?? null,
isApplicable: forceNotApplicable ? false : (answer?.isApplicable ?? null),
isApplicable: useRemoteDefault ? false : (answer?.isApplicable ?? null),
justification,
},
answer: justification,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ import { CheckGroupBlock } from './CheckGroupBlock';
import { buildCheckGroups } from './check-groups';
import { filterFindingsByConnection } from './finding-filters';
import { EvidenceJsonViewer } from './EvidenceJsonViewer';
import { MarkExceptionModal } from './MarkExceptionModal';
import { MarkExceptionModal } from '@/components/integrations/MarkExceptionModal';
import { RemediationSection } from './RemediationSection';

interface RemediationCapabilities {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,6 @@ interface EditableSOAFieldsProps {
isApplicable: boolean | null;
justification: string | null;
isPendingApproval: boolean;
isControl7?: boolean;
isFullyRemote?: boolean;
organizationId: string;
/** Called after a successful save so the table can override autofill/cache without a full reload. */
onUpdate?: (payload: SOAFieldSavePayload) => void;
Expand All @@ -47,8 +45,6 @@ export function EditableSOAFields({
isApplicable: initialIsApplicable,
justification: initialJustification,
isPendingApproval,
isControl7 = false,
isFullyRemote = false,
organizationId,
onUpdate,
}: EditableSOAFieldsProps) {
Expand All @@ -67,8 +63,11 @@ export function EditableSOAFields({
setJustification(initialJustification);
}, [initialIsApplicable, initialJustification]);

// If control 7.* and fully remote, disable editing
const isDisabled = isPendingApproval || (isControl7 && isFullyRemote);
// Editing is only locked while the document is pending approval. Physical-
// security (7.x) controls on a fully remote org are auto-filled to Not
// Applicable but stay editable — the org can move to a physical office at any
// time, so answers are never locked.
const isDisabled = isPendingApproval;

// Auto-focus justification field when dialog opens
useEffect(() => {
Expand Down
Loading
Loading