Compare commits
2 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
 Paul Nothaft
|
c5907a0833 | ||
|
Paul Nothaft
|
f13d163c74 |
@@ -1,3 +1,3 @@
|
||||
{
|
||||
".": "3.131.7-beta.0"
|
||||
".": "3.131.3-beta.0"
|
||||
}
|
||||
|
||||
@@ -5,50 +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.131.7-beta.0](https://github.com/PicPeak/picpeak/compare/v3.131.6-beta.0...v3.131.7-beta.0) (2026-09-11)
|
||||
|
||||
|
||||
### Bug Fixes
|
||||
|
||||
* **admin:** keep header-style tiles from overflowing their cards ([#1422](https://github.com/PicPeak/picpeak/issues/1422)) ([7cd7654](https://github.com/PicPeak/picpeak/commit/7cd7654f99967080724d7498b72bea4986ae825d))
|
||||
|
||||
## [3.131.6-beta.0](https://github.com/PicPeak/picpeak/compare/v3.131.5-beta.0...v3.131.6-beta.0) (2026-09-11)
|
||||
|
||||
|
||||
### Bug Fixes
|
||||
|
||||
* **gallery:** cap how many cached zips rebuild at once in the background ([#1418](https://github.com/PicPeak/picpeak/issues/1418)) ([98d2560](https://github.com/PicPeak/picpeak/commit/98d25601b465ecc44d0875b52891073720fabbbc))
|
||||
|
||||
## [3.131.5-beta.0](https://github.com/PicPeak/picpeak/compare/v3.131.4-beta.0...v3.131.5-beta.0) (2026-09-11)
|
||||
|
||||
|
||||
### Bug Fixes
|
||||
|
||||
* **gallery:** stop the pre-zip build leaking storage reads ([#1402](https://github.com/PicPeak/picpeak/issues/1402)) ([f094cc0](https://github.com/PicPeak/picpeak/commit/f094cc06a78e776795e60cb3b11e8653681a2f31))
|
||||
|
||||
## [3.131.4-beta.0](https://github.com/PicPeak/picpeak/compare/v3.131.3-beta.0...v3.131.4-beta.0) (2026-09-11)
|
||||
|
||||
|
||||
### Bug Fixes
|
||||
|
||||
* **backend:** bump sharp, nodemailer, multer, js-yaml, joi for security fixes ([#1374](https://github.com/PicPeak/picpeak/issues/1374)) ([f6b81fa](https://github.com/PicPeak/picpeak/commit/f6b81fabf05ab0bbce48e63bbdf3812b30aa10de))
|
||||
* **backend:** contain and sanitize the SQLite restore source path ([#1384](https://github.com/PicPeak/picpeak/issues/1384)) ([316bcbd](https://github.com/PicPeak/picpeak/commit/316bcbd67965ddec74801308a722b506bb8da265))
|
||||
* **backend:** enforce event ownership on short URL deletion ([#1379](https://github.com/PicPeak/picpeak/issues/1379)) ([e290207](https://github.com/PicPeak/picpeak/commit/e290207934708f8bf41676aec39a43474f0f6172))
|
||||
* **backend:** reject a replayed TOTP code within its validity window ([#1389](https://github.com/PicPeak/picpeak/issues/1389)) ([cdde937](https://github.com/PicPeak/picpeak/commit/cdde937d7fce130d67e331bd968cb2c4734902d4))
|
||||
* **backend:** require actor to hold every permission of a role they grant ([#1378](https://github.com/PicPeak/picpeak/issues/1378)) ([59ea83c](https://github.com/PicPeak/picpeak/commit/59ea83c84efdcf6d488853a25ba05f1d15ef1150))
|
||||
* **backend:** shorten payment-check token TTL and notify admin on use ([#1385](https://github.com/PicPeak/picpeak/issues/1385)) ([e324791](https://github.com/PicPeak/picpeak/commit/e3247911a071b5277d877a1fa0a39c544d3a03e8))
|
||||
* **backend:** use the strong password generator for resets and enforce must_change_password ([#1387](https://github.com/PicPeak/picpeak/issues/1387)) ([b798d8e](https://github.com/PicPeak/picpeak/commit/b798d8e4c19541da6ee1f58c2756182817b6c716))
|
||||
* **backend:** validate business-profile logo uploads by content, not filename ([#1381](https://github.com/PicPeak/picpeak/issues/1381)) ([abc9601](https://github.com/PicPeak/picpeak/commit/abc960170b1eac0fa1f3110015e8fce671d428ab))
|
||||
* **backend:** validate event id before using it in the logo storage filename ([#1382](https://github.com/PicPeak/picpeak/issues/1382)) ([38b0e1d](https://github.com/PicPeak/picpeak/commit/38b0e1d5842217030e7dd48247e523ded4b19c58))
|
||||
* **backend:** validate the S3 endpoint host before the restore download ([#1383](https://github.com/PicPeak/picpeak/issues/1383)) ([ec03089](https://github.com/PicPeak/picpeak/commit/ec03089d57c88ff0f7c21b6a4947ffb9ef9771f9))
|
||||
* **gallery:** bound and reclaim storage reads in the remaining zip builders ([#1410](https://github.com/PicPeak/picpeak/issues/1410)) ([70f5a8c](https://github.com/PicPeak/picpeak/commit/70f5a8c54e096f70c030d63414d5954190cb68a8))
|
||||
* **gallery:** keep an admin draft preview out of the guest share-login flow ([f92d4bb](https://github.com/PicPeak/picpeak/commit/f92d4bb2d9c2ea84f59dd4cfaa3a4272f1eec56b))
|
||||
* **gallery:** keep videos playable under enhanced and maximum protection ([#1404](https://github.com/PicPeak/picpeak/issues/1404)) ([1080388](https://github.com/PicPeak/picpeak/commit/1080388f28b846553cd670c66e9632275eb9c994))
|
||||
* **gallery:** let an admin preview a draft through its short share URL ([f92d4bb](https://github.com/PicPeak/picpeak/commit/f92d4bb2d9c2ea84f59dd4cfaa3a4272f1eec56b))
|
||||
* **gallery:** let an admin preview a draft through its short share URL ([#1405](https://github.com/PicPeak/picpeak/issues/1405)) ([f92d4bb](https://github.com/PicPeak/picpeak/commit/f92d4bb2d9c2ea84f59dd4cfaa3a4272f1eec56b))
|
||||
* **upload:** let the csrf gate pass application/octet-stream chunks ([#1401](https://github.com/PicPeak/picpeak/issues/1401)) ([7c0c5c1](https://github.com/PicPeak/picpeak/commit/7c0c5c1cda3921cbd9404dd77d64bdec7a9ae2aa))
|
||||
* **upload:** stop buffering a chunk body before anything checks its size ([#1406](https://github.com/PicPeak/picpeak/issues/1406)) ([4622478](https://github.com/PicPeak/picpeak/commit/4622478e44d5e63da031a2f27c5a5c335282eacb))
|
||||
|
||||
## [3.131.3-beta.0](https://github.com/PicPeak/picpeak/compare/v3.131.2-beta.0...v3.131.3-beta.0) (2026-09-10)
|
||||
|
||||
|
||||
|
||||
@@ -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,124 +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/sessionCutoff', () => ({ isTokenBeforeCutoff: 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,75 +0,0 @@
|
||||
/**
|
||||
* The chunked upload client posts each chunk as application/octet-stream and
|
||||
* the route reads the raw request stream. The CSRF gate answered every such
|
||||
* request 415 before it reached the route, so the endpoint never accepted a
|
||||
* chunk (PicPeak/picpeak#1377).
|
||||
*
|
||||
* The gate's origin check is the CSRF defence. The Content-Type list only
|
||||
* has to keep out what a cross-site page can send without a preflight, and
|
||||
* application/octet-stream is not on that list: an HTML form cannot produce
|
||||
* it and fetch() with it is not CORS-safelisted.
|
||||
*/
|
||||
const express = require('express');
|
||||
const request = require('supertest');
|
||||
|
||||
jest.mock('../../src/utils/logger', () => ({ error: jest.fn(), warn: jest.fn(), info: jest.fn(), debug: jest.fn() }));
|
||||
|
||||
function buildApp() {
|
||||
const app = express();
|
||||
// Same order as server.js: scoped JSON parser, then the gate on /api.
|
||||
app.use(['/api/admin', '/api/v1'], express.json({ limit: '50mb' }));
|
||||
app.use(express.json({ limit: '2mb' }));
|
||||
app.use('/api', require('../../src/middleware/csrf'));
|
||||
// Mirrors the chunk route in adminPhotos.js: consume the raw stream.
|
||||
app.post('/api/admin/photos/:eventId/chunked-upload/:uploadId/chunk/:chunkIndex', async (req, res) => {
|
||||
const chunks = [];
|
||||
for await (const chunk of req) chunks.push(chunk);
|
||||
res.json({ received: Buffer.concat(chunks).toString('base64') });
|
||||
});
|
||||
app.post('/api/admin/other', (req, res) => res.json({ body: req.body }));
|
||||
return app;
|
||||
}
|
||||
|
||||
const CHUNK_PATH = '/api/admin/photos/1/chunked-upload/abc/chunk/0';
|
||||
|
||||
describe('CSRF gate and application/octet-stream', () => {
|
||||
it('lets a same-origin octet-stream chunk reach the route byte for byte', async () => {
|
||||
// Not valid UTF-8, so a text decode anywhere on the path would show up.
|
||||
const payload = Buffer.concat([Buffer.from('chunkbytes'), Buffer.from([0xff, 0x00, 0xfe])]);
|
||||
const res = await request(buildApp())
|
||||
.post(CHUNK_PATH)
|
||||
.set('sec-fetch-site', 'same-origin')
|
||||
.set('Content-Type', 'application/octet-stream')
|
||||
.send(payload);
|
||||
expect(res.status).toBe(200);
|
||||
expect(Buffer.from(res.body.received, 'base64').equals(payload)).toBe(true);
|
||||
});
|
||||
|
||||
it('still rejects a cross-site octet-stream post on origin', async () => {
|
||||
const res = await request(buildApp())
|
||||
.post(CHUNK_PATH)
|
||||
.set('sec-fetch-site', 'cross-site')
|
||||
.set('Content-Type', 'application/octet-stream')
|
||||
.send(Buffer.from('chunkbytes'));
|
||||
expect(res.status).toBe(403);
|
||||
});
|
||||
|
||||
it('still rejects the types a form can send', async () => {
|
||||
const res = await request(buildApp())
|
||||
.post('/api/admin/other')
|
||||
.set('sec-fetch-site', 'same-origin')
|
||||
.set('Content-Type', 'text/plain')
|
||||
.send('x=1');
|
||||
expect(res.status).toBe(415);
|
||||
});
|
||||
|
||||
it('leaves a JSON route with an empty body on an octet-stream post', async () => {
|
||||
const res = await request(buildApp())
|
||||
.post('/api/admin/other')
|
||||
.set('sec-fetch-site', 'same-origin')
|
||||
.set('Content-Type', 'application/octet-stream')
|
||||
.send(Buffer.from('{"a":1}'));
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.body).toEqual({});
|
||||
});
|
||||
});
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
@@ -64,11 +64,6 @@ beforeAll(async () => {
|
||||
t.integer('role_id');
|
||||
t.boolean('is_active');
|
||||
t.timestamp('password_changed_at');
|
||||
// adminAuth() now selects this on every request (GHSA-h4w8-57xq-53fx
|
||||
// must_change_password enforcement) — without the column the join
|
||||
// throws and every route in this file 401s before reaching the
|
||||
// permission check it's meant to test.
|
||||
t.boolean('must_change_password');
|
||||
});
|
||||
await mockDb.schema.createTable('permissions', (t) => {
|
||||
t.increments('id');
|
||||
|
||||
@@ -1,172 +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;
|
||||
|
||||
const asAdmin = (req, id = adminId) => req.set('Authorization', `Bearer ${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 asAdmin(
|
||||
request(app).get(`/api/gallery/resolve/${DRAFT_TOKEN}?admin_preview=1`),
|
||||
);
|
||||
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 asAdmin(
|
||||
request(app).get(`/api/gallery/resolve/${identifier}?admin_preview=1`),
|
||||
);
|
||||
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 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);
|
||||
});
|
||||
});
|
||||
|
||||
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 even with admin_preview=1 but no admin token', async () => {
|
||||
const res = await request(app).get(`/api/gallery/resolve/${DRAFT_TOKEN}?admin_preview=1`);
|
||||
expect(res.status).toBe(404);
|
||||
});
|
||||
|
||||
it('404s for an admin who cannot access this event', async () => {
|
||||
const res = await asAdmin(
|
||||
request(app).get(`/api/gallery/resolve/${DRAFT_TOKEN}?admin_preview=1`),
|
||||
foreignId,
|
||||
);
|
||||
expect(res.status).toBe(404);
|
||||
});
|
||||
|
||||
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 asAdmin(
|
||||
request(app).get(`/api/gallery/resolve/${DRAFT_SLUG}?admin_preview=1`),
|
||||
);
|
||||
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 asAdmin(
|
||||
request(app).get('/api/gallery/resolve/no-such-gallery?admin_preview=1'),
|
||||
);
|
||||
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}`);
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -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,294 +0,0 @@
|
||||
/**
|
||||
* A rejected chunk must not cost its own size in memory (#1403).
|
||||
*
|
||||
* The route used to drain the whole request into an array and `Buffer.concat`
|
||||
* it before calling uploadChunk, which is where every check lives — the
|
||||
* per-file cap, the chunk index, and even "does this upload id exist". So a
|
||||
* 300MB body against an unknown upload id was read in full, added ~300MB to
|
||||
* RSS, and was then answered with an error. The size cap was real but only
|
||||
* applied after the damage.
|
||||
*
|
||||
* The contract these tests pin: uploadChunk consumes NOTHING until every check
|
||||
* has passed, and once it does start reading it stops at the remaining
|
||||
* allowance rather than trusting the sender.
|
||||
*/
|
||||
const path = require('path');
|
||||
const os = require('os');
|
||||
const fs = require('fs').promises;
|
||||
const { Readable } = require('stream');
|
||||
|
||||
process.env.STORAGE_PATH = path.join(os.tmpdir(), `picpeak-chunk-stream-test-${process.pid}`);
|
||||
|
||||
const chunkedUpload = require('../../src/services/chunkedUploadService');
|
||||
|
||||
const MB = 1024 * 1024;
|
||||
|
||||
const init = (overrides = {}) => chunkedUpload.initializeUpload({
|
||||
filename: 'clip.mp4',
|
||||
fileSize: 1,
|
||||
mimeType: 'video/mp4',
|
||||
eventId: 1,
|
||||
totalChunks: 2,
|
||||
maxFileSizeBytes: 1 * MB,
|
||||
...overrides,
|
||||
});
|
||||
|
||||
/**
|
||||
* A readable that reports how much of it was actually pulled. Bytes are
|
||||
* generated lazily, so "never read" really means the body never materialized.
|
||||
*/
|
||||
function countingSource(totalBytes, sliceSize = 64 * 1024) {
|
||||
let remaining = totalBytes;
|
||||
const source = new Readable({
|
||||
read() {
|
||||
if (remaining <= 0) return this.push(null);
|
||||
const n = Math.min(sliceSize, remaining);
|
||||
remaining -= n;
|
||||
source.bytesRead += n;
|
||||
this.push(Buffer.alloc(n));
|
||||
},
|
||||
});
|
||||
source.bytesRead = 0;
|
||||
return source;
|
||||
}
|
||||
|
||||
describe('chunked upload streams the body under a cap (#1403)', () => {
|
||||
afterAll(async () => {
|
||||
await fs.rm(process.env.STORAGE_PATH, { recursive: true, force: true }).catch(() => {});
|
||||
});
|
||||
|
||||
describe('refused before the body is read', () => {
|
||||
it('reads nothing for an unknown upload id', async () => {
|
||||
const source = countingSource(8 * MB);
|
||||
await expect(chunkedUpload.uploadChunk('does-not-exist', 0, source, { declaredBytes: 8 * MB }))
|
||||
.rejects.toThrow('Upload not found or expired');
|
||||
expect(source.bytesRead).toBe(0);
|
||||
});
|
||||
|
||||
it('reads nothing for an out-of-range chunk index', async () => {
|
||||
const { uploadId } = await init();
|
||||
const source = countingSource(8 * MB);
|
||||
await expect(chunkedUpload.uploadChunk(uploadId, 99, source, { declaredBytes: 8 * MB }))
|
||||
.rejects.toMatchObject({ code: 'INVALID_CHUNK', statusCode: 400 });
|
||||
expect(source.bytesRead).toBe(0);
|
||||
});
|
||||
|
||||
it('reads nothing when Content-Length already exceeds the cap', async () => {
|
||||
const { uploadId } = await init();
|
||||
const source = countingSource(8 * MB);
|
||||
await expect(chunkedUpload.uploadChunk(uploadId, 0, source, { declaredBytes: 8 * MB }))
|
||||
.rejects.toMatchObject({ code: 'FILE_TOO_LARGE', statusCode: 413 });
|
||||
expect(source.bytesRead).toBe(0);
|
||||
expect(chunkedUpload.getUploadStatus(uploadId)).toBeNull();
|
||||
});
|
||||
|
||||
it('counts what earlier chunks already banked when checking Content-Length', async () => {
|
||||
const { uploadId } = await init();
|
||||
await chunkedUpload.uploadChunk(uploadId, 0, Buffer.alloc(0.75 * MB));
|
||||
const source = countingSource(0.5 * MB);
|
||||
// 0.75MB banked + 0.5MB declared > the 1MB cap.
|
||||
await expect(chunkedUpload.uploadChunk(uploadId, 1, source, { declaredBytes: 0.5 * MB }))
|
||||
.rejects.toMatchObject({ code: 'FILE_TOO_LARGE', statusCode: 413 });
|
||||
expect(source.bytesRead).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('a sender that lies, or says nothing', () => {
|
||||
it('stops at the allowance instead of reading the whole body', async () => {
|
||||
const { uploadId } = await init();
|
||||
// No declaredBytes at all — the Transfer-Encoding: chunked case.
|
||||
const source = countingSource(8 * MB);
|
||||
await expect(chunkedUpload.uploadChunk(uploadId, 0, source))
|
||||
.rejects.toMatchObject({ code: 'FILE_TOO_LARGE', statusCode: 413 });
|
||||
// The overshoot is whatever the readable had already buffered ahead when
|
||||
// the cap tripped — a small constant tied to highWaterMark, NOT a
|
||||
// function of the body size. That is the whole claim: 8MB offered, ~1MB
|
||||
// read. The slack is deliberately loose so this doesn't turn into a
|
||||
// Node-version canary.
|
||||
expect(source.bytesRead).toBeLessThan(2 * MB);
|
||||
});
|
||||
|
||||
it('leaves no partial chunk file behind when it cuts a body off', async () => {
|
||||
const { uploadId } = await init();
|
||||
const meta = chunkedUpload.getUploadStatus(uploadId);
|
||||
await expect(chunkedUpload.uploadChunk(uploadId, 0, countingSource(8 * MB)))
|
||||
.rejects.toMatchObject({ statusCode: 413 });
|
||||
// abortUpload removes the whole directory; assert nothing survived it.
|
||||
await expect(fs.readdir(path.join(process.env.STORAGE_PATH, 'chunks', uploadId)))
|
||||
.rejects.toMatchObject({ code: 'ENOENT' });
|
||||
expect(meta).not.toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
// Every case here was found by an external review of the first cut of this
|
||||
// fix. All three are regressions the buffered version did not have: the
|
||||
// async iterator it replaced rejected a dead request on its own, and never
|
||||
// opened the chunk file at all until it had the whole body in hand.
|
||||
describe('failure paths the streaming rewrite introduced', () => {
|
||||
it('rejects an already-destroyed request instead of hanging forever', async () => {
|
||||
const { uploadId } = await init();
|
||||
const source = countingSource(1024);
|
||||
source.destroy();
|
||||
// pipe() on a dead stream emits neither `end` nor `error`, so without an
|
||||
// explicit check this promise never settles and the write fd leaks.
|
||||
await expect(chunkedUpload.uploadChunk(uploadId, 0, source))
|
||||
.rejects.toMatchObject({ code: 'CHUNK_PREMATURE_CLOSE', statusCode: 400 });
|
||||
});
|
||||
|
||||
it('leaves a previously banked chunk intact when a re-send fails', async () => {
|
||||
const { uploadId } = await init();
|
||||
await chunkedUpload.uploadChunk(uploadId, 0, Buffer.alloc(1000));
|
||||
const chunkPath = path.join(process.env.STORAGE_PATH, 'chunks', uploadId, 'chunk_000000');
|
||||
expect((await fs.stat(chunkPath)).size).toBe(1000);
|
||||
|
||||
// Re-send the same index, then fail it mid-flight.
|
||||
const source = new Readable({ read() {} });
|
||||
const pending = chunkedUpload.uploadChunk(uploadId, 0, source);
|
||||
source.push(Buffer.alloc(10));
|
||||
source.destroy(new Error('client went away'));
|
||||
await expect(pending).rejects.toThrow();
|
||||
|
||||
// The banked copy must still be there: receivedChunks/chunkSizes still
|
||||
// count it, so a truncated file here means status reports 100% and
|
||||
// completeUpload dies on ENOENT.
|
||||
expect((await fs.stat(chunkPath)).size).toBe(1000);
|
||||
// Still counted as received — which is exactly why the file has to still
|
||||
// be there and be the full 1000 bytes.
|
||||
expect(chunkedUpload.getUploadStatus(uploadId).receivedChunks).toBe(1);
|
||||
});
|
||||
|
||||
it('keeps two in-flight sends of the same chunk off each other\'s staging file', async () => {
|
||||
const { uploadId } = await init();
|
||||
const chunkPath = path.join(process.env.STORAGE_PATH, 'chunks', uploadId, 'chunk_000000');
|
||||
|
||||
// Two requests for the same index, overlapping. A shared .part path let
|
||||
// whichever renamed first publish bytes the other had already truncated.
|
||||
const slow = new Readable({ read() {} });
|
||||
const doomed = new Readable({ read() {} });
|
||||
const slowDone = chunkedUpload.uploadChunk(uploadId, 0, slow);
|
||||
const doomedDone = chunkedUpload.uploadChunk(uploadId, 0, doomed);
|
||||
|
||||
doomed.push(Buffer.alloc(2));
|
||||
doomed.destroy(new Error('retry gave up'));
|
||||
await expect(doomedDone).rejects.toThrow();
|
||||
|
||||
slow.push(Buffer.alloc(10));
|
||||
slow.push(null);
|
||||
await expect(slowDone).resolves.toBeTruthy();
|
||||
|
||||
// The surviving attempt's 10 bytes, not the failed one's 2.
|
||||
expect((await fs.stat(chunkPath)).size).toBe(10);
|
||||
});
|
||||
|
||||
it('leaves no staging files behind after a failure', async () => {
|
||||
const { uploadId } = await init();
|
||||
await expect(chunkedUpload.uploadChunk(uploadId, 0, countingSource(8 * MB)))
|
||||
.rejects.toMatchObject({ statusCode: 413 });
|
||||
// The cap path aborts the whole upload, so the directory is gone; what
|
||||
// must not happen is a .part file reappearing after cleanup because the
|
||||
// write stream's open() was still pending when the unlink ran.
|
||||
await new Promise((r) => setTimeout(r, 50));
|
||||
await expect(fs.readdir(path.join(process.env.STORAGE_PATH, 'chunks', uploadId)))
|
||||
.rejects.toMatchObject({ code: 'ENOENT' });
|
||||
});
|
||||
|
||||
it('counts chunks that landed while another was still streaming', async () => {
|
||||
const { uploadId } = await init({ totalChunks: 3 });
|
||||
|
||||
// Start a slow 0.75MB chunk. Its allowance is computed now, when nothing
|
||||
// else is banked.
|
||||
const slow = new Readable({ read() {} });
|
||||
const slowDone = chunkedUpload.uploadChunk(uploadId, 0, slow);
|
||||
|
||||
// A second 0.75MB chunk completes in the meantime.
|
||||
await chunkedUpload.uploadChunk(uploadId, 1, Buffer.alloc(0.75 * MB));
|
||||
|
||||
// Finishing the first must not publish: 1.5MB against a 1MB cap.
|
||||
slow.push(Buffer.alloc(0.75 * MB));
|
||||
slow.push(null);
|
||||
await expect(slowDone).rejects.toMatchObject({ code: 'FILE_TOO_LARGE', statusCode: 413 });
|
||||
expect(chunkedUpload.getUploadStatus(uploadId)).toBeNull();
|
||||
});
|
||||
|
||||
it('removes the staging file when publishing it fails', async () => {
|
||||
const { uploadId } = await init();
|
||||
const dir = path.join(process.env.STORAGE_PATH, 'chunks', uploadId);
|
||||
// Make the rename fail by putting a directory where the chunk goes.
|
||||
await fs.mkdir(path.join(dir, 'chunk_000000'), { recursive: true });
|
||||
|
||||
await expect(chunkedUpload.uploadChunk(uploadId, 0, countingSource(1024)))
|
||||
.rejects.toThrow();
|
||||
|
||||
// The fully written .part must not survive a failed publish — its name is
|
||||
// per-attempt, so retries would otherwise pile them up until expiry.
|
||||
const leftovers = (await fs.readdir(dir)).filter((f) => f.endsWith('.part'));
|
||||
expect(leftovers).toEqual([]);
|
||||
});
|
||||
|
||||
it('does not destroy the request stream when it trips the cap', async () => {
|
||||
const { uploadId } = await init();
|
||||
const source = countingSource(8 * MB);
|
||||
await expect(chunkedUpload.uploadChunk(uploadId, 0, source))
|
||||
.rejects.toMatchObject({ statusCode: 413 });
|
||||
// `source` stands in for the IncomingMessage. Destroying it would take
|
||||
// the socket down before the route could send its 413 JSON, so the client
|
||||
// would see a connection reset instead of the error.
|
||||
expect(source.destroyed).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
// These were plain Errors, so the routes answered 500 for what are plainly
|
||||
// client mistakes — a backend fault in monitoring, and an invitation to
|
||||
// retry something that can never succeed.
|
||||
describe('client-caused states carry their own status code', () => {
|
||||
it('404s an unknown upload id rather than 500', async () => {
|
||||
await expect(chunkedUpload.uploadChunk('does-not-exist', 0, Buffer.alloc(10)))
|
||||
.rejects.toMatchObject({ statusCode: 404 });
|
||||
});
|
||||
|
||||
// 409 (wrong status) and 410 (expired) share uploadStateError with the two
|
||||
// covered here. Reaching them from the public surface needs either a clock
|
||||
// or a setter the service does not expose, and a test that pretends to
|
||||
// exercise them while actually hitting the 404 path is worse than none.
|
||||
|
||||
it('404s completing an unknown upload rather than 500', async () => {
|
||||
await expect(chunkedUpload.completeUpload('does-not-exist'))
|
||||
.rejects.toMatchObject({ statusCode: 404 });
|
||||
});
|
||||
|
||||
it('400s completing an upload that is missing chunks', async () => {
|
||||
const { uploadId } = await init();
|
||||
await chunkedUpload.uploadChunk(uploadId, 0, Buffer.alloc(10));
|
||||
await expect(chunkedUpload.completeUpload(uploadId))
|
||||
.rejects.toMatchObject({ statusCode: 400 });
|
||||
});
|
||||
});
|
||||
|
||||
describe('the happy path still works', () => {
|
||||
it('writes a streamed chunk and reports progress', async () => {
|
||||
const { uploadId } = await init();
|
||||
const result = await chunkedUpload.uploadChunk(uploadId, 0, countingSource(0.25 * MB), {
|
||||
declaredBytes: 0.25 * MB,
|
||||
});
|
||||
expect(result).toMatchObject({ chunkIndex: 0, received: 1, expected: 2, complete: false });
|
||||
|
||||
const chunkPath = path.join(process.env.STORAGE_PATH, 'chunks', uploadId, 'chunk_000000');
|
||||
expect((await fs.stat(chunkPath)).size).toBe(0.25 * MB);
|
||||
});
|
||||
|
||||
it('still accepts a Buffer, the shape the service was written for', async () => {
|
||||
const { uploadId } = await init();
|
||||
const result = await chunkedUpload.uploadChunk(uploadId, 0, Buffer.alloc(0.25 * MB));
|
||||
expect(result).toMatchObject({ chunkIndex: 0, received: 1 });
|
||||
});
|
||||
|
||||
it('lets a re-sent chunk replace itself without double-counting', async () => {
|
||||
const { uploadId } = await init();
|
||||
await chunkedUpload.uploadChunk(uploadId, 0, countingSource(0.6 * MB), { declaredBytes: 0.6 * MB });
|
||||
// Same index again: the first copy's 0.6MB must not count toward the cap.
|
||||
await expect(
|
||||
chunkedUpload.uploadChunk(uploadId, 0, countingSource(0.6 * MB), { declaredBytes: 0.6 * MB }),
|
||||
).resolves.toBeTruthy();
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -1,128 +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.stopped = false;
|
||||
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('releases anything parked for a slot on shutdown', 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.regenWaiters.length).toBeGreaterThan(0);
|
||||
|
||||
// stop() must not hang on a queue that will never drain.
|
||||
const stopping = service.stop();
|
||||
release.forEach((fn) => fn());
|
||||
await expect(stopping).resolves.toBeUndefined();
|
||||
expect(service.regenWaiters).toHaveLength(0);
|
||||
});
|
||||
});
|
||||
@@ -1,191 +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 resize or watermark, so the builder takes the stream-from-storage
|
||||
// branch, which is the one that holds sockets.
|
||||
jest.mock('../../src/services/downloadRendition', () => ({
|
||||
renderPhotoForDownload: jest.fn(async () => null),
|
||||
}));
|
||||
|
||||
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,146 +0,0 @@
|
||||
/**
|
||||
* DNS-rebinding follow-up to the blind-SSRF fix in restoreServiceS3Ssrf.test.js
|
||||
* (GHSA-vm2x-c628-3cx5).
|
||||
*
|
||||
* isHostAllowed()/validateExternalUrlAsync() 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 and emailWebhookTransport.js
|
||||
* use for outbound HTTP) 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 reuses assertActorMayGrant() — the same containment already
|
||||
* applied to roles.manage (see adminRolesGuards.test.js) — 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');
|
||||
|
||||
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.banking, 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 svc.createRole(
|
||||
{ name: 'limited_user_editor', permissions: ['users.edit', 'events.view'] },
|
||||
superId,
|
||||
);
|
||||
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 svc.createRole(
|
||||
{ name: 'powerful_role', permissions: ['users.edit', 'settings.banking'] },
|
||||
superId,
|
||||
);
|
||||
powerfulRoleId = powerfulRole.id;
|
||||
|
||||
// A role whose permissions ARE a subset of what the limited actor holds.
|
||||
const modestRole = await svc.createRole(
|
||||
{ name: 'modest_role', permissions: ['events.view'] },
|
||||
superId,
|
||||
);
|
||||
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.banking, 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 svc.createRole(
|
||||
{ name: 'limited_inviter', permissions: ['users.create', 'events.view'] },
|
||||
superId,
|
||||
);
|
||||
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 svc.createRole(
|
||||
{ name: 'powerful_invite_role', permissions: ['users.create', 'settings.banking'] },
|
||||
superId,
|
||||
);
|
||||
powerfulRoleId = powerfulRole.id;
|
||||
|
||||
const modestRole = await svc.createRole(
|
||||
{ name: 'modest_invite_role', permissions: ['events.view'] },
|
||||
superId,
|
||||
);
|
||||
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,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 213: 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');
|
||||
});
|
||||
}
|
||||
};
|
||||
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "picpeak-backend",
|
||||
"version": "3.131.7-beta.0",
|
||||
"version": "3.131.3-beta.0",
|
||||
"description": "Backend for PicPeak event photo sharing platform",
|
||||
"main": "server.js",
|
||||
"engines": {
|
||||
|
||||
@@ -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(),
|
||||
};
|
||||
|
||||
|
||||
@@ -3,19 +3,6 @@ const sessionAccess = require('../services/sessionAccessService');
|
||||
const logger = require('../utils/logger');
|
||||
const { getAdminTokenFromRequest } = require('../utils/tokenUtils');
|
||||
|
||||
// GHSA-h4w8-57xq-53fx: must_change_password was written on reset (and on
|
||||
// invitation/OIDC-bypass 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. The
|
||||
// frontend already renders a blocking modal for it (MandatoryPasswordChangeModal),
|
||||
// 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
|
||||
*/
|
||||
@@ -25,7 +12,7 @@ async function adminAuth(req, res, next) {
|
||||
if (!token) {
|
||||
return res.status(401).json({ error: 'No token provided' });
|
||||
}
|
||||
|
||||
|
||||
let decoded;
|
||||
try {
|
||||
decoded = jwt.verify(token, process.env.JWT_SECRET, {
|
||||
@@ -40,23 +27,13 @@ async function adminAuth(req, res, next) {
|
||||
}
|
||||
return res.status(401).json({ error: 'Invalid token' });
|
||||
}
|
||||
|
||||
// includeProfile: true — need must_change_password for the enforcement
|
||||
// check below on every request, not just the profile/session-check routes.
|
||||
const admin = await sessionAccess.admin(decoded, { includeProfile: true });
|
||||
|
||||
const admin = await sessionAccess.admin(decoded);
|
||||
const requestIp = req.ip || req.connection?.remoteAddress;
|
||||
if (decoded.ip && requestIp && decoded.ip !== requestIp) {
|
||||
logger.info('admin session IP changed', { accountId: admin.id, tokenIp: decoded.ip, requestIp });
|
||||
}
|
||||
|
||||
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,
|
||||
@@ -64,7 +41,6 @@ async function adminAuth(req, res, next) {
|
||||
email: admin.email,
|
||||
roleId: admin.role_id,
|
||||
roleName: admin.role_name,
|
||||
mustChangePassword: !!admin.must_change_password,
|
||||
// From the token, not the database: it is a property of this session
|
||||
// rather than of the account (#1186). Carried so a route that reissues
|
||||
// the token — change-password — can preserve the choice instead of
|
||||
@@ -72,7 +48,7 @@ async function adminAuth(req, res, next) {
|
||||
rememberMe: decoded.rememberMe === true
|
||||
};
|
||||
req.token = token; // Store token for potential revocation
|
||||
|
||||
|
||||
next();
|
||||
} catch (error) {
|
||||
logger.error('Auth middleware error:', error);
|
||||
|
||||
@@ -1,12 +1,5 @@
|
||||
const { mutationOriginAllowed } = require('../utils/requestOrigin');
|
||||
|
||||
// The origin check is the CSRF defence. This list only has to keep out what
|
||||
// a cross-site page can send without a preflight: a form cannot produce JSON
|
||||
// or octet-stream, and fetch() with either is not CORS-safelisted.
|
||||
// octet-stream is how the chunked upload route receives its raw body
|
||||
// (#1377); express.json leaves it unread for everything else.
|
||||
const ALLOWED_CONTENT_TYPES = ['application/json', 'multipart/form-data', 'application/octet-stream'];
|
||||
|
||||
module.exports = function csrfProtection(req, res, next) {
|
||||
if (!['POST', 'PUT', 'PATCH', 'DELETE'].includes(req.method)) return next();
|
||||
if (!mutationOriginAllowed(req)) {
|
||||
@@ -14,9 +7,9 @@ module.exports = function csrfProtection(req, res, next) {
|
||||
}
|
||||
const contentType = (req.headers['content-type'] || '').split(';')[0].trim().toLowerCase();
|
||||
const hasBody = Number(req.headers['content-length']) > 0 || !!req.headers['transfer-encoding'];
|
||||
const allowed = ALLOWED_CONTENT_TYPES.includes(contentType) || contentType.endsWith('+json');
|
||||
if (hasBody && !allowed) {
|
||||
return res.status(415).json({ error: `Unsupported Content-Type. Use ${ALLOWED_CONTENT_TYPES.join(', ')}.` });
|
||||
const jsonLike = contentType === 'application/json' || contentType.endsWith('+json');
|
||||
if (hasBody && !jsonLike && contentType !== 'multipart/form-data') {
|
||||
return res.status(415).json({ error: 'Unsupported Content-Type. Use application/json or multipart/form-data.' });
|
||||
}
|
||||
next();
|
||||
};
|
||||
|
||||
@@ -264,11 +264,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')
|
||||
@@ -319,17 +314,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);
|
||||
@@ -344,7 +329,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()
|
||||
});
|
||||
|
||||
@@ -369,16 +353,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');
|
||||
}
|
||||
|
||||
|
||||
@@ -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,11 +102,7 @@ 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}`);
|
||||
},
|
||||
});
|
||||
@@ -131,11 +113,9 @@ const pdfLogoUpload = multer({
|
||||
// array-indexed field names, so reject any bracket-index field name.
|
||||
limits: { fileSize: 5 * 1024 * 1024, fieldArrayIndexLimit: 0 },
|
||||
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'));
|
||||
},
|
||||
});
|
||||
|
||||
@@ -381,19 +361,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
|
||||
@@ -467,19 +434,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
|
||||
|
||||
@@ -66,12 +66,8 @@ 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}`);
|
||||
},
|
||||
});
|
||||
|
||||
|
||||
@@ -24,12 +24,8 @@ 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}`);
|
||||
}
|
||||
});
|
||||
|
||||
|
||||
@@ -1706,30 +1706,18 @@ router.post('/:eventId/chunked-upload/:uploadId/chunk/:chunkIndex', adminAuth, r
|
||||
try {
|
||||
const { uploadId, chunkIndex } = req.params;
|
||||
|
||||
// The request stream is handed over unread (#1403). Every check — unknown
|
||||
// upload id, bad index, the per-file cap against Content-Length — runs
|
||||
// inside uploadChunk before a byte is consumed, and the body is then
|
||||
// streamed to the chunk file under a hard cap rather than concatenated in
|
||||
// memory. Buffering it first meant a rejected 300MB request still cost
|
||||
// 300MB of heap.
|
||||
const declaredBytes = Number(req.headers['content-length']);
|
||||
const result = await chunkedUpload.uploadChunk(uploadId, parseInt(chunkIndex), req, {
|
||||
declaredBytes: Number.isFinite(declaredBytes) ? declaredBytes : undefined,
|
||||
});
|
||||
// Get chunk data from request body
|
||||
const chunks = [];
|
||||
for await (const chunk of req) {
|
||||
chunks.push(chunk);
|
||||
}
|
||||
const chunkData = Buffer.concat(chunks);
|
||||
|
||||
const result = await chunkedUpload.uploadChunk(uploadId, parseInt(chunkIndex), chunkData);
|
||||
|
||||
res.json(result);
|
||||
} catch (error) {
|
||||
// Client-caused states (unknown/finished/expired upload, bad index, too
|
||||
// large) carry their own status. Only a genuinely unexpected error should
|
||||
// reach the 500 below and the error log with it.
|
||||
if (error.statusCode) {
|
||||
// Refusing the body early is the point — but it leaves unread bytes in
|
||||
// flight on a connection this response still advertises as keep-alive.
|
||||
// Node does not drain them, so the NEXT request on that socket hangs
|
||||
// until it times out. Retire the connection instead.
|
||||
if (!req.readableEnded) {
|
||||
res.set('Connection', 'close');
|
||||
}
|
||||
if (error.statusCode === 413 || error.statusCode === 400) {
|
||||
return res.status(error.statusCode).json({ error: error.message });
|
||||
}
|
||||
logger.error('Error uploading chunk:', error);
|
||||
@@ -1779,10 +1767,8 @@ router.post('/:eventId/chunked-upload/:uploadId/complete', adminAuth, requirePer
|
||||
photos: uploadedPhotos
|
||||
});
|
||||
} catch (error) {
|
||||
// Same rule as the chunk route: a tagged status is a client-caused state
|
||||
// (unknown/expired upload, missing chunks), not a server fault.
|
||||
if (error.statusCode) {
|
||||
return res.status(error.statusCode).json({ error: error.message });
|
||||
if (error.statusCode === 413) {
|
||||
return res.status(413).json({ error: error.message });
|
||||
}
|
||||
logger.error('Error completing chunked upload:', error);
|
||||
res.status(500).json({ error: error.message || 'Failed to complete upload' });
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
|
||||
@@ -14,8 +14,7 @@ const { body, param, validationResult } = require('express-validator');
|
||||
const { safeValidationErrors } = require('../utils/routeHelpers');
|
||||
const { adminAuth } = require('../middleware/auth');
|
||||
const { requirePermission } = require('../middleware/permissions');
|
||||
const { requireEventOwnership, canAccessEvent } = require('../middleware/ownership');
|
||||
const { db } = require('../database/db');
|
||||
const { requireEventOwnership } = require('../middleware/ownership');
|
||||
const galleryShortUrlService = require('../services/galleryShortUrlService');
|
||||
const logger = require('../utils/logger');
|
||||
|
||||
@@ -87,30 +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. 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 (!canAccessEvent(req.admin, event)) {
|
||||
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
|
||||
@@ -124,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) {
|
||||
|
||||
@@ -298,22 +298,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) {
|
||||
@@ -342,7 +328,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' },
|
||||
|
||||
@@ -29,7 +29,6 @@ const {
|
||||
} = require('../../services/downloadFilenameService');
|
||||
const { buildContentDisposition } = require('../../utils/filenameSanitizer');
|
||||
const { getStorage } = require('../../services/storage');
|
||||
const { createArchiveStreamGuard } = require('../../utils/archiveStreamGuard');
|
||||
const fs = require('fs');
|
||||
function parseByteRange(header, size) {
|
||||
if (!header || typeof header !== 'string' || !size) return null;
|
||||
@@ -402,13 +401,6 @@ async function bumpEventDownloadCounts(eventId) {
|
||||
}
|
||||
|
||||
router.get('/:slug/download-all', verifyGalleryAccess, denySlideshowToken, blockHiddenGallery, async (req, res) => {
|
||||
// Hoisted so the catch can reclaim reads opened before the failure.
|
||||
let guard = null;
|
||||
// The client hung up. An aborted archive rejects finalize() with ABORTED,
|
||||
// and the catch would then try to send JSON over a response whose ZIP
|
||||
// headers already went out — ERR_HTTP_HEADERS_SENT, unhandled, on an
|
||||
// ordinary cancelled download.
|
||||
let cancelled = false;
|
||||
try {
|
||||
// Check if downloads are allowed for this event
|
||||
if (!parseBooleanInput(req.event.allow_downloads, true)) {
|
||||
@@ -505,23 +497,6 @@ router.get('/:slug/download-all', verifyGalleryAccess, denySlideshowToken, block
|
||||
throw err;
|
||||
});
|
||||
|
||||
// Reclaim storage reads on every exit (#1399 follow-up). A guest closing
|
||||
// the tab mid-download used to leave every appended-but-undrained read
|
||||
// parked on its socket for the life of the process.
|
||||
guard = createArchiveStreamGuard({
|
||||
// A queued read that dies takes the archive with it: archiver has no
|
||||
// listener on it yet, so it would otherwise sit in the queue and stall
|
||||
// the download forever.
|
||||
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
|
||||
@@ -587,9 +562,8 @@ router.get('/:slug/download-all', verifyGalleryAccess, denySlideshowToken, block
|
||||
if (rendered) {
|
||||
archive.append(rendered, { 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 });
|
||||
}
|
||||
@@ -613,7 +587,6 @@ router.get('/:slug/download-all', verifyGalleryAccess, denySlideshowToken, block
|
||||
if (res.statusCode < 400) logActivity('gallery_downloaded', { scope: 'all' }, req.event.id, galleryActor(req));
|
||||
});
|
||||
}
|
||||
if (cancelled) return;
|
||||
await archive.finalize();
|
||||
|
||||
if (!req.isAdminPreview) {
|
||||
@@ -632,18 +605,12 @@ router.get('/:slug/download-all', verifyGalleryAccess, denySlideshowToken, block
|
||||
}
|
||||
}
|
||||
} catch (error) {
|
||||
if (guard) guard.destroyAll();
|
||||
// Nothing to say to a client that already left, and the headers are gone.
|
||||
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, blockHiddenGallery, async (req, res) => {
|
||||
// Hoisted so the catch can reclaim reads opened before the failure.
|
||||
let selectedGuard = null;
|
||||
let selectedCancelled = false;
|
||||
try {
|
||||
// Check if downloads are allowed for this event
|
||||
if (!parseBooleanInput(req.event.allow_downloads, true)) {
|
||||
@@ -712,20 +679,6 @@ router.post('/:slug/download-selected', verifyGalleryAccess, denySlideshowToken,
|
||||
// ignore double-send errors
|
||||
}
|
||||
});
|
||||
|
||||
// Same reclaim contract as download-all above (#1399 follow-up).
|
||||
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
|
||||
@@ -770,9 +723,8 @@ router.post('/:slug/download-selected', verifyGalleryAccess, denySlideshowToken,
|
||||
if (rendered) {
|
||||
archive.append(rendered, { 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 });
|
||||
}
|
||||
@@ -794,7 +746,6 @@ router.post('/:slug/download-selected', verifyGalleryAccess, denySlideshowToken,
|
||||
if (res.statusCode < 400) logActivity('gallery_downloaded', { scope: 'selected', photo_count: photoIds.length }, req.event.id, galleryActor(req));
|
||||
});
|
||||
}
|
||||
if (selectedCancelled) return;
|
||||
await archive.finalize();
|
||||
|
||||
if (!req.isAdminPreview) {
|
||||
@@ -812,8 +763,6 @@ router.post('/:slug/download-selected', verifyGalleryAccess, denySlideshowToken,
|
||||
}
|
||||
}
|
||||
} catch (error) {
|
||||
if (selectedGuard) selectedGuard.destroyAll();
|
||||
if (selectedCancelled || res.headersSent) return;
|
||||
errorResponse(res, error, 500, 'Failed to download selected photos');
|
||||
}
|
||||
});
|
||||
|
||||
@@ -70,14 +70,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',
|
||||
|
||||
@@ -29,36 +29,10 @@ async function checkSlugRedirect(slug) {
|
||||
}
|
||||
}
|
||||
|
||||
// Admin preview of an unpublished gallery (#1386). /info has honoured
|
||||
// admin_preview since #868, but 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 — exactly the shape the reporter described.
|
||||
//
|
||||
// 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) {
|
||||
// decodeAdminPreview requires this flag anyway, so checking it up front costs
|
||||
// nothing and keeps an unknown identifier from paying for a second set of
|
||||
// lookups on the public 404 path.
|
||||
if (req.query?.admin_preview !== '1') return null;
|
||||
const result = await resolveShareIdentifier(identifier, { includeDrafts: true });
|
||||
if (!result) return null;
|
||||
// verifyAdminPreview re-reads the event with SELECT * off the slug, so give
|
||||
// it the slug rather than the partial row selected above.
|
||||
req.requestedSlug = result.event.slug;
|
||||
return await verifyAdminPreview(req) ? result : null;
|
||||
}
|
||||
|
||||
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);
|
||||
@@ -105,23 +79,14 @@ router.get('/:slug/verify-token/:token', noStoreCache, handleAsync(async (req, r
|
||||
const { slug, token } = req.params;
|
||||
|
||||
const event = await db('events')
|
||||
.where({ slug, is_active: formatBoolean(true), is_archived: formatBoolean(false) })
|
||||
.select('id', 'share_link', 'share_token', 'is_draft')
|
||||
.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 a verified admin preview only (#1386). Without this
|
||||
// the preview clears /resolve and then 404s one step later, here.
|
||||
if (event.is_draft) {
|
||||
req.requestedSlug = slug;
|
||||
if (!await verifyAdminPreview(req)) {
|
||||
throw new NotFoundError('Gallery');
|
||||
}
|
||||
}
|
||||
|
||||
const expectedToken = getEventShareToken(event);
|
||||
if (!expectedToken || !timingSafeEqualStr(String(token), expectedToken)) {
|
||||
throw new NotFoundError('Gallery', 'Invalid gallery link');
|
||||
|
||||
@@ -30,28 +30,6 @@ function fileTooLargeError(maxFileSizeBytes) {
|
||||
return err;
|
||||
}
|
||||
|
||||
function prematureCloseError() {
|
||||
const err = new Error('Request body closed before the chunk was fully received');
|
||||
err.code = 'CHUNK_PREMATURE_CLOSE';
|
||||
err.statusCode = 400;
|
||||
return err;
|
||||
}
|
||||
|
||||
function overAllowanceError() {
|
||||
return Object.assign(new Error('CHUNK_OVER_ALLOWANCE'), { overAllowance: true });
|
||||
}
|
||||
|
||||
// A client-supplied upload id that is unknown, finished or expired is the
|
||||
// client's mistake, not the server's. These used to be plain Errors, so the
|
||||
// routes answered 500 — which reads as a backend fault in monitoring and
|
||||
// invites the client to retry something that will never succeed.
|
||||
function uploadStateError(message, statusCode) {
|
||||
const err = new Error(message);
|
||||
err.code = 'UPLOAD_STATE';
|
||||
err.statusCode = statusCode;
|
||||
return err;
|
||||
}
|
||||
|
||||
function invalidChunkError(message) {
|
||||
const err = new Error(message);
|
||||
err.code = 'INVALID_CHUNK';
|
||||
@@ -137,100 +115,28 @@ async function initializeUpload(options) {
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Stream `source` into `partPath`, refusing to write more than `allowance`
|
||||
* bytes (#1403). The cap is the backstop for a request that lies about its
|
||||
* Content-Length or omits it: the moment the running total passes the
|
||||
* allowance the read stops and the partial file is removed, so an oversized
|
||||
* body costs the allowance rather than its own size.
|
||||
*/
|
||||
function writeChunkStream(source, partPath, allowance) {
|
||||
const fsSync = require('fs');
|
||||
return new Promise((resolve, reject) => {
|
||||
// A client that hung up while auth and ownership were awaiting the database
|
||||
// hands us an already-dead stream. pipe() would then emit neither `end` nor
|
||||
// `error`, leaving this promise pending forever with the write descriptor
|
||||
// open. The async-iterator version this replaced rejected that case, so it
|
||||
// has to be checked explicitly rather than inferred from an event.
|
||||
if (source.destroyed || source.aborted) {
|
||||
return reject(prematureCloseError());
|
||||
}
|
||||
|
||||
const out = fsSync.createWriteStream(partPath);
|
||||
let written = 0;
|
||||
let settled = false;
|
||||
|
||||
const settle = (err, value) => {
|
||||
if (settled) return;
|
||||
settled = true;
|
||||
source.unpipe(out);
|
||||
if (err) {
|
||||
// Wait for the descriptor to actually close before unlinking. destroy()
|
||||
// does not await a pending open(), so unlinking straight away races it:
|
||||
// the unlink fails with ENOENT and the open then recreates the .part
|
||||
// file after cleanup was supposed to be done.
|
||||
const removePart = () => fsSync.unlink(partPath, () => reject(err));
|
||||
if (out.destroyed) {
|
||||
removePart();
|
||||
} else {
|
||||
out.once('close', removePart);
|
||||
out.destroy();
|
||||
}
|
||||
} else {
|
||||
resolve(value);
|
||||
}
|
||||
};
|
||||
|
||||
source.on('data', (buf) => {
|
||||
written += buf.length;
|
||||
if (written > allowance) {
|
||||
// Deliberately NOT source.destroy(). `source` is the IncomingMessage,
|
||||
// and destroying it destroys the socket under it — the 413 the route is
|
||||
// about to send would never reach the client, who would see a connection
|
||||
// reset instead of the size-limit JSON. Pausing stops the read, which is
|
||||
// the whole point of the cap.
|
||||
source.pause();
|
||||
settle(overAllowanceError());
|
||||
}
|
||||
});
|
||||
source.on('error', settle);
|
||||
source.on('aborted', () => settle(prematureCloseError()));
|
||||
source.on('close', () => {
|
||||
if (!source.readableEnded) settle(prematureCloseError());
|
||||
});
|
||||
out.on('error', settle);
|
||||
out.on('finish', () => settle(null, written));
|
||||
source.pipe(out);
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Upload a single chunk
|
||||
* @param {string} uploadId - Upload ID
|
||||
* @param {number} chunkIndex - Chunk index (0-based)
|
||||
* @param {Buffer|import('stream').Readable} source - Chunk bytes, or a stream
|
||||
* of them (the request). A stream is never read until every check below has
|
||||
* passed, so a rejected request costs nothing (#1403).
|
||||
* @param {Object} [options]
|
||||
* @param {number} [options.declaredBytes] - Content-Length, when the caller
|
||||
* has one. Checked against the remaining allowance before the body is read.
|
||||
* @param {Buffer} chunkData - Chunk data
|
||||
* @returns {Promise<Object>} - Chunk upload result
|
||||
*/
|
||||
async function uploadChunk(uploadId, chunkIndex, source, { declaredBytes } = {}) {
|
||||
async function uploadChunk(uploadId, chunkIndex, chunkData) {
|
||||
const uploadMeta = activeUploads.get(uploadId);
|
||||
|
||||
if (!uploadMeta) {
|
||||
throw uploadStateError('Upload not found or expired', 404);
|
||||
throw new Error('Upload not found or expired');
|
||||
}
|
||||
|
||||
if (uploadMeta.status !== 'in_progress') {
|
||||
throw uploadStateError(`Upload is ${uploadMeta.status}`, 409);
|
||||
throw new Error(`Upload is ${uploadMeta.status}`);
|
||||
}
|
||||
|
||||
// Check expiration
|
||||
if (Date.now() > uploadMeta.expiresAt) {
|
||||
await abortUpload(uploadId);
|
||||
throw uploadStateError('Upload expired', 410);
|
||||
throw new Error('Upload expired');
|
||||
}
|
||||
|
||||
// Only the announced chunk indices are valid — anything else would merge
|
||||
@@ -242,73 +148,19 @@ async function uploadChunk(uploadId, chunkIndex, source, { declaredBytes } = {})
|
||||
// Enforce the per-file cap on the running byte total. The upload is
|
||||
// aborted, not just rejected: the chunks on disk are already over the
|
||||
// limit and the client can't complete the file any more.
|
||||
//
|
||||
// What this chunk may still contribute — everything already banked, minus a
|
||||
// re-sent copy of this same index. Computed before the body is touched so a
|
||||
// Content-Length that already blows the budget is refused having read zero
|
||||
// bytes (#1403).
|
||||
const bankedBytes = totalReceivedBytes(uploadMeta) - (uploadMeta.chunkSizes.get(chunkIndex) || 0);
|
||||
const allowance = uploadMeta.maxFileSizeBytes - bankedBytes;
|
||||
|
||||
if (Number.isFinite(declaredBytes) && declaredBytes > allowance) {
|
||||
const receivedBytes = totalReceivedBytes(uploadMeta) - (uploadMeta.chunkSizes.get(chunkIndex) || 0) + chunkData.length;
|
||||
if (receivedBytes > uploadMeta.maxFileSizeBytes) {
|
||||
await abortUpload(uploadId);
|
||||
throw fileTooLargeError(uploadMeta.maxFileSizeBytes);
|
||||
}
|
||||
|
||||
// Write chunk to disk
|
||||
const chunkPath = path.join(uploadMeta.uploadDir, `chunk_${String(chunkIndex).padStart(6, '0')}`);
|
||||
let chunkLength;
|
||||
|
||||
if (Buffer.isBuffer(source)) {
|
||||
if (bankedBytes + source.length > uploadMeta.maxFileSizeBytes) {
|
||||
await abortUpload(uploadId);
|
||||
throw fileTooLargeError(uploadMeta.maxFileSizeBytes);
|
||||
}
|
||||
await fs.writeFile(chunkPath, source);
|
||||
chunkLength = source.length;
|
||||
} else {
|
||||
// Staged through a sibling .part file, then renamed. Writing the canonical
|
||||
// path directly truncates it the moment the stream opens, so a re-sent
|
||||
// chunk that then failed left receivedChunks/chunkSizes still claiming the
|
||||
// old copy: status reported 100% and completeUpload died on ENOENT.
|
||||
//
|
||||
// The suffix is per-attempt, not per-index: two in-flight requests for the
|
||||
// same chunk would otherwise share one staging file, and whichever renamed
|
||||
// first would publish bytes the other had already truncated.
|
||||
const partPath = `${chunkPath}.${crypto.randomBytes(6).toString('hex')}.part`;
|
||||
try {
|
||||
chunkLength = await writeChunkStream(source, partPath, allowance);
|
||||
// Re-check the aggregate before publishing. `allowance` was computed
|
||||
// before the body arrived, so a chunk that completed while this one was
|
||||
// still streaming is not counted in it — two overlapping 0.75MB chunks
|
||||
// under a 1MB cap would otherwise both be accepted. The buffered version
|
||||
// got this right for free by checking after the read; streaming has to
|
||||
// ask again.
|
||||
const bankedNow = totalReceivedBytes(uploadMeta) - (uploadMeta.chunkSizes.get(chunkIndex) || 0);
|
||||
if (bankedNow + chunkLength > uploadMeta.maxFileSizeBytes) {
|
||||
await fs.rm(partPath, { force: true }).catch(() => {});
|
||||
await abortUpload(uploadId);
|
||||
throw fileTooLargeError(uploadMeta.maxFileSizeBytes);
|
||||
}
|
||||
await fs.rename(partPath, chunkPath).catch(async (renameErr) => {
|
||||
// A failed publish (ENOSPC, a vanished directory) left the fully
|
||||
// written staging file behind. Its name is per-attempt, so a client
|
||||
// that retries instead of aborting just accumulates more of them until
|
||||
// the upload expires.
|
||||
await fs.rm(partPath, { force: true }).catch(() => {});
|
||||
throw renameErr;
|
||||
});
|
||||
} catch (err) {
|
||||
if (err.overAllowance) {
|
||||
await abortUpload(uploadId);
|
||||
throw fileTooLargeError(uploadMeta.maxFileSizeBytes);
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
await fs.writeFile(chunkPath, chunkData);
|
||||
|
||||
// Mark chunk as received
|
||||
uploadMeta.receivedChunks.add(chunkIndex);
|
||||
uploadMeta.chunkSizes.set(chunkIndex, chunkLength);
|
||||
uploadMeta.chunkSizes.set(chunkIndex, chunkData.length);
|
||||
|
||||
const progress = (uploadMeta.receivedChunks.size / uploadMeta.expectedChunks) * 100;
|
||||
|
||||
@@ -338,13 +190,12 @@ async function completeUpload(uploadId) {
|
||||
const uploadMeta = activeUploads.get(uploadId);
|
||||
|
||||
if (!uploadMeta) {
|
||||
throw uploadStateError('Upload not found or expired', 404);
|
||||
throw new Error('Upload not found or expired');
|
||||
}
|
||||
|
||||
// Verify all chunks received
|
||||
if (uploadMeta.receivedChunks.size !== uploadMeta.expectedChunks) {
|
||||
throw uploadStateError(
|
||||
`Missing chunks: received ${uploadMeta.receivedChunks.size} of ${uploadMeta.expectedChunks}`, 400);
|
||||
throw new Error(`Missing chunks: received ${uploadMeta.receivedChunks.size} of ${uploadMeta.expectedChunks}`);
|
||||
}
|
||||
|
||||
uploadMeta.status = 'merging';
|
||||
|
||||
@@ -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'],
|
||||
|
||||
@@ -28,7 +28,6 @@ const crypto = require('crypto');
|
||||
const archiver = require('archiver');
|
||||
const { db } = require('../database/db');
|
||||
const { getStorage } = require('./storage');
|
||||
const { createArchiveStreamGuard } = require('../utils/archiveStreamGuard');
|
||||
const { getUseOriginalFilenames, getZipEntryNames } = require('./downloadFilenameService');
|
||||
const { renderPhotoForDownload, resolveWatermarkSettings } = require('./downloadRendition');
|
||||
const { resolvePhotoStorageKey, resolvePhotoFilePath } = require('./photoResolver');
|
||||
@@ -298,17 +297,8 @@ class DownloadJobService {
|
||||
const output = fs.createWriteStream(tmpPath);
|
||||
// level 0 — photos are already compressed, so deflate only burns CPU.
|
||||
const archive = archiver('zip', { zlib: { level: 0 } });
|
||||
// Bound and reclaim the storage reads (#1399 follow-up). The per-photo
|
||||
// catch below deliberately skips a bad source, but it never destroyed
|
||||
// the stream it had already opened, so every skipped photo leaked a
|
||||
// socket for the life of the process.
|
||||
// A queued read that dies would otherwise stall the archive and hold
|
||||
// its slot for the life of the build, so it fails the job instead.
|
||||
const guard = createArchiveStreamGuard({
|
||||
onFatalError: (err) => { guard.destroyAll(); archive.abort(); reject(err); },
|
||||
});
|
||||
output.on('close', resolve);
|
||||
archive.on('error', (err) => { guard.destroyAll(); reject(err); });
|
||||
archive.on('error', reject);
|
||||
archive.pipe(output);
|
||||
|
||||
(async () => {
|
||||
@@ -322,8 +312,7 @@ class DownloadJobService {
|
||||
} else {
|
||||
const key = resolvePhotoStorageKey(event, photo);
|
||||
if (key) {
|
||||
if (!await guard.acquire()) break;
|
||||
archive.append(guard.track(await storage.get(key)), { name });
|
||||
archive.append(await storage.get(key), { name });
|
||||
} else {
|
||||
archive.file(resolvePhotoFilePath(event, photo), { name });
|
||||
}
|
||||
@@ -342,7 +331,7 @@ class DownloadJobService {
|
||||
}
|
||||
}
|
||||
archive.finalize();
|
||||
})().catch((err) => { guard.destroyAll(); reject(err); });
|
||||
})().catch(reject);
|
||||
});
|
||||
|
||||
if (appended === 0) {
|
||||
|
||||
@@ -30,72 +30,18 @@ const logger = require('../utils/logger');
|
||||
|
||||
const DEBOUNCE_MS = 5000;
|
||||
|
||||
// How many storage reads may be open at once while the archive is built.
|
||||
// archiver drains its queue one entry at a time, so a stream appended ahead of
|
||||
// its turn just parks an S3 socket with a full receive buffer. The SDK agent
|
||||
// pool is 50 sockets wide and shared with uploads, thumbnails and gallery
|
||||
// reads, so an unbounded loop over a large event starves the whole process.
|
||||
// Two keeps the next photo's round trip overlapped with the current write
|
||||
// without ever leaving more than one socket idle.
|
||||
const MAX_INFLIGHT_READS = 2;
|
||||
// How many cached zips may be REBUILT at once in the background (#1399).
|
||||
//
|
||||
// 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
|
||||
this.stopped = false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Run a BACKGROUND rebuild under the concurrency cap (#1399). 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.stopped) return undefined;
|
||||
if (this.regenActive >= MAX_CONCURRENT_REGENS) {
|
||||
await new Promise((resolve) => this.regenWaiters.push(resolve));
|
||||
// Shutdown can drain the queue while we were parked.
|
||||
if (this.stopped) return undefined;
|
||||
}
|
||||
this.regenActive += 1;
|
||||
try {
|
||||
return await fn();
|
||||
} finally {
|
||||
this.regenActive -= 1;
|
||||
const next = this.regenWaiters.shift();
|
||||
if (next) next();
|
||||
}
|
||||
}
|
||||
|
||||
async stop() {
|
||||
this.stopped = true;
|
||||
for (const timer of this.debounceTimers.values()) clearTimeout(timer);
|
||||
this.debounceTimers.clear();
|
||||
// Release anything parked for a slot so shutdown can't hang on a queue
|
||||
// that will never drain — they check `stopped` and return without building.
|
||||
const waiters = this.regenWaiters.splice(0);
|
||||
for (const resume of waiters) resume();
|
||||
await Promise.allSettled([...this.activeBuilds.values()].map(build => build.promise));
|
||||
this.versions.clear();
|
||||
this.buildCancellers.clear();
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -171,40 +117,6 @@ class DownloadZipService {
|
||||
const storage = getStorage();
|
||||
let tmpDir;
|
||||
|
||||
// Storage reads appended to the archive but not yet drained. A failed or
|
||||
// invalidated build must destroy them: archiver's own abort() leaves the
|
||||
// source streams alone, and an unread S3 response body holds its socket
|
||||
// open for the life of the process (the SDK arms its socketTimeout on a
|
||||
// 3s delay and clears it as soon as the response headers land, so
|
||||
// nothing ever reclaims the socket).
|
||||
const openReads = new Set();
|
||||
let cancelled = false;
|
||||
let slotWaiter = null;
|
||||
|
||||
const wakeSlotWaiter = () => {
|
||||
if (!slotWaiter) return;
|
||||
const resume = slotWaiter;
|
||||
slotWaiter = null;
|
||||
resume();
|
||||
};
|
||||
const releaseRead = (stream) => {
|
||||
openReads.delete(stream);
|
||||
wakeSlotWaiter();
|
||||
};
|
||||
// Assigned once the archive exists. Bumping the generation counter only
|
||||
// stops the build the next time the loop looks at it, and the loop can be
|
||||
// parked waiting for a read slot that a stalled archive will never free,
|
||||
// so invalidation cancels the build directly instead of leaving a note.
|
||||
let failBuild = null;
|
||||
|
||||
const trackRead = (stream) => {
|
||||
openReads.add(stream);
|
||||
stream.once('end', () => releaseRead(stream));
|
||||
stream.once('close', () => releaseRead(stream));
|
||||
stream.once('error', () => releaseRead(stream));
|
||||
return stream;
|
||||
};
|
||||
|
||||
try {
|
||||
const event = await db('events').where({ id: eventId }).first();
|
||||
if (!event) return { success: false, error: 'Event not found' };
|
||||
@@ -250,37 +162,15 @@ class DownloadZipService {
|
||||
const useOriginal = await getUseOriginalFilenames();
|
||||
const entryNames = getZipEntryNames(photos, useOriginal);
|
||||
|
||||
this.buildCancellers.set(eventId, () => {
|
||||
if (failBuild) failBuild(new Error('Build invalidated'));
|
||||
});
|
||||
|
||||
// Build zip — level 0 (store only) since photos are already compressed
|
||||
await new Promise((resolve, reject) => {
|
||||
const output = fs.createWriteStream(tmpPath);
|
||||
const archive = archiver('zip', { zlib: { level: 0 } });
|
||||
|
||||
failBuild = (err) => {
|
||||
if (cancelled) return;
|
||||
cancelled = true;
|
||||
wakeSlotWaiter();
|
||||
// abort() throws if archiver already tore itself down.
|
||||
try { archive.abort(); } catch (_) { /* already aborted */ }
|
||||
reject(err);
|
||||
};
|
||||
|
||||
output.on('close', resolve);
|
||||
archive.on('error', failBuild);
|
||||
archive.on('error', reject);
|
||||
archive.pipe(output);
|
||||
|
||||
// Block until archiver has drained enough of its queue for another
|
||||
// read. Also returns when the build is cancelled, so a stalled
|
||||
// archive cannot park the loop here forever.
|
||||
const waitForReadSlot = async () => {
|
||||
while (!cancelled && openReads.size >= MAX_INFLIGHT_READS) {
|
||||
await new Promise((resume) => { slotWaiter = resume; });
|
||||
}
|
||||
};
|
||||
|
||||
const uniqueTypes = new Set(photos.map(p => p.type)).size;
|
||||
const hasMultipleTypes = uniqueTypes > 1;
|
||||
|
||||
@@ -288,9 +178,9 @@ class DownloadZipService {
|
||||
for (let i = 0; i < photos.length; i += 1) {
|
||||
const photo = photos[i];
|
||||
// Check if build was invalidated
|
||||
if (cancelled) return;
|
||||
if (this.versions.get(eventId) !== version) {
|
||||
return failBuild(new Error('Build invalidated'));
|
||||
archive.abort();
|
||||
return reject(new Error('Build invalidated'));
|
||||
}
|
||||
|
||||
const entryName = entryNames[i];
|
||||
@@ -322,17 +212,8 @@ class DownloadZipService {
|
||||
if (rendered) {
|
||||
archive.append(rendered, { name: archiveName });
|
||||
} else if (storageKey) {
|
||||
await waitForReadSlot();
|
||||
if (cancelled) return;
|
||||
const stream = await storage.get(storageKey);
|
||||
// The build can be cancelled while the read is in flight, and a
|
||||
// stream nobody appends is a stream nobody closes.
|
||||
if (cancelled || this.versions.get(eventId) !== version) {
|
||||
stream.destroy();
|
||||
if (cancelled) return;
|
||||
return failBuild(new Error('Build invalidated'));
|
||||
}
|
||||
archive.append(trackRead(stream), { name: archiveName });
|
||||
archive.append(stream, { name: archiveName });
|
||||
} else {
|
||||
const filePath = resolvePhotoFilePath(event, photo);
|
||||
archive.file(filePath, { name: archiveName });
|
||||
@@ -342,7 +223,7 @@ class DownloadZipService {
|
||||
archive.finalize();
|
||||
};
|
||||
|
||||
addPhotos().catch(failBuild);
|
||||
addPhotos().catch(reject);
|
||||
});
|
||||
|
||||
// Check version again — another invalidation may have arrived
|
||||
@@ -371,11 +252,6 @@ class DownloadZipService {
|
||||
logger.error('downloadZipService._build error', { eventId, error: err.message });
|
||||
return { success: false, error: err.message };
|
||||
} finally {
|
||||
this.buildCancellers.delete(eventId);
|
||||
for (const stream of openReads) {
|
||||
stream.destroy();
|
||||
}
|
||||
openReads.clear();
|
||||
if (tmpDir) {
|
||||
await fsp.rm(tmpDir, { recursive: true, force: true }).catch(() => {});
|
||||
}
|
||||
@@ -390,11 +266,6 @@ class DownloadZipService {
|
||||
// Bump version to signal any in-flight build is stale
|
||||
this.versions.set(eventId, (this.versions.get(eventId) || 0) + 1);
|
||||
|
||||
// Stop the in-flight build now so it releases its storage reads, rather
|
||||
// than when it next reaches the top of its loop.
|
||||
const cancelBuild = this.buildCancellers.get(eventId);
|
||||
if (cancelBuild) cancelBuild();
|
||||
|
||||
// Cancel pending debounce
|
||||
const timer = this.debounceTimers.get(eventId);
|
||||
if (timer) clearTimeout(timer);
|
||||
@@ -407,9 +278,7 @@ class DownloadZipService {
|
||||
// Debounce regeneration
|
||||
const newTimer = setTimeout(() => {
|
||||
this.debounceTimers.delete(eventId);
|
||||
// Through the cap (#1399): 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);
|
||||
|
||||
@@ -459,17 +459,7 @@ async function getGalleryPhotos({ event, query = {}, identity, accessLevel, admi
|
||||
reveal_at: hiddenForGuest ? (event.reveal_at || null) : undefined,
|
||||
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 in routes/gallery/media.js.
|
||||
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');
|
||||
// Watermark version (cache-busting) + admin-preview flag (#868). In
|
||||
// preview mode no gallery cookie is minted, so each <img> request must
|
||||
// re-assert the admin session — thread the flag onto every /api/gallery
|
||||
|
||||
@@ -13,15 +13,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
|
||||
@@ -236,7 +227,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,
|
||||
@@ -392,45 +383,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:
|
||||
@@ -499,19 +451,6 @@ async function recordPaymentCheckAction({ token, action, amountMinor, ip, adminI
|
||||
adminId ? `admin:${adminId}` : 'public:payment-check');
|
||||
} catch (_) { /* non-fatal */ }
|
||||
|
||||
// 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, {
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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');
|
||||
|
||||
@@ -1038,26 +959,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) {
|
||||
@@ -1127,12 +1036,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
|
||||
@@ -1631,10 +1535,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;
|
||||
@@ -1728,44 +1628,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.
|
||||
// validateExternalUrlAsync's resolved addresses get pinned into the
|
||||
// S3Client's requestHandler via pinnedRequestOptions — the same
|
||||
// primitive webhookDeliveryWorker.js/emailWebhookTransport.js use for
|
||||
// outbound HTTP — 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 { validateExternalUrlAsync } = require('../utils/networkValidation');
|
||||
const { pinnedRequestOptions } = require('../utils/pinnedRequest');
|
||||
const endpointUrl = /^https?:\/\//.test(s3Config.endpoint)
|
||||
? s3Config.endpoint
|
||||
: `https://${s3Config.endpoint}`;
|
||||
const urlCheck = await validateExternalUrlAsync(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);
|
||||
@@ -1939,14 +1805,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
|
||||
};
|
||||
@@ -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 a verified admin preview.
|
||||
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,10 +140,9 @@ const resolveShareIdentifier = async (identifier, { includeDrafts = false } = {}
|
||||
'event_date',
|
||||
'expires_at',
|
||||
'is_active',
|
||||
'is_archived',
|
||||
'is_draft'
|
||||
'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) {
|
||||
|
||||
@@ -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
|
||||
}
|
||||
};
|
||||
|
||||
|
||||
@@ -7,7 +7,7 @@ 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 { getAbsoluteFrontendUrl } = require('../utils/frontendUrl');
|
||||
const { queueEmail } = require('./emailProcessor');
|
||||
@@ -48,16 +48,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
|
||||
@@ -281,16 +271,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');
|
||||
@@ -494,10 +474,7 @@ async function resetAdminPassword(id, resetById) {
|
||||
throw new ValidationError('This account is managed by your identity provider (SSO) — reset the password there.');
|
||||
}
|
||||
|
||||
// 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({
|
||||
|
||||
@@ -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,7 +1,7 @@
|
||||
{
|
||||
"name": "picpeak-frontend",
|
||||
"private": true,
|
||||
"version": "3.131.7-beta.0",
|
||||
"version": "3.131.3-beta.0",
|
||||
"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, { useRef, useState, useEffect } from 'react';
|
||||
import { useTranslation } from 'react-i18next';
|
||||
import { Play, Pause, Volume2, VolumeX, Maximize, Minimize, AlertTriangle } from 'lucide-react';
|
||||
import { Play, Pause, Volume2, VolumeX, Maximize, Minimize } from 'lucide-react';
|
||||
|
||||
interface VideoPlayerProps {
|
||||
src: string;
|
||||
@@ -25,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);
|
||||
@@ -54,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);
|
||||
@@ -83,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;
|
||||
@@ -188,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'
|
||||
@@ -250,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();
|
||||
});
|
||||
});
|
||||
@@ -250,24 +250,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
|
||||
|
||||
@@ -1770,9 +1770,7 @@
|
||||
"folderPhotoCount_one": "{{count}} Foto",
|
||||
"folderPhotoCount_other": "{{count}} Fotos",
|
||||
"preparingProgress_one": "{{count}} Foto verpackt",
|
||||
"preparingProgress_other": "{{count}} Fotos verpackt",
|
||||
"videoFormatUnsupported": "Dieses Videoformat kann in diesem Browser nicht abgespielt werden. Laden Sie es herunter, um es anzusehen.",
|
||||
"videoLoadFailed": "Dieses Video konnte nicht geladen werden."
|
||||
"preparingProgress_other": "{{count}} Fotos verpackt"
|
||||
},
|
||||
"categories": {
|
||||
"title": "Fotokategorien",
|
||||
|
||||
@@ -1268,9 +1268,7 @@
|
||||
"folderPhotoCount_one": "{{count}} photos",
|
||||
"folderPhotoCount_other": "{{count}} photos",
|
||||
"preparingProgress_one": "{{count}} photos packaged",
|
||||
"preparingProgress_other": "{{count}} photos packaged",
|
||||
"videoFormatUnsupported": "This video format cannot be played in this browser. Download it to watch it.",
|
||||
"videoLoadFailed": "This video could not be loaded."
|
||||
"preparingProgress_other": "{{count}} photos packaged"
|
||||
},
|
||||
"categories": {
|
||||
"title": "Photo Categories",
|
||||
|
||||
@@ -315,9 +315,7 @@
|
||||
"backToGallery": "Todas las fotos",
|
||||
"downloadFolder": "Descargar carpeta ({{count}})",
|
||||
"downloadFolderCapped": "Descargar las primeras {{limit}} de {{total}}",
|
||||
"downloadEverything": "Descargar todas las fotos",
|
||||
"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."
|
||||
"downloadEverything": "Descargar todas las fotos"
|
||||
},
|
||||
"categories": {
|
||||
"title": "Categorías de fotos",
|
||||
|
||||
@@ -336,9 +336,7 @@
|
||||
"backToGallery": "Toutes les photos",
|
||||
"downloadFolder": "Télécharger le dossier ({{count}})",
|
||||
"downloadFolderCapped": "Télécharger les {{limit}} premières sur {{total}}",
|
||||
"downloadEverything": "Télécharger toutes les 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."
|
||||
"downloadEverything": "Télécharger toutes les photos"
|
||||
},
|
||||
"categories": {
|
||||
"title": "Catégories de photos",
|
||||
|
||||
@@ -336,9 +336,7 @@
|
||||
"backToGallery": "Alle foto's",
|
||||
"downloadFolder": "Map downloaden ({{count}})",
|
||||
"downloadFolderCapped": "Eerste {{limit}} van {{total}} downloaden",
|
||||
"downloadEverything": "Alle foto's downloaden",
|
||||
"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."
|
||||
"downloadEverything": "Alle foto's downloaden"
|
||||
},
|
||||
"categories": {
|
||||
"title": "Fotocategorieen",
|
||||
|
||||
@@ -344,9 +344,7 @@
|
||||
"backToGallery": "Todas as fotos",
|
||||
"downloadFolder": "Baixar pasta ({{count}})",
|
||||
"downloadFolderCapped": "Baixar as primeiras {{limit}} de {{total}}",
|
||||
"downloadEverything": "Baixar todas as fotos",
|
||||
"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."
|
||||
"downloadEverything": "Baixar todas as fotos"
|
||||
},
|
||||
"categories": {
|
||||
"title": "Categorias de Fotos",
|
||||
|
||||
@@ -352,9 +352,7 @@
|
||||
"backToGallery": "Все фото",
|
||||
"downloadFolder": "Скачать папку ({{count}})",
|
||||
"downloadFolderCapped": "Скачать первые {{limit}} из {{total}}",
|
||||
"downloadEverything": "Скачать все фото",
|
||||
"videoFormatUnsupported": "Этот формат видео не воспроизводится в этом браузере. Скачайте файл, чтобы посмотреть его.",
|
||||
"videoLoadFailed": "Не удалось загрузить это видео."
|
||||
"downloadEverything": "Скачать все фото"
|
||||
},
|
||||
"categories": {
|
||||
"title": "Категории фото",
|
||||
|
||||
@@ -336,9 +336,7 @@
|
||||
"backToGallery": "Vse fotografije",
|
||||
"downloadFolder": "Prenesi mapo ({{count}})",
|
||||
"downloadFolderCapped": "Prenesi prvih {{limit}} od {{total}}",
|
||||
"downloadEverything": "Prenesi vse fotografije",
|
||||
"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."
|
||||
"downloadEverything": "Prenesi vse fotografije"
|
||||
},
|
||||
"categories": {
|
||||
"title": "Kategorije fotografij",
|
||||
|
||||
Reference in New Issue
Block a user