Compare commits

...

17 Commits

Author SHA1 Message Date
Paul Nothaft 0c73bf2cdc chore(stable): release 3.45.12 (#955)
Build and Push Docker Images / build-backend (linux/amd64, ubuntu-latest) (push) Waiting to run
Build and Push Docker Images / build-backend (linux/arm64, ubuntu-24.04-arm) (push) Waiting to run
Build and Push Docker Images / merge-backend (push) Blocked by required conditions
Build and Push Docker Images / build-frontend (linux/amd64, ubuntu-latest) (push) Waiting to run
Build and Push Docker Images / build-frontend (linux/arm64, ubuntu-24.04-arm) (push) Waiting to run
Build and Push Docker Images / merge-frontend (push) Blocked by required conditions
Build and Push Docker Images / summary (push) Blocked by required conditions
2026-08-02 20:25:40 +02:00
Paul Nothaft 2c7b5dfd02 fix(security): redact gallery share tokens from analytics tracking (GHSA-7m6c) (stable) (#953)
* fix(security): redact gallery share tokens from analytics page-view tracking (GHSA-7m6c)

* fix(security): codex round-1 — actually disable raw auto-tracking (GHSA-7m6c)

The previous patch was inert: App.tsx passed autoTrack:true (so Umami's
data-auto-track=false was never set) and the sanitized trackPageView had no
caller (useAnalytics sits outside <Router>), so the raw token URL still hit
the collector.

- Umami: drop autoTrack:true → data-auto-track=false; page views now come
  from a sanitized manual tracker.
- Rybbit: its initial-load auto pageview can't be intercepted client-side, so
  use native data-mask-patterns=['/gallery/**'] to strip the token on every
  auto-tracked view; skip manual tracking for it to avoid double counting.
- Mount <AnalyticsRouteTracker/> INSIDE <Router> so manual tracking runs.

---------

Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>
2026-08-02 08:39:40 +02:00
Paul Nothaft 5d5db4e766 fix(security): authz/ownership gaps (token binding, auth revocation, feedback/customer ownership, token logging) (stable) (#951)
* fix(security): close authz/ownership gaps (secure-download binding, photo-auth+logout revocation, feedback/customer ownership, token logging)

* fix(security): codex round-1 — complete admin-token invalidation + preserve foreign assignments

- photoAuth: mirror adminAuth's active-admin lookup + iat<password_changed_at
  check in the admin branch, so a deactivated admin or a pre-password-change
  token can no longer fetch every photo (GHSA-x55x was only revoke+cutoff).
- adminAuth logout: revoke req.token (the token adminAuth authenticated with,
  cookie OR header) instead of header-only, and clear the auth cookie — a
  cookie-based logout previously left the JWT live (GHSA-cjqh).
- adminCustomers PUT /:id/events: preserve the customer's existing
  assignments to events the caller does NOT own, so a restricted admin can't
  revoke another admin's customer-event links via full-list replacement.

* fix(security): codex round-2 — don't 403 legit restricted-admin assignment edits

The Manage-galleries dialog submits the full initial assignment list, so a
restricted admin editing a customer that already has a foreign assignment hit
the denied.length 403 before the preservation logic ran. Reject only
NEWLY-supplied foreign/nonexistent ids; retain foreign ids the customer is
already assigned to (they can't be added or removed by a non-owner).

---------

Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>
2026-08-02 08:39:32 +02:00
Paul Nothaft e5dccf1664 fix(security): neutralize spreadsheet formulas in all CSV/export cell-writers (CSV injection cluster) (#949)
Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>
2026-08-02 08:38:55 +02:00
Paul Nothaft bfafecedc7 fix(security): unauth share_token leak (HIGH) + restore path-traversal, logo file-read (stable) (#947)
* fix(security): stop unauth share_token leak + block restore path-traversal, logo-path file read, branding path keys

* test: update resolveLogoFile for the c7x5 containment (reject outside-storage absolute paths, keep inside)

* fix(security): codex round-1 — escape LIKE wildcards in share-link resolve, keep in-storage absolute logos, guard restore verification

- shareLinkService: escape %/_ in the link_partial LIKE fallback so an
  anonymous /resolve/____… wildcard can't match an arbitrary share_link and
  leak its bearer token (reopened GHSA-rh8r). Explicit ESCAPE for SQLite.
- resolveLogoFile: re-add the raw absolute candidate but keep it subject to
  the storage-root containment filter (GHSA-c7x5) so legit in-storage
  absolute logos resolve while /etc/passwd stays rejected.
- restoreService: apply the same pathEscapes guard in post-restore
  verification so a skipped traversal entry isn't fs.access'd/hashed.

---------

Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>
2026-08-02 08:38:47 +02:00
Paul Nothaft 2c5a094c5c chore(stable): release 3.45.11 (#936)
Build and Push Docker Images / build-backend (linux/amd64, ubuntu-latest) (push) Waiting to run
Build and Push Docker Images / build-backend (linux/arm64, ubuntu-24.04-arm) (push) Waiting to run
Build and Push Docker Images / merge-backend (push) Blocked by required conditions
Build and Push Docker Images / build-frontend (linux/amd64, ubuntu-latest) (push) Waiting to run
Build and Push Docker Images / build-frontend (linux/arm64, ubuntu-24.04-arm) (push) Waiting to run
Build and Push Docker Images / merge-frontend (push) Blocked by required conditions
Build and Push Docker Images / summary (push) Blocked by required conditions
2026-08-01 17:38:06 +02:00
Paul Nothaft 2462ba6897 fix(security): close authorization/ownership gaps (token scope, mass-assignment, category hero, project docs) (stable) (#944)
* fix(security): close authorization/ownership gaps (token scope, mass-assignment, category hero, project docs)

* fix(security): block archive columns in event mass-assignment per review

* fix(security): comprehensive event mass-assignment denylist + deal-cascade cross-domain permission gate (codex r2)

* fix(security): case-insensitive complete event denylist + project_id + empty-update no-op (codex r3)

---------

Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>
2026-08-01 17:36:57 +02:00
Paul Nothaft 90275f88e9 fix(security): resolve DNS before vetting external hostnames (SSRF cluster) (stable) (#942)
* fix(security): resolve DNS before vetting external hostnames (SSRF cluster)

* fix(security): harden SSRF fix per review (rsync backup path, S3 config-save, webhook transient-DNS retry)

* fix(security): S3 endpoint validation on any endpoint update + no-connect on unresolved webhook host (codex r2)

---------

Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>
2026-08-01 17:36:51 +02:00
Paul Nothaft 34a7b1c013 fix(security): block guest access to hidden/client-only photos across bulk + secure routes (stable) (#940)
* fix(security): block guest access to hidden/client-only photos across bulk + secure routes

* fix(security): harden hidden-photo fix per review (stale ZIP cache, legacy token mint, SQLite bool, client rebuild)

* fix(security): invalidate ZIP cache on photo visibility/category change (codex r2)

* fix(security): recheck photo visibility at signed/secure serve time (TOCTOU) + invalidate ZIP on client visibility change (codex r3)

---------

Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>
2026-08-01 17:36:46 +02:00
Paul Nothaft 7419c68337 fix(security): bump sanitize-html to 2.17.5 (CVE-2026-53606) (stable) (#938)
Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>
2026-08-01 17:36:40 +02:00
Paul Nothaft fc99e2b233 fix(uploads): prevent cross-photo contamination from filename collisions and non-atomic writes (#931) (stable) (#934)
* fix(uploads): prevent cross-photo contamination from filename collisions and non-atomic writes (#931)

* test: pin the suffixed photo filename format in the NFD pipeline suite (#931)

* chore(deps): promote p-limit to a direct dependency for the watermark limiter (#931)

* test: make the suffix-uniqueness check deterministic-in-practice (#931)

* fix(uploads): widen the anti-collision suffix to 48 bits (#931)

* fix(uploads): hide staging files from list() + share one watermark limiter process-wide (#931)

* fix(uploads): reclaim orphaned staging files + revalidate watermark settings in queued jobs (#931)

---------

Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>
2026-08-01 12:29:23 +02:00
Paul Nothaft 7974b9c6d7 chore(stable): release 3.45.10 (#923)
Build and Push Docker Images / build-backend (linux/amd64, ubuntu-latest) (push) Waiting to run
Build and Push Docker Images / build-backend (linux/arm64, ubuntu-24.04-arm) (push) Waiting to run
Build and Push Docker Images / merge-backend (push) Blocked by required conditions
Build and Push Docker Images / build-frontend (linux/amd64, ubuntu-latest) (push) Waiting to run
Build and Push Docker Images / build-frontend (linux/arm64, ubuntu-24.04-arm) (push) Waiting to run
Build and Push Docker Images / merge-frontend (push) Blocked by required conditions
Build and Push Docker Images / summary (push) Blocked by required conditions
2026-07-30 21:20:10 +02:00
Paul Nothaft 60cbda5b22 fix(security): close GHSA-g94x (cross-gallery photo read) + GHSA-pv6w (admin DB export) (stable) (#925)
* fix(security): close two access-control advisories (GHSA-g94x, GHSA-pv6w) (stable)

GHSA-g94x-8vv8-3c9f (HIGH) — the secure-image VIEW route
(/secure-images/:slug/secure/:photoId/:token) validated only the token
signature and took the gallery/photo from the URL, so a token minted on
any PUBLIC gallery read every other gallery's photos with no password
(its download sibling has verifyGalleryAccess; the view route can't —
it serves via <img src> with no header). Bind the token to its scope
instead: the URL photoId must equal the token's minted photoId (photos
belong to exactly one gallery, and minting is gallery-scoped), and the
gallery embedded in the token's sessionId must equal the URL gallery.

GHSA-pv6w-rj34-wj9v (MEDIUM) — GET /admin/backup/picpeak/export dumps
every table unredacted (bcrypt hashes, 2FA, SMTP/SSO/WhatsApp/webhook/S3
secrets) and was gated only by backup.create, which the built-in admin
role holds. Gate it behind super_admin, matching the restore side
(backup.restore, already admin-denied) and the masked config APIs.

Regression tests pin both: cross-gallery token reads 403 (photo and
gallery checks), backup export 403 for admin / passes for super_admin.

Stable port of #924. secureImages on stable has no reveal-mode block, so
only the token-binding checks are added; the backup export gate is
identical.

* fix(security): review follow-ups on the export gate (GHSA-pv6w)

- test: place the mocked export in its own mkdtemp dir. The route
  recursively deletes path.dirname(filePath) after download, so a stub
  in bare os.tmpdir() made the super_admin test wipe the whole temp
  root — other jest workers' DB files included (latent CI flake).
- ui: hide PicpeakExportCard from non-super_admins. The role keeps
  settings.view + backup.create, so after the gate its Download button
  always 403'd with a generic toast; gate the card on role super_admin
  to match the endpoint.

* fix(security): keep the token-mismatch audit values within varchar(20) (GHSA-g94x review)

image_access_logs.access_type is varchar(20) (migration 038), but
'token_gallery_mismatch' is 22 chars — on Postgres the audit write
threw value-too-long and logImageAccess swallowed it, so the security
event went unrecorded (the 403 still fired; log is best-effort). Shorten
to 'photo_mismatch' / 'gallery_mismatch' (14/16).

---------

Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>
2026-07-30 14:24:34 +02:00
Paul Nothaft a27d19b4d1 fix(admin): expose view/download counters in the admin photos list (#895 follow-up) (stable) (#915)
* fix(admin): expose view/download counters in the admin photos list (#895 follow-up) (stable)

st-ivan's re-test after #904: statistics panel and event summary now
agree, but the per-image Engagement column still shows 0. Root cause:
the admin photos LIST endpoint maps rows to an explicit response object
that includes like/comment/rating/favorite counts but never included
view_count or download_count — the grid reads photo.view_count ?? 0,
so the column showed 0 regardless of what the DB counted. This mapper,
not stale data, is also why per-image downloads always displayed 0 in
the original report.

Suite extended with a list-endpoint assertion (beacon + download, then
the admin list reflects 1/1 and untouched photos 0/0). The skip test now
neutralizes the route's background pre-zip build, whose async ENOENT
against the intentionally missing file could land mid-suite.

Includes the one-line chunkedUploadService unref from #911 so the test
suite can mount adminPhotos regardless of merge order (identical change,
merges cleanly either way).

* test: widen the fire-and-forget settle window (#895 follow-up)

The 100ms settle was marginal on loaded CI runners — the counter
increments are deliberately fire-and-forget, and the 909 PRs flaked on
exactly these assertions. 400ms keeps the suite fast while giving slow
runners room.

---------

Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>
2026-07-30 12:15:38 +02:00
Paul Nothaft d68d84e5c8 fix(admin): serve videos with their real MIME type in the admin photo view (#908) (stable) (#911)
* fix(admin): serve videos with their real MIME type in the admin photo view (#908) (stable)

The admin view route built Content-Type from the filename extension —
image/<ext> — which is invalid for videos (image/mp4). The admin player
fetches this URL into a blob that inherits the type, and browsers
refuse to play a <video> blob labeled image/*: blank/grey preview,
while download (which already uses photo.mime_type) worked fine.

Stored mime_type now wins; videos without one fall back to video/mp4,
images to the extension, and extensionless files to image/jpeg instead
of the equally invalid bare 'image/'.

Also unrefs chunkedUploadService's module-level hourly cleanup interval:
it kept Jest from exiting for any suite requiring adminPhotos (it's why
adminPhotos.reference sits on the CI ignore list). Production behavior
unchanged — the HTTP listener keeps the process alive.

New adminPhotoContentType suite pins all four MIME cases.

* fix(admin): harden admin photo Content-Type resolution (#908 review round)

External review findings, all verified:

- The header is now ALWAYS image/* or video/*. photos.mime_type is
  never echoed verbatim unless it is a video/ type — the chunked-upload
  path stores the client-sent MIME unvalidated, so a stored text/html
  served inline under the app origin was a same-origin XSS hazard.
- MIME-less videos map from the extension via the shared
  EXTENSION_TO_MIME (.mov → video/quicktime, .webm → video/webm)
  instead of a blanket video/mp4 that would mislabel them.
- Images ignore the stored MIME entirely: migration 039 backfilled
  image/jpeg onto every legacy row (PNGs included), so trusting it
  would regress previously-correct extension-derived types. Extension
  wins, normalized (jpg → image/jpeg).

Suite extended to 8 MIME cases including the XSS guard and the
039-backfill immunity.

* fix(admin): validate stored video MIME as a full header-safe token (#908 review round 2)

A prefix check let malformed client-stored values through:
'video/mp4\r\nX: y' makes res.setHeader throw ERR_INVALID_CHAR — a
permanent 500 for that photo — and a bare 'video/' is an invalid type.
Strict /^video\/[\w.+-]+$/ now gates the stored value; anything else
falls back to the extension map. Two new tests pin both shapes.

* fix(admin): map-only image Content-Type — no raw extension interpolation (#908 review round 3)

image/${ext} could synthesize image/svg+xml (scriptable when served
inline) or header-invalid values from client-controlled chunked-upload
filenames. The shared EXTENSION_TO_MIME map is now the allowlist on the
image side too; unmapped extensions serve as image/jpeg — browsers
sniff image bytes in img/blob contexts, so a mislabel is harmless where
an injected type is not.

* fix(admin): own-property lookup in the extension MIME map (#908 review round)

A client-controlled filename ending in .constructor / .__proto__ /
.toString made EXTENSION_TO_MIME[ext] return an inherited Object.prototype
member (truthy), and the downstream extMime.startsWith threw —
a permanent 500 on the admin view for that photo instead of the JPEG /
mp4 fallback. hasOwnProperty-gated now; test pins both a .constructor
image and a .__proto__ video.

* fix(admin): honor safe stored image MIME for auto-imported formats (#908 review round 2)

My previous round made the image side map-only to dodge the migration
039 image/jpeg backfill and image/svg+xml — but that regressed the S3
auto-importer (STORAGE_AUTO_IMPORT), which stores correct types for
avif/bmp/tiff/heic whose extensions aren't in EXTENSION_TO_MIME. Those
now served as image/jpeg (JPEG-labelled non-JPEG bytes).

Precedence is now mapped-extension (still corrects the 039 backfill on
PNGs) -> stored MIME IF in a safe raster allowlist (avif/bmp/tiff/heic
+ the mapped ones) -> image/jpeg. Allowlist, not a regex: image/svg+xml
stays excluded (scriptable inline). Tests pin avif preserved and svg
degraded to jpeg.

* fix(admin): allow any header-safe raster MIME, deny svg/xml (#908 review round 3)

The round-2 hand-listed Set kept missing formats the S3 auto-importer
stores (apng/ico/jxl beyond avif/bmp/tiff). Replace it with a regex:
honor image/<token> EXCEPT the scriptable svg / *+xml family. Covers
every current and future raster type in one rule while still blocking
inline-scriptable svg and header injection. Tests pin apng + x-icon
preserved, svg still degraded to jpeg.

---------

Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>
2026-07-30 12:14:58 +02:00
Paul Nothaft 6891769124 fix(admin): stop marking events expired up to 24h early (#909) (stable) (#917)
* fix(admin): stop marking events expired up to 24h early (#909) (stable)

differenceInDays truncates to whole days, so an event expiring in a few
hours returned 0 and three admin surfaces treated it as gone:

- EventsListPage: status chip said 'Expired' (days <= 0) while the
  public gallery — which compares real timestamps — correctly showed
  'expires in X hours'. This is the reporter's exact symptom.
- EventDetailsPage: same isExpired math on the detail view.
- AdminDashboard: the expiring-soon card showed '0 days left' on the
  final day.

Expired is now gated on the actual timestamp (expires_at <= now), and
the countdown chips use ceiling days so the last day reads '1 day
left' instead of flipping to Expired/0.

* fix(admin): drop already-expired events from the dashboard card (#909 review round)

The expiring-soon card ran Math.max(1, ceil(delta)), so an event that
expired while the dashboard sat open (its query isn't polled) showed
'1 day left' indefinitely from the stale cached row. Expired rows are
now filtered out before render; the delta is therefore always positive
and the clamp is gone.

* fix(admin): refresh expiry status live at the boundary (#909 review round 2)

Two review findings on the admin expiry surfaces:

- The dashboard 'expiring soon' card, list badges, and detail banner are
  all computed inline from Date.now() at render, so a page left open
  across an event's expiry kept showing 'active'/'1 day left' until an
  unrelated render — which for editor/viewer roles (no health poll)
  never happens.
- My round-1 client-side filter on the dashboard desynced the visible
  list from the cached total/stat ('no events expiring' beside 'view
  all N').

Both are fixed by new useExpiryRefresh: it fires once at the soonest
future expiry (setTimeout, overflow-guarded). The dashboard refetches
its expiring + stats queries — the backend already excludes expired
events, so rows/total/stats come back consistent (filter removed). The
list and detail pages bump a tick so the inline badges recompute. Hooks
are placed above the loading early-returns (rules-of-hooks is disabled
in eslint, so this was a latent crash otherwise).

* fix(admin): expiry-refresh precision + filtered refetch (#909 review round 3)

Three refinements to round-2's live-expiry work:

- useExpiryRefresh now re-arms past setTimeout's ~24.8-day overflow
  limit (capped wake-up that re-evaluates) instead of dropping the timer,
  so a page mounted for weeks still updates.
- The dashboard requests the expiring list ordered by expires_at asc, so
  the five shown rows ARE the soonest to expire — the timer schedules
  against the true next boundary even when >5 events are expiring
  (getEvents gains optional sortBy/sortOrder; backend already whitelists
  expires_at).
- EventsListPage refetches instead of only re-rendering at the boundary:
  under the 'expiring' filter the backend drops expired rows, so a plain
  tick would leave a stale 'Expired' row + total. refetch keeps rows and
  totals correct under every filter.

---------

Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>
2026-07-30 12:14:54 +02:00
Paul Nothaft b32ba1ed6b ci: batch stable releases into one daily version (stable) (#920)
* ci: batch stable releases into one daily version (stable)

The stable release PR was auto-merged the instant it went green, so a
day with N bugfixes produced N patch releases (3.45.8 AND 3.45.9 on
2026-07-29 alone) — N upgrade notifications for stable users and N
full Docker build cycles.

Fixes now accumulate in release-please's rolling release PR and are
cut as ONE version per day by release-stable-daily.yml (18:00 UTC).
Approval/merge mechanics are unchanged from the inline step (#719):
approve as github-actions[bot], auto-merge as the PAT so the merge
triggers the tag-cutting run.

- Urgent fix? workflow_dispatch the daily job or merge the release PR
  by hand — the schedule is a default, not a gate.
- Beta is untouched: instant beta releases are load-bearing for
  same-day reporter verification.
- schedule only fires from the default branch; the stable copy of the
  new workflow is inert and exists to keep branches in sync.

* ci: harden the daily stable-release cut (review round) (stable)

Mirror of the #919 hardening — fork-PR head-name spoof (require
--base stable + same-repo head) and no longer swallowing the
auto-merge-enable failure on the sole automatic stable cut.

* ci: accept an immediately-merged release PR as success (review round 2) (stable)

Mirror of #919: MERGED state = success (the normal 18:00 case where
checks were already green and --auto merges immediately), pending
auto-merge = success, still-open-no-auto-merge = real failure.

* ci: read release-PR state + auto-merge in one snapshot (review round 3) (stable)

Mirror of #919 — collapse the two racing gh pr view calls into one.

---------

Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>
2026-07-30 12:14:31 +02:00
65 changed files with 2801 additions and 255 deletions
+6 -26
View File
@@ -27,32 +27,12 @@ jobs:
manifest-file: .release-please-manifest.json
target-branch: stable
# Auto-approve + auto-merge the open stable release PR. See the beta
# workflow for the full rationale. Skipped on the release-cutting run and
# whenever no PAT is configured.
- name: Auto-approve and enable auto-merge on the release PR
if: ${{ steps.release.outputs.release_created != 'true' }}
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
RELEASE_PAT: ${{ secrets.RELEASE_PLEASE_TOKEN }}
# No checkout in this job — set the repo explicitly so gh works
# without a git remote (same pattern as whatsnew, 2a5f0a8).
GH_REPO: ${{ github.repository }}
run: |
if [ -z "$RELEASE_PAT" ]; then
echo "RELEASE_PLEASE_TOKEN not set — skipping auto-merge (manual review still required)."
exit 0
fi
pr=$(gh pr list --head release-please--branches--stable --state open --json number --jq '.[0].number // empty')
if [ -n "$pr" ]; then
# Approve as github-actions[bot] (GITHUB_TOKEN, ≠ the PAT author) so it
# is a valid review; enable auto-merge as the PAT so the merge commit is
# attributed to a real identity and triggers the tag-cutting run (#719).
gh pr review "$pr" --approve --body "Automated approval — release-please version bump + changelog (#719)." || true
GH_TOKEN="$RELEASE_PAT" gh pr merge "$pr" --squash --auto || true
else
echo "No open release PR to auto-merge."
fi
# NOTE: stable release PRs are intentionally NOT auto-merged here
# anymore. Fixes accumulate in the rolling release PR and are cut as
# ONE patch version per day by release-stable-daily.yml (18:00 UTC,
# or on demand via workflow_dispatch / a manual merge of the release
# PR). Beta keeps instant releases — see release-please-beta.yml —
# because same-day reporter verification depends on it.
- name: Output Release Info
if: ${{ steps.release.outputs.release_created }}
@@ -0,0 +1,86 @@
name: Cut Stable Release (daily batch)
# Stable fixes accumulate in release-please's rolling release PR instead of
# each cutting its own patch version (the old per-merge auto-merge produced
# e.g. 3.45.8 AND 3.45.9 on the same day). This workflow merges the open
# stable release PR once a day, so a day of N bugfixes ships as ONE version
# with all N changelog entries — and one Docker build instead of N.
#
# - schedule only fires from the default branch (main); the stable copy of
# this file is inert and exists to keep the branches in sync.
# - Need a release NOW? Run this via workflow_dispatch, or merge the
# release PR by hand — the schedule is a default, not a gate.
# - Approval/merge mechanics mirror the old inline step (#719): approve as
# github-actions[bot] (GITHUB_TOKEN, a valid distinct reviewer), enable
# auto-merge as the PAT so the merge attributes to a real identity and
# triggers the tag-cutting run. --auto waits for green checks.
on:
schedule:
- cron: '0 18 * * *'
workflow_dispatch:
permissions:
contents: write
pull-requests: write
jobs:
merge-stable-release-pr:
runs-on: ubuntu-latest
steps:
- name: Approve and enable auto-merge on the open stable release PR
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
RELEASE_PAT: ${{ secrets.RELEASE_PLEASE_TOKEN }}
# No checkout — set the repo explicitly so gh works without a
# git remote (same pattern as whatsnew, 2a5f0a8).
GH_REPO: ${{ github.repository }}
run: |
if [ -z "$RELEASE_PAT" ]; then
echo "RELEASE_PLEASE_TOKEN not set — skipping (manual review required)."
exit 0
fi
# Strict selection (review P1): this job runs daily even without a
# stable push, and `gh pr list --head` matches the branch NAME only
# — a fork PR can spoof `release-please--branches--stable`. Pin the
# base to stable AND require a same-repo head (isCrossRepository
# == false); a fork PR is cross-repository, so it can never be
# picked and auto-merged with the privileged PAT.
pr=$(gh pr list \
--base stable \
--head release-please--branches--stable \
--state open \
--json number,isCrossRepository \
--jq '[.[] | select(.isCrossRepository == false)] | .[0].number // empty')
if [ -z "$pr" ]; then
echo "No open same-repo stable release PR — nothing to cut today."
exit 0
fi
# Approve is tolerant — a pre-existing approval already satisfies
# branch protection and re-approving can return non-zero.
gh pr review "$pr" --approve --body "Automated approval — daily stable release batch (release-please version bump + changelog)." || echo "::warning::approve returned non-zero (PR may already be approved)"
# But the auto-merge enable is the load-bearing step: this scheduled
# job is the ONLY automatic stable cut, so DON'T swallow its failure
# (review P2) — an expired/under-scoped PAT would otherwise stop
# releases while the workflow stays green.
GH_TOKEN="$RELEASE_PAT" gh pr merge "$pr" --squash --auto
# `gh pr merge --auto` merges IMMEDIATELY when the required checks
# are already green — the normal case at 18:00, since the fixes
# merged hours earlier and CI passed. So success is EITHER the PR is
# already merged OR an auto-merge request is now pending; only a PR
# that is still open with no auto-merge request is a real failure
# (expired/under-scoped PAT) worth failing the job on (review round 2).
# One snapshot of both fields (review round 3): querying state and
# autoMergeRequest separately races — auto-merge can complete
# between the two calls, so the first sees OPEN and the second sees
# the request already cleared on the now-merged PR → false failure.
read -r state automerge < <(gh pr view "$pr" --json state,autoMergeRequest \
--jq '[.state, (.autoMergeRequest != null)] | @tsv')
if [ "$state" = "MERGED" ]; then
echo "Stable release PR #$pr merged immediately (checks were already green)."
elif [ "$automerge" = "true" ]; then
echo "Auto-merge enabled on stable release PR #$pr — merges when checks are green."
else
echo "::error::stable release PR #$pr is still open with no auto-merge — check RELEASE_PLEASE_TOKEN scope/expiry."
exit 1
fi
+1 -1
View File
@@ -1 +1 @@
{".":"3.45.9"}
{".":"3.45.12"}
+31
View File
@@ -5,6 +5,37 @@ All notable changes to PicPeak will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
## [3.45.12](https://github.com/PicPeak/picpeak/compare/v3.45.11...v3.45.12) (2026-08-02)
### Bug Fixes
* **security:** authz/ownership gaps (token binding, auth revocation, feedback/customer ownership, token logging) (stable) ([#951](https://github.com/PicPeak/picpeak/issues/951)) ([5d5db4e](https://github.com/PicPeak/picpeak/commit/5d5db4e766eee23a6678b399cdb9cc449fcea198))
* **security:** neutralize spreadsheet formulas in all CSV/export cell-writers (CSV injection cluster) ([#949](https://github.com/PicPeak/picpeak/issues/949)) ([e5dccf1](https://github.com/PicPeak/picpeak/commit/e5dccf166419bb571b052990aada14801e16ac79))
* **security:** redact gallery share tokens from analytics tracking (GHSA-7m6c) (stable) ([#953](https://github.com/PicPeak/picpeak/issues/953)) ([2c7b5df](https://github.com/PicPeak/picpeak/commit/2c7b5dfd020ac1fb2acdb7667998b2f373ce99d9))
* **security:** unauth share_token leak (HIGH) + restore path-traversal, logo file-read (stable) ([#947](https://github.com/PicPeak/picpeak/issues/947)) ([bfafece](https://github.com/PicPeak/picpeak/commit/bfafecedc755790374565281287b9777ae0f5315))
## [3.45.11](https://github.com/PicPeak/picpeak/compare/v3.45.10...v3.45.11) (2026-08-01)
### Bug Fixes
* **security:** block guest access to hidden/client-only photos across bulk + secure routes (stable) ([#940](https://github.com/PicPeak/picpeak/issues/940)) ([34a7b1c](https://github.com/PicPeak/picpeak/commit/34a7b1c0137cf4cba53f17270f619ce9755c4c98))
* **security:** bump sanitize-html to 2.17.5 (CVE-2026-53606) (stable) ([#938](https://github.com/PicPeak/picpeak/issues/938)) ([7419c68](https://github.com/PicPeak/picpeak/commit/7419c683375d12650c448caa47ffe0444a7e1458))
* **security:** close authorization/ownership gaps (token scope, mass-assignment, category hero, project docs) (stable) ([#944](https://github.com/PicPeak/picpeak/issues/944)) ([2462ba6](https://github.com/PicPeak/picpeak/commit/2462ba6897c93b3f0834d60cc2e6827a45487062))
* **security:** resolve DNS before vetting external hostnames (SSRF cluster) (stable) ([#942](https://github.com/PicPeak/picpeak/issues/942)) ([90275f8](https://github.com/PicPeak/picpeak/commit/90275f88e9af523ba3cc254cef2ca6f48a2a4129))
* **uploads:** prevent cross-photo contamination from filename collisions and non-atomic writes ([#931](https://github.com/PicPeak/picpeak/issues/931)) (stable) ([#934](https://github.com/PicPeak/picpeak/issues/934)) ([fc99e2b](https://github.com/PicPeak/picpeak/commit/fc99e2b233b4a7c81c410f7d557e7dff270437bd))
## [3.45.10](https://github.com/PicPeak/picpeak/compare/v3.45.9...v3.45.10) (2026-07-30)
### Bug Fixes
* **admin:** expose view/download counters in the admin photos list ([#895](https://github.com/PicPeak/picpeak/issues/895) follow-up) (stable) ([#915](https://github.com/PicPeak/picpeak/issues/915)) ([a27d19b](https://github.com/PicPeak/picpeak/commit/a27d19b4d147c33166b47a6bac12b8d1be32daf1))
* **admin:** serve videos with their real MIME type in the admin photo view ([#908](https://github.com/PicPeak/picpeak/issues/908)) (stable) ([#911](https://github.com/PicPeak/picpeak/issues/911)) ([d68d84e](https://github.com/PicPeak/picpeak/commit/d68d84e5c8cfcd123c95e474d2b87d153764f710))
* **admin:** stop marking events expired up to 24h early ([#909](https://github.com/PicPeak/picpeak/issues/909)) (stable) ([#917](https://github.com/PicPeak/picpeak/issues/917)) ([6891769](https://github.com/PicPeak/picpeak/commit/6891769124f77d5af1bbe8eb0932a86753d89e7d))
* **security:** close GHSA-g94x (cross-gallery photo read) + GHSA-pv6w (admin DB export) (stable) ([#925](https://github.com/PicPeak/picpeak/issues/925)) ([60cbda5](https://github.com/PicPeak/picpeak/commit/60cbda5b2228e0bddf5356eae279d9e7916e9ac2))
## [3.45.9](https://github.com/PicPeak/picpeak/compare/v3.45.8...v3.45.9) (2026-07-29)
@@ -0,0 +1,238 @@
/**
* Admin photo view route Content-Type (#908).
*
* The route built `image/<ext>` from the filename, producing invalid
* types like image/mp4 for videos. AdminAuthenticatedVideo fetches this
* URL into a blob whose type inherits the header, and browsers refuse to
* play a <video> blob labeled image/* — blank/grey admin video preview.
*
* Pins (incl. external-review hardening):
* - the header is ALWAYS image/* or video/*: a stored non-media MIME
* (chunked uploads store the client-sent type unvalidated) is never
* echoed — text/html inline under the app origin would be XSS
* - stored video/ MIME wins; MIME-less videos map from the extension
* (.mov → video/quicktime), unknown video extensions get video/mp4
* - images IGNORE the stored MIME (migration 039 backfilled image/jpeg
* onto every legacy row, PNGs included) and use the extension,
* normalized (jpg → image/jpeg); extensionless files get image/jpeg
*/
const path = require('path');
const fs = require('fs');
const os = require('os');
process.env.NODE_ENV = 'test';
process.env.TEST_DATABASE_PATH = path.join(
fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-admin-ct-')), 'db.sqlite',
);
process.env.JWT_SECRET = process.env.JWT_SECRET || 'admin-ct-test-secret';
process.env.STORAGE_PATH = fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-admin-ct-storage-'));
const request = require('supertest');
const express = require('express');
const bcrypt = require('bcrypt');
const jwt = require('jsonwebtoken');
const { bootCrmDb, seedMinimal } = require('../integration/helpers/crmDb');
const SLUG = 'admin-ct-test-event';
describe('admin photo view Content-Type (#908)', () => {
let db;
let cleanup;
let app;
let eventId;
let adminToken;
const addPhoto = async (filename, extra = {}) => {
const dir = path.join(process.env.STORAGE_PATH, 'events/active', SLUG);
fs.mkdirSync(dir, { recursive: true });
fs.writeFileSync(path.join(dir, filename), Buffer.from(`bytes-${filename}`));
const r = await db('photos').insert({
event_id: eventId,
filename,
path: `${SLUG}/${filename}`,
type: 'individual',
uploaded_at: new Date().toISOString(),
...extra,
}).returning('id');
return r[0]?.id ?? r[0];
};
const getPhotoRes = (photoId) => request(app)
.get(`/api/admin/photos/${eventId}/photo/${photoId}`)
.set('Authorization', `Bearer ${adminToken}`);
beforeAll(async () => {
({ db, cleanup } = await bootCrmDb());
await seedMinimal(db);
const inserted = await db('events').insert({
slug: SLUG,
event_type: 'wedding',
event_name: 'Admin CT Test',
event_date: '2026-08-01',
host_email: 'host@example.com',
admin_email: 'admin@example.com',
password_hash: 'x',
share_link: `/gallery/${SLUG}/share`,
share_token: 'admin-ct-share',
expires_at: new Date(Date.now() + 7 * 24 * 3600 * 1000).toISOString(),
is_active: 1,
is_archived: 0,
is_draft: 0,
created_at: new Date().toISOString(),
}).returning('id');
eventId = inserted[0]?.id ?? inserted[0];
const superRole = await db('roles').where({ name: 'super_admin' }).first();
const [rootId] = await db('admin_users').insert({
username: 'admin-ct-admin',
email: 'admin-ct-admin@example.com',
password_hash: await bcrypt.hash('AdminCt123', 4),
role_id: superRole.id,
is_active: 1,
created_at: new Date(),
updated_at: new Date(),
}).returning('id').then((r) => [r[0]?.id || r[0]]);
adminToken = jwt.sign(
{ id: rootId, username: 'admin-ct-admin', type: 'admin', role: 'super_admin', loginTime: Date.now() },
process.env.JWT_SECRET,
{ expiresIn: '1h', issuer: 'picpeak-auth' }
);
app = express();
app.use(express.json());
app.use('/api/admin/photos', require('../../src/routes/adminPhotos'));
}, 120000);
afterAll(async () => { if (cleanup) await cleanup(); });
it('serves a video with its stored mime_type, not image/<ext>', async () => {
const id = await addPhoto('clip.mp4', { media_type: 'video', mime_type: 'video/mp4' });
const res = await getPhotoRes(id);
expect(res.status).toBe(200);
expect(res.headers['content-type']).toBe('video/mp4');
});
it('maps MIME-less videos from their extension (.mov → video/quicktime)', async () => {
const id = await addPhoto('clip-nomime.mov', { media_type: 'video' });
const res = await getPhotoRes(id);
expect(res.status).toBe(200);
expect(res.headers['content-type']).toBe('video/quicktime');
});
it('falls back to video/mp4 for a video with an unknown extension', async () => {
const id = await addPhoto('clip-unknown.xyz', { media_type: 'video' });
const res = await getPhotoRes(id);
expect(res.status).toBe(200);
expect(res.headers['content-type']).toBe('video/mp4');
});
it('rejects malformed video/ MIME values that would break setHeader', async () => {
// Header-invalid chars in the stored value must not 500 the route —
// fall back to the extension map instead.
const id = await addPhoto('crlf.mp4', {
media_type: 'video',
mime_type: 'video/mp4\r\nX-Evil: 1',
});
const res = await getPhotoRes(id);
expect(res.status).toBe(200);
expect(res.headers['content-type']).toBe('video/mp4');
expect(res.headers['x-evil']).toBeUndefined();
const bare = await addPhoto('bare.webm', { media_type: 'video', mime_type: 'video/' });
const res2 = await getPhotoRes(bare);
expect(res2.status).toBe(200);
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);
expect(res.status).toBe(200);
expect(res.headers['content-type']).toBe('image/png');
});
it('ignores the migration-039 image/jpeg backfill on legacy PNG rows', async () => {
const id = await addPhoto('legacy.png', { mime_type: 'image/jpeg' });
const res = await getPhotoRes(id);
expect(res.status).toBe(200);
expect(res.headers['content-type']).toBe('image/png');
});
it('normalizes jpg to the canonical image/jpeg', async () => {
const id = await addPhoto('shot.jpg');
const res = await getPhotoRes(id);
expect(res.status).toBe(200);
expect(res.headers['content-type']).toBe('image/jpeg');
});
it('keeps the extension fallback for images without a stored mime_type', async () => {
const id = await addPhoto('shot.png');
const res = await getPhotoRes(id);
expect(res.status).toBe(200);
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
// map is the allowlist, everything else is served as image/jpeg.
const svg = await addPhoto('vector.svg+xml');
const res = await getPhotoRes(svg);
expect(res.status).toBe(200);
expect(res.headers['content-type']).toBe('image/jpeg');
const weird = await addPhoto('weird.xyz');
const res2 = await getPhotoRes(weird);
expect(res2.status).toBe(200);
expect(res2.headers['content-type']).toBe('image/jpeg');
});
it('extensionless files get image/jpeg, never a bare image/', async () => {
const id = await addPhoto('noext');
const res = await getPhotoRes(id);
expect(res.status).toBe(200);
expect(res.headers['content-type']).toBe('image/jpeg');
});
});
@@ -0,0 +1,211 @@
/**
* Authorization / ownership gaps (GHSA permission cluster):
* - jm7j: API-token list must scope to the caller (non-super sees only own)
* - gprq: API-token revoke must be owner-or-super_admin
* - 3rqx: event update must not mass-assign identity/secret columns
* - j2f4: category hero must belong to that category
*/
const path = require('path');
const fs = require('fs');
const os = require('os');
process.env.NODE_ENV = 'test';
process.env.TEST_DATABASE_PATH = path.join(
fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-authz-')), 'db.sqlite',
);
process.env.JWT_SECRET = process.env.JWT_SECRET || 'authz-test-secret';
process.env.STORAGE_PATH = fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-authz-storage-'));
const request = require('supertest');
const express = require('express');
const cookieParser = require('cookie-parser');
const bcrypt = require('bcrypt');
const {
bootCrmDb, seedMinimal, assignAdminRole, mintAdminToken,
} = require('../integration/helpers/crmDb');
describe('authorization / ownership gaps', () => {
let db; let cleanup; let app;
let superId; let superTok; let adminId; let adminTok;
const grantPermissionToRole = async (roleName, permName) => {
const role = await db('roles').where({ name: roleName }).first();
const perm = await db('permissions').where({ name: permName }).first();
const exists = await db('role_permissions')
.where({ role_id: role.id, permission_id: perm.id }).first();
if (!exists) {
await db('role_permissions').insert({ role_id: role.id, permission_id: perm.id });
}
};
beforeAll(async () => {
({ db, cleanup } = await bootCrmDb());
({ adminId: superId } = await seedMinimal(db));
await assignAdminRole(db, superId, 'super_admin');
superTok = mintAdminToken(superId);
const pass = await bcrypt.hash('x', 4);
const ins = await db('admin_users').insert({
username: 'plain-admin', email: 'plain@example.com',
password_hash: pass, must_change_password: false, created_at: new Date(),
}).returning('id');
adminId = ins[0]?.id ?? ins[0];
await assignAdminRole(db, adminId, 'admin');
// Grant settings.edit to the admin role BEFORE any request populates the
// 60s permission cache, so the revoke test exercises the ownership check
// (404) rather than the missing-permission gate (403). This models a
// custom role that carries settings.edit — the scenario GHSA-gprq needs.
await grantPermissionToRole('admin', 'settings.edit');
adminTok = mintAdminToken(adminId);
app = express();
app.use(express.json());
app.use(cookieParser());
app.use('/api/admin/api-tokens', require('../../src/routes/adminApiTokens'));
app.use('/api/admin/events', require('../../src/routes/adminEvents'));
app.use('/api/admin/categories', require('../../src/routes/adminCategories'));
// eslint-disable-next-line no-unused-vars
app.use((err, req, res, next) => {
res.status(err.statusCode || err.status || 500).json({ error: err.message, code: err.code });
});
}, 120000);
afterAll(async () => { if (cleanup) await cleanup(); });
const auth = (req, tok) => req.set('Authorization', `Bearer ${tok}`);
describe('API tokens (jm7j / gprq)', () => {
let superTokenId;
beforeAll(async () => {
const res = await auth(request(app).post('/api/admin/api-tokens'), superTok)
.send({ name: 'super-token', scopes: ['read'] });
expect(res.status).toBe(201);
superTokenId = res.body.id;
});
it('non-super admin does not see another admin\'s tokens in the list', async () => {
const res = await auth(request(app).get('/api/admin/api-tokens'), adminTok);
expect(res.status).toBe(200);
expect(res.body.find((t) => t.id === superTokenId)).toBeUndefined();
});
it('super_admin sees all tokens', async () => {
const res = await auth(request(app).get('/api/admin/api-tokens'), superTok);
expect(res.status).toBe(200);
expect(res.body.find((t) => t.id === superTokenId)).toBeDefined();
});
it('a non-owner (with settings.edit) cannot revoke another admin\'s token', async () => {
const res = await auth(request(app).delete(`/api/admin/api-tokens/${superTokenId}`), adminTok);
expect(res.status).toBe(404);
const row = await db('api_tokens').where({ id: superTokenId }).first();
expect(row.revoked_at).toBeFalsy();
});
it('the owner can revoke their own token', async () => {
const res = await auth(request(app).delete(`/api/admin/api-tokens/${superTokenId}`), superTok);
expect(res.status).toBe(200);
});
});
describe('event update mass-assignment (3rqx)', () => {
it('ignores identity/secret columns in the request body', async () => {
const seedShareToken = 'orig-share-token';
const ins = await db('events').insert({
slug: 'authz-mass-assign', event_type: 'wedding', event_name: 'Before',
event_date: '2026-08-01', host_email: 'h@example.com', admin_email: 'a@example.com',
password_hash: 'orig-hash', share_link: '/gallery/authz/share', share_token: seedShareToken, expires_at: new Date(Date.now() + 7 * 864e5).toISOString(),
is_active: 1, is_archived: 0, is_draft: 0, created_by: superId,
created_at: new Date().toISOString(),
}).returning('id');
const eventId = ins[0]?.id ?? ins[0];
const res = await auth(request(app).put(`/api/admin/events/${eventId}`), superTok).send({
event_name: 'After',
created_by: 99999,
slug: 'hijacked-slug',
share_token: 'hijacked-token',
password_hash: 'hijacked-hash',
is_archived: 1,
archive_path: '/hijacked/archive/path',
hero_logo_path: '/etc/passwd',
is_draft: 1,
project_id: 99999,
// Case-variant keys — SQLite matches columns case-insensitively.
Password_Hash: 'case-hijack-hash',
Created_By: 88888,
});
expect(res.status).toBe(200);
const row = await db('events').where({ id: eventId }).first();
expect(row.event_name).toBe('After'); // legit field applied
expect(row.created_by).toBe(superId); // ownership untouched (+ case-variant)
expect(row.slug).toBe('authz-mass-assign'); // routing identity untouched
expect(row.share_token).toBe(seedShareToken); // secret untouched
expect(row.password_hash).toBe('orig-hash'); // secret untouched (+ case-variant)
expect(row.is_archived).toBeFalsy(); // archive lifecycle untouched
expect(row.archive_path).toBeFalsy(); // forged archive path rejected
expect(row.hero_logo_path).toBeFalsy(); // fs.unlink primitive blocked
expect(row.is_draft).toBeFalsy(); // publish workflow not bypassed
expect(row.project_id).toBeFalsy(); // server-managed relationship untouched
});
it('returns 200 (no-op) when the body contains only protected fields', async () => {
const ins = await db('events').insert({
slug: 'authz-empty-update', event_type: 'wedding', event_name: 'Keep',
event_date: '2026-08-01', host_email: 'h@example.com', admin_email: 'a@example.com',
password_hash: 'x', share_link: '/gallery/authz-empty/share', share_token: 'authz-empty-share',
expires_at: new Date(Date.now() + 7 * 864e5).toISOString(),
is_active: 1, is_archived: 0, is_draft: 0, created_by: superId,
created_at: new Date().toISOString(),
}).returning('id');
const id = ins[0]?.id ?? ins[0];
// Body reduces to {} after the denylist — must not 500 (Knex rejects
// .update({})).
const res = await auth(request(app).put(`/api/admin/events/${id}`), superTok)
.send({ created_by: 1, slug: 'x', is_archived: 1 });
expect(res.status).toBe(200);
const row = await db('events').where({ id }).first();
expect(row.event_name).toBe('Keep');
});
});
describe('category hero cross-category (j2f4)', () => {
it('rejects a hero photo that is not in the category', async () => {
const evIns = await db('events').insert({
slug: 'authz-cat', event_type: 'wedding', event_name: 'Cat Event',
event_date: '2026-08-01', host_email: 'h@example.com', admin_email: 'a@example.com',
password_hash: 'x', share_link: '/gallery/authz-cat/share', share_token: 'authz-cat-share', expires_at: new Date(Date.now() + 7 * 864e5).toISOString(),
is_active: 1, is_archived: 0, is_draft: 0, created_by: superId,
created_at: new Date().toISOString(),
}).returning('id');
const evId = evIns[0]?.id ?? evIns[0];
const mkCat = async (name) => {
const c = await db('photo_categories').insert({
event_id: evId, name, slug: name.toLowerCase(), created_at: new Date().toISOString(),
}).returning('id');
return c[0]?.id ?? c[0];
};
const cat1 = await mkCat('Cat1');
const cat2 = await mkCat('Cat2');
const pIns = await db('photos').insert({
event_id: evId, filename: 'p.jpg', path: 'authz-cat/p.jpg', type: 'individual',
category_id: cat1, uploaded_at: new Date().toISOString(),
}).returning('id');
const photoInCat1 = pIns[0]?.id ?? pIns[0];
// Pointing cat2's hero at a photo that lives in cat1 must be refused.
const bad = await auth(request(app).put(`/api/admin/categories/${cat2}/hero`), superTok)
.send({ hero_photo_id: photoInCat1 });
expect(bad.status).toBe(404);
// The photo's own category accepts it.
const ok = await auth(request(app).put(`/api/admin/categories/${cat1}/hero`), superTok)
.send({ hero_photo_id: photoInCat1 });
expect(ok.status).toBe(200);
});
});
});
@@ -0,0 +1,95 @@
/**
* Full-instance export is super_admin only (GHSA-pv6w-rj34-wj9v).
*
* GET /api/admin/backup/picpeak/export dumps every table unredacted (bcrypt
* hashes, 2FA, SMTP/SSO/WhatsApp/webhook/S3 secrets). It was gated only by
* requirePermission('backup.create'), which the built-in `admin` role holds —
* so any non-super_admin admin could download the whole database. Pins that
* `admin` now gets 403 and `super_admin` passes the gate.
*/
const path = require('path');
const fs = require('fs');
const os = require('os');
process.env.NODE_ENV = 'test';
process.env.TEST_DATABASE_PATH = path.join(
fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-bkexport-')), 'db.sqlite',
);
process.env.JWT_SECRET = process.env.JWT_SECRET || 'bkexport-test-secret';
// The export otherwise walks the whole DB and writes a zip — stub it so the
// super_admin happy path is fast and deterministic; the gate is what's tested.
// The route deletes path.dirname(filePath) recursively after download, so the
// stub MUST live in its own dir — a bare os.tmpdir() file would make the route
// wipe the whole temp root (and other jest workers' DB files).
const mockExportDir = fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-export-stub-'));
const mockExportPath = path.join(mockExportDir, 'export.picpeak');
fs.writeFileSync(mockExportPath, 'stub');
jest.mock('../../src/services/picpeakExportService', () => ({
createPicpeak: jest.fn(async () => ({ filePath: mockExportPath })),
}));
const request = require('supertest');
const express = require('express');
const bcrypt = require('bcrypt');
const jwt = require('jsonwebtoken');
const { bootCrmDb, seedMinimal } = require('../integration/helpers/crmDb');
describe('backup export super_admin gate (GHSA-pv6w)', () => {
let db;
let cleanup;
let app;
let adminToken; let superToken;
const mkUser = async (username, roleName) => {
const role = await db('roles').where({ name: roleName }).first();
const r = await db('admin_users').insert({
username,
email: `${username}@example.com`,
password_hash: await bcrypt.hash('Passw0rd!', 4),
role_id: role.id,
is_active: 1,
created_at: new Date(),
updated_at: new Date(),
}).returning('id');
const id = r[0]?.id ?? r[0];
return jwt.sign(
{ id, username, type: 'admin', role: roleName, loginTime: Date.now() },
process.env.JWT_SECRET,
{ expiresIn: '1h', issuer: 'picpeak-auth' },
);
};
beforeAll(async () => {
({ db, cleanup } = await bootCrmDb());
await seedMinimal(db);
adminToken = await mkUser('limited-admin', 'admin');
superToken = await mkUser('root-admin', 'super_admin');
app = express();
app.use(express.json());
app.use('/api/admin/backup', require('../../src/routes/adminBackup'));
}, 120000);
afterAll(async () => {
if (cleanup) await cleanup();
fs.rmSync(mockExportDir, { recursive: true, force: true });
});
it('denies the built-in admin role (was: full DB dump)', async () => {
const res = await request(app)
.get('/api/admin/backup/picpeak/export')
.set('Authorization', `Bearer ${adminToken}`);
expect(res.status).toBe(403);
});
it('allows super_admin', async () => {
const res = await request(app)
.get('/api/admin/backup/picpeak/export')
.set('Authorization', `Bearer ${superToken}`);
expect(res.status).not.toBe(403);
expect(res.status).toBeLessThan(500);
});
});
@@ -0,0 +1,101 @@
/**
* GHSA-2qc2 / GHSA-32h4 / GHSA-3335 — feedback moderation, deletion, and the
* pending-moderation list are by-feedback-id (or global) and lacked ownership
* scoping, so a restricted editor could act on / enumerate feedback for events
* it does not own. super_admin keeps global access.
*/
const path = require('path');
const fs = require('fs');
const os = require('os');
process.env.NODE_ENV = 'test';
process.env.TEST_DATABASE_PATH = path.join(
fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-fbown-')), 'db.sqlite',
);
process.env.JWT_SECRET = process.env.JWT_SECRET || 'fbown-test-secret';
process.env.STORAGE_PATH = fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-fbown-storage-'));
const request = require('supertest');
const express = require('express');
const cookieParser = require('cookie-parser');
const bcrypt = require('bcrypt');
const { bootCrmDb, seedMinimal, assignAdminRole, mintAdminToken } = require('../integration/helpers/crmDb');
describe('feedback ownership scoping', () => {
let db; let cleanup; let app;
let superTok; let editorTok; let editorId;
let foreignFeedbackId;
const auth = (req, tok) => req.set('Authorization', `Bearer ${tok}`);
beforeAll(async () => {
({ db, cleanup } = await bootCrmDb());
const { adminId: superId } = await seedMinimal(db);
await assignAdminRole(db, superId, 'super_admin');
superTok = mintAdminToken(superId);
const ins = await db('admin_users').insert({
username: 'editor', email: 'editor@example.com',
password_hash: await bcrypt.hash('x', 4), must_change_password: false, created_at: new Date(),
}).returning('id');
editorId = ins[0]?.id ?? ins[0];
await assignAdminRole(db, editorId, 'editor');
editorTok = mintAdminToken(editorId);
// Event owned by super_admin (NOT the editor).
const ev = await db('events').insert({
slug: 'fbown-foreign', event_type: 'wedding', event_name: 'Foreign',
event_date: '2026-08-01', host_email: 'h@e.com', admin_email: 'a@e.com',
password_hash: 'x', share_link: '/g/fbown/s', share_token: 'fbown-share',
expires_at: new Date(Date.now() + 7 * 864e5).toISOString(),
is_active: 1, is_archived: 0, is_draft: 0, created_by: superId,
created_at: new Date().toISOString(),
}).returning('id');
const eventId = ev[0]?.id ?? ev[0];
const ph = await db('photos').insert({
event_id: eventId, filename: 'p.jpg', path: 'fbown-foreign/p.jpg', type: 'individual',
uploaded_at: new Date().toISOString(),
}).returning('id');
const photoId = ph[0]?.id ?? ph[0];
const fb = await db('photo_feedback').insert({
photo_id: photoId, event_id: eventId, feedback_type: 'comment',
comment_text: 'hi', is_approved: 0, is_hidden: 0, created_at: new Date().toISOString(),
}).returning('id');
foreignFeedbackId = fb[0]?.id ?? fb[0];
app = express();
app.use(express.json());
app.use(cookieParser());
app.use('/api/admin/feedback', require('../../src/routes/adminFeedback'));
}, 120000);
afterAll(async () => { if (cleanup) await cleanup(); });
it('editor cannot moderate feedback on an event it does not own (404)', async () => {
const res = await auth(request(app).put(`/api/admin/feedback/feedback/${foreignFeedbackId}/approve`), editorTok);
expect(res.status).toBe(404);
const row = await db('photo_feedback').where({ id: foreignFeedbackId }).first();
expect([false, 0]).toContain(row.is_approved); // untouched
});
it('editor cannot delete foreign feedback, row survives', async () => {
const res = await auth(request(app).delete(`/api/admin/feedback/feedback/${foreignFeedbackId}`), editorTok);
// Denied either at the events.delete permission layer (editor lacks it →
// 403) or the ownership layer (404) — both must leave the row intact.
expect([403, 404]).toContain(res.status);
expect(await db('photo_feedback').where({ id: foreignFeedbackId }).first()).toBeDefined();
});
it('editor sees no foreign feedback in pending-moderation', async () => {
const res = await auth(request(app).get('/api/admin/feedback/feedback/pending-moderation'), editorTok);
expect(res.status).toBe(200);
expect(res.body.find((f) => f.id === foreignFeedbackId)).toBeUndefined();
});
it('super_admin CAN moderate and see it', async () => {
const pending = await auth(request(app).get('/api/admin/feedback/feedback/pending-moderation'), superTok);
expect(pending.body.find((f) => f.id === foreignFeedbackId)).toBeDefined();
const res = await auth(request(app).put(`/api/admin/feedback/feedback/${foreignFeedbackId}/approve`), superTok);
expect(res.status).toBe(200);
});
});
@@ -0,0 +1,87 @@
/**
* GHSA-rh8r-7x3h-36rv — the unauthenticated GET /api/gallery/resolve/:identifier
* must NOT return a gallery's secret share_token (nor the share links that
* embed it) for a bare *slug* lookup. Slugs appear in gallery URLs and are
* guessable; handing back the secret turns a known slug into share-link
* access to a no-password gallery. The token is only returned when the caller
* resolved via the token / full share link (i.e. already holds it).
*/
const path = require('path');
const fs = require('fs');
const os = require('os');
process.env.NODE_ENV = 'test';
process.env.TEST_DATABASE_PATH = path.join(
fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-resolve-')), 'db.sqlite',
);
process.env.JWT_SECRET = process.env.JWT_SECRET || 'resolve-test-secret';
process.env.STORAGE_PATH = fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-resolve-storage-'));
const request = require('supertest');
const express = require('express');
const cookieParser = require('cookie-parser');
const { bootCrmDb, seedMinimal } = require('../integration/helpers/crmDb');
const SLUG = 'resolve-test-event';
const SHARE_TOKEN = 'a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6';
describe('GET /api/gallery/resolve/:identifier (GHSA-rh8r)', () => {
let db; let cleanup; let app;
beforeAll(async () => {
({ db, cleanup } = await bootCrmDb());
await seedMinimal(db);
await db('events').insert({
slug: SLUG,
event_type: 'wedding',
event_name: 'Resolve Test',
event_date: '2026-08-01',
host_email: 'h@example.com',
admin_email: 'a@example.com',
password_hash: 'x',
share_link: `/gallery/${SLUG}/${SHARE_TOKEN}`,
share_token: SHARE_TOKEN,
require_password: 0, // no-password → the token IS the access credential
expires_at: new Date(Date.now() + 7 * 864e5).toISOString(),
is_active: 1, is_archived: 0, is_draft: 0,
created_at: new Date().toISOString(),
});
app = express();
app.use(express.json());
app.use(cookieParser());
app.use('/api/gallery', require('../../src/routes/gallery'));
}, 120000);
afterAll(async () => { if (cleanup) await cleanup(); });
it('does NOT leak the share_token (or share links) for a bare slug lookup', async () => {
const res = await request(app).get(`/api/gallery/resolve/${SLUG}`);
expect(res.status).toBe(200);
expect(res.body.slug).toBe(SLUG);
expect(res.body.matchType).toBe('slug');
// The secret must be absent — and must not sneak out via the share links.
expect(res.body.token).toBeUndefined();
expect(res.body.share_link).toBeUndefined();
expect(res.body.share_url).toBeUndefined();
expect(JSON.stringify(res.body)).not.toContain(SHARE_TOKEN);
});
it('DOES return the token when the caller already resolved via the token', async () => {
const res = await request(app).get(`/api/gallery/resolve/${SHARE_TOKEN}`);
expect(res.status).toBe(200);
expect(res.body.token).toBe(SHARE_TOKEN);
expect(res.body.matchType).toMatch(/token/);
});
it('does NOT leak the token via SQL LIKE wildcards in the link_partial fallback', async () => {
// Before the escaping fix, an anonymous request of 32 underscores matched
// any share_link ending in a 32-char token (`_` = single-char wildcard),
// resolved as matchType 'link_partial', and handed back the bearer token.
// The share_token here has no underscores, so an escaped LIKE must miss.
const res = await request(app).get(`/api/gallery/resolve/${'_'.repeat(SHARE_TOKEN.length)}`);
expect(res.status).toBe(404);
expect(res.body.token).toBeUndefined();
expect(JSON.stringify(res.body)).not.toContain(SHARE_TOKEN);
});
});
@@ -0,0 +1,240 @@
/**
* Hidden/client-only photo access control across the bulk + secure photo
* routes (GHSA cluster: fpwq / ghf8 / 3jvw / 9cc4 / 2hqg / jc22).
*
* A photo with visibility='hidden' is client-only. The main photo-list and
* single-photo download/view routes enforced this, but the bulk-download,
* protected-image, and secure-image routes shipped without the check —
* letting an ordinary guest reach hidden photos. These tests pin that
* guests are refused and PIN-clients (accessLevel='client') still succeed.
*/
const path = require('path');
const fs = require('fs');
const os = require('os');
process.env.NODE_ENV = 'test';
process.env.TEST_DATABASE_PATH = path.join(
fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-hidden-')), 'db.sqlite',
);
process.env.JWT_SECRET = process.env.JWT_SECRET || 'hidden-photo-test-secret';
process.env.STORAGE_PATH = fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-hidden-storage-'));
const request = require('supertest');
const express = require('express');
const cookieParser = require('cookie-parser');
const jwt = require('jsonwebtoken');
const { bootCrmDb, seedMinimal } = require('../integration/helpers/crmDb');
const SLUG = 'hidden-photo-test-event';
describe('hidden-photo access control (GHSA cluster)', () => {
let db;
let cleanup;
let app;
let eventId;
let visibleId;
let hiddenId;
const guestToken = () => jwt.sign(
{ eventId, eventSlug: SLUG, type: 'gallery' },
process.env.JWT_SECRET,
{ expiresIn: '1h', issuer: 'picpeak-auth' }
);
const clientToken = () => jwt.sign(
{ eventId, eventSlug: SLUG, type: 'gallery', accessLevel: 'client' },
process.env.JWT_SECRET,
{ expiresIn: '1h', issuer: 'picpeak-auth' }
);
beforeAll(async () => {
({ db, cleanup } = await bootCrmDb());
await seedMinimal(db);
const inserted = await db('events').insert({
slug: SLUG,
event_type: 'wedding',
event_name: 'Hidden Photo Test',
event_date: '2026-08-01',
host_email: 'host@example.com',
admin_email: 'admin@example.com',
password_hash: 'x',
share_link: `/gallery/${SLUG}/share`,
share_token: 'hidden-photo-share',
expires_at: new Date(Date.now() + 7 * 24 * 3600 * 1000).toISOString(),
is_active: 1, is_archived: 0, is_draft: 0, allow_downloads: 1,
created_at: new Date().toISOString(),
}).returning('id');
eventId = inserted[0]?.id ?? inserted[0];
const photoDir = path.join(process.env.STORAGE_PATH, 'events/active', SLUG);
fs.mkdirSync(photoDir, { recursive: true });
// A real 1x1 PNG so the protected /view route's Sharp processing path
// succeeds (fake bytes 500 on metadata()). Content, not extension,
// drives Sharp's format detection.
const PNG_1x1 = Buffer.from(
'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNk+M8AAAMCAQGV2rY9AAAAAElFTkSuQmCC',
'base64'
);
const mkPhoto = async (filename, visibility) => {
fs.writeFileSync(path.join(photoDir, filename), PNG_1x1);
const p = await db('photos').insert({
event_id: eventId,
filename,
path: `${SLUG}/${filename}`,
type: 'individual',
visibility,
uploaded_at: new Date().toISOString(),
}).returning('id');
return p[0]?.id ?? p[0];
};
visibleId = await mkPhoto('visible.jpg', 'visible');
hiddenId = await mkPhoto('hidden.jpg', 'hidden');
app = express();
app.use(express.json());
app.use(cookieParser());
app.use('/api/gallery', require('../../src/routes/gallery'));
app.use('/api/images', require('../../src/routes/protectedImages'));
app.use('/api/secure-images', require('../../src/routes/secureImages'));
}, 120000);
afterAll(async () => { if (cleanup) await cleanup(); });
describe('download-selected (GHSA-ghf8, medium)', () => {
it('omits a hidden photo for a guest even when its id is requested', async () => {
const res = await request(app)
.post(`/api/gallery/${SLUG}/download-selected`)
.set('Authorization', `Bearer ${guestToken()}`)
.send({ photo_ids: [visibleId, hiddenId] });
// The visible photo still zips; the hidden one is filtered out. If
// only the hidden id were requested, the filter empties the set → 404.
expect(res.status).toBe(200);
const solo = await request(app)
.post(`/api/gallery/${SLUG}/download-selected`)
.set('Authorization', `Bearer ${guestToken()}`)
.send({ photo_ids: [hiddenId] });
expect(solo.status).toBe(404);
});
it('includes the hidden photo for a client', async () => {
const res = await request(app)
.post(`/api/gallery/${SLUG}/download-selected`)
.set('Authorization', `Bearer ${clientToken()}`)
.send({ photo_ids: [hiddenId] });
expect(res.status).toBe(200);
});
});
describe('download-all (GHSA-fpwq, medium)', () => {
it('streams for a guest without erroring (hidden photos filtered)', async () => {
const res = await request(app)
.get(`/api/gallery/${SLUG}/download-all`)
.set('Authorization', `Bearer ${guestToken()}`);
expect(res.status).toBe(200);
});
});
describe('protected-image view (GHSA-9cc4)', () => {
it('403s a hidden photo for a guest', async () => {
const res = await request(app)
.get(`/api/images/${SLUG}/photo/${hiddenId}/view`)
.set('Authorization', `Bearer ${guestToken()}`);
expect(res.status).toBe(403);
});
it('serves a visible photo for a guest', async () => {
const res = await request(app)
.get(`/api/images/${SLUG}/photo/${visibleId}/view`)
.set('Authorization', `Bearer ${guestToken()}`);
expect(res.status).toBe(200);
});
it('serves a hidden photo for a client', async () => {
const res = await request(app)
.get(`/api/images/${SLUG}/photo/${hiddenId}/view`)
.set('Authorization', `Bearer ${clientToken()}`);
expect(res.status).toBe(200);
});
});
describe('signed-URL mint (GHSA-3jvw)', () => {
it('403s minting a signed URL for a hidden photo as a guest', async () => {
const res = await request(app)
.post(`/api/images/${SLUG}/photo/${hiddenId}/generate-url`)
.set('Authorization', `Bearer ${guestToken()}`);
expect(res.status).toBe(403);
});
it('mints for a client', async () => {
const res = await request(app)
.post(`/api/images/${SLUG}/photo/${hiddenId}/generate-url`)
.set('Authorization', `Bearer ${clientToken()}`);
expect(res.status).toBe(200);
expect(res.body.url).toContain('/signed/');
});
});
describe('legacy secure-token mint (protectedImages generate-secure-token)', () => {
it('403s a hidden photo for a guest', async () => {
const res = await request(app)
.post(`/api/images/${SLUG}/photo/${hiddenId}/generate-secure-token`)
.set('Authorization', `Bearer ${guestToken()}`);
expect(res.status).toBe(403);
});
it('mints for a client', async () => {
const res = await request(app)
.post(`/api/images/${SLUG}/photo/${hiddenId}/generate-secure-token`)
.set('Authorization', `Bearer ${clientToken()}`);
expect(res.status).toBe(200);
expect(res.body.token).toBeDefined();
});
});
describe('secure-token mint (GHSA-2hqg)', () => {
it('403s minting a secure token for a hidden photo as a guest', async () => {
const res = await request(app)
.post(`/api/secure-images/${SLUG}/generate-token`)
.set('Authorization', `Bearer ${guestToken()}`)
.send({ photoId: hiddenId });
expect(res.status).toBe(403);
});
it('mints for a client', async () => {
const res = await request(app)
.post(`/api/secure-images/${SLUG}/generate-token`)
.set('Authorization', `Bearer ${clientToken()}`)
.send({ photoId: hiddenId });
expect(res.status).toBe(200);
expect(res.body.token).toBeDefined();
});
});
// A capability minted while a photo is visible must stop serving once the
// photo is hidden — unless minted by a client (clientBypass in the token).
describe('signed-URL TOCTOU (hidden AFTER minting)', () => {
afterEach(async () => {
await db('photos').where({ id: visibleId }).update({ visibility: 'visible' });
});
it("a guest's pre-minted signed URL stops serving once the photo is hidden", async () => {
const mint = await request(app)
.post(`/api/images/${SLUG}/photo/${visibleId}/generate-url`)
.set('Authorization', `Bearer ${guestToken()}`);
expect(mint.status).toBe(200);
const url = mint.body.url;
// Still visible → serves.
expect((await request(app).get(url)).status).toBe(200);
// Hide it → the guest token (no clientBypass) must now be refused.
await db('photos').where({ id: visibleId }).update({ visibility: 'hidden' });
expect((await request(app).get(url)).status).toBe(403);
});
it("a client's pre-minted signed URL keeps serving after the photo is hidden", async () => {
const mint = await request(app)
.post(`/api/images/${SLUG}/photo/${visibleId}/generate-url`)
.set('Authorization', `Bearer ${clientToken()}`);
expect(mint.status).toBe(200);
const url = mint.body.url;
await db('photos').where({ id: visibleId }).update({ visibility: 'hidden' });
expect((await request(app).get(url)).status).toBe(200);
});
});
});
@@ -52,7 +52,7 @@ describe('photo engagement counters (#895)', () => {
const getPhoto = async (id) => db('photos').where('id', id).first();
// The counter writes are fire-and-forget on purpose — give the event
// loop a beat before asserting.
const settle = () => new Promise((r) => setTimeout(r, 100));
const settle = () => new Promise((r) => setTimeout(r, 400));
beforeAll(async () => {
({ db, cleanup } = await bootCrmDb());
@@ -115,6 +115,7 @@ describe('photo engagement counters (#895)', () => {
app.use(cookieParser());
app.use('/api/gallery', require('../../src/routes/gallery'));
app.use('/api/admin/events', require('../../src/routes/adminEvents'));
app.use('/api/admin/photos', require('../../src/routes/adminPhotos'));
}, 120000);
afterAll(async () => {
@@ -200,6 +201,13 @@ describe('photo engagement counters (#895)', () => {
// Own event so the on-the-fly archiver path is guaranteed — the
// main event may have a cached zip from the previous test's
// background generation, and racing its build/invalidate hangs.
// The route also fires a background pre-zip build after streaming;
// against this event's intentionally missing file it crashes with
// an async ENOENT that jest attributes to whatever test is running
// by then — neutralize it, it's not under test here.
const downloadZipService = require('../../src/services/downloadZipService');
const generateZipSpy = jest.spyOn(downloadZipService, 'generateZip')
.mockResolvedValue({ success: false, error: 'disabled in test' });
const slug2 = `${SLUG}-skip`;
const ev = await db('events').insert({
slug: slug2,
@@ -248,6 +256,33 @@ describe('photo engagement counters (#895)', () => {
expect((await db('photos').where('id', ids2[0]).first()).download_count).toBe(1);
// photo-1's source was missing → skipped from the zip → not counted
expect((await db('photos').where('id', ids2[1]).first()).download_count).toBe(0);
generateZipSpy.mockRestore();
});
});
describe('admin photos list exposes the counters (#895 follow-up)', () => {
it('returns view_count and download_count so the Engagement column can render them', async () => {
// The list mapper builds an explicit object — before this fix it
// omitted both fields, so the admin table showed 0 forever even
// though the DB counted correctly.
await request(app)
.post(`/api/gallery/${SLUG}/photo/${photoIds[0]}/view`)
.set('Authorization', `Bearer ${galleryToken()}`);
await request(app)
.get(`/api/gallery/${SLUG}/download/${photoIds[0]}`)
.set('Authorization', `Bearer ${galleryToken()}`);
await settle();
const res = await request(app)
.get(`/api/admin/photos/${eventId}/photos`)
.set('Authorization', `Bearer ${adminToken}`);
expect(res.status).toBe(200);
const row = res.body.photos.find((p) => p.id === photoIds[0]);
expect(row.view_count).toBe(1);
expect(row.download_count).toBe(1);
const untouched = res.body.photos.find((p) => p.id === photoIds[1]);
expect(untouched.view_count).toBe(0);
expect(untouched.download_count).toBe(0);
});
});
@@ -0,0 +1,136 @@
/**
* Secure-image view route token binding (GHSA-g94x-8vv8-3c9f).
*
* The view route GET /api/secure-images/:slug/secure/:photoId/:token serves
* via <img src> with the token in the URL, so it can't carry a gallery-token
* header like the download sibling. Before the fix it validated only the
* token signature and took the gallery/photo from the URL — so a token minted
* on any PUBLIC gallery read every other gallery's photos with no password.
*
* Pins that the route now enforces the scope inside the token:
* - the URL photoId must equal the token's minted photoId
* - the gallery embedded in the token's sessionId must equal the URL gallery
* A token minted on gallery A cannot read gallery B under either check; a
* token used on its own gallery+photo passes the binding.
*/
const path = require('path');
const fs = require('fs');
const os = require('os');
process.env.NODE_ENV = 'test';
process.env.TEST_DATABASE_PATH = path.join(
fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-secimg-')), 'db.sqlite',
);
process.env.JWT_SECRET = process.env.JWT_SECRET || 'secimg-test-secret';
process.env.STORAGE_PATH = fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-secimg-storage-'));
// Stub the anti-bot/rate-limit middleware so the fingerprint is deterministic
// — the token below is minted with the same fingerprint, so verifySecureToken
// passes and the binding logic under test is what decides the outcome.
jest.mock('../../src/middleware/secureImageMiddleware', () => ({
secureImageAccess: (req, _res, next) => {
req.clientInfo = { fingerprint: 'test-fp', ip: '127.0.0.1', userAgent: 'jest' };
next();
},
getSecurityStatus: (_req, res) => res.json({ ok: true }),
}));
const request = require('supertest');
const express = require('express');
const { bootCrmDb, seedMinimal } = require('../integration/helpers/crmDb');
const secureImageService = require('../../src/services/secureImageService');
describe('secure-image view route token binding (GHSA-g94x)', () => {
let db;
let cleanup;
let app;
let galleryA; let galleryB;
let photoA; let photoB;
const mkEvent = async (slug, requirePassword) => {
const r = await db('events').insert({
slug,
event_type: 'wedding',
event_name: slug,
event_date: '2026-08-01',
host_email: 'h@example.com',
admin_email: 'a@example.com',
password_hash: 'x',
require_password: requirePassword ? 1 : 0,
share_link: `/gallery/${slug}/share`,
share_token: `${slug}-share`,
expires_at: new Date(Date.now() + 7 * 24 * 3600 * 1000).toISOString(),
is_active: 1,
is_archived: 0,
is_draft: 0,
created_at: new Date().toISOString(),
}).returning('id');
return r[0]?.id ?? r[0];
};
const mkPhoto = async (eventId, slug, filename) => {
const dir = path.join(process.env.STORAGE_PATH, 'events/active', slug);
fs.mkdirSync(dir, { recursive: true });
fs.writeFileSync(path.join(dir, filename), Buffer.from('img'));
const r = await db('photos').insert({
event_id: eventId,
filename,
path: `${slug}/${filename}`,
type: 'individual',
uploaded_at: new Date().toISOString(),
}).returning('id');
return r[0]?.id ?? r[0];
};
// Mint a token exactly as the mint route does — bound to (photoId, gallery
// sessionId, fingerprint) — bypassing the anti-bot HTTP path.
const mint = (photoId, eventId) => secureImageService.generateSecureToken(
photoId,
`gallery_public_${eventId}_${Date.now()}`,
{ clientFingerprint: 'test-fp', maxUses: 100, expiresIn: 3600 },
);
const view = (slug, photoId, token) => request(app)
.get(`/api/secure-images/${slug}/secure/${photoId}/${token}`);
beforeAll(async () => {
({ db, cleanup } = await bootCrmDb());
await seedMinimal(db);
galleryA = await mkEvent('secimg-public-a', false); // public — token source
galleryB = await mkEvent('secimg-private-b', true); // password-protected — victim
photoA = await mkPhoto(galleryA, 'secimg-public-a', 'a.jpg');
photoB = await mkPhoto(galleryB, 'secimg-private-b', 'b.jpg');
app = express();
app.use(express.json());
app.use('/api/secure-images', require('../../src/routes/secureImages'));
}, 120000);
afterAll(async () => { if (cleanup) await cleanup(); });
it('rejects a gallery-A token used against gallery B (cross-photo)', async () => {
const token = mint(photoA, galleryA);
const res = await view('secimg-private-b', photoB, token);
expect(res.status).toBe(403);
expect(res.body.error).toMatch(/not valid for this photo/i);
});
it('rejects a gallery-A token replayed on gallery B with A\'s photoId', async () => {
const token = mint(photoA, galleryA);
// URL photoId matches the token, so the photo check passes — the gallery
// check (sessionId gallery A != URL gallery B) must catch it.
const res = await view('secimg-private-b', photoA, token);
expect(res.status).toBe(403);
expect(res.body.error).toMatch(/not valid for this gallery/i);
});
it('lets a token read its own gallery + photo (binding passes)', async () => {
const token = mint(photoA, galleryA);
const res = await view('secimg-public-a', photoA, token);
// Binding passes; serving may 200/404/500 depending on the pipeline, but
// it must NOT be rejected as a token mismatch.
expect(res.status).not.toBe(403);
});
});
Binary file not shown.
@@ -0,0 +1,128 @@
/**
* DNS-resolving SSRF guard (GHSA SSRF cluster: webhook / S3 / rsync / SMTP /
* IMAP). The literal isPrivateIP check can't see that a public-looking
* hostname resolves to an internal/metadata IP; isHostAllowed resolves the
* name and vets every A/AAAA record.
*/
jest.mock('dns', () => {
const actual = jest.requireActual('dns');
return { ...actual, promises: { ...actual.promises, lookup: jest.fn() } };
});
const dns = require('dns');
const {
isHostAllowed,
validateExternalUrlAsync,
classifyHost,
} = require('../../src/utils/networkValidation');
const lookup = dns.promises.lookup;
describe('classifyHost', () => {
beforeEach(() => lookup.mockReset());
it('distinguishes private, unresolved, ok, and invalid', async () => {
lookup.mockResolvedValue([{ address: '10.0.0.5', family: 4 }]);
expect(await classifyHost('evil.example')).toBe('private');
lookup.mockResolvedValue([{ address: '93.184.216.34', family: 4 }]);
expect(await classifyHost('example.com')).toBe('ok');
lookup.mockRejectedValue(new Error('EAI_AGAIN'));
expect(await classifyHost('blip.example')).toBe('unresolved');
lookup.mockResolvedValue([]);
expect(await classifyHost('empty.example')).toBe('unresolved');
expect(await classifyHost('')).toBe('invalid');
expect(await classifyHost('10.0.0.1')).toBe('private'); // literal, no lookup
});
});
describe('isHostAllowed', () => {
beforeEach(() => lookup.mockReset());
it('rejects a public hostname that resolves to a private IP', async () => {
lookup.mockResolvedValue([{ address: '10.0.0.5', family: 4 }]);
expect(await isHostAllowed('evil.example.com')).toBe(false);
});
it('rejects when the hostname resolves to the cloud metadata IP', async () => {
lookup.mockResolvedValue([{ address: '169.254.169.254', family: 4 }]);
expect(await isHostAllowed('metadata-rebind.example')).toBe(false);
});
it('rejects when ANY resolved address is private (rebinding / mixed records)', async () => {
lookup.mockResolvedValue([
{ address: '93.184.216.34', family: 4 },
{ address: '169.254.169.254', family: 4 },
]);
expect(await isHostAllowed('rebind.example')).toBe(false);
});
it('allows a hostname that resolves only to public IPs', async () => {
lookup.mockResolvedValue([{ address: '93.184.216.34', family: 4 }]);
expect(await isHostAllowed('example.com')).toBe(true);
});
it('fails closed when resolution errors', async () => {
lookup.mockRejectedValue(new Error('ENOTFOUND'));
expect(await isHostAllowed('nxdomain.invalid')).toBe(false);
});
it('fails closed on an empty resolution', async () => {
lookup.mockResolvedValue([]);
expect(await isHostAllowed('empty.example')).toBe(false);
});
it('rejects literal private IPs and blocked names without resolving', async () => {
expect(await isHostAllowed('127.0.0.1')).toBe(false);
expect(await isHostAllowed('10.0.0.1')).toBe(false);
expect(await isHostAllowed('localhost')).toBe(false);
expect(await isHostAllowed('metadata.google.internal')).toBe(false);
expect(await isHostAllowed('foo.internal')).toBe(false);
expect(lookup).not.toHaveBeenCalled();
});
it('allows a public IP literal without resolving', async () => {
expect(await isHostAllowed('93.184.216.34')).toBe(true);
expect(lookup).not.toHaveBeenCalled();
});
it('rejects empty / non-string input', async () => {
expect(await isHostAllowed('')).toBe(false);
expect(await isHostAllowed(null)).toBe(false);
});
});
describe('validateExternalUrlAsync', () => {
beforeEach(() => lookup.mockReset());
it('rejects a URL whose host resolves to a private address', async () => {
lookup.mockResolvedValue([{ address: '10.1.2.3', family: 4 }]);
const r = await validateExternalUrlAsync('https://evil.example/hook');
expect(r.valid).toBe(false);
});
it('accepts a URL whose host resolves public', async () => {
lookup.mockResolvedValue([{ address: '93.184.216.34', family: 4 }]);
expect((await validateExternalUrlAsync('https://example.com/hook')).valid).toBe(true);
});
it('rejects a malformed URL', async () => {
expect((await validateExternalUrlAsync('not a url')).valid).toBe(false);
});
it('reports reason=unresolved for a transient lookup failure (retryable)', async () => {
lookup.mockRejectedValue(new Error('EAI_AGAIN'));
const r = await validateExternalUrlAsync('https://blip.example/hook');
expect(r.valid).toBe(false);
expect(r.reason).toBe('unresolved');
});
it('reports reason=private for a resolved-private host (permanent)', async () => {
lookup.mockResolvedValue([{ address: '169.254.169.254', family: 4 }]);
const r = await validateExternalUrlAsync('https://rebind.example/hook');
expect(r.valid).toBe(false);
expect(r.reason).toBe('private');
});
});
@@ -0,0 +1,40 @@
/**
* Unit tests for the shared hidden-photo access-control helper.
*
* Pins the rule that ordinary gallery guests never receive photos with
* visibility='hidden' (NULL = visible), while PIN-clients see everything.
*/
const {
canSeeHiddenPhotos,
isPhotoHiddenFromViewer,
} = require('../../src/utils/photoVisibility');
describe('canSeeHiddenPhotos', () => {
it('is true only for the client access level', () => {
expect(canSeeHiddenPhotos('client')).toBe(true);
expect(canSeeHiddenPhotos('guest')).toBe(false);
expect(canSeeHiddenPhotos('slideshow')).toBe(false);
expect(canSeeHiddenPhotos(undefined)).toBe(false);
});
});
describe('isPhotoHiddenFromViewer', () => {
it('blocks a hidden photo from guests', () => {
expect(isPhotoHiddenFromViewer({ visibility: 'hidden' }, 'guest')).toBe(true);
expect(isPhotoHiddenFromViewer({ visibility: 'hidden' }, 'slideshow')).toBe(true);
});
it('lets clients see hidden photos', () => {
expect(isPhotoHiddenFromViewer({ visibility: 'hidden' }, 'client')).toBe(false);
});
it('treats visible and NULL visibility as viewable by everyone', () => {
expect(isPhotoHiddenFromViewer({ visibility: 'visible' }, 'guest')).toBe(false);
expect(isPhotoHiddenFromViewer({ visibility: null }, 'guest')).toBe(false);
expect(isPhotoHiddenFromViewer({}, 'guest')).toBe(false);
});
it('is null-safe', () => {
expect(isPhotoHiddenFromViewer(null, 'guest')).toBe(false);
});
});
@@ -99,10 +99,22 @@ describe('resolveLogoFile', () => {
}
});
it('treats absolute paths as-is when they exist', async () => {
it('rejects an absolute path OUTSIDE the storage roots (GHSA-c7x5)', async () => {
// The raw-absolute candidate was an arbitrary-file-read primitive
// (logo_path: '/etc/passwd' → rasterised into a PDF). Absolute paths
// outside the storage roots are now dropped even if they exist.
existsSpy.mockImplementation((p) => p === '/abs/path/logo.png');
getAppSetting.mockResolvedValue(null);
const out = await resolveLogoFile({ logo_path: '/abs/path/logo.png' });
expect(out).toBe('/abs/path/logo.png');
expect(out).toBeNull();
});
it('still accepts an absolute path INSIDE the storage root', async () => {
// The legitimate case: multer stores the uploaded logo under
// storage/uploads/logos with an absolute path — that stays resolvable.
existsSpy.mockImplementation((p) => p === '/app/storage/uploads/logos/logo.png');
getAppSetting.mockResolvedValue(null);
const out = await resolveLogoFile({ logo_path: '/app/storage/uploads/logos/logo.png' });
expect(out).toBe('/app/storage/uploads/logos/logo.png');
});
});
+35 -39
View File
@@ -1,12 +1,12 @@
{
"name": "picpeak-backend",
"version": "3.45.6",
"version": "3.45.10",
"lockfileVersion": 3,
"requires": true,
"packages": {
"": {
"name": "picpeak-backend",
"version": "3.45.6",
"version": "3.45.10",
"dependencies": {
"@aws-sdk/client-s3": "^3.850.0",
"@aws-sdk/lib-storage": "^3.850.0",
@@ -42,13 +42,14 @@
"node-stream-zip": "^1.15.0",
"nodemailer": "^9.0.1",
"otplib": "^12.0.1",
"p-limit": "^3.1.0",
"pdf-lib": "^1.17.1",
"pdfkit": "^0.17.2",
"pg": "^8.16.3",
"postcss": "8.5.18",
"qrcode": "^1.5.4",
"react-i18next": "^15.6.0",
"sanitize-html": "^2.17.0",
"sanitize-html": "2.17.5",
"sharp": "0.35.3",
"sqlite3": "^5.1.6",
"swagger-jsdoc": "^6.2.8",
@@ -5226,6 +5227,12 @@
"integrity": "sha512-KALDyEYgpY+Rlob/iriUtjV6d5Eq+Y191A5g4UqLAi8CyGP9N1+FdVbkc1SxKc2r4YAYqG8JzO2KGL+AizD70Q==",
"license": "MIT"
},
"node_modules/dayjs": {
"version": "1.11.21",
"resolved": "https://registry.npmjs.org/dayjs/-/dayjs-1.11.21.tgz",
"integrity": "sha512-98IT+HOahAisibz/yjKbzuOBwYcjJ7BCLPzARyHiyEBmRz4fatF+KPJszEHXsGYjUG234aH/cOjW1wwTbKUZlA==",
"license": "MIT"
},
"node_modules/debug": {
"version": "4.4.3",
"resolved": "https://registry.npmjs.org/debug/-/debug-4.4.3.tgz",
@@ -6785,19 +6792,7 @@
"url": "https://github.com/sponsors/KillyMXI"
}
},
"node_modules/html-to-text/node_modules/entities": {
"version": "7.0.1",
"resolved": "https://registry.npmjs.org/entities/-/entities-7.0.1.tgz",
"integrity": "sha512-TWrgLOFUQTH994YUyl1yT4uyavY5nNB5muff+RtWaqNVCAK408b5ZnnbNAUEWLTCpum9w6arT70i1XdQ4UeOPA==",
"license": "BSD-2-Clause",
"engines": {
"node": ">=0.12"
},
"funding": {
"url": "https://github.com/fb55/entities?sponsor=1"
}
},
"node_modules/html-to-text/node_modules/htmlparser2": {
"node_modules/htmlparser2": {
"version": "10.1.0",
"resolved": "https://registry.npmjs.org/htmlparser2/-/htmlparser2-10.1.0.tgz",
"integrity": "sha512-VTZkM9GWRAtEpveh7MSF6SjjrpNVNNVJfFup7xTY3UpFtm67foy9HDVXneLtFVt4pMz5kZtgNcvCniNFb1hlEQ==",
@@ -6816,23 +6811,16 @@
"entities": "^7.0.1"
}
},
"node_modules/htmlparser2": {
"version": "8.0.2",
"resolved": "https://registry.npmjs.org/htmlparser2/-/htmlparser2-8.0.2.tgz",
"integrity": "sha512-GYdjWKDkbRLkZ5geuHs5NY1puJ+PXwP7+fHPRz06Eirsb9ugf6d8kkXav6ADhcODhFFPMIXyxkxSuMf3D6NCFA==",
"funding": [
"https://github.com/fb55/htmlparser2?sponsor=1",
{
"type": "github",
"url": "https://github.com/sponsors/fb55"
}
],
"license": "MIT",
"dependencies": {
"domelementtype": "^2.3.0",
"domhandler": "^5.0.3",
"domutils": "^3.0.1",
"entities": "^4.4.0"
"node_modules/htmlparser2/node_modules/entities": {
"version": "7.0.1",
"resolved": "https://registry.npmjs.org/entities/-/entities-7.0.1.tgz",
"integrity": "sha512-TWrgLOFUQTH994YUyl1yT4uyavY5nNB5muff+RtWaqNVCAK408b5ZnnbNAUEWLTCpum9w6arT70i1XdQ4UeOPA==",
"license": "BSD-2-Clause",
"engines": {
"node": ">=0.12"
},
"funding": {
"url": "https://github.com/fb55/entities?sponsor=1"
}
},
"node_modules/http-cache-semantics": {
@@ -8232,6 +8220,15 @@
"integrity": "sha512-Xq9nH7KlWZmXAtodXDDRE7vs6DU1gTU8zYDHDiWLSip45Egwq3plLHzPn27NgvzL2r1LMPC1vdqh98sQxtqj4A==",
"license": "MIT"
},
"node_modules/launder": {
"version": "1.7.1",
"resolved": "https://registry.npmjs.org/launder/-/launder-1.7.1.tgz",
"integrity": "sha512-mU6WRz5EusL9ZZuiZ5SO4Y6C0P9PAUR9iwdb6bzj4KDihm28DiHFw+/yk9DBH4f+Pv1wuzQ4e2jV3oQ7mkIqvw==",
"license": "MIT",
"dependencies": {
"dayjs": "^1.11.7"
}
},
"node_modules/lazystream": {
"version": "1.0.1",
"resolved": "https://registry.npmjs.org/lazystream/-/lazystream-1.0.1.tgz",
@@ -9558,7 +9555,6 @@
"version": "3.1.0",
"resolved": "https://registry.npmjs.org/p-limit/-/p-limit-3.1.0.tgz",
"integrity": "sha512-TYOanM3wGwNGsZN2cVTYPArw454xnXj5qmWF1bEoAc4+cU/ol7GVh7odevjp1FNHduHc3KZMcFduxU5Xc6uJRQ==",
"dev": true,
"license": "MIT",
"dependencies": {
"yocto-queue": "^0.1.0"
@@ -10846,15 +10842,16 @@
"license": "MIT"
},
"node_modules/sanitize-html": {
"version": "2.17.0",
"resolved": "https://registry.npmjs.org/sanitize-html/-/sanitize-html-2.17.0.tgz",
"integrity": "sha512-dLAADUSS8rBwhaevT12yCezvioCA+bmUTPH/u57xKPT8d++voeYE6HeluA/bPbQ15TwDBG2ii+QZIEmYx8VdxA==",
"version": "2.17.5",
"resolved": "https://registry.npmjs.org/sanitize-html/-/sanitize-html-2.17.5.tgz",
"integrity": "sha512-ZmU1joGRrvoyctKIiuwUxqR6moLoU2Wk+2bMccN6f7UwhAmwYDvWziqPxRDDN2Qip62NqnIrVrT9akbL6Wretg==",
"license": "MIT",
"dependencies": {
"deepmerge": "^4.2.2",
"escape-string-regexp": "^4.0.0",
"htmlparser2": "^8.0.0",
"htmlparser2": "^10.1.0",
"is-plain-object": "^5.0.0",
"launder": "^1.7.1",
"parse-srcset": "^1.0.2",
"postcss": "^8.3.11"
}
@@ -12590,7 +12587,6 @@
"version": "0.1.0",
"resolved": "https://registry.npmjs.org/yocto-queue/-/yocto-queue-0.1.0.tgz",
"integrity": "sha512-rVksvsnNCdJ/ohGc6xgPwyN8eheCxsiLM8mxuE/t/mOVqJewPuO1miLpTHQiRgTKCLexL4MeAFVagts7HmNZ2Q==",
"dev": true,
"license": "MIT",
"engines": {
"node": ">=10"
+3 -2
View File
@@ -1,6 +1,6 @@
{
"name": "picpeak-backend",
"version": "3.45.9",
"version": "3.45.12",
"description": "Backend for PicPeak event photo sharing platform",
"main": "server.js",
"engines": {
@@ -51,13 +51,14 @@
"node-stream-zip": "^1.15.0",
"nodemailer": "^9.0.1",
"otplib": "^12.0.1",
"p-limit": "^3.1.0",
"pdf-lib": "^1.17.1",
"pdfkit": "^0.17.2",
"pg": "^8.16.3",
"postcss": "8.5.18",
"qrcode": "^1.5.4",
"react-i18next": "^15.6.0",
"sanitize-html": "^2.17.0",
"sanitize-html": "2.17.5",
"sharp": "0.35.3",
"sqlite3": "^5.1.6",
"swagger-jsdoc": "^6.2.8",
+28 -1
View File
@@ -3,6 +3,7 @@ const jwt = require('jsonwebtoken');
const { db } = require('../database/db');
const { formatBoolean } = require('../utils/dbCompat');
const { getGalleryTokenFromRequest } = require('../utils/tokenUtils');
const { isTokenRevoked } = require('../utils/tokenRevocation');
const logger = require('../utils/logger');
async function photoAuth(req, res, next) {
@@ -89,7 +90,33 @@ async function photoAuth(req, res, next) {
// Check if it's an admin token (admins can view all photos)
if (decoded.type === 'admin') {
// For both thumbnails and photos with admin token, allow access
// Enforce the same revocation / session-cutoff invalidation that
// adminAuth does — otherwise a validly-signed admin JWT keeps
// serving photos after logout, password change, or explicit
// revocation (GHSA-x55x).
if (await isTokenRevoked(decoded)) {
return res.status(401).json({ error: 'Session expired' });
}
// adminAuth also (a) rejects tokens for a now-deactivated admin and
// (b) rejects any token minted before the admin's last password
// change. Token revocation alone doesn't cover those, so without
// these two checks a stale or pre-password-change admin token still
// fetches every photo.
const admin = await db('admin_users')
.where({ id: decoded.id, is_active: formatBoolean(true) })
.select('id', 'password_changed_at')
.first();
if (!admin) {
return res.status(401).json({ error: 'Session expired' });
}
if (admin.password_changed_at) {
const passwordChangedSeconds = Math.floor(
new Date(admin.password_changed_at).getTime() / 1000
);
if (decoded.iat < passwordChangedSeconds) {
return res.status(401).json({ error: 'Session expired' });
}
}
return next();
}
} catch (err) {
+14 -1
View File
@@ -19,7 +19,10 @@ const router = express.Router();
// the plaintext, never recoverable after creation.
router.get('/', adminAuth, requirePermission('settings.view'), async (req, res) => {
try {
const tokens = await db('api_tokens')
// Scope to the caller's own tokens unless super_admin — the previous
// query returned every admin's token metadata (name/preview/scopes/
// owner) to any settings.view holder (GHSA-jm7j).
const tokensQuery = db('api_tokens')
.leftJoin('admin_users', 'admin_users.id', 'api_tokens.created_by')
.select(
'api_tokens.id',
@@ -33,6 +36,10 @@ router.get('/', adminAuth, requirePermission('settings.view'), async (req, res)
'admin_users.username as owner_username'
)
.orderBy('api_tokens.created_at', 'desc');
if (req.admin.roleName !== 'super_admin') {
tokensQuery.where('api_tokens.created_by', req.admin.id);
}
const tokens = await tokensQuery;
res.json(tokens);
} catch (error) {
logger.error('Failed to list API tokens', { error: error.message });
@@ -101,6 +108,12 @@ router.delete('/:id', adminAuth, requirePermission('settings.edit'), async (req,
const { id } = req.params;
const row = await db('api_tokens').where({ id }).first();
if (!row) return res.status(404).json({ error: 'Token not found' });
// Only the token's owner (or a super_admin) may revoke it — otherwise
// any settings.edit holder could revoke another admin's tokens
// (GHSA-gprq). 404 rather than 403 so a non-owner can't probe token ids.
if (req.admin.roleName !== 'super_admin' && row.created_by !== req.admin.id) {
return res.status(404).json({ error: 'Token not found' });
}
if (row.revoked_at) return res.status(400).json({ error: 'Token already revoked' });
await db('api_tokens').where({ id }).update({ revoked_at: new Date() });
+13 -4
View File
@@ -8,7 +8,7 @@ const { endSession } = require('../middleware/sessionTimeout');
const { validatePasswordStrength } = require('../utils/passwordGenerator');
const { handleAsync, validateRequest, successResponse } = require('../utils/routeHelpers');
const { NotFoundError, ConflictError, ValidationError } = require('../utils/errors');
const { setAdminAuthCookie } = require('../utils/tokenUtils');
const { setAdminAuthCookie, clearAdminAuthCookie } = require('../utils/tokenUtils');
const { IDENTITY_PRESERVING_NORMALIZE_EMAIL } = require('../utils/emailNormalization');
const mfaService = require('../services/mfaService');
const router = express.Router();
@@ -168,12 +168,21 @@ router.post('/change-password', [
// Logout
router.post('/logout', adminAuth, handleAsync(async (req, res) => {
// Get token from header
const token = req.headers.authorization?.split(' ')[1];
// Use the token adminAuth actually authenticated with (req.token) — it may
// have come from the admin_token cookie, not the Authorization header. The
// old header-only read skipped revocation entirely for cookie-based logout,
// leaving the JWT valid until expiry while reporting a successful logout.
const token = req.token;
if (token) {
// End the session
// End the in-memory session AND revoke the JWT (GHSA-cjqh) — the token
// is otherwise valid until expiry, so photoAuth/adminAuth would keep
// honouring it after logout. isTokenRevoked() checks this store.
endSession(token);
const { revokeToken } = require('../utils/tokenRevocation');
await revokeToken(token, 'logout');
}
// Clear the auth cookie so the browser stops sending the (now revoked) JWT.
clearAdminAuthCookie(res);
// Log activity
await logActivity('admin_logout',
+33 -7
View File
@@ -1,7 +1,7 @@
const express = require('express');
const { db } = require('../database/db');
const { adminAuth } = require('../middleware/auth');
const { requirePermission } = require('../middleware/permissions');
const { requirePermission, requireSuperAdmin } = require('../middleware/permissions');
const { clearAdminAuthCookie } = require('../utils/tokenUtils');
const { revokeToken } = require('../utils/tokenRevocation');
const { triggerManualBackup, getBackupStatus, cleanupOldBackupRuns, getBackupManifest, validateBackupManifest } = require('../services/backupService');
@@ -57,14 +57,33 @@ router.put('/config', adminAuth, requirePermission('backup.create'), async (req,
}
break;
case 's3':
if (!updates.backup_s3_endpoint || !updates.backup_s3_bucket ||
if (!updates.backup_s3_endpoint || !updates.backup_s3_bucket ||
!updates.backup_s3_access_key || !updates.backup_s3_secret_key) {
return res.status(400).json({ error: 'S3 backup requires endpoint, bucket, and credentials' });
}
break;
}
}
// SSRF: validate an S3 endpoint whenever one is supplied — NOT only when
// the payload also flips backup_destination_type to 's3'. The PUT
// persists every backup_* field independently, so with S3 already
// selected a caller could PATCH just backup_s3_endpoint to a
// private-resolving host; the management ops (manifest, bucket/file
// browse, cleanup, test-upload) then connect without going through
// testConnection. Prod-only; dev points at localhost MinIO deliberately.
if (process.env.NODE_ENV === 'production'
&& updates.backup_s3_endpoint && updates.backup_s3_endpoint !== '••••••••') {
const rawEndpoint = updates.backup_s3_endpoint;
const withProto = /^https?:\/\//.test(rawEndpoint) ? rawEndpoint : `https://${rawEndpoint}`;
let epHost = null;
try { epHost = new URL(withProto).hostname; } catch { epHost = null; }
const { isHostAllowed } = require('../utils/networkValidation');
if (!epHost || !(await isHostAllowed(epHost))) {
return res.status(400).json({ error: 'S3 endpoint resolves to a private or internal network address' });
}
}
// Update settings
for (const [key, value] of Object.entries(updates)) {
if (key.startsWith('backup_')) {
@@ -139,7 +158,12 @@ router.post('/run', adminAuth, requirePermission('backup.create'), async (req, r
// SECURITY: the file contains plaintext secrets (SMTP password, admin password
// hashes, API keys). The download UI must warn before offering it. We surface
// the flag as a response header too so the client can double-confirm.
router.get('/picpeak/export', adminAuth, requirePermission('backup.create'), async (req, res) => {
// Full-instance export dumps every table unredacted — bcrypt password
// hashes, 2FA columns, and all integration secrets (SMTP/SSO/WhatsApp/
// webhook/S3) in cleartext. The built-in `admin` role holds backup.create,
// but is denied this data everywhere else (config APIs mask secrets as
// ********). Gate the raw dump behind super_admin (GHSA-pv6w-rj34-wj9v).
router.get('/picpeak/export', adminAuth, requireSuperAdmin(), async (req, res) => {
const fsSync = require('fs');
try {
const includePhotos = req.query.includePhotos === 'true' || req.query.includePhotos === '1';
@@ -351,9 +375,11 @@ router.post('/test-connection', adminAuth, requirePermission('backup.create'), a
break;
}
// SSRF protection: block connections to private/internal addresses
const { isPrivateIP } = require('../utils/networkValidation');
if (isPrivateIP(host)) {
// SSRF protection: resolve the host and block any private/internal
// address. ssh does its own DNS at connect time, so a literal-only
// check let a hostname resolving to an internal IP through (#GHSA-4jh8).
const { isHostAllowed } = require('../utils/networkValidation');
if (!(await isHostAllowed(host))) {
res.json({ success: false, message: 'Host cannot be a private or internal network address' });
break;
}
+19 -4
View File
@@ -141,8 +141,18 @@ router.put('/:id', adminAuth, requirePermission('settings.edit'), [
.trim()
};
// Update hero_photo_id if provided (including null to clear it)
// Update hero_photo_id if provided (including null to clear it). A
// non-null hero must belong to this category (GHSA-j2f4) — the general
// update path previously wrote it with no membership check at all.
if (Object.prototype.hasOwnProperty.call(req.body, 'hero_photo_id')) {
if (hero_photo_id) {
const heroPhoto = await db('photos')
.where({ id: hero_photo_id, category_id: id })
.first();
if (!heroPhoto) {
return res.status(404).json({ error: 'Photo not found in this category' });
}
}
updateData.hero_photo_id = hero_photo_id || null;
}
@@ -192,11 +202,16 @@ router.put('/:id/hero', adminAuth, requirePermission('settings.edit'), [
return res.status(404).json({ error: 'Category not found' });
}
// If hero_photo_id is provided, verify it belongs to a photo in this category
// If hero_photo_id is provided, verify the photo actually belongs to
// THIS category — checking existence alone let an admin point a
// category's hero at a photo from a different category or event
// (GHSA-j2f4).
if (hero_photo_id) {
const photo = await db('photos').where('id', hero_photo_id).first();
const photo = await db('photos')
.where({ id: hero_photo_id, category_id: id })
.first();
if (!photo) {
return res.status(404).json({ error: 'Photo not found' });
return res.status(404).json({ error: 'Photo not found in this category' });
}
}
+40 -2
View File
@@ -11,6 +11,8 @@ const { body, param, query } = require('express-validator');
const { adminAuth } = require('../middleware/auth');
const { requirePermission } = require('../middleware/permissions');
const { requireFeatureFlag } = require('../middleware/requireFeatureFlag');
const { filterOwnedEventIds } = require('../middleware/ownership');
const { db } = require('../database/db');
// Hour-entry routes are gated by the hoursLogging master so a direct API hit
// can't read/edit/delete/bill logged hours while the feature is off (the
@@ -528,9 +530,45 @@ router.put('/:id/events', [
body('event_ids.*').isInt({ min: 1 }),
], handleAsync(async (req, res) => {
validateRequest(req);
const customerId = parseInt(req.params.id, 10);
const submitted = req.body.event_ids.map(Number);
// The customer's CURRENT assignments. The "Manage galleries" dialog submits
// the full initial list back — including any events owned by OTHER admins —
// so we need this to tell "retain an existing foreign assignment" apart from
// "newly grant a foreign event".
const existingEventIds = (await db('event_customer_assignments')
.where('customer_account_id', customerId)
.pluck('event_id')).map(Number);
const existingSet = new Set(existingEventIds);
// Events the caller may act on (GHSA-xr6x). A denied id is only acceptable
// when the customer ALREADY has that assignment (a foreign event the caller
// is merely keeping); a denied id that isn't already assigned is a fresh
// attempt to mint access to a foreign/nonexistent event → reject.
const { allowed } = await filterOwnedEventIds(req.admin, submitted);
const allowedSet = new Set(allowed.map(Number));
const illegalNew = submitted.filter((id) => !allowedSet.has(id) && !existingSet.has(id));
if (illegalNew.length) {
return res.status(403).json({ error: 'One or more events are not yours to assign' });
}
// setAssignmentsForCustomer replaces the FULL assignment list, deleting any
// existing row not in the submitted set. A restricted admin must not be able
// to revoke another admin's customer↔event links that way, so always retain
// the customer's existing assignments to events the caller does NOT own —
// regardless of whether the client echoed them back. super_admin owns
// everything, so nothing is force-preserved for them.
let finalEventIds = allowed.map(Number);
if (req.admin.roleName !== 'super_admin' && existingEventIds.length) {
const { allowed: ownedExisting } = await filterOwnedEventIds(req.admin, existingEventIds);
const ownedExistingSet = new Set(ownedExisting.map(Number));
const foreignExisting = existingEventIds.filter((id) => !ownedExistingSet.has(id));
finalEventIds = [...new Set([...finalEventIds, ...foreignExisting])];
}
const result = await customerAccountsService.setAssignmentsForCustomer(
parseInt(req.params.id, 10),
req.body.event_ids,
customerId,
finalEventIds,
req.admin.id,
);
successResponse(res, result);
+16 -14
View File
@@ -66,9 +66,11 @@ router.post('/config', [
tls_reject_unauthorized
} = req.body;
// Validate SMTP host is not a private/internal address (SSRF protection)
const { isPrivateIP } = require('../utils/networkValidation');
if (isPrivateIP(smtp_host)) {
// Validate SMTP host is not a private/internal address (SSRF protection).
// Resolves DNS so a public-looking hostname pointing at an internal IP
// is caught, not just literal private addresses (#GHSA-ch64).
const { isHostAllowed } = require('../utils/networkValidation');
if (!(await isHostAllowed(smtp_host))) {
return res.status(400).json({ error: 'SMTP host cannot point to a private or internal network address' });
}
@@ -152,8 +154,8 @@ router.post('/incoming-config', [
const errors = validationResult(req);
if (!errors.isEmpty()) return res.status(400).json({ errors: errors.array() });
const { imap_host, imap_port, imap_secure, imap_user, imap_pass, imap_folder } = req.body;
const { isPrivateIP } = require('../utils/networkValidation');
if (isPrivateIP(imap_host)) {
const { isHostAllowed } = require('../utils/networkValidation');
if (!(await isHostAllowed(imap_host))) {
return res.status(400).json({ error: 'IMAP host cannot point to a private or internal network address' });
}
const existing = await db('email_configs').first();
@@ -183,8 +185,8 @@ router.post('/incoming-config/folders', adminAuth, requirePermission('email.view
try {
const { imap_host, imap_port, imap_secure, imap_user, imap_pass } = req.body || {};
if (imap_host) {
const { isPrivateIP } = require('../utils/networkValidation');
if (isPrivateIP(imap_host)) {
const { isHostAllowed } = require('../utils/networkValidation');
if (!(await isHostAllowed(imap_host))) {
return res.status(400).json({ error: 'IMAP host cannot point to a private or internal network address' });
}
}
@@ -205,8 +207,8 @@ router.post('/incoming-config/test', adminAuth, requirePermission('email.view'),
try {
const { imap_host, imap_port, imap_secure, imap_user, imap_pass, imap_folder } = req.body || {};
if (imap_host) {
const { isPrivateIP } = require('../utils/networkValidation');
if (isPrivateIP(imap_host)) {
const { isHostAllowed } = require('../utils/networkValidation');
if (!(await isHostAllowed(imap_host))) {
return res.status(400).json({ error: 'IMAP host cannot point to a private or internal network address' });
}
}
@@ -385,11 +387,11 @@ router.post('/accounts', adminAuth, messagingGate, requirePermission('email.edit
if (!b.account_key) return res.status(400).json({ error: 'account_key is required' });
// SSRF guard — mirror /config + /incoming-config: neither the IMAP nor the
// SMTP host may point at a private/internal address.
const { isPrivateIP } = require('../utils/networkValidation');
if (b.imap_host && isPrivateIP(b.imap_host)) {
const { isHostAllowed } = require('../utils/networkValidation');
if (b.imap_host && !(await isHostAllowed(b.imap_host))) {
return res.status(400).json({ error: 'IMAP host cannot point to a private or internal network address' });
}
if (b.smtp_host && isPrivateIP(b.smtp_host)) {
if (b.smtp_host && !(await isHostAllowed(b.smtp_host))) {
return res.status(400).json({ error: 'SMTP host cannot point to a private or internal network address' });
}
const patch = {
@@ -434,8 +436,8 @@ router.post('/accounts', adminAuth, messagingGate, requirePermission('email.edit
router.post('/accounts/test', adminAuth, messagingGate, requirePermission('email.view'), async (req, res) => {
try {
const b = req.body || {};
const { isPrivateIP } = require('../utils/networkValidation');
if (b.imap_host && isPrivateIP(b.imap_host)) {
const { isHostAllowed } = require('../utils/networkValidation');
if (b.imap_host && !(await isHostAllowed(b.imap_host))) {
return res.status(400).json({ error: 'IMAP host cannot point to a private or internal network address' });
}
let pass = b.imap_pass;
+55 -4
View File
@@ -1269,6 +1269,52 @@ module.exports = (router) => {
const { id } = req.params;
const updates = { ...req.body };
// Strip identity/provenance/secret columns from the mass-assigned
// body (GHSA-3rqx). The handler spreads req.body straight into the
// events UPDATE, so without this an events.edit holder could rewrite
// ownership (created_by), routing identity (slug/share_link), the
// share/client tokens, or the password hashes directly. Plaintext
// `password`/`client_password` inputs are NOT stripped — those are the
// supported way to change credentials and get hashed below; the
// tokens are regenerated internally where needed.
// The handler spreads req.body straight into the events UPDATE, so any
// column an events.edit holder names is writable unless blocked here.
// This is a COMPLETE deny-set of every server-managed / permission-gated
// events column (enumerated from the schema); everything else is a
// legitimate edit-form field and passes through, including input-only
// keys (password/client_password) the handler transforms below. New
// server-managed columns MUST be added here. (codex review — GHSA-3rqx.)
const IMMUTABLE_EVENT_COLUMNS = [
// Identity / provenance
'id', 'created_by', 'created_at', 'updated_at', 'slug',
// Routing + share/client tokens (generated at create / internally)
'share_link', 'share_token', 'client_share_token', 'show_share_token',
// Secrets (set via the plaintext password/client_password inputs)
'password_hash', 'client_password_hash',
// Server-consumed file paths — e.g. DELETE /:id/logo fs.unlink()s
// hero_logo_path, so a forged value is an arbitrary-delete primitive.
'hero_logo_path', 'hero_logo_url', 'archive_path', 'download_zip_path',
// Server-managed timestamps
'download_zip_generated_at', 'archived_at', 'revealed_at', 'event_reminder_sent_at',
// Lifecycle — governed by dedicated permission-gated routes
// (events.archive/restore, publish, activate/deactivate), not events.edit.
'is_archived', 'is_draft', 'is_active',
// Relationships — managed by projectService.assignEvent + its
// customer-consistency checks, and events.edit ≠ quotes/contracts perms.
'project_id', 'quote_id',
// Legacy mirrors — rejected explicitly below in favour of customer_*.
'host_name', 'host_email',
];
// Case-insensitive match: SQLite treats quoted identifiers
// case-insensitively, so a `{ "Password_Hash": ... }` key would
// otherwise survive a case-sensitive delete and still hit the real
// column (codex review).
const denied = new Set(IMMUTABLE_EVENT_COLUMNS.map((c) => c.toLowerCase()));
for (const key of Object.keys(updates)) {
if (denied.has(key.toLowerCase())) delete updates[key];
}
const customerColumnsAvailable = await hasCustomerContactColumns();
if (Object.prototype.hasOwnProperty.call(updates, 'host_name') || Object.prototype.hasOwnProperty.call(updates, 'host_email')) {
@@ -1497,10 +1543,15 @@ module.exports = (router) => {
}
}
// Update event
await db('events')
.where('id', id)
.update(updates);
// Update event. Skip the write when the denylist (or masked secrets)
// left nothing to change — Knex rejects .update({}) with an error,
// which would surface as a 500 for an otherwise-valid no-op request
// (e.g. a body of only protected fields). (codex review.)
if (Object.keys(updates).length > 0) {
await db('events')
.where('id', id)
.update(updates);
}
// Customer-account assignments (#354). Same skip semantics as POST:
// ignore when the customer portal flag is off so stale tabs don't
+39 -8
View File
@@ -1,4 +1,5 @@
const express = require('express');
const { neutralizeSpreadsheetFormula } = require('../utils/spreadsheetSafe');
const router = express.Router();
const { adminAuth } = require('../middleware/auth');
const { requirePermission } = require('../middleware/permissions');
@@ -164,6 +165,24 @@ router.get('/events/:eventId/feedback',
}
);
// Ownership guard for by-feedback-id routes (GHSA-2qc2 / GHSA-32h4). These
// take a :feedbackId (not :eventId), so requireEventOwnership can't apply —
// resolve the feedback's event and enforce the same rule (super_admin sees
// all; others need to own the event, or it's ownerless/legacy). Returns
// false and sends a 404 (not 403 — don't leak which feedback ids exist)
// when the caller may not act on it.
async function assertOwnsFeedback(req, res, feedbackId) {
if (req.admin.roleName === 'super_admin') return true;
const fb = await db('photo_feedback').where('id', feedbackId).first('event_id');
if (!fb) { res.status(404).json({ error: 'Feedback not found' }); return false; }
const event = await db('events').where('id', fb.event_id).first('created_by');
if (event && event.created_by && event.created_by !== req.admin.id) {
res.status(404).json({ error: 'Feedback not found' });
return false;
}
return true;
}
// Moderate feedback (approve/hide/reject)
router.put('/feedback/:feedbackId/:action',
adminAuth,
@@ -171,11 +190,12 @@ router.put('/feedback/:feedbackId/:action',
async (req, res) => {
try {
const { feedbackId, action } = req.params;
if (!['approve', 'hide', 'reject'].includes(action)) {
return res.status(400).json({ error: 'Invalid action' });
}
if (!(await assertOwnsFeedback(req, res, feedbackId))) return;
await feedbackService.moderateFeedback(feedbackId, action, req.admin.id);
res.json({ success: true });
@@ -193,7 +213,8 @@ router.delete('/feedback/:feedbackId',
async (req, res) => {
try {
const { feedbackId } = req.params;
if (!(await assertOwnsFeedback(req, res, feedbackId))) return;
await feedbackService.deleteFeedback(feedbackId, req.admin.id);
res.json({ success: true });
@@ -347,7 +368,15 @@ router.get('/feedback/pending-moderation',
requirePermission('events.view'),
async (req, res) => {
try {
const pending = await feedbackService.getPendingModeration();
// Scope to the caller's owned events unless super_admin (GHSA-3335).
let ownedEventIds = null;
if (req.admin.roleName !== 'super_admin') {
const rows = await db('events')
.where((q) => q.whereNull('created_by').orWhere('created_by', req.admin.id))
.select('id');
ownedEventIds = rows.map((r) => r.id);
}
const pending = await feedbackService.getPendingModeration(null, ownedEventIds);
res.json(pending);
} catch (error) {
logger.error('Error getting pending moderation:', error);
@@ -450,11 +479,13 @@ function convertToCSV(data) {
const value = row[header];
if (value === null || value === undefined) return '';
if (typeof value === 'boolean') return value ? 'yes' : 'no';
if (typeof value === 'string'
&& (value.includes(',') || value.includes('"') || value.includes('\n') || value.includes('\r'))) {
return `"${value.replace(/"/g, '""')}"`;
// Formula-neutralize user-controlled cells (guest_name/comment_text)
// before quoting — quoting alone doesn't stop `=cmd()` (GHSA-3cw3).
const neutralized = neutralizeSpreadsheetFormula(value);
if (neutralized.includes(',') || neutralized.includes('"') || neutralized.includes('\n') || neutralized.includes('\r')) {
return `"${neutralized.replace(/"/g, '""')}"`;
}
return value;
return neutralized;
}).join(',');
});
+6 -1
View File
@@ -39,8 +39,13 @@ function serializeGuest(row) {
};
}
const { neutralizeSpreadsheetFormula } = require('../utils/spreadsheetSafe');
function escapeCsvCell(value) {
const str = value == null ? '' : String(value);
// Neutralize spreadsheet formulas FIRST (a `=cmd()` guest name executes on
// open — RFC-4180 quoting doesn't stop it), then quote-wrap (GHSA-wc99 /
// GHSA-f4fp).
const str = neutralizeSpreadsheetFormula(value);
if (/[,"\n\r]/.test(str)) {
return `"${str.replace(/"/g, '""')}"`;
}
+11 -2
View File
@@ -502,6 +502,15 @@ router.get('/export', adminAuth, requirePermission('settings.view'), async (req,
/**
* Helper function to convert data to CSV
*/
const { neutralizeSpreadsheetFormula } = require('../utils/spreadsheetSafe');
function csvCell(value) {
// Formula-neutralize, then RFC-4180 quote (the previous join('') did
// neither — GHSA-37p4).
const s = neutralizeSpreadsheetFormula(value);
return /[,"\n\r]/.test(s) ? `"${s.replace(/"/g, '""')}"` : s;
}
function convertToCSV(data) {
// Simplified CSV conversion for security logs
const headers = ['timestamp', 'event_type', 'client_ip', 'details'];
@@ -511,8 +520,8 @@ function convertToCSV(data) {
log.client_ip,
JSON.stringify(log.details || {})
]);
return [headers.join(','), ...rows.map(row => row.join(','))].join('\n');
return [headers.join(','), ...rows.map(row => row.map(csvCell).join(','))].join('\n');
}
module.exports = router;
+88 -5
View File
@@ -315,9 +315,10 @@ router.post('/:eventId/upload', adminAuth, requirePermission('photos.upload'), r
const crypto = require('crypto');
const uploadId = crypto.randomBytes(16).toString('hex');
// Counter base — same approximation as before. Strict uniqueness is
// already enforced by the filename template + DB unique index, so a
// small race here just retries a counter on conflict (rare).
// Counter base — a per-request approximation (concurrent upload
// requests can compute the same base; there is NO unique index on
// photos.filename). Uniqueness of the final path comes from the
// random suffix inside generatePhotoFilename (#931).
const existingCount = await db('photos')
.where({ event_id: eventId, type: photoType })
.count('id as count')
@@ -756,6 +757,16 @@ router.patch('/:eventId/photos/:photoId', adminAuth, requirePermission('photos.e
.where({ id: photoId, event_id: eventId })
.update(updateData);
// A visibility or category change alters which photos belong in the
// guest download bundle — drop the cached ZIP so it rebuilds fresh,
// otherwise a hide→unhide cycle can leave the stale cache omitting
// photos added in between (codex review).
if (updateData.visibility !== undefined
|| Object.prototype.hasOwnProperty.call(updateData, 'category_id')
|| Object.prototype.hasOwnProperty.call(updateData, 'type')) {
downloadZipService.invalidate(parseInt(eventId, 10));
}
// Fetch and return the updated photo
const updatedPhoto = await db('photos')
.where({ id: photoId, event_id: eventId })
@@ -904,6 +915,14 @@ router.post('/:eventId/photos/bulk-update', adminAuth, requirePermission('photos
.where('event_id', eventId)
.update(updateData);
// Visibility/category changes alter the guest download bundle — drop the
// cached ZIP so it rebuilds fresh (codex review).
if (updateData.visibility !== undefined
|| Object.prototype.hasOwnProperty.call(updateData, 'category_id')
|| Object.prototype.hasOwnProperty.call(updateData, 'type')) {
downloadZipService.invalidate(parseInt(eventId, 10));
}
res.json({ message: `${photoIds.length} photos updated successfully` });
} catch (error) {
errorResponse(res, error, 500, 'Failed to update photos');
@@ -1091,7 +1110,13 @@ router.get('/:eventId/photos', adminAuth, requirePermission('photos.view'), requ
average_rating: photo.average_rating || 0,
comment_count: commentMap[photo.id] || 0,
like_count: photo.like_count || 0,
favorite_count: photo.favorite_count || 0
favorite_count: photo.favorite_count || 0,
// Engagement counters (#895 follow-up): the grid reads these, but
// this explicit mapper never included them — so the Engagement
// column showed 0 regardless of what the DB counted. This, not
// stale data, was why per-image downloads always displayed 0.
view_count: photo.view_count || 0,
download_count: photo.download_count || 0
}))
});
} catch (error) {
@@ -1126,7 +1151,65 @@ router.get('/:eventId/photo/:photoId', adminAuth, requirePermission('photos.view
const event = await db('events').where('id', eventId).first();
const storageKey = resolvePhotoStorageKey(event, photo);
res.setHeader('Content-Type', `image/${path.extname(photo.filename).slice(1)}`);
// Content-Type resolution (#908 + external review). Invariant: the
// header is ALWAYS image/* or video/*.
// - photos.mime_type is never echoed verbatim unless it is a video/
// type: the chunked-upload path stores the client-sent MIME
// unvalidated, so a stored text/html served inline under the app
// origin would be a same-origin XSS gift.
// - Images ignore the stored value entirely — migration 039
// backfilled image/jpeg onto every legacy row (PNGs included), so
// the extension is the more trustworthy signal; normalized via the
// shared map (image/jpg → image/jpeg), jpeg fallback when unknown.
// - Videos prefer a stored video/ type, then the extension map
// (.mov → video/quicktime, .webm → video/webm, …), then video/mp4.
// The old ext-derived image/<ext> (image/mp4) is what made the
// admin player's blob unplayable (#908).
const { EXTENSION_TO_MIME } = require('../services/uploadSettings');
const ext = path.extname(photo.filename).slice(1).toLowerCase();
// 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
// 'video/' is equally invalid; both fall back to the extension map.
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/<token> 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/'));
// 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 <img>/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) || storedImageMime || 'image/jpeg';
res.setHeader('Content-Type', contentType);
res.setHeader('Cache-Control', 'private, max-age=3600');
res.setHeader('Cross-Origin-Resource-Policy', 'cross-origin');
+35 -5
View File
@@ -15,8 +15,31 @@ const { requirePermission, userHasAnyPermission } = require('../middleware/permi
const { handleAsync, validateRequest, successResponse } = require('../utils/routeHelpers');
const projectService = require('../services/projectService');
const { db } = require('../database/db');
const { ForbiddenError } = require('../utils/errors');
const router = express.Router();
// A deal that spans both quotes and contracts cascades a project link across
// BOTH tables (projectService.linkDealToProject). So attaching one document
// must also require manage permission on the OTHER domain the cascade will
// touch — otherwise quotes.manage alone could re-point a linked contract, and
// vice versa (GHSA-v4vw / codex review). No-op when the deal touches only the
// one domain, or on older instances without the deal_uuid column.
async function assertCascadePermitted(req, docTable, docId, otherTable, otherPerm) {
let doc;
try {
doc = await db(docTable).where({ id: docId }).first('deal_uuid');
} catch { return; }
if (!doc || !doc.deal_uuid) return;
let linked;
try {
linked = await db(otherTable).where({ deal_uuid: doc.deal_uuid }).first('id');
} catch { return; }
if (!linked) return;
if (!(await userHasAnyPermission(req.admin.id, [otherPerm]))) {
throw new ForbiddenError(`This deal also links a ${otherTable.replace(/s$/, '')}; the ${otherPerm} permission is required`);
}
}
router.use(adminAuth);
// Projects is feature-flagged like bills/quotes — when off, the whole cockpit
@@ -105,23 +128,30 @@ router.post('/:id/events',
);
// Attach a quote to the project (quotes carry no event_id — migration 121).
// Requires quotes.manage in addition to events.edit — attaching a quote
// mutates a separately-permissioned document domain (GHSA-v4vw).
router.post('/:id/quotes',
requirePermission('events.edit'),
requirePermission(['events.edit', 'quotes.manage'], { requireAll: true }),
[param('id').isInt({ min: 1 }), body('quoteId').isInt({ min: 1 })],
handleAsync(async (req, res) => {
validateRequest(req);
const result = await projectService.assignQuote(parseInt(req.params.id, 10), parseInt(req.body.quoteId, 10));
const quoteId = parseInt(req.body.quoteId, 10);
await assertCascadePermitted(req, 'quotes', quoteId, 'contracts', 'contracts.manage');
const result = await projectService.assignQuote(parseInt(req.params.id, 10), quoteId);
return successResponse(res, result, 200, 'Quote attached to project');
}),
);
// Attach a contract to the project.
// Attach a contract to the project. Requires contracts.manage in addition
// to events.edit (GHSA-v4vw).
router.post('/:id/contracts',
requirePermission('events.edit'),
requirePermission(['events.edit', 'contracts.manage'], { requireAll: true }),
[param('id').isInt({ min: 1 }), body('contractId').isInt({ min: 1 })],
handleAsync(async (req, res) => {
validateRequest(req);
const result = await projectService.assignContract(parseInt(req.params.id, 10), parseInt(req.body.contractId, 10));
const contractId = parseInt(req.body.contractId, 10);
await assertCascadePermitted(req, 'contracts', contractId, 'quotes', 'quotes.manage');
const result = await projectService.assignContract(parseInt(req.params.id, 10), contractId);
return successResponse(res, result, 200, 'Contract attached to project');
}),
);
+5 -5
View File
@@ -20,7 +20,7 @@ const { body, query, validationResult } = require('express-validator');
const { db, logActivity } = require('../database/db');
const { adminAuth } = require('../middleware/auth');
const { requirePermission } = require('../middleware/permissions');
const { validateExternalUrl } = require('../utils/networkValidation');
const { validateExternalUrlAsync } = require('../utils/networkValidation');
const webhookService = require('../services/webhookService');
const logger = require('../utils/logger');
@@ -78,9 +78,9 @@ router.post(
requirePermission('settings.edit'),
[
body('name').isString().trim().isLength({ min: 1, max: 100 }),
body('url').isString().isLength({ max: 2048 }).custom((url) => {
body('url').isString().isLength({ max: 2048 }).custom(async (url) => {
if (ALLOW_PRIVATE_URLS) return true;
const check = validateExternalUrl(url);
const check = await validateExternalUrlAsync(url);
if (!check.valid) throw new Error(check.error);
return true;
}),
@@ -160,9 +160,9 @@ router.put(
requirePermission('settings.edit'),
[
body('name').optional().isString().trim().isLength({ min: 1, max: 100 }),
body('url').optional().isString().isLength({ max: 2048 }).custom((url) => {
body('url').optional().isString().isLength({ max: 2048 }).custom(async (url) => {
if (ALLOW_PRIVATE_URLS) return true;
const check = validateExternalUrl(url);
const check = await validateExternalUrlAsync(url);
if (!check.valid) throw new Error(check.error);
return true;
}),
+69 -25
View File
@@ -30,6 +30,7 @@ const { handleAsync, errorResponse } = require('../utils/routeHelpers');
const { NotFoundError } = require('../utils/errors');
const { ensureThumbnail, ensureHeroImage, ensurePreviewImage, withLocalCopy } = require('../services/imageProcessor');
const downloadZipService = require('../services/downloadZipService');
const { applyPhotoVisibilityFilter, canSeeHiddenPhotos } = require('../utils/photoVisibility');
const {
getUseOriginalFilenames,
pickRawDownloadName,
@@ -83,9 +84,20 @@ router.get('/resolve/:identifier', handleAsync(async (req, res) => {
}
const { event, matchType, shareToken } = result;
const linkVariants = await buildShareLinkVariants({ slug: event.slug, shareToken });
const requiresPassword = !(event.require_password === false || event.require_password === 0 || event.require_password === '0');
// The share_token is a bearer secret. Only return it (and the share
// links/URLs that embed it) when the caller already proved they hold it —
// i.e. they resolved via the token or the full share link. A bare *slug*
// lookup (slugs appear in gallery URLs and are guessable) must NOT hand
// back the secret, or an anonymous caller could turn a known slug into
// share-link access to a no-password gallery (GHSA-rh8r).
const callerHasToken = matchType !== 'slug';
if (!callerHasToken) {
return res.json({ slug: event.slug, matchType, requires_password: requiresPassword });
}
const linkVariants = await buildShareLinkVariants({ slug: event.slug, shareToken });
res.json({
slug: event.slug,
token: shareToken,
@@ -784,6 +796,10 @@ router.patch('/:slug/photos/:photoId/visibility', verifyGalleryAccess, async (re
.where({ id: photoId, event_id: req.event.id })
.update({ visibility });
// A client hiding/showing a photo changes the guest download bundle —
// drop the cached ZIP so it rebuilds fresh (codex review).
downloadZipService.invalidate(req.event.id);
res.json({ message: 'Photo visibility updated', visibility });
} catch (error) {
errorResponse(res, error, 500, 'Failed to update photo visibility');
@@ -812,6 +828,10 @@ router.patch('/:slug/photos/visibility/bulk', verifyGalleryAccess, async (req, r
.where('event_id', req.event.id)
.update({ visibility });
// Client bulk hide/show alters the guest download bundle — invalidate
// the cached ZIP (codex review).
downloadZipService.invalidate(req.event.id);
res.json({ message: `${count} photos updated`, visibility });
} catch (error) {
errorResponse(res, error, 500, 'Failed to update photo visibility');
@@ -956,8 +976,21 @@ router.get('/:slug/download-all', verifyGalleryAccess, denySlideshowToken, async
return res.status(403).json({ error: 'Downloads are disabled for this gallery' });
}
// Try to serve pre-generated zip (instant download with Content-Length)
const zipInfo = await downloadZipService.getZipInfo(req.event.id);
// Try to serve pre-generated zip (instant download with Content-Length).
// Guests may use the prebuilt cache ONLY when the event has no hidden
// photos: a cache built before a photo was hidden — or before this
// visibility-aware builder shipped — could otherwise still leak it, and
// getZipInfo only checks the DB pointer + file stat, not freshness. When
// hidden photos exist, guests fall through to the visibility-filtered
// stream below. PIN-clients always stream a full archive.
const isClient = canSeeHiddenPhotos(req.accessLevel);
const eventHasHidden = await db('photos')
.where({ event_id: req.event.id, visibility: 'hidden' })
.first()
.then(Boolean);
const zipInfo = (isClient || eventHasHidden)
? null
: await downloadZipService.getZipInfo(req.event.id);
if (zipInfo) {
const storage = getStorage();
@@ -1006,23 +1039,31 @@ router.get('/:slug/download-all', verifyGalleryAccess, denySlideshowToken, async
return;
}
// Fallback: on-the-fly streaming (existing behavior)
// Also trigger background zip generation for next time
downloadZipService.generateZip(req.event.id).catch(err =>
logger.warn('Background zip generation failed', { eventId: req.event.id, error: err.message })
);
// Fallback: on-the-fly streaming (existing behavior). Only pre-build the
// guest cache when it will actually be served next time — a guest
// download of an event with no hidden photos. Client bypasses and
// hidden-photo events always stream, so rebuilding the guest archive on
// those requests is wasted I/O (codex review).
if (!isClient && !eventHasHidden) {
downloadZipService.generateZip(req.event.id).catch(err =>
logger.warn('Background zip generation failed', { eventId: req.event.id, error: err.message })
);
}
// Fetch photos — exclude photos in categories that disabled downloads (#640).
// Uncategorised photos are always included; categories without the column
// (pre-migration-135) fall through the LEFT JOIN's null and are included.
const photos = await db('photos')
.leftJoin('photo_categories', 'photos.category_id', 'photo_categories.id')
.where('photos.event_id', req.event.id)
.where(function () {
this.whereNull('photos.category_id')
.orWhere('photo_categories.allow_downloads', true)
.orWhereNull('photo_categories.allow_downloads');
})
const photos = await applyPhotoVisibilityFilter(
db('photos')
.leftJoin('photo_categories', 'photos.category_id', 'photo_categories.id')
.where('photos.event_id', req.event.id)
.where(function () {
this.whereNull('photos.category_id')
.orWhere('photo_categories.allow_downloads', true)
.orWhereNull('photo_categories.allow_downloads');
}),
req.accessLevel
)
.select('photos.*')
.orderBy('photos.type', 'asc')
.orderBy('photos.uploaded_at', 'desc');
@@ -1172,15 +1213,18 @@ router.post('/:slug/download-selected', verifyGalleryAccess, denySlideshowToken,
// Fetch photos — exclude photos in categories that disabled downloads (#640).
// Same LEFT JOIN pattern as the download-all endpoint.
const photos = await db('photos')
.leftJoin('photo_categories', 'photos.category_id', 'photo_categories.id')
.where('photos.event_id', req.event.id)
.whereIn('photos.id', photoIds)
.where(function () {
this.whereNull('photos.category_id')
.orWhere('photo_categories.allow_downloads', true)
.orWhereNull('photo_categories.allow_downloads');
})
const photos = await applyPhotoVisibilityFilter(
db('photos')
.leftJoin('photo_categories', 'photos.category_id', 'photo_categories.id')
.where('photos.event_id', req.event.id)
.whereIn('photos.id', photoIds)
.where(function () {
this.whereNull('photos.category_id')
.orWhere('photo_categories.allow_downloads', true)
.orWhereNull('photo_categories.allow_downloads');
}),
req.accessLevel
)
.select('photos.*')
.orderBy('photos.uploaded_at', 'desc');
+53 -14
View File
@@ -7,6 +7,7 @@ const secureImageService = require('../services/secureImageService');
const { getStorage } = require('../services/storage');
const { resolvePhotoStorageKey, resolvePhotoFilePath } = require('../services/photoResolver');
const { withLocalCopy } = require('../services/imageProcessor');
const { isPhotoHiddenFromViewer, canSeeHiddenPhotos } = require('../utils/photoVisibility');
const crypto = require('crypto');
const logger = require('../utils/logger');
const { timingSafeEqualStr } = require('../utils/timingSafe');
@@ -16,10 +17,14 @@ const router = express.Router();
/**
* Generate a signed URL token for image access
*/
function generateImageToken(photoId, expiresIn = 3600) {
function generateImageToken(photoId, expiresIn = 3600, clientBypass = false) {
const secret = process.env.JWT_SECRET;
const expires = Date.now() + (expiresIn * 1000);
const data = `${photoId}:${expires}`;
// Third segment: whether the minter was a PIN-client, letting the serve
// route still deliver a photo hidden AFTER minting (TOCTOU) — a guest's
// token carries 0, so it stops working the moment the photo is hidden.
// Old two-segment tokens verify unchanged and read the flag as no-bypass.
const data = `${photoId}:${expires}:${clientBypass ? 1 : 0}`;
const signature = crypto.createHmac('sha256', secret).update(data).digest('hex');
return `${Buffer.from(data).toString('base64')}.${signature}`;
}
@@ -32,20 +37,23 @@ function verifyImageToken(token) {
const secret = process.env.JWT_SECRET;
const [data, signature] = token.split('.');
const decoded = Buffer.from(data, 'base64').toString();
const [photoId, expires] = decoded.split(':');
const [photoId, expires, clientFlag] = decoded.split(':');
// Verify signature (constant-time — avoids leaking the HMAC byte-by-byte)
const expectedSignature = crypto.createHmac('sha256', secret).update(decoded).digest('hex');
if (!timingSafeEqualStr(signature, expectedSignature)) {
return null;
}
// Check expiration
if (Date.now() > parseInt(expires)) {
return null;
}
return { photoId: parseInt(photoId), expires: parseInt(expires) };
return {
photoId: parseInt(photoId),
expires: parseInt(expires),
clientBypass: clientFlag === '1',
};
} catch (error) {
return null;
}
@@ -79,6 +87,12 @@ router.get('/:slug/photo/:photoId/view', verifyGalleryAccess, async (req, res) =
return res.status(404).json({ error: 'Photo not found' });
}
// Block guest access to hidden/client-only photos (parity with the
// gallery single-photo routes).
if (isPhotoHiddenFromViewer(photo, req.accessLevel)) {
return res.status(403).json({ error: 'Photo not available' });
}
// Check for suspicious activity
const isSuspicious = await secureImageService.detectSuspiciousActivity(clientFingerprint, photoId);
if (isSuspicious) {
@@ -191,15 +205,23 @@ router.post('/:slug/photo/:photoId/generate-secure-token', verifyGalleryAccess,
return res.status(404).json({ error: 'Photo not found' });
}
// Don't mint a secure-image capability for a hidden/client-only photo
// when the caller isn't a client — the serve route is token-only.
if (isPhotoHiddenFromViewer(photo, req.accessLevel)) {
return res.status(403).json({ error: 'Photo not available' });
}
// Create client fingerprint
const clientFingerprint = secureImageService.createClientFingerprint(req);
// Generate secure token
// Generate secure token. clientBypass lets a client's token keep serving
// a photo hidden after minting; a guest's stops at the serve route.
const token = secureImageService.generateSecureToken(photoId, req.sessionID || 'anonymous', {
expiresIn,
maxUses: protectionLevel === 'maximum' ? 1 : 3,
clientFingerprint,
protectionLevel
protectionLevel,
clientBypass: canSeeHiddenPhotos(req.accessLevel)
});
res.json({
@@ -233,9 +255,19 @@ router.post('/:slug/photo/:photoId/generate-url', verifyGalleryAccess, async (re
if (!photo) {
return res.status(404).json({ error: 'Photo not found' });
}
// Generate signed token
const token = generateImageToken(photoId);
// Refuse to mint a signed URL for a hidden/client-only photo when the
// caller isn't a client. The signed-serve route below is token-only
// (no gallery auth), so the access decision has to happen here at mint
// time — mirroring how the reveal-bypass flag is baked into the token.
if (isPhotoHiddenFromViewer(photo, req.accessLevel)) {
return res.status(403).json({ error: 'Photo not available' });
}
// Generate signed token. The client-bypass flag lets a PIN-client's
// token keep serving a photo hidden after minting; a guest's token
// (clientBypass=0) stops the moment the photo is hidden.
const token = generateImageToken(photoId, 3600, canSeeHiddenPhotos(req.accessLevel));
const signedUrl = `/api/images/${req.params.slug}/photo/${photoId}/signed/${token}`;
res.json({
@@ -283,7 +315,14 @@ router.get('/:slug/photo/:photoId/signed/:token', async (req, res) => {
if (!photo) {
return res.status(404).json({ error: 'Photo not found' });
}
// Recheck visibility at serve time (TOCTOU): a photo hidden AFTER the
// URL was minted must stop serving, unless the token was minted by a
// client (clientBypass) — mirroring the reveal-mode check above.
if (photo.visibility === 'hidden' && !tokenData.clientBypass) {
return res.status(403).json({ error: 'Photo not available' });
}
// Get watermark settings
const watermarkSettings = await watermarkService.getWatermarkSettings();
+71 -1
View File
@@ -13,6 +13,7 @@ const {
pickRawDownloadName,
} = require('../services/downloadFilenameService');
const { buildContentDisposition } = require('../utils/filenameSanitizer');
const { isPhotoHiddenFromViewer, canSeeHiddenPhotos } = require('../utils/photoVisibility');
const router = express.Router();
@@ -40,6 +41,12 @@ router.post('/:slug/generate-token', async (req, res, next) => {
return res.status(404).json({ error: 'Photo not found' });
}
// Don't mint a secure-image capability for a hidden/client-only photo
// when the caller isn't a client (the token is reusable up to 3×).
if (isPhotoHiddenFromViewer(photo, req.accessLevel)) {
return res.status(403).json({ error: 'Photo not available' });
}
// Create client fingerprint
const clientFingerprint = secureImageService.createClientFingerprint(req);
@@ -51,7 +58,10 @@ router.post('/:slug/generate-token', async (req, res, next) => {
expiresIn: protectionLevel === 'maximum' ? 180 : 300, // 3-5 minutes
maxUses: accessType === 'download' ? 1 : 3,
clientFingerprint,
protectionLevel
protectionLevel,
// TOCTOU: a client's token keeps serving a photo hidden after minting;
// a guest's stops the moment it's hidden (checked at the serve route).
clientBypass: canSeeHiddenPhotos(req.accessLevel)
};
const token = secureImageService.generateSecureToken(
@@ -137,6 +147,34 @@ router.get('/:slug/secure/:photoId/:token',
return res.status(404).json({ error: 'Gallery not found' });
}
// Bind the token to the gallery + photo it was minted for
// (GHSA-g94x-8vv8-3c9f). This route serves via <img src> with the
// token in the URL, so it can't require verifyGalleryAccess like the
// download sibling does. Instead enforce the scope already inside the
// token: it is minted for one photoId (and photos belong to exactly
// one gallery), and its sessionId records the minting gallery's id.
// Without this, a token minted on any PUBLIC gallery reads every other
// gallery's photos with no password.
const tokenPhotoId = Number(tokenValidation.data?.photoId);
if (!Number.isInteger(tokenPhotoId) || tokenPhotoId !== Number(photoId)) {
await secureImageService.logImageAccess(
photoId, event.id, req.clientInfo, 'photo_mismatch'
);
return res.status(403).json({ error: 'Token not valid for this photo' });
}
// Defense in depth: the sessionId embeds the gallery the token was
// minted for (`gallery_public_<id>_...` / `gallery_<id>_...`). Reject a
// token whose gallery is parseable and differs from this one.
const sessionEventId = Number(
(String(tokenValidation.data?.sessionId || '').match(/^gallery_(?:public_)?(\d+)_/) || [])[1]
);
if (Number.isInteger(sessionEventId) && sessionEventId !== Number(event.id)) {
await secureImageService.logImageAccess(
photoId, event.id, req.clientInfo, 'gallery_mismatch'
);
return res.status(403).json({ error: 'Token not valid for this gallery' });
}
// Verify photo exists and belongs to event
const photo = await db('photos')
.where({ id: photoId, event_id: event.id })
@@ -146,6 +184,13 @@ router.get('/:slug/secure/:photoId/:token',
return res.status(404).json({ error: 'Photo not found' });
}
// Recheck visibility at serve time (TOCTOU): a photo hidden AFTER the
// token was minted must stop serving, unless the token was minted by a
// client (clientBypass) — mirroring the reveal-mode check above.
if (photo.visibility === 'hidden' && !tokenValidation.data?.clientBypass) {
return res.status(403).json({ error: 'Photo not available' });
}
// Resolve photo through storage backend (managed) or fall back to local
// path (external reference mode). secureImageService needs a local file,
// so we materialize a tmp copy via withLocalCopy in S3 mode.
@@ -293,6 +338,14 @@ router.get('/:slug/secure-download/:photoId/:token',
return res.status(403).json({ error: 'Invalid or expired token' });
}
// Bind the token to the photo it was minted for (GHSA-crxv) — the
// /secure serve route does this, but secure-download did not, so a
// token minted for photo A could download photo B (incl. a hidden one).
const tokenPhotoId = Number(tokenValidation.data?.photoId);
if (!Number.isInteger(tokenPhotoId) || tokenPhotoId !== Number(photoId)) {
return res.status(403).json({ error: 'Token not valid for this photo' });
}
// Verify photo exists
const photo = await db('photos')
.where({ id: photoId, event_id: req.event.id })
@@ -302,6 +355,23 @@ router.get('/:slug/secure-download/:photoId/:token',
return res.status(404).json({ error: 'Photo not found' });
}
// Block guest access to hidden/client-only photos.
if (isPhotoHiddenFromViewer(photo, req.accessLevel)) {
return res.status(403).json({ error: 'Photo not available' });
}
// Per-category download opt-out (#640) — the regular single-photo
// download enforces this too; the secure path skipped it. SQLite
// returns the boolean as numeric 0, so check both forms.
if (photo.category_id) {
const cat = await db('photo_categories')
.where('id', photo.category_id)
.first('allow_downloads');
if (cat && (cat.allow_downloads === false || cat.allow_downloads === 0)) {
return res.status(403).json({ error: 'Downloads are disabled for this category' });
}
}
// Resolve photo through storage backend (managed) or local disk (external).
const storageKey = resolvePhotoStorageKey(req.event, photo);
+6 -4
View File
@@ -1,4 +1,5 @@
const archiver = require('archiver');
const { neutralizeSpreadsheetFormula } = require('../utils/spreadsheetSafe');
const fs = require('fs');
const fsp = require('fs').promises;
const path = require('path');
@@ -261,12 +262,13 @@ function convertToCSV(data) {
const csvRows = data.map(row => {
return headers.map(header => {
const value = row[header];
// Escape quotes and wrap in quotes if contains comma
if (typeof value === 'string' && (value.includes(',') || value.includes('"'))) {
// Formula-neutralize before quoting (guest_name/comment_text are
// user-controlled); the old check didn't even escape \n/\r (GHSA-q82f).
const value = neutralizeSpreadsheetFormula(row[header]);
if (value.includes(',') || value.includes('"') || value.includes('\n') || value.includes('\r')) {
return `"${value.replace(/"/g, '""')}"`;
}
return value || '';
return value;
}).join(',');
});
+8
View File
@@ -845,6 +845,14 @@ function parseRsyncStats(output) {
async function performRsyncBackup(config, files) {
const { spawnAsync } = require('../utils/safeExec');
// SSRF: the /test-connection route validates the host, but a scheduled or
// manual /run reaches here directly with the stored host. Resolve-and-vet
// it right before ssh/rsync does its own DNS at connect time, so a host
// that resolves to an internal address can't be reached (GHSA-4jh8).
const { isHostAllowed } = require('../utils/networkValidation');
if (!(await isHostAllowed(config.backup_rsync_host))) {
throw new Error('rsync host resolves to a private or internal network address');
}
// Anchored excludes for the de-selected What-to-Backup paths; rsync
// otherwise transfers the whole storage root regardless of the walker's
// file list (which only feeds manifests and file state).
+6 -2
View File
@@ -281,8 +281,12 @@ async function cleanupExpiredUploads() {
return expiredIds.length;
}
// Run cleanup every hour
setInterval(cleanupExpiredUploads, 60 * 60 * 1000);
// Run cleanup every hour. unref so this module-level housekeeping timer
// never holds the process open on its own — in production the HTTP
// listener keeps the loop alive, and in Jest this exact handle kept the
// runner from exiting for every suite that requires adminPhotos (#908;
// it is why adminPhotos.reference sits on the CI ignore list).
setInterval(cleanupExpiredUploads, 60 * 60 * 1000).unref();
module.exports = {
initializeUpload,
@@ -112,8 +112,15 @@ class DownloadZipService {
const event = await db('events').where({ id: eventId }).first();
if (!event) return { success: false, error: 'Event not found' };
// The prebuilt zip is served to ordinary gallery guests (the
// download-all fast path), so it must exclude hidden/client-only
// photos — NULL visibility counts as visible (pre-migration rows).
// PIN-clients bypass this cache and stream a full archive instead.
const photos = await db('photos')
.where({ event_id: eventId })
.where(function () {
this.where('visibility', 'visible').orWhereNull('visibility');
})
.select('*')
.orderBy('type', 'asc')
.orderBy('uploaded_at', 'desc');
+6 -2
View File
@@ -400,7 +400,7 @@ class FeedbackService {
/**
* Get feedback requiring moderation
*/
async getPendingModeration(eventId = null) {
async getPendingModeration(eventId = null, ownedEventIds = null) {
try {
let query = db('photo_feedback')
.join('photos', 'photo_feedback.photo_id', 'photos.id')
@@ -408,9 +408,13 @@ class FeedbackService {
.where('photo_feedback.is_approved', false)
.where('photo_feedback.is_hidden', false)
.where('photo_feedback.feedback_type', 'comment');
if (eventId) {
query = query.where('photo_feedback.event_id', eventId);
} else if (Array.isArray(ownedEventIds)) {
// Scope to the caller's owned events (GHSA-3335) — an empty set
// matches nothing, so a restricted admin sees only their own.
query = query.whereIn('photo_feedback.event_id', ownedEventIds.length ? ownedEventIds : [-1]);
}
const pending = await query
+5 -1
View File
@@ -6,6 +6,7 @@
const archiver = require('archiver');
const { PassThrough } = require('stream');
const { XmpGenerator } = require('./xmpGenerator');
const { neutralizeSpreadsheetFormula } = require('../utils/spreadsheetSafe');
const { db } = require('../database/db');
const path = require('path');
const fs = require('fs').promises;
@@ -162,7 +163,10 @@ class PhotoExportService {
const csvContent = [
headers.join(','),
...rows.map(row => row.map(cell => `"${String(cell).replace(/"/g, '""')}"`).join(','))
// Formula-neutralize each cell before quoting — filenames/categories
// are user-controlled, and quoting alone doesn't stop `=cmd()`
// execution (GHSA-5364).
...rows.map(row => row.map(cell => `"${neutralizeSpreadsheetFormula(cell).replace(/"/g, '""')}"`).join(','))
].join('\n');
return {
+5 -3
View File
@@ -326,9 +326,11 @@ async function queueFilesForProcessing(files, options = {}) {
if (fileList.length === 0) return { uploadId, photos: queued, errors };
// Counter base — same approximation the upload route used pre-async.
// Strict uniqueness is still enforced by the filename template; on a
// collision the worker would just fail one photo.
// Counter base — a per-request approximation (concurrent calls can
// compute the same base). Uniqueness of the final path comes from the
// random suffix inside generatePhotoFilename (#931) — before that
// suffix, a counter collision silently overwrote the first photo's
// bytes at its already-recorded path.
const existingCount = await db('photos')
.where({ event_id: eventId, type: photoType })
.count('id as count')
+5 -2
View File
@@ -1047,7 +1047,9 @@ async function sendQuote(id, adminId) {
});
try {
await logActivity('quote_sent', { quoteId: id, token }, null, `admin:${adminId}`);
// Do NOT log the raw bearer token — it grants quote actions and the
// activity log is readable later (GHSA-prch). The quoteId is the audit key.
await logActivity('quote_sent', { quoteId: id }, null, `admin:${adminId}`);
} catch (_) {}
// Fire the quote.sent workflow trigger (best-effort; emit is fail-closed when
@@ -1272,7 +1274,8 @@ async function recordResponse({ token, action, ip, tosAccepted }) {
});
try {
await logActivity(`quote_${newStatus}`, { quoteId: quote.id, token: tokenRow.token }, null, 'customer:public');
// Raw bearer token must not reach the activity log (GHSA-prch).
await logActivity(`quote_${newStatus}`, { quoteId: quote.id }, null, 'customer:public');
} catch (_) {}
// Defer the workflow emit until the 15-min toggle window locks — so accepting
+34 -1
View File
@@ -12,6 +12,15 @@ const backupManifest = require('./backupManifest');
const S3StorageAdapter = require('./storage/s3Storage');
const { queueEmail } = require('./emailProcessor');
const { formatBoolean } = require('../utils/dbCompat');
// A manifest is attacker-influenceable (hand-crafted backup). Reject any
// entry path that would resolve OUTSIDE its intended base directory
// (traversal / absolute path) before any fs write. The target may not exist
// yet, so resolve rather than realpath (GHSA-fm58).
function pathEscapes(baseDir, candidate) {
const rel = path.relative(path.resolve(baseDir), path.resolve(candidate));
return !rel || rel === '..' || rel.startsWith('..' + path.sep) || path.isAbsolute(rel);
}
const { formatBytes } = require('../utils/formatBytes');
const os = require('os');
@@ -771,7 +780,14 @@ class RestoreService {
for (const file of filesToDownload) {
const s3Key = path.posix.join(prefix, file.path);
const localFilePath = path.join(localPath, file.path);
// Containment guard (GHSA-fm58): reject a manifest path that would
// write outside the download staging dir.
if (pathEscapes(localPath, localFilePath)) {
this.log('error', `Refusing unsafe manifest path on download: ${file.path}`);
continue;
}
await fs.mkdir(path.dirname(localFilePath), { recursive: true });
try {
@@ -1204,6 +1220,14 @@ END $$;`
const sourcePath = path.join(backupPath, file.path);
const targetPath = path.join(storagePath, file.path);
// Containment guard (GHSA-fm58): a crafted manifest path like
// `../../etc/cron.d/x` would otherwise escape the storage root and
// overwrite arbitrary files. Skip any entry that escapes.
if (pathEscapes(backupPath, sourcePath) || pathEscapes(storagePath, targetPath)) {
errors.push(`Refusing unsafe manifest path: ${file.path}`);
continue;
}
// Check if source file exists
try {
await fs.access(sourcePath);
@@ -1375,6 +1399,15 @@ END $$;`
for (const file of filesToVerify) {
const filePath = path.join(storagePath, file.path);
// Same containment guard as performFilesRestore: a traversal
// manifest entry (e.g. `../../etc/passwd`) was skipped during the
// restore, so it must not be fs.access'd/hashed here either —
// otherwise an existing outside file makes the skipped entry look
// "verified" (and we'd read an arbitrary file off disk).
if (pathEscapes(storagePath, filePath)) {
verification.errors.push(`Refusing unsafe manifest path on verification: ${file.path}`);
continue;
}
try {
await fs.access(filePath);
+6 -1
View File
@@ -21,7 +21,11 @@ class SecureImageService {
expiresIn = 300, // 5 minutes default
maxUses = 1,
clientFingerprint = '',
protectionLevel = 'standard'
protectionLevel = 'standard',
// Whether the minter was a PIN-client — lets the serve route keep
// delivering a photo hidden AFTER minting (TOCTOU). A guest's token
// carries false, so it stops the moment the photo is hidden.
clientBypass = false
} = options;
const tokenData = {
@@ -32,6 +36,7 @@ class SecureImageService {
maxUses,
usedCount: 0,
protectionLevel,
clientBypass,
createdAt: Date.now()
};
+10 -1
View File
@@ -159,7 +159,16 @@ const resolveShareIdentifier = async (identifier) => {
return { event, matchType: 'link', shareToken: getEventShareToken(event) };
}
event = await baseQuery.clone().where('share_link', 'like', `%/${trimmed}`).first();
// GHSA-rh8r hardening: `trimmed` is attacker-controlled, so escape LIKE
// wildcards (`%`, `_`, and the escape char itself) before embedding it.
// Otherwise an anonymous `/resolve/________…________` (32 underscores)
// matches ANY share_link via single-char wildcards, resolves as
// matchType 'link_partial', and the /resolve route hands back the
// gallery's bearer token — reopening the very hole the token-withholding
// fix closed. Explicit ESCAPE clause because SQLite has no default LIKE
// escape character (Postgres defaults to backslash, but we set it for both).
const likeTail = `%/${trimmed.replace(/[\\%_]/g, (c) => `\\${c}`)}`;
event = await baseQuery.clone().whereRaw('share_link LIKE ? ESCAPE \'\\\'', [likeTail]).first();
if (event) {
return { event, matchType: 'link_partial', shareToken: getEventShareToken(event) };
}
+34 -2
View File
@@ -6,6 +6,11 @@ const crypto = require('crypto');
const logger = require('../../utils/logger');
// Staging files older than this are considered orphaned by a crash between
// copy and rename, and are reclaimed during list() walks. Generous enough
// that no legitimate in-flight copy (even multi-GB on slow NFS) hits it.
const STAGING_RECLAIM_AGE_MS = 60 * 60 * 1000;
/**
* Filesystem-backed implementation of the StorageBackend interface.
* All keys are relative to `root` (typically process.env.STORAGE_PATH).
@@ -67,8 +72,18 @@ class LocalFsStorage {
async putFromFile(relPath, localPath, _options = {}) {
const abs = this._resolve(relPath);
await fsp.mkdir(path.dirname(abs), { recursive: true });
// copyFile is atomic from the destination's perspective on POSIX.
await fsp.copyFile(localPath, abs);
// copyFile truncates and rewrites the destination in place, so a
// concurrent reader (thumbnail/watermark generation, photo serving)
// can observe partial or foreign bytes mid-copy (#931). Copy to a
// sibling tmp file and rename, like put() above — rename IS atomic.
const tmp = `${abs}.tmp.${process.pid}.${crypto.randomBytes(4).toString('hex')}`;
try {
await fsp.copyFile(localPath, tmp);
await fsp.rename(tmp, abs);
} catch (err) {
await fsp.unlink(tmp).catch(() => {});
throw err;
}
}
async get(relPath) {
@@ -126,6 +141,23 @@ class LocalFsStorage {
throw err;
}
for (const ent of dirents) {
// Hide in-flight staging files (put/putFromFile write `<key>.tmp.<pid>.<hex>`
// siblings before the atomic rename). Without this filter a
// concurrent archive/backup listing could stream a partial tmp
// entry or fail when the rename wins the race (#931). Stale ones
// (a crash between copy and rename orphans them) are reclaimed
// here — hiding without reclaiming would let interrupted uploads
// accumulate invisible files until the volume fills.
if (/\.tmp\.\d+\.[0-9a-f]+$/.test(ent.name)) {
const childAbs = path.join(dir, ent.name);
try {
const st = await fsp.stat(childAbs);
if (Date.now() - st.mtimeMs > STAGING_RECLAIM_AGE_MS) {
await fsp.unlink(childAbs).catch(() => {});
}
} catch { /* vanished (rename/cleanup won the race) — fine */ }
continue;
}
const childAbs = path.join(dir, ent.name);
const childRel = relBase ? `${relBase}/${ent.name}` : ent.name;
if (ent.isDirectory()) {
@@ -0,0 +1,130 @@
/**
* #931 LocalFsStorage.putFromFile must be atomic. The old implementation
* used fs.copyFile straight onto the destination, which truncates and
* rewrites in place: a concurrent reader (thumbnail/watermark generation,
* photo serving) could observe partial or foreign bytes mid-copy. The fix
* copies to a sibling tmp file and renames, like put() always did.
*/
const fs = require('fs');
const fsp = require('fs/promises');
const os = require('os');
const path = require('path');
const LocalFsStorage = require('../LocalFsStorage');
describe('LocalFsStorage.putFromFile', () => {
let root;
let srcDir;
let storage;
beforeEach(async () => {
root = await fsp.mkdtemp(path.join(os.tmpdir(), 'picpeak-lfs-root-'));
srcDir = await fsp.mkdtemp(path.join(os.tmpdir(), 'picpeak-lfs-src-'));
storage = new LocalFsStorage({ root });
});
afterEach(async () => {
await fsp.rm(root, { recursive: true, force: true });
await fsp.rm(srcDir, { recursive: true, force: true });
});
it('writes the source bytes to the destination key', async () => {
const src = path.join(srcDir, 'a.jpg');
await fsp.writeFile(src, Buffer.from('photo-a-bytes'));
await storage.putFromFile('events/active/ev/a.jpg', src);
const out = await fsp.readFile(path.join(root, 'events/active/ev/a.jpg'));
expect(out.toString()).toBe('photo-a-bytes');
});
it('leaves no tmp files behind after a successful write', async () => {
const src = path.join(srcDir, 'a.jpg');
await fsp.writeFile(src, Buffer.from('photo-a-bytes'));
await storage.putFromFile('events/active/ev/a.jpg', src);
const entries = await fsp.readdir(path.join(root, 'events/active/ev'));
expect(entries).toEqual(['a.jpg']);
});
it('leaves no tmp files behind when the source is missing', async () => {
await expect(
storage.putFromFile('events/active/ev/missing.jpg', path.join(srcDir, 'nope.jpg'))
).rejects.toThrow();
const entries = await fsp.readdir(path.join(root, 'events/active/ev')).catch(() => []);
expect(entries.filter((e) => e.includes('.tmp.'))).toEqual([]);
});
it('hides in-flight staging files from list()', async () => {
const src = path.join(srcDir, 'a.jpg');
await fsp.writeFile(src, Buffer.from('photo-a-bytes'));
await storage.putFromFile('events/active/ev/a.jpg', src);
// Simulate a concurrent writer's staging file: archiveEvent lists
// this exact prefix and must never see (stream/delete) it.
await fsp.writeFile(
path.join(root, 'events/active/ev/b.jpg.tmp.12345.deadbeef'),
Buffer.from('partial')
);
const keys = (await storage.list('events/active/ev')).map((e) => e.key ?? e);
expect(JSON.stringify(keys)).toContain('a.jpg');
expect(JSON.stringify(keys)).not.toContain('.tmp.');
});
it('reclaims stale orphaned staging files during list()', async () => {
const dir = path.join(root, 'events/active/ev');
await fsp.mkdir(dir, { recursive: true });
const fresh = path.join(dir, 'f.jpg.tmp.111.aaaaaaaa');
const stale = path.join(dir, 's.jpg.tmp.222.bbbbbbbb');
await fsp.writeFile(fresh, Buffer.from('in-flight'));
await fsp.writeFile(stale, Buffer.from('orphaned'));
// Age the "stale" one past the reclaim threshold (1h).
const old = new Date(Date.now() - 2 * 60 * 60 * 1000);
await fsp.utimes(stale, old, old);
await storage.list('events/active/ev');
// Fresh in-flight staging survives (a live copy may still rename it);
// the crash orphan is gone.
await expect(fsp.stat(fresh)).resolves.toBeDefined();
await expect(fsp.stat(stale)).rejects.toThrow();
});
it('never exposes a partially written destination (tmp+rename atomicity)', async () => {
// A large-ish payload so the copy is not a single instantaneous block.
const big = Buffer.alloc(8 * 1024 * 1024, 0xab);
const src = path.join(srcDir, 'big.bin');
await fsp.writeFile(src, big);
const key = 'events/active/ev/big.bin';
const dest = path.join(root, key);
// Poll the destination while the copy runs: it must either not exist
// yet or already have the full size — never an in-between truncated
// state (which is exactly what in-place copyFile produced).
const observed = [];
const poller = (async () => {
for (let i = 0; i < 200; i++) {
try {
const st = fs.statSync(dest);
observed.push(st.size);
} catch {
// not there yet — fine
}
await new Promise((r) => setImmediate(r));
}
})();
await storage.putFromFile(key, src);
await poller;
for (const size of observed) {
expect(size).toBe(big.length);
}
const out = await fsp.stat(dest);
expect(out.size).toBe(big.length);
});
});
+13
View File
@@ -131,6 +131,19 @@ class S3StorageAdapter extends stream.EventEmitter {
*/
async testConnection() {
try {
// Resolve-and-vet the custom endpoint before the network round-trip
// (the constructor's literal check can't catch a public-looking
// hostname that resolves to an internal IP). Prod-only, matching the
// constructor gate — dev points at localhost MinIO deliberately.
if (process.env.NODE_ENV === 'production' && this.config.endpoint) {
const { isHostAllowed } = require('../../utils/networkValidation');
const { hostname } = new URL(
/^https?:\/\//.test(this.config.endpoint) ? this.config.endpoint : `https://${this.config.endpoint}`
);
if (!(await isHostAllowed(hostname))) {
throw new Error('S3 endpoint resolves to a private or internal network address');
}
}
await this.s3Client.send(new HeadBucketCommand({ Bucket: this.bucket }));
logger.info(`Successfully connected to S3 bucket: ${this.bucket}`);
return true;
@@ -8,6 +8,7 @@
* - Tracking regeneration progress
*/
const pLimit = require('p-limit');
const { db } = require('../database/db');
const watermarkService = require('./watermarkService');
const { resolvePhotoStorageKey, resolvePhotoFilePath } = require('./photoResolver');
@@ -22,6 +23,13 @@ class WatermarkGeneratorService {
this.batchSize = 10;
// Concurrent processing limit
this.concurrentLimit = 2;
// ONE process-wide limiter for every sharp pipeline this service
// spawns (#931). Per-invocation limiters would stack: overlapping
// regenerateAll/generateForEvent calls each brought their own cap,
// and the fire-and-forget generateForPhoto side-effect (one per
// uploaded photo) had no cap at all — a 363-photo bulk upload could
// decode 363 full-resolution images concurrently (#628 OOM class).
this.limit = pLimit(this.concurrentLimit);
}
/**
@@ -62,11 +70,21 @@ class WatermarkGeneratorService {
// (external reference mode). watermarkService needs a local file path.
const event = { slug: photo.slug, source_mode: photo.source_mode, external_path: photo.external_path };
const storageKey = resolvePhotoStorageKey(event, photo);
const result = storageKey
? await withLocalCopy(storageKey, (lp) =>
watermarkService.generateAndSaveWatermark(photo, lp, settings)
)
: await watermarkService.generateAndSaveWatermark(photo, resolvePhotoFilePath(event, photo), settings);
const result = await this.limit(async () => {
// Revalidate inside the limited slot: a long queue (bulk upload)
// can hold this job for minutes, during which an admin may disable
// watermarking — running with the captured settings would recreate
// files AFTER clearAllWatermarks() wiped them (#931 round 3).
const fresh = await watermarkService.getWatermarkSettings();
if (!fresh || !fresh.enabled) {
return { success: false, watermarkPath: null, error: 'Watermarking is disabled' };
}
return storageKey
? withLocalCopy(storageKey, (lp) =>
watermarkService.generateAndSaveWatermark(photo, lp, fresh)
)
: watermarkService.generateAndSaveWatermark(photo, resolvePhotoFilePath(event, photo), fresh);
});
if (result.success) {
// Update database with watermark path
@@ -122,7 +140,11 @@ class WatermarkGeneratorService {
return { ...results, errors: ['Watermarking is disabled'] };
}
// Process in batches
// Process in batches. processPhotoWatermark routes every sharp
// pipeline through the shared instance limiter — a bare Promise.all
// over the batch ran all 10 at once, decoding 10 full-resolution
// images simultaneously (#931; same OOM class as #628 in the
// thumbnail path).
for (let i = 0; i < photos.length; i += this.batchSize) {
const batch = photos.slice(i, i + this.batchSize);
@@ -168,11 +190,21 @@ class WatermarkGeneratorService {
try {
const event = { slug: photo.slug, source_mode: photo.source_mode, external_path: photo.external_path };
const storageKey = resolvePhotoStorageKey(event, photo);
const result = storageKey
? await withLocalCopy(storageKey, (lp) =>
watermarkService.generateAndSaveWatermark(photo, lp, settings)
)
: await watermarkService.generateAndSaveWatermark(photo, resolvePhotoFilePath(event, photo), settings);
const result = await this.limit(async () => {
// Same revalidation as generateForPhoto: batch jobs queue for a
// long time, and a disable mid-run must not recreate files after
// clearAllWatermarks(). The batch's `settings` snapshot is still
// used for rendering; only the enabled gate is rechecked.
const fresh = await watermarkService.getWatermarkSettings();
if (!fresh || !fresh.enabled) {
return { success: false, watermarkPath: null, error: 'Watermarking is disabled' };
}
return storageKey
? withLocalCopy(storageKey, (lp) =>
watermarkService.generateAndSaveWatermark(photo, lp, settings)
)
: watermarkService.generateAndSaveWatermark(photo, resolvePhotoFilePath(event, photo), settings);
});
if (result.success) {
await db('photos')
@@ -235,7 +267,8 @@ class WatermarkGeneratorService {
logger.info(`Starting watermark regeneration for ${photos.length} photos`);
// Process in batches
// Process in batches, capped at concurrentLimit parallel sharp
// pipelines via the shared instance limiter (see generateForEvent).
for (let i = 0; i < photos.length; i += this.batchSize) {
// Check if job was cancelled
if (!this.activeJobs.has(jobId)) {
+47 -5
View File
@@ -2,7 +2,7 @@ const axios = require('axios');
const { db } = require('../database/db');
const logger = require('../utils/logger');
const { signPayload, renderTemplate } = require('./webhookService');
const { validateExternalUrl } = require('../utils/networkValidation');
const { validateExternalUrlAsync } = require('../utils/networkValidation');
const POLL_INTERVAL_MS = parseInt(process.env.WEBHOOK_DELIVERY_INTERVAL_MS || '5000', 10);
const CONCURRENCY = parseInt(process.env.WEBHOOK_DELIVERY_CONCURRENCY || '5', 10);
@@ -91,12 +91,24 @@ async function deliverOne(row) {
return;
}
// Re-validate URL per delivery — DNS-rebinding mitigation. Admin can opt
// out via WEBHOOK_ALLOW_PRIVATE_URLS=true for local-receiver dev runs.
// Re-validate URL per delivery — DNS-rebinding mitigation. Resolves the
// host and vets every A/AAAA record (a public-looking name that now
// resolves to an internal IP is rejected). Admin can opt out via
// WEBHOOK_ALLOW_PRIVATE_URLS=true for local-receiver dev runs.
if (!allowPrivateUrls) {
const urlCheck = validateExternalUrl(webhook.url);
const urlCheck = await validateExternalUrlAsync(webhook.url);
if (!urlCheck.valid) {
await markFailedFinal(row, `URL rejected: ${urlCheck.error}`);
// A transient lookup failure ('unresolved' — EAI_AGAIN, resolver
// briefly down) must NOT connect: falling through to axios would let
// an attacker SERVFAIL this preflight and answer axios's own lookup
// with a private/metadata IP, defeating the guard. Schedule the
// normal retry/backoff instead — no request is made. A confirmed
// policy rejection (resolves-to-private / malformed) is permanent.
if (urlCheck.reason === 'unresolved') {
await scheduleTransientRetry(row, webhook, 'URL host did not resolve — retrying');
} else {
await markFailedFinal(row, `URL rejected: ${urlCheck.error}`);
}
return;
}
}
@@ -217,6 +229,36 @@ async function markFailedFinal(row, reason) {
await db('webhooks').where({ id: row.webhook_id }).update({ last_failure_at: new Date() });
}
// Schedule the normal retry/backoff for a transient failure that must not
// make a network request (e.g. the SSRF preflight lookup failed). Mirrors
// the failure branch of the main delivery path: retry until MAX_ATTEMPTS,
// then give up. No response fields — nothing was sent.
async function scheduleTransientRetry(row, webhook, errorMsg) {
const newAttempt = row.attempt_count + 1;
if (newAttempt >= MAX_ATTEMPTS) {
await db('webhook_deliveries')
.where({ id: row.id })
.update({
status: 'failed',
last_error: errorMsg,
attempt_count: newAttempt,
completed_at: new Date(),
next_retry_at: null,
});
} else {
const backoff = BACKOFF_MS[Math.min(newAttempt - 1, BACKOFF_MS.length - 1)];
await db('webhook_deliveries')
.where({ id: row.id })
.update({
status: 'pending',
last_error: errorMsg,
attempt_count: newAttempt,
next_retry_at: new Date(Date.now() + backoff),
});
}
await db('webhooks').where({ id: webhook.id }).update({ last_failure_at: new Date() });
}
function stringifyBody(data) {
if (data == null) return null;
if (typeof data === 'string') return data;
@@ -89,23 +89,25 @@ describe('sanitizeFilename — edge cases', () => {
});
describe('generatePhotoFilename — composed name uses the NFD pipeline', () => {
it('round-trips Ägypten + individual → Agypten_individual_0050.jpg (#607)', () => {
// The trailing _[0-9a-f]{12} is the anti-collision suffix (#931) that
// keeps concurrent uploads from assigning the same final storage path.
it('round-trips Ägypten + individual → Agypten_individual_0050 (#607)', () => {
expect(generatePhotoFilename('Ägypten', 'individual', 50, '.jpg'))
.toBe('Agypten_individual_0050.jpg');
.toMatch(/^Agypten_individual_0050_[0-9a-f]{12}\.jpg$/);
});
it('handles missing category by defaulting to "uncategorized"', () => {
expect(generatePhotoFilename('Wedding', null, 1, '.jpg'))
.toBe('Wedding_uncategorized_0001.jpg');
.toMatch(/^Wedding_uncategorized_0001_[0-9a-f]{12}\.jpg$/);
});
it('zero-pads the counter to 4 digits', () => {
expect(generatePhotoFilename('e', 'c', 7, '.png')).toBe('e_c_0007.png');
expect(generatePhotoFilename('e', 'c', 1234, '.png')).toBe('e_c_1234.png');
// 5+ digit counters intentionally overflow the pad — pinned because
// the unique index in the photos table doesn't care about pad width,
// only string uniqueness.
expect(generatePhotoFilename('e', 'c', 99999, '.png')).toBe('e_c_99999.png');
expect(generatePhotoFilename('e', 'c', 7, '.png')).toMatch(/^e_c_0007_[0-9a-f]{12}\.png$/);
expect(generatePhotoFilename('e', 'c', 1234, '.png')).toMatch(/^e_c_1234_[0-9a-f]{12}\.png$/);
// 5+ digit counters intentionally overflow the pad — pad width never
// mattered for uniqueness (there is no unique index on filenames);
// the random suffix is what guarantees it.
expect(generatePhotoFilename('e', 'c', 99999, '.png')).toMatch(/^e_c_99999_[0-9a-f]{12}\.png$/);
});
});
+13 -2
View File
@@ -1,4 +1,5 @@
const path = require('path');
const crypto = require('crypto');
/**
* Sanitize a string to be used as a filename component
@@ -59,8 +60,18 @@ function generatePhotoFilename(eventName, categoryName, counter, extension) {
const sanitizedEvent = sanitizeFilename(eventName, 30);
const sanitizedCategory = sanitizeFilename(categoryName || 'uncategorized', 20);
const paddedCounter = String(counter).padStart(4, '0');
return `${sanitizedEvent}_${sanitizedCategory}_${paddedCounter}${extension}`;
// Random suffix (#931): the counter base is `count(*)+1` computed per
// upload request, so two concurrent bulk-upload requests can assign the
// same counter to different photos. Since files are written to their
// final path before any row exists (and photos has no unique index on
// filename — one can't be added without a dedupe migration on installs
// that already carry historical duplicates), a collision silently
// overwrites the first photo's bytes at its recorded path — cross-photo
// contamination. 48 bits keep the collision odds negligible even for
// pathological concurrency (two simultaneous 2000-photo uploads: ~7e-12).
const suffix = crypto.randomBytes(6).toString('hex');
return `${sanitizedEvent}_${sanitizedCategory}_${paddedCounter}_${suffix}${extension}`;
}
/**
+68 -1
View File
@@ -1,5 +1,6 @@
const { URL } = require('url');
const net = require('net');
const dns = require('dns').promises;
/**
* Check if a hostname or IP resolves to a private/internal network address.
@@ -162,6 +163,11 @@ function isPrivateIPv6(ip) {
/**
* Validate a URL string, rejecting private/internal targets.
*
* NOTE: literal-only. For a hostname (not an IP), this checks the string but
* NOT what it resolves to `evil.example` with an A record of 10.0.0.5
* passes. Prefer isHostAllowed / validateExternalUrlAsync at any call site
* that then actually connects; kept for synchronous callers and fast checks.
* @param {string} urlString - URL to validate
* @returns {{ valid: boolean, error?: string }}
*/
@@ -177,4 +183,65 @@ function validateExternalUrl(urlString) {
}
}
module.exports = { isPrivateIP, validateExternalUrl };
/**
* Resolve a hostname and reject if it (or ANY of its A/AAAA records) points
* at a private/internal address. Closes the SSRF hole where a public-looking
* hostname resolves to an internal IP or the cloud metadata endpoint the
* literal isPrivateIP check alone can't see that. Fails closed on resolution
* failure. IP literals are decided by isPrivateIP without a lookup.
*
* Residual: a determined attacker who controls DNS can still rebind between
* this check and the client's own resolution (TOCTOU). Fully closing that
* needs pinning the connection to the vetted IP, which the underlying
* clients (nodemailer/imap/ssh/aws-sdk) don't cleanly support; these actions
* are admin-only, so resolve-and-vet is the proportionate mitigation.
*
* @param {string} hostname
* @returns {Promise<boolean>} true when safe to connect
*/
async function classifyHost(hostname) {
if (!hostname || typeof hostname !== 'string') return 'invalid';
// Literal check first: IP literals, blocked names, .internal/.local/.localhost.
if (isPrivateIP(hostname)) return 'private';
// An IP literal is fully decided above — no name to resolve.
const bare = hostname.replace(/^\[|\]$/g, '');
if (net.isIP(bare)) return 'ok';
let addresses;
try {
addresses = await dns.lookup(hostname, { all: true });
} catch {
return 'unresolved'; // transient/NXDOMAIN — caller decides retry vs reject
}
if (!addresses.length) return 'unresolved';
return addresses.every((a) => !isPrivateIP(a.address)) ? 'ok' : 'private';
}
async function isHostAllowed(hostname) {
// Fail-closed boolean for save/test call sites: anything not clearly 'ok'
// (including a transient lookup failure) is rejected.
return (await classifyHost(hostname)) === 'ok';
}
/**
* Async, DNS-resolving counterpart to validateExternalUrl. Returns a `reason`
* so callers with retry semantics (e.g. the webhook worker) can distinguish a
* policy rejection ('private'/'invalid') from a transient lookup failure
* ('unresolved') that should be retried rather than permanently failed.
* @param {string} urlString
* @returns {Promise<{ valid: boolean, error?: string, reason: string }>}
*/
async function validateExternalUrlAsync(urlString) {
let parsed;
try {
parsed = new URL(urlString);
} catch {
return { valid: false, error: 'Invalid URL format', reason: 'invalid' };
}
const reason = await classifyHost(parsed.hostname);
if (reason !== 'ok') {
return { valid: false, error: 'URL points to a private or internal network address', reason };
}
return { valid: true, reason: 'ok' };
}
module.exports = { isPrivateIP, validateExternalUrl, isHostAllowed, validateExternalUrlAsync, classifyHost };
+46
View File
@@ -0,0 +1,46 @@
/**
* Shared hidden-photo access control.
*
* PicPeak photos carry a `visibility` column: 'visible' (or NULL, for
* pre-migration rows) is shown to everyone; 'hidden' is client-only. A
* gallery viewer's `req.accessLevel` is 'client' for a PIN-client login and
* something else ('guest'/'slideshow'/) for an ordinary guest.
*
* The main photo-list query and the single-photo download/view routes each
* enforced this inline, but several bulk/secure paths (download-all,
* download-selected, protected-image view, signed-URL mint, secure-token
* mint, secure-download) shipped without it letting ordinary guests reach
* hidden/client-only photos. These helpers centralise the rule so every
* sink applies exactly the same predicate.
*/
// PIN-clients see hidden photos; everyone else does not.
function canSeeHiddenPhotos(accessLevel) {
return accessLevel === 'client';
}
/**
* Append the guest visibility filter to a knex `photos` query. No-op for
* clients. NULL visibility is treated as visible (pre-migration default).
* The query must reference the table as `photos` (all call sites do).
*/
function applyPhotoVisibilityFilter(query, accessLevel) {
if (canSeeHiddenPhotos(accessLevel)) return query;
return query.where(function () {
this.where('photos.visibility', 'visible').orWhereNull('photos.visibility');
});
}
/**
* Single-photo predicate: true when this photo must be blocked for a viewer
* at the given access level. Mirrors the inline guards in gallery.js.
*/
function isPhotoHiddenFromViewer(photo, accessLevel) {
return !!photo && photo.visibility === 'hidden' && !canSeeHiddenPhotos(accessLevel);
}
module.exports = {
canSeeHiddenPhotos,
applyPhotoVisibilityFilter,
isPhotoHiddenFromViewer,
};
+22 -6
View File
@@ -57,18 +57,34 @@ function generateCandidates(raw, storageRoot) {
if (!value) return [];
const stripped = value.replace(/^\/+/, '');
const baseName = path.basename(value);
const cwdStorage = path.join(process.cwd(), 'storage');
// Build candidate set; dedup at the end so we don't stat the same
// file twice when the inputs overlap.
const candidates = [
path.isAbsolute(value) ? value : null,
// Keep the raw absolute value as a candidate so a legitimate multer path
// (branding_logo_path is stored absolute) or an absolute logo inside a
// non-standard storage subdir still resolves. The containment filter
// below is what enforces safety — it drops this candidate when it points
// outside the storage roots, so `/etc/passwd` is still rejected.
...(path.isAbsolute(value) ? [value] : []),
path.join(storageRoot, stripped),
path.join(storageRoot, 'uploads', 'logos', baseName),
path.join(storageRoot, 'branding', baseName),
path.join(process.cwd(), 'storage', stripped),
path.join(process.cwd(), 'storage', 'uploads', 'logos', baseName),
path.join(process.cwd(), 'storage', 'branding', baseName),
].filter(Boolean);
return [...new Set(candidates)];
path.join(cwdStorage, stripped),
path.join(cwdStorage, 'uploads', 'logos', baseName),
path.join(cwdStorage, 'branding', baseName),
];
// GHSA-c7x5: only read logo files INSIDE the storage roots. An admin-set
// logo_path of `/etc/passwd` was previously rasterised into a PDF; the
// filter below drops any candidate (including the raw absolute one and any
// `..`-escaping stripped path) that resolves outside the roots. baseName-
// based candidates are inherently contained.
const roots = [path.resolve(storageRoot), path.resolve(cwdStorage)];
const contained = candidates.filter((c) => {
const r = path.resolve(c);
return roots.some((root) => r === root || r.startsWith(root + path.sep));
});
return [...new Set(contained)];
}
function pickExisting(candidates) {
+1 -1
View File
@@ -1,7 +1,7 @@
{
"name": "picpeak-frontend",
"private": true,
"version": "3.45.9",
"version": "3.45.12",
"type": "module",
"scripts": {
"dev": "vite",
+9 -4
View File
@@ -3,7 +3,7 @@ import { BrowserRouter as Router, Routes, Route, Navigate, useParams } from 'rea
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
import { ToastContainer } from 'react-toastify';
import 'react-toastify/dist/ReactToastify.css';
import { analyticsService } from './services/analytics.service';
import { analyticsService, AnalyticsRouteTracker } from './services/analytics.service';
import { GalleryAuthProvider, MaintenanceProvider } from './contexts';
import { ThemeProvider } from './contexts/ThemeContext';
@@ -115,8 +115,10 @@ function AnalyticsBootstrap() {
provider: 'rybbit',
hostUrl: settings.rybbit_url,
websiteId: settings.rybbit_website_id,
autoTrack: true,
doNotTrack: true,
// Mask every /gallery/* path (they embed the share token) so Rybbit's
// auto-tracked page views never carry the secret (GHSA-7m6c).
maskPatterns: ['/gallery/**'],
});
return;
}
@@ -138,7 +140,9 @@ function AnalyticsBootstrap() {
provider: 'umami',
hostUrl: settings.umami_url,
websiteId: settings.umami_website_id,
autoTrack: true,
// autoTrack omitted → data-auto-track="false": Umami must NOT read the
// raw window.location (token leak). Page views come from the manual,
// sanitized AnalyticsRouteTracker instead (GHSA-7m6c).
doNotTrack: true,
});
return;
@@ -151,7 +155,7 @@ function AnalyticsBootstrap() {
provider: 'umami',
hostUrl: envUmamiUrl,
websiteId: envUmamiWebsiteId,
autoTrack: true,
// autoTrack omitted → data-auto-track="false" (see above, GHSA-7m6c).
doNotTrack: true,
});
}
@@ -194,6 +198,7 @@ function App() {
<DynamicFavicon />
<RobotsMetaTags />
<Router>
<AnalyticsRouteTracker />
<MaintenanceWrapper>
<SkipLink />
<Routes>
+43
View File
@@ -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<string | null | undefined>,
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]);
}
+29 -4
View File
@@ -1,4 +1,4 @@
import React from 'react';
import React, { useCallback } from 'react';
import { useNavigate } from 'react-router-dom';
import {
Calendar,
@@ -15,7 +15,9 @@ import {
Check,
X
} from 'lucide-react';
import { differenceInDays, parseISO } from 'date-fns';
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,10 @@ export const AdminDashboard: React.FC = () => {
) : (
<div className="space-y-3">
{expiringEvents.map((event) => {
const daysLeft = differenceInDays(parseISO(event.expires_at!), new Date());
// 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 (
<div
@@ -19,6 +19,7 @@ import { useTranslation } from 'react-i18next';
import { Button, Card, Loading } from '../../components/common';
import { useLocalizedDate } from '../../hooks/useLocalizedDate';
import { useMutationWithToast } from '../../hooks';
import { useAdminAuth } from '../../contexts/AdminAuthContext';
import { BackupDashboard } from '../../components/admin/BackupDashboard';
import { BackupConfiguration } from '../../components/admin/BackupConfiguration';
import { BackupHistory } from '../../components/admin/BackupHistory';
@@ -33,6 +34,11 @@ type TabId = 'dashboard' | 'configuration' | 'history' | 'restore' | 'integrity'
export const BackupManagement: React.FC = () => {
const [activeTab, setActiveTab] = useState<TabId>('dashboard');
const { t } = useTranslation();
// Full-instance export contains every secret, so the endpoint is
// super_admin-only (GHSA-pv6w) — hide the card for other roles instead
// of showing a button that always 403s.
const { user } = useAdminAuth();
const isSuperAdmin = user?.role?.name === 'super_admin';
const { formatDateTime: fmtDateTime } = useLocalizedDate();
const tabs = [
@@ -201,7 +207,7 @@ export const BackupManagement: React.FC = () => {
onRunBackup={() => manualBackupMutation.mutate()}
isBackupRunning={backupStatus?.isRunning || manualBackupMutation.isPending}
/>
<PicpeakExportCard />
{isSuperAdmin && <PicpeakExportCard />}
</div>
)}
+17 -5
View File
@@ -1,7 +1,7 @@
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 { differenceInDays } from 'date-fns';
import { toast } from 'react-toastify';
import { useLocalizedDate } from '../../hooks/useLocalizedDate';
@@ -101,6 +101,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],
@@ -277,9 +284,14 @@ export const EventDetailsPage: React.FC = () => {
}
const expiresAtDate = safeParseDate(event.expires_at);
const daysUntilExpiration = expiresAtDate ? differenceInDays(expiresAtDate, new Date()) : null;
const isExpired = daysUntilExpiration !== null && daysUntilExpiration <= 0;
const isExpiring = daysUntilExpiration !== null && daysUntilExpiration > 0 && daysUntilExpiration <= 7;
// Timestamp comparison, not truncated whole days (#909): the old
// differenceInDays <= 0 marked events "expired" up to 24h early.
// Ceiling keeps the countdown at "1 day" through the final day.
const isExpired = expiresAtDate !== null && expiresAtDate.getTime() <= Date.now();
const daysUntilExpiration = expiresAtDate
? Math.ceil((expiresAtDate.getTime() - Date.now()) / 86400000)
: null;
const isExpiring = !isExpired && daysUntilExpiration !== null && daysUntilExpiration > 0 && daysUntilExpiration <= 7;
const handleStartEdit = () => {
setEditForm({
+19 -4
View File
@@ -1,4 +1,5 @@
import React, { useState, useEffect } from 'react';
import { useExpiryRefresh } from '../../hooks/useExpiryRefresh';
import { useNavigate, useSearchParams } from 'react-router-dom';
import {
Plus,
@@ -18,7 +19,7 @@ import {
ChevronLeft,
ChevronRight
} from 'lucide-react';
import { parseISO, differenceInDays } from 'date-fns';
import { parseISO } from 'date-fns';
import { toast } from 'react-toastify';
import { useModal, useMutationWithToast } from '../../hooks';
import { useLocalizedDate } from '../../hooks/useLocalizedDate';
@@ -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;
@@ -258,8 +267,14 @@ export const EventsListPage: React.FC = () => {
if (!event.expires_at) return { label: t('events.active'), color: 'text-green-600 dark:text-green-400 bg-green-100 dark:bg-green-900/40' };
const days = differenceInDays(parseISO(event.expires_at), new Date());
if (days <= 0) return { label: t('events.expired'), color: 'text-red-600 dark:text-red-400 bg-red-100 dark:bg-red-900/40' };
// Expired means the timestamp has actually passed (#909):
// differenceInDays truncates to whole days, so an event expiring in a
// few hours returned 0 and showed "Expired" while the public gallery
// (which compares real timestamps) correctly showed it active.
const expiresAt = parseISO(event.expires_at);
if (expiresAt.getTime() <= Date.now()) return { label: t('events.expired'), color: 'text-red-600 dark:text-red-400 bg-red-100 dark:bg-red-900/40' };
// Ceiling so the last day reads "1 day left", never "0 days".
const days = Math.ceil((expiresAt.getTime() - Date.now()) / 86400000);
if (days <= 7) return { label: t('events.daysLeft', { count: days }), color: 'text-orange-600 dark:text-orange-400 bg-orange-100 dark:bg-orange-900/40' };
return { label: t('events.active'), color: 'text-green-600 dark:text-green-400 bg-green-100 dark:bg-green-900/40' };
+57 -7
View File
@@ -29,6 +29,9 @@ interface RybbitInitConfig extends BaseInitConfig {
provider: 'rybbit';
websiteId: string;
hostUrl: string;
// URL path patterns whose value must never reach the collector (they embed
// the gallery share token). Rendered into Rybbit's data-mask-patterns.
maskPatterns?: string[];
}
interface CustomInitConfig extends BaseInitConfig {
@@ -56,7 +59,7 @@ declare global {
};
rybbit?: {
event: (eventName: string, eventData?: any) => void;
pageview?: () => void;
pageview?: (path?: string) => void;
};
}
}
@@ -85,7 +88,12 @@ class AnalyticsService {
script.defer = true;
script.src = `${config.hostUrl.replace(/\/+$/, '')}/script.js`;
script.setAttribute('data-website-id', config.websiteId);
if (config.autoTrack === false) script.setAttribute('data-auto-track', 'false');
// Auto-track OFF by default (GHSA-7m6c): Umami's auto page-view capture
// reads window.location verbatim, so a gallery URL /gallery/:slug/:token
// would ship the secret share token to the analytics collector. Page
// views are fired manually through trackPageView(), which redacts the
// token. Only an explicit autoTrack:true opts back into raw capture.
if (config.autoTrack !== true) script.setAttribute('data-auto-track', 'false');
if (config.doNotTrack !== false) script.setAttribute('data-do-not-track', 'true');
if (config.domains?.length) script.setAttribute('data-domains', config.domains.join(','));
document.head.appendChild(script);
@@ -100,6 +108,16 @@ class AnalyticsService {
script.defer = true;
script.src = `${config.hostUrl.replace(/\/+$/, '')}/api/script.js`;
script.setAttribute('data-site-id', config.websiteId);
// GHSA-7m6c: Rybbit auto-tracks page views (initial load + SPA route
// changes) reading window.location, so a gallery URL would ship the raw
// share token. Unlike Umami we CAN'T fix this with a manual tracker —
// the initial-load pageview fires before any of our code runs. Instead
// use Rybbit's native data-mask-patterns, which replaces matching paths
// with the pattern string in analytics, stripping the token on every
// auto-tracked pageview including the first.
if (config.maskPatterns?.length) {
script.setAttribute('data-mask-patterns', JSON.stringify(config.maskPatterns));
}
document.head.appendChild(script);
} else if (config.provider === 'custom') {
// The admin-pasted HTML is sanitised server-side (see
@@ -148,13 +166,35 @@ class AnalyticsService {
// 'none' / 'custom' / unloaded → silently ignore.
}
// Redact secrets from a URL before it reaches the analytics collector
// (GHSA-7m6c): drop the query string entirely and replace token-looking
// path segments (long hex / opaque IDs — e.g. the gallery share token in
// /gallery/:slug/:token) with a placeholder. Failing safe: on any parse
// issue return just the pathname without the query.
private sanitizeTrackedUrl(url: string): string {
try {
const pathOnly = url.split('?')[0].split('#')[0];
return pathOnly
.split('/')
.map((seg) =>
/^[0-9a-fA-F]{16,}$/.test(seg) || /^[A-Za-z0-9_-]{20,}$/.test(seg) ? '[redacted]' : seg)
.join('/');
} catch {
return url.split('?')[0];
}
}
trackPageView(url?: string, referrer?: string) {
if (!this.initialized) return;
if (this.provider === 'umami' && typeof window !== 'undefined' && window.umami) {
window.umami.trackView(url, referrer, this.websiteId || undefined);
} else if (this.provider === 'rybbit' && typeof window !== 'undefined' && window.rybbit?.pageview) {
window.rybbit.pageview();
}
// Only Umami is manually tracked here: its auto-track is disabled (so the
// raw token URL never hits the collector) and this sanitized call is the
// ONLY page-view source. Rybbit keeps its own auto-tracking with
// data-mask-patterns doing the redaction, so a manual call would
// double-count — skip it. 'none'/'custom' have no page-view API.
if (this.provider !== 'umami' || typeof window === 'undefined' || !window.umami) return;
const raw = url ?? window.location.pathname;
const safe = this.sanitizeTrackedUrl(raw);
window.umami.trackView(safe, referrer, this.websiteId || undefined);
}
// Gallery-specific tracking events
@@ -209,3 +249,13 @@ export const useAnalytics = () => {
return analyticsService;
};
// Renderless component that drives manual page-view tracking. MUST be mounted
// INSIDE <Router> (useLocation needs router context) — that's why the
// AnalyticsBootstrap init, which lives outside the Router, can't do this
// itself. Without a mounted caller trackPageView never fires and Umami — whose
// auto-track we deliberately disable — records nothing.
export const AnalyticsRouteTracker = (): null => {
useAnalytics();
return null;
};
+9 -1
View File
@@ -91,7 +91,9 @@ export const eventsService = {
page: number = 1,
limit: number = 20,
status?: EventStatusFilter,
search?: string
search?: string,
sortBy?: string,
sortOrder?: 'asc' | 'desc'
): Promise<EventsListResponse> {
const params = new URLSearchParams({
page: page.toString(),
@@ -104,6 +106,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<EventsListResponse>(`/admin/events?${params}`);
const data: any = response.data;