diff --git a/apps/mesh/e2e/tests/basic-usage-grant.spec.ts b/apps/mesh/e2e/tests/basic-usage-grant.spec.ts index 489e35b016..c49b488464 100644 --- a/apps/mesh/e2e/tests/basic-usage-grant.spec.ts +++ b/apps/mesh/e2e/tests/basic-usage-grant.spec.ts @@ -9,15 +9,27 @@ * * - A member with a restrictive CUSTOM role (not owner/admin, and whose * stored permission does NOT list the tool) can still call a basic-usage - * tool, yet is denied a non-basic tool it was never granted. This is the - * test that actually exercises the runtime grant. + * tool, yet is denied tools it was never granted — including agents:manage + * and connections:manage tools, which only the built-in user role gets. This + * is the test that actually exercises the runtime grant. + * - A member on the built-in "user" role gets basic-usage AND agents:manage + + * connections:manage (USER_ROLE_CAPABILITY_IDS) — so it can create an agent + * and a connection — but is still denied other gated tools and org + * management via Better Auth's native endpoints (invite-member). Guards two + * regressions: the `self: ["*"]` grant + wildcard fallback re-granting every + * tool, and the `user` role spreading `adminAc` (org-admin statements) + * instead of member-level `memberAc`. * - A NON-member cannot call a basic-usage tool against the org — the grant * must never leak past membership. * - * Tool choices have all-optional input schemas, so a denial surfaces as an - * access error rather than a schema-validation error: - * - AUTOMATION_LIST → basic-usage - * - MONITORING_STATS → NOT basic-usage (monitoring:view capability) + * Tool choices (denials use all-optional input schemas, or valid `data`, so a + * denial surfaces as an access error rather than a schema-validation error): + * - AUTOMATION_LIST → basic-usage + * - MONITORING_STATS → NOT basic-usage (monitoring:view) + * - COLLECTION_VIRTUAL_MCP_CREATE → agents:manage (built-in user role only) + * - COLLECTION_CONNECTIONS_CREATE → connections:manage (built-in user role + * only). Sent with valid `data`; an unreachable URL is swallowed server-side + * so the user's create still succeeds. */ import type { Client } from "pg"; @@ -35,6 +47,39 @@ const toolCallBody = (name: string) => ({ params: { name, arguments: {} }, }); +// COLLECTION_VIRTUAL_MCP_CREATE (agents:manage) with VALID minimal data, so a +// denial is an access error rather than input validation. `title` + an (empty) +// `connections` array are the only required fields. +const agentCreateBody = () => ({ + jsonrpc: "2.0" as const, + id: 1, + method: "tools/call", + params: { + name: "COLLECTION_VIRTUAL_MCP_CREATE", + arguments: { data: { title: "gating probe", connections: [] } }, + }, +}); + +// COLLECTION_CONNECTIONS_CREATE (connections:manage) with VALID minimal data, so +// a denial is an access error rather than input validation. The handler swallows +// an unreachable URL (fetchToolsFromMCP().catch(() => null)), so a granted call +// still creates the row. +const connectionCreateBody = () => ({ + jsonrpc: "2.0" as const, + id: 1, + method: "tools/call", + params: { + name: "COLLECTION_CONNECTIONS_CREATE", + arguments: { + data: { + title: "gating probe conn", + connection_type: "HTTP", + connection_url: "https://example.com/mcp", + }, + }, + }, +}); + test.describe("runtime basic-usage grant", () => { let db: Client; @@ -153,6 +198,169 @@ test.describe("runtime basic-usage grant", () => { ).toBe(true); expect(errText).toMatch(/access denied|permission/i); + // agents:manage is NOT basic-usage and is NOT granted to this custom role, + // so creating an agent is denied. (The built-in user role IS granted + // agents:manage — see the next test — proving this is role-specific.) + const agentRes = await memberCtx.post(`/api/${owner.orgSlug}/mcp/self`, { + data: agentCreateBody(), + headers: MCP_HEADERS, + }); + const agentDenied = (await agentRes.json()) as { + result?: { isError?: boolean; content?: Array<{ text?: string }> }; + error?: { message?: string }; + }; + const agentErr = + agentDenied.result?.content?.[0]?.text ?? + agentDenied.error?.message ?? + ""; + expect( + agentDenied.result?.isError === true || !!agentDenied.error, + `expected COLLECTION_VIRTUAL_MCP_CREATE to be denied for the custom role, got: ${JSON.stringify( + agentDenied, + )}`, + ).toBe(true); + expect(agentErr).toMatch(/access denied|permission/i); + + // connections:manage is likewise NOT granted to this custom role → denied. + const connRes = await memberCtx.post(`/api/${owner.orgSlug}/mcp/self`, { + data: connectionCreateBody(), + headers: MCP_HEADERS, + }); + const connDenied = (await connRes.json()) as { + result?: { isError?: boolean; content?: Array<{ text?: string }> }; + error?: { message?: string }; + }; + const connErr = + connDenied.result?.content?.[0]?.text ?? connDenied.error?.message ?? ""; + expect( + connDenied.result?.isError === true || !!connDenied.error, + `expected COLLECTION_CONNECTIONS_CREATE to be denied for the custom role, got: ${JSON.stringify( + connDenied, + )}`, + ).toBe(true); + expect(connErr).toMatch(/access denied|permission/i); + + await ownerCtx.dispose(); + await memberCtx.dispose(); + }); + + test("the built-in user role gets basic-usage but is denied gated tools", async ({ + playwright, + }) => { + const ownerCtx = await newApiContext(playwright); + const owner = await signUpViaApi(ownerCtx); + const orgRow = await db.query<{ id: string }>( + `SELECT id FROM "organization" WHERE slug = $1`, + [owner.orgSlug], + ); + const orgId = orgRow.rows[0]?.id; + if (!orgId) throw new Error("org not found after signup"); + + // A second user, invited and LEFT on the built-in "user" role (no custom + // role reassignment). This is the path that regressed: the `user` role is + // defined with `self: ["*"]`, and the runtime wildcard fallback would + // otherwise grant it every tool once the owner/admin-only bypass is in + // place. It must get basic-usage only. + const memberCtx = await newApiContext(playwright); + const member = await signUpViaApi(memberCtx); + + const invite = await ownerCtx.post("/api/auth/organization/invite-member", { + data: { organizationId: orgId, email: member.email, role: "user" }, + }); + expect(invite.ok()).toBe(true); + const inviteJson = (await invite.json()) as { + id?: string; + invitation?: { id?: string }; + }; + const invitationId = inviteJson.id ?? inviteJson.invitation?.id; + expect(invitationId).toBeTruthy(); + + const accept = await memberCtx.post( + "/api/auth/organization/accept-invitation", + { data: { invitationId } }, + ); + expect( + accept.ok(), + `accept-invitation failed: ${await accept.text().catch(() => "")}`, + ).toBe(true); + + // Basic-usage tool → granted at runtime regardless of role. + const automations = await callSelfMcpTool<{ automations: unknown[] }>( + memberCtx, + owner.orgSlug, + "AUTOMATION_LIST", + {}, + ); + expect(Array.isArray(automations.automations)).toBe(true); + + // agents:manage IS granted to the built-in user role + // (USER_ROLE_CAPABILITY_IDS) → creating an agent succeeds. callSelfMcpTool + // throws on an access denial, so a returned item proves the grant. + const created = await callSelfMcpTool<{ item: { id: string } }>( + memberCtx, + owner.orgSlug, + "COLLECTION_VIRTUAL_MCP_CREATE", + { data: { title: "user-managed agent", connections: [] } }, + ); + expect(created.item?.id).toBeTruthy(); + + // connections:manage IS granted to the built-in user role too → creating a + // connection succeeds (the unreachable URL is swallowed server-side). + const createdConn = await callSelfMcpTool<{ item: { id: string } }>( + memberCtx, + owner.orgSlug, + "COLLECTION_CONNECTIONS_CREATE", + { + data: { + title: "user-managed conn", + connection_type: "HTTP", + connection_url: "https://example.com/mcp", + }, + }, + ); + expect(createdConn.item?.id).toBeTruthy(); + + // Gated tool (monitoring:view) → denied. Before enforcing the user role, + // the `self: ["*"]` grant + wildcard fallback leaked full access here. + const deniedRes = await memberCtx.post(`/api/${owner.orgSlug}/mcp/self`, { + data: toolCallBody("MONITORING_STATS"), + headers: MCP_HEADERS, + }); + const denied = (await deniedRes.json()) as { + result?: { isError?: boolean; content?: Array<{ text?: string }> }; + error?: { message?: string }; + }; + const errText = + denied.result?.content?.[0]?.text ?? denied.error?.message ?? ""; + expect( + denied.result?.isError === true || !!denied.error, + `expected MONITORING_STATS to be denied for built-in user, got: ${JSON.stringify( + denied, + )}`, + ).toBe(true); + expect(errText).toMatch(/access denied|permission/i); + + // The built-in user role uses member-level org statements (memberAc), not + // adminAc — so Better Auth's native org endpoints reject org management. + // invite-member requires `invitation: ["create"]`, which memberAc omits; + // adminAc would have granted it. + const escalation = await memberCtx.post( + "/api/auth/organization/invite-member", + { + data: { + organizationId: orgId, + email: `escalation-${Date.now()}@example.com`, + role: "user", + }, + }, + ); + expect( + escalation.ok(), + `expected invite-member to be denied for built-in user, got ${escalation.status()}: ${await escalation + .text() + .catch(() => "")}`, + ).toBe(false); + await ownerCtx.dispose(); await memberCtx.dispose(); }); diff --git a/apps/mesh/e2e/tests/connections-agents-monitor-gating.spec.ts b/apps/mesh/e2e/tests/connections-agents-monitor-gating.spec.ts index 67ec3cc6ce..c48a2c6c5f 100644 --- a/apps/mesh/e2e/tests/connections-agents-monitor-gating.spec.ts +++ b/apps/mesh/e2e/tests/connections-agents-monitor-gating.spec.ts @@ -1,10 +1,11 @@ /** * E2E: capability gating of the Connections, Agents and Monitor surfaces. * - * Drives the real browser as a built-in "user" member (no gated capabilities): + * Drives the real browser as a built-in "user" member: * - Monitor is capability-gated (monitoring:view) → no-access panel; - * - Connections + Agents stay viewable (basic-usage), but their create - * affordances are hidden (connections:manage / agents:manage). + * - Connections AND Agents are manageable — the built-in user role is granted + * connections:manage + agents:manage (USER_ROLE_CAPABILITY_IDS), so both + * create affordances are shown. */ import type { APIRequestContext, Page } from "@playwright/test"; @@ -64,7 +65,7 @@ test.describe("connections / agents / monitor gating", () => { await db?.end(); }); - test("a plain member can't manage connections, agents, or view monitoring", async ({ + test("a plain member can't view monitoring, but can manage connections and agents", async ({ page, playwright, }) => { @@ -85,24 +86,25 @@ test.describe("connections / agents / monitor gating", () => { timeout: 15_000, }); - // Connections page loads (viewing is basic-usage) but the create CTA is - // hidden without connections:manage. + // Connections page loads and the create CTA IS shown — the built-in user + // role is granted connections:manage (USER_ROLE_CAPABILITY_IDS). await page.goto(`/${owner.orgSlug}/settings/connections`); await expect(page.getByPlaceholder("Search for a connection")).toBeVisible({ timeout: 15_000, }); await expect( page.getByRole("button", { name: "Custom Connection" }), - ).toHaveCount(0); + ).toBeVisible(); - // Agents page loads but the create CTA is hidden without agents:manage. + // Agents page loads and the create CTA IS shown — the built-in user role + // is granted agents:manage (USER_ROLE_CAPABILITY_IDS). await page.goto(`/${owner.orgSlug}/settings/agents`); await expect(page.getByPlaceholder("Search for an agent...")).toBeVisible({ timeout: 15_000, }); await expect( page.getByRole("button", { name: "Create Agent" }), - ).toHaveCount(0); + ).toBeVisible(); await ownerCtx.dispose(); }); diff --git a/apps/mesh/e2e/tests/my-capabilities.spec.ts b/apps/mesh/e2e/tests/my-capabilities.spec.ts index 5136859f8c..7bdc59fc12 100644 --- a/apps/mesh/e2e/tests/my-capabilities.spec.ts +++ b/apps/mesh/e2e/tests/my-capabilities.spec.ts @@ -98,9 +98,16 @@ test.describe("GET /api/auth/custom/my-capabilities/:slug", () => { const body = (await res.json()) as CapabilitiesResponse; expect(body.role).toBe("user"); - const values = Object.values(body.capabilities); - expect(values.length).toBeGreaterThan(0); - expect(values.every((v) => v === false)).toBe(true); + // The built-in user role is granted agents:manage + connections:manage via + // USER_ROLE_CAPABILITY_IDS; every other gated capability stays false. + expect(body.capabilities["agents:manage"]).toBe(true); + expect(body.capabilities["connections:manage"]).toBe(true); + const granted = new Set(["agents:manage", "connections:manage"]); + const others = Object.entries(body.capabilities) + .filter(([id]) => !granted.has(id)) + .map(([, isGranted]) => isGranted); + expect(others.length).toBeGreaterThan(0); + expect(others.every((isGranted) => isGranted === false)).toBe(true); await ownerCtx.dispose(); await memberCtx.dispose(); diff --git a/apps/mesh/src/api/routes/auth.ts b/apps/mesh/src/api/routes/auth.ts index 1bfbfa4cd2..f4e51582fa 100644 --- a/apps/mesh/src/api/routes/auth.ts +++ b/apps/mesh/src/api/routes/auth.ts @@ -28,6 +28,7 @@ import { import { allCapabilitiesGranted, resolveCapabilities, + USER_ROLE_TOOLS, } from "@/tools/registry-metadata"; const app = new Hono(); @@ -237,9 +238,18 @@ app.get("/my-capabilities/:slug", async (c) => { return c.json({ role, capabilities: allCapabilitiesGranted() }); } - // Any other role (built-in "user" or a custom role) is resolved from its - // stored permission. Built-in "user" has no organizationRole row, so its - // permission is empty and it resolves to no gated capabilities. + // The built-in "user" role has no organizationRole row; its gated grants are + // baked into code (USER_ROLE_TOOLS, empty by default), NOT stored in the DB. + // Resolve from that set so UI gating matches the role's `self` grant in the + // auth config. Empty set → no gated capabilities, same as before. + if (role === "user") { + return c.json({ + role, + capabilities: resolveCapabilities({ self: [...USER_ROLE_TOOLS] }), + }); + } + + // A custom role is resolved from its stored permission. const customRole = await db .selectFrom("organizationRole") .select(["permission"]) diff --git a/apps/mesh/src/auth/index.ts b/apps/mesh/src/auth/index.ts index dd7d12c7ca..5de32bfa3b 100644 --- a/apps/mesh/src/auth/index.ts +++ b/apps/mesh/src/auth/index.ts @@ -10,7 +10,7 @@ */ import { getSettings } from "../settings"; -import { getToolsByCategory } from "@/tools/registry-metadata"; +import { getToolsByCategory, USER_ROLE_TOOLS } from "@/tools/registry-metadata"; import { sso } from "@better-auth/sso"; import { organization } from "@decocms/better-auth/plugins"; import { betterAuth, BetterAuthOptions } from "better-auth"; @@ -31,6 +31,7 @@ import { import { adminAc, defaultStatements, + memberAc, } from "@decocms/better-auth/plugins/organization/access"; import { getConfig } from "@/core/config"; @@ -102,21 +103,32 @@ 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`'s `self` is exactly USER_ROLE_TOOLS — the gated tools granted to every +// member beyond basic-usage (empty by default; see registry-metadata). It is +// enforced at runtime: only owner/admin bypass (see ADMIN_ROLES), so this grant +// IS consulted. It must NEVER contain `"*"` — `createBoundAuthClient` falls back +// to a `{ self: ["*"] }` wildcard probe when the exact check misses, and a `"*"` +// here would satisfy it and hand every member full access (the bypass we removed). +// Specific tool names match only via the exact check. A member otherwise gets +// only basic-usage (granted out-of-band in AccessControl) plus connection-scoped +// grants, and can't create roles (allowedRolesToCreateResources = ADMIN_ROLES). const creatorSelf = ["*", ...allTools]; +// `user` spreads `memberAc` (the org plugin's member role), NOT `adminAc`. These +// org statements (organization/member/invitation/team/ac) gate Better Auth's +// native org-plugin endpoints — not MCP tools, which our AccessControl checks on +// `self`/connection buckets. `adminAc` grants org:update + member/invitation/team +// management; spreading it here would let a plain member manage the org via those +// endpoints. `memberAc` grants only `ac: ["read"]` (read roles for the UI). const user = ac.newRole({ - self: ["*"], - ...adminAc.statements, + self: [...USER_ROLE_TOOLS], + ...memberAc.statements, }) as Role; const admin = ac.newRole({ diff --git a/apps/mesh/src/auth/roles.ts b/apps/mesh/src/auth/roles.ts index e8ac96d649..679eb758a0 100644 --- a/apps/mesh/src/auth/roles.ts +++ b/apps/mesh/src/auth/roles.ts @@ -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"]; diff --git a/apps/mesh/src/core/context-factory.ts b/apps/mesh/src/core/context-factory.ts index c6169207e9..ef892ba4d2 100644 --- a/apps/mesh/src/core/context-factory.ts +++ b/apps/mesh/src/core/context-factory.ts @@ -264,11 +264,11 @@ export function createBoundAuthClient(ctx: AuthContext): BoundAuthClient { requestedPermission: Permission, options?: { organizationId?: string }, ): Promise => { - // 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])) { return true; } @@ -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, @@ -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, organizationId: string, role: string, ): Promise { - // 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; } diff --git a/apps/mesh/src/tools/registry-metadata.ts b/apps/mesh/src/tools/registry-metadata.ts index a119adfdb7..2cb6978ef5 100644 --- a/apps/mesh/src/tools/registry-metadata.ts +++ b/apps/mesh/src/tools/registry-metadata.ts @@ -1322,6 +1322,34 @@ export const BASIC_USAGE_TOOLS: ReadonlySet = new Set( ?.tools ?? [], ); +/** + * Gated capability ids additionally granted to the built-in `user` role, beyond + * basic-usage. EMPTY by default — every member is otherwise enforced down to + * basic-usage only. Add a capability id here (e.g. "agents:manage") to grant ALL + * of its tools to every member of every org. This is a GLOBAL change with no + * migration; for per-org grants use a custom role instead. + */ +const USER_ROLE_CAPABILITY_IDS: string[] = [ + "agents:manage", + "connections:manage", +]; + +/** + * Tools the built-in `user` role gets beyond basic-usage, derived from + * USER_ROLE_CAPABILITY_IDS. Single source of truth for the two layers that must + * stay in sync: + * - enforcement: baked into the `user` role's `self` grant (auth/index.ts) + * - UI gating: the MY_CAPABILITIES endpoint (api/routes/auth.ts) + * + * List specific tool names only — never `"*"` — so the wildcard fallback in + * `createBoundAuthClient` can't be tricked into granting everything. + */ +export const USER_ROLE_TOOLS: ReadonlySet = new Set( + PERMISSION_CAPABILITIES.filter((c) => + USER_ROLE_CAPABILITY_IDS.includes(c.id), + ).flatMap((c) => c.tools), +); + export function getCapabilitySections(): Array<{ section: string; capabilities: PermissionCapability[];