From afcbfe1335799dabfef1dadf84fe7503c8efc6ea Mon Sep 17 00:00:00 2001 From: Dotta Date: Sun, 27 Sep 2026 21:29:13 -0500 Subject: [PATCH] fix: preserve canonical history on managed plugin reset Record explicit managed-agent instruction resets through the canonical revision CAS path under exact plugin ownership and capability checks. Preserve prior content and plugin audit identity without synthetic user authority. Co-Authored-By: Paperclip --- .../__tests__/plugin-managed-agents.test.ts | 79 ++++++++++++++++++- .../services/agent-instruction-revisions.ts | 55 ++++++++++--- server/src/services/plugin-managed-agents.ts | 60 ++++++++++---- 3 files changed, 168 insertions(+), 26 deletions(-) diff --git a/server/src/__tests__/plugin-managed-agents.test.ts b/server/src/__tests__/plugin-managed-agents.test.ts index 34ba238c554..b800680b907 100644 --- a/server/src/__tests__/plugin-managed-agents.test.ts +++ b/server/src/__tests__/plugin-managed-agents.test.ts @@ -7,6 +7,8 @@ import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest"; import { activityLog, agentConfigRevisions, + agentInstructionRevisions, + agentInstructionHeads, agents, approvals, companies, @@ -23,6 +25,7 @@ import { } from "./helpers/embedded-postgres.js"; import { buildHostServices } from "../services/plugin-host-services.js"; import { agentService } from "../services/agents.js"; +import { agentInstructionRevisionService } from "../services/agent-instruction-revisions.js"; const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport(); const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip; @@ -312,7 +315,7 @@ describeEmbeddedPostgres("plugin-managed agents", () => { it("materializes declared managed agent instructions with local folder paths", async () => { const previousHome = process.env.PAPERCLIP_HOME; const previousInstance = process.env.PAPERCLIP_INSTANCE_ID; - const tempHome = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-managed-agent-home-")); + const tempHome = await fs.realpath(await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-managed-agent-home-"))); const wikiRoot = await fs.realpath(await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-managed-agent-wiki-"))); process.env.PAPERCLIP_HOME = tempHome; process.env.PAPERCLIP_INSTANCE_ID = "test"; @@ -369,6 +372,34 @@ describeEmbeddedPostgres("plugin-managed agents", () => { expect(content).toContain("You are the LLM Wiki Maintainer."); expect(content).toContain(`Wiki root: \`${wikiRoot}\``); expect(content).toContain(`Wiki schema: \`${path.join(wikiRoot, "AGENTS.md")}\``); + + // A managed plugin reset must preserve the previous entry in canonical + // history instead of deleting a shared bundle or bypassing its write guard. + const edited = "# User-customized wiki instructions\n"; + await fs.writeFile(instructionsFilePath as string, edited); + const reset = await services.agents.managedReset({ companyId, agentKey: "wiki-maintainer" }); + expect(reset.status).toBe("reset"); + expect(await fs.readFile(instructionsFilePath as string, "utf8")).toBe(content); + const history = await db.select().from(agentInstructionRevisions) + .where(eq(agentInstructionRevisions.agentId, created.agentId!)); + expect(history).toHaveLength(2); + const seed = history.find((revision) => revision.source === "seed")!; + const saved = history.find((revision) => revision.id !== seed.id)!; + expect(Buffer.from(seed.contentBase64, "base64").toString()).toBe(edited); + expect(Buffer.from(saved.contentBase64, "base64").toString()).toBe(content); + expect(saved).toMatchObject({ parentRevisionId: seed.id, baseRevisionId: seed.id, + actorUserId: null, actorAgentId: null, responsibleUserId: null }); + const [head] = await db.select().from(agentInstructionHeads) + .where(eq(agentInstructionHeads.agentId, created.agentId!)); + expect(head.revisionId).toBe(saved.id); + const repeated = await services.agents.managedReset({ companyId, agentKey: "wiki-maintainer" }); + expect(repeated.status).toBe("reset"); + expect(await db.select().from(agentInstructionRevisions) + .where(eq(agentInstructionRevisions.agentId, created.agentId!))).toHaveLength(2); + const audit = await db.select().from(activityLog).where(eq(activityLog.action, "agent.instructions_revision_committed")); + expect(audit).toHaveLength(2); + expect(audit.every((row) => row.actorType === "plugin" && row.actorId === pluginId)).toBe(true); + } finally { if (previousHome === undefined) delete process.env.PAPERCLIP_HOME; else process.env.PAPERCLIP_HOME = previousHome; @@ -379,6 +410,52 @@ describeEmbeddedPostgres("plugin-managed agents", () => { } }); + it("fences plugin instruction reset by exact ownership, current capability and canonical CAS", async () => { + const previousHome = process.env.PAPERCLIP_HOME; + const tempHome = await fs.realpath(await fs.mkdtemp(path.join(os.tmpdir(), "plugin-reset-cas-"))); + process.env.PAPERCLIP_HOME = tempHome; + try { + const pluginManifest = manifest(); + pluginManifest.agents![0]!.instructions = { content: "# Default\n" }; + const { companyId, pluginId, services } = await seedCompanyAndPlugin({ manifest: pluginManifest }); + const created = await services.agents.managedReconcile({ companyId, agentKey: "wiki-maintainer" }); + const target = { companyId, agentId: created.agentId! }; + const actor = { type: "plugin" as const, pluginId, pluginKey: pluginManifest.id, agentKey: "wiki-maintainer" }; + const revisions = agentInstructionRevisionService(db); + const baseline = (await revisions.readForPluginReset(target, actor))!; + const input = { ...target, entryFile: "AGENTS.md", baseRevisionId: baseline.revision.id, content: "# Reset\n" }; + await expect(revisions.commitPluginReset(input, { ...actor, pluginId: randomUUID() })).rejects.toMatchObject({ status: 403 }); + await expect(revisions.commitPluginReset({ ...input, companyId: randomUUID() }, actor)).rejects.toMatchObject({ status: 404 }); + await expect(revisions.commitPluginReset(input, { ...actor, agentKey: "foreign-agent" })).rejects.toMatchObject({ status: 403 }); + await db.update(plugins).set({ manifestJson: { ...pluginManifest, capabilities: [] } }).where(eq(plugins.id, pluginId)); + await expect(revisions.commitPluginReset(input, actor)).rejects.toMatchObject({ status: 403 }); + await db.update(plugins).set({ manifestJson: pluginManifest }).where(eq(plugins.id, pluginId)); + const originalMetadata = created.agent!.metadata; + await db.update(agents).set({ metadata: {} }).where(eq(agents.id, target.agentId)); + await expect(revisions.commitPluginReset(input, actor)).rejects.toMatchObject({ status: 403 }); + await db.update(agents).set({ metadata: originalMetadata }).where(eq(agents.id, target.agentId)); + const [binding] = await db.select().from(pluginManagedResources).where(eq(pluginManagedResources.resourceId, target.agentId)); + await db.update(pluginManagedResources).set({ resourceId: randomUUID() }).where(eq(pluginManagedResources.id, binding.id)); + await expect(revisions.commitPluginReset(input, actor)).rejects.toMatchObject({ status: 403 }); + await db.update(pluginManagedResources).set({ resourceId: target.agentId }).where(eq(pluginManagedResources.id, binding.id)); + const results = await Promise.allSettled([ + revisions.commitPluginReset(input, actor), + revisions.commitPluginReset({ ...input, content: "# Concurrent reset\n" }, actor), + ]); + expect(results.filter((result) => result.status === "fulfilled")).toHaveLength(1); + const loser = results.find((result) => result.status === "rejected") as PromiseRejectedResult; + expect(loser.reason).toMatchObject({ status: 409, details: { code: "INSTRUCTION_REVISION_CONFLICT" } }); + const winner = results.find((result) => result.status === "fulfilled") as PromiseFulfilledResult>>; + expect(await fs.readFile(created.agent!.adapterConfig.instructionsFilePath as string, "utf8")).toBe(winner.value.content); + const history = await db.select().from(agentInstructionRevisions).where(eq(agentInstructionRevisions.agentId, target.agentId)); + expect(history).toHaveLength(2); + expect(history.find((row) => row.id === baseline.revision.id)?.contentBase64).toBe(Buffer.from("# Default\n").toString("base64")); + } finally { + if (previousHome === undefined) delete process.env.PAPERCLIP_HOME; else process.env.PAPERCLIP_HOME = previousHome; + await fs.rm(tempHome, { recursive: true, force: true }); + } + }); + it("repairs a missing binding by relinking a same-company managed agent marker", async () => { const { companyId, pluginId, pluginManifest, services } = await seedCompanyAndPlugin(); const agentId = randomUUID(); diff --git a/server/src/services/agent-instruction-revisions.ts b/server/src/services/agent-instruction-revisions.ts index 95b1da9f7d1..2809890b7ed 100644 --- a/server/src/services/agent-instruction-revisions.ts +++ b/server/src/services/agent-instruction-revisions.ts @@ -3,6 +3,8 @@ import { and, desc, eq, getTableColumns, lt, or, sql } from "drizzle-orm"; import { activityLog, agents, + plugins, + pluginManagedResources, agentInstructionHeads as heads, agentInstructionRevisions as revisions, type Db, @@ -39,6 +41,8 @@ import { type Tx = Parameters[0]>[0]; type Revision = typeof revisions.$inferSelect; +type PluginResetActor = { type: "plugin"; pluginId: string; pluginKey: string; agentKey: string }; +type RevisionActor = AuthorizationActor | PluginResetActor; const { contentBase64: _contentColumn, ...revisionMetadataColumns } = getTableColumns(revisions); export type InstructionTarget = { companyId: string; agentId: string }; @@ -129,6 +133,31 @@ export function agentInstructionRevisionService(db: Db) { ); return bound; } + // 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) { + const [plugin] = await tx.select().from(plugins).where(and(eq(plugins.id, actor.pluginId), eq(plugins.pluginKey, actor.pluginKey))); + const marker = agent.metadata?.paperclipManagedResource as Record | undefined; + const [binding] = await tx.select().from(pluginManagedResources).where(and( + eq(pluginManagedResources.pluginId, actor.pluginId), eq(pluginManagedResources.pluginKey, actor.pluginKey), + eq(pluginManagedResources.companyId, target.companyId), eq(pluginManagedResources.resourceKind, "agent"), + eq(pluginManagedResources.resourceKey, actor.agentKey), eq(pluginManagedResources.resourceId, target.agentId), + )); + if (!plugin || plugin.status !== "ready" || !plugin.manifestJson.capabilities.includes("agents.managed") + || !plugin.manifestJson.agents?.some((declaration) => declaration.agentKey === actor.agentKey) + || !binding || marker?.pluginId !== actor.pluginId || marker.pluginKey !== actor.pluginKey + || marker.resourceKind !== "agent" || marker.resourceKey !== actor.agentKey) { + throw forbidden("Plugin reset is limited to its declared, bound managed agent"); + } + return actor; + } + async function readForPluginReset(target: InstructionTarget, actor: PluginResetActor) { + return db.transaction(async (tx) => { + const state = await lockTarget(tx, target); + await authorizePluginReset(tx, actor, target, state.agent); + const row = await seed(tx, target, state, actor); + return row ? snapshot(row) : null; + }); + } async function head(tx: Tx, target: InstructionTarget, entryFile: string) { const [result] = await tx .select({ revision: revisions }) @@ -142,7 +171,7 @@ export function agentInstructionRevisionService(db: Db) { target: InstructionTarget, entryFile: string, bytes: Buffer, - actor: AuthorizationActor, + actor: RevisionActor, source: AgentInstructionSource, parentRevisionId: string | null, baseRevisionId: string | null, @@ -163,8 +192,8 @@ export function agentInstructionRevisionService(db: Db) { actorAgentId: actor.type === "agent" ? actor.agentId : null, actorUserId: actor.type === "board" ? actor.userId : null, responsibleUserId: - actor.type === "board" ? actor.userId : actor.onBehalfOfUserId, - sourceRunId: actor.runId, + actor.type === "board" ? actor.userId : actor.type === "agent" ? actor.onBehalfOfUserId : null, + sourceRunId: actor.type === "plugin" ? null : actor.runId, }) .returning(); await tx @@ -176,16 +205,17 @@ export function agentInstructionRevisionService(db: Db) { }); await tx.insert(activityLog).values({ companyId: target.companyId, - actorType: actor.type === "board" ? "user" : "agent", - actorId: (actor.type === "board" ? actor.userId : actor.agentId)!, + actorType: actor.type === "plugin" ? "plugin" : actor.type === "board" ? "user" : "agent", + actorId: (actor.type === "plugin" ? actor.pluginId : actor.type === "board" ? actor.userId : actor.agentId)!, agentId: actor.type === "agent" ? actor.agentId : null, - runId: actor.runId, + runId: actor.type === "plugin" ? null : actor.runId, responsibleUserId: row.responsibleUserId, action: "agent.instructions_revision_committed", entityType: "agent", entityId: target.agentId, details: { revisionId: row.id, + ...(actor.type === "plugin" ? { pluginKey: actor.pluginKey, managedAgentKey: actor.agentKey } : {}), entryFile, source, parentRevisionId, @@ -200,7 +230,7 @@ export function agentInstructionRevisionService(db: Db) { tx: Tx, target: InstructionTarget, state: Awaited>, - actor: AuthorizationActor, + actor: RevisionActor, ) { const current = await head(tx, target, state.entryFile); if (current) return current; @@ -271,13 +301,15 @@ export function agentInstructionRevisionService(db: Db) { | (Omit & { restoreRevisionId: string; }), - actor: AuthorizationActor, + actor: RevisionActor, ): Promise { instructionPath(input.entryFile); const bytes = "content" in input ? instructionBytes(input.content) : null; const result = await db.transaction(async (tx) => { - const state = await lockTarget(tx, input, actor); - const bound = await authorizeInstructionCommit(tx, actor, state.agent); + const state = await lockTarget(tx, input, actor.type === "plugin" ? undefined : actor); + const bound = actor.type === "plugin" + ? await authorizePluginReset(tx, actor, input, state.agent) + : await authorizeInstructionCommit(tx, actor, state.agent); if (state.entryFile !== input.entryFile) throw conflict( "Configured instruction entry changed; read the current entry and retry", @@ -460,6 +492,9 @@ export function agentInstructionRevisionService(db: Db) { return { readCommittedForRuntime, readCurrent, + readForPluginReset, + commitPluginReset: (input: Omit, actor: PluginResetActor) => + commitInternal({ ...input, source: "api" }, actor), readRevision, history, diff, diff --git a/server/src/services/plugin-managed-agents.ts b/server/src/services/plugin-managed-agents.ts index c3a1bafd437..9700693f6a3 100644 --- a/server/src/services/plugin-managed-agents.ts +++ b/server/src/services/plugin-managed-agents.ts @@ -12,11 +12,12 @@ import type { PluginManagedAgentDeclaration, PluginManagedAgentResolution, } from "@paperclipai/shared"; -import { notFound } from "../errors.js"; +import { conflict, notFound } from "../errors.js"; import { agentService } from "./agents.js"; import { approvalService } from "./approvals.js"; import { logActivity } from "./activity-log.js"; import { agentInstructionsBundleMode, agentInstructionsService } from "./agent-instructions.js"; +import { agentInstructionRevisionService } from "./agent-instruction-revisions.js"; const MANAGED_AGENT_ENTITY_TYPE = "managed_agent"; const DEFAULT_MANAGED_AGENT_ADAPTER_TYPE = "process"; @@ -336,23 +337,41 @@ export function pluginManagedAgentService( const declared = declaredInstructionFiles(declaration, variables); if (!declared) return agent; - const materialized = await instructions.materializeManagedBundle( - agent, - declared.files, - { - entryFile: declared.entryFile, - replaceExisting: materializeOptions.replaceExisting, - clearLegacyPromptTemplate: true, - }, - ); + let adapterConfig: Record; + if (materializeOptions.replaceExisting) { + const revisions = agentInstructionRevisionService(db); + const target = { companyId, agentId: agent.id }; + const actor = { type: "plugin" as const, pluginId: options.pluginId, pluginKey: options.pluginKey, agentKey: declaration.agentKey }; + const baseline = await revisions.readForPluginReset(target, actor); + const receipt = await revisions.commitPluginReset({ ...target, entryFile: declared.entryFile, + baseRevisionId: baseline?.revision.id ?? null, content: declared.files[declared.entryFile] ?? "" }, actor); + if (receipt.materialization === "pending") throw conflict("Instruction revision saved; retry reset to repair its disk copy", { revisionId: receipt.revision.id }); + const refreshed = await agentSvc.getById(agent.id); + if (!refreshed) throw notFound("Managed agent not found"); + for (const [file, content] of Object.entries(declared.files)) { + if (file !== declared.entryFile) await instructions.writeFile(refreshed, file, content); + } + const bundle = await instructions.getBundle(refreshed); + for (const file of bundle.files) { + if (!file.isEntryFile && !file.virtual && !(file.path in declared.files)) await instructions.deleteFile(refreshed, file.path); + } + adapterConfig = { ...refreshed.adapterConfig }; + delete adapterConfig.promptTemplate; + delete adapterConfig.bootstrapPromptTemplate; + } else { + const materialized = await instructions.materializeManagedBundle(agent, declared.files, { + entryFile: declared.entryFile, replaceExisting: false, clearLegacyPromptTemplate: true, + }); + adapterConfig = materialized.adapterConfig; + } const updated = await agentSvc.update(agent.id, { - adapterConfig: materialized.adapterConfig, + adapterConfig, }, { recordRevision: { source: `plugin:${optionsForRevisionSource()}:managed-agent-instructions`, }, }); - return (updated as Agent | null) ?? { ...agent, adapterConfig: materialized.adapterConfig }; + return (updated as Agent | null) ?? { ...agent, adapterConfig }; } async function managedInstructionDefaultDrift( @@ -433,7 +452,7 @@ export function pluginManagedAgentService( spentMonthlyCents: 0, lastHeartbeatAt: null, }) as Agent; - created = await materializeDeclaredInstructions(companyId, created, declaration, { replaceExisting: true }); + created = await materializeDeclaredInstructions(companyId, created, declaration, { replaceExisting: false }); let approvalId: string | null = null; if (requiresApproval) { @@ -596,8 +615,19 @@ export function pluginManagedAgentService( ? reconciled.agent.metadata : {}; const adapterType = await resolveManagedAdapterType(companyId, declaration); + // Reset content through the canonical CAS path before changing defaults. + // A conflict must leave the existing adapter configuration untouched. + const withInstructions = await materializeDeclaredInstructions(companyId, reconciled.agent, declaration, { replaceExisting: true }); + const defaults = declarationPatch(declaration, { adapterType }); + const adapterConfig = { ...defaults.adapterConfig } as Record; + if (declaration.instructions) { + for (const key of ["instructionsBundleMode", "instructionsRootPath", "instructionsEntryFile", "instructionsFilePath"]) { + if (withInstructions.adapterConfig[key] !== undefined) adapterConfig[key] = withInstructions.adapterConfig[key]; + } + } const updated = await agentSvc.update(reconciled.agent.id, { - ...declarationPatch(declaration, { adapterType }), + ...defaults, + adapterConfig, metadata: managedMetadata(options.pluginId, options.pluginKey, declaration, currentMetadata), }, { recordRevision: { @@ -605,7 +635,7 @@ export function pluginManagedAgentService( }, }); if (!updated) throw notFound("Managed agent not found"); - const updatedAgent = await materializeDeclaredInstructions(companyId, updated as Agent, declaration, { replaceExisting: true }); + const updatedAgent = updated as Agent; await upsertBinding(companyId, declaration, updatedAgent.id, {}, adapterType); await logActivity(db, { companyId,