From 123e8de921856ef8fea822f5658ae64f23b1a50d Mon Sep 17 00:00:00 2001 From: Kevin Jahns Date: Wed, 5 Aug 2026 12:31:41 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=92=EF=B8=8F(collaboration)=20harden?= =?UTF-8?q?=20the=20create-ydoc=20endpoint?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address the findings of an adversarial review of the new endpoint: - Only the backend admin token may attribute content to another user via the X-User-Id header. The endpoint uses the default access purpose, so any editor with update ability can call it — honoring the header for them would let an editor forge the attribution history of the first revision (the websocket path likewise stamps the server-side identity). Regular callers now always author as themselves; verified: an editor session posting X-User-Id gets its own userid stamped. - Reject non-main ?branch= requests (400). Cookie users are main-only via getAccessType, but the admin token bypasses it and could seed an orphan (org, docid, branch) room no user-facing path reads — while dodging the branch-scoped 409 existence check. - Correct the concurrent-create comment: two racing creates merge as independently generated updates (fresh clientIDs), so the seeded content appears twice — user-visible duplication, not merely a doubly-attributed revision. Still accepted (Django creates each doc once and a duplicated seed is user-fixable), but the tradeoff is now stated accurately. Co-Authored-By: Claude Fable 5 Signed-off-by: Kevin Jahns --- src/yhub-server/server.js | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/src/yhub-server/server.js b/src/yhub-server/server.js index 20f940583..4f7f303f2 100644 --- a/src/yhub-server/server.js +++ b/src/yhub-server/server.js @@ -195,6 +195,13 @@ const api = [ if (!UUID4.test(req.docid)) { return jsonResponse(400, { error: 'Room name is invalid' }); } + if (req.branch !== 'main') { + // cookie users are main-only via getAccessType, but the admin + // token bypasses it — reject explicitly so an admin create can't + // seed an orphan non-main room (and dodge the 409 check, which is + // branch-scoped) + return jsonResponse(400, { error: 'Unknown branch' }); + } const body = await req.bytes(); // req.bytes() resolves to a Node Buffer, but the compute-task schema // requires an exact Uint8Array (lib0 $constructedBy compares the @@ -216,8 +223,10 @@ const api = [ } // covers persisted state AND uncompacted stream messages. Not atomic // with addMessage below (yhub has no atomic create): two concurrent - // creates can both pass — acceptable, Yjs merges both updates; worst - // case is a doubly-attributed first revision, never corruption. + // creates can both pass the check and their updates merge — with + // independently generated updates (fresh clientIDs) the seeded + // content then appears twice. Accepted: Django creates each doc + // once, and a duplicated seed is user-fixable, unlike corruption. const { gcDoc } = await req.yhub.getDoc( req.room, { gc: true, nongc: false }, @@ -226,9 +235,13 @@ const api = [ if (gcDoc != null && gcDoc.byteLength > 3) { return jsonResponse(409, { error: 'Document already exists' }); } - // attribute the initial content to the acting user when the caller - // names one, else to the caller's identity ('system' for admin tokens) - const userid = req.headers['x-user-id'] || req.authInfo.userid; + // Only the backend admin token may attribute the content to another + // user; regular callers always author as themselves — honoring a + // client-supplied header would let any editor forge the attribution + // history (the ws path likewise stamps the server-side identity). + const userid = + (req.authInfo.admin === true && req.headers['x-user-id']) || + req.authInfo.userid; let result; try { // diffs the posted update against the (empty) current doc and