feat(permissions): enforce the built-in user role (remove its bypass)#3656
Open
viktormarinho wants to merge 6 commits into
Open
feat(permissions): enforce the built-in user role (remove its bypass)#3656viktormarinho wants to merge 6 commits into
viktormarinho wants to merge 6 commits into
Conversation
Until now the built-in `user` role bypassed all permission checks via `createBoundAuthClient.hasPermission`, which returned true for every member of BUILTIN_ROLES. So UI capability gating only truly bit custom roles; a `user` had full server-side access. Flip the runtime bypass to ADMIN_ROLES (owner/admin) only. A `user` is now enforced like any member: it receives basic-usage (granted out-of-band in AccessControl) plus its explicit Better Auth / connection grants, and nothing else. This is the capstone of the capability-RBAC work; basic-usage was rounded out first (#3654) so normal members keep working after the flip. Mechanics that make this safe: - AccessControl already bypassed only admin/owner (access-control.ts); the unit tests already model `user` as enforced. This aligns the second bypass site with the first. - fetchRolePermissions still treats `user` as built-in (no organizationRole row), returning undefined → its Better Auth role grant is consulted. - The Better Auth `user` role is `self: ["*"]`; authorize() matches actions literally (the reason owner/admin enumerate the full tool list), so `["*"]` grants no specific tool → gated tools are denied. - The MY_CAPABILITIES endpoint already reports all-false for `user`, so the UI was already restricted; this just makes the server match. Comments updated at all three sites (roles.ts, context-factory.ts, auth/index.ts) to reflect that built-in != bypass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
🧪 BenchmarkShould we run the Virtual MCP strategy benchmark for this PR? React with 👍 to run the benchmark.
Benchmark will run on the next push after you react. |
Contributor
Release OptionsSuggested: Minor ( React with an emoji to override the release type:
Current version:
|
Contributor
There was a problem hiding this comment.
1 issue found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/mesh/src/core/context-factory.ts">
<violation number="1" location="apps/mesh/src/core/context-factory.ts:271">
P1: `user` is still effectively granted wildcard `self` access via the explicit wildcard fallback, so the intended privilege reduction is not fully enforced.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| // built-in `user` role is enforced like any member: it gets basic-usage | ||
| // (granted out-of-band in AccessControl) plus its explicit Better Auth / | ||
| // connection grants, and nothing else. See ADMIN_ROLES in auth/roles.ts. | ||
| if (role && ADMIN_ROLES.includes(role as (typeof ADMIN_ROLES)[number])) { |
Contributor
There was a problem hiding this comment.
P1: user is still effectively granted wildcard self access via the explicit wildcard fallback, so the intended privilege reduction is not fully enforced.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/mesh/src/core/context-factory.ts, line 271:
<comment>`user` is still effectively granted wildcard `self` access via the explicit wildcard fallback, so the intended privilege reduction is not fully enforced.</comment>
<file context>
@@ -264,11 +264,11 @@ export function createBoundAuthClient(ctx: AuthContext): BoundAuthClient {
+ // built-in `user` role is enforced like any member: it gets basic-usage
+ // (granted out-of-band in AccessControl) plus its explicit Better Auth /
+ // connection grants, and nothing else. See ADMIN_ROLES in auth/roles.ts.
+ if (role && ADMIN_ROLES.includes(role as (typeof ADMIN_ROLES)[number])) {
return true;
}
</file context>
The bypass flip alone left the privilege reduction incomplete: the
built-in `user` role is defined with `self: ["*"]`, and
`createBoundAuthClient.hasPermission` falls back to an explicit
`{ self: ["*"] }` wildcard probe when the exact check misses. That probe
matches the user role's own `self: ["*"]`, so a member was still granted
every `self` tool — re-introducing the full access we just removed.
Root cause is the role definition, not the bypass. Define the built-in
`user` role with `self: []` so neither the exact check nor the wildcard
fallback matches any gated tool. Basic-usage is granted out-of-band in
AccessControl, so legitimate member usage is unaffected.
Add an e2e regression test: a member left on the built-in `user` role
gets basic-usage (AUTOMATION_LIST) but is denied a gated tool
(MONITORING_STATS). The existing test only covered a custom role with an
empty permission set, which never held the `self` wildcard — so this
exact path was untested.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The built-in `user` role spread `...adminAc.statements` (a copy of the admin/owner definitions). Those org statements — organization:update, member:create/update/delete, invitation:create/cancel, team:*, ac:* — gate Better Auth's NATIVE org-plugin endpoints (update org, invite/remove members, update member roles, create roles, cancel invitations). So a plain member could manage the org through those endpoints, independent of the MCP-tool bypass we just removed (AccessControl only checks self/connection buckets, so MCP tools were unaffected — but the native endpoints enforce on these statements directly). Spread `...memberAc.statements` instead — the org plugin's member role, which grants only `ac: ["read"]` (read roles for the UI) and no org management. owner/admin keep adminAc. Extend the built-in-user e2e: the member is now also denied invite-member (requires invitation:["create"], which memberAc omits and adminAc granted). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…role Add a single source of truth, USER_ROLE_TOOLS in registry-metadata, derived from USER_ROLE_CAPABILITY_IDS — the gated capabilities granted to every member beyond basic-usage. Empty by default, so behavior is unchanged; flipping one capability id on grants all its tools to every member of every org (no migration). Wired through the two layers that must stay in sync: - Enforcement: the built-in `user` role's `self` grant is now `[...USER_ROLE_TOOLS]` (auth/index.ts). Specific tool names only — never `"*"` — so the `createBoundAuthClient` wildcard fallback can't leak. - UI gating: the MY_CAPABILITIES endpoint resolves the built-in `user` role from USER_ROLE_TOOLS instead of the (empty) organizationRole row, so affordances match what the server allows. For per-org or per-member grants, custom roles remain the mechanism; this constant is global to the built-in user role. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Set USER_ROLE_CAPABILITY_IDS = ["agents:manage"], so every member can
create/update/delete agents and edit plugin config / pinned views — via
both layers already wired up: the user role's `self` grant (enforcement)
and the MY_CAPABILITIES endpoint (UI gating).
e2e updates:
- basic-usage-grant: the built-in user role can create an agent
(COLLECTION_VIRTUAL_MCP_CREATE succeeds), while a restrictive custom
role with only basic-usage is denied the same call. Sent with valid
`data` so the custom-role denial is an access error, not validation.
- my-capabilities: the user role now resolves agents:manage = true, every
other gated capability false.
- connections-agents-monitor-gating: the "Create Agent" affordance is now
shown for a plain member (still can't manage connections or view
monitoring).
Verified the enforcement + gating logic locally: Better Auth authorize()
grants the agent tools to the user role and still denies monitoring/org
tools, the `{ self: ["*"] }` wildcard probe, and org-plugin statements;
resolveCapabilities lights up only agents:manage.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add "connections:manage" to USER_ROLE_CAPABILITY_IDS alongside
agents:manage, so every member can create/update/delete connections too —
via the same two wired layers (user role `self` grant + MY_CAPABILITIES).
e2e updates:
- basic-usage-grant: the built-in user role can now create a connection
(COLLECTION_CONNECTIONS_CREATE succeeds; the unreachable URL is swallowed
server-side), while a basic-only custom role is denied the same call.
- my-capabilities: the user role resolves agents:manage + connections:manage
true, every other gated capability false.
- connections-agents-monitor-gating: the "Custom Connection" affordance is
now shown for a plain member (still can't view monitoring).
Verified locally: Better Auth authorize() grants the connection + agent
tools to the user role and still denies monitoring/member tools and the
`{ self: ["*"] }` wildcard probe; resolveCapabilities lights up only
connections:manage + agents:manage.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
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.
Summary
Capstone of the capability-RBAC work. Until now the built-in
userrole bypassed all permission checks server-side, so UI capability gating only truly bit custom roles — auserhad full org access via the API.This flips the runtime bypass to
ADMIN_ROLES(owner/admin) only. Auseris now enforced like any member: it gets basic-usage (granted out-of-band inAccessControl) plus its explicit Better Auth / connection grants, and nothing else.Prerequisite (#3654, merged) rounded out
basic-usageto cover every normal-member flow first, so members keep working after the flip.The change
One functional line —
createBoundAuthClient.hasPermissionincontext-factory.ts:Plus comment fixes at the three sites that previously implied built-in == bypass (
roles.ts,context-factory.ts,auth/index.ts).Why this is safe
user.AccessControl.checkResource(access-control.ts) only ever bypassedadmin/owner; its unit tests already modeluseras enforced. This aligns the second site (createBoundAuthClient) with the first.fetchRolePermissionsstill treatsuseras built-in (noorganizationRolerow) → returnsundefined→ its Better Auth role grant is consulted.userrole isself: ["*"], andauthorize()matches actions literally. That's the documented reason owner/admin must enumerate the full tool list (creatorSelf). So["*"]authorizes no specific tool → gated tools are denied; only basic-usage (granted separately) remains.MY_CAPABILITIESendpoint already reports all-false capabilities for auser(my-capabilities.spec.tsassertsvalues.every(v => v === false)), andsettings-capability-gating.spec.tsalready drives auseras a restricted member. This change makes the server match what the UI already shows.Testing
bun run check✅ ·bun run lint✅ (0/0) ·bun run knip✅ ·bun run fmt✅bun testonaccess-control/mesh-context/define-tool✅ (53 pass) — these already encodeuseras enforced.my-capabilities,settings-capability-gating,create-scoped-role) already cover the restricted-userbehavior end-to-end; they require Postgres+NATS infra to run locally.No new unit test added: the flip lives in
createBoundAuthClient, which depends on the real Better AuthhasPermission(integration territory, already covered by the e2e specs); a mock-only unit test would be brittle per TESTING.md.Rollout note
This is a real privilege reduction for the
userrole. Verify in staging that normal members can still chat, view agents/connections, use the home/shell, file picker, and credits banner (all covered by basic-usage) before promoting.🤖 Generated with Claude Code
Summary by cubic
Enforces permission checks for the built-in "user" role and grants
agents:manageandconnections:manageviaUSER_ROLE_TOOLS, so members can manage agents and connections but not other gated areas. Only "owner" and "admin" keep the bypass; this closes wildcard leaks and aligns server rules with UI gating.Bug Fixes
ADMIN_ROLES(owner/admin); enforceuser.userwithself: [...USER_ROLE_TOOLS](no"*") and spreadmemberAcorg statements to block native org management.usergets basic-usage, can’t view monitoring or org-admin endpoints; restricted custom roles can’t manage agents/connections.New Features
USER_ROLE_TOOLS(fromUSER_ROLE_CAPABILITY_IDS) as the source of truth for gated tools granted touser; both enforcement andMY_CAPABILITIESresolve from it.USER_ROLE_CAPABILITY_IDS = ["agents:manage", "connections:manage"]: members can create/manage agents and connections; other gated capabilities remain denied and UI shows Create Agent and Custom Connection.Written for commit 7b23f7c. Summary will update on new commits.