Commit graph

4 commits

Author SHA1 Message Date
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>
2026-05-03 12:30:49 +01:00
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 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>
2026-05-03 13:59:38 +08:00
John McLear
b47fcd819b
fix(docs): replace dead privacy.md link with GitHub URL (#7641)
PR #7546 added a relative link in `doc/privacy.md` pointing to
`../docs/superpowers/specs/2026-04-18-gdpr-pr1-deletion-controls-design.md`,
which lives outside vitepress's `doc/` source root. VitePress reports
it as a dead link and the docs deploy on develop fails:

    (!) Found dead link
        ./../docs/superpowers/specs/2026-04-18-gdpr-pr1-deletion-controls-design
        in file /home/runner/work/etherpad/etherpad/doc/privacy.md
    Error: 1 dead link(s) found.

Point the link at the file on GitHub instead so the published site
resolves it and readers can still find the spec.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-01 13:58:47 +01:00
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>
2026-05-01 13:47:40 +01:00