refactor(auth): remove ROLE_PERMISSIONS fallback, fail closed instead - #15
Merged
Conversation
dec-identity-unification Phase 5c's second half: checkInlineEditPermission() and runPluginResponseTransforms()'s pages.update gate both fell back to a flat ROLE_PERMISSIONS table when authz was undefined -- authz creation failing at startup, an in-process object construction that essentially never fails in practice. Every other admin permission check already treats authz.check() as the sole authority; this closes the same gap in these last two call sites. Both now fail closed (deny) instead of silently degrading to a separate, less-audited mechanism. The one place that genuinely can't call the real async authz.check() -- ResponseTransformContext.auth.hasPermission(), a published synchronous hook API plugin-inline-edit already depends on -- now reads roleHasPermission() (new, src/auth/authz-schema.ts), a synchronous lookup against the same canonical actionToRelations schema authz.check() itself uses, instead of consulting plugin-admin's hand-maintained mirror. This is the one legitimate reason a role-only fallback ever needed to exist, now sourced from a single canonical definition instead of two tables that had to be kept in sync by hand. Companion change to @dune/plugin-admin's 3.0.0, which removes ROLE_PERMISSIONS/hasPermission() entirely now that nothing in core consults them. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
checkInlineEditPermissionForSiteUser() still degraded to a raw roles[] check (editor/admin string match) when authz was undefined — the same parallel, less-audited mechanism PR #15 removed everywhere else. Since authz.check() is async and this function is already async, nothing prevents failing closed here too. authz.check() is now the sole authority for every core-side permission decision, no exceptions. Found during security audit of #15 (Finding 1).
…th.hasPermission() against misuse roleHasPermission() is exported from @dune/core/auth/authz-schema, which invites third-party plugins to reintroduce role-table authorization anywhere. Its answer is a synchronous role-only approximation that can diverge from authz.check() (revoked tuples, direct grants, group membership are invisible to it). It stays exported — the published synchronous hook API genuinely needs it — but both its doc and the ResponseTransformContext.auth.hasPermission() contract now state loudly that it is not for access decisions: use authz.check(). Found during security audit of #15 (Finding 2).
…st admin-tier role roles[0] is not necessarily an admin role: the merged User type mixes admin-tier roles with content-gating tags (e.g. ["member", "admin"]) in no guaranteed order. An admin whose tag came first passed the real pages.update authz.check() gate but got auth.role === "member", so the published synchronous hasPermission() under-privileged relative to what the sole authority had just decided (fail-closed direction, but a silent contract regression for the published hook API vs the old highestValidRole() behavior). New highestAdminRole() in @dune/core/auth/authz-schema picks the highest-ranked admin-tier role (admin > editor > author), ignoring tags and unknown roles; arrays with no admin-tier role yield "" as before. Found during security audit of #15 (Finding 3).
…hema
Remove "admin.access" from VALID_PERMISSIONS — it never had a
corresponding action in the canonical actionToRelations schema (it only
ever existed in the removed ROLE_PERMISSIONS table) and is now flagged
as unknown, same as any other nonexistent permission. Update the
hasPermission() lint pattern from the removed two-argument
(authResult, "perm") form to the current single-argument hook API form
(auth.hasPermission("perm")).
Found during security audit of #15 (nits).
highestAdminRole() (added in bdeb897 to fix roles[0] under-privileging ResponseTransformContext) had its rank table private to authz-schema.ts, and authz-schema.ts itself had no export map entry at all -- nothing outside this package could import roleHasPermission()/highestAdminRole() despite roleHasPermission()'s own doc already describing them as importable. Exported ADMIN_ROLE_RANK as the canonical rank table highestAdminRole() itself now reads from, and added the missing "./auth/authz-schema" entry to deno.json's exports map. This lets @dune/plugin-admin's role-utils.ts (companion change, that repo) derive its own ROLE_RANK/highestValidRole() from this instead of keeping a second, hand-maintained copy of the same three numbers -- the exact "two tables kept in sync by convention" pattern the ROLE_PERMISSIONS removal (PR #15) was about eliminating, caught a second time during review of that same PR's follow-up commits. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
It never had an actionToRelations entry — panel access is canThey: "access" on app:admin — and validate.ts already flags it as unknown. Co-authored-by: Cursor <cursoragent@cursor.com>
requirePermission() is authz.check() only (missing authz denies). Update the authz skill, plugin-authoring guards note, admin.authzStore comment, and the authorization doc so operators do not debug a fallback that cannot fire. Co-authored-by: Cursor <cursoragent@cursor.com>
3.0.0 removes hasPermission(); this core no longer calls it. Keep the pins aligned so a site that upgrades the built-in admin plugin does not hit a TypeError on cookie-bearing content requests. deno.lock still resolves 2.x until 3.0.0 is on JSR. Co-authored-by: Cursor <cursoragent@cursor.com>
…commit 91d1efb bumped this to 2155d6e, a commit that only ever existed inside this checkout's own docs/content working tree -- it was never pushed to (or even present in) duneorg/dune-docs, because it was made by editing the submodule checkout directly instead of the canonical /Users/xrs/claude/dune-docs repo. Reverted the pointer back to 7601861 (dune-docs' actual current main tip) and reapplied the same doc edit properly: duneorg/dune-docs#3, not yet merged. Bump the pointer again once that lands. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
zumbrunn
added a commit
that referenced
this pull request
Aug 31, 2026
Defers the @dune/plugin-admin@^3.0 pin bump to a follow-up release — see the CHANGELOG entry for why bumping it now would break every site with the admin panel enabled. Also fixes two stale users.manage test assertions left over from PR #15's own branch (predated the removal in 3336468, never reconciled during merge), and drops the version number from ROADMAP.md's header so it stops going stale every release (the thing #13 just fixed once already).
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
1 participant
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.
dec-identity-unification Phase 5c's second half:
checkInlineEditPermission()andrunPluginResponseTransforms()'spages.updategate both fell back to a flatROLE_PERMISSIONStable whenauthzwas undefined — authz creation failing at startup, an in-process object construction that essentially never fails in practice. Every other admin permission check already treatsauthz.check()as the sole authority; this closes the same gap in these last two call sites. Both now fail closed (deny) instead of silently degrading to a separate, less-audited mechanism.The one place that genuinely can't call the real async
authz.check()—ResponseTransformContext.auth.hasPermission(), a published synchronous hook APIplugin-inline-editalready depends on — now readsroleHasPermission()(new,src/auth/authz-schema.ts), a synchronous lookup against the same canonicalactionToRelationsschemaauthz.check()itself uses, instead of consulting plugin-admin's hand-maintained mirror. This is the one legitimate reason a role-only fallback ever needed to exist, now sourced from a single canonical definition instead of two tables that had to be kept in sync by hand.Companion change to
@dune/plugin-admin's 3.0.0: duneorg/plugin-admin#3, which removesROLE_PERMISSIONS/hasPermission()entirely now that nothing in core consults them. Merge either order.