Commit graph

24 commits

Author SHA1 Message Date
John McLear
860ab68c04
test: downstream client compatibility gate (Phase 1) (#7923)
* docs: design for downstream client compatibility tests

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

* docs: Phase 1 implementation plan for downstream compat tests

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

* test(downstream): add golden wire-vector generator

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

* test(downstream): add committed golden wire-vectors fixture

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

* test(downstream): assert wire-vectors fixture stability + consistency

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

* test(downstream): pin socket.io handshake + USER_CHANGES sequence

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

* test(downstream): snapshot client-facing HTTP API shapes

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

* test(downstream): add client manifest (entries disabled pending Phase 2)

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

* ci(downstream): add downstream-smoke workflow (boot/self-check/teardown + manifest scaffold)

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

* ci(downstream): validate settings rewrite + ignore docs/** (Qodo)

Fail fast if the template's port/auth literals drift so a no-op sed can't
silently boot the smoke server on the wrong port/auth. Also ignore docs/**
(not just doc/**) so docs-only PRs don't trigger the boot job.

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

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-09 11:40:58 +01:00
John McLear
4b30653a3d
chore(root): drop three redundant top-level files (#7839)
* chore(root): drop redundant top-level files

Three small clean-ups to make the project root easier to scan for new
contributors:

- best_practices.md was a near-duplicate of CONTRIBUTING.md. Merge its
  two unique bullets (PRs MUST include a description / flag empty
  descriptions as incomplete) into CONTRIBUTING.md and remove the file.
- .pr_agent.toml is a two-line Qodo PR-bot config. It has no functional
  references in the repo and Qodo currently doesn't review this repo.
- tests/ -> src/tests was an unused root symlink. Verified no workflow,
  script, eslint config, or playwright config relies on the root path
  (all callers use src/tests/... or the ep_etherpad-lite package path).

No CI, build, or docs updates needed.

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

* chore(root): fix dead tests/frontend + best_practices.md refs

Follow-up on Qodo review feedback for #7839:

- CONTRIBUTING.md: front-end-tests path now reads src/tests/frontend/
  instead of the dead tests/frontend/ (the root tests symlink is gone).
  The browser URL <yourdomainhere>/tests/frontend stays — that's an
  Express route served by the test runner, not a filesystem path.
- docs/superpowers/specs/...openapi-design.md: drop best_practices.md
  from the parenthetical list of policy sources (now CONTRIBUTING.md
  and AGENTS.MD), since best_practices.md has been removed.

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-25 15:15:17 +01:00
John McLear
86edd67f58
feat: support X-Forwarded-Prefix and X-Ingress-Path (#7802) (#7806)
* docs: design for URL base-path support (#7802)

Spec covers the architecture, header handling rules, components touched,
backwards-compatibility story, risks, and test plan for honoring
X-Forwarded-Prefix / X-Ingress-Path under trustProxy.

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

* docs: amend #7802 spec with discovery of pre-existing proxy-path helpers

After exploring the codebase, much of the proposed architecture is
already in place (sanitizeProxyPath, padBootstrap.js basePath derivation,
admin SPA rewrite). Spec now reflects the actual delta: header source
expansion, /manifest.json prefix-awareness, socialMeta proxyPath honoring,
and template URL touch-ups for index/timeslider/pad/export.

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

* docs(plan): implementation plan for URL base-path support (#7802)

Adds the bite-sized TDD task list to ship X-Forwarded-Prefix /
X-Ingress-Path support: extends sanitizeProxyPath, makes /manifest.json
and socialMeta prefix-aware, touches up the remaining leading-slash
URLs in index/pad/timeslider/export templates, fixes a pre-existing
manifest .. count bug. Drops the originally-proposed <base href>
belt-and-braces after discovering it'd break the existing relative
URLs in pad.html/timeslider.html and wouldn't help plugin DOM
injection anyway (path-absolute URLs ignore <base>'s path component).

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

* feat(proxy): accept X-Forwarded-Prefix and X-Ingress-Path under trustProxy (#7802)

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

* feat(pwa): make /manifest.json honor sanitised proxy-path (#7802)

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

* feat(social-meta): honor proxyPath in from-request og:url and og:image (#7802)

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

* feat(templates): index.html manifest + jslicense links honor proxyPath (#7802)

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

* feat(templates): pad.html reconnect/jslicense honor proxyPath; fix manifest .. count (#7802)

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

* feat(templates): timeslider.html reconnect/jslicense honor proxyPath; fix manifest .. count (#7802)

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

* feat(templates): export_html.html manifest honors proxyPath when available (#7802)

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

* test: end-to-end coverage for X-Forwarded-Prefix / X-Ingress-Path (#7802)

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

* docs(settings): trustProxy also enables X-Forwarded-Prefix / X-Ingress-Path (#7802)

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-18 13:27:59 +01:00
John McLear
4d998d6ef2
fix(admin): show resolved runtime values on /admin/settings (#7803) (#7807)
* docs: spec for admin/settings resolved runtime values (#7803)

Side-channel resolved+redacted settings alongside raw file blob.
Form view dropdowns and env pill chips reflect actual runtime values
instead of falling back to template defaults. Save round-trip is
unchanged so ${VAR:default} literals stay intact on disk.

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

* docs: implementation plan for admin/settings resolved runtime (#7803)

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

* feat(admin): add redactor for resolved settings payload (#7803)

Pure helper that walks the live settings module and replaces known
sensitive paths (users.*.password, dbSettings.password,
sso.clients[*].client_secret, sessionKey, …) with [REDACTED] sentinel.

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

* feat(admin): emit redacted runtime settings on /settings socket load (#7803)

Existing 'results' raw-file blob is unchanged so the textarea editor
and saveSettings round-trip continue to preserve \${VAR:default}
literals on disk. New 'resolved' field carries the in-memory settings
module run through the redactor — admin SPA can use it to show actual
runtime values next to env-var placeholders.

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

* feat(admin): show resolved runtime value on EnvPill (#7803)

Admin SPA now stores the resolved field from the /settings socket
payload and exposes useResolvedAt(path) to walk it. EnvPill renders a
"→ active value" chip when the path is resolved, or "→ ••••••" with a
redacted tooltip when the server returned the [REDACTED] sentinel.
Old-server fallback (undefined resolved) keeps current behaviour.

The admin test script glob now picks up .test.tsx alongside .test.ts
so the new EnvPill tests run under tsx --test.

Closes #7803.

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-18 13:27:53 +01:00
John McLear
29dac6bfcc
fix(pad): redesign outdated-version notice (#7799) (#7804)
* docs: design spec for #7799 outdated-notice redesign

Per-pad first-author gating, dismissable gritter, minor-or-more rule, drop vulnerable UI.

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

* docs: implementation plan for #7799 outdated-notice redesign

12 bite-sized tasks, TDD-first where applicable; closes the spec end-to-end.

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

* feat(updater): add isMinorOrMoreBehind, drop major/vulnerable helpers

Adds isMinorOrMoreBehind(current, latest) which returns true only when
the latest release is at least one minor version ahead (patch-only deltas
return false). Removes isMajorBehind, parseVulnerableBelow, and
isVulnerable from versionCompare.ts — callers in updateStatus.ts,
VersionChecker.ts, and index.ts will be updated in subsequent tasks.

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

* refactor(updater): drop vulnerable-below directive and state field

Remove VulnerableBelowDirective type, UpdateState.vulnerableBelow field, and
all related scraping/checking logic (parseVulnerableBelow, isVulnerable imports).
Clean up Notifier, OpenAPI schema, and all test fixtures to match.

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

* refactor(updater): drop residual EmailSendLog vulnerable fields

Remove `vulnerableAt` and `vulnerableNewReleaseTag` from the
`EmailSendLog` interface, `EMPTY_STATE`, and the `isValidEmail`
validator — these backed the removed `vulnerable`/`vulnerable-new-release`
email kinds and are now dead code.

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

* feat(updater): add firstAuthorOf helper

Export firstAuthorOf() from updateStatus.ts — finds the lowest-numbered
author attrib in a pad's pool, skipping empty-string placeholders.
Covered by 6 vitest cases in tests/backend-new/specs/hooks/express/.

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

* feat(updater): add resolveRequestAuthor helper for HTTP GET

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

* feat(updater): pad-aware /api/version-status with first-author gating

Replace global badge cache with a per-(padId, authorId) LRU cache. The
new response shape is {outdated: 'minor' | null, isFirstAuthor: boolean};
the old 'severe'/'vulnerable' enum is dropped entirely. computeOutdated
now resolves the pad's first author and compares it against the session
author before returning outdated:'minor', so the notice is only shown to
the person who created the pad.

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

* fix(updater): switch isSevere signal from major-only to minor-or-more behind

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

* test(updater): end-to-end coverage for /api/version-status

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

* docs(openapi): /api/version-status pad-aware shape and gating

Add the /api/version-status GET operation to the admin OpenAPI spec with
the new pad-aware response shape: outdated enum reduced to [minor]|null,
isFirstAuthor boolean, and an optional padId query param.

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

* chore(pad): remove unused #version-badge template and CSS

* feat(pad): replace persistent badge with first-author outdated gritter

Renames pad_version_badge.ts → pad_outdated_notice.ts and rewrites it
as a fire-and-forget gritter notice that only shows when the API reports
outdated=minor AND the current user is the pad's first author.  Wires
the new maybeShowOutdatedNotice() call into pad.ts immediately after
showPrivacyBannerIfEnabled().

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

* test(pad): playwright coverage for outdated notice gritter

Six Playwright specs exercise maybeShowOutdatedNotice: null response,
isFirstAuthor:false guard, positive appearance + text, X-dismiss,
500 server error tolerance, and 8 s auto-fade.

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

* docs(pad): outdated-notice redesign + drop vulnerable-below docs

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

* chore(test): remove stale specs for deleted #version-badge surface

Delete the GET /api/version-status describe block from the legacy mocha
spec (asserted outdated:null and outdated:'severe' — both no longer match
the new response shape). The new vitest spec at
tests/backend-new/specs/hooks/express/updateStatus.test.ts covers this
surface comprehensively.

Delete src/tests/frontend-new/specs/pad-version-badge.spec.ts entirely:
all three tests reference the #version-badge DOM element removed in Task 8
and stub 'severe'/'vulnerable' enum values that no longer exist.

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

* chore: clean stale references to vulnerable/severe in types, emails, docs

- Remove OutdatedLevel type (null|'severe') from types.ts — no consumers
  remain after the badge redesign removed the severe tier.
- Fix Notifier severe-email body: was "more than one major release behind"
  but isSevere now fires on minor-or-more, so update to "at least one
  minor release behind the latest published version".
- Drop "vulnerability directives" from the /admin/update/status OpenAPI
  description; replace with the actual response fields.
- Remove stale vulnerableBelow field from UpdateStatusPayload in
  admin/src/store/store.ts — server no longer sends it.
- Fix docs/admin/updates.md: "pad-side badge" → "pad-side notice".

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

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-18 12:23:40 +01:00
John McLear
962bfe8649
feat(updater): tier 4 — autonomous update in maintenance window (#7607) (#7753)
* docs(updater): plan tier 4 — autonomous update in maintenance window (#7607)

Maps PR 4 of the auto-update design spec (§"Tier 4 — autonomous") to concrete
files, tasks, and verification steps. Subsequent commits scaffold against this
plan.

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

* feat(updater): MaintenanceWindow module — wall-clock window math for tier 4

Pure module: parseWindow, inWindow, nextWindowStart. Supports tz=local|utc
and cross-midnight ranges. Used by upcoming Scheduler + UpdatePolicy changes.

22 vitest unit tests cover format validation, same-day + cross-midnight
boundaries, and host-local vs UTC clock comparisons. DST handling is
absorbed by JS Date constructor's wall-clock normalization (documented in
the file header).

Refs #7607

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

* feat(updater): tier 4 backend — window-gated UpdatePolicy + Scheduler

Wires MaintenanceWindow into the existing tier 3 backend so autonomous
updates only fire while `now` is inside `updates.maintenanceWindow`.

UpdatePolicy
  - new optional `maintenanceWindow` input
  - canAutonomous flips on only for git+tier=autonomous+parse-valid window
  - new reasons `maintenance-window-missing` / `maintenance-window-invalid`
  - rollback-failed still wins over window denial

Scheduler
  - decideSchedule snaps scheduledFor forward to nextWindowStart when
    canAutonomous + grace lands outside the window
  - decideTriggerApply returns a new `{action: 'defer'}` when canAutonomous
    + fire-time is outside the window; carries nextStart for the runner
  - canAutonomous=false preserves Tier 3 behavior unchanged

index.ts wires settings.updates.maintenanceWindow through both passes and
re-arms the timer on defer. Status endpoint surface (nextWindowOpensAt) +
admin UI picker land in a follow-up commit.

Settings adds `maintenanceWindow: {start, end, tz} | null`, defaulting to
null. settings.json.template / settings.json.docker document the shape.

Tests
  - 22 vitest cases for MaintenanceWindow already cover the math
  - 4 new UpdatePolicy cases for the window outcomes
  - 6 new Scheduler cases for tier-4 schedule/trigger paths
  - Full backend-new suite: 629 passed (35 files)

Refs #7607

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

* feat(updater): tier 4 admin UI — window status, deferred subtitle, banner

GET /admin/update/status now returns:
  - `maintenanceWindow`: the parsed window object (admin sessions only)
  - `nextWindowOpensAt`: ISO of the next window opening when tier=autonomous

UpdatePage
  - new "Maintenance window" section when tier=autonomous, shows current
    window summary + next opens at, or "Not configured" when unset
  - scheduled panel now appends a "deferred until <iso>" line when the
    backend has snapped scheduledFor to the next window opening

UpdateBanner
  - new variant when tier=autonomous and policy.reason is
    `maintenance-window-missing` or `maintenance-window-invalid`, linking
    to /admin/update

i18n
  - 8 new keys under `update.banner.*`, `update.page.policy.*`,
    `update.page.scheduled.*`, `update.window.*` (en.json only;
    translations follow via the usual locale workflow)

Interactive picker is intentionally deferred — admins edit
`updates.maintenanceWindow` via the parsed JSONC settings editor (#7709).
A follow-up commit may add a thin write-through component if the JSONC
round-trip turns out to be too rough for typical operators.

Refs #7607

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

* docs(updater): tier 4 — window gate, DST notes, runbook §12 (#7607)

CHANGELOG: flip Tier 4 from "designed, not yet implemented" to current.
Document maintenanceWindow shape, snap-forward, defer-at-fire, and the
two missing/invalid policy reasons.

doc/admin/updates.md: new "Tier 4 — autonomous in a maintenance window"
section with config example, policy gating, DST/timezone notes, admin UI
behavior.

runbook: §12 walks a disposable VM through missing-window, malformed,
outside-window deferral, fire-at-opening, and window-closes-mid-grace.
Adds five sign-off checklist items.

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

* test(updater): tier 4 window-boundary integration (#7607)

Mocha integration covering the four scenarios called out in the spec
§"Tier 4 — autonomous":

  - outside-window: decideSchedule snaps scheduledFor forward to the
    next opening and the snapped value round-trips through saveState
  - inside-window at fire-time: decideTriggerApply returns fire
  - window-closes-mid-grace: decideTriggerApply returns defer with
    nextStart at the next opening; persisted state moves forward
  - cancel during deferred-grace: state returns to idle, and the next
    decideSchedule pass re-emits a schedule snapped to the next opening

All 4 cases passing locally under tsx mocha.

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

* feat(updater): real SMTP via nodemailer (mail.* settings) (#7607)

Replaces the (would send email) stub introduced in PR #7601 with a
nodemailer-backed transport. The dependency is lazy-imported so installs
that don't set mail.host pay no runtime cost.

Settings additions
  - new top-level mail block: host, port, secure, from, auth (user/pass)
  - mail.host=null keeps the legacy log-only behaviour; the Notifier
    still updates dedupe state so we don't re-evaluate every tick
  - settings.json.template documents the shape inline
  - settings.json.docker reads MAIL_HOST / MAIL_FROM / MAIL_PORT /
    MAIL_SECURE from env so operators can configure via container env

Transport
  - lazy import('nodemailer') on first send
  - transport cached by host; settings reload picks up new host without
    needing a restart
  - send errors are swallowed (logged warn) so a transient SMTP failure
    can never poison the surrounding updater state machine
  - successful sends log at info; legacy "(would send email)" path
    remains the visible signal when mail is disabled

Refs #7607

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

* feat(updater): preflight checks target tag's engines.node (#7607)

Before mutating the working tree, runPreflight now reads the target tag's
package.json via `git show <tag>:package.json` and verifies that
process.versions.node satisfies its engines.node range. Failures land at
preflight-failed cleanly (no rollback needed — nothing has changed yet).

Motivation: a release that bumps the Node floor used to either fail
mid-`pnpm install` (which then rolls back successfully) or restart on the
new build and crash in the boot path (which then rolls back via the
health-check timer). Both paths recover, but they burn a drain + restart
cycle on a condition we can reject upfront.

Implementation
  - new PreflightReason `node-engine-mismatch`
  - new dep `readTargetEnginesNode(tag)` — runs the git-show as a child
    process with stdio captured to a string; missing tag / missing file /
    malformed JSON / missing engines.node all resolve to null (treated as
    "no constraint, pass")
  - uses existing semver dep with includePrerelease: true
  - new PreflightInput field `currentNodeVersion`; threaded from
    process.versions.node in both wirings (scheduler + manual apply)
  - check runs *after* signature verification so we trust the package.json
  - PreflightResult carries an optional `detail` string; applyPipeline
    appends it to the lastResult.reason so the admin UI shows e.g.
    "node-engine-mismatch: target requires Node >=26.0.0, running 25.0.0"

Tests: 6 new vitest cases (no engines.node, satisfies, fails below floor,
caret range, loose-spaced range, ordering after signature). Full
backend-new: 635 passed (was 629).

Refs #7607

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

* feat(updater): email admin on auto-rollback / preflight-failed (#7607)

Before this commit, only the terminal rollback-failed state emailed the
admin. Auto-recovered failures (rolled-back-install-failed, rolled-back-
build-failed, rolled-back-health-check, rolled-back-crash-loop) and pre-
flight-failed surfaced only via the /admin/update banner — so a 3am
autonomous update that failed because of, say, a Node engine bump would
roll back silently and stay invisible until the admin next logged in.

Notifier
  - new EmailKinds: 'update-preflight-failed', 'update-rolled-back',
    'update-rollback-failed'
  - new pure decideOutcomeEmail(input) → {toSend, newState}
  - dedupe key `<outcome>:<targetTag>` in EmailSendLog.lastFailureKey:
    same outcome on same tag emits one email per cycle (kills retry-loop
    spam); a different outcome or different tag resets the key
  - rollback-failed always fires (terminal — overrides dedupe)
  - state.ts validator + loadState backfill the new field for legacy
    state files (Tier 1/2/3 installs upgrading in place)

Wiring
  - new index.ts helper notifyApplyFailure() loads state, runs the pure
    notifier, sends (via the nodemailer-backed sendEmailViaSmtp from the
    previous commit), persists the new dedupe key — all best-effort
  - schedulerTriggerApply: fires on applyUpdate returning preflight-failed
    or rolled-back
  - /admin/update/apply HTTP handler: same
  - boot path in expressCreateServer: if state.lastResult is a failure
    outcome we haven't already emailed about, fire then. Covers:
      - health-check timeout rollback (timer expired between boots)
      - crash-loop forced rollback caught on a later boot
      - preflight-failed where the process didn't get to email before exit
      - unacknowledged rollback-failed terminal

Tests
  - 8 new vitest cases for decideOutcomeEmail (adminEmail=null, each
    outcome's content, dedupe by tag, dedupe by outcome, rollback-failed
    bypass)
  - Full backend-new suite: 643 passed (was 635)

Refs #7607

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

* fix(updater): address Qodo review on tier 4

- UpdatePage: only show "deferred until" subtitle when scheduledFor
  actually matches nextWindowOpensAt. The previous `scheduledFor >
  now + 60s` heuristic misfired during a normal in-window 15-min
  grace period.
- applyPipeline: return the enriched preflight reason (`reason:
  detail`) instead of only `pf.reason`, so /admin/update/apply 409
  bodies and failure-notify emails preserve diagnostics like the
  Node engine mismatch detail.
- updater/index: key the cached nodemailer transport on the full
  set of SMTP options (host + port + secure + auth) so runtime
  changes to port/credentials via reloadSettings() invalidate
  the cache.

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-17 13:50:34 +01:00
John McLear
278acb10cb
Drop swagger-ui, document telemetry, add opt-outs (#7524) (#7757)
* docs: design spec for #7524 drop swagger-ui + privacy opt-outs

Three-deliverable plan: vendor RapiDoc to replace swagger-ui-express
(Scarf-injecting), add privacy.updateCheck and privacy.pluginCatalog
opt-outs for our two outbound calls, and ship PRIVACY.md as a public
stance doc.

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

* docs: implementation plan for #7524 swagger-ui + privacy opt-outs

Twelve TDD-flavoured tasks: privacy settings shape, UpdateCheck +
installer opt-outs (each with a failing-test-first cycle), admin
backend/UI plumbing, dependency drop, vendored RapiDoc, PRIVACY.md,
final verification matrix.

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

* feat(privacy): add privacy block to settings shape

Adds privacy.updateCheck and privacy.pluginCatalog, both defaulting to
true so behavior is unchanged until operators opt out.

Refs #7524

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

* feat(privacy): honour privacy.updateCheck=false in UpdateCheck

check() and getLatestVersion() now early-return when the setting is
off. Logs once on first skip. The admin "update available" panel
already tolerates an undefined latestVersion.

Refs #7524

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

* feat(privacy): honour privacy.pluginCatalog=false in installer

Extracts the gate into pluginCatalogGuard.ts so it can be unit-tested
under vitest without dragging in the CJS require() chain from
installer.ts. getAvailablePlugins() now throws the tagged disabled
error before any fetch.

Refs #7524

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

* feat(privacy): emit results:catalogDisabled when pluginCatalog off

Short-circuits the four catalog-driven socket events. The install/
uninstall events are untouched so operators can still install by
plugin name even when the catalog is disabled.

Refs #7524

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

* fix(bin): stalePlugins reads updateServer and honours privacy flag

Was hardcoding static.etherpad.org and ignoring opt-out. Now exits 0
cleanly when privacy.pluginCatalog=false.

Refs #7524

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

* docs(settings): document privacy block in settings template

Refs #7524

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

* feat(api-docs): replace swagger-ui-express with RapiDoc shell

Drops the swagger-ui-express dep (third-party Scarf telemetry pixel,
see swagger-api/swagger-ui#10573) and serves /api-docs with a static
HTML shell that mounts <rapi-doc>. /api-docs.json is unchanged.

The vendored RapiDoc asset is added in the next commit so the tree is
broken for one diff hunk — pair this with the rapidoc-min.js commit
during review.

Refs #7524

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

* feat(api-docs): vendor RapiDoc 9.3.4 (MIT) as static asset

Pinned bundle with checksum in VERSION. Replaces swagger-ui-dist which
shipped a Scarf telemetry pixel.

Disables RapiDoc's bundled Google Fonts request via load-fonts="false"
plus explicit regular-font/mono-font system stacks — RapiDoc's CSS
@font-face rules would otherwise fetch Open Sans from fonts.gstatic.com
at render time.

Also fixes the /api-docs route's res.sendFile to use an absolute path
resolved via settings.root (the previous {root: 'src/static'} was
resolved from CWD which is already src/, producing src/src/static).

Refs #7524

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

* feat(admin): banner when plugin catalog is disabled

Subscribes to results:catalogDisabled and renders a localized info
banner on the plugins page. install/uninstall still function via CLI.

Refs #7524

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

* docs: PRIVACY.md and README/CHANGELOG pointers

Publishes Etherpad's stance on telemetry: two documented, opt-out
outbound calls; no third-party analytics; no install-time phone-homes
in our deps.

Refs #7524

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

* fix(admin): await checkPluginForUpdates and emit array on error

Qodo flagged that checkUpdates emitted the unresolved Promise (missing
await) and emitted {} for updatable on the error path, both breaking
the admin UI's expected string[] shape. Pre-existing bug surfaced when
the surrounding block was edited for the privacy.pluginCatalog gate.

Refs #7524

* feat(api-docs): swap RapiDoc for Scalar (actively maintained)

Per @SamTV12345's review on #7757: RapiDoc has been effectively
unmaintained for a while. Scalar (https://github.com/scalar/scalar)
is MIT-licensed, actively developed, and ships a self-contained
standalone bundle that works the same way for our purposes.

Privacy posture is preserved by configuring the embed:
  - withDefaultFonts: false   (no fonts.scalar.com woff2 fetch)
  - telemetry: false          (defensive)
  - agent.disabled: true      (no api.scalar.com/vector/* calls)
  - mcp.disabled:   true      (no MCP integration)
  - showDeveloperTools: 'never'
  - hideClientButton: true

Verified with headless Chromium: page loads /api-docs, mounts Scalar,
renders the Etherpad OpenAPI document, and makes zero requests to
any host other than localhost.

Vendor:
  - src/static/vendor/scalar/standalone.js     (@scalar/api-reference 1.57.2)
  - src/static/vendor/scalar/VERSION           (sha256 pinned)
  - src/static/vendor/scalar/LICENSE           (MIT)
Removed:
  - src/static/vendor/rapidoc/*

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-16 18:34:22 +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
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
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
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
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
7b9a5eb01a
fix(a11y): dialog semantics, focus management, icon labels, html lang (#7584)
* fix(a11y): negotiate lang/dir per request and set on <html>

Server-renders the html element with `lang` and `dir` matching the
client's Accept-Language header (negotiated against availableLangs from
i18n hooks). Falls back to `en`/`ltr` if no match.

This gives screen readers a correct document language during the brief
window before client-side html10n refines it (l10n.ts already sets both
attributes after locale data loads).

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

* fix(a11y): dialog semantics on popups; fix aria-role typo on userlist

Adds role=dialog, aria-modal=true, and either aria-labelledby (when an
h1 is present) or aria-label (for popups without an h1) to:

  - #settings, #import_export, #embed, #skin-variants (labelledby)
  - #connectivity, #users, #mycolorpicker (aria-label)

Fixes the invalid aria-role="document" attribute on #otherusers; it's
now role=region with aria-live=polite so screen readers announce
collaborator joins/leaves.

Container aria-label values are English-only for now — Etherpad's
html10n implementation only supports localizing specific attributes
(title, alt, placeholder, etc), not aria-label on container nodes.
Localization can follow once html10n grows that affordance.

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

* fix(a11y): focus management and Escape-to-close for popups

Three additions to toggleDropDown / _bodyKeyEvent:

  - Remember the trigger element (document.activeElement) when opening
    a popup, so we can restore focus when it closes.
  - On open, focus the first focusable element inside the popup so
    keyboard users land inside the dialog instead of staying on the
    trigger button.
  - Escape pressed while focus is inside a popup closes it, then the
    restore-focus path runs and the trigger button is refocused.

Replaces the previous behavior where Escape from inside a popup did
nothing; users had to click outside to dismiss.

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

* fix(a11y): make chaticon and chat header controls real buttons

- #chaticon: <div onclick> → <button type=button> with aria-label
- #titlecross / #titlesticky: <a onClick> → <button type=button>
  with aria-label (Close chat / Pin chat to screen)
- Decorative chat-bubble glyph gets aria-hidden=true so it isn't
  read alongside the button label
- #chatcounter labelled "Unread messages"
- Inline onclick attributes moved to chat.init() handlers
- CSS reset on the new buttons (transparent bg, no border, inherit
  font/color) so they match the prior visual design
- :focus-visible outlines for keyboard users

Existing test selectors (#chaticon, #titlecross, #titlesticky) are
unchanged and continue to work — they never relied on element type.

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

* fix(a11y): accessible names for icon-only toolbar/export controls

- Export links (#exportetherpada, #exporthtmla, #exportplaina,
  #exportworda, #exportpdfa, #exportopena): added aria-label so the
  link is announced as e.g. "Export as PDF". The inner icon span
  gets aria-hidden=true so screen readers don't read both the icon
  text and the link label.

- Show-more toolbar toggle (.show-more-icon-btn): converted from
  <span> to <button type=button> with aria-label and aria-expanded.
  The click handler now toggles aria-expanded alongside the
  full-icons class so assistive tech reflects the open/closed state.

- Theme switcher knob: aria-label changed from "theme-switcher-knob"
  (a class-style identifier, not human text) to "Toggle theme".

Aria-label values are English-only for now. Etherpad's html10n
implementation only localizes a fixed attribute list (title, alt,
placeholder, value, innerHTML, textContent); aria-label is not
included, so a clean l10n path requires a follow-up to either
extend html10n or set aria-label client-side after locale loads.

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

* test(a11y): cover dialog semantics, html lang, icon button labels

New Playwright spec verifies the a11y guarantees added by this branch:

  - <html> has a non-empty lang attribute
  - settings/import_export/embed/users popups expose role=dialog,
    aria-modal=true, and either aria-labelledby (when an h1 exists)
    or aria-label (when none does)
  - Escape from inside the settings popup closes it AND restores
    focus to the trigger button
  - Export links each carry a descriptive aria-label
  - #chaticon is a real <button> with aria-label
  - #titlecross / #titlesticky are real <button>s with aria-label
  - #otherusers uses role=region + aria-live=polite + aria-label
    (and the previous aria-role typo is gone)
  - .show-more-icon-btn is a <button> with aria-label and
    aria-expanded

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

* fix(a11y): address Qodo review feedback from PR #7584

1. Users Escape close broken - toggleDropDown('none') intentionally
   skips the users module so switching between other popups doesn't
   hide the user list. That meant Escape couldn't dismiss the Users
   popup either. The Escape branch now checks for #users as the
   focused popup and closes it explicitly (respecting stickyUsers)
   before falling through to the normal close-all path.

2. Embed focus overridden - the rAF auto-focus in toggleDropDown
   grabbed the first focusable descendant, which stole focus from
   command handlers that target a specific control (notably the Embed
   command's #linkinput). rAF now bails out if focus is already
   inside the newly-opened popup.

3. Button click blurs :focus before toggleDropDown captures trigger -
   discovered while investigating the Firefox Playwright failure for
   "settings popup Escape restores focus". Button.bind() calls
   $(':focus').trigger('blur') before invoking the callback, so by
   the time toggleDropDown() captured document.activeElement as the
   restore target it was already <body>. The click handler now
   stashes padeditbar._lastTrigger to the clicked <button> before
   blur runs; toggleDropDown only falls back to activeElement when
   the pre-stash didn't happen (keyboard shortcut path).

4. html10n overwrites aria-label - html10n unconditionally set
   aria-label to the translated string, clobbering explicit aria-label
   on elements that also carry data-l10n-id. setAttribute now only
   fires when the element has no aria-label; explicit author labels
   win, unlabelled translated elements still get a name.

5. Button visual reset - the show-more-icon-btn and #chaticon
   conversions inherited UA default button border/background/padding,
   shifting icon glyphs visibly off-centre. Added appearance /
   background / border / padding resets.

6. Export links test assumes soffice is installed - #exportworda,
   #exportpdfa, #exportopena are removed client-side by pad_impexp.ts
   when clientVars.exportAvailable === 'no'. The test now skips links
   absent at runtime.

Verified locally: all 10 a11y_dialogs specs pass on both Chromium and
Firefox; backend suite remains 799/799 passing; ts-check clean.

* fix(a11y): close popups with no focusable content; unbreak chat-icon layout

Round 2 of #7584 review follow-ups.

1. Users popup Escape still didn't close the dialog (user-confirmed).
   Root cause: _bodyKeyEvent is bound to the OUTER document's body.
   When #users opens, the command handler tries to focus
   #myusernameedit but that input is `disabled`, so focus stays in the
   ace editor iframe. Keydown from inside the iframe does not bubble
   to the outer document, so Esc never reaches _bodyKeyEvent.
   Fix: in the open-popup rAF, if no command handler placed focus
   inside the dialog, focus the popup div itself (with tabindex=-1).
   That keeps subsequent keydown events on the outer document so
   Esc can dismiss the popup. Also broadened the Esc branch to fire
   whenever any popup is `.popup-show`, regardless of where :focus
   lives — some popups legitimately have no focusable content at
   open.
   Added a regression test that opens #users and asserts Esc closes
   it. Passes on both Chromium and Firefox.

2. Chat icon (#chaticon) visual still wrong after the first CSS fix.
   - My previous `border: 0` reset was overriding the intended
     `border: 1px solid #ccc; border-bottom: none` from the earlier
     rule. Removed `border: 0`; the earlier explicit border suffices
     to suppress UA defaults.
   - The `<span class="buttonicon">` inside `#chaticon` was picking
     up the global `.buttonicon { display: flex; }` rule meant for
     toolbar button instances, which broke the inline layout of the
     label + glyph + counter row. Added a scoped
     `#chaticon .buttonicon { display: inline; }` override.

All 11 a11y_dialogs specs pass on Chromium and Firefox. Backend
suite and ts-check remain clean.

* fix(a11y): only stash _lastTrigger for dropdown-opening buttons

Round 3 follow-up. The previous Button.bind() change stashed every
clicked toolbar button as padeditbar._lastTrigger before blurring :focus.
That was necessary for popup-opening buttons (settings, import_export,
etc.) so Escape could return focus to them — but it also fired for
non-popup toolbar buttons (list toggles, bold/italic, indent/outdent,
clearauthorship). For those, the stash held a stale reference that
interfered with subsequent editor interactions and regressed Playwright
tests: ordered_list, unordered_list, undo_clear_authorship.

Fix: only stash when the clicked command is a registered dropdown
(settings, import_export, embed, showusers, savedrevision,
connectivity). Other commands return focus to the ace editor as before
and leave _lastTrigger alone.

Verified locally on Chromium:
  - ordered_list.spec.ts: 6/6 pass (was 4/6)
  - unordered_list.spec.ts: 6/6 pass (was 4/6)
  - undo_clear_authorship.spec.ts: 2/2 pass (was 0/2)
  - a11y_dialogs.spec.ts: 11/11 pass (unchanged)

* fix(a11y): address Qodo review round 4 for PR #7584

#1 Stale aria-label after relocalize
  html10n.translateNode() refused to overwrite any existing aria-label,
  which also skipped updates on language change (pad.applyLanguage()
  re-runs localize). Use a `data-l10n-aria-label="true"` marker: set
  aria-label + marker when html10n populates it, overwrite only if the
  marker is present. Explicit template-supplied aria-labels stay as-is;
  html10n-generated ones refresh on relocalize.

#2 Escape won't close colorpicker
  _bodyKeyEvent caught Escape on any `.popup.popup-show` but only
  closed dropdown popups via toggleDropDown('none'). Popups opened
  outside the editbar framework (#mycolorpicker, toggled directly by
  pad_userlist.ts) stayed open while preventDefault() swallowed the
  key. Now the Escape branch manually closes any popup that
  toggleDropDown('none') cannot reach (non-dropdown ids, plus #users
  unless pinned) and leaves registered dropdowns for toggleDropDown to
  close so its focus-restore sees the transition.

#3 Stale focus restoration
  toggleDropDown('none') restored focus to _lastTrigger even when no
  popup was open on entry, which meant background callers
  (connectivity setup, periodic state handling) could yank focus out
  of the editor to a stale toolbar button. Gated the restore on
  `wasAnyOpen === true` so it only fires when there was a popup to
  close.

#11 English aria-label overrides i18n (export links, chat icon)
  Removed the hard-coded English aria-label from export anchors and
  removed aria-hidden from their inner localized spans. Screen readers
  now get the localized child text as the accessible name (Etherpad,
  HTML, PDF, etc.), matching the visible UI language.
  Removed the English aria-label from #chaticon and #titlesticky as
  well — both have data-l10n-id, so html10n populates a localized
  aria-label via the marker mechanism in #1. #titlecross keeps its
  static aria-label because it has no data-l10n-id yet.

#4 4-space indent in a11y spec
  Two tests had continuation lines at 4-space indent violating the
  repo's 2-space rule. Folded the signatures onto one line.

Updated a11y_dialogs.spec.ts to assert accessible-name presence rather
than hard-coded English for elements whose names now come from the
localized text. Still asserts static English for #titlecross (not
localized yet).

Verified locally (dev server restarted for each round):
  - a11y_dialogs.spec.ts: 11/11 on Chromium, 11/11 on Firefox
  - ordered_list + unordered_list + undo_clear_authorship: 13/13 on Chromium
  - Full backend suite: 799 passing, 0 failing
  - tsc --noEmit clean in our code

#9 Popup behavior documentation: deferred to a follow-up doc PR so
this PR stays focused on the a11y code changes. The new keyboard
behavior (Escape-to-close, focus-restore-to-trigger) is small enough
to summarize in a short doc/ addition.

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-24 03:04:18 +01:00
John McLear
053f6d8343
fix(#7570): bundle DB drivers, add regression CI (#7572)
* docs: design spec for issue #7570 (ueberdb2 driver bundling)

Spec for the upstream ueberDB fix (move 10 drivers back from optional
peer deps to dependencies) plus downstream etherpad-lite safety net
(explicit driver list + build-test-db-drivers CI job covering all 10
via presence check and MySQL+Postgres smoke tests).

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

* docs: implementation plan for issue #7570 ueberdb2 driver bundling

Covers upstream ueberDB PR (move drivers from optional peer deps back
to dependencies, publish 5.0.46) and downstream etherpad-lite PR
(bump ueberdb2, defensive driver list, build-test-db-drivers CI job
with presence + MySQL + Postgres stages gating publish).

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

* fix(#7570): bundle DB drivers, add regression CI

- Bump ueberdb2 to ^5.0.47 (upstream ueberDB PR #939 re-bundles drivers
  as real dependencies instead of optional peer deps, fixing the class
  of Docker-prod "Cannot find module" failures).
- Declare all 10 ueberdb2 DB drivers as direct src dependencies as a
  defensive safety net against a future upstream drift.
- Add build-test-db-drivers CI job that blocks the publish job:
    * all-10-drivers presence check in the built prod image
    * end-to-end MySQL smoke (reproduces the #7570 repro)
    * end-to-end Postgres smoke
  Any stage failure blocks Docker Hub / GHCR publish.

Supersedes #7571.

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

* fix(ci): run driver presence test from src/ so node_modules resolves

The presence test ran node from the default cwd (/opt/etherpad-lite),
but the drivers are installed under /opt/etherpad-lite/src/node_modules
by the monorepo workspace. Adding `-w /opt/etherpad-lite/src` makes
Node resolve modules from src/node_modules where pnpm places them.

Matches how the production container itself runs: `pnpm run prod` is
invoked from src/ (cross-env + node --require tsx/cjs node/server.ts).

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-20 18:10:01 +01:00