Skip to content

Commit 440436a

Browse files
fix(auth): authorize API keys solely by their own allowlist (no owner-role widening) (#4180)
* fix(auth): authorize API keys solely by their own allowlist A key's owner role no longer widens it: an API key is authorized strictly by its stored `permissions`. A key an admin scopes to e.g. ORGANIZATION_GET cannot call any other tool — the privilege escalation reported by an agent run (a read-only key was minting admin keys via API_KEY_CREATE). - `AccessControl.checkResource` now dispatches to one of two self-contained codepaths by principal type: an **API-key** principal is decided SOLELY by the key's allowlist (`checkApiKeyAccess` → `checkApiKeyPermission`) — no role, no admin/owner bypass, no basic-usage membership floor, no Better Auth; a **member** principal (session / MCP OAuth / mesh JWT) keeps the existing membership-floor + role-bypass + Better Auth path (`checkMemberAccess`). The flag lives on `boundAuth.isApiKeyPrincipal`, so REST and MCP share one signal. - `createBoundAuthClient.hasPermission` mirrors the split (API key → `checkApiKeyPermission` before any role logic). - Remove the "default permissions" concept. `API_KEY_CREATE` now requires `permissions`, and the three internal minters that relied on the default pass an explicit scope: the per-run agent MCP key and the dev-link key act as the user (`{ "*": ["*"] }`), the org-fs mount uses `{ self: ["*"] }` (its file ops are covered by that scope). "A key without a scope" no longer exists. A key is therefore exactly its allowlist — not allowlist ∪ basic-usage. Internal keys carry a wildcard so they are unaffected; user-scoped keys are tightened to precisely what they were scoped to. Browser sessions, MCP OAuth, and mesh JWTs are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(e2e): an API key is capped at its allowlist regardless of owner role Black-box HTTP regression: an admin-owned key scoped to AUTOMATION_LIST is denied MONITORING_STATS, API_KEY_CREATE, and a basic-usage tool outside its allowlist (403) — a key is a capability, not a member. A wildcard key keeps full access; API_KEY_CREATE without `permissions` is rejected (400). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent f1dacab commit 440436a

13 files changed

Lines changed: 485 additions & 102 deletions

File tree

apps/mesh/src/api/routes/decopilot/dispatch-run.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -397,6 +397,12 @@ async function mintMcpEndpoint(
397397
}> {
398398
const apiKey = await ctx.boundAuth.apiKey.create({
399399
name: apiKeyName,
400+
// The per-run key is the agent's own callback credential — it proxies to
401+
// `/mcp/virtual-mcp/<agentId>` (a `vir_*` resource) and acts on behalf of
402+
// the user for the duration of the run, so it needs full access. With no
403+
// implicit default (auth/index.ts), the scope must be explicit; wildcard
404+
// matches the prior behavior (full access via the admin bypass).
405+
permissions: { "*": ["*"] },
400406
expiresIn: MCP_KEY_TTL_SECONDS,
401407
metadata: {
402408
organization: {
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
import { describe, expect, it } from "bun:test";
2+
import { checkApiKeyPermission } from "./api-key-permissions";
3+
4+
describe("checkApiKeyPermission", () => {
5+
it("grants when the exact (resource, tool) is in the allowlist", () => {
6+
expect(
7+
checkApiKeyPermission(
8+
{ self: ["ORGANIZATION_GET"] },
9+
{ self: ["ORGANIZATION_GET"] },
10+
),
11+
).toBe(true);
12+
});
13+
14+
it("denies a tool outside the allowlist (the escalation we block)", () => {
15+
expect(
16+
checkApiKeyPermission(
17+
{ self: ["ORGANIZATION_GET"] },
18+
{ self: ["API_KEY_CREATE"] },
19+
),
20+
).toBe(false);
21+
});
22+
23+
it("denies when the resource is not granted at all", () => {
24+
expect(
25+
checkApiKeyPermission(
26+
{ self: ["ORGANIZATION_GET"] },
27+
{ conn_123: ["SEND_MESSAGE"] },
28+
),
29+
).toBe(false);
30+
});
31+
32+
it("grants every tool for a resource with a wildcard", () => {
33+
expect(checkApiKeyPermission({ self: ["*"] }, { self: ["ANYTHING"] })).toBe(
34+
true,
35+
);
36+
});
37+
38+
it("honors a wildcard `*` resource granting all tools (full key)", () => {
39+
expect(
40+
checkApiKeyPermission({ "*": ["*"] }, { conn_9: ["SEND_MESSAGE"] }),
41+
).toBe(true);
42+
expect(checkApiKeyPermission({ "*": ["*"] }, { self: ["ANYTHING"] })).toBe(
43+
true,
44+
);
45+
});
46+
47+
it("denies everything for an empty / absent allowlist (fail-closed)", () => {
48+
expect(checkApiKeyPermission({}, { self: ["ORGANIZATION_GET"] })).toBe(
49+
false,
50+
);
51+
});
52+
53+
it("requires ALL requested tools to be covered", () => {
54+
expect(
55+
checkApiKeyPermission(
56+
{ self: ["ORGANIZATION_GET"] },
57+
{ self: ["ORGANIZATION_GET", "API_KEY_CREATE"] },
58+
),
59+
).toBe(false);
60+
});
61+
});
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
/**
2+
* API-key authorization.
3+
*
4+
* An API key is authorized SOLELY by its own stored `permissions` allowlist
5+
* ({ resource: [tools] }) — the owner's org role never widens it. A full-access
6+
* key carries an explicit wildcard (`{ "*": ["*"] }` or `{ self: ["*"] }`); a
7+
* key with no allowlist grants nothing (fail-closed). There is intentionally no
8+
* "default permission" fallback: every key is created with an explicit scope
9+
* (see API_KEY_CREATE and the internal minters), so "a key without a scope"
10+
* does not exist.
11+
*/
12+
13+
import type { Permission } from "../storage/types";
14+
15+
/**
16+
* Does an API key's stored allowlist grant the requested permission?
17+
*
18+
* Both are `{ resource: [tools] }` maps. Every (resource, tool) in `requested`
19+
* must be covered by `granted` — directly, by a per-resource `"*"`, or by a
20+
* wildcard `"*"` resource (itself optionally `"*"` for all tools). Returns false
21+
* on the first uncovered entry (fail-closed); an empty/absent allowlist grants
22+
* nothing.
23+
*/
24+
export function checkApiKeyPermission(
25+
granted: Permission,
26+
requested: Permission,
27+
): boolean {
28+
for (const [resource, tools] of Object.entries(requested)) {
29+
const grantedTools = granted[resource];
30+
31+
if (!grantedTools || grantedTools.length === 0) {
32+
// No grant for this resource — fall back to a wildcard `"*"` resource.
33+
const wildcardTools = granted["*"];
34+
if (!wildcardTools || wildcardTools.length === 0) {
35+
return false;
36+
}
37+
if (wildcardTools.includes("*")) {
38+
continue; // wildcard resource grants every tool
39+
}
40+
const wildcardSet = new Set(wildcardTools);
41+
for (const tool of tools) {
42+
if (!wildcardSet.has(tool)) return false;
43+
}
44+
continue;
45+
}
46+
47+
if (grantedTools.includes("*")) {
48+
continue; // wildcard grants all tools for this resource
49+
}
50+
51+
const grantedSet = new Set(grantedTools);
52+
for (const tool of tools) {
53+
if (!grantedSet.has(tool)) return false;
54+
}
55+
}
56+
57+
return true;
58+
}

apps/mesh/src/auth/dev-link-session.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,9 @@ export async function bootstrapDevLinkSession(
115115
body: {
116116
name: "dev-link (auto-minted by bun run dev)",
117117
userId: user.id,
118+
// Dev loopback acting as the user — full access. With no implicit
119+
// default (auth/index.ts), the scope must be explicit.
120+
permissions: { "*": ["*"] },
118121
// 30 days — re-minted on file deletion, far longer than any
119122
// single dev session.
120123
expiresIn: 60 * 60 * 24 * 30,

apps/mesh/src/auth/index.ts

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -311,20 +311,10 @@ const plugins = [
311311
}
312312
return null;
313313
},
314-
permissions: {
315-
defaultPermissions: {
316-
self: [
317-
"ORGANIZATION_LIST",
318-
"ORGANIZATION_GET", // Organization read access
319-
"ORGANIZATION_MEMBER_LIST", // Member read access
320-
"COLLECTION_CONNECTIONS_LIST",
321-
"COLLECTION_CONNECTIONS_GET", // Connection read access
322-
"API_KEY_CREATE", // API key creation
323-
"API_KEY_LIST", // API key listing (metadata only)
324-
// Note: API_KEY_UPDATE and API_KEY_DELETE are not default - users must explicitly request
325-
],
326-
},
327-
},
314+
// No `defaultPermissions`: every key is created with an explicit scope (the
315+
// API_KEY_CREATE tool requires `permissions`, and the internal minters pass
316+
// their own). A key is authorized solely by its allowlist — see
317+
// auth/api-key-permissions.ts. A key with no allowlist grants nothing.
328318
rateLimit: {
329319
enabled: false,
330320
},

apps/mesh/src/core/access-control.test.ts

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,51 @@ describe("AccessControl", () => {
143143
expect(ac.granted()).toBe(true);
144144
});
145145

146+
it("does NOT bypass the admin role for an API-key principal", async () => {
147+
// A key scoped to ORGANIZATION_GET is authorized solely by that allowlist —
148+
// the owner's admin/owner role must not widen it. The flag lives on
149+
// boundAuth, which enforces the key's permissions.
150+
const keyBoundAuth = {
151+
...createMockBoundAuth({ self: ["ORGANIZATION_GET"] }),
152+
isApiKeyPrincipal: true,
153+
} as BoundAuthClient;
154+
155+
const allowed = new AccessControl(
156+
"user_1",
157+
"ORGANIZATION_GET",
158+
keyBoundAuth,
159+
"admin", // owner is an admin — must NOT grant beyond the allowlist
160+
);
161+
await allowed.check();
162+
expect(allowed.granted()).toBe(true);
163+
164+
const denied = new AccessControl(
165+
"user_1",
166+
"API_KEY_CREATE", // out of scope — the exact escalation we are blocking
167+
keyBoundAuth,
168+
"admin",
169+
);
170+
await expect(denied.check()).rejects.toThrow(ForbiddenError);
171+
expect(denied.granted()).toBe(false);
172+
});
173+
174+
it("still bypasses the admin role for a non-API-key principal (session)", async () => {
175+
// isApiKeyPrincipal unset (browser session / MCP OAuth) → admin bypass.
176+
const boundAuth = {
177+
...createMockBoundAuth({ self: ["ORGANIZATION_GET"] }),
178+
isApiKeyPrincipal: false,
179+
} as BoundAuthClient;
180+
181+
const ac = new AccessControl(
182+
"user_1",
183+
"API_KEY_CREATE",
184+
boundAuth,
185+
"owner",
186+
);
187+
await ac.check();
188+
expect(ac.granted()).toBe(true);
189+
});
190+
146191
it("should check connection-specific permissions", async () => {
147192
const ac = new AccessControl(
148193
"user_1",
@@ -229,6 +274,21 @@ describe("AccessControl", () => {
229274
await expect(ac.check()).rejects.toThrow(ForbiddenError);
230275
expect(ac.granted()).toBe(false);
231276
});
277+
278+
it("does NOT grant basic-usage to an API-key principal (allowlist only)", async () => {
279+
// An API key is a capability, not a member — it takes the api-key codepath
280+
// and is decided solely by its allowlist, so a basic-usage tool NOT in the
281+
// key's scope is denied even though the owner is an admin.
282+
const keyBoundAuth = {
283+
...createMockBoundAuth({}), // key grants nothing
284+
isApiKeyPrincipal: true,
285+
} as BoundAuthClient;
286+
287+
const ac = new AccessControl("user_1", tool, keyBoundAuth, "admin");
288+
289+
await expect(ac.check()).rejects.toThrow(ForbiddenError);
290+
expect(ac.granted()).toBe(false);
291+
});
232292
});
233293

234294
describe("granted", () => {

apps/mesh/src/core/access-control.ts

Lines changed: 43 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -181,51 +181,58 @@ export class AccessControl {
181181
return false;
182182
}
183183

184-
// Basic-usage tools are granted to every authenticated org MEMBER at
185-
// runtime, regardless of their role. Resolving them here — instead of
186-
// baking them into each role's stored permission — means the set evolves
187-
// with a one-line edit to BASIC_USAGE_TOOLS, with no role-backfill
188-
// migration.
189-
//
190-
// Both signals are required and checked explicitly:
191-
// - `userId`: a verified authenticated principal. `boundAuth` is NOT a
192-
// proxy for auth — it's constructed for every request, including
193-
// anonymous ones, so it must never gate this grant.
194-
// - `role`: derived from the caller's membership row in the target org
195-
// (context-factory / resolveOrgFromPath); set only for members. The old
196-
// bake-in model denied non-members implicitly (no role → no stored
197-
// permission), so we keep that boundary explicit here.
184+
// Two kinds of principal, each with its OWN self-contained rule:
185+
// - API key → the key's stored allowlist is the whole decision. It is a
186+
// capability, not a member: no role, no basic-usage, no Better Auth.
187+
// - everyone else (session / MCP OAuth / mesh JWT) → membership floor +
188+
// admin/owner bypass + Better Auth grants.
189+
return this.boundAuth?.isApiKeyPrincipal
190+
? this.checkApiKeyAccess(resource)
191+
: this.checkMemberAccess(resource);
192+
}
193+
194+
/**
195+
* API-key authorization: the key's stored allowlist is the entire decision.
196+
* Never inherits the owner's role, basic-usage floor, or any Better Auth
197+
* grant — so a "read-only" key minted by an admin can't act beyond its scope.
198+
* See auth/api-key-permissions.ts (`checkApiKeyPermission`).
199+
*/
200+
private async checkApiKeyAccess(resource: string): Promise<boolean> {
201+
// `isApiKeyPrincipal` is only set on a real bound client, so this is
202+
// defensive; the dispatcher already routed non-bound principals away.
203+
if (!this.boundAuth) return false;
204+
return this.boundAuth.hasPermission({ [this.connectionId]: [resource] });
205+
}
206+
207+
/**
208+
* Member authorization (session / MCP OAuth / mesh JWT).
209+
*
210+
* Basic-usage tools are granted to every authenticated org MEMBER regardless
211+
* of role — resolved here, not baked into each role, so the set evolves with
212+
* a one-line edit to BASIC_USAGE_TOOLS. Both signals are required: `userId`
213+
* (a verified principal — `boundAuth` exists for every request, even
214+
* anonymous, so it must not gate this) and `role` (set only for members).
215+
* Admin/owner bypass everything; everyone else falls through to the stored /
216+
* Better Auth grant.
217+
*/
218+
private async checkMemberAccess(resource: string): Promise<boolean> {
198219
if (this.userId && this.role && BASIC_USAGE_TOOLS.has(resource)) {
199220
return true;
200221
}
201-
202-
// Admin and owner roles bypass all checks (they have full access)
203222
if (this.role === "admin" || this.role === "owner") {
204223
return true;
205224
}
206-
207-
// No bound auth client = deny (should not happen in normal flow)
208225
if (!this.boundAuth) {
209226
return false;
210227
}
211-
212-
// Build permission check - use connectionId as the resource key
213-
const permissionToCheck: Record<string, string[]> = {};
214-
if (this.connectionId) {
215-
permissionToCheck[this.connectionId] = [resource];
216-
}
217-
218-
// Delegate to the bound auth client. When an organizationId is set
219-
// (path-resolved org), pass it through so Better Auth uses it instead of
220-
// the session's active org. Pass `this.role` too: it is the member's role
221-
// for THIS org (constructor + resolveOrgFromPath keep role and
222-
// organizationId in lock-step), so boundAuth can resolve built-in roles
223-
// in-memory without a Better Auth round-trip. admin/owner already returned
224-
// above, so this only accelerates the `user` role; custom roles fall back.
225-
return this.boundAuth.hasPermission(permissionToCheck, {
226-
organizationId: this.organizationId,
227-
role: this.role,
228-
});
228+
// Pass `this.role` so boundAuth can resolve built-in roles in-memory (no
229+
// Better Auth round-trip); admin/owner already returned above, so this only
230+
// accelerates the `user` role. `organizationId` (path-resolved org) makes
231+
// Better Auth check the right org instead of the session's active one.
232+
return this.boundAuth.hasPermission(
233+
{ [this.connectionId]: [resource] },
234+
{ organizationId: this.organizationId, role: this.role },
235+
);
229236
}
230237

231238
/**

0 commit comments

Comments
 (0)