security: allow integrator sessionID cookie to be HttpOnly (#7045) (#7755)

* security: allow integrator sessionID cookie to be HttpOnly (#7045)

The integrator-set sessionID cookie was forced to be non-HttpOnly because
Etherpad's own client JS read it via document.cookie and forwarded it in
the socket.io CLIENT_READY payload, exposing it to XSS.

Mirror the GDPR PR3 author-token migration: read sessionID from the
socket.io handshake's Cookie header in PadMessageHandler.handleClientReady,
falling back to the legacy message-level field with a one-time deprecation
warning per socket. Drop the client-side Cookies.get('sessionID') reads in
pad.ts and timeslider.ts so the field is no longer sent by current clients.

Existing integrators that set sessionID without HttpOnly keep working
unchanged; the field on the message becomes optional and integrators
should now mark the cookie HttpOnly; Secure; SameSite=Lax.

Closes #7045

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(security): treat undecodable handshake cookies as absent (Qodo #7755)

decodeURIComponent() throws URIError on malformed values like `%ZZ`. The
unguarded call in PadMessageHandler.handleClientReady's readCookie() let
a single bad cookie abort CLIENT_READY for that socket, allowing
unauthenticated peers to spam server error logs and lock themselves out
of pads.

Catch URIError and treat the value as absent so the legacy message-level
field still serves as a fallback. Other error classes still propagate.
Add a backend test that asserts a `sessionID=%ZZ` cookie no longer
aborts the handshake.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

doc/cookies.md

@@ -15,4 +15,4 @@ Etherpad HTTP API clients may make use (if they choose so) to send another cooki
 
 | Name      | Sample value                       | Domain      | Usage description                                                                                                                                                                                                                                                                                                                                                                                                                          |
 |-----------|------------------------------------|-------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
-| sessionID | s.1c70968b333b25476a2c7bdd0e0bed17 | example.org | Sessions can be created between a group and an author. This allows an author to access more than one group. The sessionID will be set as a cookie to the client and is valid until a certain date. The session cookie can also contain multiple comma-separated sessionIDs, allowing a user to edit pads in different groups at the same time. More info - https://github.com/ether/etherpad-lite/blob/develop/doc/api/http_api.md#session |
+| sessionID | s.1c70968b333b25476a2c7bdd0e0bed17 | example.org | Sessions can be created between a group and an author. This allows an author to access more than one group. The sessionID is set as a cookie by the integrator and is valid until a certain date. The session cookie can also contain multiple comma-separated sessionIDs, allowing a user to edit pads in different groups at the same time. Since [#7045](https://github.com/ether/etherpad/issues/7045) Etherpad reads this cookie server-side from the socket.io handshake, so integrators **should** set it as `HttpOnly; Secure; SameSite=Lax` to mitigate XSS. More info - https://github.com/ether/etherpad-lite/blob/develop/doc/api/http_api.md#session |

src/node/handler/PadMessageHandler.ts

@@ -85,9 +85,12 @@ exports.socketio = () => {
  *       - auth: Object with the following properties copied from the client's CLIENT_READY message:
  *           - padID: Pad ID requested by the user. Unlike the padId property described below, this
  *             may be a read-only pad ID.
- *           - sessionID: Copied from the client's sessionID cookie, which should be the value
- *             returned from the createSession() HTTP API. This will be null/undefined if
- *             createSession() isn't used or the portal doesn't set the sessionID cookie.
+ *           - sessionID: The value returned from the createSession() HTTP API, normally set as
+ *             the `sessionID` cookie by the integrator. Read from the socket.io handshake's
+ *             Cookie header (so the cookie can be HttpOnly — issue #7045) and falls back to a
+ *             deprecated `sessionID` field on the CLIENT_READY message for legacy clients.
+ *             This will be null/undefined if createSession() isn't used or the integrator
+ *             doesn't set the sessionID cookie.
  *           - token: User-supplied token.
  *       - author: The user's author ID.
  *       - padId: The real (not read-only) ID of the pad.
@@ -395,12 +398,31 @@ exports.handleMessage = async (socket:any, message: ClientVarMessage) => {
     // once so the migration is visible in logs. The socket.io handshake does
     // not run cookie-parser, so pull the cookie directly from the Cookie
     // header.
+    //
+    // The same applies to the integrator-set `sessionID` cookie (issue #7045):
+    // historically the client read it from `document.cookie`, which forced the
+    // cookie to be non-HttpOnly and exposed it to XSS. Now we read it from the
+    // handshake Cookie header so integrators can set it `HttpOnly`.
     const cookiePrefix = settings.cookie?.prefix || '';
     const cookieHeader: string = socket.request?.headers?.cookie || '';
-    const cookieName = `${cookiePrefix}token`;
-    const cookieMatch = cookieHeader.split(/;\s*/).find(
-        (c) => c.split('=')[0] === cookieName);
-    const cookieToken = cookieMatch ? decodeURIComponent(cookieMatch.split('=').slice(1).join('=')) : null;
+    const readCookie = (name: string): string | null => {
+      const match = cookieHeader.split(/;\s*/).find(
+          (c) => c.split('=')[0] === name);
+      if (!match) return null;
+      const raw = match.split('=').slice(1).join('=');
+      // A malformed value (e.g. `name=%ZZ`) makes decodeURIComponent throw
+      // URIError. Without this guard a single bad cookie aborts CLIENT_READY,
+      // letting an unauthenticated peer spam server error logs and block
+      // itself from joining (flagged by Qodo on #7755). Treat undecodable
+      // values as absent.
+      try {
+        return decodeURIComponent(raw);
+      } catch (err) {
+        if (err instanceof URIError) return null;
+        throw err;
+      }
+    };
+    const cookieToken = readCookie(`${cookiePrefix}token`);
     const legacyToken = typeof message.token === 'string' ? message.token : null;
     const resolvedToken = cookieToken || legacyToken;
     if (!cookieToken && legacyToken && !thisSession.legacyTokenWarned) {
@@ -410,11 +432,23 @@ exports.handleMessage = async (socket:any, message: ClientVarMessage) => {
           'See docs/superpowers/specs/2026-04-19-gdpr-pr3-anon-identity-design.md');
       thisSession.legacyTokenWarned = true;
     }
+    const cookieSessionID =
+        readCookie(`${cookiePrefix}sessionID`) || readCookie('sessionID');
+    const legacySessionID =
+        typeof message.sessionID === 'string' ? message.sessionID : null;
+    const resolvedSessionID = cookieSessionID || legacySessionID;
+    if (!cookieSessionID && legacySessionID && !thisSession.legacySessionIdWarned) {
+      messageLogger.warn(
+          'client sent sessionID via CLIENT_READY message; integrators should ' +
+          'set the sessionID cookie as HttpOnly (issue #7045). The in-message ' +
+          'field is deprecated and will be removed in a future release.');
+      thisSession.legacySessionIdWarned = true;
+    }
     // Remember this information since we won't have the cookie in further socket.io messages. This
     // information will be used to check if the sessionId of this connection is still valid since it
     // could have been deleted by the API.
     thisSession.auth = {
-      sessionID: message.sessionID,
+      sessionID: resolvedSessionID,
       padID: message.padId,
       token: resolvedToken,
     };

src/static/js/pad.ts

@@ -305,10 +305,10 @@ const sendClientReady = (isReconnect) => {
     document.title = `${padId.replace(/_+/g, ' ')} | ${title}`;
   }
 
-  const cp = (window as any).clientVars?.cookiePrefix || '';
   // The author token lives in an HttpOnly cookie set by the server (GDPR PR3 /
-  // ether/etherpad#6701). The browser never reads or writes it; the server
-  // reads the cookie from the socket.io handshake inside handleClientReady.
+  // ether/etherpad#6701). The integrator-set `sessionID` cookie can also be
+  // HttpOnly now (issue #7045). The browser never reads or writes either; the
+  // server reads them from the socket.io handshake inside handleClientReady.
 
   // If known, propagate the display name and color to the server in the CLIENT_READY message. This
   // allows the server to include the values in its reply CLIENT_VARS message (which avoids
@@ -320,11 +320,13 @@ const sendClientReady = (isReconnect) => {
     name: params.get('userName'),
   };
 
+  // The integrator-set `sessionID` cookie is read server-side from the
+  // socket.io handshake (issue #7045) so it can be HttpOnly. We no longer
+  // forward it via the CLIENT_READY payload.
   const msg: any = {
     component: 'pad',
     type: 'CLIENT_READY',
     padId,
-    sessionID: Cookies.get(`${cp}sessionID`) || Cookies.get('sessionID'),
     userInfo,
   };
   const overrides = getMyViewOverrides();

src/static/js/timeslider.ts

@@ -157,13 +157,15 @@ const init = () => {
 };
 
 // sends a message over the socket
+// The integrator-set `sessionID` cookie is consumed server-side from the
+// socket.io handshake (issue #7045). It does not need to ride on every
+// message; the server only reads it during CLIENT_READY.
 const sendSocketMsg = (type, data) => {
   socket.emit("message", {
     component: 'pad', // FIXME: Remove this stupidity!
     type,
     data,
     padId,
-    sessionID: Cookies.get(`${cp}sessionID`) || Cookies.get('sessionID'),
   });
 };
 

src/static/js/types/SocketIOMessage.ts

@@ -190,8 +190,10 @@ export type ClientReadyMessage = {
   type: 'CLIENT_READY',
   component: string,
   padId: string,
-  sessionID: string,
-  token: string,
+  /** @deprecated since #7045 — read server-side from the HttpOnly cookie. */
+  sessionID?: string,
+  /** @deprecated since GDPR PR3 — read server-side from the HttpOnly cookie. */
+  token?: string,
   userInfo: UserInfo,
   padSettingsDefaults?: PadOption,
   reconnect?: boolean
@@ -329,8 +331,10 @@ export type SocketClientReadyMessage = {
   type: string
   component: string
   padId: string
-  sessionID: string
-  token: string
+  /** @deprecated since #7045 — read server-side from the HttpOnly cookie. */
+  sessionID?: string
+  /** @deprecated since GDPR PR3 — read server-side from the HttpOnly cookie. */
+  token?: string
   userInfo: {
     colorId: string|null
     name: string|null

src/tests/backend/specs/sessionIdCookie.ts

@@ -0,0 +1,106 @@
+'use strict';
+
+/**
+ * Regression test for https://github.com/ether/etherpad/issues/7045.
+ *
+ * Before the fix, Etherpad's client-side JavaScript read the integrator-set
+ * `sessionID` cookie via `document.cookie` and forwarded it in the socket.io
+ * CLIENT_READY payload. That forced integrators to mark the cookie as
+ * non-HttpOnly, exposing it to XSS.
+ *
+ * The fix moves the read to the server: `PadMessageHandler.handleClientReady`
+ * now pulls `sessionID` out of the socket.io handshake's `Cookie` header so
+ * integrators can mark the cookie `HttpOnly; Secure; SameSite=Lax`.
+ *
+ * The legacy message-level `sessionID` field is still accepted as a one-release
+ * fallback, with a one-time warning per socket.
+ */
+
+const assert = require('assert').strict;
+const common = require('../common');
+const padManager = require('../../../node/db/PadManager');
+const {sessioninfos} = require('../../../node/handler/PadMessageHandler');
+import settings from '../../../node/utils/Settings';
+const io = require('socket.io-client');
+
+const cookiePrefix = () => settings.cookie?.prefix || '';
+
+describe(__filename, function () {
+  this.timeout(30000);
+  let socket: any;
+
+  before(async function () { await common.init(); });
+
+  beforeEach(async function () {
+    assert(socket == null);
+  });
+
+  afterEach(async function () {
+    if (socket) socket.close();
+    socket = null;
+    if (await padManager.doesPadExist('pad')) {
+      const pad = await padManager.getPad('pad');
+      await pad.remove();
+    }
+  });
+
+  const connectWithCookie = async (cookieHeader: string) => {
+    const s = io(`${common.baseUrl}/`, {
+      forceNew: true,
+      query: {cookie: cookieHeader, padId: 'pad'},
+    });
+    await common.waitForSocketEvent(s, 'connect', 5000);
+    return s;
+  };
+
+  const sendClientReady = async (s: any, message: any) => {
+    s.emit('message', {
+      component: 'pad',
+      type: 'CLIENT_READY',
+      padId: 'pad',
+      ...message,
+    });
+    const reply: any = await common.waitForSocketEvent(s, 'message', 5000);
+    assert.equal(reply.type, 'CLIENT_VARS');
+  };
+
+  it('reads sessionID from the handshake Cookie header', async function () {
+    socket = await connectWithCookie('sessionID=s.aaaaaaaaaaaaaaaa');
+    await sendClientReady(socket, {});
+    assert.equal(sessioninfos[socket.id].auth.sessionID, 's.aaaaaaaaaaaaaaaa');
+  });
+
+  it('honours the configured cookie prefix', async function () {
+    socket = await connectWithCookie(`${cookiePrefix()}sessionID=s.bbbbbbbbbbbbbbbb`);
+    await sendClientReady(socket, {});
+    assert.equal(sessioninfos[socket.id].auth.sessionID, 's.bbbbbbbbbbbbbbbb');
+  });
+
+  it('falls back to message.sessionID for legacy clients (no cookie)', async function () {
+    socket = await connectWithCookie('');
+    await sendClientReady(socket, {sessionID: 's.cccccccccccccccc'});
+    assert.equal(sessioninfos[socket.id].auth.sessionID, 's.cccccccccccccccc');
+  });
+
+  it('prefers the cookie over the legacy message field', async function () {
+    socket = await connectWithCookie('sessionID=s.dddddddddddddddd');
+    await sendClientReady(socket, {sessionID: 's.eeeeeeeeeeeeeeee'});
+    assert.equal(sessioninfos[socket.id].auth.sessionID, 's.dddddddddddddddd');
+  });
+
+  it('records null when no sessionID is provided', async function () {
+    socket = await connectWithCookie('');
+    await sendClientReady(socket, {});
+    assert.equal(sessioninfos[socket.id].auth.sessionID, null);
+  });
+
+  it('treats a malformed (undecodable) cookie as absent rather than aborting', async function () {
+    // %ZZ is not a valid percent-encoded sequence; decodeURIComponent() throws
+    // URIError. Without the guard this would tear down CLIENT_READY and let
+    // any client log-spam the server (Qodo bug on #7755). The handshake must
+    // still complete and fall through to the message-field fallback.
+    socket = await connectWithCookie('sessionID=%ZZ');
+    await sendClientReady(socket, {sessionID: 's.ffffffffffffffff'});
+    assert.equal(sessioninfos[socket.id].auth.sessionID, 's.ffffffffffffffff');
+  });
+});