Compare commits

..

1 Commits

Author SHA1 Message Date
Paul Nothaft 6af5241c15 docs: align stable security and backport policy 2026-09-07 23:13:05 +02:00
86 changed files with 326 additions and 5466 deletions
+1 -1
View File
@@ -1 +1 @@
{".":"3.46.12"}
{".":"3.46.10"}
-15
View File
@@ -5,21 +5,6 @@ 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.46.12](https://github.com/PicPeak/picpeak/compare/v3.46.11...v3.46.12) (2026-09-10)
### Bug Fixes
* **backup:** honor the configured database-backup destination path ([#1367](https://github.com/PicPeak/picpeak/issues/1367)) ([3420745](https://github.com/PicPeak/picpeak/commit/34207456e6c1ed2908ab34335ab0fd4d99503e90))
* **video:** try metadata extraction and thumbnail generation independently ([#1372](https://github.com/PicPeak/picpeak/issues/1372)) ([1316ed0](https://github.com/PicPeak/picpeak/commit/1316ed05b3938da59eb8cb263a5334909e351bfe))
## [3.46.11](https://github.com/PicPeak/picpeak/compare/v3.46.10...v3.46.11) (2026-09-08)
### Documentation
* align stable security and backport policy ([#1352](https://github.com/PicPeak/picpeak/issues/1352)) ([143c403](https://github.com/PicPeak/picpeak/commit/143c4035ec38683634d0e3d493032e2965f4a46f))
## [3.46.10](https://github.com/PicPeak/picpeak/compare/v3.46.9...v3.46.10) (2026-09-07)
@@ -1,195 +0,0 @@
/**
* POST /api/admin/business-profile/logo and PUT /api/admin/business-profile
* — GHSA-6wrv-9pr4-hhmw regression coverage.
*
* The upload route used to take the stored file extension straight from
* the client-supplied filename and only checked `file.mimetype` against an
* allowlist — a file could declare an image MIME type while carrying a
* `.html`/`.js` extension and arbitrary content, land in the same-origin
* `/uploads/logos` static mount, and execute as script. The mass-assignable
* `logoPath` field on PUT compounded it: an attacker could point the
* "logo" at any other uploaded file.
*
* These tests pin:
* (a) a MIME/extension mismatch is rejected at upload,
* (b) the extension actually written to disk always matches the
* validated MIME type, never the client-supplied filename,
* (c) legitimate PNG/JPEG/SVG uploads still succeed,
* (d) `logoPath` on PUT cannot be set to an arbitrary string pointing at
* another file, only to a path the upload route itself produced.
*
* Defense-in-depth (not a re-opening of the above): fileFilter only pairs
* the claimed MIME type against the extension — it can't see the bytes,
* since it runs before multer finishes writing the stream to disk. A file
* whose declared MIME/extension pair is valid but whose actual content
* doesn't match (e.g. a PNG-declared upload that isn't really a PNG) is
* now caught by validateFileContent() (magic-number check) after multer
* writes it, closing the gap where declared-vs-actual content diverges.
*/
const path = require('path');
const fs = require('fs');
const os = require('os');
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-bplogo-test-'));
process.env.NODE_ENV = 'test';
process.env.TEST_DATABASE_PATH = path.join(tmpDir, 'db.sqlite');
process.env.STORAGE_PATH = path.join(tmpDir, 'storage');
fs.mkdirSync(process.env.STORAGE_PATH, { recursive: true });
process.env.JWT_SECRET = process.env.JWT_SECRET || 'bplogo-route-test-secret';
const request = require('supertest');
const {
bootCrmDb, seedMinimal, assignAdminRole, mintAdminToken, buildRouteApp,
} = require('./helpers/crmDb');
// Real magic-number-prefixed payloads, for content-sniffing to accept.
const REAL_PNG_BYTES = Buffer.concat([
Buffer.from([0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A]),
Buffer.from('not a real png body, but the header is real'),
]);
const REAL_JPEG_BYTES = Buffer.concat([
Buffer.from([0xFF, 0xD8, 0xFF]),
Buffer.from('not a real jpeg body, but the header is real'),
]);
describe('business profile — logo upload content/extension validation', () => {
let db;
let cleanup;
let app;
let token;
const uploadLogo = (buffer, filename, mimetype) => request(app)
.post('/api/admin/business-profile/logo')
.set('Authorization', `Bearer ${token}`)
.attach('logo', buffer, { filename, contentType: mimetype });
const put = (payload) => request(app)
.put('/api/admin/business-profile')
.set('Authorization', `Bearer ${token}`)
.send(payload);
const get = () => request(app)
.get('/api/admin/business-profile')
.set('Authorization', `Bearer ${token}`);
const profileOf = (res) => (res.body.data || res.body).profile;
beforeAll(async () => {
({ db, cleanup } = await bootCrmDb());
const { adminId } = await seedMinimal(db);
await assignAdminRole(db, adminId, 'super_admin');
token = mintAdminToken(adminId);
app = buildRouteApp('/api/admin/business-profile', require('../../src/routes/adminBusinessProfile'));
}, 120000);
afterAll(async () => {
if (cleanup) await cleanup();
});
// fileFilter rejections surface via Express's generic error handler
// (the pre-existing behaviour of every sibling logo/favicon upload
// route in this codebase — none of them special-case multer's
// fileFilter `Error` into a 400 either), so the status code itself
// can be 400 or 500 depending on environment. What actually matters
// for GHSA-6wrv-9pr4-hhmw is that the request never succeeds and
// nothing with the dangerous extension is ever written to disk.
const logosDirFiles = () => {
const logosDir = path.join(process.env.STORAGE_PATH, 'uploads', 'logos');
return fs.existsSync(logosDir) ? fs.readdirSync(logosDir) : [];
};
it('rejects an HTML/script payload disguised as an image via mismatched extension', async () => {
const evil = Buffer.from('<script>alert(document.domain)</script>');
const res = await uploadLogo(evil, 'evil.html', 'image/svg+xml');
expect(res.status).not.toBe(200);
expect(logosDirFiles().some((f) => f.endsWith('.html'))).toBe(false);
});
it('rejects a .js file disguised with an image MIME type', async () => {
const evil = Buffer.from('alert(1)');
const res = await uploadLogo(evil, 'evil.js', 'image/png');
expect(res.status).not.toBe(200);
expect(logosDirFiles().some((f) => f.endsWith('.js'))).toBe(false);
});
it('rejects a disallowed MIME type outright', async () => {
const res = await uploadLogo(Buffer.from('whatever'), 'file.pdf', 'application/pdf');
expect(res.status).not.toBe(200);
expect(logosDirFiles().some((f) => f.endsWith('.pdf'))).toBe(false);
});
it('accepts a legitimate PNG upload and stores it with a .png extension', async () => {
const res = await uploadLogo(REAL_PNG_BYTES, 'logo.png', 'image/png');
expect(res.status).toBe(200);
const logoPath = (res.body.data || res.body).logoPath;
expect(logoPath).toMatch(/^\/uploads\/logos\/pdf-logo-\d+\.png$/);
const onDisk = path.join(process.env.STORAGE_PATH, logoPath.replace(/^\//, ''));
expect(fs.existsSync(onDisk)).toBe(true);
expect(profileOf(await get()).logoPath).toBe(logoPath);
});
it('accepts a legitimate JPEG upload and stores it with a .jpg extension', async () => {
const res = await uploadLogo(REAL_JPEG_BYTES, 'logo.jpg', 'image/jpeg');
expect(res.status).toBe(200);
const logoPath = (res.body.data || res.body).logoPath;
expect(logoPath).toMatch(/^\/uploads\/logos\/pdf-logo-\d+\.jpg$/);
});
it('rejects a PNG-declared upload whose bytes are not actually a PNG, and leaves nothing on disk', async () => {
const before = logosDirFiles();
const res = await uploadLogo(Buffer.from('totally not a png'), 'logo.png', 'image/png');
expect(res.status).toBe(400);
expect(res.body.error).toMatch(/content does not match/i);
// No new file left behind: the rejected upload's own file was cleaned
// up, and every other file on disk (if any) is unchanged.
expect(logosDirFiles()).toEqual(before);
});
it('accepts a legitimate SVG upload and always stores it with a .svg extension, even under a spoofed filename', async () => {
const svg = Buffer.from('<svg xmlns="http://www.w3.org/2000/svg"><rect width="1" height="1"/></svg>');
// Client-declared filename ext is .svg here to pass validateFileType
// (mismatched ext is covered by the rejection tests above); the point
// of this test is that the ON-DISK extension comes from the MIME type
// lookup table, not path.extname(originalname).
const res = await uploadLogo(svg, 'vector-logo.svg', 'image/svg+xml');
expect(res.status).toBe(200);
const logoPath = (res.body.data || res.body).logoPath;
expect(logoPath).toMatch(/^\/uploads\/logos\/pdf-logo-\d+\.svg$/);
});
it('rejects logoPath on PUT set to an arbitrary string pointing at another file', async () => {
const before = profileOf(await get()).logoPath;
const res = await put({ logoPath: '/uploads/logos/cms-somepage-1234.png' });
expect(res.status).toBe(400);
expect(profileOf(await get()).logoPath).toBe(before);
});
it('rejects logoPath on PUT with a path-traversal payload', async () => {
const res = await put({ logoPath: '/uploads/logos/../../../../etc/passwd' });
expect(res.status).toBe(400);
});
it('accepts logoPath on PUT when it matches the pattern this route itself writes', async () => {
const upload = await uploadLogo(REAL_PNG_BYTES, 'logo2.png', 'image/png');
const uploadedPath = (upload.body.data || upload.body).logoPath;
// Round-trip: PUT-ing back the exact value the upload endpoint
// returned (what the frontend's generic profile save does) must
// keep working.
const res = await put({ logoPath: uploadedPath });
expect(res.status).toBe(200);
expect(profileOf(await get()).logoPath).toBe(uploadedPath);
});
it('still allows clearing logoPath with an empty string', async () => {
const res = await put({ logoPath: '' });
expect(res.status).toBe(200);
expect(profileOf(await get()).logoPath).toBe('');
});
});
@@ -1,123 +0,0 @@
/**
* GHSA-h4w8-57xq-53fx enforcement half: `must_change_password` was written
* by the admin password-reset flow (userManagementService.resetAdminPassword)
* and returned in a few response payloads, but no route-blocking logic ever
* checked it — a reset admin could keep using the old/weak password on every
* protected route indefinitely. adminAuth() is now the server-side backstop:
* a flagged admin gets 403 MUST_CHANGE_PASSWORD on everything except the
* routes they need to clear the flag (change-password) or leave (logout).
*
* Mirrors the mocking shape of adminAuthRoleFallback.test.js — a stub `db`
* chain, no real SQLite needed, so this stays a fast unit test.
*/
const jwt = require('jsonwebtoken');
jest.mock('../../src/utils/tokenRevocation', () => ({ isTokenRevoked: jest.fn().mockResolvedValue(false) }));
jest.mock('../../src/utils/logger', () => ({ warn: jest.fn(), error: jest.fn(), debug: jest.fn(), info: jest.fn() }));
let mockMustChangePassword = false;
const mockAdminRow = { id: 7, username: 'scoped', email: 's@example.com', password_changed_at: null, role_id: 1, role_name: 'editor' };
jest.mock('../../src/database/db', () => ({
db: () => ({
leftJoin() { return this; },
where() { return this; },
select() { return this; },
first: () => Promise.resolve({ ...mockAdminRow, must_change_password: mockMustChangePassword }),
}),
}));
const { adminAuth } = require('../../src/middleware/auth');
const SECRET = 'test-secret-for-must-change-password';
function makeReq(originalUrl) {
const token = jwt.sign(
{ id: mockAdminRow.id, type: 'admin' },
SECRET,
{ algorithm: 'HS256', issuer: 'picpeak-auth' },
);
return { headers: { authorization: `Bearer ${token}` }, ip: '127.0.0.1', connection: {}, originalUrl };
}
function makeRes() {
return {
statusCode: null,
body: null,
status(code) { this.statusCode = code; return this; },
json(payload) { this.body = payload; return this; },
};
}
describe('adminAuth must_change_password enforcement (GHSA-h4w8-57xq-53fx)', () => {
const OLD_SECRET = process.env.JWT_SECRET;
beforeAll(() => { process.env.JWT_SECRET = SECRET; });
afterAll(() => { process.env.JWT_SECRET = OLD_SECRET; });
beforeEach(() => { mockMustChangePassword = false; });
it('blocks an arbitrary protected route with 403 MUST_CHANGE_PASSWORD when the flag is set', async () => {
mockMustChangePassword = true;
const req = makeReq('/api/admin/dashboard/stats');
const res = makeRes();
const next = jest.fn();
await adminAuth(req, res, next);
expect(next).not.toHaveBeenCalled();
expect(res.statusCode).toBe(403);
expect(res.body).toEqual(expect.objectContaining({ code: 'MUST_CHANGE_PASSWORD' }));
expect(req.admin).toBeUndefined();
});
it('does not block when the flag is not set', async () => {
mockMustChangePassword = false;
const req = makeReq('/api/admin/dashboard/stats');
const res = makeRes();
const next = jest.fn();
await adminAuth(req, res, next);
expect(next).toHaveBeenCalled();
expect(req.admin.mustChangePassword).toBe(false);
});
it.each([
['/api/admin/auth/change-password'],
['/api/admin/auth/logout'],
])('still allows %s through when the flag is set', async (originalUrl) => {
mockMustChangePassword = true;
const req = makeReq(originalUrl);
const res = makeRes();
const next = jest.fn();
await adminAuth(req, res, next);
expect(next).toHaveBeenCalled();
expect(req.admin.mustChangePassword).toBe(true);
expect(res.statusCode).toBeNull();
});
it('allows the exempt change-password path even with a query string', async () => {
mockMustChangePassword = true;
const req = makeReq('/api/admin/auth/change-password?foo=bar');
const res = makeRes();
const next = jest.fn();
await adminAuth(req, res, next);
expect(next).toHaveBeenCalled();
});
it('does not exempt a route that merely starts with the change-password path', async () => {
mockMustChangePassword = true;
const req = makeReq('/api/admin/auth/change-password-history');
const res = makeRes();
const next = jest.fn();
await adminAuth(req, res, next);
expect(next).not.toHaveBeenCalled();
expect(res.statusCode).toBe(403);
});
});
@@ -1,126 +0,0 @@
/**
* Same bug class as GHSA-9q5j-vqfw-32hr (fixed in adminEvents/logo.js) —
* the signed-PDF upload's multer `filename` callback built the stored
* path directly from `req.params.id` with no integer validation:
*
* filename: (req, file, cb) => {
* cb(null, `contract-${req.params.id}-${Date.now()}${ext}`);
* }
*
* `POST /:id/upload-signed-pdf` declares `param('id').isInt({ min: 1 })`,
* but express-validator's check only runs inside the route handler via
* validateRequest(req) — AFTER multer has already parsed the multipart
* body and invoked the filename callback. A traversal payload in the raw
* `:id` URL segment reaches multer completely unvalidated.
*
* Fixed by rejecting any non-positive-integer id before it is used to
* build the filename, independent of the declared-but-too-late
* express-validator check.
*/
const path = require('path');
const fs = require('fs');
const os = require('os');
// ALLOWED_MEDIA_TYPES in fileSecurityUtils.js only defines image/video
// entries, so the route's real fileFilter (validateFileType(..., ['application/pdf']))
// rejects every PDF upload with "Only PDF files are allowed" — a
// separate, pre-existing bug unrelated to the path-traversal fix under
// test here (also present in publicContracts.js, which is why neither
// suite exercises a successful upload). Stub validateFileType so this
// suite can drive the full route, including the filename-callback fix,
// end-to-end.
jest.mock('../../src/utils/fileSecurityUtils', () => {
const actual = jest.requireActual('../../src/utils/fileSecurityUtils');
return {
...actual,
validateFileType: (filename, mimetype, allowedTypes) => allowedTypes.includes(mimetype),
};
});
process.env.NODE_ENV = 'test';
process.env.TEST_DATABASE_PATH = path.join(
fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-contracts-signed-pdf-')), 'db.sqlite'
);
process.env.JWT_SECRET = process.env.JWT_SECRET || 'admin-contracts-signed-pdf-test-secret';
const request = require('supertest');
const {
bootCrmDb, seedMinimal, assignAdminRole, mintAdminToken, buildRouteApp,
} = require('../integration/helpers/crmDb');
describe('POST /api/admin/contracts/:id/upload-signed-pdf — path traversal guard', () => {
let db; let cleanup; let app; let adminId; let customerId; let token;
beforeAll(async () => {
({ db, cleanup } = await bootCrmDb());
({ adminId, customerId } = await seedMinimal(db));
await assignAdminRole(db, adminId, 'super_admin');
token = mintAdminToken(adminId);
// Feature flag defaults OFF on a fresh install — the contracts
// router 403s every route until it's on.
await db('feature_flags').where({ key: 'contracts' }).update({ value: true });
app = buildRouteApp('/api/admin/contracts', require('../../src/routes/adminContracts'));
}, 120000);
afterAll(async () => { await cleanup(); });
const auth = (req) => req.set('Authorization', `Bearer ${token}`);
const signedDir = () => path.join(process.env.STORAGE_PATH, 'uploads/contracts/signed');
async function insertContract(over = {}) {
const base = {
contract_number: `K-TEST-${Math.random().toString(16).slice(2, 8)}`,
customer_account_id: customerId,
title: 'Test Contract',
issue_date: new Date().toISOString().slice(0, 10),
status: 'sent',
language: 'de',
created_at: new Date().toISOString(),
...over,
};
const inserted = await db('contracts').insert(base).returning('id');
return inserted[0]?.id ?? inserted[0];
}
it('rejects a traversal payload in the id param instead of writing outside uploads/contracts/signed', async () => {
// '../../../../tmp/pwned' URL-encoded so the raw request path still
// has a single segment (matches Express's `:id`), but Express
// decodes the param back into literal '../' sequences before the
// route sees it.
const traversalId = encodeURIComponent('../../../../tmp/pwned');
const res = await auth(
request(app).post(`/api/admin/contracts/${traversalId}/upload-signed-pdf`)
).attach('file', Buffer.from('%PDF-1.4 fake'), 'signed.pdf');
expect(res.status).toBeGreaterThanOrEqual(400);
expect(res.body.error).toMatch(/invalid contract id/i);
// No file should have been written anywhere — the filename callback
// must error out before multer opens a write stream.
const escapedFile = path.join(os.tmpdir(), 'pwned');
expect(fs.existsSync(escapedFile)).toBe(false);
if (fs.existsSync(signedDir())) {
expect(fs.readdirSync(signedDir())).toHaveLength(0);
}
});
it('still accepts a normal numeric contract id', async () => {
const id = await insertContract();
const res = await auth(
request(app).post(`/api/admin/contracts/${id}/upload-signed-pdf`)
).attach('file', Buffer.from('%PDF-1.4 fake'), 'signed.pdf');
expect(res.status).toBe(200);
const files = fs.readdirSync(signedDir());
expect(files.some((f) => f.startsWith(`contract-${id}-`))).toBe(true);
const row = await db('contracts').where({ id }).first();
expect(row.status).toBe('fully_signed');
expect(row.signed_pdf_path).toMatch(new RegExp(`contract-${id}-`));
});
});
@@ -1,122 +0,0 @@
/**
* GHSA-9q5j-vqfw-32hr — the event-logo upload's multer `filename` callback
* built the stored path directly from `req.params.id` with no integer
* validation:
*
* filename: (req, file, cb) => {
* cb(null, `event-${req.params.id}-logo-${Date.now()}${ext}`);
* }
*
* A traversal payload in the `:id` route param (URL-encoded so it still
* matches a single Express path segment, then decoded back into literal
* `../` sequences by Express before handlers see it) could escape the
* intended uploads/logos/events/ directory. Most directly reachable via a
* super_admin session: requireEventOwnership short-circuits with next() and
* zero DB lookup for that role (src/middleware/ownership.js), so nothing
* upstream of multer validates the id first.
*
* Fixed by rejecting any non-positive-integer id before it is used to build
* the filename, regardless of role or ownership-check ordering.
*/
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-events-logo-')), 'db.sqlite'
);
process.env.JWT_SECRET = process.env.JWT_SECRET || 'admin-events-logo-test-secret';
const express = require('express');
const cookieParser = require('cookie-parser');
const request = require('supertest');
const { bootCrmDb, seedMinimal, assignAdminRole, mintAdminToken } = require('../integration/helpers/crmDb');
async function insertEvent(db, adminId, over = {}) {
const base = {
slug: `ev-${Math.random().toString(16).slice(2)}`,
event_type: 'wedding',
event_name: 'Test Wedding',
event_date: '2026-05-29',
host_email: 'host@example.com',
admin_email: 'admin@example.com',
password_hash: 'x',
share_link: `/gallery/share-${Math.random().toString(16).slice(2)}`,
share_token: `st-${Math.random().toString(16).slice(2)}`,
expires_at: new Date(Date.now() + 7 * 24 * 3600 * 1000).toISOString(),
is_active: 1, is_archived: 0, is_draft: 0,
created_by: adminId,
created_at: new Date().toISOString(),
...over,
};
const r = await db('events').insert(base).returning('id');
return r[0]?.id ?? r[0];
}
describe('POST /api/admin/events/:id/logo — path traversal guard', () => {
let db; let cleanup; let app; let adminId; let token;
beforeAll(async () => {
({ db, cleanup } = await bootCrmDb());
({ adminId } = await seedMinimal(db));
// super_admin: requireEventOwnership short-circuits with no DB lookup
// for this role, so it reaches multer with nothing upstream having
// validated the id — the exact path GHSA-9q5j-vqfw-32hr exploited.
await assignAdminRole(db, adminId, 'super_admin');
token = mintAdminToken(adminId);
app = express();
app.use(express.json());
app.use(cookieParser());
app.use('/api/admin/events', require('../../src/routes/adminEvents'));
// 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 () => { await cleanup(); });
const auth = (req) => req.set('Authorization', `Bearer ${token}`);
const logoDir = () => path.join(process.env.STORAGE_PATH, 'uploads/logos/events');
it('rejects a traversal payload in the id param instead of writing outside uploads/logos/events', async () => {
// '../../../../tmp/pwned' URL-encoded so the raw request path still has
// a single segment (matches Express's `:id`), but Express decodes the
// param back into literal '../' sequences before the route sees it.
const traversalId = encodeURIComponent('../../../../tmp/pwned');
const res = await auth(
request(app).post(`/api/admin/events/${traversalId}/logo`)
).attach('logo', Buffer.from('fake image data'), 'logo.png');
expect(res.status).toBeGreaterThanOrEqual(400);
expect(res.body.error).toMatch(/invalid event id/i);
// No file should have been written anywhere — the filename callback
// must error out before multer opens a write stream.
const escapedFile = path.join(os.tmpdir(), 'pwned');
expect(fs.existsSync(escapedFile)).toBe(false);
if (fs.existsSync(logoDir())) {
expect(fs.readdirSync(logoDir())).toHaveLength(0);
}
});
it('still accepts a normal numeric event id', async () => {
const id = await insertEvent(db, adminId, { event_name: 'Logo Event' });
const res = await auth(
request(app).post(`/api/admin/events/${id}/logo`)
).attach('logo', Buffer.from('fake image data'), 'logo.png');
expect(res.status).toBe(200);
expect(res.body.hero_logo_url).toMatch(new RegExp(`^/uploads/logos/events/event-${id}-logo-`));
const files = fs.readdirSync(logoDir());
expect(files.some((f) => f.startsWith(`event-${id}-logo-`))).toBe(true);
const row = await db('events').where({ id }).first();
expect(row.hero_logo_url).toBe(res.body.hero_logo_url);
});
});
-213
View File
@@ -38,7 +38,6 @@ const { authenticator } = require('otplib');
const {
bootCrmDb, mintAdminToken, buildRouteApp,
} = require('../integration/helpers/crmDb');
const mfaService = require('../../src/services/mfaService');
jest.setTimeout(120000);
@@ -236,138 +235,6 @@ describe('MFA disable — /api/admin/auth/mfa/disable', () => {
expect(row.two_factor_secret).toBeNull();
expect(row.two_factor_recovery_codes).toBeNull();
});
// Concurrency regression: a plain UPDATE with no conditional guard let two
// requests carrying the same captured code both read the same
// two_factor_last_used_step and both persist, defeating replay protection.
// The guarded UPDATE (mfaService.persistTotpStep) makes only the first
// writer's affected-row count > 0; the loser must be rejected.
it('two concurrent disable requests with the SAME captured code: only one succeeds', async () => {
const admin = await seedAdmin();
const { secret, token } = await enroll(admin.id);
const code = authenticator.generate(secret);
const [r1, r2] = await Promise.all([
request(adminApp)
.post('/api/admin/auth/mfa/disable')
.set('Authorization', `Bearer ${token}`)
.send({ code }),
request(adminApp)
.post('/api/admin/auth/mfa/disable')
.set('Authorization', `Bearer ${token}`)
.send({ code }),
]);
expect([r1.status, r2.status].sort()).toEqual([200, 400]);
const status = await request(adminApp)
.get('/api/admin/auth/mfa/status')
.set('Authorization', `Bearer ${token}`);
expect(status.body.enabled).toBe(false);
});
});
describe('MFA regenerate recovery codes — /api/admin/auth/mfa/recovery-codes', () => {
it('a valid TOTP regenerates the recovery codes and persists the step', async () => {
const admin = await seedAdmin();
const { secret, token } = await enroll(admin.id);
const res = await request(adminApp)
.post('/api/admin/auth/mfa/recovery-codes')
.set('Authorization', `Bearer ${token}`)
.send({ code: authenticator.generate(secret) });
expect(res.status).toBe(200);
expect(res.body.recoveryCodes).toHaveLength(10);
});
it('a wrong code is rejected (400)', async () => {
const admin = await seedAdmin();
const { secret, token } = await enroll(admin.id);
const valid = authenticator.generate(secret);
const wrong = valid === '000000' ? '111111' : '000000';
const res = await request(adminApp)
.post('/api/admin/auth/mfa/recovery-codes')
.set('Authorization', `Bearer ${token}`)
.send({ code: wrong });
expect(res.status).toBe(400);
});
// Concurrency regression (see the disable test above for the mechanism):
// this is the endpoint called out as the worst lost-update case, since it
// both rotates the recovery codes and (previously) persisted the step in
// one unconditional UPDATE.
it('two concurrent regenerations with the SAME captured code: only one succeeds', async () => {
const admin = await seedAdmin();
const { secret, token } = await enroll(admin.id);
const code = authenticator.generate(secret);
const [r1, r2] = await Promise.all([
request(adminApp)
.post('/api/admin/auth/mfa/recovery-codes')
.set('Authorization', `Bearer ${token}`)
.send({ code }),
request(adminApp)
.post('/api/admin/auth/mfa/recovery-codes')
.set('Authorization', `Bearer ${token}`)
.send({ code }),
]);
expect([r1.status, r2.status].sort()).toEqual([200, 400]);
const winner = r1.status === 200 ? r1 : r2;
expect(winner.body.recoveryCodes).toHaveLength(10);
const row = await db('admin_users').where({ id: admin.id }).first();
expect(row.two_factor_last_used_step).not.toBeNull();
});
});
describe('mfaService.persistTotpStep — atomic replay-tracking persist', () => {
// Deterministic simulation of the race: two "concurrent" requests that
// read the SAME two_factor_last_used_step and computed the SAME totpStep
// from the same captured code. Calling persistTotpStep twice in a row with
// that identical totpStep reproduces exactly the DB-level outcome of a
// true race, without relying on event-loop timing.
it('the second writer with the same totpStep affects 0 rows and is rejected', async () => {
const admin = await seedAdmin();
const { secret } = await enroll(admin.id);
const row = await db('admin_users').where({ id: admin.id }).first();
const code = authenticator.generate(secret);
const totpStep = mfaService.verifyTotpEncryptedStep(code, row.two_factor_secret, null);
expect(totpStep).toEqual(expect.any(Number));
const first = await mfaService.persistTotpStep(db, admin.id, totpStep, { updated_at: new Date() });
expect(first).toBe(true);
// The row's two_factor_last_used_step has now already advanced to
// totpStep by the time this "losing" write runs — the guard condition
// (whereNull OR < totpStep) is false, so 0 rows are affected.
const second = await mfaService.persistTotpStep(db, admin.id, totpStep, { updated_at: new Date() });
expect(second).toBe(false);
const after = await db('admin_users').where({ id: admin.id }).first();
expect(Number(after.two_factor_last_used_step)).toBe(totpStep);
});
it('succeeds when the new step advances past the current one', async () => {
const admin = await seedAdmin();
const { secret } = await enroll(admin.id);
const row = await db('admin_users').where({ id: admin.id }).first();
const code = authenticator.generate(secret);
const totpStep = mfaService.verifyTotpEncryptedStep(code, row.two_factor_secret, null);
const ok = await mfaService.persistTotpStep(db, admin.id, totpStep, {});
expect(ok).toBe(true);
const nextStepAuthenticator = authenticator.clone({ epoch: Date.now() + 30000 });
const nextCode = nextStepAuthenticator.generate(secret);
const nextStep = mfaService.verifyTotpEncryptedStep(nextCode, row.two_factor_secret, totpStep);
expect(nextStep).toBeGreaterThan(totpStep);
const advanced = await mfaService.persistTotpStep(db, admin.id, nextStep, {});
expect(advanced).toBe(true);
});
});
describe('Admin login challenge — /api/auth/admin/login[/mfa]', () => {
@@ -417,86 +284,6 @@ describe('Admin login challenge — /api/auth/admin/login[/mfa]', () => {
expect(res.body.user.id).toBe(admin.id);
});
// GHSA-qcwx-r25m-j869: verifyTotp() was stateless, so otplib's window:1
// tolerance let the same 6-digit code complete two independent logins
// within its ~90s validity window. mfaService now tracks each admin's
// last-consumed TOTP step and rejects a code that doesn't advance past it.
it('#GHSA-qcwx-r25m-j869 — a TOTP code cannot be replayed into a second login', async () => {
const admin = await seedAdmin();
const { secret } = await enroll(admin.id);
const code = authenticator.generate(secret);
// First use of the code completes a login.
const c1 = await request(authApp)
.post('/api/auth/admin/login')
.send({ username: admin.username, password: admin.password });
const first = await request(authApp)
.post('/api/auth/admin/login/mfa')
.send({ mfaToken: c1.body.mfaToken, code });
expect(first.status).toBe(200);
expect(first.body.user).toBeDefined();
// Replaying the SAME code for an independent second login must fail,
// even though otplib's window:1 tolerance still considers it valid.
const c2 = await request(authApp)
.post('/api/auth/admin/login')
.send({ username: admin.username, password: admin.password });
const replay = await request(authApp)
.post('/api/auth/admin/login/mfa')
.send({ mfaToken: c2.body.mfaToken, code });
expect(replay.status).toBe(401);
expect(replay.body.code).toBe('MFA_INVALID');
expect(replay.body.user).toBeUndefined();
// A freshly generated code for the NEXT TOTP step is not a replay and
// succeeds. Generated via a cloned authenticator with a future epoch
// rather than mocking Date.now(), so mfaService's own step computation
// (real Date.now()) still lands the match one step ahead.
const nextStepAuthenticator = authenticator.clone({ epoch: Date.now() + 30000 });
const nextCode = nextStepAuthenticator.generate(secret);
const c3 = await request(authApp)
.post('/api/auth/admin/login')
.send({ username: admin.username, password: admin.password });
const third = await request(authApp)
.post('/api/auth/admin/login/mfa')
.send({ mfaToken: c3.body.mfaToken, code: nextCode });
expect(third.status).toBe(200);
expect(third.body.user).toBeDefined();
expect(third.body.user.id).toBe(admin.id);
});
// Concurrency regression: verifyTotpEncryptedStep()'s "does this advance"
// check was read against a snapshot taken earlier in the request, then a
// PLAIN update persisted the step — two concurrent requests carrying the
// SAME captured code could both pass the check and both complete a login
// before either write landed. The persist is now a conditional UPDATE
// (mfaService.persistTotpStep), so only the first writer's affected-row
// count is > 0 and the other is correctly treated as a replay.
it('two concurrent login/mfa requests with the SAME captured code: only one completes', async () => {
const admin = await seedAdmin();
const { secret } = await enroll(admin.id);
const code = authenticator.generate(secret);
const c1 = await request(authApp)
.post('/api/auth/admin/login')
.send({ username: admin.username, password: admin.password });
const c2 = await request(authApp)
.post('/api/auth/admin/login')
.send({ username: admin.username, password: admin.password });
const [r1, r2] = await Promise.all([
request(authApp).post('/api/auth/admin/login/mfa').send({ mfaToken: c1.body.mfaToken, code }),
request(authApp).post('/api/auth/admin/login/mfa').send({ mfaToken: c2.body.mfaToken, code }),
]);
expect([r1.status, r2.status].sort()).toEqual([200, 401]);
const winner = r1.status === 200 ? r1 : r2;
const loser = r1.status === 200 ? r2 : r1;
expect(winner.body.user).toBeDefined();
expect(loser.body.user).toBeUndefined();
expect(loser.body.code).toBe('MFA_INVALID');
});
it('login/mfa with a wrong code is 401 MFA_INVALID', async () => {
const admin = await seedAdmin();
const { secret } = await enroll(admin.id);
@@ -1,151 +0,0 @@
/**
* GHSA-9h7q-2jpf-vj85 — DELETE /api/admin/short-urls/:id only checked
* `events.edit` permission, with no ownership scoping. GET and POST for an
* event's short URLs both chain requireEventOwnership; DELETE takes the
* short URL row's own :id (not :eventId), so any admin holding events.edit
* could delete another admin's branded gallery short URL. The route now
* resolves the short URL's event first and applies the same ownership
* predicate requireEventOwnership uses. 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-suown-')), 'db.sqlite',
);
process.env.JWT_SECRET = process.env.JWT_SECRET || 'suown-test-secret';
process.env.STORAGE_PATH = fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-suown-storage-'));
const request = require('supertest');
const express = require('express');
const cookieParser = require('cookie-parser');
const { bootCrmDb, assignAdminRole, mintAdminToken } = require('../integration/helpers/crmDb');
describe('short URL delete ownership scoping', () => {
let db; let cleanup; let app; let service;
let superTok; let ownerTok; let foreignTok;
let ownerId;
let foreignShortUrlId;
const auth = (req, tok) => req.set('Authorization', `Bearer ${tok}`);
async function seedEvent(createdBy, slugSuffix) {
const farFuture = new Date(Date.now() + 365 * 86400000).toISOString();
const [id] = await db('events').insert({
slug: `suown-${slugSuffix}`,
event_type: 'wedding',
event_name: 'Test Event',
event_date: '2026-08-01',
host_email: 'h@e.com',
admin_email: 'a@e.com',
password_hash: 'x',
share_link: `suown-${slugSuffix}`,
share_token: `suown-share-${slugSuffix}`,
expires_at: farFuture,
is_active: true,
is_archived: false,
created_by: createdBy,
created_at: new Date().toISOString(),
});
return db('events').where({ id }).first();
}
beforeAll(async () => {
({ db, cleanup } = await bootCrmDb());
service = require('../../src/services/galleryShortUrlService');
const superIns = await db('admin_users').insert({
username: 'suown-super', email: 'suown-super@example.com',
password_hash: 'x', must_change_password: false, created_at: new Date(),
}).returning('id');
const superId = superIns[0]?.id ?? superIns[0];
await assignAdminRole(db, superId, 'super_admin');
superTok = mintAdminToken(superId);
const ownerIns = await db('admin_users').insert({
username: 'suown-owner', email: 'suown-owner@example.com',
password_hash: 'x', must_change_password: false, created_at: new Date(),
}).returning('id');
ownerId = ownerIns[0]?.id ?? ownerIns[0];
await assignAdminRole(db, ownerId, 'editor');
ownerTok = mintAdminToken(ownerId);
const foreignIns = await db('admin_users').insert({
username: 'suown-foreign', email: 'suown-foreign@example.com',
password_hash: 'x', must_change_password: false, created_at: new Date(),
}).returning('id');
const foreignId = foreignIns[0]?.id ?? foreignIns[0];
await assignAdminRole(db, foreignId, 'editor');
foreignTok = mintAdminToken(foreignId);
// Event owned by `owner`, NOT `foreign`.
await seedEvent(ownerId, 'owned');
app = express();
app.use(express.json());
app.use(cookieParser());
app.use('/api/admin', require('../../src/routes/adminShortUrls'));
}, 120000);
afterAll(async () => { if (cleanup) await cleanup(); });
beforeEach(async () => {
// Fresh short URL per DELETE test so earlier deletes don't interfere.
const event = await db('events').where({ created_by: ownerId }).first();
const row = await service.createShortUrl({
eventId: event.id,
customSlug: `suown-target-${Date.now()}-${Math.random().toString(36).slice(2, 6)}`,
createdBy: ownerId,
});
foreignShortUrlId = row.id;
});
it('an admin who does not own the event cannot delete its short URL (403, row survives)', async () => {
const res = await auth(
request(app).delete(`/api/admin/short-urls/${foreignShortUrlId}`),
foreignTok,
);
expect(res.status).toBe(403);
const row = await db('gallery_short_urls').where({ id: foreignShortUrlId }).first();
expect(row).toBeDefined();
expect(row.deleted_at).toBeFalsy();
});
it('the owning admin can delete its own short URL', async () => {
const res = await auth(
request(app).delete(`/api/admin/short-urls/${foreignShortUrlId}`),
ownerTok,
);
expect(res.status).toBe(204);
const row = await db('gallery_short_urls').where({ id: foreignShortUrlId }).first();
expect(row.deleted_at).toBeTruthy();
});
it('super_admin can delete any short URL', async () => {
const res = await auth(
request(app).delete(`/api/admin/short-urls/${foreignShortUrlId}`),
superTok,
);
expect(res.status).toBe(204);
const row = await db('gallery_short_urls').where({ id: foreignShortUrlId }).first();
expect(row.deleted_at).toBeTruthy();
});
it('deleting a nonexistent short URL id returns 404', async () => {
const res = await auth(
request(app).delete('/api/admin/short-urls/9999999'),
superTok,
);
expect(res.status).toBe(404);
});
it('deleting a nonexistent short URL id as a non-owner also returns 404 (existence check runs first)', async () => {
const res = await auth(
request(app).delete('/api/admin/short-urls/9999999'),
foreignTok,
);
expect(res.status).toBe(404);
});
});
@@ -1,122 +0,0 @@
/**
* PUT /api/admin/database-backup/config must reject a
* database_backup_destination_path that resolves inside a publicly served
* directory (GHSA-jw8m-43r2-jqrm class, #1365).
*
* Before #1365, database_backup_destination_path was silently ignored by
* databaseBackupService.backup() (a destructuring bug always fell back to
* the hardcoded /backup/database), so this setting being freely writable by
* any backup.create holder — the built-in `admin` role has it without
* settings.edit or backup.restore — was harmless. Making the setting
* actually take effect reopens the exact exfiltration path GHSA-jw8m fixed
* for the per-request override, through the persisted setting instead.
*/
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-dbbackup-config-')), 'db.sqlite',
);
process.env.JWT_SECRET = process.env.JWT_SECRET || 'dbbackup-config-test-secret';
process.env.STORAGE_PATH = fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-dbbackup-storage-'));
const request = require('supertest');
const express = require('express');
const bcrypt = require('bcrypt');
const jwt = require('jsonwebtoken');
const { bootCrmDb, seedMinimal } = require('../integration/helpers/crmDb');
describe('database backup destination-path config guard (GHSA-jw8m class, #1365)', () => {
let db; let cleanup; let app; let adminToken;
beforeAll(async () => {
({ db, cleanup } = await bootCrmDb());
await seedMinimal(db);
const role = await db('roles').where({ name: 'admin' }).first();
const r = await db('admin_users').insert({
username: 'limited-admin',
email: 'limited-admin-config@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];
adminToken = jwt.sign(
{ id, username: 'limited-admin', type: 'admin', role: 'admin', loginTime: Date.now() },
process.env.JWT_SECRET,
{ expiresIn: '1h', issuer: 'picpeak-auth' },
);
app = express();
app.use(express.json());
app.use('/api/admin/database-backup', require('../../src/routes/adminDatabaseBackup'));
}, 120000);
afterAll(async () => { if (cleanup) await cleanup(); });
it('rejects a destination inside the public uploads/logos mount', async () => {
const res = await request(app)
.put('/api/admin/database-backup/config')
.set('Authorization', `Bearer ${adminToken}`)
.send({ database_backup_destination_path: path.join(process.env.STORAGE_PATH, 'uploads', 'logos') });
expect(res.status).toBe(400);
// The seeded default must survive untouched — the rejected value never lands.
const row = await db('app_settings').where({ setting_key: 'database_backup_destination_path' }).first();
expect(JSON.parse(row.setting_value)).toBe('/backup/database');
});
it('rejects a destination inside the public fonts mount', async () => {
const res = await request(app)
.put('/api/admin/database-backup/config')
.set('Authorization', `Bearer ${adminToken}`)
.send({ database_backup_destination_path: path.join(process.env.STORAGE_PATH, 'fonts') });
expect(res.status).toBe(400);
});
it('accepts a destination outside any public mount', async () => {
const safePath = path.join(process.env.STORAGE_PATH, 'db-backups');
const res = await request(app)
.put('/api/admin/database-backup/config')
.set('Authorization', `Bearer ${adminToken}`)
.send({ database_backup_destination_path: safePath });
expect(res.status).toBe(200);
const row = await db('app_settings').where({ setting_key: 'database_backup_destination_path' }).first();
expect(JSON.parse(row.setting_value)).toBe(safePath);
});
// A retention of 0 or less pushes cleanupOldBackups' cutoff to today or
// the future, deleting every completed backup on the next scheduled run
// — a backup.create holder achieving what backup.delete gates on /cleanup.
it.each([-1, 0])('rejects database_backup_retention_days=%s', async (bad) => {
const res = await request(app)
.put('/api/admin/database-backup/config')
.set('Authorization', `Bearer ${adminToken}`)
.send({ database_backup_retention_days: bad });
expect(res.status).toBe(400);
});
it('accepts a positive database_backup_retention_days', async () => {
const res = await request(app)
.put('/api/admin/database-backup/config')
.set('Authorization', `Bearer ${adminToken}`)
.send({ database_backup_retention_days: 90 });
expect(res.status).toBe(200);
const row = await db('app_settings').where({ setting_key: 'database_backup_retention_days' }).first();
expect(JSON.parse(row.setting_value)).toBe(90);
});
});
@@ -1,268 +0,0 @@
/**
* Previewing an unpublished gallery through its SHORT share URL (#1386).
*
* /info has honoured admin_preview since #868, but two sibling routes never
* did, and both sit on the short-URL path:
*
* GET /resolve/:identifier — filtered drafts out via ACTIVE_EVENT_FILTER
* GET /:slug/verify-token/:token — same, inline
*
* With "use short gallery URLs" OFF the admin's View Gallery link carries the
* slug, GalleryPage never calls /resolve, and the preview worked. With it ON
* the link is the token form, GalleryPage resolves it first, and the draft
* 404'd as "Gallery Not Found" — which is exactly what was reported.
*
* The relaxation is admin-preview-only, so the other half of these tests is
* the part that must NOT move: anonymous callers still get 404 for a draft,
* and GHSA-rh8r's rule (never hand a share_token back on a bare slug lookup)
* has to survive the new path too.
*/
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-draft-preview-')), 'db.sqlite',
);
process.env.JWT_SECRET = process.env.JWT_SECRET || 'draft-preview-test-secret';
process.env.STORAGE_PATH = fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-draft-preview-storage-'));
const request = require('supertest');
const express = require('express');
const cookieParser = require('cookie-parser');
const { bootCrmDb, seedMinimal, assignAdminRole, mintAdminToken } = require('../integration/helpers/crmDb');
// Share-token fixtures, deliberately low-entropy and obviously fake. They
// have to satisfy SHARE_TOKEN_REGEX (32 hex chars), and random-looking hex of
// that shape is exactly what secret scanners flag — GitGuardian raised two
// "Generic High Entropy Secret" findings on the first version of this file.
const DRAFT_SLUG = 'draft-preview-event';
const DRAFT_TOKEN = 'deadbeefdeadbeefdeadbeefdeadbeef';
const LIVE_SLUG = 'published-event';
const LIVE_TOKEN = 'feedfacefeedfacefeedfacefeedface';
describe('draft preview through the short share URL (#1386)', () => {
let db; let cleanup; let app; let adminId; let foreignId;
// Two transports. admin_preview=1 is an intent flag authenticated by the
// admin cookie — what the frontend sends. ?preview=<jwt> is the legacy
// hand-built-link form, kept working.
const preview = (id = adminId) => `preview=${mintAdminToken(id)}`;
const asAdmin = (req, id = adminId) => req.set('Cookie', `admin_token=${mintAdminToken(id)}`);
async function insertEvent({ slug, token, isDraft }) {
await db('events').insert({
slug,
event_type: 'wedding',
event_name: slug,
event_date: '2026-09-01',
host_email: 'h@example.com',
admin_email: 'a@example.com',
password_hash: 'x',
share_link: `/gallery/${slug}/${token}`,
share_token: token,
require_password: 0,
expires_at: new Date(Date.now() + 7 * 864e5).toISOString(),
is_active: 1,
is_archived: 0,
is_draft: isDraft ? 1 : 0,
created_by: adminId,
created_at: new Date().toISOString(),
});
}
beforeAll(async () => {
({ db, cleanup } = await bootCrmDb());
({ adminId } = await seedMinimal(db));
await assignAdminRole(db, adminId);
const [row] = await db('admin_users').insert({
username: 'foreign', email: 'foreign@example.test', password_hash: 'unused', is_active: 1,
}).returning('id');
foreignId = row?.id ?? row;
await assignAdminRole(db, foreignId, 'viewer');
await insertEvent({ slug: DRAFT_SLUG, token: DRAFT_TOKEN, isDraft: true });
await insertEvent({ slug: LIVE_SLUG, token: LIVE_TOKEN, isDraft: false });
app = express();
app.use(express.json());
app.use(cookieParser());
app.use('/api/gallery', require('../../src/routes/gallery'));
}, 120000);
afterAll(async () => { if (cleanup) await cleanup(); });
describe('the reported case — admin previewing a draft', () => {
it('resolves the draft by share token (was 404 "Gallery Not Found")', async () => {
const res = await request(app).get(`/api/gallery/resolve/${DRAFT_TOKEN}?${preview()}`);
expect(res.status).toBe(200);
expect(res.body.slug).toBe(DRAFT_SLUG);
expect(res.body.matchType).toBe('token');
});
it('resolves the draft by full share link', async () => {
const identifier = encodeURIComponent(`/gallery/${DRAFT_SLUG}/${DRAFT_TOKEN}`);
const res = await request(app).get(`/api/gallery/resolve/${identifier}?${preview()}`);
expect(res.status).toBe(200);
expect(res.body.slug).toBe(DRAFT_SLUG);
});
it('clears verify-token for the draft, the next step of the same flow', async () => {
const res = await request(app)
.get(`/api/gallery/${DRAFT_SLUG}/verify-token/${DRAFT_TOKEN}?${preview()}`);
expect(res.status).toBe(200);
expect(res.body.valid).toBe(true);
});
});
// The transport the SHIPPED frontend uses. The first cut of this fix only
// tested ?preview=, which the browser never sends on an API call — so the
// suite passed while the feature stayed broken end to end. Caught in review.
describe('admin_preview=1 authenticated by the admin cookie', () => {
it('resolves the draft', async () => {
const res = await asAdmin(
request(app).get(`/api/gallery/resolve/${DRAFT_TOKEN}?admin_preview=1`),
);
expect(res.status).toBe(200);
expect(res.body.slug).toBe(DRAFT_SLUG);
});
it('clears verify-token', async () => {
const res = await asAdmin(
request(app).get(`/api/gallery/${DRAFT_SLUG}/verify-token/${DRAFT_TOKEN}?admin_preview=1`),
);
expect(res.status).toBe(200);
expect(res.body.valid).toBe(true);
});
it('serves /info for the draft', async () => {
const res = await asAdmin(
request(app).get(`/api/gallery/${DRAFT_SLUG}/info?admin_preview=1`),
);
expect(res.status).toBe(200);
});
it('serves draft MEDIA, which is what the flag on the URL is for', async () => {
// AuthenticatedImage/Video use native fetch and never see the axios
// interceptor, so the flag has to travel on the media URL itself. Without
// it the preview loaded metadata and showed no images at all.
const res = await asAdmin(
request(app).get(`/api/gallery/${DRAFT_SLUG}/photos?admin_preview=1`),
);
expect(res.status).toBe(200);
});
it('404s with the flag but no admin cookie — the flag authorizes nothing', async () => {
const res = await request(app).get(`/api/gallery/resolve/${DRAFT_TOKEN}?admin_preview=1`);
expect(res.status).toBe(404);
});
it('404s with the flag and a cookie that is not an admin JWT', async () => {
const res = await request(app)
.get(`/api/gallery/resolve/${DRAFT_TOKEN}?admin_preview=1`)
.set('Cookie', 'admin_token=not-a-jwt');
expect(res.status).toBe(404);
});
});
describe('what must not move', () => {
it('404s an anonymous resolve of the draft token', async () => {
const res = await request(app).get(`/api/gallery/resolve/${DRAFT_TOKEN}`);
expect(res.status).toBe(404);
});
it('404s when ?preview= carries a token that is not a valid admin JWT', async () => {
const res = await request(app).get(`/api/gallery/resolve/${DRAFT_TOKEN}?preview=not-a-jwt`);
expect(res.status).toBe(404);
});
it('404s when ?preview= is absent entirely', async () => {
const res = await request(app).get(`/api/gallery/resolve/${DRAFT_TOKEN}?preview=`);
expect(res.status).toBe(404);
});
it('404s a non-owning admin on verify-token too (#1411)', async () => {
// This route selected its own columns and omitted created_by, so the
// ownership check saw an ownerless event and waved the caller through
// while /resolve and /info refused them.
const res = await asAdmin(
request(app).get(`/api/gallery/${DRAFT_SLUG}/verify-token/${DRAFT_TOKEN}?admin_preview=1`),
foreignId,
);
expect(res.status).toBe(404);
});
it('404s an admin who does not own the event (#1411)', async () => {
// Was 200: a valid signature was the whole check, so any admin previewed
// any draft, including another photographer's. Now ownership applies —
// the same rule requireEventOwnership enforces everywhere else.
const res = await asAdmin(
request(app).get(`/api/gallery/resolve/${DRAFT_TOKEN}?admin_preview=1`),
foreignId,
);
expect(res.status).toBe(404);
const info = await asAdmin(
request(app).get(`/api/gallery/${DRAFT_SLUG}/info?admin_preview=1`),
foreignId,
);
expect(info.status).toBe(404);
});
it('404s an admin whose role grants no gallery permissions (#1411)', async () => {
// The owner, but stripped of events.view/photos.view.
const original = (await db('admin_users').where({ id: adminId }).first()).role_id;
await db('admin_users').where({ id: adminId }).update({ role_id: null });
try {
const res = await asAdmin(
request(app).get(`/api/gallery/resolve/${DRAFT_TOKEN}?admin_preview=1`),
);
expect(res.status).toBe(404);
} finally {
await db('admin_users').where({ id: adminId }).update({ role_id: original });
}
});
it('404s an admin whose account has been deactivated (#1411)', async () => {
await db('admin_users').where({ id: adminId }).update({ is_active: 0 });
try {
const res = await asAdmin(
request(app).get(`/api/gallery/resolve/${DRAFT_TOKEN}?admin_preview=1`),
);
expect(res.status).toBe(404);
} finally {
await db('admin_users').where({ id: adminId }).update({ is_active: 1 });
}
});
it('404s an anonymous verify-token for the draft', async () => {
const res = await request(app)
.get(`/api/gallery/${DRAFT_SLUG}/verify-token/${DRAFT_TOKEN}`);
expect(res.status).toBe(404);
});
it('still withholds the share_token on a bare slug lookup (GHSA-rh8r)', async () => {
// The draft path must not become a way around the token-withholding rule.
const res = await request(app).get(`/api/gallery/resolve/${DRAFT_SLUG}?${preview()}`);
expect(res.status).toBe(200);
expect(res.body.matchType).toBe('slug');
expect(res.body.token).toBeUndefined();
expect(res.body.share_link).toBeUndefined();
expect(res.body.share_url).toBeUndefined();
expect(JSON.stringify(res.body)).not.toContain(DRAFT_TOKEN);
});
it('leaves the published gallery resolving anonymously, as before', async () => {
const res = await request(app).get(`/api/gallery/resolve/${LIVE_TOKEN}`);
expect(res.status).toBe(200);
expect(res.body.slug).toBe(LIVE_SLUG);
expect(res.body.token).toBe(LIVE_TOKEN);
});
it('still 404s an identifier that matches nothing', async () => {
const res = await request(app).get(`/api/gallery/resolve/no-such-gallery?${preview()}`);
expect(res.status).toBe(404);
});
});
});
@@ -1,163 +0,0 @@
/**
* Videos under enhanced/maximum image protection (#1370).
*
* Both halves of the video path used to be routed through /api/secure-images
* once an event left `standard` protection, and neither half could carry a
* video:
*
* 1. galleryQueryService emitted `/api/secure-images/{slug}/secure/{id}/{{token}}`
* as the video's `url`. The lightbox drops that straight into a <video>
* element, nothing substitutes `{{token}}` (the helper that could is
* unreferenced), and the route answers 403 "Invalid or expired token".
* 2. Even with a valid token it would still fail: the secure-images route
* pipes every byte through secureImageService.processProtectedImage,
* which calls sharp() and throws on an mp4 → 404.
*
* The guest saw a poster frozen at 0:00 with no error of any kind.
*
* Videos now keep the JWT route at every protection level. That is not a new
* exposure — thumbnails of those same videos have always been served from it —
* so these tests also pin the inverse: still images must keep bouncing to the
* secure endpoint. Every assertion here fails on the unfixed code except the
* two guarding images.
*/
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-video-urls-')), 'db.sqlite',
);
process.env.JWT_SECRET = process.env.JWT_SECRET || 'video-urls-test-secret';
process.env.STORAGE_PATH = fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-video-urls-storage-'));
const request = require('supertest');
const express = require('express');
const cookieParser = require('cookie-parser');
const { bootCrmDb, seedMinimal } = require('../integration/helpers/crmDb');
const SLUG = 'protected-video-gallery';
const VIDEO_BYTES = Buffer.from('not really an mp4, but the route only streams bytes');
describe('videos stay playable under enhanced/maximum protection (#1370)', () => {
let db; let cleanup; let app; let eventId; let videoId; let imageId;
async function setProtection(level) {
await db('events').where('id', eventId).update({ protection_level: level });
}
async function photoPayload(id) {
const res = await request(app).get(`/api/gallery/${SLUG}/photos`);
expect(res.status).toBe(200);
const photo = res.body.photos.find((p) => p.id === id);
expect(photo).toBeDefined();
return photo;
}
beforeAll(async () => {
({ db, cleanup } = await bootCrmDb());
await seedMinimal(db);
const ev = await db('events').insert({
slug: SLUG,
event_type: 'wedding',
event_name: 'Protected Video',
event_date: '2026-09-01',
host_email: 'h@example.com',
admin_email: 'a@example.com',
password_hash: 'x',
share_link: `/gallery/${SLUG}/s`,
share_token: 'protected-video-share',
expires_at: new Date(Date.now() + 7 * 864e5).toISOString(),
is_active: 1,
is_archived: 0,
is_draft: 0,
// Password-free so verifyGalleryAccess takes the public path, same as
// the sibling gallery suites.
require_password: 0,
created_at: new Date().toISOString(),
}).returning('id');
eventId = ev[0]?.id ?? ev[0];
const mediaDir = path.join(process.env.STORAGE_PATH, 'events/active', SLUG, 'individual');
fs.mkdirSync(mediaDir, { recursive: true });
fs.writeFileSync(path.join(mediaDir, 'clip.mp4'), VIDEO_BYTES);
fs.writeFileSync(path.join(mediaDir, 'still.jpg'), Buffer.from('jpeg-ish'));
const vid = await db('photos').insert({
event_id: eventId,
filename: 'clip.mp4',
path: `${SLUG}/individual/clip.mp4`,
type: 'individual',
media_type: 'video',
mime_type: 'video/mp4',
duration: 43,
uploaded_at: new Date().toISOString(),
}).returning('id');
videoId = vid[0]?.id ?? vid[0];
const img = await db('photos').insert({
event_id: eventId,
filename: 'still.jpg',
path: `${SLUG}/individual/still.jpg`,
type: 'individual',
uploaded_at: new Date().toISOString(),
}).returning('id');
imageId = img[0]?.id ?? img[0];
app = express();
app.use(express.json());
app.use(cookieParser());
app.use('/api/gallery', require('../../src/routes/gallery'));
}, 120000);
afterAll(async () => { if (cleanup) await cleanup(); });
describe.each(['enhanced', 'maximum'])('protection_level = %s', (level) => {
beforeAll(async () => { await setProtection(level); });
test('the video url is the JWT route, not a {{token}} template', async () => {
const photo = await photoPayload(videoId);
expect(photo.url).toBe(`/api/gallery/${SLUG}/photo/${videoId}`);
expect(photo.url).not.toContain('{{token}}');
expect(photo.requires_token).toBe(false);
});
test('the video streams instead of bouncing to the secure endpoint', async () => {
const res = await request(app).get(`/api/gallery/${SLUG}/photo/${videoId}`);
expect(res.status).toBe(200);
expect(res.headers['content-type']).toBe('video/mp4');
expect(res.headers['accept-ranges']).toBe('bytes');
expect(Buffer.from(res.body)).toEqual(VIDEO_BYTES);
});
test('range requests still work, so seeking is possible', async () => {
const res = await request(app)
.get(`/api/gallery/${SLUG}/photo/${videoId}`)
.set('Range', 'bytes=0-9');
expect(res.status).toBe(206);
expect(res.headers['content-range']).toBe(`bytes 0-9/${VIDEO_BYTES.length}`);
});
test('still images keep bouncing to the secure endpoint', async () => {
const photo = await photoPayload(imageId);
expect(photo.url).toBe(`/api/secure-images/${SLUG}/secure/${imageId}/{{token}}`);
expect(photo.requires_token).toBe(true);
const res = await request(app).get(`/api/gallery/${SLUG}/photo/${imageId}`);
expect(res.status).toBe(302);
expect(res.body.error).toBe('Secure access required');
});
});
describe('protection_level = standard', () => {
beforeAll(async () => { await setProtection('standard'); });
test('both media types take the JWT route, as before', async () => {
expect((await photoPayload(videoId)).url).toBe(`/api/gallery/${SLUG}/photo/${videoId}`);
expect((await photoPayload(imageId)).url).toBe(`/api/gallery/${SLUG}/photo/${imageId}`);
});
});
});
@@ -25,18 +25,14 @@ process.env.STORAGE_PATH = path.join(tmpDir, 'storage');
fs.mkdirSync(process.env.STORAGE_PATH, { recursive: true });
process.env.JWT_SECRET = process.env.JWT_SECRET || 'crm-route-test-secret';
const express = require('express');
const cookieParser = require('cookie-parser');
const request = require('supertest');
const { bootCrmDb, seedMinimal, createPublicToken, buildRouteApp } = require('../integration/helpers/crmDb');
const tokenGuards = require('../../src/utils/publicTokenGuards');
const { errorHandler } = require('../../src/middleware/errorHandler');
describe('publicContracts routes', () => {
let db;
let cleanup;
let app;
let appWithErrorHandler;
let customerId;
let contractId;
@@ -55,17 +51,6 @@ describe('publicContracts routes', () => {
contractId = inserted[0]?.id ?? inserted[0];
app = buildRouteApp('/api/public/contracts', require('../../src/routes/publicContracts'));
// A second app instance wired to the REAL production error handler
// (buildRouteApp's is a simplified stand-in that only reads
// err.statusCode/err.status, which a bare MulterError doesn't set).
// Used below to verify the actual 4xx contract end-to-end, not just
// that multer aborted the request.
appWithErrorHandler = express();
appWithErrorHandler.use(express.json());
appWithErrorHandler.use(cookieParser());
appWithErrorHandler.use('/api/public/contracts', require('../../src/routes/publicContracts'));
appWithErrorHandler.use(errorHandler);
}, 120000);
afterAll(async () => {
@@ -146,40 +131,6 @@ describe('publicContracts routes', () => {
.attach('file', Buffer.from('%PDF-1.4 fake'), 'signed.pdf');
expect(res.status).toBe(404);
});
// CVE-2026-82333 regression (#1374 follow-up): multer 2.3.0 added an
// opt-in `fieldArrayIndexLimit` that must be set to actually close the
// field-parser DoS — the version bump alone does nothing. This route is
// unauthenticated (token-in-URL only), so it's the sharpest place to
// prove a crafted request with an oversized array-index field name
// (`evil[999999999]`) is rejected rather than accepted or left to hang.
it('rejects a multipart request with an oversized array-index field name', async () => {
const token = await createPublicToken(db, 'contract_action_tokens', {
contract_id: contractId,
});
const res = await request(app)
.post(`/api/public/contracts/${token}/upload-signed-pdf`)
.field('evil[999999999]', 'x')
.attach('file', Buffer.from('%PDF-1.4 fake'), 'signed.pdf');
// multer aborts the request before the handler runs; buildRouteApp's
// generic error handler falls back to 500 for a bare MulterError
// (see appWithErrorHandler test below for the real 4xx contract), so
// here we only assert the upload was NOT accepted/processed.
expect(res.status).toBeGreaterThanOrEqual(400);
expect(res.body.error).not.toBe(undefined);
});
it('maps the oversized array-index rejection to a 400 through the real error handler', async () => {
const token = await createPublicToken(db, 'contract_action_tokens', {
contract_id: contractId,
});
const res = await request(appWithErrorHandler)
.post(`/api/public/contracts/${token}/upload-signed-pdf`)
.field('evil[999999999]', 'x')
.attach('file', Buffer.from('%PDF-1.4 fake'), 'signed.pdf');
expect(res.status).toBe(400);
expect(res.body.code).toBe('VALIDATION_ERROR');
});
});
describe('GET /:token/pdf', () => {
@@ -22,38 +22,29 @@ process.env.JWT_SECRET = process.env.JWT_SECRET || 'restorepath-test-secret';
const { bootCrmDb, seedMinimal } = require('../integration/helpers/crmDb');
// `bootCrmDb()` hands back the process-wide `db` singleton (module cache —
// see its own comment), so it must only be called ONCE per test file: a
// second call re-runs migrations against the same connection, and the first
// call's `cleanup()` (db.destroy()) would tear down the connection both
// describe blocks below share. Boot once at file scope; each describe below
// only touches app_settings / env vars, never the connection lifecycle.
let db; let cleanup; let checkRestorePathsAllowed;
beforeAll(async () => {
({ db, cleanup } = await bootCrmDb());
await seedMinimal(db);
({ checkRestorePathsAllowed } = require('../../src/routes/adminRestore')._internal);
}, 120000);
afterAll(async () => { if (cleanup) await cleanup(); });
async function setBackupSetting(key, value) {
const existing = await db('app_settings').where({ setting_key: key }).first();
if (existing) {
await db('app_settings').where({ setting_key: key }).update({ setting_value: JSON.stringify(value) });
} else {
await db('app_settings').insert({
setting_key: key, setting_value: JSON.stringify(value), setting_type: 'backup',
});
}
}
describe('restore path allowlist (GHSA-fw4c)', () => {
let db; let cleanup; let checkRestorePathsAllowed;
beforeAll(async () => {
({ db, cleanup } = await bootCrmDb());
await seedMinimal(db);
// Configure a backup root so the allowlist is actually active.
await setBackupSetting('backup_destination_path', '/backup');
});
for (const [key, value] of [['backup_destination_path', '/backup']]) {
const existing = await db('app_settings').where({ setting_key: key }).first();
if (existing) {
await db('app_settings').where({ setting_key: key }).update({ setting_value: JSON.stringify(value) });
} else {
await db('app_settings').insert({
setting_key: key, setting_value: JSON.stringify(value), setting_type: 'backup',
});
}
}
({ checkRestorePathsAllowed } = require('../../src/routes/adminRestore')._internal);
}, 120000);
afterAll(async () => { if (cleanup) await cleanup(); });
it('allows the wizard\'s source TYPE tokens', async () => {
for (const source of ['local', 's3', 'upload']) {
@@ -93,89 +84,3 @@ describe('restore path allowlist (GHSA-fw4c)', () => {
expect(err).toBeNull();
});
});
/**
* GHSA-xfvx-j447-732c: `checkRestorePathsAllowed` constrained the top-level
* `source`/`manifestPath` request fields (GHSA-fw4c above), but never looked
* INSIDE the manifest itself. `manifest.database.backup_file` — handed
* straight to restoreService's candidate resolution and eventually
* interpolated into `sqlite3 .restore '<path>'` — was unchecked, so an
* absolute path there could point the restore at an arbitrary file even
* though `source`/`manifestPath` both passed containment.
*/
describe('restore path allowlist — manifest database.backup_file containment (GHSA-xfvx)', () => {
let tmpRoot;
beforeAll(async () => {
await setBackupSetting('backup_destination_path', '/backup');
tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-xfvx-manifest-'));
// Additional allowed root via the documented escape hatch — keeps this
// describe block's fixtures out of the shared '/backup' root above.
process.env.RESTORE_ALLOWED_ROOTS = tmpRoot;
});
afterAll(() => {
delete process.env.RESTORE_ALLOWED_ROOTS;
fs.rmSync(tmpRoot, { recursive: true, force: true });
});
const writeManifest = (name, databaseSection) => {
const manifestPath = path.join(tmpRoot, name);
fs.writeFileSync(manifestPath, JSON.stringify({
manifest: { version: '1.0', id: 'test' },
backup: { type: 'full' },
system: { platform: 'linux' },
application: { version: '1.0.0' },
files: { count: 0, manifest: [] },
database: databaseSection,
verification: { total_checksum: null, checksum_algorithm: null },
}));
return manifestPath;
};
it('rejects a manifest whose database.backup_file is an absolute path outside every configured root', async () => {
const manifestPath = writeManifest('evil-1.json', { backup_file: '/etc/passwd' });
const err = await checkRestorePathsAllowed({ source: 'local', manifestPath });
expect(err).toMatch(/database\.backup_file must be inside a configured backup location/i);
});
it('accepts a manifest whose database.backup_file is an absolute path inside a configured root', async () => {
const dbFile = path.join(tmpRoot, 'database', 'picpeak-db-sqlite-1.sql.gz');
fs.mkdirSync(path.dirname(dbFile), { recursive: true });
fs.writeFileSync(dbFile, 'not a real sqlite dump, just a fixture');
const manifestPath = writeManifest('legit-1.json', { backup_file: dbFile });
const err = await checkRestorePathsAllowed({ source: 'local', manifestPath });
expect(err).toBeNull();
});
it('does not choke on a manifest whose database.backup_file is a legitimate relative path', async () => {
// Relative candidates are resolved against restoreService's own
// `backupPath` (which this route-level pre-check doesn't have — it only
// sees `source`/`manifestPath`), so this layer intentionally defers
// relative-path containment to restoreService.performDatabaseRestore
// and must not false-positive here.
const manifestPath = writeManifest('legit-2.json', { backup_file: 'database/picpeak-db-sqlite-1.sql.gz' });
const err = await checkRestorePathsAllowed({ source: 'local', manifestPath });
expect(err).toBeNull();
});
it('rejects everything when no backup location is configured at all (fail closed, not fail open)', async () => {
// Simulate an install that never had backup_destination_path /
// backup_manifest_path seeded/configured, and isn't using the
// RESTORE_ALLOWED_ROOTS escape hatch either.
const savedRoots = process.env.RESTORE_ALLOWED_ROOTS;
delete process.env.RESTORE_ALLOWED_ROOTS;
await db('app_settings').whereIn('setting_key', ['backup_destination_path', 'backup_manifest_path']).del();
try {
const err = await checkRestorePathsAllowed({
source: '/backup/run-1', manifestPath: '/backup/run-1/manifest.json',
});
expect(err).toMatch(/no backup location is configured/i);
} finally {
process.env.RESTORE_ALLOWED_ROOTS = savedRoots;
await setBackupSetting('backup_destination_path', '/backup');
}
});
});
@@ -1,127 +0,0 @@
/**
* Background zip rebuilds are capped (#1399).
*
* invalidateAll() invalidates every event holding a cached zip, and each
* invalidate() arms its own debounce timer in the same tick — so they all fire
* together. Every build opens its own storage reads, so a settings change
* across 25 events was enough to exhaust the S3 agent pool and stall uploads,
* thumbnails and gallery reads until the burst drained.
*
* The cap is on the BACKGROUND path only: a guest waiting on a download must
* not be queued behind a settings-change burst.
*/
jest.mock('../../src/database/db', () => ({ db: jest.fn() }));
jest.mock('../../src/utils/logger', () => ({
info: jest.fn(), warn: jest.fn(), error: jest.fn(), debug: jest.fn(),
}));
const { db } = require('../../src/database/db');
const service = require('../../src/services/downloadZipService');
const flush = () => new Promise((r) => setImmediate(r));
describe('downloadZipService background regen concurrency (#1399)', () => {
let peak;
let inFlight;
let release;
beforeEach(() => {
// setImmediate must stay real: the flush() helper below rides on it, and
// jest's modern fake timers mock it too.
jest.useFakeTimers({ doNotFake: ['setImmediate'] });
peak = 0;
inFlight = 0;
release = [];
service.regenActive = 0;
service.regenWaiters = [];
service.debounceTimers.clear();
service.activeBuilds.clear();
jest.spyOn(service, 'generateZip').mockImplementation(() => {
inFlight += 1;
peak = Math.max(peak, inFlight);
return new Promise((resolve) => {
release.push(() => { inFlight -= 1; resolve(); });
});
});
jest.spyOn(service, '_cleanup').mockResolvedValue(undefined);
});
afterEach(() => {
jest.useRealTimers();
jest.restoreAllMocks();
});
it('never runs more than two rebuilds at once, however many fire together', async () => {
const rows = Array.from({ length: 12 }, (_, i) => ({ id: i + 1 }));
db.mockReturnValue({
whereNotNull: () => ({ select: () => Promise.resolve(rows) }),
});
await service.invalidateAll();
// Every debounce timer was armed in the same tick — fire them all.
jest.runAllTimers();
await flush();
expect(peak).toBe(2);
expect(service.generateZip).toHaveBeenCalledTimes(2);
});
it('starts the next rebuild as each one finishes', async () => {
const rows = Array.from({ length: 5 }, (_, i) => ({ id: i + 1 }));
db.mockReturnValue({
whereNotNull: () => ({ select: () => Promise.resolve(rows) }),
});
await service.invalidateAll();
jest.runAllTimers();
await flush();
expect(service.generateZip).toHaveBeenCalledTimes(2);
release.shift()();
await flush();
expect(service.generateZip).toHaveBeenCalledTimes(3);
expect(peak).toBe(2);
while (release.length) { release.shift()(); await flush(); }
expect(service.generateZip).toHaveBeenCalledTimes(5);
expect(peak).toBe(2);
});
it('does not queue a foreground download behind the burst', async () => {
const rows = Array.from({ length: 6 }, (_, i) => ({ id: i + 1 }));
db.mockReturnValue({
whereNotNull: () => ({ select: () => Promise.resolve(rows) }),
});
await service.invalidateAll();
jest.runAllTimers();
await flush();
expect(service.generateZip).toHaveBeenCalledTimes(2);
// A guest asking for a zip right now calls generateZip directly. It must
// not park behind the two rebuilds already holding the slots.
service.generateZip(999);
await flush();
expect(service.generateZip).toHaveBeenCalledWith(999);
expect(inFlight).toBe(3);
});
it('leaves the queue empty once every rebuild has run', async () => {
const rows = Array.from({ length: 5 }, (_, i) => ({ id: i + 1 }));
db.mockReturnValue({
whereNotNull: () => ({ select: () => Promise.resolve(rows) }),
});
await service.invalidateAll();
jest.runAllTimers();
await flush();
expect(service.regenWaiters.length).toBeGreaterThan(0);
while (release.length) { release.shift()(); await flush(); }
// Nothing parked, nothing counted as running — no slot leaked on the way
// through, which is what would quietly wedge the next burst.
expect(service.regenWaiters).toHaveLength(0);
expect(service.regenActive).toBe(0);
});
});
@@ -1,193 +0,0 @@
/**
* A failed pre-zip build must not leave storage reads open.
*
* The builder opened one storage read per photo and handed the raw stream to
* archiver. archiver drains its queue one entry at a time, so on an S3 backend
* every photo beyond the one being written parked a socket with a full receive
* buffer, and the error path (a source stream dying, or a photo upload
* invalidating the build) walked away from all of them. archiver's abort()
* does not touch the source streams, and the AWS SDK arms its socket timeout
* on a 3s delay then clears it once the response headers arrive, so nothing
* ever reclaimed those sockets. On a live server 43 of the 50 pooled sockets
* ended up stuck for days and photo uploads stopped completing.
*/
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-zipleak-')), 'db.sqlite',
);
process.env.JWT_SECRET = process.env.JWT_SECRET || 'zipleak-test-secret';
process.env.STORAGE_PATH = fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-zipleak-storage-'));
const { Readable } = require('stream');
const PHOTO_COUNT = 6;
const MAX_INFLIGHT_READS = 2;
// One storage read. It never ends on its own, which is what a large photo
// looks like to the builder: the bytes only move while archiver pulls them.
class StoredObject extends Readable {
constructor(key, failAfterReads, chunks) {
super();
this.key = key;
this.failAfterReads = failAfterReads;
this.chunks = chunks;
this.reads = 0;
}
_read() {
this.reads += 1;
if (this.failAfterReads && this.reads > this.failAfterReads) {
// What a dropped connection to S3 looks like in Node.
this.destroy(new Error('aborted'));
return;
}
this.push(this.reads > this.chunks ? null : Buffer.alloc(4096, 1));
}
}
const reads = { opened: [], live: 0, peak: 0 };
const failingKey = { value: null };
const onOpen = { fn: null };
// A read only finishes when the build pulls the whole object. Photos big
// enough to matter never finish inside one archiver turn, and a stream that
// ends on its own would be auto-destroyed and hide the leak.
const objectChunks = { value: Number.POSITIVE_INFINITY };
function openStoredObject(key) {
const stream = new StoredObject(key, key === failingKey.value ? 1 : 0, objectChunks.value);
reads.opened.push(stream);
reads.live += 1;
if (reads.live > reads.peak) reads.peak = reads.live;
let settled = false;
const settle = () => { if (!settled) { settled = true; reads.live -= 1; } };
stream.once('end', settle);
stream.once('close', settle);
if (onOpen.fn) onOpen.fn(reads.opened.length);
return stream;
}
const mockStorage = {
kind: () => 's3',
get: jest.fn(async (key) => openStoredObject(key)),
getToFile: jest.fn(async () => undefined),
putFromFile: jest.fn(async () => undefined),
stat: jest.fn(async () => ({ size: 1234, mtime: new Date() })),
delete: jest.fn(async () => undefined),
exists: jest.fn(async () => true),
};
jest.mock('../../src/services/storage', () => ({
getStorage: () => mockStorage,
initStorage: async () => mockStorage,
}));
// Nothing to watermark, so the builder takes the stream-from-storage branch,
// which is the one that holds sockets. (This branch has no rendition step —
// the resize/watermark split that main mocks out here does not exist yet.)
jest.mock('../../src/services/watermarkService', () => ({
getWatermarkSettings: jest.fn(async () => ({ enabled: false })),
applyWatermark: jest.fn(),
}));
const { bootCrmDb, seedMinimal } = require('../integration/helpers/crmDb');
const downloadZipService = require('../../src/services/downloadZipService');
describe('pre-zip build releases its storage reads', () => {
let db; let cleanup; let eventId;
beforeAll(async () => {
({ db, cleanup } = await bootCrmDb());
await seedMinimal(db);
const ev = await db('events').insert({
slug: 'zipleak',
event_type: 'wedding',
event_name: 'Zip Leak',
event_date: '2026-09-01',
host_email: 'h@example.com',
admin_email: 'a@example.com',
password_hash: 'x',
share_link: '/gallery/zipleak/s',
share_token: 'zipleak-share',
expires_at: new Date(Date.now() + 7 * 864e5).toISOString(),
is_active: 1,
is_archived: 0,
is_draft: 0,
require_password: 0,
allow_downloads: 1,
created_at: new Date().toISOString(),
}).returning('id');
eventId = ev[0]?.id ?? ev[0];
for (let i = 0; i < PHOTO_COUNT; i += 1) {
await db('photos').insert({
event_id: eventId,
filename: `photo-${i}.jpg`,
path: `zipleak/photo-${i}.jpg`,
type: 'individual',
source_origin: 'managed',
mime_type: 'image/jpeg',
visibility: 'visible',
uploaded_at: new Date(Date.now() - i * 1000).toISOString(),
});
}
}, 120000);
afterAll(async () => { if (cleanup) await cleanup(); });
beforeEach(() => {
reads.opened = [];
reads.live = 0;
reads.peak = 0;
failingKey.value = null;
onOpen.fn = null;
objectChunks.value = Number.POSITIVE_INFINITY;
mockStorage.get.mockClear();
downloadZipService.versions.clear();
downloadZipService.activeBuilds.clear();
});
it('destroys every open read when a source stream dies mid-build', async () => {
// The oldest photo is written first, so failing it strands the rest.
failingKey.value = 'events/active/zipleak/photo-0.jpg';
const result = await downloadZipService.generateZip(eventId);
expect(result.success).toBe(false);
expect(reads.opened.length).toBeGreaterThan(1);
const stranded = reads.opened.filter((s) => !s.destroyed);
expect(stranded.map((s) => s.key)).toEqual([]);
});
it('destroys every open read when an upload invalidates the build', async () => {
// What adminPhotos does on every upload, delete and bulk edit, landing
// while the archive is half built.
onOpen.fn = (count) => {
if (count !== 2) return;
downloadZipService.invalidate(eventId);
// invalidate() also schedules a rebuild; this test is not about that.
clearTimeout(downloadZipService.debounceTimers.get(eventId));
downloadZipService.debounceTimers.delete(eventId);
};
const result = await downloadZipService.generateZip(eventId);
expect(result).toEqual({ success: false, error: 'Build invalidated' });
expect(reads.opened.filter((s) => !s.destroyed).map((s) => s.key)).toEqual([]);
});
it('never holds more storage reads open than the build needs', async () => {
objectChunks.value = 8;
const result = await downloadZipService.generateZip(eventId);
expect(result.success).toBe(true);
expect(mockStorage.get).toHaveBeenCalledTimes(PHOTO_COUNT);
expect(reads.peak).toBeLessThanOrEqual(MAX_INFLIGHT_READS);
});
});
@@ -102,7 +102,6 @@ jest.mock('../../src/utils/logger', () => ({
}));
const invoiceService = require('../../src/services/invoiceService');
const emailProcessor = require('../../src/services/emailProcessor');
function resetChains() {
for (const k of Object.keys(tableChains)) delete tableChains[k];
@@ -262,10 +261,7 @@ describe('invoiceService.releaseForDelivery', () => {
});
describe('invoiceService.recordPaymentCheckAction', () => {
beforeEach(() => {
resetChains();
emailProcessor.queueEmail.mockClear();
});
beforeEach(() => resetChains());
it('rejects invalid actions', async () => {
await expect(invoiceService.recordPaymentCheckAction({
@@ -325,71 +321,6 @@ describe('invoiceService.recordPaymentCheckAction', () => {
token: 'a'.repeat(64), action: 'partial', amountMinor: 9999,
})).rejects.toMatchObject({ statusCode: 400 });
});
// GHSA-wg94-f86h-vq68 hardening: every write via this unauthenticated
// route notifies the admin. Uses 'paid_full' as the exercised action —
// it stays inside markPaid (no workflow-engine / PDF-rendering
// dependencies to stub) while still going through the full
// recordPaymentCheckAction write path.
it('queues an admin notification email after a successful action', async () => {
pickChainFor('invoice_payment_check_tokens')._firstValue = {
id: 1, used_at: null,
expires_at: new Date(Date.now() + 86400000),
};
pickChainFor('invoices')._firstValue = {
id: 5, invoice_number: 'INV-0005', status: 'overdue',
total_amount_minor: 10000, paid_amount_minor: 0, late_fee_amount_minor: 0,
customer_account_id: 7, created_by_admin_id: 42,
currency: 'CHF', language: 'de', event_id: null,
};
pickChainFor('admin_users')._firstValue = { id: 42, email: 'admin@example.com', username: 'admin' };
pickChainFor('business_profile')._firstValue = null;
pickChainFor('customer_accounts')._firstValue = { id: 7, email: 'c@example.com', display_name: 'Test Customer' };
const result = await invoiceService.recordPaymentCheckAction({
token: 'a'.repeat(64), action: 'paid_full', ip: '203.0.113.7',
});
expect(result).toEqual({ applied: 'paid_full' });
expect(emailProcessor.queueEmail).toHaveBeenCalledTimes(1);
const [, recipientEmail, templateKey, data] = emailProcessor.queueEmail.mock.calls[0];
expect(recipientEmail).toBe('admin@example.com');
expect(templateKey).toBe('invoice_payment_check_action_recorded');
expect(data.invoice_number).toBe('INV-0005');
expect(data.action).toBe('paid_full');
expect(data.ip).toBe('203.0.113.7');
});
it('does not fail (or roll back) the ledger write when the admin notification fails to send', async () => {
pickChainFor('invoice_payment_check_tokens')._firstValue = {
id: 1, used_at: null,
expires_at: new Date(Date.now() + 86400000),
};
pickChainFor('invoices')._firstValue = {
id: 5, invoice_number: 'INV-0005', status: 'overdue',
total_amount_minor: 10000, paid_amount_minor: 0, late_fee_amount_minor: 0,
customer_account_id: 7, created_by_admin_id: 42,
currency: 'CHF', language: 'de', event_id: null,
};
pickChainFor('admin_users')._firstValue = { id: 42, email: 'admin@example.com', username: 'admin' };
pickChainFor('business_profile')._firstValue = null;
pickChainFor('customer_accounts')._firstValue = { id: 7, email: 'c@example.com', display_name: 'Test Customer' };
emailProcessor.queueEmail.mockRejectedValueOnce(new Error('smtp down'));
// The write itself (token consumption + markPaid) must still
// succeed — the notification is best-effort only.
const result = await invoiceService.recordPaymentCheckAction({
token: 'a'.repeat(64), action: 'paid_full', ip: '203.0.113.7',
});
expect(result).toEqual({ applied: 'paid_full' });
// Token was actually consumed (the real assertion that the write
// committed): the mock chain's .update() ran with used_at set.
const tokenChain = pickChainFor('invoice_payment_check_tokens');
expect(tokenChain.update).toHaveBeenCalledWith(
expect.objectContaining({ used_at: expect.any(Date), used_action: 'paid_full' }),
);
});
});
describe('invoiceService.queuePaymentCheckEmail', () => {
@@ -440,35 +371,4 @@ describe('invoiceService.queuePaymentCheckEmail', () => {
expect(res.sent).toBe(true);
expect(res.token).toMatch(/^[a-f0-9]{64}$/);
});
// GHSA-wg94-f86h-vq68 hardening: token TTL shortened from 30 days to 72h.
it('mints a token with a ~72h TTL, not the old 30-day window', async () => {
pickChainFor('invoices')._firstValue = {
id: 1, status: 'overdue',
customer_account_id: 5,
created_by_admin_id: 42,
total_amount_minor: 10000,
currency: 'CHF',
language: 'de',
reminder_level: 0,
due_date: '2026-05-01',
last_payment_check_at: null,
event_id: null,
};
pickChainFor('admin_users')._firstValue = { id: 42, email: 'admin@example.com', username: 'admin' };
pickChainFor('business_profile')._firstValue = null;
pickChainFor('customer_accounts')._firstValue = { id: 5, email: 'c@example.com', display_name: 'Test' };
const before = Date.now();
const res = await invoiceService.queuePaymentCheckEmail(1);
expect(res.sent).toBe(true);
const tokenChain = pickChainFor('invoice_payment_check_tokens');
const insertedRow = tokenChain.insert.mock.calls[0][0];
const ttlMs = new Date(insertedRow.expires_at).getTime() - before;
expect(ttlMs).toBeGreaterThan(71 * 60 * 60 * 1000);
expect(ttlMs).toBeLessThanOrEqual(72 * 60 * 60 * 1000 + 5000);
// Well under the old 30-day TTL — the actual regression guard.
expect(ttlMs).toBeLessThan(24 * 60 * 60 * 1000 * 30);
});
});
@@ -92,52 +92,6 @@ describe('mfaService — TOTP verification', () => {
});
});
describe('mfaService — replay protection (GHSA-qcwx-r25m-j869)', () => {
it('verifyTotp accepts a code once and rejects the same code as a replay', () => {
const secret = mfaService.generateSecret();
const code = authenticator.generate(secret);
// First use: no lastUsedStep yet, so it's accepted.
expect(mfaService.verifyTotp(code, secret)).toBe(true);
// Simulate persisting the matched step and replaying the same code: the
// matched step must strictly advance past lastUsedStep, so this fails.
const step = mfaService.currentTotpStep();
expect(mfaService.verifyTotp(code, secret, step)).toBe(false);
// A lastUsedStep the code hasn't caught up to yet also rejects it.
expect(mfaService.verifyTotp(code, secret, step + 1)).toBe(false);
});
it('verifyTotpEncryptedStep returns the matched step on success and null on replay', () => {
const secret = mfaService.generateSecret();
const stored = mfaService.encryptSecret(secret);
const code = authenticator.generate(secret);
const step = mfaService.verifyTotpEncryptedStep(code, stored, null);
expect(step).toEqual(expect.any(Number));
expect(step).toBeGreaterThan(0);
// Replaying the same code against the just-persisted step is rejected.
expect(mfaService.verifyTotpEncryptedStep(code, stored, step)).toBeNull();
});
it('a freshly generated code for the next TOTP step is accepted after a replay is rejected', () => {
const secret = mfaService.generateSecret();
const code = authenticator.generate(secret);
const step = mfaService.verifyTotpEncryptedStep(code, mfaService.encryptSecret(secret), null)
|| mfaService.currentTotpStep();
// Same-step replay: rejected.
expect(mfaService.verifyTotp(code, secret, step)).toBe(false);
// A code minted for the next step (via a cloned authenticator with a
// future epoch, not by mocking Date.now()) advances past last_used_step.
const nextStepAuthenticator = authenticator.clone({ epoch: Date.now() + 30000 });
const nextCode = nextStepAuthenticator.generate(secret);
expect(mfaService.verifyTotp(nextCode, secret, step)).toBe(true);
});
});
describe('mfaService — otpauth URI / QR', () => {
it('builds an otpauth:// URI containing issuer, account and secret', () => {
const secret = mfaService.generateSecret();
@@ -1,173 +0,0 @@
/**
* GHSA-xfvx-j447-732c: the SQLite restore path let an attacker-influenced
* `manifest.database.backup_file` replace the live database.
*
* Two independent bugs, both fixed here:
*
* 1. Candidate resolution (restoreService.js's performDatabaseRestore,
* ~L1000) tried an absolute `dbBackupFile` and a
* `path.join(backupPath, dbBackupFile)` candidate with NO check that
* the resolved path actually stayed inside the configured backup
* root — a manifest could point `.restore` at any file on disk.
*
* 2. The resolved path was interpolated unescaped into a
* `sqlite3 .restore '<path>'` dot-command string. sqlite3's CLI
* parses that string itself (not the shell), so a single quote in
* the path breaks out of the quoted argument regardless of
* spawn()'s `shell: false` argv separation.
*
* These tests pin the fix directly against the exported helpers
* (`resolveContainedDbBackupCandidates`, `assertSafeSqlitePath`,
* `isContainedInRoots`, `getConfiguredBackupRoots`) — the exact functions
* `performDatabaseRestore` calls before ever running `sqlite3 .restore` —
* rather than driving the full restore (which does a real `db.destroy()` +
* live-file swap against the shared app db and isn't worth the added
* fragility for what's fundamentally a path-validation contract).
*/
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-restoresvc-')), 'db.sqlite',
);
process.env.JWT_SECRET = process.env.JWT_SECRET || 'restoresvc-test-secret';
const { bootCrmDb, seedMinimal } = require('../integration/helpers/crmDb');
describe('restoreService — sqlite restore path safety (GHSA-xfvx)', () => {
let db; let cleanup; let _internal;
let backupPath;
beforeAll(async () => {
({ db, cleanup } = await bootCrmDb());
await seedMinimal(db);
// The restore run's resolved local backup root — analogous to
// `localBackupPath` in restoreService.restore(). Real directory with a
// real database/ subfolder, matching what a genuine backup run leaves
// on disk.
backupPath = fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-xfvx-backuproot-'));
fs.mkdirSync(path.join(backupPath, 'database'), { recursive: true });
({ _internal } = require('../../src/services/restoreService'));
}, 120000);
afterAll(async () => { if (cleanup) await cleanup(); });
describe('assertSafeSqlitePath — the sqlite3 dot-command injection gate', () => {
it.each([
['/backup/database/picpeak-db-sqlite-1.sql'],
[`${backupPath || '/backup'}/database/picpeak-db-sqlite-2024-01-01.sql.gz`],
])('accepts a normal backup path: %s', (p) => {
expect(() => _internal.assertSafeSqlitePath(p)).not.toThrow();
});
it.each([
['/backup/database/x\'; DROP TABLE admin_users; --.sql'],
['/backup/database/x\' .restore \'/etc/passwd'],
['/backup/database/x\n.shell rm -rf /'],
['/backup/database/has space.sql'],
['/backup/database/semi;colon.sql'],
[null],
[undefined],
[42],
])('rejects an unsafe/non-string path: %j', (p) => {
expect(() => _internal.assertSafeSqlitePath(p)).toThrow(/unsafe path/i);
});
});
describe('isContainedInRoots', () => {
it('accepts a path inside a root', () => {
expect(_internal.isContainedInRoots('/backup/database/x.sql', ['/backup'])).toBe(true);
});
it('accepts a root path equal to the root itself', () => {
expect(_internal.isContainedInRoots('/backup', ['/backup'])).toBe(true);
});
it('rejects a path outside every root', () => {
expect(_internal.isContainedInRoots('/etc/passwd', ['/backup'])).toBe(false);
});
it('rejects a sibling directory that merely shares a prefix', () => {
// '/backup-evil' starts with the string '/backup' but is NOT inside it.
expect(_internal.isContainedInRoots('/backup-evil/x.sql', ['/backup'])).toBe(false);
});
it('rejects a `..`-traversal path that resolves outside the root', () => {
expect(_internal.isContainedInRoots('/backup/../etc/passwd', ['/backup'])).toBe(false);
});
});
describe('getConfiguredBackupRoots', () => {
afterEach(async () => {
delete process.env.RESTORE_ALLOWED_ROOTS;
await db('app_settings').whereIn('setting_key', ['backup_destination_path', 'backup_manifest_path']).del();
});
it('always includes the trusted root even with nothing else configured', async () => {
const roots = await _internal.getConfiguredBackupRoots('/some/trusted/backup-path');
expect(roots).toContain(path.resolve('/some/trusted/backup-path'));
});
it('adds configured backup_destination_path / backup_manifest_path and RESTORE_ALLOWED_ROOTS', async () => {
await db('app_settings').insert([
{ setting_key: 'backup_destination_path', setting_value: JSON.stringify('/backup/dest'), setting_type: 'backup' },
{ setting_key: 'backup_manifest_path', setting_value: JSON.stringify('/backup/manifests'), setting_type: 'backup' },
]);
process.env.RESTORE_ALLOWED_ROOTS = '/extra/root';
const roots = await _internal.getConfiguredBackupRoots('/trusted');
expect(roots).toEqual(expect.arrayContaining([
path.resolve('/trusted'),
path.resolve('/backup/dest'),
path.resolve('/backup/manifests'),
path.resolve('/extra/root'),
]));
});
});
describe('resolveContainedDbBackupCandidates — the manifest.database.backup_file gate', () => {
it('rejects an absolute backup_file outside every configured root, but still offers the safe legacy basename candidate', async () => {
const candidates = await _internal.resolveContainedDbBackupCandidates(
backupPath, '/etc/passwd', () => {}
);
// The raw absolute escape must NOT be present.
expect(candidates).not.toContain('/etc/passwd');
// Candidate (3), the basename-only legacy reconstruct, is inherently
// safe (can't escape backupPath) and stays available as a fallback.
expect(candidates).toContain(path.join(backupPath, 'database', 'passwd'));
});
it('rejects a `..`-traversal relative backup_file, keeping only the contained legacy candidate', async () => {
const candidates = await _internal.resolveContainedDbBackupCandidates(
backupPath, '../../../../etc/passwd', () => {}
);
const escaped = candidates.some((c) => !_internal.isContainedInRoots(c, [path.resolve(backupPath)]));
expect(escaped).toBe(false);
expect(candidates).toContain(path.join(backupPath, 'database', 'passwd'));
});
it('accepts a legitimate relative backup_file recorded by a real backup run', async () => {
const candidates = await _internal.resolveContainedDbBackupCandidates(
backupPath, 'database/picpeak-db-sqlite-2024-01-01.sql.gz', () => {}
);
expect(candidates).toContain(path.join(backupPath, 'database', 'picpeak-db-sqlite-2024-01-01.sql.gz'));
// Every returned candidate must actually be safe to use.
for (const c of candidates) {
expect(_internal.isContainedInRoots(c, [path.resolve(backupPath)])).toBe(true);
}
});
it('accepts a legitimate absolute backup_file that IS inside backupPath (the real dumper shape)', async () => {
const absFile = path.join(backupPath, 'database', 'picpeak-db-sqlite-2024-02-02.sql.gz');
const candidates = await _internal.resolveContainedDbBackupCandidates(
backupPath, absFile, () => {}
);
expect(candidates).toContain(absFile);
});
});
});
@@ -1,147 +0,0 @@
/**
* DNS-rebinding follow-up to the blind-SSRF fix in restoreServiceS3Ssrf.test.js
* (GHSA-vm2x-c628-3cx5).
*
* isHostAllowed()/validateExternalUrlWithAddresses() are check-then-connect
* on their own: they resolve the S3 endpoint hostname once to vet it, then
* hand a bare hostname to the AWS SDK, which resolves it AGAIN when it
* actually connects. An attacker who controls DNS for the endpoint hostname
* (or an infra DNS-rebinding condition) can answer the first lookup with a
* public IP and the second with a private/metadata one.
*
* downloadFileFromS3() now builds pinned http/https agents (pinnedRequest.js
* — the same primitive webhookDeliveryWorker.js on main uses for outbound
* HTTP; ported here since stable didn't have it yet) from the validated
* address and passes them into S3StorageAdapter, which threads them into
* the S3Client's NodeHttpHandler requestHandler. This asserts that wiring:
* the agents S3StorageAdapter receives resolve the endpoint hostname to
* ONLY the address vetted during validation, and never fall through to a
* second, real DNS lookup that a rebinding attacker could answer
* differently.
*/
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-restores3pin-')), 'db.sqlite',
);
process.env.JWT_SECRET = process.env.JWT_SECRET || 'restores3pin-test-secret';
jest.mock('dns', () => {
const actual = jest.requireActual('dns');
return { ...actual, promises: { ...actual.promises, lookup: jest.fn() }, lookup: jest.fn() };
});
let capturedConfig;
jest.mock('../../src/services/storage/s3Storage', () =>
jest.fn().mockImplementation((config) => {
capturedConfig = config;
return { download: jest.fn().mockResolvedValue(undefined) };
})
);
const dns = require('dns');
const promiseLookup = dns.promises.lookup;
const S3StorageAdapter = require('../../src/services/storage/s3Storage');
const { RestoreService } = require('../../src/services/restoreService');
describe('downloadFileFromS3 DNS-rebinding pinning', () => {
let restoreService;
let originalNodeEnv;
beforeEach(() => {
restoreService = new RestoreService();
capturedConfig = undefined;
promiseLookup.mockReset();
dns.lookup.mockReset();
S3StorageAdapter.mockClear();
originalNodeEnv = process.env.NODE_ENV;
process.env.NODE_ENV = 'production';
});
afterEach(() => {
process.env.NODE_ENV = originalNodeEnv;
capturedConfig?.httpAgent?.destroy();
capturedConfig?.httpsAgent?.destroy();
});
it('passes pinned http/https agents into S3StorageAdapter built from the validated address', async () => {
promiseLookup.mockResolvedValue([{ address: '93.184.216.34', family: 4 }]);
await restoreService.downloadFileFromS3(
's3://backups/manifest.json',
'/tmp/whatever/manifest.json',
{ endpoint: 'rebind.example.com', accessKeyId: 'k', secretAccessKey: 's' }
);
expect(S3StorageAdapter).toHaveBeenCalledTimes(1);
expect(capturedConfig.httpAgent).toBeInstanceOf(require('http').Agent);
expect(capturedConfig.httpsAgent).toBeInstanceOf(require('https').Agent);
});
it('the pinned agent never performs a second DNS lookup — rebinding to a private IP on the real resolver is ignored', async () => {
// First (validation) lookup: public IP, passes the preflight.
promiseLookup.mockResolvedValue([{ address: '93.184.216.34', family: 4 }]);
// If the pinned agent ever fell through to a real lookup, this would
// hand back a private/metadata address — simulating the rebind.
dns.lookup.mockImplementation((_hostname, options, callback) => {
if (typeof options === 'function') { callback = options; options = {}; }
callback(null, ...(options?.all ? [[{ address: '169.254.169.254', family: 4 }]] : ['169.254.169.254', 4]));
});
await restoreService.downloadFileFromS3(
's3://backups/manifest.json',
'/tmp/whatever/manifest.json',
{ endpoint: 'rebind.example.com', accessKeyId: 'k', secretAccessKey: 's' }
);
const pinnedLookup = capturedConfig.httpAgent.options.lookup;
expect(typeof pinnedLookup).toBe('function');
const result = await new Promise((resolve, reject) => {
pinnedLookup('rebind.example.com', {}, (err, address, family) => {
if (err) return reject(err);
resolve({ address, family });
});
});
// Only the address vetted during validation is ever handed back —
// never the private address the real resolver would now answer with.
expect(result).toEqual({ address: '93.184.216.34', family: 4 });
expect(dns.lookup).not.toHaveBeenCalled();
});
it('rejects a lookup for any hostname other than the one that was validated', async () => {
promiseLookup.mockResolvedValue([{ address: '93.184.216.34', family: 4 }]);
await restoreService.downloadFileFromS3(
's3://backups/manifest.json',
'/tmp/whatever/manifest.json',
{ endpoint: 'rebind.example.com', accessKeyId: 'k', secretAccessKey: 's' }
);
const pinnedLookup = capturedConfig.httpAgent.options.lookup;
await expect(new Promise((resolve, reject) => {
pinnedLookup('attacker-controlled.example', {}, (err, address) => {
if (err) return reject(err);
resolve(address);
});
})).rejects.toThrow(/hostname changed/i);
});
it('does not pin agents when no custom endpoint is configured (default AWS, no rebinding surface)', async () => {
await restoreService.downloadFileFromS3(
's3://backups/manifest.json',
'/tmp/whatever/manifest.json',
{ accessKeyId: 'k', secretAccessKey: 's' }
);
expect(promiseLookup).not.toHaveBeenCalled();
expect(capturedConfig.httpAgent).toBeUndefined();
expect(capturedConfig.httpsAgent).toBeUndefined();
});
});
@@ -1,109 +0,0 @@
/**
* Blind SSRF via the restore S3 download path (GHSA-vm2x-c628-3cx5).
*
* downloadFileFromS3() built a bare S3StorageAdapter and called .download()
* directly, never running the DNS-resolving isHostAllowed() guard that
* testConnection() applies elsewhere — so an admin with backup.restore could
* point the request-supplied S3 endpoint at an internal/metadata address for
* unauthenticated egress via the server. `s3Config` here is fully attacker
* controlled (POST /api/admin/restore/validate and /restore/start take it
* straight from the request body — see routes/adminRestore.js), unlike the
* scheduled-backup S3 endpoint, which is vetted at settings-save time.
*/
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-restores3ssrf-')), 'db.sqlite',
);
process.env.JWT_SECRET = process.env.JWT_SECRET || 'restores3ssrf-test-secret';
jest.mock('dns', () => {
const actual = jest.requireActual('dns');
return { ...actual, promises: { ...actual.promises, lookup: jest.fn() } };
});
jest.mock('../../src/services/storage/s3Storage', () =>
jest.fn().mockImplementation(() => ({
download: jest.fn().mockResolvedValue(undefined),
}))
);
const dns = require('dns');
const lookup = dns.promises.lookup;
const S3StorageAdapter = require('../../src/services/storage/s3Storage');
const { RestoreService } = require('../../src/services/restoreService');
describe('downloadFileFromS3 SSRF guard (GHSA-vm2x-c628-3cx5)', () => {
let restoreService;
let originalNodeEnv;
beforeEach(() => {
restoreService = new RestoreService();
lookup.mockReset();
S3StorageAdapter.mockClear();
originalNodeEnv = process.env.NODE_ENV;
process.env.NODE_ENV = 'production';
});
afterEach(() => {
process.env.NODE_ENV = originalNodeEnv;
});
it('rejects an endpoint hostname that resolves to a private/internal address before any network call', async () => {
lookup.mockResolvedValue([{ address: '10.0.0.5', family: 4 }]);
await expect(
restoreService.downloadFileFromS3(
's3://backups/manifest.json',
'/tmp/whatever/manifest.json',
{ endpoint: 'evil-rebind.example.com', accessKeyId: 'k', secretAccessKey: 's' }
)
).rejects.toThrow(/private or internal network address/i);
expect(S3StorageAdapter).not.toHaveBeenCalled();
});
it('rejects an endpoint hostname that resolves to the cloud metadata address', async () => {
lookup.mockResolvedValue([{ address: '169.254.169.254', family: 4 }]);
await expect(
restoreService.downloadFileFromS3(
's3://backups/manifest.json',
'/tmp/whatever/manifest.json',
{ endpoint: 'metadata-rebind.example.com', accessKeyId: 'k', secretAccessKey: 's' }
)
).rejects.toThrow(/private or internal network address/i);
expect(S3StorageAdapter).not.toHaveBeenCalled();
});
it('allows a legitimate public S3 endpoint through to download()', async () => {
lookup.mockResolvedValue([{ address: '93.184.216.34', family: 4 }]);
await restoreService.downloadFileFromS3(
's3://backups/manifest.json',
'/tmp/whatever/manifest.json',
{ endpoint: 's3.example-cdn.com', accessKeyId: 'k', secretAccessKey: 's' }
);
expect(S3StorageAdapter).toHaveBeenCalledTimes(1);
});
it('does not require the guard outside production (dev MinIO stays usable), but still downloads', async () => {
process.env.NODE_ENV = 'development';
lookup.mockResolvedValue([{ address: '10.0.0.5', family: 4 }]); // would be rejected in prod
await restoreService.downloadFileFromS3(
's3://backups/manifest.json',
'/tmp/whatever/manifest.json',
{ endpoint: 'localhost:9000', accessKeyId: 'k', secretAccessKey: 's' }
);
expect(lookup).not.toHaveBeenCalled();
expect(S3StorageAdapter).toHaveBeenCalledTimes(1);
});
});
@@ -1,100 +0,0 @@
/**
* GHSA-h4w8-57xq-53fx entropy half: resetAdminPassword used to mint the
* emailed temp password with generateReadablePassword() — 10 adjectives x
* 10 nouns x crypto.randomInt(1000,9999) x 5 specials, ~2^21 possibilities,
* brute-forceable. It now uses generateSecurePassword(16) (90-char charset),
* same as every other security-sensitive password path in this file.
*
* Verified against a real SQLite DB (full core-migration set) so the
* emailed plaintext, the stored hash, and must_change_password are all
* checked end to end rather than against a mock.
*/
const path = require('path');
const fs = require('fs');
const os = require('os');
// bootCrmDb() sets TEST_DATABASE_PATH itself, but only in time for requires
// that happen AFTER it runs (inside beforeAll). userManagementService.js
// requires database/db.js at module load — i.e. before beforeAll — so that
// connection has to be pointed at a fresh, unused test DB up front, or it
// falls back to the shared default path and collides with whatever another
// test file already migrated onto it. Same workaround as
// userManagementService.activateDelete.test.js.
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-reset-pw-test-'));
process.env.NODE_ENV = 'test';
process.env.TEST_DATABASE_PATH = path.join(tmpDir, 'db.sqlite');
process.env.STORAGE_PATH = path.join(tmpDir, 'storage');
fs.mkdirSync(process.env.STORAGE_PATH, { recursive: true });
process.env.JWT_SECRET = process.env.JWT_SECRET || 'reset-pw-test-secret';
const bcrypt = require('bcrypt');
const { bootCrmDb, seedMinimal, assignAdminRole } = require('../integration/helpers/crmDb');
const userManagementService = require('../../src/services/userManagementService');
// The wordlist generateReadablePassword() used to produce:
// <Adjective><Noun><4 digits><1 special>, e.g. "SwiftEagle4821!"
const READABLE_WORDLIST_PATTERN = /^(Swift|Bright|Strong|Happy|Clever|Brave|Noble|Quick|Sharp|Bold)(Eagle|Mountain|River|Thunder|Forest|Ocean|Falcon|Dragon|Phoenix|Tiger)\d{4}[!@#$%]$/;
describe('userManagementService.resetAdminPassword (GHSA-h4w8-57xq-53fx)', () => {
let db;
let cleanup;
let actorId;
let targetId;
beforeAll(async () => {
({ db, cleanup } = await bootCrmDb());
({ adminId: actorId } = await seedMinimal(db));
await assignAdminRole(db, actorId, 'super_admin');
const editor = await db('roles').where({ name: 'editor' }).first();
const targetInsert = await db('admin_users').insert({
username: 'reset-target', email: 'reset-target@example.com',
password_hash: await bcrypt.hash('old-password', 4),
role_id: editor?.id || null,
is_active: 1, must_change_password: false, created_at: new Date().toISOString(),
}).returning('id');
targetId = targetInsert[0]?.id ?? targetInsert[0];
}, 120000);
afterAll(async () => { if (cleanup) await cleanup(); });
it('generates a high-entropy password, not one drawn from the adjective/noun wordlist', async () => {
const before = await db('admin_users').where({ id: targetId }).first();
await userManagementService.resetAdminPassword(targetId, actorId);
const emailRow = await db('email_queue')
.where({ recipient_email: 'reset-target@example.com', email_type: 'admin_password_reset' })
.orderBy('id', 'desc')
.first();
expect(emailRow).toBeDefined();
const emailData = JSON.parse(emailRow.email_data);
const newPassword = emailData.new_password;
// generateSecurePassword(16): fixed 16-char length, not the wordlist's
// variable-length "WordWord####!" shape.
expect(newPassword).toHaveLength(16);
expect(newPassword).not.toMatch(READABLE_WORDLIST_PATTERN);
// generateSecurePassword guarantees at least one of each character class.
expect(newPassword).toMatch(/[a-z]/);
expect(newPassword).toMatch(/[A-Z]/);
expect(newPassword).toMatch(/[0-9]/);
expect(newPassword).toMatch(/[!@#$%^&*()_+\-=[\]{}|;:,.<>?]/);
// The emailed plaintext actually matches what got persisted.
const after = await db('admin_users').where({ id: targetId }).first();
expect(after.password_hash).not.toBe(before.password_hash);
await expect(bcrypt.compare(newPassword, after.password_hash)).resolves.toBe(true);
});
it('sets must_change_password so the enforcement backstop kicks in on next login', async () => {
await db('admin_users').where({ id: targetId }).update({ must_change_password: false });
await userManagementService.resetAdminPassword(targetId, actorId);
const after = await db('admin_users').where({ id: targetId }).first();
expect(after.must_change_password === true || after.must_change_password === 1).toBe(true);
});
});
@@ -1,235 +0,0 @@
/**
* Privilege-escalation guard for PUT /api/admin/users/:id and
* POST /api/admin/users/invite (GHSA-rv8w-m6mx-7j4q).
*
* updateAdminUser's role-change path previously enforced only:
* (a) non-super_admin actors can't grant the super_admin role
* (b) no self-role-update / demoting the last super_admin
* It never checked whether the ACTOR's own permission set covers the
* permissions carried by the role being granted — so an admin holding
* only `users.edit` could hand any other admin a role (including the
* built-in `admin` role) carrying far more permissions than the actor
* itself held.
*
* createInvitation() had the identical gap: it only ever blocked
* granting super_admin, so an admin holding only `users.create` could
* invite a brand-new admin into any other role — including one carrying
* far more permissions than the inviter itself held — via
* POST /admin/users/invite.
*
* The fix adds assertActorMayGrant() — a local containment guard, since
* stable does not yet have main's custom-role-creation service or its
* roles.manage equivalent — inside both updateAdminUser's role_id branch
* and createInvitation().
*
* Both describe blocks below share a single bootCrmDb() call: the
* `db` module (`src/database/db.js`) is a singleton keyed off
* TEST_DATABASE_PATH at first require, and bootCrmDb's own comment
* warns that a second call after the first's cleanup() destroys the
* pool, leaving "Unable to acquire a connection" for every later query.
*/
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-rolegrantguard-')), 'db.sqlite',
);
process.env.JWT_SECRET = process.env.JWT_SECRET || 'rolegrantguard-test-secret';
process.env.STORAGE_PATH = fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-rolegrantguard-storage-'));
const { bootCrmDb, seedMinimal, assignAdminRole } = require('../integration/helpers/crmDb');
const svc = require('../../src/services/userManagementService');
const { clearPermissionCache } = require('../../src/middleware/permissions');
// Stable has no custom-role-creation service (that's main-only); build a role
// directly against the roles/permissions/role_permissions schema instead.
async function createRole(db, name, permissionNames) {
const [roleRow] = await db('roles').insert({
name, display_name: name, is_system: false, priority: 10, created_at: new Date(), updated_at: new Date(),
}).returning('id');
const roleId = roleRow?.id ?? roleRow;
if (permissionNames.length > 0) {
const perms = await db('permissions').whereIn('name', permissionNames).select('id', 'name');
if (perms.length !== permissionNames.length) {
throw new Error(`Missing seeded permission(s) for: ${permissionNames.join(', ')}`);
}
await db('role_permissions').insert(perms.map((p) => ({ role_id: roleId, permission_id: p.id })));
}
return { id: roleId };
}
let db; let cleanup;
let superId;
beforeAll(async () => {
({ db, cleanup } = await bootCrmDb());
({ adminId: superId } = await seedMinimal(db));
await assignAdminRole(db, superId, 'super_admin');
clearPermissionCache();
}, 120000);
afterAll(async () => { if (cleanup) await cleanup(); });
describe('updateAdminUser — role-grant privilege-escalation guard (GHSA-rv8w-m6mx-7j4q)', () => {
let limitedRoleId; let limitedId; // holds only users.edit + events.view
let powerfulRoleId; // carries settings.edit, which limitedId does NOT hold
let modestRoleId; // carries only events.view, a subset of what limitedId holds
let targetId; // account whose role limitedId will try to change
beforeAll(async () => {
// The attacker in GHSA-rv8w-m6mx-7j4q: users.edit only, nothing else.
const limitedRole = await createRole(db, 'limited_user_editor', ['users.edit', 'events.view']);
limitedRoleId = limitedRole.id;
const limitedIns = await db('admin_users').insert({
username: 'limited', email: 'limited@example.com', password_hash: 'x',
role_id: limitedRoleId, must_change_password: false, created_at: new Date(),
}).returning('id');
limitedId = limitedIns[0]?.id ?? limitedIns[0];
// A role carrying a permission the limited actor does not hold.
const powerfulRole = await createRole(db, 'powerful_role', ['users.edit', 'settings.edit']);
powerfulRoleId = powerfulRole.id;
// A role whose permissions ARE a subset of what the limited actor holds.
const modestRole = await createRole(db, 'modest_role', ['events.view']);
modestRoleId = modestRole.id;
clearPermissionCache();
}, 120000);
beforeEach(async () => {
// Fresh target for every test, role reset to modestRole so role-change
// assertions always start from a known baseline.
const existing = await db('admin_users').where({ username: 'target' }).first();
if (existing) {
targetId = existing.id;
await db('admin_users').where({ id: targetId }).update({ role_id: modestRoleId });
} else {
const ins = await db('admin_users').insert({
username: 'target', email: 'target@example.com', password_hash: 'x',
role_id: modestRoleId, must_change_password: false, created_at: new Date(),
}).returning('id');
targetId = ins[0]?.id ?? ins[0];
}
});
it('refuses to let an admin grant a role carrying permissions the admin lacks', async () => {
await expect(
svc.updateAdminUser(
targetId,
{ role_id: powerfulRoleId },
limitedId,
{ roleName: 'limited_user_editor' },
),
).rejects.toThrow(/only grant permissions your own role/i);
// Target's role must be unchanged.
const row = await db('admin_users').where({ id: targetId }).first();
expect(row.role_id).toBe(modestRoleId);
});
it('refuses to let an admin grant the built-in admin role beyond its own permissions', async () => {
const adminRole = await db('roles').where({ name: 'admin' }).first();
await expect(
svc.updateAdminUser(
targetId,
{ role_id: adminRole.id },
limitedId,
{ roleName: 'limited_user_editor' },
),
).rejects.toThrow(/only grant permissions your own role/i);
});
it('allows an admin to grant a role whose permissions it already holds', async () => {
const updated = await svc.updateAdminUser(
targetId,
{ role_id: limitedRoleId },
limitedId,
{ roleName: 'limited_user_editor' },
);
expect(updated.role_id).toBe(limitedRoleId);
});
it('super_admin can still grant any role, including one carrying more permissions than a limited actor holds', async () => {
const updated = await svc.updateAdminUser(
targetId,
{ role_id: powerfulRoleId },
superId,
{ roleName: 'super_admin' },
);
expect(updated.role_id).toBe(powerfulRoleId);
});
});
describe('createInvitation — role-grant privilege-escalation guard (GHSA-rv8w-m6mx-7j4q)', () => {
let limitedRoleId; let limitedId; // holds only users.create + events.view
let powerfulRoleId; // carries settings.edit, which limitedId does NOT hold
let modestRoleId; // carries only events.view, a subset of what limitedId holds
let inviteCounter = 0;
beforeAll(async () => {
const limitedRole = await createRole(db, 'limited_inviter', ['users.create', 'events.view']);
limitedRoleId = limitedRole.id;
const limitedIns = await db('admin_users').insert({
username: 'limited_inviter', email: 'limited_inviter@example.com', password_hash: 'x',
role_id: limitedRoleId, must_change_password: false, created_at: new Date(),
}).returning('id');
limitedId = limitedIns[0]?.id ?? limitedIns[0];
const powerfulRole = await createRole(db, 'powerful_invite_role', ['users.create', 'settings.edit']);
powerfulRoleId = powerfulRole.id;
const modestRole = await createRole(db, 'modest_invite_role', ['events.view']);
modestRoleId = modestRole.id;
clearPermissionCache();
}, 120000);
function nextEmail() {
inviteCounter += 1;
return `invitee-${inviteCounter}@example.com`;
}
it('refuses to let an admin invite someone into a role carrying permissions the admin lacks', async () => {
await expect(
svc.createInvitation({
email: nextEmail(),
roleId: powerfulRoleId,
invitedById: limitedId,
inviterRoleName: 'limited_inviter',
}),
).rejects.toThrow(/only grant permissions your own role/i);
});
it('allows an admin to invite someone into a role whose permissions it already holds', async () => {
const invitation = await svc.createInvitation({
email: nextEmail(),
roleId: limitedRoleId,
invitedById: limitedId,
inviterRoleName: 'limited_inviter',
});
expect(invitation.role).toBeTruthy();
});
it('allows an admin to invite someone into a role that is a subset of its own permissions', async () => {
const invitation = await svc.createInvitation({
email: nextEmail(),
roleId: modestRoleId,
invitedById: limitedId,
inviterRoleName: 'limited_inviter',
});
expect(invitation.role).toBeTruthy();
});
it('super_admin can still invite into any role, including one carrying more permissions than a limited actor holds', async () => {
const invitation = await svc.createInvitation({
email: nextEmail(),
roleId: powerfulRoleId,
invitedById: superId,
inviterRoleName: 'super_admin',
});
expect(invitation.role).toBeTruthy();
});
});
@@ -1,62 +0,0 @@
/**
* generateVideoPlaceholder() must not touch the database when the caller
* already supplies width/height (videoProcessor.js's thumbnail-generation
* fallback does exactly this).
*
* Why it matters: processUploadedPhotos() (chunked video upload) holds a
* per-file SQLite transaction open across thumbnail generation. SQLite's
* knex pool defaults to a single connection, so any second, un-transacted
* db() query made while that transaction is open blocks until
* acquireConnectionTimeout (60s in production) — verified directly against
* an isolated SQLite db (codex review of #1371/#1372). Passing explicit
* dimensions must skip getThumbnailSettings()'s db() call entirely, not
* just tolerate its failure.
*/
const path = require('path');
const fs = require('fs').promises;
const os = require('os');
const mockDbSpy = jest.fn(() => {
throw new Error('db() must not be called when width/height are supplied');
});
jest.mock('../../src/database/db', () => ({ db: (...args) => mockDbSpy(...args) }));
const storageModule = require('../../src/services/storage');
const LocalFsStorage = require('../../src/services/storage/LocalFsStorage');
describe('generateVideoPlaceholder skips the settings DB lookup given explicit dimensions', () => {
let storage;
let root;
let imageProcessor;
beforeAll(async () => {
root = await fs.mkdtemp(path.join(os.tmpdir(), 'picpeak-vidplaceholder-'));
storage = new LocalFsStorage({ root });
await storage.init();
storageModule.setStorageForTesting(storage);
imageProcessor = require('../../src/services/imageProcessor');
}, 30000);
afterAll(async () => {
storageModule.resetStorage();
await fs.rm(root, { recursive: true, force: true }).catch(() => {});
});
afterEach(() => mockDbSpy.mockClear());
it('never calls db() when width/height are provided', async () => {
const key = await imageProcessor.generateVideoPlaceholder('demo.mp4', { width: 300, height: 300 });
expect(key).toBe('thumbnails/thumb_demo.jpg');
expect(await storage.exists(key)).toBe(true);
expect(mockDbSpy).not.toHaveBeenCalled();
});
it('falls through to defaults (not a throw) when db() fails and no dimensions were given', async () => {
const key = await imageProcessor.generateVideoPlaceholder('demo2.mp4');
expect(key).toBe('thumbnails/thumb_demo2.jpg');
expect(mockDbSpy).toHaveBeenCalled();
});
});
@@ -1,122 +0,0 @@
/**
* Bounded, reclaimable storage reads for archiver downloads (#1399 follow-up).
*
* archiver drains the sources it is handed one at a time, so appending a
* storage read per photo opens N and drains one. Every other read parks its
* socket holding unread bytes, and nothing reclaims them: archiver's abort()
* does not touch source streams, and the S3 SDK clears its socket timeout as
* soon as response headers land. That is the mechanism behind the incident in
* PR #1402 — 43 of 50 pooled sockets held, uploads starved, restart required.
*
* #1402 fixes the cached-zip builder. These are the guarantees the same guard
* has to give the three remaining call sites, two of which need no admin
* credentials to reach.
*/
const { Readable } = require('stream');
const { createArchiveStreamGuard } = require('../../src/utils/archiveStreamGuard');
const makeStream = () => new Readable({ read() {} });
describe('archiveStreamGuard (#1399 follow-up)', () => {
it('lets the configured number of reads run at once', async () => {
const guard = createArchiveStreamGuard({ maxInFlight: 2 });
expect(await guard.acquire()).toBe(true);
guard.track(makeStream());
expect(await guard.acquire()).toBe(true);
guard.track(makeStream());
expect(guard.openCount).toBe(2);
});
it('parks the next acquire until a read finishes', async () => {
const guard = createArchiveStreamGuard({ maxInFlight: 1 });
await guard.acquire();
const first = guard.track(makeStream());
let resumed = false;
const pending = guard.acquire().then((ok) => { resumed = ok; });
await new Promise((r) => setImmediate(r));
expect(resumed).toBe(false); // still parked — this is the cap doing its job
first.push(null);
first.resume();
await pending;
expect(resumed).toBe(true);
});
it('releases a slot when a read errors, not just when it ends', async () => {
const guard = createArchiveStreamGuard({ maxInFlight: 1 });
await guard.acquire();
const stream = guard.track(makeStream());
stream.on('error', () => {});
stream.destroy(new Error('socket died'));
// Without the error listener the slot would never come back and the next
// photo would park forever.
expect(await guard.acquire()).toBe(true);
});
it('reports a failed read so the caller can abort the archive', async () => {
// A stream that errors while still QUEUED has no archiver listener on it
// yet. Releasing its slot and saying nothing leaves a dead stream in the
// queue, and the archive hangs when it reaches it.
const seen = [];
const guard = createArchiveStreamGuard({ maxInFlight: 2, onFatalError: (e) => seen.push(e) });
await guard.acquire();
const queued = guard.track(makeStream());
queued.on('error', () => {});
queued.destroy(new Error('socket died'));
await new Promise((r) => setImmediate(r)); // 'error' lands on the next tick
expect(seen).toHaveLength(1);
expect(seen[0].message).toBe('socket died');
});
it('stays quiet about reads it destroyed itself', async () => {
// destroyAll is the caller's own teardown; reporting those back as fatal
// would re-enter the abort path it is already running.
const seen = [];
const guard = createArchiveStreamGuard({ onFatalError: (e) => seen.push(e) });
await guard.acquire();
const s1 = guard.track(makeStream());
s1.on('error', () => {});
guard.destroyAll();
await new Promise((r) => setImmediate(r));
expect(seen).toHaveLength(0);
});
it('destroys every read still holding bytes', async () => {
const guard = createArchiveStreamGuard({ maxInFlight: 5 });
const streams = [makeStream(), makeStream(), makeStream()];
for (const s of streams) { await guard.acquire(); guard.track(s); }
expect(guard.openCount).toBe(3);
guard.destroyAll();
expect(streams.every((s) => s.destroyed)).toBe(true);
expect(guard.openCount).toBe(0);
});
it('wakes a parked acquire on destroyAll so the loop can exit', async () => {
const guard = createArchiveStreamGuard({ maxInFlight: 1 });
await guard.acquire();
guard.track(makeStream());
const pending = guard.acquire();
guard.destroyAll();
// false, so the caller breaks out instead of appending to a dead archive.
expect(await pending).toBe(false);
});
it('destroys a stream tracked after shutdown rather than leaking it', () => {
const guard = createArchiveStreamGuard();
guard.destroyAll();
const late = guard.track(makeStream());
expect(late.destroyed).toBe(true);
expect(guard.openCount).toBe(0);
});
it('tolerates destroyAll twice — exit paths overlap', () => {
const guard = createArchiveStreamGuard();
guard.track(makeStream());
guard.destroyAll();
expect(() => guard.destroyAll()).not.toThrow();
});
});
@@ -1,29 +0,0 @@
/**
* Migration 180: TOTP replay protection for admin MFA (GHSA-qcwx-r25m-j869).
*
* verifyTotp()/verifyTotpEncrypted() were stateless: otplib's window:1
* tolerance means a captured 6-digit code stays valid across several real
* time-steps (~90s), so the same code could complete two independent admin
* logins. `two_factor_last_used_step` tracks, per admin, the absolute TOTP
* time-step (Math.floor(Date.now() / 30000)) that their last successfully
* consumed code matched; mfaService now rejects a code whose matched step
* doesn't advance past it.
*
* Additive and idempotent: only adds a column, guarded by hasColumn, so it
* is safe to re-run and touches no existing data.
*/
exports.up = async function (knex) {
if (!(await knex.schema.hasColumn('admin_users', 'two_factor_last_used_step'))) {
await knex.schema.alterTable('admin_users', (t) => {
t.integer('two_factor_last_used_step').nullable();
});
}
};
exports.down = async function (knex) {
if (await knex.schema.hasColumn('admin_users', 'two_factor_last_used_step')) {
await knex.schema.alterTable('admin_users', (t) => {
t.dropColumn('two_factor_last_used_step');
});
}
};
+135 -135
View File
@@ -1,12 +1,12 @@
{
"name": "picpeak-backend",
"version": "3.46.12",
"version": "3.46.8",
"lockfileVersion": 3,
"requires": true,
"packages": {
"": {
"name": "picpeak-backend",
"version": "3.46.12",
"version": "3.46.8",
"dependencies": {
"@aws-sdk/client-s3": "^3.850.0",
"@aws-sdk/lib-storage": "^3.850.0",
@@ -37,7 +37,7 @@
"knex": "^2.4.2",
"mailparser": "^3.9.9",
"mime-types": "^3.0.1",
"multer": "2.3.0",
"multer": "2.2.0",
"node-cron": "^3.0.2",
"node-stream-zip": "^1.15.0",
"nodemailer": "^9.0.1",
@@ -50,7 +50,7 @@
"qrcode": "^1.5.4",
"react-i18next": "^15.6.0",
"sanitize-html": "2.17.7",
"sharp": "0.35.4",
"sharp": "0.35.3",
"sqlite3": "^5.1.6",
"swagger-jsdoc": "^6.2.8",
"swagger-ui-express": "^5.0.1",
@@ -1689,9 +1689,9 @@
}
},
"node_modules/@img/sharp-darwin-arm64": {
"version": "0.35.4",
"resolved": "https://registry.npmjs.org/@img/sharp-darwin-arm64/-/sharp-darwin-arm64-0.35.4.tgz",
"integrity": "sha512-Uhfl4V4lhP2nbUVF9+hyH1+luj86f1gUFeo8ALYxFoULoU+G87D43BfeMP8XHsk9boxAnCY/bf2EHwhA7MuGsA==",
"version": "0.35.3",
"resolved": "https://registry.npmjs.org/@img/sharp-darwin-arm64/-/sharp-darwin-arm64-0.35.3.tgz",
"integrity": "sha512-RMnFX7YQsMoh7lWfcM4NEHHymBX/rLuKNPVM84XE9ONPcaSCDgE7CHIHpSgPcO2xcRthgBy1HfNO319mwhIAkg==",
"cpu": [
"arm64"
],
@@ -1707,13 +1707,13 @@
"url": "https://opencollective.com/libvips"
},
"optionalDependencies": {
"@img/sharp-libvips-darwin-arm64": "1.3.3"
"@img/sharp-libvips-darwin-arm64": "1.3.2"
}
},
"node_modules/@img/sharp-darwin-x64": {
"version": "0.35.4",
"resolved": "https://registry.npmjs.org/@img/sharp-darwin-x64/-/sharp-darwin-x64-0.35.4.tgz",
"integrity": "sha512-hWniXY3bG5qKpkKrAwPe4y+VTPmf086YQAnkxWh7uA1YrlRouWGa0M0Mxj3ZjnXFkv7/TD1bTy9lGUK26vRvWw==",
"version": "0.35.3",
"resolved": "https://registry.npmjs.org/@img/sharp-darwin-x64/-/sharp-darwin-x64-0.35.3.tgz",
"integrity": "sha512-Xo+5uFBtLN0BKqieTxiFzFPQAUlBbbH5iBKyRX/z1JrbnYsHTfKJnUfL8+p2TPXr1pXqao4eeL4Rl144uDpK9w==",
"cpu": [
"x64"
],
@@ -1729,20 +1729,20 @@
"url": "https://opencollective.com/libvips"
},
"optionalDependencies": {
"@img/sharp-libvips-darwin-x64": "1.3.3"
"@img/sharp-libvips-darwin-x64": "1.3.2"
}
},
"node_modules/@img/sharp-freebsd-wasm32": {
"version": "0.35.4",
"resolved": "https://registry.npmjs.org/@img/sharp-freebsd-wasm32/-/sharp-freebsd-wasm32-0.35.4.tgz",
"integrity": "sha512-lIsKw/BU+kjB4eZjxrYrZmwOJYi3Ajrv66iAlBmUPyKc3HpnloevB1g3wxGD9P/5BbQ1brBGl65VRRrCvQDEqA==",
"version": "0.35.3",
"resolved": "https://registry.npmjs.org/@img/sharp-freebsd-wasm32/-/sharp-freebsd-wasm32-0.35.3.tgz",
"integrity": "sha512-lUxcqWIj2wMQ9BrwNjngcr1gWUr5xgaGThBRqPPalIC2n67Cqj1uPh8NnA/ZhAg8hUbKl+kVHKwgUIwe6ZYPrg==",
"license": "Apache-2.0",
"optional": true,
"os": [
"freebsd"
],
"dependencies": {
"@img/sharp-wasm32": "0.35.4"
"@img/sharp-wasm32": "0.35.3"
},
"engines": {
"node": ">=20.9.0"
@@ -1752,9 +1752,9 @@
}
},
"node_modules/@img/sharp-libvips-darwin-arm64": {
"version": "1.3.3",
"resolved": "https://registry.npmjs.org/@img/sharp-libvips-darwin-arm64/-/sharp-libvips-darwin-arm64-1.3.3.tgz",
"integrity": "sha512-suTBPTDGrI9WodccaDdwZItTSaBYASlBk1NSfElSHrUfzu3szG6lvIF58+WiFvnfzuK8ZBFS5zE00PxqxnRiPg==",
"version": "1.3.2",
"resolved": "https://registry.npmjs.org/@img/sharp-libvips-darwin-arm64/-/sharp-libvips-darwin-arm64-1.3.2.tgz",
"integrity": "sha512-9J6ypZFpQBj4YnePGoq/S38w6nz+vqg5WZLrLGY4YuSemdMq47GMLBPO42MzwdGwpg/agZ7xzZcFHa48xlywfg==",
"cpu": [
"arm64"
],
@@ -1768,9 +1768,9 @@
}
},
"node_modules/@img/sharp-libvips-darwin-x64": {
"version": "1.3.3",
"resolved": "https://registry.npmjs.org/@img/sharp-libvips-darwin-x64/-/sharp-libvips-darwin-x64-1.3.3.tgz",
"integrity": "sha512-FVJZ5mITMobmXIz/hPDTw0EintTW5H3WfrxwLqEqjiIihlu+hVRyGrFQ60xl0Lxn7Bt3zdpevPaQi0HEzqz9fw==",
"version": "1.3.2",
"resolved": "https://registry.npmjs.org/@img/sharp-libvips-darwin-x64/-/sharp-libvips-darwin-x64-1.3.2.tgz",
"integrity": "sha512-m2pW1n6cns9VaubNwsZ+c3CRYjxNQWgJ5gPlnL1nbBcpkBvFm6SCFN5o0psFHI8w9n11NKhFkeEDns98tiqbEw==",
"cpu": [
"x64"
],
@@ -1784,9 +1784,9 @@
}
},
"node_modules/@img/sharp-libvips-linux-arm": {
"version": "1.3.3",
"resolved": "https://registry.npmjs.org/@img/sharp-libvips-linux-arm/-/sharp-libvips-linux-arm-1.3.3.tgz",
"integrity": "sha512-3rbU4vqXXc3hY/OiXdl52xZvT0F1yEngWfvqudtPJg/KkyiaQw2DRsFrNzpmLvfavbwOq3qXn36GP8obHRULQA==",
"version": "1.3.2",
"resolved": "https://registry.npmjs.org/@img/sharp-libvips-linux-arm/-/sharp-libvips-linux-arm-1.3.2.tgz",
"integrity": "sha512-1eMLzy92I4J6rmi4mAT8yC3HxOtniyGELlzGbNMLLeqe052ahFQ0h6LFq+lh5DsDIdYViIDst08abvSbcEdLXQ==",
"cpu": [
"arm"
],
@@ -1800,9 +1800,9 @@
}
},
"node_modules/@img/sharp-libvips-linux-arm64": {
"version": "1.3.3",
"resolved": "https://registry.npmjs.org/@img/sharp-libvips-linux-arm64/-/sharp-libvips-linux-arm64-1.3.3.tgz",
"integrity": "sha512-0DaL0A6Xu6sQSQFwe4iVCrKWU2cCTItnRsYsCdxAMm9NF6twAA9BKnoqy4hqz4+azQ0JHuA26qiUKsf1XJ/v5A==",
"version": "1.3.2",
"resolved": "https://registry.npmjs.org/@img/sharp-libvips-linux-arm64/-/sharp-libvips-linux-arm64-1.3.2.tgz",
"integrity": "sha512-dqVSFynCox4C/J8kT16V7SIFAns0IjgLwkvYT7p8LQVmJ5OS5b6tI9IGflxTeuBS//zXeFIUbwt5dwxyZ17cnA==",
"cpu": [
"arm64"
],
@@ -1816,9 +1816,9 @@
}
},
"node_modules/@img/sharp-libvips-linux-ppc64": {
"version": "1.3.3",
"resolved": "https://registry.npmjs.org/@img/sharp-libvips-linux-ppc64/-/sharp-libvips-linux-ppc64-1.3.3.tgz",
"integrity": "sha512-cdn1OvUBwsXhbC0zSzJnNzf5MZ/mTrobawDvNXBTxe8VtqKAm0sRuEY2Evzovb/w9JMk4TvRxqt1mekSuJz64w==",
"version": "1.3.2",
"resolved": "https://registry.npmjs.org/@img/sharp-libvips-linux-ppc64/-/sharp-libvips-linux-ppc64-1.3.2.tgz",
"integrity": "sha512-3z0NHDxD6n5I9gc05U1eW1AyRm+Gznzq3naMrthPNqE6oYykcogW0l/jfpJdjYnuNl8R7yI9pNbE1XiUeyq0Aw==",
"cpu": [
"ppc64"
],
@@ -1832,9 +1832,9 @@
}
},
"node_modules/@img/sharp-libvips-linux-riscv64": {
"version": "1.3.3",
"resolved": "https://registry.npmjs.org/@img/sharp-libvips-linux-riscv64/-/sharp-libvips-linux-riscv64-1.3.3.tgz",
"integrity": "sha512-HjPVx7yKz+0lqdhDlTw1tt90wamBoxhiXpvl1XZpJLiHH4RCJ5yDTqH+VlYPv2fwFs89JFw4c1IexYOcQUi4IQ==",
"version": "1.3.2",
"resolved": "https://registry.npmjs.org/@img/sharp-libvips-linux-riscv64/-/sharp-libvips-linux-riscv64-1.3.2.tgz",
"integrity": "sha512-bsb4rI+NldGOsXuej2r8OdSS8+zXDVaCWxyWrcv6kneTOlgAHtZABRzBBCwdsPiD90J4myNJuHpg6kA20ImW/w==",
"cpu": [
"riscv64"
],
@@ -1848,9 +1848,9 @@
}
},
"node_modules/@img/sharp-libvips-linux-s390x": {
"version": "1.3.3",
"resolved": "https://registry.npmjs.org/@img/sharp-libvips-linux-s390x/-/sharp-libvips-linux-s390x-1.3.3.tgz",
"integrity": "sha512-neWLh+3yCNThxnfy3c4BbVBeGgt9aftno+XbT56iK28RgeDs3UOFWviLWlUu0bArYVYJaFDK+RRohbicUNCm8Q==",
"version": "1.3.2",
"resolved": "https://registry.npmjs.org/@img/sharp-libvips-linux-s390x/-/sharp-libvips-linux-s390x-1.3.2.tgz",
"integrity": "sha512-/ABshyj8gCpyIrNXnHn4LorDJ0HHm1VhXPBlxZ8zAtfVPAaSafXPGn+sUSIRiwaSBy0mmFjSjiXI5mkcwdChKQ==",
"cpu": [
"s390x"
],
@@ -1864,9 +1864,9 @@
}
},
"node_modules/@img/sharp-libvips-linux-x64": {
"version": "1.3.3",
"resolved": "https://registry.npmjs.org/@img/sharp-libvips-linux-x64/-/sharp-libvips-linux-x64-1.3.3.tgz",
"integrity": "sha512-4vKmvAst9nrowcqquKFAyZJUDolUaIp8uRiN0mWFguJ1IplC9/pitXtlnnlU4aa/eJw3J7i67V+pwUL+wZGdsA==",
"version": "1.3.2",
"resolved": "https://registry.npmjs.org/@img/sharp-libvips-linux-x64/-/sharp-libvips-linux-x64-1.3.2.tgz",
"integrity": "sha512-ITPEtgffGJ0S6G9dRyw/366tJQqFRcHWPHhC+Stpg3Z8AEMrDrTr2lhdz4f/Y/HMbRh//7Z5mBzEpVdi62Oc3w==",
"cpu": [
"x64"
],
@@ -1880,9 +1880,9 @@
}
},
"node_modules/@img/sharp-libvips-linuxmusl-arm64": {
"version": "1.3.3",
"resolved": "https://registry.npmjs.org/@img/sharp-libvips-linuxmusl-arm64/-/sharp-libvips-linuxmusl-arm64-1.3.3.tgz",
"integrity": "sha512-Y9kQaLMuNoB0bPYOOdcZMaseNrFpPodIWWMrx+CZyydf2xn68j9WYc6sWWRrDwNkzCQjKYfc68L7jKjGlHMibw==",
"version": "1.3.2",
"resolved": "https://registry.npmjs.org/@img/sharp-libvips-linuxmusl-arm64/-/sharp-libvips-linuxmusl-arm64-1.3.2.tgz",
"integrity": "sha512-zE9EdiUzUmg5mDT5a1rk5fYJ6GWPloTwWBYDS14naqHsL+EaMpDj1AWnpLgh3u0YCORv2Tt50wrcrpYqkP97Kw==",
"cpu": [
"arm64"
],
@@ -1896,9 +1896,9 @@
}
},
"node_modules/@img/sharp-libvips-linuxmusl-x64": {
"version": "1.3.3",
"resolved": "https://registry.npmjs.org/@img/sharp-libvips-linuxmusl-x64/-/sharp-libvips-linuxmusl-x64-1.3.3.tgz",
"integrity": "sha512-fj8Mv0HHfD1Rr+4I68+3agJynxDWtBFgicTbSOb9Bke6pIwzGcJ+RX/yHjmiEGFMCavY/dxvem7MyNaJF+wDiw==",
"version": "1.3.2",
"resolved": "https://registry.npmjs.org/@img/sharp-libvips-linuxmusl-x64/-/sharp-libvips-linuxmusl-x64-1.3.2.tgz",
"integrity": "sha512-m0lrLiUt+lBYnCFr8qV/65yMR4E/c7/wf78I5eKTdkEakFAlZ9QlzEM3QIhhAwVeUhLAHLcCq7a7Vszq/oFNZQ==",
"cpu": [
"x64"
],
@@ -1912,9 +1912,9 @@
}
},
"node_modules/@img/sharp-linux-arm": {
"version": "0.35.4",
"resolved": "https://registry.npmjs.org/@img/sharp-linux-arm/-/sharp-linux-arm-0.35.4.tgz",
"integrity": "sha512-7OAS8gI0EReKGVN2HssHlM6umJgxF5VI3xN0p9FA91p/YO+ou5hiNghLdZ5BEHztwaaK5+bLKRf8x/o2L2nk9A==",
"version": "0.35.3",
"resolved": "https://registry.npmjs.org/@img/sharp-linux-arm/-/sharp-linux-arm-0.35.3.tgz",
"integrity": "sha512-affVWCTLooy8TSxbDx2qkzuDeaWLNVBA+P//FNBirHsXpP2fuBhk5AuboYUnrDnzoXes8GFjpTx0SBFOCRg+FA==",
"cpu": [
"arm"
],
@@ -1930,13 +1930,13 @@
"url": "https://opencollective.com/libvips"
},
"optionalDependencies": {
"@img/sharp-libvips-linux-arm": "1.3.3"
"@img/sharp-libvips-linux-arm": "1.3.2"
}
},
"node_modules/@img/sharp-linux-arm64": {
"version": "0.35.4",
"resolved": "https://registry.npmjs.org/@img/sharp-linux-arm64/-/sharp-linux-arm64-0.35.4.tgz",
"integrity": "sha512-De4jpEnAU8Hd5oT0j1G3uL4ZvTuipVMn7YC6vPaJhy6/7EwEae0SVAoBrUMYQbkLGDm85taVWwuPc1a44LTzCQ==",
"version": "0.35.3",
"resolved": "https://registry.npmjs.org/@img/sharp-linux-arm64/-/sharp-linux-arm64-0.35.3.tgz",
"integrity": "sha512-QgKDspHPnrU+GQ55XPhGwyhC8acLVOOSyAvo1oVfFmrIXLkDNmGWzAfDZ4xK8oSA1qBQrALcHX0G5UZni/SuFQ==",
"cpu": [
"arm64"
],
@@ -1952,13 +1952,13 @@
"url": "https://opencollective.com/libvips"
},
"optionalDependencies": {
"@img/sharp-libvips-linux-arm64": "1.3.3"
"@img/sharp-libvips-linux-arm64": "1.3.2"
}
},
"node_modules/@img/sharp-linux-ppc64": {
"version": "0.35.4",
"resolved": "https://registry.npmjs.org/@img/sharp-linux-ppc64/-/sharp-linux-ppc64-0.35.4.tgz",
"integrity": "sha512-2oYZJeIl4kCcMGk4ouZVjnkCtFrpQFlNEtJ6GbxzhHQchwH0NH/qEb9ykmOl29dqwMq+JhFdZn+1ak2FKhI9fQ==",
"version": "0.35.3",
"resolved": "https://registry.npmjs.org/@img/sharp-linux-ppc64/-/sharp-linux-ppc64-0.35.3.tgz",
"integrity": "sha512-sMd8rDxmpLOwv/7N44klFjOD5DUO7FLdjiXDI0hoxYaf7Ar262dQIEkosE98bps+5HPLtp/EvNqeqQtOycP/IA==",
"cpu": [
"ppc64"
],
@@ -1974,13 +1974,13 @@
"url": "https://opencollective.com/libvips"
},
"optionalDependencies": {
"@img/sharp-libvips-linux-ppc64": "1.3.3"
"@img/sharp-libvips-linux-ppc64": "1.3.2"
}
},
"node_modules/@img/sharp-linux-riscv64": {
"version": "0.35.4",
"resolved": "https://registry.npmjs.org/@img/sharp-linux-riscv64/-/sharp-linux-riscv64-0.35.4.tgz",
"integrity": "sha512-cPbNChoRURAWdebDIHSenxRpgEdy7JkPydSnUxRm9VvKD7m0/xVaR/8Fzlu81pk5nHEvHH87UZUA7cTtwnbJSA==",
"version": "0.35.3",
"resolved": "https://registry.npmjs.org/@img/sharp-linux-riscv64/-/sharp-linux-riscv64-0.35.3.tgz",
"integrity": "sha512-0Eob78yjlYPfL5vMNWAW55l3R9Y6BQS/gOfe0ZcP9mEz9ohhKSt4im1hayiknXgf8AWrFqMvJcKIdmLmEe7yeQ==",
"cpu": [
"riscv64"
],
@@ -1996,13 +1996,13 @@
"url": "https://opencollective.com/libvips"
},
"optionalDependencies": {
"@img/sharp-libvips-linux-riscv64": "1.3.3"
"@img/sharp-libvips-linux-riscv64": "1.3.2"
}
},
"node_modules/@img/sharp-linux-s390x": {
"version": "0.35.4",
"resolved": "https://registry.npmjs.org/@img/sharp-linux-s390x/-/sharp-linux-s390x-0.35.4.tgz",
"integrity": "sha512-RY0JFY8Fd6RonCBtHz+DvadaPkXDSI1AUn6yWL9TipqkZ1vY8w8evqdgyDFnkm4/K1ve1TvZiaePP5oSd4+WVQ==",
"version": "0.35.3",
"resolved": "https://registry.npmjs.org/@img/sharp-linux-s390x/-/sharp-linux-s390x-0.35.3.tgz",
"integrity": "sha512-KgAxQ0DxpNOq1rG2t5cgTgShJFGSuU7XO45cqC+1NVOuZnP6tlgZRuSYOfNupGkHID0o3cJOsw4DVeJpMovcGw==",
"cpu": [
"s390x"
],
@@ -2018,13 +2018,13 @@
"url": "https://opencollective.com/libvips"
},
"optionalDependencies": {
"@img/sharp-libvips-linux-s390x": "1.3.3"
"@img/sharp-libvips-linux-s390x": "1.3.2"
}
},
"node_modules/@img/sharp-linux-x64": {
"version": "0.35.4",
"resolved": "https://registry.npmjs.org/@img/sharp-linux-x64/-/sharp-linux-x64-0.35.4.tgz",
"integrity": "sha512-9qvvEAuk8k89TfWUoX2htWjbAMX8p+NxCppjpcg5k6xMsjhBQPTsoIh36h9Qde4WRuGpJeYnOjdosDn/cnv+OA==",
"version": "0.35.3",
"resolved": "https://registry.npmjs.org/@img/sharp-linux-x64/-/sharp-linux-x64-0.35.3.tgz",
"integrity": "sha512-8pqvxubL2PGdhlPy6GLqzDYMUjyRmKAwKHYKixpdJYBUK7PJ0C029XdsnpFIdgRZG68fZiGdHVWcKPvtiPB4cA==",
"cpu": [
"x64"
],
@@ -2040,13 +2040,13 @@
"url": "https://opencollective.com/libvips"
},
"optionalDependencies": {
"@img/sharp-libvips-linux-x64": "1.3.3"
"@img/sharp-libvips-linux-x64": "1.3.2"
}
},
"node_modules/@img/sharp-linuxmusl-arm64": {
"version": "0.35.4",
"resolved": "https://registry.npmjs.org/@img/sharp-linuxmusl-arm64/-/sharp-linuxmusl-arm64-0.35.4.tgz",
"integrity": "sha512-KB5jxpfWQTr0nc3xdHtWChdbifHrBGsd2SM62Eyxrl8afikm+f5qGBU75SJIZBT/S1MC8XyacdlXBMSWq6OURA==",
"version": "0.35.3",
"resolved": "https://registry.npmjs.org/@img/sharp-linuxmusl-arm64/-/sharp-linuxmusl-arm64-0.35.3.tgz",
"integrity": "sha512-Vz0iQjzzcSX3HCbfwFfCSG/9SCIqyO0mH2sXyiHaAYfBk0cRsCWXRyQYX0ovCK/PAQBbTzQ0dsPQHh5MAFL59w==",
"cpu": [
"arm64"
],
@@ -2062,13 +2062,13 @@
"url": "https://opencollective.com/libvips"
},
"optionalDependencies": {
"@img/sharp-libvips-linuxmusl-arm64": "1.3.3"
"@img/sharp-libvips-linuxmusl-arm64": "1.3.2"
}
},
"node_modules/@img/sharp-linuxmusl-x64": {
"version": "0.35.4",
"resolved": "https://registry.npmjs.org/@img/sharp-linuxmusl-x64/-/sharp-linuxmusl-x64-0.35.4.tgz",
"integrity": "sha512-f+eZJZIQNEEd26RPSW+76chwOf1XtA2Y/O+5ocVyLliHkeih3e+jhLVBdNTd2rS3IbNXK8+ug93Vf5ZXtF5Lxg==",
"version": "0.35.3",
"resolved": "https://registry.npmjs.org/@img/sharp-linuxmusl-x64/-/sharp-linuxmusl-x64-0.35.3.tgz",
"integrity": "sha512-6O1NPKcDVj9QEdg7Hx549EX8U0rp6yXQERqru6yRN7fGBn32UvIRJUlWnk+8xDCiG76hXVBbX82NZ/ZKr0euIg==",
"cpu": [
"x64"
],
@@ -2084,17 +2084,17 @@
"url": "https://opencollective.com/libvips"
},
"optionalDependencies": {
"@img/sharp-libvips-linuxmusl-x64": "1.3.3"
"@img/sharp-libvips-linuxmusl-x64": "1.3.2"
}
},
"node_modules/@img/sharp-wasm32": {
"version": "0.35.4",
"resolved": "https://registry.npmjs.org/@img/sharp-wasm32/-/sharp-wasm32-0.35.4.tgz",
"integrity": "sha512-zQnl4Kwp7Q6NHsENtU2T/00Zi+w3AQNwz3+UaTyVBy2FpXrzXzGjndpK61onhZjRtRpQXxCTeqw19bVyXOh7jA==",
"version": "0.35.3",
"resolved": "https://registry.npmjs.org/@img/sharp-wasm32/-/sharp-wasm32-0.35.3.tgz",
"integrity": "sha512-cZ0XkcYGpHZkqW6iCkqTcmUC0CD9DhD5d/qeZlZkfRBn6GnHniZXLUo5+9xw8Iv76YE6LQFN9YNBlKREcCG76w==",
"license": "Apache-2.0 AND LGPL-3.0-or-later AND MIT",
"optional": true,
"dependencies": {
"@emnapi/runtime": "^1.11.3"
"@emnapi/runtime": "^1.11.1"
},
"engines": {
"node": ">=20.9.0"
@@ -2104,16 +2104,16 @@
}
},
"node_modules/@img/sharp-webcontainers-wasm32": {
"version": "0.35.4",
"resolved": "https://registry.npmjs.org/@img/sharp-webcontainers-wasm32/-/sharp-webcontainers-wasm32-0.35.4.tgz",
"integrity": "sha512-ESfNkywmCfPNyaZjxooddJQiQ+l/nTpGEOGthxiLnIHXC/CmcBixnfwUleX9mCz9ovrUUvKMap/pm8RYbzfwaA==",
"version": "0.35.3",
"resolved": "https://registry.npmjs.org/@img/sharp-webcontainers-wasm32/-/sharp-webcontainers-wasm32-0.35.3.tgz",
"integrity": "sha512-2rnq7bX3NzeR2T4YWgz8qiG4h3TSdMe+vN1iQXpJleSJ3SM5zQ8Fy2SyyXAWlbxpEZ2Y+Z4u1BePgJEYbSy80Q==",
"cpu": [
"wasm32"
],
"license": "Apache-2.0",
"optional": true,
"dependencies": {
"@img/sharp-wasm32": "0.35.4"
"@img/sharp-wasm32": "0.35.3"
},
"engines": {
"node": ">=20.9.0"
@@ -2123,9 +2123,9 @@
}
},
"node_modules/@img/sharp-win32-arm64": {
"version": "0.35.4",
"resolved": "https://registry.npmjs.org/@img/sharp-win32-arm64/-/sharp-win32-arm64-0.35.4.tgz",
"integrity": "sha512-iNdlBX9gLVvqe2I3uIJSIKTq6wckP/DYxZtcqxm09x5Gi24DnFBmPAWZmr60ZyYMG0xlzo6goG3670ar+RXvRw==",
"version": "0.35.3",
"resolved": "https://registry.npmjs.org/@img/sharp-win32-arm64/-/sharp-win32-arm64-0.35.3.tgz",
"integrity": "sha512-4bPwFdMbeC4JQ8L8LOyWp6nsHcboP5fxkp6iPOXz2Vg49R42TuMs2whkJ5OAP4/Ul035qOzy0AecOF9VOscn4w==",
"cpu": [
"arm64"
],
@@ -2142,9 +2142,9 @@
}
},
"node_modules/@img/sharp-win32-ia32": {
"version": "0.35.4",
"resolved": "https://registry.npmjs.org/@img/sharp-win32-ia32/-/sharp-win32-ia32-0.35.4.tgz",
"integrity": "sha512-kqRsbaa5CS6KHlpxnN7WhE6vAAugXyZButpRdvDWetlv6Qv4N9WTcrWzF7tXfB9T7MsoadqdI8hmwLq6UlLvtw==",
"version": "0.35.3",
"resolved": "https://registry.npmjs.org/@img/sharp-win32-ia32/-/sharp-win32-ia32-0.35.3.tgz",
"integrity": "sha512-r53mXsBN6lFUDiST764SvgwUdHAqM4rPAiDzAmf4fLoB6X/rkfyTrLCg6+g17wJJiCmB3JYgHuUldCWUIRFSXw==",
"cpu": [
"ia32"
],
@@ -2161,9 +2161,9 @@
}
},
"node_modules/@img/sharp-win32-x64": {
"version": "0.35.4",
"resolved": "https://registry.npmjs.org/@img/sharp-win32-x64/-/sharp-win32-x64-0.35.4.tgz",
"integrity": "sha512-XtmnYhBcrORsJ4XJngyzr/EWP0hRZLAZRFaApdKuviyqF78+ylxh2y06ZmtULAMOnObJ3ucpN0AcwSWnMowTRg==",
"version": "0.35.3",
"resolved": "https://registry.npmjs.org/@img/sharp-win32-x64/-/sharp-win32-x64-0.35.3.tgz",
"integrity": "sha512-D4y1vNeZrIIJCN+uHaWVtH86B+aCrdMYYjicy9pXHvbGZeGYLLSd3wdVuC37FxVXlU1ARsk84eKWfWMXGYEqvA==",
"cpu": [
"x64"
],
@@ -7964,9 +7964,9 @@
}
},
"node_modules/joi": {
"version": "17.13.7",
"resolved": "https://registry.npmjs.org/joi/-/joi-17.13.7.tgz",
"integrity": "sha512-MF80Dm5Y2veNy8QWVx9Bj3ui4mo7+VPSPsR1M+oaHXV0Gx6zGX9a2F+OZG3Blby9tOlzU9Rs5FUimlEhbKtfnQ==",
"version": "17.13.4",
"resolved": "https://registry.npmjs.org/joi/-/joi-17.13.4.tgz",
"integrity": "sha512-1RuuER6kmt8K8I3nIWvPZKi5RQCb568ZPyY4Pwjlua+yo+63ZTmIwxLZH0heBmiKN4uxjvCiarDrjaeH84xicQ==",
"license": "BSD-3-Clause",
"dependencies": {
"@hapi/hoek": "^9.3.0",
@@ -7991,9 +7991,9 @@
"license": "MIT"
},
"node_modules/js-yaml": {
"version": "4.3.2",
"resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-4.3.2.tgz",
"integrity": "sha512-SFNOvSJ+Dgf/9An904Yx+CgSlIPCkIpao4qo51lpee25TIRejdH3rhR4EZMGoNx3/TP3O+wzWuiTFl4sqbltzA==",
"version": "4.3.1",
"resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-4.3.1.tgz",
"integrity": "sha512-CY6crGq313MX8GkwvB7tzgp99vjQxY1++5y10/BKN/GUfHqWaOGQMNZkBvqSzsZKWk/ijwHlWzzkLulsGHhjWQ==",
"funding": [
{
"type": "github",
@@ -9067,9 +9067,9 @@
"license": "MIT"
},
"node_modules/multer": {
"version": "2.3.0",
"resolved": "https://registry.npmjs.org/multer/-/multer-2.3.0.tgz",
"integrity": "sha512-cjNbm3sttszgZeGfJR124D+jFEfkXCVAsoPBmFn9X7UxmDSFHWqE2CoEj0vrmSpuAFnqWR1Szcm9QTsiHr60Xw==",
"version": "2.2.0",
"resolved": "https://registry.npmjs.org/multer/-/multer-2.2.0.tgz",
"integrity": "sha512-6rdyFg2kLrMh9Jee7/BMPuV9lEAd7lLW2YUpF9/YxR7njyoUwwQ0ZPh3TaIY50Sw6vlyD2HW3wGOkTS4P79xrQ==",
"license": "MIT",
"dependencies": {
"append-field": "^1.0.0",
@@ -9270,9 +9270,9 @@
}
},
"node_modules/nodemailer": {
"version": "9.1.1",
"resolved": "https://registry.npmjs.org/nodemailer/-/nodemailer-9.1.1.tgz",
"integrity": "sha512-izw9mVKFix6YSnC9eLgV6g1opl9DUlRio9ZNcq+Wu9Ujn2UwF+8Nl0B8nz22kEC+CTZCvinkxwJ0DeFbb6NwcQ==",
"version": "9.0.1",
"resolved": "https://registry.npmjs.org/nodemailer/-/nodemailer-9.0.1.tgz",
"integrity": "sha512-Gwv8SQewT616ZM/URn0H54b8PWo/Wum7md3EW2aWy1lO27+WZCX+Xyak3J+NlmHUjDh5ME+uesJUDRbR3Ye8Bw==",
"license": "MIT-0",
"engines": {
"node": ">=6.0.0"
@@ -11061,9 +11061,9 @@
"license": "ISC"
},
"node_modules/sharp": {
"version": "0.35.4",
"resolved": "https://registry.npmjs.org/sharp/-/sharp-0.35.4.tgz",
"integrity": "sha512-n++8XWcj+jCOr2IOl7h8LbKnGBDY4aPbmprMONBNFdn0ImXqpGVv5zliDs0V9HbmbCQLpbuo2ej9rAoOQTvMDA==",
"version": "0.35.3",
"resolved": "https://registry.npmjs.org/sharp/-/sharp-0.35.3.tgz",
"integrity": "sha512-ej0zVHuZGHCiABXcNxeYhpRnPNPAcvbG8RMdBAhDAxLKkCRVSpK3Iyu7qbqw3JMzoj0REeM6f3tJLtVwl0023Q==",
"license": "Apache-2.0",
"dependencies": {
"@img/colour": "^1.1.0",
@@ -11077,31 +11077,31 @@
"url": "https://opencollective.com/libvips"
},
"optionalDependencies": {
"@img/sharp-darwin-arm64": "0.35.4",
"@img/sharp-darwin-x64": "0.35.4",
"@img/sharp-freebsd-wasm32": "0.35.4",
"@img/sharp-libvips-darwin-arm64": "1.3.3",
"@img/sharp-libvips-darwin-x64": "1.3.3",
"@img/sharp-libvips-linux-arm": "1.3.3",
"@img/sharp-libvips-linux-arm64": "1.3.3",
"@img/sharp-libvips-linux-ppc64": "1.3.3",
"@img/sharp-libvips-linux-riscv64": "1.3.3",
"@img/sharp-libvips-linux-s390x": "1.3.3",
"@img/sharp-libvips-linux-x64": "1.3.3",
"@img/sharp-libvips-linuxmusl-arm64": "1.3.3",
"@img/sharp-libvips-linuxmusl-x64": "1.3.3",
"@img/sharp-linux-arm": "0.35.4",
"@img/sharp-linux-arm64": "0.35.4",
"@img/sharp-linux-ppc64": "0.35.4",
"@img/sharp-linux-riscv64": "0.35.4",
"@img/sharp-linux-s390x": "0.35.4",
"@img/sharp-linux-x64": "0.35.4",
"@img/sharp-linuxmusl-arm64": "0.35.4",
"@img/sharp-linuxmusl-x64": "0.35.4",
"@img/sharp-webcontainers-wasm32": "0.35.4",
"@img/sharp-win32-arm64": "0.35.4",
"@img/sharp-win32-ia32": "0.35.4",
"@img/sharp-win32-x64": "0.35.4"
"@img/sharp-darwin-arm64": "0.35.3",
"@img/sharp-darwin-x64": "0.35.3",
"@img/sharp-freebsd-wasm32": "0.35.3",
"@img/sharp-libvips-darwin-arm64": "1.3.2",
"@img/sharp-libvips-darwin-x64": "1.3.2",
"@img/sharp-libvips-linux-arm": "1.3.2",
"@img/sharp-libvips-linux-arm64": "1.3.2",
"@img/sharp-libvips-linux-ppc64": "1.3.2",
"@img/sharp-libvips-linux-riscv64": "1.3.2",
"@img/sharp-libvips-linux-s390x": "1.3.2",
"@img/sharp-libvips-linux-x64": "1.3.2",
"@img/sharp-libvips-linuxmusl-arm64": "1.3.2",
"@img/sharp-libvips-linuxmusl-x64": "1.3.2",
"@img/sharp-linux-arm": "0.35.3",
"@img/sharp-linux-arm64": "0.35.3",
"@img/sharp-linux-ppc64": "0.35.3",
"@img/sharp-linux-riscv64": "0.35.3",
"@img/sharp-linux-s390x": "0.35.3",
"@img/sharp-linux-x64": "0.35.3",
"@img/sharp-linuxmusl-arm64": "0.35.3",
"@img/sharp-linuxmusl-x64": "0.35.3",
"@img/sharp-webcontainers-wasm32": "0.35.3",
"@img/sharp-win32-arm64": "0.35.3",
"@img/sharp-win32-ia32": "0.35.3",
"@img/sharp-win32-x64": "0.35.3"
},
"peerDependenciesMeta": {
"@types/node": {
+3 -3
View File
@@ -1,6 +1,6 @@
{
"name": "picpeak-backend",
"version": "3.46.12",
"version": "3.46.10",
"description": "Backend for PicPeak event photo sharing platform",
"main": "server.js",
"engines": {
@@ -46,7 +46,7 @@
"knex": "^2.4.2",
"mailparser": "^3.9.9",
"mime-types": "^3.0.1",
"multer": "2.3.0",
"multer": "2.2.0",
"node-cron": "^3.0.2",
"node-stream-zip": "^1.15.0",
"nodemailer": "^9.0.1",
@@ -59,7 +59,7 @@
"qrcode": "^1.5.4",
"react-i18next": "^15.6.0",
"sanitize-html": "2.17.7",
"sharp": "0.35.4",
"sharp": "0.35.3",
"sqlite3": "^5.1.6",
"swagger-jsdoc": "^6.2.8",
"swagger-ui-express": "^5.0.1",
-1
View File
@@ -36,7 +36,6 @@ const MFA_CLEAR = {
two_factor_secret: null,
two_factor_recovery_codes: null,
two_factor_enrolled_at: null,
two_factor_last_used_step: null,
updated_at: new Date(),
};
@@ -38,10 +38,6 @@ jest.mock('../utils/tokenUtils', () => ({
getGalleryTokenFromRequest: jest.fn(),
}));
jest.mock('../utils/tokenRevocation', () => ({
isTokenRevoked: jest.fn().mockResolvedValue(false),
}));
jest.mock('../utils/dbCompat', () => ({
formatBoolean: (v) => (v ? 1 : 0),
}));
@@ -49,8 +45,7 @@ jest.mock('../utils/dbCompat', () => ({
const jwt = require('jsonwebtoken');
const { db } = require('../database/db');
const { getGalleryTokenFromRequest } = require('../utils/tokenUtils');
const { isTokenRevoked } = require('../utils/tokenRevocation');
const { verifyGalleryAccess, previewClaimed, verifyAdminPreview } = require('../middleware/gallery');
const { verifyGalleryAccess } = require('../middleware/gallery');
function makeRes() {
const res = {};
@@ -98,155 +93,6 @@ beforeEach(() => {
db.mockReset();
jwt.verify.mockReset();
getGalleryTokenFromRequest.mockReset();
isTokenRevoked.mockReset();
isTokenRevoked.mockResolvedValue(false);
});
// ---- revoked gallery token (GHSA-q7f7-gjx8-mf6h) -----------------------
describe('verifyGalleryAccess — revoked token', () => {
it('returns 401 TOKEN_REVOKED and never reaches the events query when revoked', async () => {
getGalleryTokenFromRequest.mockReturnValue('tkn');
jwt.verify.mockReturnValue({ type: 'gallery', eventId: 42 });
isTokenRevoked.mockResolvedValue(true);
const req = makeReq();
const res = makeRes();
const next = jest.fn();
await verifyGalleryAccess(req, res, next);
expect(next).not.toHaveBeenCalled();
expect(res.status).toHaveBeenCalledWith(401);
expect(res.json).toHaveBeenCalledWith(
expect.objectContaining({ code: 'TOKEN_REVOKED' }),
);
expect(db).not.toHaveBeenCalled();
});
it('proceeds normally when the token is not revoked', async () => {
getGalleryTokenFromRequest.mockReturnValue('tkn');
jwt.verify.mockReturnValue({ type: 'gallery', eventId: 42 });
isTokenRevoked.mockResolvedValue(false);
const eventsChain = {};
eventsChain.where = jest.fn().mockReturnValue(eventsChain);
eventsChain.select = jest.fn().mockReturnValue(eventsChain);
eventsChain.first = jest.fn().mockResolvedValue({
id: 42, slug: 'test-event', is_active: true, is_archived: false,
});
db.mockImplementationOnce(() => eventsChain);
const req = makeReq();
const res = makeRes();
const next = jest.fn();
await verifyGalleryAccess(req, res, next);
expect(isTokenRevoked).toHaveBeenCalledWith(
expect.objectContaining({ type: 'gallery', eventId: 42 }),
);
expect(next).toHaveBeenCalledTimes(1);
expect(res.status).not.toHaveBeenCalled();
});
});
// ---- revoked admin-preview token --------------------------------------
//
// The preview credential is decoded independently of the main gallery-token
// flow above, and once never checked isTokenRevoked — a revoked admin session
// kept granting preview access through a bookmarked or shared link
// indefinitely (same gap as GHSA-q7f7-gjx8-mf6h, in a sibling path).
//
// That check now lives in verifyAdminPreview rather than in the predicate the
// event lookup is shaped with. previewClaimed stays deliberately cheap and
// signature-only — it decides whether drafts are INCLUDED in the query, never
// whether they are served — and every lookup it shapes is gated behind
// verifyAdminPreview before anything reaches the caller. So a revoked token
// can still widen a query and still cannot preview anything.
describe('previewClaimed — signature only, by design', () => {
it('accepts a syntactically valid admin token without consulting revocation', () => {
jwt.verify.mockReturnValue({ type: 'admin', id: 1 });
isTokenRevoked.mockResolvedValue(true);
expect(previewClaimed({ query: { preview: 'revoked-admin-jwt' } })).toBe(true);
// Deliberately NOT consulted here: this predicate is synchronous and only
// shapes the lookup. Authorization happens in verifyAdminPreview.
expect(isTokenRevoked).not.toHaveBeenCalled();
});
it('rejects a non-admin token', () => {
jwt.verify.mockReturnValue({ type: 'gallery', eventId: 42 });
expect(previewClaimed({ query: { preview: 'not-an-admin-jwt' } })).toBe(false);
});
it('rejects a request carrying no preview credential at all', () => {
expect(previewClaimed({ query: {} })).toBe(false);
});
});
describe('verifyAdminPreview — token revocation', () => {
it('refuses a revoked admin token', async () => {
jwt.verify.mockReturnValue({ type: 'admin', id: 1 });
isTokenRevoked.mockResolvedValue(true);
const result = await verifyAdminPreview(
{ query: { preview: 'revoked-admin-jwt' }, headers: {} },
{ id: 42, created_by: 1 },
);
expect(result).toBe(false);
expect(isTokenRevoked).toHaveBeenCalledWith(
expect.objectContaining({ type: 'admin' }),
);
});
it('fails closed when the revocation store cannot be read', async () => {
jwt.verify.mockReturnValue({ type: 'admin', id: 1 });
isTokenRevoked.mockRejectedValue(new Error('db down'));
const result = await verifyAdminPreview(
{ query: { preview: 'valid-admin-jwt' }, headers: {} },
{ id: 42, created_by: 1 },
);
// A transient fault must not become a free preview.
expect(result).toBe(false);
});
it('refuses when there is no event to authorize against', async () => {
jwt.verify.mockReturnValue({ type: 'admin', id: 1 });
isTokenRevoked.mockResolvedValue(false);
expect(await verifyAdminPreview({ query: { preview: 'jwt' }, headers: {} }, null)).toBe(false);
});
});
describe('verifyGalleryAccess — a revoked preview token cannot open a draft', () => {
it('answers 404 for the draft instead of granting access', async () => {
getGalleryTokenFromRequest.mockReturnValue(undefined); // no gallery-scoped token
jwt.verify.mockReturnValue({ type: 'admin', id: 1 }); // decoded preview token
isTokenRevoked.mockResolvedValue(true);
const eventsChain = {};
eventsChain.where = jest.fn().mockReturnValue(eventsChain);
eventsChain.select = jest.fn().mockReturnValue(eventsChain);
eventsChain.first = jest.fn().mockResolvedValue({
id: 42, slug: 'test-event', is_active: true, is_archived: false,
is_draft: true, require_password: false,
});
db.mockImplementation(() => eventsChain);
const req = makeReq();
req.query = { preview: 'revoked-admin-jwt' };
const res = makeRes();
const next = jest.fn();
await verifyGalleryAccess(req, res, next);
// The lookup was widened (previewClaimed is signature-only), but the draft
// is refused at the gate — which is the contract that actually matters.
expect(next).not.toHaveBeenCalled();
expect(res.status).toHaveBeenCalledWith(404);
});
});
// ---- customer-minted JWT, assignment intact ----------------------------
+4 -14
View File
@@ -120,14 +120,7 @@ const createPhotoUploader = (options = {}) => {
files: options.maxFiles || 2000,
fieldSize: 10 * 1024 * 1024,
parts: 10000,
headerPairs: 2000,
// CVE-2026-82333: no preset in this factory is currently wired up to
// a route (nothing imports createPhotoUploader et al. — routes build
// their own multer instances directly), but every preset gets the
// limit anyway so it can't be adopted later without it. None of the
// uploaders this factory builds have a legitimate use for
// array-indexed field names.
fieldArrayIndexLimit: 0
headerPairs: 2000
},
fileFilter: createFileFilter(ALLOWED_TYPES.media, {
validateMagicNumbers: true
@@ -153,8 +146,7 @@ const createLogoUploader = (options = {}) => {
}
}),
limits: {
fileSize: options.maxSize || SIZE_LIMITS.medium,
fieldArrayIndexLimit: 0 // CVE-2026-82333 — see createPhotoUploader comment
fileSize: options.maxSize || SIZE_LIMITS.medium
},
fileFilter: createFileFilter(ALLOWED_TYPES.logos, {
skipMagicValidation: ['image/svg+xml']
@@ -180,8 +172,7 @@ const createFaviconUploader = (options = {}) => {
}
}),
limits: {
fileSize: options.maxSize || SIZE_LIMITS.small,
fieldArrayIndexLimit: 0 // CVE-2026-82333 — see createPhotoUploader comment
fileSize: options.maxSize || SIZE_LIMITS.small
},
fileFilter: createFileFilter(ALLOWED_TYPES.favicons, {
skipMagicValidation: ['image/x-icon', 'image/vnd.microsoft.icon']
@@ -203,8 +194,7 @@ const createGalleryUploader = (destDir, options = {}) => {
dest: destDir,
limits: {
fileSize: options.maxSize || SIZE_LIMITS.large,
files: options.maxFiles || 10,
fieldArrayIndexLimit: 0 // CVE-2026-82333 — see createPhotoUploader comment
files: options.maxFiles || 10
},
fileFilter: createFileFilter(ALLOWED_TYPES.photos)
};
+2 -24
View File
@@ -6,18 +6,6 @@ const { isTokenRevoked } = require('../utils/tokenRevocation');
const logger = require('../utils/logger');
const { getAdminTokenFromRequest } = require('../utils/tokenUtils');
// GHSA-h4w8-57xq-53fx: must_change_password was written on reset (and on
// invitation paths) but nothing server-side ever checked it — a forced-reset
// admin could keep using the old/weak password indefinitely because the flag
// only ever reached the frontend as a response field. This is the backstop
// for callers that skip the UI entirely. Every route gated by adminAuth() is
// blocked except the ones a flagged admin needs to clear the flag or leave:
// change their password, and log out.
const MUST_CHANGE_PASSWORD_EXEMPT_PATHS = new Set([
'/api/admin/auth/change-password',
'/api/admin/auth/logout',
]);
/**
* Enhanced admin authentication middleware with revocation checking
*/
@@ -83,7 +71,6 @@ async function adminAuth(req, res, next) {
'admin_users.username',
'admin_users.email',
'admin_users.password_changed_at',
'admin_users.must_change_password',
'roles.id as role_id',
'roles.name as role_name'
)
@@ -102,7 +89,7 @@ async function adminAuth(req, res, next) {
logger.debug('Roles table not available, falling back to basic auth', { error: joinError.message });
admin = await db('admin_users')
.where({ id: decoded.id, is_active: formatBoolean(true) })
.select('id', 'username', 'email', 'password_changed_at', 'must_change_password')
.select('id', 'username', 'email', 'password_changed_at')
.first();
if (admin) {
admin.role_id = null;
@@ -132,22 +119,13 @@ async function adminAuth(req, res, next) {
}
}
if (admin.must_change_password
&& !MUST_CHANGE_PASSWORD_EXEMPT_PATHS.has(req.originalUrl.split('?')[0])) {
return res.status(403).json({
error: 'Password change required before continuing',
code: 'MUST_CHANGE_PASSWORD'
});
}
// Add user info to request (enhanced with role)
req.admin = {
id: admin.id,
username: admin.username,
email: admin.email,
roleId: admin.role_id,
roleName: admin.role_name,
mustChangePassword: !!admin.must_change_password
roleName: admin.role_name
};
req.token = token; // Store token for potential revocation
-10
View File
@@ -92,16 +92,6 @@ const handleKnownErrors = (err) => {
return new ValidationError('Unexpected file field');
}
// CVE-2026-82333: multer 2.3.0's fieldArrayIndexLimit rejects multipart
// field names with an oversized bracket array index (e.g. `a[99999999]`)
// before the DoS-prone field parser runs. Without this mapping the
// resulting MulterError has no .statusCode/.status and falls through to
// a 500 here, so map it to a proper 400 like the other multer limits.
if (err.code === 'LIMIT_FIELD_ARRAY_INDEX') {
const { ValidationError } = require('../utils/errors');
return new ValidationError('Field name array index too large');
}
return err;
};
+10 -148
View File
@@ -2,134 +2,18 @@ const jwt = require('jsonwebtoken');
const { db, withRetry } = require('../database/db');
const { formatBoolean } = require('../utils/dbCompat');
const { getGalleryTokenFromRequest } = require('../utils/tokenUtils');
const { userHasAllPermissions } = require('./permissions');
const { isTokenRevoked } = require('../utils/tokenRevocation');
const logger = require('../utils/logger');
// Admin preview of an unpublished gallery.
//
// Two transports (#1386):
//
// admin_preview=1 — an INTENT flag, authenticated by the admin's existing
// HttpOnly admin_token cookie (or an Authorization
// bearer). This is the one the frontend uses. The cookie
// rides along on same-origin requests automatically,
// including the native fetch() that AuthenticatedImage
// uses, so media works too — and no credential ever
// appears in a URL.
//
// preview=<jwt> — the original transport, kept so existing hand-built
// links keep working. It puts an admin JWT in the query
// string, which reaches nginx access logs, browser
// history and Referer headers, so nothing emits it any
// more.
function previewTokenFrom(req) {
if (req.query?.admin_preview === '1') {
const header = req.headers?.authorization;
const bearer = header && header.startsWith('Bearer ') ? header.substring(7) : null;
const candidate = req.cookies?.admin_token || bearer;
if (candidate) return candidate;
}
return req.query?.preview || null;
}
function decodeAdminToken(token) {
if (!token) return null;
// Check if the request carries a valid admin preview token (Feature 3)
function isAdminPreview(req) {
const previewToken = req.query?.preview;
if (!previewToken) return false;
try {
const decoded = jwt.verify(token, process.env.JWT_SECRET, {
algorithms: ['HS256'], issuer: 'picpeak-auth',
});
return decoded.type === 'admin' ? decoded : null;
const decoded = jwt.verify(previewToken, process.env.JWT_SECRET, { issuer: 'picpeak-auth' });
return decoded.type === 'admin';
} catch {
return null;
}
}
/**
* Signature-only predicate. It proves the caller holds SOME valid admin token
* and nothing else — not that the account still exists, not that the token is
* unrevoked, and not that this admin may see this event.
*
* Its only legitimate use is shaping the event lookup, which has to decide
* whether to include drafts BEFORE there is an event to authorize against.
* Every such lookup must be followed by assertDraftPreviewAllowed (#1411).
*/
function previewClaimed(req) {
return decodeAdminToken(previewTokenFrom(req)) !== null;
}
/**
* Full authorization for previewing a specific event (#1411).
*
* The signature check above used to be the whole story, so any valid admin
* token previewed any draft — including one created by a different admin, and
* including an account whose role grants neither events.view nor photos.view.
* `main` closes this via access.authorize; this is the same rule applied where
* this branch keeps its checks.
*/
async function verifyAdminPreview(req, event) {
const decoded = decodeAdminToken(previewTokenFrom(req));
if (!decoded || !event) return false;
// A signed-out or rotated session must stop previewing, same as it stops
// reaching every other admin surface. This is the check isAdminPreview
// carried for GHSA-q7f7-gjx8-mf6h — a revoked admin session must not keep
// granting preview access through a bookmarked or shared link — kept here,
// at the point where preview is actually authorized rather than where the
// event lookup is merely shaped.
try {
if (await isTokenRevoked(decoded)) return false;
} catch (error) {
// Fail closed: a transient DB fault must not become a free preview.
logger.warn('Admin preview revocation check failed', { error: error.message });
return false;
}
let admin;
try {
admin = await db('admin_users')
.leftJoin('roles', 'roles.id', 'admin_users.role_id')
.where({ 'admin_users.id': decoded.id, 'admin_users.is_active': formatBoolean(true) })
.select('admin_users.id', 'roles.name as role_name')
.first();
} catch (error) {
// Same posture as adminAuth's join fallback: an install whose roles table
// predates the schema still has admins, but it has no role to check, so
// ownership below is the only gate that applies.
logger.debug('Admin preview role lookup failed', { error: error.message });
admin = await db('admin_users')
.where({ id: decoded.id, is_active: formatBoolean(true) })
.select('id').first();
if (admin) admin.role_name = null;
}
if (!admin) return false;
// Ownership: super_admin sees everything, everyone else sees ownerless
// (legacy/system) events plus their own — the rule requireEventOwnership
// and scopeEventsQuery already enforce elsewhere.
const owns = admin.role_name === 'super_admin'
|| !event.created_by
|| Number(event.created_by) === Number(admin.id);
if (!owns) return false;
try {
return await userHasAllPermissions(admin.id, ['events.view', 'photos.view']);
} catch (error) {
logger.warn('Admin preview permission check failed', { error: error.message });
return false;
}
}
/**
* Gate a loaded event behind the preview rules. Published events pass through
* untouched; a draft is visible only to an authorized admin preview. Returns
* false when the caller must be told the gallery does not exist.
*/
async function assertDraftPreviewAllowed(req, event) {
if (!event) return true;
const isDraft = event.is_draft === true || event.is_draft === 1 || event.is_draft === '1';
if (!isDraft) return true;
return verifyAdminPreview(req, event);
}
// Middleware to verify gallery access
@@ -144,7 +28,7 @@ async function verifyGalleryAccess(req, res, next) {
return res.status(401).json({ error: 'No token provided' });
}
const adminPreview = previewClaimed(req);
const adminPreview = isAdminPreview(req);
event = await withRetry(async () => {
const q = db('events')
.where({
@@ -162,12 +46,6 @@ async function verifyGalleryAccess(req, res, next) {
return res.status(404).json({ error: 'Gallery not found or expired' });
}
// The lookup above included drafts on a signature-only check. Authorize
// the draft now that there is an event to authorize against (#1411).
if (!await assertDraftPreviewAllowed(req, event)) {
return res.status(404).json({ error: 'Gallery not found or expired' });
}
const requiresPassword = !(event.require_password === false || event.require_password === 0 || event.require_password === '0');
if (!requiresPassword) {
req.event = event;
@@ -211,18 +89,10 @@ async function verifyGalleryAccess(req, res, next) {
return res.status(403).json({ error: 'Invalid token type for gallery access' });
}
// Gallery logout writes to the revocation store (see routes/auth.js),
// but nothing on this path ever read it back (GHSA-q7f7-gjx8-mf6h) — a
// logged-out gallery JWT kept working until natural expiry.
if (await isTokenRevoked(decoded)) {
logger.warn('[verifyGalleryAccess] Revoked token used', { eventId: decoded.eventId });
return res.status(401).json({ error: 'Token has been revoked', code: 'TOKEN_REVOKED' });
}
// If we have a slug in the URL params or from pre-middleware, verify it matches
if (requestedSlug) {
// Verify by slug and ensure it matches the token's event
const adminPreviewToken = previewClaimed(req);
const adminPreviewToken = isAdminPreview(req);
event = await withRetry(async () => {
const q = db('events')
.where({
@@ -242,7 +112,7 @@ async function verifyGalleryAccess(req, res, next) {
}
} else {
// Fallback to using eventId from token
const adminPreviewFallback = previewClaimed(req);
const adminPreviewFallback = isAdminPreview(req);
event = await withRetry(async () => {
const q = db('events')
.where({
@@ -262,12 +132,6 @@ async function verifyGalleryAccess(req, res, next) {
return res.status(404).json({ error: 'Gallery not found or expired' });
}
// Same gate as the public branch above (#1411): the draft was included in
// the lookup on a signature-only check and has to be authorized here.
if (!await assertDraftPreviewAllowed(req, event)) {
return res.status(404).json({ error: 'Gallery not found or expired' });
}
// Customer-minted gallery JWTs (#354): when the customer obtained
// this token via /api/customer/events/:slug/access-token, the
// payload carries `via:'customer'` and `customerId`. The admin
@@ -338,7 +202,5 @@ function denySlideshowToken(req, res, next) {
module.exports = {
verifyGalleryAccess,
denySlideshowToken,
previewClaimed,
verifyAdminPreview,
assertDraftPreviewAllowed
isAdminPreview
};
+2 -27
View File
@@ -254,11 +254,6 @@ router.post('/mfa/setup', adminAuth, handleAsync(async (req, res) => {
// Complete enrollment: verify a code against the provisional secret, enable
// MFA, and return one-time recovery codes (shown exactly once).
//
// No replay tracking here: this confirms an already-authenticated session
// still holds the authenticator (no new session is granted), and starting
// the last-used-step counter here would reject the very next login if it
// lands in the same 30s TOTP step as this call.
router.post('/mfa/enable', [
adminAuth,
body('code').notEmpty().withMessage('Verification code is required')
@@ -309,17 +304,7 @@ router.post('/mfa/disable', [
throw new ValidationError('Two-factor authentication is not enabled');
}
// Persist the matched step atomically right here (see mfaService.persistTotpStep):
// two concurrent requests carrying the same captured code can't both read the
// same last-used step and both win — only the first writer's UPDATE affects a
// row, so a losing concurrent request is correctly treated as invalid below.
const totpStep = mfaService.verifyTotpEncryptedStep(
req.body.code, admin.two_factor_secret, admin.two_factor_last_used_step
);
let totpOk = false;
if (totpStep !== null) {
totpOk = await mfaService.persistTotpStep(db, admin.id, totpStep, { updated_at: new Date() });
}
const totpOk = mfaService.verifyTotpEncrypted(req.body.code, admin.two_factor_secret);
let recoveryOk = false;
if (!totpOk) {
const stored = mfaService.parseRecoveryCodes(admin.two_factor_recovery_codes);
@@ -334,7 +319,6 @@ router.post('/mfa/disable', [
two_factor_secret: null,
two_factor_recovery_codes: null,
two_factor_enrolled_at: null,
two_factor_last_used_step: null,
updated_at: new Date()
});
@@ -359,16 +343,7 @@ router.post('/mfa/recovery-codes', [
if (!isMfaEnabled(admin)) {
throw new ValidationError('Two-factor authentication is not enabled');
}
// Persist the matched step atomically right here (see mfaService.persistTotpStep):
// two concurrent requests carrying the same captured code can't both read the
// same last-used step and both win — only the first writer's UPDATE affects a
// row, so a losing concurrent request is correctly treated as invalid below.
const totpStep = mfaService.verifyTotpEncryptedStep(
req.body.code, admin.two_factor_secret, admin.two_factor_last_used_step
);
const totpOk = totpStep !== null
&& await mfaService.persistTotpStep(db, admin.id, totpStep, { updated_at: new Date() });
if (!totpOk) {
if (!mfaService.verifyTotpEncrypted(req.body.code, admin.two_factor_secret)) {
throw new ValidationError('Invalid verification code');
}
+1 -5
View File
@@ -191,11 +191,7 @@ const picpeakUpload = multer({
destination: (req, file, cb) => cb(null, os.tmpdir()),
filename: (req, file, cb) => cb(null, `picpeak-upload-${Date.now()}-${crypto.randomBytes(6).toString('hex')}.picpeak`),
}),
// CVE-2026-82333: this route only ever consumes a single unnamed file
// field (`backup`) — no legitimate bracket-indexed field name (e.g.
// `a[0]`) exists in its form. fieldArrayIndexLimit: 0 rejects any field
// name using array-index syntax at all, closing multer's field-parser DoS.
limits: { fileSize: 5 * 1024 * 1024 * 1024, fieldArrayIndexLimit: 0 }, // 5 GB — .picpeak with photos can be large
limits: { fileSize: 5 * 1024 * 1024 * 1024 }, // 5 GB — .picpeak with photos can be large
});
// Upload + restore a .picpeak onto THIS instance. DESTRUCTIVE: full override of
+6 -53
View File
@@ -23,7 +23,6 @@ const { requirePermission } = require('../middleware/permissions');
const { handleAsync, validateRequest, successResponse } = require('../utils/routeHelpers');
const { getStoragePath } = require('../config/storage');
const { uploadedPdfLogoPath } = require('../utils/safePath');
const { validateFileType, validateFileContent, ALLOWED_MEDIA_TYPES } = require('../utils/fileSecurityUtils');
const businessProfileService = require('../services/businessProfileService');
const { db } = require('../database/db');
const { validateIban } = require('../utils/iban');
@@ -96,19 +95,6 @@ const router = express.Router();
// but accepts SVG in addition to PNG / JPEG — the PDF renderer
// rasterises SVGs to PNG on the fly via resolveLogoFile() so the
// admin can drop a vector logo here and have it work in print.
//
// GHSA-6wrv-9pr4-hhmw: this route used to take the stored extension
// straight from `file.originalname` and only checked `file.mimetype`
// against an allowlist — a file could declare an image MIME type
// while carrying a `.html`/`.js` extension and arbitrary content, get
// served same-origin from /uploads/logos with that extension, and
// execute as script in the browser. Fixed the same way every sibling
// upload route (adminSettings.js, adminCMS.js) already does it:
// `validateFileType()` pairs the claimed MIME type against the
// extension, and the extension actually written to disk is looked up
// from the validated MIME type — never taken from client input.
const PDF_LOGO_ALLOWED_MIME_TYPES = ['image/png', 'image/jpeg', 'image/svg+xml'];
const pdfLogoStorage = multer.diskStorage({
destination: async (_req, _file, cb) => {
const dir = path.join(getStoragePath(), 'uploads/logos');
@@ -116,26 +102,18 @@ const pdfLogoStorage = multer.diskStorage({
cb(null, dir);
},
filename: (_req, file, cb) => {
// fileFilter (below) runs before this and already rejected any
// mimetype outside PDF_LOGO_ALLOWED_MIME_TYPES, so the lookup below
// always hits. The extension is derived from the validated MIME
// type, never from file.originalname.
const ext = ALLOWED_MEDIA_TYPES[file.mimetype]?.extensions[0] || '.png';
const ext = path.extname(file.originalname) || '.png';
cb(null, `pdf-logo-${Date.now()}${ext}`);
},
});
const pdfLogoUpload = multer({
storage: pdfLogoStorage,
// CVE-2026-82333: single unnamed `logo` field only — no legitimate
// array-indexed field names, so reject any bracket-index field name.
limits: { fileSize: 5 * 1024 * 1024, fieldArrayIndexLimit: 0 },
limits: { fileSize: 5 * 1024 * 1024 },
fileFilter: (_req, file, cb) => {
if (validateFileType(file.originalname, file.mimetype, PDF_LOGO_ALLOWED_MIME_TYPES)) {
cb(null, true);
} else {
cb(new Error('Only PNG, JPEG and SVG logos are allowed'));
}
const allowed = ['image/png', 'image/jpeg', 'image/svg+xml'];
if (allowed.includes(file.mimetype)) cb(null, true);
else cb(new Error('Only PNG, JPEG and SVG logos are allowed'));
},
});
@@ -375,19 +353,6 @@ router.post(
return res.status(400).json({ error: 'No logo file uploaded' });
}
// fileFilter above only pairs the claimed MIME type against the
// extension — it runs on the in-flight stream, before any bytes are
// written, so it can't inspect content. Content-sniff the bytes multer
// just wrote to disk (magic numbers) before trusting them; SVG has no
// magic-number check (validateFileContent returns true for it), it's
// protected by the CSP header instead. Matches the cleanup-then-reject
// pattern createFileUploadValidator() uses for other upload routes.
const contentIsValid = await validateFileContent(req.file.path, req.file.mimetype);
if (!contentIsValid) {
try { await fs.unlink(req.file.path); } catch (_) { /* ignore */ }
return res.status(400).json({ error: 'File content does not match its declared type' });
}
// Clean up the previous PDF logo on disk if it was uploaded via
// this same endpoint (matches the pdf-logo-* prefix). We leave
// anything else untouched — the admin may have set logo_path to
@@ -461,19 +426,7 @@ router.put(
body('defaultLocale').optional({ values: 'falsy' }).isString().isLength({ max: 8 }),
body('defaultQrFormat').optional({ values: 'falsy' }).isIn(['swiss', 'epc', 'none']),
body('footerLine').optional({ values: 'falsy' }).isString().isLength({ max: 255 }),
// GHSA-6wrv-9pr4-hhmw: logoPath is mass-assignable here, so it must
// only ever be settable to a path the POST /logo upload route itself
// produced (or '' to clear it, allowed by `values: 'falsy'` above) —
// not an arbitrary string chaining in a file uploaded elsewhere.
// uploadedPdfLogoPath() is the same pattern check the delete/replace
// cleanup path already trusts to name a file this route wrote.
body('logoPath').optional({ values: 'falsy' }).isString().isLength({ max: 512 })
.custom((value) => {
if (!uploadedPdfLogoPath(value, getStoragePath())) {
throw new Error('logoPath must be a path produced by the logo upload endpoint');
}
return true;
}),
body('logoPath').optional({ values: 'falsy' }).isString().isLength({ max: 512 }),
// Bundled-fonts dropdown (migration 121). Free-text upload field
// (pdfFontTtfPath, migration 103) was retired from the UI in
// favour of this dropdown; the column stays in the DB so any
+1 -3
View File
@@ -31,9 +31,7 @@ const pageLogoStorage = multer.diskStorage({
const pageLogoUpload = multer({
storage: pageLogoStorage,
// CVE-2026-82333: single unnamed `logo` field only — no legitimate
// array-indexed field names, so reject any bracket-index field name.
limits: { fileSize: 5 * 1024 * 1024, fieldArrayIndexLimit: 0 },
limits: { fileSize: 5 * 1024 * 1024 },
fileFilter: (_req, file, cb) => {
const allowed = ['image/jpeg', 'image/png', 'image/gif', 'image/svg+xml'];
if (validateFileType(file.originalname, file.mimetype, allowed)) cb(null, true);
+2 -8
View File
@@ -65,20 +65,14 @@ const signedPdfStorage = multer.diskStorage({
cb(null, uploadDir);
},
filename: (req, file, cb) => {
const contractId = Number(req.params.id);
if (!Number.isInteger(contractId) || contractId <= 0) {
return cb(new Error('Invalid contract id'));
}
const ext = path.extname(file.originalname) || '.pdf';
cb(null, `contract-${contractId}-${Date.now()}${ext}`);
cb(null, `contract-${req.params.id}-${Date.now()}${ext}`);
},
});
const signedPdfUpload = multer({
storage: signedPdfStorage,
// CVE-2026-82333: single unnamed `file` field only — no legitimate
// array-indexed field names, so reject any bracket-index field name.
limits: { fileSize: 10 * 1024 * 1024, fieldArrayIndexLimit: 0 }, // 10 MB
limits: { fileSize: 10 * 1024 * 1024 }, // 10 MB
fileFilter: (req, file, cb) => {
const allowed = ['application/pdf'];
if (validateFileType(file.originalname, file.mimetype, allowed)) return cb(null, true);
+2 -23
View File
@@ -2,7 +2,7 @@ const express = require('express');
const router = express.Router();
const { adminAuth } = require('../middleware/auth');
const { requirePermission } = require('../middleware/permissions');
const { databaseBackupService, isUnderPubliclyServableRoot } = require('../services/databaseBackup');
const { databaseBackupService } = require('../services/databaseBackup');
const { db } = require('../database/db');
const logger = require('../utils/logger');
const { getPagination } = require('../utils/routeHelpers');
@@ -60,28 +60,7 @@ router.put('/config', requirePermission('backup.create'), async (req, res) => {
'database_backup_email_on_failure',
'database_backup_email_on_success'
];
// A backup.create holder (the built-in `admin` role has it without
// settings.edit or backup.restore) could otherwise point backups at a
// public static mount and fetch the dump unauthenticated — see
// isUnderPubliclyServableRoot's comment (GHSA-jw8m-43r2-jqrm class).
if (
typeof req.body.database_backup_destination_path === 'string'
&& isUnderPubliclyServableRoot(req.body.database_backup_destination_path)
) {
return res.status(400).json({ error: 'Destination path must not be inside a publicly served directory' });
}
// A retention of 0 or less pushes cleanupOldBackups' cutoff to today or
// the future, deleting every completed backup on the next scheduled run
// — a backup.create holder achieving what backup.delete gates on /cleanup.
if (
req.body.database_backup_retention_days !== undefined
&& (!Number.isFinite(req.body.database_backup_retention_days) || req.body.database_backup_retention_days < 1)
) {
return res.status(400).json({ error: 'database_backup_retention_days must be a positive number' });
}
const updates = [];
for (const [key, value] of Object.entries(req.body)) {
+2 -8
View File
@@ -23,20 +23,14 @@ const eventLogoStorage = multer.diskStorage({
cb(null, uploadDir);
},
filename: (req, file, cb) => {
const eventId = Number(req.params.id);
if (!Number.isInteger(eventId) || eventId <= 0) {
return cb(new Error('Invalid event id'));
}
const ext = path.extname(file.originalname);
cb(null, `event-${eventId}-logo-${Date.now()}${ext}`);
cb(null, `event-${req.params.id}-logo-${Date.now()}${ext}`);
}
});
const eventLogoUpload = multer({
storage: eventLogoStorage,
// CVE-2026-82333: single unnamed `logo` field only — no legitimate
// array-indexed field names, so reject any bracket-index field name.
limits: { fileSize: 5 * 1024 * 1024, fieldArrayIndexLimit: 0 }, // 5MB
limits: { fileSize: 5 * 1024 * 1024 }, // 5MB
fileFilter: (req, file, cb) => {
const allowedMimeTypes = ['image/jpeg', 'image/png', 'image/gif', 'image/svg+xml'];
if (validateFileType(file.originalname, file.mimetype, allowedMimeTypes)) {
+1 -4
View File
@@ -41,10 +41,7 @@ function diskUpload(subdir) {
},
filename: (_req, file, cb) => cb(null, `${subdir.split('/').pop()}-${Date.now()}${path.extname(file.originalname) || ''}`),
}),
// CVE-2026-82333: both callers (`inboundUpload` → 'file', `proofUpload`
// → 'proof') take a single unnamed field — no legitimate array-indexed
// field names, so reject any bracket-index field name.
limits: { fileSize: 15 * 1024 * 1024, fieldArrayIndexLimit: 0 },
limits: { fileSize: 15 * 1024 * 1024 },
fileFilter: (_req, file, cb) => (ALLOWED_MIME.includes(file.mimetype) ? cb(null, true) : cb(new Error('Only PDF, JPEG or PNG files are allowed'))),
});
}
+1 -3
View File
@@ -67,9 +67,7 @@ const importedInvoiceStorage = multer.diskStorage({
});
const importedInvoiceUpload = multer({
storage: importedInvoiceStorage,
// CVE-2026-82333: single unnamed `pdf` field only — no legitimate
// array-indexed field names, so reject any bracket-index field name.
limits: { fileSize: 10 * 1024 * 1024, fieldArrayIndexLimit: 0 },
limits: { fileSize: 10 * 1024 * 1024 },
fileFilter: (_req, file, cb) => {
if (file.mimetype === 'application/pdf') cb(null, true);
else cb(new Error('Only PDF files are allowed for imported invoices'));
+1 -6
View File
@@ -72,12 +72,7 @@ const upload = multer({
files: 2000, // Hard safety ceiling; actual limit enforced dynamically
fieldSize: 10 * 1024 * 1024, // 10MB for non-file fields
parts: 10000,
headerPairs: 2000,
// CVE-2026-82333: files arrive as repeated `photos` parts via
// .array('photos', N) — not bracket-indexed field names like
// `photos[0]` — so no legitimate field name uses array-index syntax
// at all. Reject any that do.
fieldArrayIndexLimit: 0
headerPairs: 2000
},
fileFilter: (req, file, cb) => {
// req.allowedMimeTypes is populated by the middleware that runs before multer
+6 -54
View File
@@ -810,72 +810,24 @@ async function checkRestorePathsAllowed({ source, manifestPath }) {
if (extra.trim()) roots.push(extra.trim());
}
if (roots.length === 0) {
// GHSA-xfvx: nothing configured to compare against used to mean "a
// restore can't be scoped, so don't pretend to enforce" — returning
// null (allow). That's fail-OPEN: on a fresh install (or one where an
// operator never set backup_destination_path/backup_manifest_path) any
// authenticated `backup.restore` caller could point source/manifestPath
// — and, via the manifest, database.backup_file — at literally any path
// on disk. Require configuration instead of silently allowing
// everything; the normal restore wizard already needs one of these
// settings populated to discover backups in the first place.
logger.warn('Refusing restore: no backup location configured to scope it to', { candidates });
return 'No backup location is configured (backup_destination_path / backup_manifest_path). ' +
'Configure one before restoring.';
// Nothing configured to compare against — a restore can't be scoped, so
// don't pretend to enforce. Discovery would find nothing either.
return null;
}
const resolvedRoots = roots.map((r) => path.resolve(r));
const isInsideRoots = (candidate) => {
for (const candidate of candidates) {
const resolved = path.resolve(candidate);
return resolvedRoots.some(
const inside = resolvedRoots.some(
(root) => resolved === root || resolved.startsWith(root + path.sep)
);
};
for (const candidate of candidates) {
if (!isInsideRoots(candidate)) {
if (!inside) {
logger.warn('Refusing restore path outside the configured backup roots', {
candidate, roots,
});
return 'Backup source and manifest path must be inside a configured backup location';
}
}
// GHSA-xfvx: source/manifestPath containment alone isn't enough — the
// manifest FILE (which just passed containment above) can itself carry a
// `database.backup_file` field that restoreService's candidate resolution
// used to hand straight to `sqlite3 .restore` with no containment check at
// all. Peek at the manifest here (it's already proven to live inside an
// allowed root) and reject an ABSOLUTE backup_file that escapes the same
// roots — the case that's unambiguous to check without re-deriving
// restoreService's own `backupPath` resolution for the relative-path
// candidates. This is deliberately defense in depth, not the only gate:
// restoreService.performDatabaseRestore independently re-derives and
// enforces containment (including relative/`..` candidates) against
// `backupPath` right before ever using the resolved path, and remains the
// authoritative check for S3-sourced manifests (downloaded after this
// pre-check runs).
if (manifestPath && !isS3(manifestPath) && !isTypeToken(manifestPath)) {
try {
const raw = await fs.readFile(manifestPath, 'utf8');
const trimmed = raw.trimStart();
const parsed = (trimmed.startsWith('{') || trimmed.startsWith('['))
? JSON.parse(raw)
: null; // non-JSON (e.g. YAML) manifests are re-checked inside restoreService
const dbBackupFile = parsed?.database?.backup_file;
if (typeof dbBackupFile === 'string' && path.isAbsolute(dbBackupFile) && !isInsideRoots(dbBackupFile)) {
logger.warn('Refusing restore: manifest database.backup_file escapes configured backup roots', {
manifestPath, backupFile: dbBackupFile,
});
return 'Manifest database.backup_file must be inside a configured backup location';
}
} catch (_) {
// Unreadable/corrupt/non-JSON manifest: let the normal restore flow
// surface the real error (loadAndValidateManifest) instead of failing
// this pre-check for an unrelated reason.
}
}
return null;
}
+2 -7
View File
@@ -50,10 +50,7 @@ const { validateFileType } = require('../utils/fileSecurityUtils');
const upload = multer({
storage,
// CVE-2026-82333: single unnamed field (`logo` or `watermarkLogo`) per
// route — no legitimate array-indexed field names, so reject any
// bracket-index field name.
limits: { fileSize: 5 * 1024 * 1024, fieldArrayIndexLimit: 0 }, // 5MB
limits: { fileSize: 5 * 1024 * 1024 }, // 5MB
fileFilter: (req, file, cb) => {
// Note: SVG files are excluded from magic number validation for logos
const allowedMimeTypes = ['image/jpeg', 'image/png', 'image/gif', 'image/svg+xml'];
@@ -81,9 +78,7 @@ const faviconStorage = multer.diskStorage({
const faviconUpload = multer({
storage: faviconStorage,
// CVE-2026-82333: single unnamed `favicon` field only — no legitimate
// array-indexed field names, so reject any bracket-index field name.
limits: { fileSize: 2 * 1024 * 1024, fieldArrayIndexLimit: 0 }, // 2MB — roomy enough for a 512×512+ square PNG
limits: { fileSize: 2 * 1024 * 1024 }, // 2MB — roomy enough for a 512×512+ square PNG
fileFilter: (req, file, cb) => {
const allowedMimeTypes = ['image/png', 'image/x-icon', 'image/vnd.microsoft.icon'];
const name = file.originalname.toLowerCase();
+4 -29
View File
@@ -15,7 +15,6 @@ const { safeValidationErrors } = require('../utils/routeHelpers');
const { adminAuth } = require('../middleware/auth');
const { requirePermission } = require('../middleware/permissions');
const { requireEventOwnership } = require('../middleware/ownership');
const { db } = require('../database/db');
const galleryShortUrlService = require('../services/galleryShortUrlService');
const logger = require('../utils/logger');
@@ -87,31 +86,6 @@ router.post(
},
);
/**
* Ownership guard for the by-short-url-id DELETE route (GHSA-9h7q-2jpf-vj85).
* GET/POST take :eventId directly so requireEventOwnership applies as-is;
* DELETE takes the short URL row's own :id, so resolve its event first and
* apply the same ownership predicate requireEventOwnership uses (event has
* no owner, or the admin owns it). Sends the response and returns false
* when the caller may not act on it (404 if the row doesn't exist, 403 if
* it exists but belongs to another admin).
*/
async function assertOwnsShortUrl(req, res, id) {
const row = await db('gallery_short_urls').where({ id }).first('event_id');
if (!row) {
res.status(404).json({ error: 'Short URL not found' });
return false;
}
if (req.admin.roleName !== 'super_admin') {
const event = await db('events').where({ id: row.event_id }).first('created_by');
if (event && event.created_by && event.created_by !== req.admin.id) {
res.status(403).json({ error: 'Access denied' });
return false;
}
}
return true;
}
/**
* DELETE /api/admin/short-urls/:id
* Soft-delete. The public route serves 410 Gone on a deleted row so the
@@ -125,9 +99,10 @@ router.delete(
const errors = validationResult(req);
if (!errors.isEmpty()) return res.status(400).json({ errors: safeValidationErrors(errors) });
try {
const id = parseInt(req.params.id, 10);
if (!(await assertOwnsShortUrl(req, res, id))) return;
const ok = await galleryShortUrlService.softDelete(id, req.admin?.id || null);
const ok = await galleryShortUrlService.softDelete(
parseInt(req.params.id, 10),
req.admin?.id || null,
);
if (!ok) return res.status(404).json({ error: 'Short URL not found' });
res.status(204).end();
} catch (err) {
+2 -17
View File
@@ -243,22 +243,8 @@ router.post('/admin/login/mfa', [
return res.status(401).json({ error: getGenericAuthError() });
}
// TOTP first, then a one-time recovery code. verifyTotpEncryptedStep also
// enforces replay protection (GHSA-qcwx-r25m-j869): a code whose matched
// step doesn't advance past this admin's two_factor_last_used_step is
// rejected, so the same code can't complete two logins. The step is
// persisted atomically (persistTotpStep) right here, immediately after a
// match, so two concurrent requests carrying the same captured code
// can't both read the same last-used step and both win — only the first
// writer's UPDATE affects a row; the loser falls through and is treated
// as a replay below.
const totpStep = mfaService.verifyTotpEncryptedStep(
code, admin.two_factor_secret, admin.two_factor_last_used_step
);
let ok = false;
if (totpStep !== null) {
ok = await mfaService.persistTotpStep(db, admin.id, totpStep, { updated_at: new Date() });
}
// TOTP first, then a one-time recovery code.
let ok = mfaService.verifyTotpEncrypted(code, admin.two_factor_secret);
let usedRecovery = false;
let remainingHashes = null;
if (!ok) {
@@ -287,7 +273,6 @@ router.post('/admin/login/mfa', [
{ type: 'admin', id: admin.id, name: admin.username }
);
}
// else: the TOTP step was already persisted atomically above.
await logActivity('admin_mfa_login',
{ admin_id: admin.id, method: usedRecovery ? 'recovery_code' : 'totp' },
+10 -118
View File
@@ -26,7 +26,7 @@ function resolveHeroLogoVisible(perEvent, globalDefault) {
}
const watermarkService = require('../services/watermarkService');
const watermarkGeneratorService = require('../services/watermarkGeneratorService');
const { verifyGalleryAccess, denySlideshowToken, verifyAdminPreview } = require('../middleware/gallery');
const { verifyGalleryAccess, denySlideshowToken, isAdminPreview } = require('../middleware/gallery');
const { resolveGuest } = require('../middleware/guestAuth');
const { generateGuestIdentifier } = require('../middleware/feedbackRateLimit');
const secureImageService = require('../services/secureImageService');
@@ -46,7 +46,6 @@ const {
} = require('../services/downloadFilenameService');
const { buildContentDisposition } = require('../utils/filenameSanitizer');
const { getStorage } = require('../services/storage');
const { createArchiveStreamGuard } = require('../utils/archiveStreamGuard');
const { setGalleryAuthCookies } = require('../utils/tokenUtils');
// Read globals from app_settings (the real table) — settingsService.getSetting
// queries a non-existent `settings` table and throws.
@@ -111,36 +110,11 @@ async function checkSlugRedirect(slug) {
}
}
// Admin preview of an unpublished gallery (#1386). The /info route below has
// honoured ?preview= for drafts for a while; this route never did, so the
// short-URL form of a draft's share link 404'd with "Gallery Not Found" while
// the long slug form worked.
//
// Deliberately a second lookup on the miss path rather than a widened filter:
// the published case keeps its single query and cannot start returning drafts
// however this evolves, and an unverified caller never gets so far as knowing
// the draft exists.
async function resolveDraftForAdminPreview(req, identifier) {
// A preview credential is required either way, so checking up front costs
// nothing and keeps an unknown identifier from paying for a second set of
// lookups on the public 404 path. admin_preview=1 is what the frontend
// sends; the bare ?preview=<jwt> is the legacy hand-built-link form.
if (req.query?.admin_preview !== '1' && !req.query?.preview) return null;
const result = await resolveShareIdentifier(identifier, { includeDrafts: true });
if (!result) return null;
// Authorized against THIS event, not just against a valid signature (#1411).
return await verifyAdminPreview(req, result.event) ? result : null;
}
// Resolve gallery identifier (slug or token) to canonical data
router.get('/resolve/:identifier', handleAsync(async (req, res) => {
const { identifier } = req.params;
let result = await resolveShareIdentifier(identifier);
if (!result) {
result = await resolveDraftForAdminPreview(req, identifier);
}
// If not found, check for redirect
if (!result) {
const newSlug = await checkSlugRedirect(identifier);
@@ -186,24 +160,14 @@ router.get('/:slug/verify-token/:token', handleAsync(async (req, res) => {
const { slug, token } = req.params;
const event = await db('events')
.where({ slug, is_active: formatBoolean(true), is_archived: formatBoolean(false) })
// created_by is the ownership input for verifyAdminPreview (#1411).
// Omitting it made every draft look ownerless here, so a non-owning admin
// holding events.view/photos.view validated another photographer's share
// link while /resolve and /info correctly refused them.
.select('id', 'share_link', 'share_token', 'is_draft', 'created_by')
.where({ slug, is_active: formatBoolean(true), is_archived: formatBoolean(false), is_draft: formatBoolean(false) })
.select('id', 'share_link', 'share_token')
.first();
if (!event) {
throw new NotFoundError('Gallery');
}
// Drafts are visible to an authorized admin preview only (#1386, #1411).
// Without this the preview clears /resolve and then 404s one step later.
if (event.is_draft && !await verifyAdminPreview(req, event)) {
throw new NotFoundError('Gallery');
}
const expectedToken = getEventShareToken(event);
if (!expectedToken || !timingSafeEqualStr(String(token), expectedToken)) {
throw new NotFoundError('Gallery', 'Invalid gallery link');
@@ -246,8 +210,6 @@ router.get('/:slug/info', async (req, res) => {
'hero_divider_style',
'hero_image_anchor',
'is_draft',
// Ownership input for the preview check (#1411).
'created_by',
'default_photo_sort',
// Per-event promotional override (#440). Resolution into a
// ready-to-render markdown string happens below so the
@@ -275,10 +237,8 @@ router.get('/:slug/info', async (req, res) => {
return res.status(404).json({ error: 'Gallery has been archived and is no longer available' });
}
// Check if event is a draft (allow an AUTHORIZED admin preview — #1411:
// a valid signature alone used to be enough, so any admin previewed any
// draft, including one belonging to a different photographer).
if (event.is_draft && !await verifyAdminPreview(req, event)) {
// Check if event is a draft (allow admin preview)
if (event.is_draft && !isAdminPreview(req)) {
return res.status(404).json({ error: 'Gallery is not yet published' });
}
@@ -889,17 +849,7 @@ router.get('/:slug/photos', verifyGalleryAccess, resolveGuest, async (req, res)
},
categories: categories,
photos: photos.map(photo => {
// Videos always take the JWT route (#1370). The secure-images template
// below can never serve one — the route runs the bytes through sharp,
// which throws on an mp4 — and nothing substitutes the {{token}}
// placeholder for the <video> element either, so under enhanced/maximum
// a video resolved to a 403 and the lightbox sat at 0:00. The matching
// exemption is on the /photo/:photoId route below.
const isVideo = photo.media_type === 'video'
|| (photo.mime_type && photo.mime_type.startsWith('video/'));
const useJwtUrl = isVideo
|| protectionSettings.protection_level === 'basic'
|| protectionSettings.protection_level === 'standard';
const useJwtUrl = (protectionSettings.protection_level === 'basic' || protectionSettings.protection_level === 'standard');
// Add watermark version to URLs for cache busting when settings change
const wmQuery = wmVersion ? `?${wmVersion}` : '';
const photoUrl = useJwtUrl ?
@@ -1366,10 +1316,6 @@ async function bumpEventDownloadCounts(eventId) {
}
router.get('/:slug/download-all', verifyGalleryAccess, denySlideshowToken, async (req, res) => {
// Hoisted so the catch can reclaim reads opened before the failure, and so a
// cancelled download never reaches finalize() (see the close handler below).
let guard = null;
let cancelled = false;
try {
// Check if downloads are allowed for this event
if (!parseBooleanInput(req.event.allow_downloads, true)) {
@@ -1484,22 +1430,6 @@ router.get('/:slug/download-all', verifyGalleryAccess, denySlideshowToken, async
throw err;
});
// Bound and reclaim the storage reads. archiver drains its sources one at
// a time, so appending one read per photo parks an S3 socket per photo
// holding unread bytes, and nothing reclaims them — archiver's abort()
// does not touch source streams, and the SDK clears its socket timeout as
// soon as response headers land.
guard = createArchiveStreamGuard({
onFatalError: () => { cancelled = true; guard.destroyAll(); archive.abort(); },
});
res.on('close', () => {
if (!res.writableFinished) {
cancelled = true;
guard.destroyAll();
archive.abort();
}
});
archive.pipe(res);
// Get watermark settings - apply if global setting OR event-level setting is enabled
@@ -1568,9 +1498,8 @@ router.get('/:slug/download-all', verifyGalleryAccess, denySlideshowToken, async
archive.append(watermarkedBuffer, { name: archiveName });
} else if (storageKey) {
if (!await guard.acquire()) break;
const stream = await storage.get(storageKey);
archive.append(guard.track(stream), { name: archiveName });
archive.append(stream, { name: archiveName });
} else {
archive.file(resolvePhotoFilePath(req.event, photo), { name: archiveName });
}
@@ -1585,7 +1514,6 @@ router.get('/:slug/download-all', verifyGalleryAccess, denySlideshowToken, async
}
}
if (cancelled) return;
await archive.finalize();
// Log bulk download
@@ -1602,19 +1530,12 @@ router.get('/:slug/download-all', verifyGalleryAccess, denySlideshowToken, async
.increment('download_count', 1).catch(() => {});
}
} catch (error) {
if (guard) guard.destroyAll();
// The client already left and the ZIP headers are gone; sending JSON here
// throws ERR_HTTP_HEADERS_SENT out of an async handler with nothing to
// catch it.
if (cancelled || res.headersSent) return;
errorResponse(res, error, 500, 'Failed to create download archive');
}
});
// Download selected photos as ZIP
router.post('/:slug/download-selected', verifyGalleryAccess, denySlideshowToken, async (req, res) => {
let selectedGuard = null;
let selectedCancelled = false;
try {
// Check if downloads are allowed for this event
if (!parseBooleanInput(req.event.allow_downloads, true)) {
@@ -1674,19 +1595,6 @@ router.post('/:slug/download-selected', verifyGalleryAccess, denySlideshowToken,
// ignore double-send errors
}
});
selectedGuard = createArchiveStreamGuard({
onFatalError: () => { selectedCancelled = true; selectedGuard.destroyAll(); archive.abort(); },
});
archive.on('error', () => selectedGuard.destroyAll());
res.on('close', () => {
if (!res.writableFinished) {
selectedCancelled = true;
selectedGuard.destroyAll();
archive.abort();
}
});
archive.pipe(res);
// Check watermark settings - apply if global setting OR event-level setting is enabled
@@ -1733,9 +1641,8 @@ router.post('/:slug/download-selected', verifyGalleryAccess, denySlideshowToken,
: await watermarkService.applyWatermark(resolvePhotoFilePath(req.event, photo), effectiveSettings);
archive.append(buf, { name });
} else if (storageKey) {
if (!await selectedGuard.acquire()) break;
const stream = await selectedStorage.get(storageKey);
archive.append(selectedGuard.track(stream), { name });
archive.append(stream, { name });
} else {
archive.file(resolvePhotoFilePath(req.event, photo), { name });
}
@@ -1750,7 +1657,6 @@ router.post('/:slug/download-selected', verifyGalleryAccess, denySlideshowToken,
}
}
if (selectedCancelled) return;
await archive.finalize();
await db('access_logs').insert({
@@ -1766,8 +1672,6 @@ router.post('/:slug/download-selected', verifyGalleryAccess, denySlideshowToken,
.increment('download_count', 1).catch(() => {});
}
} catch (error) {
if (selectedGuard) selectedGuard.destroyAll();
if (selectedCancelled || res.headersSent) return;
errorResponse(res, error, 500, 'Failed to download selected photos');
}
});
@@ -1830,14 +1734,7 @@ router.get('/:slug/photo/:photoId',
// Check protection level - basic and standard protection allow direct JWT access
const protectionLevel = req.event.protection_level || 'standard';
// Videos are exempt (#1370). The secure-images endpoint this bounces to
// pipes every byte through sharp (secureImageService.processProtectedImage),
// which throws on an mp4 — so under enhanced/maximum a video was
// unservable by either route, and the lightbox showed a poster stuck at
// 0:00. Serving it here instead is not a new exposure: thumbnails of the
// same videos already come from this route at every protection level, and
// the guest still needs a valid gallery token to get here at all.
if (!isVideo && (protectionLevel === 'enhanced' || protectionLevel === 'maximum')) {
if (protectionLevel === 'enhanced' || protectionLevel === 'maximum') {
// For enhanced/maximum protection, redirect to secure endpoint
return res.status(302).json({
error: 'Secure access required',
@@ -2445,12 +2342,7 @@ router.post('/:eventId/upload', verifyGalleryAccess, denySlideshowToken, async (
dest: tempUploadDir,
limits: {
fileSize: 50 * 1024 * 1024, // 50MB per file (separate concern from #613)
files: maxFilesPerUpload,
// CVE-2026-82333: files arrive as repeated `photos` parts via
// .array(), not bracket-indexed field names like `photos[0]` — no
// legitimate field name uses array-index syntax at all. Reject any
// that do.
fieldArrayIndexLimit: 0
files: maxFilesPerUpload
},
fileFilter: (req, file, cb) => {
if (validateFileType(file.originalname, file.mimetype, allowedMimeTypes)) {
+1 -4
View File
@@ -62,10 +62,7 @@ const signedPdfStorage = multer.diskStorage({
const signedPdfUpload = multer({
storage: signedPdfStorage,
// CVE-2026-82333: single unnamed `file` field only, and this route is
// unauthenticated (token-only) — no legitimate array-indexed field
// names, so reject any bracket-index field name.
limits: { fileSize: 10 * 1024 * 1024, fieldArrayIndexLimit: 0 }, // 10 MB
limits: { fileSize: 10 * 1024 * 1024 }, // 10 MB
fileFilter: (req, file, cb) => {
if (validateFileType(file.originalname, file.mimetype, ['application/pdf'])) return cb(null, true);
return cb(new Error('Only PDF files are allowed'));
+1 -3
View File
@@ -57,9 +57,7 @@ const photoStorage = multer.diskStorage({
});
const photoUpload = multer({
storage: photoStorage,
// CVE-2026-82333: single unnamed `photo` field only — no legitimate
// array-indexed field names, so reject any bracket-index field name.
limits: { fileSize: 100 * 1024 * 1024, fieldArrayIndexLimit: 0 }, // 100MB per file for v1
limits: { fileSize: 100 * 1024 * 1024 }, // 100MB per file for v1
fileFilter: (_req, file, cb) => {
if (/^image\//.test(file.mimetype)) cb(null, true);
else cb(new Error('Only image uploads are accepted on this endpoint'));
@@ -1,3 +1,4 @@
const { DatabaseBackupService } = require('../databaseBackup');
const { db } = require('../../database/db');
const fs = require('fs').promises;
const path = require('path');
@@ -8,10 +9,6 @@ jest.mock('../../database/db');
jest.mock('../../utils/logger');
jest.mock('../emailProcessor');
jest.mock('child_process');
jest.mock('node-cron', () => ({ schedule: jest.fn(() => ({ stop: jest.fn() })) }));
const { DatabaseBackupService, startScheduledBackups, databaseBackupService, isUnderPubliclyServableRoot } = require('../databaseBackup');
const cron = require('node-cron');
describe('DatabaseBackupService', () => {
let service;
@@ -191,213 +188,6 @@ describe('DatabaseBackupService', () => {
});
});
describe('backup() destination path resolution (#1365)', () => {
// getBackupConfig() returns database_backup_*-prefixed keys.
// Regression: backup() used to destructure the unprefixed names
// (`destinationPath`, ...) straight off that object, which never
// matched, so the configured path was silently ignored and every
// run tried to create the hardcoded /backup/database default.
it('creates the directory from database_backup_destination_path when configured', async () => {
db.mockReturnValue({
where: jest.fn().mockReturnThis(),
select: jest.fn().mockResolvedValue([
{ setting_key: 'database_backup_destination_path', setting_value: JSON.stringify('/data/db-backups') }
])
});
const stop = new Error('stop after mkdir — nothing past it matters for this test');
const mkdirSpy = jest.spyOn(fs, 'mkdir').mockRejectedValue(stop);
await expect(service.backup({})).rejects.toThrow(stop.message);
expect(mkdirSpy).toHaveBeenCalledWith('/data/db-backups', { recursive: true });
mkdirSpy.mockRestore();
});
it('falls back to /backup/database only when nothing is configured', async () => {
db.mockReturnValue({
where: jest.fn().mockReturnThis(),
select: jest.fn().mockResolvedValue([])
});
const stop = new Error('stop after mkdir');
const mkdirSpy = jest.spyOn(fs, 'mkdir').mockRejectedValue(stop);
await expect(service.backup({})).rejects.toThrow(stop.message);
expect(mkdirSpy).toHaveBeenCalledWith('/backup/database', { recursive: true });
mkdirSpy.mockRestore();
});
});
describe('isUnderPubliclyServableRoot (GHSA-jw8m class, #1365)', () => {
const originalStoragePath = process.env.STORAGE_PATH;
const storage = '/tmp/picpeak-test-storage';
beforeEach(() => {
process.env.STORAGE_PATH = storage;
});
afterAll(() => {
if (originalStoragePath === undefined) {
delete process.env.STORAGE_PATH;
} else {
process.env.STORAGE_PATH = originalStoragePath;
}
});
it.each([
path.join(storage, 'uploads', 'logos'),
path.join(storage, 'uploads', 'logos', 'sub'),
path.join(storage, 'uploads', 'favicons'),
path.join(storage, 'fonts'),
path.join(storage, 'fonts', 'inter'),
// Bundled fallback fonts — nodejs-owned per the Dockerfile's
// COPY --chown, and served at the same public /fonts route.
path.resolve(__dirname, '../../../assets/fonts'),
// Case-insensitive-but-preserving filesystems (APFS, NTFS, Docker
// Desktop bind mounts of either) resolve this to the same directory
// as uploads/logos even though path.resolve() never folds case.
path.join(storage, 'UPLOADS', 'Logos')
])('flags %s as publicly servable', (candidate) => {
expect(isUnderPubliclyServableRoot(candidate)).toBe(true);
});
it.each([
path.join(storage, 'backups'),
path.join(storage, 'uploads', 'contracts', 'signed'),
path.join(storage, 'uploads', 'transfers', '123'),
'/data/db-backups'
])('does not flag %s', (candidate) => {
expect(isUnderPubliclyServableRoot(candidate)).toBe(false);
});
it('backup() refuses a destination inside a publicly servable root without ever calling mkdir', async () => {
const publicPath = path.join(storage, 'uploads', 'logos');
db.mockReturnValue({
where: jest.fn().mockReturnThis(),
select: jest.fn().mockResolvedValue([
{ setting_key: 'database_backup_destination_path', setting_value: JSON.stringify(publicPath) }
])
});
const mkdirSpy = jest.spyOn(fs, 'mkdir');
await expect(service.backup({})).rejects.toThrow('publicly served directory');
expect(mkdirSpy).not.toHaveBeenCalled();
mkdirSpy.mockRestore();
});
it('flags FRONTEND_DIR — the all-in-one image serves its built SPA unauthenticated', () => {
const originalFrontendDir = process.env.FRONTEND_DIR;
process.env.FRONTEND_DIR = '/app/frontend/dist';
try {
expect(isUnderPubliclyServableRoot('/app/frontend/dist')).toBe(true);
expect(isUnderPubliclyServableRoot(path.join('/app/frontend/dist', 'assets'))).toBe(true);
} finally {
if (originalFrontendDir === undefined) delete process.env.FRONTEND_DIR;
else process.env.FRONTEND_DIR = originalFrontendDir;
}
});
it('resolves a symlinked alias of a public root to the same real directory (all-in-one /app/storage -> /data/storage)', async () => {
const os = require('os');
const realRoot = await fs.mkdtemp(path.join(os.tmpdir(), 'picpeak-real-'));
const linkRoot = path.join(os.tmpdir(), `picpeak-link-${process.pid}-${Date.now()}`);
await fs.mkdir(path.join(realRoot, 'uploads', 'logos'), { recursive: true });
await fs.symlink(realRoot, linkRoot, 'dir');
try {
// STORAGE_PATH (what the guard's roots are built from) is the real
// path; the attacker-supplied destination goes through the symlink
// — exactly the all-in-one image's /app/storage -> /data/storage.
process.env.STORAGE_PATH = realRoot;
const aliased = path.join(linkRoot, 'uploads', 'logos');
expect(isUnderPubliclyServableRoot(aliased)).toBe(true);
} finally {
await fs.unlink(linkRoot);
await fs.rm(realRoot, { recursive: true, force: true });
}
});
});
describe('startScheduledBackups (#1365)', () => {
// Same key-mismatch bug as backup(): getBackupConfig() returns
// database_backup_*-prefixed keys, but this read `config.enabled` /
// `config.schedule` / `config.retentionDays` — always undefined, so
// the scheduler silently treated every install as disabled.
it('does not start the schedule while database_backup_enabled is false', async () => {
db.mockReturnValue({
where: jest.fn().mockReturnThis(),
select: jest.fn().mockResolvedValue([
{ setting_key: 'database_backup_enabled', setting_value: 'false' }
])
});
await startScheduledBackups();
expect(cron.schedule).not.toHaveBeenCalled();
});
it('starts the schedule with the configured cron when database_backup_enabled is true', async () => {
db.mockReturnValue({
where: jest.fn().mockReturnThis(),
select: jest.fn().mockResolvedValue([
{ setting_key: 'database_backup_enabled', setting_value: 'true' },
{ setting_key: 'database_backup_schedule', setting_value: JSON.stringify('0 4 * * *') }
])
});
await startScheduledBackups();
expect(cron.schedule).toHaveBeenCalledWith('0 4 * * *', expect.any(Function));
});
it('re-reads retention on every tick instead of the value captured at schedule start (#1365)', async () => {
db.mockReturnValue({
where: jest.fn().mockReturnThis(),
select: jest.fn().mockResolvedValue([
{ setting_key: 'database_backup_enabled', setting_value: 'true' },
{ setting_key: 'database_backup_retention_days', setting_value: JSON.stringify(30) }
])
});
await startScheduledBackups();
const tick = cron.schedule.mock.calls[0][1];
// A /config update between schedule-start and this tick raised
// retention to 365 — the closed-over 30 must not be what runs.
db.mockReturnValue({
where: jest.fn().mockReturnThis(),
select: jest.fn().mockResolvedValue([
{ setting_key: 'database_backup_enabled', setting_value: 'true' },
{ setting_key: 'database_backup_retention_days', setting_value: JSON.stringify(365) }
])
});
jest.spyOn(databaseBackupService, 'backup').mockResolvedValue({ success: true });
const cleanupSpy = jest.spyOn(databaseBackupService, 'cleanupOldBackups').mockResolvedValue(undefined);
await tick();
expect(cleanupSpy).toHaveBeenCalledWith(365);
jest.restoreAllMocks();
});
});
describe('cleanupOldBackups destructive-retention guard (#1365)', () => {
it.each([-1, 0, NaN, Infinity])('refuses retentionDays=%s without touching the database', async (bad) => {
const dbSpy = jest.fn();
db.mockImplementation(dbSpy);
await service.cleanupOldBackups(bad);
expect(dbSpy).not.toHaveBeenCalled();
});
});
describe('cleanupOldBackups', () => {
it('should delete old backup files and records', async () => {
const oldBackups = [
@@ -1,125 +0,0 @@
jest.mock('../../utils/logger');
jest.mock('fluent-ffmpeg');
jest.mock('../storage', () => ({
getStorage: jest.fn()
}));
jest.mock('../imageProcessor', () => ({
generateVideoPlaceholder: jest.fn(),
DEFAULT_THUMBNAIL_WIDTH: 300,
DEFAULT_THUMBNAIL_HEIGHT: 300
}));
const ffmpeg = require('fluent-ffmpeg');
const { getStorage } = require('../storage');
const { generateVideoPlaceholder } = require('../imageProcessor');
const {
extractVideoMetadata,
processUploadedVideo
} = require('../videoProcessor');
describe('extractVideoMetadata (#1370)', () => {
afterEach(() => jest.clearAllMocks());
it('returns null duration rather than 0 when ffprobe has none, so "unknown" and "a real 0s clip" stay distinguishable', async () => {
ffmpeg.ffprobe = jest.fn((videoPath, cb) => {
cb(null, {
streams: [{ codec_type: 'video', width: 1920, height: 1080, codec_name: 'hevc' }],
format: {} // no duration field at all
});
});
const metadata = await extractVideoMetadata('/tmp/video.mp4');
expect(metadata.duration).toBeNull();
expect(metadata.width).toBe(1920);
expect(metadata.videoCodec).toBe('hevc');
});
it('floors a real duration', async () => {
ffmpeg.ffprobe = jest.fn((videoPath, cb) => {
cb(null, { streams: [], format: { duration: 12.9 } });
});
const metadata = await extractVideoMetadata('/tmp/video.mp4');
expect(metadata.duration).toBe(12);
});
});
describe('processUploadedVideo degrades gracefully instead of rejecting the whole video (#1370)', () => {
let storage;
beforeEach(() => {
storage = { putFromFile: jest.fn().mockResolvedValue(undefined), exists: jest.fn().mockResolvedValue(true) };
getStorage.mockReturnValue(storage);
generateVideoPlaceholder.mockResolvedValue('thumbnails/thumb_placeholder.jpg');
});
afterEach(() => jest.clearAllMocks());
it('keeps the thumbnail when only metadata extraction fails', async () => {
ffmpeg.ffprobe = jest.fn((videoPath, cb) => cb(new Error('moov atom not found')));
ffmpeg.mockImplementation(() => ({
screenshots: jest.fn(function screenshots({ filename, folder }) {
require('fs').writeFileSync(require('path').join(folder, filename), 'jpeg-bytes');
return this;
}),
on(event, handler) {
if (event === 'end') setImmediate(handler);
return this;
}
}));
const result = await processUploadedVideo('/tmp/video.mp4', 'thumbnails/thumb_video.jpg');
expect(result.success).toBe(true);
expect(result.metadata).toBeNull();
expect(result.thumbnailKey).toBe('thumbnails/thumb_video.jpg');
// A real thumbnail already succeeded — never touch the placeholder path.
expect(generateVideoPlaceholder).not.toHaveBeenCalled();
});
it('falls back to the SVG placeholder when thumbnail generation fails, so the gallery never falls back to rendering the raw video as an <img> (codex review)', async () => {
ffmpeg.ffprobe = jest.fn((videoPath, cb) => {
cb(null, {
streams: [{ codec_type: 'video', width: 1080, height: 1920, codec_name: 'h264' }],
format: { duration: 5.4 }
});
});
ffmpeg.mockImplementation(() => ({
screenshots() { return this; },
on(event, handler) {
if (event === 'error') setImmediate(() => handler(new Error('ffmpeg seek failed')));
return this;
}
}));
const result = await processUploadedVideo('/tmp/video.mp4', 'thumbnails/thumb_wedding_001.jpg');
expect(result.success).toBe(true);
expect(result.metadata).toEqual(expect.objectContaining({ duration: 5, videoCodec: 'h264' }));
// thumbnailKey is always thumbnails/thumb_<name>.jpg — strip the prefix
// back to a filename so generateVideoPlaceholder recomputes the same key.
// Explicit width/height so generateVideoPlaceholder skips its DB-backed
// settings lookup — this can run inside an open per-file SQLite
// transaction (chunked video upload), where that lookup deadlocks.
expect(generateVideoPlaceholder).toHaveBeenCalledWith('wedding_001.jpg', { width: 300, height: 300 });
expect(result.thumbnailKey).toBe('thumbnails/thumb_placeholder.jpg');
expect(storage.putFromFile).not.toHaveBeenCalled();
});
it('throws when metadata, thumbnail generation, AND the placeholder all fail, so the caller surfaces a retryable failure instead of completing with nothing to show (codex review)', async () => {
ffmpeg.ffprobe = jest.fn((videoPath, cb) => cb(new Error('Invalid data found when processing input')));
ffmpeg.mockImplementation(() => ({
screenshots() { return this; },
on(event, handler) {
if (event === 'error') setImmediate(() => handler(new Error('ffmpeg seek failed')));
return this;
}
}));
generateVideoPlaceholder.mockRejectedValue(new Error('sharp render failed'));
await expect(processUploadedVideo('/tmp/corrupt.mp4', 'thumbnails/thumb_corrupt.jpg'))
.rejects.toThrow('Unable to generate any thumbnail');
});
});
+1 -1
View File
@@ -1043,7 +1043,7 @@ function buildManifestFiles(backedUpFiles, allFiles) {
async function saveManifestToLocal(manifest, manifestFileName, config) {
const manifestDir = config.backup_manifest_path
|| path.join(config.backup_destination_path || path.join(getStoragePath(), 'backups'), 'manifests');
|| path.join(config.backup_destination_path || '/backup', 'manifests');
await fs.mkdir(manifestDir, { recursive: true });
const manifestPath = path.join(manifestDir, manifestFileName);
await backupManifest.saveManifest(manifest, manifestPath, config.backup_manifest_format || 'json');
-50
View File
@@ -360,56 +360,6 @@ Rechnung {{invoice_number}} für {{customer_name}}{{#if event_name}} ({{event_na
Automatische Benachrichtigung keine Aktion erforderlich.`,
},
},
invoice_payment_check_action_recorded: {
// GHSA-wg94-f86h-vq68 hardening: the payment-check link at
// /payment-check/:token is unauthenticated by design (see
// publicPaymentCheck.js) — token possession is the only gate.
// This notifies the admin every time that link is used to write
// to the invoice ledger, so the no-login convenience stays but an
// admin always sees the action happen.
category: 'billing', feature_flag: 'bills',
variables: ['invoice_number', 'customer_name', 'event_name', 'action', 'has_amount', 'amount', 'ip', 'recorded_at'],
en: {
subject: 'Payment-check action recorded: invoice {{invoice_number}}',
body_html: `<h2>Payment-check link used</h2>
<p>Someone used the unauthenticated payment-check link for invoice <strong>{{invoice_number}}</strong>{{#if customer_name}} ({{customer_name}}){{/if}}{{#if event_name}}, {{event_name}}{{/if}} and recorded: <strong>{{action}}</strong>{{#if has_amount}} ({{amount}}){{/if}}.</p>
<table role="presentation" cellpadding="6" cellspacing="0" border="0" style="border-collapse: collapse; margin: 16px 0;">
<tr><td style="color: #666;">Action</td><td><strong>{{action}}</strong></td></tr>
{{#if has_amount}}<tr><td style="color: #666;">Amount</td><td>{{amount}}</td></tr>{{/if}}
<tr><td style="color: #666;">IP address</td><td>{{ip}}</td></tr>
<tr><td style="color: #666;">Recorded at</td><td>{{recorded_at}}</td></tr>
</table>
<p style="font-size: 13px; color: #666;">This link requires no login only the token in the URL. If you don't recognise this action, review the invoice in the admin panel.</p>`,
body_text: `Payment-check link used
Invoice {{invoice_number}}{{#if customer_name}} ({{customer_name}}){{/if}}{{#if event_name}}, {{event_name}}{{/if}} recorded: {{action}}{{#if has_amount}} ({{amount}}){{/if}}.
IP address: {{ip}}
Recorded at: {{recorded_at}}
This link requires no login only the token in the URL. If you don't recognise this action, review the invoice in the admin panel.`,
},
de: {
subject: 'Zahlungsprüfung ausgelöst: Rechnung {{invoice_number}}',
body_html: `<h2>Zahlungsprüfungs-Link verwendet</h2>
<p>Der nicht-authentifizierte Zahlungsprüfungs-Link für Rechnung <strong>{{invoice_number}}</strong>{{#if customer_name}} ({{customer_name}}){{/if}}{{#if event_name}}, {{event_name}}{{/if}} wurde verwendet und hat erfasst: <strong>{{action}}</strong>{{#if has_amount}} ({{amount}}){{/if}}.</p>
<table role="presentation" cellpadding="6" cellspacing="0" border="0" style="border-collapse: collapse; margin: 16px 0;">
<tr><td style="color: #666;">Aktion</td><td><strong>{{action}}</strong></td></tr>
{{#if has_amount}}<tr><td style="color: #666;">Betrag</td><td>{{amount}}</td></tr>{{/if}}
<tr><td style="color: #666;">IP-Adresse</td><td>{{ip}}</td></tr>
<tr><td style="color: #666;">Erfasst am</td><td>{{recorded_at}}</td></tr>
</table>
<p style="font-size: 13px; color: #666;">Dieser Link erfordert kein Login nur den Token in der URL. Falls Ihnen diese Aktion unbekannt vorkommt, prüfen Sie die Rechnung im Admin-Bereich.</p>`,
body_text: `Zahlungsprüfungs-Link verwendet
Rechnung {{invoice_number}}{{#if customer_name}} ({{customer_name}}){{/if}}{{#if event_name}}, {{event_name}}{{/if}} erfasst: {{action}}{{#if has_amount}} ({{amount}}){{/if}}.
IP-Adresse: {{ip}}
Erfasst am: {{recorded_at}}
Dieser Link erfordert kein Login nur den Token in der URL. Falls Ihnen diese Aktion unbekannt vorkommt, prüfen Sie die Rechnung im Admin-Bereich.`,
},
},
invoice_collections_handoff: {
category: 'billing', feature_flag: 'bills',
variables: ['invoice_number', 'customer_name', 'customer_email', 'customer_address', 'event_name', 'original_amount', 'late_fee_amount', 'paid_amount', 'outstanding_amount', 'due_date', 'reminder_level'],
+13 -116
View File
@@ -4,7 +4,7 @@ const crypto = require('crypto');
const { spawnAsync, spawnToFile } = require('../utils/safeExec');
const zlib = require('zlib');
const { pipeline } = require('stream/promises');
const { createReadStream, createWriteStream, realpathSync } = require('fs');
const { createReadStream, createWriteStream } = require('fs');
const { db } = require('../database/db');
const knexConfig = require('../../knexfile');
const logger = require('../utils/logger');
@@ -16,76 +16,6 @@ const packageJson = require('../../package.json');
const CHUNK_SIZE = 1024 * 1024; // 1MB chunks for streaming
const PROGRESS_INTERVAL = 100; // Report progress every 100 rows
function getStoragePath() {
return process.env.STORAGE_PATH || path.join(__dirname, '../../../storage');
}
// Public, unauthenticated static mounts (server.js) that must never become a
// backup destination — a dump landing there is downloadable by anyone who
// learns or guesses the filename, GHSA-jw8m-43r2-jqrm's exact class. Before
// #1365, `database_backup_destination_path` was silently ignored (a
// destructuring bug always fell back to the hardcoded /backup/database), so
// this setting being freely writable by any backup.create holder — the
// built-in `admin` role has it without settings.edit or backup.restore — was
// harmless. Making the setting actually take effect reopens that exact
// exfiltration path unless it's rejected here too.
function getPubliclyServableRoots() {
const storage = getStoragePath();
return [
path.join(storage, 'uploads', 'logos'),
path.join(storage, 'uploads', 'favicons'),
path.join(storage, 'fonts'),
// Bundled fallback fonts (server.js mounts both at /fonts, storage wins
// on overlap but express.static falls through to this one on a miss).
// COPY --chown=nodejs:nodejs in the Dockerfile makes this nodejs-owned
// and therefore writable at runtime, not just a read-only image layer.
path.resolve(__dirname, '../../assets/fonts'),
// The all-in-one image's built frontend bundle (Dockerfile.aio ships it
// nodejs-owned) — server.js serves it unauthenticated as the SPA itself.
process.env.FRONTEND_DIR || path.resolve(__dirname, '../../../frontend/dist')
];
}
// Resolves symlinks in whatever prefix of candidatePath currently exists,
// then re-appends any not-yet-created remainder literally. A plain
// fs.realpathSync would throw ENOENT for the common case where the backup
// destination doesn't exist yet; a plain path.resolve() would miss the
// all-in-one image's `/app/storage -> /data/storage` symlink (Dockerfile.aio),
// which lets `/app/storage/uploads/logos` alias the real public logos
// directory under a name that never lexically matches it.
function resolveRealish(candidatePath) {
let current = path.resolve(candidatePath);
const remainder = [];
for (;;) {
try {
const real = realpathSync(current);
return remainder.length ? path.join(real, ...remainder) : real;
} catch (error) {
if (error.code !== 'ENOENT') {
return path.resolve(candidatePath);
}
const parent = path.dirname(current);
if (parent === current) {
return path.resolve(candidatePath);
}
remainder.unshift(path.basename(current));
current = parent;
}
}
}
function isUnderPubliclyServableRoot(candidatePath) {
// Lowercased comparison: on a case-insensitive-but-preserving filesystem
// (default macOS APFS, NTFS, and Docker Desktop's bind-mount passthrough
// of either) `STORAGE_PATH/UPLOADS/logos` and `.../uploads/logos` name the
// same directory on disk even though path.resolve() never folds case.
const resolved = resolveRealish(candidatePath).toLowerCase();
return getPubliclyServableRoots().some((root) => {
const resolvedRoot = resolveRealish(root).toLowerCase();
return resolved === resolvedRoot || resolved.startsWith(resolvedRoot + path.sep);
});
}
/**
* Database Backup Service
* Supports both SQLite and PostgreSQL with proper escaping,
@@ -366,33 +296,15 @@ class DatabaseBackupService {
let backupRun = null;
try {
// Get configuration. getBackupConfig() returns the raw
// database_backup_*-prefixed setting keys, not the unprefixed
// names used internally below — map them explicitly rather than
// spreading `config` straight into the destructure, which silently
// matched nothing and always fell through to the hardcoded
// defaults (notably `/backup/database`, regardless of what was
// configured).
// Get configuration
const config = await this.getBackupConfig();
const {
destinationPath = '/backup/database',
compress = true,
validateIntegrity = true,
includeChecksums = true
} = {
destinationPath: config.database_backup_destination_path,
compress: config.database_backup_compress,
validateIntegrity: config.database_backup_validate_integrity,
includeChecksums: config.database_backup_include_checksums,
...options
};
} = { ...config, ...options };
if (isUnderPubliclyServableRoot(destinationPath)) {
throw new Error(
`Refusing to write a database backup to a publicly served directory: ${destinationPath}`
);
}
// Create backup directory
await fs.mkdir(destinationPath, { recursive: true });
@@ -511,7 +423,7 @@ class DatabaseBackupService {
logger.info(`Database backup completed: ${finalFile} (${(finalStats.size / 1024 / 1024).toFixed(2)} MB) in ${durationSeconds}s`);
// Send success notification if configured
if (config.database_backup_email_on_success) {
if (config.emailOnSuccess) {
await this.sendBackupNotification('success', {
duration: durationSeconds,
size: finalStats.size,
@@ -545,7 +457,7 @@ class DatabaseBackupService {
// Send failure notification
const config = await this.getBackupConfig();
if (config.database_backup_email_on_failure) {
if (config.emailOnFailure) {
await this.sendBackupNotification('failure', {
error: error.message
});
@@ -626,19 +538,10 @@ class DatabaseBackupService {
* Clean up old backups
*/
async cleanupOldBackups(retentionDays = 30) {
// A zero/negative/non-finite value pushes the cutoff to today or the
// future, matching (and deleting) every completed backup — including
// the one a scheduled run just created. Defense in depth: PUT /config
// already rejects such values, but this is also reachable with
// whatever database_backup_retention_days happens to be persisted.
if (!Number.isFinite(retentionDays) || retentionDays < 1) {
logger.error(`Refusing to clean up backups with invalid retentionDays: ${retentionDays}`);
return;
}
try {
const cutoffDate = new Date();
cutoffDate.setDate(cutoffDate.getDate() - retentionDays);
// Get old backup records
const oldBackups = await db('database_backup_runs')
.where('completed_at', '<', cutoffDate)
@@ -783,30 +686,25 @@ async function startScheduledBackups() {
try {
const config = await databaseBackupService.getBackupConfig();
if (!config.database_backup_enabled) {
if (!config.enabled) {
logger.info('Database backup service is disabled');
return;
}
// Stop existing schedule
if (backupSchedule) {
backupSchedule.stop();
}
// Default schedule: 3 AM daily (offset from file backups at 2 AM)
const schedule = config.database_backup_schedule || '0 3 * * *';
const schedule = config.schedule || '0 3 * * *';
backupSchedule = cron.schedule(schedule, async () => {
logger.info('Starting scheduled database backup');
try {
await databaseBackupService.backup();
// Re-read retention on every tick rather than closing over the value
// from schedule start — a retention-only /config update doesn't
// restart the schedule (only enabled/schedule changes do), so the
// closed-over value would otherwise run stale until next restart.
const latestConfig = await databaseBackupService.getBackupConfig();
await databaseBackupService.cleanupOldBackups(latestConfig.database_backup_retention_days || 30);
await databaseBackupService.cleanupOldBackups(config.retentionDays || 30);
} catch (error) {
logger.error('Scheduled database backup failed:', error);
}
@@ -833,6 +731,5 @@ module.exports = {
databaseBackupService,
startScheduledBackups,
stopScheduledBackups,
isUnderPubliclyServableRoot,
DatabaseBackupService // Export class for testing
};
+3 -75
View File
@@ -24,52 +24,15 @@ const watermarkService = require('./watermarkService');
const { resolvePhotoStorageKey, resolvePhotoFilePath } = require('./photoResolver');
const { getStorage } = require('./storage');
const { getUseOriginalFilenames, getZipEntryNames } = require('./downloadFilenameService');
const { createArchiveStreamGuard } = require('../utils/archiveStreamGuard');
const logger = require('../utils/logger');
const DEBOUNCE_MS = 5000;
// How many cached zips may be REBUILT at once in the background.
//
// invalidateAll() invalidates every event that has a cached zip, and each
// invalidate() arms its own debounce timer in the same tick — so they all fire
// together and, before this, every one of them started building at once. Each
// build opens its own storage reads, so 25 events was enough to exhaust the S3
// agent pool and stall uploads, thumbnails and gallery reads until the burst
// finished.
//
// This caps the BACKGROUND path only. A foreground generateZip() — a guest
// actually waiting for a download — is never queued behind a rebuild.
const MAX_CONCURRENT_REGENS = 2;
class DownloadZipService {
constructor() {
this.activeBuilds = new Map(); // eventId -> { promise, version }
this.debounceTimers = new Map(); // eventId -> setTimeout handle
this.versions = new Map(); // eventId -> generation counter
this.buildCancellers = new Map(); // eventId -> abort the in-flight build
this.regenActive = 0; // background rebuilds running right now
this.regenWaiters = []; // resolvers parked waiting for a slot
}
/**
* Run a BACKGROUND rebuild under the concurrency cap. Foreground callers
* deliberately do not go through here: someone is waiting on that response,
* and making them queue behind a settings-change burst would trade one stall
* for another.
*/
async _withRegenSlot(fn) {
if (this.regenActive >= MAX_CONCURRENT_REGENS) {
await new Promise((resolve) => this.regenWaiters.push(resolve));
}
this.regenActive += 1;
try {
return await fn();
} finally {
this.regenActive -= 1;
const next = this.regenWaiters.shift();
if (next) next();
}
}
/**
@@ -144,7 +107,6 @@ class DownloadZipService {
async _build(eventId, version) {
const storage = getStorage();
let tmpDir;
let buildGuard = null;
try {
const event = await db('events').where({ id: eventId }).first();
@@ -190,28 +152,8 @@ class DownloadZipService {
const output = fs.createWriteStream(tmpPath);
const archive = archiver('zip', { zlib: { level: 0 } });
// Bound and reclaim the storage reads. archiver drains its sources one
// at a time, so appending one read per photo parks an S3 socket per
// photo holding unread bytes; nothing reclaims them, because
// archiver's abort() does not touch source streams and the SDK clears
// its socket timeout as soon as response headers land. An unbounded
// loop over a large event starves uploads, thumbnails and gallery
// reads for the duration of the build.
buildGuard = createArchiveStreamGuard({
onFatalError: (err) => { buildGuard.destroyAll(); archive.abort(); reject(err); },
});
// Invalidation cancels the build directly rather than leaving a note
// for the loop: with a read cap in place the loop can be parked waiting
// for a slot that a stalled archive will never free.
this.buildCancellers.set(eventId, () => {
buildGuard.destroyAll();
archive.abort();
reject(new Error('Build invalidated'));
});
output.on('close', resolve);
archive.on('error', (err) => { buildGuard.destroyAll(); reject(err); });
archive.on('error', reject);
archive.pipe(output);
const uniqueTypes = new Set(photos.map(p => p.type)).size;
@@ -222,7 +164,6 @@ class DownloadZipService {
const photo = photos[i];
// Check if build was invalidated
if (this.versions.get(eventId) !== version) {
buildGuard.destroyAll();
archive.abort();
return reject(new Error('Build invalidated'));
}
@@ -262,9 +203,8 @@ class DownloadZipService {
logger.warn('Skipping watermark in pre-zip', { photoId: photo.id, error: err.message });
}
} else if (storageKey) {
if (!await buildGuard.acquire()) return;
const stream = await storage.get(storageKey);
archive.append(buildGuard.track(stream), { name: archiveName });
archive.append(stream, { name: archiveName });
} else {
const filePath = resolvePhotoFilePath(event, photo);
archive.file(filePath, { name: archiveName });
@@ -303,10 +243,6 @@ class DownloadZipService {
logger.error('downloadZipService._build error', { eventId, error: err.message });
return { success: false, error: err.message };
} finally {
// Every exit path — success, invalidated, thrown — has to reclaim the
// reads, or they hold their sockets for the life of the process.
if (buildGuard) buildGuard.destroyAll();
this.buildCancellers.delete(eventId);
if (tmpDir) {
await fsp.rm(tmpDir, { recursive: true, force: true }).catch(() => {});
}
@@ -325,12 +261,6 @@ class DownloadZipService {
const timer = this.debounceTimers.get(eventId);
if (timer) clearTimeout(timer);
// Cancel an in-flight build directly. Bumping the version only stops it the
// next time the loop looks, and with a read cap the loop can be parked
// waiting for a slot a stalled archive will never free.
const cancelBuild = this.buildCancellers.get(eventId);
if (cancelBuild) cancelBuild();
// Fire-and-forget cleanup
this._cleanup(eventId).catch(err =>
logger.warn('downloadZipService.invalidate cleanup error', { eventId, error: err.message })
@@ -339,9 +269,7 @@ class DownloadZipService {
// Debounce regeneration
const newTimer = setTimeout(() => {
this.debounceTimers.delete(eventId);
// Through the cap: invalidateAll arms every one of these in the same
// tick, so without it they all start building together.
this._withRegenSlot(() => this.generateZip(eventId)).catch(err =>
this.generateZip(eventId).catch(err =>
logger.warn('downloadZipService debounced regen error', { eventId, error: err.message })
);
}, DEBOUNCE_MS);
+3 -11
View File
@@ -369,15 +369,9 @@ async function generateVideoPlaceholder(originalFilename, options = {}) {
const thumbnailRelKey = path.posix.join('thumbnails', thumbnailFilename);
const storage = getStorage();
// Skip the settings lookup when the caller already supplies dimensions.
// This can run from inside an open per-file SQLite transaction (chunked
// video upload's fallback path in videoProcessor.js) — a second,
// un-transacted db() query for settings there deadlocks against SQLite's
// single-connection pool until acquireConnectionTimeout (60s), reproduced
// directly against an isolated SQLite db (codex review of #1371/#1372).
const settings = (options.width && options.height) ? {} : await getThumbnailSettings();
const width = options.width || settings.width || DEFAULT_THUMBNAIL_WIDTH;
const height = options.height || settings.height || DEFAULT_THUMBNAIL_HEIGHT;
const settings = await getThumbnailSettings();
const width = settings.width || DEFAULT_THUMBNAIL_WIDTH;
const height = settings.height || DEFAULT_THUMBNAIL_HEIGHT;
if (options.regenerate) {
await storage.delete(thumbnailRelKey).catch(() => {});
@@ -870,6 +864,4 @@ module.exports = {
ensurePreviewImage,
extractCaptureDate,
withLocalCopy,
DEFAULT_THUMBNAIL_WIDTH,
DEFAULT_THUMBNAIL_HEIGHT,
};
+1 -62
View File
@@ -12,15 +12,6 @@ const { ensureInt } = require('../../utils/numericHelpers');
const { formatMajor } = require('./helpers');
const { applyReminder, resolveAdminEmailForInvoice, resolvePerReminderFeeMinor, resolveSkontoPercentForInvoice } = require('./reminders');
// Payment-check token lifetime (GHSA-wg94-f86h-vq68 hardening). This
// unauthenticated magic link is the only gate on a write to the
// invoice ledger, so it's kept short rather than the prior 30 days.
// The scheduler re-queues a fresh token daily (throttled by
// last_payment_check_at, see queuePaymentCheckEmail below) for as
// long as the invoice stays past its reminder cutoff, so a short TTL
// doesn't strand an admin who hasn't acted yet — they just get a new
// link on the next tick.
const PAYMENT_CHECK_TOKEN_TTL_MS = 72 * 60 * 60 * 1000; // 72h
/**
* Record a payment against an invoice. Supports partial payments
@@ -235,7 +226,7 @@ async function queuePaymentCheckEmail(invoiceId, { skipThrottle = false } = {})
}
const token = crypto.randomBytes(32).toString('hex');
const expiresAt = new Date(now.getTime() + PAYMENT_CHECK_TOKEN_TTL_MS);
const expiresAt = new Date(now.getTime() + 30 * 24 * 60 * 60 * 1000);
await db('invoice_payment_check_tokens').insert({
invoice_id: invoiceId,
token,
@@ -387,45 +378,6 @@ async function getPaymentCheckByToken(token) {
};
}
/**
* Best-effort admin notification for every write via the public,
* unauthenticated payment-check route (GHSA-wg94-f86h-vq68
* hardening). Token possession is the only gate on that route, so
* this fires on every successful action 'paid_full', 'partial',
* 'unpaid', 'paid_with_skonto' regardless of what the ledger
* effect ends up being, so an admin always sees the action happen.
* Callers MUST wrap this in try/catch: a failed send must never
* fail (or roll back) the ledger write it's reporting on.
*/
async function notifyAdminOfPaymentCheckAction({ invoice, action, amountMinor, ip }) {
const adminContact = await resolveAdminEmailForInvoice(invoice);
if (!adminContact?.email) {
logger.warn('Payment-check action notification skipped — no admin email resolved',
{ invoiceId: invoice.id, action });
return;
}
const profile = await db('business_profile').where({ id: 1 }).first();
const locale = invoice.language || profile?.default_locale || 'de';
const customer = await db('customer_accounts').where({ id: invoice.customer_account_id }).first();
await emailProcessor.queueEmail(invoice.event_id || null, adminContact.email,
'invoice_payment_check_action_recorded', {
invoice_number: invoice.invoice_number,
customer_name: customer?.company_name
|| customer?.display_name
|| [customer?.first_name, customer?.last_name].filter(Boolean).join(' ')
|| customer?.email || '',
event_name: invoice.event_name || '',
__language: locale,
action,
amount: amountMinor ? formatMajor(ensureInt(amountMinor), invoice.currency, locale) : '',
has_amount: !!amountMinor,
ip: ip || 'unknown',
recorded_at: formatShortDate(new Date()),
});
}
/**
* Record the admin's payment-check action and fire the downstream
* consequences:
@@ -494,19 +446,6 @@ async function recordPaymentCheckAction({ token, action, amountMinor, ip, adminI
adminId ? `admin:${adminId}` : 'public:payment-check');
} catch (_) {}
// Notify the admin this write happened. Best-effort / non-blocking
// — the ledger write above already committed, and a failed
// notification send must not undo or fail it.
if (!adminId) {
try {
await notifyAdminOfPaymentCheckAction({ invoice, action, amountMinor, ip });
} catch (err) {
logger.warn('Payment-check action admin notification failed', {
invoiceId: invoice.id, action, err: err.message,
});
}
}
// --- Apply the action -----------------------------------------
if (action === 'paid_full') {
await markPaid(invoice.id, {
+9 -93
View File
@@ -3,11 +3,7 @@
*
* Responsibilities:
* - generate/verify TOTP secrets (otplib, standard SHA1/6-digit/30s so
* Google Authenticator / Authy / 1Password all work), with replay
* protection: verifyTotpEncryptedStep() rejects a code whose matched
* time-step doesn't advance past the admin's last consumed one
* (GHSA-qcwx-r25m-j869 otplib's window:1 tolerance alone lets a
* captured code stay valid across several time-steps, ~90s);
* Google Authenticator / Authy / 1Password all work);
* - encrypt the secret at rest (AES-256-GCM) so a DB leak alone doesn't
* yield working authenticator seeds;
* - generate/verify one-time recovery codes, hashed (bcrypt) and single-use;
@@ -71,102 +67,25 @@ function decryptSecret(stored) {
return pt.toString('utf8');
}
/** Absolute TOTP time-step for "now" (Math.floor(Date.now() / 30000)). */
function currentTotpStep() {
return Math.floor(Date.now() / 30000);
}
/**
* Core TOTP check. Returns the matched absolute time-step (always a
* positive, truthy integer) when `code` is valid for `plainSecret`;
* otherwise `null`.
*
* When `lastUsedStep` is given, a code whose matched step doesn't advance
* past it is treated as invalid replay protection. Without this, otplib's
* window:1 tolerance lets a captured code stay valid across several real
* time-steps (~90s), so the same code could complete two independent admin
* logins (GHSA-qcwx-r25m-j869).
*/
function matchTotpStep(code, plainSecret, lastUsedStep) {
if (!code || !plainSecret) return null;
/** Verify a 6-digit TOTP code against the (plaintext) secret. */
function verifyTotp(code, plainSecret) {
if (!code || !plainSecret) return false;
try {
const token = String(code).replace(/\s+/g, '');
const delta = authenticator.checkDelta(token, plainSecret);
if (typeof delta !== 'number') return null;
const step = currentTotpStep() + delta;
if (typeof lastUsedStep === 'number' && step <= lastUsedStep) return null;
return step;
} catch {
return null;
}
}
/**
* Verify a 6-digit TOTP code against the (plaintext) secret. Pass
* `lastUsedStep` (the admin's previously-consumed step) to also enforce
* replay protection see matchTotpStep().
*/
function verifyTotp(code, plainSecret, lastUsedStep) {
return matchTotpStep(code, plainSecret, lastUsedStep) !== null;
}
/** Verify a code against a STORED (encrypted) secret. */
function verifyTotpEncrypted(code, storedSecret, lastUsedStep) {
try {
return verifyTotp(code, decryptSecret(storedSecret), lastUsedStep);
return authenticator.verify({ token: String(code).replace(/\s+/g, ''), secret: plainSecret });
} catch {
return false;
}
}
/**
* Like verifyTotpEncrypted(), but returns the matched step (or `null` when
* the code is invalid/replayed) instead of a boolean, so a caller that
* grants a session or a sensitive action can persist it as the admin's new
* `two_factor_last_used_step`.
*/
function verifyTotpEncryptedStep(code, storedSecret, lastUsedStep) {
/** Verify a code against a STORED (encrypted) secret. */
function verifyTotpEncrypted(code, storedSecret) {
try {
return matchTotpStep(code, decryptSecret(storedSecret), lastUsedStep);
return verifyTotp(code, decryptSecret(storedSecret));
} catch {
return null;
return false;
}
}
/**
* Persist a newly-matched TOTP step, but only if it still advances
* `two_factor_last_used_step` at write time (`db('admin_users').where('id',
* adminId).whereNull(...).orWhere(...).update(...)`).
*
* matchTotpStep()'s "does this advance past lastUsedStep" check is read
* against a snapshot taken earlier in the request. Two concurrent requests
* carrying the same captured code can both read the same lastUsedStep and
* both pass that check before either write lands a plain, unconditional
* UPDATE would let both persist, defeating replay protection. Guarding the
* UPDATE with the same condition and checking the affected-row count makes
* only the first writer succeed; a losing concurrent request gets 0 affected
* rows and must be treated as a replay by the caller.
*
* @param {object} db - knex instance
* @param {number} adminId
* @param {number} totpStep - matched step from verifyTotpEncryptedStep()
* @param {object} [extraFields] - additional columns to set in the same UPDATE
* @returns {Promise<boolean>} true if this call won the race and persisted
*/
async function persistTotpStep(db, adminId, totpStep, extraFields = {}) {
const affected = await db('admin_users')
.where('id', adminId)
.where(function () {
this.whereNull('two_factor_last_used_step')
.orWhere('two_factor_last_used_step', '<', totpStep);
})
.update({
two_factor_last_used_step: totpStep,
...extraFields
});
return affected > 0;
}
/** otpauth:// URI for an authenticator app. */
function buildOtpauthUri(accountName, plainSecret) {
return authenticator.keyuri(accountName, ISSUER, plainSecret);
@@ -250,11 +169,8 @@ module.exports = {
generateSecret,
encryptSecret,
decryptSecret,
currentTotpStep,
verifyTotp,
verifyTotpEncrypted,
verifyTotpEncryptedStep,
persistTotpStep,
buildOtpauthUri,
buildQrDataUrl,
generateRecoveryCodes,
+11 -152
View File
@@ -22,85 +22,6 @@ function pathEscapes(baseDir, candidate) {
const rel = path.relative(path.resolve(baseDir), path.resolve(candidate));
return !rel || rel === '..' || rel.startsWith('..' + path.sep) || path.isAbsolute(rel);
}
// GHSA-xfvx: `manifest.database.backup_file` is just as attacker-influenceable
// as the file-manifest entries `pathEscapes` guards above (hand-crafted or
// tampered backup manifest) — an absolute path or a `..`-laden relative one
// must not be allowed to point the SQLite/PG restore at an arbitrary file on
// disk. Resolve the SAME operator-configured backup roots that
// `adminRestore.js`'s `checkRestorePathsAllowed` (GHSA-fw4c) enforces for the
// top-level `source`/`manifestPath` request fields, plus the already-trusted
// `backupPath` this restore run resolved to (always included, so this never
// fails open even when no backup_destination_path/backup_manifest_path is
// configured yet).
async function getConfiguredBackupRoots(trustedRoot) {
const roots = [];
if (trustedRoot) roots.push(trustedRoot);
try {
const rows = await db('app_settings')
.whereIn('setting_key', ['backup_destination_path', 'backup_manifest_path'])
.select('setting_value');
for (const row of rows) {
let value;
try { value = JSON.parse(row.setting_value); } catch (_) { value = row.setting_value; }
if (value) roots.push(value);
}
} catch (_) {
// best effort — fall through to whatever roots we already have
}
for (const extra of (process.env.RESTORE_ALLOWED_ROOTS || '').split(':')) {
if (extra.trim()) roots.push(extra.trim());
}
return roots.map((r) => path.resolve(r));
}
function isContainedInRoots(candidate, resolvedRoots) {
const resolved = path.resolve(candidate);
return resolvedRoots.some(
(root) => resolved === root || resolved.startsWith(root + path.sep)
);
}
// sqlite3's `.restore`/`.backup` are dot-commands parsed by sqlite3's OWN
// tokenizer, not the shell — spawn()'s argv separation (shell: false) does
// NOT protect against a single quote embedded in the path breaking out of
// the `.restore '<path>'` argument, since the whole `.restore '<path>'`
// string is one argv element that sqlite3 re-parses itself. sqlite3 offers
// no parameterized dot-command form, so constrain the path to a
// conservative safe charset before it is ever interpolated (GHSA-xfvx).
const SAFE_SQLITE_PATH_RE = /^[A-Za-z0-9._/-]+$/;
function assertSafeSqlitePath(p) {
if (typeof p !== 'string' || !SAFE_SQLITE_PATH_RE.test(p)) {
throw new Error(`Refusing to run sqlite3 against an unsafe path: ${p}`);
}
}
// GHSA-xfvx: the layered candidate resolution for `manifest.database.backup_file`
// (see performDatabaseRestore), factored out so the containment rule can be
// pinned directly in tests without exercising the surrounding DB-swap/spawn
// side effects. `warn` is an optional `(msg, meta) => void` logger hook.
async function resolveContainedDbBackupCandidates(backupPath, dbBackupFile, warn) {
const allowedRoots = await getConfiguredBackupRoots(backupPath);
const rawCandidates = [
// (1) Honour absolute paths recorded by the dumper.
path.isAbsolute(dbBackupFile) ? dbBackupFile : null,
// (2) Relative-to-backupPath as-stored (no basename munging).
path.join(backupPath, dbBackupFile),
// (3) Legacy reconstruct. Inherently safe: path.basename() strips any
// directory component, so this candidate can never escape backupPath.
path.join(backupPath, 'database', path.basename(dbBackupFile)),
].filter(Boolean);
return rawCandidates.filter((candidate) => {
const contained = isContainedInRoots(candidate, allowedRoots);
if (!contained && warn) {
warn('Refusing database backup candidate outside configured backup roots', {
candidate, dbBackupFile,
});
}
return contained;
});
}
const { formatBytes } = require('../utils/formatBytes');
const os = require('os');
@@ -971,26 +892,14 @@ class RestoreService {
// `Database backup file not found: local/database/...sql.gz`
// even though the file existed at exactly the path the manifest
// recorded.
// GHSA-xfvx: `dbBackupFile` comes straight out of the manifest, which is
// attacker-influenceable (hand-crafted or tampered backup). Neither
// candidate (1) nor (2) below used to be checked for containment, so a
// manifest could point `.restore` at an arbitrary file anywhere on disk
// (absolute path, or `../../` traversal through the path.join). Resolve
// each candidate and drop any that escape the configured backup roots
// BEFORE it's ever fs.access'd/candidate-listed. Candidate (3) is
// inherently safe (path.basename() strips any directory component) and
// is always inside `backupPath`, which is itself always one of the
// allowed roots below.
const candidates = await resolveContainedDbBackupCandidates(
backupPath, dbBackupFile, (msg, meta) => this.log('warn', msg, meta)
);
if (candidates.length === 0) {
throw new Error(
'Database backup file path is not inside a configured backup location. ' +
`Manifest recorded path: ${dbBackupFile}.`
);
}
const candidates = [
// (1) Honour absolute paths recorded by the dumper.
path.isAbsolute(dbBackupFile) ? dbBackupFile : null,
// (2) Relative-to-backupPath as-stored (no basename munging).
path.join(backupPath, dbBackupFile),
// (3) Legacy reconstruct.
path.join(backupPath, 'database', path.basename(dbBackupFile)),
].filter(Boolean);
let dbBackupPath = null;
for (const candidate of candidates) {
@@ -1060,12 +969,7 @@ class RestoreService {
await fs.copyFile(dbPath, currentBackup);
try {
// Restore from backup. `restoreFile` is contained-checked above,
// but the FILENAME component still comes from the manifest — a
// quote in it would break out of the `.restore '<path>'` dot-
// command sqlite3 parses (GHSA-xfvx). Charset-validate right
// before use as the final gate.
assertSafeSqlitePath(restoreFile);
// Restore from backup
await spawnAsync('sqlite3', [dbPath, `.restore '${restoreFile}'`]);
// Verify integrity
@@ -1562,10 +1466,6 @@ END $$;`
if (this.dbType === 'sqlite') {
const dbPath = knexConfig.connection.filename;
// Defense in depth: same dot-command injection surface as the
// main restore path (GHSA-xfvx), even though this path is
// internally generated rather than manifest-controlled.
assertSafeSqlitePath(decompressedPath);
await spawnAsync('sqlite3', [dbPath, `.restore '${decompressedPath}'`]);
} else {
const { host, port, user, password, database } = knexConfig.connection;
@@ -1659,42 +1559,10 @@ END $$;`
throw new Error('Invalid S3 URL format');
}
// SSRF guard: this method calls S3StorageAdapter.download() directly
// rather than going through testConnection(), so it must re-run the same
// DNS-resolving host check testConnection() applies — otherwise an
// admin-configured S3 endpoint could point at a private/internal or
// cloud-metadata address for unauthenticated egress via the server.
// Prod-only, matching S3StorageAdapter's own gate (dev points at
// localhost MinIO deliberately).
//
// A boolean isHostAllowed() preflight is check-then-connect: the AWS
// SDK re-resolves the endpoint hostname on its own when it actually
// connects, so a DNS-rebinding attacker (or an infra rebinding
// condition) could answer the preflight lookup with a public address
// and the SDK's own later lookup with a private/metadata one.
// validateExternalUrlWithAddresses's resolved addresses get pinned
// into the S3Client's requestHandler via pinnedRequestOptions, so the
// connection can only land on an address that was actually vetted.
let pinnedAgents = {};
if (process.env.NODE_ENV === 'production' && s3Config && s3Config.endpoint) {
const { validateExternalUrlWithAddresses } = require('../utils/networkValidation');
const { pinnedRequestOptions } = require('../utils/pinnedRequest');
const endpointUrl = /^https?:\/\//.test(s3Config.endpoint)
? s3Config.endpoint
: `https://${s3Config.endpoint}`;
const urlCheck = await validateExternalUrlWithAddresses(endpointUrl);
if (!urlCheck.valid) {
throw new Error('S3 endpoint resolves to a private or internal network address');
}
const { httpAgent, httpsAgent } = pinnedRequestOptions(urlCheck);
pinnedAgents = { httpAgent, httpsAgent };
}
const [, bucket, key] = s3PathMatch;
const s3Client = new S3StorageAdapter({
...s3Config,
bucket,
...pinnedAgents
bucket
});
await s3Client.download(key, localPath);
@@ -1868,14 +1736,5 @@ const restoreService = new RestoreService();
module.exports = {
restoreService,
RestoreService, // Export class for testing
// Exposed for tests: the manifest `database.backup_file` containment +
// sqlite dot-command charset rules (GHSA-xfvx) are worth pinning directly.
_internal: {
getConfiguredBackupRoots,
isContainedInRoots,
assertSafeSqlitePath,
pathEscapes,
resolveContainedDbBackupCandidates,
},
RestoreService // Export class for testing
};
+3 -15
View File
@@ -118,15 +118,7 @@ const ACTIVE_EVENT_FILTER = {
is_draft: formatBoolean(false)
};
// Same filter minus the draft gate, for admin preview only (#1386). Callers
// MUST authorize before returning anything it matched — see the /resolve
// route, which only reaches for it after isAdminPreview accepts the caller.
const UNPUBLISHED_EVENT_FILTER = {
is_active: formatBoolean(true),
is_archived: formatBoolean(false)
};
const resolveShareIdentifier = async (identifier, { includeDrafts = false } = {}) => {
const resolveShareIdentifier = async (identifier) => {
if (!identifier) {
return null;
}
@@ -148,13 +140,9 @@ const resolveShareIdentifier = async (identifier, { includeDrafts = false } = {}
'event_date',
'expires_at',
'is_active',
'is_archived',
'is_draft',
// Ownership input for the preview check (#1411) — a draft is only
// previewable by an admin who may see this event.
'created_by'
'is_archived'
)
.where(includeDrafts ? UNPUBLISHED_EVENT_FILTER : ACTIVE_EVENT_FILTER);
.where(ACTIVE_EVENT_FILTER);
let event = await baseQuery.clone().where({ slug: trimmed }).first();
if (event) {
+1 -11
View File
@@ -42,10 +42,6 @@ class S3StorageAdapter extends stream.EventEmitter {
* @param {number} [config.retryDelay=1000] - Initial retry delay in milliseconds
* @param {number} [config.connectionTimeout=120000] - Ms to acquire+establish a socket
* @param {number} [config.socketTimeout=60000] - Ms of socket inactivity before a request fails
* @param {http.Agent} [config.httpAgent] - Pre-built http.Agent to pin connections to a
* DNS-resolved address set (see utils/pinnedRequest). Opt-in; when omitted the SDK's
* default agent (its own DNS resolution) is used, matching prior behavior.
* @param {https.Agent} [config.httpsAgent] - Same as httpAgent, for TLS connections.
*/
constructor(config) {
super();
@@ -94,13 +90,7 @@ class S3StorageAdapter extends stream.EventEmitter {
// into a bounded failure, not to enforce latency targets.
requestHandler: {
connectionTimeout: this.config.connectionTimeout,
socketTimeout: this.config.socketTimeout,
// Opt-in DNS pinning (see utils/pinnedRequest): only set when a
// caller explicitly passes agents built from a resolved address
// set. Every other caller leaves these undefined and gets the
// SDK's default agent behavior, unchanged.
...(this.config.httpAgent && { httpAgent: this.config.httpAgent }),
...(this.config.httpsAgent && { httpsAgent: this.config.httpsAgent })
socketTimeout: this.config.socketTimeout
}
};
+3 -63
View File
@@ -7,48 +7,11 @@ const bcrypt = require('bcrypt');
const crypto = require('crypto');
const { db, logActivity } = require('../database/db');
const { formatBoolean } = require('../utils/dbCompat');
const { generateSecurePassword } = require('../utils/passwordGenerator');
const { generateReadablePassword } = require('../utils/passwordGenerator');
const { getBcryptRounds } = require('../utils/passwordValidation');
const { queueEmail } = require('./emailProcessor');
const logger = require('../utils/logger');
const { ConflictError, ForbiddenError, NotFoundError, ValidationError } = require('../utils/errors');
/**
* Guard against granting a role whose permissions exceed the actor's own.
* Super admins may grant any role. Everyone else may only grant permissions
* they already hold themselves (GHSA-rv8w-m6mx-7j4q).
* @param {number} actorId - ID of the admin performing the grant
* @param {string[]} permissionNames - permission names carried by the target role
*/
async function assertActorMayGrant(actorId, permissionNames) {
if (!permissionNames || permissionNames.length === 0) {
return;
}
const actor = await db('admin_users').where('id', actorId).first();
if (!actor) {
throw new NotFoundError('Admin user', actorId);
}
const actorRole = await db('roles').where('id', actor.role_id).first();
if (actorRole && actorRole.name === 'super_admin') {
return;
}
const actorPermissions = await db('role_permissions')
.join('permissions', 'permissions.id', 'role_permissions.permission_id')
.where('role_permissions.role_id', actor.role_id)
.pluck('permissions.name');
const actorPermissionSet = new Set(actorPermissions);
const missing = permissionNames.filter((name) => !actorPermissionSet.has(name));
if (missing.length > 0) {
throw new ForbiddenError(
`You can only grant permissions your own role already holds. Missing: ${missing.join(', ')}`
);
}
}
const { ConflictError, NotFoundError, ValidationError } = require('../utils/errors');
/**
* Create a new admin user invitation
@@ -84,16 +47,6 @@ async function createInvitation({ email, roleId, invitedById, inviterRoleName })
throw new ValidationError('Only Super Admins can invite new Super Admins');
}
// Privilege-escalation guard (GHSA-rv8w-m6mx-7j4q): holding `users.create`
// must not let an actor invite someone into a role carrying permissions
// they don't themselves have — same containment updateAdminUser already
// gives role assignment, reused here for invitations.
const targetRolePermissions = await db('role_permissions')
.join('permissions', 'permissions.id', 'role_permissions.permission_id')
.where('role_permissions.role_id', role.id)
.pluck('permissions.name');
await assertActorMayGrant(invitedById, targetRolePermissions);
// Generate secure invitation token (64 characters hex = 32 bytes)
const token = crypto.randomBytes(32).toString('hex');
const expiresAt = new Date(Date.now() + 7 * 24 * 60 * 60 * 1000); // 7 days
@@ -310,16 +263,6 @@ async function updateAdminUser(id, updates, updatedById, requestingAdmin = {}) {
throw new ValidationError('Only Super Admins can assign the Super Admin role');
}
// Privilege-escalation guard (GHSA-rv8w-m6mx-7j4q): holding `users.edit`
// must not let an actor hand out a role carrying permissions they don't
// themselves have — same containment assertActorMayGrant already gives
// `roles.manage` for role create/edit, reused here for role assignment.
const targetRolePermissions = await db('role_permissions')
.join('permissions', 'permissions.id', 'role_permissions.permission_id')
.where('role_permissions.role_id', role.id)
.pluck('permissions.name');
await assertActorMayGrant(updatedById, targetRolePermissions);
// Prevent self-role-update
if (id === updatedById) {
throw new ValidationError('Cannot change your own role');
@@ -516,10 +459,7 @@ async function resetAdminPassword(id, resetById) {
throw new NotFoundError('Admin user', id);
}
// GHSA-h4w8-57xq-53fx: this password is emailed to the admin and is a live
// credential until they change it, so it needs real entropy — not the
// ~2^21 wordlist-based generateReadablePassword() used for gallery resets.
const newPassword = generateSecurePassword(16);
const newPassword = generateReadablePassword();
const passwordHash = await bcrypt.hash(newPassword, getBcryptRounds());
await db('admin_users').where('id', id).update({
+19 -95
View File
@@ -32,10 +32,7 @@ async function extractVideoMetadata(videoPath) {
const audioStream = metadata.streams.find(s => s.codec_type === 'audio');
const result = {
// null (not 0) when ffprobe genuinely has no duration — a real
// 0-second clip and "unknown" must stay distinguishable, since
// downstream code treats `duration != null` as "trust this value".
duration: metadata.format.duration != null ? Math.floor(metadata.format.duration) : null,
duration: Math.floor(metadata.format.duration || 0),
width: videoStream?.width || null,
height: videoStream?.height || null,
videoCodec: videoStream?.codec_name || null,
@@ -133,108 +130,35 @@ async function getVideoDuration(videoPath) {
* Process an uploaded video: extract metadata and produce a thumbnail through
* the storage backend.
*
* Metadata extraction and thumbnail generation are independent, best-effort
* steps mirroring how the image pipeline treats thumbnail/dimension/EXIF
* failures (log a warning, keep the upload). This used to gate everything
* behind isValidVideo(), which rejects the whole video if ffprobe can't read
* even one of duration/width/height common on some iPhone/Lightroom-
* exported MP4s (#1370). Callers (photoProcessor.js's processPhoto and
* processUploadedPhotos) already catch that throw and fall back to a static
* placeholder thumbnail plus a metadata-only retry (codex review of #845),
* but that fallback never got a REAL thumbnail even when
* generateVideoThumbnail() would have succeeded on its own thumbnailing
* doesn't need valid duration/width/height, it just seeks and grabs a frame.
* Trying both steps independently means a real thumbnail (and whatever
* metadata ffprobe *can* read) survives far more often. metadata is still
* allowed to come back null (ffprobe failed) a video with no thumbnail
* would fall back to rendering the raw video as an <img> in the gallery
* grid (`photo.thumbnail_url || photo.url`), so this only resolves when a
* real thumbnail or the SVG placeholder produced *something*; if both fail
* (storage backend down, disk full not a quirk of one file) it throws
* instead, so the caller surfaces a retryable failure rather than silently
* completing with nothing to show.
*
* @param {string} videoPath - Local path to the source video (ffmpeg requires fs).
* @param {string} thumbnailKey - Relative storage key for the thumbnail.
* @returns {Promise<{success: boolean, metadata: Object|null, thumbnailKey: string}>}
* @returns {Promise<{success: boolean, metadata: Object, thumbnailKey: string}>}
*/
async function processUploadedVideo(videoPath, thumbnailKey, options = {}) {
let metadata = null;
try {
metadata = await extractVideoMetadata(videoPath);
} catch (error) {
logger.error('Video metadata extraction failed — continuing without duration/codec/dimensions', {
error: error.message,
videoPath
});
}
const isValid = await isValidVideo(videoPath);
if (!isValid) {
throw new Error('Invalid video file');
}
let generatedThumbnailKey = null;
try {
const metadata = await extractVideoMetadata(videoPath);
await generateVideoThumbnail(videoPath, thumbnailKey, options);
const storage = getStorage();
if (await storage.exists(thumbnailKey)) {
generatedThumbnailKey = thumbnailKey;
const exists = await storage.exists(thumbnailKey);
if (!exists) {
throw new Error('Thumbnail generation failed (not in storage)');
}
return {
success: true,
metadata,
thumbnailKey
};
} catch (error) {
logger.error('Video thumbnail generation failed — continuing without a thumbnail', {
error: error.message,
videoPath
});
logger.error('Error processing video', { error: error.message, videoPath });
throw error;
}
// Never return "success" with no thumbnail at all: the gallery grid
// (GridGalleryLayout/JustifiedGalleryLayout) falls back to
// `photo.thumbnail_url || photo.url` when there's no thumbnail, which
// makes AuthenticatedImage download the full ORIGINAL VIDEO and try to
// render it as an <img> — a broken tile and a multi-GB fetch just from
// opening the gallery (codex review, #1371/#1372). Fall back to the same
// ffmpeg-free SVG placeholder the callers already generate for a total
// processing failure, so a bare thumbnail-generation failure degrades to
// that placeholder too, not to "no thumbnail". thumbnailKey is always
// `thumbnails/thumb_<name>.jpg` (see callers) — strip the prefix back to
// a filename so generateVideoPlaceholder recomputes this exact same key.
if (!generatedThumbnailKey) {
try {
const {
generateVideoPlaceholder,
DEFAULT_THUMBNAIL_WIDTH,
DEFAULT_THUMBNAIL_HEIGHT
} = require('./imageProcessor');
const placeholderFilename = path.basename(thumbnailKey).replace(/^thumb_/, '');
// Explicit width/height make generateVideoPlaceholder skip its
// configured-thumbnail-size DB lookup (see its own comment) — this
// call can run from inside processUploadedPhotos' open per-file
// SQLite transaction, where that lookup would otherwise deadlock.
const placeholderKey = await generateVideoPlaceholder(placeholderFilename, {
width: DEFAULT_THUMBNAIL_WIDTH,
height: DEFAULT_THUMBNAIL_HEIGHT
});
if (placeholderKey) {
generatedThumbnailKey = placeholderKey;
}
} catch (error) {
logger.error('Video placeholder generation also failed', { error: error.message, videoPath });
}
}
// A real thumbnail AND the ffmpeg-free SVG placeholder both failing points
// at something systemic (storage backend down, disk full) rather than a
// quirk of this one file — that's worth surfacing as a retryable failure
// rather than silently completing with no thumbnail at all, which would
// make the gallery fall back to rendering the raw video as an <img>
// (codex review, #1371/#1372). Metadata (if any was extracted) is lost
// here, same trade-off the callers' own pre-existing total-failure
// handling already makes.
if (!generatedThumbnailKey) {
throw new Error('Unable to generate any thumbnail (real or placeholder) for this video');
}
return {
success: true,
metadata,
thumbnailKey: generatedThumbnailKey
};
}
/**
-100
View File
@@ -1,100 +0,0 @@
/**
* Bounded, reclaimable storage reads for archiver-based downloads.
*
* archiver consumes the sources it is handed one at a time. Appending a
* storage read per photo in a tight loop therefore opens N reads and drains
* one, and every other one parks an S3 socket holding megabytes of unread
* body. Nothing reclaims them on its own: archiver's abort() does not touch
* its source streams, and the SDK arms its socket timeout on a 3s delay and
* clears it the moment response headers land, so a fast response never gets
* one at all.
*
* That is the shape of the incident in PR #1402 43 of 50 pooled sockets
* ESTABLISHED with unread bytes, uploads and gallery reads starved behind
* them, a process restart the only way out. #1402 fixes the cached-zip
* builder. This is the same guard for the other three call sites, two of
* which a gallery guest can reach with no admin credentials at all.
*
* Local-filesystem installs are unaffected they take archiver's
* `archive.file(path)` branch and open no sockets which is most likely why
* this went unnoticed for so long.
*/
// Two in flight: one being drained, one ready to go. Enough to keep archiver
// fed, few enough that a build cannot monopolise the agent pool.
const DEFAULT_MAX_IN_FLIGHT = 2;
function createArchiveStreamGuard({ maxInFlight = DEFAULT_MAX_IN_FLIGHT, onFatalError } = {}) {
const openReads = new Set();
let waiter = null;
let closed = false;
const wake = () => {
if (!waiter) return;
const resume = waiter;
waiter = null;
resume();
};
const release = (stream) => {
openReads.delete(stream);
wake();
};
return {
/** Park until a read slot frees up. Returns false once destroyAll ran. */
async acquire() {
while (!closed && openReads.size >= maxInFlight) {
await new Promise((resolve) => { waiter = resolve; });
}
return !closed;
},
/** Register a stream and hand it straight back, for inline use. */
track(stream) {
if (closed) {
stream.destroy();
return stream;
}
openReads.add(stream);
stream.once('end', () => release(stream));
stream.once('close', () => release(stream));
stream.once('error', (err) => {
release(stream);
// A stream that errors while still QUEUED behind another has no
// archiver listener on it yet, so archiver never learns it failed.
// Absorbing the error here and leaving the dead stream in the queue
// makes the archive hang forever when it reaches it — and in
// downloadJobService the build keeps its slot with it. Hand the
// failure to the caller, which aborts the archive.
if (!closed && typeof onFatalError === 'function') {
onFatalError(err);
}
});
return stream;
},
/**
* Destroy every read still holding bytes. Safe to call more than once
* the exit paths overlap (client disconnect and an error can both fire).
*/
destroyAll() {
closed = true;
for (const stream of openReads) {
try {
stream.destroy();
} catch {
// Already gone; nothing to reclaim.
}
}
openReads.clear();
wake();
},
get openCount() {
return openReads.size;
},
};
}
module.exports = { createArchiveStreamGuard, DEFAULT_MAX_IN_FLIGHT };
+1 -49
View File
@@ -244,52 +244,4 @@ async function validateExternalUrlAsync(urlString) {
return { valid: true, reason: 'ok' };
}
/**
* Same DNS-resolving vetting as classifyHost, but also returns the exact
* addresses that were checked the piece classifyHost intentionally
* discards. Needed by any caller that then wants to PIN its connection to
* those addresses (utils/pinnedRequest.js) rather than trust a second,
* independent resolution done later by the underlying client closing the
* TOCTOU/DNS-rebinding gap classifyHost's own doc comment calls out as
* residual risk. Purely additive: existing classifyHost/validateExternalUrlAsync
* callers and their return shapes are untouched.
* @param {string} urlString
* @returns {Promise<{ valid: boolean, error?: string, reason: string, hostname?: string, addresses?: Array<{address: string, family: number}> }>}
*/
async function validateExternalUrlWithAddresses(urlString) {
let parsed;
try {
parsed = new URL(urlString);
} catch {
return { valid: false, error: 'Invalid URL format', reason: 'invalid' };
}
const hostname = parsed.hostname.replace(/^\[|\]$/g, '');
if (isPrivateIP(parsed.hostname)) {
return { valid: false, error: 'URL points to a private or internal network address', reason: 'private' };
}
if (net.isIP(hostname)) {
return { valid: true, reason: 'ok', hostname, addresses: [{ address: hostname, family: net.isIP(hostname) }] };
}
let addresses;
try {
addresses = await dns.lookup(parsed.hostname, { all: true });
} catch {
return { valid: false, error: 'URL points to a private or internal network address', reason: 'unresolved' };
}
if (!addresses.length) {
return { valid: false, error: 'URL points to a private or internal network address', reason: 'unresolved' };
}
if (addresses.some((a) => isPrivateIP(a.address))) {
return { valid: false, error: 'URL points to a private or internal network address', reason: 'private' };
}
return { valid: true, reason: 'ok', hostname, addresses };
}
module.exports = {
isPrivateIP,
validateExternalUrl,
isHostAllowed,
validateExternalUrlAsync,
validateExternalUrlWithAddresses,
classifyHost,
};
module.exports = { isPrivateIP, validateExternalUrl, isHostAllowed, validateExternalUrlAsync, classifyHost };
-28
View File
@@ -1,28 +0,0 @@
/** Axios/Node lookup: connect only to the addresses vetted for this delivery.
* Keep the original URL for Host, TLS SNI and certificate verification.
* Disable environment proxies (which would resolve the destination themselves)
* and redirects. No reusable agent/socket can carry an old DNS decision.
*/
const http = require('http');
const https = require('https');
function pinnedRequestOptions(check) {
if (!check?.valid || !check.hostname || !check.addresses?.length) {
throw new Error('A validated destination is required');
}
const addresses = check.addresses.map(({ address, family }) => ({ address, family }));
const lookup = (hostname, options, callback) => {
if (typeof options === 'function') { callback = options; options = {}; }
if (hostname !== check.hostname) return callback(new Error('Destination hostname changed'));
const family = typeof options === 'number' ? options : options?.family;
const matches = family ? addresses.filter(a => a.family === family) : addresses;
if (!matches.length) return callback(new Error('No validated address for requested family'));
if (options?.all) return callback(null, matches);
callback(null, matches[0].address, matches[0].family);
};
return {
proxy: false, maxRedirects: 0,
httpAgent: new http.Agent({ lookup, keepAlive: false }),
httpsAgent: new https.Agent({ lookup, keepAlive: false }),
};
}
module.exports = { pinnedRequestOptions };
+1 -1
View File
@@ -1,7 +1,7 @@
{
"name": "picpeak-frontend",
"private": true,
"version": "3.46.12",
"version": "3.46.10",
"type": "module",
"scripts": {
"dev": "vite",
@@ -22,13 +22,7 @@ export const HeaderStyleCard: React.FC<HeaderStyleCardProps> = ({ localTheme, ha
<p className="text-sm text-neutral-600 dark:text-neutral-400 mb-4">
{t('branding.headerStyleDescription', 'Choose how the gallery header appears. The header style is independent of the photo layout.')}
</p>
{/* auto-fit/minmax rather than viewport breakpoints (#1412): the
breakpoints size the columns off the WINDOW, but this card sits in a
settings panel that is far narrower, so `lg:grid-cols-3` produced
three ~85px columns no German string could fit in. A minimum track
width lets the column count follow the container instead, and drops
to fewer columns when there is no room. */}
<div className="grid grid-cols-[repeat(auto-fit,minmax(9rem,1fr))] gap-4">
<div className="grid grid-cols-1 sm:grid-cols-2 lg:grid-cols-3 gap-4">
{(Object.keys(headerStyleIcons) as HeaderStyleType[]).map((style) => (
<button
type="button"
@@ -40,21 +34,14 @@ export const HeaderStyleCard: React.FC<HeaderStyleCardProps> = ({ localTheme, ha
: 'border-neutral-200 dark:border-neutral-700 hover:border-neutral-300 dark:hover:border-neutral-600'
}`}
>
{/* min-w-0 + break-words: a grid item will not shrink below its
min-content width, and German compounds here are long enough to
exceed a narrow column "Veranstaltungsinfo-Overlay" and
"Veranstaltungsdetails" spilled out of the card and over the
neighbouring one at three columns in a narrow panel (#1412).
Any translation can do this, so the constraint belongs on the
element rather than on the strings. */}
<div className="flex flex-col items-center text-center w-full min-w-0">
<div className="flex flex-col items-center text-center">
<div className="mb-2 text-neutral-700 dark:text-neutral-300">
{headerStyleIcons[style]}
</div>
<span className="w-full break-words font-medium text-sm capitalize text-neutral-900 dark:text-neutral-100">
<span className="font-medium text-sm capitalize text-neutral-900 dark:text-neutral-100">
{t(`branding.headerStyleOptions.${style}`, style)}
</span>
<span className="w-full break-words text-xs text-neutral-600 dark:text-neutral-400 mt-1">
<span className="text-xs text-neutral-600 dark:text-neutral-400 mt-1">
{t(`branding.headerStyleDescriptions.${style}`, '')}
</span>
</div>
@@ -74,7 +61,7 @@ export const HeaderStyleCard: React.FC<HeaderStyleCardProps> = ({ localTheme, ha
<p className="text-xs text-neutral-600 dark:text-neutral-400 mb-4">
{t('branding.heroDividerDescription', 'Choose how the transition between the hero image and gallery content looks.')}
</p>
<div className="grid grid-cols-[repeat(auto-fit,minmax(6rem,1fr))] gap-3">
<div className="grid grid-cols-2 sm:grid-cols-3 lg:grid-cols-5 gap-3">
{(Object.keys(dividerStylePreviews) as HeroDividerStyle[]).map((divider) => (
<button
type="button"
@@ -86,12 +73,12 @@ export const HeaderStyleCard: React.FC<HeaderStyleCardProps> = ({ localTheme, ha
: 'border-neutral-200 dark:border-neutral-700 hover:border-neutral-300 dark:hover:border-neutral-600'
}`}
>
<div className="flex flex-col items-center w-full min-w-0">
<div className="flex flex-col items-center">
<div className="w-full mb-2 bg-neutral-800 rounded-t overflow-hidden">
<div className="h-8"></div>
{dividerStylePreviews[divider]}
</div>
<span className="w-full break-words text-xs font-medium capitalize text-neutral-900 dark:text-neutral-100">
<span className="text-xs font-medium capitalize text-neutral-900 dark:text-neutral-100">
{t(`branding.dividerOptions.${divider}`, divider)}
</span>
</div>
@@ -1,6 +1,5 @@
import React, { useState, useEffect, useRef, useCallback } from 'react';
import { buildResourceUrl } from '../../utils/url';
import { withAdminPreview } from '../../utils/adminPreview';
import {
getActiveGallerySlug,
getGalleryToken,
@@ -141,15 +140,11 @@ export const AuthenticatedImage: React.FC<AuthenticatedImageProps> = ({
// Build full URL for the image. Only relative paths are app-owned;
// an absolute URL is passed through untouched.
const isRelative = rawUrl.startsWith('/');
// Flag goes on while the URL is still relative: buildResourceUrl can
// return an absolute URL in split deployments, and withAdminPreview
// deliberately refuses those (#1386).
const previewUrl = isRelative ? withAdminPreview(rawUrl) : rawUrl;
const fullImageUrl = previewUrl.startsWith('/admin')
? buildResourceUrl(`/api${previewUrl}`)
const fullImageUrl = rawUrl.startsWith('/admin')
? buildResourceUrl(`/api${rawUrl}`)
: isRelative
? buildResourceUrl(previewUrl)
: previewUrl;
? buildResourceUrl(rawUrl)
: rawUrl;
const headers: Record<string, string> = {};
// Attach the gallery bearer token ONLY to relative (same-app) image
@@ -1,7 +1,5 @@
import React, { useRef, useState, useEffect } from 'react';
import { useTranslation } from 'react-i18next';
import { Play, Pause, Volume2, VolumeX, Maximize, Minimize, AlertTriangle } from 'lucide-react';
import { withAdminPreview } from '../../utils/adminPreview';
import { Play, Pause, Volume2, VolumeX, Maximize, Minimize } from 'lucide-react';
interface VideoPlayerProps {
src: string;
@@ -26,10 +24,8 @@ export const VideoPlayer: React.FC<VideoPlayerProps> = ({
width = '100%',
height = 'auto'
}) => {
const { t } = useTranslation();
const videoRef = useRef<HTMLVideoElement>(null);
const [isPlaying, setIsPlaying] = useState(false);
const [loadError, setLoadError] = useState<string | null>(null);
const [isMuted, setIsMuted] = useState(muted);
const [isFullscreen, setIsFullscreen] = useState(false);
const [progress, setProgress] = useState(0);
@@ -55,28 +51,11 @@ export const VideoPlayer: React.FC<VideoPlayerProps> = ({
const handlePause = () => setIsPlaying(false);
const handleEnded = () => setIsPlaying(false);
// Without this the element just sits on its poster at 0:00 and says
// nothing (#1370) — a guest cannot tell a failed request from a codec
// their browser will not decode, and neither could we from their report.
// MEDIA_ERR_SRC_NOT_SUPPORTED is the one worth naming: it is what an
// HEVC/H.265 phone recording does everywhere except Safari, and the
// photographer's answer is to download the file rather than retry.
const handleError = () => {
const code = video.error?.code;
setLoadError(
code === MediaError.MEDIA_ERR_SRC_NOT_SUPPORTED
? t('gallery.videoFormatUnsupported', 'This video format cannot be played in this browser. Download it to watch it.')
: t('gallery.videoLoadFailed', 'This video could not be loaded.')
);
setIsPlaying(false);
};
video.addEventListener('timeupdate', handleTimeUpdate);
video.addEventListener('loadedmetadata', handleLoadedMetadata);
video.addEventListener('play', handlePlay);
video.addEventListener('pause', handlePause);
video.addEventListener('ended', handleEnded);
video.addEventListener('error', handleError);
return () => {
video.removeEventListener('timeupdate', handleTimeUpdate);
@@ -84,15 +63,8 @@ export const VideoPlayer: React.FC<VideoPlayerProps> = ({
video.removeEventListener('play', handlePlay);
video.removeEventListener('pause', handlePause);
video.removeEventListener('ended', handleEnded);
video.removeEventListener('error', handleError);
};
}, [t]);
// Arrowing to the next video in the lightbox reuses this element, so a
// stale error would otherwise stick to a clip that loads fine.
useEffect(() => {
setLoadError(null);
}, [src]);
}, []);
const togglePlayPause = () => {
const video = videoRef.current;
@@ -177,13 +149,10 @@ export const VideoPlayer: React.FC<VideoPlayerProps> = ({
onMouseMove={handleMouseMove}
onMouseLeave={() => isPlaying && setShowControls(false)}
>
{/* A bare <video src> never touches the axios interceptor, so a draft
preview needs the flag on the URL itself (#1386) otherwise the
gallery renders and the video 404s. */}
<video
ref={videoRef}
src={withAdminPreview(src)}
poster={poster ? withAdminPreview(poster) : poster}
src={src}
poster={poster}
autoPlay={autoPlay}
muted={muted}
loop={loop}
@@ -192,16 +161,7 @@ export const VideoPlayer: React.FC<VideoPlayerProps> = ({
onClick={togglePlayPause}
/>
{loadError && (
<div className="absolute inset-0 flex items-center justify-center bg-black/70 p-6 text-center">
<div className="flex flex-col items-center gap-2 text-white">
<AlertTriangle size={28} />
<span className="text-sm max-w-xs">{loadError}</span>
</div>
</div>
)}
{controls && !loadError && (
{controls && (
<div
className={`absolute bottom-0 left-0 right-0 bg-gradient-to-t from-black/80 to-transparent p-4 transition-opacity duration-300 ${
showControls ? 'opacity-100' : 'opacity-0'
@@ -254,7 +214,7 @@ export const VideoPlayer: React.FC<VideoPlayerProps> = ({
)}
{/* Play button overlay when paused */}
{!isPlaying && showControls && !loadError && (
{!isPlaying && showControls && (
<div className="absolute inset-0 flex items-center justify-center">
<button
onClick={togglePlayPause}
@@ -1,80 +0,0 @@
/**
* A video that fails to load has to say so (#1370).
*
* The element carried no `error` listener, so every failure an unplayable
* codec, a 403, a missing file rendered identically: the poster frame, the
* transport stuck at "0:00 / 0:00", and a play button that did nothing. The
* reporter could not tell us which one they had hit, and neither could we.
*
* MEDIA_ERR_SRC_NOT_SUPPORTED gets its own wording because it is by far the
* most common cause in practice (HEVC/H.265 phone footage plays in Safari and
* nowhere else) and the useful advice for it download the file is not the
* advice for a transport failure.
*/
import { describe, it, expect, beforeAll } from 'vitest';
import { render, screen, fireEvent } from '@testing-library/react';
import { VideoPlayer } from '../VideoPlayer';
function failWith(video: HTMLVideoElement, code: number) {
Object.defineProperty(video, 'error', { value: { code }, configurable: true });
fireEvent.error(video);
}
describe('VideoPlayer load errors (#1370)', () => {
beforeAll(() => {
// jsdom implements HTMLMediaElement but not the MediaError constants.
if (typeof MediaError === 'undefined') {
(globalThis as unknown as { MediaError: unknown }).MediaError = {
MEDIA_ERR_ABORTED: 1,
MEDIA_ERR_NETWORK: 2,
MEDIA_ERR_DECODE: 3,
MEDIA_ERR_SRC_NOT_SUPPORTED: 4,
};
}
});
it('shows the transport controls while nothing has gone wrong', () => {
const { container } = render(<VideoPlayer src="/api/gallery/e/photo/1" />);
expect(container.querySelector('video')).toBeInTheDocument();
// Two: the transport button and the centre overlay.
expect(screen.getAllByLabelText('Play')).toHaveLength(2);
expect(screen.getByText('0:00 / 0:00')).toBeInTheDocument();
});
it('names the codec case and points at the download', () => {
const { container } = render(<VideoPlayer src="/api/gallery/e/photo/1" />);
failWith(container.querySelector('video')!, MediaError.MEDIA_ERR_SRC_NOT_SUPPORTED);
expect(screen.getByText(/cannot be played in this browser/i)).toBeInTheDocument();
expect(screen.getByText(/Download it/i)).toBeInTheDocument();
});
it('reports a transport failure without blaming the format', () => {
const { container } = render(<VideoPlayer src="/api/gallery/e/photo/1" />);
failWith(container.querySelector('video')!, MediaError.MEDIA_ERR_NETWORK);
expect(screen.getByText(/could not be loaded/i)).toBeInTheDocument();
expect(screen.queryByText(/cannot be played in this browser/i)).not.toBeInTheDocument();
});
it('hides the 0:00 transport, which only ever misled', () => {
const { container } = render(<VideoPlayer src="/api/gallery/e/photo/1" />);
failWith(container.querySelector('video')!, MediaError.MEDIA_ERR_SRC_NOT_SUPPORTED);
expect(screen.queryByText('0:00 / 0:00')).not.toBeInTheDocument();
// Including the centre overlay, which was the dead play button in the
// screenshots on the issue.
expect(screen.queryAllByLabelText('Play')).toHaveLength(0);
});
it('clears the error when the lightbox arrows to the next video', () => {
const { container, rerender } = render(<VideoPlayer src="/api/gallery/e/photo/1" />);
failWith(container.querySelector('video')!, MediaError.MEDIA_ERR_SRC_NOT_SUPPORTED);
expect(screen.getByText(/cannot be played in this browser/i)).toBeInTheDocument();
rerender(<VideoPlayer src="/api/gallery/e/photo/2" />);
expect(screen.queryByText(/cannot be played in this browser/i)).not.toBeInTheDocument();
expect(screen.getByText('0:00 / 0:00')).toBeInTheDocument();
});
});
-10
View File
@@ -115,16 +115,6 @@ api.interceptors.request.use(
}
}
}
// Admin draft preview (#1386). The gallery tab was opened with
// ?admin_preview=1; forward that intent flag on every gallery API call
// so the backend applies the draft bypass. The HttpOnly admin_token
// cookie authenticates it server-side — no credential in the URL.
// Harmless for guests: without a valid admin cookie the check fails
// closed and they get exactly what they got before.
if (new URLSearchParams(window.location.search).get('admin_preview') === '1') {
config.params = { ...(config.params as Record<string, unknown> | undefined), admin_preview: 1 };
}
}
}
@@ -249,24 +249,6 @@ export const GalleryAuthProvider: React.FC<GalleryAuthProviderProps> = ({ childr
if (routeInfo.token) {
const verify = await galleryService.verifyToken(currentSlug, routeInfo.token);
if (verify?.valid) {
// An admin preview does not take a guest session (#1386). The admin
// cookie plus admin_preview=1 already authorizes every gallery call,
// and shareLinkLogin refuses drafts AND records a failed attempt
// when it does — so opening a draft preview five times would lock
// share-link logins out for that IP, even after publishing.
const isAdminPreview = typeof window !== 'undefined'
&& new URLSearchParams(window.location.search).get('admin_preview') === '1';
if (isAdminPreview) {
const previewData = await galleryService.getGalleryPhotos(currentSlug);
if (previewData?.event) {
const previewEvent = normalizeEvent(previewData.event);
setEvent(previewEvent);
setActiveGallerySlug(currentSlug);
setIsAuthenticated(true);
return;
}
}
const response = await authService.shareLinkLogin(currentSlug, routeInfo.token);
if (response?.event) {
// Store token and slug BEFORE setting authenticated state to avoid
+1 -3
View File
@@ -931,9 +931,7 @@
},
"photosCount_one": "{{count}} Foto",
"photosCount_other": "{{count}} Fotos",
"poweredBy": "Bereitgestellt von PicPeak",
"videoFormatUnsupported": "Dieses Videoformat kann in diesem Browser nicht abgespielt werden. Laden Sie es herunter, um es anzusehen.",
"videoLoadFailed": "Dieses Video konnte nicht geladen werden."
"poweredBy": "Bereitgestellt von PicPeak"
},
"categories": {
"title": "Fotokategorien",
+1 -3
View File
@@ -472,9 +472,7 @@
"photosSelected_one": "{{count}} photo selected",
"photosSelected_other": "{{count}} photos selected",
"downloadSelected_one": "Download {{count}} photo",
"downloadSelected_other": "Download {{count}} photos",
"videoFormatUnsupported": "This video format cannot be played in this browser. Download it to watch it.",
"videoLoadFailed": "This video could not be loaded."
"downloadSelected_other": "Download {{count}} photos"
},
"categories": {
"title": "Photo Categories",
+1 -3
View File
@@ -313,9 +313,7 @@
"anonymous": "Anónimo"
},
"rated": "Valorado",
"commented": "Comentado",
"videoFormatUnsupported": "Este formato de vídeo no se puede reproducir en este navegador. Descárgalo para verlo.",
"videoLoadFailed": "No se ha podido cargar este vídeo."
"commented": "Comentado"
},
"categories": {
"title": "Categorías de fotos",
+1 -3
View File
@@ -334,9 +334,7 @@
"photosSelected_one": "{{count}} photo sélectionnée",
"photosSelected_other": "{{count}} photos sélectionnées",
"downloadSelected_one": "Télécharger {{count}} photo",
"downloadSelected_other": "Télécharger {{count}} photos",
"videoFormatUnsupported": "Ce format vidéo ne peut pas être lu dans ce navigateur. Téléchargez-la pour la regarder.",
"videoLoadFailed": "Cette vidéo n'a pas pu être chargée."
"downloadSelected_other": "Télécharger {{count}} photos"
},
"categories": {
"title": "Catégories de photos",
+1 -3
View File
@@ -334,9 +334,7 @@
},
"photosCount_one": "{{count}} foto",
"photosCount_other": "{{count}} foto's",
"poweredBy": "Mogelijk gemaakt door PicPeak",
"videoFormatUnsupported": "Deze video-indeling kan niet in deze browser worden afgespeeld. Download de video om hem te bekijken.",
"videoLoadFailed": "Deze video kon niet worden geladen."
"poweredBy": "Mogelijk gemaakt door PicPeak"
},
"categories": {
"title": "Fotocategorieen",
+1 -3
View File
@@ -342,9 +342,7 @@
"photosCount_many": "{{count}} fotos",
"photosCount_one": "{{count}} foto",
"photosCount_other": "{{count}} fotos",
"poweredBy": "Desenvolvido por PicPeak",
"videoFormatUnsupported": "Este formato de vídeo não pode ser reproduzido neste navegador. Baixe o arquivo para assistir.",
"videoLoadFailed": "Não foi possível carregar este vídeo."
"poweredBy": "Desenvolvido por PicPeak"
},
"categories": {
"title": "Categorias de Fotos",
+1 -3
View File
@@ -350,9 +350,7 @@
"photosCount_many": "{{count}} фото",
"photosCount_one": "{{count}} фото",
"photosCount_other": "{{count}} фото",
"poweredBy": "Работает на PicPeak",
"videoFormatUnsupported": "Этот формат видео не воспроизводится в этом браузере. Скачайте файл, чтобы посмотреть его.",
"videoLoadFailed": "Не удалось загрузить это видео."
"poweredBy": "Работает на PicPeak"
},
"categories": {
"title": "Категории фото",
+1 -3
View File
@@ -334,9 +334,7 @@
"photosSelected_one": "Izbrana {{count}} fotografija",
"photosSelected_other": "Izbranih {{count}} fotografij",
"downloadSelected_one": "Prenesi {{count}} fotografijo",
"downloadSelected_other": "Prenesi {{count}} fotografij",
"videoFormatUnsupported": "Tega videoformata v tem brskalniku ni mogoče predvajati. Prenesite ga, da si ga ogledate.",
"videoLoadFailed": "Tega videa ni bilo mogoče naložiti."
"downloadSelected_other": "Prenesi {{count}} fotografij"
},
"categories": {
"title": "Kategorije fotografij",
@@ -19,6 +19,7 @@ import type { Event } from '../../../types';
import { Button, Card } from '../../../components/common';
import { useLocalizedDate } from '../../../hooks/useLocalizedDate';
import { useFeatureFlags } from '../../../contexts/FeatureFlagsContext';
import { eventsService } from '../../../services/events.service';
import { buildShareLinkUrl } from '../../../utils/url';
import { isGalleryPublic } from '../../../utils/accessControl';
import type { FeedbackSettings as FeedbackSettingsType } from '../../../services/feedback.service';
@@ -190,7 +191,7 @@ export const EventDetailsHeader: React.FC<EventDetailsHeaderProps> = ({
{event.share_link && !isEditing && (
<a
href={event.is_draft
? `${buildShareLinkUrl(event.share_link)}${buildShareLinkUrl(event.share_link).includes('?') ? '&' : '?'}admin_preview=1`
? `${buildShareLinkUrl(event.share_link)}${buildShareLinkUrl(event.share_link).includes('?') ? '&' : '?'}preview=${eventsService.getPreviewToken() || ''}`
: buildShareLinkUrl(event.share_link)
}
target="_blank"
+5
View File
@@ -284,6 +284,11 @@ export const eventsService = {
},
// Get admin preview token (uses existing admin session token)
getPreviewToken(): string | null {
const token = sessionStorage.getItem('admin_token') || localStorage.getItem('admin_token');
return token;
},
// Rename event
async renameEvent(eventId: number, newEventName: string, resendEmail: boolean = false): Promise<{
success: boolean;
+2 -8
View File
@@ -2,7 +2,6 @@ import { api } from '../config/api';
import type { GalleryInfo, GalleryData, GalleryStats, ResolvedGalleryIdentifier } from '../types';
import { normalizeRequirePassword } from '../utils/accessControl';
import { parseContentDispositionFilename } from '../utils/contentDisposition';
import { withAdminPreview } from '../utils/adminPreview';
// iOS is the only platform whose system share sheet exposes a
// first-party "Save Image" / "Save to Photos" action for files
@@ -89,12 +88,7 @@ export const galleryService = {
async savePhotoToDevice(slug: string, photoId: number, filename: string): Promise<void> {
if (!isIOS()) {
this.triggerDirectDownload(
// Native anchor download: bypasses the axios interceptor, so a draft
// preview needs the flag on the URL itself (#1386). Applied to the
// relative path BEFORE getUri: with an absolute VITE_API_URL getUri
// returns an absolute URL, and withAdminPreview refuses those by
// design, which would silently drop the flag.
api.getUri({ url: withAdminPreview(`/gallery/${slug}/download/${photoId}`) }),
api.getUri({ url: `/gallery/${slug}/download/${photoId}` }),
filename,
);
return;
@@ -221,7 +215,7 @@ export const galleryService = {
// Native browser download — the server sends Content-Length so
// the browser shows a real progress bar and mobile doesn't crash.
const link = document.createElement('a');
link.href = withAdminPreview(`/api/gallery/${slug}/download-all`);
link.href = `/api/gallery/${slug}/download-all`;
link.setAttribute('download', `${slug}.zip`);
document.body.appendChild(link);
link.click();
-23
View File
@@ -1,23 +0,0 @@
/**
* Admin draft preview (#1386).
*
* The gallery tab is opened with `?admin_preview=1`, and the axios interceptor
* forwards that flag on every gallery API call. Anything that does NOT go
* through axios a native `fetch`, a `<video src>`, an `<a href>` download
* has to put the flag on its own URL, or `verifyGalleryAccess` filters the
* unpublished event out and answers 404.
*
* The flag is an intent signal only: the admin's HttpOnly `admin_token` cookie
* is what actually authenticates it, and it rides along on its own because all
* of these are same-origin. No credential is ever placed in a URL.
*/
export function withAdminPreview(url: string | null | undefined): string {
if (!url) return url || '';
// Relative (app-owned) URLs only. Never append to an absolute URL: that
// could point at any origin, and the flag would be a hint to a third party
// about what the admin is doing.
if (!url.startsWith('/')) return url;
if (typeof window === 'undefined') return url;
if (new URLSearchParams(window.location.search).get('admin_preview') !== '1') return url;
return `${url}${url.includes('?') ? '&' : '?'}admin_preview=1`;
}