From b075f7cd2bc539e4f3a768a5a3d4e2482e64cfb7 Mon Sep 17 00:00:00 2001
From: Tofik Hasanov <annexcies@gmail.com>
Date: Tue, 7 Jul 2026 16:54:51 -0400
Subject: [PATCH] fix(isms): lock before writing control links so edits
 serialize with approve
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Follow-up flagged on #3364: addControls/removeControl wrote the control-link rows
and only then called invalidateApprovalIfNeeded (which holds the per-document
lock), so the write itself ran outside the lock — it could interleave with
approve() before the lock was taken. (Impact was limited because control links
are not part of the frozen version snapshot, but the mutation should still be
serialized like every other content edit.)

Both transactions now take lockDocument() as their first statement, before the
createMany/deleteMany. The conditional invalidate is unchanged (real-change-only),
and the lock is re-entrant so re-taking it there is a no-op. Adds an ordering test.

Tests: 325 api ISMS tests passing; typecheck clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012CweXoSSEP89mX93u3Bdcp
---
 .../src/isms/isms-document-control.service.spec.ts | 14 ++++++++++++++
 apps/api/src/isms/isms-document-control.service.ts |  9 +++++++++
 2 files changed, 23 insertions(+)

diff --git a/apps/api/src/isms/isms-document-control.service.spec.ts b/apps/api/src/isms/isms-document-control.service.spec.ts
index b209192048..7ffa17c4d9 100644
--- a/apps/api/src/isms/isms-document-control.service.spec.ts
+++ b/apps/api/src/isms/isms-document-control.service.spec.ts
@@ -90,6 +90,20 @@ describe('IsmsDocumentControlService', () => {
         skipDuplicates: true,
       });
     });
+
+    it('takes the per-document lock BEFORE writing links (serializes vs approve)', async () => {
+      await service.addControls({
+        documentId: 'doc_1',
+        organizationId: 'org_1',
+        controlIds: ['ctl_1', 'ctl_2'],
+      });
+
+      const lockOrder = (mockDb.$executeRaw as jest.Mock).mock
+        .invocationCallOrder[0];
+      const writeOrder = (mockDb.ismsDocumentControlLink.createMany as jest.Mock)
+        .mock.invocationCallOrder[0];
+      expect(lockOrder).toBeLessThan(writeOrder);
+    });
   });
 
   describe('removeControl', () => {
diff --git a/apps/api/src/isms/isms-document-control.service.ts b/apps/api/src/isms/isms-document-control.service.ts
index c3454f5c9e..1d5a715c0e 100644
--- a/apps/api/src/isms/isms-document-control.service.ts
+++ b/apps/api/src/isms/isms-document-control.service.ts
@@ -5,6 +5,7 @@ import {
 } from '@nestjs/common';
 import { db } from '@db';
 import { invalidateApprovalIfNeeded } from './utils/approval';
+import { lockDocument } from './utils/document-lock';
 
 /**
  * Org-level mapping between an ISMS document and the organization's Controls
@@ -41,6 +42,11 @@ export class IsmsDocumentControlService {
     // register/narrative edits). Only a REAL change invalidates — an idempotent
     // re-link that inserts nothing must not downgrade an approved document.
     await db.$transaction(async (tx) => {
+      // Take the per-document lock BEFORE writing links so the mutation is fully
+      // serialized against approve() (which holds the same lock). invalidate is
+      // only called on a real change, and the lock is re-entrant, so re-taking it
+      // there is a no-op.
+      await lockDocument(tx, documentId);
       const { count } = await tx.ismsDocumentControlLink.createMany({
         data: uniqueControlIds.map((controlId) => ({
           ismsDocumentId: documentId,
@@ -68,6 +74,9 @@ export class IsmsDocumentControlService {
     // Only a real unlink (a row actually deleted) invalidates sign-off; removing
     // a control that wasn't linked must not downgrade an approved document.
     await db.$transaction(async (tx) => {
+      // Lock before the delete so the mutation serializes against approve()
+      // (same lock); invalidate only on a real unlink, re-taking the lock no-ops.
+      await lockDocument(tx, documentId);
       const { count } = await tx.ismsDocumentControlLink.deleteMany({
         where: { ismsDocumentId: documentId, controlId },
       });