From 09aeba6787a4d9d398c9a3d5064361ef27554605 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Mon, 15 Jun 2026 09:17:35 -0400 Subject: [PATCH 1/2] fix(framework-editor): unblock framework import (10k requirement limit + content shape) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two prod bugs hit when importing an exported framework (e.g. the first NIST SP800-53 framework): 1. 5000-char limit on import. FRAME-2 raised the requirement description cap to 10000 only on the standalone requirement DTOs; the import flow uses a separate ImportRequirementDto that was still @MaxLength(5000). NIST PL-2's requirement text is >6000 chars, so import failed validation. Raise it to 10000 to match (control/policy/task stay 5000, consistent with their standalone create DTOs). 2. "policyTemplates[].content must be an object". Policy content is an untyped Json column that legitimately holds either a {type:'doc',content:[...]} object or a bare ProseMirror node array (legacy data). Export emits it verbatim, but the import DTO required @IsObject, which rejects arrays. - Relax ImportPolicyTemplateDto.content to accept array-or-object. - Normalize content to a canonical doc node on import-persist (and on export for forward-compat) via normalizeTipTapDoc. This also replaces the old `(content ?? {})` fallback that wrote a bare {} — which onboarding silently turned into an empty policy. Adds the IsObjectOrArray validator, the normalizeTipTapDoc helper, and tests for both (DTO validation boundaries + helper shape cases). All 60 framework- editor tests pass. Co-Authored-By: Claude Opus 4.8 --- .../dto/import-framework.dto.spec.ts | 107 ++++++++++++++++++ .../framework/dto/import-framework.dto.ts | 12 +- .../framework/framework-export.service.ts | 12 +- .../framework/normalize-tiptap-doc.spec.ts | 36 ++++++ .../framework/normalize-tiptap-doc.ts | 47 ++++++++ .../is-object-or-array.validator.ts | 35 ++++++ 6 files changed, 242 insertions(+), 7 deletions(-) create mode 100644 apps/api/src/framework-editor/framework/dto/import-framework.dto.spec.ts create mode 100644 apps/api/src/framework-editor/framework/normalize-tiptap-doc.spec.ts create mode 100644 apps/api/src/framework-editor/framework/normalize-tiptap-doc.ts create mode 100644 apps/api/src/framework-editor/validators/is-object-or-array.validator.ts diff --git a/apps/api/src/framework-editor/framework/dto/import-framework.dto.spec.ts b/apps/api/src/framework-editor/framework/dto/import-framework.dto.spec.ts new file mode 100644 index 0000000000..635ca8a3f6 --- /dev/null +++ b/apps/api/src/framework-editor/framework/dto/import-framework.dto.spec.ts @@ -0,0 +1,107 @@ +// Mock @db so importing the DTO (which pulls enums from @db) doesn't spin up a +// real Prisma client. The mocked enums are what @IsEnum validates against, so +// the payloads below use these values. +jest.mock('@db', () => ({ + Frequency: { monthly: 'monthly', yearly: 'yearly' }, + Departments: { it: 'it', admin: 'admin' }, + EvidenceFormType: { policy: 'policy' }, + TaskAutomationStatus: { manual: 'manual' }, +})); + +import { plainToInstance } from 'class-transformer'; +import { validate, type ValidationError } from 'class-validator'; +import { Departments, Frequency } from '@db'; +import { ImportFrameworkDto } from './import-framework.dto'; + +function collectMessages(errors: ValidationError[]): string[] { + const out: string[] = []; + for (const e of errors) { + if (e.constraints) out.push(...Object.values(e.constraints)); + if (e.children?.length) out.push(...collectMessages(e.children)); + } + return out; +} + +const FREQUENCY = Object.values(Frequency)[0] as Frequency; +const DEPARTMENT = Object.values(Departments)[0] as Departments; + +function basePayload( + overrides: { requirementDescription?: string; policyContent?: unknown } = {}, +) { + return { + version: '1', + framework: { name: 'NIST SP800-53', version: '5', description: 'Low impact', visible: false }, + requirements: [ + { + name: 'System Security and Privacy Plans', + identifier: 'PL-2', + description: overrides.requirementDescription ?? 'Develop security and privacy plans.', + requirementFamily: 'PL - Planning', + }, + ], + policyTemplates: [ + { + name: 'Access Control Policy', + description: 'Policy', + frequency: FREQUENCY, + department: DEPARTMENT, + content: + 'policyContent' in overrides ? overrides.policyContent : { type: 'doc', content: [] }, + }, + ], + controlTemplates: [], + taskTemplates: [], + }; +} + +async function validatePayload(plain: Record) { + const dto = plainToInstance(ImportFrameworkDto, plain, { enableImplicitConversion: true }); + return collectMessages(await validate(dto, { whitelist: true })); +} + +describe('ImportFrameworkDto', () => { + it('accepts a valid payload', async () => { + expect(await validatePayload(basePayload())).toHaveLength(0); + }); + + // Bug A — the import path must allow the same 10,000-char requirement + // descriptions the standalone requirement editor allows (NIST PL-2 > 6000). + it('accepts a 10,000-char requirement description', async () => { + expect( + await validatePayload(basePayload({ requirementDescription: 'x'.repeat(10_000) })), + ).toHaveLength(0); + }); + + it('rejects a requirement description longer than 10,000 chars', async () => { + const messages = await validatePayload( + basePayload({ requirementDescription: 'x'.repeat(10_001) }), + ); + expect(messages.some((m) => m.includes('10000'))).toBe(true); + }); + + // Bug B — policy content may be a doc object OR a bare node array. + it('accepts policy content as a bare node array', async () => { + expect( + await validatePayload(basePayload({ policyContent: [{ type: 'paragraph', content: [] }] })), + ).toHaveLength(0); + }); + + it('accepts policy content as a doc object', async () => { + expect( + await validatePayload( + basePayload({ policyContent: { type: 'doc', content: [{ type: 'paragraph' }] } }), + ), + ).toHaveLength(0); + }); + + it('rejects policy content that is a primitive', async () => { + const messages = await validatePayload(basePayload({ policyContent: 'not-json' })); + expect(messages.some((m) => m.toLowerCase().includes('object or an array'))).toBe(true); + }); + + it('still rejects oversized policy content (size guard kept)', async () => { + const huge = [{ type: 'text', text: 'x'.repeat(520_000) }]; + const messages = await validatePayload(basePayload({ policyContent: huge })); + expect(messages.some((m) => m.toLowerCase().includes('maximum allowed size'))).toBe(true); + }); +}); diff --git a/apps/api/src/framework-editor/framework/dto/import-framework.dto.ts b/apps/api/src/framework-editor/framework/dto/import-framework.dto.ts index ba5a93dd39..8b8d1acb80 100644 --- a/apps/api/src/framework-editor/framework/dto/import-framework.dto.ts +++ b/apps/api/src/framework-editor/framework/dto/import-framework.dto.ts @@ -7,7 +7,6 @@ import { IsOptional, IsArray, IsInt, - IsObject, IsEnum, MaxLength, ValidateNested, @@ -21,6 +20,7 @@ import { TaskAutomationStatus, } from '@db'; import { MaxJsonSize } from '../../validators/max-json-size.validator'; +import { IsObjectOrArray } from '../../validators/is-object-or-array.validator'; class ImportFrameworkMetaDto { @ApiProperty() @@ -60,10 +60,12 @@ class ImportRequirementDto { @MaxLength(255) identifier?: string; + // Matches the standalone requirement DTOs (FRAME-2). NIST SP800-53 control + // text routinely exceeds 5000 chars (e.g. PL-2 > 6000). @ApiProperty() @IsString() @IsNotEmpty() - @MaxLength(5000) + @MaxLength(10000) description: string; @ApiPropertyOptional() @@ -145,11 +147,13 @@ class ImportPolicyTemplateDto { @IsEnum(Departments) department: Departments; + // TipTap content is stored either as a `{ type: 'doc', … }` object or a bare + // node array (legacy data) — accept both; it's normalized on persist. @ApiPropertyOptional() - @IsObject() + @IsObjectOrArray() @IsOptional() @MaxJsonSize() - content?: Record; + content?: Record | unknown[]; } class ImportTaskTemplateDto { diff --git a/apps/api/src/framework-editor/framework/framework-export.service.ts b/apps/api/src/framework-editor/framework/framework-export.service.ts index db8b655a3b..da66a42d1c 100644 --- a/apps/api/src/framework-editor/framework/framework-export.service.ts +++ b/apps/api/src/framework-editor/framework/framework-export.service.ts @@ -6,6 +6,7 @@ import { } from '@nestjs/common'; import { db, Prisma, EvidenceFormType } from '@db'; import type { ImportFrameworkDto } from './dto/import-framework.dto'; +import { normalizeTipTapDoc } from './normalize-tiptap-doc'; export interface ExportedFramework { version: string; @@ -36,7 +37,7 @@ export interface ExportedFramework { description: string; frequency: string; department: string; - content: Record; + content: Prisma.InputJsonObject; }>; taskTemplates: Array<{ name: string; @@ -151,7 +152,9 @@ export class FrameworkExportService { description: p.description, frequency: p.frequency, department: p.department, - content: p.content as Record, + // Emit a canonical doc node so every exported file is homogeneous, + // regardless of how the policy's content was stored. + content: normalizeTipTapDoc(p.content), })), taskTemplates: taskTemplates.map((t) => ({ name: t.name, @@ -204,7 +207,10 @@ export class FrameworkExportService { description: p.description, frequency: p.frequency, department: p.department, - content: (p.content ?? {}) as Prisma.InputJsonValue, + // Normalize array/object/empty content to a canonical doc node — + // accepts the array shape legacy exports emit and avoids writing a + // bare `{}` (which onboarding turns into an empty policy). + content: normalizeTipTapDoc(p.content), }, }), ), diff --git a/apps/api/src/framework-editor/framework/normalize-tiptap-doc.spec.ts b/apps/api/src/framework-editor/framework/normalize-tiptap-doc.spec.ts new file mode 100644 index 0000000000..98c9c70383 --- /dev/null +++ b/apps/api/src/framework-editor/framework/normalize-tiptap-doc.spec.ts @@ -0,0 +1,36 @@ +import { normalizeTipTapDoc } from './normalize-tiptap-doc'; + +describe('normalizeTipTapDoc', () => { + it('wraps a bare node array into a doc', () => { + const nodes = [{ type: 'paragraph', content: [] }]; + expect(normalizeTipTapDoc(nodes)).toEqual({ type: 'doc', content: nodes }); + }); + + it('passes through an existing doc node', () => { + const doc = { type: 'doc', content: [{ type: 'paragraph' }] }; + expect(normalizeTipTapDoc(doc)).toEqual(doc); + }); + + it('empties a doc node with a non-array content', () => { + expect(normalizeTipTapDoc({ type: 'doc' })).toEqual({ type: 'doc', content: [] }); + }); + + it('wraps a single non-doc node', () => { + const node = { type: 'paragraph', content: [{ type: 'text', text: 'hi' }] }; + expect(normalizeTipTapDoc(node)).toEqual({ type: 'doc', content: [node] }); + }); + + it('returns an empty doc for null / undefined', () => { + expect(normalizeTipTapDoc(null)).toEqual({ type: 'doc', content: [] }); + expect(normalizeTipTapDoc(undefined)).toEqual({ type: 'doc', content: [] }); + }); + + it('returns an empty doc for a bare object (the {} footgun)', () => { + expect(normalizeTipTapDoc({})).toEqual({ type: 'doc', content: [] }); + }); + + it('returns an empty doc for primitive values', () => { + expect(normalizeTipTapDoc('a string')).toEqual({ type: 'doc', content: [] }); + expect(normalizeTipTapDoc(42)).toEqual({ type: 'doc', content: [] }); + }); +}); diff --git a/apps/api/src/framework-editor/framework/normalize-tiptap-doc.ts b/apps/api/src/framework-editor/framework/normalize-tiptap-doc.ts new file mode 100644 index 0000000000..922dbaad45 --- /dev/null +++ b/apps/api/src/framework-editor/framework/normalize-tiptap-doc.ts @@ -0,0 +1,47 @@ +// Type-only import: keeps this helper (and its unit test) from instantiating +// the Prisma client at module load. +import type { Prisma } from '@db'; + +/** + * Normalize arbitrary stored/imported policy content into a `{ type: 'doc', … }` + * document node. + * + * Policy content lives in an untyped `Json` column and legitimately appears in + * several shapes: a proper doc object, a bare array of nodes (legacy/seeded + * data), a single node, or nothing. Export passes whatever is stored through + * verbatim, and the import DTO used to require an object — so array-shaped + * content was rejected ("content must be an object"). + * + * This mirrors the top-level branches of the editor's + * `validateAndFixTipTapContent` (packages/ui) without the deep node repair — + * onboarding/render consumers run their own sanitization. We only need a valid + * top-level doc shape. Returning `{ type: 'doc', content: [] }` for empty input + * (instead of a bare `{}`) also avoids the footgun where onboarding silently + * turns `{}` into an empty policy. + */ +export function normalizeTipTapDoc(content: unknown): Prisma.InputJsonObject { + // Bare array of nodes → wrap as a doc. + if (Array.isArray(content)) { + return { type: 'doc', content: content as Prisma.InputJsonValue[] }; + } + + if (content !== null && typeof content === 'object') { + const node = content as { type?: unknown; content?: unknown }; + + // Already a doc → keep its content array (or empty it if malformed). + if (node.type === 'doc') { + return { + type: 'doc', + content: Array.isArray(node.content) ? (node.content as Prisma.InputJsonValue[]) : [], + }; + } + + // A single non-doc node (has a string type) → wrap it. + if (typeof node.type === 'string') { + return { type: 'doc', content: [content as Prisma.InputJsonValue] }; + } + } + + // Missing / empty / non-node value → empty doc. + return { type: 'doc', content: [] }; +} diff --git a/apps/api/src/framework-editor/validators/is-object-or-array.validator.ts b/apps/api/src/framework-editor/validators/is-object-or-array.validator.ts new file mode 100644 index 0000000000..2bead4e4bb --- /dev/null +++ b/apps/api/src/framework-editor/validators/is-object-or-array.validator.ts @@ -0,0 +1,35 @@ +import { + registerDecorator, + ValidationOptions, + ValidatorConstraint, + ValidatorConstraintInterface, +} from 'class-validator'; + +/** + * Accepts any non-null object OR array. Unlike class-validator's `@IsObject` + * (which explicitly rejects arrays), this allows both shapes — needed for + * TipTap content, which is legitimately stored either as a `{ type: 'doc', … }` + * object or as a bare array of ProseMirror nodes. + */ +@ValidatorConstraint({ name: 'isObjectOrArray', async: false }) +export class IsObjectOrArrayConstraint implements ValidatorConstraintInterface { + validate(value: unknown): boolean { + return typeof value === 'object' && value !== null; + } + + defaultMessage(): string { + return 'value must be an object or an array'; + } +} + +export function IsObjectOrArray(validationOptions?: ValidationOptions) { + return function (object: object, propertyName: string) { + registerDecorator({ + target: object.constructor, + propertyName, + options: validationOptions, + constraints: [], + validator: IsObjectOrArrayConstraint, + }); + }; +} From e2f22a2c6162069e79c611b5d0fbf9f8f5c6bbe1 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Mon, 15 Jun 2026 09:21:30 -0400 Subject: [PATCH 2/2] fix(framework-editor): raise all import description limits to 10k Extend the import limit fix beyond requirement descriptions: control, policy, and task descriptions on import also go 5000 -> 10000. Import is the bulk-load path for externally-authored frameworks (e.g. NIST), where any description can be verbose, and we can't assume only requirements exceed 5000. All four columns are unbounded Postgres text, so there's no DB risk. Adds a test covering 10k control/policy/task descriptions. Co-Authored-By: Claude Opus 4.8 --- .../dto/import-framework.dto.spec.ts | 21 +++++++++++++++++++ .../framework/dto/import-framework.dto.ts | 6 +++--- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/apps/api/src/framework-editor/framework/dto/import-framework.dto.spec.ts b/apps/api/src/framework-editor/framework/dto/import-framework.dto.spec.ts index 635ca8a3f6..fc52ec406a 100644 --- a/apps/api/src/framework-editor/framework/dto/import-framework.dto.spec.ts +++ b/apps/api/src/framework-editor/framework/dto/import-framework.dto.spec.ts @@ -79,6 +79,27 @@ describe('ImportFrameworkDto', () => { expect(messages.some((m) => m.includes('10000'))).toBe(true); }); + // Import is the bulk-load path for externally-authored frameworks (NIST), so + // every description field accepts 10,000 — not just requirements. + it('accepts 10,000-char control / policy / task descriptions', async () => { + const long = 'x'.repeat(10_000); + const payload = { + ...basePayload(), + controlTemplates: [{ name: 'C', description: long, controlFamily: 'AC' }], + policyTemplates: [ + { + name: 'P', + description: long, + frequency: FREQUENCY, + department: DEPARTMENT, + content: { type: 'doc', content: [] }, + }, + ], + taskTemplates: [{ name: 'T', description: long, frequency: FREQUENCY, department: DEPARTMENT }], + }; + expect(await validatePayload(payload)).toHaveLength(0); + }); + // Bug B — policy content may be a doc object OR a bare node array. it('accepts policy content as a bare node array', async () => { expect( diff --git a/apps/api/src/framework-editor/framework/dto/import-framework.dto.ts b/apps/api/src/framework-editor/framework/dto/import-framework.dto.ts index 8b8d1acb80..c5f3b91463 100644 --- a/apps/api/src/framework-editor/framework/dto/import-framework.dto.ts +++ b/apps/api/src/framework-editor/framework/dto/import-framework.dto.ts @@ -85,7 +85,7 @@ class ImportControlTemplateDto { @ApiProperty() @IsString() @IsNotEmpty() - @MaxLength(5000) + @MaxLength(10000) description: string; @ApiPropertyOptional({ example: 'AC - Access Control' }) @@ -136,7 +136,7 @@ class ImportPolicyTemplateDto { @ApiProperty() @IsString() @IsNotEmpty() - @MaxLength(5000) + @MaxLength(10000) description: string; @ApiProperty() @@ -166,7 +166,7 @@ class ImportTaskTemplateDto { @ApiProperty() @IsString() @IsNotEmpty() - @MaxLength(5000) + @MaxLength(10000) description: string; @ApiProperty()