From 287a1fd4291ba50a65eff2f61bb046c86d285e7c Mon Sep 17 00:00:00 2001 From: John McLear Date: Wed, 1 Apr 2026 21:07:22 +0100 Subject: [PATCH 1/6] fix: allow undo of clear authorship colors without disconnect (#2802) When a user clears authorship colors and then undoes, the undo changeset re-applies author attributes for all authors who contributed text. The server was rejecting this because it treated any changeset containing another author's ID as impersonation, disconnecting the user. The fix distinguishes between: - '+' ops (new text): still reject if attributed to another author - '=' ops (attribute changes on existing text): allow restoring other authors' attributes, which is needed for undo of clear authorship Also removes the client-side workaround in undomodule.ts that prevented clear authorship from being undone at all, and adds backend + frontend tests covering the multi-author undo scenario. Fixes: https://github.com/ether/etherpad-lite/issues/2802 Co-Authored-By: Claude Opus 4.6 (1M context) --- src/node/handler/PadMessageHandler.ts | 24 +- src/static/js/undomodule.ts | 8 +- .../backend/specs/undo_clear_authorship.ts | 303 ++++++++++++++++++ .../specs/clear_authorship_color.spec.ts | 22 +- .../specs/undo_clear_authorship.spec.ts | 151 +++++++++ 5 files changed, 489 insertions(+), 19 deletions(-) create mode 100644 src/tests/backend/specs/undo_clear_authorship.ts create mode 100644 src/tests/frontend-new/specs/undo_clear_authorship.spec.ts diff --git a/src/node/handler/PadMessageHandler.ts b/src/node/handler/PadMessageHandler.ts index 080ab044a0c..cd60aed87c5 100644 --- a/src/node/handler/PadMessageHandler.ts +++ b/src/node/handler/PadMessageHandler.ts @@ -652,7 +652,12 @@ const handleUserChanges = async (socket:any, message: { // Verify that the changeset has valid syntax and is in canonical form checkRep(changeset); - // Validate all added 'author' attribs to be the same value as the current user + // Validate all added 'author' attribs to be the same value as the current user. + // Exception: '=' ops (attribute changes on existing text) are allowed to restore other authors' + // IDs, but only if that author already exists in the pad's pool (i.e., they genuinely + // contributed to this pad). This is necessary for undoing "clear authorship colors", which + // re-applies the original author attributes for all authors. + // See https://github.com/ether/etherpad-lite/issues/2802 for (const op of deserializeOps(unpack(changeset).ops)) { // + can add text with attribs // = can change or add attribs @@ -664,8 +669,21 @@ const handleUserChanges = async (socket:any, message: { // an attribute number isn't in the pool). const opAuthorId = AttributeMap.fromString(op.attribs, wireApool).get('author'); if (opAuthorId && opAuthorId !== thisSession.author) { - throw new Error(`Author ${thisSession.author} tried to submit changes as author ` + - `${opAuthorId} in changeset ${changeset}`); + if (op.opcode === '+') { + // Inserting new text as another author is always impersonation. + throw new Error(`Author ${thisSession.author} tried to submit changes as author ` + + `${opAuthorId} in changeset ${changeset}`); + } + if (op.opcode === '=') { + // Allow restoring author attributes on existing text (undo of clear authorship), + // but only if the author ID is already known to this pad. This prevents a user + // from attributing text to a fabricated author who never contributed to the pad. + const knownAuthor = pad.pool.putAttrib(['author', opAuthorId], true) !== -1; + if (!knownAuthor) { + throw new Error(`Author ${thisSession.author} tried to set unknown author ` + + `${opAuthorId} on existing text in changeset ${changeset}`); + } + } } } diff --git a/src/static/js/undomodule.ts b/src/static/js/undomodule.ts index c3bdd2fab9d..542fb7157ff 100644 --- a/src/static/js/undomodule.ts +++ b/src/static/js/undomodule.ts @@ -212,13 +212,7 @@ const undoModule = (() => { } } if (!merged) { - /* - * Push the event on the undo stack only if it exists, and if it's - * not a "clearauthorship". This disallows undoing the removal of the - * authorship colors, but is a necessary stopgap measure against - * https://github.com/ether/etherpad-lite/issues/2802 - */ - if (event && (event.eventType !== 'clearauthorship')) { + if (event) { stack.pushEvent(event); } } diff --git a/src/tests/backend/specs/undo_clear_authorship.ts b/src/tests/backend/specs/undo_clear_authorship.ts new file mode 100644 index 00000000000..34e36c74e91 --- /dev/null +++ b/src/tests/backend/specs/undo_clear_authorship.ts @@ -0,0 +1,303 @@ +'use strict'; + +/** + * Tests for https://github.com/ether/etherpad-lite/issues/2802 + * + * When User B clears authorship colors (removing all author attributes) and then undoes, + * the undo changeset re-applies author attributes for ALL authors (A and B). The server + * rejects this because User B is submitting changes containing User A's author ID, causing + * a disconnect with "badChangeset". + * + * The server should allow undo of clear authorship without disconnecting the user. + */ + +import {PadType} from "../../../node/types/PadType"; + +const assert = require('assert').strict; +const common = require('../common'); +const padManager = require('../../../node/db/PadManager'); +import AttributePool from '../../../static/js/AttributePool'; +import padutils from '../../../static/js/pad_utils'; + +describe(__filename, function () { + let agent: any; + let pad: PadType | null; + let padId: string; + let socketA: any; + let socketB: any; + let revA: number; + let revB: number; + + before(async function () { + agent = await common.init(); + }); + + beforeEach(async function () { + padId = common.randomString(); + assert(!await padManager.doesPadExist(padId)); + pad = await padManager.getPad(padId, ''); + await pad!.setText('\n'); + assert.equal(pad!.text(), '\n'); + }); + + afterEach(async function () { + if (socketA != null) socketA.close(); + socketA = null; + if (socketB != null) socketB.close(); + socketB = null; + if (pad != null) await pad.remove(); + pad = null; + }); + + /** + * Connect a user to the pad with a unique author token. + */ + const connectUser = async () => { + const res = await agent.get(`/p/${padId}`).expect(200); + const socket = await common.connect(res); + const token = padutils.generateAuthorToken(); + const {type, data: clientVars} = await common.handshake(socket, padId, token); + assert.equal(type, 'CLIENT_VARS'); + const rev = clientVars.collab_client_vars.rev; + const author = clientVars.userId; + return {socket, rev, author}; + }; + + const sendUserChanges = async (socket: any, baseRev: number, changeset: string, apool?: any) => { + await common.sendUserChanges(socket, { + baseRev, + changeset, + ...(apool ? {apool} : {}), + }); + }; + + /** + * Wait for an ACCEPT_COMMIT message, skipping any other COLLABROOM messages + * (like USER_NEWINFO, NEW_CHANGES, etc.) that may arrive first. + */ + const waitForAcceptCommit = async (socket: any, wantRev: number) => { + for (;;) { + const msg = await common.waitForSocketEvent(socket, 'message'); + if (msg.disconnect) { + throw new Error(`Unexpected disconnect: ${JSON.stringify(msg)}`); + } + if (msg.type === 'COLLABROOM' && msg.data?.type === 'ACCEPT_COMMIT') { + assert.equal(msg.data.newRev, wantRev); + return; + } + // Skip non-ACCEPT_COMMIT messages (USER_NEWINFO, NEW_CHANGES, etc.) + } + }; + + /** + * Drain messages from a socket until we get an ACCEPT_COMMIT or disconnect. + * Returns the message for assertion. + */ + const waitForNextCommitOrDisconnect = async (socket: any): Promise => { + for (;;) { + const msg = await common.waitForSocketEvent(socket, 'message'); + if (msg.disconnect) return msg; + if (msg.type === 'COLLABROOM' && msg.data?.type === 'ACCEPT_COMMIT') return msg; + // Skip USER_NEWINFO, NEW_CHANGES, etc. + } + }; + + /** + * Drain non-ACCEPT_COMMIT messages so the socket is ready for the next operation. + * Waits briefly then consumes any queued messages. + */ + const drainMessages = async (socket: any) => { + await new Promise(resolve => setTimeout(resolve, 500)); + }; + + describe('undo of clear authorship colors (bug #2802)', function () { + it('should not disconnect when undoing clear authorship with multiple authors', async function () { + this.timeout(30000); + + // Step 1: Connect User A + const userA = await connectUser(); + socketA = userA.socket; + revA = userA.rev; + + // Step 2: User A types "hello" with their author attribute + const apoolA = new AttributePool(); + apoolA.putAttrib(['author', userA.author]); + await Promise.all([ + waitForAcceptCommit(socketA, revA + 1), + sendUserChanges(socketA, revA, 'Z:1>5*0+5$hello', apoolA), + ]); + revA += 1; + + // Step 3: Connect User B (after User A's text is committed) + await drainMessages(socketA); + const userB = await connectUser(); + socketB = userB.socket; + revB = userB.rev; + // User B joins and sees the pad at the current head revision + await drainMessages(socketA); + + // Step 4: User B types " world" with their author attribute + const apoolB = new AttributePool(); + apoolB.putAttrib(['author', userB.author]); + await Promise.all([ + waitForAcceptCommit(socketB, revB + 1), + sendUserChanges(socketB, revB, 'Z:6>6=5*0+6$ world', apoolB), + ]); + revB += 1; + + // Wait for User A to see the change + await drainMessages(socketA); + revA = revB; + + // The pad now has "hello world\n" with two different authors + assert.equal(pad!.text(), 'hello world\n'); + + // Step 5: User B clears authorship colors (sets author to '' on all text) + const clearPool = new AttributePool(); + clearPool.putAttrib(['author', '']); + await Promise.all([ + waitForAcceptCommit(socketB, revB + 1), + sendUserChanges(socketB, revB, 'Z:c>0*0=b$', clearPool), + ]); + revB += 1; + await drainMessages(socketA); + revA = revB; + + // Step 6: User B undoes the clear authorship + // This is the critical part - the undo changeset re-applies the original + // author attributes, which include User A's author ID. + // The server currently rejects this because User B is submitting changes + // with User A's author ID. + const undoPool = new AttributePool(); + undoPool.putAttrib(['author', userA.author]); // 0 = author A + undoPool.putAttrib(['author', userB.author]); // 1 = author B + // Undo restores: "hello" with author A (5 chars), " world" with author B (6 chars) + const undoChangeset = 'Z:c>0*0=5*1=6$'; + + // This should NOT disconnect User B - that's the bug (#2802) + const result = await Promise.all([ + waitForNextCommitOrDisconnect(socketB), + sendUserChanges(socketB, revB, undoChangeset, undoPool), + ]); + + const msg = result[0]; + assert.notDeepEqual(msg, {disconnect: 'badChangeset'}, + 'User was disconnected with badChangeset - bug #2802'); + assert.equal(msg.type, 'COLLABROOM'); + assert.equal(msg.data.type, 'ACCEPT_COMMIT'); + }); + + it('should allow clear authorship changeset with empty author from any user', async function () { + // Connect one user, write text, then clear authorship + const userA = await connectUser(); + socketA = userA.socket; + revA = userA.rev; + + // User A types text + const apoolA = new AttributePool(); + apoolA.putAttrib(['author', userA.author]); + await Promise.all([ + waitForAcceptCommit(socketA, revA + 1), + sendUserChanges(socketA, revA, 'Z:1>5*0+5$hello', apoolA), + ]); + revA += 1; + + // User A clears authorship (sets author='') + // This should always be allowed since author='' is not impersonation + const clearPool = new AttributePool(); + clearPool.putAttrib(['author', '']); + await Promise.all([ + waitForAcceptCommit(socketA, revA + 1), + sendUserChanges(socketA, revA, 'Z:6>0*0=5$', clearPool), + ]); + }); + + it('changeset restoring own author after clear should be accepted', async function () { + // User clears their own authorship and then undoes (restoring own author attr) + const userA = await connectUser(); + socketA = userA.socket; + revA = userA.rev; + + // User A types text with author attribute + const apoolA = new AttributePool(); + apoolA.putAttrib(['author', userA.author]); + await Promise.all([ + waitForAcceptCommit(socketA, revA + 1), + sendUserChanges(socketA, revA, 'Z:1>5*0+5$hello', apoolA), + ]); + revA += 1; + + // User A clears authorship (sets author='') + const clearPool = new AttributePool(); + clearPool.putAttrib(['author', '']); + await Promise.all([ + waitForAcceptCommit(socketA, revA + 1), + sendUserChanges(socketA, revA, 'Z:6>0*0=5$', clearPool), + ]); + revA += 1; + + // User A undoes the clear - restoring their own author attribute + // This should be accepted since it's their own author ID + await Promise.all([ + waitForAcceptCommit(socketA, revA + 1), + sendUserChanges(socketA, revA, 'Z:6>0*0=5$', apoolA), + ]); + }); + + it('changeset impersonating another author for new text should still be rejected', async function () { + // Security: a user should NOT be able to write NEW text attributed to another author + const userA = await connectUser(); + socketA = userA.socket; + revA = userA.rev; + + await drainMessages(socketA); + + const userB = await connectUser(); + socketB = userB.socket; + revB = userB.rev; + + await drainMessages(socketA); + + // User B tries to insert text attributed to User A - this should be rejected + const fakePool = new AttributePool(); + fakePool.putAttrib(['author', userA.author]); + + const result = await Promise.all([ + waitForNextCommitOrDisconnect(socketB), + sendUserChanges(socketB, revB, 'Z:1>5*0+5$hello', fakePool), + ]); + + assert.deepEqual(result[0], {disconnect: 'badChangeset'}, + 'Should reject changeset that impersonates another author for new text'); + }); + + it('should reject = op with fabricated author who never contributed to the pad', async function () { + // Security: even for = ops, reject author IDs that don't exist in the pad's pool. + // This prevents attributing text to users who never touched the pad. + const userA = await connectUser(); + socketA = userA.socket; + revA = userA.rev; + + // User A types text + const apoolA = new AttributePool(); + apoolA.putAttrib(['author', userA.author]); + await Promise.all([ + waitForAcceptCommit(socketA, revA + 1), + sendUserChanges(socketA, revA, 'Z:1>5*0+5$hello', apoolA), + ]); + revA += 1; + + // User A tries to set a fabricated author ID on existing text via a = op + const fakePool = new AttributePool(); + fakePool.putAttrib(['author', 'a.fabricatedAuthorId']); + + const result = await Promise.all([ + waitForNextCommitOrDisconnect(socketA), + sendUserChanges(socketA, revA, 'Z:6>0*0=5$', fakePool), + ]); + + assert.deepEqual(result[0], {disconnect: 'badChangeset'}, + 'Should reject = op with fabricated author not in pad pool'); + }); + }); +}); diff --git a/src/tests/frontend-new/specs/clear_authorship_color.spec.ts b/src/tests/frontend-new/specs/clear_authorship_color.spec.ts index 961deb4f4fc..179aec37f81 100644 --- a/src/tests/frontend-new/specs/clear_authorship_color.spec.ts +++ b/src/tests/frontend-new/specs/clear_authorship_color.spec.ts @@ -35,7 +35,10 @@ test('clear authorship color', async ({page}) => { }) -test("makes text clear authorship colors and checks it can't be undone", async function ({page}) { +test("clear authorship colors can be undone to restore author colors", async function ({page}) { + // Fix for https://github.com/ether/etherpad-lite/issues/2802 + // Previously, undo of clear authorship was blocked as a workaround. + // Now the server properly allows it, so undo should restore author colors. const innnerPad = await getPadBody(page); const padText = "Hello" @@ -48,19 +51,20 @@ test("makes text clear authorship colors and checks it can't be undone", async f const retrievedClasses = await innnerPad.locator('div span').nth(0).getAttribute('class') expect(retrievedClasses).toContain('author'); - await firstDivClass.focus() await clearAuthorship(page); expect(await firstDivClass.getAttribute('class')).not.toContain('author'); + // Undo should restore authorship colors await undoChanges(page); - const changedFirstDiv = innnerPad.locator('div').nth(0) - expect(await changedFirstDiv.getAttribute('class')).not.toContain('author'); - - - await pressUndoButton(page); - const secondChangedFirstDiv = innnerPad.locator('div').nth(0) - expect(await secondChangedFirstDiv.getAttribute('class')).not.toContain('author'); + await page.waitForTimeout(1000); + const spanAfterUndo = innnerPad.locator('div span').nth(0); + const classesAfterUndo = await spanAfterUndo.getAttribute('class'); + expect(classesAfterUndo).toContain('author'); + + // User should not be disconnected + const disconnected = page.locator('.disconnected, .unreachable'); + expect(await disconnected.isVisible()).toBe(false); }); diff --git a/src/tests/frontend-new/specs/undo_clear_authorship.spec.ts b/src/tests/frontend-new/specs/undo_clear_authorship.spec.ts new file mode 100644 index 00000000000..9c7c20cb326 --- /dev/null +++ b/src/tests/frontend-new/specs/undo_clear_authorship.spec.ts @@ -0,0 +1,151 @@ +import {expect, test} from "@playwright/test"; +import { + clearAuthorship, + clearPadContent, + getPadBody, + goToNewPad, + goToPad, + selectAllText, + undoChanges, + writeToPad +} from "../helper/padHelper"; + +/** + * Tests for https://github.com/ether/etherpad-lite/issues/2802 + * + * Reproduction steps from the bug report: + * 1. User A logs in, enables author colors, types something + * 2. User B logs in to same pad, enables author colors, types something + * 3. User B clicks "clear authorship colors" + * 4. User B clicks "undo" + * => User B is disconnected from the pad + * + * The undo of clear authorship re-applies author attributes for all authors, + * but the server rejects it because User B is submitting changes containing + * User A's author ID. + */ +test.describe('undo clear authorship colors with multiple authors (bug #2802)', function () { + let padId: string; + + test('User B should not be disconnected after undoing clear authorship', async function ({browser}) { + // User 1 creates a pad and types text + const context1 = await browser.newContext(); + const page1 = await context1.newPage(); + padId = await goToNewPad(page1); + const body1 = await getPadBody(page1); + await body1.click(); + await clearPadContent(page1); + await writeToPad(page1, 'Hello from User A'); + + // Wait for text to be committed + await page1.waitForTimeout(1000); + + // Verify User A's text has authorship + const user1Span = body1.locator('div').first().locator('span'); + await expect(user1Span.first()).toHaveAttribute('class', /author-/); + + // User 2 joins the same pad in a different browser context (different author) + const context2 = await browser.newContext(); + const page2 = await context2.newPage(); + await goToPad(page2, padId); + const body2 = await getPadBody(page2); + + // Wait for User A's text to appear for User B + await expect(body2.locator('div').first()).toContainText('Hello from User A'); + + // User B types on a new line + await body2.click(); + await page2.keyboard.press('End'); + await page2.keyboard.press('Enter'); + await page2.keyboard.type('Hello from User B'); + + // Wait for sync + await page2.waitForTimeout(1000); + + // Both users should see both lines + await expect(body1.locator('div').nth(1)).toContainText('Hello from User B'); + + // Verify we have authorship colors from two different authors + const authorSpans = body2.locator('[class*="author-"]'); + await expect(authorSpans.first()).toBeVisible(); + + // User B clears authorship colors + await body2.click(); + await selectAllText(page2); + await clearAuthorship(page2); + + // Wait for clear to propagate + await page2.waitForTimeout(1000); + + // Verify authorship is cleared + const clearedBody = await getPadBody(page2); + const authorClassesAfterClear = clearedBody.locator('[class*="author-"]'); + // After clearing, there should be no author classes + await expect(authorClassesAfterClear).toHaveCount(0); + + // THIS IS THE BUG: User B undoes the clear authorship + // Currently, the undo is blocked client-side as a workaround. + // The proper fix should allow the undo without causing a disconnect. + await undoChanges(page2); + + // Wait for the undo to take effect + await page2.waitForTimeout(2000); + + // User B should NOT be disconnected + const disconnectedBanner = page2.locator('.disconnected, .unreachable'); + await expect(disconnectedBanner).not.toBeVisible(); + + // The authorship colors should be restored after undo + const restoredAuthorSpans = clearedBody.locator('[class*="author-"]'); + await expect(restoredAuthorSpans.first()).toBeVisible({timeout: 5000}); + + // User B should still be able to type (not disconnected) + await body2.click(); + await page2.keyboard.press('End'); + await page2.keyboard.press('Enter'); + await page2.keyboard.type('Still connected!'); + + await page2.waitForTimeout(1000); + + // The text should appear for User A too (proves User B is still connected and syncing) + await expect(body1.locator('div').nth(2)).toContainText('Still connected!'); + + // Cleanup + await context1.close(); + await context2.close(); + }); + + test('single user can undo clear authorship without disconnect', async function ({page}) { + // Even with a single user, undo of clear authorship should work + await goToNewPad(page); + const body = await getPadBody(page); + await body.click(); + await clearPadContent(page); + await writeToPad(page, 'Some text with authorship'); + + await page.waitForTimeout(500); + + // Verify authorship exists + const authorSpan = body.locator('[class*="author-"]'); + await expect(authorSpan.first()).toBeVisible(); + + // Clear authorship + await selectAllText(page); + await clearAuthorship(page); + await page.waitForTimeout(500); + + // Verify cleared + await expect(body.locator('[class*="author-"]')).toHaveCount(0); + + // Undo - currently blocked by the workaround, should work with proper fix + await undoChanges(page); + await page.waitForTimeout(1000); + + // Should not be disconnected + const disconnectedBanner = page.locator('.disconnected, .unreachable'); + await expect(disconnectedBanner).not.toBeVisible(); + + // Authorship should be restored + await expect(body.locator('[class*="author-"]').first()).toBeVisible({timeout: 5000}); + }); +}); From d159738d13f8b261777f408cd9bc39837dd55ba7 Mon Sep 17 00:00:00 2001 From: John McLear Date: Wed, 1 Apr 2026 21:35:33 +0100 Subject: [PATCH 2/6] fix: use robust Playwright assertions in authorship undo tests - Use toHaveAttribute with regex instead of raw getAttribute + toContain - Check div/span attributes within pad body instead of broad selectors - Use Playwright auto-retry (expect with timeout) instead of toHaveCount(0) Co-Authored-By: Claude Opus 4.6 (1M context) --- .../specs/clear_authorship_color.spec.ts | 25 ++++++------- .../specs/undo_clear_authorship.spec.ts | 37 +++++-------------- 2 files changed, 22 insertions(+), 40 deletions(-) diff --git a/src/tests/frontend-new/specs/clear_authorship_color.spec.ts b/src/tests/frontend-new/specs/clear_authorship_color.spec.ts index 179aec37f81..bff46d4af1c 100644 --- a/src/tests/frontend-new/specs/clear_authorship_color.spec.ts +++ b/src/tests/frontend-new/specs/clear_authorship_color.spec.ts @@ -39,32 +39,31 @@ test("clear authorship colors can be undone to restore author colors", async fun // Fix for https://github.com/ether/etherpad-lite/issues/2802 // Previously, undo of clear authorship was blocked as a workaround. // Now the server properly allows it, so undo should restore author colors. - const innnerPad = await getPadBody(page); + const padBody = await getPadBody(page); const padText = "Hello" // type some text await clearPadContent(page); await writeToPad(page, padText); - // get the first text element out of the inner iframe - const firstDivClass = innnerPad.locator('div').nth(0) - const retrievedClasses = await innnerPad.locator('div span').nth(0).getAttribute('class') - expect(retrievedClasses).toContain('author'); + // verify authorship exists on the span + const span = padBody.locator('div span').nth(0); + await expect(span).toHaveAttribute('class', /author-/); - await firstDivClass.focus() + await padBody.locator('div').nth(0).focus() + await selectAllText(page); await clearAuthorship(page); - expect(await firstDivClass.getAttribute('class')).not.toContain('author'); + + // verify authorship is cleared + await expect(padBody.locator('div').nth(0)).not.toHaveAttribute('class', /author/); // Undo should restore authorship colors await undoChanges(page); - await page.waitForTimeout(1000); - const spanAfterUndo = innnerPad.locator('div span').nth(0); - const classesAfterUndo = await spanAfterUndo.getAttribute('class'); - expect(classesAfterUndo).toContain('author'); - // User should not be disconnected + // verify authorship is restored and user is not disconnected + await expect(padBody.locator('div span').nth(0)).toHaveAttribute('class', /author-/, {timeout: 5000}); const disconnected = page.locator('.disconnected, .unreachable'); - expect(await disconnected.isVisible()).toBe(false); + await expect(disconnected).not.toBeVisible(); }); diff --git a/src/tests/frontend-new/specs/undo_clear_authorship.spec.ts b/src/tests/frontend-new/specs/undo_clear_authorship.spec.ts index 9c7c20cb326..8c7d2d70f2c 100644 --- a/src/tests/frontend-new/specs/undo_clear_authorship.spec.ts +++ b/src/tests/frontend-new/specs/undo_clear_authorship.spec.ts @@ -41,8 +41,7 @@ test.describe('undo clear authorship colors with multiple authors (bug #2802)', await page1.waitForTimeout(1000); // Verify User A's text has authorship - const user1Span = body1.locator('div').first().locator('span'); - await expect(user1Span.first()).toHaveAttribute('class', /author-/); + await expect(body1.locator('div span').first()).toHaveAttribute('class', /author-/); // User 2 joins the same pad in a different browser context (different author) const context2 = await browser.newContext(); @@ -66,38 +65,25 @@ test.describe('undo clear authorship colors with multiple authors (bug #2802)', await expect(body1.locator('div').nth(1)).toContainText('Hello from User B'); // Verify we have authorship colors from two different authors - const authorSpans = body2.locator('[class*="author-"]'); - await expect(authorSpans.first()).toBeVisible(); + await expect(body2.locator('div span').first()).toHaveAttribute('class', /author-/); // User B clears authorship colors await body2.click(); await selectAllText(page2); await clearAuthorship(page2); - // Wait for clear to propagate - await page2.waitForTimeout(1000); - - // Verify authorship is cleared - const clearedBody = await getPadBody(page2); - const authorClassesAfterClear = clearedBody.locator('[class*="author-"]'); - // After clearing, there should be no author classes - await expect(authorClassesAfterClear).toHaveCount(0); + // Wait for clear to propagate and verify authorship is cleared on the div + await expect(body2.locator('div').first()).not.toHaveAttribute('class', /author/, {timeout: 5000}); // THIS IS THE BUG: User B undoes the clear authorship - // Currently, the undo is blocked client-side as a workaround. - // The proper fix should allow the undo without causing a disconnect. await undoChanges(page2); - // Wait for the undo to take effect - await page2.waitForTimeout(2000); - // User B should NOT be disconnected const disconnectedBanner = page2.locator('.disconnected, .unreachable'); await expect(disconnectedBanner).not.toBeVisible(); // The authorship colors should be restored after undo - const restoredAuthorSpans = clearedBody.locator('[class*="author-"]'); - await expect(restoredAuthorSpans.first()).toBeVisible({timeout: 5000}); + await expect(body2.locator('div span').first()).toHaveAttribute('class', /author-/, {timeout: 5000}); // User B should still be able to type (not disconnected) await body2.click(); @@ -126,26 +112,23 @@ test.describe('undo clear authorship colors with multiple authors (bug #2802)', await page.waitForTimeout(500); // Verify authorship exists - const authorSpan = body.locator('[class*="author-"]'); - await expect(authorSpan.first()).toBeVisible(); + await expect(body.locator('div span').first()).toHaveAttribute('class', /author-/); // Clear authorship await selectAllText(page); await clearAuthorship(page); - await page.waitForTimeout(500); - // Verify cleared - await expect(body.locator('[class*="author-"]')).toHaveCount(0); + // Verify cleared - check the div, not a broad selector + await expect(body.locator('div').first()).not.toHaveAttribute('class', /author/, {timeout: 5000}); - // Undo - currently blocked by the workaround, should work with proper fix + // Undo should restore authorship await undoChanges(page); - await page.waitForTimeout(1000); // Should not be disconnected const disconnectedBanner = page.locator('.disconnected, .unreachable'); await expect(disconnectedBanner).not.toBeVisible(); // Authorship should be restored - await expect(body.locator('[class*="author-"]').first()).toBeVisible({timeout: 5000}); + await expect(body.locator('div span').first()).toHaveAttribute('class', /author-/, {timeout: 5000}); }); }); From 3bc8bd1c09bccc895d08c7f2b4395544127b2821 Mon Sep 17 00:00:00 2001 From: John McLear Date: Wed, 1 Apr 2026 22:02:13 +0100 Subject: [PATCH 3/6] fix: handle confirm dialog and sync timing in Playwright tests - Add page.on('dialog') handler to accept the confirm dialog triggered by clearAuthorship when no text is selected (clears whole pad) - Use auto-retrying toHaveAttribute assertions instead of raw getAttribute - Increase cross-user sync timeouts to 15s for CI reliability - Add retries: 2 to multi-user test for CI flakiness - Scope assertions to pad body spans instead of broad selectors Co-Authored-By: Claude Opus 4.6 (1M context) --- .../specs/clear_authorship_color.spec.ts | 14 ++++---- .../specs/undo_clear_authorship.spec.ts | 33 +++++++++---------- 2 files changed, 24 insertions(+), 23 deletions(-) diff --git a/src/tests/frontend-new/specs/clear_authorship_color.spec.ts b/src/tests/frontend-new/specs/clear_authorship_color.spec.ts index bff46d4af1c..f97f9e280d7 100644 --- a/src/tests/frontend-new/specs/clear_authorship_color.spec.ts +++ b/src/tests/frontend-new/specs/clear_authorship_color.spec.ts @@ -47,21 +47,23 @@ test("clear authorship colors can be undone to restore author colors", async fun await writeToPad(page, padText); // verify authorship exists on the span - const span = padBody.locator('div span').nth(0); - await expect(span).toHaveAttribute('class', /author-/); + await expect(padBody.locator('div span').first()).toHaveAttribute('class', /author-/); - await padBody.locator('div').nth(0).focus() - await selectAllText(page); + // Accept the confirm dialog triggered by clearAuthorship when no text is selected + page.on('dialog', dialog => dialog.accept()); + + // Click somewhere in the pad to deselect, then clear (triggers whole-pad clear via confirm) + await padBody.click(); await clearAuthorship(page); // verify authorship is cleared - await expect(padBody.locator('div').nth(0)).not.toHaveAttribute('class', /author/); + await expect(padBody.locator('div span').first()).toHaveAttribute('class', /^(?!.*author-)/, {timeout: 5000}); // Undo should restore authorship colors await undoChanges(page); // verify authorship is restored and user is not disconnected - await expect(padBody.locator('div span').nth(0)).toHaveAttribute('class', /author-/, {timeout: 5000}); + await expect(padBody.locator('div span').first()).toHaveAttribute('class', /author-/, {timeout: 5000}); const disconnected = page.locator('.disconnected, .unreachable'); await expect(disconnected).not.toBeVisible(); }); diff --git a/src/tests/frontend-new/specs/undo_clear_authorship.spec.ts b/src/tests/frontend-new/specs/undo_clear_authorship.spec.ts index 8c7d2d70f2c..099624405a9 100644 --- a/src/tests/frontend-new/specs/undo_clear_authorship.spec.ts +++ b/src/tests/frontend-new/specs/undo_clear_authorship.spec.ts @@ -25,6 +25,7 @@ import { * User A's author ID. */ test.describe('undo clear authorship colors with multiple authors (bug #2802)', function () { + test.describe.configure({ retries: 2 }); let padId: string; test('User B should not be disconnected after undoing clear authorship', async function ({browser}) { @@ -50,7 +51,7 @@ test.describe('undo clear authorship colors with multiple authors (bug #2802)', const body2 = await getPadBody(page2); // Wait for User A's text to appear for User B - await expect(body2.locator('div').first()).toContainText('Hello from User A'); + await expect(body2.locator('div').first()).toContainText('Hello from User A', {timeout: 10000}); // User B types on a new line await body2.click(); @@ -58,22 +59,20 @@ test.describe('undo clear authorship colors with multiple authors (bug #2802)', await page2.keyboard.press('Enter'); await page2.keyboard.type('Hello from User B'); - // Wait for sync - await page2.waitForTimeout(1000); - // Both users should see both lines - await expect(body1.locator('div').nth(1)).toContainText('Hello from User B'); + await expect(body1.locator('div').nth(1)).toContainText('Hello from User B', {timeout: 15000}); // Verify we have authorship colors from two different authors await expect(body2.locator('div span').first()).toHaveAttribute('class', /author-/); - // User B clears authorship colors - await body2.click(); - await selectAllText(page2); + // Accept the confirm dialog that clearAuthorship triggers + page2.on('dialog', dialog => dialog.accept()); + + // User B clears authorship colors (without selecting - clears whole pad) await clearAuthorship(page2); - // Wait for clear to propagate and verify authorship is cleared on the div - await expect(body2.locator('div').first()).not.toHaveAttribute('class', /author/, {timeout: 5000}); + // Wait for clear to propagate and verify authorship is cleared + await expect(body2.locator('div span').first()).toHaveAttribute('class', /^(?!.*author-)/, {timeout: 5000}); // THIS IS THE BUG: User B undoes the clear authorship await undoChanges(page2); @@ -91,10 +90,8 @@ test.describe('undo clear authorship colors with multiple authors (bug #2802)', await page2.keyboard.press('Enter'); await page2.keyboard.type('Still connected!'); - await page2.waitForTimeout(1000); - // The text should appear for User A too (proves User B is still connected and syncing) - await expect(body1.locator('div').nth(2)).toContainText('Still connected!'); + await expect(body1.locator('div').nth(2)).toContainText('Still connected!', {timeout: 15000}); // Cleanup await context1.close(); @@ -114,12 +111,14 @@ test.describe('undo clear authorship colors with multiple authors (bug #2802)', // Verify authorship exists await expect(body.locator('div span').first()).toHaveAttribute('class', /author-/); - // Clear authorship - await selectAllText(page); + // Accept the confirm dialog + page.on('dialog', dialog => dialog.accept()); + + // Clear authorship (no selection - clears whole pad) await clearAuthorship(page); - // Verify cleared - check the div, not a broad selector - await expect(body.locator('div').first()).not.toHaveAttribute('class', /author/, {timeout: 5000}); + // Verify cleared + await expect(body.locator('div span').first()).toHaveAttribute('class', /^(?!.*author-)/, {timeout: 5000}); // Undo should restore authorship await undoChanges(page); From 81457c59bc93f2c28a4f8ec73000b7a8ce255f23 Mon Sep 17 00:00:00 2001 From: John McLear Date: Wed, 1 Apr 2026 22:07:40 +0100 Subject: [PATCH 4/6] fix: use persistent socket listeners to avoid missing messages in CI Replace sequential waitForSocketEvent loops with single persistent listeners that filter messages inline. This prevents race conditions where messages arrive between off/on listener cycles, causing timeouts on slower CI runners. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../backend/specs/undo_clear_authorship.ts | 113 +++++++++--------- 1 file changed, 57 insertions(+), 56 deletions(-) diff --git a/src/tests/backend/specs/undo_clear_authorship.ts b/src/tests/backend/specs/undo_clear_authorship.ts index 34e36c74e91..2bd814c955a 100644 --- a/src/tests/backend/specs/undo_clear_authorship.ts +++ b/src/tests/backend/specs/undo_clear_authorship.ts @@ -72,42 +72,57 @@ describe(__filename, function () { }; /** - * Wait for an ACCEPT_COMMIT message, skipping any other COLLABROOM messages - * (like USER_NEWINFO, NEW_CHANGES, etc.) that may arrive first. + * Wait for an ACCEPT_COMMIT or disconnect message, ignoring other messages. + * Uses a single persistent listener to avoid missing messages between on/off cycles. */ - const waitForAcceptCommit = async (socket: any, wantRev: number) => { - for (;;) { - const msg = await common.waitForSocketEvent(socket, 'message'); - if (msg.disconnect) { - throw new Error(`Unexpected disconnect: ${JSON.stringify(msg)}`); - } - if (msg.type === 'COLLABROOM' && msg.data?.type === 'ACCEPT_COMMIT') { - assert.equal(msg.data.newRev, wantRev); - return; - } - // Skip non-ACCEPT_COMMIT messages (USER_NEWINFO, NEW_CHANGES, etc.) - } + const waitForCommitOrDisconnect = (socket: any, timeoutMs = 10000): Promise => { + return new Promise((resolve, reject) => { + const timeout = setTimeout(() => { + socket.off('message', handler); + reject(new Error(`timed out waiting for ACCEPT_COMMIT or disconnect after ${timeoutMs}ms`)); + }, timeoutMs); + const handler = (msg: any) => { + if (msg.disconnect) { + clearTimeout(timeout); + socket.off('message', handler); + resolve(msg); + } else if (msg.type === 'COLLABROOM' && msg.data?.type === 'ACCEPT_COMMIT') { + clearTimeout(timeout); + socket.off('message', handler); + resolve(msg); + } + // Ignore USER_NEWINFO, NEW_CHANGES, etc. + }; + socket.on('message', handler); + }); }; - /** - * Drain messages from a socket until we get an ACCEPT_COMMIT or disconnect. - * Returns the message for assertion. - */ - const waitForNextCommitOrDisconnect = async (socket: any): Promise => { - for (;;) { - const msg = await common.waitForSocketEvent(socket, 'message'); - if (msg.disconnect) return msg; - if (msg.type === 'COLLABROOM' && msg.data?.type === 'ACCEPT_COMMIT') return msg; - // Skip USER_NEWINFO, NEW_CHANGES, etc. + const waitForAcceptCommit = async (socket: any, wantRev: number) => { + const msg = await waitForCommitOrDisconnect(socket); + if (msg.disconnect) { + throw new Error(`Unexpected disconnect: ${JSON.stringify(msg)}`); } + assert.equal(msg.data.newRev, wantRev); }; /** - * Drain non-ACCEPT_COMMIT messages so the socket is ready for the next operation. - * Waits briefly then consumes any queued messages. + * Wait for a specific message type, ignoring others. Used for cross-user sync. */ - const drainMessages = async (socket: any) => { - await new Promise(resolve => setTimeout(resolve, 500)); + const waitForNewChanges = (socket: any, timeoutMs = 10000): Promise => { + return new Promise((resolve, reject) => { + const timeout = setTimeout(() => { + socket.off('message', handler); + reject(new Error(`timed out waiting for NEW_CHANGES after ${timeoutMs}ms`)); + }, timeoutMs); + const handler = (msg: any) => { + if (msg.type === 'COLLABROOM' && msg.data?.type === 'NEW_CHANGES') { + clearTimeout(timeout); + socket.off('message', handler); + resolve(); + } + }; + socket.on('message', handler); + }); }; describe('undo of clear authorship colors (bug #2802)', function () { @@ -129,24 +144,22 @@ describe(__filename, function () { revA += 1; // Step 3: Connect User B (after User A's text is committed) - await drainMessages(socketA); const userB = await connectUser(); socketB = userB.socket; revB = userB.rev; - // User B joins and sees the pad at the current head revision - await drainMessages(socketA); // Step 4: User B types " world" with their author attribute const apoolB = new AttributePool(); apoolB.putAttrib(['author', userB.author]); + const userASeesB = waitForNewChanges(socketA); await Promise.all([ waitForAcceptCommit(socketB, revB + 1), sendUserChanges(socketB, revB, 'Z:6>6=5*0+6$ world', apoolB), ]); revB += 1; - // Wait for User A to see the change - await drainMessages(socketA); + // Wait for User A to see User B's change + await userASeesB; revA = revB; // The pad now has "hello world\n" with two different authors @@ -160,14 +173,10 @@ describe(__filename, function () { sendUserChanges(socketB, revB, 'Z:c>0*0=b$', clearPool), ]); revB += 1; - await drainMessages(socketA); - revA = revB; // Step 6: User B undoes the clear authorship // This is the critical part - the undo changeset re-applies the original // author attributes, which include User A's author ID. - // The server currently rejects this because User B is submitting changes - // with User A's author ID. const undoPool = new AttributePool(); undoPool.putAttrib(['author', userA.author]); // 0 = author A undoPool.putAttrib(['author', userB.author]); // 1 = author B @@ -175,12 +184,10 @@ describe(__filename, function () { const undoChangeset = 'Z:c>0*0=5*1=6$'; // This should NOT disconnect User B - that's the bug (#2802) - const result = await Promise.all([ - waitForNextCommitOrDisconnect(socketB), - sendUserChanges(socketB, revB, undoChangeset, undoPool), - ]); + const resultP = waitForCommitOrDisconnect(socketB); + await sendUserChanges(socketB, revB, undoChangeset, undoPool); + const msg = await resultP; - const msg = result[0]; assert.notDeepEqual(msg, {disconnect: 'badChangeset'}, 'User was disconnected with badChangeset - bug #2802'); assert.equal(msg.type, 'COLLABROOM'); @@ -250,24 +257,19 @@ describe(__filename, function () { socketA = userA.socket; revA = userA.rev; - await drainMessages(socketA); - const userB = await connectUser(); socketB = userB.socket; revB = userB.rev; - await drainMessages(socketA); - // User B tries to insert text attributed to User A - this should be rejected const fakePool = new AttributePool(); fakePool.putAttrib(['author', userA.author]); - const result = await Promise.all([ - waitForNextCommitOrDisconnect(socketB), - sendUserChanges(socketB, revB, 'Z:1>5*0+5$hello', fakePool), - ]); + const resultP = waitForCommitOrDisconnect(socketB); + await sendUserChanges(socketB, revB, 'Z:1>5*0+5$hello', fakePool); + const msg = await resultP; - assert.deepEqual(result[0], {disconnect: 'badChangeset'}, + assert.deepEqual(msg, {disconnect: 'badChangeset'}, 'Should reject changeset that impersonates another author for new text'); }); @@ -291,12 +293,11 @@ describe(__filename, function () { const fakePool = new AttributePool(); fakePool.putAttrib(['author', 'a.fabricatedAuthorId']); - const result = await Promise.all([ - waitForNextCommitOrDisconnect(socketA), - sendUserChanges(socketA, revA, 'Z:6>0*0=5$', fakePool), - ]); + const resultP = waitForCommitOrDisconnect(socketA); + await sendUserChanges(socketA, revA, 'Z:6>0*0=5$', fakePool); + const msg = await resultP; - assert.deepEqual(result[0], {disconnect: 'badChangeset'}, + assert.deepEqual(msg, {disconnect: 'badChangeset'}, 'Should reject = op with fabricated author not in pad pool'); }); }); From 1aefafae43dfd45cb6e154b33243668a657c2783 Mon Sep 17 00:00:00 2001 From: John McLear Date: Wed, 1 Apr 2026 22:13:53 +0100 Subject: [PATCH 5/6] fix: reject - ops with foreign author to prevent pool injection The '-' op attribs are discarded from the document but still get added to the pad's attribute pool by moveOpsToNewPool. Without this check, an attacker could inject a fabricated author ID into the pool via a '-' op, then use a '=' op to attribute text to that fabricated author (bypassing the pool existence check). Now all non-'=' ops (+, -) with foreign author IDs are rejected. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/node/handler/PadMessageHandler.ts | 13 +++++--- .../backend/specs/undo_clear_authorship.ts | 33 +++++++++++++++++++ 2 files changed, 41 insertions(+), 5 deletions(-) diff --git a/src/node/handler/PadMessageHandler.ts b/src/node/handler/PadMessageHandler.ts index cd60aed87c5..1894a689e7a 100644 --- a/src/node/handler/PadMessageHandler.ts +++ b/src/node/handler/PadMessageHandler.ts @@ -669,11 +669,6 @@ const handleUserChanges = async (socket:any, message: { // an attribute number isn't in the pool). const opAuthorId = AttributeMap.fromString(op.attribs, wireApool).get('author'); if (opAuthorId && opAuthorId !== thisSession.author) { - if (op.opcode === '+') { - // Inserting new text as another author is always impersonation. - throw new Error(`Author ${thisSession.author} tried to submit changes as author ` + - `${opAuthorId} in changeset ${changeset}`); - } if (op.opcode === '=') { // Allow restoring author attributes on existing text (undo of clear authorship), // but only if the author ID is already known to this pad. This prevents a user @@ -683,6 +678,14 @@ const handleUserChanges = async (socket:any, message: { throw new Error(`Author ${thisSession.author} tried to set unknown author ` + `${opAuthorId} on existing text in changeset ${changeset}`); } + } else { + // Reject '+' ops (inserting new text as another author) and '-' ops (deleting + // with another author's attribs). While '-' attribs are discarded from the + // document, they are added to the pad's attribute pool by moveOpsToNewPool, + // which could be exploited to inject fabricated author IDs into the pool and + // bypass the '=' op pool check above. + throw new Error(`Author ${thisSession.author} tried to submit changes as author ` + + `${opAuthorId} in changeset ${changeset}`); } } } diff --git a/src/tests/backend/specs/undo_clear_authorship.ts b/src/tests/backend/specs/undo_clear_authorship.ts index 2bd814c955a..5ec73ca17f1 100644 --- a/src/tests/backend/specs/undo_clear_authorship.ts +++ b/src/tests/backend/specs/undo_clear_authorship.ts @@ -300,5 +300,38 @@ describe(__filename, function () { assert.deepEqual(msg, {disconnect: 'badChangeset'}, 'Should reject = op with fabricated author not in pad pool'); }); + + it('should reject - op with foreign author to prevent pool injection', async function () { + // Security: a '-' op with a foreign author's attribs should be rejected. + // While '-' attribs are discarded from the document, they are added to the + // pad's attribute pool by moveOpsToNewPool. Without this check, an attacker + // could inject a fabricated author ID into the pool via a '-' op, then use + // a '=' op to attribute text to that fabricated author. + const userA = await connectUser(); + socketA = userA.socket; + revA = userA.rev; + + // User A types text + const apoolA = new AttributePool(); + apoolA.putAttrib(['author', userA.author]); + await Promise.all([ + waitForAcceptCommit(socketA, revA + 1), + sendUserChanges(socketA, revA, 'Z:1>5*0+5$hello', apoolA), + ]); + revA += 1; + + // User A tries to delete a char with a fabricated author attrib via a - op + // This would inject the fabricated author into the pad pool + const fakePool = new AttributePool(); + fakePool.putAttrib(['author', 'a.fabricatedAuthorId']); + + const resultP = waitForCommitOrDisconnect(socketA); + // Delete 1 char with fabricated author attrib + await sendUserChanges(socketA, revA, 'Z:6<1*0-1$', fakePool); + const msg = await resultP; + + assert.deepEqual(msg, {disconnect: 'badChangeset'}, + 'Should reject - op with fabricated author to prevent pool injection'); + }); }); }); From 02f11dd99dc97326ce20144fed9b848d87a2becb Mon Sep 17 00:00:00 2001 From: John McLear Date: Sat, 18 Apr 2026 17:12:47 +0100 Subject: [PATCH 6/6] test: use not.toHaveClass for cleared authorship spans Addresses Qodo review: linestylefilter skips attribs with empty values, so a span with author='' has no class attribute at all. The previous negative-lookahead regex on the class attribute failed against a null attribute and was flaky in CI. Switch to not.toHaveClass(/author-/), which also passes when the attribute is missing. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../frontend-new/specs/clear_authorship_color.spec.ts | 2 +- .../frontend-new/specs/undo_clear_authorship.spec.ts | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/tests/frontend-new/specs/clear_authorship_color.spec.ts b/src/tests/frontend-new/specs/clear_authorship_color.spec.ts index f97f9e280d7..3e09ef3d824 100644 --- a/src/tests/frontend-new/specs/clear_authorship_color.spec.ts +++ b/src/tests/frontend-new/specs/clear_authorship_color.spec.ts @@ -57,7 +57,7 @@ test("clear authorship colors can be undone to restore author colors", async fun await clearAuthorship(page); // verify authorship is cleared - await expect(padBody.locator('div span').first()).toHaveAttribute('class', /^(?!.*author-)/, {timeout: 5000}); + await expect(padBody.locator('div span').first()).not.toHaveClass(/author-/, {timeout: 5000}); // Undo should restore authorship colors await undoChanges(page); diff --git a/src/tests/frontend-new/specs/undo_clear_authorship.spec.ts b/src/tests/frontend-new/specs/undo_clear_authorship.spec.ts index 099624405a9..b51f05c1c8c 100644 --- a/src/tests/frontend-new/specs/undo_clear_authorship.spec.ts +++ b/src/tests/frontend-new/specs/undo_clear_authorship.spec.ts @@ -71,8 +71,10 @@ test.describe('undo clear authorship colors with multiple authors (bug #2802)', // User B clears authorship colors (without selecting - clears whole pad) await clearAuthorship(page2); - // Wait for clear to propagate and verify authorship is cleared - await expect(body2.locator('div span').first()).toHaveAttribute('class', /^(?!.*author-)/, {timeout: 5000}); + // Wait for clear to propagate and verify authorship is cleared. linestylefilter + // drops attribs with empty values, so spans without authorship may have no class + // attribute at all; use a negated class matcher that handles both cases. + await expect(body2.locator('div span').first()).not.toHaveClass(/author-/, {timeout: 5000}); // THIS IS THE BUG: User B undoes the clear authorship await undoChanges(page2); @@ -117,8 +119,8 @@ test.describe('undo clear authorship colors with multiple authors (bug #2802)', // Clear authorship (no selection - clears whole pad) await clearAuthorship(page); - // Verify cleared - await expect(body.locator('div span').first()).toHaveAttribute('class', /^(?!.*author-)/, {timeout: 5000}); + // Verify cleared (spans without authorship may have no class attribute at all). + await expect(body.locator('div span').first()).not.toHaveClass(/author-/, {timeout: 5000}); // Undo should restore authorship await undoChanges(page);