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 <noreply@anthropic.com>

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: '<script>ga()</script>' } 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: '<script>ga()</script>' } 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: '<script>ga()</script>' } as any;
+
+    await controller.updateWorkspace(
+      res,
+      dto,
+      userWith(UserRole.ADMIN),
+      workspace,
+    );
+
+    expect(update).toHaveBeenCalledWith('w1', dto);
+  });
+});

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<string, unknown>) {
+  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: '<script>ga()</script>' });
+    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);
+  });
+});

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('<script>m()</script>');
   });
+
+  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: '<script>same()</script>' },
+    });
+
+    await service.update('w1', {
+      trackerHead: '<script>same()</script>',
+    } as any);
+
+    expect(updateSetting).toHaveBeenCalledWith(
+      'w1',
+      'trackerHead',
+      '<script>same()</script>',
+      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: '<script>same()</script>' },
+    });
+
+    await service.update('w1', {
+      trackerHead: '<script>same()</script>',
+    } 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: '<script>same()</script>',
+        htmlEmbed: false,
+      },
+    });
+
+    await service.update('w1', {
+      trackerHead: '<script>same()</script>',
+      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);
+  });
 });