diff --git a/apps/api/src/frameworks/dto/update-custom-framework.dto.spec.ts b/apps/api/src/frameworks/dto/update-custom-framework.dto.spec.ts new file mode 100644 index 0000000000..17d3e9b039 --- /dev/null +++ b/apps/api/src/frameworks/dto/update-custom-framework.dto.spec.ts @@ -0,0 +1,67 @@ +import { plainToInstance } from 'class-transformer'; +import { validate, type ValidationError } from 'class-validator'; +import { UpdateCustomFrameworkDto } from './update-custom-framework.dto'; + +function propsWithErrors(errors: ValidationError[]): string[] { + return errors.map((e) => e.property); +} + +async function validatePayload(payload: Record) { + const dto = plainToInstance(UpdateCustomFrameworkDto, payload); + return validate(dto, { whitelist: true, forbidNonWhitelisted: true }); +} + +function transform(payload: Record) { + return plainToInstance(UpdateCustomFrameworkDto, payload); +} + +describe('UpdateCustomFrameworkDto', () => { + it('accepts a name-only payload', async () => { + expect(await validatePayload({ name: 'Internal Controls' })).toHaveLength(0); + }); + + it('accepts a description-only payload', async () => { + expect(await validatePayload({ description: 'Covers X' })).toHaveLength(0); + }); + + it('accepts an empty payload (field-level; empty PATCH is rejected in the service)', async () => { + expect(await validatePayload({})).toHaveLength(0); + }); + + it('rejects an explicit null name', async () => { + const errors = await validatePayload({ name: null }); + expect(propsWithErrors(errors)).toContain('name'); + }); + + it('rejects an explicit null description', async () => { + const errors = await validatePayload({ description: null }); + expect(propsWithErrors(errors)).toContain('description'); + }); + + it('rejects a non-string name', async () => { + const errors = await validatePayload({ name: 42 }); + expect(propsWithErrors(errors)).toContain('name'); + }); + + it('rejects an empty-string name (MinLength)', async () => { + const errors = await validatePayload({ name: '' }); + expect(propsWithErrors(errors)).toContain('name'); + }); + + it('rejects a whitespace-only name (trimmed to empty)', async () => { + const errors = await validatePayload({ name: ' ' }); + expect(propsWithErrors(errors)).toContain('name'); + }); + + it('trims surrounding whitespace from a valid name', async () => { + const payload = { name: ' Internal Controls ' }; + expect(await validatePayload(payload)).toHaveLength(0); + expect(transform(payload).name).toBe('Internal Controls'); + }); + + it('trims the description', async () => { + expect(transform({ description: ' Covers X ' }).description).toBe( + 'Covers X', + ); + }); +}); diff --git a/apps/api/src/frameworks/dto/update-custom-framework.dto.ts b/apps/api/src/frameworks/dto/update-custom-framework.dto.ts new file mode 100644 index 0000000000..13d2bd708e --- /dev/null +++ b/apps/api/src/frameworks/dto/update-custom-framework.dto.ts @@ -0,0 +1,30 @@ +import { IsString, MaxLength, MinLength, ValidateIf } from 'class-validator'; +import { Transform } from 'class-transformer'; +import { ApiPropertyOptional } from '@nestjs/swagger'; + +// Trim strings so a whitespace-only value collapses to '' and is rejected by +// MinLength. Guard on typeof so null/non-strings pass through unchanged and are +// still rejected by @IsString (using value?.trim() would turn null into +// undefined, which ValidateIf would then treat as an omitted field). +const trimIfString = ({ value }: { value: unknown }) => + typeof value === 'string' ? value.trim() : value; + +export class UpdateCustomFrameworkDto { + // ValidateIf (rather than @IsOptional) only skips validation when the field is + // omitted (undefined). An explicit `null` still runs @IsString and is rejected + // with a 400, instead of slipping through to a non-null DB column. + @ApiPropertyOptional({ description: 'Framework name', example: 'Internal Controls' }) + @ValidateIf((_, value) => value !== undefined) + @Transform(trimIfString) + @IsString() + @MinLength(1) + @MaxLength(120) + name?: string; + + @ApiPropertyOptional({ description: 'Framework description' }) + @ValidateIf((_, value) => value !== undefined) + @Transform(trimIfString) + @IsString() + @MaxLength(2000) + description?: string; +} diff --git a/apps/api/src/frameworks/frameworks.controller.spec.ts b/apps/api/src/frameworks/frameworks.controller.spec.ts index 970e46be5c..eb67a5b5ca 100644 --- a/apps/api/src/frameworks/frameworks.controller.spec.ts +++ b/apps/api/src/frameworks/frameworks.controller.spec.ts @@ -41,6 +41,7 @@ describe('FrameworksController', () => { const mockService = { findAll: jest.fn(), findAvailable: jest.fn(), + updateCustom: jest.fn(), delete: jest.fn(), getUpdateStatus: jest.fn(), getUpdatePreview: jest.fn(), @@ -158,6 +159,19 @@ describe('FrameworksController', () => { }); }); + describe('updateCustom', () => { + it('should delegate to service and return the updated framework', async () => { + const updated = { id: 'cfrm_A', name: 'CSC/CPRT' }; + mockService.updateCustom.mockResolvedValue(updated); + + const dto = { name: 'CSC/CPRT', description: 'Renamed' }; + const result = await controller.updateCustom('org_1', 'fi1', dto); + + expect(result).toEqual(updated); + expect(service.updateCustom).toHaveBeenCalledWith('fi1', 'org_1', dto); + }); + }); + describe('getUpdateStatus', () => { it('should return update status with { data }', async () => { const mockStatus = { diff --git a/apps/api/src/frameworks/frameworks.controller.ts b/apps/api/src/frameworks/frameworks.controller.ts index 68a347fa1a..31b309c4d2 100644 --- a/apps/api/src/frameworks/frameworks.controller.ts +++ b/apps/api/src/frameworks/frameworks.controller.ts @@ -5,6 +5,7 @@ import { Delete, Get, Param, + Patch, Post, Query, UseGuards, @@ -12,6 +13,7 @@ import { import { ApiTags, ApiBearerAuth, + ApiBody, ApiOperation, ApiQuery, } from '@nestjs/swagger'; @@ -28,6 +30,7 @@ import type { AuthContext as AuthContextType } from '../auth/types'; import { FrameworksService } from './frameworks.service'; import { AddFrameworksDto } from './dto/add-frameworks.dto'; import { CreateCustomFrameworkDto } from './dto/create-custom-framework.dto'; +import { UpdateCustomFrameworkDto } from './dto/update-custom-framework.dto'; import { CreateCustomRequirementDto } from './dto/create-custom-requirement.dto'; import { LinkRequirementsDto } from './dto/link-requirements.dto'; import { LinkControlsDto } from './dto/link-controls.dto'; @@ -144,6 +147,22 @@ export class FrameworksController { return this.frameworksService.createCustom(organizationId, dto); } + @Patch(':id/custom') + @RequirePermission('framework', 'update') + @ApiOperation({ + summary: 'Update a custom framework', + description: + "Update the name and/or description of an organization's custom framework. Only custom frameworks are editable; platform frameworks return 400.", + }) + @ApiBody({ type: UpdateCustomFrameworkDto }) + async updateCustom( + @OrganizationId() organizationId: string, + @Param('id') id: string, + @Body() dto: UpdateCustomFrameworkDto, + ) { + return this.frameworksService.updateCustom(id, organizationId, dto); + } + @Post(':id/requirements') @RequirePermission('framework', 'update') @ApiOperation({ summary: 'Add a custom requirement to a framework instance' }) diff --git a/apps/api/src/frameworks/frameworks.service.spec.ts b/apps/api/src/frameworks/frameworks.service.spec.ts index dad4f7b141..43cc77d236 100644 --- a/apps/api/src/frameworks/frameworks.service.spec.ts +++ b/apps/api/src/frameworks/frameworks.service.spec.ts @@ -1,5 +1,5 @@ import { Test, TestingModule } from '@nestjs/testing'; -import { NotFoundException } from '@nestjs/common'; +import { BadRequestException, NotFoundException } from '@nestjs/common'; import { FrameworksService } from './frameworks.service'; import { TimelinesService } from '../timelines/timelines.service'; @@ -35,6 +35,7 @@ jest.mock('@db', () => ({ }, customFramework: { findMany: jest.fn(), + update: jest.fn(), }, }, // The frameworks-timeline helper imports FindingType (a Prisma enum) at module @@ -135,6 +136,79 @@ describe('FrameworksService', () => { }); }); + describe('updateCustom', () => { + it('should update the custom framework name and description', async () => { + (mockDb.frameworkInstance.findUnique as jest.Mock).mockResolvedValue({ + customFrameworkId: 'cfrm_A', + }); + const updated = { + id: 'cfrm_A', + name: 'CSC/CPRT', + description: 'Renamed', + }; + (mockDb.customFramework.update as jest.Mock).mockResolvedValue(updated); + + const result = await service.updateCustom('fi1', 'org_1', { + name: 'CSC/CPRT', + description: 'Renamed', + }); + + expect(result).toEqual(updated); + expect(mockDb.frameworkInstance.findUnique).toHaveBeenCalledWith({ + where: { id: 'fi1', organizationId: 'org_1' }, + select: { customFrameworkId: true }, + }); + expect(mockDb.customFramework.update).toHaveBeenCalledWith({ + where: { id: 'cfrm_A' }, + data: { name: 'CSC/CPRT', description: 'Renamed' }, + }); + }); + + it('should only update the fields that are provided', async () => { + (mockDb.frameworkInstance.findUnique as jest.Mock).mockResolvedValue({ + customFrameworkId: 'cfrm_A', + }); + (mockDb.customFramework.update as jest.Mock).mockResolvedValue({}); + + await service.updateCustom('fi1', 'org_1', { name: 'Just the name' }); + + expect(mockDb.customFramework.update).toHaveBeenCalledWith({ + where: { id: 'cfrm_A' }, + data: { name: 'Just the name' }, + }); + }); + + it('should throw BadRequestException when no fields are provided', async () => { + await expect( + service.updateCustom('fi1', 'org_1', {}), + ).rejects.toThrow(BadRequestException); + expect(mockDb.frameworkInstance.findUnique).not.toHaveBeenCalled(); + expect(mockDb.customFramework.update).not.toHaveBeenCalled(); + }); + + it('should throw NotFoundException when instance not found', async () => { + (mockDb.frameworkInstance.findUnique as jest.Mock).mockResolvedValue( + null, + ); + + await expect( + service.updateCustom('missing', 'org_1', { name: 'x' }), + ).rejects.toThrow(NotFoundException); + expect(mockDb.customFramework.update).not.toHaveBeenCalled(); + }); + + it('should throw BadRequestException for a platform framework', async () => { + (mockDb.frameworkInstance.findUnique as jest.Mock).mockResolvedValue({ + customFrameworkId: null, + }); + + await expect( + service.updateCustom('fi_platform', 'org_1', { name: 'x' }), + ).rejects.toThrow(BadRequestException); + expect(mockDb.customFramework.update).not.toHaveBeenCalled(); + }); + }); + describe('getScores', () => { it('should call getOverviewScores and getCurrentMember when userId is provided', async () => { const mockScores = { policies: 10, tasks: 5 }; diff --git a/apps/api/src/frameworks/frameworks.service.ts b/apps/api/src/frameworks/frameworks.service.ts index 7ee1d65b7f..f3bc7c8d74 100644 --- a/apps/api/src/frameworks/frameworks.service.ts +++ b/apps/api/src/frameworks/frameworks.service.ts @@ -465,6 +465,43 @@ export class FrameworksService { }); } + async updateCustom( + frameworkInstanceId: string, + organizationId: string, + input: { name?: string; description?: string }, + ) { + const data: { name?: string; description?: string } = {}; + if (input.name !== undefined) data.name = input.name; + if (input.description !== undefined) data.description = input.description; + + // Reject an empty PATCH up front instead of issuing a no-op write. + if (Object.keys(data).length === 0) { + throw new BadRequestException('No fields to update'); + } + + const frameworkInstance = await db.frameworkInstance.findUnique({ + where: { id: frameworkInstanceId, organizationId }, + select: { customFrameworkId: true }, + }); + + if (!frameworkInstance) { + throw new NotFoundException('Framework instance not found'); + } + + // Only org-authored custom frameworks carry editable metadata. Platform + // frameworks derive their name/description from the shared global template. + if (!frameworkInstance.customFrameworkId) { + throw new BadRequestException('Only custom frameworks can be edited'); + } + + // Ownership is already enforced by the org-scoped instance lookup above, + // so keying the update by the custom framework id is safe. + return db.customFramework.update({ + where: { id: frameworkInstance.customFrameworkId }, + data, + }); + } + async createRequirement( frameworkInstanceId: string, organizationId: string, diff --git a/apps/api/src/integration-platform/services/basic-auth-credential-fields.spec.ts b/apps/api/src/integration-platform/services/basic-auth-credential-fields.spec.ts new file mode 100644 index 0000000000..d3794c2c40 --- /dev/null +++ b/apps/api/src/integration-platform/services/basic-auth-credential-fields.spec.ts @@ -0,0 +1,39 @@ +import type { BasicAuthConfig } from '@trycompai/integration-platform'; +import { buildBasicAuthCredentialFields } from './basic-auth-credential-fields'; + +describe('buildBasicAuthCredentialFields', () => { + it('maps usernameField/passwordField to labeled fields (Fivetran api_key/api_secret)', () => { + const config: BasicAuthConfig = { + usernameField: 'api_key', + passwordField: 'api_secret', + }; + + const fields = buildBasicAuthCredentialFields(config); + + // Ids must equal the config field names so the runtime finds them when + // building the Basic auth header; labels must read as API Key / API Secret. + expect(fields).toEqual([ + { + id: 'api_key', + label: 'API Key', + type: 'text', + required: true, + placeholder: 'Enter API Key', + }, + { + id: 'api_secret', + label: 'API Secret', + type: 'password', + required: true, + placeholder: 'Enter API Secret', + }, + ]); + }); + + it('falls back to generic username/password when field names are unset', () => { + const fields = buildBasicAuthCredentialFields({} as BasicAuthConfig); + + expect(fields.map((f) => f.id)).toEqual(['username', 'password']); + expect(fields.map((f) => f.label)).toEqual(['Username', 'Password']); + }); +}); diff --git a/apps/api/src/integration-platform/services/basic-auth-credential-fields.ts b/apps/api/src/integration-platform/services/basic-auth-credential-fields.ts new file mode 100644 index 0000000000..d68226ca5f --- /dev/null +++ b/apps/api/src/integration-platform/services/basic-auth-credential-fields.ts @@ -0,0 +1,58 @@ +import type { + BasicAuthConfig, + CredentialField, +} from '@trycompai/integration-platform'; + +/** + * Turn a credential field name into a human-readable label. + * e.g. "api_key" → "API Key", "api_secret" → "API Secret", "username" → "Username". + */ +function humanizeFieldName(fieldName: string): string { + return fieldName + .split(/[_\s]+/) + .filter(Boolean) + .map((word) => + word.toLowerCase() === 'api' + ? 'API' + : word.charAt(0).toUpperCase() + word.slice(1), + ) + .join(' '); +} + +/** + * Build the credential form fields for a Basic-auth integration from its auth + * config. + * + * The field ids MUST equal the config's `usernameField`/`passwordField` so the + * values the customer enters are stored under the same keys the runtime reads + * when it builds the `Authorization: Basic` header (see runtime/check-context.ts). + * Fivetran, for example, maps Basic auth to `api_key`/`api_secret` — without + * these synthesized fields the connect form falls back to generic + * `username`/`password` ids, the runtime looks up the never-set real field names + * and sends `Basic base64(":")`, and every check fails with a 401. + */ +export function buildBasicAuthCredentialFields( + config: BasicAuthConfig, +): CredentialField[] { + const usernameField = config.usernameField || 'username'; + const passwordField = config.passwordField || 'password'; + const usernameLabel = humanizeFieldName(usernameField); + const passwordLabel = humanizeFieldName(passwordField); + + return [ + { + id: usernameField, + label: usernameLabel, + type: 'text', + required: true, + placeholder: `Enter ${usernameLabel}`, + }, + { + id: passwordField, + label: passwordLabel, + type: 'password', + required: true, + placeholder: `Enter ${passwordLabel}`, + }, + ]; +} diff --git a/apps/api/src/integration-platform/services/dynamic-manifest-loader.service.ts b/apps/api/src/integration-platform/services/dynamic-manifest-loader.service.ts index 9bdb30f449..1f98da6da6 100644 --- a/apps/api/src/integration-platform/services/dynamic-manifest-loader.service.ts +++ b/apps/api/src/integration-platform/services/dynamic-manifest-loader.service.ts @@ -20,6 +20,7 @@ import { DynamicIntegrationRepository, type DynamicIntegrationWithChecks, } from '../repositories/dynamic-integration.repository'; +import { buildBasicAuthCredentialFields } from './basic-auth-credential-fields'; import type { DynamicCheck } from '@db'; @Injectable() @@ -153,6 +154,15 @@ export class DynamicManifestLoaderService > | null; const syncVariables = syncDef?.variables as CheckVariable[] | undefined; + // Basic-auth integrations declare their credential field names (e.g. Fivetran + // maps Basic auth to api_key/api_secret) but ship no credentialFields. Synthesize + // them so the connect form labels the inputs correctly and — critically — stores + // the values under the same keys the runtime reads to build the Basic header. + const credentialFields = + auth.type === 'basic' + ? buildBasicAuthCredentialFields(auth.config) + : undefined; + return { id: integration.slug, name: integration.name, @@ -164,6 +174,7 @@ export class DynamicManifestLoaderService baseUrl: integration.baseUrl ?? undefined, defaultHeaders: (integration.defaultHeaders as Record) ?? undefined, + credentialFields, capabilities: (integration.capabilities as unknown as IntegrationCapability[]) ?? [ 'checks', diff --git a/apps/app/src/app/(app)/[orgId]/frameworks/[frameworkInstanceId]/components/EditCustomFrameworkSheet.test.tsx b/apps/app/src/app/(app)/[orgId]/frameworks/[frameworkInstanceId]/components/EditCustomFrameworkSheet.test.tsx new file mode 100644 index 0000000000..e1bfb8b030 --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/frameworks/[frameworkInstanceId]/components/EditCustomFrameworkSheet.test.tsx @@ -0,0 +1,110 @@ +import { render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { + setMockPermissions, + ADMIN_PERMISSIONS, + AUDITOR_PERMISSIONS, + mockHasPermission, +} from '@/test-utils/mocks/permissions'; + +vi.mock('@/hooks/use-permissions', () => ({ + usePermissions: () => ({ + permissions: {}, + hasPermission: mockHasPermission, + }), +})); + +const updateCustomFramework = vi.fn(); +vi.mock('@/hooks/use-frameworks', () => ({ + useFrameworks: () => ({ updateCustomFramework }), +})); + +vi.mock('sonner', () => ({ + toast: { success: vi.fn(), error: vi.fn() }, +})); + +import { EditCustomFrameworkSheet } from './EditCustomFrameworkSheet'; + +const frameworkInstance = { + id: 'frm_1', + organizationId: 'org_123', + customFrameworkId: 'cfrm_1', + customFramework: { + id: 'cfrm_1', + name: 'CMMC', + description: 'Original description', + }, + controls: [], +} as any; + +function renderSheet(overrides: Partial> = {}) { + return render( + , + ); +} + +describe('EditCustomFrameworkSheet', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('renders nothing when the user lacks framework:update', () => { + setMockPermissions(AUDITOR_PERMISSIONS); + renderSheet(); + expect(screen.queryByText('Edit Custom Framework')).not.toBeInTheDocument(); + }); + + it('pre-fills the form with the current name and description', () => { + setMockPermissions(ADMIN_PERMISSIONS); + renderSheet(); + expect(screen.getByText('Edit Custom Framework')).toBeInTheDocument(); + expect(screen.getByDisplayValue('CMMC')).toBeInTheDocument(); + expect( + screen.getByDisplayValue('Original description'), + ).toBeInTheDocument(); + }); + + it('submits the edited values via updateCustomFramework', async () => { + setMockPermissions(ADMIN_PERMISSIONS); + updateCustomFramework.mockResolvedValue({}); + const onClose = vi.fn(); + const onUpdated = vi.fn(); + renderSheet({ onClose, onUpdated }); + + const user = userEvent.setup(); + const nameInput = screen.getByDisplayValue('CMMC'); + await user.clear(nameInput); + await user.type(nameInput, 'CSC/CPRT'); + await user.click(screen.getByRole('button', { name: /save changes/i })); + + await waitFor(() => { + expect(updateCustomFramework).toHaveBeenCalledWith('frm_1', { + name: 'CSC/CPRT', + description: 'Original description', + }); + }); + expect(onUpdated).toHaveBeenCalled(); + expect(onClose).toHaveBeenCalled(); + }); + + it('rejects a whitespace-only name and does not submit', async () => { + setMockPermissions(ADMIN_PERMISSIONS); + renderSheet(); + + const user = userEvent.setup(); + const nameInput = screen.getByDisplayValue('CMMC'); + await user.clear(nameInput); + await user.type(nameInput, ' '); + await user.click(screen.getByRole('button', { name: /save changes/i })); + + expect(await screen.findByText('Name is required')).toBeInTheDocument(); + expect(updateCustomFramework).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/app/src/app/(app)/[orgId]/frameworks/[frameworkInstanceId]/components/EditCustomFrameworkSheet.tsx b/apps/app/src/app/(app)/[orgId]/frameworks/[frameworkInstanceId]/components/EditCustomFrameworkSheet.tsx new file mode 100644 index 0000000000..f0d24eb063 --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/frameworks/[frameworkInstanceId]/components/EditCustomFrameworkSheet.tsx @@ -0,0 +1,150 @@ +'use client'; + +import { useFrameworks } from '@/hooks/use-frameworks'; +import { usePermissions } from '@/hooks/use-permissions'; +import type { FrameworkInstanceWithControls } from '@/lib/types/framework'; +import { + Button, + Sheet, + SheetBody, + SheetContent, + SheetHeader, + SheetTitle, +} from '@trycompai/design-system'; +import { + Form, + FormControl, + FormField, + FormItem, + FormLabel, + FormMessage, +} from '@trycompai/ui/form'; +import { Input } from '@trycompai/ui/input'; +import { Textarea } from '@trycompai/ui/textarea'; +import { zodResolver } from '@hookform/resolvers/zod'; +import { useEffect, useState } from 'react'; +import { useForm } from 'react-hook-form'; +import { toast } from 'sonner'; +import { z } from 'zod'; + +const schema = z.object({ + // trim() runs before min(1), so whitespace-only names are rejected and the + // stored value is trimmed. + name: z.string().trim().min(1, 'Name is required').max(120), + description: z.string().max(2000), +}); + +type FormValues = z.infer; + +interface EditCustomFrameworkSheetProps { + isOpen: boolean; + onClose: () => void; + frameworkInstance: FrameworkInstanceWithControls; + onUpdated?: () => void; +} + +export function EditCustomFrameworkSheet({ + isOpen, + onClose, + frameworkInstance, + onUpdated, +}: EditCustomFrameworkSheetProps) { + const { updateCustomFramework } = useFrameworks(); + const { hasPermission } = usePermissions(); + const [isSubmitting, setIsSubmitting] = useState(false); + + const form = useForm({ + resolver: zodResolver(schema), + defaultValues: { + name: frameworkInstance.customFramework?.name ?? '', + description: frameworkInstance.customFramework?.description ?? '', + }, + mode: 'onChange', + }); + + // Refresh the form with the latest values each time the sheet opens. + useEffect(() => { + if (isOpen) { + form.reset({ + name: frameworkInstance.customFramework?.name ?? '', + description: frameworkInstance.customFramework?.description ?? '', + }); + } + }, [isOpen, frameworkInstance, form]); + + if (!hasPermission('framework', 'update')) return null; + + const handleSubmit = async (values: FormValues) => { + if (isSubmitting) return; + setIsSubmitting(true); + try { + await updateCustomFramework(frameworkInstance.id, values); + toast.success('Custom framework updated'); + onUpdated?.(); + onClose(); + } catch (error) { + toast.error( + error instanceof Error ? error.message : 'Failed to update framework', + ); + } finally { + setIsSubmitting(false); + } + }; + + return ( + !open && onClose()}> + + + Edit Custom Framework + + +
+ + ( + + Name + + + + + + )} + /> + ( + + Description + +