Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions settings.json.template
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,13 @@
* Settings controlling the session cookie issued by Etherpad.
*/
"cookie": {
/*
* Prefix for all cookie names set by Etherpad. Set this to "ep_" or similar
* if Etherpad's cookie names (token, sessionID, etc.) conflict with those
* of another application on the same domain. Default: "" (no prefix).
*/
// "prefix": "ep_",

/*
* How often (in milliseconds) the key used to sign the express_sid cookie
* should be rotated. Long rotation intervals reduce signature verification
Expand Down
1 change: 1 addition & 0 deletions src/node/handler/PadMessageHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1058,6 +1058,7 @@ const handleClientReady = async (socket:any, message: ClientReadyMessage) => {
settings.scrollWhenFocusLineIsOutOfViewport.percentageToScrollWhenUserPressesArrowUp,
},
initialChangesets: [], // FIXME: REMOVE THIS SHIT,
cookiePrefix: settings.cookie.prefix,
mode: process.env.NODE_ENV
};

Expand Down
2 changes: 1 addition & 1 deletion src/node/hooks/express.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ exports.restartServer = async () => {
saveUninitialized: false,
// Set the cookie name to a javascript identifier compatible string. Makes code handling it
// cleaner :)
name: 'express_sid',
name: `${settings.cookie.prefix}express_sid`,
cookie: {
maxAge: sessionLifetime || undefined, // Convert 0 to null.
Comment thread
qodo-free-for-open-source-projects[bot] marked this conversation as resolved.
sameSite: settings.cookie.sameSite,
Expand Down
6 changes: 5 additions & 1 deletion src/node/hooks/express/importexport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,12 @@ exports.expressCreateServer = (hookName:string, args:ArgsExpressType, cb:Functio
(async () => {
// @ts-ignore
const {session: {user} = {}} = req;
const p = settings.cookie.prefix;
const {accessStatus, authorID: authorId} = await securityManager.checkAccess(
req.params.pad, req.cookies.sessionID, req.cookies.token, user);
req.params.pad,
req.cookies[`${p}sessionID`] || req.cookies.sessionID,
req.cookies[`${p}token`] || req.cookies.token,
user);
if (accessStatus !== 'grant' || !webaccess.userCanModify(req.params.pad, req)) {
return res.status(403).send('Forbidden');
}
Expand Down
1 change: 1 addition & 0 deletions src/node/hooks/express/specialpages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,7 @@ exports.expressCreateServer = async (_hookName: string, args: ArgsExpressType, c
})

const indexString = eejs.require('ep_etherpad-lite/templates/indexBootstrap.js', {
settings,
})

const timeSliderString = eejs.require('ep_etherpad-lite/templates/timeSliderBootstrap.js', {
Expand Down
6 changes: 4 additions & 2 deletions src/node/hooks/express/tokenTransfer.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {ArgsExpressType} from "../../types/ArgsExpressType";
const db = require('../../db/DB');
import crypto from 'crypto'
import settings from '../../utils/Settings';


type TokenTransferRequest = {
Expand Down Expand Up @@ -38,8 +39,9 @@ export const expressCreateServer = (hookName:string, {app}:ArgsExpressType) =>

const token = await db.get(`${tokenTransferKey}:${id}`)

res.cookie('token', tokenData.token, {path: '/', maxAge: 1000*60*60*24*365});
res.cookie('prefsHttp', tokenData.prefsHttp, {path: '/', maxAge: 1000*60*60*24*365});
const p = settings.cookie.prefix;
res.cookie(`${p}token`, tokenData.token, {path: '/', maxAge: 1000*60*60*24*365});
res.cookie(`${p}prefsHttp`, tokenData.prefsHttp, {path: '/', maxAge: 1000*60*60*24*365});
Comment thread
qodo-free-for-open-source-projects[bot] marked this conversation as resolved.
res.send(token);
})
}
7 changes: 6 additions & 1 deletion src/node/padaccess.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
'use strict';
const securityManager = require('./db/SecurityManager');
import settings from './utils/Settings';

// checks for padAccess
module.exports = async (req: { params?: any; cookies?: any; session?: any; }, res: { status: (arg0: number) => { (): any; new(): any; send: { (arg0: string): void; new(): any; }; }; }) => {
const {session: {user} = {}} = req;
const p = settings.cookie.prefix;
const accessObj = await securityManager.checkAccess(
req.params.pad, req.cookies.sessionID, req.cookies.token, user);
req.params.pad,
req.cookies[`${p}sessionID`] || req.cookies.sessionID,
req.cookies[`${p}token`] || req.cookies.token,
Comment on lines +8 to +12

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. sessionid still default cookie 📎 Requirement gap ⛨ Security

Because settings.cookie.prefix defaults to '', Etherpad still relies on the generic sessionID
cookie name by default, which keeps default deployments prone to collisions with other web
frameworks. This violates the requirement to avoid sessionID as the default cookie name.
Agent Prompt
## Issue description
Default deployments still use the generic `sessionID` cookie name because `cookie.prefix` defaults to `''`, which does not satisfy the requirement to avoid `sessionID` by default.

## Issue Context
The code already supports migration by falling back to unprefixed cookie names when reading (`prefixed || unprefixed`). This makes it feasible to change the default to an Etherpad-specific namespace while keeping compatibility for existing deployments/portals.

## Fix Focus Areas
- src/node/utils/Settings.ts[530-538]
- src/node/padaccess.ts[8-13]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — cookie prefix is now validated to only allow [a-zA-Z0-9_-] characters, preventing header injection.

user);
Comment on lines +8 to +13

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Foreign token blocks access 🐞 Bug ≡ Correctness

In pad access checks, when cookie.prefix is set the code falls back to the unprefixed token cookie
without validating it, so a conflicting token cookie from another app can cause
SecurityManager.checkAccess() to deny with “invalid author token”. This defeats the purpose of
enabling a prefix and can break direct /import and /export requests with unexpected 403s until the
client creates the prefixed cookie.
Agent Prompt
## Issue description
When `cookie.prefix` is configured, server-side routes that call `SecurityManager.checkAccess()` fall back to the unprefixed `token` cookie even if it belongs to another app. If that cookie is present but not a valid Etherpad author token, `SecurityManager.checkAccess()` returns DENY, causing 403s on routes like import/export.

## Issue Context
The whole point of `cookie.prefix` is to avoid cookie collisions on shared domains. The current fallback logic can still be triggered by other frameworks’ generic `token` cookies, producing denials instead of ignoring the foreign value.

## Fix Focus Areas
- Add a small helper to compute the token passed to `checkAccess`:
  - Prefer the prefixed cookie.
  - Only fall back to the unprefixed cookie **if it passes** `padutils.isValidAuthorToken()` (or otherwise treat it as absent).
- Apply the helper consistently anywhere server-side reads `req.cookies.token` for access decisions.

### Suggested focus locations
- src/node/padaccess.ts[6-13]
- src/node/hooks/express/importexport.ts[76-86]
- src/node/db/SecurityManager.ts[112-126] (reference behavior; no change necessarily required here)

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't fix — with default prefix "", the fallback reads the same cookie name. When a user sets a custom prefix, the fallback to unprefixed name ensures existing tokens continue to work during migration. A foreign/invalid token simply fails auth as before.


if (accessObj.accessStatus === 'grant') {
// there is access, continue
Expand Down
9 changes: 9 additions & 0 deletions src/node/utils/Settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,7 @@ export type SettingsType = {
trustProxy: boolean,
cookie: {
keyRotationInterval: number,
prefix: string,
sameSite: boolean | "lax" | "strict" | "none" | undefined,
sessionLifetime: number,
sessionRefreshInterval: number,
Expand Down Expand Up @@ -530,6 +531,7 @@ const settings: SettingsType = {
*/
cookie: {
keyRotationInterval: 1 * 24 * 60 * 60 * 1000,
prefix: '',
sameSite: 'lax',
Comment thread
qodo-free-for-open-source-projects[bot] marked this conversation as resolved.
sessionLifetime: 10 * 24 * 60 * 60 * 1000,
sessionRefreshInterval: 1 * 24 * 60 * 60 * 1000,
Expand Down Expand Up @@ -1064,6 +1066,13 @@ export const reloadSettings = () => {
'use automatic key rotation instead (see the cookie.keyRotationInterval setting).');
}

// Validate cookie prefix to prevent header injection via cookie names
if (settings.cookie.prefix && !/^[a-zA-Z0-9_-]*$/.test(settings.cookie.prefix)) {
logger.error(`cookie.prefix "${settings.cookie.prefix}" contains invalid characters. ` +
'Only alphanumeric characters, hyphens, and underscores are allowed. Using empty prefix.');
settings.cookie.prefix = '';
}

if (settings.dbType === 'dirty') {
const dirtyWarning = 'DirtyDB is used. This is not recommended for production.';
if (!settings.suppressErrorsInPadText) {
Expand Down
4 changes: 3 additions & 1 deletion src/static/js/l10n.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ import html10n from '../js/vendors/html10n';

// Set language for l10n
let regexpLang: string | undefined;
let language = document.cookie.match(/language=((\w{2,3})(-\w+)?)/);
const cp = ((window as any).clientVars?.cookiePrefix || '').replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
let language = document.cookie.match(new RegExp(`${cp}language=((\\w{2,3})(-\\w+)?)`))
|| document.cookie.match(/language=((\w{2,3})(-\w+)?)/);
if (language) regexpLang = language[1];

html10n.mt.bind('indexed', () => {
Expand Down
10 changes: 6 additions & 4 deletions src/static/js/pad.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,8 @@ const getParameters = [
callback: (val) => {
console.log('Val is', val)
html10n.localize([val, 'en']);
Cookies.set('language', val);
const prefix = (window as any).clientVars?.cookiePrefix || '';
Cookies.set(`${prefix}language`, val);
Comment thread
qodo-free-for-open-source-projects[bot] marked this conversation as resolved.
},
},
];
Expand Down Expand Up @@ -183,10 +184,11 @@ const sendClientReady = (isReconnect) => {
document.title = `${padId.replace(/_+/g, ' ')} | ${title}`;
}

let token = Cookies.get('token');
const cp = (window as any).clientVars?.cookiePrefix || '';
let token = Cookies.get(`${cp}token`) || Cookies.get('token');
if (token == null || !padutils.isValidAuthorToken(token)) {
token = padutils.generateAuthorToken();
Cookies.set('token', token, {expires: 60});
Cookies.set(`${cp}token`, token, {expires: 60});
}

// If known, propagate the display name and color to the server in the CLIENT_READY message. This
Expand All @@ -203,7 +205,7 @@ const sendClientReady = (isReconnect) => {
component: 'pad',
type: 'CLIENT_READY',
padId,
sessionID: Cookies.get('sessionID'),
sessionID: Cookies.get(`${cp}sessionID`) || Cookies.get('sessionID'),
token,
userInfo,
};
Expand Down
10 changes: 8 additions & 2 deletions src/static/js/pad_cookie.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ import {Cookies} from "./pad_utils";

exports.padcookie = new class {
constructor() {
this.cookieName_ = window.location.protocol === 'https:' ? 'prefs' : 'prefsHttp';
const prefix = (window as any).clientVars?.cookiePrefix || '';
this.cookieName_ = prefix + (window.location.protocol === 'https:' ? 'prefs' : 'prefsHttp');
}

init() {
Expand All @@ -43,7 +44,12 @@ exports.padcookie = new class {

readPrefs_() {
try {
const json = Cookies.get(this.cookieName_);
let json = Cookies.get(this.cookieName_);
// Fall back to unprefixed cookie for migration
if (json == null) {
const unprefixed = window.location.protocol === 'https:' ? 'prefs' : 'prefsHttp';
if (unprefixed !== this.cookieName_) json = Cookies.get(unprefixed);
}
if (json == null) return null;
return JSON.parse(json);
} catch (e) {
Expand Down
3 changes: 2 additions & 1 deletion src/static/js/pad_editor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,8 @@ const padeditor = (() => {
});
$('#languagemenu').val(html10n.getLanguage());
$('#languagemenu').on('change', () => {
Cookies.set('language', $('#languagemenu').val());
const cp = (window as any).clientVars?.cookiePrefix || '';
Cookies.set(`${cp}language`, $('#languagemenu').val());
html10n.localize([$('#languagemenu').val(), 'en']);
if ($('select').niceSelect) {
$('select').niceSelect('update');
Expand Down
8 changes: 5 additions & 3 deletions src/static/js/timeslider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import padutils from './pad_utils'
const socketio = require('./socketio');
import html10n from '../js/vendors/html10n'
let token, padId, exportLinks, socket, changesetLoader, BroadcastSlider;
let cp = '';

const init = () => {
padutils.setupGlobalExceptionHandler();
Expand All @@ -48,10 +49,11 @@ const init = () => {
document.title = `${padId.replace(/_+/g, ' ')} | ${document.title}`;

// ensure we have a token
token = Cookies.get('token');
cp = (window as any).clientVars?.cookiePrefix || '';
token = Cookies.get(`${cp}token`) || Cookies.get('token');
if (token == null) {
token = `t.${randomString()}`;
Cookies.set('token', token, {expires: 60});
Cookies.set(`${cp}token`, token, {expires: 60});
}

socket = socketio.connect(exports.baseURL, '/', {query: {padId}});
Expand Down Expand Up @@ -101,7 +103,7 @@ const sendSocketMsg = (type, data) => {
data,
padId,
token,
sessionID: Cookies.get('sessionID'),
sessionID: Cookies.get(`${cp}sessionID`) || Cookies.get('sessionID'),
});
};

Expand Down
6 changes: 4 additions & 2 deletions src/static/js/welcome.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ function getCookie(name: string) {
}


const cp = (window as any).clientVars?.cookiePrefix || '';

function handleTransferOfSession() {
const transferNowButton = document.querySelector('[data-l10n-id="index.transferSessionNow"]')! as HTMLButtonElement;

Expand All @@ -25,8 +27,8 @@ function handleTransferOfSession() {
"Content-Type": "application/json"
},
body: JSON.stringify({
prefsHttp: getCookie('prefsHttp'),
token: getCookie('token'),
prefsHttp: getCookie(`${cp}prefsHttp`) || getCookie('prefsHttp'),
token: getCookie(`${cp}token`) || getCookie('token'),
})
})

Expand Down
1 change: 1 addition & 0 deletions src/templates/indexBootstrap.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@

(async () => {
window.clientVars = { cookiePrefix: <%-JSON.stringify(settings.cookie.prefix)%> };
window.$ = window.jQuery = require('ep_etherpad-lite/static/js/rjquery').jQuery;
require('ep_etherpad-lite/static/js/l10n')
require('ep_etherpad-lite/static/js/index')
Expand Down
1 change: 1 addition & 0 deletions src/templates/padBootstrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
// This is needed to fetch /pluginfw/plugin-definitions.json, which happens before the server
// sends the CLIENT_VARS message.
randomVersionString: <%-JSON.stringify(settings.randomVersionString)%>,
cookiePrefix: <%-JSON.stringify(settings.cookie.prefix)%>,
};

// Allow other frames to access this frame's modules.
Expand Down
3 changes: 3 additions & 0 deletions src/templates/padViteBootstrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ window.clientVars = {
// This is needed to fetch /pluginfw/plugin-definitions.json, which happens before the server
// sends the CLIENT_VARS message.
randomVersionString: "7a7bdbad",
// Must match cookie.prefix in settings.json (default: "").
// This file is only used in Vite dev mode and is not template-processed.
cookiePrefix: "",
};

(async () => {
Expand Down
1 change: 1 addition & 0 deletions src/templates/timeSliderBootstrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ window.clientVars = {
// This is needed to fetch /pluginfw/plugin-definitions.json, which happens before the
// server sends the CLIENT_VARS message.
randomVersionString: <%-JSON.stringify(settings.randomVersionString)%>,
cookiePrefix: <%-JSON.stringify(settings.cookie.prefix)%>,
};
let BroadcastSlider;

Expand Down
Loading