fix(instructions): enforce scoped authority for peer reads

Require configuration-read grants or strict responsible-user target edit authority before exposing peer instruction content, history, and candidates. Preserve self, board, authorized delegated edits, and external host restrictions. Read decisions do not consume or bypass write consent.

Co-Authored-By: Paperclip <noreply@paperclip.ing>

doc/agent-instruction-revisions.md

@@ -99,6 +99,13 @@ confirmation inside the commit transaction. Low-trust, task-bridge, and skill-te
 containment remains enforced. This path enforces the responsible-user ceiling even
 if the general responsible-user policy is configured in shadow mode.
 
+Peer instruction reads require the existing `agent_config:read` grant or the
+same strict delegated target content-edit authority. General same-company agent
+visibility is insufficient for bundle files, history, diffs, and candidates.
+Read checks do not bypass or consume protected-change consent. Self and board
+reads retain their existing company visibility rules; external host bundles keep
+their instance-admin and configuration-read restrictions.
+
 ## HTTP and editor
 
 Existing company-scoped agent URLs now support:

server/src/__tests__/agent-instruction-revisions.test.ts

@@ -8,6 +8,7 @@ import { agents, agentApiKeys, companies, authUsers, companyMemberships, princip
   agentInstructionRevisions, agentInstructionHeads, issueThreadInteractions, issues, createDb } from "@paperclipai/db";
 import { startEmbeddedPostgresTestDatabase } from "./helpers/embedded-postgres.js";
 import { agentInstructionRevisionService } from "../services/agent-instruction-revisions.js";
+import { agentInstructionWorkingCopyService } from "../services/agent-instruction-working-copies.js";
 import { instructionBytes, instructionPath, readInstructionBytes } from "../services/agent-instruction-files.js";
 import { agentInstructionsService, resolveManagedInstructionsRoot } from "../services/agent-instructions.js";
 import type { AuthorizationActor } from "../services/authorization.js";
@@ -60,6 +61,34 @@ describe("canonical instruction revisions", () => {
   async function save(content: string, baseRevisionId: string | null) {
     return service.commit({ ...target(), entryFile, content, baseRevisionId, source: "cleanup" }, actor);
   }
+  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));
+    const read = () => {
+      if (operation === "current") return service.readCurrent(target(), actor);
+      if (operation === "history") return service.history({ ...target(), entryFile }, actor);
+      if (operation === "revision") return service.readRevision({ ...target(), entryFile, revisionId: first.revision.id }, actor);
+      if (operation === "diff") return service.diff({ ...target(), entryFile, fromRevisionId: first.revision.id, toRevisionId: first.revision.id }, actor);
+      return agentInstructionWorkingCopyService(db).list(companyId, agentId, actor);
+    };
+    await expect(read()).rejects.toMatchObject({ status: 403 });
+    await db.insert(principalPermissionGrants).values({ companyId, principalType: "user", principalId: userId, permissionKey: "agents:configure", scope: { agentIds: [actorId] } });
+    await expect(read()).rejects.toMatchObject({ status: 403 });
+  });
+  it("allows delegated peer content reads but preserves explicit agent target restrictions", async () => {
+    expect(await db.select().from(principalPermissionGrants).where(eq(principalPermissionGrants.principalId, actorId))).toHaveLength(0);
+    expect((await base()).content).toBe(initial);
+    await db.insert(principalPermissionGrants).values({ companyId, principalType: "agent", principalId: actorId, permissionKey: "agents:configure", scope: { agentIds: [actorId] } });
+    await expect(service.readCurrent(target(), actor)).rejects.toMatchObject({ status: 403 });
+  });
+  it("allows self and board reads without requiring instruction edit permission", async () => {
+    await db.delete(principalPermissionGrants).where(eq(principalPermissionGrants.principalId, userId));
+    const selfRoot = resolveManagedInstructionsRoot({ id: actorId, companyId, name: "Writer", adapterConfig: {} });
+    await fs.mkdir(selfRoot, { recursive: true });
+    await fs.writeFile(path.join(selfRoot, "AGENTS.md"), "Self instructions");
+    expect((await service.readCurrent({ companyId, agentId: actorId }, actor))?.content).toBe("Self instructions");
+    expect((await service.readCurrent(target(), { type: "board", userId, source: "session" }))?.content).toBe(initial);
+  });
   it("seeds configured bytes, preserves immutable history, deduplicates, diffs and restores as a new revision", async () => {
     const first = await base();
     expect(first.content).toBe(initial);
@@ -171,6 +200,8 @@ describe("canonical instruction revisions", () => {
       payload: { version: 1, prompt: "Apply the instruction change?", detailsMarkdown: "```diff\n+approved\n```", target: { type: "custom", key: `agent:${agentId}:instructions`, revisionId: "proposal" } },
       result: { version: 1, outcome: "accepted" }, resolvedByUserId: userId, resolvedAt: new Date(),
     });
+    expect((await service.readCurrent(target(), actor))?.revision.id).toBe(first.revision.id);
+    expect((await db.select().from(issueThreadInteractions).where(eq(issueThreadInteractions.id, interactionId)))[0].result).not.toHaveProperty("consumedAt");
     await expect(save("approved", randomUUID())).rejects.toMatchObject({ status: 409 });
     expect((await db.select().from(issueThreadInteractions).where(eq(issueThreadInteractions.id, interactionId)))[0].result).not.toHaveProperty("consumedAt");
     const receipt = await save("approved", first.revision.id);

server/src/__tests__/agent-instructions-routes.test.ts

@@ -30,6 +30,9 @@ vi.mock("../services/agent-instruction-working-copies.js", () => ({ agentInstruc
 const mockInstructionRevisions = vi.hoisted(() => ({ readCurrent: vi.fn(), commit: vi.fn(), restore: vi.fn(), history: vi.fn(), readRevision: vi.fn(), diff: vi.fn(), materializeCurrent: vi.fn() }));
 vi.mock("../services/agent-instruction-revisions.js", () => ({ agentInstructionRevisionService: () => mockInstructionRevisions }));
 
+const mockAuthorizeInstructionRead = vi.hoisted(() => vi.fn());
+vi.mock("../services/agent-instruction-authorization.js", () => ({ authorizeInstructionRead: mockAuthorizeInstructionRead }));
+
 const mockAccessService = vi.hoisted(() => ({
   canUser: vi.fn(),
   decide: vi.fn(),
@@ -208,6 +211,7 @@ describe("agent instructions bundle routes", () => {
     vi.doUnmock("../middleware/index.js");
     registerModuleMocks();
     vi.clearAllMocks();
+    mockAuthorizeInstructionRead.mockImplementation(async (_db, actor) => actor);
     mockInstructionRevisions.readCurrent.mockResolvedValue(null);
     mockInstructionRevisions.commit.mockResolvedValue({ revision: { id: "33333333-3333-4333-8333-333333333333", entryFile: "AGENTS.md", byteLength: 18 }, content: "# Updated Agent\n", changed: true, materialization: "current" });
     mockBuiltInAgentService.ensureCompanyDefaultAgentGrants.mockResolvedValue(0);
@@ -294,6 +298,8 @@ describe("agent instructions bundle routes", () => {
       entryFile: "AGENTS.md",
     });
     expect(mockAgentInstructionsService.getBundle).toHaveBeenCalled();
+    expect(mockAuthorizeInstructionRead).toHaveBeenCalledWith(expect.anything(), expect.objectContaining({ type: "board" }),
+      expect.objectContaining({ companyId: "company-1", id: "11111111-1111-4111-8111-111111111111" }));
   });
 
   it("requires instance-admin access for every external instruction entry point", async () => {
@@ -339,6 +345,7 @@ describe("agent instructions bundle routes", () => {
     expect(mockAgentInstructionsService.updateBundle).not.toHaveBeenCalled();
     expect(mockAgentInstructionsService.writeFile).not.toHaveBeenCalled();
     expect(mockAgentInstructionsService.deleteFile).not.toHaveBeenCalled();
+    expect(mockAuthorizeInstructionRead).not.toHaveBeenCalled();
   });
 
   it("treats a host root mislabeled as managed as external", async () => {
@@ -391,6 +398,7 @@ describe("agent instructions bundle routes", () => {
 
     expect(res.status, JSON.stringify(res.body)).toBe(200);
     expect(mockAgentInstructionsService.getBundle).toHaveBeenCalled();
+    expect(mockAuthorizeInstructionRead).not.toHaveBeenCalled();
   });
 
   it("rejects a company admin that requests a new external instruction root", async () => {
@@ -492,7 +500,8 @@ describe("agent instructions bundle routes", () => {
     expect(mockAgentService.create).not.toHaveBeenCalled();
   });
 
-  it("denies instruction reads when the visibility policy denies access", async () => {
+  it.each(["", "/file?path=AGENTS.md", "/file?path=private-support.md"])(
+    "denies peer instruction reads before any data dispatch: %s", async (suffix) => {
     mockAgentService.getById.mockImplementation(async (id: string) => {
       if (id === "agent-reader") {
         return {
@@ -504,11 +513,10 @@ describe("agent instructions bundle routes", () => {
       }
       return makeAgent();
     });
-    mockAccessService.decide.mockResolvedValue({
-      allowed: false,
-      reason: "deny_no_grant",
-      explanation: "Missing permission: agents:configure or agents:suggest-changes.",
-    });
+    // General agent visibility is allowed. It must not authorize instruction
+    // metadata, canonical entry bytes, or supporting files from a peer.
+    const { forbidden } = await vi.importActual<typeof import("../errors.js")>("../errors.js");
+    mockAuthorizeInstructionRead.mockRejectedValue(forbidden("Missing permission to read peer instructions"));
 
     const res = await requestApp(
       await createApp({
@@ -518,20 +526,18 @@ describe("agent instructions bundle routes", () => {
         source: "agent_key",
       }),
       (baseUrl) => request(baseUrl)
-        .get("/api/agents/11111111-1111-4111-8111-111111111111/instructions-bundle"),
+        .get(`/api/agents/11111111-1111-4111-8111-111111111111/instructions-bundle${suffix}`),
     );
 
     expect(res.status, JSON.stringify(res.body)).toBe(403);
     expect(res.body.error).toContain("Missing permission");
-    expect(mockAccessService.decide).toHaveBeenCalledWith(expect.objectContaining({
-      action: "agent:read",
-      resource: {
-        type: "agent",
-        companyId: "company-1",
-        agentId: "11111111-1111-4111-8111-111111111111",
-      },
-    }));
+    expect(mockAuthorizeInstructionRead).toHaveBeenCalledWith(expect.anything(), expect.objectContaining({
+      type: "agent", agentId: "agent-reader",
+    }), { companyId: "company-1", id: "11111111-1111-4111-8111-111111111111" });
     expect(mockAgentInstructionsService.getBundle).not.toHaveBeenCalled();
+    expect(mockAgentInstructionsService.readFile).not.toHaveBeenCalled();
+    expect(mockInstructionRevisions.readCurrent).not.toHaveBeenCalled();
+    expect(mockInstructionRevisions.materializeCurrent).not.toHaveBeenCalled();
   });
 
   it("allows agents to read their own instructions bundles", async () => {
@@ -548,9 +554,12 @@ describe("agent instructions bundle routes", () => {
 
     expect(res.status, JSON.stringify(res.body)).toBe(200);
     expect(mockAgentInstructionsService.getBundle).toHaveBeenCalled();
+    expect(mockAuthorizeInstructionRead).toHaveBeenCalledWith(expect.anything(), expect.objectContaining({
+      type: "agent", agentId: "11111111-1111-4111-8111-111111111111",
+    }), { companyId: "company-1", id: "11111111-1111-4111-8111-111111111111" });
   });
 
-  it("allows agents with suggest grants to read peer instructions bundles", async () => {
+  it.each(["AGENTS.md", "private-support.md"])("allows an authorized peer to read instruction file %s", async (filePath) => {
     mockAccessService.decide.mockResolvedValue({
       allowed: true,
       reason: "allow_explicit_grant",
@@ -578,21 +587,16 @@ describe("agent instructions bundle routes", () => {
       }),
       (baseUrl) => request(baseUrl)
         .get("/api/agents/11111111-1111-4111-8111-111111111111/instructions-bundle/file")
-        .query({ path: "AGENTS.md" }),
+        .query({ path: filePath }),
     );
 
     expect(res.status, JSON.stringify(res.body)).toBe(200);
-    expect(mockAccessService.decide).toHaveBeenCalledWith(expect.objectContaining({
-      action: "agent:read",
-      resource: {
-        type: "agent",
-        companyId: "company-1",
-        agentId: "11111111-1111-4111-8111-111111111111",
-      },
-    }));
+    expect(mockAuthorizeInstructionRead).toHaveBeenCalledWith(expect.anything(), expect.objectContaining({
+      type: "agent", agentId: "coach-agent",
+    }), { companyId: "company-1", id: "11111111-1111-4111-8111-111111111111" });
     expect(mockAgentInstructionsService.readFile).toHaveBeenCalledWith(
       expect.objectContaining({ id: "11111111-1111-4111-8111-111111111111" }),
-      "AGENTS.md",
+      filePath,
     );
   });
 

server/src/routes/agents.ts

@@ -67,6 +67,7 @@ import { validate } from "../middleware/validate.js";
 import { inheritNativeRunnerAdapterConfig } from "../services/native-runtime/native-agent-runtime-inheritance.js";
 import { agentInstructionRevisionService } from "../services/agent-instruction-revisions.js";
 import { agentInstructionWorkingCopyService } from "../services/agent-instruction-working-copies.js";
+import { authorizeInstructionRead } from "../services/agent-instruction-authorization.js";
 import { instructionPath } from "../services/agent-instruction-files.js";
 import { agentInstructionsBundleMode, deriveBundleState } from "../services/agent-instructions.js";
 import {
@@ -5081,9 +5082,12 @@ export function agentRoutes(
     const id = req.params.id as string;
     const existing = await getAccessibleResource(req, res, svc.getById(id), "Agent not found");
     if (!existing) return;
-    const readDecision = await access.decide({ actor: req.actor, action: "agent:read", resource: { type: "agent", companyId: existing.companyId, agentId: existing.id } });
-    if (!readDecision.allowed) throw forbidden(readDecision.explanation, authorizationDeniedDetails(readDecision));
     assertExternalInstructionsAdmin(req, existing);
+    if (agentInstructionsBundleMode(existing) === "external") {
+      await assertCanReadAgent(req, existing);
+    } else {
+      await authorizeInstructionRead(db, req.actor, { companyId: existing.companyId, id: existing.id });
+    }
     if (agentInstructionsBundleMode(existing) !== "external") {
       const target = { companyId: existing.companyId, agentId: existing.id };
       const current = await instructionRevisions.readCurrent(target, req.actor);
@@ -5151,9 +5155,12 @@ export function agentRoutes(
     const id = req.params.id as string;
     const existing = await getAccessibleResource(req, res, svc.getById(id), "Agent not found");
     if (!existing) return;
-    const readDecision = await access.decide({ actor: req.actor, action: "agent:read", resource: { type: "agent", companyId: existing.companyId, agentId: existing.id } });
-    if (!readDecision.allowed) throw forbidden(readDecision.explanation, authorizationDeniedDetails(readDecision));
     assertExternalInstructionsAdmin(req, existing);
+    if (agentInstructionsBundleMode(existing) === "external") {
+      await assertCanReadAgent(req, existing);
+    } else {
+      await authorizeInstructionRead(db, req.actor, { companyId: existing.companyId, id: existing.id });
+    }
 
     const relativePath = typeof req.query.path === "string" ? req.query.path : "";
     if (!relativePath.trim()) {

server/src/services/agent-instruction-authorization.ts

@@ -134,6 +134,36 @@ export async function resolveInstructionActor(
   };
 }
 
+/** Instruction content is narrower than general same-company agent visibility. */
+export async function authorizeInstructionRead(
+  db: Connection,
+  actor: AuthorizationActor,
+  target: { companyId: string; id: string },
+) {
+  const bound = await resolveInstructionActor(db, actor);
+  const access = authorizationService(db);
+  const resource = { type: "agent" as const, companyId: target.companyId, agentId: target.id };
+  const peer = bound.type === "agent" && bound.agentId !== target.id;
+  let decision = await access.decide({
+    actor: bound,
+    action: peer ? "agent_config:read" : "agent:read",
+    resource,
+    scope: { targetAgentId: target.id },
+  });
+  if (!decision.allowed && peer) {
+    // Ordinary delegated instruction edits must remain possible without a new
+    // blanket agent-admin grant. The existing content-edit decision enforces
+    // current responsible-user target access and explicit agent containment.
+    // This is a read-only decision: no write, consent bypass, or consumption.
+    decision = await access.decide({
+      actor: bound, action: "agent_instructions:update", resource,
+      scope: { targetAgentId: target.id, requiresChangeGrant: true },
+    });
+  }
+  if (!decision.allowed) throw forbidden(decision.explanation, authorizationDeniedDetails(decision));
+  return bound;
+}
+
 /** Current target edit access + agent containment + protected-change consent, shared by all writers. */
 export async function authorizeInstructionCommit(
   db: Connection,

server/src/services/agent-instruction-revisions.ts

@@ -31,13 +31,9 @@ import {
 } from "./agent-instruction-files.js";
 import {
   authorizeInstructionCommit,
-  resolveInstructionActor,
+  authorizeInstructionRead,
 } from "./agent-instruction-authorization.js";
-import {
-  authorizationService,
-  authorizationDeniedDetails,
-  type AuthorizationActor,
-} from "./authorization.js";
+import type { AuthorizationActor } from "./authorization.js";
 
 type Tx = Parameters<Parameters<Db["transaction"]>[0]>[0];
 type Revision = typeof revisions.$inferSelect;
@@ -116,22 +112,7 @@ export function agentInstructionRevisionService(db: Db) {
     actor: AuthorizationActor,
     target: InstructionTarget,
   ) {
-    const bound = await resolveInstructionActor(tx, actor);
-    const decision = await authorizationService(tx).decide({
-      actor: bound,
-      action: "agent:read",
-      resource: {
-        type: "agent",
-        companyId: target.companyId,
-        agentId: target.agentId,
-      },
-    });
-    if (!decision.allowed)
-      throw forbidden(
-        decision.explanation,
-        authorizationDeniedDetails(decision),
-      );
-    return bound;
+    return authorizeInstructionRead(tx, actor, { companyId: target.companyId, id: target.agentId });
   }
   // Trusted host-only reset authority. No HTTP or agent tool accepts this actor.
   async function authorizePluginReset(tx: Tx, actor: PluginResetActor, target: InstructionTarget, agent: typeof agents.$inferSelect) {