From 8b6cd3c74f2aeb5d38ebfeee04bbc211d6fa2c0c Mon Sep 17 00:00:00 2001 From: Paul Nothaft <53005142+the-luap@users.noreply.github.com> Date: Thu, 13 Aug 2026 18:51:12 +0200 Subject: [PATCH] fix(gallery): coerce SQLite 0/1 booleans in the guest surface (#1028) (#1037) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SQLite stores booleans as 0/1, Postgres as true/false. The guest gallery compared strictly against `true`/`false`, so every flag read backwards on SQLite installs: allow_downloads: 0 !== false → true (header Download button shown with downloads disabled) allow_user_uploads: 1 === true → false (upload button hidden with uploads enabled) Worse, all five download guards used `allow_downloads === false`, which never fires against a stored 0 — so on SQLite "Allow photo downloads = off" was inert end to end: single photo, download-all, download-selected, download-jobs and the job-status poll all kept serving, as did the secure-images download route. Per-category blocking (#640) was ignored for the same reason, the protection toggles (right-click, devtools, canvas, watermark) reported false while enabled, overlay_protection was stuck on, and show_feedback_to_guests leaked feedback with the setting off. The /info endpoint was already correct — it checks 0/'0' explicitly. The two payloads had simply drifted. Everything now goes through parseBooleanInput (utils/parsers.js), which normalises both engines and takes a per-column default so legacy NULL rows keep their documented behaviour. Tests run on the SQLite harness, so they assert the real engine values. Every one of them fails on the unfixed code — the payload assertions return the inverted value, and the guard assertions never get their 403 (the request proceeds to serve instead). Claude-Session: https://claude.ai/code/session_0168gubtwYYacJv8weAjy8DM Co-authored-by: Paul Nothaft --- .../routes/gallerySqliteBooleanFlags.test.js | 188 ++++++++++++++++++ backend/src/routes/gallery.js | 37 ++-- backend/src/routes/secureImages.js | 6 +- 3 files changed, 214 insertions(+), 17 deletions(-) create mode 100644 backend/__tests__/routes/gallerySqliteBooleanFlags.test.js diff --git a/backend/__tests__/routes/gallerySqliteBooleanFlags.test.js b/backend/__tests__/routes/gallerySqliteBooleanFlags.test.js new file mode 100644 index 00000000..7da9deec --- /dev/null +++ b/backend/__tests__/routes/gallerySqliteBooleanFlags.test.js @@ -0,0 +1,188 @@ +/** + * SQLite boolean coercion in the guest gallery surface (#1028). + * + * SQLite stores booleans as 0/1; Postgres stores true/false. The /photos + * payload and every download guard compared strictly against `true`/`false`, + * so on SQLite: + * + * allow_downloads: 0 !== false → true (button shown while disabled) + * allow_user_uploads: 1 === true → false (button hidden while enabled) + * if (allow_downloads === false) → never fires, so ALL download endpoints + * kept serving with downloads switched off + * + * (The download-jobs route asserted on main is #858, which is beta-only — + * this branch covers the three download endpoints that exist here.) + * + * The harness runs on SQLite, so these assertions exercise the real engine + * values rather than a mock. Every test here fails on the unfixed code. + */ + +const path = require('path'); +const fs = require('fs'); +const os = require('os'); + +process.env.NODE_ENV = 'test'; +process.env.TEST_DATABASE_PATH = path.join( + fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-sqlite-flags-')), 'db.sqlite', +); +process.env.JWT_SECRET = process.env.JWT_SECRET || 'sqlite-flags-test-secret'; +process.env.STORAGE_PATH = fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-sqlite-flags-storage-')); + +const request = require('supertest'); +const express = require('express'); +const cookieParser = require('cookie-parser'); +const { bootCrmDb, seedMinimal } = require('../integration/helpers/crmDb'); + +const SLUG = 'sqlite-flags-gallery'; + +describe('gallery flags survive SQLite 0/1 storage (#1028)', () => { + let db; let cleanup; let app; let eventId; let photoId; + + async function setEventFlags(patch) { + await db('events').where('id', eventId).update(patch); + } + + async function getPayload() { + const res = await request(app).get(`/api/gallery/${SLUG}/photos`); + expect(res.status).toBe(200); + return res.body.event; + } + + beforeAll(async () => { + ({ db, cleanup } = await bootCrmDb()); + await seedMinimal(db); + + const ev = await db('events').insert({ + slug: SLUG, + event_type: 'wedding', + event_name: 'SQLite Flags', + event_date: '2026-08-01', + host_email: 'h@example.com', + admin_email: 'a@example.com', + password_hash: 'x', + share_link: `/gallery/${SLUG}/s`, + share_token: 'sqlite-flags-share', + expires_at: new Date(Date.now() + 7 * 864e5).toISOString(), + is_active: 1, + is_archived: 0, + is_draft: 0, + // Password-free so verifyGalleryAccess takes the public path and loads + // the row with SELECT * — i.e. the raw 0/1 values, same as production. + require_password: 0, + created_at: new Date().toISOString(), + }).returning('id'); + eventId = ev[0]?.id ?? ev[0]; + + const ph = await db('photos').insert({ + event_id: eventId, + filename: 'p.jpg', + path: `${SLUG}/p.jpg`, + type: 'individual', + uploaded_at: new Date().toISOString(), + }).returning('id'); + photoId = ph[0]?.id ?? ph[0]; + + app = express(); + app.use(express.json()); + app.use(cookieParser()); + app.use('/api/gallery', require('../../src/routes/gallery')); + }, 120000); + + afterAll(async () => { if (cleanup) await cleanup(); }); + + test('the engine under test really is SQLite storing 0/1', async () => { + expect(['sqlite3', 'better-sqlite3']).toContain(db.client.config.client); + await setEventFlags({ allow_downloads: 0 }); + const row = await db('events').where('id', eventId).first('allow_downloads'); + expect(row.allow_downloads).toBe(0); + }); + + describe('with downloads disabled (allow_downloads = 0)', () => { + beforeAll(async () => { + await setEventFlags({ allow_downloads: 0, allow_user_uploads: 1 }); + }); + + test('payload reports allow_downloads false (was true — header button shown)', async () => { + expect((await getPayload()).allow_downloads).toBe(false); + }); + + test('payload reports allow_user_uploads true (was false — upload button hidden)', async () => { + expect((await getPayload()).allow_user_uploads).toBe(true); + }); + + test('single-photo download is refused', async () => { + const res = await request(app).get(`/api/gallery/${SLUG}/download/${photoId}`); + expect(res.status).toBe(403); + }); + + test('download-all is refused', async () => { + const res = await request(app).get(`/api/gallery/${SLUG}/download-all`); + expect(res.status).toBe(403); + }); + + test('download-selected is refused', async () => { + const res = await request(app) + .post(`/api/gallery/${SLUG}/download-selected`) + .send({ photo_ids: [photoId] }); + expect(res.status).toBe(403); + }); + }); + + describe('with downloads enabled (allow_downloads = 1)', () => { + beforeAll(async () => { + await setEventFlags({ allow_downloads: 1, allow_user_uploads: 0 }); + }); + + test('payload reports allow_downloads true / allow_user_uploads false', async () => { + const event = await getPayload(); + expect(event.allow_downloads).toBe(true); + expect(event.allow_user_uploads).toBe(false); + }); + + test('download-all is no longer refused', async () => { + const res = await request(app).get(`/api/gallery/${SLUG}/download-all`); + expect(res.status).not.toBe(403); + }); + }); + + describe('protection flags', () => { + test('0/1 protection toggles are reported the way they are stored', async () => { + await setEventFlags({ + disable_right_click: 1, + enable_devtools_protection: 1, + use_canvas_rendering: 1, + watermark_downloads: 1, + overlay_protection: 0, + }); + const event = await getPayload(); + expect(event.disable_right_click).toBe(true); + expect(event.enable_devtools_protection).toBe(true); + expect(event.use_canvas_rendering).toBe(true); + expect(event.watermark_downloads).toBe(true); + expect(event.overlay_protection).toBe(false); + }); + }); + + describe('per-category download blocking (#640) on SQLite', () => { + test('a category with allow_downloads = 0 is reported as blocked', async () => { + const cat = await db('photo_categories').insert({ + name: 'Blocked', slug: 'blocked', event_id: eventId, is_global: 0, allow_downloads: 0, + }).returning('id'); + const categoryId = cat[0]?.id ?? cat[0]; + await db('photos').where('id', photoId).update({ category_id: categoryId }); + + await setEventFlags({ allow_downloads: 1 }); + const res = await request(app).get(`/api/gallery/${SLUG}/photos`); + expect(res.status).toBe(200); + + const category = res.body.categories.find((c) => c.id === categoryId); + expect(category.allow_downloads).toBe(false); + const photo = res.body.photos.find((p) => p.id === photoId); + expect(photo.category_allow_downloads).toBe(false); + + // …and the per-category guard on the single-photo route fires. + const dl = await request(app).get(`/api/gallery/${SLUG}/download/${photoId}`); + expect(dl.status).toBe(403); + }); + }); +}); diff --git a/backend/src/routes/gallery.js b/backend/src/routes/gallery.js index 420808eb..25838962 100644 --- a/backend/src/routes/gallery.js +++ b/backend/src/routes/gallery.js @@ -2,6 +2,11 @@ const express = require('express'); const jwt = require('jsonwebtoken'); const { db } = require('../database/db'); const { formatBoolean } = require('../utils/dbCompat'); +// SQLite stores booleans as 0/1, Postgres as true/false (#1028). Strict +// comparisons against `true`/`false` therefore read every flag backwards on +// SQLite — parseBooleanInput normalises both engines and takes the per-column +// default for legacy NULL rows. +const { parseBooleanInput } = require('../utils/parsers'); const { getAppSetting } = require('../utils/appSettings'); const archiver = require('archiver'); const path = require('path'); @@ -534,7 +539,7 @@ router.get('/:slug/photos', verifyGalleryAccess, resolveGuest, async (req, res) // Check if feedback should be visible to guests const feedbackService = require('../services/feedbackService'); const feedbackSettings = await feedbackService.getEventFeedbackSettings(req.event.id); - const showFeedbackToGuests = isClient || feedbackSettings.show_feedback_to_guests !== false; + const showFeedbackToGuests = isClient || parseBooleanInput(feedbackSettings.show_feedback_to_guests, true); // Then get comment counts separately const commentCounts = await db('photo_feedback') @@ -599,7 +604,7 @@ router.get('/:slug/photos', verifyGalleryAccess, resolveGuest, async (req, res) // Per-category download flag (#640). false explicitly disables; the // gallery hides the download button. Defaults true so categories // created before migration 135 keep working. - allow_downloads: cat.allow_downloads !== false + allow_downloads: parseBooleanInput(cat.allow_downloads, true) })); } @@ -626,9 +631,9 @@ router.get('/:slug/photos', verifyGalleryAccess, resolveGuest, async (req, res) const protectionSettings = { protection_level: req.event.protection_level || 'standard', image_quality: req.event.image_quality || 85, - use_canvas_rendering: req.event.use_canvas_rendering === true, + use_canvas_rendering: parseBooleanInput(req.event.use_canvas_rendering, false), fragmentation_level: req.event.fragmentation_level || 3, - overlay_protection: req.event.overlay_protection !== false + overlay_protection: parseBooleanInput(req.event.overlay_protection, true) }; // Lightbox preview tier (#492). When the admin opts in, the @@ -675,13 +680,15 @@ router.get('/:slug/photos', verifyGalleryAccess, resolveGuest, async (req, res) color_theme: req.event.color_theme, expires_at: req.event.expires_at, hero_photo_id: req.event.hero_photo_id, - allow_downloads: req.event.allow_downloads !== false, - allow_user_uploads: req.event.allow_user_uploads === true, - disable_right_click: req.event.disable_right_click === true, - watermark_downloads: req.event.watermark_downloads === true, + // Defaults match /info: downloads on unless explicitly disabled, + // uploads off unless explicitly enabled (#1028). + allow_downloads: parseBooleanInput(req.event.allow_downloads, true), + allow_user_uploads: parseBooleanInput(req.event.allow_user_uploads, false), + disable_right_click: parseBooleanInput(req.event.disable_right_click, false), + watermark_downloads: parseBooleanInput(req.event.watermark_downloads, false), watermark_text: req.event.watermark_text, - enable_devtools_protection: req.event.enable_devtools_protection === true, - use_canvas_rendering: req.event.use_canvas_rendering === true, + enable_devtools_protection: parseBooleanInput(req.event.enable_devtools_protection, false), + use_canvas_rendering: parseBooleanInput(req.event.use_canvas_rendering, false), hero_logo_visible: resolveHeroLogoVisible(req.event.hero_logo_visible, globalHeroLogoVisible), hero_logo_size: req.event.hero_logo_size || globalLogoSize || 'medium', hero_logo_position: req.event.hero_logo_position || 'top', @@ -745,7 +752,7 @@ router.get('/:slug/photos', verifyGalleryAccess, resolveGuest, async (req, res) // Per-category download permission (#640). Defaults true for photos // without a category or for categories that pre-date migration 135. category_allow_downloads: photo.category_id && categoryMap[photo.category_id] - ? categoryMap[photo.category_id].allow_downloads !== false + ? parseBooleanInput(categoryMap[photo.category_id].allow_downloads, true) : true, category_slug: photo.category_id && categoryMap[photo.category_id] ? categoryMap[photo.category_id].slug : null, size: photo.size_bytes, @@ -855,7 +862,7 @@ router.get('/:slug/download/:photoId', verifyGalleryAccess, denySlideshowToken, const { photoId } = req.params; // Check if downloads are allowed for this event - if (req.event.allow_downloads === false) { + if (!parseBooleanInput(req.event.allow_downloads, true)) { return res.status(403).json({ error: 'Downloads are disabled for this gallery' }); } @@ -879,7 +886,7 @@ router.get('/:slug/download/:photoId', verifyGalleryAccess, denySlideshowToken, const cat = await db('photo_categories') .where('id', photo.category_id) .first('allow_downloads'); - if (cat && cat.allow_downloads === false) { + if (cat && !parseBooleanInput(cat.allow_downloads, true)) { return res.status(403).json({ error: 'Downloads are disabled for this category' }); } } @@ -983,7 +990,7 @@ async function bumpEventDownloadCounts(eventId) { router.get('/:slug/download-all', verifyGalleryAccess, denySlideshowToken, async (req, res) => { try { // Check if downloads are allowed for this event - if (req.event.allow_downloads === false) { + if (!parseBooleanInput(req.event.allow_downloads, true)) { return res.status(403).json({ error: 'Downloads are disabled for this gallery' }); } @@ -1203,7 +1210,7 @@ router.get('/:slug/download-all', verifyGalleryAccess, denySlideshowToken, async router.post('/:slug/download-selected', verifyGalleryAccess, denySlideshowToken, async (req, res) => { try { // Check if downloads are allowed for this event - if (req.event.allow_downloads === false) { + if (!parseBooleanInput(req.event.allow_downloads, true)) { return res.status(403).json({ error: 'Downloads are disabled for this gallery' }); } diff --git a/backend/src/routes/secureImages.js b/backend/src/routes/secureImages.js index a8668ab2..ac660d10 100644 --- a/backend/src/routes/secureImages.js +++ b/backend/src/routes/secureImages.js @@ -5,6 +5,7 @@ const secureImageService = require('../services/secureImageService'); const secureImageMiddleware = require('../middleware/secureImageMiddleware'); const logger = require('../utils/logger'); const { formatBoolean } = require('../utils/dbCompat'); +const { parseBooleanInput } = require('../utils/parsers'); const { resolvePhotoFilePath, resolvePhotoStorageKey } = require('../services/photoResolver'); const { withLocalCopy } = require('../services/imageProcessor'); const { getStorage } = require('../services/storage'); @@ -323,8 +324,9 @@ router.get('/:slug/secure-download/:photoId/:token', try { const { photoId, token } = req.params; - // Check if downloads are allowed - if (req.event.allow_downloads === false) { + // Check if downloads are allowed. SQLite stores the flag as 0/1, so a + // strict `=== false` never fired there and the guard was inert (#1028). + if (!parseBooleanInput(req.event.allow_downloads, true)) { return res.status(403).json({ error: 'Downloads are disabled for this gallery' }); }