From ce038b0b0f4d12926650181f6a001912ae9769ff Mon Sep 17 00:00:00 2001
From: John McLear <john@mclear.co.uk>
Date: Wed, 17 Jun 2026 15:18:23 +0100
Subject: [PATCH] fix(pad): keep token-less Delete pad reachable without
 pad-wide settings (#7959) (#7960)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* fix(pad): keep token-less Delete pad reachable without pad-wide settings (#7959)

The token-less "Delete pad" button (#delete-pad) was nested inside the
enablePadWideSettings-gated pad-settings section, so disabling pad-wide
settings removed the only way to delete a pad without a recovery token.
Combined with #7926 hiding the token disclosure when deletion needs no
token (e.g. allowPadDeletionByAllUsers), a user who was allowed to delete
could be left with no deletion UI at all.

Pad deletion is unrelated to pad-wide settings, so:

- Move #delete-pad out of the enablePadWideSettings block in pad.html; it
  is now always rendered and hidden by default.
- Add a canDeletePad clientVar (isCreator || allowPadDeletionByAllUsers)
  and drive the button's visibility from it in pad_editor.ts, mirroring the
  existing canDeleteWithoutToken handling for the token disclosure.

The two controls are now mutually coherent and neither depends on
enablePadWideSettings: the plain button shows when this session can delete
without a token, the recovery-token disclosure shows otherwise.

Tests:
- backend padDeletionUiPlacement.ts: #delete-pad is rendered with
  enablePadWideSettings both on and off (fails without the template move).
- backend socketio.ts: canDeletePad reflects the creator/allow-all matrix,
  including a non-creator who only gains it under allowPadDeletionByAllUsers.
- frontend pad_settings.spec.ts: asserts #delete-pad is no longer a
  descendant of #pad-settings-section.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(pad): never let readonly sessions delete via token-less paths (#7959)

Qodo review of #7960: `canDeletePad` was `isCreator || allowPadDeletionByAllUsers`,
so under allowPadDeletionByAllUsers a readonly viewer received
canDeletePad=true and the relocated #delete-pad button unhid for them.
Worse, the server-side handlePadDelete `flagOk`/`creatorOk` branches never
checked session.readonly either, so a readonly-link holder could actually
delete the pad without a token — a data-loss hole that the new always-rendered
button would expose.

Exclude readonly sessions from both the clientVar and the server's token-less
authorization paths. A valid recovery token (tokenOk) stays a sufficient
credential regardless of session mode.

Test: socketio.ts asserts a readonly viewer gets canDeletePad=false and that a
token-less PAD_DELETE from a readonly session leaves the pad intact (red before
this change on the clientVar assertion).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 src/node/handler/PadMessageHandler.ts         | 20 ++++-
 src/static/js/pad_editor.ts                   |  7 ++
 src/templates/pad.html                        | 14 ++--
 .../backend/specs/padDeletionUiPlacement.ts   | 44 +++++++++++
 src/tests/backend/specs/socketio.ts           | 74 +++++++++++++++++++
 .../frontend-new/specs/pad_settings.spec.ts   |  5 +-
 6 files changed, 156 insertions(+), 8 deletions(-)
 create mode 100644 src/tests/backend/specs/padDeletionUiPlacement.ts

diff --git a/src/node/handler/PadMessageHandler.ts b/src/node/handler/PadMessageHandler.ts
index 4f18693cc..7ede7d391 100644
--- a/src/node/handler/PadMessageHandler.ts
+++ b/src/node/handler/PadMessageHandler.ts
@@ -305,8 +305,13 @@ const handlePadDelete = async (socket: any, padDeleteMessage: PadDeleteMessage)
   // back to the creator-cookie path, otherwise a creator pasting a wrong
   // recovery token into the disclosure field would still succeed — masking a
   // typo and contradicting the UI.
-  const creatorOk = !tokenSupplied && isCreator;
-  const flagOk = !tokenSupplied && !isCreator && settings.allowPadDeletionByAllUsers;
+  // Readonly sessions can never delete via the token-less paths: they cannot
+  // edit the pad, so they must not be able to destroy it just because
+  // allowPadDeletionByAllUsers is on (issue #7959). A valid recovery token
+  // (tokenOk) remains a sufficient credential regardless of session mode.
+  const writable = !session.readonly;
+  const creatorOk = !tokenSupplied && isCreator && writable;
+  const flagOk = !tokenSupplied && !isCreator && settings.allowPadDeletionByAllUsers && writable;
 
   if (creatorOk || tokenOk || flagOk) {
     await retrievedPad.remove();
@@ -1321,6 +1326,16 @@ const handleClientReady = async (socket:any, message: ClientReadyMessage) => {
     const hasGetAuthorIdHook = (plugins.hooks.getAuthorId || []).length > 0;
     const hasDurableIdentity = hasGetAuthorIdHook && !!(user && user.username);
     const canDeleteWithoutToken = settings.allowPadDeletionByAllUsers || hasDurableIdentity;
+    // Whether this session may delete the pad with no token at all: the creator
+    // on this device (creator-cookie still present), or any user when the
+    // instance opted everyone in. Drives the plain "Delete pad" button, which is
+    // independent of enablePadWideSettings (issue #7959) — deletion is not a
+    // pad-wide setting and must stay reachable when that section is disabled.
+    // Readonly viewers are excluded: they cannot edit, let alone delete, so
+    // allowPadDeletionByAllUsers must not hand them a delete button (the server
+    // enforces the same in handlePadDelete).
+    const canDeletePad =
+        !sessionInfo.readonly && (isCreator || settings.allowPadDeletionByAllUsers);
     const padDeletionToken =
         isCreator && !canDeleteWithoutToken
         ? await padDeletionManager.createDeletionTokenIfAbsent(sessionInfo.padId)
@@ -1346,6 +1361,7 @@ const handleClientReady = async (socket:any, message: ClientReadyMessage) => {
       // redundant, so the client labels the action "Delete Pad" instead of
       // "Delete with token" (issue #7926). See showDeletionTokenModalIfPresent.
       canDeleteWithoutToken,
+      canDeletePad,
       // Allow-listed copy — settings.privacyBanner could carry extra nested
       // keys from a hand-edited settings.json; sending those by reference
       // would leak them to every browser. See getPublicPrivacyBanner().
diff --git a/src/static/js/pad_editor.ts b/src/static/js/pad_editor.ts
index 0382a813a..bd721e62c 100644
--- a/src/static/js/pad_editor.ts
+++ b/src/static/js/pad_editor.ts
@@ -159,6 +159,13 @@ const padeditor = (() => {
       $('#delete-pad-with-token').prop(
           'hidden', !!(window as any).clientVars?.canDeleteWithoutToken);
 
+      // The plain "Delete pad" button is shown whenever this session can delete
+      // without a token (creator on this device, or allowPadDeletionByAllUsers).
+      // It is independent of pad-wide settings so it stays reachable when that
+      // section is disabled (issue #7959).
+      $('#delete-pad').prop(
+          'hidden', !(window as any).clientVars?.canDeletePad);
+
       // delete pad using a recovery token (second device / no creator cookie)
       $('#delete-pad-token-submit').on('click', () => {
         const token = String($('#delete-pad-token-input').val() || '').trim();
diff --git a/src/templates/pad.html b/src/templates/pad.html
index 27f21948c..1a1efe19b 100644
--- a/src/templates/pad.html
+++ b/src/templates/pad.html
@@ -391,13 +391,17 @@
                 </p>
                 <% e.end_block(); %>
               </div>
-              <button class="btn btn-danger" data-l10n-id="pad.settings.deletePad" id="delete-pad">Delete pad</button>
             </div><% } %>
           </div>
-          <!-- Visibility is controlled at runtime in pad_editor.ts: hidden when
-               clientVars.canDeleteWithoutToken (issue #7926). Rendered for every
-               session (incl. requireAuthentication) because a recovery token may
-               be issued whenever the creator lacks a durable cross-device identity. -->
+          <!-- Pad deletion is independent of pad-wide settings (issue #7959):
+               both controls are always rendered and pad_editor.ts shows exactly
+               the one that applies. #delete-pad (token-less) is shown when
+               clientVars.canDeletePad — the creator on this device, or any user
+               when allowPadDeletionByAllUsers is on. The recovery-token
+               disclosure is shown when clientVars.canDeleteWithoutToken is false
+               (issue #7926): a creator on a second device, or under
+               requireAuthentication without a durable cross-device identity. -->
+          <button class="btn btn-danger" data-l10n-id="pad.settings.deletePad" id="delete-pad" hidden>Delete pad</button>
           <details id="delete-pad-with-token" hidden>
             <summary data-l10n-id="pad.deletionToken.deleteWithToken">Delete with token</summary>
             <label for="delete-pad-token-input" data-l10n-id="pad.deletionToken.tokenFieldLabel">Pad deletion token</label>
diff --git a/src/tests/backend/specs/padDeletionUiPlacement.ts b/src/tests/backend/specs/padDeletionUiPlacement.ts
new file mode 100644
index 000000000..e23596b80
--- /dev/null
+++ b/src/tests/backend/specs/padDeletionUiPlacement.ts
@@ -0,0 +1,44 @@
+'use strict';
+
+import {MapArrayType} from '../../../node/types/MapType';
+import settings from '../../../node/utils/Settings';
+
+const assert = require('assert').strict;
+const common = require('../common');
+
+// Regression coverage for issue #7959. The token-less "Delete pad" button
+// (#delete-pad) used to be nested inside the `enablePadWideSettings`-gated
+// pad-settings section, so disabling pad-wide settings removed the only way to
+// delete a pad without a recovery token. Pad deletion is unrelated to pad-wide
+// settings, so the button must be rendered regardless of that flag (its
+// visibility is then driven at runtime by clientVars.canDeletePad).
+describe(__filename, function () {
+  this.timeout(30000);
+  let agent: any;
+  const backup: MapArrayType<any> = {};
+
+  before(async function () { agent = await common.init(); });
+
+  beforeEach(async function () {
+    backup.enablePadWideSettings = settings.enablePadWideSettings;
+  });
+
+  afterEach(async function () {
+    settings.enablePadWideSettings = backup.enablePadWideSettings;
+  });
+
+  const hasDeletePadButton = (html: string): boolean =>
+    /id="delete-pad"/.test(html);
+
+  it('renders the Delete pad button with pad-wide settings enabled', async function () {
+    settings.enablePadWideSettings = true;
+    const res = await agent.get('/p/deleteUiPlacementOn').expect(200);
+    assert.equal(hasDeletePadButton(res.text), true);
+  });
+
+  it('renders the Delete pad button with pad-wide settings disabled (#7959)', async function () {
+    settings.enablePadWideSettings = false;
+    const res = await agent.get('/p/deleteUiPlacementOff').expect(200);
+    assert.equal(hasDeletePadButton(res.text), true);
+  });
+});
diff --git a/src/tests/backend/specs/socketio.ts b/src/tests/backend/specs/socketio.ts
index 441f8110e..209b7610f 100644
--- a/src/tests/backend/specs/socketio.ts
+++ b/src/tests/backend/specs/socketio.ts
@@ -540,6 +540,9 @@ describe(__filename, function () {
           'creator should get a token so the client can show the save-token modal');
       assert.ok(cv.data.padDeletionToken.length >= 32);
       assert.equal(cv.data.canDeleteWithoutToken, false);
+      // The creator can always delete without a token on this device, so the
+      // plain "Delete pad" button is offered (issue #7959).
+      assert.equal(cv.data.canDeletePad, true);
     });
 
     it('no token (and so no modal) when allowPadDeletionByAllUsers is true', async function () {
@@ -554,8 +557,77 @@ describe(__filename, function () {
       // can already delete the pad without a token in this configuration.
       assert.equal(cv.data.padDeletionToken, null);
       assert.equal(cv.data.canDeleteWithoutToken, true);
+      assert.equal(cv.data.canDeletePad, true);
     });
 
+    it('non-creator gets canDeletePad=false by default, true under allowPadDeletionByAllUsers (#7959)',
+        async function () {
+          const supertest = require('supertest');
+          // The creator (default cookie jar) establishes the pad's rev-0 author.
+          const resCreator = await agent.get('/p/pad').expect(200);
+          socket = await common.connect(resCreator);
+          const cvCreator: any = await common.handshake(socket, 'pad');
+          assert.equal(cvCreator.data.canDeletePad, true, 'creator can always delete');
+
+          // A different browser (separate cookie jar) is NOT the creator, so with
+          // allowPadDeletionByAllUsers off it must not be offered the token-less
+          // Delete pad button.
+          const otherBrowser = supertest(common.baseUrl);
+          const resOther = await otherBrowser.get('/p/pad').expect(200);
+          const otherSocket = await common.connect(resOther);
+          try {
+            const cvOther: any = await common.handshake(otherSocket, 'pad');
+            assert.equal(cvOther.data.canDeletePad, false,
+                'non-creator must not see Delete pad by default');
+          } finally {
+            otherSocket.close();
+          }
+
+          // With everyone opted in, the same non-creator CAN delete, so the
+          // button must be offered — independent of enablePadWideSettings (#7959).
+          // @ts-ignore - public setting toggled per test
+          settings.allowPadDeletionByAllUsers = true;
+          const otherBrowser2 = supertest(common.baseUrl);
+          const resOther2 = await otherBrowser2.get('/p/pad').expect(200);
+          const otherSocket2 = await common.connect(resOther2);
+          try {
+            const cvOther2: any = await common.handshake(otherSocket2, 'pad');
+            assert.equal(cvOther2.data.canDeletePad, true,
+                'allowPadDeletionByAllUsers must offer Delete pad to everyone');
+          } finally {
+            otherSocket2.close();
+          }
+        });
+
+    it('readonly viewer is denied canDeletePad and token-less deletion under allowPadDeletionByAllUsers (#7959)',
+        async function () {
+          // @ts-ignore - public setting toggled per test
+          settings.allowPadDeletionByAllUsers = true;
+          // Creator establishes the pad (rev-0 author) and yields its read-only id.
+          const resCreator = await agent.get('/p/pad').expect(200);
+          const creatorSocket = await common.connect(resCreator);
+          const cvCreator: any = await common.handshake(creatorSocket, 'pad');
+          const readOnlyId = cvCreator.data.readOnlyId;
+          assert.ok(readOnlyManager.isReadOnlyId(readOnlyId));
+          creatorSocket.close();
+
+          // A read-only viewer must NOT be offered the token-less delete button,
+          // even with deletion opened to all users — readonly viewers cannot edit,
+          // let alone delete (issue #7959).
+          const resRo = await agent.get(`/p/${readOnlyId}`).expect(200);
+          socket = await common.connect(resRo);
+          const cvRo: any = await common.handshake(socket, readOnlyId);
+          assert.equal(cvRo.data.readonly, true);
+          assert.equal(cvRo.data.canDeletePad, false,
+              'readonly viewers must not get the token-less Delete pad button');
+
+          // ...and the server must refuse a token-less PAD_DELETE from a readonly
+          // session, or allowPadDeletionByAllUsers becomes a data-loss hole.
+          await common.sendPadDelete(socket, {padId: 'pad'}).catch(() => {});
+          assert.ok(await padManager.doesPadExist('pad'),
+              'readonly session must not be able to delete the pad without a token');
+        });
+
     it('authenticated creator WITHOUT a getAuthorId hook still gets a token', async function () {
       // requireAuthentication alone is NOT durable: the authorID still comes from
       // the per-browser token cookie, so this user would be stranded on a second
@@ -567,6 +639,7 @@ describe(__filename, function () {
       assert.equal(cv.type, 'CLIENT_VARS');
       assert.equal(typeof cv.data.padDeletionToken, 'string');
       assert.equal(cv.data.canDeleteWithoutToken, false);
+      assert.equal(cv.data.canDeletePad, true);
     });
 
     it('authenticated creator WITH a getAuthorId hook gets no token (durable identity)',
@@ -579,6 +652,7 @@ describe(__filename, function () {
           assert.equal(cv.type, 'CLIENT_VARS');
           assert.equal(cv.data.padDeletionToken, null);
           assert.equal(cv.data.canDeleteWithoutToken, true);
+          assert.equal(cv.data.canDeletePad, true);
         });
   });
 
diff --git a/src/tests/frontend-new/specs/pad_settings.spec.ts b/src/tests/frontend-new/specs/pad_settings.spec.ts
index 1fbd74f86..070491cef 100644
--- a/src/tests/frontend-new/specs/pad_settings.spec.ts
+++ b/src/tests/frontend-new/specs/pad_settings.spec.ts
@@ -3,7 +3,7 @@ import {goToNewPad, goToPad, sendChatMessage, showChat} from "../helper/padHelpe
 import {showSettings} from "../helper/settingsHelper";
 
 test.describe('creator-owned pad settings', () => {
-  test('shows pad settings only to the creator and keeps delete pad there', async ({page, browser}) => {
+  test('shows pad settings only to the creator; delete pad is creator-gated but separate', async ({page, browser}) => {
     const padId = await goToNewPad(page);
 
     const context2 = await browser.newContext();
@@ -19,6 +19,9 @@ test.describe('creator-owned pad settings', () => {
     await expect(page.locator('#pad-settings-section')).toBeVisible();
     await expect(page.locator('#delete-pad')).toBeVisible();
     await expect(page.locator('#padsettings-enforcecheck')).toBeVisible();
+    // The delete-pad button is no longer nested inside the pad-wide settings
+    // section: deletion is independent of enablePadWideSettings (issue #7959).
+    await expect(page.locator('#pad-settings-section #delete-pad')).toHaveCount(0);
 
     await expect(page2.locator('#user-settings-section > h2')).toHaveText('User Settings');
     await expect(page2.locator('#theme-toggle-row')).toBeVisible();