Compare commits
17 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 0c73bf2cdc | |||
| 2c7b5dfd02 | |||
| 5d5db4e766 | |||
| e5dccf1664 | |||
| bfafecedc7 | |||
| 2c5a094c5c | |||
| 2462ba6897 | |||
| 90275f88e9 | |||
| 34a7b1c013 | |||
| 7419c68337 | |||
| fc99e2b233 | |||
| 7974b9c6d7 | |||
| 60cbda5b22 | |||
| a27d19b4d1 | |||
| d68d84e5c8 | |||
| 6891769124 | |||
| b32ba1ed6b |
@@ -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 @@
|
||||
{".":"3.45.9"}
|
||||
{".":"3.45.12"}
|
||||
|
||||
@@ -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');
|
||||
});
|
||||
});
|
||||
|
||||
Generated
+35
-39
@@ -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"
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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() });
|
||||
|
||||
@@ -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',
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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' });
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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(',');
|
||||
});
|
||||
|
||||
|
||||
@@ -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, '""')}"`;
|
||||
}
|
||||
|
||||
@@ -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;
|
||||
@@ -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');
|
||||
|
||||
|
||||
@@ -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');
|
||||
}),
|
||||
);
|
||||
|
||||
@@ -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;
|
||||
}),
|
||||
|
||||
@@ -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');
|
||||
|
||||
|
||||
@@ -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();
|
||||
|
||||
|
||||
@@ -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);
|
||||
|
||||
|
||||
@@ -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(',');
|
||||
});
|
||||
|
||||
|
||||
@@ -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).
|
||||
|
||||
@@ -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');
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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')
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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);
|
||||
|
||||
|
||||
@@ -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()
|
||||
};
|
||||
|
||||
|
||||
@@ -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) };
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
@@ -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)) {
|
||||
|
||||
@@ -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$/);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -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}`;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -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 };
|
||||
|
||||
@@ -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,
|
||||
};
|
||||
@@ -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,7 +1,7 @@
|
||||
{
|
||||
"name": "picpeak-frontend",
|
||||
"private": true,
|
||||
"version": "3.45.9",
|
||||
"version": "3.45.12",
|
||||
"type": "module",
|
||||
"scripts": {
|
||||
"dev": "vite",
|
||||
|
||||
@@ -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>
|
||||
|
||||
@@ -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]);
|
||||
}
|
||||
@@ -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>
|
||||
)}
|
||||
|
||||
|
||||
@@ -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({
|
||||
|
||||
@@ -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' };
|
||||
|
||||
@@ -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;
|
||||
};
|
||||
|
||||
@@ -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;
|
||||
|
||||
Reference in New Issue
Block a user