fix(rbac): enforce app:read for app access, use permissions for member invites, gate portal

* fix(rbac): enforce app:read for app access, use permissions for member invites, gate portal

Remove the APP_IMPLYING_RESOURCES fallback that let custom roles bypass
the App Access toggle. Replace hardcoded role string checks in the member
invite flow with RBAC permission checks (member:create/update), and add
privilege escalation prevention for non-admin callers. Add portal:read /
compliance-obligation check to the portal so unapproved roles are
redirected.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(rbac): resolve caller member permissions instead of hardcoded role checks

Replace role string matching (isAdmin/isAuditor) with actual RBAC
permission resolution — resolves the caller's member actions from both
built-in and custom roles via BUILT_IN_ROLE_PERMISSIONS + DB lookup.
Uses member:delete as the signal for full control (can assign any role)
vs restricted (can only assign employee/contractor/custom roles).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(rbac): let the permission guard handle member invite authorization

Remove redundant validateAssignableRoles — the @RequirePermission guard
on the controller already checks member:create. If the admin gave a
custom role Members: Write, that role can invite. No second layer of
role-string checks needed.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(rbac): add server-side role assignment validation based on member permission level

Resolve the caller's member actions from RBAC (built-in + custom roles).
Write-level access (all CRUD) can assign any role. Partial access (e.g.
auditor with create+read only) can only assign restricted roles
(employee/contractor) and custom roles — cannot assign privileged
built-in roles.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(rbac): use permission checks for allowedBuiltInRoles on people page

Replace hardcoded isAdminOrOwner/isAuditor role string checks with
Write-level member permission check (all CRUD actions). Mirrors the
backend validation logic.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* refactor(auth): add parseRolePermissions/parseRoleObligations helpers

Extract the repeated typeof/JSON.parse pattern for OrganizationRole
fields into typed helpers in the auth package. Replaces verbose
defensive checks with one-liner calls that return typed objects.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* refactor: clean up parse helpers and role checks

Add try/catch to JSON parse helpers, extract generic parseJsonField,
add isRestrictedRole() to eliminate verbose readonly casts, and
make portal-access checks consistent.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* chore: add cleanup skill for mandatory post-implementation code review

Committed to the repo so all Claude agents working in this codebase
will have it available and are required to run it after writing code.
Checks for verbose patterns, inconsistent idioms, missing error
handling, and readability issues.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* chore: update PostToolUse hook to remind about cleanup skill

The hook now fires for all TS files in apps/ and packages/ and reminds
agents to run the cleanup skill before committing.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Mariano Fuentes <marfuen98@gmail.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: 3 people

.claude/settings.json

@@ -6,7 +6,7 @@
         "hooks": [
           {
             "type": "command",
-            "command": "file=\"$CLAUDE_FILE_PATH\"; if echo \"$file\" | grep -qE '\\.(ts|tsx)$' && echo \"$file\" | grep -qE '^(apps/api|apps/app)/'; then echo 'TypeScript file modified — remember to run typecheck before committing'; fi"
+            "command": "file=\"$CLAUDE_FILE_PATH\"; if echo \"$file\" | grep -qE '\\.(ts|tsx)$' && echo \"$file\" | grep -qE '^(apps/|packages/)'; then echo 'TypeScript file modified — run the cleanup skill and typecheck before committing'; fi"
           }
         ]
       },

.claude/skills/cleanup/SKILL.md

@@ -0,0 +1,102 @@
+---
+name: cleanup
+description: "MUST run after writing or modifying code — reviews changed files for verbose patterns, inconsistencies, and readability issues before considering work done"
+---
+
+# Post-Implementation Cleanup
+
+**This skill is mandatory.** After writing or modifying code, you MUST review all changed files before reporting the task as complete. Code must be readable at a glance.
+
+## When to Run
+
+- After completing any implementation work
+- After fixing bugs
+- After refactoring
+- Before committing
+
+## Checklist
+
+For every file you changed, verify:
+
+### 1. No Verbose Defensive Checks
+
+Extract repeated patterns into typed helpers.
+
+```tsx
+// ❌ Verbose and repeated
+const perms = typeof role.permissions === 'string'
+  ? JSON.parse(role.permissions) : role.permissions;
+if (perms && typeof perms === 'object' && Array.isArray(perms.portal) && perms.portal.length > 0) {
+
+// ✅ Typed helper
+const perms = parseRolePermissions(role.permissions);
+if (perms?.portal?.length) {
+```
+
+### 2. Consistent Idioms Across Files
+
+The same check must use the same pattern everywhere.
+
+```tsx
+// ❌ Inconsistent
+file1: perms?.portal?.length > 0
+file2: perms?.portal?.length
+
+// ✅ Pick one
+perms?.portal?.length
+```
+
+### 3. No Redundant Type Casts
+
+If you need a cast to satisfy TypeScript, extract a helper function instead.
+
+```tsx
+// ❌ Verbose cast repeated in every file
+const restrictedRoles: readonly string[] = RESTRICTED_ROLES;
+restrictedRoles.includes(role);
+
+// ✅ Helper in shared package
+export function isRestrictedRole(role: string): boolean {
+  return (RESTRICTED_ROLES as readonly string[]).includes(role);
+}
+```
+
+### 4. Error Handling on Boundaries
+
+`JSON.parse`, external API calls, and DB queries at system boundaries need error handling.
+
+```tsx
+// ❌ Unguarded parse
+const parsed = JSON.parse(value);
+
+// ✅ Safe parse
+try {
+  return JSON.parse(value);
+} catch {
+  return null;
+}
+```
+
+### 5. Shared Patterns Belong in Shared Packages
+
+If the same logic appears in 2+ apps (api, app, portal), extract it to a shared package (`packages/auth`, `packages/db`, etc.).
+
+### 6. No Dead Code
+
+- Remove unused imports
+- Remove unused variables
+- Remove unused function parameters
+- Remove props that are always null/false
+
+### 7. Readable at a Glance
+
+- Function and variable names should convey intent without reading the implementation
+- One-liner expressions over multi-line when equally clear
+- No nested ternaries
+
+## How to Run
+
+1. List all files you modified: `git diff --name-only`
+2. Read each file and check against this checklist
+3. Fix any issues found
+4. Typecheck after fixes: `npx tsc --noEmit`

apps/api/src/people/people-invite.service.ts

@@ -2,15 +2,17 @@ import {
   Injectable,
   Logger,
   BadRequestException,
-  ForbiddenException,
 } from '@nestjs/common';
 import { db } from '@db';
 import { triggerEmail } from '../email/trigger-email';
 import { InviteEmail } from '../email/templates/invite-member';
 import { InvitePortalEmail } from '@trycompai/email';
 import {
   BUILT_IN_ROLE_OBLIGATIONS,
-  RESTRICTED_ROLES,
+  BUILT_IN_ROLE_PERMISSIONS,
+  isRestrictedRole,
+  parseRoleObligations,
+  parseRolePermissions,
 } from '@trycompai/auth';
 import type { InviteItemDto } from './dto/invite-people.dto';
 import { checkAutoCompletePhases } from '../frameworks/frameworks-timeline.helper';
@@ -37,38 +39,26 @@ export class PeopleInviteService {
   }): Promise<InviteResult[]> {
     const { organizationId, invites, callerUserId, callerRole } = params;
 
-    const isAdmin =
-      callerRole.includes('admin') || callerRole.includes('owner');
-    const isAuditor = callerRole.includes('auditor');
-
-    if (!isAdmin && !isAuditor) {
-      throw new ForbiddenException(
-        "You don't have permission to invite members.",
-      );
-    }
+    const callerMemberActions = await this.resolveCallerMemberActions(
+      callerRole,
+      organizationId,
+    );
 
     const results: InviteResult[] = [];
 
     for (const invite of invites) {
       try {
-        // Auditors can only invite auditors
-        if (isAuditor && !isAdmin) {
-          const onlyAuditor =
-            invite.roles.length === 1 && invite.roles[0] === 'auditor';
-          if (!onlyAuditor) {
-            results.push({
-              email: invite.email,
-              success: false,
-              error: "Auditors can only invite users with the 'auditor' role.",
-            });
-            continue;
-          }
+        const roleError = this.validateAssignableRoles(
+          invite.roles,
+          callerMemberActions,
+        );
+        if (roleError) {
+          results.push({ email: invite.email, success: false, error: roleError });
+          continue;
         }
 
         const email = invite.email.toLowerCase();
-        const restrictedRoles: readonly string[] = RESTRICTED_ROLES;
-        const isStrictlyEmployee =
-          invite.roles.every((role) => restrictedRoles.includes(role));
+        const isStrictlyEmployee = invite.roles.every(isRestrictedRole);
 
         const hasCompliance = await this.rolesHaveComplianceObligation(
           invite.roles,
@@ -436,13 +426,64 @@ export class PeopleInviteService {
       select: { obligations: true },
     });
 
-    return customRoles.some((role) => {
-      const obligations =
-        typeof role.obligations === 'string'
-          ? JSON.parse(role.obligations)
-          : role.obligations || {};
-      return !!obligations.compliance;
-    });
+    return customRoles.some((role) =>
+      parseRoleObligations(role.obligations).compliance,
+    );
+  }
+
+  /**
+   * Write-level = all CRUD actions. Callers with Write can assign any role.
+   * Partial access (e.g. auditor with create+read) can only assign
+   * restricted roles (employee/contractor) and custom roles.
+   */
+  private validateAssignableRoles(
+    targetRoles: string[],
+    callerMemberActions: Set<string>,
+  ): string | null {
+    const hasWriteAccess = ['create', 'read', 'update', 'delete'].every((a) =>
+      callerMemberActions.has(a),
+    );
+    if (hasWriteAccess) return null;
+
+    const disallowed = targetRoles.filter(
+      (r) => !isRestrictedRole(r) && Object.hasOwn(BUILT_IN_ROLE_PERMISSIONS, r),
+    );
+    if (disallowed.length > 0) {
+      return `You cannot assign privileged roles: ${disallowed.join(', ')}.`;
+    }
+    return null;
+  }
+
+  private async resolveCallerMemberActions(
+    callerRole: string,
+    organizationId: string,
+  ): Promise<Set<string>> {
+    const roles = callerRole.split(',').map((r) => r.trim());
+    const actions = new Set<string>();
+    const customRoleNames: string[] = [];
+
+    for (const role of roles) {
+      const builtIn = BUILT_IN_ROLE_PERMISSIONS[role];
+      if (builtIn?.member) {
+        for (const a of builtIn.member) actions.add(a);
+      }
+      if (!builtIn) customRoleNames.push(role);
+    }
+
+    if (customRoleNames.length > 0) {
+      const customRoles = await db.organizationRole.findMany({
+        where: { organizationId, name: { in: customRoleNames } },
+        select: { permissions: true },
+      });
+      for (const role of customRoles) {
+        const perms = parseRolePermissions(role.permissions);
+        if (perms?.member) {
+          for (const a of perms.member) actions.add(a);
+        }
+      }
+    }
+
+    return actions;
   }
 
   private buildPortalUrl(organizationId: string): string {

apps/app/src/app/(app)/[orgId]/people/components/PeoplePageTabs.tsx

@@ -10,6 +10,7 @@ import {
   TabsTrigger,
 } from '@trycompai/design-system';
 import { Add } from '@trycompai/design-system/icons';
+import type { Role } from '@db';
 import { usePathname, useRouter, useSearchParams } from 'next/navigation';
 import type { ReactNode } from 'react';
 import { useCallback, useState } from 'react';
@@ -28,6 +29,7 @@ interface PeoplePageTabsProps {
   showSettings: boolean;
   canInviteUsers: boolean;
   canManageMembers: boolean;
+  allowedBuiltInRoles: Role[];
   organizationId: string;
 }
 
@@ -89,6 +91,7 @@ export function PeoplePageTabs({
   showSettings,
   canInviteUsers,
   canManageMembers,
+  allowedBuiltInRoles,
   organizationId,
 }: PeoplePageTabsProps) {
   const pathname = usePathname();
@@ -164,9 +167,7 @@ export function PeoplePageTabs({
         open={isInviteModalOpen}
         onOpenChange={setIsInviteModalOpen}
         organizationId={organizationId}
-        allowedBuiltInRoles={
-          canManageMembers ? ['admin', 'auditor', 'employee', 'contractor'] : ['auditor']
-        }
+        allowedBuiltInRoles={allowedBuiltInRoles}
       />
     </Tabs>
   );

apps/app/src/app/(app)/[orgId]/people/page.tsx

@@ -3,6 +3,7 @@ import { resolveUserPermissions } from '@/lib/permissions.server';
 import { auth } from '@/utils/auth';
 import { db } from '@db/server';
 import type { Metadata } from 'next';
+import type { Role } from '@db';
 import { headers } from 'next/headers';
 import { redirect } from 'next/navigation';
 import { Suspense } from 'react';
@@ -32,15 +33,23 @@ export default async function PeoplePage({ params }: { params: Promise<{ orgId:
     where: { organizationId: orgId, userId: session.user.id },
   });
   const currentUserRoles = currentUserMember?.role?.split(',').map((r) => r.trim()) ?? [];
-  const canManageMembers = currentUserRoles.some((role) => ['owner', 'admin'].includes(role));
-  const isAuditor = currentUserRoles.includes('auditor');
-  const canInviteUsers = canManageMembers || isAuditor;
   const isCurrentUserOwner = currentUserRoles.includes('owner');
 
   const userPermissions = await resolveUserPermissions(
     currentUserMember?.role ?? null,
     orgId,
   );
+  const canManageMembers = hasPermission(userPermissions, 'member', 'update');
+  const canInviteUsers = hasPermission(userPermissions, 'member', 'create');
+
+  const hasWriteMemberAccess =
+    canInviteUsers &&
+    hasPermission(userPermissions, 'member', 'read') &&
+    canManageMembers &&
+    hasPermission(userPermissions, 'member', 'delete');
+  const allowedBuiltInRoles: Role[] = hasWriteMemberAccess
+    ? ['admin', 'auditor', 'employee', 'contractor']
+    : ['employee', 'contractor'];
   const canManageOrgSettings = hasPermission(
     userPermissions,
     'organization',
@@ -85,6 +94,7 @@ export default async function PeoplePage({ params }: { params: Promise<{ orgId:
       showEmployeeTasks
       canInviteUsers={canInviteUsers}
       canManageMembers={canManageMembers}
+      allowedBuiltInRoles={allowedBuiltInRoles}
       organizationId={orgId}
     />
   );

apps/app/src/lib/permissions.ts

@@ -153,17 +153,6 @@ export function getDefaultRoute(permissions: UserPermissions, orgId: string): st
   return null;
 }
 
-/**
- * Resources that imply the user should have access to the main app.
- * Portal-only resources (policy, compliance) are excluded — employees/contractors
- * have those but should NOT enter the app.
- */
-const APP_IMPLYING_RESOURCES = new Set([
-  'organization', 'member', 'control', 'evidence', 'risk', 'vendor',
-  'task', 'framework', 'audit', 'finding', 'questionnaire', 'integration',
-  'apiKey', 'trust', 'pentest',
-]);
-
 /** Compliance route segments — used to determine if the Compliance rail icon should show. */
 const COMPLIANCE_ROUTE_SEGMENTS = [
   'overview', 'frameworks', 'controls', 'policies', 'tasks', 'documents', 'people',
@@ -181,22 +170,11 @@ export function canAccessCompliance(permissions: UserPermissions): boolean {
 /**
  * Check if user can access the main app (as opposed to portal-only).
  *
- * Returns true if the user has explicit `app:read` (built-in roles like owner/admin/auditor),
- * OR if they have any permission on a resource that implies app access (e.g. a custom role
- * with only `pentest:read`).
- *
- * Employees and contractors are portal-only — they only have `policy:read`
- * and compliance obligations, neither of which is in APP_IMPLYING_RESOURCES.
+ * Requires explicit `app:read` — controlled by the "App Access" toggle
+ * on custom roles, and included by default in owner/admin/auditor.
  */
 export function canAccessApp(permissions: UserPermissions): boolean {
-  if (hasPermission(permissions, 'app', 'read')) return true;
-
-  for (const resource of Object.keys(permissions)) {
-    if (APP_IMPLYING_RESOURCES.has(resource) && permissions[resource]?.length > 0) {
-      return true;
-    }
-  }
-  return false;
+  return hasPermission(permissions, 'app', 'read');
 }
 
 /**