mirror of
https://github.com/suitenumerique/docs.git
synced 2026-09-07 18:27:54 +02:00
🔒️(collaboration) harden the create-ydoc endpoint
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 <noreply@anthropic.com> Signed-off-by: Kevin Jahns <kevin.jahns@protonmail.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
64b46de526
commit
123e8de921
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user