diff --git a/backend/__tests__/integration/imageSecurityDefaults.test.js b/backend/__tests__/integration/imageSecurityDefaults.test.js index 74ca0c11..b2998705 100644 --- a/backend/__tests__/integration/imageSecurityDefaults.test.js +++ b/backend/__tests__/integration/imageSecurityDefaults.test.js @@ -24,10 +24,11 @@ describe('image-security creation defaults', () => { let cleanup; let getImageSecurityDefaults; let resolveImageSecurityColumns; + let readBooleanSetting; beforeAll(async () => { ({ db, cleanup } = await bootCrmDb()); - ({ getImageSecurityDefaults, resolveImageSecurityColumns } = + ({ getImageSecurityDefaults, resolveImageSecurityColumns, readBooleanSetting } = require('../../src/routes/adminEvents/helpers')); }, 120000); @@ -149,6 +150,31 @@ describe('image-security creation defaults', () => { }); }); + describe('readBooleanSetting shares the same decoder', () => { + // Every reader of app_settings has to agree, or the settings tab shows + // protection disabled while newly created galleries turn it on. + const setRaw2 = async (key, raw) => { + await db('app_settings') + .insert({ setting_key: key, setting_value: raw, setting_type: 'security' }) + .onConflict('setting_key').merge(); + }; + + it('reads a double-encoded false as false, not as absent', async () => { + await setRaw2('enable_devtools_protection', JSON.stringify(JSON.stringify(false))); + expect(await readBooleanSetting('enable_devtools_protection')).toBe(false); + }); + + it('still reads a singly-encoded value', async () => { + await setRaw2('enable_devtools_protection', JSON.stringify(true)); + expect(await readBooleanSetting('enable_devtools_protection')).toBe(true); + }); + + it('returns undefined for a non-boolean, so the caller keeps its default', async () => { + await setRaw2('enable_devtools_protection', JSON.stringify('sometimes')); + expect(await readBooleanSetting('enable_devtools_protection')).toBeUndefined(); + }); + }); + describe('resolveImageSecurityColumns', () => { it('omits every column when neither the request nor the settings supply one', () => { expect(resolveImageSecurityColumns({}, {})).toEqual({}); diff --git a/backend/src/routes/adminEvents/helpers.js b/backend/src/routes/adminEvents/helpers.js index f6fef319..82e1af16 100644 --- a/backend/src/routes/adminEvents/helpers.js +++ b/backend/src/routes/adminEvents/helpers.js @@ -73,14 +73,37 @@ const getEventFieldRequirements = async () => { // Helper to read app_settings booleans by key, used to inherit per-setting // defaults onto new events. Returns `undefined` for missing/non-boolean rows // so callers can fall back to a legacy default. +/** + * Decode an app_settings value into the JS value it represents. + * + * setting_value is JSON text on SQLite and may already be decoded by the + * driver on a PG json column, so one parse does not normalise both. On top + * of that, the Image Security tab used to PUT back values it had read + * undecoded, wrapping another layer of quoting around each one on every + * save — the GET handler decodes now, but installs carry however many + * layers they accumulated before that. + * + * Every reader of app_settings has to agree about this, or the admin UI + * shows one thing while event creation does another. + * + * Terminates: each parse of a string is strictly shorter than its input. + */ +const decodeSettingValue = (raw) => { + let value = raw; + while (typeof value === 'string') { + let parsed; + try { parsed = JSON.parse(value); } catch { break; } + if (parsed === value) break; + value = parsed; + } + return value; +}; + const readBooleanSetting = async (key) => { try { const setting = await db('app_settings').where('setting_key', key).first(); if (!setting) return undefined; - let value = setting.setting_value; - if (typeof value === 'string') { - try { value = JSON.parse(value); } catch { /* keep raw */ } - } + const value = decodeSettingValue(setting.setting_value); return typeof value === 'boolean' ? value : undefined; } catch (error) { logger.error('Failed to read app setting', { key, error: error.message }); @@ -170,14 +193,7 @@ const getImageSecurityDefaults = async (trx = null) => { const read = (key) => { const row = rows.find((r) => r.setting_key === key); if (!row) return undefined; - let value = row.setting_value; - while (typeof value === 'string') { - let parsed; - try { parsed = JSON.parse(value); } catch { break; } - if (parsed === value) break; - value = parsed; - } - return value; + return decodeSettingValue(row.setting_value); }; const level = read('default_protection_level'); @@ -684,6 +700,7 @@ module.exports = { getStoragePath, getEventFieldRequirements, readBooleanSetting, + decodeSettingValue, getDownloadProtectionDefaults, getImageSecurityDefaults, resolveImageSecurityColumns, diff --git a/backend/src/routes/adminImageSecurity.js b/backend/src/routes/adminImageSecurity.js index e83df3b3..92a51a09 100644 --- a/backend/src/routes/adminImageSecurity.js +++ b/backend/src/routes/adminImageSecurity.js @@ -4,6 +4,7 @@ const { adminAuth } = require('../middleware/auth'); const { requirePermission } = require('../middleware/permissions'); const secureImageMiddleware = require('../middleware/secureImageMiddleware'); const logger = require('../utils/logger'); +const { decodeSettingValue } = require('./adminEvents/helpers'); const router = express.Router(); @@ -40,14 +41,7 @@ router.get('/settings', adminAuth, requirePermission(['settings.view', 'image_se // edited, until consumers could no longer read them (#1296). Decode // here so a round trip is idempotent. This terminates: each parse of // a string is strictly shorter than its input. - let value = setting.setting_value; - while (typeof value === 'string') { - let parsed; - try { parsed = JSON.parse(value); } catch { break; } - if (parsed === value) break; - value = parsed; - } - config[setting.setting_key] = value; + config[setting.setting_key] = decodeSettingValue(setting.setting_value); }); res.json(config); diff --git a/backend/src/routes/v1/events.js b/backend/src/routes/v1/events.js index 57be3edd..121c22ea 100644 --- a/backend/src/routes/v1/events.js +++ b/backend/src/routes/v1/events.js @@ -37,7 +37,7 @@ const logger = require('../../utils/logger'); const { slugify } = require('../../utils/slug'); const { formatBoolean } = require('../../utils/dbCompat'); const { parseBooleanInput } = require('../../utils/parsers'); -const { getImageSecurityDefaults, resolveImageSecurityColumns } = require('../adminEvents/helpers'); +const { getImageSecurityDefaults, resolveImageSecurityColumns, decodeSettingValue } = require('../adminEvents/helpers'); const { isValidEventType } = require('../../services/eventTypeService'); const { replacePhoto } = require('../../services/photoReplacementService'); const { getMaxFileSizeBytes, DEFAULT_MAX_FILE_SIZE_MB } = require('../../services/uploadSettings'); @@ -236,10 +236,11 @@ router.post( if (devtoolsInput === undefined) { const setting = await db('app_settings').where('setting_key', 'enable_devtools_protection').first(); if (setting) { - try { - const parsed = JSON.parse(setting.setting_value); - if (typeof parsed === 'boolean') devtoolsFallback = parsed; - } catch { /* keep true */ } + // Shared decoder: a legacy row can carry several layers of JSON + // quoting, and a single parse would leave the string 'false' here, + // reject it, and quietly enable protection the operator disabled. + const parsed = decodeSettingValue(setting.setting_value); + if (typeof parsed === 'boolean') devtoolsFallback = parsed; } } const enable_devtools_protection = parseBooleanInput(devtoolsInput, devtoolsFallback); diff --git a/backend/src/services/contract/conversions.js b/backend/src/services/contract/conversions.js index 2b3548cb..0450c992 100644 --- a/backend/src/services/contract/conversions.js +++ b/backend/src/services/contract/conversions.js @@ -229,6 +229,8 @@ async function convertToEvent(contractId, adminId) { || (await resolveDefaultEventType()); const eventCols = await db('events').columnInfo(); + const { getImageSecurityDefaults, resolveImageSecurityColumns } = require('../../routes/adminEvents/helpers'); + const imageSecurityColumns = resolveImageSecurityColumns({}, await getImageSecurityDefaults()); const candidate = { slug: `contract-${contract.contract_number.toLowerCase()}-${crypto.randomBytes(3).toString('hex')}`, // Prefer the contract's event_name snapshot (set on the contract @@ -255,6 +257,11 @@ async function convertToEvent(contractId, adminId) { quote_id: null, created_at: new Date(), updated_at: new Date(), + // #1296 — a signed standalone contract converts straight to a gallery + // here, without going through quoteService, so the global Image Security + // defaults have to be applied on this path too. Not inside a transaction, + // so the global db read is fine. + ...imageSecurityColumns, }; const eventRow = {}; for (const [k, v] of Object.entries(candidate)) {