feat(gdpr): pad deletion controls (PR1 of #6701)#7546
feat(gdpr): pad deletion controls (PR1 of #6701)#7546JohnMcLear wants to merge 16 commits intodevelopfrom
Conversation
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>
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>
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>
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>
Creator cookie → valid deletion token → allowPadDeletionByAllUsers flag. Anyone else still gets the existing refusal shout.
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.
Review Summary by Qodofeat(gdpr): pad deletion controls with one-time tokens and recovery flow
WalkthroughsDescription• One-time sha256-hashed deletion token, returned plaintext once on pad creation • Three-way authorization: creator cookie, valid token, or allowPadDeletionByAllUsers flag • Browser creators see token modal and can delete via recovery-token field in settings • Comprehensive backend and frontend test coverage with Playwright and Mocha specs Diagramflowchart LR
CreatePad["createPad/createGroupPad"] -->|generates token| PDM["PadDeletionManager"]
PDM -->|stores hash| DB["Database"]
PDM -->|returns plaintext| API["API Response"]
API -->|browser only| ClientVars["clientVars.padDeletionToken"]
ClientVars -->|creator session| Modal["Token Modal"]
Modal -->|acknowledged| Settings["Settings Popup"]
Settings -->|delete-with-token| PAD_DELETE["PAD_DELETE Message"]
PAD_DELETE -->|three-way auth| Handler["handlePadDelete"]
Handler -->|creator OR token OR flag| Remove["Pad.remove"]
Remove -->|cleanup| PDM
File Changes1. src/node/db/PadDeletionManager.ts
|
Code Review by Qodo
1. Deletion token not feature-flagged
|
| // Only the original creator of the pad (revision 0 author) receives the | ||
| // deletion token, and only on their first arrival — subsequent visits get | ||
| // null because createDeletionTokenIfAbsent() only emits a plaintext token | ||
| // once. Readonly sessions never see it. | ||
| const isCreator = | ||
| !sessionInfo.readonly && sessionInfo.author === await pad.getRevisionAuthor(0); | ||
| const padDeletionToken = isCreator | ||
| ? await padDeletionManager.createDeletionTokenIfAbsent(sessionInfo.padId) | ||
| : null; |
There was a problem hiding this comment.
1. Deletion token not feature-flagged 📘 Rule violation ☼ Reliability
Pad deletion tokens (API response + creator modal) are enabled unconditionally, changing default behavior rather than being gated behind a disabled-by-default feature flag. This violates the requirement that new features be behind an off-by-default flag to preserve pre-change behavior unless explicitly enabled.
Agent Prompt
## Issue description
Deletion tokens are generated and surfaced by default (API response and UI modal) without a feature flag. Compliance requires new features to be behind a disabled-by-default flag, with the default path matching pre-change behavior.
## Issue Context
Current behavior:
- `createPad()` always returns `{deletionToken: ...}`
- Creator sessions always attempt token creation and receive `clientVars.padDeletionToken`
## Fix Focus Areas
- src/node/db/API.ts[520-546]
- src/node/handler/PadMessageHandler.ts[1004-1012]
- src/node/utils/Settings.ts[172-176]
- settings.json.template[480-486]
- settings.json.docker[483-489]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
|
||
| // create pad | ||
| await getPadSafe(padID, false, text, authorId); | ||
| return {deletionToken: await padDeletionManager.createDeletionTokenIfAbsent(padID)}; | ||
| }; | ||
|
|
||
| /** | ||
| deletePad(padID) deletes a pad | ||
| deletePad(padID, [deletionToken]) deletes a pad | ||
|
|
||
| Example returns: | ||
|
|
||
| {code: 0, message:"ok", data: null} | ||
| {code: 1, message:"padID does not exist", data: null} | ||
| {code: 1, message:"invalid deletionToken", data: null} | ||
| @param {String} padID the id of the pad | ||
| @param {String} [deletionToken] recovery token issued by createPad | ||
| */ | ||
| exports.deletePad = async (padID: string) => { | ||
| exports.deletePad = async (padID: string, deletionToken?: string) => { | ||
| const pad = await getPadSafe(padID, true); | ||
| // apikey-authenticated callers (no deletionToken supplied) are trusted. | ||
| // When a caller supplies a deletionToken, it must validate unless the | ||
| // instance has opted everyone in via allowPadDeletionByAllUsers. | ||
| if (deletionToken !== undefined && deletionToken !== '' && | ||
| !settings.allowPadDeletionByAllUsers && | ||
| !await padDeletionManager.isValidDeletionToken(padID, deletionToken)) { | ||
| throw new CustomError('invalid deletionToken', 'apierror'); | ||
| } |
There was a problem hiding this comment.
2. Http api docs not updated 📘 Rule violation ⚙ Maintainability
The PR changes the public HTTP API (createPad now returns deletionToken; deletePad accepts optional deletionToken) but the HTTP API documentation still describes the old signatures/returns. This can cause integrator confusion and violates the requirement to update docs in the same PR when changing APIs.
Agent Prompt
## Issue description
The HTTP API docs are out of date relative to the changed endpoints:
- `createPad` now returns `data: {deletionToken: string|null}`
- `deletePad` now accepts optional `deletionToken` and may return `invalid deletionToken`
## Issue Context
The OpenAPI schema arg list was updated, but the human-readable HTTP API documentation (`doc/api/http_api.md`) still shows the previous signatures and example returns.
## Fix Focus Areas
- src/node/db/API.ts[520-548]
- src/node/handler/APIHandler.ts[53-56]
- doc/api/http_api.md[519-592]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| exports.createDeletionTokenIfAbsent = async (padId: string): Promise<string | null> => { | ||
| if (await DB.db.get(getDeletionTokenKey(padId)) != null) return null; | ||
| const deletionToken = randomString(32); | ||
| await DB.db.set(getDeletionTokenKey(padId), { | ||
| createdAt: Date.now(), | ||
| hash: hashDeletionToken(deletionToken).toString('hex'), | ||
| }); | ||
| return deletionToken; |
There was a problem hiding this comment.
3. Token creation race 🐞 Bug ≡ Correctness
createDeletionTokenIfAbsent() uses a non-atomic read-then-write, so concurrent calls for the same pad can each return different plaintext tokens and overwrite the stored hash, making at least one returned token permanently invalid.
Agent Prompt
### Issue description
`createDeletionTokenIfAbsent()` does a check-then-set on the DB key. Under concurrency, two calls for the same `padId` can both return different plaintext tokens, but only the last stored hash will validate, causing at least one client to save an unusable recovery token.
### Issue Context
This function is called from multiple entry points (creator `CLIENT_READY`, `createPad`, `createGroupPad`). Even without multi-process, two creator tabs can race.
### Fix Focus Areas
- src/node/db/PadDeletionManager.ts[13-21]
- src/node/handler/PadMessageHandler.ts[1008-1012]
### Suggested fix
Implement a per-`padId` in-memory mutex/in-flight promise map around the token creation path:
- Re-check existence after acquiring the lock.
- Ensure only one call performs the `set` and returns the plaintext token; concurrent callers should reliably return `null`.
If Etherpad can run multi-process in your deployment, document this limitation or consider a DB-level atomic primitive if available.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
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>
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>
Summary
allowPadDeletionByAllUsersflag (defaults tofalse) to widen deletion rightsPAD_DELETEand RESTdeletePad: creator cookie, valid token, or settings flagFirst of the five GDPR PRs outlined in #6701. Remaining scope (IP audit, identity hardening, cookie banner, author erasure) stays in follow-ups.
Design spec:
docs/superpowers/specs/2026-04-18-gdpr-pr1-deletion-controls-design.mdImplementation plan:
docs/superpowers/plans/2026-04-18-gdpr-pr1-deletion-controls.mdTest plan
pnpm --filter ep_etherpad-lite run ts-checkmocha tests/backend/specs/padDeletionManager.ts tests/backend/specs/api/deletePad.ts— 14 tests passnpx playwright test pad_deletion_token --project=chromium— 3 tests pass