fix(auth): stop resolveCapabilities leaking connection-scoped grants into global capabilities - #5554
Conversation
…into global capabilities
resolveCapabilities unioned actions from EVERY permission bucket, including
resource-scoped ones (e.g. a specific connection id from a custom role's
per-connection toolSet), when computing which management capabilities a
role's stored permission grants.
Failure scenario: a custom role grants tools only on one connection
(permission = { "conn_abc": ["ORGANIZATION_MEMBER_ADD", ...] }). If that
connection happens to expose a tool whose name matches a Studio management
tool id, resolveCapabilities incorrectly reports the unrelated "members:manage"
capability as granted org-wide. The MY_CAPABILITIES endpoint
(apps/api/src/api/routes/auth.ts) surfaces this to the web UI
(use-capability.ts), so a member could see management actions (e.g. approve
join request) as available and then get denied by the real permission check,
which correctly stays resource-scoped via AccessControl.checkResource.
Only "self" and "*" are global tool buckets per the existing
hasGlobalGrant logic just above; the fix makes grantedActions source from
those same two buckets only, instead of every key in the permission map.
Added packages/shared/src/tools/registry-metadata.test.ts (no test existed for
this pure function) covering: connection-scoped bucket does NOT grant, self
bucket does grant, and the org-wide */self wildcard grants everything.
To verify: bun test packages/shared/src/tools/registry-metadata.test.ts
Ran locally: bun run fmt, bunx tsc --noEmit (packages/shared), the targeted
test above, and bunx oxlint on both changed files. Full CI validates the rest.
There was a problem hiding this comment.
1 issue found across 2 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="packages/shared/src/tools/registry-metadata.ts">
<violation number="1" location="packages/shared/src/tools/registry-metadata.ts:1624">
P2: This change makes an existing test in apps/api/src/tools/registry-metadata.test.ts fail. That test ('aggregates tools across resource buckets') asserts that a capability whose tools are split between `self` and a connection bucket (`conn_x`) is still granted. Because resolveCapabilities now sources grantedActions only from the `self` and `*` buckets, the connection-bucket tools are dropped and that capability resolves to false. If the narrowed semantics are intended, the apps/api test (and any other consumer relying on cross-bucket aggregation) needs to be updated in the same PR; as-is, the runtime behavior changed more broadly than the 'unrelated capability leak' framing suggests and leaves an existing test red.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| if (!Array.isArray(actions)) continue; | ||
| for (const action of actions) grantedActions.add(action); | ||
| } | ||
| for (const action of arrayBucket("self")) grantedActions.add(action); |
There was a problem hiding this comment.
P2: This change makes an existing test in apps/api/src/tools/registry-metadata.test.ts fail. That test ('aggregates tools across resource buckets') asserts that a capability whose tools are split between self and a connection bucket (conn_x) is still granted. Because resolveCapabilities now sources grantedActions only from the self and * buckets, the connection-bucket tools are dropped and that capability resolves to false. If the narrowed semantics are intended, the apps/api test (and any other consumer relying on cross-bucket aggregation) needs to be updated in the same PR; as-is, the runtime behavior changed more broadly than the 'unrelated capability leak' framing suggests and leaves an existing test red.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/shared/src/tools/registry-metadata.ts, line 1624:
<comment>This change makes an existing test in apps/api/src/tools/registry-metadata.test.ts fail. That test ('aggregates tools across resource buckets') asserts that a capability whose tools are split between `self` and a connection bucket (`conn_x`) is still granted. Because resolveCapabilities now sources grantedActions only from the `self` and `*` buckets, the connection-bucket tools are dropped and that capability resolves to false. If the narrowed semantics are intended, the apps/api test (and any other consumer relying on cross-bucket aggregation) needs to be updated in the same PR; as-is, the runtime behavior changed more broadly than the 'unrelated capability leak' framing suggests and leaves an existing test red.</comment>
<file context>
@@ -1615,12 +1615,14 @@ export function resolveCapabilities(
- if (!Array.isArray(actions)) continue;
- for (const action of actions) grantedActions.add(action);
- }
+ for (const action of arrayBucket("self")) grantedActions.add(action);
+ for (const action of arrayBucket("*")) grantedActions.add(action);
}
</file context>
Source: bug found while auditing
packages/shared/src/tools/tool-io.tsand its neighboringtools/module for type-safety/correctness issues (that file itself is 100% generated interface declarations with no logic to harden;registry-metadata.tsin the same directory has the real runtime helpers).The bug:
resolveCapabilities(used byMY_CAPABILITIES/apps/api/src/api/routes/auth.tsto tell the web UI which management capabilities a custom role's stored permission grants) unioned actions from every bucket in the permission map — including resource-scoped buckets like a specific connection id from a custom role's per-connectiontoolSet— into a singlegrantedActionsset used to decide whether a capability (e.g.members:manage) is enabled.Failure scenario: a custom role is granted tools on exactly one connection (
permission = { "conn_abc": ["ORGANIZATION_MEMBER_ADD", ...] }). If that connection happens to expose a tool whose name collides with a Studio management-tool id (fully attacker/integration-controlled, since MCP connections define their own tool names),resolveCapabilitiesreports the unrelatedmembers:managecapability as globally granted. The web UI (use-capability.ts/use-join-requests.ts) then shows management actions like "approve join request" as available to a member who doesn't actually have them — the real enforcement path (AccessControl.checkResource, which checkshasPermission({ [connectionId]: [resource] })scoped to"self"by default) still correctly denies it, so this is a UI-gating bug, not an auth bypass, but it's a real capability-resolution defect matching the auth org/permission-scoping code path.Fix: only
"self"and"*"are global tool buckets (matching thehasGlobalGrantwildcard logic already a few lines above in the same function) —grantedActionsnow sources from those two buckets only, not every key in the permission map.Regression test: added
packages/shared/src/tools/registry-metadata.test.ts(no test existed for this pure function before) covering: a connection-scoped bucket does NOT grant an unrelated capability, theselfbucket does grant it, and the org-wide*/selfwildcard still grants everything.Reviewer verification:
bun test packages/shared/src/tools/registry-metadata.test.tsLocally ran:
bun run fmt,bunx tsc --noEmitinpackages/shared, the targeted test above, andbunx oxlinton both changed files — all green. Full CI validates the rest.Summary by cubic
Fixes capability resolution so connection-scoped tool grants no longer leak into global management capabilities, preventing the UI from showing org-wide actions a user cannot perform. Global grants are now derived only from
"self"and"*"buckets to align with existing wildcard logic.resolveCapabilitiesinpackages/shared/src/tools/registry-metadata.tsto use only"self"and"*"forgrantedActions; ignore resource-scoped buckets (e.g., connection IDs).packages/shared/src/tools/registry-metadata.test.tsto verify: connection-scoped grants don’t enable management capabilities;"self"grants do;"*"wildcard enables all.Written for commit 3a8d831. Summary will update on new commits.