fix(security): one settings decoder, and the last creation path

Round-four review follow-ups.

Every reader of app_settings now shares decodeSettingValue. The previous
commit taught the GET handler to decode, which on a legacy SQLite install
made the tab show devtools protection as disabled while
readBooleanSetting — parsing once, getting the string 'false', rejecting
it — left new galleries with it enabled. A decoder used by only some
readers is worse than none, because the UI and the behaviour disagree.
readBooleanSetting, getImageSecurityDefaults, the v1 devtools fallback
and the settings GET all use it now.

Standalone contract conversion covered. contract/conversions.js takes
Path B and inserts its own event row when the contract has no source
quote, so signed standalone contracts were the last path still landing
on the migration-038 column defaults.

Refs #1296

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({});

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;
+    const value = decodeSettingValue(setting.setting_value);
-    if (typeof value === 'string') {
-      try { value = JSON.parse(value); } catch { /* keep raw */ }
-    }
     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;
+      return decodeSettingValue(row.setting_value);
-      while (typeof value === 'string') {
-        let parsed;
-        try { parsed = JSON.parse(value); } catch { break; }
-        if (parsed === value) break;
-        value = parsed;
-      }
-      return value;
     };
 
     const level = read('default_protection_level');
@@ -684,6 +700,7 @@ module.exports = {
   getStoragePath,
   getEventFieldRequirements,
   readBooleanSetting,
+  decodeSettingValue,
   getDownloadProtectionDefaults,
   getImageSecurityDefaults,
   resolveImageSecurityColumns,

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;
+      config[setting.setting_key] = decodeSettingValue(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;
     });
 
     res.json(config);

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 {
+          // Shared decoder: a legacy row can carry several layers of JSON
-            const parsed = JSON.parse(setting.setting_value);
+          // quoting, and a single parse would leave the string 'false' here,
-            if (typeof parsed === 'boolean') devtoolsFallback = parsed;
+          // reject it, and quietly enable protection the operator disabled.
-          } catch { /* keep true */ }
+          const parsed = decodeSettingValue(setting.setting_value);
+          if (typeof parsed === 'boolean') devtoolsFallback = parsed;
         }
       }
       const enable_devtools_protection = parseBooleanInput(devtoolsInput, devtoolsFallback);

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)) {