From eacf679fe23a75ab91c1bc19c642f3c2c02186a0 Mon Sep 17 00:00:00 2001 From: Dotta Date: Fri, 21 Aug 2026 03:27:20 +0000 Subject: [PATCH] fix: lock instance-level access removal Paperclip-Task: PAP-17797 --- server/src/__tests__/access-service.test.ts | 61 +++++++++++++++++++++ server/src/services/access.ts | 10 +++- 2 files changed, 69 insertions(+), 2 deletions(-) diff --git a/server/src/__tests__/access-service.test.ts b/server/src/__tests__/access-service.test.ts index f1cbb2fb63d..73074ab485f 100644 --- a/server/src/__tests__/access-service.test.ts +++ b/server/src/__tests__/access-service.test.ts @@ -242,6 +242,67 @@ describeEmbeddedPostgres("access service", () => { ).rejects.toThrow("Instance admins cannot be removed from company access"); }); + it("sweeps personal grants when instance-level company access is removed", async () => { + const { company, owner } = await createCompanyWithOwner(db); + const member = await db.insert(companyMemberships).values({ + companyId: company.id, + principalType: "user", + principalId: `member-${randomUUID()}`, + status: "active", + membershipRole: "member", + }).returning().then((rows) => rows[0]!); + const agent = await db.insert(agents).values({ + companyId: company.id, + name: "Instance access delegated agent", + role: "worker", + adapterType: "process", + adapterConfig: {}, + }).returning().then((rows) => rows[0]!); + const application = await db.insert(toolApplications).values({ + companyId: company.id, + applicationKey: `instance-access-${randomUUID()}`, + name: "Instance access app", + type: "mcp", + status: "active", + }).returning().then((rows) => rows[0]!); + const connection = await db.insert(toolConnections).values({ + companyId: company.id, + applicationId: application.id, + name: "Instance access connection", + uid: `instance-access-${randomUUID()}`, + connectionKind: "managed", + ownership: "customer", + transport: "mcp_remote", + authKind: "oauth", + credentialPolicy: "per_user", + status: "active", + enabled: true, + }).returning().then((rows) => rows[0]!); + const grant = await db.insert(connectionGrants).values({ + companyId: company.id, + connectionId: connection.id, + kind: "user", + subjectUserId: member.principalId, + status: "active", + }).returning().then((rows) => rows[0]!); + await db.insert(connectionGrantDelegations).values({ + companyId: company.id, + grantId: grant.id, + agentId: agent.id, + createdByUserId: member.principalId, + }); + + await accessService(db).setUserCompanyAccess(member.principalId, [], { + actorUserId: owner.principalId, + }); + + expect(await db.select().from(companyMemberships).where(eq(companyMemberships.id, member.id))) + .toEqual([expect.objectContaining({ status: "archived" })]); + expect(await db.select().from(connectionGrantDelegations)).toHaveLength(0); + expect(await db.select().from(connectionGrants).where(eq(connectionGrants.id, grant.id))) + .toEqual([expect.objectContaining({ status: "revoked" })]); + }); + it("revokes personal connection access and destroys user-scoped secrets when membership is archived", async () => { const { company, owner } = await createCompanyWithOwner(db); const member = await db.insert(companyMemberships).values({ diff --git a/server/src/services/access.ts b/server/src/services/access.ts index 48709a4167d..e5adcd3028e 100644 --- a/server/src/services/access.ts +++ b/server/src/services/access.ts @@ -585,11 +585,17 @@ export function accessService(db: Db) { companyIds: string[], options: { actorUserId?: string | null } = {}, ) { - const existing = await listUserCompanyAccess(userId); - const existingByCompany = new Map(existing.map((row) => [row.companyId, row])); const target = new Set(companyIds); await db.transaction(async (tx) => { + // Serialize every company-access removal/reactivation with personal OAuth + // completion, which locks the same membership row before writing secrets. + const existing = await tx + .select() + .from(companyMemberships) + .where(and(eq(companyMemberships.principalType, "user"), eq(companyMemberships.principalId, userId))) + .for("update"); + const existingByCompany = new Map(existing.map((row) => [row.companyId, row])); const toArchive = existing.filter((row) => !target.has(row.companyId) && row.status !== "archived"); if (toArchive.length > 0 && options.actorUserId && options.actorUserId === userId) { throw conflict("You cannot remove yourself");