fix(security): close the remaining image-security default gaps

Round-two review follow-ups.

Settings survive the tab round trip. GET returns setting_value without
decoding it and ImageSecurityTab PUTs the whole fetched object back
through JSON.stringify, so on SQLite one visit to the tab re-encodes
every value it read. A single parse then yields the string "true", the
type checks reject it, and the defaults go quietly dead — the exact bug
this change exists to fix, returning by a different route. The reader
now unwraps until the value stops being a JSON string, bounded.

Array overrides rejected. express-validator applies isInt/isIn/isBoolean
element-wise, so `image_quality: [72]` passed the chain and arrived
still an array — a PG insert error, and `[false]` coerced to true by
formatBoolean. Both create routes now use .not().isArray(), and the
shared resolver ignores non-scalars for any future caller.

Two more creation paths covered. quoteService.convertToEvent builds its
own events row, so CRM-converted galleries fell back to column defaults.
/:id/duplicate copies fifteen source columns including
enable_devtools_protection but missed these four, so duplicating a
'maximum' gallery produced a 'standard' one — a duplicate now inherits
the source's values, not the current globals, since copying the gallery
is the point.

The PUT /:id chain has the same array weakness. Pre-existing and outside
this fix; left alone deliberately.

Refs #1296

backend/__tests__/integration/imageSecurityDefaults.test.js

@@ -107,6 +107,39 @@ describe('image-security creation defaults', () => {
     await expect(getImageSecurityDefaults()).resolves.toEqual({});
   });
 
+  describe('double-encoded settings (the settings tab round trip)', () => {
+    // GET returns setting_value undecoded and the tab PUTs the whole object
+    // back through JSON.stringify, so on SQLite one visit to the tab turns
+    // every value it read into a doubly-encoded string. A single parse left
+    // a string behind, the type checks rejected it, and the defaults went
+    // silently dead again.
+    const setRaw = 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 boolean', async () => {
+      await setRaw('enable_canvas_rendering', JSON.stringify(JSON.stringify(true)));
+      expect(await getImageSecurityDefaults()).toEqual({ use_canvas_rendering: true });
+    });
+
+    it('reads a double-encoded protection level', async () => {
+      await setRaw('default_protection_level', JSON.stringify(JSON.stringify('enhanced')));
+      expect(await getImageSecurityDefaults()).toEqual({ protection_level: 'enhanced' });
+    });
+
+    it('reads a double-encoded integer', async () => {
+      await setRaw('default_image_quality', JSON.stringify(JSON.stringify(72)));
+      expect(await getImageSecurityDefaults()).toEqual({ image_quality: 72 });
+    });
+
+    it('still rejects a malformed value however many times it was encoded', async () => {
+      await setRaw('default_image_quality', JSON.stringify(JSON.stringify('72oops')));
+      expect(await getImageSecurityDefaults()).toEqual({});
+    });
+  });
+
   describe('resolveImageSecurityColumns', () => {
     it('omits every column when neither the request nor the settings supply one', () => {
       expect(resolveImageSecurityColumns({}, {})).toEqual({});

backend/src/routes/adminEvents/crud.js

@@ -232,10 +232,10 @@ module.exports = (router) => {
     // Image security. PUT /:id has validated these all along; create
     // accepted none of them, so a value sent here used to be dropped on the
     // floor and the column default applied instead (#1296).
-    body('protection_level').optional().isIn(['basic', 'standard', 'enhanced', 'maximum']),
-    body('use_canvas_rendering').optional().isBoolean().toBoolean(),
-    body('image_quality').optional().isInt({ min: 1, max: 100 }).toInt(),
-    body('fragmentation_level').optional().isInt({ min: 1, max: 10 }).toInt(),
+    body('protection_level').optional().not().isArray().isIn(['basic', 'standard', 'enhanced', 'maximum']),
+    body('use_canvas_rendering').optional().not().isArray().isBoolean().toBoolean(),
+    body('image_quality').optional().not().isArray().isInt({ min: 1, max: 100 }).toInt(),
+    body('fragmentation_level').optional().not().isArray().isInt({ min: 1, max: 10 }).toInt(),
     body('watermark_downloads').optional().isBoolean(),
     body('watermark_text').optional().trim(),
     // #328 follow-up: per-event opt-in for presigned-URL "Download All".
@@ -1419,6 +1419,15 @@ module.exports = (router) => {
         allow_downloads: source.allow_downloads,
         disable_right_click: source.disable_right_click,
         enable_devtools_protection: source.enable_devtools_protection,
+        // A duplicate inherits the source's protection settings, NOT the
+        // current global defaults — copying the gallery is the whole point.
+        // These four sat next to enable_devtools_protection and were simply
+        // missed, so duplicating a 'maximum' event produced a 'standard' one
+        // (#1296).
+        protection_level: source.protection_level,
+        image_quality: source.image_quality,
+        use_canvas_rendering: source.use_canvas_rendering,
+        fragmentation_level: source.fragmentation_level,
         watermark_downloads: source.watermark_downloads,
         watermark_text: source.watermark_text,
         allow_presigned_download: source.allow_presigned_download,

backend/src/routes/adminEvents/helpers.js

@@ -147,12 +147,25 @@ const getImageSecurityDefaults = async () => {
       ])
       .select('setting_key', 'setting_value');
 
+    // app_settings holds JSON text on SQLite, while a PG json column comes
+    // back already decoded — so one parse is not enough to normalise both.
+    // Worse, GET /api/admin/image-security/settings returns setting_value
+    // without decoding it and the settings tab PUTs the whole fetched object
+    // straight back through JSON.stringify, so opening the tab and saving
+    // re-encodes every value it read as text. After one such round trip
+    // `true` is stored as "\"true\"" and a single parse yields the string
+    // 'true', which the type checks below reject — the settings would go
+    // quietly dead again, which is the bug this whole change exists to fix.
+    // Unwrap until it stops being a JSON string, bounded so nothing spins.
     const read = (key) => {
       const row = rows.find((r) => r.setting_key === key);
       if (!row) return undefined;
       let value = row.setting_value;
-      if (typeof value === 'string') {
-        try { value = JSON.parse(value); } catch { /* keep raw */ }
+      for (let i = 0; i < 4 && typeof value === 'string'; i += 1) {
+        let parsed;
+        try { parsed = JSON.parse(value); } catch { break; }
+        if (parsed === value) break;
+        value = parsed;
       }
       return value;
     };
@@ -203,7 +216,16 @@ const getImageSecurityDefaults = async () => {
 const resolveImageSecurityColumns = (body = {}, defaults = {}) => {
   const { formatBoolean } = require('../../utils/dbCompat');
   const columns = {};
-  const pick = (key) => (body[key] !== undefined ? body[key] : defaults[key]);
+  // express-validator runs isInt/isIn/isBoolean element-wise on arrays, so a
+  // single-element array like `image_quality: [72]` passes the route's chain
+  // and arrives here still an array. The routes reject those with
+  // .not().isArray(); this guard means any future caller cannot write one
+  // into a scalar column (a PG insert error, or `[false]` coerced to true).
+  const scalar = (v) => (v !== null && typeof v === 'object' ? undefined : v);
+  const pick = (key) => {
+    const fromBody = scalar(body[key]);
+    return fromBody !== undefined ? fromBody : defaults[key];
+  };
 
   const level = pick('protection_level');
   if (level !== undefined) columns.protection_level = level;

backend/src/routes/v1/events.js

@@ -184,10 +184,10 @@ router.post(
     body('color_theme').optional({ nullable: true }).isString().trim(),
     body('feedback_enabled').optional().isBoolean(),
     body('enable_devtools_protection').optional().isBoolean(),
-    body('protection_level').optional().isIn(['basic', 'standard', 'enhanced', 'maximum']),
-    body('use_canvas_rendering').optional().isBoolean().toBoolean(),
-    body('image_quality').optional().isInt({ min: 1, max: 100 }).toInt(),
-    body('fragmentation_level').optional().isInt({ min: 1, max: 10 }).toInt(),
+    body('protection_level').optional().not().isArray().isIn(['basic', 'standard', 'enhanced', 'maximum']),
+    body('use_canvas_rendering').optional().not().isArray().isBoolean().toBoolean(),
+    body('image_quality').optional().not().isArray().isInt({ min: 1, max: 100 }).toInt(),
+    body('fragmentation_level').optional().not().isArray().isInt({ min: 1, max: 10 }).toInt(),
     body('hero_logo_visible').optional().isBoolean(),
     body('hero_logo_size').optional().isIn(['small', 'medium', 'large', 'xlarge']),
     body('hero_logo_position').optional().isIn(['top', 'center', 'bottom'])

backend/src/services/quoteService.js

@@ -1683,6 +1683,8 @@ async function convertToEvent(quoteId, adminId, options = {}) {
     // ask the DB which columns exist and only keep the matching pairs
     // — bullet-proof against schema drift in either direction.
     const eventCols = await trx('events').columnInfo();
+    const { getImageSecurityDefaults, resolveImageSecurityColumns } = require('../routes/adminEvents/helpers');
+    const imageSecurityColumns = resolveImageSecurityColumns({}, await getImageSecurityDefaults());
     const candidate = {
       slug: `quote-${quote.quote_number.toLowerCase()}-${crypto.randomBytes(3).toString('hex')}`,
       event_name: quote.event_name || `Event ${quote.quote_number}`,
@@ -1705,6 +1707,11 @@ async function convertToEvent(quoteId, adminId, options = {}) {
       quote_id: quote.id,
       created_at: new Date(),
       updated_at: new Date(),
+      // #1296 — a converted quote produces a real gallery, so the global
+      // Image Security defaults have to reach it too. Required lazily: this
+      // is a service reaching into a route helper, and the lazy form keeps
+      // the module graph acyclic the way the storage require below does.
+      ...imageSecurityColumns,
     };
     const eventRow = {};
     for (const [k, v] of Object.entries(candidate)) {