fix(security): decode settings at the API boundary and honour the transaction

Round-three review follow-ups.

getImageSecurityDefaults now accepts a transaction, the way getAppSetting
two lines above it already does. quoteService.convertToEvent called it
from inside db.transaction() through the global db; sqlite3 runs a
single-connection pool, so that read would have waited on the connection
its own transaction was holding until the acquire timeout, and the
helper's catch would then have swallowed the error and dropped the
defaults silently.

The double-encoding is fixed where it starts. GET
/admin/image-security/settings returned setting_value undecoded, so it
shipped "true" to a tab that types the field as boolean — and since the
tab PUTs the whole object back through JSON.stringify, every save
wrapped another layer around values nobody edited. It decodes now, so a
round trip is idempotent. The tab is the only consumer of that endpoint.

The reader unwraps to any depth instead of four. The depth on an
existing install is however many times someone opened that tab, which is
not a number to cap. It terminates because each parse of a string is
strictly shorter than its input.

Refs #1296
This commit is contained in:
Paul Nothaft
2026-09-05 07:37:19 +02:00
parent 19c518aaa5
commit 0e560ebb19
4 changed files with 40 additions and 8 deletions
+16 -3
View File
@@ -32,9 +32,22 @@ router.get('/settings', adminAuth, requirePermission(['settings.view', 'image_se
const config = {};
settings.forEach(setting => {
// PostgreSQL JSON columns are already parsed by the driver
// Just use the value directly - no need to JSON.parse
config[setting.setting_key] = setting.setting_value;
// setting_value is JSON text on SQLite, and already decoded by the
// driver on a PG json column — so returning it raw shipped strings
// like "true" to a tab that types the field as boolean. Worse, the
// tab PUTs this whole object straight back through JSON.stringify,
// so every save wrapped another layer of quoting around values nobody
// 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;
});
res.json(config);