Skip to content

Commit 85c941f

Browse files
JohnMcLearclaude
andauthored
feat(padOptions): pass plugin-namespaced ep_* keys through applyPadSettings (#7698)
* 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent c7ac1cc commit 85c941f

10 files changed

Lines changed: 295 additions & 5 deletions

File tree

doc/plugins.md

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,77 @@ operations in `templates/`, in files of type ".ejs", since Etherpad uses EJS for
238238
HTML templating. See the following link for more information about EJS:
239239
<https://github.com/visionmedia/ejs>.
240240

241+
## Plugin-namespaced pad-wide options
242+
243+
Plugins can ride the existing `padoptions` COLLABROOM rail to store
244+
pad-wide settings — broadcast to every connected client, persisted with the
245+
pad, and honored by `enforceSettings` — instead of inventing their own
246+
message type and storage. The model matches how `enablePadWideSettings`
247+
works for native toggles like sticky chat or line numbers.
248+
249+
### Capability detection
250+
251+
```js
252+
let padOptionsPluginPassthrough = false;
253+
try {
254+
// The require throws on Etherpad versions that predate this capability;
255+
// plugins should degrade gracefully (typically falling back to a per-user
256+
// cookie toggle) when the flag is missing.
257+
padOptionsPluginPassthrough =
258+
require('ep_etherpad-lite/node/utils/PluginCapabilities')
259+
.padOptionsPluginPassthrough === true;
260+
} catch (_e) { /* older core */ }
261+
```
262+
263+
The flag means the core has the passthrough patch *available*. Whether it
264+
is actually *enabled* at runtime is a separate per-instance setting — see
265+
below.
266+
267+
### Runtime flag
268+
269+
The passthrough is gated by `settings.enablePluginPadOptions`, default
270+
`false`. Operators must opt in via `settings.json`:
271+
272+
```json
273+
{
274+
"enablePluginPadOptions": true
275+
}
276+
```
277+
278+
When enabled, the server reflects the value to every client via
279+
`clientVars.enablePluginPadOptions` so plugins can detect both *capable*
280+
(static) and *active* (per-pad request) at the same point.
281+
282+
### Key namespace
283+
284+
Plugins must use keys matching `/^ep_[a-z0-9_]+$/`. The recommended pattern
285+
is `ep_<plugin_name>` (e.g. `ep_table_of_contents`); compose multiple
286+
pad-wide settings under one key as a plain object:
287+
288+
```js
289+
pad.changePadOption('ep_my_plugin', {enabled: true, depth: 3});
290+
```
291+
292+
The server passes through any matching key on the existing `padoptions`
293+
message, persists it with the pad, and broadcasts it to every connected
294+
client. `pad.padOptions.ep_my_plugin` reflects the latest value on every
295+
client.
296+
297+
### Validation
298+
299+
Server-side `Pad.normalizePadSettings()` enforces three rules on every
300+
plugin-namespaced key:
301+
302+
- Values must round-trip through `JSON.stringify` (no functions, symbols,
303+
BigInt, or circular references).
304+
- Each key's serialized payload must fit within **64 KB**.
305+
- The combined size of all `ep_*` values per pad must fit within **256 KB**.
306+
307+
Values that fail any of these rules are dropped with a `console.warn`; the
308+
rest of the settings round-trip cleanly. The caps prevent a misbehaving
309+
plugin from bloating the persisted pad payload or the COLLABROOM
310+
broadcast.
311+
241312
## Writing and running front-end tests for your plugin
242313

243314
Etherpad allows you to easily create front-end tests for plugins.

settings.json.template

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -760,6 +760,17 @@
760760
**/
761761
"enablePadWideSettings": "${ENABLE_PAD_WIDE_SETTINGS:true}",
762762

763+
/*
764+
* Allow plugins to ride the existing padoptions COLLABROOM rail by
765+
* accepting pad-wide values under plugin-namespaced keys matching
766+
* /^ep_[a-z0-9_]+$/ (e.g. ep_table_of_contents). Values are validated
767+
* (JSON-safe, 64 KB per key, 256 KB total) and broadcast to every
768+
* connected client just like native pad-wide toggles. Disabled by
769+
* default; flip to true once your plugins (e.g. ep_plugin_helpers'
770+
* padToggle) require it. See doc/plugins.md.
771+
**/
772+
"enablePluginPadOptions": "${ENABLE_PLUGIN_PAD_OPTIONS:false}",
773+
763774
/*
764775
* Optional privacy banner shown once the pad loads. Disabled by default.
765776
*

snap/tests/lib.sh

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,14 +57,23 @@ assert_exit() {
5757
}
5858

5959
# assert_grep cmd needle name — fail if cmd's combined output doesn't match
60+
#
61+
# Uses a here-string instead of `printf | grep -q` because `set -o pipefail`
62+
# (declared at the top of this file) propagates SIGPIPE failures: when grep
63+
# -q matches early it closes its stdin, printf gets SIGPIPE on its next
64+
# write, and pipefail makes the whole pipeline exit non-zero — even though
65+
# the grep itself succeeded. The failure mode is timing-dependent, only
66+
# tripping when the captured output is large enough that printf hasn't
67+
# flushed before grep matches and exits. A here-string feeds grep its input
68+
# in one shot with no pipe in between.
6069
assert_grep() {
6170
local needle="$1" name="$2"; shift 2
6271
local out
6372
out=$("$@" 2>&1 || true)
64-
if printf '%s' "$out" | grep -q -F -- "$needle"; then
73+
if grep -q -F -- "$needle" <<<"$out"; then
6574
pass "$name"
6675
else
67-
fail "$name" "expected output to contain: $needle; got: $(printf '%s' "$out" | head -3)"
76+
fail "$name" "expected output to contain: $needle; got: $(head -3 <<<"$out")"
6877
fi
6978
}
7079

src/node/db/Pad.ts

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,43 @@ type PadSettings = {
4343
chatAndUsers: boolean;
4444
lang: string | null;
4545
view: PadViewSettings;
46+
// Plugin-namespaced pad-wide options ride alongside the core keys.
47+
// Anything matching /^ep_[a-z0-9_]+$/ is preserved verbatim by
48+
// normalizePadSettings so plugins can use the existing padoptions
49+
// broadcast/persist rail without forking their own transport.
50+
[pluginKey: string]: any;
51+
};
52+
53+
const PLUGIN_KEY_RE = /^ep_[a-z0-9_]+$/;
54+
// Per-key serialized JSON size cap: ~64 KB. Pad-wide settings are persisted
55+
// with the pad and broadcast to every connected client on every change, so
56+
// plugins must keep their values small. A misbehaving plugin shouldn't bloat
57+
// the pad payload or the broadcast.
58+
const PLUGIN_KEY_MAX_BYTES = 64 * 1024;
59+
// Combined ep_* size cap: ~256 KB. Same rationale, aggregated.
60+
const PLUGIN_TOTAL_MAX_BYTES = 256 * 1024;
61+
62+
// Returns true iff `v` round-trips through JSON.stringify cleanly (no
63+
// functions, symbols, BigInt, or circular references) and serializes to at
64+
// most `maxBytes` UTF-8 bytes. Returns the serialized length on success so
65+
// callers can enforce a cumulative cap without serializing twice.
66+
const validatePluginValue = (
67+
key: string, value: unknown, maxBytes: number): {ok: true, bytes: number} | {ok: false, reason: string} => {
68+
let serialized: string;
69+
try {
70+
serialized = JSON.stringify(value);
71+
} catch (e: any) {
72+
return {ok: false, reason: `JSON.stringify failed: ${e && e.message || e}`};
73+
}
74+
if (serialized === undefined) {
75+
// JSON.stringify returns undefined for top-level functions/undefined.
76+
return {ok: false, reason: 'value is not JSON-serializable (function/undefined)'};
77+
}
78+
const bytes = Buffer.byteLength(serialized, 'utf8');
79+
if (bytes > maxBytes) {
80+
return {ok: false, reason: `serialized size ${bytes}B exceeds per-key cap ${maxBytes}B`};
81+
}
82+
return {ok: true, bytes};
4683
};
4784

4885
/**
@@ -87,7 +124,7 @@ class Pad {
87124

88125
static normalizePadSettings(rawPadSettings: any = {}): PadSettings {
89126
const rawView = rawPadSettings.view ?? {};
90-
return {
127+
const result: PadSettings = {
91128
enforceSettings: !!rawPadSettings.enforceSettings,
92129
showChat: rawPadSettings.showChat == null ? settings.padOptions.showChat !== false :
93130
!!rawPadSettings.showChat,
@@ -109,6 +146,28 @@ class Pad {
109146
!!rawView.fadeInactiveAuthorColors,
110147
},
111148
};
149+
if (settings.enablePluginPadOptions) {
150+
let totalBytes = 0;
151+
for (const [k, v] of Object.entries(rawPadSettings)) {
152+
if (!PLUGIN_KEY_RE.test(k)) continue;
153+
const check = validatePluginValue(k, v, PLUGIN_KEY_MAX_BYTES);
154+
if (!check.ok) {
155+
// Drop and log. Persistence/broadcast still rejects the value, but
156+
// the rest of the settings round-trip cleanly.
157+
console.warn(`[normalizePadSettings] dropping ${k}: ${check.reason}`);
158+
continue;
159+
}
160+
if (totalBytes + check.bytes > PLUGIN_TOTAL_MAX_BYTES) {
161+
console.warn(
162+
`[normalizePadSettings] dropping ${k}: combined ep_* size ` +
163+
`would exceed cap ${PLUGIN_TOTAL_MAX_BYTES}B`);
164+
continue;
165+
}
166+
totalBytes += check.bytes;
167+
result[k] = v;
168+
}
169+
}
170+
return result;
112171
}
113172

114173
apool() {

src/node/handler/PadMessageHandler.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1174,6 +1174,7 @@ const handleClientReady = async (socket:any, message: ClientReadyMessage) => {
11741174
},
11751175
enableDarkMode: settings.enableDarkMode,
11761176
enablePadWideSettings: settings.enablePadWideSettings,
1177+
enablePluginPadOptions: settings.enablePluginPadOptions,
11771178
padDeletionToken,
11781179
// Allow-listed copy — settings.privacyBanner could carry extra nested
11791180
// keys from a hand-edited settings.json; sending those by reference
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
'use strict';
2+
3+
// Capability flags exposed to Etherpad plugins for runtime feature detection.
4+
// Plugins should `try { require('ep_etherpad-lite/node/utils/PluginCapabilities') }
5+
// catch { /* old core */ }` and degrade gracefully when a flag is missing.
6+
//
7+
// IMPORTANT: a flag here means the core implements the capability — it does
8+
// not mean the capability is currently enabled on this Etherpad instance.
9+
// Capabilities can be gated by per-instance settings; plugins must inspect
10+
// the relevant runtime flag (typically reflected through clientVars) to
11+
// decide whether to actually use the feature on a given pad load.
12+
13+
// True when applyPadSettings (client + server) preserves keys matching
14+
// /^ep_[a-z0-9_]+$/ on pad.padOptions. The runtime gate is
15+
// settings.enablePluginPadOptions (default false), mirrored to clients via
16+
// clientVars.enablePluginPadOptions. See doc/plugins.md for the full
17+
// contract (key namespace, validation, size caps).
18+
export const padOptionsPluginPassthrough = true;

src/node/utils/Settings.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,7 @@ export type SettingsType = {
184184
updateServer: string,
185185
enableDarkMode: boolean,
186186
enablePadWideSettings: boolean,
187+
enablePluginPadOptions: boolean,
187188
allowPadDeletionByAllUsers: boolean,
188189
privacyBanner: {
189190
enabled: boolean,
@@ -332,7 +333,7 @@ export type SettingsType = {
332333
requireAdminForStatus: boolean,
333334
},
334335
adminEmail: string | null,
335-
getPublicSettings: () => Pick<SettingsType, "title" | "skinVariants"|"randomVersionString"|"skinName"|"toolbar"| "exposeVersion"| "gitVersion" | "enablePadWideSettings" | "privacyBanner">,
336+
getPublicSettings: () => Pick<SettingsType, "title" | "skinVariants"|"randomVersionString"|"skinName"|"toolbar"| "exposeVersion"| "gitVersion" | "enablePadWideSettings" | "enablePluginPadOptions" | "privacyBanner">,
336337
}
337338

338339
const settings: SettingsType = {
@@ -397,6 +398,11 @@ const settings: SettingsType = {
397398
updateServer: "https://static.etherpad.org",
398399
enableDarkMode: true,
399400
enablePadWideSettings: true,
401+
// New plugin-padOption passthrough is opt-in per AGENTS.MD §52 ("New
402+
// features should be placed behind feature flags and disabled by
403+
// default"). Flip to true to let plugins (e.g. ep_plugin_helpers'
404+
// padToggle) ride the existing padoptions broadcast/persist rail.
405+
enablePluginPadOptions: false,
400406
allowPadDeletionByAllUsers: false,
401407
privacyBanner: {
402408
enabled: false,
@@ -770,6 +776,7 @@ const settings: SettingsType = {
770776
skinName: settings.skinName,
771777
skinVariants: settings.skinVariants,
772778
enablePadWideSettings: settings.enablePadWideSettings,
779+
enablePluginPadOptions: settings.enablePluginPadOptions,
773780
privacyBanner: getPublicPrivacyBanner(),
774781
}
775782
},

src/static/js/pad.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -874,6 +874,16 @@ const pad = {
874874
pad.padOptions.view[k] = v;
875875
}
876876
}
877+
// Plugin-namespaced keys (ep_*) are passed through verbatim so plugins
878+
// can ride the existing padoptions broadcast/persist rail. Gated on
879+
// settings.enablePluginPadOptions (mirrored to clientVars by
880+
// getPublicSettings). Server-side normalizePadSettings preserves the
881+
// same keys symmetrically.
882+
if (clientVars.enablePluginPadOptions) {
883+
for (const [k, v] of Object.entries(opts)) {
884+
if (/^ep_[a-z0-9_]+$/.test(k)) pad.padOptions[k] = v;
885+
}
886+
}
877887
normalizeChatOptions(pad.padOptions);
878888
pad.refreshPadSettingsControls();
879889
pad.applyOptionsChange();

src/static/js/types/SocketIOMessage.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,12 @@ export type PadOption = {
260260
"alwaysShowChat"?: boolean,
261261
"chatAndUsers"?: boolean,
262262
"lang"?: null|string,
263-
view? : MapArrayType<boolean|string>
263+
view? : MapArrayType<boolean|string>,
264+
// Plugin-namespaced pad-wide options (gated by settings.enablePluginPadOptions).
265+
// The runtime regex is /^ep_[a-z0-9_]+$/ — TypeScript template literals
266+
// can't constrain that exactly, so this signature accepts any ep_-prefixed
267+
// string and applyPadSettings/normalizePadSettings reject the rest.
268+
[k: `ep_${string}`]: unknown,
264269
}
265270

266271

0 commit comments

Comments
 (0)