diff --git a/src/backend/core/management/commands/clean_document.py b/src/backend/core/management/commands/clean_document.py index 5e041a80c..525868b4d 100644 --- a/src/backend/core/management/commands/clean_document.py +++ b/src/backend/core/management/commands/clean_document.py @@ -1,13 +1,5 @@ """Clean a document by resetting it (keeping its title) and deleting all descendants.""" -# TODO(yhub): this sandbox reset no longer erases the document content. It purges -# the S3 versions, but yhub durably retains the Yjs document in its own Postgres -# and re-serves it on the next websocket connect (CRDT merge with the empty -# seed resurrects the purged content). yhub has no delete API; until it grows -# one, the interim remediation is to run, against yhub's stores: -# DELETE FROM yhub_ydoc_v1 WHERE org='docs' AND docid=''; -# and drop the `yhub:room:docs::*` redis keys. - import logging from django.conf import settings @@ -28,6 +20,7 @@ from core.models import ( LinkTrace, Thread, ) +from core.services.yhub_services import YHubError, YHubService logger = logging.getLogger("impress.commands.clean_document") @@ -150,8 +143,56 @@ class Command(BaseCommand): logger.warning("Failed to delete S3 attachment %s", key) self.stdout.write(f"Deleted {len(all_attachment_keys)} attachment(s) from S3.") + + # After the object storage, never before: what is erased here can be + # seeded back from a legacy object that is still in the bucket, and the + # first read of the document is all it takes. + self._erase_collaboration_content(all_documents) + self.stdout.write("Done.") + def _erase_collaboration_content(self, documents): + """ + Erase the content of the documents on the collaboration server. + + That is where the content lives: without this the reset only clears the + database and the object storage, and the next editor to connect is + served the document that was supposed to be gone. + + The room is emptied and left usable rather than deleted — the root + document keeps its id and goes on being edited. Its descendants are + deleted for good by then and would not mind either way. + + The editors are disconnected, but each of them holds a copy of the + document: one that reconnects with it syncs the content back into the + emptied room. Run this when nobody is editing, and have anyone who was + reload the page. + """ + service = YHubService() + failed = [] + + for doc in documents: + try: + service.reset_ydoc(doc) + except YHubError: + logger.warning( + "Failed to erase the collaboration content of document %s", doc.id + ) + failed.append(doc.id) + + erased = len(documents) - len(failed) + self.stdout.write(f"Erased collaboration content for {erased} document(s).") + + if failed: + # loud, and by id: the content of these documents is still being + # served, so the reset is not done until they are dealt with + self.stderr.write( + "Collaboration content NOT erased for " + f"{len(failed)} document(s): {', '.join(str(id) for id in failed)}. " + "Their content is still served by the collaboration server, " + "run the command again." + ) + def _clean_root_relations(self, document): """ Delete the relations attached to the root document: accesses (except diff --git a/src/backend/core/services/yhub_services.py b/src/backend/core/services/yhub_services.py index 214decbd1..327fa55f6 100644 --- a/src/backend/core/services/yhub_services.py +++ b/src/backend/core/services/yhub_services.py @@ -320,6 +320,25 @@ class YHubService: """ return self.request("post", self.build_url("restore-ydoc", document)) + def reset_ydoc(self, document): + """ + Erase the content of a document on the collaboration server. + + The document itself stays: its room is emptied and left usable, as if + it had never been written. This is what resetting a document means once + the content lives there — deleting the room would answer 404 for a + document that goes on existing. + + Irreversible, and it is meant to be: the editors are disconnected and + the content is gone from the collaboration server for good, history + included. Only the backend can ask for it. + """ + return self.request( + "post", + self.build_url("reset-ydoc", document), + headers=self.build_user_header(self.user_id), + ) + def migrate(self, document, force=False): """ Replay the legacy version history of a document into the collaboration server. diff --git a/src/backend/core/tests/commands/test_clean_document.py b/src/backend/core/tests/commands/test_clean_document.py index 2f8468416..bcc554b4d 100644 --- a/src/backend/core/tests/commands/test_clean_document.py +++ b/src/backend/core/tests/commands/test_clean_document.py @@ -11,10 +11,23 @@ from botocore.exceptions import ClientError from core import choices, factories, models from core.choices import LinkReachChoices, LinkRoleChoices +from core.services.yhub_services import ServiceUnavailableError pytestmark = pytest.mark.django_db +@pytest.fixture(autouse=True, name="mock_yhub") +def mock_yhub_fixture(): + """ + Stand in for the collaboration server, which holds the content the command + erases. Autouse: every run of the command reaches it. + """ + with mock.patch( + "core.management.commands.clean_document.YHubService" + ) as mock_service: + yield mock_service.return_value + + def purged_keys(mock_storage): """ Return the set of object keys whose versions were purged from S3, i.e. the @@ -378,3 +391,50 @@ def test_clean_document_with_options(settings): child.file_key, grandchild.file_key, } + + +def test_clean_document_erases_the_collaboration_content(settings, mock_yhub): + """ + The content lives on the collaboration server, so resetting a document + means erasing it there too — for the root and for the descendants the + command deletes. + """ + settings.DEBUG = True + + root = factories.DocumentFactory(title="Root") + child = factories.DocumentFactory(parent=root) + grandchild = factories.DocumentFactory(parent=child) + + with mock.patch("core.management.commands.clean_document.default_storage"): + call_command("clean_document", str(root.id)) + + assert mock_yhub.reset_ydoc.call_args_list == [ + mock.call(root), + mock.call(child), + mock.call(grandchild), + ] + + +def test_clean_document_reports_the_documents_it_could_not_erase( + settings, mock_yhub, capsys +): + """ + A document the collaboration server would not erase is named, and does not + deprive the ones after it of their erasure: its content is still served, so + the reset is not done. + """ + settings.DEBUG = True + + root = factories.DocumentFactory(title="Root") + child = factories.DocumentFactory(parent=root) + mock_yhub.reset_ydoc.side_effect = [ServiceUnavailableError("yhub is down"), None] + + with mock.patch("core.management.commands.clean_document.default_storage"): + call_command("clean_document", str(root.id)) + + assert mock_yhub.reset_ydoc.call_args_list == [mock.call(root), mock.call(child)] + + captured = capsys.readouterr() + assert "Erased collaboration content for 1 document(s)." in captured.out + assert str(root.id) in captured.err + assert str(child.id) not in captured.err diff --git a/src/backend/core/tests/test_services_yhub_services.py b/src/backend/core/tests/test_services_yhub_services.py index e39219252..8d780677e 100644 --- a/src/backend/core/tests/test_services_yhub_services.py +++ b/src/backend/core/tests/test_services_yhub_services.py @@ -273,6 +273,24 @@ def test_restore_ydoc(mock_request): ) +@patch("requests.request") +def test_reset_ydoc(mock_request): + """Should ask yhub to erase the content of the document.""" + mock_request.return_value.ok = True + user = UserFactory.build() + + response = YHubService(user=user).reset_ydoc(DOCUMENT) + + assert response is mock_request.return_value + args, kwargs = mock_request.call_args + assert args == ( + "post", + f"http://yhub:3002/collaboration/reset-ydoc/v1/docs/{DOCUMENT.id!s}", + ) + # who erased the content, for the record yhub keeps of the deletion it does + assert kwargs["headers"]["X-User-Id"] == str(user.pk) + + @patch("requests.request") def test_restore_ydoc_erased_content(mock_request): """A document whose content was erased should report the conflict it is.""" diff --git a/src/yhub-server/README.md b/src/yhub-server/README.md index 1a7c9ee19..3b3e7d607 100644 --- a/src/yhub-server/README.md +++ b/src/yhub-server/README.md @@ -46,6 +46,9 @@ It is not a fork of yhub — it is a thin wrapper: deletion of a document — admin JWT only, like `reset-connections`. Deleting one needs nothing custom, the built-in `DELETE .../ydoc/` does it (see "Deletion" below); restoring has no built-in route, +- exposes `POST /collaboration/reset-ydoc/v1/{org}/{docid}`, which erases the + content of a document and leaves its room usable — admin JWT only, and + irreversible (see "Deletion" below), - notifies the Django backend on `POST /api/v1.0/documents/{id}/content-updated/` whenever the worker persists new content for a document, so that lists ordered by `updated_at` @@ -65,9 +68,9 @@ the websocket and the built-in document APIs (`ydoc`, `rollback`, `prune`, `changeset`, `activity`) are all guarded by the same cookie-based document authorization and are meant to be reachable by browsers, as is `/collaboration/jwks/v1`, which carries public keys and nothing else. The one -exception is `/collaboration/reset-connections/`, `/collaboration/migrate/` and -`/collaboration/restore-ydoc/`, which are backend-internal and should not be -routed through the public ingress. +exception is `/collaboration/reset-connections/`, `/collaboration/migrate/`, +`/collaboration/restore-ydoc/` and `/collaboration/reset-ydoc/`, which are +backend-internal and should not be routed through the public ingress. ## Container image @@ -141,11 +144,46 @@ a subtree without asking what became of each document in it. Erasing the content for good is a third operation (`YHub.deleteDoc(room, { hard: true })`), reachable from inside this process only — yhub deliberately -keeps it off the REST API. Nothing here calls it: Docs never erases a document -either, a soft-deleted one simply stops being restorable after -`TRASHBIN_CUTOFF_DAYS`. Note that a hard deletion is final for that room — the +keeps it off the REST API. It is not what deleting a document in Docs does: a +soft-deleted one simply stops being restorable after `TRASHBIN_CUTOFF_DAYS`, +and its content is kept. Note that a hard deletion is final for that room — the docid can never be written again, and `restore-ydoc` answers 409 for it. +### Resetting (`POST /collaboration/reset-ydoc/v1/{org}/{docid}`) + +One caller does erase content: the backend's `clean_document` command, which +resets the onboarding sandbox. It empties a document rather than deleting it — +the Django document keeps its id and goes on being edited — so neither deletion +fits: a soft one answers 404 for a document that still exists, and a hard one is +final for the room. + +This endpoint hard-deletes and then drops the deletion record, which is what +leaves the room writable again. That order matters: the record is also the +barrier that refuses every write while the erasure runs, so a compaction that +was already merging cannot put the content back. Compaction is disabled for the +room around the whole sequence, and the content is read back afterwards — if it +reappeared, the erasure runs once more, and the endpoint answers 500 rather than +report an erasure it did not achieve. + +Irreversible, admin JWT only, and backend-internal. + +**Erasing a room does not erase the copies of it.** The editors are disconnected +(close code 4404), but a Yjs client holds the whole document in memory: one that +reconnects with its copy syncs it back into the empty room, and the content is +returned. The room accepting writes again is what makes this a reset rather than +a deletion, so the room itself cannot refuse them. + +Connected clients could be dealt with, and deliberately are not: broadcasting an +update that deletes everything, before the kick, empties them for good — a Yjs +client with garbage collection on (what an editor runs, Docs refuses `gc=false` +connections to users) drops the deleted content rather than keeping it as +history, so it has nothing left to push back. What that does not cover is a +client that was offline or backgrounded at that moment, which comes back with +its copy intact either way. + +So: reset a document when nobody is editing it, and have anyone who was reload +the page. + ## Soft migration (`SOFT_MIGRATION=true`) Documents were historically stored by the Django backend in the S3 media diff --git a/src/yhub-server/server.js b/src/yhub-server/server.js index 34560d20a..f2cfc9a39 100644 --- a/src/yhub-server/server.js +++ b/src/yhub-server/server.js @@ -61,6 +61,9 @@ const UUID4 = // an empty Yjs update (what `Y.encodeStateAsUpdate(new Y.Doc())` encodes to) — // hardcoded so we don't import @y/y for two bytes const EMPTY_YDOC = new Uint8Array([0, 0]); +// yhub's "no effective content" convention: an empty update encodes to 2 bytes, +// and anything up to 3 is read as an empty document +const EMPTY_UPDATE_MAX_BYTES = 3; // uws buffers the whole body before the handler sees it, so this cap does not // bound upload memory — it bounds what a single create hands to a compute // worker and writes to the valkey stream as one message. Creates carry one @@ -69,6 +72,7 @@ const EMPTY_YDOC = new Uint8Array([0, 0]); const MAX_CREATE_BYTES = 10 * 1024 * 1024; const touchLog = logger.child({ module: 'updated-at-notifier' }); +const resetLog = logger.child({ module: 'reset-ydoc' }); const BACKEND_NOTIFY_TIMEOUT_MS = 5000; // Audience of the tokens the backend accepts from us. It must match the one @@ -354,6 +358,36 @@ const jsonResponse = (status, body) => headers: { 'content-type': 'application/json' }, }); +// Does the room hold anything? Covers persisted rows and the messages still on +// the stream, which is what makes it an answer about the content rather than +// about the storage. +const hasContent = async (yhub, room) => { + const { gcDoc } = await yhub.getDoc( + room, + { gc: true, nongc: false }, + { gcOnMerge: false }, + ); + return gcDoc != null && gcDoc.byteLength > EMPTY_UPDATE_MAX_BYTES; +}; + +// Erase every trace of a room's content and leave it writable again. +// +// The erasure is yhub's hard deletion: it clears the stream, disconnects the +// editors and drops every row and asset, irreversibly. Its tombstone is also +// the barrier that stops a compaction still in flight from writing the content +// back — every `store` is refused while it is there, and the purge runs behind +// it — so the room is only made writable again, by dropping the tombstone, +// once there is nothing left to write back. +// +// Dropping the tombstone is what makes this a reset rather than a deletion: +// yhub has no such operation, a hard deletion is final for the room and even +// `restoreDoc` refuses it. Here the document id belongs to a Django document +// that goes on living, so the room has to be usable again. +const eraseContent = async (yhub, room, by) => { + await yhub.deleteDoc(room, { hard: true, by }); + await yhub.persistence.deleteTombstone(room); +}; + const api = [ // GET /collaboration/jwks/v1 — the public keys verifying the tokens we sign // to call the backend, in the JSON Web Key Set format (RFC 7517). Global @@ -493,9 +527,9 @@ const api = [ // ("HTTP/1.1 413 ") — legal, and callers switch on the code return jsonResponse(413, { error: 'Update too large' }); } - // <= 3 bytes is yhub's "no effective content" convention (an empty - // update encodes to 2 bytes) — reject before it reaches a worker - if (update.byteLength <= 3) { + // yhub's "no effective content" convention — reject before it reaches + // a worker + if (update.byteLength <= EMPTY_UPDATE_MAX_BYTES) { return jsonResponse(400, { error: 'Empty update' }); } // covers persisted state AND uncompacted stream messages. Not atomic @@ -509,7 +543,7 @@ const api = [ { gc: true, nongc: false }, { gcOnMerge: false }, ); - if (gcDoc != null && gcDoc.byteLength > 3) { + if (gcDoc != null && gcDoc.byteLength > EMPTY_UPDATE_MAX_BYTES) { return jsonResponse(409, { error: 'Document already exists' }); } // Only the backend admin token may attribute the content to another @@ -603,6 +637,61 @@ const api = [ }, }, }), + // POST /collaboration/reset-ydoc/v1/{org}/{docid} — erase the content of a + // document and leave the room usable, as if it had never been written. + // + // What the backend's `clean_document` command needs to reset the onboarding + // sandbox: the Django document keeps its id and goes on being edited, so + // deleting the room is not an option — a hard deletion is final and even a + // soft one would answer 404 for a document that still exists. Backend-internal + // and admin-only, like the deletions it is built on: this destroys content + // with no way back. + createApiEndpoint('reset-ydoc', { + accessPurpose: 'reset', + post: { + handler: async (req) => { + if (req.org !== ORG) { + return jsonResponse(400, { error: 'Unknown org' }); + } + if (!UUID4.test(req.docid)) { + return jsonResponse(400, { error: 'Room name is invalid' }); + } + if (req.branch !== 'main') { + return jsonResponse(400, { error: 'Unknown branch' }); + } + const by = req.headers['x-user-id'] || req.authInfo.userid; + // Nothing compacts this room while the erasure runs: this drops the + // task already waiting for it and refuses to enqueue another, which + // leaves one writer to race with — a task a worker had claimed before + // this call. The tombstone barrier covers it right up to the moment + // the room is made writable again, so it can only land after that, + // and the second pass below is what picks it up. + await req.yhub.stream.disableCompaction(req.room); + try { + await eraseContent(req.yhub, req.room, by); + if (await hasContent(req.yhub, req.room)) { + resetLog.warn( + { docid: req.docid }, + 'content came back while it was being erased, erasing again', + ); + await eraseContent(req.yhub, req.room, by); + if (await hasContent(req.yhub, req.room)) { + // saying it is erased when it is not is the one answer this + // endpoint must never give + return jsonResponse(500, { + error: 'Document content came back after being erased', + }); + } + } + } finally { + // even on failure: leaving compaction off would freeze the room for + // every later edit, a worse state than the one we came to fix + await req.yhub.stream.enableCompaction(req.room); + } + return jsonResponse(200, { message: 'Document content erased' }); + }, + }, + }), ]; // Django orders the document lists by `updated_at` and no edit goes through it