diff --git a/packages/shared/src/constants.ts b/packages/shared/src/constants.ts index aab701a6703..287ae7835be 100644 --- a/packages/shared/src/constants.ts +++ b/packages/shared/src/constants.ts @@ -849,7 +849,10 @@ export type JoinRequestStatus = (typeof JOIN_REQUEST_STATUSES)[number]; export const PERMISSION_KEYS = [ "agents:create", + "agents:configure", + "agents:suggest-changes", "skills:create", + "skills:suggest-changes", "environments:manage", "users:invite", "users:manage_permissions", diff --git a/packages/shared/src/types/company-portability.ts b/packages/shared/src/types/company-portability.ts index fd2bb5b1c19..1434ab733a2 100644 --- a/packages/shared/src/types/company-portability.ts +++ b/packages/shared/src/types/company-portability.ts @@ -1,6 +1,6 @@ import type { AgentEnvConfig } from "./secrets.js"; import type { RoutineVariable } from "./routine.js"; -import type { IssueCommentAuthorType } from "../constants.js"; +import type { IssueCommentAuthorType, PermissionKey } from "../constants.js"; import type { IssueCommentMetadata, IssueCommentPresentation } from "./issue.js"; export interface CompanyPortabilityInclude { @@ -145,6 +145,10 @@ export interface CompanyPortabilityAgentManifestEntry { adapterConfig: Record; runtimeConfig: Record; permissions: Record; + permissionGrants: Array<{ + permissionKey: PermissionKey; + scope: Record | null; + }>; budgetMonthlyCents: number; metadata: Record | null; } diff --git a/packages/shared/src/validators/company-portability.ts b/packages/shared/src/validators/company-portability.ts index d6d81d964e2..8a1e97a6cc6 100644 --- a/packages/shared/src/validators/company-portability.ts +++ b/packages/shared/src/validators/company-portability.ts @@ -1,4 +1,5 @@ import { z } from "zod"; +import { PERMISSION_KEYS } from "../constants.js"; import { MAX_COMPANY_ATTACHMENT_MAX_BYTES } from "../constants.js"; import { issueCommentAuthorTypeSchema, @@ -70,6 +71,10 @@ export const portabilityAgentManifestEntrySchema = z.object({ adapterConfig: z.record(z.string(), z.unknown()), runtimeConfig: z.record(z.string(), z.unknown()), permissions: z.record(z.string(), z.unknown()), + permissionGrants: z.array(z.object({ + permissionKey: z.enum(PERMISSION_KEYS), + scope: z.record(z.string(), z.unknown()).nullable().default(null), + })).default([]), budgetMonthlyCents: z.number().int().nonnegative(), metadata: z.record(z.string(), z.unknown()).nullable(), }); diff --git a/server/src/__tests__/adapter-model-refresh-routes.test.ts b/server/src/__tests__/adapter-model-refresh-routes.test.ts index 68553e9d31d..a328a16c555 100644 --- a/server/src/__tests__/adapter-model-refresh-routes.test.ts +++ b/server/src/__tests__/adapter-model-refresh-routes.test.ts @@ -80,6 +80,7 @@ function registerModuleMocks() { agentInstructionsService: () => mockAgentInstructionsService, accessService: () => mockAccessService, approvalService: () => mockApprovalService, + builtInAgentService: () => ({ ensureCompanyDefaultAgentGrants: vi.fn() }), companySkillService: () => mockCompanySkillService, budgetService: () => mockBudgetService, heartbeatService: () => mockHeartbeatService, diff --git a/server/src/__tests__/agent-adapter-validation-routes.test.ts b/server/src/__tests__/agent-adapter-validation-routes.test.ts index 40a7266ac65..f17158ce378 100644 --- a/server/src/__tests__/agent-adapter-validation-routes.test.ts +++ b/server/src/__tests__/agent-adapter-validation-routes.test.ts @@ -69,6 +69,7 @@ vi.mock("../services/index.js", () => ({ agentInstructionsService: () => mockAgentInstructionsService, accessService: () => mockAccessService, approvalService: () => mockApprovalService, + builtInAgentService: () => ({ ensureCompanyDefaultAgentGrants: vi.fn() }), companySkillService: () => mockCompanySkillService, budgetService: () => mockBudgetService, heartbeatService: () => mockHeartbeatService, @@ -94,6 +95,7 @@ function registerModuleMocks() { agentInstructionsService: () => mockAgentInstructionsService, accessService: () => mockAccessService, approvalService: () => mockApprovalService, + builtInAgentService: () => ({ ensureCompanyDefaultAgentGrants: vi.fn() }), companySkillService: () => mockCompanySkillService, budgetService: () => mockBudgetService, heartbeatService: () => mockHeartbeatService, diff --git a/server/src/__tests__/agent-cross-tenant-authz-routes.test.ts b/server/src/__tests__/agent-cross-tenant-authz-routes.test.ts index 765f0c507f1..738d2ebf208 100644 --- a/server/src/__tests__/agent-cross-tenant-authz-routes.test.ts +++ b/server/src/__tests__/agent-cross-tenant-authz-routes.test.ts @@ -182,6 +182,7 @@ vi.mock("../services/index.js", () => ({ agentInstructionsService: () => mockAgentInstructionsService, accessService: () => mockAccessService, approvalService: () => mockApprovalService, + builtInAgentService: () => ({ ensureCompanyDefaultAgentGrants: vi.fn() }), companySkillService: () => mockCompanySkillService, budgetService: () => mockBudgetService, heartbeatService: () => mockHeartbeatService, diff --git a/server/src/__tests__/agent-instructions-routes.test.ts b/server/src/__tests__/agent-instructions-routes.test.ts index c2f7dd65289..5fca425834d 100644 --- a/server/src/__tests__/agent-instructions-routes.test.ts +++ b/server/src/__tests__/agent-instructions-routes.test.ts @@ -8,6 +8,10 @@ const mockAgentService = vi.hoisted(() => ({ resolveByReference: vi.fn(), })); +const mockBuiltInAgentService = vi.hoisted(() => ({ + ensureCompanyDefaultAgentGrants: vi.fn(), +})); + const mockAgentInstructionsService = vi.hoisted(() => ({ getBundle: vi.fn(), readFile: vi.fn(), @@ -42,6 +46,7 @@ vi.mock("../services/index.js", () => ({ agentInstructionsService: () => mockAgentInstructionsService, accessService: () => mockAccessService, approvalService: () => ({}), + builtInAgentService: () => mockBuiltInAgentService, companySkillService: () => ({ listRuntimeSkillEntries: vi.fn() }), budgetService: () => ({}), environmentService: () => mockEnvironmentService, @@ -73,6 +78,7 @@ function registerModuleMocks() { agentInstructionsService: () => mockAgentInstructionsService, accessService: () => mockAccessService, approvalService: () => ({}), + builtInAgentService: () => mockBuiltInAgentService, companySkillService: () => ({ listRuntimeSkillEntries: vi.fn() }), budgetService: () => ({}), heartbeatService: () => ({}), @@ -193,6 +199,7 @@ describe("agent instructions bundle routes", () => { vi.doUnmock("../middleware/index.js"); registerModuleMocks(); vi.clearAllMocks(); + mockBuiltInAgentService.ensureCompanyDefaultAgentGrants.mockResolvedValue(0); mockSyncInstructionsBundleConfigFromFilePath.mockImplementation((_agent, config) => config); mockFindServerAdapter.mockImplementation((_type: string) => ({ type: _type })); mockAccessService.decide.mockResolvedValue({ @@ -290,7 +297,11 @@ describe("agent instructions bundle routes", () => { } return makeAgent(); }); - mockAccessService.hasPermission.mockResolvedValue(false); + mockAccessService.decide.mockResolvedValue({ + allowed: false, + reason: "deny_no_grant", + explanation: "Missing permission: agents:configure or agents:suggest-changes.", + }); const res = await requestApp( await createApp({ @@ -305,6 +316,14 @@ describe("agent instructions bundle routes", () => { expect(res.status, JSON.stringify(res.body)).toBe(403); expect(res.body.error).toContain("Missing permission"); + expect(mockAccessService.decide).toHaveBeenCalledWith(expect.objectContaining({ + action: "agent_config:read", + resource: { + type: "agent", + companyId: "company-1", + agentId: "11111111-1111-4111-8111-111111111111", + }, + })); expect(mockAgentInstructionsService.getBundle).not.toHaveBeenCalled(); }); @@ -324,7 +343,18 @@ describe("agent instructions bundle routes", () => { expect(mockAgentInstructionsService.getBundle).toHaveBeenCalled(); }); - it("allows the built-in Reflection Coach to read peer instructions bundles", async () => { + it("allows agents with suggest grants to read peer instructions bundles", async () => { + mockAccessService.decide.mockResolvedValue({ + allowed: true, + reason: "allow_explicit_grant", + explanation: "Allowed by explicit grant agents:suggest-changes.", + grant: { + principalType: "agent", + principalId: "coach-agent", + permissionKey: "agents:suggest-changes", + scope: null, + }, + }); mockAgentService.getById.mockImplementation(async (id: string) => { if (id === "coach-agent") { return makeReflectionCoachAgent({ id: "coach-agent" }); @@ -345,6 +375,14 @@ describe("agent instructions bundle routes", () => { ); expect(res.status, JSON.stringify(res.body)).toBe(200); + expect(mockAccessService.decide).toHaveBeenCalledWith(expect.objectContaining({ + action: "agent_config:read", + resource: { + type: "agent", + companyId: "company-1", + agentId: "11111111-1111-4111-8111-111111111111", + }, + })); expect(mockAgentInstructionsService.readFile).toHaveBeenCalledWith( expect.objectContaining({ id: "11111111-1111-4111-8111-111111111111" }), "AGENTS.md", diff --git a/server/src/__tests__/agent-live-run-routes.test.ts b/server/src/__tests__/agent-live-run-routes.test.ts index b17235b5e8a..f611dfa66ef 100644 --- a/server/src/__tests__/agent-live-run-routes.test.ts +++ b/server/src/__tests__/agent-live-run-routes.test.ts @@ -63,6 +63,7 @@ function registerModuleMocks() { hasPermission: vi.fn(async () => true), }), approvalService: () => ({}), + builtInAgentService: () => ({ ensureCompanyDefaultAgentGrants: vi.fn() }), companySkillService: () => ({ listRuntimeSkillEntries: vi.fn() }), budgetService: () => ({}), heartbeatService: () => mockHeartbeatService, diff --git a/server/src/__tests__/agent-permissions-routes.test.ts b/server/src/__tests__/agent-permissions-routes.test.ts index 9aeda84ac52..1822baee2d4 100644 --- a/server/src/__tests__/agent-permissions-routes.test.ts +++ b/server/src/__tests__/agent-permissions-routes.test.ts @@ -51,6 +51,10 @@ const mockAgentService = vi.hoisted(() => ({ resolveByReference: vi.fn(), })); +const mockBuiltInAgentService = vi.hoisted(() => ({ + ensureCompanyDefaultAgentGrants: vi.fn(), +})); + const mockAccessService = vi.hoisted(() => ({ canUser: vi.fn(), decide: vi.fn(), @@ -195,6 +199,7 @@ function registerModuleMocks() { agentInstructionsService: () => mockAgentInstructionsService, accessService: () => mockAccessService, approvalService: () => mockApprovalService, + builtInAgentService: () => mockBuiltInAgentService, companySkillService: () => mockCompanySkillService, budgetService: () => mockBudgetService, heartbeatService: () => mockHeartbeatService, @@ -309,6 +314,7 @@ describe.sequential("agent permission routes", () => { mockAgentService.updatePermissions.mockReset(); mockAgentService.getChainOfCommand.mockReset(); mockAgentService.resolveByReference.mockReset(); + mockBuiltInAgentService.ensureCompanyDefaultAgentGrants.mockReset(); mockAccessService.canUser.mockReset(); mockAccessService.decide.mockReset(); mockAccessService.hasPermission.mockReset(); @@ -354,6 +360,7 @@ describe.sequential("agent permission routes", () => { }); mockAgentService.update.mockResolvedValue(baseAgent); mockAgentService.updatePermissions.mockResolvedValue(baseAgent); + mockBuiltInAgentService.ensureCompanyDefaultAgentGrants.mockResolvedValue(0); mockAccessService.canUser.mockResolvedValue(true); mockAccessService.decide.mockImplementation(async (input: { action?: string }) => { const allowed = Boolean(await mockAccessService.canUser()); @@ -866,6 +873,7 @@ describe.sequential("agent permission routes", () => { true, "agent-admin-user", ); + expect(mockBuiltInAgentService.ensureCompanyDefaultAgentGrants).toHaveBeenCalledWith(companyId); }); it("rejects direct agent creation when new agents require board approval", async () => { @@ -1699,11 +1707,10 @@ describe.sequential("agent permission routes", () => { expect(res.status).toBe(200); }); - it("denies an agent actor without agents:create when reading peer config", async () => { - // Agent actors must still pass the agents:create gate (explicit - // grant OR canCreateAgents permission on the agent record). A peer - // agent in the same company without that permission must not be - // able to read another agent's configuration. + it("denies an agent actor without configure or suggest grants when reading peer config", async () => { + // Agent actors must pass the agent configuration read ladder. A peer + // agent in the same company without agents:configure or + // agents:suggest-changes must not read another agent's configuration. const peerAgentId = "33333333-3333-4333-8333-333333333333"; const peerAgent = { ...baseAgent, id: peerAgentId }; mockAgentService.getById.mockImplementation(async (id: string) => { @@ -1713,7 +1720,11 @@ describe.sequential("agent permission routes", () => { } return null; }); - mockAccessService.hasPermission.mockResolvedValue(false); + mockAccessService.decide.mockResolvedValue({ + allowed: false, + reason: "deny_no_grant", + explanation: "Missing permission: agents:configure or agents:suggest-changes.", + }); const app = await createApp({ type: "agent", @@ -1726,11 +1737,15 @@ describe.sequential("agent permission routes", () => { const res = await request(app).get(`/api/agents/${peerAgentId}/configuration`); expect(res.status).toBe(403); + expect(mockAccessService.decide).toHaveBeenCalledWith(expect.objectContaining({ + action: "agent_config:read", + resource: { type: "company", companyId }, + })); }); - it("allows an agent actor with agents:create grant to read peer config", async () => { - // When an agent actor has an explicit agents:create grant in the - // access service, the read gate must let them through. + it("allows an agent actor with agents:suggest-changes grant to read peer config", async () => { + // Suggest-tier authority implies read access so the agent can prepare a + // consented diff without receiving direct change authority. const peerAgentId = "44444444-4444-4444-8444-444444444444"; const peerAgent = { ...baseAgent, id: peerAgentId }; mockAgentService.getById.mockImplementation(async (id: string) => { @@ -1740,11 +1755,17 @@ describe.sequential("agent permission routes", () => { } return null; }); - mockAccessService.hasPermission.mockImplementation( - async (_companyId: string, _principalType: string, principalId: string, key: string) => { - return principalId === agentId && key === "agents:create"; + mockAccessService.decide.mockResolvedValue({ + allowed: true, + reason: "allow_explicit_grant", + explanation: "Allowed by explicit grant agents:suggest-changes.", + grant: { + principalType: "agent", + principalId: agentId, + permissionKey: "agents:suggest-changes", + scope: null, }, - ); + }); const app = await createApp({ type: "agent", @@ -1757,6 +1778,10 @@ describe.sequential("agent permission routes", () => { const res = await request(app).get(`/api/agents/${peerAgentId}/configuration`); expect(res.status).toBe(200); + expect(mockAccessService.decide).toHaveBeenCalledWith(expect.objectContaining({ + action: "agent_config:read", + resource: { type: "company", companyId }, + })); }); }); diff --git a/server/src/__tests__/agent-skills-routes.test.ts b/server/src/__tests__/agent-skills-routes.test.ts index 6a30f256af8..83352c41897 100644 --- a/server/src/__tests__/agent-skills-routes.test.ts +++ b/server/src/__tests__/agent-skills-routes.test.ts @@ -85,6 +85,7 @@ vi.mock("../services/index.js", () => ({ agentInstructionsService: () => mockAgentInstructionsService, accessService: () => mockAccessService, approvalService: () => mockApprovalService, + builtInAgentService: () => ({ ensureCompanyDefaultAgentGrants: vi.fn() }), companySkillService: () => mockCompanySkillService, budgetService: () => mockBudgetService, environmentService: () => mockEnvironmentService, @@ -123,6 +124,7 @@ function registerModuleMocks() { agentInstructionsService: () => mockAgentInstructionsService, accessService: () => mockAccessService, approvalService: () => mockApprovalService, + builtInAgentService: () => ({ ensureCompanyDefaultAgentGrants: vi.fn() }), companySkillService: () => mockCompanySkillService, budgetService: () => mockBudgetService, heartbeatService: () => mockHeartbeatService, diff --git a/server/src/__tests__/agent-test-environment-routes.test.ts b/server/src/__tests__/agent-test-environment-routes.test.ts index 40730c051b1..dc3e7ce445d 100644 --- a/server/src/__tests__/agent-test-environment-routes.test.ts +++ b/server/src/__tests__/agent-test-environment-routes.test.ts @@ -45,6 +45,7 @@ vi.mock("../services/index.js", () => ({ agentInstructionsService: () => ({}), accessService: () => mockAccessService, approvalService: () => ({}), + builtInAgentService: () => ({ ensureCompanyDefaultAgentGrants: vi.fn() }), companySkillService: () => ({ listRuntimeSkillEntries: vi.fn(async () => []), resolveRequestedSkillKeys: vi.fn(async () => []), diff --git a/server/src/__tests__/authorization-service.test.ts b/server/src/__tests__/authorization-service.test.ts index 213b192e473..2751f2fe694 100644 --- a/server/src/__tests__/authorization-service.test.ts +++ b/server/src/__tests__/authorization-service.test.ts @@ -12,7 +12,7 @@ import { principalPermissionGrants, projects, } from "@paperclipai/db"; -import { LOW_TRUST_REVIEW_PRESET } from "@paperclipai/shared"; +import { LOW_TRUST_REVIEW_PRESET, type PermissionKey } from "@paperclipai/shared"; import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase, @@ -100,7 +100,7 @@ async function grantAgentPermission( db: ReturnType, companyId: string, agentId: string, - permissionKey: "tasks:assign" | "tasks:assign_scope", + permissionKey: PermissionKey, scope: Record | null = null, ) { await db.insert(companyMemberships).values({ @@ -141,7 +141,7 @@ async function grantUserPermission( db: ReturnType, companyId: string, userId: string, - permissionKey: "tasks:assign" | "tasks:assign_scope", + permissionKey: PermissionKey, scope: Record | null = null, ) { await db.insert(companyMemberships).values({ @@ -224,24 +224,11 @@ describeEmbeddedPostgres("authorization service", () => { expect(decision.explanation).toContain("Allowed by explicit grant tasks:assign"); }); - it("allows agent grants for agent configuration decisions", async () => { - const company = await createCompany(db, "AgentGrant"); + it("allows suggest grants to read peer agent configuration", async () => { + const company = await createCompany(db, "AgentReadGrant"); const actorAgent = await createAgent(db, company.id); const targetAgent = await createAgent(db, company.id); - await db.insert(companyMemberships).values({ - companyId: company.id, - principalType: "agent", - principalId: actorAgent.id, - status: "active", - membershipRole: "member", - }); - await db.insert(principalPermissionGrants).values({ - companyId: company.id, - principalType: "agent", - principalId: actorAgent.id, - permissionKey: "agents:create", - grantedByUserId: null, - }); + await grantAgentPermission(db, company.id, actorAgent.id, "agents:suggest-changes"); const decision = await authorizationService(db).decide({ actor: { type: "agent", agentId: actorAgent.id, companyId: company.id, source: "agent_key" }, @@ -249,8 +236,133 @@ describeEmbeddedPostgres("authorization service", () => { resource: { type: "agent", companyId: company.id, agentId: targetAgent.id }, }); - expect(decision.allowed).toBe(true); - expect(decision.grant?.permissionKey).toBe("agents:create"); + expect(decision).toMatchObject({ + allowed: true, + reason: "allow_explicit_grant", + grant: { + principalType: "agent", + principalId: actorAgent.id, + permissionKey: "agents:suggest-changes", + }, + }); + }); + + it("enforces direct or consented suggest grants for agent configuration changes", async () => { + const company = await createCompany(db, "AgentChangeGrant"); + const directAgent = await createAgent(db, company.id); + const suggestAgent = await createAgent(db, company.id); + const noGrantAgent = await createAgent(db, company.id); + const targetAgent = await createAgent(db, company.id); + await grantAgentPermission(db, company.id, directAgent.id, "agents:configure"); + await grantAgentPermission(db, company.id, suggestAgent.id, "agents:suggest-changes"); + await db.insert(companyMemberships).values({ + companyId: company.id, + principalType: "agent", + principalId: noGrantAgent.id, + status: "active", + membershipRole: "member", + }); + + const authz = authorizationService(db); + await expect(authz.decide({ + actor: { type: "agent", agentId: directAgent.id, companyId: company.id, source: "agent_key" }, + action: "agent_config:update", + resource: { type: "agent", companyId: company.id, agentId: targetAgent.id }, + scope: { requiresChangeGrant: true }, + })).resolves.toMatchObject({ + allowed: true, + reason: "allow_direct_change", + grant: { permissionKey: "agents:configure" }, + }); + + await expect(authz.decide({ + actor: { type: "agent", agentId: suggestAgent.id, companyId: company.id, source: "agent_key" }, + action: "agent_config:update", + resource: { type: "agent", companyId: company.id, agentId: targetAgent.id }, + scope: { requiresChangeGrant: true }, + })).resolves.toMatchObject({ + allowed: false, + reason: "deny_missing_consent", + grant: { permissionKey: "agents:suggest-changes" }, + }); + + await expect(authz.decide({ + actor: { type: "agent", agentId: suggestAgent.id, companyId: company.id, source: "agent_key" }, + action: "agent_config:update", + resource: { type: "agent", companyId: company.id, agentId: targetAgent.id }, + scope: { requiresChangeGrant: true, consentedChange: true }, + })).resolves.toMatchObject({ + allowed: true, + reason: "allow_consented_change", + grant: { permissionKey: "agents:suggest-changes" }, + }); + + await expect(authz.decide({ + actor: { type: "agent", agentId: noGrantAgent.id, companyId: company.id, source: "agent_key" }, + action: "agent_config:update", + resource: { type: "agent", companyId: company.id, agentId: targetAgent.id }, + scope: { requiresChangeGrant: true }, + })).resolves.toMatchObject({ + allowed: false, + reason: "deny_no_grant", + }); + }); + + it("enforces direct or consented suggest grants for skill configuration changes", async () => { + const company = await createCompany(db, "SkillChangeGrant"); + const directAgent = await createAgent(db, company.id); + const suggestAgent = await createAgent(db, company.id); + const noGrantAgent = await createAgent(db, company.id); + await grantAgentPermission(db, company.id, directAgent.id, "skills:create"); + await grantAgentPermission(db, company.id, suggestAgent.id, "skills:suggest-changes"); + await db.insert(companyMemberships).values({ + companyId: company.id, + principalType: "agent", + principalId: noGrantAgent.id, + status: "active", + membershipRole: "member", + }); + + const authz = authorizationService(db); + await expect(authz.decide({ + actor: { type: "agent", agentId: directAgent.id, companyId: company.id, source: "agent_key" }, + action: "skill_config:update", + resource: { type: "company", companyId: company.id }, + })).resolves.toMatchObject({ + allowed: true, + reason: "allow_direct_change", + grant: { permissionKey: "skills:create" }, + }); + + await expect(authz.decide({ + actor: { type: "agent", agentId: suggestAgent.id, companyId: company.id, source: "agent_key" }, + action: "skill_config:update", + resource: { type: "company", companyId: company.id }, + })).resolves.toMatchObject({ + allowed: false, + reason: "deny_missing_consent", + grant: { permissionKey: "skills:suggest-changes" }, + }); + + await expect(authz.decide({ + actor: { type: "agent", agentId: suggestAgent.id, companyId: company.id, source: "agent_key" }, + action: "skill_config:update", + resource: { type: "company", companyId: company.id }, + scope: { consentedChange: true }, + })).resolves.toMatchObject({ + allowed: true, + reason: "allow_consented_change", + grant: { permissionKey: "skills:suggest-changes" }, + }); + + await expect(authz.decide({ + actor: { type: "agent", agentId: noGrantAgent.id, companyId: company.id, source: "agent_key" }, + action: "skill_config:update", + resource: { type: "company", companyId: company.id }, + })).resolves.toMatchObject({ + allowed: false, + reason: "deny_no_grant", + }); }); it("denies cross-company agent decisions before grant evaluation", async () => { diff --git a/server/src/__tests__/built-in-agents.test.ts b/server/src/__tests__/built-in-agents.test.ts index 6775477ac71..2537f12949e 100644 --- a/server/src/__tests__/built-in-agents.test.ts +++ b/server/src/__tests__/built-in-agents.test.ts @@ -9,6 +9,7 @@ import { budgetPolicies, builtInManagedResources, companies, + companyMemberships, companySkillVersions, companySkills, createDb, @@ -68,6 +69,7 @@ describeEmbeddedPostgres("built-in agents", () => { await db.delete(companySkillVersions); await db.delete(companySkills); await db.delete(principalPermissionGrants); + await db.delete(companyMemberships); await db.delete(agentConfigRevisions); await db.delete(activityLog); await db.delete(approvals); @@ -80,6 +82,14 @@ describeEmbeddedPostgres("built-in agents", () => { await tempDb?.cleanup(); }); + async function permissionKeysForAgent(agentId: string) { + const grants = await db + .select() + .from(principalPermissionGrants) + .where(eq(principalPermissionGrants.principalId, agentId)); + return grants.map((grant) => grant.permissionKey).sort(); + } + async function seedCompany(options: { requireApproval?: boolean } = {}) { const companyId = randomUUID(); await db.insert(companies).values({ @@ -415,7 +425,7 @@ describeEmbeddedPostgres("built-in agents", () => { it("auto-provisions a paused Reflection Coach bundle with skill sync and a disabled routine", async () => { const companyId = await seedCompany({ requireApproval: false }); - await agentService(db).create(companyId, { + const root = await agentService(db).create(companyId, { name: "CEO", role: "ceo", status: "idle", @@ -427,6 +437,12 @@ describeEmbeddedPostgres("built-in agents", () => { const result = await reconcileBuiltInAgentsOnStartup(db); expect(result.autoEnsured).toBeGreaterThanOrEqual(1); + expect(result.defaultGrantsEnsured).toBeGreaterThanOrEqual(4); + + const rootGrantKeys = await permissionKeysForAgent(root.id); + expect(rootGrantKeys).toEqual(expect.arrayContaining(["agents:configure", "skills:create"])); + expect(rootGrantKeys).not.toContain("agents:suggest-changes"); + expect(rootGrantKeys).not.toContain("skills:suggest-changes"); const state = await builtInAgentService(db).get(companyId, "reflection-coach"); expect(state).toMatchObject({ @@ -487,6 +503,10 @@ describeEmbeddedPostgres("built-in agents", () => { cronExpression: "0 9 * * 1", timezone: "UTC", }); + const coachGrantKeys = await permissionKeysForAgent(state.agentId!); + expect(coachGrantKeys).toEqual(expect.arrayContaining(["agents:suggest-changes", "skills:suggest-changes"])); + expect(coachGrantKeys).not.toContain("agents:configure"); + expect(coachGrantKeys).not.toContain("skills:create"); }); it("preserves new-agent approval gates during automatic Reflection Coach provisioning", async () => { @@ -757,11 +777,11 @@ describeEmbeddedPostgres("built-in agents", () => { timezone: "UTC", }); - const grants = await db - .select() - .from(principalPermissionGrants) - .where(eq(principalPermissionGrants.principalId, state.agentId!)); - expect(grants.map((grant) => grant.permissionKey)).not.toContain("tasks:assign"); + const grantKeys = await permissionKeysForAgent(state.agentId!); + expect(grantKeys).toEqual(expect.arrayContaining(["agents:suggest-changes", "skills:suggest-changes"])); + expect(grantKeys).not.toContain("tasks:assign"); + expect(grantKeys).not.toContain("agents:configure"); + expect(grantKeys).not.toContain("skills:create"); }); it("controls the Reflection Coach routine schedule without enabling it by default", async () => { diff --git a/server/src/__tests__/company-portability.test.ts b/server/src/__tests__/company-portability.test.ts index 001ea3fd1cb..b7f76b7ff89 100644 --- a/server/src/__tests__/company-portability.test.ts +++ b/server/src/__tests__/company-portability.test.ts @@ -480,6 +480,55 @@ describe("company portability", () => { expect(exported.warnings).toContain("Agent claudecoder PATH override was omitted from export because it is system-dependent."); }); + it("exports agent permission grants through the Paperclip extension and manifest", async () => { + const db = { + select: vi.fn((selection: Record) => ({ + from: vi.fn(() => ({ + where: vi.fn(async () => { + if (!selection.permissionKey) return []; + return [ + { + principalId: "agent-1", + permissionKey: "agents:suggest-changes", + scope: null, + }, + { + principalId: "agent-1", + permissionKey: "skills:create", + scope: { targetAgentIds: ["agent-1"] }, + }, + ]; + }), + })), + })), + }; + const portability = companyPortabilityService(db as any); + + const exported = await portability.exportBundle("company-1", { + include: { + company: true, + agents: true, + projects: false, + issues: false, + }, + }); + + const extension = asTextFile(exported.files[".paperclip.yaml"]); + expect(extension).toContain("permissionGrants:"); + expect(extension).toContain('permissionKey: "agents:suggest-changes"'); + expect(extension).toContain('permissionKey: "skills:create"'); + expect(exported.manifest.agents.find((agent) => agent.slug === "claudecoder")?.permissionGrants).toEqual([ + { + permissionKey: "agents:suggest-changes", + scope: null, + }, + { + permissionKey: "skills:create", + scope: { targetAgentIds: ["agent-1"] }, + }, + ]); + }); + it("exports hire approval policy only when approval is required", async () => { const portability = companyPortabilityService({} as any); @@ -1559,6 +1608,90 @@ describe("company portability", () => { ); }); + it("imports agent permission grants from package metadata", async () => { + const portability = companyPortabilityService({} as any); + agentSvc.list.mockResolvedValue([]); + agentSvc.create.mockImplementation(async (_companyId: string, input: Record) => ({ + id: "agent-imported", + name: input.name, + adapterType: input.adapterType, + adapterConfig: input.adapterConfig, + runtimeConfig: input.runtimeConfig, + status: input.status, + })); + + await portability.importBundle({ + source: { + type: "inline", + files: { + "COMPANY.md": [ + "---", + "name: Import", + "includes:", + " - agents/coder/AGENTS.md", + "---", + "", + ].join("\n"), + "agents/coder/AGENTS.md": [ + "---", + "name: Coder", + "slug: coder", + "kind: agent", + "---", + "", + "# Coder", + "", + ].join("\n"), + ".paperclip.yaml": [ + "schema: paperclip/v1", + "agents:", + " coder:", + " adapter:", + " type: process", + " config: {}", + " permissionGrants:", + " - permissionKey: agents:suggest-changes", + " - permissionKey: skills:create", + " scope:", + " targetAgentIds:", + " - agent-imported", + "", + ].join("\n"), + }, + }, + include: { + company: false, + agents: true, + projects: false, + issues: false, + }, + target: { + mode: "existing_company", + companyId: "company-1", + }, + collisionStrategy: "rename", + }, "user-1"); + + expect(accessSvc.setPrincipalPermission).toHaveBeenCalledWith( + "company-1", + "agent", + "agent-imported", + "agents:suggest-changes", + true, + "user-1", + null, + ); + expect(accessSvc.setPrincipalPermission).toHaveBeenCalledWith( + "company-1", + "agent", + "agent-imported", + "skills:create", + true, + "user-1", + { targetAgentIds: ["agent-imported"] }, + ); + }); + it("removes import secrets created before a later import failure", async () => { const portability = companyPortabilityService({} as any); agentSvc.list.mockResolvedValue([]); diff --git a/server/src/__tests__/company-skills-routes.test.ts b/server/src/__tests__/company-skills-routes.test.ts index 8f097a2d5df..32c3ef9961f 100644 --- a/server/src/__tests__/company-skills-routes.test.ts +++ b/server/src/__tests__/company-skills-routes.test.ts @@ -8,6 +8,7 @@ const mockAgentService = vi.hoisted(() => ({ const mockAccessService = vi.hoisted(() => ({ canUser: vi.fn(), + decide: vi.fn(), hasPermission: vi.fn(), })); @@ -78,6 +79,30 @@ const mockReflectionCoachMutationGate = vi.hoisted(() => ({ assertConsented: vi.fn(), })); +function allowSkillChangeDecision(reason = "allow_direct_change") { + return { + allowed: true, + action: "skill_config:update", + reason, + explanation: "Allowed.", + grant: { + principalType: "agent", + principalId: "agent-1", + permissionKey: reason === "allow_consented_change" ? "skills:suggest-changes" : "skills:create", + scope: null, + }, + }; +} + +function denySkillChangeDecision(reason = "deny_no_grant", explanation = "Missing permission: skills:create or skills:suggest-changes.") { + return { + allowed: false, + action: "skill_config:update", + reason, + explanation, + }; +} + function registerModuleMocks() { vi.doMock("../routes/authz.js", async () => vi.importActual("../routes/authz.js")); @@ -603,8 +628,9 @@ describe("company skill mutation permissions", () => { }); mockLogActivity.mockResolvedValue(undefined); mockAccessService.canUser.mockResolvedValue(true); + mockAccessService.decide.mockResolvedValue(allowSkillChangeDecision()); mockAccessService.hasPermission.mockResolvedValue(false); - mockReflectionCoachMutationGate.assertConsented.mockResolvedValue(false); + mockReflectionCoachMutationGate.assertConsented.mockResolvedValue(undefined); }); it("allows local board operators to mutate company skills", async () => { @@ -662,7 +688,10 @@ describe("company skill mutation permissions", () => { .send({}) .expect(200); - expect(mockAccessService.canUser).toHaveBeenCalledWith("company-1", "board-user", "skills:create"); + expect(mockAccessService.decide).toHaveBeenCalledWith(expect.objectContaining({ + action: "skill_config:update", + resource: { type: "company", companyId: "company-1" }, + })); expect(mockAccessService.canUser).not.toHaveBeenCalledWith("company-1", "board-user", "agents:create"); expect(mockCompanySkillService.createLocalSkill).toHaveBeenCalled(); expect(mockCompanySkillService.importFromSource).toHaveBeenCalled(); @@ -674,7 +703,7 @@ describe("company skill mutation permissions", () => { }); it("blocks board users without skills:create from mutating company skills", async () => { - mockAccessService.canUser.mockResolvedValue(false); + mockAccessService.decide.mockResolvedValue(denySkillChangeDecision()); const res = await request(await createApp({ type: "board", @@ -687,8 +716,11 @@ describe("company skill mutation permissions", () => { .send({ source: "https://github.com/vercel-labs/agent-browser" }); expect(res.status, JSON.stringify(res.body)).toBe(403); - expect(res.body.error).toBe("Missing permission: skills:create"); - expect(mockAccessService.canUser).toHaveBeenCalledWith("company-1", "board-user", "skills:create"); + expect(res.body.error).toBe("Missing permission: skills:create or skills:suggest-changes."); + expect(mockAccessService.decide).toHaveBeenCalledWith(expect.objectContaining({ + action: "skill_config:update", + resource: { type: "company", companyId: "company-1" }, + })); expect(mockAccessService.canUser).not.toHaveBeenCalledWith("company-1", "board-user", "agents:create"); expect(mockCompanySkillService.importFromSource).not.toHaveBeenCalled(); }); @@ -934,7 +966,8 @@ describe("company skill mutation permissions", () => { }); }); - it("blocks same-company agents with skill creation disabled from mutating company skills", async () => { + it("blocks same-company agents without skill change grants from mutating company skills", async () => { + mockAccessService.decide.mockResolvedValue(denySkillChangeDecision()); mockAgentService.getById.mockResolvedValue({ id: "55555555-5555-4555-8555-555555555555", companyId: "company-1", @@ -951,8 +984,11 @@ describe("company skill mutation permissions", () => { .send({ source: "https://github.com/vercel-labs/agent-browser" }); expect(res.status, JSON.stringify(res.body)).toBe(403); - expect(res.body.error).toBe("Missing permission: skills:create"); - expect(mockAccessService.hasPermission).toHaveBeenCalledWith("company-1", "agent", "55555555-5555-4555-8555-555555555555", "skills:create"); + expect(res.body.error).toBe("Missing permission: skills:create or skills:suggest-changes."); + expect(mockAccessService.decide).toHaveBeenCalledWith(expect.objectContaining({ + action: "skill_config:update", + resource: { type: "company", companyId: "company-1" }, + })); expect(mockAccessService.hasPermission).not.toHaveBeenCalledWith("company-1", "agent", "55555555-5555-4555-8555-555555555555", "agents:create"); expect(mockCompanySkillService.importFromSource).not.toHaveBeenCalled(); }); @@ -1139,11 +1175,12 @@ describe("company skill mutation permissions", () => { }); }); - it("allows agents with canCreateSkills to mutate company skills", async () => { + it("allows agents with direct skills:create grants to mutate company skills", async () => { + mockAccessService.decide.mockResolvedValue(allowSkillChangeDecision("allow_direct_change")); mockAgentService.getById.mockResolvedValue({ id: "55555555-5555-4555-8555-555555555555", companyId: "company-1", - permissions: { canCreateSkills: true }, + permissions: { canCreateSkills: false }, }); const res = await request(await createApp({ @@ -1156,25 +1193,23 @@ describe("company skill mutation permissions", () => { .send({ source: "https://github.com/vercel-labs/agent-browser" }); expect(res.status, JSON.stringify(res.body)).toBe(201); + expect(mockAccessService.decide).toHaveBeenCalledWith(expect.objectContaining({ + action: "skill_config:update", + resource: { type: "company", companyId: "company-1" }, + })); + expect(mockReflectionCoachMutationGate.assertConsented).not.toHaveBeenCalled(); expect(mockCompanySkillService.importFromSource).toHaveBeenCalledWith( "company-1", "https://github.com/vercel-labs/agent-browser", ); }); - it("rejects Reflection Coach skill mutations when the server-side gate is not satisfied", async () => { + it("rejects suggest-tier skill mutations when the consent gate is not satisfied", async () => { const { forbidden } = await import("../errors.js"); - mockAgentService.getById.mockResolvedValue({ - id: "reflection-coach", - companyId: "company-1", - permissions: { canCreateSkills: true }, - metadata: { - paperclipBuiltInAgent: { - key: "reflection-coach", - featureKeys: ["reflection-coach"], - }, - }, - }); + mockAccessService.decide.mockResolvedValue(denySkillChangeDecision( + "deny_missing_consent", + "Permission skills:suggest-changes requires accepted change consent before applying this mutation.", + )); mockReflectionCoachMutationGate.assertConsented.mockRejectedValue(forbidden("gate required", { code: "reflection_coach_mutation_gate_required", })); @@ -1189,6 +1224,7 @@ describe("company skill mutation permissions", () => { .send({ name: "Reflection Draft", slug: "reflection-draft", markdown: "# Draft" }); expect(res.status, JSON.stringify(res.body)).toBe(403); + expect(res.body.error).toBe("Permission skills:suggest-changes requires accepted change consent before applying this mutation."); expect(mockReflectionCoachMutationGate.assertConsented).toHaveBeenCalledWith({ companyId: "company-1", actorAgentId: "reflection-coach", @@ -1198,7 +1234,13 @@ describe("company skill mutation permissions", () => { expect(mockCompanySkillService.createLocalSkill).not.toHaveBeenCalled(); }); - it("allows same-company agents with missing skill creation permission to mutate company skills", async () => { + it("allows suggest-tier skill mutations after accepted change consent", async () => { + mockAccessService.decide + .mockResolvedValueOnce(denySkillChangeDecision( + "deny_missing_consent", + "Permission skills:suggest-changes requires accepted change consent before applying this mutation.", + )) + .mockResolvedValueOnce(allowSkillChangeDecision("allow_consented_change")); mockAgentService.getById.mockResolvedValue({ id: "55555555-5555-4555-8555-555555555555", companyId: "company-1", @@ -1215,57 +1257,29 @@ describe("company skill mutation permissions", () => { .send({ source: "https://github.com/vercel-labs/agent-browser" }); expect(res.status, JSON.stringify(res.body)).toBe(201); + expect(mockReflectionCoachMutationGate.assertConsented).toHaveBeenCalledWith({ + companyId: "company-1", + actorAgentId: "agent-1", + actorRunId: "run-1", + targetKeys: ["skill-import:https://github.com/vercel-labs/agent-browser"], + }); + expect(mockAccessService.decide).toHaveBeenLastCalledWith(expect.objectContaining({ + action: "skill_config:update", + resource: { type: "company", companyId: "company-1" }, + scope: { consentedChange: true }, + })); expect(mockCompanySkillService.importFromSource).toHaveBeenCalledWith( "company-1", "https://github.com/vercel-labs/agent-browser", ); }); - it("allows agents with explicit skills:create grants to mutate company skills", async () => { + it("blocks same-company agents without skill change or suggest grants", async () => { + mockAccessService.decide.mockResolvedValue(denySkillChangeDecision()); mockAgentService.getById.mockResolvedValue({ id: "55555555-5555-4555-8555-555555555555", companyId: "company-1", - permissions: { canCreateSkills: false }, - }); - mockAccessService.hasPermission.mockImplementation(async ( - _companyId: string, - _principalType: string, - _principalId: string, - key: string, - ) => { - return key === "skills:create"; - }); - - const res = await request(await createApp({ - type: "agent", - agentId: "55555555-5555-4555-8555-555555555555", - companyId: "company-1", - runId: "run-1", - })) - .post("/api/companies/company-1/skills/import") - .send({ source: "https://github.com/vercel-labs/agent-browser" }); - - expect(res.status, JSON.stringify(res.body)).toBe(201); - expect(mockAccessService.hasPermission).toHaveBeenCalledWith("company-1", "agent", "55555555-5555-4555-8555-555555555555", "skills:create"); - expect(mockCompanySkillService.importFromSource).toHaveBeenCalledWith( - "company-1", - "https://github.com/vercel-labs/agent-browser", - ); - }); - - it("does not allow explicit agents:create grants to mutate company skills", async () => { - mockAgentService.getById.mockResolvedValue({ - id: "55555555-5555-4555-8555-555555555555", - companyId: "company-1", - permissions: { canCreateSkills: false }, - }); - mockAccessService.hasPermission.mockImplementation(async ( - _companyId: string, - _principalType: string, - _principalId: string, - key: string, - ) => { - return key === "agents:create"; + permissions: {}, }); const res = await request(await createApp({ @@ -1278,8 +1292,44 @@ describe("company skill mutation permissions", () => { .send({ source: "https://github.com/vercel-labs/agent-browser" }); expect(res.status, JSON.stringify(res.body)).toBe(403); - expect(res.body.error).toBe("Missing permission: skills:create"); - expect(mockAccessService.hasPermission).toHaveBeenCalledWith("company-1", "agent", "55555555-5555-4555-8555-555555555555", "skills:create"); + expect(res.body.error).toBe("Missing permission: skills:create or skills:suggest-changes."); + expect(mockAccessService.decide).toHaveBeenCalledWith(expect.objectContaining({ + action: "skill_config:update", + resource: { type: "company", companyId: "company-1" }, + })); + expect(mockCompanySkillService.importFromSource).not.toHaveBeenCalled(); + }); + + it("does not allow explicit agents:create grants to mutate company skills", async () => { + mockAccessService.decide.mockResolvedValue(denySkillChangeDecision()); + mockAgentService.getById.mockResolvedValue({ + id: "agent-1", + companyId: "company-1", + permissions: { canCreateSkills: false }, + }); + mockAccessService.hasPermission.mockImplementation(async ( + _companyId: string, + _principalType: string, + _principalId: string, + key: string, + ) => key === "agents:create"); + + const res = await request(await createApp({ + type: "agent", + agentId: "agent-1", + companyId: "company-1", + runId: "run-1", + })) + .post("/api/companies/company-1/skills/import") + .send({ source: "https://github.com/vercel-labs/agent-browser" }); + + expect(res.status, JSON.stringify(res.body)).toBe(403); + expect(res.body.error).toBe("Missing permission: skills:create or skills:suggest-changes."); + expect(mockAccessService.decide).toHaveBeenCalledWith(expect.objectContaining({ + action: "skill_config:update", + resource: { type: "company", companyId: "company-1" }, + })); + expect(mockAccessService.hasPermission).not.toHaveBeenCalledWith("company-1", "agent", "agent-1", "agents:create"); expect(mockCompanySkillService.importFromSource).not.toHaveBeenCalled(); }); diff --git a/server/src/routes/agents.ts b/server/src/routes/agents.ts index 08770367685..8f020d8218b 100644 --- a/server/src/routes/agents.ts +++ b/server/src/routes/agents.ts @@ -41,6 +41,7 @@ import { agentInstructionsService, accessService, approvalService, + builtInAgentService, companySkillService, budgetService, heartbeatService, @@ -52,7 +53,7 @@ import { syncInstructionsBundleConfigFromFilePath, workspaceOperationService, } from "../services/index.js"; -import { conflict, forbidden, notFound, unprocessable } from "../errors.js"; +import { conflict, forbidden, HttpError, notFound, unprocessable } from "../errors.js"; import { assertBoard, assertCompanyAccess, assertInstanceAdmin, getActorInfo } from "./authz.js"; import { assertNoAgentHostWorkspaceCommandMutation, @@ -99,8 +100,8 @@ import { recoveryService } from "../services/recovery/service.js"; import { resolveCoreTrustPreset } from "../services/trust-preset-resolver.js"; import { readObject } from "../lib/objects.js"; import { listInvalidOrgChainDescendantIds } from "../services/agent-invokability.js"; -import { readBuiltInAgentMarker } from "../services/built-in-agent-metadata.js"; import { + AGENT_PROFILE_CHANGE_CONSENT_FIELDS, agentInstructionsChangeTargetKey, agentProfileChangeTargetKey, changeConsentGateService, @@ -696,30 +697,22 @@ export function agentRoutes( // read-only operation available to any board (human) member of the // company. Responses go through `redactAgentConfiguration` so secrets // are never exposed. Mutations and environment probes still gate on - // agents:create via assertCanCreateAgentsForCompany / assertCanUpdateAgent. + // agents:create or agents:configure via the mutating route helpers. // - // For AGENT actors we keep the previous, stricter gate: an agent must - // either have an explicit `agents:create` grant or the legacy - // `canCreateAgents` permission on its own record. Agents are - // non-human principals — they should not be able to introspect peer - // agents' configurations just by virtue of being in the same company. + // For AGENT actors we keep a stricter gate: an agent must have either + // agents:configure or agents:suggest-changes before it can inspect peer + // agent configuration for a proposed diff. assertCompanyAccess(req, companyId); if (req.actor.type === "agent") { - if (!req.actor.agentId) throw forbidden("Agent authentication required"); - const actorAgent = await svc.getById(req.actor.agentId); - if (!actorAgent || actorAgent.companyId !== companyId) { - throw forbidden("Agent key cannot access another company"); + const decision = await access.decide({ + actor: req.actor, + action: "agent_config:read", + resource: { type: "company", companyId }, + }); + if (!decision.allowed) { + throw forbidden(decision.explanation, authorizationDeniedDetails(decision)); } - const allowedByGrant = await access.hasPermission( - companyId, - "agent", - actorAgent.id, - "agents:create", - ); - if (!allowedByGrant && !canCreateAgents(actorAgent)) { - throw forbidden("Missing permission: can create agents"); - } - return actorAgent; + return req.actor.agentId ? await svc.getById(req.actor.agentId) : null; } return null; } @@ -739,26 +732,21 @@ export function agentRoutes( async function actorCanReadConfigurationsForCompany(req: Request, companyId: string) { // Mirrors assertCanReadConfigurations but returns a boolean instead of - // throwing. Board actors only need company access; agent actors must - // still pass the agents:create gate (explicit grant or canCreateAgents - // on their own record) so peer agents cannot snoop each others' - // configurations. + // throwing. Board actors only need company access; agent actors must pass + // the agent configuration read grant ladder so peer agents cannot snoop + // each others' configurations. try { assertCompanyAccess(req, companyId); } catch { return false; } if (req.actor.type === "board") return true; - if (!req.actor.agentId) return false; - const actorAgent = await svc.getById(req.actor.agentId); - if (!actorAgent || actorAgent.companyId !== companyId) return false; - const allowedByGrant = await access.hasPermission( - companyId, - "agent", - actorAgent.id, - "agents:create", - ); - return allowedByGrant || canCreateAgents(actorAgent); + const decision = await access.decide({ + actor: req.actor, + action: "agent_config:read", + resource: { type: "company", companyId }, + }); + return decision.allowed; } async function buildSkippedWakeupResponse( @@ -853,20 +841,14 @@ export function agentRoutes( } if (actorAgent.id === targetAgent.id) return; - const builtInMarker = readBuiltInAgentMarker(actorAgent.metadata); - if (builtInMarker?.key === "reflection-coach") { - return; - } + const decision = await access.decide({ + actor: req.actor, + action: "agent_config:read", + resource: { type: "agent", companyId: targetAgent.companyId, agentId: targetAgent.id }, + }); + if (decision.allowed) return; - const allowedByGrant = await access.hasPermission( - targetAgent.companyId, - "agent", - actorAgent.id, - "agents:create", - ); - if (allowedByGrant || canCreateAgents(actorAgent)) return; - - throw forbidden("Missing permission: can create agents"); + throw forbidden(decision.explanation, authorizationDeniedDetails(decision)); } function assertKnownAdapterType(type: string | null | undefined): string { @@ -1329,48 +1311,70 @@ export function agentRoutes( } } - async function assertCanManageInstructionsPath(req: Request, targetAgent: { id: string; companyId: string }) { + async function assertCanApplyProtectedAgentChange( + req: Request, + targetAgent: { id: string; companyId: string }, + targetKeys: string[], + ) { assertCompanyAccess(req, targetAgent.companyId); - if (req.actor.type === "board") { - await assertBoardCanManageAgentsForCompany(req, targetAgent.companyId); + const changeScope = { requiresChangeGrant: true }; + const decision = await access.decide({ + actor: req.actor, + action: "agent_config:update", + resource: { type: "agent", companyId: targetAgent.companyId, agentId: targetAgent.id }, + scope: changeScope, + }); + if (decision.allowed) { return; } - if (await isReflectionCoachActor(req, targetAgent.companyId)) { - const allowedByReflectionCoachGate = await changeConsentGateService(db).assertConsented({ - companyId: targetAgent.companyId, - actorAgentId: req.actor.agentId, - actorRunId: req.actor.runId ?? null, - targetKeys: [agentInstructionsChangeTargetKey(targetAgent.id)], + if (decision.reason === "deny_missing_consent" && req.actor.type === "agent" && targetKeys.length > 0) { + try { + await changeConsentGateService(db).assertConsented({ + companyId: targetAgent.companyId, + actorAgentId: req.actor.agentId, + actorRunId: req.actor.runId ?? null, + targetKeys, + }); + } catch (err) { + if (err instanceof HttpError && err.status === 403) { + throw forbidden(decision.explanation, authorizationDeniedDetails(decision)); + } + throw err; + } + + const consentedDecision = await access.decide({ + actor: req.actor, + action: "agent_config:update", + resource: { type: "agent", companyId: targetAgent.companyId, agentId: targetAgent.id }, + scope: { ...changeScope, consentedChange: true }, }); - if (allowedByReflectionCoachGate) return; + if (consentedDecision.allowed) { + return; + } + throw forbidden(consentedDecision.explanation, authorizationDeniedDetails(consentedDecision)); } - throw forbidden( - "Only board-authenticated callers can manage instructions path or bundle configuration", + throw forbidden(decision.explanation, authorizationDeniedDetails(decision)); + } + + async function assertCanManageInstructionsPath(req: Request, targetAgent: { id: string; companyId: string }) { + await assertCanApplyProtectedAgentChange( + req, + targetAgent, + [agentInstructionsChangeTargetKey(targetAgent.id)], ); } - async function isReflectionCoachActor(req: Request, companyId: string) { - if (req.actor.type !== "agent" || !req.actor.agentId) return false; - const actorAgent = await svc.getById(req.actor.agentId); - if (!actorAgent || actorAgent.companyId !== companyId) { - throw forbidden("Agent key cannot access another company"); - } - return readBuiltInAgentMarker(actorAgent.metadata)?.key === "reflection-coach"; - } - - async function assertReflectionCoachAgentDescriptionMutationGate( + async function assertCanApplyAgentProfileChange( req: Request, targetAgent: { id: string; companyId: string }, ) { - if (!(await isReflectionCoachActor(req, targetAgent.companyId))) return; - await changeConsentGateService(db).assertConsented({ - companyId: targetAgent.companyId, - actorAgentId: req.actor.type === "agent" ? req.actor.agentId : null, - actorRunId: req.actor.runId ?? null, - targetKeys: [agentProfileChangeTargetKey(targetAgent.id)], - }); + await assertCanApplyProtectedAgentChange( + req, + targetAgent, + [agentProfileChangeTargetKey(targetAgent.id)], + ); } function assertNoAgentInstructionsConfigMutation( @@ -2533,6 +2537,7 @@ export function agentRoutes( agent.id, req.actor.type === "board" ? (req.actor.userId ?? null) : null, ); + await builtInAgentService(db).ensureCompanyDefaultAgentGrants(companyId); if (agent.budgetMonthlyCents > 0) { await budgets.upsertPolicy( @@ -2861,7 +2866,7 @@ export function agentRoutes( res.status(404).json({ error: "Agent not found" }); return; } - await assertCanUpdateAgent(req, existing); + assertCompanyAccess(req, existing.companyId); if (hasOwn(req.body as object, "permissions")) { res.status(422).json({ error: "Use /api/agents/:id/permissions for permission changes" }); @@ -2973,8 +2978,14 @@ export function agentRoutes( }, ); } - if (touchesAgentProfileChangeConsentFields(patchData)) { - await assertReflectionCoachAgentDescriptionMutationGate(req, existing); + const touchesProfileFields = touchesAgentProfileChangeConsentFields(patchData); + const profileOnlyChange = touchesProfileFields && Object.keys(patchData).every((key) => + (AGENT_PROFILE_CHANGE_CONSENT_FIELDS as readonly string[]).includes(key), + ); + if (profileOnlyChange) { + await assertCanApplyAgentProfileChange(req, existing); + } else { + await assertCanUpdateAgent(req, existing); } const actor = getActorInfo(req); diff --git a/server/src/routes/company-skills.ts b/server/src/routes/company-skills.ts index f1169d077e3..1f8c094090a 100644 --- a/server/src/routes/company-skills.ts +++ b/server/src/routes/company-skills.ts @@ -34,7 +34,6 @@ import { import { forbidden } from "../errors.js"; import { assertAuthenticated, assertCompanyAccess, getActorInfo } from "./authz.js"; import { getTelemetryClient } from "../telemetry.js"; -import { readBuiltInAgentMarker } from "../services/built-in-agent-metadata.js"; import { changeConsentGateService, skillChangeTargetKey, @@ -59,11 +58,6 @@ export function companySkillRoutes(db: Db) { const issues = issueService(db); const heartbeat = heartbeatService(db); - function canCreateSkills(agent: { permissions: Record | null | undefined }) { - if (!agent.permissions || typeof agent.permissions !== "object") return true; - return (agent.permissions as Record).canCreateSkills !== false; - } - function asString(value: unknown): string | null { if (typeof value !== "string") return null; const trimmed = value.trim(); @@ -126,56 +120,42 @@ export function companySkillRoutes(db: Db) { return targetKeys; } - async function isReflectionCoachActor(req: Request, companyId: string) { - if (req.actor.type !== "agent" || !req.actor.agentId) return false; - const actorAgent = await agents.getById(req.actor.agentId); - if (!actorAgent || actorAgent.companyId !== companyId) { - throw forbidden("Agent key cannot access another company"); - } - return readBuiltInAgentMarker(actorAgent.metadata)?.key === "reflection-coach"; - } - - async function assertReflectionCoachSkillMutationGate(req: Request, companyId: string, targetKeys: string[]) { - if (!(await isReflectionCoachActor(req, companyId))) return; - await changeConsentGateService(db).assertConsented({ - companyId, - actorAgentId: req.actor.type === "agent" ? req.actor.agentId : null, - actorRunId: req.actor.runId ?? null, - targetKeys, - }); - } - - async function assertCanMutateCompanySkills(req: Request, companyId: string) { + async function assertCanMutateCompanySkills(req: Request, companyId: string, targetKeys: string[]) { assertCompanyAccess(req, companyId); + const decision = await access.decide({ + actor: req.actor, + action: "skill_config:update", + resource: { type: "company", companyId }, + }); + if (decision.allowed) { + return; + } - if (req.actor.type === "board") { - if (req.actor.source === "local_implicit" || req.actor.isInstanceAdmin) return; - const allowed = await access.canUser(companyId, req.actor.userId, "skills:create"); - if (!allowed) { - throw forbidden("Missing permission: skills:create"); + if (decision.reason === "deny_missing_consent" && req.actor.type === "agent" && targetKeys.length > 0) { + try { + await changeConsentGateService(db).assertConsented({ + companyId, + actorAgentId: req.actor.agentId, + actorRunId: req.actor.runId ?? null, + targetKeys, + }); + } catch { + throw forbidden(decision.explanation, { reason: decision.reason }); } - return; + + const consentedDecision = await access.decide({ + actor: req.actor, + action: "skill_config:update", + resource: { type: "company", companyId }, + scope: { consentedChange: true }, + }); + if (consentedDecision.allowed) { + return; + } + throw forbidden(consentedDecision.explanation, { reason: consentedDecision.reason }); } - if (!req.actor.agentId) { - throw forbidden("Agent authentication required"); - } - - const actorAgent = await agents.getById(req.actor.agentId); - if (!actorAgent || actorAgent.companyId !== companyId) { - throw forbidden("Agent key cannot access another company"); - } - - if (canCreateSkills(actorAgent)) { - return; - } - - const allowedByGrant = await access.hasPermission(companyId, "agent", actorAgent.id, "skills:create"); - if (allowedByGrant) { - return; - } - - throw forbidden("Missing permission: skills:create"); + throw forbidden(decision.explanation, { reason: decision.reason }); } async function assertCanStartSkillTestRuns(req: Request, companyId: string) { @@ -655,8 +635,7 @@ export function companySkillRoutes(db: Db) { async (req, res) => { const companyId = req.params.companyId as string; const skillId = req.params.skillId as string; - await assertCanMutateCompanySkills(req, companyId); - await assertReflectionCoachSkillMutationGate(req, companyId, skillMutationTargets({ skillId })); + await assertCanMutateCompanySkills(req, companyId, skillMutationTargets({ skillId })); const result = await svc.createVersion(companyId, skillId, req.body, skillActor(req)); const actor = getActorInfo(req); await logActivity(db, { @@ -724,8 +703,7 @@ export function companySkillRoutes(db: Db) { async (req, res) => { const companyId = req.params.companyId as string; const skillId = req.params.skillId as string; - await assertCanMutateCompanySkills(req, companyId); - await assertReflectionCoachSkillMutationGate(req, companyId, skillMutationTargets({ + await assertCanMutateCompanySkills(req, companyId, skillMutationTargets({ skillId, slug: req.body.slug, })); @@ -858,8 +836,7 @@ export function companySkillRoutes(db: Db) { validate(companySkillCreateSchema), async (req, res) => { const companyId = req.params.companyId as string; - await assertCanMutateCompanySkills(req, companyId); - await assertReflectionCoachSkillMutationGate(req, companyId, skillMutationTargets({ + await assertCanMutateCompanySkills(req, companyId, skillMutationTargets({ slug: req.body.slug, })); const result = await svc.createLocalSkill(companyId, req.body, skillActor(req)); @@ -890,8 +867,7 @@ export function companySkillRoutes(db: Db) { async (req, res) => { const companyId = req.params.companyId as string; const skillId = req.params.skillId as string; - await assertCanMutateCompanySkills(req, companyId); - await assertReflectionCoachSkillMutationGate(req, companyId, skillMutationTargets({ skillId })); + await assertCanMutateCompanySkills(req, companyId, skillMutationTargets({ skillId })); const result = await svc.updateSkill(companyId, skillId, req.body); const actor = getActorInfo(req); @@ -921,8 +897,7 @@ export function companySkillRoutes(db: Db) { async (req, res) => { const companyId = req.params.companyId as string; const skillId = req.params.skillId as string; - await assertCanMutateCompanySkills(req, companyId); - await assertReflectionCoachSkillMutationGate(req, companyId, skillMutationTargets({ skillId })); + await assertCanMutateCompanySkills(req, companyId, skillMutationTargets({ skillId })); const result = await svc.updateFile( companyId, skillId, @@ -986,9 +961,8 @@ export function companySkillRoutes(db: Db) { validate(companySkillImportSchema), async (req, res) => { const companyId = req.params.companyId as string; - await assertCanMutateCompanySkills(req, companyId); const source = String(req.body.source ?? ""); - await assertReflectionCoachSkillMutationGate(req, companyId, skillMutationTargets({ source })); + await assertCanMutateCompanySkills(req, companyId, skillMutationTargets({ source })); const result = await svc.importFromSource(companyId, source); const actor = getActorInfo(req); @@ -1027,8 +1001,7 @@ export function companySkillRoutes(db: Db) { validate(companySkillInstallCatalogSchema), async (req, res) => { const companyId = req.params.companyId as string; - await assertCanMutateCompanySkills(req, companyId); - await assertReflectionCoachSkillMutationGate(req, companyId, skillMutationTargets({ + await assertCanMutateCompanySkills(req, companyId, skillMutationTargets({ catalogSkillId: req.body.catalogSkillId, slug: req.body.slug, })); @@ -1063,8 +1036,7 @@ export function companySkillRoutes(db: Db) { validate(companySkillProjectScanRequestSchema), async (req, res) => { const companyId = req.params.companyId as string; - await assertCanMutateCompanySkills(req, companyId); - await assertReflectionCoachSkillMutationGate(req, companyId, skillMutationTargets({ scanProjects: true })); + await assertCanMutateCompanySkills(req, companyId, skillMutationTargets({ scanProjects: true })); const result = await svc.scanProjectWorkspaces(companyId, req.body); const actor = getActorInfo(req); @@ -1095,8 +1067,7 @@ export function companySkillRoutes(db: Db) { router.delete("/companies/:companyId/skills/:skillId", async (req, res) => { const companyId = req.params.companyId as string; const skillId = req.params.skillId as string; - await assertCanMutateCompanySkills(req, companyId); - await assertReflectionCoachSkillMutationGate(req, companyId, skillMutationTargets({ skillId })); + await assertCanMutateCompanySkills(req, companyId, skillMutationTargets({ skillId })); const result = await svc.deleteSkill(companyId, skillId); if (!result) { res.status(404).json({ error: "Skill not found" }); @@ -1127,8 +1098,7 @@ export function companySkillRoutes(db: Db) { async (req, res) => { const companyId = req.params.companyId as string; const skillId = req.params.skillId as string; - await assertCanMutateCompanySkills(req, companyId); - await assertReflectionCoachSkillMutationGate(req, companyId, skillMutationTargets({ skillId })); + await assertCanMutateCompanySkills(req, companyId, skillMutationTargets({ skillId })); const result = await svc.auditSkill(companyId, skillId); if (!result) { res.status(404).json({ error: "Skill not found" }); @@ -1164,8 +1134,7 @@ export function companySkillRoutes(db: Db) { async (req, res) => { const companyId = req.params.companyId as string; const skillId = req.params.skillId as string; - await assertCanMutateCompanySkills(req, companyId); - await assertReflectionCoachSkillMutationGate(req, companyId, skillMutationTargets({ skillId })); + await assertCanMutateCompanySkills(req, companyId, skillMutationTargets({ skillId })); const before = await svc.getById(companyId, skillId); const result = await svc.installUpdate(companyId, skillId, req.body); if (!result) { @@ -1205,8 +1174,7 @@ export function companySkillRoutes(db: Db) { async (req, res) => { const companyId = req.params.companyId as string; const skillId = req.params.skillId as string; - await assertCanMutateCompanySkills(req, companyId); - await assertReflectionCoachSkillMutationGate(req, companyId, skillMutationTargets({ skillId })); + await assertCanMutateCompanySkills(req, companyId, skillMutationTargets({ skillId })); const before = await svc.getById(companyId, skillId); const result = await svc.resetSkill(companyId, skillId, req.body); if (!result) { diff --git a/server/src/services/authorization.ts b/server/src/services/authorization.ts index 247b3d5cdc5..d52f855a12f 100644 --- a/server/src/services/authorization.ts +++ b/server/src/services/authorization.ts @@ -56,6 +56,7 @@ export type AuthorizationAction = | PermissionKey | "agent_config:read" | "agent_config:update" + | "skill_config:update" | "agent:read" | "agent:wake" | "company_scope:read" @@ -93,6 +94,8 @@ export type AuthorizationDecision = { | "allow_local_board" | "allow_instance_admin" | "allow_explicit_grant" + | "allow_direct_change" + | "allow_consented_change" | "allow_legacy_agent_creator" | "allow_issue_mention_grant" | "allow_self" @@ -104,6 +107,8 @@ export type AuthorizationDecision = { | "deny_company_boundary" | "deny_missing_membership" | "deny_missing_grant" + | "deny_missing_consent" + | "deny_no_grant" | "deny_policy_restricted" | "deny_low_trust_boundary" | "deny_scope" @@ -125,7 +130,9 @@ function companyIdForResource(resource: AuthorizationResource) { } function permissionForAction(action: AuthorizationAction): PermissionKey | null { - if (action === "agent_config:read" || action === "agent_config:update") return "agents:create"; + if (action === "agent_config:read" || action === "agent_config:update" || action === "skill_config:update") { + return null; + } if ( action === "agent:read" || action === "agent:wake" || @@ -466,6 +473,10 @@ function activeResponsibleUserCanAuthorizeIssueAction( ); } +function scopeBoolean(scope: Record | null | undefined, key: string) { + return scope?.[key] === true; +} + export function authorizationDeniedDetails(decision: AuthorizationDecision) { return { ...(decision.code ? { code: decision.code } : {}), @@ -1305,6 +1316,94 @@ export function authorizationService(db: Db) { return broadDecision; } + async function decideWithAgentConfigReadGrant( + principalType: PrincipalType, + principalId: string, + ): Promise { + const configureDecision = await decidePrincipalGrant({ + companyId, + principalType, + principalId, + action: input.action, + permissionKey: "agents:configure", + scope: input.scope, + }); + if (configureDecision.allowed || configureDecision.reason === "deny_missing_membership") { + return configureDecision; + } + + const suggestDecision = await decidePrincipalGrant({ + companyId, + principalType, + principalId, + action: input.action, + permissionKey: "agents:suggest-changes", + scope: input.scope, + }); + if (suggestDecision.allowed || configureDecision.reason === "deny_missing_grant") { + return suggestDecision; + } + return configureDecision; + } + + async function decideWithProtectedChangeGrants( + principalType: PrincipalType, + principalId: string, + keys: { direct: PermissionKey; suggest: PermissionKey }, + ): Promise { + const directDecision = await decidePrincipalGrant({ + companyId, + principalType, + principalId, + action: input.action, + permissionKey: keys.direct, + scope: input.scope, + }); + if (directDecision.allowed) { + return allow({ + action: input.action, + reason: "allow_direct_change", + explanation: `Allowed by direct change permission ${keys.direct}.`, + grant: directDecision.grant, + }); + } + if (directDecision.reason === "deny_missing_membership") return directDecision; + + const suggestDecision = await decidePrincipalGrant({ + companyId, + principalType, + principalId, + action: input.action, + permissionKey: keys.suggest, + scope: input.scope, + }); + if (suggestDecision.allowed) { + if (scopeBoolean(input.scope, "consentedChange")) { + return allow({ + action: input.action, + reason: "allow_consented_change", + explanation: `Allowed by suggest permission ${keys.suggest} after accepted change consent.`, + grant: suggestDecision.grant, + }); + } + return deny({ + action: input.action, + reason: "deny_missing_consent", + explanation: `Permission ${keys.suggest} requires accepted change consent before applying this mutation.`, + grant: suggestDecision.grant, + }); + } + if (suggestDecision.reason === "deny_missing_membership") return suggestDecision; + if (directDecision.reason === "deny_scope") return directDecision; + if (suggestDecision.reason === "deny_scope") return suggestDecision; + + return deny({ + action: input.action, + reason: "deny_no_grant", + explanation: `Missing permission: ${keys.direct} or ${keys.suggest}.`, + }); + } + async function denyForAssignmentPolicyIfNeeded( policyEffect: AssignmentPolicyEffect, ): Promise { @@ -1465,6 +1564,21 @@ export function authorizationService(db: Db) { if (policyEffect.kind === "restricted") return denyRestrictedAssignmentPolicy(policyEffect); return grantDecision; } + if (input.action === "agent_config:read") { + return decideWithAgentConfigReadGrant("user", input.actor.userId); + } + if (input.action === "agent_config:update") { + return decideWithProtectedChangeGrants("user", input.actor.userId, { + direct: "agents:configure", + suggest: "agents:suggest-changes", + }); + } + if (input.action === "skill_config:update") { + return decideWithProtectedChangeGrants("user", input.actor.userId, { + direct: "skills:create", + suggest: "skills:suggest-changes", + }); + } return decidePrincipalGrant({ companyId, principalType: "user", @@ -1641,7 +1755,8 @@ export function authorizationService(db: Db) { if ( input.action === "agent_config:update" && input.resource.type === "agent" && - input.resource.agentId === actorAgentId + input.resource.agentId === actorAgentId && + !scopeBoolean(input.scope, "requiresChangeGrant") ) { return allow({ action: input.action, @@ -1650,6 +1765,31 @@ export function authorizationService(db: Db) { }); } + if (input.action === "agent_config:read") { + if (input.resource.type === "agent" && input.resource.agentId === actorAgentId) { + return allow({ + action: input.action, + reason: "allow_self", + explanation: "Allowed because the actor is reading its own agent configuration.", + }); + } + return decideWithAgentConfigReadGrant("agent", actorAgentId); + } + + if (input.action === "agent_config:update") { + return decideWithProtectedChangeGrants("agent", actorAgentId, { + direct: "agents:configure", + suggest: "agents:suggest-changes", + }); + } + + if (input.action === "skill_config:update") { + return decideWithProtectedChangeGrants("agent", actorAgentId, { + direct: "skills:create", + suggest: "skills:suggest-changes", + }); + } + if (permissionKey) { const grantDecision = await decidePrincipalGrant({ companyId, @@ -1664,8 +1804,6 @@ export function authorizationService(db: Db) { if ( (input.action === "agents:create" || - input.action === "agent_config:read" || - input.action === "agent_config:update" || input.action === "tasks:manage_active_checkouts") && canCreateAgentsLegacy(actorAgent) ) { diff --git a/server/src/services/built-in-agents.ts b/server/src/services/built-in-agents.ts index e4fcae5b3d7..c0502c526f9 100644 --- a/server/src/services/built-in-agents.ts +++ b/server/src/services/built-in-agents.ts @@ -7,7 +7,7 @@ import { and, desc, eq, ne } from "drizzle-orm"; import type { Db } from "@paperclipai/db"; import { agents, builtInManagedResources, companies, issueThreadInteractions, issues, routines, routineTriggers } from "@paperclipai/db"; import { syncRoutineVariablesWithTemplate } from "@paperclipai/shared"; -import type { Agent, Approval, CompanySkill, Routine, RoutineTrigger, RoutineVariable } from "@paperclipai/shared"; +import type { Agent, Approval, CompanySkill, PermissionKey, Routine, RoutineTrigger, RoutineVariable } from "@paperclipai/shared"; import { conflict, HttpError, notFound, unprocessable } from "../errors.js"; import { logActivity } from "./activity-log.js"; import { agentInstructionsService } from "./agent-instructions.js"; @@ -19,6 +19,7 @@ import { } from "./built-in-agent-metadata.js"; import { companySkillService } from "./company-skills.js"; import { routineService } from "./routines.js"; +import { accessService } from "./access.js"; export type BuiltInAgentStatus = "not_provisioned" | "pending_approval" | "needs_setup" | "ready" | "paused"; @@ -253,6 +254,11 @@ const DEFINITIONS = validateBuiltInAgentDefinitions([ const DEFINITIONS_BY_KEY = new Map(DEFINITIONS.map((definition) => [definition.key, definition])); +const ROOT_AGENT_DEFAULT_CHANGE_GRANTS: PermissionKey[] = ["agents:configure", "skills:create"]; +const BUILT_IN_AGENT_DEFAULT_GRANTS: Record = { + "reflection-coach": ["agents:suggest-changes", "skills:suggest-changes"], +}; + function isPlainRecord(value: unknown): value is Record { return typeof value === "object" && value !== null && !Array.isArray(value); } @@ -546,6 +552,7 @@ function rowIsBuiltInAgent(row: typeof agents.$inferSelect, key: string) { export function builtInAgentService(db: Db) { const agentSvc = agentService(db); + const accessSvc = accessService(db); const approvalSvc = approvalService(db); const instructionsSvc = agentInstructionsService(); const skillSvc = companySkillService(db); @@ -560,6 +567,51 @@ export function builtInAgentService(db: Db) { return nonBuiltInRoots.length === 1 ? nonBuiltInRoots[0]!.id : null; } + async function ensureAgentDefaultGrants(companyId: string, agentId: string, grantKeys: PermissionKey[]) { + if (grantKeys.length === 0) return 0; + await accessSvc.ensureMembership(companyId, "agent", agentId, "member", "active"); + let ensured = 0; + for (const permissionKey of grantKeys) { + await accessSvc.setPrincipalPermission(companyId, "agent", agentId, permissionKey, true, null); + ensured += 1; + } + return ensured; + } + + async function ensureBuiltInAgentDefaultGrants(agent: Agent, definition: BuiltInAgentDefinition) { + if (agent.status === "pending_approval" || agent.status === "terminated") return 0; + return ensureAgentDefaultGrants( + agent.companyId, + agent.id, + BUILT_IN_AGENT_DEFAULT_GRANTS[definition.key] ?? [], + ); + } + + async function ensureRootAgentDefaultChangeGrants(companyId: string) { + const rows = await db + .select() + .from(agents) + .where(and(eq(agents.companyId, companyId), ne(agents.status, "terminated"))); + const rootCeoRows = rows.filter((agent) => + !readBuiltInAgentMarker(agent.metadata) && + !agent.reportsTo && + agent.role.trim().toLowerCase() === "ceo" && + agent.status !== "pending_approval" + ); + if (rootCeoRows.length !== 1) return 0; + return ensureAgentDefaultGrants(companyId, rootCeoRows[0]!.id, ROOT_AGENT_DEFAULT_CHANGE_GRANTS); + } + + async function ensureCompanyDefaultAgentGrants(companyId: string) { + let ensured = await ensureRootAgentDefaultChangeGrants(companyId); + for (const definition of DEFINITIONS) { + const agent = await findSingleAgent(companyId, definition); + if (!agent) continue; + ensured += await ensureBuiltInAgentDefaultGrants(agent as Agent, definition); + } + return ensured; + } + async function defaultProvisionInput(companyId: string, definition: BuiltInAgentDefinition, input: BuiltInAgentProvisionInput) { if (input.adapterType || input.adapterConfig) return input; if (!definition.bundle) return input; @@ -1279,6 +1331,7 @@ export function builtInAgentService(db: Db) { recordRevision: { source: "built-in-agent:ensure" }, }); if (!updated) throw notFound("Built-in agent not found"); + await ensureBuiltInAgentDefaultGrants(updated as Agent, definition); const resources = await reconcileBundleResources(updated as Agent, definition, "reconcile"); return state(definition, await agentSvc.getById(existing.id) as Agent, resources); } @@ -1313,6 +1366,7 @@ export function builtInAgentService(db: Db) { }, }); + await ensureBuiltInAgentDefaultGrants(created, definition); const resources = await reconcileBundleResources(created, definition, "reconcile"); return state(definition, await agentSvc.getById(created.id) as Agent, resources); } @@ -1420,6 +1474,7 @@ export function builtInAgentService(db: Db) { recordRevision: { source: "built-in-agent:reconcile-defaults" }, }); if (!updated) throw notFound("Built-in agent not found"); + await ensureBuiltInAgentDefaultGrants(updated as Agent, definition); return state(definition, updated as Agent); } @@ -1508,7 +1563,8 @@ export function builtInAgentService(db: Db) { } autoEnsured += 1; } - return { autoEnsured, pendingApprovals }; + const defaultGrantsEnsured = await ensureCompanyDefaultAgentGrants(companyId); + return { autoEnsured, pendingApprovals, defaultGrantsEnsured }; } return { @@ -1533,6 +1589,7 @@ export function builtInAgentService(db: Db) { runRoutine, requireBuiltInAgent, autoProvisionBundledAgents, + ensureCompanyDefaultAgentGrants, reconcileDefinitionDefaults, }; } @@ -1544,10 +1601,12 @@ export async function reconcileBuiltInAgentsOnStartup(db: Db) { .from(companies); let autoEnsured = 0; let pendingApprovals = 0; + let defaultGrantsEnsured = 0; for (const company of companyRows) { const result = await svc.autoProvisionBundledAgents(company.id); autoEnsured += result.autoEnsured; pendingApprovals += result.pendingApprovals; + defaultGrantsEnsured += result.defaultGrantsEnsured; } const rows = await db .select({ @@ -1581,5 +1640,5 @@ export async function reconcileBuiltInAgentsOnStartup(db: Db) { reconciled += 1; } - return { scanned, reconciled, unknown, duplicates, autoEnsured, pendingApprovals }; + return { scanned, reconciled, unknown, duplicates, autoEnsured, pendingApprovals, defaultGrantsEnsured }; } diff --git a/server/src/services/company-member-roles.ts b/server/src/services/company-member-roles.ts index d3443098d92..0c3057c07db 100644 --- a/server/src/services/company-member-roles.ts +++ b/server/src/services/company-member-roles.ts @@ -28,6 +28,7 @@ export function grantsForHumanRole( case "owner": return [ { permissionKey: "agents:create", scope: null }, + { permissionKey: "agents:configure", scope: null }, { permissionKey: "skills:create", scope: null }, { permissionKey: "environments:manage", scope: null }, { permissionKey: "users:invite", scope: null }, @@ -38,6 +39,7 @@ export function grantsForHumanRole( case "admin": return [ { permissionKey: "agents:create", scope: null }, + { permissionKey: "agents:configure", scope: null }, { permissionKey: "skills:create", scope: null }, { permissionKey: "environments:manage", scope: null }, { permissionKey: "users:invite", scope: null }, diff --git a/server/src/services/company-portability.ts b/server/src/services/company-portability.ts index 41e0498e02f..01a32c287a7 100644 --- a/server/src/services/company-portability.ts +++ b/server/src/services/company-portability.ts @@ -3,8 +3,8 @@ import { promises as fs } from "node:fs"; import { execFile } from "node:child_process"; import path from "node:path"; import { promisify } from "node:util"; -import { eq } from "drizzle-orm"; -import { builtInManagedResources, type Db } from "@paperclipai/db"; +import { and, eq, inArray } from "drizzle-orm"; +import { builtInManagedResources, principalPermissionGrants, type Db } from "@paperclipai/db"; import type { CompanyPortabilityAgentManifestEntry, CompanyPortabilityCollisionStrategy, @@ -30,6 +30,7 @@ import type { CompanyPortabilitySkillManifestEntry, CompanySkill, AgentEnvConfig, + PermissionKey, RoutineVariable, } from "@paperclipai/shared"; import { @@ -49,6 +50,7 @@ import { issueCommentMetadataSchema, issueCommentPresentationSchema, normalizeAgentUrlKey, + PERMISSION_KEYS, } from "@paperclipai/shared"; import { readPaperclipSkillSyncPreference, @@ -720,6 +722,23 @@ function asBoolean(value: unknown): boolean | null { return typeof value === "boolean" ? value : null; } +type PortableAgentPermissionGrant = CompanyPortabilityAgentManifestEntry["permissionGrants"][number]; + +const VALID_PERMISSION_KEYS = new Set(PERMISSION_KEYS); + +function normalizePortablePermissionGrants(value: unknown): PortableAgentPermissionGrant[] { + if (!Array.isArray(value)) return []; + return value.flatMap((entry): PortableAgentPermissionGrant[] => { + if (!isPlainRecord(entry)) return []; + const permissionKey = asString(entry.permissionKey); + if (!permissionKey || !VALID_PERMISSION_KEYS.has(permissionKey as PermissionKey)) return []; + return [{ + permissionKey: permissionKey as PermissionKey, + scope: isPlainRecord(entry.scope) ? entry.scope : null, + }]; + }); +} + function asInteger(value: unknown): number | null { return typeof value === "number" && Number.isInteger(value) ? value : null; } @@ -1926,6 +1945,7 @@ const YAML_KEY_PRIORITY = [ "adapter", "runtime", "permissions", + "permissionGrants", "budgetMonthlyCents", "metadata", ] as const; @@ -2692,6 +2712,7 @@ function buildManifestFromPackageFiles( const extensionAdapter = isPlainRecord(extension.adapter) ? extension.adapter : null; const extensionRuntime = isPlainRecord(extension.runtime) ? extension.runtime : null; const extensionPermissions = isPlainRecord(extension.permissions) ? extension.permissions : null; + const extensionPermissionGrants = normalizePortablePermissionGrants(extension.permissionGrants); const extensionMetadata = isPlainRecord(extension.metadata) ? extension.metadata : null; const adapterConfig = isPlainRecord(extensionAdapter?.config) ? extensionAdapter.config @@ -2715,6 +2736,7 @@ function buildManifestFromPackageFiles( adapterConfig, runtimeConfig, permissions: extensionPermissions ?? {}, + permissionGrants: extensionPermissionGrants, budgetMonthlyCents: typeof extension.budgetMonthlyCents === "number" && Number.isFinite(extension.budgetMonthlyCents) ? Math.max(0, Math.floor(extension.budgetMonthlyCents)) @@ -3004,6 +3026,27 @@ export function companyPortabilityService(db: Db, storage?: StorageService) { const strictSecretsMode = process.env.PAPERCLIP_SECRETS_STRICT_MODE === "true"; const defaultSecretProvider = getConfiguredSecretProvider(); + async function applyImportedAgentPermissionGrants( + companyId: string, + agentId: string, + permissionGrants: PortableAgentPermissionGrant[], + grantedByUserId: string | null, + ) { + if (permissionGrants.length === 0) return; + await access.ensureMembership(companyId, "agent", agentId, "member", "active"); + for (const grant of permissionGrants) { + await access.setPrincipalPermission( + companyId, + "agent", + agentId, + grant.permissionKey, + true, + grantedByUserId, + grant.scope ?? null, + ); + } + } + function assertKnownImportAdapterType(type: string | null | undefined): string { const adapterType = typeof type === "string" ? type.trim() : ""; if (!adapterType) { @@ -3346,6 +3389,33 @@ export function companyPortabilityService(db: Db, storage?: StorageService) { const slug = uniqueSlug(baseSlug, usedSlugs); idToSlug.set(agent.id, slug); } + const agentPermissionGrantRows = agentRows.length > 0 && typeof (db as { select?: unknown }).select === "function" + ? await db + .select({ + principalId: principalPermissionGrants.principalId, + permissionKey: principalPermissionGrants.permissionKey, + scope: principalPermissionGrants.scope, + }) + .from(principalPermissionGrants) + .where(and( + eq(principalPermissionGrants.companyId, companyId), + eq(principalPermissionGrants.principalType, "agent"), + inArray(principalPermissionGrants.principalId, agentRows.map((agent) => agent.id)), + )) + : []; + const permissionGrantsByAgentId = new Map(); + for (const row of agentPermissionGrantRows) { + if (!VALID_PERMISSION_KEYS.has(row.permissionKey as PermissionKey)) continue; + const grants = permissionGrantsByAgentId.get(row.principalId) ?? []; + grants.push({ + permissionKey: row.permissionKey as PermissionKey, + scope: isPlainRecord(row.scope) ? row.scope : null, + }); + permissionGrantsByAgentId.set(row.principalId, grants); + } + for (const grants of permissionGrantsByAgentId.values()) { + grants.sort((left, right) => left.permissionKey.localeCompare(right.permissionKey)); + } const projectsSvc = projectService(db); const issuesSvc = issueService(db); @@ -3609,6 +3679,7 @@ export function companyPortabilityService(db: Db, storage?: StorageService) { }, ) as Record; const portablePermissions = pruneDefaultLikeValue(agent.permissions ?? {}, { dropFalseBooleans: true }) as Record; + const portablePermissionGrants = permissionGrantsByAgentId.get(agent.id) ?? []; const agentEnvInputs = dedupeEnvInputs( envInputs .slice(envInputsStart) @@ -3651,6 +3722,7 @@ export function companyPortabilityService(db: Db, storage?: StorageService) { }, runtime: portableRuntimeConfig, permissions: portablePermissions, + permissionGrants: portablePermissionGrants.length > 0 ? portablePermissionGrants : undefined, budgetMonthlyCents: (agent.budgetMonthlyCents ?? 0) > 0 ? agent.budgetMonthlyCents : undefined, metadata: (agent.metadata as Record | null) ?? null, }); @@ -4651,6 +4723,12 @@ export function companyPortabilityService(db: Db, storage?: StorageService) { } catch (err) { warnings.push(`Failed to materialize instructions bundle for ${manifestAgent.slug}: ${err instanceof Error ? err.message : String(err)}`); } + await applyImportedAgentPermissionGrants( + targetCompany.id, + updated.id, + manifestAgent.permissionGrants ?? [], + actorUserId ?? null, + ); agentStatusById.set(updated.id, updated.status ?? agentStatusById.get(updated.id) ?? null); await secrets.syncEnvBindingsForTarget?.( targetCompany.id, @@ -4692,6 +4770,12 @@ export function companyPortabilityService(db: Db, storage?: StorageService) { } catch (err) { warnings.push(`Failed to materialize instructions bundle for ${manifestAgent.slug}: ${err instanceof Error ? err.message : String(err)}`); } + await applyImportedAgentPermissionGrants( + targetCompany.id, + created.id, + manifestAgent.permissionGrants ?? [], + actorUserId ?? null, + ); agentStatusById.set(created.id, created.status ?? createdStatus); await secrets.syncEnvBindingsForTarget?.( targetCompany.id,