feat(gdpr): configurable privacy banner (PR4 of #6701)

[JohnMcLear]

Summary

• New privacyBanner block in settings.json (enabled / title / body / learnMoreUrl / dismissal). Defaults to disabled so existing instances are unchanged.
• Banner config ships to the browser via clientVars.privacyBanner; client renders it with textContent (HTML is escaped).
• dismissible stores a per-origin flag in localStorage so the user sees the notice once; sticky shows it every load.

Part of the GDPR work in #6701. PR1 #7546, PR2 #7547, PR3 #7548 already in flight. PR5 (author erasure) is the last.

Design: docs/superpowers/specs/2026-04-19-gdpr-pr4-privacy-banner-design.md
Plan: docs/superpowers/plans/2026-04-19-gdpr-pr4-privacy-banner.md

Test plan

• pnpm --filter ep_etherpad-lite run ts-check
• Playwright — disabled-by-default / sticky / dismissible-with-persistence — 3 passing
• Chat regression — passes

[qodo-free-for-open-source-projects]

Review Summary by Qodo

(Agentic_describe updated until commit 906e145)

Add configurable GDPR privacy banner with dismissible/sticky modes

✨ Enhancement

Walkthroughs

Description

• Add configurable privacy banner via settings.json with title, body, and optional "Learn more"
  link
• Banner renders on pad load when enabled; content set via textContent to prevent XSS
• Support two dismissal modes: dismissible (localStorage-persisted per-origin) and sticky
  (always shown)
• Wire banner config through clientVars and render as gritter notification in bottom corner
• Comprehensive Playwright test coverage for all banner states and URL scheme validation

Diagram

flowchart LR
  Settings["settings.json<br/>privacyBanner block"]
  SettingsType["SettingsType<br/>typed config"]
  PublicSettings["getPublicSettings()<br/>expose to client"]
  ClientVars["clientVars<br/>privacyBanner"]
  PadTS["pad.ts<br/>postAceInit"]
  PrivacyBanner["privacy_banner.ts<br/>showPrivacyBannerIfEnabled"]
  Gritter["$.gritter.add()<br/>render banner"]
  LocalStorage["localStorage<br/>dismissal flag"]
  
  Settings --> SettingsType
  SettingsType --> PublicSettings
  PublicSettings --> ClientVars
  ClientVars --> PadTS
  PadTS --> PrivacyBanner
  PrivacyBanner --> Gritter
  PrivacyBanner --> LocalStorage

Loading

File Changes

1. src/node/utils/Settings.ts ✨ Enhancement +17/-1

Add typed privacyBanner block to settings

src/node/utils/Settings.ts

---

2. src/node/handler/PadMessageHandler.ts ✨ Enhancement +1/-0

Include privacyBanner in clientVars payload

src/node/handler/PadMessageHandler.ts

---

3. src/static/js/types/SocketIOMessage.ts ✨ Enhancement +7/-0

Add privacyBanner type to ClientVarPayload

src/static/js/types/SocketIOMessage.ts

---

View more (9)

4. src/static/js/privacy_banner.ts ✨ Enhancement +92/-0

New module rendering privacy banner with gritter

src/static/js/privacy_banner.ts

---

5. src/static/js/pad.ts ✨ Enhancement +2/-0

Call showPrivacyBannerIfEnabled after postAceInit

src/static/js/pad.ts

---

6. src/tests/frontend-new/specs/privacy_banner.spec.ts 🧪 Tests +199/-0

Comprehensive Playwright tests for banner behavior

src/tests/frontend-new/specs/privacy_banner.spec.ts

---

7. src/static/skins/colibris/src/components/popup.css Formatting +0/-1

Remove trailing whitespace from popup styles

src/static/skins/colibris/src/components/popup.css

---

8. settings.json.template ⚙️ Configuration changes +18/-0

Add privacyBanner configuration block with docs

settings.json.template

---

9. settings.json.docker ⚙️ Configuration changes +11/-0

Add privacyBanner environment variable support

settings.json.docker

---

10. doc/privacy.md 📝 Documentation +22/-0

Document privacyBanner configuration and usage

doc/privacy.md

---

11. docs/superpowers/specs/2026-04-19-gdpr-pr4-privacy-banner-design.md 📝 Documentation +183/-0

Design specification for privacy banner feature

docs/superpowers/specs/2026-04-19-gdpr-pr4-privacy-banner-design.md

---

12. docs/superpowers/plans/2026-04-19-gdpr-pr4-privacy-banner.md 📝 Documentation +595/-0

Implementation plan with task-by-task breakdown

docs/superpowers/plans/2026-04-19-gdpr-pr4-privacy-banner.md

---

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (5) 📘 Rule violations (1) 📎 Requirement gaps (0)

1. Unsafe learnMoreUrl href ☑ 🐞 Bug ⛨ Security

Description

showPrivacyBannerIfEnabled() assigns settings-provided learnMoreUrl directly to an anchor href, so a
javascript:/data: URL would execute script when clicked. This creates a user-triggered XSS vector
via configuration.

Code

src/static/js/privacy_banner.ts[R45-53]

+    linkEl.replaceChildren();
+    if (config.learnMoreUrl) {
+      const a = document.createElement('a');
+      a.href = config.learnMoreUrl;
+      a.target = '_blank';
+      a.rel = 'noopener';
+      a.textContent = 'Learn more';
+      linkEl.appendChild(a);
+    }

Evidence

The client code writes config.learnMoreUrl directly into a DOM anchor without validating allowed
schemes, and the value is operator-configurable via settings and shipped to the browser via
clientVars.

src/static/js/privacy_banner.ts[43-53]
src/node/utils/Settings.ts[159-181]
src/node/handler/PadMessageHandler.ts[1028-1037]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`src/static/js/privacy_banner.ts` sets `a.href` from `config.learnMoreUrl` without validating the URL scheme. This allows `javascript:` (and similar) URLs to execute code if a user clicks the “Learn more” link.
### Issue Context
`learnMoreUrl` is operator-controlled via `settings.json` and is sent to browsers via `clientVars`, so the client must defensively validate it.
### Fix Focus Areas
- src/static/js/privacy_banner.ts[43-54]
### Suggested fix
- Parse `config.learnMoreUrl` with `new URL()` (using `location.href` as base for relative URLs).
- Allowlist safe protocols (e.g., `http:`, `https:`; optionally `mailto:` depending on product policy).
- If invalid/unsafe, do not render the link (and optionally log a console warning in development).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. __etherpad_privacyBanner__ always registered 📘 Rule violation ⚙ Maintainability

Description

The privacy banner module registers globalThis.__etherpad_privacyBanner__ at import time, so a new
global API is exposed even when privacyBanner.enabled is false. This means the feature is not
fully gated behind the disabled-by-default flag, violating the requirement that disabled mode should
preserve the pre-change behavior/code path.

Code

src/static/js/privacy_banner.ts[R86-92]

+// End-to-end test hook. The privacy_banner module is bundled into pad.js so
+// the Playwright spec at src/tests/frontend-new/specs/privacy_banner.spec.ts
+// has no other way to reach into the real showPrivacyBannerIfEnabled — without
+// this it can only toy with the DOM and never proves the config-to-DOM wiring.
+// Namespaced under __etherpad_privacyBanner__ so it can't collide with site
+// code.
+(globalThis as any).__etherpad_privacyBanner__ = {show: showPrivacyBannerIfEnabled};

Evidence

PR Compliance ID 5 requires new features to be behind a flag and disabled by default without
behavior/code-path changes when disabled. The banner module is imported unconditionally from
pad.ts, and it unconditionally publishes a global hook on globalThis, which is observable
behavior even if the banner is disabled.

src/static/js/pad.ts[53-56]
src/static/js/privacy_banner.ts[86-92]
Best Practice: Repository guidelines

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`src/static/js/privacy_banner.ts` publishes `globalThis.__etherpad_privacyBanner__` unconditionally, and `pad.ts` imports the module unconditionally. This makes the new feature observable (new global API/side-effect) even when `privacyBanner.enabled` is `false`, which conflicts with the requirement that new features remain fully gated and behavior/code path stays effectively unchanged when disabled.
## Issue Context
The global hook appears intended only for Playwright E2E tests, but it currently ships in the normal browser bundle and runs on every pad load.
## Fix Focus Areas
- src/static/js/pad.ts[53-56]
- src/static/js/privacy_banner.ts[57-92]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Sticky banner still closable 🐞 Bug ≡ Correctness

Description

When privacyBanner.dismissal is set to "sticky", the banner is still rendered as a Gritter
notification with a close control, so users can dismiss it for the current session. This contradicts
the documented behavior (“sticky removes the close button”) and prevents operators from enforcing an
always-visible notice on the page.

Code

src/static/js/privacy_banner.ts[R71-83]

+  $.gritter.add({
+    title: config.title || '',
+    text: buildBody(config),
+    sticky: true,
+    position: 'bottom',
+    class_name: 'privacy-notice',
+    before_close: () => {
+      if (config.dismissal !== 'dismissible') return;
+      try {
+        localStorage.setItem(storageKey(location.href), '1');
+      } catch (_e) { /* best-effort */ }
+    },
+  });

Evidence

The banner is always created via $.gritter.add(...) and does not vary the rendered close control
based on dismissal. Etherpad’s bundled gritter template always includes a .gritter-close element
and binds a click handler that removes the notification, so sticky mode remains user-closeable
unless additional CSS/logic is added. The docs added in this PR explicitly state that sticky removes
the close button.

src/static/js/privacy_banner.ts[71-83]
doc/privacy.md[79-83]
src/static/js/vendors/gritter.ts[84-92]
src/static/js/vendors/gritter.ts[189-192]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`dismissal: "sticky"` is documented as removing the close button, but the current gritter-based banner always renders a close control and allows the user to close it for the current session.
## Issue Context
Etherpad’s bundled gritter implementation hardcodes the close element and its click handler, so achieving “no close button” requires either styling/templating changes or enhancements to gritter.
## Fix Focus Areas
- src/static/js/privacy_banner.ts[71-83]
- src/static/js/vendors/gritter.ts[84-92]
- src/static/js/vendors/gritter.ts[189-192]
- doc/privacy.md[79-83]
## Suggested fix
- In `showPrivacyBannerIfEnabled`, set a mode-specific class name, e.g. `class_name: 'privacy-notice privacy-notice-sticky'` when `dismissal === 'sticky'`.
- Add CSS that hides `.privacy-notice-sticky .gritter-close` (and/or disables pointer events) so the close control is removed in sticky mode.
- Alternatively (more robust), extend `vendors/gritter.ts` to support an option to omit rendering the close element when requested, then pass that option from `privacy_banner.ts`.
- Ensure docs match the implemented behavior (either truly remove the button, or update docs to say sticky is “non-persistent dismissal”).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

4. Referrer leaks via Learn more 🐞 Bug ⛨ Security

Description

The privacy banner’s Learn more link uses target="_blank" with rel="noopener" only, so browsers can
send the full pad URL as the Referer header to the learnMoreUrl site. This is inconsistent with
existing Etherpad link generation that explicitly uses noreferrer to avoid leaking pad URLs.

Code

src/static/js/privacy_banner.ts[R66-71]

+      const a = document.createElement('a');
+      a.href = safeHref;
+      a.target = '_blank';
+      a.rel = 'noopener';
+      a.textContent = 'Learn more';
+      linkEl.appendChild(a);

Evidence

The new banner link sets rel to only "noopener", while existing code paths that generate clickable
links intentionally include "noreferrer" to avoid leaking the pad URL/location.

src/static/js/privacy_banner.ts[61-71]
src/static/js/pad_utils.ts[231-243]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The privacy banner’s Learn more link is opened in a new tab but does not include `noreferrer` (or an explicit `referrerPolicy`), so the browser may send the pad URL as the HTTP Referer to the external site.
### Issue Context
Other Etherpad link generation intentionally uses `rel="noreferrer noopener"` to prevent leaking pad URLs.
### Fix Focus Areas
- src/static/js/privacy_banner.ts[61-71]
### Suggested change
- Change `a.rel = 'noopener'` to `a.rel = 'noreferrer noopener'` (matching existing patterns), and/or set `a.referrerPolicy = 'no-referrer'` for explicit behavior.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

View more (3)

5. Unvalidated dismissal values 🐞 Bug ◔ Observability

Description

If privacyBanner.dismissal is set to an unexpected value at runtime, the banner silently behaves as
sticky (no close button, no localStorage check), making operator misconfiguration hard to detect.
This behavior is not validated or warned about during settings load.

Code

src/static/js/privacy_banner.ts[R24-28]

+  if (config.dismissal === 'dismissible') {
+    try {
+      if (localStorage.getItem(storageKey(location.href)) === '1') return;
+    } catch (_e) { /* proceed without persistence */ }
+  }

Evidence

Settings loading merges nested objects without validating enum-like fields, and the client logic
only treats the exact string 'dismissible' as dismissible (everything else becomes sticky behavior
without any warning).

src/node/utils/Settings.ts[727-755]
src/static/js/privacy_banner.ts[24-28]
src/static/js/privacy_banner.ts[56-68]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`privacyBanner.dismissal` is treated as a string at runtime; any value other than `'dismissible'` results in sticky behavior with no warning. This makes misconfiguration silent and hard to debug.
### Issue Context
`storeSettings()` merges nested objects and does not validate nested field values. The client code’s equality check (`=== 'dismissible'`) means unexpected values degrade behavior.
### Fix Focus Areas
- src/node/utils/Settings.ts[955-960]
- src/static/js/privacy_banner.ts[19-70]
### Suggested fix
- Add a small normalization step after `storeSettings(settingsParsed)` in `reloadSettings()`:
- If `settings.privacyBanner.dismissal` is not `'dismissible'` or `'sticky'`, set it to `'dismissible'` (or your desired default) and `logger.warn()`.
- Optionally add a defensive fallback in `showPrivacyBannerIfEnabled()` as well (treat unknown values as `'dismissible'` or explicitly as `'sticky'` but warn).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

6. Privacy banner tests bypass logic 🐞 Bug ⚙ Maintainability

Description

The Playwright sticky and dismissible tests manually mutate DOM and attach onclick handlers, so they
can pass even if privacy_banner.ts is broken or never invoked. This significantly reduces regression
coverage for the actual feature implementation.

Code

src/tests/frontend-new/specs/privacy_banner.spec.ts[R22-58]

+  test('sticky banner is visible and has no close button', async ({page}) => {
+    await freshPad(page);
+    await page.evaluate(() => {
+      const banner = document.getElementById('privacy-banner')!;
+      banner.querySelector('.privacy-banner-title')!.textContent = 'Privacy';
+      const body = banner.querySelector('.privacy-banner-body')!;
+      body.textContent = '';
+      const p = document.createElement('p');
+      p.textContent = 'Body text';
+      body.appendChild(p);
+      (banner.querySelector('#privacy-banner-close') as HTMLElement).hidden = true;
+      banner.hidden = false;
+    });
+    await expect(page.locator('#privacy-banner')).toBeVisible();
+    await expect(page.locator('#privacy-banner-close')).toBeHidden();
+  });
+
+  test('dismissible — close button hides and persists in localStorage',
+      async ({page}) => {
+        await freshPad(page);
+        await page.evaluate(() => {
+          const banner = document.getElementById('privacy-banner')!;
+          banner.querySelector('.privacy-banner-title')!.textContent = 'Privacy';
+          const body = banner.querySelector('.privacy-banner-body')!;
+          body.textContent = '';
+          const p = document.createElement('p');
+          p.textContent = 'Body text';
+          body.appendChild(p);
+          const close = banner.querySelector('#privacy-banner-close') as HTMLButtonElement;
+          close.hidden = false;
+          close.onclick = () => {
+            banner.hidden = true;
+            localStorage.setItem(
+                `etherpad.privacyBanner.dismissed:${location.origin}`, '1');
+          };
+          banner.hidden = false;
+        });

Evidence

pad.ts invokes showPrivacyBannerIfEnabled(clientVars.privacyBanner) during postAceInit, but the
tests do not set clientVars.privacyBanner and do not call showPrivacyBannerIfEnabled(); instead they
directly set banner.hidden and close.onclick in page.evaluate().

src/static/js/pad.ts[493-504]
src/tests/frontend-new/specs/privacy_banner.spec.ts[22-58]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Current Playwright tests for sticky/dismissible behavior manipulate the DOM directly rather than exercising `showPrivacyBannerIfEnabled()`, so they won’t catch regressions in the actual banner implementation.
### Issue Context
`pad.ts` calls `showPrivacyBannerIfEnabled((clientVars as any).privacyBanner)` after editor init. The tests can invoke the module directly after `freshPad()` by setting `window.clientVars.privacyBanner` and calling `showPrivacyBannerIfEnabled()` again.
### Fix Focus Areas
- src/tests/frontend-new/specs/privacy_banner.spec.ts[17-67]
### Suggested fix
- In each non-default test:
- `page.evaluate((cfg) => { (window as any).clientVars.privacyBanner = cfg; /* obtain the module and call showPrivacyBannerIfEnabled(cfg) */ }, cfg)`.
- Prefer invoking the real module rather than manually editing `.textContent`, `hidden`, and `onclick`.
- Add an assertion that dismissal persists by reloading and verifying the banner stays hidden when `dismissible` and localStorage flag is set.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

7. Over-shares privacyBanner object 🐞 Bug ⛨ Security

Description

PadMessageHandler sends settings.privacyBanner by reference into clientVars, so any unexpected
nested keys under privacyBanner in settings.json will be serialized and exposed to browsers.
TypeScript Pick<> does not strip properties at runtime.

Code

src/node/handler/PadMessageHandler.ts[R1033-1037]

maxRevisions: 100,
},
enableDarkMode: settings.enableDarkMode,
+      privacyBanner: settings.privacyBanner,
automaticReconnectionTimeout: settings.automaticReconnectionTimeout,

Evidence

storeSettings() merges nested objects without removing unknown nested properties, and the server
forwards settings.privacyBanner directly into clientVars. Because serialization is runtime-based,
any extra keys present in the object will be sent.

src/node/utils/Settings.ts[727-755]
src/node/handler/PadMessageHandler.ts[1028-1037]
src/node/utils/Settings.ts[666-676]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`clientVars.privacyBanner` is populated with `settings.privacyBanner` directly. If the settings file includes additional nested keys under `privacyBanner`, they will be included in the object and sent to clients.
### Issue Context
`storeSettings()` uses `_.defaults()` for nested objects, which preserves unknown nested keys. TypeScript types do not enforce runtime shape.
### Fix Focus Areas
- src/node/handler/PadMessageHandler.ts[1028-1037]
- src/node/utils/Settings.ts[666-676]
- src/node/utils/Settings.ts[727-755]
### Suggested fix
- When building `clientVars`, construct a new object with only the allowed keys:
- `{enabled, title, body, learnMoreUrl, dismissal}`
- Optionally do the same in `getPublicSettings()` to centralize the public shape.
- (Optional) Freeze/clone to avoid accidental mutation.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[SamTV12345]

Besides the important flag nothing strikes my eyes. Great job :)

[SamTV12345]

Do we really need important here? Maybe a more specific rule can fix this?

[JohnMcLear]

Changed to use gritter, is nicer.

[JohnMcLear]

/review

[qodo-free-for-open-source-projects]

Code Review by Qodo

New Review Started

This review has been superseded by a new analysis

[qodo-code-review]

Persistent review updated to latest commit 5118870

[qodo-code-review]

ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one.

[qodo-free-for-open-source-projects]

Persistent review updated to latest commit 906e145