diff --git a/doc/SPEC-implementation.md b/doc/SPEC-implementation.md index 810faf75d9d..c52e309272f 100644 --- a/doc/SPEC-implementation.md +++ b/doc/SPEC-implementation.md @@ -333,6 +333,14 @@ replacement semantics, secret preservation, and revision preconditions. It uses the existing configuration and resource APIs, retaining their authorization and approval gates. Agent lifecycle commands remain separate actions. +`agents:read` permits redacted configuration, configured skills, instruction +content and history reads without write authority. `agents:configure` remains +read/write; `agents:suggest-changes` retains reads and consented changes. Own +configuration/history reads require no peer-read grant. Target scopes, company +boundaries and responsible-user authority still apply. Standard agents receive +read access by default, with a one-time read-only rollout to eligible existing +agents. See [agent permission defaults](agent-permission-defaults.md). + ## 7.3 `agent_api_keys` - `id` uuid pk diff --git a/doc/SPEC.md b/doc/SPEC.md index 98d268a26bf..28892d5e76a 100644 --- a/doc/SPEC.md +++ b/doc/SPEC.md @@ -160,6 +160,12 @@ Paperclip doesn't prescribe how an agent defines its identity or behavior. It pr Each agent has an **adapter type** and an **adapter-specific configuration blob**. The adapter defines what config fields exist. +Configuration reads and changes have separate authority. `agents:read` permits +redacted configuration and instruction reads; `agents:configure` includes reads +and direct changes. Standard agents have read access by default. Company scope, +explicit grant scopes, responsible-user authority and trust restrictions apply. +See [agent permission defaults](agent-permission-defaults.md). + #### Paperclip Protocol (What Paperclip Knows) At the protocol level, Paperclip tracks: diff --git a/doc/agent-permission-defaults.md b/doc/agent-permission-defaults.md index 8dcd1ac4c45..edf9fa18f4e 100644 --- a/doc/agent-permission-defaults.md +++ b/doc/agent-permission-defaults.md @@ -6,7 +6,8 @@ This document describes the server defaults for an ordinary, standard trust agen | Capability | Source and limit | | --- | --- | -| Agent configuration and suggestions | New standard agents receive direct, company-scoped `agents:configure` and `agents:suggest-changes` grants. Protected-change and responsible-user checks still apply. | +| Agent configuration reads | Standard agents receive company-scoped `agents:read`. This permits redacted configuration, skill settings, instruction content, and configuration/instruction history reads. It grants no write, rollback, or environment-test authority. | +| Agent configuration changes and suggestions | New standard agents also receive `agents:configure` and `agents:suggest-changes`. `agents:configure` permits reads and direct changes; `agents:suggest-changes` retains reads and consented changes. Protected-change and responsible-user checks still apply. | | Skill changes | New standard agents receive direct `skills:create` and `skills:suggest-changes` grants. | | Tool access and management | New standard agents receive `tools:manage_connections`, `tools:manage_profiles`, `tools:view_audit`, `tools:use`, and `tools:manage_runtime` grants. Tool, connection, profile, and runtime policy checks still apply. | | Audit and inbox | New standard agents receive `audit:view_agent_actions` and `inbox:manage` grants. The default inbox grant is limited to the responsible-user path and still follows that user's inbox policy. Cross-user management requires a separately configured grant or the target user's saved consent policy. | @@ -26,7 +27,11 @@ Agent-authenticated callers cannot set process adapter configuration or switch a ## Exceptions and rollout -The new direct grants are withheld from low trust agents and managed built-in agents. Low trust run or project policies can also deny privileged actions even if an agent has a grant. There is no permission backfill or upgrade migration for existing agents. A standard pending new hire receives the new default set when approved and activated. Invitation approval retains the set when it replaces a new agent's grants and preserves any explicitly scoped default grant. Existing scoped grants stay unchanged. A grant removed by an operator stays removed; ordinary updates and startup do not reapply it. +The new direct grants are withheld from low trust agents and managed built-in agents. Low trust run or project policies can also deny privileged actions even if an agent has a grant. A standard pending new hire receives the new default set when approved and activated. Invitation approval retains the set when it replaces a new agent's grants and preserves any explicitly scoped default grant. Existing scoped grants stay unchanged. A grant removed by an operator stays removed; ordinary updates and startup do not reapply it. + +Migration `0423_agent_config_read_defaults` adds only `agents:read` to existing ready, paused, pausing or resuming standard agents with an active same-company membership. It skips setup holds or setup pause reasons, managed built-ins, nonstandard trust policies, pending or terminated agents, and agents with scoped configuration/suggestion grants. An existing read grant keeps its scope. Each addition has a system audit entry. Existing write grants are unchanged. The migration intentionally adds read access even when an older write grant is absent; it does not infer that a missing write grant denies the new independent read capability. Removing the read grant after this one-time migration persists across restarts and ordinary updates. `agents:configure` and `agents:suggest-changes` still independently imply reads within their scopes. + +Agents can read their own redacted configuration, skills and configuration history without a peer-read grant. Target-scoped reads identify the target agent, so scoped read grants also work on individual configuration and instruction routes. Company-wide configuration listings require company-wide read authority. Basic same-company agent profiles remain readable through their existing visibility rules. The standard agent configuration default supports the agent setup in the linked runner task. Enabling a warm runtime can still depend on the target agent's adapter and runtime configuration, provider availability, and any responsible-user or approval checks. diff --git a/maps/agent-access-map/agents.md b/maps/agent-access-map/agents.md index 9ec77daa345..75ef16b9af0 100644 --- a/maps/agent-access-map/agents.md +++ b/maps/agent-access-map/agents.md @@ -44,6 +44,16 @@ bypass hiring authorization. Source: [runner resolver](../../server/src/services Policy: Agent creation/configuration uses action grants, resource policies and change consent; some controls require a board actor. +`agents:read` permits redacted configuration, configured skills, instruction content, +and configuration/instruction history reads. It does not permit changes, restore, +or environment probes. `agents:configure` still includes read/write authority; +`agents:suggest-changes` still includes reads and consented changes. Individual +reads use the target agent and honor target-scoped grants. Agents can read their +own redacted configuration and history without peer-read authority. Company-wide +configuration listings require company-wide read authority. Standard-agent read +defaults and the one-time existing-agent rollout are defined in +[agent permission defaults](../../doc/agent-permission-defaults.md). + The early wizard record (`setupId` on create) and completion of that record (`setupAgentId` on hire) require a board actor and company-scoped creation permission. Its creator can finish their own inactive setup with that permission; diff --git a/packages/db/src/agent-config-read-grants-migration.test.ts b/packages/db/src/agent-config-read-grants-migration.test.ts new file mode 100644 index 00000000000..7030eedee3b --- /dev/null +++ b/packages/db/src/agent-config-read-grants-migration.test.ts @@ -0,0 +1,94 @@ +import { createHash, randomUUID } from "node:crypto"; +import { readFile } from "node:fs/promises"; +import postgres from "postgres"; +import { describe, expect, it } from "vitest"; +import { applyPendingMigrations } from "./client.js"; +import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "./test-embedded-postgres.js"; + +const support = await getEmbeddedPostgresTestSupport(); +const migrationFile = "0423_agent_config_read_defaults.sql"; + +(support.supported ? describe : describe.skip)("agent configuration read default migration", () => { + it("adds only read access to existing standard agents and preserves restrictions and later revocation", async () => { + const database = await startEmbeddedPostgresTestDatabase("agent-config-read-defaults-"); + const sql = postgres(database.connectionString, { max: 1, onnotice: () => {} }); + try { + const [company] = await sql`INSERT INTO companies (name, issue_prefix) VALUES ('Read defaults', 'RD1') RETURNING id`; + const [otherCompany] = await sql`INSERT INTO companies (name, issue_prefix) VALUES ('Other company', 'RD2') RETURNING id`; + const candidates = [ + { name: "Legacy" }, + { name: "Paused", status: "paused", lifecycleState: "paused", lifecycleHolds: ["manual"], pauseReason: "manual" }, + { name: "Pausing", status: "paused", lifecycleState: "pausing", lifecycleHolds: ["manual"], pauseReason: "manual" }, + { name: "Resuming", status: "paused", lifecycleState: "resuming" }, + { name: "Configured" }, + { name: "Scoped configure" }, + { name: "Scoped suggest" }, + { name: "Scoped read" }, + { name: "Low trust", permissions: { trustPreset: "low_trust_review" } }, + { name: "Nested low trust", permissions: { authorizationPolicy: { trustPreset: "low_trust_review" } } }, + { name: "Review preset", permissions: { reviewPreset: { id: "low_trust_review" } } }, + { name: "Nested review preset", permissions: { authorizationPolicy: { reviewPreset: { id: "low_trust_review" } } } }, + { name: "Boundary", permissions: { authorizationPolicy: { trustBoundary: { mode: "low_trust_review" } } } }, + { name: "Unknown preset", permissions: { trustPreset: "unknown" } }, + { name: "Built in", metadata: { paperclipBuiltInAgent: { key: "summarizer", featureKeys: [] } } }, + { name: "Pending", status: "pending_approval" }, + { name: "Terminated", status: "terminated" }, + { name: "Setup", status: "paused", lifecycleState: "paused", lifecycleHolds: ["setup"], pauseReason: "setup" }, + { name: "Legacy setup", status: "paused", lifecycleState: "paused", pauseReason: "setup" }, + { name: "Inactive membership", membershipStatus: "inactive" }, + { name: "No membership", membershipStatus: "absent" }, + { name: "Wrong company membership", membershipStatus: "other_company" }, + ]; + const ids = new Map(); + for (const candidate of candidates) { + const id = randomUUID(); + ids.set(candidate.name, id); + await sql`INSERT INTO agents (id, company_id, name, status, lifecycle_state, lifecycle_holds, pause_reason, permissions, metadata) + VALUES (${id}, ${company!.id}, ${candidate.name}, ${candidate.status ?? "idle"}, + ${candidate.lifecycleState ?? "ready"}, ${sql.json(candidate.lifecycleHolds ?? [])}, ${candidate.pauseReason ?? null}, + ${sql.json(candidate.permissions ?? {})}, ${sql.json(candidate.metadata ?? {})})`; + if (candidate.membershipStatus === "absent") continue; + await sql`INSERT INTO company_memberships (company_id, principal_type, principal_id, status, membership_role) + VALUES (${candidate.membershipStatus === "other_company" ? otherCompany!.id : company!.id}, 'agent', ${id}, + ${candidate.membershipStatus === "inactive" ? "inactive" : "active"}, 'member')`; + } + const scope = { agentIds: [ids.get("Legacy")!] }; + for (const [name, permissionKey, grantScope] of [ + ["Configured", "agents:configure", null], + ["Scoped configure", "agents:configure", scope], + ["Scoped suggest", "agents:suggest-changes", scope], + ["Scoped read", "agents:read", scope], + ] as const) { + await sql`INSERT INTO principal_permission_grants (company_id, principal_type, principal_id, permission_key, scope) + VALUES (${company!.id}, 'agent', ${ids.get(name)!}, ${permissionKey}, ${grantScope ? sql.json(grantScope) : null})`; + } + const before = await sql`SELECT principal_id, permission_key, scope FROM principal_permission_grants ORDER BY principal_id, permission_key`; + const content = await readFile(new URL(`./migrations/${migrationFile}`, import.meta.url), "utf8"); + const hash = createHash("sha256").update(content).digest("hex"); + await sql`DELETE FROM drizzle.__drizzle_migrations WHERE hash = ${hash}`; + await applyPendingMigrations(database.connectionString); + + const after = await sql`SELECT principal_id, permission_key, scope FROM principal_permission_grants ORDER BY principal_id, permission_key`; + const added = after.filter(row => !before.some(previous => previous.principal_id === row.principal_id && previous.permission_key === row.permission_key)); + expect(added).toEqual(expect.arrayContaining(["Legacy", "Paused", "Pausing", "Resuming", "Configured"].map(name => ({ + principal_id: ids.get(name), permission_key: "agents:read", scope: null, + })))); + expect(added).toHaveLength(5); + expect(after.filter(row => row.permission_key !== "agents:read")).toEqual(before.filter(row => row.permission_key !== "agents:read")); + expect(after.find(row => row.principal_id === ids.get("Scoped read"))).toMatchObject({ scope }); + const audit = await sql`SELECT entity_id, details FROM activity_log WHERE actor_id = 'migration:agent-config-read-defaults'`; + expect(audit).toHaveLength(5); + expect(audit.every(row => row.details.added.length === 1 && row.details.added[0] === "agents:read")).toBe(true); + await sql.unsafe(content); + expect(await sql`SELECT id FROM activity_log WHERE actor_id = 'migration:agent-config-read-defaults'`).toHaveLength(5); + + await sql`DELETE FROM principal_permission_grants WHERE principal_id = ${ids.get("Legacy")!} AND permission_key = 'agents:read'`; + await applyPendingMigrations(database.connectionString); + expect(await sql`SELECT id FROM principal_permission_grants WHERE principal_id = ${ids.get("Legacy")!}`).toHaveLength(0); + expect(await sql`SELECT id FROM drizzle.__drizzle_migrations WHERE hash = ${hash}`).toHaveLength(1); + } finally { + await sql.end(); + await database.cleanup(); + } + }, 120_000); +}); diff --git a/packages/db/src/migrations/0423_agent_config_read_defaults.sql b/packages/db/src/migrations/0423_agent_config_read_defaults.sql new file mode 100644 index 00000000000..0e223f06389 --- /dev/null +++ b/packages/db/src/migrations/0423_agent_config_read_defaults.sql @@ -0,0 +1,37 @@ +WITH added AS ( + INSERT INTO principal_permission_grants ( + company_id, principal_type, principal_id, permission_key, scope + ) + SELECT agent.company_id, 'agent', agent.id::text, 'agents:read', NULL + FROM agents AS agent + JOIN company_memberships AS membership + ON membership.company_id = agent.company_id + AND membership.principal_type = 'agent' + AND membership.principal_id = agent.id::text + AND membership.status = 'active' + WHERE agent.status NOT IN ('pending_approval', 'terminated') + AND agent.lifecycle_state IN ('ready', 'pausing', 'paused', 'resuming') + AND NOT (agent.lifecycle_holds ? 'setup') + AND agent.pause_reason IS DISTINCT FROM 'setup' + AND agent.metadata->'paperclipBuiltInAgent' IS NULL + AND COALESCE(agent.permissions->>'trustPreset', 'standard') = 'standard' + AND COALESCE(agent.permissions->'authorizationPolicy'->>'trustPreset', 'standard') = 'standard' + AND agent.permissions->'reviewPreset'->>'id' IS DISTINCT FROM 'low_trust_review' + AND agent.permissions->'authorizationPolicy'->'reviewPreset'->>'id' IS DISTINCT FROM 'low_trust_review' + AND agent.permissions->'authorizationPolicy'->'trustBoundary' IS NULL + AND NOT EXISTS ( + SELECT 1 FROM principal_permission_grants AS existing + WHERE existing.company_id = agent.company_id + AND existing.principal_type = 'agent' + AND existing.principal_id = agent.id::text + AND existing.permission_key IN ('agents:configure', 'agents:suggest-changes') + AND existing.scope IS NOT NULL + AND existing.scope <> '{}'::jsonb + ) + ON CONFLICT (company_id, principal_type, principal_id, permission_key) DO NOTHING + RETURNING company_id, principal_id +) +INSERT INTO activity_log (company_id, actor_type, actor_id, action, entity_type, entity_id, details) +SELECT company_id, 'system', 'migration:agent-config-read-defaults', 'agent.permissions_updated', 'agent', principal_id, + '{"added":["agents:read"],"source":"migration"}'::jsonb +FROM added; diff --git a/packages/db/src/migrations/meta/_journal.json b/packages/db/src/migrations/meta/_journal.json index b3bc9221371..29dbf25355f 100644 --- a/packages/db/src/migrations/meta/_journal.json +++ b/packages/db/src/migrations/meta/_journal.json @@ -2948,6 +2948,13 @@ "when": 1793503729899, "tag": "0422_eminent_darwin", "breakpoints": true + }, + { + "idx": 423, + "version": "7", + "when": 1793685600000, + "tag": "0423_agent_config_read_defaults", + "breakpoints": true } ] -} \ No newline at end of file +} diff --git a/packages/shared/src/constants.ts b/packages/shared/src/constants.ts index 28f9fe42585..7d0b9cecae2 100644 --- a/packages/shared/src/constants.ts +++ b/packages/shared/src/constants.ts @@ -1009,6 +1009,7 @@ export type JoinRequestStatus = (typeof JOIN_REQUEST_STATUSES)[number]; export const PERMISSION_KEYS = [ "agents:create", + "agents:read", "agents:configure", "agents:suggest-changes", "skills:create", diff --git a/server/src/__tests__/agent-config-read-access.test.ts b/server/src/__tests__/agent-config-read-access.test.ts new file mode 100644 index 00000000000..7a1ab643df6 --- /dev/null +++ b/server/src/__tests__/agent-config-read-access.test.ts @@ -0,0 +1,99 @@ +import { createHash, randomUUID } from "node:crypto"; +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import express from "express"; +import request from "supertest"; +import { and, eq } from "drizzle-orm"; +import { describe, expect, it } from "vitest"; +import { agents, agentApiKeys, authUsers, companies, companyMemberships, createDb, principalPermissionGrants } from "@paperclipai/db"; +import { actorMiddleware } from "../middleware/auth.js"; +import { errorHandler } from "../middleware/error-handler.js"; +import { agentRoutes } from "../routes/agents.js"; +import { resolveManagedInstructionsRoot } from "../services/agent-instructions.js"; +import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "./helpers/embedded-postgres.js"; + +const support = await getEmbeddedPostgresTestSupport(); + +(support.supported ? describe : describe.skip)("agent configuration read access", () => { + it("authenticates a scoped read-only key, redacts reads, denies writes, and rechecks revocation", async () => { + const database = await startEmbeddedPostgresTestDatabase("agent-config-read-access-"); + const db = createDb(database.connectionString); + const previousHome = process.env.PAPERCLIP_HOME; + const home = await fs.mkdtemp(path.join(os.tmpdir(), "agent-config-read-home-")); + process.env.PAPERCLIP_HOME = home; + try { + const companyId = randomUUID(), foreignCompanyId = randomUUID(), actorId = randomUUID(), targetId = randomUUID(), otherId = randomUUID(), foreignId = randomUUID(), userId = randomUUID(); + const plaintext = "config-read-secret-must-not-leak"; + const token = randomUUID(); + const root = resolveManagedInstructionsRoot({ id: targetId, companyId, name: "Target", adapterConfig: {} }); + await fs.mkdir(root, { recursive: true }); + await fs.writeFile(path.join(root, "AGENTS.md"), "Peer operating instructions"); + await db.insert(companies).values([ + { id: companyId, name: "Config read", issuePrefix: "CRA" }, + { id: foreignCompanyId, name: "Foreign", issuePrefix: "CRB" }, + ]); + await db.insert(authUsers).values({ id: userId, name: "Read owner", email: `${userId}@example.test`, createdAt: new Date(), updatedAt: new Date() }); + await db.insert(agents).values([ + { id: actorId, companyId, name: "Reader" }, + { id: targetId, companyId, name: "Target", adapterConfig: { + env: { GENERIC_NAME: { type: "plain", value: plaintext } }, + instructionsBundleMode: "managed", instructionsRootPath: root, instructionsEntryFile: "AGENTS.md", + } }, + { id: otherId, companyId, name: "Outside grant" }, + { id: foreignId, companyId: foreignCompanyId, name: "Foreign target" }, + ]); + await db.insert(companyMemberships).values([ + { companyId, principalType: "user", principalId: userId, membershipRole: "operator" }, + { companyId, principalType: "agent", principalId: actorId, membershipRole: "member" }, + ]); + await db.insert(principalPermissionGrants).values([ + { companyId, principalType: "agent", principalId: actorId, permissionKey: "agents:read", scope: { agentIds: [targetId] } }, + { companyId, principalType: "user", principalId: userId, permissionKey: "agents:read" }, + ]); + await db.insert(agentApiKeys).values({ companyId, agentId: actorId, name: "Reader", responsibleUserId: userId, + keyHash: createHash("sha256").update(token).digest("hex") }); + const app = express(); + app.use(express.json()); + app.use(actorMiddleware(db, { deploymentMode: "authenticated" })); + app.use("/api", agentRoutes(db)); + app.use(errorHandler); + const get = (url: string) => request(app).get(`/api${url}`).set("Authorization", `Bearer ${token}`); + const configuration = `/agents/${targetId}/configuration`; + + const read = await get(configuration); + expect(read.status).toBe(200); + expect(JSON.stringify(read.body)).not.toContain(plaintext); + expect(read.body.adapterConfig.env.GENERIC_NAME.value).toBe("***REDACTED***"); + expect((await get(`/agents/${targetId}/skills`)).status).toBe(200); + expect((await get(`/agents/${targetId}/config-revisions`)).status).toBe(200); + const instructions = await get(`/agents/${targetId}/instructions-bundle/file?path=AGENTS.md`); + expect(instructions.status).toBe(200); + expect(instructions.body.content).toBe("Peer operating instructions"); + expect((await get(`/agents/${otherId}/configuration`)).status).toBe(403); + expect((await get(`/agents/${foreignId}/configuration`)).status).toBe(404); + expect((await get(`/companies/${companyId}/agent-configurations`)).status).toBe(403); + + const write = await request(app).patch(`/api/agents/${targetId}`).set("Authorization", `Bearer ${token}`).send({ title: "Denied edit" }); + expect(write.status).toBe(403); + const rollback = await request(app).post(`/api/agents/${targetId}/config-revisions/${randomUUID()}/rollback`).set("Authorization", `Bearer ${token}`).send({}); + expect(rollback.status).toBe(403); + expect((await db.select({ title: agents.title }).from(agents).where(eq(agents.id, targetId)))[0]!.title).toBeNull(); + + await db.delete(principalPermissionGrants).where(eq(principalPermissionGrants.principalId, actorId)); + expect((await get(configuration)).status).toBe(403); + expect((await get(`/agents/${actorId}/configuration`)).status).toBe(200); + await db.insert(principalPermissionGrants).values({ companyId, principalType: "agent", principalId: actorId, permissionKey: "agents:read" }); + await db.delete(principalPermissionGrants).where(and(eq(principalPermissionGrants.companyId, companyId), eq(principalPermissionGrants.principalId, userId))); + const humanRevoked = await get(configuration); + expect(humanRevoked.status).toBe(403); + expect(humanRevoked.body.code).toBe("RESPONSIBLE_USER_UNAUTHORIZED"); + expect(await fs.readFile(path.join(root, "AGENTS.md"), "utf8")).toBe("Peer operating instructions"); + } finally { + if (previousHome === undefined) delete process.env.PAPERCLIP_HOME; + else process.env.PAPERCLIP_HOME = previousHome; + await database.cleanup(); + await fs.rm(home, { recursive: true, force: true }); + } + }, 120_000); +}); diff --git a/server/src/__tests__/agent-default-configure-grants.test.ts b/server/src/__tests__/agent-default-configure-grants.test.ts index 2525d95655d..b46227c78a8 100644 --- a/server/src/__tests__/agent-default-configure-grants.test.ts +++ b/server/src/__tests__/agent-default-configure-grants.test.ts @@ -13,6 +13,7 @@ import { agentJoinGrantsFromDefaults } from "../services/invite-grants.js"; const support = await getEmbeddedPostgresTestSupport(); const describeDatabase = support.supported ? describe : describe.skip; const expectedNewAgentGrantKeys: PermissionKey[] = [ + "agents:read", "agents:configure", "agents:suggest-changes", "skills:create", diff --git a/server/src/__tests__/agent-instruction-revisions.test.ts b/server/src/__tests__/agent-instruction-revisions.test.ts index 6d2a516cb2c..3688dab1f7b 100644 --- a/server/src/__tests__/agent-instruction-revisions.test.ts +++ b/server/src/__tests__/agent-instruction-revisions.test.ts @@ -64,6 +64,20 @@ describe("canonical instruction revisions", () => { async function save(content: string, baseRevisionId: string | null) { return service.commit({ ...target(), entryFile, content, baseRevisionId, source: "cleanup" }, actor); } + it("reads peer instructions with read-only agent and responsible-user grants but denies saves", async () => { + await db.delete(principalPermissionGrants).where(eq(principalPermissionGrants.companyId, companyId)); + await db.insert(principalPermissionGrants).values([ + { companyId, principalType: "user", principalId: userId, permissionKey: "agents:read", scope: { agentIds: [agentId] } }, + { companyId, principalType: "agent", principalId: actorId, permissionKey: "agents:read", scope: { agentIds: [agentId] } }, + ]); + const first = await base(); + expect(first.content).toBe(initial); + await expect(service.history({ ...target(), entryFile }, actor)).resolves.toMatchObject({ revisions: [] }); + await expect(save("read access cannot write", first.revision.id)).rejects.toMatchObject({ status: 403 }); + await db.delete(principalPermissionGrants).where(eq(principalPermissionGrants.principalId, userId)); + await expect(base()).rejects.toMatchObject({ status: 403 }); + expect(await fs.readFile(path.join(root, entryFile), "utf8")).toBe(initial); + }); it.each(["current", "history", "revision", "diff", "candidates"])("denies peer %s reads without a configuration grant or responsible-user target access", async (operation) => { const first = await base(); await db.delete(principalPermissionGrants).where(eq(principalPermissionGrants.principalId, userId)); diff --git a/server/src/__tests__/agent-permissions-routes.test.ts b/server/src/__tests__/agent-permissions-routes.test.ts index 0c502b54cfc..7c319913884 100644 --- a/server/src/__tests__/agent-permissions-routes.test.ts +++ b/server/src/__tests__/agent-permissions-routes.test.ts @@ -2166,6 +2166,31 @@ describe("agent permission routes", () => { }); describe("agent configuration read gate", () => { + it.each(["configuration", "config-revisions", "config-revisions/55555555-5555-4555-8555-555555555555"])("allows self %s reads without peer configuration grants", async (suffix) => { + const plaintext = "self-config-value-must-not-leak"; + const adapterConfig = { env: { GENERIC_NAME: { type: "plain", value: plaintext } } }; + mockAgentService.getById.mockResolvedValue({ ...baseAgent, adapterConfig }); + const revision = { id: "55555555-5555-4555-8555-555555555555", beforeConfig: { adapterConfig }, afterConfig: { adapterConfig } }; + mockAgentService.listConfigRevisions.mockResolvedValue([revision]); + mockAgentService.getConfigRevision.mockResolvedValue(revision); + mockAccessService.decide.mockImplementation(async (input) => ({ + allowed: input.action === "agent_config:read" && input.resource.type === "agent" && input.resource.agentId === agentId, + reason: "allow_self", + explanation: "Self configuration read", + })); + const app = await createApp({ type: "agent", agentId, companyId, source: "agent_key" }); + + const response = await request(app).get(`/api/agents/${agentId}/${suffix}`); + + expect(response.status).toBe(200); + expect(JSON.stringify(response.body)).not.toContain(plaintext); + expect(mockAccessService.decide).toHaveBeenCalledWith(expect.objectContaining({ + action: "agent_config:read", + resource: { type: "agent", companyId, agentId }, + scope: { targetAgentId: agentId }, + })); + }); + it("allows a board member without agents:create to read agent configuration", async () => { // Board (human) users with company membership but no agents:create // grant should still be able to view agent configuration — this is @@ -2297,11 +2322,12 @@ describe("agent permission routes", () => { expect(res.status).toBe(403); expect(mockAccessService.decide).toHaveBeenCalledWith(expect.objectContaining({ action: "agent_config:read", - resource: { type: "company", companyId }, + resource: { type: "agent", companyId, agentId: peerAgentId }, + scope: { targetAgentId: peerAgentId }, })); }); - it("allows an agent actor with agents:suggest-changes grant to read peer config", async () => { + it.each(["agents:read", "agents:configure", "agents:suggest-changes"])("allows an agent actor with %s grant to read peer config", async (permissionKey) => { // 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"; @@ -2316,11 +2342,11 @@ describe("agent permission routes", () => { mockAccessService.decide.mockResolvedValue({ allowed: true, reason: "allow_explicit_grant", - explanation: "Allowed by explicit grant agents:suggest-changes.", + explanation: `Allowed by explicit grant ${permissionKey}.`, grant: { principalType: "agent", principalId: agentId, - permissionKey: "agents:suggest-changes", + permissionKey, scope: null, }, }); @@ -2338,7 +2364,8 @@ describe("agent permission routes", () => { expect(res.status).toBe(200); expect(mockAccessService.decide).toHaveBeenCalledWith(expect.objectContaining({ action: "agent_config:read", - resource: { type: "company", companyId }, + resource: { type: "agent", companyId, agentId: peerAgentId }, + scope: { targetAgentId: peerAgentId }, })); }); }); diff --git a/server/src/__tests__/authorization-service.test.ts b/server/src/__tests__/authorization-service.test.ts index a2b28921c5e..d85561ea414 100644 --- a/server/src/__tests__/authorization-service.test.ts +++ b/server/src/__tests__/authorization-service.test.ts @@ -269,6 +269,47 @@ describeEmbeddedPostgres("authorization service", () => { expect(decision.explanation).toContain("Allowed by explicit grant tasks:assign"); }); + it.each(["agent", "user"] as const)("allows %s read-only grants without configuration write authority", async (principalType) => { + const company = await createCompany(db, "ReadOnlyConfig"); + const actorAgent = await createAgent(db, company.id); + const targetAgent = await createAgent(db, company.id); + const userId = await createUser(db); + const actor: AuthorizationActor = principalType === "agent" + ? { type: "agent", agentId: actorAgent.id, companyId: company.id, source: "agent_key" } + : { type: "board", userId, source: "session" }; + if (principalType === "agent") await grantAgentPermission(db, company.id, actorAgent.id, "agents:read"); + else await grantUserPermission(db, company.id, userId, "agents:read"); + const authz = authorizationService(db); + const resource = { type: "agent" as const, companyId: company.id, agentId: targetAgent.id }; + + await expect(authz.decide({ actor, action: "agent_config:read", resource })).resolves.toMatchObject({ + allowed: true, grant: { permissionKey: "agents:read" }, + }); + for (const action of ["agent_config:update", "agent_instructions:update"] as const) { + await expect(authz.decide({ actor, action, resource, scope: { requiresChangeGrant: true } })) + .resolves.toMatchObject({ allowed: false }); + } + }); + + it("restricts configuration reads to the read grant's target scope", async () => { + const company = await createCompany(db, "ScopedReadOnlyConfig"); + const actor = await createAgent(db, company.id); + const target = await createAgent(db, company.id); + const other = await createAgent(db, company.id); + await grantAgentPermission(db, company.id, actor.id, "agents:read", { agentIds: [target.id] }); + const authz = authorizationService(db); + const read = (targetAgentId: string) => authz.decide({ + actor: { type: "agent", companyId: company.id, agentId: actor.id, source: "agent_key" }, + action: "agent_config:read", + resource: { type: "agent", companyId: company.id, agentId: targetAgentId }, + scope: { targetAgentId }, + }); + await expect(read(target.id)).resolves.toMatchObject({ allowed: true }); + await expect(read(other.id)).resolves.toMatchObject({ allowed: false, reason: "deny_scope" }); + await db.delete(principalPermissionGrants).where(eq(principalPermissionGrants.principalId, actor.id)); + await expect(read(target.id)).resolves.toMatchObject({ allowed: false, reason: "deny_missing_grant" }); + }); + it("allows suggest grants to read peer agent configuration", async () => { const company = await createCompany(db, "AgentReadGrant"); const actorAgent = await createAgent(db, company.id); @@ -292,7 +333,7 @@ describeEmbeddedPostgres("authorization service", () => { }); }); - it("falls back to the direct config-read grant decision when a suggest read grant is scoped away", async () => { + it("reports a scoped legacy read grant denial when no read grant applies", async () => { const company = await createCompany(db, "AgentReadScopedSuggestGrant"); const actorAgent = await createAgent(db, company.id); const targetAgent = await createAgent(db, company.id); @@ -308,8 +349,8 @@ describeEmbeddedPostgres("authorization service", () => { expect(decision).toMatchObject({ allowed: false, - reason: "deny_missing_grant", - explanation: "Missing permission: agents:configure.", + reason: "deny_scope", + explanation: "Permission agents:suggest-changes does not cover the requested scope.", }); }); diff --git a/server/src/lib/agent-permissions.ts b/server/src/lib/agent-permissions.ts index 24f9bc29d05..15e4fe64695 100644 --- a/server/src/lib/agent-permissions.ts +++ b/server/src/lib/agent-permissions.ts @@ -2,6 +2,7 @@ import { LOW_TRUST_REVIEW_PRESET, type PermissionKey } from "@paperclipai/shared /** Direct grants for newly activated standard agents. Existing agents are not backfilled. */ export const NEW_STANDARD_AGENT_DEFAULT_GRANT_KEYS = [ + "agents:read", "agents:configure", "agents:suggest-changes", "skills:create", diff --git a/server/src/modules/agent-lifecycle/__tests__/lifecycle.test.ts b/server/src/modules/agent-lifecycle/__tests__/lifecycle.test.ts index ab4e53d0839..53ab1392803 100644 --- a/server/src/modules/agent-lifecycle/__tests__/lifecycle.test.ts +++ b/server/src/modules/agent-lifecycle/__tests__/lifecycle.test.ts @@ -130,7 +130,7 @@ const support = external ? { supported: true } : await getEmbeddedPostgresTestSu expect(await db.select().from(activityLog).where(and(eq(activityLog.entityId, agent.id), eq(activityLog.action, "agent.updated")))).toHaveLength(1); await request(board).patch(`/api/agents/${agent.id}`).send({ name: "Stale", expectedConfigurationRevision: response.body.configurationRevision }).expect(409); const foreign = app({ type: "board", source: "session", userId: "outsider", companyIds: [], memberships: [] }); - await request(foreign).get(`/api/agents/${agent.id}/configuration`).expect(403); + await request(foreign).get(`/api/agents/${agent.id}/configuration`).expect(404); await request(foreign).patch(`/api/agents/${agent.id}`).send({ title: "Denied", expectedConfigurationRevision: saved.body.configurationRevision }).expect(404); }); diff --git a/server/src/routes/agents.ts b/server/src/routes/agents.ts index 4afd6910b08..756fed8a50e 100644 --- a/server/src/routes/agents.ts +++ b/server/src/routes/agents.ts @@ -1656,9 +1656,6 @@ export function agentRoutes( // are never exposed. Mutations and environment probes still gate on // agents:create or agents:configure via the mutating route helpers. // - // 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") { const decision = await access.decide({ @@ -1683,7 +1680,7 @@ export function agentRoutes( return agent; } - async function actorCanReadConfigurationsForCompany(req: Request, companyId: string) { + async function actorCanReadConfigurationsForCompany(req: Request, companyId: string, targetAgentId?: string) { // Mirrors assertCanReadConfigurations but returns a boolean instead of // throwing. Board actors only need company access; agent actors must pass // the agent configuration read grant ladder so peer agents cannot snoop @@ -1697,7 +1694,8 @@ export function agentRoutes( const decision = await access.decide({ actor: req.actor, action: "agent_config:read", - resource: { type: "company", companyId }, + resource: targetAgentId ? { type: "agent", companyId, agentId: targetAgentId } : { type: "company", companyId }, + ...(targetAgentId ? { scope: { targetAgentId } } : {}), }); return decision.allowed; } @@ -1839,6 +1837,7 @@ export function agentRoutes( actor: req.actor, action: "agent_config:read", resource: { type: "agent", companyId: targetAgent.companyId, agentId: targetAgent.id }, + scope: { targetAgentId: targetAgent.id }, }); if (decision.allowed) return; @@ -3799,7 +3798,7 @@ export function agentRoutes( res.status(404).json({ error: "Agent not found" }); return; } - await assertCanReadConfigurations(req, agent.companyId); + await assertCanReadAgent(req, agent); const adapter = findActiveServerAdapter(agent.adapterType); if (!adapter?.listSkills) { @@ -4188,7 +4187,7 @@ export function agentRoutes( } const canReadSensitiveDetail = isSelf ? true - : await actorCanReadConfigurationsForCompany(req, agent.companyId); + : await actorCanReadConfigurationsForCompany(req, agent.companyId, agent.id); if (!canReadSensitiveDetail) { res.json(await buildAgentDetail(agent, { restricted: true })); return; @@ -4203,7 +4202,7 @@ export function agentRoutes( res.status(404).json({ error: "Agent not found" }); return; } - await assertCanReadConfigurations(req, agent.companyId); + await assertCanReadAgent(req, agent); res.json(redactAgentConfiguration(agent)); }); @@ -4214,7 +4213,7 @@ export function agentRoutes( res.status(404).json({ error: "Agent not found" }); return; } - await assertCanReadConfigurations(req, agent.companyId); + await assertCanReadAgent(req, agent); const revisions = await svc.listConfigRevisions(id); res.json(revisions.map((revision) => redactConfigRevision(revision))); }); @@ -4227,7 +4226,7 @@ export function agentRoutes( res.status(404).json({ error: "Agent not found" }); return; } - await assertCanReadConfigurations(req, agent.companyId); + await assertCanReadAgent(req, agent); const revision = await svc.getConfigRevision(id, revisionId); if (!revision) { res.status(404).json({ error: "Revision not found" }); diff --git a/server/src/services/authorization.ts b/server/src/services/authorization.ts index a9c65b694ec..12691864f9a 100644 --- a/server/src/services/authorization.ts +++ b/server/src/services/authorization.ts @@ -1887,30 +1887,20 @@ function createAuthorizationService(db: Db | DbTransaction, readSnapshot?: ReadA 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; + let denied: AuthorizationDecision | null = null; + for (const permissionKey of ["agents:read", "agents:configure", "agents:suggest-changes"] as const) { + const decision = await decidePrincipalGrant({ + companyId, + principalType, + principalId, + action: input.action, + permissionKey, + scope: input.scope, + }); + if (decision.allowed || decision.reason === "deny_missing_membership") return decision; + if (!denied || decision.reason === "deny_scope") denied = decision; } - - const suggestDecision = await decidePrincipalGrant({ - companyId, - principalType, - principalId, - action: input.action, - permissionKey: "agents:suggest-changes", - scope: input.scope, - }); - if (suggestDecision.allowed || suggestDecision.reason === "deny_missing_grant") { - return suggestDecision; - } - return configureDecision; + return denied!; } async function decideWithProtectedChangeGrants(