From 38c316369bc7822b7ef767bec706f0a40267abcd Mon Sep 17 00:00:00 2001 From: Sebastion Date: Tue, 21 Jul 2026 17:53:07 +0100 Subject: [PATCH 1/2] fix(api/jobs): restrict privileged notification adapter fields to admins Any authenticated user could previously set notificationAdapter[].fields on POST /api/jobs. For the sqlite adapter this meant the dbPath was user-controlled, letting a non-admin cause fs.mkdirSync + a sqlite file open at any process-writable location (CWE-284 / CWE-269, leading to CWE-73). Rework per maintainer feedback (PR #371): move the check from the adapter to the authorization layer so existing installs with absolute dbPath configured by an admin keep working unchanged. Non-admin writes to privileged adapter fields (currently sqlite.dbPath) are replaced with the stored value, or dropped when none exists. Admins are unaffected. --- lib/api/routes/jobRouter.js | 49 ++++++++++++++++++++++++++++++++++++- 1 file changed, 48 insertions(+), 1 deletion(-) diff --git a/lib/api/routes/jobRouter.js b/lib/api/routes/jobRouter.js index 280f6130..de0cdce1 100644 --- a/lib/api/routes/jobRouter.js +++ b/lib/api/routes/jobRouter.js @@ -14,6 +14,51 @@ import { getSettings } from '../../services/storage/settingsStorage.js'; const DEMO_JOB_NAME = 'Demo-Job'; +/** + * Notification adapter fields that touch the filesystem or other sensitive + * process-level resources. Non-admin users must not be able to introduce or + * modify these fields on a job, otherwise any authenticated user could make + * the server open/create files at arbitrary locations writable by the process + * (e.g. the sqlite adapter's `dbPath`). + */ +const PRIVILEGED_ADAPTER_FIELDS = { + sqlite: ['dbPath'], +}; + +function getAdapterFields(adapterList, adapterId) { + if (!Array.isArray(adapterList)) return {}; + const entry = adapterList.find((a) => a && a.id === adapterId); + return (entry && entry.fields) || {}; +} + +/** + * Ensures a non-admin user cannot introduce or change privileged notification + * adapter fields (e.g. sqlite `dbPath`). If the incoming request tries to set + * such a field, its value is replaced with the stored (admin-set) value, or + * dropped entirely when no stored value exists. Admins are unaffected, so + * existing configurations continue to work unchanged. + */ +function sanitiseNotificationAdapter(notificationAdapter, existingJob, requestIsAdmin) { + if (requestIsAdmin || !Array.isArray(notificationAdapter)) return notificationAdapter; + + return notificationAdapter.map((adapter) => { + if (!adapter || typeof adapter !== 'object') return adapter; + const privilegedFields = PRIVILEGED_ADAPTER_FIELDS[adapter.id]; + if (!privilegedFields || !privilegedFields.length) return adapter; + + const existingFields = existingJob ? getAdapterFields(existingJob.notificationAdapter, adapter.id) : {}; + const fields = { ...(adapter.fields || {}) }; + for (const key of privilegedFields) { + if (Object.prototype.hasOwnProperty.call(existingFields, key)) { + fields[key] = existingFields[key]; + } else { + delete fields[key]; + } + } + return { ...adapter, fields }; + }); +} + function doesJobBelongsToUser(job, request) { const userId = request.session.currentUser; if (userId == null) return false; @@ -171,6 +216,8 @@ export default async function jobPlugin(fastify) { return reply.code(403).send({ error: 'Sorry, but you cannot change the Status of our Demo Job ;)' }); } + const safeNotificationAdapter = sanitiseNotificationAdapter(notificationAdapter, jobFromDb, isAdmin(request)); + jobStorage.upsertJob({ userId: request.session.currentUser, jobId, @@ -178,7 +225,7 @@ export default async function jobPlugin(fastify) { name, blacklist, provider, - notificationAdapter, + notificationAdapter: safeNotificationAdapter, shareWithUsers, spatialFilter, specFilter, From c0f0bd5be52957cef1591e0ce51478ed69f18cd7 Mon Sep 17 00:00:00 2001 From: Sebastion Date: Thu, 23 Jul 2026 19:56:56 +0100 Subject: [PATCH 2/2] fix(api/jobs): return 403 with clear error when non-admin sets privileged adapter field Previously the sanitiser silently stripped/overwrote privileged notification adapter fields (e.g. sqlite dbPath) for non-admin requests, leaving the user with no feedback about why their configuration didn't take effect. Now the jobs route rejects such requests with HTTP 403 and an explicit error message naming the offending adapter and field. Requests that omit the field, or echo back the admin-set value unchanged, are still accepted. --- lib/api/routes/jobRouter.js | 50 ++++++++++++++++++++++++------------- 1 file changed, 32 insertions(+), 18 deletions(-) diff --git a/lib/api/routes/jobRouter.js b/lib/api/routes/jobRouter.js index de0cdce1..a4e24ecd 100644 --- a/lib/api/routes/jobRouter.js +++ b/lib/api/routes/jobRouter.js @@ -33,30 +33,39 @@ function getAdapterFields(adapterList, adapterId) { /** * Ensures a non-admin user cannot introduce or change privileged notification - * adapter fields (e.g. sqlite `dbPath`). If the incoming request tries to set - * such a field, its value is replaced with the stored (admin-set) value, or - * dropped entirely when no stored value exists. Admins are unaffected, so - * existing configurations continue to work unchanged. + * adapter fields (e.g. sqlite `dbPath`). Returns either: + * { ok: true, notificationAdapter } — safe to persist + * { ok: false, adapterId, field } — non-admin attempted to set or + * change a privileged field + * + * Admins are unaffected, so existing configurations continue to work unchanged. + * Non-admin requests that omit privileged fields (or echo back the admin-set + * value unchanged) are also allowed through. */ function sanitiseNotificationAdapter(notificationAdapter, existingJob, requestIsAdmin) { - if (requestIsAdmin || !Array.isArray(notificationAdapter)) return notificationAdapter; + if (requestIsAdmin || !Array.isArray(notificationAdapter)) { + return { ok: true, notificationAdapter }; + } - return notificationAdapter.map((adapter) => { - if (!adapter || typeof adapter !== 'object') return adapter; + for (const adapter of notificationAdapter) { + if (!adapter || typeof adapter !== 'object') continue; const privilegedFields = PRIVILEGED_ADAPTER_FIELDS[adapter.id]; - if (!privilegedFields || !privilegedFields.length) return adapter; + if (!privilegedFields || !privilegedFields.length) continue; + const incomingFields = adapter.fields || {}; const existingFields = existingJob ? getAdapterFields(existingJob.notificationAdapter, adapter.id) : {}; - const fields = { ...(adapter.fields || {}) }; for (const key of privilegedFields) { - if (Object.prototype.hasOwnProperty.call(existingFields, key)) { - fields[key] = existingFields[key]; - } else { - delete fields[key]; - } + if (!Object.prototype.hasOwnProperty.call(incomingFields, key)) continue; + const incomingValue = incomingFields[key]; + const existingValue = existingFields[key]; + // Allow the no-op case where the client echoes back the admin-set value + // (common when editing a shared job through the UI). + if (incomingValue === existingValue) continue; + return { ok: false, adapterId: adapter.id, field: key }; } - return { ...adapter, fields }; - }); + } + + return { ok: true, notificationAdapter }; } function doesJobBelongsToUser(job, request) { @@ -216,7 +225,12 @@ export default async function jobPlugin(fastify) { return reply.code(403).send({ error: 'Sorry, but you cannot change the Status of our Demo Job ;)' }); } - const safeNotificationAdapter = sanitiseNotificationAdapter(notificationAdapter, jobFromDb, isAdmin(request)); + const sanitised = sanitiseNotificationAdapter(notificationAdapter, jobFromDb, isAdmin(request)); + if (!sanitised.ok) { + return reply.code(403).send({ + error: `You are not allowed to set the "${sanitised.field}" field on the "${sanitised.adapterId}" notification adapter. Please ask an administrator to configure this value.`, + }); + } jobStorage.upsertJob({ userId: request.session.currentUser, @@ -225,7 +239,7 @@ export default async function jobPlugin(fastify) { name, blacklist, provider, - notificationAdapter: safeNotificationAdapter, + notificationAdapter: sanitised.notificationAdapter, shareWithUsers, spatialFilter, specFilter,