mirror of
https://github.com/ether/etherpad-lite.git
synced 2026-07-18 00:57:55 +00:00
2083 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
John McLear
|
b19ad89eb0
|
Fix offline/air-gapped Docker boot: stop pnpm self-provisioning the pinned version (#7911) (#7918)
* fix(docker): don't let pnpm self-provision a pinned version on offline boot (#7911) The official Docker image installs pnpm directly via npm (corepack was dropped for Node 25+). Standalone pnpm still honours the "packageManager" pin in package.json: the image's pnpm intentionally lags that pin (pnpm 11.1.x enforces a minimum-release-age policy the frozen-lockfile build can't satisfy), so pnpm treats every invocation — including the informational `pnpm --version` probe Etherpad runs at startup — as a request to download and run the pinned build. Behind a corporate firewall / in an air-gapped install that download fails: [WARN] plugins - Failed to get pnpm version: Error: Command exited with code 1: pnpm --version which is what #7911 reported. Fix — neutralise the gap instead of closing it (closing it would break the frozen-lockfile build on 11.1.x): - Dockerfile build stage sets `pnpm_config_pm_on_fail=ignore` (the pnpm 11 successor to managePackageManagerVersions), inherited by the development and production runtime stages. pnpm then uses the installed pnpm instead of fetching the pinned one. It does not change which pnpm runs the build-time install, so the frozen-lockfile build is unaffected. - plugins.ts startup probe and the updater's pnpm-on-PATH checks run with the same flag, so the fix also covers non-Docker offline installs and the probe can never fail-loud. Add a backend spec that fails CI if the offline guard is dropped while the image pnpm differs from the package.json pin. Verified with a standalone (non-corepack) pnpm: a "packageManager" mismatch makes `pnpm --version` exit 1 by default (tries to fetch the pinned build), and exit 0 reading the local version with pm_on_fail=ignore. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: scope pnpm offline-guard check to the runtime-inherited build stage Address Qodo review: the regression spec matched ENV pnpm_config_pm_on_fail anywhere in the Dockerfile, so it would still pass if the guard were removed from the `build` stage (which the runtime stages inherit) but left in the throwaway `adminbuild` stage — reintroducing the offline failure. Extract the `build` stage block and assert the ENV is present there specifically. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
John McLear
|
ef205bf2ac
|
fix(theme-color): dark address bar + no light-flash on dark-OS load (#7606) (#7909)
* fix(theme-color): emit media-scoped dark variant for iOS Safari (#7606) The theme-color meta only had a single light value rendered server-side; dark mode was applied purely by JS (skin_variants.ts) after page load. iOS Safari colors the address bar at parse time and does not reliably repaint when JS mutates the meta later, so dark-mode iPhone users kept a white address bar above a dark toolbar (the green Chromium Playwright test masked this because Chrome does honor the dynamic update). Emit a prefers-color-scheme media-scoped pair server-side so the correct color is chosen at first paint without JS: <meta name="theme-color" content="#ffffff" media="(prefers-color-scheme: light)"> <meta name="theme-color" content="#485365" media="(prefers-color-scheme: dark)"> - Add SkinColors.darkToolbarColor() (reuses toolbarColorForTokens). - Expose enableDarkMode via getPublicSettings so the templates can gate the dark variant on it (no dark variant when dark mode can't be reached). - Apply to both pad.html and timeslider.html. - updateThemeColorMeta now updates every theme-color meta so a manual #options-darkmode toggle still wins over the media scoping on desktop/Android. - Backend + frontend tests updated to assert the media-scoped pair and the enableDarkMode-off case. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(theme-color): prevent the light-mode flash on dark-OS load (#7606) The theme-color meta fix corrects the address-bar tint, but dark-OS users still saw the whole page painted light before the JS bundle ran and applied the dark skin classes in postAceInit — a visible flash on every browser, not just the mobile address bar. Add a tiny blocking inline script in <head>, before the stylesheet, that applies the dark skin classes to <html> synchronously during parse when the client is in dark mode (matchMedia + no localStorage white-mode override). The condition mirrors pad.ts's auto-switch, which still runs on init to wire up the #options-darkmode toggle and theme the editor iframes (those don't exist yet at parse time). Gated on the same enableDarkMode + colibris check as the dark theme-color variant. Applied to pad.html and timeslider.html. Verified in Chromium: at domcontentloaded a dark-OS client's <html> already carries super-dark-editor/dark-background/super-dark-toolbar (no flash), and a light-OS client is unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(changelog): note the dark-mode address-bar + flash fix (#7606) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(theme-color): guard pre-paint script on #skinvariantsbuilder; ignore updater state Address PR review: - Copilot: the inline pre-paint dark-mode script must skip the auto-dark switch on the #skinvariantsbuilder hash, matching pad.ts — otherwise it forces super-dark classes on a dark-OS client and fights the variants builder UI. Added the guard to pad.html and timeslider.html and a backend assertion so it can't regress. - Qodo: ignore var/update-state.json (runtime updater cache) so the server run that regenerates it can't dirty the tree or be committed accidentally. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
John McLear
|
7ea9970648
|
Hardening: API request handling, random IDs, and plugin loading (#7906)
* Hardening: API request handling, token generation, and plugin loading
- pad_utils.randomString: generate the random IDs via crypto.getRandomValues
(CSPRNG) instead of Math.random.
- OAuth2Provider: constant-time password comparison and a uniform failure delay
on the OIDC interaction login; own-property user lookup.
- API.appendChatMessage: require the pad to already exist (getPadSafe),
consistent with the other content API methods.
- RestAPI /api/2: forward only the authorization header rather than merging all
request headers into the API field set.
- LinkInstaller: validate plugin dependency names before building filesystem
paths from them.
- admin file server: return a generic error message and log details server-side.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Address review: timingSafeEqual on raw bytes; align /api/2 auth fallback
- OAuth2Provider.constantTimeEquals: compare raw UTF-8 bytes with
crypto.timingSafeEqual instead of hashing them first. Resolves the CodeQL
"password hash with insufficient computational effort" alert while keeping a
content-independent comparison (length difference is covered by the uniform
failure delay).
- RestAPI /api/2: fall back to the authorization header whenever the field is
falsy (not only null), matching the openapi.ts handler so the two routers
authenticate identically (Qodo).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* appendChatMessage: throw explicit error instead of getPadSafe (review)
Per review: replace the getPadSafe(padID, true) existence check with an
explicit `throw new CustomError('padID does not exist', 'apierror')` so chat
messages can't create pads, without fetching the pad.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
John McLear
|
86c56cf827
|
Escape exported data attributes and warn on default/placeholder credentials (#7905)
* Escape exported data-* attributes; require explicit deploy credentials - ExportHtml: escape the name and value of attributes emitted by the exportHtmlAdditionalTagsWithData hook, consistent with the URL/text escaping already applied when generating exported HTML. - docker-compose: require ADMIN_PASSWORD and the database password to be set explicitly (no default fallback); default TRUST_PROXY to false. - Settings: log a warning (error level in production) when an account uses a default/placeholder password from the shipped config. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Settings: also warn on default OIDC client secrets Extend the placeholder-credential check to cover sso.clients[].client_secret, so a deployment that enables SSO without setting ADMIN_SECRET / USER_SECRET is flagged (error level under NODE_ENV=production) the same way default account passwords are. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Move docker-compose changes to a separate PR The deployment-default changes (required credentials, TRUST_PROXY) are being discussed separately; this PR keeps only the non-breaking export escaping and credential-warning changes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
John McLear
|
3e6f9d7bb2
|
fix(pad): attribute default welcome text to the system author (#7885) (#7887)
* fix(pad): attribute default welcome text to the system author (#7885) When a user opens a brand-new pad, CLIENT_READY calls getPad(padId, null, session.author), and Pad.init attributed the auto-generated default content (settings.defaultPadText or a padDefaultContent hook substitution) to that author. The welcome text — which the user never wrote — therefore carried the creator's `author` attribute and rendered in their authorship colour. Track whether the initial text came from the default-content path and, if so, attribute the initial changeset to the stable system author (Pad.SYSTEM_AUTHOR_ID) instead of the creating user. Explicitly provided text (e.g. HTTP API createPad with text + author) keeps the real author. The creating user becomes a listed author only once they actually type; their "ownership" of the pad (pad-wide settings defaults, the author token) does not depend on owning the default text, so it is unaffected. listAuthorsOfPad already filters out the system author, so the public API reports zero contributors for an untouched default pad. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(pad): keep creator as revision-0 author, only de-colour the text CI (socketio "Pad-wide settings creator gate") and Qodo both caught that attributing revision 0 to the system author stripped the creating user's ownership: isPadCreator() / the pad-wide settings gate and the deletion token all key off getRevisionAuthor(0). Decouple the two: the initial revision's meta.author stays the real creator (ownership preserved), while only the welcome text's `author` *attribute* — the thing that colours it — becomes Pad.SYSTEM_AUTHOR_ID. The system fallback for the revision author now applies solely when no author was supplied at all. Tests assert both halves: default text is system-coloured (not the creator's colour) AND the creator remains the revision-0 author; explicit text stays coloured with the creator. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(wcag): target the user-authored span, not the system welcome text The wcag_author_color spec picked the first `span[class*="author-"]` on the page to measure the user's colour contrast. With #7885 the default welcome text is now owned by the system author and renders with no background colour, so that first span is no longer the current user's — the contrast read came back as transparent rgba(0,0,0,0) and the three assertions failed. Match the author span by the text we just typed ("contrast smoke") instead, so the test measures the actual user-authored content it intends to. Verified locally: 3/3 pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
John McLear
|
d582398826
|
fix: eliminate the Windows backend silent-ELIFECYCLE flake (handler gate + Node 24.16.0) (#7866)
* fix(test): stop a single leaked promise rejection from killing the whole backend suite
Root cause of the long-standing Windows backend-test "silent ELIFECYCLE"
flake (~22% of runs, rotating across random spec files, no mocha summary,
no JS-handler trace, bypassing --report-on-fatalerror / Defender / Windows
event log / AeDebug). Found by capturing a full-memory dump of the dying
node.exe with Sysinternals ProcDump (-t dump-on-terminate) and symbolizing
it against Node 24.15.0's node.pdb. The dying thread's stack:
exit_or_terminate_process / common_exit (CRT exit)
node::Exit
node::DefaultProcessExitHandlerInternal
node::Environment::Exit
node::ReallyExit (process.exit binding)
... v8 MicrotaskQueue::RunMicrotasks ...
node::InternalCallbackScope::Close
No exception stream — a *clean* ExitProcess, not a crash. The job log
pinned the trigger:
[INFO] server - Exiting...
AssertionError at tests/backend/specs/SessionStore.ts:235
at process.processTicksAndRejections
Mechanism: a timing-fragile test (SessionStore touch/expiry specs use real
setTimeout against a 200ms-expiry session; socket.io delay-race specs are
similar) gets timed out and abandoned by mocha, but its async body keeps
running. When its trailing assertion later throws, it surfaces as an ORPHAN
unhandled rejection belonging to no awaited test. Three handlers then
escalated that into a whole-process exit:
- server.ts installed process-global uncaughtException/unhandledRejection
handlers that call exports.exit() → process.reallyExit() (production
graceful-shutdown behaviour, catastrophic in-process under mocha)
- common.ts (PR #7663) and diagnostics.ts (PR #7838) rethrew the rejection
and process.exit(1)
Because it's a deliberate, clean exit it bypassed every forensic layer; it
rotated across files because the orphan rejection lands during whatever test
is running; it's Windows-mostly because event-loop timing makes the abandoned
test's assertion fire in a *later* test's window more often there.
Fix (two halves):
1. server.ts: gate the process-global uncaughtException / unhandledRejection
/ signal handlers behind `require.main === module`. They are correct for
a real Etherpad process but must not fire when server.start() is called
in-process by a test runner — mocha owns process-level error handling
there. Mirrors the existing `if (require.main === module) exports.start()`
idiom; production (node server.js) is unchanged.
2. common.ts + diagnostics.ts: the backend-test bootstraps now LOG unhandled
rejections instead of rethrowing / exiting. Orphan rejections cannot be
cleanly attributed to a test, so rethrowing only yields an
ERR_MOCHA_MULTIPLE_DONE abort. Real failures are unaffected — an assertion
in a test's own awaited path rejects that test's promise and mocha fails
it normally, never reaching this global handler.
Verified locally: a spec that leaks a delayed rejection during a later test
now reports `3 passing` / exit 0 with the rejection logged, instead of
aborting the run.
Follow-ups (separate PRs): harden the SessionStore / socket.io timing specs
to not leak (fake timers); remove the now-unneeded diagnostic scaffolding
(diagnostics.ts heartbeat/node-report, the #7846 OS sidecar) now that the
cause is known.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(ci): run Windows backend tests on Node 25 to dodge the libuv connect overrun
Node 24.x's bundled libuv has a stack buffer overrun in the Windows TCP-connect
path (uv__tcp_connect / uv__tcp_try_connect), proven by a SilentProcessExit
full-memory dump of the dying mocha process: the main thread executes
__fastfail(FAST_FAIL_STACK_COOKIE_CHECK_FAILURE) from __report_gsfailure with
TCPWrap::Connect -> uv_tcp_connect on the stack. It fires under the backend
suite's heavy localhost connection churn, is address-family independent (occurs
on both sockaddr_in and sockaddr_in6, so an IPv4 pin does NOT help), and -- being
memory corruption -- bypasses all JS/Node observability, rotating across tests
as the "silent ELIFECYCLE" flake (~22% of Windows runs).
Empirically: Node 25 = 16/16 green; Node 24 (even with an IPv4 pin) = ~39% fail.
Node 25's newer bundled libuv does not overrun. Linux stays on Node 24 LTS (the
bug is Windows-specific). Revisit once the libuv fix is backported to 24.x.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(ci): pin Windows backend to Node 24.16.0 (libuv fix) instead of 25
Bisect (standalone repro) pinpointed the fix to Node 24.16.0 (libuv 1.52.1):
24.15.0 (libuv 1.51.0) crashes the connect overrun 4/4 on 127.0.0.1, while
24.16.0 is clean 0/8. 24.16.0 stays on the Node 24 "Krypton" LTS line, so prefer
it over Node 25 (non-LTS). Pinned explicitly because setup-node's default
check-latest:false reuses the runner's pre-cached 24.15.0 for a bare "24".
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(ci): reference upstream nodejs/node#63620 in the Windows Node-pin comment
Links the explicit 24.16.0 pin to the filed upstream issue so the pin can be
dropped back to plain "24" once the libuv connect-overrun fix is across the
supported 24.x baseline.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
Андрій Шовкошитний
|
eeac0a6703
|
fix(oidc): fix OIDCAdapter broken flows (#7837)
* fix(oidc): fix OIDCAdapter broken flows * fix(oidc): fix storage type to include string for userCode index * test(oidc): add regression tests for OIDCAdapter broken flows |
||
|
John McLear
|
79e3d46127
|
feat(settings): default settings.enablePluginPadOptions to true (#7841)
This flag gates the ep_* passthrough on padoptions that shipped in 3.0.0 (PR #7698). It was introduced as opt-in, but the intent in shipping it was to let plugins like ep_plugin_helpers' padToggle / padSelect ride the existing broadcast/persist rail out of the box — flipping the default closes the gap. Why now - ep_comments_page#422 (and sibling per-plugin reports discussed on Discord): stock 3.x deployments console.warn on every pad load because the helper detects clientVars.enablePluginPadOptions === false and tells the admin to flip it. With the flag default-true, the warning stops firing on fresh installs while still surfacing for operators who have explicitly opted out. - Plugins that already depend on ep_plugin_helpers >= 0.6 expect the pad-wide path to work; the default-false gate silently no-op'd pad.changePadOption('ep_*', …) and made the helper UI inert. Scope - Settings.ts default flipped to true; comment rewritten to describe the new "operator opt-out" model rather than the old AGENTS.MD §52 opt-in framing (that policy still applies to *new* features; this one has shipped and proven safe). - settings.json.template env-var substitution default flipped to true so docker / supervisor configs without an explicit value get the new behavior. - doc/plugins.md updated to match (default true, opt-out via settings.json) and the PluginCapabilities source comment. - Backend test describe-blocks relabeled — "true" is now "(default)", "false" is now "(operator opt-out)". Both branches still cover the same matrix so the size-cap / namespace-validation paths stay exercised. Compat - Existing deployments with an explicit `"enablePluginPadOptions": false` in settings.json keep that value — no migration needed. - Older clients only read clientVars.enablePluginPadOptions; the protocol shape is unchanged. Closes ep_comments_page#422 (helper warning suppression for stock deployments). Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
John McLear
|
d9dabe352a
|
feat(admin): explain env-var substitution in /settings, surface auth errors (#7819) (#7826)
* feat(admin): explain env-var substitution in /settings, surface auth errors (#7819) Three small, env-var-only UX improvements driven by issue #7819, where a Docker operator saved an ep_oauth block in the admin /settings raw view and reported it "disappeared" — but the underlying confusion was that settings.json on disk is a *template*, not the effective config. None of these changes is visible to installs that don't use ${VAR} placeholders. * Banner above the editor explaining the template/env-substitution model, only rendered when the loaded file contains a ${VAR} placeholder. Tells the operator that the file is not env-substituted in place and that the Effective tab shows the live values. * Effective tab in the mode toggle, read-only, also gated on ${VAR}. The backend was already emitting redacted runtime settings as `resolved` alongside every `load`; the SPA now exposes them so an operator can verify what Etherpad is actually using. * admin_auth_error event from the /settings socket handler. The handler previously silently returned when the connecting session wasn't admin, which made misrouted Traefik+SSO auth look like "save did nothing" with no error path in the UI. Emit a dedicated event before dropping the socket so the SPA can show a clear toast. Tests: - src/tests/backend/specs/admin/adminSettingsAuthError.ts — new spec for the auth_error/disconnect contract. - src/tests/frontend-new/admin-spec/adminsettings.spec.ts — new Playwright test asserting the banner + Effective tab only appear after a ${VAR} is added to settings.json, and that the Effective view is read-only + shows [REDACTED] for secrets. No behaviour change for installs without ${VAR} placeholders — banner, Effective tab, and auth-error contract are all the same as before. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(admin): drop fragile pre-condition + add reconnect-loop guard (#7819) CI's admin-UI workflow seeds settings.json by copying settings.json.template verbatim, which contains ~30 \${VAR} placeholders. The new Playwright test asserted "banner not present before adding placeholder" — true on a fresh dev machine, false in CI. Drop that assertion: the negative path is covered by the SettingsPage ENV_VAR_PATTERN regex itself; what matters at the UI level is the positive path (banner + Effective tab render correctly when placeholders are present), which this test still exercises. Also: the server's admin_auth_error path calls socket.disconnect(), which the SPA's existing disconnect handler interprets as "io server disconnect" and immediately reconnects — creating a reject/reconnect loop. Track an authErrored flag and suppress the reconnect once an auth_error has been received. Reset on successful connect, so a legitimate re-auth path still works. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
John McLear
|
d0180033e4
|
fix: page sessionstorage cleanup to avoid OOM (#7830) (#7831)
* fix: page sessionstorage cleanup to avoid OOM (#7830) SessionStore._cleanup() previously called `findKeys('sessionstorage:*', null)`, materialising every session key into a single array. On decade- old MariaDB installs with millions of sessions this OOMs the node process within ~15 minutes — see #7830. Switch to ueberdb2 6.1.0's findKeysPaged with a 500-key page size, and yield to the event loop between pages so the DB driver can release each page's buffered rows and request handlers can interleave. The break is now driven by `page.length === 0` rather than `page.length < CLEANUP_PAGE_SIZE` so a stubbed/throttled paged source still iterates the full keyspace. Adds a regression test that seeds 50 sessionstorage rows, monkey-patches `DB.findKeysPaged` to use a 4-key page, runs cleanup, and asserts every expired row is removed plus every valid row preserved across page boundaries. Closes #7830 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: address Qodo review on #7831 Four follow-ups raised by Qodo on the session cleanup paging fix: - DB.ts: fail-fast at init() if any required wrapper method (incl. findKeysPaged) is missing, so a stale ueberdb2 pin surfaces at boot rather than crashing the first cleanup run an hour later. - SessionStore: bound a single _cleanup() run to 10 minutes. Under sustained session creation the keyspace can grow faster than cleanup drains it; without a budget the next scheduled run would never fire. When the budget hits, log a warning and let the next run continue. - SessionStore: log the defensive `page[0] <= after` cursor-stall break. Previously the loop exited silently, leaving expired rows behind with no operator-visible signal of the backend regression. - Tests: the paged-cleanup regression test now removes both expiredSids AND validSids in finally, so a failed assertion doesn't leak rows. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs: note paged session cleanup in CHANGELOG + settings template CHANGELOG.md picks up an entry under 3.1.0 Notable fixes describing the OOM cause, the paged iteration, the 10-minute per-run budget, the cursor-stall logging, and the fail-fast init guard. settings.json.template's sessionCleanup comment adds the page-size, budget, and pointer to #7830 so admins can reason about the new behaviour from the template alone. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: regenerate lockfile against ueberdb2 6.1.2 Now that ether/ueberDB#983 unblocked the publish workflow (OIDC trusted publishing), ueberdb2 6.1.2 is live on npm and the `^6.1.0` pin in src/package.json resolves cleanly. Resolves the ERR_PNPM_OUTDATED_LOCKFILE that was blocking CI on this PR. 29 SessionStore backend tests still green against the published tarball. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
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> |
||
|
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> |
||
|
John McLear
|
f6ab8561ae
|
fix(pad): outdated notice — resolve author from token cookie (Qodo #7804) (#7805)
* fix(updater): resolve pad author from token cookie, not session.user Etherpad does not populate an authorID into the express-session user object for pad visitors, so resolveRequestAuthor() always returned null in production, causing computeOutdated() to return EMPTY and the pad-side gritter to never fire. Replace the session-based lookup with a cookie-based path that mirrors how the socket.io handshake resolves pad-visitor identity: read the HttpOnly `token` (or `<prefix>token`) cookie and call authorManager.getAuthorId(token, user) via dynamic import (same circular-init guard pattern as the PadManager import). Update the test harness to mock AuthorManager instead of injecting a fake req.session.user.author, and to set req.cookies.token directly. All 9 cases continue to pass. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(openapi): clarify admin spec scope includes pad-side endpoints /api/version-status is a public pad-side endpoint but lives in the admin OpenAPI document because it shares the same internal route registration. Add a note to info.description so downstream tooling consumers are not misled into treating it as an admin-only route. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> |
||
|
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> |
||
|
John McLear
|
10558ed115
|
fix(admin/pads): apply filter chip server-side, before pagination (#7798)
* fix(admin/pads): apply filter chip server-side, before pagination
Before: PadPage's filter chip (`active`/`recent`/`empty`/`stale`) ran
on the client AFTER the 12-row page slice was already on screen. On a
deployment with hundreds of pads it produced obviously wrong results
— click "empty pads" on page 1 with 100 empties and only the 0–12
empties within the current page passed the filter. thm reported this
on a 3.1.0 deployment.
Move the filter into `PadSearchQuery` so the `/settings` socket can
apply it before slicing:
1. pattern filter on names (cheap)
2. hydrate metadata for the matching pad universe iff a non-`all`
filter is set or a non-`padName` sort is requested
3. apply filter chip on the hydrated set
4. sort + slice → `total` reflects the filtered universe so the
pagination footer makes sense
The original handler also had a 4-way `if/else if` that duplicated the
hydrate-and-sort loop per `sortBy`. Folded those into one pipeline
with a single comparator switch.
Client side, `PadPage.tsx`:
- drop the client-side `filteredResults` filter (server already filters)
- chip click writes `filter` into searchParams (debounced refetch) and
resets `currentPage` to 0
- older clients that don't send `filter` keep working — server defaults
to `all`
Stats cards (totalUsers/activeCount/emptyCount) still count the visible
page only — that's a pre-existing UI limitation tracked separately.
Closes the regression thm reported.
Test plan
- `tsc --noEmit` clean (server + admin)
- New backend spec `padLoadFilter.ts` exercises filter:empty with
small `limit` to lock in the bug-fix, plus all/active/omitted cases
- `5 passing` locally on Node 25
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* address Qodo review on #7798
1. Functional setState updaters for every searchParams mutation
(Qodo bug 1). The debounced pattern handler captured a render-time
snapshot of searchParams; a faster chip click or sort change in
between would be silently reverted when the debounce fired. Now
every mutation merges against the latest state.
2. Concurrency-limited hydration (Qodo bug 3). The earlier draft
issued Promise.all over the full candidate set, fanning out to
thousands of in-flight padManager.getPad() reads on busy
deployments. New mapWithConcurrency() caps concurrent loads at 16
— empirically enough to saturate a single ueberDB driver without
pushing the event loop into back-pressure.
3. Test cleanup deletes the injected test-admin (Qodo bug 4). The
original snapshot/restore pattern saved `settings.users` by
reference; reassigning the same reference in after() left the
inserted key in place and could leak into later backend specs.
4. Document the new `filter` field on the `padLoad` socket query in
admin/README.md (Qodo rule violation 2).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
|
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> |
||
|
John McLear
|
8f499b458d
|
fix(ExportHtml): don't poison ol counter when closing a sibling ul (#7791)
When an ordered-list level was the only consumer of olItemCounts, closing any list at that depth (including an unordered list that happens to share the level) reset olItemCounts[level] to 0. A later, unrelated ordered list at the same depth then took the "counter exists but is 0" branch in the ol-opening logic and emitted `<ol class="...">` without the start attribute that line.start would have supplied. Round-trip: importing <ul>...<ul>...</ul></ul><ol><li>x<ol><li>y</li></ol></li></ol> exported the inner ol as `<ol class="number">` instead of `<ol start="2" class="number">`, because the closing of the inner bullet ul wrote olItemCounts[2]=0 before the outer ol even opened. Gate the reset on line.listTypeName === 'number' so closing an unordered list never touches the ol bookkeeping. Closing an actual ordered list still resets, as #7470 intended. Fixes #7786 Fixes #7787 Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
John McLear
|
0e90184b9b
|
fix(export): surface checkValidRev error message on bad :rev (#7792)
* fix(export): surface checkValidRev error message in response body
A non-numeric :rev (e.g. /p/foo/test1/export/txt) was reaching
checkValidRev, which throws CustomError('rev is not a number',
'apierror'). The error fell through the route handler's
.catch(next), so Express's default error renderer kicked in and
returned a 500 with the generic HTML page <title>Error</title> /
<pre>Internal Server Error</pre>. The thrown message never made it
to the body, so callers had no way to tell why the request failed.
Catch the apierror in the route handler and send err.message as a
text/plain 500 body. Other errors still propagate to next(err) so
unrelated failures keep their existing handling.
Also retitle the test (was "is 403" while asserting expect(500)) —
leftover label from an earlier expectation.
Fixes #7788
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(export): validate rev before attachment, broaden error catch
Qodo feedback on #7792:
1) ExportHandler.doExport set Content-Disposition (via res.attachment)
before calling checkValidRev. If the rev was invalid, the route-level
catch returned a plain-text 500 — but the attachment header was still
in place, so browsers offered to save the error message as a file.
Move checkValidRev to the top of doExport so an invalid rev never
touches the attachment header.
2) The catch only converted CustomError('...', 'apierror') into a
plain-text response. Other export errors (conversion failures, fs
issues, soffice problems) still fell through to Express's default
HTML renderer — non-deterministic for API callers and confusing
when they were already half-downloading a file.
Surface every export failure as a deterministic text/plain 500.
apierrors carry user-facing messages, so send err.message verbatim.
For other errors, log the full stack server-side and still emit
err.message (or 'Internal Server Error' if absent) so the response
body is never the Express HTML stack page. Also clear
Content-Disposition in the catch as a safety net.
Backend tests (importexportGetPost.ts): 58 passing, 0 failing.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
|
John McLear
|
fba4a17fcc
|
fix(API): hide SYSTEM_AUTHOR_ID from listAuthorsOfPad (#7793)
* fix(API): exclude SYSTEM_AUTHOR_ID from listAuthorsOfPad
Pad.SYSTEM_AUTHOR_ID ('a.etherpad-system') is the synthetic author
Etherpad attributes inserts to when the HTTP API receives a call
without authorId (setText, setHTML, appendText, the server-side
import flows, and plugins like ep_post_data). It exists so the
changeset's text and attribs stay in sync — without ANY author
attribute, pad.atext drifts and clients fail setDocAText
reconciliation when loading the pad. See Pad.ts:96-105 for the
full rationale.
That bookkeeping detail was leaking through listAuthorsOfPad: a
pad whose only "contributor" is the system author still reported
one authorID, which the existing tests in pad.ts and
appendTextAuthor.ts (and presumably any caller that uses
listAuthorsOfPad to count real users) treat as a real participant.
Filter SYSTEM_AUTHOR_ID at the API surface so internal attribution
stays internal. getAllAuthors() and downstream callers (copy,
anonymize, atext verification) keep seeing the synthetic id —
this only narrows the public listAuthorsOfPad response.
Fixes #7785
Fixes #7790
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(api): note that listAuthorsOfPad omits the system author
Match the runtime behaviour from the previous commit — the
synthetic 'a.etherpad-system' author used for unattributed inserts
is filtered out of the listAuthorsOfPad response.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
|
John McLear
|
8c6104c5d5
|
harden: assorted server-side tightening for 3.0.2 (#7784)
* harden: assorted security tightening across server entry points
A bundle of defence-in-depth hardening picked up during an internal
audit pass. Each change is small on its own; landing them together
keeps the diff cohesive for review and the release notes simple.
Production-side changes:
- src/node/handler/APIHandler.ts: tighten the OAuth JWT validation
path on the HTTP API. Verify the signature before reading any
claim off the payload, and require the admin claim to be strictly
true (not just present). Switch the apikey comparison to
crypto.timingSafeEqual.
- src/node/handler/{Import,Export}Handler.ts: derive temp-file path
tokens from crypto.randomBytes(16) instead of Math.random.
- src/node/hooks/express/tokenTransfer.ts: enforce a 5-minute TTL
on transfer records, make redemption single-use (remove before
response), and drop the author token from the response body —
the HttpOnly cookie is the only delivery channel.
- src/node/utils/sanitizeProxyPath.ts (new): shared sanitiser for
the `x-proxy-path` header. Used by admin.ts (HTML/JS/CSS
substitution) and specialpages.ts (legacy timeslider redirect).
Strips characters outside [A-Za-z0-9_./-], collapses leading
`//+` to a single `/`, rejects `..` traversal. admin.ts also
emits Vary: x-proxy-path and Cache-Control: private, no-store.
- src/node/db/Pad.ts + src/node/utils/ImportHtml.ts: centralise
the "every insert op carries an author attribute" invariant in
Pad.appendRevision so all non-wire callers (setText, setHTML,
restoreRevision, plugin paths) get the same check the socket
handler already enforces. Pad.init and setPadHTML now
substitute SYSTEM_AUTHOR_ID when no author is supplied — same
pattern setText/spliceText already use.
Tests:
- src/tests/backend/specs/api/jwtAdminClaim.ts (5 cases)
- src/tests/backend/specs/tokenTransfer.ts (6 cases)
- src/tests/backend/specs/proxyPathRedirect.ts (5 cases)
- src/tests/backend/specs/padInsertAuthorInvariant.ts (4 cases)
- src/tests/backend-new/specs/sanitizeProxyPath.test.ts (14 vitest cases)
- src/tests/backend/common.ts: add generateJWTTokenAdminFalse helper.
Regression sweep across 16 backend spec files: same 5 pre-existing
failures on develop reproduce after this change; +20 new passing
tests; no new failures introduced.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* harden: rewrite imported .etherpad records to satisfy the insert-op invariant
Legacy .etherpad exports (and exports from older server-internal flows
that didn't substitute SYSTEM_AUTHOR_ID) can contain `+content` insert
ops without an `author` attribute. The previous commit's appendRevision
guard rejects that shape, but setPadRaw bulk-writes records directly
to the DB and never goes through appendRevision -- so a hand-crafted
.etherpad file could persist non-conforming data that any subsequent
setText / setHTML / restoreRevision call would then refuse to extend.
Add a pre-pass over the parsed import that:
- walks revs in numeric order, sanitising each changeset's `+` ops
against the cumulative pad pool (mutating the pool to register
SYSTEM_AUTHOR_ID when needed);
- re-applies each (post-sanitisation) changeset to a running atext
so the head atext and any key-rev meta.atext / meta.pool
snapshots are re-derived in lock-step with the rewritten revs
(otherwise pad.check's deep-equal at the end of setPadRaw would
fail on the attribute-number drift between the sanitised head
state and the now-stale key-rev snapshot);
- leaves already-conforming payloads untouched (no log noise on
good imports).
Returns the number of ops rewritten so the import can log a single
warning per legacy file rather than per-record. Pure-newline `+` ops
are exempted -- same whitelist as the wire-side guard.
Tests:
- tests/backend/specs/padInsertAuthorInvariant.ts: two new cases
drive setPadRaw end-to-end with a hand-crafted legacy payload
(asserts the head atext gains a `*N` attribute reference and the
pool registers `[author, a.etherpad-system]`) and with a
conforming payload (asserts the data round-trips unchanged).
Regression sweep across 17 backend spec files: 209 passing / 12
pending / 5 failing -- same 5 pre-existing failures present on
unmodified develop. +2 new passing tests; no new failures introduced.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
dependabot[bot]
|
8b9d9f31ec
|
build(deps): bump ueberdb2 from 5.0.48 to 6.0.2 (#7734) | ||
|
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> |
||
|
John McLear
|
294158e773
|
harden: reject USER_CHANGES inserts without an author attribute (#7773)
* harden: reject USER_CHANGES inserts without an author attribute Insert ops MUST carry the author attribute reference so that pad.atext.text and pad.atext.attribs stay in lock-step. An accepted insert with empty attribs would grow text without contributing matching attribute markers, leaving the stored AText in a state where the two iterables disagree on length when reconstructed. Downstream clients then fail reconciliation in ace2_inner.ts:setDocAText with 'mismatch error setting raw text in setDocAText' on every subsequent pad load — making the affected pad effectively unloadable until manually repaired. This commit adds a single defensive check inside the existing per-op validation loop in handleUserChanges: when an op is a '+' (insert) and its attribs string doesn't yield an 'author' entry via AttributeMap.fromString, reject with badChangeset. The check piggybacks on the wireApool that was already constructed for the prior author-match validation, so no extra parsing. Test fixtures in messages.ts were updated to send proper author-attributed inserts plus the matching apool (mirroring what the JS web client always does). A new regression test 'insert without author attribute is rejected' locks in the new behaviour. * harden: also close the HTTP API / plugin path via stable system author The first commit closed the socket.io USER_CHANGES hole. This commit closes the parallel path through Pad.spliceText (used by API.setText, API.appendText, the import flow, and plugins like ep_post_data) where an unattributed insert would otherwise produce a malformed AText. Approach: instead of REJECTING (which would break ep_post_data and many existing tests that call setText/appendText without an authorId), substitute a stable system author when none is provided. The resulting changeset is properly attributed, the AText stays well-formed, and existing callers continue to work unchanged. Plugins that want named author attribution should still pass an explicit authorId (e.g., one allocated via authorManager.createAuthor). Pad.SYSTEM_AUTHOR_ID = 'a.etherpad-system' — a stable identifier that appears in the pad's attribute pool when internal callers (HTTP API, plugins, server-side imports) write text without naming an author. The existing 'attribute changes by another author' protections still apply to socket.io USER_CHANGES paths — a remote client can't impersonate the system author for inserts (their session author check fires first). Test: - Pad.ts spec adds 'spliceText with empty authorId attributes to the system author' — verifies pad text lands AND the pool contains the system-author binding. Existing tests that pass an authorId are unaffected. * harden: reject USER_CHANGES that would strand the trailing newline Etherpad's pad text always ends with '\n'. _handleUserChanges previously appended a separate `nlChangeset` correction revision whenever the applied USER_CHANGES left the pad without a trailing '\n'. The stored pad ended up well-formed, but the FIRST NEW_CHANGES broadcast (the malformed user revision itself) reached browsers BEFORE the correction did, and applyToAttribution's MergingOpAssembler aborts with "line assembler not finished" on a non-'\n'-terminated doc — the watching browser session then dropped the changeset and any subsequent edits silently no-op'd until the user reloaded. Replace the silent auto-correction with an explicit reject. Compute `applyToText(rebasedChangeset, prevText)` before appendRevision; if the result doesn't end with '\n', throw -> badChangeset disconnect. Clients must emit USER_CHANGES whose application preserves the invariant — this matches what the JS web client already does and forces non-JS clients (etherpad-pad, third-party integrations) to surface their bugs in their own logs instead of stranding the trailing newline in pad revision history. Also fixes a latent retransmission-detection bug surfaced by this PR's author-attrib changes: moveOpsToNewPool renumbers `*N` references to whatever slot the pad pool assigns, which can differ from the wire form's slot. Comparing the raw client wire against the stored revision form (`changeset === c`) then misses legitimate retransmissions and the same edit gets duplicated. Snapshot the post-pool-mapping form (`canonicalCs`) and compare that against `c` instead. Backend test additions: - 'changeset that would strand the trailing \\n is rejected' covers the new rejection path with wire `Z:6>1|1=6*0+1$X` against `hello\n`. - handleMessageSecurity test now captures roSocket's own authorId and uses it in the apool sent through roSocket, because the prior PR commit made `*0` referencing the wrong author a hard reject. All 1130 backend tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: bump etherpad-cli-client to ^4.0.3 4.0.3 sends author-attributed inserts and preserves the trailing newline, complying with this PR's tightened USER_CHANGES validation. The rate-limit CI workflow drives the test pad via this client, so without the bump the new server-side rejects fire on the very first \`pad.append()\` and the rate-limit disconnect never gets a chance to arrive — testlimits.sh exits 0 instead of 1 and the rate-limit job fails with "ratelimit was not triggered when sending every 99 ms". Refs ether/etherpad-cli-client#131 * harden: reject USER_CHANGES that name the reserved system author The session-author equality check already rejects wire `*N` that names a different real user, but `a.etherpad-system` is server- internal — it's only used when spliceText / setText is called with an empty authorId from HTTP API or plugin paths. A wire op that names it is either a confused client or an attempt to launder edits through a reserved attribution slot. Refuse. Backend test 'insert claiming the reserved system author is rejected' locks in the new behavior with wire `Z:1>5*0+5$hello` plus an apool that maps slot 0 to `a.etherpad-system`. All 1131 backend tests pass. Inline literal `'a.etherpad-system'` rather than importing the constant from `Pad.SYSTEM_AUTHOR_ID` — `require('../db/Pad')` at PadMessageHandler module scope returned a partially-initialized class via the padManager circular path, leaving the static-field access undefined at runtime. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
John McLear
|
924059257d
|
fix(a11y): name role=toolbar regions, hide linemetricsdiv from AT (#7255) (#7777)
* fix(a11y): name role=toolbar regions, hide linemetricsdiv from AT (#7255) Two regressions called out in the 2026-05-16 follow-up on #7255, after the firefox accessibility inspector flagged them: (1) The "Ether X" announcement between the editor and chat button was the outer ace iframe (titled "Ether") plus a single 'x' text leaf the renderer appends to outerdocbody for line-height measurement (linemetricsdiv in ace.ts). Add aria-hidden=true on creation so AT skips the measurement node entirely. Same approach we used for sidediv in PR #7758. (2) The two formatting/actions <ul role="toolbar"> regions and the history-mode role=toolbar div had no accessible name. Lighthouse + the firefox a11y panel both flagged this. Putting data-l10n-id directly on the <ul> would either destroy its <li> children (textContent branch) or not populate aria-label (the html10n auto-aria-label code path skips non-form-control elements), and a hidden <span> child inside the <ul> would be invalid HTML. Solution: three visually-hidden <span> labels sitting just before #editbar, each carrying data-l10n-id for translation, referenced from the toolbars via aria-labelledby. Apply the same treatment to .show-more-icon-btn, whose aria-label was previously hardcoded English (no data-l10n-id, so untranslated). Adds Playwright assertions for linemetricsdiv aria-hidden and the resolved accessible-name text of each toolbar. Updates the existing show-more test to expect aria-labelledby (it previously asserted hardcoded English aria-label). Refs #7255 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(a11y): reuse translated history-controls label key (Qodo PR review) Qodo flagged: adding `aria-labelledby="editbar-history-label"` on #history-controls overrode the `aria-label` that pad_mode.ts sets from `pad.historyMode.controlsLabel`. That key is already translated in multiple locales (en/de/nl/...); the new `pad.editor.toolbar.history` key was English-only, so non-English users would regress from a localized history-toolbar name to the English fallback. Point the hidden label span at the existing translated key instead of minting a new one, drop the new key from en.json, and update the Playwright expectation to match the translated string ("Pad history controls"). The aria-label that pad_mode.ts still writes to #history-controls is now redundant (aria-labelledby wins) but harmless, and leaving it preserves the runtime relocalization path. Refs ether/etherpad#7777 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(a11y): mark toolbar li/a wrappers presentational (Lighthouse, #7255) Lighthouse's axe-core `listitem` rule fires on every toolbar button because role="toolbar" on the <ul> overrides its implicit role="list", leaving the <li> children "orphaned" by axe's heuristic. Murphy's 2026-05-16 follow-up on #7255 attached the Chrome DevTools Lighthouse panel screenshot of this exact failure. Marking the <li>+<a> wrappers role="presentation" tells axe-core they are layout scaffolding for the toolbar role, while the inner <button> keeps its semantics for AT. Same treatment for SelectButton's <li> wrapper. The Separator's <li> also gets aria-hidden=true so AT does not announce an empty list item between toolbar buttons. CSS and JS selectors that still target `.toolbar ul li` continue to work — role="presentation" only affects the accessibility tree, not the DOM tree. No visual or behavioral change for sighted users. Adds a Playwright spec that walks every rendered toolbar <li>/<a> and asserts role="presentation" so future toolbar.ts tweaks can't silently re-introduce the Lighthouse failure. Refs ether/etherpad#7255 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(a11y): label #online_count for AT (#7255 - "number next to the user icon") Murphy's 2026-05-16 follow-up cut off mid-bullet ("It's not clear what the number next to the …"). Best guess: the user-count badge in the showusers toolbar button. Currently it exposes a bare digit to AT — "5" with no context — because the visible badge text is also the entire accessible content. Append a localized aria-label generated from a new pad.userlist.onlineCount key (plural macro for one / other) whenever the count updates, so AT announces "5 connected users" instead of the bare digit. Add role=status and aria-live=polite so the count change is announced inline without forcing the user to refocus the button. Visible badge digit unchanged. html10n.get is null-safe (falls back to an English template so AT never gets back "undefined" before the locale bundle has loaded). Adds a Playwright spec verifying role/aria-live and that the aria-label contains "connected user". Refs ether/etherpad#7255 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(a11y): tighten #online_count + plugin-emitted toolbar <li>s (#7255) Two small follow-ups on top of the toolbar/online-count work: (1) #online_count had no accessible label on solo-author pads. updateNumberOfOnlineUsers — which writes the localized aria-label — only fires on userJoin/userLeave/status change, never on initial single-author load. Call it at the end of init() so the badge ships with its label on first paint. Also bind html10n's 'localized' event so non-English users get the translated label after the locale bundle arrives (matches the keyboard-hint / history-toolbar pattern). Harden the Playwright spec to use polling toHaveAttribute instead of one-shot getAttribute. (2) Sweep role="presentation" onto plugin-emitted toolbar <li>s in pad_editbar.ts init(). Core's toolbar.ts emits its <li>s with the role already, but plugins (ep_headings2, ep_align, ep_font_*, ep_print, ...) ship their own editbarButtons.ejs templates that emit <li> directly, so Lighthouse's listitem rule kept firing on the "with plugins" test runs. Runtime sweep covers anything in the editbar at init time, no plugin coordination needed. Refs ether/etherpad#7255 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
John McLear
|
986f139a61
|
feat(metrics): 3 Prometheus counters for scaling dive (#7756) (#7762)
* feat(metrics): expose 3 Prometheus counters for the scaling dive Per the spec section 6 of #7756: enables the load-test harness to attribute *where* time goes on the server, not just the gauge headline (CPU / event-loop / memory) the dive doc starts from. New /stats/prometheus rows: - etherpad_pad_users{padId} — gauge, derived from sessioninfos on each scrape. Lets the harness confirm the pad it points at actually has the expected concurrency. - etherpad_changeset_apply_duration_seconds — histogram observed inside handleUserChanges. Separates "apply path is slow" from "fan-out is slow" when latency rises. - etherpad_socket_emits_total{type} — counter at the broadcast emit sites (handleCustomObjectMessage, handleCustomMessage, sendChatMessageToPadClients) and inside the NEW_CHANGES per-socket loop in updatePadClients. Bucketed by message type so the harness can measure the amplification factor of each lever (especially the fan-out batching lever). Metric handles live in a new prom-instruments.ts module rather than in prometheus.ts itself, so PadMessageHandler can import the recording helpers without creating a circular dependency (prometheus.ts already requires PadMessageHandler). Tests: smoke test verifies recordSocketEmit + recordChangesetApply move the underlying counters/histogram. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(metrics): address Qodo review — flag-gate, scope histogram, bound label cardinality Three issues raised on the initial PR: 1. **Feature flag.** Per project compliance rule, new features must be behind a flag and disabled by default. Adds `settings.scalingDiveMetrics` (default `false`). When off, recordSocketEmit() / recordChangesetApply() short-circuit to no-ops and the metrics are never even registered with the Prometheus register. Enable only when running the ether/etherpad-load-test scaling-dive harness. 2. **Histogram scope.** Previously the etherpad_changeset_apply_duration_seconds timer wrapped the whole handleUserChanges() body — including `await exports.updatePadClients(pad)` — so the histogram measured apply+fan-out, defeating its stated purpose. Now stopped immediately after the apply work (`assert.equal(...rev, r)`), before the ACCEPT_COMMIT socket emit and the updatePadClients call. Failed applies deliberately don't observe so the success-path distribution stays clean. 3. **Label cardinality.** handleCustomMessage was passing the user-supplied msgString (an HTTP-API param) directly as the `type` label value. A misbehaving API caller could grow prom-client's internal label map until OOM. Now bucketed against a known-types allowlist; anything outside it lands in `other`. Tests updated: 5/5 — covers happy path, "other" bucketing of unknown/unsafe labels, and that the flag-disabled state is a true no-op. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
John McLear
|
21e1ae2fa3
|
security: allow integrator sessionID cookie to be HttpOnly (#7045) (#7755)
* security: allow integrator sessionID cookie to be HttpOnly (#7045) The integrator-set sessionID cookie was forced to be non-HttpOnly because Etherpad's own client JS read it via document.cookie and forwarded it in the socket.io CLIENT_READY payload, exposing it to XSS. Mirror the GDPR PR3 author-token migration: read sessionID from the socket.io handshake's Cookie header in PadMessageHandler.handleClientReady, falling back to the legacy message-level field with a one-time deprecation warning per socket. Drop the client-side Cookies.get('sessionID') reads in pad.ts and timeslider.ts so the field is no longer sent by current clients. Existing integrators that set sessionID without HttpOnly keep working unchanged; the field on the message becomes optional and integrators should now mark the cookie HttpOnly; Secure; SameSite=Lax. Closes #7045 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(security): treat undecodable handshake cookies as absent (Qodo #7755) decodeURIComponent() throws URIError on malformed values like `%ZZ`. The unguarded call in PadMessageHandler.handleClientReady's readCookie() let a single bad cookie abort CLIENT_READY for that socket, allowing unauthenticated peers to spam server error logs and lock themselves out of pads. Catch URIError and treat the value as absent so the legacy message-level field still serves as a fallback. Other error classes still propagate. Add a backend test that asserts a `sessionID=%ZZ` cookie no longer aborts the handshake. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
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> |
||
|
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> |
||
|
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> |
||
|
John McLear
|
451bd9c3eb
|
feat(pad): scrub history in-place on the pad URL (#7659) (#7710)
* feat(pad): scrub history in-place on the pad URL (#7659) Clicking the timeslider toolbar button now keeps the user on /p/:pad and toggles a hash-based history mode (#rev/N) instead of navigating to a separate /timeslider page. The pad shell — chat, users panel, settings, plugin chrome — stays mounted across the transition. A sticky banner plus a sepia tint on the toolbar make it unmistakable that what is visible is historical, not live. Implementation: - New PadModeController (src/static/js/pad_mode.ts) owns enter/exit, the URL hash, browser back/forward, and a mutation-observer bridge from the inner timeslider's revision label/date into the outer banner. Esc and a Return-to-live button both exit history. - pad.html grows a banner element and an iframe mount slot. The live ACE iframe stays mounted but hidden during history; on exit the socket is still alive, so the user snaps straight back to the current state without a reconnect. - The /p/:pad/timeslider route 302-redirects to the pad page for direct visits (legacy bookmarks), and serves the timeslider HTML for the in-pad iframe when called with ?embed=1. The embedded variant hides the redundant title and return-to-pad button via CSS; the slider, settings, and export controls stay reachable. - Legacy #NN shortlinks are preserved through the redirect by the browser and translated to #rev/NN client-side. Tests: - New backend spec asserts the 302 redirect, pad-name preservation, and the ?embed=1 path still serves the timeslider HTML. - New padmode.spec.ts exercises toolbar entry, return-to-live, browser back, and direct /timeslider URL handling. Asserts the rendered localized banner string, not just element presence. - Existing timeslider specs that hit /p/:pad/timeslider directly now pass ?embed=1 to bypass the redirect. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(pad): make history iframe fill the editor area (#7659) Without an explicit positioning model the history-frame-mount inherited half-width from a phantom flex parent and the embedded timeslider rendered at 640×625 instead of the full editor area. Switch to the same absolute-fill model the live ACE iframe uses by making #editorcontainerbox the positioning anchor when in history mode. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(pad): address Qodo review and CI failures (#7659) Concrete review fixes for PR #7710: - Tighten the embed query check from `if (!req.query.embed)` to `req.query.embed !== '1'` so values like `?embed=0` no longer bypass the redirect. - Fix the `#rev/latest` mapping: the parser yields -1 for "latest", which the iframe sync handler was clamping to 0 and so jumping the embedded timeslider to revision 0. Resolve "latest" to the inner BroadcastSlider's upper bound instead. - Update existing backend tests (`socialMeta`, `specialpages`) that hit `/p/:pad/timeslider` directly — they now pass `?embed=1` like the rest of the suite. Without this fix three pre-existing tests failed CI (302 instead of 200). - Document the route change in `doc/skins.md` and `doc/skins.adoc`: direct visits redirect; iframe consumers use `?embed=1`. - Back out a stray `data-theme="editorial"` attribute and the hardcoded Google Fonts `<link>` tags from `pad.html` that leaked into the branch from an unrelated working-tree change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(pad): consolidate chrome and replay chat/users in history mode (#7659) Picks up the rough edges left by the initial in-place history mode: the embedded timeslider iframe was rendering its own duplicate Settings and Export buttons, and the chat panel + users list still showed live state while the editor scrubbed back in time. Chrome consolidation - Hide the entire inner editbar's right-side toolbar and modal popups in embedded mode (slider stays). Outer pad shell now owns Settings, Export, Share, Users, Chat across both modes. - Outer Settings popup grows a "History playback" section (visible only when scrubbing) with playback speed + follow-contents. Both bridge to the iframe's BroadcastSlider state. - Outer Export anchors are rewritten to /p/<pad>/<rev>/export/<type> on each scrub and restored on exit, so Save As exports the visible historical revision. Chat replay - Each chat message is annotated with data-timestamp at render time. In history mode, messages newer than the scrubbed revision's timestamp are display:none'd; a "Chat as of HH:MM" header sits above the chat log. - Restores cleanly on exit (inline display cleared, header removed). Users replay - Live users table is replaced with the embedded timeslider's authors-at-this-revision label while scrubbing; restored on exit. Plumbing - Expose padContents on window in broadcast.ts so the outer pad can read currentTime after each scrub without postMessage. - Expose BroadcastSlider on window in timeslider.ts so the outer pad can register an onSlider callback to drive replay UI. Tests - New padmode specs cover: history-only Settings section, hidden embedded chrome, chat filter + replay header, Export href rewriting + restore, authors-row swap + restore. - timeslider_line_numbers cookie-persistence test updated to bypass the now-hidden inner Settings popup (programmatic checkbox). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(pad): theme propagation, hide inert buttons, plugin loading (#7659) Picks up rough edges from the in-place history mode that turned up in real usage: Theme / dark mode - skin_variants.updateSkinVariantsClasses now also walks the history iframe (and its ace_outer/ace_inner) so toggling dark mode while scrubbing re-themes the embedded view in lockstep. - timeslider.ts inherits the parent's skinVariant tokens (super-dark-* / dark-* / full-width-editor) on first paint when it detects it is embedded — same-origin guarantee, falls through silently if not. Toolbar UX - Hide #editbar .menu_left (Bold/Italic/Lists/Indent/Undo/...) and the show-more chevron while in history mode. Those buttons target the hidden live editor and would do nothing useful; rendering them disabled-looking implied state the user doesn't have. Right-side menu (Settings / Share / Users / Chat / Home) stays at full opacity and fully interactive. Slider position - Pin the embedded #editbar to the bottom of the iframe so the outer banner and the slider can't visually compete for the same band of pixels. Reserve padding-bottom on the iframe's editorcontainerbox so the editor never scrolls under the slider. Plugin loading in timeslider - timeSliderBootstrap.js now pre-loads plugin modules into a Map and passes them to plugins.update(), mirroring padBootstrap.js. Without this the loadFn fallback called require(path) at runtime, which the esbuild-bundled timeslider couldn't resolve, so client_hooks like ep_headings2's aceRegisterBlockElements silently failed to register and historical revisions rendered without plugin chrome. Tests - New padmode specs cover: outer toolbar's left/right asymmetry, slider pinned to bottom, dark-mode class propagation into the history iframe. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(pad): move history slider into the outer toolbar (#7659) The slider previously rendered inside the embedded iframe — first at the top (where it visually competed with the banner), then briefly at the bottom (where the chat icon overlapped it). Both were wrong. Move the controls into the outer toolbar's left zone, where #editbar .menu_left is hidden in history mode and the slider can occupy the full width without colliding with anything. - pad.html grows a #history-controls div (slider + play/pause/step buttons + timer) inside #editbar, between menu_left and menu_right. Hidden by default; revealed via body.history-mode CSS. - pad.css swaps #editbar .menu_left out for #history-controls in history mode (display:none / display:flex). - timeslider.css fully hides the embedded iframe's #editbar — the outer toolbar now owns the slider, and the iframe is purely the editor surface. - pad_mode.ts wires the outer controls as a remote control: the range input calls inner BroadcastSlider.setSliderPosition, the play button calls BroadcastSlider.playpause, step buttons forward clicks to the inner #leftstep/#rightstep so they share the existing logic. An onSlider subscription mirrors inner state back into the outer slider value, timer label, and play-button .pause class. Tests - Existing timeslider.spec asserts the outer controls are visible. - New padmode specs cover: inner editbar fully hidden, outer toolbar swap (menu_left → history-controls), and outer slider drives the iframe's revision via BroadcastSlider. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(pad): exempt embedded history iframe from userdup kick (#7659) When the in-place history iframe opens its socket, the server's duplicate-author kick treats it as a stale tab and disconnects the parent pad's live socket — toolbar-overlay drops over the editor and Settings/Share/Users/Chat all stop responding. Mark the iframe's connection with `embed=1` in the socket.io handshake query, record it on sessionInfo, and skip the kick whenever either side is embedded. - timeslider.ts: detect `?embed=1` (and parent !== window) on the iframe URL, pass through as a query parameter to socketio.connect. - PadMessageHandler: read socket.handshake.query.embed on CLIENT_READY, set sessionInfo.embed; the duplicate-author kick now skips when either the connecting session OR the existing session is embedded. Behavior preserved - Two real tabs (both non-embedded): older tab still gets kicked. - Authenticated sessions still bypass the kick entirely. - Live pad socket survives entry into history mode. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(pad): a11y of history toolbar controls (#7659) The new history controls (slider + play/step/timer) had hardcoded English aria-labels, which html10n won't replace because they were present without the data-l10n-aria-label marker. Screen readers in non-English locales would have heard English. Drop the static aria labels and let html10n.translateElement populate aria-label from the data-l10n-id translation, matching how the rest of the toolbar works. - pad.html: remove hardcoded aria-label on play/step buttons and the range input; keep titles (hover tooltip) and data-l10n-id. Add role="toolbar" + data-l10n-id on the controls container so the toolbar landmark is announced. Mark play button as a toggle with aria-pressed reflecting playback state. - en.json: add pad.historyMode.controlsLabel and pad.historyMode.sliderLabel for the toolbar landmark and the slider. - pad_mode.ts: keep aria-pressed in sync with the inner playback state on every revision update. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(pad): a11y + responsive for history controls (#7659) Two issues with the previous a11y attempt: the data-l10n-id on icon buttons was setting their textContent (drawing "Playback / Pause Pad Contents" on screen next to the glyph), and there was no responsive treatment so the timer + slider could overflow narrow viewports. - pad.html: drop data-l10n-id from the icon buttons. They're now empty <button>s. Localized title (hover tooltip) and aria-label (screen reader name) are populated by pad_mode.localizeControls() using the existing timeslider.* keys, with an html10n.bind subscription so language switches re-localize. - Mark #history-timer as hide-for-mobile. - pad.css: dedicated @media (max-width: 800px) and 480px rules shrink padding, gap, and button widths so play + slider + step buttons stay on a single toolbar line at narrow viewports. Mirrors the legacy timeslider's responsive behavior. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(pad): inline Follow + Playback speed, match toolbar height (#7659) Two follow-ups from real testing: - Move "Follow pad content updates" (now "Follow") and "Playback speed" out of the Settings popup and inline them in the history-mode toolbar, alongside the slider + play/step buttons. They were always needed while scrubbing; one extra click into Settings was friction. Removed the now-empty #history-settings-section. - The history controls toolbar was visibly shorter than the live toolbar because the icon buttons sat as bare <button> elements without the live editbar's <li><a> wrapping. Add explicit min-height (40px) and per-button padding so the toolbar is the same vertical size in both modes — switching between live and history no longer reflows. - Differentiate "iframe-mounted history view" from "direct ?embed=1 visit". Only the former hides the inner timeslider editbar — direct visits keep their full chrome so existing test/legacy entry points stay independently usable. Marker: timeslider.ts adds an `iframe-mode` class on body when window.parent !== window; CSS scopes the hide to that combo. Tests - padmode spec asserts Follow + Speed live in the toolbar (not the Settings popup) and are visible in history mode, hidden in live. - timeslider*.spec direct-?embed=1 flows continue to pass because the inner editbar is no longer hidden when not iframe-mounted. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(pad): use absolute path for legacy /timeslider redirect (#7659) CI Firefox failed the legacy-URL redirect test (1 of 32 jobs); Chromium passed. The redirect Location header was a relative `../padname`, which both browsers resolve to /p/padname for `/p/padname/timeslider`. Firefox flaked on it once consistently. Switch to an absolute path including the proxy prefix so the resolution is unambiguous across browsers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(test): accept 304 on legacy timeslider redirect (#7659) CI Firefox failed `expect(res.status()).toBe(200)` because Firefox issues a conditional GET when the redirect target is the same URL the test just loaded via goToNewPad — the server returns 304 Not Modified and the test treats that as a regression. Chromium happens to send fresh requests so it stayed green. Accept either 200 or 304 — both are valid completed navigations to the pad page; what we actually care about is the pathname assertion above. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(pad): eye toggle for Follow, fix line-number alignment (#7659) Two refinements from real testing in 9002: Follow as an eye toggle - Replace the labeled checkbox with an inline-SVG eye icon. The eye is always rendered; a diagonal slash is overlaid via SVG <line> only when the underlying (visually hidden) checkbox is unchecked. Default state is on (auto-following) so the eye renders unobstructed. - Localized hover tooltip + aria-label flips with state — html10n populates "Following pad changes — click to stop following" vs "Not following pad changes — click to follow", and pad_mode.ts re-applies on every change event so screen readers narrate the action the click would take. - Hidden checkbox keeps the existing pad_mode.ts bridge code working (still reads .checked) and lets <label for="…"> handle the click. Line-number alignment fix (broadcast.ts) - The first-line height formula was `nextDocLine.offsetTop - innerdocbody.padding-top` which only computes the right value when innerdocbody is the offsetParent. In the in-pad history iframe, outerdocbody contributes its own padding-top to the offsetTop chain, so the first gutter row was 20px too tall and every subsequent line drifted out of alignment. Use the consistent `next.offsetTop - current.offsetTop` formula for every iteration — same result in the standalone timeslider, correct result in the embedded one. - New padmode spec asserts every gutter row's top matches the editor line's top within 2px, in iframe-mounted history mode. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
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>
|
||
|
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> |
||
|
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 (
|
||
|
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> |
||
|
John McLear
|
85c941fe95
|
feat(padOptions): pass plugin-namespaced ep_* keys through applyPadSettings (#7698)
* feat(padOptions): pass plugin-namespaced ep_* keys through applyPadSettings
Native pad-wide settings ride a single padOptions object: the server seeds
clientVars.initialOptions, the client mutates via pad.changePadOption(), and
the existing padoptions COLLABROOM message broadcasts changes. Plugins can't
use the same rail today because applyPadSettings (client) and
normalizePadSettings (server) silently drop any key not in their hardcoded
whitelist.
Add a passthrough loop that preserves keys matching /^ep_[a-z0-9_]+$/ on both
sides. Plugins can now stash their pad-wide values under their own namespace
(e.g. pad.padOptions.ep_table_of_contents = {enabled: true}) and inherit the
existing broadcast, persistence, creator-only-write enforcement, and
enforceSettings semantics for free.
A new src/node/utils/PluginCapabilities module exposes
padOptionsPluginPassthrough = true so plugins can feature-detect via
require() and fall back to per-user behavior on older cores.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Address Qodo review on PR #7698
Four concerns raised by Qodo (qodo-free-for-open-source-projects):
1. Feature flag — AGENTS.MD §52 requires new features behind a flag,
disabled by default. Add `enablePluginPadOptions` (default false) gating
the passthrough on both server (normalizePadSettings) and client
(applyPadSettings, via clientVars). Plugins detect the runtime state
through clientVars.enablePluginPadOptions; the static
PluginCapabilities flag stays as the "core can do this" signal.
2. Documentation — add a "Plugin-namespaced pad-wide options" section to
doc/plugins.md covering capability detection, the runtime flag, the
key namespace pattern, and the validation rules. Mirror the flag
description in settings.json.template.
3. Unbounded payload — values for ep_* keys are persisted with the pad and
broadcast to every connected client, so an unvalidated path was a
reliability hazard. Validate every ep_* value:
- Must round-trip through JSON.stringify (rejects functions, symbols,
BigInt, circular refs).
- Per-key serialized size capped at 64 KB.
- Combined ep_* size capped at 256 KB per pad.
Rejects drop the value with a console.warn line; the rest of the pad
settings round-trip cleanly.
4. PadOption type — add `[k: \`ep_${string}\`]: unknown` index signature
so the SocketIO message type matches runtime behavior; TS callers no
longer need unsafe casts to read plugin-namespaced keys.
Also extends the backend test suite with cases covering the runtime flag
(off/on), JSON-serializability rejection, per-key cap, and total cap.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(snap-tests): assert_grep — use here-string to dodge pipefail SIGPIPE
`assert_grep` ran `printf '%s' "$out" | grep -q -F -- "$needle"` under
`set -o pipefail`. When grep matched early it closed its stdin, printf
got SIGPIPE on its next write (exit 141), and pipefail propagated the
broken-pipe failure to the pipeline — making `if` see non-zero and
falling into the FAIL branch even though grep itself succeeded.
Failure was timing-dependent: it only fired when `$out` was large enough
that printf hadn't flushed before grep exited. CI ubuntu-latest tipped
into the racy path on PR #7698 once `settings.json.template` grew by 11
lines (the new `enablePluginPadOptions` flag); the symptom was the
`Wrapper unit tests` step reporting `dbType rewritten to sqlite ✗` with
"got: /*…" output even though the seeded file did contain the needle.
Replace the pipe with a here-string so grep gets its input in one shot
with no pipe between processes — no SIGPIPE possible. The fail-message
`head -3` is converted to a here-string for the same reason.
Repro on a runner whose pipe-buffer flush is slower than grep's first
match would have hit the same flake on any PR; the bug isn't about
this particular template change.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
|
John McLear
|
a356414254
|
fix(socialMeta): coerced numeric/boolean override silently dropped (#7692)
* fix(socialMeta): coerced numeric/boolean override silently dropped Qodo flagged on PR #7691: Settings.coerceValue() turns numeric-looking env vars into numbers and "true"/"false" into booleans, so e.g. SOCIAL_META_DESCRIPTION="2026" arrives at the resolver as the number 2026. The previous resolver gated on `typeof override === 'string'`, so it silently fell back to the i18n catalog with no warning — the operator's docker config would appear broken. Accept string|number|boolean and stringify before the empty-check; null / undefined / unsupported types still fall through to the catalog. Two new unit specs cover the numeric and boolean coercion paths. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(socialMeta): widen description type to match coerced runtime Action Qodo bug-find: SettingsType.socialMeta.description and SocialMetaSettings.description still claimed `string | null`, but Settings.coerceValue() can produce number|boolean from env-var-driven config — the previous resolver fix was correct at runtime but the type mismatch forced `as unknown as string` casts in the new tests. Widen both declared types to `string | number | boolean | null` to match runtime reality, drop the typeof string|number|boolean guards in the resolver (the union now narrows automatically) and remove the test casts. Behaviour is unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
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> |
||
|
John McLear
|
ab0cff4d95
|
fix(7606): sync theme-color meta with client-side dark-mode switch (#7690)
* fix(7606): sync theme-color meta with client-side dark-mode switch PR #7636 emits <meta name="theme-color"> server-side from settings.skinVariants. That covers operators who hard-code a dark toolbar in settings.json, but not the runtime path: pad.ts auto-flips the toolbar to super-dark when enableDarkMode is on, the browser reports prefers-color-scheme: dark, and no localStorage white-mode override is set, plus the user can flip it via #options-darkmode. Both paths run skinVariants.updateSkinVariantsClasses(), which until now never touched the meta — so dark-mode users kept the light #ffffff baseline and saw a white address bar above a dark toolbar (stffen on #7606 after 2.7.3). Push the toolbar-color lookup into updateSkinVariantsClasses so the meta tracks every class change: the auto-switch on init, the user toggle, and the skinVariants builder. Mirrors the CSS-source-order table from src/node/utils/SkinColors.ts (last matching *-toolbar token wins). When no meta is present (non-colibris skin, server omits it) the helper is a no-op. Adds Playwright coverage for both paths under colorScheme: 'light' (manual toggle) and 'dark' (auto-switch on dark-OS clients — the case stffen reported). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(theme-color): action Qodo PR review (1) Bug — duplicated toolbar→color table: extract the CSS-source-order mapping and the default-color constant into src/static/js/skin_toolbar_colors, re-imported by both src/node/utils/SkinColors.ts (server, EJS template helper) and src/static/js/skin_variants.ts (client, runtime updates). Lives under static/js so the browser bundle can resolve it; server-side imports of static/js modules already exist (Changeset, AttributeMap, ImportHtml, hooks). One source of truth means a future palette change can no longer silently desync the server-rendered baseline meta from the client updates, which was the exact regression that brought us here. (2) Rule violation — 4-space continuation indentation in the new Playwright spec: re-indent the themeColor helper and the multiline test(...) call to the repo's 2-space rule (.editorconfig). Existing backend coverage (configuredToolbarColor unit tests + the specialpages server-render checks for both pad and timeslider) still passes against the refactored helper, so it's regression-locked end to end. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
John McLear
|
b751fd7b9e
|
feat(settings): enable Pad-wide Settings by default; fix misleading modal title (#7679)
* feat(settings): enable Pad-wide Settings by default; fix misleading modal title The creator-owned Pad-wide Settings feature (#7545) shipped behind a flag that defaulted to false. With the flag off the modal still rendered an H1 of `pad.settings.padSettings` ("Pad-wide Settings") for *every* user, even though no pad-wide controls were ever shown. Two readers in different browsers both saw "Pad-wide Settings" as the modal title, which looked like a creator-gate regression but was just a copy bug. Two changes: 1. Flip the default of `enablePadWideSettings` to `true` (Settings.ts plus both settings templates). With the feature on, the creator (revision-0 author) gets a real "Pad-wide Settings" section gated by `clientVars.canEditPadSettings`, while every other user sees only "User Settings" — matching the design intent of #7545. This is a behavior change, so the settings comments are expanded to describe what the toggle now does. 2. Drop the conditional H1 in `src/templates/pad.html` and always use `pad.settings.title` ("Settings"). Operators who explicitly disable the feature shouldn't see a label that lies about a section that isn't rendered. Adds backend regression coverage in `tests/backend/specs/socketio.ts`: - Different browsers (different cookie jars => different authorIDs): only the first joiner gets `canEditPadSettings: true`. - Same browser, two tabs (shared HttpOnly token cookie => same authorID): both connections are the same identity, both correctly land on the creator path. * test(settings): regression coverage for the settings modal H1 Asserts the rendered `/p/<id>` HTML always uses `data-l10n-id="pad.settings.title"` for the modal heading, regardless of `enablePadWideSettings`. Catches a re-introduction of the old conditional that printed "Pad-wide Settings" for every user when the feature was off. Action of Qodo review feedback on PR #7679. |
||
|
John McLear
|
da87a4fca2
|
fix(7686): username 'false' / 'malformed color: false' for legacy settings.json (#7688)
* fix(7686): legacy padOptions.userName/userColor=false breaks pad
Settings.json files generated before December 2021 used `false` as the
default for these two string options (commit
|
||
SamTV12345
|
bfdbd2bb91
|
chore: removed axios (#7685)
* chore: removed axios * chore: pnpm |
||
|
John McLear
|
70415714e6
|
fix(socketio): don't kick authenticated duplicate-author sessions (#7656) (#7678)
* fix(socketio): don't kick authenticated duplicate-author sessions (#7656) The CLIENT_READY handler kicks any prior socket whose authorID matches the joining socket's, originally as a workaround for stale tabs in the same browser (cookie-derived authorIDs were per-browser, so "same authorID, same pad" reliably meant "page refresh / second tab in this browser"). With stable identities (basic auth, SSO, apikey, getAuthorId hook) the same authorID can legitimately appear across windows or devices, so the kick disconnects real concurrent sessions. Skip the kick when the joining socket has req.session.user set; cookie-only sessions keep the existing behavior so the userdup modal and the xxauto_reconnect path still work. * fix(socketio): suppress USER_LEAVE when other same-author sockets remain With the duplicate-author kick disabled for authenticated sessions, a single authorID can legitimately span multiple sockets in one pad. handleDisconnect was emitting USER_LEAVE on every socket close, which made clients (whose presence is keyed by authorID) drop the author entirely even when another socket of theirs was still online. Only broadcast USER_LEAVE — and only run the userLeave hook — when the disconnecting socket is the last one in the pad for that author. Adds two backend tests: - authenticated identity: closing one of two same-author sockets does NOT emit USER_LEAVE on the other. - different authors (regression): closing socket A still emits USER_LEAVE for socket B. Action of Qodo review feedback on PR #7678. |
||
|
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> |
||
|
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
|
||
|
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>
|
||
|
John McLear
|
9014d3a7c4
|
fix(colors): pick WCAG-higher-contrast text for author colors (#7565)
* feat(colors): clamp author backgrounds to WCAG 2.1 AA on render
Fixes #7377.
Authors can pick any color via the color picker, so a user who chooses
a dark red ends up with black text rendered on a background that fails
WCAG 2.1 AA (4.5:1) — unreadable, but there is no way for *viewers* to
remediate since they cannot change another author's color. Screenshot
in the issue shows exactly this.
This PR lands a viewer-side clamp. For each author background, if
neither black nor white text would satisfy the target contrast ratio,
the bg is iteratively blended toward white until black text does. The
author's stored color is untouched — turning off the new
padOptions.enforceReadableAuthorColors flag restores the raw colors
immediately.
New helpers in src/static/js/colorutils.ts:
- relativeLuminance(triple) — WCAG 2.1 relative-luminance formula
- contrastRatio(c1, c2) — in [1, 21]; >=4.5 = AA, >=7.0 = AAA
- ensureReadableBackground(hex, minContrast = 4.5)
— returns a hex that meets minContrast
against black text, preserving hue
Wire-up:
- src/static/js/ace2_inner.ts (setAuthorStyle): pass bgcolor through
ensureReadableBackground before picking text color. Gated on
padOptions.enforceReadableAuthorColors (default true). Guarded by
colorutils.isCssHex so the few non-hex values (CSS vars, etc.) skip
the clamp and pass through unchanged.
- Settings.ts / settings.json.template / settings.json.docker: new
padOptions.enforceReadableAuthorColors flag, default true, with a
matching PAD_OPTIONS_ENFORCE_READABLE_AUTHOR_COLORS env var in the
docker template.
- doc/docker.md: env-var row.
- src/tests/backend/specs/colorutils.ts: new unit coverage for the
three new helpers, including the exact #cc0000 failure case from
the issue screenshot.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(7377): simplify — just pick higher-contrast text, drop bg clamp
First iteration added an iterative bg-lightening helper
(ensureReadableBackground) gated by a new padOptions flag. CI caught the
correct simpler framing: because WCAG contrast is symmetric in [1, 21],
at least one of black/white always clears AA (4.5:1) for any sRGB
colour. The real bug was that the pre-fix textColorFromBackgroundColor
used a plain-luminosity cutoff (< 0.5 → white), which produced
sub-AA combinations like white-on-red (#ff0000) at 4.0:1.
Reduce the PR to the minimal surface:
- colorutils.textColorFromBackgroundColor now picks whichever of
black/white has the higher WCAG contrast ratio against the bg.
- colorutils.relativeLuminance and colorutils.contrastRatio are kept
as reusable building blocks; ensureReadableBackground is dropped
(no caller needed it once text selection was fixed).
- ace2_inner.ts setAuthorStyle no longer needs the opt-in flag or the
isCssHex guard — the helper handles every input its caller already
passes.
- padOptions.enforceReadableAuthorColors setting reverted along with
settings.json.template, settings.json.docker, and doc/docker.md.
- Tests replaced: instead of asserting the bg gets lightened, assert
that the chosen text colour clears AA for every primary. Covers the
exact #ff0000 failure case from the issue screenshot.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test(7377): assert relative-contrast invariant, not absolute AA
Pure primaries like #ff0000 cannot clear WCAG AA (4.5:1) against either
#222 or #fff — the best either can do is ~4.0:1. No text-colour choice
alone fixes that; bg clamping would be a separate concern. The test
should therefore verify the *real* invariant: the chosen text colour
must produce the higher contrast of the two options, regardless of
whether that contrast clears any absolute threshold.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(7377): compare against rendered #222/#fff, not pure black/white
First cut of textColorFromBackgroundColor computed contrast against
pure black (L=0) and pure white (L=1), then returned the concrete
#222/#fff the pad actually renders with. For some mid-saturation
backgrounds the two comparisons disagreed — e.g. #ff0000:
vs pure black = 5.25 → pick black → render #222 → actual 3.98
vs pure white = 4.00 → would-render #fff → actual 4.00
The helper picked the wrong option because it compared against the
wrong target. Compare against the actual rendered colours so the
returned text colour is genuinely the higher-contrast choice.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test(7377): pick unambiguous colibris test bgs
#ff0000 lives right at the boundary for the two text choices (4.00 vs
3.98), so the test for colibris-skin mapping was entangled with the
border-case selector pick. Use #ffeedd (clearly light → dark text
wins) and #111111 (clearly dark → light text wins) so the test
isolates the skin mapping from the tie-breaking logic.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(7377): use rendered text colour + clamp bg to actually meet AA
Local repro of the issue exposed two real bugs in the previous fix:
1. textColorFromBackgroundColor compared bg against a hardcoded #222 —
but in the colibris skin --super-dark-color resolves to #485365.
For the issue's exact case (#9AB3FA author bg) the selector returned
var(--super-dark-color) thinking it was getting a 7.7:1 ratio, while
the browser actually rendered 3.78:1 — identical to what the issue
screenshot reported. This PR's previous behaviour on the issue's
inputs was unchanged from the pre-fix.
2. For mid-saturation pastels (#9AB3FA) and pure primaries (#ff0000)
neither rendered dark nor white text can clear AA. Text-colour
selection alone genuinely cannot fix this band; the ensureReadable
bg clamp dropped in
|
||
|
John McLear
|
e0a989094d
|
feat(colors): add padOptions.fadeInactiveAuthorColors with toolbar UI (#7554)
Adds a new pad option `fadeInactiveAuthorColors` (default `true`) that controls whether each author's caret/background fades toward white as they go inactive. Configurable server-side (`settings.json` / `PAD_OPTIONS_FADE_INACTIVE_AUTHOR_COLORS`), per-pad in the Pad Settings panel, per-user in the My View panel, or via `?fadeInactiveAuthorColors=false`. Disabling the fade is useful on busy pads where every faded author visually counts as a second on-screen color (a 30-author pad becomes a 60-color pad), or when inactivity tracking is undesirable for whatever reason. Closes #7138. |
||
|
John McLear
|
e028016296
|
feat(admin): surface ep.json disables in /admin plugin browser (#7649)
Companion to ether/ether.github.com#395 — the admin UI's "available plugins" listing now also renders the plugin's declared `disables` (see doc/PLUGIN_FEATURE_DISABLES.md) so an operator about to click Install sees the same warning as a user browsing etherpad.org/plugins: "Disables: chat". - src/node/types/PackageInfo.ts: optional `disables?: string[]` on the registry payload type. - admin/src/pages/Plugin.ts: same on the admin-side PluginDef. - admin/src/pages/HomePage.tsx: render an amber callout under the description when `disables` is present and non-empty. Plugins without a disables field render unchanged. The plugin-registry build pipeline still has to start surfacing `disables` from ep.json into plugins.json/plugins.viewer.json — until that lands, the new callout no-ops everywhere, which is fine. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
John McLear
|
4bda757304
|
feat(api): public compactPad API + bin/compactPad CLI over existing Cleanup (#7567)
* feat(pad): compactHistory() + compactPad CLI for DB-size reclaim Fixes #6194. Long-lived pads with heavy edit history dominate the DB — the issue describes a ~400 MB Postgres after two months with ~100 users. Etherpad keeps every revision forever, and removing arbitrary middle revisions is unsafe because state is reconstructed by composing forward from key revisions. What's safe: collapse the full history into a single base revision that reproduces the current atext. The existing `copyPadWithoutHistory` already does this for a new pad ID — this PR lifts that same changeset pattern into an in-place operation and wires up an admin CLI. - `Pad.compactHistory(authorId?)` (src/node/db/Pad.ts): composes the current atext into one base changeset, deletes all existing rev records, clears saved-revision bookmarks, and appends the new rev 0. Text, attributes, and chat history are preserved; saved-revision pointers are cleared. Returns the number of revisions removed. - `API.compactPad(padID, authorId?)` (src/node/db/API.ts): public-API wrapper around compactHistory. Reports `{removed}` so callers can log savings. - `APIHandler.ts`: register `compactPad` under a new `1.3.1` version, bump `latestApiVersion`. - `bin/compactPad.ts`: admin CLI. Reports the current revision count, calls compactPad via the HTTP API, and prints how many revisions were dropped. - `src/tests/backend/specs/compactPad.ts`: four backend tests cover the empty-pad no-op, the text-preservation + head=0 contract, saved-revision cleanup, and that subsequent edits continue to append cleanly on top of the collapsed base. The operation is destructive so admins must opt in explicitly; the CLI prints the before-count, and the recommended pre-flight is an `.etherpad` export (backup). Closes #6194 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(compact): delegate to copyPadWithoutHistory via temp-pad swap The initial compactHistory() implementation built a custom base changeset and re-ran appendRevision against a reset atext — but the changeset was packed with oldLength=2 (matching copyPadWithoutHistory's dest-pad init state) while the reset atext was only length 1, so applyToText tripped its "mismatched apply: 1 / 2" assertion and every test failed with a Changeset corruption error. Switch to the tested path instead: copy the pad via copyPadWithoutHistory to a uniquely-named temp pad (inherits all its attribute/pool/changeset correctness), read the temp pad's rev records back, delete the old ones under our pad's ID, write the new records in their place, update in-memory state to match, and remove the temp pad. Errors at any step fall through with a best-effort temp-pad cleanup. Contract shifts slightly: the collapsed pad is head<=1 rather than head=0, matching the shape of a freshly-imported pad (seed rev 0 + content rev 1). Tests updated to assert that invariant plus text-preservation, saved-revision cleanup, and append-after-compact. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(6194): match the head<=1 post-compact contract Tests previously asserted head=0 exactly after compaction; the temp-pad-swap path lands at head=1 (one seed rev plus one content rev) matching the shape of a freshly-imported pad. Relax the assertions to and derive the removed-count from before-head minus after-head, so the tests still catch regressions in text-preservation, saved-revision cleanup, and append-after-compact without being tied to the exact implementation shape. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(6194): wrap existing Cleanup instead of duplicating it Develop already ships a working revision-cleanup path under `src/node/utils/Cleanup.ts` with two public helpers — `deleteAllRevisions(padId)` (collapse full history via copyPadWithoutHistory) and `deleteRevisions(padId, keepRevisions)` (keep the last N). The admin-settings UI wires these up but neither is exposed on the public API, and there's no CLI for operators who want to run compaction outside the web UI. That's the gap this PR now fills. Changes from the prior revision of this PR: - Drop `pad.compactHistory()` — it re-implemented what `Cleanup.deleteAllRevisions` already does. Remove the duplicate. - `API.compactPad(padID, keepRevisions?)` now delegates to Cleanup: • keepRevisions null/undefined → deleteAllRevisions (full collapse) • keepRevisions >= 0 → deleteRevisions(N) (keep last N) Returns {ok, mode: 'all' | 'keepLast', keepRevisions?}. - APIHandler `1.3.1`: signature updated to take `keepRevisions` instead of `authorId`. - `bin/compactPad.ts`: accepts `--keep N` for the keep-last mode, shows before/after revision counts so operators see concrete savings. - Backend tests rewritten around the public API surface (mode reporting, text preservation, input validation) rather than internal method plumbing that no longer exists. Net: strictly a thin public-API and CLI veneer over already-tested Cleanup helpers. No new low-level logic. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(6194): assert content markers, not byte-exact atext Cleanup.deleteAllRevisions internally calls copyPadWithoutHistory twice (src → tempId, tempId → src with force=true), and each round trip normalizes trailing whitespace. That meant my byte-exact atext.text assertion failed in CI: expected: '...line 3\n\n\n' actual: '...line 3\n' Swap the comparisons to use content markers (marker-alpha / beta / gamma, keep-line-N). The test still catches the real regressions — if compactPad lost content those markers would disappear — without coupling to whitespace quirks of the existing Cleanup implementation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(6194): correct API param + document compactPad in http_api docs The 1.3.1 entry in APIHandler registered `['padID', 'authorId']`, but `API.compactPad` takes `(padID, keepRevisions)` and the CLI sends a `keepRevisions` query param. APIHandler.handle dispatches by URL field name, so the previous wiring silently dropped `keepRevisions` and never ran the keep-last branch over HTTP. - Register `['padID', 'keepRevisions']` so the handler forwards the CLI/HTTP arg into the API function. - Add HTTP-level dispatch tests that hit `/api/1.3.1/compactPad` with and without `keepRevisions`. The direct `api.compactPad()` tests bypass the handler and would have missed this regression. - Document compactPad in `doc/api/http_api.md` and `http_api.adoc`, and bump the documented latest version from 1.3.0 to 1.3.1 to match `latestApiVersion`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(6194): add bin/compactAllPads for per-instance bulk compaction `bin/compactPad <padID>` covers the case where you know which pad is fat. For "reclaim space across the whole instance," composing `listAllPads` + `compactPad` yourself is annoying; this script does it. - Walks every pad on the instance and compacts it (full collapse, or `--keep N` keep-last). - Per-pad failures don't abort the run — they're logged, counted, and the script exits 1 if any failed. - `--dry-run` lists pads + revision counts without writing anything, so operators can scope impact before committing. - Reports `before → after` per pad and a total reclaimed count. Deliberately not adding a `compactAllPads` HTTP API: bulk compaction over a single HTTP request means one giant response and a long-held connection. Operators who want this should run it locally, where they can see progress and kill it cleanly. Staleness gating ("only pads older than X days") is tracked separately as a follow-up. Also registers `compactPad` and `compactAllPads` script aliases in `bin/package.json` so they show up next to the other admin CLIs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(6194): cover the bin/compactAllPads loop logic Previous commit added the script but only exercised it by hand. The loop itself — error tolerance, dry-run gating, keep-last passthrough, the empty-instance and listAllPads-failure paths — had no automated coverage. - Refactor compactAllPads.ts to export `runCompactAll(api, opts, logger)` and `parseArgs(argv)`. The CLI shell wires them up to axios+APIKEY for production; tests use an in-memory `CompactAllApi` so we don't need to stand up the apikey-auth path in mocha. - Add 9 specs covering: arg parsing, full-collapse iteration, --keep N passthrough, --dry-run skipping writes, single-pad failure not aborting the run, pre-flight count failure tolerated, a listAllPads failure short-circuiting cleanly, the empty-instance no-op, and a final end-to-end test that runs `runCompactAll` against the real `/api/1.3.1/compactPad` handler over supertest+JWT to catch contract drift between the CompactAllApi shape and the HTTP endpoints. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(6194): address Qodo review — gate, integer check, SSL Three valid concerns from the Qodo review on 75a08a13: 1. **cleanup.enabled gate.** The admin/Cleanup-socket path checks `settings.cleanup.enabled` before doing anything destructive; the public API was bypassing that gate. Now `compactPad` mirrors the admin path's check and returns a clear apierror when disabled, so exposing the API doesn't accidentally widen the cleanup-opt-in surface. 2. **Number.isFinite → Number.isInteger.** `2.5` was finite and non-negative, so the old check let it through into `Cleanup.deleteRevisions`, which does revision-index arithmetic that assumes integer math. Reject at the API boundary instead of silently misbehaving. 3. **SSL-aware baseURL in the bin scripts.** Other bin scripts hardcode `http://`, but the rest of the codebase uses `settings.ssl ? 'https' : 'http'`. The compact CLIs now do the same, so they work against HTTPS deployments. (Other bin scripts carry the same bug but fixing them is out of scope for this PR.) Tests: - New spec: `rejects fractional keepRevisions` (2.5 with the old check passed; the new one rejects). - New spec: `refuses to run when cleanup.enabled is false`. The existing API tests opt in via a before-hook + restore, so they still cover the success path under the new gate. - API docs (`http_api.md` + `http_api.adoc`) document the gate and the new error message. Skipped Qodo concerns: - "Wrong compactPad parameters" — already fixed in 26e12ff7 (the param map now correctly says `keepRevisions`, not `authorId`). - "Unbounded revision deletions" / "No session eviction" / changeset base-length / padCreate hook — these all targeted the earlier on-Pad implementation that was refactored away. The current code wraps `Cleanup.deleteAllRevisions` / `deleteRevisions`, which already handle concurrency, locking, and hook semantics. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
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> |