From 85c941fe9505c525ee0957eb0d31745d0c141afb Mon Sep 17 00:00:00 2001 From: John McLear Date: Fri, 8 May 2026 00:17:05 +0800 Subject: [PATCH] feat(padOptions): pass plugin-namespaced ep_* keys through applyPadSettings (#7698) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(padOptions): pass plugin-namespaced ep_* keys through applyPadSettings Native pad-wide settings ride a single padOptions object: the server seeds clientVars.initialOptions, the client mutates via pad.changePadOption(), and the existing padoptions COLLABROOM message broadcasts changes. Plugins can't use the same rail today because applyPadSettings (client) and normalizePadSettings (server) silently drop any key not in their hardcoded whitelist. Add a passthrough loop that preserves keys matching /^ep_[a-z0-9_]+$/ on both sides. Plugins can now stash their pad-wide values under their own namespace (e.g. pad.padOptions.ep_table_of_contents = {enabled: true}) and inherit the existing broadcast, persistence, creator-only-write enforcement, and enforceSettings semantics for free. A new src/node/utils/PluginCapabilities module exposes padOptionsPluginPassthrough = true so plugins can feature-detect via require() and fall back to per-user behavior on older cores. Co-Authored-By: Claude Opus 4.7 (1M context) * Address Qodo review on PR #7698 Four concerns raised by Qodo (qodo-free-for-open-source-projects): 1. Feature flag — AGENTS.MD §52 requires new features behind a flag, disabled by default. Add `enablePluginPadOptions` (default false) gating the passthrough on both server (normalizePadSettings) and client (applyPadSettings, via clientVars). Plugins detect the runtime state through clientVars.enablePluginPadOptions; the static PluginCapabilities flag stays as the "core can do this" signal. 2. Documentation — add a "Plugin-namespaced pad-wide options" section to doc/plugins.md covering capability detection, the runtime flag, the key namespace pattern, and the validation rules. Mirror the flag description in settings.json.template. 3. Unbounded payload — values for ep_* keys are persisted with the pad and broadcast to every connected client, so an unvalidated path was a reliability hazard. Validate every ep_* value: - Must round-trip through JSON.stringify (rejects functions, symbols, BigInt, circular refs). - Per-key serialized size capped at 64 KB. - Combined ep_* size capped at 256 KB per pad. Rejects drop the value with a console.warn line; the rest of the pad settings round-trip cleanly. 4. PadOption type — add `[k: \`ep_${string}\`]: unknown` index signature so the SocketIO message type matches runtime behavior; TS callers no longer need unsafe casts to read plugin-namespaced keys. Also extends the backend test suite with cases covering the runtime flag (off/on), JSON-serializability rejection, per-key cap, and total cap. Co-Authored-By: Claude Opus 4.7 (1M context) * fix(snap-tests): assert_grep — use here-string to dodge pipefail SIGPIPE `assert_grep` ran `printf '%s' "$out" | grep -q -F -- "$needle"` under `set -o pipefail`. When grep matched early it closed its stdin, printf got SIGPIPE on its next write (exit 141), and pipefail propagated the broken-pipe failure to the pipeline — making `if` see non-zero and falling into the FAIL branch even though grep itself succeeded. Failure was timing-dependent: it only fired when `$out` was large enough that printf hadn't flushed before grep exited. CI ubuntu-latest tipped into the racy path on PR #7698 once `settings.json.template` grew by 11 lines (the new `enablePluginPadOptions` flag); the symptom was the `Wrapper unit tests` step reporting `dbType rewritten to sqlite ✗` with "got: /*…" output even though the seeded file did contain the needle. Replace the pipe with a here-string so grep gets its input in one shot with no pipe between processes — no SIGPIPE possible. The fail-message `head -3` is converted to a here-string for the same reason. Repro on a runner whose pipe-buffer flush is slower than grep's first match would have hit the same flake on any PR; the bug isn't about this particular template change. Co-Authored-By: Claude Opus 4.7 (1M context) --------- Co-authored-by: Claude Opus 4.7 (1M context) --- doc/plugins.md | 71 ++++++++++++++++++ settings.json.template | 11 +++ snap/tests/lib.sh | 13 +++- src/node/db/Pad.ts | 61 +++++++++++++++- src/node/handler/PadMessageHandler.ts | 1 + src/node/utils/PluginCapabilities.ts | 18 +++++ src/node/utils/Settings.ts | 9 ++- src/static/js/pad.ts | 10 +++ src/static/js/types/SocketIOMessage.ts | 7 +- src/tests/backend/specs/Pad.ts | 99 ++++++++++++++++++++++++++ 10 files changed, 295 insertions(+), 5 deletions(-) create mode 100644 src/node/utils/PluginCapabilities.ts diff --git a/doc/plugins.md b/doc/plugins.md index 95fbb9c40..68637267d 100644 --- a/doc/plugins.md +++ b/doc/plugins.md @@ -238,6 +238,77 @@ operations in `templates/`, in files of type ".ejs", since Etherpad uses EJS for HTML templating. See the following link for more information about EJS: . +## Plugin-namespaced pad-wide options + +Plugins can ride the existing `padoptions` COLLABROOM rail to store +pad-wide settings — broadcast to every connected client, persisted with the +pad, and honored by `enforceSettings` — instead of inventing their own +message type and storage. The model matches how `enablePadWideSettings` +works for native toggles like sticky chat or line numbers. + +### Capability detection + +```js +let padOptionsPluginPassthrough = false; +try { + // The require throws on Etherpad versions that predate this capability; + // plugins should degrade gracefully (typically falling back to a per-user + // cookie toggle) when the flag is missing. + padOptionsPluginPassthrough = + require('ep_etherpad-lite/node/utils/PluginCapabilities') + .padOptionsPluginPassthrough === true; +} catch (_e) { /* older core */ } +``` + +The flag means the core has the passthrough patch *available*. Whether it +is actually *enabled* at runtime is a separate per-instance setting — see +below. + +### Runtime flag + +The passthrough is gated by `settings.enablePluginPadOptions`, default +`false`. Operators must opt in via `settings.json`: + +```json +{ + "enablePluginPadOptions": true +} +``` + +When enabled, the server reflects the value to every client via +`clientVars.enablePluginPadOptions` so plugins can detect both *capable* +(static) and *active* (per-pad request) at the same point. + +### Key namespace + +Plugins must use keys matching `/^ep_[a-z0-9_]+$/`. The recommended pattern +is `ep_` (e.g. `ep_table_of_contents`); compose multiple +pad-wide settings under one key as a plain object: + +```js +pad.changePadOption('ep_my_plugin', {enabled: true, depth: 3}); +``` + +The server passes through any matching key on the existing `padoptions` +message, persists it with the pad, and broadcasts it to every connected +client. `pad.padOptions.ep_my_plugin` reflects the latest value on every +client. + +### Validation + +Server-side `Pad.normalizePadSettings()` enforces three rules on every +plugin-namespaced key: + +- Values must round-trip through `JSON.stringify` (no functions, symbols, + BigInt, or circular references). +- Each key's serialized payload must fit within **64 KB**. +- The combined size of all `ep_*` values per pad must fit within **256 KB**. + +Values that fail any of these rules are dropped with a `console.warn`; the +rest of the settings round-trip cleanly. The caps prevent a misbehaving +plugin from bloating the persisted pad payload or the COLLABROOM +broadcast. + ## Writing and running front-end tests for your plugin Etherpad allows you to easily create front-end tests for plugins. diff --git a/settings.json.template b/settings.json.template index 7e6ab93aa..863286add 100644 --- a/settings.json.template +++ b/settings.json.template @@ -760,6 +760,17 @@ **/ "enablePadWideSettings": "${ENABLE_PAD_WIDE_SETTINGS:true}", + /* + * Allow plugins to ride the existing padoptions COLLABROOM rail by + * accepting pad-wide values under plugin-namespaced keys matching + * /^ep_[a-z0-9_]+$/ (e.g. ep_table_of_contents). Values are validated + * (JSON-safe, 64 KB per key, 256 KB total) and broadcast to every + * connected client just like native pad-wide toggles. Disabled by + * default; flip to true once your plugins (e.g. ep_plugin_helpers' + * padToggle) require it. See doc/plugins.md. + **/ + "enablePluginPadOptions": "${ENABLE_PLUGIN_PAD_OPTIONS:false}", + /* * Optional privacy banner shown once the pad loads. Disabled by default. * diff --git a/snap/tests/lib.sh b/snap/tests/lib.sh index af2336f34..07f561723 100755 --- a/snap/tests/lib.sh +++ b/snap/tests/lib.sh @@ -57,14 +57,23 @@ assert_exit() { } # assert_grep cmd needle name — fail if cmd's combined output doesn't match +# +# Uses a here-string instead of `printf | grep -q` because `set -o pipefail` +# (declared at the top of this file) propagates SIGPIPE failures: when grep +# -q matches early it closes its stdin, printf gets SIGPIPE on its next +# write, and pipefail makes the whole pipeline exit non-zero — even though +# the grep itself succeeded. The failure mode is timing-dependent, only +# tripping when the captured output is large enough that printf hasn't +# flushed before grep matches and exits. A here-string feeds grep its input +# in one shot with no pipe in between. assert_grep() { local needle="$1" name="$2"; shift 2 local out out=$("$@" 2>&1 || true) - if printf '%s' "$out" | grep -q -F -- "$needle"; then + if grep -q -F -- "$needle" <<<"$out"; then pass "$name" else - fail "$name" "expected output to contain: $needle; got: $(printf '%s' "$out" | head -3)" + fail "$name" "expected output to contain: $needle; got: $(head -3 <<<"$out")" fi } diff --git a/src/node/db/Pad.ts b/src/node/db/Pad.ts index ad135f21c..0e4eaaac0 100644 --- a/src/node/db/Pad.ts +++ b/src/node/db/Pad.ts @@ -43,6 +43,43 @@ type PadSettings = { chatAndUsers: boolean; lang: string | null; view: PadViewSettings; + // Plugin-namespaced pad-wide options ride alongside the core keys. + // Anything matching /^ep_[a-z0-9_]+$/ is preserved verbatim by + // normalizePadSettings so plugins can use the existing padoptions + // broadcast/persist rail without forking their own transport. + [pluginKey: string]: any; +}; + +const PLUGIN_KEY_RE = /^ep_[a-z0-9_]+$/; +// Per-key serialized JSON size cap: ~64 KB. Pad-wide settings are persisted +// with the pad and broadcast to every connected client on every change, so +// plugins must keep their values small. A misbehaving plugin shouldn't bloat +// the pad payload or the broadcast. +const PLUGIN_KEY_MAX_BYTES = 64 * 1024; +// Combined ep_* size cap: ~256 KB. Same rationale, aggregated. +const PLUGIN_TOTAL_MAX_BYTES = 256 * 1024; + +// Returns true iff `v` round-trips through JSON.stringify cleanly (no +// functions, symbols, BigInt, or circular references) and serializes to at +// most `maxBytes` UTF-8 bytes. Returns the serialized length on success so +// callers can enforce a cumulative cap without serializing twice. +const validatePluginValue = ( + key: string, value: unknown, maxBytes: number): {ok: true, bytes: number} | {ok: false, reason: string} => { + let serialized: string; + try { + serialized = JSON.stringify(value); + } catch (e: any) { + return {ok: false, reason: `JSON.stringify failed: ${e && e.message || e}`}; + } + if (serialized === undefined) { + // JSON.stringify returns undefined for top-level functions/undefined. + return {ok: false, reason: 'value is not JSON-serializable (function/undefined)'}; + } + const bytes = Buffer.byteLength(serialized, 'utf8'); + if (bytes > maxBytes) { + return {ok: false, reason: `serialized size ${bytes}B exceeds per-key cap ${maxBytes}B`}; + } + return {ok: true, bytes}; }; /** @@ -87,7 +124,7 @@ class Pad { static normalizePadSettings(rawPadSettings: any = {}): PadSettings { const rawView = rawPadSettings.view ?? {}; - return { + const result: PadSettings = { enforceSettings: !!rawPadSettings.enforceSettings, showChat: rawPadSettings.showChat == null ? settings.padOptions.showChat !== false : !!rawPadSettings.showChat, @@ -109,6 +146,28 @@ class Pad { !!rawView.fadeInactiveAuthorColors, }, }; + if (settings.enablePluginPadOptions) { + let totalBytes = 0; + for (const [k, v] of Object.entries(rawPadSettings)) { + if (!PLUGIN_KEY_RE.test(k)) continue; + const check = validatePluginValue(k, v, PLUGIN_KEY_MAX_BYTES); + if (!check.ok) { + // Drop and log. Persistence/broadcast still rejects the value, but + // the rest of the settings round-trip cleanly. + console.warn(`[normalizePadSettings] dropping ${k}: ${check.reason}`); + continue; + } + if (totalBytes + check.bytes > PLUGIN_TOTAL_MAX_BYTES) { + console.warn( + `[normalizePadSettings] dropping ${k}: combined ep_* size ` + + `would exceed cap ${PLUGIN_TOTAL_MAX_BYTES}B`); + continue; + } + totalBytes += check.bytes; + result[k] = v; + } + } + return result; } apool() { diff --git a/src/node/handler/PadMessageHandler.ts b/src/node/handler/PadMessageHandler.ts index 5a0a4c854..65ac9d762 100644 --- a/src/node/handler/PadMessageHandler.ts +++ b/src/node/handler/PadMessageHandler.ts @@ -1174,6 +1174,7 @@ const handleClientReady = async (socket:any, message: ClientReadyMessage) => { }, enableDarkMode: settings.enableDarkMode, enablePadWideSettings: settings.enablePadWideSettings, + enablePluginPadOptions: settings.enablePluginPadOptions, padDeletionToken, // Allow-listed copy — settings.privacyBanner could carry extra nested // keys from a hand-edited settings.json; sending those by reference diff --git a/src/node/utils/PluginCapabilities.ts b/src/node/utils/PluginCapabilities.ts new file mode 100644 index 000000000..428e8c855 --- /dev/null +++ b/src/node/utils/PluginCapabilities.ts @@ -0,0 +1,18 @@ +'use strict'; + +// Capability flags exposed to Etherpad plugins for runtime feature detection. +// Plugins should `try { require('ep_etherpad-lite/node/utils/PluginCapabilities') } +// catch { /* old core */ }` and degrade gracefully when a flag is missing. +// +// IMPORTANT: a flag here means the core implements the capability — it does +// not mean the capability is currently enabled on this Etherpad instance. +// Capabilities can be gated by per-instance settings; plugins must inspect +// the relevant runtime flag (typically reflected through clientVars) to +// decide whether to actually use the feature on a given pad load. + +// True when applyPadSettings (client + server) preserves keys matching +// /^ep_[a-z0-9_]+$/ on pad.padOptions. The runtime gate is +// settings.enablePluginPadOptions (default false), mirrored to clients via +// clientVars.enablePluginPadOptions. See doc/plugins.md for the full +// contract (key namespace, validation, size caps). +export const padOptionsPluginPassthrough = true; diff --git a/src/node/utils/Settings.ts b/src/node/utils/Settings.ts index ff2e93ec8..3b5e9790f 100644 --- a/src/node/utils/Settings.ts +++ b/src/node/utils/Settings.ts @@ -184,6 +184,7 @@ export type SettingsType = { updateServer: string, enableDarkMode: boolean, enablePadWideSettings: boolean, + enablePluginPadOptions: boolean, allowPadDeletionByAllUsers: boolean, privacyBanner: { enabled: boolean, @@ -332,7 +333,7 @@ export type SettingsType = { requireAdminForStatus: boolean, }, adminEmail: string | null, - getPublicSettings: () => Pick, + getPublicSettings: () => Pick, } const settings: SettingsType = { @@ -397,6 +398,11 @@ const settings: SettingsType = { updateServer: "https://static.etherpad.org", enableDarkMode: true, enablePadWideSettings: true, + // New plugin-padOption passthrough is opt-in per AGENTS.MD §52 ("New + // features should be placed behind feature flags and disabled by + // default"). Flip to true to let plugins (e.g. ep_plugin_helpers' + // padToggle) ride the existing padoptions broadcast/persist rail. + enablePluginPadOptions: false, allowPadDeletionByAllUsers: false, privacyBanner: { enabled: false, @@ -770,6 +776,7 @@ const settings: SettingsType = { skinName: settings.skinName, skinVariants: settings.skinVariants, enablePadWideSettings: settings.enablePadWideSettings, + enablePluginPadOptions: settings.enablePluginPadOptions, privacyBanner: getPublicPrivacyBanner(), } }, diff --git a/src/static/js/pad.ts b/src/static/js/pad.ts index 12406197e..6070fb894 100644 --- a/src/static/js/pad.ts +++ b/src/static/js/pad.ts @@ -874,6 +874,16 @@ const pad = { pad.padOptions.view[k] = v; } } + // Plugin-namespaced keys (ep_*) are passed through verbatim so plugins + // can ride the existing padoptions broadcast/persist rail. Gated on + // settings.enablePluginPadOptions (mirrored to clientVars by + // getPublicSettings). Server-side normalizePadSettings preserves the + // same keys symmetrically. + if (clientVars.enablePluginPadOptions) { + for (const [k, v] of Object.entries(opts)) { + if (/^ep_[a-z0-9_]+$/.test(k)) pad.padOptions[k] = v; + } + } normalizeChatOptions(pad.padOptions); pad.refreshPadSettingsControls(); pad.applyOptionsChange(); diff --git a/src/static/js/types/SocketIOMessage.ts b/src/static/js/types/SocketIOMessage.ts index f5e103d29..e1c01a7ae 100644 --- a/src/static/js/types/SocketIOMessage.ts +++ b/src/static/js/types/SocketIOMessage.ts @@ -260,7 +260,12 @@ export type PadOption = { "alwaysShowChat"?: boolean, "chatAndUsers"?: boolean, "lang"?: null|string, - view? : MapArrayType + view? : MapArrayType, + // Plugin-namespaced pad-wide options (gated by settings.enablePluginPadOptions). + // The runtime regex is /^ep_[a-z0-9_]+$/ — TypeScript template literals + // can't constrain that exactly, so this signature accepts any ep_-prefixed + // string and applyPadSettings/normalizePadSettings reject the rest. + [k: `ep_${string}`]: unknown, } diff --git a/src/tests/backend/specs/Pad.ts b/src/tests/backend/specs/Pad.ts index 13bc79e5a..0345ae87d 100644 --- a/src/tests/backend/specs/Pad.ts +++ b/src/tests/backend/specs/Pad.ts @@ -178,4 +178,103 @@ describe(__filename, function () { } }); }); + + describe('normalizePadSettings plugin passthrough (ep_* keys)', function () { + let originalFlag: boolean; + let warnSpy: any; + let warnings: string[]; + + before(function () { originalFlag = settings.enablePluginPadOptions; }); + after(function () { settings.enablePluginPadOptions = originalFlag; }); + beforeEach(function () { + warnings = []; + warnSpy = console.warn; + console.warn = (msg: string) => { warnings.push(msg); }; + }); + afterEach(function () { console.warn = warnSpy; }); + + describe('with enablePluginPadOptions = true', function () { + before(function () { settings.enablePluginPadOptions = true; }); + + it('preserves ep_* keys verbatim so plugins can ride padoptions', function () { + const ps: any = Pad.Pad.normalizePadSettings({ + ep_table_of_contents: {enabled: true, depth: 3}, + ep_font_color: 'red', + }); + assert.deepEqual(ps.ep_table_of_contents, {enabled: true, depth: 3}); + assert.equal(ps.ep_font_color, 'red'); + }); + + it('drops keys that do not match the ep_ pattern', function () { + const ps: any = Pad.Pad.normalizePadSettings({ + EP_SHOUTY: 1, // uppercase rejected + ep_: 1, // empty suffix rejected + 'ep-dashy': 1, // dash rejected + somethingElse: 1, // no prefix rejected + }); + assert.equal(ps.EP_SHOUTY, undefined); + assert.equal(ps.ep_, undefined); + assert.equal(ps['ep-dashy'], undefined); + assert.equal(ps.somethingElse, undefined); + }); + + it('does not overwrite reserved core keys when an ep_ alias is sent', function () { + // Core keys (showChat etc.) come first; ep_* loop runs after. A plugin + // key like ep_showchat is namespaced separately and cannot collide. + const ps: any = Pad.Pad.normalizePadSettings({ + showChat: false, + ep_showchat: 'plugin-value', + }); + assert.equal(ps.showChat, false); + assert.equal(ps.ep_showchat, 'plugin-value'); + }); + + it('drops a non-JSON-serializable value with a warn-log', function () { + const ps: any = Pad.Pad.normalizePadSettings({ + ep_bad: () => 'function values are not JSON-safe', + }); + assert.equal(ps.ep_bad, undefined); + assert.ok(warnings.some((w) => w.includes('ep_bad')), + `expected warn mentioning ep_bad, got: ${JSON.stringify(warnings)}`); + }); + + it('drops a value larger than the 64 KB per-key cap', function () { + const oversized = 'x'.repeat(70 * 1024); // ~70 KB string + const ps: any = Pad.Pad.normalizePadSettings({ + ep_huge: oversized, + }); + assert.equal(ps.ep_huge, undefined); + assert.ok(warnings.some((w) => w.includes('ep_huge') && w.includes('per-key cap')), + `expected per-key cap warning, got: ${JSON.stringify(warnings)}`); + }); + + it('drops keys that would exceed the cumulative 256 KB cap', function () { + // Each value is well under the per-key cap but together they exceed + // the total cap. The first few must survive; the overflowing key + // must be dropped. + const big = 'y'.repeat(60 * 1024); // ~60 KB each + const input: any = {}; + for (let i = 0; i < 6; i++) input[`ep_chunk${i}`] = big; + const ps: any = Pad.Pad.normalizePadSettings(input); + const survivors = Object.keys(ps).filter((k) => k.startsWith('ep_chunk')); + assert.ok(survivors.length < 6, + `at least one chunk must be dropped to keep total <= 256 KB, but all ${survivors.length}/6 survived`); + assert.ok(warnings.some((w) => w.includes('combined ep_* size')), + `expected combined-cap warning, got: ${JSON.stringify(warnings)}`); + }); + }); + + describe('with enablePluginPadOptions = false (default)', function () { + before(function () { settings.enablePluginPadOptions = false; }); + + it('drops every ep_* key — feature flag is opt-in', function () { + const ps: any = Pad.Pad.normalizePadSettings({ + ep_table_of_contents: {enabled: true}, + ep_font_color: 'red', + }); + assert.equal(ps.ep_table_of_contents, undefined); + assert.equal(ps.ep_font_color, undefined); + }); + }); + }); });