diff --git a/.changeset/attribution-2881.md b/.changeset/attribution-2881.md new file mode 100644 index 0000000000..7683ed4f61 --- /dev/null +++ b/.changeset/attribution-2881.md @@ -0,0 +1,10 @@ +--- +"emdash": patch +"@emdash-cms/plugin-audit-log": patch +--- + +Fixes content attribution for authenticated REST, visual editing, and MCP saves. + +- Revisions record the acting user without changing the entry owner. MCP updates preserve the existing owner, and actorless internal writes leave revision attribution unset instead of inferring it from ownership. +- `content:beforeSave` and `content:afterSave` receive an actor snapshot with the authenticated user's `id` and `role`. The snapshot is isolated between hooks so one plugin cannot change the attribution seen by another. +- The audit-log plugin stores the actor ID as `userId` on content create and update entries. diff --git a/docs/src/content/docs/plugins/creating-plugins/hooks.mdx b/docs/src/content/docs/plugins/creating-plugins/hooks.mdx index ed6c4a09dd..4247410c48 100644 --- a/docs/src/content/docs/plugins/creating-plugins/hooks.mdx +++ b/docs/src/content/docs/plugins/creating-plugins/hooks.mdx @@ -187,7 +187,7 @@ Do not put HTML in `reason`. The admin renders the value as text. From the host process, throw `ContentSaveRejectedError` (exported from `emdash`) instead. The API returns `SAVE_REJECTED` with your message. Any other exception from either execution mode fails the save with a generic `CONTENT_HOOK_ERROR` response. -**Event:** `{ content, collection, isNew, id }` — **Returns:** modified content, a sandbox hook error result, or `void`. On an update, `id` is the ID of the existing item and `content` holds only the submitted field values; load the stored item with `ctx.content.get(event.collection, event.id)`. +**Event:** `{ content, collection, isNew, id, actor }` — **Returns:** modified content, a sandbox hook error result, or `void`. On an update, `id` is the ID of the existing item and `content` holds only the submitted field values; load the stored item with `ctx.content.get(event.collection, event.id)`. Authenticated REST, visual editing, and MCP saves include `actor.id` and the numeric `actor.role`. Internal writes without an authenticated user omit `actor`. ### `content:afterSave` @@ -196,7 +196,9 @@ Runs after content is successfully saved. Use for side effects like notification ```typescript "content:afterSave": async (event, ctx) => { const contentId = String(event.content.id); - ctx.log.info(`${event.isNew ? "Created" : "Updated"} ${event.collection}/${contentId}`); + ctx.log.info(`${event.isNew ? "Created" : "Updated"} ${event.collection}/${contentId}`, { + actorId: event.actor?.id, + }); if (ctx.http) { await ctx.http.fetch("https://api.example.com/webhook", { @@ -207,7 +209,7 @@ Runs after content is successfully saved. Use for side effects like notification }, ``` -**Event:** `{ content, collection, isNew }` — **Returns:** `Promise` +**Event:** `{ content, collection, isNew, actor }` — **Returns:** `Promise`. Authenticated saves include the same optional `actor` snapshot as `content:beforeSave`. ### `content:beforeDelete` diff --git a/docs/src/content/docs/reference/hooks.mdx b/docs/src/content/docs/reference/hooks.mdx index 20d91fdae7..2b81351182 100644 --- a/docs/src/content/docs/reference/hooks.mdx +++ b/docs/src/content/docs/reference/hooks.mdx @@ -75,15 +75,21 @@ export default definePlugin({ #### Event ```ts +interface ActorInfo { + readonly id: string; + readonly role: number; +} + interface ContentHookEvent { content: Record; // Content data collection: string; // Collection slug isNew: boolean; // True for creates, false for updates id?: string; // ID of the existing item on updates; absent on creates + actor?: ActorInfo; // Authenticated user that initiated the save } ``` -On an update, `content` holds only the submitted field values. Load the stored item with `ctx.content.get(event.collection, event.id)` when the hook needs to compare against it. +On an update, `content` holds only the submitted field values. Load the stored item with `ctx.content.get(event.collection, event.id)` when the hook needs to compare against it. Authenticated REST, visual editing, and MCP saves include `actor`. Internal writes without an authenticated user omit it. #### Return value @@ -130,7 +136,7 @@ hooks: { #### Event -`content:afterSave` receives `content`, `collection`, and `isNew`. `content` is the complete saved entry, with its database ID in `content.id` and collection fields under `content.data`. The separate optional `id` used by `content:beforeSave` updates is absent after the save. +`content:afterSave` receives `content`, `collection`, `isNew`, and the optional authenticated `actor`. `content` is the complete saved entry, with its database ID in `content.id` and collection fields under `content.data`. The separate optional `id` used by `content:beforeSave` updates is absent after the save. #### Return value diff --git a/packages/core/src/astro/routes/api/content/[collection]/[id].ts b/packages/core/src/astro/routes/api/content/[collection]/[id].ts index a31713903d..dd23f74b32 100644 --- a/packages/core/src/astro/routes/api/content/[collection]/[id].ts +++ b/packages/core/src/astro/routes/api/content/[collection]/[id].ts @@ -129,11 +129,14 @@ export const PUT: APIRoute = async ({ params, request, locals, cache }) => { ? body : { ...body, authorId: undefined }; + const actor = user ? { id: user.id, role: user.role } : undefined; + // Pass _rev through for optimistic concurrency validation const result = await emdash.handleContentUpdate(collection, resolvedId, { ...writeBody, locale, _rev: body._rev, + actor, }); if (!result.success) return unwrapResult(result); diff --git a/packages/core/src/astro/routes/api/content/[collection]/index.ts b/packages/core/src/astro/routes/api/content/[collection]/index.ts index 187b8e693f..adbd994237 100644 --- a/packages/core/src/astro/routes/api/content/[collection]/index.ts +++ b/packages/core/src/astro/routes/api/content/[collection]/index.ts @@ -79,12 +79,15 @@ export const POST: APIRoute = async ({ params, request, locals, cache }) => { ); } + const actor = user ? { id: user.id, role: user.role } : undefined; + // Auto-set authorId to current user when creating content const result = await emdash.handleContentCreate(collection, { ...body, authorId: user?.id, locale: body.locale, translationOf: body.translationOf, + actor, }); if (!result.success) return unwrapResult(result); diff --git a/packages/core/src/astro/types.ts b/packages/core/src/astro/types.ts index e94ba6c939..e24217f303 100644 --- a/packages/core/src/astro/types.ts +++ b/packages/core/src/astro/types.ts @@ -298,6 +298,7 @@ export interface EmDashHandlers { taxonomies?: Record; createdAt?: string | null; publishedAt?: string | null; + actor?: { id: string; role: number }; }, ) => Promise; @@ -321,6 +322,7 @@ export interface EmDashHandlers { taxonomies?: Record; publishedAt?: string | null; _rev?: string; + actor?: { id: string; role: number }; }, ) => Promise; diff --git a/packages/core/src/database/repositories/content.ts b/packages/core/src/database/repositories/content.ts index 2699c2a919..8d44967c40 100644 --- a/packages/core/src/database/repositories/content.ts +++ b/packages/core/src/database/repositories/content.ts @@ -973,7 +973,7 @@ export class ContentRepository { collection: type, entryId: id, data: mergedData, - ...(input.authorId ? { authorId: input.authorId } : {}), + ...(input.revisionAuthorId ? { authorId: input.revisionAuthorId } : {}), }); let staged: boolean; diff --git a/packages/core/src/database/repositories/types.ts b/packages/core/src/database/repositories/types.ts index fe7f0310ce..8bb5d0cdf3 100644 --- a/packages/core/src/database/repositories/types.ts +++ b/packages/core/src/database/repositories/types.ts @@ -34,7 +34,10 @@ export interface UpdateContentInput { slug?: string | null; publishedAt?: string | null; scheduledAt?: string | null; + /** Entry owner (`ec_{collection}.author_id`). */ authorId?: string | null; + /** Revision author, separate from entry ownership. */ + revisionAuthorId?: string | null; primaryBylineId?: string | null; } diff --git a/packages/core/src/emdash-runtime.ts b/packages/core/src/emdash-runtime.ts index ba1a678bab..50a35fad0f 100644 --- a/packages/core/src/emdash-runtime.ts +++ b/packages/core/src/emdash-runtime.ts @@ -68,6 +68,7 @@ import type { SandboxRunnerFactory, } from "./plugins/sandbox/types.js"; import type { + ActorInfo, ContentHookEvent, ResolvedPlugin, MediaItem, @@ -233,7 +234,7 @@ import { publishDueContent, type PublishedRef } from "./scheduled-publish.js"; import { FTSManager } from "./search/fts-manager.js"; import { invalidateSiteSettingsCache } from "./settings/index.js"; -const DRAFT_ONLY_UPDATE_KEYS = new Set(["data", "slug", "locale", "skipRevision"]); +const DRAFT_ONLY_UPDATE_KEYS = new Set(["data", "slug", "locale", "skipRevision", "actor"]); const MAX_DRAFT_STAGE_ATTEMPTS = 32; /** @@ -2798,13 +2799,22 @@ export class EmDashRuntime { locale?: string; translationOf?: string; taxonomies?: Record; + actor?: ActorInfo; }, ) { + const actor = body.actor ? { ...body.actor } : undefined; + // Run beforeSave hooks (trusted plugins) let processedData = body.data; if (this.hooks.hasHooks("content:beforeSave")) { try { - const hookResult = await this.hooks.runContentBeforeSave(body.data, collection, true); + const hookResult = await this.hooks.runContentBeforeSave( + body.data, + collection, + true, + undefined, + actor, + ); processedData = hookResult.content; } catch (error) { return beforeSaveFailure(error); @@ -2812,7 +2822,13 @@ export class EmDashRuntime { } // Run beforeSave hooks (sandboxed plugins) - const sandboxResult = await this.runSandboxedBeforeSave(processedData, collection, true); + const sandboxResult = await this.runSandboxedBeforeSave( + processedData, + collection, + true, + undefined, + actor, + ); if (!sandboxResult.success) return sandboxResult; processedData = sandboxResult.data; @@ -2846,7 +2862,7 @@ export class EmDashRuntime { // Run afterSave hooks (fire-and-forget) if (result.success && result.data) { - this.runAfterSaveHooks(contentItemToRecord(result.data.item), collection, true); + this.runAfterSaveHooks(contentItemToRecord(result.data.item), collection, true, actor); } return result; @@ -2874,8 +2890,15 @@ export class EmDashRuntime { /** Replace the previous autosave revision after staging this save. */ skipRevision?: boolean; _rev?: string; + /** + * Acting user for this save. Used for revision attribution and + * passed to content hooks; never changes entry ownership. + */ + actor?: ActorInfo; }, ) { + const actor = body.actor ? { ...body.actor } : undefined; + // Resolve slug → ID if needed (before any lookups) const repo = new ContentRepository(this.db); const resolvedItem = await repo.findByIdOrSlug(collection, id, body.locale); @@ -2899,7 +2922,7 @@ export class EmDashRuntime { }; } } - const { _rev: _discardedRev, ...bodyWithoutRev } = body; + const { _rev: _discardedRev, actor: _discardedActor, ...bodyWithoutRev } = body; // Run beforeSave hooks if data is provided let processedData = bodyWithoutRev.data; @@ -2911,6 +2934,7 @@ export class EmDashRuntime { collection, false, resolvedItem?.id, + actor, ); processedData = hookResult.content; } catch (error) { @@ -2924,6 +2948,7 @@ export class EmDashRuntime { collection, false, resolvedItem?.id, + actor, ); if (!sandboxResult.success) return sandboxResult; processedData = sandboxResult.data; @@ -2975,7 +3000,7 @@ export class EmDashRuntime { collection, entryId: resolvedId, data: mergedData, - authorId: bodyWithoutRev.authorId ?? undefined, + authorId: actor?.id, }); let staged: boolean; @@ -3113,7 +3138,7 @@ export class EmDashRuntime { // Run afterSave hooks (fire-and-forget) if (hydrated.success && hydrated.data) { - this.runAfterSaveHooks(contentItemToRecord(hydrated.data.item), collection, false); + this.runAfterSaveHooks(contentItemToRecord(hydrated.data.item), collection, false, actor); } if (hydrated.success) { @@ -3984,6 +4009,7 @@ export class EmDashRuntime { collection: string, isNew: boolean, contentId?: string, + actor?: ActorInfo, ) { let result = content; @@ -3994,6 +4020,7 @@ export class EmDashRuntime { try { const event: ContentHookEvent = { content: result, collection, isNew }; if (contentId !== undefined) event.id = contentId; + if (actor !== undefined) event.actor = { ...actor }; const hookResult = await plugin.invokeHook("content:beforeSave", event); const inspection = inspectSandboxHookResult(hookResult); if (inspection.kind === "error") { @@ -4064,12 +4091,13 @@ export class EmDashRuntime { content: Record, collection: string, isNew: boolean, + actor?: ActorInfo, ): void { after(async () => { // Trusted plugins if (this.hooks.hasHooks("content:afterSave")) { try { - await this.hooks.runContentAfterSave(content, collection, isNew); + await this.hooks.runContentAfterSave(content, collection, isNew, actor); } catch (err) { console.error("EmDash afterSave hook error:", err); } @@ -4084,7 +4112,9 @@ export class EmDashRuntime { tasks.push( (async () => { try { - await plugin.invokeHook("content:afterSave", { content, collection, isNew }); + const event: ContentHookEvent = { content, collection, isNew }; + if (actor !== undefined) event.actor = { ...actor }; + await plugin.invokeHook("content:afterSave", event); } catch (err) { console.error(`EmDash: Sandboxed plugin ${id} afterSave error:`, err); } diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 26d5cdf06c..09f0e7c40c 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -297,6 +297,7 @@ export type { HookName, ResolvedHook, ResolvedPluginHooks, + ActorInfo, ContentHookEvent, ContentDeleteEvent, ContentPublishStateChangeEvent, diff --git a/packages/core/src/mcp/server.ts b/packages/core/src/mcp/server.ts index b82a937016..3754023485 100644 --- a/packages/core/src/mcp/server.ts +++ b/packages/core/src/mcp/server.ts @@ -975,7 +975,8 @@ export function createMcpServer( async (args, extra) => { requireScope(extra, "content:write"); requireRole(extra, Role.CONTRIBUTOR); - const { emdash, userId } = getExtra(extra); + const { emdash, userId, userRole } = getExtra(extra); + const actor = { id: userId, role: userRole }; // Creating a translation requires edit permission on the source item if (args.translationOf) { @@ -993,7 +994,7 @@ export function createMcpServer( // Publishing requires publish permission — create as draft then publish if (args.status === "published") { - const user = { id: userId, role: getExtra(extra).userRole }; + const user = { id: userId, role: userRole }; if (!hasPermission(user, "content:publish_own")) { throw new EmDashAuthError( "Insufficient permissions: publishing requires content:publish_own", @@ -1008,6 +1009,7 @@ export function createMcpServer( translationOf: args.translationOf, bylines: args.bylines, taxonomies: args.taxonomies, + actor, }); if (!result.success) return unwrap(result); const itemId = extractContentId(result.data); @@ -1026,6 +1028,7 @@ export function createMcpServer( translationOf: args.translationOf, bylines: args.bylines, taxonomies: args.taxonomies, + actor, }), ); }, @@ -1103,6 +1106,7 @@ export function createMcpServer( requireScope(extra, "content:write"); requireRole(extra, Role.AUTHOR); const { emdash, userId, userRole } = getExtra(extra); + const actor = { id: userId, role: userRole }; // Fetch item to check ownership const existing = await emdash.handleContentGet(args.collection, args.id, args.locale); @@ -1146,7 +1150,7 @@ export function createMcpServer( const updateResult = await emdash.handleContentUpdate(args.collection, resolvedId, { data, slug: args.slug, - authorId: userId, + actor, locale: args.locale, seo: args.seo, bylines: args.bylines, @@ -1176,7 +1180,7 @@ export function createMcpServer( const updateResult = await emdash.handleContentUpdate(args.collection, resolvedId, { data, slug: args.slug, - authorId: userId, + actor, locale: args.locale, seo: args.seo, bylines: args.bylines, @@ -1196,7 +1200,7 @@ export function createMcpServer( await emdash.handleContentUpdate(args.collection, resolvedId, { data, slug: args.slug, - authorId: userId, + actor, locale: args.locale, seo: args.seo, bylines: args.bylines, diff --git a/packages/core/src/plugin-types.ts b/packages/core/src/plugin-types.ts index d6a3eaf5aa..814b5dab17 100644 --- a/packages/core/src/plugin-types.ts +++ b/packages/core/src/plugin-types.ts @@ -43,6 +43,7 @@ import type { ZodType } from "zod"; import type { SandboxHookErrorEnvelope } from "./plugins/sandbox/hook-result.js"; import type { + ActorInfo, CommentAfterCreateEvent, CommentAfterCreateHandler, CommentAfterModerateEvent, @@ -272,6 +273,7 @@ export type { SandboxHookErrorEnvelope }; * portable `.d.mts`. */ export type { + ActorInfo, CommentAfterCreateEvent, CommentAfterModerateEvent, CommentBeforeCreateEvent, diff --git a/packages/core/src/plugins/hooks.ts b/packages/core/src/plugins/hooks.ts index 8004a154ba..f7208884b9 100644 --- a/packages/core/src/plugins/hooks.ts +++ b/packages/core/src/plugins/hooks.ts @@ -15,6 +15,7 @@ import type { ResolvedPlugin, ResolvedHook, PluginContext, + ActorInfo, ContentHookEvent, ContentDeleteEvent, ContentStateChangeEvent, @@ -488,13 +489,15 @@ export class HookPipeline { /** * Run content:beforeSave hooks * Returns modified content from the pipeline. `id` is the existing item's - * ID when updating. + * ID when updating. `actor` is the authenticated user that triggered the + * save, when one is available. */ async runContentBeforeSave( content: Record, collection: string, isNew: boolean, id?: string, + actor?: ActorInfo, ): Promise<{ content: Record; results: HookResult>[]; @@ -511,6 +514,7 @@ export class HookPipeline { isNew, }; if (id !== undefined) event.id = id; + if (actor !== undefined) event.actor = { ...actor }; const ctx = this.getContext(hook.pluginId); const start = Date.now(); @@ -550,6 +554,7 @@ export class HookPipeline { content: Record, collection: string, isNew: boolean, + actor?: ActorInfo, ): Promise[]> { const hooks = this.getTypedHooks("content:afterSave"); const results: HookResult[] = []; @@ -557,6 +562,7 @@ export class HookPipeline { for (const hook of hooks) { const { handler } = hook; const event: ContentHookEvent = { content, collection, isNew }; + if (actor !== undefined) event.actor = { ...actor }; const ctx = this.getContext(hook.pluginId); const start = Date.now(); diff --git a/packages/core/src/plugins/index.ts b/packages/core/src/plugins/index.ts index 5e327a2d82..d40cd96eab 100644 --- a/packages/core/src/plugins/index.ts +++ b/packages/core/src/plugins/index.ts @@ -145,6 +145,7 @@ export type { HookName, ResolvedHook, ResolvedPluginHooks, + ActorInfo, ContentHookEvent, ContentDeleteEvent, ContentPublishStateChangeEvent, diff --git a/packages/core/src/plugins/manager.ts b/packages/core/src/plugins/manager.ts index 0853a29d4b..307c687109 100644 --- a/packages/core/src/plugins/manager.ts +++ b/packages/core/src/plugins/manager.ts @@ -26,6 +26,7 @@ import { } from "./hooks.js"; import { PluginRouteRegistry, type RouteResult, type InvokeRouteOptions } from "./routes.js"; import type { + ActorInfo, PluginDefinition, ResolvedPlugin, PluginStorageConfig, @@ -306,12 +307,13 @@ export class PluginManager { collection: string, isNew: boolean, id?: string, + actor?: ActorInfo, ): Promise<{ content: Record; results: HookResult>[]; }> { this.ensureInitialized(); - return this.hookPipeline!.runContentBeforeSave(content, collection, isNew, id); + return this.hookPipeline!.runContentBeforeSave(content, collection, isNew, id, actor); } /** @@ -321,9 +323,10 @@ export class PluginManager { content: Record, collection: string, isNew: boolean, + actor?: ActorInfo, ): Promise[]> { this.ensureInitialized(); - return this.hookPipeline!.runContentAfterSave(content, collection, isNew); + return this.hookPipeline!.runContentAfterSave(content, collection, isNew, actor); } /** diff --git a/packages/core/src/plugins/types.ts b/packages/core/src/plugins/types.ts index 05f3f490e9..590dc3192e 100644 --- a/packages/core/src/plugins/types.ts +++ b/packages/core/src/plugins/types.ts @@ -804,6 +804,15 @@ export interface HookConfig { handler: THandler; } +/** + * Acting user that triggered a content hook. Present for authenticated + * saves; absent for unauthenticated or internal writes. + */ +export interface ActorInfo { + readonly id: string; + readonly role: number; +} + /** * Content hook event */ @@ -817,6 +826,12 @@ export interface ContentHookEvent { * `content:afterSave`, where `content.id` carries it. */ id?: string; + /** + * The acting user for this save. Carries the same authenticated identity + * used to set the revision author, so plugins (e.g. audit logs) can record + * who made the change. + */ + actor?: ActorInfo; } /** diff --git a/packages/core/tests/integration/content/attribution-2881.test.ts b/packages/core/tests/integration/content/attribution-2881.test.ts new file mode 100644 index 0000000000..d9545c6a32 --- /dev/null +++ b/packages/core/tests/integration/content/attribution-2881.test.ts @@ -0,0 +1,386 @@ +/** + * Content attribution keeps the acting user separate from entry ownership. + * Authenticated revisions and save hooks receive the actor, while owner + * changes affect only the content row. + */ + +import { randomUUID } from "node:crypto"; + +import { Role } from "@emdash-cms/auth"; +import Database from "better-sqlite3"; +import { SqliteDialect } from "kysely"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import { ContentRepository } from "../../../src/database/repositories/content.js"; +import { RevisionRepository } from "../../../src/database/repositories/revision.js"; +import { EmDashRuntime } from "../../../src/emdash-runtime.js"; +import type { RuntimeDependencies } from "../../../src/emdash-runtime.js"; +import { definePlugin } from "../../../src/plugins/define-plugin.js"; +import type { SandboxedPluginInstance } from "../../../src/plugins/sandbox/types.js"; +import type { ContentBeforeSaveHandler, ContentHookEvent } from "../../../src/plugins/types.js"; +import { SchemaRegistry } from "../../../src/schema/registry.js"; +import { createTestRuntime } from "../../utils/mcp-runtime.js"; +import { setupTestDatabase, teardownTestDatabase } from "../../utils/test-db.js"; + +const deferred: Array<() => void | Promise> = []; + +vi.mock("../../../src/after.js", () => ({ + after: (fn: () => void | Promise) => { + deferred.push(fn); + }, +})); + +const actorA = { id: "user_a", role: Role.AUTHOR }; +const actorB = { id: "user_b", role: Role.EDITOR }; + +async function flushDeferred(): Promise { + const tasks = deferred.splice(0); + for (const task of tasks) await task(); +} + +async function createPostCollection(registry: SchemaRegistry): Promise { + await registry.createCollection({ slug: "posts", label: "Posts" }); + await registry.createField("posts", { slug: "title", label: "Title", type: "string" }); +} + +describe("revision attribution", () => { + let db: ReturnType extends Promise ? T : never; + let runtime: EmDashRuntime; + + beforeEach(async () => { + deferred.length = 0; + db = await setupTestDatabase(); + const registry = new SchemaRegistry(db); + await createPostCollection(registry); + runtime = createTestRuntime(db); + }); + + afterEach(async () => { + vi.restoreAllMocks(); + await teardownTestDatabase(db); + }); + + it("attributes a draft revision to the acting user, not NULL", async () => { + const created = await runtime.handleContentCreate("posts", { + data: { title: "Draft" }, + slug: "draft-post", + authorId: "owner_1", + }); + expect(created.success).toBe(true); + const id = created.data!.item.id; + + const saved = await runtime.handleContentUpdate("posts", id, { + data: { title: "Draft edited" }, + actor: actorA, + }); + expect(saved.success).toBe(true); + + const revisionRepo = new RevisionRepository(db); + const latest = await revisionRepo.findLatest("posts", id); + expect(latest?.authorId).toBe(actorA.id); + }); + + it("does not reassign entry ownership when only a revision author is supplied", async () => { + const created = await runtime.handleContentCreate("posts", { + data: { title: "Owned" }, + slug: "owned-post", + authorId: "owner_1", + }); + const id = created.data!.item.id; + + const saved = await runtime.handleContentUpdate("posts", id, { + data: { title: "Owned edited" }, + actor: actorA, + }); + expect(saved.success).toBe(true); + expect(saved.success && saved.liveContentChanged).toBe(false); + + const repo = new ContentRepository(db); + const item = await repo.findById("posts", id); + expect(item?.authorId).toBe("owner_1"); + }); + + it("allows explicit ownership changes without conflating revision author", async () => { + const created = await runtime.handleContentCreate("posts", { + data: { title: "Explicit" }, + slug: "explicit-post", + authorId: "owner_1", + }); + const id = created.data!.item.id; + + const saved = await runtime.handleContentUpdate("posts", id, { + data: { title: "Explicit edited" }, + authorId: "owner_2", + actor: actorB, + }); + expect(saved.success).toBe(true); + + const repo = new ContentRepository(db); + const item = await repo.findById("posts", id); + expect(item?.authorId).toBe("owner_2"); + + const revisionRepo = new RevisionRepository(db); + const latest = await revisionRepo.findLatest("posts", id); + expect(latest?.authorId).toBe(actorB.id); + }); + + it("does not attribute an actorless revision to a new owner", async () => { + const created = await runtime.handleContentCreate("posts", { + data: { title: "Explicit" }, + slug: "actorless-owner-change", + authorId: "owner_1", + }); + const id = created.data!.item.id; + + const saved = await runtime.handleContentUpdate("posts", id, { + data: { title: "Explicit edited" }, + authorId: "owner_2", + }); + expect(saved.success).toBe(true); + + const item = await new ContentRepository(db).findById("posts", id); + expect(item?.authorId).toBe("owner_2"); + + const latest = await new RevisionRepository(db).findLatest("posts", id); + expect(latest?.authorId).toBeNull(); + }); +}); + +describe("hook actor payloads", () => { + const beforeEvents: ContentHookEvent[] = []; + const afterEvents: ContentHookEvent[] = []; + let mutateBeforeSaveActor = false; + + let sqlite: Database.Database; + let runtime: EmDashRuntime; + let repo: ContentRepository; + + function createTrustedDeps(): RuntimeDependencies { + return { + config: { + database: { + entrypoint: `test-actor-hooks-${randomUUID()}`, + config: {}, + type: "sqlite", + }, + }, + plugins: [ + definePlugin({ + id: "actor-probe", + version: "1.0.0", + capabilities: ["content:write", "content:read"], + hooks: { + "content:beforeSave": { + handler: (async (event) => { + const mutableActor = event.actor as { id: string; role: number } | undefined; + if (mutateBeforeSaveActor && mutableActor) { + mutableActor.id = "spoofed_by_hook"; + } + beforeEvents.push(event); + }) as ContentBeforeSaveHandler, + }, + "content:afterSave": { + handler: async (event) => { + afterEvents.push(event); + }, + }, + }, + }), + ], + createDialect: () => new SqliteDialect({ database: sqlite }), + createStorage: null, + sandboxEnabled: false, + sandboxedPluginEntries: [], + createSandboxRunner: null, + }; + } + + beforeEach(async () => { + deferred.length = 0; + beforeEvents.length = 0; + afterEvents.length = 0; + mutateBeforeSaveActor = false; + sqlite = new Database(":memory:"); + runtime = await EmDashRuntime.create(createTrustedDeps()); + const registry = new SchemaRegistry(runtime.db); + await createPostCollection(registry); + repo = new ContentRepository(runtime.db); + }); + + afterEach(async () => { + await runtime.stopCron(); + vi.restoreAllMocks(); + }); + + it("passes actor to content:beforeSave / content:afterSave on create", async () => { + const result = await runtime.handleContentCreate("posts", { + data: { title: "Created" }, + actor: actorA, + }); + expect(result.success).toBe(true); + + expect(beforeEvents).toHaveLength(1); + expect(beforeEvents[0]).toMatchObject({ + collection: "posts", + isNew: true, + actor: actorA, + }); + expect(beforeEvents[0]?.id).toBeUndefined(); + + await flushDeferred(); + + expect(afterEvents).toHaveLength(1); + expect(afterEvents[0]).toMatchObject({ + collection: "posts", + isNew: true, + actor: actorA, + }); + expect(afterEvents[0]?.content.id).toBe(result.data?.item.id); + }); + + it("passes actor and item id to content:beforeSave / content:afterSave on update", async () => { + const item = await repo.create({ type: "posts", data: { title: "Original" } }); + + const result = await runtime.handleContentUpdate("posts", item.id, { + data: { title: "Changed" }, + actor: actorB, + }); + expect(result.success).toBe(true); + + expect(beforeEvents).toEqual([ + { + content: { title: "Changed" }, + collection: "posts", + isNew: false, + id: item.id, + actor: actorB, + }, + ]); + + await flushDeferred(); + + expect(afterEvents).toHaveLength(1); + expect(afterEvents[0]).toMatchObject({ + collection: "posts", + isNew: false, + actor: actorB, + }); + expect(afterEvents[0]?.content.id).toBe(item.id); + }); + + it("keeps revision and downstream hook attribution stable when a hook mutates its event", async () => { + const item = await repo.create({ type: "posts", data: { title: "Original" } }); + const actor = { id: "authenticated_user", role: Role.EDITOR }; + mutateBeforeSaveActor = true; + + const result = await runtime.handleContentUpdate("posts", item.id, { + data: { title: "Changed" }, + actor, + }); + expect(result.success).toBe(true); + + const latest = await new RevisionRepository(runtime.db).findLatest("posts", item.id); + expect(beforeEvents[0]?.actor?.id).toBe("spoofed_by_hook"); + expect(latest?.authorId).toBe("authenticated_user"); + expect(actor.id).toBe("authenticated_user"); + + await flushDeferred(); + expect(afterEvents[0]?.actor).toEqual(actor); + }); +}); + +describe("sandboxed hook actor payloads", () => { + const invokeHook = vi.fn(); + + let sqlite: Database.Database; + let runtime: EmDashRuntime; + let repo: ContentRepository; + + function createSandboxedDeps(): RuntimeDependencies { + const runner = { + isAvailable: () => true, + isHealthy: () => true, + load: vi.fn().mockResolvedValue({ + id: "actor-sandboxed:1.0.0", + invokeHook, + invokeRoute: vi.fn(), + terminate: vi.fn(), + }), + setEmailSend: vi.fn(), + terminateAll: vi.fn(), + }; + return { + config: { + database: { + entrypoint: `test-sandboxed-actor-${randomUUID()}`, + config: {}, + type: "sqlite", + }, + }, + plugins: [], + createDialect: () => new SqliteDialect({ database: sqlite }), + createStorage: null, + sandboxEnabled: true, + sandboxedPluginEntries: [ + { + id: "actor-sandboxed", + version: "1.0.0", + options: {}, + code: "", + capabilities: ["content:read", "content:write"], + allowedHosts: [], + storage: {}, + }, + ], + createSandboxRunner: (() => runner) as unknown as RuntimeDependencies["createSandboxRunner"], + }; + } + + beforeEach(async () => { + invokeHook.mockReset(); + invokeHook.mockResolvedValue(undefined); + sqlite = new Database(":memory:"); + runtime = await EmDashRuntime.create(createSandboxedDeps()); + const registry = new SchemaRegistry(runtime.db); + await createPostCollection(registry); + repo = new ContentRepository(runtime.db); + }); + + afterEach(async () => { + await runtime.stopCron(); + vi.restoreAllMocks(); + }); + + it("passes actor to sandboxed content:beforeSave on create", async () => { + const result = await runtime.handleContentCreate("posts", { + data: { title: "Sandbox create" }, + actor: actorA, + }); + expect(result.success).toBe(true); + + expect(invokeHook).toHaveBeenCalledWith("content:beforeSave", { + content: { title: "Sandbox create" }, + collection: "posts", + isNew: true, + actor: actorA, + }); + }); + + it("passes actor and id to sandboxed content:beforeSave on update", async () => { + const item = await repo.create({ type: "posts", data: { title: "Original" } }); + + const result = await runtime.handleContentUpdate("posts", item.id, { + data: { title: "Changed" }, + actor: actorB, + }); + expect(result.success).toBe(true); + + expect(invokeHook).toHaveBeenCalledWith("content:beforeSave", { + content: { title: "Changed" }, + collection: "posts", + isNew: false, + id: item.id, + actor: actorB, + }); + }); +}); diff --git a/packages/core/tests/integration/plugins/audit-log-plugin.test.ts b/packages/core/tests/integration/plugins/audit-log-plugin.test.ts index 9dce351bb7..ca29647719 100644 --- a/packages/core/tests/integration/plugins/audit-log-plugin.test.ts +++ b/packages/core/tests/integration/plugins/audit-log-plugin.test.ts @@ -7,6 +7,7 @@ import { readFileSync } from "node:fs"; +import { Role } from "@emdash-cms/auth"; import { parse as parseJsonc } from "jsonc-parser"; import type { Kysely } from "kysely"; import { afterEach, beforeEach, describe, expect, it, vi, type MockInstance } from "vitest"; @@ -34,6 +35,7 @@ interface AuditLogManifest { interface AuditEntry { action: string; resourceId: string; + userId?: string; changes?: { before?: Record; after?: Record }; } @@ -143,6 +145,7 @@ describe("audit-log plugin", () => { const updated = await runtime.handleContentUpdate("post", created.data.item.id, { data: { title: "After" }, + actor: { id: "editor-user", role: Role.EDITOR }, }); expect(updated.success).toBe(true); await waitForDeferredTasks(); @@ -150,6 +153,7 @@ describe("audit-log plugin", () => { const entry = (await readEntries()).find((e) => e.action === "update"); expect(entry).toBeDefined(); expect(entry?.resourceId).toBe(created.data.item.id); + expect(entry?.userId).toBe("editor-user"); expect(entry?.changes?.before).toEqual({ title: "Before" }); expect(entry?.changes?.after).toEqual({ title: "After" }); }); diff --git a/packages/core/tests/unit/plugins/manager.test.ts b/packages/core/tests/unit/plugins/manager.test.ts index eec619d0b6..8ea7e7f7a3 100644 --- a/packages/core/tests/unit/plugins/manager.test.ts +++ b/packages/core/tests/unit/plugins/manager.test.ts @@ -14,8 +14,10 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { runMigrations } from "../../../src/database/migrations/runner.js"; import type { Database as DbSchema } from "../../../src/database/types.js"; +import type { ActorInfo as RootActorInfo } from "../../../src/index.js"; +import type { ActorInfo as PluginActorInfo } from "../../../src/plugin-types.js"; import { PluginManager, createPluginManager } from "../../../src/plugins/manager.js"; -import type { PluginDefinition } from "../../../src/plugins/types.js"; +import type { ContentHookEvent, PluginDefinition } from "../../../src/plugins/types.js"; // Test error message regex patterns const ALREADY_REGISTERED_REGEX = /already registered/; @@ -268,6 +270,43 @@ describe("PluginManager", () => { }); }); + describe("content hook dispatch", () => { + it("forwards the actor through the public manager facade", async () => { + const beforeSave = vi.fn(async (event: ContentHookEvent) => event.content); + const afterSave = vi.fn(async (_event: ContentHookEvent) => {}); + manager.register( + createTestDefinition({ + id: "actor-observer", + capabilities: ["content:write"], + hooks: { + "content:beforeSave": beforeSave, + "content:afterSave": afterSave, + }, + }), + ); + await manager.activate("actor-observer"); + const actor: RootActorInfo = { + id: "editor-user", + role: 40, + }; + const pluginActor: PluginActorInfo = actor; + + await manager.runContentBeforeSave({ title: "Draft" }, "posts", false, "post-1", pluginActor); + await manager.runContentAfterSave( + { id: "post-1", data: { title: "Draft" } }, + "posts", + false, + pluginActor, + ); + + expect(beforeSave).toHaveBeenCalledWith( + expect.objectContaining({ id: "post-1", actor }), + expect.anything(), + ); + expect(afterSave).toHaveBeenCalledWith(expect.objectContaining({ actor }), expect.anything()); + }); + }); + describe("getPluginState", () => { it("returns undefined for non-existent plugin", () => { expect(manager.getPluginState("non-existent")).toBeUndefined(); diff --git a/packages/plugins/audit-log/src/plugin.ts b/packages/plugins/audit-log/src/plugin.ts index d6c4bfd980..98d8b29370 100644 --- a/packages/plugins/audit-log/src/plugin.ts +++ b/packages/plugins/audit-log/src/plugin.ts @@ -122,6 +122,7 @@ export default { collection: event.collection, resourceId: contentId, resourceType: "content", + ...(event.actor ? { userId: event.actor.id } : {}), changes: beforeRecord || afterRecord ? { before: beforeRecord, after: afterRecord } : undefined, metadata: { slug: event.content.slug, status: event.content.status }, diff --git a/skills/creating-plugins/references/hooks.md b/skills/creating-plugins/references/hooks.md index 1619e20ec2..b116c7cfb1 100644 --- a/skills/creating-plugins/references/hooks.md +++ b/skills/creating-plugins/references/hooks.md @@ -118,7 +118,7 @@ Runs before save. Return modified content, or void to keep it unchanged. To reje } ``` -Event: `{ content: Record, collection: string, isNew: boolean }` +Event: `{ content: Record, collection: string, isNew: boolean, id?: string, actor?: { id: string, role: number } }`. Authenticated REST, visual editing, and MCP saves include a read-only actor snapshot; internal writes may omit it. On updates, `id` identifies the existing item. Returns: `Record | SandboxHookErrorEnvelope | void` ### `content:afterSave` @@ -132,7 +132,7 @@ Runs after successful save. Side effects only — logging, notifications, syncin } ``` -Event: `{ content: Record, collection: string, isNew: boolean }` +Event: `{ content: Record, collection: string, isNew: boolean, actor?: { id: string, role: number } }`. Authenticated saves include a read-only actor snapshot; internal writes may omit it. Returns: `void` ### `content:beforeDelete`