Skip to content

feat(permissions): delegated dashboard access + route-derived permission registry - #66

Open
Abdulkhalek-1 wants to merge 36 commits into
mainfrom
feat/delegated-dashboard-access
Open

feat(permissions): delegated dashboard access + route-derived permission registry#66
Abdulkhalek-1 wants to merge 36 commits into
mainfrom
feat/delegated-dashboard-access

Conversation

@Abdulkhalek-1

Copy link
Copy Markdown
Member

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).

User requirePermissions: false requirePermissions: true
Owner * *
Live guild admin * assignments ∪ isDefault roles ∪ overrides
Member with grants their grants their grants
Member without grants / non-member

Two rules carry the safety of this change:

  • requirePermissions constrains admins only — it never gates explicit grants.
  • isDefault roles 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.

requireGuildAdminrequireGuildAccess (authorizes on a non-empty resolved permission set), renamed across 21 route files. The four Discord lookup routes that backed the pickers now sit behind dashboard.lookups.view.

Two escalation holes closed

Both were reachable only by MANAGE_GUILD admins before this branch, which is why they survived — delegated access turns them into real paths.

  1. Role assignment (POST .../members) checked nothing: a dashboard.roles.manage holder could assign themselves an existing Full Admin role.
  2. isDefault promotion ran its escalation check inside if (body.permissions) while applying isDefault unconditionally — so PUT {"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 / resolveEffectivePermissions take the vocabulary as a parameter.

Boot validation runs in createApp(); since nothing in the suite boots the app, scanDeclaredPermissionKeys scans the route sources so CI catches an unknown module or bad action verb.

Also

  • The permission grid was the last untranslated surface — now fully translated across all 48 locales.
  • Overview no longer spins forever for users without actions.analytics.view (a pre-existing bug for restricted admins), and shows them what they can reach instead.
  • The role editor disables permissions the editor cannot themselves grant, and warns when a role can configure modules but not use pickers.

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-lockfile clean · 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

Abdulkhalek-1 and others added 30 commits July 28, 2026 16:23
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>
Abdulkhalek-1 and others added 6 commits July 28, 2026 23:35
…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>
@Abdulkhalek-1

Copy link
Copy Markdown
Member Author

Added: role member assignment UI

The 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 INSERT, which made the feature unusable as shipped.

The role editor now has a Members section:

  • Search-and-add with avatars, over the existing debounced member-search endpoint.
  • Provenance on every row — who granted the access and when, since this is a permission-granting surface.
  • Applies immediately. Adding grants access at that moment; removing revokes it. Not deferred to the Save button, which owns name/colour/permissions — an access grant shouldn't be a side effect of a form save, and each action gets its own audit-log entry.
  • Undo on removal, matching the automation UI's delete→Undo toast, so one misclick doesn't silently revoke someone's access.
  • Gated to mirror the server. The add control is disabled with an explanatory tooltip when you don't hold everything the role grants, and you can't select yourself — so you learn before the request rather than via a raw 403.

Two review rounds went into that last point. The self-exclusion first depended on useAuth() having resolved, and the page's loading gate doesn't wait for it — so on a hard reload a non-owner could briefly select themselves. Gating on isLoading fixed the loading race but not the errored one: useAuth uses retry: false, so a failed fetch settles at isLoading: false, data: undefined and sailed straight through. Both call sites now share one callerIdKnown flag and fail closed on loading, errored, and null-body alike.

Also extracted the duplicated member-search UI into a shared member-search-list consumed by both this picker and the existing MemberMultiSelect, whose rendered output is unchanged (the automation trigger-filter suite passes untouched).

Verification: typecheck 14/14 · unit 9/9 tasks (dashboard 1551, bot 428, systems 346) · integration 77 · --frozen-lockfile clean · no new dependency · new strings translated across all 48 locales.

🤖 Generated with Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant