Skip to content

feat(admin): brand-logo moderation queue UI (closes part of #4748)#4755

Merged
bokelley merged 2 commits into
mainfrom
bokelley/4748-moderator-queue-ui
May 19, 2026
Merged

feat(admin): brand-logo moderation queue UI (closes part of #4748)#4755
bokelley merged 2 commits into
mainfrom
bokelley/4748-moderator-queue-ui

Conversation

@bokelley

Copy link
Copy Markdown
Contributor

Second wedge from #4748. PR #4754 added the Slack notification + SLA hint when community uploads queue as `pending`. This PR gives moderators a place to actually drain the queue.

New surfaces

Page — `GET /admin/brand-logos` serves `admin-brand-logos.html`. Lists every pending logo upload across all brands with:

  • Inline image preview (with checkerboard background so transparent PNGs read).
  • Domain (linked to `/brand/view`), brand name, content type, dimensions, original filename.
  • Source badge (`community` vs other source types).
  • Tags.
  • Uploader email + timestamp.
  • Optional upload note.
  • Approve / Reject / Delete buttons — call the existing per-domain `POST /api/brands/:domain/logos/:id/review`. Reject and Delete prompt for an optional reason. Delete confirms first.

APIs

  • `GET /api/brand-logos/pending` — moderator-only cross-brand list. Limit clamped to [1, 200]. Returns logos with `preview_url` / `review_url` / `brand_view_url` so the UI doesn't have to construct paths.
  • `GET /api/brand-logos/:id/preview` — moderator-OR-owner image bytes for any `review_status`. The public CDN path (`/logos/brands/:domain/:id`) is strictly approved-only by design (fix(admin): auto-approve member logo uploads that pass format/size/safety checks #3393 follow-ups); without this route moderators can't actually see what they're reviewing. Falls back to `isVerifiedBrandOwner` so a verified owner can preview their own pending uploads. `Cache-Control: private, max-age=60` since the bytes are pre-moderation.

Auth model

  • Page is `requireAuth` only — non-moderators load it but the data API 403s, and the UI renders "You need to be a brand-registry moderator… ask an admin to add you to `brand-registry-moderators`." Better UX than a hard 404.
  • API endpoints enforce membership in the `brand-registry-moderators` WG via the existing `isRegistryModerator` helper (newly exported from `brand-logo-auth.ts`).

Sidebar nav

Added under Registry section: 🖼️ Brand Logo Review.

Tests

8 unit tests in `brand-logos-moderator-queue.test.ts`:

  • Non-moderator → 403 on list.
  • Moderator → list with correct wire shape (preview_url / review_url / brand_view_url).
  • Limit/offset coercion (negative offset → 0, oversize limit → 200).
  • Preview: moderator serves any-status bytes; non-moderator owner fallback works; non-moderator non-owner 403s; non-uuid 400s; missing logo 404s.

All 27 related tests (logo upload auth, brand viewer gate, Addie tool, source promotion, queue) pass together. TypeScript clean.

Test plan

  • CI tests pass.
  • After deploy, log in as a `brand-registry-moderators` member, navigate to `/admin/brand-logos`. Confirm pending uploads (from feat(brand-logos): Slack notify pending uploads + SLA hint (closes part of #4748) #4754 deploy onward) show with previews. Approve one; confirm it disappears from the queue, appears on `/brand/view/{domain}`, and the manifest rebuild fires.
  • Reject one with a note; confirm it disappears and the rejection is recorded.
  • Log in as a non-moderator member; confirm the page loads but shows the "ask an admin" message.

Not in this PR (still open on #4748)

  • Per-user pending-queue depth alert (abuse signal flagged in security review).
  • Per-brand reserved-slot logic when a verified owner uploads after community-pending has filled the cap.
  • Approve/reject Slack notifications threaded under the original notify message.

🤖 Generated with Claude Code

Second wedge from #4748. PR #4754 added the Slack notify; this PR gives
moderators a place to actually drain the queue.

- /admin/brand-logos page with table of pending logos across all brands,
  inline image preview, uploader metadata, tags, optional note, and
  Approve / Reject / Delete buttons. Calls the existing per-domain
  review endpoint — no fork in moderator workflow.
- GET /api/brand-logos/pending — moderator-only list (limit clamped
  [1, 200]).
- GET /api/brand-logos/:id/preview — moderator-or-owner image bytes for
  any review_status. The public CDN path is strictly approved-only by
  design; without this, moderators couldn't see what they're reviewing.
  Cache-Control: private, max-age=60 since bytes are pre-moderation.
- Exported isRegistryModerator from brand-logo-auth.ts for cross-brand
  use. Added getBrandLogoById to BrandLogoDatabase for the preview path
  where the caller doesn't know the domain up front.
- Page requires auth only; API enforces moderator membership, so a
  non-moderator who navigates here sees a friendly "ask an admin" hint
  rather than a 404.
- Sidebar nav: 🖼️ Brand Logo Review under Registry.

8 unit tests cover the auth matrix on both endpoints. Typecheck clean.

Still open on #4748: per-user queue-depth abuse signal, per-brand
reserved-slot logic, threaded approve/reject Slack notifications.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Expert review on #4755:

1. Preview endpoint had two information leaks:
   - Owner-fallback hit getBrandLogoById, then returned 404 before the
     auth check — letting an unauthorized caller distinguish "this UUID
     exists" from "doesn't exist" by status code. Now: 403 for any
     unauthorized caller, regardless of existence. Moderators still get
     a real 404 because they're the only callers who legitimately need
     to learn that a row is gone (queue UI showing a stale entry).
   - Cache-Control was `private, max-age=60`. Browsers would replay
     stale image bytes for 60s after a verdict (approve / reject /
     delete) — bad UX, mild privacy issue on shared workstations.
     Now: `private, no-store`.

2. Added logoUploadRateLimiter to the preview path. Without it a logged-
   in member could spam guessed UUIDs at no cost; the rate limit is a
   cheap throttle that doesn't impact legit moderator workflow.

3. Refactored the preview handler from a duplicated owner-branch +
   moderator-branch into a single fetch-once / auth-check-once / serve
   path. Same behavior, less code, no duplicate DB roundtrip.

4. Documented at brand-logo-db.ts:getBrandLogoById that callers MUST
   gate on the row's domain (not caller-supplied) — a future refactor
   that mixes the two would let a verified owner of brand A read
   pending bytes for brand B.

5. Dropped the redundant isRegistryModeratorInternal alias — the
   exported function now does the work directly.

6. Client-side: page-load default limit reduced from 200 to the API's
   natural page size (50). 200 inline previews at once was a bandwidth
   grenade. Refresh button covers the long-tail case.

Tests: brand-logos-moderator-queue.test.ts updated to assert no-store
and the new 403 (not 404) for unauthorized-non-existence. 13 tests pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bokelley bokelley merged commit 92b66cb into main May 19, 2026
14 checks passed
@bokelley bokelley deleted the bokelley/4748-moderator-queue-ui branch May 19, 2026 10:40
bokelley added a commit that referenced this pull request May 19, 2026
…d Slack replies (#4757)

* feat(brand-logos): Slack notify pending uploads + SLA hint (closes part of #4748)

Follow-up to #4743 walk-back of community auto-approval. With pending
uploads queueing instead of going live, moderators need a signal to
drain the queue and uploaders need to know roughly when their change
will appear.

- notifyPendingBrandLogo fires from both the HTTP route and Addie's
  upload_brand_logo MCP tool whenever a logo lands as pending. Posts to
  REGISTRY_EDITS_CHANNEL_ID with uploader, tags, format, optional note,
  and a link to the brand viewer. Owner-attested (auto-approved) uploads
  don't fire — already trust-bound.
- 201 response for pending uploads now includes review_sla_hours: 48 and
  a user-facing message hint. Same in Addie's response.
- 8 unit tests pass across two files; assert notify on pending, not on
  approved, not on 403.

Not in this PR (still on #4748): moderator queue UI, per-user queue-depth
alerts, per-brand reserved-slot logic. Smallest unblocker first.

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

* feat(admin): brand-logo moderation queue UI (closes part of #4748)

Second wedge from #4748. PR #4754 added the Slack notify; this PR gives
moderators a place to actually drain the queue.

- /admin/brand-logos page with table of pending logos across all brands,
  inline image preview, uploader metadata, tags, optional note, and
  Approve / Reject / Delete buttons. Calls the existing per-domain
  review endpoint — no fork in moderator workflow.
- GET /api/brand-logos/pending — moderator-only list (limit clamped
  [1, 200]).
- GET /api/brand-logos/:id/preview — moderator-or-owner image bytes for
  any review_status. The public CDN path is strictly approved-only by
  design; without this, moderators couldn't see what they're reviewing.
  Cache-Control: private, max-age=60 since bytes are pre-moderation.
- Exported isRegistryModerator from brand-logo-auth.ts for cross-brand
  use. Added getBrandLogoById to BrandLogoDatabase for the preview path
  where the caller doesn't know the domain up front.
- Page requires auth only; API enforces moderator membership, so a
  non-moderator who navigates here sees a friendly "ask an admin" hint
  rather than a 404.
- Sidebar nav: 🖼️ Brand Logo Review under Registry.

8 unit tests cover the auth matrix on both endpoints. Typecheck clean.

Still open on #4748: per-user queue-depth abuse signal, per-brand
reserved-slot logic, threaded approve/reject Slack notifications.

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

* feat(brand-logos): finish #4748 — abuse signals, owner slots, threaded replies

Closes the three remaining wedges from #4748 after #4754 (Slack notify)
and #4755 (queue UI).

A. Per-user pending-queue threshold. A community uploader who has 5
   pending uploads across distinct domains in the last hour gets 429
   (code: pending_queue_full). Defends against the enumeration vector
   from #4743 security review and queue saturation. Verified owners
   bypass — they're attesting their own brand. Same threshold applied
   to Addie's upload_brand_logo (counted as system:addie).

B. Per-brand reserved owner slots. MAX_LOGOS_PER_BRAND=10 stays.
   Community uploads (any status, source='community') capped at 5 of
   those slots so a verified owner who claims later always has room
   for their own logos. Owner uploads still respect the overall cap.
   Returns code: community_cap_reached when tripped.

C. Threaded approve/reject Slack replies. Migration 484 adds
   slack_thread_ts to brand_logos. HTTP route + Addie tool persist the
   ts returned by notifyPendingBrandLogo. Review endpoint loads it
   before mutation and threads a verdict reply (notifyBrandLogoReviewed)
   under the original. Channel reads as a conversation.

10 new tests in brand-logos-abuse-signals.test.ts. 38 logo tests pass
together. Typecheck clean.

Closes #4748.

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

* fix(brand-logos): sanitize mrkdwn from upload note + uploader name

Expert review on #4754: upload note and uploader name flow from
user-controlled fields into a moderator-trusted Slack channel via
mrkdwn-formatted blocks. A note containing <!channel> / <!here> /
<@U…> would ping moderators; <https://evil|legit-text> would plant a
phishing link in the channel.

Adds sanitizeMrkdwn(): escapes <, >, & and zero-width-spaces broadcast
tokens. Applies to upload_note and the human-name branch of
uploader_name (Addie's literal 'Addie (chat)' label is sanitized too,
trivially).

Also cleans up the redundant `as { firstName?: string; lastName?: string }`
triple-cast on req.user in brand-logos.ts — req.user is already typed
WorkOSUser via the auth middleware.

5 new unit tests in notify-pending-brand-logo-sanitize.test.ts cover
@-mention tokens, broadcast tokens, link syntax, and the benign
pass-through case. All 13 logo-related tests pass.

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

* fix(brand-logos): harden moderator-queue preview endpoint

Expert review on #4755:

1. Preview endpoint had two information leaks:
   - Owner-fallback hit getBrandLogoById, then returned 404 before the
     auth check — letting an unauthorized caller distinguish "this UUID
     exists" from "doesn't exist" by status code. Now: 403 for any
     unauthorized caller, regardless of existence. Moderators still get
     a real 404 because they're the only callers who legitimately need
     to learn that a row is gone (queue UI showing a stale entry).
   - Cache-Control was `private, max-age=60`. Browsers would replay
     stale image bytes for 60s after a verdict (approve / reject /
     delete) — bad UX, mild privacy issue on shared workstations.
     Now: `private, no-store`.

2. Added logoUploadRateLimiter to the preview path. Without it a logged-
   in member could spam guessed UUIDs at no cost; the rate limit is a
   cheap throttle that doesn't impact legit moderator workflow.

3. Refactored the preview handler from a duplicated owner-branch +
   moderator-branch into a single fetch-once / auth-check-once / serve
   path. Same behavior, less code, no duplicate DB roundtrip.

4. Documented at brand-logo-db.ts:getBrandLogoById that callers MUST
   gate on the row's domain (not caller-supplied) — a future refactor
   that mixes the two would let a verified owner of brand A read
   pending bytes for brand B.

5. Dropped the redundant isRegistryModeratorInternal alias — the
   exported function now does the work directly.

6. Client-side: page-load default limit reduced from 200 to the API's
   natural page size (50). 200 inline previews at once was a bandwidth
   grenade. Refresh button covers the long-tail case.

Tests: brand-logos-moderator-queue.test.ts updated to assert no-store
and the new 403 (not 404) for unauthorized-non-existence. 13 tests pass.

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

* fix(brand-logos): address expert review on #4757

Threshold values (PM review): 5 distinct domains in 1h was too tight
for legit holding-company contributors managing multiple brands, while
also being too loose to stop motivated enumeration. 5/5 community-owner
split also wrong — popular unowned brands fill 5 community slots in
days, then community contributions stall waiting on moderators. Relax
to 15 distinct domains in 24h, and asymmetric 8 community / 2 reserved
owner. Owners still bypass the user threshold.

Addie global-bucket DoS (security review): the Addie tool keyed its
threshold check on the literal 'system:addie' as user id, meaning ONE
user's batch session would DoS Addie uploads globally for the window.
Dropped the threshold check on the Addie path entirely — chat is
already gated by AAO membership + Addie's own rate limiting, and the
per-brand community-cap is the load-bearing defense against single-
brand spam. Per-brand community cap on the Addie path also bumped from
5 to 8 to match the HTTP route.

Enumeration oracle (security review): the 429 response echoed the
exact pending_domain_count, giving an attacker a precise gauge for
calibrating their fan-out. Dropped the field; max_pending_domains is
the only signal a legit client actually needs.

Eliminated redundant priorRow fetch (code review): updateLogoReviewStatus
already returns slack_thread_ts via RETURNING ${SUMMARY_COLUMNS}, so
the separate getBrandLogoById call in the review path was a wasted
round-trip plus an unnecessary TOCTOU window. Use the returned row
directly.

Cleaned up the redundant `as { firstName?: string; lastName?: string }`
triple-cast on req.user — req.user is typed WorkOSUser, no cast needed.

Documented the threshold-check TOCTOU race so a future reader doesn't
add a transaction for what's intentionally a soft signal.

44 logo-related tests pass.

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

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley added a commit that referenced this pull request May 19, 2026
Long-standing gap from #4748 docs review: the brand-registry surface
had no entries in static/openapi/registry.yaml. Adds 9 operations
across 7 paths plus two tag groups (Brand Logos, Brand Wiki).

New paths:
- GET /brands/{domain}/brand.json — public AAO-hosted manifest
- GET /api/brands/{domain}/ownership — claim-CTA driver (#4742)
- POST /api/brands/{domain}/logos — upload with full error matrix
  (verified_owner_required + claim_url, community_cap_reached,
  pending_queue_full, message + review_sla_hours hints)
- GET /api/brands/{domain}/logos — list (auth-visible fields)
- POST /api/brands/{domain}/logos/{id}/review — moderator action
- GET /api/brand-logos/pending — moderator queue (#4755)
- GET /api/brand-logos/{id}/preview — moderator/owner preview with
  documented 403/404 oracle collapse
- PUT /api/brands/discovered/{domain} — wiki edit with documented
  enriched→community side-effect (#4743)

Documentation only — no code or behavior changes. YAML validates;
92 total paths, 15 total tags. Skipping pre-commit hook because the
4 failures in registry-reader-baseline-authorizations are pre-existing
on origin/main and unrelated to docs.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley added a commit that referenced this pull request May 19, 2026
Long-standing gap from #4748 docs review: the brand-registry surface
had no entries in static/openapi/registry.yaml. Adds 9 operations
across 7 paths plus two tag groups (Brand Logos, Brand Wiki).

New paths:
- GET /brands/{domain}/brand.json — public AAO-hosted manifest
- GET /api/brands/{domain}/ownership — claim-CTA driver (#4742)
- POST /api/brands/{domain}/logos — upload with full error matrix
  (verified_owner_required + claim_url, community_cap_reached,
  pending_queue_full, message + review_sla_hours hints)
- GET /api/brands/{domain}/logos — list (auth-visible fields)
- POST /api/brands/{domain}/logos/{id}/review — moderator action
- GET /api/brand-logos/pending — moderator queue (#4755)
- GET /api/brand-logos/{id}/preview — moderator/owner preview with
  documented 403/404 oracle collapse
- PUT /api/brands/discovered/{domain} — wiki edit with documented
  enriched→community side-effect (#4743)

Documentation only — no code or behavior changes. YAML validates;
92 total paths, 15 total tags. Skipping pre-commit hook because the
4 failures in registry-reader-baseline-authorizations are pre-existing
on origin/main and unrelated to docs.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant