From 3f120465768dcb8310a7a4ceb8cec50ef4fe4fe4 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Sun, 8 Nov 2026 21:58:29 -0600 Subject: [PATCH] fix(tool-access): an unscoped tools:use grant does not turn on actions a connection left off (#20160) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - People connect apps and choose which actions of each app agents may call; the choice is saved as the connection's tool profile > - Every new standard agent also gets an unscoped `tools:use` grant, which means "this agent may use tools" > - The access policy checked explicit grants before profiles, so this default grant allowed every action of every installed connection, including actions the person left off > - The agent's effective profile showed the action off, but the gateway and the test-as-agent summary allowed it > - This pull request makes an unscoped grant stop at a connection profile: when a profile governs the tool's connection, only a grant scoped to the action, connection or application allows an action the profile leaves off > - The benefit is that the actions a person turns off for a connection stay off for new agents ## Linked Issues or Issue Description **What happened** A person connected an app with one read action and one destructive write action. When they finished the connection, they enabled only the read action. They installed the connection for a new agent, Morgan. Morgan's effective profile (`GET /api/companies/{companyId}/tools/profiles/effective/agents/{agentId}`) allowed only the read action. But `GET /api/tool-connections/{connectionId}/test-agents/{agentId}/access` reported both actions as `allowed` with reason `allow_explicit_grant`, and a chat agent told the person that Morgan could also run the destructive write. The same policy decision admits gateway calls, so Morgan could call the write. **Expected behavior** An action that the connection's profile leaves off is off for the agent. The access summary, the effective profile and the gateway agree. A grant that a person scopes to that action, connection or application still allows it. **Steps to reproduce** 1. Connect an MCP app that has a read tool and a destructive tool. In the finish step, enable only the read tool. 2. Create a new agent through the API (it gets the default unscoped `tools:use` grant) and install the connection for it. 3. Call `GET /api/tool-connections/{connectionId}/test-agents/{agentId}/access`: the destructive tool shows `allowed`, reason `allow_explicit_grant`. ## What Changed - `server/src/services/tool-access-policy.ts`: for an agent, a tool's connection is governed by a connection profile when the agent's effective profiles include the connection's own access profile (the app wizard's `app:` profile or the agent's connection-intent profile, even with no entries), or any entry that names this connection or selects this action (including application, tool-name and risk-level selectors). For a governed connection, `explicitGrant` counts only grants whose scope names an action, connection or application (`allow`, `toolName`, `catalogEntryId`, `connectionId`, `applicationId`, `applicationKey`). A grant scoped only by context (for example an agent or project) still covers every tool, so it does not override the profile. Board users keep their operator grant. - `server/src/services/tool-profile-binding-precedence.ts`: export the existing connection-profile identity check as `isConnectionAccessProfile`, so the policy and binding precedence use one definition. - `server/src/__tests__/generic-mcp-connection.test.ts`: regression tests through the gateway's access summary and the direct policy check: - only the read action enabled: the write stays off with the unscoped grant, and a grant scoped to the write allows it - every action turned off (both lists empty): both stay off; a context-only scope stays off; a connection-scoped or application-scoped grant allows them - an agent the connection's profile does not cover keeps the unscoped grant - a general profile with an application-selector exclude (no connection id) is not overridden by the unscoped grant - with company-wide access, the direct policy check denies the off action for an agent and still allows it for a board user with the unscoped grant ## Verification - New tests: all pass with the change. On `master`, 4 of 5 fail. On the first revision of this PR, the all-off, application-exclude and board-user tests fail. - Regression suites, all pass: `generic-mcp-connection`, `tool-access-policy-service`, `tool-profile-matching`, `tool-profile-entry-order`, `connection-management.integration` (238 tests), and `tool-access-service`, `tool-gateway-service`, `tool-gateway`, `tool-profile-review-service` (799 tests). - Every enforcement path uses the one `decide` function in `tool-access-policy.ts`, and `explicitGrant` is called only there. That covers the gateway tool listing and calls, the access summary, delegated and Test tab calls, the connection-intent access check, the GitHub review bot tool, and the example smoke check. - Chat eval `app-permissions.test-as-agent` (runner-opencode). Before the change (GitHub Actions run 42550085273), the access summary for Morgan reported the write as `allowed` by `allow_explicit_grant`. With the first commit cherry-picked onto the eval branch, GitHub Actions run 42552819589 passed. The summary reported `launch_read` allowed (`allow_profile`) and `launch_update` off (`deny_default`), which matches Morgan's effective profile, and the agent's reply said the same. ## Risks - Medium risk. This changes an access decision. An agent that used a connection action only through its default unscoped grant, while the connection's profile or an explicit exclude left that action off, loses that action. That is the intended fix. An operator who relied on it must enable the action in the connection or give the agent a grant that names the action, connection or application. - A grant scoped only by context (for example `{ projectId }`) no longer overrides a connection profile. Risk-level and provider-type scopes are not treated as naming an action. - Tools that no connection profile governs (for example Paperclip's own tools), agents without a connection profile, and board users keep the current behavior. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Related: #17884 (scoped `tools:use` grants at hire time). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 --- .../__tests__/generic-mcp-connection.test.ts | 100 ++++++++++++++++++ server/src/services/tool-access-policy.ts | 36 ++++++- .../tool-profile-binding-precedence.ts | 9 +- 3 files changed, 140 insertions(+), 5 deletions(-) diff --git a/server/src/__tests__/generic-mcp-connection.test.ts b/server/src/__tests__/generic-mcp-connection.test.ts index 78bf314dc4f..70b97b1d1d5 100644 --- a/server/src/__tests__/generic-mcp-connection.test.ts +++ b/server/src/__tests__/generic-mcp-connection.test.ts @@ -1053,6 +1053,106 @@ describeEmbeddedPostgres("generic remote MCP connections", () => { expect(JSON.stringify(response.body)).not.toContain("PAPERCLIP_PUBLIC_URL"); }); + it("keeps an action the connection left off from agents that hold only the default unscoped tools:use grant", async () => { + installMcpOAuthFixture({ auth: "header", requiredHeader: { name: "Authorization", value: "Bearer fixture-key-123" }, tools: [ + { name: "list_insights", description: "List insights", annotations: { readOnlyHint: true, destructiveHint: false } }, + { name: "create_insight", description: "Create an insight", annotations: { readOnlyHint: false, destructiveHint: true } }, + ] }); + const company = await createCompany(db); + const service = toolAccessService(db); + const actor = { actorType: "user" as const, actorId: "board-user", userId: "board-user" } as never; + const connected = await service.connectGalleryApp(company.id, { link: MCP_URL, name: "Fixture bearer", authMode: "bearer", + credentialValues: { "credentials.authorization": "fixture-key-123" }, grantKind: "organization" } as never, actor); + const readOnly = connected.actions.readOnly.map((row: { catalogEntryId: string }) => row.catalogEntryId); + const [agent] = await db.insert(agents).values({ companyId: company.id, name: "Morgan", role: "engineer", status: "active", adapterType: "process", adapterConfig: {}, runtimeConfig: {} }).returning(); + await service.finishGalleryAppConnection(company.id, connected.connectionId, { enabledCatalogEntryIds: readOnly, askFirstCatalogEntryIds: [], access: { agentIds: [] } }, actor); + await service.putConnectionInstalls(connected.connectionId, { installs: [{ targetType: "agent", targetId: agent!.id }] }, actor); + // Every new standard agent receives this unscoped grant. + await db.insert(principalPermissionGrants).values({ companyId: company.id, principalType: "agent", principalId: agent!.id, permissionKey: "tools:use", scope: null }); + const gateway = createToolGatewayService(db, { toolActionSigningSecret: "fixture-secret" }); + const summary = async () => (await gateway.summarizeConnectionAccessForAgent({ companyId: company.id, connectionId: connected.connectionId, agentId: agent!.id })) + .tools.map(tool => [tool.toolName, tool.decision]).sort(); + expect(await summary()).toEqual([["create_insight", "off"], ["list_insights", "allowed"]]); + // A grant scoped to the action is an explicit choice and still applies. + await db.update(principalPermissionGrants).set({ scope: { allow: ["tool:create_insight"] } }) + .where(and(eq(principalPermissionGrants.principalId, agent!.id), eq(principalPermissionGrants.permissionKey, "tools:use"))); + expect(await summary()).toEqual([["create_insight", "allowed"], ["list_insights", "allowed"]]); + }); + + async function setUpGovernedTwoActionConnection(options: { enabled: "read" | "none"; access?: "all_agents" }) { + installMcpOAuthFixture({ auth: "header", requiredHeader: { name: "Authorization", value: "Bearer fixture-key-123" }, tools: [ + { name: "list_insights", description: "List insights", annotations: { readOnlyHint: true, destructiveHint: false } }, + { name: "create_insight", description: "Create an insight", annotations: { readOnlyHint: false, destructiveHint: true } }, + ] }); + const company = await createCompany(db); + const service = toolAccessService(db); + const actor = { actorType: "user" as const, actorId: "board-user", userId: "board-user" } as never; + const connected = await service.connectGalleryApp(company.id, { link: MCP_URL, name: "Fixture bearer", authMode: "bearer", + credentialValues: { "credentials.authorization": "fixture-key-123" }, grantKind: "organization" } as never, actor); + const readOnly = connected.actions.readOnly.map((row: { catalogEntryId: string }) => row.catalogEntryId); + const [agent] = await db.insert(agents).values({ companyId: company.id, name: "Morgan", role: "engineer", status: "active", adapterType: "process", adapterConfig: {}, runtimeConfig: {} }).returning(); + await service.finishGalleryAppConnection(company.id, connected.connectionId, { + enabledCatalogEntryIds: options.enabled === "read" ? readOnly : [], askFirstCatalogEntryIds: [], + access: options.access ?? { agentIds: [] }, + }, actor); + if (!options.access) await service.putConnectionInstalls(connected.connectionId, { installs: [{ targetType: "agent", targetId: agent!.id }] }, actor); + // Every new standard agent receives this unscoped grant. + await db.insert(principalPermissionGrants).values({ companyId: company.id, principalType: "agent", principalId: agent!.id, permissionKey: "tools:use", scope: null }); + const gateway = createToolGatewayService(db, { toolActionSigningSecret: "fixture-secret" }); + const summary = async (agentId = agent!.id) => (await gateway.summarizeConnectionAccessForAgent({ companyId: company.id, connectionId: connected.connectionId, agentId })) + .tools.map(tool => [tool.toolName, tool.decision]).sort(); + const setGrantScope = (scope: Record | null) => db.update(principalPermissionGrants).set({ scope }) + .where(and(eq(principalPermissionGrants.principalId, agent!.id), eq(principalPermissionGrants.permissionKey, "tools:use"))); + const [connection] = await db.select().from(toolConnections).where(eq(toolConnections.id, connected.connectionId)); + return { company, agent: agent!, connection: connection!, summary, setGrantScope }; + } + + it("keeps every action off when the connection turns all of them off, even for the default unscoped tools:use grant", async () => { + const { connection, summary, setGrantScope } = await setUpGovernedTwoActionConnection({ enabled: "none" }); + expect(await summary()).toEqual([["create_insight", "off"], ["list_insights", "off"]]); + // A grant that only narrows when it applies (agent, project) still covers every tool, so it is not a choice about this connection. + await setGrantScope({ actorType: "agent" }); + expect(await summary()).toEqual([["create_insight", "off"], ["list_insights", "off"]]); + // Grants that name the connection or the application are explicit choices and still apply. + await setGrantScope({ connectionId: connection.id }); + expect(await summary()).toEqual([["create_insight", "allowed"], ["list_insights", "allowed"]]); + await setGrantScope({ allow: [`application:${connection.applicationId}`] }); + expect(await summary()).toEqual([["create_insight", "allowed"], ["list_insights", "allowed"]]); + }); + + it("keeps the unscoped tools:use grant for agents the connection's profile does not cover", async () => { + const { company, summary } = await setUpGovernedTwoActionConnection({ enabled: "read" }); + const [other] = await db.insert(agents).values({ companyId: company.id, name: "Riley", role: "engineer", status: "active", adapterType: "process", adapterConfig: {}, runtimeConfig: {} }).returning(); + await db.insert(principalPermissionGrants).values({ companyId: company.id, principalType: "agent", principalId: other!.id, permissionKey: "tools:use", scope: null }); + expect(await summary(other!.id)).toEqual([["create_insight", "allowed"], ["list_insights", "allowed"]]); + }); + + it("does not let the unscoped tools:use grant override a profile entry that excludes the application", async () => { + const { company, summary } = await setUpGovernedTwoActionConnection({ enabled: "read" }); + const [other] = await db.insert(agents).values({ companyId: company.id, name: "Riley", role: "engineer", status: "active", adapterType: "process", adapterConfig: {}, runtimeConfig: {} }).returning(); + await db.insert(principalPermissionGrants).values({ companyId: company.id, principalType: "agent", principalId: other!.id, permissionKey: "tools:use", scope: null }); + const [connection] = await db.select().from(toolConnections).where(eq(toolConnections.companyId, company.id)); + const [profile] = await db.insert(toolProfiles).values({ companyId: company.id, profileKey: `no-insights-${randomUUID()}`, name: "No insights", defaultAction: "allow" }).returning(); + // An application selector carries no connection id. + await db.insert(toolProfileEntries).values({ companyId: company.id, profileId: profile!.id, selectorType: "application", effect: "exclude", applicationId: connection!.applicationId }); + await db.insert(toolProfileBindings).values({ companyId: company.id, profileId: profile!.id, targetType: "agent", targetId: other!.id }); + expect(await summary(other!.id)).toEqual([["create_insight", "off"], ["list_insights", "off"]]); + }); + + it("applies the connection profile on the direct policy check and keeps a board user's unscoped grant", async () => { + const { company, agent, connection, summary } = await setUpGovernedTwoActionConnection({ enabled: "read", access: "all_agents" }); + expect(await summary()).toEqual([["create_insight", "off"], ["list_insights", "allowed"]]); + const [create] = await db.select().from(toolCatalogEntries).where(and(eq(toolCatalogEntries.connectionId, connection.id), eq(toolCatalogEntries.toolName, "create_insight"))); + const request = { connectionId: connection.id, catalogEntryId: create!.id, toolName: create!.toolName, arguments: {} }; + const policy = toolAccessPolicyService(db); + const agentDecision = await policy.decide({ companyId: company.id, actor: { actorType: "agent", actorId: agent.id, agentId: agent.id }, request }); + expect([agentDecision.decision, agentDecision.reasonCode]).toEqual(["deny", "deny_default"]); + // The connection's profile is bound to the company, which also matches board users; their operator grant still applies. + await db.insert(principalPermissionGrants).values({ companyId: company.id, principalType: "user", principalId: "board-user", permissionKey: "tools:use", scope: null }); + const userDecision = await policy.decide({ companyId: company.id, actor: { actorType: "user", actorId: "board-user" }, request }); + expect([userDecision.decision, userDecision.reasonCode]).toEqual(["allow", "allow_explicit_grant"]); + }); + it("stores a bearer key as a secret and never reads it back", async () => { installMcpOAuthFixture({ auth: "header", diff --git a/server/src/services/tool-access-policy.ts b/server/src/services/tool-access-policy.ts index cfd2f4478a6..839c2a43c54 100644 --- a/server/src/services/tool-access-policy.ts +++ b/server/src/services/tool-access-policy.ts @@ -49,6 +49,7 @@ import { toolPolicyConditionsSchema } from "@paperclipai/shared"; import { badRequest, conflict, notFound, unprocessable } from "../errors.js"; import { effectiveToolProfileBindings, + isConnectionAccessProfile, profileIdsInBindingOrder, } from "./tool-profile-binding-precedence.js"; import { recordToolRuntimeAuditWriteFailure } from "./tool-runtime-metrics.js"; @@ -760,6 +761,23 @@ function rateBucket(rule: ToolRateLimitRule, ctx: ToolAccessContext): string { }).join("|"); } +// Grant scope keys that name which tools the grant covers: an action, a +// connection or an application. Context keys such as agentId or projectId only +// narrow when the grant applies, so a grant with only those still covers every +// tool and does not count as a choice about one connection's actions. +const TOOL_GRANT_TARGET_SCOPE_KEYS = new Set([ + "allow", + "toolName", "toolNames", + "catalogEntryId", "catalogEntryIds", + "connectionId", "connectionIds", + "applicationId", "applicationIds", + "applicationKey", "applicationKeys", +]); + +function isToolTargetedGrant(scope: unknown) { + return isRecord(scope) && Object.keys(scope).some((key) => TOOL_GRANT_TARGET_SCOPE_KEYS.has(key)); +} + function scopeAllowsTool(scope: Record | null, ctx: ToolAccessContext) { if (scope === null || scope === undefined) return true; if (!isRecord(scope)) return false; @@ -1246,7 +1264,7 @@ export function toolAccessPolicyService(db: Pick { + async function explicitGrant(ctx: ToolAccessContext, cache?: ToolAccessDecisionCache, options: { toolTargetedOnly?: boolean } = {}): Promise { const principalType = ctx.actorType === "agent" ? "agent" : ctx.actorType === "user" ? "user" : null; const principalId = ctx.actorType === "agent" ? ctx.agentId : ctx.actorId; if (!principalType || !principalId) return false; @@ -1260,7 +1278,7 @@ export function toolAccessPolicyService(db: Pick scopeAllowsTool(grant.scope, ctx)); + return grants.some((grant) => (!options.toolTargetedOnly || isToolTargetedGrant(grant.scope)) && scopeAllowsTool(grant.scope, ctx)); } async function enforceRateLimit(policy: typeof toolPolicies.$inferSelect, ctx: ToolAccessContext, consume: boolean) { @@ -1650,7 +1668,19 @@ export function toolAccessPolicyService(db: Pick isConnectionAccessProfile(profile, ctx.connectionId)) + || profileState.entries.some((entry) => entry.connectionId === ctx.connectionId || profileEntryMatches(entry, ctx)) + ); + if (await explicitGrant(ctx, cache, { toolTargetedOnly: governedByConnectionProfile })) { return decision("allow", "allow_explicit_grant", "Tool access allowed by explicit grant.", effectiveProfileIds, [], { redactionPlan: redaction.redactionPlan }); } diff --git a/server/src/services/tool-profile-binding-precedence.ts b/server/src/services/tool-profile-binding-precedence.ts index 73b94c71314..c43711c80a0 100644 --- a/server/src/services/tool-profile-binding-precedence.ts +++ b/server/src/services/tool-profile-binding-precedence.ts @@ -56,7 +56,12 @@ export function profileIdsInBindingOrder` profile or an agent's `connection-intent` profile. + * With a connection id, only that connection's profiles match. + */ +export function isConnectionAccessProfile(profile: ProfileLike, connectionId?: string | null): boolean { if (!profile.metadata || typeof profile.metadata !== "object" || Array.isArray(profile.metadata)) return false; const metadata = profile.metadata as Record; if (metadata.source === "connection_intent" && typeof metadata.connectionId === "string" && typeof metadata.agentId === "string") { @@ -85,7 +90,7 @@ export function effectiveToolProfileBindings( return narrowestScopeBindings(bindings); } const appProfileIds = new Set( - profiles.filter((profile) => isWizardAppProfile(profile, connectionId)).map((profile) => profile.id), + profiles.filter((profile) => isConnectionAccessProfile(profile, connectionId)).map((profile) => profile.id), ); const accessProfileIds = new Set(profiles.filter(profile => { const metadata = profile.metadata as Record | null;