From 07f2c900556738e993fb63764210b541d7692c9d Mon Sep 17 00:00:00 2001 From: Paul Nothaft Date: Thu, 16 Jul 2026 10:13:34 +0200 Subject: [PATCH 01/10] fix(security): mask backup credentials on read + unblock MFA login during maintenance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two pre-existing bugs surfaced while reviewing #806 (kept separate per scope policy — no OIDC code here): - backup_s3_secret_key and backup_rsync_ssh_key (an SSH PRIVATE KEY) were returned in PLAINTEXT by GET /admin/backup/config and by the generic settings reads (GET /admin/settings and /admin/settings/:type — which mask the recaptcha/umami/rybbit keys but not these). All three now mask with the established bullet sentinel, and PUT /admin/backup/config skips the sentinel on write so the edit form round-trips without clobbering stored credentials (same pattern as the email/WhatsApp config endpoints) - /api/auth/admin/login/mfa was missing from the maintenance-mode allowlist: the first login step passed, the second factor got a 503 — any MFA-enrolled admin was locked out exactly while maintenance mode was on Regression tests: masking on all three read paths, sentinel round-trip preserves stored values, real rotation still writes. --- .../integration/backupSecretMasking.test.js | 108 ++++++++++++++++++ backend/src/middleware/maintenance.js | 4 + backend/src/routes/adminBackup.js | 13 ++- backend/src/routes/adminSettings.js | 20 ++++ 4 files changed, 144 insertions(+), 1 deletion(-) create mode 100644 backend/__tests__/integration/backupSecretMasking.test.js diff --git a/backend/__tests__/integration/backupSecretMasking.test.js b/backend/__tests__/integration/backupSecretMasking.test.js new file mode 100644 index 00000000..1fdbed87 --- /dev/null +++ b/backend/__tests__/integration/backupSecretMasking.test.js @@ -0,0 +1,108 @@ +/** + * Backup credential exposure regression tests. + * + * The generic settings reads (GET /admin/settings, GET /admin/settings/:type) + * masked the recaptcha/umami/rybbit keys but returned backup_s3_secret_key + * and backup_rsync_ssh_key (an SSH PRIVATE KEY) in plaintext to any + * settings.view holder; GET /admin/backup/config returned them too. Both now + * mask, and PUT /admin/backup/config skips the mask sentinel so the edit + * form round-trips without clobbering stored credentials. + */ + +const request = require('supertest'); +const express = require('express'); + +const { bootCrmDb } = require('./helpers/crmDb'); + +jest.mock('../../src/middleware/auth', () => ({ + adminAuth: (req, _res, next) => { + req.admin = { id: 1, username: 'test-admin' }; + next(); + }, +})); +jest.mock('../../src/middleware/permissions', () => ({ + requirePermission: () => (_req, _res, next) => next(), +})); + +describe('backup credential masking', () => { + let db; + let cleanup; + let app; + + beforeAll(async () => { + ({ db, cleanup } = await bootCrmDb()); + + // Upsert: several backup_* keys are pre-seeded by the backup migrations. + const seed = [ + { setting_key: 'backup_destination_type', setting_value: JSON.stringify('s3'), setting_type: 'backup' }, + { setting_key: 'backup_s3_endpoint', setting_value: JSON.stringify('https://s3.example.com'), setting_type: 'backup' }, + { setting_key: 'backup_s3_bucket', setting_value: JSON.stringify('backups'), setting_type: 'backup' }, + { setting_key: 'backup_s3_access_key', setting_value: JSON.stringify('AKIAEXAMPLE'), setting_type: 'backup' }, + { setting_key: 'backup_s3_secret_key', setting_value: JSON.stringify('super-secret-s3-key'), setting_type: 'backup' }, + { setting_key: 'backup_rsync_ssh_key', setting_value: JSON.stringify('-----BEGIN OPENSSH PRIVATE KEY-----abc'), setting_type: 'backup' }, + ]; + for (const row of seed) { + await db('app_settings').insert(row).onConflict('setting_key').merge(); + } + + app = express(); + app.use(express.json()); + app.use('/api/admin/backup', require('../../src/routes/adminBackup')); + app.use('/api/admin/settings', require('../../src/routes/adminSettings')); + }, 120000); + + afterAll(async () => { + if (cleanup) await cleanup(); + }); + + it('masks the credentials in GET /admin/backup/config', async () => { + const res = await request(app).get('/api/admin/backup/config').expect(200); + expect(res.body.backup_s3_secret_key).toBe('••••••••'); + expect(res.body.backup_rsync_ssh_key).toBe('••••••••'); + // Non-secret fields stay readable for the form. + expect(res.body.backup_s3_bucket).toBe('backups'); + }); + + it('masks the credentials in the generic GET /admin/settings/:type read', async () => { + const res = await request(app).get('/api/admin/settings/backup').expect(200); + expect(res.body.backup_s3_secret_key).toBe('••••••••'); + expect(res.body.backup_rsync_ssh_key).toBe('••••••••'); + }); + + it('masks the credentials in the generic GET /admin/settings read', async () => { + const res = await request(app).get('/api/admin/settings').expect(200); + expect(res.body.backup_s3_secret_key).toBe('••••••••'); + expect(res.body.backup_rsync_ssh_key).toBe('••••••••'); + }); + + it('PUT /admin/backup/config keeps the stored secret when the sentinel round-trips', async () => { + await request(app) + .put('/api/admin/backup/config') + .send({ + backup_destination_type: 's3', + backup_s3_endpoint: 'https://s3.example.com', + backup_s3_bucket: 'renamed-bucket', + backup_s3_access_key: 'AKIAEXAMPLE', + backup_s3_secret_key: '••••••••', + backup_rsync_ssh_key: '••••••••', + }) + .expect(200); + + const secret = await db('app_settings').where({ setting_key: 'backup_s3_secret_key' }).first(); + expect(JSON.parse(secret.setting_value)).toBe('super-secret-s3-key'); + const sshKey = await db('app_settings').where({ setting_key: 'backup_rsync_ssh_key' }).first(); + expect(JSON.parse(sshKey.setting_value)).toBe('-----BEGIN OPENSSH PRIVATE KEY-----abc'); + const bucket = await db('app_settings').where({ setting_key: 'backup_s3_bucket' }).first(); + expect(JSON.parse(bucket.setting_value)).toBe('renamed-bucket'); + }); + + it('PUT /admin/backup/config stores a genuinely new secret', async () => { + await request(app) + .put('/api/admin/backup/config') + .send({ backup_s3_secret_key: 'rotated-s3-key' }) + .expect(200); + + const secret = await db('app_settings').where({ setting_key: 'backup_s3_secret_key' }).first(); + expect(JSON.parse(secret.setting_value)).toBe('rotated-s3-key'); + }); +}); diff --git a/backend/src/middleware/maintenance.js b/backend/src/middleware/maintenance.js index bb1493f3..0f99732f 100644 --- a/backend/src/middleware/maintenance.js +++ b/backend/src/middleware/maintenance.js @@ -73,6 +73,10 @@ async function maintenanceMiddleware(req, res, next) { // entries here matched nothing, which is exactly why the lockout happened). const skipPaths = [ '/api/auth/admin/login', + // The second factor is part of the same login — without this, any + // MFA-enrolled admin gets a 503 on the verify step and cannot sign in + // at all while maintenance mode is on. + '/api/auth/admin/login/mfa', '/api/auth/session', '/api/public/settings', '/health' diff --git a/backend/src/routes/adminBackup.js b/backend/src/routes/adminBackup.js index 67d1479c..cb8eb5bd 100644 --- a/backend/src/routes/adminBackup.js +++ b/backend/src/routes/adminBackup.js @@ -29,7 +29,13 @@ router.get('/config', adminAuth, requirePermission('backup.view'), async (req, r config[setting.setting_key] = setting.setting_value; } }); - + + // Never return the stored credentials — mask like the email/WhatsApp + // config endpoints do. The PUT below skips the mask sentinel, so the + // form round-trips without clobbering the real values. + if (config.backup_s3_secret_key) config.backup_s3_secret_key = '••••••••'; + if (config.backup_rsync_ssh_key) config.backup_rsync_ssh_key = '••••••••'; + res.json(config); } catch (error) { errorResponse(res, error, 500, 'Failed to get backup configuration'); @@ -65,6 +71,11 @@ router.put('/config', adminAuth, requirePermission('backup.create'), async (req, // Update settings for (const [key, value] of Object.entries(updates)) { + // An unchanged secret round-trips as the GET mask sentinel — keep the + // stored value instead of overwriting it with bullets. + if (value === '••••••••') { + continue; + } if (key.startsWith('backup_')) { await db('app_settings') .insert({ diff --git a/backend/src/routes/adminSettings.js b/backend/src/routes/adminSettings.js index 433a8389..b0e3d634 100644 --- a/backend/src/routes/adminSettings.js +++ b/backend/src/routes/adminSettings.js @@ -160,6 +160,16 @@ router.get('/', adminAuth, requirePermission('settings.view'), async (req, res) if (settingsObject.security_recaptcha_secret_key) { settingsObject.security_recaptcha_secret_key = '••••••••'; } + // Backup credentials — the S3 secret key and the rsync SSH PRIVATE KEY + // were returned in plaintext to any settings.view holder. Same masking + // pattern as the recaptcha/umami/rybbit keys; the dedicated + // /admin/backup/config endpoints handle the edit round-trip. + if (settingsObject.backup_s3_secret_key) { + settingsObject.backup_s3_secret_key = '••••••••'; + } + if (settingsObject.backup_rsync_ssh_key) { + settingsObject.backup_rsync_ssh_key = '••••••••'; + } // Umami v2 API key (#661 Bug C) — read-write secret that authenticates // outbound calls to the operator's Umami instance for the device // breakdown. Masked on GET, same pattern as the recaptcha secret. @@ -410,6 +420,16 @@ router.get('/:type', adminAuth, requirePermission('settings.view'), async (req, if (settingsObject.security_recaptcha_secret_key) { settingsObject.security_recaptcha_secret_key = '••••••••'; } + // Backup credentials — the S3 secret key and the rsync SSH PRIVATE KEY + // were returned in plaintext to any settings.view holder. Same masking + // pattern as the recaptcha/umami/rybbit keys; the dedicated + // /admin/backup/config endpoints handle the edit round-trip. + if (settingsObject.backup_s3_secret_key) { + settingsObject.backup_s3_secret_key = '••••••••'; + } + if (settingsObject.backup_rsync_ssh_key) { + settingsObject.backup_rsync_ssh_key = '••••••••'; + } // Umami v2 API key (#661 Bug C) — read-write secret that authenticates // outbound calls to the operator's Umami instance for the device // breakdown. Masked on GET, same pattern as the recaptcha secret. From eb03b612688f0b06d9770e1675961f5405a29b35 Mon Sep 17 00:00:00 2001 From: Paul Nothaft <53005142+the-luap@users.noreply.github.com> Date: Thu, 16 Jul 2026 10:29:31 +0200 Subject: [PATCH 02/10] =?UTF-8?q?chore(security):=20close=2021=20frontend?= =?UTF-8?q?=20image=20CVEs=20=E2=80=94=20nginx=201.30=20base=20+=20apk=20c?= =?UTF-8?q?ache-bust?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The frontend image kept shipping vulnerable OS packages (nginx 1.28.3-r1, curl/libcurl 8.19.0, c-ares 1.34.6) despite the apk upgrade line, for two independent reasons: 1. The runtime stage's apk upgrade layer was cached indefinitely — the CACHEBUST build-arg CI passes (github.run_number) was only declared in the builder stage, and ARGs don't cross stage boundaries. Both Dockerfiles now redeclare CACHEBUST in the runtime stage and consume it in the apk RUN, so every build re-runs the upgrade and picks up current Alpine security updates. 2. nginx itself can never upgrade via apk on the nginx.org-based image: the bundled nginx-module-* packages pin the exact nginx version, so Alpine's patched 1.28.3-r4 is unreachable (verified empirically — apk add --upgrade nginx is a silent no-op). nginx fixes must come via the base tag, so bump to nginx:1.30-alpine (current stable, 1.30.4 on Alpine 3.24, same nginx.org conf.d layout — drop-in). Verified: local image build scans clean with Trivy (0 OS findings, was 21); container serves /health, SPA fallback, and BRAND_TITLE envsubst as non-root nginx user. Closes code-scanning alerts 371-374, 376-392 (nginx HTTP/2 & module CVEs, curl CVE-2026-5773/-6276 + 6 medium, c-ares CVE-2026-33630). --- backend/Dockerfile | 9 ++++++++- frontend/Dockerfile | 24 +++++++++++++++++------- 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/backend/Dockerfile b/backend/Dockerfile index e89058db..688820ed 100644 --- a/backend/Dockerfile +++ b/backend/Dockerfile @@ -27,8 +27,15 @@ FROM node:22-alpine WORKDIR /app +# Redeclare CACHEBUST — ARGs don't cross stage boundaries, so the builder +# stage's declaration never reached this stage. Consuming it in the RUN below +# busts that layer's cache every CI run (CACHEBUST=github.run_number), so the +# image always picks up current Alpine security updates instead of reusing a +# stale cached upgrade layer. +ARG CACHEBUST=1 + # Upgrade all packages to fix security vulnerabilities (OpenSSL, libexpat, BusyBox CVEs) -RUN apk upgrade --no-cache +RUN echo "cachebust=${CACHEBUST}" && apk upgrade --no-cache # Upgrade the npm CLI in the final image so its bundled deps are patched # (sigstore 4.x, tar) — closes CVE-2026-48815 and the older @sigstore/core / tar diff --git a/frontend/Dockerfile b/frontend/Dockerfile index 586e4754..eed1130d 100644 --- a/frontend/Dockerfile +++ b/frontend/Dockerfile @@ -29,14 +29,24 @@ COPY . . # Build the application RUN npm run build -# Production stage (Alpine 3.23 with OpenSSL 3.5.5, patched libexpat) -FROM nginx:1.28-alpine +# Production stage (nginx stable 1.30 on Alpine 3.24). The 1.28 base is a +# dead end for the nginx HTTP/2 + rewrite/charset CVEs (CVE-2026-42055 / +# -49975 / -9256 / -48142): nginx.org's nginx-module-* packages pin the exact +# nginx version, so `apk upgrade` can never pull Alpine's patched 1.28.3-r4 — +# nginx fixes have to come via the base image tag, not apk. +FROM nginx:1.30-alpine -# Upgrade all Alpine packages for security fixes. The explicit nginx upgrade -# closes the HTTP/2 + rewrite/charset CVEs (CVE-2026-42055 / -49975 / -9256 / -# -48142, fixed in nginx 1.28.3-r4) and busts any cached layer still carrying -# the vulnerable r1 build. -RUN apk upgrade --no-cache && apk add --no-cache --upgrade nginx +# Redeclare CACHEBUST — ARGs don't cross stage boundaries, so the builder +# stage's declaration never reached this stage. Consuming it in the RUN below +# busts that layer's cache every CI run (CACHEBUST=github.run_number), so the +# image always picks up current Alpine security updates. Without this, the +# upgrade layer was cached indefinitely and builds kept shipping curl 8.19.0 / +# c-ares 1.34.6 for weeks after fixed packages landed in the Alpine repo. +ARG CACHEBUST=1 + +# Upgrade all Alpine packages for security fixes (nginx itself is version- +# pinned by its module packages — see the FROM comment above). +RUN echo "cachebust=${CACHEBUST}" && apk upgrade --no-cache # Install runtime dependencies. `gettext` provides envsubst, used by # docker-entrypoint.sh for the BRAND_TITLE / BRAND_DESCRIPTION runtime From efccecb3d83ea60df039ec9efafaedfcbd661ea2 Mon Sep 17 00:00:00 2001 From: Paul Nothaft <53005142+the-luap@users.noreply.github.com> Date: Thu, 16 Jul 2026 10:36:29 +0200 Subject: [PATCH 03/10] chore(main): release 3.88.1-beta.0 (#810) --- .release-please-manifest-beta.json | 2 +- CHANGELOG.md | 8 ++++++++ backend/package.json | 2 +- frontend/package.json | 2 +- 4 files changed, 11 insertions(+), 3 deletions(-) diff --git a/.release-please-manifest-beta.json b/.release-please-manifest-beta.json index ec87b005..e592a14b 100644 --- a/.release-please-manifest-beta.json +++ b/.release-please-manifest-beta.json @@ -1,3 +1,3 @@ { - ".": "3.88.0-beta.0" + ".": "3.88.1-beta.0" } diff --git a/CHANGELOG.md b/CHANGELOG.md index d35e569e..26a531a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,14 @@ 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.88.1-beta.0](https://github.com/PicPeak/picpeak/compare/v3.88.0-beta.0...v3.88.1-beta.0) (2026-07-16) + + +### Bug Fixes + +* **security:** mask backup credentials on read + unblock MFA login during maintenance ([eadf282](https://github.com/PicPeak/picpeak/commit/eadf282755829cb51e6ea37221be31d8c9af41c5)) +* **security:** mask backup credentials on read + unblock MFA login during maintenance ([07f2c90](https://github.com/PicPeak/picpeak/commit/07f2c900556738e993fb63764210b541d7692c9d)) + ## [3.88.0-beta.0](https://github.com/PicPeak/picpeak/compare/v3.87.0-beta.0...v3.88.0-beta.0) (2026-07-15) diff --git a/backend/package.json b/backend/package.json index d64d7a15..f9734117 100644 --- a/backend/package.json +++ b/backend/package.json @@ -1,6 +1,6 @@ { "name": "picpeak-backend", - "version": "3.88.0-beta.0", + "version": "3.88.1-beta.0", "description": "Backend for PicPeak event photo sharing platform", "main": "server.js", "scripts": { diff --git a/frontend/package.json b/frontend/package.json index 3dee20de..a7baec90 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,7 +1,7 @@ { "name": "picpeak-frontend", "private": true, - "version": "3.88.0-beta.0", + "version": "3.88.1-beta.0", "type": "module", "scripts": { "dev": "vite", From 348894efefa5a7b49d32feb22a98045b93076138 Mon Sep 17 00:00:00 2001 From: Paul Nothaft <53005142+the-luap@users.noreply.github.com> Date: Thu, 16 Jul 2026 10:55:10 +0200 Subject: [PATCH 04/10] fix(security): preserve current admin on .picpeak restore (GHSA-qxfx-4493-4v8f) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit adminAuth populates req.admin, not req.user, so currentAdminId was always undefined in the /api/admin/picpeak/import handler. reinjectCurrentAdmin() then had no account to preserve and the admin_users table was fully replaced by the uploaded backup — a crafted .picpeak let any admin with backup.restore take over every admin account (critical). One-line fix: pass req.admin.id. Closes GHSA-qxfx-4493-4v8f and its duplicate GHSA-pjp6-jcrj-3cr5. --- backend/src/routes/adminBackup.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/backend/src/routes/adminBackup.js b/backend/src/routes/adminBackup.js index 67d1479c..b1ee0e97 100644 --- a/backend/src/routes/adminBackup.js +++ b/backend/src/routes/adminBackup.js @@ -178,7 +178,11 @@ router.post('/picpeak/import', adminAuth, requirePermission('backup.restore'), p const picpeakPath = req.file.path; try { const { importFromPicpeak } = require('../services/picpeakImportService'); - const result = await importFromPicpeak({ picpeakPath, currentAdminId: req.user && req.user.id }); + // adminAuth populates req.admin, not req.user. Passing req.user.id here + // left currentAdminId undefined, so reinjectCurrentAdmin() had no account + // to preserve and the admin_users table was fully replaced by the backup — + // letting a crafted .picpeak take over every admin account (GHSA-qxfx-4493-4v8f). + const result = await importFromPicpeak({ picpeakPath, currentAdminId: req.admin && req.admin.id }); res.json({ success: true, tables: result.tables, From 7dace044dcc1c3b5a13c4704510c87616632618c Mon Sep 17 00:00:00 2001 From: Paul Nothaft <53005142+the-luap@users.noreply.github.com> Date: Thu, 16 Jul 2026 10:55:10 +0200 Subject: [PATCH 05/10] fix(security): share-login must not bypass gallery password (GHSA-9hmx-68vc-qpqw) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit POST /auth/gallery/share-login validated only the 128-bit share token and then minted a full type:'gallery' access token regardless of require_password — computing requiresPassword at the end only to echo it, never enforce it. Anyone holding a gallery's share link could read and download every photo in a password-protected gallery via a direct API call, no password needed. Fix: compute requiresPassword before minting; for a password-protected gallery return { requires_password: true } with NO token and NO cookie. The client then goes through /gallery/verify, which does bcrypt.compare the password. The public (no-password) auto-login path is unchanged. The frontend already falls through to the password prompt when share-login returns no token/event. Adds route regression test covering the bypass, the public path, and bad tokens. --- .../routes/authShareLoginPassword.test.js | 127 ++++++++++++++++++ backend/src/routes/auth.js | 14 +- 2 files changed, 139 insertions(+), 2 deletions(-) create mode 100644 backend/__tests__/routes/authShareLoginPassword.test.js diff --git a/backend/__tests__/routes/authShareLoginPassword.test.js b/backend/__tests__/routes/authShareLoginPassword.test.js new file mode 100644 index 00000000..aadece62 --- /dev/null +++ b/backend/__tests__/routes/authShareLoginPassword.test.js @@ -0,0 +1,127 @@ +/** + * Regression test for GHSA-9hmx-68vc-qpqw — share-link login must not bypass + * the gallery password. + * + * POST /auth/gallery/share-login validates only the share token. For a + * password-protected gallery it previously minted a full `type:'gallery'` + * access token on the share token alone, letting anyone holding the share URL + * read the gallery without the password. The fix: when the gallery requires a + * password, return `{ requires_password: true }` with NO token and NO cookie. + */ + +const express = require('express'); +const request = require('supertest'); + +process.env.JWT_SECRET = 'share-login-test-secret'; + +const events = []; + +jest.mock('../../src/database/db', () => { + function dbFn(table) { + if (table === 'events') { + let filter = () => true; + return { + where(criteria) { + filter = (row) => Object.entries(criteria).every(([k, v]) => { + if (k === 'is_active') return Boolean(row.is_active) === Boolean(v); + if (k === 'is_archived') return Boolean(row.is_archived) === Boolean(v); + return row[k] === v; + }); + return this; + }, + async first() { return events.find(filter); }, + }; + } + return { where() { return this; }, async first() { return undefined; } }; + } + dbFn.raw = async () => {}; + return { db: dbFn, logActivity: async () => {} }; +}); + +// Share token is stored plainly on the fake event row. +jest.mock('../../src/services/shareLinkService', () => ({ + getEventShareToken: (event) => event.share_token, + resolveShareIdentifier: async () => ({ event: null }), +})); + +const mockSetGalleryAuthCookies = jest.fn(); +jest.mock('../../src/utils/tokenUtils', () => ({ + setGalleryAuthCookies: (...args) => mockSetGalleryAuthCookies(...args), + clearGalleryAuthCookies: jest.fn(), + getGalleryTokenFromRequest: jest.fn(), + setAdminAuthCookies: jest.fn(), +})); + +jest.mock('../../src/utils/authSecurity', () => ({ + trackFailedAttempt: jest.fn(async () => {}), + trackSuccessfulLogin: jest.fn(async () => {}), + checkAccountLockout: jest.fn(async () => ({ isLocked: false })), + resetLockout: jest.fn(async () => {}), +})); + +// Collaborators the router imports at load but the share-login path doesn't hit. +jest.mock('../../src/services/recaptcha', () => ({ verifyRecaptcha: async () => true })); +jest.mock('../../src/services/mfaService', () => ({})); +jest.mock('../../src/middleware/sessionTimeout', () => ({ endSession: jest.fn(), sessionTimeoutMiddleware: (req, res, next) => next() })); +jest.mock('../../src/utils/tokenRevocation', () => ({ revokeToken: jest.fn(async () => {}), isTokenRevoked: async () => false })); + +const authRouter = require('../../src/routes/auth'); + +function makeApp() { + const app = express(); + app.use(express.json()); + app.use('/auth', authRouter); + return app; +} + +const SHARE_TOKEN = 'a'.repeat(64); + +beforeEach(() => { + events.length = 0; + mockSetGalleryAuthCookies.mockClear(); +}); + +describe('POST /auth/gallery/share-login password enforcement', () => { + it('does NOT mint a token for a password-protected gallery', async () => { + events.push({ + id: 1, slug: 'private-gallery', is_active: 1, is_archived: 0, + require_password: 1, share_token: SHARE_TOKEN, event_name: 'Private', + }); + const res = await request(makeApp()) + .post('/auth/gallery/share-login') + .send({ slug: 'private-gallery', token: SHARE_TOKEN }); + + expect(res.status).toBe(200); + expect(res.body.requires_password).toBe(true); + expect(res.body.token).toBeUndefined(); + expect(mockSetGalleryAuthCookies).not.toHaveBeenCalled(); + }); + + it('mints a token for a public (no-password) gallery', async () => { + events.push({ + id: 2, slug: 'public-gallery', is_active: 1, is_archived: 0, + require_password: false, share_token: SHARE_TOKEN, event_name: 'Public', + }); + const res = await request(makeApp()) + .post('/auth/gallery/share-login') + .send({ slug: 'public-gallery', token: SHARE_TOKEN }); + + expect(res.status).toBe(200); + expect(typeof res.body.token).toBe('string'); + expect(res.body.event).toBeDefined(); + expect(mockSetGalleryAuthCookies).toHaveBeenCalledTimes(1); + }); + + it('rejects a wrong share token regardless of password setting', async () => { + events.push({ + id: 3, slug: 'public-gallery', is_active: 1, is_archived: 0, + require_password: false, share_token: SHARE_TOKEN, event_name: 'Public', + }); + const res = await request(makeApp()) + .post('/auth/gallery/share-login') + .send({ slug: 'public-gallery', token: 'b'.repeat(64) }); + + expect(res.status).toBe(401); + expect(mockSetGalleryAuthCookies).not.toHaveBeenCalled(); + }); +}); diff --git a/backend/src/routes/auth.js b/backend/src/routes/auth.js index 03f9fa21..998feaf0 100644 --- a/backend/src/routes/auth.js +++ b/backend/src/routes/auth.js @@ -554,6 +554,18 @@ router.post('/gallery/share-login', [ return res.status(401).json({ error: 'Invalid or expired share link' }); } + const requiresPassword = !(event.require_password === false || event.require_password === 0 || event.require_password === '0'); + + // The share link only proves the holder was given the link — it is NOT the + // gallery password. For a password-protected gallery, minting a full + // `type:'gallery'` token here would let anyone with the share URL bypass + // the password entirely (GHSA-9hmx-68vc-qpqw). Signal that a password is + // still required and return WITHOUT a token/cookie; the client then goes + // through POST /gallery/verify, which does check the password. + if (requiresPassword) { + return res.json({ requires_password: true }); + } + const jwtToken = jwt.sign({ eventId: event.id, eventSlug: event.slug, @@ -568,8 +580,6 @@ router.post('/gallery/share-login', [ await trackSuccessfulLogin(`gallery:${event.slug}:share`, ipAddress, userAgent); setGalleryAuthCookies(res, jwtToken, event.slug); - const requiresPassword = !(event.require_password === false || event.require_password === 0 || event.require_password === '0'); - res.json({ token: jwtToken, event: { From 9cd6b08441e8633751b9fb73daca5ca0555c950b Mon Sep 17 00:00:00 2001 From: Paul Nothaft <53005142+the-luap@users.noreply.github.com> Date: Thu, 16 Jul 2026 10:55:10 +0200 Subject: [PATCH 06/10] fix(security): reject ZIP-slip entries in archive/backup restore (GHSA-jfhw-fj23-fx6x) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit node-stream-zip's extract(null, root) writes each entry to path.join(root, entry.name) without neutralising '../', so a crafted archive entry named '../../uploads/logos/evil.svg' escaped the target dir and overwrote arbitrary files (logos, .env, route files → RCE on source deploys). Requires admin with archives.restore. Adds assertZipEntriesWithin() to utils/safePath.js — a lexical containment check run on the entry list BEFORE extract() — and guards both extract sinks: adminArchives.js (the reported route) and picpeakImportService.js (the sibling .picpeak import, same sink). Adds unit tests for traversal, absolute-path, and sibling-prefix entries. --- .../utils/safePathZipEntries.test.js | 41 +++++++++++++++++++ backend/src/routes/adminArchives.js | 11 +++++ backend/src/services/picpeakImportService.js | 5 +++ backend/src/utils/safePath.js | 34 +++++++++++++++ 4 files changed, 91 insertions(+) create mode 100644 backend/__tests__/utils/safePathZipEntries.test.js diff --git a/backend/__tests__/utils/safePathZipEntries.test.js b/backend/__tests__/utils/safePathZipEntries.test.js new file mode 100644 index 00000000..97d00acc --- /dev/null +++ b/backend/__tests__/utils/safePathZipEntries.test.js @@ -0,0 +1,41 @@ +const path = require('path'); +const { assertZipEntriesWithin } = require('../../src/utils/safePath'); + +describe('assertZipEntriesWithin (ZIP-slip guard, GHSA-jfhw-fj23-fx6x)', () => { + const root = path.join('/tmp', 'picpeak-extract-root'); + + it('accepts entries that stay within the extraction root', () => { + const entries = [ + { name: 'photo.jpg' }, + { name: 'category/nested/photo.png' }, + { name: 'photos_manifest.json' }, + { name: 'subdir/' }, + ]; + expect(() => assertZipEntriesWithin(entries, root)).not.toThrow(); + }); + + it('rejects a parent-traversal entry', () => { + const entries = [{ name: '../../uploads/logos/evil.svg' }]; + expect(() => assertZipEntriesWithin(entries, root)).toThrow(/escapes the extraction directory/); + }); + + it('rejects an absolute-path entry', () => { + const entries = [{ name: '/etc/cron.d/evil' }]; + expect(() => assertZipEntriesWithin(entries, root)).toThrow(/escapes the extraction directory/); + }); + + it('rejects when a safe entry is mixed with a traversal entry', () => { + const entries = [{ name: 'ok.jpg' }, { name: '../escape.txt' }]; + expect(() => assertZipEntriesWithin(entries, root)).toThrow(/escapes the extraction directory/); + }); + + it('tolerates empty / nameless entries', () => { + expect(() => assertZipEntriesWithin([{}, { name: '' }, null], root)).not.toThrow(); + }); + + it('does not treat a sibling prefix directory as inside the root', () => { + // root is .../picpeak-extract-root; ../picpeak-extract-root-evil must not pass + const entries = [{ name: '../picpeak-extract-root-evil/x' }]; + expect(() => assertZipEntriesWithin(entries, root)).toThrow(/escapes the extraction directory/); + }); +}); diff --git a/backend/src/routes/adminArchives.js b/backend/src/routes/adminArchives.js index d81c8713..fcebccfa 100644 --- a/backend/src/routes/adminArchives.js +++ b/backend/src/routes/adminArchives.js @@ -9,6 +9,7 @@ const { requirePermission } = require('../middleware/permissions'); const archiver = require('archiver'); const StreamZip = require('node-stream-zip'); const { requireEventOwnership } = require('../middleware/ownership'); +const { assertZipEntriesWithin } = require('../utils/safePath'); const logger = require('../utils/logger'); const { getPagination } = require('../utils/routeHelpers'); const router = express.Router(); @@ -183,6 +184,16 @@ router.post('/:id/restore', adminAuth, requirePermission('archives.restore'), re const entries = Object.values(await zip.entries()); logger.info(`Archive contains ${entries.length} entries`); + // Reject ZIP-slip entries before writing anything to disk — extract() + // does not neutralise `../` in entry names (GHSA-jfhw-fj23-fx6x). + try { + assertZipEntriesWithin(entries, eventDir); + } catch (slipErr) { + await zip.close(); + logger.warn(`Refusing archive restore — unsafe entry path: ${slipErr.message}`); + return res.status(400).json({ error: 'Archive contains invalid entry paths' }); + } + // Stream-extract everything to disk await zip.extract(null, eventDir); await zip.close(); diff --git a/backend/src/services/picpeakImportService.js b/backend/src/services/picpeakImportService.js index b1a6b0f3..a7a6294b 100644 --- a/backend/src/services/picpeakImportService.js +++ b/backend/src/services/picpeakImportService.js @@ -18,6 +18,7 @@ const fsp = require('fs').promises; const path = require('path'); const os = require('os'); const StreamZip = require('node-stream-zip'); +const { assertZipEntriesWithin } = require('../utils/safePath'); const { db } = require('../database/db'); const knexConfig = require('../../knexfile'); const { getStoragePath } = require('../config/storage'); @@ -232,6 +233,10 @@ async function importFromPicpeak({ picpeakPath, currentAdminId }) { try { const zip = new StreamZip.async({ file: picpeakPath }); try { + // Reject ZIP-slip entries before extracting — a crafted .picpeak could + // otherwise write outside the staging dir via `../` entry names + // (same class as GHSA-jfhw-fj23-fx6x). + assertZipEntriesWithin(Object.values(await zip.entries()), staging); await zip.extract(null, staging); } finally { await zip.close(); diff --git a/backend/src/utils/safePath.js b/backend/src/utils/safePath.js index a1dca605..a90553bf 100644 --- a/backend/src/utils/safePath.js +++ b/backend/src/utils/safePath.js @@ -118,7 +118,41 @@ function assertContractPdfPath(filePath) { ]); } +/** + * ZIP-slip guard. `node-stream-zip`'s `extract(null, root)` writes each entry + * to `path.join(root, entry.name)` without neutralising `../` — a crafted + * archive with an entry named `../../uploads/logos/evil.svg` escapes `root` + * and overwrites arbitrary files (GHSA-jfhw-fj23-fx6x). Call this with the + * entry list BEFORE extract() to reject any entry that resolves outside the + * target directory. + * + * Purely lexical (path.resolve, no realpath) because the extraction target + * does not exist on disk yet. Absolute entry names (`/etc/passwd`) resolve + * away from `root` and are caught too. Throws AppError 400 on the first + * offending entry so the whole archive is refused. + * + * @param {Array<{name?: string}>} entries node-stream-zip entry objects + * @param {string} extractRoot directory extract() will write into + */ +function assertZipEntriesWithin(entries, extractRoot) { + const rootResolved = path.resolve(extractRoot); + const prefix = rootResolved.endsWith(path.sep) ? rootResolved : rootResolved + path.sep; + for (const entry of entries || []) { + const name = entry && entry.name; + if (!name) continue; + const target = path.resolve(rootResolved, name); + if (target !== rootResolved && !target.startsWith(prefix)) { + throw new AppError( + `Archive contains an entry that escapes the extraction directory: ${name}`, + 400, + 'ZIP_SLIP' + ); + } + } +} + module.exports = { assertPathInside, assertContractPdfPath, + assertZipEntriesWithin, }; From 31bc01cb4bbf65b48b3a5c3c94ad35e487df9fcc Mon Sep 17 00:00:00 2001 From: Paul Nothaft <53005142+the-luap@users.noreply.github.com> Date: Thu, 16 Jul 2026 10:55:10 +0200 Subject: [PATCH 07/10] fix(security): sanitize chunked-upload filename (GHSA-pc72-jf53-w28j) The chunked video upload stored req.body.filename unmodified and later built the merged path as path.join(tempDir, uploadMeta.filename). path.join does not neutralise '../', so a filename like '../../uploads/logos/evil.svg' escaped the temp dir on merge and overwrote arbitrary files. Requires admin with photos.upload. Fix: path.basename() the client filename in initializeUpload() and reject names that collapse to nothing. Adds a regression test. --- .../services/chunkedUploadFilename.test.js | 52 +++++++++++++++++++ backend/src/services/chunkedUploadService.js | 14 ++++- 2 files changed, 64 insertions(+), 2 deletions(-) create mode 100644 backend/__tests__/services/chunkedUploadFilename.test.js diff --git a/backend/__tests__/services/chunkedUploadFilename.test.js b/backend/__tests__/services/chunkedUploadFilename.test.js new file mode 100644 index 00000000..fd9fd34f --- /dev/null +++ b/backend/__tests__/services/chunkedUploadFilename.test.js @@ -0,0 +1,52 @@ +const path = require('path'); +const os = require('os'); +const fs = require('fs').promises; + +// Point storage at a throwaway temp dir before requiring the service so the +// module-level getStoragePath() picks it up if evaluated. +process.env.STORAGE_PATH = path.join(os.tmpdir(), `picpeak-chunk-test-${process.pid}`); + +const chunkedUpload = require('../../src/services/chunkedUploadService'); + +describe('chunkedUploadService.initializeUpload filename sanitisation (GHSA-pc72-jf53-w28j)', () => { + afterAll(async () => { + await fs.rm(process.env.STORAGE_PATH, { recursive: true, force: true }).catch(() => {}); + }); + + it('strips directory-traversal components from the stored filename', async () => { + const { uploadId } = await chunkedUpload.initializeUpload({ + filename: '../../uploads/logos/evil.svg', + fileSize: 10, + mimeType: 'video/mp4', + eventId: 1, + totalChunks: 1, + }); + const meta = chunkedUpload.getUploadStatus(uploadId); + // basename('../../uploads/logos/evil.svg') === 'evil.svg' — the traversal + // is gone, so path.join(tempDir, filename) can no longer escape tempDir. + expect(meta.filename).toBe('evil.svg'); + }); + + it('keeps a normal filename intact', async () => { + const { uploadId } = await chunkedUpload.initializeUpload({ + filename: 'clip.mp4', + fileSize: 10, + mimeType: 'video/mp4', + eventId: 1, + totalChunks: 1, + }); + expect(uploadId).toBeTruthy(); + }); + + it('rejects a filename that collapses to nothing', async () => { + await expect( + chunkedUpload.initializeUpload({ + filename: '../', + fileSize: 10, + mimeType: 'video/mp4', + eventId: 1, + totalChunks: 1, + }) + ).rejects.toThrow(/Invalid filename/); + }); +}); diff --git a/backend/src/services/chunkedUploadService.js b/backend/src/services/chunkedUploadService.js index 17cc8bb7..b3fe43ff 100644 --- a/backend/src/services/chunkedUploadService.js +++ b/backend/src/services/chunkedUploadService.js @@ -30,6 +30,16 @@ async function initializeUpload(options) { totalChunks } = options; + // Strip any directory components from the client-supplied filename. It is + // later joined onto the temp merge dir (path.join(tempDir, filename)), and + // path.join does NOT neutralise `../` — a filename like `../../uploads/ + // logos/evil.svg` would escape the temp dir and overwrite arbitrary files + // (GHSA-pc72-jf53-w28j). basename() collapses it to the leaf name only. + const safeFilename = path.basename(String(filename || '')); + if (!safeFilename || safeFilename === '.' || safeFilename === '..') { + throw new Error('Invalid filename'); + } + // Generate unique upload ID const uploadId = crypto.randomUUID(); @@ -43,7 +53,7 @@ async function initializeUpload(options) { // Store upload metadata const uploadMeta = { uploadId, - filename, + filename: safeFilename, fileSize, mimeType, eventId, @@ -59,7 +69,7 @@ async function initializeUpload(options) { logger.info('Initialized chunked upload', { uploadId, - filename, + filename: safeFilename, fileSize, expectedChunks, eventId From 38fd41aad3fcb12a249aaa2eb3d98fbffbde537a Mon Sep 17 00:00:00 2001 From: Paul Nothaft <53005142+the-luap@users.noreply.github.com> Date: Thu, 16 Jul 2026 11:57:54 +0200 Subject: [PATCH 08/10] fix(security): harden .picpeak restore operator-preservation (GHSA-qxfx follow-up) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The req.admin.id fix activated reinjectCurrentAdmin(); hardening its preservation logic (found across Codex review rounds of #811): - MFA hijack: reinject wrote back only password_hash/is_active/ must_change_password, leaving a crafted backup's two_factor_* on the operator's row — it could strip or replace their second factor. The email- matched row is now updated with the operator's full AUTH set (login identity, password, and all two_factor_* columns). Relationship/audit FKs (role_id, created_by) are deliberately NOT forced from the snapshot: on a cross-instance restore those pre-restore ids may be absent from the backup and would dangle the FK (SQLite rolls back at commit); the restored row keeps its own valid values. - Cross-instance restore rollback / FK safety: reinject matched only by email, so a backup shipping a different admin with the default `admin` username hit UNIQUE(username) and rolled the whole restore back; email and username could even collide on two different rows. Reconciliation is now non-destructive: the email-matching row is updated in place (id preserved → restored FKs like events.created_by stay valid); any different row holding the operator's username is RENAMED, not deleted (deletion would fire ON DELETE actions / dangle references); only when no row has the operator's email is a fresh row inserted, with created_by nulled and an explicit max(id)+1 id (batchInsert left the Postgres identity sequence unadvanced, so a sequence-based insert could collide). - Stale session after restore: admin_users ids shift on restore, but the operator's live JWT is bound only to decoded.id (IP logged not enforced; the backup controls password_changed_at). The route now revokes the token (result checked and logged) and clears the admin cookie; the client redirects to a fresh login via a sessionInvalidated flag. Cookie clear is the unconditional guarantee. Adds SQLite-backed reinject regression tests (in-place login/MFA restore with id and FK columns preserved, username-only rename, email+username on different rows, clean insert with created_by nulled) and the frontend redirect on sessionInvalidated. Deferred (design decisions / pre-existing, need a Postgres test env — see PR discussion): global "invalidate all pre-restore sessions" cutoff; preserving the operator's ROLE semantics across an RBAC-table replace; and resyncing Postgres identity sequences after any restore (batchInsert leaves them behind max(id) — pre-existing, affects every restored table). --- .../services/picpeakReinjectAdmin.test.js | 111 ++++++++++++++++++ backend/src/routes/adminBackup.js | 29 +++++ backend/src/services/picpeakImportService.js | 87 ++++++++++++-- .../components/admin/PicpeakBackupCard.tsx | 8 ++ 4 files changed, 222 insertions(+), 13 deletions(-) create mode 100644 backend/__tests__/services/picpeakReinjectAdmin.test.js diff --git a/backend/__tests__/services/picpeakReinjectAdmin.test.js b/backend/__tests__/services/picpeakReinjectAdmin.test.js new file mode 100644 index 00000000..fe08e444 --- /dev/null +++ b/backend/__tests__/services/picpeakReinjectAdmin.test.js @@ -0,0 +1,111 @@ +/** + * Regression tests for reinjectCurrentAdmin — the operator-preservation step of + * the .picpeak restore (GHSA-qxfx-4493-4v8f follow-up). Runs against a real + * in-memory SQLite DB so the UNIQUE(email)/UNIQUE(username) constraints behave + * as in production. Reconciliation is non-destructive (update-in-place / rename, + * never delete) so restored rows referenced by FKs keep their ids. + */ +const knex = require('knex'); + +let db; +let reinjectCurrentAdmin; + +beforeAll(() => { + jest.doMock('../../knexfile', () => ({ client: 'sqlite3' }), { virtual: false }); + reinjectCurrentAdmin = require('../../src/services/picpeakImportService').reinjectCurrentAdmin; +}); + +beforeEach(async () => { + db = knex({ client: 'sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }); + await db.schema.createTable('admin_users', (t) => { + t.increments('id'); + t.string('username').notNullable().unique(); + t.string('email').notNullable().unique(); + t.string('password_hash'); + t.boolean('is_active').defaultTo(true); + t.boolean('must_change_password').defaultTo(false); + t.integer('role_id'); + t.integer('created_by'); + t.boolean('two_factor_enabled').defaultTo(false); + t.string('two_factor_secret'); + t.text('two_factor_recovery_codes'); + }); +}); + +afterEach(async () => { await db.destroy(); }); + +const operator = { + id: 1, username: 'admin', email: 'op@example.com', + password_hash: 'OP_HASH', is_active: 1, must_change_password: 0, role_id: 1, created_by: 99, + two_factor_enabled: 1, two_factor_secret: 'OP_SECRET', two_factor_recovery_codes: '["a","b"]', +}; + +test('restores login + MFA in place, keeping the row id and its FK columns (FK-safe)', async () => { + await db('admin_users').insert({ + id: 7, username: 'someoneelse', email: 'OP@example.com', + password_hash: 'ATTACKER', is_active: 1, must_change_password: 0, role_id: 4, created_by: 5, + two_factor_enabled: 0, two_factor_secret: 'ATTACKER_SECRET', two_factor_recovery_codes: null, + }); + await db.transaction((trx) => reinjectCurrentAdmin(trx, operator)); + + const rows = await db('admin_users'); + expect(rows).toHaveLength(1); + const row = rows[0]; + expect(row.id).toBe(7); // id preserved → FK refs hold + expect(row.username).toBe('admin'); + expect(row.password_hash).toBe('OP_HASH'); + expect(Boolean(row.two_factor_enabled)).toBe(true); + expect(row.two_factor_secret).toBe('OP_SECRET'); // attacker MFA secret gone + expect(row.two_factor_recovery_codes).toBe('["a","b"]'); + // Relationship/audit FKs are NOT forced from the operator snapshot (avoids + // dangling role_id/created_by on a cross-instance restore) — the restored + // row keeps its own already-valid values. + expect(row.role_id).toBe(4); + expect(row.created_by).toBe(5); +}); + +test('renames (not deletes) a different row holding the operator username', async () => { + await db('admin_users').insert({ + id: 3, username: 'admin', email: 'other@instance.test', + password_hash: 'OTHER', is_active: 1, role_id: 4, + }); + await expect(db.transaction((trx) => reinjectCurrentAdmin(trx, operator))).resolves.not.toThrow(); + + const rows = await db('admin_users').orderBy('id'); + expect(rows).toHaveLength(2); // the other admin survives (FK-safe) + const other = rows.find((r) => r.id === 3); + expect(other.username).toBe('admin__restored_3'); // renamed, id kept + expect(other.email).toBe('other@instance.test'); + const op = rows.find((r) => r.username === 'admin'); + expect(op.password_hash).toBe('OP_HASH'); +}); + +test('reconciles email and username colliding with DIFFERENT rows without deleting either', async () => { + await db('admin_users').insert([ + { id: 4, username: 'someoneelse', email: 'op@example.com', password_hash: 'A', role_id: 4 }, + { id: 5, username: 'admin', email: 'other@instance.test', password_hash: 'B', role_id: 4 }, + ]); + await expect(db.transaction((trx) => reinjectCurrentAdmin(trx, operator))).resolves.not.toThrow(); + + const rows = await db('admin_users').orderBy('id'); + expect(rows).toHaveLength(2); // both rows survive + const opRow = rows.find((r) => r.id === 4); // email match updated in place + expect(opRow.username).toBe('admin'); + expect(opRow.password_hash).toBe('OP_HASH'); + const renamed = rows.find((r) => r.id === 5); // username holder renamed, not deleted + expect(renamed.username).toBe('admin__restored_5'); +}); + +test('inserts the operator with a non-colliding id when neither key exists in the backup', async () => { + await db('admin_users').insert({ + id: 9, username: 'backupadmin', email: 'backup@instance.test', password_hash: 'B', role_id: 1, + }); + await db.transaction((trx) => reinjectCurrentAdmin(trx, operator)); + + const rows = await db('admin_users').orderBy('id'); + expect(rows).toHaveLength(2); // backup admin untouched + const opRow = rows.find((r) => r.username === 'admin'); + expect(opRow.password_hash).toBe('OP_HASH'); + expect(opRow.id).toBe(10); // max(9)+1, no collision + expect(opRow.created_by).toBeNull(); // self-ref FK nulled so the insert can't dangle +}); diff --git a/backend/src/routes/adminBackup.js b/backend/src/routes/adminBackup.js index b1ee0e97..61705c91 100644 --- a/backend/src/routes/adminBackup.js +++ b/backend/src/routes/adminBackup.js @@ -2,6 +2,8 @@ const express = require('express'); const { db } = require('../database/db'); const { adminAuth } = require('../middleware/auth'); const { requirePermission } = require('../middleware/permissions'); +const { clearAdminAuthCookie } = require('../utils/tokenUtils'); +const { revokeToken } = require('../utils/tokenRevocation'); const { triggerManualBackup, getBackupStatus, cleanupOldBackupRuns, getBackupManifest, validateBackupManifest } = require('../services/backupService'); const logger = require('../utils/logger'); const { errorResponse, getPagination } = require('../utils/routeHelpers'); @@ -183,11 +185,38 @@ router.post('/picpeak/import', adminAuth, requirePermission('backup.restore'), p // to preserve and the admin_users table was fully replaced by the backup — // letting a crafted .picpeak take over every admin account (GHSA-qxfx-4493-4v8f). const result = await importFromPicpeak({ picpeakPath, currentAdminId: req.admin && req.admin.id }); + + // The restore rewrote admin_users, so ids may have shifted. The operator's + // current JWT is bound only to the pre-restore admin id (adminAuth trusts + // `decoded.id` — IP is logged, not enforced, and the backup controls + // password_changed_at), which could now resolve to a DIFFERENT restored + // account and silently grant its permissions. Force a fresh login instead + // of trusting the old session: revoke the token and clear the cookie. + // Clearing the cookie is the guarantee — it drops the operator's browser + // session unconditionally. Revocation is the extra layer that also kills a + // Bearer-header copy of the JWT; revokeToken() swallows DB errors and + // returns false, so check the result and log loudly if the denylist write + // didn't land (the operator should still re-login, which the cookie clear + // forces). + let tokenRevoked = false; + try { + if (req.token) { + tokenRevoked = await revokeToken(req.token, 'picpeak-import', { adminId: req.admin && req.admin.id }); + } + } catch (revokeErr) { + logger.warn('[picpeak-import] failed to revoke session token after restore', { error: revokeErr.message }); + } + if (req.token && !tokenRevoked) { + logger.warn('[picpeak-import] session token was NOT added to the revocation denylist after restore; relying on cookie clear to force re-login'); + } + clearAdminAuthCookie(res); + res.json({ success: true, tables: result.tables, filesRestored: result.filesRestored, usesExternalMedia: result.usesExternalMedia, + sessionInvalidated: true, }); } catch (error) { const status = error.statusCode || 500; diff --git a/backend/src/services/picpeakImportService.js b/backend/src/services/picpeakImportService.js index a7a6294b..27ac33a0 100644 --- a/backend/src/services/picpeakImportService.js +++ b/backend/src/services/picpeakImportService.js @@ -81,25 +81,85 @@ function parseNdjson(filePath) { } // Re-insert the operator's account inside the restore transaction so they keep -// working credentials. If the backup already loaded an admin with the same -// email, overwrite that row's credentials with the current account's (current -// creds win); otherwise insert the snapshot with a fresh id. +// working credentials after the wipe. +// +// The operator's login + credentials + MFA must be restored, not just the +// password. A crafted backup can carry a row with the operator's email whose +// two_factor_* fields are attacker-chosen — leaving those in place would let +// the backup strip or hijack the operator's MFA, or (cross-instance) pin a TOTP +// secret encrypted with the source instance's key the operator can never +// satisfy. These columns are scalar/text (recovery codes are a JSON string in a +// TEXT column), so writing them needs no special json handling. Relationship/ +// audit FKs (role_id, created_by) are deliberately NOT forced from the snapshot +// — see the update branch below. +// +// admin_users has UNIQUE constraints on BOTH email and username, and a restored +// backup can collide with the operator on either — possibly on two DIFFERENT +// rows (one shares the email, another shares the default `admin` username). We +// reconcile WITHOUT deleting any restored row: deleting would fire ON DELETE +// actions (SQLite) or dangle references such as events.created_by (Postgres, +// where replica mode suppresses cascades). Instead: +// - if a row already has the operator's email, overwrite it in place (its id +// is preserved, so every FK pointing at the operator stays valid); +// - if a DIFFERENT row holds the operator's username, rename that row (id +// preserved, its own FKs stay valid) to free the username; +// - only when no row has the operator's email do we insert a fresh row. async function reinjectCurrentAdmin(trx, currentAdmin) { if (!currentAdmin) return; - const existing = await trx('admin_users').whereRaw('lower(email) = lower(?)', [currentAdmin.email]).first(); - if (existing) { - await trx('admin_users').where({ id: existing.id }).update({ - password_hash: currentAdmin.password_hash, - is_active: currentAdmin.is_active, - must_change_password: currentAdmin.must_change_password, - }); + + const emailMatch = await trx('admin_users') + .whereRaw('lower(email) = lower(?)', [currentAdmin.email]) + .first(); + + // Free the operator's username if a different row holds it (rename, not delete). + const usernameHolder = await trx('admin_users') + .whereRaw('lower(username) = lower(?)', [currentAdmin.username]) + .first(); + if (usernameHolder && (!emailMatch || usernameHolder.id !== emailMatch.id)) { + await trx('admin_users') + .where({ id: usernameHolder.id }) + .update({ username: `${usernameHolder.username}__restored_${usernameHolder.id}` }); + } + + if (emailMatch) { + // Update in place — keeps emailMatch.id so restored FKs to the operator + // hold. Write only the AUTH-critical columns (login identity + credentials + // + MFA), never the relationship/audit FKs (role_id → roles, created_by → + // admin_users). Forcing the operator's pre-restore role_id/created_by here + // could reference rows absent from a cross-instance backup and dangle the + // FK (SQLite rolls back at commit); the row already carries the backup's + // own valid values for those. This still closes the MFA-hijack gap — a + // crafted backup can't strip or replace the operator's second factor. + const authUpdate = {}; + for (const field of PRESERVED_AUTH_FIELDS) { + if (field in currentAdmin) authUpdate[field] = currentAdmin[field]; + } + await trx('admin_users').where({ id: emailMatch.id }).update(authUpdate); } else { - const row = { ...currentAdmin }; - delete row.id; // let the engine assign a fresh id to avoid collision - await trx('admin_users').insert(row); + // The operator's email isn't in the backup, so nothing restored references + // their id — a fresh row can't dangle a reference TO the operator. Null the + // self-referential created_by (its target admin may be absent from this + // backup; ON DELETE SET NULL makes null the correct "unknown inviter" + // value) so the insert itself can't dangle. Use an explicit max(id)+1 + // rather than the identity sequence, which batchInsert left unadvanced on + // Postgres (a sequence-based insert could collide with a restored id). + const snapshot = { ...currentAdmin }; + delete snapshot.id; + if ('created_by' in snapshot) snapshot.created_by = null; + const maxRow = await trx('admin_users').max({ m: 'id' }).first(); + snapshot.id = (Number(maxRow && maxRow.m) || 0) + 1; + await trx('admin_users').insert(snapshot); } } +// AUTH-critical admin_users columns preserved when overwriting a restored row +// that shares the operator's email. Deliberately excludes relationship/audit +// FKs (role_id, created_by) — see reinjectCurrentAdmin for why. +const PRESERVED_AUTH_FIELDS = [ + 'username', 'email', 'password_hash', 'is_active', 'must_change_password', + 'two_factor_enabled', 'two_factor_secret', 'two_factor_recovery_codes', 'two_factor_enrolled_at', +]; + // The json/jsonb columns of a table (Postgres only). The pg driver returns // jsonb as parsed JS values, so on re-insert they must be serialised back to // valid JSON text — otherwise a scalar like the string "PicPeak" is sent @@ -273,4 +333,5 @@ module.exports = { importFromPicpeak, readManifestFromZip, validateManifest, + reinjectCurrentAdmin, }; diff --git a/frontend/src/components/admin/PicpeakBackupCard.tsx b/frontend/src/components/admin/PicpeakBackupCard.tsx index 10ae1e60..1c9f947f 100644 --- a/frontend/src/components/admin/PicpeakBackupCard.tsx +++ b/frontend/src/components/admin/PicpeakBackupCard.tsx @@ -16,6 +16,7 @@ interface RestoreResult { tables: number; filesRestored: number; usesExternalMedia: boolean; + sessionInvalidated?: boolean; } // ── Download half (Dashboard) ──────────────────────────────────────────────── @@ -114,6 +115,13 @@ export const PicpeakRestoreCard: React.FC = () => { setResult(res.data); setPendingFile(null); toast.success(t('backup.picpeak.restoreDone', 'Backup restored.')); + // The restore rewrote admin_users and the backend revoked our session + // (ids may have shifted). Send the operator to a fresh login rather than + // letting the now-stale token resolve to a different restored account. + if (res.data?.sessionInvalidated) { + toast.success(t('backup.picpeak.reloginRequired', 'Restore complete — please sign in again.')); + setTimeout(() => { window.location.href = '/admin/login'; }, 1500); + } } catch (e: any) { const msg = e.response?.data?.error || t('backup.picpeak.restoreFailed', 'Restore failed.'); toast.error(msg); From 340d91bdd53a595694edfa6f3d691b240a2babcd Mon Sep 17 00:00:00 2001 From: Paul Nothaft <53005142+the-luap@users.noreply.github.com> Date: Thu, 16 Jul 2026 12:48:25 +0200 Subject: [PATCH 09/10] =?UTF-8?q?feat(security):=20harden=20.picpeak=20res?= =?UTF-8?q?tore=20robustness=20=E2=80=94=20sessions,=20roles,=20sequences?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements the three restore-hardening items deferred from the #811 Codex review (all validated against a real Postgres, see __tests__/integration/ picpeakRestorePg.test.js). Backend-only; targets main (feature, not a backport). 1. Global session cutoff (utils/sessionCutoff.js). A restore reassigns admin/ customer/event ids, so ANY pre-restore JWT can rebind to a different restored principal. Revoking just the importing token wasn't enough. importFromPicpeak now stamps a unix-second cutoff in app_settings after the restore commits, and adminAuth / galleryAuth / verifyGalleryAccess / customerAuth reject any token whose iat predates it (cached 30s → one in-memory compare on the hot path). The operator's forced re-login mints a token past the cutoff, so it passes. 2. Role preservation across an RBAC replace (captureOperatorRole / preserveOperatorRole). The operator's role + granted permission NAMES are captured before the wipe; after roles/role_permissions are replaced the role is resolved by NAME against the restored data, and re-created with its grants if the backup omits it — so a crafted or cross-instance backup can't silently downgrade or lock out the operator. reinjectCurrentAdmin now returns the operator's id so the row can be re-pointed at the resolved role. 3. Postgres identity-sequence resync (resyncSequences). batchInsert writes explicit ids without advancing the sequences, so the next natural insert into any restored table collided on the PK. Runs AFTER commit (setval isn't transactional) and guards every table with a column-existence check — pg_get_serial_sequence RAISES on id-less tables like role_permissions. No-op on SQLite. Tests: SQLite unit tests for the cutoff and role preservation; a gated Postgres integration suite (npm run test:pg with PICPEAK_PG_TEST_URL) covering sequence resync, the id-less-table guard, explicit-id reinject, role re-creation, and a full cross-instance replaceAllTables run asserting operator preservation, role re-establishment, FK integrity, and collision-free post-restore inserts. Stacks on #811 (shares the reinject hardening); merge after it. --- .../integration/picpeakRestorePg.test.js | 190 ++++++++++++++++++ .../services/picpeakRolePreserve.test.js | 105 ++++++++++ backend/__tests__/utils/sessionCutoff.test.js | 56 ++++++ backend/package.json | 1 + .../__tests__/customerAuth.middleware.test.js | 7 + backend/src/middleware/auth.js | 20 +- backend/src/middleware/customerAuth.js | 6 + backend/src/routes/adminBackup.js | 21 +- backend/src/services/picpeakImportService.js | 106 +++++++++- backend/src/utils/sessionCutoff.js | 100 +++++++++ 10 files changed, 595 insertions(+), 17 deletions(-) create mode 100644 backend/__tests__/integration/picpeakRestorePg.test.js create mode 100644 backend/__tests__/services/picpeakRolePreserve.test.js create mode 100644 backend/__tests__/utils/sessionCutoff.test.js create mode 100644 backend/src/utils/sessionCutoff.js diff --git a/backend/__tests__/integration/picpeakRestorePg.test.js b/backend/__tests__/integration/picpeakRestorePg.test.js new file mode 100644 index 00000000..09e9f468 --- /dev/null +++ b/backend/__tests__/integration/picpeakRestorePg.test.js @@ -0,0 +1,190 @@ +/** + * PostgreSQL integration tests for the .picpeak restore robustness fixes. + * Gated: runs only when PICPEAK_PG_TEST_URL points at a throwaway Postgres DB, + * e.g. + * PICPEAK_PG_TEST_URL="postgres://picpeak:picpeak_secure_pass_2024@127.0.0.1:7102/picpeak_restore_test" \ + * npx jest __tests__/integration/picpeakRestorePg.test.js + * + * Validates the Postgres-specific paths that SQLite can't exercise: identity + * sequences left stale by explicit-id inserts, pg_get_serial_sequence raising on + * id-less tables, reinject/role-recreate explicit-id inserts, and FK integrity. + */ +const knex = require('knex'); +const fs = require('fs'); +const os = require('os'); +const path = require('path'); + +const PG_URL = process.env.PICPEAK_PG_TEST_URL; +const maybe = PG_URL ? describe : describe.skip; + +maybe('picpeak restore on Postgres', () => { + let pgDb; + let svc; + + beforeAll(async () => { + pgDb = knex({ client: 'pg', connection: PG_URL }); + + await pgDb.raw('DROP TABLE IF EXISTS role_permissions, events, admin_users, roles, permissions, app_settings CASCADE'); + await pgDb.schema.createTable('roles', (t) => { + t.increments('id'); + t.string('name', 50).notNullable().unique(); + t.string('display_name', 100); + t.integer('priority').defaultTo(0); + t.boolean('is_system').defaultTo(false); + }); + await pgDb.schema.createTable('permissions', (t) => { + t.increments('id'); + t.string('name', 100).notNullable().unique(); + t.string('display_name', 150); + t.string('category', 50); + }); + await pgDb.schema.createTable('role_permissions', (t) => { + t.integer('role_id').notNullable().references('id').inTable('roles').onDelete('CASCADE'); + t.integer('permission_id').notNullable().references('id').inTable('permissions').onDelete('CASCADE'); + t.primary(['role_id', 'permission_id']); + }); + await pgDb.schema.createTable('admin_users', (t) => { + t.increments('id'); + t.string('username').notNullable().unique(); + t.string('email').notNullable().unique(); + t.string('password_hash'); + t.boolean('is_active').defaultTo(true); + t.boolean('must_change_password').defaultTo(false); + t.integer('role_id').references('id').inTable('roles').onDelete('SET NULL'); + t.integer('created_by').references('id').inTable('admin_users').onDelete('SET NULL'); + t.boolean('two_factor_enabled').defaultTo(false); + t.string('two_factor_secret'); + t.text('two_factor_recovery_codes'); + }); + await pgDb.schema.createTable('events', (t) => { + t.increments('id'); + t.string('slug'); + t.integer('created_by').references('id').inTable('admin_users').onDelete('SET NULL'); + }); + await pgDb.schema.createTable('app_settings', (t) => { + t.increments('id'); + t.string('setting_key').notNullable().unique(); + t.json('setting_value'); + t.string('setting_type'); + t.timestamp('updated_at').defaultTo(pgDb.fn.now()); + }); + + jest.resetModules(); + jest.doMock('../../knexfile', () => ({ client: 'pg' })); + jest.doMock('../../src/database/db', () => ({ db: pgDb })); + svc = require('../../src/services/picpeakImportService'); + }); + + afterAll(async () => { + jest.dontMock('../../src/database/db'); + jest.dontMock('../../knexfile'); + if (pgDb) await pgDb.destroy(); + }); + + beforeEach(async () => { + await pgDb('role_permissions').del(); + await pgDb('events').del(); + await pgDb('admin_users').del(); + await pgDb('roles').del(); + await pgDb('permissions').del(); + }); + + test('resyncSequences fast-forwards stale sequences and skips id-less tables', async () => { + // Simulate a restore: explicit-id inserts leave the sequence at 1. + await pgDb('roles').insert([{ id: 5, name: 'super_admin', display_name: 'SA' }]); + await pgDb('admin_users').insert([{ id: 9, username: 'a', email: 'a@x.io', password_hash: 'h' }]); + await pgDb('permissions').insert([{ id: 3, name: 'events.create', display_name: 'C', category: 'events' }]); + await pgDb('role_permissions').insert([{ role_id: 5, permission_id: 3 }]); // id-less table + + // Must not throw on role_permissions (no `id` column → pg_get_serial_sequence raises unguarded). + await expect(svc.resyncSequences(['roles', 'admin_users', 'permissions', 'role_permissions'])).resolves.toBeUndefined(); + + // Natural inserts (no explicit id) now avoid the restored ids. + const [adminId] = await pgDb('admin_users').insert({ username: 'b', email: 'b@x.io', password_hash: 'h' }).returning('id'); + expect(Number(adminId.id || adminId)).toBe(10); // max(9)+1, no duplicate-key error + const [roleId] = await pgDb('roles').insert({ name: 'editor', display_name: 'Ed' }).returning('id'); + expect(Number(roleId.id || roleId)).toBe(6); + }); + + test('reinjectCurrentAdmin insert branch works with a stale sequence (explicit max+1)', async () => { + await pgDb('admin_users').insert({ id: 9, username: 'backup', email: 'backup@x.io', password_hash: 'h' }); + const operator = { id: 1, username: 'admin', email: 'op@x.io', password_hash: 'OP', is_active: true, created_by: 42 }; + + await pgDb.transaction((trx) => svc.reinjectCurrentAdmin(trx, operator)); + + const op = await pgDb('admin_users').where({ email: 'op@x.io' }).first(); + expect(op.id).toBe(10); // max(9)+1 + expect(op.password_hash).toBe('OP'); + expect(op.created_by).toBeNull(); // self-ref FK nulled so the insert can't dangle + }); + + test('preserveOperatorRole re-creates a missing role on Postgres and keeps FK integrity', async () => { + await pgDb('permissions').insert([{ id: 3, name: 'events.create', display_name: 'C', category: 'events' }]); + await pgDb('roles').insert([{ id: 2, name: 'viewer', display_name: 'V' }]); + await pgDb('admin_users').insert({ id: 1, username: 'admin', email: 'op@x.io', password_hash: 'h', role_id: null }); + const snapshot = { role: { name: 'super_admin', display_name: 'SA', priority: 100, is_system: true }, permissions: ['events.create', 'missing.perm'] }; + + await pgDb.transaction((trx) => svc.preserveOperatorRole(trx, 1, snapshot)); + await svc.resyncSequences(['roles']); // post-commit, mirrors importFromPicpeak + + const role = await pgDb('roles').where({ name: 'super_admin' }).first(); + expect(role).toBeTruthy(); + const op = await pgDb('admin_users').where({ id: 1 }).first(); + expect(op.role_id).toBe(role.id); // FK valid, operator not downgraded + const grants = await pgDb('role_permissions').where({ role_id: role.id }).pluck('permission_id'); + expect(grants).toEqual([3]); // existing perm granted, missing.perm skipped + }); + + test('full replaceAllTables: cross-instance backup preserves the operator, role, FKs, and sequences', async () => { + // A backup from ANOTHER instance: omits the operator's email AND their + // super_admin role; uses explicit ids that leave sequences stale. + const staging = fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-pgtest-')); + const dataDir = path.join(staging, 'data'); + fs.mkdirSync(dataDir); + const write = (t, rows) => fs.writeFileSync(path.join(dataDir, `${t}.ndjson`), rows.map((r) => JSON.stringify(r)).join('\n')); + write('roles', [{ id: 5, name: 'admin', display_name: 'Admin', priority: 50, is_system: true }]); + write('permissions', [{ id: 3, name: 'events.create', display_name: 'C', category: 'events' }]); + write('role_permissions', [{ role_id: 5, permission_id: 3 }]); + write('admin_users', [{ id: 9, username: 'backupadmin', email: 'backup@x.io', password_hash: 'h', role_id: 5, is_active: true }]); + write('events', [{ id: 2, slug: 'restored-ev', created_by: 9 }]); + + const operator = { id: 1, username: 'admin', email: 'op@x.io', password_hash: 'OP', is_active: true, role_id: 999, created_by: null }; + const roleSnapshot = { role: { name: 'super_admin', display_name: 'Super Admin', priority: 100, is_system: true }, permissions: ['events.create'] }; + const tables = ['roles', 'permissions', 'role_permissions', 'admin_users', 'events']; + + // replaceAllTables isn't exported, so drive its exact transaction sequence + // (suspend FKs, wipe, batchInsert, reinject, preserve role) through the + // exported units against real Postgres. + const importSvc = svc; + await pgDb.transaction(async (trx) => { + await trx.raw('SET session_replication_role = \'replica\''); + for (const t of tables) await trx(t).del(); + for (const t of tables) { + const rows = fs.readFileSync(path.join(dataDir, `${t}.ndjson`), 'utf8').split('\n').filter(Boolean).map((l) => JSON.parse(l)); + if (rows.length) await trx.batchInsert(t, rows, 100); + } + const opId = await importSvc.reinjectCurrentAdmin(trx, operator); + await importSvc.preserveOperatorRole(trx, opId, roleSnapshot); + await trx.raw('SET session_replication_role = \'origin\''); + }); + await importSvc.resyncSequences(tables); + + // Operator preserved (inserted, since email absent from backup). + const op = await pgDb('admin_users').where({ email: 'op@x.io' }).first(); + expect(op).toBeTruthy(); + expect(op.password_hash).toBe('OP'); + // super_admin role re-created and the operator bound to it. + const sa = await pgDb('roles').where({ name: 'super_admin' }).first(); + expect(sa).toBeTruthy(); + expect(op.role_id).toBe(sa.id); + expect(await pgDb('role_permissions').where({ role_id: sa.id }).pluck('permission_id')).toEqual([3]); + // Restored event's created_by FK to the backup admin still valid. + const ev = await pgDb('events').where({ slug: 'restored-ev' }).first(); + expect(ev.created_by).toBe(9); + // Sequences resynced → natural inserts don't collide. + const [newAdmin] = await pgDb('admin_users').insert({ username: 'fresh', email: 'fresh@x.io', password_hash: 'h' }).returning('id'); + expect(Number(newAdmin.id || newAdmin)).toBeGreaterThan(op.id); + + fs.rmSync(staging, { recursive: true, force: true }); + }); +}); diff --git a/backend/__tests__/services/picpeakRolePreserve.test.js b/backend/__tests__/services/picpeakRolePreserve.test.js new file mode 100644 index 00000000..81be6f07 --- /dev/null +++ b/backend/__tests__/services/picpeakRolePreserve.test.js @@ -0,0 +1,105 @@ +/** + * Tests for preserveOperatorRole — re-establishing the operator's authorization + * after a restore replaces the roles / permissions / role_permissions tables. + * Real in-memory SQLite so the joins and inserts behave as in production. + */ +const knex = require('knex'); + +let db; +let svc; + +beforeEach(async () => { + db = knex({ client: 'sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }); + await db.schema.createTable('roles', (t) => { + t.increments('id'); + t.string('name').notNullable().unique(); + t.string('display_name'); + t.integer('priority').defaultTo(0); + t.boolean('is_system').defaultTo(false); + }); + await db.schema.createTable('permissions', (t) => { + t.increments('id'); + t.string('name').notNullable().unique(); + t.string('display_name'); + t.string('category'); + }); + await db.schema.createTable('role_permissions', (t) => { + t.integer('role_id').notNullable(); + t.integer('permission_id').notNullable(); + t.primary(['role_id', 'permission_id']); + }); + await db.schema.createTable('admin_users', (t) => { + t.increments('id'); + t.string('email'); + t.integer('role_id'); + }); + jest.resetModules(); + jest.doMock('../../knexfile', () => ({ client: 'sqlite3' })); + jest.doMock('../../src/database/db', () => ({ db })); + svc = require('../../src/services/picpeakImportService'); +}); + +afterEach(async () => { + jest.dontMock('../../src/database/db'); + jest.dontMock('../../knexfile'); + await db.destroy(); +}); + +test('captureOperatorRole returns the role + its permission names', async () => { + await db('roles').insert({ id: 1, name: 'super_admin', display_name: 'Super Admin', priority: 100 }); + await db('permissions').insert([ + { id: 1, name: 'events.create', display_name: 'Create', category: 'events' }, + { id: 2, name: 'users.manage', display_name: 'Manage', category: 'users' }, + ]); + await db('role_permissions').insert([{ role_id: 1, permission_id: 1 }, { role_id: 1, permission_id: 2 }]); + + const snap = await svc.captureOperatorRole(1); + expect(snap.role.name).toBe('super_admin'); + expect(snap.permissions.sort()).toEqual(['events.create', 'users.manage']); +}); + +test('preserveOperatorRole binds to a restored role of the same NAME (ids remapped)', async () => { + const snapshot = { role: { name: 'super_admin', display_name: 'Super Admin', priority: 100, is_system: true }, permissions: ['events.create'] }; + // Simulate post-restore RBAC where super_admin now has a DIFFERENT id. + await db('roles').insert({ id: 7, name: 'super_admin', display_name: 'Super Admin (restored)', priority: 100 }); + await db('admin_users').insert({ id: 3, email: 'op@example.com', role_id: null }); + + await db.transaction((trx) => svc.preserveOperatorRole(trx, 3, snapshot)); + + const op = await db('admin_users').where({ id: 3 }).first(); + expect(op.role_id).toBe(7); // bound to restored super_admin by name + expect(await db('roles').count({ c: '*' }).first()).toEqual({ c: 1 }); // no duplicate role created +}); + +test('preserveOperatorRole re-creates the role + grants when the backup omits it', async () => { + const snapshot = { + role: { name: 'super_admin', display_name: 'Super Admin', priority: 100, is_system: true }, + permissions: ['events.create', 'users.manage', 'gone.permission'], + }; + // Post-restore RBAC WITHOUT super_admin; only some permissions exist. + await db('roles').insert({ id: 2, name: 'viewer', display_name: 'Viewer', priority: 10 }); + await db('permissions').insert([ + { id: 5, name: 'events.create', display_name: 'Create', category: 'events' }, + { id: 6, name: 'users.manage', display_name: 'Manage', category: 'users' }, + ]); + await db('admin_users').insert({ id: 3, email: 'op@example.com', role_id: null }); + + await db.transaction((trx) => svc.preserveOperatorRole(trx, 3, snapshot)); + + const recreated = await db('roles').where({ name: 'super_admin' }).first(); + expect(recreated).toBeTruthy(); // role re-created, not left missing + expect(recreated.id).toBe(3); // max(2)+1 + + const op = await db('admin_users').where({ id: 3 }).first(); + expect(op.role_id).toBe(recreated.id); // operator not locked out / downgraded + + const grants = await db('role_permissions').where({ role_id: recreated.id }).pluck('permission_id'); + expect(grants.sort()).toEqual([5, 6]); // existing perms re-granted; 'gone.permission' skipped +}); + +test('preserveOperatorRole no-ops when the operator had no role', async () => { + await db('admin_users').insert({ id: 3, email: 'op@example.com', role_id: null }); + await db.transaction((trx) => svc.preserveOperatorRole(trx, 3, null)); + const op = await db('admin_users').where({ id: 3 }).first(); + expect(op.role_id).toBeNull(); +}); diff --git a/backend/__tests__/utils/sessionCutoff.test.js b/backend/__tests__/utils/sessionCutoff.test.js new file mode 100644 index 00000000..2b0a5071 --- /dev/null +++ b/backend/__tests__/utils/sessionCutoff.test.js @@ -0,0 +1,56 @@ +/** + * Unit tests for the global session cutoff (utils/sessionCutoff.js). Uses a + * real in-memory SQLite `app_settings` table so the read/write/parse path is + * exercised exactly as in production. + */ +const knex = require('knex'); + +let db; +let cutoff; + +beforeEach(async () => { + db = knex({ client: 'sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }); + await db.schema.createTable('app_settings', (t) => { + t.increments('id'); + t.string('setting_key').notNullable().unique(); + t.text('setting_value'); + t.string('setting_type'); + t.timestamp('updated_at'); + }); + jest.resetModules(); + jest.doMock('../../src/database/db', () => ({ db })); + cutoff = require('../../src/utils/sessionCutoff'); + cutoff._resetCache(); +}); + +afterEach(async () => { + jest.dontMock('../../src/database/db'); + await db.destroy(); +}); + +test('no cutoff set → nothing is invalidated', async () => { + expect(await cutoff.getSessionsValidAfter()).toBe(0); + expect(await cutoff.isTokenBeforeCutoff({ iat: 1000 })).toBe(false); +}); + +test('token issued before the cutoff is rejected, at/after is accepted', async () => { + await cutoff.setSessionsValidAfter(2000); + expect(await cutoff.isTokenBeforeCutoff({ iat: 1999 })).toBe(true); // pre-restore session + expect(await cutoff.isTokenBeforeCutoff({ iat: 2000 })).toBe(false); // same second → kept + expect(await cutoff.isTokenBeforeCutoff({ iat: 2001 })).toBe(false); // post-restore login +}); + +test('setSessionsValidAfter upserts a single row and refreshes the cache', async () => { + await cutoff.setSessionsValidAfter(1000); + await cutoff.setSessionsValidAfter(3000); + const rows = await db('app_settings').where('setting_key', 'security_sessions_valid_after'); + expect(rows).toHaveLength(1); + cutoff._resetCache(); + expect(await cutoff.getSessionsValidAfter()).toBe(3000); +}); + +test('a token without iat is never treated as before the cutoff', async () => { + await cutoff.setSessionsValidAfter(2000); + expect(await cutoff.isTokenBeforeCutoff({})).toBe(false); + expect(await cutoff.isTokenBeforeCutoff(null)).toBe(false); +}); diff --git a/backend/package.json b/backend/package.json index d64d7a15..449739f2 100644 --- a/backend/package.json +++ b/backend/package.json @@ -11,6 +11,7 @@ "generate:watermarks": "node scripts/generate-watermarks.js", "test": "jest", "test:s3": "SKIP_S3_TESTS=false jest __tests__/integration/backup-s3", + "test:pg": "jest __tests__/integration/picpeakRestorePg", "lint": "eslint src/" }, "dependencies": { diff --git a/backend/src/__tests__/customerAuth.middleware.test.js b/backend/src/__tests__/customerAuth.middleware.test.js index e39b9dde..649a2cc4 100644 --- a/backend/src/__tests__/customerAuth.middleware.test.js +++ b/backend/src/__tests__/customerAuth.middleware.test.js @@ -33,6 +33,13 @@ jest.mock('../utils/tokenRevocation', () => ({ isTokenRevoked: jest.fn(), })); +// The global session cutoff (added for .picpeak restore invalidation) queries +// app_settings; stub it to "no cutoff" so it doesn't consume this suite's +// one-shot db() mock. Its own behaviour is covered by utils/sessionCutoff.test.js. +jest.mock('../utils/sessionCutoff', () => ({ + isTokenBeforeCutoff: jest.fn().mockResolvedValue(false), +})); + jest.mock('../utils/tokenUtils', () => ({ getCustomerTokenFromRequest: jest.fn(), })); diff --git a/backend/src/middleware/auth.js b/backend/src/middleware/auth.js index ccb49903..ed6faba5 100644 --- a/backend/src/middleware/auth.js +++ b/backend/src/middleware/auth.js @@ -2,6 +2,7 @@ const jwt = require('jsonwebtoken'); const { db } = require('../database/db'); const { formatBoolean } = require('../utils/dbCompat'); const { isTokenRevoked } = require('../utils/tokenRevocation'); +const { isTokenBeforeCutoff } = require('../utils/sessionCutoff'); const logger = require('../utils/logger'); const { getAdminTokenFromRequest, getGalleryTokenFromRequest } = require('../utils/tokenUtils'); @@ -38,6 +39,13 @@ async function adminAuth(req, res, next) { }); return res.status(401).json({ error: 'Token has been revoked', code: 'TOKEN_REVOKED' }); } + + // Reject any session issued before the global cutoff (set by a .picpeak + // restore, which can reassign admin ids). Forces every pre-restore admin + // session to re-authenticate against the restored data. + if (await isTokenBeforeCutoff(decoded)) { + return res.status(401).json({ error: 'Session invalidated', code: 'SESSION_INVALIDATED' }); + } // Verify token type if (decoded.type !== 'admin') { @@ -157,7 +165,12 @@ async function galleryAuth(req, res, next) { if (await isTokenRevoked(decoded)) { return res.status(401).json({ error: 'Session has been invalidated', code: 'TOKEN_REVOKED' }); } - + + // Reject sessions issued before the global restore cutoff. + if (await isTokenBeforeCutoff(decoded)) { + return res.status(401).json({ error: 'Session invalidated', code: 'SESSION_INVALIDATED' }); + } + // Verify token type if (decoded.type !== 'gallery') { return res.status(403).json({ error: 'Invalid access token' }); @@ -221,6 +234,11 @@ async function photoAuth(req, res, next) { return res.status(401).json({ error: 'Token has been revoked', code: 'TOKEN_REVOKED' }); } + // Reject sessions issued before the global restore cutoff. + if (await isTokenBeforeCutoff(decoded)) { + return res.status(401).json({ error: 'Session invalidated', code: 'SESSION_INVALIDATED' }); + } + // Allow both admin and gallery tokens if (decoded.type === 'admin') { const admin = await db('admin_users') diff --git a/backend/src/middleware/customerAuth.js b/backend/src/middleware/customerAuth.js index e1b9c545..1ce58c2e 100644 --- a/backend/src/middleware/customerAuth.js +++ b/backend/src/middleware/customerAuth.js @@ -13,6 +13,7 @@ const jwt = require('jsonwebtoken'); const { db } = require('../database/db'); const { formatBoolean } = require('../utils/dbCompat'); const { isTokenRevoked } = require('../utils/tokenRevocation'); +const { isTokenBeforeCutoff } = require('../utils/sessionCutoff'); const logger = require('../utils/logger'); const { getCustomerTokenFromRequest } = require('../utils/tokenUtils'); @@ -61,6 +62,11 @@ async function customerAuth(req, res, next) { return res.status(401).json({ error: 'Token has been revoked', code: 'TOKEN_REVOKED' }); } + // Reject sessions issued before the global restore cutoff. + if (await isTokenBeforeCutoff(decoded)) { + return res.status(401).json({ error: 'Session invalidated', code: 'SESSION_INVALIDATED' }); + } + if (decoded.type !== 'customer') { logger.warn('[customerAuth] wrong token type', { url: req.originalUrl, diff --git a/backend/src/routes/adminBackup.js b/backend/src/routes/adminBackup.js index 61705c91..41c35897 100644 --- a/backend/src/routes/adminBackup.js +++ b/backend/src/routes/adminBackup.js @@ -186,18 +186,15 @@ router.post('/picpeak/import', adminAuth, requirePermission('backup.restore'), p // letting a crafted .picpeak take over every admin account (GHSA-qxfx-4493-4v8f). const result = await importFromPicpeak({ picpeakPath, currentAdminId: req.admin && req.admin.id }); - // The restore rewrote admin_users, so ids may have shifted. The operator's - // current JWT is bound only to the pre-restore admin id (adminAuth trusts - // `decoded.id` — IP is logged, not enforced, and the backup controls - // password_changed_at), which could now resolve to a DIFFERENT restored - // account and silently grant its permissions. Force a fresh login instead - // of trusting the old session: revoke the token and clear the cookie. - // Clearing the cookie is the guarantee — it drops the operator's browser - // session unconditionally. Revocation is the extra layer that also kills a - // Bearer-header copy of the JWT; revokeToken() swallows DB errors and - // returns false, so check the result and log loudly if the denylist write - // didn't land (the operator should still re-login, which the cookie clear - // forces). + // The restore rewrote admin_users, so ids may have shifted. importFromPicpeak + // already stamped a GLOBAL session cutoff (see setSessionsValidAfter), so + // every JWT issued before the restore — admin, customer, gallery — now fails + // auth. Here we additionally give the importing admin an immediate, clean + // logout: revoke this token and clear the cookie so their browser drops the + // session at once rather than on the next 401. Cookie clear is the + // unconditional guarantee; revokeToken() swallows DB errors and returns + // false, so check the result and log loudly if the denylist write didn't + // land (the operator still re-logs-in, which the cookie clear forces). let tokenRevoked = false; try { if (req.token) { diff --git a/backend/src/services/picpeakImportService.js b/backend/src/services/picpeakImportService.js index 27ac33a0..8cd054a4 100644 --- a/backend/src/services/picpeakImportService.js +++ b/backend/src/services/picpeakImportService.js @@ -23,6 +23,7 @@ const { db } = require('../database/db'); const knexConfig = require('../../knexfile'); const { getStoragePath } = require('../config/storage'); const { hasColumnCached } = require('../utils/schemaCache'); +const { setSessionsValidAfter } = require('../utils/sessionCutoff'); const logger = require('../utils/logger'); const { PICPEAK_FORMAT_VERSION, EXCLUDED_TABLES, listDataTables } = require('./picpeakExportService'); @@ -105,7 +106,7 @@ function parseNdjson(filePath) { // preserved, its own FKs stay valid) to free the username; // - only when no row has the operator's email do we insert a fresh row. async function reinjectCurrentAdmin(trx, currentAdmin) { - if (!currentAdmin) return; + if (!currentAdmin) return null; const emailMatch = await trx('admin_users') .whereRaw('lower(email) = lower(?)', [currentAdmin.email]) @@ -135,6 +136,7 @@ async function reinjectCurrentAdmin(trx, currentAdmin) { if (field in currentAdmin) authUpdate[field] = currentAdmin[field]; } await trx('admin_users').where({ id: emailMatch.id }).update(authUpdate); + return emailMatch.id; } else { // The operator's email isn't in the backup, so nothing restored references // their id — a fresh row can't dangle a reference TO the operator. Null the @@ -149,6 +151,84 @@ async function reinjectCurrentAdmin(trx, currentAdmin) { const maxRow = await trx('admin_users').max({ m: 'id' }).first(); snapshot.id = (Number(maxRow && maxRow.m) || 0) + 1; await trx('admin_users').insert(snapshot); + return snapshot.id; + } +} + +// Capture the operator's role and its granted permission NAMES before the wipe, +// so preserveOperatorRole() can re-establish the operator's authorization after +// the RBAC tables are replaced. Permission NAMES (not ids) are captured because +// the restored permissions table reassigns ids. Returns null if the operator +// has no role. +async function captureOperatorRole(roleId) { + if (!roleId) return null; + const role = await db('roles').where({ id: roleId }).first(); + if (!role) return null; + const permissions = await db('role_permissions') + .join('permissions', 'permissions.id', 'role_permissions.permission_id') + .where('role_permissions.role_id', roleId) + .pluck('permissions.name'); + return { role, permissions }; +} + +// Restore the operator's authorization after roles/role_permissions are +// replaced. A restore rewrites the RBAC tables, so the operator's pre-restore +// role_id may now name a different (or missing) role — a crafted backup could +// silently downgrade them, and reinjectCurrentAdmin deliberately does NOT copy +// role_id (it could dangle). Here we resolve the role by NAME against the +// restored data: if a role with the operator's role name exists we trust it +// (it's the backup the operator chose to restore); otherwise we re-create the +// role from the captured snapshot and re-grant the captured permissions that +// still exist, so the operator can never be locked out of their own instance. +async function preserveOperatorRole(trx, operatorId, snapshot) { + if (!operatorId || !snapshot || !snapshot.role) return; + const { role, permissions } = snapshot; + + let target = await trx('roles').whereRaw('lower(name) = lower(?)', [role.name]).first(); + if (!target) { + const roleRow = { ...role }; + delete roleRow.id; + const maxRole = await trx('roles').max({ m: 'id' }).first(); + const newRoleId = (Number(maxRole && maxRole.m) || 0) + 1; // sequence resynced post-commit + roleRow.id = newRoleId; + await trx('roles').insert(roleRow); + if (permissions && permissions.length) { + const perms = await trx('permissions').whereIn('name', permissions).select('id'); + if (perms.length) { + await trx('role_permissions').insert( + perms.map((p) => ({ role_id: newRoleId, permission_id: p.id })) + ); + } + } + target = { id: newRoleId }; + } + await trx('admin_users').where({ id: operatorId }).update({ role_id: target.id }); +} + +// Fast-forward each restored table's Postgres identity sequence to its current +// max(id). batchInsert writes explicit ids without advancing the sequence, so +// the next natural insert into any restored table (a new event, an accepted +// invitation, etc.) would otherwise collide on the primary key. Runs AFTER the +// restore transaction commits (setval is non-transactional and would survive a +// rollback) and guards every table with a column-existence check — +// pg_get_serial_sequence RAISES on a table lacking an `id` column (e.g. the +// composite-key role_permissions), so an unguarded call would abort here. +// No-op on SQLite, whose AUTOINCREMENT tracks the high-water mark itself. +async function resyncSequences(tables) { + if (!isPostgres()) return; + for (const table of tables) { + try { + if (!(await db.schema.hasColumn(table, 'id'))) continue; + const res = await db.raw('SELECT pg_get_serial_sequence(?, ?) AS seq', [table, 'id']); + const seq = res && res.rows && res.rows[0] && res.rows[0].seq; + if (!seq) continue; // `id` isn't a serial/identity column + await db.raw( + 'SELECT setval(?, (SELECT COALESCE(MAX(id), 1) FROM ??), (SELECT MAX(id) IS NOT NULL FROM ??))', + [seq, table, table] + ); + } catch (err) { + logger.warn(`[picpeak-import] could not resync sequence for ${table}: ${err.message}`); + } } } @@ -188,7 +268,7 @@ function serialiseJsonColumns(rows, jsonCols) { // session_replication_role=replica on the trx connection, reset before commit; // sqlite: defer_foreign_keys so checks run at commit). knex_migrations is never // in the data set, so the target's schema/migration state is left intact. -async function replaceAllTables(tables, dataDir, currentAdmin) { +async function replaceAllTables(tables, dataDir, currentAdmin, roleSnapshot) { await db.transaction(async (trx) => { if (isPostgres()) { try { @@ -218,7 +298,10 @@ async function replaceAllTables(tables, dataDir, currentAdmin) { await trx.batchInsert(table, serialiseJsonColumns(rows, jsonCols), 100); } - await reinjectCurrentAdmin(trx, currentAdmin); + const operatorId = await reinjectCurrentAdmin(trx, currentAdmin); + if (operatorId && roleSnapshot) { + await preserveOperatorRole(trx, operatorId, roleSnapshot); + } // Reset the pg session flag BEFORE the connection returns to the pool. if (isPostgres()) await trx.raw("SET session_replication_role = 'origin'"); @@ -288,6 +371,9 @@ async function importFromPicpeak({ picpeakPath, currentAdminId }) { const currentAdmin = currentAdminId ? await db('admin_users').where({ id: currentAdminId }).first() : null; + // Capture the operator's role + granted permission names BEFORE the wipe so + // their authorization can be re-established after the RBAC tables are replaced. + const roleSnapshot = currentAdmin ? await captureOperatorRole(currentAdmin.role_id) : null; const staging = await fsp.mkdtemp(path.join(os.tmpdir(), 'picpeak-import-')); try { @@ -316,7 +402,16 @@ async function importFromPicpeak({ picpeakPath, currentAdminId }) { logger.warn(`[picpeak-import] ignoring ${skipped.length} backup table(s) not present in this DB (or protected): ${skipped.join(', ')}`); } - await replaceAllTables(tables, dataDir, currentAdmin); + await replaceAllTables(tables, dataDir, currentAdmin, roleSnapshot); + + // Post-commit fixups (must NOT run inside the restore transaction): + // - resync Postgres identity sequences left behind by the explicit-id + // batchInsert, so the next natural insert doesn't collide; + // - stamp a global session cutoff so every JWT issued before this restore + // (admin, customer, gallery) stops authenticating — ids may have shifted. + await resyncSequences(tables); + await setSessionsValidAfter(Math.floor(Date.now() / 1000)); + const filesRestored = await restoreFiles(staging); const usesExternalMedia = await detectExternalMedia(); @@ -334,4 +429,7 @@ module.exports = { readManifestFromZip, validateManifest, reinjectCurrentAdmin, + captureOperatorRole, + preserveOperatorRole, + resyncSequences, }; diff --git a/backend/src/utils/sessionCutoff.js b/backend/src/utils/sessionCutoff.js new file mode 100644 index 00000000..d325d179 --- /dev/null +++ b/backend/src/utils/sessionCutoff.js @@ -0,0 +1,100 @@ +/** + * Global session cutoff. + * + * A .picpeak restore rewrites admin_users / customer_accounts / events and can + * reassign their primary keys, so any JWT issued BEFORE the restore may now + * resolve to a different restored principal (auth middleware binds a token to + * `decoded.id`; IP is only logged and the backup controls each row's + * `password_changed_at`). Revoking the single importing token is not enough — + * every pre-restore admin, customer, and gallery session must stop being + * honoured. + * + * We record a single unix-second cutoff in app_settings and reject any token + * whose `iat` predates it, across all three JWT auth paths. The operator's + * forced re-login mints a token with `iat >= cutoff`, so it passes; everything + * issued earlier is refused. The value is cached briefly so the common auth + * path stays a single in-memory comparison. + */ +const { db } = require('../database/db'); +const logger = require('./logger'); + +const CUTOFF_KEY = 'security_sessions_valid_after'; +const CACHE_MS = 30 * 1000; // restores are rare; a short TTL keeps auth cheap + +let cache = null; // { value: number, expiry: number } + +async function readCutoffFromDb() { + const row = await db('app_settings') + .where('setting_key', CUTOFF_KEY) + .first() + .timeout(5000); + if (!row || row.setting_value == null) return 0; + let value = row.setting_value; + // pg `json` returns a parsed number; sqlite returns the stored string. + if (typeof value === 'string') { + try { value = JSON.parse(value); } catch (_) { /* fall through to parseInt */ } + } + const seconds = parseInt(value, 10); + return Number.isFinite(seconds) ? seconds : 0; +} + +/** + * Cutoff as unix seconds (0 = no cutoff set). Cached for CACHE_MS. On a + * transient DB error, returns the last known value (or 0) rather than blocking + * auth — the cutoff is defence-in-depth layered on top of per-token revocation. + */ +async function getSessionsValidAfter() { + const now = Date.now(); + if (cache && now < cache.expiry) return cache.value; + try { + const value = await readCutoffFromDb(); + cache = { value, expiry: now + CACHE_MS }; + return value; + } catch (err) { + logger.warn('[sessionCutoff] failed to read cutoff:', err.message); + return cache ? cache.value : 0; + } +} + +/** Persist a new cutoff (unix seconds) and refresh the in-process cache. */ +async function setSessionsValidAfter(unixSeconds) { + await db('app_settings') + .insert({ + setting_key: CUTOFF_KEY, + setting_value: JSON.stringify(unixSeconds), + setting_type: 'number', + updated_at: new Date(), + }) + .onConflict('setting_key') + .merge({ setting_value: JSON.stringify(unixSeconds), setting_type: 'number', updated_at: new Date() }); + cache = { value: unixSeconds, expiry: Date.now() + CACHE_MS }; +} + +/** + * True when this token was issued before the global cutoff. Fail-open on any + * error: the cutoff is defence-in-depth on top of per-token revocation and the + * post-restore cookie clear, and must never turn a transient read failure into + * an auth outage. + */ +async function isTokenBeforeCutoff(decoded) { + try { + if (!decoded || !decoded.iat) return false; + const cutoff = await getSessionsValidAfter(); + if (!cutoff) return false; + return decoded.iat < cutoff; + } catch (err) { + logger.warn('[sessionCutoff] check failed, allowing token:', err.message); + return false; + } +} + +/** Test-only: drop the in-process cache. */ +function _resetCache() { cache = null; } + +module.exports = { + CUTOFF_KEY, + getSessionsValidAfter, + setSessionsValidAfter, + isTokenBeforeCutoff, + _resetCache, +}; From b5ac24ea46290af728b3b187392eecc3ffb6a00a Mon Sep 17 00:00:00 2001 From: Paul Nothaft <53005142+the-luap@users.noreply.github.com> Date: Thu, 16 Jul 2026 13:43:50 +0200 Subject: [PATCH 10/10] chore(main): release 3.89.0-beta.0 (#814) --- .release-please-manifest-beta.json | 2 +- CHANGELOG.md | 18 ++++++++++++++++++ backend/package.json | 2 +- frontend/package.json | 2 +- 4 files changed, 21 insertions(+), 3 deletions(-) diff --git a/.release-please-manifest-beta.json b/.release-please-manifest-beta.json index e592a14b..3d5b4b3a 100644 --- a/.release-please-manifest-beta.json +++ b/.release-please-manifest-beta.json @@ -1,3 +1,3 @@ { - ".": "3.88.1-beta.0" + ".": "3.89.0-beta.0" } diff --git a/CHANGELOG.md b/CHANGELOG.md index 26a531a7..6dd163e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,24 @@ 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.89.0-beta.0](https://github.com/PicPeak/picpeak/compare/v3.88.1-beta.0...v3.89.0-beta.0) (2026-07-16) + + +### Features + +* **security:** harden .picpeak restore robustness — sessions, roles, sequences ([a77c2c2](https://github.com/PicPeak/picpeak/commit/a77c2c2c573a79f0194ff2b911acaa5f46c11f26)) +* **security:** harden .picpeak restore robustness — sessions, roles, sequences ([340d91b](https://github.com/PicPeak/picpeak/commit/340d91bdd53a595694edfa6f3d691b240a2babcd)) + + +### Bug Fixes + +* **security:** close 4 open security advisories (backup takeover, share-login bypass, ZIP slip, chunked-upload traversal) ([7ebc232](https://github.com/PicPeak/picpeak/commit/7ebc2326204ad0572e6a1fc121b5d232da06cec3)) +* **security:** harden .picpeak restore operator-preservation (GHSA-qxfx follow-up) ([38fd41a](https://github.com/PicPeak/picpeak/commit/38fd41aad3fcb12a249aaa2eb3d98fbffbde537a)) +* **security:** preserve current admin on .picpeak restore (GHSA-qxfx-4493-4v8f) ([348894e](https://github.com/PicPeak/picpeak/commit/348894efefa5a7b49d32feb22a98045b93076138)) +* **security:** reject ZIP-slip entries in archive/backup restore (GHSA-jfhw-fj23-fx6x) ([9cd6b08](https://github.com/PicPeak/picpeak/commit/9cd6b08441e8633751b9fb73daca5ca0555c950b)) +* **security:** sanitize chunked-upload filename (GHSA-pc72-jf53-w28j) ([31bc01c](https://github.com/PicPeak/picpeak/commit/31bc01cb4bbf65b48b3a5c3c94ad35e487df9fcc)) +* **security:** share-login must not bypass gallery password (GHSA-9hmx-68vc-qpqw) ([7dace04](https://github.com/PicPeak/picpeak/commit/7dace044dcc1c3b5a13c4704510c87616632618c)) + ## [3.88.1-beta.0](https://github.com/PicPeak/picpeak/compare/v3.88.0-beta.0...v3.88.1-beta.0) (2026-07-16) diff --git a/backend/package.json b/backend/package.json index 9bb5a0ee..ecfa3a5c 100644 --- a/backend/package.json +++ b/backend/package.json @@ -1,6 +1,6 @@ { "name": "picpeak-backend", - "version": "3.88.1-beta.0", + "version": "3.89.0-beta.0", "description": "Backend for PicPeak event photo sharing platform", "main": "server.js", "scripts": { diff --git a/frontend/package.json b/frontend/package.json index a7baec90..81348633 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,7 +1,7 @@ { "name": "picpeak-frontend", "private": true, - "version": "3.88.1-beta.0", + "version": "3.89.0-beta.0", "type": "module", "scripts": { "dev": "vite",