diff --git a/backend/__tests__/routes/adminPhotoContentType.test.js b/backend/__tests__/routes/adminPhotoContentType.test.js index 8e0850e8..f38bf836 100644 --- a/backend/__tests__/routes/adminPhotoContentType.test.js +++ b/backend/__tests__/routes/adminPhotoContentType.test.js @@ -147,6 +147,30 @@ describe('admin photo view Content-Type (#908)', () => { expect(res2.headers['content-type']).toBe('video/webm'); }); + it('preserves an auto-imported avif via the safe stored-MIME allowlist', async () => { + // .avif isn't in EXTENSION_TO_MIME; s3AutoImporter stores image/avif. + // Map-only would mislabel it image/jpeg — the allowlist keeps it. + const id = await addPhoto('imported.avif', { mime_type: 'image/avif' }); + const res = await getPhotoRes(id); + expect(res.status).toBe(200); + expect(res.headers['content-type']).toBe('image/avif'); + }); + + it('preserves other importer raster types too (apng, x-icon)', async () => { + const apng = await addPhoto('anim.apng', { mime_type: 'image/apng' }); + expect((await getPhotoRes(apng)).headers['content-type']).toBe('image/apng'); + const ico = await addPhoto('fav.ico', { mime_type: 'image/x-icon' }); + expect((await getPhotoRes(ico)).headers['content-type']).toBe('image/x-icon'); + }); + + it('does NOT honor a stored scriptable image type (image/svg+xml)', async () => { + // svg is inline-scriptable and must never be echoed — allowlist excludes it. + const id = await addPhoto('vector.svg', { mime_type: 'image/svg+xml' }); + const res = await getPhotoRes(id); + expect(res.status).toBe(200); + expect(res.headers['content-type']).toBe('image/jpeg'); + }); + it('never echoes a stored non-media MIME type (inline XSS guard)', async () => { const id = await addPhoto('evil.png', { mime_type: 'text/html' }); const res = await getPhotoRes(id); @@ -175,6 +199,21 @@ describe('admin photo view Content-Type (#908)', () => { expect(res.headers['content-type']).toBe('image/png'); }); + it('handles Object.prototype key extensions without a 500 (.constructor)', async () => { + // The extension-to-MIME lookup must be own-property only — a raw + // index access returns an inherited function for these keys and the + // downstream startsWith throws. Serve image/jpeg instead of 500. + const id = await addPhoto('payload.constructor'); + const res = await getPhotoRes(id); + expect(res.status).toBe(200); + expect(res.headers['content-type']).toBe('image/jpeg'); + + const id2 = await addPhoto('payload.__proto__', { media_type: 'video' }); + const res2 = await getPhotoRes(id2); + expect(res2.status).toBe(200); + expect(res2.headers['content-type']).toBe('video/mp4'); + }); + it('does not synthesize types from unmapped image extensions', async () => { // Raw interpolation would produce image/svg+xml (scriptable inline) // or arbitrary strings from client-controlled filenames — the shared diff --git a/backend/src/routes/adminPhotos.js b/backend/src/routes/adminPhotos.js index 889631b8..8831a47a 100644 --- a/backend/src/routes/adminPhotos.js +++ b/backend/src/routes/adminPhotos.js @@ -1148,7 +1148,13 @@ router.get('/:eventId/photo/:photoId', adminAuth, requirePermission('photos.view // admin player's blob unplayable (#908). const { EXTENSION_TO_MIME } = require('../services/uploadSettings'); const ext = path.extname(photo.filename).slice(1).toLowerCase(); - const extMime = EXTENSION_TO_MIME[ext] || null; + // Own-property lookup (review): a client-controlled filename ending in + // .constructor / .__proto__ / .toString would otherwise return an + // inherited Object.prototype member, and the extMime.startsWith below + // would throw — a permanent 500 for that photo instead of the fallback. + const extMime = Object.prototype.hasOwnProperty.call(EXTENSION_TO_MIME, ext) + ? EXTENSION_TO_MIME[ext] + : null; // Full-token validation, not just a prefix check: the stored value is // client-controlled, and header-invalid characters (video/mp4\r\nX: y) // would make setHeader throw — a permanent 500 for that photo. Bare @@ -1156,18 +1162,34 @@ router.get('/:eventId/photo/:photoId', adminAuth, requirePermission('photos.view const storedVideoMime = photo.mime_type && /^video\/[\w.+-]+$/.test(photo.mime_type) ? photo.mime_type : null; + // Honor a stored image MIME for any header-safe RASTER type (#908 + // review): the S3 auto-importer accepts arbitrary image/* from + // mime-types and stores it (avif/bmp/tiff/heic/apng/ico/jxl/…), and a + // hand-listed allowlist kept missing formats. Allow image/ but + // NEVER the scriptable svg / *+xml family (image/svg+xml executes + // inline). The strict token + anchors also block header injection + // (image/x\r\nY:). Migration 039's blanket image/jpeg backfill on + // legacy rows is why the mapped extension still wins ahead of this. + const storedImageMime = + photo.mime_type && + /^image\/[\w.+-]+$/.test(photo.mime_type) && + !/^image\/svg|xml/i.test(photo.mime_type) + ? photo.mime_type + : null; const isVideo = photo.media_type === 'video' || Boolean(storedVideoMime) || Boolean(extMime && extMime.startsWith('video/')); - // Map-only on the image side too: interpolating the raw extension - // would synthesize image/svg+xml (scriptable inline) or header-invalid - // values from client-controlled chunked-upload filenames. Anything the - // shared map doesn't know is served as image/jpeg — browsers sniff - // image bytes in /blob contexts, so a mislabel is harmless where - // an injected type is not. + // Never interpolate the raw extension on the image side: it would + // synthesize image/svg+xml (scriptable inline) or header-invalid values + // from client-controlled chunked-upload filenames. Precedence is + // mapped-extension (also corrects the 039 legacy-jpeg backfill on PNGs) + // -> safe stored raster MIME (auto-imported avif/bmp/tiff) -> image/jpeg. + // A stored type outside the allowlist degrades to image/jpeg; browsers + // sniff image bytes in /blob contexts, so a mislabel is harmless + // where an injected type is not. const contentType = isVideo ? storedVideoMime || (extMime && extMime.startsWith('video/') ? extMime : null) || 'video/mp4' - : (extMime && extMime.startsWith('image/') ? extMime : null) || 'image/jpeg'; + : (extMime && extMime.startsWith('image/') ? extMime : null) || storedImageMime || 'image/jpeg'; res.setHeader('Content-Type', contentType); res.setHeader('Cache-Control', 'private, max-age=3600'); res.setHeader('Cross-Origin-Resource-Policy', 'cross-origin'); diff --git a/frontend/src/hooks/useExpiryRefresh.ts b/frontend/src/hooks/useExpiryRefresh.ts new file mode 100644 index 00000000..e10e0164 --- /dev/null +++ b/frontend/src/hooks/useExpiryRefresh.ts @@ -0,0 +1,43 @@ +import { useEffect, useState } from 'react'; + +// setTimeout stores its delay in a signed 32-bit int; anything larger +// overflows and fires immediately. Events expiring weeks out don't need a +// live tick anyway, so we simply don't schedule past this horizon. +const MAX_TIMEOUT_MS = 2 ** 31 - 1; + +/** + * Fire `onExpiry` once, at the soonest future timestamp in `timestamps` + * (#909 review). Admin expiry badges are computed inline from Date.now() + * at render time, so without this a page left mounted across an event's + * expiry keeps showing the stale "active"/"1 day left" state until an + * unrelated render happens — which for editor/viewer roles (no health + * poll) may never occur. When the callback updates state/data, the next + * expiry reschedules automatically. + */ +export function useExpiryRefresh( + timestamps: Array, + onExpiry: () => void, +): void { + const next = timestamps + .map((t) => (t ? new Date(t).getTime() : NaN)) + .filter((n) => Number.isFinite(n) && n > Date.now()) + .sort((a, b) => a - b)[0]; + + // Bumped by a capped wake-up so the effect re-evaluates and re-arms when + // the target is further out than a single setTimeout can represent. + const [rearm, setRearm] = useState(0); + useEffect(() => { + if (next === undefined) return; + // +1s so the timer lands just past the boundary, not exactly on it. + const delay = next - Date.now() + 1000; + if (delay > MAX_TIMEOUT_MS) { + // Too far for one timer (setTimeout overflows past ~24.8 days and + // fires immediately). Wake at the cap and re-arm with a now-smaller + // remaining delay, so a page left mounted for weeks still updates. + const id = window.setTimeout(() => setRearm((n) => n + 1), MAX_TIMEOUT_MS); + return () => window.clearTimeout(id); + } + const id = window.setTimeout(onExpiry, Math.max(0, delay)); + return () => window.clearTimeout(id); + }, [next, onExpiry, rearm]); +} diff --git a/frontend/src/pages/admin/AdminDashboard.tsx b/frontend/src/pages/admin/AdminDashboard.tsx index 2240f867..d4ea94fa 100644 --- a/frontend/src/pages/admin/AdminDashboard.tsx +++ b/frontend/src/pages/admin/AdminDashboard.tsx @@ -1,4 +1,4 @@ -import React from 'react'; +import React, { useCallback } from 'react'; import { useNavigate } from 'react-router-dom'; import { Calendar, @@ -16,6 +16,8 @@ import { X } from 'lucide-react'; import { parseISO } from 'date-fns'; +import { useQueryClient } from '@tanstack/react-query'; +import { useExpiryRefresh } from '../../hooks/useExpiryRefresh'; import { useTranslation } from 'react-i18next'; import { useLocalizedDate } from '../../hooks/useLocalizedDate'; import { useMutationWithToast } from '../../hooks'; @@ -76,9 +78,29 @@ export const AdminDashboard: React.FC = () => { // which silently missed any expiring event outside the first 100 rows. const { data: expiringEventsData, isLoading: eventsLoading } = useQuery({ queryKey: ['admin-events-summary', 'expiring'], - queryFn: () => eventsService.getEvents(1, 5, 'expiring'), + // Order by soonest expiry so the five shown rows ARE the earliest to + // expire — useExpiryRefresh then schedules against the true next boundary + // even when >5 events are expiring (#909 review round 3). + queryFn: () => eventsService.getEvents(1, 5, 'expiring', undefined, 'expires_at', 'asc'), }); + // Keep the "expiring soon" card honest when a row crosses its expiry while + // the dashboard sits open (#909 review). Filtering client-side desynced the + // list from the cached total/stat; instead we refetch the whole set at the + // boundary — the backend returns rows/total/stats that already exclude the + // now-expired event, so everything stays consistent. Fixes the stale + // "1 day left" for roles without the health poll (editor/viewer). Placed + // with the other top-level hooks, above the loading early-return. + const queryClient = useQueryClient(); + const refreshExpiring = useCallback(() => { + queryClient.invalidateQueries({ queryKey: ['admin-events-summary', 'expiring'] }); + queryClient.invalidateQueries({ queryKey: ['admin-dashboard-stats'] }); + }, [queryClient]); + useExpiryRefresh( + (expiringEventsData?.events ?? []).map((e: any) => e.expires_at), + refreshExpiring, + ); + // Pending workflow approvals — only when the workflow engine is live. These // are the human-in-the-loop gates (e.g. "review invoice before sending"). const { flags } = useFeatureFlags(); @@ -237,7 +259,9 @@ export const AdminDashboard: React.FC = () => { ) : (
{expiringEvents.map((event) => { - // Ceiling (#909): truncation showed "0 days" on the last day. + // Ceiling so the final partial day reads "1 day", not "0" + // (#909); clamped since a row can sit at the boundary for the + // instant before useExpiryRefresh refetches it away. const daysLeft = Math.max(1, Math.ceil((parseISO(event.expires_at!).getTime() - Date.now()) / 86400000)); return ( diff --git a/frontend/src/pages/admin/EventDetailsPage.tsx b/frontend/src/pages/admin/EventDetailsPage.tsx index 93b59307..540c499f 100644 --- a/frontend/src/pages/admin/EventDetailsPage.tsx +++ b/frontend/src/pages/admin/EventDetailsPage.tsx @@ -1,4 +1,5 @@ -import React, { useState, useEffect, useMemo } from 'react'; +import React, { useState, useEffect, useMemo, useCallback } from 'react'; +import { useExpiryRefresh } from '../../hooks/useExpiryRefresh'; import { useParams, useNavigate } from 'react-router-dom'; import { useTranslation } from 'react-i18next'; import { toast } from 'react-toastify'; @@ -101,6 +102,13 @@ export const EventDetailsPage: React.FC = () => { enabled: !!id, }); + // Flip the expiry banner live when the timestamp passes with the page open + // (#909 review) — isExpired further down is computed inline from Date.now(). + // Kept here with the other hooks, above the loading early-return. + const [, setExpiryTick] = useState(0); + const bumpExpiryTick = useCallback(() => setExpiryTick((n) => n + 1), []); + useExpiryRefresh([event?.expires_at], bumpExpiryTick); + // Fetch feedback settings const { data: eventFeedbackSettings } = useQuery({ queryKey: ['admin-event-feedback-settings', id], diff --git a/frontend/src/pages/admin/EventsListPage.tsx b/frontend/src/pages/admin/EventsListPage.tsx index cd1d3408..b323918d 100644 --- a/frontend/src/pages/admin/EventsListPage.tsx +++ b/frontend/src/pages/admin/EventsListPage.tsx @@ -1,4 +1,5 @@ import React, { useState, useEffect } from 'react'; +import { useExpiryRefresh } from '../../hooks/useExpiryRefresh'; import { useNavigate, useSearchParams } from 'react-router-dom'; import { Plus, @@ -141,7 +142,7 @@ export const EventsListPage: React.FC = () => { // Fetch events — fully server-side: pagination, status filter, and search // (#346 — counters and search were previously bounded to the first 100 rows). - const { data, isLoading, error } = useQuery({ + const { data, isLoading, error, refetch } = useQuery({ queryKey: ['admin-events', statusFilter ?? 'all', debouncedSearchTerm, page], queryFn: () => eventsService.getEvents(page, PAGE_SIZE, statusFilter, debouncedSearchTerm || undefined), placeholderData: (prev) => prev, @@ -230,6 +231,14 @@ export const EventsListPage: React.FC = () => { // Filtering and searching now happen server-side. Use the response directly, // ordered as the backend returned them (created_at desc by default). const events: Event[] = data?.events ?? []; + + // Refetch when the soonest event expiry passes (#909 review): the status + // badge is computed inline from Date.now(), and under the "expiring" filter + // the backend drops the row once expires_at <= now — so a plain re-render + // would leave a stale "Expired" row (and total) in that filtered view. + // refetch() re-runs with the current page/filter/search: rows and totals + // both correct under every filter. + useExpiryRefresh(events.map((e) => e.expires_at), refetch); const pagination = data?.pagination; const totalPages = pagination?.totalPages ?? 1; const filteredCount = pagination?.total ?? 0; diff --git a/frontend/src/services/events.service.ts b/frontend/src/services/events.service.ts index 682b27d6..08ea99d5 100644 --- a/frontend/src/services/events.service.ts +++ b/frontend/src/services/events.service.ts @@ -95,7 +95,9 @@ export const eventsService = { page: number = 1, limit: number = 20, status?: EventStatusFilter, - search?: string + search?: string, + sortBy?: string, + sortOrder?: 'asc' | 'desc' ): Promise { const params = new URLSearchParams({ page: page.toString(), @@ -108,6 +110,12 @@ export const eventsService = { if (search) { params.append('search', search); } + if (sortBy) { + params.append('sortBy', sortBy); + } + if (sortOrder) { + params.append('sortOrder', sortOrder); + } const response = await api.get(`/admin/events?${params}`); const data: any = response.data;