Skip to content

Commit ce2bfde

Browse files
HMarzbanclaude
HMarzban
and
claude
committed
fix(hocuspocus): re-check ghost accounts before delete and clear soft-deleted profiles
The ghost-account delete no longer trusts the cached list. Each id is checked again before the delete. A user who signed in, an admin, or an unknown id is refused. The single route answers 409, and the bulk route counts the id as failed. A non-uuid id answers the house 400. A failed impact read or admin read now deletes nothing. Before, a failed impact read could fall through to a hard delete. Revoking admin access is now one locked call to public.admin_revoke_admin. Two admins who revoke each other at the same moment cannot leave zero admins. The migration pairs with its script, and seed.sql and the webapp types are regenerated. Push the migration to production by hand before this server ships. A soft delete now clears the public profile. It clears the name, photo, bio and links, sets a deleted_ username, removes the user's avatar objects, and bans the user. Messages keep their author. Refs #412 Refs #427 Co-Authored-By: Claude Opus 5.5 <[email protected]>
1 parent 043dda2 commit ce2bfde

13 files changed

Lines changed: 269 additions & 74 deletions

File tree

‎apps/hocuspocus.server/API.md‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1294,15 +1294,16 @@ The `dlq` route returns `push` and `email`, each with `jobs` and `count`. Each j
12941294

12951295
**Ghost accounts audit**
12961296

1297-
| Method | Path | Purpose |
1298-
| ------ | ------------------------------------------- | ------------------------- |
1299-
| GET | `/audit/ghost-accounts` | List ghost accounts |
1300-
| GET | `/audit/ghost-accounts/summary` | Category summary |
1301-
| GET | `/audit/ghost-accounts/:id/impact` | FK dependency check |
1302-
| DELETE | `/audit/ghost-accounts/:id` | Smart-delete one |
1303-
| POST | `/audit/ghost-accounts/bulk-delete` | Bulk delete (max 50) |
1304-
| POST | `/audit/ghost-accounts/resend-confirmation` | Resend magic link |
1305-
| POST | `/audit/ghost-accounts/cleanup-anonymous` | Clean stale anon sessions |
1297+
| Method | Path | Purpose |
1298+
| ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ------------------------------------------- | ------------------------- |
1299+
| GET | `/audit/ghost-accounts` | List ghost accounts |
1300+
| GET | `/audit/ghost-accounts/summary` | Category summary |
1301+
| GET | `/audit/ghost-accounts/:id/impact` | FK dependency check |
1302+
| DELETE | `/audit/ghost-accounts/:id` | Smart-delete one |
1303+
| POST | `/audit/ghost-accounts/bulk-delete` | Bulk delete (max 50) |
1304+
| POST | `/audit/ghost-accounts/resend-confirmation` | Resend magic link |
1305+
| POST | `/audit/ghost-accounts/cleanup-anonymous` | Clean stale anon sessions |
1306+
| Both ghost-account delete routes check each id again before they delete. They refuse a user who signed in after the list loaded, an admin, and an unknown id. `DELETE /audit/ghost-accounts/:id` then answers `409` with `{ error }`. A non-uuid id answers `400` `VALIDATION_ERROR`. The bulk route counts a refused id under `failed` and adds a line to `errors`. A failed impact read or admin read also stops the delete. The single route answers `500`, and the bulk route counts the id as failed. A soft delete bans the user and clears the public profile: name, photo, bio and links. It also removes the `user_avatars` objects and sets the username to `deleted_` plus 12 hex characters of the id. |
13061307

13071308
## Push notifications
13081309

‎apps/hocuspocus.server/CHANGELOG.md‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,11 @@ This file is the operator and API changelog. The pad product lives in the [root
2424

2525
### Migration
2626

27+
**Admin revoke ([#412](https://github.com/docs-plus/docs.plus/issues/412)).** Apply the
28+
Supabase migration `20261009120400_admin_revoke_admin` before this server
29+
ships. Without it, every admin revoke answers `500`. Then check that only
30+
`service_role` can run `public.admin_revoke_admin(uuid)`.
31+
2732
**Media objects ([#408](https://github.com/docs-plus/docs.plus/issues/408)).** New S3 uploads are
2833
private. Objects written before this release stay `public-read` until you
2934
change them. Deploy the server first, then set the existing objects private.
@@ -611,6 +616,21 @@ SMTP_PASS=...
611616
A token already past `exp` is refused like an invalid one. Each signed-in tab
612617
reconnects about once per token lifetime, which is 1 hour by default
613618
([#430](https://github.com/docs-plus/docs.plus/issues/430)).
619+
- **The ghost-account delete checks each account again before it deletes
620+
([#412](https://github.com/docs-plus/docs.plus/issues/412)).**
621+
`DELETE /api/admin/audit/ghost-accounts/:id` answers `409` when the user
622+
signed in after the list loaded, is an admin, or does not exist. A non-uuid
623+
id answers the house `400`. The bulk delete counts a refused id under `failed`.
624+
A failed impact read or admin read now deletes nothing. Before, a failed
625+
impact read could fall through to a hard delete. Revoking admin access is one
626+
locked call to `public.admin_revoke_admin`, so two revokes at the same moment
627+
cannot leave zero admins. See Migration.
628+
- **A soft delete clears the public profile
629+
([#427](https://github.com/docs-plus/docs.plus/issues/427)).** When a
630+
ghost-account delete keeps the row, it now clears the name, photo, bio and
631+
links. It sets the username to `deleted_` plus the first 12 hex characters
632+
of the id, and removes the user's `user_avatars` objects. Messages keep their
633+
author, who shows the placeholder name and the default avatar.
614634

615635
### Removed
616636

‎apps/hocuspocus.server/CLAUDE.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,8 @@ Moved verbatim out of the repo-root [AGENTS.md](../../AGENTS.md). The `### Supab
223223
### Admin API And Dashboard
224224

225225
- **Any admin-only data path goes through an `is_admin()`-gated `SECURITY DEFINER` RPC or the `service_role` hocuspocus REST API — never a direct anon-key table read.** The browser client is RLS-scoped `authenticated`. Several admin tables/columns are revoked from that role (`users.email`, `push_subscriptions`, `email_queue`) or scoped to the caller's own rows. Direct reads therefore return wrong, zeroed, or `42501` results. Aggregations must paginate past the 1000-row cap.
226+
- **The ghost-account delete trusts no cached list (#412).** `assertDeletableGhost` (`src/api/services/adminGhostAccounts.service.ts`) is the one gate, and the single and bulk paths call it for each id. It re-reads the auth user and refuses a user who is no longer a ghost, an admin, or an unknown id. A hard delete cascades to `admin_users`, so it would bypass the last-admin guard. A failed impact read or admin read deletes nothing. Both paths lowercase the id first, because Prisma compares `ownerId` as text. Revoking admin goes only through `public.admin_revoke_admin`. Never count and delete in two calls, because two admins could then remove each other.
227+
- **Every user soft delete goes through `softDeleteUsers` (#427).** The `users` read policies are `USING (true)`, so `deleted_at` alone leaves the name, photo, bio and links public. The helper clears them and sets a `deleted_<12 hex>` username. It then removes the `user_avatars` objects and bans the user. A new deletion path must call the helper, never write `deleted_at` alone. Messages keep their author, because the row stays.
226228

227229
### Production And Docker Compose
228230

‎apps/hocuspocus.server/src/api/controllers/admin-audit.controller.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { EMAIL_DLQ_NAME } from '../../lib/email/jobIdentity'
44
import { adminLogger } from '../../lib/logger'
55
import { maskEmail } from '../../lib/maskEmail'
66
import { createRedisConnection } from '../../lib/redis'
7-
import { mediaStorageQuerySchema } from '../../schemas/admin.schema'
7+
import { type GhostDeleteInput, mediaStorageQuerySchema } from '../../schemas/admin.schema'
88
import type { AppContext } from '../../types/hono.types'
99
import { toBullMQConnection } from '../../types/redis.types'
1010
import * as ghost from '../services/adminGhostAccounts.service'
@@ -349,17 +349,17 @@ export async function getGhostDeletionImpact(c: AppContext) {
349349

350350
/**
351351
* Smart-delete: hard-delete (cascades) when the account has no blocking messages
352-
* and owns no documents; otherwise soft-delete — set deleted_at and ban the user.
352+
* and owns no documents; otherwise soft-delete. Answers 409 when the account is
353+
* gone, no longer a ghost, or an admin, because the dashboard list can be stale.
353354
*/
354355
export async function deleteGhostAccount(c: AppContext) {
355356
try {
356357
const adminAuth = getSupabaseClient()
357358
if (!adminAuth) return c.json({ error: 'Supabase not configured' }, 500)
358359

359-
const userId = c.req.param('id')
360-
if (!userId) return c.json({ error: 'Missing user id' }, 400)
361-
360+
const { id: userId } = c.req.valid('param' as never) as GhostDeleteInput
362361
const result = await ghost.deleteGhostAccount(adminAuth, c.get('prisma'), userId)
362+
if (result.status === 'refused') return c.json({ error: result.reason }, 409)
363363
if (result.status === 'error') throw new Error(result.message)
364364
if (result.status === 'soft_delete') {
365365
return c.json({ success: true, strategy: 'soft_delete', reason: result.reason })

‎apps/hocuspocus.server/src/api/routers/admin.router.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
ghostAccountsQuerySchema,
1515
ghostBulkDeleteSchema,
1616
ghostCleanupAnonymousSchema,
17+
ghostDeleteSchema,
1718
ghostResendSchema,
1819
listDocumentsQuerySchema,
1920
mcpUsageQuerySchema,
@@ -219,7 +220,11 @@ admin.get('/audit/ghost-accounts/summary', adminController.getGhostAccountsSumma
219220

220221
admin.get('/audit/ghost-accounts/:id/impact', adminController.getGhostDeletionImpact)
221222

222-
admin.delete('/audit/ghost-accounts/:id', adminController.deleteGhostAccount)
223+
admin.delete(
224+
'/audit/ghost-accounts/:id',
225+
zValidator('param', ghostDeleteSchema, houseEnvelopeHook),
226+
adminController.deleteGhostAccount
227+
)
223228

224229
admin.post(
225230
'/audit/ghost-accounts/bulk-delete',

0 commit comments

Comments
 (0)