From 7c0b5a149da2a24f71b27d288c95eb04c146cc5b Mon Sep 17 00:00:00 2001 From: claude code agent 227 Date: Sun, 21 Jun 2026 05:46:35 +0300 Subject: [PATCH] test(workspace): cover trackerHead DTO validation, CASL gate, no-op audit (#98) DTO: trackerHead @IsString/@MaxLength(20000) + htmlEmbed @IsBoolean accept/reject cases. CASL: a non-admin updating trackerHead/htmlEmbed gets ForbiddenException (update not called); owner/admin proceed. Audit: a no-op trackerHead re-save doesn't enter the audit diff. Co-Authored-By: Claude Opus 4.8 --- .../controllers/workspace-update-gate.spec.ts | 95 +++++++++++++++++++ .../dto/update-workspace.dto.spec.ts | 66 +++++++++++++ .../services/workspace-html-embed.spec.ts | 58 +++++++++++ 3 files changed, 219 insertions(+) create mode 100644 apps/server/src/core/workspace/controllers/workspace-update-gate.spec.ts create mode 100644 apps/server/src/core/workspace/dto/update-workspace.dto.spec.ts diff --git a/apps/server/src/core/workspace/controllers/workspace-update-gate.spec.ts b/apps/server/src/core/workspace/controllers/workspace-update-gate.spec.ts new file mode 100644 index 00000000..93ebe38a --- /dev/null +++ b/apps/server/src/core/workspace/controllers/workspace-update-gate.spec.ts @@ -0,0 +1,95 @@ +import { ForbiddenException } from '@nestjs/common'; +import { WorkspaceController } from './workspace.controller'; +import WorkspaceAbilityFactory from '../../casl/abilities/workspace-ability.factory'; +import { UserRole } from '../../../common/helpers/types/permission'; + +// Pins the admin gate on WorkspaceController.updateWorkspace: writing workspace +// settings (including the admin-only trackerHead snippet and the htmlEmbed +// toggle) requires Manage settings ability. A MEMBER must be Forbidden BEFORE +// workspaceService.update is ever called; OWNER/ADMIN pass through. +// +// The REAL WorkspaceAbilityFactory is used (the gate under test); only the leaf +// service deps are stubbed. The controller is constructed directly with stubs, +// mirroring the other controller specs in this codebase. + +function buildController() { + const update = jest + .fn() + .mockResolvedValue({ id: 'w1', hostname: 'acme' }); + const workspaceService = { update }; + + const controller = new WorkspaceController( + workspaceService as any, + {} as any, // workspaceInvitationService + new WorkspaceAbilityFactory(), // REAL ability factory (the gate under test) + {} as any, // workspaceRepo + {} as any, // environmentService + {} as any, // licenseCheckService + ); + + return { controller, update }; +} + +const res = { clearCookie: jest.fn() } as any; +const workspace = { id: 'w1', hostname: 'acme' } as any; +const userWith = (role: UserRole) => ({ id: 'u1', role }) as any; + +describe('WorkspaceController.updateWorkspace settings gate', () => { + it('forbids a MEMBER from writing trackerHead and never calls update', async () => { + const { controller, update } = buildController(); + + await expect( + controller.updateWorkspace( + res, + { trackerHead: '' } as any, + userWith(UserRole.MEMBER), + workspace, + ), + ).rejects.toBeInstanceOf(ForbiddenException); + + expect(update).not.toHaveBeenCalled(); + }); + + it('forbids a MEMBER from toggling htmlEmbed and never calls update', async () => { + const { controller, update } = buildController(); + + await expect( + controller.updateWorkspace( + res, + { htmlEmbed: true } as any, + userWith(UserRole.MEMBER), + workspace, + ), + ).rejects.toBeInstanceOf(ForbiddenException); + + expect(update).not.toHaveBeenCalled(); + }); + + it('allows an OWNER to write trackerHead (update is called with the dto)', async () => { + const { controller, update } = buildController(); + const dto = { trackerHead: '' } as any; + + await controller.updateWorkspace( + res, + dto, + userWith(UserRole.OWNER), + workspace, + ); + + expect(update).toHaveBeenCalledWith('w1', dto); + }); + + it('allows an ADMIN to write trackerHead (update is called with the dto)', async () => { + const { controller, update } = buildController(); + const dto = { trackerHead: '' } as any; + + await controller.updateWorkspace( + res, + dto, + userWith(UserRole.ADMIN), + workspace, + ); + + expect(update).toHaveBeenCalledWith('w1', dto); + }); +}); diff --git a/apps/server/src/core/workspace/dto/update-workspace.dto.spec.ts b/apps/server/src/core/workspace/dto/update-workspace.dto.spec.ts new file mode 100644 index 00000000..2ef48315 --- /dev/null +++ b/apps/server/src/core/workspace/dto/update-workspace.dto.spec.ts @@ -0,0 +1,66 @@ +import 'reflect-metadata'; +import { plainToInstance } from 'class-transformer'; +import { validate } from 'class-validator'; +import { UpdateWorkspaceDto } from './update-workspace.dto'; + +// API-boundary validation for the two html-embed/tracker settings fields: +// - trackerHead: optional string, max 20000 chars (admin-authored snippet); +// - htmlEmbed: optional boolean (workspace master toggle). +// All other fields are optional, so a payload carrying just the field under test +// isolates that field's constraints. + +async function validateDto(payload: Record) { + const dto = plainToInstance(UpdateWorkspaceDto, payload); + return validate(dto as object); +} + +function hasError(errors: any[], property: string, constraint?: string) { + const err = errors.find((e) => e.property === property); + if (!err) return false; + if (!constraint) return true; + return Object.keys(err.constraints ?? {}).includes(constraint); +} + +describe('UpdateWorkspaceDto.trackerHead validation', () => { + it('accepts a normal trackerHead string', async () => { + const errors = await validateDto({ trackerHead: '' }); + expect(hasError(errors, 'trackerHead')).toBe(false); + }); + + it('accepts exactly 20000 characters', async () => { + const errors = await validateDto({ trackerHead: 'a'.repeat(20000) }); + expect(hasError(errors, 'trackerHead')).toBe(false); + }); + + it('rejects 20001 characters with a maxLength error', async () => { + const errors = await validateDto({ trackerHead: 'a'.repeat(20001) }); + expect(hasError(errors, 'trackerHead', 'maxLength')).toBe(true); + }); + + it('rejects a non-string trackerHead with an isString error', async () => { + const errors = await validateDto({ trackerHead: 123 }); + expect(hasError(errors, 'trackerHead', 'isString')).toBe(true); + }); + + it('accepts an omitted trackerHead (optional)', async () => { + const errors = await validateDto({}); + expect(hasError(errors, 'trackerHead')).toBe(false); + }); +}); + +describe('UpdateWorkspaceDto.htmlEmbed validation', () => { + it('accepts htmlEmbed: true', async () => { + const errors = await validateDto({ htmlEmbed: true }); + expect(hasError(errors, 'htmlEmbed')).toBe(false); + }); + + it('accepts htmlEmbed: false', async () => { + const errors = await validateDto({ htmlEmbed: false }); + expect(hasError(errors, 'htmlEmbed')).toBe(false); + }); + + it('rejects a non-boolean htmlEmbed with an isBoolean error', async () => { + const errors = await validateDto({ htmlEmbed: 'yes' }); + expect(hasError(errors, 'htmlEmbed', 'isBoolean')).toBe(true); + }); +}); diff --git a/apps/server/src/core/workspace/services/workspace-html-embed.spec.ts b/apps/server/src/core/workspace/services/workspace-html-embed.spec.ts index fbab1f6f..c9bb08ce 100644 --- a/apps/server/src/core/workspace/services/workspace-html-embed.spec.ts +++ b/apps/server/src/core/workspace/services/workspace-html-embed.spec.ts @@ -142,4 +142,62 @@ describe('WorkspaceService.update — htmlEmbed toggle persistence (real code)', expect(logged.changes.before.trackerHead).toBe(''); expect(logged.changes.after.trackerHead).toBe(''); }); + + it('still persists trackerHead on a no-op re-save (prev === input)', async () => { + // updateSetting must run even when the value is unchanged: the toggle write + // is idempotent and should not be skipped just because the audit diff is + // empty. + const { service, updateSetting } = buildService({ + settingsBefore: { trackerHead: '' }, + }); + + await service.update('w1', { + trackerHead: '', + } as any); + + expect(updateSetting).toHaveBeenCalledWith( + 'w1', + 'trackerHead', + '', + expect.anything(), + ); + }); + + it('does NOT audit a no-op trackerHead re-save (no before/after diff)', async () => { + // prev === input, and trackerHead is the only field touched, so the audit + // diff is empty and auditService.log must NOT fire — trackerHead never + // enters the audit payload on a no-op. + const { service, auditService } = buildService({ + settingsBefore: { trackerHead: '' }, + }); + + await service.update('w1', { + trackerHead: '', + } as any); + + expect(auditService.log).not.toHaveBeenCalled(); + }); + + it('keeps trackerHead OUT of the audit diff on a no-op while another field changes', async () => { + // trackerHead is re-saved identically (no-op) but htmlEmbed flips, so an + // audit IS logged — yet it must carry only htmlEmbed, never the unchanged + // trackerHead key. + const { service, auditService } = buildService({ + settingsBefore: { + trackerHead: '', + htmlEmbed: false, + }, + }); + + await service.update('w1', { + trackerHead: '', + htmlEmbed: true, + } as any); + + expect(auditService.log).toHaveBeenCalledTimes(1); + const logged = auditService.log.mock.calls[0][0]; + expect(logged.changes.after.htmlEmbed).toBe(true); + expect('trackerHead' in logged.changes.before).toBe(false); + expect('trackerHead' in logged.changes.after).toBe(false); + }); });