Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions apps/mesh/src/auth/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,16 +102,18 @@ const ac = createAccessControl(statement);
// The role-creating built-in roles (owner/admin) must enumerate every `self`
// tool, not just `["*"]`. Better Auth's access-control `authorize()` matches
// actions literally — `["*"]` authorizes only a request for the literal action
// "*", NOT specific tools. Runtime checks bypass this for built-in roles, but
// "*", NOT specific tools. Runtime checks bypass this for owner/admin, but
// `create-role` gates the *creator* on whether they hold each permission they
// grant; with `self: ["*"]` an owner is reported as "missing self:SOME_TOOL"
// and can't create a capability-scoped custom role at all. Enumerating the full
// tool list fixes that.
//
// `user` is intentionally left as-is: it can't create roles
// (allowedRolesToCreateResources = ADMIN_ROLES), and its `self` is never
// consulted at runtime (the built-in-role bypass short-circuits first). Giving
// it the full tool list would only mis-signal that "user" holds full access.
// `user` keeps `self: ["*"]` and is NOT given the full tool list: it is
// enforced at runtime (only owner/admin bypass — see ADMIN_ROLES), so its grant
// IS now consulted. By the literal-match rule above, `["*"]` authorizes no
// specific tool, so a member gets only basic-usage (granted out-of-band in
// AccessControl) plus any connection-scoped grants — never full access. It also
// can't create roles (allowedRolesToCreateResources = ADMIN_ROLES).
const creatorSelf = ["*", ...allTools];

const user = ac.newRole({
Expand Down
10 changes: 7 additions & 3 deletions apps/mesh/src/auth/roles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,18 @@
*/

/**
* Built-in roles that have full access (owner, admin, user)
* These bypass custom permission checks
* Built-in (non-custom) roles. These have no row in the organizationRole table,
* so `fetchRolePermissions` returns no stored permissions for them.
*
* NOTE: being built-in does NOT mean bypassing permission checks. Only
* ADMIN_ROLES (owner/admin) bypass; the `user` role is enforced like any
* member — it receives basic-usage plus its explicit grants and nothing else.
*/
export const BUILTIN_ROLES = ["owner", "admin", "user"] as const;

export type BuiltinRole = (typeof BUILTIN_ROLES)[number];

/**
* Roles that have admin privileges
* Roles with full org access — they bypass all permission checks at runtime.
*/
export const ADMIN_ROLES: BuiltinRole[] = ["owner", "admin"];
20 changes: 11 additions & 9 deletions apps/mesh/src/core/context-factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -264,11 +264,11 @@ export function createBoundAuthClient(ctx: AuthContext): BoundAuthClient {
requestedPermission: Permission,
options?: { organizationId?: string },
): Promise<boolean> => {
// Built-in roles bypass all permission checks
if (
role &&
BUILTIN_ROLES.includes(role as (typeof BUILTIN_ROLES)[number])
) {
// Only owner/admin bypass all permission checks (full org access). The
// 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])) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>

return true;
}

Expand Down Expand Up @@ -439,7 +439,7 @@ export function createBoundAuthClient(ctx: AuthContext): BoundAuthClient {

import { createMCPProxy } from "@/api/routes/mcp-proxy-factory";
import { ConnectionEntity } from "@/tools/connection/schema";
import { BUILTIN_ROLES } from "../auth/roles";
import { ADMIN_ROLES, BUILTIN_ROLES } from "../auth/roles";
import { OrgScopedThreadStorage, SqlThreadStorage } from "@/storage/threads";
import {
OrgScopedAsyncResearchJobStorage,
Expand All @@ -459,15 +459,17 @@ import { DevObjectStorage } from "../object-storage/dev-object-storage";
import { decorateStorageWithAssetHoisting } from "../object-storage/asset-hoister";

/**
* Fetch role permissions from the database
* Returns undefined for built-in roles (they bypass permission checks)
* Fetch role permissions from the database.
* Built-in roles have no row in the organizationRole table, so this returns
* undefined for them. owner/admin additionally bypass all checks at runtime;
* `user` falls through to its (intentionally empty) Better Auth role grant.
*/
export async function fetchRolePermissions(
db: Kysely<Database>,
organizationId: string,
role: string,
): Promise<Permission | undefined> {
// Built-in roles bypass permission checks
// Built-in roles have no custom-role permission row to fetch.
if (BUILTIN_ROLES.includes(role as (typeof BUILTIN_ROLES)[number])) {
return undefined;
}
Expand Down
Loading