mirror of
https://github.com/ether/etherpad-lite.git
synced 2026-07-17 16:47:05 +00:00
11 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
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
|
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
|
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
|
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
|
e39dbde887
|
feat(updater): tier 1 — notify admin and pad users of available updates (#7601)
* docs(updater): add four-tier auto-update design spec
Four-tier opt-in self-update subsystem (off / notify / manual / auto / autonomous).
GitHub Releases as source of truth; install-method auto-detection with admin
override; in-process execution with supervisor restart; 60s drain + announce;
auto-rollback on health-check failure with crash-loop guard. Pad-side severe/
vulnerable badge that does not leak the running version. Top-level adminEmail
with escalating cadence (weekly while vulnerable, monthly while severe).
Refs: docs/superpowers/specs/2026-04-25-auto-update-design.md
* docs(updater): add PR 1 (Tier 1 notify) implementation plan
Bite-sized TDD task breakdown for shipping Tier 1 notify only:
- VersionChecker, InstallMethodDetector, UpdatePolicy, Notifier, state modules
- /admin/update/status (admin-auth) and /api/version-status (public, no version leak)
- Admin UI banner + read-only update page + nav link
- Pad-side severe/vulnerable footer badge
- Settings: updates.* block + top-level adminEmail
- Tests: vitest unit + mocha integration + Playwright admin/pad
- CHANGELOG + doc/admin/updates.md
PRs 2-4 (manual/auto/autonomous) get their own plans after PR 1 lands.
* feat(updater): add shared types for auto-update subsystem
* feat(updater): clarify OutdatedLevel and EMPTY_STATE doc, drop path header
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* feat(updater): add semver helpers and vulnerable-below parser
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(updater): tighten semver regex to reject four-part versions
* feat(updater): add state persistence with schema validation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(updater): reject null email and array latest in state validation
typeof null === 'object' meant {email:null} passed the old isValid check,
which would crash downstream Notifier code reading email.severeAt. Likewise,
an array would pass the typeof latest === 'object' branch. Introduce
isPlainObject helper (null-safe, Array.isArray guard) and use it for both
fields. Adds two regression tests covering the exact broken inputs.
* feat(updater): add install-method detector with override
* feat(updater): add policy evaluator
* feat(updater): add GitHub Releases checker with ETag support
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(updater): validate release fields and preserve ETag on prerelease
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* feat(updater): add email cadence decider
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(updater): tagChanged email fires regardless of cadence; drop unused field
* feat(settings): add updates.* and adminEmail settings
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* feat(updater): wire boot hook and periodic checker
Register expressCreateServer/shutdown hooks in ep.json and implement
the boot-wiring module that detects install method, starts the polling
interval and runs the notifier dedupe pass each tick.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* feat(updater): add /admin/update/status and /api/version-status endpoints
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* i18n(updater): add english strings for update banner, page, and pad badge
* feat(updater): add pad footer badge for severe/vulnerable status
* feat(admin-ui): add update banner, page, and nav link
Add UpdateStatusPayload to the zustand store, a persistent UpdateBanner
rendered in the App layout, a /update page showing version details and
changelog, and a Bell nav link — all wired to the /admin/update/status
endpoint added in Task 10.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* test(updater): add Playwright specs for admin banner/page and pad badge
* docs(updater): document tier 1 settings, badge, email cadence
* refactor(updater): dedupe helpers, fix misleading log, add banner styling
- Export stateFilePath from index.ts and import it in updateStatus.ts (removes local duplicate)
- Import getEpVersion from Settings.ts in both index.ts and updateStatus.ts (removes two local definitions)
- Fix misleading 'backing off' log message — no backoff is implemented, just retries at next interval
- Remove EMPTY_STATE_FOR_TESTS re-export from state.ts; state.test.ts now imports EMPTY_STATE directly from types.ts
- Add .update-banner and .update-page CSS rules to admin/src/index.css
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(updater): address review feedback — async wrap, tier=off skip, poll race, opt-in admin gate
- Wrap /api/version-status and /admin/update/status with a small async helper
so a rejected promise becomes next(err) instead of an unhandled rejection.
- Short-circuit route registration when updates.tier === 'off' so the heavier
opt-out also removes the HTTP surface (matches pre-PR behavior for that case).
- Add an in-flight guard around performCheck() so overlapping interval ticks
can't race on update-state.json writes or duplicate email decisions; track
the initial setTimeout handle and clear it in shutdown().
- Add updates.requireAdminForStatus (default false) so admins can lock
/admin/update/status to authenticated admin sessions without disabling the
updater. Default false preserves current behavior (the running version is
already exposed publicly via /health). Backend specs cover unauth → 401,
non-admin → 403, admin → 200.
- Bump admin troubleshooting menu count test 5 → 6 to account for the new
Update nav link.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(updater): address Qodo round-2 review feedback
Round 2 of Qodo review on #7601. Addressing the action-required items:
#1 Badge bypassed pad baseURL — derive basePath the same way
padBootstrap.js does (`new URL('..', window.location.href).pathname`)
and prefix the fetch with it. Subpath deployments now reach
/<prefix>/api/version-status instead of 404ing.
#2 Updater poller could get stuck — `getCurrentState()` is now inside
the try/finally so a one-time loadState() rejection can't leave
`checkInFlight=true` and permanently silence polling.
#3 Updates off hung admin page — UpdatePage now self-fetches and
renders explicit `disabled` (404), `unauthorized` (401/403), and
`error` states instead of staying on "Loading...". Banner-driven
prefetch is still honoured if it landed first.
#11 NaN polling interval — coerce `checkIntervalHours` to a number,
clamp to [1h, 168h], log a warning and fall back to 6h on
non-finite input. Math.max(1, NaN) === NaN previously meant a
malformed settings.json could turn the poller into a tight loop.
#13 State validation accepted broken subfields — `isValid()` now
inspects `latest.{version,tag,body,publishedAt,htmlUrl,prerelease}`,
`vulnerableBelow[].{announcedBy,threshold}`, and
`email.{severeAt,vulnerableAt,vulnerableNewReleaseTag}`. A
hand-edited file with a number where a string is expected is now
treated as corrupt and reset to EMPTY_STATE rather than crashing
later in semver parsing or email rendering.
#14 Badge cache stampede — wrap `computeOutdated()` in a single-flight
promise so concurrent requests at cache expiry await one shared
computation instead of fanning out into N redundant disk reads.
Plus six new state.test.ts cases covering each new validation guard.
Pushing back on the remaining items:
#4 `updates.tier` defaults to `notify` — intentional. The whole point
of tier 1 is to surface the "you are behind" signal to admins by
default. Opt-in defeats the purpose; the existing failure mode
(admin never hears about a security-relevant release) is exactly
what this PR is fixing.
#5/#8 Admin status endpoint admin-auth — `currentVersion` is already
public via `/health`, so wrapping the route in admin-auth doesn't
reduce the disclosure surface meaningfully. Operators who want it
gated set `updates.requireAdminForStatus=true` (already wired and
covered by the comment on the route handler).
#10 Plain `https://` URLs in planning doc — planning markdown is
viewed in editors and on GitHub where protocol-relative URLs would
either render literally or break entirely. Keeping `https://`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
|
||
|
John McLear
|
85f9a5f2f5
|
feat: Open Graph & Twitter Card metadata for pad/timeslider/home (closes #7599) (#7635)
* docs(spec): Open Graph metadata for pad pages (issue #7599) Spec for adding og:* and twitter:card meta tags to /p/:pad, the timeslider, and the homepage so shared links unfurl with a useful preview in chat apps. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(spec): expand OG spec — i18n (locale map + og:locale) and a11y (image:alt) Address review feedback: socialDescription accepts a per-language map, og:locale is emitted from the negotiated render language, and image:alt attributes are emitted for screen readers in chat clients. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat: emit Open Graph & Twitter Card metadata for pad/timeslider/home Closes #7599. Pad URLs shared in chat apps (WhatsApp, Signal, Slack, etc.) previously unfurled with no preview because the rendered HTML carried no OG or Twitter Card metadata. This change emits og:title, og:description, og:image, og:url, og:site_name, og:type, og:locale, og:image:alt and the equivalent twitter:* tags on the pad page, the timeslider, and the homepage. A new settings.json key `socialDescription` controls the description. It accepts either a plain string applied to every locale or a per-language map keyed by BCP-47 tag with an optional `default` fallback. og:locale is emitted from the language already negotiated via req.acceptsLanguages and og:image:alt provides screen-reader text for chat-client previews. Pad names from the URL are HTML-escaped before being interpolated into og:title to prevent reflected XSS via crafted pad IDs. Tests: src/tests/backend/specs/socialMeta.ts covers the default, per-locale override, locale fallback, URL decoding, XSS escape, and the timeslider/homepage variants. Semver: minor (new setting; templates emit additional tags but no existing behavior changes). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(test): use valid pad-name char in URL-decode test Spaces aren't allowed in pad names — Etherpad redirected /p/Has%20Space* to a sanitized name (302), so the og:title assertion failed. Use %2D ("-") instead, which is a valid pad-name character and still exercises the URL-decode path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(socialMeta): don't double-decode pad name from req.params.pad Express has already URL-decoded :pad route params before they reach the handler. Calling decodeURIComponent on the result throws URIError for pad names containing a literal "%" — e.g. the URL /p/100%25 yields req.params.pad === "100%", and decodeURIComponent("100%") throws. This would have prevented the page from rendering for some valid pad IDs. Drop the redundant decode and add a regression test for the "%" case. Reported by Qodo on PR #7635. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(socialMeta): source description from i18n catalog, drop settings key Per review: the OG description is a translatable string and belongs in Etherpad's locale files alongside the rest of the UI strings, not in settings.json. Operators who want to override it per-language continue to use the standard customLocaleStrings mechanism — no new config surface. Changes: - Add "pad.social.description" to src/locales/en.json (default English). - Export i18n.locales so server-side renderers can look up translations. - socialMeta.renderSocialMeta now takes a `locales` map and resolves renderLang → primary subtag → en, instead of taking a per-locale map from settings. - Remove `socialDescription` from Settings.ts, settings.json.template, settings.json.docker (the key never shipped). - Update tests and spec doc to reflect i18n-sourced description. Reported by Qodo on PR #7635 (also confirmed feature is fine to land default-on; no flag needed). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(socialMeta): add unit tests for pure helpers 21 cases exercising buildSocialMetaHtml and renderSocialMeta directly, without HTTP/DB. Covers tag enumeration, HTML escaping, og:locale region formatting, title composition (pad/timeslider/home), description i18n resolution (exact/primary/en fallback, missing catalog), image URL (default favicon vs absolute settings.favicon vs alt text), canonical URL building with query-string stripping, the literal "%" no-throw regression, and attribute-breakout escape. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(socialMeta): defend og:url/og:image against host-header poisoning Previously og:url and og:image were built from req.protocol + req.get('host'), both of which can be client-controlled (Host header directly, or X-Forwarded-* under trust proxy). A crafted Host could make the server emit OG tags pointing at an attacker's origin — harmful if any cache fronts the response or if a vulnerable proxy forwards the headers unsanitized. Two-layer defense: 1. New optional setting `publicURL` lets operators pin the canonical origin used for shared link previews ("https://pad.example"). When set, og:url and og:image use it unconditionally. Sanitized at use time: must be http(s)://host[:port] with no path, no userinfo, no trailing slash; malformed values fall back to the request. 2. When `publicURL` is unset, the request-derived fallback now strictly validates the Host header against /^[a-z0-9]([a-z0-9.-]{0,253}[a-z0-9])?(:\d{1,5})?$/i and caps the scheme to "http"/"https". A crafted Host (CRLF injection, userinfo, "<script>") is replaced with "localhost" instead of being echoed into og:url. Reported by Qodo on PR #7635. Tests: 5 new unit cases covering publicURL preference, trailing-slash strip, malformed-publicURL fallback, Host validation, scheme cap. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(socialMeta): tighten types, drop `any` - `req: any` -> express `Request` (covers acceptsLanguages/protocol/get/originalUrl). - `settings: any` -> local `SocialMetaSettings` interface narrowed to the three fields we actually read (title/favicon/publicURL); avoids coupling to the full Settings module surface. - `availableLangs: {[k: string]: any}` -> `{[lang: string]: unknown}`; only keys are read, so values stay deliberately unconstrained. No runtime change. All 26 socialMeta unit tests still pass. Per Sam's review on #7635. --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
John McLear
|
7b9a5eb01a
|
fix(a11y): dialog semantics, focus management, icon labels, html lang (#7584)
* fix(a11y): negotiate lang/dir per request and set on <html>
Server-renders the html element with `lang` and `dir` matching the
client's Accept-Language header (negotiated against availableLangs from
i18n hooks). Falls back to `en`/`ltr` if no match.
This gives screen readers a correct document language during the brief
window before client-side html10n refines it (l10n.ts already sets both
attributes after locale data loads).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(a11y): dialog semantics on popups; fix aria-role typo on userlist
Adds role=dialog, aria-modal=true, and either aria-labelledby (when an
h1 is present) or aria-label (for popups without an h1) to:
- #settings, #import_export, #embed, #skin-variants (labelledby)
- #connectivity, #users, #mycolorpicker (aria-label)
Fixes the invalid aria-role="document" attribute on #otherusers; it's
now role=region with aria-live=polite so screen readers announce
collaborator joins/leaves.
Container aria-label values are English-only for now — Etherpad's
html10n implementation only supports localizing specific attributes
(title, alt, placeholder, etc), not aria-label on container nodes.
Localization can follow once html10n grows that affordance.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(a11y): focus management and Escape-to-close for popups
Three additions to toggleDropDown / _bodyKeyEvent:
- Remember the trigger element (document.activeElement) when opening
a popup, so we can restore focus when it closes.
- On open, focus the first focusable element inside the popup so
keyboard users land inside the dialog instead of staying on the
trigger button.
- Escape pressed while focus is inside a popup closes it, then the
restore-focus path runs and the trigger button is refocused.
Replaces the previous behavior where Escape from inside a popup did
nothing; users had to click outside to dismiss.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(a11y): make chaticon and chat header controls real buttons
- #chaticon: <div onclick> → <button type=button> with aria-label
- #titlecross / #titlesticky: <a onClick> → <button type=button>
with aria-label (Close chat / Pin chat to screen)
- Decorative chat-bubble glyph gets aria-hidden=true so it isn't
read alongside the button label
- #chatcounter labelled "Unread messages"
- Inline onclick attributes moved to chat.init() handlers
- CSS reset on the new buttons (transparent bg, no border, inherit
font/color) so they match the prior visual design
- :focus-visible outlines for keyboard users
Existing test selectors (#chaticon, #titlecross, #titlesticky) are
unchanged and continue to work — they never relied on element type.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(a11y): accessible names for icon-only toolbar/export controls
- Export links (#exportetherpada, #exporthtmla, #exportplaina,
#exportworda, #exportpdfa, #exportopena): added aria-label so the
link is announced as e.g. "Export as PDF". The inner icon span
gets aria-hidden=true so screen readers don't read both the icon
text and the link label.
- Show-more toolbar toggle (.show-more-icon-btn): converted from
<span> to <button type=button> with aria-label and aria-expanded.
The click handler now toggles aria-expanded alongside the
full-icons class so assistive tech reflects the open/closed state.
- Theme switcher knob: aria-label changed from "theme-switcher-knob"
(a class-style identifier, not human text) to "Toggle theme".
Aria-label values are English-only for now. Etherpad's html10n
implementation only localizes a fixed attribute list (title, alt,
placeholder, value, innerHTML, textContent); aria-label is not
included, so a clean l10n path requires a follow-up to either
extend html10n or set aria-label client-side after locale loads.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test(a11y): cover dialog semantics, html lang, icon button labels
New Playwright spec verifies the a11y guarantees added by this branch:
- <html> has a non-empty lang attribute
- settings/import_export/embed/users popups expose role=dialog,
aria-modal=true, and either aria-labelledby (when an h1 exists)
or aria-label (when none does)
- Escape from inside the settings popup closes it AND restores
focus to the trigger button
- Export links each carry a descriptive aria-label
- #chaticon is a real <button> with aria-label
- #titlecross / #titlesticky are real <button>s with aria-label
- #otherusers uses role=region + aria-live=polite + aria-label
(and the previous aria-role typo is gone)
- .show-more-icon-btn is a <button> with aria-label and
aria-expanded
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(a11y): address Qodo review feedback from PR #7584
1. Users Escape close broken - toggleDropDown('none') intentionally
skips the users module so switching between other popups doesn't
hide the user list. That meant Escape couldn't dismiss the Users
popup either. The Escape branch now checks for #users as the
focused popup and closes it explicitly (respecting stickyUsers)
before falling through to the normal close-all path.
2. Embed focus overridden - the rAF auto-focus in toggleDropDown
grabbed the first focusable descendant, which stole focus from
command handlers that target a specific control (notably the Embed
command's #linkinput). rAF now bails out if focus is already
inside the newly-opened popup.
3. Button click blurs :focus before toggleDropDown captures trigger -
discovered while investigating the Firefox Playwright failure for
"settings popup Escape restores focus". Button.bind() calls
$(':focus').trigger('blur') before invoking the callback, so by
the time toggleDropDown() captured document.activeElement as the
restore target it was already <body>. The click handler now
stashes padeditbar._lastTrigger to the clicked <button> before
blur runs; toggleDropDown only falls back to activeElement when
the pre-stash didn't happen (keyboard shortcut path).
4. html10n overwrites aria-label - html10n unconditionally set
aria-label to the translated string, clobbering explicit aria-label
on elements that also carry data-l10n-id. setAttribute now only
fires when the element has no aria-label; explicit author labels
win, unlabelled translated elements still get a name.
5. Button visual reset - the show-more-icon-btn and #chaticon
conversions inherited UA default button border/background/padding,
shifting icon glyphs visibly off-centre. Added appearance /
background / border / padding resets.
6. Export links test assumes soffice is installed - #exportworda,
#exportpdfa, #exportopena are removed client-side by pad_impexp.ts
when clientVars.exportAvailable === 'no'. The test now skips links
absent at runtime.
Verified locally: all 10 a11y_dialogs specs pass on both Chromium and
Firefox; backend suite remains 799/799 passing; ts-check clean.
* fix(a11y): close popups with no focusable content; unbreak chat-icon layout
Round 2 of #7584 review follow-ups.
1. Users popup Escape still didn't close the dialog (user-confirmed).
Root cause: _bodyKeyEvent is bound to the OUTER document's body.
When #users opens, the command handler tries to focus
#myusernameedit but that input is `disabled`, so focus stays in the
ace editor iframe. Keydown from inside the iframe does not bubble
to the outer document, so Esc never reaches _bodyKeyEvent.
Fix: in the open-popup rAF, if no command handler placed focus
inside the dialog, focus the popup div itself (with tabindex=-1).
That keeps subsequent keydown events on the outer document so
Esc can dismiss the popup. Also broadened the Esc branch to fire
whenever any popup is `.popup-show`, regardless of where :focus
lives — some popups legitimately have no focusable content at
open.
Added a regression test that opens #users and asserts Esc closes
it. Passes on both Chromium and Firefox.
2. Chat icon (#chaticon) visual still wrong after the first CSS fix.
- My previous `border: 0` reset was overriding the intended
`border: 1px solid #ccc; border-bottom: none` from the earlier
rule. Removed `border: 0`; the earlier explicit border suffices
to suppress UA defaults.
- The `<span class="buttonicon">` inside `#chaticon` was picking
up the global `.buttonicon { display: flex; }` rule meant for
toolbar button instances, which broke the inline layout of the
label + glyph + counter row. Added a scoped
`#chaticon .buttonicon { display: inline; }` override.
All 11 a11y_dialogs specs pass on Chromium and Firefox. Backend
suite and ts-check remain clean.
* fix(a11y): only stash _lastTrigger for dropdown-opening buttons
Round 3 follow-up. The previous Button.bind() change stashed every
clicked toolbar button as padeditbar._lastTrigger before blurring :focus.
That was necessary for popup-opening buttons (settings, import_export,
etc.) so Escape could return focus to them — but it also fired for
non-popup toolbar buttons (list toggles, bold/italic, indent/outdent,
clearauthorship). For those, the stash held a stale reference that
interfered with subsequent editor interactions and regressed Playwright
tests: ordered_list, unordered_list, undo_clear_authorship.
Fix: only stash when the clicked command is a registered dropdown
(settings, import_export, embed, showusers, savedrevision,
connectivity). Other commands return focus to the ace editor as before
and leave _lastTrigger alone.
Verified locally on Chromium:
- ordered_list.spec.ts: 6/6 pass (was 4/6)
- unordered_list.spec.ts: 6/6 pass (was 4/6)
- undo_clear_authorship.spec.ts: 2/2 pass (was 0/2)
- a11y_dialogs.spec.ts: 11/11 pass (unchanged)
* fix(a11y): address Qodo review round 4 for PR #7584
#1 Stale aria-label after relocalize
html10n.translateNode() refused to overwrite any existing aria-label,
which also skipped updates on language change (pad.applyLanguage()
re-runs localize). Use a `data-l10n-aria-label="true"` marker: set
aria-label + marker when html10n populates it, overwrite only if the
marker is present. Explicit template-supplied aria-labels stay as-is;
html10n-generated ones refresh on relocalize.
#2 Escape won't close colorpicker
_bodyKeyEvent caught Escape on any `.popup.popup-show` but only
closed dropdown popups via toggleDropDown('none'). Popups opened
outside the editbar framework (#mycolorpicker, toggled directly by
pad_userlist.ts) stayed open while preventDefault() swallowed the
key. Now the Escape branch manually closes any popup that
toggleDropDown('none') cannot reach (non-dropdown ids, plus #users
unless pinned) and leaves registered dropdowns for toggleDropDown to
close so its focus-restore sees the transition.
#3 Stale focus restoration
toggleDropDown('none') restored focus to _lastTrigger even when no
popup was open on entry, which meant background callers
(connectivity setup, periodic state handling) could yank focus out
of the editor to a stale toolbar button. Gated the restore on
`wasAnyOpen === true` so it only fires when there was a popup to
close.
#11 English aria-label overrides i18n (export links, chat icon)
Removed the hard-coded English aria-label from export anchors and
removed aria-hidden from their inner localized spans. Screen readers
now get the localized child text as the accessible name (Etherpad,
HTML, PDF, etc.), matching the visible UI language.
Removed the English aria-label from #chaticon and #titlesticky as
well — both have data-l10n-id, so html10n populates a localized
aria-label via the marker mechanism in #1. #titlecross keeps its
static aria-label because it has no data-l10n-id yet.
#4 4-space indent in a11y spec
Two tests had continuation lines at 4-space indent violating the
repo's 2-space rule. Folded the signatures onto one line.
Updated a11y_dialogs.spec.ts to assert accessible-name presence rather
than hard-coded English for elements whose names now come from the
localized text. Still asserts static English for #titlecross (not
localized yet).
Verified locally (dev server restarted for each round):
- a11y_dialogs.spec.ts: 11/11 on Chromium, 11/11 on Firefox
- ordered_list + unordered_list + undo_clear_authorship: 13/13 on Chromium
- Full backend suite: 799 passing, 0 failing
- tsc --noEmit clean in our code
#9 Popup behavior documentation: deferred to a follow-up doc PR so
this PR stays focused on the a11y code changes. The new keyboard
behavior (Escape-to-close, focus-restore-to-trigger) is small enough
to summarize in a short doc/ addition.
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
|
John McLear
|
053f6d8343
|
fix(#7570): bundle DB drivers, add regression CI (#7572)
* docs: design spec for issue #7570 (ueberdb2 driver bundling) Spec for the upstream ueberDB fix (move 10 drivers back from optional peer deps to dependencies) plus downstream etherpad-lite safety net (explicit driver list + build-test-db-drivers CI job covering all 10 via presence check and MySQL+Postgres smoke tests). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs: implementation plan for issue #7570 ueberdb2 driver bundling Covers upstream ueberDB PR (move drivers from optional peer deps back to dependencies, publish 5.0.46) and downstream etherpad-lite PR (bump ueberdb2, defensive driver list, build-test-db-drivers CI job with presence + MySQL + Postgres stages gating publish). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(#7570): bundle DB drivers, add regression CI - Bump ueberdb2 to ^5.0.47 (upstream ueberDB PR #939 re-bundles drivers as real dependencies instead of optional peer deps, fixing the class of Docker-prod "Cannot find module" failures). - Declare all 10 ueberdb2 DB drivers as direct src dependencies as a defensive safety net against a future upstream drift. - Add build-test-db-drivers CI job that blocks the publish job: * all-10-drivers presence check in the built prod image * end-to-end MySQL smoke (reproduces the #7570 repro) * end-to-end Postgres smoke Any stage failure blocks Docker Hub / GHCR publish. Supersedes #7571. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(ci): run driver presence test from src/ so node_modules resolves The presence test ran node from the default cwd (/opt/etherpad-lite), but the drivers are installed under /opt/etherpad-lite/src/node_modules by the monorepo workspace. Adding `-w /opt/etherpad-lite/src` makes Node resolve modules from src/node_modules where pnpm places them. Matches how the production container itself runs: `pnpm run prod` is invoked from src/ (cross-env + node --require tsx/cjs node/server.ts). --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |