ReviewOS

stacks/bench-review

moderation: flag queue, comment/photo takedown, bulk actions, claim evidence, and signup abuse defenses are missing

#51
Closed glennmichael123 opened this 9 days ago ·
9 days ago

Problem

Bench publishes reviews of named, sitting judges, so the moderation pipeline is the product's legal and trust backbone. Right now the pipeline has one working surface (the pending-review queue) and gaps everywhere else: flags go into a black hole, comments and photos reach the public with no admin removal path, claims to a judge's identity carry no evidence, and account creation is cheap to script. Together these make harassment, doxxing, and impersonation both easy to produce and hard to clean up.

Findings

  • Flags are write-only. review_flags rows are inserted (app/Actions/Reviews/FlagReviewAction.ts:105-113) but no admin route, action, view, or store ever reads or resolves them; status is permanently open. Because moderators are emailed only on the first open flag (FlagReviewAction.ts:93-118), each review emits exactly one email ever — miss it and 50 later flags are silent.
  • Comments auto-publish with no takedown. app/Actions/Reviews/CommentSubmitAction.ts:88-96 inserts status: 'published'; no admin comment route exists anywhere in routes/, no email-verification gate applies, and FlagReviewAction covers reviews only. Removal currently requires direct DB surgery.
  • Photos bypass moderation entirely. review_photos has no status column (migration 0000000005), UploadReviewPhotosAction.ts:44-62 checks ownership but never review status — so photos can be attached to an already-published review without resetting it to pending (unlike text edits, UpdateMyReviewAction.ts:117) — the upload route has no throttle (routes/api.ts:166-169), and the admin ReviewsTable renders no images, so moderators approve blind.
  • No bulk moderation. Only single-{id} PATCH/DELETE routes exist (routes/api.ts:385-396); UpdateReviewStatusAction.ts:35 accepts one id and sends an author email inline per call, making moderator throughput the publication bottleneck for a pending-by-default queue.
  • Judge-claim flow captures zero identity evidence. ClaimJudgeProfileAction.ts:24-29,62-72 accepts only judgeId; the credential queue (CredentialQueueIndexAction.ts:22-32) shows admins nothing to verify against, yet approval unlocks official judge responses (SubmitJudgeResponseAction.ts:43-54) — a mistaken approval is public impersonation of a judge.
  • Registration relies on IP throttle alone. routes/api.ts:71-74 has throttle:5,10m and nothing else: no CAPTCHA/Turnstile, honeypot, or disposable-email blocking anywhere in app/config/resources, so verified throwaway accounts are trivially scriptable.

Acceptance criteria

  • GET /api/admin/flags (open-first) and PATCH /api/admin/flags/{id} (dismiss/actioned, with logModeration) exist; open-flag counts appear in ReviewIndexAction rows; flags auto-close on review reject/delete so the first-open-flag email re-arms
  • Admins can set a comment's status to rejected via an authed admin endpoint, and CommentSubmitAction requires a verified email
  • Photo upload to a published review resets it to pending; photo thumbnails render in the admin reviews queue; the upload route is throttled (e.g. throttle:10,10m)
  • Review status updates accept an ids[] batch with per-row moderation logging, and ReviewsTable.stx gains checkboxes plus a bulk publish/reject bar
  • Judge claims require an evidence field (court email verification loop, or at minimum free-text + document reference) that is displayed in the credential queue beside approve/reject
  • Registration includes a CAPTCHA/Turnstile check and a disposable-email-domain blocklist ahead of register()

From the 2026-06-10 production-readiness audit.

8 days ago

Triage — one of the five pieces landed. Keeping this open.

  • Flag queue exists. GET /api/admin/flags + POST /api/admin/flags/{id}/resolve, with a real moderation surface at resources/views/admin/flags.stx (FlagsTable component, open/resolved/dismissed filters).

Still missing:

  • Comment and photo takedown. No admin routes touch comments or photos. A moderator can unpublish a review but cannot take down a single abusive comment or an uploaded image without going to the database. (A review takedown does now hide its thread — the viewable-review middleware was added to the comments read route — but that is takedown by proxy, not a per-comment tool.)
  • Bulk actions
  • Claim evidence capture
  • Signup abuse defenses

Adjacent and worth noting rather than duplicating: #49's suspend endpoint and banned_at landed, so the "only tool is a hard delete" problem is no longer true for accounts — just for individual pieces of content.

8 days ago

Criterion 2 is done (f896ede5, b682ab3f). Both halves of it.

  • Admins can set a comment's status to rejected — PATCH /api/admin/comments/{id}, auth + admin gated, backed by a queue at /admin/comments (Published / Removed / All, with Remove and Restore). An endpoint with no surface would have repeated the mistake #48 was filed for.
  • CommentSubmitAction requires a verified email — the same gate reviews have had. Commenting was the one way to put text on a judge's page without ever proving an address.

Design decisions worth recording:

Soft and reversible, not a delete. The row stays with status = 'rejected'. The thing a moderator acted on survives for anyone who later questions the decision — on a platform publishing criticism of named sitting judges, someone will. Hard deletion destroys the evidence trail that #53 is about, and the author's own DELETE /api/me/comments/{id} remains the only path that truly removes a row. Restore exists because an irreversible tool makes moderators hesitant rather than careful.

The comment counter is recounted, not decremented. The public thread renders only published rows, so leaving judge_reviews.comments alone would advertise replies nobody can read. Recounting also heals drift rather than compounding it — tested by corrupting the counter to 99 and confirming it comes back as the true count, not 98.

Re-sending the current status writes no audit entry. One decision should not leave two records suggesting two.

The queue shows the author's real name even where the comment is anonymised publicly, with a marker saying so. Judging harassment means knowing who wrote it and whether the same account keeps doing it.

Verified by driving the real action against a database copy (18 cases: flip, soft-not-deleted, counter down and back, both directions audited, no-op repeat, 422 on unknown status, 404 on unknown id, drift correction) and against the built artifact in the browser (per-row buttons, anonymised marker, correct PATCH body, error state with no misleading empty state).


Remaining on this issue:

  • Criterion 1's tail: open-flag counts in ReviewIndexAction rows, and flags auto-closing on review reject/delete so the first-open-flag email re-arms. The queue and resolve endpoint themselves are done.
  • Photo moderation — upload to a published review should reset it to pending, thumbnails in the admin reviews queue, upload route throttle
  • Bulk review actions
  • Judge-claim evidence capture
  • Registration CAPTCHA + disposable-email blocklist (needs a provider decision and keys — not something I can pick unilaterally)
8 days ago

Photo moderation — criterion 3 done (6bfd13d0, 2ef7f668).

  • A photo on a published review sends it back to pending. Text edits have always done this; photos did not, so the one channel that could put an unreviewed image on a live page about a named sitting judge was also the only channel that bypassed moderation. Publish the text, wait for approval, attach anything.
  • Thumbnails render in the admin reviews queue. Moderators were approving blind — the queue showed title, judge, reviewer and rating, and nothing of the images, which are the highest-risk content on the page.
  • The upload route is throttled — already was, throttle:10,10m. That part of the finding is stale.

The reset happens after the writes and only when something landed, so a review is never knocked offline by an upload that then failed, and a batch rejected by the mime gate leaves a published review alone. Pending and rejected reviews are untouched — already out of public view. The author is told via returned_to_queue, because someone whose review silently vanished would reasonably conclude the upload broke something.

Five tests drive the real action with a real 1×1 PNG, so the whole path runs — multipart parse, mime gate, sharp pipeline, row write, and the status decision that depends on all of it having worked. Published → pending and says so; pending stays and says nothing; rejected is not resurrected; a non-owner gets the same 404 a missing review gives, with nothing written; a rejected upload leaves a published review published.

review_photos still has no status column, so there is no per-photo takedown — removing one means deleting it. That is worth its own criterion if per-photo moderation is wanted; today a moderator's lever is the review as a whole.


Unrelated finding, worth its own issue: 15 of the 24 PNGs under public/images/ are corrupt. Their signature reads \x89PNG\n\x1a\n — the \r is gone, so file reports "data" and browsers refuse to decode them. It looks like a CRLF→LF normalisation that ran over binaries. public/images/avatars/avatar-1.png through avatar-4.png are among them, as are the diagram and screenshot assets. I found it only because I picked one as a test fixture and the thumbnail would not render.


Remaining here: open-flag counts in ReviewIndexAction rows and flags auto-closing on reject/delete; bulk review actions; judge-claim evidence; registration CAPTCHA + disposable-email blocklist (needs a provider decision).

8 days ago

Criterion 1 is now complete (2a2b11a5).

  • GET /api/admin/flags and a resolve endpoint, with logModeration — were already done
  • Open-flag counts appear in ReviewIndexAction rows. A moderator working the review queue could not see that a review had been reported at all — flags live on their own page, so deciding whether to publish something meant knowing to go and look first. The count now sits on the row the decision is made on, and links to the queue.
  • Flags auto-close on reject, so the first-open-flag email re-arms.

That last one is the one with teeth. Moderators are emailed only on the first open flag against a review — FlagReviewAction counts open rows before inserting, so that anonymous flagging cannot spam them. The corollary: a flag left open after the review was dealt with means that count is never zero again, so a review that is rejected, edited by its author and re-published can be reported a hundred more times in silence. Deleting a review already cleared its flags; rejecting one did not.

Resolved rather than dismissed, since the flag was acted on. Already-closed flags are untouched — a dismissal was a judgement call, and rejecting the review later should not rewrite it as "acted on". Publishing deliberately does not close flags: approving something that was reported does not answer the report.

Five tests cover it, and they caught a flake of their own making worth recording: each test file passed alone while five failed in a full run, because users.email is globally unique, the suite shares one database, and two files happened to seed the same fixture address — so whichever ran second collided. Fixture emails are namespaced per file now, and the suite was run twice to confirm it is not order-dependent.


Remaining on this issue:

  • Bulk review actions (ids[] batch with per-row logging, checkboxes and a bulk bar in ReviewsTable)
  • Judge-claim evidence capture
  • Registration CAPTCHA + disposable-email blocklist — needs a provider decision and keys, so not something I can pick

Also still true: review_photos has no status column, so there is no per-photo takedown. A moderator's lever is the review as a whole. Worth its own criterion if per-photo moderation is wanted.

8 days ago

Bulk moderation is in — feat(admin): approve or reject a batch of reviews in one request (1d62c8f) and feat(admin): checkboxes and a bulk publish/reject bar on the reviews queue (e535795).

POST /api/admin/reviews/bulk-status takes ids[] plus a status. Every row goes through the same code path the single-id endpoint uses, so each one gets its own audit row, author notification and email, flag close-out and publish fan-out. Nothing about a batch is cheaper — it is only fewer clicks. That required lifting the ~250 lines of consequences out of UpdateReviewStatusAction into app/Helpers/reviewModeration.ts, which both endpoints now call; a second copy would have drifted invisibly, since every side effect in there is deliberately best-effort.

Rows run serially (each can send two emails) and one failure does not abort the batch — a stale id reports against its own row and the other decisions still land. Capped at 50, which the UI cannot reach since the queue pages at 25.

ReviewsTable.stx gains per-row checkboxes, a page select-all with a tri-state dash, and a bar with Publish selected / Reject selected / Clear. Both bulk buttons confirm first — publish is not the safer direction, since it emails every author and notifies each claimed judge and their followers. Selection is page-scoped: the store clears it on every fetch, so nothing can be actioned after it scrolls out of view.

Six tests cover per-row audit logging, partial failure, no-op rows, flags closing for every row of a reject, id dedupe and the input guards. The UI was verified end to end against a stub API.

Unchecking the fourth acceptance box. Remaining on this issue: judge-claim evidence capture, and the registration CAPTCHA + disposable-email blocklist (that one needs a provider decision).

8 days ago

Correction to the last line above: that should read checking the fourth acceptance box, not unchecking. It is ticked in the issue body now.

8 days ago

Judge-claim evidence is in — 9a35dd94 (backend) and 1d86b256 (UI). Ticking the fifth acceptance box.

ClaimJudgeProfileAction took exactly one input, judgeId, so the queue showed an admin a name, an email and a dropdown selection. Claims now require free-text evidence (20 char floor — enough to rule out the empty gesture, not enough to pretend it is a real check) plus an optional http(s) link, both stored on users beside the other credential_* columns.

The link is restricted to http/https in app/Helpers/credentialEvidence.ts. The queue renders it clickable for the one session on the site that can approve the claim, so javascript:/data: there would be the worst place on the site to accept a scheme; a bad URL is a 422 rather than a silent drop, since an admin seeing no link cannot tell whether one was offered.

UpdateCredentialClaimAction now always overwrites those columns, including with empty — otherwise a user could file a judge claim with evidence, switch to a bar-admission claim, and leave the admin reading evidence for a claim that no longer exists.

The queue renders the evidence directly above Approve/Reject, with the URL printed as its own link text so the destination is visible rather than labelled. Claims without evidence (older ones, and reviewer-credential claims, which do not require it) say so explicitly instead of showing an empty panel.

Six tests, each falsified before being trusted. Verified end to end against a stub API.

A court-email verification loop would be stronger than free text and is still worth doing — it needs a mail-domain policy decision first, which is why the issue allowed "at minimum free-text + document reference".

Remaining here: the registration CAPTCHA + disposable-email blocklist, which needs a provider decision.

8 days ago

Turnstile is in — 32555d8c (verification + blocklist), d0db8759 (widget, CSP), e7b1f01a (gate, docs). That was the last open criterion, so all six boxes are now ticked.

Signup abuse defenses. Two checks ahead of register(), cheapest first. A disposable-email-domain blocklist (small static list on purpose — not a vendored 40k file nobody updates, not a runtime fetch on the signup path; suffix matching anchored on a dot so notmailinator.com is not caught), then Turnstile verified server-side with a 5s timeout.

Turnstile fails closed on both a negative verdict and an unreachable endpoint — the value of the check is that it cannot be skipped, and an outage is something an operator can see and fix where a quiet fallthrough is not. The unreachable case answers 503 with a retry message rather than calling the person a robot. An unset secret passes through so local development works, and gate check 24 fails a release build where either key is missing, or where a site key is set but the artifact carries no widget.

The CSP needed widening or this would have failed in the worst way: script-src is per-page and hash-based, so Cloudflare's script would have been refused and the form would have looked complete while the button did nothing. The origin is added only to pages that reference it.

I also ticked the first three boxes after verifying them — they were done in earlier sessions and never marked. One honest deviation: the flag queue exposes POST /api/admin/flags/{id}/resolve rather than PATCH /api/admin/flags/{id}. Functionally equivalent, logModeration included.

Everything verified end to end against Cloudflare's always-passes test key: script loads with no CSP violation, widget writes a token, the POST carries it, and the widget resets after a failed signup (tokens are single-use — without the reset, a retry after an ordinary rejection fails the security check instead).

Ready to close unless you want anything else here. Two things worth recording as follow-ups rather than blockers: a court-email verification loop would beat free-text claim evidence, and the blocklist is a speed bump — the email verification gate behind it is what actually holds.

8 days ago

Closing — all six acceptance criteria met and verified.

  • Flag queue reads and resolves, with logModeration; open-flag counts on the review rows; flags auto-close on reject and on delete, which re-arms the first-open-flag email
  • Comment takedown via an authed admin endpoint; CommentSubmitAction requires a verified email
  • A photo on a published review sends it back to pending; thumbnails render in the moderation queue; the upload route is throttled 10,10m
  • Bulk publish/reject with per-row audit logging, checkboxes and a bulk bar
  • Judge claims require evidence, shown beside approve/reject in the credential queue
  • Registration carries Turnstile plus a disposable-email blocklist, with gate check 24 failing a release build that is missing either key

Follow-ups worth their own issues rather than holding this open:

  • a court-email verification loop would beat free-text claim evidence (needs a mail-domain policy)
  • review_photos has no status column, so there is no per-photo takedown — removal is still all-or-nothing on the review
  • the disposable-email list is the head of the distribution, not a wall

Operationally: TURNSTILE_SITE_KEY and TURNSTILE_SECRET_KEY need to exist before a production build, and the site key has to be on the build runner — it is baked into register.html at build time.

Sign in to comment on this issue.