feat(core): refuse MCP content writes to an entry someone else is editing - #3059
feat(core): refuse MCP content writes to an entry someone else is editing#3059danielmlr wants to merge 3 commits into
Conversation
…ting
The edit lock refuses REST and CLI writes against another user's lease,
but the MCP content tools call the handlers directly and never reached
that check, so an agent could still write over an entry someone had open
in the admin.
content_update, content_delete, content_publish, content_unpublish,
content_schedule, content_unschedule, content_discard_draft and
revision_restore now run claimEntryLockForWrite after their permission
checks, as the REST routes do, and take an optional overrideLock. A
refusal carries ENTRY_LOCKED and the holder in _meta.details. The _rev
description already tells an agent to re-read and retry on CONFLICT, and
both codes come back from the same tools, so the overrideLock description
says that re-reading never clears ENTRY_LOCKED and that the flag is for
when the user asks to write anyway.
POST /revisions/{revisionId}/restore overwrites the entry as well, but
the edit lock never covered it, which left revision_restore without a
REST check to mirror. The route now runs the same check and reads
overrideLock from an optional body, as /publish and /unpublish do. The
admin's restore button sits inside the editor's read-only fieldset, so
only the lock holder restores from the admin, and the holder passes.
In content_update the publish-permission check for a status change moves
ahead of the lock check, so no tool names a holder to a caller who lacks
permission for the write. Every role that passes the edit check also
passes that one, so only the order changes.
The MCP authorization unit test stubbed the handlers with an empty object
for db, which the lock query cannot run against, so it now gets a
migrated in-memory database.
The unreleased entry-edit-lock changeset drops the sentence saying the
MCP content tools do not honour the lock yet, which would be false in a
release that ships both changes.
🦋 Changeset detectedLatest commit: bb6f089 The changes in this PR will be included in the next version bump. This PR includes changesets to release 17 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Overlapping PRsThis PR modifies files that are also changed by other open PRs:
This may cause merge conflicts or duplicated work. A maintainer will coordinate. |
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/auth-atproto
@emdash-cms/blocks
@emdash-cms/cloudflare
@emdash-cms/contentful-to-portable-text
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/plugin-cli
@emdash-cms/plugin-types
@emdash-cms/registry-client
@emdash-cms/registry-lexicons
@emdash-cms/registry-moderation
@emdash-cms/registry-verification
@emdash-cms/sandbox-workerd
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-atproto
@emdash-cms/plugin-audit-log
@emdash-cms/plugin-color
@emdash-cms/plugin-embeds
@emdash-cms/plugin-field-kit
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
Resolves the conflict in guides/working-with-content.mdx: emdash-cms#3040 rewrote the guide, so the MCP sentence and the mcp-server reference link now sit in the lock paragraph under "Work with another editor".
![emdashbot[bot]](https://avatars.githubusercontent.com/in/3255304?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR adds the missing entry edit-lock checks to MCP content-write tools and the POST /revisions/{revisionId}/restore route, which #2919 left out. The approach is sound: it reuses claimEntryLockForWrite, places the check after permission/ownership gates so unauthorized callers do not learn the lock holder, and adds an overrideLock escape hatch matching the REST API. The corresponding docs and changeset updates are clean.
I checked the changed server code, the revision-restore route, the shared lock handler, the schema additions, the integration/unit tests, and the docs/changeset prose. The lock checks cover content_update, content_delete, content_publish, content_unpublish, content_schedule, content_unschedule, content_discard_draft, and revision_restore in MCP, plus the equivalent REST restore route. Tests exercise refusal, override, own-lease extension (implicitly through success), and permission-before-lock ordering. No logic bugs, regressions, or convention violations were found.
One note: the activate_skill call for writing-emdash-docs could not be satisfied because that skill name is not registered as an active skill in this environment, but I reviewed the documentation changes manually against the implementation. Everything checks out. Clean change.
1 participant
What does this PR do?
Prevents an AI tool connected over MCP from writing over an entry someone else has open in the admin. #2919 made the REST routes refuse a write against another user's edit lock, but the MCP content tools call the handlers directly and skip that check.
content_update,content_delete,content_publish,content_unpublish,content_schedule,content_unschedule,content_discard_draftandrevision_restorenow run the same check after their permission checks and returnENTRY_LOCKEDwith the holder in_meta.details.overrideLock: truewrites anyway. Re-reading never clears the refusal, so the flag's description tells agents to set it only when the user asks.content_restore,content_permanent_deleteand collections with locking off are unchanged.POST /revisions/{revisionId}/restoreoverwrites the entry too, but #2919 did not cover it, so it now runs the same check and takesoverrideLockin an optional body. The admin's restore button sits inside the editor's read-only fieldset, so only the lock holder restores from the admin.The unreleased #2919 changeset loses its sentence saying the MCP tools do not honour the lock yet.
Part of #2819; #2919 left this half to its own PR.
Type of change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change)pnpm formathas been runmessages.pochanges except in translation PRs — a workflow extracts catalogs on merge tomain. (n/a: no admin UI change)AI-generated code disclosure
Screenshots / test output
entry-lock.test.tsruns each tool through a real MCP client against another user's lease. Every assertion fails under one source mutation, run one at a time: the check removed from a tool or the route,overrideLockignored or dropped from a schema, the wrong user checked, or the lock checked before permissions. The authorization unit test's handler stub gets a migrated in-memory database for the lock query.