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

.github/workflows/deb-package.yml

@@ -172,10 +172,14 @@ jobs:
           sudo test -L /opt/etherpad/settings.json
           sudo test -L /opt/etherpad/var
           [ "$(sudo readlink /opt/etherpad/var)" = "/var/lib/etherpad/var" ]
-          sudo test -L /opt/etherpad/src/plugin_packages
-          [ "$(sudo readlink /opt/etherpad/src/plugin_packages)" = "/var/lib/etherpad/plugin_packages" ]
-          sudo test -d /var/lib/etherpad/plugin_packages
-          [ "$(sudo stat -c '%U' /var/lib/etherpad/plugin_packages)" = "etherpad" ]
+          # plugin_packages must stay in-tree -- Node.js resolves symlinks
+          # to realpath before walking node_modules, so symlinking it
+          # outside /opt broke require("ep_etherpad-lite/...") in
+          # admin-installed plugins (ether/ep_comments_page#416).
+          sudo test -d /opt/etherpad/src/plugin_packages
+          sudo test ! -L /opt/etherpad/src/plugin_packages
+          [ "$(sudo stat -c '%G' /opt/etherpad/src/plugin_packages)" = "etherpad" ]
+          [ "$(sudo stat -c '%a' /opt/etherpad/src/plugin_packages)" = "2775" ]
           [ "$(stat -c '%G' /opt/etherpad/src/node_modules)" = "etherpad" ]
           sudo test -f /var/lib/etherpad/var/installed_plugins.json
           sudo grep -q '"ep_etherpad-lite"' /var/lib/etherpad/var/installed_plugins.json
@@ -197,8 +201,71 @@ jobs:
             exit 1
           fi
           sudo systemctl stop etherpad
+          # Regression: simulate a pre-fix install (plugin_packages as a
+          # symlink to /var/lib/etherpad/plugin_packages, with a marker
+          # plugin inside) and re-run the postinst. The new postinst must
+          # migrate the contents back in-tree and drop the symlink so
+          # admin-installed plugins keep resolving ep_etherpad-lite
+          # (ether/ep_comments_page#416).
+          sudo rm -rf /opt/etherpad/src/plugin_packages
+          sudo mkdir -p /var/lib/etherpad/plugin_packages/.versions/ep_migration_marker
+          echo '{"name":"ep_migration_marker"}' | \
+            sudo tee /var/lib/etherpad/plugin_packages/.versions/ep_migration_marker/package.json >/dev/null
+          sudo chown -R etherpad:etherpad /var/lib/etherpad/plugin_packages
+          sudo ln -sfn /var/lib/etherpad/plugin_packages /opt/etherpad/src/plugin_packages
+          sudo dpkg-reconfigure etherpad
+          sudo test -d /opt/etherpad/src/plugin_packages
+          sudo test ! -L /opt/etherpad/src/plugin_packages
+          sudo test -f /opt/etherpad/src/plugin_packages/.versions/ep_migration_marker/package.json
+          [ "$(sudo stat -c '%a' /opt/etherpad/src/plugin_packages)" = "2775" ]
+
+          # Regression: stage the ep_layout_trip_wire test fixture into
+          # plugin_packages and confirm etherpad loads it. The fixture's
+          # index.js does the require('ep_etherpad-lite/...') calls that
+          # broke under the old symlinked layout (#416). If the layout
+          # ever regresses, the marker line never reaches the journal
+          # and this step fails.
+          PLUGIN_DIR=/opt/etherpad/src/plugin_packages
+          FIXTURE_DIR=$GITHUB_WORKSPACE/packaging/test-fixtures/ep_layout_trip_wire
+          sudo install -d -o etherpad -g etherpad -m 2775 "${PLUGIN_DIR}/.versions"
+          sudo cp -a "${FIXTURE_DIR}" "${PLUGIN_DIR}/.versions/ep_layout_trip_wire@1.0.0"
+          sudo ln -sfn .versions/ep_layout_trip_wire@1.0.0 "${PLUGIN_DIR}/ep_layout_trip_wire"
+          sudo ln -sfn ../plugin_packages/ep_layout_trip_wire \
+            /opt/etherpad/src/node_modules/ep_layout_trip_wire
+          sudo chown -R etherpad:etherpad "${PLUGIN_DIR}/.versions/ep_layout_trip_wire@1.0.0" \
+            "${PLUGIN_DIR}/ep_layout_trip_wire" \
+            /opt/etherpad/src/node_modules/ep_layout_trip_wire
+          echo '{"plugins":[{"name":"ep_etherpad-lite","version":"0.0.0"},{"name":"ep_layout_trip_wire","version":"1.0.0"}]}' \
+            | sudo tee /var/lib/etherpad/var/installed_plugins.json >/dev/null
+          sudo chown etherpad:etherpad /var/lib/etherpad/var/installed_plugins.json
+          sudo systemctl start etherpad
+          ok=
+          for i in $(seq 1 30); do
+            if curl -fsS http://127.0.0.1:9001/health; then ok=1; break; fi
+            sleep 2
+          done
+          if [ -z "${ok}" ]; then
+            sudo journalctl -u etherpad --no-pager -n 300 || true
+            exit 1
+          fi
+          # Marker proves the require('ep_etherpad-lite/...') calls in
+          # the fixture's index.js all resolved.
+          sudo journalctl -u etherpad --no-pager -n 500 \
+            | grep -F 'ep_layout_trip_wire: plugin_packages layout OK'
+          # And no MODULE_NOT_FOUND involving ep_etherpad-lite, anywhere.
+          if sudo journalctl -u etherpad --no-pager -n 500 \
+              | grep -E "Cannot find module '?ep_etherpad-lite"; then
+            echo "::error::ep_etherpad-lite require failed inside an installed plugin"
+            exit 1
+          fi
+          sudo systemctl stop etherpad
+
           sudo dpkg --purge etherpad
           ! id etherpad 2>/dev/null
+          # Purge must clean up runtime-created plugin artifacts that
+          # dpkg didn't ship (ether/ep_comments_page#416, Qodo #3).
+          sudo test ! -e /opt/etherpad/src/plugin_packages
+          sudo test ! -e /var/lib/etherpad
 
       - name: Upload artifact
         uses: actions/upload-artifact@v7

packaging/scripts/postinstall.sh

@@ -68,30 +68,39 @@ EOF
 
     # 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
 
     if [ -d /run/systemd/system ] && command -v systemctl >/dev/null 2>&1; then
       systemctl daemon-reload || true

packaging/scripts/postremove.sh

@@ -20,6 +20,22 @@ case "$1" in
     ;;
 
   purge)
+    # Runtime-created plugin artifacts that dpkg did not ship and so
+    # will not have cleaned up: the .versions/ stage that
+    # live-plugin-manager populates inside plugin_packages, and the
+    # corresponding ep_* symlinks in node_modules. After this PR
+    # plugin_packages lives in-tree under ${APP_DIR}/src/, so a stale
+    # purge would otherwise leave admin-installed plugins behind.
+    # See ether/ep_comments_page#416.
+    rm -rf "${APP_DIR}/src/plugin_packages"
+    if [ -d "${APP_DIR}/src/node_modules" ]; then
+      find "${APP_DIR}/src/node_modules" -maxdepth 1 -name 'ep_*' \
+        -exec rm -rf {} +
+    fi
+    # Belt-and-braces: anything else dpkg didn't manage inside the
+    # application tree gets cleaned up on purge too.
+    rm -rf "${APP_DIR}"
+
     rm -rf /etc/etherpad
     rm -rf /var/lib/etherpad
     rm -rf /var/log/etherpad

packaging/systemd/etherpad.service

@@ -40,11 +40,15 @@ SystemCallArchitectures=native
 RestrictAddressFamilies=AF_UNIX AF_INET AF_INET6 AF_NETLINK
 UMask=0027
 
-# /opt/etherpad/src/node_modules must be writable so the admin UI can
-# create symlinks for newly installed plugins alongside the bundled deps.
-# /opt/etherpad/src/plugin_packages is symlinked into /var/lib/etherpad
-# by postinstall, so it's already covered by the entry below.
-ReadWritePaths=/var/lib/etherpad /var/log/etherpad /etc/etherpad /opt/etherpad/src/node_modules
+# /opt/etherpad/src/{node_modules,plugin_packages} must be writable so the
+# admin UI can install new plugins alongside the bundled deps. We expose
+# both as writable paths under ProtectSystem=strict rather than symlinking
+# plugin_packages outside the tree -- Node.js resolves symlinks to their
+# realpath before walking node_modules, so a plugin installed under
+# /var/lib/etherpad cannot reach the bundled ep_etherpad-lite in
+# /opt/etherpad/node_modules and every require('ep_etherpad-lite/...')
+# fails with MODULE_NOT_FOUND (see ether/ep_comments_page#416).
+ReadWritePaths=/var/lib/etherpad /var/log/etherpad /etc/etherpad /opt/etherpad/src/node_modules /opt/etherpad/src/plugin_packages
 
 LimitNOFILE=65536
 

packaging/test-fixtures/ep_layout_trip_wire/README.md

@@ -0,0 +1,17 @@
+# ep_layout_trip_wire
+
+Test fixture for the Debian package CI. Not published, not loaded by
+any production Etherpad install.
+
+Exists to catch regressions in the packaging layout. When the `.deb`
+postinstall symlinked `/opt/etherpad/src/plugin_packages` outside the
+etherpad tree, Node.js resolved the symlink to its realpath before
+walking `node_modules` and every `require('ep_etherpad-lite/...')` in
+admin-installed plugins threw `MODULE_NOT_FOUND` (see
+[ether/ep_comments_page#416](https://github.com/ether/ep_comments_page/issues/416)).
+
+`packaging/test-local.sh` and `.github/workflows/deb-package.yml` stage
+this plugin into `/opt/etherpad/src/plugin_packages/.versions/...`,
+start etherpad, and grep `journalctl` for the marker emitted from
+`expressCreateServer`. If any of the `require('ep_etherpad-lite/...')`
+calls in `index.js` fail, the marker never appears and the test fails.

packaging/test-fixtures/ep_layout_trip_wire/ep.json

@@ -0,0 +1,10 @@
+{
+  "parts": [
+    {
+      "name": "trip_wire",
+      "hooks": {
+        "expressCreateServer": "ep_layout_trip_wire/index"
+      }
+    }
+  ]
+}

packaging/test-fixtures/ep_layout_trip_wire/index.js

@@ -0,0 +1,26 @@
+'use strict';
+
+// Each of these require()s would throw MODULE_NOT_FOUND if
+// /opt/etherpad/src/plugin_packages were symlinked outside the etherpad
+// tree -- Node resolves the symlink to its realpath before walking
+// node_modules and never reaches the bundled ep_etherpad-lite under
+// /opt/etherpad/node_modules. See ether/ep_comments_page#416.
+const eejs = require('ep_etherpad-lite/node/eejs/');
+const settings = require('ep_etherpad-lite/node/utils/Settings');
+const log4js = require('ep_etherpad-lite/node_modules/log4js');
+const {randomString} = require('ep_etherpad-lite/static/js/pad_utils');
+
+const logger = log4js.getLogger('ep_layout_trip_wire');
+
+// CI greps the journal for this exact line.
+const MARKER = 'ep_layout_trip_wire: plugin_packages layout OK';
+
+exports.expressCreateServer = (hookName, ctx, cb) => {
+  // Touch each binding so a future "require but never use" lint can't
+  // dead-code-eliminate them and silently weaken the test.
+  void eejs.require;
+  void settings.title;
+  void randomString;
+  logger.info(MARKER);
+  return cb();
+};

packaging/test-fixtures/ep_layout_trip_wire/package.json

@@ -0,0 +1,8 @@
+{
+  "name": "ep_layout_trip_wire",
+  "version": "1.0.0",
+  "description": "Test fixture - exercises require('ep_etherpad-lite/...') from a deb-installed location. Do not publish.",
+  "main": "index.js",
+  "license": "Apache-2.0",
+  "private": true
+}

packaging/test-local.sh

@@ -138,10 +138,14 @@ docker exec "${CONTAINER_NAME}" bash -lc '
   test -L /opt/etherpad/settings.json
   test -L /opt/etherpad/var
   [ "$(readlink /opt/etherpad/var)" = "/var/lib/etherpad/var" ]
-  test -L /opt/etherpad/src/plugin_packages
-  [ "$(readlink /opt/etherpad/src/plugin_packages)" = "/var/lib/etherpad/plugin_packages" ]
-  test -d /var/lib/etherpad/plugin_packages
-  [ "$(stat -c %U /var/lib/etherpad/plugin_packages)" = "etherpad" ]
+  # plugin_packages must stay in-tree -- Node.js resolves symlinks to
+  # realpath before walking node_modules, so symlinking it outside /opt
+  # broke require("ep_etherpad-lite/...") in admin-installed plugins.
+  # See ether/ep_comments_page#416.
+  test -d /opt/etherpad/src/plugin_packages
+  test ! -L /opt/etherpad/src/plugin_packages
+  [ "$(stat -c %G /opt/etherpad/src/plugin_packages)" = "etherpad" ]
+  [ "$(stat -c %a /opt/etherpad/src/plugin_packages)" = "2775" ]
   [ "$(stat -c %G /opt/etherpad/src/node_modules)" = "etherpad" ]
   test -f /var/lib/etherpad/var/installed_plugins.json
   grep -q "ep_etherpad-lite" /var/lib/etherpad/var/installed_plugins.json
@@ -181,6 +185,98 @@ echo "==> /health OK"
 docker exec "${CONTAINER_NAME}" curl -fsS http://127.0.0.1:9001/health
 echo
 
+echo "==> Asserting upgrade-from-symlink migration (ether/ep_comments_page#416)"
+# Stop etherpad, recreate the pre-fix symlink layout with a marker plugin,
+# re-run the postinst, and verify the migration restored the in-tree
+# directory and preserved the marker payload.
+if [ -z "${NO_SYSTEMD}" ]; then
+  docker exec "${CONTAINER_NAME}" systemctl stop etherpad
+fi
+docker exec "${CONTAINER_NAME}" bash -lc '
+  set -eux
+  rm -rf /opt/etherpad/src/plugin_packages
+  mkdir -p /var/lib/etherpad/plugin_packages/.versions/ep_migration_marker
+  echo "{\"name\":\"ep_migration_marker\"}" \
+    > /var/lib/etherpad/plugin_packages/.versions/ep_migration_marker/package.json
+  chown -R etherpad:etherpad /var/lib/etherpad/plugin_packages
+  ln -sfn /var/lib/etherpad/plugin_packages /opt/etherpad/src/plugin_packages
+  dpkg-reconfigure etherpad
+  test -d /opt/etherpad/src/plugin_packages
+  test ! -L /opt/etherpad/src/plugin_packages
+  test -f /opt/etherpad/src/plugin_packages/.versions/ep_migration_marker/package.json
+  [ "$(stat -c %a /opt/etherpad/src/plugin_packages)" = "2775" ]
+'
+
+echo "==> Staging ep_layout_trip_wire fixture and verifying it loads (ether/ep_comments_page#416)"
+# Copy the fixture into the container under .versions/, wire up the
+# symlinks live-plugin-manager would normally create, and list it in
+# installed_plugins.json so etherpad picks it up on start. The fixture
+# loads several ep_etherpad-lite/* modules at require-time, which was
+# the exact failure mode in #416.
+docker cp "${REPO_ROOT}/packaging/test-fixtures/ep_layout_trip_wire" \
+  "${CONTAINER_NAME}:/tmp/ep_layout_trip_wire"
+docker exec "${CONTAINER_NAME}" bash -lc '
+  set -eux
+  install -d -o etherpad -g etherpad -m 2775 /opt/etherpad/src/plugin_packages/.versions
+  rm -rf /opt/etherpad/src/plugin_packages/.versions/ep_layout_trip_wire@1.0.0
+  mv /tmp/ep_layout_trip_wire \
+    /opt/etherpad/src/plugin_packages/.versions/ep_layout_trip_wire@1.0.0
+  ln -sfn .versions/ep_layout_trip_wire@1.0.0 \
+    /opt/etherpad/src/plugin_packages/ep_layout_trip_wire
+  ln -sfn ../plugin_packages/ep_layout_trip_wire \
+    /opt/etherpad/src/node_modules/ep_layout_trip_wire
+  chown -R etherpad:etherpad \
+    /opt/etherpad/src/plugin_packages/.versions/ep_layout_trip_wire@1.0.0 \
+    /opt/etherpad/src/plugin_packages/ep_layout_trip_wire \
+    /opt/etherpad/src/node_modules/ep_layout_trip_wire
+  echo "{\"plugins\":[{\"name\":\"ep_etherpad-lite\",\"version\":\"0.0.0\"},{\"name\":\"ep_layout_trip_wire\",\"version\":\"1.0.0\"}]}" \
+    > /var/lib/etherpad/var/installed_plugins.json
+  chown etherpad:etherpad /var/lib/etherpad/var/installed_plugins.json
+'
+
+if [ -z "${NO_SYSTEMD}" ]; then
+  docker exec "${CONTAINER_NAME}" systemctl start etherpad
+else
+  docker exec -d "${CONTAINER_NAME}" runuser -u etherpad -- \
+    bash -c "cd /opt/etherpad && NODE_ENV=production /usr/bin/etherpad >/tmp/etherpad.log 2>&1"
+fi
+
+echo "==> Waiting for /health (after fixture restart)"
+ok=
+for i in $(seq 1 30); do
+  if docker exec "${CONTAINER_NAME}" curl -fsS http://127.0.0.1:9001/health >/dev/null 2>&1; then
+    ok=1; break
+  fi
+  sleep 2
+done
+[ -n "${ok}" ] || { echo "!! /health never came back after staging fixture"; \
+  if [ -z "${NO_SYSTEMD}" ]; then \
+    docker exec "${CONTAINER_NAME}" journalctl -u etherpad --no-pager -n 300; \
+  else \
+    docker exec "${CONTAINER_NAME}" tail -n 300 /tmp/etherpad.log; \
+  fi; exit 1; }
+
+if [ -z "${NO_SYSTEMD}" ]; then
+  docker exec "${CONTAINER_NAME}" bash -lc '
+    journalctl -u etherpad --no-pager -n 500 \
+      | grep -F "ep_layout_trip_wire: plugin_packages layout OK"
+    if journalctl -u etherpad --no-pager -n 500 \
+        | grep -E "Cannot find module .ep_etherpad-lite"; then
+      echo "!! ep_etherpad-lite require failed inside installed plugin" >&2
+      exit 1
+    fi
+  '
+else
+  docker exec "${CONTAINER_NAME}" bash -lc '
+    grep -F "ep_layout_trip_wire: plugin_packages layout OK" /tmp/etherpad.log
+    if grep -E "Cannot find module .ep_etherpad-lite" /tmp/etherpad.log; then
+      echo "!! ep_etherpad-lite require failed inside installed plugin" >&2
+      exit 1
+    fi
+  '
+fi
+echo "==> Trip-wire fixture loaded cleanly"
+
 if [ "${MODE}" = "shell" ]; then
   echo
   echo "Container left running as '${CONTAINER_NAME}'. Useful commands:"
@@ -199,5 +295,9 @@ else
 fi
 docker exec "${CONTAINER_NAME}" dpkg --purge etherpad
 docker exec "${CONTAINER_NAME}" bash -c '! id etherpad 2>/dev/null'
+# Purge must wipe runtime-created plugin artifacts that dpkg didn't ship
+# (ether/ep_comments_page#416, Qodo #3).
+docker exec "${CONTAINER_NAME}" bash -c '! [ -e /opt/etherpad/src/plugin_packages ]'
+docker exec "${CONTAINER_NAME}" bash -c '! [ -e /var/lib/etherpad ]'
 
 echo "==> All checks passed."