mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-03 17:51:58 +01:00
fix(server): enforce MCP wizard action scope
Prevent connection installs from adding connection-wide allow entries, keep wizard-managed app profiles additive across binding precedence, and repair legacy over-grants with a data migration. Add decision-level regression coverage for generic and curated MCP paths. Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
28c0c279aa
commit
048c3578e3
8 files changed
+268
-44
No files matched your search
@@ -0,0 +1,10 @@
|
||||
DELETE FROM "tool_profile_entries" AS "entry"
|
||||
USING "tool_profiles" AS "profile"
|
||||
WHERE "entry"."profile_id" = "profile"."id"
|
||||
AND "entry"."company_id" = "profile"."company_id"
|
||||
AND "entry"."selector_type" = 'connection'
|
||||
AND "entry"."effect" = 'include'
|
||||
AND "entry"."connection_id" IS NOT NULL
|
||||
AND "profile"."profile_key" = 'app:' || "entry"."connection_id"::text
|
||||
AND "profile"."metadata" ->> 'source' IN ('app_gallery_finish', 'tool_connection_install')
|
||||
AND "profile"."metadata" ->> 'connectionId' = "entry"."connection_id"::text;
|
||||
+8
-1
@@ -1569,6 +1569,13 @@
|
||||
"when": 1787101144413,
|
||||
"tag": "0225_drop_claude_setup_token_sessions",
|
||||
"breakpoints": true
|
||||
},
|
||||
{
|
||||
"idx": 226,
|
||||
"version": "7",
|
||||
"when": 1787101145413,
|
||||
"tag": "0226_remove_app_connection_wide_includes",
|
||||
"breakpoints": true
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
@@ -1270,15 +1270,26 @@ describeEmbeddedPostgres("generic remote MCP connections", () => {
|
||||
const company = await createCompany(db);
|
||||
const service = toolAccessService(db);
|
||||
const policy = toolAccessPolicyService(db);
|
||||
const [agent] = await db.insert(agents).values({
|
||||
companyId: company.id,
|
||||
name: `Generic MCP agent ${randomUUID()}`,
|
||||
role: "engineer",
|
||||
status: "active",
|
||||
adapterType: "process",
|
||||
adapterConfig: {},
|
||||
runtimeConfig: {},
|
||||
}).returning();
|
||||
const [agent, outsideAgent] = await db.insert(agents).values([
|
||||
{
|
||||
companyId: company.id,
|
||||
name: `Generic MCP agent ${randomUUID()}`,
|
||||
role: "engineer",
|
||||
status: "active",
|
||||
adapterType: "process",
|
||||
adapterConfig: {},
|
||||
runtimeConfig: {},
|
||||
},
|
||||
{
|
||||
companyId: company.id,
|
||||
name: `Outside MCP agent ${randomUUID()}`,
|
||||
role: "engineer",
|
||||
status: "active",
|
||||
adapterType: "process",
|
||||
adapterConfig: {},
|
||||
runtimeConfig: {},
|
||||
},
|
||||
]).returning();
|
||||
|
||||
const connected = await service.connectGalleryApp(company.id, { link: MCP_URL, name: "Fixture governance" });
|
||||
const readEntry = connected.catalog.find((entry) => entry.toolName === "list_insights")!;
|
||||
@@ -1297,9 +1308,9 @@ describeEmbeddedPostgres("generic remote MCP connections", () => {
|
||||
expect.objectContaining({ targetType: "agent", targetId: agent!.id }),
|
||||
]);
|
||||
|
||||
const decisionInput = (catalogEntryId: string, toolName: string) => ({
|
||||
const decisionInput = (catalogEntryId: string, toolName: string, agentId = agent!.id) => ({
|
||||
companyId: company.id,
|
||||
actor: { actorType: "agent" as const, actorId: agent!.id, agentId: agent!.id },
|
||||
actor: { actorType: "agent" as const, actorId: agentId, agentId },
|
||||
request: { connectionId: connected.connectionId, catalogEntryId, toolName },
|
||||
});
|
||||
|
||||
@@ -1310,6 +1321,19 @@ describeEmbeddedPostgres("generic remote MCP connections", () => {
|
||||
.resolves.toMatchObject({ allowed: true, reasonCode: "allow_profile" });
|
||||
await expect(policy.decide(decisionInput(writeEntry.id, "create_insight")))
|
||||
.resolves.toMatchObject({ allowed: false, reasonCode: "deny_default" });
|
||||
await expect(policy.decide(decisionInput(readEntry.id, "list_insights", outsideAgent!.id)))
|
||||
.resolves.toMatchObject({ allowed: false, reasonCode: "deny_default" });
|
||||
|
||||
// Simulate a profile written by the legacy install path. Saving installs
|
||||
// must self-heal this over-broad entry as well as avoiding it for new apps.
|
||||
await db.insert(toolProfileEntries).values({
|
||||
companyId: company.id,
|
||||
profileId: finished.profile.id,
|
||||
selectorType: "connection",
|
||||
effect: "include",
|
||||
applicationId: connected.connection.applicationId,
|
||||
connectionId: connected.connectionId,
|
||||
});
|
||||
|
||||
// Installation targets the chosen agent, same as a curated connection.
|
||||
await service.putConnectionInstalls(connected.connectionId, {
|
||||
@@ -1318,6 +1342,19 @@ describeEmbeddedPostgres("generic remote MCP connections", () => {
|
||||
const installs = await db.select().from(toolConnectionInstalls)
|
||||
.where(eq(toolConnectionInstalls.connectionId, connected.connectionId));
|
||||
expect(installs).toEqual([expect.objectContaining({ targetType: "agent", targetId: agent!.id })]);
|
||||
const installedProfileEntries = await db.select().from(toolProfileEntries)
|
||||
.where(eq(toolProfileEntries.profileId, finished.profile.id));
|
||||
expect(installedProfileEntries).not.toEqual(expect.arrayContaining([
|
||||
expect.objectContaining({
|
||||
selectorType: "connection",
|
||||
connectionId: connected.connectionId,
|
||||
effect: "include",
|
||||
}),
|
||||
]));
|
||||
await expect(policy.decide(decisionInput(readEntry.id, "list_insights")))
|
||||
.resolves.toMatchObject({ allowed: true, reasonCode: "allow_profile" });
|
||||
await expect(policy.decide(decisionInput(writeEntry.id, "create_insight")))
|
||||
.resolves.toMatchObject({ allowed: false, reasonCode: "deny_default" });
|
||||
|
||||
// Revoke: archiving the connection removes access but keeps the trail.
|
||||
await service.archiveConnection(connected.connectionId, company.id);
|
||||
|
||||
@@ -5384,6 +5384,20 @@ describeEmbeddedPostgres("tool access service", () => {
|
||||
expect.objectContaining({ id: duplicateEntry.id, riskLevel: "write", isWrite: true }),
|
||||
]));
|
||||
|
||||
// A narrower profile must not make an app shared with "All agents"
|
||||
// disappear. App action selection is an additive capability assignment;
|
||||
// ordinary profile precedence still governs non-app defaults.
|
||||
const existingAgentProfile = await service.createProfile(company.id, {
|
||||
profileKey: `existing-agent-profile-${randomUUID()}`,
|
||||
name: "Existing agent defaults",
|
||||
defaultAction: "deny",
|
||||
});
|
||||
await service.bindProfile(
|
||||
existingAgentProfile.id,
|
||||
{ targetType: "agent", targetId: agent.id },
|
||||
{ actorType: "user", actorId: "board" },
|
||||
);
|
||||
|
||||
await expect(service.finishGalleryAppConnection(otherCompany.id, connection.id, {
|
||||
enabledCatalogEntryIds: [fetchEntry.id],
|
||||
askFirstCatalogEntryIds: [],
|
||||
@@ -7040,11 +7054,14 @@ describeEmbeddedPostgres("tool access service", () => {
|
||||
expect(unusedRow!.lastUsedAt).toBeNull();
|
||||
});
|
||||
|
||||
it("syncs installs, auto-extends agent access, and exposes install state", async () => {
|
||||
it("syncs installs without widening action access or calling the remote tool", async () => {
|
||||
const company = await createCompany(db);
|
||||
const agent = await createAgent(db, company.id);
|
||||
const { connection } = await createRemoteToolFixture(db, company.id);
|
||||
const app = createRouteApp(db);
|
||||
const fetchMock = vi.spyOn(globalThis, "fetch");
|
||||
const app = createRouteApp(db, undefined, createToolGatewayService(db, {
|
||||
toolActionSigningSecret: "test-secret",
|
||||
}));
|
||||
|
||||
const put = await request(app)
|
||||
.put(`/api/tool-connections/${connection.id}/installs`)
|
||||
@@ -7071,7 +7088,17 @@ describeEmbeddedPostgres("tool access service", () => {
|
||||
|
||||
const effective = await toolAccessService(db).getEffectiveProfilesForAgent(company.id, agent.id);
|
||||
expect(effective.installedConnections.map((item) => item.id)).toEqual([connection.id]);
|
||||
expect(effective.allowedTools.some((tool) => tool.connectionId === connection.id)).toBe(true);
|
||||
expect(effective.allowedTools.some((tool) => tool.connectionId === connection.id)).toBe(false);
|
||||
|
||||
const deniedCall = await request(app)
|
||||
.post(`/api/tool-connections/${connection.id}/test-calls`)
|
||||
.send({ agentId: agent.id, toolName: "send_email", parameters: { to: "a@example.com" } })
|
||||
.expect(200);
|
||||
expect(deniedCall.body).toMatchObject({
|
||||
decision: "off",
|
||||
error: { reasonCode: "deny_default" },
|
||||
});
|
||||
expect(fetchMock).not.toHaveBeenCalled();
|
||||
|
||||
const get = await request(app).get(`/api/tool-connections/${connection.id}`);
|
||||
expect(get.status).toBe(200);
|
||||
|
||||
@@ -42,7 +42,10 @@ import type {
|
||||
} from "@paperclipai/shared";
|
||||
import { toolPolicyConditionsSchema } from "@paperclipai/shared";
|
||||
import { badRequest, conflict, notFound, unprocessable } from "../errors.js";
|
||||
import { narrowestScopeBindings, profileIdsInBindingOrder } from "./tool-profile-binding-precedence.js";
|
||||
import {
|
||||
effectiveToolProfileBindings,
|
||||
profileIdsInBindingOrder,
|
||||
} from "./tool-profile-binding-precedence.js";
|
||||
import { recordToolRuntimeAuditWriteFailure } from "./tool-runtime-metrics.js";
|
||||
|
||||
type ToolAccessContext = {
|
||||
@@ -1002,11 +1005,16 @@ export function toolAccessPolicyService(db: Db) {
|
||||
|
||||
async function effectiveProfiles(ctx: ToolAccessContext) {
|
||||
const bindings = await db.select().from(toolProfileBindings).where(eq(toolProfileBindings.companyId, ctx.companyId));
|
||||
const activeBindings = narrowestScopeBindings(bindings.filter((binding) => targetMatches(binding, ctx)));
|
||||
if (activeBindings.length === 0) return { profiles: [], entries: [] as Array<typeof toolProfileEntries.$inferSelect> };
|
||||
const matchingBindings = bindings.filter((binding) => targetMatches(binding, ctx));
|
||||
if (matchingBindings.length === 0) return { profiles: [], entries: [] as Array<typeof toolProfileEntries.$inferSelect> };
|
||||
const candidateProfileIds = profileIdsInBindingOrder(matchingBindings);
|
||||
const candidateProfiles = await db.select().from(toolProfiles).where(and(
|
||||
eq(toolProfiles.companyId, ctx.companyId),
|
||||
inArray(toolProfiles.id, candidateProfileIds),
|
||||
));
|
||||
const activeBindings = effectiveToolProfileBindings(matchingBindings, candidateProfiles, ctx.connectionId);
|
||||
const profileIds = profileIdsInBindingOrder(activeBindings);
|
||||
const profiles = await db.select().from(toolProfiles).where(and(eq(toolProfiles.companyId, ctx.companyId), inArray(toolProfiles.id, profileIds)));
|
||||
const profilesById = new Map(profiles.map((profile) => [profile.id, profile]));
|
||||
const profilesById = new Map(candidateProfiles.map((profile) => [profile.id, profile]));
|
||||
const activeProfiles = profileIds
|
||||
.map((profileId) => profilesById.get(profileId) ?? null)
|
||||
.filter((profile): profile is typeof toolProfiles.$inferSelect => Boolean(profile && profile.status === "active"));
|
||||
|
||||
@@ -130,7 +130,11 @@ import { guardedRemoteHttpFetch, type GuardedRemoteHttpFetchOptions } from "./re
|
||||
import { secretService } from "./secrets.js";
|
||||
import { toolAccessPolicyService } from "./tool-access-policy.js";
|
||||
import { readSignedToolArgumentsPayload } from "./tool-content-guards.js";
|
||||
import { narrowestScopeBindings, profileIdsInBindingOrder } from "./tool-profile-binding-precedence.js";
|
||||
import {
|
||||
effectiveToolProfileBindings,
|
||||
narrowestScopeBindings,
|
||||
profileIdsInBindingOrder,
|
||||
} from "./tool-profile-binding-precedence.js";
|
||||
import { recordToolRuntimeAuditWriteFailure, TOOL_RUNTIME_AUDIT_WRITE_FAILURE_METRIC } from "./tool-runtime-metrics.js";
|
||||
import { createToolRuntimeSupervisor, ToolRuntimeSupervisorError } from "./tool-runtime-supervisor.js";
|
||||
|
||||
@@ -2875,7 +2879,7 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {}
|
||||
}
|
||||
|
||||
async function appProfileForConnection(
|
||||
dbClient: Pick<Db, "select" | "insert">,
|
||||
dbClient: Pick<Db, "select" | "insert" | "delete">,
|
||||
connection: typeof toolConnections.$inferSelect,
|
||||
) {
|
||||
const profileKey = `app:${connection.id}`;
|
||||
@@ -2895,26 +2899,19 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {}
|
||||
metadata: { source: "tool_connection_install", connectionId: connection.id },
|
||||
}).returning();
|
||||
}
|
||||
const [existingEntry] = await dbClient
|
||||
.select({ id: toolProfileEntries.id })
|
||||
.from(toolProfileEntries)
|
||||
// Installation controls where a connection is exposed, not which actions
|
||||
// it grants. The app wizard's catalog-entry includes are the authority for
|
||||
// action selection, so remove the legacy connection-wide include that used
|
||||
// to silently turn every installed action on.
|
||||
await dbClient
|
||||
.delete(toolProfileEntries)
|
||||
.where(and(
|
||||
eq(toolProfileEntries.companyId, connection.companyId),
|
||||
eq(toolProfileEntries.profileId, profile.id),
|
||||
eq(toolProfileEntries.selectorType, "connection"),
|
||||
eq(toolProfileEntries.effect, "include"),
|
||||
eq(toolProfileEntries.connectionId, connection.id),
|
||||
))
|
||||
.limit(1);
|
||||
if (!existingEntry) {
|
||||
await dbClient.insert(toolProfileEntries).values({
|
||||
companyId: connection.companyId,
|
||||
profileId: profile.id,
|
||||
selectorType: "connection",
|
||||
effect: "include",
|
||||
applicationId: connection.applicationId,
|
||||
connectionId: connection.id,
|
||||
});
|
||||
}
|
||||
));
|
||||
return profile;
|
||||
}
|
||||
|
||||
@@ -8260,11 +8257,11 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {}
|
||||
.from(toolProfileBindings)
|
||||
.where(eq(toolProfileBindings.companyId, companyId))
|
||||
.orderBy(asc(toolProfileBindings.priority), asc(toolProfileBindings.createdAt));
|
||||
const bindings = narrowestScopeBindings(allBindings.filter((binding) =>
|
||||
const matchingBindings = allBindings.filter((binding) =>
|
||||
(binding.targetType === "company" && binding.targetId === companyId)
|
||||
|| (binding.targetType === "agent" && binding.targetId === agentId)
|
||||
));
|
||||
if (bindings.length === 0) {
|
||||
);
|
||||
if (matchingBindings.length === 0) {
|
||||
return {
|
||||
agentId,
|
||||
profiles: [],
|
||||
@@ -8275,12 +8272,14 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {}
|
||||
installedConnections: await resolveInstalledConnectionsForAgent(companyId, agentId),
|
||||
};
|
||||
}
|
||||
const profileIds = profileIdsInBindingOrder(bindings);
|
||||
const profiles = await db
|
||||
const candidateProfileIds = profileIdsInBindingOrder(matchingBindings);
|
||||
const candidateProfiles = await db
|
||||
.select()
|
||||
.from(toolProfiles)
|
||||
.where(and(eq(toolProfiles.companyId, companyId), inArray(toolProfiles.id, profileIds)));
|
||||
const profilesById = new Map(profiles.map((profile) => [profile.id, profile]));
|
||||
.where(and(eq(toolProfiles.companyId, companyId), inArray(toolProfiles.id, candidateProfileIds)));
|
||||
const bindings = effectiveToolProfileBindings(matchingBindings, candidateProfiles);
|
||||
const profileIds = profileIdsInBindingOrder(bindings);
|
||||
const profilesById = new Map(candidateProfiles.map((profile) => [profile.id, profile]));
|
||||
const activeProfiles = profileIds
|
||||
.map((profileId) => profilesById.get(profileId) ?? null)
|
||||
.filter((profile): profile is typeof toolProfiles.$inferSelect => Boolean(profile && profile.status === "active"));
|
||||
|
||||
@@ -0,0 +1,93 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import {
|
||||
effectiveToolProfileBindings,
|
||||
narrowestScopeBindings,
|
||||
} from "./tool-profile-binding-precedence.js";
|
||||
|
||||
const createdAt = new Date("2026-08-11T00:00:00.000Z");
|
||||
|
||||
describe("tool profile binding precedence", () => {
|
||||
it("keeps ordinary profiles at the narrowest matching scope", () => {
|
||||
const companyBinding = {
|
||||
profileId: "company-profile",
|
||||
targetType: "company" as const,
|
||||
targetId: "company-1",
|
||||
priority: 100,
|
||||
createdAt,
|
||||
};
|
||||
const agentBinding = {
|
||||
profileId: "agent-profile",
|
||||
targetType: "agent" as const,
|
||||
targetId: "agent-1",
|
||||
priority: 100,
|
||||
createdAt,
|
||||
};
|
||||
|
||||
expect(narrowestScopeBindings([companyBinding, agentBinding])).toEqual([agentBinding]);
|
||||
expect(effectiveToolProfileBindings(
|
||||
[companyBinding, agentBinding],
|
||||
[
|
||||
{ id: "company-profile", profileKey: "company-default", metadata: {} },
|
||||
{ id: "agent-profile", profileKey: "agent-default", metadata: {} },
|
||||
],
|
||||
"connection-1",
|
||||
)).toEqual([agentBinding]);
|
||||
});
|
||||
|
||||
it("carries the wizard-managed app assignment alongside a narrower profile", () => {
|
||||
const appBinding = {
|
||||
profileId: "app-profile",
|
||||
targetType: "company" as const,
|
||||
targetId: "company-1",
|
||||
priority: 100,
|
||||
createdAt,
|
||||
};
|
||||
const agentBinding = {
|
||||
profileId: "agent-profile",
|
||||
targetType: "agent" as const,
|
||||
targetId: "agent-1",
|
||||
priority: 100,
|
||||
createdAt,
|
||||
};
|
||||
|
||||
expect(effectiveToolProfileBindings(
|
||||
[appBinding, agentBinding],
|
||||
[
|
||||
{
|
||||
id: "app-profile",
|
||||
profileKey: "app:connection-1",
|
||||
metadata: { source: "app_gallery_finish", connectionId: "connection-1" },
|
||||
},
|
||||
{ id: "agent-profile", profileKey: "agent-default", metadata: {} },
|
||||
],
|
||||
"connection-1",
|
||||
)).toEqual([agentBinding, appBinding]);
|
||||
});
|
||||
|
||||
it("does not overlay a wizard profile onto another connection", () => {
|
||||
const appBinding = {
|
||||
profileId: "app-profile",
|
||||
targetType: "company" as const,
|
||||
targetId: "company-1",
|
||||
priority: 100,
|
||||
createdAt,
|
||||
};
|
||||
const agentBinding = {
|
||||
profileId: "agent-profile",
|
||||
targetType: "agent" as const,
|
||||
targetId: "agent-1",
|
||||
priority: 100,
|
||||
createdAt,
|
||||
};
|
||||
|
||||
expect(effectiveToolProfileBindings(
|
||||
[appBinding, agentBinding],
|
||||
[{
|
||||
id: "app-profile",
|
||||
profileKey: "app:connection-1",
|
||||
metadata: { source: "app_gallery_finish", connectionId: "connection-1" },
|
||||
}],
|
||||
"connection-2",
|
||||
)).toEqual([agentBinding]);
|
||||
});
|
||||
});
|
||||
@@ -3,10 +3,17 @@ import type { ToolProfileBindingTargetType } from "@paperclipai/shared";
|
||||
type BindingLike = {
|
||||
profileId: string;
|
||||
targetType: ToolProfileBindingTargetType;
|
||||
targetId: string;
|
||||
priority: number;
|
||||
createdAt: Date | string;
|
||||
};
|
||||
|
||||
type ProfileLike = {
|
||||
id: string;
|
||||
profileKey: string;
|
||||
metadata: unknown;
|
||||
};
|
||||
|
||||
const TOOL_PROFILE_SCOPE_PRECEDENCE: Record<ToolProfileBindingTargetType, number> = {
|
||||
// Named gateways bind one concrete MCP endpoint instance, so they should
|
||||
// override broader run, agent, and company defaults when both match.
|
||||
@@ -48,3 +55,39 @@ export function profileIdsInBindingOrder<T extends Pick<BindingLike, "profileId"
|
||||
}
|
||||
return ordered;
|
||||
}
|
||||
|
||||
function isWizardAppProfile(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<string, unknown>;
|
||||
if (metadata.source !== "app_gallery_finish" || typeof metadata.connectionId !== "string") return false;
|
||||
if (profile.profileKey !== `app:${metadata.connectionId}`) return false;
|
||||
return connectionId === undefined || connectionId === null || metadata.connectionId === connectionId;
|
||||
}
|
||||
|
||||
/**
|
||||
* App-wizard assignments are additive capabilities: choosing an app for all
|
||||
* agents (or for one agent) must not disappear merely because that agent also
|
||||
* has a narrower general-purpose profile. Ordinary profiles still use the
|
||||
* narrowest-scope rule; only the profile managed by the app wizard is carried
|
||||
* alongside that winning tier.
|
||||
*/
|
||||
export function effectiveToolProfileBindings<T extends BindingLike>(
|
||||
bindings: T[],
|
||||
profiles: ProfileLike[],
|
||||
connectionId?: string | null,
|
||||
): T[] {
|
||||
const appProfileIds = new Set(
|
||||
profiles.filter((profile) => isWizardAppProfile(profile, connectionId)).map((profile) => profile.id),
|
||||
);
|
||||
const selected = [
|
||||
...narrowestScopeBindings(bindings),
|
||||
...bindings.filter((binding) => appProfileIds.has(binding.profileId)),
|
||||
];
|
||||
const seen = new Set<string>();
|
||||
return selected.filter((binding) => {
|
||||
const key = `${binding.targetType}:${binding.targetId}:${binding.profileId}`;
|
||||
if (seen.has(key)) return false;
|
||||
seen.add(key);
|
||||
return true;
|
||||
});
|
||||
}
|
||||
Reference in new issue
Block a user