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

[JohnMcLear]

Closes #7959. Follow-up to #7926 (#7929 + #7930).

The bug

The token-less Delete pad button (#delete-pad) lived inside the
enablePadWideSettings-gated pad-settings section of pad.html, while the
recovery-token disclosure (#delete-pad-with-token) is rendered
unconditionally and hidden by JS when clientVars.canDeleteWithoutToken.

So pad deletion was accidentally coupled to the pad-wide settings feature
flag — a separate concern. With enablePadWideSettings: false:

• the pad-settings section (and #delete-pad) isn't rendered, and
• if allowPadDeletionByAllUsers: true, the token disclosure is correctly
  hidden (no token needed),

…leaving a user who is allowed to delete with no deletion UI at all —
exactly what @n-bux reported.

(The reporter's first observation — the token button still showing under
allowPadDeletionByAllUsers — was already addressed by #7930 on develop;
they were on 3.3.1, which shipped #7929 but not #7930. This PR fixes the
remaining, still-open half.)

The fix

Pad deletion is independent of pad-wide settings:

• Move #delete-pad out of the enablePadWideSettings block; 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 (e.g. a creator
on a second device).

Tests

• backend padDeletionUiPlacement.ts (new): renders the real pad.html
  via the server and asserts #delete-pad is present 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.

🤖 Generated with Claude Code

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

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

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0)

1. Readonly pad can delete ✓ Resolved 🐞 Bug ⛨ Security

Description

If allowPadDeletionByAllUsers is enabled, readonly sessions will receive
clientVars.canDeletePad=true and the UI will unhide #delete-pad even on readonly pads. The backend
PAD_DELETE handler also allows token-less deletion under allowPadDeletionByAllUsers without checking
session.readonly, so anyone with a readonly link can delete pads (data loss).

Code

src/node/handler/PadMessageHandler.ts[R1323-1329]

   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.
+    const canDeletePad = isCreator || settings.allowPadDeletionByAllUsers;

Evidence

The PR now always renders #delete-pad and unhides it based on canDeletePad; because canDeletePad
becomes true when allowPadDeletionByAllUsers is true (even in readonly sessions), readonly users can
attempt deletion. The backend accepts PAD_DELETE without a token via the allowPadDeletionByAllUsers
flag and does not check readonly, enabling actual deletion from readonly sessions. Repo design docs
explicitly scope allowPadDeletionByAllUsers to users who can edit the pad.

src/node/handler/PadMessageHandler.ts[1295-1333]
src/static/js/pad_editor.ts[152-168]
src/templates/pad.html[396-411]
src/node/handler/PadMessageHandler.ts[290-334]
docs/superpowers/specs/2026-04-18-gdpr-pr1-deletion-controls-design.md[138-145]

Agent prompt

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

## Issue description
With `allowPadDeletionByAllUsers: true`, readonly sessions can see and use the token-less delete button because `canDeletePad` ignores `sessionInfo.readonly`, and the server-side `handlePadDelete()` `flagOk` branch also ignores `session.readonly`.
### Issue Context
`allowPadDeletionByAllUsers` is documented as allowing any user who can *edit* a pad to delete it; readonly viewers should not gain deletion rights.
### Fix Focus Areas
- src/node/handler/PadMessageHandler.ts[1323-1333]
- src/node/handler/PadMessageHandler.ts[290-314]
- src/static/js/pad_editor.ts[152-168]
### Suggested fix
1. In `handleClientReady`, compute `canDeletePad` so it is **false for readonly sessions**, for example:
 - `const canDeletePad = !sessionInfo.readonly && (isCreator || settings.allowPadDeletionByAllUsers);`
 (or equivalent).
2. In `handlePadDelete`, prevent token-less deletion from readonly sessions by adding `!session.readonly` to the non-token authorization paths (at minimum the `flagOk` branch; consider also `creatorOk` for consistency with the UI).
3. Optionally add/adjust a test to cover that a readonly session does not receive `canDeletePad=true` under `allowPadDeletionByAllUsers`, and that PAD_DELETE is rejected from readonly when no token is supplied.

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

---

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

PR Summary by Qodo

Fix Delete pad UI when pad-wide settings are disabled
🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

Walkthroughs

Description

• Decouple token-less Delete pad UI from enablePadWideSettings feature flag.
• Add canDeletePad clientVar and toggle deletion controls consistently in pad_editor.
• Add backend and frontend regressions covering deletion UI placement and permissions.

Diagram

sequenceDiagram
  participant U as "Browser UI"
  participant H as "PadMessageHandler"
  participant T as "pad.html"
  participant E as "pad_editor.ts"
  participant S as "Settings"

  U->>H: GET /p/:pad
  H->>T: Render HTML (always includes delete controls)
  T-->>U: HTML with #delete-pad + #delete-pad-with-token (hidden)

  U->>H: Socket CLIENT_READY
  H->>S: Read allowPadDeletionByAllUsers
  S-->>H: Flag value
  H-->>U: CLIENT_VARS (canDeletePad, canDeleteWithoutToken, token?)

  U->>E: Initialize editor
  alt canDeletePad
    E-->>U: Show #delete-pad
  else
    E-->>U: Show token disclosure
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Server-render delete UI based on permissions at GET time

• ➕ Avoids client-side hide/show and any flash of hidden content
• ➕ Keeps permission-driven UI decisions in one place (server)
• ➖ Requires computing creator/identity state during initial HTTP render (may duplicate socket-ready logic)
• ➖ More coupling between HTTP route rendering and socket permission computation

2. Keep delete UI inside pad settings but split the pad-wide feature flag gate

• ➕ Minimal template movement if the settings section is already the intended place
• ➖ Still risks future coupling between unrelated feature flags and deletion UI
• ➖ Harder to reason about which parts of the settings UI are gated by what

Recommendation: Current approach is appropriate: always render both deletion controls and drive visibility from explicit clientVars (canDeletePad / canDeleteWithoutToken). It avoids re-coupling deletion to UI feature flags and aligns with the existing token-disclosure behavior, while keeping permission logic centralized in the server’s CLIENT_VARS computation.

File Changes

Bug fix (3)

PadMessageHandler.ts Add canDeletePad clientVar independent of pad-wide settings +7/-0

Add canDeletePad clientVar independent of pad-wide settings

• Introduces a new canDeletePad flag (isCreator || allowPadDeletionByAllUsers) and includes it in CLIENT_VARS. This decouples the token-less delete button’s eligibility from enablePadWideSettings.

src/node/handler/PadMessageHandler.ts

---

pad_editor.ts Toggle token-less Delete pad button via clientVars.canDeletePad +7/-0

Toggle token-less Delete pad button via clientVars.canDeletePad

• Adds runtime visibility control for #delete-pad based on clientVars.canDeletePad. Keeps the recovery-token disclosure behavior unchanged and ensures the correct control is shown.

src/static/js/pad_editor.ts

---

pad.html Move #delete-pad outside pad-wide settings block and render hidden by default +9/-5

Move #delete-pad outside pad-wide settings block and render hidden by default

• Relocates the token-less delete button so it is always present in the DOM, regardless of enablePadWideSettings. Updates inline documentation to describe the two-control visibility contract.

src/templates/pad.html

---

Tests (3)

padDeletionUiPlacement.ts Add backend regression test for Delete pad DOM placement +44/-0

Add backend regression test for Delete pad DOM placement

• New test renders real pad HTML and asserts #delete-pad exists with enablePadWideSettings both enabled and disabled. Prevents reintroducing the UI coupling from issue #7959.

src/tests/backend/specs/padDeletionUiPlacement.ts

---

socketio.ts Test canDeletePad clientVar across creator and allow-all scenarios +45/-0

Test canDeletePad clientVar across creator and allow-all scenarios

• Extends existing socketio specs to assert canDeletePad is set for creators, for allowPadDeletionByAllUsers, and not set for non-creators by default. Covers the permission matrix that drives the UI.

src/tests/backend/specs/socketio.ts

---

pad_settings.spec.ts Assert delete button is not nested under pad-wide settings section +4/-1

Assert delete button is not nested under pad-wide settings section

• Updates the creator-owned pad settings Playwright test to verify #delete-pad is visible for the creator but not a descendant of #pad-settings-section. Ensures deletion remains independent from enablePadWideSettings.

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

---