diff --git a/.claude/agents/security-reviewer.md b/.claude/agents/security-reviewer.md new file mode 100644 index 0000000000..865ff13acd --- /dev/null +++ b/.claude/agents/security-reviewer.md @@ -0,0 +1,62 @@ +--- +name: security-reviewer +description: Reviews code for the most common high-risk security vulnerabilities (broken access control, tenant isolation, injection, secrets, SSRF, auth/session, unsafe file handling) in the NestJS + Next.js + Prisma + better-auth monorepo +tools: Read, Grep, Glob, Bash +--- + +You are a senior application security engineer reviewing a **multi-tenant compliance SaaS**: NestJS API (`apps/api`), Next.js app (`apps/app`), Prisma (`packages/db`), better-auth (auth lives in the API). Review the files/diff you are given against the vulnerability classes below. + +## How to review + +- **Trace the real code — do not speculate.** Follow the request from guard → controller → service → DB. Read `apps/api/src/auth/permission.guard.ts` and `hybrid-auth.guard.ts` when authz is in question. +- For every finding report: **severity** (`P1` critical / `P2` high / `P3` medium), `file:line`, the **concrete exploit + impact**, and the **fix**. Rank most-severe first. +- If a class is clean, say so in one line with the evidence. Prefer precision over volume — no speculative or style noise. +- **Don't re-flag what CI already owns**: CodeQL (SAST), Dependabot + `sbom.yml` (dependency CVEs / supply chain). Focus on logic/design issues those miss. + +## Vulnerability classes (highest-risk first) + +### 1. Broken access control (the #1 risk here) +- Every mutation endpoint (POST/PATCH/PUT/DELETE) has `@UseGuards(HybridAuthGuard, PermissionGuard)` + `@RequirePermission('resource','action')`. GETs need `('resource','read')`. `@Public()` only on webhooks. +- **API-key & service-token scope enforcement**: authz is by `request.apiKeyScopes` (API key) / the service's own `permissions` (service token) — never by the acting user's roles. Flag anything that authorizes off a *resolved/attributed* user. +- **Authz ≠ attribution**: the result of `ActingUserResolver.resolve()` (`acting.userId`/`memberId`) may be used ONLY for audit rows / `createdBy` / assignment — never to make a permission decision. +- **IDOR**: any lookup/update/delete by an id from the client (`@Param`, body) must also be scoped so the caller can only reach their own org's row. + +### 2. Multi-tenant isolation +- **Every** Prisma query is scoped by `organizationId`, and that org id comes from the auth context (`@OrganizationId()` / `request.organizationId`), NEVER from a client-supplied body/param/header (except the validated `x-organization-id` service-token path). +- Cross-org reference: creating/linking a row must verify the referenced id belongs to the same org. + +### 3. Injection +- Prisma raw APIs: `$queryRawUnsafe` / `$executeRawUnsafe` / string-interpolated `$queryRaw` = SQL injection. Require parameterized tagged templates or `Prisma.sql`. +- Command injection (`child_process` exec/spawn with interpolated input), path traversal in file paths (`../`), and unsafe `req.body` handling. + +### 4. XSS / output handling (Next.js) +- `dangerouslySetInnerHTML`, unsanitized rich text (TipTap/HTML content) rendered without sanitization, `href`/redirect built from user input (`javascript:` URLs), unvalidated open redirects. + +### 5. Secrets & sensitive-data exposure +- Hardcoded secrets/keys/tokens; secrets or full tokens written to logs; `.env` committed; secrets returned in API responses or echoed to the client. +- **PII / secret in the audit or activity data JSON.** (This repo has a history of keys leaking via git notes — flag any new secret material in tracked files.) + +### 6. Authentication & session +- All auth goes through the API (no local better-auth instance in app/portal). Session checks proxy `/api/auth/get-session`. No JWTs. Raw `fetch()` to the API includes `credentials: 'include'`. +- Trust boundaries: service-token `x-user-id` and API-key attribution must resolve to an **active** member (`deactivated:false`, `isActive:true`) of the bound org. + +### 7. SSRF & outbound requests +- Server-side `fetch`/HTTP to a **user-controlled URL** (vendor website research, integrations, cloud connectors, webhooks) — validate scheme/host, block internal/link-local ranges (169.254.169.254, 127.0.0.0/8, 10/172.16/192.168), don't follow redirects into internal space. + +### 8. Unsafe file upload / storage +- S3 presigned URLs scoped to the org's prefix + constrained content-type/size; uploaded content-type validated; no path traversal in object keys; buckets not public (OAC/BPA). + +### 9. Mass assignment +- Spreading unvalidated `...dto` / `...req.body` directly into `db.*.create/update` lets a caller set fields they shouldn't (status, ownerId, organizationId, flags). Require an explicit field allowlist for anything security-relevant. + +### 10. DoS / resource exhaustion +- Unbounded queries (no pagination/limit), expensive synchronous work on the request path (offload to Trigger.dev), unbounded loops over caller-supplied arrays. (This repo took prod down via an in-process bulk export.) + +### 11. Error handling / info disclosure +- Internal errors, stack traces, or DB/driver messages returned to the client. Use NestJS exceptions with clean messages. + +### 12. Audit trail / non-repudiation +- Security-relevant mutations are audit-logged, attributed to the acting user, and non-session callers carry a provenance marker (`ActingUserResolver` `callerLabel` → `via API key "…"`), so an owner-fallback attribution isn't mistaken for a session action. + +## Output +A ranked list of findings (severity, `file:line`, exploit/impact, fix), then a one-line "clean" note for each class with no findings. End with the single highest-priority action. diff --git a/.claude/skills/production-readiness/SKILL.md b/.claude/skills/production-readiness/SKILL.md index 1a377c0b45..13a110b3fe 100644 --- a/.claude/skills/production-readiness/SKILL.md +++ b/.claude/skills/production-readiness/SKILL.md @@ -6,11 +6,12 @@ disable-model-invocation: true Run a comprehensive production readiness check on $ARGUMENTS. -Use parallel subagents to run all four audits simultaneously: +Use parallel subagents to run all five audits simultaneously: 1. audit-rbac on $ARGUMENTS 2. audit-hooks on $ARGUMENTS 3. audit-design-system on $ARGUMENTS 4. audit-tests on $ARGUMENTS +5. security-review on $ARGUMENTS Then run full monorepo verification: ```bash diff --git a/.claude/skills/security-review/SKILL.md b/.claude/skills/security-review/SKILL.md new file mode 100644 index 0000000000..1d5317c01b --- /dev/null +++ b/.claude/skills/security-review/SKILL.md @@ -0,0 +1,35 @@ +--- +name: security-review +description: Check code for the most common, high-risk security vulnerabilities (broken access control, tenant isolation, injection, secrets, SSRF, auth/session, unsafe file handling, mass assignment) before it ships. Use after editing any API controller, guard, or auth code (apps/api/src/auth/**), a Prisma schema/query, a file-upload/webhook handler, or before committing/pushing security-sensitive changes. +--- + +Review code for high-risk security vulnerabilities and **fix confirmed high-severity issues immediately**. Complements CI (CodeQL/Dependabot/SBOM) by catching logic/design flaws those miss — do not re-flag dependency CVEs or generic lint. + +## 1. Scope + +- If `$ARGUMENTS` names files/dirs, review those. +- Otherwise review the current change set: `git diff --name-only origin/main...HEAD` (fall back to `git diff --name-only` for uncommitted work). Skip generated files, lockfiles, and `*.spec.ts` (unless the change is in a spec). + +**Review whenever the change touches a trust boundary or a resource — judge by behavior, not path.** That includes: any controller **or service** (services are where IDOR, tenant-scoping, and mass-assignment bugs actually live), Next.js route handlers (`app/api/**`, `route.ts`), Trigger.dev jobs (`apps/api/src/trigger/**` — SSRF, elevated context), guards/auth (`apps/api/src/auth/**`, `packages/auth/**`), any Prisma query (especially lookups/updates/deletes by a client-supplied id → IDOR), middleware, file upload/storage, webhooks, outbound requests to a dynamic URL, crypto/token handling, and any frontend that renders user-supplied HTML or builds redirects from input. + +Only if the change is genuinely non-security (pure presentation, types, copy, config with no secrets) — say so and stop, don't invent findings. + +## 2. Review + +Dispatch the **`security-reviewer`** agent on the scoped files. For a broad review (a full PR / many files), fan out **parallel** `security-reviewer` agents by dimension group so coverage is thorough: +- **Access control & tenancy** — guards, `@RequirePermission`, API-key/service-token scope enforcement, authz-vs-attribution, IDOR, `organizationId` scoping. +- **Injection, XSS & mass assignment** — raw Prisma/SQL, command/path injection, `dangerouslySetInnerHTML`/unsanitized HTML, `...dto` spread into `create/update`. +- **Secrets, SSRF, auth/session & file handling** — hardcoded/logged secrets, outbound fetch to user URLs, session/cookie/attribution boundaries, S3/upload safety, DoS, info disclosure. + +The agent's full checklist lives in `.claude/agents/security-reviewer.md`. + +## 3. Verify (no false positives) + +For each `P1`/`P2` finding, adversarially verify before acting: dispatch one `security-reviewer` (or read the code yourself) prompted to **refute** it — trace the guard→controller→service→DB path. Drop anything that can't be confirmed. Attribution-only use of a resolved actor, org-scoped queries, and parameterized Prisma are NOT findings. + +## 4. Fix & report + +- **Fix** confirmed `P1`/`P2` issues immediately (mirror the surrounding code; add a regression test for auth/attribution changes). +- **Report** `P3`s and anything needing a product decision, with `severity · file:line · exploit/impact · fix`. +- Run `bunx turbo run typecheck --filter=@trycompai/api --filter=@trycompai/app` after fixes. +- End with a one-line verdict: clean, or the highest-priority action remaining. diff --git a/.github/security-scan-instructions.md b/.github/security-scan-instructions.md new file mode 100644 index 0000000000..dbc7de5073 --- /dev/null +++ b/.github/security-scan-instructions.md @@ -0,0 +1,47 @@ +# Security review — repo-specific instructions + +Multi-tenant compliance SaaS: NestJS API (`apps/api`), Next.js app (`apps/app`), +Prisma (`packages/db`), better-auth (auth lives in the API). Apply these +high-signal, repo-specific checks in addition to the standard vulnerability +classes. Verify against the actual code (guard → controller → service → DB); do +not speculate. + +## Access control (highest priority) +- Every mutation endpoint (POST/PATCH/PUT/DELETE) must have + `@UseGuards(HybridAuthGuard, PermissionGuard)` + `@RequirePermission('resource','action')`. + `@Public()` only on webhooks. +- API-key requests are authorized by `request.apiKeyScopes`; service tokens by the + service's own `permissions`. Flag any authorization decision derived from a + *resolved / attributed* user rather than the credential's own scopes. +- **Authz ≠ attribution:** the output of `ActingUserResolver.resolve()` + (`acting.userId` / `acting.memberId`) may be used ONLY for audit rows, + `createdBy`, or assignment — never to grant a permission. +- **IDOR:** any lookup/update/delete by a client-supplied id + (`@Param`/body) must also be scoped so the caller can only reach their own + org's row. + +## Multi-tenant isolation +- Every Prisma query must be scoped by `organizationId`, and that value must come + from the auth context (`@OrganizationId()` / `request.organizationId`), NEVER + from a client-supplied body/param/header — except the validated + `x-organization-id` service-token path. + +## Repo hotspots +- **SSRF:** server-side fetch to a user-controlled URL — vendor website research + (Trigger.dev jobs), integration/cloud connectors, webhooks. Require scheme/host + validation and blocking of internal/link-local ranges. +- **Secrets:** no hardcoded or logged secrets/tokens; API responses must not leak + keys; check the audit `data` JSON for secret/PII. +- **File uploads:** S3 presigned URLs scoped to the org prefix, content-type/size + constrained, no public buckets, no path traversal in object keys. +- **Mass assignment:** spreading `...dto` / `...req.body` into `db.*.create/update` + can let a caller set fields they shouldn't (status, ownerId, organizationId). +- **Non-repudiation:** security-relevant mutations should be audit-logged and + attributed; API-key / service-token actions carry a `via API key "…"` + provenance marker. + +## Do NOT flag (known-safe patterns) +- The `...(authContext.userId && { authenticatedUser })` response-echo pattern — + it is not attribution. +- Org-scoped Prisma queries, parameterized/tagged-template queries, and + attribution-only use of the resolved actor. diff --git a/.github/workflows/security-review.yml b/.github/workflows/security-review.yml new file mode 100644 index 0000000000..a8f36b7e32 --- /dev/null +++ b/.github/workflows/security-review.yml @@ -0,0 +1,35 @@ +name: Security Review + +# Anthropic's official Claude security reviewer. Runs on every PR, analyzes only +# the changed files (diff-aware), and posts inline comments with findings + fixes. +# This is the authoritative gate; the local Claude Code hook + skill are the +# fast dev-time layer. Prereq: add an ANTHROPIC_API_KEY repo secret (enabled for +# Claude Code). Make this a required check via branch protection to block merges. + +on: + pull_request: + +permissions: + pull-requests: write + contents: read + +jobs: + security-review: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + ref: ${{ github.event.pull_request.head.sha || github.sha }} + fetch-depth: 2 + # TODO: pin to a release SHA for supply-chain safety once a tag is chosen. + - uses: anthropics/claude-code-security-review@main + with: + comment-pr: true + claude-api-key: ${{ secrets.ANTHROPIC_API_KEY }} + # Review the latest diff on EVERY push to a PR — not just the first + # commit — so code added after the initial review is still checked. + run-every-commit: true + # Repo-specific checks (tenant isolation, authz-vs-attribution, IDOR) + # layered on top of the default audit. + custom-security-scan-instructions: .github/security-scan-instructions.md + exclude-directories: packages/db/src/generated diff --git a/apps/api/package.json b/apps/api/package.json index 423679e678..cb34ae6c38 100644 --- a/apps/api/package.json +++ b/apps/api/package.json @@ -89,7 +89,7 @@ "@upstash/ratelimit": "^2.0.8", "@upstash/redis": "^1.34.2", "@upstash/vector": "^1.2.2", - "adm-zip": "^0.5.16", + "adm-zip": "^0.6.0", "ai": "^6.0.175", "archiver": "^7.0.1", "axios": "^1.16.0", diff --git a/apps/api/src/audit/audit-log.interceptor.spec.ts b/apps/api/src/audit/audit-log.interceptor.spec.ts index e8130d665b..d33fa1fcb3 100644 --- a/apps/api/src/audit/audit-log.interceptor.spec.ts +++ b/apps/api/src/audit/audit-log.interceptor.spec.ts @@ -17,6 +17,7 @@ const mockFindUnique = jest.fn(); const mockPolicyFindUnique = jest.fn(); const mockMemberFindMany = jest.fn(); const mockControlFindMany = jest.fn(); +const mockResolve = jest.fn(); jest.mock('@db', () => ({ db: { auditLog: { @@ -63,8 +64,19 @@ jest.mock('@db', () => ({ Prisma: {}, })); +// permission.guard (imported transitively via the interceptor) pulls @trycompai/auth, +// which loads better-auth's ESM subpaths. Mock it so the spec doesn't have to transform +// them — we only need the permission metadata keys, not the real role tables. +jest.mock('@trycompai/auth', () => ({ + statement: {}, + BUILT_IN_ROLE_PERMISSIONS: {}, + RESTRICTED_ROLES: [], + PRIVILEGED_ROLES: [], +})); + // Import after mocks import { AuditLogInterceptor } from './audit-log.interceptor'; +import { ActingUserResolver } from '../auth/acting-user.service'; import { PERMISSIONS_KEY } from '../auth/permission.guard'; import { AUDIT_READ_KEY, SKIP_AUDIT_LOG_KEY } from './skip-audit-log.decorator'; @@ -79,6 +91,8 @@ describe('AuditLogInterceptor', () => { organizationId?: string; userId?: string; memberId?: string; + isApiKey?: boolean; + apiKeyCreatedByMemberId?: string | null; params?: Record; body?: Record; } = {}, @@ -89,8 +103,13 @@ describe('AuditLogInterceptor', () => { method: overrides.method ?? 'PATCH', url: overrides.url ?? '/v1/policies/pol_123', organizationId: overrides.organizationId ?? 'org_123', - userId: overrides.userId ?? 'user_123', - memberId: overrides.memberId ?? 'mem_123', + // API-key requests never carry a session userId/memberId — the guard + // doesn't set them, so the interceptor must resolve the actor. Only + // default to the session user for non-API-key requests. + userId: overrides.isApiKey ? overrides.userId : overrides.userId ?? 'user_123', + memberId: overrides.isApiKey ? overrides.memberId : overrides.memberId ?? 'mem_123', + isApiKey: overrides.isApiKey ?? false, + apiKeyCreatedByMemberId: overrides.apiKeyCreatedByMemberId, params: overrides.params ?? { id: 'pol_123' }, body: overrides.body ?? undefined, headers: {}, @@ -109,7 +128,11 @@ describe('AuditLogInterceptor', () => { beforeEach(async () => { const module: TestingModule = await Test.createTestingModule({ - providers: [AuditLogInterceptor, Reflector], + providers: [ + AuditLogInterceptor, + Reflector, + { provide: ActingUserResolver, useValue: { resolve: mockResolve } }, + ], }).compile(); interceptor = module.get(AuditLogInterceptor); @@ -124,6 +147,11 @@ describe('AuditLogInterceptor', () => { mockMemberFindMany.mockResolvedValue([]); mockControlFindMany.mockReset(); mockControlFindMany.mockResolvedValue([]); + mockResolve.mockReset(); + // Default: resolver returns no attributable user. resolve() must always + // return a promise (the interceptor calls .then on it), so a bare jest.fn() + // returning undefined would throw. Tests needing a real actor override this. + mockResolve.mockResolvedValue({ userId: null, source: 'org-owner-fallback' }); }); it('should skip GET requests', (done) => { @@ -185,6 +213,90 @@ describe('AuditLogInterceptor', () => { }); }); + it('logs API-key mutations, attributing to the resolved actor (key creator)', (done) => { + jest.spyOn(reflector, 'getAllAndOverride').mockImplementation((key) => { + if (key === PERMISSIONS_KEY) { + return [{ resource: 'vendor', actions: ['create'] }]; + } + if (key === SKIP_AUDIT_LOG_KEY) return false; + return undefined; + }); + mockResolve.mockResolvedValue({ + userId: 'usr_creator', + memberId: 'mem_creator', + source: 'api-key-creator', + callerLabel: 'via API key "CI Pipeline"', + }); + + const context = createMockExecutionContext({ + method: 'POST', + url: '/v1/vendors', + params: {}, + isApiKey: true, + apiKeyCreatedByMemberId: 'mem_creator', + }); + const handler = createMockCallHandler({ id: 'vnd_new' }); + + interceptor.intercept(context, handler).subscribe({ + next: () => { + setTimeout(() => { + expect(mockResolve).toHaveBeenCalledWith( + expect.objectContaining({ isApiKey: true, organizationId: 'org_123' }), + 'org_123', + ); + expect(mockCreate).toHaveBeenCalledWith({ + data: expect.objectContaining({ + organizationId: 'org_123', + userId: 'usr_creator', + memberId: 'mem_creator', + entityType: 'vendor', + entityId: 'vnd_new', + // Provenance marker recorded so the owner/creator attribution + // isn't read as a session action. + description: expect.stringContaining('[via API key "CI Pipeline"]'), + }), + }); + // ...and captured structurally in the data JSON. + const call = mockCreate.mock.calls[0][0]; + expect(call.data.data.via).toBe('via API key "CI Pipeline"'); + done(); + }, 50); + }, + }); + }); + + it('skips the audit log for API-key requests when no actor can be resolved', (done) => { + jest.spyOn(reflector, 'getAllAndOverride').mockImplementation((key) => { + if (key === PERMISSIONS_KEY) { + return [{ resource: 'vendor', actions: ['create'] }]; + } + if (key === SKIP_AUDIT_LOG_KEY) return false; + return undefined; + }); + mockResolve.mockResolvedValue({ + userId: null, + source: 'org-owner-fallback', + }); + + const context = createMockExecutionContext({ + method: 'POST', + url: '/v1/vendors', + params: {}, + isApiKey: true, + }); + const handler = createMockCallHandler({ id: 'vnd_new' }); + + interceptor.intercept(context, handler).subscribe({ + next: () => { + setTimeout(() => { + expect(mockResolve).toHaveBeenCalled(); + expect(mockCreate).not.toHaveBeenCalled(); + done(); + }, 50); + }, + }); + }); + it('should log PATCH requests with entity ID from params', (done) => { jest.spyOn(reflector, 'getAllAndOverride').mockImplementation((key) => { if (key === PERMISSIONS_KEY) { diff --git a/apps/api/src/audit/audit-log.interceptor.ts b/apps/api/src/audit/audit-log.interceptor.ts index e8f7667768..4e954cb244 100644 --- a/apps/api/src/audit/audit-log.interceptor.ts +++ b/apps/api/src/audit/audit-log.interceptor.ts @@ -10,6 +10,7 @@ import { AuditLogEntityType, db, Prisma } from '@db'; import { Observable, from, switchMap, tap } from 'rxjs'; import { PERMISSIONS_KEY, RequiredPermission } from '../auth/permission.guard'; import { AuthenticatedRequest } from '../auth/types'; +import { ActingUserResolver } from '../auth/acting-user.service'; import { AUDIT_READ_KEY, SKIP_AUDIT_LOG_KEY } from './skip-audit-log.decorator'; import { MEMBER_REF_FIELDS, @@ -39,7 +40,10 @@ import { export class AuditLogInterceptor implements NestInterceptor { private readonly logger = new Logger(AuditLogInterceptor.name); - constructor(private readonly reflector: Reflector) {} + constructor( + private readonly reflector: Reflector, + private readonly actingUser: ActingUserResolver, + ) {} intercept(context: ExecutionContext, next: CallHandler): Observable { const request = context.switchToHttp().getRequest(); @@ -66,8 +70,8 @@ export class AuditLogInterceptor implements NestInterceptor { return next.handle(); } - const { organizationId, userId, memberId, impersonatedBy } = request; - if (!organizationId || !userId) { + const { organizationId, impersonatedBy } = request; + if (!organizationId) { return next.handle(); } @@ -114,11 +118,49 @@ export class AuditLogInterceptor implements NestInterceptor { }; }); - return from(safePreFlightPromise).pipe( - switchMap(({ previousValues, memberNames, relationMappingResult }) => + // Resolve who to attribute this mutation to. Session / service-token-acting + // callers already have req.userId (zero-cost). API-key callers don't — resolve + // the responsible user (key creator, else org owner) so the action is still + // audited and attributed to a real person instead of being silently dropped. + const actorPromise: Promise<{ + userId: string | null; + memberId: string | undefined; + // Provenance marker for non-session callers (e.g. `via API key "CI"`), + // recorded in the audit trail so an owner-fallback / creator attribution + // isn't mistaken for the user personally acting in a session. + callerLabel: string | undefined; + }> = request.userId + ? Promise.resolve({ + userId: request.userId, + memberId: request.memberId, + callerLabel: undefined, + }) + : this.actingUser + .resolve(request, organizationId) + .then((a) => ({ + userId: a.userId, + memberId: a.memberId, + callerLabel: a.callerLabel, + })) + .catch((err) => { + this.logger.error('Audit actor resolution failed', err); + return { + userId: null, + memberId: undefined, + callerLabel: undefined, + }; + }); + + return from(Promise.all([safePreFlightPromise, actorPromise])).pipe( + switchMap(([{ previousValues, memberNames, relationMappingResult }, actor]) => next.handle().pipe( tap({ next: (responseBody) => { + // No attributable user (e.g. an org with zero owner-role members). + // Skip logging rather than persist a null userId FK — mirrors the + // resolver's soft-failure contract. + if (!actor.userId) return; + const commentCtx = extractCommentContext( request.url, method, @@ -216,8 +258,8 @@ export class AuditLogInterceptor implements NestInterceptor { void this.persist( organizationId, - userId, - memberId, + actor.userId, + actor.memberId, method, request.url, resource, @@ -228,6 +270,7 @@ export class AuditLogInterceptor implements NestInterceptor { commentCtx, descriptionOverride, impersonatedBy, + actor.callerLabel, ).catch((err) => { this.logger.error('Failed to create audit log entry', err); }); @@ -293,6 +336,7 @@ export class AuditLogInterceptor implements NestInterceptor { commentContext: AuditContextOverride | null, descriptionOverride: string | null, impersonatedBy?: string, + callerLabel?: string, ): Promise { const entityType = commentContext?.entityType ?? RESOURCE_TO_ENTITY_TYPE[resource] ?? null; @@ -317,6 +361,16 @@ export class AuditLogInterceptor implements NestInterceptor { if (impersonatedBy) { auditData.impersonatedBy = impersonatedBy; } + // Record automated-caller provenance (e.g. 'via API key "CI Pipeline"') so an + // owner-fallback / key-creator attribution isn't read as the user personally + // acting in a session — preserves non-repudiation of the audit trail. + if (callerLabel) { + auditData.via = callerLabel; + } + + const finalDescription = `${impersonatedBy ? '[Impersonated] ' : ''}${description}${ + callerLabel ? ` [${callerLabel}]` : '' + }`; await db.auditLog.create({ data: { @@ -325,9 +379,7 @@ export class AuditLogInterceptor implements NestInterceptor { memberId: memberId ?? null, entityType, entityId, - description: impersonatedBy - ? `[Impersonated] ${description}` - : description, + description: finalDescription, data: auditData as Prisma.InputJsonValue, }, }); diff --git a/apps/api/src/audit/audit-log.resolvers.ts b/apps/api/src/audit/audit-log.resolvers.ts index 906d3b56c2..3c58086d24 100644 --- a/apps/api/src/audit/audit-log.resolvers.ts +++ b/apps/api/src/audit/audit-log.resolvers.ts @@ -46,6 +46,19 @@ const RESOURCE_CONTROL_MODELS: Record = { risks: 'risk', }; +/** + * Singularize a plural resource segment for human-readable audit descriptions. + * Prefers the known Prisma model name (policies→policy, risks→risk); falls back + * to English rules ("ies"→"y", else drop trailing "s") so we never emit the + * naive-strip artifact "policie". + */ +function singularize(resource: string): string { + const known = RESOURCE_CONTROL_MODELS[resource]; + if (known) return known; + if (resource.endsWith('ies')) return `${resource.slice(0, -3)}y`; + return resource.replace(/s$/, ''); +} + async function fetchControlIds( resource: string, parentId: string, @@ -103,7 +116,7 @@ export async function buildRelationMappingChanges( const afterIds = [...new Set([...currentIds, ...newIds])]; const afterDisplay = afterIds.map((id) => nameMap[id] || id).join(', '); - const singularResource = resource.replace(/s$/, ''); + const singularResource = singularize(resource); return { changes: { controls: { previous: prevDisplay, current: afterDisplay } }, @@ -132,7 +145,7 @@ export async function buildRelationMappingChanges( ? afterIds.map((id) => nameMap[id] || id).join(', ') : 'None'; - const singularResource = resource.replace(/s$/, ''); + const singularResource = singularize(resource); return { changes: { controls: { previous: prevDisplay, current: afterDisplay } }, diff --git a/apps/api/src/auth/acting-user.service.spec.ts b/apps/api/src/auth/acting-user.service.spec.ts index 3d28ba71ca..32f453841e 100644 --- a/apps/api/src/auth/acting-user.service.spec.ts +++ b/apps/api/src/auth/acting-user.service.spec.ts @@ -35,6 +35,7 @@ describe('ActingUserResolver', () => { it('returns req.userId without a DB query', async () => { const req = makeReq({ userId: 'usr_session_alice', + memberId: 'mem_session_alice', authType: 'session', }); @@ -42,6 +43,7 @@ describe('ActingUserResolver', () => { expect(result).toEqual({ userId: 'usr_session_alice', + memberId: 'mem_session_alice', source: 'session', }); // Critical regression guard — session auth must NEVER hit the DB @@ -83,6 +85,7 @@ describe('ActingUserResolver', () => { describe('API key caller (owner fallback)', () => { it('resolves to the org owner and labels the caller for the audit log', async () => { mockDb.member.findFirst.mockResolvedValueOnce({ + id: 'mem_owner_carol', userId: 'usr_owner_carol', }); @@ -97,6 +100,8 @@ describe('ActingUserResolver', () => { const result = await resolver.resolve(req, 'org_1'); expect(result.userId).toBe('usr_owner_carol'); + // The fallback owner's member is surfaced too, for Member-FK sinks. + expect(result.memberId).toBe('mem_owner_carol'); expect(result.source).toBe('org-owner-fallback'); expect(result.callerLabel).toBe('via API key "CI Pipeline"'); }); @@ -198,6 +203,65 @@ describe('ActingUserResolver', () => { }); }); + describe('API key with a recorded creator', () => { + it("attributes to the creating member's user (source api-key-creator)", async () => { + // Creator lookup returns an active member of the org. + mockDb.member.findFirst.mockResolvedValueOnce({ userId: 'usr_creator_dave' }); + + const req = makeReq({ + userId: undefined, + authType: 'api-key', + isApiKey: true, + apiKeyId: 'apk_1', + apiKeyName: 'Mariano CLI', + apiKeyCreatedByMemberId: 'mem_creator', + }); + + const result = await resolver.resolve(req, 'org_1'); + + expect(result.userId).toBe('usr_creator_dave'); + expect(result.memberId).toBe('mem_creator'); + expect(result.source).toBe('api-key-creator'); + expect(result.callerLabel).toBe('via API key "Mariano CLI"'); + // Single lookup: the creator, scoped to the org + active membership. + // No owner fallback query is made. + expect(mockDb.member.findFirst).toHaveBeenCalledTimes(1); + expect(mockDb.member.findFirst).toHaveBeenCalledWith( + expect.objectContaining({ + where: expect.objectContaining({ + id: 'mem_creator', + organizationId: 'org_1', + deactivated: false, + isActive: true, + }), + }), + ); + }); + + it('falls back to the org owner when the creator is no longer an active member', async () => { + // 1st findFirst = creator lookup (null → offboarded/removed), + // 2nd findFirst = owner fallback. + mockDb.member.findFirst + .mockResolvedValueOnce(null) + .mockResolvedValueOnce({ userId: 'usr_owner_carol' }); + + const req = makeReq({ + userId: undefined, + authType: 'api-key', + isApiKey: true, + apiKeyId: 'apk_1', + apiKeyName: 'Old Key', + apiKeyCreatedByMemberId: 'mem_offboarded', + }); + + const result = await resolver.resolve(req, 'org_1'); + + expect(result.userId).toBe('usr_owner_carol'); + expect(result.source).toBe('org-owner-fallback'); + expect(mockDb.member.findFirst).toHaveBeenCalledTimes(2); + }); + }); + describe('service token without x-user-id (owner fallback)', () => { it('resolves to the org owner with a service-flavored caller label', async () => { mockDb.member.findFirst.mockResolvedValueOnce({ userId: 'usr_owner' }); diff --git a/apps/api/src/auth/acting-user.service.ts b/apps/api/src/auth/acting-user.service.ts index 9091375e57..6336ce04a9 100644 --- a/apps/api/src/auth/acting-user.service.ts +++ b/apps/api/src/auth/acting-user.service.ts @@ -15,6 +15,7 @@ import type { AuthenticatedRequest } from './types'; export type ActingUserSource = | 'session' | 'service-token-acting' + | 'api-key-creator' | 'org-owner-fallback'; export interface ResolvedActingUser { @@ -22,6 +23,12 @@ export interface ResolvedActingUser { * available (e.g. an org with zero owner-role members — caller should * surface a 400 with an actionable message). */ userId: string | null; + /** Member (org membership) to attribute the mutation to, when known. Populated + * for every path: the session member (Path 1/2), the key creator (Path 3), or + * the fallback owner's member (Path 4). Undefined only when the request had no + * member and no owner was found. Needed for Member-FK sinks (e.g. audit rows, + * isms enteredById). */ + memberId?: string; source: ActingUserSource; /** Short label for audit log descriptions. Only set when source is * 'org-owner-fallback' — session and explicit service-token acting @@ -40,10 +47,14 @@ export interface ResolvedActingUser { * 2. Service tokens calling on behalf of a specific user — HybridAuthGuard * sets `req.userId` from the `x-user-id` header after Member validation. * Same short-circuit as session. - * 3. API keys, or service tokens without `x-user-id` — no per-user identity - * exists. We attribute to the org's OLDEST owner (deterministic + stable - * across deletes of newer owners). This is consistent with how 19+ - * other places in the codebase already look up org owners + * 3. API keys with a recorded creator — attribute to the member who created + * the key (if still an active member of the org), so the audit trail + * reflects who set up the automation. + * 4. Everything else (legacy API keys with no recorded creator, keys whose + * creator was deactivated/removed, or service tokens without `x-user-id`) + * — no per-user identity exists, so we attribute to the org's OLDEST owner + * (deterministic + stable across deletes of newer owners), consistent with + * how 19+ other places in the codebase look up org owners * (`Member.role.contains('owner')`). * * Returning null userId is a soft failure — callers must surface a 400 with @@ -63,13 +74,39 @@ export class ActingUserResolver { if (req.userId) { return { userId: req.userId, + memberId: req.memberId, source: req.isServiceToken ? 'service-token-acting' : 'session', }; } - // Path 3 — fall back to the org's owner. - const ownerUserId = await this.findOrgOwnerUserId(organizationId); - if (!ownerUserId) { + // Path 3 — API key with a recorded creator who is still an active member + // of this org. Attribute to that member's user so the audit trail reflects + // who set up the automation, not the org owner. Legacy keys (no recorded + // creator) and keys whose creator has been deactivated/removed fall through + // to the owner fallback below. + if (req.isApiKey && req.apiKeyCreatedByMemberId) { + const creator = await db.member.findFirst({ + where: { + id: req.apiKeyCreatedByMemberId, + organizationId, + deactivated: false, + isActive: true, + }, + select: { userId: true }, + }); + if (creator) { + return { + userId: creator.userId, + memberId: req.apiKeyCreatedByMemberId, + source: 'api-key-creator', + callerLabel: this.buildCallerLabel(req), + }; + } + } + + // Path 4 — fall back to the org's owner. + const owner = await this.findOrgOwner(organizationId); + if (!owner) { // No owner found. Don't invent one — the caller should reject the // mutation with a clear message so the customer can fix the role // assignment themselves. @@ -84,7 +121,8 @@ export class ActingUserResolver { } return { - userId: ownerUserId, + userId: owner.userId, + memberId: owner.memberId, source: 'org-owner-fallback', callerLabel: this.buildCallerLabel(req), }; @@ -103,9 +141,9 @@ export class ActingUserResolver { * `deactivated: false` + `isActive: true` excludes offboarded owners so we * don't attribute new mutations to a user who no longer has org access. */ - private async findOrgOwnerUserId( + private async findOrgOwner( organizationId: string, - ): Promise { + ): Promise<{ memberId: string; userId: string } | null> { const owner = await db.member.findFirst({ where: { organizationId, @@ -114,9 +152,9 @@ export class ActingUserResolver { role: { contains: 'owner' }, }, orderBy: { createdAt: 'asc' }, - select: { userId: true }, + select: { id: true, userId: true }, }); - return owner?.userId ?? null; + return owner ? { memberId: owner.id, userId: owner.userId } : null; } /** diff --git a/apps/api/src/auth/api-key.service.ts b/apps/api/src/auth/api-key.service.ts index e78b2a6783..1ccd3010d6 100644 --- a/apps/api/src/auth/api-key.service.ts +++ b/apps/api/src/auth/api-key.service.ts @@ -19,6 +19,10 @@ export interface ApiKeyValidationResult { apiKeyName: string; organizationId: string; scopes: string[]; + /** Member (org membership) that created this key, or null for legacy keys + * created before creator attribution existed. Used by ActingUserResolver + * to attribute API-key mutations to the real creator. */ + createdByMemberId: string | null; } @Injectable() @@ -61,6 +65,7 @@ export class ApiKeyService { name: string, expiresAt?: string, scopes?: string[], + createdByMemberId?: string | null, ) { // New keys must have explicit scopes — no more legacy empty-scope keys if (!scopes || scopes.length === 0) { @@ -110,6 +115,7 @@ export class ApiKeyService { expiresAt: expirationDate, organizationId, scopes, + createdByMemberId: createdByMemberId ?? null, }, select: { id: true, @@ -200,6 +206,7 @@ export class ApiKeyService { organizationId: true, expiresAt: true, scopes: true, + createdByMemberId: true, }, }); @@ -228,6 +235,7 @@ export class ApiKeyService { organizationId: true, expiresAt: true, scopes: true, + createdByMemberId: true, }, }); const legacyMatch = legacyRecords.find((record) => { @@ -247,6 +255,7 @@ export class ApiKeyService { apiKeyName: legacyMatch.name, organizationId: legacyMatch.organizationId, scopes: legacyMatch.scopes, + createdByMemberId: legacyMatch.createdByMemberId ?? null, }; } } @@ -273,6 +282,7 @@ export class ApiKeyService { apiKeyName: matchingRecord.name, organizationId: matchingRecord.organizationId, scopes: matchingRecord.scopes, + createdByMemberId: matchingRecord.createdByMemberId ?? null, }; } catch (error) { this.logger.error('Error validating API key:', error); diff --git a/apps/api/src/auth/hybrid-auth.guard.spec.ts b/apps/api/src/auth/hybrid-auth.guard.spec.ts index ec4b7ce528..fd30ac1841 100644 --- a/apps/api/src/auth/hybrid-auth.guard.spec.ts +++ b/apps/api/src/auth/hybrid-auth.guard.spec.ts @@ -25,12 +25,20 @@ jest.mock('./auth.server', () => ({ // (device-agent style) to bind the organization for the MCP OAuth path. const mockUserFindUnique = jest.fn(); const mockMemberFindMany = jest.fn(); +const mockMemberFindFirst = jest.fn(); +const mockOrgFindUnique = jest.fn(); const mockMcpBindingFindUnique = jest.fn(); const mockOrgRoleFindMany = jest.fn(); jest.mock('@db', () => ({ db: { user: { findUnique: (...args: unknown[]) => mockUserFindUnique(...args) }, - member: { findMany: (...args: unknown[]) => mockMemberFindMany(...args) }, + member: { + findMany: (...args: unknown[]) => mockMemberFindMany(...args), + findFirst: (...args: unknown[]) => mockMemberFindFirst(...args), + }, + organization: { + findUnique: (...args: unknown[]) => mockOrgFindUnique(...args), + }, mcpOrgBinding: { findUnique: (...args: unknown[]) => mockMcpBindingFindUnique(...args), }, @@ -40,6 +48,14 @@ jest.mock('@db', () => ({ }, })); +// Service-token validation is a pure lookup; mock it so tests can present a +// valid token and reach the x-user-id acting-member resolution. +const mockResolveServiceByToken = jest.fn(); +jest.mock('./service-token.config', () => ({ + resolveServiceByToken: (...args: unknown[]) => + mockResolveServiceByToken(...args), +})); + // Mock @trycompai/auth — the app-access gate reads BUILT_IN_ROLE_PERMISSIONS to // decide which roles grant app access. owner/admin/auditor do; employee does not. jest.mock('@trycompai/auth', () => ({ @@ -362,3 +378,86 @@ describe('HybridAuthGuard — MCP OAuth path', () => { await expect(guard.canActivate(context)).rejects.toThrow(ForbiddenException); }); }); + +describe('HybridAuthGuard — service token x-user-id acting member', () => { + let guard: HybridAuthGuard; + let reflector: Reflector; + + const createContext = ( + headers: Record, + ): { context: ExecutionContext; request: Record } => { + const request: Record = { headers }; + const context = { + switchToHttp: () => ({ getRequest: () => request }), + getHandler: () => jest.fn(), + getClass: () => jest.fn(), + } as unknown as ExecutionContext; + return { context, request }; + }; + + const svcHeaders = (userId?: string): Record => ({ + 'x-service-token': 'valid_service_token', + 'x-organization-id': 'org_1', + ...(userId ? { 'x-user-id': userId } : {}), + }); + + beforeEach(async () => { + jest.clearAllMocks(); + const module: TestingModule = await Test.createTestingModule({ + providers: [ + HybridAuthGuard, + { + provide: ApiKeyService, + useValue: { extractApiKey: jest.fn(), validateApiKey: jest.fn() }, + }, + Reflector, + ], + }).compile(); + guard = module.get(HybridAuthGuard); + reflector = module.get(Reflector); + jest.spyOn(reflector, 'getAllAndOverride').mockReturnValue(false); + // Valid service token + existing org so we reach the x-user-id block. + mockResolveServiceByToken.mockReturnValue({ + definition: { name: 'Trigger.dev' }, + }); + mockOrgFindUnique.mockResolvedValue({ id: 'org_1' }); + mockMemberFindFirst.mockResolvedValue(null); + }); + + it('attributes an ACTIVE member: sets request.userId + memberId, scoped to active memberships', async () => { + mockMemberFindFirst.mockResolvedValue({ + id: 'mem_active', + userId: 'usr_active', + }); + + const { context, request } = createContext(svcHeaders('usr_active')); + await expect(guard.canActivate(context)).resolves.toBe(true); + + expect(request.userId).toBe('usr_active'); + expect(request.memberId).toBe('mem_active'); + // The lookup must exclude deactivated/inactive memberships. + expect(mockMemberFindFirst).toHaveBeenCalledWith( + expect.objectContaining({ + where: expect.objectContaining({ + userId: 'usr_active', + organizationId: 'org_1', + deactivated: false, + isActive: true, + }), + }), + ); + }); + + it('does NOT attribute a deactivated/inactive member (no userId/memberId set)', async () => { + // The active-only filter yields no row for an offboarded member. + mockMemberFindFirst.mockResolvedValue(null); + + const { context, request } = createContext(svcHeaders('usr_offboarded')); + await expect(guard.canActivate(context)).resolves.toBe(true); + + expect(request.userId).toBeUndefined(); + expect(request.memberId).toBeUndefined(); + // Auth still succeeds as a service token, just with no acting user. + expect(request.isServiceToken).toBe(true); + }); +}); diff --git a/apps/api/src/auth/hybrid-auth.guard.ts b/apps/api/src/auth/hybrid-auth.guard.ts index 0937b91d3b..4448b21970 100644 --- a/apps/api/src/auth/hybrid-auth.guard.ts +++ b/apps/api/src/auth/hybrid-auth.guard.ts @@ -80,7 +80,10 @@ export class HybridAuthGuard implements CanActivate { // without an extra DB lookup. request.apiKeyId = result.apiKeyId; request.apiKeyName = result.apiKeyName; - // API keys are organization-scoped and are not tied to a specific user/member. + // The member who created the key (if recorded). Lets ActingUserResolver + // attribute mutations to the real creator instead of the org owner. + request.apiKeyCreatedByMemberId = result.createdByMemberId; + // API keys are organization-scoped; no session user/member is attached here. request.userRoles = null; return true; @@ -125,14 +128,26 @@ export class HybridAuthGuard implements CanActivate { const actingUserId = request.headers['x-user-id'] as string; if (actingUserId) { const member = await db.member.findFirst({ - where: { userId: actingUserId, organizationId }, - select: { userId: true }, + // Only active memberships may act — an offboarded/deactivated user must + // not receive new audit / enteredById attribution. Mirrors the filters + // ActingUserResolver applies to its creator/owner lookups. + where: { + userId: actingUserId, + organizationId, + deactivated: false, + isActive: true, + }, + select: { id: true, userId: true }, }); if (member) { request.userId = actingUserId; + // Set the acting membership too, so Member-FK sinks (audit rows, + // enteredById, etc.) can attribute to the acting member and not just + // the user. + request.memberId = member.id; } else { this.logger.warn( - `Service token x-user-id "${actingUserId}" not found in org ${organizationId}`, + `Service token x-user-id "${actingUserId}" is not an active member of org ${organizationId}`, ); } } diff --git a/apps/api/src/auth/types.ts b/apps/api/src/auth/types.ts index bf3c11093f..140fac2fa6 100644 --- a/apps/api/src/auth/types.ts +++ b/apps/api/src/auth/types.ts @@ -15,6 +15,7 @@ export interface AuthenticatedRequest extends Request { apiKeyScopes?: string[]; // Scopes for API key auth (empty = legacy full access) apiKeyId?: string; // ApiKey row id — only set for API key auth. Used by ActingUserResolver / audit log attribution. apiKeyName?: string; // Human-readable API key name (e.g. "CI Pipeline") — only set for API key auth. + apiKeyCreatedByMemberId?: string | null; // Member (org membership) that created the key — only set for API key auth. Lets ActingUserResolver attribute mutations to the real creator instead of the org owner. impersonatedBy?: string; // User ID of the admin who initiated impersonation (only set during impersonation sessions) sessionId?: string; // Session ID (only set for session auth) sessionDeviceAgent?: boolean; // Whether the session is a device-agent session (only set for session auth) diff --git a/apps/api/src/cloud-security/cloud-security.controller.ts b/apps/api/src/cloud-security/cloud-security.controller.ts index c919d5d970..2572fbaadd 100644 --- a/apps/api/src/cloud-security/cloud-security.controller.ts +++ b/apps/api/src/cloud-security/cloud-security.controller.ts @@ -287,7 +287,7 @@ export class CloudSecurityController { async scan( @Param('connectionId') connectionId: string, @OrganizationId() organizationId: string, - @Req() req: { userId?: string; authType?: string }, + @Req() req: AuthenticatedRequest, ) { this.logger.log( `Cloud security scan requested for connection ${connectionId}`, @@ -325,23 +325,36 @@ export class CloudSecurityController { const failedCount = result.findings.filter((f) => !f.passed).length; const passedCount = result.findings.filter((f) => f.passed).length; - // Only write audit log when we have a real userId (session auth). - // API key auth has no user context, and auditLog.userId is a FK to User. - const scanUserId = req.userId; - if (scanUserId) - await logCloudSecurityActivity({ - organizationId, - userId: scanUserId, - connectionId, - action: 'scan_completed', - description: `Ran cloud security scan — ${totalFindings} findings (${failedCount} failed, ${passedCount} passed)`, - metadata: { - totalFindings, - failedCount, - passedCount, - provider: result.provider, - }, - }); + // Attribute to the acting user. Session callers already have req.userId; + // API key / service token callers resolve to the key creator or org owner. + // This is best-effort: the scan has already completed, so a transient + // resolver/audit failure (or an org with no attributable user) must not turn + // it into a failed HTTP request — that would invite the caller to re-run the + // scan. Log and move on. (auditLog.userId is a FK to User, so skip on null.) + try { + const acting = await this.actingUser.resolve(req, organizationId); + if (acting.userId) + await logCloudSecurityActivity({ + organizationId, + userId: acting.userId, + connectionId, + action: 'scan_completed', + // Append the caller marker (e.g. 'via API key "CI"') for non-session + // callers so an owner-fallback attribution isn't read as a session + // action — mirrors the exception / scan-mode endpoints. + description: `Ran cloud security scan — ${totalFindings} findings (${failedCount} failed, ${passedCount} passed)${ + acting.callerLabel ? ` [${acting.callerLabel}]` : '' + }`, + metadata: { + totalFindings, + failedCount, + passedCount, + provider: result.provider, + }, + }); + } catch (err) { + this.logger.error('Failed to record cloud security scan activity', err); + } return { success: true, diff --git a/apps/api/src/isms/documents/generate.spec.ts b/apps/api/src/isms/documents/generate.spec.ts index 3547f853ec..163aba2e44 100644 --- a/apps/api/src/isms/documents/generate.spec.ts +++ b/apps/api/src/isms/documents/generate.spec.ts @@ -155,4 +155,22 @@ describe('runDerivation', () => { // A regenerate must never clobber the customer's manual narrative. expect(tx.ismsDocument.update).not.toHaveBeenCalled(); }); + + it('seeds the internal-audit programme onto an empty draft (CS-724)', async () => { + const tx = buildTx(); + await runDerivation({ tx: asTx(tx), type: 'internal_audit', ...baseArgs }); + const updated = tx.ismsDocument.update.mock.calls[0][0].data; + expect(updated.draftNarrative.programme).toContain( + 'Acme runs an annual internal audit', + ); + }); + + it('preserves an edited internal-audit programme on regenerate (CS-724)', async () => { + const tx = buildTx(); + tx.ismsDocument.findUnique.mockResolvedValue({ + draftNarrative: { programme: 'Customer-edited programme.' }, + }); + await runDerivation({ tx: asTx(tx), type: 'internal_audit', ...baseArgs }); + 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 a1a3270587..cfd717f2a8 100644 --- a/apps/api/src/isms/documents/generate.ts +++ b/apps/api/src/isms/documents/generate.ts @@ -262,6 +262,13 @@ export async function runDerivation({ await seedMetricsIfMissing({ tx, documentId }); return; } + if (type === 'internal_audit') { + // Only the Programme paragraph is derivable; audit instances are + // customer-created (their Controls Tested rows seed at audit creation). + // Seed-if-empty, so a regenerate never clobbers an edited programme. + await generateNarrative({ tx, documentId, type, data }); + return; + } if (isNarrativeType(type)) { await generateNarrative({ tx, documentId, type, data }); return; diff --git a/apps/api/src/isms/documents/internal-audit-defaults.ts b/apps/api/src/isms/documents/internal-audit-defaults.ts new file mode 100644 index 0000000000..e0077d626f --- /dev/null +++ b/apps/api/src/isms/documents/internal-audit-defaults.ts @@ -0,0 +1,163 @@ +import type { SeedAuditControlDefinition } from './types'; + +/** + * Default text for the Internal Audit document (clause 9.2, CS-724). Every + * field ships with auditor-defensible template text a layperson can accept + * unedited; the customer may edit any of it. Kept ASCII-safe for the PDF + * renderer (jsPDF standard fonts cannot render characters like ≥). + */ + +/** The Programme paragraph, rendered verbatim into the generated document. */ +export function defaultProgrammeText(organizationName: string): string { + return `${organizationName} runs an annual internal audit of the whole ISMS. Each audit checks conformity to ISO/IEC 27001:2022 and effective implementation of the controls in the Statement of Applicability. Findings are tracked to closure through the platform and reviewed at the next management review.`; +} + +/** Default Scope for a new audit instance. */ +export const DEFAULT_AUDIT_SCOPE = + 'The whole ISMS as defined in the ISMS Scope Statement (Clause 4.3).'; + +/** Default Criteria for a new audit instance. */ +export const DEFAULT_AUDIT_CRITERIA = + 'ISO/IEC 27001:2022 and the Statement of Applicability.'; + +/** + * The bracketed verdict choices in the conclusion template: "Overall, this + * audit found the ISMS to [...] to ISO/IEC 27001:2022." + */ +export const CONCLUSION_VERDICT_TEXT: Record = { + conform: 'conform', + substantially_conform: + 'substantially conform with the non-conformities recorded below', + not_yet_conform: 'not yet conform', +}; + +/** Assemble the rendered conclusion sentence for a chosen verdict. */ +export function conclusionSentence(verdict: string): string { + const choice = CONCLUSION_VERDICT_TEXT[verdict] ?? verdict; + return `Overall, this audit found the ISMS to ${choice} to ISO/IEC 27001:2022. Corrective actions are tracked in the findings table.`; +} + +/** + * The fifteen default Controls Tested rows seeded per audit, covering the + * most-probed management-system clauses and high-impact Annex A controls. + * "Where to find it" entries begin with "Comp AI" to signal the default + * location; the customer overwrites the field when evidence lives elsewhere. + * Seeding is idempotent by controlKey (seedAuditControlsIfMissing) and never + * overwrites customer edits. + */ +export const SEED_AUDIT_CONTROL_DEFINITIONS: SeedAuditControlDefinition[] = [ + { + controlKey: 'clause_4_1_context', + controlRef: 'Clause 4.1 Context', + whatWasTested: + 'Whether the org has identified internal and external issues affecting the ISMS.', + whereToFind: 'Comp AI > ISMS > Documents > Context of the Organization', + }, + { + controlKey: 'clause_4_3_scope', + controlRef: 'Clause 4.3 Scope', + whatWasTested: + 'Whether the ISMS scope is documented, current, and reflects the certificate scope statement.', + whereToFind: 'Comp AI > ISMS > Documents > ISMS Scope Statement', + }, + { + controlKey: 'clause_5_2_policy', + controlRef: 'Clause 5.2 Policy', + whatWasTested: + 'Whether the information security policy is documented, approved, and communicated.', + whereToFind: 'Comp AI > Policies > Information Security & Privacy Governance', + }, + { + controlKey: 'clause_6_1_risk', + controlRef: 'Clause 6.1 Risk', + whatWasTested: + 'Whether risks affecting the ISMS are identified, assessed, and have a recorded treatment.', + whereToFind: 'Comp AI > Risks (risk register)', + }, + { + controlKey: 'clause_7_2_competence', + controlRef: 'Clause 7.2 Competence', + whatWasTested: + 'Whether the competence of persons doing ISMS work is defined and evidenced.', + whereToFind: 'Comp AI > ISMS > Roles > per-holder competence records', + }, + { + controlKey: 'clause_8_1_operational_planning', + controlRef: 'Clause 8.1 Operational planning', + whatWasTested: + 'Whether ISMS processes have been planned, implemented, and controlled.', + whereToFind: 'Comp AI > Policies + Controls + Tasks', + }, + { + controlKey: 'clause_9_1_monitoring', + controlRef: 'Clause 9.1 Monitoring', + whatWasTested: + 'Whether info-security performance and ISMS effectiveness are being measured.', + whereToFind: 'Comp AI > ISMS > Monitoring', + }, + { + controlKey: 'clause_9_3_management_review', + controlRef: 'Clause 9.3 Management review', + whatWasTested: + 'Whether top management reviews the ISMS at planned intervals with required inputs and outputs.', + whereToFind: 'Comp AI > ISMS > Management Review', + }, + { + controlKey: 'a_5_1_policies', + controlRef: 'A.5.1 Policies', + whatWasTested: + 'Whether an information security policy set is defined, approved, published and reviewed.', + whereToFind: 'Comp AI > Policies (full list)', + }, + { + controlKey: 'a_5_15_access_control', + controlRef: 'A.5.15 Access control', + whatWasTested: + 'Whether access rights are granted, reviewed and removed on a defined basis.', + whereToFind: + 'Comp AI > Policies > Access Control & Least Privilege + related evidence tasks', + }, + { + controlKey: 'a_5_19_supplier_relationships', + controlRef: 'A.5.19 Supplier relationships', + whatWasTested: + 'Whether information-security risks associated with suppliers are identified and managed.', + whereToFind: + 'Comp AI > Vendors > vendor register + Comp AI > Policies > Vendor & Third-Party Risk', + }, + { + controlKey: 'a_5_24_incident_management', + controlRef: 'A.5.24 Incident management', + whatWasTested: + 'Whether an information-security incident management process is defined and evidenced.', + whereToFind: + 'Comp AI > Policies > Incident Response & Breach Notification + Comp AI > Findings', + }, + { + controlKey: 'a_8_7_malware', + controlRef: 'A.8.7 Malware', + whatWasTested: + 'Whether protection against malware is in place and supported by user awareness.', + whereToFind: + 'Comp AI > Policies > Security Configuration Hardening & Anti-Malware', + }, + { + controlKey: 'a_8_13_backup', + controlRef: 'A.8.13 Backup', + whatWasTested: + 'Whether backups of information, software and systems are taken and tested.', + whereToFind: + "Comp AI > Evidence tasks tagged 'backup' + Comp AI > Policies > Backup, Business Continuity & DR", + }, + { + controlKey: 'a_8_24_cryptography', + controlRef: 'A.8.24 Cryptography', + whatWasTested: + 'Whether rules for the effective use of cryptography are defined and applied.', + whereToFind: 'Comp AI > Policies > Encryption & Crypto Controls', + }, +]; + +export const SEED_AUDIT_CONTROL_KEYS = SEED_AUDIT_CONTROL_DEFINITIONS.map( + (control) => control.controlKey, +); diff --git a/apps/api/src/isms/documents/internal-audit-export-data.spec.ts b/apps/api/src/isms/documents/internal-audit-export-data.spec.ts new file mode 100644 index 0000000000..aca27998a5 --- /dev/null +++ b/apps/api/src/isms/documents/internal-audit-export-data.spec.ts @@ -0,0 +1,177 @@ +import { + loadInternalAuditExtras, + mapAudits, + type AuditWithExportIncludes, +} from './internal-audit-export-data'; + +jest.mock('@db', () => ({ db: {} })); + +const extras = { + memberNames: { mem_1: 'Alex Petrisor', mem_2: 'jane@acme.io' }, +}; + +const baseAudit = { + id: 'aud_1', + documentId: 'doc_1', + reference: 'IA-2026-01', + scope: 'The whole ISMS.', + criteria: 'ISO/IEC 27001:2022 and the SoA.', + auditorName: 'Sarah Chen, Assured Compliance Ltd', + plannedStartDate: new Date('2026-05-15T00:00:00.000Z'), + plannedEndDate: new Date('2026-05-20T00:00:00.000Z'), + status: 'complete', + conclusionVerdict: 'substantially_conform', + conclusionNotes: 'Ready for Stage 2.', + signoffAuditorName: 'Sarah Chen', + signoffAuditorDate: new Date('2026-05-20T00:00:00.000Z'), + signoffSpoName: null, + signoffSpoDate: null, + signoffTopMgmtName: null, + signoffTopMgmtDate: null, + position: 0, + controls: [ + { + id: 'ac_1', + controlRef: 'Clause 6.1 Risk', + whatWasTested: 'Whether risks are identified and treated.', + whereToFind: 'Comp AI > Risks (risk register)', + result: 'observation_raised', + notes: 'See F-03.', + }, + { + id: 'ac_2', + controlRef: 'A.8.24 Cryptography', + whatWasTested: 'Whether crypto rules are applied.', + whereToFind: 'Comp AI > Policies > Encryption & Crypto Controls', + result: null, + notes: null, + }, + ], + findings: [ + { + id: 'af_1', + reference: 'F-01', + type: 'nc_minor', + clauseOrControl: 'Clause 9.1 (Monitoring)', + description: 'Three metrics unmeasured for 90 days.', + ownerMemberId: 'mem_1', + dueDate: new Date('2026-06-15T00:00:00.000Z'), + status: 'open', + closureEvidence: null, + }, + { + id: 'af_2', + reference: 'F-02', + type: 'ofi', + clauseOrControl: null, + description: 'Document the quarterly access review.', + ownerMemberId: 'mem_gone', + dueDate: null, + status: 'in_progress', + closureEvidence: null, + }, + ], +} as unknown as AuditWithExportIncludes; + +describe('mapAudits', () => { + it('maps an audit with humanized labels, dates and the conclusion sentence', () => { + const [row] = mapAudits([baseAudit], extras); + + expect(row.reference).toBe('IA-2026-01'); + expect(row.status).toBe('Complete'); + expect(row.plannedStartDate).toBe('2026-05-15'); + expect(row.plannedEndDate).toBe('2026-05-20'); + expect(row.conclusion).toBe( + 'Overall, this audit found the ISMS to substantially conform with the non-conformities recorded below to ISO/IEC 27001:2022. Corrective actions are tracked in the findings table.', + ); + expect(row.conclusionNotes).toBe('Ready for Stage 2.'); + }); + + it('maps control rows, dashing an unset result', () => { + const [row] = mapAudits([baseAudit], extras); + expect(row.controls).toEqual([ + { + controlRef: 'Clause 6.1 Risk', + whatWasTested: 'Whether risks are identified and treated.', + whereToFind: 'Comp AI > Risks (risk register)', + result: 'Observation raised', + notes: 'See F-03.', + }, + { + controlRef: 'A.8.24 Cryptography', + whatWasTested: 'Whether crypto rules are applied.', + whereToFind: 'Comp AI > Policies > Encryption & Crypto Controls', + result: '—', + notes: '', + }, + ]); + }); + + it('resolves finding owners, falling back for removed members', () => { + const [row] = mapAudits([baseAudit], extras); + expect(row.findings[0]).toMatchObject({ + reference: 'F-01', + type: 'NC minor', + ownerName: 'Alex Petrisor', + dueDate: '2026-06-15', + status: 'Open', + }); + expect(row.findings[1]).toMatchObject({ + type: 'OFI', + ownerName: 'Former member', + dueDate: '', + status: 'In progress', + }); + }); + + it('builds the three fixed sign-off slots in reference-document order', () => { + const [row] = mapAudits([baseAudit], extras); + expect(row.signoffs).toEqual([ + { role: 'Auditor', name: 'Sarah Chen', date: '2026-05-20' }, + { role: 'Information Security Manager / SPO', name: '', date: '' }, + { role: 'Top Management', name: '', date: '' }, + ]); + }); + + it('renders a null conclusion while no verdict is chosen', () => { + const [row] = mapAudits( + [ + { + ...baseAudit, + conclusionVerdict: null, + } as unknown as AuditWithExportIncludes, + ], + extras, + ); + expect(row.conclusion).toBeNull(); + }); +}); + +describe('loadInternalAuditExtras', () => { + it('resolves member display names with the name → email → placeholder fallback', async () => { + const client = { + member: { + findMany: jest.fn().mockResolvedValue([ + { id: 'mem_1', user: { name: 'Alex Petrisor', email: 'a@x.io' } }, + { id: 'mem_2', user: { name: null, email: 'jane@acme.io' } }, + { id: 'mem_3', user: null }, + ]), + }, + }; + + const result = await loadInternalAuditExtras({ + organizationId: 'org_1', + client: client as never, + }); + + expect(client.member.findMany).toHaveBeenCalledWith({ + where: { organizationId: 'org_1' }, + select: { id: true, user: { select: { name: true, email: true } } }, + }); + expect(result.memberNames).toEqual({ + mem_1: 'Alex Petrisor', + mem_2: 'jane@acme.io', + mem_3: 'Unknown member', + }); + }); +}); diff --git a/apps/api/src/isms/documents/internal-audit-export-data.ts b/apps/api/src/isms/documents/internal-audit-export-data.ts new file mode 100644 index 0000000000..f5058b8af6 --- /dev/null +++ b/apps/api/src/isms/documents/internal-audit-export-data.ts @@ -0,0 +1,150 @@ +import { db } from '@db'; +import type { Prisma } from '@db'; +import { conclusionSentence } from './internal-audit-defaults'; +import type { AuditExportRow, AuditSignoffExportRow } from './types'; + +/** + * Extra data the Internal Audit document (9.2) needs at export time but that + * isn't on the audit rows: display names for finding owners (findings store a + * plain memberId, no FK). Resolved once and frozen into the version snapshot + * so a historical export re-renders byte-faithfully with the names that were + * current at publish. + */ +export interface InternalAuditExtras { + /** memberId → display name (name, else email, else a placeholder). */ + memberNames: Record; +} + +type Client = Prisma.TransactionClient | typeof db; + +function memberDisplayName( + user: { name: string | null; email: string | null } | null, +): string { + return user?.name?.trim() || user?.email?.trim() || 'Unknown member'; +} + +/** Load the Internal Audit document's export extras for an organization. */ +export async function loadInternalAuditExtras({ + organizationId, + client, +}: { + organizationId: string; + client?: Client; +}): Promise { + const prisma = client ?? db; + + const members = await prisma.member.findMany({ + where: { organizationId }, + select: { id: true, user: { select: { name: true, email: true } } }, + }); + + const memberNames: Record = {}; + for (const member of members) { + memberNames[member.id] = memberDisplayName(member.user); + } + + return { memberNames }; +} + +/** The audit shape mapAudits consumes (EXPORT_DOCUMENT_INCLUDE's audits). */ +export type AuditWithExportIncludes = Prisma.IsmsAuditGetPayload<{ + include: { controls: true; findings: true }; +}>; + +export const AUDIT_STATUS_LABELS: Record = { + planned: 'Planned', + in_progress: 'In progress', + complete: 'Complete', +}; + +export const CONTROL_RESULT_LABELS: Record = { + conformity_confirmed: 'Conformity confirmed', + nonconformity_raised: 'Non-conformity raised', + observation_raised: 'Observation raised', + not_sampled: 'Not sampled this cycle', +}; + +export const FINDING_TYPE_LABELS: Record = { + nc_major: 'NC major', + nc_minor: 'NC minor', + ofi: 'OFI', + observation: 'Observation', +}; + +export const FINDING_STATUS_LABELS: Record = { + open: 'Open', + in_progress: 'In progress', + closed: 'Closed', +}; + +function formatDateYmd(date: Date | null): string | null { + return date ? date.toISOString().slice(0, 10) : null; +} + +/** The three fixed sign-off slots, in the order the reference document uses. */ +function mapSignoffs(audit: AuditWithExportIncludes): AuditSignoffExportRow[] { + return [ + { + role: 'Auditor', + name: audit.signoffAuditorName ?? '', + date: formatDateYmd(audit.signoffAuditorDate) ?? '', + }, + { + role: 'Information Security Manager / SPO', + name: audit.signoffSpoName ?? '', + date: formatDateYmd(audit.signoffSpoDate) ?? '', + }, + { + role: 'Top Management', + name: audit.signoffTopMgmtName ?? '', + date: formatDateYmd(audit.signoffTopMgmtDate) ?? '', + }, + ]; +} + +/** + * Map audit rows (with their controls and findings) into export rows, + * resolving owner names and humanizing enum labels. All audits render — a + * planned audit appears with its plan table and an empty report, which is the + * honest state of the programme. + */ +export function mapAudits( + audits: AuditWithExportIncludes[], + extras: InternalAuditExtras, +): AuditExportRow[] { + return audits.map((audit) => ({ + reference: audit.reference, + scope: audit.scope, + criteria: audit.criteria, + auditorName: audit.auditorName ?? '', + plannedStartDate: formatDateYmd(audit.plannedStartDate), + plannedEndDate: formatDateYmd(audit.plannedEndDate), + status: AUDIT_STATUS_LABELS[audit.status] ?? audit.status, + conclusion: audit.conclusionVerdict + ? conclusionSentence(audit.conclusionVerdict) + : null, + conclusionNotes: audit.conclusionNotes, + controls: audit.controls.map((control) => ({ + controlRef: control.controlRef, + whatWasTested: control.whatWasTested, + whereToFind: control.whereToFind, + result: control.result + ? (CONTROL_RESULT_LABELS[control.result] ?? control.result) + : '—', + notes: control.notes ?? '', + })), + findings: audit.findings.map((finding) => ({ + reference: finding.reference, + type: FINDING_TYPE_LABELS[finding.type] ?? finding.type, + clauseOrControl: finding.clauseOrControl ?? '', + description: finding.description, + ownerName: finding.ownerMemberId + ? (extras.memberNames[finding.ownerMemberId] ?? 'Former member') + : '', + dueDate: formatDateYmd(finding.dueDate) ?? '', + status: FINDING_STATUS_LABELS[finding.status] ?? finding.status, + closureEvidence: finding.closureEvidence ?? '', + })), + signoffs: mapSignoffs(audit), + })); +} diff --git a/apps/api/src/isms/documents/internal-audit.spec.ts b/apps/api/src/isms/documents/internal-audit.spec.ts new file mode 100644 index 0000000000..529fbf10f8 --- /dev/null +++ b/apps/api/src/isms/documents/internal-audit.spec.ts @@ -0,0 +1,352 @@ +import type { Prisma } from '@db'; +import { + auditValidationMessages, + buildInternalAuditSections, + deriveInternalAuditNarrative, + seedAuditControlsIfMissing, +} from './internal-audit'; +import { SEED_AUDIT_CONTROL_DEFINITIONS } from './internal-audit-defaults'; +import type { AuditExportRow, DocumentExportInput, IsmsPlatformData } from './types'; + +const baseInput: DocumentExportInput = { + contextIssues: [], + interestedParties: [], + requirements: [], + objectives: [], + narrative: { programme: 'Acme runs an annual internal audit of the whole ISMS.' }, +}; + +const audit: AuditExportRow = { + reference: 'IA-2026-01', + scope: 'The whole ISMS as defined in the ISMS Scope Statement (Clause 4.3).', + criteria: 'ISO/IEC 27001:2022 and the Statement of Applicability.', + auditorName: 'Sarah Chen, Assured Compliance Ltd', + plannedStartDate: '2026-05-15', + plannedEndDate: '2026-05-20', + status: 'Complete', + conclusion: + 'Overall, this audit found the ISMS to substantially conform with the non-conformities recorded below to ISO/IEC 27001:2022. Corrective actions are tracked in the findings table.', + conclusionNotes: null, + controls: [ + { + controlRef: 'Clause 9.1 Monitoring', + whatWasTested: 'Whether info-security performance is being measured.', + whereToFind: 'Comp AI > ISMS > Monitoring', + result: 'Non-conformity raised', + notes: 'Three metrics overdue. See F-01.', + }, + ], + findings: [ + { + reference: 'F-01', + type: 'NC minor', + clauseOrControl: 'Clause 9.1 (Monitoring)', + description: 'Three of nine metrics have no measurement in 90 days.', + ownerName: 'Alex Petrisor', + dueDate: '2026-06-15', + status: 'Open', + closureEvidence: '', + }, + ], + signoffs: [ + { role: 'Auditor', name: 'Sarah Chen', date: '2026-05-20' }, + { role: 'Information Security Manager / SPO', name: '', date: '' }, + { role: 'Top Management', name: 'Raoul Plickat', date: '2026-05-22' }, + ], +}; + +describe('auditValidationMessages (clause 9.2 submit gate)', () => { + it('requires at least one audit', () => { + expect(auditValidationMessages({ audits: [] })).toEqual([ + 'At least one internal audit must be recorded.', + ]); + }); + + it('requires a conclusion verdict on completed audits only', () => { + const messages = auditValidationMessages({ + audits: [ + { reference: 'IA-2026-01', status: 'complete', conclusionVerdict: null }, + { reference: 'IA-2026-02', status: 'in_progress', conclusionVerdict: null }, + ], + }); + expect(messages).toEqual([ + 'Audit IA-2026-01 is complete but has no conclusion verdict.', + ]); + }); + + it('passes with a planned audit or a concluded complete audit', () => { + expect( + auditValidationMessages({ + audits: [ + { reference: 'IA-2026-01', status: 'planned', conclusionVerdict: null }, + ], + }), + ).toEqual([]); + expect( + auditValidationMessages({ + audits: [ + { + reference: 'IA-2026-01', + status: 'complete', + conclusionVerdict: 'conform', + }, + ], + }), + ).toEqual([]); + }); +}); + +describe('deriveInternalAuditNarrative', () => { + it('templates the programme paragraph with the organization name', () => { + const narrative = deriveInternalAuditNarrative({ + organizationName: 'Acme Corp', + } as IsmsPlatformData); + expect(narrative.programme).toContain('Acme Corp runs an annual internal audit'); + expect(narrative.programme).toContain('ISO/IEC 27001:2022'); + }); +}); + +describe('seedAuditControlsIfMissing', () => { + const makeTx = ( + existing: Array<{ controlKey: string | null; position: number }>, + ) => { + const tx = { + ismsAuditControl: { + findMany: jest.fn().mockResolvedValue(existing), + createMany: jest.fn().mockResolvedValue({ count: 0 }), + }, + }; + return tx as unknown as Prisma.TransactionClient & typeof tx; + }; + + it('seeds all fifteen defaults into a new audit', async () => { + const tx = makeTx([]); + await seedAuditControlsIfMissing({ + tx, + auditId: 'aud_1', + documentId: 'doc_1', + }); + + const { data, skipDuplicates } = ( + tx.ismsAuditControl.createMany as jest.Mock + ).mock.calls[0][0]; + expect(skipDuplicates).toBe(true); + expect(data).toHaveLength(15); + expect(data.map((row: { controlKey: string }) => row.controlKey)).toEqual( + SEED_AUDIT_CONTROL_DEFINITIONS.map((control) => control.controlKey), + ); + expect(data[0]).toMatchObject({ + auditId: 'aud_1', + documentId: 'doc_1', + source: 'derived', + derivedFrom: `seed:${SEED_AUDIT_CONTROL_DEFINITIONS[0].controlKey}`, + position: 0, + }); + // Result deliberately unset: the auditor records it per row. + expect(data[0].result).toBeUndefined(); + }); + + it('creates only the missing seeds, after existing positions', async () => { + const tx = makeTx([ + { controlKey: 'clause_4_1_context', position: 0 }, + { controlKey: null, position: 20 }, // custom row + ]); + await seedAuditControlsIfMissing({ + tx, + auditId: 'aud_1', + documentId: 'doc_1', + }); + + const { data } = (tx.ismsAuditControl.createMany as jest.Mock).mock + .calls[0][0]; + expect(data).toHaveLength(14); + expect( + data.map((row: { controlKey: string }) => row.controlKey), + ).not.toContain('clause_4_1_context'); + expect(data[0].position).toBe(21); + }); + + it('is a no-op when every seed already exists (never deletes/overwrites)', async () => { + const tx = makeTx( + SEED_AUDIT_CONTROL_DEFINITIONS.map((control, index) => ({ + controlKey: control.controlKey, + position: index, + })), + ); + await seedAuditControlsIfMissing({ + tx, + auditId: 'aud_1', + documentId: 'doc_1', + }); + expect(tx.ismsAuditControl.createMany).not.toHaveBeenCalled(); + }); +}); + +describe('buildInternalAuditSections', () => { + it('renders the reference-document sections in order for a single audit', () => { + const sections = buildInternalAuditSections({ + ...baseInput, + audits: [audit], + }); + expect(sections.map((section) => section.heading)).toEqual([ + 'Purpose', + 'Programme', + 'Audit plan', + 'Controls Tested', + 'Findings', + 'Conclusion', + 'Sign-off', + ]); + }); + + it('suffixes per-audit headings with the reference when several audits exist', () => { + const sections = buildInternalAuditSections({ + ...baseInput, + audits: [audit, { ...audit, reference: 'IA-2027-01' }], + }); + const headings = sections.map((section) => section.heading); + expect(headings).toContain('Audit plan — IA-2026-01'); + expect(headings).toContain('Sign-off — IA-2027-01'); + }); + + it('renders the programme paragraph verbatim and an empty-audits placeholder', () => { + const sections = buildInternalAuditSections({ ...baseInput, audits: [] }); + expect(sections[1].paragraphs?.[0]?.text).toBe( + 'Acme runs an annual internal audit of the whole ISMS.', + ); + expect(sections[2].heading).toBe('Audits'); + expect(sections[2].emptyText).toBe('No internal audits recorded yet.'); + }); + + it('shows a programme placeholder when the narrative is missing or invalid', () => { + const sections = buildInternalAuditSections({ + ...baseInput, + narrative: null, + audits: [], + }); + expect(sections[1].emptyText).toBe('No audit programme recorded.'); + }); + + it('renders the audit plan as label/value rows', () => { + const sections = buildInternalAuditSections({ + ...baseInput, + audits: [audit], + }); + expect(sections[2].keyValues).toEqual([ + { label: 'Reference', value: 'IA-2026-01' }, + { + label: 'Scope', + value: + 'The whole ISMS as defined in the ISMS Scope Statement (Clause 4.3).', + }, + { + label: 'Criteria', + value: 'ISO/IEC 27001:2022 and the Statement of Applicability.', + }, + { label: 'Auditor', value: 'Sarah Chen, Assured Compliance Ltd' }, + { label: 'Planned start date', value: '2026-05-15' }, + { label: 'Planned end date', value: '2026-05-20' }, + { label: 'Status', value: 'Complete' }, + ]); + }); + + it('renders the five-column Controls Tested table', () => { + const sections = buildInternalAuditSections({ + ...baseInput, + audits: [audit], + }); + const table = sections[3].table; + expect(table?.headers).toEqual([ + 'Control reference', + 'What was tested', + 'Where to find it', + 'Result', + 'Notes', + ]); + expect(table?.rows).toEqual([ + [ + 'Clause 9.1 Monitoring', + 'Whether info-security performance is being measured.', + 'Comp AI > ISMS > Monitoring', + 'Non-conformity raised', + 'Three metrics overdue. See F-01.', + ], + ]); + }); + + it('renders the seven-column findings table, or "No findings raised."', () => { + const withFindings = buildInternalAuditSections({ + ...baseInput, + audits: [audit], + }); + expect(withFindings[4].table?.headers).toEqual([ + 'Ref', + 'Type', + 'Clause / control', + 'Description', + 'Owner', + 'Due date', + 'Status', + ]); + + const noFindings = buildInternalAuditSections({ + ...baseInput, + audits: [{ ...audit, findings: [] }], + }); + expect(noFindings[4].emptyText).toBe('No findings raised.'); + expect(noFindings[4].table).toBeUndefined(); + // No intro either: the renderers show emptyText only for empty sections. + expect(noFindings[4].intro).toBeUndefined(); + }); + + it('appends closure evidence to a finding description in the export', () => { + const sections = buildInternalAuditSections({ + ...baseInput, + audits: [ + { + ...audit, + findings: [ + { + ...audit.findings[0], + status: 'Closed', + closureEvidence: 'Restore test evidenced in task ev_123.', + }, + ], + }, + ], + }); + expect(sections[4].table?.rows[0][3]).toBe( + 'Three of nine metrics have no measurement in 90 days. — Closure evidence: Restore test evidenced in task ev_123.', + ); + }); + + it('renders the conclusion sentence plus notes, or a placeholder', () => { + const withNotes = buildInternalAuditSections({ + ...baseInput, + audits: [{ ...audit, conclusionNotes: 'Ready for Stage 2.' }], + }); + expect(withNotes[5].paragraphs?.map((paragraph) => paragraph.text)).toEqual([ + audit.conclusion, + 'Ready for Stage 2.', + ]); + + const noVerdict = buildInternalAuditSections({ + ...baseInput, + audits: [{ ...audit, conclusion: null, conclusionNotes: null }], + }); + expect(noVerdict[5].emptyText).toBe('No conclusion recorded yet.'); + }); + + it('renders the three sign-off slots with dashes for unsigned slots', () => { + const sections = buildInternalAuditSections({ + ...baseInput, + audits: [audit], + }); + expect(sections[6].table?.headers).toEqual(['Role', 'Signatory', 'Date']); + expect(sections[6].table?.rows).toEqual([ + ['Auditor', 'Sarah Chen', '2026-05-20'], + ['Information Security Manager / SPO', '—', '—'], + ['Top Management', 'Raoul Plickat', '2026-05-22'], + ]); + }); +}); diff --git a/apps/api/src/isms/documents/internal-audit.ts b/apps/api/src/isms/documents/internal-audit.ts new file mode 100644 index 0000000000..03e34de7ae --- /dev/null +++ b/apps/api/src/isms/documents/internal-audit.ts @@ -0,0 +1,285 @@ +import { z } from 'zod'; +import type { Prisma } from '@db'; +import type { IsmsExportSection } from '../utils/export-shared'; +import type { + AuditExportRow, + DocumentExportInput, + IsmsPlatformData, +} from './types'; +import { + defaultProgrammeText, + SEED_AUDIT_CONTROL_DEFINITIONS, +} from './internal-audit-defaults'; + +type Tx = Prisma.TransactionClient; + +/** + * Narrative shape persisted on the Internal Audit document (clause 9.2): the + * Programme paragraph shown at the top of the page and rendered verbatim into + * the generated document. The audits themselves live in their own registers. + */ +export const internalAuditNarrativeSchema = z.object({ + programme: z.string().trim().min(1), +}); + +export type InternalAuditNarrative = z.infer< + typeof internalAuditNarrativeSchema +>; + +/** Derive the default Programme paragraph (hardcoded default, editable). */ +export function deriveInternalAuditNarrative( + data: IsmsPlatformData, +): InternalAuditNarrative { + return { programme: defaultProgrammeText(data.organizationName) }; +} + +/** + * Clause-9.2 completeness check, shared by the submit-for-approval server gate + * and the client Submit button (internal-audit-constants.ts mirrors it). + * Requires at least one audit instance (an ISMS with no internal audit is the + * Stage-2 blocker this feature exists to remove) and a conclusion verdict on + * every completed audit (the document would otherwise render a conclusion with + * an unselected bracket). Returns unmet requirements; empty = ready. + */ +export function auditValidationMessages({ + audits, +}: { + audits: Array<{ + reference: string; + status: string; + conclusionVerdict: string | null; + }>; +}): string[] { + if (audits.length === 0) { + return ['At least one internal audit must be recorded.']; + } + return audits + .filter((audit) => audit.status === 'complete' && !audit.conclusionVerdict) + .map( + (audit) => + `Audit ${audit.reference} is complete but has no conclusion verdict.`, + ); +} + +/** + * Seed the fifteen default Controls Tested rows for an audit, idempotently by + * `controlKey`. Only creates seed rows that are missing — it NEVER deletes or + * overwrites, so re-running can never clobber the customer's edits, results, + * or notes (same guarantee as seedMetricsIfMissing). Called when an audit + * instance is created. + */ +export async function seedAuditControlsIfMissing({ + tx, + auditId, + documentId, +}: { + tx: Tx; + auditId: string; + documentId: string; +}): Promise { + const existing = await tx.ismsAuditControl.findMany({ + where: { auditId }, + select: { controlKey: true, position: true }, + }); + const existingKeys = new Set( + existing + .map((control) => control.controlKey) + .filter((key): key is string => !!key), + ); + const missing = SEED_AUDIT_CONTROL_DEFINITIONS.filter( + (control) => !existingKeys.has(control.controlKey), + ); + if (missing.length === 0) return; + + const maxPosition = existing.reduce( + (max, control) => Math.max(max, control.position), + -1, + ); + + await tx.ismsAuditControl.createMany({ + data: missing.map((control, index) => ({ + auditId, + documentId, + controlKey: control.controlKey, + controlRef: control.controlRef, + whatWasTested: control.whatWasTested, + whereToFind: control.whereToFind, + source: 'derived' as const, + derivedFrom: `seed:${control.controlKey}`, + position: maxPosition + 1 + index, + })), + // Belt-and-braces with @@unique([auditId, controlKey]): a concurrent + // create racing this seed is absorbed silently. + skipDuplicates: true, + }); +} + +// ---- Export section builder ------------------------------------------------- + +function parseProgramme(narrative: unknown): string | null { + const parsed = internalAuditNarrativeSchema.safeParse(narrative); + return parsed.success ? parsed.data.programme : null; +} + +/** Per-audit sections: plan table, Controls Tested, Findings, Conclusion, Sign-off. */ +function buildAuditSections( + audit: AuditExportRow, + suffix: string, +): IsmsExportSection[] { + const sections: IsmsExportSection[] = [ + { + heading: `Audit plan${suffix}`, + keyValues: [ + { label: 'Reference', value: audit.reference }, + { label: 'Scope', value: audit.scope }, + { label: 'Criteria', value: audit.criteria }, + { label: 'Auditor', value: audit.auditorName || '—' }, + { label: 'Planned start date', value: audit.plannedStartDate ?? '—' }, + { label: 'Planned end date', value: audit.plannedEndDate ?? '—' }, + { label: 'Status', value: audit.status }, + ], + }, + ]; + + // The renderers show emptyText only when a section has no other content, so + // the intro is included only when there are rows to introduce. + if (audit.controls.length > 0) { + sections.push({ + heading: `Controls Tested${suffix}`, + intro: + 'Each control below was sampled by the auditor. The "Where to find it" column names the location the auditor referenced. The "Result" column records the auditor\'s finding for each control. Rows marked "Not sampled this cycle" were deliberately scoped out of this audit.', + table: { + headers: [ + 'Control reference', + 'What was tested', + 'Where to find it', + 'Result', + 'Notes', + ], + rows: audit.controls.map((control) => [ + control.controlRef, + control.whatWasTested, + control.whereToFind, + control.result, + control.notes, + ]), + }, + }); + } else { + sections.push({ + heading: `Controls Tested${suffix}`, + emptyText: 'No controls recorded for this audit.', + }); + } + + if (audit.findings.length > 0) { + sections.push({ + heading: `Findings${suffix}`, + intro: + 'Non-conformities, opportunities for improvement (OFIs), and observations raised during this audit. Each is tracked to closure in Comp AI.', + table: { + headers: [ + 'Ref', + 'Type', + 'Clause / control', + 'Description', + 'Owner', + 'Due date', + 'Status', + ], + rows: audit.findings.map((finding) => [ + finding.reference, + finding.type, + finding.clauseOrControl, + // Closure evidence rides in the description cell so the exported + // record shows how a corrective action was evidenced without + // widening the reference document's seven-column table. + finding.closureEvidence + ? `${finding.description} — Closure evidence: ${finding.closureEvidence}` + : finding.description, + finding.ownerName, + finding.dueDate, + finding.status, + ]), + }, + }); + } else { + sections.push({ + heading: `Findings${suffix}`, + emptyText: 'No findings raised.', + }); + } + + sections.push( + audit.conclusion + ? { + heading: `Conclusion${suffix}`, + paragraphs: [ + { text: audit.conclusion }, + ...(audit.conclusionNotes ? [{ text: audit.conclusionNotes }] : []), + ], + } + : { + heading: `Conclusion${suffix}`, + emptyText: 'No conclusion recorded yet.', + }, + { + heading: `Sign-off${suffix}`, + table: { + headers: ['Role', 'Signatory', 'Date'], + rows: audit.signoffs.map((signoff) => [ + signoff.role, + signoff.name || '—', + signoff.date || '—', + ]), + }, + }, + ); + + return sections; +} + +/** + * Build the Internal Audit Programme, Plan and Report document (clause 9.2). + * Contents and order follow the CS-724 ticket and reference document: Purpose, + * Programme, then per audit its plan, Controls Tested, Findings, Conclusion, + * and Sign-off. `audits` (names and labels already resolved) is populated by + * loadInternalAuditExtras at export-input assembly (see + * internal-audit-export-data.ts). With a single audit the headings match the + * reference document verbatim; with several, each block carries its reference. + */ +export function buildInternalAuditSections( + input: DocumentExportInput, +): IsmsExportSection[] { + const audits = input.audits ?? []; + const programme = parseProgramme(input.narrative); + + const sections: IsmsExportSection[] = [ + { + heading: 'Purpose', + paragraphs: [ + { + text: 'This document records the internal audit programme and the results of the internal audits of the Information Security Management System, in accordance with ISO/IEC 27001:2022, Clause 9.2. It is retained as documented information and made available to the certification body on request.', + }, + ], + }, + programme + ? { heading: 'Programme', paragraphs: [{ text: programme }] } + : { heading: 'Programme', emptyText: 'No audit programme recorded.' }, + ]; + + if (audits.length === 0) { + sections.push({ + heading: 'Audits', + emptyText: 'No internal audits recorded yet.', + }); + return sections; + } + + for (const audit of audits) { + const suffix = audits.length > 1 ? ` — ${audit.reference}` : ''; + sections.push(...buildAuditSections(audit, suffix)); + } + + return sections; +} diff --git a/apps/api/src/isms/documents/registry.ts b/apps/api/src/isms/documents/registry.ts index 759e8b0e67..4f9d1cb298 100644 --- a/apps/api/src/isms/documents/registry.ts +++ b/apps/api/src/isms/documents/registry.ts @@ -7,6 +7,11 @@ import { buildRequirementsSections } from './requirements'; import { buildObjectivesSections } from './objectives'; import { buildRolesSections } from './roles'; import { buildMonitoringSections } from './monitoring'; +import { + buildInternalAuditSections, + deriveInternalAuditNarrative, + internalAuditNarrativeSchema, +} from './internal-audit'; import { buildScopeSections, deriveScopeNarrative, @@ -35,6 +40,7 @@ const EXPORT_SECTION_BUILDERS: Record< objectives_plan: buildObjectivesSections, roles_and_responsibilities: buildRolesSections, monitoring: buildMonitoringSections, + internal_audit: buildInternalAuditSections, isms_scope: buildScopeSections, leadership_commitment: buildLeadershipSections, }; @@ -49,16 +55,22 @@ export function buildExportSections({ return EXPORT_SECTION_BUILDERS[type](input); } -/** Zod schema validating the narrative payload for each singleton document type. */ +/** + * Zod schema validating the narrative payload for each document type that + * stores one. Covers the singleton documents plus the Internal Audit document, + * whose narrative holds only the Programme paragraph (its audits live in their + * own registers, so it is NOT a narrative type). + */ export function narrativeSchemaForType( type: IsmsDocumentType, ): ZodTypeAny | null { if (type === 'isms_scope') return ismsScopeNarrativeSchema; if (type === 'leadership_commitment') return leadershipNarrativeSchema; + if (type === 'internal_audit') return internalAuditNarrativeSchema; return null; } -/** Derive the default narrative payload for a singleton document type. */ +/** Derive the default narrative payload for a document type that stores one. */ export function deriveNarrativeForType({ type, data, @@ -68,6 +80,7 @@ export function deriveNarrativeForType({ }): Record | null { if (type === 'isms_scope') return deriveScopeNarrative(data); if (type === 'leadership_commitment') return deriveLeadershipNarrative(data); + if (type === 'internal_audit') return deriveInternalAuditNarrative(data); return null; } diff --git a/apps/api/src/isms/documents/snapshot.ts b/apps/api/src/isms/documents/snapshot.ts index e810cae04a..3bb06ebe61 100644 --- a/apps/api/src/isms/documents/snapshot.ts +++ b/apps/api/src/isms/documents/snapshot.ts @@ -94,6 +94,9 @@ const TYPE_DRIFT_SOURCES: Record> = { // static defaults + customer edits + measurements); no platform snapshot // input changes its rendering, so it can never be platform-drift stale. monitoring: [], + // Internal Audit (9.2) renders from its own audits register + the Programme + // narrative (customer-owned once seeded); it never goes platform-drift stale. + internal_audit: [], isms_scope: [ 'frameworks', 'vendors', diff --git a/apps/api/src/isms/documents/types.ts b/apps/api/src/isms/documents/types.ts index 53e13db03d..17c9ec653d 100644 --- a/apps/api/src/isms/documents/types.ts +++ b/apps/api/src/isms/documents/types.ts @@ -146,6 +146,64 @@ export interface MetricExportRow { currentValue: string; } +/** The fifteen seeded Controls Tested rows and their pre-filled text (9.2). */ +export interface SeedAuditControlDefinition { + controlKey: string; + controlRef: string; + whatWasTested: string; + whereToFind: string; +} + +/** One Controls Tested row, resolved for export (result/notes humanized). */ +export interface AuditControlExportRow { + controlRef: string; + whatWasTested: string; + whereToFind: string; + /** Humanized result label ("Conformity confirmed") or a dash when unset. */ + result: string; + notes: string; +} + +/** One finding row, resolved for export (owner name frozen at build time). */ +export interface AuditFindingExportRow { + reference: string; + /** Humanized type label ("NC minor"). */ + type: string; + clauseOrControl: string; + description: string; + ownerName: string; + dueDate: string; + /** Humanized status label ("Open"). */ + status: string; + /** How a closed corrective action was evidenced; empty when not recorded. */ + closureEvidence: string; +} + +/** One sign-off slot rendered in the audit's sign-off table. */ +export interface AuditSignoffExportRow { + role: string; + name: string; + date: string; +} + +/** An audit instance, resolved for export: fields + child tables + sign-off. */ +export interface AuditExportRow { + reference: string; + scope: string; + criteria: string; + auditorName: string; + plannedStartDate: string | null; + plannedEndDate: string | null; + /** Humanized status label ("In progress"). */ + status: string; + /** Assembled conclusion sentence, or null while no verdict is chosen. */ + conclusion: string | null; + conclusionNotes: string | null; + controls: AuditControlExportRow[]; + findings: AuditFindingExportRow[]; + signoffs: AuditSignoffExportRow[]; +} + /** * The organization profile that fills the narrative parts of the Context of the * Organization document (clause 4.1) — overview table, mission, intended @@ -197,4 +255,6 @@ export interface DocumentExportInput { band?: IsmsTeamSizeBand; /** Active metrics with resolved people + values — only populated for the Monitoring document (9.1). */ metrics?: MetricExportRow[]; + /** Audit instances with resolved names — only populated for the Internal Audit document (9.2). */ + audits?: AuditExportRow[]; } diff --git a/apps/api/src/isms/isms-audit-control.service.spec.ts b/apps/api/src/isms/isms-audit-control.service.spec.ts new file mode 100644 index 0000000000..3ebd845f4a --- /dev/null +++ b/apps/api/src/isms/isms-audit-control.service.spec.ts @@ -0,0 +1,169 @@ +import { NotFoundException } from '@nestjs/common'; +import { db } from '@db'; +import { IsmsAuditControlService } from './isms-audit-control.service'; + +jest.mock('@db', () => { + const db = { + ismsDocument: { + findUnique: jest.fn(), + update: jest.fn(), + }, + ismsAudit: { findFirst: jest.fn() }, + ismsAuditControl: { + findFirst: jest.fn(), + create: jest.fn(), + update: jest.fn(), + delete: jest.fn(), + }, + ismsAuditFinding: { updateMany: jest.fn() }, + $executeRaw: jest.fn(), + $transaction: jest.fn((cb: (tx: unknown) => unknown) => cb(db)), + }; + return { db }; +}); + +const mockDb = jest.mocked(db); + +describe('IsmsAuditControlService', () => { + let service: IsmsAuditControlService; + + beforeEach(() => { + jest.clearAllMocks(); + (mockDb.ismsDocument.findUnique as jest.Mock).mockResolvedValue({ + status: 'draft', + }); + service = new IsmsAuditControlService(); + }); + + describe('create', () => { + const args = { + documentId: 'doc_1', + organizationId: 'org_1', + dto: { auditId: 'aud_1', controlRef: 'A.8.16 Monitoring activities' }, + }; + + it('requires the audit to belong to the org + document (type-scoped)', async () => { + (mockDb.ismsAudit.findFirst as jest.Mock).mockResolvedValue(null); + await expect(service.create(args)).rejects.toThrow(NotFoundException); + expect(mockDb.ismsAudit.findFirst).toHaveBeenCalledWith({ + where: { + id: 'aud_1', + documentId: 'doc_1', + document: { organizationId: 'org_1', type: 'internal_audit' }, + }, + }); + }); + + it('creates a customer row (controlKey null, source manual) after the last position', async () => { + (mockDb.ismsAudit.findFirst as jest.Mock).mockResolvedValue({ + id: 'aud_1', + documentId: 'doc_1', + }); + (mockDb.ismsAuditControl.findFirst as jest.Mock).mockResolvedValue({ + position: 14, + }); + (mockDb.ismsAuditControl.create as jest.Mock).mockResolvedValue({}); + + await service.create(args); + + expect(mockDb.ismsAuditControl.create).toHaveBeenCalledWith({ + data: expect.objectContaining({ + auditId: 'aud_1', + documentId: 'doc_1', + controlKey: null, + controlRef: 'A.8.16 Monitoring activities', + result: null, + source: 'manual', + position: 15, + }), + }); + }); + }); + + describe('update', () => { + beforeEach(() => { + (mockDb.ismsAuditControl.findFirst as jest.Mock).mockResolvedValue({ + id: 'ac_1', + documentId: 'doc_1', + controlKey: 'clause_4_1_context', + source: 'derived', + }); + (mockDb.ismsAuditControl.update as jest.Mock).mockResolvedValue({}); + }); + + it('records a result and flips source to manual (seeded rows editable)', async () => { + await service.update({ + controlId: 'ac_1', + organizationId: 'org_1', + dto: { result: 'conformity_confirmed' }, + }); + + expect(mockDb.ismsAuditControl.update).toHaveBeenCalledWith({ + where: { id: 'ac_1' }, + data: expect.objectContaining({ + result: 'conformity_confirmed', + source: 'manual', + }), + }); + }); + + it('clears a result back to unset with null', async () => { + await service.update({ + controlId: 'ac_1', + organizationId: 'org_1', + dto: { result: null, notes: '' }, + }); + + const { data } = (mockDb.ismsAuditControl.update as jest.Mock).mock + .calls[0][0]; + expect(data.result).toBeNull(); + expect(data.notes).toBeNull(); + }); + + it('throws NotFoundException for a row outside the organization', async () => { + (mockDb.ismsAuditControl.findFirst as jest.Mock).mockResolvedValue(null); + await expect( + service.update({ controlId: 'ac_x', organizationId: 'org_1', dto: {} }), + ).rejects.toThrow(NotFoundException); + }); + }); + + describe('remove', () => { + it('deletes any row, including seeded ones (removals stick)', async () => { + (mockDb.ismsAuditControl.findFirst as jest.Mock).mockResolvedValue({ + id: 'ac_1', + documentId: 'doc_1', + controlKey: 'clause_4_1_context', + controlRef: 'Clause 4.1 Context', + }); + (mockDb.ismsAuditControl.delete as jest.Mock).mockResolvedValue({}); + + const result = await service.remove({ + controlId: 'ac_1', + organizationId: 'org_1', + }); + + expect(result).toEqual({ success: true }); + expect(mockDb.ismsAuditControl.delete).toHaveBeenCalledWith({ + where: { id: 'ac_1' }, + }); + }); + + it('labels linked label-less findings with the row reference before deleting', async () => { + (mockDb.ismsAuditControl.findFirst as jest.Mock).mockResolvedValue({ + id: 'ac_1', + documentId: 'doc_1', + controlKey: null, + controlRef: 'A.8.13 Backup', + }); + (mockDb.ismsAuditControl.delete as jest.Mock).mockResolvedValue({}); + + await service.remove({ controlId: 'ac_1', organizationId: 'org_1' }); + + expect(mockDb.ismsAuditFinding.updateMany).toHaveBeenCalledWith({ + where: { controlId: 'ac_1', clauseOrControl: null }, + data: { clauseOrControl: 'A.8.13 Backup' }, + }); + }); + }); +}); diff --git a/apps/api/src/isms/isms-audit-control.service.ts b/apps/api/src/isms/isms-audit-control.service.ts new file mode 100644 index 0000000000..70ac8645a7 --- /dev/null +++ b/apps/api/src/isms/isms-audit-control.service.ts @@ -0,0 +1,171 @@ +import { Injectable, NotFoundException } from '@nestjs/common'; +import { db } from '@db'; +import type { Prisma } from '@db'; +import { invalidateApprovalIfNeeded } from './utils/approval'; +import { lockDocument } from './utils/document-lock'; +import type { + CreateAuditControlInput, + UpdateAuditControlInput, +} from './registers/register-registry'; + +/** + * CRUD for an audit's Controls Tested rows (clause 9.2, CS-724). The fifteen + * default rows are created idempotently by seedAuditControlsIfMissing when the + * audit is created; this service handles customer-added rows and edits. The + * ticket allows adding, editing, and removing rows freely — seeded rows are + * deletable too (nothing re-seeds an existing audit, so they stay removed). + * Editing flips source to 'manual' (records the override). + */ +@Injectable() +export class IsmsAuditControlService { + async create({ + documentId, + organizationId, + dto, + }: { + documentId: string; + organizationId: string; + dto: CreateAuditControlInput; + }) { + const audit = await this.requireAudit({ + auditId: dto.auditId, + documentId, + organizationId, + }); + + return db.$transaction(async (tx) => { + await lockDocument(tx, audit.documentId); + const position = + dto.position ?? (await this.nextPosition({ tx, auditId: audit.id })); + await invalidateApprovalIfNeeded({ tx, documentId: audit.documentId }); + return tx.ismsAuditControl.create({ + data: { + auditId: audit.id, + documentId: audit.documentId, + controlKey: null, // customer-added row + controlRef: dto.controlRef, + whatWasTested: dto.whatWasTested ?? '', + whereToFind: dto.whereToFind ?? '', + result: dto.result ?? null, + notes: dto.notes?.trim() || null, + source: 'manual', + position, + }, + }); + }); + } + + async update({ + controlId, + organizationId, + dto, + }: { + controlId: string; + organizationId: string; + dto: UpdateAuditControlInput; + }) { + const control = await this.requireControl({ controlId, organizationId }); + + return db.$transaction(async (tx) => { + // Same per-document lock submit/approve take, so an edit can't race the + // submission's completeness check. + await lockDocument(tx, control.documentId); + await invalidateApprovalIfNeeded({ tx, documentId: control.documentId }); + return tx.ismsAuditControl.update({ + where: { id: controlId }, + data: { + controlRef: dto.controlRef ?? undefined, + whatWasTested: dto.whatWasTested ?? undefined, + whereToFind: dto.whereToFind ?? undefined, + result: dto.result === undefined ? undefined : dto.result, + notes: + dto.notes === undefined ? undefined : dto.notes?.trim() || null, + position: dto.position ?? undefined, + source: 'manual', + }, + }); + }); + } + + async remove({ + controlId, + organizationId, + }: { + controlId: string; + organizationId: string; + }) { + const control = await this.requireControl({ controlId, organizationId }); + await db.$transaction(async (tx) => { + await lockDocument(tx, control.documentId); + await invalidateApprovalIfNeeded({ tx, documentId: control.documentId }); + // Findings linked to this row survive: the FK is SET NULL and each + // finding keeps its clauseOrControl text as the frozen label. A linked + // finding created without one (possible via the API) inherits the row's + // reference now, so no finding is left unlabelled by the deletion. + await tx.ismsAuditFinding.updateMany({ + where: { controlId, clauseOrControl: null }, + data: { clauseOrControl: control.controlRef }, + }); + await tx.ismsAuditControl.delete({ where: { id: controlId } }); + }); + return { success: true }; + } + + private async nextPosition({ + tx, + auditId, + }: { + tx: Prisma.TransactionClient; + auditId: string; + }) { + const last = await tx.ismsAuditControl.findFirst({ + where: { auditId }, + orderBy: { position: 'desc' }, + select: { position: true }, + }); + return (last?.position ?? -1) + 1; + } + + private async requireAudit({ + auditId, + documentId, + organizationId, + }: { + auditId: string; + documentId: string; + organizationId: string; + }) { + // The audit must belong to THIS org's Internal Audit document (and to the + // document in the URL, so a row can't be attached across documents). + const audit = await db.ismsAudit.findFirst({ + where: { + id: auditId, + documentId, + document: { organizationId, type: 'internal_audit' }, + }, + }); + if (!audit) { + throw new NotFoundException('Audit not found'); + } + return audit; + } + + private async requireControl({ + controlId, + organizationId, + }: { + controlId: string; + organizationId: string; + }) { + const control = await db.ismsAuditControl.findFirst({ + where: { + id: controlId, + audit: { document: { organizationId, type: 'internal_audit' } }, + }, + }); + if (!control) { + throw new NotFoundException('Audit control row not found'); + } + return control; + } +} diff --git a/apps/api/src/isms/isms-audit-finding.service.spec.ts b/apps/api/src/isms/isms-audit-finding.service.spec.ts new file mode 100644 index 0000000000..14ecdd3e6a --- /dev/null +++ b/apps/api/src/isms/isms-audit-finding.service.spec.ts @@ -0,0 +1,271 @@ +import { NotFoundException } from '@nestjs/common'; +import { db } from '@db'; +import { IsmsAuditFindingService } from './isms-audit-finding.service'; + +jest.mock('@db', () => { + const db = { + ismsDocument: { + findUnique: jest.fn(), + update: jest.fn(), + }, + ismsAudit: { findFirst: jest.fn() }, + ismsAuditControl: { findFirst: jest.fn() }, + member: { findFirst: jest.fn() }, + ismsAuditFinding: { + findFirst: jest.fn(), + findMany: jest.fn(), + create: jest.fn(), + update: jest.fn(), + delete: jest.fn(), + }, + $executeRaw: jest.fn(), + $transaction: jest.fn((cb: (tx: unknown) => unknown) => cb(db)), + }; + return { db }; +}); + +const mockDb = jest.mocked(db); + +describe('IsmsAuditFindingService', () => { + let service: IsmsAuditFindingService; + + beforeEach(() => { + jest.clearAllMocks(); + (mockDb.ismsDocument.findUnique as jest.Mock).mockResolvedValue({ + status: 'draft', + }); + (mockDb.ismsAuditFinding.findMany as jest.Mock).mockResolvedValue([]); + service = new IsmsAuditFindingService(); + }); + + describe('create', () => { + const args = { + documentId: 'doc_1', + organizationId: 'org_1', + dto: { + auditId: 'aud_1', + type: 'nc_minor' as const, + description: 'Three of ten access reviews had no approval evidence.', + }, + }; + + beforeEach(() => { + (mockDb.ismsAudit.findFirst as jest.Mock).mockResolvedValue({ + id: 'aud_1', + documentId: 'doc_1', + }); + (mockDb.ismsAuditFinding.findFirst as jest.Mock).mockResolvedValue(null); + (mockDb.ismsAuditFinding.create as jest.Mock).mockResolvedValue({}); + }); + + it('creates a standalone finding with a generated F-01 reference and open status', async () => { + await service.create(args); + + expect(mockDb.ismsAuditFinding.create).toHaveBeenCalledWith({ + data: expect.objectContaining({ + auditId: 'aud_1', + documentId: 'doc_1', + reference: 'F-01', + type: 'nc_minor', + controlId: null, + status: 'open', + position: 0, + }), + }); + }); + + it('continues the per-audit sequence past deleted references', async () => { + (mockDb.ismsAuditFinding.findMany as jest.Mock).mockResolvedValue([ + { reference: 'F-01' }, + { reference: 'F-03' }, + ]); + + await service.create(args); + + expect(mockDb.ismsAuditFinding.create).toHaveBeenCalledWith({ + data: expect.objectContaining({ reference: 'F-04' }), + }); + }); + + it('rejects a related control from another audit', async () => { + (mockDb.ismsAuditControl.findFirst as jest.Mock).mockResolvedValue(null); + + await expect( + service.create({ + ...args, + dto: { ...args.dto, controlId: 'ac_other' }, + }), + ).rejects.toThrow(NotFoundException); + expect(mockDb.ismsAuditControl.findFirst).toHaveBeenCalledWith({ + where: { id: 'ac_other', auditId: 'aud_1' }, + select: { id: true, controlRef: true }, + }); + expect(mockDb.ismsAuditFinding.create).not.toHaveBeenCalled(); + }); + + it('inherits the clause label from the linked control when none is given', async () => { + (mockDb.ismsAuditControl.findFirst as jest.Mock).mockResolvedValue({ + id: 'ac_1', + controlRef: 'Clause 9.1 Monitoring', + }); + + await service.create({ + ...args, + dto: { ...args.dto, controlId: 'ac_1' }, + }); + + expect(mockDb.ismsAuditFinding.create).toHaveBeenCalledWith({ + data: expect.objectContaining({ + controlId: 'ac_1', + clauseOrControl: 'Clause 9.1 Monitoring', + }), + }); + }); + + it('keeps an explicitly provided clause label over the linked control', async () => { + (mockDb.ismsAuditControl.findFirst as jest.Mock).mockResolvedValue({ + id: 'ac_1', + controlRef: 'Clause 9.1 Monitoring', + }); + + await service.create({ + ...args, + dto: { + ...args.dto, + controlId: 'ac_1', + clauseOrControl: 'Clause 9.1(b)', + }, + }); + + expect(mockDb.ismsAuditFinding.create).toHaveBeenCalledWith({ + data: expect.objectContaining({ clauseOrControl: 'Clause 9.1(b)' }), + }); + }); + + it('rejects an owner that is not an active org member', async () => { + (mockDb.member.findFirst as jest.Mock).mockResolvedValue(null); + + await expect( + service.create({ + ...args, + dto: { ...args.dto, ownerMemberId: 'mem_x' }, + }), + ).rejects.toThrow(NotFoundException); + expect(mockDb.member.findFirst).toHaveBeenCalledWith({ + where: { id: 'mem_x', organizationId: 'org_1', deactivated: false }, + }); + }); + }); + + describe('update', () => { + beforeEach(() => { + (mockDb.ismsAuditFinding.findFirst as jest.Mock).mockResolvedValue({ + id: 'af_1', + auditId: 'aud_1', + documentId: 'doc_1', + }); + (mockDb.ismsAuditFinding.update as jest.Mock).mockResolvedValue({}); + }); + + it('closes a finding with closure evidence and a cleared due date', async () => { + await service.update({ + findingId: 'af_1', + organizationId: 'org_1', + dto: { + status: 'closed', + closureEvidence: 'Restore test evidenced in task ev_123.', + dueDate: null, + }, + }); + + const { data } = (mockDb.ismsAuditFinding.update as jest.Mock).mock + .calls[0][0]; + expect(data.status).toBe('closed'); + expect(data.closureEvidence).toBe('Restore test evidenced in task ev_123.'); + expect(data.dueDate).toBeNull(); + // The server-generated reference is never updatable. + expect(data.reference).toBeUndefined(); + }); + + it('inherits the clause label when linking a control to an unlabelled finding', async () => { + (mockDb.ismsAuditFinding.findFirst as jest.Mock).mockResolvedValue({ + id: 'af_1', + auditId: 'aud_1', + documentId: 'doc_1', + clauseOrControl: null, + }); + (mockDb.ismsAuditControl.findFirst as jest.Mock).mockResolvedValue({ + id: 'ac_1', + controlRef: 'A.5.15 Access control', + }); + + await service.update({ + findingId: 'af_1', + organizationId: 'org_1', + dto: { controlId: 'ac_1' }, + }); + + const { data } = (mockDb.ismsAuditFinding.update as jest.Mock).mock + .calls[0][0]; + expect(data.controlId).toBe('ac_1'); + expect(data.clauseOrControl).toBe('A.5.15 Access control'); + }); + + it('keeps an existing clause label when relinking (labels are user-owned)', async () => { + (mockDb.ismsAuditFinding.findFirst as jest.Mock).mockResolvedValue({ + id: 'af_1', + auditId: 'aud_1', + documentId: 'doc_1', + clauseOrControl: 'Clause 9.1 (Monitoring)', + }); + (mockDb.ismsAuditControl.findFirst as jest.Mock).mockResolvedValue({ + id: 'ac_2', + controlRef: 'A.8.13 Backup', + }); + + await service.update({ + findingId: 'af_1', + organizationId: 'org_1', + dto: { controlId: 'ac_2' }, + }); + + const { data } = (mockDb.ismsAuditFinding.update as jest.Mock).mock + .calls[0][0]; + expect(data.controlId).toBe('ac_2'); + expect(data.clauseOrControl).toBe('Clause 9.1 (Monitoring)'); + }); + + it('unlinks the related control with null', async () => { + await service.update({ + findingId: 'af_1', + organizationId: 'org_1', + dto: { controlId: null }, + }); + + const { data } = (mockDb.ismsAuditFinding.update as jest.Mock).mock + .calls[0][0]; + expect(data.controlId).toBeNull(); + expect(mockDb.ismsAuditControl.findFirst).not.toHaveBeenCalled(); + }); + }); + + describe('remove', () => { + it('deletes the finding', async () => { + (mockDb.ismsAuditFinding.findFirst as jest.Mock).mockResolvedValue({ + id: 'af_1', + documentId: 'doc_1', + }); + (mockDb.ismsAuditFinding.delete as jest.Mock).mockResolvedValue({}); + + const result = await service.remove({ + findingId: 'af_1', + organizationId: 'org_1', + }); + + expect(result).toEqual({ success: true }); + expect(mockDb.ismsAuditFinding.delete).toHaveBeenCalledWith({ + where: { id: 'af_1' }, + }); + }); + }); +}); diff --git a/apps/api/src/isms/isms-audit-finding.service.ts b/apps/api/src/isms/isms-audit-finding.service.ts new file mode 100644 index 0000000000..d6ebf7bff7 --- /dev/null +++ b/apps/api/src/isms/isms-audit-finding.service.ts @@ -0,0 +1,269 @@ +import { Injectable, NotFoundException } from '@nestjs/common'; +import { db } from '@db'; +import type { Prisma } from '@db'; +import { invalidateApprovalIfNeeded } from './utils/approval'; +import { lockDocument } from './utils/document-lock'; +import { parseOptionalDate } from './utils/parse-optional-date'; +import type { + CreateAuditFindingInput, + UpdateAuditFindingInput, +} from './registers/register-registry'; + +/** + * CRUD for an audit's findings (clause 9.2, CS-724). Most findings are raised + * from a Controls Tested row (non-conformity / observation) and link back to + * it; standalone findings are also allowed. The reference is server-generated + * ("F-NN", per-audit sequence) and immutable. + */ +@Injectable() +export class IsmsAuditFindingService { + async create({ + documentId, + organizationId, + dto, + }: { + documentId: string; + organizationId: string; + dto: CreateAuditFindingInput; + }) { + const audit = await this.requireAudit({ + auditId: dto.auditId, + documentId, + organizationId, + }); + const control = await this.resolveControl({ + controlId: dto.controlId, + auditId: audit.id, + }); + const ownerMemberId = await this.resolveMember({ + memberId: dto.ownerMemberId, + organizationId, + }); + const dueDate = parseOptionalDate(dto.dueDate); + + return db.$transaction(async (tx) => { + // The lock also serializes reference generation, so concurrent creates + // can never both compute the same next "F-NN". + await lockDocument(tx, audit.documentId); + const position = + dto.position ?? (await this.nextPosition({ tx, auditId: audit.id })); + await invalidateApprovalIfNeeded({ tx, documentId: audit.documentId }); + return tx.ismsAuditFinding.create({ + data: { + auditId: audit.id, + documentId: audit.documentId, + reference: await this.nextReference({ tx, auditId: audit.id }), + type: dto.type, + controlId: control?.id ?? null, + // A linked finding without its own clause text inherits the row's + // reference, so unlinking (or deleting the row) never leaves the + // finding unlabelled in the generated document. + clauseOrControl: + dto.clauseOrControl?.trim() || control?.controlRef || null, + description: dto.description, + ownerMemberId: ownerMemberId ?? null, + dueDate: dueDate ?? null, + status: dto.status ?? 'open', + closureEvidence: dto.closureEvidence?.trim() || null, + position, + }, + }); + }); + } + + async update({ + findingId, + organizationId, + dto, + }: { + findingId: string; + organizationId: string; + dto: UpdateAuditFindingInput; + }) { + const finding = await this.requireFinding({ findingId, organizationId }); + const control = await this.resolveControl({ + controlId: dto.controlId, + auditId: finding.auditId, + }); + // Same inheritance on link change: when this update links a control and + // the finding's effective clause text is empty, derive it from the row. + const requestedClause = + dto.clauseOrControl === undefined + ? finding.clauseOrControl + : dto.clauseOrControl?.trim() || null; + const derivedClause = requestedClause ?? control?.controlRef ?? null; + const ownerMemberId = await this.resolveMember({ + memberId: dto.ownerMemberId, + organizationId, + }); + + return db.$transaction(async (tx) => { + // Same per-document lock submit/approve take, so an edit can't race the + // submission's completeness check. + await lockDocument(tx, finding.documentId); + await invalidateApprovalIfNeeded({ tx, documentId: finding.documentId }); + return tx.ismsAuditFinding.update({ + where: { id: findingId }, + data: { + type: dto.type ?? undefined, + controlId: dto.controlId === undefined ? undefined : (control?.id ?? null), + clauseOrControl: + dto.clauseOrControl === undefined && control === undefined + ? undefined + : derivedClause, + description: dto.description ?? undefined, + ownerMemberId: + dto.ownerMemberId === undefined ? undefined : ownerMemberId, + dueDate: parseOptionalDate(dto.dueDate), + status: dto.status ?? undefined, + closureEvidence: + dto.closureEvidence === undefined + ? undefined + : dto.closureEvidence?.trim() || null, + position: dto.position ?? undefined, + }, + }); + }); + } + + async remove({ + findingId, + organizationId, + }: { + findingId: string; + organizationId: string; + }) { + const finding = await this.requireFinding({ findingId, organizationId }); + await db.$transaction(async (tx) => { + await lockDocument(tx, finding.documentId); + await invalidateApprovalIfNeeded({ tx, documentId: finding.documentId }); + await tx.ismsAuditFinding.delete({ where: { id: findingId } }); + }); + return { success: true }; + } + + /** + * Next "F-NN" for the audit: max + 1 over the surviving rows, so deleting an + * older finding never reissues its number (deleting the newest frees it — + * acceptable: findings are draft rows until published, and every published + * version freezes its own findings in the version contentSnapshot). + */ + private async nextReference({ + tx, + auditId, + }: { + tx: Prisma.TransactionClient; + auditId: string; + }): Promise { + const existing = await tx.ismsAuditFinding.findMany({ + where: { auditId }, + select: { reference: true }, + }); + const maxSequence = existing.reduce((max, finding) => { + const match = /^F-(\d+)$/.exec(finding.reference); + return match ? Math.max(max, Number.parseInt(match[1], 10)) : max; + }, 0); + return `F-${String(maxSequence + 1).padStart(2, '0')}`; + } + + /** + * The linked Controls Tested row must belong to the SAME audit. Returns the + * row (id + reference) so callers can inherit the clause label; undefined = + * link untouched, null = explicitly unlinked. + */ + private async resolveControl({ + controlId, + auditId, + }: { + controlId: string | null | undefined; + auditId: string; + }): Promise<{ id: string; controlRef: string } | null | undefined> { + if (controlId === undefined) return undefined; + const trimmed = controlId?.trim(); + if (!trimmed) return null; + const control = await db.ismsAuditControl.findFirst({ + where: { id: trimmed, auditId }, + select: { id: true, controlRef: true }, + }); + if (!control) { + throw new NotFoundException('Audit control row not found in this audit'); + } + return control; + } + + private async resolveMember({ + memberId, + organizationId, + }: { + memberId: string | null | undefined; + organizationId: string; + }): Promise { + if (memberId === undefined) return undefined; + const trimmed = memberId?.trim(); + if (!trimmed) return null; + // Finding owners must be active People members. + const member = await db.member.findFirst({ + where: { id: trimmed, organizationId, deactivated: false }, + }); + if (!member) { + throw new NotFoundException('Active member not found in organization'); + } + return trimmed; + } + + private async nextPosition({ + tx, + auditId, + }: { + tx: Prisma.TransactionClient; + auditId: string; + }) { + const last = await tx.ismsAuditFinding.findFirst({ + where: { auditId }, + orderBy: { position: 'desc' }, + select: { position: true }, + }); + return (last?.position ?? -1) + 1; + } + + private async requireAudit({ + auditId, + documentId, + organizationId, + }: { + auditId: string; + documentId: string; + organizationId: string; + }) { + const audit = await db.ismsAudit.findFirst({ + where: { + id: auditId, + documentId, + document: { organizationId, type: 'internal_audit' }, + }, + }); + if (!audit) { + throw new NotFoundException('Audit not found'); + } + return audit; + } + + private async requireFinding({ + findingId, + organizationId, + }: { + findingId: string; + organizationId: string; + }) { + const finding = await db.ismsAuditFinding.findFirst({ + where: { + id: findingId, + audit: { document: { organizationId, type: 'internal_audit' } }, + }, + }); + if (!finding) { + throw new NotFoundException('Finding not found'); + } + return finding; + } +} diff --git a/apps/api/src/isms/isms-audit.service.spec.ts b/apps/api/src/isms/isms-audit.service.spec.ts new file mode 100644 index 0000000000..cc940bad58 --- /dev/null +++ b/apps/api/src/isms/isms-audit.service.spec.ts @@ -0,0 +1,265 @@ +import { BadRequestException, NotFoundException } from '@nestjs/common'; +import { db } from '@db'; +import { IsmsAuditService } from './isms-audit.service'; +import { + DEFAULT_AUDIT_CRITERIA, + DEFAULT_AUDIT_SCOPE, +} from './documents/internal-audit-defaults'; + +jest.mock('@db', () => { + const db = { + ismsDocument: { + findFirst: jest.fn(), + findUnique: jest.fn(), + update: jest.fn(), + }, + ismsAudit: { + findFirst: jest.fn(), + findUniqueOrThrow: jest.fn(), + findMany: jest.fn(), + create: jest.fn(), + update: jest.fn(), + delete: jest.fn(), + }, + ismsAuditControl: { + findMany: jest.fn(), + createMany: jest.fn(), + }, + $executeRaw: jest.fn(), + $transaction: jest.fn((cb: (tx: unknown) => unknown) => cb(db)), + }; + return { db }; +}); + +const mockDb = jest.mocked(db); +const currentYear = new Date().getUTCFullYear(); + +describe('IsmsAuditService', () => { + let service: IsmsAuditService; + + beforeEach(() => { + jest.clearAllMocks(); + (mockDb.ismsDocument.findUnique as jest.Mock).mockResolvedValue({ + status: 'draft', + }); + (mockDb.ismsAudit.findMany as jest.Mock).mockResolvedValue([]); + (mockDb.ismsAuditControl.findMany as jest.Mock).mockResolvedValue([]); + (mockDb.ismsAuditControl.createMany as jest.Mock).mockResolvedValue({ + count: 15, + }); + service = new IsmsAuditService(); + }); + + describe('create', () => { + const args = { documentId: 'doc_1', organizationId: 'org_1', dto: {} }; + + it('throws NotFoundException when the internal-audit document is missing', async () => { + (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue(null); + await expect(service.create(args)).rejects.toThrow(NotFoundException); + expect(mockDb.ismsDocument.findFirst).toHaveBeenCalledWith({ + where: { id: 'doc_1', organizationId: 'org_1', type: 'internal_audit' }, + }); + }); + + it('creates an audit with template defaults and a generated reference', async () => { + (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ + id: 'doc_1', + }); + (mockDb.ismsAudit.findFirst as jest.Mock).mockResolvedValue(null); + (mockDb.ismsAudit.create as jest.Mock).mockResolvedValue({ + id: 'aud_1', + }); + + await service.create(args); + + expect(mockDb.ismsAudit.create).toHaveBeenCalledWith({ + data: expect.objectContaining({ + documentId: 'doc_1', + reference: `IA-${currentYear}-01`, + scope: DEFAULT_AUDIT_SCOPE, + criteria: DEFAULT_AUDIT_CRITERIA, + auditorName: null, + position: 0, + }), + }); + }); + + it('seeds the fifteen default Controls Tested rows in the same transaction', async () => { + (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ + id: 'doc_1', + }); + (mockDb.ismsAudit.findFirst as jest.Mock).mockResolvedValue(null); + (mockDb.ismsAudit.create as jest.Mock).mockResolvedValue({ + id: 'aud_1', + }); + + await service.create(args); + + const { data } = (mockDb.ismsAuditControl.createMany as jest.Mock).mock + .calls[0][0]; + expect(data).toHaveLength(15); + expect(data[0]).toMatchObject({ auditId: 'aud_1', documentId: 'doc_1' }); + }); + + it('continues the per-year sequence from the highest existing reference', async () => { + (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ + id: 'doc_1', + }); + (mockDb.ismsAudit.findMany as jest.Mock).mockResolvedValue([ + { reference: `IA-${currentYear}-01` }, + { reference: `IA-${currentYear}-04` }, + ]); + (mockDb.ismsAudit.findFirst as jest.Mock).mockResolvedValue({ + position: 3, + }); + (mockDb.ismsAudit.create as jest.Mock).mockResolvedValue({ + id: 'aud_2', + }); + + await service.create(args); + + expect(mockDb.ismsAudit.create).toHaveBeenCalledWith({ + data: expect.objectContaining({ + reference: `IA-${currentYear}-05`, + position: 4, + }), + }); + }); + + it('rejects an invalid planned date before opening the transaction', async () => { + (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ + id: 'doc_1', + }); + await expect( + service.create({ + ...args, + dto: { plannedStartDate: '2026-02-30' }, + }), + ).rejects.toThrow(BadRequestException); + expect(mockDb.ismsAudit.create).not.toHaveBeenCalled(); + }); + + it('rejects a planned end date before the start date', async () => { + (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ + id: 'doc_1', + }); + await expect( + service.create({ + ...args, + dto: { plannedStartDate: '2026-05-20', plannedEndDate: '2026-05-15' }, + }), + ).rejects.toThrow(BadRequestException); + expect(mockDb.ismsAudit.create).not.toHaveBeenCalled(); + }); + }); + + describe('update', () => { + beforeEach(() => { + (mockDb.ismsAudit.findFirst as jest.Mock).mockResolvedValue({ + id: 'aud_1', + documentId: 'doc_1', + plannedStartDate: null, + plannedEndDate: null, + }); + (mockDb.ismsAudit.update as jest.Mock).mockResolvedValue({}); + }); + + it('rejects an end date before the STORED start date (re-read under the document lock)', async () => { + (mockDb.ismsAudit.findUniqueOrThrow as jest.Mock).mockResolvedValue({ + plannedStartDate: new Date('2026-05-15T00:00:00.000Z'), + plannedEndDate: null, + }); + + await expect( + service.update({ + auditId: 'aud_1', + organizationId: 'org_1', + dto: { plannedEndDate: '2026-05-10' }, + }), + ).rejects.toThrow(BadRequestException); + expect(mockDb.ismsAudit.update).not.toHaveBeenCalled(); + }); + + it('allows fixing an inverted schedule by moving both dates together', async () => { + (mockDb.ismsAudit.findUniqueOrThrow as jest.Mock).mockResolvedValue({ + plannedStartDate: new Date('2026-06-10T00:00:00.000Z'), + plannedEndDate: new Date('2026-05-01T00:00:00.000Z'), + }); + + await service.update({ + auditId: 'aud_1', + organizationId: 'org_1', + dto: { plannedStartDate: '2026-06-01', plannedEndDate: '2026-06-05' }, + }); + expect(mockDb.ismsAudit.update).toHaveBeenCalled(); + }); + + it('updates status and conclusion, leaving omitted fields untouched', async () => { + await service.update({ + auditId: 'aud_1', + organizationId: 'org_1', + dto: { status: 'complete', conclusionVerdict: 'conform' }, + }); + + const { data } = (mockDb.ismsAudit.update as jest.Mock).mock.calls[0][0]; + expect(data.status).toBe('complete'); + expect(data.conclusionVerdict).toBe('conform'); + expect(data.scope).toBeUndefined(); + expect(data.signoffAuditorName).toBeUndefined(); + }); + + it('saves sign-off slots and clears them with null/empty', async () => { + await service.update({ + auditId: 'aud_1', + organizationId: 'org_1', + dto: { + signoffAuditorName: 'Sarah Chen', + signoffAuditorDate: '2026-05-20', + signoffSpoName: null, + signoffSpoDate: '', + }, + }); + + const { data } = (mockDb.ismsAudit.update as jest.Mock).mock.calls[0][0]; + expect(data.signoffAuditorName).toBe('Sarah Chen'); + expect(data.signoffAuditorDate).toEqual( + new Date('2026-05-20T00:00:00.000Z'), + ); + expect(data.signoffSpoName).toBeNull(); + expect(data.signoffSpoDate).toBeNull(); + }); + + it('throws NotFoundException for an audit outside the organization', async () => { + (mockDb.ismsAudit.findFirst as jest.Mock).mockResolvedValue(null); + await expect( + service.update({ auditId: 'aud_x', organizationId: 'org_1', dto: {} }), + ).rejects.toThrow(NotFoundException); + expect(mockDb.ismsAudit.findFirst).toHaveBeenCalledWith({ + where: { + id: 'aud_x', + document: { organizationId: 'org_1', type: 'internal_audit' }, + }, + }); + }); + }); + + describe('remove', () => { + it('deletes the audit (cascading to controls and findings)', async () => { + (mockDb.ismsAudit.findFirst as jest.Mock).mockResolvedValue({ + id: 'aud_1', + documentId: 'doc_1', + }); + (mockDb.ismsAudit.delete as jest.Mock).mockResolvedValue({}); + + const result = await service.remove({ + auditId: 'aud_1', + organizationId: 'org_1', + }); + + expect(result).toEqual({ success: true }); + expect(mockDb.ismsAudit.delete).toHaveBeenCalledWith({ + where: { id: 'aud_1' }, + }); + }); + }); +}); diff --git a/apps/api/src/isms/isms-audit.service.ts b/apps/api/src/isms/isms-audit.service.ts new file mode 100644 index 0000000000..31b474e4ce --- /dev/null +++ b/apps/api/src/isms/isms-audit.service.ts @@ -0,0 +1,266 @@ +import { + BadRequestException, + Injectable, + NotFoundException, +} from '@nestjs/common'; +import { db } from '@db'; +import type { Prisma } from '@db'; +import { seedAuditControlsIfMissing } from './documents/internal-audit'; +import { + DEFAULT_AUDIT_CRITERIA, + DEFAULT_AUDIT_SCOPE, +} from './documents/internal-audit-defaults'; +import { invalidateApprovalIfNeeded } from './utils/approval'; +import { lockDocument } from './utils/document-lock'; +import { parseOptionalDate } from './utils/parse-optional-date'; +import type { + CreateAuditInput, + UpdateAuditInput, +} from './registers/register-registry'; + +/** + * CRUD for the Internal Audit instances register (clause 9.2, CS-724). Every + * audit is customer-created; scope and criteria pre-fill with the template + * defaults, the reference is server-generated ("IA-YYYY-NN"), and the fifteen + * default Controls Tested rows are seeded in the same transaction. Deleting an + * audit cascades to its control rows and findings. + */ +@Injectable() +export class IsmsAuditService { + async create({ + documentId, + organizationId, + dto, + }: { + documentId: string; + organizationId: string; + dto: CreateAuditInput; + }) { + await this.requireDocument({ documentId, organizationId }); + const plannedStartDate = parseOptionalDate(dto.plannedStartDate); + const plannedEndDate = parseOptionalDate(dto.plannedEndDate); + this.assertDateOrder({ + start: plannedStartDate ?? null, + end: plannedEndDate ?? null, + }); + + return db.$transaction(async (tx) => { + // The lock also serializes reference generation, so concurrent creates + // can never both compute the same next "IA-YYYY-NN". + await lockDocument(tx, documentId); + const position = + dto.position ?? (await this.nextPosition({ tx, documentId })); + await invalidateApprovalIfNeeded({ tx, documentId }); + const audit = await tx.ismsAudit.create({ + data: { + documentId, + reference: await this.nextReference({ tx, documentId }), + scope: dto.scope?.trim() || DEFAULT_AUDIT_SCOPE, + criteria: dto.criteria?.trim() || DEFAULT_AUDIT_CRITERIA, + auditorName: dto.auditorName?.trim() || null, + plannedStartDate: plannedStartDate ?? null, + plannedEndDate: plannedEndDate ?? null, + position, + }, + }); + // The default sample set the customer sees when they first open the audit. + await seedAuditControlsIfMissing({ tx, auditId: audit.id, documentId }); + return audit; + }); + } + + async update({ + auditId, + organizationId, + dto, + }: { + auditId: string; + organizationId: string; + dto: UpdateAuditInput; + }) { + const audit = await this.requireAudit({ auditId, organizationId }); + const plannedStartDate = parseOptionalDate(dto.plannedStartDate); + const plannedEndDate = parseOptionalDate(dto.plannedEndDate); + + return db.$transaction(async (tx) => { + // Same per-document lock submit/approve take, so an edit can't race the + // submission's completeness check. + await lockDocument(tx, audit.documentId); + // Validate the EFFECTIVE schedule (dto values merged over the stored + // row) UNDER the lock, re-reading the stored dates: two concurrent + // partial updates — one moving the start, one the end — serialize on + // the document lock, so the second sees the first's committed value and + // an inverted range can never be persisted. + if (plannedStartDate !== undefined || plannedEndDate !== undefined) { + const current = await tx.ismsAudit.findUniqueOrThrow({ + where: { id: auditId }, + select: { plannedStartDate: true, plannedEndDate: true }, + }); + this.assertDateOrder({ + start: + plannedStartDate === undefined + ? current.plannedStartDate + : plannedStartDate, + end: + plannedEndDate === undefined + ? current.plannedEndDate + : plannedEndDate, + }); + } + await invalidateApprovalIfNeeded({ tx, documentId: audit.documentId }); + return tx.ismsAudit.update({ + where: { id: auditId }, + data: { + scope: dto.scope ?? undefined, + criteria: dto.criteria ?? undefined, + auditorName: + dto.auditorName === undefined + ? undefined + : dto.auditorName?.trim() || null, + plannedStartDate, + plannedEndDate, + status: dto.status ?? undefined, + conclusionVerdict: + dto.conclusionVerdict === undefined + ? undefined + : dto.conclusionVerdict, + conclusionNotes: + dto.conclusionNotes === undefined + ? undefined + : dto.conclusionNotes?.trim() || null, + signoffAuditorName: this.optionalText(dto.signoffAuditorName), + signoffAuditorDate: parseOptionalDate(dto.signoffAuditorDate), + signoffSpoName: this.optionalText(dto.signoffSpoName), + signoffSpoDate: parseOptionalDate(dto.signoffSpoDate), + signoffTopMgmtName: this.optionalText(dto.signoffTopMgmtName), + signoffTopMgmtDate: parseOptionalDate(dto.signoffTopMgmtDate), + position: dto.position ?? undefined, + }, + }); + }); + } + + async remove({ + auditId, + organizationId, + }: { + auditId: string; + organizationId: string; + }) { + const audit = await this.requireAudit({ auditId, organizationId }); + await db.$transaction(async (tx) => { + await lockDocument(tx, audit.documentId); + await invalidateApprovalIfNeeded({ tx, documentId: audit.documentId }); + // Cascades to the audit's control rows and findings. + await tx.ismsAudit.delete({ where: { id: auditId } }); + }); + return { success: true }; + } + + /** Three-state text field: undefined = leave, null/empty = clear. */ + private optionalText( + value: string | null | undefined, + ): string | null | undefined { + if (value === undefined) return undefined; + return value?.trim() || null; + } + + /** A planned schedule must not end before it starts (client gate mirrored). */ + private assertDateOrder({ + start, + end, + }: { + start: Date | null; + end: Date | null; + }): void { + if (start && end && end.getTime() < start.getTime()) { + throw new BadRequestException( + 'Planned end date must be on or after the planned start date', + ); + } + } + + /** + * Next "IA-YYYY-NN" for the document: the year of creation plus a two-digit + * per-document, per-year sequence. Max + 1 over the surviving rows, so + * deleting an older audit never reissues its number (deleting the newest + * audit frees its number — acceptable, since audits are freely deletable + * drafts until published). Runs under the per-document lock. + */ + private async nextReference({ + tx, + documentId, + }: { + tx: Prisma.TransactionClient; + documentId: string; + }): Promise { + const year = new Date().getUTCFullYear(); + const prefix = `IA-${year}-`; + const existing = await tx.ismsAudit.findMany({ + where: { documentId, reference: { startsWith: prefix } }, + select: { reference: true }, + }); + const maxSequence = existing.reduce((max, audit) => { + const sequence = Number.parseInt( + audit.reference.slice(prefix.length), + 10, + ); + return Number.isNaN(sequence) ? max : Math.max(max, sequence); + }, 0); + return `${prefix}${String(maxSequence + 1).padStart(2, '0')}`; + } + + private async nextPosition({ + tx, + documentId, + }: { + tx: Prisma.TransactionClient; + documentId: string; + }) { + const last = await tx.ismsAudit.findFirst({ + where: { documentId }, + orderBy: { position: 'desc' }, + select: { position: true }, + }); + return (last?.position ?? -1) + 1; + } + + private async requireDocument({ + documentId, + organizationId, + }: { + documentId: string; + organizationId: string; + }) { + // Scope to the Internal Audit document type: audit rows must never attach + // to another ISMS document (they'd be invisible to the Clause 9.2 export). + const document = await db.ismsDocument.findFirst({ + where: { id: documentId, organizationId, type: 'internal_audit' }, + }); + if (!document) { + throw new NotFoundException('ISMS internal audit document not found'); + } + return document; + } + + private async requireAudit({ + auditId, + organizationId, + }: { + auditId: string; + organizationId: string; + }) { + // Type-scoped like create: an audit row can only ever live on THIS org's + // Internal Audit document (belt-and-braces with the creation guard). + const audit = await db.ismsAudit.findFirst({ + where: { + id: auditId, + document: { organizationId, type: 'internal_audit' }, + }, + }); + if (!audit) { + throw new NotFoundException('Audit not found'); + } + return audit; + } +} diff --git a/apps/api/src/isms/isms-registers.controller.spec.ts b/apps/api/src/isms/isms-registers.controller.spec.ts index eed5933b68..588f5e0a2e 100644 --- a/apps/api/src/isms/isms-registers.controller.spec.ts +++ b/apps/api/src/isms/isms-registers.controller.spec.ts @@ -5,6 +5,8 @@ import type { Request } from 'express'; import { HybridAuthGuard } from '../auth/hybrid-auth.guard'; import { PermissionGuard } from '../auth/permission.guard'; import { PERMISSIONS_KEY } from '../auth/permission.guard'; +import { ActingUserResolver } from '../auth/acting-user.service'; +import type { AuthenticatedRequest } from '../auth/types'; import { IsmsRegistersController } from './isms-registers.controller'; import { IsmsContextIssueService } from './isms-context-issue.service'; import { IsmsInterestedPartyService } from './isms-interested-party.service'; @@ -14,11 +16,17 @@ import { IsmsRoleService } from './isms-role.service'; import { IsmsRoleAssignmentService } from './isms-role-assignment.service'; import { IsmsMetricService } from './isms-metric.service'; import { IsmsMeasurementService } from './isms-measurement.service'; +import { IsmsAuditService } from './isms-audit.service'; +import { IsmsAuditControlService } from './isms-audit-control.service'; +import { IsmsAuditFindingService } from './isms-audit-finding.service'; import { IsmsNarrativeService } from './isms-narrative.service'; jest.mock('../auth/auth.server', () => ({ auth: { api: { getSession: jest.fn() } }, })); +// createRow pulls in ActingUserResolver, which imports @db; mock it so the real +// Prisma client isn't constructed at load (the resolver is a mocked provider). +jest.mock('@db', () => ({ db: {} })); jest.mock('../auth/hybrid-auth.guard', () => ({ HybridAuthGuard: class MockHybridAuthGuard {}, })); @@ -54,12 +62,21 @@ jest.mock('./isms-metric.service', () => ({ jest.mock('./isms-measurement.service', () => ({ IsmsMeasurementService: class {}, })); +jest.mock('./isms-audit.service', () => ({ + IsmsAuditService: class {}, +})); +jest.mock('./isms-audit-control.service', () => ({ + IsmsAuditControlService: class {}, +})); +jest.mock('./isms-audit-finding.service', () => ({ + IsmsAuditFindingService: class {}, +})); jest.mock('./isms-narrative.service', () => ({ IsmsNarrativeService: class {}, })); const reqWith = (body: Record) => - ({ body }) as unknown as Request; + ({ body }) as unknown as Request & AuthenticatedRequest; describe('IsmsRegistersController', () => { let controller: IsmsRegistersController; @@ -105,7 +122,23 @@ describe('IsmsRegistersController', () => { remove: jest.fn(), bulkCreate: jest.fn(), }; + const auditService = { + create: jest.fn(), + update: jest.fn(), + remove: jest.fn(), + }; + const auditControlService = { + create: jest.fn(), + update: jest.fn(), + remove: jest.fn(), + }; + const auditFindingService = { + create: jest.fn(), + update: jest.fn(), + remove: jest.fn(), + }; const narrativeService = { save: jest.fn() }; + const mockActingUser = { resolve: jest.fn() }; const mockGuard = { canActivate: jest.fn().mockReturnValue(true) }; @@ -124,7 +157,11 @@ describe('IsmsRegistersController', () => { { provide: IsmsRoleAssignmentService, useValue: roleAssignmentService }, { provide: IsmsMetricService, useValue: metricService }, { provide: IsmsMeasurementService, useValue: measurementService }, + { provide: IsmsAuditService, useValue: auditService }, + { provide: IsmsAuditControlService, useValue: auditControlService }, + { provide: IsmsAuditFindingService, useValue: auditFindingService }, { provide: IsmsNarrativeService, useValue: narrativeService }, + { provide: ActingUserResolver, useValue: mockActingUser }, ], }) .overrideGuard(HybridAuthGuard) @@ -135,6 +172,12 @@ describe('IsmsRegistersController', () => { controller = module.get(IsmsRegistersController); jest.clearAllMocks(); + // Default: no attributable actor resolved (e.g. org with no owner). Tests + // that exercise API-key attribution override this with mockResolvedValueOnce. + mockActingUser.resolve.mockResolvedValue({ + userId: null, + source: 'org-owner-fallback', + }); }); describe('createRow', () => { @@ -244,7 +287,9 @@ describe('IsmsRegistersController', () => { }); }); - it('passes memberId null under API-key auth (no session member)', async () => { + it('passes memberId null when neither a session nor a resolved member exists', async () => { + // No session member and the resolver found no actor (default mock: + // org-owner-fallback with no member). enteredById is nullable. const body = { metricId: 'met_1', periodStart: '2026-07-01', value: '5' }; await controller.createRow( 'doc_1', @@ -258,6 +303,111 @@ describe('IsmsRegistersController', () => { ); }); + it('attributes enteredBy to the resolved member under API-key auth (no session member)', async () => { + // The bug: API-key callers have no req.memberId, so enteredById was + // silently null. The resolver now supplies the acting member (key creator + // or org-owner fallback), which createRow must forward as enteredById. + mockActingUser.resolve.mockResolvedValueOnce({ + userId: 'usr_owner', + memberId: 'mem_resolved', + source: 'api-key-creator', + }); + const body = { metricId: 'met_1', periodStart: '2026-07-01', value: '5' }; + await controller.createRow( + 'doc_1', + 'measurements', + reqWith(body), + 'org_1', + undefined, + ); + expect(measurementService.create).toHaveBeenCalledWith( + expect.objectContaining({ memberId: 'mem_resolved' }), + ); + }); + + it('prefers the session member over the resolved actor for attribution', async () => { + // Session behavior must be unchanged: the raw session memberId wins even + // when the resolver would return a different member. + mockActingUser.resolve.mockResolvedValueOnce({ + userId: 'usr_owner', + memberId: 'mem_resolved', + source: 'org-owner-fallback', + }); + const body = { metricId: 'met_1', periodStart: '2026-07-01', value: '5' }; + await controller.createRow( + 'doc_1', + 'measurements', + reqWith(body), + 'org_1', + 'mem_session', + ); + expect(measurementService.create).toHaveBeenCalledWith( + expect.objectContaining({ memberId: 'mem_session' }), + ); + }); + + it('dispatches audits create with an empty body (defaults server-side)', async () => { + await controller.createRow( + 'doc_1', + 'audits', + reqWith({}), + 'org_1', + 'mem_1', + ); + expect(auditService.create).toHaveBeenCalledWith({ + documentId: 'doc_1', + organizationId: 'org_1', + dto: {}, + }); + }); + + it('dispatches audit-controls and audit-findings create with parsed dtos', async () => { + const controlBody = { auditId: 'aud_1', controlRef: 'A.8.16' }; + await controller.createRow( + 'doc_1', + 'audit-controls', + reqWith(controlBody), + 'org_1', + 'mem_1', + ); + expect(auditControlService.create).toHaveBeenCalledWith({ + documentId: 'doc_1', + organizationId: 'org_1', + dto: controlBody, + }); + + const findingBody = { + auditId: 'aud_1', + type: 'observation', + description: 'No restore test evidenced.', + }; + await controller.createRow( + 'doc_1', + 'audit-findings', + reqWith(findingBody), + 'org_1', + 'mem_1', + ); + expect(auditFindingService.create).toHaveBeenCalledWith({ + documentId: 'doc_1', + organizationId: 'org_1', + dto: findingBody, + }); + }); + + it('rejects an audit-findings create without a description', async () => { + await expect( + controller.createRow( + 'doc_1', + 'audit-findings', + reqWith({ auditId: 'aud_1', type: 'ofi' }), + 'org_1', + 'mem_1', + ), + ).rejects.toBeInstanceOf(BadRequestException); + expect(auditFindingService.create).not.toHaveBeenCalled(); + }); + it('throws BadRequestException for an unknown register', async () => { await expect( controller.createRow('doc_1', 'nope', reqWith({}), 'org_1', undefined), @@ -306,6 +456,29 @@ describe('IsmsRegistersController', () => { ).rejects.toBeInstanceOf(BadRequestException); expect(measurementService.bulkCreate).not.toHaveBeenCalled(); }); + + it('attributes to the resolved actor when there is no session member (API key)', async () => { + mockActingUser.resolve.mockResolvedValueOnce({ + userId: 'usr_creator', + memberId: 'mem_creator', + source: 'api-key-creator', + }); + + await controller.bulkCreateMeasurements( + 'doc_1', + reqWith({ + measurements: [ + { metricId: 'met_1', periodStart: '2026-07-01', value: '1' }, + ], + }), + 'org_1', + undefined, + ); + + expect(measurementService.bulkCreate).toHaveBeenCalledWith( + expect.objectContaining({ memberId: 'mem_creator' }), + ); + }); }); describe('updateRow', () => { diff --git a/apps/api/src/isms/isms-registers.controller.ts b/apps/api/src/isms/isms-registers.controller.ts index e0a3a4e470..451dfc3daf 100644 --- a/apps/api/src/isms/isms-registers.controller.ts +++ b/apps/api/src/isms/isms-registers.controller.ts @@ -23,6 +23,8 @@ import { MemberId, OrganizationId } from '@/auth/auth-context.decorator'; import { HybridAuthGuard } from '@/auth/hybrid-auth.guard'; import { PermissionGuard } from '../auth/permission.guard'; import { RequirePermission } from '../auth/require-permission.decorator'; +import { ActingUserResolver } from '../auth/acting-user.service'; +import type { AuthenticatedRequest } from '../auth/types'; import { IsmsContextIssueService } from './isms-context-issue.service'; import { IsmsInterestedPartyService } from './isms-interested-party.service'; import { IsmsRequirementService } from './isms-requirement.service'; @@ -31,6 +33,9 @@ import { IsmsRoleService } from './isms-role.service'; import { IsmsRoleAssignmentService } from './isms-role-assignment.service'; import { IsmsMetricService } from './isms-metric.service'; import { IsmsMeasurementService } from './isms-measurement.service'; +import { IsmsAuditService } from './isms-audit.service'; +import { IsmsAuditControlService } from './isms-audit-control.service'; +import { IsmsAuditFindingService } from './isms-audit-finding.service'; import { IsmsNarrativeService } from './isms-narrative.service'; import { createRegisterRegistry, @@ -68,9 +73,21 @@ const REGISTER_ROW_BODY = { cadence: { type: 'string' }, plan: { type: 'string' }, measurementMethod: { type: 'string' }, + // Per-register status: objectives use the first four values, audits the + // next three, and findings open/in_progress/closed. status: { type: 'string', - enum: ['not_started', 'on_track', 'at_risk', 'met'], + enum: [ + 'not_started', + 'on_track', + 'at_risk', + 'met', + 'planned', + 'in_progress', + 'complete', + 'open', + 'closed', + ], }, // Roles register (5.3) + role assignments (7.2 competence) responsibilities: { type: 'string' }, @@ -113,6 +130,47 @@ const REGISTER_ROW_BODY = { }, value: { type: 'string' }, note: { type: 'string', nullable: true }, + // Internal audit register (9.2): audits + audit-controls + audit-findings + scope: { type: 'string' }, + criteria: { type: 'string' }, + auditorName: { type: 'string', nullable: true }, + plannedStartDate: { type: 'string', nullable: true }, + plannedEndDate: { type: 'string', nullable: true }, + conclusionVerdict: { + type: 'string', + enum: ['conform', 'substantially_conform', 'not_yet_conform'], + nullable: true, + }, + conclusionNotes: { type: 'string', nullable: true }, + signoffAuditorName: { type: 'string', nullable: true }, + signoffAuditorDate: { type: 'string', nullable: true }, + signoffSpoName: { type: 'string', nullable: true }, + signoffSpoDate: { type: 'string', nullable: true }, + signoffTopMgmtName: { type: 'string', nullable: true }, + signoffTopMgmtDate: { type: 'string', nullable: true }, + auditId: { type: 'string' }, + controlRef: { type: 'string' }, + whatWasTested: { type: 'string' }, + whereToFind: { type: 'string' }, + result: { + type: 'string', + enum: [ + 'conformity_confirmed', + 'nonconformity_raised', + 'observation_raised', + 'not_sampled', + ], + nullable: true, + }, + notes: { type: 'string', nullable: true }, + type: { + type: 'string', + enum: ['nc_major', 'nc_minor', 'ofi', 'observation'], + }, + controlId: { type: 'string', nullable: true }, + clauseOrControl: { type: 'string', nullable: true }, + dueDate: { type: 'string', nullable: true }, + closureEvidence: { type: 'string', nullable: true }, position: { type: 'integer', minimum: 0 }, }, }, @@ -181,8 +239,12 @@ export class IsmsRegistersController { roleService: IsmsRoleService, roleAssignmentService: IsmsRoleAssignmentService, metricService: IsmsMetricService, + auditService: IsmsAuditService, + auditControlService: IsmsAuditControlService, + auditFindingService: IsmsAuditFindingService, private readonly measurementService: IsmsMeasurementService, private readonly narrativeService: IsmsNarrativeService, + private readonly actingUser: ActingUserResolver, ) { this.registry = createRegisterRegistry({ contextIssues: contextIssueService, @@ -193,6 +255,9 @@ export class IsmsRegistersController { roleAssignments: roleAssignmentService, metrics: metricService, measurements: this.measurementService, + audits: auditService, + auditControls: auditControlService, + auditFindings: auditFindingService, }); } @@ -215,17 +280,23 @@ export class IsmsRegistersController { @Param('id') id: string, @Param('register') register: string, // Read req.body directly: the global ValidationPipe mangles nested JSON. - @Req() req: Request, + @Req() req: AuthenticatedRequest, @OrganizationId() organizationId: string, // Session-auth member; undefined under API-key auth. Measurements record // it as the immutable enteredById. @MemberId() memberId: string | undefined, ) { + // Resolve the acting member so attribution survives API-key auth, where + // req.memberId is undefined. Prefer the raw session member (unchanged + // session behavior), else the resolved actor (API-key creator or org owner + // fallback), else null — enteredById is nullable. + const acting = await this.actingUser.resolve(req, organizationId); + const enteredByMemberId = memberId ?? acting.memberId ?? null; return this.resolve(register).create({ documentId: id, organizationId, data: req.body, - memberId: memberId ?? null, + memberId: enteredByMemberId, }); } @@ -277,14 +348,19 @@ export class IsmsRegistersController { async bulkCreateMeasurements( @Param('id') id: string, // Read req.body directly: the global ValidationPipe mangles nested JSON. - @Req() req: Request, + @Req() req: AuthenticatedRequest, @OrganizationId() organizationId: string, @MemberId() memberId: string | undefined, ) { + // Same session-first attribution as createRow: prefer the session member, + // else the resolved actor (API-key creator / owner fallback), else null — + // so bulk saves via API key don't persist a null enteredById. + const acting = await this.actingUser.resolve(req, organizationId); + const enteredByMemberId = memberId ?? acting.memberId ?? null; return this.measurementService.bulkCreate({ documentId: id, organizationId, - memberId: memberId ?? null, + memberId: enteredByMemberId, dto: parseMeasurementBulkBody(req.body), }); } diff --git a/apps/api/src/isms/isms-version.service.spec.ts b/apps/api/src/isms/isms-version.service.spec.ts index 2c25de491a..c7c4ba2b45 100644 --- a/apps/api/src/isms/isms-version.service.spec.ts +++ b/apps/api/src/isms/isms-version.service.spec.ts @@ -26,6 +26,7 @@ jest.mock('./utils/export-payload', () => ({ resolveOrgProfile: jest.fn(), resolveRolesExtras: jest.fn(), resolveMonitoringExtras: jest.fn(), + resolveInternalAuditExtras: jest.fn(), parseExportSnapshot: jest.fn(() => null), })); jest.mock('./utils/export-metadata', () => ({ diff --git a/apps/api/src/isms/isms-version.service.ts b/apps/api/src/isms/isms-version.service.ts index 6d71c6a2f1..22e3a9a1b4 100644 --- a/apps/api/src/isms/isms-version.service.ts +++ b/apps/api/src/isms/isms-version.service.ts @@ -15,6 +15,7 @@ import { buildExportInput, parseExportSnapshot, renderSnapshot, + resolveInternalAuditExtras, resolveMonitoringExtras, resolveOrgProfile, resolveRolesExtras, @@ -73,11 +74,13 @@ export class IsmsVersionService { const orgProfile = await resolveOrgProfile(document, tx); const rolesExtras = await resolveRolesExtras(document, tx); const monitoringExtras = await resolveMonitoringExtras(document, tx); + const internalAuditExtras = await resolveInternalAuditExtras(document, tx); const input = buildExportInput({ document, orgProfile, rolesExtras, monitoringExtras, + internalAuditExtras, }); const metadata = buildExportMetadata({ type: document.type, diff --git a/apps/api/src/isms/isms.module.ts b/apps/api/src/isms/isms.module.ts index d7df28867b..b82c50a8fe 100644 --- a/apps/api/src/isms/isms.module.ts +++ b/apps/api/src/isms/isms.module.ts @@ -13,6 +13,9 @@ import { IsmsRoleService } from './isms-role.service'; import { IsmsRoleAssignmentService } from './isms-role-assignment.service'; import { IsmsMetricService } from './isms-metric.service'; import { IsmsMeasurementService } from './isms-measurement.service'; +import { IsmsAuditService } from './isms-audit.service'; +import { IsmsAuditControlService } from './isms-audit-control.service'; +import { IsmsAuditFindingService } from './isms-audit-finding.service'; import { IsmsNarrativeService } from './isms-narrative.service'; import { IsmsProfileController } from './wizard/isms-profile.controller'; import { IsmsProfileService } from './wizard/isms-profile.service'; @@ -40,6 +43,9 @@ import { AttachmentsModule } from '../attachments/attachments.module'; IsmsRoleAssignmentService, IsmsMetricService, IsmsMeasurementService, + IsmsAuditService, + IsmsAuditControlService, + IsmsAuditFindingService, IsmsNarrativeService, IsmsProfileService, ], @@ -56,6 +62,9 @@ import { AttachmentsModule } from '../attachments/attachments.module'; IsmsRoleAssignmentService, IsmsMetricService, IsmsMeasurementService, + IsmsAuditService, + IsmsAuditControlService, + IsmsAuditFindingService, IsmsNarrativeService, IsmsProfileService, ], 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 5ffa16ced4..1c065ec8a5 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 @@ -72,7 +72,7 @@ describe('IsmsService ensureSetup fallback to ISMS_TYPE_DEFINITIONS (no template const result = await service.ensureSetup(dto); expect(mockDb.ismsDocument.createMany).toHaveBeenCalledTimes(1); - expect(createManyData()).toHaveLength(7); + expect(createManyData()).toHaveLength(8); // Definition-derived docs carry no templateId. expect(createManyData()[0].templateId).toBeNull(); expect(result.success).toBe(true); @@ -96,7 +96,7 @@ describe('IsmsService ensureSetup fallback to ISMS_TYPE_DEFINITIONS (no template await service.ensureSetup(dto); - expect(createManyData()).toHaveLength(8); + expect(createManyData()).toHaveLength(9); expect(createManyData()[0].requirementId).toBeNull(); }); }); diff --git a/apps/api/src/isms/isms.service.lifecycle.spec.ts b/apps/api/src/isms/isms.service.lifecycle.spec.ts index 5184ab0772..846a3f2efd 100644 --- a/apps/api/src/isms/isms.service.lifecycle.spec.ts +++ b/apps/api/src/isms/isms.service.lifecycle.spec.ts @@ -18,6 +18,7 @@ jest.mock('@db', () => { }, member: { findFirst: jest.fn(), findMany: jest.fn() }, ismsRole: { findMany: jest.fn() }, + ismsAudit: { findMany: jest.fn() }, // lockDocument runs $executeRaw; submitForApproval wraps validate+update in a tx. $executeRaw: jest.fn(), $transaction: jest.fn((cb: (tx: unknown) => unknown) => cb(db)), @@ -193,6 +194,57 @@ describe('IsmsService document lifecycle', () => { await service.submitForApproval(args); expect(mockDb.ismsDocument.update).toHaveBeenCalled(); }); + + it('blocks an Internal Audit doc with no audits recorded (clause 9.2)', async () => { + (mockDb.member.findFirst as jest.Mock).mockResolvedValue({ id: 'mem_1' }); + (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ + id: 'doc_1', + type: 'internal_audit', + }); + (mockDb.ismsAudit.findMany as jest.Mock).mockResolvedValue([]); + + await expect(service.submitForApproval(args)).rejects.toThrow( + BadRequestException, + ); + expect(mockDb.ismsDocument.update).not.toHaveBeenCalled(); + }); + + it('blocks an Internal Audit doc with a completed audit missing its verdict', async () => { + (mockDb.member.findFirst as jest.Mock).mockResolvedValue({ id: 'mem_1' }); + (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ + id: 'doc_1', + type: 'internal_audit', + }); + (mockDb.ismsAudit.findMany as jest.Mock).mockResolvedValue([ + { + reference: 'IA-2026-01', + status: 'complete', + conclusionVerdict: null, + }, + ]); + + await expect(service.submitForApproval(args)).rejects.toThrow( + BadRequestException, + ); + }); + + it('allows an Internal Audit doc with a planned audit on record', async () => { + (mockDb.member.findFirst as jest.Mock).mockResolvedValue({ id: 'mem_1' }); + (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ + id: 'doc_1', + type: 'internal_audit', + }); + (mockDb.ismsAudit.findMany as jest.Mock).mockResolvedValue([ + { reference: 'IA-2026-01', status: 'planned', conclusionVerdict: null }, + ]); + (mockDb.ismsDocument.update as jest.Mock).mockResolvedValue({ + id: 'doc_1', + status: 'needs_review', + }); + + await service.submitForApproval(args); + expect(mockDb.ismsDocument.update).toHaveBeenCalled(); + }); }); describe('decline', () => { diff --git a/apps/api/src/isms/isms.service.spec.ts b/apps/api/src/isms/isms.service.spec.ts index ccb4ed9ebb..f764be4b56 100644 --- a/apps/api/src/isms/isms.service.spec.ts +++ b/apps/api/src/isms/isms.service.spec.ts @@ -1,5 +1,5 @@ import { NotFoundException } from '@nestjs/common'; -import { db } from '@db'; +import { db, Prisma } from '@db'; import { IsmsService } from './isms.service'; import type { IsmsVersionService } from './isms-version.service'; @@ -10,11 +10,15 @@ jest.mock('@db', () => ({ ismsDocument: { findMany: jest.fn(), createMany: jest.fn(), + updateMany: jest.fn(), }, ismsMetric: { findMany: jest.fn() }, + organization: { findUnique: jest.fn() }, control: { findMany: jest.fn() }, ismsDocumentControlLink: { createMany: jest.fn() }, }, + // The programme seed filters on the Prisma JSON-null sentinel. + Prisma: { AnyNull: Symbol.for('prisma.AnyNull') }, })); jest.mock('./documents/data-source', () => ({ collectPlatformData: jest.fn(), @@ -115,6 +119,43 @@ describe('IsmsService ensureSetup', () => { }); }); + it('seeds the programme on a new Internal Audit doc atomically — only while the narrative is still NULL (CS-724)', async () => { + ( + mockDb.frameworkEditorFramework.findUnique as jest.Mock + ).mockResolvedValue({ id: 'fw_1', requirements: [] }); + mockTemplates.mockResolvedValue([]); + (mockDb.organization.findUnique as jest.Mock).mockResolvedValue({ + name: 'Acme Corp', + }); + (mockDb.ismsDocument.findMany as jest.Mock) + .mockResolvedValueOnce([]) // existing-types probe + .mockResolvedValueOnce([{ id: 'doc_ia', type: 'internal_audit' }]) // created lookup + .mockResolvedValueOnce([]); // final list + + await service.ensureSetup(dto); + + // The empty-narrative filter (NULL or {}, generateNarrative's definition) + // is the concurrency guard: a narrative written between provisioning and + // this seed (concurrent setup call or an early customer edit) makes the + // update match zero rows instead of overwriting. + expect(mockDb.ismsDocument.updateMany).toHaveBeenCalledWith({ + where: { + id: 'doc_ia', + OR: [ + { draftNarrative: { equals: Prisma.AnyNull } }, + { draftNarrative: { equals: {} } }, + ], + }, + data: { + draftNarrative: { + programme: expect.stringContaining( + 'Acme Corp runs an annual internal audit', + ), + }, + }, + }); + }); + it('reports overdueMetricCount on the monitoring document row (CS-723)', async () => { const { addPeriods, periodStartFor } = jest.requireActual< typeof import('./utils/metric-periods') @@ -220,9 +261,9 @@ describe('IsmsService ensureSetup', () => { await service.ensureSetup(dto); expect(mockDb.ismsDocument.createMany).toHaveBeenCalledTimes(1); - // 1 template-driven + 7 definition fallbacks: a type shipped before its + // 1 template-driven + 8 definition fallbacks: a type shipped before its // template seed re-runs (e.g. monitoring, CS-723) still provisions. - expect(createManyData()).toHaveLength(8); + expect(createManyData()).toHaveLength(9); expect(createManyData()[0]).toMatchObject({ type: 'context_of_organization', title: 'Context of the Organization', @@ -306,9 +347,9 @@ describe('IsmsService ensureSetup', () => { await service.ensureSetup(dto); - // objectives (template) + 6 definition fallbacks; the existing + // objectives (template) + 7 definition fallbacks; the existing // context_of_organization is skipped. - expect(createManyData()).toHaveLength(7); + expect(createManyData()).toHaveLength(8); expect(createManyData()[0].type).toBe('objectives_plan'); expect( createManyData().map((doc: { type: string }) => doc.type), @@ -407,6 +448,7 @@ describe('IsmsService ensureSetup', () => { { type: 'roles_and_responsibilities' }, { type: 'objectives_plan' }, { type: 'monitoring' }, + { type: 'internal_audit' }, ]) .mockResolvedValueOnce([]); diff --git a/apps/api/src/isms/isms.service.ts b/apps/api/src/isms/isms.service.ts index ac6b470d6c..357e5dcee7 100644 --- a/apps/api/src/isms/isms.service.ts +++ b/apps/api/src/isms/isms.service.ts @@ -4,8 +4,7 @@ import { Injectable, NotFoundException, } from '@nestjs/common'; -import { db } from '@db'; -import type { Prisma } from '@db'; +import { db, Prisma } from '@db'; import { SubmitIsmsForApprovalDto } from './dto/submit-isms-for-approval.dto'; import { deriveControlLinks, resolveDocumentPlans } from './utils/ensure-setup-plan'; import { collectPlatformData } from './documents/data-source'; @@ -15,6 +14,8 @@ import { metricValidationMessages, seedMetricsIfMissing, } from './documents/monitoring'; +import { auditValidationMessages } from './documents/internal-audit'; +import { defaultProgrammeText } from './documents/internal-audit-defaults'; import { updateDraftSnapshot } from './utils/draft-snapshot'; import { EXPORT_DOCUMENT_INCLUDE } from './utils/export-payload'; import { lockDocument } from './utils/document-lock'; @@ -214,6 +215,40 @@ export class IsmsService { seedMetricsIfMissing({ tx, documentId: monitoringDoc.id }), ); } + + // Same first-load guarantee for Internal Audit (9.2): the Programme + // paragraph opens with its default text. The write is conditional on the + // narrative still being NULL (its creation state), so it is atomic: under + // concurrent setup calls — where the "created" lookup can also match a row + // the other call just created — an early customer edit can never be + // clobbered (the seed simply matches zero rows). + const internalAuditDoc = created.find( + (doc) => doc.type === 'internal_audit', + ); + if (internalAuditDoc) { + const organization = await db.organization.findUnique({ + where: { id: organizationId }, + select: { name: true }, + }); + await db.ismsDocument.updateMany({ + where: { + id: internalAuditDoc.id, + // "Empty" matches generateNarrative's definition: NULL (the + // creation state) or an empty object — never a populated draft. + OR: [ + { draftNarrative: { equals: Prisma.AnyNull } }, + { draftNarrative: { equals: {} } }, + ], + }, + data: { + draftNarrative: { + programme: defaultProgrammeText( + organization?.name ?? 'The organization', + ), + }, + }, + }); + } } async getDocument({ @@ -250,6 +285,13 @@ export class IsmsService { objective: { select: { id: true, objective: true, target: true } }, }, }, + audits: { + orderBy: { position: 'asc' }, + include: { + controls: { orderBy: { position: 'asc' } }, + findings: { orderBy: { position: 'asc' } }, + }, + }, controlLinks: { select: { id: true, @@ -301,6 +343,11 @@ export class IsmsService { if (document.type === 'monitoring') { await this.assertMonitoringComplete({ tx, documentId }); } + // Clause 9.2: at least one audit, and a conclusion verdict on every + // completed audit (CS-724). + if (document.type === 'internal_audit') { + await this.assertInternalAuditComplete({ tx, documentId }); + } return tx.ismsDocument.update({ where: { id: documentId }, @@ -525,6 +572,25 @@ export class IsmsService { } } + private async assertInternalAuditComplete({ + tx, + documentId, + }: { + tx: Prisma.TransactionClient; + documentId: string; + }) { + const audits = await tx.ismsAudit.findMany({ + where: { documentId }, + select: { reference: true, status: true, conclusionVerdict: true }, + }); + const messages = auditValidationMessages({ audits }); + if (messages.length > 0) { + throw new BadRequestException( + `This Clause 9.2 document is not ready to submit. ${messages.join(' ')}`, + ); + } + } + private async requireDocument({ documentId, organizationId, diff --git a/apps/api/src/isms/registers/register-registry.ts b/apps/api/src/isms/registers/register-registry.ts index b4d8b764f2..c653897c49 100644 --- a/apps/api/src/isms/registers/register-registry.ts +++ b/apps/api/src/isms/registers/register-registry.ts @@ -8,6 +8,9 @@ import type { IsmsRoleService } from '../isms-role.service'; import type { IsmsRoleAssignmentService } from '../isms-role-assignment.service'; import type { IsmsMetricService } from '../isms-metric.service'; import type { IsmsMeasurementService } from '../isms-measurement.service'; +import type { IsmsAuditService } from '../isms-audit.service'; +import type { IsmsAuditControlService } from '../isms-audit-control.service'; +import type { IsmsAuditFindingService } from '../isms-audit-finding.service'; /** * One generic dispatch for every ISMS register row (context issues, interested @@ -28,6 +31,20 @@ const COMPETENCE_BASIS = [ 'combination', ] as const; const METRIC_CADENCE = ['monthly', 'quarterly'] as const; +const AUDIT_STATUS = ['planned', 'in_progress', 'complete'] as const; +const AUDIT_CONCLUSION_VERDICT = [ + 'conform', + 'substantially_conform', + 'not_yet_conform', +] as const; +const AUDIT_CONTROL_RESULT = [ + 'conformity_confirmed', + 'nonconformity_raised', + 'observation_raised', + 'not_sampled', +] as const; +const AUDIT_FINDING_TYPE = ['nc_major', 'nc_minor', 'ofi', 'observation'] as const; +const AUDIT_FINDING_STATUS = ['open', 'in_progress', 'closed'] as const; const schemas = { contextIssueCreate: z.object({ @@ -176,6 +193,78 @@ const schemas = { value: z.string().trim().min(1).optional(), note: z.string().nullish(), }), + // reference is server-generated ("IA-YYYY-NN") and immutable; scope/criteria + // default to the clause-9.2 template text when omitted or blank. + auditCreate: z.object({ + scope: z.string().optional(), + criteria: z.string().optional(), + auditorName: z.string().nullish(), + plannedStartDate: z.string().nullish(), // ISO date string + plannedEndDate: z.string().nullish(), + position, + }), + auditUpdate: z.object({ + scope: z.string().trim().min(1).optional(), + criteria: z.string().trim().min(1).optional(), + // Nullish: undefined = leave as-is, null = clear. + auditorName: z.string().nullish(), + plannedStartDate: z.string().nullish(), + plannedEndDate: z.string().nullish(), + status: z.enum(AUDIT_STATUS).optional(), + conclusionVerdict: z.enum(AUDIT_CONCLUSION_VERDICT).nullish(), + conclusionNotes: z.string().nullish(), + // Sign-off slots: free-text names + ISO dates, each independently clearable. + signoffAuditorName: z.string().nullish(), + signoffAuditorDate: z.string().nullish(), + signoffSpoName: z.string().nullish(), + signoffSpoDate: z.string().nullish(), + signoffTopMgmtName: z.string().nullish(), + signoffTopMgmtDate: z.string().nullish(), + position, + }), + auditControlCreate: z.object({ + auditId: z.string(), + controlRef: z.string().trim().min(1), + whatWasTested: z.string().optional(), + whereToFind: z.string().optional(), + // Nullish: a row can be drafted before the auditor records an outcome. + result: z.enum(AUDIT_CONTROL_RESULT).nullish(), + notes: z.string().nullish(), + position, + }), + auditControlUpdate: z.object({ + controlRef: z.string().trim().min(1).optional(), + whatWasTested: z.string().optional(), + whereToFind: z.string().optional(), + result: z.enum(AUDIT_CONTROL_RESULT).nullish(), + notes: z.string().nullish(), + position, + }), + // reference is server-generated ("F-NN") and immutable. + auditFindingCreate: z.object({ + auditId: z.string(), + type: z.enum(AUDIT_FINDING_TYPE), + // Nullish: null/absent = a standalone finding not tied to a control row. + controlId: z.string().nullish(), + clauseOrControl: z.string().nullish(), + description: z.string().trim().min(1), + ownerMemberId: z.string().nullish(), + dueDate: z.string().nullish(), // ISO date string + status: z.enum(AUDIT_FINDING_STATUS).optional(), + closureEvidence: z.string().nullish(), + position, + }), + auditFindingUpdate: z.object({ + type: z.enum(AUDIT_FINDING_TYPE).optional(), + controlId: z.string().nullish(), + clauseOrControl: z.string().nullish(), + description: z.string().trim().min(1).optional(), + ownerMemberId: z.string().nullish(), + dueDate: z.string().nullish(), + status: z.enum(AUDIT_FINDING_STATUS).optional(), + closureEvidence: z.string().nullish(), + position, + }), } as const; /** One-save payload for the "Metrics due" / backfill views. */ @@ -216,6 +305,20 @@ export type UpdateMeasurementInput = z.infer; export type BulkCreateMeasurementInput = z.infer< typeof measurementBulkCreateSchema >; +export type CreateAuditInput = z.infer; +export type UpdateAuditInput = z.infer; +export type CreateAuditControlInput = z.infer< + typeof schemas.auditControlCreate +>; +export type UpdateAuditControlInput = z.infer< + typeof schemas.auditControlUpdate +>; +export type CreateAuditFindingInput = z.infer< + typeof schemas.auditFindingCreate +>; +export type UpdateAuditFindingInput = z.infer< + typeof schemas.auditFindingUpdate +>; export const ISMS_REGISTER_KEYS = [ 'context-issues', @@ -226,6 +329,9 @@ export const ISMS_REGISTER_KEYS = [ 'role-assignments', 'metrics', 'measurements', + 'audits', + 'audit-controls', + 'audit-findings', ] as const; export type IsmsRegisterKey = (typeof ISMS_REGISTER_KEYS)[number]; @@ -266,6 +372,9 @@ export interface RegisterServices { roleAssignments: IsmsRoleAssignmentService; metrics: IsmsMetricService; measurements: IsmsMeasurementService; + audits: IsmsAuditService; + auditControls: IsmsAuditControlService; + auditFindings: IsmsAuditFindingService; } /** Build the register → handler map from the injected per-register services. */ @@ -408,6 +517,54 @@ export function createRegisterRegistry( organizationId, }), }, + audits: { + create: ({ documentId, organizationId, data }) => + services.audits.create({ + documentId, + organizationId, + dto: parse(schemas.auditCreate, data), + }), + update: ({ rowId, organizationId, data }) => + services.audits.update({ + auditId: rowId, + organizationId, + dto: parse(schemas.auditUpdate, data), + }), + remove: ({ rowId, organizationId }) => + services.audits.remove({ auditId: rowId, organizationId }), + }, + 'audit-controls': { + create: ({ documentId, organizationId, data }) => + services.auditControls.create({ + documentId, + organizationId, + dto: parse(schemas.auditControlCreate, data), + }), + update: ({ rowId, organizationId, data }) => + services.auditControls.update({ + controlId: rowId, + organizationId, + dto: parse(schemas.auditControlUpdate, data), + }), + remove: ({ rowId, organizationId }) => + services.auditControls.remove({ controlId: rowId, organizationId }), + }, + 'audit-findings': { + create: ({ documentId, organizationId, data }) => + services.auditFindings.create({ + documentId, + organizationId, + dto: parse(schemas.auditFindingCreate, data), + }), + update: ({ rowId, organizationId, data }) => + services.auditFindings.update({ + findingId: rowId, + organizationId, + dto: parse(schemas.auditFindingUpdate, data), + }), + remove: ({ rowId, organizationId }) => + services.auditFindings.remove({ findingId: rowId, organizationId }), + }, }; } diff --git a/apps/api/src/isms/utils/document-types.spec.ts b/apps/api/src/isms/utils/document-types.spec.ts index 64d257afd1..a6118439a8 100644 --- a/apps/api/src/isms/utils/document-types.spec.ts +++ b/apps/api/src/isms/utils/document-types.spec.ts @@ -1,8 +1,8 @@ import { ISMS_TYPE_DEFINITIONS, matchRequirementId } from './document-types'; describe('ISMS_TYPE_DEFINITIONS', () => { - it('defines all eight foundational document types with clauses', () => { - expect(ISMS_TYPE_DEFINITIONS).toHaveLength(8); + it('defines all nine foundational document types with clauses', () => { + expect(ISMS_TYPE_DEFINITIONS).toHaveLength(9); const types = ISMS_TYPE_DEFINITIONS.map((d) => d.type); expect(types).toEqual( expect.arrayContaining([ @@ -14,6 +14,7 @@ describe('ISMS_TYPE_DEFINITIONS', () => { 'roles_and_responsibilities', 'objectives_plan', 'monitoring', + 'internal_audit', ]), ); }); @@ -25,6 +26,13 @@ describe('ISMS_TYPE_DEFINITIONS', () => { expect(monitoring?.clause).toBe('9.1'); }); + it('maps internal_audit to clause 9.2', () => { + const internalAudit = ISMS_TYPE_DEFINITIONS.find( + (d) => d.type === 'internal_audit', + ); + expect(internalAudit?.clause).toBe('9.2'); + }); + it('maps 4.2 to both interested-parties documents', () => { const clause42 = ISMS_TYPE_DEFINITIONS.filter((d) => d.clause === '4.2'); expect(clause42.map((d) => d.type)).toEqual([ diff --git a/apps/api/src/isms/utils/document-types.ts b/apps/api/src/isms/utils/document-types.ts index 55c69e94a3..149aebf739 100644 --- a/apps/api/src/isms/utils/document-types.ts +++ b/apps/api/src/isms/utils/document-types.ts @@ -73,6 +73,13 @@ export const ISMS_TYPE_DEFINITIONS: IsmsTypeDefinition[] = [ description: 'The metrics the organization monitors — what is measured, how, when, by whom, and who analyses the results (ISO 27001 clause 9.1).', }, + { + type: 'internal_audit', + clause: '9.2', + title: 'Internal Audit', + description: + 'The internal audit programme and the plan, controls tested, findings and conclusion of each internal audit of the ISMS (ISO 27001 clause 9.2).', + }, ]; /** diff --git a/apps/api/src/isms/utils/export-payload.ts b/apps/api/src/isms/utils/export-payload.ts index 2e64b032a3..c8e48ad3bb 100644 --- a/apps/api/src/isms/utils/export-payload.ts +++ b/apps/api/src/isms/utils/export-payload.ts @@ -12,6 +12,11 @@ import { mapMetrics, type MonitoringExtras, } from '../documents/monitoring-export-data'; +import { + loadInternalAuditExtras, + mapAudits, + type InternalAuditExtras, +} from '../documents/internal-audit-export-data'; import type { DocumentExportInput, IsmsOrgProfile, @@ -63,6 +68,13 @@ export const EXPORT_DOCUMENT_INCLUDE = { }, }, }, + audits: { + orderBy: { position: 'asc' }, + include: { + controls: { orderBy: { position: 'asc' } }, + findings: { orderBy: { position: 'asc' } }, + }, + }, } satisfies Prisma.IsmsDocumentInclude; export type LoadedExportDocument = Prisma.IsmsDocumentGetPayload<{ @@ -114,6 +126,18 @@ export async function resolveMonitoringExtras( }); } +/** The Internal Audit document (9.2) resolves finding-owner names; other types don't. */ +export async function resolveInternalAuditExtras( + document: LoadedExportDocument, + client?: Prisma.TransactionClient, +): Promise { + if (document.type !== 'internal_audit') return undefined; + return loadInternalAuditExtras({ + organizationId: document.organizationId, + client, + }); +} + function formatDateYmd(date: Date | null): string | null { return date ? date.toISOString().slice(0, 10) : null; } @@ -151,11 +175,13 @@ export function buildExportInput({ orgProfile, rolesExtras, monitoringExtras, + internalAuditExtras, }: { document: LoadedExportDocument; orgProfile?: IsmsOrgProfile; rolesExtras?: RolesExtras; monitoringExtras?: MonitoringExtras; + internalAuditExtras?: InternalAuditExtras; }): DocumentExportInput { return { contextIssues: document.contextIssues.map((issue) => ({ @@ -190,6 +216,9 @@ export function buildExportInput({ metrics: monitoringExtras ? mapMetrics(document.metrics, monitoringExtras) : undefined, + audits: internalAuditExtras + ? mapAudits(document.audits, internalAuditExtras) + : undefined, }; } @@ -211,11 +240,13 @@ export async function buildDraftSnapshot( const orgProfile = await resolveOrgProfile(document); const rolesExtras = await resolveRolesExtras(document); const monitoringExtras = await resolveMonitoringExtras(document); + const internalAuditExtras = await resolveInternalAuditExtras(document); const input = buildExportInput({ document, orgProfile, rolesExtras, monitoringExtras, + internalAuditExtras, }); const metadata = buildExportMetadata({ type: document.type, diff --git a/apps/api/src/isms/wizard/isms-profile.service.ts b/apps/api/src/isms/wizard/isms-profile.service.ts index 37ff28db88..be66f9dd55 100644 --- a/apps/api/src/isms/wizard/isms-profile.service.ts +++ b/apps/api/src/isms/wizard/isms-profile.service.ts @@ -34,6 +34,7 @@ const GENERATION_ORDER: Record = { roles_and_responsibilities: 5, objectives_plan: 6, monitoring: 7, + internal_audit: 8, }; const GENERATION_ORDER_DEFAULT = Object.keys(GENERATION_ORDER).length; diff --git a/apps/api/src/openapi-docs.spec.ts b/apps/api/src/openapi-docs.spec.ts index 2319299b9a..67a515fb99 100644 --- a/apps/api/src/openapi-docs.spec.ts +++ b/apps/api/src/openapi-docs.spec.ts @@ -263,4 +263,75 @@ describe('OpenAPI document', () => { expect(apiKeyOps.length).toBeGreaterThan(0); }); }); + + // Guardrail against the CS-761 regression: two DTO classes were both named + // `CreateVersionDto` — one in policies (optional sourceVersionId/changelog), + // one in tasks/automations (REQUIRED version + scriptKey). They collided on + // the OpenAPI component name, so the automations shape overwrote the policies + // component. That made the create-policy-version MCP tool demand version + + // scriptKey, which the policies endpoint's ValidationPipe (whitelist + + // forbidNonWhitelisted) then rejected with 400 — no valid path through it. + describe('create-policy-version tool schema', () => { + type SchemaLike = { + $ref?: string; + properties?: Record; + required?: string[]; + }; + + const resolveRequestBodyComponent = ( + routePath: string, + ): { name: string; schema: SchemaLike } | undefined => { + const operation = ( + document.paths[routePath] as + | { post?: { requestBody?: unknown } } + | undefined + )?.post; + const bodySchema = ( + operation?.requestBody as + | { content?: { 'application/json'?: { schema?: SchemaLike } } } + | undefined + )?.content?.['application/json']?.schema; + const ref = bodySchema?.$ref; + if (!ref) return undefined; + const name = ref.replace('#/components/schemas/', ''); + const schema = ( + document.components?.schemas as Record | undefined + )?.[name]; + if (!schema) return undefined; + return { name, schema }; + }; + + it('does not share an OpenAPI component with the automations create-version body', () => { + // Root-cause guard: the two `CreateVersionDto` classes must resolve to + // DISTINCT components, or one silently overwrites the other. + const policy = resolveRequestBodyComponent('/v1/policies/{id}/versions'); + const automation = resolveRequestBodyComponent( + '/v1/tasks/{taskId}/automations/{automationId}/versions', + ); + + expect(policy).toBeDefined(); + expect(automation).toBeDefined(); + expect(policy?.name).not.toBe(automation?.name); + }); + + it('exposes the optional policies shape, not the automations version/scriptKey fields', () => { + const policy = resolveRequestBodyComponent('/v1/policies/{id}/versions'); + expect(policy).toBeDefined(); + + const properties = policy?.schema.properties ?? {}; + const required = policy?.schema.required ?? []; + + // The policies endpoint accepts only these optional fields; the tool + // schema must match so an MCP client can call it without being blocked. + expect(Object.keys(properties)).toEqual( + expect.arrayContaining(['sourceVersionId', 'changelog']), + ); + // The automations-only fields must be absent — their presence (required) + // is exactly what triggered the "property scriptKey should not exist" 400. + expect(properties).not.toHaveProperty('scriptKey'); + expect(properties).not.toHaveProperty('version'); + expect(required).not.toContain('scriptKey'); + expect(required).not.toContain('version'); + }); + }); }); diff --git a/apps/api/src/organization/organization.controller.spec.ts b/apps/api/src/organization/organization.controller.spec.ts index 7e44303b07..29f296ba62 100644 --- a/apps/api/src/organization/organization.controller.spec.ts +++ b/apps/api/src/organization/organization.controller.spec.ts @@ -61,6 +61,7 @@ describe('OrganizationController', () => { isApiKey: false, isPlatformAdmin: false, userId: 'usr_123', + memberId: 'mem_123', userEmail: 'test@example.com', userRoles: ['owner'], }; @@ -252,4 +253,50 @@ describe('OrganizationController', () => { ).rejects.toThrow('settings is required and must be an array'); }); }); + + describe('createApiKey', () => { + beforeEach(() => { + mockApiKeyService.create.mockResolvedValue({ id: 'apk_1', key: 'comp_x' }); + }); + + it('attributes a session-created key to the creating member', async () => { + await controller.createApiKey('org_123', sessionAuthContext, { + name: 'CI Pipeline', + scopes: ['vendor:read'], + }); + + expect(mockApiKeyService.create).toHaveBeenCalledWith( + 'org_123', + 'CI Pipeline', + undefined, + ['vendor:read'], + 'mem_123', + ); + }); + + it('forwards null creator when created via API key / service token (no session member)', async () => { + await controller.createApiKey('org_123', apiKeyAuthContext, { + name: 'Nested Key', + scopes: ['vendor:read'], + }); + + expect(mockApiKeyService.create).toHaveBeenCalledWith( + 'org_123', + 'Nested Key', + undefined, + ['vendor:read'], + null, + ); + }); + + it('throws BadRequestException when name is missing', async () => { + await expect( + controller.createApiKey('org_123', sessionAuthContext, { + name: '', + scopes: ['vendor:read'], + }), + ).rejects.toThrow(BadRequestException); + expect(mockApiKeyService.create).not.toHaveBeenCalled(); + }); + }); }); diff --git a/apps/api/src/organization/organization.controller.ts b/apps/api/src/organization/organization.controller.ts index e0951b0f8d..d55657f6fd 100644 --- a/apps/api/src/organization/organization.controller.ts +++ b/apps/api/src/organization/organization.controller.ts @@ -359,6 +359,7 @@ export class OrganizationController { @ApiOperation({ summary: 'Create a new API key' }) async createApiKey( @OrganizationId() organizationId: string, + @AuthContext() authContext: AuthContextType, @Body() body: { name: string; expiresAt?: string; scopes?: string[] }, ) { if (!body.name) { @@ -369,6 +370,9 @@ export class OrganizationController { body.name, body.expiresAt, body.scopes, + // Attribute the key to the member creating it (session auth). Null when + // created via API key/service token — falls back to org owner at use time. + authContext.memberId ?? null, ); } diff --git a/apps/api/src/policies/dto/version.dto.ts b/apps/api/src/policies/dto/version.dto.ts index 9051a04cd9..20f9eda52a 100644 --- a/apps/api/src/policies/dto/version.dto.ts +++ b/apps/api/src/policies/dto/version.dto.ts @@ -1,7 +1,13 @@ -import { ApiProperty } from '@nestjs/swagger'; +import { ApiProperty, ApiSchema } from '@nestjs/swagger'; import { Transform } from 'class-transformer'; import { IsArray, IsBoolean, IsOptional, IsString } from 'class-validator'; +// A second class named `CreateVersionDto` exists in tasks/automations with a +// different, REQUIRED shape (version! + scriptKey!). Without a distinct OpenAPI +// component name the two collide, and the automations shape overwrites this +// one — which broke the create-policy-version MCP tool (it demanded fields the +// policies endpoint's ValidationPipe then rejected with 400). Keep this name. +@ApiSchema({ name: 'CreatePolicyVersionDto' }) export class CreateVersionDto { @ApiProperty({ description: 'Optional version ID to base the new version on', diff --git a/apps/api/src/policies/policies.controller.spec.ts b/apps/api/src/policies/policies.controller.spec.ts index b9a5aa5998..386ee08409 100644 --- a/apps/api/src/policies/policies.controller.spec.ts +++ b/apps/api/src/policies/policies.controller.spec.ts @@ -1,7 +1,8 @@ import { Test, TestingModule } from '@nestjs/testing'; import { HybridAuthGuard } from '../auth/hybrid-auth.guard'; import { PermissionGuard } from '../auth/permission.guard'; -import type { AuthContext } from '../auth/types'; +import { ActingUserResolver } from '../auth/acting-user.service'; +import type { AuthContext, AuthenticatedRequest } from '../auth/types'; import { PoliciesController } from './policies.controller'; import { PoliciesService } from './policies.service'; @@ -98,6 +99,7 @@ jest.mock('ai', () => ({ describe('PoliciesController', () => { let controller: PoliciesController; let policiesService: jest.Mocked; + let actingUser: jest.Mocked; const mockPoliciesService = { findAll: jest.fn(), @@ -119,8 +121,34 @@ describe('PoliciesController', () => { denyChanges: jest.fn(), }; + const mockActingUser = { + resolve: jest.fn(), + }; + const mockGuard = { canActivate: jest.fn().mockReturnValue(true) }; + function sessionReq(): AuthenticatedRequest { + return { + userId: 'usr_123', + organizationId: 'org_123', + authType: 'session', + isApiKey: false, + isServiceToken: false, + } as unknown as AuthenticatedRequest; + } + + function apiKeyReq(): AuthenticatedRequest { + return { + userId: undefined, + organizationId: 'org_123', + authType: 'api-key', + isApiKey: true, + isServiceToken: false, + apiKeyId: 'apk_1', + apiKeyName: 'CI Pipeline', + } as unknown as AuthenticatedRequest; + } + const mockAuthContext: AuthContext = { organizationId: 'org_123', authType: 'session', @@ -136,7 +164,10 @@ describe('PoliciesController', () => { beforeEach(async () => { const module: TestingModule = await Test.createTestingModule({ controllers: [PoliciesController], - providers: [{ provide: PoliciesService, useValue: mockPoliciesService }], + providers: [ + { provide: PoliciesService, useValue: mockPoliciesService }, + { provide: ActingUserResolver, useValue: mockActingUser }, + ], }) .overrideGuard(HybridAuthGuard) .useValue(mockGuard) @@ -146,6 +177,7 @@ describe('PoliciesController', () => { controller = module.get(PoliciesController); policiesService = module.get(PoliciesService); + actingUser = module.get(ActingUserResolver) as jest.Mocked; jest.clearAllMocks(); }); @@ -224,13 +256,18 @@ describe('PoliciesController', () => { }); describe('publishAllPolicies', () => { - it('should call policiesService.publishAll with correct params', async () => { + it('should call policiesService.publishAll with the resolved acting user', async () => { const mockResult = { count: 3 }; mockPoliciesService.publishAll.mockResolvedValue(mockResult); + actingUser.resolve.mockResolvedValueOnce({ + userId: 'usr_123', + source: 'session', + }); const result = await controller.publishAllPolicies( orgId, mockAuthContext, + sessionReq(), ); expect(policiesService.publishAll).toHaveBeenCalledWith( @@ -244,6 +281,40 @@ describe('PoliciesController', () => { authenticatedUser: { id: 'usr_123', email: 'test@example.com' }, }); }); + + it('attributes bulk publish to the resolved user/member for API-key callers', async () => { + // Regression: API-key callers have no authContext.userId, so the + // per-policy audit rows used to be dropped. The controller must resolve + // the acting user (key creator / org owner) and pass it to the service. + const mockResult = { count: 2 }; + mockPoliciesService.publishAll.mockResolvedValue(mockResult); + actingUser.resolve.mockResolvedValueOnce({ + userId: 'usr_owner', + memberId: 'mem_owner', + source: 'org-owner-fallback', + callerLabel: 'via API key "CI Pipeline"', + }); + + const apiKeyAuthContext: AuthContext = { + ...mockAuthContext, + userId: undefined, + userEmail: undefined, + authType: 'api-key', + isApiKey: true, + }; + + await controller.publishAllPolicies(orgId, apiKeyAuthContext, apiKeyReq()); + + expect(actingUser.resolve).toHaveBeenCalledWith( + expect.objectContaining({ isApiKey: true }), + orgId, + ); + expect(policiesService.publishAll).toHaveBeenCalledWith( + orgId, + 'usr_owner', + 'mem_owner', + ); + }); }); describe('downloadAllPolicies', () => { diff --git a/apps/api/src/policies/policies.controller.ts b/apps/api/src/policies/policies.controller.ts index 7416cb8e7a..d50b9f37f2 100644 --- a/apps/api/src/policies/policies.controller.ts +++ b/apps/api/src/policies/policies.controller.ts @@ -43,7 +43,11 @@ import { AuthContext, OrganizationId } from '../auth/auth-context.decorator'; import { HybridAuthGuard } from '../auth/hybrid-auth.guard'; import { PermissionGuard } from '../auth/permission.guard'; import { RequirePermission, RequirePermissions } from '../auth/require-permission.decorator'; -import type { AuthContext as AuthContextType } from '../auth/types'; +import { ActingUserResolver } from '../auth/acting-user.service'; +import type { + AuthContext as AuthContextType, + AuthenticatedRequest, +} from '../auth/types'; import { CreatePolicyDto } from './dto/create-policy.dto'; import { UpdatePolicyDto } from './dto/update-policy.dto'; import { AISuggestPolicyRequestDto } from './dto/ai-suggest-policy.dto'; @@ -110,7 +114,10 @@ function parsePolicyIdsParam( required: false, }) export class PoliciesController { - constructor(private readonly policiesService: PoliciesService) {} + constructor( + private readonly policiesService: PoliciesService, + private readonly actingUser: ActingUserResolver, + ) {} @Get() @RequirePermission('policy', 'read') @@ -162,11 +169,17 @@ export class PoliciesController { async publishAllPolicies( @OrganizationId() organizationId: string, @AuthContext() authContext: AuthContextType, + @Req() req: AuthenticatedRequest, ) { + // Resolve the acting user so per-policy audit rows are attributed correctly. + // Session callers have authContext.userId; API-key / service-token callers + // resolve to the key creator (else org owner) — without this, the granular + // audit rows would be dropped for API-key auth (userId undefined). + const acting = await this.actingUser.resolve(req, organizationId); const data = await this.policiesService.publishAll( organizationId, - authContext.userId, - authContext.memberId, + acting.userId ?? undefined, + acting.memberId ?? undefined, ); return { diff --git a/apps/api/src/trigger/policies/update-policy-helpers.spec.ts b/apps/api/src/trigger/policies/update-policy-helpers.spec.ts index 7f1c7cdf52..e2068c34ea 100644 --- a/apps/api/src/trigger/policies/update-policy-helpers.spec.ts +++ b/apps/api/src/trigger/policies/update-policy-helpers.spec.ts @@ -1,21 +1,48 @@ import { db } from '@db'; import { generateObject } from 'ai'; -import { processPolicyUpdate } from './update-policy-helpers'; +import { + processPolicyUpdate, + updatePolicyInDatabase, +} from './update-policy-helpers'; jest.mock('@db', () => ({ db: { organization: { findUnique: jest.fn() }, policy: { findUnique: jest.fn(), update: jest.fn() }, frameworkEditorPolicyTemplate: { findUnique: jest.fn() }, - policyVersion: { create: jest.fn(), deleteMany: jest.fn() }, + policyVersion: { + findFirst: jest.fn(), + create: jest.fn(), + deleteMany: jest.fn(), + }, $transaction: jest.fn(), }, + Prisma: { + PrismaClientKnownRequestError: class PrismaClientKnownRequestError {}, + }, + PolicyStatus: { + draft: 'draft', + published: 'published', + needs_review: 'needs_review', + }, })); jest.mock('@trigger.dev/sdk', () => ({ logger: { info: jest.fn(), warn: jest.fn(), error: jest.fn() }, })); +// Detached-PDF cleanup imports the S3 SDK dynamically; the mock intercepts it +// so tests can assert deletions without touching AWS. +const s3Send = jest.fn(); +jest.mock('@aws-sdk/client-s3', () => ({ + S3Client: class { + send = s3Send; + }, + DeleteObjectCommand: class { + constructor(public readonly input: { Bucket: string; Key: string }) {} + }, +})); + jest.mock('ai', () => ({ generateObject: jest.fn(), NoObjectGeneratedError: { isInstance: jest.fn(() => false) }, @@ -79,6 +106,7 @@ describe('processPolicyUpdate (individual policy regeneration)', () => { cb({ policy: { update: jest.fn() }, policyVersion: { + findFirst: jest.fn().mockResolvedValue(null), create: jest.fn(({ data }: { data: { content: unknown[] } }) => { storedContent = data.content; return { id: 'pv_1' }; @@ -127,3 +155,283 @@ describe('processPolicyUpdate (individual policy regeneration)', () => { expect(result.policyName).toBe('Information Security Policy'); }); }); + +// CS-766: Regenerating a PUBLISHED, signed policy must not touch the live +// version. It must append a new DRAFT version (for the approval workflow) while +// leaving policy.content, currentVersionId, signedBy, pdfUrl and the existing +// versions intact. Only publishing that draft (elsewhere) clears signedBy and +// re-triggers signing. +describe('updatePolicyInDatabase (published policy regeneration)', () => { + const REGEN_CONTENT = [ + { + type: 'paragraph', + content: [{ type: 'text', text: 'Regenerated draft content' }], + }, + ]; + + let txPolicyUpdate: jest.Mock; + let txVersionCreate: jest.Mock; + let txVersionDeleteMany: jest.Mock; + let txVersionFindFirst: jest.Mock; + + beforeEach(() => { + jest.clearAllMocks(); + + // A published policy: v1 is the current, signed, live version. + (db.policy.findUnique as jest.Mock).mockResolvedValue({ + id: 'pol_1', + status: 'published', + content: [ + { type: 'paragraph', content: [{ type: 'text', text: 'Published v1' }] }, + ], + currentVersionId: 'pv_1', + signedBy: ['mem_a', 'mem_b'], + pdfUrl: 'org_1/policies/pol_1/v1.pdf', + versions: [{ id: 'pv_1', pdfUrl: null, version: 1 }], + }); + + txPolicyUpdate = jest.fn(); + txVersionCreate = jest.fn(() => ({ id: 'pv_2' })); + txVersionDeleteMany = jest.fn(); + txVersionFindFirst = jest.fn().mockResolvedValue({ version: 1 }); + + (db.$transaction as jest.Mock).mockImplementation( + async (cb: (tx: unknown) => Promise) => + cb({ + policy: { update: txPolicyUpdate }, + policyVersion: { + findFirst: txVersionFindFirst, + create: txVersionCreate, + deleteMany: txVersionDeleteMany, + }, + }), + ); + }); + + it('appends a new draft version and preserves the published version + signatures', async () => { + await updatePolicyInDatabase('pol_1', REGEN_CONTENT, 'mem_regen'); + + // Existing versions (and their PDFs) must survive — the published version + // must not be destroyed. + expect(txVersionDeleteMany).not.toHaveBeenCalled(); + + // A brand-new version is appended at the next number (not overwriting v1). + expect(txVersionCreate).toHaveBeenCalledTimes(1); + const createData = txVersionCreate.mock.calls[0][0].data; + expect(createData.version).toBe(2); + expect(createData.changelog).toBe('Regenerated policy content'); + expect(JSON.stringify(createData.content)).toContain( + 'Regenerated draft content', + ); + + // The published policy row must NOT be mutated: no signature wipe, no live + // content swap, no currentVersion repoint. + const policyUpdateData = txPolicyUpdate.mock.calls.map( + (call) => (call[0] as { data?: Record })?.data ?? {}, + ); + for (const data of policyUpdateData) { + expect(data).not.toHaveProperty('signedBy'); + expect(data).not.toHaveProperty('content'); + expect(data).not.toHaveProperty('currentVersionId'); + } + }); +}); + +// CS-766 follow-up: Regenerating a DRAFT policy (never published, unsigned) must +// SURFACE the regenerated content. The editor renders the current version's +// content (falling back to policy.content), so regeneration overwrites the +// current draft version IN PLACE and syncs policy.content/draftContent — it must +// NOT append an unattached version that leaves the draft showing stale text. +describe('updatePolicyInDatabase (draft policy regeneration)', () => { + const REGEN_CONTENT = [ + { + type: 'paragraph', + content: [{ type: 'text', text: 'Regenerated draft content' }], + }, + ]; + + let txPolicyUpdate: jest.Mock; + let txPolicyFind: jest.Mock; + let txVersionUpdate: jest.Mock; + let txVersionCreate: jest.Mock; + let txVersionDeleteMany: jest.Mock; + + beforeEach(() => { + jest.clearAllMocks(); + + // A draft policy uploaded as a PDF: displayFormat is 'PDF' and both the + // policy and its current version carry a stale pdfUrl (the old document). + (db.policy.findUnique as jest.Mock).mockResolvedValue({ + id: 'pol_1', + status: 'draft', + currentVersionId: 'pv_1', + displayFormat: 'PDF', + pdfUrl: 'org_1/policies/pol_1/uploaded.pdf', + currentVersion: { pdfUrl: 'org_1/policies/pol_1/v1.pdf' }, + content: [ + { type: 'paragraph', content: [{ type: 'text', text: 'Stale draft' }] }, + ], + signedBy: [], + versions: [ + { id: 'pv_1', pdfUrl: 'org_1/policies/pol_1/v1.pdf', version: 1 }, + ], + }); + + txPolicyUpdate = jest.fn(); + // The in-transaction (row-locked) re-read that captures the detached keys. + txPolicyFind = jest.fn().mockResolvedValue({ + pdfUrl: 'org_1/policies/pol_1/uploaded.pdf', + currentVersion: { pdfUrl: 'org_1/policies/pol_1/v1.pdf' }, + }); + txVersionUpdate = jest.fn(); + txVersionCreate = jest.fn(() => ({ id: 'pv_2' })); + txVersionDeleteMany = jest.fn(); + + (db.$transaction as jest.Mock).mockImplementation( + async (cb: (tx: unknown) => Promise) => + cb({ + $executeRaw: jest.fn(), + policy: { update: txPolicyUpdate, findUniqueOrThrow: txPolicyFind }, + policyVersion: { + update: txVersionUpdate, + create: txVersionCreate, + deleteMany: txVersionDeleteMany, + findFirst: jest.fn().mockResolvedValue({ version: 1 }), + }, + }), + ); + }); + + it('overwrites the current draft version in place and syncs policy content (no unattached version)', async () => { + await updatePolicyInDatabase('pol_1', REGEN_CONTENT, 'mem_regen'); + + // The regenerated content overwrites the CURRENT draft version in place so + // the editor (which reads currentVersion.content) surfaces it. + expect(txVersionUpdate).toHaveBeenCalledTimes(1); + const versionUpdate = txVersionUpdate.mock.calls[0][0]; + expect(versionUpdate.where.id).toBe('pv_1'); + expect(JSON.stringify(versionUpdate.data.content)).toContain( + 'Regenerated draft content', + ); + + // No unattached extra version is appended (and nothing is deleted) for a + // draft — the working version is edited in place. + expect(txVersionCreate).not.toHaveBeenCalled(); + expect(txVersionDeleteMany).not.toHaveBeenCalled(); + + // policy.content AND draftContent advance to the regenerated content so the + // draft no longer shows stale text; currentVersionId is not repointed. + expect(txPolicyUpdate).toHaveBeenCalledTimes(1); + const policyUpdate = txPolicyUpdate.mock.calls[0][0].data; + expect(JSON.stringify(policyUpdate.content)).toContain( + 'Regenerated draft content', + ); + expect(JSON.stringify(policyUpdate.draftContent)).toContain( + 'Regenerated draft content', + ); + expect(policyUpdate).not.toHaveProperty('currentVersionId'); + }); + + it('clears stale PDF references and switches to EDITOR display when the draft was uploaded as a PDF', async () => { + await updatePolicyInDatabase('pol_1', REGEN_CONTENT, 'mem_regen'); + + // Regeneration produces EDITOR content: the policy must switch back to the + // editor and drop its stale policy-level PDF, otherwise the page opens on + // the PDF tab / export keeps serving the old uploaded document. + const policyUpdate = txPolicyUpdate.mock.calls[0][0].data; + expect(policyUpdate.displayFormat).toBe('EDITOR'); + expect(policyUpdate.pdfUrl).toBeNull(); + + // The current version's stale PDF (used first by render/export via + // currentVersion.pdfUrl ?? policy.pdfUrl) must be cleared too. + const versionUpdate = txVersionUpdate.mock.calls[0][0]; + expect(versionUpdate.data.pdfUrl).toBeNull(); + }); + + describe('detached-PDF S3 cleanup', () => { + const ENV_KEYS = [ + 'APP_AWS_BUCKET_NAME', + 'APP_AWS_ACCESS_KEY_ID', + 'APP_AWS_SECRET_ACCESS_KEY', + ] as const; + const ORIGINAL_ENV = Object.fromEntries( + ENV_KEYS.map((key) => [key, process.env[key]]), + ); + + const configureS3Env = () => { + process.env.APP_AWS_BUCKET_NAME = 'test-bucket'; + process.env.APP_AWS_ACCESS_KEY_ID = 'test-key'; + process.env.APP_AWS_SECRET_ACCESS_KEY = 'test-secret'; + }; + + afterEach(() => { + for (const key of ENV_KEYS) { + const original = ORIGINAL_ENV[key]; + if (original === undefined) delete process.env[key]; + else process.env[key] = original; + } + }); + + it('deletes the PDF keys captured inside the locked transaction', async () => { + configureS3Env(); + + await updatePolicyInDatabase('pol_1', REGEN_CONTENT, 'mem_regen'); + + // The keys come from the in-transaction re-read (not the outer snapshot), + // so a PDF uploaded concurrently before the row lock is still covered. + expect(txPolicyFind).toHaveBeenCalledTimes(1); + + const deletedKeys = s3Send.mock.calls.map( + (call) => (call[0] as { input: { Bucket: string; Key: string } }).input, + ); + expect(deletedKeys).toEqual([ + { Bucket: 'test-bucket', Key: 'org_1/policies/pol_1/uploaded.pdf' }, + { Bucket: 'test-bucket', Key: 'org_1/policies/pol_1/v1.pdf' }, + ]); + }); + + it('deduplicates when the policy and its version share one PDF key', async () => { + configureS3Env(); + txPolicyFind.mockResolvedValue({ + pdfUrl: 'org_1/policies/pol_1/shared.pdf', + currentVersion: { pdfUrl: 'org_1/policies/pol_1/shared.pdf' }, + }); + + await updatePolicyInDatabase('pol_1', REGEN_CONTENT, 'mem_regen'); + + expect(s3Send).toHaveBeenCalledTimes(1); + }); + + it('never fails the regeneration when an S3 delete fails (logged for cleanup)', async () => { + configureS3Env(); + s3Send.mockRejectedValueOnce(new Error('AccessDenied')); + + await expect( + updatePolicyInDatabase('pol_1', REGEN_CONTENT, 'mem_regen'), + ).resolves.toBeUndefined(); + // Both keys are still attempted; the failure is logged, not thrown. + expect(s3Send).toHaveBeenCalledTimes(2); + }); + + it('skips deletion (with a warning) when the S3 configuration is missing', async () => { + configureS3Env(); + delete process.env.APP_AWS_BUCKET_NAME; + + await updatePolicyInDatabase('pol_1', REGEN_CONTENT, 'mem_regen'); + + expect(s3Send).not.toHaveBeenCalled(); + }); + + it('does not touch S3 for an editor-mode draft with no PDFs', async () => { + configureS3Env(); + txPolicyFind.mockResolvedValue({ + pdfUrl: null, + currentVersion: { pdfUrl: null }, + }); + + await updatePolicyInDatabase('pol_1', REGEN_CONTENT, 'mem_regen'); + + expect(s3Send).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/apps/api/src/trigger/policies/update-policy-helpers.ts b/apps/api/src/trigger/policies/update-policy-helpers.ts index 7913092f1d..fb42f31380 100644 --- a/apps/api/src/trigger/policies/update-policy-helpers.ts +++ b/apps/api/src/trigger/policies/update-policy-helpers.ts @@ -1,9 +1,9 @@ -import { db } from '@db'; +import { DeleteObjectCommand, S3Client } from '@aws-sdk/client-s3'; +import { db, Prisma, PolicyStatus } from '@db'; import type { FrameworkEditorFramework, FrameworkEditorPolicyTemplate, Policy, - Prisma, } from '@db'; import { logger } from '@trigger.dev/sdk'; import { processTemplate } from './process-policy-template'; @@ -67,6 +67,48 @@ export async function fetchOrganizationAndPolicy( return { organization, policy, policyTemplate }; } +// Mirror PoliciesService.versionCreateRetries: retry version creation on a +// unique-constraint race so two near-simultaneous regenerations don't collide +// on the [policyId, version] key. +const POLICY_VERSION_CREATE_RETRIES = 3; + +/** + * Best-effort removal of the PDF objects a draft regeneration detached + * (pdfUrl stores the raw S3 key — same contract as the policies controller). + * Runs AFTER the database transaction commits so a failed delete can never + * roll back the content update; failures are logged with their keys so the + * objects can be cleaned up later instead of orphaning silently. + */ +async function deleteDetachedPdfObjects(keys: string[]): Promise { + if (keys.length === 0) return; + // Same APP_AWS_* configuration as the other trigger tasks (evidence export) + // — but non-throwing: cleanup is best-effort and must never fail the + // regeneration, so missing configuration is logged and skipped. + const bucketName = process.env.APP_AWS_BUCKET_NAME; + const accessKeyId = process.env.APP_AWS_ACCESS_KEY_ID; + const secretAccessKey = process.env.APP_AWS_SECRET_ACCESS_KEY; + if (!bucketName || !accessKeyId || !secretAccessKey) { + logger.warn( + `APP_AWS_* S3 configuration missing; skipped deleting detached policy PDFs: ${keys.join(', ')}`, + ); + return; + } + const s3 = new S3Client({ + region: process.env.APP_AWS_REGION || 'us-east-1', + credentials: { accessKeyId, secretAccessKey }, + ...(process.env.APP_AWS_ENDPOINT + ? { endpoint: process.env.APP_AWS_ENDPOINT, forcePathStyle: true } + : {}), + }); + for (const key of keys) { + try { + await s3.send(new DeleteObjectCommand({ Bucket: bucketName, Key: key })); + } catch (error) { + logger.warn(`Failed to delete detached policy PDF ${key}: ${error}`); + } + } +} + export async function updatePolicyInDatabase( policyId: string, content: Record[], @@ -75,79 +117,118 @@ export async function updatePolicyInDatabase( try { const policy = await db.policy.findUnique({ where: { id: policyId }, - include: { versions: { select: { id: true, pdfUrl: true } } }, + select: { id: true, status: true, currentVersionId: true }, }); if (!policy) throw new Error(`Policy not found: ${policyId}`); - // Delete S3 files for existing versions - const pdfUrlsToDelete = policy.versions - .map((v) => v.pdfUrl) - .filter((url): url is string => !!url); + const versionContent = content as unknown as Prisma.InputJsonValue[]; - if (pdfUrlsToDelete.length > 0) { - try { - const { S3Client, DeleteObjectCommand } = - await import('@aws-sdk/client-s3'); - const bucketName = process.env.APP_AWS_BUCKET_NAME; - if (bucketName) { - const s3 = new S3Client({ - region: process.env.AWS_REGION || 'us-east-1', + // A draft policy has never been published: there is no live, signed content + // to protect. The editor renders the CURRENT version's content + // (currentVersion.content, falling back to policy.content), so regeneration + // must overwrite the draft's working content IN PLACE — mirroring how + // PoliciesService.updateById persists draft content edits. Appending an + // unattached version instead (the published path below) would leave + // policy.content / currentVersionId pointing at the old text, so the user + // regenerates but keeps seeing the stale draft (CS-766). + // + // A draft can be in PDF display mode (the user uploaded a PDF as its + // content): displayFormat = 'PDF' with pdfUrl set on the policy and/or the + // current version. Regeneration produces EDITOR content, so we must clear + // those stale PDF references and switch displayFormat back to 'EDITOR' — + // otherwise the page opens on the PDF tab and export/render (which use + // currentVersion.pdfUrl ?? policy.pdfUrl) keep serving the old uploaded + // document instead of the regenerated content (CS-766). + if (policy.status === PolicyStatus.draft) { + // The uploaded-PDF keys this regeneration detaches, captured INSIDE the + // transaction under a row lock: a concurrent PDF upload (a plain UPDATE + // on the policy row) blocks on the lock, so the captured keys are + // exactly the values the updates below clear — nothing committed in + // between can slip an untracked orphan past the cleanup. Deleted from + // S3 only after the transaction commits. + const detachedPdfKeys = await db.$transaction(async (tx) => { + await tx.$executeRaw`SELECT id FROM "Policy" WHERE id = ${policyId} FOR UPDATE`; + const current = await tx.policy.findUniqueOrThrow({ + where: { id: policyId }, + select: { pdfUrl: true, currentVersion: { select: { pdfUrl: true } } }, + }); + if (policy.currentVersionId) { + await tx.policyVersion.update({ + where: { id: policy.currentVersionId }, + data: { + content: versionContent, + changelog: 'Regenerated policy content', + pdfUrl: null, + }, }); - await Promise.allSettled( - pdfUrlsToDelete.map((pdfUrl) => - s3.send( - new DeleteObjectCommand({ Bucket: bucketName, Key: pdfUrl }), - ), - ), - ); } - } catch (s3Error) { - logger.error(`Error deleting S3 files during regeneration: ${s3Error}`); - } - } - - await db.$transaction(async (tx) => { - // Clear version references first to avoid FK constraint issues during deletion. - // Clear approverId alongside pendingVersionId so the two fields never diverge - // — any lingering approverId without a pending version produces the inconsistent - // state behind CS-254/260/261 ("No pending version to approve"). - if (policy.versions.length > 0) { await tx.policy.update({ where: { id: policyId }, data: { - currentVersionId: null, - pendingVersionId: null, - approverId: null, + content: versionContent, + draftContent: versionContent, + pdfUrl: null, + displayFormat: 'EDITOR', }, }); - await tx.policyVersion.deleteMany({ where: { policyId } }); - } - - const newVersion = await tx.policyVersion.create({ - data: { - policyId, - version: 1, - content: content as unknown as Prisma.InputJsonValue[], - publishedById: memberId || null, - changelog: 'Regenerated policy content', - }, + return [ + ...new Set( + [current.pdfUrl, current.currentVersion?.pdfUrl].filter( + (key): key is string => !!key, + ), + ), + ]; }); + await deleteDetachedPdfObjects(detachedPdfKeys); + return; + } - await tx.policy.update({ - where: { id: policyId }, - data: { - content: content as unknown as Prisma.InputJsonValue[], - draftContent: content as unknown as Prisma.InputJsonValue[], - currentVersionId: newVersion.id, - pendingVersionId: null, - approverId: null, - signedBy: [], - pdfUrl: null, - displayFormat: 'EDITOR', - }, - }); - }); + // Published / needs_review: regeneration must NOT mutate the live policy. + // This previously deleted every version (and its PDF) and overwrote + // policy.content / currentVersionId while clearing signedBy — replacing the + // live, signed policy with unreviewed AI content and wiping every signature + // (CS-766). Instead, create a new DRAFT version holding the regenerated + // content and leave the published content, currentVersionId, signatures, PDF + // and existing versions untouched. The draft is reviewed through the normal + // version workflow; only publishing it (which clears signedBy) re-triggers + // signing. + for ( + let attempt = 1; + attempt <= POLICY_VERSION_CREATE_RETRIES; + attempt += 1 + ) { + try { + await db.$transaction(async (tx) => { + const latestVersion = await tx.policyVersion.findFirst({ + where: { policyId }, + orderBy: { version: 'desc' }, + select: { version: true }, + }); + const nextVersion = (latestVersion?.version ?? 0) + 1; + + await tx.policyVersion.create({ + data: { + policyId, + version: nextVersion, + content: versionContent, + publishedById: memberId || null, + changelog: 'Regenerated policy content', + }, + }); + }); + return; + } catch (error) { + if ( + error instanceof Prisma.PrismaClientKnownRequestError && + error.code === 'P2002' && + attempt < POLICY_VERSION_CREATE_RETRIES + ) { + continue; + } + throw error; + } + } } catch (dbError) { logger.error(`Failed to update policy in database: ${dbError}`); throw dbError; diff --git a/apps/api/src/vendors/vendors.controller.spec.ts b/apps/api/src/vendors/vendors.controller.spec.ts index 76391db6e6..60629d1729 100644 --- a/apps/api/src/vendors/vendors.controller.spec.ts +++ b/apps/api/src/vendors/vendors.controller.spec.ts @@ -1,7 +1,9 @@ import { Test, TestingModule } from '@nestjs/testing'; +import { BadRequestException } from '@nestjs/common'; import { HybridAuthGuard } from '../auth/hybrid-auth.guard'; import { PermissionGuard } from '../auth/permission.guard'; -import type { AuthContext } from '../auth/types'; +import { ActingUserResolver } from '../auth/acting-user.service'; +import type { AuthContext, AuthenticatedRequest } from '../auth/types'; // Mock auth.server to avoid importing better-auth ESM in Jest jest.mock('../auth/auth.server', () => ({ @@ -19,6 +21,26 @@ jest.mock('@trycompai/auth', () => ({ BUILT_IN_ROLE_PERMISSIONS: {}, })); +// The controller graph imports @db (Prisma client + many enums referenced by the +// vendor DTOs' @ApiProperty decorators). Mock it so the real Prisma client isn't +// constructed at load. Any enum member access (e.g. VendorCategory.other, +// RiskTreatmentType.accept) returns the member name as its value, so we don't +// have to enumerate every enum. Service + resolver are mocked providers, so `db` +// itself is never queried. +jest.mock('@db', () => { + const enumStub = new Proxy( + {}, + { get: (_target, prop) => (typeof prop === 'string' ? prop : undefined) }, + ); + const known: Record = { __esModule: true, db: {}, Prisma: {} }; + return new Proxy(known, { + get: (target, prop) => { + if (typeof prop !== 'string') return undefined; + return prop in target ? target[prop] : enumStub; + }, + }); +}); + import { VendorsController } from './vendors.controller'; import { VendorsService } from './vendors.service'; @@ -36,8 +58,15 @@ describe('VendorsController', () => { deleteById: jest.fn(), }; + const mockActingUser = { resolve: jest.fn() }; + const mockGuard = { canActivate: jest.fn().mockReturnValue(true) }; + // The resolver reads off the request; it's mocked here, so a minimal shape + // is enough. Session requests carry userId; API-key requests don't. + const sessionReq = { userId: 'usr_123' } as unknown as AuthenticatedRequest; + const apiKeyReq = { isApiKey: true } as unknown as AuthenticatedRequest; + const mockAuthContext: AuthContext = { organizationId: 'org_123', authType: 'session', @@ -59,7 +88,10 @@ describe('VendorsController', () => { beforeEach(async () => { const module: TestingModule = await Test.createTestingModule({ controllers: [VendorsController], - providers: [{ provide: VendorsService, useValue: mockVendorsService }], + providers: [ + { provide: VendorsService, useValue: mockVendorsService }, + { provide: ActingUserResolver, useValue: mockActingUser }, + ], }) .overrideGuard(HybridAuthGuard) .useValue(mockGuard) @@ -71,6 +103,12 @@ describe('VendorsController', () => { vendorsService = module.get(VendorsService); jest.clearAllMocks(); + // Default: session caller resolves to their own user. + mockActingUser.resolve.mockResolvedValue({ + userId: 'usr_123', + memberId: 'mem_123', + source: 'session', + }); }); describe('searchGlobalVendors', () => { @@ -183,6 +221,7 @@ describe('VendorsController', () => { dto as any, 'org_123', mockAuthContext, + sessionReq, ); expect(result).toMatchObject(createdVendor); @@ -191,6 +230,7 @@ describe('VendorsController', () => { id: 'usr_123', email: 'test@example.com', }); + // Session caller: create is attributed to the resolved (session) user. expect(vendorsService.create).toHaveBeenCalledWith( 'org_123', dto, @@ -198,15 +238,24 @@ describe('VendorsController', () => { ); }); - it('should not include authenticatedUser when userId is missing', async () => { + it('attributes API-key creates to the resolved actor (key creator / owner)', async () => { const dto = { name: 'New Vendor' }; const createdVendor = { id: 'vnd_new', name: 'New Vendor' }; mockVendorsService.create.mockResolvedValue(createdVendor); + // API-key caller has no session userId; the resolver supplies the + // responsible user so the auto-created assessment task is attributed to + // a real person instead of falling back to the org owner downstream. + mockActingUser.resolve.mockResolvedValueOnce({ + userId: 'usr_creator', + memberId: 'mem_creator', + source: 'api-key-creator', + }); const result = await controller.createVendor( dto as any, 'org_123', apiKeyAuthContext, + apiKeyReq, ); expect(result.authenticatedUser).toBeUndefined(); @@ -214,9 +263,22 @@ describe('VendorsController', () => { expect(vendorsService.create).toHaveBeenCalledWith( 'org_123', dto, - undefined, + 'usr_creator', ); }); + + it('rejects with 400 when no actor can be resolved (no owner)', async () => { + const dto = { name: 'New Vendor' }; + mockActingUser.resolve.mockResolvedValueOnce({ + userId: null, + source: 'org-owner-fallback', + }); + + await expect( + controller.createVendor(dto as any, 'org_123', apiKeyAuthContext, apiKeyReq), + ).rejects.toThrow(BadRequestException); + expect(vendorsService.create).not.toHaveBeenCalled(); + }); }); describe('updateVendor', () => { @@ -270,7 +332,7 @@ describe('VendorsController', () => { const result = await controller.triggerAssessment( 'vnd_1', 'org_123', - mockAuthContext, + sessionReq, ); expect(result).toEqual({ @@ -284,19 +346,36 @@ describe('VendorsController', () => { ); }); - it('should pass undefined userId when auth context has no userId', async () => { + it('attributes API-key triggers to the resolved actor', async () => { mockVendorsService.triggerAssessment.mockResolvedValue({ status: 'pending', }); + mockActingUser.resolve.mockResolvedValueOnce({ + userId: 'usr_owner', + memberId: 'mem_owner', + source: 'org-owner-fallback', + }); - await controller.triggerAssessment('vnd_1', 'org_123', apiKeyAuthContext); + await controller.triggerAssessment('vnd_1', 'org_123', apiKeyReq); expect(vendorsService.triggerAssessment).toHaveBeenCalledWith( 'vnd_1', 'org_123', - undefined, + 'usr_owner', ); }); + + it('rejects with 400 when no actor can be resolved (no owner)', async () => { + mockActingUser.resolve.mockResolvedValueOnce({ + userId: null, + source: 'org-owner-fallback', + }); + + await expect( + controller.triggerAssessment('vnd_1', 'org_123', apiKeyReq), + ).rejects.toThrow(BadRequestException); + expect(vendorsService.triggerAssessment).not.toHaveBeenCalled(); + }); }); describe('deleteVendor', () => { diff --git a/apps/api/src/vendors/vendors.controller.ts b/apps/api/src/vendors/vendors.controller.ts index 41d47bb101..6860479483 100644 --- a/apps/api/src/vendors/vendors.controller.ts +++ b/apps/api/src/vendors/vendors.controller.ts @@ -1,4 +1,5 @@ import { + BadRequestException, Controller, Get, Post, @@ -7,6 +8,7 @@ import { Body, Param, Query, + Req, UseGuards, } from '@nestjs/common'; import { @@ -22,7 +24,11 @@ import { AuthContext, OrganizationId } from '../auth/auth-context.decorator'; import { HybridAuthGuard } from '../auth/hybrid-auth.guard'; import { PermissionGuard } from '../auth/permission.guard'; import { RequirePermission } from '../auth/require-permission.decorator'; -import type { AuthContext as AuthContextType } from '../auth/types'; +import { ActingUserResolver } from '../auth/acting-user.service'; +import type { + AuthContext as AuthContextType, + AuthenticatedRequest, +} from '../auth/types'; import { CreateVendorDto } from './dto/create-vendor.dto'; import { UpdateVendorDto } from './dto/update-vendor.dto'; import { VendorsService } from './vendors.service'; @@ -40,7 +46,10 @@ import { DELETE_VENDOR_RESPONSES } from './schemas/delete-vendor.responses'; @UseGuards(HybridAuthGuard, PermissionGuard) @ApiSecurity('apikey') export class VendorsController { - constructor(private readonly vendorsService: VendorsService) {} + constructor( + private readonly vendorsService: VendorsService, + private readonly actingUser: ActingUserResolver, + ) {} @Get('global/search') @RequirePermission('vendor', 'read') @@ -123,11 +132,25 @@ export class VendorsController { @Body() createVendorDto: CreateVendorDto, @OrganizationId() organizationId: string, @AuthContext() authContext: AuthContextType, + @Req() req: AuthenticatedRequest, ) { + // Attribute the vendor + its auto-generated assessment task to the acting + // user. Session callers have authContext.userId; API-key / service-token + // callers resolve to the key creator (else org owner) so the "created this + // task" activity credits a real person, not the org owner by default. + const acting = await this.actingUser.resolve(req, organizationId); + if (!acting.userId) { + // No attributable user (org has no active owner). Reject with an + // actionable message rather than create a vendor whose assessment task + // has no acting user — matches ActingUserResolver's contract. + throw new BadRequestException( + 'Cannot attribute this action — your organization must have at least one active user with the "owner" role.', + ); + } const vendor = await this.vendorsService.create( organizationId, createVendorDto, - authContext.userId, // Pass user ID for task assignment + acting.userId, // Pass user ID for task assignment ); return { @@ -185,12 +208,18 @@ export class VendorsController { async triggerAssessment( @Param('id') vendorId: string, @OrganizationId() organizationId: string, - @AuthContext() authContext: AuthContextType, + @Req() req: AuthenticatedRequest, ) { + const acting = await this.actingUser.resolve(req, organizationId); + if (!acting.userId) { + throw new BadRequestException( + 'Cannot attribute this action — your organization must have at least one active user with the "owner" role.', + ); + } const result = await this.vendorsService.triggerAssessment( vendorId, organizationId, - authContext.userId, + acting.userId, ); return { diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/[type]/page.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/[type]/page.tsx index e60edfb7c2..213d007f9e 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/[type]/page.tsx +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/[type]/page.tsx @@ -8,6 +8,7 @@ import { resolveUserPermissions } from '@/lib/permissions.server'; import { auth } from '@/utils/auth'; import { ContextOfOrganizationClient } from '../components/ContextOfOrganizationClient'; import { InterestedPartiesClient } from '../components/InterestedPartiesClient'; +import { InternalAuditClient } from '../components/InternalAuditClient'; import type { ApproverOption } from '../components/IsmsApprovalSection'; import { LeadershipClient } from '../components/LeadershipClient'; import { MonitoringClient } from '../components/MonitoringClient'; @@ -33,6 +34,8 @@ interface IsmsDetailClientProps { approverOptions: ApproverOption[]; /** All active members (for the Roles member pickers); superset of approvers. */ memberOptions: ApproverOption[]; + /** Internal Auditor holder(s) from Roles (5.3) — only for the 9.2 document. */ + auditorOptions?: string[]; } const ISMS_DETAIL_CLIENTS: Record< @@ -45,6 +48,7 @@ const ISMS_DETAIL_CLIENTS: Record< objectives_plan: ObjectivesClient, roles_and_responsibilities: RolesClient, monitoring: MonitoringClient, + internal_audit: InternalAuditClient, isms_scope: ScopeClient, leadership_commitment: LeadershipClient, }; @@ -163,6 +167,39 @@ export default async function IsmsDocumentPage({ .map((p) => ({ id: p.id, name: p.user?.name ?? p.user?.email ?? 'Unknown' })) .sort((a, b) => a.name.localeCompare(b.name)); + // The Internal Audit auditor dropdown is whoever Roles (5.3) says the + // Internal Auditor is — assigned members and/or the external firm. No + // defaulting on our part: an empty list means Roles is not configured yet. + let auditorOptions: string[] = []; + if (documentType === 'internal_audit') { + const rolesDoc = setupResult.data?.documents?.find( + (doc) => doc.type === 'roles_and_responsibilities', + ); + if (rolesDoc) { + const rolesResult = await serverApi.get( + `/v1/isms/documents/${rolesDoc.id}`, + ); + const auditorRole = rolesResult.data?.roles?.find( + (role) => role.roleKey === 'internal_auditor', + ); + const memberNames = new Map(memberOptions.map((m) => [m.id, m.name])); + const holders = (auditorRole?.assignments ?? []) + .map((assignment) => memberNames.get(assignment.memberId)) + .filter((name): name is string => !!name); + const routeHolder = auditorRole?.auditRouteMemberId + ? memberNames.get(auditorRole.auditRouteMemberId) + : undefined; + const firm = auditorRole?.auditFirmName?.trim(); + auditorOptions = [ + ...new Set( + [...holders, routeHolder, firm].filter( + (name): name is string => !!name, + ), + ), + ]; + } + } + const DetailClient = ISMS_DETAIL_CLIENTS[documentType]; return ( @@ -175,6 +212,7 @@ export default async function IsmsDocumentPage({ currentMemberId={currentMember?.id ?? null} approverOptions={approverOptions} memberOptions={memberOptions} + auditorOptions={auditorOptions} /> ); diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/AuditCard.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/AuditCard.tsx new file mode 100644 index 0000000000..b8d86f89e7 --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/AuditCard.tsx @@ -0,0 +1,276 @@ +'use client'; + +import { zodResolver } from '@hookform/resolvers/zod'; +import { + AlertDialog, + AlertDialogAction, + AlertDialogCancel, + AlertDialogContent, + AlertDialogDescription, + AlertDialogFooter, + AlertDialogHeader, + AlertDialogTitle, + Badge, + Button, + Grid, + Heading, + HStack, + Stack, +} from '@trycompai/design-system'; +import { Edit, TrashCan } from '@trycompai/design-system/icons'; +import { useEffect, useState } from 'react'; +import { useForm } from 'react-hook-form'; +import type { IsmsAudit } from '../isms-types'; +import { AuditControlsTable } from './AuditControlsTable'; +import { AuditFields } from './AuditFields'; +import { + AuditFindingsSection, + type FindingPrefill, +} from './AuditFindingsSection'; +import { AuditSignoffCard } from './AuditSignoffCard'; +import type { ApproverOption } from './IsmsApprovalSection'; +import { + auditDetailsSchema, + type AuditControlFormValues, + type AuditDetailsFormValues, + type FindingFormValues, + type SignoffFormValues, +} from './audit-schema'; +import { + AUDIT_STATUS_LABELS, + conclusionSentence, +} from './internal-audit-constants'; +import { IsmsRegisterCard, IsmsRegisterField } from './shared'; + +/** All mutations an audit card (and its child sections) can perform. */ +export interface AuditHandlers { + onUpdateAudit: (auditId: string, values: AuditDetailsFormValues) => Promise; + onDeleteAudit: (auditId: string) => Promise; + onSaveSignoff: (auditId: string, values: SignoffFormValues) => Promise; + onCreateControl: (auditId: string, values: AuditControlFormValues) => Promise; + onUpdateControl: (controlId: string, payload: Record) => Promise; + onDeleteControl: (controlId: string) => Promise; + onCreateFinding: (auditId: string, values: FindingFormValues) => Promise; + onUpdateFinding: (findingId: string, values: FindingFormValues) => Promise; + onDeleteFinding: (findingId: string) => Promise; +} + +interface AuditCardProps extends AuditHandlers { + audit: IsmsAudit; + canEdit: boolean; + memberOptions: ApproverOption[]; + auditorOptions: string[]; +} + +function toFormValues(audit: IsmsAudit): AuditDetailsFormValues { + return { + scope: audit.scope, + criteria: audit.criteria, + auditorName: audit.auditorName ?? '', + plannedStartDate: audit.plannedStartDate?.slice(0, 10) ?? '', + plannedEndDate: audit.plannedEndDate?.slice(0, 10) ?? '', + status: audit.status, + conclusionVerdict: audit.conclusionVerdict ?? '', + conclusionNotes: audit.conclusionNotes ?? '', + }; +} + +export function AuditCard({ + audit, + canEdit, + memberOptions, + auditorOptions, + onUpdateAudit, + onDeleteAudit, + onSaveSignoff, + onCreateControl, + onUpdateControl, + onDeleteControl, + onCreateFinding, + onUpdateFinding, + onDeleteFinding, +}: AuditCardProps) { + const [isEditing, setIsEditing] = useState(false); + const [isDeleting, setIsDeleting] = useState(false); + const [confirmOpen, setConfirmOpen] = useState(false); + // Set when a Controls Tested row is marked non-conformity / observation: + // the findings section opens a linked-finding form pre-filled from the row. + const [findingPrefill, setFindingPrefill] = useState( + null, + ); + + const { + control, + handleSubmit, + reset, + formState: { isDirty, isValid, isSubmitting }, + } = useForm({ + resolver: zodResolver(auditDetailsSchema), + mode: 'onChange', + defaultValues: toFormValues(audit), + }); + + useEffect(() => { + if (!isEditing) reset(toFormValues(audit)); + }, [audit, isEditing, reset]); + + const handleSave = handleSubmit(async (values) => { + try { + await onUpdateAudit(audit.id, values); + } catch { + return; + } + setIsEditing(false); + }); + + const handleDelete = async () => { + setConfirmOpen(false); + setIsDeleting(true); + try { + await onDeleteAudit(audit.id); + } catch { + // Error already surfaced via toast by the caller. + } finally { + setIsDeleting(false); + } + }; + + const headerActions = canEdit ? ( + isEditing ? ( + + + + + ) : ( + + +