feat(permissions): delegated dashboard access + route-derived permission registry - #66
feat(permissions): delegated dashboard access + route-derived permission registry#66Abdulkhalek-1 wants to merge 36 commits into
Conversation
Lets guild members with explicit dashboard grants see and manage a server without Discord MANAGE_GUILD, and derives the permission registry from the route table instead of a hand-maintained constant. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
15 TDD tasks covering non-admin grant resolution, the requireGuildAccess gate, the role-assignment escalation fix, the route-table-derived registry, and the overview landing for delegated users. Also corrects the spec: escalation guards already exist on role create, update, preset create, and user overrides — only role assignment is unguarded. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…he existing resolve test Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
resolveUserPermissions now calls getGuildAuthority instead of isUserGuildAdmin + getGuildOwnerId, and no longer short-circuits to an empty set for non-admin members. A member with a DashboardRoleAssignment or DashboardUserPermission row now gets those grants regardless of the requirePermissions toggle, which continues to govern only whether admins are constrained. isDefault roles still merge only on the admin path, so enabling the setting never admits the whole server at once. ResolvedPermissions gains isGuildMember; middleware.test.ts mocks updated to stay type-clean.
Review findings on the requireGuildAccess rename:
- Type createMockRequest's return (MockRequest, with an optional
resolvedPermissions: ResolvedPermissions field) so tests can read
request.resolvedPermissions directly instead of casting it back with
`as`. Cleaned up two pre-existing reads of the same kind while here.
- "rejects a member holding no grants" and "rejects a non-member" both
use an empty permission set, which 403s identically under the old
has("*") gate and the new size>0 gate — they're regression guards,
not proof of the rewrite. "allows a non-admin member holding explicit
grants" already covers the discriminating case (non-empty set, no
"*"); renamed it and added a comment explaining why it's the one that
actually distinguishes old from new behavior, instead of adding a
duplicate.
…oughs Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Assert hasPermission was called with the exact "dashboard.lookups.view" key (not just that some 403 occurred) and cover all four Discord passthrough routes via it.each, not just /channels. Also drops a session guilds:[] override that played no role in the outcome. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
validatePermissionRegistry only ran inside createApp(), which no test calls (it hits a real DB and process.exits on missing config), so a route declaring an unknown module or bad action verb passed pnpm test clean and only broke a real boot. Add scanDeclaredPermissionKeys(), which statically extracts every requirePermission(...) literal from the route sources, and assert it validates and fully survives buildPermissionRegistry — the same check now runs in CI without booting anything. Also replace the "groups keys by module in MODULE_META order" test: its dashboard/tickets pair happened to agree both alphabetically and by MODULE_META.order, so it would still pass with the module sort deleted. moderation/actions genuinely disagrees between the two orderings.
The literal-key drift check filtered out every wildcard entry, so ROLE_PRESETS wildcards (moderation.*, welcome.*, etc. — nearly all of the preset data) had no coverage left after the static PERMISSION_REGISTRY-based "all preset permissions are valid" test was removed. Add a case that expands each preset's wildcard entries against the scanned declared-key vocabulary and fails naming the dead preset:pattern pair, so a stale wildcard (e.g. after a module rename) is caught instead of silently granting zero permissions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An empty grid was indistinguishable from a role having no permissions available, so a failed fetch silently disabled the permission editor. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
vi.hoisted's factory now declares its return type instead of casting the initial `data: undefined` value, per the project's no-`as` rule for test files. Also re-indents the permission grid JSX to match its new nesting inside the loading/error/empty ternary (no logic change). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…bels Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…test infra The bot compose service doesn't bind-mount the root lockfile, so adding vitest to packages/i18n required a Docker-side lockfile regeneration that resolved a narrower dependency graph than the committed one (dropped the vite@7.3.1 peer entries used elsewhere), breaking `pnpm install --frozen-lockfile`. Revert packages/i18n/package.json and pnpm-lock.yaml to their pre-task-11 state and delete packages/i18n/vitest.config.ts. Move the locale-coverage test to apps/dashboard/tests/i18n/permission-keys.test.ts instead — the dashboard is the only consumer of the permissions namespace, already has vitest wired and runs in CI, and the test only reads JSON off disk so it needs no new dependency. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The sr/permissions.json file's pre-existing convention is Serbian Latin (no diacritics: "Naziv uloge", "Moderacija", "Podesavanja servera"). The strings added by task 11 were Serbian Cyrillic, so the role editor would render mixed-script UI (Cyrillic permission labels next to Latin chrome). Re-translate permissionCategories.dashboard, all 19 resources values, all 4 permissionActions values, and roleEditor.permissionLabel/registryError/ registryEmpty into Latin, reusing the file's own established vocabulary where it already exists (e.g. resources.audit = "Revizijski zapis" to match tabs.auditLog; resources.panels = "Paneli" to match permissionCategories.roles "Paneli uloga"; permissionActions.purge = "Brisanje" to match the existing delete-flow strings). Audited all other 47 non-English locales for the same class of mistake (script/register mismatch between newly-added and pre-existing strings) — sr was the only real defect. ja flagged in an automated per-character script scan but is a false positive: Japanese naturally mixes Kanji/ Katakana/Hiragana within a single sentence, and the added ja strings match the file's existing mixed usage exactly (e.g. resources.audit "監査ログ" is byte-identical to the pre-existing tabs.auditLog value). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
AccessSummary's t() defaulted to the "overview" namespace, so nav links rendered raw keys like "nav.tickets" instead of translated labels in every locale. Prefix with common: and strengthen the OverviewPage tests to assert the prefixed key and full three-state exclusivity. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ps warning Alert was missing variant="warning", so the container fell back to the neutral default style with only the icon signaling severity. Also add render tests for the warning Alert, the wildcard-covers-lookups case (proves matchPermission, not Set.has, drives the check), the Grant picker access button actually clearing the warning, and the dashboard.roles.manage admission note appearing/not appearing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Calls the real resolveUserPermissions() against a real Postgres test DB, mocking only the Discord API layer (owner/admin/member/non-member), so the tests fail if delegated resolution breaks rather than only asserting Prisma round-trips. Covers: a non-admin role assignment resolving to exactly that role's permissions; identical resolution regardless of requirePermissions for non-admins; per-user override merging; isDefault roles applying only to admins under requirePermissions; non-members with stale grant rows resolving empty; and cache invalidation after an assignment is removed.
The original gate model said MANAGE_GUILD was required to reach the dashboard at all. Authority now comes from ownership, live Discord admin rights, or an explicit dashboard grant. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
PUT /api/guilds/:guildId/dashboard-roles/:roleId applied `isDefault`
unconditionally while the escalation check only ran inside
`if (body.permissions)`. A delegated `dashboard.roles.manage` holder
could send `{"isDefault": true}` alone to promote any existing role
(e.g. one holding `*`) to the guild default, granting its permissions
to every live Discord admin with no permissions field in the request
to catch.
Promoting a role to default is equivalent to granting everyone its
permissions, so run the same matchPermission loop used by the other
four guards in this file, but against the role's existing persisted
permissions (parsed with safeParsePermissions) since the request body
may not include `permissions` at all. Demotion (isDefault: false)
cannot escalate and stays unguarded.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…rantable checkboxes
Two related permissions-page fixes surfaced in final review:
- useDashboardSettings requires dashboard.settings.manage, but the page
itself is reachable with just dashboard.roles.view (e.g. the built-in
Viewer preset). On a 403 there, `settings` was undefined and the code
silently fell back to requirePermissions = false, rendering "The
permission system is disabled... all admins have full access" on a
guild where it is actually enabled — a false statement about the
guild's security posture shown to the exact user class this branch
admits. The status card and disabled banner now render only once the
settings fetch actually succeeds; nothing is guessed. The Create Role
button and Audit Log tab also rendered unconditionally and 403'd on
use — both now gate on can("dashboard.roles.manage") /
can("dashboard.audit.view").
- The role editor's permission grid only ever disabled checkboxes for
hasWildcard; a delegated user could tick a box for a permission they
don't hold, save, and get a raw untranslated server error. Each
checkbox is now also disabled when the current user is not the owner
and doesn't hold that permission themselves (via the client
matchPermission against usePermissions(guildId)'s own resolved set),
with a tooltip explaining why.
New tooltip copy (roleEditor.cannotGrantTooltip) is translated in all
48 locales; sr stays Latin-script per convention.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tGuildAuthority arg order safeJsonParse<T>(json, fallback) in permissions.ts used `JSON.parse(json) as T` and only caught a throw, so syntactically-valid-but-wrong-shaped JSON (e.g. `"5"`, which parses to the number 5) was returned uncast and then iterated by loadGrantedPermissions's `for...of` — a string would iterate per character, a number throws a TypeError inside requireGuildAccess, locking the requesting user out of the guild on a malformed DB row instead of failing safe to no permissions. roles-routes.ts already had the correct shape as a local safeParsePermissions (unknown + Array.isArray + a string filter, no cast). Promoted it to permissions.ts as the one exported implementation and pointed both call sites at it, deleting the duplicate. Also: resolveUserPermissions(userId, guildId) calls getGuildAuthority(guildId, userId) — the two functions have inverted parameter orders, both plain strings, so the compiler can't catch an accidental swap. Added a toHaveBeenCalledWith assertion pinning the correct order. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- isUserGuildAdmin (guildAuthz.ts) had zero production callers left — Tasks 2-3 moved every call site onto getGuildAuthority directly. Deleted the function and its dedicated test block (its behavior overlaps what getGuildAuthority's own tests already cover); removed the now-stale isUserGuildAdmin stub from resolveUserPermissions.test's guildAuthz.js mock, since permissions.ts no longer imports it either. - ensureFreshGuilds (session.ts) had no callers at all, production or otherwise, once requireGuildAccess moved onto the live getGuildAuthority check instead of session-cached guilds. Deleted it along with its now-unused FRESH_GUILD_THRESHOLD constant and test block. - docs/features/dashboard-permissions.md described PERMISSION_REGISTRY, PermissionDefinition, and PermissionModule living in packages/types — all deleted by this branch. Rewrote that section to describe the runtime-derived registry (getDeclaredPermissions in middleware.ts, buildPermissionRegistry/validatePermissionRegistry in permissionRegistry.ts, served by GET .../permission-registry). - Recorded two deliberate deviations from the design spec: the emitted i18n key shapes reuse the existing permissionCategories/ permissionActions namespaces instead of the spec's modules.*/actions.*, and per-permission descriptions were dropped in favor of the raw dotted key; validatePermissionRegistry always throws rather than logging in production, chosen deliberately as fail-fast. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…lation check
Fix 3 disabled per-permission checkboxes the caller can't grant but
left the module-level "select all" checkbox unguarded. A delegated
user holding only tickets.* could tick "select all" on the moderation
module, see it checked with no tooltip, save, and get a raw
untranslated 403 from the server guard — the exact failure Fix 3
exists to prevent. Server-side this was always safe (the write is
rejected); it was a UX hole, not an escalation.
toggleModuleWildcard's ADD path only ever runs when the wildcard key
itself isn't already present — even when every individual permission
in the module happens to already be granted, clicking still swaps
those for the single wildcard key, which the server validates as its
own permission via matchPermission. So `hasWildcard` (not `allGranted`)
is the only state that determines whether a click adds or removes;
disabling is gated on the same expression the server checks —
matchPermission(myPermissionSet, `${mod.key}.*`) — and only for the
add direction. Removing an already-present wildcard is de-escalation
and stays unguarded, same reasoning as isDefault: false in Fix 1.
Wraps the disabled state in the existing Tooltip pattern, reusing
roleEditor.cannotGrantTooltip (no new i18n key).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds the missing activation path for delegated dashboard access: a Members section in the role editor to add/remove role assignments, applying immediately rather than deferring to Save. Reuses useRoleMembers/ useAssignRoleMember/useRemoveRoleMember and useMemberSearch/useMembersByIds end to end. - New RoleMemberPicker (single-select add control), reusing useMemberSearch and a debounce hook extracted from MemberMultiSelect into a shared useDebounced hook. - Provenance rows (avatar, resolved display name, who assigned + when), with graceful fallback to the raw id when the assigner no longer resolves. - Add control mirrors the server's assignment escalation check exactly (role.permissions vs. the caller's permissions via matchPermission), gated with the same Tooltip/cannotGrantTooltip pattern as the permission checkboxes above it; the current user is excluded from their own add options for non-owners so the server's self-assign 403 is unreachable. - Explicit success/error toasts on both mutations (no bare mutateAsync). - Loading/error/empty states for the member list, mirroring the existing permission-registry three-state handling in this file. - 15 new tests covering rendering, provenance fallback, all three list states, both mutations' exact call args, the escalation gate, self- exclusion, and failed-mutation error surfacing. - 12 new i18n keys translated in all 48 locales. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…earch UI Addresses two review findings against the previous role-member-assignment commit: - The add control's self-exclusion relied solely on filtering results by currentUser.userId, but useAuth() is an independent query with no route loader forcing it to resolve before RoleEditor mounts. On a hard reload or deep link, a non-owner could render with their identity still unknown and get a usable (unfiltered) add control — the server still 403s, but that's the "explained via a raw 403" outcome the brief said to make unreachable. Now cannotAssignRole also fails closed while useAuth is loading, for non-owners. - RoleMemberPicker had copied MemberMultiSelect's debounced search input and results-list rendering wholesale instead of just its debounce hook. Extracted the shared part into shared/ui/member-search-list.tsx (search input + loading/hint/no-results/option-list), parameterised by labels and the per-option select handler; both pickers now consume it. Automation's ConditionsEditor (which renders MemberMultiSelect for trigger filters) is unaffected — its own test suite stays green. Also added an Undo affordance to the member-removal toast, matching the existing rule-delete pattern in rules.tsx exactly: the removal still fires immediately, but the success toast's Undo action re-assigns the same role/user pair rather than deferring the request. 2 new i18n keys (toast.memberRestored, reusing the existing common:actions.undo label) translated in all 48 locales. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ot loading state Follow-up to 973028d: gating cannotAssignRole/addExcludeIds on useAuth's isLoading only closed the "still fetching" window. useAuth is a plain useQuery with retry:false, so a non-401 fetch error settles at isLoading:false, data:undefined — the same shape as "never fetched yet". In that state the previous fix's isLoading check was false, so a non-owner who held every permission the role grants got a usable, unfiltered add control: the same self-assign reachability the finding was about, through a different door. Replace the loading check with a single derived callerIdKnown flag (owner, or a resolved currentUser.userId) that both the disable check and the self-exclusion list read, so the two conditions cannot drift apart again. Fails closed identically whether the identity fetch is loading, errored, or settled with a null/undefined body. Added a regression test for the errored-settled case (previously unreachable in tests) and kept the existing loading-state tests passing. No new user-facing string; pure logic change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Added: role member assignment UIThe branch as first pushed had a gap — there was no way to actually give someone a dashboard role. The API, hooks, and member-search endpoint all existed, and the role editor even showed a member count, but nothing rendered an add/remove control. The only activation path was a hand-written SQL The role editor now has a Members section:
Two review rounds went into that last point. The self-exclusion first depended on Also extracted the duplicated member-search UI into a shared Verification: typecheck 14/14 · unit 9/9 tasks (dashboard 1551, bot 428, systems 346) · integration 77 · 🤖 Generated with Claude Code |
Lets a guild member holding an explicit dashboard grant use the dashboard without Discord
MANAGE_GUILD, and replaces the hand-maintained permission registry with one derived from the route table.Supersedes the original gate model in
docs/features/dashboard-permissions.md("Non-MANAGE_GUILD access: No"). Design:docs/superpowers/specs/2026-07-28-delegated-dashboard-access-design.md.Access model
Authority now comes from three sources: guild ownership, live Discord admin rights (checked against the bot's view, never the cached OAuth snapshot), or an explicit grant (
DashboardRoleAssignment/DashboardUserPermission).requirePermissions: falserequirePermissions: true***isDefaultroles ∪ overridesTwo rules carry the safety of this change:
requirePermissionsconstrains admins only — it never gates explicit grants.isDefaultroles apply to admins only. If they applied to members, enabling the toggle would hand every member of the server whatever the default role holds. That is one line (includeDefaultRoles: authority.isAdmin) with a test asserting the negative directly.requireGuildAdmin→requireGuildAccess(authorizes on a non-empty resolved permission set), renamed across 21 route files. The four Discord lookup routes that backed the pickers now sit behinddashboard.lookups.view.Two escalation holes closed
Both were reachable only by
MANAGE_GUILDadmins before this branch, which is why they survived — delegated access turns them into real paths.POST .../members) checked nothing: adashboard.roles.manageholder could assign themselves an existing Full Admin role.isDefaultpromotion ran its escalation check insideif (body.permissions)while applyingisDefaultunconditionally — soPUT {"isDefault": true}alone promoted a*role to guild default, granting every admin*.Both now validate against the role's existing persisted permissions. Demotion stays unguarded (it cannot escalate).
Permission registry
requirePermission()self-registers every key it enforces at route-registration time, so the route table is the registry — a key cannot appear in the UI without a route behind it. The 305-line static list is deleted;expandWildcard/resolveEffectivePermissionstake the vocabulary as a parameter.Boot validation runs in
createApp(); since nothing in the suite boots the app,scanDeclaredPermissionKeysscans the route sources so CI catches an unknown module or bad action verb.Also
actions.analytics.view(a pre-existing bug for restricted admins), and shows them what they can reach instead.Verification
typecheck 14/14 · unit 9/9 tasks (dashboard 114 files / 1532 tests, bot 428, systems 346) · integration 9 files / 77 tests ·
pnpm install --frozen-lockfileclean · dashboard boots with registry validation passing.Not covered by automation: the live-authority path needs a manual check with two real Discord accounts — assign a dashboard role to a non-admin and confirm the server appears with the delegated badge, the sidebar filters, and a non-permitted route shows the denied page.
🤖 Generated with Claude Code