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 <paul@MacStudio-von-Paul.local>
This commit is contained in:
@@ -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);
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -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' });
|
||||
}
|
||||
|
||||
|
||||
@@ -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' });
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user