mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-04 11:01:16 +01:00
feat(auth): add independent agent configuration read permission (#18766)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Agents need to inspect configuration and instructions when they set up teammates. > - Configuration reads currently depend on configure or suggestion grants. > - Legacy standard agents did not receive those newer grants. Some self-read routes also lose the target agent identity. > - This pull request adds an independent read grant and applies only that grant to eligible existing agents. > - The benefit is that agents can inspect and verify configuration without new write authority. ## Linked Issues or Issue Description Refs #15283. Related earlier proposals: #8064 and #12050. Those proposals target older authorization code; this change uses the current configure/suggest model and adds a read-only rollout for existing agents. Related bounded peer metadata proposal: #14160. **What existing behavior does this improve?** Agent configuration, configured skill, instruction, and revision reads through the existing APIs and native instruction tools. **Current behavior** An agent needs `agents:configure` or `agents:suggest-changes` to inspect a peer. Older standard agents can hire a teammate but cannot verify the saved instructions. Configuration and revision routes also reject self-reads because they authorize a company resource instead of the target agent. **Proposed behavior** Add `agents:read` for redacted configuration and instruction reads. Keep `agents:configure` as read/write and preserve suggestion-grant compatibility. Standard new agents receive the read grant. A one-time migration grants only reads to eligible existing standard agents. Individual routes authorize the target agent and grant scope. **Reason and benefit** Read access can be granted without configuration changes, rollback, or environment probes. Legacy agents can verify teammates without receiving new write permissions. ## What Changed - Add `agents:read` to the shared permission contract and standard-agent defaults. - Accept read, configure, or suggestion grants for configuration reads, with target scope and the responsible-user ceiling. - Fix target-aware self and scoped configuration, skill, and revision reads. - Add an audited data migration. Skip low-trust, built-in, inactive, pending, terminated, setup, and scoped configuration/suggestion cases. Preserve existing read scopes and later revocation. - Update the specifications, default-permission documentation, and agent access map. ## Verification - Focused acceptance: 276 tests passed across seven suites on commit `a37314df88`: authorization, real-key HTTP access, route authorization, canonical instructions, default grants, migration replay, and lifecycle configuration. - The HTTP journey uses a real hashed agent API key and a disposable PostgreSQL database. It verifies redaction, scoped reads, denied edits and rollback, cross-company isolation, and both agent and responsible-user revocation. - New read-grant regressions failed before the authorization change. A realistic paused/pausing/resuming migration fixture also failed before the lifecycle filter correction and passed afterward. Setup-held paused agents stay excluded. - `pnpm -r typecheck` and `pnpm build` passed on the rebased branch; the final lifecycle migration correction also passed the database package typecheck and build. The latest commit changes only a test assertion. - `pnpm --filter @paperclipai/db check:migrations`, `node scripts/check-maps.mjs`, and `git diff --check` passed. - Full current-head [CI run](https://github.com/paperclipai/paperclip/actions/runs/41641531426) passed: typecheck, build, all general and serialized test lanes, Runner checks, all eight browser shards, canary dry run, policy, and aggregate gates. The local full-suite attempt was stopped after exposing the outdated cross-company 403 assertion; interrupted local full-suite attempts are not counted as passes. ## Risks The migration deliberately expands redacted configuration and instruction visibility for eligible existing standard agents in the same company. An absent older write grant does not deny the new read capability. It adds no write grant. Scoped configure/suggestion grants and all existing read-grant scopes stay intact. Administrators can remove the independent read grant after the one-time rollout; ordinary updates and startup do not restore it. Configure and suggestion grants still imply reads within their own scopes. Cross-company configuration/history reads now return 404 through the common target-aware helper. No schema change or credential-value disclosure is added. ## Model Used OpenAI GPT-6 through Codex, with reasoning, repository tools, and code execution. The host does not expose a more specific runtime model ID or context-window size. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
DottaandPaperclip authored and GitHub committed
1 parent
44a9a043b8
commit
86b365f992
17 files changed
+385
-45
No files matched your search
@@ -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
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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.
|
||||
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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<string, string>();
|
||||
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);
|
||||
});
|
||||
@@ -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;
|
||||
+8
-1
@@ -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
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
@@ -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",
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
@@ -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",
|
||||
|
||||
@@ -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));
|
||||
|
||||
@@ -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 },
|
||||
}));
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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.",
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
|
||||
|
||||
@@ -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" });
|
||||
|
||||
@@ -1887,30 +1887,20 @@ function createAuthorizationService(db: Db | DbTransaction, readSnapshot?: ReadA
|
||||
principalType: PrincipalType,
|
||||
principalId: string,
|
||||
): Promise<AuthorizationDecision> {
|
||||
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(
|
||||
|
||||
Reference in new issue
Block a user