diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index d6eef358d50..7a1847389f5 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -453,8 +453,8 @@ importers: specifier: ^4.0.5 version: 4.0.5(eslint@10.4.0)(typescript@6.0.3) etherpad-cli-client: - specifier: ^4.0.2 - version: 4.0.2 + specifier: ^4.0.3 + version: 4.0.3 mocha: specifier: ^11.7.5 version: 11.7.5 @@ -3377,8 +3377,8 @@ packages: resolution: {integrity: sha512-aIL5Fx7mawVa300al2BnEE4iNvo1qETxLrPI/o05L7z6go7fCw1J6EQmbK4FmJ2AS7kgVF/KEZWufBfdClMcPg==} engines: {node: '>= 0.6'} - etherpad-cli-client@4.0.2: - resolution: {integrity: sha512-oAyJxJj4UH0AAEmMPpMiTHKd2IT14OWnKaPzWTqLh3BJ8nyEv/IAMA5scAGjet0xKNERm+k7VSnfeJQvN4adKQ==} + etherpad-cli-client@4.0.3: + resolution: {integrity: sha512-nQt5FxpmfQHwJ546TJXKMT4fMIS1crvU+NvHAypfboHTsbFQMIPOu4ZANSirRzE1lYnH4APUBDwMQqlKeQt9rw==} engines: {node: '>=18.0.0'} hasBin: true @@ -4002,6 +4002,7 @@ packages: js-cookie@3.0.6: resolution: {integrity: sha512-mYhz0Og/Wv8bZJcBC6rRzYG+rYf8DyQSK3rcUbuGHQIgSsX6uynAIVoaPgqf0udH2AGS953hGFy3w6on1rJzMw==} engines: {node: '>=20'} + deprecated: Missing corresponding tag/release on GitHub js-levenshtein@1.1.6: resolution: {integrity: sha512-X2BB11YZtrRqY4EnQcLX5Rh373zbK4alC1FW7D7MBhL2gtcC17cTnr6DmfHZeS0s2rTHjUTMMHfG7gO8SSdw+g==} @@ -8768,10 +8769,10 @@ snapshots: '@rushstack/eslint-patch': 1.16.1 '@typescript-eslint/eslint-plugin': 7.18.0(@typescript-eslint/parser@7.18.0(eslint@10.4.0)(typescript@6.0.3))(eslint@10.4.0)(typescript@6.0.3) '@typescript-eslint/parser': 7.18.0(eslint@10.4.0)(typescript@6.0.3) - eslint-import-resolver-typescript: 3.9.1(eslint-plugin-import@2.32.0)(eslint@10.4.0) + eslint-import-resolver-typescript: 3.9.1(eslint-plugin-import@2.32.0(@typescript-eslint/parser@7.18.0(eslint@10.4.0)(typescript@6.0.3))(eslint@10.4.0))(eslint@10.4.0) eslint-plugin-cypress: 2.15.2(eslint@10.4.0) eslint-plugin-eslint-comments: 3.2.0(eslint@10.4.0) - eslint-plugin-import: 2.32.0(@typescript-eslint/parser@7.18.0(eslint@10.4.0)(typescript@6.0.3))(eslint-import-resolver-typescript@3.9.1)(eslint@10.4.0) + eslint-plugin-import: 2.32.0(@typescript-eslint/parser@7.18.0(eslint@10.4.0)(typescript@6.0.3))(eslint-import-resolver-typescript@3.9.1(eslint-plugin-import@2.32.0(@typescript-eslint/parser@7.18.0(eslint@10.4.0)(typescript@6.0.3))(eslint@10.4.0))(eslint@10.4.0))(eslint@10.4.0) eslint-plugin-mocha: 10.5.0(eslint@10.4.0) eslint-plugin-n: 17.24.0(eslint@10.4.0)(typescript@6.0.3) eslint-plugin-prefer-arrow: 1.2.3(eslint@10.4.0) @@ -8792,7 +8793,7 @@ snapshots: transitivePeerDependencies: - supports-color - eslint-import-resolver-typescript@3.9.1(eslint-plugin-import@2.32.0)(eslint@10.4.0): + eslint-import-resolver-typescript@3.9.1(eslint-plugin-import@2.32.0(@typescript-eslint/parser@7.18.0(eslint@10.4.0)(typescript@6.0.3))(eslint@10.4.0))(eslint@10.4.0): dependencies: '@nolyfill/is-core-module': 1.0.39 debug: 4.4.3(supports-color@8.1.1) @@ -8803,18 +8804,18 @@ snapshots: stable-hash: 0.0.5 tinyglobby: 0.2.16 optionalDependencies: - eslint-plugin-import: 2.32.0(@typescript-eslint/parser@7.18.0(eslint@10.4.0)(typescript@6.0.3))(eslint-import-resolver-typescript@3.9.1)(eslint@10.4.0) + eslint-plugin-import: 2.32.0(@typescript-eslint/parser@7.18.0(eslint@10.4.0)(typescript@6.0.3))(eslint-import-resolver-typescript@3.9.1(eslint-plugin-import@2.32.0(@typescript-eslint/parser@7.18.0(eslint@10.4.0)(typescript@6.0.3))(eslint@10.4.0))(eslint@10.4.0))(eslint@10.4.0) transitivePeerDependencies: - supports-color - eslint-module-utils@2.12.1(@typescript-eslint/parser@7.18.0(eslint@10.4.0)(typescript@6.0.3))(eslint-import-resolver-node@0.3.10)(eslint-import-resolver-typescript@3.9.1)(eslint@10.4.0): + eslint-module-utils@2.12.1(@typescript-eslint/parser@7.18.0(eslint@10.4.0)(typescript@6.0.3))(eslint-import-resolver-node@0.3.10)(eslint-import-resolver-typescript@3.9.1(eslint-plugin-import@2.32.0(@typescript-eslint/parser@7.18.0(eslint@10.4.0)(typescript@6.0.3))(eslint@10.4.0))(eslint@10.4.0))(eslint@10.4.0): dependencies: debug: 3.2.7 optionalDependencies: '@typescript-eslint/parser': 7.18.0(eslint@10.4.0)(typescript@6.0.3) eslint: 10.4.0 eslint-import-resolver-node: 0.3.10 - eslint-import-resolver-typescript: 3.9.1(eslint-plugin-import@2.32.0)(eslint@10.4.0) + eslint-import-resolver-typescript: 3.9.1(eslint-plugin-import@2.32.0(@typescript-eslint/parser@7.18.0(eslint@10.4.0)(typescript@6.0.3))(eslint@10.4.0))(eslint@10.4.0) transitivePeerDependencies: - supports-color @@ -8836,7 +8837,7 @@ snapshots: eslint: 10.4.0 ignore: 5.3.2 - eslint-plugin-import@2.32.0(@typescript-eslint/parser@7.18.0(eslint@10.4.0)(typescript@6.0.3))(eslint-import-resolver-typescript@3.9.1)(eslint@10.4.0): + eslint-plugin-import@2.32.0(@typescript-eslint/parser@7.18.0(eslint@10.4.0)(typescript@6.0.3))(eslint-import-resolver-typescript@3.9.1(eslint-plugin-import@2.32.0(@typescript-eslint/parser@7.18.0(eslint@10.4.0)(typescript@6.0.3))(eslint@10.4.0))(eslint@10.4.0))(eslint@10.4.0): dependencies: '@rtsao/scc': 1.1.0 array-includes: 3.1.9 @@ -8847,7 +8848,7 @@ snapshots: doctrine: 2.1.0 eslint: 10.4.0 eslint-import-resolver-node: 0.3.10 - eslint-module-utils: 2.12.1(@typescript-eslint/parser@7.18.0(eslint@10.4.0)(typescript@6.0.3))(eslint-import-resolver-node@0.3.10)(eslint-import-resolver-typescript@3.9.1)(eslint@10.4.0) + eslint-module-utils: 2.12.1(@typescript-eslint/parser@7.18.0(eslint@10.4.0)(typescript@6.0.3))(eslint-import-resolver-node@0.3.10)(eslint-import-resolver-typescript@3.9.1(eslint-plugin-import@2.32.0(@typescript-eslint/parser@7.18.0(eslint@10.4.0)(typescript@6.0.3))(eslint@10.4.0))(eslint@10.4.0))(eslint@10.4.0) hasown: 2.0.2 is-core-module: 2.16.1 is-glob: 4.0.3 @@ -8997,7 +8998,7 @@ snapshots: etag@1.8.1: {} - etherpad-cli-client@4.0.2: + etherpad-cli-client@4.0.3: dependencies: socket.io-client: 4.8.3 superagent: 10.3.0 diff --git a/src/node/db/Pad.ts b/src/node/db/Pad.ts index 0e4eaaac0e4..9e5fd7de33d 100644 --- a/src/node/db/Pad.ts +++ b/src/node/db/Pad.ts @@ -93,6 +93,17 @@ exports.cleanText = (txt:string): string => txt.replace(/\r\n/g, '\n') .replace(/\t/g, ' '); class Pad { + /** + * Stable author id used to attribute inserts coming from internal callers + * (HTTP API setText/appendText with no authorId, plugins like ep_post_data, + * server-side import flows). Without ANY author attribute, pad.atext.text + * and pad.atext.attribs drift out of sync — clients then fail + * setDocAText reconciliation in ace2_inner.ts when loading the pad. Using + * a fixed system author keeps the AText well-formed without requiring + * every plugin to allocate its own author up-front. + */ + static readonly SYSTEM_AUTHOR_ID = 'a.etherpad-system'; + private db: Database; private atext: AText; private pool: AttributePool; @@ -415,9 +426,19 @@ class Pad { (!ins && start > 0 && orig[start - 1] === '\n'); if (!willEndWithNewline) ins += '\n'; if (ndel === 0 && ins.length === 0) return; - const attribs = authorId ? [['author', authorId] as [string, string]] : undefined; + // An unattributed insert (empty authorId + non-empty ins) would produce + // an AText where `text` and `attribs` disagree on length — clients fail + // setDocAText reconciliation on load. Backward-compat fix: if the caller + // didn't provide an authorId, attribute the insert to a stable system + // author. ep_post_data and other plugins that want named attribution + // should still pass an explicit authorId. + const effectiveAuthorId = + (ins.length > 0 && !authorId) ? Pad.SYSTEM_AUTHOR_ID : authorId; + const attribs = effectiveAuthorId + ? [['author', effectiveAuthorId] as [string, string]] + : undefined; const changeset = makeSplice(orig, start, ndel, ins, attribs, this.pool); - await this.appendRevision(changeset, authorId); + await this.appendRevision(changeset, effectiveAuthorId); } /** diff --git a/src/node/handler/PadMessageHandler.ts b/src/node/handler/PadMessageHandler.ts index 3a27a7ac7be..592e5b9e857 100644 --- a/src/node/handler/PadMessageHandler.ts +++ b/src/node/handler/PadMessageHandler.ts @@ -24,7 +24,7 @@ import {MapArrayType} from "../types/MapType"; import AttributeMap from '../../static/js/AttributeMap'; const padManager = require('../db/PadManager'); const padDeletionManager = require('../db/PadDeletionManager'); -import {checkRep, cloneAText, compose, deserializeOps, follow, identity, inverse, makeAText, makeSplice, moveOpsToNewPool, mutateAttributionLines, mutateTextLines, oldLen, prepareForWire, splitAttributionLines, splitTextLines, unpack} from '../../static/js/Changeset'; +import {applyToText, checkRep, cloneAText, compose, deserializeOps, follow, identity, inverse, makeAText, moveOpsToNewPool, mutateAttributionLines, mutateTextLines, oldLen, prepareForWire, splitAttributionLines, splitTextLines, unpack} from '../../static/js/Changeset'; import ChatMessage from '../../static/js/ChatMessage'; import AttributePool from '../../static/js/AttributePool'; const AttributeManager = require('../../static/js/AttributeManager'); @@ -875,12 +875,49 @@ const handleUserChanges = async (socket:any, message: { `${opAuthorId} in changeset ${changeset}`); } } + // Reject '+' ops that do not carry the author attribute. The standard + // JS client always tags inserts with the author; rejecting unattributed + // inserts here keeps pad.atext.text and pad.atext.attribs in lock-step. + // Without this check, an insert op with empty attribs grows the text + // without contributing matching markers to the attribs string, leaving + // the stored AText in a state where the two iterables disagree on + // length — applyToAText then desyncs and breaks reconciliation in + // every client that later loads the pad. + if (op.opcode === '+' && !opAuthorId) { + throw new Error(`Author ${thisSession.author} submitted an insert without an ` + + `author attribute in changeset ${changeset}`); + } + // Defense-in-depth: reject any wire-borne `*N` that resolves to the + // system author. The session-author equality check above already + // catches the case where `*N` claims a different real user as + // author, but `Pad.SYSTEM_AUTHOR_ID` is server-internal — it's + // only used when `spliceText` / `setText` are called with an empty + // authorId from HTTP API or plugin paths. No legitimate + // socket.io session ever writes as the system author, so a wire + // op that names it is either a confused client or an attempt to + // launder writes through a reserved attribution slot. Either way, + // refuse. + // Hardcoded mirror of `Pad.SYSTEM_AUTHOR_ID` from src/node/db/Pad.ts. + // A `const {Pad} = require('../db/Pad')` at module scope returned + // a partially-initialized class here (circular load via padManager), + // so the static-field access ended up undefined and short-circuited + // the check at runtime. Inline literal is the simplest fix. + if (opAuthorId === 'a.etherpad-system') { + throw new Error(`Author ${thisSession.author} attempted to submit changes as the ` + + `reserved system author ${opAuthorId} in changeset ${changeset}`); + } } // ex. adoptChangesetAttribs // Afaik, it copies the new attributes from the changeset, to the global Attribute Pool let rebasedChangeset = moveOpsToNewPool(changeset, wireApool, pad.pool); + // Snapshot the post-pool-mapping form so the retransmission check below + // can recognise our changeset against the stored revision form. Comparing + // the raw client `changeset` against `c` would miss legitimate + // retransmissions whenever moveOpsToNewPool renumbered an attribute + // (e.g. `*0` -> `*1` because the pad pool already had something at slot 0). + const canonicalCs = rebasedChangeset; // ex. applyUserChanges let r = baseRev; @@ -891,9 +928,9 @@ const handleUserChanges = async (socket:any, message: { while (r < pad.getHeadRevisionNumber()) { r++; const {changeset: c, meta: {author: authorId}} = await pad.getRevision(r); - if (changeset === c && thisSession.author === authorId) { + if (canonicalCs === c && thisSession.author === authorId) { // Assume this is a retransmission of an already applied changeset. - rebasedChangeset = identity(unpack(changeset).oldLen); + rebasedChangeset = identity(unpack(canonicalCs).oldLen); } // At this point, both "c" (from the pad) and "changeset" (from the // client) are relative to revision r - 1. The follow function @@ -909,6 +946,22 @@ const handleUserChanges = async (socket:any, message: { `Can't apply changeset ${rebasedChangeset} with oldLen ` + `${oldLen(rebasedChangeset)} to document of length ${prevText.length}`); } + // Defensive: reject any rebased changeset whose application would leave + // the pad text not ending with '\n'. Previously the server silently + // appended a separate `nlChangeset` correction revision; that worked + // for the stored pad but the FIRST broadcast (the malformed user + // revision) reached browsers BEFORE the correction did, and the + // browser's line assembler asserts "line assembler not finished" on + // a doc that doesn't end with '\n', taking the session out. Refuse to + // accept such changesets — clients must always preserve the + // trailing-newline invariant. + const projectedText = applyToText(rebasedChangeset, prevText); + if (!projectedText.endsWith('\n')) { + throw new Error( + `Rejected USER_CHANGES whose application would leave the pad ` + + `without a trailing '\\n' (length ${projectedText.length}). ` + + `Every USER_CHANGES must preserve the "doc ends with \\n" invariant.`); + } const newRev = await pad.appendRevision(rebasedChangeset, thisSession.author); // The head revision will either stay the same or increase by 1 depending on whether the @@ -920,12 +973,6 @@ const handleUserChanges = async (socket:any, message: { await pad.appendRevision(correctionChangeset, thisSession.author); } - // Make sure the pad always ends with an empty line. - if (pad.text().lastIndexOf('\n') !== pad.text().length - 1) { - const nlChangeset = makeSplice(pad.text(), pad.text().length - 1, 0, '\n'); - await pad.appendRevision(nlChangeset, thisSession.author); - } - // The client assumes that ACCEPT_COMMIT and NEW_CHANGES messages arrive in order. Make sure we // have already sent any previous ACCEPT_COMMIT and NEW_CHANGES messages. assert.equal(thisSession.rev, r); diff --git a/src/package.json b/src/package.json index 84f0b8e0015..d1747401c89 100644 --- a/src/package.json +++ b/src/package.json @@ -125,7 +125,7 @@ "chokidar": "^5.0.0", "eslint": "^10.4.0", "eslint-config-etherpad": "^4.0.5", - "etherpad-cli-client": "^4.0.2", + "etherpad-cli-client": "^4.0.3", "mocha": "^11.7.5", "mocha-froth": "^0.2.10", "nodeify": "^1.0.1", diff --git a/src/tests/backend/specs/Pad.ts b/src/tests/backend/specs/Pad.ts index 0345ae87d1e..eec7898322e 100644 --- a/src/tests/backend/specs/Pad.ts +++ b/src/tests/backend/specs/Pad.ts @@ -61,15 +61,36 @@ describe(__filename, function () { pad = await padManager.getPad(padId, ''); // spliceText is an existing runtime Pad method; cast avoids // adding a type-only declaration to PadType in this PR. - await (pad as any).spliceText(0, 0, '100\u00a0km'); + await (pad as any).spliceText(0, 0, '100\u00a0km', 'a.test'); assert.equal(pad!.text(), '100\u00a0km\n'); }); it('setText round-trips U+00A0', async function () { pad = await padManager.getPad(padId, ''); - await pad!.setText('a\u00a0b\n'); + await pad!.setText('a\u00a0b\n', 'a.test'); assert.equal(pad!.text(), 'a\u00a0b\n'); }); + + it('spliceText with empty authorId attributes to the system author', async function () { + pad = await padManager.getPad(padId, ''); + // An unattributed insert (empty authorId, non-empty ins) used to + // produce an AText where text and attribs disagreed on length \u2014 + // clients then failed setDocAText reconciliation on load. The + // server now substitutes a stable system author so the AText stays + // well-formed without forcing every caller to allocate one up-front. + await (pad as any).spliceText(0, 0, 'plugin-text', ''); + assert.equal(pad!.text(), 'plugin-text\n'); + const pool: any = (pad as any).pool; + let sawSystemAuthor = false; + for (const k of Object.keys(pool.numToAttrib || {})) { + const a = pool.numToAttrib[k]; + if (a[0] === 'author' && a[1] === 'a.etherpad-system') { + sawSystemAuthor = true; + break; + } + } + assert(sawSystemAuthor, 'expected system-author binding in pad pool'); + }); }); describe('padDefaultContent hook', function () { diff --git a/src/tests/backend/specs/messages.ts b/src/tests/backend/specs/messages.ts index c25057569dd..d305da03c66 100644 --- a/src/tests/backend/specs/messages.ts +++ b/src/tests/backend/specs/messages.ts @@ -23,6 +23,8 @@ describe(__filename, function () { agent = await common.init(); }); + let authorId: string; + let roAuthorId: string; beforeEach(async function () { backups.hooks = {handleMessageSecurity: plugins.hooks.handleMessageSecurity}; plugins.hooks.handleMessageSecurity = []; @@ -36,11 +38,18 @@ describe(__filename, function () { const {type, data: clientVars} = await common.handshake(socket, padId); assert.equal(type, 'CLIENT_VARS'); rev = clientVars.collab_client_vars.rev; + authorId = clientVars.userId; roPadId = await readOnlyManager.getReadOnlyId(padId); res = await agent.get(`/p/${roPadId}`).expect(200); roSocket = await common.connect(res); - await common.handshake(roSocket, roPadId); + const roHandshake = await common.handshake(roSocket, roPadId); + // Capture roSocket's own author so tests that send USER_CHANGES via + // roSocket can build apools that match thisSession.author server-side; + // otherwise the wire's *0 reference points at the writer-socket's + // author and the server rejects with badChangeset on the + // "author mismatch" check added in this PR. + roAuthorId = (roHandshake as any).data.userId; await new Promise(resolve => setTimeout(resolve, 1000)); }); @@ -167,8 +176,14 @@ describe(__filename, function () { }); describe('USER_CHANGES', function () { + // Insert ops MUST carry the author attribute (server-side validation + // added in this PR — see PadMessageHandler.ts). Helper assembles the + // wire form `*0+N` plus the matching apool entry for the session author. + const authorPool = () => + ({numToAttrib: {0: ['author', authorId]}, nextNum: 1}); const sendUserChanges = - async (socket:any, cs:any) => await common.sendUserChanges(socket, {baseRev: rev, changeset: cs}); + async (socket:any, cs:any, apool: any = authorPool()) => + await common.sendUserChanges(socket, {baseRev: rev, changeset: cs, apool}); const assertAccepted = async (socket:any, wantRev: number) => { await common.waitForAcceptCommit(socket, wantRev); rev = wantRev; @@ -181,7 +196,7 @@ describe(__filename, function () { it('changes are applied', async function () { await Promise.all([ assertAccepted(socket, rev + 1), - sendUserChanges(socket, 'Z:1>5+5$hello'), + sendUserChanges(socket, 'Z:1>5*0+5$hello'), ]); assert.equal(pad!.text(), 'hello\n'); }); @@ -193,8 +208,66 @@ describe(__filename, function () { ]); }); + it('insert without author attribute is rejected', async function () { + // Defensive validation: every '+' op must carry the author attrib so + // pad.atext.text and pad.atext.attribs stay in lock-step. An insert + // with empty attribs would grow text without contributing matching + // attribute markers, leaving the stored AText in a state where the + // two iterables disagree on length — downstream clients then fail + // reconciliation on every subsequent pad load. + await Promise.all([ + assertRejected(socket), + sendUserChanges(socket, 'Z:1>5+5$hello', {numToAttrib: {}, nextNum: 0}), + ]); + }); + + it('insert claiming the reserved system author is rejected', async function () { + // The author equality check above already rejects wire-borne `*N` + // that names another real user, but `a.etherpad-system` is + // server-internal — only the spliceText / setText paths use it, + // never a live socket.io session. A client that tries to write as + // the system author is either confused or trying to launder edits + // through a reserved attribution slot. Refuse. + const systemPool = { + numToAttrib: {0: ['author', 'a.etherpad-system']}, + nextNum: 1, + }; + await Promise.all([ + assertRejected(socket), + sendUserChanges(socket, 'Z:1>5*0+5$hello', systemPool), + ]); + }); + + it('changeset that would strand the trailing \\n is rejected', async function () { + // Defensive validation: every USER_CHANGES must leave the pad ending + // with '\n'. The pre-existing handler used to auto-append a + // correction revision when the trailing '\n' got stranded, but the + // first NEW_CHANGES broadcast (the malformed user revision) reached + // browsers BEFORE the correction did, and the browser's line + // assembler asserts "line assembler not finished" on a non-'\n'- + // terminated doc — kicking the watching session offline. Refuse such + // changesets up front instead. + // + // Seed the pad as 'hello\n' (6 chars, 1 line), then send a changeset + // that explicitly keeps all 6 chars (consuming the trailing '\n') + // and inserts 'X' AFTER. Projected text = 'hello\nX' — doesn't end + // with '\n', must be rejected. The keep spans 1 newline so the wire + // must carry the `|1` marker to be in canonical form. + await Promise.all([ + assertAccepted(socket, rev + 1), + sendUserChanges(socket, 'Z:1>5*0+5$hello'), + ]); + assert.equal(pad!.text(), 'hello\n'); + await Promise.all([ + assertRejected(socket), + sendUserChanges(socket, 'Z:6>1|1=6*0+1$X'), + ]); + // Pad must be unchanged after the rejection. + assert.equal(pad!.text(), 'hello\n'); + }); + it('retransmission is accepted, has no effect', async function () { - const cs = 'Z:1>5+5$hello'; + const cs = 'Z:1>5*0+5$hello'; await Promise.all([ assertAccepted(socket, rev + 1), sendUserChanges(socket, cs), @@ -210,11 +283,11 @@ describe(__filename, function () { it('identity changeset is accepted, has no effect', async function () { await Promise.all([ assertAccepted(socket, rev + 1), - sendUserChanges(socket, 'Z:1>5+5$hello'), + sendUserChanges(socket, 'Z:1>5*0+5$hello'), ]); await Promise.all([ assertAccepted(socket, rev), - sendUserChanges(socket, 'Z:6>0$'), + sendUserChanges(socket, 'Z:6>0$', {numToAttrib: {}, nextNum: 0}), ]); assert.equal(pad!.text(), 'hello\n'); }); @@ -222,20 +295,26 @@ describe(__filename, function () { it('non-identity changeset with no net change is accepted, has no effect', async function () { await Promise.all([ assertAccepted(socket, rev + 1), - sendUserChanges(socket, 'Z:1>5+5$hello'), + sendUserChanges(socket, 'Z:1>5*0+5$hello'), ]); await Promise.all([ assertAccepted(socket, rev), - sendUserChanges(socket, 'Z:6>0-5+5$hello'), + sendUserChanges(socket, 'Z:6>0-5*0+5$hello'), ]); assert.equal(pad!.text(), 'hello\n'); }); it('handleMessageSecurity can grant one-time write access', async function () { - const cs = 'Z:1>5+5$hello'; + const cs = 'Z:1>5*0+5$hello'; + // Use roSocket's own author in the apool so the *0 reference matches + // thisSession.author server-side. Without this, the new author-attrib + // validation rejects the changeset before handleMessageSecurity gets + // a chance to permit it. + const roPool = () => + ({numToAttrib: {0: ['author', roAuthorId]}, nextNum: 1}); const errRegEx = /write attempt on read-only pad/; // First try to send a change and verify that it was dropped. - await assert.rejects(sendUserChanges(roSocket, cs), errRegEx); + await assert.rejects(sendUserChanges(roSocket, cs, roPool()), errRegEx); // sendUserChanges() waits for message ack, so if the message was accepted then head should // have already incremented by the time we get here. assert.equal(pad!.head, rev); // Not incremented. @@ -244,13 +323,14 @@ describe(__filename, function () { plugins.hooks.handleMessageSecurity.push({hook_fn: () => 'permitOnce'}); await Promise.all([ assertAccepted(roSocket, rev + 1), - sendUserChanges(roSocket, cs), + sendUserChanges(roSocket, cs, roPool()), ]); assert.equal(pad!.text(), 'hello\n'); // The next change should be dropped. plugins.hooks.handleMessageSecurity = []; - await assert.rejects(sendUserChanges(roSocket, 'Z:6>6=5+6$ world'), errRegEx); + await assert.rejects( + sendUserChanges(roSocket, 'Z:6>6=5*0+6$ world', roPool()), errRegEx); assert.equal(pad!.head, rev); // Not incremented. assert.equal(pad!.text(), 'hello\n'); });