From bd77ffe78d625bb68c430640f4f30231e9cf48e8 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Sun, 25 Oct 2026 04:12:14 -0700 Subject: [PATCH] perf(connections): scope profile rule and binding reads (#17589) Read current company/agent profile bindings and rules matching the exact catalog rows being evaluated. Preserve all five selector kinds, null application matches, exclusions, and the existing access evaluator without a cross-call cache. Verified with full build and typecheck, 652 affected tests across ten suites, all 18 strengthened effective-assignment tests, exclusion mutation and byte counterfactuals, complete exact-head CI, and Greptile 5/5 Medium with no unresolved comments. Co-Authored-By: Paperclip --- doc/DATABASE.md | 7 ++ .../tool-effective-assignments.test.ts | 116 ++++++++++++++++++ .../tool-access/profiles/resolution.ts | 72 ++++++----- 3 files changed, 163 insertions(+), 32 deletions(-) diff --git a/doc/DATABASE.md b/doc/DATABASE.md index a814c50cfad..762ea37f2d9 100644 --- a/doc/DATABASE.md +++ b/doc/DATABASE.md @@ -135,6 +135,13 @@ authority once. Different task bindings must both pass. The batch is discarded after that delivery; later events still recheck grants, membership, task existence, and company boundaries. +Connection-assignment checks select profile bindings for the current company or +agent and read rules that can match the selected connection's catalog rows. +All five selector kinds, null application matching, exclusions, and profile +precedence retain their existing evaluator. Rule filtering uses the catalog +values already read by that check, so a later catalog change cannot drop an +exclusion from the decision being evaluated. There is no cross-call cache. + Filesystem preparation uses current task/run and selected-configuration checks. Historical root-reader authorization has been retired; no retained-reader batch is loaded at admission or publication. Workspace-record disclosure still uses task diff --git a/server/src/__tests__/tool-effective-assignments.test.ts b/server/src/__tests__/tool-effective-assignments.test.ts index 13de5533fbf..4b99a23eec2 100644 --- a/server/src/__tests__/tool-effective-assignments.test.ts +++ b/server/src/__tests__/tool-effective-assignments.test.ts @@ -169,6 +169,122 @@ describeDatabase("effective tool assignments", () => { expect((await service.getConnectionAssignmentsForAgent(f.company.id, f.agent.id, [f.connection.id])).allowedConnectionIds).toEqual([]); }); + it("omits unrelated rules and bindings while retaining current connection decisions", async () => { + const f = await fixture(); + const [otherApplication] = await db.insert(toolApplications).values({ companyId: f.company.id, name: "Other app", type: "mcp_http" }).returning(); + const [otherConnection] = await db.insert(toolConnections).values({ companyId: f.company.id, applicationId: otherApplication.id, + name: "Other connection", uid: randomUUID(), transport: "mcp_remote" }).returning(); + const [otherTool] = await db.insert(toolCatalogEntries).values({ companyId: f.company.id, applicationId: otherApplication.id, + connectionId: otherConnection.id, name: "other", toolName: "other", versionHash: "other", riskLevel: "destructive" }).returning(); + await db.insert(toolProfileEntries).values(Array.from({ length: 200 }, (_, index) => [ + { selectorType: "application" as const, applicationId: otherApplication.id }, + { selectorType: "connection" as const, connectionId: otherConnection.id }, + { selectorType: "catalog_entry" as const, catalogEntryId: otherTool.id }, + { selectorType: "tool_name" as const, toolName: `unrelated_${index}` }, + { selectorType: "risk_level" as const, riskLevel: "destructive" as const }, + ].map(entry => ({ companyId: f.company.id, profileId: f.profile.id, effect: index % 2 ? "include" as const : "exclude" as const, ...entry }))).flat()); + await db.insert(toolProfileBindings).values(Array.from({ length: 200 }, () => ({ companyId: f.company.id, + profileId: f.profile.id, targetType: "agent" as const, targetId: randomUUID() }))); + const [include, exclude] = await db.insert(toolProfileEntries).values([ + { companyId: f.company.id, profileId: f.profile.id, selectorType: "connection", effect: "include", connectionId: f.connection.id }, + { companyId: f.company.id, profileId: f.profile.id, selectorType: "catalog_entry", effect: "exclude", catalogEntryId: f.write.id }, + ]).returning(); + const service = toolAccessService(db), before = await compare(f); + const savedRules = await db.select().from(toolProfileEntries); + const events: Array> = [], now = Date.now(); + const capture = createDatabaseResponseCapture({ start: now - 100, end: now + 59_000 }, event => events.push(event)); + installDatabaseResponseCapture(db, { toolProfileEntries, toolProfileBindings }, capture); + try { + const scoped = await service.getConnectionAssignmentsForAgent(f.company.id, f.agent.id, [f.connection.id]); + capture.flush(); + const rules = events.filter(event => event.event === "paperclip.db_response_capture.entry.v1" && event.table === "tool_profile_entries"); + const bindings = events.filter(event => event.event === "paperclip.db_response_capture.entry.v1" && event.table === "tool_profile_bindings"); + expect(rules).toHaveLength(1); + expect(Number(rules[0].responseValueBytes)).toBeLessThan(4096); + expect(rules[0]).toMatchObject({ calls: 1, rows: 2 }); + expect(bindings).toHaveLength(1); + expect(bindings[0]).toMatchObject({ calls: 1, rows: 1 }); + expect(scoped).toEqual({ allowedConnectionIds: [f.connection.id], installedConnections: before.full.installedConnections }); + expect(await db.select().from(toolProfileEntries)).toEqual(savedRules); + await db.update(toolProfileEntries).set({ effect: "exclude" }).where(eq(toolProfileEntries.id, include.id)); + expect((await service.getConnectionAssignmentsForAgent(f.company.id, f.agent.id, [f.connection.id])).allowedConnectionIds).toEqual([]); + await db.delete(toolProfileEntries).where(eq(toolProfileEntries.id, include.id)); + expect((await service.getConnectionAssignmentsForAgent(f.company.id, f.agent.id, [f.connection.id])).allowedConnectionIds).toEqual([]); + expect((await db.select().from(toolProfileEntries).where(eq(toolProfileEntries.id, exclude.id)))[0]).toEqual(exclude); + } finally { capture.flush(); } + }); + + it("preserves null application matches and exact special-character tool names", async () => { + const f = await fixture(), toolName = "quoted' path\\tool_🙂"; + await db.update(toolCatalogEntries).set({ status: "removed" }).where(eq(toolCatalogEntries.id, f.write.id)); + await db.update(toolCatalogEntries).set({ applicationId: null, toolName }).where(eq(toolCatalogEntries.id, f.read.id)); + await db.update(toolProfiles).set({ defaultAction: "allow" }).where(eq(toolProfiles.id, f.profile.id)); + const [rule] = await db.insert(toolProfileEntries).values({ companyId: f.company.id, profileId: f.profile.id, + selectorType: "application", applicationId: null, effect: "exclude" }).returning(); + const service = toolAccessService(db); + expect((await compare(f)).assignments.allowedTools).toEqual([]); + expect((await service.getConnectionAssignmentsForAgent(f.company.id, f.agent.id, [f.connection.id])).allowedConnectionIds).toEqual([]); + await db.update(toolProfileEntries).set({ selectorType: "tool_name", toolName }).where(eq(toolProfileEntries.id, rule.id)); + expect((await compare(f)).assignments.allowedTools).toEqual([]); + expect((await service.getConnectionAssignmentsForAgent(f.company.id, f.agent.id, [f.connection.id])).allowedConnectionIds).toEqual([]); + await db.update(toolProfileEntries).set({ toolName: `${toolName}different` }).where(eq(toolProfileEntries.id, rule.id)); + expect((await compare(f)).assignments.allowedTools.map(tool => tool.id)).toEqual([f.read.id]); + expect((await service.getConnectionAssignmentsForAgent(f.company.id, f.agent.id, [f.connection.id])).allowedConnectionIds).toEqual([f.connection.id]); + }); + + it.each(["application", "connection", "catalog_entry", "tool_name", "risk_level"] as const)("denies the connection when a %s exclusion blocks its only active tool", async selectorType => { + const f = await fixture(); + await db.update(toolCatalogEntries).set({ status: "removed" }).where(eq(toolCatalogEntries.id, f.write.id)); + await db.update(toolProfiles).set({ defaultAction: "allow" }).where(eq(toolProfiles.id, f.profile.id)); + const [rule] = await db.insert(toolProfileEntries).values({ companyId: f.company.id, profileId: f.profile.id, selectorType, effect: "exclude", + ...(selectorType === "application" ? { applicationId: f.application.id } : {}), + ...(selectorType === "connection" ? { connectionId: f.connection.id } : {}), + ...(selectorType === "catalog_entry" ? { catalogEntryId: f.read.id } : {}), + ...(selectorType === "tool_name" ? { toolName: f.read.toolName } : {}), + ...(selectorType === "risk_level" ? { riskLevel: "read" as const } : {}), + }).returning(); + const service = toolAccessService(db); + expect((await service.getConnectionAssignmentsForAgent(f.company.id, f.agent.id, [f.connection.id])).allowedConnectionIds).toEqual([]); + expect((await compare(f)).assignments.allowedTools).toEqual([]); + await db.delete(toolProfileEntries).where(eq(toolProfileEntries.id, rule.id)); + expect((await service.getConnectionAssignmentsForAgent(f.company.id, f.agent.id, [f.connection.id])).allowedConnectionIds).toEqual([f.connection.id]); + }); + + it("filters rules against the catalog rows being evaluated when the catalog changes during the read", async () => { + const f = await fixture(); + await db.update(toolProfiles).set({ defaultAction: "allow" }).where(eq(toolProfiles.id, f.profile.id)); + await db.update(toolCatalogEntries).set({ status: "removed" }).where(eq(toolCatalogEntries.id, f.write.id)); + await db.insert(toolProfileEntries).values({ companyId: f.company.id, profileId: f.profile.id, + selectorType: "tool_name", toolName: f.read.toolName, effect: "exclude" }); + let changed = false; + const changingDb = new Proxy(db, { get(target, key) { + if (key !== "select") return Reflect.get(target, key); + return (...fields: Parameters) => { + const builder = target.select(...fields), from = builder.from.bind(builder); + builder.from = ((...args: Parameters) => { + const selection = from(...args); + if (args[0] === toolCatalogEntries) { + const execute = selection.execute.bind(selection); + selection.execute = async () => { + const rows = await execute(); + if (!changed) { + changed = true; + await db.update(toolCatalogEntries).set({ toolName: "changed-after-read" }).where(eq(toolCatalogEntries.id, f.read.id)); + } + return rows; + }; + } + return selection; + }) as typeof builder.from; + return builder; + }; + } }); + const service = toolAccessService(changingDb); + expect((await service.getConnectionAssignmentsForAgent(f.company.id, f.agent.id, [f.connection.id])).allowedConnectionIds).toEqual([]); + expect(changed).toBe(true); + expect((await service.getConnectionAssignmentsForAgent(f.company.id, f.agent.id, [f.connection.id])).allowedConnectionIds).toEqual([f.connection.id]); + }); + it("keeps other companies' catalog rows out of scoped assignments", async () => { const f = await fixture(); const other = await fixture(); diff --git a/server/src/services/tool-access/profiles/resolution.ts b/server/src/services/tool-access/profiles/resolution.ts index 3cc4d36df4b..c237636eb2b 100644 --- a/server/src/services/tool-access/profiles/resolution.ts +++ b/server/src/services/tool-access/profiles/resolution.ts @@ -1,4 +1,4 @@ -import { and, asc, eq, inArray } from "drizzle-orm"; +import { and, asc, eq, inArray, or, sql, type AnyColumn } from "drizzle-orm"; import type { Db } from "@paperclipai/db"; import { agents, toolCatalogEntries, toolProfileBindings, toolProfileEntries, toolProfiles } from "@paperclipai/db"; import type { ToolCatalogEntry, ToolConnection, ToolProfileEffectiveSummary, ToolProfileWithDetails } from "@paperclipai/shared"; @@ -10,6 +10,12 @@ type ProfileAssignmentDependencies = { toCatalogEntry(row: typeof toolCatalogEntries.$inferSelect): ToolCatalogEntry; }; +function catalogValuesContain(column: AnyColumn, values: Array) { + const distinct = [...new Set(values)]; + return sql`(${JSON.stringify(distinct.filter(value => value !== null))}::jsonb ? ${column}::text + or (${distinct.includes(null)} and ${column} is null))`; +} + function resolveCatalogAccess( profiles: Array>, entries: Array, @@ -69,20 +75,17 @@ export function createToolProfileResolver(db: Db, { assertOptionalAgent, resolve agentId, "Tool profile effective agent", ); - const allBindings = await db + const matchingBindings = await db .select() .from(toolProfileBindings) - .where(eq(toolProfileBindings.companyId, companyId)) + .where(and(eq(toolProfileBindings.companyId, companyId), or( + and(eq(toolProfileBindings.targetType, "company"), eq(toolProfileBindings.targetId, companyId)), + and(eq(toolProfileBindings.targetType, "agent"), eq(toolProfileBindings.targetId, agentId)), + ))) .orderBy( asc(toolProfileBindings.priority), asc(toolProfileBindings.createdAt), ); - const matchingBindings = allBindings.filter( - (binding) => - (binding.targetType === "company" && - binding.targetId === companyId) || - (binding.targetType === "agent" && binding.targetId === agentId), - ); const candidateProfileIds = profileIdsInBindingOrder(matchingBindings); if (candidateProfileIds.length === 0) return { bindings: [], activeProfiles: [] }; const candidateProfiles = await db @@ -194,29 +197,34 @@ export function createToolProfileResolver(db: Db, { assertOptionalAgent, resolve if (activeProfiles.length === 0 || connectionIds.length === 0) { return { allowedConnectionIds: [], installedConnections: await resolveInstalledConnectionsForAgent(companyId, agentId) }; } - const [entries, catalog] = await Promise.all([ - db.select({ - profileId: toolProfileEntries.profileId, - effect: toolProfileEntries.effect, - selectorType: toolProfileEntries.selectorType, - applicationId: toolProfileEntries.applicationId, - connectionId: toolProfileEntries.connectionId, - catalogEntryId: toolProfileEntries.catalogEntryId, - toolName: toolProfileEntries.toolName, - riskLevel: toolProfileEntries.riskLevel, - }).from(toolProfileEntries).where(and( - eq(toolProfileEntries.companyId, companyId), inArray(toolProfileEntries.profileId, activeProfiles.map(profile => profile.id)), - )), - db.select({ - id: toolCatalogEntries.id, - applicationId: toolCatalogEntries.applicationId, - connectionId: toolCatalogEntries.connectionId, - toolName: toolCatalogEntries.toolName, - riskLevel: toolCatalogEntries.riskLevel, - }).from(toolCatalogEntries).where(and( - eq(toolCatalogEntries.companyId, companyId), eq(toolCatalogEntries.status, "active"), inArray(toolCatalogEntries.connectionId, connectionIds), - )), - ]); + const catalog = await db.select({ + id: toolCatalogEntries.id, + applicationId: toolCatalogEntries.applicationId, + connectionId: toolCatalogEntries.connectionId, + toolName: toolCatalogEntries.toolName, + riskLevel: toolCatalogEntries.riskLevel, + }).from(toolCatalogEntries).where(and( + eq(toolCatalogEntries.companyId, companyId), eq(toolCatalogEntries.status, "active"), inArray(toolCatalogEntries.connectionId, connectionIds), + )); + const entries = catalog.length === 0 ? [] : await db.select({ + profileId: toolProfileEntries.profileId, + effect: toolProfileEntries.effect, + selectorType: toolProfileEntries.selectorType, + applicationId: toolProfileEntries.applicationId, + connectionId: toolProfileEntries.connectionId, + catalogEntryId: toolProfileEntries.catalogEntryId, + toolName: toolProfileEntries.toolName, + riskLevel: toolProfileEntries.riskLevel, + }).from(toolProfileEntries).where(and( + eq(toolProfileEntries.companyId, companyId), inArray(toolProfileEntries.profileId, activeProfiles.map(profile => profile.id)), + or( + and(eq(toolProfileEntries.selectorType, "application"), catalogValuesContain(toolProfileEntries.applicationId, catalog.map(entry => entry.applicationId))), + and(eq(toolProfileEntries.selectorType, "connection"), catalogValuesContain(toolProfileEntries.connectionId, catalog.map(entry => entry.connectionId))), + and(eq(toolProfileEntries.selectorType, "catalog_entry"), catalogValuesContain(toolProfileEntries.catalogEntryId, catalog.map(entry => entry.id))), + and(eq(toolProfileEntries.selectorType, "tool_name"), catalogValuesContain(toolProfileEntries.toolName, catalog.map(entry => entry.toolName))), + and(eq(toolProfileEntries.selectorType, "risk_level"), catalogValuesContain(toolProfileEntries.riskLevel, catalog.map(entry => entry.riskLevel))), + ), + )); const { allowedCatalogIds } = resolveCatalogAccess(activeProfiles, entries, catalog); return { allowedConnectionIds: [...new Set(catalog.filter(entry => allowedCatalogIds.has(entry.id)).map(entry => entry.connectionId))],