Skip to content

Commit 4878420

Browse files
JohnMcLearclaude
andauthored
feat(gdpr): configurable privacy banner (PR4 of #6701) (#7549)
* docs: PR4 GDPR privacy banner design spec * docs: PR4 GDPR privacy banner implementation plan * feat(gdpr): typed privacyBanner setting block + public getter exposure * feat(gdpr): send privacyBanner config to the browser via clientVars * feat(gdpr): privacy banner DOM (hidden by default) * feat(gdpr): render privacy banner on pad load when enabled * style(gdpr): privacy banner layout * test+fix(gdpr): privacy banner Playwright + hidden-attr CSS override * docs(gdpr): privacyBanner configuration section * fix(gdpr): reject unsafe learnMoreUrl schemes Qodo review: showPrivacyBannerIfEnabled assigned config.learnMoreUrl directly to <a href>, so a misconfigured settings.privacyBanner. learnMoreUrl of `javascript:alert(1)` or `data:…<script>…` would run script on click. Validate via URL parsing and allow only http(s) / mailto; everything else yields no link. Playwright regression guards the four cases (javascript, data, https, mailto). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(privacy-banner): drop unneeded !important on [hidden] rule Class+attribute selector already outranks `.privacy-banner { display: flex }` on specificity (0,2,0 vs 0,1,0), so `!important` was redundant. Adds a comment explaining why so a future reader doesn't put it back. Per Sam's review on #7549. * refactor(privacy-banner): render as a persistent gritter, not custom DOM Drops the bespoke #privacy-banner template + ~50 lines of popup.css and delegates to $.gritter.add({sticky: true, position: 'bottom'}). The notice now matches every other gritter on the pad (theme variables, shadow, animation, (X) close), sits in the bottom corner instead of above the editor, and inherits dark-mode handling for free. The two dismissal modes survive intact: - dismissible: gritter closes on (X); before_close persists a flag in localStorage so the notice is suppressed on subsequent loads. - sticky: closes for the current session only; never persists; the next pad load shows it again. learnMoreUrl still goes through the same safeUrl() filter so a javascript:/data:/vbscript: URL can't smuggle a script handler into the anchor (Qodo's review concern remains addressed). Tests: src/tests/frontend-new/specs/privacy_banner.spec.ts now drives the real showPrivacyBannerIfEnabled via a __etherpad_privacyBanner__ test hook and asserts against the rendered gritter, instead of the previous tests that mutated DOM by hand and never exercised the function under test. Coverage adds: enabled=false short-circuit, dismissible-flag-respected on subsequent show, sticky-ignores-flag, sticky-close-does-not-persist, javascript: rejection, data: rejection, and mailto: allow-list. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(privacy-banner): noreferrer + validate dismissal (Qodo) Two follow-ups from Qodo's review on #7549: 1. The Learn-more link now sets `rel="noreferrer noopener"` (was just `noopener`). Without `noreferrer` the browser sends the pad URL as a Referer to the operator-configured external policy site, which leaks pad identifiers to a third party. Matches the rel pattern already used by pad_utils.ts. 2. `privacyBanner.dismissal` is now validated in reloadSettings(): an unknown value falls back to 'dismissible' with a `logger.warn`, in the same shape as the existing ipLogging validation a few lines up. The client also guards defensively (treats anything other than the exact string 'sticky' as 'dismissible') so that hot-reload paths that skip the server validator can't silently degrade a typo'd 'sticky' into "no close button persisted, no localStorage suppression". Test added: spec asserts the rel attribute, and a new test exercises the dismissal fallback (sets dismissal:'wat', asserts the gritter is shown, the (X) closes it, and the dismissal flag is persisted — i.e. the unknown value is treated like 'dismissible'). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(privacy-banner): gate test hook on webdriver, align doc with sticky behavior Two follow-ups from Qodo's second review on #7549. Rule violation: __etherpad_privacyBanner__ was published on every pad load even when privacyBanner.enabled was false, so the disabled-by- default feature still added an observable global. Gate the assignment on `navigator.webdriver` — Playwright/ChromeDriver/Selenium set this to true; production browsers do not — so the hook is only present for tests and the disabled path is genuinely zero-side-effect. Bug 3 (sticky still closable): doc/privacy.md previously claimed `dismissal: "sticky"` removes the close button, but the gritter implementation always renders (X). Aligning the doc with reality — sticky now means "shows on every load, but closable for the session" — rather than adding bespoke CSS to a vanilla gritter (matches the "don't style it differently than other gritter messages" preference that drove the gritter migration in 906e145). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(privacy-banner): allow-list keys before sending to clientVars (Qodo) storeSettings() merges nested objects with _.defaults() and preserves unknown nested keys, and TypeScript's Pick<> doesn't strip at runtime. The previous wire path forwarded settings.privacyBanner by reference into both clientVars and getPublicSettings(), so any extra keys an operator typed (or pasted) under privacyBanner — credentials, internal notes, anything — would have shipped to every browser on every pad load. Adds getPublicPrivacyBanner() in Settings.ts that returns a literal with only {enabled, title, body, learnMoreUrl, dismissal}, and uses it from both leak sites (PadMessageHandler.ts clientVars and getPublicSettings()). Single source of truth for the wire shape. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent a0b85dd commit 4878420

12 files changed

Lines changed: 1229 additions & 2 deletions

File tree

doc/privacy.md

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,3 +59,38 @@ See
5959
[`docs/superpowers/specs/2026-04-18-gdpr-pr1-deletion-controls-design.md`](https://github.com/ether/etherpad/blob/develop/docs/superpowers/specs/2026-04-18-gdpr-pr1-deletion-controls-design.md)
6060
for the deletion-token mechanism. Full author erasure is tracked as a
6161
follow-up in [ether/etherpad#6701](https://github.com/ether/etherpad/issues/6701).
62+
63+
## Privacy banner (optional)
64+
65+
The `privacyBanner` block in `settings.json` lets you display a short
66+
notice to every pad user — data-processing statement, retention
67+
policy, contact for erasure requests, etc.
68+
69+
```jsonc
70+
"privacyBanner": {
71+
"enabled": true,
72+
"title": "Privacy notice",
73+
"body": "This instance stores pad content for 90 days. Contact privacy@example.com to request erasure.",
74+
"learnMoreUrl": "https://example.com/privacy",
75+
"dismissal": "dismissible"
76+
}
77+
```
78+
79+
The banner is rendered as a persistent gritter notification at the
80+
bottom of the page (it inherits the same look as every other gritter
81+
on the pad — no custom skin needed). The body is plain text (HTML is
82+
escaped); each line becomes its own paragraph.
83+
84+
`dismissal` controls how the close (×) is handled:
85+
86+
- `"dismissible"` (default) — when the user closes the gritter, the
87+
choice is persisted in `localStorage` per origin and the banner is
88+
not shown again on subsequent pad loads.
89+
- `"sticky"` — closing the gritter only hides it for the current
90+
session; the next pad load shows it again. (The close control is
91+
not removed; for an operator-enforced non-closable notice, render
92+
the policy out-of-band — e.g., a skin override or a reverse-proxy
93+
ribbon.)
94+
95+
Unknown `dismissal` values are coerced to `"dismissible"` with a
96+
`logger.warn` at settings load.

0 commit comments

Comments
 (0)