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..cf3fdefc36 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', () => { diff --git a/apps/api/src/integration-platform/controllers/oauth.controller.ts b/apps/api/src/integration-platform/controllers/oauth.controller.ts index 29fec34596..8007d2249b 100644 --- a/apps/api/src/integration-platform/controllers/oauth.controller.ts +++ b/apps/api/src/integration-platform/controllers/oauth.controller.ts @@ -182,6 +182,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 +325,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 +396,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 +422,14 @@ export class OAuthController { }); } + // 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); // Provider-specific post-OAuth actions @@ -643,5 +696,4 @@ export class OAuthController { } return url.toString(); } - } 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..b91f83c877 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,14 @@ export class ExportIsmsDocumentDto { }) @IsIn(['pdf', 'docx']) format!: 'pdf' | 'docx'; + + @ApiPropertyOptional({ + description: + "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() + @IsString() + versionId?: string; } 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-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-document-control.service.spec.ts b/apps/api/src/isms/isms-document-control.service.spec.ts index c91cb7c11d..7ffa17c4d9 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)), }; @@ -88,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 }, }); 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-narrative.service.spec.ts b/apps/api/src/isms/isms-narrative.service.spec.ts index a1e6db166e..c1deacca14 100644 --- a/apps/api/src/isms/isms-narrative.service.spec.ts +++ b/apps/api/src/isms/isms-narrative.service.spec.ts @@ -9,11 +9,8 @@ jest.mock('@db', () => { findUnique: jest.fn(), update: jest.fn(), }, - ismsDocumentVersion: { - findFirst: jest.fn(), - create: 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)), }; @@ -36,6 +33,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 +77,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 +93,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 +114,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 +121,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-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-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..fc6e203815 --- /dev/null +++ b/apps/api/src/isms/isms.service.approve.spec.ts @@ -0,0 +1,219 @@ +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 = { + $executeRaw: jest.fn().mockResolvedValue(0), + ismsDocument: { + updateMany: jest.fn().mockResolvedValue({ count: 1 }), + 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' }), + ); + // 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 (status was already + // set by the atomic claim above). + expect(tx.ismsDocument.update).toHaveBeenCalledWith({ + where: { id: 'doc_1' }, + data: { 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, + }), + ); + }); + + 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.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..2defb6ba8f 100644 --- a/apps/api/src/isms/isms.service.lifecycle.spec.ts +++ b/apps/api/src/isms/isms.service.lifecycle.spec.ts @@ -5,40 +5,37 @@ 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: { findFirst: jest.fn(), update: jest.fn(), + updateMany: 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/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 +62,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 +77,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,7 +120,7 @@ describe('IsmsService document lifecycle', () => { }); }); - describe('approve', () => { + describe('decline', () => { const args = { documentId: 'doc_1', organizationId: 'org_1', @@ -130,29 +129,17 @@ describe('IsmsService document lifecycle', () => { it('throws NotFoundException when member not found', async () => { (mockDb.member.findFirst as jest.Mock).mockResolvedValue(null); - await expect(service.approve(args)).rejects.toThrow(NotFoundException); + await expect(service.decline(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', + status: 'approved', 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); + await expect(service.decline(args)).rejects.toThrow(BadRequestException); }); it('throws ForbiddenException when not the assigned approver', async () => { @@ -161,120 +148,43 @@ describe('IsmsService document lifecycle', () => { 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', - 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.decline(args)).rejects.toThrow(NotFoundException); + await expect(service.decline(args)).rejects.toThrow(ForbiddenException); }); - it('throws BadRequestException when document is not pending approval', 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: 'approved', + status: 'needs_review', approverId: 'mem_1', }); - await expect(service.decline(args)).rejects.toThrow(BadRequestException); - }); + (mockDb.ismsDocument.updateMany as jest.Mock).mockResolvedValue({ count: 1 }); - 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({ + await service.decline(args); + + 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_other', + approverId: 'mem_1', }); - await expect(service.decline(args)).rejects.toThrow(ForbiddenException); + expect(call.data.status).toBe('declined'); + expect(call.data.declinedAt).toBeInstanceOf(Date); }); - it('sets declined status and declinedAt', async () => { + 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.update as jest.Mock).mockResolvedValue({ - id: 'doc_1', - status: 'declined', - }); - - await service.decline(args); + (mockDb.ismsDocument.updateMany as jest.Mock).mockResolvedValue({ count: 0 }); - const call = (mockDb.ismsDocument.update as jest.Mock).mock.calls[0][0]; - expect(call.data.status).toBe('declined'); - expect(call.data.declinedAt).toBeInstanceOf(Date); + await expect(service.decline(args)).rejects.toThrow(BadRequestException); }); }); }); diff --git a/apps/api/src/isms/isms.service.spec.ts b/apps/api/src/isms/isms.service.spec.ts index d8a1f34410..7d801200fa 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, @@ -88,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 4d92c33d3f..39777c115c 100644 --- a/apps/api/src/isms/isms.service.ts +++ b/apps/api/src/isms/isms.service.ts @@ -9,7 +9,10 @@ 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 { lockDocument } from './utils/document-lock'; +import { IsmsVersionService } from './isms-version.service'; /** * ISMS foundational document lifecycle: setup, retrieval and sign-off. Context @@ -18,6 +21,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 +66,13 @@ export class IsmsService { type: doc.type, status: doc.status, requirementId: doc.requirementId, - hasApprovedVersion: doc.status === 'approved', + // 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', })), }; } @@ -136,7 +147,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 +219,37 @@ export class IsmsService { organizationId, frameworkId: document.frameworkId, }); + const now = new Date(); + + // Freeze the draft into a new immutable published version and promote it to + // currentVersion. Editing afterwards reverts status to draft but leaves this + // published version live and exportable (CS-701). + const published = await db.$transaction(async (tx) => { + // 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'); + } - 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). + // 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 +258,35 @@ 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: { 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 }); @@ -239,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/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 }, 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 { 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..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 @@ -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..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 @@ -63,12 +63,18 @@ export function ScopeClient(props: ScopeClientProps) { } }; - const versions = Array.isArray(document.versions) ? document.versions : []; - const narrative = toScopeNarrative(versions[0]?.narrative); + 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 (
{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) --- 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.