Commit graph

2 commits

Author SHA1 Message Date
John McLear
85c941fe95
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>
2026-05-07 17:17:05 +01:00
John McLear
02e37e0112
feat(packaging): publish Etherpad as a Snap (#7558)
* feat(packaging): publish Etherpad as a Snap

Adds first-class Snap packaging so Ubuntu / snapd users can install via
`sudo snap install etherpad-lite`.

- snap/snapcraft.yaml — core24, strict confinement, builds with pnpm
  against a pinned Node.js 22 runtime. Version is auto-derived from
  src/package.json so `snap info` tracks upstream release numbering.
- snap/local/bin/etherpad-service — launch wrapper that seeds
  $SNAP_COMMON/etc/settings.json on first run (rewriting the default
  dirty-DB path to a writable $SNAP_COMMON location) and execs Etherpad
  via `node --import tsx/esm`.
- snap/local/bin/etherpad-healthcheck-wrapper — HTTP probe for external
  supervisors, falling back to Node if curl isn't staged.
- snap/local/bin/etherpad-cli — thin passthrough to Etherpad's bin/
  scripts (importSqlFile, checkPad, etc.).
- snap/hooks/configure — exposes `snap set etherpad-lite port=<n>` and
  `ip=<addr>` with validation, restarts the service when running.
- snap/README.md — build / install / configure / publish instructions.
- .github/workflows/snap-publish.yml — builds on every v* tag, uploads
  a short-lived artifact, publishes to `edge`, and then promotes to
  `stable` through a manually-approved GitHub Environment. Requires a
  one-time `snapcraft register etherpad-lite` plus provisioning of the
  `SNAPCRAFT_STORE_CREDENTIALS` repo secret (instructions inline).

Pad data (dirty DB, logs) lives in /var/snap/etherpad-lite/common/ and
survives snap refreshes. The read-only $SNAP squashfs is never written
to at runtime.

Refs #7529

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

* fix(snap): pass --settings flag, env-subst ip/port, 2-space indent

Addresses Qodo review feedback on #7558:

1. Settings file ignored: Etherpad's Settings loader reads `argv.settings`,
   not the `EP_SETTINGS` env var. Without `--settings`, the launcher's
   seeded $SNAP_COMMON/etc/settings.json is never loaded; Etherpad falls
   back to <install-root>/settings.json, which lives on the read-only
   squashfs — so the default dirty-DB path ends up unwritable and the
   daemon fails to persist pads. Fix: pass `--settings "${SETTINGS}"` to
   node; drop the EP_SETTINGS export.

2. `snap set` overrides were no-ops: the seeded settings.json carries the
   template's literal `"ip": "0.0.0.0"` / `"port": 9001` values, which
   override the env-based defaults Etherpad exposes via ${…}
   substitution. Users following the README saw the listener stay put
   after `snap set etherpad-lite port=…`. Fix: after copying the
   template on first run, rewrite the top-level `ip` and `port` lines
   to `"${IP:0.0.0.0}"` / `"${PORT:9001}"`. Use `0,/…/` anchors so the
   `dbSettings.port` entry further down stays literal.

3. Indentation: reflow the new shell scripts from 4-space to 2-space to
   match the repo style rule.

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

* fix(snap): default seeded settings to sqlite, not dirty

settings.json.template's own comment says dirty is for testing only.
A Snap install is the "not testing" case — shipping it by default
means every `sudo snap install etherpad-lite` starts on a DB the
project explicitly recommends against.

Rewrite the postinstall sed to switch dbType: "dirty" → "sqlite" and
point filename at $SNAP_COMMON/var/etherpad.db. sqlite is already
shipped in-tree via ueberdb2 → rusty-store-kv (prebuilt napi-rs
binary, no build deps), so this works under strict confinement with
zero snap.yaml changes.

Only affects first-run seeding; existing $SNAP_COMMON/etc/settings.json
is never touched on refresh.

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

* fix(snap): rename to "etherpad", glob tag filter, harden cli

- Snap is registered as `etherpad` (the project's only name) — drops the
  legacy `etherpad-lite` from the name, app, paths, install dir, configure
  hook, README and workflow artifact. The daemon app shares the snap name,
  so `snap install etherpad` exposes a bare `etherpad` command; the bin/
  passthrough is now `etherpad.cli`.
- snap-publish.yml: GitHub Actions tag filters use globs, not regex. The
  prior `v?[0-9]+.[0-9]+.[0-9]+` pattern would never match a real release
  tag (Qodo review). Replace with two glob entries covering `vX.Y.Z` and
  `X.Y.Z`.
- etherpad-cli: reject path-traversal in the `<bin-script>` arg (anything
  containing `/`, `..`, or empty) and add a default `*)` case so files
  with unsupported extensions fail loud instead of silently exiting 0
  (Qodo review).

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

* fix(snap): unbreak build — refresh corepack, drop pnpm prune

Two issues hit on the first real `snapcraft pack` of this recipe:

- `corepack prepare pnpm@10.33.0 --activate` failed with
  `Cannot find matching keyid` because Node 22.12's bundled corepack
  ships a stale signing-key list and rejects newer pnpm releases
  (nodejs/corepack#612). Refresh corepack itself via npm before
  preparing pnpm.
- `pnpm prune --prod` is interactive on workspace projects: it asks
  "The modules directories will be removed and reinstalled from
  scratch. Proceed? (Y/n)" and deadlocks on stdin under sudo + tee.
  Replace it with the explicit "wipe node_modules + prod reinstall"
  pattern, which is non-interactive, faster (pnpm resolves the prod
  graph from its CAS cache), and byte-identical in result.

Verified locally: `snapcraft pack --destructive-mode` produces
`etherpad_2.6.1_amd64.snap` end-to-end in ~3 min.

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

* fix(snap): unbreak runtime — tsx resolution, var/ writability, env

Three runtime crashes surfaced when actually installing the built snap
under strict confinement. Fixed each, plus a smoke-test script.

- `tsx` is in the `src` workspace's node_modules under pnpm hoisting,
  not at the snap install root. The wrapper now `cd "${APP_DIR}/src"`
  and uses bare `--import tsx` (matching `bin/cleanRun.sh`); the prior
  `--import tsx/esm` triggered ERR_REQUIRE_CYCLE on Etherpad's mixed
  CJS/ESM source tree.
- Etherpad's plugin installer writes `var/installed_plugins.json` via
  __dirname-relative paths, which resolve to absolute paths inside the
  read-only snap squashfs (EROFS). snap layouts can't intercept paths
  inside `$SNAP`, so replace the shipped `var/` dir with a symlink to
  `/var/snap/etherpad/common/etherpad-app-var/` (auto-created by the
  wrapper on first run). Persistent state survives `snap refresh`.
- Drop the unused `EP_SETTINGS` and `EP_DATA_DIR` env vars from the
  app's `environment:` block. Etherpad's settings loader doesn't read
  them — it reads `argv.settings`, which the wrapper already passes via
  `--settings`. They were producing `[WARN] settings - Unknown Setting`
  noise on every start.

Add `snap/tests/smoke.sh`: rebuild + install + configure test port 9003
+ assert listener + curl /health + tail logs. Local verified output:
  HTTP 200, body {"status":"pass","releaseId":"2.6.1"}, server logs
  `Etherpad is running` on `http://0.0.0.0:9003/`.

.gitignore now excludes destructive-mode build outputs (parts/, stage/,
prime/, .craft/, *.snap).

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

* test(snap): wrapper unit tests, PR CI build, expanded docs

Coverage in snap/tests/ (47 assertions, ~5s, no snapd/sudo/network):
- test-snapcraft-yaml.sh: required keys, name validity, daemon-app
  matches snap name, no etherpad-lite regression, env-var whitelist.
- test-cli.sh: path-traversal rejection, .ts/.sh dispatch, default-case
  rejection, no-args usage.
- test-configure.sh: port (1-65535) and ip (v4/v6) validation via
  mocked snapctl.
- test-service-bootstrap.sh: first-run seeding from
  settings.json.template, sed rewrite of dbType/filename/ip/port,
  writable-dir creation, snapctl override propagation to node env,
  idempotency on second run, default fallbacks.
- run-all.sh: bash -n syntax check on every wrapper + hook, then
  sources each test file and reports totals. All assertions use port
  9003 (project test convention).

CI in .github/workflows/snap-build.yml:
- Triggers on PR / push-to-develop touching snap/, settings.json.template,
  or the workflow itself.
- Job 1 wrapper-tests: runs run-all.sh.
- Job 2 snap-pack: snapcraft pack --destructive-mode, uploads .snap as
  PR artifact for sideload.
- Stays separate from snap-publish.yml (tag-triggered, store-bound).

snap/README.md fully rewritten:
- User-facing usage, install, configure
- Architecture: file layout, var/-symlink rationale, settings.json
  rewrite rationale, double-pnpm-install rationale, daemon-name-shares-
  snap-name rationale
- Three test layers with exactly when/why to run each
- Dev workflow loop
- Publishing maintainer setup
- Troubleshooting for every failure mode hit during this PR (EROFS,
  tsx not found, ERR_REQUIRE_CYCLE, snap-store-down, pnpm prune hang)

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

* docs(snap): replace dead snapcraft.io/docs/releasing-to-the-snap-store link

That URL now 404s. Point at the canonical documentation.ubuntu.com
locations instead, broken out into the specific pages a maintainer
actually needs:

- Register a snap (to claim the name)
- snapcraft export-login (to generate the SNAPCRAFT_STORE_CREDENTIALS
  secret)
- Publishing how-to index (root index for everything else)

Same fix in the snap-publish.yml header comment.

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

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-02 13:19:10 +01:00