mirror of
https://github.com/ether/etherpad-lite.git
synced 2026-07-18 00:57:55 +00:00
36 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
John McLear
|
f47626049c
|
fix(pad): don't issue a deletion token (or show its modal) when allowPadDeletionByAllUsers is on (#7929)
When `allowPadDeletionByAllUsers` is true, anyone can delete a pad with no token at all (handlePadDelete's flagOk branch), so the one-time deletion token is pointless and the "Save your pad deletion token" modal only overwhelms users who will never need it. Gate token issuance on `!settings.allowPadDeletionByAllUsers` so no token reaches clientVars; the client's showDeletionTokenModalIfPresent() then returns early and the modal never appears. No new setting — it derives automatically from the existing one. Closes #7926. Co-authored-by: Claude Opus 4.8 (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
|
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
|
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
|
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
|
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
|
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
|
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
|
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> |
||
|
John McLear
|
6195289198
|
feat(gdpr): IP/privacy audit (PR2 of #6701) (#7547)
* docs: PR2 GDPR IP/privacy audit design spec Second of five GDPR PRs (#6701). Audit identifies four log-sites that leak IPs despite disableIPlogging=true, proposes a tri-state ipLogging setting with a back-compat shim, and specifies a doc/privacy.md that documents Etherpad's actual IP handling. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs: PR2 GDPR IP/privacy audit implementation plan 7 TDD-structured tasks: anonymizeIp helper + unit tests, tri-state ipLogging setting with disableIPlogging deprecation shim, wiring through 5 leaking log sites, clientVars.clientIp removal, access-log integration test, doc/privacy.md, and PR handoff. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(gdpr): anonymizeIp helper with v4/v6/v4-mapped truncation * feat(gdpr): tri-state ipLogging setting + disableIPlogging shim * fix(gdpr): route every IP log site through anonymizeIp Closes four leaks where disableIPlogging was silently ignored (rate-limit warn, both auth-log calls in webaccess, import/export rate-limit warn) and normalises the four that did honour the flag onto the new ipLogging tri-state via the shared helper. * chore(gdpr): drop dead clientVars.clientIp placeholder Server side: remove the literal '127.0.0.1' assignments from both clientVars and collab_client_vars. Type side: drop clientIp from ClientVarPayload and ServerVar. pad.getClientIp now returns the same '127.0.0.1' literal as a plugin-compat shim (pad_utils.uniqueId still uses it as a prefix). * test(gdpr): ipLogging modes + disableIPlogging shim * docs(gdpr): operator-facing privacy and IP handling statement * fix(gdpr): validate ipLogging at load + regression test for log sites Qodo review: - settings.ipLogging is loaded as a trusted union but nothing enforced the shape. An unknown value (e.g. a typo or null) silently fell through to anonymizeIp's "truncated" branch and emitted partially redacted IPs. Fall back to "anonymous" with a WARN at load time. - New regression test scans the four known log-sites for raw req.ip / socket.request.ip / request.ip inside logger calls that don't wrap through anonymizeIp / logIp, so a future edit that re-introduces a raw IP fails CI. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
John McLear
|
9e352ca4f3
|
fix(clientVars): stop mutating the shared plugin registry during sanitization (#7587)
PadMessageHandler built the `pluginsSanitized` payload for clientVars by
aliasing `plugins.plugins` and then mutating each entry's `package` field
in place:
let pluginsSanitized: any = plugins.plugins;
Object.keys(plugins.plugins).forEach(function(element) {
const p: any = plugins.plugins[element].package;
pluginsSanitized[element].package = {name: p.name, version: p.version};
});
Because `pluginsSanitized` is a reference to `plugins.plugins`, the
assignment clobbered the server-side plugin registry. After the first
pad connection, every plugin's `package` object held only `{name,
version}` — `realPath`, `path`, and `location` were gone.
Minify.ts resolves `/static/plugins/ep_*/...` URLs via
`plugin.package.realPath`. Once the field disappeared, every subsequent
static asset request for a bundled plugin 500'd with:
TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of
type string. Received undefined
at Object.join (node:path:1354:7)
at _minify (src/node/utils/Minify.ts:181:23)
Symptoms on Chromium: plugin CSS/JS assets fail to load (e.g.
/static/plugins/ep_font_size/static/css/size.css returns 500), so
plugins partially render or don't work at all. Firefox swallows the
resulting console errors quietly.
Fix: extract the sanitization into a pure helper `sanitizePluginsForWire`
that returns a fresh object graph and never touches the input. The
helper is covered by a new backend spec that:
* verifies the sanitized output has only {name, version} in `package`
* asserts the input registry's realPath/path/location survive the call
* runs the call repeatedly and confirms non-destructiveness
* mutates the returned copy and asserts the input is independent
Verified live with the dev server: before the fix, `/static/plugins/
ep_font_size/static/css/size.css` 500'd after visiting any pad; after
the fix it returns 200 both before and after pad connections.
|
||
Stefan Müller
|
fe6a373bf8
|
feat: Remove paths from plugin packages (#7580) | ||
|
John McLear
|
e0ccdb4d9f
|
Add creator-owned pad settings defaults (#7545)
* Add creator-owned pad settings defaults Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Refine pad settings layout Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix settings popup heading and width Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Explain enforced user settings Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Cover creator override flow Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Let creators bypass enforced settings Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address pad settings follow-ups Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
John McLear
|
4137109efe
|
fix: allow undo of clear authorship colors without disconnect (#7430)
* fix: allow undo of clear authorship colors without disconnect (#2802) When a user clears authorship colors and then undoes, the undo changeset re-applies author attributes for all authors who contributed text. The server was rejecting this because it treated any changeset containing another author's ID as impersonation, disconnecting the user. The fix distinguishes between: - '+' ops (new text): still reject if attributed to another author - '=' ops (attribute changes on existing text): allow restoring other authors' attributes, which is needed for undo of clear authorship Also removes the client-side workaround in undomodule.ts that prevented clear authorship from being undone at all, and adds backend + frontend tests covering the multi-author undo scenario. Fixes: https://github.com/ether/etherpad-lite/issues/2802 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: use robust Playwright assertions in authorship undo tests - Use toHaveAttribute with regex instead of raw getAttribute + toContain - Check div/span attributes within pad body instead of broad selectors - Use Playwright auto-retry (expect with timeout) instead of toHaveCount(0) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: handle confirm dialog and sync timing in Playwright tests - Add page.on('dialog') handler to accept the confirm dialog triggered by clearAuthorship when no text is selected (clears whole pad) - Use auto-retrying toHaveAttribute assertions instead of raw getAttribute - Increase cross-user sync timeouts to 15s for CI reliability - Add retries: 2 to multi-user test for CI flakiness - Scope assertions to pad body spans instead of broad selectors Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: use persistent socket listeners to avoid missing messages in CI Replace sequential waitForSocketEvent loops with single persistent listeners that filter messages inline. This prevents race conditions where messages arrive between off/on listener cycles, causing timeouts on slower CI runners. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: reject - ops with foreign author to prevent pool injection The '-' op attribs are discarded from the document but still get added to the pad's attribute pool by moveOpsToNewPool. Without this check, an attacker could inject a fabricated author ID into the pool via a '-' op, then use a '=' op to attribute text to that fabricated author (bypassing the pool existence check). Now all non-'=' ops (+, -) with foreign author IDs are rejected. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test: use not.toHaveClass for cleared authorship spans Addresses Qodo review: linestylefilter skips attribs with empty values, so a span with author='' has no class attribute at all. The previous negative-lookahead regex on the class attribute failed against a null attribute and was flaky in CI. Switch to not.toHaveClass(/author-/), which also passes when the attribute is missing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> |
||
|
John McLear
|
7ec581afca
|
feat!: replace Abiword with LibreOffice and add DOCX export (#7539)
* feat!: replace Abiword with LibreOffice and add DOCX export (#4805)
The Abiword converter is dropped. Abiword's DOCX export is weak and the
project is niche on modern platforms; LibreOffice (soffice) is the
common deployment path and now serves as the sole converter backend.
DOCX is added as an export format and becomes the new target for the
"Microsoft Word" UI button. The /export/doc URL still works for legacy
API consumers.
BREAKING CHANGE: The 'abiword' setting, the INSTALL_ABIWORD Dockerfile
build arg, the abiwordAvailable clientVar, and the
#importmessageabiword UI element (with locale key
pad.importExport.abiword.innerHTML) are removed. Deployments relying on
Abiword must configure 'soffice' instead.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat: add docxExport feature flag and abiword deprecation WARN
- Add `docxExport: true` setting to opt out of DOCX (use legacy DOC)
- Pass `docxExport` to client via clientVars
- Use `docxExport` flag in pad_impexp.ts for Word button format
- Emit a specific WARN when deprecated `abiword` config is detected
- Update settings.json.template and settings.json.docker with docxExport
- Add docxExport to ClientVarPayload type in SocketIOMessage.ts
Agent-Logs-Url: https://github.com/ether/etherpad/sessions/9afc5291-73b2-4b66-b028-feed39e7056f
Co-authored-by: JohnMcLear <220864+JohnMcLear@users.noreply.github.com>
* refactor: extract wordFormat variable and improve docxExport comment
Agent-Logs-Url: https://github.com/ether/etherpad/sessions/9afc5291-73b2-4b66-b028-feed39e7056f
Co-authored-by: JohnMcLear <220864+JohnMcLear@users.noreply.github.com>
* fix: restore import-limitation message when no converter is configured
The abiword removal dropped both the #importmessageabiword DOM element
and its locale key, but Copilot's refactor still expected the show()
call to surface a message when exportAvailable === 'no'. Result: users
with no soffice binary got silent failure instead of an explanation.
Add #importmessagenoconverter back with updated, LibreOffice-focused
copy (new locale key pad.importExport.noConverter.innerHTML) and flip
the hidden prop when the client knows no converter is available.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* i18n: inline English fallback for noConverter import message
The original abiword message existed in ~70 locale files and was
removed from all of them by this PR. The replacement key was only
added to en.json, so non-English users had an empty div until
translators localize. Follow the project's usual pad.html pattern
(e.g. line 146's "Font type:") and include the English text inside
the div as the fallback content; html10n replaces it when a
translation is available.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Revert "i18n: inline English fallback for noConverter import message"
This reverts commit
|
||
|
John McLear
|
31e0a61126
|
fix: capture head revision atomically with atext to prevent mismatched apply (#7480)
* fix: capture head revision atomically with atext to prevent mismatched apply When constructing CLIENT_VARS, pad.atext was captured at one point but pad.getHeadRevisionNumber() was called later. If concurrent edits advanced the revision between these two reads, the client received initialAttributedText from rev N but rev=N+3, causing "mismatched apply" errors when the next changeset arrived (expecting rev N+3 text). Now captures headRev at the same time as atext and uses the captured value consistently in CLIENT_VARS and sessionInfo. Fixes #4040 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: flush missed revisions after socket joins pad room During handleClientReady(), the server awaits the clientVars hook before socket.join(). Any revisions appended during that await window are broadcast to existing room members but the connecting socket misses them. Call updatePadClients(pad) after joining to flush any such revisions. Also adds a regression test that injects a slow clientVars hook and verifies the connecting client receives catch-up changesets for edits that occurred during the hook await window. Fixes #4040 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test: fix race condition in clientVars hook test Listen for messages during handshake to avoid missing NEW_CHANGES that arrive before the explicit waitForSocketEvent listener is attached. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: initialize sessionInfo.time before catch-up updatePadClients The catch-up updatePadClients() call introduced in this PR could send NEW_CHANGES with timeDelta=NaN because sessionInfo.time was never set for new sessions. NaN poisons the client-side broadcast/timeslider currentTime tracking. Initialize sessionInfo.time to the timestamp of the snapshot revision before the catch-up flush, with a fallback to Date.now() if the revision date is unavailable. Also strengthens the regression tests: - Validate that initialAttributedText matches the pad AText at the EXACT advertised rev (not just the latest pad text), using pad.getInternalRevisionAText(rev). - Add a load test that hammers the pad with concurrent edits while multiple clients connect, asserting CLIENT_VARS consistency under the exact race condition the fix is targeting. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test: replace open-ended load loop with bounded mid-handshake edit The previous load test ran 'while (!stopLoad) await pad.setText(...)' in the background while the test connected clients. This saturated ueberDB's write queue and on shutdown the queued writes never drained, hanging the mocha process for the full 6h GitHub Actions job timeout. Replace it with a bounded approach: a clientVars hook lands 3 edits mid-handshake (deterministic, no background loop, no shutdown hang). Still exercises the exact race the fix targets — an edit advancing the rev after the atext snapshot but before CLIENT_VARS is sent — and asserts AText / rev consistency via getInternalRevisionAText. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test: address remaining Qodo concerns on PR #7480 Addresses Qodo review items 1, 2, 5 from https://github.com/ether/etherpad-lite/issues/comments/4194702740 : - Concern 1 (no loadTesting reproduction test): the suite now toggles settings.loadTest = true in before(), restores in after(). The middle test also pre-populates the pad with 20 revisions before connecting so we genuinely exercise a busy/loaded pad rather than a fresh one. - Concern 2 (no CLIENT_VARS / NEW_CHANGES delay test): the slow clientVars hook in the middle test now has explicit setTimeout delays before AND after the mid-handshake edits, so the race window between atext snapshot and CLIENT_VARS send is observably wide rather than relying on async scheduling alone. The test also collects post-handshake messages and asserts a NEW_CHANGES catch-up arrives when the pad advanced past the advertised rev. - Concern 5 (test doesn't validate rev): both rev-consistency tests use pad.getInternalRevisionAText(advertisedRev) and assert text and attribs match, not just `pad.text() === clientVars.text`. Concerns 3 (connect can miss revisions) and 4 (NaN timeDelta) were already addressed in earlier commits on this branch via the catch-up updatePadClients() call and the sessionInfo.time initialization. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> |
||
|
John McLear
|
474918a881
|
feat: make cookie names configurable with prefix setting (#7450)
* feat: make cookie names configurable with prefix setting Add cookie.prefix setting (default "ep_") that gets prepended to all cookie names set by Etherpad. This prevents conflicts with other applications on the same domain that use generic cookie names like "sessionID" or "token". Affected cookies: token, sessionID, language, prefs/prefsHttp, express_sid. The prefix is passed to the client via clientVars.cookiePrefix in the bootstrap templates so it's available before the handshake. Server-side cookie reads fall back to unprefixed names for backward compatibility during migration. Fixes #664 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: default cookie prefix to empty string for backward compatibility Changing the default to "ep_" would invalidate all existing sessions on upgrade since express-session only looks for the configured cookie name. Default to "" (no prefix) so upgrades are non-breaking — users opt-in to prefixed names by setting cookie.prefix in settings.json. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address Qodo review — cookie prefix migration and fallbacks - l10n.ts: Read prefixed language cookie with fallback to unprefixed - welcome.ts: Use cookiePrefix for token transfer reads - timeslider.ts: Use prefix for sessionID in socket messages - pad_cookie.ts: Fall back to unprefixed prefs cookie for migration - indexBootstrap.js: Pass cookiePrefix via clientVars to welcome page - specialpages.ts: Pass settings to indexBootstrap template Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: escape regex metacharacters in cookie prefix, document Vite hardcode - l10n.ts: Escape special regex characters in cookiePrefix before using it in RegExp constructor to prevent runtime errors - padViteBootstrap.js: Add comment noting the hardcoded prefix is dev-only and must match settings.json Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * security: validate cookie prefix to prevent header injection Reject cookie.prefix values containing characters outside [a-zA-Z0-9_-] to prevent HTTP header injection via crafted cookie names (e.g., \r\n sequences). Falls back to empty prefix with an error log. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> |
||
|
John McLear
|
66249b5d7e
|
fix: correct numConnectedUsers count for joining user (#7453)
numConnectedUsers in CLIENT_VARS was computed from roomSockets.length before the new socket joined the room, so the joining user always saw a count one less than the actual number. Added +1 to include the joining user in the count. Fixes #6145 Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> |
||
|
John McLear
|
a42e072900
|
fix: wait for server confirmation before navigating after pad delete (#7432)
* fix: wait for server confirmation before navigating after pad delete
The delete pad handler navigated to '/' immediately after sending the
PAD_DELETE message. Firefox (and some mobile Chrome) would close the
WebSocket before the message reached the server, causing the delete to
silently fail.
Now the client waits for the server's {disconnect: 'deleted'} response
before navigating. Also awaits pad.remove() on the server side to
ensure the operation completes before the response is sent.
Fixes: https://github.com/ether/etherpad-lite/issues/7306
Fixes: https://github.com/ether/etherpad-lite/issues/7311
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* fix: handle non-creator delete and add timeout fallback
- Listen for 'shout' event to show error when non-creator tries to
delete (server sends shoutMessage instead of deleting)
- Add 5-second timeout fallback in case the server doesn't respond
(socket dropped, server crashed, etc.)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
||
Phillip
|
06f67b4c01
|
fix prometheus metric and add total users and active pad count (#7179)
* fix prometheus metric registration * add totalUsers and activePads metric to prometheus |
||
SamTV12345
|
8588d99f12
|
chore: migrated settings to esm6 (#7062)
* chore: migrated settings to esm6 * chore: fixed frontends * chore: fixed missing usage of specialpages * chore: fixed last errors for settings * chore: fixed favicon test |
||
|
SamTV12345
|
920308a627
|
chore: moved first files to esm (#7061)
* chore: moved first files to esm * chore: moved first files to esm * chore: fix read only manager |
||
|
SamTV12345
|
60a40d53a7
|
chore: enabled dark mode (#7057) | ||
|
SamTV12345
|
1e3a61e5fb
|
feat(pad-settings): added possibility to delete pad by the creator (#6730) | ||
|
Stefan Müller
|
1ad9418a6f
|
Add code for revision cleanup (#6442)
* Add initial code for revision cleanup * Some improvements - code cleanup * Cleanup logging * Add button in admin backend to cleanup revisions of a specific pad * Disable cleanup by default and show errors in admin area * Improve cleanup code * Load revisions for cleanup in parallel * Consider saved revisions during pad cleanup |
||
|
Stefan Müller
|
113884d071
|
Fix timeslider datetime is wrong on new changes (#6651) | ||
|
SamTV12345
|
28e04bdf71
|
Feat/changeset ts (#6594)
* Migrated changeset * Added more tests. * Fixed test scopes |
||
|
SamTV12345
|
7e3ad03e2f
|
Moved to ts (#6593)
* Moved to ts * Fixed type check * Removed js suffixes * Migrated to ts * Fixed ts. * Fixed type check * Installed missing d ts |
||
|
SamTV12345
|
d6d636955c
|
Feat/bundle js (#6511)
* Added minify * Added POC for browser * Moved first js files to ts * Fixed caret positioning * Added support for plugins * Fixed get undefined. * Removed require of socketio, l10n, html10n and error reporter * Added minify * Added POC for browser * Moved first js files to ts * Fixed caret positioning * Added support for plugins * Fixed get undefined. * Removed require of socketio, l10n, html10n and error reporter * Fixed popup not showing * Fixed timeslider * Reworked paths * Fixed loading * Don't generate sources map in production mode * Non working hmr * Added live reloading. * Fixed timeslider when hot reloading * Removed eval * Fixed. * Fixed env * Fixed frontend tests. * Added minifying via lightningcss * Added minify via esbuild * Fixed diagnostic url * Removed lightningcss * Fixed types * Fixed alias * Fixed loadtest * Fixed * Fixed loading ep_font_color3 * Restructure windows build * Fixed windows build * Fixed pnpm lock --------- Co-authored-by: SamTv12345 <samtv12345@samtv12345.com> |
||
|
SamTV12345
|
e2233b61c9
|
Fixed totalUsers being undefined thus not being displayed. (#6342) | ||
|
SamTV12345
|
29c776d30d | Fixed kickSessions method. | ||
|
SamTV12345
|
d34b964cc2
|
Fixed frontend tests. (#6210)
* Fixed frontend tests. * Use old socket io syntax. * uSE ESM: * Remove padvar. * Remove cypress. |
||
Hossein Marzban
|
4887cd952a
|
Revise transport Socket.io@3/4 (#6188)
* feat :migrate socket.io 2 -> 3 * fix: backend test * fix: ts error * rm * reset the test timeout * fix: socket transports * fix: ts * fix: merge * fix: merge * resolve merge * clean * clean |
||
|
SamTV12345
|
295a2a758b
|
Added backend in typescript. (#6185) |
Renamed from src/node/handler/PadMessageHandler.js (Browse further)