fix(deb): keep plugin_packages in-tree so admin-installed plugins can resolve ep_etherpad-lite

[JohnMcLear]

Summary

• Postinstall no longer symlinks /opt/etherpad/src/plugin_packages to /var/lib/etherpad/plugin_packages; it stays a real in-tree directory and is made group-writable by the etherpad user, same pattern already used for src/node_modules.
• etherpad.service adds /opt/etherpad/src/plugin_packages to ReadWritePaths= so the admin UI can still write under ProtectSystem=strict.
• Upgrade path: if a previous install left the symlink in place, the postinstall copies its target's contents back in-tree before dropping the symlink, so existing third-party plugins aren't lost.

Why

Reproduced from ether/ep_comments_page#416: admin-installs of ep_comments_page on a .deb-based deployment crash the service on every restart with:

Failed to load hook function ".../ep_comments_page/index:expressCreateServer" ...
Error: Cannot find module 'ep_etherpad-lite/node/eejs/'
Require stack:
- /var/lib/etherpad/plugin_packages/.versions/ep_plugin_helpers@0.5.2/eejs-blocks.js
- /var/lib/etherpad/plugin_packages/.versions/ep_plugin_helpers@0.5.2/index.js
- /var/lib/etherpad/plugin_packages/.versions/ep_comments_page@11.1.7/index.js
- /opt/etherpad/src/static/js/pluginfw/...

Root cause is in our packaging, not the plugin. The old postinstall created /opt/etherpad/src/plugin_packages → /var/lib/etherpad/plugin_packages. live-plugin-manager writes new plugins to the symlink target's realpath, and Node.js resolves the realpath before walking node_modules upward — so the resolver visits /var/lib/etherpad/, /var/lib/, /var/ and never reaches the bundled ep_etherpad-lite symlink in /opt/etherpad/node_modules. Every require('ep_etherpad-lite/...') (and the matching esbuild client-bundle build) throws MODULE_NOT_FOUND, etherpad exits with Error occurred while starting Etherpad, and systemd restart-loops.

Keeping plugin_packages in-tree means the resolver walks /opt/etherpad/src/plugin_packages/.versions/... → /opt/etherpad/src/node_modules → /opt/etherpad/node_modules/ep_etherpad-lite (which symlinks to ../src). Lookups succeed and the service comes up cleanly.

Reproduction (before this PR)

Verified locally on a fresh ether/etherpad@v2.7.3 clone with pnpm run plugins i ep_comments_page:

Layout	Result
src/plugin_packages real directory (workspace dev install)	✅ Etherpad starts, all hooks load
src/plugin_packages → /tmp/... symlink (current .deb layout)	❌ Same hook-load errors as the issue, esbuild build fails, process exits

After this PR the .deb layout matches the working case.

Test plan

• CI builds .deb artifacts cleanly with updated postinstall and unit.
• Manual: fresh .deb install → admin UI installs ep_comments_page → systemctl restart etherpad → journalctl -u etherpad shows no hook-load errors and the pad loads with the comment toolbar.
• Manual: upgrade from a pre-fix .deb that has /opt/etherpad/src/plugin_packages pointing at /var/lib/etherpad/plugin_packages (with existing third-party plugins inside) → migration copies them in-tree, etherpad restarts cleanly.

🤖 Generated with Claude Code

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

[qodo-free-for-open-source-projects]

Review Summary by Qodo

Fix admin-installed plugins by keeping plugin_packages in-tree

🐞 Bug fix

Walkthroughs

Description

• Keep plugin_packages as real in-tree directory instead of symlink
• Fixes admin-installed plugins unable to resolve ep_etherpad-lite module
• Make plugin_packages and .versions subdirectory group-writable by etherpad
• Migrate existing symlinked plugin packages back in-tree on upgrade

Diagram

flowchart LR
  A["Old Layout:<br/>plugin_packages symlink<br/>→ /var/lib/etherpad"] -->|"Node.js resolves<br/>to realpath"| B["Resolver at<br/>/var/lib/etherpad"]
  B -->|"Cannot reach"| C["❌ ep_etherpad-lite<br/>in /opt/etherpad"]
  D["New Layout:<br/>plugin_packages real<br/>directory in /opt"] -->|"Resolver walks<br/>in-tree"| E["/opt/etherpad/src<br/>node_modules"]
  E -->|"Finds"| F["✅ ep_etherpad-lite<br/>symlink"]

Loading

File Changes

1. packaging/scripts/postinstall.sh 🐞 Bug fix +29/-20

Convert plugin_packages from symlink to in-tree directory

• Removed symlink creation; plugin_packages now stays as real in-tree directory
• Added migration logic to copy contents from old symlink target back in-tree on upgrade
• Made plugin_packages and .versions subdirectory group-writable by etherpad user
• Consolidated directory permission setup for both node_modules and plugin_packages

packaging/scripts/postinstall.sh

---

2. packaging/systemd/etherpad.service ⚙️ Configuration changes +9/-5

Expose plugin_packages as writable path in systemd unit

• Added /opt/etherpad/src/plugin_packages to ReadWritePaths= configuration
• Updated comments to explain why both node_modules and plugin_packages must be writable
• Documented the symlink resolution issue that necessitates keeping paths in-tree

packaging/systemd/etherpad.service

---

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0)

1. No test for plugin_packages migration ✓ Resolved 📘 Rule violation ☼ Reliability

Description

This PR changes Debian postinstall/systemd behavior for src/plugin_packages but does not
add/update an automated regression test to validate the new directory layout and upgrade migration.
Without a test, future packaging changes could reintroduce the MODULE_NOT_FOUND failure or break
upgrades unnoticed.

Code

packaging/scripts/postinstall.sh[R69-103]

# Plugin install paths. Etherpad's admin UI installs plugins into
# ${root}/src/plugin_packages and creates symlinks under
-    # ${root}/src/node_modules. Both are under /opt and would EACCES
-    # under the etherpad user without these adjustments.
-    PLUGIN_PKG_LIVE=/var/lib/etherpad/plugin_packages
-    PLUGIN_PKG_LINK="${APP_DIR}/src/plugin_packages"
+    # ${root}/src/node_modules. Both ship root-owned but need to be
+    # group-writable by the etherpad user so the admin UI can add packages.
+    PLUGIN_PKG_DIR="${APP_DIR}/src/plugin_packages"
NODE_MODULES_DIR="${APP_DIR}/src/node_modules"
-    mkdir -p "${PLUGIN_PKG_LIVE}"
-    if [ -e "${PLUGIN_PKG_LINK}" ] && [ ! -L "${PLUGIN_PKG_LINK}" ]; then
-      cp -a "${PLUGIN_PKG_LINK}/." "${PLUGIN_PKG_LIVE}/" 2>/dev/null || true
-      rm -rf "${PLUGIN_PKG_LINK}"
+    # Migrate any previous install that symlinked plugin_packages outside
+    # the tree (see ether/ep_comments_page#416). Node.js resolves symlinks
+    # to their realpath before walking node_modules, so plugins installed
+    # under /var/lib/etherpad/plugin_packages couldn't reach the bundled
+    # ep_etherpad-lite in /opt/etherpad/node_modules and every
+    # require('ep_etherpad-lite/...') threw MODULE_NOT_FOUND. Pull the
+    # symlink target's contents back in-tree and drop the symlink.
+    if [ -L "${PLUGIN_PKG_DIR}" ]; then
+      OLD_PLUGIN_PKG_LIVE=$(readlink -f "${PLUGIN_PKG_DIR}" 2>/dev/null || true)
+      rm -f "${PLUGIN_PKG_DIR}"
+      mkdir -p "${PLUGIN_PKG_DIR}"
+      if [ -n "${OLD_PLUGIN_PKG_LIVE}" ] && [ -d "${OLD_PLUGIN_PKG_LIVE}" ]; then
+        cp -a "${OLD_PLUGIN_PKG_LIVE}/." "${PLUGIN_PKG_DIR}/" 2>/dev/null || true
+      fi
fi
-    # chown after the cp -- cp -a preserves the (root) ownership of the
-    # staged source files and would re-root anything we chowned earlier.
-    chown -hR etherpad:etherpad "${PLUGIN_PKG_LIVE}"
-    ln -sfn "${PLUGIN_PKG_LIVE}" "${PLUGIN_PKG_LINK}"
+    mkdir -p "${PLUGIN_PKG_DIR}"
-    # node_modules is bundled (root-owned contents); the directory itself
-    # must be group-writable by etherpad so plugin installs can create
-    # symlinks alongside the shipped packages. ReadWritePaths in the unit
-    # also exposes it as writable under ProtectSystem=strict.
-    if [ -d "${NODE_MODULES_DIR}" ]; then
-      chgrp etherpad "${NODE_MODULES_DIR}"
-      chmod 2775 "${NODE_MODULES_DIR}"
-    fi
+    # plugin_packages and node_modules contain bundled (root-owned)
+    # packages; the *directories themselves* (plus plugin_packages/.versions
+    # where live-plugin-manager stages downloads) must be group-writable by
+    # etherpad so the admin UI can install new plugins alongside the
+    # shipped deps. The unit's ReadWritePaths= also exposes both paths as
+    # writable under ProtectSystem=strict.
+    for dir in "${PLUGIN_PKG_DIR}" "${PLUGIN_PKG_DIR}/.versions" "${NODE_MODULES_DIR}"; do
+      [ -d "${dir}" ] || continue
+      chgrp etherpad "${dir}"
+      chmod 2775 "${dir}"
+    done

Evidence

PR Compliance ID 1 requires a regression test for bug fixes. The PR changes postinstall.sh to stop
using the /var/lib/etherpad/plugin_packages symlink and to migrate old symlink installs, and
updates systemd ReadWritePaths= accordingly, but the existing packaging smoke test still asserts
the old symlink behavior (test -L /opt/etherpad/src/plugin_packages), meaning there is no passing
automated test that would fail if this fix were reverted.

packaging/scripts/postinstall.sh[69-103]
packaging/systemd/etherpad.service[43-51]
packaging/test-local.sh[133-150]
Best Practice: Repository guidelines

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The PR fixes a Debian packaging bug by keeping `src/plugin_packages` in-tree and adjusting permissions/systemd sandboxing, but there is no updated regression test to enforce the new behavior.
## Issue Context
`packaging/test-local.sh` currently asserts the old symlink-based layout for `/opt/etherpad/src/plugin_packages`, so it will not validate (and will likely fail under) the new intended behavior.
## Fix Focus Areas
- packaging/test-local.sh[133-150]
- packaging/scripts/postinstall.sh[69-103]
- packaging/systemd/etherpad.service[43-51]

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

---

2. CI symlink assertions stale ✓ Resolved 🐞 Bug ≡ Correctness

Description

packaging/scripts/postinstall.sh no longer creates /opt/etherpad/src/plugin_packages as a symlink,
but the Debian CI workflow and packaging/test-local.sh still assert it is a symlink to
/var/lib/etherpad/plugin_packages, causing CI/test failures.

Code

packaging/scripts/postinstall.sh[R83-92]

+    if [ -L "${PLUGIN_PKG_DIR}" ]; then
+      OLD_PLUGIN_PKG_LIVE=$(readlink -f "${PLUGIN_PKG_DIR}" 2>/dev/null || true)
+      rm -f "${PLUGIN_PKG_DIR}"
+      mkdir -p "${PLUGIN_PKG_DIR}"
+      if [ -n "${OLD_PLUGIN_PKG_LIVE}" ] && [ -d "${OLD_PLUGIN_PKG_LIVE}" ]; then
+        cp -a "${OLD_PLUGIN_PKG_LIVE}/." "${PLUGIN_PKG_DIR}/" 2>/dev/null || true
+      fi
fi
-    # chown after the cp -- cp -a preserves the (root) ownership of the
-    # staged source files and would re-root anything we chowned earlier.
-    chown -hR etherpad:etherpad "${PLUGIN_PKG_LIVE}"
-    ln -sfn "${PLUGIN_PKG_LIVE}" "${PLUGIN_PKG_LINK}"
+    mkdir -p "${PLUGIN_PKG_DIR}"
-    # node_modules is bundled (root-owned contents); the directory itself
-    # must be group-writable by etherpad so plugin installs can create
-    # symlinks alongside the shipped packages. ReadWritePaths in the unit
-    # also exposes it as writable under ProtectSystem=strict.
-    if [ -d "${NODE_MODULES_DIR}" ]; then
-      chgrp etherpad "${NODE_MODULES_DIR}"
-      chmod 2775 "${NODE_MODULES_DIR}"
-    fi

Evidence

The new postinstall logic explicitly removes the symlink (if present) and creates a real directory,
but CI and the local packaging test script still assert the path is a symlink pointing to
/var/lib/etherpad/plugin_packages.

packaging/scripts/postinstall.sh[69-103]
.github/workflows/deb-package.yml[170-179]
packaging/test-local.sh[133-146]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`postinstall.sh` now migrates away from a symlinked `src/plugin_packages` and ensures it is a real directory. CI and local packaging tests still check for the old symlink layout, so they will fail.
### Issue Context
The deb packaging validation in `.github/workflows/deb-package.yml` and `packaging/test-local.sh` must be updated to match the new in-tree directory layout and permissions.
### Fix Focus Areas
- .github/workflows/deb-package.yml[170-179]
- packaging/test-local.sh[133-146]
### What to change
- Replace `test -L /opt/etherpad/src/plugin_packages` + `readlink` assertions with `test -d /opt/etherpad/src/plugin_packages`.
- Replace `/var/lib/etherpad/plugin_packages` existence/ownership checks with permission/group checks on `/opt/etherpad/src/plugin_packages` (and optionally `/opt/etherpad/src/plugin_packages/.versions` if you decide to pre-create it), consistent with `postinstall.sh` (group=etherpad, mode 2775).

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

---

3. Purge cleanup misses plugins 🐞 Bug ☼ Reliability

Description

Admin-installed plugins are written to /opt/etherpad/src/plugin_packages, but
packaging/scripts/postremove.sh does not remove that directory on purge, so third-party plugin files
can be left behind after a purge.

Code

packaging/scripts/postinstall.sh[R73-103]

+    PLUGIN_PKG_DIR="${APP_DIR}/src/plugin_packages"
NODE_MODULES_DIR="${APP_DIR}/src/node_modules"
-    mkdir -p "${PLUGIN_PKG_LIVE}"
-    if [ -e "${PLUGIN_PKG_LINK}" ] && [ ! -L "${PLUGIN_PKG_LINK}" ]; then
-      cp -a "${PLUGIN_PKG_LINK}/." "${PLUGIN_PKG_LIVE}/" 2>/dev/null || true
-      rm -rf "${PLUGIN_PKG_LINK}"
+    # Migrate any previous install that symlinked plugin_packages outside
+    # the tree (see ether/ep_comments_page#416). Node.js resolves symlinks
+    # to their realpath before walking node_modules, so plugins installed
+    # under /var/lib/etherpad/plugin_packages couldn't reach the bundled
+    # ep_etherpad-lite in /opt/etherpad/node_modules and every
+    # require('ep_etherpad-lite/...') threw MODULE_NOT_FOUND. Pull the
+    # symlink target's contents back in-tree and drop the symlink.
+    if [ -L "${PLUGIN_PKG_DIR}" ]; then
+      OLD_PLUGIN_PKG_LIVE=$(readlink -f "${PLUGIN_PKG_DIR}" 2>/dev/null || true)
+      rm -f "${PLUGIN_PKG_DIR}"
+      mkdir -p "${PLUGIN_PKG_DIR}"
+      if [ -n "${OLD_PLUGIN_PKG_LIVE}" ] && [ -d "${OLD_PLUGIN_PKG_LIVE}" ]; then
+        cp -a "${OLD_PLUGIN_PKG_LIVE}/." "${PLUGIN_PKG_DIR}/" 2>/dev/null || true
+      fi
fi
-    # chown after the cp -- cp -a preserves the (root) ownership of the
-    # staged source files and would re-root anything we chowned earlier.
-    chown -hR etherpad:etherpad "${PLUGIN_PKG_LIVE}"
-    ln -sfn "${PLUGIN_PKG_LIVE}" "${PLUGIN_PKG_LINK}"
+    mkdir -p "${PLUGIN_PKG_DIR}"
-    # node_modules is bundled (root-owned contents); the directory itself
-    # must be group-writable by etherpad so plugin installs can create
-    # symlinks alongside the shipped packages. ReadWritePaths in the unit
-    # also exposes it as writable under ProtectSystem=strict.
-    if [ -d "${NODE_MODULES_DIR}" ]; then
-      chgrp etherpad "${NODE_MODULES_DIR}"
-      chmod 2775 "${NODE_MODULES_DIR}"
-    fi
+    # plugin_packages and node_modules contain bundled (root-owned)
+    # packages; the *directories themselves* (plus plugin_packages/.versions
+    # where live-plugin-manager stages downloads) must be group-writable by
+    # etherpad so the admin UI can install new plugins alongside the
+    # shipped deps. The unit's ReadWritePaths= also exposes both paths as
+    # writable under ProtectSystem=strict.
+    for dir in "${PLUGIN_PKG_DIR}" "${PLUGIN_PKG_DIR}/.versions" "${NODE_MODULES_DIR}"; do
+      [ -d "${dir}" ] || continue
+      chgrp etherpad "${dir}"
+      chmod 2775 "${dir}"
+    done

Evidence

Core plugin installation writes to ${settings.root}/src/plugin_packages, the postinstall script
ensures that directory exists and is writable, but the purge script does not delete anything under
/opt/etherpad.

src/static/js/pluginfw/installer.ts[21-28]
packaging/scripts/postinstall.sh[69-103]
packaging/scripts/postremove.sh[22-33]
packaging/nfpm.yaml[47-59]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
With `plugin_packages` now kept inside `/opt/etherpad/src/plugin_packages`, plugins installed via the admin UI will be stored under `/opt/etherpad`. The `postremove.sh purge` action currently deletes `/etc/etherpad`, `/var/lib/etherpad`, and `/var/log/etherpad` but does not delete `/opt/etherpad/src/plugin_packages`, so purging can leave installed plugins behind.
### Issue Context
Etherpad installs plugins into `settings.root/src/plugin_packages` at runtime. The package definition installs the application tree to `/opt/etherpad`, so runtime plugin artifacts can accumulate there.
### Fix Focus Areas
- packaging/scripts/postremove.sh[9-37]
### What to change
- In the `purge)` case, explicitly remove runtime-created plugin artifacts under `/opt/etherpad` (at minimum `${APP_DIR}/src/plugin_packages`; consider also `${APP_DIR}/src/node_modules` if plugins create symlinks there, or remove `${APP_DIR}` entirely if purge is intended to remove all application+plugin artifacts).
- Keep the `remove)` behavior as-is if the intent is to preserve plugins on non-purge removal.

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

---

[JohnMcLear]

Addressed Qodo review item 3 ("Purge cleanup misses plugins") in a6ec3cc:

packaging/scripts/postremove.sh purge: rm -rf plugin_packages, any runtime ep_* symlinks in src/node_modules, and the whole APP_DIR as belt-and-braces. Sandbox-verified that a staged .versions/ep_runtime@1.0.0/ + node_modules/ep_runtime symlink are both gone after the new purge runs.

Test coverage added: both packaging/test-local.sh and the deb-package workflow now assert /opt/etherpad/src/plugin_packages and /var/lib/etherpad are absent after dpkg --purge etherpad. Also rebased onto develop (ab60ed3) to pick up the Windows + Node 24 backend-test flake fix from #7748.