Summary
POST /documents/:slug/ops performs a route-level broad reapply of the persisted DB row into the live collab room after non-rewrite.apply mutations, using source: 'rest-ops'. Unlike every other server-side write source, rest-ops is not in shouldBlockLegacyLiveApplySource, so this reapply is not blocked when a live shared doc is present. A point-in-time DB snapshot pushed into a live room can replay stale markdown over concurrent edits that are not yet persisted.
The repo's own collab-security test already asserts this reapply should be gone, but the corresponding refactor was never landed, so the test fails on main.
Where
server/routes.ts, in the /documents/:slug/ops success path (around line 1729-1743):
if (op !== 'rewrite.apply') {
try {
const collabRuntime = getCollabRuntime();
if (collabRuntime.enabled) {
const updatedDoc = getDocumentBySlug(slug);
if (updatedDoc) {
const applyOptions = {
markdown: typeof updatedDoc.markdown === 'string' ? updatedDoc.markdown : undefined,
marks: parseJson(updatedDoc.marks),
source: 'rest-ops',
};
await applyCanonicalDocumentToCollab(slug, applyOptions);
...
The guard that is supposed to stop legacy reverse-flow writes onto live docs is shouldBlockLegacyLiveApplySource in server/collab.ts:
function shouldBlockLegacyLiveApplySource(source: string): boolean {
if (!source) return false;
return source === 'rest-put'
|| source === 'engine'
|| source === 'engine-suggestion-accept'
|| source === 'library'
|| source.startsWith('agent')
|| source.startsWith('rewrite:');
}
rest-ops is absent here, while its REST sibling rest-put (PUT /documents/:slug) is present. applyCanonicalDocumentToCollabInner only blocks-and-marks-stale when shouldBlockLegacyLiveApplySource(origin) is true, so a rest-ops apply proceeds onto a live room.
Impact
A non-rewrite.apply /ops mutation that lands while a client is live can overwrite the live fragment with the DB row markdown. Any concurrent client edit that has not yet been persisted to the row is at risk of being clobbered (and the agent op can land against shifted content), returning 200 success.
Evidence
src/tests/collab-security.test.ts encodes the intended end state:
assert(
routesSource.includes('Route-level reapplication from the DB can replay stale markdown')
&& !routesSource.includes("source: 'rest-ops'"),
'Expected /documents/:slug/ops to avoid broad DB reapply after engine-backed mutations',
);
On current main, routes.ts still contains source: 'rest-ops' and does not contain that comment, so this assertion fails. The intended refactor (route stops reapplying; non-rewrite ops propagate through the canonical Yjs path the way rewrite.apply does) appears to have never been completed.
Question / possible directions
Is the rest-ops route-level reapply intended, or is the collab-security guard the intended target state? Two directions, depending on the answer:
- Complete the refactor the test anticipates: have non-
rewrite.apply /ops mutations commit through the canonical rewrite/Yjs path and drop the route-level DB reapply, so the live room is never re-seeded from a stale row.
- Minimal hardening for parity: add
rest-ops to shouldBlockLegacyLiveApplySource so it is treated like rest-put (block on live docs, mark the projection stale, let clients resync from canonical).
Happy to send a PR for whichever direction you prefer.
Summary
POST /documents/:slug/opsperforms a route-level broad reapply of the persisted DB row into the live collab room after non-rewrite.applymutations, usingsource: 'rest-ops'. Unlike every other server-side write source,rest-opsis not inshouldBlockLegacyLiveApplySource, so this reapply is not blocked when a live shared doc is present. A point-in-time DB snapshot pushed into a live room can replay stale markdown over concurrent edits that are not yet persisted.The repo's own
collab-securitytest already asserts this reapply should be gone, but the corresponding refactor was never landed, so the test fails onmain.Where
server/routes.ts, in the/documents/:slug/opssuccess path (around line 1729-1743):The guard that is supposed to stop legacy reverse-flow writes onto live docs is
shouldBlockLegacyLiveApplySourceinserver/collab.ts:rest-opsis absent here, while its REST siblingrest-put(PUT/documents/:slug) is present.applyCanonicalDocumentToCollabInneronly blocks-and-marks-stale whenshouldBlockLegacyLiveApplySource(origin)is true, so arest-opsapply proceeds onto a live room.Impact
A non-
rewrite.apply/opsmutation that lands while a client is live can overwrite the live fragment with the DB row markdown. Any concurrent client edit that has not yet been persisted to the row is at risk of being clobbered (and the agent op can land against shifted content), returning200 success.Evidence
src/tests/collab-security.test.tsencodes the intended end state:On current
main,routes.tsstill containssource: 'rest-ops'and does not contain that comment, so this assertion fails. The intended refactor (route stops reapplying; non-rewrite ops propagate through the canonical Yjs path the wayrewrite.applydoes) appears to have never been completed.Question / possible directions
Is the
rest-opsroute-level reapply intended, or is thecollab-securityguard the intended target state? Two directions, depending on the answer:rewrite.apply/opsmutations commit through the canonical rewrite/Yjs path and drop the route-level DB reapply, so the live room is never re-seeded from a stale row.rest-opstoshouldBlockLegacyLiveApplySourceso it is treated likerest-put(block on live docs, mark the projection stale, let clients resync from canonical).Happy to send a PR for whichever direction you prefer.