Add moderator delisting for published extensions - #215
Merged
Conversation
The Paygate extension (paygate.love, github.com/hanihiyoze/paygate-fossbilling)
has gone dark: its source repo is deleted and its site is down, so its
catalogue listing links to a dead download with no way to reach the
developer. There was no way to remove a published extension from the
catalogue at all - withdraw() only ever worked pre-publish, and moderation
routes could approve/reject revisions but never touch a live one.
Adds extensions.delisted_at/delist_reason (plain ADD COLUMN, no table
rebuild - see the schema.ts comment for why a CHECK constraint here isn't
worth the rebuild's FK risk) and POST /extensions/{id}/delist
(moderator-only). Delisted rows are hidden from both public catalogue reads
but keep their content and history, so the owner can still see why via
GET /extensions/mine and a moderator can re-list by hand later.
No email/notification on delist yet - the reason is visible to the owner in
their dashboard; actually notifying them needs picking an email provider,
which is out of scope here.
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
api | 421f7e6 | Commit Preview URL Branch Preview URL |
Sep 04 2026, 09:00 AM |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 13 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Contributor
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Requires human review: Auto-approval blocked by 6 unresolved issues from previous reviews.
Re-trigger cubic
… reasons - v1's /extensions/v1/list and /:id endpoints share the extensions table with v2 but filtered only on published_at, so a delisted extension (e.g. the Paygate case this PR exists for) stayed visible to FOSSBilling installs even after v2 hid it. Both v1 queries now also require delisted_at IS NULL, with regression tests. - delist() left extensions.updated_at untouched, unlike every other write to this table (approve()'s publish statement, for one). Now bumps it. - delistBlockedError()'s diagnostic SELECT wasn't wrapped in try/catch, so a transient DB error there would throw out of delist() as a raw exception instead of the structured DatabaseResult the moderation route expects. Wrapped, matching the rest of this file's error handling. - DelistReasonSchema's min(1) let a whitespace-only reason through, which would then be shown to the owner as "why" verbatim. Added .trim(). - README's 'Reading Owner State' intro still said 'three independent fields' after the delisted paragraph introduced a fourth; fixed the count. Not changed: cubic also flagged delist() returning the caller-supplied id verbatim instead of resolving to canonical casing. Checked against withdraw() and create() in the same file - both do exactly the same thing (echo the input id, not a re-fetched canonical one), so this is consistent with, not a deviation from, how every other id-addressed write here already behaves. Replying on the review thread with this instead of changing it. Also not changed here: ReviewNoteRequiredSchema (used by reject and by developer-claim rejection) has the identical missing-trim gap DelistReasonSchema had. Pre-existing and outside this PR's diff; flagging separately rather than expanding scope.
Contributor
There was a problem hiding this comment.
0 issues found across 6 files (changes from recent commits).
Requires human review: Adds moderator delisting for published extensions: a new delist endpoint, migration 0022 adding delisted_at/delist_reason columns with index changes, and updates to public and owned read paths. Database schema change and API contract changes warrant human review.
Re-trigger cubic
Follow-up to the cubic finding on DelistReasonSchema's whitespace-only reason: review_note (used by reject and by developer-claim rejection) had the identical gap. Added .trim() before .min(1) there too, with a whitespace-only regression test on both routes.
Contributor
There was a problem hiding this comment.
0 issues found across 3 files (changes from recent commits).
Requires human review: Adds moderator delisting for published extensions: schema migration 0022, new moderator-only delist endpoint, and public read paths excluding delisted rows. Database schema change and API contract changes warrant human review.
Re-trigger cubic
Confirms the current state: the owner can still see a delisted extension's
full record (including the delist reason) via GET /extensions/mine/{id},
unaffected by delisted state since getOwned() never filters on it. A
moderator's only access to a delisted extension besides delisting it is
GET /extensions/{id}/revisions (the one owner-extensions.ts route with a
moderator bypass on the ownership check) - which returns revision history,
not the delist reason/timestamp itself. Anyone else gets 403 from both
routes.
…o delist-published-extensions
Closes the gap the delisted-extension access-control check surfaced: a
moderator could delist an extension but had no dedicated way to see the
delisted state (reason/timestamp) afterward - only the owner could, via
GET /extensions/mine/{id}. Adds a moderator-only equivalent under
/moderation rather than reusing GET /extensions/{id} (public,
published-only) or GET /extensions/mine/{id} (owner-only, 403s everyone
else) - same OwnedExtension response shape, resolved via the existing
getOwned() rather than a new query.
Namespaced /moderation/extensions/{id} specifically to avoid colliding with
public-extensions.ts's GET /extensions/{id}, which is registered on the
same app and would otherwise win or lose ordering-dependently.
Updated the access-control regression test to use this instead of the
GET /extensions/{id}/revisions workaround it previously relied on to prove
moderator access existed at all.
Contributor
There was a problem hiding this comment.
0 issues found across 3 files (changes from recent commits).
Requires human review: Adds moderator delisting for published extensions: schema migration 0022, new moderator-only endpoints, and public read paths (v1 and v2) excluding delisted rows. Schema change and new API contract require human review.
Re-trigger cubic
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
There was no way to remove a published extension from the catalogue at all.
withdraw()only ever worked pre-publish ("A published extension cannot be withdrawn by its owner — consumers pin the id"), and the moderation routes could approve/reject revisions but never touch an already-live one.What
extensions.delisted_at/delist_reasoncolumns, added via a plainADD COLUMNmigration (0022) — no table rebuild. I initially added aCHECKconstraint tying the two together, which forces SQLite to rebuild the table; that path hit a real bug in drizzle-kit's generated migration (it selected the new columns from the old table in theINSERT ... SELECT, which doesn't have them yet) and would also have reopened the FK-during-rebuild landmineextension_revisionsalready forced migration 0021 to work around. Documented the trade-off in the schema comment and dropped the CHECK —delist()is the only writer and already guards onpublished_at IS NOT NULL.ExtensionsDatabase.delist()— atomic, moderator-only, guarded the same wayreject()guards on revision status.POST /extensions/{id}/delist(moderator-only). Public catalogue reads (list,getById) now exclude delisted rows; the owner'sGET /extensions/minestill shows the extension plus the delist reason.migrations.test.ts: several cases held back only migration0021by exact filename so they could seed pre-restructure rows and watch them cross it. Since 0022 comes after 0021 and depends on its schema, those needed to hold back "0021 and anything after," not just 0021 itself.