diff --git a/doc/connections/CONNECTOR-PLAYBOOK.md b/doc/connections/CONNECTOR-PLAYBOOK.md index 9343d1441c0..b79df0be8be 100644 --- a/doc/connections/CONNECTOR-PLAYBOOK.md +++ b/doc/connections/CONNECTOR-PLAYBOOK.md @@ -764,6 +764,17 @@ IDs to an auditable profile, and leaves **Ask first** available for any action. The open default changes the initial policy; it does not create a route around a policy the operator has applied. +Saving a connection's installation list grants its app profile only to newly +added targets. Targets that remain installed keep their existing profile +assignments. If an operator removes an app profile from an installed agent or +company, a later installation-list save must preserve that removal. This also +applies when the list is unchanged or another agent is removed. Removing and +then reinstalling a target is a new installation and uses the current setup +defaults. +Every non-empty installation-list save also removes legacy connection-wide +include entries from the managed app profile. Keep this cleanup when no targets +are added, so the selected action entries remain authoritative. + If a destructive provider cannot be safe with those defaults, add a narrowly reviewed provider policy with tests. Do not hide a dangerous tool by misclassifying it as read, and do not silently change global defaults in a diff --git a/maps/agent-access-map/app-permissions.md b/maps/agent-access-map/app-permissions.md index 25258dcac9b..5d91fa39f81 100644 --- a/maps/agent-access-map/app-permissions.md +++ b/maps/agent-access-map/app-permissions.md @@ -94,8 +94,12 @@ Gaps: This individual operation has source review but no recorded HTTP verificat API: `present` — `PUT /api/tool-connections/{connectionId}/installs`; JSON REST. Agent access: `available` — agent REST and default native `search_api` / `call_api`. Policy: `tools:manage_profiles` and `tools:manage_connections`; [shared management policy](../../doc/plans/2026-10-13-connection-api-parity.md#authorization-contract). Resolved resources and every affected target are checked; personal ownership remains the responsible user. +Only new installation targets receive the app-profile binding. Existing targets +retain their saved profile assignments, including deliberately removed grants. +Non-empty saves still remove legacy connection-wide include entries from the +managed app profile when no new targets are added. Evidence: `source-only` — [route](../../server/src/routes/tool-access.ts); reviewed in the working tree based on the source commit above. -Verification: `not-run` — individual operation not executed in this audit; shared authorization and representative family flows are exercised by the authenticated integration suite. Follow the route schema, inspect persistence/audit, and assert owner, scope and revoked-authority denial. +Verification: `not-run` — individual agent-authenticated operation not executed in this audit; shared authorization and representative family flows are exercised by the authenticated integration suite. [Installation permission regressions](../../server/src/__tests__/tool-connection-installs.test.ts) exercise operator HTTP requests, persisted assignments, tool-policy decisions, and access-extension audit records with PostgreSQL. Those fixtures do not qualify agent authentication. Follow the route schema, inspect persistence/audit, and assert owner, scope and revoked-authority denial. Gaps: This individual operation has source review but no recorded HTTP verification. Provider availability and live setup remain unqualified. ## `action-policy` diff --git a/maps/feature-map/app-permissions.md b/maps/feature-map/app-permissions.md index 41da9f9496a..524cc9b4feb 100644 --- a/maps/feature-map/app-permissions.md +++ b/maps/feature-map/app-permissions.md @@ -20,6 +20,12 @@ Agent access: [companion](../agent-access-map/app-permissions.md). - `test-action`: `app-permissions/test-action` — choose an actor and inputs and inspect a real action result. - `account-maintenance`: `app-permissions/account-maintenance` — inspect accounts, reconnect/revoke, and review connection activity. +Changing the connection's agent list preserves profile assignments for agents +that remain installed. Saving the list must not restore a removed app-profile +assignment. New installations retain the current app-profile defaults. +Non-empty saves also remove legacy connection-wide include rules, including +unchanged lists and removals with no new targets. + ## How to get to it (user POV) ### `permissions` @@ -45,6 +51,14 @@ Preconditions: follow the [baseline](./README.md#before-driving-a-journey). Prep Automated: [app detail](../../ui/src/pages/apps/AppDetail.test.tsx) covers agent/action controls, quarantined actions, and navigation using mocked requests. +[Installation permission regressions](../../server/src/__tests__/tool-connection-installs.test.ts) +exercise the real install API with a local operator and PostgreSQL. They check +that adding, removing, or retaining agents preserves removed app-profile grants +and denies write tools for agents with read-only profiles. They also check +company installations, new-agent defaults, legacy connection-wide include +cleanup, and access-extension audit records. +These fixtures do not qualify an agent-authenticated request or a live provider. + [Runtime assignment tests](../../server/src/__tests__/agent-runtime-assignments.test.ts) use PostgreSQL to verify committed assignment notifications, bounded snapshots, rollback, and concurrent AI adoption. The owning lifecycle plugin can read diff --git a/server/src/__tests__/tool-connection-installs.test.ts b/server/src/__tests__/tool-connection-installs.test.ts new file mode 100644 index 00000000000..2791a7a8174 --- /dev/null +++ b/server/src/__tests__/tool-connection-installs.test.ts @@ -0,0 +1,226 @@ +import { randomUUID } from "node:crypto"; +import express from "express"; +import request from "supertest"; +import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { + activityLog, + agents, + companies, + closeRegisteredClients, + connectionGrants, + createDb, + toolApplications, + toolCatalogEntries, + toolConnections, + toolProfileBindings, + toolProfileEntries, + toolProfiles, + type Db, +} from "@paperclipai/db"; +import { and, eq } from "drizzle-orm"; +import { errorHandler } from "../middleware/index.js"; +import { toolAccessRoutes } from "../routes/tool-access.js"; +import { toolAccessPolicyService } from "../services/tool-access-policy.js"; +import { toolAccessService } from "../services/tool-access.js"; +import { + getEmbeddedPostgresTestSupport, + startEmbeddedPostgresTestDatabase, +} from "./helpers/embedded-postgres.js"; + +const externalDatabaseUrl = process.env.PAPERCLIP_TOOL_ACCESS_TEST_DATABASE_URL?.trim(); +const support = externalDatabaseUrl ? { supported: true } : await getEmbeddedPostgresTestSupport(); +const describeDatabase = support.supported ? describe : describe.skip; + +function createRouteApp(db: Db) { + const app = express(); + app.use(express.json()); + app.use((req, _res, next) => { + req.actor = { + type: "board", + userId: "board-user", + isInstanceAdmin: true, + source: "local_implicit", + }; + next(); + }); + app.use("/api", toolAccessRoutes(db)); + app.use(errorHandler); + return app; +} + +describeDatabase("connection install permission preservation", () => { + let database: Awaited>; + let db: Db; + + beforeAll(async () => { + if (externalDatabaseUrl) { + db = createDb(externalDatabaseUrl); + return; + } + database = await startEmbeddedPostgresTestDatabase("paperclip-install-permissions-"); + db = createDb(database.connectionString); + }, 90_000); + + afterAll(async () => { + if (externalDatabaseUrl) await closeRegisteredClients(externalDatabaseUrl); + await database?.cleanup(); + }); + + async function fixture() { + const [company] = await db.insert(companies).values({ + name: "Install permission fixture", + issuePrefix: `IP${randomUUID().slice(0, 6)}`, + }).returning(); + const companyId = company.id; + const agentIds = [randomUUID(), randomUUID(), randomUUID(), randomUUID()]; + await db.insert(agents).values(agentIds.map(id => ({ + id, companyId, name: id, status: "active", adapterType: "process", + }))); + const [application] = await db.insert(toolApplications).values({ + companyId, name: "Fixture app", type: "mcp_http", + }).returning(); + const [connection] = await db.insert(toolConnections).values({ + companyId, applicationId: application.id, name: "Fixture connection", + uid: randomUUID(), transport: "mcp_remote", status: "active", enabled: true, + }).returning(); + await db.insert(connectionGrants).values({ + companyId, connectionId: connection.id, kind: "organization", status: "active", isDefault: true, + }); + const catalog = await db.insert(toolCatalogEntries).values([ + { name: "read_email", riskLevel: "read" as const, isReadOnly: true, isWrite: false }, + { name: "send_email", riskLevel: "write" as const, isReadOnly: false, isWrite: true }, + ].map(tool => ({ + ...tool, toolName: tool.name, companyId, applicationId: application.id, + connectionId: connection.id, versionHash: "fixture-v1", + }))).returning(); + const [appProfile] = await db.insert(toolProfiles).values({ + companyId, profileKey: `app:${connection.id}`, name: "Fixture app access", + defaultAction: "deny", metadata: { source: "app_gallery_finish", connectionId: connection.id }, + }).returning(); + await db.insert(toolProfileEntries).values(catalog.map(entry => ({ + companyId, profileId: appProfile.id, selectorType: "catalog_entry" as const, + catalogEntryId: entry.id, connectionId: connection.id, + }))); + const [limitedProfile] = await db.insert(toolProfiles).values({ + companyId, profileKey: "read-only", name: "Read email only", defaultAction: "deny", + }).returning(); + await db.insert(toolProfileEntries).values({ + companyId, profileId: limitedProfile.id, selectorType: "catalog_entry", catalogEntryId: catalog[0].id, + }); + const app = createRouteApp(db); + const service = toolAccessService(db); + const policy = toolAccessPolicyService(db); + const putTargets = (installs: Array<{ targetType: "agent" | "company"; targetId: string }>) => request(app) + .put(`/api/tool-connections/${connection.id}/installs`) + .send({ installs }) + .expect(response => expect(response.status, JSON.stringify(response.body)).toBe(200)); + const putInstalls = (ids: string[]) => putTargets(ids.map(targetId => ({ targetType: "agent", targetId }))); + const appBindings = () => db.select().from(toolProfileBindings) + .where(and(eq(toolProfileBindings.companyId, companyId), eq(toolProfileBindings.profileId, appProfile.id))); + const extensions = () => db.select().from(activityLog).where(and( + eq(activityLog.companyId, companyId), eq(activityLog.action, "tool_connection.install_access_extended"), + )); + const decide = (agentId: string, toolName: string) => policy.decide({ + companyId, actor: { actorType: "agent", actorId: agentId, agentId }, + request: { + connectionId: connection.id, catalogEntryId: catalog.find(entry => entry.toolName === toolName)!.id, + toolName, arguments: {}, + }, + }); + return { companyId, agentIds, connection, catalog, appProfile, limitedProfile, service, putTargets, putInstalls, appBindings, extensions, decide }; + } + + it.each(["adding an agent", "removing an agent", "saving the same list"])( + "preserves removed app grants when %s", + async change => { + const f = await fixture(); + const [first, second, fullAccess, added] = f.agentIds; + await f.putInstalls([first, second, fullAccess]); + for (const agentId of [first, second]) { + await expect(f.decide(agentId, "send_email")).resolves.toMatchObject({ decision: "allow" }); + await f.service.bindProfile(f.limitedProfile.id, { targetType: "agent", targetId: agentId }); + await f.service.unbindProfile(f.appProfile.id, { targetType: "agent", targetId: agentId }); + await expect(f.decide(agentId, "read_email")).resolves.toMatchObject({ decision: "allow" }); + await expect(f.decide(agentId, "send_email")).resolves.toMatchObject({ decision: "deny", reasonCode: "deny_default" }); + } + const priorBindings = await f.appBindings(); + const priorEvents = await f.extensions(); + const desired = change === "adding an agent" ? [first, second, fullAccess, added] + : change === "removing an agent" ? [first, second] : [first, second, fullAccess]; + + await f.putInstalls(desired); + + for (const agentId of [first, second]) { + await expect(f.decide(agentId, "read_email")).resolves.toMatchObject({ decision: "allow" }); + await expect(f.decide(agentId, "send_email")).resolves.toMatchObject({ decision: "deny", reasonCode: "deny_default" }); + const effective = await f.service.getEffectiveProfilesForAgent(f.companyId, agentId); + expect(effective.installedConnections.map(connection => connection.id)).toContain(f.connection.id); + } + const bindings = await f.appBindings(); + expect(bindings.map(binding => binding.targetId).sort()).toEqual( + desired.filter(id => id !== first && id !== second).sort(), + ); + expect((await f.service.listConnectionInstalls(f.connection.id)).map(install => install.targetId).sort()) + .toEqual([...desired].sort()); + const newEvents = (await f.extensions()).filter(event => !priorEvents.some(prior => prior.id === event.id)); + expect(newEvents.map(event => event.details?.targetId)).toEqual(change === "adding an agent" ? [added] : []); + if (change !== "removing an agent") { + expect(bindings.find(binding => binding.targetId === fullAccess)).toEqual(priorBindings[0]); + await expect(f.decide(fullAccess, "send_email")).resolves.toMatchObject({ decision: "allow" }); + } + if (change === "adding an agent") { + await expect(f.decide(added, "send_email")).resolves.toMatchObject({ decision: "allow", reasonCode: "allow_profile" }); + } + }, + ); + + it.each(["saving the same list", "removing an agent"])( + "removes legacy connection-wide includes when %s", + async change => { + const f = await fixture(); + const [first, second] = f.agentIds; + await f.putInstalls([first, second]); + const writeEntry = f.catalog.find(entry => entry.toolName === "send_email")!; + await db.delete(toolProfileEntries).where(and( + eq(toolProfileEntries.profileId, f.appProfile.id), eq(toolProfileEntries.catalogEntryId, writeEntry.id), + )); + await db.insert(toolProfileEntries).values({ + companyId: f.companyId, profileId: f.appProfile.id, selectorType: "connection", + effect: "include", connectionId: f.connection.id, + }); + await expect(f.decide(first, "send_email")).resolves.toMatchObject({ decision: "allow", reasonCode: "allow_profile" }); + const priorBindings = await f.appBindings(); + const priorEvents = await f.extensions(); + const desired = change === "removing an agent" ? [first] : [first, second]; + + await f.putInstalls(desired); + + await expect(f.decide(first, "read_email")).resolves.toMatchObject({ decision: "allow" }); + await expect(f.decide(first, "send_email")).resolves.toMatchObject({ decision: "deny", reasonCode: "deny_default" }); + expect(await db.select().from(toolProfileEntries).where(and( + eq(toolProfileEntries.profileId, f.appProfile.id), eq(toolProfileEntries.selectorType, "connection"), + ))).toEqual([]); + expect((await f.appBindings()).sort((a, b) => a.targetId.localeCompare(b.targetId))).toEqual( + priorBindings.filter(binding => desired.includes(binding.targetId)).sort((a, b) => a.targetId.localeCompare(b.targetId)), + ); + expect(await f.extensions()).toEqual(priorEvents); + }, + ); + + it("preserves a removed company app grant when saving a company install", async () => { + const f = await fixture(); + const input = { installs: [{ targetType: "company" as const, targetId: f.companyId }] }; + await f.putTargets(input.installs); + await f.service.unbindProfile(f.appProfile.id, { targetType: "company", targetId: f.companyId }); + const priorEvents = await f.extensions(); + + await f.putTargets(input.installs); + + await expect(f.decide(f.agentIds[0], "send_email")).resolves.toMatchObject({ decision: "deny", reasonCode: "deny_default" }); + expect(await f.appBindings()).toEqual([]); + expect(await f.extensions()).toEqual(priorEvents); + expect(await f.service.listConnectionInstalls(f.connection.id)).toEqual([ + expect.objectContaining({ targetType: "company", targetId: f.companyId }), + ]); + }); +}); diff --git a/server/src/services/tool-access/connections/installs.ts b/server/src/services/tool-access/connections/installs.ts index e16cda72c2c..6e4459a4f37 100644 --- a/server/src/services/tool-access/connections/installs.ts +++ b/server/src/services/tool-access/connections/installs.ts @@ -275,7 +275,7 @@ export function createConnectionInstalls(db: Db, { now, getConnectionRow, assert } if (requested.size > 0 && connection.connectionPurpose !== "ai") { const profile = await appProfileForConnection(tx, connection); - for (const install of requested.values()) { + for (const install of additions) { const [binding] = await tx .insert(toolProfileBindings) .values({