Conversation
… (CS-189)
The Auditor View tab was gated on roles.includes(Role.auditor) — a
literal string match against the built-in "auditor" role. That made the
tab invisible to any custom organization role (e.g. a "Comp AI" custom
role a customer set up for our support team) even when that role was
granted audit:read, and also hid it from owners/admins who implicitly
have the permission. The route layout already enforces audit:read via
requireRoutePermission, so the literal-role gate was both redundant and
wrong.
Switched to canAccessRoute(permissions, 'auditor') in:
- AppSidebar nav item (design-system sidebar)
- MainMenu item (legacy sidebar + mobile menu) — reads permissions via
usePermissions() so callers no longer need to pass hasAuditorRole
- app-shell-search-groups command palette entry
Also removed the redundant roles.includes(Role.auditor) page-level
check in auditor/(overview)/page.tsx; auditor/layout.tsx's existing
requireRoutePermission('auditor', orgId) still enforces audit:read
server-side.
Aligns with CLAUDE.md RBAC guidance: "No manual role string parsing —
always use permission checks."
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…w-up)
Follow-up after CTO review: owner/admin should NOT see the Auditor View
tab or access the page, even though their implicit all-permissions
include audit:read. The tab is scoped to audit work; it should only
appear for users whose role explicitly grants audit access.
New rule:
Show Auditor View iff
- user has the built-in `auditor` role, OR
- user has a custom org role that explicitly grants `audit:read`.
Implementation:
- New pure helper `canAccessAuditorView(roleString, customRolePermissions)`
in lib/permissions.ts. Separates "permission came from built-in role"
(owner/admin/everyone) from "permission came from custom role".
- Server-side helpers in lib/permissions.server.ts:
- `resolveCustomRolePermissions` fetches only the user's custom-role
permissions (built-in roles excluded).
- `resolveAuditorViewAccess` runs the check for the current user.
- `requireAuditorViewAccess` is the route guard — replaces
`requireRoutePermission('auditor', orgId)` on the auditor layout.
- usePermissions() now returns `canAccessAuditorView` + `customPermissions`
so client components don't have to replicate the rule.
- Org layout computes the flag server-side and forwards it through
AppShellWrapper → AppSidebar + getAppShellSearchGroups.
- main-menu reads the flag directly from usePermissions().
Behavior matrix:
- auditor → visible ✅
- owner,auditor / admin,auditor → visible ✅
- CompAI custom role with audit:read → visible ✅
- owner alone / admin alone → hidden (implicit audit:read doesn't count)
- owner,ReadOnlyCustomRole (no audit) → hidden
- employee / contractor → hidden ✅
Adds 9 unit tests locking in every row of that matrix.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Tofik Hasanov <72318342+tofikwest@users.noreply.github.com>
…ew gating
Per CS-189 product decision: owners/admins should not see the Auditor View
tab unless they explicitly opt in via a custom role granting audit:read.
Removing audit:read from the built-in owner/admin roles lets the standard
canAccessRoute('auditor') check do the work — the merged permissions
naturally cover owner,auditor / admin,auditor and custom roles with
audit:read, while hiding owner/admin alone.
audit:read is not referenced anywhere in the codebase outside
ROUTE_PERMISSIONS.auditor, so dropping it is safe. Owner/admin still
retain audit:create and audit:update for SOA management.
Deletes ~100 lines of special-case code that was previously needed to
distinguish implicit vs explicit audit:read:
- canAccessAuditorView
- resolveCustomRolePermissions
- resolveAuditorViewAccess
- requireAuditorViewAccess
- customPermissions plumbing through AppShellWrapper / AppSidebar /
search groups / MainMenu / usePermissions
Tests updated to exercise the full resolution path
(resolveBuiltInPermissions + canAccessRoute).
…ditor View gating" This reverts commit c4a4da5.
…rization fix(policies): remove stray brace breaking nest build
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
…mission-gating fix(rbac): gate Auditor View on audit:read instead of role string (CS-189)
Contributor
|
🎉 This PR is included in version 3.26.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is an automated pull request to release the candidate branch into production, which will trigger a deployment.
It was created by the [Production PR] action.
Summary by cubic
Deploy the release to production. Includes CS-189 RBAC updates to gate the Auditor View and a fix for an API build error.
auditorrole or a custom role that grantsaudit:read; added a server-side guard and tests, and updated sidebar, search, and menu to use this flag.apps/api/src/policies/policies.service.tsthat broke the build.Written for commit fa16795. Summary will update on new commits.