Skip to content

[dev] [carhartlewis] lewis/comp-framework-controls#2593

Open
github-actions[bot] wants to merge 11 commits intomainfrom
lewis/comp-framework-controls
Open

[dev] [carhartlewis] lewis/comp-framework-controls#2593
github-actions[bot] wants to merge 11 commits intomainfrom
lewis/comp-framework-controls

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions github-actions bot commented Apr 17, 2026

This is an automated pull request to merge lewis/comp-framework-controls into dev.
It was created by the [Auto Pull Request] action.


Summary by cubic

Adds per‑org custom frameworks and requirements with strict tenant checks, plus control linking for policies, tasks, requirements, and required evidence document types. Compliance scoring now honors required document types and uses evidence submission time; OpenAPI and UI updated end‑to‑end. Schema splits org‑custom data into CustomFramework/CustomRequirement with composite tenant FKs.

  • New Features

    • API: POST /v1/frameworks/custom, POST /v1/frameworks/:id/requirements, POST /v1/frameworks/:id/requirements/link, POST /v1/frameworks/:id/requirements/:requirementKey/controls/link; POST /v1/controls/:id/policies/link, POST /v1/controls/:id/tasks/link, POST /v1/controls/:id/requirements/link, POST /v1/controls/:id/document-types/link, and GET /v1/controls/options; available frameworks include org‑custom; enum path params validated.
    • UI: sheets to create a custom framework, add/link requirements, link existing controls to a requirement, and link policies/tasks/requirements/document types to a control; new control detail page with Policies/Tasks/Documents tabs and submission counts; identifier column + search; long titles/descriptions truncate; actions disable while unlinking.
    • Data/Scoring: control create/link endpoints validate org ownership for all IDs; policies/tasks/frameworks endpoints normalize platform + custom frameworks; compliance scoring uses required document types and evidence submittedAt; link endpoints return DB counts; dedupes policyIds/taskIds before validation to prevent false rejections.
  • Migration

    • Split per‑org data into CustomFramework and CustomRequirement; FrameworkInstance and RequirementMap reference platform or custom (DB CHECK ensures exactly one).
    • Add composite FKs on custom refs to enforce same‑org tenancy at the DB layer; run Prisma migrations to apply schema and indexes.

Written for commit 5de8a67. Summary will update on new commits.

…points

- Implemented new API endpoints for creating custom frameworks and requirements in the FrameworksController.
- Added DTOs for CreateCustomFrameworkDto, CreateCustomRequirementDto, LinkRequirementsDto, and LinkControlsDto.
- Enhanced findAvailable method to filter frameworks by organization ID.
- Introduced client components for adding and linking requirements, including AddCustomRequirementSheet and LinkRequirementSheet.
- Updated FrameworkOverview to include new actions for managing custom requirements.
- Added UI components for creating custom frameworks and linking existing controls.

This update enhances the framework management capabilities, allowing users to create and manage custom frameworks and their associated requirements effectively.
@vercel
Copy link
Copy Markdown

vercel bot commented Apr 17, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
app Ready Ready Preview, Comment Apr 17, 2026 10:31pm
comp-framework-editor Ready Ready Preview, Comment Apr 17, 2026 10:31pm
portal Ready Ready Preview, Comment Apr 17, 2026 10:31pm

Request Review

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Apr 17, 2026

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ carhartlewis
❌ Lewis Carhart


Lewis Carhart seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@mintlify
Copy link
Copy Markdown
Contributor

mintlify bot commented Apr 17, 2026

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
CompAI 🟢 Ready View Preview Apr 17, 2026, 8:18 PM

💡 Tip: Enable Workflows to automatically generate PRs for you.

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4 issues found across 25 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/api/src/frameworks/frameworks.service.ts">

<violation number="1" location="apps/api/src/frameworks/frameworks.service.ts:174">
P1: Multi-tenancy gap: the requirement lookup doesn't scope by `organizationId`, so on a shared global framework, one org could link controls to another org's custom requirement. Add an `organizationId` filter matching the pattern used in `linkRequirements`.</violation>

<violation number="2" location="apps/api/src/frameworks/frameworks.service.ts:260">
P2: Calling `linkRequirements` multiple times with the same IDs will create duplicate requirement records each time, since there's no unique constraint and no deduplication check. Consider checking for existing requirements with the same `identifier` and `frameworkId` before inserting, or adding a uniqueness constraint.</violation>
</file>

<file name="apps/api/src/frameworks/dto/create-custom-requirement.dto.ts">

<violation number="1" location="apps/api/src/frameworks/dto/create-custom-requirement.dto.ts:13">
P2: `identifier` accepts an empty string. Add `@MinLength(1)` to match the validation on `name` — an empty identifier is almost certainly invalid.</violation>
</file>

<file name="apps/app/src/app/(app)/[orgId]/frameworks/[frameworkInstanceId]/components/LinkRequirementSheet.tsx">

<violation number="1" location="apps/app/src/app/(app)/[orgId]/frameworks/[frameworkInstanceId]/components/LinkRequirementSheet.tsx:49">
P2: The SWR loading state is not handled. When the sheet opens and data is fetching, `options` defaults to `[]`, so users briefly see "No requirements available" before the real data arrives. Destructure `isLoading` from `useSWR` and show a loading indicator instead.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.

Comment thread apps/api/src/frameworks/frameworks.service.ts Outdated
Comment thread apps/api/src/frameworks/frameworks.service.ts Outdated
Comment thread apps/api/src/frameworks/dto/create-custom-requirement.dto.ts
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@vercel vercel bot temporarily deployed to Preview – portal April 17, 2026 20:26 Inactive
@vercel vercel bot temporarily deployed to Preview – portal April 17, 2026 20:28 Inactive
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 issues found across 1 file (changes from recent commits).

Requires human review: Auto-approval blocked by 4 unresolved issues from previous reviews.

@Marfuen
Copy link
Copy Markdown
Contributor

Marfuen commented Apr 17, 2026

Code review

Scoping FrameworkEditorFramework / FrameworkEditorRequirement by organizationId is a reasonable way to add org-custom frameworks. One architectural note first, then two concrete bugs.

Architectural note (not blocking): The rest of this codebase treats FrameworkEditor* as pure platform-wide definitions and has a separate per-org instance tier (FrameworkEditorControlTemplateControl, FrameworkEditorPolicyTemplatePolicy, FrameworkEditorTaskTemplateTask, EvidenceSubmission). This PR mixes per-org rows into the definition tables for Framework and Requirement only, so the two tiers now have divergent patterns. That inconsistency is the root cause of bug #1 below — read paths that previously assumed "definition rows are global" now silently cross tenants.

Found 2 issues:

  1. Cross-tenant requirement leak in findOne and findRequirement — both query frameworkEditorRequirement by frameworkId only, with no organizationId filter. Platform framework frameworkIds (e.g. SOC 2) are shared across orgs. Once Org A calls createRequirement or linkRequirements against their SOC 2 instance, those rows (organizationId = orgA) are attached to the same frameworkId, and Org B's GET /v1/frameworks/:id + GET /v1/frameworks/:id/requirements/:key will return them. Same OR: [{ organizationId: null }, { organizationId }] scope that's used correctly in linkRequirements/linkControlsToRequirement is missing here.

evidenceSubmissions,
] = await Promise.all([
db.frameworkEditorRequirement.findMany({
where: { frameworkId: fi.frameworkId },
orderBy: { name: 'asc' },
}),
db.task.findMany({
where: { organizationId, controls: { some: { organizationId } } },
include: { controls: true },
}),

const [allReqDefs, relatedControls, tasks] = await Promise.all([
db.frameworkEditorRequirement.findMany({
where: { frameworkId: fi.frameworkId },
}),
db.requirementMap.findMany({
where: { frameworkInstanceId, requirementId: requirementKey },
include: {

  1. linkRequirements dedup bypass against existing platform requirements — the existing lookup filters organizationId: <requestingOrg>, so a platform row (organizationId: null) already attached to the framework with the same identifier is not caught, and a duplicate org-scoped row gets created. The migration only adds non-unique indexes, so the DB won't stop it either. Scope should mirror sources: OR: [{ organizationId: null }, { organizationId }], and ideally a unique constraint on (frameworkId, organizationId, identifier) should back it up (also protects createRequirement, which has no dedup at all).

}
const existing = await db.frameworkEditorRequirement.findMany({
where: {
frameworkId: fi.frameworkId,
organizationId,
identifier: { in: sources.map((r) => r.identifier) },
},
select: { identifier: true },
});
const existingIdentifiers = new Set(existing.map((r) => r.identifier));
const toCreate = sources.filter(
(r) => !existingIdentifiers.has(r.identifier),

https://github.com/trycompai/comp/blob/c3a50dbd05278ab88ebc896d05d7de0ddf836f9a/packages/db/prisma/migrations/20260417200000_custom_frameworks_requirements/migration.sql#L1-L21

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

…ts to controls

- Implemented new API endpoints in ControlsController for linking existing policies, tasks, and requirements to a control.
- Added corresponding DTOs: LinkPoliciesDto, LinkTasksDto, and LinkRequirementsToControlDto.
- Enhanced ControlsService with methods to handle linking logic and validation.
- Updated UI components to support linking actions, including LinkPolicySheet, LinkTaskSheet, and LinkRequirementForControlSheet.
- Improved data fetching with useControlOptions hook for better management of linked items.

This update enhances the control management capabilities, allowing users to effectively link existing resources to controls.
@vercel vercel bot temporarily deployed to Preview – portal April 17, 2026 21:10 Inactive
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 19 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/app/src/app/(app)/[orgId]/frameworks/[frameworkInstanceId]/requirements/[requirementKey]/page.tsx">

<violation number="1" location="apps/app/src/app/(app)/[orgId]/frameworks/[frameworkInstanceId]/requirements/[requirementKey]/page.tsx:32">
P2: Remove this debug `console.log` block from the server-rendered page; it logs internal API error payloads on every request and can leak unnecessary details to production logs.</violation>
</file>

<file name="apps/api/src/controls/controls.service.ts">

<violation number="1" location="apps/api/src/controls/controls.service.ts:297">
P2: `linkRequirements` can return an incorrect `count` because it reports `validMappings.length` even when `createMany(..., skipDuplicates: true)` skips duplicates.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.

Comment thread apps/api/src/controls/controls.service.ts Outdated
…tables

Previously the branch added a nullable organizationId column to the platform
FrameworkEditorFramework / FrameworkEditorRequirement tables so a single row
was either platform (null) or org-custom (set). That hybrid shape mismatched
the template/instance pattern used everywhere else in the codebase and caused
two cross-tenant reads (frameworks.service.findOne, findRequirement) to leak
one org's custom requirements to another org on the same framework.

Move the org-custom data into dedicated CustomFramework / CustomRequirement
tables. FrameworkInstance.frameworkId and RequirementMap.requirementId become
nullable and gain customFrameworkId / customRequirementId siblings with DB
CHECK constraints enforcing exactly one of the two is set. The editor tables
are pure platform definitions again, so the leaks vanish structurally (no
shared table to filter) rather than relying on filter discipline in each read.

- Schema: revert organizationId from FrameworkEditor tables; add
  CustomFramework + CustomRequirement; relax/branch FrameworkInstance and
  RequirementMap FKs with CHECK constraints
- Migration: move existing per-org rows into the new tables, repoint FKs,
  drop the old columns
- API: rewrite FrameworksService findOne / findRequirement / createCustom /
  createRequirement / linkRequirements / linkControlsToRequirement /
  findAvailable to branch on platform vs custom. Update ControlsService
  create + linkRequirements + DTOs to accept customRequirementId (with
  exactly-one validation) and persist documentTypes in the same transaction
- Frontend: plumb isCustom through the requirement/control sheets, widen
  FrameworkInstanceWithControls / FrameworkInstanceForTasks to surface
  customFramework, and fix all fw.framework.* null-unsafe reads
- Tests: frameworks.service.spec regression coverage that a custom FI never
  reads from frameworkEditorRequirement and a platform FI never reads from
  customRequirement

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Drop the per-request console.log in the requirement page that was dumping
  internal API status/error payloads into server logs
- Return result.count from createMany in linkRequirements,
  linkControlsToRequirement, and linkDocumentTypes so skipDuplicates-skipped
  rows no longer inflate the reported count

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

11 issues found across 40 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/app/src/app/(app)/[orgId]/frameworks/[frameworkInstanceId]/controls/[controlId]/components/DocumentsTable.tsx">

<violation number="1" location="apps/app/src/app/(app)/[orgId]/frameworks/[frameworkInstanceId]/controls/[controlId]/components/DocumentsTable.tsx:116">
P3: Disable all unlink buttons while a request is pending so users don’t click enabled controls that are blocked by `if (pending) return`.</violation>
</file>

<file name="apps/app/src/app/(app)/[orgId]/controls/hooks/useControls.ts">

<violation number="1" location="apps/app/src/app/(app)/[orgId]/controls/hooks/useControls.ts:18">
P2: `requirementMappings` now allows invalid combinations of requirement IDs, but the backend requires exactly one of `requirementId` or `customRequirementId` per mapping.</violation>
</file>

<file name="apps/api/src/controls/controls.controller.ts">

<violation number="1" location="apps/api/src/controls/controls.controller.ts:162">
P2: Validate `formType` with an enum pipe on the route param; the current signature does not enforce runtime enum validation.</violation>
</file>

<file name="apps/app/src/app/(app)/[orgId]/frameworks/page.tsx">

<violation number="1" location="apps/app/src/app/(app)/[orgId]/frameworks/page.tsx:32">
P2: This filter only excludes platform frameworks and misses custom framework instances. Already-enabled custom frameworks can still appear as addable.</violation>
</file>

<file name="apps/api/src/policies/policies.controller.ts">

<violation number="1" location="apps/api/src/policies/policies.controller.ts:234">
P2: Filtering to `fi.framework` here causes policy regeneration to silently ignore org custom frameworks. Include `customFramework` instances as well so the AI context reflects all selected frameworks.</violation>
</file>

<file name="apps/app/src/app/(app)/[orgId]/frameworks/[frameworkInstanceId]/controls/[controlId]/components/documentTypeLabels.ts">

<violation number="1" location="apps/app/src/app/(app)/[orgId]/frameworks/[frameworkInstanceId]/controls/[controlId]/components/documentTypeLabels.ts:5">
P2: `DOCUMENT_TYPE_LABELS` is typed too loosely (`Record<string, string>`), so this hardcoded list can drift from the canonical evidence-form type set without compile-time errors.</violation>
</file>

<file name="apps/app/src/app/(app)/[orgId]/overview/components/FrameworksOverview.tsx">

<violation number="1" location="apps/app/src/app/(app)/[orgId]/overview/components/FrameworksOverview.tsx:85">
P2: Also check `customFramework?.id` when filtering available frameworks; otherwise already-added custom frameworks are treated as still available.</violation>
</file>

<file name="apps/api/src/controls/controls.service.ts">

<violation number="1" location="apps/api/src/controls/controls.service.ts:249">
P1: Validate policy/task ownership by `organizationId` before connecting in `create()`, otherwise a control can be linked to another organization’s records if their IDs are provided.</violation>

<violation number="2" location="apps/api/src/controls/controls.service.ts:262">
P1: `create()` should enforce the same org/framework validation as `linkRequirements()` before writing `requirementMap` rows; otherwise invalid or cross-tenant requirement links can be persisted.</violation>
</file>

<file name="packages/db/prisma/schema/custom-framework.prisma">

<violation number="1" location="packages/db/prisma/schema/custom-framework.prisma:28">
P1: `CustomRequirement` does not enforce tenant consistency between `organizationId` and `customFrameworkId`, allowing cross-org requirement-to-framework links.</violation>
</file>

<file name="apps/api/src/frameworks/frameworks-scores.helper.ts">

<violation number="1" location="apps/api/src/frameworks/frameworks-scores.helper.ts:227">
P1: Use submission timestamp (submittedAt) for evidence recency checks; using createdAt can produce incorrect compliance completion.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.

Comment thread apps/api/src/controls/controls.service.ts
Comment thread apps/api/src/controls/controls.service.ts Outdated
Comment thread packages/db/prisma/schema/custom-framework.prisma Outdated
Comment thread apps/api/src/frameworks/frameworks-scores.helper.ts Outdated
Comment thread apps/app/src/app/(app)/[orgId]/controls/hooks/useControls.ts Outdated
Comment thread apps/app/src/app/(app)/[orgId]/frameworks/page.tsx Outdated
Comment thread apps/api/src/policies/policies.controller.ts Outdated
Comment thread apps/app/src/app/(app)/[orgId]/overview/components/FrameworksOverview.tsx Outdated
controls.service.create() accepted policyIds, taskIds, and requirementMappings
(frameworkInstanceId, requirementId, customRequirementId) without verifying
they belong to the caller's organization. Prisma FKs only check existence,
not tenancy, so a user with control:create in org A could submit a
frameworkInstanceId from org B — the resulting RequirementMap would join the
attacker's control to the victim's framework, persistently rendering attacker
content inside org B's compliance UI via FrameworksService.findOne /
findRequirement includes.

Validate every FK against organizationId before the transaction, mirroring
the logic already present in linkRequirements:

- policies / tasks: findMany filtered by { id in, organizationId } and
  reject if any are missing
- frameworkInstance: must belong to caller org
- requirementId: platform requirement's frameworkId must match the FI's
  frameworkId
- customRequirementId: custom requirement must be in caller org and its
  customFrameworkId must match the FI's customFrameworkId

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…om-framework parity

- 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<EvidenceFormType, string> 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) <noreply@anthropic.com>
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 1 file (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/api/src/controls/controls.service.ts">

<violation number="1" location="apps/api/src/controls/controls.service.ts:306">
P2: Duplicate `policyIds` are incorrectly treated as invalid because validation compares against the non-deduplicated input length.</violation>

<violation number="2" location="apps/api/src/controls/controls.service.ts:321">
P2: Duplicate `taskIds` are incorrectly rejected due to comparing fetched row count with the non-deduplicated input length.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.

Comment thread apps/api/src/controls/controls.service.ts Outdated
Comment thread apps/api/src/controls/controls.service.ts Outdated
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 issues found across 13 files (changes from recent commits).

Requires human review: Auto-approval blocked by 11 unresolved issues from previous reviews.

validatePolicyIds / validateTaskIds compared findMany row count to the raw
input length. Duplicate ids in the request (e.g. [p1, p1, p2]) made the
input longer than the set of rows returned, so a legitimate request with
repeated ids was rejected as invalid. Dedupe via Set before the findMany
lookup and length check.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@vercel vercel bot temporarily deployed to Preview – portal April 17, 2026 22:28 Inactive
@vercel vercel bot temporarily deployed to Preview – app April 17, 2026 22:28 Inactive
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 issues found across 1 file (changes from recent commits).

Requires human review: Auto-approval blocked by 1 unresolved issue from previous reviews.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants