diff --git a/apps/api/src/isms/documents/data-source.ts b/apps/api/src/isms/documents/data-source.ts index 50c2577122..e76efba692 100644 --- a/apps/api/src/isms/documents/data-source.ts +++ b/apps/api/src/isms/documents/data-source.ts @@ -1,5 +1,6 @@ import { createHash } from 'node:crypto'; import { db } from '@db'; +import type { Prisma } from '@db'; import { parseStoredAnswers } from '../wizard/wizard-schema'; import type { IsmsPlatformData } from './types'; @@ -16,10 +17,19 @@ const HIGH_IMPACT = ['major', 'severe']; export async function collectPlatformData({ organizationId, frameworkId, + client, }: { organizationId: string; frameworkId: string; + /** + * Optional transaction client. The approval flow passes its transaction so + * the drift baseline is read at the SAME point in time as the rows frozen + * into the published version — otherwise a concurrent edit between the two + * reads makes a just-approved document immediately show as stale. + */ + client?: Prisma.TransactionClient; }): Promise { + const dbc = client ?? db; const [ organization, frameworkInstances, @@ -34,15 +44,15 @@ export async function collectPlatformData({ partiesRows, acceptanceRows, ] = await Promise.all([ - db.organization.findUnique({ + dbc.organization.findUnique({ where: { id: organizationId }, select: { name: true }, }), - db.frameworkInstance.findMany({ + dbc.frameworkInstance.findMany({ where: { organizationId }, select: { framework: { select: { name: true } } }, }), - db.vendor.findMany({ + dbc.vendor.findMany({ where: { organizationId }, select: { id: true, @@ -61,14 +71,14 @@ export async function collectPlatformData({ assignee: { select: { user: { select: { name: true, email: true } } } }, }, }), - db.member.count({ where: { organizationId, deactivated: false } }), - db.member.groupBy({ + dbc.member.count({ where: { organizationId, deactivated: false } }), + dbc.member.groupBy({ by: ['department'], where: { organizationId, deactivated: false }, _count: { _all: true }, }), - db.device.count({ where: { organizationId } }), - db.risk.findMany({ + dbc.device.count({ where: { organizationId } }), + dbc.risk.findMany({ where: { organizationId }, select: { id: true, @@ -86,18 +96,18 @@ export async function collectPlatformData({ assignee: { select: { user: { select: { name: true, email: true } } } }, }, }), - db.employeeTrainingVideoCompletion.count({ + dbc.employeeTrainingVideoCompletion.count({ where: { member: { organizationId } }, }), - db.frameworkEditorFramework.findUnique({ + dbc.frameworkEditorFramework.findUnique({ where: { id: frameworkId }, select: { name: true }, }), - db.ismsProfile.findUnique({ + dbc.ismsProfile.findUnique({ where: { organizationId_frameworkId: { organizationId, frameworkId } }, select: { answers: true }, }), - db.ismsInterestedParty.findMany({ + dbc.ismsInterestedParty.findMany({ where: { document: { organizationId, @@ -107,7 +117,7 @@ export async function collectPlatformData({ }, select: { id: true, name: true, category: true }, }), - db.riskAcceptance.findMany({ + dbc.riskAcceptance.findMany({ where: { organizationId }, select: { id: true, riskId: true, vendorId: true }, }), diff --git a/apps/api/src/isms/documents/risk-methodology-defaults.ts b/apps/api/src/isms/documents/risk-methodology-defaults.ts index 9f5200ab9b..380fce36b0 100644 --- a/apps/api/src/isms/documents/risk-methodology-defaults.ts +++ b/apps/api/src/isms/documents/risk-methodology-defaults.ts @@ -3,7 +3,7 @@ // behavior: the five-band risk level scale (very-low .. very-high, computed as // likelihood x impact in apps/api/src/risks/risk-level.ts) and the platform's // four treatment strategies (mitigate/avoid/transfer/accept) with their ISO -// 27002 option names (Modify/Avoid/Share/Retain). Keep text ASCII-safe: the +// 27001 option names (Modify/Avoid/Share/Retain). Keep text ASCII-safe: the // PDF renderer's standard fonts cannot render glyphs like ">=" ligatures. /** Platform likelihood levels, in ascending order (matches the Likelihood enum). */ diff --git a/apps/api/src/isms/documents/risk-methodology.ts b/apps/api/src/isms/documents/risk-methodology.ts index 4433b0388b..2ff5abd58c 100644 --- a/apps/api/src/isms/documents/risk-methodology.ts +++ b/apps/api/src/isms/documents/risk-methodology.ts @@ -168,7 +168,7 @@ export function buildRiskMethodologySections( { heading: 'Risk level matrix', intro: - 'The risk level is derived from the product of likelihood and impact (1-25), normalized to a 1-10 score and mapped to five bands: Very low (1-5), Low (6-10), Medium (11-15), High (16-20), and Very high (21-25). Each risk in the register carries its calculated level for both its inherent and residual states.', + 'The risk level is derived from the product of likelihood and impact (1-25), normalized to a 1-10 score (the product divided by 2.5, rounded up) and banded by score: Very low (score 1-2), Low (3-4), Medium (5-6), High (7-8), Very high (9-10). Expressed as the raw product, the bands are: Very low 1-5, Low 6-10, Medium 11-15, High 16-20, Very high 21-25. Each risk in the register carries its calculated level for both its inherent and residual states.', table: riskMatrixTable(), }, { diff --git a/apps/api/src/isms/documents/risk-treatment-readiness.ts b/apps/api/src/isms/documents/risk-treatment-readiness.ts new file mode 100644 index 0000000000..0cb7dfca89 --- /dev/null +++ b/apps/api/src/isms/documents/risk-treatment-readiness.ts @@ -0,0 +1,34 @@ +import { db } from '@db'; +import type { Prisma } from '@db'; +import { riskTreatmentValidationMessages } from './risk-treatment-plan'; + +/** + * Clause-6.1.3 readiness: the RTP renders from the platform Risk Register + + * Vendors (org-scoped), not from register rows of its own — so readiness reads + * those tables. Archived risks are out of the plan (see + * loadRiskTreatmentExtras). Shared by the submit gate and the page payload. + */ +export async function loadRiskTreatmentReadinessMessages({ + organizationId, + client, +}: { + organizationId: string; + client?: Prisma.TransactionClient; +}): Promise { + const dbc = client ?? db; + const [risks, vendors] = await Promise.all([ + dbc.risk.findMany({ + where: { organizationId, status: { not: 'archived' } }, + select: { assigneeId: true }, + }), + dbc.vendor.findMany({ + where: { organizationId }, + select: { assigneeId: true }, + }), + ]); + return riskTreatmentValidationMessages({ + riskCount: risks.length, + risksWithoutOwner: risks.filter((risk) => !risk.assigneeId).length, + vendorsWithoutOwner: vendors.filter((vendor) => !vendor.assigneeId).length, + }); +} diff --git a/apps/api/src/isms/isms.service.approve.spec.ts b/apps/api/src/isms/isms.service.approve.spec.ts index 3b1f19af1d..b76fe11da2 100644 --- a/apps/api/src/isms/isms.service.approve.spec.ts +++ b/apps/api/src/isms/isms.service.approve.spec.ts @@ -16,6 +16,16 @@ jest.mock('@db', () => ({ member: { findFirst: jest.fn() }, $transaction: jest.fn(), }, + Prisma: { + TransactionIsolationLevel: { RepeatableRead: 'RepeatableRead' }, + PrismaClientKnownRequestError: class PrismaClientKnownRequestError extends Error { + code: string; + constructor(message: string, { code }: { code: string }) { + super(message); + this.code = code; + } + }, + }, })); jest.mock('./documents/data-source', () => ({ collectPlatformData: jest.fn(), @@ -140,9 +150,12 @@ describe('IsmsService.approve (CS-701 versioning)', () => { await service.approve(args); + // The baseline is collected INSIDE the approval transaction (client + // threaded) so it reads the same point in time as the frozen rows. expect(mockCollect).toHaveBeenCalledWith({ organizationId: 'org_1', frameworkId: 'fw_1', + client: expect.anything(), }); // Re-derives in-tx from the same snapshot: persisted rows and the frozen // version come from one pass. diff --git a/apps/api/src/isms/isms.service.ts b/apps/api/src/isms/isms.service.ts index 0788a2761a..a306b54d14 100644 --- a/apps/api/src/isms/isms.service.ts +++ b/apps/api/src/isms/isms.service.ts @@ -26,7 +26,7 @@ import { import { defaultProcedureText } from './documents/management-review-defaults'; import { defaultRiskMethodologyNarrative } from './documents/risk-methodology'; import { loadRiskTreatmentExtras } from './documents/risk-treatment-export-data'; -import { riskTreatmentValidationMessages } from './documents/risk-treatment-plan'; +import { loadRiskTreatmentReadinessMessages } from './documents/risk-treatment-readiness'; import { updateDraftSnapshot } from './utils/draft-snapshot'; import { EXPORT_DOCUMENT_INCLUDE } from './utils/export-payload'; import { lockDocument } from './utils/document-lock'; @@ -450,20 +450,28 @@ export class IsmsService { const document = await this.requireDocument({ documentId, organizationId }); this.assertPendingApprovalBy({ document, member }); - const snapshot = await collectPlatformData({ - organizationId, - frameworkId: document.frameworkId, - }); const now = new Date(); // Freeze the draft into a new immutable published version and promote it to // currentVersion. Editing afterwards reverts status to draft but leaves this // published version live and exportable (CS-701). - const published = await db.$transaction(async (tx) => { + const publish = async (tx: Prisma.TransactionClient) => { // Serialize concurrent approvals (and register-row creates, which take the // same lock) on this document so they can't interleave and double-publish. await lockDocument(tx, documentId); + // Collect the drift baseline INSIDE the transaction so it reads the same + // point in time as the rows frozen into the published version below. + // The transaction runs REPEATABLE READ (one MVCC snapshot for every + // statement), so this baseline and the rows loaded further down can + // never observe different data — a concurrent platform edit commits + // entirely before or entirely after this approval. + const snapshot = await collectPlatformData({ + organizationId, + frameworkId: document.frameworkId, + client: tx, + }); + // Atomically claim the approval: the check-then-act guard above runs before // the transaction, so under READ COMMITTED a racing approve/decline could // read the same stale `needs_review`. This conditional update only matches @@ -512,6 +520,24 @@ export class IsmsService { data: { currentVersionId: result.versionId }, }); return result; + }; + + // REPEATABLE READ can abort with a write conflict (P2034) when a racing + // lifecycle write commits between our snapshot and our claim update — + // retry once; the rerun takes a fresh snapshot and either wins cleanly or + // fails the conditional claim with the friendly BadRequestException. + const runPublish = () => + db.$transaction(publish, { + isolationLevel: Prisma.TransactionIsolationLevel.RepeatableRead, + }); + const published = await runPublish().catch((error: unknown) => { + if ( + error instanceof Prisma.PrismaClientKnownRequestError && + error.code === 'P2034' + ) { + return runPublish(); + } + throw error; }); // Render + upload the frozen exports outside the transaction (Policies @@ -717,38 +743,6 @@ export class IsmsService { } } - /** - * Clause-6.1.3 readiness: the RTP renders from the platform Risk Register + - * Vendors (org-scoped), not from register rows of its own — so readiness - * reads those tables. Archived risks are out of the plan (see - * loadRiskTreatmentExtras). Shared by the submit gate and the page payload. - */ - private async riskTreatmentReadinessMessages({ - organizationId, - client, - }: { - organizationId: string; - client?: Prisma.TransactionClient; - }): Promise { - const dbc = client ?? db; - const [risks, vendors] = await Promise.all([ - dbc.risk.findMany({ - where: { organizationId, status: { not: 'archived' } }, - select: { assigneeId: true }, - }), - dbc.vendor.findMany({ - where: { organizationId }, - select: { assigneeId: true }, - }), - ]); - return riskTreatmentValidationMessages({ - riskCount: risks.length, - risksWithoutOwner: risks.filter((risk) => !risk.assigneeId).length, - vendorsWithoutOwner: vendors.filter((vendor) => !vendor.assigneeId) - .length, - }); - } - private async assertRiskTreatmentPlanComplete({ tx, organizationId, @@ -756,7 +750,7 @@ export class IsmsService { tx: Prisma.TransactionClient; organizationId: string; }) { - const messages = await this.riskTreatmentReadinessMessages({ + const messages = await loadRiskTreatmentReadinessMessages({ organizationId, client: tx, }); @@ -788,7 +782,7 @@ export class IsmsService { } const [extras, validationMessages] = await Promise.all([ loadRiskTreatmentExtras({ organizationId }), - this.riskTreatmentReadinessMessages({ organizationId }), + loadRiskTreatmentReadinessMessages({ organizationId }), ]); return { ...extras, validationMessages }; } diff --git a/apps/api/src/isms/utils/pdf-renderer.ts b/apps/api/src/isms/utils/pdf-renderer.ts index 73fa84e369..e920794315 100644 --- a/apps/api/src/isms/utils/pdf-renderer.ts +++ b/apps/api/src/isms/utils/pdf-renderer.ts @@ -115,7 +115,7 @@ export function renderIsmsPdf({ const threeCol = table.headers.length === 3; const cellFills = table.cellFills; // Never let a header word break mid-word ("Acceptanc/e"): floor each - // column at its header's longest word. Wide tables (the 6.1.3 plan has 9 + // column at its header's longest word. Wide tables (the 6.1.3 plan has 10 // columns) otherwise get squeezed below the header width by autotable's // content-proportional sizing. pdf.setFont('helvetica', 'bold'); diff --git a/apps/api/src/risks/risk-acceptances.controller.ts b/apps/api/src/risks/risk-acceptances.controller.ts index 0cbe24c1e5..f9a43b0294 100644 --- a/apps/api/src/risks/risk-acceptances.controller.ts +++ b/apps/api/src/risks/risk-acceptances.controller.ts @@ -56,6 +56,7 @@ export class RiskAcceptancesController { @ApiResponse(LIST_RISK_ACCEPTANCES_RESPONSES[401]) @ApiResponse(LIST_RISK_ACCEPTANCES_RESPONSES[403]) @ApiResponse(LIST_RISK_ACCEPTANCES_RESPONSES[404]) + @ApiResponse(LIST_RISK_ACCEPTANCES_RESPONSES[500]) async listRiskAcceptances( @Param('id') riskId: string, @OrganizationId() organizationId: string, @@ -85,6 +86,7 @@ export class RiskAcceptancesController { @ApiResponse(RECORD_RISK_ACCEPTANCE_RESPONSES[401]) @ApiResponse(RECORD_RISK_ACCEPTANCE_RESPONSES[403]) @ApiResponse(RECORD_RISK_ACCEPTANCE_RESPONSES[404]) + @ApiResponse(RECORD_RISK_ACCEPTANCE_RESPONSES[500]) async recordRiskAcceptance( @Param('id') riskId: string, @Body() dto: CreateRiskAcceptanceDto, diff --git a/apps/api/src/risks/risk-acceptances.service.spec.ts b/apps/api/src/risks/risk-acceptances.service.spec.ts index 5a8147dde2..5d2ccbeefb 100644 --- a/apps/api/src/risks/risk-acceptances.service.spec.ts +++ b/apps/api/src/risks/risk-acceptances.service.spec.ts @@ -5,6 +5,12 @@ const mockDb = { vendor: { findFirst: jest.fn() }, member: { findFirst: jest.fn() }, riskAcceptance: { findMany: jest.fn(), create: jest.fn() }, + // The create paths run inside a transaction with a subject row-lock; the + // callback receives this same mock as the transaction client. + $transaction: jest.fn( + (fn: (tx: typeof mockDb) => unknown): unknown => fn(mockDb), + ), + $queryRaw: jest.fn().mockResolvedValue([]), }; jest.mock('@db', () => ({ db: mockDb })); @@ -67,6 +73,22 @@ describe('RiskAcceptancesService', () => { expect(view.levelLabel).toBe('Very low'); }); + it('row-locks the risk and writes through the same transaction', async () => { + mockDb.risk.findFirst.mockResolvedValue(baseRisk); + mockDb.member.findFirst.mockResolvedValue(activeMember); + mockDb.riskAcceptance.create.mockResolvedValue(storedRow); + + await service.createForRisk('rsk_1', ORG, {}); + + // The subject lock serializes concurrent residual edits with the + // read-freeze-insert sequence, so a fresh acceptance can never be + // recorded against an already-superseded rating. + expect(mockDb.$transaction).toHaveBeenCalledTimes(1); + expect(mockDb.$queryRaw).toHaveBeenCalledTimes(1); + const rawQuery = mockDb.$queryRaw.mock.calls[0][0].join('?'); + expect(rawQuery).toContain('FOR UPDATE'); + }); + it('defaults the acceptor to the risk owner (assignee)', async () => { mockDb.risk.findFirst.mockResolvedValue(baseRisk); mockDb.member.findFirst.mockResolvedValue(activeMember); diff --git a/apps/api/src/risks/risk-acceptances.service.ts b/apps/api/src/risks/risk-acceptances.service.ts index a568037e29..336f3f83fe 100644 --- a/apps/api/src/risks/risk-acceptances.service.ts +++ b/apps/api/src/risks/risk-acceptances.service.ts @@ -3,7 +3,13 @@ import { Injectable, NotFoundException, } from '@nestjs/common'; -import { db, type Impact, type Likelihood, type RiskAcceptance } from '@db'; +import { + db, + type Impact, + type Likelihood, + type Prisma, + type RiskAcceptance, +} from '@db'; import { CreateRiskAcceptanceDto } from './dto/create-risk-acceptance.dto'; import { LEVEL_LABEL, ratingLevel, type RiskLevel } from './risk-level'; @@ -70,27 +76,36 @@ export class RiskAcceptancesService { */ assertAccess?: (risk: { assigneeId: string | null }) => void, ): Promise { - const risk = await db.risk.findFirst({ - where: { id: riskId, organizationId }, - select: { - assigneeId: true, - residualLikelihood: true, - residualImpact: true, - }, - }); - if (!risk) { - throw new NotFoundException( - `Risk with ID ${riskId} not found in organization ${organizationId}`, - ); - } - assertAccess?.(risk); + // Row-lock the risk for the whole read-freeze-insert sequence: a + // concurrent residual PATCH blocks until this commits, so the frozen + // rating is always the rating at acceptance time (never instantly stale). + return db.$transaction(async (tx) => { + // Org-scoped so a foreign-tenant id can never acquire (even briefly) + // another organization's row lock. + await tx.$queryRaw`SELECT id FROM "Risk" WHERE id = ${riskId} AND "organizationId" = ${organizationId} FOR UPDATE`; + const risk = await tx.risk.findFirst({ + where: { id: riskId, organizationId }, + select: { + assigneeId: true, + residualLikelihood: true, + residualImpact: true, + }, + }); + if (!risk) { + throw new NotFoundException( + `Risk with ID ${riskId} not found in organization ${organizationId}`, + ); + } + assertAccess?.(risk); - return this.createAcceptance({ - organizationId, - subject: { riskId }, - dto, - ownerMemberId: risk.assigneeId, - current: risk, + return this.createAcceptance({ + tx, + organizationId, + subject: { riskId }, + dto, + ownerMemberId: risk.assigneeId, + current: risk, + }); }); } @@ -109,19 +124,30 @@ export class RiskAcceptancesService { organizationId: string, dto: CreateRiskAcceptanceDto, ): Promise { - const vendor = await this.findVendorRating(vendorId, organizationId); + // Same row-lock rationale as createForRisk. + return db.$transaction(async (tx) => { + // Org-scoped so a foreign-tenant id can never acquire (even briefly) + // another organization's row lock. + await tx.$queryRaw`SELECT id FROM "Vendor" WHERE id = ${vendorId} AND "organizationId" = ${organizationId} FOR UPDATE`; + const vendor = await this.findVendorRating(vendorId, organizationId, tx); - return this.createAcceptance({ - organizationId, - subject: { vendorId }, - dto, - ownerMemberId: vendor.assigneeId, - current: vendor.rating, + return this.createAcceptance({ + tx, + organizationId, + subject: { vendorId }, + dto, + ownerMemberId: vendor.assigneeId, + current: vendor.rating, + }); }); } - private async findVendorRating(vendorId: string, organizationId: string) { - const vendor = await db.vendor.findFirst({ + private async findVendorRating( + vendorId: string, + organizationId: string, + client?: Prisma.TransactionClient, + ) { + const vendor = await (client ?? db).vendor.findFirst({ where: { id: vendorId, organizationId }, select: { assigneeId: true, @@ -160,13 +186,14 @@ export class RiskAcceptancesService { } private async createAcceptance(params: { + tx: Prisma.TransactionClient; organizationId: string; subject: { riskId: string } | { vendorId: string }; dto: CreateRiskAcceptanceDto; ownerMemberId: string | null; current: ResidualRating; }): Promise { - const { organizationId, subject, dto, ownerMemberId, current } = params; + const { tx, organizationId, subject, dto, ownerMemberId, current } = params; const acceptorId = dto.acceptedById ?? ownerMemberId; if (!acceptorId) { @@ -175,7 +202,7 @@ export class RiskAcceptancesService { ); } - const member = await db.member.findFirst({ + const member = await tx.member.findFirst({ where: { id: acceptorId, organizationId }, select: { deactivated: true, @@ -193,7 +220,7 @@ export class RiskAcceptancesService { ); } - const row = await db.riskAcceptance.create({ + const row = await tx.riskAcceptance.create({ data: { organizationId, ...subject, diff --git a/apps/api/src/risks/schemas/risk-acceptances.responses.ts b/apps/api/src/risks/schemas/risk-acceptances.responses.ts index 52ddbc5ece..6ebcf0d769 100644 --- a/apps/api/src/risks/schemas/risk-acceptances.responses.ts +++ b/apps/api/src/risks/schemas/risk-acceptances.responses.ts @@ -81,6 +81,39 @@ const NOT_FOUND = (entity: string): ApiResponseOptions => ({ }, }); +// Auth fields the controllers spread into every response (sibling pattern: +// get-risk-by-id.responses.ts). +const AUTH_RESPONSE_PROPERTIES = { + authType: { + type: 'string', + enum: ['api-key', 'session', 'service'], + description: 'How the request was authenticated', + }, + authenticatedUser: { + type: 'object', + description: 'User information (only for session auth)', + properties: { + id: { type: 'string', example: 'usr_def456ghi789' }, + email: { type: 'string', example: 'user@example.com' }, + }, + }, +}; + +const INTERNAL_ERROR: ApiResponseOptions = { + status: 500, + description: 'Internal server error', + content: { + 'application/json': { + schema: { + type: 'object', + properties: { + message: { type: 'string', example: 'Internal server error' }, + }, + }, + }, + }, +}; + const UNAUTHORIZED: ApiResponseOptions = { status: 401, description: 'Unauthorized - Invalid authentication', @@ -128,6 +161,7 @@ export const LIST_RISK_ACCEPTANCES_RESPONSES: Record< type: 'object', properties: { data: { type: 'array', items: RISK_ACCEPTANCE_SCHEMA }, + ...AUTH_RESPONSE_PROPERTIES, }, }, }, @@ -136,6 +170,7 @@ export const LIST_RISK_ACCEPTANCES_RESPONSES: Record< 401: UNAUTHORIZED, 403: FORBIDDEN, 404: NOT_FOUND('Risk'), + 500: INTERNAL_ERROR, }; export const RECORD_RISK_ACCEPTANCE_RESPONSES: Record< @@ -145,7 +180,17 @@ export const RECORD_RISK_ACCEPTANCE_RESPONSES: Record< 201: { status: 201, description: 'Acceptance recorded successfully', - content: { 'application/json': { schema: RISK_ACCEPTANCE_SCHEMA } }, + content: { + 'application/json': { + schema: { + type: 'object', + properties: { + ...RISK_ACCEPTANCE_SCHEMA.properties, + ...AUTH_RESPONSE_PROPERTIES, + }, + }, + }, + }, }, 400: { status: 400, @@ -168,4 +213,5 @@ export const RECORD_RISK_ACCEPTANCE_RESPONSES: Record< 401: UNAUTHORIZED, 403: FORBIDDEN, 404: NOT_FOUND('Risk'), + 500: INTERNAL_ERROR, }; diff --git a/apps/api/src/vendors/vendor-acceptances.controller.ts b/apps/api/src/vendors/vendor-acceptances.controller.ts index e1a2f39037..fde34ccc93 100644 --- a/apps/api/src/vendors/vendor-acceptances.controller.ts +++ b/apps/api/src/vendors/vendor-acceptances.controller.ts @@ -45,6 +45,7 @@ export class VendorAcceptancesController { @ApiResponse(LIST_VENDOR_ACCEPTANCES_RESPONSES[401]) @ApiResponse(LIST_VENDOR_ACCEPTANCES_RESPONSES[403]) @ApiResponse(LIST_VENDOR_ACCEPTANCES_RESPONSES[404]) + @ApiResponse(LIST_VENDOR_ACCEPTANCES_RESPONSES[500]) async listVendorAcceptances( @Param('id') vendorId: string, @OrganizationId() organizationId: string, @@ -72,6 +73,7 @@ export class VendorAcceptancesController { @ApiResponse(RECORD_VENDOR_ACCEPTANCE_RESPONSES[401]) @ApiResponse(RECORD_VENDOR_ACCEPTANCE_RESPONSES[403]) @ApiResponse(RECORD_VENDOR_ACCEPTANCE_RESPONSES[404]) + @ApiResponse(RECORD_VENDOR_ACCEPTANCE_RESPONSES[500]) async recordVendorAcceptance( @Param('id') vendorId: string, @Body() dto: CreateRiskAcceptanceDto, diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/RiskLevelMatrixPreview.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/RiskLevelMatrixPreview.tsx index 221275a7e8..083bf91173 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/components/RiskLevelMatrixPreview.tsx +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/RiskLevelMatrixPreview.tsx @@ -59,8 +59,9 @@ export function RiskLevelMatrixPreview() { Computed from likelihood x impact (1-25), normalized to the same 1-10 score the risk - badges use: Very low (1-5), Low (6-10), Medium (11-15), High (16-20), Very high (21-25). - This table renders into the document and is not editable. + badges use (score bands: Very low 1-2, Low 3-4, Medium 5-6, High 7-8, Very high 9-10 — + i.e. raw products 1-5, 6-10, 11-15, 16-20, 21-25). This table renders into the document + and is not editable. ); diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/RiskTreatmentPlanClient.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/RiskTreatmentPlanClient.tsx index 6c06c1a34e..0084975b2e 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/components/RiskTreatmentPlanClient.tsx +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/RiskTreatmentPlanClient.tsx @@ -29,10 +29,17 @@ export function RiskTreatmentPlanClient(props: RiskTreatmentPlanClientProps) { const { riskTreatment, error, isLoading, mutateRiskTreatment } = useIsmsRiskTreatment(documentId); - const blockedReason = - riskTreatment && riskTreatment.validationMessages.length > 0 - ? riskTreatment.validationMessages.join(' ') - : null; + // null = ready to submit. Fail closed: a load/revalidation error blocks + // submission even when stale cached data is still present (SWR keeps the + // previous data on a failed revalidate), and loading blocks until the + // server has confirmed readiness. + const blockedReason = error + ? 'The readiness check could not be loaded — refresh the page and try again.' + : riskTreatment + ? riskTreatment.validationMessages.length > 0 + ? riskTreatment.validationMessages.join(' ') + : null + : 'Checking the plan is ready to submit...'; return ( option.id === defaultAcceptorId, + ) + ? (defaultAcceptorId ?? '') + : ''; const { control, handleSubmit, @@ -70,7 +78,7 @@ export function RecordAcceptanceDialog({ formState: { isSubmitting, errors }, } = useForm({ resolver: zodResolver(acceptanceSchema), - defaultValues: { acceptedById: defaultAcceptorId ?? '', notes: '' }, + defaultValues: { acceptedById: validDefaultId, notes: '' }, }); const acceptorId = watch('acceptedById'); @@ -78,7 +86,7 @@ export function RecordAcceptanceDialog({ acceptorOptions.find((option) => option.id === acceptorId)?.name ?? 'the selected member'; const handleClose = (next: boolean) => { - if (!next) reset({ acceptedById: defaultAcceptorId ?? '', notes: '' }); + if (!next) reset({ acceptedById: validDefaultId, notes: '' }); onOpenChange(next); }; diff --git a/apps/app/src/components/risks/acceptance/ResidualAcceptanceCard.tsx b/apps/app/src/components/risks/acceptance/ResidualAcceptanceCard.tsx index cc66f3e7e9..d0a84417d5 100644 --- a/apps/app/src/components/risks/acceptance/ResidualAcceptanceCard.tsx +++ b/apps/app/src/components/risks/acceptance/ResidualAcceptanceCard.tsx @@ -60,7 +60,10 @@ export function ResidualAcceptanceCard({ acceptorOptions, canUpdate, }: ResidualAcceptanceCardProps) { - const { acceptances, latest, isLoading, recordAcceptance } = useAcceptances(kind, subjectId); + const { acceptances, latest, isLoading, error, mutate, recordAcceptance } = useAcceptances( + kind, + subjectId, + ); const [dialogOpen, setDialogOpen] = useState(false); const currentLevelLabel = @@ -95,7 +98,9 @@ export function ResidualAcceptanceCard({

Residual risk acceptance

{latest && !latestStale && Accepted} {latest && latestStale && Stale} - {!latest && !isLoading && Awaiting acceptance} + {!latest && !isLoading && !error && ( + Awaiting acceptance + )}
The risk owner's formal, dated acceptance of the residual risk (ISO 27001 @@ -114,7 +119,16 @@ export function ResidualAcceptanceCard({ )}
- {isLoading && acceptances.length === 0 ? ( + {error ? ( + // A failed load must never read as "no acceptance recorded" — that + // could prompt a duplicate record of an existing formal acceptance. +
+ Couldn't load the acceptance history. + +
+ ) : isLoading && acceptances.length === 0 ? (
diff --git a/apps/app/src/hooks/use-risk-acceptances.ts b/apps/app/src/hooks/use-risk-acceptances.ts index 3f00ba1dbb..fa9f7e0ccc 100644 --- a/apps/app/src/hooks/use-risk-acceptances.ts +++ b/apps/app/src/hooks/use-risk-acceptances.ts @@ -48,7 +48,12 @@ export function useAcceptances(kind: AcceptanceSubjectKind, subjectId: string | const endpoint = endpointFor(kind, subjectId); const swr = useApiSWR(endpoint); - const acceptances = swr.data?.data?.data ?? []; + // apiClient resolves API failures into ApiResponse.error (the fetcher never + // throws), so swr.error alone misses forbidden/missing responses — surface + // both so callers can distinguish a failed load from an empty history. + const apiError = swr.data?.error ? new Error(swr.data.error) : null; + const error = swr.error ?? apiError; + const acceptances = error ? [] : (swr.data?.data?.data ?? []); const latest = acceptances[0] ?? null; const recordAcceptance = useCallback( @@ -68,7 +73,7 @@ export function useAcceptances(kind: AcceptanceSubjectKind, subjectId: string | acceptances, latest, isLoading: swr.isLoading, - error: swr.error, + error, mutate: swr.mutate, recordAcceptance, };