feat(packaging): publish Etherpad as a Snap

[JohnMcLear]

Summary

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

Part of #7529 — top-3 deployment targets (Snap, Apt, Home Assistant).

• snap/snapcraft.yaml — core24, strict confinement, pnpm + pinned Node.js 22. Version auto-derived from src/package.json. The daemon app shares the snap name, so users get a bare etherpad command; the bin/ passthrough is exposed as etherpad.cli.
• snap/local/bin/etherpad-service — launch wrapper; seeds settings.json into $SNAP_COMMON on first run, rewrites the shipped dirty default to sqlite at $SNAP_COMMON/var/etherpad.db, env-substitutes ip/port so snap set actually takes effect, and execs Node from ${APP_DIR}/src with --import tsx --settings <seeded> (Etherpad's loader reads argv.settings, not EP_SETTINGS; tsx lives in the src workspace's node_modules under pnpm hoisting).
• snap/local/bin/etherpad-healthcheck-wrapper — HTTP /health probe for external supervisors.
• snap/local/bin/etherpad-cli — passthrough to bin/ scripts (importSqlFile, checkPad, …). Rejects path-traversal and unsupported extensions.
• snap/hooks/configure — exposes snap set etherpad port=<n> / ip=<addr> with validation.
• snap/README.md — full architecture, testing, dev workflow, publishing, and troubleshooting docs.
• snap/tests/ — wrapper unit tests + smoke harness (see "Test plan" below).
• .github/workflows/snap-build.yml — runs unit tests + snapcraft pack --destructive-mode on every PR touching snap/. Uploads the built .snap as an artifact.
• .github/workflows/snap-publish.yml — tag-triggered build → edge → gated stable via GitHub Environment approval. Tag filter uses GitHub Actions glob syntax (vX.Y.Z and X.Y.Z).

Default DB is sqlite, not dirty

settings.json.template ships with dbType: "dirty", and the template itself warns "You shouldn't use 'dirty' for anything else than testing". A Snap install is exactly the "not testing" case, so the launch wrapper's first-run sed switches the seeded config to sqlite ($SNAP_COMMON/var/etherpad.db). sqlite is already in-tree via ueberdb2 → rusty-store-kv (prebuilt napi-rs binary), so strict confinement works with zero snap.yaml changes. Existing seeded settings.json files are never touched on refresh.

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

var/installed_plugins.json writability

Etherpad's plugin installer (src/static/js/pluginfw/installer.ts) writes var/installed_plugins.json at runtime via __dirname-relative paths, which resolve to absolute paths inside the read-only snap squashfs and raise EROFS. Snap layouts can't intercept paths inside $SNAP, so the build replaces the shipped var/ directory with a symlink pointing to /var/snap/etherpad/common/etherpad-app-var/. The wrapper mkdir -ps the target on first run; the kernel transparently follows the symlink to writable storage that survives snap refresh.

Test plan — verified locally on Ubuntu 24.04

Wrapper unit tests (bash snap/tests/run-all.sh, ~5 s, no snapd/sudo): 47/47 passing

• 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 (../, subdir, empty), .ts/.sh dispatch, default-case rejection, no-args usage
• test-configure.sh — port (1–65535 integer) 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

Build (snapcraft pack --destructive-mode): succeeds in ~3 min, produces etherpad_2.6.1_amd64.snap. CI workflow snap-build.yml reproduces this on every PR.

End-to-end smoke (bash snap/tests/smoke.sh):

• ✅ sudo snap install --dangerous etherpad_2.6.1_amd64.snap installs cleanly
• ✅ sudo snap set etherpad port=9003 && sudo snap restart etherpad relocates the listener (verified via ss -tlnp | grep :9003; production default 9001 is preserved when no override is set)
• ✅ curl http://127.0.0.1:9003/health → HTTP 200 {"status":"pass","releaseId":"2.6.1"}
• ✅ sudo snap services etherpad → enabled / active
• ✅ grep dbType /var/snap/etherpad/common/etc/settings.json → "sqlite" (wrapper sed rewrite confirmed)
• ✅ grep ip /var/snap/etherpad/common/etc/settings.json → "${IP:0.0.0.0}" (env-substitution confirmed)
• ✅ /var/snap/etherpad/common/var/etherpad.db created by the daemon
• ✅ /var/snap/etherpad/common/etherpad-app-var/installed_plugins.json created by Etherpad's plugin migration (proves the var/ symlink works under strict confinement)
• ✅ Daemon log: [INFO] http - HTTP server listening for connections and Etherpad is running
• ✅ snap refresh (simulated by reinstalling): /var/snap/etherpad/common/ preserved

The local rusty-store-kv musl native module loaded cleanly under strict confinement (no manual library-binding needed); the library linter warning about it is the standard prebuilt-binary noise, not a runtime problem.

Maintainer action required (one-time)

1. snapcraft register etherpad — claims the name.
2. Generate a store credential and store it as repo secret SNAPCRAFT_STORE_CREDENTIALS:

   snapcraft export-login --snaps etherpad \
     --channels edge,stable \
     --acls package_access,package_push,package_release -
   

3. Create a GitHub Environment snap-store-stable with required reviewers so stable promotion is gated.

See Register a snap and snapcraft export-login.

Refs #7529

🤖 Generated with Claude Code

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

Review Summary by Qodo

(Agentic_describe updated until commit b86e80a)

Add Snap packaging with wrappers, tests, and publishing workflows

✨ Enhancement

Walkthroughs

Description

• Adds comprehensive Snap packaging for Ubuntu/snapd users
• Implements launch wrapper with first-run bootstrap and settings rewriting
• Includes CLI passthrough, healthcheck, and configuration hook
• Provides extensive test suite and CI/CD workflows for building and publishing

Diagram

flowchart LR
  A["snapcraft.yaml<br/>core24 strict"] --> B["etherpad-service<br/>launch wrapper"]
  A --> C["etherpad-cli<br/>bin passthrough"]
  A --> D["etherpad-healthcheck<br/>HTTP probe"]
  B --> E["settings.json<br/>seeded + rewritten"]
  E --> F["sqlite DB<br/>$SNAP_COMMON/var"]
  G["configure hook<br/>port/ip validation"] --> H["snap set<br/>etherpad"]
  I["snap-build.yml<br/>PR verification"] --> J["wrapper tests<br/>+ snap-pack"]
  K["snap-publish.yml<br/>tag-triggered"] --> L["edge channel<br/>+ stable gate"]

Loading

File Changes

1. snap/snapcraft.yaml ⚙️ Configuration changes +161/-0

Core Snap recipe with Node.js 22 and pnpm build

snap/snapcraft.yaml

---

2. snap/local/bin/etherpad-service ✨ Enhancement +71/-0

Launch wrapper with settings bootstrap and env substitution

snap/local/bin/etherpad-service

---

3. snap/local/bin/etherpad-cli ✨ Enhancement +35/-0

Passthrough to bin/ scripts with path-traversal protection

snap/local/bin/etherpad-cli

---

View more (12)

4. snap/local/bin/etherpad-healthcheck-wrapper ✨ Enhancement +20/-0

HTTP health probe with curl fallback to Node

snap/local/bin/etherpad-healthcheck-wrapper

---

5. snap/hooks/configure ✨ Enhancement +24/-0

Validates port/ip settings and restarts daemon

snap/hooks/configure

---

6. snap/README.md 📝 Documentation +264/-0

Complete architecture, testing, and publishing documentation

snap/README.md

---

7. snap/tests/lib.sh 🧪 Tests +73/-0

Shared test helpers with assertions and formatting

snap/tests/lib.sh

---

8. snap/tests/run-all.sh 🧪 Tests +47/-0

Test runner orchestrating all unit test suites

snap/tests/run-all.sh

---

9. snap/tests/test-snapcraft-yaml.sh 🧪 Tests +80/-0

Validates snapcraft.yaml structure and required keys

snap/tests/test-snapcraft-yaml.sh

---

10. snap/tests/test-cli.sh 🧪 Tests +77/-0

Unit tests for CLI path-traversal and script dispatch

snap/tests/test-cli.sh

---

11. snap/tests/test-configure.sh 🧪 Tests +62/-0

Unit tests for port/ip validation via mocked snapctl

snap/tests/test-configure.sh

---

12. snap/tests/test-service-bootstrap.sh 🧪 Tests +138/-0

Unit tests for settings seeding and env-var propagation

snap/tests/test-service-bootstrap.sh

---

13. snap/tests/smoke.sh 🧪 Tests +53/-0

End-to-end smoke test with install and health check

snap/tests/smoke.sh

---

14. .github/workflows/snap-build.yml ⚙️ Configuration changes +71/-0

PR verification workflow for wrapper tests and snap packing

.github/workflows/snap-build.yml

---

15. .github/workflows/snap-publish.yml ⚙️ Configuration changes +92/-0

Tag-triggered build and publish to edge/stable channels

.github/workflows/snap-publish.yml

---

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

Code Review by Qodo

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

1. CLI tsx invocation broken 🐞 Bug ≡ Correctness

Description

snap/local/bin/etherpad-cli runs .ts scripts with --import tsx/esm and does not cd into
${APP_DIR}/src, contradicting the snap’s own documentation and the daemon wrapper; this can cause
ERR_REQUIRE_CYCLE_MODULE and/or Cannot find package 'tsx' when users run etherpad.cli ....

Code

snap/local/bin/etherpad-cli[R6-33]

+APP_DIR="${SNAP}/opt/etherpad"
+NODE_BIN="${SNAP}/opt/node/bin/node"
+export PATH="${SNAP}/opt/node/bin:${PATH}"
+
+if [ "$#" -eq 0 ]; then
+  echo "Usage: etherpad.cli <bin-script> [args...]"
+  echo "Available scripts:"
+  ls "${APP_DIR}/bin" | grep -E '\.(ts|sh)$' | sed 's/^/  /'
+  exit 2
+fi
+
+SCRIPT_NAME="$1"; shift
+
+# Reject path-traversal attempts: only a bare filename is allowed, since
+# the script lookup is anchored at $APP_DIR/bin and must not escape it.
+case "${SCRIPT_NAME}" in
+  */*|*..*|"")
+    echo "invalid script name: ${SCRIPT_NAME} (must be a bare filename)" >&2
+    exit 2 ;;
+esac
+
+SCRIPT_PATH="${APP_DIR}/bin/${SCRIPT_NAME}"
+[ -f "${SCRIPT_PATH}" ] || { echo "no such script: ${SCRIPT_NAME}" >&2; exit 2; }
+
+case "${SCRIPT_PATH}" in
+  *.sh)  exec "${SCRIPT_PATH}" "$@" ;;
+  *.ts)  exec "${NODE_BIN}" --import tsx/esm "${SCRIPT_PATH}" "$@" ;;
+  *)     echo "unsupported script type: ${SCRIPT_NAME} (expected .sh or .ts)" >&2

Evidence

The CLI wrapper uses the ESM-only tsx loader (tsx/esm) and never changes working directory before
executing Node. The snap README explicitly warns that tsx/esm triggers ERR_REQUIRE_CYCLE_MODULE
and that node must be run from ${APP_DIR}/src so tsx can be resolved under the pnpm workspace
layout; the service wrapper already follows this guidance.

snap/local/bin/etherpad-cli[6-9]
snap/local/bin/etherpad-cli[30-34]
snap/README.md[241-247]
snap/local/bin/etherpad-service[59-70]
bin/cleanRun.sh[36-39]

Agent prompt

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

## Issue description
`snap/local/bin/etherpad-cli` is supposed to run Etherpad’s `bin/*.ts` scripts, but it currently:
1) uses `--import tsx/esm` (known to break Etherpad with `ERR_REQUIRE_CYCLE_MODULE`), and
2) does not `cd` to `${APP_DIR}/src`, which the snap docs say is required for `tsx` module resolution under pnpm hoisting.
This makes `etherpad.cli <ts-script>` unreliable or non-functional.
### Issue Context
The daemon wrapper (`etherpad-service`) already uses the correct approach: `cd "${APP_DIR}/src"` and `--import tsx`.
### Fix Focus Areas
- snap/local/bin/etherpad-cli[6-35]
- snap/tests/test-cli.sh[55-67]
### Concrete change
- Before executing node, `cd "${APP_DIR}/src"`.
- For `.ts` scripts, use: `exec "${NODE_BIN}" --import tsx "${SCRIPT_PATH}" "$@"`
- Update `snap/tests/test-cli.sh` expectations accordingly (it currently asserts `tsx/esm`).

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

---

2. Tag filter never matches ☑ 🐞 Bug ☼ Reliability

Description

In .github/workflows/snap-publish.yml the tag filter uses a regex-like pattern
(v?[0-9]+.[0-9]+.[0-9]+) but GitHub Actions uses glob matching, so + is treated literally and
typical release tags like v2.6.1 will not match. As a result the Snap publish workflow will not
trigger on release tags and nothing will be built/published automatically.

Code

.github/workflows/snap-publish.yml[R17-21]

+on:
+  push:
+    tags:
+      - 'v?[0-9]+.[0-9]+.[0-9]+'
+  workflow_dispatch:

Evidence

The new workflow’s tag filter includes + tokens, which are not glob operators and therefore will
be treated as literal plus characters in the tag name; this prevents expected semver tags from
triggering the workflow. The same pattern appears elsewhere in the repo (likely copy/paste),
reinforcing that this specific string is what will be used at runtime.

.github/workflows/snap-publish.yml[17-21]
.github/workflows/docker.yml[6-13]
Best Practice: GitHub Actions workflow syntax (ref filters are glob patterns)

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 workflow tag trigger pattern is written like a regex, but GitHub Actions uses glob matching for `on.push.tags`. The current pattern will not match normal semver tags like `v2.6.1`, so the workflow will not run on releases.
### Issue Context
The workflow intends to run on tags matching `v?X.Y.Z` (optional leading `v`).
### Fix Focus Areas
- .github/workflows/snap-publish.yml[17-21]
### Suggested change
Replace the single regex-like entry with glob patterns, for example:
- `v[0-9]*.[0-9]*.[0-9]*`
- `[0-9]*.[0-9]*.[0-9]*`
(or whichever variant matches your actual tagging scheme).

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

---

3. CLI path traversal exec ☑ 🐞 Bug ⛨ Security

Description

snap/local/bin/etherpad-cli builds SCRIPT_PATH from unvalidated user input, so a caller can pass
a value containing / or .. to escape the intended ${APP_DIR}/bin directory and execute
arbitrary .ts/.sh files shipped in the snap. Additionally, there is no default case branch, so
if a file exists but is not .ts or .sh the command silently does nothing and exits successfully.

Code

snap/local/bin/etherpad-cli[R17-24]

+SCRIPT_NAME="$1"; shift
+SCRIPT_PATH="${APP_DIR}/bin/${SCRIPT_NAME}"
+[ -f "${SCRIPT_PATH}" ] || { echo "no such script: ${SCRIPT_NAME}"; exit 2; }
+
+case "${SCRIPT_PATH}" in
+  *.sh)  exec "${SCRIPT_PATH}" "$@" ;;
+  *.ts)  exec "${NODE_BIN}" --import tsx/esm "${SCRIPT_PATH}" "$@" ;;
+esac

Evidence

The wrapper directly concatenates the user-provided script name into a filesystem path without
rejecting path separators, and then executes the file based solely on its extension. This enables
directory traversal out of bin/ and makes execution behavior dependent on attacker-controlled
paths.

snap/local/bin/etherpad-cli[17-24]

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 snap CLI wrapper allows path traversal via the `<bin-script>` argument and can execute unintended files. It also silently succeeds for unsupported extensions.
### Issue Context
This command is meant to be a thin, safe passthrough to scripts under `$SNAP/opt/etherpad-lite/bin`.
### Fix Focus Areas
- snap/local/bin/etherpad-cli[17-24]
### Suggested change
- Reject any `SCRIPT_NAME` that contains `/` or `..` (or normalize to `basename` and compare).
- Optionally enforce an allowlist derived from `$APP_DIR/bin`.
- Add a default `*)` case that prints an error like `unsupported script type` and exits non-zero.

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

---

View more (3)

4. 4-space indent in scripts ☑ 📘 Rule violation ⚙ Maintainability

Description

New Bash scripts use 4-space indentation inside control blocks, violating the repository rule
requiring exactly 2-space indentation. This reduces consistency and can cause style-check failures
if enforced.

Code

snap/hooks/configure[R9-13]

+if [ -n "${PORT}" ]; then
+    if ! [[ "${PORT}" =~ ^[0-9]+$ ]] || [ "${PORT}" -lt 1 ] || [ "${PORT}" -gt 65535 ]; then
+        echo "port must be an integer 1-65535" >&2
+        exit 1
+    fi

Evidence

PR Compliance ID 10 requires 2-space indentation and no tabs; the newly added scripts indent block
contents with 4 spaces (e.g., the if bodies).

snap/hooks/configure[9-13]
snap/local/bin/etherpad-cli[10-14]
snap/local/bin/etherpad-healthcheck-wrapper[8-11]
snap/local/bin/etherpad-service[23-30]
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 newly added Bash scripts use 4-space indentation inside `if`/`case` blocks, but the project compliance rule requires 2-space indentation (and no tabs).
## Issue Context
This PR adds several wrapper scripts under `snap/`. To stay compliant and consistent with repo formatting expectations, indentation should be normalized to 2 spaces.
## Fix Focus Areas
- snap/hooks/configure[9-13]
- snap/local/bin/etherpad-cli[10-14]
- snap/local/bin/etherpad-healthcheck-wrapper[8-11]
- snap/local/bin/etherpad-service[23-30]

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

---

5. Settings file ignored ☑ 🐞 Bug ≡ Correctness

Description

The snap seeds $SNAP_COMMON/etc/settings.json and exports EP_SETTINGS, but Etherpad’s settings
loader does not read EP_SETTINGS, so it will keep looking for <install-root>/settings.json and fall
back to defaults. Defaults include a DB file under <install-root>/var/rusty.db, which is inside the
read-only snap mount, so the daemon will fail to persist data and may fail to start.

Code

snap/local/bin/etherpad-service[R39-43]

+cd "${APP_DIR}"
+export EP_SETTINGS="${SETTINGS}"
+export NODE_ENV=production
+
+exec "${NODE_BIN}" --import tsx/esm src/node/server.ts "$@"

Evidence

The wrapper exports EP_SETTINGS and execs server.ts, but Etherpad only supports overriding the
settings file via the --settings/-s CLI arg (argv.settings), not an EP_SETTINGS environment
variable. When no settings file is found, Etherpad continues with defaults, including a default DB
filename under settings.root/var/rusty.db, which resolves inside the snap’s read-only install
root.

snap/local/bin/etherpad-service[22-43]
src/node/utils/Settings.ts[301-306]
src/node/utils/Cli.ts[25-44]
src/node/utils/Settings.ts[685-689]

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 snap wrapper exports `EP_SETTINGS` and seeds `$SNAP_COMMON/etc/settings.json`, but Etherpad ignores `EP_SETTINGS` and uses `argv.settings` (from `--settings/-s`) or defaults to `<install-root>/settings.json`. This prevents the snap from using the seeded writable settings and can force DB paths into the read-only snap mount.
### Issue Context
The snap already sets `EP_SETTINGS` (wrapper + snapcraft.yaml). The minimal, snap-friendly fix is to make Etherpad honor `process.env.EP_SETTINGS` (and optionally `process.env.EP_CREDENTIALS`) as a fallback when CLI flags are not provided.
### Fix Focus Areas
- src/node/utils/Settings.ts[301-306]
- src/node/utils/Cli.ts[25-44]
- snap/local/bin/etherpad-service[39-43]
### Suggested approach
- Update Settings filename resolution to prefer `argv.settings`, else `process.env.EP_SETTINGS`, else `'settings.json'`.
- Do the same for credentials (`argv.credentials` -> `process.env.EP_CREDENTIALS` -> `'credentials.json'`).
- Keep CLI flag precedence so existing behavior is unchanged.
- (Optional) Add a small unit/integration check or comment documenting the env var support.

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

---

6. snap set overrides no-op ☑ 🐞 Bug ≡ Correctness

Description

The service wrapper exports PORT/IP from snapctl, but the seeded settings.json uses literal ip/port
values, so the configured settings file will override the env-based defaults and ignore `snap set
port= / snap set ip=`. Users following the snap README will not see the listener move after
restart.

Code

snap/local/bin/etherpad-service[R32-37]

+PORT_OVERRIDE="$(snapctl get port || true)"
+IP_OVERRIDE="$(snapctl get ip || true)"
+: "${PORT_OVERRIDE:=9001}"
+: "${IP_OVERRIDE:=0.0.0.0}"
+export PORT="${PORT_OVERRIDE}"
+export IP="${IP_OVERRIDE}"

Evidence

The wrapper sets PORT and IP environment variables, but the bootstrapped settings.json is
copied from settings.json.template, where ip and port are hard-coded (not ${IP...} /
${PORT...} placeholders). Etherpad’s default port reads from process.env.PORT, but once a
settings file is loaded those explicit values take precedence, so snap-set overrides won’t apply.

snap/local/bin/etherpad-service[22-37]
settings.json.template[151-162]
src/node/utils/Settings.ts[341-349]

Agent prompt

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

## Issue description
`snap set etherpad-lite port=...` / `ip=...` is implemented by exporting `PORT`/`IP`, but the seeded settings file hard-codes `"ip": "0.0.0.0"` and `"port": 9001`, which overrides env defaults. As a result, snap config changes do nothing.
### Issue Context
Etherpad supports env-var substitution **inside** settings.json via strings like `"${PORT:9001}"`, but the current template copy does not use that syntax for `ip`/`port`.
### Fix Focus Areas
- snap/local/bin/etherpad-service[22-37]
- settings.json.template[151-162]
### Suggested approach
- During first-run bootstrap (right after copying the template), rewrite the `ip` and `port` entries to use Etherpad’s substitution syntax:
- `"ip": "${IP:0.0.0.0}"`
- `"port": "${PORT:9001}"`  (must be quoted per template rules)
- Only apply the rewrite if the file still contains the template’s default literal values, to avoid overwriting user customizations.
- Keep the existing dirty-db path rewrite.

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

---

7. Weak IP validation 🐞 Bug ☼ Reliability

Description

The configure hook’s ip validation only checks allowed characters ([0-9a-fA-F.:]+) but not
actual IPv4/IPv6 structure, so invalid values can pass validation and cause the daemon to fail to
bind and restart-loop.

Code

snap/hooks/configure[R16-20]

+IP="$(snapctl get ip || true)"
+if [ -n "${IP}" ] && ! [[ "${IP}" =~ ^[0-9a-fA-F.:]+$ ]]; then
+  echo "ip must be a valid IPv4/IPv6 address" >&2
+  exit 1
+fi

Evidence

The hook’s regex is a character whitelist rather than a real IP parser, yet the hook error message
claims it enforces a valid IPv4/IPv6 address. This allows malformed addresses to be accepted by
snap set etherpad ip=....

snap/hooks/configure[16-20]
snap/README.md[42-44]

Agent prompt

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

## Issue description
`snap/hooks/configure` claims to validate IPv4/IPv6 addresses, but the current regex only ensures the value contains hex digits/dots/colons. Many invalid addresses will pass and later cause bind failures.
### Issue Context
This is a validation hook, so it should reject malformed values early to avoid service flapping.
### Fix Focus Areas
- snap/hooks/configure[16-20]
- snap/tests/test-configure.sh[49-61]
### Concrete change
Use a real parser instead of a character whitelist, for example:
- `python3 -c 'import ipaddress,sys; ipaddress.ip_address(sys.argv[1])' "$IP"`
and treat non-zero exit as invalid.
Add a couple of negative test cases that currently pass but shouldn’t (e.g. `::::`, `....`).

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

---

8. Wrong service active check 🐞 Bug ☼ Reliability

Description

The configure hook uses grep -q active on snapctl services output, which also matches the
substring in inactive, so it can attempt a restart even when the service is inactive/disabled.
This can cause snap set to fail if the restart command errors under those states.

Code

snap/hooks/configure[R22-24]

+if snapctl services etherpad-lite.etherpad-lite 2>/dev/null | grep -q active; then
+    snapctl restart etherpad-lite.etherpad-lite
+fi

Evidence

The hook’s status check is substring-based (active), which is contained within inactive, so the
conditional can evaluate true for inactive services and run an unintended restart.

snap/hooks/configure[22-24]

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 configure hook checks service status with `grep -q active`, which matches both `active` and `inactive`, potentially triggering restarts at the wrong time.
### Issue Context
This runs during `snap set ...` and can make configuration fail if an unnecessary restart errors.
### Fix Focus Areas
- snap/hooks/configure[22-24]
### Suggested approach
- Replace `grep -q active` with a whole-word match (e.g., `grep -qw active`), or parse the status column explicitly.
- Ensure the condition only triggers when the service is actually active.

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

---

9. Node download not verified 🐞 Bug ⛨ Security

Description

snapcraft.yaml downloads and extracts a Node.js tarball without verifying its checksum or signature.
A compromised download would be baked into the built snap and distributed to users.

Code

snap/snapcraft.yaml[R100-105]

+      NODE_TGZ="node-v${NODE_VERSION}-linux-${NODE_ARCH}.tar.xz"
+      curl -fsSL "https://nodejs.org/dist/v${NODE_VERSION}/${NODE_TGZ}" \
+        -o "/tmp/${NODE_TGZ}"
+      mkdir -p "${CRAFT_PART_INSTALL}/opt/node"
+      tar -xJf "/tmp/${NODE_TGZ}" -C "${CRAFT_PART_INSTALL}/opt/node" \
+        --strip-components=1

Evidence

The build recipe fetches a Node.js tarball via curl and extracts it directly, with no integrity
check step (checksum or signature verification) before use.

snap/snapcraft.yaml[92-105]
Best Practice: SLSA / supply-chain integrity best practices

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 snap build downloads Node.js and extracts it without verifying integrity, creating a supply-chain risk.
### Issue Context
Even with HTTPS, checksum/signature verification is recommended for externally fetched build artifacts that become part of a shipped package.
### Fix Focus Areas
- snap/snapcraft.yaml[92-105]
### Suggested approach
- Download Node’s `SHASUMS256.txt` for the pinned version and verify the tarball checksum before extracting.
- (Optional, stronger) Verify the signed SHASUMS file using Node release keys.
- Fail the build if verification fails.

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

---

10. Healthcheck trusts PORT blindly 🐞 Bug ☼ Reliability

Description

The healthcheck wrapper interpolates PORT from snapctl get directly into the URL/Node snippet
without locally validating it, creating a fragile cross-file dependency on the configure hook and
increasing the chance of surprising failures if validation changes.

Code

snap/local/bin/etherpad-healthcheck-wrapper[R5-17]

+PORT="$(snapctl get port 2>/dev/null || true)"
+: "${PORT:=9001}"
+
+if command -v curl >/dev/null 2>&1; then
+  exec curl --fail --silent --show-error --max-time 5 \
+    "http://127.0.0.1:${PORT}/health"
+fi
+
+NODE_BIN="${SNAP}/opt/node/bin/node"
+exec "${NODE_BIN}" -e '
+  const http = require("http");
+  http.get("http://127.0.0.1:'"${PORT}"'/health", r => {
+    if (r.statusCode === 200) process.exit(0);

Evidence

The healthcheck reads PORT and uses it to construct the health URL. The configure hook currently
enforces numeric port values, but the healthcheck wrapper itself has no guardrails, so it relies on
external validation always being present and correct.

snap/local/bin/etherpad-healthcheck-wrapper[5-20]
snap/hooks/configure[8-14]

Agent prompt

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

## Issue description
`etherpad-healthcheck-wrapper` uses the snap-configured port without validating it locally. Today this works because `snap/hooks/configure` validates `port`, but this implicit coupling is fragile.
### Issue Context
Defense-in-depth: the healthcheck should be robust even if config validation changes.
### Fix Focus Areas
- snap/local/bin/etherpad-healthcheck-wrapper[5-17]
### Concrete change
Add a simple numeric + range check (or fallback to 9001) before building the URL, e.g.:
- `if ! [[ "$PORT" =~ ^[0-9]+$ ]] || [ "$PORT" -lt 1 ] || [ "$PORT" -gt 65535 ]; then PORT=9001; fi`

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

---

11. Smoke script hardcodes paths 🐞 Bug ⚙ Maintainability

Description

snap/tests/smoke.sh says it should be run from the worktree root, but it hardcodes an absolute
WORKTREE and SNAP_FILE, so it will fail for other developers without editing the script.

Code

snap/tests/smoke.sh[R4-13]

+# Run from the worktree root: bash snap/tests/smoke.sh
+set -uo pipefail
+
+WORKTREE="/home/jose/etherpad/etherpad-lite/.claude/worktrees/pkg-snap"
+SNAP_FILE="${WORKTREE}/etherpad_2.6.1_amd64.snap"
+TEST_PORT=9003
+BUILD_LOG=/tmp/snapcraft-build.log
+
+cd "${WORKTREE}" || exit 1
+

Evidence

The script’s header comment documents running it from the repo root, but the implementation
immediately cds to a developer-specific absolute path and expects a fixed snap filename, which
makes it non-portable within the repository.

snap/tests/smoke.sh[2-13]

Agent prompt

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

## Issue description
`snap/tests/smoke.sh` is checked into the repo but is not runnable by others because it hardcodes a local WORKTREE and expected snap filename.
### Issue Context
Even if it’s intended for local-only use, keeping it portable reduces contributor friction.
### Fix Focus Areas
- snap/tests/smoke.sh[4-23]
### Concrete change
- Set `WORKTREE` from `git rev-parse --show-toplevel` (or `pwd` with a sanity check).
- After packing, discover the produced snap via `ls -t etherpad_*.snap | head -1` rather than hardcoding `etherpad_2.6.1_amd64.snap`.

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

---

12. Artifact path not robust 🐞 Bug ☼ Reliability

Description

The publish jobs pass needs.build.outputs.snap-file into action-publish, but they don’t derive the
snap filename from the downloaded artifact, so publishing can break if the build output contains a
path or if the artifact is extracted under a different directory. Making the publish step locate
*.snap after download avoids coupling to action-build’s output format.

Code

.github/workflows/snap-publish.yml[R27-66]

+  build:
+    runs-on: ubuntu-latest
+    outputs:
+      snap-file: ${{ steps.build.outputs.snap }}
+    steps:
+      - name: Check out
+        uses: actions/checkout@v6
+
+      - name: Build snap
+        id: build
+        uses: snapcore/action-build@v1
+
+      - name: Upload snap artifact
+        uses: actions/upload-artifact@v4
+        with:
+          name: etherpad-lite-snap
+          path: ${{ steps.build.outputs.snap }}
+          if-no-files-found: error
+          retention-days: 7
+
+  publish-edge:
+    needs: build
+    if: github.event_name == 'push'
+    runs-on: ubuntu-latest
+    permissions:
+      contents: read
+    steps:
+      - name: Download snap artifact
+        uses: actions/download-artifact@v4
+        with:
+          name: etherpad-lite-snap
+
+      - name: Publish to edge
+        uses: snapcore/action-publish@v1
+        env:
+          SNAPCRAFT_STORE_CREDENTIALS: ${{ secrets.SNAPCRAFT_STORE_CREDENTIALS }}
+        with:
+          snap: ${{ needs.build.outputs.snap-file }}
+          release: edge
+

Evidence

The workflow uploads an artifact, then in later jobs downloads it, but still uses the build job’s
recorded output to decide what file to publish rather than the downloaded file’s actual path/name in
the publish job workspace.

.github/workflows/snap-publish.yml[27-66]

Agent prompt

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

## Issue description
Publishing relies on `${{ needs.build.outputs.snap-file }}` instead of determining the snap file path from the downloaded artifact. This can fail if the output includes a non-portable path or if the artifact extracts into a subdirectory.
### Issue Context
`actions/download-artifact` materializes files into the publish job filesystem; that filesystem should be the source of truth for the snap path.
### Fix Focus Areas
- .github/workflows/snap-publish.yml[27-66]
### Suggested approach
- Set an explicit download path (e.g., `path: snap-out/`).
- Pass a stable glob/path to action-publish, e.g. `snap: snap-out/*.snap`, or add a step that resolves the single `.snap` filename and exports it to an env var used by both publish jobs.

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

---

[JohnMcLear]

Qodo's Snap set overrides no-op concern is already addressed on current HEAD. snap/local/bin/etherpad-service rewrites the seeded settings.json to use Etherpad's env-substitution syntax ("ip": "${IP:0.0.0.0}", "port": "${PORT:9001}") on first-run bootstrap, so snap set etherpad-lite port=... now takes effect. See the sed -i block at lines 37-40.

[JohnMcLear]

/review

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

Persistent review updated to latest commit 0db0063

[JohnMcLear]

still working on testing this one

[qodo-code-review]

ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one.

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

Persistent review updated to latest commit b86e80a

[JohnMcLear]

Pushed: rebased on latest develop (single .gitignore conflict resolved by keeping both deb-packaging and snap entries), description updated with full local test results, 47/47 wrapper unit tests still passing.

@CodiumAI-Agent /review

(reposting for an updated Qodo pass on commits a4e16b1..4e34d8a now that the rename, build fixes, runtime fixes, tests, CI, and docs have all landed)

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

1. Cli tsx invocation broken 🐞 Bug ≡ Correctness

snap/local/bin/etherpad-cli runs .ts scripts with --import tsx/esm and does not cd into
${APP_DIR}/src, contradicting the snap’s own documentation and the daemon wrapper; this can cause
ERR_REQUIRE_CYCLE_MODULE and/or Cannot find package 'tsx' when users run etherpad.cli ....

Agent Prompt

### Issue description
`snap/local/bin/etherpad-cli` is supposed to run Etherpad’s `bin/*.ts` scripts, but it currently:
1) uses `--import tsx/esm` (known to break Etherpad with `ERR_REQUIRE_CYCLE_MODULE`), and
2) does not `cd` to `${APP_DIR}/src`, which the snap docs say is required for `tsx` module resolution under pnpm hoisting.

This makes `etherpad.cli <ts-script>` unreliable or non-functional.

### Issue Context
The daemon wrapper (`etherpad-service`) already uses the correct approach: `cd "${APP_DIR}/src"` and `--import tsx`.

### Fix Focus Areas
- snap/local/bin/etherpad-cli[6-35]
- snap/tests/test-cli.sh[55-67]

### Concrete change
- Before executing node, `cd "${APP_DIR}/src"`.
- For `.ts` scripts, use: `exec "${NODE_BIN}" --import tsx "${SCRIPT_PATH}" "$@"`
- Update `snap/tests/test-cli.sh` expectations accordingly (it currently asserts `tsx/esm`).

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