diff --git a/.release-please-manifest-beta.json b/.release-please-manifest-beta.json index ec87b005..3d5b4b3a 100644 --- a/.release-please-manifest-beta.json +++ b/.release-please-manifest-beta.json @@ -1,3 +1,3 @@ { - ".": "3.88.0-beta.0" + ".": "3.89.0-beta.0" } diff --git a/CHANGELOG.md b/CHANGELOG.md index d35e569e..6dd163e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,32 @@ 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) + + +### 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/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/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/__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__/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/__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/__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/__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/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/__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 1febdaa4..39a2dd45 100644 --- a/backend/package.json +++ b/backend/package.json @@ -1,6 +1,6 @@ { "name": "picpeak-backend", - "version": "3.88.0-beta.0", + "version": "3.89.0-beta.0", "description": "Backend for PicPeak event photo sharing platform", "main": "server.js", "scripts": { @@ -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/middleware/maintenance.js b/backend/src/middleware/maintenance.js index 5f992db8..0a328b5f 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', // SSO variants of the admin login (#798) — same reasoning: an SSO-only // (JIT-provisioned) admin has no password, so blocking these would make // maintenance mode admin-proof for them. 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/routes/adminBackup.js b/backend/src/routes/adminBackup.js index 67d1479c..46673bc3 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'); @@ -29,7 +31,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 +73,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({ @@ -178,12 +191,40 @@ 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 }); + + // 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) { + 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/routes/adminSettings.js b/backend/src/routes/adminSettings.js index eac0d152..a46bc64d 100644 --- a/backend/src/routes/adminSettings.js +++ b/backend/src/routes/adminSettings.js @@ -171,6 +171,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. @@ -547,6 +557,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. diff --git a/backend/src/routes/auth.js b/backend/src/routes/auth.js index 31e9c9c6..ba3e1b56 100644 --- a/backend/src/routes/auth.js +++ b/backend/src/routes/auth.js @@ -560,6 +560,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, @@ -574,8 +586,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: { 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 diff --git a/backend/src/services/picpeakImportService.js b/backend/src/services/picpeakImportService.js index b1a6b0f3..8cd054a4 100644 --- a/backend/src/services/picpeakImportService.js +++ b/backend/src/services/picpeakImportService.js @@ -18,10 +18,12 @@ 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'); const { hasColumnCached } = require('../utils/schemaCache'); +const { setSessionsValidAfter } = require('../utils/sessionCutoff'); const logger = require('../utils/logger'); const { PICPEAK_FORMAT_VERSION, EXCLUDED_TABLES, listDataTables } = require('./picpeakExportService'); @@ -80,25 +82,164 @@ 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, - }); + if (!currentAdmin) return null; + + 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); + return emailMatch.id; } 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); + 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}`); + } + } +} + +// 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 @@ -127,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 { @@ -157,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'"); @@ -227,11 +371,18 @@ 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 { 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(); @@ -251,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(); @@ -268,4 +428,8 @@ module.exports = { importFromPicpeak, readManifestFromZip, validateManifest, + reinjectCurrentAdmin, + captureOperatorRole, + preserveOperatorRole, + resyncSequences, }; 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, }; 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, +}; 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 diff --git a/frontend/package.json b/frontend/package.json index 3dee20de..81348633 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.89.0-beta.0", "type": "module", "scripts": { "dev": "vite", 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);