From b8b7a3b6fac7e96a513671b93a86d5ddd7302c0e Mon Sep 17 00:00:00 2001 From: chasprowebdev Date: Mon, 20 Jul 2026 12:06:02 -0400 Subject: [PATCH 1/5] fix(app): grant portal permission automatically when Employee Compliance is enabled --- .../components/PermissionMatrix.test.tsx | 66 +++++++++++++++++++ .../roles/components/PermissionMatrix.tsx | 15 +++++ 2 files changed, 81 insertions(+) diff --git a/apps/app/src/app/(app)/[orgId]/settings/roles/components/PermissionMatrix.test.tsx b/apps/app/src/app/(app)/[orgId]/settings/roles/components/PermissionMatrix.test.tsx index 2d1ae27fc0..d9c6215b11 100644 --- a/apps/app/src/app/(app)/[orgId]/settings/roles/components/PermissionMatrix.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/settings/roles/components/PermissionMatrix.test.tsx @@ -291,6 +291,72 @@ describe('PermissionMatrix', () => { }); }); +describe('Employee Compliance obligation implies portal access', () => { + function findComplianceSwitch(): HTMLElement { + const label = screen.getByText('Employee Compliance'); + const row = label.closest('[class*="flex"]') as HTMLElement; + const switchEl = row.querySelector('[role="switch"]'); + if (!switchEl) throw new Error('Employee Compliance switch not found'); + return switchEl as HTMLElement; + } + + it('adds portal:read/update permissions when the compliance obligation is enabled', () => { + const mockOnChange = vi.fn(); + const mockOnObligationsChange = vi.fn(); + render( + , + ); + + fireEvent.click(findComplianceSwitch()); + + expect(mockOnObligationsChange).toHaveBeenCalledWith({ compliance: true }); + expect(mockOnChange).toHaveBeenCalledWith({ + control: ['read'], + portal: ['read', 'update'], + }); + }); + + it('removes portal permissions when the compliance obligation is disabled', () => { + const mockOnChange = vi.fn(); + const mockOnObligationsChange = vi.fn(); + render( + , + ); + + fireEvent.click(findComplianceSwitch()); + + expect(mockOnObligationsChange).toHaveBeenCalledWith({}); + expect(mockOnChange).toHaveBeenCalledWith({ control: ['read'] }); + }); + + it('does not call onChange (permissions) when an unrelated resource permission changes', () => { + // Only the 'compliance' obligation toggle should sync portal — a plain + // resource-permission change must not go through the obligation branch. + const mockOnChange = vi.fn(); + render(); + + const controlsText = screen.getByText('Controls'); + const controlsRow = controlsText.closest('[class*="grid"]'); + const radios = controlsRow?.querySelectorAll('[data-slot="radio-group-item"]'); + if (radios && radios[1]) { + fireEvent.click(radios[1]); // No Access -> Read: a real change + } + + expect(mockOnChange).toHaveBeenCalledTimes(1); + expect(mockOnChange).toHaveBeenCalledWith({ control: ['read'] }); + }); +}); + describe('Utility Functions', () => { describe('getAccessLevel', () => { it('returns "none" for empty permissions', () => { diff --git a/apps/app/src/app/(app)/[orgId]/settings/roles/components/PermissionMatrix.tsx b/apps/app/src/app/(app)/[orgId]/settings/roles/components/PermissionMatrix.tsx index 0b08e9f058..c1fa106907 100644 --- a/apps/app/src/app/(app)/[orgId]/settings/roles/components/PermissionMatrix.tsx +++ b/apps/app/src/app/(app)/[orgId]/settings/roles/components/PermissionMatrix.tsx @@ -235,6 +235,21 @@ export function PermissionMatrix({ value, onChange, obligations, onObligationsCh delete newObligations[key]; } onObligationsChange(newObligations); + + // The 'compliance' obligation (Employee Compliance) implies portal + // self-service access (sign policies, watch training, etc.) — there's + // no separate matrix row for the 'portal' resource itself (it's + // excluded from RESOURCE_LABELS/RESOURCE_SECTIONS), so keep it in sync + // with this toggle instead. + if (key === 'compliance') { + const newPermissions = { ...value }; + if (enabled) { + newPermissions.portal = [...statement.portal]; + } else { + delete newPermissions.portal; + } + onChange(newPermissions); + } }; const handleToggleChange = (resourceKey: string, enabled: boolean) => { From 65904fbf306cdff75ee3f9500abf1e1892247f2c Mon Sep 17 00:00:00 2001 From: chasprowebdev Date: Mon, 20 Jul 2026 12:30:58 -0400 Subject: [PATCH 2/5] fix(app): update test name in PermissionMatrix --- .../[orgId]/settings/roles/components/PermissionMatrix.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/app/src/app/(app)/[orgId]/settings/roles/components/PermissionMatrix.test.tsx b/apps/app/src/app/(app)/[orgId]/settings/roles/components/PermissionMatrix.test.tsx index d9c6215b11..66630fde57 100644 --- a/apps/app/src/app/(app)/[orgId]/settings/roles/components/PermissionMatrix.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/settings/roles/components/PermissionMatrix.test.tsx @@ -339,7 +339,7 @@ describe('Employee Compliance obligation implies portal access', () => { expect(mockOnChange).toHaveBeenCalledWith({ control: ['read'] }); }); - it('does not call onChange (permissions) when an unrelated resource permission changes', () => { + it('does not inject portal permissions when an unrelated resource permission changes', () => { // Only the 'compliance' obligation toggle should sync portal — a plain // resource-permission change must not go through the obligation branch. const mockOnChange = vi.fn(); From d5c7c8c808c3a3d6a8fdcd9e1992e1aa834bcd67 Mon Sep 17 00:00:00 2001 From: chasprowebdev Date: Mon, 20 Jul 2026 12:34:23 -0400 Subject: [PATCH 3/5] fix(app): assert onObligationsChange isn't called on unrelated permission changes --- .../roles/components/PermissionMatrix.test.tsx | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/apps/app/src/app/(app)/[orgId]/settings/roles/components/PermissionMatrix.test.tsx b/apps/app/src/app/(app)/[orgId]/settings/roles/components/PermissionMatrix.test.tsx index 66630fde57..da8ab3c718 100644 --- a/apps/app/src/app/(app)/[orgId]/settings/roles/components/PermissionMatrix.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/settings/roles/components/PermissionMatrix.test.tsx @@ -339,11 +339,19 @@ describe('Employee Compliance obligation implies portal access', () => { expect(mockOnChange).toHaveBeenCalledWith({ control: ['read'] }); }); - it('does not inject portal permissions when an unrelated resource permission changes', () => { + it('does not inject portal permissions or touch obligations when an unrelated resource permission changes', () => { // Only the 'compliance' obligation toggle should sync portal — a plain // resource-permission change must not go through the obligation branch. const mockOnChange = vi.fn(); - render(); + const mockOnObligationsChange = vi.fn(); + render( + , + ); const controlsText = screen.getByText('Controls'); const controlsRow = controlsText.closest('[class*="grid"]'); @@ -354,6 +362,7 @@ describe('Employee Compliance obligation implies portal access', () => { expect(mockOnChange).toHaveBeenCalledTimes(1); expect(mockOnChange).toHaveBeenCalledWith({ control: ['read'] }); + expect(mockOnObligationsChange).not.toHaveBeenCalled(); }); }); From f12c7804a3c8deb85e0d5149ab502feef24171cd Mon Sep 17 00:00:00 2001 From: chasprowebdev Date: Mon, 20 Jul 2026 13:03:32 -0400 Subject: [PATCH 4/5] fix(api): enforce compliance-obligation-implies-portal invariant server-side --- apps/api/src/roles/roles.service.spec.ts | 212 ++++++++++++++++++++++- apps/api/src/roles/roles.service.ts | 62 ++++++- 2 files changed, 262 insertions(+), 12 deletions(-) diff --git a/apps/api/src/roles/roles.service.spec.ts b/apps/api/src/roles/roles.service.spec.ts index ee709e6f5f..b09d3a9311 100644 --- a/apps/api/src/roles/roles.service.spec.ts +++ b/apps/api/src/roles/roles.service.spec.ts @@ -28,6 +28,7 @@ jest.mock('@trycompai/auth', () => { apiKey: ['create', 'read', 'delete'], app: ['read'], trust: ['read', 'update'], + portal: ['read', 'update'], }; const BUILT_IN_ROLE_PERMISSIONS: Record> = { @@ -340,6 +341,80 @@ describe('RolesService', () => { service.createRole(organizationId, dto, ['employee', 'auditor']), ).resolves.toBeDefined(); }); + + it('grants portal:read/update when the compliance obligation is set, even if not requested', async () => { + // Regression: a role created with obligations.compliance=true via any + // caller other than the roles settings UI (which syncs this itself in + // PermissionMatrix.tsx) would otherwise lack 'portal' entirely and be + // unable to reach portal-gated endpoints (e.g. training completions). + const dto = { + name: 'devops-engineer', + permissions: { control: ['read'] }, + obligations: { compliance: true }, + }; + + (mockDb.organizationRole.findFirst as jest.Mock).mockResolvedValue(null); + (mockDb.organizationRole.count as jest.Mock).mockResolvedValue(0); + (mockDb.organizationRole.create as jest.Mock).mockImplementation( + ({ data }) => ({ + id: 'rol_devops', + ...data, + createdAt: new Date(), + updatedAt: new Date(), + }), + ); + + const result = await service.createRole(organizationId, dto, ['owner']); + + expect(result.permissions.portal).toEqual( + expect.arrayContaining(['read', 'update']), + ); + }); + + it('does not add portal without the compliance obligation', async () => { + const dto = { + name: 'read-only-role', + permissions: { control: ['read'] }, + }; + + (mockDb.organizationRole.findFirst as jest.Mock).mockResolvedValue(null); + (mockDb.organizationRole.count as jest.Mock).mockResolvedValue(0); + (mockDb.organizationRole.create as jest.Mock).mockImplementation( + ({ data }) => ({ + id: 'rol_readonly', + ...data, + createdAt: new Date(), + updatedAt: new Date(), + }), + ); + + const result = await service.createRole(organizationId, dto, ['owner']); + + expect(result.permissions.portal).toBeUndefined(); + }); + + it('does not duplicate portal actions already requested alongside the compliance obligation', async () => { + const dto = { + name: 'portal-explicit', + permissions: { portal: ['read'] }, + obligations: { compliance: true }, + }; + + (mockDb.organizationRole.findFirst as jest.Mock).mockResolvedValue(null); + (mockDb.organizationRole.count as jest.Mock).mockResolvedValue(0); + (mockDb.organizationRole.create as jest.Mock).mockImplementation( + ({ data }) => ({ + id: 'rol_portal', + ...data, + createdAt: new Date(), + updatedAt: new Date(), + }), + ); + + const result = await service.createRole(organizationId, dto, ['owner']); + + expect(result.permissions.portal.sort()).toEqual(['read', 'update']); + }); }); describe('listRoles', () => { @@ -488,6 +563,105 @@ describe('RolesService', () => { ), ).rejects.toThrow(ForbiddenException); }); + + it('grants portal:read/update when enabling the compliance obligation on an obligations-only update', async () => { + // An obligations-only PATCH (no `permissions` in the request) must + // still merge portal into the role's EXISTING stored permissions — + // otherwise re-saving obligations alone wouldn't fix a role that was + // missing portal. + const existingRole = { + id: roleId, + name: 'devops-engineer', + permissions: JSON.stringify({ control: ['read'] }), + obligations: '{}', + }; + + (mockDb.organizationRole.findFirst as jest.Mock).mockResolvedValue( + existingRole, + ); + (mockDb.organizationRole.update as jest.Mock).mockImplementation( + ({ data }) => ({ + id: roleId, + name: existingRole.name, + ...data, + updatedAt: new Date(), + }), + ); + + const result = await service.updateRole( + organizationId, + roleId, + { obligations: { compliance: true } }, + ['owner'], + ); + + expect(result.permissions.portal).toEqual( + expect.arrayContaining(['read', 'update']), + ); + expect(result.permissions.control).toEqual(['read']); + }); + + it('grants portal:read/update when updating permissions on a role whose existing obligations already require compliance', async () => { + // A permissions-only update (no `obligations` in the request) must + // still see the role's EXISTING obligations to know portal applies. + const existingRole = { + id: roleId, + name: 'devops-engineer', + permissions: JSON.stringify({ control: ['read'] }), + obligations: JSON.stringify({ compliance: true }), + }; + + (mockDb.organizationRole.findFirst as jest.Mock).mockResolvedValue( + existingRole, + ); + (mockDb.organizationRole.update as jest.Mock).mockImplementation( + ({ data }) => ({ + id: roleId, + name: existingRole.name, + obligations: existingRole.obligations, + ...data, + updatedAt: new Date(), + }), + ); + + const result = await service.updateRole( + organizationId, + roleId, + { permissions: { control: ['read', 'update'] } }, + ['owner'], + ); + + expect(result.permissions.portal).toEqual( + expect.arrayContaining(['read', 'update']), + ); + }); + + it('does not touch permissions when neither permissions nor obligations are part of the update', async () => { + const existingRole = { + id: roleId, + name: 'old-name', + permissions: JSON.stringify({ control: ['read'] }), + obligations: '{}', + }; + + (mockDb.organizationRole.findFirst as jest.Mock) + .mockResolvedValueOnce(existingRole) + .mockResolvedValueOnce(null); + (mockDb.organizationRole.update as jest.Mock).mockResolvedValue({ + ...existingRole, + name: 'new-name', + updatedAt: new Date(), + }); + + await service.updateRole(organizationId, roleId, { name: 'new-name' }, [ + 'owner', + ]); + + expect(mockDb.organizationRole.update).toHaveBeenCalledWith({ + where: { id: roleId }, + data: { name: 'new-name' }, + }); + }); }); describe('deleteRole', () => { @@ -746,7 +920,10 @@ describe('RolesService', () => { it('returns the hardcoded default when no override row exists', async () => { (mockDb.organizationRole.findFirst as jest.Mock).mockResolvedValue(null); - const result = await service.getBuiltInObligations(organizationId, 'owner'); + const result = await service.getBuiltInObligations( + organizationId, + 'owner', + ); expect(result).toEqual({ compliance: true }); }); @@ -754,7 +931,10 @@ describe('RolesService', () => { (mockDb.organizationRole.findFirst as jest.Mock).mockResolvedValue({ obligations: JSON.stringify({ compliance: false }), }); - const result = await service.getBuiltInObligations(organizationId, 'owner'); + const result = await service.getBuiltInObligations( + organizationId, + 'owner', + ); expect(result).toEqual({ compliance: false }); }); @@ -762,13 +942,19 @@ describe('RolesService', () => { (mockDb.organizationRole.findFirst as jest.Mock).mockResolvedValue({ obligations: { compliance: false }, }); - const result = await service.getBuiltInObligations(organizationId, 'owner'); + const result = await service.getBuiltInObligations( + organizationId, + 'owner', + ); expect(result).toEqual({ compliance: false }); }); it('returns empty for admin (no default compliance, no override)', async () => { (mockDb.organizationRole.findFirst as jest.Mock).mockResolvedValue(null); - const result = await service.getBuiltInObligations(organizationId, 'admin'); + const result = await service.getBuiltInObligations( + organizationId, + 'admin', + ); expect(result).toEqual({}); }); @@ -784,7 +970,10 @@ describe('RolesService', () => { (mockDb.organizationRole.findFirst as jest.Mock).mockResolvedValue({ obligations: '{}', }); - const result = await service.getBuiltInObligations(organizationId, 'owner'); + const result = await service.getBuiltInObligations( + organizationId, + 'owner', + ); expect(result).toEqual({ compliance: true }); }); }); @@ -804,7 +993,10 @@ describe('RolesService', () => { { compliance: false }, ); - expect(result).toEqual({ name: 'owner', obligations: { compliance: false } }); + expect(result).toEqual({ + name: 'owner', + obligations: { compliance: false }, + }); expect(mockDb.organizationRole.upsert).toHaveBeenCalledWith({ where: { organizationId_name: { organizationId, name: 'owner' } }, create: expect.objectContaining({ @@ -924,14 +1116,18 @@ describe('RolesService', () => { const result = await service.listRoles(organizationId); // Override row must not appear as a custom role - expect(result.customRoles.map((r) => r.name)).toEqual(['compliance-lead']); + expect(result.customRoles.map((r) => r.name)).toEqual([ + 'compliance-lead', + ]); // Built-in entries carry effective obligations — owner reflects the override const ownerEntry = result.builtInRoles.find((r) => r.name === 'owner'); expect(ownerEntry?.obligations).toEqual({ compliance: false }); // Other built-ins still show their hardcoded defaults const adminEntry = result.builtInRoles.find((r) => r.name === 'admin'); expect(adminEntry?.obligations).toEqual({}); - const employeeEntry = result.builtInRoles.find((r) => r.name === 'employee'); + const employeeEntry = result.builtInRoles.find( + (r) => r.name === 'employee', + ); expect(employeeEntry?.obligations).toEqual({ compliance: true }); }); diff --git a/apps/api/src/roles/roles.service.ts b/apps/api/src/roles/roles.service.ts index 12bb421e6f..32ab2a69ff 100644 --- a/apps/api/src/roles/roles.service.ts +++ b/apps/api/src/roles/roles.service.ts @@ -79,6 +79,32 @@ export class RolesService { } } + /** + * Enforces the compliance -> portal permission invariant before a role is + * persisted: any role whose obligations require compliance (sign policies, + * watch training, etc.) must also carry `portal:read/update`. The + * custom-role editor UI keeps this in sync as a callback + * (PermissionMatrix.tsx's `handleObligationChange`), but that only covers + * the one client — a role created or updated any other way (public API, + * MCP, a future UI) could set `obligations.compliance` without `portal` + * and end up unable to reach portal-gated endpoints (e.g. training video + * completions). Normalizing here, right before every write, closes that + * gap regardless of caller. One-directional by design: it never strips an + * explicitly granted `portal` permission just because compliance is false. + */ + private withCompliancePortalInvariant( + permissions: Record, + obligations: RoleObligations, + ): Record { + if (!obligations.compliance) return permissions; + + const portalActions = new Set([ + ...(permissions.portal ?? []), + ...statement.portal, + ]); + return { ...permissions, portal: [...portalActions] }; + } + /** * Check if caller has all the permissions they're trying to grant. * Prevents privilege escalation. @@ -242,11 +268,16 @@ export class RolesService { } // Create the role + const obligations = dto.obligations || {}; + const permissions = this.withCompliancePortalInvariant( + dto.permissions, + obligations, + ); const role = await db.organizationRole.create({ data: { name: dto.name, - permissions: JSON.stringify(dto.permissions), - obligations: JSON.stringify(dto.obligations || {}), + permissions: JSON.stringify(permissions), + obligations: JSON.stringify(obligations), organizationId, }, }); @@ -408,13 +439,36 @@ export class RolesService { ); } + // Re-derive the compliance -> portal invariant whenever either + // permissions or obligations change, using the existing row's stored + // value for whichever side wasn't part of this request (e.g. an + // obligations-only update must still see the role's current + // permissions to merge portal into). + let permissionsToPersist: Record | undefined; + if (dto.permissions !== undefined || dto.obligations !== undefined) { + const effectiveObligations = + dto.obligations !== undefined + ? dto.obligations + : parseObligationsField(role.obligations); + const effectivePermissions: Record = + dto.permissions !== undefined + ? dto.permissions + : typeof role.permissions === 'string' + ? (JSON.parse(role.permissions) as Record) + : role.permissions; + permissionsToPersist = this.withCompliancePortalInvariant( + effectivePermissions, + effectiveObligations, + ); + } + // Update the role const updated = await db.organizationRole.update({ where: { id: roleId }, data: { ...(dto.name && { name: dto.name }), - ...(dto.permissions && { - permissions: JSON.stringify(dto.permissions), + ...(permissionsToPersist && { + permissions: JSON.stringify(permissionsToPersist), }), ...(dto.obligations !== undefined && { obligations: JSON.stringify(dto.obligations), From e0f84b3a47ca515de68fc450b6cf7eaa32954969 Mon Sep 17 00:00:00 2001 From: chasprowebdev Date: Mon, 20 Jul 2026 16:16:06 -0400 Subject: [PATCH 5/5] fix(api): enforce compliance-obligation-implies-portal invariant server-side --- apps/api/src/roles/roles.service.spec.ts | 52 ++++++++++++++++++++++++ apps/api/src/roles/roles.service.ts | 40 +++++++++++------- 2 files changed, 78 insertions(+), 14 deletions(-) diff --git a/apps/api/src/roles/roles.service.spec.ts b/apps/api/src/roles/roles.service.spec.ts index b09d3a9311..30d4c7572f 100644 --- a/apps/api/src/roles/roles.service.spec.ts +++ b/apps/api/src/roles/roles.service.spec.ts @@ -415,6 +415,28 @@ describe('RolesService', () => { expect(result.permissions.portal.sort()).toEqual(['read', 'update']); }); + + it('rejects creating a compliance-obligated role when the caller lacks portal access (privilege escalation)', async () => { + // Regression: the compliance -> portal invariant must be validated, + // not just applied — otherwise a caller without 'portal' could grant + // it to a role merely by setting obligations.compliance=true. + const dto = { + name: 'devops-engineer', + permissions: { control: ['read'] }, // 'auditor' has this + obligations: { compliance: true }, + }; + + // 'auditor' in this mock has control:['read'] but no 'portal' key. + await expect( + service.createRole(organizationId, dto, ['auditor']), + ).rejects.toThrow(ForbiddenException); + await expect( + service.createRole(organizationId, dto, ['auditor']), + ).rejects.toThrow( + "Cannot grant 'portal:read' permission - you don't have this permission", + ); + expect(mockDb.organizationRole.create).not.toHaveBeenCalled(); + }); }); describe('listRoles', () => { @@ -564,6 +586,36 @@ describe('RolesService', () => { ).rejects.toThrow(ForbiddenException); }); + it('rejects an obligations-only update that would grant portal access when the caller lacks it (privilege escalation)', async () => { + // Regression (P1): before this check ran against permissionsToPersist, + // an obligations-only update never validated privilege escalation at + // all, so any caller could grant a role portal access just by + // flipping obligations.compliance=true — even a caller without portal + // permission themselves. + const existingRole = { + id: roleId, + name: 'devops-engineer', + permissions: JSON.stringify({ control: ['read'] }), // 'auditor' has this + obligations: '{}', + }; + + (mockDb.organizationRole.findFirst as jest.Mock).mockResolvedValue( + existingRole, + ); + + // 'auditor' has control:['read'] (matches the existing role) but no + // 'portal' key — the newly-merged portal grant must be caught. + await expect( + service.updateRole( + organizationId, + roleId, + { obligations: { compliance: true } }, + ['auditor'], + ), + ).rejects.toThrow(ForbiddenException); + expect(mockDb.organizationRole.update).not.toHaveBeenCalled(); + }); + it('grants portal:read/update when enabling the compliance obligation on an obligations-only update', async () => { // An obligations-only PATCH (no `permissions` in the request) must // still merge portal into the role's EXISTING stored permissions — diff --git a/apps/api/src/roles/roles.service.ts b/apps/api/src/roles/roles.service.ts index 32ab2a69ff..2b688c17ca 100644 --- a/apps/api/src/roles/roles.service.ts +++ b/apps/api/src/roles/roles.service.ts @@ -232,13 +232,23 @@ export class RolesService { ); } - // Validate permissions + // Validate permission shape (resource/action names) as submitted. this.validatePermissions(dto.permissions); - // Check for privilege escalation + // Derive the final permission set (including the compliance -> portal + // invariant) before checking privilege escalation, so a caller can't + // grant themselves/others portal access merely by setting + // obligations.compliance=true without explicitly requesting 'portal'. + const obligations = dto.obligations || {}; + const permissions = this.withCompliancePortalInvariant( + dto.permissions, + obligations, + ); + + // Check for privilege escalation against the FINAL permission set. await this.validateNoPrivilegeEscalation( callerRoles, - dto.permissions, + permissions, organizationId, ); @@ -268,11 +278,6 @@ export class RolesService { } // Create the role - const obligations = dto.obligations || {}; - const permissions = this.withCompliancePortalInvariant( - dto.permissions, - obligations, - ); const role = await db.organizationRole.create({ data: { name: dto.name, @@ -429,14 +434,9 @@ export class RolesService { } } - // Validate and check permissions if provided + // Validate permission shape (resource/action names) as submitted. if (dto.permissions) { this.validatePermissions(dto.permissions); - await this.validateNoPrivilegeEscalation( - callerRoles, - dto.permissions, - organizationId, - ); } // Re-derive the compliance -> portal invariant whenever either @@ -460,6 +460,18 @@ export class RolesService { effectivePermissions, effectiveObligations, ); + + // Validate against the FINAL permission set that will actually be + // written — not just the submitted `dto.permissions`. This also + // catches the portal grant the invariant above may have just added + // on an obligations-only update (no `dto.permissions` at all), which + // would otherwise let a caller without portal access grant it to a + // role simply by toggling the compliance obligation. + await this.validateNoPrivilegeEscalation( + callerRoles, + permissionsToPersist, + organizationId, + ); } // Update the role