fix(frameworks): address review feedback — tenant-FK hardening + custom-framework parity

carhartlewis · claude · carhartlewis · commit d296e2a5309a · 2026-04-17T23:26:14.000+01:00
- Schema: enforce tenant consistency on CustomFramework FKs with composite
  (id, organizationId) references. CustomRequirement and FrameworkInstance
  can no longer point at another org's CustomFramework, even if application
  code regresses. Migration adds a guard that aborts if any existing row
  already violates the invariant.
- Scoring: use EvidenceSubmission.submittedAt (canonical submission time)
  instead of createdAt for the "within last 6 months" recency check;
  update all evidenceSubmission selects/orderBys in frameworks.service
  to match.
- Policies (both admin + org): include customFramework when collecting the
  AI policy-regeneration context so org-custom frameworks influence the
  prompt instead of being silently dropped.
- Frontend framework list: exclude already-added custom frameworks from the
  "available to add" list on both overview and frameworks pages.
- useControls createControl payload: tighten requirementMappings to a
  discriminated union so invalid requirementId+customRequirementId combos
  fail at compile time (matches the backend's exactly-one rule).
- Controls controller: validate :formType path param with ParseEnumPipe.
- Document-type labels map: type as Record&lt;EvidenceFormType, string&gt; so
  Prisma enum drift becomes a compile error.
- DocumentsTable: disable every unlink button while any unlink is pending
  so users can't click enabled controls that no-op.

Co-Authored-By: Claude Opus 4.7 (1M context) &lt;noreply@anthropic.com&gt;
diff --git a/apps/api/src/admin-organizations/admin-policies.controller.ts b/apps/api/src/admin-organizations/admin-policies.controller.ts
@@ -130,24 +130,41 @@ export class AdminPoliciesController {
   ) {
     const instances = await db.frameworkInstance.findMany({
       where: { organizationId: orgId },
-      include: { framework: true },
+      include: { framework: true, customFramework: true },
     });
 
+    const normalized = instances.map((fi) => {
+      if (fi.framework) {
+        return {
+          id: fi.framework.id,
+          name: fi.framework.name,
+          version: fi.framework.version,
+          description: fi.framework.description,
+          visible: fi.framework.visible,
+          createdAt: fi.framework.createdAt,
+          updatedAt: fi.framework.updatedAt,
+        };
+      }
+      if (fi.customFramework) {
+        return {
+          id: fi.customFramework.id,
+          name: fi.customFramework.name,
+          version: fi.customFramework.version,
+          description: fi.customFramework.description,
+          visible: true,
+          createdAt: fi.customFramework.createdAt,
+          updatedAt: fi.customFramework.updatedAt,
+        };
+      }
+      return null;
+    });
     const uniqueFrameworks = Array.from(
       new Map(
-        instances
-          .filter((fi) => fi.framework)
-          .map((fi) => [fi.framework!.id, fi.framework!]),
+        normalized
+          .filter((f): f is NonNullable<typeof f> => f !== null)
+          .map((f) => [f.id, f]),
       ).values(),
-    ).map((f) => ({
-      id: f.id,
-      name: f.name,
-      version: f.version,
-      description: f.description,
-      visible: f.visible,
-      createdAt: f.createdAt,
-      updatedAt: f.updatedAt,
-    }));
+    );
 
     const contextEntries = await db.context.findMany({
       where: { organizationId: orgId },
diff --git a/apps/api/src/controls/controls.controller.ts b/apps/api/src/controls/controls.controller.ts
@@ -4,6 +4,7 @@ import {
   Delete,
   Get,
   Param,
+  ParseEnumPipe,
   Post,
   Query,
   UseGuards,
@@ -159,7 +160,8 @@ export class ControlsController {
   async unlinkDocumentType(
     @OrganizationId() organizationId: string,
     @Param('id') id: string,
-    @Param('formType') formType: EvidenceFormType,
+    @Param('formType', new ParseEnumPipe(EvidenceFormType))
+    formType: EvidenceFormType,
   ) {
     return this.controlsService.unlinkDocumentType(
       id,
diff --git a/apps/api/src/frameworks/frameworks-scores.helper.ts b/apps/api/src/frameworks/frameworks-scores.helper.ts
@@ -224,7 +224,7 @@ interface TaskWithControls {
 
 interface EvidenceSubmissionForScoring {
   formType: string;
-  createdAt: Date | string;
+  submittedAt: Date | string;
 }
 
 function isControlCompleted(
@@ -252,14 +252,14 @@ function isControlCompleted(
   if (documentTypes.length > 0) {
     const sorted = [...evidenceSubmissions].sort(
       (a, b) =>
-        new Date(b.createdAt).getTime() - new Date(a.createdAt).getTime(),
+        new Date(b.submittedAt).getTime() - new Date(a.submittedAt).getTime(),
     );
     const now = Date.now();
     for (const dt of documentTypes) {
       const latest = sorted.find((es) => es.formType === dt.formType);
       if (
         !latest ||
-        now - new Date(latest.createdAt).getTime() > SIX_MONTHS_MS
+        now - new Date(latest.submittedAt).getTime() > SIX_MONTHS_MS
       ) {
         documentsComplete = false;
         break;
diff --git a/apps/api/src/frameworks/frameworks.service.ts b/apps/api/src/frameworks/frameworks.service.ts
@@ -121,7 +121,7 @@ export class FrameworksService {
       }),
       db.evidenceSubmission.findMany({
         where: { organizationId },
-        select: { formType: true, createdAt: true },
+        select: { formType: true, submittedAt: true },
       }),
     ]);
 
@@ -199,8 +199,8 @@ export class FrameworksService {
                 organizationId,
                 formType: { in: Array.from(allFormTypes) },
               },
-              select: { id: true, formType: true, createdAt: true },
-              orderBy: { createdAt: 'desc' },
+              select: { id: true, formType: true, submittedAt: true },
+              orderBy: { submittedAt: 'desc' },
             })
           : Promise.resolve([]),
       ]);
@@ -503,8 +503,8 @@ export class FrameworksService {
               organizationId,
               formType: { in: Array.from(formTypes) },
             },
-            select: { id: true, formType: true, createdAt: true },
-            orderBy: { createdAt: 'desc' },
+            select: { id: true, formType: true, submittedAt: true },
+            orderBy: { submittedAt: 'desc' },
           })
         : [];
 
diff --git a/apps/api/src/policies/policies.controller.ts b/apps/api/src/policies/policies.controller.ts
@@ -225,24 +225,43 @@ export class PoliciesController {
 
     const instances = await db.frameworkInstance.findMany({
       where: { organizationId },
-      include: { framework: true },
+      include: { framework: true, customFramework: true },
     });
 
+    // Normalize platform + org-custom frameworks into a single shape so the AI
+    // context reflects every framework the org has enabled, not just platform.
+    const normalized = instances.map((fi) => {
+      if (fi.framework) {
+        return {
+          id: fi.framework.id,
+          name: fi.framework.name,
+          version: fi.framework.version,
+          description: fi.framework.description,
+          visible: fi.framework.visible,
+          createdAt: fi.framework.createdAt,
+          updatedAt: fi.framework.updatedAt,
+        };
+      }
+      if (fi.customFramework) {
+        return {
+          id: fi.customFramework.id,
+          name: fi.customFramework.name,
+          version: fi.customFramework.version,
+          description: fi.customFramework.description,
+          visible: true,
+          createdAt: fi.customFramework.createdAt,
+          updatedAt: fi.customFramework.updatedAt,
+        };
+      }
+      return null;
+    });
     const uniqueFrameworks = Array.from(
       new Map(
-        instances
-          .filter((fi) => fi.framework)
-          .map((fi) => [fi.framework!.id, fi.framework!]),
+        normalized
+          .filter((f): f is NonNullable<typeof f> => f !== null)
+          .map((f) => [f.id, f]),
       ).values(),
-    ).map((f) => ({
-      id: f.id,
-      name: f.name,
-      version: f.version,
-      description: f.description,
-      visible: f.visible,
-      createdAt: f.createdAt,
-      updatedAt: f.updatedAt,
-    }));
+    );
 
     const contextEntries = await db.context.findMany({
       where: { organizationId },
diff --git a/apps/app/src/app/(app)/[orgId]/controls/hooks/useControls.ts b/apps/app/src/app/(app)/[orgId]/controls/hooks/useControls.ts
@@ -9,16 +9,16 @@ interface ControlsApiResponse {
   pageCount: number;
 }
 
+type RequirementMappingPayload =
+  | { requirementId: string; customRequirementId?: never; frameworkInstanceId: string }
+  | { requirementId?: never; customRequirementId: string; frameworkInstanceId: string };
+
 interface CreateControlPayload {
   name: string;
   description: string;
   policyIds?: string[];
   taskIds?: string[];
-  requirementMappings?: {
-    requirementId?: string;
-    customRequirementId?: string;
-    frameworkInstanceId: string;
-  }[];
+  requirementMappings?: RequirementMappingPayload[];
   documentTypes?: string[];
 }
 
diff --git a/apps/app/src/app/(app)/[orgId]/frameworks/[frameworkInstanceId]/controls/[controlId]/components/DocumentsTable.tsx b/apps/app/src/app/(app)/[orgId]/frameworks/[frameworkInstanceId]/controls/[controlId]/components/DocumentsTable.tsx
@@ -113,7 +113,7 @@ export function DocumentsTable({
                   <Button
                     size="sm"
                     variant="ghost"
-                    disabled={pending === row.formType}
+                    disabled={pending !== null}
                     onClick={(e) => {
                       e.stopPropagation();
                       handleUnlink(row.formType);
diff --git a/apps/app/src/app/(app)/[orgId]/frameworks/[frameworkInstanceId]/controls/[controlId]/components/documentTypeLabels.ts b/apps/app/src/app/(app)/[orgId]/frameworks/[frameworkInstanceId]/controls/[controlId]/components/documentTypeLabels.ts
@@ -1,8 +1,12 @@
+import { EvidenceFormType } from '@db';
+
 // Prisma `EvidenceFormType` enum values are snake_case at the TS level
 // (Prisma maps them to kebab-case strings in the DB). The `/documents/[formType]`
 // URL uses the kebab-case form, so we convert when navigating.
 
-export const DOCUMENT_TYPE_LABELS: Record<string, string> = {
+// Typed as Record<EvidenceFormType, string> so adding a new form-type to the
+// Prisma enum without updating this map becomes a compile error.
+export const DOCUMENT_TYPE_LABELS: Record<EvidenceFormType, string> = {
   meeting: 'Meeting',
   board_meeting: 'Board Meeting',
   it_leadership_meeting: 'IT Leadership Meeting',
@@ -17,10 +21,12 @@ export const DOCUMENT_TYPE_LABELS: Record<string, string> = {
   tabletop_exercise: 'Tabletop Exercise',
 };
 
-export const ALL_DOCUMENT_TYPES: string[] = Object.keys(DOCUMENT_TYPE_LABELS);
+export const ALL_DOCUMENT_TYPES = Object.keys(
+  DOCUMENT_TYPE_LABELS,
+) as EvidenceFormType[];
 
-export function getDocumentTypeLabel(formType: string): string {
-  return DOCUMENT_TYPE_LABELS[formType] ?? formType;
+export function getDocumentTypeLabel(formType: EvidenceFormType | string): string {
+  return DOCUMENT_TYPE_LABELS[formType as EvidenceFormType] ?? formType;
 }
 
 export function toDocumentUrlSlug(formType: string): string {
diff --git a/apps/app/src/app/(app)/[orgId]/frameworks/page.tsx b/apps/app/src/app/(app)/[orgId]/frameworks/page.tsx
@@ -29,7 +29,11 @@ export default async function FrameworksPage({ params }: { params: Promise<{ org
 
   const availableToAdd = allFrameworks.filter(
     (framework) =>
-      !frameworksWithControls.some((fc) => fc.framework?.id === framework.id),
+      !frameworksWithControls.some(
+        (fc) =>
+          fc.framework?.id === framework.id ||
+          fc.customFramework?.id === framework.id,
+      ),
   );
 
   return (
diff --git a/apps/app/src/app/(app)/[orgId]/overview/components/FrameworksOverview.tsx b/apps/app/src/app/(app)/[orgId]/overview/components/FrameworksOverview.tsx
@@ -82,7 +82,12 @@ export function FrameworksOverview({
   );
 
   const availableFrameworksToAdd = allFrameworks.filter(
-    (framework) => !frameworksWithControls.some((fc) => fc.framework?.id === framework.id),
+    (framework) =>
+      !frameworksWithControls.some(
+        (fc) =>
+          fc.framework?.id === framework.id ||
+          fc.customFramework?.id === framework.id,
+      ),
   );
 
   return (
diff --git a/packages/db/prisma/migrations/20260417220000_tenant_consistent_custom_fks/migration.sql b/packages/db/prisma/migrations/20260417220000_tenant_consistent_custom_fks/migration.sql
@@ -0,0 +1,51 @@
+-- Enforce tenant consistency on CustomFramework references at the DB level:
+-- a CustomRequirement or FrameworkInstance that points at a CustomFramework
+-- must be in the same organization as that framework. Previously this was
+-- only enforced in application code; this migration upgrades the FKs to
+-- composite (id, organizationId) so Postgres rejects cross-tenant links.
+
+-- Defense-in-depth sanity check: fail the migration loudly if any existing
+-- rows violate the invariant, rather than silently letting the composite FK
+-- rewrite proceed against bad data.
+DO $$
+DECLARE
+  bad_req bigint;
+  bad_fi  bigint;
+BEGIN
+  SELECT count(*) INTO bad_req
+  FROM "CustomRequirement" cr
+  JOIN "CustomFramework" cf ON cf."id" = cr."customFrameworkId"
+  WHERE cr."organizationId" <> cf."organizationId";
+
+  SELECT count(*) INTO bad_fi
+  FROM "FrameworkInstance" fi
+  JOIN "CustomFramework" cf ON cf."id" = fi."customFrameworkId"
+  WHERE fi."organizationId" <> cf."organizationId";
+
+  IF bad_req > 0 OR bad_fi > 0 THEN
+    RAISE EXCEPTION
+      'tenant_consistent_custom_fks: % cross-tenant CustomRequirement rows and % cross-tenant FrameworkInstance rows must be fixed before this migration can run',
+      bad_req, bad_fi;
+  END IF;
+END $$;
+
+-- Unique constraint required for the composite FK target.
+ALTER TABLE "CustomFramework"
+    ADD CONSTRAINT "CustomFramework_id_organizationId_key"
+    UNIQUE ("id", "organizationId");
+
+-- CustomRequirement: replace the single-column FK with a composite FK.
+ALTER TABLE "CustomRequirement" DROP CONSTRAINT "CustomRequirement_customFrameworkId_fkey";
+ALTER TABLE "CustomRequirement"
+    ADD CONSTRAINT "CustomRequirement_customFrameworkId_organizationId_fkey"
+    FOREIGN KEY ("customFrameworkId", "organizationId")
+    REFERENCES "CustomFramework"("id", "organizationId")
+    ON DELETE CASCADE ON UPDATE CASCADE;
+
+-- FrameworkInstance: same treatment for its optional customFrameworkId.
+ALTER TABLE "FrameworkInstance" DROP CONSTRAINT "FrameworkInstance_customFrameworkId_fkey";
+ALTER TABLE "FrameworkInstance"
+    ADD CONSTRAINT "FrameworkInstance_customFrameworkId_organizationId_fkey"
+    FOREIGN KEY ("customFrameworkId", "organizationId")
+    REFERENCES "CustomFramework"("id", "organizationId")
+    ON DELETE CASCADE ON UPDATE CASCADE;
diff --git a/packages/db/prisma/schema/custom-framework.prisma b/packages/db/prisma/schema/custom-framework.prisma
@@ -13,6 +13,7 @@ model CustomFramework {
   createdAt DateTime @default(now())
   updatedAt DateTime @default(now()) @updatedAt
 
+  @@unique([id, organizationId])
   @@index([organizationId])
 }
 
@@ -25,7 +26,9 @@ model CustomRequirement {
   organizationId    String
   organization      Organization    @relation(fields: [organizationId], references: [id], onDelete: Cascade)
   customFrameworkId String
-  customFramework   CustomFramework @relation(fields: [customFrameworkId], references: [id], onDelete: Cascade)
+  // Composite FK onto (id, organizationId) so tenant consistency with the
+  // referenced CustomFramework is enforced at the DB level.
+  customFramework   CustomFramework @relation(fields: [customFrameworkId, organizationId], references: [id, organizationId], onDelete: Cascade)
 
   requirementMaps RequirementMap[]
 
diff --git a/packages/db/prisma/schema/framework.prisma b/packages/db/prisma/schema/framework.prisma
@@ -8,7 +8,9 @@ model FrameworkInstance {
   framework   FrameworkEditorFramework? @relation(fields: [frameworkId], references: [id], onDelete: Cascade)
 
   customFrameworkId String?
-  customFramework   CustomFramework? @relation(fields: [customFrameworkId], references: [id], onDelete: Cascade)
+  // Composite FK onto (id, organizationId) so an FI can only reference a
+  // CustomFramework in its own org. Enforced at the DB level.
+  customFramework   CustomFramework? @relation(fields: [customFrameworkId, organizationId], references: [id, organizationId], onDelete: Cascade)
 
   // Relationships
   organization       Organization     @relation(fields: [organizationId], references: [id], onDelete: Cascade)
