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) <noreply@anthropic.com>

Author: JohnMcLear

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}`);
         }
       }
     }

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');
+    });
   });
 });