Mirrors/etherpad-lite
1
0
Fork 0
mirror of https://github.com/ether/etherpad-lite.git synced 2026-07-21 00:59:11 +00:00
Code Issues Projects Releases Wiki Activity
9768 commits 252 branches 149 tags 113 MiB
Commit graph

2059 commits

Author SHA1 Message Date
John McLear
986f139a61
feat(metrics): 3 Prometheus counters for scaling dive (#7756) (#7762) 
* feat(metrics): expose 3 Prometheus counters for the scaling dive

Per the spec section 6 of #7756: enables the load-test harness to
attribute *where* time goes on the server, not just the gauge headline
(CPU / event-loop / memory) the dive doc starts from.

New /stats/prometheus rows:

- etherpad_pad_users{padId} — gauge, derived from sessioninfos on
  each scrape. Lets the harness confirm the pad it points at actually
  has the expected concurrency.

- etherpad_changeset_apply_duration_seconds — histogram observed
  inside handleUserChanges. Separates "apply path is slow" from
  "fan-out is slow" when latency rises.

- etherpad_socket_emits_total{type} — counter at the broadcast
  emit sites (handleCustomObjectMessage, handleCustomMessage,
  sendChatMessageToPadClients) and inside the NEW_CHANGES per-socket
  loop in updatePadClients. Bucketed by message type so the harness
  can measure the amplification factor of each lever (especially the
  fan-out batching lever).

Metric handles live in a new prom-instruments.ts module rather than
in prometheus.ts itself, so PadMessageHandler can import the
recording helpers without creating a circular dependency
(prometheus.ts already requires PadMessageHandler).

Tests: smoke test verifies recordSocketEmit + recordChangesetApply
move the underlying counters/histogram.

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

* fix(metrics): address Qodo review — flag-gate, scope histogram, bound label cardinality

Three issues raised on the initial PR:

1. **Feature flag.** Per project compliance rule, new features must
   be behind a flag and disabled by default. Adds
   `settings.scalingDiveMetrics` (default `false`). When off,
   recordSocketEmit() / recordChangesetApply() short-circuit to
   no-ops and the metrics are never even registered with the
   Prometheus register. Enable only when running the
   ether/etherpad-load-test scaling-dive harness.

2. **Histogram scope.** Previously the
   etherpad_changeset_apply_duration_seconds timer wrapped the
   whole handleUserChanges() body — including
   `await exports.updatePadClients(pad)` — so the histogram
   measured apply+fan-out, defeating its stated purpose. Now
   stopped immediately after the apply work (`assert.equal(...rev,
   r)`), before the ACCEPT_COMMIT socket emit and the
   updatePadClients call. Failed applies deliberately don't observe
   so the success-path distribution stays clean.

3. **Label cardinality.** handleCustomMessage was passing the
   user-supplied msgString (an HTTP-API param) directly as the
   `type` label value. A misbehaving API caller could grow
   prom-client's internal label map until OOM. Now bucketed against
   a known-types allowlist; anything outside it lands in `other`.

Tests updated: 5/5 — covers happy path, "other" bucketing of
unknown/unsafe labels, and that the flag-disabled state is a true
no-op.

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-15 19:54:42 +01:00
John McLear
21e1ae2fa3
security: allow integrator sessionID cookie to be HttpOnly (#7045) (#7755) 
* security: allow integrator sessionID cookie to be HttpOnly (#7045)

The integrator-set sessionID cookie was forced to be non-HttpOnly because
Etherpad's own client JS read it via document.cookie and forwarded it in
the socket.io CLIENT_READY payload, exposing it to XSS.

Mirror the GDPR PR3 author-token migration: read sessionID from the
socket.io handshake's Cookie header in PadMessageHandler.handleClientReady,
falling back to the legacy message-level field with a one-time deprecation
warning per socket. Drop the client-side Cookies.get('sessionID') reads in
pad.ts and timeslider.ts so the field is no longer sent by current clients.

Existing integrators that set sessionID without HttpOnly keep working
unchanged; the field on the message becomes optional and integrators
should now mark the cookie HttpOnly; Secure; SameSite=Lax.

Closes #7045

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

* fix(security): treat undecodable handshake cookies as absent (Qodo #7755)

decodeURIComponent() throws URIError on malformed values like `%ZZ`. The
unguarded call in PadMessageHandler.handleClientReady's readCookie() let
a single bad cookie abort CLIENT_READY for that socket, allowing
unauthenticated peers to spam server error logs and lock themselves out
of pads.

Catch URIError and treat the value as absent so the legacy message-level
field still serves as a fallback. Other error classes still propagate.
Add a backend test that asserts a `sessionID=%ZZ` cookie no longer
aborts the handshake.

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-15 19:44:55 +01:00
John McLear
ab60ed33c1
admin: parsed JSONC settings editor (takes over #7666, closes #7603) (#7709) 
* admin: parsed JSONC settings editor with form view (#7603, #7666)

Takes over #7666 / closes #7603. Squashed rebase of 32 commits onto
current develop (which has since absorbed admin design rework #7716
and admin i18n fixes #7736 — granular history preserved on
takeover/7666-admin-settings-editor before this squash, see PR
description for the original commit log).

Highlights:

- New parsed JSONC settings editor under
  admin/src/components/settings/ — FormView, ModeToggle,
  ParseErrorBanner, JsoncNode dispatcher, leaf widgets (string,
  number, bool, null, env pill), and pure helpers (comments,
  envPill, jsoncEdit, labels, templateComments).
- ${VAR:default} env placeholders render as editable inline inputs
  that round-trip through the raw textarea (env-pill spec asserts
  this; docker-template spec protects against form-view degradation
  on env-heavy configs).
- Schema-driven help text sourced from settings.json.template,
  inlined at build time via vite (drops the runtime fs.allow
  widening that earlier iterations needed).
- ModeToggle switches between FormView and raw textarea on
  /admin/settings; parse errors surface in a non-blocking banner.
- jsonc-parser dep added; pure helpers wrap modify() for stable
  edits that preserve key order and trailing comments (stops at
  end-of-line so trailing-comment trains don't bleed into the next
  property).
- i18n keys added for form mode, parse error, env pill,
  default_label, and input aria.
- Playwright specs cover form view, env pill, parse error banner,
  raw round-trip, and form-mode regressions called out in #7666
  review (stable React keys from AST offsets, save-toast on server
  ack only, NumberInput draft sync, parse-error flash during
  initial load, .settings CSS conflict resolution, focus retention
  via rAF, IconButton type defaulting to 'button').

Co-authored-by: Ayushi Gupta <ayushigupta36881@gmail.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(admin): stabilise React keys to prevent focus loss in settings editor

Switch React keys in JsoncNode and FormView from byte offsets to stable
JSON paths (`getNodePath(...).join('.')`). Byte offsets shift on every
keystroke because the edit changes the surrounding character count,
which forces React to remount inputs and lose focus mid-typing.

- Object children key on the property path.
- Array elements key on their JSON path index.
- Add a Playwright regression test pinning focus stability for array
  element edits.

Co-authored-by: John McLear <john@mclear.co.uk>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(admin): stop trailing /* */ comments from bleeding into next key's label (#7740)

In the parsed settings form view, each key's row was rendering its label
as the previous keys' source lines concatenated together. Root cause:
findLeading() in admin/src/components/settings/comments.ts treated any
line ending in `*/` as a comment continuation, so a JSON line like
  "altF9": true, /* focus on the File Menu and/or editbar */
was absorbed into the next sibling's leading comment block, and then
each subsequent key picked up an even longer accumulation.

- Tighten findLeading's isComment check to only match structural comment
  lines (`//`, `/*`, or a `*`-prefixed continuation/close), so JSON code
  with a trailing block comment no longer matches.
- Surface leading and trailing comments separately from the template
  map. Leaf rows with only a trailing same-line comment now render the
  humanized key as the row label and the comment as the help text below
  the control, matching settings.json.template's convention (and #7740's
  recommendation that "helper text should be below").
- Add unit tests pinning the regression and the JSDoc/`//` leading
  styles, plus a Playwright spec that asserts altC's row carries a
  clean label and the "focus on the Chat window" help text.

Closes #7740.

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

---------

Co-authored-by: Ayushi Gupta <ayushigupta36881@gmail.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
 2026-05-15 10:41:37 +01:00
John McLear
6075708818
feat(gdpr): admin UI for author erasure (follow-up to #7550) (#7667) 
* docs(gdpr): admin UI for author erasure — design spec

Follow-up to PR5 (#7550): adds an in-product /admin/authors page so
operators can search by name or external mapper, preview the impact
of an Art. 17 erasure (token mappings, mapper bindings, chat
messages, affected pads), and commit it without crafting a curl.
Backend uses three new admin-socket events on settings_admin (not
REST), so the existing public REST endpoint and its
gdprAuthorErasure.enabled flag keep their current single meaning.
The page stays discoverable when the flag is off — banner + disabled
buttons explain how to enable it.

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

* docs(gdpr): admin UI for author erasure — implementation plan

Step-by-step TDD plan for the /admin/authors follow-up to PR5
(#7550). Nine tasks covering: lastSeen field on globalAuthor writes,
anonymizeAuthor({dryRun}), authorManager.searchAuthors helper,
three new admin-socket events (authorLoad / anonymizeAuthorPreview /
anonymizeAuthor) + settings-flag delivery, frontend types/swatch/
i18n, store/route/sidebar wiring, AuthorPage.tsx with two-step
modal and disabled-flag banner, Playwright coverage, and the PR/
Qodo workflow. References the spec at
docs/superpowers/specs/2026-05-03-gdpr-admin-author-erasure-ui-design.md.

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

* feat(authors): stamp lastSeen on globalAuthor writes

Adds a lastSeen timestamp to the globalAuthor record on createAuthor,
setAuthorName, and setAuthorColorId. Read paths are not modified to
keep the write cost zero per page load. Pre-existing records gain the
field on their next identity write — no migration sweep, callers that
read the field tolerate undefined.

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

* feat(authors): anonymizeAuthor({dryRun}) for preview

Adds an opt-in dryRun option that walks the same token/mapper/chat
loops and returns identical counter shape without touching the
database. The public REST endpoint is unchanged (it never passes the
flag), so production behaviour is identical. Used by the upcoming
admin-UI two-step erase modal to show 'will clear: N mappings, K
chat messages' before the irreversible commit.

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

* docs(authors): restore lost rationale on anonymizeAuthor + document dryRun

Code review on the previous commit caught that the dryRun refactor
silently dropped four WHY-comments (lazy-require cycle, drop-mappings-
first ordering, zero-identity-without-sentinel split, sentinel-last
discipline) and left the new opts parameter undocumented. Restored
the comments verbatim and added a one-line JSDoc note for dryRun.

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

* feat(authors): authorManager.searchAuthors helper

In-memory enumeration of globalAuthor:* with a join on mapper2author:*
for the mapper column. Filter (substring on name OR mapper OR
authorID — the authorID match lets admins verify a specific erased
record where name and mapper bindings are gone), sort
(name | lastSeen), paginate, cap the pre-pagination set at 1000 to
prevent runaway scans. Powers the upcoming /admin/authors page.

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

* fix(authors): tie-break searchAuthors sort on authorID

Code review pointed out that ties in the primary sort key (common on
lastSeen for authors created the same ms, possible on identical
names too) fell back to findKeys enumeration order — not guaranteed
stable across DB backends. Adding an authorID secondary sort makes
pagination safe across requests. Also fix a misleading 'default'
note in the JSDoc — includeErased is required, not optional.

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

* feat(authors): admin-socket events for author erasure UI

Adds three handlers on the /settings admin namespace:
- authorLoad: paginated search via authorManager.searchAuthors
- anonymizeAuthorPreview: dry-run counters, always available to
  authenticated admins (read-only)
- anonymizeAuthor: live commit, gated on gdprAuthorErasure.enabled
  (returns {error: 'disabled'} when off)

Extends the load reply with a flags.gdprAuthorErasure boolean so the
client knows whether to render the disabled-flag banner without an
extra round-trip.

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

* test(authors): clean up admin-socket test mutations

Code review found two test-isolation issues:
- settings.users 'restoration' was a no-op (savedUsers held a
  reference to the same object that the test mutated). The test-admin
  key now gets explicitly removed in after().
- The Promise that awaited connect/connect_error left the loser
  listener attached for the lifetime of the socket, risking an
  unhandled rejection on a later spurious connect_error event.
  Listeners are now paired with off() so only the winner survives.

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

* feat(admin): types, ColorSwatch, and en.json for authors page

Standalone primitives for the upcoming /admin/authors page:
- AuthorSearch.ts: query/result/preview wire types matching the new
  admin-socket events
- ColorSwatch.tsx: resolves a globalAuthor.colorId (palette index or
  raw hex) to a small inline-styled swatch
- ep_admin_authors/en.json: every user-visible string the page needs,
  loaded by the existing namespace-as-static-asset i18n strategy

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

* feat(admin): /admin/authors page

Adds a searchable, sortable, paginated authors page mirroring the
existing PadPage shape. Search matches name OR mapper substring;
'Show erased' toggle off by default; cap-at-1000 hint surfaces when
the backend caps the pre-pagination set. Two-step erase modal: dry-
run preview shows what will be cleared, then a Continue button
commits the irreversible erasure. Disabled-flag banner explains how
to enable when gdprAuthorErasure.enabled is false; per-row Erase
button is disabled with a tooltip in the same condition.

Sidebar gets a Users link between Pads and Communication. App.tsx
listens for the new flags.gdprAuthorErasure on the connect-time
settings push so the page knows the flag state without an extra
round trip. ep_admin_authors namespace is added to i18next's ns
list so all translation keys resolve.

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

* fix(authors): i18n the pagination controls

Spec review caught three hardcoded English strings in the
/admin/authors pagination footer ('Previous Page', 'Next Page',
'X out of Y'). Carried over from PadPage.tsx via the plan template,
which had the same gap. Added three new keys to ep_admin_authors
and routed the spans through Trans/t().

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

* fix(authors): banner CSS, IconButton attribute drop, erase phase string

Code review on the AuthorPage commit caught three issues:

- Disabled-flag banner used dialog-confirm-content classname which is
  position: fixed + centered + z-index: 101, making it render as a
  modal-style overlay over the table. Drop the className and define
  the banner with inline styles only; add role='alert' for SR users.
- The Erase IconButton spread {data-disabled-reason: …} alongside
  {disabled: true}, but IconButton only forwards a small allowlist of
  props — the data attribute was silently dropped. Replaced with a
  conditional title that flips to the disabled-reason string when the
  button is disabled (which IconButton does forward).
- 'Erasing…' string was rendered during loading-preview, but the
  string literally describes the commit phase. Added a new
  loading-preview key for the preview-loading state, and surface the
  existing 'erasing' string under the buttons during the committing
  phase.

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

* test(admin): Playwright /admin/authors + fix i18n key shape

The earlier en.json shipped namespace-prefixed JSON keys
('ep_admin_authors:title': 'Authors') which is the wrong shape:
i18next splits the lookup on ':' to extract the namespace, then looks
up the bare key in the loaded namespace data. The existing convention
(admin/public/ep_admin_pads/en.json) uses flat keys without the
namespace prefix; matching it makes every
<Trans i18nKey='ep_admin_authors:foo'/> resolve to the intended
translated string. Strings render as English fallback without this
fix; only the page-title test passes (and only by substring accident).

Also adds the Playwright coverage required by Task 8: localized
title, empty-state message on a fresh search tag, disabled banner
toggling with gdprAuthorErasure.enabled.

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

* fix(test): JSONC-tolerant settings parse + sidebar count = 7

CI on PR #7667 surfaced two test failures caused by my changes:

1. setErasureFlag() in admin_authors_page.spec.ts used JSON.parse on
   the raw settings.json textarea content. The CI environment loads
   settings.json.template which has unquoted property names, trailing
   commas, and block + line comments — JSON.parse rejects all three.
   Switched to `new Function('return (' + raw + ')')` which evaluates
   the textarea as a JS object literal, accepting every shape
   Etherpad's own settings loader handles.

2. admintroubleshooting.spec.ts hardcoded
   `menu.locator('li').toHaveCount(6)`. The new /authors sidebar
   entry made it 7. Updated the assertion and the sidebar comment.

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

* fix(authors): IconButton title, Dialog.Title, preview errors, settings restart

Final whole-branch review found three Important UX/a11y defects, plus
CI flagged one runtime defect:

- IconButton renders the title prop as a visible <span>, not as the
  HTML title attribute. Disabled rows were displaying the 80-character
  'Author erasure is disabled...' string next to every trash icon.
  Reverted to the short 'Erase' label; the page-level banner already
  explains the disabled state.
- Radix Dialog.Content was missing Dialog.Title. Wrapped the existing
  <h3> in <Dialog.Title asChild> so screen readers can announce the
  dialog purpose.
- onPreview proceeded to render the preview UI even when the backend
  reply carried {error}, leaving 'Will clear undefined token
  mappings...' on screen. Now mirrors onErase.
- The disabled-banner-hidden Playwright test failed because
  settings.json save does not hot-reload. setErasureFlag now
  restartEtherpad's after saveSettings and re-logins.

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

* fix(authors): action Qodo review — lastSeen, flag-gating, defensive payloads

Qodo on PR #7667 surfaced three issues:

1. (Bug, Correctness) lastSeen lost or stale.
   - mapAuthorWithDBKey only updated `timestamp` for returning authors
     so the admin /authors 'Last seen' column drifted on every reconnect
     without an identity write. Now stamps both timestamp and lastSeen.
   - anonymizeAuthor's two db.set calls overwrote globalAuthor without
     preserving lastSeen, blanking the column for erased rows. Both
     writes now carry forward `existing.lastSeen ?? existing.timestamp`.
   - searchAuthors falls back to rec.timestamp when rec.lastSeen is
     missing so legacy records aren't blank.

2. (Rule violation, Security) /authors route not flag-gated.
   The new admin-socket read paths (authorLoad, anonymizeAuthorPreview)
   were always-on; only the destructive anonymizeAuthor was gated.
   Project rule (Compliance ID 6) requires new features behind a flag,
   disabled by default. All three handlers now check
   gdprAuthorErasure.enabled and return {error:'disabled'} when off.
   The sidebar 'Authors' link is hidden when the flag is off
   (deep-link to /admin/authors still works and renders the existing
   disabled banner so docs can point to it).

3. (Bug, Reliability) Socket destructure throws on missing payload.
   Handlers signed `async ({authorID}: {authorID: string}) => …`
   threw before try/catch when a client emitted with no payload,
   producing an unhandled rejection. Switched to
   `async (payload: any) => { const authorID = payload?.authorID; … }`.

Test impact: anonymizeAuthorSocket gains two regressions (authorLoad
disabled-shape, payload-less emits don't crash) and updates the
preview-when-flag-off test to assert {error:'disabled'} per the new
gating posture (was 'preview still works'). admintroubleshooting
sidebar-count reverts 7 → 6 since the Authors link is now conditional
on the flag (off by default in the test environment).

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-14 08:11:48 +01:00
John McLear
cbe551b432
feat(updater): tier 3 — auto update with grace window (#7607) (#7720) 
* feat(updater): scheduled execution state + graceStartTag dedupe field (#7607)

Preparation for Tier 3 of the auto-update subsystem:
- ExecutionStatus gains `scheduled` (targetTag, scheduledFor, startedAt).
- EmailSendLog gains `graceStartTag` for one-shot grace-start email dedupe.
- state validator accepts the new shape, requires per-status fields,
  and backfills graceStartTag=null on a Tier 1/2 state file.

Plus the implementation plan at
docs/superpowers/plans/2026-05-11-auto-update-pr3-tier3-auto.md.

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

* feat(updater): decideSchedule pure decision function (#7607)

Adds src/node/updater/Scheduler.ts with the Tier 3 pure decision logic:
- schedules when canAuto + idle/verified/terminal-cleared
- reschedules when a newer tag appears mid-grace
- emits a grace-start email (once per tag) when adminEmail is set
- cancels a stale schedule when policy flips canAuto off
- no-ops during in-flight / terminal states
- clamps preApplyGraceMinutes to [0, 7 days]

Also extends Notifier's EmailKind union with 'grace-start' so the
decision result types correctly.

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

* feat(updater): scheduler timer runner with arm/cancel (#7607)

Adds createSchedulerRunner to Scheduler.ts:
- arm(): clears any prior timer, sets a fresh one for scheduledFor
- cancel(): clears the pending timer, idempotent
- past scheduledFor → fires with delay=0 (rehydrate after restart-in-grace)
- single-fire-per-arm semantics; armedFor cleared on fire

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

* refactor(updater): extract apply pipeline shared by HTTP + scheduler (#7607)

Lifts the preflight → drain → execute orchestration out of the
/admin/update/apply HTTP handler into src/node/updater/applyPipeline.ts.
The HTTP handler keeps its 4xx status mapping; the pipeline owns the
state transitions, lock release, drain coordination, and rollback hand-
off. The new ApplyPipelineDeps interface accepts an onAccepted callback
so the HTTP path can still 202 mid-flow while the Tier 3 scheduler path
(next commit) can no-op.

Adds `scheduled` to the apply allowed-entry list so an admin can "Apply
now" during the Tier 3 grace window.

13 vitest cases cover happy / preflight-failed / cancelled / busy /
lock-held / scheduled-entry / rollback / lock-release. Existing 12
mocha integration tests still pass without change.

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

* feat(updater): wire Tier 3 scheduler into boot + performCheck (#7607)

- expressCreateServer instantiates the scheduler runner and rehydrates
  the timer when a prior boot left state.execution = scheduled
- performCheck evaluates decideSchedule after the notifier pass:
  schedule transitions state + sends grace-start email + arms timer;
  cancel-schedule resets to idle + cancels timer
- shutdown cancels the timer
- exposes cancelScheduler() so the cancel endpoint (next commit) can
  drop the pending schedule
- buildSchedulerApplyDeps() supplies the full production-wired pipeline
  deps (preflight, executor, rollback) for the scheduler-triggered apply

Adds tests/backend/specs/updater-scheduler-integration.ts covering
boot-rehydrate fire-on-past and the decision-to-state round-trip.

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

* feat(updater): cancel handler supports Tier 3 scheduled state (#7607)

POST /admin/update/cancel now accepts execution.status === 'scheduled'
in addition to preflight/draining. The handler calls cancelScheduler()
to drop the pending in-process timer, then transitions state to idle
with lastResult.outcome = 'cancelled' (mirroring the existing pattern).

Adds a Tier 3 integration test that seeds a scheduled state, calls
/admin/update/cancel, and asserts the state machine landed correctly.

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

* feat(admin): countdown + cancel UI for Tier 3 scheduled updates (#7607)

- store.ts: extend Execution union with the scheduled variant
- UpdatePage.tsx: render countdown panel during scheduled; Apply button
  is relabelled "Apply now" so the admin can skip the remaining grace;
  Cancel button accepts scheduled state
- UpdateBanner.tsx: dedicated scheduled banner with live remaining time
- en.json: new i18n keys (execution.scheduled, banner.scheduled,
  page.scheduled.{title,countdown,apply_now})

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

* test(updater): playwright spec for Tier 3 scheduled UI (#7607)

Three cases against a mocked /admin/update/status:
- countdown panel + Apply now + Cancel render when execution is scheduled
- Cancel button posts /admin/update/cancel and triggers re-fetch
- /admin (banner) shows "Auto-update to <tag> scheduled" copy

Mirrors the existing update-page-actions.spec.ts mock pattern (page.route).

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

* docs(updater): document Tier 3 auto with grace window (#7607)

- doc/admin/updates.md: flip Tier 3 from "designed, not yet implemented"
  to current; expand preApplyGraceMinutes table row; add a Tier 3
  section explaining schedule / cancel / Apply now / restart-in-grace
  and the grace-start email
- settings.json.template: clarify the preApplyGraceMinutes comment
- CHANGELOG.md: Unreleased entry for Tier 3
- runbook §11: full Tier 3 smoke (happy, cancel, apply-now, restart-in-
  grace, email) plus the additional sign-off checkboxes

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

* fix(admin): UpdatePage handles missing execution field; scope spec locator (#7607)

Two CI fixes for PR #7720:

1. UpdatePage.tsx — optional-chain us.execution.status. Integration test
   stubs (update-banner.spec.ts) ship payloads without the Tier 2/3
   execution / lastResult / lockHeld fields; without optional chaining
   on the new scheduled-derivation line the whole page crashed before
   the h1 rendered, breaking the unrelated "renders current version"
   test.

2. update-scheduled.spec.ts — scope the v2.7.2 assertion to the
   .update-scheduled section. The regex was matching three elements
   (banner, countdown panel, changelog link) and tripped Playwright's
   strict-mode locator check.

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

* fix(updater): address Qodo review (Tier 3 race conditions + tier-off bypass) (#7607)

Four fixes for bugs flagged by Qodo's review of PR #7720:

1. **Tier=off bypasses scheduler** (correctness). expressCreateServer
   used to instantiate the scheduler and rehydrate any persisted
   `scheduled` state regardless of `updates.tier`. A user who set
   `tier: "off"` after a schedule had been persisted would still see
   the timer fire after restart. The boot path now skips scheduler
   creation when tier is off and explicitly clears a stale scheduled
   state to idle (logged so the admin sees what happened).

2. **Timer fire skips state recheck** (reliability). The scheduler's
   timer callback called applyUpdate() directly. Race: admin clicks
   Cancel at the same instant the timer fires, or the tier flips
   during the grace window. Now schedulerTriggerApply re-loads state
   and re-evaluates policy via a new pure decideTriggerApply() helper
   in Scheduler.ts. If state is no longer scheduled (or scheduled for a
   different tag), aborts. If policy now denies auto, persists state
   back to idle and aborts.

3. **Apply-now leaves scheduler timer armed** (correctness). The apply
   endpoint accepts `scheduled` as an entry status but didn't cancel
   the in-process scheduler timer. After the admin clicks Apply now,
   the still-armed timer could later fire and attempt another apply
   (especially if the manual one finishes in preflight-failed, which
   is also an allowed-entry status). Apply handler now calls
   cancelScheduler() when entering from `scheduled`.

4. **scheduledFor not validated as timestamp** (reliability). State
   validator only required scheduledFor / startedAt etc. to be
   non-empty strings; a hand-edited "scheduledFor": "garbage" would
   pass validation and yield NaN delay → immediate fire. The
   validator now requires known timestamp fields to be parseable
   via Date.parse().

Tests: 6 new decideTriggerApply cases + 3 new state.ts validation
cases. 189 vitest pass / 29 mocha integration pass / ts-check clean.

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-12 20:50:06 +01:00
John McLear
451bd9c3eb
feat(pad): scrub history in-place on the pad URL (#7659) (#7710) 
* feat(pad): scrub history in-place on the pad URL (#7659)

Clicking the timeslider toolbar button now keeps the user on /p/:pad and
toggles a hash-based history mode (#rev/N) instead of navigating to a
separate /timeslider page. The pad shell — chat, users panel, settings,
plugin chrome — stays mounted across the transition. A sticky banner
plus a sepia tint on the toolbar make it unmistakable that what is
visible is historical, not live.

Implementation:

- New PadModeController (src/static/js/pad_mode.ts) owns enter/exit,
  the URL hash, browser back/forward, and a mutation-observer bridge
  from the inner timeslider's revision label/date into the outer
  banner. Esc and a Return-to-live button both exit history.
- pad.html grows a banner element and an iframe mount slot. The live
  ACE iframe stays mounted but hidden during history; on exit the
  socket is still alive, so the user snaps straight back to the
  current state without a reconnect.
- The /p/:pad/timeslider route 302-redirects to the pad page for
  direct visits (legacy bookmarks), and serves the timeslider HTML
  for the in-pad iframe when called with ?embed=1. The embedded
  variant hides the redundant title and return-to-pad button via
  CSS; the slider, settings, and export controls stay reachable.
- Legacy #NN shortlinks are preserved through the redirect by the
  browser and translated to #rev/NN client-side.

Tests:

- New backend spec asserts the 302 redirect, pad-name preservation,
  and the ?embed=1 path still serves the timeslider HTML.
- New padmode.spec.ts exercises toolbar entry, return-to-live,
  browser back, and direct /timeslider URL handling. Asserts the
  rendered localized banner string, not just element presence.
- Existing timeslider specs that hit /p/:pad/timeslider directly
  now pass ?embed=1 to bypass the redirect.

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

* fix(pad): make history iframe fill the editor area (#7659)

Without an explicit positioning model the history-frame-mount inherited
half-width from a phantom flex parent and the embedded timeslider
rendered at 640×625 instead of the full editor area. Switch to the same
absolute-fill model the live ACE iframe uses by making
#editorcontainerbox the positioning anchor when in history mode.

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

* fix(pad): address Qodo review and CI failures (#7659)

Concrete review fixes for PR #7710:

- Tighten the embed query check from `if (!req.query.embed)` to
  `req.query.embed !== '1'` so values like `?embed=0` no longer bypass
  the redirect.
- Fix the `#rev/latest` mapping: the parser yields -1 for "latest",
  which the iframe sync handler was clamping to 0 and so jumping the
  embedded timeslider to revision 0. Resolve "latest" to the inner
  BroadcastSlider's upper bound instead.
- Update existing backend tests (`socialMeta`, `specialpages`) that
  hit `/p/:pad/timeslider` directly — they now pass `?embed=1` like
  the rest of the suite. Without this fix three pre-existing tests
  failed CI (302 instead of 200).
- Document the route change in `doc/skins.md` and `doc/skins.adoc`:
  direct visits redirect; iframe consumers use `?embed=1`.
- Back out a stray `data-theme="editorial"` attribute and the
  hardcoded Google Fonts `<link>` tags from `pad.html` that leaked
  into the branch from an unrelated working-tree change.

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

* feat(pad): consolidate chrome and replay chat/users in history mode (#7659)

Picks up the rough edges left by the initial in-place history mode:
the embedded timeslider iframe was rendering its own duplicate Settings
and Export buttons, and the chat panel + users list still showed live
state while the editor scrubbed back in time.

Chrome consolidation
- Hide the entire inner editbar's right-side toolbar and modal popups
  in embedded mode (slider stays). Outer pad shell now owns Settings,
  Export, Share, Users, Chat across both modes.
- Outer Settings popup grows a "History playback" section (visible
  only when scrubbing) with playback speed + follow-contents. Both
  bridge to the iframe's BroadcastSlider state.
- Outer Export anchors are rewritten to /p/<pad>/<rev>/export/<type>
  on each scrub and restored on exit, so Save As exports the visible
  historical revision.

Chat replay
- Each chat message is annotated with data-timestamp at render time.
  In history mode, messages newer than the scrubbed revision's
  timestamp are display:none'd; a "Chat as of HH:MM" header sits
  above the chat log.
- Restores cleanly on exit (inline display cleared, header removed).

Users replay
- Live users table is replaced with the embedded timeslider's
  authors-at-this-revision label while scrubbing; restored on exit.

Plumbing
- Expose padContents on window in broadcast.ts so the outer pad can
  read currentTime after each scrub without postMessage.
- Expose BroadcastSlider on window in timeslider.ts so the outer pad
  can register an onSlider callback to drive replay UI.

Tests
- New padmode specs cover: history-only Settings section, hidden
  embedded chrome, chat filter + replay header, Export href
  rewriting + restore, authors-row swap + restore.
- timeslider_line_numbers cookie-persistence test updated to bypass
  the now-hidden inner Settings popup (programmatic checkbox).

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

* fix(pad): theme propagation, hide inert buttons, plugin loading (#7659)

Picks up rough edges from the in-place history mode that turned up in
real usage:

Theme / dark mode
- skin_variants.updateSkinVariantsClasses now also walks the history
  iframe (and its ace_outer/ace_inner) so toggling dark mode while
  scrubbing re-themes the embedded view in lockstep.
- timeslider.ts inherits the parent's skinVariant tokens (super-dark-*
  / dark-* / full-width-editor) on first paint when it detects it is
  embedded — same-origin guarantee, falls through silently if not.

Toolbar UX
- Hide #editbar .menu_left (Bold/Italic/Lists/Indent/Undo/...) and the
  show-more chevron while in history mode. Those buttons target the
  hidden live editor and would do nothing useful; rendering them
  disabled-looking implied state the user doesn't have. Right-side menu
  (Settings / Share / Users / Chat / Home) stays at full opacity and
  fully interactive.

Slider position
- Pin the embedded #editbar to the bottom of the iframe so the outer
  banner and the slider can't visually compete for the same band of
  pixels. Reserve padding-bottom on the iframe's editorcontainerbox so
  the editor never scrolls under the slider.

Plugin loading in timeslider
- timeSliderBootstrap.js now pre-loads plugin modules into a Map and
  passes them to plugins.update(), mirroring padBootstrap.js. Without
  this the loadFn fallback called require(path) at runtime, which the
  esbuild-bundled timeslider couldn't resolve, so client_hooks like
  ep_headings2's aceRegisterBlockElements silently failed to register
  and historical revisions rendered without plugin chrome.

Tests
- New padmode specs cover: outer toolbar's left/right asymmetry, slider
  pinned to bottom, dark-mode class propagation into the history iframe.

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

* feat(pad): move history slider into the outer toolbar (#7659)

The slider previously rendered inside the embedded iframe — first at the
top (where it visually competed with the banner), then briefly at the
bottom (where the chat icon overlapped it). Both were wrong. Move the
controls into the outer toolbar's left zone, where #editbar .menu_left
is hidden in history mode and the slider can occupy the full width
without colliding with anything.

- pad.html grows a #history-controls div (slider + play/pause/step
  buttons + timer) inside #editbar, between menu_left and menu_right.
  Hidden by default; revealed via body.history-mode CSS.
- pad.css swaps #editbar .menu_left out for #history-controls in
  history mode (display:none / display:flex).
- timeslider.css fully hides the embedded iframe's #editbar — the
  outer toolbar now owns the slider, and the iframe is purely the
  editor surface.
- pad_mode.ts wires the outer controls as a remote control: the
  range input calls inner BroadcastSlider.setSliderPosition, the play
  button calls BroadcastSlider.playpause, step buttons forward clicks
  to the inner #leftstep/#rightstep so they share the existing logic.
  An onSlider subscription mirrors inner state back into the outer
  slider value, timer label, and play-button .pause class.

Tests
- Existing timeslider.spec asserts the outer controls are visible.
- New padmode specs cover: inner editbar fully hidden, outer toolbar
  swap (menu_left → history-controls), and outer slider drives the
  iframe's revision via BroadcastSlider.

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

* fix(pad): exempt embedded history iframe from userdup kick (#7659)

When the in-place history iframe opens its socket, the server's
duplicate-author kick treats it as a stale tab and disconnects the
parent pad's live socket — toolbar-overlay drops over the editor and
Settings/Share/Users/Chat all stop responding. Mark the iframe's
connection with `embed=1` in the socket.io handshake query, record it
on sessionInfo, and skip the kick whenever either side is embedded.

- timeslider.ts: detect `?embed=1` (and parent !== window) on
  the iframe URL, pass through as a query parameter to socketio.connect.
- PadMessageHandler: read socket.handshake.query.embed on CLIENT_READY,
  set sessionInfo.embed; the duplicate-author kick now skips when
  either the connecting session OR the existing session is embedded.

Behavior preserved
- Two real tabs (both non-embedded): older tab still gets kicked.
- Authenticated sessions still bypass the kick entirely.
- Live pad socket survives entry into history mode.

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

* fix(pad): a11y of history toolbar controls (#7659)

The new history controls (slider + play/step/timer) had hardcoded
English aria-labels, which html10n won't replace because they were
present without the data-l10n-aria-label marker. Screen readers in
non-English locales would have heard English. Drop the static aria
labels and let html10n.translateElement populate aria-label from the
data-l10n-id translation, matching how the rest of the toolbar works.

- pad.html: remove hardcoded aria-label on play/step buttons and the
  range input; keep titles (hover tooltip) and data-l10n-id. Add
  role="toolbar" + data-l10n-id on the controls container so the
  toolbar landmark is announced. Mark play button as a toggle with
  aria-pressed reflecting playback state.
- en.json: add pad.historyMode.controlsLabel and
  pad.historyMode.sliderLabel for the toolbar landmark and the slider.
- pad_mode.ts: keep aria-pressed in sync with the inner playback state
  on every revision update.

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

* fix(pad): a11y + responsive for history controls (#7659)

Two issues with the previous a11y attempt: the data-l10n-id on icon
buttons was setting their textContent (drawing "Playback / Pause Pad
Contents" on screen next to the glyph), and there was no responsive
treatment so the timer + slider could overflow narrow viewports.

- pad.html: drop data-l10n-id from the icon buttons. They're now
  empty <button>s. Localized title (hover tooltip) and aria-label
  (screen reader name) are populated by pad_mode.localizeControls()
  using the existing timeslider.* keys, with an html10n.bind
  subscription so language switches re-localize.
- Mark #history-timer as hide-for-mobile.
- pad.css: dedicated @media (max-width: 800px) and 480px rules
  shrink padding, gap, and button widths so play + slider + step
  buttons stay on a single toolbar line at narrow viewports. Mirrors
  the legacy timeslider's responsive behavior.

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

* feat(pad): inline Follow + Playback speed, match toolbar height (#7659)

Two follow-ups from real testing:

- Move "Follow pad content updates" (now "Follow") and "Playback speed"
  out of the Settings popup and inline them in the history-mode
  toolbar, alongside the slider + play/step buttons. They were always
  needed while scrubbing; one extra click into Settings was friction.
  Removed the now-empty #history-settings-section.
- The history controls toolbar was visibly shorter than the live
  toolbar because the icon buttons sat as bare <button> elements
  without the live editbar's <li><a> wrapping. Add explicit
  min-height (40px) and per-button padding so the toolbar is the same
  vertical size in both modes — switching between live and history
  no longer reflows.
- Differentiate "iframe-mounted history view" from "direct ?embed=1
  visit". Only the former hides the inner timeslider editbar — direct
  visits keep their full chrome so existing test/legacy entry points
  stay independently usable. Marker: timeslider.ts adds an
  `iframe-mode` class on body when window.parent !== window; CSS
  scopes the hide to that combo.

Tests
- padmode spec asserts Follow + Speed live in the toolbar (not the
  Settings popup) and are visible in history mode, hidden in live.
- timeslider*.spec direct-?embed=1 flows continue to pass because the
  inner editbar is no longer hidden when not iframe-mounted.

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

* fix(pad): use absolute path for legacy /timeslider redirect (#7659)

CI Firefox failed the legacy-URL redirect test (1 of 32 jobs); Chromium
passed. The redirect Location header was a relative `../padname`, which
both browsers resolve to /p/padname for `/p/padname/timeslider`. Firefox
flaked on it once consistently. Switch to an absolute path including
the proxy prefix so the resolution is unambiguous across browsers.

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

* fix(test): accept 304 on legacy timeslider redirect (#7659)

CI Firefox failed `expect(res.status()).toBe(200)` because Firefox
issues a conditional GET when the redirect target is the same URL the
test just loaded via goToNewPad — the server returns 304 Not Modified
and the test treats that as a regression. Chromium happens to send
fresh requests so it stayed green.

Accept either 200 or 304 — both are valid completed navigations to the
pad page; what we actually care about is the pathname assertion above.

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

* feat(pad): eye toggle for Follow, fix line-number alignment (#7659)

Two refinements from real testing in 9002:

Follow as an eye toggle
- Replace the labeled checkbox with an inline-SVG eye icon. The eye is
  always rendered; a diagonal slash is overlaid via SVG <line> only
  when the underlying (visually hidden) checkbox is unchecked. Default
  state is on (auto-following) so the eye renders unobstructed.
- Localized hover tooltip + aria-label flips with state — html10n
  populates "Following pad changes — click to stop following" vs
  "Not following pad changes — click to follow", and pad_mode.ts
  re-applies on every change event so screen readers narrate the
  action the click would take.
- Hidden checkbox keeps the existing pad_mode.ts bridge code working
  (still reads .checked) and lets <label for="…"> handle the click.

Line-number alignment fix (broadcast.ts)
- The first-line height formula was
    `nextDocLine.offsetTop - innerdocbody.padding-top`
  which only computes the right value when innerdocbody is the
  offsetParent. In the in-pad history iframe, outerdocbody contributes
  its own padding-top to the offsetTop chain, so the first gutter row
  was 20px too tall and every subsequent line drifted out of
  alignment. Use the consistent `next.offsetTop - current.offsetTop`
  formula for every iteration — same result in the standalone
  timeslider, correct result in the embedded one.
- New padmode spec asserts every gutter row's top matches the editor
  line's top within 2px, in iframe-mounted history mode.

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-10 16:21:56 +01:00
John McLear
cbf71285a2
feat(api): clean up published OpenAPI spec for downstream codegens (#7714) 
* feat(api): clean up published OpenAPI spec for downstream codegens

The /api/openapi.json doc had four issues that broke generated tooling
(printingpress.dev, openapi-generator, Postman): empty top-level tags
array, every operation duplicated as GET+POST, 14 operations missing
from the resources map (drift since API 1.2.8), and empty summaries on
several tracked ops.

This PR splits the runtime spec from the published spec via a {public}
flag on generateDefinitionForVersion: the runtime definition fed to
openapi-backend keeps both verbs (existing third-party clients that
call GET /api/x.x.x/foo?apikey=... continue to work), while the spec
served at /api/openapi.json, /rest/openapi.json, and per-version paths
advertises only POST.

Other changes:
  - top-level tags array declares pad/author/session/group/chat/server
  - per-op tags override added to SwaggerUIResource type so chat ops
    (still nested under pad for routing) and checkToken can be tagged
    without changing existing REST URLs
  - 14 missing ops (getAttributePool, getRevisionChangeset, copyPad,
    movePad, getPadID, getSavedRevisionsCount, listSavedRevisions,
    saveRevision, restoreRevision, appendText, copyPadWithoutHistory,
    compactPad, anonymizeAuthor, getStats) backfilled with summaries
  - empty summaries on listSessionsOfGroup, listAllGroups,
    createDiffHTML, createPad filled in
  - new backend tests assert the public spec shape (tags, summaries,
    POST-only) and that runtime routing still resolves both verbs

Driven by integrating Etherpad with printingpress.dev: pointing the
generator at the previous spec produced a 96-command CLI with no
resource grouping and many empty descriptions. Design notes in
docs/superpowers/specs/2026-05-10-openapi-cleanup-design.md.

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

* fix(api): keep checkToken at /rest/<ver>/pad/checkToken (Qodo #2)

Moving checkToken to a new `server` resource broke REST-style
backward compat: existing callers of /rest/<ver>/pad/checkToken
would have hit `code: 3` (no such function). The whole point of
per-op tag overrides is to preserve REST URLs while still grouping
correctly in OpenAPI tags — checkToken should follow the same
pattern as the chat ops.

Keep checkToken in `resources.pad`, give it `tags: ['server']`,
and add a regression test asserting /rest/<ver>/pad/checkToken
still resolves. The `server` resource still exists for `getStats`
(genuinely new server-level op with no prior REST URL).

Updates the design doc accordingly.

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-10 13:02:05 +01:00
John McLear
efb8328084
feat(updater): tier 2 — manual-click update from /admin/update (#7607) (#7704) 
* docs(updater): PR 2 (Tier 2 manual-click) implementation plan

20-task TDD plan for shipping the manual-click update flow on top of the
Tier 1 (notify) work merged in #7601. Covers UpdateExecutor, RollbackHandler,
SessionDrainer, lock + trustedKeys, four admin endpoints (apply / cancel /
acknowledge / log), admin UI updates, integration tests against a tmp git
repo, and a manual smoke runbook for the spec's "before each tier ships"
gate. Plan deliberately scopes signature verification to an opt-in stub
(updates.requireSignature: false default) to avoid blocking on a separate
release-signing project.

Plan: docs/superpowers/plans/2026-05-08-auto-update-pr2-manual-click.md
Spec: docs/superpowers/specs/2026-04-25-auto-update-design.md
Issue: ether/etherpad#7607

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

* feat(updater): extend state + settings for Tier 2 manual-click

Adds ExecutionStatus discriminated union, bootCount, and lastResult to
UpdateState, plus the preApplyGraceMinutes/drainSeconds/diskSpaceMinMB/
requireSignature/trustedKeysPath knobs that Tier 2's executor needs.
loadState backfills the new fields on Tier 1 state files so existing
installs keep working.

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

* feat(updater): PID-based update.lock with stale-pid reaping

Single-flight guard for Tier 2's UpdateExecutor. Atomic O_CREAT|O_EXCL
acquire; on EEXIST, sends signal 0 to the recorded PID and reaps if dead.
Unparseable / partially-written lock files are treated as stale rather
than fatal so a half-written lock from a SIGKILL'd parent doesn't lock
the install out forever.

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

* feat(updater): verifyReleaseTag — gpg-via-git stub for Tier 2 preflight

Default updates.requireSignature=false: log a warning and return ok with
reason=signature-not-required. Set true to make preflight refuse a tag
whose signature does not verify under the system keyring (or
trustedKeysPath via GNUPGHOME). Etherpad's release process does not yet
sign tags consistently; turning the check on by default would break
Tier 2 for every admin and forcing a release-signing change is out of
scope for this PR.

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

* feat(updater): preflight check pipeline for Tier 2

Pure orchestrator over injected probes for install-method, working tree,
disk space, pnpm presence, lock state, remote tag existence and
signature verification. Cheap-and-definitive checks run first; first
failure short-circuits with a typed reason that the route layer will
surface in the preflight-failed admin banner.

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

* feat(updater): rolling update.log helpers (appendLine + tailLines)

Direct file-append + size-based rotation rather than a log4js appender —
avoids re-configuring log4js on top of the user's existing logconfig.
appendLine creates parents, rotates at 10MB (configurable), keeps 5
backups by default. tailLines reads the last N lines for /admin/update/log.

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

* feat(updater): SessionDrainer + handshake guard

Drainer schedules T-60 / -30 / -10 broadcasts and resolves at T=0;
isAcceptingConnections() flips off for the duration. PadMessageHandler
consults the flag at the start of CLIENT_READY and disconnects new
joiners with reason "updateInProgress" — existing sockets are
unaffected. Drains shorter than 30s collapse the early timers to fire
ASAP rather than queue past the drain end.

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

* feat(updater): UpdateExecutor — snapshot, fetch/checkout/install/build, exit 75

Pure-DI orchestrator: spawnFn, copyFile, readSha, saveState, exit are all
injected so unit tests run the full pipeline without spawning real
children or mutating the real install. Streams stdout/stderr to
update.log via the now-best-effort appendLine helper (swallows fs errors
so the executor itself never breaks on read-only / unwritable log dirs).
Failure paths transition to rolling-back and return — the route layer
hands off to RollbackHandler which owns the rollback exit, so we don't
double-exit and lose tail lines.

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

* feat(updater): RollbackHandler — health-check timer + crash-loop guard

checkPendingVerification arms a 60s timer at boot when state is
pending-verification and increments bootCount; bootCount>2 forces an
immediate rollback (crash-loop guard). markVerified persists the
verified state and stops the timer. performRollback restores the
backup lockfile, runs git checkout <fromSha> and pnpm install, lands on
rolled-back or rollback-failed (terminal) on sub-step failure, exits 75
either way so the supervisor restart brings the new state up.

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

* feat(updater): wire RollbackHandler into boot + UpdatePolicy honours rollback-failed

- expressCreateServer now invokes checkPendingVerification before polling starts
  so a previous boot's pending-verification either re-arms the health-check
  timer or, when bootCount has climbed past the crash-loop threshold, forces
  an immediate rollback.
- server.ts calls markBootHealthy after state hits RUNNING so /health-being-up
  is the implicit happy-path signal that cancels the rollback timer.
- /admin/update/status surfaces execution + lastResult + lockHeld so the admin
  UI can render the right Apply / Cancel / Acknowledge state.
- UpdatePolicy gains an `executionStatus` input. While it equals 'rollback-failed',
  canAuto / canAutonomous are denied (reason: rollback-failed-terminal); manual
  stays on because clicking Apply IS the intervention the terminal state needs.

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

* feat(updater): apply / cancel / acknowledge / log endpoints

Strict admin-only POSTs that drive Tier 2's manual-click flow:
- POST /admin/update/apply: acquire lock, persist preflight, run preflight,
  drain $drainSeconds, executeUpdate (which exits 75 on success), or run
  performRollback on a failure path (also exits 75).
- POST /admin/update/cancel: cancel a pre-execute drain/preflight, write
  cancelled lastResult, release lock.
- POST /admin/update/acknowledge: clear terminal states (preflight-failed,
  rolled-back, rollback-failed) back to idle. lastResult is preserved so
  the admin still sees what happened.
- GET /admin/update/log: tail var/log/update.log (200 lines) for the in-
  progress UI. Strict admin auth.

Also:
- socketio hook exports getIo() so the apply endpoint can broadcast the
  drain shoutMessage outside the regular hook surface.
- ep.json registers updateActions after admin/updateStatus.
- 11 mocha integration tests cover auth, policy denial, execution-busy,
  acknowledge-clears-terminal, log content-type.

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

* feat(updater): admin UI Apply/Cancel/Acknowledge + live log stream

UpdatePage renders the right action set based on execution.status:
Apply when idle/verified and policy allows, Cancel during
preflight/draining, Acknowledge on terminal preflight-failed /
rolled-back / rollback-failed. While the executor is in flight
(preflight/draining/executing/rolling-back) the page polls
/admin/update/log + /admin/update/status once a second and shows the
rolling tail; polling stops automatically when the run terminates.

lastResult and policy denial reasons surface localised copy. Buttons
disable themselves while a network round-trip is in flight to dodge
double-clicks. New i18n keys live under update.page.{apply,cancel,
acknowledge,log,execution,policy.*,last_result.*}, update.execution.*,
update.banner.terminal.rollback-failed, and update.drain.{t60,t30,t10}.

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

* feat(updater): pad shoutMessage renders update.drain.* via html10n

broadcastShout now sends {messageKey, values, sticky} so the existing
pad-side shout pipeline can route through html10n.get(). The renderer
gains a values pass-through so update.drain.t60 etc. interpolate
{{seconds}}, and gives updater shouts a different gritter title (the
banner.title localised string) so users know it's a system event
rather than a generic admin message.

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

* feat(updater): rollback uses git checkout -f + integration suite over tmp git repo

RollbackHandler now does git checkout -f <fromSha> BEFORE overlaying the
backup lockfile. Without -f, git refuses checkout when there are
unstaged modifications to files it would overwrite — exactly the case
after a partial executor run that mutated the working tree. With -f the
partial mutation is discarded and the working tree returns to fromSha
cleanly. The backup-lockfile copy is still done (belt-and-braces) but
tolerates ENOENT since checkout already restored the right lockfile.

The new integration suite at src/tests/backend/specs/updater-integration.ts
exercises the full pipeline against a disposable git repo: happy path,
install-fail rollback, build-fail rollback, crash-loop guard, and a
target-sha-doesn't-exist rollback-failed terminal case. 5 mocha tests.

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

* test(updater): Playwright admin Apply / Cancel / Acknowledge flow

Stubs /admin/update/status (and /admin/update/apply for the apply path)
at the route level so we can assert UI transitions without actually
running an update. Four scenarios:
- Apply button POSTs and re-fetches status (>=2 status fetches total).
- install-method-not-writable hides the button and shows localised
  denial copy.
- rollback-failed terminal state shows the Acknowledge button and the
  "Manual intervention required" lastResult copy.
- lockHeld=true hides Apply even when policy.canManual is on.

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

* feat(updater): admin banner shows rollback-failed terminal alert

When execution.status === 'rollback-failed' the banner switches to a
role=alert with the strong update.banner.terminal.rollback-failed copy
and overrides the regular "update available" framing — an admin who
left the system in this state needs to fix it before any other admin
work matters. Other terminal states (preflight-failed, rolled-back) are
informational and surface on the page itself, not the banner.

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

* docs(updater): Tier 2 admin docs + manual smoke runbook + CHANGELOG

doc/admin/updates.md gains a full Tier 2 section: prerequisites
(git install + process supervisor with sample systemd unit), Apply
flow with timings, every failure mode and the resulting state, the
four endpoints, and the signature-verification opt-in. Settings
table picks up the new updates.* knobs.

docs/superpowers/specs/2026-04-25-auto-update-runbook.md is the
manual smoke runbook the design spec calls for: disposable VM,
systemd unit, every observable transition (happy path, install/
build-fail rollback, crash-loop guard, rollback-failed terminal,
cancel during drain) plus a sign-off checklist for the release cut.

CHANGELOG Unreleased section explains the supervisor requirement
and points readers at the runbook.

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

* docs(updater): note docker-friendly update flows as follow-up work

Tier 2 refuses Apply on installMethod=docker because in-container
mutation doesn't survive a container restart. Adds a future-work note
covering the two reasonable paths for an in-product docker Apply
button (instructions-only vs deploy-webhook) and explicitly rules out
mounting /var/run/docker.sock as a footgun. Watchtower gets a pointer
for admins who want fully autonomous docker updates today.

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

* fix(updater): address Qodo review (1-6) + Playwright strict-mode CI fix

1. Tier 2 endpoints now gate on tier in {manual, auto, autonomous} —
   notify and off return 404 to match the prior PR-1 behaviour. Gate is
   evaluated per-request via app.use middleware so a settings.json reload
   takes effect without a full restart, and so integration tests can flip
   the tier dynamically. Adds a regression test that exercises 404 at
   tier=notify across all four endpoints.

2. cancel/apply race fixed: /admin/update/cancel no longer releases the
   lock — apply's finally block owns it for the request's lifetime. Apply
   now reloads state after preflight and aborts with 409 cancelled-during-
   preflight if execution.status is no longer 'preflight' for the same
   targetTag. Prevents a second apply from sneaking in while the first is
   still running its slow checks, and prevents the post-cancel apply from
   continuing into drain/execute.

3. SessionDrainer now restores acceptingConnections=true at drain
   completion (not just on cancel). The lock + persisted execution.status
   prevent a fresh apply from racing in — the in-memory flag was redundant
   safety that turned into a wedge if the executor threw post-drain. Adds
   a unit test asserting the flag is restored after natural drain end.

4. PadMessageHandler drain guard switched from socket.json.send (a
   socket.io v2/v3 API that may not exist on v4) to socket.emit('message',
   ...) for consistency with the other disconnect paths in the file.

5. Spawn 'error' handlers added to runStep helpers in UpdateExecutor and
   RollbackHandler, plus the gpg verify-tag spawn in trustedKeys. Without
   them, a missing/unexecutable binary leaves the promise hanging forever
   and the update flow stuck in-flight. SpawnFn type extended to allow
   on('error', ...) listeners cleanly. Spawn errors now resolve with code
   1 + the error message in stderr, so the existing failure-detection
   branches fire normally.

6. executeUpdate body wrapped in try/catch. An exception from readSha,
   saveState, copyFile, or any step now lands in a rolling-back persist +
   returns failed-checkout, so the route's post-executor rollback path
   picks it up. State can no longer wedge at 'executing'. The catch's
   inner saveState is itself try/wrapped so a write-after-write failure
   doesn't crash the route either.

CI: Playwright update-page-actions strict-mode violation fixed. Both the
banner and the lastResult <p> contain "Manual intervention required";
selector now scopes to p.last-result-rollback-failed for the lastResult
assertion specifically.

129 vitest unit tests + 23 mocha integration tests passing; ts-check clean.

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

* fix(updater): address Qodo #7 (status leak) + #8 (short-drain values)

#7. /admin/update/status now redacts diagnostic strings for unauth callers
even when requireAdminForStatus is left at its default (false). Status
enum + outcome enum are kept (the admin banner / pad-side badge need them
to render the right UI) but execution.reason / execution.fromSha /
execution.targetTag and the same fields on lastResult are stripped.
Authed admin sessions still get the full payload — they're looking at
their own server's diagnostics. Two new mocha tests cover both paths:
"redacts execution.reason / lastResult.reason for unauth callers" and
"returns full diagnostic payload to authed admin sessions".

#8. SessionDrainer no longer schedules T-30 / T-10 broadcasts when the
configured drainSeconds can't honour them. Previously, with drainSeconds
< 30 the T-30 timer fired at zero remaining but the broadcast still
claimed "30 seconds" — misleading. Now T-30 only schedules when
drainSeconds > 30 and T-10 only when > 10. Admins picking a short drain
get fewer announcements but each carries an accurate countdown. The
opening announcement now reports the configured drain length rather
than a hardcoded 60. Two updated unit tests: drainSeconds=15 (skips
T-30, still fires T-10) and drainSeconds=5 (skips both).

131 vitest unit + 26 mocha integration tests passing; ts-check clean.

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

* fix(updater): address Qodo follow-up — tag injection, rollback rejections, state validation

Qodo posted three new concerns after the first fix push.

1. Git tag option injection (security). The release tag from GitHub's
   tag_name flowed into `git checkout` / `git verify-tag` as a positional
   arg. A tag starting with '-' would be parsed as an option and could
   bypass signature verification or change checkout semantics. Mitigated
   in three layers:

   - New refSafety helper (isValidTag / assertValidTag / refsTagsForm)
     enforces a strict subset of git's check-ref-format spec: rejects
     leading '-' or '.', whitespace, control chars, and ~ ^ : ? * [ \\
     and the '..' sequence.
   - VersionChecker validates tag_name before persisting to state, so a
     malformed value from a misconfigured githubRepo never lands on disk.
   - UpdateExecutor calls assertValidTag and uses the refs/tags/<tag>
     form for git checkout. trustedKeys also validates and adds '--' to
     git verify-tag for an end-of-options marker. updateActions does an
     up-front isValidTag check on state.latest.tag so a corrupt state
     file gets a clean 409 instead of a 500.

2. Unhandled rollback rejections. checkPendingVerification was firing
   `void deps.saveState(...)` and `void performRollback(...)` without
   .catch(), so an fs error during boot's rollback path would bubble out
   as an unhandled rejection. Both callsites now go through fireSaveState
   / fireRollback helpers that catch and log; rollback rejections fall
   through to a best-effort terminal-state write + exit 75 so the
   supervisor can re-try the next boot with bootCount++.

3. Execution state under-validated. isValidExecution previously checked
   only that `status` was a known enum value, so a hand-edited state file
   with `{execution: {status: 'pending-verification'}}` (missing fromSha
   / targetTag / deadlineAt) would pass validation and reach
   RollbackHandler with undefined refs. The validator now consults a
   per-status required-fields map mirroring the ExecutionStatus union in
   types.ts and rejects empty strings as well as missing fields. Same
   tightening applied to lastResult.outcome (must be in the allowed enum,
   not just any string). Six new unit tests cover hand-edited corruption.

145 vitest + 26 mocha tests green; ts-check clean.

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-10 09:00:07 +01:00
John McLear
4f1b524864
feat(admin): document admin endpoints in OpenAPI (#7693) (#7705) 
* chore(admin): typesafe API client + TanStack Query rails (#7638) (#2)

* docs(admin): design for typesafe API client + TanStack Query rails (#7638)

Rails-only scope: codegen toolchain, runtime client, and provider — no
call-site migrations. Admin endpoints are not yet covered by the OpenAPI
spec, so a separate issue will follow before any migration is useful.

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

* docs(admin): implementation plan for typesafe API client rails (#7638)

Step-by-step task breakdown for the rails-only PR: codegen toolchain,
runtime client, TanStack Query provider, CI freshness check, docs. No
call-site migrations until admin endpoints are added to the OpenAPI
spec (separate follow-up).

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

* feat(api): export generateDefinitionForVersion from openapi hook

Required by the admin codegen script (#7638) to dump the OpenAPI spec
without booting Express. No behavior change for the request hook.

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

* chore(admin): add OpenAPI codegen + TanStack Query deps (#7638)

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

* chore(admin): add OpenAPI spec dump entry (#7638)

Loaded via tsx by gen-api.mjs in the next commit. Writes JSON to a file
path argument so log4js stdout output (from Settings init) does not
pollute the spec output.

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

* chore(admin): wire OpenAPI codegen into build (#7638)

Adds gen:api script and amends build/build-copy to regenerate
admin/src/api/schema.d.ts before compiling. The generated file is
checked in so it shows up in PR review and so a fresh checkout doesn't
need codegen to typecheck.

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

* feat(admin): typed openapi-fetch + react-query client (#7638)

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

* feat(admin): TanStack Query provider, dev-only devtools (#7638)

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

* feat(admin): mount TanStack Query provider at root (#7638)

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

* test(admin): smoke test for typed openapi-fetch client (#7638)

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

* ci(admin): verify generated OpenAPI schema is up to date (#7638)

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

* docs(admin): document OpenAPI codegen workflow (#7638)

Replaces the default Vite scaffold README with admin-specific scripts
table and codegen workflow notes.

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

* build(admin): exclude __tests__ from tsc include (#7638)

The smoke test imports node:test/node:assert which need @types/node.
Admin source is browser-only, so excluding __tests__ from the production
typecheck is cleaner than adding Node types to the bundle config. The
test still runs under tsx, which doesn't share this constraint.

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

* chore: regenerate lockfile with pnpm 10 to restore overrides block (#7638)

Adding admin deps with pnpm 11 stripped the top-level \`overrides:\`
section from pnpm-lock.yaml, which CI uses pnpm 10 to verify. Result:
ERR_PNPM_LOCKFILE_CONFIG_MISMATCH on every job. Re-running pnpm 10
\`install --lockfile-only\` restores the overrides block; the new admin
package entries land in the same commit. Two stale lockfile entries
not present in package.json (\`serialize-javascript\` version pin and
\`uuid@<14.0.0\`) were normalized by the regen — package.json is the
source of truth for those.

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

* build(docker): preserve strictDepBuilds=false in trimmed workspace yaml (#7638)

Adding tsx as an admin devDep brings esbuild@0.27.x into the resolved
subgraph, and the Dockerfile's runtime stage was overwriting
pnpm-workspace.yaml with a stripped-down version that lost the
strictDepBuilds=false setting from the source repo. With pnpm 10's
default of strictDepBuilds=true, the install then errors on
ERR_PNPM_IGNORED_BUILDS for esbuild + scarf rather than warning.

Restore the strictDepBuilds=false and the @scarf/scarf ignore in the
trimmed yaml so the production install matches develop's behavior.

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

* fix(admin): point client baseUrl at /api/<version> via codegen (#7638)

Qodo flagged: with baseUrl='/' and schema paths like '/createGroup',
calls landed at /createGroup, but the backend mounts the FLAT-style
spec under /api/<version>/. So once a call site lands, every request
404s.

gen:api now also emits admin/src/api/version.ts containing
LATEST_API_VERSION (read from info.version in the spec) and a derived
API_BASE_URL = `/api/<version>`. client.ts imports API_BASE_URL.
Workflow freshness check covers both generated files.

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

* build(admin): cross-platform spawn in gen-api.mjs (#7638)

Windows CI failed because spawnSync('pnpm', ...) cannot resolve
pnpm.cmd without a shell. Set shell:true on win32 only so Linux/macOS
runs avoid Node's DEP0190 warning. All spawn args are literal strings,
so the shell variant is not an injection risk.

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

---------

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

* chore(admin): gitignore generated schema/version, regen on every script (#7638)

Qodo flagged the committed admin/src/api/schema.d.ts as a build artifact
that violates the rule against committing generated files (rule 467291,
"Exclude build artifacts and runtime-generated files from version control").

This commit:
- Adds admin/src/api/{schema.d.ts,version.ts} to .gitignore.
- Removes both from VCS (the prior squash had committed them).
- Chains \`gen:api\` into the dev and test scripts so a fresh checkout
  lands a working dev server / test run without an extra step. build
  and build-copy already chained gen:api.
- Drops the now-redundant CI freshness diff step from
  frontend-admin-tests.yml — with the files no longer committed, the
  build step's gen:api invocation is the only check needed.
- Updates admin/README.md to describe the new generated-file workflow.

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

* docs(admin): design for admin OpenAPI coverage (#7693)

Adds the design doc for documenting `/admin-auth/*` and `/admin/update/status`
in the OpenAPI spec so the typed client generated by #7695 (`admin/src/api/
schema.d.ts`) gains admin call-sites the day it lands.

This PR is stacked on #7695 — codegen rails must merge first. Schema-only,
no call-site migrations (those are an explicit follow-up named in #7693).

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

* docs(admin): correct UpdateStatus schema to base-branch shape (#7693)

The first draft mirrored the Tier 2 (#7607) response shape, but this PR
stacks on #7695 whose updateStatus.ts only emits the Tier 1 fields. Fix
the schema, install-method enum, and tier enum to match types.ts and
the actual handler. Tier 2 amends UpdateStatus when it lands.

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

* docs(admin): implementation plan for admin OpenAPI coverage (#7693)

Step-by-step task breakdown for the schema-only PR: stub the document,
add /admin-auth/ + /admin/update/status paths and sub-schemas,
collision regression, /admin/openapi.json route, codegen merge, and CI
verification. No call-site migrations.

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

* feat(admin): stub OpenAPI document for admin endpoints (#7693)

Adds generateAdminDefinition() returning a minimal valid OpenAPI 3.0
document with no paths yet, plus security schemes for the two auth
modes (Basic + session cookie). Subsequent tasks fill in the actual
admin paths.

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

* feat(admin): document POST /admin-auth/ in OpenAPI (#7693)

Adds verifyAdminAccess as the operation that the admin UI's LoginScreen
and App session check both call. Documents Basic auth, session cookie,
and anonymous request modes plus their 200/401/403 responses.

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

* feat(admin): document GET /admin/update/status in OpenAPI (#7693)

Adds getUpdateStatus operation plus UpdateStatus, ReleaseInfo,
PolicyResult, and VulnerableBelowDirective sub-schemas. Property names
and enums mirror src/node/updater/types.ts and the response object
emitted by updateStatus.ts. Tier 2 (#7607) will amend UpdateStatus when
it ships execution/lastResult/lockHeld.

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

* test(admin): regression net for admin/public OpenAPI collisions (#7693)

Cross-checks admin paths, operationIds, and schema names against the
latest public spec. Today there are no overlaps; the test exists to
catch future renames before they break the merged client codegen.

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

* feat(admin): expose admin OpenAPI doc at /admin/openapi.json (#7693)

Mounts the admin OpenAPI document at /admin/openapi.json (CORS: *) via
an expressPreSession hook, matching the /api/openapi.json convention.
The admin SPA wildcard at /admin/{*filename} registers later in
expressCreateServer, so the JSON route wins. Live-route test confirms
JSON content-type and CORS header.

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

* feat(admin): mergeOpenAPI helper for codegen pipeline (#7693)

Pure-JS deep-merge of two OpenAPI 3.0 documents. Unions paths and
components by key; throws on collisions. Public document's info,
servers, and root security win over the admin document's. Used by
dump-spec.ts to produce a single merged JSON for openapi-typescript.

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

* docs(admin): fix stale "Section 3" reference in spec (#7693)

Drafting numbered the design walkthrough; final spec uses titled
sections. Update the back-reference accordingly.

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

* feat(admin): include admin OpenAPI in generated client (#7693)

Modifies dump-spec.ts to import generateAdminDefinition alongside the
public generator and feed both through mergeOpenAPI before writing the
JSON consumed by openapi-typescript. The resulting admin/src/api/
schema.d.ts paths interface now exposes /admin-auth/ and
/admin/update/status, ready for typed call-site adoption in a follow-up.

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

* fix(admin): gate /admin/openapi.json behind a feature flag (#7693)

Address Qodo finding 1: new features must ship behind a flag, disabled
by default (CONTRIBUTING.md, AGENTS.MD, best_practices.md). Adds
settings.adminOpenAPI.enabled (default false). expressPreSession
returns early when the flag is off, so the route is dormant on a fresh
install.

The codegen pipeline imports generateAdminDefinition() in-process and
does not depend on the runtime route, so default-off has no effect on
the typed client. Operators who want third-party tooling (Postman,
swagger-ui, downstream clients) to consume the spec at runtime opt in
via settings.

Adds:
- SettingsType + defaults entry in src/node/utils/Settings.ts
- settings.json.template documentation
- A backend spec asserting expressPreSession is a no-op when the flag
  is off (live route test now sets the flag explicitly)

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

* fix(admin): split fetchClient by surface; admin client baseUrl='/' (#7693)

Address Qodo finding 2 (correctness bug). The merged schema introduced
in this PR exposes both public-API paths (under /api/<version>/) and
admin paths (at root, e.g. /admin-auth/). The single fetchClient from
#7695 has baseUrl=API_BASE_URL ("/api/<version>"), so calling an admin
path through it would resolve to /api/<version>/admin-auth/ — wrong.

Fix:
- Narrow the generated `paths` interface by URL prefix
  (`/admin*` vs everything else).
- Export two clients: `fetchClient` (public, baseUrl=/api/<version>)
  and `adminFetchClient` (admin, baseUrl='/').
- Mirror with `$api` / `$adminApi` query hook factories.

TypeScript now rejects mixing surfaces at compile time:
  fetchClient.GET('/admin-auth/')        // type error: not in PublicPaths
  adminFetchClient.GET('/createGroup')   // type error: not in AdminPaths

Smoke test updated to assert all four exports exist.

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

* docs(admin): spec — feature flag + dual-client design (#7693)

Reflect Qodo-driven changes:
- /admin/openapi.json route is now feature-flagged (default off).
- admin/src/api/client.ts splits paths by URL prefix and exports two
  clients (public + admin) so TypeScript rejects mixing surfaces.

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

* fix(admin): check adminOpenAPI flag per-request, not at hook setup (#7693)

The first feature-flag commit (dbcfb3902) gated the route by checking
the flag inside expressPreSession and skipping app.get() when off. That
broke under the full backend suite because common.init() is cached: the
first spec that calls it boots the server with whatever flag state was
present at that moment, and later specs cannot retroactively register
the route by toggling the flag.

Move the flag check inside the request handler:

- Route is always registered.
- Handler returns 404 application/json when the flag is off (so callers
  get a clear "feature disabled" signal rather than the SPA wildcard's
  text/html catch-all).
- Handler returns the spec + CORS * when the flag is on.

Tests now toggle the flag in-process and exercise both states against
the shared agent — no server restart needed. Added a "returns 404 JSON
when off" test alongside the existing "200 + spec + CORS when on".

Full backend suite: 1007 passing, 6 pending, 0 failures.

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-10 15:55:33 +08:00
John McLear
c47ffd5705
feat(export): native DOCX export via html-to-docx (opt-in) (#7568) 
* feat(export): native DOCX export via html-to-docx (opt-in)

Addresses #7538. The current DOCX export path shells out to LibreOffice,
which means every deployment that wants a Word download either installs
soffice (~500 MB) or loses that export. This PR adds a pure-JS
alternative: render the HTML via the existing exporthtml pipeline, then
feed it to the `html-to-docx` library in-process to produce a valid
.docx buffer — no soffice required, no subprocess spawn, no temp file
dance for the DOCX case.

Behavior:
- `settings.nativeDocxExport` (default `false`) gates the new path so
  existing deployments see zero behavior change.
- When enabled, `type === 'docx'` requests skip the LibreOffice branch,
  run `html-to-docx(html)`, and return the buffer with the
  `application/vnd.openxmlformats-officedocument.wordprocessingml.document`
  content-type.
- If the native converter throws, the handler falls through to the
  existing LibreOffice path — so flipping the flag on is safe even on a
  mixed-installation where soffice is still present as a backstop.
- Other export formats (pdf, odt, rtf, txt, html, etherpad) are
  unchanged.

Files:
- `src/package.json`: `html-to-docx` dep (pure JS, no binary reqs)
- `src/node/handler/ExportHandler.ts`: new DOCX branch gated on the
  setting, with fall-through on error
- `src/node/utils/Settings.ts`, `settings.json.template`,
  `settings.json.docker`, `doc/docker.md`: wire up the new setting +
  env var (`NATIVE_DOCX_EXPORT`)
- `src/tests/backend/specs/export.ts`: two new tests — asserts the
  exported buffer is a valid ZIP (PK\x03\x04 signature) and the
  response carries the correct content-type — both with
  `settings.soffice = 'false'` to prove the path doesn't need soffice
  at all.

Out of scope for this PR:
- Native PDF export (would need a PDF rendering step — separate
  undertaking, and the issue acknowledges the `pdfkit`/puppeteer size
  trade-off).

Closes #7538

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

* test(7538): skip native DOCX test when html-to-docx isn't installed

The upgrade-from-latest-release CI job installs deps from the previous
release's package.json (before this PR adds html-to-docx) and then
git-checkouts this branch's code without re-running pnpm install.
Under that one workflow the new test can't find the module and fails
on the LibreOffice fallback, masking that the native path actually
works in every normal install.

Guard the describe block with require.resolve('html-to-docx'); Mocha's
this.skip() on before cascades to the sibling its. Regular backend
tests (pnpm install against this branch's lockfile) still exercise it.

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

* docs(7538): spec for soffice-free DOCX/PDF export and DOCX import

Captures the agreed scope expansion of PR #7568: replace the flag-gated
native DOCX path with a soffice-first selection cascade, add native PDF
export via pdfkit + a small htmlparser2-driven walker, and add native
DOCX import via mammoth. Also defines a shared HTML sanitizer
(stripRemoteImages) used by both export converters to close the
SSRF surface that Qodo flagged on the html-to-docx path.

The spec drops the nativeDocxExport setting and its env var; with
soffice configured, behavior is unchanged, and with soffice null,
docx/pdf export and docx import all work in-process. odt/doc/rtf
(and pdf import) keep needing soffice and are documented as such.

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

* docs(7538): implementation plan for native DOCX/PDF and DOCX import

Bite-sized TDD task breakdown of the soffice-free export/import work:
rebase, deps, sanitizer, PDF walker, mammoth wrapper, ExportHandler
cascade, route guard, ImportHandler branch, UI fix, flag rollback,
verification + Qodo reply.

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

* chore(7538): add pdfkit, htmlparser2, mammoth deps

Pure-JS, no native binaries:
- pdfkit ^0.18.0  (PDF rendering)
- htmlparser2 ^12 (SAX parser used by walker + sanitizer)
- mammoth ^1.12   (DOCX -> HTML for native import)
- @types/pdfkit ^0.17 (dev)

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

* feat(7538): add stripRemoteImages HTML sanitizer

Drops <img src=> elements pointing at non-data, non-relative URLs to
prevent the DOCX/PDF converters from making outbound requests via
plugin-modified HTML. Closes Qodo finding #4 against the
html-to-docx path; will be wired into both export branches in
the cascade refactor.

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

* feat(7538): native PDF export via pdfkit + htmlparser2 walker

Renders pad HTML to a PDF Buffer in-process: headings, paragraphs,
lists, links, inline emphasis, data:-URI images. Remote images are
explicitly skipped at the walker (defense-in-depth on top of the
shared stripRemoteImages sanitizer).

PDFs are emitted with compress:false so accessibility/SEO indexers
that don't FlateDecode can still extract text. Pads are small enough
that the size cost is negligible.

Walker is 167 LOC, well under the spec's 500-LOC bail-out
threshold for switching to pdfmake+html-to-pdfmake+jsdom.

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

* feat(7538): native DOCX import via mammoth

Wraps mammoth.convertToHtml so a soffice-less Etherpad can ingest
.docx files. Images are coerced to data: URIs at the converter
boundary so the import pipeline never sees a remote src=.

Includes a tiny generated DOCX fixture (heading, paragraph, list)
under tests/backend/specs/fixtures/ for both this wrapper test and
the upcoming end-to-end import test.

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

* feat(7538): soffice-first cascade in ExportHandler

Replaces the flag-gated DOCX branch with a deterministic dispatch:
soffice if configured, native DOCX/PDF otherwise, 5xx on native
error. Both native paths run plugin-modified HTML through
stripRemoteImages first.

Test changes:
- existing native DOCX block now sets soffice=null (was 'false', a
  truthy non-null string that sidestepped the route guard); fixes
  Qodo finding #3.
- new native PDF integration tests assert %PDF- header and
  application/pdf content-type with soffice=null.
- new negative test: with soffice=null, /export/odt still returns
  the 'not enabled' message.
- the legacy 500-on-export-error test now uses /bin/false so it
  exercises the soffice error path explicitly (the cascade dropped
  the ad-hoc 'false' string; .doc has no native path so this still
  works as a soffice error probe).

Integration tests for native DOCX/PDF currently fail because the
/export route guard still treats both formats as soffice-only;
the next commit fixes that.

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

* fix(7538): allow docx/pdf through export guard without soffice

Tightens the no-soffice block to ['odt','doc'] only — formats with
no native path. docx and pdf are handed to ExportHandler, which
dispatches to the in-process converters. Closes Qodo finding #2.

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

* feat(7538): native DOCX import path in ImportHandler

When soffice is null and the upload is .docx, run mammoth and feed
the resulting HTML through setPadHTML. Other office formats
(pdf/odt/doc/rtf) are explicitly rejected with uploadFailed instead
of silently falling through to the ASCII-only path.

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

* fix(7538): always show DOCX/PDF export links

Native paths (#7538) make DOCX and PDF available regardless of
soffice presence, so unconditionally render those links. ODT still
gates on exportAvailable. Closes Qodo finding #2 on the UI side.

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

* refactor(7538): drop nativeDocxExport flag

Selection is now purely soffice-presence-driven (cascade in
ExportHandler). The opt-in setting and its NATIVE_DOCX_EXPORT env
var are no longer needed -- soffice configured means soffice path;
soffice null means native path (DOCX, PDF, and DOCX import).
Reverts the additive surface introduced earlier in this PR.

Also updates the SOFFICE doc row to reflect that null no longer
means 'plain text and HTML only' -- docx/pdf export and docx
import now work natively without soffice.

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

* test(7538): tighten link annotation assertion

CodeQL flagged the loose 'raw.includes("etherpad.org")' as
'incomplete URL substring sanitization' (a false positive in test
context, but worth fixing). Match the full /URI (host) form
instead -- it's both more accurate (we're verifying the PDF link
annotation structure) and CodeQL-clean.

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

* fix(7538): cleaner DOCX/PDF output + round-trip test coverage

DOCX:
- New extractBody helper drops <head>/<style> and the leading
  newline inside <body> so html-to-docx doesn't render CSS or
  prefix paragraphs with empty space.
- New wrapLooseLines pre-processor wraps loose pad lines in <p>
  before the converter sees them. html-to-docx renders <br>
  outside <p> as a new <w:p> (full empty line in Word); inside
  <p> it correctly emits <w:br/> (soft break). Etherpad's HTML
  uses bare <br> for every line, so this was making single
  Enters look like double Enters in the Word output.

PDF:
- Walker SKIP_TAGS rejects head/style/script/title/meta/link
  content -- prior version dumped CSS into the rendered PDF.
- New breakLine() helper combines flushLine() with moveDown(1).
  pdfkit's text('', false) closes the continued run but does
  NOT advance the cursor, so consecutive runs were stacking at
  the same y-coordinate. <br>, end-of-block, and list items
  now use breakLine().
- ontext collapses runs of whitespace and drops pure-whitespace
  text nodes so pretty-printed source HTML doesn't render its
  formatting newlines.

Round-trip:
- New backend test: pad text -> DOCX export -> DOCX import ->
  new pad. Asserts content survives the trip.
- New PDF sanity test: extracts visible text from the PDF stream
  and asserts the source pad text appears verbatim.
- 6 new unit tests for extractBody and wrapLooseLines plus 1 for
  PDF walker SKIP_TAGS coverage.

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

* fix(7538): CodeQL ReDoS + import.ts type error

- BR_PARA_RE was /(?:\s*<br>\s*){2,}/ -- two adjacent \s* runs can
  match the same chars, so on '<br>\t<br>\t<br>...' the regex
  backtracks exponentially. Re-anchored to match a fixed first <br>
  followed by one or more additional <br>s, so each whitespace run
  has exactly one home.
- import.ts: fetchBuffer was typed Promise<Buffer> but call sites
  chained .expect(200) on it, which only works on supertest's Test
  object. Return the Test (typed any) so the chain is preserved.

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

* fix(7538): plugin-aware HTML cleanup, PDF text-align, monospace

ep_headings2/ep_align emit one heading-styled blank-line block after
every styled line in the pad ('<h1 style=text-align:right></h1>'),
which both html-to-docx and our pdfkit walker render as a full empty
paragraph. Plus the pdfkit walker had no support for text-align or
monospace, so right/center alignment and 'code' lines rendered the
same as plain body text.

- New dropEmptyBlocks helper strips empty h1-h6/p/code/pre/div/
  blockquote wrappers in preprocessing. Iterates so nested empties
  collapse too. Applied before both DOCX and PDF conversion.
- PDF walker now reads style='text-align:left|center|right|justify'
  on block elements (h1-6, p, div) and passes it as pdfkit's align
  option. align is applied once per continued run, then reset on
  flushLine so the next block can pick up its own value.
- PDF walker handles <code>, <tt>, <kbd>, <samp> as inline monospace
  (Courier) and <pre> as block monospace (Courier + breakLine on
  open/close).

11 new unit tests:
- 4 for dropEmptyBlocks (heading wrappers, code, nesting,
  pass-through)
- 1 for PDF text-align (compares the BT matrix x for left vs right)
- 2 for Courier in <code> and <pre>

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

* fix(7538): separate adjacent heading-style blocks on import

Round-trip bug: ep_headings2 emits <h1>/<h2>/<code> in pad HTML.
Mammoth round-trips them as adjacent <h1>A</h1><h2>B</h2><p>C</p>
on import. Etherpad's server-side content collector has a default
_blockElems set of just {div, p, pre, li}, and ep_headings2 only
registers the CLIENT-side aceRegisterBlockElements -- not the
server-side ccRegisterBlockElements. So h1/h2/code end up being
treated as inline by the importer, and adjacent blocks merge into
a single pad line.

Fix: insert <br> after </h1>...</h6>/</code> when followed by
another block. Server-side workaround keeps this PR self-contained
regardless of plugin version. The right long-term fix is to extend
ep_plugin_helpers' lineAttribute factory to register both hooks
(filed as a follow-up).

Tests:
- 5 unit tests for separateAdjacentHeadingBlocks
- New end-to-end round-trip test asserts H1+H2+P land on three
  separate pad lines after the import path.

Plus the prior PDF text-align/Courier/code commit also included
here:
- code/tt/kbd/samp inherit text-align from style attribute
- pre inherits text-align too

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

* fix(7538): blank-line round-trip + DOCX code monospace + a==c tests

DOCX round-trip dropping blank pad lines:
- wrapLooseLines now emits an explicit <p></p> marker for each blank
  line in a <br> run, instead of collapsing all gaps into a single
  paragraph break. (N consecutive <br>s -> 1 paragraph boundary +
  N-2 empty <p></p> markers, mapping to N-1 blank pad lines.)
- mammoth's docxBufferToHtml now passes ignoreEmptyParagraphs:false
  so the empty <w:p> entries survive the import side. mammoth's
  default of true was silently dropping them.
- dropEmptyBlocks no longer strips <p></p> -- that's the meaningful
  marker for the round-trip. Empty <h1>/<code>/<pre>/<div>/
  <blockquote> are still stripped (plugin noise).

DOCX <code> rendering as monospace:
- New applyMonospaceToCode wraps code/tt/kbd/samp/pre content in a
  <span style="font-family:'Courier New', monospace">. html-to-docx
  honors that and emits <w:rFonts w:ascii="Courier New".../>, which
  Word renders as Courier. The bare <code> tag is otherwise just
  a no-op for html-to-docx.
- Applied only on the DOCX export path (PDF walker already handles
  monospace via Courier font selection).

Round-trip tests:
- New a==c suite: txt, etherpad, html, docx -- export from src,
  import to dst, re-export and compare against the meaningful
  invariant (line text for binary formats; trimmed body for HTML).
- HTML test tolerates one trailing <br> per round-trip because
  setPadHTML appends a final <p> on import; this is pre-existing
  core behavior, not our bug.
- DOCX test normalizes trailing newline run (same reason).

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

* fix(7538): preserve <w:jc> alignment through mammoth round-trip

mammoth doesn't expose Word's paragraph alignment (`<w:jc>`) when it
converts a docx to HTML -- there's no equivalent in its style-mapping
machinery. To keep alignment through DOCX round-trips we walk the
docx's document.xml directly, pull the `w:val` from each `<w:p>`'s
`<w:jc>`, and inject `style="text-align:..."` onto the matching
block element in mammoth's output by document order.

Word's w:jc accepts more values than CSS text-align; we map left/
start, center, right/end, both/justify/distribute and skip the rest
(start/end take left/right because we don't track ltr/rtl from the
docx for now).

Combines with the upstream ep_align PR (ether/ep_align#183) for the
full round-trip: this PR makes the mammoth output carry the
alignment style; ep_align#183 makes the importer pick it up.

Closes the alignment side of #7538.

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

* fix(7538): preserve <a href> inside <code>/<pre> in DOCX export

html-to-docx silently drops <a href> children of <code>/<pre> tags
(and of styled <span>s, but the <code> wrapper is the active offender
here). The pad-export HTML produced by ep_headings2 + ep_align uses
<code style='text-align:right'>...<a href>...</a>...</code> for each
'Code'-style line, which lost its links on every DOCX export.

Workaround: applyMonospaceToCode now drops the code/pre/tt/kbd/samp
wrapper entirely. The non-anchor content gets wrapped in monospace
spans; anchors are emitted unstyled so they keep their hyperlink. For
block-level usage (<pre>, or <code> with an inline style attr) we
emit a wrapping <p> and forward the text-align style. Run BEFORE
wrapLooseLines so the <p> doesn't get double-wrapped.

Tests added:
- inline <code> -> just a styled span (no <code> wrapper)
- <code style='text-align:right'> -> <p style> wrap
- <pre> -> always block-wrapped
- <tt>/<kbd>/<samp> -> inline span only
- regression: <a href> inside <code> survives html-to-docx round-trip
  with both the URL in word/_rels/document.xml.rels AND a <w:hyperlink>
  in the document body

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

* fix(7538): HTML import line-break drift between blocks

Etherpad's HTML export wraps each pad line in <p>...</p> (or <h1>,
<code>, etc.) and then appends a <br> between lines. The closing
block tag already ends the line for contentcollector, so the trailing
<br> is redundant -- and on import the server collector counts BOTH
as line breaks, doubling every blank line between paragraphs and
inserting an extra blank between adjacent headings.

Two fixes, both gated on the runtime block-element registry so they
don't double-trigger when the underlying plugin already handles
adjacency:

1. HTML import path now runs the new collapseRedundantBrAfterBlocks
   helper before setPadHTML. Drops a single <br> immediately
   following </p>/</h1-6>/</code>/</pre>/</div>/</blockquote>/</ul>/
   </ol>/</li>/</table>/</tr>/</td>/</th>. Multiple consecutive
   <br>s after a block keep all but the first (the rest still
   represent intentional blank lines).

2. The DOCX-import separateAdjacentHeadingBlocks workaround now
   checks whether 'h1' is in the runtime ccRegisterBlockElements
   set before inserting <br>s. When ep_headings2 has the new server
   hook (per ep_plugin_helpers#14 + the upcoming ep_headings2 PR),
   the workaround correctly stays out of the way -- otherwise it
   adds an extra blank line per heading transition.

Also fixed a subtle ts-check failure on the import.ts test changes
and a leftover implicit-any in ImportDocxNative's alignment
preserver.

Tests added:
- collapseRedundantBrAfterBlocks: 5 unit tests (each block tag,
  whitespace tolerance, multiple <br> keeping intentional blanks)
- HTML import: 'does not introduce a blank line between H1 and H2',
  'preserves blank-line count between H1 and H2 (realistic shape)'
  reproduces the 5-blanks-where-2-expected bug from the user's
  round-trip pad.

1054 backend tests pass locally (the 6 failures are the pre-existing
favicon/webaccess send@1.x dotfile-path issue from running under
.claude/, doesn't reach CI).

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

* test(7538): skip plugin-dependent HTML import tests on no-plugin CI

The two new HTML-import-adjacency tests assume ep_headings2 (or
another plugin) has registered h1/h2 as server-side block elements
via ccRegisterBlockElements. Without that, contentcollector treats
<h1>/<h2> as inline and adjacent ones merge into a single pad line
-- making the assertions inapplicable.

CI's backend-tests job runs without plugins installed, so guard
the describe block with a runtime hooks.callAll() check and skip
when h1 isn't a registered block. Local dev with ep_headings2 (and
the local plugin patch wiring ccRegisterBlockElements) still
exercises both tests.

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-08 18:33:50 +01:00
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
a356414254
fix(socialMeta): coerced numeric/boolean override silently dropped (#7692) 
* fix(socialMeta): coerced numeric/boolean override silently dropped

Qodo flagged on PR #7691: Settings.coerceValue() turns numeric-looking env
vars into numbers and "true"/"false" into booleans, so e.g.
SOCIAL_META_DESCRIPTION="2026" arrives at the resolver as the number 2026.
The previous resolver gated on `typeof override === 'string'`, so it
silently fell back to the i18n catalog with no warning — the operator's
docker config would appear broken.

Accept string|number|boolean and stringify before the empty-check; null /
undefined / unsupported types still fall through to the catalog. Two new
unit specs cover the numeric and boolean coercion paths.

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

* fix(socialMeta): widen description type to match coerced runtime

Action Qodo bug-find: SettingsType.socialMeta.description and
SocialMetaSettings.description still claimed `string | null`, but
Settings.coerceValue() can produce number|boolean from env-var-driven
config — the previous resolver fix was correct at runtime but the type
mismatch forced `as unknown as string` casts in the new tests.

Widen both declared types to `string | number | boolean | null` to match
runtime reality, drop the typeof string|number|boolean guards in the
resolver (the union now narrows automatically) and remove the test casts.
Behaviour is unchanged.

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 18:38:17 +08:00
John McLear
90aafb115e
feat(socialMeta): settings.socialMeta.description override (#7599 follow-up) (#7691) 
Issue #7599 follow-up from @stffen: the OG description has no obvious
settings.json knob and the i18n catalog default is English, but most
preview crawlers (WhatsApp, Signal, Slack, Telegram, Facebook) don't
send Accept-Language and so always hit the English fallback regardless
of how many locale files translate `pad.social.description`.

Keep the i18n catalog as the default source — translatable strings
belong in locale files, per the original Qodo review on PR #7635 — but
add an explicit `socialMeta.description` setting that wins when set as
a non-empty string, regardless of negotiated language. This is the
lever that fixes the crawler case for non-English instances without
re-introducing per-language config in settings.json (operators who
want that still use customLocaleStrings).

- Empty/whitespace overrides are treated as unset (would otherwise
  silently blank the preview).
- Override is HTML-escaped via the same path as every other value.
- og:locale stays language-negotiated; only the description is forced.
- Documented next to publicURL in settings.json.template and
  settings.json.docker (env var SOCIAL_META_DESCRIPTION). The
  customLocaleStrings example now spells out pad.social.description so
  operators discover both routes.

5 new unit specs + 4 new integration specs cover override-wins,
null/missing fallback, blank-treated-as-unset, and HTML-escaping.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
 2026-05-07 18:12:23 +08:00
John McLear
ab0cff4d95
fix(7606): sync theme-color meta with client-side dark-mode switch (#7690) 
* fix(7606): sync theme-color meta with client-side dark-mode switch

PR #7636 emits <meta name="theme-color"> server-side from
settings.skinVariants. That covers operators who hard-code a dark
toolbar in settings.json, but not the runtime path: pad.ts auto-flips
the toolbar to super-dark when enableDarkMode is on, the browser
reports prefers-color-scheme: dark, and no localStorage white-mode
override is set, plus the user can flip it via #options-darkmode.
Both paths run skinVariants.updateSkinVariantsClasses(), which until
now never touched the meta — so dark-mode users kept the light
#ffffff baseline and saw a white address bar above a dark toolbar
(stffen on #7606 after 2.7.3).

Push the toolbar-color lookup into updateSkinVariantsClasses so the
meta tracks every class change: the auto-switch on init, the user
toggle, and the skinVariants builder. Mirrors the CSS-source-order
table from src/node/utils/SkinColors.ts (last matching *-toolbar
token wins). When no meta is present (non-colibris skin, server
omits it) the helper is a no-op.

Adds Playwright coverage for both paths under
colorScheme: 'light' (manual toggle) and 'dark' (auto-switch on
dark-OS clients — the case stffen reported).

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

* fix(theme-color): action Qodo PR review

(1) Bug — duplicated toolbar→color table: extract the CSS-source-order
mapping and the default-color constant into src/static/js/skin_toolbar_colors,
re-imported by both src/node/utils/SkinColors.ts (server, EJS template
helper) and src/static/js/skin_variants.ts (client, runtime updates). Lives
under static/js so the browser bundle can resolve it; server-side imports
of static/js modules already exist (Changeset, AttributeMap, ImportHtml,
hooks). One source of truth means a future palette change can no longer
silently desync the server-rendered baseline meta from the client updates,
which was the exact regression that brought us here.

(2) Rule violation — 4-space continuation indentation in the new
Playwright spec: re-indent the themeColor helper and the multiline
test(...) call to the repo's 2-space rule (.editorconfig).

Existing backend coverage (configuredToolbarColor unit tests + the
specialpages server-render checks for both pad and timeslider) still
passes against the refactored helper, so it's regression-locked end to
end.

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 10:43:29 +01:00
John McLear
b751fd7b9e
feat(settings): enable Pad-wide Settings by default; fix misleading modal title (#7679) 
* feat(settings): enable Pad-wide Settings by default; fix misleading modal title

The creator-owned Pad-wide Settings feature (#7545) shipped behind a flag that
defaulted to false. With the flag off the modal still rendered an H1 of
`pad.settings.padSettings` ("Pad-wide Settings") for *every* user, even though
no pad-wide controls were ever shown. Two readers in different browsers both
saw "Pad-wide Settings" as the modal title, which looked like a creator-gate
regression but was just a copy bug.

Two changes:

1. Flip the default of `enablePadWideSettings` to `true` (Settings.ts plus
   both settings templates). With the feature on, the creator (revision-0
   author) gets a real "Pad-wide Settings" section gated by
   `clientVars.canEditPadSettings`, while every other user sees only "User
   Settings" — matching the design intent of #7545. This is a behavior change,
   so the settings comments are expanded to describe what the toggle now does.

2. Drop the conditional H1 in `src/templates/pad.html` and always use
   `pad.settings.title` ("Settings"). Operators who explicitly disable the
   feature shouldn't see a label that lies about a section that isn't
   rendered.

Adds backend regression coverage in `tests/backend/specs/socketio.ts`:
- Different browsers (different cookie jars => different authorIDs): only the
  first joiner gets `canEditPadSettings: true`.
- Same browser, two tabs (shared HttpOnly token cookie => same authorID):
  both connections are the same identity, both correctly land on the creator
  path.

* test(settings): regression coverage for the settings modal H1

Asserts the rendered `/p/<id>` HTML always uses
`data-l10n-id="pad.settings.title"` for the modal heading, regardless of
`enablePadWideSettings`. Catches a re-introduction of the old conditional
that printed "Pad-wide Settings" for every user when the feature was off.

Action of Qodo review feedback on PR #7679.
 2026-05-07 10:01:04 +01:00
John McLear
da87a4fca2
fix(7686): username 'false' / 'malformed color: false' for legacy settings.json (#7688) 
* fix(7686): legacy padOptions.userName/userColor=false breaks pad

Settings.json files generated before December 2021 used `false` as the
default for these two string options (commit 8c857a85a switched the
template default to `null` and noted "this change has no effect due to
a bug in how pad options are processed; that bug will be fixed in a
future commit" — the follow-up never landed). pad.ts:getParams() then
runs `false.toString()`, the resulting string "false" passes the
`!== false` sentinel check at _afterHandshake, and notifyChangeName
ships USERINFO_UPDATE with name="false" and colorId="false" (clobbered
via clientVars.userColor). The server's hex regex rejects the colour
and throws `malformed color: false`; the user sees their name as
"false" and a white swatch.

Defense in depth:
- Server: Settings.ts::reloadSettings() coerces legacy boolean false
  to null for padOptions.userName / padOptions.userColor and warns the
  operator, matching the existing disableIPlogging shim pattern.
- Client: the pad.ts userName / userColor callbacks reject the
  literal "false" string so URL params (?userName=false) and any
  other path that surfaces the sentinel as a string are also no-ops.
- Backend regression test mirrors the shim and asserts it normalizes
  legacy false, leaves explicit values intact, leaves null untouched,
  does not coerce other padOptions keys, and does not coerce the
  string "false" (that path is the client guard's responsibility).

Closes #7686

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

* fix(7686): guard padOptions shim against non-object config (Qodo)

Qodo flagged that storeSettings() will overwrite settings.padOptions
raw with whatever settings.json supplies — including null, primitives,
or arrays — which would make the new userName/userColor shim crash on
property access. Add a shape guard so the shim is a no-op for malformed
padOptions, and extend the regression test to cover null / primitive /
array shapes.

This doesn't change which configs work (Pad.ts also assumes padOptions
is an object and would already crash on a null padOptions when a pad
is opened) but it stops the shim from being the loud thing in the
stack trace if someone hits it.

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 09:32:17 +01:00
SamTV12345
bfdbd2bb91
chore: removed axios (#7685) 
* chore: removed axios

* chore: pnpm
 2026-05-06 22:06:04 +02:00
John McLear
70415714e6
fix(socketio): don't kick authenticated duplicate-author sessions (#7656) (#7678) 
* fix(socketio): don't kick authenticated duplicate-author sessions (#7656)

The CLIENT_READY handler kicks any prior socket whose authorID matches the
joining socket's, originally as a workaround for stale tabs in the same
browser (cookie-derived authorIDs were per-browser, so "same authorID, same
pad" reliably meant "page refresh / second tab in this browser").

With stable identities (basic auth, SSO, apikey, getAuthorId hook) the same
authorID can legitimately appear across windows or devices, so the kick
disconnects real concurrent sessions. Skip the kick when the joining socket
has req.session.user set; cookie-only sessions keep the existing behavior so
the userdup modal and the xxauto_reconnect path still work.

* fix(socketio): suppress USER_LEAVE when other same-author sockets remain

With the duplicate-author kick disabled for authenticated sessions, a single
authorID can legitimately span multiple sockets in one pad. handleDisconnect
was emitting USER_LEAVE on every socket close, which made clients (whose
presence is keyed by authorID) drop the author entirely even when another
socket of theirs was still online.

Only broadcast USER_LEAVE — and only run the userLeave hook — when the
disconnecting socket is the last one in the pad for that author.

Adds two backend tests:
- authenticated identity: closing one of two same-author sockets does NOT
  emit USER_LEAVE on the other.
- different authors (regression): closing socket A still emits USER_LEAVE
  for socket B.

Action of Qodo review feedback on PR #7678.
 2026-05-06 20:08:36 +02:00
John McLear
69bb1e19c5
feat(gdpr): author erasure (PR5 of #6701) (#7550) 
* docs: PR5 GDPR author erasure design spec

* docs: PR5 GDPR author erasure implementation plan

* feat(gdpr): AuthorManager.anonymizeAuthor — Art. 17 erasure

* test(gdpr): AuthorManager.anonymizeAuthor unit tests

* feat(gdpr): REST anonymizeAuthor on API version 1.3.1

* test(gdpr): REST anonymizeAuthor end-to-end

* docs(gdpr): right-to-erasure section + anonymizeAuthor example

* fix(gdpr): make anonymizeAuthor resumable on partial failure

Qodo review: the `erased: true` sentinel was written before the chat
scrub loop, so a throw during scrub left chat messages untouched
while subsequent calls short-circuited on `existing.erased` and never
finished. Split the write: zero the display identity first (still
hides the name), run the chat scrub, and only then stamp
`erased: true` so a retry resumes the sweep. Regression test
covers the partial-run → retry path.

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-03 12:30:49 +01:00
John McLear
487842006c
feat(gdpr): configurable privacy banner (PR4 of #6701) (#7549) 
* docs: PR4 GDPR privacy banner design spec

* docs: PR4 GDPR privacy banner implementation plan

* feat(gdpr): typed privacyBanner setting block + public getter exposure

* feat(gdpr): send privacyBanner config to the browser via clientVars

* feat(gdpr): privacy banner DOM (hidden by default)

* feat(gdpr): render privacy banner on pad load when enabled

* style(gdpr): privacy banner layout

* test+fix(gdpr): privacy banner Playwright + hidden-attr CSS override

* docs(gdpr): privacyBanner configuration section

* fix(gdpr): reject unsafe learnMoreUrl schemes

Qodo review: showPrivacyBannerIfEnabled assigned config.learnMoreUrl
directly to <a href>, so a misconfigured settings.privacyBanner.
learnMoreUrl of `javascript:alert(1)` or `data:…<script>…` would run
script on click. Validate via URL parsing and allow only http(s) /
mailto; everything else yields no link. Playwright regression guards
the four cases (javascript, data, https, mailto).

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

* fix(privacy-banner): drop unneeded !important on [hidden] rule

Class+attribute selector already outranks `.privacy-banner { display: flex }`
on specificity (0,2,0 vs 0,1,0), so `!important` was redundant. Adds a
comment explaining why so a future reader doesn't put it back.

Per Sam's review on #7549.

* refactor(privacy-banner): render as a persistent gritter, not custom DOM

Drops the bespoke #privacy-banner template + ~50 lines of popup.css and
delegates to $.gritter.add({sticky: true, position: 'bottom'}). The
notice now matches every other gritter on the pad (theme variables,
shadow, animation, (X) close), sits in the bottom corner instead of
above the editor, and inherits dark-mode handling for free.

The two dismissal modes survive intact:
  - dismissible: gritter closes on (X); before_close persists a flag
    in localStorage so the notice is suppressed on subsequent loads.
  - sticky: closes for the current session only; never persists; the
    next pad load shows it again.

learnMoreUrl still goes through the same safeUrl() filter so a
javascript:/data:/vbscript: URL can't smuggle a script handler into the
anchor (Qodo's review concern remains addressed).

Tests: src/tests/frontend-new/specs/privacy_banner.spec.ts now drives
the real showPrivacyBannerIfEnabled via a __etherpad_privacyBanner__
test hook and asserts against the rendered gritter, instead of the
previous tests that mutated DOM by hand and never exercised the
function under test. Coverage adds: enabled=false short-circuit,
dismissible-flag-respected on subsequent show, sticky-ignores-flag,
sticky-close-does-not-persist, javascript: rejection, data: rejection,
and mailto: allow-list.

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

* fix(privacy-banner): noreferrer + validate dismissal (Qodo)

Two follow-ups from Qodo's review on #7549:

1. The Learn-more link now sets `rel="noreferrer noopener"` (was just
   `noopener`). Without `noreferrer` the browser sends the pad URL as a
   Referer to the operator-configured external policy site, which leaks
   pad identifiers to a third party.  Matches the rel pattern already
   used by pad_utils.ts.

2. `privacyBanner.dismissal` is now validated in reloadSettings(): an
   unknown value falls back to 'dismissible' with a `logger.warn`, in
   the same shape as the existing ipLogging validation a few lines up.
   The client also guards defensively (treats anything other than the
   exact string 'sticky' as 'dismissible') so that hot-reload paths
   that skip the server validator can't silently degrade a typo'd
   'sticky' into "no close button persisted, no localStorage suppression".

Test added: spec asserts the rel attribute, and a new test exercises
the dismissal fallback (sets dismissal:'wat', asserts the gritter is
shown, the (X) closes it, and the dismissal flag is persisted — i.e.
the unknown value is treated like 'dismissible').

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

* fix(privacy-banner): gate test hook on webdriver, align doc with sticky behavior

Two follow-ups from Qodo's second review on #7549.

Rule violation: __etherpad_privacyBanner__ was published on every pad
load even when privacyBanner.enabled was false, so the disabled-by-
default feature still added an observable global. Gate the assignment
on `navigator.webdriver` — Playwright/ChromeDriver/Selenium set this
to true; production browsers do not — so the hook is only present for
tests and the disabled path is genuinely zero-side-effect.

Bug 3 (sticky still closable): doc/privacy.md previously claimed
`dismissal: "sticky"` removes the close button, but the gritter
implementation always renders (X). Aligning the doc with reality —
sticky now means "shows on every load, but closable for the session"
— rather than adding bespoke CSS to a vanilla gritter (matches the
"don't style it differently than other gritter messages" preference
that drove the gritter migration in 906e145).

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

* fix(privacy-banner): allow-list keys before sending to clientVars (Qodo)

storeSettings() merges nested objects with _.defaults() and preserves
unknown nested keys, and TypeScript's Pick<> doesn't strip at runtime.
The previous wire path forwarded settings.privacyBanner by reference
into both clientVars and getPublicSettings(), so any extra keys an
operator typed (or pasted) under privacyBanner — credentials, internal
notes, anything — would have shipped to every browser on every pad
load.

Adds getPublicPrivacyBanner() in Settings.ts that returns a literal
with only {enabled, title, body, learnMoreUrl, dismissal}, and uses it
from both leak sites (PadMessageHandler.ts clientVars and
getPublicSettings()). Single source of truth for the wire shape.

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-03 13:59:38 +08:00
John McLear
49bc33f019
feat(gdpr): HttpOnly author-token cookie (PR3 of #6701) (#7548) 
* docs: PR3 GDPR anonymous identity hardening design spec

* docs: PR3 GDPR anon identity implementation plan

* feat(gdpr): ensureAuthorTokenCookie helper — HttpOnly server-set author token

* feat(gdpr): set HttpOnly author-token cookie from the pad routes

* feat(gdpr): read author token from cookie first, keep message.token fallback

* feat(gdpr): stop generating the author token client-side

* test(gdpr): server sets + reuses the HttpOnly author-token cookie

* fix+test(gdpr): parse token cookie from handshake Cookie header

socket.io handshake doesn't run cookie-parser, so socket.request.cookies
is undefined. Parse the Cookie header directly in handleClientReady so
the HttpOnly token actually resolves. Playwright spec covers HttpOnly
attribute, reload-stability, and context-isolation.

* docs(gdpr): token cookie is now HttpOnly + server-set

* fix(gdpr): close two HttpOnly token bypasses

Qodo review:
- Timeslider still ran the pre-PR3 JS-cookie path: it read
  Cookies.get('${cp}token') (which HttpOnly hides), then generated a
  fresh plaintext token and overwrote the server's HttpOnly cookie with
  it, and sent token in every socket message. Strip the token read/
  write entirely from timeslider.ts and from the outgoing message
  shape; the server reads the cookie off the socket.io handshake just
  like on /p/:pad.
- tokenTransfer re-issued the author cookie without HttpOnly, undoing
  the hardening the first time a user transferred a session. Re-set
  it as HttpOnly + Secure (on HTTPS) + SameSite=Lax. Also stop
  trusting the body-supplied token on POST: read it off req.cookies
  server-side so the client never needs JS access to the token.

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-03 05:56:56 +01:00
John McLear
9014d3a7c4
fix(colors): pick WCAG-higher-contrast text for author colors (#7565) 
* feat(colors): clamp author backgrounds to WCAG 2.1 AA on render

Fixes #7377.

Authors can pick any color via the color picker, so a user who chooses
a dark red ends up with black text rendered on a background that fails
WCAG 2.1 AA (4.5:1) — unreadable, but there is no way for *viewers* to
remediate since they cannot change another author's color. Screenshot
in the issue shows exactly this.

This PR lands a viewer-side clamp. For each author background, if
neither black nor white text would satisfy the target contrast ratio,
the bg is iteratively blended toward white until black text does. The
author's stored color is untouched — turning off the new
padOptions.enforceReadableAuthorColors flag restores the raw colors
immediately.

New helpers in src/static/js/colorutils.ts:

- relativeLuminance(triple)  — WCAG 2.1 relative-luminance formula
- contrastRatio(c1, c2)      — in [1, 21]; >=4.5 = AA, >=7.0 = AAA
- ensureReadableBackground(hex, minContrast = 4.5)
                             — returns a hex that meets minContrast
                               against black text, preserving hue

Wire-up:

- src/static/js/ace2_inner.ts (setAuthorStyle): pass bgcolor through
  ensureReadableBackground before picking text color. Gated on
  padOptions.enforceReadableAuthorColors (default true). Guarded by
  colorutils.isCssHex so the few non-hex values (CSS vars, etc.) skip
  the clamp and pass through unchanged.
- Settings.ts / settings.json.template / settings.json.docker: new
  padOptions.enforceReadableAuthorColors flag, default true, with a
  matching PAD_OPTIONS_ENFORCE_READABLE_AUTHOR_COLORS env var in the
  docker template.
- doc/docker.md: env-var row.
- src/tests/backend/specs/colorutils.ts: new unit coverage for the
  three new helpers, including the exact #cc0000 failure case from
  the issue screenshot.

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

* refactor(7377): simplify — just pick higher-contrast text, drop bg clamp

First iteration added an iterative bg-lightening helper
(ensureReadableBackground) gated by a new padOptions flag. CI caught the
correct simpler framing: because WCAG contrast is symmetric in [1, 21],
at least one of black/white always clears AA (4.5:1) for any sRGB
colour. The real bug was that the pre-fix textColorFromBackgroundColor
used a plain-luminosity cutoff (< 0.5 → white), which produced
sub-AA combinations like white-on-red (#ff0000) at 4.0:1.

Reduce the PR to the minimal surface:

- colorutils.textColorFromBackgroundColor now picks whichever of
  black/white has the higher WCAG contrast ratio against the bg.
- colorutils.relativeLuminance and colorutils.contrastRatio are kept
  as reusable building blocks; ensureReadableBackground is dropped
  (no caller needed it once text selection was fixed).
- ace2_inner.ts setAuthorStyle no longer needs the opt-in flag or the
  isCssHex guard — the helper handles every input its caller already
  passes.
- padOptions.enforceReadableAuthorColors setting reverted along with
  settings.json.template, settings.json.docker, and doc/docker.md.
- Tests replaced: instead of asserting the bg gets lightened, assert
  that the chosen text colour clears AA for every primary. Covers the
  exact #ff0000 failure case from the issue screenshot.

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

* test(7377): assert relative-contrast invariant, not absolute AA

Pure primaries like #ff0000 cannot clear WCAG AA (4.5:1) against either
#222 or #fff — the best either can do is ~4.0:1. No text-colour choice
alone fixes that; bg clamping would be a separate concern. The test
should therefore verify the *real* invariant: the chosen text colour
must produce the higher contrast of the two options, regardless of
whether that contrast clears any absolute threshold.

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

* fix(7377): compare against rendered #222/#fff, not pure black/white

First cut of textColorFromBackgroundColor computed contrast against
pure black (L=0) and pure white (L=1), then returned the concrete
#222/#fff the pad actually renders with. For some mid-saturation
backgrounds the two comparisons disagreed — e.g. #ff0000:
  vs pure black = 5.25 → pick black → render #222 → actual 3.98
  vs pure white = 4.00 → would-render #fff → actual 4.00
The helper picked the wrong option because it compared against the
wrong target. Compare against the actual rendered colours so the
returned text colour is genuinely the higher-contrast choice.

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

* test(7377): pick unambiguous colibris test bgs

#ff0000 lives right at the boundary for the two text choices (4.00 vs
3.98), so the test for colibris-skin mapping was entangled with the
border-case selector pick. Use #ffeedd (clearly light → dark text
wins) and #111111 (clearly dark → light text wins) so the test
isolates the skin mapping from the tie-breaking logic.

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

* fix(7377): use rendered text colour + clamp bg to actually meet AA

Local repro of the issue exposed two real bugs in the previous fix:

1. textColorFromBackgroundColor compared bg against a hardcoded #222 —
   but in the colibris skin --super-dark-color resolves to #485365.
   For the issue's exact case (#9AB3FA author bg) the selector returned
   var(--super-dark-color) thinking it was getting a 7.7:1 ratio, while
   the browser actually rendered 3.78:1 — identical to what the issue
   screenshot reported. This PR's previous behaviour on the issue's
   inputs was unchanged from the pre-fix.

2. For mid-saturation pastels (#9AB3FA) and pure primaries (#ff0000)
   neither rendered dark nor white text can clear AA. Text-colour
   selection alone genuinely cannot fix this band; the ensureReadable
   bg clamp dropped in ce0c5c283 was load-bearing.

Changes:

- colorutils.ts: per-skin SKIN_TEXT_COLORS table with darkRef/lightRef
  matching what the browser actually paints (colibris #485365,
  default #222). Re-introduces ensureReadableBackground, but skin-aware
  and symmetric — blends bg toward white or black depending on which
  text colour wins, so it works for both light and dark backgrounds.
- ace2_inner.ts: setAuthorStyle runs the bg through the clamp before
  picking text colour. Gated on padOptions.enforceReadableAuthorColors
  (default true).
- Settings.ts / settings.json.template / settings.json.docker /
  doc/docker.md: padOption + PAD_OPTIONS_ENFORCE_READABLE_AUTHOR_COLORS
  env var.
- tests: failing-then-green coverage for the issue's exact case
  (#9AB3FA + colibris), the previously-impossible #ff0000, the
  no-mutation case, non-hex pass-through, and a sweep over primaries.

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

* test(7377): add e2e DOM-contrast spec + extra unit cases

The previous coverage was unit-only, which is what let the original wrong-
reference-colour bug ship — the algorithm tests were green but nothing
exercised what the browser actually paints. New coverage:

Playwright (src/tests/frontend-new/specs/wcag_author_color.spec.ts):
- Sets the user's colour to the issue's exact #9AB3FA, types text, reads
  the rendered author span's computed bg + colour from the inner frame,
  and asserts the WCAG ratio between the two is >= 4.5. Repeated for
  #ff0000 (the other historically-failing case).
- Asserts #ffeedd (already AA-friendly) is rendered unchanged — guards
  against the clamp mutating colours that don't need it.

Backend additions (src/tests/backend/specs/colorutils.ts):
- Symmetric-clamp test: dark mid-saturation bg where light text wins, the
  clamp must darken (not lighten). Direction check via relativeLuminance.
- minContrast parameter: AAA (7.0) must produce more clamping than AA.
- Output shape: result must be a parseable hex string (round-trip safe).
- Short-hex (#abc) input is accepted and normalised.

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-03 12:56:30 +08:00
John McLear
e0a989094d
feat(colors): add padOptions.fadeInactiveAuthorColors with toolbar UI (#7554) 
Adds a new pad option `fadeInactiveAuthorColors` (default `true`) that controls whether each author's caret/background fades toward white as they go inactive. Configurable server-side (`settings.json` / `PAD_OPTIONS_FADE_INACTIVE_AUTHOR_COLORS`), per-pad in the Pad Settings panel, per-user in the My View panel, or via `?fadeInactiveAuthorColors=false`.

Disabling the fade is useful on busy pads where every faded author visually counts as a second on-screen color (a 30-author pad becomes a 60-color pad), or when inactivity tracking is undesirable for whatever reason.

Closes #7138.
 2026-05-03 04:01:13 +08:00
John McLear
e028016296
feat(admin): surface ep.json disables in /admin plugin browser (#7649) 
Companion to ether/ether.github.com#395 — the admin UI's "available
plugins" listing now also renders the plugin's declared `disables`
(see doc/PLUGIN_FEATURE_DISABLES.md) so an operator about to click
Install sees the same warning as a user browsing etherpad.org/plugins:
"Disables: chat".

- src/node/types/PackageInfo.ts: optional `disables?: string[]` on
  the registry payload type.
- admin/src/pages/Plugin.ts: same on the admin-side PluginDef.
- admin/src/pages/HomePage.tsx: render an amber callout under the
  description when `disables` is present and non-empty. Plugins
  without a disables field render unchanged.

The plugin-registry build pipeline still has to start surfacing
`disables` from ep.json into plugins.json/plugins.viewer.json — until
that lands, the new callout no-ops everywhere, which is fine.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
 2026-05-02 10:29:25 +01:00
John McLear
4bda757304
feat(api): public compactPad API + bin/compactPad CLI over existing Cleanup (#7567) 
* feat(pad): compactHistory() + compactPad CLI for DB-size reclaim

Fixes #6194. Long-lived pads with heavy edit history dominate the DB —
the issue describes a ~400 MB Postgres after two months with ~100
users. Etherpad keeps every revision forever, and removing arbitrary
middle revisions is unsafe because state is reconstructed by composing
forward from key revisions.

What's safe: collapse the full history into a single base revision
that reproduces the current atext. The existing `copyPadWithoutHistory`
already does this for a new pad ID — this PR lifts that same changeset
pattern into an in-place operation and wires up an admin CLI.

- `Pad.compactHistory(authorId?)` (src/node/db/Pad.ts): composes the
  current atext into one base changeset, deletes all existing rev
  records, clears saved-revision bookmarks, and appends the new rev 0.
  Text, attributes, and chat history are preserved; saved-revision
  pointers are cleared. Returns the number of revisions removed.
- `API.compactPad(padID, authorId?)` (src/node/db/API.ts): public-API
  wrapper around compactHistory. Reports `{removed}` so callers can
  log savings.
- `APIHandler.ts`: register `compactPad` under a new `1.3.1` version,
  bump `latestApiVersion`.
- `bin/compactPad.ts`: admin CLI. Reports the current revision count,
  calls compactPad via the HTTP API, and prints how many revisions
  were dropped.
- `src/tests/backend/specs/compactPad.ts`: four backend tests cover
  the empty-pad no-op, the text-preservation + head=0 contract,
  saved-revision cleanup, and that subsequent edits continue to
  append cleanly on top of the collapsed base.

The operation is destructive so admins must opt in explicitly; the CLI
prints the before-count, and the recommended pre-flight is an
`.etherpad` export (backup).

Closes #6194

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

* fix(compact): delegate to copyPadWithoutHistory via temp-pad swap

The initial compactHistory() implementation built a custom base
changeset and re-ran appendRevision against a reset atext — but the
changeset was packed with oldLength=2 (matching copyPadWithoutHistory's
dest-pad init state) while the reset atext was only length 1, so
applyToText tripped its "mismatched apply: 1 / 2" assertion and every
test failed with a Changeset corruption error.

Switch to the tested path instead: copy the pad via
copyPadWithoutHistory to a uniquely-named temp pad (inherits all its
attribute/pool/changeset correctness), read the temp pad's rev records
back, delete the old ones under our pad's ID, write the new records in
their place, update in-memory state to match, and remove the temp pad.
Errors at any step fall through with a best-effort temp-pad cleanup.

Contract shifts slightly: the collapsed pad is head<=1 rather than
head=0, matching the shape of a freshly-imported pad (seed rev 0 +
content rev 1). Tests updated to assert that invariant plus
text-preservation, saved-revision cleanup, and append-after-compact.

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

* test(6194): match the head<=1 post-compact contract

Tests previously asserted head=0 exactly after compaction; the
temp-pad-swap path lands at head=1 (one seed rev plus one content
rev) matching the shape of a freshly-imported pad. Relax the
assertions to  and derive the removed-count from
before-head minus after-head, so the tests still catch regressions in
text-preservation, saved-revision cleanup, and append-after-compact
without being tied to the exact implementation shape.

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

* refactor(6194): wrap existing Cleanup instead of duplicating it

Develop already ships a working revision-cleanup path under
`src/node/utils/Cleanup.ts` with two public helpers —
`deleteAllRevisions(padId)` (collapse full history via
copyPadWithoutHistory) and `deleteRevisions(padId, keepRevisions)`
(keep the last N). The admin-settings UI wires these up but neither
is exposed on the public API, and there's no CLI for operators who
want to run compaction outside the web UI. That's the gap this PR
now fills.

Changes from the prior revision of this PR:

- Drop `pad.compactHistory()` — it re-implemented what
  `Cleanup.deleteAllRevisions` already does. Remove the duplicate.
- `API.compactPad(padID, keepRevisions?)` now delegates to Cleanup:
    • keepRevisions null/undefined → deleteAllRevisions (full collapse)
    • keepRevisions >= 0          → deleteRevisions(N)  (keep last N)
  Returns {ok, mode: 'all' | 'keepLast', keepRevisions?}.
- APIHandler `1.3.1`: signature updated to take `keepRevisions`
  instead of `authorId`.
- `bin/compactPad.ts`: accepts `--keep N` for the keep-last mode,
  shows before/after revision counts so operators see concrete
  savings.
- Backend tests rewritten around the public API surface (mode
  reporting, text preservation, input validation) rather than
  internal method plumbing that no longer exists.

Net: strictly a thin public-API and CLI veneer over already-tested
Cleanup helpers. No new low-level logic.

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

* test(6194): assert content markers, not byte-exact atext

Cleanup.deleteAllRevisions internally calls copyPadWithoutHistory
twice (src → tempId, tempId → src with force=true), and each round
trip normalizes trailing whitespace. That meant my byte-exact
atext.text assertion failed in CI:
  expected: '...line 3\n\n\n'
  actual:   '...line 3\n'

Swap the comparisons to use content markers (marker-alpha / beta /
gamma, keep-line-N). The test still catches the real regressions —
if compactPad lost content those markers would disappear — without
coupling to whitespace quirks of the existing Cleanup implementation.

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

* fix(6194): correct API param + document compactPad in http_api docs

The 1.3.1 entry in APIHandler registered `['padID', 'authorId']`, but
`API.compactPad` takes `(padID, keepRevisions)` and the CLI sends a
`keepRevisions` query param. APIHandler.handle dispatches by URL field
name, so the previous wiring silently dropped `keepRevisions` and never
ran the keep-last branch over HTTP.

- Register `['padID', 'keepRevisions']` so the handler forwards the
  CLI/HTTP arg into the API function.
- Add HTTP-level dispatch tests that hit `/api/1.3.1/compactPad` with
  and without `keepRevisions`. The direct `api.compactPad()` tests
  bypass the handler and would have missed this regression.
- Document compactPad in `doc/api/http_api.md` and `http_api.adoc`,
  and bump the documented latest version from 1.3.0 to 1.3.1 to match
  `latestApiVersion`.

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

* feat(6194): add bin/compactAllPads for per-instance bulk compaction

`bin/compactPad <padID>` covers the case where you know which pad is
fat. For "reclaim space across the whole instance," composing
`listAllPads` + `compactPad` yourself is annoying; this script does it.

- Walks every pad on the instance and compacts it (full collapse, or
  `--keep N` keep-last).
- Per-pad failures don't abort the run — they're logged, counted, and
  the script exits 1 if any failed.
- `--dry-run` lists pads + revision counts without writing anything,
  so operators can scope impact before committing.
- Reports `before → after` per pad and a total reclaimed count.

Deliberately not adding a `compactAllPads` HTTP API: bulk compaction
over a single HTTP request means one giant response and a long-held
connection. Operators who want this should run it locally, where they
can see progress and kill it cleanly. Staleness gating ("only pads
older than X days") is tracked separately as a follow-up.

Also registers `compactPad` and `compactAllPads` script aliases in
`bin/package.json` so they show up next to the other admin CLIs.

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

* test(6194): cover the bin/compactAllPads loop logic

Previous commit added the script but only exercised it by hand. The
loop itself — error tolerance, dry-run gating, keep-last passthrough,
the empty-instance and listAllPads-failure paths — had no automated
coverage.

- Refactor compactAllPads.ts to export `runCompactAll(api, opts, logger)`
  and `parseArgs(argv)`. The CLI shell wires them up to axios+APIKEY
  for production; tests use an in-memory `CompactAllApi` so we don't
  need to stand up the apikey-auth path in mocha.
- Add 9 specs covering: arg parsing, full-collapse iteration,
  --keep N passthrough, --dry-run skipping writes, single-pad failure
  not aborting the run, pre-flight count failure tolerated, a
  listAllPads failure short-circuiting cleanly, the empty-instance
  no-op, and a final end-to-end test that runs `runCompactAll`
  against the real `/api/1.3.1/compactPad` handler over supertest+JWT
  to catch contract drift between the CompactAllApi shape and the
  HTTP endpoints.

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

* fix(6194): address Qodo review — gate, integer check, SSL

Three valid concerns from the Qodo review on 75a08a13:

1. **cleanup.enabled gate.** The admin/Cleanup-socket path checks
   `settings.cleanup.enabled` before doing anything destructive; the
   public API was bypassing that gate. Now `compactPad` mirrors the
   admin path's check and returns a clear apierror when disabled, so
   exposing the API doesn't accidentally widen the cleanup-opt-in
   surface.

2. **Number.isFinite → Number.isInteger.** `2.5` was finite and
   non-negative, so the old check let it through into
   `Cleanup.deleteRevisions`, which does revision-index arithmetic
   that assumes integer math. Reject at the API boundary instead of
   silently misbehaving.

3. **SSL-aware baseURL in the bin scripts.** Other bin scripts
   hardcode `http://`, but the rest of the codebase uses
   `settings.ssl ? 'https' : 'http'`. The compact CLIs now do the
   same, so they work against HTTPS deployments. (Other bin scripts
   carry the same bug but fixing them is out of scope for this PR.)

Tests:
- New spec: `rejects fractional keepRevisions` (2.5 with the old
  check passed; the new one rejects).
- New spec: `refuses to run when cleanup.enabled is false`. The
  existing API tests opt in via a before-hook + restore, so they
  still cover the success path under the new gate.
- API docs (`http_api.md` + `http_api.adoc`) document the gate and
  the new error message.

Skipped Qodo concerns:
- "Wrong compactPad parameters" — already fixed in 26e12ff7
  (the param map now correctly says `keepRevisions`, not `authorId`).
- "Unbounded revision deletions" / "No session eviction" / changeset
  base-length / padCreate hook — these all targeted the earlier
  on-Pad implementation that was refactored away. The current code
  wraps `Cleanup.deleteAllRevisions` / `deleteRevisions`, which
  already handle concurrency, locking, and hook semantics.

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-01 14:23:54 +01:00
John McLear
5e8704f8d8
feat(gdpr): pad deletion controls (PR1 of #6701) (#7546) 
* docs: PR1 GDPR deletion-controls design spec

First of five GDPR PRs tracked in #6701. PR1 covers deletion controls:
one-time deletion token, allowPadDeletionByAllUsers flag, authorisation
matrix for handlePadDelete and the REST deletePad endpoint, a single
token-display modal for browser pad creators, and test coverage.

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

* docs: PR1 GDPR deletion-controls implementation plan

13 TDD-structured tasks covering PadDeletionManager unit tests, socket
+ REST three-way auth, clientVars wiring, one-time token modal,
delete-with-token UI, Playwright coverage, and PR handoff.

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

* feat(gdpr): scaffolding for pad deletion tokens

PadDeletionManager stores a sha256-hashed per-pad deletion token and
verifies it with timing-safe comparison. createPad / createGroupPad
return the plaintext token once on first creation, and Pad.remove()
cleans it up. Gated behind the new allowPadDeletionByAllUsers flag
which defaults to false to preserve existing behaviour.

Part of #6701 (GDPR PR1).

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

* fix+test(gdpr): lazy DB access in PadDeletionManager + unit tests

Capturing DB.db at module-load time was null until DB.init() ran, which
broke importing the module outside a live server (including from the
test runner). Switch to DB.db.* at call time and add unit tests
exercising create/verify/remove plus timing-safe comparison.

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

* feat(gdpr): three-way auth for socket PAD_DELETE

Creator cookie → valid deletion token → allowPadDeletionByAllUsers flag.
Anyone else still gets the existing refusal shout.

* feat(gdpr): optional deletionToken on programmatic deletePad

* feat(gdpr): advertise optional deletionToken on REST deletePad

* test(gdpr): cover deletePad authorisation matrix via REST

* feat(gdpr): surface padDeletionToken in clientVars for creators only

Revision-0 author on their first CLIENT_READY visit receives the
plaintext token; all subsequent CLIENT_READYs receive null because
createDeletionTokenIfAbsent is idempotent. Readonly sessions and any
other user never see the token.

* i18n(gdpr): strings for deletion-token modal and delete-with-token flow

* feat(gdpr): token modal + delete-with-token disclosure markup

* feat(gdpr): show deletion token once, allow delete via recovery token

* style(gdpr): modal + delete-with-token layout

* test(gdpr): Playwright coverage for deletion-token modal + delete-with-token

* fix(test): auto-dismiss deletion-token modal in goToNewPad helper

The token modal introduced in PR1 blocks clicks for every Playwright
test that creates a new pad via the shared helper. Add a one-line
dismissal so unrelated tests keep passing, and have the deletion-token
spec navigate inline via newPadKeepingModal() when it needs the modal
open to capture the token.

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

* fix(test): dismiss deletion-token modal without focus transfer

Clicking the ack button transferred focus out of the pad iframe, which
made subsequent keyboard-driven tests (Tab / Enter) silently miss the
editor. Swap the click for a page.evaluate() that hides the modal and
nulls clientVars.padDeletionToken directly, leaving focus where it was.

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

* fix(gdpr): PadDeletionManager race + document createPad/deletePad

Qodo review:
- createDeletionTokenIfAbsent() was a non-atomic read-then-write. Two
  concurrent callers for the same pad could both return different
  plaintext tokens while only the later hash was stored, leaving the
  first caller with an unusable recovery token. Serialise per-pad via a
  Promise chain and add a regression test that fires 8 concurrent
  calls and asserts exactly one plaintext is emitted and validates.
- doc/api/http_api.md now documents createPad returning deletionToken
  and deletePad accepting the optional deletionToken parameter.

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

* fix(gdpr): always render delete-with-token in settings popup

The rebase onto develop placed the delete-pad-with-token details inside
the pad-settings-section conditional, which is only rendered when
enablePadWideSettings is true AND the section is toggled visible.
Second-device recovery (typing the captured token on a fresh browser)
must work without pad-wide settings enabled, so move the details out
to sit alongside the existing pad_deletion_token.spec.ts expectations.

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

* fix(gdpr): require valid token when supplied, gate on auth, harden a11y/i18n

- PadMessageHandler: a supplied deletion token must validate; do not fall
  back to the creator-cookie path when the token is wrong (was deleting
  the pad anyway when the creator pasted a wrong token into the field).
- Skip token issuance + UI when requireAuthentication is on (creator
  identity is stable, recovery token is redundant noise).
- Server emits messageKey instead of hardcoded English; both shout
  handlers (inline alert and global gritter) localize via html10n.
- Suppress the global "Admin message" gritter for pad.deletionToken.*
  shouts to avoid the "Admin message: undefined" duplicate.
- Token-modal a11y: role=dialog, aria-modal, aria-labelledby/describedby,
  visually-hidden label on the token input, aria-live on Copy, focus to
  the token input on open and restore on dismiss.
- Style the "Delete Pad with Token" disclosure to match the Delete pad
  button; align the Copy/value row; pad the disclosure label.

Tests: Playwright now covers the creator-with-wrong-token path, asserts
no "Admin message" / "undefined" gritter on denial; backend API test
covers requireAuthentication suppressing the token.

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-01 13:50:04 +01:00
John McLear
6195289198
feat(gdpr): IP/privacy audit (PR2 of #6701) (#7547) 
* docs: PR2 GDPR IP/privacy audit design spec

Second of five GDPR PRs (#6701). Audit identifies four log-sites that
leak IPs despite disableIPlogging=true, proposes a tri-state ipLogging
setting with a back-compat shim, and specifies a doc/privacy.md that
documents Etherpad's actual IP handling.

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

* docs: PR2 GDPR IP/privacy audit implementation plan

7 TDD-structured tasks: anonymizeIp helper + unit tests, tri-state
ipLogging setting with disableIPlogging deprecation shim, wiring
through 5 leaking log sites, clientVars.clientIp removal, access-log
integration test, doc/privacy.md, and PR handoff.

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

* feat(gdpr): anonymizeIp helper with v4/v6/v4-mapped truncation

* feat(gdpr): tri-state ipLogging setting + disableIPlogging shim

* fix(gdpr): route every IP log site through anonymizeIp

Closes four leaks where disableIPlogging was silently ignored
(rate-limit warn, both auth-log calls in webaccess, import/export
rate-limit warn) and normalises the four that did honour the flag
onto the new ipLogging tri-state via the shared helper.

* chore(gdpr): drop dead clientVars.clientIp placeholder

Server side: remove the literal '127.0.0.1' assignments from both
clientVars and collab_client_vars. Type side: drop clientIp from
ClientVarPayload and ServerVar. pad.getClientIp now returns the same
'127.0.0.1' literal as a plugin-compat shim (pad_utils.uniqueId still
uses it as a prefix).

* test(gdpr): ipLogging modes + disableIPlogging shim

* docs(gdpr): operator-facing privacy and IP handling statement

* fix(gdpr): validate ipLogging at load + regression test for log sites

Qodo review:
- settings.ipLogging is loaded as a trusted union but nothing enforced
  the shape. An unknown value (e.g. a typo or null) silently fell
  through to anonymizeIp's "truncated" branch and emitted partially
  redacted IPs. Fall back to "anonymous" with a WARN at load time.
- New regression test scans the four known log-sites for raw
  req.ip / socket.request.ip / request.ip inside logger calls that
  don't wrap through anonymizeIp / logIp, so a future edit that
  re-introduces a raw IP fails CI.

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-01 13:47:40 +01:00
John McLear
e39dbde887
feat(updater): tier 1 — notify admin and pad users of available updates (#7601) 
* docs(updater): add four-tier auto-update design spec

Four-tier opt-in self-update subsystem (off / notify / manual / auto / autonomous).
GitHub Releases as source of truth; install-method auto-detection with admin
override; in-process execution with supervisor restart; 60s drain + announce;
auto-rollback on health-check failure with crash-loop guard. Pad-side severe/
vulnerable badge that does not leak the running version. Top-level adminEmail
with escalating cadence (weekly while vulnerable, monthly while severe).

Refs: docs/superpowers/specs/2026-04-25-auto-update-design.md

* docs(updater): add PR 1 (Tier 1 notify) implementation plan

Bite-sized TDD task breakdown for shipping Tier 1 notify only:
- VersionChecker, InstallMethodDetector, UpdatePolicy, Notifier, state modules
- /admin/update/status (admin-auth) and /api/version-status (public, no version leak)
- Admin UI banner + read-only update page + nav link
- Pad-side severe/vulnerable footer badge
- Settings: updates.* block + top-level adminEmail
- Tests: vitest unit + mocha integration + Playwright admin/pad
- CHANGELOG + doc/admin/updates.md

PRs 2-4 (manual/auto/autonomous) get their own plans after PR 1 lands.

* feat(updater): add shared types for auto-update subsystem

* feat(updater): clarify OutdatedLevel and EMPTY_STATE doc, drop path header

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat(updater): add semver helpers and vulnerable-below parser

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(updater): tighten semver regex to reject four-part versions

* feat(updater): add state persistence with schema validation

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(updater): reject null email and array latest in state validation

typeof null === 'object' meant {email:null} passed the old isValid check,
which would crash downstream Notifier code reading email.severeAt. Likewise,
an array would pass the typeof latest === 'object' branch. Introduce
isPlainObject helper (null-safe, Array.isArray guard) and use it for both
fields. Adds two regression tests covering the exact broken inputs.

* feat(updater): add install-method detector with override

* feat(updater): add policy evaluator

* feat(updater): add GitHub Releases checker with ETag support

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(updater): validate release fields and preserve ETag on prerelease

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat(updater): add email cadence decider

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(updater): tagChanged email fires regardless of cadence; drop unused field

* feat(settings): add updates.* and adminEmail settings

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat(updater): wire boot hook and periodic checker

Register expressCreateServer/shutdown hooks in ep.json and implement
the boot-wiring module that detects install method, starts the polling
interval and runs the notifier dedupe pass each tick.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat(updater): add /admin/update/status and /api/version-status endpoints

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* i18n(updater): add english strings for update banner, page, and pad badge

* feat(updater): add pad footer badge for severe/vulnerable status

* feat(admin-ui): add update banner, page, and nav link

Add UpdateStatusPayload to the zustand store, a persistent UpdateBanner
rendered in the App layout, a /update page showing version details and
changelog, and a Bell nav link — all wired to the /admin/update/status
endpoint added in Task 10.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* test(updater): add Playwright specs for admin banner/page and pad badge

* docs(updater): document tier 1 settings, badge, email cadence

* refactor(updater): dedupe helpers, fix misleading log, add banner styling

- Export stateFilePath from index.ts and import it in updateStatus.ts (removes local duplicate)
- Import getEpVersion from Settings.ts in both index.ts and updateStatus.ts (removes two local definitions)
- Fix misleading 'backing off' log message — no backoff is implemented, just retries at next interval
- Remove EMPTY_STATE_FOR_TESTS re-export from state.ts; state.test.ts now imports EMPTY_STATE directly from types.ts
- Add .update-banner and .update-page CSS rules to admin/src/index.css

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(updater): address review feedback — async wrap, tier=off skip, poll race, opt-in admin gate

- Wrap /api/version-status and /admin/update/status with a small async helper
  so a rejected promise becomes next(err) instead of an unhandled rejection.
- Short-circuit route registration when updates.tier === 'off' so the heavier
  opt-out also removes the HTTP surface (matches pre-PR behavior for that case).
- Add an in-flight guard around performCheck() so overlapping interval ticks
  can't race on update-state.json writes or duplicate email decisions; track
  the initial setTimeout handle and clear it in shutdown().
- Add updates.requireAdminForStatus (default false) so admins can lock
  /admin/update/status to authenticated admin sessions without disabling the
  updater. Default false preserves current behavior (the running version is
  already exposed publicly via /health). Backend specs cover unauth → 401,
  non-admin → 403, admin → 200.
- Bump admin troubleshooting menu count test 5 → 6 to account for the new
  Update nav link.

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

* fix(updater): address Qodo round-2 review feedback

Round 2 of Qodo review on #7601. Addressing the action-required items:

#1 Badge bypassed pad baseURL — derive basePath the same way
   padBootstrap.js does (`new URL('..', window.location.href).pathname`)
   and prefix the fetch with it. Subpath deployments now reach
   /<prefix>/api/version-status instead of 404ing.

#2 Updater poller could get stuck — `getCurrentState()` is now inside
   the try/finally so a one-time loadState() rejection can't leave
   `checkInFlight=true` and permanently silence polling.

#3 Updates off hung admin page — UpdatePage now self-fetches and
   renders explicit `disabled` (404), `unauthorized` (401/403), and
   `error` states instead of staying on "Loading...". Banner-driven
   prefetch is still honoured if it landed first.

#11 NaN polling interval — coerce `checkIntervalHours` to a number,
   clamp to [1h, 168h], log a warning and fall back to 6h on
   non-finite input. Math.max(1, NaN) === NaN previously meant a
   malformed settings.json could turn the poller into a tight loop.

#13 State validation accepted broken subfields — `isValid()` now
   inspects `latest.{version,tag,body,publishedAt,htmlUrl,prerelease}`,
   `vulnerableBelow[].{announcedBy,threshold}`, and
   `email.{severeAt,vulnerableAt,vulnerableNewReleaseTag}`. A
   hand-edited file with a number where a string is expected is now
   treated as corrupt and reset to EMPTY_STATE rather than crashing
   later in semver parsing or email rendering.

#14 Badge cache stampede — wrap `computeOutdated()` in a single-flight
   promise so concurrent requests at cache expiry await one shared
   computation instead of fanning out into N redundant disk reads.

Plus six new state.test.ts cases covering each new validation guard.

Pushing back on the remaining items:

#4 `updates.tier` defaults to `notify` — intentional. The whole point
   of tier 1 is to surface the "you are behind" signal to admins by
   default. Opt-in defeats the purpose; the existing failure mode
   (admin never hears about a security-relevant release) is exactly
   what this PR is fixing.

#5/#8 Admin status endpoint admin-auth — `currentVersion` is already
   public via `/health`, so wrapping the route in admin-auth doesn't
   reduce the disclosure surface meaningfully. Operators who want it
   gated set `updates.requireAdminForStatus=true` (already wired and
   covered by the comment on the route handler).

#10 Plain `https://` URLs in planning doc — planning markdown is
   viewed in editors and on GitHub where protocol-relative URLs would
   either render literally or break entirely. Keeping `https://`.

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

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
 2026-05-01 20:02:12 +08:00
John McLear
85f9a5f2f5
feat: Open Graph & Twitter Card metadata for pad/timeslider/home (closes #7599) (#7635) 
* docs(spec): Open Graph metadata for pad pages (issue #7599)

Spec for adding og:* and twitter:card meta tags to /p/:pad,
the timeslider, and the homepage so shared links unfurl with
a useful preview in chat apps.

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

* docs(spec): expand OG spec — i18n (locale map + og:locale) and a11y (image:alt)

Address review feedback: socialDescription accepts a per-language map,
og:locale is emitted from the negotiated render language, and image:alt
attributes are emitted for screen readers in chat clients.

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

* feat: emit Open Graph & Twitter Card metadata for pad/timeslider/home

Closes #7599.

Pad URLs shared in chat apps (WhatsApp, Signal, Slack, etc.) previously
unfurled with no preview because the rendered HTML carried no OG or
Twitter Card metadata. This change emits og:title, og:description,
og:image, og:url, og:site_name, og:type, og:locale, og:image:alt and
the equivalent twitter:* tags on the pad page, the timeslider, and the
homepage.

A new settings.json key `socialDescription` controls the description.
It accepts either a plain string applied to every locale or a per-language
map keyed by BCP-47 tag with an optional `default` fallback. og:locale
is emitted from the language already negotiated via req.acceptsLanguages
and og:image:alt provides screen-reader text for chat-client previews.

Pad names from the URL are HTML-escaped before being interpolated into
og:title to prevent reflected XSS via crafted pad IDs.

Tests: src/tests/backend/specs/socialMeta.ts covers the default,
per-locale override, locale fallback, URL decoding, XSS escape, and
the timeslider/homepage variants.

Semver: minor (new setting; templates emit additional tags but no
existing behavior changes).

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

* fix(test): use valid pad-name char in URL-decode test

Spaces aren't allowed in pad names — Etherpad redirected /p/Has%20Space*
to a sanitized name (302), so the og:title assertion failed. Use %2D
("-") instead, which is a valid pad-name character and still exercises
the URL-decode path.

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

* fix(socialMeta): don't double-decode pad name from req.params.pad

Express has already URL-decoded :pad route params before they reach the
handler. Calling decodeURIComponent on the result throws URIError for
pad names containing a literal "%" — e.g. the URL /p/100%25 yields
req.params.pad === "100%", and decodeURIComponent("100%") throws.

This would have prevented the page from rendering for some valid pad
IDs. Drop the redundant decode and add a regression test for the "%"
case.

Reported by Qodo on PR #7635.

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

* refactor(socialMeta): source description from i18n catalog, drop settings key

Per review: the OG description is a translatable string and belongs in
Etherpad's locale files alongside the rest of the UI strings, not in
settings.json. Operators who want to override it per-language continue to
use the standard customLocaleStrings mechanism — no new config surface.

Changes:
- Add "pad.social.description" to src/locales/en.json (default English).
- Export i18n.locales so server-side renderers can look up translations.
- socialMeta.renderSocialMeta now takes a `locales` map and resolves
  renderLang → primary subtag → en, instead of taking a per-locale map
  from settings.
- Remove `socialDescription` from Settings.ts, settings.json.template,
  settings.json.docker (the key never shipped).
- Update tests and spec doc to reflect i18n-sourced description.

Reported by Qodo on PR #7635 (also confirmed feature is fine to land
default-on; no flag needed).

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

* test(socialMeta): add unit tests for pure helpers

21 cases exercising buildSocialMetaHtml and renderSocialMeta directly,
without HTTP/DB. Covers tag enumeration, HTML escaping, og:locale
region formatting, title composition (pad/timeslider/home), description
i18n resolution (exact/primary/en fallback, missing catalog), image URL
(default favicon vs absolute settings.favicon vs alt text), canonical
URL building with query-string stripping, the literal "%" no-throw
regression, and attribute-breakout escape.

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

* fix(socialMeta): defend og:url/og:image against host-header poisoning

Previously og:url and og:image were built from req.protocol +
req.get('host'), both of which can be client-controlled (Host header
directly, or X-Forwarded-* under trust proxy). A crafted Host could
make the server emit OG tags pointing at an attacker's origin —
harmful if any cache fronts the response or if a vulnerable proxy
forwards the headers unsanitized.

Two-layer defense:

1. New optional setting `publicURL` lets operators pin the canonical
   origin used for shared link previews ("https://pad.example"). When
   set, og:url and og:image use it unconditionally. Sanitized at use
   time: must be http(s)://host[:port] with no path, no userinfo, no
   trailing slash; malformed values fall back to the request.

2. When `publicURL` is unset, the request-derived fallback now strictly
   validates the Host header against /^[a-z0-9]([a-z0-9.-]{0,253}[a-z0-9])?(:\d{1,5})?$/i
   and caps the scheme to "http"/"https". A crafted Host (CRLF
   injection, userinfo, "<script>") is replaced with "localhost"
   instead of being echoed into og:url.

Reported by Qodo on PR #7635.

Tests: 5 new unit cases covering publicURL preference, trailing-slash
strip, malformed-publicURL fallback, Host validation, scheme cap.

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

* refactor(socialMeta): tighten types, drop `any`

- `req: any` -> express `Request` (covers acceptsLanguages/protocol/get/originalUrl).
- `settings: any` -> local `SocialMetaSettings` interface narrowed to the three
  fields we actually read (title/favicon/publicURL); avoids coupling to the
  full Settings module surface.
- `availableLangs: {[k: string]: any}` -> `{[lang: string]: unknown}`; only
  keys are read, so values stay deliberately unconstrained.

No runtime change. All 26 socialMeta unit tests still pass.

Per Sam's review on #7635.

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
 2026-05-01 10:43:29 +01:00
John McLear
63cae17720
feat(pad): add theme-color meta to match toolbar on mobile (#7606) (#7636) 
* feat(pad): add <meta name="theme-color"> matching toolbar (#7606)

Mobile browsers paint the address-bar / status-bar area above the
viewport. Without theme-color this is a system color that does not
match the Etherpad toolbar, leaving a visible gap above the pad.

Render <meta name="theme-color"> server-side so the bar matches the
configured toolbar on first paint. Light + dark variants are emitted
with prefers-color-scheme media queries when dark mode is enabled.
Colors are derived from settings.skinVariants via a new SkinColors
helper (mirrors --bg-color in the colibris pad-variants.css).

Closes #7606

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

* fix(timeslider): emit single theme-color matching configured toolbar

Qodo flagged a mismatch: timeslider does not switch skin variants on
prefers-color-scheme, so emitting a dark theme-color via media query
would leave dark-mode devices with a dark address bar over a light
toolbar. Drop the media-query metas on timeslider and emit one
unconditional theme-color resolved from settings.skinVariants.

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

* fix(pad): emit unconditional theme-color so dark-OS users still match

Qodo flagged that gating the light theme-color on
prefers-color-scheme: light leaves no applicable meta on dark-OS
devices when enableDarkMode is false — the address bar then uses a
system color while the toolbar stays light.

Drop the light media query so the light theme-color is the baseline,
and let the prefers-color-scheme: dark meta override it when dark
mode is enabled.

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

* fix(theme-color): align dark meta with client-side super-dark override

Two related Qodo findings on the SkinColors helper:

- The pad client's dark-mode auto-switch (pad.ts L650) forces
  super-dark-toolbar regardless of the configured skinVariants, so
  the prefers-color-scheme: dark meta must always be #485365 — not
  whichever dark variant the operator configured.
- When skinVariants only carries a dark token (e.g. dark-toolbar),
  the previous helper left the baseline meta at #ffffff, so light-OS
  users would see white above a dark toolbar.

Replace toolbarThemeColors() with configuredToolbarColor() (used as
the unconditional baseline) and a fixed DARK_MODE_TOOLBAR_COLOR
constant (used in the prefers-color-scheme: dark meta).

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

* refactor(theme-color): server-side only, drop fragile dark media query

Address remaining Qodo findings on the theme-color rollout:

- (#1) Skip emitting the meta entirely when settings.skinName is not
  colibris — the helper only knows colibris's --bg-color values, so
  on no-skin or third-party skins the previous code would emit a
  white meta over a non-white toolbar.
- (#4) Drop the prefers-color-scheme: dark variant. The pad's
  client-side dark mode is also gated on a localStorage white-mode
  override that no media query can express, so the dark meta could
  paint a dark address bar over a still-light toolbar. The single
  baseline meta always matches what the user sees on first paint.
- (#8) Remove the redundant module.exports assignment; rely on the
  ES named export only (tsx handles the require() interop).
- (#9) Iterate the toolbar variants in CSS source order and let the
  last match win, matching the cascade in pad-variants.css when
  multiple *-toolbar tokens are present.

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-01 10:25:24 +01:00
John McLear
884ac93b4e
feat(editor): add IDE-style line ops (duplicate / delete) (#7564) 
* feat(editor): add IDE-style line ops (duplicate / delete)

Addresses #6433 — the issue asked for VS-Code-style multi-line editing
for collaborative markdown editing. Full multi-cursor support would need
a rep-model rewrite; this PR lands the two highest-value single-cursor
line ops now so users get the actual ergonomic wins without that lift:

- Ctrl/Cmd+Shift+D: duplicate the current line, or every line in a
  multi-line selection. Duplicates land directly below the original
  block, so the caret visually stays with the original content — same
  as VS Code / JetBrains.
- Ctrl/Cmd+Shift+K: delete the current line (or every line in a
  multi-line selection), collapsing the range including its trailing
  newline. Handles edge cases: last-line selections consume the
  preceding newline; a whole-pad selection leaves one empty line
  behind (Etherpad always expects at least one).

Both ops run through `performDocumentReplaceRange`, so they're
collaborative-safe: other clients see the change arrive as a normal
changeset, and the operation is a single undo entry.

Wire-up:
- `src/node/utils/Settings.ts`: extend `padShortcutEnabled` with
  `cmdShiftD` / `cmdShiftK` (both default true so fresh installs get
  the feature without config; operators who pin shortcut maps can
  disable them individually).
- `src/static/js/ace2_inner.ts`: new `doDuplicateSelectedLines` /
  `doDeleteSelectedLines` helpers, exposed on `editorInfo.ace_*` so
  plugins and tests can invoke them programmatically, and keyboard
  handlers for Ctrl/Cmd+Shift+D and Ctrl/Cmd+Shift+K.

Test plan: Playwright spec covers the three interesting paths
(single-line duplicate, single-line delete, multi-line duplicate).

Closes #6433

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

* test(6433): type the bodyLines helper parameter

* fix(6433): preserve char attributes on duplicate + correct whole-pad delete

Addresses Qodo review feedback on #7564:

1. `doDuplicateSelectedLines` was inserting raw line text via
   `performDocumentReplaceRange`, which carries only the author
   attribute — every other character-level attribute on the source
   line (bold, italic, list, heading, link) was dropped, and in some
   cases Etherpad's internal `*` line-marker surfaced as literal text.

   Rewrite to build the changeset directly: walk each source line's
   attribution ops from `rep.alines[i]`, split the line text at op
   boundaries, and call `builder.insert(segment, op.attribs)` once per
   op. Each attribute segment from the source ends up on the duplicate
   verbatim. Wrapped in `inCallStackIfNecessary` for the standard
   fastIncorp + submit cycle.

2. `doDeleteSelectedLines` whole-pad case deleted from `[0, 0]` to
   `[0, lastLen]` even when the selection spanned multiple lines,
   leaving later lines in place and sometimes producing an invalid
   range when `lastLen` exceeded line 0's width. Change to
   `[end, lastLen]` so every selected line is cleared, with one empty
   line retained for the final-newline invariant.

3. Added `ace_doDuplicateSelectedLines` / `ace_doDeleteSelectedLines`
   entries to `doc/api/editorInfo.md` so plugin authors can discover
   the new surface.

4. New Playwright spec asserting `<b>` tags survive duplication.

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

* revert(6433): drop the attributed-duplicate changeset, keep whole-pad delete fix

The attributed-changeset rewrite for doDuplicateSelectedLines tripped
over the insertion-past-final-newline edge case — CI caught the basic
single-line duplicate regressing (gamma → [alpha, beta, gamma] with no
new gamma appearing because the hand-rolled changeset ended up invalid
at the end-of-pad boundary). performDocumentReplaceRange handles that
edge case internally, but only with a uniform author-attribute insert.

Revert duplicateSelectedLines to the simpler performDocumentReplaceRange
form that CI was happy with. Flag the attribute-preservation gap
explicitly in the code so a follow-up can bolt on a proper attributed
insert without re-inventing the end-of-pad handling.

Whole-pad delete fix and editorInfo.md docs stay. Attribute-preservation
test in line_ops.spec.ts is removed along with the broken code.

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-04-29 09:21:05 +01:00
John McLear
0b40bfc784
feat(packaging): add Debian (.deb) build via nfpm with systemd unit (v2) (#7583) 
* feat(packaging): add Debian (.deb) build via nfpm with systemd unit

First-class Debian packaging for Etherpad, producing
etherpad_<version>_<arch>.deb artefacts for amd64 and arm64 from a
single nfpm manifest. Installing the package gives users:

- /opt/etherpad with a prebuilt, self-contained node_modules/ — no
  pnpm required at runtime, just `nodejs (>= 20)`.
- etherpad system user/group, created via `adduser` in preinst.
- /etc/etherpad/settings.json seeded from the template on first
  install, preserved across upgrades, removed on `purge`. Seed rewrites
  dbType from the template's dev-only `dirty` default to `sqlite`,
  pointed at /var/lib/etherpad/etherpad.db so fresh installs get an
  ACID-safe DB without manual config. sqlite is shipped by ueberdb2
  (rusty-store-kv), so no additional apt deps are needed.
- /var/lib/etherpad owned by etherpad:etherpad, writable under the
  hardened unit's ProtectSystem=strict.
- /lib/systemd/system/etherpad.service — hardened unit
  (NoNewPrivileges, ProtectSystem=strict, ProtectHome, PrivateTmp,
  RestrictAddressFamilies) with Restart=on-failure.
- /usr/bin/etherpad CLI wrapper running `node --import tsx/esm`.

CI (.github/workflows/deb-package.yml) triggers on v* tags, builds both
arches via native runners (ubuntu-latest + ubuntu-24.04-arm),
smoke-tests the amd64 package end-to-end (install → verify sqlite
default → systemctl start → curl /health → purge → confirm user
removed), and attaches the artefacts to the GitHub Release.

Re-introduces the work from #7559 (reverted in #7582) with two
corrections:

1. Package name and all installed paths use `etherpad`, not
   `etherpad-lite` — matches the repo rename. Kept replaces/conflicts
   on `etherpad-lite` so any dev builds of the reverted PR upgrade
   cleanly.
2. Default dbType is `sqlite`, not `dirty`. The template's own comment
   says dirty is for testing only; shipping it by default to everyone
   who runs `apt install etherpad` is the wrong tradeoff for a
   production package.

Publishing to an APT repo (Cloudsmith, Launchpad PPA, self-hosted
reprepro) is intentionally out of scope — needs a governance decision
on who holds the signing key. Recipes are documented in
packaging/README.md.

Refs #7529, #7559, #7582

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

* fix(packaging): address PR review — startup crashes, supply chain, Node LTS

Addresses Qodo and SamTV12345 review feedback on #7583:

- postinstall: symlink /opt/etherpad/var → /var/lib/etherpad/var so
  ProtectSystem=strict doesn't block runtime writes (var/js,
  installed_plugins.json, etc.). Existing ReadWritePaths covers it.
- postinstall: seed installed_plugins.json with ep_etherpad-lite so
  checkForMigration() does not spawn `pnpm ls` on first boot — pnpm is
  not a runtime dep, and the bundled node_modules already contains
  every shipped plugin. Prevents network plugin installs at first run.
- postremove: clean up the new var symlink on remove.
- workflow: verify nfpm .deb sha256 against upstream checksums.txt
  before sudo dpkg -i (defense in depth).
- workflow: bump Node 22 → 24 (current LTS, per SamTV12345). The deb
  Depends stays at nodejs (>= 20) to match Etherpad's engines.node.
- workflow: smoke-test now asserts the var symlink and seeded
  installed_plugins.json exist post-install.
- workflow: publish stable etherpad-latest_{amd64,arm64}.deb aliases
  alongside the versioned files in the GitHub Release.
- README: bump Node guidance to 24, document /releases/latest URL,
  link to engines.node floor.

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

* fix(packaging): tsx CJS hook, plugin paths writable, glob tag triggers

Addresses second-round Qodo review on #7583:

- bin/etherpad: switch from `--import tsx/.../esm` to `--require
  tsx/cjs`. server.ts uses `exports.start = ...` which throws under
  the ESM loader; the prod script in src/package.json uses tsx/cjs
  for the same reason.
- postinstall: symlink /opt/etherpad/src/plugin_packages →
  /var/lib/etherpad/plugin_packages and chgrp /opt/etherpad/src/node_modules
  to etherpad with mode 2775. Otherwise admin-UI plugin install
  EACCESes — those are the dirs LinkInstaller writes to.
- systemd unit: add /opt/etherpad/src/node_modules to ReadWritePaths
  so symlink creation by the etherpad user is allowed under
  ProtectSystem=strict. plugin_packages is already covered via the
  symlink into /var/lib/etherpad.
- postremove: clean up the new plugin_packages symlink on remove.
- workflow: tag filters were `v[0-9]+.[0-9]+.[0-9]+`, but Actions tag
  filters are globs, not regex. `[0-9]+` matches one character, so
  multi-digit tags like v2.10.0 would never trigger. Switch to
  `v*.*.*` / `v*.*.*-*`, matching handleRelease.yml.
- workflow smoke test now asserts plugin_packages symlink target,
  ownership of plugin_packages and node_modules.
- test-local.sh: new script that builds the .deb and runs the same
  smoke test in a throwaway systemd-enabled Docker container, so
  failures are caught before pushing.
- README: document test-local.sh.

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

* chore(packaging): test-local.sh — fix cgroups v2, add --no-systemd mode

- systemd-in-docker on cgroups v2 needs --cgroupns=host and a writable
  /sys/fs/cgroup mount; the previous :ro version booted to nothing.
- New --no-systemd mode: drops the systemd container in favour of plain
  ubuntu:24.04 + manual launch under the etherpad user. Validates the
  postinstall, wrapper, plugin paths, and /health without depending on
  the host's systemd-in-docker setup. Use it when --privileged systemd
  containers don't boot on your kernel/docker combo.
- On systemd container exit the script now dumps the last 50 log lines
  and points at --no-systemd as the fallback.

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

* chore(packaging): test-local.sh — reuse cached image in --no-systemd

If ubuntu:24.04 isn't on disk and the registry is unreachable, fall
back to whichever ubuntu/debian image is already cached (e.g. the
jrei/systemd-ubuntu image we pulled for the systemd path). Avoids a
registry round-trip on flaky networks.

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

* fix: handle spawn errors in run_cmd; deb-package install order + offline-safe test

src/node/utils/run_cmd.ts:
  Without `proc.on('error', ...)` a spawn failure (e.g. ENOENT for a
  missing binary) is emitted as an unlistened 'error' event, which
  Node treats as an uncaught exception that bypasses the awaiting
  try/catch and kills the process. The .deb hits this on first boot
  because plugins.ts spawns `pnpm --version` for a startup log line
  and pnpm isn't a runtime dep — Etherpad logs "Starting" then
  immediately stops. Reject the promise on 'error' so the existing
  try/catch in the caller actually catches it.

packaging/scripts/postinstall.sh:
  chown /var/lib/etherpad/plugin_packages AFTER `cp -a` from the
  staged tree — `cp -a` preserves source (root) ownership and was
  re-rooting the directory we'd just chowned to etherpad. Same
  ordering the var symlink block already used.

packaging/test-local.sh:
  Run `CI=1 pnpm install --frozen-lockfile` before staging so the
  package is built from a fresh, lockfile-consistent tree (matches
  CI). Fixes spurious "Cannot find module 'X'" failures from stale
  local symlinks pointing at out-of-date pnpm store paths.

End-to-end test now passes: postinstall asserts pass, /health
returns 200, dpkg --purge cleans up.

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

* chore: gitignore packaging build artefacts; drop accidental commit

Drop packaging/etc/settings.json.dist that snuck into the previous
commit (generated at build time by test-local.sh / CI from
settings.json.template). Add /staging/, /dist/, /packaging/etc/ to
.gitignore so they don't recur.

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

* fix(plugins): downgrade missing-pnpm log from ERROR to debug

The startup IIFE that logs the pnpm version is informational only.
pnpm is a dev-only dependency: admin-UI plugin install goes through
live-plugin-manager directly, and plugin migration is short-circuited
when var/installed_plugins.json is present (e.g. on packaged
installs). A missing pnpm on PATH is therefore expected on hardened
deployments and shouldn't surface as a red ERROR in journalctl.

Detect ENOENT specifically and log at debug; treat other errors
(permission denied, etc.) as warnings.

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

* test(packaging): smoke deb on PRs + backend test for run_cmd spawn errors

CI gap: deb-package.yml only fired on v* tag pushes, so a PR that
broke the .deb wasn't caught until release time. Wire it to PRs and
develop pushes via a paths filter covering packaging files and the
runtime files Etherpad needs at first boot. The release job already
gates on `if: startsWith(github.ref, 'refs/tags/v')` so PR runs
won't try to publish.

Test gap: the run_cmd.ts spawn-error fix (commit 5eee7895a) had no
test, which is how the bug shipped originally — plugins.ts spawned
`pnpm --version` at startup, the rejection was never caught, and
the .deb crashed mid-boot. Add a backend spec that exercises:
  - ENOENT for a missing binary -> rejects (regression test)
  - successful command -> resolves stdout
  - non-zero exit -> rejects with code

backend-tests.yml's recursive mocha glob picks up the new spec
automatically; no workflow change needed there.

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

* fix(packaging-ci): use NodeSource LTS for the smoke test (was Ubuntu's node 18)

ubuntu-latest's default apt nodejs is 18.19.1, but our package requires
nodejs (>= 20). The smoke test was doing `apt-get install nodejs`
followed by `dpkg -i ... || apt-get install -f`, which on a node-18
host fails the dep check, then `-f` "fixes" by REMOVING the etherpad
package — and the next assertion (test -x /usr/bin/etherpad) crashes.

Match what packaging/test-local.sh and the README recommend: install
node from NodeSource (current LTS) before installing the .deb.

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

* fix(packaging-ci): sudo-prefix smoke assertions that read /etc/etherpad

postinstall sets /etc/etherpad to 0750 root:etherpad (DB creds live
here) and /var/lib/etherpad similarly. The GH Actions runner user
isn't in the etherpad group, so 'test -f /etc/etherpad/settings.json'
hits EACCES. Add sudo to each check that crosses one of those dirs.

(Wrapping the whole block in `sudo bash <<EOF` would have been
cleaner but YAML literal-block + heredoc terminator don't play well
together at this indent.)

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

* fix(packaging): close chown -R symlink-deref escalation; Pre-Depends adduser

postinstall:
  Use `chown -hR` instead of `chown -R` on /var/lib/etherpad/var and
  /var/lib/etherpad/plugin_packages. Both directories are writable by
  the unprivileged etherpad service user, so a symlink planted there
  could redirect root's chown onto arbitrary system files (e.g.
  /etc/shadow) on the next `apt upgrade`. -hR makes chown act on the
  symlink itself rather than its target — standard mitigation for this
  TOCTOU-style local privilege escalation.

nfpm:
  Move adduser from Depends to Pre-Depends. preinst creates the
  etherpad user before unpacking; with plain `dpkg -i` (no apt) the
  Depends list isn't installed beforehand, so a minimal system without
  adduser would fail preinst before unpack and apt-get -f couldn't
  recover. Pre-Depends guarantees adduser is configured first.

Both flagged in Qodo's persistent review of 3daf300f0.

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

* fix(packaging): predepends lives at top-level deb:, not under overrides

nfpm's Overridables schema doesn't include predepends; it's a deb-only
top-level field. Previous commit nested it under overrides.deb, which
caused nfpm to reject the entire manifest with "field predepends not
found in type nfpm.Overridables" and broke both arch builds.

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

* fix(packaging): four Qodo follow-ups (CI ordering, secure node install, disable on remove, writable settings)

deb-package.yml:
  - Move 'Resolve version' (which calls `node -p`) to AFTER setup-node
    so it doesn't depend on the runner image preinstalling node.
  - Replace `curl ... | sudo bash` NodeSource installer with the
    explicit gpg-key + sources.list approach. Same outcome (NodeSource
    LTS apt repo), but no execution of network-fetched code as root.
    Reduces blast radius if NodeSource's setup endpoint is ever
    compromised — we only trust the signed apt repo metadata.

postinstall.sh:
  - /etc/etherpad/settings.json now etherpad:etherpad mode 0660 (was
    root:etherpad 0640). The admin /admin/settings UI persists changes
    by writing back to settings.settingsFilename; with the previous
    perms the etherpad user could read but not write, so saving via
    the admin UI failed silently. Group-only access preserved (DB
    creds still unreadable by other users).

postremove.sh:
  - On `dpkg --remove`, run `systemctl disable etherpad.service` before
    `daemon-reload` so the wants/ symlink doesn't dangle after dpkg
    deletes the unit file.

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

* fix(packaging): narrow workflow token scope; pin local nfpm to NFPM_VERSION

deb-package.yml:
  Workflow-level permissions was `contents: write` so the build job got
  write access on every PR run, even though only the release job needs
  it (to attach release assets). Narrow the workflow default to
  `contents: read` and let the release job opt back in to write — it
  already declares its own job-level `contents: write` block, so this
  is just removing an over-broad default.

test-local.sh:
  The script defined NFPM_VERSION but then unconditionally ran
  `goreleaser/nfpm:latest`, so local builds could diverge from CI's
  pinned v2.43.0. Use the variable in the docker tag (stripping the
  leading "v" to match the image's tag scheme).

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-04-27 10:33:30 +01:00
John McLear
d619f03214
fix(settings): derive randomVersionString from release identity (#7563) 
* fix(settings): derive randomVersionString from release identity

Fixes #7213.

Etherpad appends a `?v=<token>` cache-buster to static assets and
embeds the same token as `clientVars.randomVersionString` in the
padbootstrap JS bundle produced by specialpages.ts. Because esbuild's
content-hash feeds back into the generated bundle filename
(`padbootstrap-<hash>.min.js`), the token's value determines the file
that clients are told to load.

Historically the token was `randomString(4)`, regenerated on every
boot. In a horizontally-scaled deployment (ingress → etherpad
service → multiple pods) that meant every pod produced a different
filename for the same built artifact. A client that loaded the HTML
from pod A would request `padbootstrap-ABCD.min.js` from pod B and
hit a 404 when the upstream balancer placed the follow-up request
elsewhere.

Derive the token deterministically so pods of the same build emit
identical filenames, while still rotating on release so clients
invalidate their cache correctly:

  ETHERPAD_VERSION_STRING env  →  verbatim (integrator override)
  else                         →  sha256(version + "|" + gitVersion)[:8]

Backwards-compatible: single-pod deployments see the same effective
behavior (token rotates each release). Integrators who want to pin
the token explicitly — e.g. tying it to their own deploy ID — can set
`ETHERPAD_VERSION_STRING` in the environment.

Test coverage added in src/tests/backend/specs/settings.ts:
- Default shape is an 8-hex-char sha256 prefix.
- ETHERPAD_VERSION_STRING override is respected verbatim.

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

* test(7213): call reloadSettings() to exercise ETHERPAD_VERSION_STRING

The token is assigned inside reloadSettings, not parseSettings, so a
parseSettings-only call never sees the env var. Drive reloadSettings
directly, restoring the file paths and the prior token afterwards so
other tests see a clean module state.

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-04-27 03:36:35 +01:00
John McLear
83a42afbae
fix(export): /export/etherpad honors the :rev URL segment (#7566) 
Fixes #5071. `/p/:pad/:rev/export/etherpad` has always ignored the rev
parameter and returned the full pad history, unlike the txt/html
export endpoints which use the same route but do respect rev. Users
wanting to back up or inspect a snapshot of a pad at a specific rev
got every later revision in the payload instead — both wasteful and
a surprise when the downloaded .etherpad blob contained content that
had supposedly been reverted.

Change:
  - `exportEtherpad.getPadRaw(padId, readOnlyId, revNum?)` now takes an
    optional revNum. When supplied, it clamps to `min(revNum, pad.head)`,
    iterates only revs 0..effectiveHead, and ships a shallow-cloned pad
    object whose `head` and `atext` reflect the requested snapshot. The
    original live Pad is still passed to the `exportEtherpad` hook so
    plugin callbacks see the real document.
  - `ExportHandler` passes `req.params.rev` through on the `etherpad`
    type, matching the existing behavior of `txt` and `html`.
  - Chat history is intentionally left full (it is not rev-anchored).

Adds three backend regression tests under `ExportEtherpad.ts`:
  - default (no revNum) still exports the full history
  - explicit revNum limits exported revs and rewrites the serialized
    head so re-import reconstructs the pad at that rev
  - revNum above head is treated as full history, preventing accidental
    truncation of short pads

Out of scope: `getHTML(padID, rev)` on the API side is already honoring
rev in current code (exportHtml.getPadHTML threads the parameter
through), so the earlier report on that API call appears to be
resolved. This PR does not touch it.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
 2026-04-26 10:29:02 +02:00
John McLear
a1bb958b18
fix(pad): stop hardcoding lang='en', let the client auto-detect locale (#7586) (#7588) 
`Pad.normalizePadSettings()` was defaulting `lang` to the literal string
'en' when `rawPadSettings.lang` was not a string. That value flowed into
`clientVars.padOptions.lang` and then into `getParams()` in pad.ts,
which calls `html10n.localize([serverValue, 'en'])` as a callback for
the `lang` setting. The result: every pad forced English on load,
overriding the browser's Accept-Language and the existing auto-detect
chain in l10n.ts (cookie -> navigator.language -> 'en').

The regression was introduced in #7545 ("Add creator-owned pad settings
defaults", commit e0ccdb4d9). 2.6.1 did not have this default, so
auto-detect worked there. 2.7.0 broke it.

Fix: default `lang` to null. The client's existing flow already handles
null correctly — getParams() at pad.ts:172 has
`if (serverValue == null) continue;`, so the forced-localize callback
simply does not fire, and l10n.ts's browser-language auto-detect runs.
Pad-settings dropdown consumer at pad.ts:489 already uses
`padOptions.lang || 'en'` so null renders fine there too.

`PadSettings.lang` is now typed `string | null` to match.

Added three backend regression tests under `normalizePadSettings lang`:
  * defaults to null when lang is absent (so client auto-detects)
  * preserves an explicit string lang (creator override still works)
  * drops non-string lang values to null rather than coercing to 'en'

Manual verification: with Firefox set to German, loading a fresh pad
now renders the UI in German. Index and timeslider continued to work
as before. Setting `?lang=de` or a language cookie continues to
override browser detection, as intended.

Fixes #7586
 2026-04-23 10:38:46 +01:00
John McLear
9e352ca4f3
fix(clientVars): stop mutating the shared plugin registry during sanitization (#7587) 
PadMessageHandler built the `pluginsSanitized` payload for clientVars by
aliasing `plugins.plugins` and then mutating each entry's `package` field
in place:

    let pluginsSanitized: any = plugins.plugins;
    Object.keys(plugins.plugins).forEach(function(element) {
      const p: any = plugins.plugins[element].package;
      pluginsSanitized[element].package = {name: p.name, version: p.version};
    });

Because `pluginsSanitized` is a reference to `plugins.plugins`, the
assignment clobbered the server-side plugin registry. After the first
pad connection, every plugin's `package` object held only `{name,
version}` — `realPath`, `path`, and `location` were gone.

Minify.ts resolves `/static/plugins/ep_*/...` URLs via
`plugin.package.realPath`. Once the field disappeared, every subsequent
static asset request for a bundled plugin 500'd with:

    TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of
    type string. Received undefined
        at Object.join (node:path:1354:7)
        at _minify (src/node/utils/Minify.ts:181:23)

Symptoms on Chromium: plugin CSS/JS assets fail to load (e.g.
/static/plugins/ep_font_size/static/css/size.css returns 500), so
plugins partially render or don't work at all. Firefox swallows the
resulting console errors quietly.

Fix: extract the sanitization into a pure helper `sanitizePluginsForWire`
that returns a fresh object graph and never touches the input. The
helper is covered by a new backend spec that:
  * verifies the sanitized output has only {name, version} in `package`
  * asserts the input registry's realPath/path/location survive the call
  * runs the call repeatedly and confirms non-destructiveness
  * mutates the returned copy and asserts the input is independent

Verified live with the dev server: before the fix, `/static/plugins/
ep_font_size/static/css/size.css` 500'd after visiting any pad; after
the fix it returns 200 both before and after pad connections.
 2026-04-23 10:18:57 +01:00
John McLear
67e542d2b9
fix(editor): preserve U+00A0 non-breaking space (#3037) (#7585) 
* fix(editor): preserve U+00A0 non-breaking space (#3037)

Non-breaking spaces were silently normalized to regular spaces at every
ingestion point, so typed/pasted/imported nbsps never reached the
changeset and users could not glue words against line-wrap in French or
other languages that require nbsp typography.

Removed the four strip sites that replaced U+00A0 with U+0020:
  - src/node/db/Pad.ts cleanText
  - src/static/js/contentcollector.ts textify
  - src/static/js/ace2_inner.ts textify
  - src/static/js/ace2_inner.ts importText raw-text guard

Updated both processSpaces functions (domline and ExportHtml) to tokenize
U+00A0 as a separate unit, emit it verbatim as &nbsp;, and treat it as
content (not whitespace) for the run-collapse bookkeeping so adjacent
regular-space runs aren't miscounted.

Added backend round-trip tests for spliceText and setText, and extended
the cleanText case table. Updated the existing contentcollector and
importexport specs whose expectations encoded the previous buggy
behavior; they now assert genuine nbsp preservation.

Verified manually in Firefox: clipboard U+00A0 → paste → pad → getText
returns c2 a0; getHTML emits `100&nbsp;km`.

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

* fix(contentcollector): collapse display-artifact nbsp runs on DOM read-back

processSpaces is a lossy one-way display transform: leading/trailing
spaces and all-but-the-last of a run get rendered as &nbsp; so HTML
doesn't collapse them. When incorporateUserChanges reads text back from
the DOM, those display-artifact nbsps were being stored in the changeset
model instead of being normalized back to plain spaces.

This broke handleReturnIndentation, whose /^ *(?:)/ regex only matches
ASCII spaces: auto-indent after `foo:\n` produced 4 spaces instead of
the expected prev-indent (2) + THE_TAB (4) = 6, because the previous
line's model had nbsps where it used to have spaces.

Fix: in contentcollector.textify, collapse any [  ]+ run back to
plain spaces UNLESS the run is pure U+00A0 AND strictly interior to
word chars. That preserves user-intended typographic nbsps like
"100 km" while undoing the one-way display transform.

Updated 7 contentcollector tests and 7 importexport tests whose
assertions needed to reflect the new rule (boundary/mixed runs collapse;
pure-interior nbsp runs preserve).

Fixes the Playwright regression in indentation.spec.ts:117 that the
previous commit introduced.

* fix(contentcollector): canonicalize nbsp runs at line assembly, not per text node

Addresses Qodo code review feedback on PR #7585.

## Bug fix — nbsp lost at DOM text-node boundary

The previous approach ran the "collapse display-artifact nbsp" rule inside
textify(), which is called per individual DOM TEXT_NODE. A user-intended
nbsp sitting at a text-node boundary (e.g., <span>100</span><span>&nbsp;km
</span>) was incorrectly seen as non-interior (before === '' for the second
text node) and normalized back to a regular space.

Fix: move the canonicalization out of textify() and run it on each
fully assembled line string inside cc.finish(). The rule remains:

    [  ]+ run  ->  plain spaces
                   UNLESS pure U+00A0 AND strictly interior to non-ws chars

It is length-preserving, so attribute offsets and line lengths are
unaffected.

Added a regression test (contentcollector.spec.ts) for the cross-span
case.

## Docs concern

Reverted the type-only addition of spliceText to PadType. spliceText
is an existing Pad runtime method; the backend test now uses a cast
(`(pad as any).spliceText`) so the PR does not expand the declared
public type surface, avoiding a separate documentation requirement.

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
 2026-04-23 08:57:30 +01:00
Stefan Müller
fe6a373bf8
feat: Remove paths from plugin packages (#7580) 2026-04-22 10:52:53 +01:00
Stefan Müller
e8c9efb5c5
chore: Rename some occurences of etherpad-lite to etherpad (#7552) 
* chore: Rename some occurences of etherpad-lite to etherpad

* chore: Adjust etherpad git urls

* chore: Rename more occurences from etherpad-lite to etherpad

* chore: Adjust default text
 2026-04-19 16:53:57 +02:00
John McLear
e0ccdb4d9f
Add creator-owned pad settings defaults (#7545) 
* Add creator-owned pad settings defaults

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Refine pad settings layout

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Fix settings popup heading and width

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Explain enforced user settings

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Cover creator override flow

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Let creators bypass enforced settings

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Address pad settings follow-ups

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
 2026-04-19 11:13:44 +01:00
John McLear
4137109efe
fix: allow undo of clear authorship colors without disconnect (#7430) 
* fix: allow undo of clear authorship colors without disconnect (#2802)

When a user clears authorship colors and then undoes, the undo changeset
re-applies author attributes for all authors who contributed text. The
server was rejecting this because it treated any changeset containing
another author's ID as impersonation, disconnecting the user.

The fix distinguishes between:
- '+' ops (new text): still reject if attributed to another author
- '=' ops (attribute changes on existing text): allow restoring other
  authors' attributes, which is needed for undo of clear authorship

Also removes the client-side workaround in undomodule.ts that prevented
clear authorship from being undone at all, and adds backend + frontend
tests covering the multi-author undo scenario.

Fixes: https://github.com/ether/etherpad-lite/issues/2802

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

* fix: use robust Playwright assertions in authorship undo tests

- Use toHaveAttribute with regex instead of raw getAttribute + toContain
- Check div/span attributes within pad body instead of broad selectors
- Use Playwright auto-retry (expect with timeout) instead of toHaveCount(0)

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

* fix: handle confirm dialog and sync timing in Playwright tests

- Add page.on('dialog') handler to accept the confirm dialog triggered
  by clearAuthorship when no text is selected (clears whole pad)
- Use auto-retrying toHaveAttribute assertions instead of raw getAttribute
- Increase cross-user sync timeouts to 15s for CI reliability
- Add retries: 2 to multi-user test for CI flakiness
- Scope assertions to pad body spans instead of broad selectors

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

* fix: use persistent socket listeners to avoid missing messages in CI

Replace sequential waitForSocketEvent loops with single persistent
listeners that filter messages inline. This prevents race conditions
where messages arrive between off/on listener cycles, causing timeouts
on slower CI runners.

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

* fix: reject - ops with foreign author to prevent pool injection

The '-' op attribs are discarded from the document but still get added
to the pad's attribute pool by moveOpsToNewPool. Without this check, an
attacker could inject a fabricated author ID into the pool via a '-' op,
then use a '=' op to attribute text to that fabricated author (bypassing
the pool existence check).

Now all non-'=' ops (+, -) with foreign author IDs are rejected.

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

* test: use not.toHaveClass for cleared authorship spans

Addresses Qodo review: linestylefilter skips attribs with empty values,
so a span with author='' has no class attribute at all. The previous
negative-lookahead regex on the class attribute failed against a null
attribute and was flaky in CI. Switch to not.toHaveClass(/author-/),
which also passes when the attribute is missing.

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

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
 2026-04-19 09:09:03 +01:00
John McLear
7ec581afca
feat!: replace Abiword with LibreOffice and add DOCX export (#7539) 
* feat!: replace Abiword with LibreOffice and add DOCX export (#4805)

The Abiword converter is dropped. Abiword's DOCX export is weak and the
project is niche on modern platforms; LibreOffice (soffice) is the
common deployment path and now serves as the sole converter backend.

DOCX is added as an export format and becomes the new target for the
"Microsoft Word" UI button. The /export/doc URL still works for legacy
API consumers.

BREAKING CHANGE: The 'abiword' setting, the INSTALL_ABIWORD Dockerfile
build arg, the abiwordAvailable clientVar, and the
#importmessageabiword UI element (with locale key
pad.importExport.abiword.innerHTML) are removed. Deployments relying on
Abiword must configure 'soffice' instead.

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

* feat: add docxExport feature flag and abiword deprecation WARN

- Add `docxExport: true` setting to opt out of DOCX (use legacy DOC)
- Pass `docxExport` to client via clientVars
- Use `docxExport` flag in pad_impexp.ts for Word button format
- Emit a specific WARN when deprecated `abiword` config is detected
- Update settings.json.template and settings.json.docker with docxExport
- Add docxExport to ClientVarPayload type in SocketIOMessage.ts

Agent-Logs-Url: https://github.com/ether/etherpad/sessions/9afc5291-73b2-4b66-b028-feed39e7056f

Co-authored-by: JohnMcLear <220864+JohnMcLear@users.noreply.github.com>

* refactor: extract wordFormat variable and improve docxExport comment

Agent-Logs-Url: https://github.com/ether/etherpad/sessions/9afc5291-73b2-4b66-b028-feed39e7056f

Co-authored-by: JohnMcLear <220864+JohnMcLear@users.noreply.github.com>

* fix: restore import-limitation message when no converter is configured

The abiword removal dropped both the #importmessageabiword DOM element
and its locale key, but Copilot's refactor still expected the show()
call to surface a message when exportAvailable === 'no'. Result: users
with no soffice binary got silent failure instead of an explanation.

Add #importmessagenoconverter back with updated, LibreOffice-focused
copy (new locale key pad.importExport.noConverter.innerHTML) and flip
the hidden prop when the client knows no converter is available.

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

* i18n: inline English fallback for noConverter import message

The original abiword message existed in ~70 locale files and was
removed from all of them by this PR. The replacement key was only
added to en.json, so non-English users had an empty div until
translators localize. Follow the project's usual pad.html pattern
(e.g. line 146's "Font type:") and include the English text inside
the div as the fallback content; html10n replaces it when a
translation is available.

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

* Revert "i18n: inline English fallback for noConverter import message"

This reverts commit f336f24d. Follow the project convention: add the
new locale key to en.json only and let translations catch up via the
translation system, rather than putting inline fallback in the template.

* i18n: leave non-English locale files untouched

The PR had removed pad.importExport.abiword.innerHTML from ~82 locale
files alongside its removal from en.json. The replacement message uses
a new key (pad.importExport.noConverter.innerHTML) in en.json only, so
churning every localisation file for a key that is no longer referenced
produces useless translation diffs. Restore every non-en locale file to
its pre-PR state.

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

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: JohnMcLear <220864+JohnMcLear@users.noreply.github.com>
 2026-04-19 09:08:22 +01:00
John McLear
66f49bb808
docs(openapi): document apikey auth in openapi.json (#7534) 
* docs(openapi): document apikey auth in openapi.json (#7532)

The API accepts the key via ?apikey=, ?api_key=, or the apikey header, but
only ?apikey= was advertised in /api-docs.json. /api/{version}/openapi.json
was worse: it hardcoded an OAuth2 scheme even when Etherpad was started in
apikey auth mode.

Switch both generators on settings.authenticationMethod and publish apiKey
schemes for the query (apikey, api_key) and header (apikey) variants. The
openapi.ts definition is now regenerated per request so runtime settings
are reflected.

The raw authorization: <key> header still works in code but is deliberately
not documented — pinning it in the spec would ossify a quirk.

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

* refactor(openapi): add apiKeyAlias/apiKeyHeader conditionally in RestAPI.ts

In SSO mode, apiKeyAlias and apiKeyHeader were always present in
securitySchemes even though they're only relevant when
authenticationMethod is 'apikey'. Mirror the pattern used for the sso
scheme: add these two schemes dynamically inside the apikey branch, and
mark them optional in the TypeScript type annotation.

Agent-Logs-Url: https://github.com/ether/etherpad/sessions/1d440432-7389-462e-9aac-9a3c027640e8

Co-authored-by: JohnMcLear <220864+JohnMcLear@users.noreply.github.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: JohnMcLear <220864+JohnMcLear@users.noreply.github.com>
 2026-04-18 17:09:18 +01:00
John McLear
31e0a61126
fix: capture head revision atomically with atext to prevent mismatched apply (#7480) 
* fix: capture head revision atomically with atext to prevent mismatched apply

When constructing CLIENT_VARS, pad.atext was captured at one point but
pad.getHeadRevisionNumber() was called later. If concurrent edits
advanced the revision between these two reads, the client received
initialAttributedText from rev N but rev=N+3, causing "mismatched apply"
errors when the next changeset arrived (expecting rev N+3 text).

Now captures headRev at the same time as atext and uses the captured
value consistently in CLIENT_VARS and sessionInfo.

Fixes #4040

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

* fix: flush missed revisions after socket joins pad room

During handleClientReady(), the server awaits the clientVars hook before
socket.join(). Any revisions appended during that await window are
broadcast to existing room members but the connecting socket misses them.
Call updatePadClients(pad) after joining to flush any such revisions.

Also adds a regression test that injects a slow clientVars hook and
verifies the connecting client receives catch-up changesets for edits
that occurred during the hook await window.

Fixes #4040

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

* test: fix race condition in clientVars hook test

Listen for messages during handshake to avoid missing NEW_CHANGES that
arrive before the explicit waitForSocketEvent listener is attached.

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

* fix: initialize sessionInfo.time before catch-up updatePadClients

The catch-up updatePadClients() call introduced in this PR could send
NEW_CHANGES with timeDelta=NaN because sessionInfo.time was never set
for new sessions. NaN poisons the client-side broadcast/timeslider
currentTime tracking.

Initialize sessionInfo.time to the timestamp of the snapshot revision
before the catch-up flush, with a fallback to Date.now() if the
revision date is unavailable.

Also strengthens the regression tests:
- Validate that initialAttributedText matches the pad AText at the
  EXACT advertised rev (not just the latest pad text), using
  pad.getInternalRevisionAText(rev).
- Add a load test that hammers the pad with concurrent edits while
  multiple clients connect, asserting CLIENT_VARS consistency under
  the exact race condition the fix is targeting.

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

* test: replace open-ended load loop with bounded mid-handshake edit

The previous load test ran 'while (!stopLoad) await pad.setText(...)'
in the background while the test connected clients. This saturated
ueberDB's write queue and on shutdown the queued writes never drained,
hanging the mocha process for the full 6h GitHub Actions job timeout.

Replace it with a bounded approach: a clientVars hook lands 3 edits
mid-handshake (deterministic, no background loop, no shutdown hang).
Still exercises the exact race the fix targets — an edit advancing
the rev after the atext snapshot but before CLIENT_VARS is sent —
and asserts AText / rev consistency via getInternalRevisionAText.

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

* test: address remaining Qodo concerns on PR #7480

Addresses Qodo review items 1, 2, 5 from
https://github.com/ether/etherpad-lite/issues/comments/4194702740 :

- Concern 1 (no loadTesting reproduction test): the suite now toggles
  settings.loadTest = true in before(), restores in after(). The
  middle test also pre-populates the pad with 20 revisions before
  connecting so we genuinely exercise a busy/loaded pad rather than a
  fresh one.

- Concern 2 (no CLIENT_VARS / NEW_CHANGES delay test): the slow
  clientVars hook in the middle test now has explicit setTimeout
  delays before AND after the mid-handshake edits, so the race window
  between atext snapshot and CLIENT_VARS send is observably wide
  rather than relying on async scheduling alone. The test also
  collects post-handshake messages and asserts a NEW_CHANGES catch-up
  arrives when the pad advanced past the advertised rev.

- Concern 5 (test doesn't validate rev): both rev-consistency tests
  use pad.getInternalRevisionAText(advertisedRev) and assert text and
  attribs match, not just `pad.text() === clientVars.text`.

Concerns 3 (connect can miss revisions) and 4 (NaN timeDelta) were
already addressed in earlier commits on this branch via the catch-up
updatePadClients() call and the sessionInfo.time initialization.

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

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
 2026-04-07 18:30:08 +01:00
John McLear
8c1b8b0902
fix: add setters to CJS compatibility layer in Settings (#7481) 
The CJS compatibility block added in fd97532 only defined getters,
making settings properties read-only for plugins using require().
Plugins like ep_webrtc need to mutate settings (e.g. requireAuthentication)
in tests. Add setters so CJS consumers can write properties too.

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
 2026-04-06 16:19:23 +01:00
John McLear
29faec4a04
fix: increase max socket.io message size to 10MB for large pastes (#7474) 
* fix: increase max socket.io message size to 10MB for large pastes

The default maxHttpBufferSize of 50KB caused socket.io to drop
connections when pasting >10,000 characters. Increased to 10MB which
safely accommodates large paste operations.

Fixes #4951

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

* chore: reduce default maxHttpBufferSize to 1MB

10MB was too generous and creates a DoS vector. 1MB (socket.io's own
default) is sufficient for large pastes while limiting memory abuse.

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

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
 2026-04-06 15:12:08 +01:00
John McLear
2814e5b913
fix: dev mode entrypoint paths respect x-proxy-path header (#7472) 
* fix: prevent race condition in session cleanup timeout

When the cleanup timeout fires, check the in-memory exp.real before
reading from the DB. If touch() extended the expiry (but the old
timeout fires late, e.g. on slow CI), reschedule instead of reading
potentially stale cached data from the DB and destroying the session.

Also increased test expiry times so the "touch after eligible for
refresh" test isn't sensitive to event loop delays on slow machines.

Fixes flaky SessionStore test from #7448.

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

* fix: dev mode entrypoint paths respect x-proxy-path header

In dev mode, the /watch/* script paths were hard-coded as absolute
paths without considering the x-proxy-path header used for subdirectory
reverse proxy setups. This caused 404s for the script tags when hosting
Etherpad on a subdirectory URL (e.g., /pad).

Now reads the x-proxy-path header from the request and prefixes the
entrypoint path, matching how admin.ts handles proxy paths.

Fixes #7137

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

* test: make proxy path tests deterministic in production mode

Tests now verify entrypoint paths and x-proxy-path header handling
in production mode (where tests run) rather than conditionally
asserting only in dev mode.

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

* security: sanitize x-proxy-path header to prevent XSS

The header value was injected directly into <script src="...">
without sanitization. An attacker who can set request headers could
inject arbitrary HTML/JS. Now only allows path-safe characters.

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

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
 2026-04-06 13:32:33 +01:00
John McLear
270e3c6576
fix: very old .etherpad imports could break import due to lack of aut… (#7473) 
* fix: very old .etherpad imports could break import due to lack of author metadata, allow this now

* test: add regression tests for old .etherpad import without author

Tests that importing an old .etherpad export (circa 2014) where
revision records lack meta.author succeeds without error, and that
getRevisionAuthor returns '' for such revisions.

Covers the fix for #6785.

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

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
 2026-04-06 13:31:57 +01:00
John McLear
6a3094b244
fix: sort language dropdown alphabetically by native name (#7477) 
* fix: sort language dropdown alphabetically by native name

Languages in the settings dropdown were ordered by language code,
making it hard to find specific languages. Now sorted alphabetically
by their native display name.

Fixes #3263

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

* test: verify language dropdown is sorted by native name

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

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
 2026-04-06 13:28:57 +01:00
John McLear
ac118cfde7
fix: preserve ordered list numbering across bullet interruptions in export (#7470) 
* fix: preserve ordered list numbering across unordered list interruptions in export

When ordered lists were interrupted by unordered lists, each new <ol>
segment started at 1 instead of continuing the previous numbering.
Track running counts per indent level and emit start attributes.

Fixes #6471

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

* fix: respect explicit start attributes and reset counters per level

- line.start takes priority over counter-based continuation when present
- Counter is seeded from line.start to keep subsequent continuations aligned
- Counters for closed indent levels are cleared when list depth decreases

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

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
 2026-04-06 11:24:01 +01:00