Add moderated home image uploads - #3
Merged
Merged
Conversation
Recreates the classic Cybertown image-check flow: an uploaded home image is held 'pending' and hidden behind a 'NOT CHECKED!' placeholder until a Block Leader / Deputy / admin approves it via the CHECK tool. Backend: - Migration adds image_status/image_checked_by/image_checked_at to home (existing images grandfathered to 'approved'). - uploadHomeImage sets status 'pending'; reset/remove clear it. - GET /home/:username only exposes the real image once approved. - New moderator endpoints (gated by canStaff/canAdmin): GET /home/moderation/queue, POST /home/moderation/:placeId/approve, POST /home/moderation/:placeId/reject (deletes the file). Frontend: - main2d.vue shows the NOT CHECKED! placeholder for pending images. - New HomeImageCheckPage lists the pending queue with Approve/Reject. - Wires the previously-inert CHECK button in BlockTools to open it. Also gitignores spa/assets/homes-uploads (runtime user uploads), matching the existing spa/assets/object ignore. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…(QA finding) Blocking finding: pending (unapproved) uploads were written to spa/assets/homes-uploads, which nginx serves publicly, so anyone could fetch an unchecked image directly at /assets/homes-uploads/<placeId>.webp - bypassing moderation entirely even though the public home API hid it. Storage redesign: - Pending uploads now go to a private directory outside every nginx-served path (PRIVATE_UPLOADS_DIR, default /usr/src/app/private-uploads/homes-pending), gitignored and created on demand. Approved images live in the public dir as before. - Approval promotes the file from private -> public (rename, with an EXDEV copy+unlink fallback for separate mounts). The record is flipped to approved via an atomic UPDATE ... WHERE image_status='pending' so concurrent/duplicate approvals cannot both proceed; the file only becomes public after that claim succeeds, and a failed move reverts to pending - the image is never publicly readable while still marked pending. - Rejection/removal/reset delete the file from both directories (derived from the numeric place id, so no path traversal) and clear the DB fields; rejection uses the same atomic claim. Missing files are tolerated; re-upload after rejection works. - Replacing an approved image with a new upload removes the old public file and holds the new one privately/pending, so it is not exposed until re-approved. Authenticated preview endpoint: - New GET /home/moderation/:placeId/image streams the pending image to a moderator only: 401 unauthenticated, 403 non-moderator, 404 when absent, image/webp with no-store, and resolved from a validated numeric id (never a client path). - The CHECK queue now loads previews through this endpoint as authenticated blobs (the apiToken header cannot ride on a plain <img src>), and the queue payload no longer exposes a public URL. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
DJAscendance
force-pushed
the
feature/home-image-moderation
branch
from
July 17, 2026 12:33
c69be8b to
255b008
Compare
…ass race) Pending home images were all written to a single shared private filename (<placeId>.webp), and approval published "whatever is currently pending" rather than the exact image the moderator reviewed. So an approval begun for image A could publish a different, unchecked image B that had replaced A under that shared name - deterministically (review A -> owner swaps to B -> approve) and under a concurrent approve/upload race. Anonymous users could then fetch B. Fix - revision-bound uploads serialized by a per-home row lock: - Add home.image_revision: every upload gets a fresh unguessable token and its own private file <placeId>-<revision>.webp, so a replacement upload never overwrites the file an in-flight approval is reading. The public file stays the canonical <placeId>.webp (stable URL). - All image mutations (upload/approve/reject/remove/reset) run inside a transaction that first takes SELECT ... FOR UPDATE on the home row, serializing them across processes. - Approve/reject are bound to the revision the moderator reviewed (sent from the queue). If the current revision no longer matches, the API returns 409 and nothing is published. Approval publishes the reviewed revision via an atomic temp-then-rename into the public dir; the private copy is removed after commit. Net invariant: the public file only ever contains an approved revision's bytes; an unchecked upload can never be promoted. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Unit spec (home.service.spec.ts): approval/rejection refuse a revision that no longer matches the reviewed one (or is absent), and write nothing. - Integration QA script (api/qa/home-image-moderation-race.sh): races approve(A) vs upload(B) and the review->swap->approve case over many iterations, asserting the unchecked replacement is never publicly reachable (409 on stale approve). - CLAUDE.md: correct the stale note that all home images live under spa/assets/homes-uploads (only approved do; pending live in the private PRIVATE_UPLOADS_DIR), and document the concurrency contract. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ommit races) The row lock only protects work done while its transaction is open. Several filesystem cleanups ran AFTER the transaction committed and the lock was released, so in a multi-process deployment they were not serialized against a later operation. Follow-up review found: - Upload / remove / reset / reject deleted the shared canonical PUBLIC file after commit, outside the lock. A concurrent operation that committed in the gap (e.g. an approval that just published an image) could have its public file deleted by the earlier request. - remove / reset used a wildcard delete of every "<placeId>-*.webp" pending file, which could wipe a concurrent upload's freshly-written file, leaving a pending record with no file (broken preview, un-approvable). - approveHomeImage published the public file inside the transaction; if the commit then failed, the rollback left the image public while the record was still pending. - approve's post-commit unlink was not wrapped, so a transient FS error returned HTTP 400 even though the approval had already committed and published. - Legacy images left pending before the image_revision migration had a NULL revision and became permanently un-moderatable (stuck in the queue). Fixes: - deletePublicImageIfState(): re-acquires the home row lock and deletes the canonical public file ONLY while the record still holds the exact (status, revision) the caller committed, so an old request can never clobber a newer operation's public file. - remove / reset capture the exact revision under the lock and delete only that immutable private file (no wildcard). Reject/approve delete only their captured revision. - approveHomeImage compensates on rollback: if publishing wrote the public file but the commit failed, the (state-guarded) public file is removed. Post-commit unlinks are now best-effort and never fail an already-committed operation. - publishApprovedImage sweeps stale ".tmp-<placeId>-*" staging files from the public dir. - Migration backfills legacy NULL-revision pending rows to "no image" so the queue is never stuck (unchecked images are never exposed; owner re-uploads). Adds hermetic unit tests for the state-guarded cleanup / capture / rollback compensation, and extends the QA race script with the post-commit cleanup scenarios. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ation Record the design rule that prevented the post-commit races: after releasing the home row lock, cleanup must be revision-specific or routed through the state-guarded deletePublicImageIfState(), never an unguarded delete of the shared public path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
DJAscendance
changed the title
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.
Adds the home-image review workflow used by the Block Tools CHECK feature.
Homeowners can upload an image, but it stays private and shows a “NOT CHECKED!” placeholder until a staff member approves it. Rejected images are removed and must be uploaded again.
What changed
Behavior
Verification