From d95dc2e34a96e577a99886893a4537064b02a445 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov <72318342+tofikwest@users.noreply.github.com> Date: Tue, 7 Jul 2026 13:27:27 -0400 Subject: [PATCH 1/7] feat(isms): document versioning and history (CS-701) (#3361) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(isms): document versioning and history (CS-701) Bring ISMS documents onto the Policies versioning model. Approving a document freezes an immutable published version (v1). Editing afterwards creates a draft without touching the published version, which stays live and exportable. Re-approving publishes the next version and moves the old one into history. Rendered DOCX/PDF are retained per published version so an auditor sampling v1 later downloads exactly what was approved. - schema: IsmsDocument gains currentVersion/draftNarrative/draftSnapshot; IsmsDocumentVersion gains contentSnapshot/changelog/publishedBy. Backfill migration (pre-GA, flag-gated) moves the old single-version model over. - api: new IsmsVersionService freezes a version at approval and renders + uploads PDF/DOCX to S3 after the txn; lists history; serves a version's export (stored file -> snapshot re-render -> live fallback). Editing an approved document reverts status to draft but preserves the published version. Drift baseline + draft narrative moved onto the document. New GET /v1/isms/documents/:id/versions; optional versionId on export. - app: version-history section on every document detail page with per-version PDF/DOCX download, and a "Published vN / editing creates a new draft" signal. - tests: 312 api + 119 app tests passing. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_012CweXoSSEP89mX93u3Bdcp * fix(isms): address CS-701 PR review — export integrity, download UX, org scoping Fixes the real issues from the automated PR review: - Versioned export never serves live/draft data (audit integrity): getVersionExport now falls back to the immutable snapshot if a stored S3 object is missing, and throws for a version with neither a file nor a snapshot instead of rendering current live data. - Default export of a clean approved document serves its retained published artifact (byte-identical to approval); only in-progress drafts export the draft. - Regenerating ("Generate from platform data") now invalidates approval, matching every other edit path, so an approved doc can't silently diverge from its published version. - Version-history download buttons are gated on real availability (file or snapshot); unavailable formats are hidden with a clear note. - Version-history fetch passes the route org (X-Organization-Id), matching the per-version download path. - Approval success is separated from the best-effort post-approval refresh so a revalidation hiccup no longer reports "Failed to approve". - Version history retains the approver via the frozen snapshot metadata if the approving member is later deleted. Not changed (deliberate): approval stays committed if export rendering fails — the immutable contentSnapshot makes every version reproducible and the AWS SDK retries transient errors; throwing would strand a doc "approved but unretryable". Tests: 316 api + 122 app ISMS tests passing; typecheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_012CweXoSSEP89mX93u3Bdcp * fix(isms): second review pass — default export, download errors, tx isolation, a11y Addresses the follow-up automated review on the fix commit: - Default export (no versionId) now serves the current PUBLISHED version whenever one exists — so v1 stays the live/exportable default while v2 is being drafted (CS-701). Only a never-published document renders the live draft. (P1) - Per-version downloads now catch export failures and surface a toast instead of failing silently / as an unhandled rejection. (P2) - The approve snapshot reads the org profile through the same transaction client (threaded through resolveOrgProfile/loadOrgProfile) so profile + register rows share one point-in-time. (P2) - Version-history download buttons get version-specific accessible labels ("Download v2 as PDF") for screen-reader / voice-control users. (P3) Tests: 318 api + 123 app ISMS tests passing; typecheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_012CweXoSSEP89mX93u3Bdcp * fix(isms): third review pass — no snapshot-less published versions, download races - Migration no longer promotes legacy approved docs to a published version. The pre-CS-701 version row had no contentSnapshot, so serving it as currentVersion would 404 on export (regression from the default-export + no-live-fallback fixes). Legacy approved docs revert to draft and re-approve once to establish a real v1; all legacy version rows are dropped. currentVersionId is now only ever set by the approve flow, which always writes a contentSnapshot — so it can never point at a snapshot-less version. (P1) - Per-version download indicator clears only if the finishing request still owns it, so starting a second download no longer clears the first's spinner early. (P3) Not changed (deliberate, re-confirmed): approval stays committed if publishRenders fails — createPublishedVersion always persists an immutable contentSnapshot, so the version re-renders on demand (no lost export); forcing a throw would strand a doc "approved but unretryable". Tests: 318 api + 123 app ISMS tests passing; typecheck clean; migration SQL validated. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_012CweXoSSEP89mX93u3Bdcp * fix(isms): migration — preserve approvals, add FKs after legacy cleanup Addresses the latest review on the CS-701 migration: - Non-destructive: existing approved documents keep their approved status. The migration no longer resets them to draft — it only drops the legacy (pre-CS-701, snapshot-less) version rows and leaves currentVersionId null, so an approved doc exports its current content on demand and captures a real versioned artifact on its next approval. currentVersionId is only ever set by the approve flow, which always writes a contentSnapshot. (P2) - Defensive ordering: the legacy-row DELETE now runs BEFORE the foreign keys are added, so FK validation can never trip on a stale publishedById. (In practice publishedById was a dead/always-null column pre-migration, so it could not have failed — but the ordering makes it robust regardless.) (P2) Verified: migration applies cleanly from scratch (fresh DB, all migrations deploy); 318 api + 123 app ISMS tests passing; typecheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_012CweXoSSEP89mX93u3Bdcp --------- Co-authored-by: Claude Opus 4.8 (1M context) --- apps/api/src/isms/documents/generate.spec.ts | 27 +- apps/api/src/isms/documents/generate.ts | 25 +- apps/api/src/isms/documents/org-profile.ts | 14 +- .../src/isms/dto/export-isms-document.dto.ts | 13 +- .../api/src/isms/isms-context.service.spec.ts | 257 ++++++++------- apps/api/src/isms/isms-context.service.ts | 139 ++++----- .../src/isms/isms-narrative.service.spec.ts | 74 ++--- apps/api/src/isms/isms-narrative.service.ts | 33 +- .../isms/isms-version.service.export.spec.ts | 196 ++++++++++++ .../api/src/isms/isms-version.service.spec.ts | 231 ++++++++++++++ apps/api/src/isms/isms-version.service.ts | 294 ++++++++++++++++++ .../isms/isms.controller.permissions.spec.ts | 6 + apps/api/src/isms/isms.controller.spec.ts | 22 ++ apps/api/src/isms/isms.controller.ts | 15 + apps/api/src/isms/isms.module.ts | 7 +- .../api/src/isms/isms.service.approve.spec.ts | 181 +++++++++++ ...isms.service.ensure-setup-fallback.spec.ts | 9 +- .../src/isms/isms.service.lifecycle.spec.ts | 135 +------- apps/api/src/isms/isms.service.spec.ts | 9 +- apps/api/src/isms/isms.service.ts | 59 +++- .../api/src/isms/utils/draft-snapshot.spec.ts | 50 +++ apps/api/src/isms/utils/draft-snapshot.ts | 28 ++ apps/api/src/isms/utils/export-generator.ts | 4 +- .../api/src/isms/utils/export-payload.spec.ts | 54 ++++ apps/api/src/isms/utils/export-payload.ts | 181 +++++++++++ .../src/isms/utils/version-snapshot.spec.ts | 78 ----- apps/api/src/isms/utils/version-snapshot.ts | 45 --- .../ContextOfOrganizationClient.test.tsx | 8 +- .../InterestedPartiesClient.test.tsx | 8 +- .../components/IsmsApprovalSection.test.tsx | 4 +- .../isms/components/IsmsApprovalSection.tsx | 27 +- .../components/IsmsControlMappings.test.tsx | 4 +- .../isms/components/IsmsDocumentShell.tsx | 27 +- .../components/IsmsVersionHistory.test.tsx | 111 +++++++ .../isms/components/IsmsVersionHistory.tsx | 141 +++++++++ .../isms/components/LeadershipClient.test.tsx | 8 +- .../isms/components/LeadershipClient.tsx | 3 +- .../isms/components/ObjectivesClient.test.tsx | 8 +- .../components/RequirementsClient.test.tsx | 8 +- .../isms/components/ScopeClient.test.tsx | 8 +- .../documents/isms/components/ScopeClient.tsx | 5 +- .../components/__test-helpers__/dsMocks.tsx | 19 ++ .../isms/hooks/exportIsmsDocument.ts | 10 +- .../hooks/useIsmsDocumentVersions.test.ts | 94 ++++++ .../isms/hooks/useIsmsDocumentVersions.ts | 85 +++++ .../[orgId]/documents/isms/isms-types.ts | 41 ++- .../migration.sql | 53 ++++ packages/db/prisma/schema/auth.prisma | 1 + packages/db/prisma/schema/isms.prisma | 49 ++- 49 files changed, 2293 insertions(+), 615 deletions(-) create mode 100644 apps/api/src/isms/isms-version.service.export.spec.ts create mode 100644 apps/api/src/isms/isms-version.service.spec.ts create mode 100644 apps/api/src/isms/isms-version.service.ts create mode 100644 apps/api/src/isms/isms.service.approve.spec.ts create mode 100644 apps/api/src/isms/utils/draft-snapshot.spec.ts create mode 100644 apps/api/src/isms/utils/draft-snapshot.ts create mode 100644 apps/api/src/isms/utils/export-payload.spec.ts create mode 100644 apps/api/src/isms/utils/export-payload.ts delete mode 100644 apps/api/src/isms/utils/version-snapshot.spec.ts delete mode 100644 apps/api/src/isms/utils/version-snapshot.ts create mode 100644 apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsVersionHistory.test.tsx create mode 100644 apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsVersionHistory.tsx create mode 100644 apps/app/src/app/(app)/[orgId]/documents/isms/hooks/useIsmsDocumentVersions.test.ts create mode 100644 apps/app/src/app/(app)/[orgId]/documents/isms/hooks/useIsmsDocumentVersions.ts create mode 100644 packages/db/prisma/migrations/20260707120000_isms_document_versioning/migration.sql diff --git a/apps/api/src/isms/documents/generate.spec.ts b/apps/api/src/isms/documents/generate.spec.ts index d4eb81cdec..3547f853ec 100644 --- a/apps/api/src/isms/documents/generate.spec.ts +++ b/apps/api/src/isms/documents/generate.spec.ts @@ -40,10 +40,10 @@ function buildTx() { ismsInterestedParty: registerTable(), ismsInterestedPartyRequirement: registerTable(), ismsObjective: registerTable(), - ismsDocument: { findFirst: jest.fn().mockResolvedValue(null) }, - ismsDocumentVersion: { + ismsDocument: { findFirst: jest.fn().mockResolvedValue(null), - create: jest.fn().mockResolvedValue({}), + // CS-701: the draft narrative lives on IsmsDocument, not a version row. + findUnique: jest.fn().mockResolvedValue(null), update: jest.fn().mockResolvedValue({}), }, }; @@ -132,24 +132,27 @@ describe('runDerivation', () => { expect(tx.ismsObjective.createMany).toHaveBeenCalled(); }); - it('creates a version with the derived narrative for isms_scope', async () => { + it('writes the derived narrative onto an empty document draft (isms_scope)', async () => { const tx = buildTx(); await runDerivation({ tx: asTx(tx), type: 'isms_scope', ...baseArgs }); - expect(tx.ismsDocumentVersion.create).toHaveBeenCalled(); - const created = tx.ismsDocumentVersion.create.mock.calls[0][0].data; - expect(created.narrative.certificateScopeSentence).toBeDefined(); + expect(tx.ismsDocument.update).toHaveBeenCalledWith( + expect.objectContaining({ where: { id: 'doc_1' } }), + ); + const updated = tx.ismsDocument.update.mock.calls[0][0].data; + expect(updated.draftNarrative.certificateScopeSentence).toBeDefined(); }); - it('updates the existing version narrative for leadership', async () => { + it('preserves a non-empty existing draft narrative (CS-437 override)', async () => { const tx = buildTx(); - tx.ismsDocumentVersion.findFirst.mockResolvedValue({ id: 'ver_1' }); + tx.ismsDocument.findUnique.mockResolvedValue({ + draftNarrative: { certificateScopeSentence: 'Manual edit' }, + }); await runDerivation({ tx: asTx(tx), type: 'leadership_commitment', ...baseArgs, }); - expect(tx.ismsDocumentVersion.update).toHaveBeenCalledWith( - expect.objectContaining({ where: { id: 'ver_1' } }), - ); + // A regenerate must never clobber the customer's manual narrative. + expect(tx.ismsDocument.update).not.toHaveBeenCalled(); }); }); diff --git a/apps/api/src/isms/documents/generate.ts b/apps/api/src/isms/documents/generate.ts index 4554fbe2b9..4497aa66f1 100644 --- a/apps/api/src/isms/documents/generate.ts +++ b/apps/api/src/isms/documents/generate.ts @@ -174,22 +174,17 @@ async function generateNarrative({ if (!derived) return; const narrative: Prisma.InputJsonValue = JSON.parse(JSON.stringify(derived)); - const latest = await tx.ismsDocumentVersion.findFirst({ - where: { documentId, isLatest: true }, + // The draft narrative lives on IsmsDocument (CS-701). Preserve a non-empty + // existing draft so a regenerate never clobbers the customer's manual edits + // (CS-437 override); seed an absent/empty draft with the derived narrative. + const document = await tx.ismsDocument.findUnique({ + where: { id: documentId }, + select: { draftNarrative: true }, }); - if (latest) { - // Preserve a non-empty narrative so a regenerate never clobbers the - // customer's manual edits (CS-437 override). An absent or empty ({}) value — - // e.g. a snapshot-only version — is still seeded with the derived narrative. - if (hasNarrativeContent(latest.narrative)) return; - await tx.ismsDocumentVersion.update({ - where: { id: latest.id }, - data: { narrative }, - }); - return; - } - await tx.ismsDocumentVersion.create({ - data: { documentId, version: 1, isLatest: true, narrative }, + if (document && hasNarrativeContent(document.draftNarrative)) return; + await tx.ismsDocument.update({ + where: { id: documentId }, + data: { draftNarrative: narrative }, }); } diff --git a/apps/api/src/isms/documents/org-profile.ts b/apps/api/src/isms/documents/org-profile.ts index be77f5ccdb..446ac9feaf 100644 --- a/apps/api/src/isms/documents/org-profile.ts +++ b/apps/api/src/isms/documents/org-profile.ts @@ -1,4 +1,5 @@ import { db } from '@db'; +import type { Prisma } from '@db'; import { DEFAULT_INTENDED_OUTCOMES } from '../wizard/wizard-defaults'; import { parseStoredAnswers } from '../wizard/wizard-schema'; import type { IsmsKeyValue } from '../utils/export-shared'; @@ -29,23 +30,30 @@ const QUESTIONS = { export async function loadOrgProfile({ organizationId, frameworkId, + client = db, }: { organizationId: string; frameworkId: string; + /** + * DB client to read through. Defaults to the global `db`; the approve flow + * passes its transaction client so the snapshot's org profile reads share the + * same isolation context as the register rows written in the same transaction. + */ + client?: Prisma.TransactionClient; }): Promise { const [organization, contextEntries, profile] = await Promise.all([ - db.organization.findUnique({ + client.organization.findUnique({ where: { id: organizationId }, select: { name: true, website: true }, }), - db.context.findMany({ + client.context.findMany({ where: { organizationId }, // Deterministic ordering so duplicate questions resolve to the same answer // every export; the earliest-created entry wins (id breaks createdAt ties). orderBy: [{ createdAt: 'asc' }, { id: 'asc' }], select: { question: true, answer: true }, }), - db.ismsProfile.findUnique({ + client.ismsProfile.findUnique({ where: { organizationId_frameworkId: { organizationId, frameworkId } }, select: { answers: true }, }), diff --git a/apps/api/src/isms/dto/export-isms-document.dto.ts b/apps/api/src/isms/dto/export-isms-document.dto.ts index 2891e57eff..632b1a2923 100644 --- a/apps/api/src/isms/dto/export-isms-document.dto.ts +++ b/apps/api/src/isms/dto/export-isms-document.dto.ts @@ -1,5 +1,5 @@ -import { ApiProperty } from '@nestjs/swagger'; -import { IsIn } from 'class-validator'; +import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger'; +import { IsIn, IsOptional, IsString } from 'class-validator'; export class ExportIsmsDocumentDto { @ApiProperty({ @@ -9,4 +9,13 @@ export class ExportIsmsDocumentDto { }) @IsIn(['pdf', 'docx']) format!: 'pdf' | 'docx'; + + @ApiPropertyOptional({ + description: + 'Published version to export. Omit to export the current working draft; ' + + 'provide a version id to download exactly what was approved at that version.', + }) + @IsOptional() + @IsString() + versionId?: string; } diff --git a/apps/api/src/isms/isms-context.service.spec.ts b/apps/api/src/isms/isms-context.service.spec.ts index a91d741002..b7d46147c3 100644 --- a/apps/api/src/isms/isms-context.service.spec.ts +++ b/apps/api/src/isms/isms-context.service.spec.ts @@ -1,22 +1,20 @@ import { NotFoundException } from '@nestjs/common'; import { db } from '@db'; import { IsmsContextService } from './isms-context.service'; +import type { IsmsVersionService } from './isms-version.service'; import { collectPlatformData } from './documents/data-source'; import { runDerivation } from './documents/generate'; import { diffPlatformSnapshots, parsePlatformSnapshot, } from './documents/snapshot'; -import { buildExportSections } from './documents/registry'; -import { generateIsmsExportFile } from './utils/export-generator'; -import { upsertLatestSnapshotVersion } from './utils/version-snapshot'; +import { updateDraftSnapshot } from './utils/draft-snapshot'; +import { renderLiveExport } from './utils/export-payload'; +import { invalidateApprovalIfNeeded } from './utils/approval'; jest.mock('@db', () => ({ db: { ismsDocument: { findFirst: jest.fn() }, - organization: { findUnique: jest.fn() }, - context: { findMany: jest.fn() }, - ismsProfile: { findUnique: jest.fn() }, $transaction: jest.fn(), }, })); @@ -30,14 +28,14 @@ jest.mock('./documents/snapshot', () => ({ diffPlatformSnapshots: jest.fn(), parsePlatformSnapshot: jest.fn(), })); -jest.mock('./documents/registry', () => ({ - buildExportSections: jest.fn(), +jest.mock('./utils/draft-snapshot', () => ({ + updateDraftSnapshot: jest.fn(), })); -jest.mock('./utils/export-generator', () => ({ - generateIsmsExportFile: jest.fn(), +jest.mock('./utils/export-payload', () => ({ + renderLiveExport: jest.fn(), })); -jest.mock('./utils/version-snapshot', () => ({ - upsertLatestSnapshotVersion: jest.fn(), +jest.mock('./utils/approval', () => ({ + invalidateApprovalIfNeeded: jest.fn(), })); const mockDb = jest.mocked(db); @@ -45,8 +43,9 @@ const mockCollect = jest.mocked(collectPlatformData); const mockRun = jest.mocked(runDerivation); const mockDiff = jest.mocked(diffPlatformSnapshots); const mockParse = jest.mocked(parsePlatformSnapshot); -const mockBuild = jest.mocked(buildExportSections); -const mockExport = jest.mocked(generateIsmsExportFile); +const mockUpdateDraft = jest.mocked(updateDraftSnapshot); +const mockRenderLive = jest.mocked(renderLiveExport); +const mockInvalidate = jest.mocked(invalidateApprovalIfNeeded); const snapshot = { organizationName: 'Acme', @@ -66,12 +65,17 @@ const snapshot = { partiesFingerprint: '', }; +const versionService = { + getVersionExport: jest.fn(), + getVersions: jest.fn(), +} as unknown as IsmsVersionService; + describe('IsmsContextService', () => { let service: IsmsContextService; beforeEach(() => { jest.clearAllMocks(); - service = new IsmsContextService(); + service = new IsmsContextService(versionService); }); describe('generate', () => { @@ -89,30 +93,37 @@ describe('IsmsContextService', () => { 'objectives_plan', 'isms_scope', 'leadership_commitment', - ])('dispatches derivation + snapshot for type %s', async (type) => { - (mockDb.ismsDocument.findFirst as jest.Mock) - .mockResolvedValueOnce({ id: 'doc_1', type, frameworkId: 'fw_1' }) - .mockResolvedValueOnce({ id: 'doc_1' }); - mockCollect.mockResolvedValue(snapshot); - const tx = {}; - (mockDb.$transaction as jest.Mock).mockImplementation((cb) => cb(tx)); + ])( + 'derives + updates the draft snapshot for type %s', + async (type) => { + (mockDb.ismsDocument.findFirst as jest.Mock) + .mockResolvedValueOnce({ id: 'doc_1', type, frameworkId: 'fw_1' }) + .mockResolvedValueOnce({ id: 'doc_1' }); + mockCollect.mockResolvedValue(snapshot); + const tx = {}; + (mockDb.$transaction as jest.Mock).mockImplementation((cb) => cb(tx)); - await service.generate(args); + await service.generate(args); - expect(mockRun).toHaveBeenCalledWith({ - tx, - type, - documentId: 'doc_1', - organizationId: 'org_1', - frameworkId: 'fw_1', - data: snapshot, - }); - expect(upsertLatestSnapshotVersion).toHaveBeenCalledWith({ - tx, - documentId: 'doc_1', - snapshot, - }); - }); + expect(mockRun).toHaveBeenCalledWith({ + tx, + type, + documentId: 'doc_1', + organizationId: 'org_1', + frameworkId: 'fw_1', + data: snapshot, + }); + // CS-701: the drift baseline is the document's draftSnapshot, not a + // version row. + expect(mockUpdateDraft).toHaveBeenCalledWith({ + tx, + documentId: 'doc_1', + snapshot, + }); + // Regenerating an approved document must revert it to draft. + expect(mockInvalidate).toHaveBeenCalledWith({ tx, documentId: 'doc_1' }); + }, + ); it('reuses pre-collected data and skips collectPlatformData', async () => { (mockDb.ismsDocument.findFirst as jest.Mock) @@ -137,7 +148,7 @@ describe('IsmsContextService', () => { frameworkId: 'fw_1', data: precollected, }); - expect(upsertLatestSnapshotVersion).toHaveBeenCalledWith({ + expect(mockUpdateDraft).toHaveBeenCalledWith({ tx, documentId: 'doc_1', snapshot: precollected, @@ -153,12 +164,12 @@ describe('IsmsContextService', () => { await expect(service.drift(args)).rejects.toThrow(NotFoundException); }); - it('compares current data against the parsed snapshot by type', async () => { + it('compares current data against the parsed draftSnapshot by type', async () => { (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ id: 'doc_1', type: 'objectives_plan', frameworkId: 'fw_1', - versions: [{ sourceSnapshot: snapshot }], + draftSnapshot: snapshot, }); mockCollect.mockResolvedValue({ ...snapshot, riskCount: 9 }); mockParse.mockReturnValue(snapshot); @@ -175,12 +186,12 @@ describe('IsmsContextService', () => { expect(result).toEqual({ isStale: true, changedSources: ['risks'] }); }); - it('treats a missing snapshot as no baseline', async () => { + it('treats a missing draftSnapshot as no baseline', async () => { (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ id: 'doc_1', type: 'context_of_organization', frameworkId: 'fw_1', - versions: [], + draftSnapshot: null, }); mockCollect.mockResolvedValue(snapshot); mockParse.mockReturnValue(null); @@ -200,7 +211,60 @@ describe('IsmsContextService', () => { }); describe('exportDocument', () => { - it('throws NotFoundException when document missing', async () => { + const result = { + fileBuffer: Buffer.from('bytes'), + mimeType: 'application/pdf', + filename: 'doc.pdf', + }; + + it('renders the live draft only when the document has no published version', async () => { + (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ + currentVersionId: null, + }); + mockRenderLive.mockResolvedValue(result); + + const out = await service.exportDocument({ + documentId: 'doc_1', + organizationId: 'org_1', + dto: { format: 'pdf' }, + }); + + expect(mockRenderLive).toHaveBeenCalledWith({ + documentId: 'doc_1', + organizationId: 'org_1', + format: 'pdf', + }); + expect(versionService.getVersionExport).not.toHaveBeenCalled(); + expect(out).toBe(result); + }); + + it.each(['approved', 'draft', 'needs_review'])( + 'serves the published version by default whenever one exists (status %s)', + async (status) => { + (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ + status, + currentVersionId: 'isms_ver_9', + }); + (versionService.getVersionExport as jest.Mock).mockResolvedValue(result); + + const out = await service.exportDocument({ + documentId: 'doc_1', + organizationId: 'org_1', + dto: { format: 'pdf' }, + }); + + expect(versionService.getVersionExport).toHaveBeenCalledWith({ + documentId: 'doc_1', + organizationId: 'org_1', + versionId: 'isms_ver_9', + format: 'pdf', + }); + expect(mockRenderLive).not.toHaveBeenCalled(); + expect(out).toBe(result); + }, + ); + + it('throws NotFoundException when the document is missing', async () => { (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue(null); await expect( service.exportDocument({ @@ -211,98 +275,23 @@ describe('IsmsContextService', () => { ).rejects.toThrow(NotFoundException); }); - const buildDocument = (type: string) => ({ - id: 'doc_1', - type, - title: 'Doc', - status: 'approved', - preparedBy: 'Comp AI', - approvedAt: null, - declinedAt: null, - framework: { name: 'ISO 27001' }, - organization: { name: 'Acme', primaryColor: '#004D3D' }, - approver: { user: { name: 'Jane', email: 'jane@acme.io' } }, - contextIssues: [ - { - kind: 'external', - category: 'Regulatory & Legal', - description: 'd', - effect: 'e', - }, - ], - interestedParties: [ - { name: 'Customers', category: 'Customer', needsExpectations: 'n' }, - ], - interestedPartyRequirements: [ - { partyName: 'Customers', requirement: 'r', treatment: 't' }, - ], - objectives: [ - { - objective: 'o', - target: 't', - cadence: 'Annual', - status: 'on_track', - plan: 'p', - measurementMethod: 'm', - }, - ], - versions: [ - { version: 2, narrative: { statement: 's', commitments: [] } }, - ], - }); - - it.each([ - ['context_of_organization', 'pdf'], - ['interested_parties_register', 'pdf'], - ['interested_parties_requirements', 'pdf'], - ['objectives_plan', 'pdf'], - ['isms_scope', 'docx'], - ['leadership_commitment', 'docx'], - ] as const)( - 'dispatches export sections for %s (%s)', - async (type, format) => { - (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue( - buildDocument(type), - ); - // The Context document loads the org profile (org + Context Q&A + - // ISMS wizard answers); other types skip it. Stub all three reads. - (mockDb.organization.findUnique as jest.Mock).mockResolvedValue({ - name: 'Acme', - website: 'https://acme.io', - }); - (mockDb.context.findMany as jest.Mock).mockResolvedValue([]); - (mockDb.ismsProfile.findUnique as jest.Mock).mockResolvedValue({ - answers: {}, - }); - mockBuild.mockReturnValue([{ heading: 'H' }]); - mockExport.mockResolvedValue({ - fileBuffer: Buffer.from('bytes'), - mimeType: format === 'pdf' ? 'application/pdf' : 'docx-mime', - filename: `doc.${format}`, - }); + it('routes to the version service when a versionId is given', async () => { + (versionService.getVersionExport as jest.Mock).mockResolvedValue(result); - const result = await service.exportDocument({ - documentId: 'doc_1', - organizationId: 'org_1', - dto: { format }, - }); + const out = await service.exportDocument({ + documentId: 'doc_1', + organizationId: 'org_1', + dto: { format: 'docx', versionId: 'isms_ver_1' }, + }); - expect(mockBuild).toHaveBeenCalledWith( - expect.objectContaining({ type }), - ); - expect(mockExport).toHaveBeenCalledWith( - expect.objectContaining({ - format, - sections: [{ heading: 'H' }], - metadata: expect.objectContaining({ - title: 'Doc', - organizationName: 'Acme', - primaryColor: '#004D3D', - }), - }), - ); - expect(result.fileBuffer).toBeInstanceOf(Buffer); - }, - ); + expect(versionService.getVersionExport).toHaveBeenCalledWith({ + documentId: 'doc_1', + organizationId: 'org_1', + versionId: 'isms_ver_1', + format: 'docx', + }); + expect(mockRenderLive).not.toHaveBeenCalled(); + expect(out).toBe(result); + }); }); }); diff --git a/apps/api/src/isms/isms-context.service.ts b/apps/api/src/isms/isms-context.service.ts index ec3e995ab0..cae5156c06 100644 --- a/apps/api/src/isms/isms-context.service.ts +++ b/apps/api/src/isms/isms-context.service.ts @@ -3,36 +3,42 @@ import { db } from '@db'; import { ExportIsmsDocumentDto } from './dto/export-isms-document.dto'; import { collectPlatformData } from './documents/data-source'; import { runDerivation } from './documents/generate'; -import { loadOrgProfile } from './documents/org-profile'; -import { buildExportSections } from './documents/registry'; import { diffPlatformSnapshots, parsePlatformSnapshot, } from './documents/snapshot'; -import type { DocumentExportInput, IsmsPlatformData } from './documents/types'; -import { - generateIsmsExportFile, - type IsmsExportResult, -} from './utils/export-generator'; -import { buildExportMetadata } from './utils/export-metadata'; -import { upsertLatestSnapshotVersion } from './utils/version-snapshot'; +import type { IsmsPlatformData } from './documents/types'; +import { invalidateApprovalIfNeeded } from './utils/approval'; +import { updateDraftSnapshot } from './utils/draft-snapshot'; +import { renderLiveExport } from './utils/export-payload'; +import type { IsmsExportResult } from './utils/export-generator'; +import { IsmsVersionService } from './isms-version.service'; const DOCUMENT_INCLUDE = { - versions: { where: { isLatest: true }, take: 1 }, + currentVersion: { select: { id: true, version: true, publishedAt: true } }, contextIssues: { orderBy: { position: 'asc' } }, interestedParties: { orderBy: { position: 'asc' } }, interestedPartyRequirements: { orderBy: { position: 'asc' } }, objectives: { orderBy: { position: 'asc' } }, + controlLinks: { + select: { + id: true, + controlId: true, + control: { select: { id: true, name: true } }, + }, + }, } as const; /** * ISMS document derivation, drift detection and export. Dispatches by document * type to the per-document handlers under ./documents. Document lifecycle - * (approve/decline/submit) lives in IsmsService; register CRUD in the register - * services. + * (approve/decline/submit) lives in IsmsService, version history + per-version + * export in IsmsVersionService, and register CRUD in the register services. */ @Injectable() export class IsmsContextService { + constructor(private readonly versionService: IsmsVersionService) {} + async generate({ documentId, organizationId, @@ -62,6 +68,10 @@ export class IsmsContextService { })); await db.$transaction(async (tx) => { + // Regenerating changes the working draft, so an approved document must + // revert to draft and be re-approved — matching every other edit path + // (register CRUD, narrative save). Never silently mutate an approved doc. + await invalidateApprovalIfNeeded({ tx, documentId }); await runDerivation({ tx, type: document.type, @@ -70,7 +80,7 @@ export class IsmsContextService { frameworkId: document.frameworkId, data, }); - await upsertLatestSnapshotVersion({ tx, documentId, snapshot: data }); + await updateDraftSnapshot({ tx, documentId, snapshot: data }); }); return this.loadDocument({ documentId, organizationId }); @@ -85,7 +95,7 @@ export class IsmsContextService { }): Promise<{ isStale: boolean; changedSources: string[] }> { const document = await db.ismsDocument.findFirst({ where: { id: documentId, organizationId }, - include: { versions: { where: { isLatest: true }, take: 1 } }, + select: { type: true, frameworkId: true, draftSnapshot: true }, }); if (!document) { throw new NotFoundException('ISMS document not found'); @@ -95,13 +105,18 @@ export class IsmsContextService { organizationId, frameworkId: document.frameworkId, }); - const previous = parsePlatformSnapshot( - document.versions[0]?.sourceSnapshot, - ); + const previous = parsePlatformSnapshot(document.draftSnapshot); return diffPlatformSnapshots({ type: document.type, previous, current }); } + /** + * Export a document. + * - With `versionId`: serve that published version (stored file or snapshot). + * - Without: serve the current PUBLISHED version whenever one exists — it stays + * live and exportable while a new draft is in progress (CS-701). Only render + * the live draft when the document has never been published yet. + */ async exportDocument({ documentId, organizationId, @@ -111,83 +126,35 @@ export class IsmsContextService { organizationId: string; dto: ExportIsmsDocumentDto; }): Promise { + if (dto.versionId) { + return this.versionService.getVersionExport({ + documentId, + organizationId, + versionId: dto.versionId, + format: dto.format, + }); + } + const document = await db.ismsDocument.findFirst({ where: { id: documentId, organizationId }, - include: { - framework: { select: { name: true } }, - organization: { - select: { name: true, website: true, primaryColor: true }, - }, - approver: { select: { user: { select: { name: true, email: true } } } }, - ...DOCUMENT_INCLUDE, - }, + select: { currentVersionId: true }, }); if (!document) { throw new NotFoundException('ISMS document not found'); } - // The Context document (clause 4.1) renders an org overview, mission and - // intended outcomes; other document types don't need the profile. - const orgProfile = - document.type === 'context_of_organization' - ? await loadOrgProfile({ - organizationId, - frameworkId: document.frameworkId, - }) - : undefined; - - const input: DocumentExportInput = { - contextIssues: document.contextIssues.map((issue) => ({ - kind: issue.kind, - category: issue.category, - description: issue.description, - effect: issue.effect, - })), - interestedParties: document.interestedParties.map((party) => ({ - name: party.name, - category: party.category, - needsExpectations: party.needsExpectations, - })), - requirements: document.interestedPartyRequirements.map((row) => ({ - partyName: row.partyName, - requirement: row.requirement, - treatment: row.treatment, - })), - objectives: document.objectives.map((objective) => ({ - objective: objective.objective, - target: objective.target, - cadence: objective.cadence, - status: objective.status, - plan: objective.plan, - measurementMethod: objective.measurementMethod, - })), - narrative: document.versions[0]?.narrative ?? null, - orgProfile, - }; - - const sections = buildExportSections({ type: document.type, input }); + // A published version is the document's official artifact and remains the + // default export even while an approved document is being re-drafted. + if (document.currentVersionId) { + return this.versionService.getVersionExport({ + documentId, + organizationId, + versionId: document.currentVersionId, + format: dto.format, + }); + } - return generateIsmsExportFile({ - sections, - format: dto.format, - metadata: buildExportMetadata({ - type: document.type, - title: document.title, - frameworkName: document.framework.name || 'ISO 27001', - version: document.versions[0]?.version ?? 1, - status: document.status, - preparedBy: document.preparedBy, - owner: null, - approverName: - document.approver?.user?.name || - document.approver?.user?.email || - null, - approvedAt: document.approvedAt, - declinedAt: document.declinedAt, - organizationName: document.organization.name, - primaryColor: document.organization.primaryColor, - }), - }); + return renderLiveExport({ documentId, organizationId, format: dto.format }); } private async loadDocument({ diff --git a/apps/api/src/isms/isms-narrative.service.spec.ts b/apps/api/src/isms/isms-narrative.service.spec.ts index a1e6db166e..2770acd489 100644 --- a/apps/api/src/isms/isms-narrative.service.spec.ts +++ b/apps/api/src/isms/isms-narrative.service.spec.ts @@ -9,11 +9,6 @@ jest.mock('@db', () => { findUnique: jest.fn(), update: jest.fn(), }, - ismsDocumentVersion: { - findFirst: jest.fn(), - create: jest.fn(), - update: jest.fn(), - }, // Run the callback with the same mock as the transaction client. $transaction: jest.fn((cb: (tx: unknown) => unknown) => cb(db)), }; @@ -36,6 +31,9 @@ describe('IsmsNarrativeService', () => { beforeEach(() => { jest.clearAllMocks(); service = new IsmsNarrativeService(); + (mockDb.ismsDocument.update as jest.Mock).mockResolvedValue({ + id: 'doc_1', + }); }); it('throws NotFoundException when document missing', async () => { @@ -77,14 +75,14 @@ describe('IsmsNarrativeService', () => { ).rejects.toThrow(BadRequestException); }); - it('creates a version when none exists', async () => { + it('writes the validated narrative onto the document draft (CS-701)', async () => { (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ id: 'doc_1', type: 'isms_scope', }); - (mockDb.ismsDocumentVersion.findFirst as jest.Mock).mockResolvedValue(null); - (mockDb.ismsDocumentVersion.create as jest.Mock).mockResolvedValue({ - id: 'ver_1', + // Non-approved: invalidateApprovalIfNeeded is a no-op. + (mockDb.ismsDocument.findUnique as jest.Mock).mockResolvedValue({ + status: 'draft', }); await service.save({ @@ -93,38 +91,20 @@ describe('IsmsNarrativeService', () => { narrative: validScope, }); - expect(mockDb.ismsDocumentVersion.create).toHaveBeenCalledWith( - expect.objectContaining({ - data: expect.objectContaining({ documentId: 'doc_1', version: 1 }), - }), - ); - }); - - it('updates the latest version when present', async () => { - (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ - id: 'doc_1', - type: 'isms_scope', - }); - (mockDb.ismsDocumentVersion.findFirst as jest.Mock).mockResolvedValue({ - id: 'ver_1', - }); - (mockDb.ismsDocumentVersion.update as jest.Mock).mockResolvedValue({ - id: 'ver_1', - }); - - await service.save({ - documentId: 'doc_1', - organizationId: 'org_1', - narrative: validScope, + // The draft narrative lives on IsmsDocument, never on a version row. + expect(mockDb.ismsDocument.update).toHaveBeenCalledTimes(1); + expect(mockDb.ismsDocument.update).toHaveBeenCalledWith({ + where: { id: 'doc_1' }, + data: { + draftNarrative: expect.objectContaining({ + certificateScopeSentence: 'The ISMS covers Acme.', + }), + }, + select: { id: true, draftNarrative: true }, }); - - expect(mockDb.ismsDocumentVersion.update).toHaveBeenCalledWith( - expect.objectContaining({ where: { id: 'ver_1' } }), - ); - expect(mockDb.ismsDocument.update).not.toHaveBeenCalled(); }); - it('reverts an approved document to draft so it needs re-approval', async () => { + it('reverts an approved document to draft, then writes the narrative', async () => { (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ id: 'doc_1', type: 'isms_scope', @@ -132,12 +112,6 @@ describe('IsmsNarrativeService', () => { (mockDb.ismsDocument.findUnique as jest.Mock).mockResolvedValue({ status: 'approved', }); - (mockDb.ismsDocumentVersion.findFirst as jest.Mock).mockResolvedValue({ - id: 'ver_1', - }); - (mockDb.ismsDocumentVersion.update as jest.Mock).mockResolvedValue({ - id: 'ver_1', - }); await service.save({ documentId: 'doc_1', @@ -145,9 +119,19 @@ describe('IsmsNarrativeService', () => { narrative: validScope, }); - expect(mockDb.ismsDocument.update).toHaveBeenCalledWith({ + // First the approval invalidation, then the narrative write — both in-tx. + expect(mockDb.ismsDocument.update).toHaveBeenNthCalledWith(1, { where: { id: 'doc_1' }, data: { status: 'draft', approvedAt: null, approverId: null }, }); + expect(mockDb.ismsDocument.update).toHaveBeenNthCalledWith(2, { + where: { id: 'doc_1' }, + data: { + draftNarrative: expect.objectContaining({ + certificateScopeSentence: 'The ISMS covers Acme.', + }), + }, + select: { id: true, draftNarrative: true }, + }); }); }); diff --git a/apps/api/src/isms/isms-narrative.service.ts b/apps/api/src/isms/isms-narrative.service.ts index fd06a3ceea..6194395abd 100644 --- a/apps/api/src/isms/isms-narrative.service.ts +++ b/apps/api/src/isms/isms-narrative.service.ts @@ -10,8 +10,10 @@ import { invalidateApprovalIfNeeded } from './utils/approval'; /** * Saves the singleton-document narrative (clauses 4.3 ISMS Scope and 5.1 - * Leadership Commitment) into the document's latest version. The payload is - * validated against the per-type Zod schema before persisting. + * Leadership Commitment) onto the document's working DRAFT (`draftNarrative`). + * The payload is validated against the per-type Zod schema before persisting. + * CS-701: the draft narrative lives on IsmsDocument, not the version row, so an + * edit never touches a published version. */ @Injectable() export class IsmsNarrativeService { @@ -51,27 +53,16 @@ export class IsmsNarrativeService { JSON.stringify(parsed.data), ); - // The latest-version read, approval invalidation, and the narrative write - // must all be atomic: reading the latest version outside the transaction - // lets a concurrent save update a stale version or hit the unique - // constraint, and a failed write must not leave the document reverted to - // draft without the new content. + // Approval invalidation and the narrative write must be atomic: a failed + // write must not leave the document reverted to draft without the new + // content. Reverting to draft leaves the published `currentVersion` intact — + // v1 stays live while the draft is edited. return db.$transaction(async (tx) => { - const latest = await tx.ismsDocumentVersion.findFirst({ - where: { documentId, isLatest: true }, - }); - await invalidateApprovalIfNeeded({ tx, documentId }); - - if (latest) { - return tx.ismsDocumentVersion.update({ - where: { id: latest.id }, - data: { narrative: value }, - }); - } - - return tx.ismsDocumentVersion.create({ - data: { documentId, version: 1, isLatest: true, narrative: value }, + return tx.ismsDocument.update({ + where: { id: documentId }, + data: { draftNarrative: value }, + select: { id: true, draftNarrative: true }, }); }); } diff --git a/apps/api/src/isms/isms-version.service.export.spec.ts b/apps/api/src/isms/isms-version.service.export.spec.ts new file mode 100644 index 0000000000..66c44e448b --- /dev/null +++ b/apps/api/src/isms/isms-version.service.export.spec.ts @@ -0,0 +1,196 @@ +import { Logger, NotFoundException } from '@nestjs/common'; +import { db } from '@db'; +import type { AttachmentsService } from '../attachments/attachments.service'; +import { IsmsVersionService } from './isms-version.service'; +import type { IsmsExportSnapshot } from './utils/export-payload'; +import { parseExportSnapshot, renderSnapshot } from './utils/export-payload'; + +jest.mock('@db', () => ({ + db: { + ismsDocumentVersion: { + findFirst: jest.fn(), + update: jest.fn(), + }, + }, +})); +jest.mock('./utils/export-payload', () => ({ + buildExportInput: jest.fn(() => ({ rows: [] })), + parseExportSnapshot: jest.fn(), + renderSnapshot: jest.fn(), + resolveOrgProfile: jest.fn(), +})); +jest.mock('./utils/export-metadata', () => ({ + buildExportMetadata: jest.fn(() => ({ version: 0 })), +})); + +const mockDb = jest.mocked(db); +const mockRenderSnapshot = jest.mocked(renderSnapshot); +const mockParse = jest.mocked(parseExportSnapshot); + +describe('IsmsVersionService export/render', () => { + let service: IsmsVersionService; + const attachments = { + getObjectBuffer: jest.fn(), + uploadBuffer: jest.fn(), + } as unknown as AttachmentsService; + + beforeEach(() => { + jest.clearAllMocks(); + jest.spyOn(Logger.prototype, 'error').mockImplementation(() => undefined); + jest.spyOn(Logger.prototype, 'warn').mockImplementation(() => undefined); + service = new IsmsVersionService(attachments); + }); + + describe('getVersionExport', () => { + const args = { + documentId: 'doc_1', + organizationId: 'org_1', + versionId: 'isms_ver_1', + format: 'pdf' as const, + }; + + it('throws NotFoundException when the version is missing', async () => { + (mockDb.ismsDocumentVersion.findFirst as jest.Mock).mockResolvedValue( + null, + ); + await expect(service.getVersionExport(args)).rejects.toThrow( + NotFoundException, + ); + }); + + it('serves the stored file from S3 when a rendered key exists', async () => { + (mockDb.ismsDocumentVersion.findFirst as jest.Mock).mockResolvedValue({ + version: 3, + pdfUrl: 'org/isms/doc/v3.pdf', + docxUrl: null, + contentSnapshot: {}, + document: { title: 'Context Doc' }, + }); + const buffer = Buffer.from('stored-pdf'); + (attachments.getObjectBuffer as jest.Mock).mockResolvedValue(buffer); + + const result = await service.getVersionExport(args); + + expect(attachments.getObjectBuffer).toHaveBeenCalledWith( + 'org/isms/doc/v3.pdf', + ); + expect(result).toEqual({ + fileBuffer: buffer, + mimeType: 'application/pdf', + filename: 'context-doc-v3.pdf', + }); + expect(mockRenderSnapshot).not.toHaveBeenCalled(); + }); + + it('re-renders from the content snapshot when no stored file exists', async () => { + (mockDb.ismsDocumentVersion.findFirst as jest.Mock).mockResolvedValue({ + version: 2, + pdfUrl: null, + docxUrl: null, + contentSnapshot: { type: 'x' }, + document: { title: 'Doc' }, + }); + const snapshot = { type: 'x' } as unknown as IsmsExportSnapshot; + mockParse.mockReturnValue(snapshot); + const rendered = { + fileBuffer: Buffer.from('re'), + mimeType: 'application/pdf', + filename: 'doc-v2.pdf', + }; + mockRenderSnapshot.mockResolvedValue(rendered); + + const result = await service.getVersionExport(args); + + expect(attachments.getObjectBuffer).not.toHaveBeenCalled(); + expect(mockRenderSnapshot).toHaveBeenCalledWith(snapshot, 'pdf'); + expect(result).toBe(rendered); + }); + + it('falls back to the snapshot when the stored S3 object is unavailable', async () => { + (mockDb.ismsDocumentVersion.findFirst as jest.Mock).mockResolvedValue({ + version: 3, + pdfUrl: 'org/isms/doc/v3.pdf', + docxUrl: null, + contentSnapshot: { type: 'x' }, + document: { title: 'Doc' }, + }); + (attachments.getObjectBuffer as jest.Mock).mockRejectedValue( + new Error('NoSuchKey'), + ); + const snapshot = { type: 'x' } as unknown as IsmsExportSnapshot; + mockParse.mockReturnValue(snapshot); + const rendered = { + fileBuffer: Buffer.from('re'), + mimeType: 'application/pdf', + filename: 'doc-v3.pdf', + }; + mockRenderSnapshot.mockResolvedValue(rendered); + + const result = await service.getVersionExport(args); + + expect(attachments.getObjectBuffer).toHaveBeenCalledWith( + 'org/isms/doc/v3.pdf', + ); + expect(mockRenderSnapshot).toHaveBeenCalledWith(snapshot, 'pdf'); + expect(result).toBe(rendered); + }); + + it('throws when a version has neither a stored file nor a snapshot (never serves live data)', async () => { + (mockDb.ismsDocumentVersion.findFirst as jest.Mock).mockResolvedValue({ + version: 1, + pdfUrl: null, + docxUrl: null, + contentSnapshot: null, + document: { title: 'Doc' }, + }); + mockParse.mockReturnValue(null); + + await expect(service.getVersionExport(args)).rejects.toThrow( + NotFoundException, + ); + expect(mockRenderSnapshot).not.toHaveBeenCalled(); + }); + }); + + describe('publishRenders', () => { + const args = { + organizationId: 'org_1', + documentId: 'doc_1', + versionId: 'isms_ver_1', + version: 2, + snapshot: {} as IsmsExportSnapshot, + }; + + it('uploads both formats and patches the version with the keys', async () => { + mockRenderSnapshot + .mockResolvedValueOnce({ + fileBuffer: Buffer.from('pdf'), + mimeType: 'application/pdf', + filename: 'a.pdf', + }) + .mockResolvedValueOnce({ + fileBuffer: Buffer.from('docx'), + mimeType: 'docx-mime', + filename: 'a.docx', + }); + + await service.publishRenders(args); + + expect(attachments.uploadBuffer).toHaveBeenCalledTimes(2); + const update = (mockDb.ismsDocumentVersion.update as jest.Mock).mock + .calls[0][0]; + expect(update.where).toEqual({ id: 'isms_ver_1' }); + expect(update.data.pdfUrl).toEqual(expect.stringContaining('.pdf')); + expect(update.data.docxUrl).toEqual(expect.stringContaining('.docx')); + }); + + it('swallows render/upload errors and never patches the version', async () => { + mockRenderSnapshot.mockRejectedValue(new Error('render boom')); + + await expect(service.publishRenders(args)).resolves.toBeUndefined(); + + expect(attachments.uploadBuffer).not.toHaveBeenCalled(); + expect(mockDb.ismsDocumentVersion.update).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/apps/api/src/isms/isms-version.service.spec.ts b/apps/api/src/isms/isms-version.service.spec.ts new file mode 100644 index 0000000000..ceff4d5764 --- /dev/null +++ b/apps/api/src/isms/isms-version.service.spec.ts @@ -0,0 +1,231 @@ +import { NotFoundException } from '@nestjs/common'; +import { db } from '@db'; +import type { Prisma } from '@db'; +import type { AttachmentsService } from '../attachments/attachments.service'; +import { IsmsVersionService } from './isms-version.service'; +import type { + IsmsExportSnapshot, + LoadedExportDocument, +} from './utils/export-payload'; +import { parseExportSnapshot } from './utils/export-payload'; + +// getVersionExport + publishRenders live in isms-version.service.export.spec.ts. +jest.mock('@db', () => ({ + db: { + ismsDocument: { findFirst: jest.fn() }, + ismsDocumentVersion: { + findFirst: jest.fn(), + findMany: jest.fn(), + updateMany: jest.fn(), + create: jest.fn(), + }, + }, +})); +jest.mock('./utils/export-payload', () => ({ + buildExportInput: jest.fn(() => ({ rows: [] })), + resolveOrgProfile: jest.fn(), + parseExportSnapshot: jest.fn(() => null), +})); +jest.mock('./utils/export-metadata', () => ({ + buildExportMetadata: jest.fn(() => ({ version: 0 })), +})); + +const mockDb = jest.mocked(db); +const mockParse = jest.mocked(parseExportSnapshot); + +function asTx(mock: unknown): Prisma.TransactionClient { + return mock as Prisma.TransactionClient; +} + +const buildDocument = ( + over: Partial = {}, +): LoadedExportDocument => + ({ + id: 'doc_1', + type: 'context_of_organization', + title: 'Context Doc', + preparedBy: 'Comp AI', + framework: { name: 'ISO 27001' }, + organization: { name: 'Acme', primaryColor: '#004D3D' }, + approver: { user: { name: 'Jane', email: 'jane@acme.io' } }, + draftNarrative: null, + ...over, + }) as unknown as LoadedExportDocument; + +describe('IsmsVersionService', () => { + let service: IsmsVersionService; + const attachments = { + getObjectBuffer: jest.fn(), + uploadBuffer: jest.fn(), + } as unknown as AttachmentsService; + + beforeEach(() => { + jest.clearAllMocks(); + service = new IsmsVersionService(attachments); + }); + + describe('createPublishedVersion', () => { + const buildTx = (lastVersion: number | null) => ({ + ismsDocumentVersion: { + findFirst: jest + .fn() + .mockResolvedValue( + lastVersion == null ? null : { version: lastVersion }, + ), + updateMany: jest.fn().mockResolvedValue({ count: 1 }), + create: jest.fn().mockResolvedValue({ id: 'isms_ver_new' }), + }, + }); + const now = new Date('2026-07-07T00:00:00.000Z'); + + it('computes the next version, demotes prior latest and creates the row', async () => { + const tx = buildTx(2); + + const result = await service.createPublishedVersion({ + tx: asTx(tx), + document: buildDocument({ draftNarrative: { statement: 's' } }), + memberId: 'mem_1', + now, + snapshotData: { src: true }, + }); + + // next = max(2) + 1 + expect(tx.ismsDocumentVersion.findFirst).toHaveBeenCalledWith( + expect.objectContaining({ orderBy: { version: 'desc' } }), + ); + // one-latest-per-document invariant: prior latest demoted first + expect(tx.ismsDocumentVersion.updateMany).toHaveBeenCalledWith({ + where: { documentId: 'doc_1', isLatest: true }, + data: { isLatest: false }, + }); + const createData = tx.ismsDocumentVersion.create.mock.calls[0][0].data; + expect(createData).toMatchObject({ + documentId: 'doc_1', + version: 3, + isLatest: true, + publishedById: 'mem_1', + publishedAt: now, + }); + expect(createData.narrative).toEqual({ statement: 's' }); + expect(createData.contentSnapshot).toBeDefined(); + expect(createData.sourceSnapshot).toEqual({ src: true }); + expect(result).toEqual({ + versionId: 'isms_ver_new', + version: 3, + snapshot: expect.objectContaining({ type: 'context_of_organization' }), + }); + }); + + it('starts at version 1 when the document has no prior versions', async () => { + const tx = buildTx(null); + + const result = await service.createPublishedVersion({ + tx: asTx(tx), + document: buildDocument(), + memberId: 'mem_1', + now, + snapshotData: {}, + }); + + expect(tx.ismsDocumentVersion.create.mock.calls[0][0].data.version).toBe( + 1, + ); + // Empty draft narrative falls back to {}. + expect(tx.ismsDocumentVersion.create.mock.calls[0][0].data.narrative).toEqual( + {}, + ); + expect(result.version).toBe(1); + }); + }); + + describe('getVersions', () => { + const args = { documentId: 'doc_1', organizationId: 'org_1' }; + + it('throws NotFoundException when the document is missing', async () => { + (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue(null); + await expect(service.getVersions(args)).rejects.toThrow(NotFoundException); + }); + + it('lists published versions, mapping name, flags and isCurrent', async () => { + (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ + currentVersionId: 'isms_ver_2', + }); + (mockDb.ismsDocumentVersion.findMany as jest.Mock).mockResolvedValue([ + { + id: 'isms_ver_2', + version: 2, + publishedAt: new Date('2026-07-07T00:00:00.000Z'), + changelog: 'second', + pdfUrl: 'k.pdf', + docxUrl: null, + publishedBy: { user: { name: 'Jane', email: 'jane@acme.io' } }, + }, + { + id: 'isms_ver_1', + version: 1, + publishedAt: new Date('2026-07-06T00:00:00.000Z'), + changelog: null, + pdfUrl: null, + docxUrl: null, + publishedBy: { user: { name: null, email: 'bob@acme.io' } }, + }, + ]); + + const result = await service.getVersions(args); + + // Only published rows are queried, newest first. + expect(mockDb.ismsDocumentVersion.findMany).toHaveBeenCalledWith( + expect.objectContaining({ + where: { documentId: 'doc_1', publishedAt: { not: null } }, + orderBy: { version: 'desc' }, + }), + ); + expect(result.currentVersionId).toBe('isms_ver_2'); + expect(result.versions[0]).toMatchObject({ + id: 'isms_ver_2', + version: 2, + publishedByName: 'Jane', + hasPdf: true, + hasDocx: false, + isCurrent: true, + }); + // Falls back to email when the name is null; not the current version. + expect(result.versions[1]).toMatchObject({ + publishedByName: 'bob@acme.io', + hasPdf: false, + isCurrent: false, + }); + }); + + it('keeps a version downloadable + attributed via the frozen snapshot when files and member are gone', async () => { + (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ + currentVersionId: 'isms_ver_1', + }); + (mockDb.ismsDocumentVersion.findMany as jest.Mock).mockResolvedValue([ + { + id: 'isms_ver_1', + version: 1, + publishedAt: new Date('2026-07-06T00:00:00.000Z'), + changelog: null, + pdfUrl: null, // stored files absent (e.g. render/upload failed at publish) + docxUrl: null, + contentSnapshot: { type: 'x' }, + publishedBy: null, // approving member later deleted + }, + ]); + mockParse.mockReturnValue({ + metadata: { approverName: 'Frozen Approver' }, + } as unknown as IsmsExportSnapshot); + + const result = await service.getVersions(args); + + // Downloadable via snapshot re-render even without stored files, and the + // approver survives via the frozen snapshot metadata. + expect(result.versions[0]).toMatchObject({ + publishedByName: 'Frozen Approver', + hasPdf: true, + hasDocx: true, + }); + }); + }); +}); diff --git a/apps/api/src/isms/isms-version.service.ts b/apps/api/src/isms/isms-version.service.ts new file mode 100644 index 0000000000..d54a0a9926 --- /dev/null +++ b/apps/api/src/isms/isms-version.service.ts @@ -0,0 +1,294 @@ +import { Injectable, Logger, NotFoundException } from '@nestjs/common'; +import { db } from '@db'; +import type { Prisma } from '@db'; +import { AttachmentsService } from '../attachments/attachments.service'; +import { buildExportMetadata } from './utils/export-metadata'; +import { + DOCX_MIME_TYPE, + type IsmsExportFormat, +} from './utils/export-shared'; +import { + sanitizeExportName, + type IsmsExportResult, +} from './utils/export-generator'; +import { + buildExportInput, + parseExportSnapshot, + renderSnapshot, + resolveOrgProfile, + type IsmsExportSnapshot, + type LoadedExportDocument, +} from './utils/export-payload'; + +/** True when a JSON value is a non-empty plain object (a saved narrative). */ +function isNarrativeObject(value: Prisma.JsonValue | null): boolean { + return ( + value != null && + typeof value === 'object' && + !Array.isArray(value) && + Object.keys(value).length > 0 + ); +} + +/** + * ISMS document version lifecycle (CS-701): freeze the working draft into an + * immutable published version at approval, list published-version history, and + * serve a specific version's export (stored file first, snapshot re-render as + * fallback). Document approval/decline live in IsmsService; live-draft export in + * IsmsContextService. + */ +@Injectable() +export class IsmsVersionService { + private readonly logger = new Logger(IsmsVersionService.name); + + constructor(private readonly attachments: AttachmentsService) {} + + /** + * Freeze the current draft into a new published version row inside `tx`. The + * caller sets `IsmsDocument.currentVersionId` to the returned id. Rendering + + * S3 upload happen AFTER the transaction via publishRenders, so a render/upload + * failure never orphans a DB row (mirrors the Policies publish flow). + */ + async createPublishedVersion({ + tx, + document, + memberId, + now, + snapshotData, + changelog, + }: { + tx: Prisma.TransactionClient; + document: LoadedExportDocument; + memberId: string; + now: Date; + snapshotData: unknown; + changelog?: string | null; + }): Promise<{ versionId: string; version: number; snapshot: IsmsExportSnapshot }> { + const nextVersion = await this.nextVersion(tx, document.id); + + // Read the org profile through the same transaction so the snapshot's profile + // and the register rows written in this transaction share one point in time. + const orgProfile = await resolveOrgProfile(document, tx); + const input = buildExportInput({ document, orgProfile }); + const metadata = buildExportMetadata({ + type: document.type, + title: document.title, + frameworkName: document.framework.name || 'ISO 27001', + version: nextVersion, + status: 'approved', + preparedBy: document.preparedBy, + owner: null, + approverName: + document.approver?.user?.name || + document.approver?.user?.email || + null, + approvedAt: now, + declinedAt: null, + organizationName: document.organization.name, + primaryColor: document.organization.primaryColor, + }); + const snapshot: IsmsExportSnapshot = { type: document.type, input, metadata }; + + // Keep the one-latest-per-document invariant (partial unique index). + await tx.ismsDocumentVersion.updateMany({ + where: { documentId: document.id, isLatest: true }, + data: { isLatest: false }, + }); + + const narrative: Prisma.InputJsonValue = isNarrativeObject( + document.draftNarrative, + ) + ? JSON.parse(JSON.stringify(document.draftNarrative)) + : {}; + + const created = await tx.ismsDocumentVersion.create({ + data: { + documentId: document.id, + version: nextVersion, + isLatest: true, + narrative, + contentSnapshot: JSON.parse(JSON.stringify(snapshot)), + sourceSnapshot: JSON.parse(JSON.stringify(snapshotData)), + publishedById: memberId, + publishedAt: now, + changelog: changelog ?? null, + }, + select: { id: true }, + }); + + return { versionId: created.id, version: nextVersion, snapshot }; + } + + /** + * Render both formats and upload them to S3, then store the keys on the version. + * Best-effort: a failure is logged, not thrown — historical export falls back to + * re-rendering from the version's contentSnapshot. + */ + async publishRenders({ + organizationId, + documentId, + versionId, + version, + snapshot, + }: { + organizationId: string; + documentId: string; + versionId: string; + version: number; + snapshot: IsmsExportSnapshot; + }): Promise { + try { + const [pdf, docx] = await Promise.all([ + renderSnapshot(snapshot, 'pdf'), + renderSnapshot(snapshot, 'docx'), + ]); + const base = `${organizationId}/isms/${documentId}/v${version}-${Date.now()}`; + const pdfKey = `${base}.pdf`; + const docxKey = `${base}.docx`; + await Promise.all([ + this.attachments.uploadBuffer(pdfKey, pdf.fileBuffer, pdf.mimeType), + this.attachments.uploadBuffer(docxKey, docx.fileBuffer, docx.mimeType), + ]); + await db.ismsDocumentVersion.update({ + where: { id: versionId }, + data: { pdfUrl: pdfKey, docxUrl: docxKey }, + }); + } catch (err) { + this.logger.error( + `ISMS version ${versionId} render/upload failed; export will fall back to on-demand render`, + err instanceof Error ? err.stack : String(err), + ); + } + } + + /** List a document's published versions (history), newest first. */ + async getVersions({ + documentId, + organizationId, + }: { + documentId: string; + organizationId: string; + }) { + const document = await db.ismsDocument.findFirst({ + where: { id: documentId, organizationId }, + select: { currentVersionId: true }, + }); + if (!document) { + throw new NotFoundException('ISMS document not found'); + } + + const versions = await db.ismsDocumentVersion.findMany({ + where: { documentId, publishedAt: { not: null } }, + orderBy: { version: 'desc' }, + select: { + id: true, + version: true, + publishedAt: true, + changelog: true, + pdfUrl: true, + docxUrl: true, + // The frozen snapshot backs both the approver-name fallback (survives an + // approver being deleted) and download availability (a version is still + // downloadable via snapshot re-render even if its stored file is absent). + contentSnapshot: true, + publishedBy: { select: { user: { select: { name: true, email: true } } } }, + }, + }); + + return { + currentVersionId: document.currentVersionId, + versions: versions.map((v) => { + const snapshot = parseExportSnapshot(v.contentSnapshot); + const hasSnapshot = snapshot != null; + return { + id: v.id, + version: v.version, + publishedAt: v.publishedAt, + changelog: v.changelog, + // Live member first; fall back to the approver name frozen at publish so + // history keeps an approver even if the member is later removed. + publishedByName: + v.publishedBy?.user?.name || + v.publishedBy?.user?.email || + snapshot?.metadata?.approverName || + null, + hasPdf: v.pdfUrl != null || hasSnapshot, + hasDocx: v.docxUrl != null || hasSnapshot, + isCurrent: v.id === document.currentVersionId, + }; + }), + }; + } + + /** + * Export a specific published version: serve its stored file (byte-identical to + * what was approved), else re-render from its snapshot, else — for a legacy + * migrated version with neither — render from current live data. + */ + async getVersionExport({ + documentId, + organizationId, + versionId, + format, + }: { + documentId: string; + organizationId: string; + versionId: string; + format: IsmsExportFormat; + }): Promise { + const version = await db.ismsDocumentVersion.findFirst({ + where: { id: versionId, documentId, document: { organizationId } }, + select: { + version: true, + pdfUrl: true, + docxUrl: true, + contentSnapshot: true, + document: { select: { title: true } }, + }, + }); + if (!version) { + throw new NotFoundException('ISMS document version not found'); + } + + const mimeType = format === 'pdf' ? 'application/pdf' : DOCX_MIME_TYPE; + const filename = `${sanitizeExportName(version.document.title)}-v${version.version}.${format}`; + + // Prefer the byte-identical stored artifact. If its S3 object is missing or + // temporarily unavailable, fall through to the immutable snapshot rather than + // failing the download. + const key = format === 'pdf' ? version.pdfUrl : version.docxUrl; + if (key) { + try { + const fileBuffer = await this.attachments.getObjectBuffer(key); + return { fileBuffer, mimeType, filename }; + } catch (err) { + this.logger.warn( + `Stored export ${key} unavailable for ISMS version ${versionId}; re-rendering from snapshot`, + err instanceof Error ? err.message : String(err), + ); + } + } + + const snapshot = parseExportSnapshot(version.contentSnapshot); + if (snapshot) return renderSnapshot(snapshot, format); + + // Neither a stored file nor a snapshot exists (only possible for a legacy / + // migrated version). Never serve current live data for a specific historical + // version — an auditor must get exactly what was approved, or a clear failure. + throw new NotFoundException( + 'No retained export exists for this document version', + ); + } + + private async nextVersion( + tx: Prisma.TransactionClient, + documentId: string, + ): Promise { + const last = await tx.ismsDocumentVersion.findFirst({ + where: { documentId }, + orderBy: { version: 'desc' }, + select: { version: true }, + }); + return (last?.version ?? 0) + 1; + } +} diff --git a/apps/api/src/isms/isms.controller.permissions.spec.ts b/apps/api/src/isms/isms.controller.permissions.spec.ts index 145432787d..7a9820255a 100644 --- a/apps/api/src/isms/isms.controller.permissions.spec.ts +++ b/apps/api/src/isms/isms.controller.permissions.spec.ts @@ -29,6 +29,9 @@ jest.mock('./isms.service', () => ({ jest.mock('./isms-context.service', () => ({ IsmsContextService: class MockIsmsContextService {}, })); +jest.mock('./isms-version.service', () => ({ + IsmsVersionService: class MockIsmsVersionService {}, +})); jest.mock('./isms-document-control.service', () => ({ IsmsDocumentControlService: class MockIsmsDocumentControlService {}, })); @@ -51,6 +54,9 @@ describe('IsmsController permission metadata', () => { expect(permissionsFor('drift')).toEqual([ { resource: 'evidence', actions: ['read'] }, ]); + expect(permissionsFor('getVersions')).toEqual([ + { resource: 'evidence', actions: ['read'] }, + ]); expect(permissionsFor('exportDocument')).toEqual([ { resource: 'evidence', actions: ['read'] }, ]); diff --git a/apps/api/src/isms/isms.controller.spec.ts b/apps/api/src/isms/isms.controller.spec.ts index fde926f23d..f01ce15224 100644 --- a/apps/api/src/isms/isms.controller.spec.ts +++ b/apps/api/src/isms/isms.controller.spec.ts @@ -8,6 +8,7 @@ import type { AuthContext as AuthContextType } from '../auth/types'; import { IsmsController } from './isms.controller'; import { IsmsService } from './isms.service'; import { IsmsContextService } from './isms-context.service'; +import { IsmsVersionService } from './isms-version.service'; import { IsmsDocumentControlService } from './isms-document-control.service'; const mockResolveRolePermissions = jest.mocked(resolveRolePermissions); @@ -54,6 +55,9 @@ jest.mock('./isms.service', () => ({ jest.mock('./isms-context.service', () => ({ IsmsContextService: class MockIsmsContextService {}, })); +jest.mock('./isms-version.service', () => ({ + IsmsVersionService: class MockIsmsVersionService {}, +})); jest.mock('./isms-document-control.service', () => ({ IsmsDocumentControlService: class MockIsmsDocumentControlService {}, })); @@ -73,6 +77,9 @@ describe('IsmsController', () => { drift: jest.fn(), exportDocument: jest.fn(), }; + const mockVersionService = { + getVersions: jest.fn(), + }; const mockDocumentControlService = { addControls: jest.fn(), removeControl: jest.fn(), @@ -86,6 +93,7 @@ describe('IsmsController', () => { providers: [ { provide: IsmsService, useValue: mockIsmsService }, { provide: IsmsContextService, useValue: mockContextService }, + { provide: IsmsVersionService, useValue: mockVersionService }, { provide: IsmsDocumentControlService, useValue: mockDocumentControlService, @@ -311,6 +319,20 @@ describe('IsmsController', () => { }); }); + it('getVersions delegates to the version service', async () => { + mockVersionService.getVersions.mockResolvedValue({ + currentVersionId: 'isms_ver_1', + versions: [], + }); + + await controller.getVersions('doc_1', 'org_1'); + + expect(mockVersionService.getVersions).toHaveBeenCalledWith({ + documentId: 'doc_1', + organizationId: 'org_1', + }); + }); + it('exportDocument sets headers and sends the buffer', async () => { const fileBuffer = Buffer.from('pdf-data'); mockContextService.exportDocument.mockResolvedValue({ diff --git a/apps/api/src/isms/isms.controller.ts b/apps/api/src/isms/isms.controller.ts index 0cdbbec885..00934ff575 100644 --- a/apps/api/src/isms/isms.controller.ts +++ b/apps/api/src/isms/isms.controller.ts @@ -32,6 +32,7 @@ import { resolveRolePermissions, permissionsGrant } from '../auth/app-access'; import { resolveServiceByName } from '../auth/service-token.config'; import { IsmsService } from './isms.service'; import { IsmsContextService } from './isms-context.service'; +import { IsmsVersionService } from './isms-version.service'; import { IsmsDocumentControlService } from './isms-document-control.service'; import { EnsureIsmsSetupDto } from './dto/ensure-isms-setup.dto'; import { SubmitIsmsForApprovalDto } from './dto/submit-isms-for-approval.dto'; @@ -46,6 +47,7 @@ export class IsmsController { constructor( private readonly ismsService: IsmsService, private readonly contextService: IsmsContextService, + private readonly versionService: IsmsVersionService, private readonly documentControlService: IsmsDocumentControlService, ) {} @@ -222,6 +224,19 @@ export class IsmsController { return this.contextService.drift({ documentId: id, organizationId }); } + @Get('documents/:id/versions') + @RequirePermission('evidence', 'read') + @ApiOperation({ + summary: "List an ISMS document's published version history", + }) + @ApiOkResponse({ description: 'Published versions, newest first' }) + async getVersions( + @Param('id') id: string, + @OrganizationId() organizationId: string, + ) { + return this.versionService.getVersions({ documentId: id, organizationId }); + } + @Post('documents/:id/export') @RequirePermission('evidence', 'read') @ApiOperation({ summary: 'Export an ISMS document as PDF or DOCX' }) diff --git a/apps/api/src/isms/isms.module.ts b/apps/api/src/isms/isms.module.ts index 88a134904a..9d894b737a 100644 --- a/apps/api/src/isms/isms.module.ts +++ b/apps/api/src/isms/isms.module.ts @@ -3,6 +3,7 @@ import { IsmsController } from './isms.controller'; import { IsmsRegistersController } from './isms-registers.controller'; import { IsmsService } from './isms.service'; import { IsmsContextService } from './isms-context.service'; +import { IsmsVersionService } from './isms-version.service'; import { IsmsContextIssueService } from './isms-context-issue.service'; import { IsmsDocumentControlService } from './isms-document-control.service'; import { IsmsInterestedPartyService } from './isms-interested-party.service'; @@ -12,9 +13,11 @@ import { IsmsNarrativeService } from './isms-narrative.service'; import { IsmsProfileController } from './wizard/isms-profile.controller'; import { IsmsProfileService } from './wizard/isms-profile.service'; import { AuthModule } from '../auth/auth.module'; +import { AttachmentsModule } from '../attachments/attachments.module'; @Module({ - imports: [AuthModule], + // AttachmentsModule: S3 access for retaining per-version rendered exports. + imports: [AuthModule, AttachmentsModule], controllers: [ IsmsController, IsmsRegistersController, @@ -23,6 +26,7 @@ import { AuthModule } from '../auth/auth.module'; providers: [ IsmsService, IsmsContextService, + IsmsVersionService, IsmsContextIssueService, IsmsDocumentControlService, IsmsInterestedPartyService, @@ -34,6 +38,7 @@ import { AuthModule } from '../auth/auth.module'; exports: [ IsmsService, IsmsContextService, + IsmsVersionService, IsmsContextIssueService, IsmsDocumentControlService, IsmsInterestedPartyService, diff --git a/apps/api/src/isms/isms.service.approve.spec.ts b/apps/api/src/isms/isms.service.approve.spec.ts new file mode 100644 index 0000000000..cdedf89f6e --- /dev/null +++ b/apps/api/src/isms/isms.service.approve.spec.ts @@ -0,0 +1,181 @@ +import { + BadRequestException, + ForbiddenException, + NotFoundException, +} from '@nestjs/common'; +import { db } from '@db'; +import { IsmsService } from './isms.service'; +import type { IsmsVersionService } from './isms-version.service'; +import { collectPlatformData } from './documents/data-source'; +import { runDerivation } from './documents/generate'; +import { updateDraftSnapshot } from './utils/draft-snapshot'; + +jest.mock('@db', () => ({ + db: { + ismsDocument: { findFirst: jest.fn() }, + member: { findFirst: jest.fn() }, + $transaction: jest.fn(), + }, +})); +jest.mock('./documents/data-source', () => ({ + collectPlatformData: jest.fn(), +})); +jest.mock('./documents/generate', () => ({ + runDerivation: jest.fn(), +})); +jest.mock('./utils/draft-snapshot', () => ({ + updateDraftSnapshot: jest.fn(), +})); + +const mockDb = jest.mocked(db); +const mockCollect = jest.mocked(collectPlatformData); +const mockRunDerivation = jest.mocked(runDerivation); +const mockUpdateDraft = jest.mocked(updateDraftSnapshot); + +const versionService = { + createPublishedVersion: jest.fn(), + publishRenders: jest.fn(), + getVersions: jest.fn(), + getVersionExport: jest.fn(), +} as unknown as IsmsVersionService; + +const platformData = { + organizationName: 'Acme', + frameworkNames: ['ISO 27001'], + vendorCount: 1, + subProcessorCount: 0, + vendorsByCategory: {}, + subProcessorNames: [], + infraVendorNames: [], + memberCount: 1, + membersByDepartment: {}, + deviceCount: 0, + riskCount: 0, + highRiskCount: 0, + hasTrainingProgram: false, + wizardAnswers: {}, + partiesFingerprint: '', +}; + +describe('IsmsService.approve (CS-701 versioning)', () => { + let service: IsmsService; + const args = { + documentId: 'doc_1', + organizationId: 'org_1', + userId: 'usr_1', + }; + + beforeEach(() => { + jest.clearAllMocks(); + service = new IsmsService(versionService); + }); + + it('throws NotFoundException when member not found', async () => { + (mockDb.member.findFirst as jest.Mock).mockResolvedValue(null); + await expect(service.approve(args)).rejects.toThrow(NotFoundException); + }); + + it('throws BadRequestException when document is not pending approval', async () => { + (mockDb.member.findFirst as jest.Mock).mockResolvedValue({ id: 'mem_1' }); + (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ + id: 'doc_1', + status: 'draft', + approverId: 'mem_1', + frameworkId: 'fw_1', + }); + await expect(service.approve(args)).rejects.toThrow(BadRequestException); + }); + + it('throws ForbiddenException when no approver is assigned', async () => { + (mockDb.member.findFirst as jest.Mock).mockResolvedValue({ id: 'mem_1' }); + (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ + id: 'doc_1', + status: 'needs_review', + approverId: null, + frameworkId: 'fw_1', + }); + await expect(service.approve(args)).rejects.toThrow(ForbiddenException); + }); + + it('throws ForbiddenException when not the assigned approver', async () => { + (mockDb.member.findFirst as jest.Mock).mockResolvedValue({ id: 'mem_1' }); + (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ + id: 'doc_1', + status: 'needs_review', + approverId: 'mem_other', + frameworkId: 'fw_1', + }); + await expect(service.approve(args)).rejects.toThrow(ForbiddenException); + }); + + it('freezes a published version, promotes it and marks approved', async () => { + (mockDb.member.findFirst as jest.Mock).mockResolvedValue({ id: 'mem_1' }); + const reloaded = { id: 'doc_1', type: 'context_of_organization' }; + (mockDb.ismsDocument.findFirst as jest.Mock) + .mockResolvedValueOnce({ + id: 'doc_1', + status: 'needs_review', + approverId: 'mem_1', + frameworkId: 'fw_1', + type: 'context_of_organization', + }) + .mockResolvedValueOnce({ id: 'doc_1', status: 'approved' }); + mockCollect.mockResolvedValue(platformData); + const tx = { + ismsDocument: { + findUniqueOrThrow: jest.fn().mockResolvedValue(reloaded), + update: jest.fn().mockResolvedValue({}), + }, + }; + (mockDb.$transaction as jest.Mock).mockImplementation((cb) => cb(tx)); + (versionService.createPublishedVersion as jest.Mock).mockResolvedValue({ + versionId: 'isms_ver_1', + version: 1, + snapshot: {}, + }); + (versionService.publishRenders as jest.Mock).mockResolvedValue(undefined); + + await service.approve(args); + + expect(mockCollect).toHaveBeenCalledWith({ + organizationId: 'org_1', + frameworkId: 'fw_1', + }); + // Re-derives in-tx from the same snapshot: persisted rows and the frozen + // version come from one pass. + expect(mockRunDerivation).toHaveBeenCalledWith({ + tx, + type: 'context_of_organization', + documentId: 'doc_1', + organizationId: 'org_1', + frameworkId: 'fw_1', + data: expect.objectContaining({ organizationName: 'Acme' }), + }); + // The drift baseline is refreshed on the document (CS-701). + expect(mockUpdateDraft).toHaveBeenCalledWith( + expect.objectContaining({ tx, documentId: 'doc_1' }), + ); + // A published version is created from the reloaded (re-derived) document. + expect(versionService.createPublishedVersion).toHaveBeenCalledWith( + expect.objectContaining({ tx, document: reloaded, memberId: 'mem_1' }), + ); + // The document is promoted to the freshly-created version. + expect(tx.ismsDocument.update).toHaveBeenCalledWith({ + where: { id: 'doc_1' }, + data: expect.objectContaining({ + status: 'approved', + declinedAt: null, + currentVersionId: 'isms_ver_1', + }), + }); + // Renders + uploads happen AFTER the transaction (Policies pattern). + expect(versionService.publishRenders).toHaveBeenCalledWith( + expect.objectContaining({ + organizationId: 'org_1', + documentId: 'doc_1', + versionId: 'isms_ver_1', + version: 1, + }), + ); + }); +}); diff --git a/apps/api/src/isms/isms.service.ensure-setup-fallback.spec.ts b/apps/api/src/isms/isms.service.ensure-setup-fallback.spec.ts index ce1dd0a85e..318f0adfe4 100644 --- a/apps/api/src/isms/isms.service.ensure-setup-fallback.spec.ts +++ b/apps/api/src/isms/isms.service.ensure-setup-fallback.spec.ts @@ -1,5 +1,6 @@ import { db } from '@db'; import { IsmsService } from './isms.service'; +import type { IsmsVersionService } from './isms-version.service'; jest.mock('@db', () => ({ db: { @@ -19,12 +20,12 @@ jest.mock('./documents/data-source', () => ({ jest.mock('./documents/generate', () => ({ runDerivation: jest.fn(), })); -jest.mock('./utils/version-snapshot', () => ({ - upsertLatestSnapshotVersion: jest.fn(), -})); const mockDb = jest.mocked(db); +// ensureSetup never touches the version service; a bare stub satisfies the ctor. +const versionService = {} as unknown as IsmsVersionService; + /** Convenience accessor for the first createMany call's `data` array. */ const createManyData = () => (mockDb.ismsDocument.createMany as jest.Mock).mock.calls[0][0].data; @@ -37,7 +38,7 @@ describe('IsmsService ensureSetup fallback to ISMS_TYPE_DEFINITIONS (no template beforeEach(() => { jest.clearAllMocks(); - service = new IsmsService(); + service = new IsmsService(versionService); (mockDb.control.findMany as jest.Mock).mockResolvedValue([]); (mockDb.ismsDocument.createMany as jest.Mock).mockResolvedValue({ count: 0, diff --git a/apps/api/src/isms/isms.service.lifecycle.spec.ts b/apps/api/src/isms/isms.service.lifecycle.spec.ts index 41ea994e3a..80e1311c5e 100644 --- a/apps/api/src/isms/isms.service.lifecycle.spec.ts +++ b/apps/api/src/isms/isms.service.lifecycle.spec.ts @@ -5,10 +5,10 @@ import { } from '@nestjs/common'; import { db } from '@db'; import { IsmsService } from './isms.service'; -import { collectPlatformData } from './documents/data-source'; -import { runDerivation } from './documents/generate'; -import { upsertLatestSnapshotVersion } from './utils/version-snapshot'; +import type { IsmsVersionService } from './isms-version.service'; +// approve() (the CS-701 freeze/version flow) is covered in +// isms.service.approve.spec.ts. jest.mock('@db', () => ({ db: { ismsDocument: { @@ -19,26 +19,22 @@ jest.mock('@db', () => ({ $transaction: jest.fn(), }, })); -jest.mock('./documents/data-source', () => ({ - collectPlatformData: jest.fn(), -})); -jest.mock('./documents/generate', () => ({ - runDerivation: jest.fn(), -})); -jest.mock('./utils/version-snapshot', () => ({ - upsertLatestSnapshotVersion: jest.fn(), -})); const mockDb = jest.mocked(db); -const mockCollect = jest.mocked(collectPlatformData); -const mockRunDerivation = jest.mocked(runDerivation); + +const versionService = { + createPublishedVersion: jest.fn(), + publishRenders: jest.fn(), + getVersions: jest.fn(), + getVersionExport: jest.fn(), +} as unknown as IsmsVersionService; describe('IsmsService document lifecycle', () => { let service: IsmsService; beforeEach(() => { jest.clearAllMocks(); - service = new IsmsService(); + service = new IsmsService(versionService); }); describe('getDocument', () => { @@ -65,7 +61,7 @@ describe('IsmsService document lifecycle', () => { ); }); - it('includes control links with the linked control id and name', async () => { + it('includes the current version and control links', async () => { (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ id: 'doc_1', controlLinks: [], @@ -80,6 +76,8 @@ describe('IsmsService document lifecycle', () => { controlId: true, control: { select: { id: true, name: true } }, }); + // CS-701: the document carries its live/published version for display. + expect(callArgs.include.currentVersion).toBeDefined(); }); }); @@ -121,111 +119,6 @@ describe('IsmsService document lifecycle', () => { }); }); - describe('approve', () => { - const args = { - documentId: 'doc_1', - organizationId: 'org_1', - userId: 'usr_1', - }; - - it('throws NotFoundException when member not found', async () => { - (mockDb.member.findFirst as jest.Mock).mockResolvedValue(null); - await expect(service.approve(args)).rejects.toThrow(NotFoundException); - }); - - it('throws BadRequestException when document is not pending approval', async () => { - (mockDb.member.findFirst as jest.Mock).mockResolvedValue({ id: 'mem_1' }); - (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ - id: 'doc_1', - status: 'draft', - approverId: 'mem_1', - frameworkId: 'fw_1', - }); - await expect(service.approve(args)).rejects.toThrow(BadRequestException); - }); - - it('throws ForbiddenException when no approver is assigned', async () => { - (mockDb.member.findFirst as jest.Mock).mockResolvedValue({ id: 'mem_1' }); - (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ - id: 'doc_1', - status: 'needs_review', - approverId: null, - frameworkId: 'fw_1', - }); - await expect(service.approve(args)).rejects.toThrow(ForbiddenException); - }); - - it('throws ForbiddenException when not the assigned approver', async () => { - (mockDb.member.findFirst as jest.Mock).mockResolvedValue({ id: 'mem_1' }); - (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ - id: 'doc_1', - status: 'needs_review', - approverId: 'mem_other', - frameworkId: 'fw_1', - }); - await expect(service.approve(args)).rejects.toThrow(ForbiddenException); - }); - - it('snapshots data and marks approved', async () => { - (mockDb.member.findFirst as jest.Mock).mockResolvedValue({ id: 'mem_1' }); - (mockDb.ismsDocument.findFirst as jest.Mock) - .mockResolvedValueOnce({ - id: 'doc_1', - status: 'needs_review', - approverId: 'mem_1', - frameworkId: 'fw_1', - type: 'context_of_organization', - }) - .mockResolvedValueOnce({ id: 'doc_1', status: 'approved' }); - mockCollect.mockResolvedValue({ - organizationName: 'Acme', - frameworkNames: ['ISO 27001'], - vendorCount: 1, - subProcessorCount: 0, - vendorsByCategory: {}, - subProcessorNames: [], - infraVendorNames: [], - memberCount: 1, - membersByDepartment: {}, - deviceCount: 0, - riskCount: 0, - highRiskCount: 0, - hasTrainingProgram: false, - wizardAnswers: {}, - partiesFingerprint: '', - }); - const tx = { - ismsDocument: { update: jest.fn().mockResolvedValue({}) }, - }; - (mockDb.$transaction as jest.Mock).mockImplementation((cb) => cb(tx)); - - await service.approve(args); - - expect(mockCollect).toHaveBeenCalledWith({ - organizationId: 'org_1', - frameworkId: 'fw_1', - }); - // Re-derives inside the transaction from the same snapshot, so the - // persisted rows and the snapshot baseline come from one pass. - expect(mockRunDerivation).toHaveBeenCalledWith({ - tx, - type: 'context_of_organization', - documentId: 'doc_1', - organizationId: 'org_1', - frameworkId: 'fw_1', - data: expect.objectContaining({ organizationName: 'Acme' }), - }); - expect(upsertLatestSnapshotVersion).toHaveBeenCalled(); - expect(tx.ismsDocument.update).toHaveBeenCalledWith({ - where: { id: 'doc_1' }, - data: expect.objectContaining({ - status: 'approved', - declinedAt: null, - }), - }); - }); - }); - describe('decline', () => { const args = { documentId: 'doc_1', diff --git a/apps/api/src/isms/isms.service.spec.ts b/apps/api/src/isms/isms.service.spec.ts index d8a1f34410..af4c4e5d0d 100644 --- a/apps/api/src/isms/isms.service.spec.ts +++ b/apps/api/src/isms/isms.service.spec.ts @@ -1,6 +1,7 @@ import { NotFoundException } from '@nestjs/common'; import { db } from '@db'; import { IsmsService } from './isms.service'; +import type { IsmsVersionService } from './isms-version.service'; jest.mock('@db', () => ({ db: { @@ -20,12 +21,12 @@ jest.mock('./documents/data-source', () => ({ jest.mock('./documents/generate', () => ({ runDerivation: jest.fn(), })); -jest.mock('./utils/version-snapshot', () => ({ - upsertLatestSnapshotVersion: jest.fn(), -})); const mockDb = jest.mocked(db); +// ensureSetup never touches the version service; a bare stub satisfies the ctor. +const versionService = {} as unknown as IsmsVersionService; + /** Convenience accessor for the first createMany call's `data` array. */ const createManyData = () => (mockDb.ismsDocument.createMany as jest.Mock).mock.calls[0][0].data; @@ -35,7 +36,7 @@ describe('IsmsService ensureSetup', () => { beforeEach(() => { jest.clearAllMocks(); - service = new IsmsService(); + service = new IsmsService(versionService); (mockDb.control.findMany as jest.Mock).mockResolvedValue([]); (mockDb.ismsDocument.createMany as jest.Mock).mockResolvedValue({ count: 0, diff --git a/apps/api/src/isms/isms.service.ts b/apps/api/src/isms/isms.service.ts index 4d92c33d3f..15dbc19746 100644 --- a/apps/api/src/isms/isms.service.ts +++ b/apps/api/src/isms/isms.service.ts @@ -9,7 +9,9 @@ import { SubmitIsmsForApprovalDto } from './dto/submit-isms-for-approval.dto'; import { deriveControlLinks, resolveDocumentPlans } from './utils/ensure-setup-plan'; import { collectPlatformData } from './documents/data-source'; import { runDerivation } from './documents/generate'; -import { upsertLatestSnapshotVersion } from './utils/version-snapshot'; +import { updateDraftSnapshot } from './utils/draft-snapshot'; +import { EXPORT_DOCUMENT_INCLUDE } from './utils/export-payload'; +import { IsmsVersionService } from './isms-version.service'; /** * ISMS foundational document lifecycle: setup, retrieval and sign-off. Context @@ -18,6 +20,8 @@ import { upsertLatestSnapshotVersion } from './utils/version-snapshot'; */ @Injectable() export class IsmsService { + constructor(private readonly versionService: IsmsVersionService) {} + /** * List the org's ISMS documents, provisioning missing ones first only when the * caller can write (`evidence:update`). Read-only callers never trigger writes. @@ -61,7 +65,9 @@ export class IsmsService { type: doc.type, status: doc.status, requirementId: doc.requirementId, - hasApprovedVersion: doc.status === 'approved', + // A published version exists (the document has been approved at least + // once) — independent of whether the draft has since been edited. + hasApprovedVersion: doc.currentVersionId != null, })), }; } @@ -136,7 +142,11 @@ export class IsmsService { const document = await db.ismsDocument.findFirst({ where: { id: documentId, organizationId }, include: { - versions: { where: { isLatest: true }, take: 1 }, + // The live/published version, for the "Published: vN" display. Full + // history is fetched separately via IsmsVersionService.getVersions. + currentVersion: { + select: { id: true, version: true, publishedAt: true }, + }, contextIssues: { orderBy: { position: 'asc' } }, interestedParties: { orderBy: { position: 'asc' } }, interestedPartyRequirements: { orderBy: { position: 'asc' } }, @@ -204,10 +214,14 @@ export class IsmsService { organizationId, frameworkId: document.frameworkId, }); + const now = new Date(); - await db.$transaction(async (tx) => { - // Re-derive in the same transaction so the persisted rows and the snapshot - // baseline come from one pass (otherwise the approved content can drift). + // 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) => { + // Re-derive in the same transaction so the persisted rows and the frozen + // snapshot come from one pass (otherwise the approved content can drift). await runDerivation({ tx, type: document.type, @@ -216,11 +230,40 @@ export class IsmsService { frameworkId: document.frameworkId, data: snapshot, }); - await upsertLatestSnapshotVersion({ tx, documentId, snapshot }); + await updateDraftSnapshot({ tx, documentId, snapshot }); + + const reloaded = await tx.ismsDocument.findUniqueOrThrow({ + where: { id: documentId }, + include: EXPORT_DOCUMENT_INCLUDE, + }); + const result = await this.versionService.createPublishedVersion({ + tx, + document: reloaded, + memberId: member.id, + now, + snapshotData: snapshot, + }); + await tx.ismsDocument.update({ where: { id: documentId }, - data: { status: 'approved', approvedAt: new Date(), declinedAt: null }, + data: { + status: 'approved', + approvedAt: now, + declinedAt: null, + currentVersionId: result.versionId, + }, }); + return result; + }); + + // Render + upload the frozen exports outside the transaction (Policies + // pattern) so an S3 hiccup never orphans the published version. + await this.versionService.publishRenders({ + organizationId, + documentId, + versionId: published.versionId, + version: published.version, + snapshot: published.snapshot, }); return this.getDocument({ documentId, organizationId }); diff --git a/apps/api/src/isms/utils/draft-snapshot.spec.ts b/apps/api/src/isms/utils/draft-snapshot.spec.ts new file mode 100644 index 0000000000..ddbaefd9d4 --- /dev/null +++ b/apps/api/src/isms/utils/draft-snapshot.spec.ts @@ -0,0 +1,50 @@ +import type { Prisma } from '@db'; +import { updateDraftSnapshot } from './draft-snapshot'; + +// A fake transaction client: only the ismsDocument.update the unit touches is +// stubbed. No module mock — the unit under test is imported real. +function makeTx() { + const ismsDocument = { update: jest.fn() }; + const tx = { ismsDocument } as unknown as Prisma.TransactionClient; + return { tx, ismsDocument }; +} + +const snapshot = { frameworkNames: ['ISO 27001'], vendorCount: 3 }; + +describe('updateDraftSnapshot', () => { + it('writes the serialized snapshot onto the document draftSnapshot', async () => { + const { tx, ismsDocument } = makeTx(); + + await updateDraftSnapshot({ tx, documentId: 'doc_1', snapshot }); + + expect(ismsDocument.update).toHaveBeenCalledWith({ + where: { id: 'doc_1' }, + data: { draftSnapshot: snapshot }, + }); + }); + + it('serializes the snapshot through JSON, dropping undefined fields', async () => { + const { tx, ismsDocument } = makeTx(); + + await updateDraftSnapshot({ + tx, + documentId: 'doc_2', + snapshot: { keep: 'yes', drop: undefined, nested: { ok: 1 } }, + }); + + const updateArg = ismsDocument.update.mock.calls[0][0]; + expect(updateArg.data.draftSnapshot).toEqual({ + keep: 'yes', + nested: { ok: 1 }, + }); + expect('drop' in updateArg.data.draftSnapshot).toBe(false); + }); + + it('scopes the write to the given document id', async () => { + const { tx, ismsDocument } = makeTx(); + + await updateDraftSnapshot({ tx, documentId: 'doc_3', snapshot }); + + expect(ismsDocument.update.mock.calls[0][0].where).toEqual({ id: 'doc_3' }); + }); +}); diff --git a/apps/api/src/isms/utils/draft-snapshot.ts b/apps/api/src/isms/utils/draft-snapshot.ts new file mode 100644 index 0000000000..5991798468 --- /dev/null +++ b/apps/api/src/isms/utils/draft-snapshot.ts @@ -0,0 +1,28 @@ +import type { Prisma } from '@db'; + +/** + * Persist the derived-data snapshot as the document's DRAFT drift baseline. + * + * CS-701: version rows are now published-only, immutable snapshots, so the live + * drift baseline moved off the version row onto `IsmsDocument.draftSnapshot`. + * Drift is detected by comparing this against the current derived data. Runs on + * the transaction client so it stays atomic with the derivation that produced it. + */ +export async function updateDraftSnapshot({ + tx, + documentId, + snapshot, +}: { + tx: Prisma.TransactionClient; + documentId: string; + snapshot: unknown; +}): Promise { + const draftSnapshot: Prisma.InputJsonValue = JSON.parse( + JSON.stringify(snapshot), + ); + + await tx.ismsDocument.update({ + where: { id: documentId }, + data: { draftSnapshot }, + }); +} diff --git a/apps/api/src/isms/utils/export-generator.ts b/apps/api/src/isms/utils/export-generator.ts index e43ca195c0..3c9115c4b8 100644 --- a/apps/api/src/isms/utils/export-generator.ts +++ b/apps/api/src/isms/utils/export-generator.ts @@ -25,7 +25,7 @@ export async function generateIsmsExportFile({ metadata: IsmsExportMetadata; format: IsmsExportFormat; }): Promise { - const baseName = `${sanitizeName(metadata.title)}-v${metadata.version}`; + const baseName = `${sanitizeExportName(metadata.title)}-v${metadata.version}`; if (format === 'docx') { return { @@ -42,7 +42,7 @@ export async function generateIsmsExportFile({ }; } -function sanitizeName(name: string): string { +export function sanitizeExportName(name: string): string { return (name || 'isms-document') .toLowerCase() .replace(/[^a-z0-9]+/g, '-') diff --git a/apps/api/src/isms/utils/export-payload.spec.ts b/apps/api/src/isms/utils/export-payload.spec.ts new file mode 100644 index 0000000000..5473dd7815 --- /dev/null +++ b/apps/api/src/isms/utils/export-payload.spec.ts @@ -0,0 +1,54 @@ +import { draftVersionNumber, parseExportSnapshot } from './export-payload'; +import type { LoadedExportDocument } from './export-payload'; + +// export-payload imports the db client at module load; a bare stub is enough +// since these pure helpers never touch it. +jest.mock('@db', () => ({ db: {} })); + +const buildDocument = ( + over: Partial = {}, +): LoadedExportDocument => + ({ status: 'draft', currentVersion: null, ...over }) as unknown as LoadedExportDocument; + +describe('draftVersionNumber', () => { + it('returns 1 before anything has been published', () => { + expect(draftVersionNumber(buildDocument({ currentVersion: null }))).toBe(1); + }); + + it('returns the published version when the approved draft is clean', () => { + expect( + draftVersionNumber( + buildDocument({ status: 'approved', currentVersion: { version: 3 } }), + ), + ).toBe(3); + }); + + it('returns the next number while the draft is being edited', () => { + expect( + draftVersionNumber( + buildDocument({ status: 'draft', currentVersion: { version: 3 } }), + ), + ).toBe(4); + }); +}); + +describe('parseExportSnapshot', () => { + it('returns null for null / undefined', () => { + expect(parseExportSnapshot(null)).toBeNull(); + expect(parseExportSnapshot(undefined)).toBeNull(); + }); + + it('returns null for an array', () => { + expect(parseExportSnapshot([1, 2] as never)).toBeNull(); + }); + + it('returns null when required keys are missing', () => { + expect(parseExportSnapshot({ type: 'x', input: {} })).toBeNull(); + expect(parseExportSnapshot({ input: {}, metadata: {} })).toBeNull(); + }); + + it('returns the snapshot when type, input and metadata are present', () => { + const snapshot = { type: 'x', input: { rows: [] }, metadata: { version: 1 } }; + expect(parseExportSnapshot(snapshot)).toBe(snapshot); + }); +}); diff --git a/apps/api/src/isms/utils/export-payload.ts b/apps/api/src/isms/utils/export-payload.ts new file mode 100644 index 0000000000..efd9346c33 --- /dev/null +++ b/apps/api/src/isms/utils/export-payload.ts @@ -0,0 +1,181 @@ +import { NotFoundException } from '@nestjs/common'; +import { db } from '@db'; +import type { IsmsDocumentType, Prisma } from '@db'; +import { buildExportSections } from '../documents/registry'; +import { loadOrgProfile } from '../documents/org-profile'; +import type { DocumentExportInput, IsmsOrgProfile } from '../documents/types'; +import { buildExportMetadata } from './export-metadata'; +import { + generateIsmsExportFile, + type IsmsExportFormat, + type IsmsExportResult, +} from './export-generator'; +import type { IsmsExportMetadata } from './export-shared'; + +/** + * The immutable content snapshot stored on a published IsmsDocumentVersion. + * Everything needed to re-render a historical version byte-faithfully if its + * stored file is ever missing: the document type, the export input (register + * rows + narrative + org profile) and the export metadata frozen at publish. + */ +export interface IsmsExportSnapshot { + type: IsmsDocumentType; + input: DocumentExportInput; + metadata: IsmsExportMetadata; +} + +/** Prisma include for a document being exported or snapshotted. */ +export const EXPORT_DOCUMENT_INCLUDE = { + framework: { select: { name: true } }, + organization: { select: { name: true, website: true, primaryColor: true } }, + approver: { select: { user: { select: { name: true, email: true } } } }, + currentVersion: { select: { version: true } }, + contextIssues: { orderBy: { position: 'asc' } }, + interestedParties: { orderBy: { position: 'asc' } }, + interestedPartyRequirements: { orderBy: { position: 'asc' } }, + objectives: { orderBy: { position: 'asc' } }, +} satisfies Prisma.IsmsDocumentInclude; + +export type LoadedExportDocument = Prisma.IsmsDocumentGetPayload<{ + include: typeof EXPORT_DOCUMENT_INCLUDE; +}>; + +/** Load a document with everything the export/snapshot builders need. */ +export async function loadExportDocument( + documentId: string, + organizationId: string, +): Promise { + return db.ismsDocument.findFirst({ + where: { id: documentId, organizationId }, + include: EXPORT_DOCUMENT_INCLUDE, + }); +} + +/** The Context document (4.1) renders an org overview; other types don't need it. */ +export async function resolveOrgProfile( + document: LoadedExportDocument, + client?: Prisma.TransactionClient, +): Promise { + if (document.type !== 'context_of_organization') return undefined; + return loadOrgProfile({ + organizationId: document.organizationId, + frameworkId: document.frameworkId, + client, + }); +} + +/** Map the loaded document's live rows + draft narrative into export input. */ +export function buildExportInput({ + document, + orgProfile, +}: { + document: LoadedExportDocument; + orgProfile?: IsmsOrgProfile; +}): DocumentExportInput { + return { + contextIssues: document.contextIssues.map((issue) => ({ + kind: issue.kind, + category: issue.category, + description: issue.description, + effect: issue.effect, + })), + interestedParties: document.interestedParties.map((party) => ({ + name: party.name, + category: party.category, + needsExpectations: party.needsExpectations, + })), + requirements: document.interestedPartyRequirements.map((row) => ({ + partyName: row.partyName, + requirement: row.requirement, + treatment: row.treatment, + })), + objectives: document.objectives.map((objective) => ({ + objective: objective.objective, + target: objective.target, + cadence: objective.cadence, + status: objective.status, + plan: objective.plan, + measurementMethod: objective.measurementMethod, + })), + narrative: document.draftNarrative ?? null, + orgProfile, + }; +} + +/** + * The version number to show for the current draft: the published version when + * the draft is clean (approved), the next number when edits are in progress, or + * 1 before anything has been published. + */ +export function draftVersionNumber(document: LoadedExportDocument): number { + const published = document.currentVersion?.version ?? 0; + if (!document.currentVersion) return 1; + return document.status === 'approved' ? published : published + 1; +} + +/** Build the full export snapshot ({type,input,metadata}) for the current draft. */ +export async function buildDraftSnapshot( + document: LoadedExportDocument, +): Promise { + const orgProfile = await resolveOrgProfile(document); + const input = buildExportInput({ document, orgProfile }); + const metadata = buildExportMetadata({ + type: document.type, + title: document.title, + frameworkName: document.framework.name || 'ISO 27001', + version: draftVersionNumber(document), + status: document.status, + preparedBy: document.preparedBy, + owner: null, + approverName: + document.approver?.user?.name || document.approver?.user?.email || null, + approvedAt: document.approvedAt, + declinedAt: document.declinedAt, + organizationName: document.organization.name, + primaryColor: document.organization.primaryColor, + }); + return { type: document.type, input, metadata }; +} + +/** Render a document's current DRAFT to a file (used by the export endpoint). */ +export async function renderLiveExport({ + documentId, + organizationId, + format, +}: { + documentId: string; + organizationId: string; + format: IsmsExportFormat; +}): Promise { + const document = await loadExportDocument(documentId, organizationId); + if (!document) { + throw new NotFoundException('ISMS document not found'); + } + return renderSnapshot(await buildDraftSnapshot(document), format); +} + +/** Render a stored/derived export snapshot to a file. */ +export function renderSnapshot( + snapshot: IsmsExportSnapshot, + format: IsmsExportFormat, +): Promise { + const sections = buildExportSections({ + type: snapshot.type, + input: snapshot.input, + }); + return generateIsmsExportFile({ + sections, + metadata: snapshot.metadata, + format, + }); +} + +/** Parse a stored contentSnapshot JSON back into an export snapshot, or null. */ +export function parseExportSnapshot( + value: Prisma.JsonValue | null | undefined, +): IsmsExportSnapshot | null { + if (!value || typeof value !== 'object' || Array.isArray(value)) return null; + const record = value as Record; + if (!record.type || !record.input || !record.metadata) return null; + return record as unknown as IsmsExportSnapshot; +} diff --git a/apps/api/src/isms/utils/version-snapshot.spec.ts b/apps/api/src/isms/utils/version-snapshot.spec.ts deleted file mode 100644 index b9b1484efc..0000000000 --- a/apps/api/src/isms/utils/version-snapshot.spec.ts +++ /dev/null @@ -1,78 +0,0 @@ -import type { Prisma } from '@db'; -import { upsertLatestSnapshotVersion } from './version-snapshot'; - -// A fake transaction client: only the ismsDocumentVersion methods the unit -// touches are stubbed. No module mock — the unit under test is imported real. -function makeTx() { - const ismsDocumentVersion = { - findFirst: jest.fn(), - update: jest.fn(), - create: jest.fn(), - }; - const tx = { ismsDocumentVersion } as unknown as Prisma.TransactionClient; - return { tx, ismsDocumentVersion }; -} - -const snapshot = { frameworkNames: ['ISO 27001'], vendorCount: 3 }; - -describe('upsertLatestSnapshotVersion', () => { - it('updates the existing latest version with the serialized snapshot (UPDATE branch)', async () => { - const { tx, ismsDocumentVersion } = makeTx(); - ismsDocumentVersion.findFirst.mockResolvedValue({ id: 'ver_existing' }); - - await upsertLatestSnapshotVersion({ - tx, - documentId: 'doc_1', - snapshot, - }); - - expect(ismsDocumentVersion.findFirst).toHaveBeenCalledWith({ - where: { documentId: 'doc_1', isLatest: true }, - }); - expect(ismsDocumentVersion.update).toHaveBeenCalledWith({ - where: { id: 'ver_existing' }, - data: { sourceSnapshot: snapshot }, - }); - expect(ismsDocumentVersion.create).not.toHaveBeenCalled(); - }); - - it('creates version 1 marked latest when none exists (CREATE branch)', async () => { - const { tx, ismsDocumentVersion } = makeTx(); - ismsDocumentVersion.findFirst.mockResolvedValue(null); - - await upsertLatestSnapshotVersion({ - tx, - documentId: 'doc_2', - snapshot, - }); - - expect(ismsDocumentVersion.update).not.toHaveBeenCalled(); - expect(ismsDocumentVersion.create).toHaveBeenCalledWith({ - data: { - documentId: 'doc_2', - version: 1, - isLatest: true, - narrative: {}, - sourceSnapshot: snapshot, - }, - }); - }); - - it('serializes the snapshot through JSON, dropping undefined fields', async () => { - const { tx, ismsDocumentVersion } = makeTx(); - ismsDocumentVersion.findFirst.mockResolvedValue(null); - - await upsertLatestSnapshotVersion({ - tx, - documentId: 'doc_3', - snapshot: { keep: 'yes', drop: undefined, nested: { ok: 1 } }, - }); - - const createArg = ismsDocumentVersion.create.mock.calls[0][0]; - expect(createArg.data.sourceSnapshot).toEqual({ - keep: 'yes', - nested: { ok: 1 }, - }); - expect('drop' in createArg.data.sourceSnapshot).toBe(false); - }); -}); diff --git a/apps/api/src/isms/utils/version-snapshot.ts b/apps/api/src/isms/utils/version-snapshot.ts deleted file mode 100644 index 227a79c060..0000000000 --- a/apps/api/src/isms/utils/version-snapshot.ts +++ /dev/null @@ -1,45 +0,0 @@ -import type { Prisma } from '@db'; - -const EMPTY_NARRATIVE: Prisma.InputJsonValue = {}; - -/** - * Persist the derived-data snapshot onto the document's latest version, creating - * version 1 if none exists. The snapshot is the drift baseline. Serializing - * through JSON keeps it a plain Prisma.InputJsonValue without unsafe casts. The - * existing narrative is preserved (only sourceSnapshot is written). - */ -export async function upsertLatestSnapshotVersion({ - tx, - documentId, - snapshot, -}: { - tx: Prisma.TransactionClient; - documentId: string; - snapshot: unknown; -}): Promise { - const sourceSnapshot: Prisma.InputJsonValue = JSON.parse( - JSON.stringify(snapshot), - ); - - const latest = await tx.ismsDocumentVersion.findFirst({ - where: { documentId, isLatest: true }, - }); - - if (latest) { - await tx.ismsDocumentVersion.update({ - where: { id: latest.id }, - data: { sourceSnapshot }, - }); - return; - } - - await tx.ismsDocumentVersion.create({ - data: { - documentId, - version: 1, - isLatest: true, - narrative: EMPTY_NARRATIVE, - sourceSnapshot, - }, - }); -} diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/ContextOfOrganizationClient.test.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/ContextOfOrganizationClient.test.tsx index a3176b169b..c4dd38c70e 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/components/ContextOfOrganizationClient.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/ContextOfOrganizationClient.test.tsx @@ -67,6 +67,10 @@ vi.mock('./IsmsControlMappings', () => ({ IsmsControlMappings: () =>
, })); +vi.mock('./IsmsVersionHistory', () => ({ + IsmsVersionHistory: () =>
, +})); + import { ContextOfOrganizationClient } from './ContextOfOrganizationClient'; const ISSUES: IsmsContextIssue[] = [ @@ -106,7 +110,9 @@ function makeDocument(overrides: Partial = {}): IsmsDocument { interestedPartyRequirements: [], objectives: [], controlLinks: [], - versions: [], + draftNarrative: null, + currentVersionId: null, + currentVersion: null, ...overrides, }; } diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/InterestedPartiesClient.test.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/InterestedPartiesClient.test.tsx index 17866d9ea7..7618d876e2 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/components/InterestedPartiesClient.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/InterestedPartiesClient.test.tsx @@ -92,6 +92,10 @@ vi.mock('./IsmsControlMappings', () => ({ IsmsControlMappings: () =>
, })); +vi.mock('./IsmsVersionHistory', () => ({ + IsmsVersionHistory: () =>
, +})); + import { InterestedPartiesClient } from './InterestedPartiesClient'; const PARTIES: IsmsInterestedParty[] = [ @@ -129,7 +133,9 @@ function makeDocument(overrides: Partial = {}): IsmsDocument { interestedPartyRequirements: [], objectives: [], controlLinks: [], - versions: [], + draftNarrative: null, + currentVersionId: null, + currentVersion: null, ...overrides, }; } diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsApprovalSection.test.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsApprovalSection.test.tsx index c31741c2a5..9e1cccdbe3 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsApprovalSection.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsApprovalSection.test.tsx @@ -105,7 +105,9 @@ function makeDocument(overrides: Partial = {}): IsmsDocument { interestedPartyRequirements: [], objectives: [], controlLinks: [], - versions: [], + draftNarrative: null, + currentVersionId: null, + currentVersion: null, ...overrides, }; } diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsApprovalSection.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsApprovalSection.tsx index c46ab07f27..d5bc90fd50 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsApprovalSection.tsx +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsApprovalSection.tsx @@ -75,6 +75,15 @@ export function IsmsApprovalSection({ const approvedDate = formatDate(document.approvedAt); const declinedDate = formatDate(document.declinedAt); + // Versioning context (CS-701): a published version can stay live while the + // draft is edited. `hasDraftChanges` = a published version exists but the + // working draft is no longer approved (edits in progress). + const publishedVersion = document.currentVersion?.version ?? null; + const hasPublishedVersion = publishedVersion != null; + const nextDraftVersion = (publishedVersion ?? 0) + 1; + const hasDraftChanges = + hasPublishedVersion && (isDeclined || (!isApproved && !isPending)); + // The plain submit button is only offered on un-submitted drafts. Pending and // resolved (approved / declined) documents render their own state instead. const showSubmitButton = canManage && !isPending && !isResolved; @@ -98,7 +107,9 @@ export function IsmsApprovalSection({
{isApproved && ( - Approved + + Approved{publishedVersion ? ` · Published as v${publishedVersion}` : ''} + This document was approved by{' '} @@ -109,6 +120,20 @@ export function IsmsApprovalSection({ )} + {hasDraftChanges && ( + + Editing creates a new draft + + Published version{' '} + + v{publishedVersion} + {' '} + stays live and exportable. Your changes are an in-progress draft (v + {nextDraftVersion}) — submit it for approval to publish. + + + )} + {isDeclined && ( Declined diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsControlMappings.test.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsControlMappings.test.tsx index ca860ccc51..269a667b86 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsControlMappings.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsControlMappings.test.tsx @@ -192,7 +192,9 @@ function makeDocument(overrides: Partial = {}): IsmsDocument { interestedPartyRequirements: [], objectives: [], controlLinks: LINKS, - versions: [], + draftNarrative: null, + currentVersionId: null, + currentVersion: null, ...overrides, }; } 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 419474124c..6e50a82601 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 @@ -21,10 +21,12 @@ import { toast } from 'sonner'; import { usePermissions } from '@/hooks/use-permissions'; import { useIsmsDocument, type UseIsmsDocumentReturn } from '../hooks/useIsmsDocument'; import { useIsmsDrift } from '../hooks/useIsmsDrift'; +import { useIsmsDocumentVersions } from '../hooks/useIsmsDocumentVersions'; import type { IsmsDocument as IsmsDocumentData } from '../isms-types'; import { DriftBanner } from './DriftBanner'; import { IsmsControlMappings } from './IsmsControlMappings'; import { IsmsApprovalSection, type ApproverOption } from './IsmsApprovalSection'; +import { IsmsVersionHistory } from './IsmsVersionHistory'; import { IsmsPageHeader } from './shared'; /** Arguments passed to the per-document body render-prop. */ @@ -94,6 +96,14 @@ export function IsmsDocumentShell({ handleExport, } = hook; const { isStale, drift, mutateDrift } = useIsmsDrift(documentId); + const { + versions, + isLoading: versionsLoading, + error: versionsError, + downloadingVersionId, + downloadVersion, + mutateVersions, + } = useIsmsDocumentVersions(documentId, organizationId); const [isGenerating, setIsGenerating] = useState(false); @@ -122,10 +132,15 @@ export function IsmsDocumentShell({ const handleApprove = async () => { try { await approve(); - toast.success('Document approved'); } catch (caught) { toast.error(caught instanceof Error ? caught.message : 'Failed to approve'); + return; } + // Approval already persisted. Refreshing the history list + drift baseline is + // best-effort — a revalidation hiccup must not look like an approval failure + // (which would tempt the user to retry an already-published document). + toast.success('Document approved'); + await Promise.allSettled([mutateVersions(), mutateDrift()]); }; const handleDecline = async () => { @@ -244,6 +259,16 @@ export function IsmsDocumentShell({ {document && ( )} + + {document && ( + + )} ); } diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsVersionHistory.test.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsVersionHistory.test.tsx new file mode 100644 index 0000000000..5125201a52 --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsVersionHistory.test.tsx @@ -0,0 +1,111 @@ +import { fireEvent, render, screen } from '@testing-library/react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import type { IsmsPublishedVersion } from '../isms-types'; +import { ismsDesignSystemMock, ismsIconsMock, ismsSharedMock } from './__test-helpers__/dsMocks'; + +vi.mock('@trycompai/design-system', () => ismsDesignSystemMock()); +vi.mock('@trycompai/design-system/icons', () => ismsIconsMock()); +vi.mock('./shared', () => ismsSharedMock()); + +import { IsmsVersionHistory } from './IsmsVersionHistory'; + +const VERSIONS: IsmsPublishedVersion[] = [ + { + id: 'v2', + version: 2, + publishedAt: '2026-07-06T10:00:00.000Z', + changelog: 'Updated scope', + publishedByName: 'Alice Admin', + hasPdf: true, + hasDocx: true, + isCurrent: true, + }, + { + id: 'v1', + version: 1, + publishedAt: '2026-06-01T10:00:00.000Z', + changelog: null, + publishedByName: 'Bob Owner', + hasPdf: true, + hasDocx: false, + isCurrent: false, + }, +]; + +const baseProps = { + versions: VERSIONS, + isLoading: false, + error: null, + downloadingVersionId: null, + onDownload: vi.fn(), +}; + +describe('IsmsVersionHistory', () => { + beforeEach(() => vi.clearAllMocks()); + + it('lists every published version with its approver, date and changelog', () => { + render(); + + expect(screen.getByText('v2')).toBeInTheDocument(); + expect(screen.getByText('v1')).toBeInTheDocument(); + // Approver + date + changelog surface on the row subtitle. + expect(screen.getByText(/Alice Admin/)).toBeInTheDocument(); + expect(screen.getByText(/Updated scope/)).toBeInTheDocument(); + expect(screen.getByText(/Bob Owner/)).toBeInTheDocument(); + // Only the current version is marked. + expect(screen.getByText('Current')).toBeInTheDocument(); + }); + + it('downloads a specific version as PDF or DOCX', () => { + render(); + + const pdfButtons = screen.getAllByRole('button', { name: /PDF/i }); + const docxButtons = screen.getAllByRole('button', { name: /DOCX/i }); + // First row is v2 (newest first). + fireEvent.click(pdfButtons[0]); + expect(baseProps.onDownload).toHaveBeenCalledWith('v2', 'pdf'); + + fireEvent.click(docxButtons[0]); + expect(baseProps.onDownload).toHaveBeenCalledWith('v2', 'docx'); + }); + + it('only offers formats that are actually available for each version', () => { + render(); + // v1 has hasDocx=false → only v2 exposes a DOCX button (both have PDF). + expect(screen.getAllByRole('button', { name: /PDF/i })).toHaveLength(2); + expect(screen.getAllByRole('button', { name: /DOCX/i })).toHaveLength(1); + }); + + it('shows no download buttons and a note when a version has no retained export', () => { + const noExport: IsmsPublishedVersion[] = [ + { ...VERSIONS[1], id: 'v0', version: 0, hasPdf: false, hasDocx: false }, + ]; + render(); + expect(screen.queryByRole('button', { name: /PDF/i })).not.toBeInTheDocument(); + expect(screen.queryByRole('button', { name: /DOCX/i })).not.toBeInTheDocument(); + expect(screen.getByText('Export unavailable')).toBeInTheDocument(); + }); + + it('renders an empty state when there are no published versions', () => { + render(); + expect(screen.getByText('No published versions yet')).toBeInTheDocument(); + }); + + it('renders an error state when loading history fails', () => { + render( + , + ); + expect( + screen.getByText("Couldn't load version history"), + ).toBeInTheDocument(); + }); + + it('shows a loading state before the first load resolves', () => { + render(); + expect(screen.getByRole('status')).toBeInTheDocument(); + }); +}); diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsVersionHistory.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsVersionHistory.tsx new file mode 100644 index 0000000000..fd5881252b --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsVersionHistory.tsx @@ -0,0 +1,141 @@ +'use client'; + +import { + Badge, + Button, + Item, + ItemActions, + ItemContent, + ItemGroup, + ItemMedia, + ItemTitle, + Section, + Spinner, + Text, +} from '@trycompai/design-system'; +import { Document, Download, Time } from '@trycompai/design-system/icons'; +import { IsmsEmptyState } from './shared'; +import type { IsmsExportFormat, IsmsPublishedVersion } from '../isms-types'; + +interface IsmsVersionHistoryProps { + versions: IsmsPublishedVersion[]; + isLoading: boolean; + error: unknown; + /** Version id currently downloading, so its buttons can show a spinner. */ + downloadingVersionId: string | null; + onDownload: (versionId: string, format: IsmsExportFormat) => void | Promise; +} + +/** Format an ISO timestamp as a short human date, or null when unparseable. */ +function formatDate(value: string | null): string | null { + if (!value) return null; + const parsed = new Date(value); + if (Number.isNaN(parsed.getTime())) return null; + return parsed.toLocaleDateString(undefined, { + year: 'numeric', + month: 'short', + day: 'numeric', + }); +} + +/** One published version's "Published {date} by {name}" line, with changelog. */ +function versionSubtitle(version: IsmsPublishedVersion): string { + const date = formatDate(version.publishedAt); + const published = date ? `Published ${date}` : 'Published'; + const by = version.publishedByName ? ` by ${version.publishedByName}` : ''; + const note = version.changelog ? ` — ${version.changelog}` : ''; + return `${published}${by}${note}`; +} + +/** + * Version history for an ISMS document (CS-701): a list of published versions, + * newest first, each with its date, approver and per-version PDF/DOCX download. + * Rendered once inside IsmsDocumentShell so all six documents inherit it. Data is + * owned by the shell (useIsmsDocumentVersions) and passed in as props. + */ +export function IsmsVersionHistory({ + versions, + isLoading, + error, + downloadingVersionId, + onDownload, +}: IsmsVersionHistoryProps) { + return ( +
+ {isLoading && versions.length === 0 ? ( +
+ +
+ ) : error && versions.length === 0 ? ( + + ) : versions.length === 0 ? ( + + ) : ( + + {versions.map((version) => ( + + + + + + {`v${version.version}`} + {version.isCurrent && Current} + + + {versionSubtitle(version)} + + + + {version.hasPdf && ( + + )} + {version.hasDocx && ( + + )} + {!version.hasPdf && !version.hasDocx && ( + + Export unavailable + + )} + + + ))} + + )} +
+ ); +} diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/LeadershipClient.test.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/LeadershipClient.test.tsx index 307f5c56bd..e0dd17d495 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/components/LeadershipClient.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/LeadershipClient.test.tsx @@ -122,6 +122,10 @@ vi.mock('./IsmsControlMappings', () => ({ IsmsControlMappings: () =>
, })); +vi.mock('./IsmsVersionHistory', () => ({ + IsmsVersionHistory: () =>
, +})); + vi.mock('./shared', () => ({ IsmsPageHeader: ({ clause, @@ -169,7 +173,9 @@ function makeDocument(overrides: Partial = {}): IsmsDocument { interestedPartyRequirements: [], objectives: [], controlLinks: [], - versions: [{ id: 'v1', version: 1, isLatest: true, narrative: NARRATIVE }], + draftNarrative: NARRATIVE, + currentVersionId: null, + currentVersion: null, ...overrides, }; } diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/LeadershipClient.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/LeadershipClient.tsx index f52baee3d5..ae89fe368d 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/components/LeadershipClient.tsx +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/LeadershipClient.tsx @@ -21,8 +21,7 @@ interface LeadershipClientProps { function extractNarrative( document: IsmsDocumentData, ): Partial | null { - const versions = Array.isArray(document.versions) ? document.versions : []; - const narrative = versions[0]?.narrative ?? null; + const narrative = document.draftNarrative ?? null; if (!narrative || typeof narrative !== 'object') return null; return narrative as Partial; } diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/ObjectivesClient.test.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/ObjectivesClient.test.tsx index 59098b9fd0..424590c6f0 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/components/ObjectivesClient.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/ObjectivesClient.test.tsx @@ -67,6 +67,10 @@ vi.mock('./IsmsControlMappings', () => ({ IsmsControlMappings: () =>
, })); +vi.mock('./IsmsVersionHistory', () => ({ + IsmsVersionHistory: () =>
, +})); + import { ObjectivesClient } from './ObjectivesClient'; const OBJECTIVES: IsmsObjective[] = [ @@ -112,7 +116,9 @@ function makeDocument(overrides: Partial = {}): IsmsDocument { interestedPartyRequirements: [], objectives: OBJECTIVES, controlLinks: [], - versions: [], + draftNarrative: null, + currentVersionId: null, + currentVersion: null, ...overrides, }; } 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 123d855946..258f1c7920 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 @@ -71,6 +71,10 @@ vi.mock('./IsmsControlMappings', () => ({ IsmsControlMappings: () =>
, })); +vi.mock('./IsmsVersionHistory', () => ({ + IsmsVersionHistory: () =>
, +})); + import { RequirementsClient } from './RequirementsClient'; const REQUIREMENTS: IsmsInterestedPartyRequirement[] = [ @@ -110,7 +114,9 @@ function makeDocument(overrides: Partial = {}): IsmsDocument { interestedPartyRequirements: REQUIREMENTS, objectives: [], controlLinks: [], - versions: [], + draftNarrative: null, + currentVersionId: null, + currentVersion: null, ...overrides, }; } diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/ScopeClient.test.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/ScopeClient.test.tsx index 68f01ec0e9..8ebc910886 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/components/ScopeClient.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/ScopeClient.test.tsx @@ -118,6 +118,10 @@ vi.mock('./IsmsControlMappings', () => ({ IsmsControlMappings: () =>
, })); +vi.mock('./IsmsVersionHistory', () => ({ + IsmsVersionHistory: () =>
, +})); + vi.mock('./shared', () => ({ IsmsPageHeader: ({ clause, @@ -160,7 +164,9 @@ function makeDocument(overrides: Partial = {}): IsmsDocument { interestedPartyRequirements: [], objectives: [], controlLinks: [], - versions: [{ id: 'v1', version: 1, isLatest: true, narrative: NARRATIVE }], + draftNarrative: NARRATIVE, + currentVersionId: null, + currentVersion: null, ...overrides, }; } diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/ScopeClient.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/ScopeClient.tsx index 29df85dc2d..f3b006621d 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/components/ScopeClient.tsx +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/ScopeClient.tsx @@ -63,12 +63,11 @@ export function ScopeClient(props: ScopeClientProps) { } }; - const versions = Array.isArray(document.versions) ? document.versions : []; - const narrative = toScopeNarrative(versions[0]?.narrative); + const narrative = toScopeNarrative(document.draftNarrative); return (
{children}
, Heading: ({ children }: { children: ReactNode }) =>

{children}

, + Item: ({ children }: { children: ReactNode }) =>
{children}
, + ItemGroup: ({ children }: { children: ReactNode }) =>
{children}
, + ItemMedia: ({ children }: { children?: ReactNode }) =>
{children}
, + ItemContent: ({ children }: { children: ReactNode }) =>
{children}
, + ItemTitle: ({ children }: { children: ReactNode }) =>
{children}
, + ItemActions: ({ children }: { children: ReactNode }) =>
{children}
, Dialog: ({ children }: { children: ReactNode }) =>
{children}
, DialogContent: ({ children }: { children: ReactNode }) =>
{children}
, DialogDescription: ({ children }: { children: ReactNode }) =>

{children}

, @@ -78,6 +84,7 @@ export function ismsIconsMock() { Document: Icon, Download: Icon, Edit: Icon, + Time: Icon, Flag: Icon, ListChecked: Icon, MachineLearningModel: Icon, @@ -90,6 +97,18 @@ export function ismsIconsMock() { export function ismsSharedMock() { return { + IsmsEmptyState: ({ + title, + description, + }: { + title: ReactNode; + description?: ReactNode; + }) => ( +
+

{title}

+ {description ?

{description}

: null} +
+ ), IsmsPageHeader: ({ clause, title, diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/hooks/exportIsmsDocument.ts b/apps/app/src/app/(app)/[orgId]/documents/isms/hooks/exportIsmsDocument.ts index 64889e41d2..67ba9ed11f 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/hooks/exportIsmsDocument.ts +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/hooks/exportIsmsDocument.ts @@ -6,6 +6,11 @@ interface ExportIsmsDocumentParams { documentId: string; format: IsmsExportFormat; organizationId?: string; + /** + * Published version to export. Omit to export the current working draft; provide + * a version id to download exactly what was approved at that version (CS-701). + */ + versionId?: string; } function parseFilename(contentDisposition: string | null, fallback: string): string { @@ -14,11 +19,12 @@ function parseFilename(contentDisposition: string | null, fallback: string): str return match ? match[1] : fallback; } -/** Downloads an ISMS document export (pdf/docx/md) via the API and triggers a browser save. */ +/** Downloads an ISMS document export (pdf/docx) via the API and triggers a browser save. */ export async function exportIsmsDocument({ documentId, format, organizationId, + versionId, }: ExportIsmsDocumentParams): Promise { // Route through the shared apiClient so the request carries the same // credentials + organization header context as every other API call. @@ -26,7 +32,7 @@ export async function exportIsmsDocument({ method: 'POST', organizationId, headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ format }), + body: JSON.stringify(versionId ? { format, versionId } : { format }), }); if (!response.ok) { diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/hooks/useIsmsDocumentVersions.test.ts b/apps/app/src/app/(app)/[orgId]/documents/isms/hooks/useIsmsDocumentVersions.test.ts new file mode 100644 index 0000000000..ad7b053fce --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/hooks/useIsmsDocumentVersions.test.ts @@ -0,0 +1,94 @@ +import { act, renderHook } from '@testing-library/react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { api } from '@/lib/api-client'; + +// Capture the SWR key + fetcher so we can assert the endpoint without firing it +// on mount. `vi.hoisted` keeps the holder available inside the hoisted vi.mock. +const swr = vi.hoisted(() => ({ + key: undefined as unknown, + fetcher: null as null | ((key: unknown) => unknown), +})); + +vi.mock('@/lib/api-client', () => ({ + api: { get: vi.fn() }, +})); + +vi.mock('swr', () => ({ + default: (key: unknown, fetcher: (k: unknown) => unknown) => { + swr.key = key; + swr.fetcher = fetcher; + return { data: undefined, error: undefined, isLoading: false, mutate: vi.fn() }; + }, +})); + +vi.mock('./exportIsmsDocument', () => ({ + exportIsmsDocument: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock('sonner', () => ({ + toast: { error: vi.fn(), success: vi.fn() }, +})); + +import { useIsmsDocumentVersions } from './useIsmsDocumentVersions'; +import { exportIsmsDocument } from './exportIsmsDocument'; +import { toast } from 'sonner'; + +const getMock = vi.mocked(api.get); +const exportMock = vi.mocked(exportIsmsDocument); +const toastErrorMock = vi.mocked(toast.error); + +const DOC_ID = 'doc_1'; +const ORG_ID = 'org_1'; + +beforeEach(() => { + vi.clearAllMocks(); + swr.key = undefined; + swr.fetcher = null; +}); + +describe('useIsmsDocumentVersions', () => { + it('keys SWR on the document and fetches GET /v1/isms/documents/:id/versions', async () => { + getMock.mockResolvedValue({ data: { currentVersionId: null, versions: [] }, status: 200 }); + + renderHook(() => useIsmsDocumentVersions(DOC_ID, ORG_ID)); + + expect(swr.key).toEqual(['/v1/isms/documents', DOC_ID, 'versions', ORG_ID]); + await swr.fetcher?.(['/v1/isms/documents', DOC_ID, 'versions', ORG_ID]); + // Passes the route org (X-Organization-Id), matching the download path. + expect(getMock).toHaveBeenCalledWith( + `/v1/isms/documents/${DOC_ID}/versions`, + ORG_ID, + ); + }); + + it('does not fetch when no document id is provided', () => { + renderHook(() => useIsmsDocumentVersions(null, ORG_ID)); + expect(swr.key).toBeNull(); + }); + + it('downloads a specific published version via exportIsmsDocument', async () => { + const { result } = renderHook(() => useIsmsDocumentVersions(DOC_ID, ORG_ID)); + + await act(async () => { + await result.current.downloadVersion('isms_ver_1', 'pdf'); + }); + + expect(exportMock).toHaveBeenCalledWith({ + documentId: DOC_ID, + format: 'pdf', + organizationId: ORG_ID, + versionId: 'isms_ver_1', + }); + }); + + it('surfaces a toast when a version download fails', async () => { + exportMock.mockRejectedValueOnce(new Error('boom')); + const { result } = renderHook(() => useIsmsDocumentVersions(DOC_ID, ORG_ID)); + + await act(async () => { + await result.current.downloadVersion('isms_ver_1', 'pdf'); + }); + + expect(toastErrorMock).toHaveBeenCalledWith('boom'); + }); +}); diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/hooks/useIsmsDocumentVersions.ts b/apps/app/src/app/(app)/[orgId]/documents/isms/hooks/useIsmsDocumentVersions.ts new file mode 100644 index 0000000000..ae80c58f25 --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/hooks/useIsmsDocumentVersions.ts @@ -0,0 +1,85 @@ +'use client'; + +import { useState } from 'react'; +import { toast } from 'sonner'; +import useSWR from 'swr'; +import { api } from '@/lib/api-client'; +import type { IsmsExportFormat, IsmsVersionHistory } from '../isms-types'; +import { exportIsmsDocument } from './exportIsmsDocument'; + +/** + * Published version history for an ISMS document (CS-701). Keyed on + * `['/v1/isms/documents', documentId, 'versions']` so the detail page's history + * section stays in sync. Exposes a `downloadVersion` helper that fetches a + * specific published version's stored export, and `mutateVersions` to revalidate + * after an approval promotes a new version. + */ +export function useIsmsDocumentVersions( + documentId: string | null, + organizationId?: string, +) { + const { data, error, isLoading, mutate } = useSWR( + documentId + ? (['/v1/isms/documents', documentId, 'versions', organizationId] as const) + : null, + async ([base, id, , orgId]: readonly [ + string, + string, + string, + string | undefined, + ]) => { + // Pass the route org so history stays in the same org context as the + // per-version downloads (X-Organization-Id), not the session's active org. + const response = await api.get( + `${base}/${id}/versions`, + orgId, + ); + if (response.error || !response.data) { + throw new Error(response.error ?? 'Failed to load version history'); + } + return response.data; + }, + ); + + const [downloadingVersionId, setDownloadingVersionId] = useState( + null, + ); + + const downloadVersion = async ( + versionId: string, + format: IsmsExportFormat, + ): Promise => { + if (!documentId) return; + setDownloadingVersionId(versionId); + try { + await exportIsmsDocument({ + documentId, + format, + organizationId, + versionId, + }); + } catch (caught) { + // Surface the failure instead of a silent no-op / unhandled rejection. + toast.error( + caught instanceof Error ? caught.message : 'Failed to download version', + ); + } finally { + // Only clear if this request still owns the indicator — a later download of + // a different version must not have its spinner cleared by an earlier one. + setDownloadingVersionId((current) => + current === versionId ? null : current, + ); + } + }; + + return { + history: data ?? null, + versions: data?.versions ?? [], + currentVersionId: data?.currentVersionId ?? null, + error, + isLoading, + downloadingVersionId, + downloadVersion, + mutateVersions: mutate, + }; +} diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/isms-types.ts b/apps/app/src/app/(app)/[orgId]/documents/isms/isms-types.ts index 033193da38..1c160d16d8 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/isms-types.ts +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/isms-types.ts @@ -129,13 +129,29 @@ export interface IsmsControlLink { control: { id: string; name: string }; } -/** Latest version row included on the fetched document. */ -export interface IsmsDocumentVersion { +/** The live/published version summary included on the fetched document (CS-701). */ +export interface IsmsCurrentVersion { id: string; version: number; - isLatest: boolean; - /** Per-type narrative payload (scope / leadership); null for register-only docs. */ - narrative: IsmsScopeNarrative | IsmsLeadershipNarrative | Record | null; + publishedAt: string | null; +} + +/** A published version in the version-history feed (CS-701). */ +export interface IsmsPublishedVersion { + id: string; + version: number; + publishedAt: string | null; + changelog: string | null; + publishedByName: string | null; + hasPdf: boolean; + hasDocx: boolean; + isCurrent: boolean; +} + +/** Response of `GET /v1/isms/documents/:id/versions`. */ +export interface IsmsVersionHistory { + currentVersionId: string | null; + versions: IsmsPublishedVersion[]; } /** Summary row returned by `ensure-setup`. */ @@ -154,8 +170,10 @@ export interface IsmsEnsureSetupResponse { /** * Full document returned by `GET /v1/isms/documents/:id`. Every register array is - * always present (empty when the document type does not use that register), and - * `versions[0]` (when present) carries the singleton narrative. + * always present (empty when the document type does not use that register). The + * editable draft lives on the document itself (register rows + `draftNarrative`); + * `currentVersion` is the live/published version (CS-701). Version history is + * fetched separately via `GET /v1/isms/documents/:id/versions`. */ export interface IsmsDocument { id: string; @@ -170,7 +188,14 @@ export interface IsmsDocument { interestedPartyRequirements: IsmsInterestedPartyRequirement[]; objectives: IsmsObjective[]; controlLinks: IsmsControlLink[]; - versions: IsmsDocumentVersion[]; + /** Working-draft narrative for the singleton documents (Scope, Leadership). */ + draftNarrative: + | IsmsScopeNarrative + | IsmsLeadershipNarrative + | Record + | null; + currentVersionId: string | null; + currentVersion: IsmsCurrentVersion | null; } export interface IsmsDriftResult { diff --git a/packages/db/prisma/migrations/20260707120000_isms_document_versioning/migration.sql b/packages/db/prisma/migrations/20260707120000_isms_document_versioning/migration.sql new file mode 100644 index 0000000000..bd3d69c426 --- /dev/null +++ b/packages/db/prisma/migrations/20260707120000_isms_document_versioning/migration.sql @@ -0,0 +1,53 @@ +-- ISMS document versioning & history (CS-701) +-- Brings ISMS documents onto the Policies versioning model: a published-version +-- pointer + working-draft fields on the parent, and per-version publish metadata + +-- an immutable content snapshot on the version. + +-- AlterTable: working-draft holders + current-published-version pointer. +ALTER TABLE "IsmsDocument" ADD COLUMN "currentVersionId" TEXT, +ADD COLUMN "draftNarrative" JSONB, +ADD COLUMN "draftSnapshot" JSONB; + +-- AlterTable: per-version changelog + immutable rendered-content snapshot. +ALTER TABLE "IsmsDocumentVersion" ADD COLUMN "changelog" TEXT, +ADD COLUMN "contentSnapshot" JSONB; + +-- --------------------------------------------------------------------------- +-- Data backfill. Runs BEFORE the new indexes/constraints so the legacy-row +-- cleanup can never trip foreign-key validation. ISMS is pre-GA (flag-gated), +-- so volumes are tiny. +-- --------------------------------------------------------------------------- + +-- 1. Move the working narrative + drift baseline off the (previously single) +-- version row onto the parent document, which now holds the editable draft. +UPDATE "IsmsDocument" d +SET "draftNarrative" = v."narrative", + "draftSnapshot" = v."sourceSnapshot" +FROM "IsmsDocumentVersion" v +WHERE v."documentId" = d."id" AND v."isLatest" = true; + +-- 2. The pre-CS-701 version row was only ever a mutable draft holder with no +-- frozen contentSnapshot, so it cannot serve as an immutable published version +-- (a snapshot-less "current version" would 404 on export). Drop every legacy +-- row; its content is preserved on the parent by step 1. +-- +-- Approvals are intentionally preserved: `currentVersionId` stays null, so an +-- approved document keeps its status and simply exports its (unchanged) current +-- content on demand until its next approval captures a real versioned artifact. +-- From here on a published version is ONLY created by the approve flow, which +-- always writes a contentSnapshot — so currentVersionId never points at a +-- snapshot-less version. This DELETE also clears the legacy (always-null) +-- `publishedById` values before the FK below is added. +DELETE FROM "IsmsDocumentVersion"; + +-- CreateIndex +CREATE UNIQUE INDEX "IsmsDocument_currentVersionId_key" ON "IsmsDocument"("currentVersionId"); + +-- CreateIndex +CREATE INDEX "IsmsDocumentVersion_documentId_publishedAt_idx" ON "IsmsDocumentVersion"("documentId", "publishedAt"); + +-- AddForeignKey +ALTER TABLE "IsmsDocument" ADD CONSTRAINT "IsmsDocument_currentVersionId_fkey" FOREIGN KEY ("currentVersionId") REFERENCES "IsmsDocumentVersion"("id") ON DELETE SET NULL ON UPDATE CASCADE; + +-- AddForeignKey +ALTER TABLE "IsmsDocumentVersion" ADD CONSTRAINT "IsmsDocumentVersion_publishedById_fkey" FOREIGN KEY ("publishedById") REFERENCES "Member"("id") ON DELETE SET NULL ON UPDATE CASCADE; diff --git a/packages/db/prisma/schema/auth.prisma b/packages/db/prisma/schema/auth.prisma index b6d01031c4..0219fc9834 100644 --- a/packages/db/prisma/schema/auth.prisma +++ b/packages/db/prisma/schema/auth.prisma @@ -218,6 +218,7 @@ model Member { approvedPolicies Policy[] @relation("PolicyApprover") // Policies where this member is an approver approvedSOADocuments SOADocument[] @relation("SOADocumentApprover") // SOA documents where this member is an approver approvedIsmsDocuments IsmsDocument[] @relation("IsmsDocumentApprover") // ISMS documents where this member is an approver + publishedIsmsVersions IsmsDocumentVersion[] @relation("IsmsDocumentVersionPublisher") // ISMS document versions this member published risks Risk[] tasks Task[] vendors Vendor[] diff --git a/packages/db/prisma/schema/isms.prisma b/packages/db/prisma/schema/isms.prisma index 536b3b165f..a8ea070346 100644 --- a/packages/db/prisma/schema/isms.prisma +++ b/packages/db/prisma/schema/isms.prisma @@ -50,18 +50,35 @@ model IsmsDocument { title String status IsmsDocumentStatus @default(draft) - // Approval / sign-off (mirrors SOADocument). + // Approval / sign-off (mirrors SOADocument). These track the WORKING DRAFT's + // approval lifecycle; the published version's own approver/date live on the + // IsmsDocumentVersion row (publishedBy/publishedAt). preparedBy String @default("Comp AI") approverId String? approver Member? @relation("IsmsDocumentApprover", fields: [approverId], references: [id], onDelete: SetNull, onUpdate: Cascade) approvedAt DateTime? declinedAt DateTime? + // Versioning (CS-701): mirrors the Policies model (Policy.currentVersionId + + // draftContent). The live register tables + `draftNarrative` are the editable + // DRAFT; `versions` holds immutable PUBLISHED snapshots; `currentVersion` is the + // one that is live and exportable. Editing an approved document reverts `status` + // to draft but never touches `currentVersion`, so v1 stays live while v2 is drafted. + currentVersionId String? @unique + currentVersion IsmsDocumentVersion? @relation("IsmsCurrentVersion", fields: [currentVersionId], references: [id], onDelete: SetNull) + // Working-draft narrative for the singleton documents (Scope 4.3, Leadership 5.1). + // Register documents keep their draft in their own child tables. Replaces the old + // `versions[0].narrative` draft-holder. + draftNarrative Json? + // Drift baseline for the current draft: the derived platform inputs captured at + // the last generate/approve. Replaces the old `versions[0].sourceSnapshot`. + draftSnapshot Json? + createdAt DateTime @default(now()) updatedAt DateTime @updatedAt // Relationships - versions IsmsDocumentVersion[] + versions IsmsDocumentVersion[] @relation("IsmsDocumentVersions") contextIssues IsmsContextIssue[] // 4.1 register interestedParties IsmsInterestedParty[] // 4.2a register interestedPartyRequirements IsmsInterestedPartyRequirement[] // 4.2b/c register @@ -82,25 +99,38 @@ model IsmsDocument { model IsmsDocumentVersion { id String @id @default(dbgenerated("generate_prefixed_cuid('isms_ver'::text)")) documentId String - document IsmsDocument @relation(fields: [documentId], references: [id], onDelete: Cascade) + document IsmsDocument @relation("IsmsDocumentVersions", fields: [documentId], references: [id], onDelete: Cascade) + // Back-relation for IsmsDocument.currentVersion: set when this version is the + // live/published one for its document. + currentForDocument IsmsDocument? @relation("IsmsCurrentVersion") version Int @default(1) isLatest Boolean @default(true) - // Per-section saved narrative + section-level overrides (stored as data, not free - // text, so the model is localisation-ready). + // Frozen published narrative for the singleton documents (Scope, Leadership). + // Empty ({}) for register documents, whose frozen content lives in contentSnapshot. narrative Json - // Snapshot of derived inputs captured at generate / approve. Drift is detected by - // comparing this against the current derived data. + // The full, immutable content snapshot of this published version: the rendered + // export input (register rows + narrative + org profile) plus the export metadata + // captured at publish. Lets a historical version be re-rendered byte-faithfully if + // the stored file is ever missing. Null only for legacy/migrated versions. + contentSnapshot Json? + // Snapshot of derived platform inputs captured at publish, retained for the record + // (the live drift baseline lives on IsmsDocument.draftSnapshot). sourceSnapshot Json? - // Rendered, branded exports. + // Rendered, branded exports retained per published version (S3 keys). Auditors + // sampling an old version download exactly what was approved at the time. pdfUrl String? docxUrl String? - // User/member id who published this version (plain id, like SOAAnswer.createdBy). + // The member who published (approved) this version, and when. `publishedAt != null` + // is the marker of a published version (drafts are not materialized as rows). publishedById String? + publishedBy Member? @relation("IsmsDocumentVersionPublisher", fields: [publishedById], references: [id], onDelete: SetNull) publishedAt DateTime? + // Optional free-text note describing what changed in this version. + changelog String? createdAt DateTime @default(now()) updatedAt DateTime @updatedAt @@ -108,6 +138,7 @@ model IsmsDocumentVersion { @@unique([documentId, version]) @@index([documentId]) @@index([documentId, isLatest]) + @@index([documentId, publishedAt]) } // --- Register: Context of the Organization (clause 4.1) --- From a5a9243ab01453f45d81480ab52b9a74744b6888 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov <72318342+tofikwest@users.noreply.github.com> Date: Tue, 7 Jul 2026 15:06:35 -0400 Subject: [PATCH 2/7] feat(integrations): add read-only GitHub App integration (CS-710) (#3363) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(integrations): add read-only GitHub App integration (CS-710) Add a new, additive "GitHub App" integration (slug `github-app`) that connects via a GitHub App installation flow with fine-grained, read-only permissions, instead of the legacy `github` OAuth App which can only reach private repos through the broad `repo` scope (read + write). The existing `github` OAuth integration is left completely untouched so current connections keep working — customers opt into the new one. - New `github-app` manifest reuses the five existing GitHub checks verbatim (no logic duplication; check results are keyed per connection so ids can be shared across the two manifests). - Add an `appInstallFlow` flag to the OAuth config. When set, `/oauth/start` redirects to the App install URL (github.com/apps/{slug}/installations/new) with only `state` appended — client_id/response_type/scope do not apply. - The OAuth callback still exchanges the returned `code` for a user-to-server token (unchanged) and now persists `installation_id` on the connection so a future server-to-server (installation token) upgrade needs no re-connect. - App slug is admin-configurable via the existing additionalOAuthSettings mechanism ({APP_SLUG} substitution), like Rippling's app name. - Tests: new manifest test (registration, read-only flow, check reuse, legacy untouched) + OAuth controller tests for the install URL and installation_id capture. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01XyttkYNbkdYRp7DBKNuFVH * fix(integrations): route GitHub App install callback per environment Pass `redirect_uri` on the GitHub App install URL (not just `state`). A GitHub App can register multiple callback URLs (prod/staging/localhost); without an explicit redirect_uri, GitHub falls back to the first-registered one, so a staging/localhost install could bounce back to production. Send the environment-specific callback so each environment routes to its own callback. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01XyttkYNbkdYRp7DBKNuFVH * fix(integrations): fail loudly when GitHub App install URL is unresolved cubic flagged that the org-scoped OAuth-app save path (oauth-apps.controller) does not persist customSettings, so org-level `github-app` credentials would leave {APP_SLUG} unresolved and `/oauth/start` would redirect to a broken GitHub install URL. github-app is a single Comp AI-owned platform App — bring-your-own-app at the org level isn't a supported flow for it — so instead of building org-level slug persistence, guard the install flow: if any install-URL placeholder token (e.g. {APP_SLUG}) is still unresolved after credential substitution, throw PRECONDITION_FAILED with a clear setup message instead of redirecting to a broken URL. Integration-agnostic (applies to any appInstallFlow provider). The platform-credential path (our deployment) persists customSettings via the admin UI, so the happy path is unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01XyttkYNbkdYRp7DBKNuFVH * fix(integrations): don't set redirect_uri on GitHub App install URL cubic (and GitHub's documented behavior) confirmed that GitHub ignores redirect_uri on github.com/apps/{slug}/installations/new and always redirects to the App's first registered callback URL after installation. The previous commit's redirect_uri on the install URL was therefore non-functional and its comment was inaccurate. Set only `state` on the install URL and document that per-environment routing must be handled with a separate GitHub App per environment (each with its own single callback URL). redirect_uri remains on the standard OAuth authorize flow, where GitHub does honor it. Update the test to assert redirect_uri is NOT on the install URL. Refs: https://github.com/orgs/community/discussions/54273 Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01XyttkYNbkdYRp7DBKNuFVH --------- Co-authored-by: Claude Opus 4.8 (1M context) --- .../controllers/oauth.controller.spec.ts | 175 ++++++++++++++++++ .../controllers/oauth.controller.ts | 83 ++++++++- .../github-app/__tests__/manifest.test.ts | 54 ++++++ .../src/manifests/github-app/index.ts | 150 +++++++++++++++ .../src/registry/index.ts | 2 + packages/integration-platform/src/types.ts | 11 ++ 6 files changed, 468 insertions(+), 7 deletions(-) create mode 100644 packages/integration-platform/src/manifests/github-app/__tests__/manifest.test.ts create mode 100644 packages/integration-platform/src/manifests/github-app/index.ts diff --git a/apps/api/src/integration-platform/controllers/oauth.controller.spec.ts b/apps/api/src/integration-platform/controllers/oauth.controller.spec.ts index 576aaafa7e..619cb13dc5 100644 --- a/apps/api/src/integration-platform/controllers/oauth.controller.spec.ts +++ b/apps/api/src/integration-platform/controllers/oauth.controller.spec.ts @@ -313,6 +313,93 @@ describe('OAuthController', () => { expect(result.authorizationUrl).toContain('code_challenge='); expect(result.authorizationUrl).toContain('code_challenge_method=S256'); }); + + it('should return a GitHub App install URL (state only) for appInstallFlow providers', async () => { + const manifest = { + id: 'github-app', + name: 'GitHub App', + auth: { + type: 'oauth2', + config: { + authorizeUrl: + 'https://github.com/apps/{APP_SLUG}/installations/new', + tokenUrl: 'https://github.com/login/oauth/access_token', + pkce: false, + appInstallFlow: true, + additionalOAuthSettings: [{ id: 'appSlug', token: '{APP_SLUG}' }], + }, + }, + category: 'Development', + capabilities: [], + isActive: true, + }; + mockedGetManifest.mockReturnValue(manifest as never); + mockOAuthCredentialsService.getCredentials.mockResolvedValue({ + clientId: 'client_123', + clientSecret: 'secret_456', + scopes: [], + source: 'platform', + customSettings: { appSlug: 'comp-ai' }, + }); + mockProviderRepository.upsert.mockResolvedValue(undefined); + mockOAuthStateRepository.create.mockResolvedValue({ + state: 'state_install', + }); + + const result = await controller.startOAuth('org_1', 'user_1', { + providerSlug: 'github-app', + }); + + // Install URL with the slug substituted and only `state` appended. + expect(result.authorizationUrl).toContain( + 'https://github.com/apps/comp-ai/installations/new', + ); + expect(result.authorizationUrl).toContain('state=state_install'); + // GitHub ignores redirect_uri on the install URL and routes to the App's + // first registered callback, so we deliberately do NOT set it here. + expect(result.authorizationUrl).not.toContain('redirect_uri='); + // OAuth authorize params must NOT be on an install URL. + expect(result.authorizationUrl).not.toContain('client_id='); + expect(result.authorizationUrl).not.toContain('response_type='); + expect(result.authorizationUrl).not.toContain('scope='); + }); + + it('should throw PRECONDITION_FAILED for an install-flow provider when the app slug is not configured', async () => { + const manifest = { + id: 'github-app', + name: 'GitHub App', + auth: { + type: 'oauth2', + config: { + authorizeUrl: + 'https://github.com/apps/{APP_SLUG}/installations/new', + tokenUrl: 'https://github.com/login/oauth/access_token', + pkce: false, + appInstallFlow: true, + additionalOAuthSettings: [{ id: 'appSlug', token: '{APP_SLUG}' }], + }, + }, + category: 'Development', + capabilities: [], + isActive: true, + }; + mockedGetManifest.mockReturnValue(manifest as never); + // No customSettings.appSlug → {APP_SLUG} cannot be resolved. + mockOAuthCredentialsService.getCredentials.mockResolvedValue({ + clientId: 'client_123', + clientSecret: 'secret_456', + scopes: [], + source: 'organization', + }); + mockProviderRepository.upsert.mockResolvedValue(undefined); + mockOAuthStateRepository.create.mockResolvedValue({ state: 's' }); + + await expect( + controller.startOAuth('org_1', 'user_1', { + providerSlug: 'github-app', + }), + ).rejects.toThrow(HttpException); + }); }); describe('oauthCallback', () => { @@ -605,6 +692,94 @@ describe('OAuthController', () => { fetchSpy.mockRestore(); }); + it('persists installation_id on the connection for the GitHub App install flow', async () => { + const futureDate = new Date(Date.now() + 600000); + mockOAuthStateRepository.findByState.mockResolvedValue({ + state: 'valid_gh_state', + providerSlug: 'github-app', + organizationId: 'org_1', + userId: 'user_1', + codeVerifier: null, + redirectUrl: null, + expiresAt: futureDate, + }); + mockedGetManifest.mockReturnValue({ + id: 'github-app', + name: 'GitHub App', + category: 'Development', + auth: { + type: 'oauth2', + config: { + authorizeUrl: + 'https://github.com/apps/{APP_SLUG}/installations/new', + tokenUrl: 'https://github.com/login/oauth/access_token', + appInstallFlow: true, + }, + }, + capabilities: [], + isActive: true, + } as never); + mockOAuthCredentialsService.getCredentials.mockResolvedValue({ + clientId: 'client_123', + clientSecret: 'secret_456', + scopes: [], + source: 'platform', + }); + mockProviderRepository.findBySlug.mockResolvedValue({ + id: 'provider_gh', + }); + mockConnectionRepository.findByProviderAndOrg.mockResolvedValue({ + id: 'conn_gh', + metadata: {}, + variables: {}, + lastSyncAt: null, + }); + mockConnectionRepository.update.mockResolvedValue({ + id: 'conn_gh', + metadata: {}, + variables: {}, + lastSyncAt: null, + }); + mockConnectionService.activateConnection.mockResolvedValue({ + id: 'conn_gh', + }); + + const fetchSpy = jest.spyOn(global, 'fetch').mockResolvedValue({ + ok: true, + status: 200, + json: () => Promise.resolve({ access_token: 'gh_user_token' }), + text: () => Promise.resolve(''), + } as unknown as Response); + + await controller.oauthCallback( + { + code: 'auth_code', + state: 'valid_gh_state', + installation_id: '987654', + setup_action: 'install', + }, + mockRequest, + mockResponse, + ); + + await new Promise((resolve) => setImmediate(resolve)); + + expect(mockConnectionRepository.update).toHaveBeenCalledWith( + 'conn_gh', + expect.objectContaining({ + metadata: expect.objectContaining({ + githubInstallationId: '987654', + githubSetupAction: 'install', + }), + }), + ); + const redirectUrl = (mockResponse.redirect as jest.Mock).mock.calls[0][0]; + expect(redirectUrl).toContain('success=true'); + expect(redirectUrl).toContain('provider=github-app'); + + fetchSpy.mockRestore(); + }); + describe('session defense-in-depth', () => { const futureDate = new Date(Date.now() + 600000); const validState = { diff --git a/apps/api/src/integration-platform/controllers/oauth.controller.ts b/apps/api/src/integration-platform/controllers/oauth.controller.ts index 29fec34596..4f3436dac8 100644 --- a/apps/api/src/integration-platform/controllers/oauth.controller.ts +++ b/apps/api/src/integration-platform/controllers/oauth.controller.ts @@ -39,6 +39,9 @@ interface OAuthCallbackQuery { state: string; error?: string; error_description?: string; + // GitHub App installation flow returns these alongside `code`. + installation_id?: string; + setup_action?: string; } @Controller({ path: 'integrations/oauth', version: '1' }) @@ -182,6 +185,44 @@ export class OAuthController { } const authUrl = new URL(authorizeUrl); + // GitHub App installation flow: the connect step is an App *installation* + // (github.com/apps/{slug}/installations/new), not a standard OAuth authorize. + // GitHub redirects to the App's FIRST registered callback URL after install + // and IGNORES any redirect_uri passed to the install URL (confirmed GitHub + // behavior), so only `state` is set here — client_id, response_type and + // scope do not apply either. Per-environment routing is therefore handled by + // using a separate GitHub App per environment (each with its own single + // callback URL), NOT via redirect_uri. The OAuth `code` still comes back on + // that callback (with "Request user authorization during installation" + // enabled), so token exchange proceeds normally afterwards. + if (oauthConfig.appInstallFlow) { + // Every placeholder in the install URL (e.g. {APP_SLUG}) must have been + // resolved from credentials above. If a required setting like the GitHub + // App slug was never persisted (e.g. org-scoped credentials saved without + // customSettings), fail loudly with a clear error instead of redirecting + // the user to a broken GitHub URL. + const unresolvedTokens = (oauthConfig.additionalOAuthSettings ?? []) + .map((setting) => setting.token) + .filter( + (token): token is string => !!token && authorizeUrl.includes(token), + ); + if (unresolvedTokens.length > 0) { + throw new HttpException( + { + message: `GitHub App is not fully configured for ${providerSlug} (missing: ${unresolvedTokens.join(', ')}). Set the App slug in the integration credentials.`, + setupInstructions: oauthConfig.setupInstructions, + createAppUrl: oauthConfig.createAppUrl, + }, + HttpStatus.PRECONDITION_FAILED, + ); + } + authUrl.searchParams.set('state', oauthState.state); + this.logger.log( + `Starting GitHub App install flow for ${providerSlug}, org: ${organizationId} (credentials from ${credentials.source})`, + ); + return { authorizationUrl: authUrl.toString() }; + } + // Standard OAuth2 params authUrl.searchParams.set('client_id', credentials.clientId); authUrl.searchParams.set('response_type', 'code'); @@ -287,7 +328,10 @@ export class OAuthController { // is the one completing it. The `state` token alone provides CSRF // protection, but a session match guards against state-token leakage and // ensures the completing user still has an active session for the org. - const sessionMismatch = await this.checkSessionMatchesState(req, oauthState); + const sessionMismatch = await this.checkSessionMatchesState( + req, + oauthState, + ); if (sessionMismatch) { await this.oauthStateRepository.delete(state); this.logger.warn( @@ -355,11 +399,15 @@ export class OAuthController { } // Store tokens and mark connection as active - await this.credentialVaultService.storeOAuthTokens(connection.id, tokens, { - preserveExistingRefreshToken: - oauthState.providerSlug === 'gcp' || - oauthState.providerSlug === 'google-workspace', - }); + await this.credentialVaultService.storeOAuthTokens( + connection.id, + tokens, + { + preserveExistingRefreshToken: + oauthState.providerSlug === 'gcp' || + oauthState.providerSlug === 'google-workspace', + }, + ); // Mark cloud OAuth reconnect completion so reconnect banners clear after successful OAuth. if (manifest.category === 'Cloud') { @@ -377,6 +425,28 @@ export class OAuthController { }); } + // GitHub App installation flow: persist the installation id (and setup + // action) on the connection. The token stored above is a user-to-server + // token, but recording the installation id means a future server-to-server + // (installation access token) upgrade needs no re-connect. + if (query.installation_id) { + const metadata = + connection.metadata && + typeof connection.metadata === 'object' && + !Array.isArray(connection.metadata) + ? (connection.metadata as Record) + : {}; + connection = await this.connectionRepository.update(connection.id, { + metadata: { + ...metadata, + githubInstallationId: query.installation_id, + ...(query.setup_action + ? { githubSetupAction: query.setup_action } + : {}), + }, + }); + } + await this.connectionService.activateConnection(connection.id); // Provider-specific post-OAuth actions @@ -643,5 +713,4 @@ export class OAuthController { } return url.toString(); } - } diff --git a/packages/integration-platform/src/manifests/github-app/__tests__/manifest.test.ts b/packages/integration-platform/src/manifests/github-app/__tests__/manifest.test.ts new file mode 100644 index 0000000000..05a489229c --- /dev/null +++ b/packages/integration-platform/src/manifests/github-app/__tests__/manifest.test.ts @@ -0,0 +1,54 @@ +import { describe, expect, it } from 'bun:test'; +import { getAllManifests, getManifest } from '../../../registry'; +import { manifest as githubManifest } from '../../github'; +import { githubAppManifest } from '../index'; + +/** + * CS-710: the new `github-app` integration must request read-only, fine-grained + * access (a GitHub App install flow) while leaving the legacy `github` OAuth + * integration completely untouched so existing connections keep working. + */ +describe('github-app manifest (CS-710)', () => { + it('is registered in the registry', () => { + expect(getManifest('github-app')).toBeDefined(); + expect(getAllManifests().some((m) => m.id === 'github-app')).toBe(true); + }); + + it('uses a read-only GitHub App install flow (no `repo` scope requested)', () => { + const { auth } = githubAppManifest; + expect(auth.type).toBe('oauth2'); + if (auth.type !== 'oauth2') return; + expect(auth.config.appInstallFlow).toBe(true); + // GitHub Apps ignore scopes — permissions come from the App config. + expect(auth.config.scopes).toEqual([]); + expect(auth.config.authorizeUrl).toContain('{APP_SLUG}'); + expect(auth.config.authorizeUrl).toContain('installations/new'); + expect(auth.config.tokenUrl).toBe('https://github.com/login/oauth/access_token'); + }); + + it('exposes an admin-configurable app slug setting', () => { + const { auth } = githubAppManifest; + if (auth.type !== 'oauth2') throw new Error('expected oauth2 auth'); + const appSlug = auth.config.additionalOAuthSettings?.find((s) => s.id === 'appSlug'); + expect(appSlug).toBeDefined(); + expect(appSlug?.token).toBe('{APP_SLUG}'); + expect(appSlug?.required).toBe(true); + }); + + it('reuses the exact same checks as the legacy github manifest', () => { + const appCheckIds = githubAppManifest.checks?.map((c) => c.id).sort(); + const legacyCheckIds = githubManifest.checks?.map((c) => c.id).sort(); + expect(appCheckIds).toEqual(legacyCheckIds); + expect(appCheckIds?.length).toBe(5); + }); + + it('leaves the legacy github manifest untouched (still OAuth `repo` scope)', () => { + expect(githubManifest.id).toBe('github'); + expect(githubManifest.name).toBe('GitHub'); + if (githubManifest.auth.type !== 'oauth2') { + throw new Error('expected oauth2 auth'); + } + expect(githubManifest.auth.config.scopes).toContain('repo'); + expect(githubManifest.auth.config.appInstallFlow).toBeUndefined(); + }); +}); diff --git a/packages/integration-platform/src/manifests/github-app/index.ts b/packages/integration-platform/src/manifests/github-app/index.ts new file mode 100644 index 0000000000..a4a1092422 --- /dev/null +++ b/packages/integration-platform/src/manifests/github-app/index.ts @@ -0,0 +1,150 @@ +/** + * GitHub App Integration Manifest (CS-710) + * + * A read-only GitHub integration built on a GitHub *App* rather than the legacy + * `github` OAuth App. + * + * Why this exists: an OAuth App can only reach private repositories via the broad + * `repo` scope, which unavoidably grants **write** access (there is no read-only + * scope for private repos). Customers who were promised a read-only integration + * see that write permission at connect time. A GitHub App instead exposes + * fine-grained, READ-ONLY permissions and lets the customer choose exactly which + * repositories to share. + * + * This is a NEW, additive integration — the existing `github` OAuth integration + * is left completely untouched so current connections keep working. New/concerned + * customers opt into this one. + * + * Connect flow (user-to-server token): + * 1. The customer is sent to the App's installation page + * (`https://github.com/apps/{APP_SLUG}/installations/new`). + * 2. They install the App on their org/account and pick repositories. + * 3. With "Request user authorization (OAuth) during installation" enabled on + * the App, GitHub redirects back through the OAuth flow and returns a `code` + * (plus `installation_id`). + * 4. The shared OAuth callback exchanges the `code` for a user-to-server access + * token (read-only, capped by the App's permissions) and stores it like any + * other OAuth token. The `installation_id` is persisted on the connection so + * a future server-to-server (installation token) upgrade needs no re-connect. + * + * The five checks are reused verbatim from the `github` manifest — only the way + * the token is obtained differs (read-only App vs read/write OAuth App), so there + * is no logic duplication and both integrations stay in lockstep. + */ + +import type { IntegrationManifest } from '../../types'; +import { + branchProtectionCheck, + codeScanningCheck, + dependabotCheck, + sanitizedInputsCheck, + twoFactorAuthCheck, +} from '../github/checks'; + +export const githubAppManifest: IntegrationManifest = { + id: 'github-app', + name: 'GitHub App', + description: + 'Connect GitHub with secure, read-only access to the repositories you choose. Monitors repository security, branch protection, and organization settings.', + category: 'Development', + logoUrl: 'https://img.logo.dev/github.com?token=pk_AZatYxV5QDSfWpRDaBxzRQ', + docsUrl: 'https://docs.trycomp.ai/integrations/github', + + // API configuration for the ctx.fetch helper (identical to the OAuth GitHub). + baseUrl: 'https://api.github.com', + defaultHeaders: { + Accept: 'application/vnd.github.v3+json', + 'User-Agent': 'CompAI-Integration', + }, + + auth: { + type: 'oauth2', + config: { + // "Connect" installs the GitHub App. {APP_SLUG} is replaced at runtime with + // customSettings.appSlug from the configured platform/org credentials + // (reuses the same token-substitution mechanism as Rippling's {APP_NAME}). + authorizeUrl: 'https://github.com/apps/{APP_SLUG}/installations/new', + tokenUrl: 'https://github.com/login/oauth/access_token', + // GitHub Apps ignore OAuth scopes — permissions come from the App's own + // configuration (set to read-only when the App is created). Intentionally + // empty so no scope is requested at connect time. + scopes: [], + pkce: false, + clientAuthMethod: 'body', + // This is a GitHub App *installation* flow, not a standard OAuth authorize. + // The connect step redirects to the install URL; the callback still returns + // an OAuth `code` (with user-authorization-during-installation enabled). + appInstallFlow: true, + // With "Expire user authorization tokens" DISABLED on the App, the token is + // long-lived (like the legacy GitHub OAuth token), so no refresh is needed. + supportsRefreshToken: false, + revoke: { + // Revoke the *grant* (app authorization) so a reconnect starts fresh. + url: 'https://api.github.com/applications/{CLIENT_ID}/grant', + method: 'DELETE', + auth: 'basic', + body: 'json', + tokenField: 'access_token', + }, + setupInstructions: `Create the GitHub App once (a GitHub organization owner must do this): +1. GitHub > Settings > Developer settings > GitHub Apps > "New GitHub App". +2. Set "Callback URL" to the callback URL shown below, and enable + "Request user authorization (OAuth) during installation". +3. Disable "Expire user authorization tokens" (so tokens stay valid, like the + old integration). +4. Under Permissions, grant READ-ONLY access: + • Repository → Metadata, Contents, Pull requests, Administration, Dependabot alerts + • Organization → Members +5. Create the App, then generate a client secret. +6. Enter the Client ID, Client Secret, and the App slug (the "" in the + App's public URL github.com/apps/) in the credentials form.`, + createAppUrl: 'https://github.com/settings/apps/new', + additionalOAuthSettings: [ + { + id: 'appSlug', + label: 'GitHub App Slug', + type: 'text', + required: true, + placeholder: 'comp-ai', + helpText: + "Your GitHub App's slug from its public page (github.com/apps/). Used to build the installation URL.", + token: '{APP_SLUG}', + }, + ], + }, + }, + + capabilities: ['checks'], + + services: [ + { + id: 'code-security', + name: 'Code Security', + description: 'Branch protection and code review policies', + enabledByDefault: true, + implemented: true, + }, + { + id: 'dependency-management', + name: 'Dependency Management', + description: 'Automated dependency updates and vulnerability scanning', + enabledByDefault: true, + implemented: true, + }, + ], + + // Reused verbatim from the `github` manifest — same objects, same check ids. + // Check results are keyed per (connectionId, checkId) and connections are + // provider-scoped, so sharing ids across the two manifests cannot collide. + checks: [ + branchProtectionCheck, + codeScanningCheck, + dependabotCheck, + sanitizedInputsCheck, + twoFactorAuthCheck, + ], + + isActive: true, +}; + +export default githubAppManifest; diff --git a/packages/integration-platform/src/registry/index.ts b/packages/integration-platform/src/registry/index.ts index 3e10442840..2d0dc43322 100644 --- a/packages/integration-platform/src/registry/index.ts +++ b/packages/integration-platform/src/registry/index.ts @@ -12,6 +12,7 @@ import { awsManifest } from '../manifests/aws'; import { azureManifest } from '../manifests/azure'; import { gcpManifest } from '../manifests/gcp'; import { manifest as githubManifest } from '../manifests/github'; +import { githubAppManifest } from '../manifests/github-app'; import { googleWorkspaceManifest } from '../manifests/google-workspace'; import { ripplingManifest } from '../manifests/rippling'; import { vercelManifest } from '../manifests/vercel'; @@ -141,6 +142,7 @@ const allManifests: IntegrationManifest[] = [ azureManifest, gcpManifest, githubManifest, + githubAppManifest, googleWorkspaceManifest, ripplingManifest, vercelManifest, diff --git a/packages/integration-platform/src/types.ts b/packages/integration-platform/src/types.ts index acbbef67da..7f6bed8845 100644 --- a/packages/integration-platform/src/types.ts +++ b/packages/integration-platform/src/types.ts @@ -32,6 +32,17 @@ export const OAuthConfigSchema = z.object({ * Default: true */ supportsRefreshToken: z.boolean().default(true), + /** + * GitHub App installation flow. When true, the "connect" step sends the user + * to a GitHub App *installation* URL (e.g. + * `https://github.com/apps/{slug}/installations/new`) instead of a standard + * OAuth authorize URL. Only `state` is appended to that URL — + * client_id/response_type/scope do not apply to an install URL. The OAuth + * `code` still comes back on the callback (with "Request user authorization + * during installation" enabled on the App), so token exchange is unchanged. + * Default: false (standard OAuth authorize flow). + */ + appInstallFlow: z.boolean().optional(), /** * Separate URL for token refresh (if different from tokenUrl). * Most providers use the same tokenUrl for both. From d756d4a444d21447ae2bcb60b647cdacfa36a173 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov <72318342+tofikwest@users.noreply.github.com> Date: Tue, 7 Jul 2026 15:54:59 -0400 Subject: [PATCH 3/7] fix(isms): production-deploy review follow-ups (CS-701) (#3365) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the ISMS issues cubic flagged on the main->release deploy PR (#3364): - hasApprovedVersion is now status-aware (currentVersionId != null OR status === 'approved'), so a document approved before versioning existed — whose legacy version row the migration drops — still reports an approved version instead of flipping to false. (P2) - ScopeForm re-seeds when the persisted draft content changes: its remount key now reflects draftNarrative, not just the published currentVersionId, so scope text refreshes after "Generate from platform data" / reload. (P3) - export DTO description now matches runtime: omitting versionId exports the current published version (or the draft if never published), not always the draft. (P3) - Approval banner renders "(v2)" instead of "v 2" (single JSX expression). (P3) Migration is intentionally left non-destructive (approvals preserved); the hasApprovedVersion fix makes that state self-consistent without a heavy backfill for pre-GA data. Tests: 319 api + 123 app ISMS tests passing; typecheck clean. Claude-Session: https://claude.ai/code/session_012CweXoSSEP89mX93u3Bdcp Co-authored-by: Claude Opus 4.8 (1M context) --- .../src/isms/dto/export-isms-document.dto.ts | 3 ++- apps/api/src/isms/isms.service.spec.ts | 23 +++++++++++++++++++ apps/api/src/isms/isms.service.ts | 10 +++++--- .../isms/components/IsmsApprovalSection.tsx | 4 ++-- .../documents/isms/components/ScopeClient.tsx | 9 +++++++- 5 files changed, 42 insertions(+), 7 deletions(-) diff --git a/apps/api/src/isms/dto/export-isms-document.dto.ts b/apps/api/src/isms/dto/export-isms-document.dto.ts index 632b1a2923..b91f83c877 100644 --- a/apps/api/src/isms/dto/export-isms-document.dto.ts +++ b/apps/api/src/isms/dto/export-isms-document.dto.ts @@ -12,7 +12,8 @@ export class ExportIsmsDocumentDto { @ApiPropertyOptional({ description: - 'Published version to export. Omit to export the current working draft; ' + + "Published version to export. Omit to export the document's current " + + 'published version (or the working draft if it has never been published); ' + 'provide a version id to download exactly what was approved at that version.', }) @IsOptional() diff --git a/apps/api/src/isms/isms.service.spec.ts b/apps/api/src/isms/isms.service.spec.ts index af4c4e5d0d..7d801200fa 100644 --- a/apps/api/src/isms/isms.service.spec.ts +++ b/apps/api/src/isms/isms.service.spec.ts @@ -89,6 +89,29 @@ describe('IsmsService ensureSetup', () => { expect(mockDb.ismsDocument.findMany).toHaveBeenCalledTimes(1); expect(result.documents).toHaveLength(1); }); + + it('reports hasApprovedVersion from a published version OR an approved status', async () => { + (mockDb.frameworkEditorFramework.findUnique as jest.Mock).mockResolvedValue({ + id: 'fw_1', + requirements: [], + }); + (mockDb.ismsDocument.findMany as jest.Mock).mockResolvedValueOnce([ + // Published version exists. + { id: 'd1', type: 'isms_scope', status: 'draft', requirementId: null, currentVersionId: 'isms_ver_1' }, + // Approved before versioning existed: no version row, but still approved. + { id: 'd2', type: 'leadership_commitment', status: 'approved', requirementId: null, currentVersionId: null }, + // Never approved. + { id: 'd3', type: 'objectives_plan', status: 'draft', requirementId: null, currentVersionId: null }, + ]); + + const result = await service.ensureSetup({ ...dto, canWrite: false }); + + expect(result.documents.map((d) => d.hasApprovedVersion)).toEqual([ + true, + true, + false, + ]); + }); }); describe('template-driven (templates seeded)', () => { diff --git a/apps/api/src/isms/isms.service.ts b/apps/api/src/isms/isms.service.ts index 15dbc19746..e0a73497ff 100644 --- a/apps/api/src/isms/isms.service.ts +++ b/apps/api/src/isms/isms.service.ts @@ -65,9 +65,13 @@ export class IsmsService { type: doc.type, status: doc.status, requirementId: doc.requirementId, - // A published version exists (the document has been approved at least - // once) — independent of whether the draft has since been edited. - hasApprovedVersion: doc.currentVersionId != null, + // The document has an approved artifact: either a published version row + // (approved under the versioning model) or `status === 'approved'` — the + // latter covers documents approved before versioning existed, whose legacy + // version rows were dropped by the migration and which capture a real + // versioned artifact on their next approval. + hasApprovedVersion: + doc.currentVersionId != null || doc.status === 'approved', })), }; } diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsApprovalSection.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsApprovalSection.tsx index d5bc90fd50..37ba6d724a 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsApprovalSection.tsx +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsApprovalSection.tsx @@ -128,8 +128,8 @@ export function IsmsApprovalSection({ v{publishedVersion} {' '} - stays live and exportable. Your changes are an in-progress draft (v - {nextDraftVersion}) — submit it for approval to publish. + stays live and exportable. Your changes are an in-progress draft{' '} + {`(v${nextDraftVersion})`} — submit it for approval to publish. )} diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/ScopeClient.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/ScopeClient.tsx index f3b006621d..391391dd10 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/components/ScopeClient.tsx +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/ScopeClient.tsx @@ -64,10 +64,17 @@ export function ScopeClient(props: ScopeClientProps) { }; const narrative = toScopeNarrative(document.draftNarrative); + // Re-seed the form (via remount) whenever the persisted draft content + // changes — e.g. after "Generate from platform data" or a reload — not + // just when the published version changes. ScopeForm reads its defaults + // at mount, so the key must reflect the draft, not currentVersionId alone. + const formKey = `${document.currentVersionId ?? 'draft'}:${JSON.stringify( + document.draftNarrative ?? {}, + )}`; return ( Date: Tue, 7 Jul 2026 16:21:35 -0400 Subject: [PATCH 4/7] fix(isms): serialize approve/decline to prevent concurrent double-publish (CS-701) (#3366) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Flagged on the production deploy PR (#3364): assertPendingApprovalBy runs before the transaction, so two racing approve() calls could both pass the guard. Root cause (verified): under READ COMMITTED a plain in-transaction status re-read does NOT serialize — the loser reads the still-committed `needs_review`. The @@unique([documentId, version]) constraint only turns the perfectly-simultaneous case into an error; the staggered case silently creates a second published version and overwrites currentVersionId. Fix: - approve() now takes a per-document advisory lock and then claims the approval atomically via updateMany({ where: status='needs_review' + approverId }) inside the transaction, aborting when count !== 1. Exactly one caller freezes a version; losers abort before any derivation/version creation. The pre-transaction guard is kept for the normal-case specific error messages. - decline() gets the same atomic claim, so an approve/decline race can no longer leave a "declined" document that still owns a live currentVersionId. - Renamed the shared advisory-lock helper lockDocumentForPositions -> lockDocument (it now serializes approval too, not just register-row position allocation); updated the four register-create callers. Adversarially audited the whole approve/publish/version/export flow for related concurrency + IDOR issues; getVersionExport org-scoping, S3 keys, snapshot parsing, currentVersionId integrity and the migration paths verified safe. The narrow edit-during-approve window for register update/delete is low-severity/self-healing and left as a follow-up. Tests: 321 api ISMS tests passing (incl. new approve + decline race tests); typecheck clean. Claude-Session: https://claude.ai/code/session_012CweXoSSEP89mX93u3Bdcp Co-authored-by: Claude Opus 4.8 (1M context) --- .../src/isms/isms-context-issue.service.ts | 6 +-- .../src/isms/isms-interested-party.service.ts | 6 +-- apps/api/src/isms/isms-objective.service.ts | 6 +-- apps/api/src/isms/isms-requirement.service.ts | 6 +-- .../api/src/isms/isms.service.approve.spec.ts | 50 ++++++++++++++++--- .../src/isms/isms.service.lifecycle.spec.ts | 29 ++++++++--- apps/api/src/isms/isms.service.ts | 48 +++++++++++++++--- apps/api/src/isms/utils/document-lock.ts | 11 ++-- 8 files changed, 125 insertions(+), 37 deletions(-) diff --git a/apps/api/src/isms/isms-context-issue.service.ts b/apps/api/src/isms/isms-context-issue.service.ts index 7d2fa066be..daeb45c23b 100644 --- a/apps/api/src/isms/isms-context-issue.service.ts +++ b/apps/api/src/isms/isms-context-issue.service.ts @@ -2,7 +2,7 @@ import { Injectable, NotFoundException } from '@nestjs/common'; import { db } from '@db'; import type { Prisma } from '@db'; import { invalidateApprovalIfNeeded } from './utils/approval'; -import { lockDocumentForPositions } from './utils/document-lock'; +import { lockDocument } from './utils/document-lock'; import type { CreateContextIssueInput, UpdateContextIssueInput, @@ -28,7 +28,7 @@ export class IsmsContextIssueService { await this.requireDocument({ documentId, organizationId }); return db.$transaction(async (tx) => { - await lockDocumentForPositions(tx, documentId); + await lockDocument(tx, documentId); const position = dto.position ?? (await this.nextPosition({ tx, documentId })); await invalidateApprovalIfNeeded({ tx, documentId }); @@ -92,7 +92,7 @@ export class IsmsContextIssueService { /** * Next position uses max(position)+1 so it survives deletes. Runs on the * transaction client; the create first takes a per-document advisory lock - * (lockDocumentForPositions) so concurrent creates can't read the same max. + * (lockDocument) so concurrent creates can't read the same max. */ private async nextPosition({ tx, diff --git a/apps/api/src/isms/isms-interested-party.service.ts b/apps/api/src/isms/isms-interested-party.service.ts index 58b3dcbf8e..70141f275f 100644 --- a/apps/api/src/isms/isms-interested-party.service.ts +++ b/apps/api/src/isms/isms-interested-party.service.ts @@ -2,7 +2,7 @@ import { Injectable, NotFoundException } from '@nestjs/common'; import { db } from '@db'; import type { Prisma } from '@db'; import { invalidateApprovalIfNeeded } from './utils/approval'; -import { lockDocumentForPositions } from './utils/document-lock'; +import { lockDocument } from './utils/document-lock'; import type { CreateInterestedPartyInput, UpdateInterestedPartyInput, @@ -28,7 +28,7 @@ export class IsmsInterestedPartyService { await this.requireDocument({ documentId, organizationId }); return db.$transaction(async (tx) => { - await lockDocumentForPositions(tx, documentId); + await lockDocument(tx, documentId); const position = dto.position ?? (await this.nextPosition({ tx, documentId })); await invalidateApprovalIfNeeded({ tx, documentId }); @@ -89,7 +89,7 @@ export class IsmsInterestedPartyService { /** * Next position uses max(position)+1 so it survives deletes. Runs on the * transaction client; the create first takes a per-document advisory lock - * (lockDocumentForPositions) so concurrent creates can't read the same max. + * (lockDocument) so concurrent creates can't read the same max. */ private async nextPosition({ tx, diff --git a/apps/api/src/isms/isms-objective.service.ts b/apps/api/src/isms/isms-objective.service.ts index 98a5986930..512232ec5d 100644 --- a/apps/api/src/isms/isms-objective.service.ts +++ b/apps/api/src/isms/isms-objective.service.ts @@ -2,7 +2,7 @@ import { Injectable, NotFoundException } from '@nestjs/common'; import { db } from '@db'; import type { Prisma } from '@db'; import { invalidateApprovalIfNeeded } from './utils/approval'; -import { lockDocumentForPositions } from './utils/document-lock'; +import { lockDocument } from './utils/document-lock'; import type { CreateObjectiveInput, UpdateObjectiveInput, @@ -31,7 +31,7 @@ export class IsmsObjectiveService { }); return db.$transaction(async (tx) => { - await lockDocumentForPositions(tx, documentId); + await lockDocument(tx, documentId); const position = dto.position ?? (await this.nextPosition({ tx, documentId })); await invalidateApprovalIfNeeded({ tx, documentId }); @@ -118,7 +118,7 @@ export class IsmsObjectiveService { /** * Next position uses max(position)+1 so it survives deletes. Runs on the * transaction client; the create first takes a per-document advisory lock - * (lockDocumentForPositions) so concurrent creates can't read the same max. + * (lockDocument) so concurrent creates can't read the same max. */ private async nextPosition({ tx, diff --git a/apps/api/src/isms/isms-requirement.service.ts b/apps/api/src/isms/isms-requirement.service.ts index 568219d3bf..9a16673bc5 100644 --- a/apps/api/src/isms/isms-requirement.service.ts +++ b/apps/api/src/isms/isms-requirement.service.ts @@ -6,7 +6,7 @@ import { import { db } from '@db'; import type { Prisma } from '@db'; import { invalidateApprovalIfNeeded } from './utils/approval'; -import { lockDocumentForPositions } from './utils/document-lock'; +import { lockDocument } from './utils/document-lock'; import type { CreateRequirementInput, UpdateRequirementInput, @@ -37,7 +37,7 @@ export class IsmsRequirementService { } return db.$transaction(async (tx) => { - await lockDocumentForPositions(tx, documentId); + await lockDocument(tx, documentId); const position = dto.position ?? (await this.nextPosition({ tx, documentId })); await invalidateApprovalIfNeeded({ tx, documentId }); @@ -123,7 +123,7 @@ export class IsmsRequirementService { /** * Next position uses max(position)+1 so it survives deletes. Runs on the * transaction client; the create first takes a per-document advisory lock - * (lockDocumentForPositions) so concurrent creates can't read the same max. + * (lockDocument) so concurrent creates can't read the same max. */ private async nextPosition({ tx, diff --git a/apps/api/src/isms/isms.service.approve.spec.ts b/apps/api/src/isms/isms.service.approve.spec.ts index cdedf89f6e..fc6e203815 100644 --- a/apps/api/src/isms/isms.service.approve.spec.ts +++ b/apps/api/src/isms/isms.service.approve.spec.ts @@ -122,7 +122,9 @@ describe('IsmsService.approve (CS-701 versioning)', () => { .mockResolvedValueOnce({ id: 'doc_1', status: 'approved' }); mockCollect.mockResolvedValue(platformData); const tx = { + $executeRaw: jest.fn().mockResolvedValue(0), ismsDocument: { + updateMany: jest.fn().mockResolvedValue({ count: 1 }), findUniqueOrThrow: jest.fn().mockResolvedValue(reloaded), update: jest.fn().mockResolvedValue({}), }, @@ -155,18 +157,26 @@ describe('IsmsService.approve (CS-701 versioning)', () => { expect(mockUpdateDraft).toHaveBeenCalledWith( expect.objectContaining({ tx, documentId: 'doc_1' }), ); + // The approval is claimed atomically (serializes concurrent approvals): only + // matches while still awaiting this member's review. + expect(tx.ismsDocument.updateMany).toHaveBeenCalledWith({ + where: { + id: 'doc_1', + organizationId: 'org_1', + status: 'needs_review', + approverId: 'mem_1', + }, + data: expect.objectContaining({ status: 'approved', declinedAt: null }), + }); // A published version is created from the reloaded (re-derived) document. expect(versionService.createPublishedVersion).toHaveBeenCalledWith( expect.objectContaining({ tx, document: reloaded, memberId: 'mem_1' }), ); - // The document is promoted to the freshly-created version. + // The document is promoted to the freshly-created version (status was already + // set by the atomic claim above). expect(tx.ismsDocument.update).toHaveBeenCalledWith({ where: { id: 'doc_1' }, - data: expect.objectContaining({ - status: 'approved', - declinedAt: null, - currentVersionId: 'isms_ver_1', - }), + data: { currentVersionId: 'isms_ver_1' }, }); // Renders + uploads happen AFTER the transaction (Policies pattern). expect(versionService.publishRenders).toHaveBeenCalledWith( @@ -178,4 +188,32 @@ describe('IsmsService.approve (CS-701 versioning)', () => { }), ); }); + + it('aborts without creating a version when a concurrent approval already won the claim', async () => { + (mockDb.member.findFirst as jest.Mock).mockResolvedValue({ id: 'mem_1' }); + (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ + id: 'doc_1', + status: 'needs_review', + approverId: 'mem_1', + frameworkId: 'fw_1', + type: 'context_of_organization', + }); + mockCollect.mockResolvedValue(platformData); + const tx = { + $executeRaw: jest.fn().mockResolvedValue(0), + ismsDocument: { + // The racing approval already flipped status, so the claim matches 0 rows. + updateMany: jest.fn().mockResolvedValue({ count: 0 }), + findUniqueOrThrow: jest.fn(), + update: jest.fn(), + }, + }; + (mockDb.$transaction as jest.Mock).mockImplementation((cb) => cb(tx)); + + await expect(service.approve(args)).rejects.toThrow(BadRequestException); + // No derivation, no version creation once the claim fails. + expect(mockRunDerivation).not.toHaveBeenCalled(); + expect(versionService.createPublishedVersion).not.toHaveBeenCalled(); + expect(tx.ismsDocument.findUniqueOrThrow).not.toHaveBeenCalled(); + }); }); diff --git a/apps/api/src/isms/isms.service.lifecycle.spec.ts b/apps/api/src/isms/isms.service.lifecycle.spec.ts index 80e1311c5e..2defb6ba8f 100644 --- a/apps/api/src/isms/isms.service.lifecycle.spec.ts +++ b/apps/api/src/isms/isms.service.lifecycle.spec.ts @@ -14,6 +14,7 @@ jest.mock('@db', () => ({ ismsDocument: { findFirst: jest.fn(), update: jest.fn(), + updateMany: jest.fn(), }, member: { findFirst: jest.fn() }, $transaction: jest.fn(), @@ -151,23 +152,39 @@ describe('IsmsService document lifecycle', () => { await expect(service.decline(args)).rejects.toThrow(ForbiddenException); }); - it('sets declined status and declinedAt', async () => { + it('sets declined status and declinedAt via an atomic claim', async () => { (mockDb.member.findFirst as jest.Mock).mockResolvedValue({ id: 'mem_1' }); (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ id: 'doc_1', status: 'needs_review', approverId: 'mem_1', }); - (mockDb.ismsDocument.update as jest.Mock).mockResolvedValue({ - id: 'doc_1', - status: 'declined', - }); + (mockDb.ismsDocument.updateMany as jest.Mock).mockResolvedValue({ count: 1 }); await service.decline(args); - const call = (mockDb.ismsDocument.update as jest.Mock).mock.calls[0][0]; + const call = (mockDb.ismsDocument.updateMany as jest.Mock).mock.calls[0][0]; + // Guarded transition: only matches while awaiting this member's review. + expect(call.where).toEqual({ + id: 'doc_1', + organizationId: 'org_1', + status: 'needs_review', + approverId: 'mem_1', + }); expect(call.data.status).toBe('declined'); expect(call.data.declinedAt).toBeInstanceOf(Date); }); + + it('aborts when a concurrent approve already won (claim matches 0 rows)', async () => { + (mockDb.member.findFirst as jest.Mock).mockResolvedValue({ id: 'mem_1' }); + (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ + id: 'doc_1', + status: 'needs_review', + approverId: 'mem_1', + }); + (mockDb.ismsDocument.updateMany as jest.Mock).mockResolvedValue({ count: 0 }); + + await expect(service.decline(args)).rejects.toThrow(BadRequestException); + }); }); }); diff --git a/apps/api/src/isms/isms.service.ts b/apps/api/src/isms/isms.service.ts index e0a73497ff..39777c115c 100644 --- a/apps/api/src/isms/isms.service.ts +++ b/apps/api/src/isms/isms.service.ts @@ -11,6 +11,7 @@ import { collectPlatformData } from './documents/data-source'; import { runDerivation } from './documents/generate'; import { updateDraftSnapshot } from './utils/draft-snapshot'; import { EXPORT_DOCUMENT_INCLUDE } from './utils/export-payload'; +import { lockDocument } from './utils/document-lock'; import { IsmsVersionService } from './isms-version.service'; /** @@ -224,6 +225,29 @@ export class IsmsService { // currentVersion. Editing afterwards reverts status to draft but leaves this // published version live and exportable (CS-701). const published = await db.$transaction(async (tx) => { + // 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); + + // 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 + // while the document is still awaiting THIS member's review, so exactly one + // caller proceeds; a loser matches zero rows and aborts before creating a + // version. (A plain in-transaction re-read would not serialize.) + const claim = await tx.ismsDocument.updateMany({ + where: { + id: documentId, + organizationId, + status: 'needs_review', + approverId: member.id, + }, + data: { status: 'approved', approvedAt: now, declinedAt: null }, + }); + if (claim.count !== 1) { + throw new BadRequestException('Document is not pending your approval'); + } + // Re-derive in the same transaction so the persisted rows and the frozen // snapshot come from one pass (otherwise the approved content can drift). await runDerivation({ @@ -250,12 +274,7 @@ export class IsmsService { await tx.ismsDocument.update({ where: { id: documentId }, - data: { - status: 'approved', - approvedAt: now, - declinedAt: null, - currentVersionId: result.versionId, - }, + data: { currentVersionId: result.versionId }, }); return result; }); @@ -286,10 +305,23 @@ export class IsmsService { const document = await this.requireDocument({ documentId, organizationId }); this.assertPendingApprovalBy({ document, member }); - return db.ismsDocument.update({ - where: { id: documentId }, + // Atomically claim the decline so it can't race an approve (both pass the + // pre-transaction guard). Only matches while still awaiting this member's + // review, so a concurrent approve that already won leaves zero rows here. + const claim = await db.ismsDocument.updateMany({ + where: { + id: documentId, + organizationId, + status: 'needs_review', + approverId: member.id, + }, data: { status: 'declined', declinedAt: new Date() }, }); + if (claim.count !== 1) { + throw new BadRequestException('Document is not pending your approval'); + } + + return this.getDocument({ documentId, organizationId }); } /** diff --git a/apps/api/src/isms/utils/document-lock.ts b/apps/api/src/isms/utils/document-lock.ts index 22fef4f602..1ffa527ccd 100644 --- a/apps/api/src/isms/utils/document-lock.ts +++ b/apps/api/src/isms/utils/document-lock.ts @@ -1,13 +1,14 @@ import type { Prisma } from '@db'; /** - * Serialize register-row position allocation for a single document. The Postgres + * Serialize concurrent writes for a single ISMS document. The Postgres * transaction-scoped advisory lock (keyed on the document id) is held until the - * surrounding transaction commits, so two concurrent creates can't both read the - * same max(position) and persist duplicate ordering keys. Call this inside the - * create transaction, before computing the next position. + * surrounding transaction commits, so two transactions touching the same document + * run one at a time. Used by register-row creates (so concurrent creates can't + * read the same max(position)) and by approve (so concurrent approvals can't both + * freeze a published version). Call this first, inside the transaction. */ -export async function lockDocumentForPositions( +export async function lockDocument( tx: Prisma.TransactionClient, documentId: string, ): Promise { From 5c88a3dc84ec45f2ca1f3970ac3fbde61b775af7 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov <72318342+tofikwest@users.noreply.github.com> Date: Tue, 7 Jul 2026 16:40:21 -0400 Subject: [PATCH 5/7] fix(isms): serialize approve against ALL register/content edits (central lock) (#3367) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the approve-race fix: cubic correctly noted the advisory lock only protected paths that also take it, so a register UPDATE/DELETE (or narrative / control / regenerate) could still interleave while approve() loads and freezes the version, leaving an approved version that omits the edit — or an edit that never invalidated the approval. Enforced centrally instead of per-call-site: invalidateApprovalIfNeeded now takes the per-document advisory lock as its first action. Every content-mutation path already calls it (register create/update/delete, narrative save, control add/remove, regenerate), so all edits now serialize against approve() (which takes the same lock). The lock is transaction-scoped and re-entrant, so create paths that already acquire it for position allocation are unaffected. Adds utils/approval.spec.ts (lock-before-status ordering + revert/no-op behavior) and $executeRaw to the two tx mocks that exercise the real invalidate. Tests: 324 api ISMS tests passing; typecheck clean. Claude-Session: https://claude.ai/code/session_012CweXoSSEP89mX93u3Bdcp Co-authored-by: Claude Opus 4.8 (1M context) --- .../isms-document-control.service.spec.ts | 2 + .../src/isms/isms-narrative.service.spec.ts | 2 + apps/api/src/isms/utils/approval.spec.ts | 62 +++++++++++++++++++ apps/api/src/isms/utils/approval.ts | 12 ++++ 4 files changed, 78 insertions(+) create mode 100644 apps/api/src/isms/utils/approval.spec.ts 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 c91cb7c11d..b209192048 100644 --- a/apps/api/src/isms/isms-document-control.service.spec.ts +++ b/apps/api/src/isms/isms-document-control.service.spec.ts @@ -14,6 +14,8 @@ jest.mock('@db', () => { createMany: jest.fn(), deleteMany: jest.fn(), }, + // Advisory lock taken by invalidateApprovalIfNeeded (serializes vs approve). + $executeRaw: jest.fn(), // Run the callback with the same mock as the transaction client. $transaction: jest.fn((cb: (tx: unknown) => unknown) => cb(db)), }; diff --git a/apps/api/src/isms/isms-narrative.service.spec.ts b/apps/api/src/isms/isms-narrative.service.spec.ts index 2770acd489..c1deacca14 100644 --- a/apps/api/src/isms/isms-narrative.service.spec.ts +++ b/apps/api/src/isms/isms-narrative.service.spec.ts @@ -9,6 +9,8 @@ jest.mock('@db', () => { findUnique: jest.fn(), update: jest.fn(), }, + // Advisory lock taken by invalidateApprovalIfNeeded (serializes vs approve). + $executeRaw: jest.fn(), // Run the callback with the same mock as the transaction client. $transaction: jest.fn((cb: (tx: unknown) => unknown) => cb(db)), }; diff --git a/apps/api/src/isms/utils/approval.spec.ts b/apps/api/src/isms/utils/approval.spec.ts new file mode 100644 index 0000000000..9ca4a07801 --- /dev/null +++ b/apps/api/src/isms/utils/approval.spec.ts @@ -0,0 +1,62 @@ +import { invalidateApprovalIfNeeded } from './approval'; + +/** + * invalidateApprovalIfNeeded is the central serialization point: every ISMS + * content-mutation path calls it, and it must take the per-document advisory lock + * (so edits serialize against approve()) BEFORE reading/altering status. + */ +function makeTx() { + return { + $executeRaw: jest.fn().mockResolvedValue(0), + ismsDocument: { + findUnique: jest.fn(), + update: jest.fn().mockResolvedValue({}), + }, + }; +} + +describe('invalidateApprovalIfNeeded', () => { + it('acquires the per-document advisory lock before reading status', async () => { + const tx = makeTx(); + tx.ismsDocument.findUnique.mockResolvedValue({ status: 'draft' }); + + await invalidateApprovalIfNeeded({ + tx: tx as never, + documentId: 'doc_1', + }); + + expect(tx.$executeRaw).toHaveBeenCalledTimes(1); + // Lock is taken before the status read (serializes vs approve()). + expect(tx.$executeRaw.mock.invocationCallOrder[0]).toBeLessThan( + tx.ismsDocument.findUnique.mock.invocationCallOrder[0], + ); + }); + + it('reverts an approved document to draft (clearing approver + approvedAt)', async () => { + const tx = makeTx(); + tx.ismsDocument.findUnique.mockResolvedValue({ status: 'approved' }); + + await invalidateApprovalIfNeeded({ + tx: tx as never, + documentId: 'doc_1', + }); + + expect(tx.ismsDocument.update).toHaveBeenCalledWith({ + where: { id: 'doc_1' }, + data: { status: 'draft', approvedAt: null, approverId: null }, + }); + }); + + it('is a no-op for a non-approved document (but still locked)', async () => { + const tx = makeTx(); + tx.ismsDocument.findUnique.mockResolvedValue({ status: 'needs_review' }); + + await invalidateApprovalIfNeeded({ + tx: tx as never, + documentId: 'doc_1', + }); + + expect(tx.$executeRaw).toHaveBeenCalledTimes(1); + expect(tx.ismsDocument.update).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/api/src/isms/utils/approval.ts b/apps/api/src/isms/utils/approval.ts index 186bfcae04..6b0db62544 100644 --- a/apps/api/src/isms/utils/approval.ts +++ b/apps/api/src/isms/utils/approval.ts @@ -1,4 +1,5 @@ import type { Prisma } from '@db'; +import { lockDocument } from './document-lock'; /** * Editing an approved ISMS document invalidates its sign-off: revert it to draft @@ -6,6 +7,15 @@ import type { Prisma } from '@db'; * touches the document when it is currently `approved`; a no-op otherwise. Must * run inside the same transaction as the content write so a failed write does not * leave the document reverted to draft without the new content. + * + * Every content-mutation path (register create/update/delete, narrative save, + * control link add/remove, regenerate) calls this, so acquiring the per-document + * advisory lock here centrally serializes ALL edits against approve() — which + * takes the same lock. Without it, an edit could interleave while approve() is + * loading + freezing the version under READ COMMITTED, leaving an approved version + * that omits the edit (or an edit that never invalidated the approval). The lock + * is transaction-scoped and re-entrant, so create paths that already take it for + * position allocation are unaffected. */ export async function invalidateApprovalIfNeeded({ tx, @@ -14,6 +24,8 @@ export async function invalidateApprovalIfNeeded({ tx: Prisma.TransactionClient; documentId: string; }): Promise { + await lockDocument(tx, documentId); + const document = await tx.ismsDocument.findUnique({ where: { id: documentId }, select: { status: true }, From a21f8667fcbc704c4d3dd2af0380d23dceb9761d Mon Sep 17 00:00:00 2001 From: Tofik Hasanov <72318342+tofikwest@users.noreply.github.com> Date: Tue, 7 Jul 2026 16:58:04 -0400 Subject: [PATCH 6/7] fix(isms): lock before writing control links so edits serialize with approve (#3369) 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. Claude-Session: https://claude.ai/code/session_012CweXoSSEP89mX93u3Bdcp Co-authored-by: Claude Opus 4.8 (1M context) --- .../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 }, }); From 9c6f6faf19aa2dcac750e02f48aeefb0c502d471 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov <72318342+tofikwest@users.noreply.github.com> Date: Tue, 7 Jul 2026 17:15:45 -0400 Subject: [PATCH 7/7] fix(integrations): don't persist unverified GitHub installation_id (#3370) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The GitHub App install callback returns an `installation_id`, which GitHub's own docs document as not trustworthy (it can be spoofed on the callback URL). The previous code stored it on the connection metadata, but nothing reads it anywhere, so it was unused data that a future installation-token flow could later trust unsafely. Remove the storage (and its now-unused query fields + test). If GitHub App installation-token auth is added later, resolve the installation via `GET /user/installations` with the user token — which also proves the user owns it — instead of trusting the callback value. No behavior change: the integration authenticates with the OAuth user token, which is unaffected. Claude-Session: https://claude.ai/code/session_01XyttkYNbkdYRp7DBKNuFVH Co-authored-by: Claude Opus 4.8 (1M context) --- .../controllers/oauth.controller.spec.ts | 88 ------------------- .../controllers/oauth.controller.ts | 31 ++----- 2 files changed, 7 insertions(+), 112 deletions(-) diff --git a/apps/api/src/integration-platform/controllers/oauth.controller.spec.ts b/apps/api/src/integration-platform/controllers/oauth.controller.spec.ts index 619cb13dc5..cf3fdefc36 100644 --- a/apps/api/src/integration-platform/controllers/oauth.controller.spec.ts +++ b/apps/api/src/integration-platform/controllers/oauth.controller.spec.ts @@ -692,94 +692,6 @@ describe('OAuthController', () => { fetchSpy.mockRestore(); }); - it('persists installation_id on the connection for the GitHub App install flow', async () => { - const futureDate = new Date(Date.now() + 600000); - mockOAuthStateRepository.findByState.mockResolvedValue({ - state: 'valid_gh_state', - providerSlug: 'github-app', - organizationId: 'org_1', - userId: 'user_1', - codeVerifier: null, - redirectUrl: null, - expiresAt: futureDate, - }); - mockedGetManifest.mockReturnValue({ - id: 'github-app', - name: 'GitHub App', - category: 'Development', - auth: { - type: 'oauth2', - config: { - authorizeUrl: - 'https://github.com/apps/{APP_SLUG}/installations/new', - tokenUrl: 'https://github.com/login/oauth/access_token', - appInstallFlow: true, - }, - }, - capabilities: [], - isActive: true, - } as never); - mockOAuthCredentialsService.getCredentials.mockResolvedValue({ - clientId: 'client_123', - clientSecret: 'secret_456', - scopes: [], - source: 'platform', - }); - mockProviderRepository.findBySlug.mockResolvedValue({ - id: 'provider_gh', - }); - mockConnectionRepository.findByProviderAndOrg.mockResolvedValue({ - id: 'conn_gh', - metadata: {}, - variables: {}, - lastSyncAt: null, - }); - mockConnectionRepository.update.mockResolvedValue({ - id: 'conn_gh', - metadata: {}, - variables: {}, - lastSyncAt: null, - }); - mockConnectionService.activateConnection.mockResolvedValue({ - id: 'conn_gh', - }); - - const fetchSpy = jest.spyOn(global, 'fetch').mockResolvedValue({ - ok: true, - status: 200, - json: () => Promise.resolve({ access_token: 'gh_user_token' }), - text: () => Promise.resolve(''), - } as unknown as Response); - - await controller.oauthCallback( - { - code: 'auth_code', - state: 'valid_gh_state', - installation_id: '987654', - setup_action: 'install', - }, - mockRequest, - mockResponse, - ); - - await new Promise((resolve) => setImmediate(resolve)); - - expect(mockConnectionRepository.update).toHaveBeenCalledWith( - 'conn_gh', - expect.objectContaining({ - metadata: expect.objectContaining({ - githubInstallationId: '987654', - githubSetupAction: 'install', - }), - }), - ); - const redirectUrl = (mockResponse.redirect as jest.Mock).mock.calls[0][0]; - expect(redirectUrl).toContain('success=true'); - expect(redirectUrl).toContain('provider=github-app'); - - fetchSpy.mockRestore(); - }); - describe('session defense-in-depth', () => { const futureDate = new Date(Date.now() + 600000); const validState = { diff --git a/apps/api/src/integration-platform/controllers/oauth.controller.ts b/apps/api/src/integration-platform/controllers/oauth.controller.ts index 4f3436dac8..8007d2249b 100644 --- a/apps/api/src/integration-platform/controllers/oauth.controller.ts +++ b/apps/api/src/integration-platform/controllers/oauth.controller.ts @@ -39,9 +39,6 @@ interface OAuthCallbackQuery { state: string; error?: string; error_description?: string; - // GitHub App installation flow returns these alongside `code`. - installation_id?: string; - setup_action?: string; } @Controller({ path: 'integrations/oauth', version: '1' }) @@ -425,27 +422,13 @@ export class OAuthController { }); } - // GitHub App installation flow: persist the installation id (and setup - // action) on the connection. The token stored above is a user-to-server - // token, but recording the installation id means a future server-to-server - // (installation access token) upgrade needs no re-connect. - if (query.installation_id) { - const metadata = - connection.metadata && - typeof connection.metadata === 'object' && - !Array.isArray(connection.metadata) - ? (connection.metadata as Record) - : {}; - connection = await this.connectionRepository.update(connection.id, { - metadata: { - ...metadata, - githubInstallationId: query.installation_id, - ...(query.setup_action - ? { githubSetupAction: query.setup_action } - : {}), - }, - }); - } + // NOTE: GitHub's App install flow returns an `installation_id` on this + // callback, but that value is attacker-spoofable — GitHub's own docs say + // not to rely on it — so we intentionally do NOT persist it here, and + // nothing reads it today. If/when GitHub App installation-token auth is + // added, resolve the installation via `GET /user/installations` with the + // user token (which also proves the user owns it) rather than trusting the + // value from this callback. await this.connectionService.activateConnection(connection.id);